Skip to content

Commit 602c99b

Browse files
jamesadevineCopilot
andcommitted
refactor(ir): replace task helpers with typed builder structs
Replace the free-function ADO task helpers in src/compile/ir/tasks.rs (positional required args + untyped .with_input("camelKey","str") for optionals) with a tasks/ directory module of typed builder structs, one file per task. Each builder exposes new(<required>) + typed chained setters + into_step() -> TaskStep; only set fields emit inputs, with enums for constrained values and Option<bool> for bool-string inputs. Command/mode-dispatch tasks (Docker@2, DotNetCoreCLI@2, NuGetCommand@2, PowerShell@2) use a command enum with per-variant data so invalid input/command combinations are unrepresentable; tasks/docker.rs is the canonical template. Migrate the docker_installer call sites, update the ado-task-ir-contributor workflow + docs to the new convention. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 9f738c4 commit 602c99b

22 files changed

Lines changed: 2952 additions & 1534 deletions

.github/workflows/ado-task-ir-contributor.lock.yml

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

.github/workflows/ado-task-ir-contributor.md

Lines changed: 120 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
on:
33
schedule: every 4h
44
workflow_dispatch: {}
5-
description: Crawls Azure DevOps built-in task docs, contributes typed IR helper functions for uncovered tasks, converts compiler-generated Step::RawYaml usages to typed steps, and opens a focused PR per run.
5+
description: Crawls Azure DevOps built-in task docs, contributes typed IR builder structs for uncovered tasks, converts compiler-generated Step::RawYaml usages to typed steps, and opens a focused PR per run.
66
permissions:
77
contents: read
88
issues: read
@@ -42,9 +42,9 @@ The `ado-aw` compiler transforms agent markdown into Azure DevOps YAML through a
4242
- `Step::Checkout` / `Step::Download` / `Step::Publish` — special-purpose steps
4343
- `Step::RawYaml`**escape hatch** for user-authored YAML that the IR cannot model
4444

45-
The `TaskStep` struct is already fully generic — it accepts any `task: String` and an `inputs: IndexMap`. The gap is:
46-
1. **No typed factory functions** for most ADO built-in tasks — every place that uses a known ADO task must hand-craft `TaskStep::new("Foo@1", "display").with_input(...)`.
47-
2. **Compiler-generated `Step::RawYaml`** — occasionally, extension/runtime code emits `Step::RawYaml` for steps that could be expressed as `Step::Task` with a typed helper; these should be migrated.
45+
The `TaskStep` struct is fully generic — it accepts any `task: String` and an `inputs: IndexMap`. Typed coverage of specific ADO tasks lives in `src/compile/ir/tasks/` as **builder structs** (one submodule per task). Each builder exposes `new(<required>)`, one typed chained setter per optional input, and `into_step() -> TaskStep`; only inputs that were set are emitted. Constrained input values are typed enums (each with `as_ado_str()`), bool-string inputs take a Rust `bool`, and command/mode-dispatch tasks (`Docker@2`, `DotNetCoreCLI@2`, `NuGetCommand@2`, `PowerShell@2`) use a command enum with per-variant data so invalid input/command combinations are unrepresentable. `src/compile/ir/tasks/docker.rs` is the canonical template. The gaps are:
46+
1. **No typed builder** for most ADO built-in tasks — code that uses an uncovered task must hand-craft `TaskStep::new("Foo@1", "display").with_input(...)` with raw string keys.
47+
2. **Compiler-generated `Step::RawYaml`** — occasionally, extension/runtime code emits `Step::RawYaml` for steps that could be expressed as `Step::Task` with a typed builder; these should be migrated.
4848

4949
## Step 1 — Load Previous State
5050

@@ -67,10 +67,13 @@ If the prior PR is still open, emit `noop` with "Waiting on open PR #N before ad
6767

6868
## Step 2 — Audit Existing Typed Coverage
6969

70-
Examine what typed factory functions already exist:
70+
Examine which tasks already have a typed builder under `src/compile/ir/tasks/`:
7171

7272
```bash
73-
# Existing TaskStep factory functions in runtimes and extensions
73+
# One submodule per covered task; the struct + into_step are the builder.
74+
ls src/compile/ir/tasks/
75+
grep -rn "pub struct \|pub fn into_step" src/compile/ir/tasks/ --include="*.rs"
76+
# Other typed task factories that live outside tasks/ (runtimes / extensions)
7477
grep -rn "TaskStep::new\|fn.*task_step" src/runtimes src/compile/extensions --include="*.rs"
7578
```
7679

@@ -87,7 +90,7 @@ Focus on:
8790

8891
Ignore `push_raw_yaml_if_nonempty`, `step_to_raw_yaml_string`, and any function that explicitly handles user-authored YAML (these are legitimately `Step::RawYaml`).
8992

90-
Build a set of **already-typed tasks**: task identifiers that already have typed factory functions (e.g. `UseNode@1`, `UseDotNet@2`, `UsePythonVersion@0`, `NpmAuthenticate@0`, `PipAuthenticate@1`, `NuGetAuthenticate@1`).
93+
Build the set of **already-typed tasks**: the union of the task identifiers covered by `src/compile/ir/tasks/` builder structs and the runtime/extension factories (e.g. `UseNode@1`, `UseDotNet@2`, `UsePythonVersion@0`, `NpmAuthenticate@0`, `PipAuthenticate@1`, `NuGetAuthenticate@1`).
9194

9295
## Step 3 — Fetch ADO Built-In Task Catalog
9396

@@ -121,10 +124,12 @@ grep -rh "task:\|TaskStep::new" src tests --include="*.rs" --include="*.yml" --i
121124

122125
From the catalog, select **one** task that:
123126
1. Is **not** already in `completed_tasks` from state
124-
2. Is **not** already typed (no existing factory function)
127+
2. Is **not** already typed (no existing builder struct under `src/compile/ir/tasks/` and no runtime/extension factory)
125128
3. Has a stable, widely-used task identifier (prefer tasks in the `Utility`, `Build`, or `Test` categories)
126129
4. Has clear documentation on `learn.microsoft.com`
127130

131+
The chosen task will become a new submodule `src/compile/ir/tasks/<task_snake>.rs`.
132+
128133
Priority order (highest first):
129134
1. A `Step::RawYaml` in compiler-generated code that maps to a known ADO task — converting it to `Step::Task` removes tech debt directly.
130135
2. A task used frequently in the `tests/` fixtures as raw YAML strings.
@@ -149,67 +154,130 @@ Document the task:
149154
- Optional inputs with defaults
150155
- Relevant use cases in ado-aw
151156

152-
## Step 5 — Implement the Typed Helper
157+
## Step 5 — Implement the Typed Builder
153158

154-
Decide where to place the factory function:
159+
Decide where to place the builder:
155160

156-
- **If the task is tied to a specific runtime** (language install, package auth): add to `src/runtimes/<name>/mod.rs`.
157-
- **If the task is a general-purpose ADO built-in**: add to `src/compile/ir/step.rs` as a standalone constructor function alongside the existing step types, or create a new `src/compile/ir/tasks.rs` module if multiple new tasks are being added.
161+
- **If the task is tied to a specific runtime** (language install, package auth): add to `src/runtimes/<name>/mod.rs` (these remain free factory functions).
162+
- **If the task is a general-purpose ADO built-in**: create a **new submodule** `src/compile/ir/tasks/<task_snake>.rs` and declare it in `src/compile/ir/tasks/mod.rs` with `pub mod <task_snake>;` (alphabetical order). One file per task.
158163

159-
For a new `tasks.rs` module, create it at `src/compile/ir/tasks.rs` and add `pub mod tasks;` to `src/compile/ir/mod.rs`.
164+
### Builder struct shape
160165

161-
### Helper function shape
166+
Model after `src/compile/ir/tasks/copy_files.rs` (a single-mode task) and, for command/mode-dispatch tasks, `src/compile/ir/tasks/docker.rs` (the canonical command-enum template). Shared helpers (`bool_input`, `push_opt`, `push_bool`) live in `src/compile/ir/tasks/common.rs`.
162167

163-
Model after existing patterns in `src/runtimes/`:
168+
A builder is a struct with the required inputs as fields, each optional input as an `Option<…>` field, and a `display_name: Option<String>` override:
164169

