From 6544087f2fee2e1a1659e25b290a5eb40cc485aa Mon Sep 17 00:00:00 2001 From: kalil0321 Date: Wed, 22 Jul 2026 13:38:50 +0200 Subject: [PATCH 1/3] Make run/list language-aware and quote run-command paths per-platform Two gaps found while testing the language PRs (#100-#105): - discover_scripts() only globbed .py, so 'reverse-api-engineer run' and 'list' reported 'No Python scripts found' for every non-Python client, and the run path always executed the shared-venv Python interpreter. Discovery now covers all supported extensions (single source of truth in utils.OUTPUT_LANGUAGE_EXTENSIONS, shared with BaseEngineer), excludes build dirs and the vendored cJSON sources, and the run command dispatches per language - node/npx tsx/go run/mvn exec:exec/ dotnet run/php/ruby, plus compile-then-execute for C - with a clear error when the required tool is missing from PATH. - The Java/C#/PHP/Ruby/C run commands quoted paths with POSIX-only shlex.quote, which cmd.exe/PowerShell parse incorrectly for paths with spaces. BaseEngineer._quote_path() now uses subprocess.list2cmdline on Windows and shlex.quote elsewhere. Also asks the C partial for -Wall -Wextra-clean code (generated client had unused-parameter warnings in live testing). Verified live: run --json executed real generated Go/PHP/C/C#/Ruby clients from earlier agent sessions end-to-end (C compiling first). --- CHANGELOG.md | 4 + src/reverse_api/base_engineer.py | 50 ++++--- src/reverse_api/cli.py | 73 +++++++++- .../prompts/partials/_language_c.md | 1 + src/reverse_api/utils.py | 87 +++++++++++- tests/test_run_command.py | 127 +++++++++++++++++- 6 files changed, 314 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 860d17c6..fa6b869b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- **`run`/`list` now work for every output language**: `discover_scripts()` previously only found `.py` files, so `reverse-api-engineer run ` failed with "No Python scripts found" for JS/TS/Go/Java/C#/PHP/Ruby/C clients. Discovery now covers all supported extensions (excluding build dirs and the vendored cJSON sources), and the run command dispatches to the right toolchain per language (compile+execute for C), with a clear error when the required tool isn't on PATH. +- **Windows-safe run-command quoting**: the Java/C#/PHP/Ruby/C run commands quoted paths with POSIX-only `shlex.quote`, which cmd.exe/PowerShell parse incorrectly for paths containing spaces. Paths are now quoted per-platform (`subprocess.list2cmdline` on Windows). + ### Added - **Go output language**: `output_language: "go"` is now supported alongside python/javascript/typescript, generating a standard-library-first (`net/http`, `encoding/json`) Go program, with the same auth-hardcoding/refresh and bot-detection-fallback guidance as the other languages. - **Java output language**: `output_language: "java"` is now supported alongside python/javascript/typescript, generating a small Maven project using `java.net.http.HttpClient` (JDK 11+, no HTTP library dependency) and Gson for JSON, with the same auth-hardcoding/refresh guidance as the other languages. diff --git a/src/reverse_api/base_engineer.py b/src/reverse_api/base_engineer.py index e6258cdd..ff4caaef 100644 --- a/src/reverse_api/base_engineer.py +++ b/src/reverse_api/base_engineer.py @@ -3,6 +3,8 @@ import asyncio import os import shlex +import subprocess +import sys from abc import ABC, abstractmethod from pathlib import Path from typing import Any @@ -13,7 +15,13 @@ from .session import SessionManager from .sync import FileSyncWatcher, get_available_directory from .tui import THEME_PRIMARY, THEME_SECONDARY, ClaudeUI -from .utils import generate_folder_name, get_docs_dir, get_history_path, get_scripts_dir +from .utils import ( + OUTPUT_LANGUAGE_EXTENSIONS, + generate_folder_name, + get_docs_dir, + get_history_path, + get_scripts_dir, +) DEBUG = os.environ.get("DEBUG", "0") == "1" @@ -30,17 +38,9 @@ class BaseEngineer(ABC): """Abstract base class for API reverse engineering implementations.""" - _OUTPUT_LANGUAGE_EXTENSIONS = { - "python": ".py", - "javascript": ".js", - "typescript": ".ts", - "go": ".go", - "java": ".java", - "csharp": ".cs", - "php": ".php", - "ruby": ".rb", - "c": ".c", - } + # Single source of truth lives in utils.OUTPUT_LANGUAGE_EXTENSIONS so + # script discovery and the run command dispatch stay in sync with codegen. + _OUTPUT_LANGUAGE_EXTENSIONS = OUTPUT_LANGUAGE_EXTENSIONS def __init__( self, @@ -474,6 +474,18 @@ def _get_client_filename(self) -> str: return "openapi.json" return f"api_client{self._get_output_extension()}" + @staticmethod + def _quote_path(path) -> str: + """Shell-quote a path for the platform's default shell. + + shlex.quote is POSIX-only: cmd.exe/PowerShell pass its single quotes + through literally, so a spaced Windows path would break apart. + list2cmdline applies the double-quoting rules Windows shells parse. + """ + if sys.platform == "win32": + return subprocess.list2cmdline([str(path)]) + return shlex.quote(str(path)) + def _get_run_command(self) -> str: """Return the command to run the generated client.""" if self.output_language == "java": @@ -495,7 +507,7 @@ def _get_run_command(self) -> str: # exec:java invokes main() reflectively in-process, which fails # on the package-private ApiClient class the Java partial # requires ("symbolic reference class is not accessible"). - pom = shlex.quote(str(self.scripts_dir.resolve() / "pom.xml")) + pom = self._quote_path(str(self.scripts_dir.resolve() / "pom.xml")) return f"mvn -q -f {pom} compile exec:exec" if self.output_language == "csharp": # Unlike python/node/npx (which happily take a plain relative @@ -512,7 +524,7 @@ def _get_run_command(self) -> str: # re-interpreted against the agent's cwd (scripts_dir.parent. # parent) instead of the original cwd it was relative to, # pointing --project at the wrong, doubly-nested location. - csproj = shlex.quote(str(self.scripts_dir.resolve() / "ApiClient.csproj")) + csproj = self._quote_path(str(self.scripts_dir.resolve() / "ApiClient.csproj")) return f"dotnet run --project {csproj}" if self.output_language == "php": # Full path, not a bare relative "php api_client.php": the @@ -532,7 +544,7 @@ def _get_run_command(self) -> str: # re-interpreted against the agent's cwd (scripts_dir.parent. # parent) instead of the original cwd it was relative to, # pointing this command at the wrong, doubly-nested location. - path = shlex.quote(str(self.scripts_dir.resolve() / self._get_client_filename())) + path = self._quote_path(str(self.scripts_dir.resolve() / self._get_client_filename())) return f"php {path}" if self.output_language == "ruby": # Full path, not a bare relative "ruby api_client.rb": the @@ -546,7 +558,7 @@ def _get_run_command(self) -> str: # re-interpreted against the agent's cwd (scripts_dir.parent. # parent) instead of the original cwd it was relative to, # pointing this command at the wrong, doubly-nested location. - path = shlex.quote(str(self.scripts_dir.resolve() / self._get_client_filename())) + path = self._quote_path(str(self.scripts_dir.resolve() / self._get_client_filename())) return f"ruby {path}" if self.output_language == "c": # Unlike every other language here, C needs an explicit compile @@ -562,9 +574,9 @@ def _get_run_command(self) -> str: # dir.parent.parent) instead of the original cwd it was relative # to, pointing all three at the wrong, doubly-nested location. resolved = self.scripts_dir.resolve() - source = shlex.quote(str(resolved / self._get_client_filename())) - cjson = shlex.quote(str(resolved / "cJSON.c")) - binary = shlex.quote(str(resolved / "api_client")) + source = self._quote_path(str(resolved / self._get_client_filename())) + cjson = self._quote_path(str(resolved / "cJSON.c")) + binary = self._quote_path(str(resolved / "api_client")) return f"cc {source} {cjson} -lcurl -o {binary} && {binary}" return { "python": "python api_client.py", diff --git a/src/reverse_api/cli.py b/src/reverse_api/cli.py index cb0ad472..9dce4049 100644 --- a/src/reverse_api/cli.py +++ b/src/reverse_api/cli.py @@ -2663,6 +2663,27 @@ def _extract_missing_module(stderr: str) -> str | None: return missing +def _run_non_python_script(script, script_args) -> None: + """Run a non-Python generated client with its language's toolchain and exit.""" + import shutil + import subprocess + + from .utils import build_script_commands + + try: + steps, tool = build_script_commands(script, script_args) + except ValueError as e: + raise click.ClickException(str(e)) from e + if shutil.which(tool) is None: + raise click.ClickException(f"'{tool}' not found on PATH (required to run {script.name})") + returncode = 0 + for cmd in steps: + returncode = subprocess.run(cmd, cwd=str(script.parent)).returncode + if returncode != 0: + break + raise SystemExit(returncode) + + def _run_script_machine_payload( *, identifier: str, @@ -2691,7 +2712,7 @@ def _run_script_machine_payload( identifier=identifier, run_id=run_id, scripts=scripts, - error=f"No Python scripts found for run {run_id}", + error=f"No runnable scripts found for run {run_id}", error_kind_hint="engine_failure", ) @@ -2729,6 +2750,50 @@ def _run_script_machine_payload( emit_event("script_selected", run_id=run_id, script_path=str(script)) + if script.suffix != ".py": + import shutil + + from .utils import build_script_commands + + try: + steps, tool = build_script_commands(script, script_args) + except ValueError as e: + return _build_run_payload( + identifier=identifier, + run_id=run_id, + script_path=str(script), + script_args=script_args, + scripts=scripts, + error=str(e), + error_kind_hint="misuse", + ) + if shutil.which(tool) is None: + return _build_run_payload( + identifier=identifier, + run_id=run_id, + script_path=str(script), + script_args=script_args, + scripts=scripts, + error=f"'{tool}' not found on PATH (required to run {script.name})", + error_kind_hint="config_invalid", + ) + result = None + for cmd in steps: + emit_event("process_started", run_id=run_id, script_path=str(script)) + result = subprocess.run(cmd, cwd=str(script.parent), capture_output=True, text=True) + if result.returncode != 0: + break + return _build_run_payload( + identifier=identifier, + run_id=run_id, + script_path=str(script), + script_args=script_args, + returncode=result.returncode, + stdout=result.stdout or "", + stderr=result.stderr or "", + scripts=scripts, + ) + from .utils import get_base_output_dir as _get_base venv_dir = _get_base(output_dir) / ".venv" @@ -2930,7 +2995,7 @@ def emit_event(event: str, **fields) -> None: console.print(f"{scripts[0].parent}", style="dim") if not scripts: - console.print(f"[red]No Python scripts found for run {run_id}[/red]") + console.print(f"[red]No runnable scripts found for run {run_id}[/red]") raise SystemExit(1) # --ls: just list and exit @@ -2974,6 +3039,10 @@ def emit_event(event: str, **fields) -> None: if script is None: raise click.Abort() + # Non-Python clients: dispatch on extension, no venv involved + if script.suffix != ".py": + _run_non_python_script(script, script_args) + # Shared venv at ~/.reverse-api/runs/.venv (with requests pre-installed) from .utils import get_base_output_dir as _get_base diff --git a/src/reverse_api/prompts/partials/_language_c.md b/src/reverse_api/prompts/partials/_language_c.md index a903eedd..38565996 100644 --- a/src/reverse_api/prompts/partials/_language_c.md +++ b/src/reverse_api/prompts/partials/_language_c.md @@ -8,6 +8,7 @@ - Reuse one `CURL` handle across requests rather than creating a new one per call - Create a separate function for each distinct API endpoint, with a small struct for its response shape - Check every `libcurl`/allocation return value; don't ignore errors +- Keep the code warning-clean under `-Wall -Wextra` — no unused parameters or variables - Include a `main` function with example usage **Authentication & credentials:** diff --git a/src/reverse_api/utils.py b/src/reverse_api/utils.py index a5f85f55..58e69dbe 100644 --- a/src/reverse_api/utils.py +++ b/src/reverse_api/utils.py @@ -11,6 +11,23 @@ from . import __version__ +# One entry per supported output language. BaseEngineer's extension map is +# built from this, and discover_scripts()/build_script_commands() key off the +# extensions, so adding a language here is the single required registration. +OUTPUT_LANGUAGE_EXTENSIONS = { + "python": ".py", + "javascript": ".js", + "typescript": ".ts", + "go": ".go", + "java": ".java", + "csharp": ".cs", + "php": ".php", + "ruby": ".rb", + "c": ".c", +} + +SCRIPT_EXTENSIONS = frozenset(OUTPUT_LANGUAGE_EXTENSIONS.values()) + def check_for_updates() -> str | None: """Check PyPI for newer version. @@ -631,7 +648,7 @@ def resolve_run(identifier: str, session_manager, *, interactive: bool = True) - def discover_scripts(run_id: str, output_dir: str | None = None, run_metadata: dict | None = None) -> list[Path]: - """Find all executable Python scripts in a run's script directory. + """Find all runnable generated scripts in a run's script directory. Tries the stored script path from run metadata first, then falls back to the current output_dir config. @@ -642,7 +659,9 @@ def discover_scripts(run_id: str, output_dir: str | None = None, run_metadata: d run_metadata: Optional run dict with paths.script_path to resolve from Returns: - Sorted list of .py file Paths (excludes __pycache__, .venv, __init__.py) + Sorted list of script Paths in any supported output language + (excludes build/venv directories and support files like __init__.py + and the vendored cJSON sources) Raises: ValueError: If run_id contains invalid characters @@ -672,18 +691,76 @@ def discover_scripts(run_id: str, output_dir: str | None = None, run_metadata: d if not scripts_dir.exists(): return [] - exclude_dirs = {"__pycache__", ".venv"} - exclude_files = {"__init__.py"} + exclude_dirs = {"__pycache__", ".venv", "node_modules", "bin", "obj", "target"} + # Support files that share a script extension but are never the client + # itself: package markers and the vendored cJSON library (C output). + exclude_files = {"__init__.py", "cJSON.c", "cJSON.h"} scripts = [] for f in scripts_dir.iterdir(): - if f.is_file() and f.suffix == ".py" and f.name not in exclude_files: + if f.is_file() and f.suffix in SCRIPT_EXTENSIONS and f.name not in exclude_files: scripts.append(f) scripts = [s for s in scripts if not any(part in exclude_dirs for part in s.parts)] return sorted(scripts, key=lambda p: p.name) +def build_script_commands(script: Path, script_args: tuple[str, ...] = ()) -> tuple[list[list[str]], str]: + """Build the subprocess command(s) that run a non-Python generated client. + + Python clients are executed by the caller's shared-venv flow and are not + handled here. Commands use absolute paths so they work from any cwd; run + them with cwd=script.parent so relative artifacts (cookie jars, build + output) land next to the script. + + Args: + script: Path to the client script (any supported non-.py extension) + script_args: Extra arguments to pass through to the client + + Returns: + (steps, tool): ordered argv lists to run (C compiles then executes, + everything else is a single step) and the executable that must be on + PATH for them to work. + + Raises: + ValueError: For unsupported extensions, or script_args with a Java + client (mvn exec's program arguments are fixed in the pom). + """ + d = script.parent + suffix = script.suffix + if suffix == ".js": + return [["node", str(script), *script_args]], "node" + if suffix == ".ts": + return [["npx", "tsx", str(script), *script_args]], "npx" + if suffix == ".go": + return [["go", "run", str(script), *script_args]], "go" + if suffix == ".java": + if script_args: + raise ValueError( + "script arguments are not supported for Java clients: " + "mvn exec:exec's program arguments are fixed in the pom.xml" + ) + return [["mvn", "-q", "-f", str(d / "pom.xml"), "compile", "exec:exec"]], "mvn" + if suffix == ".cs": + cmd = ["dotnet", "run", "--project", str(d / "ApiClient.csproj")] + if script_args: + cmd += ["--", *script_args] + return [cmd], "dotnet" + if suffix == ".php": + return [["php", str(script), *script_args]], "php" + if suffix == ".rb": + return [["ruby", str(script), *script_args]], "ruby" + if suffix == ".c": + binary = d / script.stem + compile_cmd = ["cc", str(script)] + cjson = d / "cJSON.c" + if cjson.exists(): + compile_cmd.append(str(cjson)) + compile_cmd += ["-lcurl", "-o", str(binary)] + return [compile_cmd, [str(binary), *script_args]], "cc" + raise ValueError(f"unsupported script type: {script.name}") + + def extract_domain_from_har(har_path: Path) -> str | None: """Extract the primary domain from a HAR file. diff --git a/tests/test_run_command.py b/tests/test_run_command.py index bc6ad1bf..0213dbb2 100644 --- a/tests/test_run_command.py +++ b/tests/test_run_command.py @@ -251,6 +251,44 @@ def test_too_long_run_id_raises(self): with pytest.raises(ValueError, match="too long"): discover_scripts("a" * 65) + def test_finds_all_language_extensions(self, scripts_dir_empty, tmp_path): + for name in [ + "api_client.py", "api_client.js", "api_client.ts", "api_client.go", + "api_client.java", "api_client.cs", "api_client.php", + "api_client.rb", "api_client.c", + ]: + (scripts_dir_empty / name).write_text("") + with patch("reverse_api.utils.get_base_output_dir", return_value=tmp_path): + scripts = discover_scripts("abc123def456") + assert len(scripts) == 9 + + def test_excludes_vendored_cjson(self, scripts_dir_empty, tmp_path): + (scripts_dir_empty / "api_client.c").write_text("") + (scripts_dir_empty / "cJSON.c").write_text("") + (scripts_dir_empty / "cJSON.h").write_text("") + with patch("reverse_api.utils.get_base_output_dir", return_value=tmp_path): + scripts = discover_scripts("abc123def456") + assert [s.name for s in scripts] == ["api_client.c"] + + def test_excludes_build_dirs(self, scripts_dir_empty, tmp_path): + (scripts_dir_empty / "api_client.cs").write_text("") + for d in ["node_modules", "bin", "obj", "target"]: + sub = scripts_dir_empty / d + sub.mkdir() + (sub / "generated.cs").write_text("") + with patch("reverse_api.utils.get_base_output_dir", return_value=tmp_path): + scripts = discover_scripts("abc123def456") + assert [s.name for s in scripts] == ["api_client.cs"] + + def test_excludes_non_script_support_files(self, scripts_dir_empty, tmp_path): + (scripts_dir_empty / "api_client.java").write_text("") + (scripts_dir_empty / "pom.xml").write_text("") + (scripts_dir_empty / "go.mod").write_text("") + (scripts_dir_empty / "ApiClient.csproj").write_text("") + with patch("reverse_api.utils.get_base_output_dir", return_value=tmp_path): + scripts = discover_scripts("abc123def456") + assert [s.name for s in scripts] == ["api_client.java"] + class TestDiscoverScriptsRunMetadata: """Test discover_scripts with run_metadata (stored path) fallback.""" @@ -361,7 +399,7 @@ def test_ls_no_scripts_error(self, cli_runner, mock_cli_env): from reverse_api.cli import main result = cli_runner.invoke(main, ["run", "111222333444", "--ls"]) assert result.exit_code == 1 - assert "No Python scripts" in result.output + assert "No runnable scripts" in result.output def test_ls_with_fuzzy_name(self, cli_runner, mock_cli_env): from reverse_api.cli import main @@ -461,7 +499,7 @@ def test_no_scripts_shows_error(self, cli_runner, mock_cli_env): from reverse_api.cli import main result = cli_runner.invoke(main, ["run", "111222333444"]) assert result.exit_code == 1 - assert "No Python scripts" in result.output + assert "No runnable scripts" in result.output def test_no_scripts_shows_prompt_preview(self, cli_runner, mock_cli_env): from reverse_api.cli import main @@ -684,3 +722,88 @@ def test_trailing_newline_rejected(self): from reverse_api.cli import _extract_missing_module stderr = "ModuleNotFoundError: No module named 'requests\n'" assert _extract_missing_module(stderr) is None + + +# --------------------------------------------------------------------------- +# build_script_commands tests +# --------------------------------------------------------------------------- + +class TestBuildScriptCommands: + """Test per-language command dispatch for the run subcommand.""" + + def test_javascript(self, tmp_path): + from reverse_api.utils import build_script_commands + script = tmp_path / "api_client.js" + steps, tool = build_script_commands(script) + assert steps == [["node", str(script)]] + assert tool == "node" + + def test_typescript(self, tmp_path): + from reverse_api.utils import build_script_commands + script = tmp_path / "api_client.ts" + steps, tool = build_script_commands(script, ("--flag",)) + assert steps == [["npx", "tsx", str(script), "--flag"]] + assert tool == "npx" + + def test_go(self, tmp_path): + from reverse_api.utils import build_script_commands + script = tmp_path / "api_client.go" + steps, tool = build_script_commands(script) + assert steps == [["go", "run", str(script)]] + assert tool == "go" + + def test_java(self, tmp_path): + from reverse_api.utils import build_script_commands + script = tmp_path / "api_client.java" + steps, tool = build_script_commands(script) + assert steps == [["mvn", "-q", "-f", str(tmp_path / "pom.xml"), "compile", "exec:exec"]] + assert tool == "mvn" + + def test_java_rejects_script_args(self, tmp_path): + from reverse_api.utils import build_script_commands + with pytest.raises(ValueError, match="not supported for Java"): + build_script_commands(tmp_path / "api_client.java", ("--x",)) + + def test_csharp_passes_args_after_separator(self, tmp_path): + from reverse_api.utils import build_script_commands + script = tmp_path / "api_client.cs" + steps, tool = build_script_commands(script, ("--x",)) + assert steps == [["dotnet", "run", "--project", str(tmp_path / "ApiClient.csproj"), "--", "--x"]] + assert tool == "dotnet" + + def test_php(self, tmp_path): + from reverse_api.utils import build_script_commands + script = tmp_path / "api_client.php" + steps, tool = build_script_commands(script) + assert steps == [["php", str(script)]] + assert tool == "php" + + def test_ruby(self, tmp_path): + from reverse_api.utils import build_script_commands + script = tmp_path / "api_client.rb" + steps, tool = build_script_commands(script) + assert steps == [["ruby", str(script)]] + assert tool == "ruby" + + def test_c_compiles_then_runs_with_vendored_cjson(self, tmp_path): + from reverse_api.utils import build_script_commands + script = tmp_path / "api_client.c" + (tmp_path / "cJSON.c").write_text("") + steps, tool = build_script_commands(script, ("--x",)) + binary = str(tmp_path / "api_client") + assert steps == [ + ["cc", str(script), str(tmp_path / "cJSON.c"), "-lcurl", "-o", binary], + [binary, "--x"], + ] + assert tool == "cc" + + def test_c_without_cjson(self, tmp_path): + from reverse_api.utils import build_script_commands + script = tmp_path / "api_client.c" + steps, _ = build_script_commands(script) + assert steps[0] == ["cc", str(script), "-lcurl", "-o", str(tmp_path / "api_client")] + + def test_unsupported_extension_raises(self, tmp_path): + from reverse_api.utils import build_script_commands + with pytest.raises(ValueError, match="unsupported script type"): + build_script_commands(tmp_path / "api_client.xyz") From 38fef01ff5715038ac5470434537661e798b298f Mon Sep 17 00:00:00 2001 From: kalil0321 Date: Wed, 22 Jul 2026 13:43:48 +0200 Subject: [PATCH 2/3] Extend run-dispatch tests and fix missing-tool error classification - The missing-toolchain message contained 'not found', which _classify_error's heuristics remap to engine_failure, overriding the config_invalid hint. Reworded so the intended kind survives. - Unit tests for _quote_path on both platforms (list2cmdline quoting on win32, shlex on POSIX) and for spaced paths surviving intact in the dispatch argv lists. Live matrix via the CLI: go/php/c/csharp/ruby clients from the language-PR test sessions plus hand-written js/ts clients all ran with exit 0, including --file selection, --ls listing, args passthrough (node), the Java script-args misuse error, and the missing-tool config_invalid error under a stripped PATH. --- src/reverse_api/cli.py | 4 ++-- tests/test_base_engineer.py | 18 ++++++++++++++++++ tests/test_run_command.py | 10 ++++++++++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/reverse_api/cli.py b/src/reverse_api/cli.py index 9dce4049..52e4555b 100644 --- a/src/reverse_api/cli.py +++ b/src/reverse_api/cli.py @@ -2675,7 +2675,7 @@ def _run_non_python_script(script, script_args) -> None: except ValueError as e: raise click.ClickException(str(e)) from e if shutil.which(tool) is None: - raise click.ClickException(f"'{tool}' not found on PATH (required to run {script.name})") + raise click.ClickException(f"cannot run {script.name}: '{tool}' is missing from PATH — install it and retry") returncode = 0 for cmd in steps: returncode = subprocess.run(cmd, cwd=str(script.parent)).returncode @@ -2774,7 +2774,7 @@ def _run_script_machine_payload( script_path=str(script), script_args=script_args, scripts=scripts, - error=f"'{tool}' not found on PATH (required to run {script.name})", + error=f"cannot run {script.name}: '{tool}' is missing from PATH — install it and retry", error_kind_hint="config_invalid", ) result = None diff --git a/tests/test_base_engineer.py b/tests/test_base_engineer.py index 3962eb64..3f7bc445 100644 --- a/tests/test_base_engineer.py +++ b/tests/test_base_engineer.py @@ -432,6 +432,24 @@ def test_get_run_command_unknown(self, tmp_path): eng = self._make_engineer(tmp_path, output_language="rust") assert eng._get_run_command() == "python api_client.py" + def test_quote_path_posix(self, monkeypatch): + """POSIX platforms use shlex.quote (single quotes for spaces).""" + from reverse_api import base_engineer as be + + monkeypatch.setattr(be.sys, "platform", "linux") + from reverse_api.base_engineer import BaseEngineer + + assert BaseEngineer._quote_path("/tmp/my dir/pom.xml") == "'/tmp/my dir/pom.xml'" + + def test_quote_path_windows(self, monkeypatch): + """Windows uses list2cmdline double-quoting that cmd.exe/PowerShell parse.""" + from reverse_api import base_engineer as be + + monkeypatch.setattr(be.sys, "platform", "win32") + from reverse_api.base_engineer import BaseEngineer + + assert BaseEngineer._quote_path(r"C:\Users\John Smith\pom.xml") == '"C:\\Users\\John Smith\\pom.xml"' + class TestBaseEngineerBuildPrompt: """Test _build_analysis_prompt method.""" diff --git a/tests/test_run_command.py b/tests/test_run_command.py index 0213dbb2..9bb30cfb 100644 --- a/tests/test_run_command.py +++ b/tests/test_run_command.py @@ -807,3 +807,13 @@ def test_unsupported_extension_raises(self, tmp_path): from reverse_api.utils import build_script_commands with pytest.raises(ValueError, match="unsupported script type"): build_script_commands(tmp_path / "api_client.xyz") + + def test_paths_with_spaces_stay_intact_in_argv(self, tmp_path): + """argv lists are passed to subprocess without a shell, so spaced + paths must survive as single arguments — no quoting layer involved.""" + from reverse_api.utils import build_script_commands + d = tmp_path / "my scripts" + d.mkdir() + script = d / "api_client.php" + steps, _ = build_script_commands(script) + assert steps == [["php", str(script)]] From 21fc967e38601d55874027b27340204e61c061c5 Mon Sep 17 00:00:00 2001 From: kalil0321 Date: Wed, 22 Jul 2026 13:47:32 +0200 Subject: [PATCH 3/3] Address review: fix path-component exclusion, resolve relative scripts, document quoting boundary - discover_scripts filtered on every component of the full path, so a custom output_dir under a directory named like a build dir (e.g. /tmp/target/...) excluded all scripts. iterdir() doesn't recurse and non-files are already skipped, so the parts-based filter could only misfire - removed it. - build_script_commands now resolves the script path before building argv: callers run with cwd=script.parent, which would re-anchor a relative path (from a relative output_dir) and fail to start. - _quote_path docstring now states the Windows quoting boundary precisely: list2cmdline is cmd.exe/CreateProcess-safe; PowerShell additionally expands $/backtick inside double quotes and no quoting satisfies both shells for such paths. --- src/reverse_api/base_engineer.py | 7 ++++++- src/reverse_api/utils.py | 19 +++++++++++++------ tests/test_run_command.py | 22 ++++++++++++++++++++++ 3 files changed, 41 insertions(+), 7 deletions(-) diff --git a/src/reverse_api/base_engineer.py b/src/reverse_api/base_engineer.py index ff4caaef..2d32c6f6 100644 --- a/src/reverse_api/base_engineer.py +++ b/src/reverse_api/base_engineer.py @@ -480,7 +480,12 @@ def _quote_path(path) -> str: shlex.quote is POSIX-only: cmd.exe/PowerShell pass its single quotes through literally, so a spaced Windows path would break apart. - list2cmdline applies the double-quoting rules Windows shells parse. + list2cmdline applies the double-quoting rules cmd.exe/CreateProcess + parse. Known boundary: PowerShell still expands `$` and backtick + inside double quotes, and no quoting satisfies cmd.exe and PowerShell + simultaneously for such paths — cmd-safe is the chosen baseline, and + paths whose components contain `$`/backtick are not supported on + Windows. """ if sys.platform == "win32": return subprocess.list2cmdline([str(path)]) diff --git a/src/reverse_api/utils.py b/src/reverse_api/utils.py index 58e69dbe..9c9b9dd4 100644 --- a/src/reverse_api/utils.py +++ b/src/reverse_api/utils.py @@ -691,16 +691,19 @@ def discover_scripts(run_id: str, output_dir: str | None = None, run_metadata: d if not scripts_dir.exists(): return [] - exclude_dirs = {"__pycache__", ".venv", "node_modules", "bin", "obj", "target"} # Support files that share a script extension but are never the client # itself: package markers and the vendored cJSON library (C output). + # Build/venv directories (__pycache__, .venv, node_modules, bin, obj, + # target) need no explicit filter: iterdir() doesn't recurse, and + # matching names in *parent* components (e.g. a custom output_dir under + # /tmp/target) must not exclude anything. exclude_files = {"__init__.py", "cJSON.c", "cJSON.h"} - scripts = [] - for f in scripts_dir.iterdir(): - if f.is_file() and f.suffix in SCRIPT_EXTENSIONS and f.name not in exclude_files: - scripts.append(f) - scripts = [s for s in scripts if not any(part in exclude_dirs for part in s.parts)] + scripts = [ + f + for f in scripts_dir.iterdir() + if f.is_file() and f.suffix in SCRIPT_EXTENSIONS and f.name not in exclude_files + ] return sorted(scripts, key=lambda p: p.name) @@ -726,6 +729,10 @@ def build_script_commands(script: Path, script_args: tuple[str, ...] = ()) -> tu ValueError: For unsupported extensions, or script_args with a Java client (mvn exec's program arguments are fixed in the pom). """ + # Resolve before building argv: callers run these with cwd=script.parent, + # and a relative script path (from a relative output_dir) would otherwise + # be re-resolved by the child against that new cwd and fail to start. + script = script.resolve() d = script.parent suffix = script.suffix if suffix == ".js": diff --git a/tests/test_run_command.py b/tests/test_run_command.py index 9bb30cfb..c0981fe2 100644 --- a/tests/test_run_command.py +++ b/tests/test_run_command.py @@ -280,6 +280,16 @@ def test_excludes_build_dirs(self, scripts_dir_empty, tmp_path): scripts = discover_scripts("abc123def456") assert [s.name for s in scripts] == ["api_client.cs"] + def test_output_dir_under_build_dir_name_still_discovers(self, tmp_path): + """A custom output_dir whose *parent* path contains a build-dir name + (e.g. /tmp/target/...) must not exclude the run's scripts.""" + out = tmp_path / "target" / "myout" + d = out / "scripts" / "run1" + d.mkdir(parents=True) + (d / "api_client.go").write_text("") + scripts = discover_scripts("run1", output_dir=str(out)) + assert [s.name for s in scripts] == ["api_client.go"] + def test_excludes_non_script_support_files(self, scripts_dir_empty, tmp_path): (scripts_dir_empty / "api_client.java").write_text("") (scripts_dir_empty / "pom.xml").write_text("") @@ -808,6 +818,18 @@ def test_unsupported_extension_raises(self, tmp_path): with pytest.raises(ValueError, match="unsupported script type"): build_script_commands(tmp_path / "api_client.xyz") + def test_relative_script_path_resolved_to_absolute(self, tmp_path, monkeypatch): + """Relative script paths (from a relative output_dir) must become + absolute in the argv: callers run with cwd=script.parent, which would + otherwise re-anchor the relative path and fail to start.""" + from reverse_api.utils import build_script_commands + monkeypatch.chdir(tmp_path) + d = Path("out") / "scripts" / "run1" + d.mkdir(parents=True) + script = d / "api_client.js" + steps, _ = build_script_commands(script) + assert steps == [["node", str(tmp_path.resolve() / d / "api_client.js")]] + def test_paths_with_spaces_stay_intact_in_argv(self, tmp_path): """argv lists are passed to subprocess without a shell, so spaced paths must survive as single arguments — no quoting layer involved."""