diff --git a/AGENTS.md b/AGENTS.md index 49561a50..bced2c45 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -77,6 +77,7 @@ Every compiled pipeline runs as three sequential jobs: │ ├── mcp.rs # SafeOutputs MCP server (stdio + HTTP) │ ├── configure.rs # `configure` CLI command — orchestration shim atop `src/ado/` │ ├── enable.rs # `enable` CLI command — registers ADO build definitions for compiled pipelines and ensures they are enabled +│ ├── remove.rs # `remove` CLI command — deletes matched ADO build definitions (with --yes / tty-prompt safety) │ ├── ado/ # Shared Azure DevOps REST helpers (auth, list/match/PATCH/POST) │ │ └── mod.rs # Used by `configure` and the lifecycle commands (enable, disable, remove, list, run, status, secrets) │ ├── detect.rs # Agentic pipeline detection (helper for `configure`) diff --git a/docs/cli.md b/docs/cli.md index e4144da3..14a2b440 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -55,3 +55,10 @@ Global flags (apply to all subcommands): `--verbose, -v` (enable info-level logg - `--token ` - 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. + +- `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 ` - Override: Azure DevOps organization (URL or bare org name). Inferred from git remote by default. + - `--project ` - Override: Azure DevOps project name (inferred from git remote by default). + - `--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. diff --git a/src/ado/mod.rs b/src/ado/mod.rs index d7793ddb..7d133d26 100644 --- a/src/ado/mod.rs +++ b/src/ado/mod.rs @@ -977,12 +977,38 @@ pub async fn patch_queue_status( /// /// Calls `DELETE /_apis/build/definitions/{id}?api-version=7.1`. pub async fn delete_definition( - _client: &reqwest::Client, - _ctx: &AdoContext, - _auth: &AdoAuth, - _id: u64, + client: &reqwest::Client, + ctx: &AdoContext, + auth: &AdoAuth, + id: u64, ) -> Result<()> { - anyhow::bail!("not yet implemented: filled in by PR 4 (ado-aw remove)") + let url = format!( + "{}/{}/_apis/build/definitions/{}?api-version=7.1", + ctx.org_url.trim_end_matches('/'), + percent_encoding::utf8_percent_encode(&ctx.project, PATH_SEGMENT), + id + ); + + debug!("DELETE definition {}: {}", id, url); + + let resp = auth + .apply(client.delete(&url)) + .send() + .await + .with_context(|| format!("Failed to delete definition {}", id))?; + + let status = resp.status(); + if !status.is_success() { + let body = resp.text().await.unwrap_or_default(); + anyhow::bail!( + "ADO API returned {} when deleting definition {}: {}", + status, + id, + body + ); + } + + Ok(()) } /// Create a new build definition. diff --git a/src/main.rs b/src/main.rs index 9d19f316..b44186eb 100644 --- a/src/main.rs +++ b/src/main.rs @@ -14,6 +14,7 @@ mod init; mod logging; mod mcp; mod ndjson; +mod remove; pub mod runtimes; pub mod sanitize; mod safeoutputs; @@ -174,6 +175,30 @@ enum Commands { #[arg(long, requires = "also_set_token")] token: Option, }, + /// Delete every ADO build definition that matches a local fixture. + Remove { + /// Path to the repository root (defaults to current directory). Used + /// to auto-discover compiled pipelines, same as `compile`. + path: Option, + /// Override: Azure DevOps organization (URL like `https://dev.azure.com/myorg`, + /// or just the org name `myorg`). Inferred from git remote by default. + #[arg(long)] + org: Option, + /// Override: Azure DevOps project name (inferred from git remote by default). + #[arg(long)] + project: Option, + /// PAT for ADO API authentication (prefer setting AZURE_DEVOPS_EXT_PAT env var; + /// Azure CLI fallback if omitted). + #[arg(long, env = "AZURE_DEVOPS_EXT_PAT")] + pat: Option, + /// Required for bulk deletes (>1 match) and for any delete in a non-tty + /// context. A single match on a tty otherwise prompts interactively. + #[arg(long)] + yes: bool, + /// Preview the planned deletions without calling the ADO API. + #[arg(long)] + dry_run: bool, + }, } #[derive(Parser, Debug)] @@ -524,6 +549,7 @@ async fn main() -> Result<()> { Some(Commands::Init { .. }) => "init", Some(Commands::Configure { .. }) => "configure", Some(Commands::Enable { .. }) => "enable", + Some(Commands::Remove { .. }) => "remove", None => "ado-aw", }; @@ -656,6 +682,24 @@ async fn main() -> Result<()> { }) .await?; } + Commands::Remove { + path, + org, + project, + pat, + yes, + dry_run, + } => { + remove::run(remove::RemoveOptions { + org: org.as_deref(), + project: project.as_deref(), + pat: pat.as_deref(), + path: path.as_deref(), + yes, + dry_run, + }) + .await?; + } } Ok(()) } diff --git a/src/remove.rs b/src/remove.rs new file mode 100644 index 00000000..e7572daf --- /dev/null +++ b/src/remove.rs @@ -0,0 +1,238 @@ +//! The `remove` CLI command. +//! +//! Deletes every ADO build definition that matches a local fixture. +//! Phase 1 of the pipeline-lifecycle CLI family — see `docs/cli.md`. +//! +//! Safety: +//! +//! - Refuses to delete any ADO definition that is not matched against +//! a local fixture (this falls naturally out of [`match_definitions`]). +//! - Bulk deletes (`> 1` match) require `--yes`. Single-match deletes +//! require either `--yes` or an interactive `y/N` confirmation on +//! a tty; non-tty contexts always require `--yes`. + +use anyhow::{Context, Result}; +use std::io::IsTerminal; +use std::path::{Path, PathBuf}; + +use crate::ado::{ + MatchedDefinition, delete_definition, match_definitions, resolve_ado_context, resolve_auth, +}; +use crate::detect; + +/// Pure decision for the confirm-or-not gate. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Confirm { + /// Proceed without prompting (either `--yes` or `--dry-run`). + Proceed, + /// Prompt the operator interactively on the tty before deleting. + PromptTty, + /// Bail out — the operator must re-run with `--yes`. The string + /// is the user-visible reason. + RequireYes(String), +} + +/// Pure function: decide the gating mode for a remove operation. +/// +/// Returns [`Confirm::Proceed`] when the caller explicitly opted into +/// the operation, [`Confirm::PromptTty`] for a single match on a tty, +/// or [`Confirm::RequireYes`] when the operator must rerun with +/// `--yes` (bulk deletes, single-match in non-tty). +pub fn decide_confirm( + match_count: usize, + yes: bool, + dry_run: bool, + is_tty: bool, +) -> Confirm { + if dry_run || yes { + return Confirm::Proceed; + } + if match_count > 1 { + return Confirm::RequireYes(format!( + "{} definitions would be deleted; rerun with --yes to confirm.", + match_count + )); + } + if match_count == 1 { + if is_tty { + return Confirm::PromptTty; + } + return Confirm::RequireYes( + "stdin is not a tty; rerun with --yes to confirm.".to_string(), + ); + } + Confirm::Proceed +} + +/// CLI options for [`run`]. +pub struct RemoveOptions<'a> { + pub org: Option<&'a str>, + pub project: Option<&'a str>, + pub pat: Option<&'a str>, + pub path: Option<&'a Path>, + pub yes: bool, + pub dry_run: bool, +} + +/// Run the `remove` command. +pub async fn run(opts: RemoveOptions<'_>) -> Result<()> { + let repo_path: PathBuf = match opts.path { + Some(p) => tokio::fs::canonicalize(p) + .await + .with_context(|| format!("Could not resolve path: {}", p.display()))?, + None => tokio::fs::canonicalize(".") + .await + .context("Could not resolve current directory")?, + }; + + let auth = resolve_auth(opts.pat).await?; + let ado_ctx = resolve_ado_context(&repo_path, opts.org, opts.project).await?; + + println!( + "ADO context: org={}, project={}", + ado_ctx.org_url, ado_ctx.project + ); + println!(); + + println!("Scanning for agentic pipelines..."); + let detected = detect::detect_pipelines(&repo_path).await?; + if detected.is_empty() { + println!( + "No agentic pipelines found. Make sure your pipelines were compiled with the latest ado-aw." + ); + return Ok(()); + } + println!("Found {} agentic pipeline(s).", detected.len()); + println!(); + + let client = reqwest::Client::builder() + .timeout(std::time::Duration::from_secs(30)) + .build() + .context("Failed to create HTTP client")?; + + println!("Matching to Azure DevOps pipeline definitions..."); + let matched = match_definitions(&client, &ado_ctx, &auth, &detected).await?; + + if matched.is_empty() { + anyhow::bail!( + "No ADO definitions matched any local fixture. Run `ado-aw list` to \ + diagnose; nothing to delete." + ); + } + + println!("{} definition(s) would be deleted:", matched.len()); + for m in &matched { + println!(" - {} (id={})", m.name, m.id); + } + println!(); + + let confirm = decide_confirm( + matched.len(), + opts.yes, + opts.dry_run, + std::io::stdin().is_terminal(), + ); + match confirm { + Confirm::Proceed => {} + Confirm::RequireYes(reason) => anyhow::bail!("{}", reason), + Confirm::PromptTty => { + if !prompt_yes_no(&matched[0])? { + println!("Aborted by user."); + return Ok(()); + } + } + } + + let mut success = 0usize; + let mut failure = 0usize; + for m in &matched { + if opts.dry_run { + println!("[dry-run] ✓ would delete: {} (id={})", m.name, m.id); + success += 1; + continue; + } + match delete_definition(&client, &ado_ctx, &auth, m.id).await { + Ok(()) => { + println!("✓ deleted: {} (id={})", m.name, m.id); + success += 1; + } + Err(e) => { + eprintln!("✗ failed: {} (id={}): {:#}", m.name, m.id, e); + failure += 1; + } + } + } + + println!(); + println!("Done: {} succeeded, {} failed.", success, failure); + if failure > 0 { + anyhow::bail!("{} deletion(s) failed", failure); + } + Ok(()) +} + +fn prompt_yes_no(m: &MatchedDefinition) -> Result { + let prompt = format!("Delete '{}' (id={})?", m.name, m.id); + inquire::Confirm::new(&prompt) + .with_default(false) + .prompt() + .context("Failed to read confirmation from interactive prompt") +} + +#[cfg(test)] +mod tests { + use super::*; + + // ============ decide_confirm matrix ============ + + #[test] + fn dry_run_always_proceeds() { + assert_eq!(decide_confirm(0, false, true, false), Confirm::Proceed); + assert_eq!(decide_confirm(1, false, true, false), Confirm::Proceed); + assert_eq!(decide_confirm(5, false, true, false), Confirm::Proceed); + assert_eq!(decide_confirm(5, false, true, true), Confirm::Proceed); + } + + #[test] + fn yes_always_proceeds() { + assert_eq!(decide_confirm(0, true, false, false), Confirm::Proceed); + assert_eq!(decide_confirm(1, true, false, false), Confirm::Proceed); + assert_eq!(decide_confirm(5, true, false, false), Confirm::Proceed); + assert_eq!(decide_confirm(5, true, false, true), Confirm::Proceed); + } + + #[test] + fn bulk_without_yes_requires_yes_even_on_tty() { + match decide_confirm(3, false, false, true) { + Confirm::RequireYes(reason) => { + assert!(reason.contains("3 definitions"), "got: {}", reason); + assert!(reason.contains("--yes"), "got: {}", reason); + } + other => panic!("expected RequireYes, got {:?}", other), + } + } + + #[test] + fn single_match_on_tty_prompts() { + assert_eq!(decide_confirm(1, false, false, true), Confirm::PromptTty); + } + + #[test] + fn single_match_non_tty_requires_yes() { + match decide_confirm(1, false, false, false) { + Confirm::RequireYes(reason) => { + assert!(reason.contains("tty"), "got: {}", reason); + assert!(reason.contains("--yes"), "got: {}", reason); + } + other => panic!("expected RequireYes, got {:?}", other), + } + } + + #[test] + fn zero_matches_proceeds_so_caller_can_handle() { + // The empty case is handled earlier in `run` (bail with hint) + // but the gate itself shouldn't block. + assert_eq!(decide_confirm(0, false, false, true), Confirm::Proceed); + assert_eq!(decide_confirm(0, false, false, false), Confirm::Proceed); + } +} diff --git a/tests/remove_integration.rs b/tests/remove_integration.rs new file mode 100644 index 00000000..3fff6e5c --- /dev/null +++ b/tests/remove_integration.rs @@ -0,0 +1,41 @@ +//! Integration tests for `ado-aw remove`. + +use std::path::PathBuf; + +fn binary() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_ado-aw")) +} + +#[test] +fn remove_help_describes_command() { + let output = std::process::Command::new(binary()) + .args(["remove", "--help"]) + .output() + .expect("Failed to run ado-aw remove --help"); + assert!(output.status.success(), "--help should exit 0"); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout.contains("Delete every ADO build definition"), + "Help text should describe the remove command, got:\n{stdout}" + ); + for flag in ["--org", "--project", "--pat", "--yes", "--dry-run"] { + assert!( + stdout.contains(flag), + "Expected --help to advertise {flag}, got:\n{stdout}" + ); + } +} + +#[test] +fn remove_is_listed_in_top_level_help() { + let output = std::process::Command::new(binary()) + .arg("--help") + .output() + .expect("Failed to run ado-aw --help"); + assert!(output.status.success(), "--help should exit 0"); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout.contains("remove"), + "Top-level --help should mention the remove subcommand, got:\n{stdout}" + ); +}