diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index aadb0159..c534ab71 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -73,7 +73,7 @@ jobs: run: | set -euo pipefail cd scripts - zip -r ../ado-script.zip ado-script/gate.js ado-script/import.js ado-script/exec-context-pr.js + zip -r ../ado-script.zip ado-script/gate.js ado-script/import.js ado-script/exec-context-pr.js ado-script/exec-context-pr-synth.js - name: Upload release assets env: diff --git a/docs/front-matter.md b/docs/front-matter.md index 5982c3df..8b9e9ae7 100644 --- a/docs/front-matter.md +++ b/docs/front-matter.md @@ -104,6 +104,24 @@ on: # trigger configuration (unified under on: key) include: [main] paths: include: [src/*] + mode: synthetic # synthetic (default) | policy. Controls how + # `on.pr` builds reach the pipeline. + # - synthetic: a Setup-job script calls the + # ADO REST API on every CI build, finds the + # open PR for `Build.SourceBranch`, and + # promotes the build to PR semantics if it + # matches `branches`/`paths`. No Build + # Validation branch policy required. Zero + # or multiple matches → Agent job + # self-skips cleanly. CI trigger stays at + # the ADO default (all branches). + # - policy: the operator has installed a + # Build Validation branch policy. Compiler + # omits all synth wiring AND emits + # `trigger: none` so feature-branch pushes + # do not queue duplicate CI builds. Real + # PR-typed builds drive everything. + # See "PR Triggering in Azure Repos" below. filters: # runtime PR filters (compiled to gate step) title: "*[review]*" author: @@ -328,3 +346,78 @@ The filter gate step uses `System.AccessToken` for self-cancellation If the token is unavailable, the gate step logs a warning and the build completes as "Succeeded" (with the agent job skipped via condition) rather than "Cancelled". + +## PR Triggering in Azure Repos + +Azure DevOps Services **ignores the YAML `pr:` block unless a per-branch +Build Validation branch policy is registered server-side**. Without that +policy, a `git push` to a feature branch fires the compiled pipeline as +`Build.Reason = IndividualCI` even when an open PR exists — the gate +evaluator's "not a PR build" bypass triggers and `exec-context-pr.js` +is skipped. PR-aware agents (e.g. PR reviewers) silently degrade. + +`ado-aw` lets the agent author pick one of two coherent strategies via +`on.pr.mode`: + +| `on.pr.mode` | Synthesis wiring | Top-level `trigger:` | Use when | +|---|---|---|---| +| `synthetic` (default) | emitted (synthPr Setup step, coalesced env, broadened conditions) | ADO default (all branches) | No branch policy. **The vast majority of agents.** | +| `policy` | omitted | `trigger: none` | Operator has installed a Build Validation branch policy and wants real PR-typed builds only, no duplicate CI builds. | + +### `mode: synthetic` — how it works under the hood + +On every CI build: + +1. **Real PR build?** If `Build.Reason == PullRequest` (a branch policy + is configured), the synth step no-ops and the existing PR path + handles everything. +2. **GitHub-typed repo resource?** GitHub repos already get correct + `pr:` semantics from ADO. The synth step no-ops. +3. **Look up the PR.** Otherwise, the script calls + `GET /{project}/_apis/git/repositories/{repoId}/pullrequests` + filtered by `sourceRefName == Build.SourceBranch` and + `status = active`. +4. **Filter by target branch.** PRs whose `targetRefName` does not match + `on.pr.branches.include` (respecting `exclude`) are dropped. +5. **Exactly one match.** Zero or multiple matches → emit + `AW_SYNTHETIC_PR_SKIP=true`; the Agent job self-skips cleanly with a + single info log line. Never noisy, never red. +6. **Path filter.** If `on.pr.paths` is configured, the script enforces + it against the PR's changed-file list (which ADO's CI trigger + ignores). Empty intersection → skip. +7. **Promote.** Otherwise, emit `AW_SYNTHETIC_PR=true` plus the PR + identifiers as Setup-job outputs. Downstream `gate.js` and + `exec-context-pr.js` env blocks coalesce these with the real + `System.PullRequest.*` variables, so the gate evaluator runs the + full PR-spec predicates and `aw-context/pr/{base.sha,head.sha}` is + staged for the agent. + +### Why the CI trigger is not auto-narrowed in `mode: synthetic` + +`pr.branches.include` lists PR **target** branches (e.g. `main`), but +ADO `trigger:` fires on pushes **to** the listed branches. Narrowing +`trigger:` to `pr.branches.include` would suppress CI on the feature +branches synthPr actually needs to react to (pushing to `feature/x` +with an open PR `feature/x → main` would never queue a build). The +compiler therefore leaves the top-level `trigger:` at the ADO default +("trigger on every branch") in synth mode, and relies on the synthPr +Setup step's fast-exit for cost control: a single +`listActivePullRequestsBySourceRef` call returns `[]` on branches +without a matching PR and the Agent job self-skips cleanly via +`AW_SYNTHETIC_PR_SKIP=true`. + +### `mode: policy` — when to choose it + +Choose `mode: policy` when the operator has explicitly installed an +Azure DevOps Build Validation branch policy targeting the compiled +pipeline. In this mode the compiler: + +- Omits all synth wiring (`synthPr` step, `PR_SYNTH_SPEC` env, + `AW_SYNTHETIC_PR_SKIP` guard, coalesced env macros, broadened + `exec-context-pr.js` condition). +- Emits `trigger: none` so feature-branch pushes do not queue + duplicate CI builds alongside the policy-driven PR build. + +Result: every PR update fires exactly one PR-typed build (`Build.Reason +== PullRequest`); commit-driven CI is fully silenced. + diff --git a/prompts/create-ado-agentic-workflow.md b/prompts/create-ado-agentic-workflow.md index 935bde0c..ffe780c2 100644 --- a/prompts/create-ado-agentic-workflow.md +++ b/prompts/create-ado-agentic-workflow.md @@ -429,6 +429,8 @@ on: When `on.pr` is set: the native ADO `pr:` trigger block is generated from `branches:` and `paths:`. Runtime `filters:` compile to a gate step in the Setup job that self-cancels the build when they do not match. +**`on.pr` triggering works without a Build Validation branch policy.** By default (`mode: synthetic`), the compiler emits a Setup-job script that, on CI-triggered builds, looks up the open PR for `Build.SourceBranch` via the ADO REST API and promotes the build to PR semantics if exactly one matches `pr.branches` (and `pr.paths` if configured). Zero or multiple matches → the Agent job self-skips cleanly. Set `on.pr.mode: policy` when an operator-installed Build Validation branch policy is in place — that mode omits all synth wiring AND emits `trigger: none` so feature-branch pushes do not queue duplicate CI builds alongside the policy-driven PR build. Note that in `mode: synthetic` the top-level CI `trigger:` is **not** auto-narrowed to `pr.branches.include`: those are PR target branches, and ADO `trigger:` fires on pushes *to* listed branches, so narrowing would suppress CI on the feature branches synthPr must react to. Full reference: ["PR Triggering in Azure Repos" in `docs/front-matter.md`](../docs/front-matter.md#pr-triggering-in-azure-repos). + **PR-reviewer agents — DO NOT write your own precompute step.** When `on.pr` is set, the compiler automatically (1) fetches the PR target branch with progressive deepening, (2) resolves and stages `aw-context/pr/base.sha` + `aw-context/pr/head.sha`, (3) appends a prompt fragment listing common `git diff`/`git show`/`git log` commands and example Azure DevOps MCP tool calls (`repo_get_pull_request_by_id`, `repo_list_pull_request_threads`, `repo_create_pull_request_thread`) with the PR id / project / repo pre-filled, and (4) adds `git`, `git diff`, `git log`, `git show`, `git status`, `git rev-parse`, `git symbolic-ref` to the agent's bash allow-list. The agent runs `git diff $BASE..$HEAD` itself inside the AWF sandbox (objects are already fetched into the workspace). On failure (e.g. merge-base could not be resolved), the failure fragment tells the agent to surface the error rather than produce an empty review. Opt out via `execution-context.pr.enabled: false`. Full reference: [`docs/execution-context.md`](../docs/execution-context.md). #### Pipeline Triggers (`on.pipeline`) diff --git a/prompts/debug-ado-agentic-workflow.md b/prompts/debug-ado-agentic-workflow.md index d2315422..b538a6ea 100644 --- a/prompts/debug-ado-agentic-workflow.md +++ b/prompts/debug-ado-agentic-workflow.md @@ -32,7 +32,7 @@ Agent → Detection → SafeOutputs | **SafeOutputs** | Executes approved safe outputs (create PRs, work items, wiki pages, etc.) | Write (`permissions.write`) | Standard ADO agent | Additional optional jobs: -- **Setup** — runs before `Agent` (from `setup:` front matter) +- **Setup** — runs before `Agent` (from `setup:` front matter). When `on.pr` is set with the default `mode: synthetic`, this job also runs a `synthPr` step that calls the ADO REST API to promote CI-triggered builds to PR semantics when an open PR matches; if it sets `AW_SYNTHETIC_PR_SKIP=true`, the Agent job is skipped cleanly. With `mode: policy` there is no synthPr step (and `trigger: none` is emitted so the operator's Build Validation branch policy is the sole source of PR builds). See [`docs/front-matter.md#pr-triggering-in-azure-repos`](../docs/front-matter.md#pr-triggering-in-azure-repos). - **Teardown** — runs after `SafeOutputs` (from `teardown:` front matter) --- diff --git a/prompts/update-ado-agentic-workflow.md b/prompts/update-ado-agentic-workflow.md index 8e0e7049..93fb0f03 100644 --- a/prompts/update-ado-agentic-workflow.md +++ b/prompts/update-ado-agentic-workflow.md @@ -44,6 +44,16 @@ triggers → steps → post-steps → setup → teardown → network → permissions → parameters ``` +> **`on.pr` knob update**: when changing `on.pr.branches` or +> `on.pr.paths`, also confirm whether `mode` (default `synthetic`) is +> appropriate. In `synthetic` mode the compiler emits a Setup-job ADO +> REST call to discover the open PR for `Build.SourceBranch` and +> leaves the top-level `trigger:` at the ADO default. Switch to +> `mode: policy` only if the operator has explicitly installed a +> Build Validation branch policy — that mode emits `trigger: none` +> and drops the synth wiring. Reference: +> [`docs/front-matter.md#pr-triggering-in-azure-repos`](../docs/front-matter.md#pr-triggering-in-azure-repos). + ### Step 3 — Validate the Changes Run through the validation checklist (see below) before finalizing. Fix any issues and inform the user of corrections made. diff --git a/scripts/ado-script/.gitignore b/scripts/ado-script/.gitignore index 59cbba45..6aea872c 100644 --- a/scripts/ado-script/.gitignore +++ b/scripts/ado-script/.gitignore @@ -3,5 +3,6 @@ node_modules gate.js import.js exec-context-pr.js +exec-context-pr-synth.js schema *.tsbuildinfo diff --git a/scripts/ado-script/package-lock.json b/scripts/ado-script/package-lock.json index be43eacc..af284041 100644 --- a/scripts/ado-script/package-lock.json +++ b/scripts/ado-script/package-lock.json @@ -8,10 +8,12 @@ "name": "@ado-aw/scripts", "version": "0.0.0", "dependencies": { - "azure-devops-node-api": "^14.1.0" + "azure-devops-node-api": "^14.1.0", + "picomatch": "^4.0.4" }, "devDependencies": { "@types/node": "^20.19.39", + "@types/picomatch": "^4.0.3", "@vercel/ncc": "^0.38.4", "json-schema-to-typescript": "^15.0.4", "typescript": "^5.9.3", @@ -447,6 +449,13 @@ "undici-types": "~6.21.0" } }, + "node_modules/@types/picomatch": { + "version": "4.0.3", + "resolved": "https://registry.npmjs.org/@types/picomatch/-/picomatch-4.0.3.tgz", + "integrity": "sha512-iG0T6+nYJ9FAPmx9SsUlnwcq1ZVRuCXcVEvWnntoPlrOpwtSTKNDC9uVAxTsC3PUvJ+99n4RpAcNgBbHX3JSnQ==", + "dev": true, + "license": "MIT" + }, "node_modules/@vercel/ncc": { "version": "0.38.4", "resolved": "https://registry.npmjs.org/@vercel/ncc/-/ncc-0.38.4.tgz", @@ -1287,7 +1296,6 @@ "version": "4.0.4", "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.4.tgz", "integrity": "sha512-QP88BAKvMam/3NxH6vj2o21R6MjxZUAd6nlwAS/pnGvN9IVLocLHxGYIzFhg6fUQ+5th6P4dv4eW9jX3DSIj7A==", - "dev": true, "license": "MIT", "engines": { "node": ">=12" diff --git a/scripts/ado-script/package.json b/scripts/ado-script/package.json index 0aa0cf0c..de386bdd 100644 --- a/scripts/ado-script/package.json +++ b/scripts/ado-script/package.json @@ -7,23 +7,26 @@ "node": ">=20.0.0" }, "scripts": { - "build": "npm run codegen && npm run clean && npm run build:gate && npm run build:import && npm run build:exec-context-pr", - "clean": "node -e \"const fs=require('node:fs'); fs.rmSync('.ado-build',{recursive:true,force:true}); fs.rmSync('gate.js',{force:true}); fs.rmSync('import.js',{force:true}); fs.rmSync('exec-context-pr.js',{force:true});\"", + "build": "npm run codegen && npm run clean && npm run build:gate && npm run build:import && npm run build:exec-context-pr && npm run build:exec-context-pr-synth", + "clean": "node -e \"const fs=require('node:fs'); fs.rmSync('.ado-build',{recursive:true,force:true}); fs.rmSync('gate.js',{force:true}); fs.rmSync('import.js',{force:true}); fs.rmSync('exec-context-pr.js',{force:true}); fs.rmSync('exec-context-pr-synth.js',{force:true});\"", "build:gate": "ncc build src/gate/index.ts -o .ado-build/gate -m -t && node -e \"const fs=require('node:fs'); fs.copyFileSync('.ado-build/gate/index.js','gate.js'); fs.rmSync('.ado-build/gate',{recursive:true,force:true});\"", "build:import": "ncc build src/import/index.ts -o .ado-build/import -m -t && node -e \"const fs=require('node:fs'); fs.copyFileSync('.ado-build/import/index.js','import.js'); fs.rmSync('.ado-build/import',{recursive:true,force:true});\"", "build:exec-context-pr": "ncc build src/exec-context-pr/index.ts -o .ado-build/exec-context-pr -m -t && node -e \"const fs=require('node:fs'); fs.copyFileSync('.ado-build/exec-context-pr/index.js','exec-context-pr.js'); fs.rmSync('.ado-build/exec-context-pr',{recursive:true,force:true});\"", + "build:exec-context-pr-synth": "ncc build src/exec-context-pr-synth/index.ts -o .ado-build/exec-context-pr-synth -m -t && node -e \"const fs=require('node:fs'); fs.copyFileSync('.ado-build/exec-context-pr-synth/index.js','exec-context-pr-synth.js'); fs.rmSync('.ado-build/exec-context-pr-synth',{recursive:true,force:true});\"", "build:check": "ls -lh gate.js && wc -c gate.js", "codegen": "node -e \"require('node:fs').mkdirSync('schema', { recursive: true })\" && cargo run --quiet --manifest-path ../../Cargo.toml -- export-gate-schema --output schema/gate-spec.schema.json && npx json2ts schema/gate-spec.schema.json -o src/shared/types.gen.ts --bannerComment \"// AUTO-GENERATED from Rust IR via cargo run -- export-gate-schema. Do not edit; run npm run codegen.\"", "test": "vitest run", - "test:smoke": "npm run build:gate && npm run build:import && npm run build:exec-context-pr && vitest run -c vitest.config.smoke.ts", + "test:smoke": "npm run build:gate && npm run build:import && npm run build:exec-context-pr && npm run build:exec-context-pr-synth && vitest run -c vitest.config.smoke.ts", "lint": "echo TODO", "typecheck": "tsc --noEmit" }, "dependencies": { - "azure-devops-node-api": "^14.1.0" + "azure-devops-node-api": "^14.1.0", + "picomatch": "^4.0.4" }, "devDependencies": { "@types/node": "^20.19.39", + "@types/picomatch": "^4.0.3", "@vercel/ncc": "^0.38.4", "json-schema-to-typescript": "^15.0.4", "typescript": "^5.9.3", diff --git a/scripts/ado-script/src/exec-context-pr-synth/__tests__/harness.ts b/scripts/ado-script/src/exec-context-pr-synth/__tests__/harness.ts new file mode 100644 index 00000000..f8ac6116 --- /dev/null +++ b/scripts/ado-script/src/exec-context-pr-synth/__tests__/harness.ts @@ -0,0 +1,61 @@ +/** + * Test harness for the exec-context-pr-synth bundle. + * + * Provides: + * - `runMain(env)` — invokes the bundle's main() with a captured + * stdout buffer. + * - `makeEnv(overrides)` — returns a minimal env block populated + * with the required vars, easy to override per case. + * - `build_pr_synth_spec(spec)` — base64-encodes a PrSynthSpec JSON + * for the PR_SYNTH_SPEC env var. + */ +import { vi } from "vitest"; + +import { main } from "../index.js"; +import { _resetCompletedForTesting } from "../../shared/vso-logger.js"; + +export interface RunResult { + code: number; + output: string; +} + +export async function runMain(env: NodeJS.ProcessEnv): Promise { + _resetCompletedForTesting(); + const chunks: string[] = []; + const writeSpy = vi + .spyOn(process.stdout, "write") + .mockImplementation((c: any) => { + chunks.push(typeof c === "string" ? c : c.toString()); + return true; + }); + try { + const code = await main(env); + return { code, output: chunks.join("") }; + } finally { + writeSpy.mockRestore(); + } +} + +export function makeEnv(overrides: Record): NodeJS.ProcessEnv { + return { + BUILD_REASON: "IndividualCI", + BUILD_REPOSITORY_PROVIDER: "TfsGit", + BUILD_SOURCEBRANCH: "refs/heads/feature/x", + ADO_PROJECT: "MyProject", + ADO_REPO_ID: "00000000-0000-0000-0000-000000000000", + ...overrides, + }; +} + +export function build_pr_synth_spec( + spec: { + branches?: { include: string[]; exclude: string[] }; + paths?: { include: string[]; exclude: string[] }; + } = {}, +): string { + const full = { + branches: spec.branches ?? { include: ["main"], exclude: [] }, + paths: spec.paths ?? { include: [], exclude: [] }, + }; + return Buffer.from(JSON.stringify(full), "utf8").toString("base64"); +} diff --git a/scripts/ado-script/src/exec-context-pr-synth/__tests__/index.test.ts b/scripts/ado-script/src/exec-context-pr-synth/__tests__/index.test.ts new file mode 100644 index 00000000..3e09f323 --- /dev/null +++ b/scripts/ado-script/src/exec-context-pr-synth/__tests__/index.test.ts @@ -0,0 +1,176 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("../../shared/ado-client.js", () => ({ + listActivePullRequestsBySourceRef: vi.fn(), + getPullRequestIterations: vi.fn(), + getIterationChanges: vi.fn(), +})); + +import * as adoClient from "../../shared/ado-client.js"; +import { build_pr_synth_spec, makeEnv, runMain } from "./harness.js"; + +const mocked = adoClient as unknown as { + listActivePullRequestsBySourceRef: ReturnType; + getPullRequestIterations: ReturnType; + getIterationChanges: ReturnType; +}; + +describe("exec-context-pr-synth main", () => { + beforeEach(() => { + mocked.listActivePullRequestsBySourceRef.mockReset(); + mocked.getPullRequestIterations.mockReset(); + mocked.getIterationChanges.mockReset(); + }); + afterEach(() => vi.restoreAllMocks()); + + it("no-ops on real PR builds (BUILD_REASON=PullRequest)", async () => { + const { code, output } = await runMain( + makeEnv({ BUILD_REASON: "PullRequest", PR_SYNTH_SPEC: build_pr_synth_spec() }), + ); + expect(code).toBe(0); + expect(mocked.listActivePullRequestsBySourceRef).not.toHaveBeenCalled(); + expect(output).not.toContain("AW_SYNTHETIC_PR_SKIP"); + expect(output).not.toContain("AW_SYNTHETIC_PR=true"); + expect(output).toContain("real PR build"); + }); + + it("no-ops on GitHub-typed repos (BUILD_REPOSITORY_PROVIDER=GitHub)", async () => { + const { code, output } = await runMain( + makeEnv({ + BUILD_REPOSITORY_PROVIDER: "GitHub", + PR_SYNTH_SPEC: build_pr_synth_spec(), + }), + ); + expect(code).toBe(0); + expect(mocked.listActivePullRequestsBySourceRef).not.toHaveBeenCalled(); + expect(output).toContain("GitHub-typed repo"); + }); + + it("returns 1 (hard fail) when PR_SYNTH_SPEC is missing", async () => { + const env = makeEnv({}); + delete env.PR_SYNTH_SPEC; + const { code, output } = await runMain(env); + expect(code).toBe(1); + expect(output).toContain("PR_SYNTH_SPEC env var is missing"); + }); + + it("returns 1 when PR_SYNTH_SPEC is malformed base64", async () => { + const { code, output } = await runMain( + makeEnv({ PR_SYNTH_SPEC: "not!valid!base64!!!" }), + ); + expect(code).toBe(1); + expect(output).toContain("PR_SYNTH_SPEC"); + }); + + it("skips when source branch has no active PR (per ADO API)", async () => { + mocked.listActivePullRequestsBySourceRef.mockResolvedValue([]); + const { code, output } = await runMain( + makeEnv({ + BUILD_SOURCEBRANCH: "refs/heads/feature/unrelated", + PR_SYNTH_SPEC: build_pr_synth_spec({ branches: { include: ["main"], exclude: [] } }), + }), + ); + expect(code).toBe(0); + expect(output).toContain("AW_SYNTHETIC_PR_SKIP;isOutput=true]true"); + expect(mocked.listActivePullRequestsBySourceRef).toHaveBeenCalledOnce(); + }); + + it("skips when a PR is active but its target branch is excluded by on.pr.branches", async () => { + mocked.listActivePullRequestsBySourceRef.mockResolvedValue([ + { + pullRequestId: 1, + sourceRefName: "refs/heads/feature/x", + targetRefName: "refs/heads/release/old", + }, + ]); + const spec = build_pr_synth_spec({ branches: { include: ["main"], exclude: [] } }); + const { code, output } = await runMain(makeEnv({ PR_SYNTH_SPEC: spec })); + expect(code).toBe(0); + expect(output).toContain("AW_SYNTHETIC_PR_SKIP;isOutput=true]true"); + }); + + it("skips when >1 active PRs match (after target filter)", async () => { + mocked.listActivePullRequestsBySourceRef.mockResolvedValue([ + { pullRequestId: 1, sourceRefName: "refs/heads/feature/x", targetRefName: "refs/heads/main" }, + { pullRequestId: 2, sourceRefName: "refs/heads/feature/x", targetRefName: "refs/heads/main" }, + ]); + const { code, output } = await runMain(makeEnv({ PR_SYNTH_SPEC: build_pr_synth_spec() })); + expect(code).toBe(0); + expect(output).toContain("AW_SYNTHETIC_PR_SKIP;isOutput=true]true"); + expect(output).toContain("2 active PRs"); + }); + + it("skips when path filter rejects all changed files", async () => { + mocked.listActivePullRequestsBySourceRef.mockResolvedValue([ + { pullRequestId: 42, sourceRefName: "refs/heads/feature/x", targetRefName: "refs/heads/main" }, + ]); + mocked.getPullRequestIterations.mockResolvedValue([{ id: 1 }]); + mocked.getIterationChanges.mockResolvedValue({ + changeEntries: [{ item: { path: "/docs/readme.md" } }], + }); + const spec = build_pr_synth_spec({ + branches: { include: ["main"], exclude: [] }, + paths: { include: ["src/**"], exclude: [] }, + }); + const { code, output } = await runMain(makeEnv({ PR_SYNTH_SPEC: spec })); + expect(code).toBe(0); + expect(output).toContain("AW_SYNTHETIC_PR_SKIP;isOutput=true]true"); + expect(output).toContain("no changed file"); + }); + + it("emits AW_SYNTHETIC_PR=true + identifiers on the happy path", async () => { + mocked.listActivePullRequestsBySourceRef.mockResolvedValue([ + { + pullRequestId: 1234, + sourceRefName: "refs/heads/feature/x", + targetRefName: "refs/heads/main", + isDraft: false, + }, + ]); + mocked.getPullRequestIterations.mockResolvedValue([{ id: 7 }]); + mocked.getIterationChanges.mockResolvedValue({ + changeEntries: [{ item: { path: "/src/foo.rs" } }], + }); + const spec = build_pr_synth_spec({ + branches: { include: ["main"], exclude: [] }, + paths: { include: ["src/**"], exclude: [] }, + }); + const { code, output } = await runMain(makeEnv({ PR_SYNTH_SPEC: spec })); + expect(code).toBe(0); + expect(output).not.toContain("AW_SYNTHETIC_PR_SKIP"); + expect(output).toContain("AW_SYNTHETIC_PR;isOutput=true]true"); + expect(output).toContain("AW_SYNTHETIC_PR_ID;isOutput=true]1234"); + expect(output).toContain("AW_SYNTHETIC_PR_TARGETBRANCH;isOutput=true]refs/heads/main"); + expect(output).toContain("AW_SYNTHETIC_PR_SOURCEBRANCH;isOutput=true]refs/heads/feature/x"); + expect(output).toContain("AW_SYNTHETIC_PR_IS_DRAFT;isOutput=true]false"); + }); + + it("emits AW_SYNTHETIC_PR_IS_DRAFT=true when the PR is a draft", async () => { + mocked.listActivePullRequestsBySourceRef.mockResolvedValue([ + { + pullRequestId: 1, + sourceRefName: "refs/heads/feature/x", + targetRefName: "refs/heads/main", + isDraft: true, + }, + ]); + const { code, output } = await runMain(makeEnv({ PR_SYNTH_SPEC: build_pr_synth_spec() })); + expect(code).toBe(0); + expect(output).toContain("AW_SYNTHETIC_PR_IS_DRAFT;isOutput=true]true"); + }); + + it("skips path-filter API calls when paths.include and exclude are both empty", async () => { + mocked.listActivePullRequestsBySourceRef.mockResolvedValue([ + { + pullRequestId: 1, + sourceRefName: "refs/heads/feature/x", + targetRefName: "refs/heads/main", + }, + ]); + const { code, output } = await runMain(makeEnv({ PR_SYNTH_SPEC: build_pr_synth_spec() })); + expect(code).toBe(0); + expect(mocked.getPullRequestIterations).not.toHaveBeenCalled(); + expect(mocked.getIterationChanges).not.toHaveBeenCalled(); + expect(output).toContain("AW_SYNTHETIC_PR;isOutput=true]true"); + }); +}); diff --git a/scripts/ado-script/src/exec-context-pr-synth/__tests__/match.test.ts b/scripts/ado-script/src/exec-context-pr-synth/__tests__/match.test.ts new file mode 100644 index 00000000..837b8ed1 --- /dev/null +++ b/scripts/ado-script/src/exec-context-pr-synth/__tests__/match.test.ts @@ -0,0 +1,81 @@ +import { describe, expect, it } from "vitest"; + +import { + matchesIncludeExclude, + normaliseBranchRef, + normalisePath, + pathMatchesIncludeExclude, +} from "../match.js"; + +describe("normaliseBranchRef", () => { + it("strips refs/heads/ prefix", () => { + expect(normaliseBranchRef("refs/heads/main")).toBe("main"); + expect(normaliseBranchRef("refs/heads/feature/x")).toBe("feature/x"); + }); + it("leaves other refs alone", () => { + expect(normaliseBranchRef("main")).toBe("main"); + expect(normaliseBranchRef("refs/tags/v1")).toBe("refs/tags/v1"); + }); +}); + +describe("normalisePath", () => { + it("strips a leading slash", () => { + expect(normalisePath("/src/foo.rs")).toBe("src/foo.rs"); + }); + it("leaves paths without a leading slash alone", () => { + expect(normalisePath("src/foo.rs")).toBe("src/foo.rs"); + }); +}); + +describe("matchesIncludeExclude (branches)", () => { + it("matches when include list is empty (include-all)", () => { + expect(matchesIncludeExclude("main", [], [])).toBe(true); + expect(matchesIncludeExclude("refs/heads/main", [], [])).toBe(true); + }); + + it("matches exact branch names with refs/heads/ normalisation on BOTH sides", () => { + expect(matchesIncludeExclude("refs/heads/main", ["main"], [])).toBe(true); + expect(matchesIncludeExclude("main", ["refs/heads/main"], [])).toBe(true); + }); + + it("rejects branches that are not in the include list", () => { + expect(matchesIncludeExclude("refs/heads/feature/x", ["main"], [])).toBe(false); + }); + + it("supports * glob inside one segment", () => { + expect(matchesIncludeExclude("refs/heads/release/1.0", ["release/*"], [])).toBe(true); + }); + + it("supports ** glob across segments", () => { + expect(matchesIncludeExclude("refs/heads/feature/x/y", ["feature/**"], [])).toBe(true); + }); + + it("exclude wins over include", () => { + expect(matchesIncludeExclude("refs/heads/feature/x", ["**"], ["feature/*"])).toBe(false); + }); + + it("returns true when include matches and exclude does not", () => { + expect(matchesIncludeExclude("refs/heads/main", ["main", "release/*"], ["dev/*"])).toBe( + true, + ); + }); +}); + +describe("pathMatchesIncludeExclude (paths)", () => { + it("matches when include list is empty (include-all)", () => { + expect(pathMatchesIncludeExclude("/src/foo.rs", [], [])).toBe(true); + }); + + it("handles leading-slash normalisation on both sides", () => { + expect(pathMatchesIncludeExclude("/src/foo.rs", ["/src/**"], [])).toBe(true); + expect(pathMatchesIncludeExclude("src/foo.rs", ["src/**"], [])).toBe(true); + }); + + it("rejects paths that fall outside the include glob", () => { + expect(pathMatchesIncludeExclude("/docs/x.md", ["src/**"], [])).toBe(false); + }); + + it("exclude wins over include", () => { + expect(pathMatchesIncludeExclude("/tests/x.rs", ["**"], ["tests/**"])).toBe(false); + }); +}); diff --git a/scripts/ado-script/src/exec-context-pr-synth/index.ts b/scripts/ado-script/src/exec-context-pr-synth/index.ts new file mode 100644 index 00000000..0a3b70f9 --- /dev/null +++ b/scripts/ado-script/src/exec-context-pr-synth/index.ts @@ -0,0 +1,225 @@ +/** + * exec-context-pr-synth — Setup-job script that synthesises + * `Build.Reason == PullRequest` semantics on CI-triggered builds when + * an open PR matches the agent's `on.pr.branches` / `on.pr.paths` + * filters. + * + * Why this exists: Azure DevOps Services ignores the YAML `pr:` block + * unless a per-branch Build Validation policy is registered server- + * side. Without that policy, a push to a feature branch fires the + * pipeline as `Build.Reason = IndividualCI` even when an open PR + * exists, so the gate evaluator's "not a PR build" bypass triggers + * and `exec-context-pr.js` is skipped entirely. + * + * This script runs in the Setup job before `prGate`, calls the ADO + * REST API to find the active PR for `Build.SourceBranch`, applies + * the front-matter filters, and emits `AW_SYNTHETIC_PR*` outputs + * that downstream gate + exec-context-pr steps coalesce with the + * real `System.PullRequest.*` variables. + * + * Runtime contract (all soft skips exit 0; only spec-decode errors + * and infra failures exit non-zero): + * + * 1. real PR build (BUILD_REASON=PullRequest) → no-op + * 2. GitHub-typed repo (BUILD_REPOSITORY_PROVIDER=GitHub) → no-op + * 3. Decode PR_SYNTH_SPEC (hard fail on corruption) + * 4. fetch active PRs whose `sourceRefName == BUILD_SOURCEBRANCH` + * (no source-branch pre-filter against the spec — `on.pr.branches` + * lists *target* branches per ADO semantics, not the build's + * source branch) + * 5. filter matched PRs by `targetRefName` against + * `spec.branches.include` / `spec.branches.exclude` + * 6. count != 1 → skip + * 7. paths.include/exclude reject every changed file → skip + * 8. emit AW_SYNTHETIC_PR* outputs + */ +import { + getIterationChanges, + getPullRequestIterations, + listActivePullRequestsBySourceRef, +} from "../shared/ado-client.js"; +import { logError, logInfo, setOutput } from "../shared/vso-logger.js"; + +import { matchesIncludeExclude, normalisePath, pathMatchesIncludeExclude } from "./match.js"; +import { decodeSpec, type PrSynthSpec } from "./spec.js"; + +const SKIP_OUTPUT = "AW_SYNTHETIC_PR_SKIP"; + +function emitSkip(reason: string): void { + setOutput(SKIP_OUTPUT, "true"); + logInfo(`[synth-pr] skip: ${reason}`); +} + +export async function main(env: NodeJS.ProcessEnv = process.env): Promise { + // Step 1 — real PR build owns the path; the bundle has nothing to add. + if ((env.BUILD_REASON ?? "") === "PullRequest") { + logInfo("[synth-pr] BUILD_REASON=PullRequest; real PR build, synth skipped"); + return 0; + } + + // Step 2 — GitHub-typed repos already get correct pr: semantics from + // ADO. Synthesising would double-fire. + if ((env.BUILD_REPOSITORY_PROVIDER ?? "").toLowerCase() === "github") { + logInfo("[synth-pr] GitHub-typed repo; ADO already provides PR semantics, synth skipped"); + return 0; + } + + // Step 3 — decode the spec. Corruption is a hard failure. + let spec: PrSynthSpec; + try { + spec = decodeSpec(env.PR_SYNTH_SPEC); + } catch (e) { + logError(`[synth-pr] ${(e as Error).message}`); + return 1; + } + + const sourceBranch = env.BUILD_SOURCEBRANCH ?? ""; + if (sourceBranch.length === 0) { + emitSkip("BUILD_SOURCEBRANCH is empty"); + return 0; + } + + const project = env.ADO_PROJECT ?? ""; + const repoId = env.ADO_REPO_ID ?? ""; + if (project.length === 0 || repoId.length === 0) { + logError( + "[synth-pr] required env vars ADO_PROJECT and ADO_REPO_ID must be set by the compiler", + ); + return 1; + } + + // Step 4 — fetch active PRs whose source branch is exactly ours. + // (Skipping a source-branch pre-filter: `on.pr.branches` filters the + // PR's TARGET branch, not the build's source branch. For most CI + // builds on a non-PR branch the API returns [] cheaply.) + let prs; + try { + prs = await listActivePullRequestsBySourceRef(project, repoId, sourceBranch); + } catch (e) { + logError(`[synth-pr] listActivePullRequestsBySourceRef failed: ${(e as Error).message}`); + return 1; + } + + const matched = prs.filter((pr) => + matchesIncludeExclude( + pr.targetRefName ?? "", + spec.branches.include, + spec.branches.exclude, + ), + ); + + // Step 6 — exactly-one rule. + if (matched.length === 0) { + emitSkip(`no active PR for source ${sourceBranch} matches on.pr.branches`); + return 0; + } + if (matched.length > 1) { + emitSkip( + `${matched.length} active PRs match source ${sourceBranch}; refusing to choose`, + ); + return 0; + } + + const pr = matched[0]!; + const prId = pr.pullRequestId; + if (typeof prId !== "number") { + emitSkip("matched PR has no pullRequestId"); + return 0; + } + + // Step 7 — path filter. Only fetch iteration changes if the agent + // declared a path filter; otherwise short-circuit (the + // pathMatchesIncludeExclude call would accept everything anyway, but + // the API call is wasted). + const hasPathFilter = spec.paths.include.length > 0 || spec.paths.exclude.length > 0; + if (hasPathFilter) { + let iterations; + try { + iterations = await getPullRequestIterations(project, repoId, prId); + } catch (e) { + logError(`[synth-pr] getPullRequestIterations failed: ${(e as Error).message}`); + return 1; + } + if (iterations.length === 0) { + emitSkip(`PR ${prId} has no iterations`); + return 0; + } + const latest = iterations[iterations.length - 1]!; + const iterationId = latest.id; + if (typeof iterationId !== "number") { + emitSkip(`PR ${prId} latest iteration has no id`); + return 0; + } + let changes; + try { + changes = await getIterationChanges(project, repoId, prId, iterationId); + } catch (e) { + logError(`[synth-pr] getIterationChanges failed: ${(e as Error).message}`); + return 1; + } + const changedPaths: string[] = []; + for (const entry of changes.changeEntries ?? []) { + const itemPath = entry.item?.path; + if (typeof itemPath === "string" && itemPath.length > 0) { + changedPaths.push(normalisePath(itemPath)); + } + } + const anyAccepted = changedPaths.some((p) => + pathMatchesIncludeExclude(p, spec.paths.include, spec.paths.exclude), + ); + if (!anyAccepted) { + emitSkip( + `no changed file in PR ${prId} matches on.pr.paths (${changedPaths.length} files inspected)`, + ); + return 0; + } + } + + // Step 8 — emit synthetic PR identifiers. Downstream gate + + // exec-context-pr steps coalesce these with the corresponding + // `System.PullRequest.*` predefined variables via `$[ coalesce(...) ]` + // env wiring on the consumer steps. + // + // Format note: `System.PullRequest.TargetBranch` (and `SourceBranch`) + // are documented as the full ref form `refs/heads/`, matching + // the `targetRefName` / `sourceRefName` shape returned by the ADO + // REST API (`refs/heads/main`, `refs/heads/feature/x`, etc.). The + // coalesce on consumer steps therefore yields a consistent + // refs-prefixed value whether the build was a real PR or synth- + // promoted. (The unprefixed short form is `TargetBranchName` — + // a separate predefined variable we deliberately do not use here.) + // See . + setOutput("AW_SYNTHETIC_PR", "true"); + setOutput("AW_SYNTHETIC_PR_ID", String(prId)); + setOutput("AW_SYNTHETIC_PR_TARGETBRANCH", pr.targetRefName ?? ""); + setOutput("AW_SYNTHETIC_PR_SOURCEBRANCH", pr.sourceRefName ?? sourceBranch); + setOutput("AW_SYNTHETIC_PR_IS_DRAFT", pr.isDraft === true ? "true" : "false"); + + logInfo( + `[synth-pr] matched PR #${prId} (source=${pr.sourceRefName} target=${pr.targetRefName})`, + ); + return 0; +} + +// Top-level invocation. `process.exit` is called here (not in `main`) +// so tests can call `main(env)` and inspect the return value without +// terminating the test process. +// +// Guard: only execute when this module is the program entry point. +// When the test harness imports `main`, the import-side-effect block +// below is skipped (vitest sets `process.argv[1]` to the runner, not +// to this module). When ncc bundles for production, `process.argv[1]` +// is the resolved bundle path, which matches `import.meta.url`. +import { fileURLToPath } from "node:url"; + +if ( + typeof process.argv[1] === "string" && + process.argv[1] === fileURLToPath(import.meta.url) +) { + main() + .then((code) => process.exit(code)) + .catch((e: unknown) => { + logError(`[synth-pr] unhandled error: ${(e as Error).message}`); + process.exit(1); + }); +} diff --git a/scripts/ado-script/src/exec-context-pr-synth/match.ts b/scripts/ado-script/src/exec-context-pr-synth/match.ts new file mode 100644 index 00000000..bae6c518 --- /dev/null +++ b/scripts/ado-script/src/exec-context-pr-synth/match.ts @@ -0,0 +1,78 @@ +/** + * Glob matching helpers for branch refs and changed-file paths. + * + * Both branches and paths in front-matter use the same ADO/git-style + * glob syntax: `*` matches one path segment, `**` matches any number, + * `?` matches a single char. We normalise refs by stripping the + * `refs/heads/` prefix so authors can write either `main` or + * `refs/heads/main` and both match. + * + * Built on picomatch for correctness; picomatch is added as a direct + * dependency so ncc bundles it deterministically. + */ +import picomatch from "picomatch"; + +const REFS_HEADS_PREFIX = "refs/heads/"; + +/** Strip a leading `refs/heads/` prefix if present. */ +export function normaliseBranchRef(ref: string): string { + return ref.startsWith(REFS_HEADS_PREFIX) ? ref.slice(REFS_HEADS_PREFIX.length) : ref; +} + +/** Strip a leading `/` from a path if present (iteration-API paths start with `/`). */ +export function normalisePath(p: string): string { + return p.startsWith("/") ? p.slice(1) : p; +} + +/** + * Apply include/exclude semantics: include must match (or be empty), + * and exclude must not match. + * + * - Empty `includes` → include-all (treat as "no positive filter"). + * - Non-empty `includes` → at least one must match. + * - Non-empty `excludes` → none must match. + * + * Mirrors ADO's `branches:` / `paths:` semantics in PR triggers. + */ +export function matchesIncludeExclude( + value: string, + includes: string[], + excludes: string[], +): boolean { + const normalised = normaliseBranchRef(value); + const normIncludes = includes.map(normaliseBranchRef); + const normExcludes = excludes.map(normaliseBranchRef); + const includeMatches = + normIncludes.length === 0 || + normIncludes.some((g) => picomatch(g, { dot: true })(normalised)); + if (!includeMatches) return false; + if ( + normExcludes.length > 0 && + normExcludes.some((g) => picomatch(g, { dot: true })(normalised)) + ) { + return false; + } + return true; +} + +/** Path variant: normalises leading `/` instead of `refs/heads/`. */ +export function pathMatchesIncludeExclude( + pathValue: string, + includes: string[], + excludes: string[], +): boolean { + const normalised = normalisePath(pathValue); + const normIncludes = includes.map(normalisePath); + const normExcludes = excludes.map(normalisePath); + const includeMatches = + normIncludes.length === 0 || + normIncludes.some((g) => picomatch(g, { dot: true })(normalised)); + if (!includeMatches) return false; + if ( + normExcludes.length > 0 && + normExcludes.some((g) => picomatch(g, { dot: true })(normalised)) + ) { + return false; + } + return true; +} diff --git a/scripts/ado-script/src/exec-context-pr-synth/spec.ts b/scripts/ado-script/src/exec-context-pr-synth/spec.ts new file mode 100644 index 00000000..bb99ddf9 --- /dev/null +++ b/scripts/ado-script/src/exec-context-pr-synth/spec.ts @@ -0,0 +1,72 @@ +/** + * Decodes the compiler-emitted `PR_SYNTH_SPEC` env var into a typed + * filter spec. The compiler builds this via + * `crate::compile::filter_ir::build_pr_synth_spec` (Rust side); both + * shapes must stay in lock-step. + * + * Decode failures are HARD errors — a malformed spec indicates either + * compiler corruption or a manual env tamper. Treating it as a soft + * skip would silently widen the permitted attack surface (any branch + * could fool the synth path into matching). + */ + +export interface PrSynthSpec { + branches: { include: string[]; exclude: string[] }; + paths: { include: string[]; exclude: string[] }; +} + +export function decodeSpec(b64: string | undefined): PrSynthSpec { + if (!b64 || b64.length === 0) { + throw new Error("PR_SYNTH_SPEC env var is missing or empty"); + } + let json: string; + try { + json = Buffer.from(b64, "base64").toString("utf8"); + } catch (e) { + throw new Error(`PR_SYNTH_SPEC: base64 decode failed: ${(e as Error).message}`); + } + let parsed: unknown; + try { + parsed = JSON.parse(json); + } catch (e) { + throw new Error(`PR_SYNTH_SPEC: JSON parse failed: ${(e as Error).message}`); + } + return validate(parsed); +} + +function validate(value: unknown): PrSynthSpec { + if (!value || typeof value !== "object") { + throw new Error("PR_SYNTH_SPEC: expected object at root"); + } + const obj = value as Record; + return { + branches: validateGlobs(obj.branches, "branches"), + paths: validateGlobs(obj.paths, "paths"), + }; +} + +function validateGlobs( + value: unknown, + fieldName: string, +): { include: string[]; exclude: string[] } { + if (!value || typeof value !== "object") { + throw new Error(`PR_SYNTH_SPEC: expected ${fieldName} to be an object`); + } + const obj = value as Record; + return { + include: validateStringArray(obj.include, `${fieldName}.include`), + exclude: validateStringArray(obj.exclude, `${fieldName}.exclude`), + }; +} + +function validateStringArray(value: unknown, fieldName: string): string[] { + if (!Array.isArray(value)) { + throw new Error(`PR_SYNTH_SPEC: expected ${fieldName} to be an array of strings`); + } + for (const item of value) { + if (typeof item !== "string") { + throw new Error(`PR_SYNTH_SPEC: ${fieldName} contains non-string entry`); + } + } + return value as string[]; +} diff --git a/scripts/ado-script/src/gate/bypass.test.ts b/scripts/ado-script/src/gate/bypass.test.ts index 192ed7c5..70f8e081 100644 --- a/scripts/ado-script/src/gate/bypass.test.ts +++ b/scripts/ado-script/src/gate/bypass.test.ts @@ -81,4 +81,54 @@ describe("runBypass", () => { expect(failedCompletes).toEqual([]); expect(writes.join("")).toContain("%0A"); // embedded \n encoded }); + + // ─── Synthetic-PR support (issue #916) ──────────────────────────── + + it("does NOT bypass when AW_SYNTHETIC_PR=true even on a non-PR build reason", async () => { + process.env.ADO_BUILD_REASON = "IndividualCI"; + process.env.AW_SYNTHETIC_PR = "true"; + try { + const result = await runBypass(baseSpec); + expect(result).toBe(false); + expect(writes.join("")).not.toContain("setvariable variable=SHOULD_RUN"); + expect(writes.join("")).not.toContain("build.addbuildtag"); + } finally { + delete process.env.AW_SYNTHETIC_PR; + } + }); + + it("still bypasses when AW_SYNTHETIC_PR is anything other than 'true'", async () => { + process.env.ADO_BUILD_REASON = "IndividualCI"; + process.env.AW_SYNTHETIC_PR = "false"; + try { + const result = await runBypass(baseSpec); + expect(result).toBe(true); + } finally { + delete process.env.AW_SYNTHETIC_PR; + } + }); + + it("still bypasses when AW_SYNTHETIC_PR is unset (existing behaviour)", async () => { + process.env.ADO_BUILD_REASON = "IndividualCI"; + delete process.env.AW_SYNTHETIC_PR; + const result = await runBypass(baseSpec); + expect(result).toBe(true); + }); + + it("AW_SYNTHETIC_PR only suppresses bypass for PullRequest specs", async () => { + // For non-PR specs (e.g. PipelineCompletion), AW_SYNTHETIC_PR is + // irrelevant; bypass should fall through to the regular reason check. + process.env.ADO_BUILD_REASON = "Manual"; + process.env.AW_SYNTHETIC_PR = "true"; + try { + const pipelineSpec: GateSpec = { + ...baseSpec, + context: { ...baseSpec.context, build_reason: "ResourceTrigger", bypass_label: "pipeline-completion" }, + }; + const result = await runBypass(pipelineSpec); + expect(result).toBe(true); // bypass still triggers — synth only affects PullRequest + } finally { + delete process.env.AW_SYNTHETIC_PR; + } + }); }); diff --git a/scripts/ado-script/src/gate/bypass.ts b/scripts/ado-script/src/gate/bypass.ts index 78ef3536..56a55580 100644 --- a/scripts/ado-script/src/gate/bypass.ts +++ b/scripts/ado-script/src/gate/bypass.ts @@ -2,13 +2,21 @@ * Bypass logic: when ADO_BUILD_REASON does not match the spec's expected * build reason (e.g. spec is for PullRequest but build was Manual), the * gate auto-passes. + * + * Exception: when `AW_SYNTHETIC_PR === "true"` (set by the upstream + * `synthPr` Setup-job step in `exec-context-pr-synth.js`), the build is + * a CI-triggered run that has been promoted to "behave like a PR" by + * the synthetic-from-ci path. The build reason is still `IndividualCI` + * but we want the full PR-spec evaluation to run, not the bypass. */ import type { GateSpec } from "../shared/types.gen.js"; import { setOutput, addBuildTag, complete, logInfo } from "../shared/vso-logger.js"; export async function runBypass(spec: GateSpec): Promise { const buildReason = process.env.ADO_BUILD_REASON ?? ""; - if (buildReason !== spec.context.build_reason) { + const synthetic = + spec.context.build_reason === "PullRequest" && process.env.AW_SYNTHETIC_PR === "true"; + if (!synthetic && buildReason !== spec.context.build_reason) { // Routed through logInfo so the (compiler-controlled but theoretically // template-influenceable) bypass_label cannot smuggle a `##vso[` prefix // into the line. Mirrors the Python log line for parity. diff --git a/scripts/ado-script/src/shared/ado-client.ts b/scripts/ado-script/src/shared/ado-client.ts index 2686582a..07ac0c3d 100644 --- a/scripts/ado-script/src/shared/ado-client.ts +++ b/scripts/ado-script/src/shared/ado-client.ts @@ -10,6 +10,7 @@ import type { GitPullRequestIteration, GitPullRequestIterationChanges, } from "azure-devops-node-api/interfaces/GitInterfaces.js"; +import { PullRequestStatus } from "azure-devops-node-api/interfaces/GitInterfaces.js"; import { BuildStatus, type Build } from "azure-devops-node-api/interfaces/BuildInterfaces.js"; const SLEEP_MS = 1000; @@ -95,6 +96,40 @@ export async function getPullRequestById( }); } +/** + * Lists active pull requests whose `sourceRefName` matches the given + * value. Used by `exec-context-pr-synth` to discover the open PR for + * `Build.SourceBranch` on CI-triggered builds (no Build Validation + * branch policy required). + * + * Returns an empty array if no PRs match. The ADO REST API caps page + * size; for the synth path we deliberately fetch only the first page + * (the SDK default is 100 PRs without an explicit `$top`) since the + * synth contract requires *exactly one* match — a source branch with + * >100 simultaneous active PRs against it is pathological and the + * bundle will skip via the "multi-match" path anyway. + * + * The `?? []` guard handles the SDK's habit of returning `null` for + * empty result bodies on some REST responses; callers iterate / filter + * the result, so an empty array is the only safe contract. + */ +export async function listActivePullRequestsBySourceRef( + project: string, + repoId: string, + sourceRefName: string, +): Promise { + return withRetry("listActivePullRequestsBySourceRef", async () => { + const git = await (await getWebApi()).getGitApi(); + return ( + (await git.getPullRequests( + repoId, + { sourceRefName, status: PullRequestStatus.Active }, + project, + )) ?? [] + ); + }); +} + /** * Fetches all pull-request iterations. * diff --git a/src/compile/common.rs b/src/compile/common.rs index d1c442cd..965c90df 100644 --- a/src/compile/common.rs +++ b/src/compile/common.rs @@ -564,7 +564,29 @@ pub fn generate_pr_trigger(on_config: &Option, has_schedule: bool) -> } } -/// Generate CI trigger configuration +/// Generate CI trigger configuration. +/// +/// Three branches, in priority order: +/// 1. **Suppression by pipeline / schedule** — when a +/// pipeline-completion trigger or a schedule is configured, +/// `trigger: none` is emitted so unrelated commits do not also +/// queue a build (existing behaviour). +/// 2. **`on.pr.mode: policy`** — the operator has installed a Build +/// Validation branch policy and the agent author has declared +/// intent to rely on it. Emit `trigger: none` so feature-branch +/// pushes do not queue duplicate CI builds alongside the real +/// PR-typed build the policy fires. +/// 3. **Default** — otherwise emit the empty string (ADO's "trigger +/// on every branch" default). This is the `on.pr.mode: synthetic` +/// path: the synthPr Setup step will promote CI builds to PR +/// semantics, and the synthPr step's fast-exit (`AW_SYNTHETIC_PR_SKIP`) +/// handles wasted CI builds on branches without a matching PR. +/// +/// Note: synth mode must NOT narrow the CI trigger to +/// `pr.branches.include` — those are PR **target** branches, but ADO +/// `trigger:` fires on pushes **to** the listed branches. Narrowing +/// would suppress CI on the feature branches synthPr needs to react +/// to. pub fn generate_ci_trigger(on_config: &Option, has_schedule: bool) -> String { let has_pipeline_trigger = on_config .as_ref() @@ -572,10 +594,20 @@ pub fn generate_ci_trigger(on_config: &Option, has_schedule: bool) -> .is_some(); if has_pipeline_trigger || has_schedule { - "trigger: none".to_string() - } else { - String::new() + return "trigger: none".to_string(); } + + // Branch 2 — `on.pr.mode: policy`. The operator owns trigger semantics + // via a Build Validation branch policy, so the YAML CI trigger must be + // silenced to avoid duplicate builds. We still emit the `pr:` block + // (the policy uses the YAML `paths:` filter to refine queueing). + if let Some(pr) = on_config.as_ref().and_then(|o| o.pr.as_ref()) + && matches!(pr.mode, crate::compile::types::PrMode::Policy) + { + return "trigger: none".to_string(); + } + + String::new() } /// Generate pipeline resource YAML for pipeline completion triggers @@ -2359,23 +2391,63 @@ pub fn generate_agentic_depends_on( has_pipeline_filters: bool, expressions: &[&str], is_jobs_template_target: bool, + synthetic_pr_active: bool, ) -> String { let has_gate = has_pr_filters || has_pipeline_filters; - let has_setup = !setup_steps.is_empty() || has_gate; - let has_internal_condition = has_gate || !expressions.is_empty(); + let has_setup = !setup_steps.is_empty() || has_gate || synthetic_pr_active; + let has_internal_condition = has_gate || !expressions.is_empty() || synthetic_pr_active; // Build the shared condition body once. Reused across the internal-only // (standalone/1es/stage) path and the dual-branch jobs-template path. let condition_body: Option = if has_internal_condition { let mut parts = vec!["succeeded()".to_string()]; - if has_pr_filters { + // `mode: synthetic` (default): the synthPr Setup-job step may have + // decided this build should self-skip (no matching PR, wrong target + // branch, no matching changed files). Always honour that flag — + // it must trump every other reason to run. + if synthetic_pr_active { parts.push( - r"or( + r"ne(dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR_SKIP'], 'true')" + .to_string(), + ); + } + if has_pr_filters { + // With `mode: synthetic`, the agent should run when EITHER + // (a) the build is neither a real PR build NOR a synth-promoted + // CI build — in that case the gate doesn't apply (bypass.ts + // auto-passes) and the agent runs unconditionally, OR + // (b) the gate evaluator passed (`prGate.SHOULD_RUN=true`), + // covering both the real-PR-with-filter-match path and the + // synth-PR-with-filter-match path. + // + // CRITICAL: do NOT emit `eq(Build.Reason, 'PullRequest')` or + // `eq(synthPr.AW_SYNTHETIC_PR, 'true')` as standalone OR + // arms — that would let a real PR or synth-promoted build run + // the agent EVEN WHEN `pr.filters` failed (i.e. silently + // bypass the gate for the very builds it's meant to filter). + // + // With `mode: policy` (synth not active), the original + // two-arm condition is preserved verbatim. + if synthetic_pr_active { + parts.push( + r"or( + and( + ne(variables['Build.Reason'], 'PullRequest'), + ne(dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR'], 'true') + ), + eq(dependencies.Setup.outputs['prGate.SHOULD_RUN'], 'true') + )" + .to_string(), + ); + } else { + parts.push( + r"or( ne(variables['Build.Reason'], 'PullRequest'), eq(dependencies.Setup.outputs['prGate.SHOULD_RUN'], 'true') )" - .to_string(), - ); + .to_string(), + ); + } } if has_pipeline_filters { parts.push( @@ -3301,12 +3373,17 @@ pub async fn compile_shared( } } + let synthetic_pr_active = front_matter.is_synthetic_pr(); let agentic_depends_on = generate_agentic_depends_on( &front_matter.setup, has_pr_filters, has_pipeline_filters, &expressions, - matches!(front_matter.target, crate::compile::types::CompileTarget::Job), + matches!( + front_matter.target, + crate::compile::types::CompileTarget::Job + ), + synthetic_pr_active, ); let job_timeout = generate_job_timeout(front_matter); @@ -4693,8 +4770,14 @@ mod tests { let result = generate_pr_trigger(&triggers, true); assert!(result.contains("pr: none")); // When both pipeline and schedule are active, the comment must mention both reasons. - assert!(result.contains("schedule"), "should mention schedule: {result}"); - assert!(result.contains("upstream pipeline"), "should mention upstream pipeline: {result}"); + assert!( + result.contains("schedule"), + "should mention schedule: {result}" + ); + assert!( + result.contains("upstream pipeline"), + "should mention upstream pipeline: {result}" + ); } // ─── generate_ci_trigger ───────────────────────────────────────────────── @@ -4746,6 +4829,93 @@ mod tests { assert_eq!(result, "trigger: none"); } + // ─── generate_ci_trigger: on.pr.mode behaviour (issue #916) ────────────── + // + // The synth path (default, `mode: synthetic`) leaves the CI trigger at + // ADO default ("trigger on every branch") and relies on the synthPr + // Setup step to promote / skip per build. Policy path (`mode: policy`) + // emits `trigger: none` so the operator-installed Build Validation + // policy is the sole source of pipeline runs — no duplicate builds. + + #[test] + fn test_generate_ci_trigger_pr_mode_synthetic_keeps_default() { + let triggers = Some(crate::compile::types::OnConfig { + pipeline: None, + pr: Some(crate::compile::types::PrTriggerConfig { + branches: Some(crate::compile::types::BranchFilter { + include: vec!["main".into(), "release/*".into()], + exclude: vec!["users/*".into()], + }), + paths: None, + filters: None, + ..Default::default() // mode defaults to Synthetic + }), + schedule: None, + }); + let result = generate_ci_trigger(&triggers, false); + assert!( + result.is_empty(), + "mode: synthetic must leave the CI trigger at ADO default — \ + pr.branches.include lists PR TARGET branches, but ADO trigger: \ + fires on pushes TO listed branches, so narrowing would suppress \ + CI on the feature branches synthPr needs. Got: {result}" + ); + } + + #[test] + fn test_generate_ci_trigger_pr_mode_policy_emits_trigger_none() { + let triggers = Some(crate::compile::types::OnConfig { + pipeline: None, + pr: Some(crate::compile::types::PrTriggerConfig { + branches: Some(crate::compile::types::BranchFilter { + include: vec!["main".into()], + exclude: vec![], + }), + paths: None, + filters: None, + mode: crate::compile::types::PrMode::Policy, + }), + schedule: None, + }); + let result = generate_ci_trigger(&triggers, false); + assert_eq!( + result, "trigger: none", + "mode: policy must suppress the CI trigger so the operator-installed \ + branch policy is the sole source of pipeline runs (no duplicate builds)" + ); + } + + #[test] + fn test_generate_ci_trigger_pipeline_trigger_still_suppresses() { + // Pipeline-completion trigger continues to emit `trigger: none` + // regardless of any `on.pr` configuration; this branch is the + // long-standing rule that upstream-pipeline triggers exclude + // commit-driven CI. + let triggers = Some(crate::compile::types::OnConfig { + pipeline: Some(crate::compile::types::PipelineTrigger { + name: "Build".into(), + project: None, + branches: vec![], + filters: None, + }), + pr: Some(crate::compile::types::PrTriggerConfig { + branches: Some(crate::compile::types::BranchFilter { + include: vec!["main".into()], + exclude: vec![], + }), + paths: None, + filters: None, + ..Default::default() + }), + schedule: None, + }); + let result = generate_ci_trigger(&triggers, false); + assert_eq!( + result, "trigger: none", + "pipeline-completion trigger must continue to emit `trigger: none`" + ); + } + // ─── generate_pipeline_resources ───────────────────────────────────────── #[test] @@ -6420,6 +6590,7 @@ safe-outputs: }), paths: None, filters: None, + ..Default::default() }), schedule: None, }); @@ -6445,6 +6616,7 @@ safe-outputs: }), paths: None, filters: None, + ..Default::default() }), schedule: None, }); @@ -6470,6 +6642,7 @@ safe-outputs: exclude: vec![], }), filters: None, + ..Default::default() }), schedule: None, }); @@ -6495,6 +6668,7 @@ safe-outputs: exclude: vec!["tests/\ninjected: true".to_string()], }), filters: None, + ..Default::default() }), schedule: None, }); @@ -6520,6 +6694,7 @@ safe-outputs: }), paths: None, filters: None, + ..Default::default() }), schedule: None, }); @@ -6543,6 +6718,7 @@ safe-outputs: exclude: vec!["tests/**".to_string()], }), filters: None, + ..Default::default() }), schedule: None, }); @@ -6754,7 +6930,10 @@ safe-outputs: let exts = crate::compile::extensions::collect_extensions(&fm); let ctx = crate::compile::extensions::CompileContext::for_test(&fm); let result = generate_prepare_steps(&[], &exts, &ctx).unwrap(); - assert!(result.contains("elan-init.sh"), "should include elan installer"); + assert!( + result.contains("elan-init.sh"), + "should include elan installer" + ); assert!(result.contains("Lean 4"), "should include Lean prompt"); assert!( result.contains("--default-toolchain stable"), @@ -8140,14 +8319,14 @@ safe-outputs: #[test] fn test_generate_agentic_depends_on_empty_steps() { - assert!(generate_agentic_depends_on(&[], false, false, &[], false).is_empty()); + assert!(generate_agentic_depends_on(&[], false, false, &[], false, false).is_empty()); } #[test] fn test_generate_agentic_depends_on_with_steps() { let step: serde_yaml::Value = serde_yaml::from_str("bash: x").unwrap(); assert_eq!( - generate_agentic_depends_on(&[step], false, false, &[], false), + generate_agentic_depends_on(&[step], false, false, &[], false, false), "dependsOn: Setup" ); } @@ -8159,7 +8338,7 @@ safe-outputs: // dual-branch ${{ if }} template expressions so external template // parameters merge correctly. let step: serde_yaml::Value = serde_yaml::from_str("bash: x").unwrap(); - let out = generate_agentic_depends_on(&[step], false, false, &[], true); + let out = generate_agentic_depends_on(&[step], false, false, &[], true, false); // dependsOn branches assert!( out.contains("${{ if eq(length(parameters.dependsOn), 0) }}:"), @@ -8190,7 +8369,7 @@ safe-outputs: fn test_generate_agentic_depends_on_jobs_template_no_setup() { // No internal Setup, no gates: only the non-empty-external branches // are emitted (both dependsOn and condition default to omitted). - let out = generate_agentic_depends_on(&[], false, false, &[], true); + let out = generate_agentic_depends_on(&[], false, false, &[], true, false); assert!( !out.contains("${{ if eq(length(parameters.dependsOn), 0) }}:"), "should not emit empty-deps branch when no internal Setup: {out}" @@ -8218,7 +8397,7 @@ safe-outputs: // With internal PR gate, the condition block emits BOTH branches: // empty-external uses the existing internal expression verbatim; // non-empty-external ANDs the external clause into the body. - let out = generate_agentic_depends_on(&[], true, false, &[], true); + let out = generate_agentic_depends_on(&[], true, false, &[], true, false); assert!( out.contains("${{ if eq(parameters.condition, '') }}:"), "missing empty-condition branch: {out}" @@ -8240,6 +8419,61 @@ safe-outputs: ); } + #[test] + fn test_agentic_depends_on_synthetic_pr_active_emits_skip_guard_and_gate_enforced_pr_clause() { + // synthetic_pr_active=true + has_pr_filters=true → emits the + // AW_SYNTHETIC_PR_SKIP guard and a gate-enforced PR clause: real + // and synth PR builds must pass the gate (no permissive + // bypass arms). + let out = generate_agentic_depends_on(&[], true, false, &[], false, true); + assert!(out.contains("dependsOn: Setup"), "should depend on Setup"); + assert!( + out.contains("ne(dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR_SKIP'], 'true')"), + "must honour the synth-skip flag: {out}" + ); + // The PR clause must REQUIRE the gate for real-PR AND synth-PR + // builds — i.e. allow unconditional run only when neither + // applies. Both AND-NOT arms must be present. + assert!( + out.contains("ne(variables['Build.Reason'], 'PullRequest')"), + "must contain the `ne(Build.Reason, 'PullRequest')` AND-NOT arm: {out}" + ); + assert!( + out.contains("ne(dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR'], 'true')"), + "must contain the `ne(synthPr.AW_SYNTHETIC_PR, 'true')` AND-NOT arm: {out}" + ); + assert!( + out.contains("eq(dependencies.Setup.outputs['prGate.SHOULD_RUN'], 'true')"), + "must still accept gate-passed as an activation reason: {out}" + ); + // Defensive regression guards: the old permissive arms that + // bypassed the gate for any PR build MUST be gone. + assert!( + !out.contains("eq(variables['Build.Reason'], 'PullRequest')"), + "the buggy `eq(Build.Reason, PullRequest)` bypass arm must be gone: {out}" + ); + assert!( + !out.contains("eq(dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR'], 'true')"), + "the buggy `eq(synthPr.AW_SYNTHETIC_PR, true)` bypass arm must be gone: {out}" + ); + } + + #[test] + fn test_agentic_depends_on_synthetic_pr_without_filters_still_emits_skip_guard() { + // synthetic_pr_active=true but no filters → Setup job still + // exists (the synthPr step lives there) so the dependsOn must + // be present and the skip guard must apply. + let out = generate_agentic_depends_on(&[], false, false, &[], false, true); + assert!( + out.contains("dependsOn: Setup"), + "should depend on Setup: {out}" + ); + assert!( + out.contains("ne(dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR_SKIP'], 'true')"), + "must honour the synth-skip flag even without filters: {out}" + ); + } + #[test] fn test_generate_finalize_steps_empty() { assert!(generate_finalize_steps(&[]).is_empty()); diff --git a/src/compile/extensions/ado_script.rs b/src/compile/extensions/ado_script.rs index 41197fea..164cfef0 100644 --- a/src/compile/extensions/ado_script.rs +++ b/src/compile/extensions/ado_script.rs @@ -35,6 +35,11 @@ pub(crate) const IMPORT_EVAL_PATH: &str = "/tmp/ado-aw-scripts/ado-script/import /// Consumed by `src/compile/extensions/exec_context/pr.rs` to invoke /// the bundle from the PR contributor's prepare step. pub(crate) const EXEC_CONTEXT_PR_PATH: &str = "/tmp/ado-aw-scripts/ado-script/exec-context-pr.js"; +/// Path to the synthetic-PR-context bundle inside the unpacked +/// `ado-script.zip`. Runs in the Setup job before `prGate`; consumed +/// by [`AdoScriptExtension::setup_steps`]. +pub(crate) const EXEC_CONTEXT_PR_SYNTH_PATH: &str = + "/tmp/ado-aw-scripts/ado-script/exec-context-pr-synth.js"; const RELEASE_BASE_URL: &str = "https://github.com/githubnext/ado-aw/releases/download"; /// Single always-on extension that owns all `ado-script` bundle wiring. @@ -52,6 +57,30 @@ pub struct AdoScriptExtension { /// shared `exec_context_pr_active` predicate so this stays in /// lock-step with `ExecContextExtension`'s own activation gate. pub exec_context_pr_active: bool, + /// PR trigger config required to build `PR_SYNTH_SPEC`. `Some(_)` + /// is the single source of truth for "synthetic-from-ci path is + /// active for this agent" — `is_some()` replaces what used to be a + /// separate `synthetic_pr_active: bool` field, eliminating the + /// invariant that the two had to be set together. Drives: + /// + /// - Setup-job install/download fire (even with no `filters:`). + /// - Setup-job `synthPr` step emission (before any gate step). + /// - Downstream env coalescing (handled in `compile-coalesce-env`). + /// + /// Cloned from the front-matter because the extension outlives the + /// borrow of `FrontMatter` in `collect_extensions`. + pub pr_trigger_for_synth: Option, +} + +impl AdoScriptExtension { + /// Whether the synthetic-from-ci path is active for this agent. + /// Set when `on.pr.mode == Synthetic` (the default), in which case + /// `pr_trigger_for_synth` is populated. The compile-time + /// invariant "if active, the spec must be available" is encoded in + /// the field type, so this is just a thin accessor. + pub fn synthetic_pr_active(&self) -> bool { + self.pr_trigger_for_synth.is_some() + } } impl AdoScriptExtension { @@ -143,6 +172,35 @@ fn resolver_step() -> String { ) } +/// The synthetic-PR-context step that runs in the Setup job BEFORE +/// `prGate`. Looks up the open PR for `Build.SourceBranch` via the +/// ADO REST API and emits `AW_SYNTHETIC_PR*` outputs that downstream +/// gate + exec-context-pr steps coalesce with the real +/// `System.PullRequest.*` variables. +/// +/// `condition: ne(Build.Reason, 'PullRequest')` short-circuits the +/// bundle on real PR builds (the bundle would no-op anyway, but the +/// step-level condition skips the env-block evaluation too). +fn synthetic_pr_step(spec_b64: &str) -> String { + format!( + r#"- bash: | + set -euo pipefail + node '{EXEC_CONTEXT_PR_SYNTH_PATH}' + name: synthPr + displayName: "Resolve synthetic PR context" + condition: and(succeeded(), ne(variables['Build.Reason'], 'PullRequest')) + env: + SYSTEM_ACCESSTOKEN: $(System.AccessToken) + ADO_COLLECTION_URI: $(System.CollectionUri) + ADO_PROJECT: $(System.TeamProject) + ADO_REPO_ID: $(Build.Repository.ID) + BUILD_REASON: $(Build.Reason) + BUILD_REPOSITORY_PROVIDER: $(Build.Repository.Provider) + BUILD_SOURCEBRANCH: $(Build.SourceBranch) + PR_SYNTH_SPEC: "{spec_b64}""# + ) +} + impl CompilerExtension for AdoScriptExtension { fn name(&self) -> &str { "ado-script" @@ -162,15 +220,24 @@ impl CompilerExtension for AdoScriptExtension { fn setup_steps(&self, _ctx: &CompileContext) -> Result> { let (pr_checks, pipeline_checks) = self.lowered_checks(); - if pr_checks.is_empty() && pipeline_checks.is_empty() { + if pr_checks.is_empty() && pipeline_checks.is_empty() && !self.synthetic_pr_active() { return Ok(vec![]); } let mut steps = install_and_download_steps(); + // `pr_trigger_for_synth.is_some()` is the type-level encoding + // of "synth path is active for this agent" — no separate flag + // to keep in lock-step. If `Some(_)`, the spec is guaranteed + // available. + if let Some(pr) = self.pr_trigger_for_synth.as_ref() { + let spec_b64 = crate::compile::filter_ir::build_pr_synth_spec(pr)?; + steps.push(synthetic_pr_step(&spec_b64)); + } if !pr_checks.is_empty() { steps.push(compile_gate_step_external( GateContext::PullRequest, &pr_checks, GATE_EVAL_PATH, + self.synthetic_pr_active(), )?); } if !pipeline_checks.is_empty() { @@ -178,6 +245,10 @@ impl CompilerExtension for AdoScriptExtension { GateContext::PipelineCompletion, &pipeline_checks, GATE_EVAL_PATH, + // Pipeline-completion gates never observe synthetic PR + // semantics; the coalesce wiring only applies to + // PullRequest gates. + false, )?); } Ok(steps) @@ -412,6 +483,7 @@ mod tests { pipeline_filters: pipeline, inlined_imports: inlined, exec_context_pr_active: false, + pr_trigger_for_synth: None, } } @@ -456,6 +528,72 @@ mod tests { assert!(steps[2].contains("node '/tmp/ado-aw-scripts/ado-script/gate.js'")); } + #[test] + fn setup_steps_emits_synth_step_when_synthetic_pr_active_without_gate() { + use crate::compile::types::{BranchFilter, PrTriggerConfig}; + let ext = AdoScriptExtension { + pr_filters: None, + pipeline_filters: None, + inlined_imports: true, + exec_context_pr_active: false, + pr_trigger_for_synth: Some(PrTriggerConfig { + branches: Some(BranchFilter { + include: vec!["main".into()], + exclude: vec![], + }), + paths: None, + filters: None, + ..Default::default() + }), + }; + let fm: FrontMatter = serde_yaml::from_str("name: t\ndescription: t").unwrap(); + let ctx = CompileContext::for_test(&fm); + let steps = ext.setup_steps(&ctx).unwrap(); + assert_eq!(steps.len(), 3, "install + download + synthPr"); + assert!(steps[0].contains("NodeTool@0")); + assert!(steps[1].contains("Download ado-aw scripts")); + assert!( + steps[2].contains("name: synthPr"), + "third step must be synthPr" + ); + assert!(steps[2].contains("exec-context-pr-synth.js")); + assert!(steps[2].contains("PR_SYNTH_SPEC:")); + assert!(steps[2].contains("ne(variables['Build.Reason'], 'PullRequest')")); + } + + #[test] + fn setup_steps_emits_synth_step_before_gate_when_both_active() { + use crate::compile::types::{BranchFilter, PrTriggerConfig}; + let filters = PrFilters { + labels: Some(LabelFilter { + any_of: vec!["run-agent".into()], + ..Default::default() + }), + ..Default::default() + }; + let ext = AdoScriptExtension { + pr_filters: Some(filters), + pipeline_filters: None, + inlined_imports: true, + exec_context_pr_active: false, + pr_trigger_for_synth: Some(PrTriggerConfig { + branches: Some(BranchFilter { + include: vec!["main".into()], + exclude: vec![], + }), + paths: None, + filters: None, + ..Default::default() + }), + }; + let fm: FrontMatter = serde_yaml::from_str("name: t\ndescription: t").unwrap(); + let ctx = CompileContext::for_test(&fm); + let steps = ext.setup_steps(&ctx).unwrap(); + assert_eq!(steps.len(), 4, "install + download + synthPr + prGate"); + assert!(steps[2].contains("name: synthPr")); + assert!(steps[3].contains("name: prGate")); + } + #[test] fn gate_and_import_eval_paths_consistent_with_download_step() { let extract_dir = "/tmp/ado-aw-scripts/"; @@ -654,11 +792,8 @@ mod tests { #[test] fn rejects_absolute_posix_path_at_compile_time() { let workspace = TestWorkspace::new(); - let err = resolve_imports_inline( - "{{#runtime-import /etc/passwd}}", - &workspace.path, - ) - .unwrap_err(); + let err = + resolve_imports_inline("{{#runtime-import /etc/passwd}}", &workspace.path).unwrap_err(); assert!( err.to_string().contains("absolute paths are not allowed"), "expected absolute-path rejection, got: {err}" @@ -716,11 +851,8 @@ mod tests { #[test] fn rejects_path_containing_closing_brace() { let workspace = TestWorkspace::new(); - let err = resolve_imports_inline( - "{{#runtime-import foo}bar.md}}", - &workspace.path, - ) - .unwrap_err(); + let err = + resolve_imports_inline("{{#runtime-import foo}bar.md}}", &workspace.path).unwrap_err(); assert!( err.to_string().contains("is not allowed"), "expected `}}` rejection, got: {err}" @@ -734,13 +866,11 @@ mod tests { #[test] fn rejects_relative_path_with_dotdot_segment() { let workspace = TestWorkspace::new(); - let err = resolve_imports_inline( - "{{#runtime-import ../escape.md}}", - &workspace.path, - ) - .unwrap_err(); + let err = resolve_imports_inline("{{#runtime-import ../escape.md}}", &workspace.path) + .unwrap_err(); assert!( - err.to_string().contains("'..' path components are not allowed"), + err.to_string() + .contains("'..' path components are not allowed"), "expected '..' rejection, got: {err}" ); } @@ -748,13 +878,12 @@ mod tests { #[test] fn rejects_path_with_embedded_dotdot_segment() { let workspace = TestWorkspace::new(); - let err = resolve_imports_inline( - "{{#runtime-import sub/../../escape.md}}", - &workspace.path, - ) - .unwrap_err(); + let err = + resolve_imports_inline("{{#runtime-import sub/../../escape.md}}", &workspace.path) + .unwrap_err(); assert!( - err.to_string().contains("'..' path components are not allowed"), + err.to_string() + .contains("'..' path components are not allowed"), "expected '..' rejection, got: {err}" ); } @@ -772,7 +901,8 @@ mod tests { ) .unwrap_err(); assert!( - err.to_string().contains("'..' path components are not allowed"), + err.to_string() + .contains("'..' path components are not allowed"), "expected '..' rejection, got: {err}" ); } @@ -780,13 +910,12 @@ mod tests { #[test] fn rejects_backslash_dotdot_segment_on_windows_style_paths() { let workspace = TestWorkspace::new(); - let err = resolve_imports_inline( - r"{{#runtime-import sub\..\..\escape.md}}", - &workspace.path, - ) - .unwrap_err(); + let err = + resolve_imports_inline(r"{{#runtime-import sub\..\..\escape.md}}", &workspace.path) + .unwrap_err(); assert!( - err.to_string().contains("'..' path components are not allowed"), + err.to_string() + .contains("'..' path components are not allowed"), "expected '..' rejection, got: {err}" ); } @@ -800,16 +929,8 @@ mod tests { workspace.write("..hidden.md", "DOTHIDDEN"); workspace.write("name..md", "DOUBLE"); - let a = resolve_imports_inline( - "{{#runtime-import ..hidden.md}}", - &workspace.path, - ) - .unwrap(); - let b = resolve_imports_inline( - "{{#runtime-import name..md}}", - &workspace.path, - ) - .unwrap(); + let a = resolve_imports_inline("{{#runtime-import ..hidden.md}}", &workspace.path).unwrap(); + let b = resolve_imports_inline("{{#runtime-import name..md}}", &workspace.path).unwrap(); assert_eq!(a, "DOTHIDDEN"); assert_eq!(b, "DOUBLE"); diff --git a/src/compile/extensions/exec_context/mod.rs b/src/compile/extensions/exec_context/mod.rs index b8302303..b5c19c46 100644 --- a/src/compile/extensions/exec_context/mod.rs +++ b/src/compile/extensions/exec_context/mod.rs @@ -101,6 +101,11 @@ pub struct ExecContextExtension { /// means "is `on.pr` configured" — future trigger contributors /// will OR in their own checks here. any_contributor_active: bool, + /// Whether `on.pr.mode == Synthetic` for this agent. Passed through + /// to the PR contributor so it can emit coalesced + /// `SYSTEM_PULLREQUEST_*` env vars (real value preferred, synthPr + /// Setup-job output as fallback). + synthetic_pr_active: bool, } impl ExecContextExtension { @@ -118,11 +123,12 @@ impl ExecContextExtension { // so unit tests that construct a custom `config` (separate // from `front_matter.execution_context`) still see the right // activation answer. - let any_contributor_active = - pr_contributor_will_activate_with_cfg(&config, front_matter); + let any_contributor_active = pr_contributor_will_activate_with_cfg(&config, front_matter); + let synthetic_pr_active = front_matter.is_synthetic_pr(); Self { config, any_contributor_active, + synthetic_pr_active, } } @@ -134,7 +140,14 @@ impl ExecContextExtension { // "on by default when on.pr is configured" behaviour without // the user having to write `execution-context.pr: {}`. let pr_cfg = self.config.pr.clone().unwrap_or_default(); - vec![Contributor::Pr(PrContextContributor::new(pr_cfg))] + // The PR contributor needs to know whether `mode: synthetic` + // is on so it can emit coalesced SYSTEM_PULLREQUEST_* env vars + // (real value preferred, synthPr output as fallback). + let synthetic_pr_active = self.synthetic_pr_active; + vec![Contributor::Pr(PrContextContributor::new( + pr_cfg, + synthetic_pr_active, + ))] } } @@ -206,9 +219,7 @@ mod tests { //! unit-test time rather than at E2E time. use super::*; - use crate::compile::types::{ - ExecutionContextConfig, FrontMatter, PrContextConfig, - }; + use crate::compile::types::{ExecutionContextConfig, FrontMatter, PrContextConfig}; /// Parse a minimal markdown agent into a `FrontMatter`. fn parse_fm(src: &str) -> FrontMatter { @@ -234,8 +245,10 @@ mod tests { /// `should_activate`, this assertion trips. #[test] fn required_bash_commands_matches_pr_contributor_active_default() { - let ext = - ExecContextExtension::new(ExecutionContextConfig::default(), &pr_triggered_front_matter()); + let ext = ExecContextExtension::new( + ExecutionContextConfig::default(), + &pr_triggered_front_matter(), + ); let cmds = ext.required_bash_commands(); assert!( !cmds.is_empty(), @@ -288,8 +301,10 @@ mod tests { /// no commands. Mirrors `should_activate`'s `on.pr` gate. #[test] fn required_bash_commands_suppressed_without_on_pr() { - let ext = - ExecContextExtension::new(ExecutionContextConfig::default(), &no_trigger_front_matter()); + let ext = ExecContextExtension::new( + ExecutionContextConfig::default(), + &no_trigger_front_matter(), + ); assert!( ext.required_bash_commands().is_empty(), "without on.pr configured, required_bash_commands must be empty" diff --git a/src/compile/extensions/exec_context/pr.rs b/src/compile/extensions/exec_context/pr.rs index a7d1f282..ddff42fb 100644 --- a/src/compile/extensions/exec_context/pr.rs +++ b/src/compile/extensions/exec_context/pr.rs @@ -68,11 +68,19 @@ use super::contributor::ContextContributor; /// (unless explicitly disabled via `execution-context.pr.enabled: false`). pub(super) struct PrContextContributor { config: PrContextConfig, + /// Whether `on.pr.mode == Synthetic` for this agent. Drives + /// emission of the coalesced `SYSTEM_PULLREQUEST_*` env vars so the + /// bundle reads either real PR identifiers (true PR builds) or the + /// `synthPr` Setup-job outputs (CI builds promoted via synth). + synthetic_pr_active: bool, } impl PrContextContributor { - pub(super) fn new(config: PrContextConfig) -> Self { - Self { config } + pub(super) fn new(config: PrContextConfig, synthetic_pr_active: bool) -> Self { + Self { + config, + synthetic_pr_active, + } } } @@ -113,19 +121,58 @@ impl ContextContributor for PrContextContributor { // block. Node receives it on `process.env` and passes it to // the spawned `git` subprocess via `GIT_CONFIG_*` env vars // (never argv). It is NEVER visible to the agent step. + // + // When `mode: synthetic` is on, the PR-identifier env vars + // are emitted using `$[ coalesce(...) ]` so the bundle picks + // up either the real `System.PullRequest.*` (on a true PR + // build) OR the synthPr Setup-job output (on a CI build + // promoted via exec-context-pr-synth.js). The step's + // condition is also broadened to accept synth-promoted builds. + // + // Cross-job reference is correct here: this step runs in the + // **Agent** job (which depends on Setup), so + // `dependencies.Setup.outputs['synthPr.X']` resolves at runtime. + // (Same-job references would need `variables['synthPr.X']` + // instead — used by the gate step inside the Setup job itself.) + // Runtime expressions `$[ ... ]` are documented as valid in + // step-level `env:` blocks; see + // . + // `System.PullRequest.TargetBranch` is `refs/heads/` (full + // ref form), matching the `targetRefName` shape returned by the + // ADO REST API and stored in `AW_SYNTHETIC_PR_TARGETBRANCH`, so + // the coalesce yields a consistent value either way. + // `$[ ... ]` runtime expressions are wrapped in YAML double + // quotes because their values contain single quotes (e.g. + // `variables['System.PullRequest.PullRequestId']`). ADO accepts + // them unquoted in practice, but double-quoting matches the + // form shown in ADO docs and is strictly conformant to the + // YAML spec (which reserves `'` as a scalar indicator). + let (pr_id_macro, target_branch_macro, condition) = if self.synthetic_pr_active { + ( + "\"$[ coalesce(variables['System.PullRequest.PullRequestId'], dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR_ID']) ]\"", + "\"$[ coalesce(variables['System.PullRequest.TargetBranch'], dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR_TARGETBRANCH']) ]\"", + "or(eq(variables['Build.Reason'], 'PullRequest'), eq(dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR'], 'true'))", + ) + } else { + ( + "$(System.PullRequest.PullRequestId)", + "$(System.PullRequest.TargetBranch)", + "eq(variables['Build.Reason'], 'PullRequest')", + ) + }; format!( r#"- bash: | set -euo pipefail node '{EXEC_CONTEXT_PR_PATH}' env: SYSTEM_ACCESSTOKEN: $(System.AccessToken) - SYSTEM_PULLREQUEST_PULLREQUESTID: $(System.PullRequest.PullRequestId) - SYSTEM_PULLREQUEST_TARGETBRANCH: $(System.PullRequest.TargetBranch) + SYSTEM_PULLREQUEST_PULLREQUESTID: {pr_id_macro} + SYSTEM_PULLREQUEST_TARGETBRANCH: {target_branch_macro} SYSTEM_TEAMPROJECT: $(System.TeamProject) BUILD_REPOSITORY_NAME: $(Build.Repository.Name) BUILD_SOURCESDIRECTORY: $(Build.SourcesDirectory) displayName: "Stage PR execution context (aw-context/pr/*)" - condition: eq(variables['Build.Reason'], 'PullRequest')"# + condition: {condition}"# ) } @@ -149,3 +196,97 @@ impl ContextContributor for PrContextContributor { ] } } + +#[cfg(test)] +mod tests { + //! Direct unit tests for `PrContextContributor::prepare_step` — + //! pins both the `mode: synthetic` (default) and `mode: policy` + //! emitted YAML shapes for the env-coalesce macros and the + //! step-level `condition:`. Catches accidental regressions of the + //! coalesce wiring without round-tripping through a full snapshot + //! fixture. + use super::*; + use crate::compile::extensions::CompileContext; + use crate::compile::types::{FrontMatter, PrContextConfig}; + + fn parse_fm(src: &str) -> FrontMatter { + let (fm, _) = crate::compile::common::parse_markdown(src).unwrap(); + fm + } + + fn pr_fm() -> FrontMatter { + parse_fm( + "---\nname: test\ndescription: test\non:\n pr:\n branches:\n include: [main]\n---\n", + ) + } + + #[test] + fn prepare_step_synth_active_emits_coalesced_env_and_broadened_condition() { + let contributor = PrContextContributor::new(PrContextConfig::default(), true); + let fm = pr_fm(); + let ctx = CompileContext::for_test(&fm); + let step = contributor.prepare_step(&ctx); + + // Env: PR id + target branch are coalesced via cross-job runtime + // expressions wrapped in YAML double quotes (Agent job depends + // on Setup, so `dependencies.Setup.outputs[...]` is the correct + // form here — distinct from the gate step which is same-job). + assert!( + step.contains( + "SYSTEM_PULLREQUEST_PULLREQUESTID: \"$[ coalesce(variables['System.PullRequest.PullRequestId'], dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR_ID']) ]\"" + ), + "synth-active prepare step must coalesce PR id with synthPr fallback: {step}" + ); + assert!( + step.contains( + "SYSTEM_PULLREQUEST_TARGETBRANCH: \"$[ coalesce(variables['System.PullRequest.TargetBranch'], dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR_TARGETBRANCH']) ]\"" + ), + "synth-active prepare step must coalesce target branch with synthPr fallback: {step}" + ); + + // Condition: broadened to accept real PR builds OR synth-promoted + // CI builds. + assert!( + step.contains( + "condition: or(eq(variables['Build.Reason'], 'PullRequest'), eq(dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR'], 'true'))" + ), + "synth-active prepare step must broaden the condition to accept synth-promoted builds: {step}" + ); + } + + #[test] + fn prepare_step_synth_inactive_emits_plain_macros_and_narrow_condition() { + let contributor = PrContextContributor::new(PrContextConfig::default(), false); + let fm = pr_fm(); + let ctx = CompileContext::for_test(&fm); + let step = contributor.prepare_step(&ctx); + + // Env: plain `$(...)` macros for the real System.PullRequest.* + // predefined variables — no coalesce, no quoting. + assert!( + step.contains("SYSTEM_PULLREQUEST_PULLREQUESTID: $(System.PullRequest.PullRequestId)"), + "synth-inactive prepare step must use the plain ADO macro form: {step}" + ); + assert!( + step.contains("SYSTEM_PULLREQUEST_TARGETBRANCH: $(System.PullRequest.TargetBranch)"), + "synth-inactive prepare step must use the plain ADO macro form: {step}" + ); + + // Condition: narrow to real PR builds only. + assert!( + step.contains("condition: eq(variables['Build.Reason'], 'PullRequest')"), + "synth-inactive prepare step must keep the narrow PR-build condition: {step}" + ); + + // Defensive: the synth-mode signature MUST NOT appear when the + // synth path is inactive. + assert!( + !step.contains("synthPr.AW_SYNTHETIC_PR"), + "synth-inactive prepare step must not reference any synthPr Setup-job output: {step}" + ); + assert!( + !step.contains("coalesce("), + "synth-inactive prepare step must not emit a coalesce expression: {step}" + ); + } +} diff --git a/src/compile/extensions/mod.rs b/src/compile/extensions/mod.rs index a38d7f4c..c56e784e 100644 --- a/src/compile/extensions/mod.rs +++ b/src/compile/extensions/mod.rs @@ -625,22 +625,22 @@ macro_rules! extension_enum { mod ado_aw_marker; pub mod ado_script; -mod exec_context; mod azure_cli; +mod exec_context; mod github; mod safe_outputs; // Re-export tool/runtime extensions from their colocated homes -pub use ado_aw_marker::AdoAwMarkerExtension; -pub use azure_cli::AzureCliExtension; pub use crate::runtimes::dotnet::DotnetExtension; pub use crate::runtimes::lean::LeanExtension; pub use crate::runtimes::node::NodeExtension; pub use crate::runtimes::python::PythonExtension; pub use crate::tools::azure_devops::AzureDevOpsExtension; pub use crate::tools::cache_memory::CacheMemoryExtension; +pub use ado_aw_marker::AdoAwMarkerExtension; pub use ado_script::AdoScriptExtension; -pub use exec_context::{pr_contributor_will_activate, ExecContextExtension}; +pub use azure_cli::AzureCliExtension; +pub use exec_context::{ExecContextExtension, pr_contributor_will_activate}; pub use github::GitHubExtension; pub use safe_outputs::SafeOutputsExtension; @@ -697,18 +697,39 @@ pub fn collect_extensions(front_matter: &FrontMatter) -> Vec { Extension::AdoAwMarker(AdoAwMarkerExtension), Extension::GitHub(GitHubExtension), Extension::SafeOutputs(SafeOutputsExtension), - Extension::AdoScript(Box::new(AdoScriptExtension { - pr_filters: front_matter.pr_filters().cloned(), - pipeline_filters: front_matter.pipeline_filters().cloned(), - inlined_imports: front_matter.inlined_imports, - // Tell the ado-script extension whether the PR-context - // contributor will activate so it can fire the Agent-job - // install/download even when `inlined-imports: true` (no - // import.js needed). The two extensions stay loosely - // coupled: ExecContextExtension owns invoking the bundle; - // AdoScriptExtension owns installing it. Shared helper - // keeps the activation predicate in lock-step. - exec_context_pr_active: pr_contributor_will_activate(front_matter), + Extension::AdoScript(Box::new({ + // PR trigger config drives both the PR-context contributor + // (exec-context-pr.js) and the synthetic-from-ci path + // (exec-context-pr-synth.js). + // + // `pr_trigger_for_synth` is the SINGLE source of truth for + // synth-path activation: when `Some(_)` the extension emits + // the synthPr Setup-job step and downstream wiring; when + // `None` it doesn't. The previous separate `bool` flag is + // now derived via `AdoScriptExtension::synthetic_pr_active()`. + // The activation predicate (`mode == Synthetic`) lives in + // `FrontMatter::is_synthetic_pr()` so it stays in lock-step + // with the other two call sites (`compile_shared` and + // `ExecContextExtension::new`). + let pr_trigger_for_synth = if front_matter.is_synthetic_pr() { + front_matter.pr_trigger().cloned() + } else { + None + }; + AdoScriptExtension { + pr_filters: front_matter.pr_filters().cloned(), + pipeline_filters: front_matter.pipeline_filters().cloned(), + inlined_imports: front_matter.inlined_imports, + // Tell the ado-script extension whether the PR-context + // contributor will activate so it can fire the Agent-job + // install/download even when `inlined-imports: true` (no + // import.js needed). The two extensions stay loosely + // coupled: ExecContextExtension owns invoking the bundle; + // AdoScriptExtension owns installing it. Shared helper + // keeps the activation predicate in lock-step. + exec_context_pr_active: pr_contributor_will_activate(front_matter), + pr_trigger_for_synth, + } })), // Always-on execution-context extension. Owns the `aw-context/` // precompute pipeline. Defaults to `ExecutionContextConfig::default()` @@ -717,10 +738,7 @@ pub fn collect_extensions(front_matter: &FrontMatter) -> Vec { // the block + having no `on.pr` produces zero output. See // `extensions/exec_context/`. Extension::ExecContext(ExecContextExtension::new( - front_matter - .execution_context - .clone() - .unwrap_or_default(), + front_matter.execution_context.clone().unwrap_or_default(), front_matter, )), // Always-on Azure CLI. Tool phase — mounts host /opt/az and diff --git a/src/compile/filter_ir.rs b/src/compile/filter_ir.rs index 235e2506..7e6a6cb2 100644 --- a/src/compile/filter_ir.rs +++ b/src/compile/filter_ir.rs @@ -1118,10 +1118,31 @@ pub fn build_gate_spec(ctx: GateContext, checks: &[FilterCheck]) -> anyhow::Resu /// Compile filter checks into a bash gate step using an external evaluator /// script. ADO variables are passed via the step's `env:` block (idiomatic /// ADO pattern), and the gate spec is base64-encoded in GATE_SPEC. +/// +/// When `synthetic_pr_active` is true AND `ctx == GateContext::PullRequest`, +/// PR-identifier env vars (`ADO_PR_ID`, `ADO_SOURCE_BRANCH`, +/// `ADO_TARGET_BRANCH`) are emitted using `$[ coalesce(...) ]` so they +/// pick up either the real `System.PullRequest.*` variables (on a true +/// PR build) OR the `synthPr` Setup-job outputs (on a CI build promoted +/// by `exec-context-pr-synth.js`). Also exports `AW_SYNTHETIC_PR` so +/// `gate/bypass.ts` knows to skip the "not a PR build" bypass. +/// +/// **Same-job vs cross-job reference**: this gate step lives in the +/// **Setup job** (`AdoScriptExtension::setup_steps` returns it), the +/// same job as `synthPr`. Within the producing job, the cross-job form +/// `dependencies.Setup.outputs['synthPr.X']` is undefined (a job has +/// no entry for itself in `dependencies`), so we use the same-job +/// runtime expression `variables['synthPr.X']` instead, which resolves +/// step output variables added to the job's variable scope by prior +/// `isOutput=true` setvariable commands. See +/// . +/// Runtime expressions (`$[ ... ]`) are valid in step-level `env:` +/// blocks per the same docs. pub fn compile_gate_step_external( ctx: GateContext, checks: &[FilterCheck], evaluator_path: &str, + synthetic_pr_active: bool, ) -> anyhow::Result { use base64::{Engine as _, engine::general_purpose::STANDARD}; @@ -1147,13 +1168,128 @@ pub fn compile_gate_step_external( step.push_str(" SYSTEM_ACCESSTOKEN: $(System.AccessToken)\n"); step.push_str(&format!(" GATE_SPEC: \"{}\"\n", spec_b64)); + // Synthetic-from-ci flag: tells gate/bypass.ts that this CI build + // has been promoted to PR semantics, so the "not a PullRequest + // build" bypass must not auto-pass. Always safe to emit (the gate + // checks it strictly for the literal "true"), but only meaningful + // for PR gates. Same-job ref via `variables['synthPr.X']` — see + // function doc-comment for why this is NOT `dependencies.Setup...`. + let pr_synth_active = synthetic_pr_active && matches!(ctx, GateContext::PullRequest); + if pr_synth_active { + // YAML-quote runtime expressions whose value contains single quotes. + // Per ADO docs, `$[ ... ]` runtime expressions are valid in step + // `env:` blocks; wrapping in double quotes keeps the value + // strictly conformant to the YAML spec (which reserves `'` as a + // scalar indicator) and matches the form shown in ADO docs. + step.push_str( + " AW_SYNTHETIC_PR: \"$[ coalesce(variables['synthPr.AW_SYNTHETIC_PR'], '') ]\"\n", + ); + } + for (env_var, ado_macro) in &exports { - step.push_str(&format!(" {}: {}\n", env_var, ado_macro)); + let macro_str = if pr_synth_active { + match *env_var { + "ADO_PR_ID" => { + "\"$[ coalesce(variables['System.PullRequest.PullRequestId'], variables['synthPr.AW_SYNTHETIC_PR_ID']) ]\"" + } + "ADO_SOURCE_BRANCH" => { + "\"$[ coalesce(variables['System.PullRequest.SourceBranch'], variables['synthPr.AW_SYNTHETIC_PR_SOURCEBRANCH']) ]\"" + } + "ADO_TARGET_BRANCH" => { + "\"$[ coalesce(variables['System.PullRequest.TargetBranch'], variables['synthPr.AW_SYNTHETIC_PR_TARGETBRANCH']) ]\"" + } + _ => ado_macro, + } + } else { + ado_macro + }; + step.push_str(&format!(" {}: {}\n", env_var, macro_str)); } Ok(step) } +// ─── PR synthetic-from-ci spec (mode: synthetic) ──────────────────────────── + +/// Base64-encoded JSON spec consumed by the `exec-context-pr-synth.js` +/// bundle at runtime. Carries the PR branch/path filters the agent +/// declared in front-matter so the bundle can match an active PR by +/// `sourceRefName` and filter by `targetRefName` + changed-file paths. +/// +/// Shape: +/// ```json +/// { +/// "branches": { "include": [...], "exclude": [...] }, +/// "paths": { "include": [...], "exclude": [...] } +/// } +/// ``` +/// +/// All four arrays are always present (possibly empty) for shape stability — +/// the bundle can rely on the fields existing. +#[derive(Debug, serde::Serialize)] +struct PrSynthSpec { + branches: PrSynthGlobs, + paths: PrSynthGlobs, +} + +#[derive(Debug, serde::Serialize)] +struct PrSynthGlobs { + include: Vec, + exclude: Vec, +} + +/// Maximum decoded size of `PR_SYNTH_SPEC`. Matches the spirit of the +/// `GATE_SPEC` 8 KiB ceiling — synth specs are smaller (no checks, no +/// facts), but the same defence-in-depth bound prevents pathological +/// front-matter from blowing up the bundle's parser. +const PR_SYNTH_SPEC_MAX_BYTES: usize = 8 * 1024; + +/// Build the base64-encoded `PR_SYNTH_SPEC` value for the given PR +/// trigger configuration. +/// +/// The returned string is safe to embed inside a YAML double-quoted +/// scalar (the base64 alphabet contains no characters that require +/// YAML escaping). +pub fn build_pr_synth_spec(pr: &crate::compile::types::PrTriggerConfig) -> anyhow::Result { + use base64::{Engine as _, engine::general_purpose::STANDARD}; + + let spec = PrSynthSpec { + branches: PrSynthGlobs { + include: pr + .branches + .as_ref() + .map(|b| b.include.clone()) + .unwrap_or_default(), + exclude: pr + .branches + .as_ref() + .map(|b| b.exclude.clone()) + .unwrap_or_default(), + }, + paths: PrSynthGlobs { + include: pr + .paths + .as_ref() + .map(|p| p.include.clone()) + .unwrap_or_default(), + exclude: pr + .paths + .as_ref() + .map(|p| p.exclude.clone()) + .unwrap_or_default(), + }, + }; + + let json = serde_json::to_string(&spec)?; + anyhow::ensure!( + json.len() <= PR_SYNTH_SPEC_MAX_BYTES, + "PR_SYNTH_SPEC serialised size {} exceeds {}-byte cap; reduce the number/length of on.pr branches/paths globs", + json.len(), + PR_SYNTH_SPEC_MAX_BYTES + ); + Ok(STANDARD.encode(json.as_bytes())) +} + /// Collect ADO macro exports needed by the given checks. fn collect_ado_exports( checks: &[FilterCheck], @@ -1246,6 +1382,73 @@ mod tests { use super::*; use crate::compile::types::*; + // ─── PR_SYNTH_SPEC tests ──────────────────────────────────────────── + + #[test] + fn test_build_pr_synth_spec_roundtrip() { + use base64::{Engine as _, engine::general_purpose::STANDARD}; + use serde_json::Value; + + let pr = PrTriggerConfig { + branches: Some(BranchFilter { + include: vec!["main".into(), "release/*".into()], + exclude: vec!["test/*".into()], + }), + paths: Some(PathFilter { + include: vec!["src/*".into()], + exclude: vec!["docs/*".into()], + }), + filters: None, + ..Default::default() + }; + let b64 = build_pr_synth_spec(&pr).expect("synth spec must build"); + let decoded = STANDARD.decode(b64.as_bytes()).expect("must decode base64"); + let parsed: Value = serde_json::from_slice(&decoded).expect("must be valid JSON"); + assert_eq!( + parsed["branches"]["include"], + serde_json::json!(["main", "release/*"]) + ); + assert_eq!(parsed["branches"]["exclude"], serde_json::json!(["test/*"])); + assert_eq!(parsed["paths"]["include"], serde_json::json!(["src/*"])); + assert_eq!(parsed["paths"]["exclude"], serde_json::json!(["docs/*"])); + } + + #[test] + fn test_build_pr_synth_spec_omitted_arrays_become_empty() { + use base64::{Engine as _, engine::general_purpose::STANDARD}; + use serde_json::Value; + + let pr = PrTriggerConfig::default(); + let b64 = build_pr_synth_spec(&pr).expect("synth spec must build"); + let decoded = STANDARD.decode(b64.as_bytes()).unwrap(); + let parsed: Value = serde_json::from_slice(&decoded).unwrap(); + assert_eq!(parsed["branches"]["include"], serde_json::json!([])); + assert_eq!(parsed["branches"]["exclude"], serde_json::json!([])); + assert_eq!(parsed["paths"]["include"], serde_json::json!([])); + assert_eq!(parsed["paths"]["exclude"], serde_json::json!([])); + } + + #[test] + fn test_build_pr_synth_spec_rejects_oversize() { + // Generate enough branch globs to blow past the 8 KiB cap. + let pr = PrTriggerConfig { + branches: Some(BranchFilter { + include: (0..1000) + .map(|i| format!("very/long/branch/glob/pattern/{i}")) + .collect(), + exclude: vec![], + }), + paths: None, + filters: None, + ..Default::default() + }; + let err = build_pr_synth_spec(&pr).expect_err("oversize spec must fail"); + assert!( + err.to_string().contains("PR_SYNTH_SPEC"), + "error must mention spec: {err}" + ); + } + // ─── Fact tests ───────────────────────────────────────────────────── #[test] @@ -1556,6 +1759,7 @@ mod tests { GateContext::PullRequest, &[], "/tmp/ado-aw-scripts/ado-script/gate.js", + false, ) .unwrap(); assert!(result.is_empty()); @@ -1575,6 +1779,7 @@ mod tests { GateContext::PullRequest, &checks, "/tmp/ado-aw-scripts/ado-script/gate.js", + false, ) .unwrap(); assert!(result.contains("- bash:"), "should be a bash step"); @@ -1607,6 +1812,7 @@ mod tests { GateContext::PullRequest, &checks, "/tmp/ado-aw-scripts/ado-script/gate.js", + false, ) .unwrap(); assert!( @@ -1634,6 +1840,7 @@ mod tests { GateContext::PipelineCompletion, &checks, "/tmp/ado-aw-scripts/ado-script/gate.js", + false, ) .unwrap(); assert!( @@ -1664,6 +1871,7 @@ mod tests { GateContext::PullRequest, &checks, "/tmp/ado-aw-scripts/ado-script/gate.js", + false, ) .unwrap(); assert!( @@ -1690,17 +1898,18 @@ mod tests { GateContext::PullRequest, &checks, "/tmp/ado-aw-scripts/ado-script/gate.js", + false, ) .unwrap(); // Verify tier-1 (pipeline-var only) checks do not export API-related env vars // Look for the env: block exports (YAML format with leading spaces) let lines: Vec<&str> = result.lines().collect(); - let has_repo_id_export = lines.iter().any(|line| { - line.trim_start().starts_with("ADO_REPO_ID:") - }); - let has_pr_id_export = lines.iter().any(|line| { - line.trim_start().starts_with("ADO_PR_ID:") - }); + let has_repo_id_export = lines + .iter() + .any(|line| line.trim_start().starts_with("ADO_REPO_ID:")); + let has_pr_id_export = lines + .iter() + .any(|line| line.trim_start().starts_with("ADO_PR_ID:")); assert!( !has_repo_id_export, "should not export ADO_REPO_ID for title-only (tier-1) check" @@ -1807,16 +2016,33 @@ mod tests { GateContext::PullRequest, &checks, "/tmp/ado-aw-scripts/ado-script/gate.js", + false, ) .unwrap(); // Verify the spec captures all three filters with correct fact dependencies let spec = build_gate_spec(GateContext::PullRequest, &checks).unwrap(); - assert_eq!(spec.checks.len(), 3, "should produce 3 checks from 3 filters"); - assert!(spec.facts.iter().any(|f| f.kind == "pr_title"), "title filter requires pr_title fact"); - assert!(spec.facts.iter().any(|f| f.kind == "pr_is_draft"), "draft filter requires pr_is_draft fact"); - assert!(spec.facts.iter().any(|f| f.kind == "pr_labels"), "labels filter requires pr_labels fact"); - assert!(spec.facts.iter().any(|f| f.kind == "pr_metadata"), "API-derived facts should pull in pr_metadata dependency"); + assert_eq!( + spec.checks.len(), + 3, + "should produce 3 checks from 3 filters" + ); + assert!( + spec.facts.iter().any(|f| f.kind == "pr_title"), + "title filter requires pr_title fact" + ); + assert!( + spec.facts.iter().any(|f| f.kind == "pr_is_draft"), + "draft filter requires pr_is_draft fact" + ); + assert!( + spec.facts.iter().any(|f| f.kind == "pr_labels"), + "labels filter requires pr_labels fact" + ); + assert!( + spec.facts.iter().any(|f| f.kind == "pr_metadata"), + "API-derived facts should pull in pr_metadata dependency" + ); } // ─── Schema tests ────────────────────────────────────────────────── @@ -1868,8 +2094,7 @@ mod tests { .join("ado-script") .join("schema") .join("gate-spec.schema.json"); - std::fs::create_dir_all(schema_path.parent().unwrap()) - .expect("should create schema dir"); + std::fs::create_dir_all(schema_path.parent().unwrap()).expect("should create schema dir"); std::fs::write(&schema_path, &schema).expect("should write schema file"); // Verify it's readable and valid diff --git a/src/compile/pr_filters.rs b/src/compile/pr_filters.rs index c12bd621..97a517ac 100644 --- a/src/compile/pr_filters.rs +++ b/src/compile/pr_filters.rs @@ -96,12 +96,13 @@ pub(super) fn add_condition_to_steps( // ─── Helpers ──────────────────────────────────────────────────────────────── - // ─── Tests ────────────────────────────────────────────────────────────────── #[cfg(test)] mod tests { - use crate::compile::common::{generate_agentic_depends_on, generate_pr_trigger, generate_setup_job}; + use crate::compile::common::{ + generate_agentic_depends_on, generate_pr_trigger, generate_setup_job, + }; use crate::compile::extensions::CompileContext; use crate::compile::types::*; @@ -118,15 +119,21 @@ mod tests { let triggers = Some(OnConfig { pipeline: None, pr: Some(PrTriggerConfig::default()), - schedule: None, + schedule: None, }); let result = generate_pr_trigger(&triggers, true); // PrTriggerConfig::default() has no branches/paths, so the native block is empty // (meaning "trigger on all PRs" in ADO). The schedule/pipeline suppression ("pr: none") // must NOT be emitted because the explicit pr: key overrides it — regardless of whether // has_schedule or has_pipeline_trigger is set. - assert!(result.is_empty(), "default PrTriggerConfig should produce empty string (trigger on all PRs)"); - assert!(!result.contains("pr: none"), "triggers.pr should override schedule/pipeline suppression"); + assert!( + result.is_empty(), + "default PrTriggerConfig should produce empty string (trigger on all PRs)" + ); + assert!( + !result.contains("pr: none"), + "triggers.pr should override schedule/pipeline suppression" + ); } #[test] @@ -140,14 +147,18 @@ mod tests { }), paths: None, filters: None, + ..Default::default() }), - schedule: None, + schedule: None, }); let result = generate_pr_trigger(&triggers, false); assert!(result.contains("pr:"), "should emit pr: block"); assert!(result.contains("branches:"), "should include branches"); assert!(result.contains("main"), "should include main branch"); - assert!(result.contains("release/*"), "should include release/* branch"); + assert!( + result.contains("release/*"), + "should include release/* branch" + ); assert!(result.contains("exclude:"), "should include exclude"); assert!(result.contains("test/*"), "should include test/* exclusion"); } @@ -163,8 +174,9 @@ mod tests { exclude: vec!["docs/*".into()], }), filters: None, + ..Default::default() }), - schedule: None, + schedule: None, }); let result = generate_pr_trigger(&triggers, false); assert!(result.contains("pr:"), "should emit pr: block"); @@ -181,18 +193,24 @@ mod tests { branches: None, paths: None, filters: Some(PrFilters { - title: Some(PatternFilter { pattern: "*[agent]*".into() }), + title: Some(PatternFilter { + pattern: "*[agent]*".into(), + }), ..Default::default() }), + ..Default::default() }), - schedule: None, + schedule: None, }); let result = generate_pr_trigger(&triggers, false); // When only runtime filters are configured (no branches/paths), no native // pr: block is emitted. ADO interprets this as "trigger on all PRs" — the // runtime gate step handles the actual filtering. Do NOT change this to // emit "pr: none" or the gate will never run. - assert!(result.is_empty(), "filters-only should not emit a pr: block (use default trigger)"); + assert!( + result.is_empty(), + "filters-only should not emit a pr: block (use default trigger)" + ); } // Gate step tests now use the spec/extension directly since generate_setup_job @@ -205,27 +223,39 @@ mod tests { let fm = test_fm(); let ctx = make_ctx(&fm); let filters = PrFilters { - title: Some(PatternFilter { pattern: "*[review]*".into() }), + title: Some(PatternFilter { + pattern: "*[review]*".into(), + }), ..Default::default() }; let result = generate_setup_job(&[], "MyPool", Some(&filters), None, &[], &ctx).unwrap(); // No extension → no gate step → setup job has no steps → empty - assert!(result.is_empty(), "filters without extension should produce empty setup job"); + assert!( + result.is_empty(), + "filters without extension should produce empty setup job" + ); } #[test] fn test_generate_setup_job_with_user_steps_and_filters() { let fm = test_fm(); let ctx = make_ctx(&fm); - let step: serde_yaml::Value = serde_yaml::from_str("bash: echo hello\ndisplayName: User step").unwrap(); + let step: serde_yaml::Value = + serde_yaml::from_str("bash: echo hello\ndisplayName: User step").unwrap(); let filters = PrFilters { - title: Some(PatternFilter { pattern: "test".into() }), + title: Some(PatternFilter { + pattern: "test".into(), + }), ..Default::default() }; - let result = generate_setup_job(&[step], "MyPool", Some(&filters), None, &[], &ctx).unwrap(); + let result = + generate_setup_job(&[step], "MyPool", Some(&filters), None, &[], &ctx).unwrap(); // User steps are conditioned on gate output even without extension assert!(result.contains("User step"), "should include user step"); - assert!(result.contains("prGate.SHOULD_RUN"), "user steps should reference gate output"); + assert!( + result.contains("prGate.SHOULD_RUN"), + "user steps should reference gate output" + ); } #[test] @@ -233,43 +263,59 @@ mod tests { let fm = test_fm(); let ctx = make_ctx(&fm); let result = generate_setup_job(&[], "MyPool", None, None, &[], &ctx).unwrap(); - assert!(result.is_empty(), "no setup steps and no filters should produce empty string"); + assert!( + result.is_empty(), + "no setup steps and no filters should produce empty string" + ); } #[test] fn test_generate_agentic_depends_on_with_pr_filters() { - let result = generate_agentic_depends_on(&[], true, false, &[], false); - assert!(result.contains("dependsOn: Setup"), "should depend on Setup"); + let result = generate_agentic_depends_on(&[], true, false, &[], false, false); + assert!( + result.contains("dependsOn: Setup"), + "should depend on Setup" + ); assert!(result.contains("condition:"), "should have condition"); assert!(result.contains("Build.Reason"), "should check Build.Reason"); - assert!(result.contains("prGate.SHOULD_RUN"), "should check gate output"); + assert!( + result.contains("prGate.SHOULD_RUN"), + "should check gate output" + ); } #[test] fn test_generate_agentic_depends_on_setup_only_no_condition() { let step: serde_yaml::Value = serde_yaml::from_str("bash: echo hello").unwrap(); - let result = generate_agentic_depends_on(&[step], false, false, &[], false); + let result = generate_agentic_depends_on(&[step], false, false, &[], false, false); assert_eq!(result, "dependsOn: Setup"); - assert!(!result.contains("condition:"), "no condition without PR filters"); + assert!( + !result.contains("condition:"), + "no condition without PR filters" + ); } #[test] fn test_generate_agentic_depends_on_nothing() { - let result = generate_agentic_depends_on(&[], false, false, &[], false); + let result = generate_agentic_depends_on(&[], false, false, &[], false, false); assert!(result.is_empty()); } #[test] fn test_generate_setup_job_gate_spec_via_extension() { // Filter content is now tested via build_gate_spec, not generate_setup_job - use crate::compile::filter_ir::{build_gate_spec, lower_pr_filters, GateContext}; + use crate::compile::filter_ir::{GateContext, build_gate_spec, lower_pr_filters}; let filters = PrFilters { author: Some(IncludeExcludeFilter { include: vec!["alice@corp.com".into()], exclude: vec!["bot@noreply.com".into()], }), - source_branch: Some(PatternFilter { pattern: "feature/*".into() }), - target_branch: Some(PatternFilter { pattern: "main".into() }), + source_branch: Some(PatternFilter { + pattern: "feature/*".into(), + }), + target_branch: Some(PatternFilter { + pattern: "main".into(), + }), ..Default::default() }; let checks = lower_pr_filters(&filters); @@ -283,9 +329,11 @@ mod tests { #[test] fn test_generate_setup_job_gate_non_pr_bypass_in_spec() { - use crate::compile::filter_ir::{build_gate_spec, lower_pr_filters, GateContext}; + use crate::compile::filter_ir::{GateContext, build_gate_spec, lower_pr_filters}; let filters = PrFilters { - title: Some(PatternFilter { pattern: "test".into() }), + title: Some(PatternFilter { + pattern: "test".into(), + }), ..Default::default() }; let checks = lower_pr_filters(&filters); @@ -297,21 +345,22 @@ mod tests { #[test] fn test_generate_setup_job_gate_build_tags() { let filters = PrFilters { - title: Some(PatternFilter { pattern: "test".into() }), + title: Some(PatternFilter { + pattern: "test".into(), + }), ..Default::default() }; // Build tags are now in the evaluator, driven by spec. Verify spec content. - use crate::compile::filter_ir::{build_gate_spec, lower_pr_filters, GateContext}; + use crate::compile::filter_ir::{GateContext, build_gate_spec, lower_pr_filters}; let checks = lower_pr_filters(&filters); let spec = build_gate_spec(GateContext::PullRequest, &checks).unwrap(); assert_eq!(spec.context.tag_prefix, "pr-gate"); assert_eq!(spec.checks[0].tag_suffix, "title-mismatch"); } - #[test] fn test_gate_step_includes_api_facts_for_tier2() { - use crate::compile::filter_ir::{build_gate_spec, lower_pr_filters, GateContext}; + use crate::compile::filter_ir::{GateContext, build_gate_spec, lower_pr_filters}; let filters = PrFilters { labels: Some(LabelFilter { any_of: vec!["run-agent".into()], @@ -321,26 +370,42 @@ mod tests { }; let checks = lower_pr_filters(&filters); let spec = build_gate_spec(GateContext::PullRequest, &checks).unwrap(); - assert!(spec.facts.iter().any(|f| f.kind == "pr_metadata"), "should require pr_metadata fact"); - assert!(spec.facts.iter().any(|f| f.kind == "pr_labels"), "should require pr_labels fact"); + assert!( + spec.facts.iter().any(|f| f.kind == "pr_metadata"), + "should require pr_metadata fact" + ); + assert!( + spec.facts.iter().any(|f| f.kind == "pr_labels"), + "should require pr_labels fact" + ); } #[test] fn test_gate_step_no_api_facts_for_tier1_only() { - use crate::compile::filter_ir::{build_gate_spec, lower_pr_filters, GateContext}; + use crate::compile::filter_ir::{GateContext, build_gate_spec, lower_pr_filters}; let filters = PrFilters { - title: Some(PatternFilter { pattern: "test".into() }), + title: Some(PatternFilter { + pattern: "test".into(), + }), ..Default::default() }; let checks = lower_pr_filters(&filters); let spec = build_gate_spec(GateContext::PullRequest, &checks).unwrap(); - assert!(spec.facts.iter().any(|f| f.kind == "pr_title"), "should require pr_title fact for title filter"); - assert!(!spec.facts.iter().any(|f| f.kind == "pr_metadata"), "should not require pr_metadata for title-only"); + assert!( + spec.facts.iter().any(|f| f.kind == "pr_title"), + "should require pr_title fact for title filter" + ); + assert!( + !spec.facts.iter().any(|f| f.kind == "pr_metadata"), + "should not require pr_metadata for title-only" + ); } #[test] fn test_gate_step_labels_any_of() { - use crate::compile::filter_ir::{build_gate_spec, lower_pr_filters, GateContext, PredicateSpec}; + use crate::compile::filter_ir::{ + GateContext, PredicateSpec, build_gate_spec, lower_pr_filters, + }; let filters = PrFilters { labels: Some(LabelFilter { any_of: vec!["run-agent".into(), "needs-review".into()], @@ -363,7 +428,9 @@ mod tests { #[test] fn test_gate_step_labels_none_of() { - use crate::compile::filter_ir::{build_gate_spec, lower_pr_filters, GateContext, PredicateSpec}; + use crate::compile::filter_ir::{ + GateContext, PredicateSpec, build_gate_spec, lower_pr_filters, + }; let filters = PrFilters { labels: Some(LabelFilter { none_of: vec!["do-not-run".into()], @@ -383,7 +450,9 @@ mod tests { #[test] fn test_gate_step_draft_false() { - use crate::compile::filter_ir::{build_gate_spec, lower_pr_filters, GateContext, PredicateSpec}; + use crate::compile::filter_ir::{ + GateContext, PredicateSpec, build_gate_spec, lower_pr_filters, + }; let filters = PrFilters { draft: Some(false), ..Default::default() @@ -397,13 +466,21 @@ mod tests { } other => panic!("expected Equals, got {:?}", other), } - assert!(spec.facts.iter().any(|f| f.kind == "pr_is_draft"), "should include pr_is_draft fact"); - assert!(spec.facts.iter().any(|f| f.kind == "pr_metadata"), "should include pr_metadata dependency"); + assert!( + spec.facts.iter().any(|f| f.kind == "pr_is_draft"), + "should include pr_is_draft fact" + ); + assert!( + spec.facts.iter().any(|f| f.kind == "pr_metadata"), + "should include pr_metadata dependency" + ); } #[test] fn test_gate_step_changed_files() { - use crate::compile::filter_ir::{build_gate_spec, lower_pr_filters, GateContext, PredicateSpec}; + use crate::compile::filter_ir::{ + GateContext, PredicateSpec, build_gate_spec, lower_pr_filters, + }; let filters = PrFilters { changed_files: Some(IncludeExcludeFilter { include: vec!["src/**/*.rs".into()], @@ -414,20 +491,27 @@ mod tests { let checks = lower_pr_filters(&filters); let spec = build_gate_spec(GateContext::PullRequest, &checks).unwrap(); match &spec.checks[0].predicate { - PredicateSpec::FileGlobMatch { include, exclude, .. } => { + PredicateSpec::FileGlobMatch { + include, exclude, .. + } => { assert!(include.contains(&"src/**/*.rs".to_string())); assert!(exclude.contains(&"docs/**".to_string())); } other => panic!("expected FileGlobMatch, got {:?}", other), } - assert!(spec.facts.iter().any(|f| f.kind == "changed_files"), "should include changed_files fact"); + assert!( + spec.facts.iter().any(|f| f.kind == "changed_files"), + "should include changed_files fact" + ); } #[test] fn test_gate_step_combined_tier1_and_tier2() { - use crate::compile::filter_ir::{build_gate_spec, lower_pr_filters, GateContext}; + use crate::compile::filter_ir::{GateContext, build_gate_spec, lower_pr_filters}; let filters = PrFilters { - title: Some(PatternFilter { pattern: "\\[review\\]".into() }), + title: Some(PatternFilter { + pattern: "\\[review\\]".into(), + }), draft: Some(false), labels: Some(LabelFilter { any_of: vec!["run-agent".into()], @@ -438,20 +522,38 @@ mod tests { let checks = lower_pr_filters(&filters); let spec = build_gate_spec(GateContext::PullRequest, &checks).unwrap(); // Tier 1 fact - assert!(spec.facts.iter().any(|f| f.kind == "pr_title"), "should include pr_title"); + assert!( + spec.facts.iter().any(|f| f.kind == "pr_title"), + "should include pr_title" + ); // Tier 2 facts - assert!(spec.facts.iter().any(|f| f.kind == "pr_metadata"), "should include pr_metadata"); - assert!(spec.facts.iter().any(|f| f.kind == "pr_is_draft"), "should include pr_is_draft"); - assert!(spec.facts.iter().any(|f| f.kind == "pr_labels"), "should include pr_labels"); + assert!( + spec.facts.iter().any(|f| f.kind == "pr_metadata"), + "should include pr_metadata" + ); + assert!( + spec.facts.iter().any(|f| f.kind == "pr_is_draft"), + "should include pr_is_draft" + ); + assert!( + spec.facts.iter().any(|f| f.kind == "pr_labels"), + "should include pr_labels" + ); // Checks - assert_eq!(spec.checks.len(), 3, "should have 3 checks (title, draft, labels)"); + assert_eq!( + spec.checks.len(), + 3, + "should have 3 checks (title, draft, labels)" + ); } // ─── Tier 3 filter tests ──────────────────────────────────────────────── #[test] fn test_gate_step_time_window() { - use crate::compile::filter_ir::{build_gate_spec, lower_pr_filters, GateContext, PredicateSpec}; + use crate::compile::filter_ir::{ + GateContext, PredicateSpec, build_gate_spec, lower_pr_filters, + }; let filters = PrFilters { time_window: Some(super::super::types::TimeWindowFilter { start: "09:00".into(), @@ -473,7 +575,9 @@ mod tests { #[test] fn test_gate_step_min_and_max_changes() { - use crate::compile::filter_ir::{build_gate_spec, lower_pr_filters, GateContext, PredicateSpec}; + use crate::compile::filter_ir::{ + GateContext, PredicateSpec, build_gate_spec, lower_pr_filters, + }; let filters = PrFilters { min_changes: Some(2), max_changes: Some(100), @@ -492,7 +596,9 @@ mod tests { #[test] fn test_gate_step_build_reason_include() { - use crate::compile::filter_ir::{build_gate_spec, lower_pr_filters, GateContext, PredicateSpec}; + use crate::compile::filter_ir::{ + GateContext, PredicateSpec, build_gate_spec, lower_pr_filters, + }; let filters = PrFilters { build_reason: Some(IncludeExcludeFilter { include: vec!["PullRequest".into(), "Manual".into()], @@ -514,7 +620,9 @@ mod tests { #[test] fn test_gate_step_build_reason_exclude() { - use crate::compile::filter_ir::{build_gate_spec, lower_pr_filters, GateContext, PredicateSpec}; + use crate::compile::filter_ir::{ + GateContext, PredicateSpec, build_gate_spec, lower_pr_filters, + }; let filters = PrFilters { build_reason: Some(IncludeExcludeFilter { include: vec![], @@ -541,12 +649,22 @@ mod tests { false, &["eq(variables['Custom.ShouldRun'], 'true')"], false, + false, ); // No setup steps, no PR filters → no dependsOn, but the expression produces a condition. - assert!(!result.contains("dependsOn"), "no dependsOn without setup/filters"); + assert!( + !result.contains("dependsOn"), + "no dependsOn without setup/filters" + ); assert!(result.contains("condition:"), "should have condition"); - assert!(result.contains("Custom.ShouldRun"), "should include expression"); - assert!(result.contains("succeeded()"), "should still require succeeded"); + assert!( + result.contains("Custom.ShouldRun"), + "should include expression" + ); + assert!( + result.contains("succeeded()"), + "should still require succeeded" + ); } #[test] @@ -557,15 +675,19 @@ mod tests { false, &["eq(variables['Custom.Flag'], 'yes')"], false, + false, + ); + assert!( + result.contains("prGate.SHOULD_RUN"), + "should check gate output" ); - assert!(result.contains("prGate.SHOULD_RUN"), "should check gate output"); assert!(result.contains("Custom.Flag"), "should include expression"); assert!(result.contains("Build.Reason"), "should check build reason"); } #[test] fn test_gate_step_change_count_includes_changed_files_fact() { - use crate::compile::filter_ir::{build_gate_spec, lower_pr_filters, GateContext}; + use crate::compile::filter_ir::{GateContext, build_gate_spec, lower_pr_filters}; let filters = PrFilters { changed_files: Some(IncludeExcludeFilter { include: vec!["src/**".into()], @@ -603,20 +725,33 @@ triggers: assert_eq!(filters.time_window.as_ref().unwrap().end, "17:00"); assert_eq!(filters.min_changes, Some(5)); assert_eq!(filters.max_changes, Some(100)); - assert_eq!(filters.build_reason.as_ref().unwrap().include, vec!["PullRequest", "Manual"]); - assert_eq!(filters.expression.as_ref().unwrap(), "eq(variables['Custom.Flag'], 'true')"); + assert_eq!( + filters.build_reason.as_ref().unwrap().include, + vec!["PullRequest", "Manual"] + ); + assert_eq!( + filters.expression.as_ref().unwrap(), + "eq(variables['Custom.Flag'], 'true')" + ); } #[test] fn test_gate_step_commit_message() { - use crate::compile::filter_ir::{build_gate_spec, lower_pr_filters, GateContext, PredicateSpec}; + use crate::compile::filter_ir::{ + GateContext, PredicateSpec, build_gate_spec, lower_pr_filters, + }; let filters = PrFilters { - commit_message: Some(PatternFilter { pattern: "*[skip-agent]*".into() }), + commit_message: Some(PatternFilter { + pattern: "*[skip-agent]*".into(), + }), ..Default::default() }; let checks = lower_pr_filters(&filters); let spec = build_gate_spec(GateContext::PullRequest, &checks).unwrap(); - assert!(spec.facts.iter().any(|f| f.kind == "commit_message"), "should include commit_message fact"); + assert!( + spec.facts.iter().any(|f| f.kind == "commit_message"), + "should include commit_message fact" + ); match &spec.checks[0].predicate { PredicateSpec::GlobMatch { fact, pattern } => { assert_eq!(fact, "commit_message"); @@ -639,10 +774,18 @@ on: let val: serde_yaml::Value = serde_yaml::from_str(yaml).unwrap(); let oc: OnConfig = serde_yaml::from_value(val["on"].clone()).unwrap(); assert!(oc.schedule.is_some(), "should have schedule"); - assert_eq!(oc.schedule.unwrap().expression(), "daily around 14:00", "schedule expression should round-trip"); + assert_eq!( + oc.schedule.unwrap().expression(), + "daily around 14:00", + "schedule expression should round-trip" + ); let pr = oc.pr.expect("should have pr"); let filters = pr.filters.expect("pr should have filters"); - assert_eq!(filters.title.unwrap().pattern, "*[review]*", "title pattern should round-trip"); + assert_eq!( + filters.title.unwrap().pattern, + "*[review]*", + "title pattern should round-trip" + ); assert!(oc.pipeline.is_none(), "should not have pipeline"); } @@ -676,4 +819,3 @@ on: assert_eq!(filters.commit_message.unwrap().pattern, "*[skip-agent]*"); } } - diff --git a/src/compile/types.rs b/src/compile/types.rs index 100fb80f..6b79ae9d 100644 --- a/src/compile/types.rs +++ b/src/compile/types.rs @@ -749,6 +749,17 @@ impl FrontMatter { self.on_config.as_ref().and_then(|o| o.pr.as_ref()) } + /// Whether the synthetic-from-ci path is active for this agent — + /// i.e. `on.pr` is configured AND `on.pr.mode == PrMode::Synthetic` + /// (the default). Centralised here so the three compile-time call + /// sites (`collect_extensions`, `ExecContextExtension::new`, + /// `compile_shared`) cannot drift on the predicate if a future + /// `PrMode` variant is added. + pub fn is_synthetic_pr(&self) -> bool { + self.pr_trigger() + .is_some_and(|p| matches!(p.mode, PrMode::Synthetic)) + } + /// Get the PR runtime filters (if any). pub fn pr_filters(&self) -> Option<&PrFilters> { self.pr_trigger().and_then(|pr| pr.filters.as_ref()) @@ -1256,10 +1267,53 @@ pub struct PrTriggerConfig { /// Runtime filters evaluated via gate steps in the Setup job #[serde(default)] pub filters: Option, + /// Determines how `on.pr` builds reach the pipeline; see + /// [`PrMode`] for the two supported strategies (`synthetic`, + /// `policy`). Defaults to [`PrMode::Synthetic`]. + #[serde(default, rename = "mode")] + pub mode: PrMode, +} + +/// How `on.pr` builds reach the pipeline. +/// +/// Azure DevOps Services ignores the YAML `pr:` block unless a +/// per-branch Build Validation policy is registered server-side. This +/// enum lets the agent author pick one of two coherent strategies: +/// +/// * [`PrMode::Synthetic`] (default) — the compiler emits a Setup-job +/// script (`exec-context-pr-synth.js`) that calls the ADO REST API +/// on CI-triggered builds, finds the open PR for `Build.SourceBranch`, +/// and exposes PR identifiers as `dependencies.Setup.outputs['synthPr.*']` +/// so the gate and `exec-context-pr.js` behave as if +/// `Build.Reason == PullRequest`. The top-level `trigger:` stays at +/// ADO's "trigger on every branch" default. **No branch policy +/// required.** This is the right choice for the vast majority of +/// agents. +/// +/// * [`PrMode::Policy`] — the compiler omits all synth wiring AND +/// emits `trigger: none` at the top level, so the pipeline only +/// queues when ADO's Build Validation branch policy fires a real +/// `Build.Reason == PullRequest` build. Choose this when the +/// operator has explicitly installed a branch policy and wants to +/// avoid duplicate CI builds firing on every push. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default, Deserialize, Serialize)] +#[serde(rename_all = "kebab-case")] +pub enum PrMode { + /// Synthesise PR context from the ADO REST API on CI-triggered + /// builds. Top-level `trigger:` left at ADO default. + #[default] + Synthetic, + /// Operator-installed Build Validation branch policy drives PR + /// builds; CI trigger is suppressed via `trigger: none`. + Policy, } impl SanitizeConfigTrait for PrTriggerConfig { fn sanitize_config_fields(&mut self) { + // `mode` (PrMode enum, `Copy`) has no string content to + // sanitize — it's a closed kebab-case-deserialised enum, so + // any malformed input is already rejected at deserialisation + // time. Intentionally absent here. if let Some(ref mut b) = self.branches { b.sanitize_config_fields(); } @@ -1600,8 +1654,6 @@ timeout-minutes: 60 assert_eq!(env.get("AWS_REGION").unwrap(), "us-west-2"); } - - // ─── PermissionsConfig deserialization ─────────────────────────────── #[test] @@ -2219,6 +2271,74 @@ triggers: assert_eq!(changed.exclude, vec!["docs/**"]); } + #[test] + fn test_pr_trigger_config_mode_default_synthetic() { + let yaml = r#" +triggers: + pr: + branches: + include: [main] +"#; + let val: serde_yaml::Value = serde_yaml::from_str(yaml).unwrap(); + let tc: OnConfig = serde_yaml::from_value(val["triggers"].clone()).unwrap(); + assert_eq!( + tc.pr.unwrap().mode, + PrMode::Synthetic, + "mode must default to synthetic when omitted" + ); + } + + #[test] + fn test_pr_trigger_config_mode_explicit_synthetic() { + let yaml = r#" +triggers: + pr: + branches: + include: [main] + mode: synthetic +"#; + let val: serde_yaml::Value = serde_yaml::from_str(yaml).unwrap(); + let tc: OnConfig = serde_yaml::from_value(val["triggers"].clone()).unwrap(); + assert_eq!(tc.pr.unwrap().mode, PrMode::Synthetic); + } + + #[test] + fn test_pr_trigger_config_mode_explicit_policy() { + let yaml = r#" +triggers: + pr: + branches: + include: [main] + mode: policy +"#; + let val: serde_yaml::Value = serde_yaml::from_str(yaml).unwrap(); + let tc: OnConfig = serde_yaml::from_value(val["triggers"].clone()).unwrap(); + assert_eq!( + tc.pr.unwrap().mode, + PrMode::Policy, + "mode: policy must deserialise correctly" + ); + } + + #[test] + fn test_pr_trigger_config_mode_invalid_value_errors() { + let yaml = r#" +triggers: + pr: + branches: + include: [main] + mode: bananas +"#; + let val: serde_yaml::Value = serde_yaml::from_str(yaml).unwrap(); + let err = serde_yaml::from_value::(val["triggers"].clone()) + .expect_err("mode: bananas must be rejected at deserialisation"); + let msg = err.to_string(); + assert!( + msg.contains("bananas") || msg.contains("variant") || msg.contains("unknown"), + "error must mention the bad variant; got: {msg}" + ); + } + #[test] fn test_pr_trigger_config_paths_only() { let yaml = r#" @@ -2246,10 +2366,7 @@ triggers: "#; let val: serde_yaml::Value = serde_yaml::from_str(yaml).unwrap(); let tc: OnConfig = serde_yaml::from_value(val["triggers"].clone()).unwrap(); - assert_eq!( - tc.pipeline.as_ref().unwrap().name, - "Build Pipeline" - ); + assert_eq!(tc.pipeline.as_ref().unwrap().name, "Build Pipeline"); assert_eq!( tc.pr.unwrap().filters.unwrap().title.unwrap().pattern, "*[review]*" diff --git a/tests/compiler_tests.rs b/tests/compiler_tests.rs index a675527b..06d51930 100644 --- a/tests/compiler_tests.rs +++ b/tests/compiler_tests.rs @@ -1,7 +1,6 @@ use std::fs; use std::path::PathBuf; - /// Asserts that all required `{{ marker }}` placeholders are present in the template. fn assert_required_markers(content: &str) { let required = [ @@ -158,7 +157,6 @@ fn test_example_file_structure() { ); } - /// Test that validates the presence of required dependencies #[test] fn test_project_dependencies() { @@ -633,8 +631,7 @@ Do something. String::from_utf8_lossy(&output.stderr) ); - let compiled = - fs::read_to_string(&output_path).expect("Compiled YAML should exist on success"); + let compiled = fs::read_to_string(&output_path).expect("Compiled YAML should exist on success"); assert!( compiled.contains("SYSTEM_ACCESSTOKEN: $(System.AccessToken)"), "Executor must map SYSTEM_ACCESSTOKEN from $(System.AccessToken) by default. \ @@ -4232,14 +4229,18 @@ fn test_neither_feature_active_emits_no_node_or_download_anywhere() { } /// Per-job download placement: when the gate is inactive AND runtime imports -/// are inlined, but `on.pr` is configured and execution-context PR is not -/// disabled, the `exec-context-pr.js` bundle is the only consumer — the -/// download must land in the Agent job only. +/// are inlined, but `on.pr` is configured (default `mode: synthetic`) and +/// execution-context PR is not disabled, two bundle consumers are active: +/// +/// * `synthPr` (Setup job) — emitted by the synthetic-from-ci path +/// * `exec-context-pr.js` (Agent job) — staged by the PR contributor +/// +/// so the script bundle download MUST land in BOTH jobs. /// /// Closes a coverage gap that `dedupe_gate_only.md` previously left by /// pinning `execution-context.pr.enabled: false`. #[test] -fn test_exec_context_pr_only_downloads_bundle_in_agent_job_not_setup() { +fn test_exec_context_pr_downloads_bundle_in_both_jobs_with_synth_mode() { let yaml = compile_fixture("dedupe_exec_context_pr_only.md"); let agent = extract_job_block(&yaml, "Agent").expect("Agent job should exist"); assert!( @@ -4251,10 +4252,14 @@ fn test_exec_context_pr_only_downloads_bundle_in_agent_job_not_setup() { "Agent job is missing the exec-context-pr prepare step (the consumer of the download)" ); if let Some(setup) = extract_job_block(&yaml, "Setup") { + // Setup-job script bundle download IS expected when on.pr is + // configured (default `mode: synthetic` emits the synthPr + // step, which is a bundle consumer). Only assert the Agent + // job has the bundle download; the Setup-job download is the + // synth feature's correct behaviour. assert!( - !setup.contains("Download ado-aw scripts"), - "Setup job should NOT have the script bundle download when the only consumer is the Agent-job exec-context-pr step. \ - Setup block contents: {}", + setup.contains("Download ado-aw scripts"), + "Setup job SHOULD have the script bundle download when mode: synthetic is on (the synthPr step is a bundle consumer). Setup block: {}", setup ); } @@ -4370,6 +4375,153 @@ fn test_pr_filter_agent_depends_on_setup() { ); } +/// Regression guard for the synth-mode gate-bypass bug: with `mode: +/// synthetic` (the default) AND `on.pr.filters` present, the Agent-job +/// condition must REQUIRE the gate to pass for real-PR and synth-PR +/// builds. Earlier iterations emitted `or(eq(Build.Reason, 'PullRequest'), +/// eq(synthPr.AW_SYNTHETIC_PR, 'true'), ...)` which silently bypassed the +/// gate for any PR build — defeating the purpose of `pr.filters`. +#[test] +fn test_pr_filter_synth_mode_agent_condition_enforces_gate() { + let compiled = compile_fixture("pr-filter-tier1-agent.md"); + + // Extract the Agent-job dependsOn condition body so the assertions + // target only that section (the same strings can appear elsewhere — + // e.g. the exec-context-pr.js step's condition — and would create + // false positives if we matched the whole compiled output). + let agent_block = extract_job_block(&compiled, "Agent").expect("Agent job present"); + let condition_section = agent_block + .split("condition: |") + .nth(1) + .map(|tail| { + // Stop at the next top-level Agent-job field. `steps:` always + // exists; `pool:` / `variables:` / `workspace:` may exist + // before it. The first one we hit terminates the condition + // body. Using exact field names avoids matching inner + // condition lines that start with 4+ spaces. + let stop_at = [ + "\n pool:", + "\n steps:", + "\n variables:", + "\n workspace:", + ]; + let end = stop_at + .iter() + .filter_map(|needle| tail.find(needle)) + .min() + .unwrap_or(tail.len()); + &tail[..end] + }) + .unwrap_or(""); + + // Correct shape: the AND-NOT clause requiring (not real PR) AND + // (not synth PR) before the unconditional-run branch is taken. + // Whitespace-agnostic substring matches. + assert!( + condition_section.contains("ne(variables['Build.Reason'], 'PullRequest')") + && condition_section + .contains("ne(dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR'], 'true')"), + "Agent-job dependsOn condition must contain the AND-NOT arms \ + `ne(Build.Reason, 'PullRequest')` and `ne(synthPr.AW_SYNTHETIC_PR, 'true')` \ + so the gate is enforced for PR builds (real or synth). \ + Condition section: {condition_section}" + ); + assert!( + condition_section.contains("eq(dependencies.Setup.outputs['prGate.SHOULD_RUN'], 'true')"), + "Agent-job dependsOn condition must keep the gate-passed activation arm: \ + {condition_section}" + ); + + // Defensive: the old permissive bypass arms that bypassed the gate + // for any PR build MUST NOT appear inside the Agent-job dependsOn + // condition. + assert!( + !condition_section.contains("eq(variables['Build.Reason'], 'PullRequest')"), + "Agent-job dependsOn condition must NOT contain the buggy \ + `eq(Build.Reason, 'PullRequest')` bypass arm (would auto-run on \ + every real PR build regardless of gate): {condition_section}" + ); + assert!( + !condition_section + .contains("eq(dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR'], 'true')"), + "Agent-job dependsOn condition must NOT contain the buggy \ + `eq(synthPr.AW_SYNTHETIC_PR, 'true')` bypass arm (would auto-run on \ + every synth-promoted build regardless of gate): {condition_section}" + ); +} + +/// Regression guard for the synth-mode gate-step same-job ref bug: the +/// gate step lives in the **Setup** job (same job as `synthPr`), so its +/// env block must use `variables['synthPr.X']` (same-job runtime +/// expression) — `dependencies.Setup.outputs[...]` is undefined inside +/// the producing job and silently coalesces to empty, leaving +/// `AW_SYNTHETIC_PR` empty and causing the bypass to misfire. +#[test] +fn test_pr_filter_synth_mode_gate_step_uses_same_job_synth_ref() { + let compiled = compile_fixture("pr-filter-tier1-agent.md"); + + assert!( + compiled.contains( + "AW_SYNTHETIC_PR: \"$[ coalesce(variables['synthPr.AW_SYNTHETIC_PR'], '') ]\"" + ), + "Gate step env must use same-job `variables['synthPr.X']` runtime expression — \ + `dependencies.Setup.outputs[...]` is undefined inside the producing Setup job" + ); + // The fixture exercises source-branch and target-branch filters, + // so the synth-coalesce treatment must appear on those env vars + // using the same-job `variables[...]` form. (ADO_PR_ID is not + // exported by this fixture's filter set, so we don't assert it here.) + assert!( + compiled.contains( + "ADO_SOURCE_BRANCH: \"$[ coalesce(variables['System.PullRequest.SourceBranch'], variables['synthPr.AW_SYNTHETIC_PR_SOURCEBRANCH']) ]\"" + ), + "ADO_SOURCE_BRANCH coalesce must use same-job `variables[...]` form" + ); + assert!( + compiled.contains( + "ADO_TARGET_BRANCH: \"$[ coalesce(variables['System.PullRequest.TargetBranch'], variables['synthPr.AW_SYNTHETIC_PR_TARGETBRANCH']) ]\"" + ), + "ADO_TARGET_BRANCH coalesce must use same-job `variables[...]` form" + ); + + // The same-job gate step MUST NOT use the cross-job + // `dependencies.Setup.outputs[...]` form for synthPr references. + // (It's fine elsewhere — e.g. the Agent-job dependsOn condition — but + // not inside the Setup job's own steps.) + // The same-job gate step MUST NOT use the cross-job + // `dependencies.Setup.outputs[...]` form for synthPr references. + // (It's fine elsewhere — e.g. the Agent-job dependsOn condition — but + // not inside the Setup job's own steps.) Bound the gate-step + // section by the start of the next top-level job (`\n - job: `), + // since extract_job_block's `\n- job: ` boundary doesn't match the + // 2-space-indented job list items produced for this target. + let setup_block = extract_job_block(&compiled, "Setup").expect("Setup job present"); + let gate_section = setup_block + .split("name: prGate") + .nth(1) + .map(|tail| { + let stop_at = [ + "\n - bash:", + "\n - task:", + "\n - script:", + "\n - job: ", + ]; + let end = stop_at + .iter() + .filter_map(|needle| tail.find(needle)) + .min() + .unwrap_or(tail.len()); + &tail[..end] + }) + .unwrap_or(""); + assert!( + !gate_section.contains("dependencies.Setup.outputs['synthPr."), + "Gate step (inside Setup job) must NOT reference `dependencies.Setup.outputs['synthPr.X']` — \ + that is cross-job syntax and is undefined within the producing job. \ + Gate section: {gate_section}" + ); +} + /// Native ADO PR trigger block is emitted for branch/path filters. #[test] fn test_pr_filter_tier1_has_native_pr_trigger() { @@ -4657,7 +4809,12 @@ fn test_default_pipeline_mounts_az_and_allows_azure_hosts() { chars after the displayName). Window:\n{window}" ); // Anchor strings: lock the load-bearing parts of the advisory. - for anchor in ["/usr/bin/az", "az devops", "AZURE_DEVOPS_EXT_PAT", "missing-tool"] { + for anchor in [ + "/usr/bin/az", + "az devops", + "AZURE_DEVOPS_EXT_PAT", + "missing-tool", + ] { assert!( compiled.contains(anchor), "compiled YAML must contain advisory anchor `{anchor}`. \ @@ -5150,8 +5307,6 @@ fn test_job_target_with_setup_emits_dual_branch_dependson_with_each() { let _ = fs::remove_dir_all(&temp_dir); } - - // ============================================================================ // Execution-context extension (issue #860) // ============================================================================ @@ -5182,8 +5337,8 @@ fn test_execution_context_pr_emits_prepare_step_and_prompt_supplement() { "Should emit the PR context prepare step displayName" ); assert!( - compiled.contains("condition: eq(variables['Build.Reason'], 'PullRequest')"), - "Prepare step must be gated on PR builds at the ADO condition layer" + compiled.contains("condition: or(eq(variables['Build.Reason'], 'PullRequest'), eq(dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR'], 'true'))"), + "Prepare step must be gated on PR builds OR synthetic-PR Setup outputs at the ADO condition layer (mode: synthetic is the default)" ); assert!( compiled.contains("SYSTEM_ACCESSTOKEN: $(System.AccessToken)"), @@ -5219,15 +5374,18 @@ fn test_execution_context_pr_emits_prepare_step_and_prompt_supplement() { "v7: the git_fetch wrapper moved into the bundle" ); - // v7: env passthrough — Node reads the ADO predefined vars from - // `process.env` (see `index.ts::main` and `validate.ts`). + // v7 + mode: synthetic (the default): env passthrough — the bundle + // reads ADO predefined vars from `process.env`. The compiler emits + // coalesced macros that prefer the real `System.PullRequest.*` vars + // (true PR builds) and fall back to the `synthPr` Setup-job outputs + // (CI builds promoted via exec-context-pr-synth.js). assert!( - compiled.contains("SYSTEM_PULLREQUEST_PULLREQUESTID: $(System.PullRequest.PullRequestId)"), - "Prepare step must pass the PR id through to the bundle" + compiled.contains("SYSTEM_PULLREQUEST_PULLREQUESTID: \"$[ coalesce(variables['System.PullRequest.PullRequestId'], dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR_ID']) ]\""), + "Prepare step must pass the PR id (coalesced with synthPr fallback) through to the bundle" ); assert!( - compiled.contains("SYSTEM_PULLREQUEST_TARGETBRANCH: $(System.PullRequest.TargetBranch)"), - "Prepare step must pass the PR target branch through to the bundle" + compiled.contains("SYSTEM_PULLREQUEST_TARGETBRANCH: \"$[ coalesce(variables['System.PullRequest.TargetBranch'], dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR_TARGETBRANCH']) ]\""), + "Prepare step must pass the PR target branch (coalesced with synthPr fallback) through to the bundle" ); assert!( compiled.contains("SYSTEM_TEAMPROJECT: $(System.TeamProject)"), @@ -5302,9 +5460,9 @@ fn test_execution_context_pr_does_not_leak_system_accesstoken() { // that contains SYSTEM_ACCESSTOKEN, capture the // sibling `displayName` (if any). if let Some(Value::Mapping(env_map)) = m.get(Value::String("env".to_string())) { - let has_token = env_map.iter().any(|(k, _v)| { - matches!(k, Value::String(s) if s == "SYSTEM_ACCESSTOKEN") - }); + let has_token = env_map + .iter() + .any(|(k, _v)| matches!(k, Value::String(s) if s == "SYSTEM_ACCESSTOKEN")); if has_token { let display = m .get(Value::String("displayName".to_string())) @@ -5344,6 +5502,11 @@ fn test_execution_context_pr_does_not_leak_system_accesstoken() { // Stage 3 SafeOutputs executor — separate non-agent job; needs // the token to apply safe outputs against ADO. See PR #873. "Execute safe outputs (Stage 3)", + // Setup-job synth-PR step. Needs the token to call the ADO REST + // API to look up the active PR for `Build.SourceBranch` on + // CI-triggered builds (issue #916). Runs in the Setup job, well + // before the AWF sandbox is provisioned for the Agent job. + "Resolve synthetic PR context", ]; let mut saw_exec_context_step = false; @@ -5631,4 +5794,102 @@ Body. // v7: this validation now lives in the `exec-context-pr.js` bundle // (see `validate.ts::TARGET_BRANCH_RE`); the vitest tests under // `validate.test.ts` guard the regression there. This Rust-side -// test is removed for the same reason. \ No newline at end of file +// test is removed for the same reason. + +// ─── Synthetic-from-ci snapshot fixtures (issue #916) ──────────────────────── + +/// Fixture A: agent with on.pr and default `mode: synthetic`. +/// Compiled YAML must contain the full synth wiring (synthPr Setup step, +/// PR_SYNTH_SPEC env, broadened exec-context-pr.js condition, agent-job +/// AW_SYNTHETIC_PR_SKIP guard). The CI trigger must NOT be auto-narrowed +/// to `pr.branches.include` — those are PR target branches, and narrowing +/// would suppress CI on the feature branches synthPr actually needs. +#[test] +fn test_synthetic_pr_default_emits_full_synth_wiring() { + let compiled = compile_fixture_with_flags("synthetic-pr-default.md", &["--skip-integrity"]); + assert_valid_yaml(&compiled, "synthetic-pr-default.md"); + + // synthPr step in Setup job, before prGate. + assert!( + compiled.contains("name: synthPr"), + "Fixture A must emit the synthPr Setup-job step" + ); + assert!( + compiled.contains("PR_SYNTH_SPEC:"), + "Fixture A must emit the PR_SYNTH_SPEC env var carrying the base64 spec" + ); + assert!( + compiled.contains("exec-context-pr-synth.js"), + "Fixture A must reference the synth bundle path" + ); + + // Broadened exec-context-pr condition. + assert!( + compiled.contains( + "condition: or(eq(variables['Build.Reason'], 'PullRequest'), eq(dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR'], 'true'))" + ), + "Fixture A must broaden the exec-context-pr.js condition to accept synthetic PRs" + ); + + // Agent-job AW_SYNTHETIC_PR_SKIP guard. + assert!( + compiled.contains("ne(dependencies.Setup.outputs['synthPr.AW_SYNTHETIC_PR_SKIP'], 'true')"), + "Fixture A's Agent-job condition must honour the synth-skip flag" + ); + // NOTE: this fixture does not declare `on.pr.filters`, so the + // Agent-job condition has only the skip guard (no AND-NOT gate + // clause). The `eq(synthPr.AW_SYNTHETIC_PR, 'true')` literal is + // therefore expected ONLY in the exec-context-pr.js step's + // broadened condition (asserted above) — never as an Agent-job + // OR-arm, which would silently bypass the gate for real-PR or + // synth-PR builds. A separate fixture covers the gate-enforced + // shape when `pr.filters` is present. + + // No auto-narrowed CI trigger — `pr.branches.include` lists PR TARGET + // branches, and ADO `trigger:` fires on pushes TO listed branches, so + // narrowing would suppress CI on the feature branches synthPr needs. + assert!( + !compiled.contains("trigger:\n branches:\n include:\n - 'main'"), + "Fixture A must NOT auto-narrow the top-level CI trigger to pr.branches.include — \ + narrowing to PR target branches would defeat synthPr by suppressing CI on the \ + feature branches it must react to" + ); +} + +/// Fixture B: agent with on.pr and `mode: policy`. +/// The compiled YAML must contain NONE of the synthesis artefacts AND +/// must emit `trigger: none` so feature-branch pushes do not queue +/// duplicate CI builds alongside the operator's branch-policy-driven PR +/// builds. +#[test] +fn test_pr_mode_policy_omits_synth_and_emits_trigger_none() { + let compiled = compile_fixture_with_flags("pr-mode-policy.md", &["--skip-integrity"]); + assert_valid_yaml(&compiled, "pr-mode-policy.md"); + + for needle in &[ + "synthPr", + "AW_SYNTHETIC_PR", + "PR_SYNTH_SPEC", + "exec-context-pr-synth", + ] { + assert!( + !compiled.contains(needle), + "mode: policy must produce zero synth artefacts; \ + found {needle} in compiled YAML" + ); + } + + // CI trigger must be suppressed so we don't double-queue with the + // policy-driven PR build. + assert!( + compiled.contains("trigger: none"), + "mode: policy must emit `trigger: none` so feature-branch pushes do not \ + queue duplicate CI builds alongside the branch-policy-driven PR build" + ); + + // And of course must NOT auto-narrow either (defensive). + assert!( + !compiled.contains("trigger:\n branches:\n include:\n - 'main'"), + "mode: policy must not emit a narrowed CI trigger" + ); +} diff --git a/tests/fixtures/pr-mode-policy.md b/tests/fixtures/pr-mode-policy.md new file mode 100644 index 00000000..058d3a08 --- /dev/null +++ b/tests/fixtures/pr-mode-policy.md @@ -0,0 +1,21 @@ +--- +name: "PR Mode Policy Agent" +description: "Fixture exercising on.pr with mode: policy (issue #916)" +on: + pr: + branches: + include: [main] + mode: policy +--- + +## PR Mode Policy Agent + +This agent has `on.pr` configured with `mode: policy`, meaning the +operator has installed an Azure DevOps Build Validation branch policy +that fires real `Build.Reason == PullRequest` builds and the compiler +must: + +- emit none of the synth artefacts (`synthPr`, `AW_SYNTHETIC_PR`, + `PR_SYNTH_SPEC`, `exec-context-pr-synth`), and +- emit `trigger: none` at the top level so feature-branch pushes do not + queue duplicate CI builds alongside the policy-driven PR build. diff --git a/tests/fixtures/synthetic-pr-default.md b/tests/fixtures/synthetic-pr-default.md new file mode 100644 index 00000000..7dab497d --- /dev/null +++ b/tests/fixtures/synthetic-pr-default.md @@ -0,0 +1,24 @@ +--- +name: "Synthetic PR Default Agent" +description: "Fixture exercising on.pr with default mode: synthetic (issue #916)" +on: + pr: + branches: + include: [main] +--- + +## Synthetic PR Default Agent + +This agent has `on.pr` configured with the default `mode: synthetic`. +On a CI-triggered build (no Build Validation policy), the Setup-job +`synthPr` step looks up the open PR for `Build.SourceBranch` and exposes +its identifiers so the gate evaluator and exec-context-pr bundles +behave as if `Build.Reason == PullRequest`. + +The compiled YAML must contain: + +- A `synthPr` step in the Setup job, before the gate +- A `PR_SYNTH_SPEC:` env var carrying the base64 spec +- The broadened `exec-context-pr.js` condition (`or(...)`) +- The Agent-job `dependsOn` condition with the `AW_SYNTHETIC_PR_SKIP` guard +- No top-level `trigger:` block (ADO default = "trigger on every branch")