fix(compile): allow external dependsOn/condition on job and stage template targets

[jamesadevine]

Summary

target: stage and target: job produce ADO templates. The previous
documentation and the generated header comments instructed users to
write:

stages:
  - template: agents/review.lock.yml
    dependsOn: Build
    condition: succeeded()

…but ADO's stages.template
and jobs.template
schemas only permit template: and parameters: at a template call
site. ADO rejects dependsOn:/condition: as bare keys there. As a
result the documented external-ordering recipe never actually worked.

This PR fixes the recipe by surfacing external ordering as two
auto-injected template parameters (dependsOn, condition) and
applying them inside the template with ADO conditional template
expressions, preserving ADO's native defaults when the caller omits
them.

Behaviour

build_parameters now takes is_template_target: bool and, when true,
auto-injects:

• dependsOn (object, default [])
• condition (string, default '')

It also rejects user-declared front-matter parameters whose names
collide with the two reserved names.

For target: stage: the inner - stage: block conditionally emits
dependsOn: / condition: via ${{ if ne(...) }} blocks. Empty
defaults preserve ADO's implicit "depends on previous stage" and
succeeded() behaviour.

For target: job: the Agent job's existing internal dependsOn: Setup and condition: (PR/pipeline gates) must merge with the
external parameters because ADO only permits one dependsOn:/one
condition: per job. generate_agentic_depends_on gained an
is_jobs_template_target flag that emits mutually-exclusive ${{ if eq/ne }} branches:

• dependsOn empty branch ⇒ existing dependsOn: Setup (zero
  behavioural change today)
• dependsOn non-empty branch ⇒ ${{ each }} merges Setup + the
  caller's deps into one list
• condition empty branch ⇒ the existing internal expression
  verbatim
• condition non-empty branch ⇒ the existing expression with the
  caller's clause ANDed as an additional term

Standalone and 1ES targets are untouched (their parameters: block is
runtime UI parameters, not template-invocation parameters).

Usage after the fix

stages:
  - stage: Build
    jobs: …
  - template: agents/review.lock.yml
    parameters:
      dependsOn: Build              # or [Build, Test]
      condition: succeeded('Build') # ANDed into the agent's internal condition

jobs:
  - job: Build
    steps: …
  - template: agents/review.lock.yml
    parameters:
      dependsOn: [Build]            # list required when agent has Setup; ${{ each }} merges it with Setup
      condition: succeeded('Build')

Test plan

• cargo build ✓
• cargo test ✓ — 1644 unit tests + 118 compiler integration tests + 9 new integration tests + 9 new unit tests, all green
• cargo clippy --all-targets --all-features ✓
• cargo test --test bash_lint_tests ✓
• Hand-verified compiled lock files for:
  • minimal target: stage fixture — conditional blocks render with correct indentation and ADO-template-expression syntax
  • target: job fixture with PR filters — dual-branch dependsOn (each-merge with Setup) and dual-branch condition (existing PR-gate expression preserved verbatim in the empty branch; ANDed with ${{ parameters.condition }} in the non-empty branch)

New tests

• test_stage_target_auto_injects_depends_on_and_condition_params
• test_stage_target_emits_conditional_blocks_on_inner_stage
• test_job_target_auto_injects_depends_on_and_condition_params
• test_job_target_minimal_emits_non_empty_only_branches
• test_job_target_with_setup_emits_dual_branch_dependson_with_each
• test_standalone_target_does_not_auto_inject_template_params
• test_1es_target_does_not_auto_inject_template_params
• test_template_target_rejects_reserved_depends_on_parameter_name
• test_template_target_rejects_reserved_condition_parameter_name
• 6 new build_parameters unit tests + 3 new generate_agentic_depends_on unit tests in src/compile/common.rs

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

[github-actions]

🔍 Rust PR Review

Summary: Looks good — fixes a real ADO schema correctness bug with a well-reasoned implementation. A couple of minor observations below.

Findings

⚠️ Suggestions

• src/compile/common.rs — build_parameters invoked twice per template-target compile: generate_template_parameters (line ~1295) and compile_shared (line ~3258) both independently call build_parameters with identical inputs, running the same reserved-name validation and dependsOn/condition injection twice. Currently harmless since both calls are idempotent, but if someone later changes one callsite and not the other the parameter lists would silently diverge. Consider extracting the built list so generate_template_parameters takes a pre-built &[PipelineParameter] slice (same pattern as generate_parameters).

• src/compile/common.rs — length() on object type at stage level: stage-base.yml uses ${{ if ne(length(parameters.dependsOn), 0) }}: to detect whether the caller passed a dependsOn value. Because the parameter is declared type: object, a caller can pass either an array ([Build]) or a scalar string (Build). ADO's length() on a string returns character count (e.g. length('Build') == 5), so the branch still fires correctly — but it's semantically surprising compared to what one might mean by "non-empty list". The job path (no-setup branch) emits dependsOn: ${{ parameters.dependsOn }} and would pass a bare string through to the job's dependsOn: key, which ADO accepts; this is fine and is documented. Just worth a comment in stage-base.yml near the length() check explaining why length() on a string value still produces the right answer.

✅ What Looks Good

• Dual-branch ${{ if eq/ne }} pattern is exactly the right ADO template idiom for backward-compatible defaults — empty default preserves existing behaviour, non-default value activates the merge/passthrough path.
• ${{ each d in parameters.dependsOn }}: - ${{ d }} is correct ADO sequence-insertion syntax for merging Setup with caller-supplied deps; the type: object, default: [] pairing enforces the "must be a list" contract at the template parameter level.
• Reserved-name validation (dependsOn/condition) fires before any YAML is emitted, produces an actionable error message, and is tested for both names on both target: job and target: stage.
• Test coverage is thorough: unit tests on build_parameters and generate_agentic_depends_on, integration fixture tests for both targets, error-path tests via the compiled binary, and guards that standalone/1ES targets are unaffected.
• trim_end_matches('\n') placed only at the final output boundary — not mid-string — keeps the internal \n structure correct for replace_with_indent.

Generated by Rust PR Reviewer for issue #823 · sonnet46 1.6M · ◷