From e8ec96725c4451aa64cb97230c18d04b55d98503 Mon Sep 17 00:00:00 2001 From: abrichr Date: Mon, 19 Oct 2026 09:41:16 -0400 Subject: [PATCH 1/4] feat(agent): teach a workflow over MCP, then call it with no model calls openadapt-agent serve --teach VERIFY_YAML adds teach_start, teach_observe, teach_act, teach_screen, teach_finish, teach_abort, teach_status, teach_confirm_checks and teach_admit. The connected agent is the inner agent of compile-agent's FirstRunCompiler (compile_agent.driven.DrivenRun): run 1 goes through the same harness and save gate, the record check reads the system of record, a person confirms the proposed check in the client's form, and teach_admit admits only as the registered agent the operator configured, under a person-signed policy (self-admission needs allow_self_admission). An admitted program joins list_workflows and run_workflow and gets a taught_ tool; the server sends tools/list_changed. FirstRunCompiler takes surface= (a deployment backend section) and uses a new SurfaceHarness off the web: same gating, and a person approves every gated action because the harness can't read which record a form shows. openadapt-flow compile-agent admit TASK_DIR is the person's terminal path. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/packages.yml | 3 +- CAPABILITIES.md | 6 +- packages/agent/README.md | 42 + packages/agent/docs/DESIGN.md | 9 + packages/flow/docs/BACKEND_PARITY.md | 13 +- packages/flow/docs/COMPILE_AGENT.md | 52 +- packages/flow/openadapt_flow/agent/bridge.py | 12 + packages/flow/openadapt_flow/agent/cli.py | 194 ++++ packages/flow/openadapt_flow/agent/mcp.py | 127 ++- packages/flow/openadapt_flow/agent/teach.py | 973 ++++++++++++++++++ packages/flow/openadapt_flow/authoring.py | 6 + .../flow/openadapt_flow/compile_agent/cli.py | 74 +- .../openadapt_flow/compile_agent/driven.py | 300 ++++++ .../openadapt_flow/compile_agent/harness.py | 286 +++++ .../openadapt_flow/compile_agent/pipeline.py | 81 +- packages/flow/tests/e2e/test_mcp_teach_e2e.py | 483 +++++++++ .../test_compile_agent_surface_harness.py | 240 +++++ packages/flow/tests/test_mcp_teach_cli.py | 164 +++ .../flow/tests/test_mcp_teach_surfaces.py | 283 +++++ 19 files changed, 3316 insertions(+), 32 deletions(-) create mode 100644 packages/flow/openadapt_flow/agent/teach.py create mode 100644 packages/flow/openadapt_flow/compile_agent/driven.py create mode 100644 packages/flow/tests/e2e/test_mcp_teach_e2e.py create mode 100644 packages/flow/tests/test_compile_agent_surface_harness.py create mode 100644 packages/flow/tests/test_mcp_teach_cli.py create mode 100644 packages/flow/tests/test_mcp_teach_surfaces.py diff --git a/.github/workflows/packages.yml b/.github/workflows/packages.yml index 1e18ac96e..8a22f01d5 100644 --- a/.github/workflows/packages.yml +++ b/.github/workflows/packages.yml @@ -363,7 +363,7 @@ jobs: python-version: "3.12" cache: pip - name: Install - run: pip install -e ../types -e '.[dev]' + run: pip install -e ../types -e '.[dev,agent]' - name: Resolve Playwright version id: pw run: | @@ -398,6 +398,7 @@ jobs: tests/test_admitted_composition_runtime.py \ tests/e2e/test_structural_action.py \ tests/e2e/test_compile_agent_e2e.py \ + tests/e2e/test_mcp_teach_e2e.py \ --basetemp=runs/ci-critical --junitxml=runs/critical-junit.xml - name: Fail if any critical test was skipped or none ran run: | diff --git a/CAPABILITIES.md b/CAPABILITIES.md index 60a08dd46..00b5c2140 100644 --- a/CAPABILITIES.md +++ b/CAPABILITIES.md @@ -21,7 +21,9 @@ shipping in the next release; a pull request number means it is being merged. the verified run into a program (`openadapt_flow.jit`, released; 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. + on web, native Windows, RDP and Citrix. An agent connected over MCP can + teach itself a new workflow and then call it as a tool + (`openadapt-agent serve --teach`, in review). 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 own traffic allows them (`ApiActuator`, released; compiled from captured @@ -202,6 +204,7 @@ improvising or retrying when something is off. | 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](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](packages/agent/README.md) | +| Teach over MCP: an agent does a new task once through OpenAdapt; the run is read back, compiled and admitted (a person confirms the record check; the agent admits only under a person-signed policy), and becomes a tool it calls with no model calls. Web, and Windows, macOS, Linux, RDP and Citrix with a person approving each save | In review | Synthetic (MockMed over MCP, and through the real RDP and Citrix backends) | `openadapt-agent serve --teach`, [packages/agent](packages/agent/README.md#teach-a-new-workflow-from-your-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)) | Synthetic | `compile-agent` | | 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/` | @@ -226,6 +229,7 @@ fails when a command is added without being listed here. - **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). - **Give agents the program:** `emit-skill`, `emit-mcp`, `serve-execute`. +- **Let an agent teach itself a workflow (MCP):** `openadapt-agent serve --teach` adds `teach_start`, `teach_observe`, `teach_act`, `teach_screen`, `teach_finish`, `teach_abort`, `teach_status`, `teach_confirm_checks`, `teach_admit`, and a `taught_` tool per admitted program; a person admits a waiting draft with `compile-agent admit`. - **Proof:** `seal`, `local-seal`, `countersign`, `attach-seal`. - **Run on premises:** `service`, `no-outbound`. - **Share safely with OpenAdapt Cloud:** `sanitize`, `review-sanitized`, `approve-sanitized`, `materialize-approved`, `push`, `validate-hosted`, `report-break`, `report-run`, `connect`, `login`, `connector`. diff --git a/packages/agent/README.md b/packages/agent/README.md index b03972ed7..b24472fae 100644 --- a/packages/agent/README.md +++ b/packages/agent/README.md @@ -238,6 +238,8 @@ asyncio.run(main()) The SDK starts the server with only a few environment variables. To pass others, such as `OPENADAPT_BUNDLE_KEY`, add `env=` to `StdioServerParameters`. +To let your agent add workflows itself, see [Teach a new workflow from your agent](#teach-a-new-workflow-from-your-agent). + [scripts/smoke_client.py](scripts/smoke_client.py) is a fuller example that runs every sandbox case. From a clone of this repository, run `pip install -e .`, then: ```bash @@ -384,6 +386,46 @@ ChatGPT and Claude in a web browser can't start a server on your computer. To re It claims the job, polls openadapt.ai over outgoing HTTPS, and asks you in the terminal before a chat account may drive it. Don't confuse it with the launcher's `openadapt connect`, which pairs a computer with OpenAdapt Cloud. [docs/MAILBOX_CLI.md](docs/MAILBOX_CLI.md) has the details. +### Teach a new workflow from your agent + +`serve --teach` lets your agent add a workflow it doesn't have yet, without leaving the conversation. It does the task once through OpenAdapt; OpenAdapt checks the record it saved, compiles the run, and once the run is admitted your agent calls it as a tool. Every later call runs the compiled program with no model calls and reads the save back. + +```bash +claude mcp add --scope user openadapt-teach -- uvx --python 3.12 --from 'openadapt-agent[tutorial]' openadapt-agent serve --teach verify.yaml --url https://emr-test.example.org --runs-dir ~/.openadapt/runs +``` + +`verify.yaml` needs only a `verifier:` section that reads the app's records (`rest`, `sql`, `fhir`, `graphql` or `file`), the same file `openadapt-flow record --verify` uses. The first run makes a real entry in the app, so point `--url` at a test copy with test records. + +Your agent then: + +1. Calls `teach_start` with the task (`{note}` for each input), this run's `inputs`, the `record_fields` the saved record must have (such as its patient id), and `on_screen`, the record's identity as the screen shows it (a name and an MRN). +2. Calls `teach_observe` and `teach_act` until the task is done. On the web, `teach_observe` lists the page as short lines with a ref for each control, and before every save OpenAdapt checks that the record's identity is shown next to the control. A save on the wrong record is refused and ends the run. +3. Calls `teach_finish`. OpenAdapt compiles the run and reads the app's records: exactly one record must have appeared, with every `record_fields` value and every input. The result is the draft: its steps, its inputs, the proposed record check, and what still needs a person. +4. Calls `teach_confirm_checks`. Your MCP client shows a person the proposed check, and only their answer there, with their name, confirms it. Your agent has no argument that confirms it. +5. Calls `teach_admit` with `canary_inputs`, the values each canary saves. A canary is a real save on the same record, so use values that are safe there. + +`teach_admit` admits only under the rules in [JIT_ADMISSION.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/JIT_ADMISSION.md). The operator starts the server with `--teach-key agent.pem --teach-policy signed-policy.json`: the key of the agent registered with `openadapt-flow jit register --role agent`, and an admission policy a person signed with `openadapt-flow jit sign-policy`. The server records the run as that agent, so admitting it is self-admission, which the policy must allow (`allow_self_admission`). Without a key and a policy, `teach_admit` refuses and returns the command a person runs at a terminal instead: + +```bash +openadapt-flow compile-agent admit --admit-as "" --canary-param note="Canary test note" +``` + +Once the program is admitted, it is in `list_workflows` and `run_workflow` at once and has its own `taught_` tool, and the server sends `notifications/tools/list_changed` to clients that support it. A server started later lists every program taught in its `--teach-dir`. + +It works on a web app (`--url`) and on the app a deployment file's `backend:` section names (`--config deploy.yaml`): a native Windows, macOS or Linux window, or a local RDP or Citrix client window. Off the web, OpenAdapt can't read which record a form shows, so a person approves every save in your client's form, and a client without forms can't save. RDP and Citrix have no control tree, and the Windows, macOS and Linux backends don't list controls yet, so off the web your agent needs `--teach-screens scrubbed` to see the screen, and clicks x,y pixels. Admission there fails closed for now: a pixel-only click carries no checked record identity, so the canary is refused and says why. Network RDP (`rdp_host`) is refused. + +| Flag | What it does | +| --- | --- | +| `--teach VERIFY_YAML` | Adds `teach_start`, `teach_observe`, `teach_act`, `teach_finish`, `teach_abort`, `teach_status`, `teach_confirm_checks` and `teach_admit`, and turns on runs of taught workflows. | +| `--teach-dir DIR` | Where drafts and taught programs live (default `RUNS_DIR/teach`). | +| `--teach-key PEM`, `--teach-policy JSON` | The registered agent's encrypted key (passphrase in `OPENADAPT_JIT_KEY_PASSPHRASE`) and the signed admission policy. Only an agent key under a policy; an admitter key admits from CI with `openadapt-flow jit admit`. | +| `--key-dir DIR` | The JIT key directory (default `OPENADAPT_JIT_KEY_DIR` or `~/.openadapt/jit`). | +| `--teach-hold` | On the web, a person also approves every save in the first run. | +| `--teach-screens scrubbed` | Adds `teach_screen`: the app's screen with identifiers scrubbed, and nothing shared if scrubbing fails. `unscrubbed` skips scrubbing. Off by default. | +| `--mode production --bundles DIR` | Serves your other workflows next to the taught ones. | + +[COMPILE_AGENT.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/COMPILE_AGENT.md) describes the pipeline underneath: the harness, the record check, the canaries and the files it keeps. + ## Security - The server talks only over stdio and has no sign-in. Anything that can write to its input acts as the OS account that started it, so don't expose it over a network or a tunnel. diff --git a/packages/agent/docs/DESIGN.md b/packages/agent/docs/DESIGN.md index a0859150c..506a5911e 100644 --- a/packages/agent/docs/DESIGN.md +++ b/packages/agent/docs/DESIGN.md @@ -219,6 +219,7 @@ repeats the text. Every declared input stays required unless | `--mode production --bundles DIR` (or `--allow-run`) | production | the above plus `run_workflow` and deprecated `run_` | | `--mode attended --bundles DIR` (or `--allow-run --allow-attended-actions`) | attended | the above plus Reject, Teach, Escalate; Continue and Skip with a qualified `--config` | | `--bundles DIR --allow-attended-actions` (older flags, no runs) | attended, read-only | the read-only tools plus the same decisions on paused runs | +| `--teach VERIFY_YAML` with `--url` or `--config` | production (taught programs) | the production tools plus `teach_start`, `teach_observe`, `teach_act`, `teach_finish`, `teach_abort`, `teach_status`, `teach_confirm_checks`, `teach_admit`, `teach_screen` (with `--teach-screens`), and `taught_` for each admitted program | The read-only tools return PHI-safe projections: workflow cards, opaque ids, availability, status, and count or boolean metrics. They do not @@ -227,6 +228,14 @@ observed text, local paths, subprocess output, or exception text. The attention tools use the same boundary plus typed categories, artifact ids, and non-authorizing capability metadata. +The teach tools are the exception. While the agent does run 1 of a task, +`teach_observe` and `teach_act` return what it needs to find the right record: +on the web, the page's text and controls, scrubbed by +`openadapt_flow.privacy` except for the values the agent put in the task. Run +1 makes a real entry, so the operator points it at a test copy. The draft +report, the record check and the admission return the same kind of facts as +the other tools, plus the proposed record check's field names and values. + The deprecated per-workflow `run_` tools keep their schemas and legacy `status` field and add the contract fields. They take no `request_id`, so their descriptions say a retry can write twice. Missing diff --git a/packages/flow/docs/BACKEND_PARITY.md b/packages/flow/docs/BACKEND_PARITY.md index 24e2df2ff..b59cb3046 100644 --- a/packages/flow/docs/BACKEND_PARITY.md +++ b/packages/flow/docs/BACKEND_PARITY.md @@ -99,6 +99,7 @@ drives an app). | `effects.kind: desktop_readback` ([#1280](https://github.com/OpenAdaptAI/OpenAdapt/pull/1280)) | refused: "a web record page uses effects.kind: ui_readback" | tested (UI Automation, WinForms stand-in) | works (OCR; Accessibility tree not read yet) | works (OCR; AT-SPI tree not read yet) | tested (network RDP, OCR) | works (OCR) | | `openadapt-agent serve --authoring` (pinning a window) | tested | tested | tested (frontmost window only) | **refused** | tested | tested | | `openadapt-agent` run tools | tested | works (surface from `--config`) | works | works | works | works | +| `openadapt-agent serve --teach` (record, check, compile, admit over MCP) | tested (`tests/e2e/test_mcp_teach_e2e.py`) | works (a person approves every save) | works (same) | works (same; needs `linux_allow_physical_input`) | tested; admission refused: pixel identity | tested; admission refused: pixel identity | - `SurfaceApp`: `tests/test_jit_surface_app.py` (every kind: validation, origin binding, Citrix readiness refusal) and @@ -128,6 +129,16 @@ drives an app). `--authoring --config deploy.yaml --authoring-from-config` for every surface except network RDP, which it refuses (each run's own session would disconnect the recording's). +- `serve --teach`: `tests/test_mcp_teach_surfaces.py` teaches MockMed over + MCP through the real `RemoteDisplayBackend` and `CitrixWorkspaceBackend`: + run 1 records, passes its record check and a person confirms it; the canary + is refused with the identity-coverage reason above, so nothing is admitted. + `tests/test_compile_agent_surface_harness.py` covers the surface harness on + a native window (what is gated, nothing performed when refused). Off the + web the harness can't read which record a form shows, so a person approves + every gated action in the client's form. Native backends list no controls + yet (the `authoring_tree()` gap below), so the agent reads the screen + (`--teach-screens`) and clicks pixels there too. - An agent clicks at window coordinates. Native Linux refuses coordinate input unless the section sets `linux_allow_physical_input: true` ("coordinate input is disabled for native Linux"), so agent-driven @@ -165,5 +176,5 @@ are not app URLs. | `route` on a desktop surface: no test | [#1264](https://github.com/OpenAdaptAI/OpenAdapt/pull/1264) | [#1288](https://github.com/OpenAdaptAI/OpenAdapt/pull/1288) | | `induce --probe-on` on a desktop test copy: no test | [#1276](https://github.com/OpenAdaptAI/OpenAdapt/pull/1276) | [#1289](https://github.com/OpenAdaptAI/OpenAdapt/pull/1289) | | `run --self-repair` refuses non-web: "only browser runs can be repaired for now" | [#1265](https://github.com/OpenAdaptAI/OpenAdapt/pull/1265) | refused with a reason; desktop support after it merges | -| `compile-agent` takes `--backend`/`--config` but refuses every non-web target (exit 2, before anything runs); its observe step and save gate read the DOM | [#1277](https://github.com/OpenAdaptAI/OpenAdapt/pull/1277) | refused with a reason; desktop support (via `jit.SurfaceApp`) after it merges | +| `compile-agent` takes `--backend`/`--config` but refuses every non-web target (exit 2, before anything runs); its Claude agent reads the DOM listing | [#1277](https://github.com/OpenAdaptAI/OpenAdapt/pull/1277) | refused with a reason; the same pipeline runs on every surface through `serve --teach` (`FirstRunCompiler(surface=...)` with the surface harness) | | The GTK3 AT-SPI qualification job is in `packages/flow/.github/`, which GitHub does not run in the monorepo | `.github/workflows/` | follow-up | diff --git a/packages/flow/docs/COMPILE_AGENT.md b/packages/flow/docs/COMPILE_AGENT.md index 829bee325..e1c22f4fb 100644 --- a/packages/flow/docs/COMPILE_AGENT.md +++ b/packages/flow/docs/COMPILE_AGENT.md @@ -188,7 +188,44 @@ later.verified, later.model_calls, later.receipt_outcome # True, 0, "VERIFIED" `FirstRunCompiler` exposes the same steps one at a time (`record`, `confirm_checks`, `admit`, `emit`, `run`) for a flow where a person confirms -later. +later. A draft waiting for a person is admitted at a terminal with: + +```bash +openadapt-flow compile-agent admit --admit-as "Dana Reviewer" \ + --canary-param note="Canary test note" +``` + +It shows the person the proposed checks (unless someone confirmed them +already), the steps, and the canary result, and asks them to confirm each. +`--admit-key` admits with a registered key instead, once the checks are +confirmed. + +## An agent over MCP + +`openadapt-agent serve --teach verify.yaml` puts this pipeline behind MCP +tools, so an agent connected to the server (Claude Code, Codex, any MCP +client) teaches a task without leaving its conversation: `teach_start`, +`teach_observe` and `teach_act` for run 1, `teach_finish` for the record check +and the draft, `teach_confirm_checks` for a person to confirm the checks in the +client's form, and `teach_admit` for the canaries and the admission. The agent +is the inner agent here: `compile_agent.driven.DrivenRun` runs +`FirstRunCompiler.record` on a thread of its own and hands each tool call's +action to the harness. Admission follows [JIT_ADMISSION.md](JIT_ADMISSION.md): +the server records as the registered agent whose key the operator gave it, +so admitting is self-admission and needs a policy with +`allow_self_admission`; without a key and policy, a person admits with the +command above. An admitted program joins the server's `list_workflows` and +gets a `taught_` tool. See the +[agent README](../../agent/README.md#teach-a-new-workflow-from-your-agent). + +The MCP tools also run on Windows, macOS, Linux, RDP and Citrix: pass +`FirstRunCompiler(surface=)`. Off the web the +harness is `SurfaceHarness`. It reads the authoring session's +accessibility tree (no values, no window titles) and gates the same actions, +but it can't read which form or row a control is in, so a person approves +every gated action (the hold); without a hold, a gated action is refused. On +RDP and Citrix the canary is refused until pixel identity is qualified +([BACKEND_PARITY.md](BACKEND_PARITY.md)). Any agent works if it has a `dialect` (an `openadapt_flow.jit` action format) and `drive(request, harness)`. `ScriptedAgent` follows a list of steps by @@ -229,13 +266,12 @@ object with `messages.create`; the tests pass a fake. ## Limits -- Web apps only so far. The JIT captures and runs through a Playwright - browser, and the harness reads the page's DOM. A Windows, macOS, Linux, RDP - or Citrix target (`--backend`, or a `--config` whose `backend.kind` names - one) and any URL that is not http(s) are refused before anything runs; there - is no fallback to a browser. Those surfaces need the JIT to drive the - backends `record --backend` uses and an observer over UI Automation, - Accessibility, AT-SPI or pixels. +- The `compile-agent` command runs web apps only so far: its Claude agent + reads the page's DOM listing. A Windows, macOS, Linux, RDP or Citrix target + (`--backend`, or a `--config` whose `backend.kind` names one) and any URL + that is not http(s) are refused before anything runs; there is no fallback + to a browser. The MCP teach tools run the same pipeline on those surfaces + (above). - The save gate recognizes a click that commits by the control's label. An icon with no label, or a label that names no write ("OK"), is not gated in run 1; the record check after run 1 still refuses a wrong record, but only diff --git a/packages/flow/openadapt_flow/agent/bridge.py b/packages/flow/openadapt_flow/agent/bridge.py index 1086471e7..41759c683 100644 --- a/packages/flow/openadapt_flow/agent/bridge.py +++ b/packages/flow/openadapt_flow/agent/bridge.py @@ -237,6 +237,18 @@ def __init__( entry.legacy_id: entry for entry in self.catalog.values() if entry.legacy_id } + def add_catalog_entry(self, entry: CatalogEntry) -> None: + """Add a workflow while the server runs (a program taught over MCP). + + The run service sees it at once; ``list_workflows`` and the + ``run_workflow`` schema list it from the next call. + """ + if entry.name in self.catalog: + raise BridgeError("a workflow with that name already exists") + self.catalog[entry.name] = entry + if self._service is not None: + self._service.catalog[entry.name] = entry + @property def service(self) -> RunService: """The run service, created on first use so read-only servers stay inert.""" diff --git a/packages/flow/openadapt_flow/agent/cli.py b/packages/flow/openadapt_flow/agent/cli.py index 19ba47932..e05e722ea 100644 --- a/packages/flow/openadapt_flow/agent/cli.py +++ b/packages/flow/openadapt_flow/agent/cli.py @@ -195,6 +195,68 @@ def build_parser() -> argparse.ArgumentParser: "save must produce. Needs --guard-tasks and --guard-state." ), ) + p.add_argument( + "--teach", + default=None, + metavar="VERIFY_YAML", + help=( + "Add the teach tools: the agent does a task once in the app at --url " + "(or the --config backend: Windows, macOS, Linux, RDP, Citrix), " + "Flow checks the saved record through VERIFY_YAML's verifier, " + "compiles the run, and once it is admitted the agent calls it as a " + "tool with no model calls. Run 1 makes a real entry: use a test copy." + ), + ) + p.add_argument( + "--teach-dir", + default=None, + metavar="DIR", + help="With --teach: where drafts and taught programs live (default RUNS_DIR/teach)", + ) + p.add_argument( + "--teach-key", + default=None, + metavar="PEM", + help=( + "With --teach and --teach-policy: the encrypted key of the registered " + "agent that may admit what it taught (passphrase from " + "$OPENADAPT_JIT_KEY_PASSPHRASE). Without it, a person admits at a " + "terminal (openadapt-flow compile-agent admit)." + ), + ) + p.add_argument( + "--teach-policy", + default=None, + metavar="JSON", + help=( + "With --teach-key: the admission policy a person signed " + "(openadapt-flow jit sign-policy). The agent admits its own draft " + "only if it sets allow_self_admission." + ), + ) + p.add_argument( + "--key-dir", + default=None, + metavar="DIR", + help="With --teach: the JIT key directory (default $OPENADAPT_JIT_KEY_DIR or ~/.openadapt/jit)", + ) + p.add_argument( + "--teach-hold", + action="store_true", + help=( + "With --teach on a web app: a person also approves every save in " + "the client's form (always on for other surfaces)." + ), + ) + p.add_argument( + "--teach-screens", + choices=("off", "scrubbed", "unscrubbed"), + default="off", + help=( + "With --teach: add teach_screen so the agent sees the app (RDP and " + "Citrix need it). scrubbed shares nothing if scrubbing fails." + ), + ) p.add_argument( "--guard-tasks", default=None, @@ -587,6 +649,17 @@ def _cmd_serve(args: argparse.Namespace) -> int: if args.guard: return _serve_guard(args, serve) + if args.teach: + return _serve_teach(args, serve) + if ( + args.teach_key + or args.teach_policy + or args.teach_dir + or args.teach_hold + or args.teach_screens != "off" + ): + print("serve: the --teach-* flags need --teach VERIFY_YAML", file=sys.stderr) + return 2 desktop_pin = bool( args.windows_agent_url or args.remote_window @@ -905,6 +978,127 @@ def _serve_guard(args: argparse.Namespace, serve) -> int: return 0 +def _teach_app(args: argparse.Namespace) -> tuple[Optional[str], Any]: + """The app to teach in: (url, None) for a web app, (None, backend section) + for any other surface.""" + + if not args.config: + if not args.url: + raise ValueError("--teach needs the app: --url, or --config deploy.yaml") + return args.url, None + from openadapt_flow.deployment import load_deployment + + section = load_deployment(args.config).backend + if (section.kind or "web").strip().lower() == "web": + url = args.url or getattr(section, "url", None) + if not url: + raise ValueError("the deployment names a web app; pass --url") + return url, None + if args.url: + raise ValueError("--url names a web app; the deployment names another surface") + if section.kind == "rdp" and section.rdp_host: + raise ValueError( + "network RDP (rdp_host) can't be taught; use the local client window " + "(rdp_window), as for authoring" + ) + return None, section + + +def _serve_teach(args: argparse.Namespace, serve) -> int: + """Serve the teach tools next to the workflow tools over local stdio.""" + from openadapt_flow.agent.bridge import AgentBridge + from openadapt_flow.agent.runner import default_flow_cli + from openadapt_flow.agent.teach import TeachBridge, TeachConfig + + if args.authoring or args.tutorial or args.mode in ("sandbox", "attended"): + print( + "serve: --teach has its own recording tools; it can't be combined " + "with --authoring, --tutorial, or --mode sandbox/attended", + file=sys.stderr, + ) + return 2 + if bool(args.teach_key) != bool(args.teach_policy): + print("serve: --teach-key and --teach-policy go together", file=sys.stderr) + return 2 + if args.bundles and args.mode != "production": + print( + "serve: with --teach, --bundles needs --mode production", + file=sys.stderr, + ) + return 2 + try: + from openadapt_flow.record_sor import load_observer + + url, surface = _teach_app(args) + # Read the verifier now: a typo fails here, not at the agent's first task. + load_observer(Path(args.teach).expanduser().resolve()) + key_dir = Path(args.key_dir).expanduser().resolve() if args.key_dir else None + admitter = None + if args.teach_key: + from openadapt_flow.jit import authority, lifecycle + + admitter = authority.principal_from_key( + authority.load_encrypted_key(Path(args.teach_key)), + key_dir or lifecycle.default_key_dir(), + policy=authority.SignedAdmissionPolicy.load(args.teach_policy), + ) + runs_dir = ( + (Path(args.runs_dir) if args.runs_dir else default_runs_dir()) + .expanduser() + .resolve() + ) + config = TeachConfig( + verify=Path(args.teach).expanduser().resolve(), + cache_dir=( + Path(args.teach_dir).expanduser().resolve() + if args.teach_dir + else runs_dir / "teach" + ), + url=url, + surface=surface, + key_dir=key_dir, + admitter=admitter, + hold=args.teach_hold, + headless=not args.headed, + screens=args.teach_screens, + ) + except Exception as exc: # noqa: BLE001 - every setup problem is a refusal + print(f"serve: {exc}", file=sys.stderr) + return 2 + runner_config = RunnerConfig( + flow_cli=( + tuple(shlex.split(args.flow_cli)) if args.flow_cli else default_flow_cli() + ), + runs_dir=runs_dir, + url=args.url, + deployment_config=args.config, + policy=args.policy, + timeout_s=args.timeout, + approval_mode=args.approval_mode, + ) + bridge = AgentBridge( + Path(args.bundles) if args.bundles else None, + runner_config, + allow_run=True, + mode="production", + ) + teach = TeachBridge(config, workflows=bridge) + print( + f"openadapt-agent {__version__}: teach tools over local stdio on the " + f"{teach.backend_kind} app; admission " + + ( + f"by agent {admitter.principal_id} under policy " + f"{admitter.policy.policy.policy_id}" + if admitter is not None + else "by a person at a terminal" + ) + + f"; {len(teach.taught)} taught workflow(s); runs folder {runs_dir}", + file=sys.stderr, + ) + serve(bridge, teach=teach) + return 0 + + def _serve(serve, bridge, authoring): """Call serve without surprising 1-arg monkeypatches in existing tests.""" if authoring is None: diff --git a/packages/flow/openadapt_flow/agent/mcp.py b/packages/flow/openadapt_flow/agent/mcp.py index 67324b7c1..99fa43848 100644 --- a/packages/flow/openadapt_flow/agent/mcp.py +++ b/packages/flow/openadapt_flow/agent/mcp.py @@ -122,6 +122,50 @@ async def _confirm_with_person( raise BridgeError(declined) +async def _ask_person_name(context: Any, message: str) -> str: + """Ask the person for their name and a confirmation in the client's form.""" + session = context.session + if not client_supports_form_elicitation(session): + raise BridgeError( + "confirming the record check needs a person in an MCP client with " + "form elicitation; nothing was confirmed. A person can also run " + "`openadapt-flow compile-agent admit TASK_DIR` at a terminal" + ) + result = await session.elicit_form( + message, + { + "type": "object", + "properties": { + "name": {"type": "string", "title": "Your name"}, + "confirmed": { + "type": "boolean", + "title": "I am a person and I confirm this record check", + }, + }, + "required": ["name", "confirmed"], + }, + related_request_id=context.request_id, + ) + content = result.content or {} + name = content.get("name") + if ( + result.action != "accept" + or content.get("confirmed") is not True + or not isinstance(name, str) + or not name.strip() + ): + raise BridgeError("the person declined or cancelled; nothing was confirmed") + return name.strip() + + +async def _notify_tools_changed(context: Any) -> None: + """Tell a client that supports it that the tool list changed.""" + try: + await context.session.send_tool_list_changed() + except Exception: # noqa: BLE001 - the next tools/list shows it anyway + _LOG.debug("could not send tools/list_changed", exc_info=True) + + async def _confirm_attended_action(context: Any, name: str) -> None: """Require a second, protocol-native human confirmation before mutation.""" await _confirm_with_person( @@ -180,6 +224,7 @@ def _server_instructions( authoring: AuthoringBridge | None, bridge: AgentBridge | None = None, guard: Any = None, + teach: Any = None, ) -> str: text = "" if bridge is not None: @@ -236,6 +281,16 @@ def _server_instructions( from openadapt_flow.agent.guard_tools import GUARD_INSTRUCTIONS text += " " + GUARD_INSTRUCTIONS + if 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. " + "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. " + "Once admitted, it is in list_workflows and has a taught_ tool " + "that runs with no model calls." + ) return text.strip() @@ -243,12 +298,15 @@ def build_server( bridge: AgentBridge | None = None, authoring: AuthoringBridge | None = None, guard: Any = None, + teach: Any = None, ) -> Server: - """Wrap workflow, authoring and/or guard bridges in an MCP Server (no I/O started). + """Wrap workflow, authoring, guard and/or teach bridges in an MCP Server + (no I/O started). - ``guard`` is a :class:`~openadapt_flow.agent.guard_tools.GuardBridge`. + ``guard`` is a :class:`~openadapt_flow.agent.guard_tools.GuardBridge`; + ``teach`` is a :class:`~openadapt_flow.agent.teach.TeachBridge`. """ - if bridge is None and authoring is None and guard is None: + if bridge is None and authoring is None and guard is None and teach is None: raise ValueError( "MCP server requires a workflow, authoring or guarded-save bridge" ) @@ -258,6 +316,7 @@ def _tool_specs(): *(bridge.list_tool_specs() if bridge is not None else ()), *(authoring.list_tool_specs() if authoring is not None else ()), *(guard.list_tool_specs() if guard is not None else ()), + *(teach.list_tool_specs() if teach is not None else ()), ) async def _list_tools(context: Any = None) -> list[types.Tool]: @@ -303,6 +362,9 @@ async def _call_tool( or "tool arguments do not match the input schema" ) from None human_confirmed = False + teaching = teach is not None and teach.handles(name) + held_by_person: str | None = None + person: str | None = None admission_by_person = ( name == "decide_admission" and bridge is not None @@ -317,11 +379,36 @@ async def _call_tool( elif name == "admit" and authoring is not None and authoring.handles(name): await _confirm_admit(context, authoring) human_confirmed = True + elif teaching and name == "teach_act": + commit = await anyio.to_thread.run_sync( + teach.hold_needed, arguments or {} + ) + if commit: + await _confirm_with_person( + context, + teach.hold_message(commit), + title="I am a person and I approve this save", + unsupported=( + "this save needs a person's approval in an MCP client " + "with form elicitation; nothing was done" + ), + declined="the person did not approve the save; nothing was done", + ) + held_by_person = commit + elif teaching and name == "teach_confirm_checks": + message = await anyio.to_thread.run_sync( + teach.confirm_checks_message, arguments or {} + ) + person = await _ask_person_name(context, message) def call() -> dict[str, Any]: payload = dict(arguments or {}) if guard is not None and guard.handles(name): return guard.dispatch(name, payload) + if teaching: + return teach.dispatch( + name, payload, held_by_person=held_by_person, person=person + ) if authoring is not None and authoring.handles(name): if human_confirmed: return authoring.dispatch(name, payload, human_confirmed=True) @@ -338,6 +425,12 @@ def call() -> dict[str, Any]: # actions synchronously submit to their own non-async backend-owner # thread, so every MCP call can leave the event loop responsive. result = await anyio.to_thread.run_sync(call) + if ( + teaching + and name == "teach_admit" + and result.get("status") == "admitted" + ): + await _notify_tools_changed(context) except BridgeError as exc: return types.CallToolResult( content=[ @@ -378,7 +471,7 @@ def call() -> dict[str, Any]: server = Server( SERVER_NAME, version=__version__, - instructions=_server_instructions(authoring, bridge, guard), + instructions=_server_instructions(authoring, bridge, guard, teach), ) async def _list_v1() -> list[types.Tool]: @@ -403,10 +496,27 @@ async def _call_v2(context: Any, params: types.CallToolRequestParams): server = Server( SERVER_NAME, version=__version__, - instructions=_server_instructions(authoring, bridge, guard), + instructions=_server_instructions(authoring, bridge, guard, teach), on_list_tools=_list_v2, on_call_tool=_call_v2, ) + if teach is not None: + # A taught workflow adds a tool while the server runs: say so in the + # capabilities, so clients listen for tools/list_changed. + from mcp.server.lowlevel.server import NotificationOptions + + initialization_options = server.create_initialization_options + + def _with_tools_changed( + notification_options: Any = None, *args: Any, **kwargs: Any + ): + return initialization_options( + notification_options or NotificationOptions(tools_changed=True), + *args, + **kwargs, + ) + + server.create_initialization_options = _with_tools_changed # type: ignore[method-assign] return server @@ -440,8 +550,9 @@ async def _run_stdio( bridge: AgentBridge | None, authoring: AuthoringBridge | None = None, guard: Any = None, + teach: Any = None, ) -> None: - server = build_server(bridge, authoring=authoring, guard=guard) + server = build_server(bridge, authoring=authoring, guard=guard, teach=teach) with _protected_stdout() as wire: kwargs = {"stdout": wire} if wire is not None else {} async with stdio_server(**kwargs) as (read_stream, write_stream): @@ -454,8 +565,12 @@ def serve( bridge: AgentBridge | None = None, authoring: AuthoringBridge | None = None, guard: Any = None, + teach: Any = None, ) -> None: """Serve the bridge over stdio until the client disconnects.""" + if teach is not None: + anyio.run(_run_stdio, bridge, authoring, guard, teach) + return if guard is None: anyio.run(_run_stdio, bridge, authoring) return diff --git a/packages/flow/openadapt_flow/agent/teach.py b/packages/flow/openadapt_flow/agent/teach.py new file mode 100644 index 000000000..567138cd3 --- /dev/null +++ b/packages/flow/openadapt_flow/agent/teach.py @@ -0,0 +1,973 @@ +"""Teach tools: an MCP agent turns a task it did once into a workflow it calls. + +``openadapt-agent serve --teach VERIFY_YAML`` adds these tools to the server: + +``teach_start`` + Opens the operator's app (``--url`` or ``--config``) and starts run 1 of a + task the agent names, with its inputs, the record fields the save must + produce, and the record's identity as the screen shows it. +``teach_observe``, ``teach_act``, ``teach_screen`` + The agent does the task one action at a time. Every action goes through + the compile-agent harness (:mod:`openadapt_flow.compile_agent.harness`): + on the web it checks that the right record is on screen before each save; + on Windows, macOS, Linux, RDP and Citrix it can't, so a person approves + each save in the client's form. +``teach_finish`` + Ends run 1. Flow compiles it, reads the system of record back, and reports + the draft: its steps, its parameters, the proposed record check, and what + still needs a person. +``teach_confirm_checks`` + A person confirms the proposed record check in the client's form. An + agent can't: the form asks the person for their name. +``teach_admit`` + Runs the canaries and admits the draft under the JIT rules + (``packages/flow/docs/JIT_ADMISSION.md``): the agent admits only as the + registered agent the operator configured, under a policy a person signed. + It records as that agent, so admitting is self-admission, which the policy + must allow (``allow_self_admission``). Without a key and a policy, a + person admits at a terminal. + +An admitted program joins ``list_workflows`` and ``run_workflow`` at once, and +gets its own ``taught_`` tool; the server tells clients that support it +that its tool list changed. Each run executes the program with no model calls +and reads every save back. + +The pipeline is :class:`~openadapt_flow.compile_agent.FirstRunCompiler`. This +module only adapts it to MCP calls; it decides nothing about admission. +""" + +from __future__ import annotations + +import base64 +import re +import threading +from dataclasses import dataclass +from pathlib import Path +from typing import Any, Callable, Mapping, Optional + +from openadapt_flow.agent.bridge import BridgeError, ToolSpec + +__all__ = ["TEACH_TOOLS", "TaughtEngine", "TeachBridge", "TeachConfig"] + +TEACH_TOOLS = ( + "teach_start", + "teach_observe", + "teach_act", + "teach_screen", + "teach_finish", + "teach_abort", + "teach_status", + "teach_confirm_checks", + "teach_admit", +) +#: Tools that change state only after a person answers the client's form. +TEACH_PERSON_TOOLS = frozenset({"teach_confirm_checks"}) +TAUGHT_PREFIX = "taught_" + +_TASK_KEY = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$") +_NAME = re.compile(r"^[a-z][a-z0-9_]{0,31}$") + +_READ_ONLY = { + "readOnlyHint": True, + "destructiveHint": False, + "idempotentHint": True, + "openWorldHint": False, +} +_ACTS = { + "readOnlyHint": False, + "destructiveHint": True, + "idempotentHint": False, + "openWorldHint": True, +} +_LOCAL = { + "readOnlyHint": False, + "destructiveHint": False, + "idempotentHint": True, + "openWorldHint": False, +} + + +def _obj(properties: dict[str, Any], required: list[str]) -> dict[str, Any]: + return { + "type": "object", + "properties": properties, + "required": required, + "additionalProperties": False, + } + + +_TASK_KEY_PROP = { + "type": "string", + "description": "task_key from teach_start or teach_finish.", +} + + +@dataclass +class TeachConfig: + """What the operator fixed at server start. The agent changes none of it.""" + + verify: Path + cache_dir: Path + url: Optional[str] = None + #: A deployment ``backend:`` section for a native or remote app. + surface: Any = None + key_dir: Optional[Path] = None + #: A registered agent principal under a signed admission policy, or None. + admitter: Any = None + #: Ask a person before every save on the web too (always on elsewhere). + hold: bool = False + policy: str = "clinical-write" + headless: bool = True + #: Whether the agent sees the screen (``teach_screen``): ``off``, + #: ``scrubbed`` (identifiers scrubbed; nothing is shared if scrubbing + #: fails) or ``unscrubbed``. Off unless the operator turns it on. + screens: str = "off" + #: Tests only: the screen scrubber (default: openadapt-privacy's). + scrubber_factory: Optional[Callable[[], Any]] = None + #: Tests only: reset a demo app before every run. + reset: Optional[Callable[[], None]] = None + #: Tests only: build the live backend for a surface. + open_backend: Optional[Callable[[Any], Any]] = None + + def __post_init__(self) -> None: + self.cache_dir = Path(self.cache_dir).resolve() + if self.screens not in ("off", "scrubbed", "unscrubbed"): + raise ValueError("screens is off, scrubbed or unscrubbed") + if (self.url is None) == (self.surface is None): + raise ValueError("teach needs the app: a --url, or a --config backend") + if self.admitter is not None: + if getattr(self.admitter, "kind", None) != "agent" or ( + getattr(self.admitter, "policy", None) is None + ): + raise ValueError( + "the teach server admits only as a registered agent under a " + "signed admission policy (--teach-key with --teach-policy); " + "an admitter key admits from CI with openadapt-flow jit admit" + ) + + +def _off_thread(fn: Callable[[], Any]) -> Any: + """Run ``fn`` on a fresh thread: browsers and native backends it opens + live and close there, away from the server's worker pool.""" + + box: dict[str, Any] = {} + + def main() -> None: + try: + box["value"] = fn() + except BaseException as exc: # noqa: BLE001 - re-raised below + box["error"] = exc + + thread = threading.Thread(target=main, name="openadapt-teach", daemon=True) + thread.start() + thread.join() + if "error" in box: + raise box["error"] + return box.get("value") + + +class TaughtEngine: + """Runs an admitted taught program for :class:`RunService`: no model + calls, every save read back, a verify-run receipt.""" + + def __init__(self, task_dir: Path, config: TeachConfig) -> None: + self.task_dir = Path(task_dir) + self.config = config + + def execute( + self, entry: Any, run_id: str, inputs: dict[str, str], raw: Mapping[str, Any] + ) -> Any: + from openadapt_flow.agent.service import EngineResult + from openadapt_flow.compile_agent import FirstRunCompiler + + compiler = FirstRunCompiler.load( + self.task_dir, + reset=self.config.reset, + open_backend=self.config.open_backend, + ) + replay = compiler.run(**inputs) + if replay.verified: + reason = "saved_and_checked" + elif replay.status == "not_admitted": + reason = "workflow_unavailable" + elif replay.status == "refused": + reason = "not_ready_to_run" + elif replay.receipt_outcome == "HALTED_BEFORE_EFFECT": + reason = "stopped_for_review" + else: + # Anything else may have written: a person checks the record. + reason = "save_not_confirmed" + return EngineResult( + reason=reason, + execution_outcome=replay.status if replay.status.isupper() else None, + transaction_outcome=replay.receipt_outcome, + model_calls=replay.model_calls, + seconds=replay.seconds, + run_dir=str(replay.run_dir) if replay.run_dir else None, + bundle_dir=str(self.task_dir / "program" / "bundle"), + ) + + def refresh(self, entry: Any, run_id: str) -> Any: + return None + + +class TeachBridge: + """The teach tools over :class:`FirstRunCompiler`, one run 1 at a time.""" + + def __init__(self, config: TeachConfig, workflows: Any = None) -> None: + self.config = config + self.workflows = workflows + self.config.cache_dir.mkdir(parents=True, exist_ok=True) + self._run: Any = None + self._compiler: Any = None + self._lock = threading.Lock() + #: taught tool name -> catalog workflow name + self.taught: dict[str, str] = {} + for task_dir in sorted(self.config.cache_dir.iterdir()): + if self._is_admitted(task_dir): + self._register(task_dir) + + # -- tool surface ---------------------------------------------------------- + + def handles(self, name: str) -> bool: + if name == "teach_screen": + return self.config.screens != "off" + return name in TEACH_TOOLS or name in self.taught + + @property + def backend_kind(self) -> str: + if self.config.surface is None: + return "web" + from openadapt_flow.jit.compile_agent import SurfaceApp + + return SurfaceApp(self.config.surface).kind + + def list_tool_specs(self) -> list[ToolSpec]: + on_web = self.backend_kind == "web" + specs = [ + ToolSpec( + name="teach_start", + description=( + "Teach OpenAdapt a task by doing it once. OpenAdapt opens the " + f"app this server was set up for ({self.backend_kind}) and " + "records what you do; you then call teach_observe and " + "teach_act until the task is done, and teach_finish. This " + "run makes a real entry in that app, so use test records. " + "task is the instruction with {name} for each input; inputs " + "are this run's values; record_fields are fields the saved " + "record must have (such as its patient id); on_screen is " + "the record's identity as the screen shows it (a name, an " + "MRN), checked before every save. If the task was already " + "taught, nothing runs and you get its status." + ), + input_schema=_obj( + { + "task": {"type": "string", "minLength": 4, "maxLength": 500}, + "inputs": { + "type": "object", + "additionalProperties": {"type": "string"}, + }, + "record_fields": { + "type": "object", + "minProperties": 1, + "additionalProperties": {"type": "string"}, + }, + "on_screen": { + "type": "array", + "minItems": 1, + "items": {"type": "string", "minLength": 1}, + }, + "name": { + "type": "string", + "pattern": _NAME.pattern, + "description": "Short name for the workflow (a-z, 0-9, _).", + }, + }, + ["task", "inputs", "record_fields", "on_screen"], + ), + annotations=_ACTS, + ), + ToolSpec( + name="teach_observe", + description=( + "Read the app again. " + + ( + "You get the page as short lines: headings, text, table " + "rows, and every control with a ref such as [e3]." + if on_web + else "You get one line per control with a ref such as " + "[n_1a2b3c4d]; a remote session has none, so use " + "teach_screen and click x,y." + ) + + " Refs change after every action." + ), + input_schema=_obj({}, []), + annotations=_READ_ONLY, + ), + ToolSpec( + name="teach_act", + description=( + "Do one action in run 1: click (ref, or x and y), type " + "(text, or input to type a task input's value; ref to " + "focus a field first), press (key: Enter, Tab, ...) or " + "hotkey (key and modifiers). Returns the app's new state. " + + ( + "Before any save the harness checks that the record's " + "identity is shown next to the control; a save on the " + "wrong record is refused and ends run 1." + if on_web and not self.config.hold + else "A person approves every save in your client's form " + "before it happens; a save they don't approve ends run 1." + ) + ), + input_schema=_obj( + { + "action": { + "type": "string", + "enum": ["click", "type", "press", "hotkey"], + }, + "ref": {"type": "string", "maxLength": 64}, + "x": {"type": "integer", "minimum": 0}, + "y": {"type": "integer", "minimum": 0}, + "text": {"type": "string", "maxLength": 4000}, + "input": {"type": "string", "maxLength": 64}, + "key": {"type": "string", "maxLength": 32}, + "modifiers": { + "type": "array", + "items": {"type": "string", "maxLength": 16}, + "maxItems": 4, + }, + }, + ["action"], + ), + annotations=_ACTS, + ), + ToolSpec( + name="teach_finish", + description=( + "End run 1 when the task is done. OpenAdapt compiles what you " + "did, reads the app's records to check that exactly the " + "record the task named was saved, and returns the draft: " + "its steps, its parameters, the proposed record check, and " + "what still needs a person." + ), + input_schema=_obj({}, []), + annotations=_LOCAL, + ), + ToolSpec( + name="teach_abort", + description="Stop run 1 now. Nothing is compiled.", + input_schema=_obj({}, []), + annotations=_LOCAL, + ), + ToolSpec( + name="teach_status", + description=( + "The draft of a taught task: steps, parameters, the proposed " + "record check, and what still needs a person." + ), + input_schema=_obj({"task_key": _TASK_KEY_PROP}, ["task_key"]), + annotations=_READ_ONLY, + ), + ToolSpec( + name="teach_confirm_checks", + description=( + "Ask a person to confirm the proposed record check. Your " + "client shows them a form with the check, and only their " + "answer there confirms it; there is no argument you can " + "send to confirm it. drop_fields are fields that change on " + "every save (an id, a time stamp) to leave out of the check." + ), + input_schema=_obj( + { + "task_key": _TASK_KEY_PROP, + "drop_fields": { + "type": "array", + "items": {"type": "string", "maxLength": 64}, + "maxItems": 20, + }, + }, + ["task_key"], + ), + annotations=_LOCAL, + ), + ToolSpec( + name="teach_admit", + description=( + "Admit the draft so it can run. OpenAdapt runs canaries (real " + "saves with canary_inputs, on the record the task names, so " + "use values that are safe there) and admits only under a " + "signed admission policy the operator set up for you. " + "Without one, it tells you the command a person runs at a " + "terminal. Once admitted, the workflow is in list_workflows " + "and has its own taught_ tool." + ), + input_schema=_obj( + { + "task_key": _TASK_KEY_PROP, + "canary_inputs": { + "type": "array", + "minItems": 1, + "maxItems": 10, + "items": { + "type": "object", + "additionalProperties": {"type": "string"}, + }, + }, + }, + ["task_key", "canary_inputs"], + ), + annotations=_ACTS, + ), + ] + if self.config.screens != "off": + specs.insert( + 2, + ToolSpec( + name="teach_screen", + description=( + "The app's screen as a PNG" + + ( + ", with identifiers scrubbed (nothing is shared if " + "scrubbing fails)" + if self.config.screens == "scrubbed" + else "" + ) + + ". Use it when teach_observe is not enough (a remote " + "session has no control tree)." + ), + input_schema=_obj({}, []), + annotations=_READ_ONLY, + ), + ) + for tool, workflow in sorted(self.taught.items()): + specs.append(self._taught_spec(tool, workflow)) + return specs + + def _taught_spec(self, tool: str, workflow: str) -> ToolSpec: + from openadapt_flow.agent.cards import inputs_schema + + entry = self.workflows.catalog[workflow] + return ToolSpec( + name=tool, + description=( + f"Run the taught workflow {workflow}: {entry.card.purpose} It " + "runs a compiled program with no model calls and reads every " + "save back. Same outcomes as run_workflow; reuse request_id on " + "a retry." + ), + input_schema=_obj( + { + "inputs": inputs_schema(entry.card), + "request_id": {"type": "string", "minLength": 4, "maxLength": 128}, + }, + ["inputs", "request_id"], + ), + annotations={ + "readOnlyHint": False, + "destructiveHint": True, + "idempotentHint": True, + "openWorldHint": True, + }, + ) + + # -- what the MCP layer asks a person ------------------------------------------ + + def hold_needed(self, arguments: Mapping[str, Any]) -> Optional[str]: + """For ``teach_act``: what this action may commit, when a person must + approve it first. None when no approval is needed.""" + + run = self._active() + if not run.person_approves_saves: + return None + answer = self._run_call("preview", arguments) + if "error" in answer: + return None # teach_act reports the same error, performing nothing + return answer.get("commit") + + def hold_message(self, commit: str) -> str: + task = self._compiler.task if self._compiler is not None else None + identity = ", ".join(self._compiler.on_screen) if self._compiler else "" + return ( + "The agent is teaching OpenAdapt a task" + + (f" ({task.task_id})" if task is not None else "") + + f" and is about to do {commit!r}, which may save a record. Look at " + f"the app: is the record {identity} on screen, and should this save " + "happen? It is a real entry in this app." + ) + + def confirm_checks_message(self, arguments: Mapping[str, Any]) -> str: + compiler = self._load(arguments.get("task_key")) + proposed = compiler.proposed() + drop = set(arguments.get("drop_fields") or []) + lines = [] + for effect in proposed["effects"]: + fields = ", ".join( + f"{name} = {value}" + for name, value in effect["match"].items() + if name not in drop + ) + lines.append(f"{effect['kind']}: {fields}") + params = ", ".join(proposed["params"]) or "none" + return ( + "Every later run of this taught workflow must pass this record " + "check, read from the app's records: " + + "; ".join(lines) + + f". Inputs that change on each run: {params}." + + (f" Left out: {', '.join(sorted(drop))}." if drop else "") + + " Confirm only if this is the record the task should write. " + "Enter your own name; an agent or model name is refused." + ) + + # -- dispatch ----------------------------------------------------------------- + + def dispatch( + self, + name: str, + arguments: Optional[Mapping[str, Any]] = None, + *, + held_by_person: Optional[str] = None, + person: Optional[str] = None, + ) -> dict[str, Any]: + """Run one teach tool. + + ``held_by_person`` (the control a person approved for this + ``teach_act``) and ``person`` (the name a person gave to confirm the + record check) are set only by the MCP layer after a person answered + the client's form; no argument stands in for them. + """ + + args = dict(arguments or {}) + if name in self.taught: + return self._run_taught(name, args) + handlers = { + "teach_start": self._start, + "teach_observe": lambda a: self._observe(), + "teach_screen": lambda a: self._screen(), + "teach_finish": lambda a: self._finish(), + "teach_abort": lambda a: self._abort(), + "teach_status": lambda a: self._status(a.get("task_key")), + "teach_admit": self._admit, + } + if name == "teach_act": + return self._act(args, held_by_person=held_by_person or None) + if name == "teach_confirm_checks": + return self._confirm_checks(args, person) + if name not in handlers or not self.handles(name): + raise BridgeError("unknown tool name") + return handlers[name](args) + + # -- run 1 ---------------------------------------------------------------------- + + def _active(self) -> Any: + run = self._run + if run is None or run.done.is_set(): + raise BridgeError( + "no run 1 is in progress; call teach_start (or teach_finish to " + "see how the last one ended)" + ) + return run + + def _run_call(self, kind: str, payload: Optional[Mapping[str, Any]] = None) -> dict: + from openadapt_flow.compile_agent.driven import RunEnded + + run = self._active() + try: + return run.call(kind, payload or {}) + except RunEnded as exc: + raise BridgeError(f"{exc}; call teach_finish for the result") from None + + def _compiler_for(self, args: Mapping[str, Any]) -> Any: + from openadapt_flow.compile_agent import FirstRunCompiler + + inputs = {str(k): str(v) for k, v in (args.get("inputs") or {}).items()} + for key in inputs: + if not _NAME.fullmatch(key): + raise BridgeError(f"input names are a-z, 0-9 and _ ({key!r})") + name = args.get("name") + try: + return FirstRunCompiler( + str(args["task"]), + params=sorted(inputs), + url=self.config.url, + surface=self.config.surface, + open_backend=self.config.open_backend, + verify=self.config.verify, + expect=dict(args["record_fields"]), + on_screen=list(args["on_screen"]), + task_id=f"taught.{name}" if name else None, + policy=self.config.policy, + cache_dir=self.config.cache_dir, + key_dir=self.config.key_dir, + reset=self.config.reset, + headless=self.config.headless, + on_warning=lambda text: None, + ), inputs + except ValueError as exc: + raise BridgeError(str(exc)) from None + + def _start(self, args: Mapping[str, Any]) -> dict[str, Any]: + from openadapt_flow.compile_agent.driven import DrivenRun + from openadapt_flow.jit.compile_agent import RECORDING_WARNING + + with self._lock: + if self._run is not None and not self._run.done.is_set(): + raise BridgeError( + "run 1 of another task is in progress; teach_finish or " + "teach_abort it first" + ) + compiler, inputs = self._compiler_for(args) + jit = compiler._jit() + if jit.admitted or jit.candidate() is not None: + return {"started": False, **self._status(compiler.task.key)} + admitter = self.config.admitter + run = DrivenRun( + compiler, + inputs, + principal_id=admitter.principal_id if admitter is not None else None, + hold=self.config.hold, + ) + try: + run.start() + except Exception as exc: + # A run that opens late must not wait for actions forever. + run.abort() + raise BridgeError(f"run 1 did not start: {exc}") from None + self._run, self._compiler = run, compiler + observation = self._run_call("observe") + return { + "started": True, + "status": "recording", + "task_key": compiler.task.key, + "surface": compiler.backend_kind, + "person_approves_saves": run.person_approves_saves, + "warning": RECORDING_WARNING, + "observation": observation.get("observation"), + "next": "teach_act until the task is done, then teach_finish", + } + + def _observe(self) -> dict[str, Any]: + return {"observation": self._run_call("observe").get("observation")} + + def _screen(self) -> dict[str, Any]: + png = self._run_call("screenshot").get("png") or b"" + scrubbed = False + if self.config.screens == "scrubbed": + from openadapt_flow.agent.repair import default_scrubber, scrub_png + + try: + png, _evidence = scrub_png( + png, self.config.scrubber_factory or default_scrubber + ) + except Exception: # noqa: BLE001 - fail closed: share nothing + raise BridgeError( + "the screen could not be scrubbed, so no screen was shared" + ) from None + scrubbed = True + return { + "scrubbed": scrubbed, + "screen_png_base64": base64.b64encode(png).decode("ascii"), + } + + def _act( + self, args: Mapping[str, Any], *, held_by_person: Optional[str] + ) -> dict[str, Any]: + run = self._active() + payload = dict(args) + if held_by_person: + payload["_held_by_person"] = held_by_person + answer = self._run_call("act", payload) + if "refused" in answer: + run.done.wait(120) + return { + "status": "refused", + "detail": answer["refused"], + "run_over": True, + "next": "teach_finish shows the result; nothing was saved or compiled", + } + if "error" in answer: + return { + "status": "error", + "error": answer["error"], + "detail": answer.get("detail"), + "performed": False, + } + return {"status": "ok", "observation": answer.get("observation")} + + def _abort(self) -> dict[str, Any]: + run = self._active() + run.abort() + run.done.wait(120) + return {"status": "aborted", "compiled": False} + + def _finish(self) -> dict[str, Any]: + run = self._run + if run is None: + raise BridgeError("no run 1 to finish; call teach_start") + try: + result = run.finish() + except Exception as exc: # noqa: BLE001 - reported to the agent + return { + "status": "capture_failed", + "detail": f"{type(exc).__name__}: {exc}", + } + report = self._report(self._compiler, result.status, result.detail) + report["record_check"] = result.record_check + report["save_gate"] = [ + {"control": g.get("control"), "decision": g.get("decision")} + for g in (result.run1.get("gates") or []) + ] + return report + + # -- the draft --------------------------------------------------------------------- + + def _load(self, task_key: Any) -> Any: + from openadapt_flow.compile_agent import FirstRunCompiler + + if not isinstance(task_key, str) or not _TASK_KEY.fullmatch(task_key): + raise BridgeError("unknown task_key") + task_dir = self.config.cache_dir / task_key + if not (task_dir / "compile_agent.json").is_file(): + raise BridgeError("unknown task_key") + return FirstRunCompiler.load( + task_dir, reset=self.config.reset, open_backend=self.config.open_backend + ) + + def _status(self, task_key: Any) -> dict[str, Any]: + compiler = self._load(task_key) + jit = compiler._jit() + if jit.admitted: + status = "admitted" + elif jit.candidate() is not None: + status = "draft" + else: + status = "no_draft" + return self._report(compiler, status, None) + + def _report( + self, compiler: Any, status: str, detail: Optional[str] + ) -> dict[str, Any]: + from openadapt_flow.compile_agent.pipeline import CONFIRMED_FILE + + jit = compiler._jit() + report: dict[str, Any] = { + "status": status, + "task_key": compiler.task.key, + "surface": compiler.backend_kind, + } + if detail: + report["detail"] = detail + if status == "admitted" or jit.admitted: + report["status"] = "admitted" + report["workflow"] = self._workflow_name(compiler.dir) + report["needs_person"] = [] + return report + candidate = jit.candidate() + if candidate is None: + report["needs_person"] = [] + if status not in ("draft", "no_draft"): + report["next"] = ( + "nothing can be admitted from this run; fix the cause and " + "call teach_start again" + ) + return report + lifted = candidate.get("params") or {} + report["steps"] = [ + {"id": s["id"], "action": s["action"]} for s in jit.status()["steps"] + ] + report["parameters"] = { + "inputs": sorted(compiler.task.params), + "lifted": [p.get("name") for p in lifted.get("lifted") or []], + "not_lifted": dict(lifted.get("unlifted") or {}), + } + try: + proposed = compiler.proposed() + report["proposed_record_check"] = proposed["effects"] + except (OSError, KeyError): + report["proposed_record_check"] = None + confirmed = (compiler.out_dir / CONFIRMED_FILE).is_file() + report["record_check_confirmed"] = confirmed + needs: list[str] = [] + if not confirmed: + needs.append( + "A person confirms the proposed record check: teach_confirm_checks " + "asks them in your client's form, or they run " + f"`{self._terminal_command(compiler)}` at a terminal." + ) + if self.config.admitter is None: + needs.append( + "A person admits the program at a terminal: " + f"`{self._terminal_command(compiler)}`. This server has no " + "admission policy, so teach_admit can't admit it." + ) + report["needs_person"] = needs + report["next"] = ( + "teach_confirm_checks" + if not confirmed + else ( + "teach_admit" if self.config.admitter is not None else "a person admits" + ) + ) + return report + + def _terminal_command(self, compiler: Any) -> str: + params = " ".join( + f"--canary-param {p}=" for p in sorted(compiler.task.params) + ) + return ( + f"openadapt-flow compile-agent admit {compiler.dir} --admit-as " + + params + ).strip() + + def _confirm_checks(self, args: Mapping[str, Any], person: Optional[str]) -> dict: + from openadapt_flow.jit.lifecycle import AdmissionError + + if not person: + raise BridgeError( + "a person confirms the record check in the client's form; " + "nothing was confirmed" + ) + compiler = self._load(args.get("task_key")) + try: + compiler.confirm_checks( + principal_id=person, drop_fields=list(args.get("drop_fields") or []) + ) + except (AdmissionError, ValueError, OSError) as exc: + raise BridgeError(f"not confirmed: {exc}") from None + return {**self._report(compiler, "draft", None), "confirmed_by": person} + + def _admit(self, args: Mapping[str, Any]) -> dict[str, Any]: + from openadapt_flow.jit.lifecycle import AdmissionError + + compiler = self._load(args.get("task_key")) + if self.config.admitter is None: + return { + **self._report(compiler, "draft", None), + "status": "needs_person", + "detail": ( + "this server has no admission policy, so an agent can't " + "admit; a person admits at a terminal" + ), + } + canaries = [ + {str(k): str(v) for k, v in row.items()} + for row in args.get("canary_inputs") or [] + ] + + def admit_and_emit() -> Any: + result = compiler.admit( + admitter=self.config.admitter, canary_params=canaries + ) + if result.admitted: + result.emitted = compiler.emit() + return result + + try: + result = _off_thread(admit_and_emit) + except (AdmissionError, ValueError) as exc: + raise BridgeError(f"not admitted: {exc}") from None + out: dict[str, Any] = { + "status": result.status, + "task_key": compiler.task.key, + "detail": result.detail, + "canaries": [ + { + "status": c.get("status"), + "receipt": c.get("receipt_outcome"), + "model_calls": c.get("model_calls"), + } + for c in (result.admission or {}).get("canaries") or [] + ], + } + if result.admitted: + tool = self._register(compiler.dir) + out.update( + { + "workflow": self.taught[tool], + "tool": tool, + "mode": (result.admission or {}).get("mode"), + "admitted_by": (result.admission or {}).get("admitted_by"), + "skill": (result.emitted or {}).get("skill"), + "next": f"call {tool}, or run_workflow with this workflow", + } + ) + return out + + # -- admitted programs as workflows --------------------------------------------------- + + @staticmethod + def _is_admitted(task_dir: Path) -> bool: + return (task_dir / "compile_agent.json").is_file() and ( + task_dir / "program" / "bundle" / "admission.json" + ).is_file() + + def _workflow_name(self, task_dir: Path) -> Optional[str]: + for workflow in self.taught.values(): + entry = self.workflows.catalog.get(workflow) if self.workflows else None + engine = getattr(entry, "engine", None) + if isinstance(engine, TaughtEngine) and engine.task_dir == task_dir: + return workflow + return None + + def _register(self, task_dir: Path) -> str: + """Add an admitted program to the workflow catalog; returns its tool.""" + + import json + + from openadapt_flow.agent.cards import ( + DEFAULT_DONE_MEANS, + WorkflowCard, + _check_text, + _default_description, + default_purpose, + ) + from openadapt_flow.agent.service import CatalogEntry + + if self.workflows is None: + raise BridgeError("this server has no workflow tools to add the program to") + existing = self._workflow_name(task_dir) + if existing is not None: + return next(t for t, w in self.taught.items() if w == existing) + spec = json.loads((task_dir / "compile_agent.json").read_text()) + task = spec["task"] + base = re.sub(r"[^a-z0-9]+", "_", str(task["task_id"]).lower()).strip("_") + base = ( + base.removeprefix("taught_").removeprefix("compile_agent_")[:40] or "task" + ) + name = base + suffix = 2 + while name in self.workflows.catalog or TAUGHT_PREFIX + name in self.taught: + name = f"{base}_{suffix}" + suffix += 1 + try: + purpose = _check_text("purpose", f"Taught: {task['instruction']}") + except Exception: # noqa: BLE001 - an unusable instruction gets the default + purpose = default_purpose() + params = sorted(task.get("params") or []) + card = WorkflowCard( + name=name, + purpose=purpose, + done_means=DEFAULT_DONE_MEANS, + inputs={ + p: {"type": "string", "description": _default_description(p)} + for p in params + }, + required=params, + changes_records=True, + ) + self.workflows.add_catalog_entry( + CatalogEntry(card=card, engine=TaughtEngine(task_dir, self.config)) + ) + tool = TAUGHT_PREFIX + name + self.taught[tool] = name + return tool + + def _run_taught(self, tool: str, args: Mapping[str, Any]) -> dict[str, Any]: + return self.workflows.dispatch( + "run_workflow", + { + "workflow": self.taught[tool], + "inputs": args.get("inputs") or {}, + "request_id": args.get("request_id"), + }, + ) diff --git a/packages/flow/openadapt_flow/authoring.py b/packages/flow/openadapt_flow/authoring.py index 75e2dd6d8..48cce7151 100644 --- a/packages/flow/openadapt_flow/authoring.py +++ b/packages/flow/openadapt_flow/authoring.py @@ -576,6 +576,12 @@ def remember_node( self._nodes[node_id] = _pixels_from_mapping(backend_pixels) + def node_center(self, node_id: str) -> Optional[tuple[int, int]]: + """The backend-pixel center of an observed node, or None if unknown.""" + + target = self._nodes.get(node_id) + return target.center if target is not None else None + def observe(self) -> dict[str, Any]: """Return a PHI-safe authoring tree of the pinned window. diff --git a/packages/flow/openadapt_flow/compile_agent/cli.py b/packages/flow/openadapt_flow/compile_agent/cli.py index 279a4a1fc..3435f9d90 100644 --- a/packages/flow/openadapt_flow/compile_agent/cli.py +++ b/packages/flow/openadapt_flow/compile_agent/cli.py @@ -15,6 +15,15 @@ openadapt-flow compile-agent run --param note="Recheck in ten days" + openadapt-flow compile-agent admit --admit-as "Dana Reviewer" \\ + --canary-param note="Canary test note" + +``admit`` is for a draft that waits for a person, such as one an agent taught +over MCP (``openadapt-agent serve --teach``): at an interactive terminal the +person confirms the proposed record checks if nobody has yet, reviews the +steps, attends the canary and admits. ``--admit-key`` admits with a +registered key instead, after the checks are confirmed. + The command is re-entrant. Run it again with the same arguments and it picks up where it stopped: the agent runs only when the task has no draft and no admitted program. Confirming the proposed record checks is a person's step at @@ -56,9 +65,14 @@ def add_parser(sub: argparse._SubParsersAction) -> None: ) p.add_argument( "prompt", - help='the task, with {name} for each --param; or "run" to run a compiled task', + help=( + 'the task, with {name} for each --param; "run" to run a compiled ' + 'task; or "admit" to confirm and admit a waiting draft' + ), + ) + p.add_argument( + "task_dir", nargs="?", help="with run or admit: the compiled task directory" ) - p.add_argument("task_dir", nargs="?", help="with run: the compiled task directory") p.add_argument("--param", action="append", metavar="NAME=VALUE") p.add_argument( "--url", @@ -263,6 +277,8 @@ def cmd_compile_agent( ) params = _pairs(args.param, "--param") + if args.prompt == "admit": + return _cmd_admit(args, ask=ask, interactive=interactive) if args.prompt == "run": if not args.task_dir: raise SystemExit("compile-agent run needs the compiled task directory") @@ -335,3 +351,57 @@ def cmd_compile_agent( if result.admitted else (3 if result.status in ("draft", "needs_person") else 1) ) + + +def _cmd_admit( + args: argparse.Namespace, + *, + ask: Callable[[str], str], + interactive: Callable[[], bool], +) -> int: + """A person (or a registered key) admits a draft that waits in TASK_DIR.""" + + from openadapt_flow.compile_agent.pipeline import CONFIRMED_FILE, FirstRunCompiler + + if not args.task_dir: + raise SystemExit("compile-agent admit needs the compiled task directory") + compiler = FirstRunCompiler.load(args.task_dir) + jit = compiler._jit() + if jit.admitted: + _print({"status": "admitted", "detail": "already admitted"}, args.json) + return 0 + if jit.candidate() is None: + raise SystemExit(f"{args.task_dir} has no draft waiting") + tty = interactive() + if not (compiler.out_dir / CONFIRMED_FILE).is_file(): + if not tty: + raise SystemExit( + "a person confirms the proposed record checks at an interactive " + "terminal; there is none" + ) + name = _terminal_confirm(ask)(compiler.proposed()) + if name is None: + print("Not confirmed; nothing changed.") + return 3 + compiler.confirm_checks(principal_id=name, drop_fields=args.drop_field) + admitter = _admitter(args) + if admitter is None and not args.admit_as: + raise SystemExit("pass --admit-as YOUR NAME, or --admit-key") + if admitter is None and not tty: + raise SystemExit("--admit-as needs an interactive terminal") + result = compiler.admit( + admitter=admitter, + person=None if admitter is not None else args.admit_as, + review=_terminal_review(ask) if admitter is None else None, + canary_params=( + [_pairs(args.canary_param, "--canary-param")] if args.canary_param else None + ), + ) + if result.admitted: + result.emitted = compiler.emit() + _print(result.to_dict(), args.json) + return ( + 0 + if result.admitted + else (3 if result.status in ("draft", "needs_person") else 1) + ) diff --git a/packages/flow/openadapt_flow/compile_agent/driven.py b/packages/flow/openadapt_flow/compile_agent/driven.py new file mode 100644 index 000000000..90e2fbae0 --- /dev/null +++ b/packages/flow/openadapt_flow/compile_agent/driven.py @@ -0,0 +1,300 @@ +"""Run 1 driven from outside: an agent that sends one action per call. + +:class:`FirstRunCompiler` records an agent that owns its loop +(``drive(request, harness)``). An agent connected over MCP doesn't: it makes +one tool call per action. :class:`DrivenRun` turns those calls into an agent. +It runs :meth:`FirstRunCompiler.record` on a thread of its own, where the +browser or native backend lives, and hands each call's action to the harness +on that thread. When the caller finishes, the run is compiled and checked +exactly as for any other agent: the same save gate, the same record check, +the same draft. + +The caller decides nothing about the gate. A gated action that a person must +approve is approved by :meth:`DrivenRun.act` only when the caller says a +person did (``held_by_person``); the MCP server sets that only after the +person answered the client's own form. +""" + +from __future__ import annotations + +import queue +import threading +import time +from concurrent.futures import Future +from concurrent.futures import TimeoutError as FutureTimeout +from dataclasses import dataclass +from typing import Any, Mapping, Optional + +from openadapt_flow.compile_agent.agents import AgentLedger +from openadapt_flow.compile_agent.harness import GateRefused, GateRequest, StaleRef +from openadapt_flow.jit.adapters import UnsupportedAction +from openadapt_flow.jit.compile_agent import TaskRequest + +__all__ = ["DrivenAgent", "DrivenRun", "RunEnded", "ACTIONS"] + +#: Actions a caller sends. ``ref`` names a control in the latest observation. +ACTIONS = ("click", "type", "press", "hotkey") + +_STOP = object() + + +class RunEnded(RuntimeError): + """Run 1 is over; no more actions go to it.""" + + +@dataclass +class _Command: + kind: str + payload: Mapping[str, Any] + future: Future + + +class DrivenAgent: + """The agent :class:`FirstRunCompiler` records: actions arrive by queue. + + ``principal_id`` is the identity the recording is signed with. It must be + the registered agent that may later admit the draft, so that admitting it + is self-admission and needs a policy that allows it. + """ + + dialect = "generic" + requires_model_key = False + #: Drives the surface harness too (native windows, RDP, Citrix). + drives_surfaces = True + + def __init__(self, principal_id: Optional[str] = None) -> None: + self.principal_id = principal_id + self.request: Optional[TaskRequest] = None + self.harness: Any = None + self.ready = threading.Event() + self._inbox: "queue.Queue[Any]" = queue.Queue() + self._approved: Optional[str] = None + self.ledger = AgentLedger(agent="driven") + + # -- called by the gate, on the run thread ----------------------------------- + + def hold(self, request: GateRequest) -> bool: + """True only for the action a person approved, once.""" + + approved, self._approved = self._approved, None + return approved is not None and approved == request.control + + # -- the run thread ------------------------------------------------------------ + + def drive(self, request: TaskRequest, harness: Any) -> AgentLedger: + self.request = request + self.harness = harness + self.ready.set() + while True: + command = self._inbox.get() + if command is _STOP: + raise RunEnded("run 1 was stopped before it finished") + assert isinstance(command, _Command) + if command.kind == "finish": + command.future.set_result({"finished": True}) + break + try: + command.future.set_result(self._handle(command)) + except (StaleRef, UnsupportedAction, LookupError, ValueError) as exc: + # Nothing was performed: report it and keep the run going. + command.future.set_result( + {"error": type(exc).__name__, "detail": str(exc)} + ) + except GateRefused as exc: + command.future.set_result({"refused": str(exc)}) + raise + except BaseException as exc: + command.future.set_exception(exc) + raise + self.ledger.finished = True + self.ledger.wall_seconds = time.monotonic() - self.ledger.started_at + return self.ledger + + def _handle(self, command: _Command) -> dict[str, Any]: + h = self.harness + if command.kind == "observe": + obs = h.observe() + return {"observation": obs.text} + if command.kind == "screenshot": + # The caller decides how to scrub it before it leaves the computer. + h.stats.screenshots_requested += 1 + return {"png": h.computer.screenshot()} + native, focus = self._native(command.payload) + if command.kind == "preview": + return {"commit": h.commit_target(native)} + if command.kind == "act": + if focus is not None: + # Focus the field first; a click on a text field commits nothing. + h.click(focus) + # Only the control the person was shown is approved. If something + # else would commit now, the gate asks again and the hold says no. + held = command.payload.get("_held_by_person") + self._approved = str(held) if held else None + try: + h.act(native) + finally: + self._approved = None + return {"observation": h.observe().text} + raise ValueError(f"unknown command {command.kind!r}") + + def _native( + self, action: Mapping[str, Any] + ) -> tuple[dict[str, Any], Optional[str]]: + """A caller's action as the harness's generic action, and the ref of + a field to click first (``type`` with a ``ref``). Performs nothing.""" + + kind = action.get("action") + if kind not in ACTIONS: + raise ValueError(f"action must be one of {ACTIONS}") + if kind == "click": + if action.get("ref") is not None: + return self.harness.native_for({"ref": action["ref"]}) + if action.get("x") is None or action.get("y") is None: + raise ValueError("click needs ref, or x and y") + return {"kind": "click", "x": int(action["x"]), "y": int(action["y"])}, None + if kind == "type": + if action.get("input") is not None: + inputs = (self.request.inputs if self.request else {}) or {} + name = str(action["input"]) + if name not in inputs: + raise LookupError(f"no task input named {name!r}") + text = str(inputs[name]) + elif action.get("text") is not None: + text = str(action["text"]) + else: + raise ValueError("type needs text or input") + ref = action.get("ref") + return {"kind": "type", "text": text}, ( + str(ref) if ref is not None else None + ) + if kind == "press": + return {"kind": "press", "key": str(action.get("key") or "")}, None + return { + "kind": "hotkey", + "key": str(action.get("key") or ""), + "modifiers": [str(m) for m in action.get("modifiers") or []], + }, None + + # -- called from other threads ------------------------------------------------ + + def submit(self, kind: str, payload: Mapping[str, Any]) -> Future: + future: Future = Future() + self._inbox.put(_Command(kind, dict(payload), future)) + return future + + def stop(self) -> None: + self._inbox.put(_STOP) + + +class DrivenRun: + """One run 1 of ``compiler``'s task, driven call by call. + + ``hold`` says whether every gated action needs a person (on the web the + harness also checks the record's identity itself; on other surfaces it + can't, so a person approves every gated action there whatever ``hold`` + says). + """ + + def __init__( + self, + compiler: Any, + params: Mapping[str, Any], + *, + principal_id: Optional[str] = None, + hold: bool = False, + ) -> None: + self.compiler = compiler + self.params = dict(params) + self.agent = DrivenAgent(principal_id=principal_id) + self.hold = hold or compiler.backend_kind != "web" + self.result: Any = None + self.error: Optional[BaseException] = None + self.done = threading.Event() + self._thread: Optional[threading.Thread] = None + + @property + def person_approves_saves(self) -> bool: + return self.hold + + def start(self, timeout_s: float = 300.0) -> None: + """Open the app and start recording. Returns once the agent can act. + + Raises: + Exception: whatever stopped the run before the first action + (a draft already exists, the app did not open). + """ + + self._thread = threading.Thread( + target=self._main, name="openadapt-teach-run", daemon=True + ) + self._thread.start() + deadline = time.monotonic() + timeout_s + while not self.agent.ready.wait(0.05): + if self.done.is_set() or time.monotonic() > deadline: + break + if self.error is not None: + raise self.error + if not self.agent.ready.is_set(): + if self.done.is_set(): + raise RunEnded( + "run 1 ended before the agent could act: " + + str(getattr(self.result, "detail", None) or "") + ) + raise TimeoutError("the app did not open in time") + + def _main(self) -> None: + try: + self.result = self.compiler.record( + self.agent, + self.params, + hold=self.agent.hold if self.hold else None, + principal_id=self.agent.principal_id, + ) + except BaseException as exc: # noqa: BLE001 - handed to the caller + self.error = exc + finally: + self.done.set() + + def call( + self, + kind: str, + payload: Optional[Mapping[str, Any]] = None, + timeout_s: float = 120.0, + ) -> dict[str, Any]: + """Send one command to the run thread and wait for its answer. + + Raises: + RunEnded: run 1 is over (finished, refused, or failed). + """ + + if self.done.is_set(): + raise RunEnded("run 1 is over") + future = self.agent.submit(kind, payload or {}) + deadline = time.monotonic() + timeout_s + while True: + try: + return future.result(timeout=0.05) + except FutureTimeout: + pass + except Exception as exc: + raise RunEnded(f"run 1 failed: {type(exc).__name__}: {exc}") from exc + if self.done.is_set() and not future.done(): + raise RunEnded("run 1 is over") + if time.monotonic() > deadline: + raise TimeoutError("the action did not finish in time") + + def finish(self, timeout_s: float = 600.0) -> Any: + """End run 1, then wait for the compile and the record check.""" + + if not self.done.is_set(): + self.agent.submit("finish", {}) + if not self.done.wait(timeout_s): + raise TimeoutError("compiling run 1 did not finish in time") + if self.error is not None: + raise self.error + return self.result + + def abort(self) -> None: + """End run 1 now. Nothing more is performed and no draft is made.""" + + self.agent.stop() diff --git a/packages/flow/openadapt_flow/compile_agent/harness.py b/packages/flow/openadapt_flow/compile_agent/harness.py index cdfa0308d..924ee6d62 100644 --- a/packages/flow/openadapt_flow/compile_agent/harness.py +++ b/packages/flow/openadapt_flow/compile_agent/harness.py @@ -40,6 +40,9 @@ "HarnessStats", "SaveGate", "StaleRef", + "SurfaceControl", + "SurfaceHarness", + "SurfaceObservation", ] @@ -184,6 +187,23 @@ def type_text(self, text: str, ref: Optional[str] = None) -> Observation: def press(self, key: str) -> Observation: return self.act({"kind": "press", "key": key}) + def native_for( + self, action: Mapping[str, Any] + ) -> tuple[dict[str, Any], Optional[str]]: + """The generic action for a ref-based ``click`` and the ref it named.""" + + ref = action.get("ref") + if ref is None: + return dict(action), None + control = self._control(str(ref)) + return {"kind": "click", "x": control.x, "y": control.y}, control.ref + + def commit_target(self, native: Mapping[str, Any]) -> Optional[str]: + """What may commit a write in ``native``, or None. Nothing is performed.""" + + hit = self._commit_target(native) + return hit[0] if hit is not None else None + # -- every action ------------------------------------------------------------- def act(self, native: Mapping[str, Any]) -> Observation: @@ -322,3 +342,269 @@ def _settle(page: Any, timeout_ms: int = 1500) -> None: page.wait_for_load_state("networkidle", timeout=timeout_ms) except Exception: # noqa: BLE001 - a page that never idles is still usable pass + + +# -- every other surface ---------------------------------------------------------- + + +@dataclass(frozen=True) +class SurfaceControl: + """One control in a surface observation. ``ref`` is the authoring node id.""" + + ref: str + role: str + name: str + x: int + y: int + + +@dataclass +class SurfaceObservation: + """What the agent reads on a native or remote app. + + ``text`` has one line per control the accessibility tree shows (UI + Automation, Accessibility, AT-SPI): ``[n_1a2b3c4d] button "Save"``. A + remote session (RDP, Citrix) has no tree: ``screen_only`` is True and the + agent works from the screen and pixel clicks. + """ + + backend: str + screen_only: bool + text: str + controls: dict[str, SurfaceControl] = field(default_factory=dict) + + def find( + self, name: str, *, near: Optional[str] = None + ) -> Optional[SurfaceControl]: + """The one control named ``name`` (case-insensitive), or None.""" + + wanted = name.strip().lower() + hits = [c for c in self.controls.values() if c.name.lower() == wanted] + return hits[0] if len(hits) == 1 else None + + +#: Roles a click only focuses. Typing into them commits nothing. +_EDITABLE_ROLES = frozenset({"text_input", "combobox"}) + + +@dataclass +class SurfaceHarness: + """The harness for a native window or a remote session. + + It has the same methods as :class:`Harness`, so an agent drives either. + Observations come from the authoring session's PHI-safe tree (no field + values, no window titles). Every action goes through the same + :class:`CaptureComputer`, so the recording is the one any other capture + makes. + + The save gate gates the same actions as on the web: a click on a control + whose label names a write, a click on a point no control covers, Enter, + Space, a typed line break, and any shortcut. On these surfaces the harness + can't read which form or row a control sits in, so it can't do the + right-record check itself. A person approves every gated action instead + (the hold). Without a hold, a gated action is refused. + """ + + computer: CaptureComputer + gate: SaveGate + keep: Sequence[str] = () + evidence_dir: Optional[Path] = None + stats: HarnessStats = field(default_factory=HarnessStats) + gate_log: list[dict[str, Any]] = field(default_factory=list) + refusal: Optional[str] = None + _last: Optional[SurfaceObservation] = None + _focused_role: Optional[str] = None + + page: Any = None + + @property + def dialect(self) -> str: + return self.computer.dialect + + @property + def session(self) -> Any: + return self.computer.session + + # -- observation ------------------------------------------------------------- + + def observe(self) -> SurfaceObservation: + raw = self.session.observe() + controls: dict[str, SurfaceControl] = {} + lines: list[str] = [] + for node in raw.get("tree") or []: + ref = str(node.get("node_id") or "") + center = self.session.node_center(ref) if ref else None + if center is None: + continue + name = str(node.get("name") or "") + role = str(node.get("role") or "unknown") + controls[ref] = SurfaceControl(ref, role, name, center[0], center[1]) + lines.append(f"[{ref}] {role} " + (f'"{name}"' if name else "(no name)")) + screen_only = bool(getattr(self.session, "screen_only", False)) + if screen_only: + lines.append( + "(remote session: no control tree; read the screen and click x,y)" + ) + elif not controls: + lines.append( + "(this app's backend lists no controls; read the screen and click x,y)" + ) + self._last = SurfaceObservation( + backend=str(raw.get("backend") or ""), + screen_only=screen_only, + text="\n".join(lines), + controls=controls, + ) + self.stats.observations += 1 + return self._last + + def screenshot(self, why: str = "requested") -> bytes: + from openadapt_flow.privacy import scrub_image_bytes + + counter = _SCREENSHOT_COUNTERS[why] + png = self.computer.screenshot() + setattr(self.stats, counter, getattr(self.stats, counter) + 1) + return scrub_image_bytes(png) + + # -- actions -------------------------------------------------------------------- + + def _control(self, ref: str) -> SurfaceControl: + last = self._last + if last is None or ref not in last.controls: + raise StaleRef(f"{ref!r} is not in the latest observation; observe again") + control = last.controls[ref] + # The node id hashes the role, automation id and bounds: if the + # control moved or changed, it is gone from a fresh tree. + fresh = { + str(n.get("node_id")): n for n in self.session.observe().get("tree") or [] + } + node = fresh.get(ref) + if node is None or str(node.get("name") or "") != control.name: + raise StaleRef( + f"{ref!r} ({control.name!r}) is no longer at its place; observe again" + ) + return control + + def native_for( + self, action: Mapping[str, Any] + ) -> tuple[dict[str, Any], Optional[str]]: + """The generic action for a ref-based ``click`` and the control's name.""" + + ref = action.get("ref") + if ref is None: + return dict(action), None + control = self._control(str(ref)) + return {"kind": "click", "x": control.x, "y": control.y}, control.ref + + def click(self, ref: str) -> Observation: + control = self._control(ref) + return self.act({"kind": "click", "x": control.x, "y": control.y}) + + def type_text(self, text: str, ref: Optional[str] = None) -> Observation: + if ref is not None: + self.click(ref) + return self.act({"kind": "type", "text": text}) + + def press(self, key: str) -> Observation: + return self.act({"kind": "press", "key": key}) + + def act(self, native: Mapping[str, Any]) -> Observation: + """Perform one generic action, behind the gate (see the class doc). + + Raises: + GateRefused: the action may commit a write and no person approved it. + UnsupportedAction: the recording can't hold the action. + """ + + commit = self.commit_target(native) + if commit is not None: + self._check_gate(commit) + obs = self.computer.execute(native) + self.stats.actions += 1 + clicked = self._control_at(native) + if clicked is not None or native.get("kind") in ("click", "double_click"): + self._focused_role = clicked.role if clicked is not None else None + return obs + + def _control_at(self, native: Mapping[str, Any]) -> Optional[SurfaceControl]: + if native.get("kind") not in ("click", "double_click", "right_click"): + return None + x, y = native.get("x"), native.get("y") + if x is None or y is None or self._last is None: + return None + for control in self._last.controls.values(): + if control.x == x and control.y == y: + return control + return None + + def commit_target(self, native: Mapping[str, Any]) -> Optional[str]: + """What may commit a write in ``native``, or None (see the class doc).""" + + for op in translate(self.dialect, native): + if op.kind in ("click", "double_click", "right_click"): + control = self._control_at(native) + if control is None: + return "a click on a point the harness can't name" + if control.role in _EDITABLE_ROLES: + continue + if _observe.commit_label(control.name): + return control.name + elif op.kind == "type" and any(ch in (op.text or "") for ch in "\r\n"): + return "a typed line break" + elif op.kind == "hotkey": + return "the shortcut " + "+".join([*op.modifiers, op.key or ""]) + elif op.kind == "press" and op.key in ("Enter", "Space"): + if op.key == "Space" and self._focused_role in _EDITABLE_ROLES: + continue + return f"{op.key} on the focused control" + return None + + def _check_gate(self, control: str) -> None: + self.stats.gate_checks += 1 + shot: Optional[Path] = None + png = self.computer.screenshot() + self.stats.gate_evidence_screenshots += 1 + if self.evidence_dir is not None: + self.evidence_dir.mkdir(parents=True, exist_ok=True) + shot = self.evidence_dir / f"gate-{len(self.gate_log) + 1:02d}.png" + shot.write_bytes(png) + entry: dict[str, Any] = { + "control": control, + "identity": list(self.gate.on_screen), + "missing": [], + "url": "", + "screenshot": str(shot) if shot else None, + "at": time.time(), + "identity_check": "person", + } + hold = self.gate.hold + if hold is None: + entry["decision"] = "refused_no_hold" + self.gate_log.append(entry) + self._refuse( + f"{control!r} may commit a write. On this surface the harness " + "can't read which record it is on, so a person must approve it " + "(a hold), and there is none" + ) + return + approved = bool( + hold( + GateRequest( + control=control, + identity=tuple(self.gate.on_screen), + url="", + screenshot=shot, + ) + ) + ) + if not approved: + entry["decision"] = "refused_by_hold" + self.gate_log.append(entry) + self._refuse(f"the hold did not approve {control!r}") + entry["decision"] = "approved_by_hold" + self.gate_log.append(entry) + + def _refuse(self, reason: str) -> None: + self.stats.gate_refusals += 1 + self.refusal = reason + raise GateRefused(reason) diff --git a/packages/flow/openadapt_flow/compile_agent/pipeline.py b/packages/flow/openadapt_flow/compile_agent/pipeline.py index 68ae95995..b2363f2b1 100644 --- a/packages/flow/openadapt_flow/compile_agent/pipeline.py +++ b/packages/flow/openadapt_flow/compile_agent/pipeline.py @@ -40,11 +40,17 @@ from pathlib import Path from typing import Any, Callable, Mapping, Optional, Sequence, Union -from openadapt_flow.compile_agent.harness import GateRequest, Harness, SaveGate +from openadapt_flow.compile_agent.harness import ( + GateRequest, + Harness, + SaveGate, + SurfaceHarness, +) from openadapt_flow.jit import lifecycle from openadapt_flow.jit.compile_agent import ( CompiledAgent, JitResult, + SurfaceApp, TaskRequest, WebApp, compile_agent, @@ -276,7 +282,7 @@ def __init__( self.keep = list(keep) self.evidence_dir = evidence_dir self.principal_id = principal_id - self.harness: Optional[Harness] = None + self.harness: Optional[Union[Harness, SurfaceHarness]] = None self.ledger: Any = None self.error: Optional[str] = None @@ -285,7 +291,10 @@ def requires_model_key(self) -> bool: return bool(getattr(self.inner, "requires_model_key", True)) def run(self, request: TaskRequest, computer: Any) -> None: - self.harness = Harness( + # A browser page gets the DOM harness; any other surface (a native + # window, an RDP or Citrix client window) gets the surface harness. + harness_type = Harness if computer.page is not None else SurfaceHarness + self.harness = harness_type( computer, self.gate, keep=self.keep, evidence_dir=self.evidence_dir ) try: @@ -312,7 +321,7 @@ def __init__( prompt: str, *, params: Sequence[str], - url: str, + url: Optional[str] = None, verify: Union[str, Path], expect: Mapping[str, Any], on_screen: Sequence[str], @@ -325,10 +334,20 @@ def __init__( on_warning: Optional[Callable[[str], None]] = None, model_key: Optional[CustomerModelKey] = None, backend: str = "web", + surface: Optional[Mapping[str, Any]] = None, + open_backend: Optional[Callable[[Any], Any]] = None, ) -> None: from openadapt_flow.record_sor import load_observer - check_target(url, backend) + self.surface: Optional[Any] = None + if surface is not None: + # A deployment ``backend:`` section for a native or remote app. + # Only an agent that drives the surface harness can record it + # (record() checks); the program runs there like any JIT program. + self.surface = SurfaceApp(surface, open_backend=open_backend).backend + url = None + else: + check_target(url or "", backend) reserved = sorted(set(params) & {"reset", "task_dir"}) if reserved: @@ -354,6 +373,7 @@ def __init__( task_id or _slug(prompt), prompt, {name: str for name in params} ) self.url = url + self.open_backend = open_backend self.verify_path = Path(verify).resolve() self.expect = {str(k): str(v) for k, v in expect.items()} self.on_screen = [str(s) for s in on_screen if str(s).strip()] @@ -379,16 +399,34 @@ def dir(self) -> Path: def out_dir(self) -> Path: return self.dir / OUT_DIR + @property + def backend_kind(self) -> str: + """``web``, or the surface kind (``windows``, ``macos``, ``linux``, + ``rdp``, ``citrix``).""" + + return SurfaceApp(self.surface).kind if self.surface is not None else "web" + + def _app(self) -> Any: + if self.surface is not None: + return SurfaceApp( + self.surface, + record_reader=lambda: self.observer.system_of_record, + reset=self._on_reset, + open_backend=self.open_backend, + ) + assert self.url is not None + return WebApp( + url=self.url, + record_reader=lambda: self.observer.system_of_record, + reset=self._on_reset, + headless=self.headless, + ) + def _jit(self, agent: Any = None) -> CompiledAgent: return compile_agent( agent, task=self.task, - app=WebApp( - url=self.url, - record_reader=lambda: self.observer.system_of_record, - reset=self._on_reset, - headless=self.headless, - ), + app=self._app(), verifier=self.observer.verifier, policy=self.policy, cache_dir=self.cache_dir, @@ -425,6 +463,11 @@ def save_spec(self) -> Path: "key": self.task.key, }, "url": self.url, + "surface": ( + self.surface.model_dump(mode="json", exclude_none=True) + if self.surface is not None + else None + ), "verify": str(self.verify_path), "expect": self.expect, "on_screen": self.on_screen, @@ -437,7 +480,11 @@ def save_spec(self) -> Path: @classmethod def load( - cls, task_dir: Union[str, Path], *, reset: Optional[Callable[[], None]] = None + cls, + task_dir: Union[str, Path], + *, + reset: Optional[Callable[[], None]] = None, + open_backend: Optional[Callable[[Any], Any]] = None, ) -> "FirstRunCompiler": spec = json.loads((Path(task_dir) / SPEC_FILE).read_text()) if spec.get("schema") != SPEC_SCHEMA: @@ -446,7 +493,9 @@ def load( return cls( task["instruction"], params=task["params"], - url=spec["url"], + url=spec.get("url"), + surface=spec.get("surface"), + open_backend=open_backend, verify=spec["verify"], expect=spec["expect"], on_screen=spec["on_screen"], @@ -475,6 +524,12 @@ def record( agent's own. """ + if self.surface is not None and not getattr(agent, "drives_surfaces", False): + raise UnsupportedTarget( + f"this agent reads a web page's DOM; the {self.backend_kind!r} " + "app needs an agent that drives the surface harness (the MCP " + "teach tools do). Nothing ran." + ) self.save_spec() jit = self._jit() values = self.task.validate(params) diff --git a/packages/flow/tests/e2e/test_mcp_teach_e2e.py b/packages/flow/tests/e2e/test_mcp_teach_e2e.py new file mode 100644 index 000000000..d297498ac --- /dev/null +++ b/packages/flow/tests/e2e/test_mcp_teach_e2e.py @@ -0,0 +1,483 @@ +"""The MCP teach loop on MockMed, driven by a scripted MCP client. + +The client is the real MCP SDK session over its in-memory transport, talking +to the real server. It reads the listings the server returns and picks refs +from them, the way an agent would. No model is called anywhere. + +It asserts: + +* run 1 is recorded through teach_start / teach_act / teach_finish, passes + its record check, and the draft report names the steps, the parameter, the + proposed record check and what still needs a person; +* the agent can't admit before a person confirms the check, and a person + confirms it in the client's form; +* a server with no admission policy refuses to admit and names the terminal + command; a policy that does not allow self-admission refuses the recording + agent before any canary runs; +* under a policy that allows it, the agent admits; the server sends + tools/list_changed; the program is in list_workflows and has its own tool; +* calling that tool saves a new note, ends done, with zero model calls; +* a server started later lists the taught workflow at once; +* a run 1 whose save lands on the wrong record fails its record check and + leaves nothing to admit. +""" + +from __future__ import annotations + +import json +import re +from contextlib import asynccontextmanager +from datetime import datetime, timedelta, timezone +from pathlib import Path +from typing import Any, Iterator, Optional + +import anyio +import pytest + +pytest.importorskip("mcp") + +import mcp.types as types # noqa: E402 +from mcp import ClientSession # noqa: E402 +from mcp.shared.memory import create_client_server_memory_streams # noqa: E402 + +from openadapt_flow.agent.bridge import AgentBridge # noqa: E402 +from openadapt_flow.agent.mcp import build_server # noqa: E402 +from openadapt_flow.agent.runner import RunnerConfig # noqa: E402 +from openadapt_flow.agent.teach import TeachBridge, TeachConfig # noqa: E402 +from openadapt_flow.mockmed.fault_server import serve # noqa: E402 +from openadapt_flow.tutorial import _http_json, _records # noqa: E402 + +TASK = ( + "Open the referral for Jane Sample, start a Triage encounter, and save " + "this note: {note}" +) +NOTE = "Synthetic follow-up in two weeks" +NEW_NOTE = "Recheck knee in ten days" +AGENT_ID = "agent:mcp-teach-e2e" +PERSON = "dana.reviewer" + + +@pytest.fixture(scope="module") +def mockmed() -> Iterator[str]: + base, _db, stop = serve() + yield base.rstrip("/") + stop() + + +def _reset(base: str): + return lambda: _http_json(f"{base}/api/reset", method="POST", body={}) + + +def _verify_yaml(tmp_path: Path, base: str) -> Path: + path = tmp_path / "verify.yaml" + path.write_text( + "verifier:\n" + " kind: rest\n" + f" base_url: {base}\n" + " records_path: /api/db\n" + " records_key: records\n" + " timeout_s: 2.0\n" + " poll_interval_s: 0.05\n" + ) + return path + + +def _agent( + key_dir: Path, base: str, monkeypatch: pytest.MonkeyPatch, *, allow_self: bool +) -> Any: + """The registered agent the operator gives the server, under a policy a + person signed.""" + + from openadapt_flow.jit import authority + + monkeypatch.setenv(authority.KEY_PASSPHRASE_ENV, "e2e passphrase") + files = key_dir.parent / "keyfiles" + signer_pem, agent_pem = files / "signer.pem", files / "agent.pem" + if not agent_pem.exists(): + for role, pid, pem in ( + ("policy_signer", "dana.signer", signer_pem), + ("agent", AGENT_ID, agent_pem), + ): + authority.register_principal( + key_dir, + role=role, + principal_id=pid, + public_key_hex=authority.generate_encrypted_key(pem), + ) + now = datetime.now(timezone.utc) + policy = authority.AdmissionPolicy( + policy_id="mockmed-teach" + ("" if allow_self else "-no-self"), + signer="dana.signer", + signed_at=(now - timedelta(hours=1)).strftime("%Y-%m-%dT%H:%M:%SZ"), + expires_at=(now + timedelta(days=1)).strftime("%Y-%m-%dT%H:%M:%SZ"), + allowed_hosts=(base.split("://", 1)[1],), + allowed_actions=("click", "type"), + allowed_effects=("record_written", "field_equals"), + min_clean_canaries=2, + agent_principals=(AGENT_ID,), + allow_self_admission=allow_self, + ) + signed = authority.sign_policy(policy, authority.load_encrypted_key(signer_pem)) + return authority.principal_from_key( + authority.load_encrypted_key(agent_pem), key_dir, policy=signed + ) + + +def _server( + tmp_path: Path, base: str, *, admitter: Any = None, fault: str = "ok" +) -> tuple[Any, TeachBridge, AgentBridge]: + runs = tmp_path / "runs" + runs.mkdir(exist_ok=True) + workflows = AgentBridge( + None, RunnerConfig(runs_dir=runs), allow_run=True, mode="production" + ) + teach = TeachBridge( + TeachConfig( + verify=_verify_yaml(tmp_path, base), + cache_dir=tmp_path / "teach", + url=f"{base}/?fault={fault}&idempotency=demo#tasks", + key_dir=tmp_path / "keys", + admitter=admitter, + reset=_reset(base), + ), + workflows=workflows, + ) + return build_server(workflows, teach=teach), teach, workflows + + +class Client: + """A scripted MCP client: a person answers forms, an agent picks refs.""" + + def __init__(self, session: ClientSession, notes: list[str]) -> None: + self.session = session + self.notes = notes + self.list_changed = False + + async def call(self, name: str, arguments: Optional[dict] = None) -> dict: + result = await self.session.call_tool(name, arguments or {}) + payload = json.loads(result.content[0].text) + if getattr(result, "is_error", None) or getattr(result, "isError", None): + return {"_error": payload.get("error")} + return payload + + async def tool_names(self) -> set[str]: + return {t.name for t in (await self.session.list_tools()).tools} + + +@asynccontextmanager +async def connect(server: Any, *, person: Optional[str] = PERSON): + notes: list[str] = [] + + async def elicit(context: Any, params: Any) -> types.ElicitResult: + if person is None: + return types.ElicitResult(action="decline", content={}) + return types.ElicitResult( + action="accept", content={"name": person, "confirmed": True} + ) + + async def on_message(message: Any) -> None: + inner = getattr(message, "root", message) + if isinstance(inner, types.ToolListChangedNotification): + notes.append("tools/list_changed") + + async with create_client_server_memory_streams() as (client, transport): + async with anyio.create_task_group() as tasks: + options = server.create_initialization_options() + tasks.start_soon(server.run, *transport, options) + async with ClientSession( + *client, elicitation_callback=elicit, message_handler=on_message + ) as session: + init = await session.initialize() + client_ = Client(session, notes) + tools = init.capabilities.tools + client_.list_changed = bool( + tools + and ( + getattr(tools, "list_changed", None) + or getattr(tools, "listChanged", None) + ) + ) + yield client_ + tasks.cancel_scope.cancel() + + +def _ref(listing: str, name: str, near: Optional[str] = None) -> str: + for line in listing.splitlines(): + if near and near not in line: + continue + found = re.search(r"\[(e\d+)\] \S+ \"" + re.escape(name) + '"', line) + if found: + return found.group(1) + raise AssertionError(f"no {name!r} in listing:\n{listing}") + + +async def _teach_once(client: Client, patient: str = "Jane Sample") -> dict: + """What an agent does: start, act on refs from each listing, finish.""" + + started = await client.call( + "teach_start", + { + "task": TASK, + "inputs": {"note": NOTE}, + "record_fields": {"patient_id": "p1", "type": "Triage"}, + "on_screen": ["Jane Sample", "MRN P1"], + "name": "triage_note", + }, + ) + assert started.get("status") == "recording", started + listing = started["observation"] + for action in ( + {"action": "click", "ref": _ref(listing, "Open", patient)}, + {"action": "click", "name": "New Encounter"}, + {"action": "click", "name": "Triage"}, + {"action": "type", "input": "note", "name": "Note"}, + {"action": "click", "name": "Save Encounter"}, + ): + if "name" in action: + label = action.pop("name") + action = {**action, "ref": _ref(listing, label)} + result = await client.call("teach_act", action) + if result.get("status") == "refused": + break + assert result.get("status") == "ok", result + listing = result["observation"] + return await client.call("teach_finish") + + +def test_teach_compile_admit_then_call_with_no_model_calls( + mockmed: str, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + async def scenario() -> None: + # Run 1, recorded as the registered agent the operator configured, + # whose policy does not allow self-admission. + no_self = _agent(tmp_path / "keys", mockmed, monkeypatch, allow_self=False) + server, teach, _ = _server(tmp_path, mockmed, admitter=no_self) + async with connect(server) as client: + assert client.list_changed + tools = await client.tool_names() + assert {"teach_start", "teach_act", "teach_finish", "teach_admit"} <= tools + assert "teach_screen" not in tools # the operator didn't turn it on + + draft = await _teach_once(client) + assert draft["status"] == "draft", draft + assert draft["record_check"]["passed"] + assert draft["record_check"]["created"] == 1 + assert [g["decision"] for g in draft["save_gate"]] == ["passed_no_hold"] + assert len(draft["steps"]) >= 5 + assert draft["parameters"]["inputs"] == ["note"] + assert draft["parameters"]["lifted"] == ["note"] + (check,) = draft["proposed_record_check"] + assert check["kind"] == "record_written" + assert check["match"]["patient_id"] == "p1" + assert check["match"]["note"] == "{param: note}" + assert len(draft["needs_person"]) == 1 # confirm the check + task_key = draft["task_key"] + admission = ( + teach.config.cache_dir + / task_key + / "program" + / "bundle" + / "admission.json" + ) + + # The agent can't admit before a person confirms the check. + early = await client.call( + "teach_admit", + {"task_key": task_key, "canary_inputs": [{"note": "Canary"}]}, + ) + assert "confirm the record checks first" in early["_error"], early + + # A person confirms the check in the client's form. + confirmed = await client.call( + "teach_confirm_checks", + {"task_key": task_key, "drop_fields": ["key", "source"]}, + ) + assert confirmed.get("confirmed_by") == PERSON, confirmed + assert confirmed["record_check_confirmed"] + + # The recording agent may not admit its own draft: no canary runs. + before = _records(mockmed) + refused = await client.call( + "teach_admit", + {"task_key": task_key, "canary_inputs": [{"note": "Canary"}]}, + ) + assert refused["status"] == "needs_person", refused + assert "self-admission" in (refused.get("detail") or "") + assert refused["canaries"] == [] + assert _records(mockmed) == before + assert not admission.exists() + + # No policy on the server: refused, with the command a person runs. + server, teach, _ = _server(tmp_path, mockmed) + async with connect(server) as client: + refused = await client.call( + "teach_admit", + {"task_key": task_key, "canary_inputs": [{"note": "Canary"}]}, + ) + assert refused["status"] == "needs_person" + assert "compile-agent admit" in " ".join(refused["needs_person"]) + assert not admission.exists() + + # Under a policy that allows self-admission, the agent admits. + agent = _agent(tmp_path / "keys", mockmed, monkeypatch, allow_self=True) + server, teach, workflows = _server(tmp_path, mockmed, admitter=agent) + async with connect(server) as client: + admitted = await client.call( + "teach_admit", + { + "task_key": task_key, + "canary_inputs": [{"note": "Canary test note"}], + }, + ) + assert admitted["status"] == "admitted", admitted + assert admitted["mode"] == "agent_policy" + assert admitted["admitted_by"] == AGENT_ID + assert [c["status"] for c in admitted["canaries"]] == ["VERIFIED"] * 2 + assert all(c["model_calls"] == 0 for c in admitted["canaries"]) + assert (Path(admitted["skill"]) / "SKILL.md").is_file() + tool = admitted["tool"] + assert client.notes == ["tools/list_changed"] + assert tool in await client.tool_names() + listed = await client.call("list_workflows") + assert [w["name"] for w in listed["workflows"]] == [admitted["workflow"]] + + # Next time: the tool, a new note, no model calls, read back. + ran = await client.call( + tool, {"inputs": {"note": NEW_NOTE}, "request_id": "teach-e2e-0001"} + ) + assert ran["outcome"] == "done", ran + assert ran["model_calls"] == 0 + rows = _records(mockmed) + assert [(r["patient_id"], r["type"], r["note"]) for r in rows] == [ + ("p1", "Triage", NEW_NOTE) + ] + + # A server started later lists it at once. + server, teach, workflows = _server(tmp_path, mockmed, admitter=agent) + async with connect(server) as client: + assert tool in await client.tool_names() + again = await client.call( + "teach_start", + { + "task": TASK, + "inputs": {"note": NOTE}, + "record_fields": {"patient_id": "p1", "type": "Triage"}, + "on_screen": ["Jane Sample", "MRN P1"], + "name": "triage_note", + }, + ) + assert again["started"] is False and again["status"] == "admitted" + + anyio.run(scenario) + + +def test_a_save_on_the_wrong_record_fails_the_read_back( + mockmed: str, tmp_path: Path +) -> None: + async def scenario() -> None: + # MockMed files the note on another patient while the screen shows + # Jane Sample: the gate passes, the read-back does not. + server, teach, _ = _server(tmp_path, mockmed, fault="wrong_record") + async with connect(server) as client: + result = await _teach_once(client) + assert result["status"] == "record_check_failed", result + assert not result["record_check"]["passed"] + assert "wrong record" in result["detail"] + assert result["needs_person"] == [] + status = await client.call("teach_status", {"task_key": result["task_key"]}) + assert status["status"] == "no_draft" + + anyio.run(scenario) + + +def test_the_agent_cannot_confirm_the_check_without_a_person( + mockmed: str, tmp_path: Path +) -> None: + async def scenario() -> None: + server, teach, _ = _server(tmp_path, mockmed) + async with connect(server, person=None) as client: + draft = await _teach_once(client) + assert draft["status"] == "draft", draft + # Extra arguments can't stand in for the person. + forged = await client.call( + "teach_confirm_checks", + {"task_key": draft["task_key"], "person": PERSON}, + ) + assert "_error" in forged + declined = await client.call( + "teach_confirm_checks", {"task_key": draft["task_key"]} + ) + assert "declined" in declined["_error"] + status = await client.call("teach_status", {"task_key": draft["task_key"]}) + assert status["record_check_confirmed"] is False + + anyio.run(scenario) + + +def test_a_person_admits_at_a_terminal_and_the_server_lists_it( + mockmed: str, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys +) -> None: + """No policy: the agent teaches, a person confirms and admits at a terminal + (``openadapt-flow compile-agent admit``), and the next server lists it.""" + + from openadapt_flow.__main__ import build_parser + from openadapt_flow.compile_agent import FirstRunCompiler + from openadapt_flow.compile_agent.cli import cmd_compile_agent + + async def teach() -> dict: + server, _, _ = _server(tmp_path, mockmed) + async with connect(server) as client: + return await _teach_once(client) + + draft = anyio.run(teach) + assert draft["status"] == "draft", draft + command = next(n for n in draft["needs_person"] if "compile-agent admit" in n) + task_dir = tmp_path / "teach" / draft["task_key"] + assert str(task_dir) in command + + # The CLI has no reset hook (a real app is never reset); MockMed's demo + # idempotency key needs one before the canary. + original = FirstRunCompiler._on_reset + + def reset_first(self: FirstRunCompiler) -> None: + _reset(mockmed)() + original(self) + + monkeypatch.setattr(FirstRunCompiler, "_on_reset", reset_first) + # A person's admission needs a real terminal; pytest has none. + monkeypatch.setattr("openadapt_flow.jit.lifecycle._has_terminal", lambda: True) + answers = iter([PERSON, "ok", "ok", "ok"]) + args = build_parser().parse_args( + [ + "compile-agent", + "admit", + str(task_dir), + "--admit-as", + PERSON, + "--drop-field", + "key", + "--drop-field", + "source", + "--canary-param", + "note=Canary test note", + "--json", + ] + ) + code = cmd_compile_agent( + args, ask=lambda _: next(answers), interactive=lambda: True + ) + out = capsys.readouterr().out + result = json.loads(out[out.index("\n{") + 1 :]) + assert code == 0 and result["status"] == "admitted", result + assert result["admission"]["mode"] == "person" + + async def call() -> None: + server, teach_bridge, _ = _server(tmp_path, mockmed) + async with connect(server) as client: + (tool,) = [t for t in await client.tool_names() if t.startswith("taught_")] + ran = await client.call( + tool, {"inputs": {"note": NEW_NOTE}, "request_id": "teach-e2e-0002"} + ) + assert ran["outcome"] == "done" and ran["model_calls"] == 0, ran + + anyio.run(call) diff --git a/packages/flow/tests/test_compile_agent_surface_harness.py b/packages/flow/tests/test_compile_agent_surface_harness.py new file mode 100644 index 000000000..58dd6cf1e --- /dev/null +++ b/packages/flow/tests/test_compile_agent_surface_harness.py @@ -0,0 +1,240 @@ +"""The surface harness and the driven agent, on a fake native window. + +Windows, macOS and Linux apps give the harness an accessibility tree (UI +Automation, Accessibility, AT-SPI) but no way to read which form or row a +control is in. These tests check, with no app and no browser, that every +action that may commit a write waits for a person, that nothing is performed +when it is refused, and that the teach server's configuration keeps an agent +from admitting without a signed policy. +""" + +from __future__ import annotations + +import io +from pathlib import Path +from types import SimpleNamespace +from typing import Any, Optional + +import pytest +from PIL import Image + +from openadapt_flow.compile_agent.driven import DrivenAgent, DrivenRun +from openadapt_flow.compile_agent.harness import ( + GateRefused, + GateRequest, + SaveGate, + StaleRef, + SurfaceHarness, +) +from openadapt_flow.jit.adapters import CaptureComputer + + +def _png() -> bytes: + out = io.BytesIO() + Image.new("RGB", (8, 8), "white").save(out, format="PNG") + return out.getvalue() + + +class FakeSession: + """An authoring session over a native window with three controls.""" + + screen_only = False + + def __init__(self) -> None: + self.nodes = [ + { + "node_id": "n_00000001", + "role": "text_input", + "name": "Note", + "c": (100, 40), + }, + {"node_id": "n_00000002", "role": "button", "name": "Save", "c": (100, 80)}, + { + "node_id": "n_00000003", + "role": "button", + "name": "Open", + "c": (100, 120), + }, + ] + self.performed: list[tuple[Any, ...]] = [] + + def observe(self) -> dict[str, Any]: + return { + "backend": "windows", + "tree": [{k: v for k, v in n.items() if k != "c"} for n in self.nodes], + } + + def node_center(self, node_id: str) -> Optional[tuple[int, int]]: + return next((n["c"] for n in self.nodes if n["node_id"] == node_id), None) + + def click(self, x: int, y: int) -> None: + self.performed.append(("click", x, y)) + + def double_click(self, x: int, y: int) -> None: + self.performed.append(("double_click", x, y)) + + def right_click(self, x: int, y: int) -> None: + self.performed.append(("right_click", x, y)) + + def type_text(self, text: str) -> None: + self.performed.append(("type", text)) + + def press(self, key: str) -> None: + self.performed.append(("press", key)) + + def hotkey(self, modifiers: list[str], key: str) -> None: + self.performed.append(("hotkey", tuple(modifiers), key)) + + def screenshot(self) -> bytes: + return _png() + + +def _harness(hold: Any = None) -> tuple[SurfaceHarness, FakeSession]: + session = FakeSession() + computer = CaptureComputer(session, dialect="generic", sleep=lambda s: None) + harness = SurfaceHarness(computer, SaveGate(["Jane Sample", "MRN P1"], hold)) + harness.observe() + return harness, session + + +def test_the_observation_lists_controls_by_ref_without_values() -> None: + harness, _ = _harness() + obs = harness.observe() + assert obs.text.splitlines() == [ + '[n_00000001] text_input "Note"', + '[n_00000002] button "Save"', + '[n_00000003] button "Open"', + ] + assert obs.find("save").ref == "n_00000002" + assert not obs.screen_only + + +def test_a_save_without_a_person_is_refused_and_not_performed() -> None: + harness, session = _harness() + with pytest.raises(GateRefused, match="a person must approve it"): + harness.click("n_00000002") + assert session.performed == [] + assert harness.gate_log[-1]["decision"] == "refused_no_hold" + assert harness.refusal is not None + + +def test_a_person_approves_or_refuses_each_save() -> None: + asked: list[GateRequest] = [] + harness, session = _harness(hold=lambda req: asked.append(req) or True) + harness.click("n_00000002") + assert session.performed == [("click", 100, 80)] + assert asked[0].control == "Save" and asked[0].identity == ("Jane Sample", "MRN P1") + assert harness.gate_log[-1]["decision"] == "approved_by_hold" + + harness, session = _harness(hold=lambda req: False) + with pytest.raises(GateRefused, match="did not approve"): + harness.click("n_00000002") + assert session.performed == [] + + +@pytest.mark.parametrize( + ("steps", "gated"), + [ + ([("click", "n_00000003")], None), + ([("click", "n_00000001"), ("type", "a note")], None), + ([("click", "n_00000001"), ("press", "Space")], None), + ([("click", "n_00000001"), ("type", "a note\n")], "a typed line break"), + ([("press", "Enter")], "Enter on the focused control"), + ([("hotkey", "s")], "the shortcut Control+s"), + ([("at", (5, 5))], "a click on a point the harness can't name"), + ], +) +def test_what_is_gated(steps: list[tuple[str, Any]], gated: Optional[str]) -> None: + harness, session = _harness() + last = steps[-1] + for kind, arg in steps[:-1]: + getattr(harness, {"click": "click", "type": "type_text"}[kind])(arg) + native = { + "click": lambda a: harness.native_for({"ref": a})[0], + "type": lambda a: {"kind": "type", "text": a}, + "press": lambda a: {"kind": "press", "key": a}, + "hotkey": lambda a: {"kind": "hotkey", "key": a, "modifiers": ["ctrl"]}, + "at": lambda a: {"kind": "click", "x": a[0], "y": a[1]}, + }[last[0]](last[1]) + assert harness.commit_target(native) == gated + if gated is None: + harness.act(native) + else: + before = list(session.performed) + with pytest.raises(GateRefused): + harness.act(native) + assert session.performed == before + + +def test_a_control_that_changed_is_a_stale_ref() -> None: + harness, session = _harness() + session.nodes[1]["name"] = "Delete" + with pytest.raises(StaleRef): + harness.click("n_00000002") + assert session.performed == [] + + +def test_the_driven_agent_approves_only_the_action_a_person_approved_once() -> None: + agent = DrivenAgent() + agent._approved = "Save" + assert not agent.hold(GateRequest("Delete", (), "", None)) + assert not agent.hold(GateRequest("Save", (), "", None)) # used up + agent._approved = "Save" + assert agent.hold(GateRequest("Save", (), "", None)) + + +def test_a_person_approves_every_save_off_the_web() -> None: + for kind, expected in (("web", False), ("macos", True), ("rdp", True)): + run = DrivenRun(SimpleNamespace(backend_kind=kind), {}) + assert run.person_approves_saves is expected + assert DrivenRun(SimpleNamespace(backend_kind="web"), {}, hold=True).hold + + +def _verify(tmp_path: Path) -> Path: + path = tmp_path / "verify.yaml" + path.write_text( + "verifier:\n kind: rest\n base_url: http://127.0.0.1:9\n" + " records_path: /api/db\n records_key: records\n" + ) + return path + + +def test_only_an_agent_that_drives_surfaces_records_one(tmp_path: Path) -> None: + from openadapt_flow.compile_agent import FirstRunCompiler, ScriptedAgent + from openadapt_flow.compile_agent.pipeline import UnsupportedTarget + + compiler = FirstRunCompiler( + "Save this note: {note}", + params=["note"], + surface={"kind": "macos", "macos_app": "TextEdit"}, + verify=_verify(tmp_path), + expect={"patient_id": "p1"}, + on_screen=["Jane Sample"], + cache_dir=tmp_path / "cache", + ) + assert compiler.backend_kind == "macos" and compiler.url is None + with pytest.raises(UnsupportedTarget, match="surface harness"): + compiler.record(ScriptedAgent(steps=[]), {"note": "x"}) + # The spec round-trips with its surface, for runs after a restart. + compiler.save_spec() + again = FirstRunCompiler.load(compiler.dir) + assert again.backend_kind == "macos" + assert again.surface.macos_app == "TextEdit" + + +def test_the_teach_server_admits_only_as_an_agent_under_a_policy( + tmp_path: Path, +) -> None: + from openadapt_flow.agent.teach import TeachConfig + + common = {"verify": _verify(tmp_path), "cache_dir": tmp_path / "teach"} + with pytest.raises(ValueError, match="needs the app"): + TeachConfig(**common) + with pytest.raises(ValueError, match="needs the app"): + TeachConfig(**common, url="http://x", surface={"kind": "macos"}) + key = SimpleNamespace(kind="admitter_key", policy=None, principal_id="ci:x") + with pytest.raises(ValueError, match="registered agent under a signed"): + TeachConfig(**common, url="http://x", admitter=key) + no_policy = SimpleNamespace(kind="agent", policy=None, principal_id="agent:x") + with pytest.raises(ValueError, match="registered agent under a signed"): + TeachConfig(**common, url="http://x", admitter=no_policy) diff --git a/packages/flow/tests/test_mcp_teach_cli.py b/packages/flow/tests/test_mcp_teach_cli.py new file mode 100644 index 000000000..c36a5282b --- /dev/null +++ b/packages/flow/tests/test_mcp_teach_cli.py @@ -0,0 +1,164 @@ +"""``openadapt-agent serve --teach``: what the operator fixes at start. + +No app and no browser: ``serve`` is replaced so each test sees the bridges the +command built, or its refusal. +""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any + +import pytest + +pytest.importorskip("mcp") + +from openadapt_flow.agent.cli import main # noqa: E402 + + +@pytest.fixture() +def served(monkeypatch: pytest.MonkeyPatch) -> list[dict[str, Any]]: + calls: list[dict[str, Any]] = [] + + def fake_serve(bridge: Any = None, **kwargs: Any) -> None: + calls.append({"bridge": bridge, **kwargs}) + + monkeypatch.setattr("openadapt_flow.agent.mcp.serve", fake_serve) + return calls + + +@pytest.fixture() +def verify(tmp_path: Path) -> Path: + path = tmp_path / "verify.yaml" + path.write_text( + "verifier:\n kind: rest\n base_url: http://127.0.0.1:9\n" + " records_path: /api/db\n records_key: records\n" + ) + return path + + +def _deploy(tmp_path: Path, backend: str) -> Path: + path = tmp_path / "deploy.yaml" + path.write_text(f"name: notes\nbackend:\n{backend}") + return path + + +def test_a_web_app(served: list, verify: Path, tmp_path: Path) -> None: + code = main( + [ + "serve", + "--teach", + str(verify), + "--url", + "http://127.0.0.1:9/#tasks", + "--runs-dir", + str(tmp_path / "runs"), + ] + ) + assert code == 0 + teach = served[0]["teach"] + assert teach.backend_kind == "web" and teach.config.admitter is None + assert teach.config.cache_dir == (tmp_path / "runs" / "teach").resolve() + bridge = served[0]["bridge"] + assert bridge.allow_run and bridge.catalog == {} + names = {spec.name for spec in teach.list_tool_specs()} + assert {"teach_start", "teach_act", "teach_finish", "teach_admit"} <= names + assert "teach_screen" not in names + + +def test_a_native_app_from_the_deployment( + served: list, verify: Path, tmp_path: Path +) -> None: + deploy = _deploy(tmp_path, " kind: macos\n macos_app: TextEdit\n") + code = main( + [ + "serve", + "--teach", + str(verify), + "--config", + str(deploy), + "--teach-screens", + "scrubbed", + "--runs-dir", + str(tmp_path / "runs"), + ] + ) + assert code == 0 + teach = served[0]["teach"] + assert teach.backend_kind == "macos" + assert "teach_screen" in {spec.name for spec in teach.list_tool_specs()} + + +@pytest.mark.parametrize( + ("extra", "backend", "reason"), + [ + ([], None, "needs the app"), + (["--url", "http://x", "--teach-key", "a.pem"], None, "go together"), + ([], " kind: rdp\n rdp_host: 10.0.0.5\n", "network RDP"), + (["--url", "http://x", "--authoring"], None, "own recording tools"), + (["--url", "http://x", "--bundles", "b"], None, "--mode production"), + ], +) +def test_refusals( + served: list, + verify: Path, + tmp_path: Path, + capsys, + extra: list, + backend: Any, + reason: str, +) -> None: + argv = ["serve", "--teach", str(verify), "--runs-dir", str(tmp_path / "r"), *extra] + if backend: + argv += ["--config", str(_deploy(tmp_path, backend))] + assert main(argv) == 2 + assert reason in capsys.readouterr().err + assert served == [] + + +def test_teach_flags_need_teach(served: list, capsys) -> None: + assert main(["serve", "--teach-hold"]) == 2 + assert "need --teach" in capsys.readouterr().err + + +def test_a_verifier_that_cannot_be_read_is_refused_at_start( + served: list, tmp_path: Path, capsys +) -> None: + assert ( + main(["serve", "--teach", str(tmp_path / "missing.yaml"), "--url", "http://x"]) + == 2 + ) + assert "is not a file" in capsys.readouterr().err + + +def test_a_scrubbed_screen_that_cannot_be_scrubbed_is_not_shared( + verify: Path, tmp_path: Path +) -> None: + import threading + + from openadapt_flow.agent.bridge import BridgeError + from openadapt_flow.agent.teach import TeachBridge, TeachConfig + + def broken() -> Any: + raise RuntimeError("no scrubber") + + teach = TeachBridge( + TeachConfig( + verify=verify, + cache_dir=tmp_path / "teach", + url="http://127.0.0.1:9", + screens="scrubbed", + scrubber_factory=broken, + ) + ) + + class Run: + done = threading.Event() + + def call(self, kind: str, payload: Any = None) -> dict: + assert kind == "screenshot" + return {"png": b"\x89PNG raw frame"} + + teach._run = Run() + with pytest.raises(BridgeError, match="no screen was shared"): + teach.dispatch("teach_screen", {}) diff --git a/packages/flow/tests/test_mcp_teach_surfaces.py b/packages/flow/tests/test_mcp_teach_surfaces.py new file mode 100644 index 000000000..2d734bee8 --- /dev/null +++ b/packages/flow/tests/test_mcp_teach_surfaces.py @@ -0,0 +1,283 @@ +"""The MCP teach tools on a remote session (RDP, Citrix), through the real backends. + +MockMed runs in a browser page that Flow never reads: tests/surface_kiosk +shows it to ``RemoteDisplayBackend`` and ``CitrixWorkspaceBackend`` as a +client window, so Flow sees pixels only. A scripted MCP client teaches the +task with pixel clicks, as an agent reading the screen would. + +What it checks: + +* the observation is screen-only and the screen tool works when the operator + turned it on; +* every click a person must approve in the client's form (the harness can't + read which record a remote screen shows), and without a form nothing is + clicked; +* run 1 compiles, passes its record check against the system of record, and + a person confirms the check; +* admission fails closed with its reason: the canary is refused because a + pixel-only click carries no verified record identity (docs/BACKEND_PARITY.md). +""" + +from __future__ import annotations + +import json +from contextlib import asynccontextmanager +from datetime import datetime, timedelta, timezone +from pathlib import Path +from typing import Any, Iterator, Optional + +import pytest + +pytest.importorskip("mcp") +pytest.importorskip("playwright.sync_api") + +import anyio # noqa: E402 +import mcp.types as types # noqa: E402 +from mcp import ClientSession # noqa: E402 +from mcp.shared.memory import create_client_server_memory_streams # noqa: E402 + +from openadapt_flow.agent.bridge import AgentBridge # noqa: E402 +from openadapt_flow.agent.mcp import build_server # noqa: E402 +from openadapt_flow.agent.runner import RunnerConfig # noqa: E402 +from openadapt_flow.agent.teach import TeachBridge, TeachConfig # noqa: E402 +from openadapt_flow.mockmed.fault_server import serve # noqa: E402 +from openadapt_flow.tutorial import _http_json, _records # noqa: E402 +from tests.surface_kiosk import PageWindowClient # noqa: E402 + +TITLE = "MockMed kiosk" +SECTIONS = { + "rdp": { + "kind": "rdp", + "rdp_window": "mstsc", + "rdp_window_title": TITLE, + "rdp_max_frame_age_s": 30.0, + }, + "citrix": { + "kind": "citrix", + "rdp_window": "Citrix Viewer", + "rdp_window_title": TITLE, + "rdp_readiness_text": "MockMed", + "rdp_max_frame_age_s": 30.0, + }, +} +TASK = "Open the first referral, start a Triage encounter, and save this note: {note}" +NOTE = "Synthetic follow-up in two weeks" +AGENT_ID = "agent:mcp-teach-surface" +#: Where MockMed's controls are at 1280x800, as an agent reading the screen +#: would find them (the same points tests/fixtures/jit/anthropic_add_note.json +#: clicks). +CLICKS = [(775, 186), (114, 159), (85, 214), (344, 350)] +SAVE = (134, 457) + + +@pytest.fixture(scope="module") +def mockmed() -> Iterator[str]: + base, _db, stop = serve() + yield base.rstrip("/") + stop() + + +def _open_backend(base: str): + """Build each backend through the factory, with a fresh kiosk page as the + client window. Each call opens its own browser on the calling thread.""" + + from playwright.sync_api import sync_playwright + + from openadapt_flow._browser_setup import launch_chromium + from openadapt_flow.backends.factory import build_backend + + def open_backend(cfg: Any) -> Any: + pw = sync_playwright().start() + try: + browser = launch_chromium(pw.chromium, headless=True) + except Exception as exc: # noqa: BLE001 - no browser on this host + pw.stop() + pytest.skip(f"chromium unavailable: {exc}") + page = browser.new_page( + viewport={"width": 1280, "height": 800}, device_scale_factor=1 + ) + page.goto(f"{base}/?fault=ok&idempotency=demo#tasks") + page.wait_for_timeout(300) + backend = build_backend( + cfg, window_client=PageWindowClient(page, owner=cfg.rdp_window, title=TITLE) + ) + + def close() -> None: + browser.close() + pw.stop() + + backend.close = close # type: ignore[attr-defined] + return backend + + return open_backend + + +def _admitter(key_dir: Path, origin: str, monkeypatch: pytest.MonkeyPatch) -> Any: + from openadapt_flow.jit import authority + + monkeypatch.setenv(authority.KEY_PASSPHRASE_ENV, "surface passphrase") + files = key_dir.parent / "keyfiles" + for role, pid, name in ( + ("policy_signer", "dana.signer", "signer"), + ("agent", AGENT_ID, "agent"), + ): + authority.register_principal( + key_dir, + role=role, + principal_id=pid, + public_key_hex=authority.generate_encrypted_key(files / f"{name}.pem"), + ) + now = datetime.now(timezone.utc) + policy = authority.AdmissionPolicy( + policy_id="kiosk", + signer="dana.signer", + signed_at=(now - timedelta(hours=1)).strftime("%Y-%m-%dT%H:%M:%SZ"), + expires_at=(now + timedelta(days=1)).strftime("%Y-%m-%dT%H:%M:%SZ"), + allowed_hosts=(origin,), + allowed_actions=("click", "type"), + allowed_effects=("record_written", "field_equals"), + min_clean_canaries=1, + agent_principals=(AGENT_ID,), + allow_self_admission=True, + ) + signed = authority.sign_policy( + policy, authority.load_encrypted_key(files / "signer.pem") + ) + return authority.principal_from_key( + authority.load_encrypted_key(files / "agent.pem"), key_dir, policy=signed + ) + + +@asynccontextmanager +async def connect(server: Any, *, person: Optional[str]): + async def elicit(context: Any, params: Any) -> types.ElicitResult: + if person is None: + return types.ElicitResult(action="decline", content={}) + return types.ElicitResult( + action="accept", content={"name": person, "confirmed": True} + ) + + 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() + ) + kwargs = {"elicitation_callback": elicit} if person else {} + async with ClientSession(*client, **kwargs) as session: + await session.initialize() + yield session + tasks.cancel_scope.cancel() + + +async def _call(session: ClientSession, name: str, args: Optional[dict] = None) -> dict: + result = await session.call_tool(name, args or {}) + payload = json.loads(result.content[0].text) + if getattr(result, "is_error", None) or getattr(result, "isError", None): + return {"_error": payload.get("error")} + return payload + + +@pytest.mark.timeout(600) +@pytest.mark.parametrize("kind", ["rdp", "citrix"]) +def test_teach_on_a_remote_window( + mockmed: str, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, kind: str +) -> None: + from openadapt_flow.jit.compile_agent import SurfaceApp + + section = SECTIONS[kind] + verify = tmp_path / "verify.yaml" + verify.write_text( + "verifier:\n kind: rest\n" + f" base_url: {mockmed}\n records_path: /api/db\n records_key: records\n" + " timeout_s: 2.0\n poll_interval_s: 0.05\n" + ) + runs = tmp_path / "runs" + runs.mkdir() + workflows = AgentBridge( + None, RunnerConfig(runs_dir=runs), allow_run=True, mode="production" + ) + teach = TeachBridge( + TeachConfig( + verify=verify, + cache_dir=tmp_path / "teach", + surface=section, + key_dir=tmp_path / "keys", + admitter=_admitter( + tmp_path / "keys", SurfaceApp(section).origin, monkeypatch + ), + screens="unscrubbed", + reset=lambda: _http_json(f"{mockmed}/api/reset", method="POST", body={}), + open_backend=_open_backend(mockmed), + ), + workflows=workflows, + ) + server = build_server(workflows, teach=teach) + start = { + "task": TASK, + "inputs": {"note": NOTE}, + "record_fields": {"patient_id": "p1", "type": "Triage"}, + "on_screen": ["Jane Sample", "MRN P1"], + } + + async def scenario() -> None: + # A client with no form: the first click needs a person, so nothing + # is clicked and run 1 keeps going. + async with connect(server, person=None) as session: + started = await _call(session, "teach_start", start) + assert started["surface"] == kind and started["person_approves_saves"] + assert "remote session" in started["observation"] + screen = await _call(session, "teach_screen") + assert len(screen["screen_png_base64"]) > 1000 + x, y = CLICKS[0] + refused = await _call( + session, "teach_act", {"action": "click", "x": x, "y": y} + ) + assert "needs a person's approval" in refused["_error"] + assert _records(mockmed) == [] + assert (await _call(session, "teach_abort"))["status"] == "aborted" + + async with connect(server, person="dana.reviewer") as session: + started = await _call(session, "teach_start", start) + assert started["status"] == "recording", started + for x, y in CLICKS: + done = await _call( + session, "teach_act", {"action": "click", "x": x, "y": y} + ) + assert done["status"] == "ok", done + typed = await _call( + session, "teach_act", {"action": "type", "input": "note"} + ) + assert typed["status"] == "ok", typed + x, y = SAVE + saved = await _call( + session, "teach_act", {"action": "click", "x": x, "y": y} + ) + assert saved["status"] == "ok", saved + draft = await _call(session, "teach_finish") + assert draft["status"] == "draft", draft + assert draft["surface"] == kind + assert draft["record_check"]["passed"] + assert {g["decision"] for g in draft["save_gate"]} == {"approved_by_hold"} + assert draft["parameters"]["lifted"] == ["note"] + + confirmed = await _call( + session, + "teach_confirm_checks", + {"task_key": draft["task_key"], "drop_fields": ["key", "source"]}, + ) + assert confirmed["confirmed_by"] == "dana.reviewer", confirmed + + saved_rows = _records(mockmed) + admitted = await _call( + session, + "teach_admit", + {"task_key": draft["task_key"], "canary_inputs": [{"note": "Canary"}]}, + ) + assert admitted["status"] == "canary_failed", admitted + assert "Identity coverage" in admitted["detail"] + assert _records(mockmed) == saved_rows # refused before any input + assert not any( + t.name.startswith("taught_") for t in (await session.list_tools()).tools + ) + + anyio.run(scenario) From a8e1eb02538f10c3d508d399e7a11193d19833fa Mon Sep 17 00:00:00 2001 From: abrichr Date: Mon, 19 Oct 2026 09:42:20 -0400 Subject: [PATCH 2/4] docs: link the teach-over-MCP PR in CAPABILITIES.md Co-Authored-By: Claude Opus 5.5 --- CAPABILITIES.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CAPABILITIES.md b/CAPABILITIES.md index 00b5c2140..8d64b2152 100644 --- a/CAPABILITIES.md +++ b/CAPABILITIES.md @@ -23,7 +23,8 @@ shipping in the next release; a pull request number means it is being merged. instead of screenshots, [#1305](https://github.com/OpenAdaptAI/OpenAdapt/pull/1305)). Agents can drive the recording on web, native Windows, RDP and Citrix. An agent connected over MCP can teach itself a new workflow and then call it as a tool - (`openadapt-agent serve --teach`, in review). + (`openadapt-agent serve --teach`, + [#1335](https://github.com/OpenAdaptAI/OpenAdapt/pull/1335)). 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 own traffic allows them (`ApiActuator`, released; compiled from captured @@ -204,7 +205,7 @@ improvising or retrying when something is off. | 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](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](packages/agent/README.md) | -| Teach over MCP: an agent does a new task once through OpenAdapt; the run is read back, compiled and admitted (a person confirms the record check; the agent admits only under a person-signed policy), and becomes a tool it calls with no model calls. Web, and Windows, macOS, Linux, RDP and Citrix with a person approving each save | In review | Synthetic (MockMed over MCP, and through the real RDP and Citrix backends) | `openadapt-agent serve --teach`, [packages/agent](packages/agent/README.md#teach-a-new-workflow-from-your-agent) | +| Teach over MCP: an agent does a new task once through OpenAdapt; the run is read back, compiled and admitted (a person confirms the record check; the agent admits only under a person-signed policy), and becomes a tool it calls with no model calls. Web, and Windows, macOS, Linux, RDP and Citrix with a person approving each save | In review ([#1335](https://github.com/OpenAdaptAI/OpenAdapt/pull/1335)) | Synthetic (MockMed over MCP, and through the real RDP and Citrix backends) | `openadapt-agent serve --teach`, [packages/agent](packages/agent/README.md#teach-a-new-workflow-from-your-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)) | Synthetic | `compile-agent` | | 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/` | From 328e4fb0acc8d758a04e7c9d89f18e7f63a23cbf Mon Sep 17 00:00:00 2001 From: abrichr Date: Mon, 19 Oct 2026 09:56:00 -0400 Subject: [PATCH 3/4] fix(agent): bind each person's approval to one action; gate more off the web Review fixes for the MCP teach tools: - An approval covers one run, one exact action and the app as it was (run id, action, actions performed so far). A changed screen, another run or another action in between refuses it; nothing is done. - Off the web, every click is gated unless it is on a ref'd text field, check box, tab or a plainly navigating control; clicks at x,y, Enter, typing outside a text field, single-letter keys and shortcuts are gated. Focus resets after Tab and other focus keys, so Space after Tab is gated. The gate uses the ref's control, not a coordinate match. - On the web, typing a space on a focused button is gated. - type with a ref refuses a ref that isn't a text field. - An action that times out ends run 1 instead of running later. - A run that fails to start stays tracked until its thread ends. - One app lock for run 1, canaries and taught runs. - The server stops an unfinished run 1 on exit. Co-Authored-By: Claude Opus 5.5 --- packages/agent/README.md | 4 +- packages/flow/docs/COMPILE_AGENT.md | 11 +- packages/flow/openadapt_flow/agent/cli.py | 5 +- packages/flow/openadapt_flow/agent/mcp.py | 8 +- packages/flow/openadapt_flow/agent/teach.py | 74 ++++++-- .../openadapt_flow/compile_agent/driven.py | 90 ++++++++-- .../openadapt_flow/compile_agent/harness.py | 161 ++++++++++++++---- .../test_compile_agent_surface_harness.py | 123 ++++++++++--- 8 files changed, 377 insertions(+), 99 deletions(-) diff --git a/packages/agent/README.md b/packages/agent/README.md index b24472fae..4390e3ca1 100644 --- a/packages/agent/README.md +++ b/packages/agent/README.md @@ -412,7 +412,7 @@ openadapt-flow compile-agent admit --admit-as "" --canary- Once the program is admitted, it is in `list_workflows` and `run_workflow` at once and has its own `taught_` tool, and the server sends `notifications/tools/list_changed` to clients that support it. A server started later lists every program taught in its `--teach-dir`. -It works on a web app (`--url`) and on the app a deployment file's `backend:` section names (`--config deploy.yaml`): a native Windows, macOS or Linux window, or a local RDP or Citrix client window. Off the web, OpenAdapt can't read which record a form shows, so a person approves every save in your client's form, and a client without forms can't save. RDP and Citrix have no control tree, and the Windows, macOS and Linux backends don't list controls yet, so off the web your agent needs `--teach-screens scrubbed` to see the screen, and clicks x,y pixels. Admission there fails closed for now: a pixel-only click carries no checked record identity, so the canary is refused and says why. Network RDP (`rdp_host`) is refused. +It works on a web app (`--url`) and on the app a deployment file's `backend:` section names (`--config deploy.yaml`): a native Windows, macOS or Linux window, or a local RDP or Citrix client window. Off the web, OpenAdapt can't read which record a form shows, so a person approves, in your client's form, every action that might save: any click except on a text field, a check box, a tab or a control that plainly navigates (Open, New, Cancel), any click at x,y, Enter, typing outside a text field, and shortcuts. Each answer covers that one action on the screen as it was; if anything changes first, the action isn't done. A client without forms can't save. RDP and Citrix have no control tree, and the Windows, macOS and Linux backends don't list controls yet, so off the web your agent needs `--teach-screens scrubbed` to see the screen, and clicks x,y pixels. Admission there fails closed for now: a pixel-only click carries no checked record identity, so the canary is refused and says why. Network RDP (`rdp_host`) is refused. | Flag | What it does | | --- | --- | @@ -420,7 +420,7 @@ It works on a web app (`--url`) and on the app a deployment file's `backend:` se | `--teach-dir DIR` | Where drafts and taught programs live (default `RUNS_DIR/teach`). | | `--teach-key PEM`, `--teach-policy JSON` | The registered agent's encrypted key (passphrase in `OPENADAPT_JIT_KEY_PASSPHRASE`) and the signed admission policy. Only an agent key under a policy; an admitter key admits from CI with `openadapt-flow jit admit`. | | `--key-dir DIR` | The JIT key directory (default `OPENADAPT_JIT_KEY_DIR` or `~/.openadapt/jit`). | -| `--teach-hold` | On the web, a person also approves every save in the first run. | +| `--teach-hold` | On the web, a person also approves every save in the first run. Without it, your agent names the identity the save gate checks (`on_screen`); a value every form shows proves nothing, and then only the record check after the run and the person confirming it catch a wrong record. | | `--teach-screens scrubbed` | Adds `teach_screen`: the app's screen with identifiers scrubbed, and nothing shared if scrubbing fails. `unscrubbed` skips scrubbing. Off by default. | | `--mode production --bundles DIR` | Serves your other workflows next to the taught ones. | diff --git a/packages/flow/docs/COMPILE_AGENT.md b/packages/flow/docs/COMPILE_AGENT.md index e1c22f4fb..4ac427780 100644 --- a/packages/flow/docs/COMPILE_AGENT.md +++ b/packages/flow/docs/COMPILE_AGENT.md @@ -221,9 +221,14 @@ gets a `taught_` tool. See the The MCP tools also run on Windows, macOS, Linux, RDP and Citrix: pass `FirstRunCompiler(surface=)`. Off the web the harness is `SurfaceHarness`. It reads the authoring session's -accessibility tree (no values, no window titles) and gates the same actions, -but it can't read which form or row a control is in, so a person approves -every gated action (the hold); without a hold, a gated action is refused. On +accessibility tree (no values, no window titles). It can't read which form or +row a control is in, so it gates more than the web harness: every click except +one, named by its ref, on a text field, a check box, a tab or a control whose +label plainly navigates (Open, New, Cancel), and every click at x,y, Enter, +typing outside a text field, single-letter keys and shortcuts. A person +approves each gated action (the hold); without a hold, it is refused. Over MCP +the approval is bound to the run, the exact action and the number of actions +performed so far, so it covers only what the person was shown. On RDP and Citrix the canary is refused until pixel identity is qualified ([BACKEND_PARITY.md](BACKEND_PARITY.md)). diff --git a/packages/flow/openadapt_flow/agent/cli.py b/packages/flow/openadapt_flow/agent/cli.py index e05e722ea..4eb048f26 100644 --- a/packages/flow/openadapt_flow/agent/cli.py +++ b/packages/flow/openadapt_flow/agent/cli.py @@ -1095,7 +1095,10 @@ def _serve_teach(args: argparse.Namespace, serve) -> int: + f"; {len(teach.taught)} taught workflow(s); runs folder {runs_dir}", file=sys.stderr, ) - serve(bridge, teach=teach) + try: + serve(bridge, teach=teach) + finally: + teach.close() return 0 diff --git a/packages/flow/openadapt_flow/agent/mcp.py b/packages/flow/openadapt_flow/agent/mcp.py index 99fa43848..94f3dac2b 100644 --- a/packages/flow/openadapt_flow/agent/mcp.py +++ b/packages/flow/openadapt_flow/agent/mcp.py @@ -380,13 +380,13 @@ async def _call_tool( await _confirm_admit(context, authoring) human_confirmed = True elif teaching and name == "teach_act": - commit = await anyio.to_thread.run_sync( + hold = await anyio.to_thread.run_sync( teach.hold_needed, arguments or {} ) - if commit: + if hold: await _confirm_with_person( context, - teach.hold_message(commit), + teach.hold_message(hold), title="I am a person and I approve this save", unsupported=( "this save needs a person's approval in an MCP client " @@ -394,7 +394,7 @@ async def _call_tool( ), declined="the person did not approve the save; nothing was done", ) - held_by_person = commit + held_by_person = hold["binding"] elif teaching and name == "teach_confirm_checks": message = await anyio.to_thread.run_sync( teach.confirm_checks_message, arguments or {} diff --git a/packages/flow/openadapt_flow/agent/teach.py b/packages/flow/openadapt_flow/agent/teach.py index 567138cd3..ad4f8207d 100644 --- a/packages/flow/openadapt_flow/agent/teach.py +++ b/packages/flow/openadapt_flow/agent/teach.py @@ -41,7 +41,7 @@ import base64 import re import threading -from dataclasses import dataclass +from dataclasses import dataclass, field from pathlib import Path from typing import Any, Callable, Mapping, Optional @@ -128,6 +128,12 @@ class TeachConfig: reset: Optional[Callable[[], None]] = None #: Tests only: build the live backend for a surface. open_backend: Optional[Callable[[Any], Any]] = None + #: How long a taught run waits for run 1 or a canary to finish. + app_wait_s: float = 600.0 + + #: One writer at a time on the app: run 1, the canaries and taught runs + #: would otherwise count each other's records in their checks. + app_lock: threading.Lock = field(default_factory=threading.Lock, repr=False) def __post_init__(self) -> None: self.cache_dir = Path(self.cache_dir).resolve() @@ -185,7 +191,13 @@ def execute( reset=self.config.reset, open_backend=self.config.open_backend, ) - replay = compiler.run(**inputs) + if not self.config.app_lock.acquire(timeout=self.config.app_wait_s): + # A run 1 or a canary is still writing to the app. Nothing ran. + return EngineResult(reason="not_ready_to_run") + try: + replay = compiler.run(**inputs) + finally: + self.config.app_lock.release() if replay.verified: reason = "saved_and_checked" elif replay.status == "not_admitted": @@ -473,27 +485,30 @@ def _taught_spec(self, tool: str, workflow: str) -> ToolSpec: # -- what the MCP layer asks a person ------------------------------------------ - def hold_needed(self, arguments: Mapping[str, Any]) -> Optional[str]: - """For ``teach_act``: what this action may commit, when a person must - approve it first. None when no approval is needed.""" + def hold_needed(self, arguments: Mapping[str, Any]) -> Optional[dict[str, Any]]: + """For ``teach_act``: when a person must approve this action first, + what it may commit, a description, and the binding the approval + covers (this run, this action, the app as it is now). Else None.""" run = self._active() if not run.person_approves_saves: return None answer = self._run_call("preview", arguments) - if "error" in answer: - return None # teach_act reports the same error, performing nothing - return answer.get("commit") + if "error" in answer or not answer.get("commit"): + return None # teach_act reports any error, performing nothing + return answer - def hold_message(self, commit: str) -> str: + def hold_message(self, hold: Mapping[str, Any]) -> str: + commit = str(hold.get("commit")) task = self._compiler.task if self._compiler is not None else None identity = ", ".join(self._compiler.on_screen) if self._compiler else "" return ( "The agent is teaching OpenAdapt a task" + (f" ({task.task_id})" if task is not None else "") - + f" and is about to do {commit!r}, which may save a record. Look at " - f"the app: is the record {identity} on screen, and should this save " - "happen? It is a real entry in this app." + + f" and is about to {hold.get('action')} ({commit}), which may " + f"save a record. Look at the app: is the record {identity} on " + "screen, and should this happen? It is a real entry in this app. " + "Your answer covers only this action on the screen as it is now." ) def confirm_checks_message(self, arguments: Mapping[str, Any]) -> str: @@ -620,20 +635,30 @@ def _start(self, args: Mapping[str, Any]) -> dict[str, Any]: jit = compiler._jit() if jit.admitted or jit.candidate() is not None: return {"started": False, **self._status(compiler.task.key)} + if not self.config.app_lock.acquire(blocking=False): + raise BridgeError( + "a canary or a taught run is writing to the app; try " + "teach_start again when it is done" + ) admitter = self.config.admitter run = DrivenRun( compiler, inputs, principal_id=admitter.principal_id if admitter is not None else None, hold=self.config.hold, + on_done=self.config.app_lock.release, ) + # Kept even if it fails to start: until its thread ends, no other + # run 1 may start on the app. + self._run, self._compiler = run, compiler try: run.start() except Exception as exc: # A run that opens late must not wait for actions forever. run.abort() - raise BridgeError(f"run 1 did not start: {exc}") from None - self._run, self._compiler = run, compiler + raise BridgeError( + f"run 1 did not start: {exc}; it is being stopped" + ) from None observation = self._run_call("observe") return { "started": True, @@ -855,9 +880,16 @@ def _admit(self, args: Mapping[str, Any]) -> dict[str, Any]: ] def admit_and_emit() -> Any: - result = compiler.admit( - admitter=self.config.admitter, canary_params=canaries - ) + if not self.config.app_lock.acquire(blocking=False): + raise BridgeError( + "run 1 or a taught run is writing to the app; admit when it is done" + ) + try: + result = compiler.admit( + admitter=self.config.admitter, canary_params=canaries + ) + finally: + self.config.app_lock.release() if result.admitted: result.emitted = compiler.emit() return result @@ -893,6 +925,14 @@ def admit_and_emit() -> Any: ) return out + def close(self, timeout_s: float = 30.0) -> None: + """Stop a run 1 the agent never finished (server exit). No draft.""" + + run = self._run + if run is not None and not run.done.is_set(): + run.abort() + run.done.wait(timeout_s) + # -- admitted programs as workflows --------------------------------------------------- @staticmethod diff --git a/packages/flow/openadapt_flow/compile_agent/driven.py b/packages/flow/openadapt_flow/compile_agent/driven.py index 90e2fbae0..486c614dc 100644 --- a/packages/flow/openadapt_flow/compile_agent/driven.py +++ b/packages/flow/openadapt_flow/compile_agent/driven.py @@ -17,13 +17,15 @@ from __future__ import annotations +import json import queue import threading import time +import uuid from concurrent.futures import Future from concurrent.futures import TimeoutError as FutureTimeout from dataclasses import dataclass -from typing import Any, Mapping, Optional +from typing import Any, Callable, Mapping, Optional from openadapt_flow.compile_agent.agents import AgentLedger from openadapt_flow.compile_agent.harness import GateRefused, GateRequest, StaleRef @@ -70,6 +72,10 @@ def __init__(self, principal_id: Optional[str] = None) -> None: self._inbox: "queue.Queue[Any]" = queue.Queue() self._approved: Optional[str] = None self.ledger = AgentLedger(agent="driven") + #: This run, and how many actions it has performed: a person's + #: approval is bound to both (see :meth:`binding`). + self.run_id = uuid.uuid4().hex + self.performed = 0 # -- called by the gate, on the run thread ----------------------------------- @@ -119,29 +125,58 @@ def _handle(self, command: _Command) -> dict[str, Any]: # The caller decides how to scrub it before it leaves the computer. h.stats.screenshots_requested += 1 return {"png": h.computer.screenshot()} - native, focus = self._native(command.payload) + native, ref = self._native(command.payload) + field = ref if native["kind"] == "type" else None + if field is not None: + # type with a ref clicks the field first. That click must commit + # nothing, so it never needs its own approval. + focus_click, _ = h.native_for({"ref": field}) + if h.commit_target(focus_click, field) is not None: + raise ValueError( + f"{field!r} is not a text field; click it with its own teach_act" + ) + commit = h.commit_target(native, ref) + binding = self.binding(native, ref) if command.kind == "preview": - return {"commit": h.commit_target(native)} + return { + "commit": commit, + "binding": binding if commit else None, + "action": describe(native), + } if command.kind == "act": - if focus is not None: - # Focus the field first; a click on a text field commits nothing. - h.click(focus) - # Only the control the person was shown is approved. If something - # else would commit now, the gate asks again and the hold says no. held = command.payload.get("_held_by_person") - self._approved = str(held) if held else None + if held and held != binding: + raise ValueError( + "the app or the run changed after the person approved this " + "action, so nothing was done; call teach_act again" + ) + if field is not None: + h.click(field) + self.performed += 1 + # Only what the person was shown is approved, once. + self._approved = commit if held else None try: - h.act(native) + h.act(native, ref) finally: self._approved = None + self.performed += 1 return {"observation": h.observe().text} raise ValueError(f"unknown command {command.kind!r}") + def binding(self, native: Mapping[str, Any], ref: Optional[str]) -> str: + """What a person's approval covers: this run, this exact action, and + the app as it was after the last action performed.""" + + return json.dumps( + [self.run_id, self.performed, dict(native), ref], sort_keys=True + ) + def _native( self, action: Mapping[str, Any] ) -> tuple[dict[str, Any], Optional[str]]: - """A caller's action as the harness's generic action, and the ref of - a field to click first (``type`` with a ``ref``). Performs nothing.""" + """A caller's action as the harness's generic action, and its ref: the + control a click is on, or the field a ``type`` goes into (clicked + first). Performs nothing.""" kind = action.get("action") if kind not in ACTIONS: @@ -202,7 +237,9 @@ def __init__( *, principal_id: Optional[str] = None, hold: bool = False, + on_done: Optional[Callable[[], None]] = None, ) -> None: + self.on_done = on_done self.compiler = compiler self.params = dict(params) self.agent = DrivenAgent(principal_id=principal_id) @@ -254,6 +291,8 @@ def _main(self) -> None: self.error = exc finally: self.done.set() + if self.on_done is not None: + self.on_done() def call( self, @@ -281,7 +320,13 @@ def call( if self.done.is_set() and not future.done(): raise RunEnded("run 1 is over") if time.monotonic() > deadline: - raise TimeoutError("the action did not finish in time") + # It may still run later; never let it run behind the + # caller's back or twice after a retry. End run 1 instead. + self.abort() + raise RunEnded( + "the action did not finish in time, so run 1 was stopped; " + "check the app, then start again" + ) def finish(self, timeout_s: float = 600.0) -> Any: """End run 1, then wait for the compile and the record check.""" @@ -298,3 +343,22 @@ def abort(self) -> None: """End run 1 now. Nothing more is performed and no draft is made.""" self.agent.stop() + + +def describe(native: Mapping[str, Any]) -> str: + """One line a person reads before approving ``native``. No typed text.""" + + kind = native.get("kind") + if kind == "click": + return f"click at x={native.get('x')}, y={native.get('y')}" + if kind == "type": + text = str(native.get("text") or "") + ending = " ending in a line break" if text.endswith(("\n", "\r")) else "" + return f"type {len(text)} characters{ending}" + if kind == "press": + return f"press {native.get('key')}" + if kind == "hotkey": + return "press " + "+".join( + [*native.get("modifiers", []), str(native.get("key"))] + ) + return str(kind) diff --git a/packages/flow/openadapt_flow/compile_agent/harness.py b/packages/flow/openadapt_flow/compile_agent/harness.py index 924ee6d62..7723f74bc 100644 --- a/packages/flow/openadapt_flow/compile_agent/harness.py +++ b/packages/flow/openadapt_flow/compile_agent/harness.py @@ -198,15 +198,26 @@ def native_for( control = self._control(str(ref)) return {"kind": "click", "x": control.x, "y": control.y}, control.ref - def commit_target(self, native: Mapping[str, Any]) -> Optional[str]: - """What may commit a write in ``native``, or None. Nothing is performed.""" + def commit_target( + self, native: Mapping[str, Any], ref: Optional[str] = None + ) -> Optional[str]: + """What may commit a write in ``native``, or None. Nothing is performed. + ``ref`` is the text field a ``type`` will be typed into (it is clicked + first), so the check looks at that field rather than today's focus. + """ + + if ref is not None and native.get("kind") == "type": + control = (self._last.controls if self._last else {}).get(ref) + text = str(native.get("text") or "") + if control is not None and _editable(control): + return "a typed line break" if any(c in text for c in "\r\n") else None hit = self._commit_target(native) return hit[0] if hit is not None else None # -- every action ------------------------------------------------------------- - def act(self, native: Mapping[str, Any]) -> Observation: + def act(self, native: Mapping[str, Any], ref: Optional[str] = None) -> Observation: """Perform one native action in the computer's dialect, behind the gate. Raises: @@ -269,6 +280,12 @@ def _commit_target( elif op.kind == "type" and any(ch in (op.text or "") for ch in "\r\n"): focused = _observe.focused_element(self.page) or {} return ("a typed line break", focused.get("scope")) + elif op.kind == "type" and " " in (op.text or ""): + # A typed space on a focused button presses it. + focused = _observe.focused_element(self.page) or {} + if not focused.get("editable"): + label = str(focused.get("name") or "the focused element") + return (f"typing a space on {label}", focused.get("scope")) elif op.kind == "hotkey": focused = _observe.focused_element(self.page) or {} shortcut = "+".join([*op.modifiers, op.key or ""]) @@ -383,8 +400,59 @@ def find( return hits[0] if len(hits) == 1 else None +def _editable(control: Any) -> bool: + """A web control that takes typed text.""" + + return control.tag in ("input", "textarea") or control.role in ( + "textbox", + "combobox", + ) + + #: Roles a click only focuses. Typing into them commits nothing. _EDITABLE_ROLES = frozenset({"text_input", "combobox"}) +#: Roles whose click only changes what the form shows or holds. +_SAFE_CLICK_ROLES = _EDITABLE_ROLES | frozenset({"checkbox", "radio", "tab"}) +#: First words of labels that navigate or open something, and never write. +_NAVIGATION_WORDS = frozenset( + { + "open", + "new", + "next", + "back", + "previous", + "cancel", + "close", + "search", + "find", + "view", + "show", + "edit", + "select", + "start", + "go", + "home", + "menu", + "more", + "expand", + "collapse", + } +) +#: Keys that move focus or dismiss, which never write by themselves. +_FOCUS_KEYS = frozenset( + { + "Tab", + "Escape", + "ArrowUp", + "ArrowDown", + "ArrowLeft", + "ArrowRight", + "Home", + "End", + "PageUp", + "PageDown", + } +) @dataclass @@ -397,12 +465,15 @@ class SurfaceHarness: :class:`CaptureComputer`, so the recording is the one any other capture makes. - The save gate gates the same actions as on the web: a click on a control - whose label names a write, a click on a point no control covers, Enter, - Space, a typed line break, and any shortcut. On these surfaces the harness - can't read which form or row a control sits in, so it can't do the - right-record check itself. A person approves every gated action instead - (the hold). Without a hold, a gated action is refused. + The save gate is stricter than on the web, because the harness can't read + which form or row a control sits in and so can't do the right-record check + itself. A click is let through only on a control named by its ref whose + role only edits the form (a text field, a check box, a tab) or whose label + plainly navigates (Open, New, Cancel); every other click, including any + click at x,y, is gated. So are Enter, Space and any typing outside a text + field, a typed line break, a single-letter key (a dialog's accelerator), + and every shortcut. A person approves every gated action (the hold). + Without a hold, a gated action is refused. """ computer: CaptureComputer @@ -498,65 +569,83 @@ def native_for( def click(self, ref: str) -> Observation: control = self._control(ref) - return self.act({"kind": "click", "x": control.x, "y": control.y}) + return self.act({"kind": "click", "x": control.x, "y": control.y}, ref=ref) def type_text(self, text: str, ref: Optional[str] = None) -> Observation: if ref is not None: self.click(ref) - return self.act({"kind": "type", "text": text}) + return self.act({"kind": "type", "text": text}, ref=ref) def press(self, key: str) -> Observation: return self.act({"kind": "press", "key": key}) - def act(self, native: Mapping[str, Any]) -> Observation: + def act(self, native: Mapping[str, Any], ref: Optional[str] = None) -> Observation: """Perform one generic action, behind the gate (see the class doc). + ``ref`` names the control a click is on, or the text field a ``type`` + goes into. A click with no ref is a click at x,y. + Raises: GateRefused: the action may commit a write and no person approved it. UnsupportedAction: the recording can't hold the action. """ - commit = self.commit_target(native) + commit = self.commit_target(native, ref) if commit is not None: self._check_gate(commit) obs = self.computer.execute(native) self.stats.actions += 1 - clicked = self._control_at(native) - if clicked is not None or native.get("kind") in ("click", "double_click"): - self._focused_role = clicked.role if clicked is not None else None + kind = native.get("kind") + if kind in ("click", "double_click", "right_click"): + control = self._ref_control(ref) + self._focused_role = control.role if control is not None else None + elif kind == "hotkey" or ( + kind == "press" and str(native.get("key")) in _FOCUS_KEYS | {"Enter"} + ): + self._focused_role = None return obs - def _control_at(self, native: Mapping[str, Any]) -> Optional[SurfaceControl]: - if native.get("kind") not in ("click", "double_click", "right_click"): - return None - x, y = native.get("x"), native.get("y") - if x is None or y is None or self._last is None: + def _ref_control(self, ref: Optional[str]) -> Optional[SurfaceControl]: + if ref is None or self._last is None: return None - for control in self._last.controls.values(): - if control.x == x and control.y == y: - return control - return None + return self._last.controls.get(ref) - def commit_target(self, native: Mapping[str, Any]) -> Optional[str]: + def commit_target( + self, native: Mapping[str, Any], ref: Optional[str] = None + ) -> Optional[str]: """What may commit a write in ``native``, or None (see the class doc).""" + control = self._ref_control(ref) + focus = control.role if control is not None else self._focused_role + editable = focus in _EDITABLE_ROLES for op in translate(self.dialect, native): if op.kind in ("click", "double_click", "right_click"): - control = self._control_at(native) if control is None: - return "a click on a point the harness can't name" - if control.role in _EDITABLE_ROLES: - continue + return "a click at a point, not on a named control" if _observe.commit_label(control.name): return control.name - elif op.kind == "type" and any(ch in (op.text or "") for ch in "\r\n"): - return "a typed line break" + words = control.name.lower().split() + if control.role in _SAFE_CLICK_ROLES or ( + words and words[0] in _NAVIGATION_WORDS + ): + continue + return control.name or f"a {control.role} with no name" + elif op.kind == "type": + text = op.text or "" + if any(ch in text for ch in "\r\n"): + return "a typed line break" + if text and not editable: + return "typing outside a text field" elif op.kind == "hotkey": return "the shortcut " + "+".join([*op.modifiers, op.key or ""]) - elif op.kind == "press" and op.key in ("Enter", "Space"): - if op.key == "Space" and self._focused_role in _EDITABLE_ROLES: - continue - return f"{op.key} on the focused control" + elif op.kind == "press": + key = op.key or "" + if key in ("Enter", "Return"): + return "Enter on the focused control" + if key == "Space" and not editable: + return "Space on the focused control" + if len(key) == 1 and not editable: + return f"the key {key!r} outside a text field" return None def _check_gate(self, control: str) -> None: diff --git a/packages/flow/tests/test_compile_agent_surface_harness.py b/packages/flow/tests/test_compile_agent_surface_harness.py index 58dd6cf1e..3307b1f19 100644 --- a/packages/flow/tests/test_compile_agent_surface_harness.py +++ b/packages/flow/tests/test_compile_agent_surface_harness.py @@ -55,6 +55,7 @@ def __init__(self) -> None: "name": "Open", "c": (100, 120), }, + {"node_id": "n_00000004", "role": "button", "name": "Yes", "c": (100, 160)}, ] self.performed: list[tuple[Any, ...]] = [] @@ -104,6 +105,7 @@ def test_the_observation_lists_controls_by_ref_without_values() -> None: '[n_00000001] text_input "Note"', '[n_00000002] button "Save"', '[n_00000003] button "Open"', + '[n_00000004] button "Yes"', ] assert obs.find("save").ref == "n_00000002" assert not obs.screen_only @@ -132,40 +134,115 @@ def test_a_person_approves_or_refuses_each_save() -> None: assert session.performed == [] +NOTE, SAVE, OPEN, YES = "n_00000001", "n_00000002", "n_00000003", "n_00000004" + + @pytest.mark.parametrize( - ("steps", "gated"), + ("setup", "last", "gated"), [ - ([("click", "n_00000003")], None), - ([("click", "n_00000001"), ("type", "a note")], None), - ([("click", "n_00000001"), ("press", "Space")], None), - ([("click", "n_00000001"), ("type", "a note\n")], "a typed line break"), - ([("press", "Enter")], "Enter on the focused control"), - ([("hotkey", "s")], "the shortcut Control+s"), - ([("at", (5, 5))], "a click on a point the harness can't name"), + ([], ("click", OPEN), None), + ([], ("click", YES), "Yes"), + ([], ("click", SAVE), "Save"), + ([("click", NOTE)], ("type", "a note"), None), + ([], ("type_into", "a note"), None), + ([("click", NOTE)], ("press", "Space"), None), + ( + [("click", NOTE), ("press", "Tab")], + ("press", "Space"), + "Space on the focused control", + ), + ([("click", OPEN)], ("type", "y"), "typing outside a text field"), + ([], ("press", "y"), "the key 'y' outside a text field"), + ([("click", NOTE)], ("type", "a note\n"), "a typed line break"), + ([("click", NOTE)], ("press", "Enter"), "Enter on the focused control"), + ([], ("hotkey", "s"), "the shortcut Control+s"), + # Save's own center, but by x,y: nothing names the control. + ([], ("at", (100, 80)), "a click at a point, not on a named control"), ], ) -def test_what_is_gated(steps: list[tuple[str, Any]], gated: Optional[str]) -> None: +def test_what_is_gated(setup: list, last: tuple, gated: Optional[str]) -> None: harness, session = _harness() - last = steps[-1] - for kind, arg in steps[:-1]: - getattr(harness, {"click": "click", "type": "type_text"}[kind])(arg) - native = { - "click": lambda a: harness.native_for({"ref": a})[0], - "type": lambda a: {"kind": "type", "text": a}, - "press": lambda a: {"kind": "press", "key": a}, - "hotkey": lambda a: {"kind": "hotkey", "key": a, "modifiers": ["ctrl"]}, - "at": lambda a: {"kind": "click", "x": a[0], "y": a[1]}, - }[last[0]](last[1]) - assert harness.commit_target(native) == gated + for kind, arg in setup: + if kind == "click": + harness.click(arg) + else: + harness.press(arg) + kind, arg = last + ref = None + if kind == "click": + native, ref = harness.native_for({"ref": arg}) + elif kind == "type": + native = {"kind": "type", "text": arg} + elif kind == "type_into": + native, ref = {"kind": "type", "text": arg}, NOTE + elif kind == "press": + native = {"kind": "press", "key": arg} + elif kind == "hotkey": + native = {"kind": "hotkey", "key": arg, "modifiers": ["ctrl"]} + else: + native = {"kind": "click", "x": arg[0], "y": arg[1]} + assert harness.commit_target(native, ref) == gated + before = list(session.performed) if gated is None: - harness.act(native) + harness.act(native, ref) + assert len(session.performed) == len(before) + 1 else: - before = list(session.performed) with pytest.raises(GateRefused): - harness.act(native) + harness.act(native, ref) assert session.performed == before +def test_an_approval_covers_one_action_on_one_run_as_the_app_was() -> None: + harness, session = _harness(hold=None) + agent = DrivenAgent() + harness.gate.hold = agent.hold + agent.harness = harness + agent.request = SimpleNamespace(inputs={}) + + def call(kind: str, payload: dict) -> dict: + from concurrent.futures import Future + + from openadapt_flow.compile_agent.driven import _Command + + return agent._handle(_Command(kind, payload, Future())) + + save = {"action": "click", "ref": SAVE} + preview = call("preview", save) + assert preview["commit"] == "Save" and preview["action"] == "click at x=100, y=80" + # Another action happens while the person reads the form: the approval + # no longer covers what is on screen. + call("act", {"action": "click", "ref": OPEN}) + with pytest.raises(ValueError, match="changed after the person approved"): + call("act", {**save, "_held_by_person": preview["binding"]}) + assert ("click", 100, 80) not in session.performed + # Another run never matches. + other = DrivenAgent() + assert ( + other.binding({"kind": "click", "x": 100, "y": 80}, SAVE) + != call("preview", save)["binding"] + ) + # The fresh approval goes through, once. + fresh = call("preview", save) + call("act", {**save, "_held_by_person": fresh["binding"]}) + assert session.performed[-1] == ("click", 100, 80) + assert harness.gate_log[-1]["decision"] == "approved_by_hold" + + +def test_type_into_a_control_that_is_not_a_field_is_refused() -> None: + harness, session = _harness() + agent = DrivenAgent() + agent.harness, agent.request = harness, SimpleNamespace(inputs={}) + from concurrent.futures import Future + + from openadapt_flow.compile_agent.driven import _Command + + with pytest.raises(ValueError, match="not a text field"): + agent._handle( + _Command("preview", {"action": "type", "text": "x", "ref": SAVE}, Future()) + ) + assert session.performed == [] + + def test_a_control_that_changed_is_a_stale_ref() -> None: harness, session = _harness() session.nodes[1]["name"] = "Delete" From 41dacc28cdd99bec6a161eb4463c6e3290adf5a5 Mon Sep 17 00:00:00 2001 From: abrichr Date: Mon, 19 Oct 2026 10:34:05 -0400 Subject: [PATCH 4/4] ci: pin two reviewed gitleaks false positives (key_dir in MCP teach tests) Co-Authored-By: Claude Opus 5.5 --- .gitleaksignore | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.gitleaksignore b/.gitleaksignore index bbaa1a008..b8c11be39 100644 --- a/.gitleaksignore +++ b/.gitleaksignore @@ -48,3 +48,7 @@ eb78575c688f06d7edc04afc69389aad10e1f5fa:packages/flow/tests/fixtures/terminal_v # branch commit that added them. The lines carry gitleaks:allow from the next commit on. 3f0df2191a649540e79a621ac5406ebe2ff228e2:packages/flow/openadapt_flow/jit/authority.py:generic-api-key:101 3f0df2191a649540e79a621ac5406ebe2ff228e2:packages/flow/openadapt_flow/jit/authority.py:generic-api-key:390 +# MCP teach (PR #1335): the generic-api-key rule matched the variable name `key_dir` +# in a principal_from_key(...) call in two tests. No credential. +e8ec96725c4451aa64cb97230c18d04b55d98503:packages/flow/tests/test_mcp_teach_surfaces.py:generic-api-key:147 +e8ec96725c4451aa64cb97230c18d04b55d98503:packages/flow/tests/e2e/test_mcp_teach_e2e.py:generic-api-key:122