refactor(compile): split agentic-pipeline shape out of standalone_ir.rs and lift Agent-job conditions into Declarations

[jamesadevine]

Summary

This PR delivers two related refactors in the compile pipeline path:

1. Extract canonical agentic pipeline shape into src/compile/agentic_pipeline.rs

   • Moves shared Setup → Agent → Detection → SafeOutputs → Teardown construction out of standalone_ir.rs.
   • Keeps standalone_ir.rs as a thin wrapper, aligned with job_ir.rs / stage_ir.rs / onees_ir.rs.
   • Retargets wrapper imports and related doc references to the new module.

2. Lift Agent-job condition construction into extension declarations

   • Adds agent_conditions: Vec<Condition> to Declarations.
   • Moves Agent condition clause construction into AdoScriptExtension::build_agent_conditions().
   • Removes build_agentic_condition and introduces fold_agent_conditions to fold extension-contributed clauses with leading Condition::Succeeded.
   • Preserves previous empty-condition behavior (no explicit Agent job condition).

Follow-up review feedback is also addressed in src/compile/agentic_pipeline.rs:

• Removed a stale #[allow(unused_imports)] block/comment.
• Updated wire_explicit_dependencies to resolve Teardown through prefix.id("Teardown")? for consistent typed ID handling.

No intended behavior change from the review-follow-up cleanup; fixture outputs remain unchanged.

Test plan

• cargo build
• cargo test
• cargo clippy --all-targets -- -D warnings
• cargo test agentic_pipeline
• cargo test fold_agent_conditions
• cargo test --test bash_lint_tests
• Secret scan on changed file: src/compile/agentic_pipeline.rs (no findings)
• Parallel validation run (Code Review + CodeQL): no findings

[github-actions]

🔍 Rust PR Review

Summary: Looks good — clean, well-tested refactor with zero behaviour change. Two minor observations worth a quick cleanup pass before merge.

Findings

⚠️ Suggestions

• src/compile/agentic_pipeline.rs:73–75 — Stale comment on the #[allow(unused_imports)] block:

  // Suppress unused; this module is wired up in a sibling commit.
  #[allow(unused_imports)]
  use super::common::{generate_acquire_ado_token, generate_executor_ado_env};

  This comment was copied verbatim from standalone_ir.rs where it described a pending sibling commit. In agentic_pipeline.rs both functions are already called via the common:: qualified path (lines 203, 210, 217), so the unqualified use statement is genuinely redundant — not pending. The stale "wired up in a sibling commit" wording could mislead a future contributor into thinking there is outstanding work. The simplest fix is to remove the use, the #[allow], and the comment entirely.

• src/compile/agentic_pipeline.rs — wire_explicit_dependencies, Teardown branch — All four other job IDs are resolved through prefix.id(...)? (typed, fallible), but Teardown falls through to a bare string comparison:

  } else if j.id.as_str() == "Teardown" {

  Functionally correct today — JobPrefix::id always returns the unprefixed "Teardown" regardless of prefix — but the inconsistency means anyone who later adds "Teardown" to the prefixed arm in JobPrefix::id would silently break the wiring without a compile error. Pre-computing let teardown_id = prefix.id("Teardown")?; alongside the other four IDs at the top of the function would make the pattern uniform.

✅ What Looks Good

• Zero fixture drift across all 33 lock-file fixtures is the best possible signal for a refactor of this scope.
• Declarations::agent_conditions design — typed OutputRef over static step IDs means a future step-ID rename becomes a graph-validation compile error rather than a silently broken runtime condition.
• fold_agent_conditions contract — the None-for-empty path preserves the pre-lift behaviour (Agent job with no explicit condition: inherits ADO's implicit succeeded()), and the And([Succeeded, ...]) path preserves emission order. Both paths are pinned by unit tests.
• Error propagation in build_agent_conditions — StepId::new(...)? for static step-ID strings propagates a contract violation as a compile-phase error rather than a runtime panic.
• Declarations::Default coverage — agent_conditions correctly defaults to Vec::new() via #[derive(Default)], and ..Declarations::default() in ado_script.rs::declarations zero-initialises every unset field cleanly.

Generated by Rust PR Reviewer for issue #995 · sonnet46 2.9M · ◷

[jamesadevine]

@copilot address feedback and suggestions

[Copilot]

@copilot address feedback and suggestions

Addressed in 3e7bdce.

• Removed the stale #[allow(unused_imports)] block and sibling-commit comment in src/compile/agentic_pipeline.rs.
• Updated wire_explicit_dependencies to resolve Teardown via prefix.id("Teardown")? and compare typed IDs consistently.

Validation run: targeted cargo test filters for agentic_pipeline/fold_agent_conditions, cargo clippy --all-targets -- -D warnings, and parallel validation (Code Review + CodeQL) all passed.