diff --git a/docs/targets.md b/docs/targets.md index b7d0c9f7..ea3ea5a5 100644 --- a/docs/targets.md +++ b/docs/targets.md @@ -56,6 +56,9 @@ jobs: - job: Build steps: ... - template: agents/review.lock.yml + parameters: + dependsOn: [Build] # list of upstream job names; omit for implicit dep on previous job + condition: succeeded('Build') # optional; ANDed into the agent job's internal condition ``` #### Usage inside a user-defined stage @@ -72,9 +75,22 @@ stages: #### Notes -- Triggers (`on:`) are ignored with a warning (the parent pipeline controls triggers) +- ADO's [`jobs.template`](https://learn.microsoft.com/azure/devops/pipelines/yaml-schema/jobs-template) + schema only allows `template:` and `parameters:` at the call site. `dependsOn:` + and `condition:` as bare keys on the `- template:` line are rejected; the + compiled template surfaces them as parameters and applies them to the + agent job internally. +- When the agent has a Setup job (e.g. PR or pipeline filters), the + `dependsOn` parameter MUST be a YAML list — the template uses + `${{ each }}` to merge `Setup` with the caller's deps, and `${{ each }}` + requires an iterable. For agents without a Setup job, either a string or a + list works. +- The `condition` parameter is ANDed into the agent job's existing internal + condition (PR gate, pipeline gate, etc.). Empty default preserves ADO's + native `succeeded()` behaviour. +- Triggers (`on:`) are ignored with a warning (the parent pipeline controls triggers). - If the agent declares additional repositories via `repos:`, add them to the - parent pipeline's `resources:` block (documented in the generated file header) + parent pipeline's `resources:` block (documented in the generated file header). ### `stage` @@ -94,16 +110,24 @@ stages: - stage: Build jobs: ... - template: agents/review.lock.yml - dependsOn: Build - condition: succeeded() + parameters: + dependsOn: Build # or [Build, Test]; omit for implicit dep on previous stage + condition: succeeded('Build') # optional; omit for ADO's default succeeded() ``` -ADO natively supports `dependsOn` and `condition` at the template call site — -no template parameters are needed for stage ordering. - #### Notes -- Same 3-job chain, job-name prefixing, and pool handling as `target: job` -- Triggers (`on:`) are ignored with a warning +- ADO's [`stages.template`](https://learn.microsoft.com/azure/devops/pipelines/yaml-schema/stages-template) + schema only allows `template:` and `parameters:` at the call site — + `dependsOn:` and `condition:` as bare keys on the `- template:` line are + rejected by the YAML parser. The compiled template surfaces them as the + `dependsOn` and `condition` parameters and applies them to the inner + stage block via ADO conditional template expressions, so empty defaults + preserve ADO's implicit "depends on previous stage" and `succeeded()` + behaviour. +- The `dependsOn` parameter is typed `object`, matching ADO's native + `dependsOn:` semantics (accepts a single string or a list). +- Same 3-job chain, job-name prefixing, and pool handling as `target: job`. +- Triggers (`on:`) are ignored with a warning. - If the agent declares additional repositories via `repos:`, add them to the - parent pipeline's `resources:` block + parent pipeline's `resources:` block. diff --git a/src/compile/common.rs b/src/compile/common.rs index d1d7573e..fcee1d71 100644 --- a/src/compile/common.rs +++ b/src/compile/common.rs @@ -445,11 +445,39 @@ pub fn validate_front_matter_identity(front_matter: &FrontMatter) -> Result<()> } /// Build the final parameters list by combining user-defined parameters -/// with auto-injected parameters (e.g., `clearMemory` when memory is enabled). +/// with auto-injected parameters. +/// +/// Auto-injected parameters: +/// - `clearMemory` (boolean) — when `has_memory` is true and the user has +/// not already defined a parameter with the same name. +/// - `dependsOn` (object, default `[]`) and `condition` (string, default +/// `''`) — when `is_template_target` is true. These are applied at the +/// template call site via `parameters:` and let the parent pipeline +/// inject external stage/job ordering that ADO's `template:` invocation +/// syntax does not otherwise support. See `docs/targets.md` and the ADO +/// `stages.template` / `jobs.template` schemas. +/// +/// Errors when `is_template_target` is true and the user has declared +/// front-matter parameters with the reserved names `dependsOn` or +/// `condition` — those names are reserved for template-invocation use. pub fn build_parameters( user_params: &[PipelineParameter], has_memory: bool, -) -> Vec { + is_template_target: bool, +) -> Result> { + if is_template_target { + for reserved in ["dependsOn", "condition"] { + if let Some(collide) = user_params.iter().find(|p| p.name == reserved) { + anyhow::bail!( + "Parameter name '{}' is reserved for template-invocation use (target: job/stage). \ + Rename the parameter '{}' to something else, or change the target.", + reserved, + collide.name + ); + } + } + } + let mut params = user_params.to_vec(); // Auto-inject clearMemory parameter when memory is configured, @@ -467,7 +495,36 @@ pub fn build_parameters( ); } - params + // Auto-inject dependsOn / condition template parameters for template + // targets so callers can specify external stage/job ordering at the + // template invocation site (ADO does not permit dependsOn:/condition: + // as bare keys on a template: call — only `parameters:`). + if is_template_target { + // Prepend so they appear first in the rendered parameters block, + // ahead of user-defined params and clearMemory. + params.insert( + 0, + PipelineParameter { + name: "condition".to_string(), + display_name: None, + param_type: Some("string".to_string()), + default: Some(serde_yaml::Value::String(String::new())), + values: None, + }, + ); + params.insert( + 0, + PipelineParameter { + name: "dependsOn".to_string(), + display_name: None, + param_type: Some("object".to_string()), + default: Some(serde_yaml::Value::Sequence(Vec::new())), + values: None, + }, + ); + } + + Ok(params) } /// Generate a schedule YAML block from a fuzzy schedule expression. @@ -1238,7 +1295,11 @@ pub fn generate_template_parameters(front_matter: &FrontMatter) -> Result String { /// non-matching trigger types to proceed unconditionally, while matching /// builds require the gate to pass. /// When `expression` is provided, it's ANDed into the condition as an escape hatch. +/// +/// When `is_jobs_template_target` is true (i.e. compiling for `target: job`), +/// the output is wrapped in mutually-exclusive `${{ if }}` ADO template +/// expressions so that external `dependsOn` and `condition` template +/// parameters supplied at the template call site merge with the internal +/// Setup/gate behaviour. ADO permits only one `dependsOn:` / `condition:` +/// key per job, so we emit each as a dual-branch block keyed on whether the +/// caller passed a non-default value. +/// +/// For `target: stage`, the external params apply to the inner stage block +/// (handled directly in `stage-base.yml`) rather than the Agent job, so this +/// flag is **false** for stage targets. +/// +/// Note: when `is_jobs_template_target` is true and `has_setup` is true, +/// callers MUST pass `parameters.dependsOn` as a YAML list (the default `[]` +/// works). A bare string is not supported in the merge branch because +/// `${{ each }}` iterates objects and arrays, not scalars. For `target: job` +/// agents without any Setup/gate, the simpler `dependsOn: ${{ parameters.dependsOn }}` +/// form is emitted, which does accept either a string or a list. pub fn generate_agentic_depends_on( setup_steps: &[serde_yaml::Value], has_pr_filters: bool, has_pipeline_filters: bool, expressions: &[&str], + is_jobs_template_target: bool, ) -> String { let has_gate = has_pr_filters || has_pipeline_filters; let has_setup = !setup_steps.is_empty() || has_gate; + let has_internal_condition = has_gate || !expressions.is_empty(); - if !has_setup && expressions.is_empty() { - return String::new(); - } - - let depends = if has_setup { "dependsOn: Setup\n" } else { "" }; - - if has_gate || !expressions.is_empty() { - let mut parts = Vec::new(); - parts.push("succeeded()".to_string()); - + // Build the shared condition body once. Reused across the internal-only + // (standalone/1es/stage) path and the dual-branch jobs-template path. + let condition_body: Option = if has_internal_condition { + let mut parts = vec!["succeeded()".to_string()]; if has_pr_filters { parts.push( r"or( @@ -2323,7 +2399,6 @@ pub fn generate_agentic_depends_on( .to_string(), ); } - if has_pipeline_filters { parts.push( r"or( @@ -2333,16 +2408,75 @@ pub fn generate_agentic_depends_on( .to_string(), ); } - for expr in expressions { parts.push(expr.to_string()); } + Some(parts.join(",\n ")) + } else { + None + }; - let condition_body = parts.join(",\n "); - format!("{depends}condition: |\n and(\n {condition_body}\n )") + if !is_jobs_template_target { + // Standalone / 1ES / stage path: inline single dependsOn:/condition: + // (preserved verbatim — no behavioural change for non-job-template + // targets). + if !has_setup && expressions.is_empty() { + return String::new(); + } + let depends = if has_setup { "dependsOn: Setup\n" } else { "" }; + return if let Some(body) = condition_body { + format!("{depends}condition: |\n and(\n {body}\n )") + } else { + "dependsOn: Setup".to_string() + }; + } + + // target: job: emit dual-branch ADO template expressions so external + // dependsOn/condition template parameters merge with the internal + // Setup/gate behaviour. + let mut out = String::new(); + + // ---------- dependsOn block ---------- + if has_setup { + // Internal Setup dependency exists. Branch on whether caller passed + // an external dependsOn. + out.push_str("${{ if eq(length(parameters.dependsOn), 0) }}:\n"); + out.push_str(" dependsOn: Setup\n"); + out.push_str("${{ if ne(length(parameters.dependsOn), 0) }}:\n"); + out.push_str(" dependsOn:\n"); + out.push_str(" - Setup\n"); + out.push_str(" - ${{ each d in parameters.dependsOn }}:\n"); + out.push_str(" - ${{ d }}\n"); } else { - "dependsOn: Setup".to_string() + // No internal Setup dependency. Emit dependsOn only when caller + // passes a non-empty external value; otherwise omit the key + // entirely (ADO will use implicit "depends on previous job" + // behaviour or none if this is the first job). + out.push_str("${{ if ne(length(parameters.dependsOn), 0) }}:\n"); + out.push_str(" dependsOn: ${{ parameters.dependsOn }}\n"); + } + + // ---------- condition block ---------- + if let Some(body) = &condition_body { + // Internal condition exists. Dual-branch: caller-omitted emits the + // internal expression verbatim (no behavioural change today); + // caller-provided ANDs the external clause into the body. + out.push_str("${{ if eq(parameters.condition, '') }}:\n"); + out.push_str(&format!(" condition: |\n and(\n {body}\n )\n")); + let body_with_external = format!("{body},\n ${{{{ parameters.condition }}}}"); + out.push_str("${{ if ne(parameters.condition, '') }}:\n"); + out.push_str(&format!( + " condition: |\n and(\n {body_with_external}\n )\n" + )); + } else { + // No internal condition; only emit when caller passes external. + out.push_str("${{ if ne(parameters.condition, '') }}:\n"); + out.push_str(" condition: ${{ parameters.condition }}\n"); } + + // Trim trailing newline — replace_with_indent's first-line handling + // expects content to not end with a blank line. + out.trim_end_matches('\n').to_string() } /// Returns `Some(v.to_vec())` when `v` is non-empty, otherwise `None`. @@ -3124,7 +3258,11 @@ pub async fn compile_shared( .as_ref() .and_then(|t| t.cache_memory.as_ref()) .is_some_and(|cm| cm.is_enabled()); - let parameters = build_parameters(&front_matter.parameters, has_memory); + let is_template_target = matches!( + front_matter.target, + crate::compile::types::CompileTarget::Job | crate::compile::types::CompileTarget::Stage + ); + let parameters = build_parameters(&front_matter.parameters, has_memory, is_template_target)?; let parameters_yaml = generate_parameters(¶meters)?; let prepare_steps = generate_prepare_steps(&front_matter.steps, extensions, ctx)?; let finalize_steps = generate_finalize_steps(&front_matter.post_steps); @@ -3172,6 +3310,7 @@ pub async fn compile_shared( has_pr_filters, has_pipeline_filters, &expressions, + matches!(front_matter.target, crate::compile::types::CompileTarget::Job), ); let job_timeout = generate_job_timeout(front_matter); @@ -5662,14 +5801,14 @@ safe-outputs: #[test] fn test_build_parameters_auto_injects_clear_memory() { - let params = build_parameters(&[], true); + let params = build_parameters(&[], true, false).unwrap(); assert_eq!(params.len(), 1); assert_eq!(params[0].name, "clearMemory"); } #[test] fn test_build_parameters_no_inject_without_memory() { - let params = build_parameters(&[], false); + let params = build_parameters(&[], false, false).unwrap(); assert!(params.is_empty()); } @@ -5682,7 +5821,7 @@ safe-outputs: default: Some(serde_yaml::Value::Bool(true)), values: None, }]; - let params = build_parameters(&user, true); + let params = build_parameters(&user, true, false).unwrap(); assert_eq!(params.len(), 1, "Should not duplicate clearMemory"); assert_eq!( params[0].display_name.as_deref(), @@ -5691,6 +5830,101 @@ safe-outputs: ); } + #[test] + fn test_build_parameters_template_target_injects_depends_on_and_condition() { + let params = build_parameters(&[], false, true).unwrap(); + assert_eq!(params.len(), 2); + assert_eq!(params[0].name, "dependsOn"); + assert_eq!(params[0].param_type.as_deref(), Some("object")); + assert!(matches!( + params[0].default, + Some(serde_yaml::Value::Sequence(ref s)) if s.is_empty() + )); + assert_eq!(params[1].name, "condition"); + assert_eq!(params[1].param_type.as_deref(), Some("string")); + assert!(matches!( + params[1].default, + Some(serde_yaml::Value::String(ref s)) if s.is_empty() + )); + } + + #[test] + fn test_build_parameters_template_target_ordering() { + let user = vec![PipelineParameter { + name: "myParam".to_string(), + display_name: None, + param_type: Some("string".to_string()), + default: Some(serde_yaml::Value::String("hi".to_string())), + values: None, + }]; + let params = build_parameters(&user, true, true).unwrap(); + // Order: dependsOn, condition, clearMemory, then user params + assert_eq!(params.len(), 4); + assert_eq!(params[0].name, "dependsOn"); + assert_eq!(params[1].name, "condition"); + assert_eq!(params[2].name, "clearMemory"); + assert_eq!(params[3].name, "myParam"); + } + + #[test] + fn test_build_parameters_template_target_rejects_reserved_depends_on() { + let user = vec![PipelineParameter { + name: "dependsOn".to_string(), + display_name: None, + param_type: Some("string".to_string()), + default: None, + values: None, + }]; + let err = build_parameters(&user, false, true).unwrap_err(); + let msg = format!("{}", err); + assert!( + msg.contains("dependsOn") && msg.contains("reserved"), + "Expected reserved-name error mentioning dependsOn, got: {}", + msg + ); + } + + #[test] + fn test_build_parameters_template_target_rejects_reserved_condition() { + let user = vec![PipelineParameter { + name: "condition".to_string(), + display_name: None, + param_type: Some("string".to_string()), + default: None, + values: None, + }]; + let err = build_parameters(&user, false, true).unwrap_err(); + let msg = format!("{}", err); + assert!( + msg.contains("condition") && msg.contains("reserved"), + "Expected reserved-name error mentioning condition, got: {}", + msg + ); + } + + #[test] + fn test_build_parameters_non_template_target_allows_depends_on_name() { + // For standalone/1es, dependsOn and condition are just regular UI param names. + let user = vec![ + PipelineParameter { + name: "dependsOn".to_string(), + display_name: None, + param_type: Some("string".to_string()), + default: None, + values: None, + }, + PipelineParameter { + name: "condition".to_string(), + display_name: None, + param_type: Some("string".to_string()), + default: None, + values: None, + }, + ]; + let params = build_parameters(&user, false, false).unwrap(); + assert_eq!(params.len(), 2); + } + #[test] fn test_generate_parameters_rejects_expression_in_display_name() { let params = vec![PipelineParameter { @@ -7851,18 +8085,106 @@ safe-outputs: #[test] fn test_generate_agentic_depends_on_empty_steps() { - assert!(generate_agentic_depends_on(&[], false, false, &[]).is_empty()); + assert!(generate_agentic_depends_on(&[], false, false, &[], false).is_empty()); } #[test] fn test_generate_agentic_depends_on_with_steps() { let step: serde_yaml::Value = serde_yaml::from_str("bash: x").unwrap(); assert_eq!( - generate_agentic_depends_on(&[step], false, false, &[]), + generate_agentic_depends_on(&[step], false, false, &[], false), "dependsOn: Setup" ); } + #[test] + fn test_generate_agentic_depends_on_jobs_template_with_setup() { + // When compiling for target: job and the agent has a Setup job, both + // dependsOn and condition (no internal gates here) are emitted as + // dual-branch ${{ if }} template expressions so external template + // parameters merge correctly. + let step: serde_yaml::Value = serde_yaml::from_str("bash: x").unwrap(); + let out = generate_agentic_depends_on(&[step], false, false, &[], true); + // dependsOn branches + assert!( + out.contains("${{ if eq(length(parameters.dependsOn), 0) }}:"), + "missing empty-deps branch: {out}" + ); + assert!( + out.contains("${{ if ne(length(parameters.dependsOn), 0) }}:"), + "missing non-empty-deps branch: {out}" + ); + // Each-iteration over external list, with Setup as the leading item + assert!(out.contains("- Setup"), "missing Setup leading item: {out}"); + assert!( + out.contains("${{ each d in parameters.dependsOn }}:"), + "missing each: {out}" + ); + // condition branches (no internal gates, so external-only path) + assert!( + out.contains("${{ if ne(parameters.condition, '') }}:"), + "missing non-empty-condition branch: {out}" + ); + assert!( + out.contains("condition: ${{ parameters.condition }}"), + "missing condition inline emit: {out}" + ); + } + + #[test] + fn test_generate_agentic_depends_on_jobs_template_no_setup() { + // No internal Setup, no gates: only the non-empty-external branches + // are emitted (both dependsOn and condition default to omitted). + let out = generate_agentic_depends_on(&[], false, false, &[], true); + assert!( + !out.contains("${{ if eq(length(parameters.dependsOn), 0) }}:"), + "should not emit empty-deps branch when no internal Setup: {out}" + ); + assert!( + out.contains("${{ if ne(length(parameters.dependsOn), 0) }}:"), + "missing non-empty-deps branch: {out}" + ); + assert!( + out.contains("dependsOn: ${{ parameters.dependsOn }}"), + "missing simple dependsOn inline: {out}" + ); + assert!( + !out.contains("${{ if eq(parameters.condition, '') }}:"), + "should not emit empty-condition branch when no internal condition: {out}" + ); + assert!( + out.contains("${{ if ne(parameters.condition, '') }}:"), + "missing non-empty-condition branch: {out}" + ); + } + + #[test] + fn test_generate_agentic_depends_on_jobs_template_with_pr_gate() { + // With internal PR gate, the condition block emits BOTH branches: + // empty-external uses the existing internal expression verbatim; + // non-empty-external ANDs the external clause into the body. + let out = generate_agentic_depends_on(&[], true, false, &[], true); + assert!( + out.contains("${{ if eq(parameters.condition, '') }}:"), + "missing empty-condition branch: {out}" + ); + assert!( + out.contains("${{ if ne(parameters.condition, '') }}:"), + "missing non-empty-condition branch: {out}" + ); + // The non-empty branch ANDs the external clause in. + assert!( + out.contains("${{ parameters.condition }}"), + "missing external condition reference: {out}" + ); + // The PR gate clause appears in both branches. + assert_eq!( + out.matches("prGate.SHOULD_RUN").count(), + 2, + "PR gate clause must appear in both empty/non-empty branches: {out}" + ); + } + #[test] fn test_generate_finalize_steps_empty() { assert!(generate_finalize_steps(&[]).is_empty()); diff --git a/src/compile/job.rs b/src/compile/job.rs index 6174488f..c738dd54 100644 --- a/src/compile/job.rs +++ b/src/compile/job.rs @@ -71,14 +71,24 @@ fn generate_job_header(input_path: &Path, output_path: &Path, front_matter: &Fro header.push_str("#\n"); header.push_str("# jobs:\n"); header.push_str(&format!("# - template: {}\n", lock_path)); + header.push_str("# parameters:\n"); + header.push_str("# dependsOn: [Build] # list of upstream job names; omit for implicit dep on previous job\n"); + header.push_str("# condition: succeeded('Build') # omit for ADO's default succeeded()\n"); header.push_str("#\n"); - header.push_str("# Or inside a stage in a multi-stage pipeline:\n"); + header.push_str("# Or inside a user-defined stage in a multi-stage pipeline:\n"); header.push_str("#\n"); header.push_str("# stages:\n"); header.push_str("# - stage: AgenticReview\n"); header.push_str("# dependsOn: Build\n"); header.push_str("# jobs:\n"); header.push_str(&format!("# - template: {}\n", lock_path)); + header.push_str("#\n"); + header.push_str("# ADO's jobs.template schema only allows `template:` and `parameters:` at\n"); + header.push_str("# the call site \u{2014} `dependsOn:` / `condition:` on a `- template:` call are\n"); + header.push_str("# rejected. Pass them via `parameters:` so the template applies them inside.\n"); + header.push_str("# When the agent has a Setup job (e.g. PR/pipeline filters), `dependsOn` MUST\n"); + header.push_str("# be a list so the template can merge `Setup` with the caller's deps.\n"); + header.push_str("# See https://learn.microsoft.com/azure/devops/pipelines/yaml-schema/jobs-template\n"); // Document required resources if agent uses repos if !front_matter.repositories.is_empty() { diff --git a/src/compile/pr_filters.rs b/src/compile/pr_filters.rs index b61314cf..c12bd621 100644 --- a/src/compile/pr_filters.rs +++ b/src/compile/pr_filters.rs @@ -238,7 +238,7 @@ mod tests { #[test] fn test_generate_agentic_depends_on_with_pr_filters() { - let result = generate_agentic_depends_on(&[], true, false, &[]); + let result = generate_agentic_depends_on(&[], true, false, &[], false); assert!(result.contains("dependsOn: Setup"), "should depend on Setup"); assert!(result.contains("condition:"), "should have condition"); assert!(result.contains("Build.Reason"), "should check Build.Reason"); @@ -248,14 +248,14 @@ mod tests { #[test] fn test_generate_agentic_depends_on_setup_only_no_condition() { let step: serde_yaml::Value = serde_yaml::from_str("bash: echo hello").unwrap(); - let result = generate_agentic_depends_on(&[step], false, false, &[]); + let result = generate_agentic_depends_on(&[step], false, false, &[], false); assert_eq!(result, "dependsOn: Setup"); assert!(!result.contains("condition:"), "no condition without PR filters"); } #[test] fn test_generate_agentic_depends_on_nothing() { - let result = generate_agentic_depends_on(&[], false, false, &[]); + let result = generate_agentic_depends_on(&[], false, false, &[], false); assert!(result.is_empty()); } @@ -540,6 +540,7 @@ mod tests { false, false, &["eq(variables['Custom.ShouldRun'], 'true')"], + false, ); // No setup steps, no PR filters → no dependsOn, but the expression produces a condition. assert!(!result.contains("dependsOn"), "no dependsOn without setup/filters"); @@ -555,6 +556,7 @@ mod tests { true, false, &["eq(variables['Custom.Flag'], 'yes')"], + false, ); assert!(result.contains("prGate.SHOULD_RUN"), "should check gate output"); assert!(result.contains("Custom.Flag"), "should include expression"); diff --git a/src/compile/stage.rs b/src/compile/stage.rs index ba59a3a1..00809cae 100644 --- a/src/compile/stage.rs +++ b/src/compile/stage.rs @@ -78,8 +78,13 @@ fn generate_stage_header(input_path: &Path, output_path: &Path, front_matter: &F header.push_str("#\n"); header.push_str("# stages:\n"); header.push_str(&format!("# - template: {}\n", lock_path)); - header.push_str("# dependsOn: Build\n"); - header.push_str("# condition: succeeded()\n"); + header.push_str("# parameters:\n"); + header.push_str("# dependsOn: Build # or [Build, Test]; omit for implicit dep on previous stage\n"); + header.push_str("# condition: succeeded('Build') # omit for ADO's default succeeded()\n"); + header.push_str("#\n"); + header.push_str("# ADO's stages.template schema only allows `template:` and `parameters:` at\n"); + header.push_str("# the call site \u{2014} `dependsOn:` / `condition:` are passed via parameters.\n"); + header.push_str("# See https://learn.microsoft.com/azure/devops/pipelines/yaml-schema/stages-template\n"); // Document required resources if agent uses repos if !front_matter.repositories.is_empty() { diff --git a/src/data/stage-base.yml b/src/data/stage-base.yml index 8de31813..484b043f 100644 --- a/src/data/stage-base.yml +++ b/src/data/stage-base.yml @@ -4,6 +4,16 @@ stages: - stage: {{ stage_prefix }} displayName: {{ agent_display_name }} + # External ordering — applied only when the caller passes the corresponding + # template parameter. ADO does not permit `dependsOn:` / `condition:` as + # bare keys at a `- template:` call site (only `template:` and + # `parameters:` are valid per the `stages.template` schema), so we surface + # them as template parameters and apply them here. Empty defaults preserve + # ADO's implicit "depends on previous stage" and `succeeded()` behaviour. + ${{ if ne(length(parameters.dependsOn), 0) }}: + dependsOn: ${{ parameters.dependsOn }} + ${{ if ne(parameters.condition, '') }}: + condition: ${{ parameters.condition }} jobs: {{ setup_job }} - job: {{ stage_prefix }}_Agent diff --git a/tests/compiler_tests.rs b/tests/compiler_tests.rs index 295b60c7..7e47d522 100644 --- a/tests/compiler_tests.rs +++ b/tests/compiler_tests.rs @@ -4588,3 +4588,278 @@ fn regex_captures_markers(content: &str) -> Vec { } results } + +// ===================================================================== +// External stage/job ordering for template targets +// ===================================================================== +// +// `target: stage` and `target: job` are ADO template targets. ADO's +// `stages.template` / `jobs.template` schemas only permit `template:` and +// `parameters:` at the call site — `dependsOn:` and `condition:` as bare +// keys on the `- template:` line are rejected. The compiler surfaces them +// as auto-injected template parameters and applies them inside the +// template via ADO conditional template expressions. +// +// These tests pin the new contract: parameter declarations are emitted, +// inner stage/job blocks contain the conditional `${{ if … }}` blocks, +// internal Setup/gate behaviour is preserved when callers omit the new +// params, and standalone/1ES targets are untouched. + +#[test] +fn test_stage_target_auto_injects_depends_on_and_condition_params() { + let compiled = compile_fixture("stage-agent.md"); + assert!( + compiled.contains("- name: dependsOn\n type: object\n default: []"), + "stage target should auto-inject dependsOn param. got:\n{compiled}" + ); + assert!( + compiled.contains("- name: condition\n type: string\n default: ''"), + "stage target should auto-inject condition param. got:\n{compiled}" + ); +} + +#[test] +fn test_stage_target_emits_conditional_blocks_on_inner_stage() { + let compiled = compile_fixture("stage-agent.md"); + assert!( + compiled.contains("${{ if ne(length(parameters.dependsOn), 0) }}:"), + "stage target should emit conditional dependsOn block. got:\n{compiled}" + ); + assert!( + compiled.contains("dependsOn: ${{ parameters.dependsOn }}"), + "stage target should pass parameters.dependsOn through to the inner stage. got:\n{compiled}" + ); + assert!( + compiled.contains("${{ if ne(parameters.condition, '') }}:"), + "stage target should emit conditional condition block. got:\n{compiled}" + ); + assert!( + compiled.contains("condition: ${{ parameters.condition }}"), + "stage target should pass parameters.condition through to the inner stage. got:\n{compiled}" + ); +} + +#[test] +fn test_job_target_auto_injects_depends_on_and_condition_params() { + let compiled = compile_fixture("job-agent.md"); + assert!( + compiled.contains("- name: dependsOn\n type: object\n default: []"), + "job target should auto-inject dependsOn param. got:\n{compiled}" + ); + assert!( + compiled.contains("- name: condition\n type: string\n default: ''"), + "job target should auto-inject condition param. got:\n{compiled}" + ); +} + +#[test] +fn test_job_target_minimal_emits_non_empty_only_branches() { + // job-agent.md is a minimal fixture without Setup steps or PR/pipeline + // gates. The dependsOn and condition blocks should only emit the + // `ne(..., default)` branch — there is no internal Setup/gate to + // preserve in the alternate branch. + let compiled = compile_fixture("job-agent.md"); + assert!( + !compiled.contains("${{ if eq(length(parameters.dependsOn), 0) }}:"), + "minimal job target should not emit empty-deps branch (no internal Setup). got:\n{compiled}" + ); + assert!( + compiled.contains("${{ if ne(length(parameters.dependsOn), 0) }}:"), + "minimal job target should emit non-empty deps branch. got:\n{compiled}" + ); + assert!( + compiled.contains("dependsOn: ${{ parameters.dependsOn }}"), + "minimal job target should pass dependsOn through directly. got:\n{compiled}" + ); + assert!( + !compiled.contains("${{ if eq(parameters.condition, '') }}:"), + "minimal job target should not emit empty-condition branch (no internal condition). got:\n{compiled}" + ); + assert!( + compiled.contains("condition: ${{ parameters.condition }}"), + "minimal job target should pass condition through directly. got:\n{compiled}" + ); +} + +#[test] +fn test_standalone_target_does_not_auto_inject_template_params() { + // Standalone is a root pipeline, not a template; auto-injecting + // dependsOn/condition as runtime UI parameters would be wrong. + let compiled = compile_fixture("minimal-agent.md"); + assert!( + !compiled.contains("- name: dependsOn"), + "standalone must not auto-inject dependsOn parameter. got:\n{compiled}" + ); + assert!( + !compiled.contains("- name: condition"), + "standalone must not auto-inject condition parameter. got:\n{compiled}" + ); +} + +#[test] +fn test_1es_target_does_not_auto_inject_template_params() { + let compiled = compile_fixture("1es-test-agent.md"); + assert!( + !compiled.contains("- name: dependsOn"), + "1es must not auto-inject dependsOn parameter. got:\n{compiled}" + ); + assert!( + !compiled.contains("- name: condition"), + "1es must not auto-inject condition parameter. got:\n{compiled}" + ); +} + +#[test] +fn test_template_target_rejects_reserved_depends_on_parameter_name() { + // Compile-time validation must reject user front-matter that declares + // a parameter named dependsOn or condition under target: job/stage — + // those names are reserved for template-invocation use. + use std::sync::atomic::{AtomicU64, Ordering}; + static COUNTER: AtomicU64 = AtomicU64::new(0); + let unique_id = COUNTER.fetch_add(1, Ordering::Relaxed); + let temp_dir = std::env::temp_dir().join(format!( + "ado-aw-collision-{}-{}", + std::process::id(), + unique_id, + )); + fs::create_dir_all(&temp_dir).expect("Failed to create temp directory"); + let src = temp_dir.join("collision.md"); + fs::write( + &src, + "---\nname: Test Agent\ndescription: t\ntarget: job\nparameters:\n - name: dependsOn\n type: string\n default: x\n---\n\n# body\n", + ) + .expect("write fixture"); + + let binary_path = PathBuf::from(env!("CARGO_BIN_EXE_ado-aw")); + let out_path = temp_dir.join("collision.lock.yml"); + let output = std::process::Command::new(&binary_path) + .args([ + "compile", + src.to_str().unwrap(), + "-o", + out_path.to_str().unwrap(), + ]) + .output() + .expect("Failed to run compiler"); + + assert!( + !output.status.success(), + "Compilation should fail when user declares reserved param 'dependsOn' for target: job" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("dependsOn") && stderr.contains("reserved"), + "stderr should mention dependsOn as reserved. got:\n{stderr}" + ); + + let _ = fs::remove_dir_all(&temp_dir); +} + +#[test] +fn test_template_target_rejects_reserved_condition_parameter_name() { + use std::sync::atomic::{AtomicU64, Ordering}; + static COUNTER: AtomicU64 = AtomicU64::new(0); + let unique_id = COUNTER.fetch_add(1, Ordering::Relaxed); + let temp_dir = std::env::temp_dir().join(format!( + "ado-aw-collision-cond-{}-{}", + std::process::id(), + unique_id, + )); + fs::create_dir_all(&temp_dir).expect("Failed to create temp directory"); + let src = temp_dir.join("collision.md"); + fs::write( + &src, + "---\nname: Test Agent\ndescription: t\ntarget: stage\nparameters:\n - name: condition\n type: string\n default: x\n---\n\n# body\n", + ) + .expect("write fixture"); + + let binary_path = PathBuf::from(env!("CARGO_BIN_EXE_ado-aw")); + let out_path = temp_dir.join("collision.lock.yml"); + let output = std::process::Command::new(&binary_path) + .args([ + "compile", + src.to_str().unwrap(), + "-o", + out_path.to_str().unwrap(), + ]) + .output() + .expect("Failed to run compiler"); + + assert!( + !output.status.success(), + "Compilation should fail when user declares reserved param 'condition' for target: stage" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("condition") && stderr.contains("reserved"), + "stderr should mention condition as reserved. got:\n{stderr}" + ); + + let _ = fs::remove_dir_all(&temp_dir); +} + +#[test] +fn test_job_target_with_setup_emits_dual_branch_dependson_with_each() { + // When the agent has a Setup job (e.g. via setup: steps or PR/pipeline + // gates), the dependsOn block must emit BOTH branches: empty-external + // preserves the existing `dependsOn: Setup`; non-empty-external uses + // `${{ each }}` to merge `Setup` with the caller's deps into a single + // list. This is the only place we use `${{ each }}` in the compiler; + // the parameter MUST be declared as type: object and callers MUST + // pass a list because ${{ each }} iterates only iterables. + use std::sync::atomic::{AtomicU64, Ordering}; + static COUNTER: AtomicU64 = AtomicU64::new(0); + let unique_id = COUNTER.fetch_add(1, Ordering::Relaxed); + let temp_dir = std::env::temp_dir().join(format!( + "ado-aw-job-setup-{}-{}", + std::process::id(), + unique_id, + )); + fs::create_dir_all(&temp_dir).expect("Failed to create temp directory"); + let src = temp_dir.join("job-setup.md"); + fs::write( + &src, + "---\nname: Job With Setup\ndescription: t\ntarget: job\nsetup:\n - bash: echo s\n---\n\n# body\n", + ) + .expect("write fixture"); + + let binary_path = PathBuf::from(env!("CARGO_BIN_EXE_ado-aw")); + let out_path = temp_dir.join("job-setup.lock.yml"); + let output = std::process::Command::new(&binary_path) + .args([ + "compile", + src.to_str().unwrap(), + "-o", + out_path.to_str().unwrap(), + ]) + .output() + .expect("Failed to run compiler"); + assert!( + output.status.success(), + "compile should succeed: {}", + String::from_utf8_lossy(&output.stderr) + ); + let compiled = fs::read_to_string(&out_path).expect("read output"); + + // Both branches present + assert!( + compiled.contains("${{ if eq(length(parameters.dependsOn), 0) }}:"), + "should emit empty-deps branch when has_setup. got:\n{compiled}" + ); + assert!( + compiled.contains("${{ if ne(length(parameters.dependsOn), 0) }}:"), + "should emit non-empty-deps branch when has_setup. got:\n{compiled}" + ); + // Empty-deps branch preserves internal Setup dependency + assert!( + compiled.contains("dependsOn: Setup"), + "empty-deps branch should keep dependsOn: Setup. got:\n{compiled}" + ); + // Non-empty branch uses ${{ each }} over a list, with Setup as the first item + assert!( + compiled.contains("${{ each d in parameters.dependsOn }}:"), + "non-empty-deps branch should use ${{{{ each }}}} to iterate. got:\n{compiled}" + ); + + let _ = fs::remove_dir_all(&temp_dir); +}