From 225f2bc7bb1e112891570be4735882612a0ca6c5 Mon Sep 17 00:00:00 2001 From: abrichr Date: Fri, 4 Sep 2026 12:38:31 -0400 Subject: [PATCH] feat: one-step human admit on authoring compile compile still returns needs_human_admit and never VERIFIED. admit accepts a missing, empty, ok, yes, y, enter, or true confirm and calls session.admit. Missing Flow admit fails closed. The human does not fill schema, authority, effect, environment, or digest. --- README.md | 4 +- docs/DESIGN.md | 8 ++- src/openadapt_agent/authoring.py | 89 ++++++++++++++++++++++++++++++++ src/openadapt_agent/mcp.py | 7 +-- tests/test_authoring.py | 77 +++++++++++++++++++++++++++ tests/test_cli.py | 1 + 6 files changed, 179 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index afb7013..dbe2f0e 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ openadapt-agent serve --allow-run Local unsigned replay is free. If the tool returns unsigned success, treat it as failure. Production success without a Seal is failure. HALTED does not mint a Seal. -A first demo over stdio uses a second MCP server. `--authoring` does not enable run tools. +A first demo over stdio uses a second MCP server. `--authoring` does not enable run tools. First-demo compile needs a human ok before the tool is callable, and that ok is the whole approval. ```bash claude mcp add openadapt-authoring -- \ @@ -124,7 +124,7 @@ The client gets `list_workflows`, `get_workflow`, `get_run_report`, `list_needs_ | `run_local_quickstart` | `--allow-run` with no `--bundles` | | `reject_attention`, `teach_attention`, `escalate_attention` | `--allow-attended-actions` | | `continue_attention`, `skip_attention` | `--allow-attended-actions` plus a qualified deployment `--config` | -| `observe`, `start_record`, `click`, `halt` | `--authoring` | +| `observe`, `start_record`, `click`, `halt`, `compile`, `admit` | `--authoring` | `--headed` keeps the attended web session visible. The MCP server is stdio. The target app still has a window. diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 805425b..0a44abf 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -148,7 +148,10 @@ typing through Flow's Recorder. Hosted MCP remains pause-only. Human type during `pause_for_input` is persisted with `Recorder.record_observed` on the pause-target node, never `type_text`. `compile` wraps Flow `compile_recording` and returns `needs_human_admit`; an agent click never -paints `VERIFIED`. +paints `VERIFIED`. `admit` is the one-token human ok of the pre-filled +draft; the human does not fill schema, authority, effect, environment, or +digest. If the Flow session has no `admit`, the tool fails closed and does +not mint a Seal or write an unsigned ledger row. `--authoring` does not imply `--allow-run`. `--bundles` is optional iff `--authoring` (or the existing `--tutorial` / implied-tutorial path). The @@ -379,7 +382,8 @@ Tests cover: and local `type`; observe projection drops values/titles/screenshots and extra keys, caps the wire at 32 KiB, and uses `n_` + 8 hex node ids; pause Continue uses `record_observed` rather than `type_text`; - compile returns `needs_human_admit`; `--authoring` does not enable + compile returns `needs_human_admit`; `admit` is the one-token human + ok; `--authoring` does not enable run tools; `server.json` stays stdio with `--bundles` required; `authoring connect` parses `openadapt://runner` / pack URLs, claims `oab_`, polls `wait_seconds: 0`, prompts Allow-per-`sub`, and Continue diff --git a/src/openadapt_agent/authoring.py b/src/openadapt_agent/authoring.py index ad80415..be47521 100644 --- a/src/openadapt_agent/authoring.py +++ b/src/openadapt_agent/authoring.py @@ -60,6 +60,7 @@ "continue_input", "stop_record", "compile", + "admit", "get_command_result", "set_coach", "get_coach", @@ -191,6 +192,8 @@ _PAUSE_FIELDS = frozenset({"node_id", "param", "secret"}) _RESULT_FIELDS = frozenset({"command_id"}) _COACH_FIELDS = frozenset({"hint"}) +_ADMIT_FIELDS = frozenset({"confirm"}) +_ADMIT_ACCEPT = frozenset({"", "ok", "yes", "y", "enter"}) _DESKTOP_IPC_RELATIVE = Path(".openadapt") / "desktop_ipc.json" @@ -685,6 +688,14 @@ def _require_object(arguments: Optional[dict[str, Any]], allowed: set[str]) -> d return payload +def _confirm_is_acceptance(value: Any) -> bool: + if value is None or value is True: + return True + if isinstance(value, str) and value.strip().lower() in _ADMIT_ACCEPT: + return True + return False + + def _invoke(session: object, method: str, **kwargs: Any) -> Any: func = getattr(session, method, None) aliases = { @@ -864,6 +875,7 @@ def list_tool_specs(self) -> list[ToolSpec]: name="compile", description=( "Wrap Flow compile_recording and return needs_human_admit. " + "A named human then calls admit with a one-token ok. " "An agent click never paints VERIFIED. Refuses a session " "that had a secret pause and no TYPE/param event." ), @@ -875,6 +887,37 @@ def list_tool_specs(self) -> list[ToolSpec]: "openWorldHint": False, }, ), + ToolSpec( + name="admit", + description=( + "One-step human admit of the pre-filled draft (schema, " + "authority, effect contract, environment, digest). The " + "human does not fill those fields. Missing confirm, empty, " + "ok, yes, y, enter, or true accepts. Any other confirm " + "refuses. Does not mint a Seal or write an unsigned ledger " + "row when the Flow session has no admit." + ), + input_schema={ + "type": "object", + "properties": { + "confirm": { + "type": ["string", "boolean"], + "description": ( + "Optional one-token ok. Omit, empty, ok, yes, " + "y, enter, or true accepts. Anything else " + "refuses." + ), + } + }, + "additionalProperties": False, + }, + annotations={ + "readOnlyHint": False, + "destructiveHint": False, + "idempotentHint": True, + "openWorldHint": False, + }, + ), ToolSpec( name="get_command_result", description=( @@ -948,6 +991,7 @@ def dispatch(self, name: str, arguments: Optional[dict[str, Any]] = None) -> dic "continue_input": self._continue_input, "stop_record": self._stop_record, "compile": self._compile, + "admit": self._admit, "set_coach": self._set_coach, "get_coach": self._get_coach, "bind_status": self._bind_status, @@ -1244,6 +1288,51 @@ def _compile(self, arguments: Optional[dict[str, Any]]) -> dict[str, Any]: "status": "needs_human_admit", "recording_retained": True, } + if isinstance(workflow_id, str) and workflow_id: + public["workflow_id"] = workflow_id + summary = _safe_label(result.get("summary")) + if summary: + public["summary"] = summary + return public + + def _admit(self, arguments: Optional[dict[str, Any]]) -> dict[str, Any]: + payload = _require_object(arguments, _ADMIT_FIELDS) + confirm = payload.get("confirm") if "confirm" in payload else None + if not _confirm_is_acceptance(confirm): + raise AuthoringError( + "admit refused: confirm is not a one-token ok", + code="admit_refused", + ) + admit_fn = getattr(self.session, "admit", None) + if not callable(admit_fn): + raise AuthoringError( + "authoring session does not implement admit; " + "refusing rather than minting a Seal, writing an unsigned " + "ledger row, or claiming Production", + code="admit_unavailable", + ) + try: + try: + raw = admit_fn(confirm=confirm) + except TypeError: + try: + raw = admit_fn(confirm) + except TypeError: + raw = admit_fn() + except Exception as exc: + mapped = self._map_session_error(exc) + if mapped.get("error"): + return mapped + raise + if isinstance(raw, Mapping) and (raw.get("status") == "error" or raw.get("error")): + error = raw.get("error") + return { + "status": "error", + "error": error if isinstance(error, str) and error else "admit_refused", + } + result = _public_result(raw) + public = {"status": "admitted"} + workflow_id = result.get("workflow_id") if isinstance(workflow_id, str) and workflow_id: public["workflow_id"] = workflow_id return public diff --git a/src/openadapt_agent/mcp.py b/src/openadapt_agent/mcp.py index 56ac5e9..1f6c31e 100644 --- a/src/openadapt_agent/mcp.py +++ b/src/openadapt_agent/mcp.py @@ -121,9 +121,10 @@ def _server_instructions(authoring: AuthoringBridge | None) -> str: "and halt over this same local stdio process. Local stdio may also " "type through the recorder; hosted MCP has no type tool. Human type " "during pause_for_input is record_observed, never type_text. " - "compile returns needs_human_admit; an agent click never paints " - "VERIFIED. --authoring does not enable run tools. This process " - "must not be port-forwarded or served over HTTP." + "compile returns needs_human_admit; admit is the one-token human " + "ok. An agent click never paints VERIFIED. --authoring does not " + "enable run tools. This process must not be port-forwarded or " + "served over HTTP." ) return text diff --git a/tests/test_authoring.py b/tests/test_authoring.py index a05dc57..4949d3a 100644 --- a/tests/test_authoring.py +++ b/tests/test_authoring.py @@ -156,13 +156,25 @@ def compile(self): "success": True, } + def admit(self, confirm=None): + self.calls.append(("admit", confirm)) + return { + "status": "admitted", + "workflow_id": "wf_demo0001", + "execution_outcome": "VERIFIED", + "success": True, + "title": "Chart — Jane Roe", + } + def test_probe_tool_names_match_hosted_surface(): names = [spec.name for spec in AuthoringBridge(FakeAuthoringSession()).list_tool_specs()] for probe in AUTHORING_PROBE_TOOLS: assert probe in names assert "type" in names + assert "admit" in names assert names.index("observe") < names.index("type") + assert names.index("compile") < names.index("admit") def test_observe_drops_values_titles_screenshots_and_unsafe_names(): @@ -265,6 +277,70 @@ def test_compile_returns_needs_human_admit_never_verified(): assert "success" not in result +def test_admit_empty_confirm_and_ok_work_without_type_text(): + session = FakeAuthoringSession() + bridge = AuthoringBridge(session) + empty = bridge.dispatch("admit", {}) + assert empty == {"status": "admitted", "workflow_id": "wf_demo0001"} + assert ("admit", None) in session.calls + ok = bridge.dispatch("admit", {"confirm": "ok"}) + assert ok["status"] == "admitted" + assert ("admit", "ok") in session.calls + blank = bridge.dispatch("admit", {"confirm": ""}) + assert blank["status"] == "admitted" + assert ("admit", "") in session.calls + true_ok = bridge.dispatch("admit", {"confirm": True}) + assert true_ok["status"] == "admitted" + assert ("admit", True) in session.calls + for token in ("yes", "y", "enter", "OK"): + accepted = bridge.dispatch("admit", {"confirm": token}) + assert accepted["status"] == "admitted" + assert ("admit", token) in session.calls + assert session.typed_via_backend == [] + assert all( + call[0] != "type_text" if isinstance(call, tuple) else call != "type_text" + for call in session.calls + ) + compiled = bridge.dispatch("compile", {}) + assert compiled["status"] == "needs_human_admit" + assert "VERIFIED" not in json.dumps(compiled) + assert "VERIFIED" not in json.dumps(empty) + assert "title" not in json.dumps(empty) + + +def test_admit_garbage_confirm_refuses(): + session = FakeAuthoringSession() + bridge = AuthoringBridge(session) + with pytest.raises(AuthoringError, match="one-token ok"): + bridge.dispatch("admit", {"confirm": "ship-it"}) + with pytest.raises(AuthoringError, match="one-token ok"): + bridge.dispatch("admit", {"confirm": False}) + assert all(call != ("admit", "ship-it") for call in session.calls) + assert all(call != ("admit", False) for call in session.calls) + assert session.typed_via_backend == [] + + +def test_admit_fails_closed_without_session_admit(): + class NoAdmitSession: + def compile(self): + return { + "status": "needs_human_admit", + "workflow_id": "wf_demo0001", + "execution_outcome": "VERIFIED", + } + + bridge = AuthoringBridge(NoAdmitSession()) + compiled = bridge.dispatch("compile", {}) + assert compiled["status"] == "needs_human_admit" + assert "VERIFIED" not in json.dumps(compiled) + with pytest.raises(AuthoringError, match="does not implement admit") as exc_info: + bridge.dispatch("admit", {"confirm": "ok"}) + message = str(exc_info.value) + assert "Seal" in message + assert "unsigned" in message + assert "Production" in message + + def test_windows_native_is_coach_only(): session = FakeAuthoringSession(backend="windows") bridge = AuthoringBridge(session) @@ -312,6 +388,7 @@ async def list_names(): names = anyio.run(list_names) assert names[:4] == list(AUTHORING_PROBE_TOOLS) assert "type" in names + assert "admit" in names assert not any(name.startswith("run_") for name in names) combined = build_server( diff --git a/tests/test_cli.py b/tests/test_cli.py index 4dfac7b..c5593da 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -206,6 +206,7 @@ def fake_serve(bridge, authoring=None): names = [spec.name for spec in authoring.list_tool_specs()] assert names[:4] == ["observe", "start_record", "click", "halt"] assert "type" in names + assert "admit" in names assert captured["session_kwargs"]["out_dir"].name == "authoring" err = capsys.readouterr().err assert "authoring tools enabled" in err