Skip to content

fix(compile): use macro form for same-job synthPr gate refs #944

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jamesadevine merged 2 commits into from
Jun 10, 2026
Merged

fix(compile): use macro form for same-job synthPr gate refs #944

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ed40a83
Initial plan
Copilot Jun 10, 2026
ffb958e
fix(compile): use macro form for same-job synthPr gate refs
Copilot Jun 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 31 additions & 20 deletions src/compile/filter_ir.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1130,14 +1130,20 @@ pub fn build_gate_spec(ctx: GateContext, checks: &[FilterCheck]) -> anyhow::Resu
/// **Same-job vs cross-job reference**: this gate step lives in the
/// **Setup job** (`AdoScriptExtension::setup_steps` returns it), the
/// same job as `synthPr`. Within the producing job, the cross-job form
/// `dependencies.Setup.outputs['synthPr.X']` is undefined (a job has
/// no entry for itself in `dependencies`), so we use the same-job
/// runtime expression `variables['synthPr.X']` instead, which resolves
/// step output variables added to the job's variable scope by prior
/// `isOutput=true` setvariable commands. See
/// `dependencies.Setup.outputs['synthPr.X']` is undefined (a job has no
/// entry for itself in `dependencies`), and — critically — the same-job
/// *runtime expression* form `$[ variables['synthPr.X'] ]` also resolves
/// to **empty**: step output variables are NOT exposed to runtime
/// expressions in the producing job. They are only reachable via the
/// **macro** form `$(synthPr.X)`, which expands them from the job's
/// output-variable namespace (empty when the producing step was skipped
/// or never set the output). We therefore reference synth outputs with
/// macros and rely on macro concatenation: real `System.PullRequest.*`
/// and synth `synthPr.*` are mutually exclusive (synthPr only runs on
/// non-PR builds; `System.PullRequest.*` is empty on non-PR builds), so
/// `$(System.PullRequest.X)$(synthPr.X)` yields exactly one non-empty
/// value. See
/// <https://learn.microsoft.com/en-us/azure/devops/pipelines/process/variables#use-output-variables-from-tasks>.
/// Runtime expressions (`$[ ... ]`) are valid in step-level `env:`
/// blocks per the same docs.
pub fn compile_gate_step_external(
ctx: GateContext,
checks: &[FilterCheck],
Expand Down Expand Up @@ -1172,31 +1178,36 @@ pub fn compile_gate_step_external(
// has been promoted to PR semantics, so the "not a PullRequest
// build" bypass must not auto-pass. Always safe to emit (the gate
// checks it strictly for the literal "true"), but only meaningful
// for PR gates. Same-job ref via `variables['synthPr.X']` — see
// function doc-comment for why this is NOT `dependencies.Setup...`.
// for PR gates. Same-job ref via the macro `$(synthPr.X)` — see
// function doc-comment for why this is NOT `variables['synthPr.X']`
// (resolves empty) nor `dependencies.Setup.outputs[...]` (undefined
// in the producing job).
let pr_synth_active = synthetic_pr_active && matches!(ctx, GateContext::PullRequest);
if pr_synth_active {
// YAML-quote runtime expressions whose value contains single quotes.
// Per ADO docs, `$[ ... ]` runtime expressions are valid in step
// `env:` blocks; wrapping in double quotes keeps the value
// strictly conformant to the YAML spec (which reserves `'` as a
// scalar indicator) and matches the form shown in ADO docs.
step.push_str(
" AW_SYNTHETIC_PR: \"$[ coalesce(variables['synthPr.AW_SYNTHETIC_PR'], '') ]\"\n",
);
// Macro form: `$(synthPr.AW_SYNTHETIC_PR)` expands to "true" when
// the same-job `synthPr` step matched a PR, and to empty when it
// was skipped (real PR build) or found no PR — never the literal
// "true", so `gate/bypass.ts`'s strict `=== "true"` check stays
// correct in every case.
step.push_str(" AW_SYNTHETIC_PR: \"$(synthPr.AW_SYNTHETIC_PR)\"\n");
}

for (env_var, ado_macro) in &exports {
let macro_str = if pr_synth_active {
// Mutually-exclusive macro concatenation: on a real PR build
// `System.PullRequest.*` holds the value and `synthPr.*` is
// empty (step skipped); on a synth-promoted CI build
// `System.PullRequest.*` is empty and `synthPr.*` holds the
// value. Exactly one side is ever non-empty.
match *env_var {
"ADO_PR_ID" => {
"\"$[ coalesce(variables['System.PullRequest.PullRequestId'], variables['synthPr.AW_SYNTHETIC_PR_ID']) ]\""
"\"$(System.PullRequest.PullRequestId)$(synthPr.AW_SYNTHETIC_PR_ID)\""
}
"ADO_SOURCE_BRANCH" => {
"\"$[ coalesce(variables['System.PullRequest.SourceBranch'], variables['synthPr.AW_SYNTHETIC_PR_SOURCEBRANCH']) ]\""
"\"$(System.PullRequest.SourceBranch)$(synthPr.AW_SYNTHETIC_PR_SOURCEBRANCH)\""
}
"ADO_TARGET_BRANCH" => {
"\"$[ coalesce(variables['System.PullRequest.TargetBranch'], variables['synthPr.AW_SYNTHETIC_PR_TARGETBRANCH']) ]\""
"\"$(System.PullRequest.TargetBranch)$(synthPr.AW_SYNTHETIC_PR_TARGETBRANCH)\""
}
_ => ado_macro,
}
Expand Down
64 changes: 36 additions & 28 deletions tests/compiler_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4452,49 +4452,51 @@ fn test_pr_filter_synth_mode_agent_condition_enforces_gate() {

/// Regression guard for the synth-mode gate-step same-job ref bug: the
/// gate step lives in the **Setup** job (same job as `synthPr`), so its
/// env block must use `variables['synthPr.X']` (same-job runtime
/// expression) — `dependencies.Setup.outputs[...]` is undefined inside
/// the producing job and silently coalesces to empty, leaving
/// `AW_SYNTHETIC_PR` empty and causing the bypass to misfire.
/// env block must reference the synth outputs with the **macro** form
/// `$(synthPr.X)`. The same-job runtime-expression form
/// `$[ variables['synthPr.X'] ]` resolves to empty (step outputs are not
/// exposed to runtime expressions in the producing job), and the
/// cross-job form `dependencies.Setup.outputs[...]` is undefined inside
/// the producing job — both silently coalesce to empty, leaving
/// `AW_SYNTHETIC_PR` empty and causing the gate bypass to misfire.
#[test]
fn test_pr_filter_synth_mode_gate_step_uses_same_job_synth_ref() {
let compiled = compile_fixture("pr-filter-tier1-agent.md");

assert!(
compiled.contains(
"AW_SYNTHETIC_PR: \"$[ coalesce(variables['synthPr.AW_SYNTHETIC_PR'], '') ]\""
),
"Gate step env must use same-job `variables['synthPr.X']` runtime expression — \
`dependencies.Setup.outputs[...]` is undefined inside the producing Setup job"
compiled.contains("AW_SYNTHETIC_PR: \"$(synthPr.AW_SYNTHETIC_PR)\""),
"Gate step env must reference the same-job `synthPr` output via the macro \
`$(synthPr.AW_SYNTHETIC_PR)` — the `$[ variables['synthPr.X'] ]` runtime \
expression resolves to empty inside the producing Setup job"
);
// The fixture exercises source-branch and target-branch filters,
// so the synth-coalesce treatment must appear on those env vars
// using the same-job `variables[...]` form. (ADO_PR_ID is not
// so the synth treatment must appear on those env vars using the
// mutually-exclusive macro-concatenation form. (ADO_PR_ID is not
// exported by this fixture's filter set, so we don't assert it here.)
assert!(
compiled.contains(
"ADO_SOURCE_BRANCH: \"$[ coalesce(variables['System.PullRequest.SourceBranch'], variables['synthPr.AW_SYNTHETIC_PR_SOURCEBRANCH']) ]\""
"ADO_SOURCE_BRANCH: \"$(System.PullRequest.SourceBranch)$(synthPr.AW_SYNTHETIC_PR_SOURCEBRANCH)\""
),
"ADO_SOURCE_BRANCH coalesce must use same-job `variables[...]` form"
"ADO_SOURCE_BRANCH must concatenate the real `System.PullRequest.*` macro \
with the same-job `synthPr.*` macro"
);
assert!(
compiled.contains(
"ADO_TARGET_BRANCH: \"$[ coalesce(variables['System.PullRequest.TargetBranch'], variables['synthPr.AW_SYNTHETIC_PR_TARGETBRANCH']) ]\""
"ADO_TARGET_BRANCH: \"$(System.PullRequest.TargetBranch)$(synthPr.AW_SYNTHETIC_PR_TARGETBRANCH)\""
),
"ADO_TARGET_BRANCH coalesce must use same-job `variables[...]` form"
);

// The same-job gate step MUST NOT use the cross-job
// `dependencies.Setup.outputs[...]` form for synthPr references.
// (It's fine elsewhere — e.g. the Agent-job dependsOn condition — but
// not inside the Setup job's own steps.)
// The same-job gate step MUST NOT use the cross-job
// `dependencies.Setup.outputs[...]` form for synthPr references.
// (It's fine elsewhere — e.g. the Agent-job dependsOn condition — but
// not inside the Setup job's own steps.) Bound the gate-step
// section by the start of the next top-level job (`\n - job: `),
// since extract_job_block's `\n- job: ` boundary doesn't match the
// 2-space-indented job list items produced for this target.
"ADO_TARGET_BRANCH must concatenate the real `System.PullRequest.*` macro \
with the same-job `synthPr.*` macro"
);

// The same-job gate step MUST NOT use the broken same-job runtime
// expression form `variables['synthPr.X']` (resolves empty) nor the
// cross-job `dependencies.Setup.outputs[...]` form (undefined in the
// producing job) for synthPr references. (Both are fine elsewhere —
// e.g. the Agent-job dependsOn condition — but not inside the Setup
// job's own steps.) Bound the gate-step section by the start of the
// next top-level job (`\n - job: `), since extract_job_block's
// `\n- job: ` boundary doesn't match the 2-space-indented job list
// items produced for this target.
let setup_block = extract_job_block(&compiled, "Setup").expect("Setup job present");
let gate_section = setup_block
.split("name: prGate")
Expand All @@ -4520,6 +4522,12 @@ fn test_pr_filter_synth_mode_gate_step_uses_same_job_synth_ref() {
that is cross-job syntax and is undefined within the producing job. \
Gate section: {gate_section}"
);
assert!(
!gate_section.contains("variables['synthPr."),
"Gate step (inside Setup job) must NOT reference `variables['synthPr.X']` — \
step outputs are not exposed to runtime expressions in the producing job \
and resolve to empty. Gate section: {gate_section}"
);
}

/// Native ADO PR trigger block is emitted for branch/path filters.
Expand Down
Loading