plan: 0099 CLI exit-code split (iteration 2)
Iteration-2 plan for #175: apply the clean exit-code partition (deviation #8) on top of the clap migration — runtime failures exit 1, usage errors exit 2, no same-class inconsistency. Uses the attribution principle (command-line fault = 2, missing-environment/bad-stdin = 1) to resolve the 6 boundary cases the spec under-specified. refs #175
This commit is contained in:
@@ -0,0 +1,233 @@
|
||||
# Exit-code split (iteration 2) — Implementation Plan
|
||||
|
||||
> **Parent spec:** `docs/specs/0098-cli-clap-gnu-compliance.md`
|
||||
>
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: use the `implement` skill to run
|
||||
> this plan. Steps use `- [ ]` checkboxes for tracking.
|
||||
|
||||
**Goal:** Apply the clean exit-code partition (#175 deviation #8): runtime
|
||||
failures exit 1, usage errors exit 2, with no same-class inconsistency — on top
|
||||
of the clap migration (iteration 1, behaviour-preserving on exit codes).
|
||||
|
||||
**Architecture:** The partition rule (attribution principle): **exit 1 =
|
||||
runtime** — the command line was well-formed but the environment / recorded state
|
||||
it needs is missing, or piped stdin data is bad; **exit 2 = usage** — something
|
||||
about the command-line arguments (including a named file's content) is wrong or
|
||||
inapplicable. Only the "missing environment/recorded state" `exit(2)` sites in
|
||||
the dispatch handlers + execution fns flip to `exit(1)`; every argument-fault
|
||||
site (clap parse errors, post-parse validations, malformed/open argv blueprint
|
||||
files, invalid flag values, unsupported combinations, unknown axis/metric names)
|
||||
stays `exit(2)`. `reproduce`-diverged stays `exit(1)`. No parser structure
|
||||
changes.
|
||||
|
||||
**Tech Stack:** Rust 2024, `aura-cli` binary — `crates/aura-cli/src/main.rs`,
|
||||
`crates/aura-cli/src/graph_construct.rs`, `crates/aura-cli/tests/cli_run.rs`.
|
||||
|
||||
---
|
||||
|
||||
**Files this plan creates or modifies:**
|
||||
|
||||
- Modify: `crates/aura-cli/src/main.rs` — flip the classified RUNTIME
|
||||
`std::process::exit(2)` sites to `exit(1)`; usage sites unchanged
|
||||
- Modify: `crates/aura-cli/src/graph_construct.rs:127-240` — stdin op-script
|
||||
content + stdin-read I/O → `exit(1)`; argv-flag errors stay `exit(2)`
|
||||
- Test: `crates/aura-cli/tests/cli_run.rs` — flip the runtime domain-refusal
|
||||
exit pins (`Some(2)`→`Some(1)`), fix the hidden guard/match arms, add the
|
||||
partition property test
|
||||
- Unchanged (must stay green): `crates/aura-cli/tests/cli_broken_pipe.rs`
|
||||
(SIGPIPE), `crates/aura-cli/tests/graph_construct.rs` (no exit-integer pins)
|
||||
|
||||
---
|
||||
|
||||
## Task 1: Renegotiate the runtime exit pins to 1 (RED)
|
||||
|
||||
Flip the tests first — they go RED because the code still exits 2. Task 2 flips
|
||||
the code to make them GREEN. The message substrings are UNCHANGED; only the exit
|
||||
integer flips.
|
||||
|
||||
**Files:**
|
||||
- Test: `crates/aura-cli/tests/cli_run.rs`
|
||||
|
||||
- [ ] **Step 1: Flip the direct runtime exit-2 pins to exit-1**
|
||||
|
||||
In `crates/aura-cli/tests/cli_run.rs`, change `assert_eq!(out.status.code(),
|
||||
Some(2), ...)` to `Some(1)` in these tests (locate by name; the message
|
||||
`.contains(...)` assertions stay verbatim):
|
||||
|
||||
- `run_real_no_geometry_symbol_refuses_with_exit_2` — the `Some(2)` on the "no
|
||||
recorded geometry for symbol 'NONEXISTENT'" refusal → `Some(1)`
|
||||
- `run_real_refusal_names_no_authored_floor` — the `Some(2)` → `Some(1)`
|
||||
- `run_real_no_geometry_symbol_emits_no_stdout_report` — the `Some(2)` → `Some(1)`
|
||||
- `chart_emits_self_contained_html_for_a_persisted_run` — the missing-run
|
||||
sub-assertion `Some(2)` → `Some(1)`
|
||||
- `chart_tap_nonexistent_on_run_refuses_with_exit_2` — `Some(2)` → `Some(1)`
|
||||
(keeps "run has no tap named 'nope'", "available:")
|
||||
- `chart_unknown_name_refuses_with_exit_2_and_message` — `Some(2)` → `Some(1)`
|
||||
(keeps "no recorded run or family 'ghost'")
|
||||
- `sweep_real_no_geometry_symbol_refuses_with_exit_2` — `Some(2)` → `Some(1)`
|
||||
(keeps "no recorded geometry")
|
||||
- `trace_name_collision_across_kinds_is_refused` — BOTH `Some(2)` → `Some(1)`
|
||||
|
||||
(Optional but consistent: rename these tests dropping the `_exit_2` suffix or
|
||||
leaving it — a rename is not required and risks a filter miss; leave the names,
|
||||
only the integer changes.)
|
||||
|
||||
- [ ] **Step 2: Fix the hidden guard / match arms (drift-prone)**
|
||||
|
||||
These tests branch on `code()` and would silently misbehave if left at `Some(2)`:
|
||||
|
||||
- `run_real_sidecar_symbol_never_hits_the_pip_refusal` — the
|
||||
`match out.status.code() { Some(2) => {..."no local data for symbol
|
||||
'EURUSD'"...}, other => panic!(...) }`: change the `Some(2)` arm to `Some(1)`
|
||||
(a no-data host now exits 1; otherwise the arm misses and hits the panic).
|
||||
- `run_real_sidecar_index_pip_reaches_the_emitted_manifest` — the skip-guard
|
||||
`if out.status.code() == Some(2)` on "no local data for symbol 'GER40'":
|
||||
change to `== Some(1)`.
|
||||
- `run_real_nonvetted_symbol_resolves_pip_from_sidecar` — the skip-guard `if
|
||||
out.status.code() == Some(2)` on "no recorded geometry" / "no local data":
|
||||
change to `== Some(1)`.
|
||||
|
||||
- [ ] **Step 3: Add the partition property test**
|
||||
|
||||
Append to `crates/aura-cli/tests/cli_run.rs`:
|
||||
|
||||
```rust
|
||||
/// The exit-code partition (#175 iteration 2, deviation #8): a USAGE error (a
|
||||
/// malformed command line) exits 2; a RUNTIME failure (a well-formed command
|
||||
/// whose needed data/state is missing) exits 1. Pins the partition as a property.
|
||||
#[test]
|
||||
fn exit_codes_partition_usage_two_from_runtime_one() {
|
||||
// usage: an unknown flag is a command-line error → exit 2
|
||||
let usage = std::process::Command::new(env!("CARGO_BIN_EXE_aura"))
|
||||
.args(["run", "--bogus"]).output().expect("spawn");
|
||||
assert_eq!(usage.status.code(), Some(2),
|
||||
"unknown flag is a usage error → 2; stderr: {}", String::from_utf8_lossy(&usage.stderr));
|
||||
// runtime: a well-formed command whose symbol has no recorded data → exit 1
|
||||
let runtime = std::process::Command::new(env!("CARGO_BIN_EXE_aura"))
|
||||
.args(["run", "--real", "NONEXISTENT"]).output().expect("spawn");
|
||||
assert_eq!(runtime.status.code(), Some(1),
|
||||
"no data for a valid command is a runtime failure → 1; stderr: {}",
|
||||
String::from_utf8_lossy(&runtime.stderr));
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run the flipped tests to verify they FAIL (RED)**
|
||||
|
||||
Run: `cargo test -p aura-cli --test cli_run exit_codes_partition_usage_two_from_runtime_one`
|
||||
Expected: FAIL — the runtime arm gets `Some(2)` (code still exits 2), not `Some(1)`.
|
||||
|
||||
Run: `cargo test -p aura-cli --test cli_run run_real_no_geometry_symbol_refuses`
|
||||
Expected: FAIL — `Some(2)` received where `Some(1)` now asserted.
|
||||
|
||||
---
|
||||
|
||||
## Task 2: Flip the runtime code sites to exit 1 (GREEN)
|
||||
|
||||
Change `std::process::exit(2)` to `std::process::exit(1)` at the classified
|
||||
RUNTIME sites in `main.rs` and `graph_construct.rs`. Locate each by its enclosing
|
||||
fn + message string (line numbers drift as edits land). USAGE sites are NOT
|
||||
touched. After this, the Task-1 tests go green.
|
||||
|
||||
**Files:**
|
||||
- Modify: `crates/aura-cli/src/main.rs`
|
||||
- Modify: `crates/aura-cli/src/graph_construct.rs`
|
||||
|
||||
- [ ] **Step 1: Flip the missing-data / missing-recorded-state sites (main.rs)**
|
||||
|
||||
Change `exit(2)` → `exit(1)` at each (by fn + message):
|
||||
- `no_real_data` — `"no local data for symbol '{symbol}' …"`
|
||||
- `pip_or_refuse` — `"no recorded geometry for symbol '{symbol}' …"`
|
||||
- `emit_chart` — the store-read failure, the tap failure ("run has no tap
|
||||
named"), the read_family failure, the build_comparison failure, and `"no
|
||||
recorded run or family '{name}'…"` (all five exit sites in `emit_chart`)
|
||||
- `runs_families` — the `load_family_members` failure
|
||||
- `runs_family` — the `load_family_members` failure
|
||||
|
||||
- [ ] **Step 2: Flip the name-collision sites (main.rs)**
|
||||
|
||||
Change `exit(2)` → `exit(1)` at every `ensure_name_free` refusal (the `"{e}"`
|
||||
after `ensure_name_free(...)`), in: `run_sample`, `run_sample_real`, `run_sweep`,
|
||||
`run_walkforward`, `run_mc`, `run_macd`, `run_stage1_r`. (These are the
|
||||
trace-name-collision-across-kinds class the spec lists as runtime.)
|
||||
|
||||
- [ ] **Step 3: Flip the reproduce missing/corrupted-stored-state sites (main.rs)**
|
||||
|
||||
Change `exit(2)` → `exit(1)` in `reproduce_family_in` at: `load_family_members`
|
||||
failure, `"no such family '{id}'"`, `"family member has no topology_hash; not a
|
||||
generated run"`, the `get_blueprint` failure, `"blueprint {hash} missing from
|
||||
store"`, `"stored blueprint {hash} does not parse: {e:?}"`; and in
|
||||
`point_from_params` at `"manifest is missing param {}"`. (These are missing /
|
||||
corrupt content-addressed store state = runtime.) LEAVE `reproduce_family`'s
|
||||
diverged-member `exit(1)` unchanged (already 1).
|
||||
|
||||
- [ ] **Step 4: Normalize the persist-write class to 1 (main.rs)**
|
||||
|
||||
Change `exit(2)` → `exit(1)` at every persist WRITE failure so the whole class is
|
||||
consistent (`run_generalize`'s `"failed to persist family"` is already `exit(1)`
|
||||
— leave it; the others flip to match):
|
||||
- `persist_traces` — `"trace persist failed: {e}"` (TraceStore::write)
|
||||
- `persist_traces_r` — `"trace persist failed: {e}"` (TraceStore::write)
|
||||
- `run_sweep` — `append_family` failure
|
||||
- `run_walkforward` — `append_family` failure
|
||||
- `run_mc` — `append_family` failure
|
||||
- `run_blueprint_sweep` — `put_blueprint` and `append_family` failures
|
||||
- `run_blueprint_walkforward` — `put_blueprint` and `append_family` failures
|
||||
- `run_blueprint_mc` — `put_blueprint` and `append_family` failures
|
||||
|
||||
- [ ] **Step 5: Flip the stdin runtime sites (graph_construct.rs)**
|
||||
|
||||
Change `exit(2)` → `exit(1)` where the fault is stdin op-script CONTENT or a
|
||||
stdin-read I/O failure; LEAVE argv-flag usage errors at `exit(2)`:
|
||||
- `build_cmd` — `"reading stdin: {e}"` (stdin-read I/O) → 1
|
||||
- `build_cmd` — `build_from_str` `"{msg}"` (op-script content) → 1
|
||||
- `introspect_cmd` — the two `"reading stdin: {e}"` (stdin-read I/O) → 1
|
||||
- `introspect_cmd` — `introspect_unwired` `"{m}"` (op-script content) → 1
|
||||
- `introspect_cmd` — `build_from_str` `"{m}"` for `--content-id` (op-script
|
||||
content) → 1
|
||||
- LEAVE at `exit(2)`: `introspect_cmd` — `"usage: aura graph introspect …"`
|
||||
(argv-flag error) and `introspect_node` `"{m}"` for `--node <T>` (an invalid
|
||||
argv flag value = usage).
|
||||
|
||||
- [ ] **Step 6: Confirm the usage sites are untouched**
|
||||
|
||||
Verify (read, do not edit) that these STAY `exit(2)`: every `dispatch_*`
|
||||
argument-applicability guard; `run_data_from` (`--from`/`--to` without `--real`);
|
||||
`run_args_from` (harness / cost-flags-require-R / non-negative / window);
|
||||
`generalize_args_from` (required knobs / ≥2 instruments / duplicate / strategy);
|
||||
`dispatch_runs` rank grammar; all the dual-grammar blueprint FILE read/parse
|
||||
sites in the four `.json` branches (`read_to_string` + `blueprint_from_json` in
|
||||
`dispatch_run`/`dispatch_sweep`/`dispatch_walkforward`/`dispatch_mc` — a named
|
||||
argv file's bad content is a usage error, staying 2); the applicability refusals
|
||||
(walkforward-form-for-strategy, mc-requires-closed-blueprint, unknown `--axis`,
|
||||
invalid `--metric`/`--select`/`--strategy`).
|
||||
|
||||
- [ ] **Step 7: Build + full-suite gate**
|
||||
|
||||
Run: `cargo build --workspace`
|
||||
Expected: PASS (0 errors).
|
||||
|
||||
Run: `cargo clippy --workspace --all-targets -- -D warnings`
|
||||
Expected: PASS.
|
||||
|
||||
Run: `cargo test --workspace`
|
||||
Expected: PASS — the Task-1 flipped pins + partition property test now GREEN; the
|
||||
usage-error pins unchanged and green; `cli_broken_pipe.rs` and
|
||||
`graph_construct.rs` green untouched.
|
||||
|
||||
---
|
||||
|
||||
## Notes for the executor
|
||||
|
||||
- **Behaviour change, pinned.** This iteration deliberately changes observable
|
||||
exit codes for runtime failures (2→1). Task 1 flips the pins RED-first; Task 2
|
||||
flips the code GREEN. Do not "fix" a red Task-1 pin by reverting it — the red
|
||||
is the point until Task 2 lands.
|
||||
- **Attribution rule for any site not listed:** if the user must fix the COMMAND
|
||||
LINE (a flag value, an unsupported combination, a named file's content) → stays
|
||||
`exit(2)`; if the command was well-formed but the ENVIRONMENT/recorded state is
|
||||
missing or the STDIN data is bad → `exit(1)`.
|
||||
- **The dual-grammar quartet stays in lockstep:** the four `.json`-branch file
|
||||
read/parse sites are all usage (exit 2) — do not flip any one of them.
|
||||
- **No message-string edits.** Only the exit integer changes; every `.contains()`
|
||||
substring pin stays satisfied. The mixed "Usage:"/"usage:" casing is left as-is
|
||||
this iteration (a separate cosmetic-polish concern).
|
||||
Reference in New Issue
Block a user