mirror of
https://github.com/ultraworkers/claw-code.git
synced 2026-04-24 13:08:11 +08:00
fix: #179 — JSON mode now fully suppresses argparse stderr + preserves real error message
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).
This commit is contained in:
YeonGyu-Kim
parent
517d7e224e
commit
8247d7d2eb
54
src/main.py
54
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())
|
||||
|
||||
@ -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
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user