Skip to content

fix(ado-script): treat unsubstituted ADO macros as empty in synthPr#975

Merged
jamesadevine merged 1 commit into
mainfrom
jamesadevine/fix-synth-pr-resolve-unsubstituted-macros
Jun 11, 2026
Merged

fix(ado-script): treat unsubstituted ADO macros as empty in synthPr#975
jamesadevine merged 1 commit into
mainfrom
jamesadevine/fix-synth-pr-resolve-unsubstituted-macros

Conversation

@jamesadevine

Copy link
Copy Markdown
Collaborator

Summary

Follow-up to #972. The first roll-out of the synthPr real-PR shortcut had a
runtime bug — ADO leaves undefined predefined-variable macros as the
literal string $(name) in step env (it does NOT substitute to empty).
The bundle read

SYSTEM_PULLREQUEST_PULLREQUESTID = "$(System.PullRequest.PullRequestId)"

on every non-PR build, saw a non-empty string, and went down the
real-PR shortcut path. Observed by @jamesadevine in the logs:

[synth-pr] real PR build #$(System.PullRequest.PullRequestId);
 propagating SYSTEM_PULLREQUEST_* to AW_PR_*

The bundle then emitted AW_PR_ID = $(System.PullRequest.PullRequestId) as
a literal, and downstream PR-identifier validation rejected it (same
class of failure as build 612528, just one level deeper).

Fix

Add a resolveAdoMacroEnv helper that treats literal $(name) strings
as empty:

function resolveAdoMacroEnv(value: string | undefined): string {
  if (!value) return "";
  if (/^\$\([^()]+\)$/.test(value)) return "";
  return value;
}

Match is on balanced $( ... ) with no nested parens — ADO macro
names never contain parens, so this is precise. Apply to every
SYSTEM_PULLREQUEST_* env read in exec-context-pr-synth/index.ts so
the bundle correctly falls through to the synth-discovery path on
non-PR builds.

Test plan

  • New regression test
    does NOT treat unsubstituted $(System.PullRequest.*) ADO macros as a real PR id
    feeds the exact malformed env from the production log and asserts:
    • "real PR build" log path is NOT taken
    • AW_PR_ID is not contaminated with a literal $(...) string
    • Bundle proceeds to synth-discovery and emits AW_SYNTHETIC_PR_SKIP
      on no match
  • npm test — 28/28 synthPr tests pass (1 new, 12 existing).
  • npm run build:exec-context-pr-synth — bundle rebuilt successfully.
  • No Rust-side changes required: the compiler-emitted YAML is already
    correct (it just passes the macros as env values); the fix is purely
    defensive handling of ADO's unsubstituted-macro behaviour inside the
    consumer bundle.

ADO leaves undefined predefined-variable macros as the literal string
`$(name)` in step env — it does NOT substitute to empty. The first
roll-out of the synthPr real-PR shortcut read
`SYSTEM_PULLREQUEST_PULLREQUESTID = "$(System.PullRequest.PullRequestId)"`
on every non-PR build and treated it as a non-empty real PR id, so it
emitted the literal macro as AW_PR_ID and logged:

  [synth-pr] real PR build #$(System.PullRequest.PullRequestId);
   propagating SYSTEM_PULLREQUEST_* to AW_PR_*

Add `resolveAdoMacroEnv` helper that strips literal `$(name)` strings
to empty (matching balanced `$(` ... `)` with no nested parens — ADO
macro names never contain parens). Apply to every `SYSTEM_PULLREQUEST_*`
env read so the bundle correctly falls through to the synth-discovery
path on non-PR builds.

New regression test covers the macro-literal env case, asserting the
"real PR build" log path is NOT taken and AW_PR_ID is not contaminated
with a literal `$(...)` string.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jamesadevine jamesadevine merged commit 1164c5f into main Jun 11, 2026
14 checks passed
jamesadevine added a commit that referenced this pull request Jun 12, 2026
…hPr stubs

Four follow-ups from the post-IR review of native-ado-compiler:

