From a6b8ad57fd1549f8fdeaa67801c4a530bafb91b8 Mon Sep 17 00:00:00 2001 From: Wolfvin Date: Sat, 18 Jul 2026 09:10:59 +0700 Subject: [PATCH] fix(cli): report arg errors on stdout as JSON for machine formats (closes #315) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An argparse error printed to stderr and left stdout empty, so an agent parsing stdout saw "no results" instead of an error — the silent-wrong class of #293/#300. A _StdoutErrorParser now emits {"s":"error",...} on stdout for machine formats (json/compact/ai/sarif/...) while keeping the friendly stderr message for humans. Found by dogfooding: `search . --mode symbol execute` (the workspace-positional form other umbrellas accept) returned zero bytes on stdout. Exposed three vacuous tests in test_hybrid_engine: they invoked pre-#195 forms (`impact X `, the dropped `dead-code `) that always errored, and passed only because the error went to stderr and their `if idx >= 0` guard skipped the assert. Fixed to valid post-#195 umbrella invocations that actually assert confidence — three vacuous tests become three real ones. Added timeout=60 to every subprocess in the file so a hang (cf #303) fails fast instead of running to the 6h ceiling. Verified: trap case now emits stdout JSON error; human mode unchanged; valid calls unaffected; 4 new CLI-error tests; full suite 19 = 19 on main. Co-Authored-By: Claude Opus 4.8 --- scripts/codelens.py | 38 +++++++++++++++++++- tests/test_cli_error_format.py | 65 ++++++++++++++++++++++++++++++++++ tests/test_hybrid_engine.py | 62 +++++++++++++++++--------------- 3 files changed, 136 insertions(+), 29 deletions(-) create mode 100644 tests/test_cli_error_format.py diff --git a/scripts/codelens.py b/scripts/codelens.py index 2c3d440..58e2f82 100755 --- a/scripts/codelens.py +++ b/scripts/codelens.py @@ -1076,6 +1076,41 @@ def _check_staleness(workspace: str, args) -> Optional[Dict[str, Any]]: # ─── CLI Entry Point ────────────────────────────────────────── +# Formats an agent parses off stdout. For these, a CLI arg error must appear +# on stdout as JSON — not on stderr with an empty stdout, which reads as +# "no results" and silently misleads the agent (issue #315). +_MACHINE_FORMATS = frozenset({ + "json", "compact", "ai", "sarif", "graphml", "junit-xml", "gitlab-sast", +}) + + +def _argv_format(argv): + """The --format value from a raw argv, or None.""" + for i, a in enumerate(argv): + if a == "--format" and i + 1 < len(argv): + return argv[i + 1] + if a.startswith("--format="): + return a.split("=", 1)[1] + return None + + +class _StdoutErrorParser(argparse.ArgumentParser): + """ArgumentParser that reports arg errors on stdout as JSON for machine + formats, so an agent can always tell an error from an empty result.""" + + def error(self, message): + if _argv_format(sys.argv) in _MACHINE_FORMATS: + import json as _json + print(_json.dumps({ + "s": "error", + "error": message, + "error_type": "cli_argument", + "usage": self.format_usage().strip(), + })) + self.exit(2) + super().error(message) + + def main(): # Command count is derived from the visible (non-hidden) command set at # runtime so it can never drift from the actual number of umbrella @@ -1090,7 +1125,7 @@ def main(): _visible_registry = _get_visible_commands() _command_count = len(_visible_registry) - parser = argparse.ArgumentParser( + parser = _StdoutErrorParser( description=( f"CodeLens v{CODELENS_VERSION} — Live Codebase Reference Intelligence " f"(Tree-sitter Edition). {_command_count} commands available; run " @@ -1109,6 +1144,7 @@ def main(): ) subparsers = parser.add_subparsers( dest="command", + parser_class=_StdoutErrorParser, help="Available commands", # Issue #195: the default metavar lists every choice including # hidden commands. Override with only the 12 visible umbrella diff --git a/tests/test_cli_error_format.py b/tests/test_cli_error_format.py new file mode 100644 index 0000000..5b31780 --- /dev/null +++ b/tests/test_cli_error_format.py @@ -0,0 +1,65 @@ +""" +Tests for CLI arg errors surfacing on stdout for machine formats (issue #315). + +An argparse error used to print to stderr and leave stdout empty, so an agent +parsing stdout saw "no results" instead of an error. For machine formats the +error must appear on stdout as JSON. + +Subprocess-based (argparse lives at the process entry point). Every call passes +timeout= so a hang fails fast rather than stalling the suite (the #303 lesson). +""" + +import json +import os +import subprocess +import sys + +import pytest + +CODELENS = os.path.join( + os.path.dirname(os.path.dirname(os.path.abspath(__file__))), + "scripts", "codelens.py", +) + + +def _run(args): + return subprocess.run( + [sys.executable, CODELENS, *args], + capture_output=True, text=True, timeout=60, + ) + + +def test_arg_error_is_json_on_stdout_for_compact(): + # `.` is taken as the pattern; `execute` is an unrecognized extra arg. + r = _run(["search", ".", "--mode", "symbol", "execute", "--format", "compact"]) + + assert r.stdout.strip(), "stdout must not be empty on a CLI arg error" + payload = json.loads(r.stdout) + assert payload["s"] == "error" + assert payload["error_type"] == "cli_argument" + + +def test_arg_error_is_json_on_stdout_for_json(): + r = _run(["search", ".", "--mode", "symbol", "execute", "--format", "json"]) + + payload = json.loads(r.stdout) + assert payload["s"] == "error" + + +def test_human_mode_keeps_error_on_stderr(): + """No machine format: the friendly stderr behaviour is unchanged.""" + r = _run(["search", ".", "--mode", "symbol", "execute"]) + + assert "unrecognized arguments" in r.stderr + assert not r.stdout.strip() + + +def test_valid_call_is_unaffected(): + """The error path must not touch normal parsing.""" + r = _run(["--command-count"]) + assert r.returncode == 0 + assert r.stdout.strip().isdigit() + + +if __name__ == "__main__": + pytest.main([__file__, "-v"]) diff --git a/tests/test_hybrid_engine.py b/tests/test_hybrid_engine.py index cbbd545..42dddb7 100755 --- a/tests/test_hybrid_engine.py +++ b/tests/test_hybrid_engine.py @@ -86,7 +86,7 @@ class TestHybridIntegration: def test_lsp_status_cli(self): result = subprocess.run( [sys.executable, os.path.join(SCRIPT_DIR, "codelens.py"), "--lsp-status"], - capture_output=True, text=True, cwd=CODELENS_ROOT + capture_output=True, text=True, cwd=CODELENS_ROOT, timeout=60 ) combined = result.stdout + result.stderr assert "pyright" in combined or result.returncode == 0 @@ -95,7 +95,7 @@ def test_query_confidence_without_deep(self): result = subprocess.run( [sys.executable, os.path.join(SCRIPT_DIR, "codelens.py"), "query", "detect_dead_code", CODELENS_ROOT, "--format", "json"], - capture_output=True, text=True + capture_output=True, text=True, timeout=60 ) idx = result.stdout.find("{") if idx >= 0: @@ -104,27 +104,31 @@ def test_query_confidence_without_deep(self): assert data["confidence"] in ("high", "medium", "low") def test_impact_confidence_without_deep(self): + # Post-#195 umbrella form: `impact --name X` (was `impact X `, + # which errors — the old form left stdout empty so this test passed + # vacuously until #315 surfaced the error on stdout). result = subprocess.run( [sys.executable, os.path.join(SCRIPT_DIR, "codelens.py"), - "impact", "detect_dead_code", CODELENS_ROOT, "--format", "json"], - capture_output=True, text=True + "impact", CODELENS_ROOT, "--name", "detect_dead_code", "--format", "json"], + capture_output=True, text=True, timeout=60, ) idx = result.stdout.find("{") - if idx >= 0: - data = json.loads(result.stdout[idx:]) - assert "confidence" in data + assert idx >= 0, result.stdout + result.stderr + data = json.loads(result.stdout[idx:]) + assert "confidence" in data["r"][0] def test_ai_format_confidence_distribution(self): + # Post-#195: `audit --check dead-code` (was the dropped `dead-code` + # top-level command). ai stats are namespaced per check (#306). result = subprocess.run( [sys.executable, os.path.join(SCRIPT_DIR, "codelens.py"), - "dead-code", CODELENS_ROOT, "--format", "ai", "--top", "5"], - capture_output=True, text=True + "audit", CODELENS_ROOT, "--check", "dead-code", "--format", "ai", "--top", "5"], + capture_output=True, text=True, timeout=60, ) idx = result.stdout.find("{") - if idx >= 0: - data = json.loads(result.stdout[idx:]) - stats = data.get("stats", {}) - assert "confidence_distribution" in stats + assert idx >= 0, result.stdout + result.stderr + data = json.loads(result.stdout[idx:]) + assert "confidence_distribution" in data["stats"]["dead-code"] def test_deep_with_pyright(self): from lsp_client import detect_available_servers @@ -134,7 +138,7 @@ def test_deep_with_pyright(self): result = subprocess.run( [sys.executable, os.path.join(SCRIPT_DIR, "codelens.py"), "query", "detect_dead_code", CODELENS_ROOT, "--deep", "--format", "json"], - capture_output=True, text=True + capture_output=True, text=True, timeout=60 ) idx = result.stdout.find("{") if idx >= 0: @@ -143,23 +147,25 @@ def test_deep_with_pyright(self): assert data.get("lsp_active") is True def test_dead_code_confidence_fields(self): + # Post-#195: `audit --check dead-code`; results live under the sub-check + # envelope r[0]. result = subprocess.run( [sys.executable, os.path.join(SCRIPT_DIR, "codelens.py"), - "dead-code", CODELENS_ROOT, "--format", "json", "--top", "5"], - capture_output=True, text=True + "audit", CODELENS_ROOT, "--check", "dead-code", "--format", "json", "--top", "5"], + capture_output=True, text=True, timeout=60, ) idx = result.stdout.find("{") - if idx >= 0: - data = json.loads(result.stdout[idx:]) - results = data.get("results", {}) - found_confidence = False - for cat, items in results.items(): - if isinstance(items, list): - for item in items[:3]: - if isinstance(item, dict) and "confidence" in item: - found_confidence = True - break - assert found_confidence + assert idx >= 0, result.stdout + result.stderr + data = json.loads(result.stdout[idx:]) + results = data["r"][0].get("results", {}) + found_confidence = False + for cat, items in results.items(): + if isinstance(items, list): + for item in items[:3]: + if isinstance(item, dict) and "confidence" in item: + found_confidence = True + break + assert found_confidence def test_deep_graceful_degradation(self): import tempfile @@ -171,7 +177,7 @@ def test_deep_graceful_degradation(self): result = subprocess.run( [sys.executable, os.path.join(SCRIPT_DIR, "codelens.py"), "audit", tmpdir, "--check", "dead-code", "--deep"], - capture_output=True, text=True + capture_output=True, text=True, timeout=60 ) assert result.returncode == 0