diff --git a/docs/plans/0099-cli-exit-code-split-iter2.md b/docs/plans/0099-cli-exit-code-split-iter2.md new file mode 100644 index 0000000..e60545f --- /dev/null +++ b/docs/plans/0099-cli-exit-code-split-iter2.md @@ -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 ` (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).