feat(cli): consolidate Phase 1 pipeline-lifecycle commands (disable/remove/list/status/run/secrets) (#602)

* feat(cli): add ado-aw disable

Squash-merge of #591 (feat/cli-disable). See original PR for review history and detailed rationale.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* feat(cli): add ado-aw remove

Squash-merge of #592 (feat/cli-remove).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* feat(cli): add ado-aw list

Squash-merge of #594 (feat/cli-list).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* feat(cli): add ado-aw status

Implements PR 7 of the Phase 1 CLI overhaul. Renders per-pipeline
status — name, id, folder, queueStatus, latest-run summary, and a
deep link — one block per matched ADO definition. Read-only.

`status` is intentionally a thin renderer over the same data path as
`list` (same `list_definitions` + `match_definitions` +
`get_latest_build` sequence). The `--json` shape is byte-for-byte
identical to `list --json` so scripts can use either.

CLI surface:

    ado-aw status [PATH] --org --project --pat --json

The block renderer is a pure function (`render_blocks`) tested
against six scenarios:

- empty → placeholder line
- succeeded run with url → all fields rendered
- run with no url → synthesizes
  `{org_url}/{project}/_build/results?buildId={id}`
- no last run → "never" + no url line
- in-progress run (no `result` yet) → shows `status` value instead
- missing queueStatus → renders `?` placeholder

Depends on PR 5 (#594) for `crate::list::{ListRow, LastRun,
build_rows, render_json}`; reuses the `get_latest_build` ado/mod.rs
helper landed there.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(cli): avoid duplicate ADO definition fetch in list and status

Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/c7c2866a-891d-48c3-8336-774bba086817

Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>

* feat(cli): add ado-aw run

Squash-merge of #595 (feat/cli-run).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* feat(cli): rename configure to secrets with subcommands

Squash-merge of #600 (feat/cli-secrets).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(cli): percent-encode ctx.project in all ADO URL formatters

Code-review finding on PR #602: seven URL formatters in
`src/ado/mod.rs` interpolated `ctx.project` directly into the path
segment of the request URL while five sibling functions correctly
ran the value through `percent_encoding::utf8_percent_encode(...,
PATH_SEGMENT)`. Project names containing reserved characters
(spaces, `/`, `?`, `#`, `:` etc.) would have broken the URL or
silently produced surprising responses.

Affected functions, all now using the same encoder as
`get_repository_id`, `get_definition_full`, `patch_queue_status`,
`delete_definition`, and `create_definition`:

- `list_definitions`
- `get_definition_name`
- `update_pipeline_variable` (both GET and PUT URLs)
- `queue_build`
- `get_build`
- `get_latest_build`

The `info!()` log line at the top of `match_definitions` is
unaffected (logging, not URL construction).

The existing `path_segment_*` tests already cover the encoder
behaviour; no new test is needed since these are mechanical
substitutions of an existing pattern. Full `cargo test` (1567 unit
tests + integration crates) and `cargo clippy --all-targets
--all-features` are green after the fix.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(run): re-check --timeout between each in-flight get_build call

Code-review follow-up on PR #602: `poll_until_complete` only checked
`started.elapsed() >= timeout` at the top of each round, so with N
in-flight builds and reqwest's 30s per-call HTTP timeout, the
operator-visible wait could overshoot `--timeout` by up to N × 30s
in the pathological all-stalled case.

Re-checks the wall-clock budget between each individual `get_build`
call inside a round. When the budget is exhausted mid-round, the
current target and every remaining one are carried forward into
`pending` so the caller's `in_progress` count stays accurate (the
loop owes a status for everything it queued).

In the common case where the poll interval is several times the HTTP
timeout, the previous behaviour was indistinguishable from the new
one — the bug only matters when poll interval ≪ HTTP timeout, which
is an awkward but plausible configuration.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(cli): percent-encode project in secrets/run; make --value-stdin conflict explicit

Three follow-ups on PR #602:

