feat(smoke): run agentic smoke against compiler candidates

[jamesadevine]

Summary

• add an exact-run supply-chain.pipeline-artifact source for the compiler, AWF, and ado-script.zip, with checksum and producer-provenance validation
• add a test-only compiler-smoke harness and PR/nightly ADO orchestrator that regenerate and run all five agentic smoke workflows from the candidate compiler
• stage candidate YAML on short-lived ado-aw-mirror refs with exact commit pinning, bounded concurrent polling, fail-closed cancellation, and conservative stale-ref cleanup
• keep checked-in smoke locks aligned with the latest released compiler while unreleased PR/main output remains ephemeral
• keep release smoke scheduled/manual-only; direct PR validation is limited to the candidate compiler orchestrator, ado-script E2E, and executor E2E
• harden credentialed AgentPlayground PR definitions against fork execution, extend scheduled failure reporting, and add a root secret-rotation script

AgentPlayground definitions 2554-2559 and the inert candidate base ref are registered and authorized.

Live contract

ADO parent build 625965 and all five candidate children (625967-625971) succeeded:

• each child downloaded the exact artifact while the parent was still running
• provenance matched producer definition 2559, build 625965, and source 1da53db24316854673360cd162ed852a2b232c46
• every runtime integrity check passed under the ado-aw-mirror checkout identity
• the ephemeral candidate ref was deleted after all children became terminal

Test plan

• cargo test
• cargo clippy --all-targets --all-features
• cargo test --test bash_lint_tests
• npm run typecheck
• npm test
• npm run build
• compile and ado-aw check candidate variants of all five smoke workflows with the release-mode candidate binary
• validate the hand-authored orchestrator through the Azure Pipelines Preview API and shellcheck every Bash body
• run the full live ADO parent/five-child candidate contract

[github-actions]

🔍 Rust PR Review

Summary: Looks good — well-typed, comprehensive test coverage, and the security-sensitive shell generation is properly bounded.

Findings

✅ What Looks Good

• Typed inputs throughout: PipelineArtifactConfig uses AdoProject and ArtifactName newtypes for the user-controlled string fields, and u64 for the numeric IDs. No raw String where injection could sneak in.
• validate() is in the compile hot path: sc.validate()? at agentic_pipeline.rs:149 means the feed/pipeline-artifact mutual exclusivity check and the zero-ID guards are enforced before any codegen runs, not just in tests.
• Shell interpolation is constrained: stage_candidate_artifact_payload_bash interpolates only compiler-owned constants for staging, dest_dir, payload, and tail; the PAYLOAD_NAME='{payload}' uses shell single-quoting, and definition_id/run_id are u64 (no injection surface). The SAFETY comment correctly documents this invariant for future callers.
• #[serde(deny_unknown_fields)] on PipelineArtifactConfig prevents silently ignored config keys — consistent with the rest of the supply-chain structs.
• Test coverage is thorough: cross-target compilation (standalone, 1ES, job, stage), registry co-existence, per-job payload routing, exact task-input field assertions with inputs.len() == 7 to catch spurious latest/tags inputs, and inline provenance-marker checks.
• specific_run_inputs → latest_from_branch_inputs rename is accurate: the existing test tests source: specific without runVersion: specific, so the old name was misleading. The new exact_specific_run_inputs is what actually covers the pinned-run case.

⚠️ Suggestions

• feed_connection() / registry_connection() behavioral change (src/compile/types.rs): the refactor from .and_then(|f| f.service_connection.as_deref()).or(self.service_connection.as_deref()) to an early-return ? quietly changes the return value when feed/registry is None — previously the top-level service_connection would still be returned; now None is returned immediately. This is intentional (the new test explicitly asserts it) and semantically more correct, but it's a subtle contract change. Worth a short doc-comment on both methods noting that the top-level service_connection fallback applies only when the subsystem is present.

• Inline Python provenance validator (agentic_pipeline.rs, the long single-line python3 -c '...' in stage_candidate_artifact_payload_bash): this is hard to read and will be painful to maintain or extend. No correctness issue, but extracting it to a short inline heredoc or a named constant would significantly improve auditability.

