diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f6f7b5..753d7ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,17 @@ breaking changes may land in a minor release. `__main__.py`. Subprocess smoke tests exercise the module entry, and characterization tests pin the current CLI exit codes (typed errors and the broad backstop → 1, argparse usage → 2). +### Changed + +- **Ctrl+C outside a run now exits `130` cleanly (#241).** A `KeyboardInterrupt` escaping + `main()` outside `engine.run()` (config load, engine construction) now prints a one-line + `interrupted` to stderr and returns the new `ExitCode.INTERRUPTED` (`130` = 128 + SIGINT). + The exit code is **unchanged** — uncaught, CPython already ended the process at `130` by + re-raising SIGINT — but previously as a **bare traceback**, dumped after any partial `--json` + stdout. It is now a clean `exit(130)` with empty stdout (a Python caller reading + `subprocess.returncode` sees `130` rather than `-2`). A Ctrl+C _during_ a run is unchanged: + the engine still finalizes it as a resumable `stopped` run (rc `0`). + ## [0.9.0] — 2026-07-21 ### Added diff --git a/README.md b/README.md index 8346b44..196b3d9 100644 --- a/README.md +++ b/README.md @@ -521,15 +521,16 @@ Two things to know when matching. `message` is **not** contracted — several pr ### Exit codes -Normal command dispatch and argparse usage errors resolve to one of three released codes — safe to branch on in CI: +Normal command dispatch, argparse usage errors, and interruption resolve to one of four 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 | +| 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 | +| `130` | `INTERRUPTED` | Ctrl+C (SIGINT) interrupted the process outside a run — 128 + signal 2, the shell's convention; a one-line `interrupted` notice goes to stderr, no traceback | -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`. +No `ExitCode` values `3`–`129` or `≥131` are assigned yet. `INTERRUPTED` (`130`) covers only a Ctrl+C that lands **outside** a run — during config load or engine construction; a Ctrl+C **during** a run is caught by the engine and finalized as a clean, resumable `stopped` run, which exits `0`. 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. diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index 4f8e1ac..c6bf5b8 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -81,14 +81,21 @@ class ExitCode(IntEnum): - ``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. + - ``INTERRUPTED`` — Ctrl+C (SIGINT) escaped ``main()`` outside ``engine.run()`` + (config load, engine construction, a handler with no run loop). 130 = 128 + + SIGINT(2), the shell's conventional code for an interrupt; uncaught this path + already reached 130 (CPython re-raises SIGINT) but as a bare traceback, so + ``main()`` catches it for a clean one-line exit at the same code. The in-run + interrupt converts to a clean ``RunStopped`` (``engine.py``) and never reaches + here — a Ctrl+C during a run stays rc 0. + + Codes ``3``-``129`` and ``131+`` are intentionally absent until a consumer needs one. """ OK = 0 FAILURE = 1 USAGE = 2 + INTERRUPTED = 130 def _project(args: argparse.Namespace) -> Path: @@ -2601,6 +2608,17 @@ def add(name: str, func, help: str, *, aliases=()) -> argparse.ArgumentParser: ) as e: print(f"error: {e}", file=sys.stderr) return ExitCode.FAILURE + except KeyboardInterrupt: + # Ctrl+C on the residual surface outside engine.run() (config load, engine + # construction, a handler with no run loop). A KeyboardInterrupt is a + # BaseException, so the broad `except Exception` below never caught it. + # Uncaught it already ends at 130 — CPython re-raises SIGINT (128+2) — but as + # death-by-signal with a bare traceback dumped after any partial --json stdout. + # Catch it for a clean, intentional exit(130): one line on stderr, nothing on + # stdout. The in-run interrupt is engine.run()'s own clean RunStopped (see + # engine.py) and never reaches here. + print("interrupted", file=sys.stderr) + return ExitCode.INTERRUPTED except Exception as e: # backstop for the residual surface outside engine.run() (config load, # engine construction, render/notify): never let an unexpected exception diff --git a/tests/test_entry_point.py b/tests/test_entry_point.py index c72bccb..3933104 100644 --- a/tests/test_entry_point.py +++ b/tests/test_entry_point.py @@ -10,10 +10,10 @@ Do not convert these to in-process ``cli.main()`` calls; the subprocess *is* the test. - **Exit-code semantics.** ``main()`` maps the typed error surface and the broad - backstop both to rc 1, and argparse usage errors to rc 2 (``cli.py`` dispatch tail). - The ``test_exit_*`` cases pin today's rc for each path. They are characterization - tests — a guard rail for the later exit-code-taxonomy work (#241) and the cli.py - composition extraction (#243), not an endorsement of the current mapping. + backstop both to rc 1, argparse usage errors to rc 2, and a Ctrl+C escaping + ``engine.run()`` to rc 130 — each named by the ``ExitCode`` enum (#241). The + ``test_exit_*`` cases pin the rc for each path, a guard rail for the exit-code + taxonomy and the later cli.py composition extraction (#243). """ import json @@ -119,6 +119,27 @@ def test_exit_argparse_usage_error_is_2(capsys): assert "invalid choice" in capsys.readouterr().err +def test_exit_keyboard_interrupt_is_130(tmp_path, capsys, monkeypatch): + """Ctrl+C escaping ``main()`` outside ``engine.run()`` → rc 130 (128+SIGINT), a + one-line stderr notice, no traceback. Patched onto the dispatched handler so the + raise lands inside the dispatch try — the same escape route a config-load or + engine-construction interrupt takes. (The in-run interrupt is ``engine.run()``'s + own clean RunStopped, which this PR must not touch — see engine.py.)""" + + def boom(_args): + raise KeyboardInterrupt + + monkeypatch.setattr(cli, "cmd_validate", boom) + # --json is the strictest stdout contract: a --json command must emit *no* partial + # document on interrupt, only the whole document or nothing (machine.py). The raise + # lands before any output, so stdout stays empty and stderr carries just the notice. + assert cli.main(["validate", "--json", "--project", str(tmp_path)]) == 130 + captured = capsys.readouterr() + assert captured.out == "" # empty stdout: nothing half-emitted under --json rules + assert captured.err == "interrupted\n" # exactly one line, no stray whitespace + assert "Traceback" not in captured.err + + # --------------------------------------------------------------------- ExitCode enum @@ -128,12 +149,13 @@ def test_exit_code_enum_values(): assert cli.ExitCode.OK == 0 assert cli.ExitCode.FAILURE == 1 assert cli.ExitCode.USAGE == 2 + assert cli.ExitCode.INTERRUPTED == 130 -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_has_only_contracted_codes(): + """OK/FAILURE/USAGE/INTERRUPTED are the whole contract — codes 3-129 and 131+ 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, 130} def test_exit_code_enum_is_int_returnable():