From 38ed412fa8570666675e7c7ab71de0a3ffa19bb1 Mon Sep 17 00:00:00 2001
From: James Devine <devinejames@microsoft.com>
Date: Tue, 30 Jun 2026 22:25:58 +0100
Subject: [PATCH 1/4] feat(compile): migrate legacy {{ workspace }} markers and
 validate checkout-aware paths

Before the native-IR migration, the compiler folded {{ workspace }},
{{ working_directory }} and {{ trigger_repo_directory }} markers across
the whole generated YAML (including custom steps). After the migration
these markers flow through verbatim and are no longer substituted.

Rather than restore runtime substitution of a fixed-path anchor (an
antipattern under multi-checkout, where $(Build.SourcesDirectory) is the
shared root of every checked-out repo), this:

- Adds codemod 0004_legacy_path_markers, which rewrites the markers in
  front matter to the explicit ADO path they resolved to, derived from
  the source's own workspace:/repos:.
- Adds a warning-only path_layout_check pass that flags checkout-aware
  path mistakes (references to not-checked-out repos, the multi-checkout
  self-subfolder form under single checkout, runtime-import targets to
  not-checked-out repos) and deprecated markers left in the agent body.
- Extracts shared pure resolvers (resolve_working_directory_expr,
  contains_template_marker) from compute_effective_workspace.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 AGENTS.md                                     |   3 +
 docs/codemods.md                              |  37 +-
 docs/front-matter.md                          |  25 ++
 .../codemods/0004_legacy_path_markers.rs      | 337 ++++++++++++++++++
 src/compile/codemods/mod.rs                   |   3 +
 src/compile/common.rs                         |  91 ++++-
 src/compile/mod.rs                            |  10 +
 src/compile/path_layout_check.rs              | 301 ++++++++++++++++
 tests/codemod_tests.rs                        |  48 +++
 9 files changed, 839 insertions(+), 16 deletions(-)
 create mode 100644 src/compile/codemods/0004_legacy_path_markers.rs
 create mode 100644 src/compile/path_layout_check.rs

diff --git a/AGENTS.md b/AGENTS.md
index b578fb12..ebfecf40 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -72,6 +72,7 @@ fail-closed and only pauses when the agent actually proposed a reviewed output.
 │   │   ├── gitattributes.rs # .gitattributes management for compiled pipelines
 │   │   ├── filter_ir.rs  # Filter expression IR: Fact/Predicate types, lowering, validation, codegen
 │   │   ├── pr_filters.rs # PR trigger filter generation (native ADO + gate steps)
+│   │   ├── path_layout_check.rs # Warning-only checkout-aware path validation: $(Build.SourcesDirectory)/<seg> refs in steps, runtime-import targets, deprecated directory markers in the body
 │   │   ├── extensions/   # CompilerExtension trait and infrastructure extensions
 │   │   │   ├── mod.rs    # Trait, Extension enum, collect_extensions(), re-exports
 │   │   │   ├── ado_aw_marker.rs # Always-on metadata marker extension (emits # ado-aw-metadata JSON)
@@ -95,6 +96,8 @@ fail-closed and only pauses when the agent actually proposed a reviewed output.
 │   │   │   ├── mod.rs    # Codemod struct, CODEMODS registry, runner
 │   │   │   ├── 0001_repos_unified.rs # Legacy repositories/checkout → repos codemod
 │   │   │   ├── 0002_pool_object_form.rs # Legacy scalar pool → object form codemod
+│   │   │   ├── 0003_flatten_work_item_config.rs # Legacy work-item config flatten codemod
+│   │   │   ├── 0004_legacy_path_markers.rs # Migrate {{ workspace }}/{{ working_directory }}/{{ trigger_repo_directory }} markers → explicit ADO path exprs (resolved from workspace:/repos:)
 │   │   │   └── helpers.rs # take_key, insert_no_overwrite, rename_key, ConflictPolicy
 │   │   ├── codemod_integration_test.rs # White-box rewrite-path tests (stub registry injection)
 │   │   ├── types.rs      # Front matter grammar and types
diff --git a/docs/codemods.md b/docs/codemods.md
index ff91acf6..b3c0e1b7 100644
--- a/docs/codemods.md
+++ b/docs/codemods.md
@@ -109,7 +109,9 @@ src/compile/codemods/
 ├── mod.rs                  # Framework + CODEMODS registry
 ├── helpers.rs              # take_key, insert_no_overwrite, rename_key, ConflictPolicy
 ├── 0001_repos_unified.rs   # Legacy repositories: + checkout: → repos: codemod
