fix(compile): checkout additional repos in SafeOutputs job for create-pull-request

[Copilot]

create-pull-request SafeOutputs jobs previously checked out only self, so additional repository directories were missing and ADO's implicit checkout layout could point the executor at the wrong directory. Trigger-scoped Build.Repository.* values could also resolve repository: self to a repository resource that triggered the run rather than the pipeline repository.

Changes

• Check out every PR-eligible repos: entry only in the SafeOutputs variant that executes create-pull-request.
• Pin multi-checkout self to the compiler-owned s/self path and named repositories to s/<alias>.
• Reject reserved and duplicate checkout aliases case-insensitively to prevent Windows path collisions.
• Pass the exact self checkout directory plus resources.repositories['self'] ID/name/ref into Stage 3; retain Build.Repository.* fallbacks for direct and older invocations.
• Resolve named and self repository checkout directories consistently across MCP and executor paths.
• Cover named and self multi-checkout execution in the deterministic executor E2E harness and add compiler regressions for split approval and every target.
• Update path-layout guidance, codemods, and warnings for the fixed self path.

Validation

• cargo test
• cargo build --all-targets
• npm test (scripts/ado-script)
• npm run typecheck (scripts/ado-script)
• npm run build:executor-e2e (scripts/ado-script)

Fixes #1731

[azure-pipelines]

Azure Pipelines:
2 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 2 pipeline(s).
There may be pipelines that require an authorized user to comment /azp run to run.

[jamesadevine]

/rust-review

[github-actions]

✅ Rust PR Reviewer completed successfully!

[github-actions]

🔍 Rust PR Review

Summary: Looks good — the core fix is correct and the security posture improves. A few minor observations worth noting.

---

Findings

✅ What Looks Good

• Path traversal fix (security): The old create_pull_request.rs did ctx.source_directory.join(&self.repository) with raw user input. The new code routes through canonical_repository_alias → resolve_repository_checkout_dir, which resolves only through compiler-controlled alias keys. This is a meaningful security improvement.
• Ambiguous alias rejection: lookup_allowed_repository_alias now returns None for ambiguous trailing-name matches (e.g. "tools" when both ProjectA/tools and ProjectB/tools are configured), preventing silent wrong-repo targeting.
• Case-insensitive reserved/duplicate checks in lower_repos: Correct for Windows path safety. The removal of validate_checkout_self_collision is justified — pinning self to a compiler-owned s/self path makes the name-based collision check obsolete.
• SafeOutputsCheckoutLayout::for_variant: Clean per-variant path computation correctly handles the split-approval case where two Stage 3 jobs have different checkout sets.
• #[cfg(test)] on the old SafeOutputs::new: The routing through new_with_self_repository_directory and the None → bounding_dir fallback preserves backward compatibility for legacy/direct invocations cleanly.
• Test coverage: Good — environment variable priority, fallback behavior, ambiguous alias rejection, checkout dir distinction, and the lower_checkout path field are all unit-tested.

---

⚠️ Suggestions

• resolve_repository_checkout_dir (src/safe_outputs/mod.rs ~848): The function takes a repository: &str parameter but its callers always pass an already-canonicalized alias (from canonical_repository_alias). The internal call to canonical_repository_alias at the top is therefore redundant for those callers, and a future caller passing a raw user string expecting validation might be surprised by the double-canonicalization. Consider either renaming the parameter to alias and requiring a pre-canonicalized input (removing the internal canonical_repository_alias call), or keeping it but documenting the contract explicitly.

• else branch in create_pull_request.rs::Executor (~line 672): The branch return Ok(ExecutionResult::failure(format!("Repository alias '{}' has no configured repository", repository_alias))) is effectively unreachable — if canonical_repository_alias returns an alias key via lookup_allowed_repository_alias, that key is guaranteed to exist in ctx.allowed_repositories (since it came from iterating the map). A debug_assert! or a comment would make the invariant explicit and help future readers.

• ADO_AW_SELF_REPOSITORY_DIRECTORY as EnvValue::literal (agentic_pipeline.rs ~3183): The value $(Build.SourcesDirectory)/self contains an ADO macro. Emitting it as EnvValue::literal produces a YAML string literal, relying on ADO's runtime macro expansion in env values. This is correct ADO behavior, but it's slightly surprising — a comment noting that ADO expands $(...) in env values at agent runtime would clarify why EnvValue::AdoMacro (or a Concat variant) is not needed here.

• SafeOutputsCheckoutLayout::for_variant — trigger_repo_directory assumption: The branch has_additional_checkouts == true returns cfg.trigger_repo_directory as self_repository_directory. This is correct only if trigger_repo_directory is always $(Build.SourcesDirectory)/self in multi-checkout context. The invariant is maintained by the existing compiler, but it is implicit. A debug assertion or an inline comment referencing MULTI_CHECKOUT_SELF_PATH would make this easier to audit.

Generated by Rust PR Reviewer for #1735 · 57.4 AIC · ⌖ 8.32 AIC · ⊞ 6.5K · ◷
_{Comment /rust-review to run again}

[jamesadevine]

/azp run ado-aw candidate compiler smoke

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).