From 28835cb4068e517e3a7b50a20154e3dd41d2b805 Mon Sep 17 00:00:00 2001 From: pbean Date: Wed, 22 Jul 2026 08:46:54 -0700 Subject: [PATCH 1/2] feat(cli): name exit codes with an ExitCode enum + document them (#241) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add ExitCode(IntEnum) — OK=0, FAILURE=1, USAGE=2 — in cli.py and use it in main()'s two dispatch-tail return paths as named constants. Purely a naming + documentation change: the values are the existing released rc numbers, so behavior is unchanged (ExitCode.FAILURE == 1). - USAGE=2 names argparse's own SystemExit(2); it is documented, not rerouted. - Codes 3+ and INTERRUPTED (Ctrl+C -> 130) are deferred to the follow-up PR; the KeyboardInterrupt path is untouched here. - README gains an 'Exit codes' section under the scripting docs. - Tests assert the enum values, that no codes 3+ crept in, and that members return as ints; the existing rc characterization tests stay green. This is PR 1 of #241 (enum + docs, no behavior change). --- README.md | 14 ++++++++++++++ src/bmad_loop/cli.py | 29 +++++++++++++++++++++++++++-- tests/test_entry_point.py | 24 ++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index a836fb72..d9ada90d 100644 --- a/README.md +++ b/README.md @@ -519,6 +519,20 @@ Each run drives its agents inside a dedicated tmux session, `bmad-loop-` Two things to know when matching. `message` is **not** contracted — several problems are the raw text of an underlying config/policy/profile exception, so the wording moves with those modules; `check` is the matchable identity. And a check id's _absence_ is not a pass: the gates are chained, so a policy that fails to load leaves the binary, hook and skill gates with nothing to check, and they contribute no finding at all. Read `ok` for the verdict. +### Exit codes + +Every `bmad-loop` command exits with one of three codes — a released contract, safe to branch on in CI: + +| Code | Name | Meaning | +| ---- | --------- | --------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `0` | `OK` | the command did its job | +| `1` | `FAILURE` | the command could not do its job — a known error (bad config, git failure, unready project) or an unexpected one; the reason goes to stderr as `error: …` | +| `2` | `USAGE` | the invocation was malformed — an unknown subcommand or bad flag, caught by argument parsing before the command runs | + +No codes `≥3` are assigned yet. The names come from the `ExitCode` enum in `bmad_loop.cli`. + +One caveat repeated from above: a `--json` command reporting a **verdict** exits `1` to _carry_ that verdict, not because the command broke — `validate --json` on an unready project exits `1` but still prints its whole document. So for those commands read the document's own field (`ok`), not the exit status; `1` conflates "the checks failed" with "the command failed". For everything else, `0` vs non-zero is the answer. + ## Other coding CLIs One generic driver (`adapters/generic.py`) runs any coding CLI that fits the injection + hook-signal transport; everything CLI-specific lives in a declarative **profile** (`adapters/profile.py`), and the terminal transport itself sits behind a pluggable `TerminalMultiplexer` seam (tmux bundled; external backends like the herdr adapter install as packages — see [Terminal multiplexer backends](docs/multiplexer-backends.md)). Built-in profiles ship as TOML in `bmad_loop/data/profiles/`: diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index e2dcba22..4f8e1ac3 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -10,6 +10,7 @@ import subprocess import sys import time +from enum import IntEnum from pathlib import Path from . import ( @@ -66,6 +67,30 @@ POLICY_FILE = policy_mod.POLICY_FILE +class ExitCode(IntEnum): + """The process exit codes ``main()`` contracts — names for the released numbers. + + ``rc`` is a released contract, so these values do not move; the enum just gives + the existing numbers a name at their use sites. Being an ``IntEnum`` it returns + from ``main()`` (``-> int``) and reaches ``sys.exit`` transparently. + + - ``OK`` — a command handler returned success (``args.func`` returns it, not + ``main`` directly). + - ``FAILURE`` — a typed error surfaced to the dispatch tail, or the broad + backstop caught an unexpected exception; both print ``error: …`` to stderr. + - ``USAGE`` — an argparse usage error (unknown subcommand, bad flag). argparse + raises ``SystemExit(2)`` before dispatch, so this names its number rather than + being returned here. + + Codes ``3+`` are intentionally absent until a consumer needs one. ``INTERRUPTED`` + (Ctrl+C → 130) is deferred to its own change; today that path is unchanged. + """ + + OK = 0 + FAILURE = 1 + USAGE = 2 + + def _project(args: argparse.Namespace) -> Path: return Path(args.project).resolve() @@ -2575,13 +2600,13 @@ def add(name: str, func, help: str, *, aliases=()) -> argparse.ArgumentParser: verify.GitError, ) as e: print(f"error: {e}", file=sys.stderr) - return 1 + return ExitCode.FAILURE except Exception as e: # backstop for the residual surface outside engine.run() (config load, # engine construction, render/notify): never let an unexpected exception # die to the parked control pane with a bare traceback. print(f"error: {e}", file=sys.stderr) - return 1 + return ExitCode.FAILURE if __name__ == "__main__": diff --git a/tests/test_entry_point.py b/tests/test_entry_point.py index ef14e483..c72bccbb 100644 --- a/tests/test_entry_point.py +++ b/tests/test_entry_point.py @@ -117,3 +117,27 @@ def test_exit_argparse_usage_error_is_2(capsys): cli.main(["definitely-not-a-command"]) assert excinfo.value.code == 2 assert "invalid choice" in capsys.readouterr().err + + +# --------------------------------------------------------------------- ExitCode enum + + +def test_exit_code_enum_values(): + """``ExitCode`` names the released rc numbers — the values are the contract, so + pin them literally (a rename is fine; a renumber is a breaking change).""" + assert cli.ExitCode.OK == 0 + assert cli.ExitCode.FAILURE == 1 + assert cli.ExitCode.USAGE == 2 + + +def test_exit_code_enum_has_no_codes_3_plus(): + """Only OK/FAILURE/USAGE exist yet — codes 3+ (and INTERRUPTED=130) are deferred + until a consumer needs them, so guard against one sneaking in early.""" + assert {c.value for c in cli.ExitCode} == {0, 1, 2} + + +def test_exit_code_enum_is_int_returnable(): + """``main() -> int`` and ``sys.exit`` both consume the members transparently: + an ``IntEnum`` member *is* its int, so the failure paths can return it as-is.""" + assert isinstance(cli.ExitCode.FAILURE, int) + assert cli.ExitCode.FAILURE == 1 From bf194b6c41bedffc1fb12c761ff655e520605446 Mon Sep 17 00:00:00 2001 From: pbean Date: Wed, 22 Jul 2026 09:07:36 -0700 Subject: [PATCH 2/2] docs(cli): scope the exit-code table + note Ctrl+C=130 (#255 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit (Major): the 'every command exits with one of three codes' claim was wrong — an uncaught KeyboardInterrupt dies by SIGINT and exits 130, which is not in {0,1,2}. Narrow the contract to normal dispatch + argparse usage errors and document interruption as the one exception (130, a bare traceback today; #241 PR 2 makes it a clean message but keeps the code). 130 stays prose-only, not an ExitCode member: PR 1 must not add codes >=3 and INTERRUPTED is deferred to PR 2. --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index d9ada90d..8346b443 100644 --- a/README.md +++ b/README.md @@ -521,7 +521,7 @@ Two things to know when matching. `message` is **not** contracted — several pr ### Exit codes -Every `bmad-loop` command exits with one of three codes — a released contract, safe to branch on in CI: +Normal command dispatch and argparse usage errors resolve to one of three released codes — safe to branch on in CI: | Code | Name | Meaning | | ---- | --------- | --------------------------------------------------------------------------------------------------------------------------------------------------------- | @@ -529,7 +529,7 @@ Every `bmad-loop` command exits with one of three codes — a released contract, | `1` | `FAILURE` | the command could not do its job — a known error (bad config, git failure, unready project) or an unexpected one; the reason goes to stderr as `error: …` | | `2` | `USAGE` | the invocation was malformed — an unknown subcommand or bad flag, caught by argument parsing before the command runs | -No codes `≥3` are assigned yet. The names come from the `ExitCode` enum in `bmad_loop.cli`. +No `ExitCode` values `≥3` are assigned yet. The one path outside this table is **interruption**: Ctrl+C (SIGINT) terminates the process with **130** — today as a bare traceback; a follow-up ([#241](https://github.com/bmad-code-org/bmad-loop/issues/241) PR 2) replaces that with a clean one-line message, but the code stays 130. The names come from the `ExitCode` enum in `bmad_loop.cli`. One caveat repeated from above: a `--json` command reporting a **verdict** exits `1` to _carry_ that verdict, not because the command broke — `validate --json` on an unready project exits `1` but still prints its whole document. So for those commands read the document's own field (`ok`), not the exit status; `1` conflates "the checks failed" with "the command failed". For everything else, `0` vs non-zero is the answer.