fix(aura-cli, aura-runner): review findings — duplicate --override refusal, live discovery prose, neutral JSON refusal

In-cycle fixes for the independent review of the #319 package, RED-first
for both Importants. A repeated --override path now refuses at the shared
lexer (exit 2, path named) instead of panicking exit 101 through
reopen_all — which also un-misattributes the campaign leg's collision
prose for duplicates. override_paths' refusal points at aura graph
introspect --params (the retired --list-axes reference swept from every
live comment, one explicitly-historical mention remains). exec classifies
file targets through a Result-bearing peek: unparseable JSON refuses
neutrally before any leg is chosen. RunPresentation (single-variant)
removed; run_campaign_returning's contract note names its one deliberate
usage-class exit; C14's routing sentence and C20's family-builder
paragraph amended to current truth (superseded text to the history
sidecars); bench error strings and deletion blank-line runs tidied.

refs #319
This commit is contained in:
2026-07-25 23:21:07 +02:00
parent 9eb6d6b4f6
commit dc5f1742f4
14 changed files with 326 additions and 236 deletions
+32 -41
View File
@@ -108,15 +108,6 @@ struct CampaignRunLine<'a> {
campaign_run: &'a CampaignRunRecord,
}
/// Stdout shape of a campaign run: emit-gated member/selection lines + the
/// always-on final `campaign_run` record line. `MemberLinesOnly` (the
/// dissolved-verb sugar path's suppressed-record-line mode) retired with the
/// quintet (#319 Task 8) — `Full` is the only surviving shape.
#[derive(Clone, Copy, PartialEq)]
pub(crate) enum RunPresentation {
Full,
}
/// `aura campaign run <target>`: resolve, gate, execute, emit. Every `Err`
/// is a refusal `campaign_cmd` renders as `aura: {msg}` + exit 1; an `Ok`
/// carries the failed-cell count (#272) so the caller can exit 3 on a
@@ -161,7 +152,7 @@ pub(crate) fn run_campaign(
));
};
run_campaign_by_id(&campaign_id, env, RunPresentation::Full, parallel_instruments, overrides)
run_campaign_by_id(&campaign_id, env, parallel_instruments, overrides)
}
/// An executed campaign plus the context its presenter and the dissolved-verb
@@ -180,18 +171,15 @@ pub(crate) struct CampaignRun {
/// The one campaign executor path from a resolved content id onward: fetch
/// the stored canonical bytes by id (so file addressing and id addressing
/// produce the same realization by construction) and re-run the intrinsic
/// tier on them, execute, and emit per `presentation`. Called by
/// `run_campaign` (`RunPresentation::Full`, the sole surviving shape since
/// the quintet's dissolved-verb sugar path retired, #319 Task 8).
/// tier on them, execute, and emit. Called by `run_campaign`.
pub(crate) fn run_campaign_by_id(
campaign_id: &str,
env: &Env,
presentation: RunPresentation,
parallel_instruments: NonZeroUsize,
overrides: &[(String, Scalar)],
) -> Result<usize, String> {
let run = run_campaign_returning(campaign_id, env, parallel_instruments, overrides)?;
present_campaign(run, presentation, env)
present_campaign(run, env)
}
/// The one campaign executor path from a resolved content id up to (not
@@ -206,6 +194,15 @@ pub(crate) fn run_campaign_by_id(
/// and BEFORE the referential gate — the stored bytes and the recorded
/// `campaign_id` stay those of the ORIGINAL document; only the executed
/// run-set changes, its audit record being the raw member manifests.
///
/// One deliberate exception to "every refusal is an `Err`" (review Minor-4):
/// the injection loop's axis-collision check exits directly with code 2
/// instead of returning `Err`. That collision is a USAGE-class fault (C14) —
/// `--override` naming a path the document already declares as an axis is a
/// malformed invocation, exactly like `--override`'s own malformed-token
/// refusal (`parse_override_tokens`, exit 2) — not a RUNTIME-class refusal
/// (missing environment/recorded state, exit 1), which is what this fn's
/// `Result<_, String>` return otherwise carries end to end.
pub(crate) fn run_campaign_returning(
campaign_id: &str,
env: &Env,
@@ -316,15 +313,11 @@ of the campaign; an override overrides a bound value, never an axis"
Ok(CampaignRun { outcome, campaign, strategies, server: runner.server() })
}
/// Emit an executed campaign's results per `presentation`: the zero-survivor
/// stderr notes, the emit-gated family/selection lines, the always-on record
/// line (Full only), then trace persistence. Split out of `run_campaign_by_id`
/// so the dissolved-verb sugar can consume the outcome without this stdout tail.
fn present_campaign(
run: CampaignRun,
presentation: RunPresentation,
env: &Env,
) -> Result<usize, String> {
/// Emit an executed campaign's results: the zero-survivor stderr notes, the
/// emit-gated family/selection lines, the always-on record line, then trace
/// persistence. Split out of `run_campaign_by_id` so a forthcoming caller
/// could consume the outcome without this stdout tail.
fn present_campaign(run: CampaignRun, env: &Env) -> Result<usize, String> {
let CampaignRun { outcome, campaign, strategies, server } = run;
// Zero-survivor stderr notes (exit stays 0 — a null result is a valid
@@ -413,23 +406,21 @@ fn present_campaign(
}
}
}
if presentation == RunPresentation::Full {
println!(
"{}",
serde_json::to_string(&CampaignRunLine { campaign_run: &outcome.record })
.expect("campaign run record serializes")
);
eprintln!(
"aura: campaign run {} recorded: {} cells{}",
outcome.record.run,
outcome.record.cells.len(),
if failed == 0 {
String::new()
} else {
format!(", {failed} failed ({})", fail_labels.join(", "))
}
);
}
println!(
"{}",
serde_json::to_string(&CampaignRunLine { campaign_run: &outcome.record })
.expect("campaign run record serializes")
);
eprintln!(
"aura: campaign run {} recorded: {} cells{}",
outcome.record.run,
outcome.record.cells.len(),
if failed == 0 {
String::new()
} else {
format!(", {failed} failed ({})", fail_labels.join(", "))
}
);
// Trace persistence runs LAST, after the always-on record line (0109):
// stdout stays data-pure (the record line is the wire contract) and the
+8 -6
View File
@@ -642,10 +642,11 @@ pub fn introspect_cmd(cmd: crate::GraphIntrospectCmd, env: &aura_runner::project
} else if let Some(target) = cmd.params.as_deref() {
// --params <FILE|ID> (#196): the RAW composite param space — the one
// namespace campaign axes are validated against
// (`validate_campaign_refs`) and `aura sweep --list-axes` prints
// (#328: the wrapped `<blueprint>.<node>.<param>` form was retired —
// there is exactly one axis namespace now, so the two discovery
// surfaces agree line-for-line, see `params_lines`'s own doc comment).
// (`validate_campaign_refs`) and `exec --override`'s own resolution
// (`override_paths`, member.rs) resolves against (#328: the wrapped
// `<blueprint>.<node>.<param>` form was retired — there is exactly
// one axis namespace now, so the two surfaces agree line-for-line,
// see `params_lines`'s own doc comment).
match params_lines(target, env) {
Ok(s) => print!("{s}"),
Err(m) => {
@@ -938,8 +939,9 @@ fn resolve_blueprint_text(target: &str, env: &aura_runner::project::Env) -> Resu
/// the same form `introspect --node` already uses for param kinds. Followed
/// by one `{name}:{kind:?} default={value}` line per BOUND param (#328):
/// `bound_param_space()`'s names are already RAW (#203), so no blueprint-name
/// concatenation is needed — line-identical to the reconciled `--list-axes`
/// bound pass (`list_blueprint_axes`, main.rs), same `render_value` lexicon.
/// concatenation is needed — line-identical to the retired `aura sweep
/// --list-axes`'s own bound pass (`list_blueprint_axes`, main.rs, gone with
/// the #319 sugar retirement), same `render_value` lexicon.
/// #331 delta re-review: the FILE target is a FOURTH freshly-authored-file
/// intake this fn's own `composite_from_any` call had missed gating — a FILE
/// routes through [`composite_from_authored_text`] instead; a content id
+60 -33
View File
@@ -72,8 +72,8 @@ use aura_runner::reproduce::reproduce_family_in;
use aura_runner::measure::run_measurement;
// `render_value` is the single source (aura-runner, #295 Task 7): the
// campaign trace layout's member-key label and this shell's own
// presentation (reproduce output, `--list-axes` default printing) render a
// scalar identically.
// presentation (reproduce output, `graph introspect --params`'s bound-pass
// default printing) render a scalar identically.
use aura_runner::runner::render_value;
// Only reached from the test module's stop-regime default tests now (#319
// Task 8 — the quintet's own production callers are retired); test-only,
@@ -612,31 +612,30 @@ impl IcReport {
}
}
/// `aura sweep <blueprint.json> --list-axes`: one `<name>:<kind>` line per open
/// sweepable knob, in `param_space()` order (byte-identical to before #246),
/// followed by one `<name>:<kind> default=<value>` line per BOUND param —
/// every bound param IS a sweepable default, re-openable by naming it as an
/// `--axis` (#246), on EVERY blueprint shape (fully open, partially open, or
/// fully closed) — not only the fully-closed case: `--axis`/`override_paths`
/// accepts a bound name regardless of how many knobs happen to be open
/// alongside it, so the discovery surface must list it regardless too. Names
/// are RAW `<node>.<param>` paths (#328: the wrapped `<blueprint>.<node>.<param>`
/// form is retired from the surface) — exactly what `--axis` binds.
///
/// Lex `exec --override NODE.PARAM=VALUE` tokens (#319 Task 3/4): the same
/// int-then-float rule `parse_scalar_csv` uses for `--axis` values, shared by
/// int-then-float rule the campaign leg's `--axis` values use, shared by
/// both `exec` legs (the blueprint leg's reopen recipe and the campaign
/// leg's single-value-axis injection) so the wording and the parse rule
/// cannot drift between them. A malformed token refuses at the usage
/// boundary (exit 2) — intake, not a per-leg concern.
///
/// A repeated path (the same NODE.PARAM named twice) likewise refuses here
/// (exit 2), mirroring `--tap`'s own duplicate refusal
/// (`tap_plan_from_args`): downstream, the blueprint leg's `reopen_all`
/// folds the override set onto the signal one path at a time, so a second
/// occurrence of an already-reopened path is no longer bound and panics
/// (`NoSuchBoundParam`) — refusing the duplicate at the lexer, before either
/// leg ever resolves it, is the one gate that protects both.
fn parse_override_tokens(tokens: &[String]) -> Vec<(String, Scalar)> {
let mut ov = Vec::new();
let mut seen = BTreeSet::new();
for tok in tokens {
let Some((path_s, val_s)) = tok.split_once('=') else {
eprintln!("aura: exec: --override expects NODE.PARAM=VALUE, got `{tok}`");
std::process::exit(2);
};
if path_s.trim().is_empty() {
let path = path_s.trim();
if path.is_empty() {
eprintln!("aura: exec: --override expects NODE.PARAM=VALUE, got `{tok}`");
std::process::exit(2);
}
@@ -650,7 +649,11 @@ fn parse_override_tokens(tokens: &[String]) -> Vec<(String, Scalar)> {
}
},
};
ov.push((path_s.trim().to_string(), val));
if !seen.insert(path.to_string()) {
eprintln!("aura: exec: --override names path \"{path}\" twice");
std::process::exit(2);
}
ov.push((path.to_string(), val));
}
ov
}
@@ -659,15 +662,17 @@ fn parse_override_tokens(tokens: &[String]) -> Vec<(String, Scalar)> {
// The declarative argument grammar. clap owns argv tokenizing, scoped `--help`,
// `--version`, `--flag=value`, `--`, and long-option abbreviation; the `dispatch_*`
// handlers below convert each `*Cmd` into the argument shapes the existing execution
// fns accept, reusing the value helpers (`Selection`,
// `DataSource::from_choice`, `parse_scalar_csv`, `parse_select`,
// `parse_param_cells`). The four subcommands with an optional `[blueprint]` positional
// dispatch on `is_blueprint_file`: a first-positional naming an existing `.json` file
// selects the loaded-blueprint branch. There is no second built-in grammar anymore
// (#159 demo retirement / #220 axis generalization): without a blueprint the
// dispatcher prints a usage error and exits 2; a blueprint with `--real` routes to
// the campaign path. Usage errors exit 2; runtime refusals exit 1; a run that
// completes with ≥1 failed cell exits 3 (#272); a clean run exits 0.
// fns accept. `exec` (#319) is the one document-executing verb: it dispatches on
// `is_blueprint_file` layered with `classify_exec_document_file` (the "kind"-field
// peek) — a first-positional naming an existing `.json` file that parses as JSON
// and carries no top-level "kind" selects the loaded-blueprint leg; a file
// carrying "kind", or a target that is not a readable `.json` file at all (a
// registered content id), selects the campaign leg; a file that does not parse as
// JSON at all refuses neutrally before either leg's own validation runs.
// `--override`/`--tap` tokens are lexed by `parse_override_tokens`/
// `tap_plan_from_args` above, shared across both legs where applicable. Usage
// errors exit 2; runtime refusals exit 1; a campaign that completes with ≥1
// failed cell exits 3 (#272); a clean run exits 0.
/// The single build-time commit provenance (`option_env!("AURA_COMMIT")`,
/// falling back to `"unknown"`): both `--version` (via `version_string`) and
@@ -955,7 +960,7 @@ struct ExecCmd {
/// The file-target discriminator: a first-positional that names an existing
/// `.json` file is a document on disk. Shared by `exec`'s target routing
/// (layered with `is_campaign_document_file`) and `dispatch_graph`.
/// (layered with `classify_exec_document_file`) and `dispatch_graph`.
fn is_blueprint_file(arg: &Option<String>) -> Option<&str> {
arg.as_deref()
.filter(|a| a.ends_with(".json") && std::path::Path::new(a).is_file())
@@ -969,11 +974,19 @@ fn is_blueprint_file(arg: &Option<String>) -> Option<&str> {
/// token — it cannot tell a campaign document ending in `.json` from a
/// blueprint file, both being ordinary readable `.json` files — so `exec`'s
/// routing peeks this one cheap key before choosing a leg.
fn is_campaign_document_file(path: &str) -> bool {
std::fs::read_to_string(path)
.ok()
.and_then(|t| serde_json::from_str::<serde_json::Value>(&t).ok())
.is_some_and(|v| v.get("kind").is_some())
///
/// `Ok(true)`/`Ok(false)` classifies a file that parses as JSON at all
/// (campaign vs. not); `Err` means the file does NOT parse as JSON at all —
/// review Minor-3: a file this malformed has no leg to misattribute the
/// fault to (neither leg's own "document is not valid JSON" prose is the
/// right cause when the shape was never determined), so the caller refuses
/// neutrally instead of silently falling through to the blueprint leg.
fn classify_exec_document_file(path: &str) -> Result<bool, String> {
let text =
std::fs::read_to_string(path).map_err(|e| format!("target file is not readable: {e}"))?;
let v: serde_json::Value = serde_json::from_str(&text)
.map_err(|e| format!("target file is not valid JSON: {e}"))?;
Ok(v.get("kind").is_some())
}
/// Terminate per a campaign-path result (#272): a String error is a refusal
@@ -1017,10 +1030,24 @@ fn tap_plan_from_args(args: &[String]) -> Result<TapPlan, String> {
}
/// The one executor verb (#319): a campaign document (file or content id)
/// or a signal blueprint (single run). Flag discipline is per-leg.
/// or a signal blueprint (single run). Flag discipline is per-leg. A
/// `.json` file target is read and classified ONCE (review Minor-3): a file
/// that does not parse as JSON at all refuses neutrally here, before either
/// leg's own document-shape validation ever runs — so a malformed campaign
/// document is never misattributed to the blueprint leg's "blueprint
/// document is not valid JSON" prose.
fn dispatch_exec(a: ExecCmd, env: &aura_runner::project::Env) {
let target = Some(a.target.clone());
let blueprint_path = is_blueprint_file(&target).filter(|p| !is_campaign_document_file(p));
let file_path = is_blueprint_file(&target);
let is_campaign_file = match file_path.map(classify_exec_document_file) {
Some(Ok(is_campaign)) => is_campaign,
Some(Err(msg)) => {
eprintln!("aura: exec: {}: {msg}", file_path.expect("Some by the match arm above"));
std::process::exit(2);
}
None => false,
};
let blueprint_path = file_path.filter(|_| !is_campaign_file);
match blueprint_path {
Some(path) => exec_blueprint_leg(path, &a.r#override, &a.tap, env),
None => {