1. `src/secrets.rs::put_definition` was the last URL formatter using
   `ctx.project` unencoded. Now uses `PATH_SEGMENT` like every other
   builder in `src/ado/mod.rs`. `PATH_SEGMENT` was promoted from
   private `const` to `pub const` to support cross-module reuse.

2. `src/run.rs` was printing a deep-link to the queued build using
   unencoded `ado_ctx.project`. The URL is cosmetic (never used as an
   HTTP target), but it would render broken/unclickable for projects
   containing spaces or other URL-unsafe characters. Now encoded with
   the same `PATH_SEGMENT` encoder.

3. `ado-aw secrets set <value> --value-stdin` silently ignored
   `--value-stdin` when both were supplied (explicit positional value
   won). Added `conflicts_with = "value"` to the `value_stdin` clap
   arg so the combination is rejected at parse time with a clear
   error. Added an integration test in
   `tests/secrets_integration.rs` to pin the behaviour.

`cargo test` (1567 unit + 14 integration crates, all green) and
`cargo clippy --all-targets --all-features` pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(cli): encode synthesized URL, surface detect errors, document parse_parameters edge

Four follow-ups on PR #602:

1. `src/status.rs::render_blocks` synthesized fallback URL was passing
   `ado_project` verbatim into the path segment when `LastRun::url`
   was absent. Now uses `PATH_SEGMENT` like every other URL builder
   in the PR. URL is text-output only, but renders broken links for
   projects with spaces or reserved chars.

