fix(compile): implement review suggestions for job/stage template targets

Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/fa2ca612-1703-4107-87c3-2b2ec35429b8

Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>

Author: Copilot

docs/template-markers.md

@@ -8,6 +8,8 @@ The compiler transforms the input into valid Azure DevOps pipeline YAML based on
 
 - **Standalone**: Uses `src/data/base.yml`
 - **1ES**: Uses `src/data/1es-base.yml`
+- **Job template**: Uses `src/data/job-base.yml`
+- **Stage template**: Uses `src/data/stage-base.yml`
 
 Explicit markings are embedded in these templates that the compiler is allowed to replace e.g. `{{ engine_run }}` denotes the full engine invocation command. The compiler should not replace sections denoted by `${{ some content }}`. What follows is a mapping of markings to responsibilities (primarily for the standalone template).
 
@@ -495,3 +497,54 @@ Should be replaced with the domain the AWF-sandboxed agent uses to reach MCPG on
 The 1ES target uses the same template markers as standalone, plus the 1ES-specific `extends:` / `stages:` / `templateContext` wrapping. The 1ES template includes `templateContext.type: buildJob` for all jobs, and the pool is specified at the top-level `parameters.pool` rather than per-job.
 
 Both targets share the same execution model (Copilot CLI + AWF + MCPG) and the same set of template markers.
+
+## Job/Stage Template Markers
+
+The `target: job` and `target: stage` targets use `job-base.yml` and `stage-base.yml`
+respectively. Both include all the standard AWF/MCPG markers above, plus the two
+template-specific markers below.
+
+### {{ stage_prefix }}
+
+Replaced with a PascalCase ADO-safe identifier derived from the agent `name:` front
+matter field. Used to prefix the three job names so that including multiple templates
+in the same pipeline produces unique job identifiers.
+
+Derivation rules:
+
+- Non-ASCII-alphanumeric characters are treated as word separators (they are not
+  included in the output).
+- Each word is capitalised and the words are concatenated: `"daily code review"` →
+  `"DailyCodeReview"`.
+- An empty result (all characters stripped) falls back to `"Agent"`.
+- A result starting with a digit is prefixed with `_`: `"123start"` → `"_123start"`.
+- Names containing non-ASCII alphanumeric characters (e.g. `"über-agent"`) produce a
+  compiler warning because those characters are silently dropped.
+
+Example job names produced for `name: Daily Code Review`:
+
+```yaml
+jobs:
+  - job: DailyCodeReview_Agent
+  - job: DailyCodeReview_Detection
+    dependsOn: DailyCodeReview_Agent
+  - job: DailyCodeReview_Execution
+    dependsOn: [DailyCodeReview_Agent, DailyCodeReview_Detection]
+```
+
+### {{ template_parameters }}
+
+Replaced with the `parameters:` block that callers pass when including the template.
+Contains `clearMemory` (auto-injected when `tools.cache-memory` is configured) and any
+user-defined `parameters:` from front matter. Replaced with an empty string when no
+parameters are needed.
+
+Example output when `tools.cache-memory` is configured:
+
+```yaml
+parameters:
+- name: clearMemory
+  displayName: Clear agent memory
+  type: boolean
+  default: false
+```

src/compile/common.rs

@@ -1010,6 +1010,19 @@ pub const DEFAULT_POOL: &str = "AZS-1ES-L-MMS-ubuntu-22.04";
 /// - `"123start"` → `"_123start"` (prefix underscore for leading digit)
 /// - `"über-agent"` → `"BerAgent"` (non-ASCII stripped; ADO requires `[A-Za-z0-9_]`)
 pub fn generate_stage_prefix(name: &str) -> String {
+    // Warn if any Unicode alphanumeric characters are present — they will be
+    // treated as word-separator boundaries and stripped from the output, which
+    // may surprise users whose agent name starts with a non-ASCII letter.
+    if name.chars().any(|c| c.is_alphanumeric() && !c.is_ascii_alphanumeric()) {
+        log::warn!(
+            "Agent name '{}' contains non-ASCII alphanumeric characters; \
+             these are dropped from the ADO job-name prefix because ADO identifiers \
+             require [A-Za-z0-9_]. Rename the agent to use ASCII characters only \
+             if the prefix is important.",
+            name
+        );
+    }
+
     let pascal: String = name
         .split(|c: char| !c.is_ascii_alphanumeric())
         .filter(|s| !s.is_empty())
@@ -2483,6 +2496,19 @@ pub struct CompileConfig {
     pub skip_header: bool,
 }
 
+/// Input configuration for [`compile_template_target`].
+///
+/// Groups the template-specific settings so that the function stays within
+/// the seven-argument limit while remaining easy to extend.
+pub struct TemplateTargetConfig<'a> {
+    /// Raw YAML template string (e.g. `job-base.yml` or `stage-base.yml`).
+    pub template: &'a str,
+    /// When true, the "Verify pipeline integrity" step is omitted.
+    pub skip_integrity: bool,
+    /// When true, MCPG debug diagnostics are included in the generated pipeline.
+    pub debug_pipeline: bool,
+}
+
 /// Shared compilation flow used by both standalone and 1ES compilers.
 ///
 /// This function handles the common pipeline compilation steps:
