From 8247d7d2eba10219bda5d6d2f66e24f066803f73 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Wed, 22 Apr 2026 20:32:28 +0900 Subject: [PATCH] =?UTF-8?q?fix:=20#179=20=E2=80=94=20JSON=20mode=20now=20f?= =?UTF-8?q?ully=20suppresses=20argparse=20stderr=20+=20preserves=20real=20?= =?UTF-8?q?error=20message?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dogfood discovered #178 had two residual gaps: 1. Stderr pollution: argparse usage + error text still leaked to stderr even in JSON mode (envelope was correct on stdout, but stderr noise broke the 'machine-first protocol' contract — claws capturing both streams got dual output) 2. Generic error message: envelope carried 'invalid command or argument (argparse rejection)' instead of argparse's actual text like 'the following arguments are required: session_id' or 'invalid choice: typo (choose from ...)' Before #179: $ claw load-session --output-format json [stdout] {"error": {"message": "invalid command or argument (argparse rejection)"}} [stderr] usage: main.py load-session [-h] ... main.py load-session: error: the following arguments are required: session_id [exit 1] After #179: $ claw load-session --output-format json [stdout] {"error": {"message": "the following arguments are required: session_id"}} [stderr] (empty) [exit 1] Implementation: - New _ArgparseError exception class captures argparse's real message - main() monkey-patches parser.error (+ all subparser.error) in JSON mode to raise _ArgparseError instead of print-to-stderr + sys.exit(2) - _emit_parse_error_envelope() now receives the real message verbatim - Text mode path unchanged: still uses original argparse print+exit behavior Contract: - JSON mode: stdout carries envelope with argparse's actual error; stderr silent - Text mode: unchanged — argparse usage to stderr, exit 2 - Parse errors still error.kind='parse', retryable=false Test additions (5 new, 14 total in test_parse_error_envelope.py): - TestParseErrorStderrHygiene (5): - test_json_mode_stderr_is_silent_on_unknown_command - test_json_mode_stderr_is_silent_on_missing_arg - test_json_mode_envelope_carries_real_argparse_message - test_json_mode_envelope_carries_invalid_choice_details (verifies valid-choices list) - test_text_mode_stderr_preserved_on_unknown_command (backward compat) Operational impact: Claws capturing both stdout and stderr no longer get garbled output. The envelope message now carries discoverability info (valid command list, missing-arg name) that claws can use for retry/recovery without probing the CLI a second time. Test results: 201 → 206 passing, 3 skipped unchanged, zero regression. Pinpoint discovered via dogfood at 2026-04-22 20:30 KST (cycle #20). --- src/main.py | 54 ++++++++++++++++---- tests/test_parse_error_envelope.py | 80 ++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 10 deletions(-) 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