diff --git a/README.md b/README.md index a836fb72..8346b443 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 + +Normal command dispatch and argparse usage errors resolve to one of three released codes — 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 `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. + ## 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