-└── 0002_pool_object_form.rs # Legacy scalar pool → explicit object form codemod
+├── 0002_pool_object_form.rs # Legacy scalar pool → explicit object form codemod
+├── 0003_flatten_work_item_config.rs # Legacy work-item config flatten codemod
+└── 0004_legacy_path_markers.rs # {{ workspace }} / {{ working_directory }} / {{ trigger_repo_directory }} → explicit ADO path exprs
 ```
 
 (New codemods are appended as `<NNNN>_<id>.rs` files.)
@@ -357,6 +359,39 @@ fn describe(v: &Value) -> &'static str {
    identifiers cannot start with digits, but the file name does.
    The registry-uniqueness and filename-prefix tests keep passing.
 
+## Legacy directory markers (`0004_legacy_path_markers`)
+
+Before the native-IR migration, the compiler folded a fixed
+replacement list across the **entire** generated YAML — including
+user-authored `steps:` / `post-steps:` / `setup:` / `teardown:`. That
+fold substituted the directory markers:
+
+| Marker | Resolved to |
+|--------|-------------|
+| `{{ workspace }}`, `{{ working_directory }}` | the resolved working directory (`$(Build.SourcesDirectory)`, `$(Build.SourcesDirectory)/$(Build.Repository.Name)`, or `$(Build.SourcesDirectory)/<alias>`) |
+| `{{ trigger_repo_directory }}` | the trigger ("self") repo dir |
+
+After the IR migration these markers flow through verbatim and are no
+longer substituted. Rather than restore runtime substitution of a
+fixed-path anchor — an antipattern under multi-checkout, where
+`$(Build.SourcesDirectory)` is the **shared root** of every checked-out
+repo — the `legacy_path_markers` codemod migrates existing sources by
+rewriting each marker (interior whitespace ignored, so both
+`{{ workspace }}` and `{{workspace}}` are handled) to the explicit ADO
+path expression it resolved to, derived from the source's own
+`workspace:` / `repos:`. The rewrite walks every string scalar in the
+front-matter mapping, so markers in any field (not just custom steps)
+are migrated.
+
+The codemod cannot touch the **markdown body** (codemods are
+mapping-only). A companion warning-only pass —
+`src/compile/path_layout_check.rs` — surfaces deprecated markers left
+in the body, plus checkout-aware path mistakes such as a
+`$(Build.SourcesDirectory)/<seg>` reference whose `<seg>` is a
+declared-but-not-checked-out repo, or the multi-checkout self subfolder
+form used under a single checkout. These are advisory and never fail
+the compile.
+
 ## Tests
 
 The codemod framework is covered by three layers of tests:
diff --git a/docs/front-matter.md b/docs/front-matter.md
index 42d2be61..71016e4b 100644
--- a/docs/front-matter.md
+++ b/docs/front-matter.md
@@ -252,6 +252,31 @@ default):
 Set `workspace:` explicitly to `root`, `repo` (alias `self`), or a specific
 checked-out repository alias to override this behavior.
 
+### Deprecated directory markers
+
+Earlier releases substituted the directory markers `{{ workspace }}`,
+`{{ working_directory }}`, and `{{ trigger_repo_directory }}` inside custom
+`steps:` / `post-steps:` / `setup:` / `teardown:` blocks. These are
+**deprecated** — they encouraged hard-coding a fixed path anchor, which is
+incorrect under multi-checkout where `$(Build.SourcesDirectory)` is the shared
+root of every checked-out repository.
+
+Reference the explicit ADO path instead:
+
+- `$(Build.SourcesDirectory)` — the checkout root (the trigger repo root when
+  only `self` is checked out).
+- `$(Build.SourcesDirectory)/$(Build.Repository.Name)` — the trigger repo when
+  one or more additional repositories are checked out.
+- `$(Build.SourcesDirectory)/<alias>` — a specific checked-out repository.
+
+The `legacy_path_markers` codemod automatically rewrites any remaining markers
+in front matter to the path they resolved to on the next `compile` (see
+[`docs/codemods.md`](codemods.md)). Markers left in the **agent body** cannot be
+migrated automatically and are reported as a compile warning. The compiler also
+emits warning-only advisories when a `$(Build.SourcesDirectory)/<seg>` reference
+or a `{{#runtime-import …}}` target points at a path that will not exist under
+the resolved checkout layout.
+
 ## Repositories (`repos:`)
 
 The `repos:` field provides a compact way to declare additional repository
diff --git a/src/compile/codemods/0004_legacy_path_markers.rs b/src/compile/codemods/0004_legacy_path_markers.rs
new file mode 100644
index 00000000..022b78ba
--- /dev/null
+++ b/src/compile/codemods/0004_legacy_path_markers.rs
@@ -0,0 +1,337 @@
+//! Legacy directory markers → explicit ADO path expressions.
+//!
+//! Before the native-IR migration, the compiler folded a fixed
+//! replacement list across the **entire** generated YAML — including
+//! user-authored `steps:` / `post-steps:` / `setup:` / `teardown:`.
+//! That list expanded the directory markers:
+//!
+//! - `{{ workspace }}` and `{{ working_directory }}` → the resolved
+//!   working directory (`$(Build.SourcesDirectory)`,
+//!   `$(Build.SourcesDirectory)/$(Build.Repository.Name)`, or
+//!   `$(Build.SourcesDirectory)/<alias>`).
+//! - `{{ trigger_repo_directory }}` → the trigger ("self") repo dir.
+//!
+//! After the IR migration these markers flow through verbatim and are
+//! no longer substituted. Rather than restore runtime substitution of
+//! a fixed-path anchor (an antipattern under multi-checkout, where
+//! `$(Build.SourcesDirectory)` is the shared root of every checked-out
+//! repo), this codemod migrates existing sources by rewriting each
+//! marker to the explicit ADO path expression it resolved to, derived
+//! from the source's own `workspace:` / `repos:`.
+//!
+//! The rewrite is faithful to the legacy whole-YAML fold: it walks
+//! **every string scalar** in the front-matter mapping, so markers in
+//! any field (not just custom steps) are migrated.
+//!
+//! Idempotent: resolved expressions contain no `{{ }}`, so a second
+//! run finds nothing to rewrite. Detection-based: returns `Ok(false)`
+//! when no targeted marker is present, avoiding any `repos:`
+//! deserialization on the common path.
+
+use anyhow::Result;
+use serde_yaml::{Mapping, Value};
+
+use super::{Codemod, CodemodContext};
+use crate::compile::common::{
+    contains_template_marker, generate_trigger_repo_directory, lower_repos,
+    resolve_working_directory_expr,
+};
+use crate::compile::types::ReposItem;
+
+/// Marker base names handled by this codemod. `workspace` and
+/// `working_directory` resolve to the same working-directory
+/// expression; `trigger_repo_directory` resolves to the self-repo dir.
+const MARKER_NAMES: &[&str] = &["workspace", "working_directory", "trigger_repo_directory"];
+
+pub static CODEMOD: Codemod = Codemod {
+    id: "legacy_path_markers",
+    summary: "{{ workspace }}/{{ working_directory }}/{{ trigger_repo_directory }} -> explicit ADO path",
+    introduced_in: "0.38.0",
+    apply: apply_codemod,
+};
+
+fn apply_codemod(fm: &mut Mapping, _ctx: &CodemodContext) -> Result<bool> {
+    // Cheap detection first: only deserialize `repos:` and resolve the
+    // working directory when at least one targeted marker is present.
+    let present = fm.iter().any(|(_k, v)| value_has_any_marker(v, MARKER_NAMES));
+    if !present {
+        return Ok(false);
+    }
+
+    // Derive the checkout-alias list from the (already unified by
+    // `m0001_repos_unified`) `repos:` mapping, then resolve the markers
+    // exactly as the typed compile path would.
+    let checkout = derive_checkout_aliases(fm)?;
+    let workspace = fm
+        .get(Value::String("workspace".to_string()))
+        .and_then(|v| v.as_str())
+        .map(|s| s.to_string());
+    let agent_name = fm
+        .get(Value::String("name".to_string()))
+        .and_then(|v| v.as_str())
+        .unwrap_or("agent");
+
+    let working_directory = resolve_working_directory_expr(&workspace, &checkout, agent_name)?;
+    let trigger_repo_directory = generate_trigger_repo_directory(&checkout);
+
+    let replacements: Vec<(&str, String)> = vec![
+        ("workspace", working_directory.clone()),
+        ("working_directory", working_directory),
+        ("trigger_repo_directory", trigger_repo_directory),
+    ];
+
+    let mut changed = false;
+    for (_k, v) in fm.iter_mut() {
+        changed |= replace_in_value(v, &replacements);
+    }
+    Ok(changed)
+}
+
+/// Derive the checked-out repository aliases from the untyped `repos:`
+/// mapping, reusing the same lowering the typed path uses so the two
+/// cannot drift. Returns an empty list when `repos:` is absent (the
+/// single-checkout case, where `$(Build.SourcesDirectory)` is the repo
+/// root).
+fn derive_checkout_aliases(fm: &Mapping) -> Result<Vec<String>> {
+    let Some(repos_val) = fm.get(Value::String("repos".to_string())) else {
+        return Ok(Vec::new());
+    };
+    let items: Vec<ReposItem> = serde_yaml::from_value(repos_val.clone()).map_err(|e| {
+        anyhow::anyhow!("failed to read `repos:` while migrating legacy path markers: {e}")
+    })?;
+    let (_repositories, checkout) = lower_repos(&items)?;
+    Ok(checkout)
+}
+
+/// Recursively replace every targeted marker in the string scalars of
+/// `v`. Keys are left untouched. Returns whether any scalar changed.
+fn replace_in_value(v: &mut Value, replacements: &[(&str, String)]) -> bool {
+    match v {
+        Value::String(s) => {
+            let mut updated = s.clone();
+            for (name, repl) in replacements {
+                updated = replace_marker(&updated, name, repl);
+            }
+            if &updated != s {
+                *s = updated;
+                true
+            } else {
+                false
+            }
+        }
+        Value::Sequence(seq) => {
+            let mut changed = false;
+            for item in seq.iter_mut() {
+                changed |= replace_in_value(item, replacements);
+            }
+            changed
+        }
+        Value::Mapping(m) => {
+            let mut changed = false;
+            for (_k, val) in m.iter_mut() {
+                changed |= replace_in_value(val, replacements);
+            }
+            changed
+        }
+        _ => false,
+    }
+}
+
+/// Whether any of `names` appears as a `{{ name }}` marker in `v`'s
+/// string scalars (recursive). Whitespace inside the braces is ignored
+/// so both `{{ workspace }}` and `{{workspace}}` are detected.
+fn value_has_any_marker(v: &Value, names: &[&str]) -> bool {
+    match v {
+        Value::String(s) => names.iter().any(|n| contains_template_marker(s, n)),
+        Value::Sequence(seq) => seq.iter().any(|x| value_has_any_marker(x, names)),
+        Value::Mapping(m) => m.iter().any(|(_k, val)| value_has_any_marker(val, names)),
+        _ => false,
+    }
+}
+
+/// Replace every `{{ name }}` marker (interior whitespace ignored) in
+/// `input` with `repl`. Non-matching `{{ ... }}` spans (e.g.
+/// `{{#runtime-import ...}}` or `${{ parameters.x }}`) are preserved.
+fn replace_marker(input: &str, name: &str, repl: &str) -> String {
+    let mut out = String::with_capacity(input.len());
+    let mut i = 0;
+    while i < input.len() {
+        if input[i..].starts_with("{{") {
+            let start = i + 2;
+            if let Some(close) = input[start..].find("}}")
+                && input[start..start + close].trim() == name
+            {
+                out.push_str(repl);
+                i = start + close + 2;
+                continue;
+            }
+        }
+        let ch = input[i..].chars().next().expect("non-empty remainder");
+        out.push(ch);
+        i += ch.len_utf8();
+    }
+    out
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn ctx() -> CodemodContext {
+        CodemodContext {
+            compiler_version: "0.38.0",
+        }
+    }
+
+    fn fm_from(yaml: &str) -> Mapping {
+        serde_yaml::from_str(yaml).expect("parse front matter")
+    }
+
+    fn step_script(fm: &Mapping) -> String {
+        let steps = fm.get(Value::String("steps".to_string())).expect("steps");
+        let first = steps.as_sequence().expect("seq")[0]
+            .as_mapping()
+            .expect("map");
+        first
+            .get(Value::String("script".to_string()))
+            .and_then(|v| v.as_str())
+            .expect("script")
+            .to_string()
+    }
+
+    #[test]
+    fn noop_when_no_markers() {
+        let mut fm = fm_from("name: a\ndescription: d\nsteps:\n  - script: echo hi\n");
+        let snapshot = fm.clone();
+        let changed = apply_codemod(&mut fm, &ctx()).expect("apply");
+        assert!(!changed);
+        assert_eq!(fm, snapshot);
+    }
+
+    #[test]
+    fn workspace_marker_single_checkout_resolves_to_root() {
+        // No additional repos → $(Build.SourcesDirectory) is the repo root.
+        let mut fm = fm_from(
+            "name: a\ndescription: d\nsteps:\n  - script: cd {{ workspace }} && ls\n",
+        );
+        let changed = apply_codemod(&mut fm, &ctx()).expect("apply");
+        assert!(changed);
+        assert_eq!(step_script(&fm), "cd $(Build.SourcesDirectory) && ls");
+    }
+
+    #[test]
+    fn no_space_variant_is_migrated() {
+        let mut fm =
+            fm_from("name: a\ndescription: d\nsteps:\n  - script: cd {{workspace}}\n");
+        let changed = apply_codemod(&mut fm, &ctx()).expect("apply");
+        assert!(changed);
+        assert_eq!(step_script(&fm), "cd $(Build.SourcesDirectory)");
+    }
+
+    #[test]
+    fn working_directory_alias_resolves_same_as_workspace() {
+        let mut fm = fm_from(
+            "name: a\ndescription: d\nsteps:\n  - script: echo {{ working_directory }}\n",
+        );
+        apply_codemod(&mut fm, &ctx()).expect("apply");
+        assert_eq!(step_script(&fm), "echo $(Build.SourcesDirectory)");
+    }
+
+    #[test]
+    fn multi_checkout_workspace_repo_resolves_to_self_subfolder() {
+        let mut fm = fm_from(
+            "name: a\ndescription: d\nworkspace: repo\nrepos:\n  - org/other\nsteps:\n  - script: cd {{ workspace }}\n",
+        );
+        let changed = apply_codemod(&mut fm, &ctx()).expect("apply");
+        assert!(changed);
+        assert_eq!(
+            step_script(&fm),
+            "cd $(Build.SourcesDirectory)/$(Build.Repository.Name)"
+        );
+    }
+
+    #[test]
+    fn workspace_alias_resolves_to_alias_subfolder() {
+        let mut fm = fm_from(
+            "name: a\ndescription: d\nworkspace: other\nrepos:\n  - org/other\nsteps:\n  - script: cd {{ workspace }}\n",
+        );
+        apply_codemod(&mut fm, &ctx()).expect("apply");
+        assert_eq!(step_script(&fm), "cd $(Build.SourcesDirectory)/other");
+    }
+
+    #[test]
+    fn trigger_repo_directory_marker_multi_checkout() {
+        let mut fm = fm_from(
+            "name: a\ndescription: d\nrepos:\n  - org/other\nsteps:\n  - script: cat {{ trigger_repo_directory }}/file\n",
+        );
+        apply_codemod(&mut fm, &ctx()).expect("apply");
+        assert_eq!(
+            step_script(&fm),
+            "cat $(Build.SourcesDirectory)/$(Build.Repository.Name)/file"
+        );
+    }
+
+    #[test]
+    fn preserves_runtime_import_and_parameter_spans() {
+        let mut fm = fm_from(
+            "name: a\ndescription: d\nsteps:\n  - script: \"${{ parameters.x }} {{#runtime-import foo.md}} {{ workspace }}\"\n",
+        );
+        apply_codemod(&mut fm, &ctx()).expect("apply");
+        assert_eq!(
+            step_script(&fm),
+            "${{ parameters.x }} {{#runtime-import foo.md}} $(Build.SourcesDirectory)"
+        );
+    }
+
+    #[test]
+    fn migrates_post_steps_and_setup_and_teardown() {
+        let mut fm = fm_from(
+            "name: a\ndescription: d\n\
+             setup:\n  - script: a {{ workspace }}\n\
+             steps:\n  - script: b {{ workspace }}\n\
+             post-steps:\n  - script: c {{ workspace }}\n\
+             teardown:\n  - script: e {{ workspace }}\n",
+        );
+        let changed = apply_codemod(&mut fm, &ctx()).expect("apply");
+        assert!(changed);
+        for (key, prefix) in [
+            ("setup", "a"),
+            ("steps", "b"),
+            ("post-steps", "c"),
+            ("teardown", "e"),
+        ] {
+            let seq = fm
+                .get(Value::String(key.to_string()))
+                .and_then(|v| v.as_sequence())
+                .expect("seq");
+            let script = seq[0]
+                .as_mapping()
+                .and_then(|m| m.get(Value::String("script".to_string())))
+                .and_then(|v| v.as_str())
+                .expect("script");
+            assert_eq!(script, format!("{prefix} $(Build.SourcesDirectory)"));
+        }
+    }
+
+    #[test]
+    fn idempotent_second_run_is_noop() {
+        let mut fm = fm_from(
+            "name: a\ndescription: d\nsteps:\n  - script: cd {{ workspace }}\n",
+        );
+        let changed1 = apply_codemod(&mut fm, &ctx()).expect("first");
+        assert!(changed1);
+        let snapshot = fm.clone();
+        let changed2 = apply_codemod(&mut fm, &ctx()).expect("second");
+        assert!(!changed2, "second run must be a no-op");
+        assert_eq!(fm, snapshot);
+    }
+
+    #[test]
+    fn marker_present_ignores_other_spans() {
+        assert!(contains_template_marker("a {{ workspace }} b", "workspace"));
+        assert!(contains_template_marker("{{workspace}}", "workspace"));
+        assert!(!contains_template_marker("{{#runtime-import x}}", "workspace"));
+        assert!(!contains_template_marker("${{ parameters.x }}", "workspace"));
+        assert!(!contains_template_marker("no markers here", "workspace"));
+    }
+}
diff --git a/src/compile/codemods/mod.rs b/src/compile/codemods/mod.rs
index 5e1505d0..331a9a5a 100644
--- a/src/compile/codemods/mod.rs
+++ b/src/compile/codemods/mod.rs
@@ -39,6 +39,8 @@ mod m0001_repos_unified;
 mod m0002_pool_object_form;
 #[path = "0003_flatten_work_item_config.rs"]
 mod m0003_flatten_work_item_config;
+#[path = "0004_legacy_path_markers.rs"]
+mod m0004_legacy_path_markers;
 
 #[allow(unused_imports)] // Re-exported for future codemods; only `take_key` is in-tree use.
 pub use helpers::{ConflictPolicy, insert_no_overwrite, rename_key, take_key};
@@ -105,6 +107,7 @@ pub static CODEMODS: &[&Codemod] = &[
     &m0001_repos_unified::CODEMOD,
     &m0002_pool_object_form::CODEMOD,
     &m0003_flatten_work_item_config::CODEMOD,
+    &m0004_legacy_path_markers::CODEMOD,
 ];
 
 /// Result of running the codemod registry on a single front-matter
diff --git a/src/compile/common.rs b/src/compile/common.rs
index 5e24724a..88ef1394 100644
--- a/src/compile/common.rs
+++ b/src/compile/common.rs
@@ -775,24 +775,61 @@ pub fn compute_effective_workspace(
     checkout: &[String],
     agent_name: &str,
 ) -> Result<String> {
+    let (encoded, warn_no_checkouts) =
+        resolve_effective_workspace(explicit_workspace, checkout, agent_name)?;
+    if warn_no_checkouts {
+        let ws = explicit_workspace.as_deref().unwrap_or("repo");
+        eprintln!(
+            "Warning: Agent '{}' has workspace: {} but no additional repositories in checkout. \
+            When only 'self' is checked out, $(Build.SourcesDirectory) already contains the repository content. \
+            The workspace setting has no effect in this case.",
+            agent_name, ws
+        );
+    }
+    Ok(encoded)
+}
+
+/// Resolve the `workspace:` setting straight to the working-directory ADO
+/// path expression (e.g. `$(Build.SourcesDirectory)/<alias>`), with **no**
+/// side effects.
+///
+/// This is the side-effect-free counterpart to
+/// [`compute_effective_workspace`] + [`generate_working_directory`]. It is
+/// used by the `legacy_path_markers` codemod (codemods must stay pure — no
+/// stderr, no I/O) to migrate `{{ workspace }}` / `{{ working_directory }}`
+/// markers to the explicit path they resolved to. The "workspace: repo with
+/// no additional checkouts" advisory warning is intentionally dropped here;
+/// the normal typed compile path still surfaces it via
+/// [`compute_effective_workspace`].
+pub fn resolve_working_directory_expr(
+    explicit_workspace: &Option<String>,
+    checkout: &[String],
+    agent_name: &str,
+) -> Result<String> {
+    let (encoded, _warn) = resolve_effective_workspace(explicit_workspace, checkout, agent_name)?;
+    Ok(generate_working_directory(&encoded))
+}
+
+/// Pure resolution core shared by [`compute_effective_workspace`] (which adds
+/// a stderr advisory warning) and [`resolve_working_directory_expr`] (which
+/// must remain side-effect-free for codemod use).
+///
+/// Returns the encoded effective-workspace string (consumed by
+/// [`generate_working_directory`]) and a flag indicating whether the
+/// "workspace: repo/self with no additional checkouts" advisory applies.
+fn resolve_effective_workspace(
+    explicit_workspace: &Option<String>,
+    checkout: &[String],
+    agent_name: &str,
+) -> Result<(String, bool)> {
     let has_additional_checkouts = !checkout.is_empty();
 
     match explicit_workspace {
         Some(ws) => {
             let ws = ws.as_str();
             match ws {
-                "root" => Ok("root".to_string()),
-                "repo" | "self" => {
-                    if !has_additional_checkouts {
-                        eprintln!(
-                            "Warning: Agent '{}' has workspace: {} but no additional repositories in checkout. \
-                            When only 'self' is checked out, $(Build.SourcesDirectory) already contains the repository content. \
-                            The workspace setting has no effect in this case.",
-                            agent_name, ws
-                        );
-                    }
-                    Ok("repo".to_string())
-                }
+                "root" => Ok(("root".to_string(), false)),
+                "repo" | "self" => Ok(("repo".to_string(), !has_additional_checkouts)),
                 alias => {
                     // Defense in depth: even though aliases are constrained
                     // by `validate_checkout_list` to match a `repository:`
@@ -828,13 +865,37 @@ pub fn compute_effective_workspace(
                             checkout
                         );
                     }
-                    Ok(format!("{}{}", WORKSPACE_ALIAS_PREFIX, alias))
+                    Ok((format!("{}{}", WORKSPACE_ALIAS_PREFIX, alias), false))
+                }
+            }
+        }
+        None if has_additional_checkouts => Ok(("repo".to_string(), false)),
+        None => Ok(("root".to_string(), false)),
+    }
+}
+
+/// Whether `input` contains a `{{ name }}` template marker, ignoring
+/// interior whitespace (so both `{{ workspace }}` and `{{workspace}}`
+/// match). Non-matching `{{ ... }}` spans (e.g. `{{#runtime-import …}}`
+/// or `${{ parameters.x }}`) are ignored.
+///
+/// Shared by the `legacy_path_markers` codemod (marker detection) and
+/// the path-layout validation pass (agent-body deprecation warnings).
+pub(crate) fn contains_template_marker(input: &str, name: &str) -> bool {
+    let mut i = 0;
+    while let Some(open) = input[i..].find("{{") {
+        let start = i + open + 2;
+        match input[start..].find("}}") {
+            Some(close) => {
+                if input[start..start + close].trim() == name {
+                    return true;
                 }
+                i = start + close + 2;
             }
+            None => break,
         }
-        None if has_additional_checkouts => Ok("repo".to_string()),
-        None => Ok("root".to_string()),
     }
+    false
 }
 
 /// Generate the directory where the trigger ("self") repository is checked out.
diff --git a/src/compile/mod.rs b/src/compile/mod.rs
index baafd694..a06f3893 100644
--- a/src/compile/mod.rs
+++ b/src/compile/mod.rs
@@ -21,6 +21,7 @@ mod job_ir;
 mod onees;
 mod onees_ir;
 pub(crate) mod pr_filters;
+mod path_layout_check;
 pub mod source_path_guard;
 mod stage;
 mod stage_ir;
@@ -164,6 +165,15 @@ async fn compile_pipeline_inner(
     // Validate checkout list against repositories
     common::validate_checkout_list(&front_matter.repositories, &front_matter.checkout)?;
 
+    // Checkout-aware path-layout advisories (warning-only): surface
+    // hand-written paths that won't exist under the resolved checkout
+    // layout, plus deprecated directory markers left in the agent body.
+    for warning in
+        path_layout_check::collect_path_layout_warnings(&front_matter, &markdown_body)
+    {
+        eprintln!("Warning: {warning}");
+    }
+
     // Determine output path. By default use `.lock.yml` to match
     // gh-aw's convention for compiled-pipeline files (so they can be
     // marked as generated and merge=ours via `.gitattributes`). When the
diff --git a/src/compile/path_layout_check.rs b/src/compile/path_layout_check.rs
new file mode 100644
index 00000000..5b90aee8
--- /dev/null
+++ b/src/compile/path_layout_check.rs
@@ -0,0 +1,301 @@
+//! Checkout-aware path-layout validation (warning-only).
+//!
+//! Azure DevOps lays out checked-out repositories in two distinct
+//! shapes depending on how many repos are checked out:
+//!
+//! - **Single checkout** (only the trigger repo): `$(Build.SourcesDirectory)`
+//!   *is* the trigger repo root.
+//! - **Multi-checkout** (≥1 additional repo): every repo, including the
+//!   trigger repo, lives in a subfolder of `$(Build.SourcesDirectory)`
+//!   named after its alias — the trigger repo under
+//!   `$(Build.Repository.Name)`, each additional repo under its `repos:`
+//!   alias.
+//!
+//! When authors hand-write paths anchored at `$(Build.SourcesDirectory)`
+//! (or reference repos via `{{#runtime-import …}}`), it is easy to point
+//! at a path that will not exist under the resolved layout. This pass
+//! surfaces those mistakes as **warnings** — it never fails the compile,
+//! because the compiler cannot always resolve a path (e.g. the trigger
+//! repo's literal name behind `$(Build.Repository.Name)`), and false
+//! positives must not block builds.
+//!
+//! It also warns when deprecated directory markers
+//! (`{{ workspace }}` / `{{ working_directory }}` /
+//! `{{ trigger_repo_directory }}`) survive in the **agent body**: the
+//! `legacy_path_markers` codemod only migrates front matter, and these
+//! markers are no longer substituted at runtime.
+
+use serde_yaml::Value;
+
+use crate::compile::common::contains_template_marker;
+use crate::compile::types::FrontMatter;
+
+/// Literal prefix that anchors a path at the checkout root, including the
+/// trailing separator (so a bare `$(Build.SourcesDirectory)` root
+/// reference is not treated as having a sub-segment).
+const SOURCES_DIR_PREFIX: &str = "$(Build.SourcesDirectory)/";
+
+/// The runtime macro under which the trigger ("self") repo is checked
+/// out in multi-checkout mode.
+const SELF_REPO_SEGMENT: &str = "$(Build.Repository.Name)";
+
+/// Deprecated directory markers that the `legacy_path_markers` codemod
+/// migrates in front matter but cannot touch in the agent body.
+const DEPRECATED_MARKERS: &[&str] = &["workspace", "working_directory", "trigger_repo_directory"];
+
+/// Collect checkout-aware path-layout warnings for a compiled workflow.
+///
+/// Warning-only: the returned strings are advisory. Returns an empty
+/// vector when nothing looks wrong. Messages are de-duplicated.
+pub fn collect_path_layout_warnings(front_matter: &FrontMatter, markdown_body: &str) -> Vec<String> {
+    let mut warnings: Vec<String> = Vec::new();
+
+    let checked_out: Vec<&str> = front_matter.checkout.iter().map(String::as_str).collect();
+    let multi = !checked_out.is_empty();
+    // Repos declared in `repos:` but not checked out (`checkout: false`).
+    let declared_not_checked_out: Vec<&str> = front_matter
+        .repositories
+        .iter()
+        .map(|r| r.repository.as_str())
+        .filter(|alias| !checked_out.iter().any(|c| c == alias))
+        .collect();
+
+    // 1. `$(Build.SourcesDirectory)/<seg>` references in custom steps.
+    let mut step_scalars: Vec<&str> = Vec::new();
+    for block in [
+        &front_matter.setup,
+        &front_matter.steps,
+        &front_matter.post_steps,
+        &front_matter.teardown,
+    ] {
+        for value in block {
+            collect_string_scalars(value, &mut step_scalars);
+        }
+    }
+    for scalar in &step_scalars {
+        for seg in sources_dir_segments(scalar) {
+            if declared_not_checked_out.contains(&seg.as_str()) {
+                warnings.push(format!(
+                    "step references `$(Build.SourcesDirectory)/{seg}`, but repository `{seg}` is \
+                     declared in `repos:` with `checkout: false`; its sources will not be present \
+                     at that path. Set `checkout: true` or remove the reference."
+                ));
+            } else if !multi && seg == SELF_REPO_SEGMENT {
+                warnings.push(
+                    "step references `$(Build.SourcesDirectory)/$(Build.Repository.Name)`, but with \
+                     only the trigger repository checked out `$(Build.SourcesDirectory)` already IS \
+                     the repository root; that subfolder will not exist. Use \
+                     `$(Build.SourcesDirectory)` directly."
+                        .to_string(),
+                );
+            }
+        }
+    }
+
+    // 2. Runtime-import paths whose first segment is a declared-but-not-checked-out repo.
+    for seg in runtime_import_first_segments(markdown_body) {
+        if declared_not_checked_out.contains(&seg.as_str()) {
+            warnings.push(format!(
+                "`{{{{#runtime-import {seg}/…}}}}` targets repository `{seg}`, which is declared in \
+                 `repos:` with `checkout: false`; it will not be present in the workspace at \
+                 runtime. Set `checkout: true` for that repository."
+            ));
+        }
+    }
+
+    // 3. Deprecated directory markers surviving in the agent body.
+    for marker in DEPRECATED_MARKERS {
+        if contains_template_marker(markdown_body, marker) {
+            warnings.push(format!(
+                "deprecated directory marker `{{{{ {marker} }}}}` found in the agent body; it is no \
+                 longer substituted (the migration codemod only rewrites front matter). Replace it \
+                 with an explicit `$(Build.SourcesDirectory)…` path."
+            ));
+        }
+    }
+
+    warnings.sort();
+    warnings.dedup();
+    warnings
+}
+
+/// Recursively collect borrowed string scalars from a YAML value
+/// (mapping values and sequence items; keys are ignored).
+fn collect_string_scalars<'a>(value: &'a Value, out: &mut Vec<&'a str>) {
+    match value {
+        Value::String(s) => out.push(s.as_str()),
+        Value::Sequence(seq) => {
+            for item in seq {
+                collect_string_scalars(item, out);
+            }
+        }
+        Value::Mapping(m) => {
+            for (_k, v) in m {
+                collect_string_scalars(v, out);
+            }
+        }
+        _ => {}
+    }
+}
+
+/// Extract the first path segment after each `$(Build.SourcesDirectory)/`
+/// occurrence in `s`. The segment runs up to the next path separator,
+/// whitespace, or quote.
+fn sources_dir_segments(s: &str) -> Vec<String> {
+    let mut segs = Vec::new();
+    let mut i = 0;
+    while let Some(pos) = s[i..].find(SOURCES_DIR_PREFIX) {
+        let start = i + pos + SOURCES_DIR_PREFIX.len();
+        let seg: String = s[start..]
+            .chars()
+            .take_while(|&c| c != '/' && !c.is_whitespace() && c != '"' && c != '\'' && c != '\\')
+            .collect();
+        if !seg.is_empty() {
+            segs.push(seg);
+        }
+        i = start;
+    }
+    segs
+}
+
+/// Extract the first path segment of every `{{#runtime-import path}}` /
+/// `{{#runtime-import? path}}` marker in `body`.
+fn runtime_import_first_segments(body: &str) -> Vec<String> {
+    const KEY: &str = "{{#runtime-import";
+    let mut segs = Vec::new();
+    let mut i = 0;
+    while let Some(pos) = body[i..].find(KEY) {
+        let after = i + pos + KEY.len();
+        match body[after..].find("}}") {
+            Some(close) => {
+                let inner = body[after..after + close].trim();
+                // Optional imports are written `{{#runtime-import? path}}`.
+                let inner = inner.strip_prefix('?').unwrap_or(inner).trim();
+                if let Some(path) = inner.split_whitespace().next()
+                    && let Some(first) = path.split('/').next()
+                    && !first.is_empty()
+                {
+                    segs.push(first.to_string());
+                }
+                i = after + close + 2;
+            }
+            None => break,
+        }
+    }
+    segs
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn fm(yaml: &str) -> FrontMatter {
+        let mut fm: FrontMatter = serde_yaml::from_str(yaml).expect("parse front matter");
+        // Lower repos the way the compile pipeline does.
+        if !fm.repos.is_empty() {
+            let (repositories, checkout) =
+                crate::compile::common::lower_repos(&fm.repos).expect("lower repos");
+            fm.repositories = repositories;
+            fm.checkout = checkout;
+        }
+        fm
+    }
+
+    #[test]
+    fn no_warnings_for_clean_single_checkout() {
+        let fm = fm("name: a\ndescription: d\nsteps:\n  - script: cd $(Build.SourcesDirectory)\n");
+        assert!(collect_path_layout_warnings(&fm, "body").is_empty());
+    }
+
+    #[test]
+    fn warns_on_self_subfolder_in_single_checkout() {
+        let fm = fm(
+            "name: a\ndescription: d\nsteps:\n  - script: cd $(Build.SourcesDirectory)/$(Build.Repository.Name)\n",
+        );
+        let w = collect_path_layout_warnings(&fm, "body");
+        assert_eq!(w.len(), 1, "{w:?}");
+        assert!(w[0].contains("already IS the repository root"), "{w:?}");
+    }
+
+    #[test]
+    fn no_self_subfolder_warning_in_multi_checkout() {
+        let fm = fm(
+            "name: a\ndescription: d\nrepos:\n  - org/other\nsteps:\n  - script: cd $(Build.SourcesDirectory)/$(Build.Repository.Name)\n",
+        );
+        assert!(collect_path_layout_warnings(&fm, "body").is_empty());
+    }
+
+    #[test]
+    fn warns_on_reference_to_not_checked_out_repo() {
+        let fm = fm(
+            "name: a\ndescription: d\nrepos:\n  - name: org/other\n    checkout: false\nsteps:\n  - script: cat $(Build.SourcesDirectory)/other/file\n",
+        );
+        let w = collect_path_layout_warnings(&fm, "body");
+        assert_eq!(w.len(), 1, "{w:?}");
+        assert!(w[0].contains("`checkout: false`"), "{w:?}");
+        assert!(w[0].contains("other"), "{w:?}");
+    }
+
+    #[test]
+    fn no_warning_for_checked_out_alias_reference() {
+        let fm = fm(
+            "name: a\ndescription: d\nrepos:\n  - org/other\nsteps:\n  - script: cat $(Build.SourcesDirectory)/other/file\n",
+        );
+        assert!(collect_path_layout_warnings(&fm, "body").is_empty());
+    }
+
+    #[test]
+    fn warns_on_runtime_import_to_not_checked_out_repo() {
+        let fm = fm(
+            "name: a\ndescription: d\nrepos:\n  - name: org/other\n    checkout: false\n",
+        );
+        let body = "Read {{#runtime-import other/docs/policy.md}} please";
+        let w = collect_path_layout_warnings(&fm, body);
+        assert_eq!(w.len(), 1, "{w:?}");
+        assert!(w[0].contains("runtime-import"), "{w:?}");
+    }
+
+    #[test]
+    fn warns_on_deprecated_marker_in_body() {
+        let fm = fm("name: a\ndescription: d\n");
+        let body = "Run inside {{ workspace }} now.";
+        let w = collect_path_layout_warnings(&fm, body);
+        assert_eq!(w.len(), 1, "{w:?}");
+        assert!(w[0].contains("agent body"), "{w:?}");
+    }
+
+    #[test]
+    fn deduplicates_repeated_warnings() {
+        let fm = fm(
+            "name: a\ndescription: d\nsteps:\n  - script: cd $(Build.SourcesDirectory)/$(Build.Repository.Name)\n  - script: ls $(Build.SourcesDirectory)/$(Build.Repository.Name)\n",
+        );
+        let w = collect_path_layout_warnings(&fm, "body");
+        assert_eq!(w.len(), 1, "{w:?}");
+    }
+
+    #[test]
+    fn sources_dir_segments_extraction() {
+        assert_eq!(
+            sources_dir_segments("cd $(Build.SourcesDirectory)/foo/bar && ls"),
+            vec!["foo".to_string()]
+        );
+        assert!(sources_dir_segments("cd $(Build.SourcesDirectory)").is_empty());
+        assert_eq!(
+            sources_dir_segments("$(Build.SourcesDirectory)/$(Build.Repository.Name)/x"),
+            vec![SELF_REPO_SEGMENT.to_string()]
+        );
+    }
+
+    #[test]
+    fn runtime_import_segments_extraction() {
+        assert_eq!(
+            runtime_import_first_segments("{{#runtime-import other/x.md}}"),
+            vec!["other".to_string()]
+        );
+        assert_eq!(
+            runtime_import_first_segments("{{#runtime-import? other/x.md}}"),
+            vec!["other".to_string()]
+        );
+        assert!(runtime_import_first_segments("no imports").is_empty());
+    }
+}
diff --git a/tests/codemod_tests.rs b/tests/codemod_tests.rs
index 5b34c10d..813c0124 100644
--- a/tests/codemod_tests.rs
+++ b/tests/codemod_tests.rs
@@ -84,6 +84,54 @@ fn copy_fixture(dir: &Path, fixture_name: &str) -> PathBuf {
     dest
 }
 
+// ─── Legacy directory marker migration (codemod 0004) ──────────────────────
+
+#[test]
+fn compile_migrates_legacy_workspace_marker_in_steps() {
+    let dir = fresh_temp_dir();
+    let original = "---\nname: ws-marker\ndescription: d\nsteps:\n  - script: cd {{ workspace }} && ls\n---\n## Body\n\nHello.\n";
+    let source = write_source(dir.path(), original);
+
+    let output = run_compile(&source);
+    assert!(
+        output.status.success(),
+        "compile should succeed: stdout={:?} stderr={:?}",
+        String::from_utf8_lossy(&output.stdout),
+        String::from_utf8_lossy(&output.stderr)
+    );
+
+    // The source is rewritten in place: the marker is replaced with the
+    // explicit ADO path it resolved to (single-checkout → sources root).
+    let after = fs::read_to_string(&source).expect("re-read source");
+    assert!(
+        after.contains("cd $(Build.SourcesDirectory) && ls"),
+        "source should be migrated, got:\n{after}"
+    );
+    assert!(
+        !after.contains("{{ workspace }}"),
+        "legacy marker must be gone from source, got:\n{after}"
+    );
+
+    // The codemod warning is surfaced.
+    let stderr = String::from_utf8_lossy(&output.stderr);
+    assert!(
+        stderr.contains("applied codemods"),
+        "expected codemod warning, got stderr: {stderr}"
+    );
+
+    // The compiled lock file carries the resolved path, not the marker.
+    let lock = source.with_extension("lock.yml");
+    let lock_str = fs::read_to_string(&lock).expect("read lock");
+    assert!(
+        lock_str.contains("cd $(Build.SourcesDirectory) && ls"),
+        "lock should contain resolved path, got:\n{lock_str}"
+    );
+    assert!(
+        !lock_str.contains("{{ workspace }}"),
+        "lock must not contain the legacy marker"
+    );
+}
+
 // ─── Healthy compile (no codemods needed) ──────────────────────────────────
 
 #[test]

From 8bf799894ab36258685772e23019b9d71defb845 Mon Sep 17 00:00:00 2001
From: James Devine <devinejames@microsoft.com>
Date: Wed, 1 Jul 2026 10:45:04 +0100
Subject: [PATCH 2/4] fix(compile): align marker-scan helpers and path-segment
 advancement

Address Rust PR review feedback on #1263:

- contains_template_marker now advances one code-point past a non-matching {{ (matching replace_marker) so nested {{ ... }} forms are detected consistently and the codemod early-exit gate cannot skip a substitutable marker.

- sources_dir_segments advances i by start + seg.len() instead of start, avoiding re-scanning an extracted segment on back-to-back double prefixes.

- replace_marker uses a let-else break instead of expect() on the string-walk hot path.

Adds regression tests for the nested-marker consistency and double-prefix segment cases.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 .../codemods/0004_legacy_path_markers.rs      | 20 +++++++++++++++++-
 src/compile/common.rs                         | 21 +++++++++++--------
 src/compile/path_layout_check.rs              | 16 +++++++++++++-
 3 files changed, 46 insertions(+), 11 deletions(-)

diff --git a/src/compile/codemods/0004_legacy_path_markers.rs b/src/compile/codemods/0004_legacy_path_markers.rs
index 022b78ba..bb9d28db 100644
--- a/src/compile/codemods/0004_legacy_path_markers.rs
+++ b/src/compile/codemods/0004_legacy_path_markers.rs
@@ -166,7 +166,9 @@ fn replace_marker(input: &str, name: &str, repl: &str) -> String {
                 continue;
             }
         }
-        let ch = input[i..].chars().next().expect("non-empty remainder");
+        let Some(ch) = input[i..].chars().next() else {
+            break;
+        };
         out.push(ch);
         i += ch.len_utf8();
     }
@@ -334,4 +336,20 @@ mod tests {
         assert!(!contains_template_marker("${{ parameters.x }}", "workspace"));
         assert!(!contains_template_marker("no markers here", "workspace"));
     }
+
+    #[test]
+    fn nested_marker_detection_matches_replacement() {
+        // Regression: `contains_template_marker` (the early-exit gate) and
+        // `replace_marker` must agree on doubly-nested input. Previously the
+        // gate jumped past the outer `}}` and missed the inner marker while
+        // the replacer found and substituted it — leaving the gate returning
+        // `false` for a value that would actually be rewritten.
+        let input = "{{ bad {{ workspace }} }}";
+        assert!(contains_template_marker(input, "workspace"));
+        assert_ne!(
+            replace_marker(input, "workspace", "REPL"),
+            input,
+            "replace_marker should substitute the inner marker"
+        );
+    }
 }
