refactor(compile): reduce complexity of build_pipeline_context in agentic_pipeline.rs

[github-actions]

What was complex

build_pipeline_context in src/compile/agentic_pipeline.rs was flagged by cargo clippy -W clippy::too_many_lines at 258/100 lines — the highest of any function in the crate. It mixed several independent concerns inline: front-matter validation, extension declaration collection, engine invocation/env/install-steps computation, and extension declaration fan-out into Agent job steps/conditions.

What changed

Extracted four cohesive blocks into named helper functions, each with a doc comment noting it exists purely to reduce cognitive complexity:

• validate_pipeline_front_matter — the sequence of shared front-matter validators (identity, permissions, timeout, variable groups, safe-outputs keys, threat detection, require-approval, staged, GitHub issue outputs, comment target, work-item target, PR review events/votes, PR thread statuses, ado-aw-debug config, supply-chain).
• collect_extension_declarations — the per-extension Declarations collection loop, including warning surfacing.
• fanout_extension_declarations — fan-out of collected Declarations into Agent setup steps, prepare steps, and conditions (plus prompt-supplement raw-YAML steps).
• build_engine_setup — engine invocation, install-steps, composed env block, and Copilot BYOM/BYOK exclusion-key computation for both the Agent and Detection engines (returns a small EngineSetup struct).

Before / after

• Before: too_many_lines (258/100)
• After: too_many_lines (192/100)

build_pipeline_context is still above the lint's threshold; the remaining body (network/MCPG config generation and paths/tokens/prompt content assembly) is a natural next target for a follow-up refactor.

Verification

• No public API or observable behavior changes — no signatures changed, only internal decomposition.
• Full test suite: cargo test — 2948 passed, 0 failed.
• cargo clippy --all-targets --all-features — clean, no warnings.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• spsprodeus21.vssps.visualstudio.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "spsprodeus21.vssps.visualstudio.com"

See Network Configuration for more information.

Generated by Cyclomatic Complexity Reducer · auto · 133.3 AIC · ⌖ 12.7 AIC · ⊞ 11.1K · ◷

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).
1 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).
1 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[github-actions]

✅ PR Security Reviewer completed the security review.

🔒 Security review by PR Security Reviewer

[github-actions]

✅ Rust Code Quality Reviewer completed the Rust code quality review.

🦀 Rust code quality review by Rust Code Quality Reviewer

[github-actions]

✅ Test Quality Sentinel completed the test quality analysis.

Reviewed PR #1874: a pure internal refactor of src/compile/agentic_pipeline.rs that extracts four helper functions from build_pipeline_context to reduce cyclomatic complexity. No public API or observable behavior changes, and no test files are touched in this diff. As the Test Quality Sentinel, there is no test code in scope to review here — existing test coverage should already exercise this logic unchanged. No inline comments or review needed.

🧪 Test quality analysis by Test Quality Sentinel

[github-actions]

✅ Compiler Contract Reviewer completed the compiler contract review.

🏗️ Compiler contract review by Compiler Contract Reviewer

[github-actions]

Compiler contract review

This is a pure internal function-extraction refactor confined to src/compile/agentic_pipeline.rs (build_pipeline_context split into validate_pipeline_front_matter, collect_extension_declarations, fanout_extension_declarations, build_engine_setup). No front-matter grammar, safe-output schema, typed IR, extension registration, generated-shell, or documentation contracts are touched, and no other files changed — so none of the drift/registry checks in this review apply.

Signatures, control flow, and error propagation are preserved 1:1 in the diff (confirmed by inspection). No findings.

🏗️ Compiler contract review by Compiler Contract Reviewer · auto · 20.2 AIC · ⌖ 2.7 AIC · ⊞ 12.7K
_{Comment /review to run again}

[github-actions]

Reviewed. This is a pure mechanical refactor extracting four helper functions (validate_pipeline_front_matter, collect_extension_declarations, fanout_extension_declarations, build_engine_setup) out of build_pipeline_context in src/compile/agentic_pipeline.rs. I traced the diff end to end against the security-relevant concerns for this trust boundary (Stage 1/2/3 token isolation, generated-YAML injection, validation strength, network allowlist, safe-output integrity):

• Validation call order and arguments are unchanged (validate_pipeline_front_matter calls the exact same validators in the same sequence).
• Extension declaration collection/fan-out logic (setup steps, agent-prepare steps, conditions, prompt supplements) is copied verbatim, only relocated.
• Engine/token handling (SYSTEM_ACCESSTOKEN projection via ado_bundle.rs, BYOM/BYOK exclusion keys, detection engine env) is untouched — no new token reaches a new step type, and Copilot BYOM exclusion gating (is_copilot match) is preserved unchanged.
• No new raw String fields replace validated newtypes from src/secure.rs; no new external inputs are introduced.
• No changes to src/allowed_hosts.rs, src/sanitize.rs, or src/validate.rs.

No security regression found — this diff is security-neutral (internal decomposition only, no behavior change per the PR description and confirmed by inspection).

🔒 Security review by PR Security Reviewer · auto · 27.5 AIC · ⌖ 2.99 AIC · ⊞ 11.7K
_{Comment /review to run again}

[github-actions]

Rust review: no blocking issues

This is a clean, purely mechanical extraction of build_pipeline_context into four helpers (validate_pipeline_front_matter, collect_extension_declarations, fanout_extension_declarations, build_engine_setup). I traced each extracted block against the original and confirmed:

• Parameter/return threading is correct (e.g. threat_detection/detection_engine_config passed by reference, extension_declarations moved into fanout_extension_declarations after being borrowed by build_engine_setup — no double-move or borrow conflict since the borrow ends before the move).
• Error propagation (?) is preserved at every call site; no unwrap/expect introduced.
• EngineSetup struct destructuring reproduces the original local bindings exactly, no fields dropped or renamed incorrectly.
• No behavioral or ordering changes to validation, extension iteration, or engine env composition.

The rust-critic sub-agent ran but returned no response after the blocking wait, so I relied on my own pass only.

💡 Minor, non-blocking observation

The new free functions use fully-qualified paths like crate::compile::types::ThreatDetectionConfig instead of the module's existing use imports — harmless but slightly inconsistent with the surrounding style. Not worth blocking on.

🦀 Rust code quality review by Rust Code Quality Reviewer · auto · 36 AIC · ⌖ 3.74 AIC · ⊞ 11.5K
_{Comment /review to run again}