165170
```rust
166-
/// Returns a [`TaskStep`] for `CopyFiles@2`.
167-
///
168-
/// Copies files matching `contents` from `source_folder` to `target_folder`.
169-
/// All parameters map directly to the ADO task inputs.
170-
pub fn copy_files_step(
171-
source_folder: impl Into<String>,
172-
contents: impl Into<String>,
173-
target_folder: impl Into<String>,
174-
) -> TaskStep {
175-
TaskStep::new("CopyFiles@2", "Copy Files")
176-
.with_input("SourceFolder", source_folder)
177-
.with_input("Contents", contents)
178-
.with_input("TargetFolder", target_folder)
171+
use super::common::bool_input;
172+
use crate::compile::ir::step::TaskStep;
173+
174+
/// Builder for a [`TaskStep`] invoking `CopyFiles@2`.
175+
#[derive(Debug, Clone)]
176+
pub struct CopyFiles {
177+
contents: String,
178+
target_folder: String,
179+
source_folder: Option<String>,
180+
clean_target_folder: Option<bool>,
181+
display_name: Option<String>,
182+
}
183+
184+
impl CopyFiles {
185+
/// Required inputs are positional parameters of `new`.
186+
pub fn new(contents: impl Into<String>, target_folder: impl Into<String>) -> Self {
187+
Self {
188+
contents: contents.into(),
189+
target_folder: target_folder.into(),
190+
source_folder: None,
191+
clean_target_folder: None,
192+
display_name: None,
193+
}
194+
}
195+
196+
/// One typed chained setter per optional input.
197+
pub fn source_folder(mut self, value: impl Into<String>) -> Self {
198+
self.source_folder = Some(value.into());
199+
self
200+
}
201+
202+
/// Bool-string inputs take a Rust `bool`.
203+
pub fn clean_target_folder(mut self, value: bool) -> Self {
204+
self.clean_target_folder = Some(value);
205+
self
206+
}
207+
208+
/// Always provide a `displayName` override.
209+
pub fn with_display_name(mut self, value: impl Into<String>) -> Self {
210+
self.display_name = Some(value.into());
211+
self
212+
}
213+
214+
/// `into_step` emits required inputs always, optionals only when set.
215+
pub fn into_step(self) -> TaskStep {
216+
let mut t = TaskStep::new(
217+
"CopyFiles@2",
218+
self.display_name.unwrap_or_else(|| "Copy Files".into()),
219+
)
220+
.with_input("Contents", self.contents)
221+
.with_input("TargetFolder", self.target_folder);
222+
if let Some(v) = self.source_folder {
223+
t = t.with_input("SourceFolder", v);
224+
}
225+
if let Some(v) = self.clean_target_folder {
226+
t = t.with_input("CleanTargetFolder", bool_input(v));
227+
}
228+
t
229+
}
179230
}
180231
```
181232

182233
Guidelines:
183-
- Function name: snake_case, derived from the task display name, without the version suffix (e.g. `copy_files_step`, `publish_test_results_step`).
184-
- Required inputs: positional parameters.
185-
- Optional inputs with common defaults: keyword-style builder on the returned `TaskStep` (use `.with_input` after the initial construction).
186-
- Include a doc comment with the task identifier and a one-line description.
187-
- Do NOT add a new `Step` enum variant for standard tasks — `Step::Task(TaskStep)` is the correct representation.
234+
- Struct name: PascalCase of the task display name, without the version suffix (e.g. `CopyFiles`, `PublishTestResults`).
235+
- **Required inputs** → positional parameters of `new`.
236+
- **Optional inputs**`Option<…>` fields with one typed chained setter each; only emit them in `into_step` when set.
237+
- **Constrained values** (a fixed set of string tokens) → a typed enum with `as_ado_str(&self) -> &'static str` returning the exact ADO token. Colocate the enum with the task (reusable shared enums may live in `common.rs`).
238+
- **Bool-string inputs**`Option<bool>`; lower via `bool_input`.
239+
- **Command/mode-dispatch tasks** (each command exposes a different optional-input set) → a command **enum with per-variant data** so invalid input/command combos are unrepresentable. Wrap it in a builder struct (`new(<Command>)` plus per-command constructors) and match the variant in `into_step`. Model on `src/compile/ir/tasks/docker.rs`.
240+
- Include a doc comment with the task identifier and the ADO task reference URL.
241+
- Do NOT add a new `Step` enum variant for standard tasks — `Step::Task(<builder>.into_step())` is the correct representation.
188242

189243
### Convert RawYaml if applicable
190244

191245
If Step 4 identified a `Step::RawYaml` in compiler code that this task covers, replace it now:
192246

193247
```rust
194248
// Before:
195-
steps.push(Step::RawYaml(format!("- task: CopyFiles@2\n inputs:\n SourceFolder: {src}\n TargetFolder: {dst}")));
249+
steps.push(Step::RawYaml(format!("- task: CopyFiles@2\n inputs:\n Contents: '**'\n TargetFolder: {dst}")));
196250

197251
// After:
198-
use crate::compile::ir::tasks::copy_files_step;
199-
steps.push(Step::Task(copy_files_step(&src, "**", &dst)));
252+
use crate::compile::ir::tasks::copy_files::CopyFiles;
253+
steps.push(Step::Task(CopyFiles::new("**", &dst).into_step()));
200254
```
201255

202256
## Step 6 — Add Tests
203257