diff --git a/src/compile/common.rs b/src/compile/common.rs
index 88ef1394..fdad37a8 100644
--- a/src/compile/common.rs
+++ b/src/compile/common.rs
@@ -884,16 +884,19 @@ fn resolve_effective_workspace(
 pub(crate) fn contains_template_marker(input: &str, name: &str) -> bool {
     let mut i = 0;
     while let Some(open) = input[i..].find("{{") {
-        let start = i + open + 2;
-        match input[start..].find("}}") {
-            Some(close) => {
-                if input[start..start + close].trim() == name {
-                    return true;
-                }
-                i = start + close + 2;
-            }
-            None => break,
+        let marker_start = i + open;
+        let start = marker_start + 2;
+        if let Some(close) = input[start..].find("}}")
+            && input[start..start + close].trim() == name
+        {
+            return true;
         }
+        // Advance by one code-point past the `{{` (not past the closing
+        // `}}`) so nested `{{ ... }}` forms are still discovered. This keeps
+        // the scan consistent with `replace_marker`, which walks the input a
+        // code-point at a time; otherwise the two helpers could disagree on
+        // pathological doubly-nested input and leave a marker unsubstituted.
+        i = marker_start + 1;
     }
     false
 }
diff --git a/src/compile/path_layout_check.rs b/src/compile/path_layout_check.rs
index 5b90aee8..cb8f1465 100644
--- a/src/compile/path_layout_check.rs
+++ b/src/compile/path_layout_check.rs
@@ -150,10 +150,16 @@ fn sources_dir_segments(s: &str) -> Vec<String> {
             .chars()
             .take_while(|&c| c != '/' && !c.is_whitespace() && c != '"' && c != '\'' && c != '\\')
             .collect();
+        // Advance past the extracted segment. `i = start` (immediately after
+        // the prefix) would re-scan the extracted span on the next iteration
+        // for a back-to-back double prefix
+        // (`$(Build.SourcesDirectory)/$(Build.SourcesDirectory)/foo`), pushing
+        // the same span twice; `start + seg.len()` is the intended
+        // advancement and reports each segment once.
+        i = start + seg.len();
         if !seg.is_empty() {
             segs.push(seg);
         }
-        i = start;
     }
     segs
 }
@@ -284,6 +290,14 @@ mod tests {
             sources_dir_segments("$(Build.SourcesDirectory)/$(Build.Repository.Name)/x"),
             vec![SELF_REPO_SEGMENT.to_string()]
         );
+        // Back-to-back double prefix: the inner `$(Build.SourcesDirectory)`
+        // is extracted as the segment and the loop advances past it (rather
+        // than re-scanning from just after the outer prefix), so the segment
+        // is reported exactly once with no duplicate/re-extraction.
+        assert_eq!(
+            sources_dir_segments("$(Build.SourcesDirectory)/$(Build.SourcesDirectory)/foo"),
+            vec!["$(Build.SourcesDirectory)".to_string()]
+        );
     }
 
     #[test]

From c9a0da109f2af03f432a632a689e05e017a24ec3 Mon Sep 17 00:00:00 2001
From: James Devine <devinejames@microsoft.com>
Date: Wed, 1 Jul 2026 11:04:52 +0100
Subject: [PATCH 3/4] fix(compile): exclude ADO template exprs from legacy
 marker scan

Address second Rust PR review on #1263:

contains_template_marker and replace_marker now skip any {{ immediately preceded by a dollar sign, so an ADO template expression like ${{ workspace }} is neither detected as a legacy marker nor substituted (which would have produced a stray $ before the replacement).

Adds a regression test covering the exclusion for both helpers, and documents the deliberate narrowing of the step-path check to executable step blocks plus the empty-segment no-progress-risk invariant in sources_dir_segments.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 .../codemods/0004_legacy_path_markers.rs      | 28 ++++++++++++++++++-
 src/compile/common.rs                         |  9 +++++-
 src/compile/path_layout_check.rs              | 13 ++++++++-
 3 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/src/compile/codemods/0004_legacy_path_markers.rs b/src/compile/codemods/0004_legacy_path_markers.rs
index bb9d28db..629d646e 100644
--- a/src/compile/codemods/0004_legacy_path_markers.rs
+++ b/src/compile/codemods/0004_legacy_path_markers.rs
@@ -157,8 +157,14 @@ fn replace_marker(input: &str, name: &str, repl: &str) -> String {
     let mut i = 0;
     while i < input.len() {
         if input[i..].starts_with("{{") {
+            // Skip `${{ ... }}` (ADO template expression): it is not a legacy
+            // marker, and substituting it would leave a stray `$` before the
+            // replacement (e.g. `$$(Build.SourcesDirectory)`). Mirrors the
+            // guard in `contains_template_marker`.
+            let preceded_by_dollar = i > 0 && input.as_bytes()[i - 1] == b'$';
             let start = i + 2;
-            if let Some(close) = input[start..].find("}}")
+            if !preceded_by_dollar
+                && let Some(close) = input[start..].find("}}")
                 && input[start..start + close].trim() == name
             {
                 out.push_str(repl);
@@ -337,6 +343,26 @@ mod tests {
         assert!(!contains_template_marker("no markers here", "workspace"));
     }
 
+    #[test]
+    fn dollar_template_expression_is_not_a_marker() {
+        // `${{ workspace }}` is an ADO template expression, not a legacy
+        // marker: it must not be detected or substituted (substituting would
+        // leave a stray `$` before the replacement).
+        assert!(!contains_template_marker("${{ workspace }}", "workspace"));
+        assert!(!contains_template_marker("a ${{ workspace }} b", "workspace"));
+        assert_eq!(
+            replace_marker("${{ workspace }}", "workspace", "REPL"),
+            "${{ workspace }}"
+        );
+        // A bare (non-dollar) marker sitting alongside a `${{ }}` expression is
+        // still migrated.
+        assert!(contains_template_marker("${{ p }} {{ workspace }}", "workspace"));
+        assert_eq!(
+            replace_marker("${{ p }} {{ workspace }}", "workspace", "REPL"),
+            "${{ p }} REPL"
+        );
+    }
+
     #[test]
     fn nested_marker_detection_matches_replacement() {
         // Regression: `contains_template_marker` (the early-exit gate) and
diff --git a/src/compile/common.rs b/src/compile/common.rs
index fdad37a8..12a38ed9 100644
--- a/src/compile/common.rs
+++ b/src/compile/common.rs
@@ -886,7 +886,14 @@ pub(crate) fn contains_template_marker(input: &str, name: &str) -> bool {
     while let Some(open) = input[i..].find("{{") {
         let marker_start = i + open;
         let start = marker_start + 2;
-        if let Some(close) = input[start..].find("}}")
+        // Skip `${{ ... }}` (an ADO template expression), which is never a
+        // legacy marker even if its inner content trims to `name`. Matching it
+        // here would both raise a false positive and — in `replace_marker` —
+        // splice `repl` after the `$`, corrupting the output.
+        let preceded_by_dollar =
+            marker_start > 0 && input.as_bytes()[marker_start - 1] == b'$';
+        if !preceded_by_dollar
+            && let Some(close) = input[start..].find("}}")
             && input[start..start + close].trim() == name
         {
             return true;
diff --git a/src/compile/path_layout_check.rs b/src/compile/path_layout_check.rs
index cb8f1465..337d5291 100644
--- a/src/compile/path_layout_check.rs
+++ b/src/compile/path_layout_check.rs
@@ -61,6 +61,13 @@ pub fn collect_path_layout_warnings(front_matter: &FrontMatter, markdown_body: &
         .collect();
 
     // 1. `$(Build.SourcesDirectory)/<seg>` references in custom steps.
+    //
+    // Deliberately narrowed to the executable step blocks
+    // (`setup/steps/post_steps/teardown`). The codemod migrates every
+    // front-matter string scalar, but only step scalars are interpreted as
+    // filesystem paths at runtime, so a stray `$(Build.SourcesDirectory)/…`
+    // in a non-step field (e.g. `description:`) is harmless and intentionally
+    // not flagged here.
     let mut step_scalars: Vec<&str> = Vec::new();
     for block in [
         &front_matter.setup,
@@ -155,7 +162,11 @@ fn sources_dir_segments(s: &str) -> Vec<String> {
         // for a back-to-back double prefix
         // (`$(Build.SourcesDirectory)/$(Build.SourcesDirectory)/foo`), pushing
         // the same span twice; `start + seg.len()` is the intended
-        // advancement and reports each segment once.
+        // advancement and reports each segment once. When `seg` is empty (the
+        // char right after the prefix is a stopper like a space or quote) this
+        // is `start + 0 = start`, which still makes progress: the following
+        // `find` starts past the just-matched prefix and cannot re-match it,
+        // so there is no infinite-loop risk.
         i = start + seg.len();
         if !seg.is_empty() {
             segs.push(seg);

From fdb90b358be68d844802b718ffdf80d762442867 Mon Sep 17 00:00:00 2001
From: James Devine <devinejames@microsoft.com>
Date: Wed, 1 Jul 2026 11:16:53 +0100
Subject: [PATCH 4/4] refactor(compile): tighten visibility of crate-internal
 helpers

Address third Rust PR review on #1263: resolve_working_directory_expr and collect_path_layout_warnings are only used within the crate, so narrow them from pub to pub(crate) to keep the public API surface honest.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 src/compile/common.rs            | 2 +-
 src/compile/path_layout_check.rs | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/compile/common.rs b/src/compile/common.rs
index 12a38ed9..12880abb 100644
--- a/src/compile/common.rs
+++ b/src/compile/common.rs
@@ -801,7 +801,7 @@ pub fn compute_effective_workspace(
 /// no additional checkouts" advisory warning is intentionally dropped here;
 /// the normal typed compile path still surfaces it via
 /// [`compute_effective_workspace`].
-pub fn resolve_working_directory_expr(
+pub(crate) fn resolve_working_directory_expr(
     explicit_workspace: &Option<String>,
     checkout: &[String],
     agent_name: &str,
diff --git a/src/compile/path_layout_check.rs b/src/compile/path_layout_check.rs
index 337d5291..1d2843d6 100644
--- a/src/compile/path_layout_check.rs
+++ b/src/compile/path_layout_check.rs
@@ -47,7 +47,7 @@ const DEPRECATED_MARKERS: &[&str] = &["workspace", "working_directory", "trigger
 ///
 /// Warning-only: the returned strings are advisory. Returns an empty
 /// vector when nothing looks wrong. Messages are de-duplicated.
-pub fn collect_path_layout_warnings(front_matter: &FrontMatter, markdown_body: &str) -> Vec<String> {
+pub(crate) fn collect_path_layout_warnings(front_matter: &FrontMatter, markdown_body: &str) -> Vec<String> {
     let mut warnings: Vec<String> = Vec::new();
 
     let checked_out: Vec<&str> = front_matter.checkout.iter().map(String::as_str).collect();