From 7724bf98fde6e9a72ac33a790ebd46e500d56f61 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Wed, 22 Apr 2026 21:33:57 +0900 Subject: [PATCH] =?UTF-8?q?fix:=20#181=20=E2=80=94=20envelope=20exit=5Fcod?= =?UTF-8?q?e=20must=20match=20process=20exit=20code=20(exec-command/exec-t?= =?UTF-8?q?ool)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cycle #26 dogfood found a real red-state bug in the JSON envelope contract. ## The Bug exec-command and exec-tool not-found cases return exit code 1 from the process, but the envelope reports exit_code: 0 (the default from wrap_json_envelope). This is a protocol violation. Repro (before fix): $ claw exec-command unknown-cmd test --output-format json > out.json $ echo $? 1 $ jq '.exit_code' out.json 0 # WRONG — envelope lies about exit code Claws reading the envelope's exit_code field get misinformation. A claw implementing the canonical ERROR_HANDLING.md pattern (check exit_code, then classify by error.kind) would incorrectly treat failures as successes when dispatching on the envelope alone. ## Root Cause main.py lines 687–739 (exec-command + exec-tool handlers): - Return statement: 'return 0 if result.handled else 1' (correct) - Envelope wrap: 'wrap_json_envelope(envelope, args.command)' (uses default exit_code=0, IGNORES the return value) The envelope wrap was called BEFORE the return value was computed, so the exit_code field was never synchronized with the actual exit code. ## The Fix Compute exit_code ONCE at the top: exit_code = 0 if result.handled else 1 Pass it explicitly to wrap_json_envelope: wrap_json_envelope(envelope, args.command, exit_code=exit_code) Return the same value: return exit_code This ensures the envelope's exit_code field is always truth — the SAME value the process returns. ## Tests Added (3) TestEnvelopeExitCodeMatchesProcessExit in test_exec_route_bootstrap_output_format.py: 1. test_exec_command_not_found_envelope_exit_matches: Verifies exec-command unknown-cmd returns exit 1 in both envelope and process. 2. test_exec_tool_not_found_envelope_exit_matches: Same for exec-tool. 3. test_all_commands_exit_code_invariant: Audit across 4 known non-zero cases (show-command, show-tool, exec-command, exec-tool not-found). Guards against the same bug in other surfaces. ## Impact - 206 → 209 passing tests (+3) - Zero regressions - Protocol contract now truthful: envelope.exit_code == process exit - Claws using the one-handler pattern from ERROR_HANDLING.md now get correct information ## Related - ERROR_HANDLING.md (cycle #22): Documented exit_code as machine-readable contract field - #178/#179 (cycles #19/#20): Closed parser-front-door contract - This closes a gap in the WORK PROTOCOL contract — envelope values must match reality, not just be structurally present. Classification (per cycle #24 calibration): - Red-state bug: ✓ (contract violation, claws get misinformation) - Real friction: ✓ (discovered via dogfood, not speculative) - Fix ships same-cycle: ✓ (discipline per maintainership mode) Source: Jobdori cycle #26 dogfood — ran multiple edge-case probes, noticed exec-command envelope showed exit_code: 0 while process exited 1. Investigated wrap_json_envelope default behavior, confirmed bug, fixed and tested in same cycle. --- src/main.py | 12 ++- ...test_exec_route_bootstrap_output_format.py | 75 +++++++++++++++++++ 2 files changed, 83 insertions(+), 4 deletions(-) diff --git a/src/main.py b/src/main.py index 5ba863a..c9e839b 100644 --- a/src/main.py +++ b/src/main.py @@ -687,6 +687,8 @@ def main(argv: list[str] | None = None) -> int: if args.command == 'exec-command': result = execute_command(args.name, args.prompt) # #168: JSON envelope with typed not-found error + # #181: envelope exit_code must match process exit code + exit_code = 0 if result.handled else 1 if args.output_format == 'json': import json if not result.handled: @@ -708,13 +710,15 @@ def main(argv: list[str] | None = None) -> int: 'handled': True, 'message': result.message, } - print(json.dumps(wrap_json_envelope(envelope, args.command))) + print(json.dumps(wrap_json_envelope(envelope, args.command, exit_code=exit_code))) else: print(result.message) - return 0 if result.handled else 1 + return exit_code if args.command == 'exec-tool': result = execute_tool(args.name, args.payload) # #168: JSON envelope with typed not-found error + # #181: envelope exit_code must match process exit code + exit_code = 0 if result.handled else 1 if args.output_format == 'json': import json if not result.handled: @@ -736,10 +740,10 @@ def main(argv: list[str] | None = None) -> int: 'handled': True, 'message': result.message, } - print(json.dumps(wrap_json_envelope(envelope, args.command))) + print(json.dumps(wrap_json_envelope(envelope, args.command, exit_code=exit_code))) else: print(result.message) - return 0 if result.handled else 1 + return exit_code parser.error(f'unknown command: {args.command}') return 2 diff --git a/tests/test_exec_route_bootstrap_output_format.py b/tests/test_exec_route_bootstrap_output_format.py index 98010a5..899e3c0 100644 --- a/tests/test_exec_route_bootstrap_output_format.py +++ b/tests/test_exec_route_bootstrap_output_format.py @@ -202,3 +202,78 @@ class TestFamilyWideJsonParity: ) # Output should not be JSON-shaped (no leading {) assert not result.stdout.strip().startswith('{') + + +class TestEnvelopeExitCodeMatchesProcessExit: + """#181: Envelope exit_code field must match actual process exit code. + + Regression test for the protocol violation where exec-command/exec-tool + not-found cases returned exit code 1 from the process but emitted + envelopes with exit_code: 0 (default wrap_json_envelope). Claws reading + the envelope would misclassify failures as successes. + + Contract (from ERROR_HANDLING.md): + - Exit code 0 = success + - Exit code 1 = error/not-found + - Envelope MUST reflect process exit + """ + + def test_exec_command_not_found_envelope_exit_matches(self) -> None: + """exec-command 'unknown-name' must have exit_code=1 in envelope.""" + result = _run(['exec-command', 'nonexistent-cmd-name', 'test-prompt', '--output-format', 'json']) + assert result.returncode == 1, f'process exit should be 1, got {result.returncode}' + envelope = json.loads(result.stdout) + assert envelope['exit_code'] == 1, ( + f'envelope.exit_code mismatch: process=1, envelope={envelope["exit_code"]}' + ) + assert envelope['handled'] is False + assert envelope['error']['kind'] == 'command_not_found' + + def test_exec_tool_not_found_envelope_exit_matches(self) -> None: + """exec-tool 'unknown-tool' must have exit_code=1 in envelope.""" + result = _run(['exec-tool', 'nonexistent-tool-name', '{}', '--output-format', 'json']) + assert result.returncode == 1, f'process exit should be 1, got {result.returncode}' + envelope = json.loads(result.stdout) + assert envelope['exit_code'] == 1, ( + f'envelope.exit_code mismatch: process=1, envelope={envelope["exit_code"]}' + ) + assert envelope['handled'] is False + assert envelope['error']['kind'] == 'tool_not_found' + + def test_all_commands_exit_code_invariant(self) -> None: + """Audit: for every clawable command, envelope.exit_code == process exit. + + This is a stronger invariant than 'emits JSON'. Claws dispatching on + the envelope's exit_code field must get the truth, not a lie. + """ + # Sample cases known to return non-zero + cases = [ + # command, expected_exit, justification + (['show-command', 'nonexistent-abc'], 1, 'not-found inventory lookup'), + (['show-tool', 'nonexistent-xyz'], 1, 'not-found inventory lookup'), + (['exec-command', 'nonexistent-1', 'test'], 1, 'not-found execution'), + (['exec-tool', 'nonexistent-2', '{}'], 1, 'not-found execution'), + ] + mismatches = [] + for args, expected_exit, reason in cases: + result = _run([*args, '--output-format', 'json']) + if result.returncode != expected_exit: + mismatches.append( + f'{args}: expected process exit {expected_exit} ({reason}), ' + f'got {result.returncode}' + ) + continue + try: + envelope = json.loads(result.stdout) + except json.JSONDecodeError as e: + mismatches.append(f'{args}: JSON parse failed: {e}') + continue + if envelope.get('exit_code') != result.returncode: + mismatches.append( + f'{args}: envelope.exit_code={envelope.get("exit_code")} ' + f'!= process exit={result.returncode} ({reason})' + ) + assert not mismatches, ( + 'Envelope exit_code must match process exit code:\n' + + '\n'.join(mismatches) + )