Repository navigation
fix: confirm admit with a person and report approval waits - #64
Conversation
admit accepted an empty or omitted confirm, so an agent could call it with no arguments and Flow wrote admit.json naming the OS user as the operator who accepted the draft. No person was asked. The tool description even told the model which tokens accept. admit now takes no arguments. The MCP layer asks the person through form elicitation, with the draft's workflow id in the prompt, before it calls the bridge, and only then marks the call as confirmed. A client without form elicitation, a decline, or a cancel refuses and nothing is accepted. AuthoringBridge.dispatch refuses admit with admit_requires_host_confirmation unless the transport set human_confirmed, so non-MCP callers can't accept a draft either. When Flow's admit takes a confirmation_channel, the bridge passes mcp_form_elicitation so admit.json can record how the acceptance was obtained (the Flow side of this is separate work). Ledger: B074 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The README and the agents pages showed `serve --allow-run`, `serve --authoring` and the `[tutorial]` extra while PyPI shipped 2.0.1, whose `serve` still required --bundles and had no tutorial extra, so a partner's first command exited with code 2. Main's README now installs from GitHub and says that 2.0.1 predates the sandbox, so the docs here match the code again. This adds a test that pulls every `openadapt-agent serve|authoring| emit-skill` command from README.md, llms.txt and the shipped skill, and checks that this code's parser accepts it and that every `openadapt-agent[extra]` they name is declared in pyproject. The test fails against the v2.0.1 parser and passes on main. Publishing a release from main is still needed before the docs on PyPI-facing pages (openadapt-ops get-started, openadapt-web agents.txt) are true. Ledger: B072 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The attended-action confirmation refused any client whose elicitation capability had no form key. MCP 2025-11-25 says an empty elicitation object means form mode, and clients written for 2025-06-18, which had no form or url keys, always send it that way. Those clients could show the form but every continue, skip, teach, escalate and reject was refused. The check now lives in client_supports_form_elicitation: a client can show a form when it declares form, or declares elicitation with neither form nor url. A client that declares only url, or no elicitation, is still refused before anything is submitted. admit uses the same check. Ledger: B083 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`serve --authoring` without --url exited on every Mac with a generic
"needs a locally pinned window" message. Two things were wrong. The
tutorial extra pulled Flow's browser extra only, so pyobjc (Quartz) was
missing and Flow's frontmost-window lookup quietly returned nothing. And
with pyobjc installed, the pin still failed: it looked the frontmost
window up with find_windows("", None), but Flow matches an exact owner,
so an empty owner never matches any window.
The tutorial extra now includes Flow's macos extra, whose pyobjc
requirements carry darwin-only markers. The pin reads the frontmost
window's owner and title from Quartz by window id, then asks Flow's
find_windows for that exact owner and title so the uniqueness check
still applies. Each failure now says what to do: pyobjc missing (names
openadapt-flow[macos]), no window in front, the owner unknown, titles
hidden until Screen Recording is allowed, or several windows with one
title. Errors never include a window title. uv.lock is updated for the
new extra.
Ledger: B077
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Outside the sandbox, `--runs-dir` defaulted to ./runs, relative to
wherever the MCP client started the server, which is usually the user's
project. Run reports, frames and entered values could land in a git
repository where `git add .` commits them, and a server started from
another folder saw a different Needs Attention queue and request-id
ledger, so earlier pauses were orphaned. RunnerConfig, the authoring
recording folder and the MCPB entry point (which fell back to "runs")
had the same relative default, while the MCPB manifest already said
${HOME}/.openadapt/runs.
There is now one default, default_runs_dir(): OPENADAPT_AGENT_RUNS_DIR
when set, else ~/.openadapt/runs. The CLI resolves the folder once to an
absolute path and passes it to runs, pauses and the ledger; authoring
and the MCPB entry point use the same default; FlowRunner creates the
folder owner-only. The startup line names the folder so an operator can
find their pauses. The sandbox keeps ~/.openadapt/agent-sandbox.
Ledger: B082
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The deprecated run_<id> tools popped "url" from the arguments as the target-app override before checking them against the workflow's parameters. A workflow with its own "url" parameter therefore always looked like it was missing that input and every call was refused. With --allow-synthetic-recorded-defaults and --allow-url-override, the run reported success after typing the recorded URL into the workflow and sending the caller's value to the app address instead. The schema also wrote the override over the declared parameter's description. bundles.url_override_key picks the override's argument name: "url" when the workflow has no url parameter (unchanged for existing callers), otherwise "target_url_override" (with a suffix if that is taken too). The schema and the bridge both use it, so a declared url always reaches the workflow's inputs and the override reaches --url only under its own name and only with --allow-url-override. Ledger: B084 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The MCPB had no way to set a certifying policy: manifest.json had no policy field and build_argv never passed --policy, so get_workflow answered "Certification was not evaluated" unless the user happened to pick a deployment YAML that carried one. And "Allow Needs Attention decisions" listed continue, skip, teach, escalate and reject for every client, although a client without form elicitation gets a refusal from each of them. The manifest now has an optional "Certifying policy" setting wired to OPENADAPT_AGENT_POLICY, and build_argv passes it as --policy. The deployment config and attended-decision descriptions say what each one is for and that decisions need a client with form elicitation. The tool list leaves the attended decision tools out for a client that can't show a confirmation form; calling one anyway is still refused before anything is submitted. get_workflow now says how to set a policy when none is configured. A test ties every OPENADAPT_AGENT_* variable the entry point reads to an mcp_config.env entry and a user_config key. Ledger: B086 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
openadapt-flow now stops before each save under the Standard and Regulated profiles until a person approves it (each_item), and waits up to 15 minutes by default. This server didn't know about that: - run_workflow and get_run said "running" for the whole wait, with nothing telling the caller that a person had to act. - The server killed Flow after --timeout (600 s by default) even while Flow was waiting for the person, so the person's time was cut short, the pause they were answering was lost, and the result read "The run didn't finish in time", a timeout dressed as a failure. - When Flow itself gave up waiting, the result said "Stopped before saving because something didn't match", which is not what happened. Now: - While a save waits for an answer (an unanswered, unexpired item-approval request in the run folder), run_workflow and get_run return needs_review with reason waiting_for_approval. next_action says a person at that computer approves or declines the save and names `openadapt-flow item-approval show`. record_changed is "unknown", because an earlier save in the same run may have gone through, and the copy says only that this save hasn't been made. This live result is never stored. run_workflow returns as soon as the wait begins; a get_run that finds the run already waiting keeps waiting for the answer instead of returning at once. - Time Flow spends waiting for a person no longer counts toward --timeout (Flow bounds the wait; the server allows at most 4 hours of person time in all). Flow runs go through _run_flow_process, a subprocess.run stand-in that does this; tests stub that function now. - A report whose approval block says approval_required, with a transaction outcome that proves no effect, reads needs_review with reason approval_needed. An uncertain save stays not_sure_if_saved and a declined save stays stopped_by_person. - The server never passes --approval-mode unless the operator starts it with `serve --approval-mode`, so not_gated is never chosen for them and the calling agent can't choose it. The mode is recorded in every result under technical.approval_mode (from Flow's report, else the operator's flag) and in the startup line. The flag is refused when this interpreter's Flow has no approval modes. - The needs_review rows in the README, llms.txt and the skills no longer say "Nothing was written" unconditionally: they point at record_changed, which is "unknown" only while a run waits for an approval. tests/test_approval_flow.py drives this end to end with real openadapt-flow main, a real browser and MockMed (opt in with OPENADAPT_AGENT_E2E=1; it skips until the installed Flow has approval modes). Run locally against Flow main: with a 20 s server timeout and a 25 s approval wait, the server didn't kill the run and Flow stopped by itself; an approval given through item-approval saved exactly once. Ledger: B272 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Both Server(...) calls in build_server left out version=, so the initialize response's serverInfo.version was the MCP SDK's own version under MCP 1 (for example "1.28.0") and an empty string under MCP 2. Clients, audit logs and support triage couldn't tell which openadapt-agent release served a session. build_server now passes version=openadapt_agent.__version__ on both SDK branches. A new test checks create_initialization_options() and the serverInfo a real client session receives, for the bridge and the authoring server; it fails on MCP 1.28 and 2.3 without the change. Ledger: B091 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
docs/MAILBOX_CLI.md offered `openadapt connect '<runner-link-or-pack-url>'` as a "meta-package alias" and told the job page to show `pip: openadapt connect '<runner-link>'`. The launcher's `openadapt connect` is Cloud pairing: it takes only --pairing or --uri, so a runner link exits 2 with "Got unexpected extra argument". A test also required the README to contain "openadapt connect". The doc now says there is no launcher alias, that `openadapt connect` is Cloud pairing and rejects runner links, and that the job page should offer `openadapt-agent authoring connect '<runner-link>'`. The README already pointed at the right command. The test that required "openadapt connect" is gone; a new test fails on any README, llms.txt, docs, skills or source line that passes an argument straight to `openadapt connect`. Ledger: B088 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`serve --authoring` registers 14 tools (AUTHORING_TOOLS), but no doc listed them all. The README named 7 with "such as", llms.txt named 4 "and more", and DESIGN.md named the 4 probe tools and `type`. A partner engineer had no way to learn about continue_input, get_command_result, set_coach, get_coach or bind_status short of reading the code. DESIGN.md now has a "Recording tools" section with a table of all 14 tools in AUTHORING_TOOLS order, each marked probe or local with one line on what it does, and the README links to it. A test requires the table to match AUTHORING_TOOLS exactly and the README to link it. The other two B087 claims are settled on main: pyproject, server.json and manifest.json carry no Beta label (a new test keeps it out), and a production-eligible VERIFIED report now reads as success, which docs/DISTRIBUTION.md already says. PyPI and the MCP registry keep showing 2.0.1 until the next release. Ledger: B087 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The new ~/.openadapt/runs default (B082) moved the request-id ledger and pauses away from ./runs. A server upgraded in place, with no --runs-dir, would start with an empty ledger next to the old one: it wouldn't know the request_ids sent before and could enter one of them again. When no folder is named (no --runs-dir, no OPENADAPT_AGENT_RUNS_DIR) and ./runs/openadapt-agent exists, production and attended servers now exit 2 and name both folders, so the operator picks one with --runs-dir. Ledger: B082 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When the frontmost window was missing from its app's window list (Flow's find_windows skips windows with no size), the pin said more than one window had the same title. It now says macOS didn't list the frontmost window, and keeps the shared-title message for the case it describes. Ledger: B077 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The approval tests stubbed _run_flow_process, so nothing checked that the runner and the service find Flow's approval request in the run folder. Two tests now start a child process that writes a request the way Flow does: the runner doesn't kill it at --timeout while it waits, and run_workflow returns needs_review with reason waiting_for_approval, then done after the answer. Both fail on main. Ledger: B272 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The scan reads every branch, so the fixture on PR #64's branch fails gitleaks on every PR. The allowlist matches only that value in that file. Signed-off-by: Richard Abrich <richard.abrich@mldsai.com>
Keep the version-pinned Server and the _list_v1 wrapper from this branch. Signed-off-by: Richard Abrich <richard.abrich@mldsai.com>
|
Self-review. Not merging in this pass.
|
The admit fallback retried on TypeError, so a TypeError raised inside Flow's admit would run it again. The call shape now comes from the signature. Merging main left two test_initialize_reports_the_package_version definitions; the second silently replaced the first. Keep the parametrized one. Signed-off-by: Richard Abrich <richard.abrich@mldsai.com>
Signed-off-by: Richard Abrich <richard.abrich@mldsai.com>
Signed-off-by: Richard Abrich <richard.abrich@mldsai.com>
Signed-off-by: Richard Abrich <richard.abrich@mldsai.com>
|
Merge pass. Admit fails closed. Fixed in this pass:
Also reviewed: approval-wait handling (only an unanswered, unexpired request counts as waiting; the time a person spends answering is capped at 4 h), the |
This branch fixes ten problems in openadapt-agent and adds a guard test for one that main had already fixed. The two largest changes:
admitneeds a person's answer in the MCP client's confirmation form. Before, an agent could accept its own draft.needs_review. Before, it read as running, then hit the timeout or read as a failure.Fixes
admitwith no arguments. Flow then recorded the draft as accepted by the OS user, and nobody was asked.admittakes no arguments. The server asks the person through MCP form elicitation and refuses on decline, cancel, or a client that can't show forms. A direct call without that confirmation refuses withadmit_requires_host_confirmation.test_mcp_server.py::test_admit_requires_form_elicitation,test_authoring.py::test_admit_without_host_confirmation_refuseselicitation: {}(form mode under MCP 2025-11-25, and every 2025-06-18 client) had every attended decision refused.elicitationobject counts as form support. Clients that declare onlyurl, or no elicitation, are still refused.test_mcp_server.py::test_elicitation_capability_check_is_backward_compatibleserve --authoringwithout--urlfailed on every Mac. The tutorial extra had no pyobjc, and the pin searched for windows with an empty owner, which never matches.openadapt-flow[macos]. The pin reads the front window's owner and title from Quartz, then checks that window is unique. Each failure names its cause and never includes a window title.test_authoring.py::test_pin_native_darwin_*,test_tutorial_extra_includes_macos_deps./runsin the folder the client started the server in, often a git repository. A server started elsewhere saw a different request-id ledger and pause queue.~/.openadapt/runs(orOPENADAPT_AGENT_RUNS_DIR), resolved once and created owner-only, and the startup line names it. If./runsfrom an earlier version exists and no folder is named, the server refuses to start, so a resentrequest_idcan't run twice.test_cli.py::test_serve_default_runs_dir_is_not_cwd,test_cli.py::test_serve_refuses_to_skip_an_earlier_versions_runs_folder,test_mcpb_entry.py::test_mcpb_defaults_to_local_read_only_serverurlinput could never run through the deprecatedrun_<id>tools, and with recorded defaults the caller's value went to the app address.url, the override is namedtarget_url_override.test_bridge.py::test_declared_url_param_does_not_collide_with_overrideand two related tests--policy. Decision tools are hidden from clients without form elicitation. A call still refuses.test_mcpb_entry.py::test_mcpb_passes_the_certifying_policy,test_every_mcpb_variable_is_wired_to_user_config,test_mcp_server.py::test_attended_tools_are_listed_only_for_clients_that_can_confirmrunning. The server killed Flow at--timeoutduring the wait, and when Flow gave up waiting the result said something didn't match.run_workflowandget_runreturnneeds_review, reasonwaiting_for_approval,record_changed: "unknown". That live result is never stored. Time spent waiting for a person doesn't count toward--timeout(4 hours at most). After Flow gives up, the reason isapproval_needed, and only on a proven-no-effect outcome.--approval-modeis set only by the operator and is recorded in every result.tests/test_approval.py(12 tests, two with a real child process),tests/test_approval_flow.py(end to end, opt-in)serverInfo.versionwas the SDK's version under MCP 1 and empty under MCP 2.test_mcp_server.py::test_initialize_reports_the_package_versiondocs/MAILBOX_CLI.mdtold pip users to pass a runner link toopenadapt connect. That command is Cloud pairing and exits 2.openadapt-agent authoring connect.test_distribution.py::test_runner_link_docs_do_not_use_cloud_pairing_commanddocs/DESIGN.mdhas a table of all 14, linked from the README. A guard keeps the Beta label out of listings.test_distribution.py::test_docs_list_every_authoring_tool,test_listings_carry_no_beta_labelopenadapt-agent serve,authoringandemit-skillcommand in the README, llms.txt and the shipped skill must parse, and every named extra must exist. It fails against the v2.0.1 parser.tests/test_readme_commands.pyNot fixed here
tests/test_approval_flow.pyskips until they are.admitshould require and record how the acceptance was obtained, and stop naming the OS user as the operator. This server already passesconfirmation_channel="mcp_form_elicitation"once Flow'sadmitaccepts it.approval_neededtells the caller a person may "approve the save and resume the run, or end it". Flow's docs say a resumed run that opened its own browser (as--urlruns do) writes nothing. The copy fails safe, since nothing is retried, but whether to tell the caller to resend with a newrequest_idis a product decision.--url, the pin uses whatever window is in front when the server starts. That can be the MCP client or a terminal. This matches the documented behavior, and the pin doesn't refuse those windows.agents.txt) and Desktop's approval screens, which don't exist yet. Until thennext_actionnamesopenadapt-flow item-approval.How it was tested
origin/main(4b0b9bb) in a separate worktree that had only the test files copied in. Each one failed there because of the bug and passes on this branch. Some tests depend on helpers this branch adds (_run_flow_process,default_runs_dir,openadapt_agent.approval). For those, an equivalent test without the helper ran against main: B084 with the earlier subprocess stub, B082 by callingbuild_argv, and B272 with a child process that writes Flow's approval request. On main that child process was killed at the timeout and reporteddonewhile it waited, and Flow giving up read asunexpected_screen.OPENADAPT_AGENT_E2E=1against openadapt-flow main, real Chromium and MockMed:tests/test_approval_flow.py2 passed,tests/test_sandbox_flow.py5 passed.ruff check src tests scripts,uv lock --locked,python -m build,check_release_artifacts.py,check_dist.py,check_source_boundary.py --require-dist, andmcpb validate manifest.json(mcpb 2.1.2) all pass.Before merging
needs_reviewrows in the README, llms.txt and the skills no longer say "Nothing was written" for every case. They say it unlessrecord_changedisunknown, which happens only while a run waits for an approval. An earlier save in that run may have gone through../runsto~/.openadapt/runs. A production or attended server that finds an earlier version's./runsrefuses to start until it's given--runs-dir. The release notes should say so.admitnow refuses on clients that can't show a form.🤖 Generated with Claude Code