Finding F-5 of the "Brownfield CLI/TUI Refactor Assessment" (local report, gitignored under _bmad-output/implementation-artifacts/).
Evidence
- Every failure path exits rc 1: the typed-error clause and the broad backstop in
main() both return 1 (cli.py:2571-2584). Argparse yields rc 2. Nothing is documented.
- KeyboardInterrupt outside
engine.run() (e.g. during config load or engine construction) escapes main() as a bare traceback with rc 1. Inside run() it is already converted to a clean RunStopped path (engine.py:333-348) — that path's rc must NOT change.
Recommendation — two PRs
PR 1 (docs only, no behavior change): add ExitCode(IntEnum) — OK=0, FAILURE=1, USAGE=2 — used in main()'s return paths purely as named constants; document exit codes where the CLI is documented; assert enum values in tests. Requires the exit-code characterization tests (see the python -m bmad_loop issue) to exist and be green first.
PR 2 (released-contract change, ships alone and revertibly): catch KeyboardInterrupt at the main() boundary, print a clean one-line message to stderr, return ExitCode.INTERRUPTED=130. Test: raise KeyboardInterrupt from a monkeypatched cmd handler; assert rc 130, clean stderr, empty stdout under --json rules. Terse CHANGELOG entry with a migration note (previously rc 1 + traceback).
Do NOT add codes 3+ (deferred until a consumer asks).
Ordering
Sessions 4-5 of the refactor program. PR 1 depends on the entry-point/characterization PR being merged; PR 2 requires PR 1 merged (verify on main first).
Traps
- The
engine.run() interrupt path (engine.py:333-348) already yields a clean stop — its rc must stay unchanged; only the escape route outside run() changes.
- rc semantics are a released contract: PR 2 contains nothing else.
Finding F-5 of the "Brownfield CLI/TUI Refactor Assessment" (local report, gitignored under
_bmad-output/implementation-artifacts/).Evidence
main()bothreturn 1(cli.py:2571-2584). Argparse yields rc 2. Nothing is documented.engine.run()(e.g. during config load or engine construction) escapesmain()as a bare traceback with rc 1. Insiderun()it is already converted to a cleanRunStoppedpath (engine.py:333-348) — that path's rc must NOT change.Recommendation — two PRs
PR 1 (docs only, no behavior change): add
ExitCode(IntEnum)—OK=0,FAILURE=1,USAGE=2— used inmain()'s return paths purely as named constants; document exit codes where the CLI is documented; assert enum values in tests. Requires the exit-code characterization tests (see thepython -m bmad_loopissue) to exist and be green first.PR 2 (released-contract change, ships alone and revertibly): catch KeyboardInterrupt at the
main()boundary, print a clean one-line message to stderr, returnExitCode.INTERRUPTED=130. Test: raise KeyboardInterrupt from a monkeypatched cmd handler; assert rc 130, clean stderr, empty stdout under--jsonrules. Terse CHANGELOG entry with a migration note (previously rc 1 + traceback).Do NOT add codes 3+ (deferred until a consumer asks).
Ordering
Sessions 4-5 of the refactor program. PR 1 depends on the entry-point/characterization PR being merged; PR 2 requires PR 1 merged (verify on main first).
Traps
engine.run()interrupt path (engine.py:333-348) already yields a clean stop — its rc must stay unchanged; only the escape route outsiderun()changes.