fix(cli): propagate subcommand exit codes from the top-level dispatch - #96
Merged
Conversation
Found while auditing the crm plugin (#88): `hermes crm show nobody` prints "Unknown contact: nobody" and exits 0, because main()'s top-level dispatch called `args.func(args)` and discarded whatever it returned. This isn't crm-specific — every command handler that follows the 0/1/2 exit-code convention (plugin CLI commands, `hermes kanban`, `hermes migrate`, `hermes slack`) was silently losing its exit code the same way, breaking `$?`-based scripting and cron error detection for all of them. Propagate a non-None, non-zero int return via sys.exit(). Verified safe against a full audit of hermes_cli/main.py's ~49 set_defaults(func=...) registrations: no handler returns a non-zero int for a reason other than "this is a real error" (most wrappers call-and-discard internally and stay None regardless; the ones with explicit returns already follow the 0/1/2 convention). Adds a subprocess-level regression test file exercising the real `python -m hermes_cli.main` entry point (0/1/2 via the crm plugin, since its exit-code contract is well-defined), and tightens an existing kanban test that had an explicit comment documenting the swallow-behavior — it was checking side effects only because there was nothing else to check; now it also asserts the exit code. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bym5HhKPQ3CWb5r9bCvBq4
🔎 Lint report:
|
Pre-existing, unrelated to this PR's exit-code changes — found while triaging CI red across the fan-out audit follow-up PRs. _setup_standard_platform() (hermes_cli/gateway.py) calls its own imported prompt_yes_no, not hermes_cli.setup's. Since Matrix now shows as "already configured" in these tests' env fixture, _configure_platform() reaches the "Reconfigure Matrix?" prompt, which was only mocked on the setup_mod side — the real gateway_mod reference fell through to a live input() call and crashed under pytest's captured stdout with "reading from stdin while output is captured". Mock gateway_mod.prompt_yes_no too, matching the pattern PR #93 independently arrived at for the same root cause. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bym5HhKPQ3CWb5r9bCvBq4
dizhaky
marked this pull request as ready for review
July 17, 2026 16:02
dizhaky
added a commit
that referenced
this pull request
Jul 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Found while auditing the
crmplugin (#88):hermes crm show nobodyprints"Unknown contact: nobody"and exits 0, becausemain()'s top-level dispatch calledargs.func(args)and discarded whatever it returned.This isn't
crm-specific — every command handler that follows the 0/1/2 exit-code convention (plugin CLI commands,hermes kanban,hermes migrate,hermes slack) was silently losing its exit code the same way, breaking$?-based scripting and cron error detection for all of them.Related Issue
Fixes #
Type of Change
Changes Made
hermes_cli/main.py: propagate a non-None, non-zero int return fromargs.func(args)viasys.exit().tests/hermes_cli/test_cli_exit_code_propagation.py(new): subprocess-level tests against the realpython -m hermes_cli.mainentry point covering the 0/1/2 convention via thecrmplugin (whose exit-code contract is well-defined and easy to isolate).tests/hermes_cli/test_kanban_core_functionality.py: tightened an existing test that had an explicit comment documenting the old swallow-behavior (it was checking side effects only because there was nothing else to check) — now also asserts the exit code.How to Test
pytest tests/hermes_cli/test_cli_exit_code_propagation.py -q→ 3 passed.pytest tests/hermes_cli/test_kanban_core_functionality.py tests/hermes_cli/test_kanban_cli.py tests/hermes_cli/test_migrate_xai.py tests/hermes_cli/test_slack_cli.py tests/hermes_cli/test_plugin_cli_registration.py -q→ 254 passed, confirming no regression across every handler category that has explicit test coverage of its return value.hermes crm show nobody; echo $?→ prints the error, exits1(was0).Safety analysis
This changes the dispatch for all ~49
set_defaults(func=...)registrations inhermes_cli/main.py, not justcrm, so before applying it I had a subagent trace every one of them to their actual definitions and check for any handler that returns a non-zero int for a reason other than signaling a real error (e.g. a count, a boolean-as-int, an object). None were found — every handler either implicitly returnsNone(most corecmd_*wrappers call-and-discard internally) or already follows the 0/1/2 convention (plugin commands,kanban,migrate,slack).cmd_proxyalready converts its own non-zero return toSystemExitbefore returning, so it's unaffected either way.Checklist
Code
fix(cli): …)ruff checkpassesDocumentation & Housekeeping
cli-config.yaml.example— N/A (no config keys changed)🤖 Generated with Claude Code
https://claude.ai/code/session_01Bym5HhKPQ3CWb5r9bCvBq4
Generated by Claude Code