From 202104edd8f44a1e5e678c08e386047cb6b7aec5 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Tue, 5 May 2026 14:54:37 +0000
Subject: [PATCH 1/2] fix(compile): remove empty env block from executor step
 when no write permissions

Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/e4d2855e-c0f7-4fc9-8214-5b9942c995dd

Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>
---
 docs/template-markers.md |  4 +--
 src/compile/common.rs    | 11 ++++++--
 src/data/1es-base.yml    |  3 +--
 src/data/base.yml        |  3 +--
 tests/compiler_tests.rs  | 57 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 70 insertions(+), 8 deletions(-)

diff --git a/docs/template-markers.md b/docs/template-markers.md
index e18f7ac3..a5fe6dae 100644
--- a/docs/template-markers.md
+++ b/docs/template-markers.md
@@ -444,9 +444,9 @@ If `permissions.write` is not configured, this marker is replaced with an empty
 
 ## {{ executor_ado_env }}
 
-Generates environment variable entries for the Stage 3 executor step when `permissions.write` is configured. Sets `SYSTEM_ACCESSTOKEN` to the write service connection token (`SC_WRITE_TOKEN`).
+Generates the complete `env:` block (including the `env:` key) for the Stage 3 executor step when `permissions.write` is configured. Sets `SYSTEM_ACCESSTOKEN` to the write service connection token (`SC_WRITE_TOKEN`).
 
-If `permissions.write` is not configured, this marker is replaced with an empty string. Note: `System.AccessToken` is never used directly — all ADO tokens come from explicitly configured service connections.
+If `permissions.write` is not configured, this marker is replaced with an empty string so that no `env:` block is emitted at all. Note: `System.AccessToken` is never used directly — all ADO tokens come from explicitly configured service connections.
 
 ## {{ compiler_version }}
 