@@ -2771,6 +2797,85 @@ pub async fn compile_shared(
     }
 }
 
+/// Shared compilation flow for template-producing compilers (`target: job` and
+/// `target: stage`).
+///
+/// Handles the full setup — collecting extensions, building the compile context,
+/// generating the stage prefix and template parameters, computing AWF/MCPG
+/// values — and delegates to [`compile_shared`].  The caller supplies:
+///
+/// - `cfg`: target-specific settings (template string, integrity / debug flags).
+/// - `header_fn`: a function that generates the leading comment block prepended
+///   to the compiled YAML.  The two template compilers use different header
+///   layouts, so this lets each compiler keep its own generator while sharing
+///   all of the boilerplate setup.
+///
+/// Returns the final YAML string with the header prepended.
+pub async fn compile_template_target(
+    input_path: &Path,
+    output_path: &Path,
+    front_matter: &FrontMatter,
+    markdown_body: &str,
+    cfg: TemplateTargetConfig<'_>,
+    header_fn: impl FnOnce(&Path, &FrontMatter) -> String,
+) -> Result<String> {
+    // Collect extensions (needed before compile_shared for MCPG config)
+    let extensions = super::extensions::collect_extensions(front_matter);
+
+    // Build compile context for MCPG config generation
+    let input_dir = input_path.parent().unwrap_or(Path::new("."));
+    let ctx = CompileContext::new(front_matter, input_dir).await?;
+
+    // Generate stage prefix for job-name uniqueness and template parameters
+    let stage_prefix = generate_stage_prefix(&front_matter.name);
+    let template_params = generate_template_parameters(front_matter)?;
+
+    // AWF / MCPG values (same as standalone)
+    let allowed_domains = generate_allowed_domains(front_matter, &extensions)?;
+    let awf_mounts = generate_awf_mounts(&extensions);
+    let awf_paths = collect_awf_path_prepends(&extensions);
+    let awf_path_step = generate_awf_path_step(&awf_paths);
+    let enabled_tools_args = generate_enabled_tools_args(front_matter);
+
+    let config_obj = generate_mcpg_config(front_matter, &ctx, &extensions)?;
+    let mcpg_config_json =
+        serde_json::to_string_pretty(&config_obj).context("Failed to serialize MCPG config")?;
+    let mcpg_docker_env = generate_mcpg_docker_env(front_matter, &extensions);
+    let mcpg_step_env = generate_mcpg_step_env(&extensions);
+
+    let config = CompileConfig {
+        template: cfg.template.to_string(),
+        extra_replacements: vec![
+            ("{{ stage_prefix }}".into(), stage_prefix),
+            ("{{ template_parameters }}".into(), template_params),
+            ("{{ firewall_version }}".into(), AWF_VERSION.into()),
+            ("{{ mcpg_version }}".into(), MCPG_VERSION.into()),
+            ("{{ mcpg_image }}".into(), MCPG_IMAGE.into()),
+            ("{{ mcpg_port }}".into(), MCPG_PORT.to_string()),
+            ("{{ mcpg_domain }}".into(), MCPG_DOMAIN.into()),
+            ("{{ allowed_domains }}".into(), allowed_domains),
+            ("{{ awf_mounts }}".into(), awf_mounts),
+            ("{{ awf_path_step }}".into(), awf_path_step),
+            ("{{ enabled_tools_args }}".into(), enabled_tools_args),
+            ("{{ mcpg_config }}".into(), mcpg_config_json),
+            ("{{ mcpg_docker_env }}".into(), mcpg_docker_env),
+            ("{{ mcpg_step_env }}".into(), mcpg_step_env),
+        ],
+        skip_integrity: cfg.skip_integrity,
+        debug_pipeline: cfg.debug_pipeline,
+        has_awf_paths: !awf_paths.is_empty(),
+        skip_header: true,
+    };
+
+    let yaml = compile_shared(
+        input_path, output_path, front_matter, markdown_body,
+        &extensions, &ctx, config,
+    ).await?;
+
+    let header = header_fn(input_path, front_matter);
+    Ok(format!("{}{}", header, yaml))
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

src/compile/job.rs

@@ -7,22 +7,14 @@
 //! - Directly in a flat pipeline's `jobs:` list
 //! - Inside a user-defined stage's `jobs:` list
 
-use anyhow::{Context, Result};
+use anyhow::Result;
 use async_trait::async_trait;
-use log::{info, warn};
+use log::warn;
 use std::path::Path;
 
 use super::Compiler;
 use super::common::{
-    AWF_VERSION, MCPG_VERSION, MCPG_IMAGE, MCPG_PORT, MCPG_DOMAIN,
-    CompileConfig, compile_shared,
-    generate_allowed_domains,
-    generate_awf_mounts,
-    generate_awf_path_step,
-    collect_awf_path_prepends,
-    generate_enabled_tools_args,
-    generate_mcpg_config, generate_mcpg_docker_env, generate_mcpg_step_env,
-    generate_stage_prefix, generate_template_parameters,
+    compile_template_target, TemplateTargetConfig,
     generate_header_comment,
 };
 use super::types::FrontMatter;
@@ -45,70 +37,22 @@ impl Compiler for JobCompiler {
         skip_integrity: bool,
         debug_pipeline: bool,
     ) -> Result<String> {
-        info!("Compiling for job template target");
-
         if front_matter.on_config.is_some() {
             warn!("on: trigger configuration is ignored for target: job (triggers are the parent pipeline's concern)");
         }
 
-        // Collect extensions (needed before compile_shared for MCPG config)
-        let extensions = super::extensions::collect_extensions(front_matter);
-
-        // Build compile context for MCPG config generation
-        let input_dir = input_path.parent().unwrap_or(std::path::Path::new("."));
-        let ctx = super::extensions::CompileContext::new(front_matter, input_dir).await?;
-
-        // Generate stage prefix for job-name uniqueness
-        let stage_prefix = generate_stage_prefix(&front_matter.name);
-
-        // Generate template-level parameters
-        let template_params = generate_template_parameters(front_matter)?;
-
-        // Same AWF/MCPG values as standalone
-        let allowed_domains = generate_allowed_domains(front_matter, &extensions)?;
-        let awf_mounts = generate_awf_mounts(&extensions);
-        let awf_paths = collect_awf_path_prepends(&extensions);
-        let awf_path_step = generate_awf_path_step(&awf_paths);
-        let enabled_tools_args = generate_enabled_tools_args(front_matter);
-
-        let config_obj = generate_mcpg_config(front_matter, &ctx, &extensions)?;
-        let mcpg_config_json =
-            serde_json::to_string_pretty(&config_obj).context("Failed to serialize MCPG config")?;
-        let mcpg_docker_env = generate_mcpg_docker_env(front_matter, &extensions);
-        let mcpg_step_env = generate_mcpg_step_env(&extensions);
-
-        let config = CompileConfig {
-            template: include_str!("../data/job-base.yml").to_string(),
-            extra_replacements: vec![
-                ("{{ stage_prefix }}".into(), stage_prefix),
-                ("{{ template_parameters }}".into(), template_params),
-                ("{{ firewall_version }}".into(), AWF_VERSION.into()),
-                ("{{ mcpg_version }}".into(), MCPG_VERSION.into()),
-                ("{{ mcpg_image }}".into(), MCPG_IMAGE.into()),
-                ("{{ mcpg_port }}".into(), MCPG_PORT.to_string()),
-                ("{{ mcpg_domain }}".into(), MCPG_DOMAIN.into()),
-                ("{{ allowed_domains }}".into(), allowed_domains),
-                ("{{ awf_mounts }}".into(), awf_mounts),
-                ("{{ awf_path_step }}".into(), awf_path_step),
-                ("{{ enabled_tools_args }}".into(), enabled_tools_args),
-                ("{{ mcpg_config }}".into(), mcpg_config_json),
-                ("{{ mcpg_docker_env }}".into(), mcpg_docker_env),
-                ("{{ mcpg_step_env }}".into(), mcpg_step_env),
-            ],
-            skip_integrity,
-            debug_pipeline,
-            has_awf_paths: !awf_paths.is_empty(),
-            skip_header: true,
-        };
-
-        let yaml = compile_shared(
-            input_path, output_path, front_matter, markdown_body,
-            &extensions, &ctx, config,
-        ).await?;
-
-        // Prepend custom header with job-template usage instructions
-        let header = generate_job_header(input_path, front_matter);
-        Ok(format!("{}{}", header, yaml))
+        compile_template_target(
+            input_path,
+            output_path,
+            front_matter,
+            markdown_body,
+            TemplateTargetConfig {
+                template: include_str!("../data/job-base.yml"),
+                skip_integrity,
+                debug_pipeline,
+            },
+            generate_job_header,
+        ).await
     }
 }
 

src/compile/mod.rs

@@ -196,6 +196,7 @@ async fn compile_pipeline_inner(
         CompileTarget::Job => Box::new(job::JobCompiler),
         CompileTarget::Stage => Box::new(stage::StageCompiler),
     };
+    info!("Using {} compiler", compiler.target_name());
 
     // Compile (no source mutation yet — a failure here must leave the
     // source byte-identical).