2. `src/list.rs` and `src/status.rs` were swallowing
   `detect_pipelines` errors via `.unwrap_or_default()`, making
   "detection failed" indistinguishable from "no pipelines compiled
   here" — both produce zero matches downstream. Both commands are
   read-only and useful even with partial inputs (`list --all`
   doesn't need fixtures at all), so we don't bail; we emit a
   `warning: failed to scan local pipelines: …` to stderr so the
   operator can distinguish the two cases.

3. `src/run.rs::parse_parameters` silently rejects values containing
   commas (the `,` split happens before the `=` split, so the
   trailing fragment falls into the "no `=`" rejection path). The
   behaviour is intentional — commas are the documented pair
   separator — but it was undocumented. Added a doc comment
   spelling out the constraint and the one-pair-per-flag
   workaround, plus a new
   `parse_parameters_values_with_commas_split_pre_equals` unit test
   that pins both the rejection and the workaround. The doc comment
   tells future contributors to update the test if comma escaping
   is ever added.

4. `src/secrets.rs::run_set_github_token` carries an undocumented
   invariant: the deprecation warning must be emitted before any
   fallible I/O, because the integration test
   `configure_invocation_still_works_and_warns` exercises it by
   driving the function with a path that triggers an early
   canonicalize failure. Added an `IMPORTANT — invariant for the
   integration test` doc comment so a later refactor that defers
   the `eprintln!` (e.g. lazy auth init) will spot the constraint.

`cargo test` (1568 unit + 14 integration crates, all green) and
`cargo clippy --all-targets --all-features` pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(cli): correct parse_parameters doc; cap consecutive poll errors; warn on empty status

Three follow-ups on PR #602:

1. `parse_parameters` doc-comment bug: the first bullet was tagged
   ✅ but described the same broken case as the third (✅
   `--parameters 'urls=a,b' --parameters mode=fast` still splits on
   the comma inside `urls=a,b` and fails on the trailing `b`
   fragment). Rewrote the bullet list so all broken examples are ❌
   and only the genuine "one pair per flag, no commas in values"
   workaround is ✅. Also clarified that there is currently no way
   to escape a comma inside a single `--parameters` argument, and
   pointed at the existing
   `parse_parameters_values_with_commas_split_pre_equals` unit
   test as the behaviour anchor.

2. `poll_until_complete` couldn't distinguish a permanent error
   (deleted build, revoked PAT, 404) from a transient one — both
   pushed the target back onto `next_pending` and silently retried
   until `--timeout`. Added a per-build `consecutive_errors:
   HashMap<u64, usize>` counter that resets on any successful poll
   and bails out of that specific build after
   `MAX_CONSECUTIVE_POLL_ERRORS = 3` consecutive failures, counting
   it as failed. Transient blips still retry; persistent failures
   surface within `3 × poll_interval` (default 30s) instead of
   waiting out the full `--timeout` (default 1800s).

3. `status` was silently rendering `(no matched definitions)` when
   the fixture matcher returned zero hits, which is
   indistinguishable from running in the wrong directory. Added an
   `eprintln!` warning that mirrors the existing
   `failed to scan local pipelines: …` message. The command stays
   non-fatal (read-only) by design, unlike `disable` which bails.

`cargo test` (1568 unit + 14 integration crates, all green) and
`cargo clippy --all-targets --all-features` pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(cli): no silent allowOverride downgrade; surface comma hint; harden dry-run

Four follow-ups on PR #602:

1. **`apply_variable_set`: silent `allowOverride` downgrade on
   secret rotation.** Previously, running `secrets set TOKEN <new>`
   without `--allow-override` would re-emit the variable with
   `allowOverride: false`, silently downgrading any variable that
   was previously created (manually or by another tool) with
   `allowOverride: true`. The legacy `configure` code in
   src/configure.rs had explicit preservation logic; the
   consolidated `apply_variable_set` had lost it.

   Changed the signature from `allow_override: bool` to
   `allow_override: Option<bool>`:
   - `Some(true)` / `Some(false)` — force the flag (CLI
     `--allow-override` passes `Some(true)`).
   - `None` — **preserve** existing variable's `allowOverride`
     when overwriting; default to `false` when creating.

   `run_set` translates the CLI flag: `--allow-override` → `Some(true)`;
   absence → `None`. The deprecation alias (`run_set_github_token`)
   stays at `allow_override: false` on the CLI side, which now maps
   to `None` (preserve) — restoring parity with the pre-consolidation
   `configure` body. Help text in `src/main.rs` and `docs/cli.md`
   updated. Five new unit tests pin the matrix:
   - `Some(true)` / `Some(false)` / `None` × create/overwrite
   - Specifically asserts `None` preserves `allowOverride: true`
     (the silent-downgrade regression guard).

2. **`run.rs::print_queue_plan` silent serialize-failure.**
   `serde_json::to_string_pretty(&body).unwrap_or_default()` would
   have printed blank output if serialization ever failed. The
   value is provably JSON-safe, but defensive code should surface
   regressions instead of silently swallowing them. Switched to
   `unwrap_or_else(|e| format!("<serialization error: {e}>"))`.

3. **`run.rs::parse_parameters` opaque comma-in-value error.**
   When a user writes `--parameters urls=https://a,b`, the error
   was `Invalid --parameters pair 'b': expected key=value (no '='
   found).` — technically accurate but doesn't hint at the comma
   constraint documented above the function. Added a
   raw-argument-contains-comma detection branch that produces a
   self-diagnosable hint: `... Hint: values must not contain
   commas. The raw argument '...' was split on ',' before the
   '=' split; use a separate --parameters flag per pair.`

4. **`run.rs::dispatch` deliberate partial-queue + `--wait`
   behaviour.** When `--wait` is set and some builds fail to
   queue, the code polls the successfully-queued ones rather than
   bailing early; `queue_failure` is folded into the final exit
   code. This is intentional and the only sensible UX, but lacked
   a comment. Added a multi-paragraph block explaining all three
   cases (partial queue, zero queued, all queued) and why
   `poll_until_complete` is called with the partial slice.

Not addressed (acknowledged follow-ups, tracked elsewhere):
- Sequential `get_latest_build` fanout in `list`/`status`. Already
  documented inline; tracked as a future improvement.

`cargo test` (1572 unit + 14 integration crates, all green) and
`cargo clippy --all-targets --all-features` pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(cli): remove bails when no fixtures found; surface --parameters comma constraint in --help

Two follow-ups on PR #602:

1. **`remove` silently exited Ok(()) when no fixtures were
   detected.** For a destructive command this is the wrong UX —
   running `ado-aw remove` in the wrong directory currently
   printed "No agentic pipelines found." and exited success,
   giving no signal that nothing happened. Now mirrors `disable`:
   bails with a non-zero exit and tells the operator which path
   was scanned plus the recovery hint:

       No local agentic pipeline fixtures were found under <path>.
       Run `ado-aw compile` first (or point `ado-aw remove` at the
       repo root). `remove` refuses to exit success in this state
       because it's destructive.

2. **`--parameters` comma constraint was documented in the module
   doc-comment but not in `--help` text.** A user who writes
   `--parameters redirect_uri=https://a,b` would only learn about
   the constraint by reading the source. Added an inline
   `VALUES MUST NOT CONTAIN COMMAS …` blurb to the clap `help`
   attribute and updated `docs/cli.md` to match. The integration
   test now asserts the constraint appears in `--help` so a
   refactor that drops the warning will be caught at CI.

`cargo test` (1572 unit + 14 integration crates, all green) and
`cargo clippy --all-targets --all-features` pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>

Author: 3 people

AGENTS.md

@@ -75,8 +75,14 @@ Every compiled pipeline runs as three sequential jobs:
 │   ├── fuzzy_schedule.rs # Fuzzy schedule parsing
 │   ├── logging.rs        # File-based logging infrastructure
 │   ├── mcp.rs            # SafeOutputs MCP server (stdio + HTTP)
-│   ├── configure.rs      # `configure` CLI command — orchestration shim atop `src/ado/`
+│   ├── configure.rs      # `configure` CLI command (deprecated) — hidden alias forwarding to `secrets set GITHUB_TOKEN`
+│   ├── secrets.rs        # `secrets set/list/delete` subcommand group — manages pipeline variables (never prints values from `list`)
 │   ├── enable.rs         # `enable` CLI command — registers ADO build definitions for compiled pipelines and ensures they are enabled
+│   ├── disable.rs        # `disable` CLI command — sets queueStatus to disabled (default) or paused on matched definitions
+│   ├── remove.rs         # `remove` CLI command — deletes matched ADO build definitions (with --yes / tty-prompt safety)
+│   ├── list.rs           # `list` CLI command — renders matched ADO definitions with their latest-run state (text or JSON)
+│   ├── status.rs         # `status` CLI command — denser per-pipeline status block (thin renderer over `list`'s data path)
+│   ├── run.rs            # `run` CLI command — queues builds for matched definitions, optional polling to completion (module entry is `dispatch`)
 │   ├── ado/              # Shared Azure DevOps REST helpers (auth, list/match/PATCH/POST)
 │   │   └── mod.rs        # Used by `configure` and the `enable` command (ADO REST helpers: auth, list/match/PATCH/POST)
 │   ├── detect.rs         # Agentic pipeline detection (helper for `configure`)

docs/cli.md

@@ -35,14 +35,23 @@ Global flags (apply to all subcommands): `--verbose, -v` (enable info-level logg
   - `--ado-project <name>` - Azure DevOps project name override
   - `--dry-run` - Validate inputs but skip ADO API calls (useful for local testing and QA review)
 
-- `configure` - Detect agentic pipelines in a local repository and update the `GITHUB_TOKEN` pipeline variable on their Azure DevOps build definitions
-  - `--token <token>` / `GITHUB_TOKEN` env var - The new GITHUB_TOKEN value (prompted if omitted)
-  - `--org <url>` - Override: Azure DevOps organization URL (e.g. `https://dev.azure.com/myorg`) or just the org name (e.g. `myorg`, auto-prefixed to the canonical URL). Inferred from git remote by default.
-  - `--project <name>` - Override: Azure DevOps project name (inferred from git remote by default)
-  - `--pat <pat>` / `AZURE_DEVOPS_EXT_PAT` env var - PAT for ADO API authentication (prompted if omitted)
-  - `--path <path>` - Path to the repository root (defaults to current directory)
-  - `--dry-run` - Preview changes without applying them
-  - `--definition-ids <ids>` - Explicit pipeline definition IDs to update (comma-separated, skips auto-detection)
+- `configure` *(deprecated; hidden in --help)* - Alias forwarding to `secrets set GITHUB_TOKEN`. Existing scripts keep working but get a stderr warning. The alias will be removed in the next minor release.
+
+- `secrets set <name> [<value>] [PATH]` - Set a pipeline variable (with `isSecret=true`) on every matched ADO definition. Value resolution: positional `<value>` → `--value-stdin` (one line) → interactive tty prompt with echo off.
+  - `--allow-override` - Force `allowOverride=true` on the set variable. When omitted, `allowOverride` is **preserved** on existing variables (so secret rotation does not silently downgrade an existing `allowOverride=true`) and defaults to `false` for new variables.
+  - `--value-stdin` - Read the value from a single line on stdin.
+  - `--dry-run` - Print the planned set without calling the ADO API.
+  - `--org / --project / --pat` - ADO context overrides (same semantics as the other lifecycle commands).
+  - `--definition-ids <ids>` - Explicit pipeline definition IDs (comma-separated; skips local-fixture auto-detection).
+
+- `secrets list [PATH]` - List variable names and their `isSecret` / `allowOverride` flags on every matched definition. **Never prints values.**
+  - `--json` - Emit machine-readable JSON.
+  - `--org / --project / --pat / --definition-ids` - As above.
+
+- `secrets delete <name> [PATH]` - Delete the named variable from every matched definition. No-op when the variable is absent.
+  - `--dry-run` - Print the planned deletion plan without calling the ADO API.
+  - `--org / --project / --pat / --definition-ids` - As above.
+
 
 - `enable [PATH]` - Register an ADO build definition for each compiled pipeline discovered under `PATH` (or the current directory) and ensure it is `enabled`. For each fixture, matches against the existing ADO definitions by `yamlFilename` first, then by sanitized display name; creates a new definition when neither matches, flips `queueStatus` to `enabled` when an existing definition is `disabled` / `paused`, and skips when it is already `enabled`. Fail-soft per fixture; exits non-zero if any fixture failed.
   - `--org <url>` - Override: Azure DevOps organization (URL or bare org name). Inferred from git remote by default.
@@ -55,3 +64,42 @@ Global flags (apply to all subcommands): `--verbose, -v` (enable info-level logg
   - `--token <value>` - The token value for `--also-set-token`. Falls back to `$GITHUB_TOKEN`, then to an interactive prompt. Requires `--also-set-token`.
 
   **Source-repo scope (Phase 1):** `enable` requires the local git remote to be an Azure DevOps Git remote (the source repo is what gets registered as the definition's repository). GitHub-hosted source repos are gated on a follow-up.
+
+- `disable [PATH]`- Set `queueStatus` to `disabled` (default) or `paused` on every ADO build definition that matches a local fixture under `PATH`. Refuses to touch any ADO definition that is not the target of a local fixture match — that safety property falls naturally out of the same yaml-path + name match used by `configure`. Skips definitions that are already at the requested status; fail-soft per fixture; exits non-zero if any patch failed or if zero local fixtures matched ADO definitions.
+  - `--org <url>` - Override: Azure DevOps organization (URL or bare org name). Inferred from git remote by default.
+  - `--project <name>` - Override: Azure DevOps project name (inferred from git remote by default).
+  - `--pat <pat>` / `AZURE_DEVOPS_EXT_PAT` env var - PAT for ADO API authentication (Azure CLI fallback if omitted).
+  - `--paused` - Use `queueStatus: paused` instead of `disabled`. Paused definitions still queue scheduled runs but the queue is held; disabled definitions reject all queue requests.
+  - `--dry-run` - Print the planned `from → to` transitions without calling the ADO API.
+
+- `remove [PATH]` - **Destructive.** Delete every ADO build definition that matches a local fixture under `PATH`. The same `match_definitions` safety property as `disable` applies: definitions without a local fixture are never in scope. Bulk deletes (`>1` match) require `--yes`; a single match on a tty prompts interactively (`y/N`); non-tty contexts always require `--yes`. Fail-soft per fixture; exits non-zero if any deletion failed or if zero local fixtures matched ADO definitions.
+  - `--org <url>` - Override: Azure DevOps organization (URL or bare org name). Inferred from git remote by default.
+  - `--project <name>` - Override: Azure DevOps project name (inferred from git remote by default).
+  - `--pat <pat>` / `AZURE_DEVOPS_EXT_PAT` env var - PAT for ADO API authentication (Azure CLI fallback if omitted).
+  - `--yes` - Required for bulk deletes (>1 match) and for any delete in a non-tty context. A single match on a tty otherwise prompts interactively.
+  - `--dry-run` - Print the planned deletions without calling the ADO API.
+
+- `list [PATH]` - Render every ADO build definition that matches a local fixture (under `PATH`) along with its `queueStatus`, ADO folder, and latest-run summary. Pass `--all` to also include definitions with no matching local fixture. Output defaults to a human-readable table; `--json` emits a stable JSON array suitable for scripting.
+  - `--org <url>` - Override: Azure DevOps organization (URL or bare org name). Inferred from git remote by default.
+  - `--project <name>` - Override: Azure DevOps project name (inferred from git remote by default).
+  - `--pat <pat>` / `AZURE_DEVOPS_EXT_PAT` env var - PAT for ADO API authentication (Azure CLI fallback if omitted).
+  - `--all` - Include ADO definitions that do not match any local fixture.
+  - `--json` - Emit machine-readable JSON.
+
+- `status [PATH]` - Per-pipeline status: name, id, folder, `queueStatus`, latest-run summary, and a deep link — one block per matched definition. Read-only. `--json` emits the same shape as `list --json` so scripts can use either.
+  - `--org <url>` - Override: Azure DevOps organization (URL or bare org name). Inferred from git remote by default.
+  - `--project <name>` - Override: Azure DevOps project name (inferred from git remote by default).
+  - `--pat <pat>` / `AZURE_DEVOPS_EXT_PAT` env var - PAT for ADO API authentication (Azure CLI fallback if omitted).
+  - `--json` - Emit machine-readable JSON (same shape as `list --json`).
+
+- `run [PATH]` - Queue an ADO build for every ADO definition that matches a local fixture (under `PATH`). With `--wait`, poll each queued build until completion and exit with an aggregate result — 0 only if every queued build succeeded.
+  - `--org <url>` - Override: Azure DevOps organization (URL or bare org name). Inferred from git remote by default.
+  - `--project <name>` - Override: Azure DevOps project name (inferred from git remote by default).
+  - `--pat <pat>` / `AZURE_DEVOPS_EXT_PAT` env var - PAT for ADO API authentication (Azure CLI fallback if omitted).
+  - `--branch <ref>` - Source branch to queue. Defaults to the definition's `defaultBranch`.
+  - `--parameters <k=v[,k=v...]>` - ADO `templateParameters`. Repeatable and/or comma-separated. All values are strings (ADO coerces as the definition requires). Rejects malformed pairs (missing `=`). **Values must not contain commas** — each raw argument is split on `,` before the `=` split, so `key=https://a,b` is rejected. Use one `--parameters` flag per pair when values contain commas.
+  - `--wait` - Poll each queued build to completion before exiting.
+  - `--poll-interval <secs>` - Polling period when `--wait` is set (default 10).
+  - `--timeout <secs>` - Hard cap on the polling loop when `--wait` is set (default 1800).
+  - `--dry-run` - Print the planned `templateParameters` body without calling the ADO API.
+