diff --git a/src/main.py b/src/main.py index 6727aa3..5ba863a 100644 --- a/src/main.py +++ b/src/main.py @@ -203,13 +203,28 @@ def build_parser() -> argparse.ArgumentParser: return parser +class _ArgparseError(Exception): + """#179: internal exception capturing argparse's real error message. + + Subclassed ArgumentParser raises this instead of printing + exiting, + so JSON mode can preserve the actual error (e.g. 'the following arguments + are required: session_id') in the envelope. + """ + def __init__(self, message: str) -> None: + super().__init__(message) + self.message = message + + def _emit_parse_error_envelope(argv: list[str], message: str) -> None: - """#178: emit JSON envelope for argparse-level errors when --output-format json is requested. + """#178/#179: emit JSON envelope for argparse-level errors when --output-format json is requested. Pre-scans argv for --output-format json. If found, prints a parse-error envelope to stdout (per SCHEMAS.md 'error' envelope shape) instead of letting argparse dump help text to stderr. This preserves the JSON contract for claws that can't parse argparse usage messages. + + #179 update: `message` now carries argparse's actual error text, not a generic + rejection string. Stderr is fully suppressed in JSON mode. """ import json # Extract the attempted command (argv[0] is the first positional) @@ -246,16 +261,35 @@ def main(argv: list[str] | None = None) -> int: if argv is None: argv = sys.argv[1:] parser = build_parser() - # #178: catch argparse errors and emit JSON envelope when --output-format json present - try: - args = parser.parse_args(argv) - except SystemExit as exc: - # argparse exits with SystemExit on error (code 2) or --help (code 0) - if exc.code != 0 and _wants_json_output(argv): - # Reconstruct a generic parse-error message - _emit_parse_error_envelope(argv, 'invalid command or argument (argparse rejection)') + json_mode = _wants_json_output(argv) + # #178/#179: capture argparse errors with real message and emit JSON envelope + # when --output-format json is requested. In JSON mode, stderr is silenced + # so claws only see the envelope on stdout. + if json_mode: + # Monkey-patch parser.error to raise instead of print+exit. This preserves + # the original error message text (e.g. 'argument X: invalid choice: ...'). + original_error = parser.error + def _json_mode_error(message: str) -> None: + raise _ArgparseError(message) + parser.error = _json_mode_error # type: ignore[method-assign] + # Also patch all subparsers + for action in parser._actions: + if hasattr(action, 'choices') and isinstance(action.choices, dict): + for subp in action.choices.values(): + subp.error = _json_mode_error # type: ignore[method-assign] + try: + args = parser.parse_args(argv) + except _ArgparseError as err: + _emit_parse_error_envelope(argv, err.message) return 1 - raise + except SystemExit as exc: + # Defensive: if argparse exits via some other path (e.g. --help in JSON mode) + if exc.code != 0: + _emit_parse_error_envelope(argv, 'argparse exited with non-zero code') + return 1 + raise + else: + args = parser.parse_args(argv) manifest = build_port_manifest() if args.command == 'summary': print(QueryEnginePort(manifest).render_summary()) diff --git a/tests/test_parse_error_envelope.py b/tests/test_parse_error_envelope.py index 3efe7c1..c76134d 100644 --- a/tests/test_parse_error_envelope.py +++ b/tests/test_parse_error_envelope.py @@ -157,3 +157,83 @@ class TestParseErrorSchemaCompliance: ) envelope = json.loads(result.stdout) assert envelope['error']['retryable'] is False + + +class TestParseErrorStderrHygiene: + """#179: JSON mode must fully suppress argparse stderr output. + + Before #179: stderr leaked argparse usage + error text even when --output-format json. + After #179: stderr is silent; envelope carries the real error message verbatim. + """ + + def test_json_mode_stderr_is_silent_on_unknown_command(self) -> None: + """Unknown command in JSON mode: stderr empty.""" + result = subprocess.run( + CLI + ['nonexistent-cmd', '--output-format', 'json'], + cwd=REPO_ROOT, + capture_output=True, + text=True, + ) + assert result.stderr == '', ( + f"JSON mode stderr must be empty; got:\n{result.stderr!r}" + ) + + def test_json_mode_stderr_is_silent_on_missing_arg(self) -> None: + """Missing required arg in JSON mode: stderr empty (no argparse usage leak).""" + result = subprocess.run( + CLI + ['load-session', '--output-format', 'json'], + cwd=REPO_ROOT, + capture_output=True, + text=True, + ) + assert result.stderr == '', ( + f"JSON mode stderr must be empty on missing arg; got:\n{result.stderr!r}" + ) + + def test_json_mode_envelope_carries_real_argparse_message(self) -> None: + """#179: envelope.error.message contains argparse's actual text, not generic rejection.""" + result = subprocess.run( + CLI + ['load-session', '--output-format', 'json'], + cwd=REPO_ROOT, + capture_output=True, + text=True, + ) + envelope = json.loads(result.stdout) + # Real argparse message: 'the following arguments are required: session_id' + msg = envelope['error']['message'] + assert 'session_id' in msg, ( + f"envelope.error.message must carry real argparse text mentioning missing arg; got: {msg!r}" + ) + assert 'required' in msg.lower(), ( + f"envelope.error.message must indicate what is required; got: {msg!r}" + ) + + def test_json_mode_envelope_carries_invalid_choice_details(self) -> None: + """#179: unknown command envelope includes valid-choice list from argparse.""" + result = subprocess.run( + CLI + ['typo-command', '--output-format', 'json'], + cwd=REPO_ROOT, + capture_output=True, + text=True, + ) + envelope = json.loads(result.stdout) + msg = envelope['error']['message'] + assert 'invalid choice' in msg.lower(), ( + f"envelope must mention 'invalid choice'; got: {msg!r}" + ) + # Should include at least one valid command name for discoverability + assert 'bootstrap' in msg or 'summary' in msg, ( + f"envelope must include valid choices for discoverability; got: {msg!r}" + ) + + def test_text_mode_stderr_preserved_on_unknown_command(self) -> None: + """Text mode: argparse stderr behavior unchanged (backward compat).""" + result = subprocess.run( + CLI + ['nonexistent-cmd'], + cwd=REPO_ROOT, + capture_output=True, + text=True, + ) + # Text mode still dumps argparse help to stderr + assert 'invalid choice' in result.stderr + assert result.returncode == 2