From d3c892db4be8ffe3a14c50d806594ab47ee0bd11 Mon Sep 17 00:00:00 2001 From: Richard Abrich Date: Sun, 19 Jul 2026 03:22:45 -0400 Subject: [PATCH] fix: forward all engine options through openadapt flow record The explicit click wrapper for `openadapt flow record` enumerated only the web-era options (--url/--out/--secret/--param/--headless), so `openadapt flow record --backend windows` failed with "No such option" even though the engine (openadapt-flow >=1.17) records web, windows, macos, linux, and rdp. It also forced --url, which the engine only requires for --backend web. Convert record to the same generic passthrough used by run/push/teach: every engine option (--backend, --agent-url, --macos-app, --macos-window-title, --linux-app, --linux-window-title, --linux-allow-physical-input, --rdp-host, --task, ...) now forwards verbatim, --help renders the engine's real help, and future engine options need no launcher release. Existing web invocations forward byte-for-byte unchanged. Adds regression tests for backend-option forwarding, verbatim web invocation, and engine-help rendering. Co-Authored-By: Claude Fable 5 --- openadapt/cli.py | 39 +++++----------- tests/test_cli_smoke.py | 99 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 108 insertions(+), 30 deletions(-) diff --git a/openadapt/cli.py b/openadapt/cli.py index a18db2cec..d8bb084d8 100644 --- a/openadapt/cli.py +++ b/openadapt/cli.py @@ -47,6 +47,10 @@ def list_commands(self, ctx): _FLOW_PASSTHROUGH_COMMANDS = { + "record": ( + "Record YOUR app (web browser by default; --backend " + "windows/macos/linux/rdp for native desktop capture)." + ), "induce": "Induce a parameterized program from multiple recordings.", "run": "Run a bundle under a fail-closed deployment configuration.", "resume": "Resume a durably paused run from its verified checkpoint.", @@ -224,34 +228,13 @@ def connect(pairing, uri, host, device_name, destination_kind, trusted_host): _run_flow(argv) -@flow.command("record") -@click.option("--url", required=True, help="URL of the app to record against") -@click.option("--out", required=True, help="Recording output directory") -@click.option( - "--secret", - multiple=True, - metavar="FIELD", - help="Mark a typed field as SECRET (value never persisted). Repeatable.", -) -@click.option( - "--param", - multiple=True, - metavar="FIELD", - help="Record a typed field as a PARAMETER (overridable at replay). Repeatable.", -) -@click.option( - "--headless", is_flag=True, help="Run the browser headless (scripted/CI recording)" -) -def flow_record(url, out, secret, param, headless): - """Record YOUR app interactively in a headed browser.""" - argv = ["record", "--url", url, "--out", out] - for value in secret: - argv += ["--secret", value] - for value in param: - argv += ["--param", value] - if headless: - argv.append("--headless") - _run_flow(argv) +# NOTE: `record` is intentionally NOT wrapped with explicit click options. +# It delegates through _FlowPassthroughGroup so every engine option +# (--backend web/windows/macos/linux/rdp, --agent-url, --macos-app, +# --linux-app, --rdp-host, --task, ...) forwards verbatim and new engine +# options never need a launcher release. An earlier explicit wrapper here +# hid --backend (and required --url, which the engine only requires for +# --backend web). @flow.command("demo-record") diff --git a/tests/test_cli_smoke.py b/tests/test_cli_smoke.py index 5374f6923..4ffa94de9 100644 --- a/tests/test_cli_smoke.py +++ b/tests/test_cli_smoke.py @@ -149,6 +149,12 @@ def test_doctor_lists_flow_as_core_not_extras(): FLOW_VERBS = {"demo-record", "record", "compile", "replay", "lint", "certify"} +# Verbs wrapped with explicit click options (local --help, no engine import). +# `record` is intentionally absent: it delegates through the passthrough +# group so every engine option (--backend, --agent-url, ...) forwards +# verbatim — its --help comes from the engine itself. +FLOW_WRAPPED_VERBS = FLOW_VERBS - {"record"} + def test_launcher_requires_pairing_enabled_flow_release(): """Every install route must resolve an engine with transactional pairing.""" @@ -188,13 +194,102 @@ def test_flow_help_lists_verbs(): def test_flow_subcommand_help_renders(): - """Each flow subcommand renders --help without importing flow.""" + """Each explicitly wrapped flow subcommand renders --help without + importing flow.""" runner = CliRunner() - for verb in FLOW_VERBS: + for verb in FLOW_WRAPPED_VERBS: result = runner.invoke(cli_main, ["flow", verb, "--help"]) assert result.exit_code == 0, f"`flow {verb} --help` failed: {result.output}" +def test_flow_record_help_is_engine_help(): + """`openadapt flow record --help` must render the ENGINE's help (with + --backend and the desktop-backend options), not a launcher wrapper + that hides them.""" + _require_openadapt_flow() + result = CliRunner().invoke(cli_main, ["flow", "record", "--help"]) + assert result.exit_code == 0, result.output + for option in ("--backend", "--agent-url", "--task"): + assert option in result.output, ( + f"{option} missing from `flow record --help`; the launcher is " + "hiding engine options again" + ) + + +def test_flow_record_forwards_backend_options(monkeypatch): + """Regression (launcher <=1.7.0): the explicit record wrapper rejected + `--backend windows` with "No such option". record must forward every + engine option verbatim, like run/push.""" + captured = {} + monkeypatch.setattr( + "openadapt.cli._run_flow", lambda argv: captured.update(argv=list(argv)) + ) + result = CliRunner().invoke( + cli_main, + [ + "flow", + "record", + "--backend", + "windows", + "--agent-url", + "http://localhost:5001", + "--out", + "rec", + "--task", + "triage note", + ], + ) + assert result.exit_code == 0, result.output + assert captured["argv"] == [ + "record", + "--backend", + "windows", + "--agent-url", + "http://localhost:5001", + "--out", + "rec", + "--task", + "triage note", + ] + + +def test_flow_record_web_invocation_forwards_verbatim(monkeypatch): + """The pre-existing web invocation shape keeps working unchanged.""" + captured = {} + monkeypatch.setattr( + "openadapt.cli._run_flow", lambda argv: captured.update(argv=list(argv)) + ) + result = CliRunner().invoke( + cli_main, + [ + "flow", + "record", + "--url", + "http://localhost:3000", + "--out", + "rec", + "--secret", + "password", + "--param", + "note", + "--headless", + ], + ) + assert result.exit_code == 0, result.output + assert captured["argv"] == [ + "record", + "--url", + "http://localhost:3000", + "--out", + "rec", + "--secret", + "password", + "--param", + "note", + "--headless", + ] + + def test_flow_missing_shows_install_hint(monkeypatch): """When openadapt-flow isn't installed, `openadapt flow ` exits nonzero with a pip install hint instead of a traceback."""