diff --git a/src/compile/common.rs b/src/compile/common.rs
index 3e65e7de..6e2767e8 100644
--- a/src/compile/common.rs
+++ b/src/compile/common.rs
@@ -914,7 +914,10 @@ pub fn generate_acquire_ado_token(service_connection: Option<&str>, variable_nam
 /// When not configured, omits ADO access tokens entirely.
 pub fn generate_executor_ado_env(write_service_connection: Option<&str>) -> String {
     match write_service_connection {
-        Some(_) => "SYSTEM_ACCESSTOKEN: $(SC_WRITE_TOKEN)".to_string(),
+        // The two-space indent on the value line is the YAML relative indent for a
+        // key nested under `env:`. replace_with_indent prepends the base indentation
+        // from the marker's position in the template to each subsequent line.
+        Some(_) => "env:\n  SYSTEM_ACCESSTOKEN: $(SC_WRITE_TOKEN)".to_string(),
         None => String::new(),
     }
 }
@@ -3741,6 +3744,10 @@ mod tests {
     #[test]
     fn test_generate_executor_ado_env_with_connection() {
         let result = generate_executor_ado_env(Some("my-sc"));
+        assert!(
+            result.contains("env:"),
+            "Executor env block should include the 'env:' key"
+        );
         assert!(
             result.contains("SYSTEM_ACCESSTOKEN: $(SC_WRITE_TOKEN)"),
             "Executor should use SC_WRITE_TOKEN"
@@ -3756,7 +3763,7 @@ mod tests {
     fn test_generate_executor_ado_env_none_empty() {
         assert!(
             generate_executor_ado_env(None).is_empty(),
-            "None service connection should produce empty env block"
+            "None service connection should produce empty string (no env block)"
         );
     }
 
diff --git a/src/data/1es-base.yml b/src/data/1es-base.yml
index 7b0af5ca..e0407036 100644
--- a/src/data/1es-base.yml
+++ b/src/data/1es-base.yml
@@ -662,8 +662,7 @@ extends:
                     exit $EXIT_CODE
                   displayName: Execute safe outputs (Stage 3)
                   workingDirectory: {{ working_directory }}
-                  env:
-                    {{ executor_ado_env }}
+                  {{ executor_ado_env }}
 
                 - bash: |
                     # Copy all logs to output directory for artifact upload
diff --git a/src/data/base.yml b/src/data/base.yml
index b3437792..24a9ba80 100644
--- a/src/data/base.yml
+++ b/src/data/base.yml
@@ -630,8 +630,7 @@ jobs:
           exit $EXIT_CODE
         displayName: Execute safe outputs (Stage 3)
         workingDirectory: {{ working_directory }}
-        env:
-          {{ executor_ado_env }}
+        {{ executor_ado_env }}
 
       - bash: |
           # Copy all logs to output directory for artifact upload
diff --git a/tests/compiler_tests.rs b/tests/compiler_tests.rs
index b66f380d..d00fc4df 100644
--- a/tests/compiler_tests.rs
+++ b/tests/compiler_tests.rs
@@ -3481,3 +3481,60 @@ fn test_pr_filter_gate_steps_nested_in_setup_job() {
     });
     assert!(has_download, "Download step should be inside Setup job's steps list");
 }
+
+/// Test that a pipeline without `permissions.write` does not emit a bare `env:` block
+/// on the "Execute safe outputs" step (i.e. no invalid empty-mapping YAML).
+#[test]
+fn test_executor_step_no_empty_env_block_without_write_permissions() {
+    // minimal-agent.md has no permissions.write — executor env must be absent
+    let compiled = compile_fixture("minimal-agent.md");
+    assert_valid_yaml(&compiled, "minimal-agent.md");
+
+    // Parse structurally: serde_yaml rejects `env:` with no children in strict mode
+    // and the assert_valid_yaml call above already exercises this.
+    // Additionally verify the executor step contains no `env:` key at all.
+    let execute_block_start = compiled
+        .find("Execute safe outputs (Stage 3)")
+        .expect("Should have executor step");
+    // Find the next step after the executor step (to bound our search window)
+    let after_execute = &compiled[execute_block_start..];
+    let next_step_offset = after_execute[1..]
+        .find("- bash:")
+        .map(|i| i + 1)
+        .unwrap_or(after_execute.len());
+    let executor_step_text = &after_execute[..next_step_offset];
+
+    assert!(
+        !executor_step_text.contains("env:"),
+        "Executor step should not contain an 'env:' block when write permissions are absent: {executor_step_text}"
+    );
+}
+
+/// Test that a pipeline with `permissions.write` emits a correctly-indented `env:` block
+/// on the "Execute safe outputs" step.
+#[test]
+fn test_executor_step_has_env_block_with_write_permissions() {
+    // complete-agent.md has permissions.write configured
+    let compiled = compile_fixture("complete-agent.md");
+    assert_valid_yaml(&compiled, "complete-agent.md");
+
+    assert!(
+        compiled.contains("SYSTEM_ACCESSTOKEN: $(SC_WRITE_TOKEN)"),
+        "Executor step should have SYSTEM_ACCESSTOKEN when write permissions are configured: {compiled}"
+    );
+
+    // Verify the env key and value appear in the correct block by checking both
+    // are present in the same neighbourhood.
+    let execute_block_start = compiled
+        .find("Execute safe outputs (Stage 3)")
+        .expect("Should have executor step");
+    let after_execute = &compiled[execute_block_start..];
+    assert!(
+        after_execute.contains("env:"),
+        "Executor step should contain 'env:' key after the displayName: {after_execute}"
+    );
+    assert!(
+        after_execute.contains("SYSTEM_ACCESSTOKEN: $(SC_WRITE_TOKEN)"),
+        "Executor step should include SYSTEM_ACCESSTOKEN: {after_execute}"
+    );
+}

From 69ab847d84fb5f30dd423f848507031741a80f65 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Tue, 5 May 2026 15:12:54 +0000
Subject: [PATCH 2/2] fix(test): correct misleading comment about serde_yaml
 and bare env: validity

Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/42b1fb99-3470-483e-acc3-99d1f8299528

Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>
---
 tests/compiler_tests.rs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/compiler_tests.rs b/tests/compiler_tests.rs
index d00fc4df..05ea4fad 100644
--- a/tests/compiler_tests.rs
+++ b/tests/compiler_tests.rs
@@ -3490,9 +3490,9 @@ fn test_executor_step_no_empty_env_block_without_write_permissions() {
     let compiled = compile_fixture("minimal-agent.md");
     assert_valid_yaml(&compiled, "minimal-agent.md");
 
-    // Parse structurally: serde_yaml rejects `env:` with no children in strict mode
-    // and the assert_valid_yaml call above already exercises this.
-    // Additionally verify the executor step contains no `env:` key at all.
+    // Verify the executor step contains no `env:` key at all.
+    // Note: a bare `env:` with no children is valid YAML (parsed as null), so
+    // assert_valid_yaml alone would not catch the regression this test guards against.
     let execute_block_start = compiled
         .find("Execute safe outputs (Stage 3)")
         .expect("Should have executor step");