fix(compile): gate exec-context-pr synth path in bash, not step condition

src/compile/extensions/exec_context/pr.rs

@@ -126,8 +126,7 @@ impl ContextContributor for PrContextContributor {
         // are emitted using `$[ coalesce(...) ]` so the bundle picks
         // up either the real `System.PullRequest.*` (on a true PR
         // build) OR the synthPr Setup-job output (on a CI build
-        // promoted via exec-context-pr-synth.js). The step's
-        // condition is also broadened to accept synth-promoted builds.
+        // promoted via exec-context-pr-synth.js).
         //
         // Cross-job reference is correct here: this step runs in the
         // **Agent** job (which depends on Setup), so
@@ -147,30 +146,70 @@ impl ContextContributor for PrContextContributor {
         // them unquoted in practice, but double-quoting matches the
         // form shown in ADO docs and is strictly conformant to the
         // YAML spec (which reserves `'` as a scalar indicator).
-        let (pr_id_macro, target_branch_macro, condition) = if self.synthetic_pr_active {
+        //
+        // ## Synth-active gating — bash, not step `condition:`
+        //
+        // ADO step-level `condition:` fields CANNOT reference
+        // `dependencies.<Job>.outputs[...]`. That syntax is only legal
+        // in **job**-level `condition:`, in `variables:` mappings,
+        // and in step-level `env:` values (via `$[ ... ]`). Attempting
+        // to use it in a step condition produces a pipeline-validation
+        // error ("Unrecognized value: 'dependencies'") and the build
+        // fails before the Agent job starts.
+        //
+        // We therefore project the synth flag through the same
+        // step-level `env:` indirection used for the PR id / target
+        // branch and gate in the bash body. The step still emits as
+        // `succeeded` in the ADO UI on non-PR / non-synth builds
+        // (with a single skip log line) rather than as `skipped` —
+        // a minor cosmetic cost for avoiding a cross-cutting
+        // template / trait change.
+        //
+        // The synth-INACTIVE branch is unchanged: its
+        // `condition: eq(variables['Build.Reason'], 'PullRequest')`
+        // only reads `variables[...]`, which IS legal at step level.
+        let (pr_id_macro, target_branch_macro, prelude, condition) = if self.synthetic_pr_active {
             (
                 "\"$[ coalesce(variables['System.PullRequest.PullRequestId'], dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR_ID']) ]\"",
                 "\"$[ coalesce(variables['System.PullRequest.TargetBranch'], dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR_TARGETBRANCH']) ]\"",
-                "or(eq(variables['Build.Reason'], 'PullRequest'), eq(dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR'], 'true'))",
+                // Bash gate. `coalesce(..., '')` on the env-var side
+                // ensures `$AW_SYNTHETIC_PR` is `""` (not an unresolved
+                // `$()` literal) when Setup's `synthPr` step skipped
+                // itself on a real-PR build. Both var refs are quoted
+                // for shellcheck; both args to `[` are literal-string
+                // comparisons so `set -u` is safe on either side.
+                "    if [ \"$BUILD_REASON\" != \"PullRequest\" ] && [ \"$AW_SYNTHETIC_PR\" != \"true\" ]; then\n      echo \"[aw-context] Not a PR build and not synth-promoted; skipping exec-context-pr.\"\n      exit 0\n    fi\n",
+                "succeeded()",
             )
         } else {
             (
                 "$(System.PullRequest.PullRequestId)",
                 "$(System.PullRequest.TargetBranch)",
+                "",
                 "eq(variables['Build.Reason'], 'PullRequest')",
             )
         };
+        // Synth-active path adds two env vars (BUILD_REASON + the
+        // coalesced synth flag) the bash prelude reads. They're
+        // omitted on the synth-inactive path because that path's step
+        // condition (a plain `variables[...]` ref) is legal at step
+        // level and needs no in-bash gate.
+        let synth_env = if self.synthetic_pr_active {
+            "\n    BUILD_REASON: $(Build.Reason)\n    AW_SYNTHETIC_PR: \"$[ coalesce(dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR'], '') ]\""
+        } else {
+            ""
+        };
         format!(
             r#"- bash: |
     set -euo pipefail
-    node '{EXEC_CONTEXT_PR_PATH}'
+{prelude}    node '{EXEC_CONTEXT_PR_PATH}'
   env:
     SYSTEM_ACCESSTOKEN: $(System.AccessToken)
     SYSTEM_PULLREQUEST_PULLREQUESTID: {pr_id_macro}
     SYSTEM_PULLREQUEST_TARGETBRANCH: {target_branch_macro}
     SYSTEM_TEAMPROJECT: $(System.TeamProject)
     BUILD_REPOSITORY_NAME: $(Build.Repository.Name)
-    BUILD_SOURCESDIRECTORY: $(Build.SourcesDirectory)
+    BUILD_SOURCESDIRECTORY: $(Build.SourcesDirectory){synth_env}
   displayName: "Stage PR execution context (aw-context/pr/*)"
   condition: {condition}"#
         )
@@ -221,7 +260,7 @@ mod tests {
     }
 
     #[test]
-    fn prepare_step_synth_active_emits_coalesced_env_and_broadened_condition() {
+    fn prepare_step_synth_active_emits_coalesced_env_and_bash_synth_guard() {
         let contributor = PrContextContributor::new(PrContextConfig::default(), true);
         let fm = pr_fm();
         let ctx = CompileContext::for_test(&fm);
@@ -244,13 +283,54 @@ mod tests {
             "synth-active prepare step must coalesce target branch with synthPr fallback: {step}"
         );
 
-        // Condition: broadened to accept real PR builds OR synth-promoted
-        // CI builds.
+        // Env: BUILD_REASON + synth flag projected through env so the
+        // bash gate has plain `$BUILD_REASON` / `$AW_SYNTHETIC_PR` to
+        // read (cross-job refs are illegal in step `condition:` but
+        // legal in step `env:` values).
+        assert!(
+            step.contains("BUILD_REASON: $(Build.Reason)"),
+            "synth-active prepare step must project Build.Reason through env for the bash guard: {step}"
+        );
+        assert!(
+            step.contains(
+                "AW_SYNTHETIC_PR: \"$[ coalesce(dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR'], '') ]\""
+            ),
+            "synth-active prepare step must project the synth flag through env (coalesced to '' on real-PR builds): {step}"
+        );
+
+        // Bash guard: literal `if` chain that exits 0 when neither
+        // condition holds. Variables are double-quoted so shellcheck
+        // is clean and `set -u` is safe.
+        assert!(
+            step.contains(
+                "if [ \"$BUILD_REASON\" != \"PullRequest\" ] && [ \"$AW_SYNTHETIC_PR\" != \"true\" ]; then"
+            ),
+            "synth-active prepare step must include the bash gate (replaces the illegal step-level condition): {step}"
+        );
         assert!(
             step.contains(
-                "condition: or(eq(variables['Build.Reason'], 'PullRequest'), eq(dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR'], 'true'))"
+                "[aw-context] Not a PR build and not synth-promoted; skipping exec-context-pr."
+            ),
+            "synth-active prepare step must emit a single skip log line so the no-op is discoverable: {step}"
+        );
+
+        // Step condition: must be `succeeded()` (the only legal form
+        // here — cross-job dep refs are illegal at step level).
+        assert!(
+            step.contains("condition: succeeded()"),
+            "synth-active prepare step must use `condition: succeeded()` and gate in bash: {step}"
+        );
+
+        // Regression trap: the v6.x emission put a cross-job ref in
+        // the step `condition:`. ADO rejects that with
+        // "Unrecognized value: 'dependencies'" and the pipeline never
+        // starts the Agent job. Must NEVER come back.
+        assert!(
+            !step.contains(
+                "condition: or(eq(variables['Build.Reason'], 'PullRequest'), eq(dependencies.Setup.outputs"
             ),
-            "synth-active prepare step must broaden the condition to accept synth-promoted builds: {step}"
+            "synth-active prepare step must NOT use the illegal cross-job dep ref in step `condition:` \
+             (only legal in job-level conditions / `variables:` mappings / step `env:` values): {step}"
         );
     }
 

tests/compiler_tests.rs

@@ -5336,9 +5336,27 @@ fn test_execution_context_pr_emits_prepare_step_and_prompt_supplement() {
         compiled.contains("Stage PR execution context (aw-context/pr/*)"),
         "Should emit the PR context prepare step displayName"
     );
+    // The synth-active prepare step uses `condition: succeeded()` and
+    // gates in bash (cross-job `dependencies.Setup.outputs[...]` refs
+    // are ILLEGAL in step-level `condition:` — ADO rejects them with
+    // "Unrecognized value: 'dependencies'"). The synth flag is
+    // projected through step `env:` (legal there) so a tiny bash
+    // prelude can do the gate.
     assert!(
-        compiled.contains("condition: or(eq(variables['Build.Reason'], 'PullRequest'), eq(dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR'], 'true'))"),
-        "Prepare step must be gated on PR builds OR synthetic-PR Setup outputs at the ADO condition layer (mode: synthetic is the default)"
+        compiled.contains(
+            "if [ \"$BUILD_REASON\" != \"PullRequest\" ] && [ \"$AW_SYNTHETIC_PR\" != \"true\" ]; then"
+        ),
+        "Prepare step must include the bash gate that replaces the (illegal) cross-job step condition (mode: synthetic is the default)"
+    );
+    assert!(
+        compiled.contains(
+            "AW_SYNTHETIC_PR: \"$[ coalesce(dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR'], '') ]\""
+        ),
+        "Prepare step must project the synth flag through env (coalesced to '' on real-PR builds) for the bash gate to read"
+    );
+    assert!(
+        compiled.contains("BUILD_REASON: $(Build.Reason)"),
+        "Prepare step must project Build.Reason through env so the bash gate sees a plain $BUILD_REASON"
     );
     assert!(
         compiled.contains("SYSTEM_ACCESSTOKEN: $(System.AccessToken)"),
@@ -5535,6 +5553,113 @@ fn test_execution_context_pr_does_not_leak_system_accesstoken() {
     );
 }
 
+/// Regression trap for an entire bug class: ADO step-level `condition:`
+/// fields MUST NOT reference `dependencies.<Job>.outputs[...]`. That
+/// syntax is only legal in **job**-level conditions, in `variables:`
+/// mappings, and in step-level `env:` values (via `$[ ... ]`). Using
+/// it in a step condition produces a pipeline-validation error
+/// ("Unrecognized value: 'dependencies'") and the build fails before
+/// any job step runs.
+///
+/// This test walks compiled YAML for fixtures exercising both the
+/// synth-active (default-mode) and synth-inactive (no-synth) PR
+/// contributor paths AND a fixture with `mode: synthetic` + explicit
+/// `pr.filters`. If any step's `condition:` value contains the
+/// substring `dependencies.`, the test fails with a pointer to the
+/// offending step. This protects every extension that emits a step
+/// condition, not just `exec_context/pr`.
+///
+/// History: v6.x emitted `condition: or(eq(variables['Build.Reason'],
+/// 'PullRequest'), eq(dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR'],
+/// 'true'))` on the "Stage PR execution context" step. ADO accepted
+/// the YAML at parse time (it IS valid YAML) but rejected the
+/// expression at expand time, breaking every synth-active pipeline.
+#[test]
+fn test_no_step_condition_references_cross_job_dependencies() {
+    use serde_yaml::Value;
+
+    /// Fixtures that should exercise different branches of step
+    /// `condition:` emission (synth-active, synth-inactive, with
+    /// filters, etc.). Add new fixtures here as we add new extensions
+    /// or new step-condition emission paths.
+    const FIXTURES: &[&str] = &[
+        "synthetic-pr-default.md",     // synth-active, no pr.filters
+        "execution-context-agent.md",  // exec_context_pr active, default config
+        "pr-mode-policy.md",           // synth-inactive, on.pr present
+        "minimal-agent.md",            // no on.pr at all
+    ];
+
+    /// Walk the YAML. For every mapping that has a `condition:` key,
+    /// if the value contains `dependencies.`, record the enclosing
+    /// `displayName` (or `job` / `stage` as fallback identifiers) so
+    /// the failure points at a specific step.
+    ///
+    /// Distinguishes step vs job/stage by the presence of sibling
+    /// keys that are step-exclusive (`bash`, `script`, `task`,
+    /// `displayName` is shared, but together with one of the
+    /// step keys it's clearly a step). Jobs/stages have their own
+    /// `condition:` and DO allow cross-job dep refs, so they must
+    /// be filtered OUT.
+    fn walk(v: &Value, hits: &mut Vec<String>) {
+        match v {
+            Value::Mapping(m) => {
+                if let Some(Value::String(cond)) = m.get(Value::String("condition".to_string())) {
+                    if cond.contains("dependencies.") {
+                        // Decide: step or job/stage? A mapping is a STEP if it
+                        // has any of the step-exclusive keys. Otherwise treat
+                        // as job/stage (legal location for the ref) and skip.
+                        let is_step = ["bash", "script", "task", "powershell", "pwsh", "checkout"]
+                            .iter()
+                            .any(|k| m.contains_key(Value::String((*k).to_string())));
+                        if is_step {
+                            let display = m
+                                .get(Value::String("displayName".to_string()))
+                                .and_then(|d| d.as_str())
+                                .unwrap_or("<no displayName>");
+                            hits.push(format!(
+                                "step `{display}` has illegal cross-job dep ref in condition: `{cond}`"
+                            ));
+                        }
+                    }
+                }
+                for (_, vv) in m {
+                    walk(vv, hits);
+                }
+            }
+            Value::Sequence(seq) => {
+                for item in seq {
+                    walk(item, hits);
+                }
+            }
+            _ => {}
+        }
+    }
+
+    for fixture in FIXTURES {
+        let compiled = compile_fixture_with_flags(fixture, &["--skip-integrity"]);
+        let yaml_content: String = compiled
+            .lines()
+            .skip_while(|line| line.starts_with('#') || line.is_empty())
+            .collect::<Vec<_>>()
+            .join("\n");
+        let parsed: Value = serde_yaml::from_str(&yaml_content)
+            .unwrap_or_else(|e| panic!("{fixture}: compiled YAML must parse: {e}"));
+
+        let mut hits: Vec<String> = Vec::new();
+        walk(&parsed, &mut hits);
+
+        assert!(
+            hits.is_empty(),
+            "{fixture}: found {} step(s) with illegal cross-job `dependencies.X.outputs[...]` \
+             refs in `condition:`. ADO rejects these at pipeline-expansion time with \
+             \"Unrecognized value: 'dependencies'\". Project the value through step `env:` \
+             (legal via `$[ ... ]`) and gate in the script body instead. Offenders:\n  - {}",
+            hits.len(),
+            hits.join("\n  - ")
+        );
+    }
+}
+
 /// When the agent is not PR-triggered, the execution-context extension
 /// must NOT emit the PR prepare step.
 #[test]
@@ -5823,12 +5948,22 @@ fn test_synthetic_pr_default_emits_full_synth_wiring() {
         "Fixture A must reference the synth bundle path"
     );
 
-    // Broadened exec-context-pr condition.
+    // Broadened exec-context-pr gate — now a bash guard rather than a
+    // step-level `condition:` (cross-job `dependencies.Setup.outputs[...]`
+    // refs are ILLEGAL in step `condition:`; ADO rejects them with
+    // "Unrecognized value: 'dependencies'"). The synth flag is
+    // projected through step `env:` so the bash prelude can read it.
+    assert!(
+        compiled.contains(
+            "if [ \"$BUILD_REASON\" != \"PullRequest\" ] && [ \"$AW_SYNTHETIC_PR\" != \"true\" ]; then"
+        ),
+        "Fixture A must include the bash gate on exec-context-pr.js (accepts real-PR OR synth-promoted builds)"
+    );
     assert!(
         compiled.contains(
-            "condition: or(eq(variables['Build.Reason'], 'PullRequest'), eq(dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR'], 'true'))"
+            "AW_SYNTHETIC_PR: \"$[ coalesce(dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR'], '') ]\""
         ),
-        "Fixture A must broaden the exec-context-pr.js condition to accept synthetic PRs"
+        "Fixture A must project the synth flag into the exec-context-pr.js step env for the bash gate"
     );
 
     // Agent-job AW_SYNTHETIC_PR_SKIP guard.
@@ -5840,10 +5975,12 @@ fn test_synthetic_pr_default_emits_full_synth_wiring() {
     // Agent-job condition has only the skip guard (no AND-NOT gate
     // clause). The `eq(synthPr.AW_SYNTHETIC_PR, 'true')` literal is
     // therefore expected ONLY in the exec-context-pr.js step's
-    // broadened condition (asserted above) — never as an Agent-job
-    // OR-arm, which would silently bypass the gate for real-PR or
-    // synth-PR builds. A separate fixture covers the gate-enforced
-    // shape when `pr.filters` is present.
+    // env-projected `AW_SYNTHETIC_PR` value (asserted above as the
+    // `coalesce(dependencies.Setup.outputs[...]', '')` shape) — never
+    // as an Agent-job OR-arm in the dependsOn condition, which would
+    // silently bypass the gate for real-PR or synth-PR builds. A
+    // separate fixture covers the gate-enforced shape when
+    // `pr.filters` is present.
 
     // No auto-narrowed CI trigger — `pr.branches.include` lists PR TARGET
     // branches, and ADO `trigger:` fires on pushes TO listed branches, so