Generated by Rust PR Reviewer for #1628 · 73.3 AIC · ⌖ 5.72 AIC · ⊞ 6.2K · ◷

[github-actions]

🔍 Rust PR Review

Summary: Looks good — well-structured feature with solid validation, clean type use, and good test coverage. A few minor points worth noting.

Findings

⚠️ Suggestions

• src/compile/agentic_pipeline.rs:stage_candidate_artifact_payload_bash — The staging, dest_dir, payload, and tail parameters are raw &str injected directly into the generated shell script. The doc comment correctly states these must be "compiler-owned constants", and all current callers use hardcoded literals, so there's no real risk now. That said, nothing enforces this at the type level if a future caller passes user-derived content. A newtype wrapper or at least a debug_assert! would make the contract harder to violate accidentally. Low priority since the function is pub(crate) and called from only three sites.

• src/secure.rs:AdoProject allowlist — The invalid_name_char closure blocks a good set of dangerous chars ($, {, }, /, \, etc.) but permits several others that ADO itself disallows in project names (e.g. @, %, ^, ~, &). Since the value lands in a typed DownloadPipelineArtifact@2 task input rather than a shell expansion, this isn't an injection risk, but the validator could give a false "valid" for names that ADO itself would reject. Consider aligning the allowlist with ADO's project naming rules.

✅ What Looks Good

• PipelineArtifactConfig uses AdoProject and ArtifactName validated newtypes for the string fields — the security-critical path follows the project's established src/secure.rs pattern exactly as documented.
• definition_id/run_id are u64 (non-negative by type) with explicit > 0 checks in validate(), and the test suite covers the zero-ID rejection case.
• Mutual exclusivity of feed + pipeline-artifact is validated and tested.
• feed_connection() / registry_connection() refactoring correctly restricts the top-level service_connection fallback so it no longer leaks to unrelated targets (e.g. the registry connection no longer applies when only pipeline-artifact is set). The behavior change is clearly documented in both the method doc-comments and the new assertions in the test.
• CANDIDATE_PROVENANCE_VALIDATOR_PY is safely single-quoted in the generated shell (python3 -c '...') and contains no single quotes itself, so there is no shell injection through the embedded Python.
• serde(deny_unknown_fields) on PipelineArtifactConfig prevents silent field acceptance.
• Test coverage is thorough: parse, zero-ID rejection, mutual exclusivity, invalid chars, and unknown fields are all exercised.

Generated by Rust PR Reviewer for #1628 · 48.4 AIC · ⌖ 5.79 AIC · ⊞ 6.2K · ◷

[github-actions]

🔍 Rust PR Review

Summary: looks good — well-structured feature with solid security boundaries and good test coverage; two minor observations worth considering.

Findings

⚠️ Suggestions

• src/compile/agentic_pipeline.rs — CANDIDATE_PROVENANCE_VALIDATOR_PY bash embedding: The Python script is placed inside a single-quoted python3 -c '...' bash argument. The script currently contains zero literal single quotes, so it works correctly. The {definition!r} / {build!r} repr specifiers emit runtime output to stderr so they don't affect shell parsing. The fragility is that any future edit adding a Python string literal with single-quote delimiters (e.g. print('done')) would silently break the bash — the existing comment warns about this, but a compile-time assertion (assert!(!CANDIDATE_PROVENANCE_VALIDATOR_PY.contains('\''));) or switching to a heredoc would make the invariant machine-enforced rather than comment-only.

• src/compile/types.rs — PipelineArtifactConfig zero-ID validation: definition_id: u64 and run_id: u64 are validated as non-zero in validate(), not at deserialization time. Deserialization already rejects negative values (serde can't fit -1 into u64), so only 0 slips through. NonZeroU64 would catch it at parse time and make the constraint self-documenting, but the current approach is acceptable and consistent with how other cross-field rules work here.

✅ What Looks Good

• The latent bug in feed_connection() and registry_connection() is correctly fixed: the old and_then(...).or(self.service_connection...) chain would return the top-level service connection even when no feed/registry was configured; the new early-return ? prevents that leak.
• stage_candidate_artifact_payload_bash carries a clear SAFETY comment documenting that all shell-interpolated arguments are compiler-owned constants, and every caller upholds this. No user-controlled values enter the generated bash.
• ##vso[task.complete result=Failed] in locate_one uses a static string — $name derives from $PAYLOAD_NAME which is a compiler-owned constant, so there is no ##vso injection surface.
• AdoProject validated newtype correctly blocks /, \, $, {, }, leading _, and leading/trailing . with a clean GUID alternative branch.
• ADO_AW_COMPILE_REMOTE_URL override fails hard on invalid URLs rather than silently falling back, and the three-case unit test (None / valid ADO URL / non-ADO URL) pins the contract.
• PipelineArtifactConfig uses #[serde(deny_unknown_fields)] — typo-resistant and consistent with project style.

Generated by Rust PR Reviewer for #1628 · 67.6 AIC · ⌖ 5.76 AIC · ⊞ 6.2K · ◷

[github-actions]

🔍 Rust PR Review

Summary: Looks good — the new pipeline-artifact supply-chain source is well-implemented with no significant bugs or security concerns. A few minor observations below.

Findings

✅ What Looks Good

• definition_id / run_id are u64 — these are the only values interpolated directly into the generated bash (as {definition_id} / {run_id} in stage_candidate_artifact_payload_bash). Being unsigned integers they can only produce decimal strings, so there's zero shell-injection risk.
• CANDIDATE_PROVENANCE_VALIDATOR_PY contains no single quotes — the Python validator is embedded as a python3 -c '...' single-quoted argument, so the invariant is correctly maintained.
• AdoProject uses the validated_string! macro — character denylist covers the obvious injection vectors (/, \, $, {, }, ;, #, etc.).
• validate() is called at agentic_pipeline.rs:149 — the feed ↔ pipeline-artifact mutual-exclusion check and the positive-ID guards are enforced at compile time, not just in tests.
• feed_connection() / registry_connection() behavioral fix is correct — the old and_then(...).or(top_level) form could leak the top-level service connection even when no feed was configured. The new early-return form matches the documented contract; all call sites already guard with if let Some(feed).
• locate_one + sha256sum pipeline — the count != 1 guard is a solid "exactly one match" check. sub(/^\*/, "", candidate) correctly strips the Windows binary-mode prefix. The awk→sha256sum pipe looks correct.

⚠️ Suggestions

• stage_candidate_artifact_payload_bash — no type-level enforcement for tail/dest_dir/staging (agentic_pipeline.rs, new pub(crate) function): the // SAFETY: comment is the only guard against a future caller accidentally passing user-controlled data for these three parameters. All current call sites use string literals, so this is not a current bug. A private wrapper (or at minimum a debug_assert! that staging / dest_dir start with $(Pipeline.Workspace) or /tmp/) would make the invariant machine-checkable rather than documentation-only.

• rotate-agentplayground-secrets.ps1 — Remove-Variable issuesToken is only in finally (lines 78–86): if Read-Host for the issues token succeeds but Set-AdoAwSecret throws, the try block doesn't explicitly clear $issuesToken before propagating. The finally block does handle it with -ErrorAction SilentlyContinue, so the secret is cleared on any exit path. This is safe but slightly asymmetric compared to the copilotToken handling where Remove-Variable is called eagerly inside the try block after use.

• AdoProject validation — @ is not in the denylist (secure.rs, invalid_name_char closure): ADO project names can't contain @. Since AdoProject values only flow into typed DownloadPipelineArtifact@2 task inputs (never shell-interpolated), this is not a security issue — just a minor spec gap worth noting for completeness.

Generated by Rust PR Reviewer for #1628 · 74.5 AIC · ⌖ 5.83 AIC · ⊞ 6.2K · ◷