From 0fafeba2eb7944ca0c8c36582bce6625d7168b3c Mon Sep 17 00:00:00 2001 From: leonininder <8566513+leonininder@users.noreply.github.com> Date: Mon, 2 Nov 2026 17:33:16 +0800 Subject: [PATCH 1/4] feat: add Codex agent adapter --- README.md | 4 +- skills/reverse-api-engineer/SKILL.md | 4 +- .../reverse-api-engineer/references/setup.md | 6 +- src/reverse_api/cli.py | 103 +++- src/reverse_api/codex_engineer.py | 515 ++++++++++++++++++ src/reverse_api/config.py | 3 +- src/reverse_api/engineer.py | 23 +- src/reverse_api/prompts/auto/system.md | 3 +- tests/test_cli_agent_json.py | 24 + tests/test_cli_followups.py | 86 +++ tests/test_codex_engineer.py | 166 ++++++ tests/test_engineer.py | 21 + 12 files changed, 942 insertions(+), 16 deletions(-) create mode 100644 src/reverse_api/codex_engineer.py create mode 100644 tests/test_codex_engineer.py diff --git a/README.md b/README.md index dcb951a5..bc92fc49 100644 --- a/README.md +++ b/README.md @@ -119,6 +119,7 @@ Settings live in `~/.reverse-api/config.json` and can be edited via `/settings` "opencode_provider": "opencode", "copilot_model": "gpt-5", "cursor_model": "composer-2.5", + "codex_model": null, "output_dir": null, "output_language": "python", "real_time_sync": true, @@ -127,7 +128,8 @@ Settings live in `~/.reverse-api/config.json` and can be edited via `/settings` ``` - **Models**: Sonnet 5.5, Opus 5.5 (most capable), Opus 4.6 and Haiku 5.5 (fastest). Sonnet 4.6 stays the default until you choose another; it and Haiku 4.5 are still accepted in config and by `--model`. For OpenCode see [models.dev](https://models.dev). -- **SDK**: `claude` (default), `opencode`, `cursor`, or `copilot` (GitHub Copilot). +- **SDK**: `claude` (default), `opencode`, `cursor`, `copilot` (GitHub Copilot), or `codex`. +- **Codex SDK**: `sdk: "codex"` reuses the local Codex CLI login, preferring the Codex Desktop-bundled CLI and falling back to `codex` on `PATH`. `codex_model` is optional; RAE defaults to `gpt-6-luna`. Codex browser capture currently requires `agent_provider: "auto"`, which provides Playwright MCP and a HAR. The adapter supplies per-run MCP configuration and does not edit the global Codex config. - **Claude settings isolation**: Claude sessions don't load your Claude Code settings files, `CLAUDE.md`, hooks, plugins, permission rules or MCP servers, so your personal setup doesn't steer the agent. Organization-managed policy still applies, and Claude Code still reads its own state such as `~/.claude.json` and auto memory. If `~/.claude/settings.json` provides your authentication (`apiKeyHelper`, or Bedrock, Vertex or proxy variables under `env`), set `claude_setting_sources` to the layers to load, e.g. `["user"]` or `["user", "project", "local"]`. Each layer you list is loaded in full, including its hooks, plugins, permission rules and MCP servers. - **OpenCode setup**: with `sdk: "opencode"`, RAE reuses an existing server or downloads/starts `opencode-ai@latest` through `npx`; a global OpenCode installation is not required. Fresh configurations default to the free `opencode/big-pickle` model. `/settings` shows a loading spinner, then offers a searchable **OpenCode Provider / Model** picker populated from the server's connected, tool-capable catalog and marks free OpenCode options. Before creating a session, RAE validates the saved pair again and suggests currently available free models when configuration is invalid. Compatible older servers are reused with a version warning. Node.js 20+ is required for automatic startup. Password-protected servers use `OPENCODE_SERVER_PASSWORD` and optional `OPENCODE_SERVER_USERNAME`. Override startup with `OPENCODE_BASE_URL`, `RAE_OPENCODE_PACKAGE`, or `RAE_OPENCODE_AUTO_START=0`. - **Ollama through OpenCode**: choose provider `ollama` in `/settings`; RAE starts an installed Ollama daemon if needed, lists only installed models with tool calling and 64k+ context, and supplies OpenCode's provider config inline. Models are never downloaded silently. Override with `RAE_OLLAMA_BASE_URL` or `RAE_OLLAMA_AUTO_START=0`. diff --git a/skills/reverse-api-engineer/SKILL.md b/skills/reverse-api-engineer/SKILL.md index beb2336d..6fbe0d6e 100644 --- a/skills/reverse-api-engineer/SKILL.md +++ b/skills/reverse-api-engineer/SKILL.md @@ -45,10 +45,12 @@ reverse-api-engineer show --json reverse-api-engineer agent -p "" -u "" --dry-run ``` -This validates the prompt, URL, config and environment without opening a browser. Fix every `error` check before running. A `warn` on the API key is fine when the SDK authenticates another way (for example a logged-in Claude Code). +This validates the prompt, URL, config and environment without opening a browser. Fix every `error` check before running. A `warn` on the API key is fine when the SDK authenticates another way (for example a logged-in Claude Code or Codex CLI). `would_run.agent_provider` in the output is the browser the run will use. `chrome-mcp` means the user's own Chrome with their logins; `auto` and `agent-browser` start a fresh browser without them. When the task needs the user's login and the provider is not `chrome-mcp`, ask before switching it (see [sessions](references/sessions.md)). +The `codex` SDK reuses the local Codex CLI login. In the current RAE adapter, Codex agent capture requires `agent_provider: "auto"` so Playwright MCP records the HAR; it does not support `chrome-mcp` or `agent-browser` capture. `agent --dry-run` reports Codex login and browser-provider checks before opening a browser. + For `chrome-mcp`, the `chrome-mcp:auto-connect` check covers the connection to the user's Chrome: - `ok`: Chrome is reachable. The user still has to accept Chrome's connection prompt when the run starts. diff --git a/skills/reverse-api-engineer/references/setup.md b/skills/reverse-api-engineer/references/setup.md index 27b76c58..d9c52ecc 100644 --- a/skills/reverse-api-engineer/references/setup.md +++ b/skills/reverse-api-engineer/references/setup.md @@ -17,8 +17,9 @@ rae calls a model through the SDK selected in its config: | `opencode` | An OpenCode server rae starts or reuses. Defaults to a free model; can route to local models through Ollama | | `copilot` | GitHub Copilot credentials (`GITHUB_COPILOT_TOKEN`) | | `cursor` | `CURSOR_API_KEY` | +| `codex` | Existing local Codex CLI login; Codex Desktop's bundled CLI is preferred when present, then `codex` on `PATH` | -`agent --dry-run` reports which of these it can see. A missing env var is a warning, not an error, because some SDKs find credentials on their own. +`agent --dry-run` reports which of these it can see. A missing env var is a warning, not an error, because some SDKs find credentials on their own. Codex defaults to `gpt-6-luna` unless `codex_model` or `--model` overrides it. ## Settings @@ -28,8 +29,9 @@ Settings live in `~/.reverse-api/config.json`. Only two have per-run flags on `a |---|---| | `agent_provider` | `chrome-mcp` (the user's real Chrome, no HAR; default for new configs in recent versions), `auto` (Playwright MCP, records a HAR; the default in older versions), `agent-browser` (Vercel agent-browser CLI, records a HAR) | | `output_language` | `python`, `javascript`, `typescript`, `go`, `java`, `csharp`, `php`, `ruby`, `c`, `powershell` | -| `sdk` | `claude`, `opencode`, `copilot`, `cursor` | +| `sdk` | `claude`, `opencode`, `copilot`, `cursor`, `codex` | | `claude_code_model` | Model for the `claude` SDK; `agent -m` overrides it per run | +| `codex_model` | Optional model for the `codex` SDK; defaults to `gpt-6-luna` | | `output_dir` | Where runs are stored; `null` means `~/.reverse-api/runs` | The user can also change them interactively by running `reverse-api-engineer` and typing `/settings`. diff --git a/src/reverse_api/cli.py b/src/reverse_api/cli.py index 174098cc..6c4f3c9d 100644 --- a/src/reverse_api/cli.py +++ b/src/reverse_api/cli.py @@ -130,6 +130,8 @@ def default_model_for_configured_sdk(sdk: str | None = None) -> str: return str(config_manager.get("copilot_model") or "gpt-5") if s == "cursor": return str(config_manager.get("cursor_model") or "composer-2.5") + if s == "codex": + return str(config_manager.get("codex_model") or "gpt-6-luna") return str(config_manager.get("claude_code_model") or "claude-sonnet-4-6") @@ -453,11 +455,26 @@ def _build_dry_run_payload( "copilot": "GITHUB_COPILOT_TOKEN", "cursor": "CURSOR_API_KEY", }.get(sdk) - if sdk_env_var is None: + if sdk == "codex": + from .codex_engineer import codex_login_status + + codex_ok, codex_message = codex_login_status() + checks.append({ + "name": "sdk:codex", + "status": "ok" if codex_ok else "error", + "message": codex_message, + }) + if agent_provider != "auto": + checks.append({ + "name": "sdk:codex:browser", + "status": "error", + "message": "Codex agent capture requires agent_provider='auto' to save a Playwright HAR", + }) + elif sdk_env_var is None: checks.append({ "name": "sdk", "status": "error", - "message": f"unknown sdk {sdk!r}; expected 'claude', 'opencode', 'copilot', or 'cursor'", + "message": f"unknown sdk {sdk!r}; expected 'claude', 'opencode', 'copilot', 'cursor', or 'codex'", }) elif not os.environ.get(sdk_env_var): checks.append({ @@ -582,14 +599,16 @@ def _build_dry_run_payload( "opencode": "opencode_model", "copilot": "copilot_model", "cursor": "cursor_model", + "codex": "codex_model", }.get(sdk, "claude_code_model") sdk_default_model = { "claude": "claude-sonnet-4-6", "opencode": "claude-opus-4-6", "copilot": "gpt-5", "cursor": "composer-2.5", + "codex": "gpt-6-luna", }.get(sdk, "claude-sonnet-4-6") - resolved_model = model or config_manager.get(sdk_model_key, sdk_default_model) + resolved_model = model or config_manager.get(sdk_model_key) or sdk_default_model return { "schema_version": AGENT_JSON_SCHEMA_VERSION, @@ -855,6 +874,7 @@ def _handle_settings_action(mode_color: str = THEME_PRIMARY) -> bool: # noqa: C choices = [ Choice(title="Agent Provider", value="agent_provider"), Choice(title="Claude Code Model", value="claude_code_model"), + Choice(title="Codex Model", value="codex_model"), Choice(title="Copilot Model", value="copilot_model"), Choice(title="Cursor Model", value="cursor_model"), Choice(title="Cursor Web Search", value="cursor_web_search"), @@ -906,6 +926,7 @@ def _handle_settings_action(mode_color: str = THEME_PRIMARY) -> bool: # noqa: C elif action == "sdk": sdk_choices = [ Choice(title="claude", value="claude"), + Choice(title="codex", value="codex"), Choice(title="copilot", value="copilot"), Choice(title="cursor", value="cursor"), Choice(title="opencode", value="opencode"), @@ -927,6 +948,17 @@ def _handle_settings_action(mode_color: str = THEME_PRIMARY) -> bool: # noqa: C config_manager.set("sdk", sdk) console.print(f" [dim]updated[/dim] sdk: {sdk}\n") + elif action == "codex_model": + current = str(config_manager.get("codex_model") or "") + model = questionary.text( + "Codex model (blank uses GPT-6 Luna)", + default=current, + ).ask() + if model is not None: + selected = model.strip() or None + config_manager.set("codex_model", selected) + console.print(f" [dim]updated[/dim] codex model: {selected or 'gpt-6-luna'}\n") + elif action == "output_language": lang_choices = [ Choice(title="python", value="python"), @@ -1524,7 +1556,7 @@ def _session_browser_note(agent_provider: str, headless: bool) -> str | None: return None -def run_auto_capture( +def run_auto_capture( # noqa: C901 - SDK/provider/session lifecycle dispatch prompt: str | None = None, url: str | None = None, model: str | None = None, @@ -1619,16 +1651,32 @@ def run_auto_capture( url=url, model=model, mode=mode_label, + sdk=sdk, paths={"har_dir": str(get_har_dir(run_id, output_dir))}, ) try: output_language = config_manager.get("output_language", "python") from .auto_engineer import ClaudeAutoEngineer, CopilotAutoEngineer, OpenCodeAutoEngineer + from .codex_engineer import CodexAutoEngineer from .cursor_engineer import CursorAutoEngineer - engineer: ClaudeAutoEngineer | CopilotAutoEngineer | OpenCodeAutoEngineer | CursorAutoEngineer - if sdk == "opencode": + engineer: ClaudeAutoEngineer | CopilotAutoEngineer | OpenCodeAutoEngineer | CursorAutoEngineer | CodexAutoEngineer + if sdk == "codex": + engineer = CodexAutoEngineer( + run_id=run_id, + prompt=prompt, + codex_model=model or config_manager.get("codex_model", "default"), + output_dir=output_dir, + agent_provider=agent_provider, + enable_sync=config_manager.get("real_time_sync", False), + sdk=sdk, + output_language=output_language, + interactive=interactive, + headless=headless, + start_url=url, + ) + elif sdk == "opencode": from .auto_engineer import OpenCodeAutoEngineer engineer = OpenCodeAutoEngineer( @@ -1704,7 +1752,10 @@ def run_auto_capture( if resume: core.resume_session, core.cost_before = _resume_point(resume) # A new browser stretch records its own HAR instead of replacing the run's capture. - engineer.mcp_run_id = f"{run_id}-{int(time.time())}" + if hasattr(engineer, "mcp_run_id"): + engineer.mcp_run_id = f"{run_id}-{int(time.time())}" + if sdk == "codex": + engineer.har_path = get_har_dir(engineer.mcp_run_id, output_dir) / "recording.har" if json_event_sink is not None: from .json_stream import attach_json_stream_to_engineer @@ -1755,6 +1806,15 @@ def run_auto_capture( "mode": mode_label, "script_path": (result or {}).get("script_path"), "usage": (result or {}).get("usage", {}), + **( + { + "har_path": (result or {}).get("har_path"), + "har_entries": (result or {}).get("har_entries", 0), + "mcp_tool_calls": (result or {}).get("mcp_tool_calls", []), + } + if sdk == "codex" + else {} + ), **( {"error": "interrupted"} if interrupted @@ -2032,6 +2092,7 @@ def run_engineer( "opencode": resume_model or config_manager.get("opencode_model", DEFAULT_OPENCODE_MODEL), "copilot": resume_model or config_manager.get("copilot_model", "gpt-5"), "cursor": model or config_manager.get("cursor_model", "composer-2.5"), + "codex": resume_model or model or config_manager.get("codex_model") or "gpt-6-luna", }.get(sdk) or model or config_manager.get("claude_code_model", "claude-sonnet-4-6") on_session = _session_recorder(run_id, "engineer", sdk, session_model, output_dir, output_mode) @@ -2101,6 +2162,26 @@ def run_engineer( cost_before=cost_before, on_session=on_session, ) + elif sdk == "codex": + result = run_reverse_engineering( + run_id=run_id, + har_path=har_path, + prompt=prompt, + model=session_model, + codex_model=session_model, + output_dir=output_dir, + sdk=sdk, + enable_sync=enable_sync, + additional_instructions=additional_instructions, + is_fresh=is_fresh, + output_language=output_language, + output_mode=output_mode, + interactive=interactive, + json_event_sink=json_event_sink, + resume_session=resume_session, + cost_before=cost_before, + on_session=on_session, + ) else: result = run_reverse_engineering( run_id=run_id, @@ -2456,8 +2537,12 @@ def _show_error_payload(message: str, rid: str | None = None) -> dict: usage = run.get("usage", {}) details["input_tokens"] = usage.get("input_tokens", 0) details["output_tokens"] = usage.get("output_tokens", 0) - details["cache_creation_input_tokens"] = usage.get("cache_creation_input_tokens", 0) - details["cache_read_input_tokens"] = usage.get("cache_read_input_tokens", 0) + details["cache_creation_input_tokens"] = usage.get( + "cache_creation_input_tokens", usage.get("cache_write_tokens", usage.get("cache_creation_tokens", 0)) + ) + details["cache_read_input_tokens"] = usage.get( + "cache_read_input_tokens", usage.get("cache_read_tokens", 0) + ) # Artifact paths with existence har_dir = base_dir / "har" / rid diff --git a/src/reverse_api/codex_engineer.py b/src/reverse_api/codex_engineer.py new file mode 100644 index 00000000..b9c72dab --- /dev/null +++ b/src/reverse_api/codex_engineer.py @@ -0,0 +1,515 @@ +"""Codex CLI adapter for reverse-api-engineer. + +Codex runs as a local codex exec --json subprocess. MCP configuration is +passed as per-invocation overrides, so RAE does not edit the user's Codex +configuration file. +""" + +from __future__ import annotations + +import asyncio +import json +import os +import shutil +import subprocess +import sys +from pathlib import Path +from typing import Any + +from .base_engineer import BaseEngineer +from .utils import get_har_dir + +DEFAULT_CODEX_MODEL = "gpt-6-luna" +RAE_PLAYWRIGHT_MCP_PACKAGE = "rae-playwright-mcp@0.0.9" + + +def resolve_codex_command() -> list[str] | None: + """Return a directly executable Codex CLI command, including on Windows. + + npm installs a codex.cmd shim on Windows. Launching its JavaScript + entrypoint through Node avoids passing model prompts through cmd.exe. + """ + if sys.platform == "win32": + local_app_data = os.environ.get("LOCALAPPDATA") + if local_app_data: + app_bin_dir = Path(local_app_data) / "OpenAI" / "Codex" / "bin" + try: + app_clis = sorted( + app_bin_dir.glob("*/codex.exe"), + key=lambda path: path.stat().st_mtime, + reverse=True, + ) + except OSError: + app_clis = [] + if app_clis: + return [str(app_clis[0])] + + executable = shutil.which("codex") + if not executable: + return None + + if sys.platform == "win32" and Path(executable).suffix.lower() in {".cmd", ".bat"}: + node = shutil.which("node") + cli_js = Path(executable).parent / "node_modules" / "@openai" / "codex" / "bin" / "codex.js" + if node and cli_js.is_file(): + return [node, str(cli_js)] + + return [executable] + + +def _is_codex_desktop_command(command: list[str]) -> bool: + """Whether this command is the app-bundled CLI, which supports approval review.""" + if sys.platform != "win32" or not command or not os.environ.get("LOCALAPPDATA"): + return False + root = (Path(os.environ["LOCALAPPDATA"]) / "OpenAI" / "Codex" / "bin").resolve() + try: + Path(command[0]).resolve().relative_to(root) + return True + except (OSError, ValueError): + return False + + +def codex_login_status() -> tuple[bool, str]: + """Check CLI authentication without returning potentially identifying output.""" + command = resolve_codex_command() + if not command: + return False, "Codex CLI not found in PATH" + try: + result = subprocess.run( + [*command, "login", "status"], + capture_output=True, + timeout=15, + check=False, + env=os.environ.copy(), + ) + except (OSError, subprocess.TimeoutExpired) as exc: + return False, f"could not check Codex CLI login ({type(exc).__name__})" + if result.returncode != 0: + return False, "Codex CLI is not logged in; run codex login first" + return True, "Codex CLI authenticated" + + +def _toml_string(value: str) -> str: + """JSON basic strings use the same escapes needed by TOML basic strings.""" + return json.dumps(value, ensure_ascii=False) + + +def _toml_array(values: list[str]) -> str: + return "[" + ", ".join(_toml_string(value) for value in values) + "]" + + +class CodexEngineer(BaseEngineer): + """Generate an API client or OpenAPI spec with the authenticated Codex CLI.""" + + def __init__(self, *args: Any, codex_model: str | None = None, **kwargs: Any) -> None: + configured_model = codex_model if codex_model not in (None, "") else kwargs.get("model") + if not configured_model or str(configured_model).lower() == "default": + configured_model = DEFAULT_CODEX_MODEL + kwargs["model"] = configured_model + kwargs["sdk"] = "codex" + super().__init__(*args, **kwargs) + self.codex_model = configured_model + self._active_codex_tools: dict[str, str] = {} + self._mcp_tool_calls: list[str] = [] + self._turn_error: str | None = None + + def _workspace_cwd(self) -> Path: + """Keep Codex's workspace-write sandbox inside this RAE run directory.""" + return self.scripts_dir.parent.parent.resolve() + + def _codex_mcp_overrides(self, server_name: str, command: str, args: list[str], env: dict[str, str] | None = None) -> list[str]: + """Build isolated Codex CLI overrides for one stdio MCP server.""" + prefix = f"mcp_servers.{server_name}" + overrides = [ + f"{prefix}.command={_toml_string(command)}", + f"{prefix}.args={_toml_array(args)}", + f"{prefix}.required=true", + ] + if env: + toml_env = "{" + ", ".join(f"{key} = {_toml_string(value)}" for key, value in sorted(env.items())) + "}" + overrides.append(f"{prefix}.env={toml_env}") + return [part for override in overrides for part in ("--config", override)] + + def _build_exec_args(self, prompt: str, mcp_overrides: list[str] | None = None) -> list[str]: + command = resolve_codex_command() + if not command: + raise RuntimeError("Codex CLI not found in PATH") + + args = [*command, "exec"] + if self.resume_session: + args.extend(["resume", self.resume_session]) + args.extend(["--json", "--ignore-user-config"]) + if _is_codex_desktop_command(command): + # This CLI mode routes requests through Codex's automatic reviewer + # and includes the workspace-write sandbox; it replaces --sandbox. + args.append("--approve-for-me") + else: + args.extend(["--sandbox", "workspace-write"]) + args.extend(["--cd", str(self._workspace_cwd()), "--skip-git-repo-check"]) + if self.codex_model.strip().lower() not in {"", "default"}: + args.extend(["--model", self.codex_model]) + if mcp_overrides: + args.extend(mcp_overrides) + args.append(prompt) + return args + + @staticmethod + def _item_text(item: dict[str, Any]) -> str: + text = item.get("text") + if isinstance(text, str): + return text + content = item.get("content") + if isinstance(content, list): + parts = [str(part.get("text")) for part in content if isinstance(part, dict) and isinstance(part.get("text"), str)] + return "\n".join(parts) + return "" + + @staticmethod + def _tool_result_text(item: dict[str, Any]) -> str | None: + value = item.get("result", item.get("output")) + if value is None: + for field in ("aggregated_output", "stdout", "stderr", "content"): + value = item.get(field) + if value is not None: + break + if value is None: + return None + if isinstance(value, str): + return value + try: + return json.dumps(value, ensure_ascii=False) + except (TypeError, ValueError): + return str(value) + + def _handle_codex_event(self, event: dict[str, Any]) -> None: + event_type = str(event.get("type") or "") + if event_type == "thread.started": + self._record_session(event.get("thread_id")) + return + + if event_type in {"turn.failed", "error"}: + error = event.get("error") or event.get("message") or "Codex turn failed" + self._turn_error = str(error) + return + + if event_type == "turn.completed": + usage = event.get("usage") + if isinstance(usage, dict): + for source, target in ( + ("input_tokens", "input_tokens"), + ("cached_input_tokens", "cache_read_tokens"), + ("output_tokens", "output_tokens"), + ): + value = usage.get(source) + if isinstance(value, (int, float)): + self.usage_metadata[target] = self.usage_metadata.get(target, 0) + int(value) + self._report_usage() + return + + if event_type not in {"item.started", "item.updated", "item.completed"}: + return + item = event.get("item") + if not isinstance(item, dict): + return + + item_type = str(item.get("type") or "") + item_id = str(item.get("id") or item.get("item_id") or "") + if item_type == "agent_message" and event_type == "item.completed": + text = self._item_text(item) + if text: + self.ui.thinking_block(text) + self.message_store.save_thinking(text) + return + + if item_type == "mcp_tool_call": + server = str(item.get("server") or item.get("server_name") or "mcp") + tool = str(item.get("tool") or item.get("name") or "tool") + tool_name = f"mcp__{server}__{tool}" + if server.lower().startswith("playwright") and ( + event_type == "item.started" or item_id not in self._active_codex_tools + ): + self._mcp_tool_calls.append(tool_name) + if event_type == "item.started" and item_id and item_id not in self._active_codex_tools: + tool_input = item.get("arguments") or item.get("input") or {} + if not isinstance(tool_input, dict): + tool_input = {"arguments": tool_input} + self._active_codex_tools[item_id] = tool_name + self.ui.tool_start(tool_name, tool_input, call_id=item_id or None) + self.message_store.save_tool_start(tool_name, tool_input) + elif event_type == "item.completed": + if item_id in self._active_codex_tools: + tool_name = self._active_codex_tools.pop(item_id) + is_error = bool(item.get("is_error") or item.get("status") == "error") + output = self._tool_result_text(item) + self.ui.tool_result(tool_name, is_error, output, call_id=item_id or None) + self.message_store.save_tool_result(tool_name, is_error, output) + return + + if item_type == "command_execution": + command = str(item.get("command") or "") + if event_type == "item.started" and item_id and item_id not in self._active_codex_tools: + self._active_codex_tools[item_id] = "Bash" + self.ui.tool_start("Bash", {"command": command}, call_id=item_id or None) + self.message_store.save_tool_start("Bash", {"command": command}) + elif event_type == "item.completed" and item_id in self._active_codex_tools: + tool_name = self._active_codex_tools.pop(item_id) + output = self._tool_result_text(item) + is_error = bool(item.get("status") in {"failed", "declined", "error"} or item.get("exit_code") not in (None, 0)) + self.ui.tool_result(tool_name, is_error, output, call_id=item_id or None) + self.message_store.save_tool_result(tool_name, is_error, output) + + async def _run_codex( # noqa: C901 - stream parsing, process teardown, output validation + self, prompt: str, mcp_overrides: list[str] | None = None, mode: str = "engineer" + ) -> dict[str, Any] | None: + try: + argv = self._build_exec_args(prompt, mcp_overrides) + except Exception as exc: + message = str(exc) + self.ui.error(message) + self.message_store.save_error(message) + return None + + self._turn_error = None + self._active_codex_tools.clear() + self._mcp_tool_calls.clear() + self.ui.header(self.run_id, self.prompt, self.codex_model, "codex", mode=mode) + self.ui.start_analysis() + + try: + proc = await asyncio.create_subprocess_exec( + *argv, + stdin=asyncio.subprocess.DEVNULL, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.PIPE, + cwd=str(self._workspace_cwd()), + env=os.environ.copy(), + limit=32 * 1024 * 1024, + ) + except OSError as exc: + message = f"Failed to start Codex CLI: {type(exc).__name__}: {exc}" + self.ui.error(message) + self.message_store.save_error(message) + return None + + stderr_tail = bytearray() + + async def pump_stderr() -> None: + assert proc.stderr is not None + while chunk := await proc.stderr.read(65536): + stderr_tail.extend(chunk) + if len(stderr_tail) > 32768: + del stderr_tail[:-32768] + + stderr_task = asyncio.create_task(pump_stderr()) + assert proc.stdout is not None + try: + while True: + try: + line = await asyncio.wait_for(proc.stdout.readline(), timeout=900.0) + except TimeoutError: + self._turn_error = "Codex CLI produced no output for 15 minutes" + break + if not line: + break + try: + event = json.loads(line.decode("utf-8", errors="replace")) + except json.JSONDecodeError: + continue + if isinstance(event, dict): + self._handle_codex_event(event) + + try: + await asyncio.wait_for(proc.wait(), timeout=30.0) + except TimeoutError: + proc.terminate() + await proc.wait() + await asyncio.wait_for(stderr_task, timeout=10.0) + except asyncio.CancelledError: + if proc.returncode is None: + proc.terminate() + try: + await asyncio.wait_for(proc.wait(), timeout=10.0) + except TimeoutError: + proc.kill() + await proc.wait() + stderr_task.cancel() + raise + except Exception as exc: + if self._turn_error is None: + self._turn_error = f"Codex process error: {type(exc).__name__}: {exc}" + if proc.returncode is None: + proc.terminate() + try: + await asyncio.wait_for(proc.wait(), timeout=10.0) + except TimeoutError: + proc.kill() + await proc.wait() + finally: + if not stderr_task.done(): + stderr_task.cancel() + try: + await stderr_task + except asyncio.CancelledError: + pass + + if proc.returncode not in (None, 0) and self._turn_error is None: + stderr = stderr_tail.decode("utf-8", errors="replace").strip() + self._turn_error = stderr[-2000:] or f"Codex CLI exited with code {proc.returncode}" + if self._turn_error: + self.ui.error(self._turn_error) + self.message_store.save_error(self._turn_error) + return None + + if getattr(self, "_require_browser_capture", False): + validation_error = self._validate_browser_capture() + if validation_error: + self.ui.error(validation_error) + self.message_store.save_error(validation_error) + return None + + client_path = self.scripts_dir / self._get_client_filename() + if self.output_mode == "docs": + client_path = self.scripts_dir / "openapi.yaml" + if not client_path.exists(): + client_path = self.scripts_dir / "openapi.json" + if not client_path.is_file(): + message = f"Codex completed without creating the expected output file: {client_path}" + self.ui.error(message) + self.message_store.save_error(message) + return None + + script_path = str(client_path) + local_path = str(self.local_scripts_dir / client_path.name) if self.local_scripts_dir else None + self.ui.success(script_path, local_path) + result = { + "script_path": script_path, + "usage": self.usage_metadata, + "session_id": self.sdk_session_id, + "mcp_tool_calls": list(self._mcp_tool_calls), + } + if getattr(self, "_require_browser_capture", False): + result["har_path"] = str(self.har_path) + result["har_entries"] = self._har_entry_count + self.message_store.save_result(result) + return result + + def _validate_browser_capture(self) -> str | None: + if not self._mcp_tool_calls: + return "Codex completed without calling the required Playwright browser MCP tools" + if not self.har_path.is_file(): + return f"Playwright MCP did not write the expected HAR: {self.har_path}" + try: + har = json.loads(self.har_path.read_text(encoding="utf-8")) + except (OSError, UnicodeDecodeError, json.JSONDecodeError) as exc: + return f"Playwright MCP wrote an unreadable HAR ({type(exc).__name__})" + entries = har.get("log", {}).get("entries", []) if isinstance(har, dict) else [] + if not isinstance(entries, list) or not entries: + return "Playwright MCP wrote a HAR, but it contains no network entries" + self._har_entry_count = len(entries) + return None + + async def analyze_and_generate(self) -> dict[str, Any] | None: + system_prompt, user_message = self._build_prompts() + opening = await self._opening_message(user_message) + if not opening: + return {} + prompt = opening if self.resume_session else f"{system_prompt}\n\n{user_message}" + return await self._run_codex(prompt) + + +class CodexAutoEngineer(CodexEngineer): + """Capture traffic with browser MCP and generate a client in one Codex run.""" + + def __init__(self, run_id: str, prompt: str, output_dir: str | None = None, agent_provider: str = "auto", **kwargs: Any) -> None: + if agent_provider != "auto": + raise ValueError("Codex agent capture currently requires agent_provider='auto' (Playwright MCP with HAR)") + self.mcp_run_id = run_id + self.agent_provider = agent_provider + self.headless = bool(kwargs.pop("headless", False)) + self.start_url = kwargs.pop("start_url", None) + har_dir = get_har_dir(run_id, output_dir) + configured_model = kwargs.pop("codex_model", None) + if configured_model in (None, ""): + configured_model = kwargs.get("model") + if not configured_model or str(configured_model).lower() == "default": + configured_model = DEFAULT_CODEX_MODEL + kwargs["model"] = configured_model + kwargs["sdk"] = "codex" + super().__init__(run_id=run_id, har_path=har_dir / "recording.har", prompt=prompt, output_dir=output_dir, **kwargs) + self._require_browser_capture = True + self._har_entry_count = 0 + + def _get_active_prompts(self) -> tuple[str, str]: + from .prompts import load + + language_name = self._get_language_name() + client_filename = self._get_client_filename() + output_files = self._get_auto_output_files(language_name, client_filename) + system_prompt = load( + "auto/system", + browser_tool_label="Playwright MCP", + language_name=language_name, + codegen_instructions=self._get_codegen_instructions(), + output_files=output_files, + ) + start_url_block = "\n" + load("auto/start_url", start_url=self.start_url) if self.start_url else "" + user_message = load( + "auto/user_playwright", + prompt=self.prompt, + scripts_dir=str(self.scripts_dir), + har_path=str(self.har_path), + start_url_block=start_url_block, + ) + user_message += ( + "\n\nDo not install packages or execute the generated client during this capture. " + "Generate it from the captured traffic; it will be inspected and run separately. " + "Avoid printing the full page body or full network log into the conversation. After identifying " + "the rankings GET request, parse its response in the browser and return only the first 15 rows " + "with the fields needed by the client. Do not assign a rank to unidentified zero-activity aggregate " + "rows; report if one is omitted, and fail closed on malformed nonzero rows. Use the native " + "file-edit/apply-patch capability to write " + "the two requested files in the output directory; do not use Bash to inspect unrelated local files." + ) + return system_prompt, user_message + + def _playwright_mcp_overrides(self) -> list[str]: + command_line = " ".join( + [ + "npx -y", + RAE_PLAYWRIGHT_MCP_PACKAGE, + "--run-id", + self.mcp_run_id, + "--headless", + "--browser chrome", + ] + ) + if os.name == "nt": + command, args = shutil.which("cmd.exe") or "cmd.exe", ["/d", "/s", "/c", command_line] + else: + command, args = ( + shutil.which("npx") or "npx", + ["-y", RAE_PLAYWRIGHT_MCP_PACKAGE, "--run-id", self.mcp_run_id, "--headless", "--browser", "chrome"], + ) + # HAR is written under /har//recording.har. + har_base = str(self.har_path.parent.parent) + return self._codex_mcp_overrides( + "playwright", + command, + args, + env={"PLAYWRIGHT_MCP_SAVE_HAR_PATH": har_base}, + ) + + def _validate_browser_capture(self) -> str | None: + error = super()._validate_browser_capture() + if error: + return error + self._emit_json_event({"event": "browser_capture_verified", "har_path": str(self.har_path), "har_entries": self._har_entry_count}) + return None + + async def analyze_and_generate(self) -> dict[str, Any] | None: + system_prompt, user_message = self._get_active_prompts() + opening = await self._opening_message(user_message) + if not opening: + return {} + prompt = opening if self.resume_session else f"{system_prompt}\n\n{user_message}" + return await self._run_codex(prompt, self._playwright_mcp_overrides(), mode="agent") diff --git a/src/reverse_api/config.py b/src/reverse_api/config.py index ac420ff3..63e4b3b1 100644 --- a/src/reverse_api/config.py +++ b/src/reverse_api/config.py @@ -27,6 +27,7 @@ # Optional override: list of setting source ids, e.g. ["project","user","all"]. None uses cursor_web_search. "cursor_setting_sources": None, "copilot_model": "gpt-5", # Model for Copilot SDK sessions + "codex_model": None, # None uses the default model selected by the local Codex CLI "opencode_model": DEFAULT_OPENCODE_MODEL, "opencode_provider": DEFAULT_OPENCODE_PROVIDER, "opencode_auto_start": True, @@ -37,7 +38,7 @@ "output_dir": None, # None means use ~/.reverse-api/runs "output_language": "python", # "python", "javascript", "typescript", "go", "java", "csharp", "php", "ruby", "c", or "powershell" "real_time_sync": True, # Enable real-time file sync during engineering - "sdk": "claude", # "claude", "opencode", "copilot", or "cursor" + "sdk": "claude", # "claude", "opencode", "copilot", "cursor", or "codex" } diff --git a/src/reverse_api/engineer.py b/src/reverse_api/engineer.py index 7c3b55dc..96baeb0a 100644 --- a/src/reverse_api/engineer.py +++ b/src/reverse_api/engineer.py @@ -411,6 +411,7 @@ def run_reverse_engineering( opencode_model: str | None = None, copilot_model: str | None = None, cursor_model: str | None = None, + codex_model: str | None = None, cursor_web_search: bool = True, cursor_setting_sources: list[str] | None = None, enable_sync: bool = False, @@ -428,11 +429,12 @@ def run_reverse_engineering( resume_session, cost_before and on_session reopen and record the SDK session, see BaseEngineer. Args: - sdk: "claude", "opencode", "copilot", or "cursor" - determines which SDK to use + sdk: "claude", "opencode", "copilot", "cursor", or "codex" - determines which SDK to use opencode_provider: Provider ID for OpenCode (e.g., "anthropic") opencode_model: Model ID for OpenCode (e.g., "claude-sonnet-4-6") copilot_model: Model ID for Copilot (e.g., "gpt-5") cursor_model: Model id for Cursor SDK (e.g., "composer-2.5") + codex_model: Optional model id for the local Codex CLI; None uses its configured default. cursor_web_search: When True, load extra Cursor setting layers so WebFetch/WebSearch and plugins apply. cursor_setting_sources: Optional explicit list (overrides cursor_web_search), e.g. ["project","user","all"]. enable_sync: Enable real-time file syncing during engineering @@ -501,6 +503,25 @@ def run_reverse_engineering( copilot_model=copilot_model, interactive=interactive, ) + elif sdk == "codex": + from .codex_engineer import CodexEngineer + + engineer = CodexEngineer( + run_id=run_id, + har_path=har_path, + prompt=prompt, + model=model, + codex_model=codex_model, + additional_instructions=additional_instructions, + output_dir=output_dir, + verbose=verbose, + enable_sync=enable_sync, + sdk=sdk, + is_fresh=is_fresh, + output_language=output_language, + output_mode=output_mode, + interactive=interactive, + ) else: engineer = ClaudeEngineer( run_id=run_id, diff --git a/src/reverse_api/prompts/auto/system.md b/src/reverse_api/prompts/auto/system.md index fe7e0284..9e4bc70f 100644 --- a/src/reverse_api/prompts/auto/system.md +++ b/src/reverse_api/prompts/auto/system.md @@ -18,10 +18,11 @@ For this provider your integration surface is: - Narrate your actions as you browse - Capture all necessary API calls before generating code - Test your generated code to verify it works +- For ranked lists, omit unidentified rows only when their ranking metric and activity counters are all zero, and report the omission; fail on unidentified nonzero rows. - Screenshots must be under 1MB — prefer element-specific screenshots or use snapshots for structure ## Output files {output_files} -Confirm the files were created and briefly summarize: APIs discovered, auth method, whether it works, any caveats. \ No newline at end of file +Confirm the files were created and briefly summarize: APIs discovered, auth method, whether it works, any caveats. diff --git a/tests/test_cli_agent_json.py b/tests/test_cli_agent_json.py index 44ae1fd1..99701ed9 100644 --- a/tests/test_cli_agent_json.py +++ b/tests/test_cli_agent_json.py @@ -221,6 +221,30 @@ def test_unknown_run_id_emits_error(self): assert "error" in payload assert payload["run_id"] == "doesnotexist" + def test_show_json_maps_codex_usage_aliases(self, tmp_path): + run = { + "run_id": "codex123", + "prompt": "capture a public page", + "mode": "auto", + "model": "gpt-6-luna", + "sdk": "codex", + "usage": {"input_tokens": 100, "output_tokens": 8, "cache_read_tokens": 64}, + "paths": {}, + } + with ( + patch("reverse_api.cli.session_manager") as sm, + patch("reverse_api.cli.config_manager") as cm, + patch("reverse_api.cli.get_base_output_dir", return_value=tmp_path), + ): + sm.get_run.return_value = run + cm.get.return_value = None + result = CliRunner().invoke(show_run, ["codex123", "--json"]) + + assert result.exit_code == 0, result.output + payload = json.loads(result.stdout.strip()) + assert payload["sdk"] == "codex" + assert payload["cache_read_input_tokens"] == 64 + # --------------------------------------------------------------------------- # `run` command: --no-interactive and --auto-install behavior diff --git a/tests/test_cli_followups.py b/tests/test_cli_followups.py index 79e32d69..ea183cf7 100644 --- a/tests/test_cli_followups.py +++ b/tests/test_cli_followups.py @@ -215,6 +215,49 @@ def test_no_interactive_flag_accepted(self): assert result.exit_code == 0, result.output +def test_auto_capture_records_selected_sdk_in_history(tmp_path): + from reverse_api.cli import run_auto_capture + + class FakeCodexAutoEngineer: + def __init__(self, **_kwargs): + self.cost_before = None + + def start_sync(self): + pass + + def stop_sync(self): + pass + + async def analyze_and_generate(self): + return {"script_path": str(tmp_path / "api_client.py"), "usage": {}} + + config = { + "sdk": "codex", + "codex_model": "gpt-6-luna", + "output_language": "python", + "real_time_sync": False, + } + with ( + patch("reverse_api.cli.config_manager.get", side_effect=lambda key, default=None: config.get(key, default)), + patch("reverse_api.cli.session_manager") as sessions, + patch("reverse_api.cli.generate_run_id", return_value="codex-run"), + patch("reverse_api.cli.get_timestamp", return_value="2026-11-02T00:00:00"), + patch("reverse_api.cli.get_har_dir", return_value=tmp_path / "har" / "codex-run"), + patch("reverse_api.codex_engineer.CodexAutoEngineer", side_effect=FakeCodexAutoEngineer), + patch("reverse_api.cli._keep_session_cost"), + ): + result = run_auto_capture( + prompt="capture a public page", + url="https://example.com", + agent_provider="auto", + interactive=False, + ) + + assert result["run_id"] == "codex-run" + sessions.add_run.assert_called_once() + assert sessions.add_run.call_args.kwargs["sdk"] == "codex" + + class TestEngineerJsonMissingRunId: """`engineer --json` without RUN_ID must emit a JSON misuse payload, not Click's plain-text 'missing argument' error. @@ -623,6 +666,49 @@ def test_dry_run_cursor_model_resolution(self, tmp_path): assert payload["would_run"]["sdk"] == "cursor" assert payload["would_run"]["model"] == "composer-2-custom" + def test_dry_run_codex_uses_cli_auth_and_default_model(self, tmp_path): + from reverse_api.cli import agent as agent_cmd + + runner = CliRunner() + with ( + patch("reverse_api.cli.config_manager") as cm, + patch("reverse_api.codex_engineer.codex_login_status", return_value=(True, "Codex CLI authenticated")), + ): + cm.get.side_effect = lambda key, default=None: { + "agent_provider": "auto", + "sdk": "codex", + "codex_model": None, + "output_dir": str(tmp_path), + }.get(key, default) + result = runner.invoke(agent_cmd, ["--dry-run", "-p", "capture", "-u", "https://openrouter.ai/rankings"]) + + assert result.exit_code == 0, result.output + payload = json.loads(result.stdout.strip()) + assert payload["status"] == "ok" + assert payload["would_run"]["sdk"] == "codex" + assert payload["would_run"]["model"] == "gpt-6-luna" + assert {"name": "sdk:codex", "status": "ok", "message": "Codex CLI authenticated"} in payload["checks"] + + def test_dry_run_codex_requires_har_preserving_browser_provider(self, tmp_path): + from reverse_api.cli import agent as agent_cmd + + runner = CliRunner() + with ( + patch("reverse_api.cli.config_manager") as cm, + patch("reverse_api.codex_engineer.codex_login_status", return_value=(True, "Codex CLI authenticated")), + ): + cm.get.side_effect = lambda key, default=None: { + "agent_provider": "chrome-mcp", + "sdk": "codex", + "output_dir": str(tmp_path), + }.get(key, default) + result = runner.invoke(agent_cmd, ["--dry-run", "-p", "capture"]) + + assert result.exit_code == 1, result.output + payload = json.loads(result.stdout.strip()) + assert payload["status"] == "error" + assert any(check["name"] == "sdk:codex:browser" and check["status"] == "error" for check in payload["checks"]) + class TestRootHelpMentionsScripted: """Item #6 partial: root --help should advertise scripted features.""" diff --git a/tests/test_codex_engineer.py b/tests/test_codex_engineer.py new file mode 100644 index 00000000..9571dfa8 --- /dev/null +++ b/tests/test_codex_engineer.py @@ -0,0 +1,166 @@ +"""Tests for the Codex CLI and per-run browser MCP adapter.""" + +import json +import os +import tomllib +from unittest.mock import MagicMock, patch + +import pytest + +import reverse_api.codex_engineer as codex_module +from reverse_api.codex_engineer import CodexAutoEngineer, CodexEngineer + + +def make_engineer(tmp_path, **kwargs): + defaults = { + "run_id": "codex-test", + "har_path": tmp_path / "har" / "codex-test" / "recording.har", + "prompt": "fetch public data", + "output_dir": str(tmp_path), + "verbose": False, + "interactive": False, + } + defaults.update(kwargs) + return CodexEngineer(**defaults) + + +def test_exec_args_use_workspace_sandbox_and_explicit_supported_default(tmp_path): + eng = make_engineer(tmp_path) + with patch("reverse_api.codex_engineer.resolve_codex_command", return_value=["node", "codex.js"]): + args = eng._build_exec_args("read this HAR") + + assert args[:2] == ["node", "codex.js"] + assert "--ignore-user-config" in args + assert args[args.index("--sandbox") + 1] == "workspace-write" + assert args[args.index("--cd") + 1] == str((tmp_path / "scripts" / "codex-test").parent.parent.resolve()) + assert args[args.index("--model") + 1] == "gpt-6-luna" + assert args[-1] == "read this HAR" + + +def test_mcp_overrides_are_per_invocation_and_required(tmp_path): + eng = make_engineer(tmp_path) + overrides = eng._codex_mcp_overrides( + "playwright", + "cmd.exe", + ["/d", "/s", "/c", "npx -y rae-playwright-mcp@0.0.9"], + env={"PLAYWRIGHT_MCP_SAVE_HAR_PATH": str(tmp_path / "runs with spaces" / "har")}, + ) + parsed = {} + for index, item in enumerate(overrides): + if item == "--config": + key, value = overrides[index + 1].split("=", 1) + parsed[key.rsplit(".", 1)[-1]] = tomllib.loads(f"{key}={value}")["mcp_servers"]["playwright"][key.rsplit(".", 1)[-1]] + + assert parsed["command"] == "cmd.exe" + assert parsed["args"] == ["/d", "/s", "/c", "npx -y rae-playwright-mcp@0.0.9"] + assert parsed["required"] is True + suffix = "runs with spaces\\har" if os.name == "nt" else "runs with spaces/har" + assert parsed["env"]["PLAYWRIGHT_MCP_SAVE_HAR_PATH"].endswith(suffix) + + +def test_windows_prefers_codex_desktop_cli(tmp_path, monkeypatch): + cli = tmp_path / "OpenAI" / "Codex" / "bin" / "desktop-build" / "codex.exe" + cli.parent.mkdir(parents=True) + cli.touch() + monkeypatch.setattr(codex_module.sys, "platform", "win32") + monkeypatch.setenv("LOCALAPPDATA", str(tmp_path)) + monkeypatch.setattr(codex_module.shutil, "which", lambda _name: "C:/npm/codex.cmd") + + assert codex_module.resolve_codex_command() == [str(cli)] + + +def test_desktop_cli_uses_workspace_scoped_approval_review(tmp_path, monkeypatch): + cli = tmp_path / "OpenAI" / "Codex" / "bin" / "desktop-build" / "codex.exe" + cli.parent.mkdir(parents=True) + cli.touch() + monkeypatch.setattr(codex_module.sys, "platform", "win32") + monkeypatch.setenv("LOCALAPPDATA", str(tmp_path)) + eng = make_engineer(tmp_path) + with patch("reverse_api.codex_engineer.resolve_codex_command", return_value=[str(cli)]): + args = eng._build_exec_args("browse public page") + + assert "--approve-for-me" in args + assert "--sandbox" not in args + + +def test_codex_events_record_session_and_playwright_tool_call(tmp_path): + eng = make_engineer(tmp_path) + eng.ui = MagicMock() + eng.message_store = MagicMock() + eng.on_session = MagicMock() + eng._handle_codex_event({"type": "thread.started", "thread_id": "thread-123"}) + eng._handle_codex_event({ + "type": "item.started", + "item": { + "id": "call-1", + "type": "mcp_tool_call", + "server": "playwright", + "tool": "browser_navigate", + "arguments": {"url": "https://openrouter.ai/rankings"}, + }, + }) + eng._handle_codex_event({ + "type": "item.completed", + "item": { + "id": "call-1", + "type": "mcp_tool_call", + "server": "playwright", + "tool": "browser_navigate", + "result": "Navigated", + }, + }) + + assert eng.sdk_session_id == "thread-123" + eng.on_session.assert_called_once_with("thread-123") + assert eng._mcp_tool_calls == ["mcp__playwright__browser_navigate"] + eng.ui.tool_start.assert_called_once() + eng.ui.tool_result.assert_called_once() + + +def test_auto_engineer_prompt_uses_shared_codegen_without_claude_only_tool(tmp_path): + eng = CodexAutoEngineer( + run_id="codex-auto", + prompt="Capture rankings", + output_dir=str(tmp_path), + agent_provider="auto", + start_url="https://openrouter.ai/rankings", + headless=True, + verbose=False, + interactive=False, + ) + + system_prompt, user_prompt = eng._get_active_prompts() + assert "Playwright MCP" in system_prompt + assert "report_client_verified" not in system_prompt + assert "https://openrouter.ai/rankings" in user_prompt + assert "Do not install packages or execute the generated client" in user_prompt + + +def test_auto_capture_validation_requires_browser_tool_and_nonempty_har(tmp_path): + eng = CodexAutoEngineer( + run_id="codex-auto", + prompt="Capture rankings", + output_dir=str(tmp_path), + agent_provider="auto", + verbose=False, + interactive=False, + ) + eng.har_path.parent.mkdir(parents=True, exist_ok=True) + eng.har_path.write_text(json.dumps({"log": {"entries": [{"request": {"method": "GET"}}]}}), encoding="utf-8") + + assert "without calling" in eng._validate_browser_capture() + eng._mcp_tool_calls.append("mcp__playwright__browser_navigate") + assert eng._validate_browser_capture() is None + assert eng._har_entry_count == 1 + + +def test_auto_capture_requires_har_preserving_provider(tmp_path): + with pytest.raises(ValueError, match="agent_provider='auto'"): + CodexAutoEngineer( + run_id="codex-auto", + prompt="Capture rankings", + output_dir=str(tmp_path), + agent_provider="chrome-mcp", + verbose=False, + interactive=False, + ) diff --git a/tests/test_engineer.py b/tests/test_engineer.py index c605bc83..c8902230 100644 --- a/tests/test_engineer.py +++ b/tests/test_engineer.py @@ -61,6 +61,27 @@ def test_dispatches_to_opencode(self, tmp_path): ) mock_cls.assert_called_once() + def test_dispatches_to_codex(self, tmp_path): + har_path = tmp_path / "test.har" + har_path.touch() + + with patch("reverse_api.codex_engineer.CodexEngineer") as mock_cls: + mock_instance = MagicMock() + mock_cls.return_value = mock_instance + mock_instance.start_sync = MagicMock() + mock_instance.stop_sync = MagicMock() + + with patch("reverse_api.engineer.asyncio.run", return_value={"test": True}): + run_reverse_engineering( + run_id="test123", + har_path=har_path, + prompt="test prompt", + sdk="codex", + codex_model="custom-model", + ) + + assert mock_cls.call_args.kwargs["codex_model"] == "custom-model" + def test_starts_sync(self, tmp_path): """Sync is started before analysis.""" har_path = tmp_path / "test.har" From eefc96d5801615c3d1e01e3bfee650000f116107 Mon Sep 17 00:00:00 2001 From: leonininder <8566513+leonininder@users.noreply.github.com> Date: Tue, 3 Nov 2026 09:28:52 +0800 Subject: [PATCH 2/4] fix: address Codex adapter review regressions --- README.md | 2 +- skills/reverse-api-engineer/SKILL.md | 2 +- .../reverse-api-engineer/references/setup.md | 6 +- src/reverse_api/cli.py | 20 ++- src/reverse_api/codex_engineer.py | 79 +++++++---- src/reverse_api/config.py | 2 +- src/reverse_api/engineer.py | 2 +- src/reverse_api/prompts/auto/system.md | 1 - tests/test_cli_agent_json.py | 21 +++ tests/test_cli_followups.py | 38 +++++- tests/test_cli_settings.py | 29 ++++ tests/test_codex_engineer.py | 129 +++++++++++++++++- 12 files changed, 290 insertions(+), 41 deletions(-) diff --git a/README.md b/README.md index bc92fc49..70221eee 100644 --- a/README.md +++ b/README.md @@ -129,7 +129,7 @@ Settings live in `~/.reverse-api/config.json` and can be edited via `/settings` - **Models**: Sonnet 5.5, Opus 5.5 (most capable), Opus 4.6 and Haiku 5.5 (fastest). Sonnet 4.6 stays the default until you choose another; it and Haiku 4.5 are still accepted in config and by `--model`. For OpenCode see [models.dev](https://models.dev). - **SDK**: `claude` (default), `opencode`, `cursor`, `copilot` (GitHub Copilot), or `codex`. -- **Codex SDK**: `sdk: "codex"` reuses the local Codex CLI login, preferring the Codex Desktop-bundled CLI and falling back to `codex` on `PATH`. `codex_model` is optional; RAE defaults to `gpt-6-luna`. Codex browser capture currently requires `agent_provider: "auto"`, which provides Playwright MCP and a HAR. The adapter supplies per-run MCP configuration and does not edit the global Codex config. +- **Codex SDK**: `sdk: "codex"` reuses the local Codex CLI login. On Windows, it prefers the Codex Desktop-bundled CLI and falls back to `codex` on `PATH`. `codex_model` is optional; RAE defaults to `gpt-6-luna`. Codex browser capture currently requires `agent_provider: "auto"`, which provides Playwright MCP and a HAR. The adapter supplies per-run MCP configuration and does not edit the global Codex config. - **Claude settings isolation**: Claude sessions don't load your Claude Code settings files, `CLAUDE.md`, hooks, plugins, permission rules or MCP servers, so your personal setup doesn't steer the agent. Organization-managed policy still applies, and Claude Code still reads its own state such as `~/.claude.json` and auto memory. If `~/.claude/settings.json` provides your authentication (`apiKeyHelper`, or Bedrock, Vertex or proxy variables under `env`), set `claude_setting_sources` to the layers to load, e.g. `["user"]` or `["user", "project", "local"]`. Each layer you list is loaded in full, including its hooks, plugins, permission rules and MCP servers. - **OpenCode setup**: with `sdk: "opencode"`, RAE reuses an existing server or downloads/starts `opencode-ai@latest` through `npx`; a global OpenCode installation is not required. Fresh configurations default to the free `opencode/big-pickle` model. `/settings` shows a loading spinner, then offers a searchable **OpenCode Provider / Model** picker populated from the server's connected, tool-capable catalog and marks free OpenCode options. Before creating a session, RAE validates the saved pair again and suggests currently available free models when configuration is invalid. Compatible older servers are reused with a version warning. Node.js 20+ is required for automatic startup. Password-protected servers use `OPENCODE_SERVER_PASSWORD` and optional `OPENCODE_SERVER_USERNAME`. Override startup with `OPENCODE_BASE_URL`, `RAE_OPENCODE_PACKAGE`, or `RAE_OPENCODE_AUTO_START=0`. - **Ollama through OpenCode**: choose provider `ollama` in `/settings`; RAE starts an installed Ollama daemon if needed, lists only installed models with tool calling and 64k+ context, and supplies OpenCode's provider config inline. Models are never downloaded silently. Override with `RAE_OLLAMA_BASE_URL` or `RAE_OLLAMA_AUTO_START=0`. diff --git a/skills/reverse-api-engineer/SKILL.md b/skills/reverse-api-engineer/SKILL.md index 6fbe0d6e..22ea7c2f 100644 --- a/skills/reverse-api-engineer/SKILL.md +++ b/skills/reverse-api-engineer/SKILL.md @@ -45,7 +45,7 @@ reverse-api-engineer show --json reverse-api-engineer agent -p "" -u "" --dry-run ``` -This validates the prompt, URL, config and environment without opening a browser. Fix every `error` check before running. A `warn` on the API key is fine when the SDK authenticates another way (for example a logged-in Claude Code or Codex CLI). +This validates the prompt, URL, config and environment without opening a browser. Fix every `error` check before running. A `warn` on the API key is fine when the SDK authenticates another way (for example a logged-in Claude Code). `would_run.agent_provider` in the output is the browser the run will use. `chrome-mcp` means the user's own Chrome with their logins; `auto` and `agent-browser` start a fresh browser without them. When the task needs the user's login and the provider is not `chrome-mcp`, ask before switching it (see [sessions](references/sessions.md)). diff --git a/skills/reverse-api-engineer/references/setup.md b/skills/reverse-api-engineer/references/setup.md index d9c52ecc..832af39b 100644 --- a/skills/reverse-api-engineer/references/setup.md +++ b/skills/reverse-api-engineer/references/setup.md @@ -17,13 +17,13 @@ rae calls a model through the SDK selected in its config: | `opencode` | An OpenCode server rae starts or reuses. Defaults to a free model; can route to local models through Ollama | | `copilot` | GitHub Copilot credentials (`GITHUB_COPILOT_TOKEN`) | | `cursor` | `CURSOR_API_KEY` | -| `codex` | Existing local Codex CLI login; Codex Desktop's bundled CLI is preferred when present, then `codex` on `PATH` | +| `codex` | Existing local Codex CLI login; on Windows, Codex Desktop's bundled CLI is preferred when present, then `codex` on `PATH` | `agent --dry-run` reports which of these it can see. A missing env var is a warning, not an error, because some SDKs find credentials on their own. Codex defaults to `gpt-6-luna` unless `codex_model` or `--model` overrides it. ## Settings -Settings live in `~/.reverse-api/config.json`. Only two have per-run flags on `agent`: `-o/--output-dir` and `-m/--model` (for the `claude` SDK). Change the others by editing that file, and only with the user's agreement since it affects every later run. Keys that matter most: +Settings live in `~/.reverse-api/config.json`. Only two have per-run flags on `agent`: `-o/--output-dir` and `-m/--model` (for the `claude` and `codex` SDKs). Change the others by editing that file, and only with the user's agreement since it affects every later run. Keys that matter most: | Key | Values | |---|---| @@ -37,3 +37,5 @@ Settings live in `~/.reverse-api/config.json`. Only two have per-run flags on `a The user can also change them interactively by running `reverse-api-engineer` and typing `/settings`. Language notes: C needs a POSIX toolchain with libcurl headers. PowerShell needs `pwsh` 7+. + +For Codex, model selection uses codex_model, then the per-run model, then gpt-6-luna. Empty or default selections also resolve to gpt-6-luna; they do not select the Codex CLI configured default. Choosing Codex in /settings also selects the required auto browser provider. diff --git a/src/reverse_api/cli.py b/src/reverse_api/cli.py index 6c4f3c9d..64f4c4db 100644 --- a/src/reverse_api/cli.py +++ b/src/reverse_api/cli.py @@ -120,6 +120,11 @@ def __init__(self) -> None: def get_metavar(self, *args: Any, **kwargs: Any) -> str: return "[" + "|".join(self.current) + "]" + def convert(self, value: Any, param: click.Parameter | None, ctx: click.Context | None) -> Any: + if ctx is not None and ctx.command.name in {"agent", "engineer"} and config_manager.get("sdk", "claude") == "codex": + return click.STRING.convert(value, param, ctx) + return super().convert(value, param, ctx) + def default_model_for_configured_sdk(sdk: str | None = None) -> str: """Return the configured default model id for the given SDK (or current config SDK).""" @@ -669,7 +674,7 @@ def _build_agent_payload( error_kind = result["error_kind"] har_path = None if run_id: - candidate = get_har_dir(run_id, output_dir) / "recording.har" + candidate = Path(result["har_path"]) if result.get("har_path") else 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, @@ -945,7 +950,11 @@ def _handle_settings_action(mode_color: str = THEME_PRIMARY) -> bool: # noqa: C ), ).ask() if sdk and sdk != "back": - config_manager.set("sdk", sdk) + if sdk == "codex": + config_manager.update({"sdk": sdk, "agent_provider": "auto"}) + console.print(" [dim]Codex capture uses a fresh Playwright browser (agent_provider: auto)[/dim]") + else: + config_manager.set("sdk", sdk) console.print(f" [dim]updated[/dim] sdk: {sdk}\n") elif action == "codex_model": @@ -1794,10 +1803,13 @@ def run_auto_capture( # noqa: C901 - SDK/provider/session lifecycle dispatch # Update session with results if result: usage = result.get("usage", {}) + paths = {"script_path": result.get("script_path")} + if result.get("har_path"): + paths.update(har_path=str(result["har_path"]), har_dir=str(Path(result["har_path"]).parent)) session_manager.update_run( run_id=run_id, usage=_run_totals(resume.get("usage") or {}, usage) if resume else usage, - paths={"script_path": result.get("script_path")}, + paths=paths, ) _keep_session_cost(run_id, core.cost_before, usage) @@ -2070,7 +2082,7 @@ def run_engineer( # Detect where it was saved paths = run_data.get("paths", {}) har_dir = Path(paths.get("har_dir", get_har_dir(run_id, None))) - har_path = har_dir / "recording.har" + har_path = Path(paths["har_path"]) if paths.get("har_path") else har_dir / "recording.har" if not har_path.exists() and not resume: if run_data.get("mode") == "chrome-mcp": message = ( diff --git a/src/reverse_api/codex_engineer.py b/src/reverse_api/codex_engineer.py index b9c72dab..3528466a 100644 --- a/src/reverse_api/codex_engineer.py +++ b/src/reverse_api/codex_engineer.py @@ -58,7 +58,7 @@ def resolve_codex_command() -> list[str] | None: def _is_codex_desktop_command(command: list[str]) -> bool: - """Whether this command is the app-bundled CLI, which supports approval review.""" + """Identify the Windows Desktop CLI that supports automatic approval review.""" if sys.platform != "win32" or not command or not os.environ.get("LOCALAPPDATA"): return False root = (Path(os.environ["LOCALAPPDATA"]) / "OpenAI" / "Codex" / "bin").resolve() @@ -108,6 +108,12 @@ def __init__(self, *args: Any, codex_model: str | None = None, **kwargs: Any) -> kwargs["model"] = configured_model kwargs["sdk"] = "codex" super().__init__(*args, **kwargs) + # Prompts and the browser subprocess run inside the output workspace, + # so relative paths from the caller must not be interpreted there again. + self.scripts_dir = self.scripts_dir.resolve() + self.har_path = self.har_path.resolve() + if self.existing_client_path is not None: + self.existing_client_path = self.existing_client_path.resolve() self.codex_model = configured_model self._active_codex_tools: dict[str, str] = {} self._mcp_tool_calls: list[str] = [] @@ -136,13 +142,11 @@ def _build_exec_args(self, prompt: str, mcp_overrides: list[str] | None = None) raise RuntimeError("Codex CLI not found in PATH") args = [*command, "exec"] - if self.resume_session: - args.extend(["resume", self.resume_session]) args.extend(["--json", "--ignore-user-config"]) if _is_codex_desktop_command(command): - # This CLI mode routes requests through Codex's automatic reviewer - # and includes the workspace-write sandbox; it replaces --sandbox. - args.append("--approve-for-me") + # Keep Desktop's approval reviewer; the --sandbox flag conflicts + # with this mode, so select workspace scope through TOML instead. + args.extend(["--approve-for-me", "--config", 'sandbox_mode="workspace-write"']) else: args.extend(["--sandbox", "workspace-write"]) args.extend(["--cd", str(self._workspace_cwd()), "--skip-git-repo-check"]) @@ -150,6 +154,9 @@ def _build_exec_args(self, prompt: str, mcp_overrides: list[str] | None = None) args.extend(["--model", self.codex_model]) if mcp_overrides: args.extend(mcp_overrides) + # Sandbox/cwd options belong to exec, not its resume subcommand. + if self.resume_session: + args.extend(["resume", self.resume_session]) args.append(prompt) return args @@ -195,8 +202,16 @@ def _handle_codex_event(self, event: dict[str, Any]) -> None: if event_type == "turn.completed": usage = event.get("usage") if isinstance(usage, dict): + # Codex input_tokens includes cached input; RAE's buckets are + # disjoint. Codex TokenUsage::non_cached_input uses this same + # subtraction: https://github.com/openai/codex/blob/main/codex-rs/protocol/src/protocol.rs + total_input = usage.get("input_tokens", 0) + cached_input = usage.get("cached_input_tokens", 0) + if isinstance(total_input, (int, float)) and isinstance(cached_input, (int, float)): + self.usage_metadata["input_tokens"] = self.usage_metadata.get("input_tokens", 0) + max( + 0, int(total_input) - int(cached_input) + ) for source, target in ( - ("input_tokens", "input_tokens"), ("cached_input_tokens", "cache_read_tokens"), ("output_tokens", "output_tokens"), ): @@ -271,7 +286,7 @@ async def _run_codex( # noqa: C901 - stream parsing, process teardown, output v self._turn_error = None self._active_codex_tools.clear() - self._mcp_tool_calls.clear() + # Retain capture evidence across code-only follow-up turns. self.ui.header(self.run_id, self.prompt, self.codex_model, "codex", mode=mode) self.ui.start_analysis() @@ -414,7 +429,29 @@ async def analyze_and_generate(self) -> dict[str, Any] | None: if not opening: return {} prompt = opening if self.resume_session else f"{system_prompt}\n\n{user_message}" - return await self._run_codex(prompt) + return await self._run_conversation(prompt) + + async def _run_conversation( + self, prompt: str, mcp_overrides: list[str] | None = None, mode: str = "engineer" + ) -> dict[str, Any] | None: + last_result = await self._run_codex(prompt, mcp_overrides, mode=mode) + if last_result is None: + return None + try: + while follow_up := await self._prompt_follow_up(): + session_id = self.sdk_session_id or self.resume_session + if not session_id: + self.ui.error("Codex did not report a session ID; cannot continue this conversation") + return last_result + self.resume_session = session_id + self.message_store.save_prompt(follow_up) + result = await self._run_codex(follow_up, mcp_overrides, mode=mode) + if result is not None: + last_result = result + except (asyncio.CancelledError, KeyboardInterrupt) as exc: + exc.partial_result = last_result # type: ignore[attr-defined] + raise + return last_result class CodexAutoEngineer(CodexEngineer): @@ -464,31 +501,25 @@ def _get_active_prompts(self) -> tuple[str, str]: "\n\nDo not install packages or execute the generated client during this capture. " "Generate it from the captured traffic; it will be inspected and run separately. " "Avoid printing the full page body or full network log into the conversation. After identifying " - "the rankings GET request, parse its response in the browser and return only the first 15 rows " - "with the fields needed by the client. Do not assign a rank to unidentified zero-activity aggregate " - "rows; report if one is omitted, and fail closed on malformed nonzero rows. Use the native " + "the requested API calls, inspect their responses in the browser and return only the fields needed " + "to implement the user's request. Use the native " "file-edit/apply-patch capability to write " "the two requested files in the output directory; do not use Bash to inspect unrelated local files." ) return system_prompt, user_message def _playwright_mcp_overrides(self) -> list[str]: - command_line = " ".join( - [ - "npx -y", - RAE_PLAYWRIGHT_MCP_PACKAGE, - "--run-id", - self.mcp_run_id, - "--headless", - "--browser chrome", - ] - ) + browser_args = ["-y", RAE_PLAYWRIGHT_MCP_PACKAGE, "--run-id", self.mcp_run_id] + if self.headless: + browser_args.append("--headless") + browser_args.extend(["--browser", "chrome"]) if os.name == "nt": + command_line = "npx " + " ".join(browser_args) command, args = shutil.which("cmd.exe") or "cmd.exe", ["/d", "/s", "/c", command_line] else: command, args = ( shutil.which("npx") or "npx", - ["-y", RAE_PLAYWRIGHT_MCP_PACKAGE, "--run-id", self.mcp_run_id, "--headless", "--browser", "chrome"], + browser_args, ) # HAR is written under /har//recording.har. har_base = str(self.har_path.parent.parent) @@ -512,4 +543,4 @@ async def analyze_and_generate(self) -> dict[str, Any] | None: if not opening: return {} prompt = opening if self.resume_session else f"{system_prompt}\n\n{user_message}" - return await self._run_codex(prompt, self._playwright_mcp_overrides(), mode="agent") + return await self._run_conversation(prompt, self._playwright_mcp_overrides(), mode="agent") diff --git a/src/reverse_api/config.py b/src/reverse_api/config.py index 63e4b3b1..0d1c4aba 100644 --- a/src/reverse_api/config.py +++ b/src/reverse_api/config.py @@ -27,7 +27,7 @@ # Optional override: list of setting source ids, e.g. ["project","user","all"]. None uses cursor_web_search. "cursor_setting_sources": None, "copilot_model": "gpt-5", # Model for Copilot SDK sessions - "codex_model": None, # None uses the default model selected by the local Codex CLI + "codex_model": None, # Falls back to the per-run model, then RAE's gpt-6-luna default "opencode_model": DEFAULT_OPENCODE_MODEL, "opencode_provider": DEFAULT_OPENCODE_PROVIDER, "opencode_auto_start": True, diff --git a/src/reverse_api/engineer.py b/src/reverse_api/engineer.py index 96baeb0a..aa72672c 100644 --- a/src/reverse_api/engineer.py +++ b/src/reverse_api/engineer.py @@ -434,7 +434,7 @@ def run_reverse_engineering( opencode_model: Model ID for OpenCode (e.g., "claude-sonnet-4-6") copilot_model: Model ID for Copilot (e.g., "gpt-5") cursor_model: Model id for Cursor SDK (e.g., "composer-2.5") - codex_model: Optional model id for the local Codex CLI; None uses its configured default. + codex_model: Optional Codex model; None falls back to model, then gpt-6-luna. cursor_web_search: When True, load extra Cursor setting layers so WebFetch/WebSearch and plugins apply. cursor_setting_sources: Optional explicit list (overrides cursor_web_search), e.g. ["project","user","all"]. enable_sync: Enable real-time file syncing during engineering diff --git a/src/reverse_api/prompts/auto/system.md b/src/reverse_api/prompts/auto/system.md index 9e4bc70f..4a1d10b5 100644 --- a/src/reverse_api/prompts/auto/system.md +++ b/src/reverse_api/prompts/auto/system.md @@ -18,7 +18,6 @@ For this provider your integration surface is: - Narrate your actions as you browse - Capture all necessary API calls before generating code - Test your generated code to verify it works -- For ranked lists, omit unidentified rows only when their ranking metric and activity counters are all zero, and report the omission; fail on unidentified nonzero rows. - Screenshots must be under 1MB — prefer element-specific screenshots or use snapshots for structure ## Output files diff --git a/tests/test_cli_agent_json.py b/tests/test_cli_agent_json.py index 99701ed9..5436ac08 100644 --- a/tests/test_cli_agent_json.py +++ b/tests/test_cli_agent_json.py @@ -35,6 +35,18 @@ class TestBuildAgentPayload: """Stable shape for the `agent --json` payload.""" + def test_resumed_har_path_takes_precedence_over_original_capture(self, tmp_path): + original = tmp_path / "har" / "abc123" / "recording.har" + resumed = tmp_path / "har" / "abc123-resumed" / "recording.har" + for path in (original, resumed): + path.parent.mkdir(parents=True) + path.write_text("{}") + with patch("reverse_api.cli.get_har_dir", return_value=original.parent): + payload = _build_agent_payload( + {"run_id": "abc123", "har_path": str(resumed)}, prompt="x", url=None + ) + assert payload["har_path"] == str(resumed) + def test_success_shape(self, tmp_path): client = tmp_path / "scripts" / "api_client.py" client.parent.mkdir() @@ -140,6 +152,15 @@ def test_har_path_resolves_against_provided_output_dir(self, tmp_path): class TestAgentCommandJson: """`agent` click command's --json / --no-interactive behavior.""" + def test_codex_model_override_reaches_capture(self): + with ( + patch("reverse_api.cli.config_manager.get", side_effect=lambda key, default=None: {"sdk": "codex"}.get(key, default)), + patch("reverse_api.cli.run_agent_capture", return_value={"run_id": "codex-run"}) as capture, + ): + result = CliRunner().invoke(agent, ["--json", "-p", "capture weather", "--model", "gpt-6-luna"]) + assert result.exit_code == 0, result.output + assert capture.call_args.kwargs["model"] == "gpt-6-luna" + def test_json_without_prompt_exits_2(self): runner = CliRunner() result = runner.invoke(agent, ["--json"]) diff --git a/tests/test_cli_followups.py b/tests/test_cli_followups.py index ea183cf7..b200f635 100644 --- a/tests/test_cli_followups.py +++ b/tests/test_cli_followups.py @@ -215,12 +215,14 @@ def test_no_interactive_flag_accepted(self): assert result.exit_code == 0, result.output -def test_auto_capture_records_selected_sdk_in_history(tmp_path): +@pytest.mark.parametrize("resuming", [False, True]) +def test_auto_capture_records_selected_sdk_in_history(tmp_path, resuming): from reverse_api.cli import run_auto_capture class FakeCodexAutoEngineer: def __init__(self, **_kwargs): self.cost_before = None + self.mcp_run_id = "codex-run" def start_sync(self): pass @@ -229,7 +231,7 @@ def stop_sync(self): pass async def analyze_and_generate(self): - return {"script_path": str(tmp_path / "api_client.py"), "usage": {}} + return {"script_path": str(tmp_path / "api_client.py"), "har_path": str(tmp_path / "resumed" / "recording.har"), "usage": {}} config = { "sdk": "codex", @@ -251,11 +253,39 @@ async def analyze_and_generate(self): url="https://example.com", agent_provider="auto", interactive=False, + resume={"run_id": "codex-run", "sdk_session": {"id": "saved-session", "sdk": "codex"}} if resuming else None, ) assert result["run_id"] == "codex-run" - sessions.add_run.assert_called_once() - assert sessions.add_run.call_args.kwargs["sdk"] == "codex" + if resuming: + sessions.add_run.assert_not_called() + else: + sessions.add_run.assert_called_once() + assert sessions.add_run.call_args.kwargs["sdk"] == "codex" + paths = sessions.update_run.call_args.kwargs["paths"] + assert paths["har_path"] == str(tmp_path / "resumed" / "recording.har") + assert paths["har_dir"] == str(tmp_path / "resumed") + + +def test_engineer_uses_recorded_resumed_har_path(tmp_path): + from reverse_api.cli import run_engineer + + resumed_har = tmp_path / "resumed" / "recording.har" + resumed_har.parent.mkdir() + resumed_har.write_text("{}") + saved = { + "run_id": "codex-run", + "prompt": "capture weather", + "paths": {"har_dir": str(tmp_path / "original"), "har_path": str(resumed_har)}, + } + with ( + patch("reverse_api.cli.session_manager") as sessions, + patch("reverse_api.cli.config_manager.get", side_effect=lambda key, default=None: {"sdk": "codex"}.get(key, default)), + patch("reverse_api.cli.run_reverse_engineering", return_value=None) as generate, + ): + sessions.get_run.return_value = saved + run_engineer("codex-run", interactive=False) + assert generate.call_args.kwargs["har_path"] == resumed_har class TestEngineerJsonMissingRunId: diff --git a/tests/test_cli_settings.py b/tests/test_cli_settings.py index b0f53c8e..08b820bd 100644 --- a/tests/test_cli_settings.py +++ b/tests/test_cli_settings.py @@ -29,6 +29,35 @@ def test_settings_stays_open_until_back(): assert action.call_count == 3 +def test_selecting_codex_sets_compatible_browser_provider(): + with ( + patch.object(cli.questionary, "select", side_effect=[_answer("sdk"), _answer("codex")]), + patch.object(cli.config_manager, "update") as update, + patch.object(cli.config_manager, "set") as set_value, + ): + assert cli._handle_settings_action("magenta") is True + update.assert_called_once_with({"sdk": "codex", "agent_provider": "auto"}) + set_value.assert_not_called() + + +@pytest.mark.parametrize("command", ["agent", "engineer"]) +@pytest.mark.parametrize("model", ["gpt-6-luna", "custom-codex-model"]) +def test_codex_model_flag_accepts_sdk_model_ids(command, model): + import click + + with patch.object(cli.config_manager, "get", return_value="codex"): + context = click.Context(getattr(cli, command)) + assert cli.ModelChoice().convert(model, None, context) == model + + +def test_claude_model_validation_still_rejects_codex_ids(): + import click + + with patch.object(cli.config_manager, "get", return_value="claude"): + with pytest.raises(click.BadParameter): + cli.ModelChoice().convert("gpt-6-luna", None, click.Context(cli.agent)) + + def _answer(value): question = MagicMock() question.ask.return_value = value diff --git a/tests/test_codex_engineer.py b/tests/test_codex_engineer.py index 9571dfa8..1248813a 100644 --- a/tests/test_codex_engineer.py +++ b/tests/test_codex_engineer.py @@ -3,7 +3,8 @@ import json import os import tomllib -from unittest.mock import MagicMock, patch +from pathlib import Path +from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -58,6 +59,16 @@ def test_mcp_overrides_are_per_invocation_and_required(tmp_path): assert parsed["env"]["PLAYWRIGHT_MCP_SAVE_HAR_PATH"].endswith(suffix) +def test_resume_keeps_exec_only_options_before_the_subcommand(tmp_path): + eng = make_engineer(tmp_path) + eng.resume_session = "thread-123" + with patch("reverse_api.codex_engineer.resolve_codex_command", return_value=["codex"]): + args = eng._build_exec_args("refine") + assert args[-3:] == ["resume", "thread-123", "refine"] + assert args.index("--sandbox") < args.index("resume") + assert args.index("--cd") < args.index("resume") + + def test_windows_prefers_codex_desktop_cli(tmp_path, monkeypatch): cli = tmp_path / "OpenAI" / "Codex" / "bin" / "desktop-build" / "codex.exe" cli.parent.mkdir(parents=True) @@ -69,7 +80,7 @@ def test_windows_prefers_codex_desktop_cli(tmp_path, monkeypatch): assert codex_module.resolve_codex_command() == [str(cli)] -def test_desktop_cli_uses_workspace_scoped_approval_review(tmp_path, monkeypatch): +def test_desktop_cli_uses_explicit_writable_workspace(tmp_path, monkeypatch): cli = tmp_path / "OpenAI" / "Codex" / "bin" / "desktop-build" / "codex.exe" cli.parent.mkdir(parents=True) cli.touch() @@ -80,6 +91,7 @@ def test_desktop_cli_uses_workspace_scoped_approval_review(tmp_path, monkeypatch args = eng._build_exec_args("browse public page") assert "--approve-for-me" in args + assert 'sandbox_mode="workspace-write"' in args assert "--sandbox" not in args @@ -164,3 +176,116 @@ def test_auto_capture_requires_har_preserving_provider(tmp_path): verbose=False, interactive=False, ) + + +def test_unrelated_capture_has_no_rankings_or_first_fifteen_instructions(tmp_path): + eng = CodexAutoEngineer(run_id="weather", prompt="Fetch all 48 hourly weather records", output_dir=str(tmp_path), interactive=False) + _, prompt = eng._get_active_prompts() + assert "Fetch all 48 hourly weather records" in prompt + assert "rankings GET" not in prompt + assert "first 15" not in prompt + + +@pytest.mark.parametrize("headless", [False, True]) +def test_browser_launch_honors_headless_on_both_platforms(tmp_path, monkeypatch, headless): + eng = CodexAutoEngineer(run_id="browser", prompt="Fetch weather", output_dir=str(tmp_path), headless=headless, interactive=False) + # Construct paths before mocking os.name: pathlib uses the real host type. + for platform in ("nt", "posix"): + with monkeypatch.context() as patcher: + patcher.setattr(codex_module.os, "name", platform) + args = " ".join(eng._playwright_mcp_overrides()) + assert ("--headless" in args) is headless + + +@pytest.mark.parametrize("auto", [False, True]) +def test_relative_paths_are_absolute_in_prompts_and_browser_environment(tmp_path, monkeypatch, auto): + monkeypatch.chdir(tmp_path) + if auto: + eng = CodexAutoEngineer(run_id="relative", prompt="Fetch weather", output_dir="runs", interactive=False) + _, prompt = eng._get_active_prompts() + assert str((tmp_path / "runs" / "har").resolve()).replace("\\", "\\\\") in " ".join(eng._playwright_mcp_overrides()) + else: + eng = make_engineer(tmp_path, output_dir="runs", har_path=Path("runs/har/relative/recording.har")) + _, prompt = eng._build_prompts() + assert eng.scripts_dir.is_absolute() + assert eng.har_path.is_absolute() + assert str(eng.scripts_dir) in prompt + assert str(eng.har_path) in prompt + + +@pytest.mark.parametrize("auto", [False, True]) +@pytest.mark.parametrize("resumed", [False, True]) +async def test_followups_resume_same_session_and_keep_latest_success(tmp_path, auto, resumed): + eng = (CodexAutoEngineer(run_id="turns", prompt="Fetch weather", output_dir=str(tmp_path)) if auto else make_engineer(tmp_path, interactive=True)) + eng.resume_session = "thread-original" if resumed else None + messages = ["resume request", "refine", "try again", None] if resumed else ["refine", "try again", None] + eng._prompt_follow_up = AsyncMock(side_effect=messages) + eng.ui = MagicMock() + eng.message_store = MagicMock() + calls = [] + results = [{"script_path": "first"}, {"script_path": "refined"}, None] + + async def turn(prompt, *args, **kwargs): + calls.append((prompt, eng.resume_session)) + eng._record_session("thread-original") + return results[len(calls) - 1] + + eng._run_codex = turn + result = await eng.analyze_and_generate() + assert result == {"script_path": "refined"} + assert calls[1:] == [("refine", "thread-original"), ("try again", "thread-original")] + eng.message_store.save_prompt.assert_any_call("refine") + + +def test_codex_usage_separates_cached_input_from_uncached_input(tmp_path): + eng = make_engineer(tmp_path) + for _ in range(2): + eng._handle_codex_event({"type": "turn.completed", "usage": {"input_tokens": 18542, "cached_input_tokens": 11776, "output_tokens": 5}}) + assert eng.usage_metadata["input_tokens"] == 13532 + assert eng.usage_metadata["cache_read_tokens"] == 23552 + assert eng.usage_metadata["output_tokens"] == 10 + + +@pytest.mark.parametrize("relative", [False, True]) +async def test_capture_subprocess_artifacts_and_code_only_followup(tmp_path, monkeypatch, relative): + monkeypatch.chdir(tmp_path) + output = "runs with spaces" if relative else str(tmp_path / "runs with spaces") + eng = CodexAutoEngineer(run_id="process", prompt="Fetch weather", output_dir=output, interactive=False) + eng.ui = MagicMock() + eng.message_store = MagicMock() + child = tmp_path / "controlled_child.py" + child.write_text( + "import json, sys\n" + "from pathlib import Path\n" + "scripts, har = Path(sys.argv[1]), Path(sys.argv[2])\n" + "if sys.argv[3] == 'capture':\n" + " scripts.mkdir(parents=True, exist_ok=True)\n" + " (scripts / 'api_client.py').write_text('# generated client')\n" + " har.parent.mkdir(parents=True, exist_ok=True)\n" + " har.write_text(json.dumps({'log': {'entries': [{'request': {'method': 'GET'}}]}}))\n" + " print(json.dumps({'type': 'item.started', 'item': {'type': 'mcp_tool_call', " + "'id': 'nav', 'server': 'playwright', 'tool': 'browser_navigate'}}))\n" + "print(json.dumps({'type': 'thread.started', 'thread_id': 'process-thread'}))\n" + "print(json.dumps({'type': 'turn.completed', 'usage': {'input_tokens': 20, 'cached_input_tokens': 10, 'output_tokens': 2}}))\n", + encoding="utf-8", + ) + eng._build_exec_args = lambda prompt, overrides: [codex_module.sys.executable, str(child), str(eng.scripts_dir), str(eng.har_path), prompt] + first = await eng._run_codex("capture", eng._playwright_mcp_overrides(), mode="agent") + assert first is not None + assert first["har_entries"] == 1 + assert Path(first["script_path"]).is_file() + assert Path(first["har_path"]).is_file() + assert not (tmp_path / "runs with spaces" / "runs with spaces").exists() + # A refinement can reuse the verified capture without calling browser MCP again. + eng.resume_session = "process-thread" + followup = await eng._run_codex("edit only", eng._playwright_mcp_overrides(), mode="agent") + assert followup is not None + assert followup["har_entries"] == 1 + assert followup["usage"]["input_tokens"] == 20 + + +async def test_noninteractive_stops_after_one_turn(tmp_path): + eng = make_engineer(tmp_path) + eng._run_codex = AsyncMock(return_value={"script_path": "first"}) + assert await eng.analyze_and_generate() == {"script_path": "first"} + eng._run_codex.assert_awaited_once() From ce07a41d4a46c536c12faf9226663088e2f36e84 Mon Sep 17 00:00:00 2001 From: leonininder <8566513+leonininder@users.noreply.github.com> Date: Tue, 3 Nov 2026 09:38:37 +0800 Subject: [PATCH 3/4] fix: retain absolute HAR path when resuming Codex capture --- src/reverse_api/cli.py | 2 +- tests/test_cli_followups.py | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/reverse_api/cli.py b/src/reverse_api/cli.py index 64f4c4db..3ad3e3f3 100644 --- a/src/reverse_api/cli.py +++ b/src/reverse_api/cli.py @@ -1764,7 +1764,7 @@ def run_auto_capture( # noqa: C901 - SDK/provider/session lifecycle dispatch if hasattr(engineer, "mcp_run_id"): engineer.mcp_run_id = f"{run_id}-{int(time.time())}" if sdk == "codex": - engineer.har_path = get_har_dir(engineer.mcp_run_id, output_dir) / "recording.har" + engineer.har_path = (get_har_dir(engineer.mcp_run_id, output_dir) / "recording.har").resolve() if json_event_sink is not None: from .json_stream import attach_json_stream_to_engineer diff --git a/tests/test_cli_followups.py b/tests/test_cli_followups.py index b200f635..9000388a 100644 --- a/tests/test_cli_followups.py +++ b/tests/test_cli_followups.py @@ -288,6 +288,42 @@ def test_engineer_uses_recorded_resumed_har_path(tmp_path): assert generate.call_args.kwargs["har_path"] == resumed_har +def test_resumed_codex_capture_keeps_relative_output_har_absolute(tmp_path, monkeypatch): + from reverse_api.cli import run_auto_capture + from reverse_api.codex_engineer import CodexAutoEngineer + + monkeypatch.chdir(tmp_path) + expected = tmp_path / "runs" / "har" / "codex-run-1234" / "recording.har" + observed = [] + + async def capture(engineer): + observed.append(engineer.har_path) + assert engineer.har_path == expected + assert str(expected) in engineer._get_active_prompts()[1] + return {"har_path": str(engineer.har_path), "usage": {}} + + with ( + patch( + "reverse_api.cli.config_manager.get", + side_effect=lambda key, default=None: {"sdk": "codex", "real_time_sync": False}.get(key, default), + ), + patch("reverse_api.cli.session_manager"), + patch("reverse_api.cli.time.time", return_value=1234), + patch("reverse_api.cli._keep_session_cost"), + patch.object(CodexAutoEngineer, "analyze_and_generate", capture), + ): + result = run_auto_capture( + prompt="capture weather", + output_dir="runs", + agent_provider="auto", + interactive=False, + resume={"run_id": "codex-run", "sdk_session": {"id": "saved-session", "sdk": "codex"}}, + ) + + assert observed == [expected] + assert result["har_path"] == str(expected) + + class TestEngineerJsonMissingRunId: """`engineer --json` without RUN_ID must emit a JSON misuse payload, not Click's plain-text 'missing argument' error. From ab04f0664ecce56291e029edb0ce409fa897008b Mon Sep 17 00:00:00 2001 From: leonininder <8566513+leonininder@users.noreply.github.com> Date: Tue, 3 Nov 2026 09:53:10 +0800 Subject: [PATCH 4/4] test: verify fresh and resumed Codex HAR stretches --- .../reverse-api-engineer/references/setup.md | 2 +- tests/test_cli_followups.py | 18 ++++++++++++++---- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/skills/reverse-api-engineer/references/setup.md b/skills/reverse-api-engineer/references/setup.md index 832af39b..a83930cb 100644 --- a/skills/reverse-api-engineer/references/setup.md +++ b/skills/reverse-api-engineer/references/setup.md @@ -38,4 +38,4 @@ The user can also change them interactively by running `reverse-api-engineer` an Language notes: C needs a POSIX toolchain with libcurl headers. PowerShell needs `pwsh` 7+. -For Codex, model selection uses codex_model, then the per-run model, then gpt-6-luna. Empty or default selections also resolve to gpt-6-luna; they do not select the Codex CLI configured default. Choosing Codex in /settings also selects the required auto browser provider. +For Codex agent runs, model selection uses the per-run --model override, then codex_model, then gpt-6-luna. A selected value of default also resolves to gpt-6-luna rather than the Codex CLI configured default. Choosing Codex in /settings also selects the required auto browser provider. diff --git a/tests/test_cli_followups.py b/tests/test_cli_followups.py index 9000388a..4c9fb9d8 100644 --- a/tests/test_cli_followups.py +++ b/tests/test_cli_followups.py @@ -219,10 +219,14 @@ def test_no_interactive_flag_accepted(self): def test_auto_capture_records_selected_sdk_in_history(tmp_path, resuming): from reverse_api.cli import run_auto_capture + expected_mcp_run_id = "codex-run-1234" if resuming else "codex-run" + expected_har = tmp_path / "har" / expected_mcp_run_id / "recording.har" + class FakeCodexAutoEngineer: def __init__(self, **_kwargs): self.cost_before = None self.mcp_run_id = "codex-run" + self.har_path = tmp_path / "har" / "codex-run" / "recording.har" def start_sync(self): pass @@ -231,7 +235,12 @@ def stop_sync(self): pass async def analyze_and_generate(self): - return {"script_path": str(tmp_path / "api_client.py"), "har_path": str(tmp_path / "resumed" / "recording.har"), "usage": {}} + assert self.mcp_run_id == expected_mcp_run_id + assert self.har_path == expected_har + if resuming: + assert self.mcp_run_id != "codex-run" + assert self.har_path.parent != tmp_path / "har" / "codex-run" + return {"script_path": str(tmp_path / "api_client.py"), "har_path": str(self.har_path), "usage": {}} config = { "sdk": "codex", @@ -244,7 +253,8 @@ async def analyze_and_generate(self): patch("reverse_api.cli.session_manager") as sessions, patch("reverse_api.cli.generate_run_id", return_value="codex-run"), patch("reverse_api.cli.get_timestamp", return_value="2026-11-02T00:00:00"), - patch("reverse_api.cli.get_har_dir", return_value=tmp_path / "har" / "codex-run"), + patch("reverse_api.cli.get_har_dir", side_effect=lambda run_id, output_dir: tmp_path / "har" / run_id), + patch("reverse_api.cli.time.time", return_value=1234), patch("reverse_api.codex_engineer.CodexAutoEngineer", side_effect=FakeCodexAutoEngineer), patch("reverse_api.cli._keep_session_cost"), ): @@ -263,8 +273,8 @@ async def analyze_and_generate(self): sessions.add_run.assert_called_once() assert sessions.add_run.call_args.kwargs["sdk"] == "codex" paths = sessions.update_run.call_args.kwargs["paths"] - assert paths["har_path"] == str(tmp_path / "resumed" / "recording.har") - assert paths["har_dir"] == str(tmp_path / "resumed") + assert paths["har_path"] == str(expected_har) + assert paths["har_dir"] == str(expected_har.parent) def test_engineer_uses_recorded_resumed_har_path(tmp_path):