fix(compile): repair synthetic-PR gate + Stage step env var plumbing

[jamesadevine]

fix(compile): repair synthetic-PR gate + Stage step env var plumbing

Two interlocking ADO behaviours were silently collapsing the synthetic-PR
path on CI-triggered builds:

1. Same-job gate step (Setup/prGate): env values used macro
   concatenation $(System.PullRequest.PullRequestId)$(synthPr.AW_SYNTHETIC_PR_ID).
   On non-PR builds ADO leaves undefined predefined macros as the
   literal string "$(System.PullRequest.PullRequestId)", so the
   concatenated value parsed as NaN in gate.js's pr_metadata fact —
   the check failed with "Missing ADO env vars …", pr_is_draft was
   skipped, and SHOULD_RUN defaulted to true (gate bypassed real
   evaluation). Switched to runtime expressions
   $[ coalesce(variables['System.PullRequest.X'], variables['AW_SYNTHETIC_PR_X']) ],
   which silently substitute empty for undefined vars. To make the
   right-hand side readable as a regular variable, exec-context-pr-synth
   now emits each AW_SYNTHETIC_PR* via both setOutput (cross-job)
   AND setVar (same-job, new helper in shared/vso-logger).

2. Cross-job Stage step (Agent/"Stage PR execution context"): env
   values referenced $[ coalesce(dependencies.Setup.outputs['synthPr.X'], '') ]
   at step-env scope. Empirically (msazuresphere/4x4 build #612290) this
   form resolves to the empty string even when the same expression works
   at job-condition scope, causing the bash guard to short-circuit on
   a synth-promoted build and the agent to emit noop. Switched to the
   documented safe pattern: hoist via a new {{ agent_job_variables }}
   marker (Agent-job-level variables: block) that pulls
   dependencies.Setup.outputs[...] at JOB scope, then consume from
   step-env via the $(name) macro / variables['name'] runtime
   expression. Job-level variables are the documented safe location
   for cross-job output references.

Validates against build #612290 (synthetic PR matched #38551 but agent
emitted noop). All 281 bundle tests + 1815+ Rust tests + clippy pass.

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

[github-actions]

🔍 Rust PR Review

Summary: Looks good — the fix correctly addresses the two ADO behavioral constraints. Design is sound and well-documented.

Findings

⚠️ Suggestions

• src/compile/common.rs:1096 — AW_SYNTHETIC_PR_IS_DRAFT intentionally excluded from Agent-job hoist with no comment. exec-context-pr-synth/index.ts now calls emitBoth for all 5 AW_SYNTHETIC_PR* variables including IS_DRAFT, but generate_agent_job_variables only hoists 4. No current Agent-job step consumes IS_DRAFT so this is correct — but it's a latent footgun: if a future step tries to read $(AW_SYNTHETIC_PR_IS_DRAFT) in the Agent job it will silently get the empty string. A one-line comment (e.g. // IS_DRAFT is intentionally not hoisted — no current Agent-job consumer) would prevent confusion.

• tests/compiler_tests.rs:~5687 — negative assertion under-specifies the constraint. The guard !stage_step.contains("dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR']") only covers the unqualified AW_SYNTHETIC_PR variant; the substring 'synthPr.AW_SYNTHETIC_PR'] doesn't appear in the _ID or _TARGETBRANCH forms (those have _ID' / _TARGETBRANCH' after the name so the '] suffix doesn't match). The code correctly removed those direct step-env references too — adding sibling asserts for synthPr.AW_SYNTHETIC_PR_ID'] and synthPr.AW_SYNTHETIC_PR_TARGETBRANCH'] would make the regression fence complete.

✅ What Looks Good

• Dual-emit strategy is correct: setOutput + setVar in exec-context-pr-synth/index.ts properly satisfies both cross-job consumers (Agent job condition: / variables: hoist) and same-job consumers (gate step in Setup job). The vso-logger doc-comment explains the ADO contract clearly.
• escapeProperty in vso-logger.ts escapes ], ;, and = — synthetic PR values (branch names, numeric ID) cannot break out of the ##vso[task.setvariable ...] command header even on adversarial branch names.
• coalesce(..., '') pattern: runtime expressions substitute empty for undefined predefined variables (unlike macros, which leave the literal $(...) string), so the gate step will never see NaN input again on non-PR CI builds.
• Job-level variable hoist is the documented ADO pattern and the doc-comment in generate_agent_job_variables explicitly links the Microsoft reference and the empirical failure evidence (build #612290), making the rationale durable.
• All four base templates (base.yml, 1es-base.yml, job-base.yml, stage-base.yml) receive the {{ agent_job_variables }} marker, and replace_with_indent handles the empty-string case cleanly (no stray keys on non-synth pipelines).

Generated by Rust PR Reviewer for issue #956 · sonnet46 2.4M · ◷