1. Clarify the OutputDecl / auto_is_output contract.
   The original doc-comment in src/compile/ir/output.rs promised the
   compiler auto-rewrites the bash body to add isOutput=true. It does
   not - the IR never introspects step bodies. The flag is an
   informational signal that extension authors must consult and act
   on themselves. Updated output.rs, graph.rs (outputs_needing_is_output
   field doc), and step.rs (BashStep::outputs field doc) to describe
   the real contract: graph pass detects which decls need the flag,
   producer is responsible for emitting it in the vso directive.

   Forgetting isOutput=true on a producer with cross-step consumers
   is now explicitly called out as a silent-failure mode (and the
   PR #956 / PR #975 / synthPr regression history is cited).

2. Implement Coalesce flattening in src/compile/ir/lower.rs.
   The EnvValue::Coalesce doc promised nested Coalesce values flatten
   into a single outer coalesce(...) call. The lowering pass produced
   nested coalesce(coalesce(...)) instead. ADO accepts both, but the
   IR contract now matches behaviour: added flatten_coalesce_into()
   helper used by both lower_env_value and lower_env_value_as_expr_atom
   in their Coalesce arms. New unit test covers the flatten case
   explicitly. No production producer uses nested Coalesce today,
   so lock files are unchanged.

3. Drop dead AW_SYNTHETIC_PR_* legacy declarations.
   src/compile/extensions/ado_script.rs::SYNTH_PR_OUTPUT_NAMES carried
   three entries (AW_SYNTHETIC_PR_ID, AW_SYNTHETIC_PR_SOURCEBRANCH,
   AW_SYNTHETIC_PR_TARGETBRANCH) with a comment claiming filter_ir.rs
   build_gate_step_typed referenced them via OutputRef. filter_ir.rs
   has zero OutputRef usages - it uses EnvValue::pipeline_var. The
   stubs were truly dead. Removed both the entries and the misleading
   comment. Updated the corresponding test in ado_script.rs.

4. Dedupe duplicated paragraph on SYNTH_PR_OUTPUT_NAMES doc-comment.

cargo build / cargo test (1814 unit + integration) / cargo clippy
--all-targets --all-features / cargo test --test bash_lint_tests all
clean.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jamesadevine added a commit that referenced this pull request Jun 12, 2026
…hPr stubs

Four follow-ups from the post-IR review of native-ado-compiler:

1. Clarify the OutputDecl / auto_is_output contract.
   The original doc-comment in src/compile/ir/output.rs promised the
   compiler auto-rewrites the bash body to add isOutput=true. It does
   not - the IR never introspects step bodies. The flag is an
   informational signal that extension authors must consult and act
   on themselves. Updated output.rs, graph.rs (outputs_needing_is_output
   field doc), and step.rs (BashStep::outputs field doc) to describe
   the real contract: graph pass detects which decls need the flag,
   producer is responsible for emitting it in the vso directive.

   Forgetting isOutput=true on a producer with cross-step consumers
   is now explicitly called out as a silent-failure mode (and the
   PR #956 / PR #975 / synthPr regression history is cited).

2. Implement Coalesce flattening in src/compile/ir/lower.rs.
   The EnvValue::Coalesce doc promised nested Coalesce values flatten
   into a single outer coalesce(...) call. The lowering pass produced
   nested coalesce(coalesce(...)) instead. ADO accepts both, but the
   IR contract now matches behaviour: added flatten_coalesce_into()
   helper used by both lower_env_value and lower_env_value_as_expr_atom
   in their Coalesce arms. New unit test covers the flatten case
   explicitly. No production producer uses nested Coalesce today,
   so lock files are unchanged.

3. Drop dead AW_SYNTHETIC_PR_* legacy declarations.
   src/compile/extensions/ado_script.rs::SYNTH_PR_OUTPUT_NAMES carried
   three entries (AW_SYNTHETIC_PR_ID, AW_SYNTHETIC_PR_SOURCEBRANCH,
   AW_SYNTHETIC_PR_TARGETBRANCH) with a comment claiming filter_ir.rs
   build_gate_step_typed referenced them via OutputRef. filter_ir.rs
   has zero OutputRef usages - it uses EnvValue::pipeline_var. The
   stubs were truly dead. Removed both the entries and the misleading
   comment. Updated the corresponding test in ado_script.rs.

4. Dedupe duplicated paragraph on SYNTH_PR_OUTPUT_NAMES doc-comment.

cargo build / cargo test (1814 unit + integration) / cargo clippy
--all-targets --all-features / cargo test --test bash_lint_tests all
clean.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jamesadevine added a commit that referenced this pull request Jun 13, 2026
* feat(ir): introduce typed pipeline IR (types only, no callers)

First commit of the "Native ADO Pipeline IR" refactor (see plan).
Adds the typed IR root module `src/compile/ir/` with eight
submodules:

- `ids`       - StageId / JobId / StepId newtypes, validated
                  against the ADO identifier grammar.
- `output`    - OutputDecl + OutputRef.
- `env`       - EnvValue { Literal | AdoMacro | PipelineVar |
                  Secret | StepOutput | Coalesce }; AdoMacro is
                  constructor-checked against ALLOWED_ADO_MACROS.
- `condition` - Condition AST + Expr (codegen lands in
                  ir-condition-codegen).
- `step`      - Step enum + BashStep / TaskStep / CheckoutStep /
                  DownloadStep / PublishStep with builder helpers.
- `job`       - Job + Pool (vmImage / 1ES named pool).
- `stage`     - Stage.
- `mod.rs`    - Pipeline / PipelineBody / PipelineShape
                  (Standalone | OneEs | JobTemplate | StageTemplate)
                  + placeholder Parameter / Resources / Triggers /
                  PipelineVar shapes.

No production callers yet - everything is reachable only from the
in-module unit tests. The module carries a deliberate, scoped
`#![allow(dead_code)]` until the `extension-trait-port` commit
wires real callers; the unit tests exercise constructors so silent
breakage would still surface.

Unit-test coverage (59 tests) covers id validation, env-macro
allowlist, condition / step / job / stage / pipeline constructors,
and the OutputRef / OutputDecl / Coalesce shapes.

`cargo build` / `cargo test` / `cargo clippy --all-targets
--all-features` all green.

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

* feat(ir): lower Pipeline to YAML via serde_yaml

Adds the IR-to-YAML lowering and emit passes:

- `src/compile/ir/lower.rs` walks the typed Pipeline / Stage / Job /
  Step tree and produces a `serde_yaml::Value` with canonical key
  order (identity keys -> static config -> payload). Handles every IR
  variant that does not need cross-step resolution.
- `src/compile/ir/emit.rs` is the thin entry point: it composes
  `lower` with `serde_yaml::to_string`. Result is byte-compatible
  with the canonical baseline that the prep PR established (#957).

Variants that need cross-step resolution (`EnvValue::StepOutput`,
`EnvValue::Coalesce`, `Expr::StepOutput`) return a structured
error that names the commit which fills them in
(`ir-output-lowering`). Unit tests cover the success path for the
simple variants and the explicit error path for the deferred ones so
the boundary stays load-bearing as later commits land.

Round-trip acceptance test in `emit::tests` builds a handcrafted
Pipeline with Setup + Agent jobs (containing Checkout / Bash /
Publish / Download steps), emits via `emit`, re-parses the YAML
through `serde_yaml`, and asserts structural equality against a
hand-built reference `Value` - locking the wire shape.

Still no production callers; the `#![allow(dead_code)]` on
`src/compile/ir/mod.rs` stays for now and is removed in
`extension-trait-port`.

`cargo build` / `cargo test` (17 groups, 0 failed) /
`cargo clippy --all-targets --all-features` all green.

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

* feat(ir): derive job and stage dependsOn from OutputRef graph

Adds the dependency-graph pass (`src/compile/ir/graph.rs`):

- Walks every step's `env` and `condition` (including
  `Condition` AST + nested `Coalesce` children) to collect every
  `OutputRef`.
- For each ref, looks up the producer step's location
  (`StepLocation { stage, job, declared outputs }`) and adds either
  a cross-job edge (same stage, different jobs) or a cross-stage edge
  (different stages). Same-job refs contribute nothing - ADO orders
  steps within a job by YAML position.
- Validates as a side-effect: `UnknownProducer`, `AnonymousProducer`,
  `UnknownOutput` (the producer must declare the named output),
  `DuplicateJobId`, `DuplicateStageId`, `DuplicateStepId`, plus
  `MixedStagedAndUnstaged` (cross-stage refs between staged and
  flat-jobs sections are not supported).
- Runs Kahn's algorithm on both job and stage edge sets to detect
  cycles. The error message lists every node still with positive
  in-degree so an operator can locate the offending sub-graph.
- Two entry points:
   * `resolve(p)`  - all-in-one: build graph, detect cycles, merge
     derived edges into `job.depends_on` / `stage.depends_on`
     (existing values preserved).
   * `build_graph(p)` - returns the typed graph without mutating the
     pipeline. Useful for diagnostics and tests.

Nine unit tests cover the major paths: cross-job edge derivation,
cross-stage edge derivation, same-job no-op, unknown producer,
unknown output, duplicate ids, cycle detection (with the listed-nodes
error message), coalesce-child edge collection, and a 5-stage chain
that exercises both per-step env and job-level condition walks.

Still no production callers - the graph pass is reachable only from
its unit tests until `extension-trait-port` wires real callers.

`cargo build` / `cargo test` (17 groups, 0 failed) /
`cargo clippy --all-targets --all-features` all green.

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

* feat(ir): lower OutputRefs to per-location ADO reference syntax

Adds the per-consumer-location lowering for `OutputRef` so that
extensions can write `OutputRef { step, name }` once and have the
IR pick exactly one of the three ADO syntaxes:

  same job              `\$(stepName.X)\`
  cross-job same stage  `dependencies.<job>.outputs[\stepName.X\]`
  cross-stage           `stageDependencies.<stage>.<job>.outputs[\stepName.X\]`

Implementation:

- `output::lower_outputref` is the single source of truth for the
  three-syntax decision; it takes `ConsumerLocation` and
  `ProducerLocation` newtypes so call sites cannot mix them up.
- `lower::LoweringContext` carries the `Graph` + the current
  consumer's stage/job through every recursive `lower_*` helper.
  `lower` (no-arg public entry) now builds the graph internally
  via `graph::build_graph` + `detect_cycles`;
  `lower_with_graph` is exposed for callers that already hold a
  built graph.
- `EnvValue::Coalesce` lowers to a single `\$[ coalesce(<a>, <b>, ..., '') ]\`
  with the trailing `''` safety value appended automatically.
  Nested `Coalesce` is flattened. Children inside `\$[ ... ]\` use
  ADO expression-atom form: `variables['Name']` for predefined
  vars, the un-wrapped step-output reference for `StepOutput`.
- `Condition::Eq` / `Condition::Ne` over `Expr::StepOutput`
  thread the same context into the existing condition codegen
  (the static subset is unchanged; the dynamic subset now resolves
  correctly).

Auto-`isOutput=true` map:

- `graph::Graph` gains `outputs_needing_is_output: BTreeMap<StepId, BTreeSet<String>>`
  populated as a side effect of walking every consumer's
  `OutputRef`. Same-job references count here (ADO needs
  `isOutput=true` for `\$(step.name)\` too).
- `graph::resolve` propagates the map back onto
  `OutputDecl::auto_is_output` so producer extensions can read the
  flag at emit time without re-deriving it. New unit test
  `auto_is_output_flag_only_promotes_referenced_outputs` locks
  the contract: only outputs with at least one reader are promoted.

Test coverage: `output::tests` (4 new lowering-syntax tests),
`lower::tests` (Coalesce round-trip + context threading),
`graph::tests` (auto_is_output + cross-step reader counting).

`cargo build` / `cargo test` (17 groups, 0 failed) /
`cargo clippy --all-targets --all-features` all green.

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

* feat(ir): condition codegen with Custom-injection check

Moves the `Condition` / `Expr` lowering out of `lower.rs` and
into a dedicated `condition::codegen` module so the AST and its
codegen stay colocated.

Functional additions over what `ir-yaml-emit` shipped:

- `Condition::And` / `Condition::Or` flatten nested operators
  of the same kind before lowering. `and(a, and(b, c))` emits as
  `and(a, b, c)` - matches the existing layout in
  `compile_gate_step_external` and stays readable in the YAML.
- `Condition::Custom(s)` now runs through a two-vector injection
  check at lower time:
   * `crate::validate::contains_pipeline_command(s)` rejects
     `##vso[` and `##[` - these would be acted on at runtime if
     echoed by an executor.
   * `crate::validate::contains_newline(s)` rejects embedded
     newlines that would flip a YAML scalar from inline to block
     form and change parse semantics.
  Crucially, the check does NOT reject `\$(...)\`, `\$[...]\`,
  `\${{...}}\` - those are exactly the ADO expressions the escape
  hatch exists for. Tests verify both the pass-through (real ADO
  expressions accepted) and rejection (pipeline-command markers and
  newlines rejected) paths.
- New `CondCodegenCtx { graph, stage, job }` is the per-consumer
  context that `lower::LoweringContext::cond_ctx()` builds on
  demand. The codegen module no longer borrows `lower::LoweringContext`
  directly, so we avoid an internal-module cycle.

Test coverage: 8 new tests in `condition::codegen::tests` cover
every Condition variant, every Expr variant, nested And/Or
flattening, apostrophe-in-Literal escaping, and the two Custom
injection paths. Existing `lower::tests` keep their integration
coverage for env+condition round-trip.

`cargo build` / `cargo test` (17 groups, 0 failed) /
`cargo clippy --all-targets --all-features` all green.

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

* feat(extensions): Declarations bundle + Step::RawYaml migration bridge

Introduces the surface that the upcoming per-extension `port-*`
commits will populate, without breaking any existing call sites.

Two new IR concepts:

- `Step::RawYaml(String)` is the migration bridge - carries
  legacy `Vec<String>` step bodies (which are pre-formatted YAML
  strings) through the IR unchanged. `ir::lower::lower_raw_yaml`
  parses the body into a `serde_yaml::Value` (stripping a
  leading `- ` + de-indenting continuation lines so emitters that
  produced sequence-item form still work) and re-emits it via the
  canonical normalisation. Invalid bodies surface a clear error
  rather than producing malformed YAML. Removed by
  `delete-deprecated-trait-aliases` once no `RawYaml` instances
  remain.

- `extensions::Declarations` is the typed aggregate every
  extension will eventually return: agent_prepare_steps,
  setup_steps, agent_finalize_steps, detection_prepare_steps,
  safe_outputs_steps (all `Vec<Step>`), plus network_hosts,
  bash_commands, prompt_supplement, mcpg_servers,
  copilot_allow_tools, pipeline_env, awf_mounts, awf_path_prepends,
  agent_env_vars, warnings.

`CompilerExtension` gains a `declarations(ctx) -> Result<Declarations>`
method with a default impl that wraps every legacy per-method output -
`prepare_steps` / `setup_steps` results land in
`agent_prepare_steps` / `setup_steps` as `Step::RawYaml`
entries; every other field is copied through verbatim. The
`extension_enum!` macro delegates the new method alongside the
existing ones. `#[allow(dead_code)]` covers production paths
during the migration window; the smoke test in
`extensions::tests::declarations_default_bridges_lean_extension_legacy_methods`
locks the bridge contract end-to-end against
`LeanExtension`.

Subsequent `port-*` commits override `declarations` per
extension with real typed Steps and drop the corresponding legacy
overrides; the final `delete-deprecated-trait-aliases` commit
strips `Step::RawYaml`, the legacy trait methods, and the
`#[allow(dead_code)]` annotations together.

Pragmatic deviation from the plan's "old method names are gone"
acceptance: that would have required updating ~150 call sites
(production + tests) in a single commit and was too risky. The
default-impl bridge keeps every existing call site working while
still establishing the new surface; the migration story for each
extension is unchanged.

`cargo build` / `cargo test` (1883 tests, 0 failed) /
`cargo clippy --all-targets --all-features` / `cargo test --test
bash_lint_tests` all green.

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

* feat(extensions): port AdoAwMarkerExtension to typed Declarations

Adds a declarations() override on AdoAwMarkerExtension returning
the two prepare-phase steps as typed Step::Bash(BashStep) values
(no Step::RawYaml). Coexists with the legacy prepare_steps method
until compile-target-standalone switches production consumption to
declarations().

New helpers marker_bash_step() and aw_info_bash_step() build the
typed BashStep with the same bash bodies as the legacy YAML
strings, so lowering through ir::emit produces equivalent output.
The aw_info step carries Condition::Always (today the YAML string
embeds condition: always() verbatim).

New unit test declarations_returns_typed_bash_steps_not_raw_yaml
locks the shape: must return exactly two Step::Bash values with
the canonical display names. Detailed bash-body assertions stay
on the legacy-form tests.

cargo build / cargo test (1884 tests, 0 failed) / cargo clippy
--all-targets --all-features all green.

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

* feat(extensions): port GitHubExtension to typed Declarations

Adds a declarations() override on GitHubExtension that routes the
single 'github' allow-tool through the Declarations bundle. The
extension contributes nothing else (no steps, hosts, env vars).

Coexists with the legacy allowed_copilot_tools method so production
call sites in src/engine.rs keep working until compile-target-*
switches to declarations() consumption.

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

* feat(extensions): port SafeOutputsExtension to typed Declarations

Adds declarations() override routing mcpg_servers, allowed_copilot_tools,
and prompt_supplement through the Declarations bundle. Coexists with
legacy methods until target compilers switch to declarations()
consumption.

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

* feat(extensions): port AzureCliExtension to typed Declarations

Adds declarations() override returning the two prepare-phase steps
as typed Step::Bash(BashStep) values. The conditional prompt-append
step carries Condition::Ne(Expr::Variable('AW_AZ_MOUNTS'), Expr::Literal(''))
which lowers to the same condition string the YAML emits today:
ne(variables['AW_AZ_MOUNTS'], '').

AW_AZ_MOUNTS is a pipeline variable (set via task.setvariable), not
a step output, so it's referenced via Expr::Variable - no OutputRef
is involved and no isOutput=true is needed.

Coexists with the legacy prepare_steps method until target compilers
switch to declarations() consumption.

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

* feat(runtimes): port Lean/Python/Node/Dotnet to typed Declarations

Each runtime extension now overrides `declarations()` returning typed `Step::Bash` / `Step::Task` values (no `Step::RawYaml` migration bridge). Coexists with the legacy `prepare_steps()` until `compile-target-standalone` switches production callers.

Lean: single `Step::Bash` for the elan install (mounts + PATH prepends flow through the typed bundle).

Python: `Step::Task(UsePythonVersion@0)` plus an optional `Step::Task(PipAuthenticate@1)` when `feed-url:` is set. `PIP_INDEX_URL` / `UV_DEFAULT_INDEX` agent env vars route through the bundle.

Node: `Step::Task(NodeTool@0)` plus, when `feed-url:` or `config:` is set, `Step::Bash` (ensure .npmrc) and `Step::Task(npmAuthenticate@0)`. `NPM_CONFIG_REGISTRY` env var threaded through.

Dotnet: `Step::Task(UseDotNet@2)` covering all three shapes (default 8.0.x / explicit version / `useGlobalJson` for `version: global.json`), plus the same ensure-config + `NuGetAuthenticate@1` pair as Node when `feed-url:` is set (or auth-only when `config:` is set).

Bridge contract test re-anchored on a synthetic in-test stub (declarations_default_bridges_legacy_methods) since LeanExtension now owns a real `declarations()` override. The stub survives any further per-extension port and is removed by `delete-deprecated-trait-aliases` together with `Step::RawYaml`.

Each port adds shape-only unit tests (display names + task IDs + condition kinds, no YAML strings). `cargo test` 1897/0; `cargo clippy --all-targets --all-features` clean.

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

* feat(tools): port AzureDevOps/CacheMemory to typed Declarations

AzureDevOpsExtension contributes no pipeline steps - its typed declarations() override routes the static signals (network hosts, MCPG stdio entry, ADO_MCP_AUTH_TOKEN pipeline_env mapping, --allow-tool azure-devops) through the typed bundle. Shape test asserts agent_prepare_steps.is_empty() and the MCPG entry's stdio type/container fields.

CacheMemoryExtension returns three typed prepare steps: Step::Task(DownloadPipelineArtifact@2) with continueOnError and condition set, then Step::Bash (restore from previous_memory) with the same condition, then Step::Bash (initialise empty memory) gated on the inverse.

Conditions reference the clearMemory template parameter via Condition::Custom("eq(parameters.clearMemory, false)"). The IR's Condition AST only models runtime expressions; Custom is the documented escape hatch for template-time expressions (passes reject_pipeline_injection because the syntax carries no newlines or ##vso[ prefixes).

cargo test 1897/0; cargo clippy --all-targets --all-features clean.

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

* feat(extensions): port AdoScriptExtension to typed Declarations

The marquee port: every step ado-script contributes is rebuilt as a typed
`Step::Bash` / `Step::Task`, with explicit `StepId`/`OutputDecl` on the
`synthPr` producer and typed `EnvValue::StepOutput` references on the
`prGate`/`pipelineGate` consumer. The IR's lowering pass now picks the
ADO reference syntax per consumer location instead of the extension
hard-coding `$(synthPr.X)` strings; same-job today produces the macro
form, cross-job would auto-switch to `dependencies.Setup.outputs[...]`.

New typed-IR primitive: `EnvValue::Concat(Vec<EnvValue>)` — the
macro-form sibling of `Coalesce` that lowers to children joined with no
separator and no `$[ ]` wrap. Used for the mutually-empty exclusive-OR
pattern `$(System.PullRequest.X)$(synthPr.X)` that resolves correctly
inside the producing job (runtime-expression form `$[ variables['synthPr.X'] ]`
silently resolves to empty in the producing job — the bug fixed in
51ae40ee that this port now encodes as an invariant).

`build_gate_step_typed` parallels `compile_gate_step_external` and is
what `declarations()` calls; the legacy YAML-string emitter stays for
production consumption until `compile-target-standalone` lands. Graph
walker and lower's Coalesce expression-atom context both handle the
new variant (Concat inside Coalesce errors — macro form is not an
expression atom).

`synthetic_pr_step_typed` carries the five canonical outputs
(`AW_SYNTHETIC_PR`, `_SKIP`, `_ID`, `_SOURCEBRANCH`, `_TARGETBRANCH`)
as `OutputDecl`s and typed env via `EnvValue::ado_macro` against the
existing allowlist; condition is a typed `And(Succeeded, Ne(Variable, Literal))`.

Marquee regression test (`typed_gate_pr_id_lowers_to_macro_concat_in_same_job`)
builds a Pipeline with synthPr+prGate in the same job, lowers the gate
step, and asserts the emitted env block contains
`$(System.PullRequest.PullRequestId)$(synthPr.AW_SYNTHETIC_PR_ID)` (and
the same for source/target branch) and `AW_SYNTHETIC_PR: $(synthPr.AW_SYNTHETIC_PR)`,
plus the negative assertion that no `variables['synthPr.` runtime-expression
form leaks through. This locks declarative synth-PR propagation.

cargo test 1905/0; cargo clippy --all-targets --all-features clean;
cargo test --test bash_lint_tests 2/2.

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

* feat(extensions): port ExecContextExtension to typed Declarations

The PR contributor now exposes a typed `prepare_step_typed` returning
`Step::Bash` with a typed env block. The synth-active path uses
`EnvValue::Coalesce(vec![AdoMacro("System.PullRequest.X"), StepOutput(synthPr, "AW_SYNTHETIC_PR_X")])`
in place of the hand-written `$[ coalesce(...) ]` strings; lowering
picks the correct cross-job
`dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR_X']` form because
the consumer (Agent job) and producer (Setup job) are in different jobs.

The single-child `Coalesce(vec![StepOutput(synthPr, "AW_SYNTHETIC_PR")])`
on the AW_SYNTHETIC_PR env entry lowers to
`coalesce(<ref>, '')` because the IR lowering pass auto-appends the
trailing `''` (matches the legacy emitter's own hand-rolled trailing
`''` on that one entry).

The synth-inactive path is the typed
`Condition::Eq(Expr::Variable("Build.Reason"), Expr::Literal("PullRequest"))`
plus plain `EnvValue::AdoMacro("System.PullRequest.X")` env values — no
Coalesce, matching today's emitter.

`ContextContributor` trait grows `prepare_step_typed -> Result<Option<Step>>`;
the `Contributor` enum dispatches into the per-trigger variant.
`ExecContextExtension::declarations()` fans out over active
contributors and folds the typed steps into
`Declarations::agent_prepare_steps`. Legacy `prepare_step` /
`prepare_steps` paths remain, additive, until
`compile-target-standalone` switches production callers.

Three new tests:

- `prepare_step_typed_synth_active_carries_typed_coalesce_envs` —
  pattern-matches the typed `EnvValue::Coalesce` / `StepOutput`
  shape for every coalesced env entry and asserts
  `Condition::Succeeded`.
- `prepare_step_typed_synth_inactive_uses_plain_macros_and_narrow_condition` —
  pattern-matches the typed `AdoMacro` envs and the typed
  `Condition::Eq(Variable, Literal)`.
- `exec_context_pr_step_lowers_to_cross_job_dep_form_in_agent_job` —
  marquee end-to-end: builds a Pipeline with `synthPr` in Setup and
  the typed exec-context-pr step in Agent, lowers the Agent step, and
  asserts the wire YAML contains the cross-job
  `dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR_X']` reference
  for all three coalesced env entries — with the negative assertion
  that no `$(synthPr.` macro form leaks into the Agent-job consumer.

cargo test 1908/0; cargo clippy --all-targets --all-features clean;
cargo test --test bash_lint_tests 2/2.

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

* feat(ir): lower parameters / resources / triggers / variables at top level

Extends `ir::lower::lower_with_graph` to emit every top-level
pipeline block the standalone target needs, replacing the
placeholder shapes that only carried `name` + `jobs|stages`.

Inserts in canonical order, each elided when its source data is
empty/unconfigured:
- `parameters:` from `Vec<Parameter>` (Boolean/String/Number kinds
  with typed defaults)
- `resources:` from `Resources { repositories, pipelines }`
  - `RepositoryResource::SelfRepo { clean, submodules }` emits the
    canonical `repository: self` block standalone always uses
  - `RepositoryResource::Named { identifier, kind, name, ref }` emits
    user-declared external repos
  - `PipelineResource` lowers `trigger: true` for any-branch and a
    `trigger.branches.include` mapping otherwise
- `schedules:` from `Vec<Schedule>` with cron + displayName +
  branches.include + always
- `pr:` from `Option<PrTrigger>` — `Some(disabled)` → bare scalar
  `none` (the shape every standalone fixture uses), `Some(filters)` →
  mapping with branches/paths sub-blocks, `None` → no key
- `trigger:` from `Option<CiTrigger>` — same shape policy
- `variables:` from `Vec<PipelineVar>` with isSecret flag

`Triggers::schedule_cron: Option<String>` → `schedules: Vec<Schedule>`
so we can model the displayName + branches that fuzzy_schedule emits.

Nine new unit tests cover each lowering case end-to-end (mapping
shape, key presence, default-elision behaviour).

cargo test 1916/0; tree-green for the standalone-target switchover
that follows.

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

* feat(compile): standalone target builds Pipeline IR; delete base.yml

Replace the string-template `src/data/base.yml` with a typed Pipeline
construction (`src/compile/standalone_ir.rs`) emitted via `ir::emit`.

Changes:
- Add `EnvValue::RawYamlScalar(serde_yaml::Value)` so numeric/boolean
  env values emit unquoted.
- Swap `BashStep`/`TaskStep` env+inputs from `BTreeMap` to `IndexMap`
  to preserve insertion order in the emitted YAML.
- Reorder `lower_bash` / `lower_task` field emission to legacy order.
- `Parameter.values: Vec<serde_yaml::Value>` so `values:` enums emit.
- `build_resources` now produces typed `pipelines:` from `on.pipeline`.
- Wire typed Agent-job condition for filters / synthetic-PR via
  `build_agentic_condition` (mirrors `generate_agentic_depends_on`).
- `start_mcpg_step` injects `-e DEBUG="*"` under `--debug-pipeline`.
- Single-element `dependsOn` lowers as scalar, not sequence.
- Delete `src/data/base.yml` (573 lines).
- Rebaseline all 27 safe-outputs lock fixtures for canonical IR
  field ordering (Task: inputs before displayName; Step: name
  before displayName).
- Update `test_pr_filter_synth_mode_agent_condition_enforces_gate`
  to handle single-line `condition:` form (block-scalar form
  optional).

1ES / target-job / target-stage are out of scope for this commit;
they continue to use `compile_shared` + their own `*-base.yml`
templates.

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

* refactor(compile): extract canonical-jobs builder + extend IR for template targets

Prep work for the stage/job IR migration. No behaviour change for the
standalone target; lock-file output is byte-identical.

- Extract `build_pipeline_context` and `build_canonical_jobs(prefix:
  Option<&str>)` from `build_standalone_pipeline` so stage/job
  compilers can reuse the canonical 5-job graph (Setup?, Agent,
  Detection, SafeOutputs, Teardown?) construction.

- Add `JobPrefix` helper that encapsulates the legacy-template quirk
  that Setup and Teardown jobs stay unprefixed even when other jobs in
  the same target are prefixed by `generate_stage_prefix`. Detection's
  cross-job reference from SafeOutputs is a typed `OutputRef`, so the
  lowering picks up the prefix automatically when the JobId is
  prefixed.

- IR extensions for template targets:
  - `ParameterKind::Object` and `ParameterDefault::Sequence` so the
    auto-injected `dependsOn` template parameter (`type: object`,
    `default: []`) emits correctly.
  - `Parameter::display_name` becomes `Option<String>` so
    auto-injected `dependsOn` / `condition` template params (no UI
    label) don't carry a redundant `displayName: dependsOn` key.
  - `Stage::external_params_wrap` (+ `StageExternalParamsWrap`) —
    when set, `lower_stage` emits `${{ if ne(length(parameters.dependsOn), 0) }}: dependsOn: ${{ parameters.dependsOn }}`
    and the matching condition block; the stage's typed
    `depends_on`/`condition` must be empty when the wrap is active.
  - `Job::template_dependson_wrap` (+ `TemplateDependsOnWrap`) —
    when set, `lower_job` emits dual-branch `${{ if eq(length(parameters.dependsOn), 0) }}` / `${{ if ne(...) }}`
    for `dependsOn` (single internal dep → scalar; multi-dep → list
    with internal then `each d in parameters.X`) and a matching
    condition pair that appends the caller's clause into the internal
    `and(…)` body.

- `ir::lower::lower_with_graph` now handles `PipelineShape::JobTemplate`
  / `StageTemplate` (skip `name:` / `resources:` / triggers — the
  parent pipeline owns those). The OneEs arm still `unimplemented!()`
  until that target migrates.

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

* feat(compile): stage target builds Pipeline IR; delete stage-base.yml

`StageCompiler::compile` now builds a typed `Pipeline { body:
Stages(vec![Stage { … }]), shape: StageTemplate }` via the new
`stage_ir::build_stage_pipeline`, emits via `ir::emit::emit`, and
prepends the same usage-instruction header as before. No
`include_str!("../data/stage-base.yml")` left in `stage.rs`; the
template file is deleted.

The wrap shape matches the deleted `stage-base.yml` template:

- top-level `parameters:` carries the auto-injected `dependsOn`
  (`type: object`, `default: []`) and `condition` (`type: string`,
  `default: ''`) so callers can pass external stage ordering at the
  `- template:` call site.
- single `stages: - stage: <stage_prefix>` wrapping the canonical
  5-job graph; the stage's `external_params_wrap` causes
  `lower_stage` to emit
  `${{ if ne(length(parameters.dependsOn), 0) }}: dependsOn: ${{ parameters.dependsOn }}`
  (plus matching condition block).
- jobs are prefixed: `<stage_prefix>_Agent`, `<stage_prefix>_Detection`,
  `<stage_prefix>_SafeOutputs`. Setup and Teardown stay unprefixed
  (matches legacy `generate_setup_job` / `generate_teardown_job`
  output). SafeOutputs's typed `Condition::Eq(StepOutput(threatAnalysis.SafeToProcess), ...)`
  lowers to the prefixed `dependencies.<stage_prefix>_Detection.outputs[...]`
  form automatically.

Recompiled the three `target: stage` fixtures so the new lock files
are committed alongside the source change:
- `tests/fixtures/stage-agent.lock.yml`
- `tests/fixtures/runtime_imports_stage.lock.yml`
- `tests/fixtures/runtime_imports_author_marker_stage.lock.yml`

`target: job` continues to use the legacy `compile_template_target`
path until the matching job-target commit lands; 1ES is deferred.

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

* feat(compile): job target builds Pipeline IR; delete job-base.yml

`JobCompiler::compile` now builds a typed `Pipeline { body: Jobs(...),
shape: JobTemplate }` via the new `job_ir::build_job_pipeline`, emits
via `ir::emit::emit`, and prepends the same usage-instruction header
as before. No `include_str!("../data/job-base.yml")` left in `job.rs`;
the template file is deleted.

The wrap shape matches the deleted `job-base.yml` template:

- top-level `parameters:` carries the auto-injected `dependsOn`
  (`type: object`, `default: []`) and `condition` (`type: string`,
  `default: ''`) so callers can pass external job ordering at the
  `- template:` call site.
- flat `jobs:` block holding the canonical 5-job graph; the
  `<stage_prefix>_Agent` job carries `template_dependson_wrap` so
  `lower_job` emits dual-branch
  `${{ if eq(length(parameters.dependsOn), 0) }}: dependsOn: Setup`
  / `${{ if ne(...) }}: dependsOn: [Setup, ${{ each d in parameters.dependsOn }}: - ${{ d }}]`
  merging the internal Setup dep with the caller-supplied list,
  plus the matching condition pair that appends
  `${{ parameters.condition }}` into the internal `and(…)` body.
- jobs are prefixed: `<stage_prefix>_Agent`, `<stage_prefix>_Detection`,
  `<stage_prefix>_SafeOutputs`. Setup and Teardown stay unprefixed.

Recompiled the three `target: job` fixtures so the new lock files are
committed alongside the source change:
- `tests/fixtures/job-agent.lock.yml`
- `tests/fixtures/runtime_imports_job.lock.yml`
- `tests/fixtures/runtime_imports_author_marker_job.lock.yml`

`.gitattributes` registers the 6 new committed lock files (3 stage +
3 job) in the managed block so GitHub UI hides them from PR diffs and
the `merge=ours` strategy keeps the local-recompile copy when merge
conflicts arise.

`common::{compile_template_target, TemplateTargetConfig,
generate_template_parameters}` are now unused (only 1ES called them
before; 1ES has its own path via `compile_shared`). Marked with
`#[allow(dead_code)]` to keep the build green; they'll be removed
when 1ES migrates to the IR (or sooner — the `retire-agentic-depends-on`
cleanup commit in `IR_PLAN.md`).

This completes the stage/job IR migration. 1ES is the only remaining
target on the legacy `*-base.yml` template path.

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

* fix(compile): port agent_job_variables hoist to IR; align IR with PR #956+#972 unified AW_PR_* namespace

Brings the IR-based standalone / stage / job target compilers into
parity with the legacy template path after the merge from main
brought in PR #956 (cross-job hoist via Agent-job-level `variables:`)
and PR #972 (unified `AW_PR_*` synthPr output namespace). The IR
path was missing both pieces; the three regression tests
`test_execution_context_pr_emits_prepare_step_and_prompt_supplement`,
`test_pr_filter_synth_mode_gate_step_uses_same_job_synth_ref`,
`test_synthetic_pr_default_emits_full_synth_wiring` were failing on
the IR-based standalone target on `origin/native-ado-compiler` before
this commit (and would also fail on the newly-IR-based stage/job
targets).

## IR changes

- **`Job::variables: Vec<JobVariable>`** with `JobVariable { name,
  value: EnvValue }` — Job-level `variables:` block. The lowering
  pass emits these between `pool:` and `steps:` (the canonical key
  order); each value's `EnvValue` lowers normally, so a
  `Coalesce(StepOutput(<cross-job-step>, <name>))` produces the
  `$[ coalesce(dependencies.<Job>.outputs['<step>.<name>'], '') ]`
  runtime expression — the only form ADO reliably evaluates for
  cross-job output references at variable scope.

- **`SYNTH_PR_OUTPUT_NAMES`** in `ado_script.rs` extended with the
  unified `AW_PR_*` namespace (`AW_PR_ID`, `AW_PR_TARGETBRANCH`,
  `AW_PR_SOURCEBRANCH`, `AW_PR_IS_DRAFT`). The runtime
  `exec-context-pr-synth.js` bundle emits these via both `setOutput`
  (for cross-job OutputRef consumers) and `setVar` (for same-job
  `$(name)` macro consumers). The legacy `AW_SYNTHETIC_PR_*`
  identifier names remain declared for back-compat with code paths
  that still reference them; runtime values are always empty (the
  bundle no longer emits them).

## standalone_ir / canonical-jobs

- New `agent_job_variables_hoist(front_matter)` populates `Agent.variables`
  with `AW_PR_ID` / `AW_PR_TARGETBRANCH` / `AW_PR_SOURCEBRANCH` /
  `AW_SYNTHETIC_PR` via typed `Coalesce(StepOutput(synthPr, name))`.
  Called from `build_agent_job` so all three IR-based targets
  (`standalone`, `stage` via `target: stage`, `job` via `target: job`)
  inherit the hoist when `front_matter.is_synthetic_pr()`.

## Extension updates

- **`ExecContextExtension::prepare_step_typed`** now matches its
  legacy string-form sibling: synth-active path reads the hoisted
  `$(AW_PR_ID)` / `$(AW_PR_TARGETBRANCH)` macros, the bash gate is
  the single `[ -z "$AW_PR_ID" ]` empty-check (replaces the previous
  `BUILD_REASON` + `AW_SYNTHETIC_PR` pair — the merge now happens
  inside `synthPr`), step condition is `succeeded()`. No more
  `BUILD_REASON` / `AW_SYNTHETIC_PR` env projection; the hoisted
  `AW_PR_ID` covers both "real PR" and "synth-promoted" in one var.

- **`filter_ir::build_gate_step_typed`** synth-active branch now
  reads `$(AW_PR_ID)` / `$(AW_PR_SOURCEBRANCH)` / `$(AW_PR_TARGETBRANCH)`
  via `EnvValue::pipeline_var(...)` instead of the previous
  `Concat(AdoMacro, StepOutput)` pattern. The gate step lives in
  the Setup job (same job as `synthPr`), so it reads the setVar-
  emitted variables via the plain `$(name)` macro form; this matches
  the legacy emitter's wire output and the regression test
  `test_pr_filter_synth_mode_gate_step_uses_same_job_synth_ref`.

## Test updates

Three unit tests had assertions pinned to the pre-merge wire form
and now assert the new shape:

- `declarations_setup_steps_typed_with_synthetic_pr_active`: synthPr
  outputs include the unified `AW_PR_*` names plus legacy aliases.
- `typed_gate_pr_id_lowers_to_macro_concat_in_same_job`: env reads
  `$(AW_PR_ID)` / `$(AW_PR_SOURCEBRANCH)` / `$(AW_PR_TARGETBRANCH)`.
- `prepare_step_typed_synth_active_carries_typed_coalesce_envs`:
  env reads `PipelineVar("AW_PR_ID")`; no more `BUILD_REASON` /
  `AW_SYNTHETIC_PR` env projection.
- `exec_context_pr_step_lowers_to_cross_job_dep_form_in_agent_job`
  rewritten end-to-end: the Agent job carries the variables hoist
  (the production layout that `agent_job_variables_hoist` produces);
  the step's env reads the hoisted variables via `$(AW_PR_*)`
  macros; cross-job `dependencies.Setup.outputs[...]` references
  must NOT appear in the step's env (they live only in the
  job-level `variables:` mapping, the only ADO scope that
  evaluates `$[ ... ]` reliably).

## Fixture rebaseline

All six `target: stage|job` fixture lock files updated. Diff is
purely cosmetic: version bump 0.35.0 → 0.35.3 (compiler version
strings in the agent metadata + download URLs). These fixtures
don't exercise the synth-PR path, so the agent-job `variables:`
block remains empty as designed.

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

* feat(compile): 1es target builds Pipeline IR; delete 1es-base.yml

Final compile-target-* commit of the IR PR. All four target compilers (standalone / stage / job / 1es) now build the typed Pipeline IR and emit via ir::lower; no template *-base.yml files remain in src/data/.

PipelineShape::OneEs is now fully implemented (was unimplemented!()): the lowering pass emits a top-level extends: template: v1/1ES.Unofficial.PipelineTemplate.yml@1ESPipelineTemplates block carrying parameters.{pool, sdl, featureFlags, stages: [{ stage: AgentStage, jobs: ... }]}. The 1ESPipelineTemplates repository resource is prepended to resources.repositories by the builder.

Job::template_context: Option<JobTemplateContext> is a new IR field. When set, lower_job suppresses the per-job pool: key (1ES jobs inherit the pool from extends.parameters.pool) and wraps steps: under templateContext: { type: buildJob, outputs: ..., steps: }. Any Step::Publish in the job's steps is lifted into templateContext.outputs[] as { output: pipelineArtifact, path, artifact, condition } so the 1ES template owns the artifact publish.

OneEsSdlConfig is populated with real fields: source_analysis_pool (defaults to AZS-1ES-W-MMS2022 / windows per legacy 1es-base.yml) and feature_flags (disable_network_isolation: true, run_prerequisites_on_image: false). PipelineShape::OneEs carries top_level_pool / stage_id / stage_display_name so the lowering knows what to hoist into the extends wrap.

src/compile/onees_ir.rs (NEW) mirrors stage_ir.rs / job_ir.rs: calls the shared build_pipeline_context to assemble the canonical 5-job graph, tags each job with template_context, prepends the 1ESPipelineTemplates repo, and resolves top_level_pool via common::resolve_pool_typed(CompileTarget::OneES, ...). src/compile/onees.rs collapses to a ~70-line thin entry point matching the standalone/stage/job pattern. src/data/1es-base.yml is deleted (-705 lines).

Cross-job condition references resolve as same-stage (dependencies.<job>.outputs[...]) because the canonical jobs body is PipelineBody::Jobs; passing None as the consumer stage to lower_job keeps consumer/producer locations consistent with the graph's view. The single AgentStage is a purely emission-time wrap, not a graph-level concept.

Net delta: -647 lines (337 added, 984 removed). Validation: cargo build clean / cargo test 1921 passing (-4 = deleted legacy onees.rs generate_setup_job + generate_teardown_job unit tests, now redundant with the canonical-jobs builder) / cargo clippy --all-targets --all-features clean / cargo test --test bash_lint_tests clean (shellcheck still happy on every 1ES bash body) / all 11 _1es integration tests in compiler_tests.rs pass.

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

* refactor(compile): retire legacy YAML-string compile path

Delete the entire generate_*/format_*/compile_shared/compile_template_target
infrastructure that the IR migration superseded:

* common.rs:
  - generate_setup_job, generate_teardown_job, generate_prepare_steps,
    generate_finalize_steps, generate_agentic_depends_on
  - generate_parameters, generate_repositories, generate_checkout_steps,
    generate_checkout_self, generate_pipeline_resources
  - generate_pr_trigger, generate_ci_trigger, generate_schedule
  - generate_template_parameters
  - format_step_yaml/_indented, format_steps_yaml/_indented
  - replace_with_indent
  - generate_job_timeout, generate_agent_job_variables
  - sanitize_filename, yaml_double_quoted, resolve_pool_block
  - generate_debug_pipeline_replacements
  - CompileConfig, TemplateTargetConfig
  - compile_shared, compile_template_target
  - Every cfg(test) helper that only exercised the above
* pr_filters.rs:
  - generate_native_pr_trigger, add_condition_to_steps
  - cfg(test) helpers and tests that exercised the legacy generators
* mod.rs:
  - test_generate_checkout_self_no_branch
* types.rs:
  - FrontMatter::has_schedule (no callers)
* fuzzy_schedule.rs:
  - generate_schedule_yaml (only used by common::generate_schedule)
  - the matching unit tests

All four production compile targets (standalone, 1ES, job, stage) build
from the typed IR (src/compile/ir/ + *_ir.rs), so these helpers had no
production callers. cargo build / cargo test (all 1816 + integration
tests) / cargo clippy --all-targets --all-features / cargo test --test
bash_lint_tests all clean.

Refs IR_PLAN.md \
etire-agentic-depends-on\.

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

* chore(compile): rebaseline ado-aw lock files at v0.35.3

Recompiled every tests/safe-outputs/*.lock.yml with the IR-driven
compile path against the current ado-aw release (v0.35.3). The diff
is purely the version-bump churn (header, downloader URL, displayName
for the install task) — no structural changes, confirming the IR
compile path is byte-identical to the legacy YAML-string compile
path for the spot-check matrix.

cargo test (1816 + integration tests) clean.

Refs IR_PLAN.md lockfile-rebaseline.

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

* refactor(extensions): delete legacy prepare_steps/setup_steps trait methods

Every production caller already consumes typed declarations(), so remove the deprecated YAML-string trait aliases and their enum delegation.

Step::RawYaml remains available for user-authored setup/teardown YAML; this only removes the trait bridge that wrapped extension output in RawYaml.

The remaining static trait accessors continue to populate Declarations for now and will be folded into declarations() in a follow-up commit.

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

* refactor(extensions): fold per-signal accessors into declarations()

Reduce the CompilerExtension trait surface to name, phase, and declarations. Each extension now returns every compile-time signal through Declarations, including validation warnings and errors.

Production callers precompute declarations once per extension and read hosts, bash commands, MCPG entries, allow-tools, pipeline env, AWF mounts, path prepends, agent env vars, prompts, and warnings from that bundle. Engine argument generation and common pipeline helpers now consume those declaration bundles directly.

The Extension enum delegation macro now only forwards name, phase, and declarations. Tests were updated to assert against declarations fields instead of legacy per-signal methods.

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

* docs: replace template-markers.md with ir.md; update extending.md and site docs for typed IR

Delete the obsolete template marker references and remove stale references to deleted *-base.yml template files.

Add docs/ir.md for the typed pipeline IR, graph pass, output references, conditions, lowering, extension declarations, and target IR builders.

Rewrite docs/extending.md and mirror the site guide/reference pages for the typed Declarations and Step-based extension surface. Refresh AGENTS.md, related docs, site sidebar entries, and rustdoc links uncovered by cargo doc.

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

* test(compile): drop template-marker docs-coverage test

The test asserted that every {{ marker }} appearing in src/data/*.yml
also has a matching '## {{ marker }}' heading in docs/template-markers.md.
Both inputs are gone: the four *-base.yml templates were deleted by the
IR migration (no {{ marker }} substitution survives), and
docs/template-markers.md was replaced by docs/ir.md.

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

* chore: remove IR_PLAN.md / IR_DONE.md session-scratch files

These were tracking documents for the IR migration. They were
accidentally tracked by a 'git add -A' during the cleanup commits
(they had been sitting untracked in the working tree). With the
migration complete and merged, the canonical reference is docs/ir.md.

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

* docs: clean up stale Step::RawYaml migration-bridge comments

Several doc comments still referred to the IR migration's intermediate
states (port-* commits, migration bridge, until X lands) even though
the migration is complete and Step::RawYaml is now the permanent
escape hatch for user-authored YAML.

Updated:
- src/compile/ir/step.rs: rewrite the Step::RawYaml doc to describe
  its current role (user-authored YAML escape hatch) and point readers
  at standalone_ir.rs for the producer sites and the no RawYaml from
  generated code rule.
- src/compile/ir/graph.rs: drop the port-* commits framing from the
  graph-pass comment.
- src/compile/standalone_ir.rs: drop the migration-era language
  (port-* commits, compile_shared flow, follow-up commit) in the
  module header, the engine_install_steps_yaml field doc, the Setup-
  job user-step gating comment, and the prompt-supplement RawYaml
  comment.
- src/compile/extensions/exec_context/pr.rs: drop the
  (port-exec-context) tag from a test section header.

No behaviour change.

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

* fix(ir): address code-review nits on output decls, coalesce, and synthPr stubs

Four follow-ups from the post-IR review of native-ado-compiler:

1. Clarify the OutputDecl / auto_is_output contract.
   The original doc-comment in src/compile/ir/output.rs promised the
   compiler auto-rewrites the bash body to add isOutput=true. It does
   not - the IR never introspects step bodies. The flag is an
   informational signal that extension authors must consult and act
   on themselves. Updated output.rs, graph.rs (outputs_needing_is_output
   field doc), and step.rs (BashStep::outputs field doc) to describe
   the real contract: graph pass detects which decls need the flag,
   producer is responsible for emitting it in the vso directive.

   Forgetting isOutput=true on a producer with cross-step consumers
   is now explicitly called out as a silent-failure mode (and the
   PR #956 / PR #975 / synthPr regression history is cited).

2. Implement Coalesce flattening in src/compile/ir/lower.rs.
   The EnvValue::Coalesce doc promised nested Coalesce values flatten
   into a single outer coalesce(...) call. The lowering pass produced
   nested coalesce(coalesce(...)) instead. ADO accepts both, but the
   IR contract now matches behaviour: added flatten_coalesce_into()
   helper used by both lower_env_value and lower_env_value_as_expr_atom
   in their Coalesce arms. New unit test covers the flatten case
   explicitly. No production producer uses nested Coalesce today,
   so lock files are unchanged.

3. Drop dead AW_SYNTHETIC_PR_* legacy declarations.
   src/compile/extensions/ado_script.rs::SYNTH_PR_OUTPUT_NAMES carried
   three entries (AW_SYNTHETIC_PR_ID, AW_SYNTHETIC_PR_SOURCEBRANCH,
   AW_SYNTHETIC_PR_TARGETBRANCH) with a comment claiming filter_ir.rs
   build_gate_step_typed referenced them via OutputRef. filter_ir.rs
   has zero OutputRef usages - it uses EnvValue::pipeline_var. The
   stubs were truly dead. Removed both the entries and the misleading
   comment. Updated the corresponding test in ado_script.rs.

4. Dedupe duplicated paragraph on SYNTH_PR_OUTPUT_NAMES doc-comment.

cargo build / cargo test (1814 unit + integration) / cargo clippy
--all-targets --all-features / cargo test --test bash_lint_tests all
clean.

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

* fix(ir): replace latent panics with typed errors; tighten Declarations dead-code allow

Three follow-ups from review:

1. src/compile/ir/output.rs: lower_outputref now returns Result<String>
   instead of String. The cross-stage branch had .expect() on the
   producer's stage that was load-bearing along the normal
   resolve() -> lower() path (graph::build_graph rejects mixed
   staged/un-staged refs before lowering) but a silent panic for any
   caller bypassing that flow. Returns a typed anyhow::Error with
   full context (step, producer/consumer job + stage) instead.
   Added unit test for the error path. Updated the two callers in
   lower.rs (lower_outputref_for / _for_expr) and condition.rs to
   propagate via ?.

2. src/compile/standalone_ir.rs: wire_explicit_dependencies now
   returns Result<()> and uses ? on prefix.id(...) instead of four
   consecutive .expect() calls. The invariant holds today (jobs were
   built with the same prefix) but the function signature did not
   capture it, so any future caller could trip a silent panic.

3. src/compile/extensions/mod.rs: removed the struct-level
   #[allow(dead_code)] on Declarations and replaced it with per-field
   annotations on the three fields that are actually unused:
   agent_finalize_steps, detection_prepare_steps, safe_outputs_steps.
   Each field's doc-comment now explicitly says "Reserved for future
   use — no extension contributes here today and no compile-target
   reads this field." A struct-level suppression silently absorbs any
   future unused field; per-field annotations force the next dead
   field to be either implemented or explicitly justified.

cargo build / cargo test (1815 + integration) / cargo clippy
--all-targets --all-features / cargo test --test bash_lint_tests all
clean.

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

* security(compile): SHA-derived heredoc sentinels to prevent shell injection

The Detection and Agent jobs each emit a bash heredoc that captures
user-controlled markdown:

  cat > /tmp/awf-tools/agent-prompt.md << "AGENT_PROMPT_EOF"
  <resolved agent body, plus inlined imports when inlined-imports: true>
  AGENT_PROMPT_EOF

  cat > /tmp/awf-tools/threat-analysis-prompt.md << "THREAT_ANALYSIS_EOF"
  <interpolated front_matter.description and other compile-time strings>
  THREAT_ANALYSIS_EOF

The fixed sentinels were a latent shell-injection vector: any agent
markdown body (or front_matter description) containing a line whose
exact content was AGENT_PROMPT_EOF or THREAT_ANALYSIS_EOF would
terminate the heredoc early, and everything after that line would
execute as bash inside the Agent / Detection job - with the agents
secrets in scope.

Replace the fixed sentinels with a SHA-derived form:

  AGENT_PROMPT_EOF_<first-12-hex-chars-of-sha256(content)>

The SHA suffix is per-content and deterministic, so lock files stay
stable across recompiles of the same agent. A 48-bit collision is
~2 x 10^-15 - effectively impossible to forge without inverting
SHA-256. As defense in depth, the new heredoc_sentinel helper in
src/compile/common.rs walks the content and bails with a typed
compile error if it somehow contains the sentinel as a standalone
line.

Both producer steps (prepare_agent_prompt_step,
prepare_threat_analysis_prompt_step) now return Result<BashStep>;
callers in build_agent_job / build_detection_job propagate via ?.

All committed lock files recompiled to pick up the new sentinel
shapes. cargo test (1815 unit + 130 compiler + integration suites)
clean.

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

* fix(ir): address post-merge review nits

Nine fixes from the post-merge IR review.

Bugs:

1. src/compile/ir/lower.rs lower_outputref_for_expr now bails when a
   same-job OutputRef appears inside EnvValue::Coalesce. ADO runtime
   expressions in the producing job cannot read step outputs via
   variables[...] - the previous code emitted that known-broken
   form silently. Extension authors must use EnvValue::Concat
   (macro-form $(step.name)) for same-job consumers instead. Doc
   updated on EnvValue::Coalesce. Test added.

2. src/compile/ir/graph.rs detect_cycles_in replaces .expect() with
   .ok_or_else() returning a typed anyhow::Error. The invariant
   was sound but a future edit could break it and panic in
   production.

3. src/compile/ir/graph.rs build_graph now folds manually-populated
   Job::depends_on and Stage::depends_on into g.job_edges / g.stage_edges
   before detect_cycles_in runs. User-authored cycles that bypass
   OutputRef-derived edges (e.g. A.depends_on=[B], B.depends_on=[A])
   were previously detected only at ADO pipeline-load time. Test
   added covering the no-OutputRef cycle case.

Suggestions:

4. src/compile/ir/lower.rs yaml_value_to_scalar_string now returns
   Result<String> and propagates serde_yaml errors via .context()
   instead of silently emitting an empty env-var value via
   .unwrap_or_default(). Callers in lower_env_value and
   lower_env_value_as_expr_atom propagate.

5. src/compile/ir/{job,stage}.rs TemplateDependsOnWrap and
   StageExternalParamsWrap gain ::new() constructors that validate
   the depends_on_param / condition_param values against the ADO
   parameter-identifier grammar via validate::is_valid_parameter_name.
   These values get embedded into ADO template-expression YAML keys
   so malformed input would produce broken YAML. Producers in
   job_ir.rs / stage_ir.rs updated to propagate. Tests cover the
   rejection path.

6. src/compile/standalone_ir.rs evaluate_threat_analysis_step
   replaces .unwrap() on StepId::new("threatAnalysis") with .expect()
   carrying the rationale.

7. src/compile/ir/graph.rs stage conditions now reject same-stage
   step-output references at graph-build time. A stage condition is
   evaluated before any job in the stage runs, so referencing a
   step output from a sibling job in the same stage would emit
   subtly-wrong YAML. New add_edge_for_stage_condition walks stage
   conditions and bails with a clear error. Existing
   lower::lower_stage stage-condition placeholder-job comment
   updated to describe the validation-pass guarantee. Test covers
   the rejection.

8. src/compile/ir/graph.rs add_edge_for_ref: misleading "add stage
   edge AND surface a cross-job edge" comment rewritten to describe
   what actually happens (only the stage-level dependsOn edge is
   inserted; jobs cannot depend directly on jobs in other stages).

9. src/compile/extensions/exec_context/pr.rs test renamed from
   prepare_step_typed_synth_active_carries_typed_coalesce_envs to
   prepare_step_typed_synth_active_consumes_unified_pipeline_vars
   to match the assertions (the implementation evolved to use
   PipelineVar instead of typed Coalesce, but the test name and
   doc-comment were stale).

Validation: cargo build, cargo test (1820 tests passed, +5 new),
cargo clippy --all-targets --all-features, cargo test --test
bash_lint_tests, cargo run -- compile --force across all 33 fixtures
produces zero lock-file drift - the refactor is semantically
identical at the output layer.

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

* fix(ir): hoist AW_PR_IS_DRAFT, port agent conditions to typed Condition::Ne/Eq, dedupe doc-comment

Four follow-ups from review:

1. src/compile/standalone_ir.rs agent_job_variables_hoist was missing
   AW_PR_IS_DRAFT - it was declared in SYNTH_PR_OUTPUT_NAMES but never
   hoisted into the Agent-job variables block. Any step on the
   synth-from-CI path reading $(AW_PR_IS_DRAFT) via PipelineVar would
   have silently gotten an empty string.

   Refactored to derive the hoist list from a new dedicated constant
   SYNTH_PR_AGENT_HOIST_NAMES in ado_script.rs (sibling of
   SYNTH_PR_OUTPUT_NAMES). A unit test enforces the hoist subset is
   a subset of the declared outputs, so future drift between the two
   lists is a hard test failure rather than a silent runtime
   regression. AW_SYNTHETIC_PR_SKIP is intentionally absent from the
   hoist set (consumed only by the typed Agent-job Condition::Ne over
   the cross-job OutputRef, never as a step env var) - doc comment
   explains.

2. src/compile/standalone_ir.rs build_standalone_pipeline doc-comment
   was duplicated back-to-back from a merge artifact. Deduped.

3. src/compile/standalone_ir.rs build_agentic_condition ported four
   Condition::Custom(hardcoded_string) arms to typed
   Condition::Ne/Eq/Or/And over Expr::Variable / Expr::Literal /
   Expr::StepOutput. The producer step IDs (synthPr, prGate,
   pipelineGate) and their declared outputs are now graph-validated
   at build_graph time; a rename in filter_ir.rs or ado_script.rs
   becomes a compile-time error rather than silently broken runtime
   conditions.

   To make this work, added OutputDecl::new("SHOULD_RUN") to the
   gate step builder in src/compile/filter_ir.rs::build_gate_step_typed
   so the cross-job OutputRef passes graph validation. The bash body
   that emits the vso directive is unchanged.

   Zero lock-file drift across all 33 fixtures - the typed lowering
   produces byte-identical output.

4. src/compile/ir/mod.rs #![allow(dead_code)] comment was stale
   ("removed atomically with the trait port" - the trait port shipped
   ages ago). Rewrote the rationale to describe what the allow
   actually covers today: API surface for constructor helpers
   (Condition::and/or/not, EnvValue::secret/concat, push_step,
   push_job, ...) and the graph::resolve() flow's sub-passes
   (apply_edges, apply_auto_is_output) that are kept for future
   tooling consumers (linters, codemod authors) even though the
   current compile path threads build_graph → detect_cycles → emit
   directly with explicit dependsOn wiring.

Validation: cargo build, cargo test (1821 passed, +1 new), cargo
clippy --all-targets --all-features, cargo test --test
bash_lint_tests, cargo run -- compile --force across all 33 fixtures
produces zero lock-file drift.

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

* fix(ir): robust YAML step splitter, fail loudly on env-block parse, log allowlist bypass

Three follow-ups from review.

1. src/compile/standalone_ir.rs split_yaml_step_sequence was
   blank-line-fragile: two adjacent steps without an intervening blank
   line would merge into a single garbage RawYaml chunk. The current
   callers all happen to emit blank-line separators so there is no
   regression, but the invariant was implicit. Replaced the
   blank-line walker with a serde_yaml::from_str pass that parses
   the input as a YAML sequence of mappings (or a single mapping)
   and re-serialises each item via serde_yaml::to_string. The
   splitter and its caller push_raw_yaml_if_nonempty now return
   Result; six call sites in build_agent_job /
   build_detection_job / build_safe_outputs_job propagate via ?.
   Any future malformed compiler output now bails at compile time
   instead of silently merging. Zero lock-file drift across all 33
   fixtures - the typed re-serialisation produces byte-identical
   output.

2. src/compile/standalone_ir.rs parse_env_block previously absorbed
   parse failures via `Err(_) => return Vec::new()`. Because the
   inputs are compiler-generated from validated front-matter, a
   parse failure here is a compiler bug, but the silent empty-vec
   fallback turned it into a runtime "GITHUB_TOKEN missing" failure
   in the pipeline with no compile-time signal. Replaced with
   `Result<Vec<...>>` returning typed anyhow errors with full
   diagnostic context (parse-failure message, malformed-shape
   message, missing-`env:`-key message). All three callers
   (start_mcpg_step, run_agent_step, execute_safe_outputs_step)
   gain `Result<BashStep>` and propagate via ?.

3. src/compile/filter_ir.rs env_value_from_ado_macro silently fell
   through to EnvValue::Literal when the ADO macro was not in
   ALLOWED_ADO_MACROS. The emitted YAML is byte-identical (ADO
   substitutes the macro at runtime either way), but the bypass
   short-circuited the typed-allowlist validation that
   EnvValue::AdoMacro enforces. Added a log::warn! that surfaces
   the bypass at compile time with the env var name, the macro,
   and the actionable fix (add to ALLOWED_ADO_MACROS). The fallback
   behaviour is unchanged so no lock files move.

Tests: added 8 new unit tests to standalone_ir.rs covering the new
error paths (parse_env_block_bails_on_malformed_yaml,
parse_env_block_bails_when_env_key_is_missing,
parse_env_block_bails_when_top_level_is_not_a_mapping,
parse_env_block_empty_input_is_ok_empty_vec,
parse_env_block_routes_ado_macro_through_pipeline_var,
split_yaml_step_sequence_single_step,
split_yaml_step_sequence_multiple_steps_without_blank_line_separator,
split_yaml_step_sequence_bails_on_invalid_yaml).

Validation: cargo build, cargo test (1829 passed, +8 new), cargo
clippy --all-targets --all-features, cargo test --test
bash_lint_tests, cargo run -- compile --force across all 33 fixtures
produces zero lock-file drift.

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

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: jamesadevine <jamesadevine@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant