Skip to content

Commit 7806369

Browse files
CopilotjamesadevineCopilot
authored
feat(compile): unify pool front-matter replacement across targets (#538)
* feat(compile): support vmImage pool syntax for non-1ES targets Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/ae57b6df-ffaf-4666-9284-28c65c7f2ced Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com> * fix(compile): simplify non-1ES pool value wiring Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/ae57b6df-ffaf-4666-9284-28c65c7f2ced Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com> * refactor(compile): unify pool marker across templates Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/a32e578f-2dbe-490f-b52c-2380cf007de9 Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com> * fix(codemods): translate legacy default pool for non-1es Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/43e0b1b8-982d-4e55-915d-95a8474898ab Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com> * fix(codemods): keep scalar pool migration as name mapping Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/ba1c7870-b00a-4daa-9761-7f9f369222a6 Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com> * fix(compile): version-gate legacy pool default injection in pool_object_form codemod When pool is absent from front matter and the binary version is at or above 0.30.0 (the release that changed the implicit default from the 1ES self-hosted pool to vmImage: ubuntu-latest), the codemod now injects pool: { name: AZS-1ES-L-MMS-ubuntu-22.04 } to prevent silent breakage of existing pipelines. - Add compiler_version field to CodemodContext - Gate absent-pool injection on version >= introduced_in - Add version_gte helper for semver comparison - Add tests for version-gated and version-ungated behaviour Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com> Co-authored-by: James Devine <devinejames@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent dcb2b33 commit 7806369

14 files changed

Lines changed: 399 additions & 66 deletions

docs/front-matter.md

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,11 @@ engine: copilot # Engine identifier. Defaults to copilot. Currently only 'copilo
1717
# model: claude-opus-4.7
1818
# timeout-minutes: 30
1919
workspace: repo # Optional: "root", "repo" (alias: "self"), or a checked-out repository alias. If not specified, defaults to "root" when no additional repositories are listed in `repos:`, and to "repo" when one or more additional repos are checked out. See "Workspace Defaults" below.
20-
pool: AZS-1ES-L-MMS-ubuntu-22.04 # Agent pool name (string format). Defaults to AZS-1ES-L-MMS-ubuntu-22.04.
21-
# pool: # Alternative object format (required for 1ES if specifying os)
20+
pool: # Optional pool configuration
21+
vmImage: ubuntu-latest # Microsoft-hosted (default for non-1ES targets)
22+
# pool: # Self-hosted pool
23+
# name: MySelfHostedPool
24+
# pool: # 1ES pool format
2225
# name: AZS-1ES-L-MMS-ubuntu-22.04
2326
# os: linux # Operating system: "linux" or "windows". Defaults to "linux".
2427
repos: # compact repository declarations (replaces repositories: + checkout:)

docs/targets.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ The output contains the same 3-job chain (Agent → Detection → Execution) as
4242
`standalone`, with:
4343
- Job names prefixed with the agent name for uniqueness (e.g., `DailyReview_Agent`)
4444
- No triggers, pipeline name, or resource declarations (the parent pipeline owns those)
45-
- Pool baked in from the front matter `pool:` field
45+
- Pool baked in from the front matter `pool:` field (`vmImage` or `name`; defaults to `vmImage: ubuntu-latest`)
4646

4747
Example front matter:
4848
```yaml

docs/template-markers.md

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -152,13 +152,13 @@ Should be replaced with the engine's log directory path, generated by `Engine::l
152152

153153
## {{ pool }}
154154

155-
Should be replaced with the agent pool name from the `pool` front matter field. Defaults to `AZS-1ES-L-MMS-ubuntu-22.04` if not specified.
155+
Used by all templates under a `pool:` block and expands to:
156+
- non-1ES targets: one line (`vmImage: <image>` or `name: <pool>`)
157+
- 1ES target: two lines (`name: <pool>` and `os: <linux|windows>`)
156158

157-
The pool configuration accepts both string and object formats:
158-
- **String format**: `pool: AZS-1ES-L-MMS-ubuntu-22.04`
159-
- **Object format**: `pool: { name: AZS-1ES-L-MMS-ubuntu-22.04, os: linux }`
160-
161-
The `os` field (defaults to "linux") is primarily used for 1ES target compatibility.
159+
Defaults:
160+
- non-1ES: `vmImage: ubuntu-latest`
161+
- 1ES: `name: AZS-1ES-L-MMS-ubuntu-22.04` + `os: linux`
162162

163163
## {{ setup_job }}
164164

src/compile/codemods/0001_repos_unified.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -242,14 +242,14 @@ mod tests {
242242

243243
fn run(input: &str) -> Mapping {
244244
let mut m: Mapping = serde_yaml::from_str(input).unwrap();
245-
let changed = apply_codemod(&mut m, &CodemodContext {}).expect("apply");
245+
let changed = apply_codemod(&mut m, &CodemodContext::current()).expect("apply");
246246
assert!(changed, "expected codemod to fire on input:\n{}", input);
247247
m
248248
}
249249

250250
fn run_noop(input: &str) -> Mapping {
251251
let mut m: Mapping = serde_yaml::from_str(input).unwrap();
252-
let changed = apply_codemod(&mut m, &CodemodContext {}).expect("apply");
252+
let changed = apply_codemod(&mut m, &CodemodContext::current()).expect("apply");
253253
assert!(!changed, "expected codemod to be a no-op on input:\n{}", input);
254254
m
255255
}
@@ -258,7 +258,7 @@ mod tests {
258258
let mut m: Mapping = serde_yaml::from_str(input).unwrap();
259259
format!(
260260
"{}",
261-
apply_codemod(&mut m, &CodemodContext {}).unwrap_err()
261+
apply_codemod(&mut m, &CodemodContext::current()).unwrap_err()
262262
)
263263
}
264264

@@ -425,10 +425,10 @@ mod tests {
425425
checkout: [a]\n",
426426
)
427427
.unwrap();
428-
let first = apply_codemod(&mut m, &CodemodContext {}).expect("first");
428+
let first = apply_codemod(&mut m, &CodemodContext::current()).expect("first");
429429
assert!(first, "first run should fire");
430430
let snapshot = m.clone();
431-
let second = apply_codemod(&mut m, &CodemodContext {}).expect("second");
431+
let second = apply_codemod(&mut m, &CodemodContext::current()).expect("second");
432432
assert!(!second, "second run should be a no-op");
433433
assert_eq!(m, snapshot, "second run must not mutate");
434434
}
Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
//! `pool: <string>` → explicit object form
2+
//!
3+
//! Non-1ES targets now support both self-hosted (`name`) and
4+
//! Microsoft-hosted (`vmImage`) pool syntax. This codemod rewrites the
5+
//! legacy scalar shorthand into an explicit object form so sources are
6+
//! unambiguous and easier to evolve.
7+
//!
8+
//! When the pool field is **absent** and the compiler version is at or
9+
//! above `INTRODUCED_IN` (the release that changed the implicit
10+
//! default from the 1ES self-hosted pool to `vmImage: ubuntu-latest`),
11+
//! the codemod pins the legacy default explicitly so existing
12+
//! pipelines are not silently broken.
13+
14+
use anyhow::Result;
15+
use serde_yaml::{Mapping, Value};
16+
17+
use super::{Codemod, CodemodContext};
18+
use crate::compile::common::DEFAULT_ONEES_POOL;
19+
20+
/// Version where the pool default changed from the legacy self-hosted
21+
/// pool to `vmImage: ubuntu-latest`.
22+
const INTRODUCED_IN: &str = "0.30.0";
23+
24+
pub static CODEMOD: Codemod = Codemod {
25+
id: "pool_object_form",
26+
summary: "pool: <string> -> pool object form (name/vmImage)",
27+
introduced_in: INTRODUCED_IN,
28+
apply: apply_codemod,
29+
};
30+
31+
/// Simple major.minor.patch comparison. Returns true when `version`
32+
/// is greater than or equal to `threshold`.
33+
fn version_gte(version: &str, threshold: &str) -> bool {
34+
let parse = |s: &str| -> (u32, u32, u32) {
35+
let mut parts = s.split('.');
36+
let major = parts.next().and_then(|p| p.parse().ok()).unwrap_or(0);
37+
let minor = parts.next().and_then(|p| p.parse().ok()).unwrap_or(0);
38+
let patch = parts.next().and_then(|p| p.parse().ok()).unwrap_or(0);
39+
(major, minor, patch)
40+
};
41+
parse(version) >= parse(threshold)
42+
}
43+
44+
fn apply_codemod(fm: &mut Mapping, ctx: &CodemodContext) -> Result<bool> {
45+
let key = Value::String("pool".to_string());
46+
47+
let Some(pool_value) = fm.get(&key).cloned() else {
48+
// Pool absent — only inject the legacy default when the
49+
// compiler version is at or above the release that changed
50+
// the implicit default. Older binaries still carry the old
51+
// default in `resolve_pool_block`, so no rewrite is needed.
52+
if !version_gte(ctx.compiler_version, INTRODUCED_IN) {
53+
return Ok(false);
54+
}
55+
let mut mapped = Mapping::new();
56+
mapped.insert(
57+
Value::String("name".to_string()),
58+
Value::String(DEFAULT_ONEES_POOL.to_string()),
59+
);
60+
fm.insert(key, Value::Mapping(mapped));
61+
return Ok(true);
62+
};
63+
64+
let Value::String(name) = pool_value else {
65+
// Already object-form (or invalid in another way) — no-op.
66+
return Ok(false);
67+
};
68+
69+
let mut mapped = Mapping::new();
70+
mapped.insert(Value::String("name".to_string()), Value::String(name));
71+
fm.insert(key, Value::Mapping(mapped));
72+
Ok(true)
73+
}
74+
75+
#[cfg(test)]
76+
mod tests {
77+
use super::*;
78+
79+
/// Build a context with an explicit version for testing.
80+
fn ctx(version: &'static str) -> CodemodContext {
81+
CodemodContext {
82+
compiler_version: version,
83+
}
84+
}
85+
86+
#[test]
87+
fn rewrites_scalar_pool_to_name_object() {
88+
let mut fm: Mapping = serde_yaml::from_str("name: x\ndescription: y\npool: MyPool").unwrap();
89+
let changed = apply_codemod(&mut fm, &ctx("0.30.0")).expect("apply");
90+
assert!(changed);
91+
assert_eq!(
92+
fm.get(Value::String("pool".into())).cloned(),
93+
Some(serde_yaml::from_str::<Value>("name: MyPool").unwrap())
94+
);
95+
}
96+
97+
#[test]
98+
fn noops_when_pool_is_already_mapping() {
99+
let mut fm: Mapping =
100+
serde_yaml::from_str("name: x\ndescription: y\npool:\n vmImage: ubuntu-latest")
101+
.unwrap();
102+
let changed = apply_codemod(&mut fm, &ctx("0.30.0")).expect("apply");
103+
assert!(!changed);
104+
assert_eq!(
105+
fm.get(Value::String("pool".into())).cloned(),
106+
Some(serde_yaml::from_str::<Value>("vmImage: ubuntu-latest").unwrap())
107+
);
108+
}
109+
110+
#[test]
111+
fn inserts_legacy_default_when_pool_absent_and_version_gte() {
112+
let mut fm: Mapping = serde_yaml::from_str("name: x\ndescription: y").unwrap();
113+
let changed = apply_codemod(&mut fm, &ctx("0.30.0")).expect("apply");
114+
assert!(changed);
115+
assert_eq!(
116+
fm.get(Value::String("pool".into())).cloned(),
117+
Some(serde_yaml::from_str::<Value>("name: AZS-1ES-L-MMS-ubuntu-22.04").unwrap())
118+
);
119+
}
120+
121+
#[test]
122+
fn noops_when_pool_absent_and_version_below() {
123+
let mut fm: Mapping = serde_yaml::from_str("name: x\ndescription: y").unwrap();
124+
let changed = apply_codemod(&mut fm, &ctx("0.29.0")).expect("apply");
125+
assert!(!changed);
126+
assert!(!fm.contains_key(Value::String("pool".into())));
127+
}
128+
129+
#[test]
130+
fn idempotent_after_inserting_legacy_default() {
131+
let mut fm: Mapping = serde_yaml::from_str("name: x\ndescription: y").unwrap();
132+
let changed1 = apply_codemod(&mut fm, &ctx("0.30.0")).expect("first apply");
133+
assert!(changed1);
134+
let changed2 = apply_codemod(&mut fm, &ctx("0.30.0")).expect("second apply");
135+
assert!(!changed2, "second run must be a no-op");
136+
}
137+
138+
#[test]
139+
fn rewrites_legacy_default_pool_to_name_object() {
140+
let mut fm: Mapping =
141+
serde_yaml::from_str("name: x\ndescription: y\npool: AZS-1ES-L-MMS-ubuntu-22.04")
142+
.unwrap();
143+
let changed = apply_codemod(&mut fm, &ctx("0.30.0")).expect("apply");
144+
assert!(changed);
145+
assert_eq!(
146+
fm.get(Value::String("pool".into())).cloned(),
147+
Some(serde_yaml::from_str::<Value>("name: AZS-1ES-L-MMS-ubuntu-22.04").unwrap())
148+
);
149+
}
150+
151+
#[test]
152+
fn scalar_rewrite_applies_regardless_of_version() {
153+
// Scalar → object rewrite is unconditional; only the
154+
// absent-pool injection is version-gated.
155+
let mut fm: Mapping =
156+
serde_yaml::from_str("name: x\ndescription: y\npool: MyPool").unwrap();
157+
let changed = apply_codemod(&mut fm, &ctx("0.28.0")).expect("apply");
158+
assert!(changed);
159+
assert_eq!(
160+
fm.get(Value::String("pool".into())).cloned(),
161+
Some(serde_yaml::from_str::<Value>("name: MyPool").unwrap())
162+
);
163+
}
164+
165+
#[test]
166+
fn version_gte_comparisons() {
167+
assert!(version_gte("0.30.0", "0.30.0"));
168+
assert!(version_gte("0.31.0", "0.30.0"));
169+
assert!(version_gte("1.0.0", "0.30.0"));
170+
assert!(version_gte("0.30.1", "0.30.0"));
171+
assert!(!version_gte("0.29.0", "0.30.0"));
172+
assert!(!version_gte("0.29.99", "0.30.0"));
173+
}
174+
}

src/compile/codemods/mod.rs

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,16 +35,32 @@ use serde_yaml::Mapping;
3535
mod helpers;
3636
#[path = "0001_repos_unified.rs"]
3737
mod m0001_repos_unified;
38+
#[path = "0002_pool_object_form.rs"]
39+
mod m0002_pool_object_form;
3840

3941
#[allow(unused_imports)] // Re-exported for future codemods; only `take_key` is in-tree use.
4042
pub use helpers::{insert_no_overwrite, rename_key, take_key, ConflictPolicy};
4143

42-
/// Forward-compatible context passed to every codemod. Currently
43-
/// empty; we keep it in the signature so future codemods can be
44-
/// given (e.g.) the source path without breaking the function
45-
/// pointer type.
44+
/// Forward-compatible context passed to every codemod.
45+
///
46+
/// Carries ambient information (e.g. compiler version) so codemods
47+
/// can condition their behaviour without hard-coding assumptions.
4648
#[non_exhaustive]
47-
pub struct CodemodContext {}
49+
pub struct CodemodContext {
50+
/// Semantic version of the running `ado-aw` binary
51+
/// (e.g. `"0.30.0"`). Codemods can compare this against their
52+
/// `introduced_in` to decide when a default has changed.
53+
pub compiler_version: &'static str,
54+
}
55+
56+
impl CodemodContext {
57+
/// Build a context using the compile-time package version.
58+
pub fn current() -> Self {
59+
Self {
60+
compiler_version: env!("CARGO_PKG_VERSION"),
61+
}
62+
}
63+
}
4864

4965
/// A single front-matter codemod.
5066
///
@@ -85,6 +101,7 @@ pub struct Codemod {
85101
/// without harm.
86102
pub static CODEMODS: &[&'static Codemod] = &[
87103
&m0001_repos_unified::CODEMOD,
104+
&m0002_pool_object_form::CODEMOD,
88105
];
89106

90107
/// Result of running the codemod registry on a single front-matter
@@ -138,10 +155,11 @@ pub(crate) fn apply_codemods_with(
138155
fm: &mut Mapping,
139156
registry: &[&'static Codemod],
140157
) -> Result<CodemodReport> {
158+
let ctx = CodemodContext::current();
141159
let mut applied: Vec<AppliedCodemod> = Vec::new();
142160
for c in registry {
143-
let changed = (c.apply)(fm, &CodemodContext {})
144-
.with_context(|| format!("codemod {} failed", c.id))?;
161+
let changed =
162+
(c.apply)(fm, &ctx).with_context(|| format!("codemod {} failed", c.id))?;
145163
if changed {
146164
applied.push(AppliedCodemod {
147165
id: c.id,

0 commit comments

Comments
 (0)