diff --git a/src/reverse_api/cli.py b/src/reverse_api/cli.py index be94944..7a8c8c2 100644 --- a/src/reverse_api/cli.py +++ b/src/reverse_api/cli.py @@ -1,5 +1,6 @@ import asyncio import json +import os import random import sys from contextlib import contextmanager @@ -54,9 +55,99 @@ config_manager = ConfigManager(get_config_path()) session_manager = SessionManager(get_history_path()) -# Stable schema version for --json outputs consumed by other agents/scripts. +# Schema version for --json outputs. Wrappers can query it via +# `reverse-api-engineer --json-schema-version`. AGENT_JSON_SCHEMA_VERSION = 1 +# Map of stable usage keys → SDK-specific candidates (first match wins). +# Lets `agent --json` and `engineer --json` emit a stable cost/token shape +# regardless of which SDK (Claude / OpenCode / Copilot) ran underneath. +_STABLE_USAGE_KEYS: dict[str, tuple[str, ...]] = { + "input_tokens": ("input_tokens",), + "output_tokens": ("output_tokens",), + "cache_read_tokens": ("cache_read_input_tokens", "cache_read_tokens"), + "cache_write_tokens": ("cache_creation_input_tokens", "cache_write_tokens"), + "total_cost_usd": ("estimated_cost_usd", "total_cost_usd", "total_cost"), +} + + +def _normalize_usage(raw: dict | None) -> dict: + """Return a stable subset of usage fields, keeping the SDK-native dict under .raw. + + Wrappers can rely on the top-level keys; per-SDK extras stay under raw. + """ + if not raw or not isinstance(raw, dict): + return {} + out: dict = {} + for stable_key, candidates in _STABLE_USAGE_KEYS.items(): + for c in candidates: + if c in raw: + out[stable_key] = raw[c] + break + out["raw"] = raw + return out + + +# Machine-readable error categories. Wrappers can react differently to each +# without pattern-matching on the human-readable `error` string. +ERROR_KINDS = ( + "misuse", # user input invalid / missing required arg + "config_invalid", # config file or env var malformed + "permission_denied", # filesystem / API perm denied + "network", # DNS / TCP / TLS / timeout + "engine_failure", # SDK or capture engine crashed mid-run + "interrupted", # KeyboardInterrupt / SIGINT + "unknown", # default fallback +) + + +def _format_error_message(error: str | BaseException | None) -> str | None: + """Render an exception or string into a human-readable error message. + + KeyboardInterrupt has no useful str() — we return the conventional + "interrupted" so downstream wrappers can match on a stable message. + Empty exception messages fall back to the class name. + """ + if error is None: + return None + if isinstance(error, KeyboardInterrupt): + return "interrupted" + if isinstance(error, BaseException): + return str(error) or type(error).__name__ + return error + + +def _classify_error(error: str | BaseException | None, *, default: str = "unknown") -> str | None: + """Map an Exception or human error message to one of the ERROR_KINDS. + + Callers may pass a stronger hint by setting `default` (e.g. `misuse` from + a CLI argument check, before any exception). + """ + if error is None: + return None + if isinstance(error, BaseException): + if isinstance(error, KeyboardInterrupt): + return "interrupted" + if isinstance(error, PermissionError): + return "permission_denied" + if isinstance(error, (ConnectionError, TimeoutError)): + return "network" + msg = str(error) + else: + msg = error + low = msg.lower() + if "permission denied" in low or "errno 13" in low: + return "permission_denied" + if "interrupted" in low: + return "interrupted" + if "is required" in low or "missing required" in low or "no such option" in low or "in non-interactive" in low: + return "misuse" + if any(kw in low for kw in ("connection refused", "timeout", "timed out", "dns", "network", "unreachable", "ssl")): + return "network" + if "not found" in low or "no run" in low or "produced no run" in low or "produced no result" in low: + return "engine_failure" + return default + @contextmanager def _quiet_consoles_for_json(): @@ -85,13 +176,195 @@ def _quiet_consoles_for_json(): c._file = original_inner +def _build_dry_run_payload( + *, + prompt: str | None, + url: str | None, + model: str | None, + output_dir: str | None, + headless: bool, +) -> dict: + """Validate config / env / deps without launching a browser. + + Returns a payload with the same top-level shape as `_build_agent_payload` + plus a `checks` array (each item: name, status: ok|warn|error, message) + and a `would_run` sub-object showing what an actual `agent` invocation + would do with these inputs. Status is `error` if any check is `error`, + otherwise `ok`. Misuse errors (missing prompt) get `error_kind=misuse`; + runtime issues (missing API key, missing node) get `config_invalid`. + """ + import shutil + import subprocess + + checks: list[dict] = [] + + # 1. Prompt + if not prompt or not prompt.strip(): + checks.append({"name": "prompt", "status": "error", "message": "--prompt is required"}) + else: + checks.append({"name": "prompt", "status": "ok", "message": f"{len(prompt)} chars"}) + + # 2. URL (optional, but if given it must look reasonable) + if url: + if url.startswith("http://") or url.startswith("https://"): + checks.append({"name": "url", "status": "ok", "message": url}) + else: + checks.append({ + "name": "url", + "status": "error", + "message": f"url must start with http:// or https://, got {url!r}", + }) + else: + checks.append({"name": "url", "status": "ok", "message": "(none — agent will pick a start)"}) + + # 3. Agent provider + agent_provider = config_manager.get("agent_provider", "auto") + if agent_provider in ("auto", "chrome-mcp"): + checks.append({"name": "agent_provider", "status": "ok", "message": agent_provider}) + else: + checks.append({ + "name": "agent_provider", + "status": "error", + "message": f"unknown agent_provider {agent_provider!r}; expected 'auto' or 'chrome-mcp'", + }) + + # 4. SDK + API key presence (we only check env var existence, not validity) + sdk = config_manager.get("sdk", "claude") + sdk_env_var = { + "claude": "ANTHROPIC_API_KEY", + "opencode": "OPENCODE_API_KEY", + "copilot": "GITHUB_COPILOT_TOKEN", + }.get(sdk) + if sdk_env_var is None: + checks.append({ + "name": "sdk", + "status": "error", + "message": f"unknown sdk {sdk!r}; expected 'claude', 'opencode', or 'copilot'", + }) + elif not os.environ.get(sdk_env_var): + checks.append({ + "name": f"sdk:{sdk}", + "status": "warn", + "message": f"{sdk_env_var} not set in env (the SDK may still resolve auth via a config file)", + }) + else: + checks.append({"name": f"sdk:{sdk}", "status": "ok", "message": f"{sdk_env_var} present"}) + + # 5. Node.js + npx for MCP servers (both auto and chrome-mcp shell out to + # `npx `; minimal Docker images sometimes ship `node` without + # `npx`, so checking only `node` would lull dry-run into a false ok). + node = shutil.which("node") + if node is None: + checks.append({ + "name": "node", + "status": "error", + "message": "node not found in PATH; required by both auto (rae-playwright-mcp) and chrome-mcp", + }) + else: + try: + ver = subprocess.run( + [node, "--version"], capture_output=True, text=True, timeout=5 + ).stdout.strip() + checks.append({"name": "node", "status": "ok", "message": ver}) + except Exception as e: + checks.append({"name": "node", "status": "warn", "message": f"could not query version: {e}"}) + + npx = shutil.which("npx") + if npx is None: + checks.append({ + "name": "npx", + "status": "error", + "message": "npx not found in PATH; both MCP servers are launched via `npx `", + }) + else: + checks.append({"name": "npx", "status": "ok", "message": npx}) + + # 6. Headed chrome-mcp requires the user has a real Chrome with auto-connect + if agent_provider == "chrome-mcp" and not headless: + checks.append({ + "name": "chrome-mcp:auto-connect", + "status": "warn", + "message": "chrome-mcp without --headless requires Chrome 146+ with auto-connect enabled at chrome://inspect/#remote-debugging — this is not auto-checkable", + }) + + # 7. Output dir writability — probe with a unique filename so we never + # clobber a real user file (a fixed name like `.dry_run_write_probe` + # could legitimately exist in someone's output dir). + import secrets + + base = Path(output_dir or config_manager.get("output_dir") or "~/.reverse-api/runs").expanduser() + try: + base.mkdir(parents=True, exist_ok=True) + probe = base / f".rae_dry_run_probe_{os.getpid()}_{secrets.token_hex(4)}" + # If somehow this exact filename already exists, refuse to touch it. + if probe.exists(): + raise FileExistsError(f"unique probe path collision: {probe}") + probe.write_text("") + probe.unlink() + checks.append({"name": "output_dir", "status": "ok", "message": str(base)}) + except Exception as e: + checks.append({ + "name": "output_dir", + "status": "error", + "message": f"{base}: {e}", + }) + + # Aggregate + has_error = any(c["status"] == "error" for c in checks) + final_error = None + error_kind = None + if has_error: + first_err = next(c for c in checks if c["status"] == "error") + final_error = f"{first_err['name']}: {first_err['message']}" + # Misuse for prompt/url; config_invalid otherwise + error_kind = "misuse" if first_err["name"] in ("prompt", "url") else "config_invalid" + + # `would_run.model` resolution mirrors the live capture path: each SDK + # has its own model config key, so picking `claude_code_model` for an + # opencode/copilot session would misreport what would actually run. + sdk_model_key = { + "claude": "claude_code_model", + "opencode": "opencode_model", + "copilot": "copilot_model", + }.get(sdk, "claude_code_model") + sdk_default_model = { + "claude": "claude-sonnet-4-6", + "opencode": "claude-opus-4-6", + "copilot": "gpt-5", + }.get(sdk, "claude-sonnet-4-6") + resolved_model = model or config_manager.get(sdk_model_key, sdk_default_model) + + return { + "schema_version": AGENT_JSON_SCHEMA_VERSION, + "status": "error" if has_error else "ok", + "run_id": None, + "prompt": prompt, + "url": url, + "mode": "dry-run", + "har_path": None, + "script_path": None, + "usage": {}, + "error": final_error, + "error_kind": error_kind, + "would_run": { + "agent_provider": agent_provider, + "sdk": sdk, + "model": resolved_model, + "output_dir": str(base), + "headless": headless, + }, + "checks": checks, + } + + def _build_agent_payload( result: dict | None, *, prompt: str | None, url: str | None, output_dir: str | None = None, - error: str | None = None, + error: str | BaseException | None = None, + error_kind_hint: str = "unknown", ) -> dict: """Normalize an agent capture result into a stable JSON shape. @@ -101,22 +374,27 @@ def _build_agent_payload( result = result or {} run_id = result.get("run_id") inner_error = result.get("error") - final_error = error or inner_error or (None if run_id else "agent capture produced no run") + final_error_obj = error if error is not None else inner_error + if final_error_obj is None and not run_id: + final_error_obj = "agent capture produced no run" + error_str = _format_error_message(final_error_obj) + error_kind = _classify_error(final_error_obj, default=error_kind_hint) if final_error_obj else None har_path = None if run_id: candidate = get_har_dir(run_id, output_dir) / "recording.har" har_path = str(candidate) if candidate.exists() else None return { "schema_version": AGENT_JSON_SCHEMA_VERSION, - "status": "error" if final_error else "ok", + "status": "error" if error_str else "ok", "run_id": run_id, "prompt": prompt, "url": url, "mode": result.get("mode"), "har_path": har_path, "script_path": result.get("script_path"), - "usage": result.get("usage") or {}, - "error": final_error, + "usage": _normalize_usage(result.get("usage")), + "error": error_str, + "error_kind": error_kind, } @@ -126,7 +404,8 @@ def _build_engineer_payload( run_id: str, prompt: str | None, fresh: bool, - error: str | None = None, + error: str | BaseException | None = None, + error_kind_hint: str = "unknown", ) -> dict: """Normalize an engineer-mode result into a stable JSON shape. @@ -135,18 +414,23 @@ def _build_engineer_payload( to --prompt (which may have been used as either a full replacement or as additional instructions depending on --fresh). """ - result = result if result is not None else {} - inner_error = result.get("error") if isinstance(result, dict) else None - final_error = error or inner_error or (None if result else "engineering produced no result") + result = result if isinstance(result, dict) else {} + inner_error = result.get("error") + final_error_obj = error if error is not None else inner_error + if final_error_obj is None and not result: + final_error_obj = "engineering produced no result" + error_str = _format_error_message(final_error_obj) + error_kind = _classify_error(final_error_obj, default=error_kind_hint) if final_error_obj else None return { "schema_version": AGENT_JSON_SCHEMA_VERSION, - "status": "error" if final_error else "ok", + "status": "error" if error_str else "ok", "run_id": run_id, "prompt": prompt, "fresh": fresh, - "script_path": result.get("script_path") if isinstance(result, dict) else None, - "usage": (result.get("usage") if isinstance(result, dict) else None) or {}, - "error": final_error, + "script_path": result.get("script_path"), + "usage": _normalize_usage(result.get("usage")), + "error": error_str, + "error_kind": error_kind, } # Mode definitions @@ -416,15 +700,26 @@ def get_prompt(): @click.group(invoke_without_command=True, context_settings=CONTEXT_SETTINGS) @click.pass_context @click.version_option(version=__version__) -def main(ctx: click.Context): +@click.option( + "--json-schema-version", + "show_schema_version", + is_flag=True, + help="Print the agent/engineer JSON schema_version this binary emits and exit.", +) +def main(ctx: click.Context, show_schema_version: bool): """reverse-api-engineer: reverse engineer apis. Run without a subcommand to start the interactive REPL; use agent, manual, or engineer for CLI mode. Most subcommands accept --json and --no-interactive for scripted use; see - ` --help` for per-command details. + ` --help` for per-command details. Wrappers that need to gate on the + payload schema can call `reverse-api-engineer --json-schema-version`. """ + if show_schema_version: + click.echo(str(AGENT_JSON_SCHEMA_VERSION)) + ctx.exit(0) + if ctx.invoked_subcommand is None: # Refuse to drop into the prompt_toolkit REPL when stdin is not a TTY: # without an interactive terminal the REPL would block on stdin @@ -1105,12 +1400,28 @@ def manual(prompt, url, reverse_engineer, model, output_dir): is_flag=True, help="Launch the MCP-controlled browser in headless mode (required on machines without an X server). For chrome-mcp this drops --autoConnect since auto-connect requires a headed Chrome instance.", ) -def agent(prompt, url, model, output_dir, no_interactive, as_json, headless): +@click.option( + "--dry-run", + is_flag=True, + help="Validate prompt/url/config/env without launching the browser. Emits a manifest of what would run + check results. Implies --json. Exits 0 if all checks pass, 1 if any error.", +) +def agent(prompt, url, model, output_dir, no_interactive, as_json, headless, dry_run): """Run autonomous agent browser session. Agent mode runs an integrated capture + reverse-engineering pipeline; if you only want a HAR recording, use `manual --no-engineer` instead. """ + if dry_run: + # --dry-run is fundamentally about emitting machine-parseable validation + # results, so it implies --json (and therefore --no-interactive). + with _quiet_consoles_for_json() as real_stdout: + payload = _build_dry_run_payload( + prompt=prompt, url=url, model=model, output_dir=output_dir, headless=headless + ) + real_stdout.write(json.dumps(payload) + "\n") + real_stdout.flush() + sys.exit(0 if payload["status"] == "ok" else 1) + no_interactive = no_interactive or as_json if no_interactive and not (prompt and prompt.strip()): @@ -1121,6 +1432,7 @@ def agent(prompt, url, model, output_dir, no_interactive, as_json, headless): url=url, output_dir=output_dir, error="--prompt is required in non-interactive/--json mode", + error_kind_hint="misuse", ) click.echo(json.dumps(misuse)) else: @@ -1154,13 +1466,15 @@ def agent(prompt, url, model, output_dir, no_interactive, as_json, headless): headless=headless, ) payload = _build_agent_payload(result, prompt=prompt, url=url, output_dir=output_dir) - except KeyboardInterrupt: + except KeyboardInterrupt as e: payload = _build_agent_payload( - {}, prompt=prompt, url=url, output_dir=output_dir, error="interrupted" + {}, prompt=prompt, url=url, output_dir=output_dir, error=e ) except Exception as e: + # Pass the exception object so _classify_error can use isinstance + # (PermissionError → permission_denied, ConnectionError → network, ...) payload = _build_agent_payload( - {}, prompt=prompt, url=url, output_dir=output_dir, error=str(e) + {}, prompt=prompt, url=url, output_dir=output_dir, error=e ) real_stdout.write(json.dumps(payload) + "\n") @@ -1489,7 +1803,7 @@ def run_auto_capture( 2 misuse """ ) -@click.argument("run_id") +@click.argument("run_id", required=False) @click.option( "--prompt", "-p", @@ -1524,6 +1838,26 @@ def run_auto_capture( ) def engineer(run_id, prompt, fresh, model, output_dir, no_interactive, as_json): """Run reverse engineering on a previous run.""" + # `run_id` is declared optional at the click level so that wrappers using + # --json get a JSON misuse payload instead of Click's plain-text "missing + # argument" error. We re-validate inline to preserve the same exit-2 UX + # for non-JSON invocations. + if not run_id: + if as_json: + misuse = _build_engineer_payload( + None, + run_id="", + prompt=prompt, + fresh=fresh, + error="RUN_ID is required", + error_kind_hint="misuse", + ) + click.echo(json.dumps(misuse)) + else: + click.echo("Usage: reverse-api-engineer engineer [OPTIONS] RUN_ID", err=True) + click.echo("\nError: Missing argument 'RUN_ID'.", err=True) + sys.exit(2) + # --fresh treats --prompt as a full replacement of the original goal; # without --fresh, --prompt is additive so the captured run's context is preserved. main_prompt = prompt if fresh else None @@ -1557,13 +1891,14 @@ def engineer(run_id, prompt, fresh, model, output_dir, no_interactive, as_json): interactive=interactive, ) payload = _build_engineer_payload(result, run_id=run_id, prompt=prompt, fresh=fresh) - except KeyboardInterrupt: + except KeyboardInterrupt as e: payload = _build_engineer_payload( - None, run_id=run_id, prompt=prompt, fresh=fresh, error="interrupted" + None, run_id=run_id, prompt=prompt, fresh=fresh, error=e ) except Exception as e: + # Pass the exception object so _classify_error can use isinstance. payload = _build_engineer_payload( - None, run_id=run_id, prompt=prompt, fresh=fresh, error=str(e) + None, run_id=run_id, prompt=prompt, fresh=fresh, error=e ) real_stdout.write(json.dumps(payload) + "\n") diff --git a/tests/test_cli_agent_json.py b/tests/test_cli_agent_json.py index f15d27d..4bd7569 100644 --- a/tests/test_cli_agent_json.py +++ b/tests/test_cli_agent_json.py @@ -28,6 +28,7 @@ "script_path", "usage", "error", + "error_kind", } @@ -40,20 +41,37 @@ def test_success_shape(self, tmp_path): "run_id": "abc123", "mode": "auto", "script_path": str(tmp_path / "scripts" / "api_client.py"), - "usage": {"input_tokens": 1, "output_tokens": 2, "total_cost": 0.001}, + # Mix of Claude SDK keys to exercise normalization (cache_creation_input_tokens + # → cache_write_tokens, estimated_cost_usd → total_cost_usd). + "usage": { + "input_tokens": 1, + "output_tokens": 2, + "cache_creation_input_tokens": 100, + "cache_read_input_tokens": 50, + "estimated_cost_usd": 0.001, + }, }, prompt="capture the X api", url="https://example.com", ) assert payload["schema_version"] == AGENT_JSON_SCHEMA_VERSION + assert set(payload.keys()) == EXPECTED_PAYLOAD_KEYS # No run produced a HAR yet on disk in this test, so har_path is None assert payload["status"] == "ok" assert payload["run_id"] == "abc123" assert payload["mode"] == "auto" assert payload["prompt"] == "capture the X api" assert payload["url"] == "https://example.com" - assert payload["usage"]["total_cost"] == 0.001 + # Stable normalized usage subset + assert payload["usage"]["input_tokens"] == 1 + assert payload["usage"]["output_tokens"] == 2 + assert payload["usage"]["cache_write_tokens"] == 100 + assert payload["usage"]["cache_read_tokens"] == 50 + assert payload["usage"]["total_cost_usd"] == 0.001 + # Raw SDK shape still available for power users + assert payload["usage"]["raw"]["cache_creation_input_tokens"] == 100 assert payload["error"] is None + assert payload["error_kind"] is None # Must be JSON-serializable (no Path objects sneaking through) json.dumps(payload) @@ -62,6 +80,7 @@ def test_no_run_id_is_error(self): assert payload["status"] == "error" assert payload["error"] assert payload["run_id"] is None + assert payload["error_kind"] == "engine_failure" def test_explicit_error_overrides(self): payload = _build_agent_payload( diff --git a/tests/test_cli_followups.py b/tests/test_cli_followups.py index c4d99af..8c03cb3 100644 --- a/tests/test_cli_followups.py +++ b/tests/test_cli_followups.py @@ -27,6 +27,7 @@ "script_path", "usage", "error", + "error_kind", } @@ -78,7 +79,10 @@ class TestBuildEngineerPayload: def test_success_shape(self): payload = _build_engineer_payload( - {"script_path": "/abs/api_client.py", "usage": {"total_cost": 0.001}}, + { + "script_path": "/abs/api_client.py", + "usage": {"input_tokens": 1, "output_tokens": 2, "total_cost": 0.001}, + }, run_id="abc123", prompt="add pagination", fresh=False, @@ -89,14 +93,18 @@ def test_success_shape(self): assert payload["prompt"] == "add pagination" assert payload["fresh"] is False assert payload["script_path"] == "/abs/api_client.py" - assert payload["usage"]["total_cost"] == 0.001 + # `total_cost` (legacy/Copilot key) is normalized to `total_cost_usd` + assert payload["usage"]["total_cost_usd"] == 0.001 + assert payload["usage"]["raw"]["total_cost"] == 0.001 assert payload["error"] is None + assert payload["error_kind"] is None assert set(payload.keys()) == EXPECTED_ENGINEER_KEYS def test_none_result_is_error(self): payload = _build_engineer_payload(None, run_id="abc", prompt=None, fresh=False) assert payload["status"] == "error" assert payload["error"] + assert payload["error_kind"] == "engine_failure" assert set(payload.keys()) == EXPECTED_ENGINEER_KEYS def test_explicit_error_overrides(self): @@ -191,6 +199,374 @@ def test_no_interactive_flag_accepted(self): assert result.exit_code == 0, result.output +class TestEngineerJsonMissingRunId: + """`engineer --json` without RUN_ID must emit a JSON misuse payload, not + Click's plain-text 'missing argument' error. + + Regression for kind-agent observation on PR #66. + """ + + def test_json_emits_misuse_payload_when_run_id_missing(self): + from reverse_api.cli import engineer + + runner = CliRunner() + result = runner.invoke(engineer, ["--json"]) + assert result.exit_code == 2 + # stdout MUST be valid JSON (wrappers expect it) + payload = json.loads(result.stdout.strip()) + assert payload["status"] == "error" + assert payload["error_kind"] == "misuse" + assert "RUN_ID" in payload["error"] + # And the JSON shape is the full engineer schema + assert "schema_version" in payload + assert "fresh" in payload + + def test_plain_invocation_still_emits_click_style_error(self): + """Non-JSON path keeps Click's familiar Usage + Error format on stderr.""" + from reverse_api.cli import engineer + + runner = CliRunner() + result = runner.invoke(engineer, []) + assert result.exit_code == 2 + # The Click-style error goes to stderr, NOT JSON + combined = (result.stderr or "") + (result.stdout or "") + assert "Usage" in combined + assert "RUN_ID" in combined + + +class TestSchemaV2Normalization: + """v2 helpers: _normalize_usage, _classify_error, --json-schema-version.""" + + def test_normalize_usage_picks_stable_keys(self): + """Claude SDK emits cache_creation_input_tokens / estimated_cost_usd; + Copilot/OpenCode use different keys. Normalization gives a stable + subset and parks everything under .raw.""" + from reverse_api.cli import _normalize_usage + + out = _normalize_usage({ + "input_tokens": 10, + "output_tokens": 20, + "cache_creation_input_tokens": 100, + "cache_read_input_tokens": 200, + "estimated_cost_usd": 0.05, + "service_tier": "standard", + "iterations": [], + }) + assert out["input_tokens"] == 10 + assert out["output_tokens"] == 20 + assert out["cache_write_tokens"] == 100 + assert out["cache_read_tokens"] == 200 + assert out["total_cost_usd"] == 0.05 + # SDK extras are preserved under raw, not promoted to top level + assert out["raw"]["service_tier"] == "standard" + assert out["raw"]["iterations"] == [] + assert "service_tier" not in out # stable subset only + assert "iterations" not in out + + def test_normalize_usage_alt_legacy_keys(self): + """`total_cost` (Copilot) and direct `cache_read_tokens` (already-normalized + input) map cleanly through the same pipeline.""" + from reverse_api.cli import _normalize_usage + + out = _normalize_usage({"total_cost": 0.42, "cache_read_tokens": 5}) + assert out["total_cost_usd"] == 0.42 + assert out["cache_read_tokens"] == 5 + + def test_normalize_usage_empty(self): + from reverse_api.cli import _normalize_usage + + assert _normalize_usage(None) == {} + assert _normalize_usage({}) == {} + assert _normalize_usage("not a dict") == {} + + def test_classify_error_kinds(self): + from reverse_api.cli import _classify_error + + assert _classify_error(None) is None + assert _classify_error(KeyboardInterrupt()) == "interrupted" + assert _classify_error(PermissionError("nope")) == "permission_denied" + assert _classify_error(ConnectionError("DNS failed")) == "network" + assert _classify_error(TimeoutError("timed out")) == "network" + assert _classify_error("[Errno 13] Permission denied: '/x'") == "permission_denied" + assert _classify_error("--prompt is required in non-interactive/--json mode") == "misuse" + assert _classify_error("agent capture produced no run") == "engine_failure" + assert _classify_error("connection refused") == "network" + assert _classify_error("totally unrecognized message") == "unknown" + # Caller can override the default + assert _classify_error("totally unrecognized message", default="engine_failure") == "engine_failure" + + def test_misuse_payload_has_misuse_kind(self): + """`agent --json` without --prompt → error_kind=misuse (not unknown).""" + from reverse_api.cli import agent + + runner = CliRunner() + result = runner.invoke(agent, ["--json"]) + payload = json.loads(result.stdout.strip()) + assert payload["error_kind"] == "misuse" + + def test_keyboard_interrupt_payload_has_interrupted_kind(self): + from reverse_api.cli import agent + + runner = CliRunner() + with patch("reverse_api.cli.run_agent_capture", side_effect=KeyboardInterrupt): + result = runner.invoke(agent, ["--json", "-p", "x"]) + payload = json.loads(result.stdout.strip().splitlines()[-1]) + assert payload["error_kind"] == "interrupted" + assert payload["error"] == "interrupted" + + def test_permission_error_payload_has_permission_denied_kind(self): + from reverse_api.cli import agent + + runner = CliRunner() + with patch( + "reverse_api.cli.run_agent_capture", + side_effect=PermissionError(13, "Permission denied", "/forbidden"), + ): + result = runner.invoke(agent, ["--json", "-p", "x"]) + payload = json.loads(result.stdout.strip().splitlines()[-1]) + assert payload["error_kind"] == "permission_denied" + + +class TestJsonSchemaVersionFlag: + """`--json-schema-version` exposes the version a wrapper can gate on.""" + + def test_root_flag_emits_version(self): + runner = CliRunner() + result = runner.invoke(main, ["--json-schema-version"]) + assert result.exit_code == 0 + from reverse_api.cli import AGENT_JSON_SCHEMA_VERSION as v + assert result.stdout.strip() == str(v) + + def test_root_flag_advertised_in_help(self): + runner = CliRunner() + result = runner.invoke(main, ["--help"]) + assert "--json-schema-version" in result.output + + +class TestAgentDryRun: + """`agent --dry-run` validates without launching the browser.""" + + def test_dry_run_ok_path(self, tmp_path): + """All checks pass → status=ok, exit 0, full payload + checks array.""" + from reverse_api.cli import agent as agent_cmd + + runner = CliRunner() + # Patch config_manager so we don't depend on the user's real config + with patch("reverse_api.cli.config_manager") as cm, \ + patch.dict("os.environ", {"ANTHROPIC_API_KEY": "fake"}, clear=False): + cm.get.side_effect = lambda key, default=None: { + "agent_provider": "auto", + "sdk": "claude", + "claude_code_model": "claude-sonnet-4-6", + "output_dir": str(tmp_path), + }.get(key, default) + result = runner.invoke( + agent_cmd, ["--dry-run", "-p", "fetch jobs", "-u", "https://example.com"] + ) + + assert result.exit_code == 0, result.output + payload = json.loads(result.stdout.strip()) + assert payload["status"] == "ok" + assert payload["mode"] == "dry-run" + assert payload["run_id"] is None + assert payload["error"] is None + assert payload["would_run"]["agent_provider"] == "auto" + assert payload["would_run"]["sdk"] == "claude" + assert payload["would_run"]["headless"] is False + # Checks include prompt, url, agent_provider, sdk, node, output_dir + check_names = {c["name"] for c in payload["checks"]} + assert "prompt" in check_names + assert "url" in check_names + assert "agent_provider" in check_names + assert "node" in check_names + assert "output_dir" in check_names + + def test_dry_run_missing_prompt_is_misuse(self, tmp_path): + """Missing --prompt → error_kind=misuse, exit 1, no browser launched.""" + from reverse_api.cli import agent as agent_cmd + + runner = CliRunner() + with patch("reverse_api.cli.config_manager") as cm: + cm.get.side_effect = lambda key, default=None: { + "agent_provider": "auto", + "sdk": "claude", + "output_dir": str(tmp_path), + }.get(key, default) + result = runner.invoke(agent_cmd, ["--dry-run"]) + + assert result.exit_code == 1 + payload = json.loads(result.stdout.strip()) + assert payload["status"] == "error" + assert payload["error_kind"] == "misuse" + assert any(c["name"] == "prompt" and c["status"] == "error" for c in payload["checks"]) + + def test_dry_run_bad_url_is_misuse(self, tmp_path): + """A url that doesn't start with http(s):// is flagged.""" + from reverse_api.cli import agent as agent_cmd + + runner = CliRunner() + with patch("reverse_api.cli.config_manager") as cm: + cm.get.side_effect = lambda key, default=None: { + "agent_provider": "auto", + "sdk": "claude", + "output_dir": str(tmp_path), + }.get(key, default) + result = runner.invoke(agent_cmd, ["--dry-run", "-p", "x", "-u", "ftp://nope"]) + + assert result.exit_code == 1 + payload = json.loads(result.stdout.strip()) + assert payload["error_kind"] == "misuse" + assert any(c["name"] == "url" and c["status"] == "error" for c in payload["checks"]) + + def test_dry_run_unwritable_output_dir_is_config_invalid(self): + """Unwritable output_dir → error_kind=config_invalid, not misuse.""" + from reverse_api.cli import agent as agent_cmd + + runner = CliRunner() + with patch("reverse_api.cli.config_manager") as cm: + cm.get.side_effect = lambda key, default=None: { + "agent_provider": "auto", + "sdk": "claude", + "output_dir": "/sys/forbidden", + }.get(key, default) + result = runner.invoke( + agent_cmd, ["--dry-run", "-p", "x", "--output-dir", "/sys/forbidden"] + ) + + assert result.exit_code == 1 + payload = json.loads(result.stdout.strip()) + assert payload["error_kind"] == "config_invalid" + assert any(c["name"] == "output_dir" and c["status"] == "error" for c in payload["checks"]) + + def test_dry_run_does_not_launch_browser(self, tmp_path): + """--dry-run must NOT call run_agent_capture (no browser, no LLM, no cost).""" + from reverse_api.cli import agent as agent_cmd + + runner = CliRunner() + with patch("reverse_api.cli.config_manager") as cm, \ + patch("reverse_api.cli.run_agent_capture") as mock_run: + cm.get.side_effect = lambda key, default=None: { + "agent_provider": "auto", + "sdk": "claude", + "output_dir": str(tmp_path), + }.get(key, default) + runner.invoke(agent_cmd, ["--dry-run", "-p", "x", "-u", "https://example.com"]) + mock_run.assert_not_called() + + def test_dry_run_help_mentions_implies_json(self): + from reverse_api.cli import agent as agent_cmd + + runner = CliRunner() + result = runner.invoke(agent_cmd, ["--help"]) + assert "--dry-run" in result.output + # Click reflows whitespace, so "Implies\n--json" or "Implies --json" + assert "Implies" in result.output and "--json" in result.output + + def test_dry_run_checks_npx_separately_from_node(self, tmp_path): + """cubic-dev-ai PR #67 review (P2): MCP servers shell out to `npx`, + so dry-run must check npx availability — not just node — otherwise + a minimal Docker image with node-but-no-npx passes dry-run and then + fails the real run.""" + from reverse_api.cli import agent as agent_cmd + + runner = CliRunner() + + # Pretend npx is missing while node is present + def fake_which(name): + if name == "node": + return "/usr/bin/node" + if name == "npx": + return None + return None + + with patch("reverse_api.cli.config_manager") as cm, \ + patch("shutil.which", side_effect=fake_which): + cm.get.side_effect = lambda key, default=None: { + "agent_provider": "auto", + "sdk": "claude", + "output_dir": str(tmp_path), + }.get(key, default) + result = runner.invoke(agent_cmd, ["--dry-run", "-p", "x"]) + + assert result.exit_code == 1 + payload = json.loads(result.stdout.strip()) + assert payload["error_kind"] == "config_invalid" + npx_check = next(c for c in payload["checks"] if c["name"] == "npx") + assert npx_check["status"] == "error" + assert "npx not found" in npx_check["message"] + + def test_dry_run_probe_does_not_clobber_existing_files(self, tmp_path): + """cubic-dev-ai PR #67 review (P2): a fixed probe filename like + `.dry_run_write_probe` could legitimately exist in a user's output + dir and would be deleted by the probe. We use a unique filename + with PID + random hex so collisions are astronomically unlikely, + and refuse to touch any path that already exists.""" + from reverse_api.cli import agent as agent_cmd + + # Pre-populate the output dir with a file that would collide with + # the OLD fixed probe name. The dry-run must not delete it. + canary = tmp_path / ".dry_run_write_probe" + canary.write_text("user data — do not delete") + + runner = CliRunner() + with patch("reverse_api.cli.config_manager") as cm: + cm.get.side_effect = lambda key, default=None: { + "agent_provider": "auto", + "sdk": "claude", + "output_dir": str(tmp_path), + }.get(key, default) + result = runner.invoke(agent_cmd, ["--dry-run", "-p", "x"]) + + assert result.exit_code == 0, result.output + # The user's pre-existing file is untouched + assert canary.exists() + assert canary.read_text() == "user data — do not delete" + + def test_dry_run_resolves_correct_model_per_sdk(self, tmp_path): + """cubic-dev-ai PR #67 review (P2): when sdk=opencode the live agent + uses `opencode_model`, not `claude_code_model`. would_run.model must + reflect what would actually run, otherwise the manifest lies.""" + from reverse_api.cli import agent as agent_cmd + + runner = CliRunner() + + # Configure opencode SDK with a custom opencode_model + with patch("reverse_api.cli.config_manager") as cm: + cm.get.side_effect = lambda key, default=None: { + "agent_provider": "auto", + "sdk": "opencode", + "opencode_model": "claude-opus-4-6-custom", + "claude_code_model": "claude-sonnet-4-6-irrelevant", + "output_dir": str(tmp_path), + }.get(key, default) + result = runner.invoke(agent_cmd, ["--dry-run", "-p", "x"]) + + assert result.exit_code == 0, result.output + payload = json.loads(result.stdout.strip()) + assert payload["would_run"]["sdk"] == "opencode" + assert payload["would_run"]["model"] == "claude-opus-4-6-custom" + # And NOT the claude_code_model that the old code would have grabbed + assert "irrelevant" not in payload["would_run"]["model"] + + def test_dry_run_copilot_model_resolution(self, tmp_path): + from reverse_api.cli import agent as agent_cmd + + runner = CliRunner() + with patch("reverse_api.cli.config_manager") as cm: + cm.get.side_effect = lambda key, default=None: { + "agent_provider": "auto", + "sdk": "copilot", + "copilot_model": "gpt-5-custom", + "output_dir": str(tmp_path), + }.get(key, default) + result = runner.invoke(agent_cmd, ["--dry-run", "-p", "x"]) + + assert result.exit_code == 0, result.output + payload = json.loads(result.stdout.strip()) + assert payload["would_run"]["sdk"] == "copilot" + assert payload["would_run"]["model"] == "gpt-5-custom" + + class TestRootHelpMentionsScripted: """Item #6 partial: root --help should advertise scripted features."""