diff --git a/AGENTS.md b/AGENTS.md index edbc798a..49561a50 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -76,6 +76,7 @@ Every compiled pipeline runs as three sequential jobs: │ ├── logging.rs # File-based logging infrastructure │ ├── 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 │ ├── 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 f0ddee39..e4144da3 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -43,3 +43,15 @@ Global flags (apply to all subcommands): `--verbose, -v` (enable info-level logg - `--path ` - Path to the repository root (defaults to current directory) - `--dry-run` - Preview changes without applying them - `--definition-ids ` - Explicit pipeline definition IDs to update (comma-separated, skips auto-detection) + +- `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 ` - 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). + - `--folder ` - ADO folder for newly-created definitions. Defaults to `\` (root). Only applied on create — existing definitions stay where they are. + - `--default-branch ` - Default branch for newly-created definitions. Defaults to `refs/heads/main`. + - `--dry-run` - Print the planned actions (and the full POST body for creates) without calling the ADO API. + - `--also-set-token` - After creating a new definition, set its `GITHUB_TOKEN` variable (as an ADO secret). + - `--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. diff --git a/src/ado/mod.rs b/src/ado/mod.rs index 138d781d..d7793ddb 100644 --- a/src/ado/mod.rs +++ b/src/ado/mod.rs @@ -200,6 +200,11 @@ pub struct DefinitionSummary { pub id: u64, pub name: String, pub process: Option, + /// `enabled`, `disabled`, or `paused`. Populated when `list_definitions` + /// is called with `includeAllProperties=true` (the default in + /// [`list_definitions`]). Older/cached responses may omit it. + #[serde(rename = "queueStatus")] + pub queue_status: Option, } #[derive(Debug, Deserialize)] @@ -786,18 +791,77 @@ pub async fn resolve_definitions( // overhaul. Locking the surface here lets the parallel command PRs depend on // stable function signatures from day one. +/// Characters that must be percent-encoded when used in a URL path +/// segment. Built from RFC 3986 §3.3: `pchar` allows unreserved +/// characters (`A-Z`, `a-z`, `0-9`, `-`, `_`, `.`, `~`), +/// percent-encoded triplets, sub-delims, and `:` / `@`. We additionally +/// encode `:`, `@`, `%`, and `/` so a repository name containing any +/// of those does not break out of the segment, and the U+0021 (`!`) +/// just for symmetry with common path-encoding tables. Notably this +/// preserves `-`, `_`, `.`, `~` which `NON_ALPHANUMERIC` would over- +/// encode (e.g. `my-repo` → `my%2Drepo`). +const PATH_SEGMENT: &percent_encoding::AsciiSet = &percent_encoding::CONTROLS + .add(b' ') + .add(b'"') + .add(b'#') + .add(b'<') + .add(b'>') + .add(b'?') + .add(b'`') + .add(b'{') + .add(b'}') + .add(b'/') + .add(b'%') + .add(b'@') + .add(b':') + .add(b'!'); + /// Look up an ADO Git repository's GUID by name. /// /// Calls `GET /_apis/git/repositories/{repoName}?api-version=7.1` and reads /// the `id` field. Required for `create_definition`, which needs a /// `repository.id` (not just a name) on the POST body. pub async fn get_repository_id( - _client: &reqwest::Client, - _ctx: &AdoContext, - _auth: &AdoAuth, - _repo_name: &str, + client: &reqwest::Client, + ctx: &AdoContext, + auth: &AdoAuth, + repo_name: &str, ) -> Result { - anyhow::bail!("not yet implemented: filled in by PR 2 (ado-aw enable)") + let url = format!( + "{}/{}/_apis/git/repositories/{}?api-version=7.1", + ctx.org_url.trim_end_matches('/'), + percent_encoding::utf8_percent_encode(&ctx.project, PATH_SEGMENT), + percent_encoding::utf8_percent_encode(repo_name, PATH_SEGMENT), + ); + + debug!("Looking up repository '{}': {}", repo_name, url); + + let resp = auth + .apply(client.get(&url)) + .send() + .await + .with_context(|| format!("Failed to look up repository '{}'", repo_name))?; + + let status = resp.status(); + if !status.is_success() { + let body = resp.text().await.unwrap_or_default(); + anyhow::bail!( + "ADO API returned {} when looking up repository '{}': {}", + status, + repo_name, + body + ); + } + + let body: serde_json::Value = resp + .json() + .await + .with_context(|| format!("Failed to parse repository response for '{}'", repo_name))?; + + body.get("id") + .and_then(|v| v.as_str()) + .map(str::to_string) + .with_context(|| format!("Repository '{}' response has no 'id' field", repo_name)) } /// Fetch the full JSON body of a build definition. @@ -806,26 +870,107 @@ pub async fn get_repository_id( /// the raw `serde_json::Value` so callers can mutate specific fields and /// PUT the result back (the standard GET → mutate → PUT cycle). pub async fn get_definition_full( - _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 2 (ado-aw enable) or PR 3 (ado-aw disable)") + 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 + ); + + let resp = auth + .apply(client.get(&url)) + .send() + .await + .with_context(|| format!("Failed to fetch definition {}", id))?; + + let status = resp.status(); + if !status.is_success() { + let body = resp.text().await.unwrap_or_default(); + anyhow::bail!( + "ADO API returned {} when fetching definition {}: {}", + status, + id, + body + ); + } + + let body = resp + .text() + .await + .with_context(|| format!("Failed to read definition {} response body", id))?; + + serde_json::from_str(&body).with_context(|| { + let snippet: String = body.chars().take(500).collect(); + format!( + "Failed to parse definition {} as JSON. \ + This usually means the PAT is invalid or expired. \ + Response body (first 500 chars):\n{snippet}", + id + ) + }) } /// PATCH the `queueStatus` field on a build definition. /// /// `status` must be one of `"enabled"`, `"disabled"`, or `"paused"`. -/// Implements the GET → mutate → PUT cycle internally. +/// Implements the GET → mutate → PUT cycle internally; the full definition +/// is round-tripped to satisfy the PUT API's "you must send the whole +/// document" requirement. pub async fn patch_queue_status( - _client: &reqwest::Client, - _ctx: &AdoContext, - _auth: &AdoAuth, - _id: u64, - _status: &str, + client: &reqwest::Client, + ctx: &AdoContext, + auth: &AdoAuth, + id: u64, + status: &str, ) -> Result<()> { - anyhow::bail!("not yet implemented: filled in by PR 2 (ado-aw enable) or PR 3 (ado-aw disable)") + match status { + "enabled" | "disabled" | "paused" => {} + other => anyhow::bail!( + "patch_queue_status: invalid status '{}', expected one of enabled/disabled/paused", + other + ), + } + + let mut definition = get_definition_full(client, ctx, auth, id) + .await + .with_context(|| format!("Failed to fetch definition {} before patching", id))?; + + definition["queueStatus"] = serde_json::Value::String(status.to_string()); + + let put_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!("PUT definition {} with queueStatus={}: {}", id, status, put_url); + + let resp = auth + .apply(client.put(&put_url)) + .header("Content-Type", "application/json") + .json(&definition) + .send() + .await + .with_context(|| format!("Failed to update queueStatus on definition {}", id))?; + + let resp_status = resp.status(); + if !resp_status.is_success() { + let body = resp.text().await.unwrap_or_default(); + anyhow::bail!( + "ADO API returned {} when updating queueStatus on definition {}: {}", + resp_status, + id, + body + ); + } + + Ok(()) } /// Delete a build definition. @@ -845,12 +990,46 @@ pub async fn delete_definition( /// Calls `POST /_apis/build/definitions?api-version=7.1` with the supplied /// JSON body and returns the new definition's `id`. pub async fn create_definition( - _client: &reqwest::Client, - _ctx: &AdoContext, - _auth: &AdoAuth, - _body: &serde_json::Value, + client: &reqwest::Client, + ctx: &AdoContext, + auth: &AdoAuth, + body: &serde_json::Value, ) -> Result { - anyhow::bail!("not yet implemented: filled in by PR 2 (ado-aw enable)") + 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), + ); + + debug!("POST new definition: {}", url); + + let resp = auth + .apply(client.post(&url)) + .header("Content-Type", "application/json") + .json(body) + .send() + .await + .context("Failed to create build definition")?; + + let status = resp.status(); + if !status.is_success() { + let resp_body = resp.text().await.unwrap_or_default(); + anyhow::bail!( + "ADO API returned {} when creating definition: {}", + status, + resp_body + ); + } + + let resp_body: serde_json::Value = resp + .json() + .await + .context("Failed to parse create-definition response")?; + + resp_body + .get("id") + .and_then(|v| v.as_u64()) + .context("create_definition response has no numeric 'id' field") } /// Queue a build for a definition. @@ -1005,6 +1184,7 @@ mod tests { id, name: name.to_string(), process: None, + queue_status: None, } } @@ -1015,6 +1195,7 @@ mod tests { process: Some(ProcessInfo { yaml_filename: Some(yaml_filename.to_string()), }), + queue_status: None, } } @@ -1148,4 +1329,61 @@ mod tests { assert_eq!(format!("{}", MatchMethod::PipelineName), "pipeline-name"); assert_eq!(format!("{}", MatchMethod::Explicit), "explicit"); } + + // ==================== DefinitionSummary deserialization ==================== + + #[test] + fn definition_summary_deserializes_queue_status() { + let raw = serde_json::json!({ + "id": 42, + "name": "Daily noop", + "queueStatus": "disabled", + "process": { "yamlFilename": "/tests/noop.lock.yml" } + }); + let def: DefinitionSummary = serde_json::from_value(raw).unwrap(); + assert_eq!(def.id, 42); + assert_eq!(def.queue_status.as_deref(), Some("disabled")); + assert_eq!( + def.process + .as_ref() + .and_then(|p| p.yaml_filename.as_deref()), + Some("/tests/noop.lock.yml") + ); + } + + #[test] + fn definition_summary_queue_status_missing_is_none() { + let raw = serde_json::json!({ "id": 1, "name": "x" }); + let def: DefinitionSummary = serde_json::from_value(raw).unwrap(); + assert!(def.queue_status.is_none()); + } + + // ==================== PATH_SEGMENT percent-encoding ==================== + + #[test] + fn path_segment_preserves_rfc3986_unreserved_chars() { + // RFC 3986 unreserved set: A-Z / a-z / 0-9 / - / _ / . / ~ + // These MUST NOT be percent-encoded in a URL path segment. + let encoded = + percent_encoding::utf8_percent_encode("my-repo_name.with~tilde", PATH_SEGMENT) + .to_string(); + assert_eq!(encoded, "my-repo_name.with~tilde"); + } + + #[test] + fn path_segment_encodes_space_and_reserved_punctuation() { + let encoded = + percent_encoding::utf8_percent_encode("my repo/with?special#chars", PATH_SEGMENT) + .to_string(); + // Spaces become %20, slashes %2F, ? becomes %3F, # becomes %23. + assert_eq!(encoded, "my%20repo%2Fwith%3Fspecial%23chars"); + } + + #[test] + fn path_segment_handles_non_ascii() { + let encoded = + percent_encoding::utf8_percent_encode("café-π", PATH_SEGMENT).to_string(); + // Non-ASCII bytes get encoded per UTF-8. + assert_eq!(encoded, "caf%C3%A9-%CF%80"); + } } diff --git a/src/enable.rs b/src/enable.rs new file mode 100644 index 00000000..b4e27cf1 --- /dev/null +++ b/src/enable.rs @@ -0,0 +1,730 @@ +//! The `enable` CLI command. +//! +//! Registers ADO build definitions for each compiled pipeline in the +//! current repository and ensures they are `enabled`. Phase 1 of the +//! pipeline-lifecycle CLI family — see `docs/cli.md`. +//! +//! Scope (Phase 1): +//! +//! - Only operates on local fixtures discovered from `PATH` (same +//! auto-discovery as `compile`). It does *not* enumerate ADO +//! definitions beyond a single `list_definitions` call used for the +//! already-exists check. +//! - Source repository must be Azure DevOps Git. GitHub-hosted source +//! repositories are gated on a follow-up. + +use anyhow::{Context, Result}; +use log::debug; +use std::path::{Path, PathBuf}; + +use crate::ado::{ + AdoAuth, AdoContext, DefinitionSummary, create_definition, get_git_remote_url, + get_repository_id, list_definitions, normalize_ado_yaml_path, parse_ado_remote, + patch_queue_status, resolve_ado_context, resolve_auth, update_pipeline_variable, +}; +use crate::compile; +use crate::detect; + +/// Outcome of inspecting a local fixture against the ADO listing. +/// +/// Pure data — no HTTP, no auth, no IO. Built by [`decide_action`] so +/// the decision logic can be exercised without touching the network. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Action { + /// No existing definition matches; create a fresh one. + Create, + /// Definition exists but is not enabled; flip queueStatus to enabled. + EnableExisting { + id: u64, + name: String, + current_status: String, + }, + /// Definition exists and is already enabled; skip. + AlreadyEnabled { id: u64, name: String }, +} + +/// Sanitize a raw front-matter `name:` for use as an ADO build +/// definition display name. +/// +/// ADO forbids `< > | : " * ? \` in definition names and rejects names +/// ending in `.`. This sanitizer also collapses internal whitespace +/// runs and caps the result at 255 characters (matching ADO's limit). +/// An empty result falls back to `"pipeline"` so callers always get a +/// non-empty string to POST. +/// +/// Distinct from the compiler's `sanitize_pipeline_agent_name`, which +/// applies the stricter `Build.BuildNumber` rules. +pub fn sanitize_ado_display_name(raw: &str) -> String { + const ADO_FORBIDDEN: &[char] = &['<', '>', '|', ':', '"', '*', '?', '\\']; + + let stripped: String = raw.chars().filter(|c| !ADO_FORBIDDEN.contains(c)).collect(); + let collapsed = stripped.split_whitespace().collect::>().join(" "); + // Trim trailing dots before *and* after capping: capping a string like + // "aaa....(254 a's)...suffix" at 255 chars can re-introduce a trailing dot + // if the 255th character is '.'. + let trimmed = collapsed.trim_end_matches('.'); + let bounded: String = trimmed.chars().take(255).collect(); + let bounded = bounded.trim_end_matches('.'); + if bounded.is_empty() { + "pipeline".to_string() + } else { + bounded.to_string() + } +} + +/// Compute the ADO `yamlFilename` value for a compiled pipeline. +/// +/// ADO stores `yamlFilename` with a leading `/` and forward slashes; +/// we normalize both here so the value we POST round-trips exactly +/// against `normalize_ado_yaml_path` on the read side. +pub fn compute_yaml_filename(repo_relative: &Path) -> String { + let s = repo_relative.to_string_lossy().replace('\\', "/"); + if s.starts_with('/') { s } else { format!("/{}", s) } +} + +/// Pure function: decide what to do for one local fixture against a +/// snapshot of the project's ADO definitions. +/// +/// Match order: yaml-path first, then exact name match. Two separate +/// passes are made so that a yaml-path match on definition B always +/// wins over a name match on definition A that appears earlier in the +/// ADO listing. Returns [`Action::Create`] when nothing matches. +pub fn decide_action( + sanitized_name: &str, + yaml_filename: &str, + definitions: &[DefinitionSummary], +) -> Action { + let target_path = normalize_ado_yaml_path(yaml_filename); + + // Pass 1: yaml-path takes precedence. + let matched = definitions.iter().find(|def| { + def.process + .as_ref() + .and_then(|p| p.yaml_filename.as_ref()) + .map(|f| normalize_ado_yaml_path(f) == target_path) + .unwrap_or(false) + }); + + // Pass 2: fall back to exact name match. + let matched = matched.or_else(|| definitions.iter().find(|def| def.name == sanitized_name)); + + match matched { + None => Action::Create, + Some(def) => { + let status = def.queue_status.clone().unwrap_or_default(); + if status == "enabled" { + Action::AlreadyEnabled { + id: def.id, + name: def.name.clone(), + } + } else { + Action::EnableExisting { + id: def.id, + name: def.name.clone(), + current_status: if status.is_empty() { + "unknown".to_string() + } else { + status + }, + } + } + } + } +} + +/// Build the JSON body for `POST /_apis/build/definitions`. Pure +/// function so we can snapshot-test the wire shape. +pub fn build_create_body( + name: &str, + folder: &str, + repo_id: &str, + repo_name: &str, + default_branch: &str, + yaml_filename: &str, +) -> serde_json::Value { + serde_json::json!({ + "name": name, + "path": folder, + "type": "build", + "queueStatus": "enabled", + "repository": { + "id": repo_id, + "type": "TfsGit", + "name": repo_name, + "defaultBranch": default_branch, + "properties": { "reportBuildStatus": "true" } + }, + "process": { + "type": 2, + "yamlFilename": yaml_filename + }, + "triggers": [] + }) +} + +/// CLI options for [`run`]. Bundled into a struct so we don't grow a +/// 10-positional-argument signature as Phase 1 evolves. +pub struct EnableOptions<'a> { + pub org: Option<&'a str>, + pub project: Option<&'a str>, + pub pat: Option<&'a str>, + pub path: Option<&'a Path>, + pub folder: &'a str, + pub default_branch: &'a str, + pub dry_run: bool, + pub also_set_token: bool, + pub token: Option<&'a str>, +} + +/// Run the `enable` command. +pub async fn run(opts: EnableOptions<'_>) -> 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")?, + }; + + // GitHub-source guard: Phase 1 only supports ADO Git source + // repositories. We use parse_ado_remote success on the raw git + // remote URL as the gate — if we can't parse it as ADO we don't + // have a `repository.name` to put in the POST body. + let remote_url = get_git_remote_url(&repo_path).await.with_context(|| { + format!( + "Could not read the git remote 'origin' from {}. \ + `ado-aw enable` requires an Azure DevOps Git remote.", + repo_path.display() + ) + })?; + let remote_ctx = match parse_ado_remote(&remote_url) { + Ok(ctx) => ctx, + Err(_) => anyhow::bail!( + "This command requires an Azure DevOps Git remote.\n\ + The current remote is {}.\n\ + Phase 1 of `ado-aw enable` does not yet support GitHub-hosted source \ + repos. Follow https://github.com/githubnext/ado-aw/issues for the \ + GitHub-source follow-up.", + remote_url + ), + }; + + // Skip interactive token resolution on dry-run: --also-set-token is + // silently suppressed in that path anyway, so prompting the operator + // for a credential they will never use is wrong UX. + let github_token = if opts.dry_run { + None + } else { + resolve_token_arg(opts.also_set_token, opts.token)? + }; + 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={}, repo={}", + ado_ctx.org_url, ado_ctx.project, remote_ctx.repo_name + ); + 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")?; + + let definitions = list_definitions(&client, &ado_ctx, &auth).await?; + + // Resolve the repository GUID once. Dry-run skips this — the + // POST body printout uses a placeholder GUID so operators still + // see the full body shape. + let repo_id = if opts.dry_run { + String::from("") + } else { + get_repository_id(&client, &ado_ctx, &auth, &remote_ctx.repo_name) + .await + .with_context(|| { + format!( + "Failed to look up ADO repository '{}'", + remote_ctx.repo_name + ) + })? + }; + + let mut success = 0usize; + let mut failure = 0usize; + let mut newly_created_ids: Vec = Vec::new(); + + for pipeline in &detected { + let source_path = repo_path.join(&pipeline.source); + let result = process_one( + &client, + &ado_ctx, + &auth, + &definitions, + &pipeline.yaml_path, + &source_path, + &repo_id, + &remote_ctx.repo_name, + opts.folder, + opts.default_branch, + opts.dry_run, + ) + .await; + match result { + Ok(Some(new_id)) => { + newly_created_ids.push(new_id); + success += 1; + } + Ok(None) => success += 1, + Err(e) => { + eprintln!("✗ failed: {}: {:#}", pipeline.source, e); + failure += 1; + } + } + } + + if opts.also_set_token && !opts.dry_run && !newly_created_ids.is_empty() { + let Some(token) = github_token.as_deref() else { + unreachable!("resolve_token_arg guarantees Some when also_set_token is true"); + }; + println!(); + println!( + "Setting GITHUB_TOKEN on {} newly-created definition(s)...", + newly_created_ids.len() + ); + for id in &newly_created_ids { + match update_pipeline_variable(&client, &ado_ctx, &auth, *id, "GITHUB_TOKEN", token) + .await + { + Ok(()) => println!(" ✓ set GITHUB_TOKEN on definition {}", id), + Err(e) => { + eprintln!(" ✗ failed to set GITHUB_TOKEN on {}: {:#}", id, e); + failure += 1; + } + } + } + } else if opts.also_set_token && opts.dry_run && !newly_created_ids.is_empty() { + println!(); + println!( + "[dry-run] would set GITHUB_TOKEN on {} newly-created definition(s)", + newly_created_ids.len() + ); + } + + println!(); + println!("Done: {} succeeded, {} failed.", success, failure); + if failure > 0 { + anyhow::bail!("{} fixture(s) failed", failure); + } + Ok(()) +} + +/// Validates the `--token` / `--also-set-token` pair and resolves the +/// effective token value when `--also-set-token` is set. +fn resolve_token_arg(also_set_token: bool, token: Option<&str>) -> Result> { + if !also_set_token { + if token.is_some() { + anyhow::bail!("--token requires --also-set-token"); + } + return Ok(None); + } + if let Some(t) = token { + return Ok(Some(t.to_string())); + } + if let Ok(env) = std::env::var("GITHUB_TOKEN") + && !env.is_empty() + { + return Ok(Some(env)); + } + let prompted = inquire::Password::new("Enter the GITHUB_TOKEN to set on newly-created definitions:") + .without_confirmation() + .prompt() + .context("Failed to read token from interactive prompt")?; + Ok(Some(prompted)) +} + +#[allow(clippy::too_many_arguments)] +async fn process_one( + client: &reqwest::Client, + ado_ctx: &AdoContext, + auth: &AdoAuth, + definitions: &[DefinitionSummary], + yaml_path: &Path, + source_path: &Path, + repo_id: &str, + repo_name: &str, + folder: &str, + default_branch: &str, + dry_run: bool, +) -> Result> { + let content = tokio::fs::read_to_string(source_path) + .await + .with_context(|| format!("Failed to read source: {}", source_path.display()))?; + let parsed = compile::parse_markdown_detailed(&content) + .with_context(|| format!("Failed to parse front matter: {}", source_path.display()))?; + let sanitized = sanitize_ado_display_name(&parsed.front_matter.name); + + let yaml_filename = compute_yaml_filename(yaml_path); + + let action = decide_action(&sanitized, &yaml_filename, definitions); + debug!( + "fixture {}: sanitized_name='{}' yaml='{}' action={:?}", + source_path.display(), + sanitized, + yaml_filename, + action + ); + + match action { + Action::AlreadyEnabled { id, name } => { + println!("↻ already enabled: {} (id={})", name, id); + Ok(None) + } + Action::EnableExisting { + id, + name, + current_status, + } => { + if dry_run { + println!( + "[dry-run] ▶ would enable: {} (id={}, current={})", + name, id, current_status + ); + return Ok(None); + } + patch_queue_status(client, ado_ctx, auth, id, "enabled") + .await + .with_context(|| format!("Failed to enable definition {}", id))?; + println!("▶ enabled: {} (id={}, was {})", name, id, current_status); + Ok(None) + } + Action::Create => { + let body = build_create_body( + &sanitized, + folder, + repo_id, + repo_name, + default_branch, + &yaml_filename, + ); + if dry_run { + let pretty = serde_json::to_string_pretty(&body).unwrap_or_default(); + println!( + "[dry-run] ✓ would create: {} ← {}", + sanitized, yaml_filename + ); + println!("{}", pretty); + return Ok(None); + } + let new_id = create_definition(client, ado_ctx, auth, &body) + .await + .with_context(|| format!("Failed to create definition for {}", sanitized))?; + println!( + "✓ registered + enabled: {} (id={}) ← {}", + sanitized, new_id, yaml_filename + ); + Ok(Some(new_id)) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::ado::ProcessInfo; + + fn def(id: u64, name: &str, yaml: Option<&str>, status: Option<&str>) -> DefinitionSummary { + DefinitionSummary { + id, + name: name.to_string(), + process: yaml.map(|y| ProcessInfo { + yaml_filename: Some(y.to_string()), + }), + queue_status: status.map(str::to_string), + } + } + + // ============ sanitize_ado_display_name ============ + + #[test] + fn sanitize_strips_forbidden_chars() { + assert_eq!( + sanitize_ado_display_name("pipe|name:foo*?\\bar\""), + "pipelinenamefoobar" + ); + } + + #[test] + fn sanitize_collapses_internal_whitespace() { + assert_eq!( + sanitize_ado_display_name("daily smoke noop"), + "daily smoke noop" + ); + } + + #[test] + fn sanitize_trims_trailing_dot() { + assert_eq!(sanitize_ado_display_name("name..."), "name"); + assert_eq!(sanitize_ado_display_name("name."), "name"); + assert_eq!(sanitize_ado_display_name("name.with.dots"), "name.with.dots"); + } + + #[test] + fn sanitize_caps_length_at_255() { + let raw = "a".repeat(300); + let out = sanitize_ado_display_name(&raw); + assert_eq!(out.chars().count(), 255); + } + + #[test] + fn sanitize_truncation_does_not_leave_trailing_dot() { + // 254 'a' chars followed by ".extra" — the first 255 chars are + // "a".repeat(254) + "." which must be trimmed back to 254 chars. + let raw = "a".repeat(254) + ".extra"; + let out = sanitize_ado_display_name(&raw); + assert!(!out.ends_with('.'), "result must not end with '.'"); + assert_eq!(out, "a".repeat(254)); + } + + #[test] + fn sanitize_falls_back_to_pipeline_when_empty() { + assert_eq!(sanitize_ado_display_name("<<<>>>"), "pipeline"); + assert_eq!(sanitize_ado_display_name(""), "pipeline"); + assert_eq!(sanitize_ado_display_name(" "), "pipeline"); + // All-dot after stripping should also fall back. + assert_eq!(sanitize_ado_display_name("..."), "pipeline"); + } + + #[test] + fn sanitize_preserves_safe_content() { + assert_eq!( + sanitize_ado_display_name("Daily safe-output smoke: noop"), + "Daily safe-output smoke noop" + ); + } + + // ============ compute_yaml_filename ============ + + #[test] + fn compute_yaml_filename_adds_leading_slash() { + assert_eq!( + compute_yaml_filename(Path::new("tests/safe-outputs/noop.lock.yml")), + "/tests/safe-outputs/noop.lock.yml" + ); + } + + #[test] + fn compute_yaml_filename_preserves_leading_slash() { + assert_eq!( + compute_yaml_filename(Path::new("/already/rooted.yml")), + "/already/rooted.yml" + ); + } + + #[test] + fn compute_yaml_filename_normalizes_backslashes() { + assert_eq!( + compute_yaml_filename(Path::new(r"tests\safe-outputs\noop.lock.yml")), + "/tests/safe-outputs/noop.lock.yml" + ); + } + + // ============ decide_action ============ + + #[test] + fn decide_action_create_when_no_match() { + let defs = vec![def(1, "other", Some("/other.yml"), Some("enabled"))]; + let action = decide_action("noop", "/tests/noop.yml", &defs); + assert_eq!(action, Action::Create); + } + + #[test] + fn decide_action_matches_by_yaml_path_even_when_name_differs() { + let defs = vec![def( + 7, + "Some old name", + Some("/tests/noop.lock.yml"), + Some("enabled"), + )]; + let action = decide_action("My noop", "/tests/noop.lock.yml", &defs); + assert_eq!( + action, + Action::AlreadyEnabled { + id: 7, + name: "Some old name".to_string() + } + ); + } + + #[test] + fn decide_action_matches_by_name_when_yaml_missing() { + let defs = vec![def(42, "Daily noop", None, Some("enabled"))]; + let action = decide_action("Daily noop", "/whatever.yml", &defs); + assert_eq!( + action, + Action::AlreadyEnabled { + id: 42, + name: "Daily noop".to_string() + } + ); + } + + #[test] + fn decide_action_enable_existing_when_disabled() { + let defs = vec![def( + 10, + "Daily noop", + Some("/tests/noop.lock.yml"), + Some("disabled"), + )]; + let action = decide_action("Daily noop", "/tests/noop.lock.yml", &defs); + assert_eq!( + action, + Action::EnableExisting { + id: 10, + name: "Daily noop".to_string(), + current_status: "disabled".to_string() + } + ); + } + + #[test] + fn decide_action_enable_existing_when_paused() { + let defs = vec![def( + 11, + "Daily noop", + Some("/tests/noop.lock.yml"), + Some("paused"), + )]; + let action = decide_action("Daily noop", "/tests/noop.lock.yml", &defs); + assert_eq!( + action, + Action::EnableExisting { + id: 11, + name: "Daily noop".to_string(), + current_status: "paused".to_string() + } + ); + } + + #[test] + fn decide_action_enable_existing_when_status_missing() { + // Old/cached responses may omit queueStatus — treat as not-enabled. + let defs = vec![def(12, "Daily noop", Some("/tests/noop.lock.yml"), None)]; + let action = decide_action("Daily noop", "/tests/noop.lock.yml", &defs); + assert_eq!( + action, + Action::EnableExisting { + id: 12, + name: "Daily noop".to_string(), + current_status: "unknown".to_string() + } + ); + } + + #[test] + fn decide_action_yaml_match_handles_backslashes_and_leading_slash() { + // ADO sometimes returns yamlFilename with backslashes or without + // the leading slash; normalize_ado_yaml_path handles both, and + // decide_action must pick this up. + let defs = vec![def( + 5, + "n", + Some(r"\tests\safe-outputs\noop.lock.yml"), + Some("enabled"), + )]; + let action = decide_action("n", "/tests/safe-outputs/noop.lock.yml", &defs); + assert_eq!( + action, + Action::AlreadyEnabled { + id: 5, + name: "n".to_string() + } + ); + } + + #[test] + fn decide_action_yaml_path_wins_over_earlier_name_match() { + // Definition A appears first and matches by name. + // Definition B appears second and matches by yaml path. + // yaml-path must win (definition B), even though A is earlier in the slice. + let defs = vec![ + def(1, "My Pipeline", None, Some("enabled")), // name match, no path + def(2, "Old Name", Some("/pipelines/agent.yml"), Some("disabled")), // path match, different name + ]; + let action = decide_action("My Pipeline", "/pipelines/agent.yml", &defs); + // Should pick definition 2 (path match) not 1 (name match). + assert_eq!( + action, + Action::EnableExisting { + id: 2, + name: "Old Name".to_string(), + current_status: "disabled".to_string() + } + ); + } + + // ============ build_create_body ============ + + #[test] + fn build_create_body_matches_expected_shape() { + let body = build_create_body( + "Daily smoke", + "\\smoke", + "abc-123", + "myrepo", + "refs/heads/main", + "/tests/noop.lock.yml", + ); + let expected = serde_json::json!({ + "name": "Daily smoke", + "path": "\\smoke", + "type": "build", + "queueStatus": "enabled", + "repository": { + "id": "abc-123", + "type": "TfsGit", + "name": "myrepo", + "defaultBranch": "refs/heads/main", + "properties": { "reportBuildStatus": "true" } + }, + "process": { + "type": 2, + "yamlFilename": "/tests/noop.lock.yml" + }, + "triggers": [] + }); + assert_eq!(body, expected); + } + + // ============ resolve_token_arg ============ + + #[test] + fn resolve_token_arg_rejects_token_without_also_set() { + let err = resolve_token_arg(false, Some("x")).unwrap_err(); + assert!(err.to_string().contains("--token requires --also-set-token")); + } + + #[test] + fn resolve_token_arg_returns_none_when_disabled() { + let v = resolve_token_arg(false, None).unwrap(); + assert!(v.is_none()); + } + + #[test] + fn resolve_token_arg_takes_explicit_token() { + let v = resolve_token_arg(true, Some("explicit")).unwrap(); + assert_eq!(v.as_deref(), Some("explicit")); + } +} diff --git a/src/main.rs b/src/main.rs index 3972263c..9d19f316 100644 --- a/src/main.rs +++ b/src/main.rs @@ -5,6 +5,7 @@ mod compile; mod configure; mod detect; mod ecosystem_domains; +mod enable; mod engine; mod execute; mod fuzzy_schedule; @@ -139,6 +140,40 @@ enum Commands { #[arg(long, value_delimiter = ',')] definition_ids: Option>, }, + /// Register an ADO build definition for each compiled pipeline and ensure it's enabled. + Enable { + /// 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; + /// prompted if omitted). + #[arg(long, env = "AZURE_DEVOPS_EXT_PAT")] + pat: Option, + /// ADO folder for newly-created definitions. Only applied on create; + /// existing definitions are left in their current folder. + #[arg(long, default_value = "\\")] + folder: String, + /// Default branch for newly-created definitions. + #[arg(long, default_value = "refs/heads/main")] + default_branch: String, + /// Preview the planned actions without calling the ADO API. + #[arg(long)] + dry_run: bool, + /// After creating new definitions, set their GITHUB_TOKEN variable. + #[arg(long)] + also_set_token: bool, + /// The GITHUB_TOKEN value to set when `--also-set-token` is used. + /// Falls back to the GITHUB_TOKEN env var, then to an interactive prompt. + #[arg(long, requires = "also_set_token")] + token: Option, + }, } #[derive(Parser, Debug)] @@ -488,6 +523,7 @@ async fn main() -> Result<()> { Some(Commands::McpHttp { .. }) => "mcp-http", Some(Commands::Init { .. }) => "init", Some(Commands::Configure { .. }) => "configure", + Some(Commands::Enable { .. }) => "enable", None => "ado-aw", }; @@ -596,6 +632,30 @@ async fn main() -> Result<()> { ) .await?; } + Commands::Enable { + path, + org, + project, + pat, + folder, + default_branch, + dry_run, + also_set_token, + token, + } => { + enable::run(enable::EnableOptions { + org: org.as_deref(), + project: project.as_deref(), + pat: pat.as_deref(), + path: path.as_deref(), + folder: &folder, + default_branch: &default_branch, + dry_run, + also_set_token, + token: token.as_deref(), + }) + .await?; + } } Ok(()) } diff --git a/tests/enable_integration.rs b/tests/enable_integration.rs new file mode 100644 index 00000000..a9ff4436 --- /dev/null +++ b/tests/enable_integration.rs @@ -0,0 +1,67 @@ +//! Integration tests for `ado-aw enable`. +//! +//! These tests run the compiled binary in `--dry-run` mode against a +//! fake org/project so no real HTTP traffic is generated. We assert +//! that: +//! +//! - The help text advertises the documented surface. +//! - `--token` without `--also-set-token` is a clap-level error. +//! +//! The decision logic (`decide_action`, `sanitize_ado_display_name`, +//! `build_create_body`) is covered by `#[cfg(test)] mod tests` inside +//! `src/enable.rs`, since wire-stubbing the full ADO REST surface from +//! an integration test would add more infrastructure than it pays off +//! for in Phase 1. + +use std::path::PathBuf; + +fn binary() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_ado-aw")) +} + +#[test] +fn enable_help_describes_command() { + let output = std::process::Command::new(binary()) + .args(["enable", "--help"]) + .output() + .expect("Failed to run ado-aw enable --help"); + assert!(output.status.success(), "--help should exit 0"); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout.contains("Register an ADO build definition"), + "Help text should describe the enable command, got:\n{stdout}" + ); + for flag in [ + "--org", + "--project", + "--pat", + "--folder", + "--default-branch", + "--dry-run", + "--also-set-token", + "--token", + ] { + assert!( + stdout.contains(flag), + "Expected --help to advertise {flag}, got:\n{stdout}" + ); + } +} + +#[test] +fn enable_rejects_token_without_also_set_token() { + // clap should reject this at parse time via `requires = "also_set_token"`. + let output = std::process::Command::new(binary()) + .args(["enable", "--token", "secret", "--dry-run"]) + .output() + .expect("Failed to run ado-aw enable"); + assert!( + !output.status.success(), + "Expected non-zero exit when --token used without --also-set-token" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("--also-set-token") || stderr.contains("also_set_token"), + "stderr should reference the requires-constraint, got:\n{stderr}" + ); +}