feat(cli): add ado-aw run

[jamesadevine]

Summary

Implements PR 6 of the Phase 1 CLI overhaul. Adds ado-aw run [PATH] — queues a build for every ADO definition that matches a local fixture, with optional --wait-then-exit-by-aggregate-result behaviour.

CLI surface

ado-aw run [PATH]
  --org <url-or-name>          # default: from git remote
  --project <name>             # default: from git remote
  --pat <pat>                  # else AZURE_DEVOPS_EXT_PAT, else az CLI
  --branch <ref>               # default: definition's defaultBranch
  --parameters k=v[,k=v...]    # repeatable; ADO templateParameters
  --wait                       # poll until completion
  --poll-interval <secs>       # default 10 (requires --wait)
  --timeout <secs>             # default 1800 (requires --wait)
  --dry-run

Naming nit

Module entry point is run::dispatch, not run::run, so call sites read crate::run::dispatch(...) cleanly. A doc comment in src/run.rs flags this so a later contributor doesn't "fix" the asymmetry.

Plumbing

• Implements the queue_build and get_build stubs in src/ado/mod.rs (PR refactor(cli): extract shared ADO REST helpers into src/ado/mod.rs #580).
• Two pure decision functions, 16 unit tests:
  • parse_parameters — comma-separated + repeatable, preserves = in values, rejects malformed pairs.
  • classify_build — (status, result) → InProgress / Succeeded / Failed.
• --poll-interval and --timeout are gated on --wait via clap requires; the integration test pins that constraint.
• Transient poll-time HTTP errors keep the target on the pending list and retry on the next tick; the polling loop bails on the overall --timeout.

Verified

• cargo build ✅
• cargo clippy --all-targets --all-features ✅
• cargo test ✅ — 16 new unit tests in run.rs, 2 integration tests in tests/run_integration.rs.

Out of scope

PR 7 (status), PR 8 (secrets), PR 9 (help-text grouping), PR 10 (doc overhaul).

[github-actions]

🔍 Rust PR Review

Summary: Looks good — clean implementation with solid test coverage; two minor issues worth addressing before merge.

Findings

🐛 Bugs / Logic Issues

• src/run.rs:39 — comma-split parameter values silently truncate at first comma
  parse_parameters splits every entry on , before splitting on =. A value that contains a comma (e.g. --parameters "filter=a,b") will be parsed as filter=a plus the bare token b, which then fails with "no '=' found". That's a confusing error message for what is effectively an undocumented limitation. Either document the restriction explicitly in the --parameters clap help text and error message, or switch to splitting on , only when no = appears to the right of the comma.

• src/run.rs:228 — --poll-interval 0 causes a spin-loop
  clap applies default_value_t = 10 but there is no value_parser minimum. Passing --wait --poll-interval 0 will call tokio::time::sleep(Duration::from_secs(0)) in a tight loop against the ADO API for up to --timeout seconds. A simple value_parser = clap::value_parser!(u64).range(1..) on the poll_interval field would guard this.

⚠️ Suggestions

• src/run.rs:277 — permanent 404s are silently retried until timeout
  The comment says "treat transient poll errors as still-in-progress", which is reasonable, but get_build returns an Err for any non-2xx response (including a permanent 404 if a build was deleted or the ID was wrong). The final summary then shows those as in_progress rather than failed. Consider checking whether the error is likely permanent (e.g. by inspecting the HTTP status code or the error message string) and counting those as failed rather than in_progress. At minimum, a warning that distinguishes "transient" from "permanent" would avoid a confusing wait-then-timeout result.

• src/ado/mod.rs:1076 — templateParameters values aren't validated as strings
  The queue_build signature takes &serde_json::Map<String, serde_json::Value> and passes it through to ADO as-is. parse_parameters in run.rs always inserts serde_json::Value::String(...), so today every call-site is safe. But the function contract doesn't enforce string values, and a future caller (e.g. safeoutputs/queue_build.rs) could pass non-string values that ADO would reject with an opaque 400. Adding a note to the doc comment (or an assertion/validation inside queue_build) would protect future callers.

✅ What Looks Good

• parse_parameters using split_once('=') correctly preserves = in values — exactly right.
• classify_build is a pure, well-tested function with all ADO result variants covered (canceled, partiallySucceeded, missing result field).
• The transient-error retry loop (keep-pending, sleep, retry) is clean and the timeout path exits gracefully.
• clap requires = "wait" constraint on --poll-interval / --timeout is properly tested in tests/run_integration.rs.
• dispatch entry point naming is a nice convention; the module-level doc comment preventing future "fixes" is appreciated.

Generated by Rust PR Reviewer for issue #595 · ● 360.4K · ◷

[jamesadevine]

Superseded by #602 (eat/cli-lifecycle-family), which consolidates all six in-flight Phase 1 lifecycle PRs into a single linear-history branch. Closing this PR; review feedback should land on #602.