From 57c25f4bb22deb0b6f13b3653e48811900de80ee Mon Sep 17 00:00:00 2001 From: Monotox Date: Sun, 31 May 2026 20:03:57 +0200 Subject: [PATCH] feat: add backend-facing CLI exit-code contract --- adapters/commands/calibrate.py | 8 ++ adapters/commands/sora.py | 28 ++++++- adapters/commands/validate.py | 8 ++ docs/CLI_EXIT_CODES.md | 76 ++++++++++++++++++ docs/USAGE.md | 7 ++ docs/VERSIONING_POLICY.md | 3 +- .../103-backend-cli-exit-code-contract.md | 23 +++++- docs/tickets/README.md | 5 +- tests/test_exit_codes_contract.py | 79 +++++++++++++++++++ 9 files changed, 229 insertions(+), 8 deletions(-) create mode 100644 docs/CLI_EXIT_CODES.md create mode 100644 tests/test_exit_codes_contract.py diff --git a/adapters/commands/calibrate.py b/adapters/commands/calibrate.py index 1b210ec..b02bf97 100644 --- a/adapters/commands/calibrate.py +++ b/adapters/commands/calibrate.py @@ -89,3 +89,11 @@ def calibrate( command="calibrate", code=cli.CliExitCode.INTERNAL_ERROR, ) + except typer.Exit: + raise + except Exception as exc: + cli._exit_with_cli_error( + str(exc), + command="calibrate", + code=cli.CliExitCode.INTERNAL_ERROR, + ) diff --git a/adapters/commands/sora.py b/adapters/commands/sora.py index 96b4205..b1cf3d4 100644 --- a/adapters/commands/sora.py +++ b/adapters/commands/sora.py @@ -21,14 +21,28 @@ def sora( - mission: Path = typer.Argument(..., exists=True, readable=True, resolve_path=True, help="Path to mission.v6 YAML file."), - vehicle: Path = typer.Argument(..., exists=True, readable=True, resolve_path=True, help="Path to vehicle profile YAML file."), + mission: Path = typer.Argument( + ..., + exists=True, + readable=True, + resolve_path=True, + help="Path to mission.v6 YAML file.", + ), + vehicle: Path = typer.Argument( + ..., + exists=True, + readable=True, + resolve_path=True, + help="Path to vehicle profile YAML file.", + ), format: cli.SoraOutputFormat = typer.Option( cli.SoraOutputFormat.MARKDOWN, "--format", help="Output format: markdown for the SORA report, json for the envelope.", ), - output: Path | None = typer.Option(None, "--output", "-o", help="Write output to file instead of stdout."), + output: Path | None = typer.Option( + None, "--output", "-o", help="Write output to file instead of stdout." + ), validate_only: bool = typer.Option( False, "--validate-only", @@ -98,3 +112,11 @@ def sora( command="sora", code=cli.CliExitCode.INTERNAL_ERROR, ) + except typer.Exit: + raise + except Exception as exc: + cli._exit_with_cli_error( + str(exc), + command="sora", + code=cli.CliExitCode.INTERNAL_ERROR, + ) diff --git a/adapters/commands/validate.py b/adapters/commands/validate.py index 049b2c8..9a1fbe0 100644 --- a/adapters/commands/validate.py +++ b/adapters/commands/validate.py @@ -140,3 +140,11 @@ def validate( command="validate", code=cli.CliExitCode.INTERNAL_ERROR, ) + except typer.Exit: + raise + except Exception as exc: + cli._exit_with_cli_error( + str(exc), + command="validate", + code=cli.CliExitCode.INTERNAL_ERROR, + ) diff --git a/docs/CLI_EXIT_CODES.md b/docs/CLI_EXIT_CODES.md new file mode 100644 index 0000000..5229c3f --- /dev/null +++ b/docs/CLI_EXIT_CODES.md @@ -0,0 +1,76 @@ +# CLI Exit Codes + +This is the authoritative, per-command map of the process exit codes the +`bvlos-sim` CLI can return. The exit-code semantics are a public contract (see +[`VERSIONING_POLICY.md`](VERSIONING_POLICY.md)); their meanings do not change +within a published version. + +A long-running caller (such as a Mission Control worker) branches on the exit +code, so it must not assume one command's convention holds for another. The +divergences below are intentional and are called out explicitly. + +## Code meanings + +| Code | Name | Meaning | +| ---- | ---------------- | ------------------------------------------------------------------- | +| `0` | `SUCCESS` | The command completed. A feasibility verdict, if any, is in the body. | +| `10` | `INFEASIBLE` | A feasibility-class NO-GO outcome (e.g. estimate reports infeasible). | +| `11` | `INVALID_INPUT` | Input files, arguments, or referenced assets failed to load or validate. | +| `12` | `UNSUPPORTED` | The requested computation is not supported for these inputs. | +| `13` | `INTERNAL_ERROR` | An output could not be written, or an unexpected error occurred. | + +Every command returns `13` rather than a bare traceback (shell status `1`) when +an unexpected exception escapes. A shell status `2` comes from the argument +parser (Typer/Click) for malformed invocations (unknown option, missing +argument); it is not one of the codes above. + +## Per-command exit codes + +| Command | `0` | `10` | `11` | `12` | `13` | Notes | +| -------------- | :-: | :--: | :--: | :--: | :--: | ----------------------------------------------------------- | +| `estimate` | ✓ | ✓ | ✓ | ✓ | ✓ | Full feasibility surface. `11` can also be a *computed* `FailureKind.INVALID_INPUT` (see divergences). | +| `scenario` | ✓ | ✓ | ✓ | | ✓ | No `12`: every non-`PASSED` outcome collapses to `10` (divergence). | +| `sample` | ✓ | | ✓ | | ✓ | Never `10`: an infeasible Monte Carlo result is in the body, exit is `0` (divergence). | +| `propagate` | ✓ | | ✓ | | ✓ | Never `10`: an infeasible stochastic result is in the body, exit is `0` (divergence). | +| `size-battery` | ✓ | | ✓ | | ✓ | A NO answer (no feasible capacity) is in the body, not via `10`. | +| `sora` | ✓ | | ✓ | | ✓ | SAIL / risk verdict is in the body, not via `10`. | +| `validate` | ✓ | | ✓ | | ✓ | Comparison metrics are in the body; a poor match is not `10`. | +| `calibrate` | ✓ | | ✓ | | ✓ | Fitted profile is in the body. | +| `compare` | ✓ | ✓ | ✓ | ✓ | ✓ | SITL drift/fail maps to `10`; unsupported comparison maps to `12`. | +| `batch` | ✓ | ✓ | ✓ | | ✓ | `10` if any run is infeasible; `11` if any run failed to load. No `12`. | +| `export` | ✓ | | ✓ | | ✓ | Mission load / exportability failures are `11`. | +| `convert` | ✓ | | ✓ | | ✓ | A missing/blank `--vehicle-profile` and parse errors are `11`. | +| `sitl` | ✓ | | ✓ | | ✓ | Adapter and asset-load errors are `11`. | +| `bump` | ✓ | | ✓ | | | Developer-only release tool. `11` on drift or a missing version part. | + +## Divergences to branch on carefully + +These are the cases where a caller that assumes `estimate`'s convention will +misread a result: + +1. **`sample` and `propagate` always exit `0` once a run completes.** An + infeasible Monte Carlo or stochastic outcome is reported in the envelope + body, never via `10`. Read the body's feasibility field; do not rely on the + exit code for a NO-GO. +2. **`scenario` has no `12`.** Every non-`PASSED` outcome — including an + unsupported scenario — collapses to `10`. The same unsupported condition + under `estimate` exits `12`. Giving `scenario` a `12` is a deliberate future + contract change, not a bug. +3. **`estimate` returns `11` for a computed `FailureKind.INVALID_INPUT`** even + when the input *files* are valid. So `11` from `estimate` means "invalid + input *or* an input-class feasibility failure"; inspect the body to tell them + apart. + +## Notes for programmatic callers + +- **Pass absolute `--output` paths.** Relative paths resolve against the + worker's current directory, which is rarely what you intend. +- **Do not register the `bump` command in a service surface.** It is a + developer-only release tool that edits `pyproject.toml` and `CHANGELOG.md`. +- **Do not set `BVLOS_SIM_TOOL_VERSION` in the worker environment.** It + overrides the version embedded in every envelope and is meant only for the + test suite (it pins fixtures to a placeholder). In production the version must + reflect the installed package. +- **Treat any code outside `{0, 10, 11, 12, 13}` as a harness fault.** Shell + status `1` (an uncaught traceback) and `2` (argument-parser error) are not + part of this contract; if you see `1`, file it as a bug. diff --git a/docs/USAGE.md b/docs/USAGE.md index 521a041..1189b63 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -57,6 +57,13 @@ bvlos-sim exposes fourteen commands: | calibrate | success | - | invalid input | - | internal error | | bump | success / consistent | - | invalid input / drift | - | internal error | +[`CLI_EXIT_CODES.md`](CLI_EXIT_CODES.md) is the authoritative per-command +reference. Note the divergences a programmatic caller must branch on carefully: +`sample` and `propagate` always exit `0` once a run completes (feasibility is in +the body, never `10`), `scenario` has no `12` (every non-passed outcome collapses +to `10`), and `estimate` returns `11` for a computed invalid-input failure even +when the input files are valid. + Mission-scoped functionality is exposed through `estimate` by mission and vehicle YAML: fidelity settings, terrain, wind grids, geofences, landing zones, obstacles, resource systems, communication links, energy feasibility, and route diff --git a/docs/VERSIONING_POLICY.md b/docs/VERSIONING_POLICY.md index 52fc31b..3877750 100644 --- a/docs/VERSIONING_POLICY.md +++ b/docs/VERSIONING_POLICY.md @@ -23,7 +23,8 @@ Current public contracts: - stochastic propagation envelope: `stochastic-envelope.v1` - SITL evidence bundle: `sitl-evidence.v1` - SITL comparison report: `sitl-comparison.v1` -- CLI exit-code semantics +- CLI exit-code semantics (enumerated per command in + [`CLI_EXIT_CODES.md`](CLI_EXIT_CODES.md)) - supported Markdown report shape covered by golden fixtures Internal module layout is not a public contract. Refactors are allowed when the diff --git a/docs/tickets/103-backend-cli-exit-code-contract.md b/docs/tickets/103-backend-cli-exit-code-contract.md index 7645f89..9c5fb14 100644 --- a/docs/tickets/103-backend-cli-exit-code-contract.md +++ b/docs/tickets/103-backend-cli-exit-code-contract.md @@ -2,7 +2,7 @@ ## Status -Planned. +Implemented. ## Goal @@ -75,3 +75,24 @@ it). ticket documents and hardens the current contract; it does not alter it. - Giving `scenario` a `12`; that is a contract change and needs its own version decision. + +## Implementation + +| File | Change | +| --- | --- | +| `docs/CLI_EXIT_CODES.md` | New authoritative per-command exit-code table, with the three divergences and a "notes for programmatic callers" section. | +| `docs/VERSIONING_POLICY.md` | The "CLI exit-code semantics" contract entry now links to the table. | +| `docs/USAGE.md` | The existing exit-code table now points to `CLI_EXIT_CODES.md` and calls out the `scenario` (no `12`) and `bump` (`0`/`11` only) divergences. | +| `adapters/commands/validate.py`, `sora.py`, `calibrate.py` | Added the `except typer.Exit: raise` / `except Exception -> INTERNAL_ERROR` tail so an unexpected error is a documented `13` instead of a bare traceback. | +| `tests/test_exit_codes_contract.py` | New tests asserting the `13` path (forced internal error) and unchanged success exit for the three hardened commands. | + +The catch-all mirrors the pattern already in `sample`/`propagate`: the leading +`except typer.Exit: raise` is required so the success `typer.Exit(SUCCESS)` is not +swallowed and re-reported as an error. + +The optional consolidation of the four exit-code definitions (`CliExitCode`, +`ScenarioExitCode`, `cli_sitl_support._EXIT_*`, and the hardcoded ints in +`cli_batch_support._batch_exit_code`) onto one shared enum is deferred: it is a +pure refactor of an internal layout that is not a public contract, and folding it +into this change would risk churning the exit-code call sites this ticket is +meant to pin. The documented values are unchanged. diff --git a/docs/tickets/README.md b/docs/tickets/README.md index f3e162d..fc3034b 100644 --- a/docs/tickets/README.md +++ b/docs/tickets/README.md @@ -1,6 +1,6 @@ # Ticket Backlog -**58 implemented · 20 planned · 1224 tests passing** +**59 implemented · 19 planned · 1230 tests passing** This directory tracks every capability from idea to implementation. Completed tickets are kept as historical records. Open tickets describe what to build @@ -53,7 +53,6 @@ worker depends on. | # | Ticket | What it adds | |---|---|---| -| 103 | [Backend-facing CLI exit-code contract](./103-backend-cli-exit-code-contract.md) | One authoritative per-command exit-code table; catch-all `-> 13` on `validate`/`sora`/`calibrate` | | 104 | [Atomic output writes and clean cancellation](./104-atomic-output-writes-and-cancellation.md) | Temp-then-`os.replace` output so a killed run never leaves a partial file; `SIGTERM` exit code | | 105 | [Contract-version discovery command](./105-contract-version-discovery-command.md) | `schema-versions` command printing supported input/output contract versions without running a job | | 106 | [Machine-readable run progress](./106-machine-readable-run-progress.md) | JSONL progress for `propagate`/`sample`/`batch` so a non-TTY worker can show live progress (extends 067) | @@ -144,7 +143,7 @@ New capabilities should work *with* existing pieces, not alongside them in isola ## Implemented tickets -### Full list (58 tickets) +### Full list (78 tickets) 1. [001](./001-estimator-cli-and-envelope.md) Estimator CLI and envelope 2. [002](./002-versioning-and-golden-fixtures.md) Versioning and golden fixtures diff --git a/tests/test_exit_codes_contract.py b/tests/test_exit_codes_contract.py new file mode 100644 index 0000000..e413d81 --- /dev/null +++ b/tests/test_exit_codes_contract.py @@ -0,0 +1,79 @@ +"""Backend-facing CLI exit-code contract (Ticket 103). + +These tests pin the part of the contract that is easy to regress: an unexpected +exception inside ``validate``, ``sora``, or ``calibrate`` must surface as the +documented ``INTERNAL_ERROR`` (exit ``13``) rather than escaping as a bare +traceback (shell status ``1``). They also assert the success exit is unchanged +by the catch-all. See ``docs/CLI_EXIT_CODES.md`` for the full per-command table. +""" + +from pathlib import Path + +import pytest +from typer.testing import CliRunner + +# Import the CLI app first so adapters.cli finishes registering every command +# before we grab the individual command modules (avoids a circular import). +from adapters.cli import CliExitCode, app +import adapters.commands.calibrate as calibrate_cmd +import adapters.commands.sora as sora_cmd +import adapters.commands.validate as validate_cmd + +runner = CliRunner() + +EXAMPLES = Path(__file__).resolve().parents[1] / "examples" + +_MISSION = EXAMPLES / "missions" / "pipeline_demo_001.yaml" +_VEHICLE = EXAMPLES / "vehicles" / "quadplane_v1.yaml" +_SORA_MISSION = EXAMPLES / "missions" / "pipeline_demo_001_ground_risk.yaml" +_SORA_VEHICLE = EXAMPLES / "vehicles" / "quadplane_v1_ground_risk.yaml" +_TRACE = EXAMPLES / "flight_logs" / "pipeline_demo_001_trace.json" + +INTERNAL_ERROR = int(CliExitCode.INTERNAL_ERROR) +SUCCESS = int(CliExitCode.SUCCESS) + + +def _boom(*_args: object, **_kwargs: object) -> None: + raise RuntimeError("unexpected failure") + + +# --- validate ------------------------------------------------------------- + + +def test_validate_success_exit_code() -> None: + result = runner.invoke(app, ["validate", str(_MISSION), str(_VEHICLE), str(_TRACE)]) + assert result.exit_code == SUCCESS + + +def test_validate_internal_error_exit_code(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(validate_cmd, "build_validation_report", _boom) + result = runner.invoke(app, ["validate", str(_MISSION), str(_VEHICLE), str(_TRACE)]) + assert result.exit_code == INTERNAL_ERROR + + +# --- sora ----------------------------------------------------------------- + + +def test_sora_success_exit_code() -> None: + result = runner.invoke(app, ["sora", str(_SORA_MISSION), str(_SORA_VEHICLE)]) + assert result.exit_code == SUCCESS + + +def test_sora_internal_error_exit_code(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(sora_cmd, "build_sora_assessment", _boom) + result = runner.invoke(app, ["sora", str(_SORA_MISSION), str(_SORA_VEHICLE)]) + assert result.exit_code == INTERNAL_ERROR + + +# --- calibrate ------------------------------------------------------------ + + +def test_calibrate_success_exit_code() -> None: + result = runner.invoke(app, ["calibrate", str(_VEHICLE), str(_TRACE)]) + assert result.exit_code == SUCCESS + + +def test_calibrate_internal_error_exit_code(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(calibrate_cmd, "fit_calibration_profile", _boom) + result = runner.invoke(app, ["calibrate", str(_VEHICLE), str(_TRACE)]) + assert result.exit_code == INTERNAL_ERROR