204-
Add at least one unit test to the same file (or `tests/compiler_tests.rs` for integration tests):
258+
Add at least one `#[cfg(test)] mod tests` unit test to the new task submodule (or `tests/compiler_tests.rs` for integration tests), building via the struct and asserting on `task` / `inputs`:
205259

206260
```rust
207-
#[test]
208-
fn copy_files_step_creates_task_with_inputs() {
209-
let t = copy_files_step("$(Build.SourcesDirectory)", "**/*.rs", "$(Build.ArtifactStagingDirectory)");
210-
assert_eq!(t.task, "CopyFiles@2");
211-
assert_eq!(t.inputs.get("SourceFolder").map(|s| s.as_str()), Some("$(Build.SourcesDirectory)"));
212-
assert_eq!(t.inputs.get("TargetFolder").map(|s| s.as_str()), Some("$(Build.ArtifactStagingDirectory)"));
261+
#[cfg(test)]
262+
mod tests {
263+
use super::*;
264+
265+
#[test]
266+
fn creates_task_with_inputs() {
267+
let t = CopyFiles::new("**/*.rs", "$(Build.ArtifactStagingDirectory)")
268+
.source_folder("$(Build.SourcesDirectory)")
269+
.into_step();
270+
assert_eq!(t.task, "CopyFiles@2");
271+
assert_eq!(t.inputs.get("Contents").map(String::as_str), Some("**/*.rs"));
272+
assert_eq!(
273+
t.inputs.get("TargetFolder").map(String::as_str),
274+
Some("$(Build.ArtifactStagingDirectory)")
275+
);
276+
assert_eq!(
277+
t.inputs.get("SourceFolder").map(String::as_str),
278+
Some("$(Build.SourcesDirectory)")
279+
);
280+
}
213281
}
214282
```
215283

@@ -260,27 +328,29 @@ jq \
260328
If changes were made, open a PR with:
261329

262330
**Title** — conventional commits format:
263-
- `feat(ir): add typed helper for <TaskName@version>` — for new factory functions
331+
- `feat(ir): add typed builder for <TaskName@version>` — for a new builder struct
264332
- `refactor(ir): replace RawYaml with typed TaskStep for <TaskName@version>` — for RawYaml conversions
265-
- `feat(ir): add tasks module with typed helpers for <TaskA> and <TaskB>` — if a new module is created
266333

267334
**Body**:
268335

269336
```markdown
270337
## Summary
271338

272-
Adds a typed factory function for `<TaskName@version>` to the ado-aw IR.
339+
Adds a typed builder struct for `<TaskName@version>` to the ado-aw IR.
273340

274341
## Motivation
275342

276343
Previously, any code that needed to emit this ADO task step had to hand-craft
277-
`TaskStep::new(...)` with raw string inputs. This PR introduces a well-typed
278-
helper that validates required inputs at the call site and provides a clear API.
344+
`TaskStep::new(...)` with raw string input keys. This PR introduces a typed
345+
builder struct (`new(<required>)` + typed optional setters + `into_step()`) so
346+
required inputs are positional, optional inputs and their constrained values are
347+
type-checked, and call sites stop using stringly-typed keys.
279348

280349
## Changes
281350

282-
- `src/compile/ir/tasks.rs` (or relevant file): `<fn_name>()` factory function
283-
- `tests/...`: unit tests for the new helper
351+
- `src/compile/ir/tasks/<task_snake>.rs`: new `<TaskName>` builder struct (+ any
352+
typed enums) and its `#[cfg(test)] mod tests`
353+
- `src/compile/ir/tasks/mod.rs`: `pub mod <task_snake>;` declaration
284354
- (if applicable) `src/compile/...`: converted `Step::RawYaml``Step::Task`
285355

286356
## ADO Task Reference

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ Every compiled pipeline runs as three sequential jobs:
5555
│ │ │ ├── mod.rs # IR module entry point and shared types
5656
│ │ │ ├── ids.rs # Stable IDs for jobs/steps/outputs in the IR
5757
│ │ │ ├── step.rs # Step declarations and typed step variants
58-
│ │ │ ├── tasks.rs # Typed factory helpers for built-in ADO TaskStep creation
58+
│ │ │ ├── tasks/ # Typed builder structs for built-in ADO tasks (one file per task; new()+typed setters+into_step(); command-enum dispatch for Docker/DotNet/NuGet/PowerShell; docker.rs canonical template)
5959
│ │ │ ├── job.rs # Job declarations and typed job graph nodes
6060
│ │ │ ├── stage.rs # Stage declarations and typed stage graph nodes
6161
│ │ │ ├── env.rs # Typed environment and variable modeling

0 commit comments

Comments
 (0)