feat: add runtime parameters support with auto-injected clearMemory

[jamesadevine]

Summary

Adds a parameters front matter field that emits Azure DevOps runtime parameters at the top of generated pipeline YAML. Parameters are surfaced in the ADO UI when manually queuing a run.

First use case: Clear Memory

When safe-outputs.memory is configured, a clearMemory boolean parameter (default: false) is automatically injected. Setting it to true when queuing a run skips downloading the previous memory artifact and starts the agent with a fresh memory directory.

Front matter syntax

parameters:
  - name: verbose
    displayName: "Verbose output"
    type: boolean
    default: false
  - name: region
    displayName: "Target region"
    type: string
    default: "us-east"
    values:
      - us-east
      - eu-west

Changes

• src/compile/types.rs — PipelineParameter struct + parameters field on FrontMatter
• src/compile/common.rs — generate_parameters() helper
• src/compile/standalone.rs — build_parameters() with clearMemory auto-injection + conditional memory download
• src/compile/onees.rs — wired parameters for 1ES target
• templates/base.yml & templates/1es-base.yml — {{ parameters }} template marker
• tests/compiler_tests.rs — 4 new integration tests (all 44 pass)
• AGENTS.md — documented parameters field, template marker, and clearMemory auto-injection

How it works

1. User-defined parameters are serialized to YAML and placed after name: in the pipeline
2. When safe-outputs.memory is configured, clearMemory (boolean, default false) is auto-prepended unless the user already defines one
3. Memory download/restore steps get condition: eq('${{ parameters.clearMemory }}', false)
4. A fresh-init step runs when clearMemory=true

[github-actions]

🔍 Rust PR Review

Summary: Looks good overall — clean design with one real bug, one error-handling gap, and a security concern worth addressing.

Findings

🐛 Bugs / Logic Issues

• src/compile/onees.rs:63 — The 1ES compiler calls generate_parameters(&front_matter.parameters) directly, bypassing build_parameters(). This means when safe-outputs.memory is configured on a target: 1es pipeline, the clearMemory parameter is never auto-injected. The standalone compiler correctly calls build_parameters() first. Either build_parameters() should be called here too (after extracting has_memory), or a comment should explicitly state that memory auto-injection is unsupported for 1ES.

• src/compile/common.rs (diff context) — The generate_parameters doc comment was inserted in the wrong position. The existing generate_schedule doc comments (/// Generate a schedule YAML block... / /// When no explicit schedule branches are configured...) now appear as the start of generate_parameters's doc block, and generate_schedule silently loses its documentation. This is cosmetic but actively misleading — the misattributed sentence refers to scheduling logic, not parameters.

🔒 Security Concerns

• src/compile/types.rs:229 — PipelineParameter::name accepts arbitrary strings with no validation. ADO parameter names must be valid identifiers ([A-Za-z_][A-Za-z0-9_]*). A user-supplied name containing ${{, spaces, or newlines would be serialized verbatim into the top-level parameters: block of the generated pipeline YAML, potentially injecting ADO template expressions or breaking YAML structure. Given that other generators in this codebase validate user-supplied values at compile time (e.g., tool names, env var names), name should be validated similarly — either at deserialization time or inside generate_parameters.

⚠️ Suggestions

• src/compile/common.rs:95,103 — generate_parameters returns String and uses .expect() on both serde_yaml::to_value(p) and serde_yaml::to_string(...). Every other fallible generate_* function in common.rs returns anyhow::Result<String>. While serialization of a known struct is extremely unlikely to panic in practice, this deviates from the project convention ("No unwrap()/expect() on user-facing paths") and silently makes both call-sites in standalone.rs and onees.rs infallible. Consider returning Result<String> and propagating with ?.

✅ What Looks Good

• The build_parameters() / generate_parameters() split is clean — injection logic stays in standalone.rs and YAML generation stays in common.rs.
• clearMemory deduplication by name (rather than position) is correct.
• The condition syntax eq('${{ parameters.clearMemory }}', false) is the canonical ADO pattern for template-parameter-driven step conditions — it works because ADO coerces the boolean to a string before comparison.
• serde_yaml::Value for default and values fields is the right call — it handles all ADO parameter types without special-casing.
• Test coverage is solid: the four integration tests cover the happy path, auto-injection, deduplication, and unreplaced-marker regression.

Generated by Rust PR Reviewer for issue #166 · ● 1.1M · ◷

[github-actions]

🔍 Rust PR Review

Summary: Looks good overall — well-structured feature with solid test coverage. Two minor issues worth addressing before merge.

---

Findings

⚠️ Suggestions

• src/compile/common.rs — generate_parameters() — .expect() on public code path

  .map(|p| serde_yaml::to_value(p).expect("PipelineParameter should serialize"))
  .expect("parameters should serialize to YAML");

  This function is pub and called from the compiler's hot path. While PipelineParameter contains only well-typed fields that should always serialize, the project convention (enforced everywhere else) is for fallible functions to return anyhow::Result. A future change that adds a field containing an un-serializable type (e.g., a non-serde-annotated type) would silently panic rather than surface an error to the user.

  Suggest changing the signature to pub fn generate_parameters(parameters: &[PipelineParameter]) -> Result<String> and using ? with .context("failed to serialize pipeline parameters").

• src/compile/types.rs — No validation on PipelineParameter::name

  The name field is pub name: String with no compile-time validation. An empty string, a name containing spaces, or a name starting with a digit are all silently accepted and will produce YAML that ADO rejects at pipeline registration time. The error message from ADO will be unhelpful.

  Suggest adding a compile-time check in build_parameters() / generate_parameters() that validates names against a simple regex ([A-Za-z_][A-Za-z0-9_]*) and emits a bail! if violated — consistent with how tool names are validated in validate_enabled_tool_names.

✅ What Looks Good

• Auto-injection deduplication logic is clean and correct — the !params.iter().any(|p| p.name == "clearMemory") guard correctly prevents duplication when the user provides their own definition.
• serde_yaml::Value for default and values is the right choice — it transparently handles all ADO parameter types (booleans, strings, lists, numbers) without needing a custom enum.
• replace_with_indent handles the empty string case correctly — when no parameters are defined, the marker is replaced with "" leaving a harmless blank line in the output.
• Condition syntax eq('${{ parameters.clearMemory }}', false) is the standard ADO pattern for boolean template parameters in step conditions. At pipeline parse time ${{ parameters.clearMemory }} expands to false/true, and ADO's condition evaluator coerces both sides to strings before comparison, so this works correctly.
• Test coverage is excellent — the four integration tests cover user-defined params, auto-injection, deduplication, and unreplaced-marker regression. The unreplaced-marker test in particular is good defensive coverage.
• 1ES path correctly skips clearMemory injection since the 1ES target doesn't support the memory feature, passing front_matter.parameters directly to generate_parameters is intentional and appropriate.

Generated by Rust PR Reviewer for issue #166 · ● 1.3M · ◷

[github-actions]

🔍 Rust PR Review

Summary: Looks good overall — well-structured feature with solid injection prevention, but one logic bug worth fixing and a couple of minor observations.

Findings

🐛 Bugs / Logic Issues

• src/compile/onees.rs:65-66 — clearMemory is auto-injected into 1ES pipelines when has_memory is true, but the 1ES template (templates/1es-base.yml) has no memory download steps that reference ${{ parameters.clearMemory }}. The parameter would appear in the ADO "Queue run" UI but have zero effect — potentially confusing to operators. The fix is to only pass has_memory: true into build_parameters for the standalone target, or to guard injection on whether the target actually emits memory steps:

  // onees.rs — memory isn't supported in 1ES; don't inject clearMemory
  let parameters = build_parameters(&front_matter.parameters, false);

⚠️ Suggestions

• src/compile/common.rs — reject_ado_expressions — The check guards against ${{ (template expressions) and $( (macro syntax) but misses $[ (runtime expression syntax). This is a minor gap in coverage; a user could theoretically embed $[variables.secret] in a displayName. Easy fix:

  if value.contains("${{") || value.contains("$(") || value.contains("$[") {

• src/compile/common.rs — generate_parameters indentation — The function uses format!("parameters:\n{}", yaml) where serde_yaml::to_string() emits list items at column 0 (- name: foo). The output is valid YAML, but the ADO convention is to indent list items under parameters:. This is cosmetic and doesn't affect correctness — just noting it differs from the example output shown in the docs, which shows indented items.

• src/compile/types.rs — no param_type validation — The param_type field accepts any string; an invalid ADO type (type: badvalue) will silently pass compilation and fail at ADO parse time. A compile-time warning (not an error) on unrecognised types would improve DX:

  const KNOWN_TYPES: &[&str] = &["boolean", "string", "number", "object", "step", "stepList", "job", "jobList", "deployment", "deploymentList", "stage", "stageList"];

✅ What Looks Good

• Injection prevention is thorough: parameter names are validated against [A-Za-z_][A-Za-z0-9_]*, and ${{/$( expressions are rejected in displayName, default, and values. The layered approach (name validation + expression rejection) is exactly right for user-controlled fields embedded in generated YAML.
• clearMemory deduplication correctly uses a name check (!params.iter().any(|p| p.name == "clearMemory")) and inserts at position 0, maintaining user-defined parameters at the end.
• Condition syntax eq(${{ parameters.clearMemory }}, false) is correct — the template expression expands at parse time to a boolean literal, which ADO's eq() condition function handles properly.
• Test coverage is comprehensive: unit tests for name validation + expression rejection, and 4 integration tests covering the full compilation round-trip including no-duplication and marker-exhaustion.
• PipelineParameter serde attributes are correct — #[serde(rename = "type")] on param_type properly handles both deserialization from front matter and serialization into the pipeline YAML.

Generated by Rust PR Reviewer for issue #166 · ● 1.8M · ◷