Skip to content

fix(compile): unify synthetic-PR variable namespace via ado-script #972

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jamesadevine merged 1 commit into from
Jun 11, 2026
Merged

fix(compile): unify synthetic-PR variable namespace via ado-script #972

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions docs/template-markers.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,12 +300,14 @@ Generates the Agent job's `variables:` block. Currently emits content **only** w

When active, this hoists the relevant `synthPr` Setup-job step outputs into Agent-job-level variables using `$[ coalesce(dependencies.Setup.outputs['synthPr.X'], '') ]` runtime expressions:

- `AW_SYNTHETIC_PR`
- `AW_SYNTHETIC_PR_ID`
- `AW_SYNTHETIC_PR_TARGETBRANCH`
- `AW_SYNTHETIC_PR_SOURCEBRANCH`
- `AW_PR_ID` — resolved PR id (real on PR builds, discovered on synth-promoted CI builds)
- `AW_PR_TARGETBRANCH` — resolved PR target branch (`refs/heads/<name>`)
- `AW_PR_SOURCEBRANCH` — resolved PR source branch
- `AW_SYNTHETIC_PR` — `"true"` only when this build was synth-promoted from CI; empty on real PR builds

The hoist exists because `dependencies.<job>.outputs[...]` references at step-level `env:` scope proved unreliable in practice (empirically observed in `msazuresphere/4x4` build #612290: the same reference resolved correctly at job-condition scope but returned the empty string at step-env scope, causing the `Stage PR execution context` step's bash guard to misfire and the agent to emit `noop` on a synth-promoted build). Job-level `variables:` is the documented safe location for cross-job output references; subsequent step `env:` blocks then consume the hoisted values via the `$(name)` macro or a `$[ coalesce(variables['name'], ...) ]` runtime expression.
The hoist exists because ADO `$[ ... ]` runtime expressions are ONLY evaluated inside `variables:` mappings and `condition:` fields — putting them in step `env:` values passes the literal expression string verbatim to bash (empirically observed in `msazuresphere/4x4` build #612528: the `Stage PR execution context` step received `PR_ID='$[ coalesce(...)...` as a literal and PR-identifier validation rejected it). Job-level `variables:` is the documented safe location for cross-job output references; subsequent step `env:` blocks then consume the hoisted values via the plain `$(name)` macro (no `$[ ... ]` in step env, ever).

The real-vs-synth merge happens inside `exec-context-pr-synth.js` so consumers read a single canonical name regardless of whether the build is a real PR or a synth-promoted CI build.

## {{ working_directory }}

Expand Down
113 changes: 88 additions & 25 deletions scripts/ado-script/src/exec-context-pr-synth/__tests__/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,66 @@ describe("exec-context-pr-synth main", () => {
});
afterEach(() => vi.restoreAllMocks());

it("no-ops on real PR builds (BUILD_REASON=PullRequest)", async () => {
// ── Real-PR path ─────────────────────────────────────────────────
//
// On a real PR build, ADO populates `SYSTEM_PULLREQUEST_*` env vars
// directly. The bundle propagates them into the canonical `AW_PR_*`
// namespace so downstream consumers can read a single name regardless
// of the build's reason. No API call is made; no `AW_SYNTHETIC_PR`
// flag is emitted (this is not a synth-promotion).

it("propagates SYSTEM_PULLREQUEST_* to AW_PR_* on real PR builds", async () => {
const { code, output } = await runMain(
makeEnv({ BUILD_REASON: "PullRequest", PR_SYNTH_SPEC: build_pr_synth_spec() }),
makeEnv({
BUILD_REASON: "PullRequest",
SYSTEM_PULLREQUEST_PULLREQUESTID: "4242",
SYSTEM_PULLREQUEST_TARGETBRANCH: "refs/heads/main",
SYSTEM_PULLREQUEST_SOURCEBRANCH: "refs/heads/feature/x",
SYSTEM_PULLREQUEST_ISDRAFT: "false",
PR_SYNTH_SPEC: build_pr_synth_spec(),
}),
);
expect(code).toBe(0);
expect(mocked.listActivePullRequestsBySourceRef).not.toHaveBeenCalled();
// AW_PR_* are emitted as BOTH output (cross-job) and var (same-job).
expect(output).toContain("AW_PR_ID;isOutput=true]4242");
expect(output).toContain("##vso[task.setvariable variable=AW_PR_ID]4242");
expect(output).toContain("AW_PR_TARGETBRANCH;isOutput=true]refs/heads/main");
expect(output).toContain(
"##vso[task.setvariable variable=AW_PR_TARGETBRANCH]refs/heads/main",
);
expect(output).toContain("AW_PR_SOURCEBRANCH;isOutput=true]refs/heads/feature/x");
expect(output).toContain(
"##vso[task.setvariable variable=AW_PR_SOURCEBRANCH]refs/heads/feature/x",
);
expect(output).toContain("AW_PR_IS_DRAFT;isOutput=true]false");
// No synth-promotion flag on a real PR build.
expect(output).not.toContain("AW_SYNTHETIC_PR;isOutput=true]true");
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 () => {
it("detects real PR build by SYSTEM_PULLREQUEST_PULLREQUESTID, not BUILD_REASON", async () => {
// Defensive: even on builds where BUILD_REASON isn't "PullRequest"
// for some reason (e.g. a manual re-queue of a PR build), the
// presence of a SYSTEM_PULLREQUEST_PULLREQUESTID is the authoritative
// signal — that's the value we need to propagate.
const { code, output } = await runMain(
makeEnv({
BUILD_REASON: "Manual",
SYSTEM_PULLREQUEST_PULLREQUESTID: "99",
SYSTEM_PULLREQUEST_TARGETBRANCH: "refs/heads/main",
PR_SYNTH_SPEC: build_pr_synth_spec(),
}),
);
expect(code).toBe(0);
expect(mocked.listActivePullRequestsBySourceRef).not.toHaveBeenCalled();
expect(output).toContain("AW_PR_ID;isOutput=true]99");
});

// ── GitHub repo path ────────────────────────────────────────────

it("skips with empty AW_PR_* on GitHub-typed repos (CI builds)", async () => {
const { code, output } = await runMain(
makeEnv({
BUILD_REPOSITORY_PROVIDER: "GitHub",
Expand All @@ -44,8 +92,15 @@ describe("exec-context-pr-synth main", () => {
expect(code).toBe(0);
expect(mocked.listActivePullRequestsBySourceRef).not.toHaveBeenCalled();
expect(output).toContain("GitHub-typed repo");
// SKIP marker tells the Agent job's condition to opt out cleanly.
expect(output).toContain("AW_SYNTHETIC_PR_SKIP;isOutput=true]true");
// Empty AW_PR_* so same-job consumers see stable defined variables.
expect(output).toContain("##vso[task.setvariable variable=AW_PR_ID]");
expect(output).not.toContain("AW_PR_ID;isOutput=true]4242");
});

// ── Hard failures ────────────────────────────────────────────────

it("returns 1 (hard fail) when PR_SYNTH_SPEC is missing", async () => {
const env = makeEnv({});
delete env.PR_SYNTH_SPEC;
Expand All @@ -62,6 +117,13 @@ describe("exec-context-pr-synth main", () => {
expect(output).toContain("PR_SYNTH_SPEC");
});

// ── Soft skips (CI build, ADO repo, no matching PR) ─────────────
//
// Every soft-skip path emits empty AW_PR_* via setVar+setOutput so
// downstream consumers see stable defined variables (rather than the
// literal `$(AW_PR_ID)` string that ADO leaves when a macro is
// undefined). The SKIP marker gates the Agent job's `condition:`.

it("skips when source branch has no active PR (per ADO API)", async () => {
mocked.listActivePullRequestsBySourceRef.mockResolvedValue([]);
const { code, output } = await runMain(
Expand All @@ -72,6 +134,8 @@ describe("exec-context-pr-synth main", () => {
);
expect(code).toBe(0);
expect(output).toContain("AW_SYNTHETIC_PR_SKIP;isOutput=true]true");
// Empty defaults for AW_PR_*.
expect(output).toContain("##vso[task.setvariable variable=AW_PR_ID]");
expect(mocked.listActivePullRequestsBySourceRef).toHaveBeenCalledOnce();
});

Expand All @@ -87,6 +151,7 @@ describe("exec-context-pr-synth main", () => {
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("##vso[task.setvariable variable=AW_PR_ID]");
});

it("skips when >1 active PRs match (after target filter)", async () => {
Expand Down Expand Up @@ -118,7 +183,9 @@ describe("exec-context-pr-synth main", () => {
expect(output).toContain("no changed file");
});

it("emits AW_SYNTHETIC_PR=true + identifiers on the happy path", async () => {
// ── Happy path: synth-promote a CI build with a matching PR ─────

it("emits AW_PR_* + AW_SYNTHETIC_PR=true on the synth happy path", async () => {
mocked.listActivePullRequestsBySourceRef.mockResolvedValue([
{
pullRequestId: 1234,
Expand All @@ -138,31 +205,27 @@ describe("exec-context-pr-synth main", () => {
const { code, output } = await runMain(makeEnv({ PR_SYNTH_SPEC: spec }));
expect(code).toBe(0);
expect(output).not.toContain("AW_SYNTHETIC_PR_SKIP");
// Each AW_SYNTHETIC_PR* variable is emitted TWICE: once as an
// output (cross-job, consumed by the Agent job condition + the
// Agent-job-level `variables:` hoist) and once as a regular
// variable (same-job, consumed by the prGate step's env block
// via `$[ coalesce(variables['AW_SYNTHETIC_PR_X'], ...) ]`).
// See `setVar` in `shared/vso-logger.ts` for the rationale.
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");
// Regular-variable counterparts (no `isOutput`). Each line is a
// separate ##vso command terminated by `]value`.
expect(output).toContain("##vso[task.setvariable variable=AW_SYNTHETIC_PR]true");
expect(output).toContain("##vso[task.setvariable variable=AW_SYNTHETIC_PR_ID]1234");
// AW_PR_* emitted TWICE: once as output (cross-job, hoisted into the
// Agent job's `variables:` block) and once as a regular variable
// (same-job, consumed by the Setup-job gate step's `env:` via
// `$(AW_PR_*)` macros). See `setVar` in `shared/vso-logger.ts`.
expect(output).toContain("AW_PR_ID;isOutput=true]1234");
expect(output).toContain("##vso[task.setvariable variable=AW_PR_ID]1234");
expect(output).toContain("AW_PR_TARGETBRANCH;isOutput=true]refs/heads/main");
expect(output).toContain(
"##vso[task.setvariable variable=AW_SYNTHETIC_PR_TARGETBRANCH]refs/heads/main",
"##vso[task.setvariable variable=AW_PR_TARGETBRANCH]refs/heads/main",
);
expect(output).toContain("AW_PR_SOURCEBRANCH;isOutput=true]refs/heads/feature/x");
expect(output).toContain(
"##vso[task.setvariable variable=AW_SYNTHETIC_PR_SOURCEBRANCH]refs/heads/feature/x",
"##vso[task.setvariable variable=AW_PR_SOURCEBRANCH]refs/heads/feature/x",
);
expect(output).toContain("##vso[task.setvariable variable=AW_SYNTHETIC_PR_IS_DRAFT]false");
expect(output).toContain("AW_PR_IS_DRAFT;isOutput=true]false");
// Synth-promotion flag (only on this path, not real PR or skip).
expect(output).toContain("AW_SYNTHETIC_PR;isOutput=true]true");
expect(output).toContain("##vso[task.setvariable variable=AW_SYNTHETIC_PR]true");
});

it("emits AW_SYNTHETIC_PR_IS_DRAFT=true when the PR is a draft", async () => {
it("emits AW_PR_IS_DRAFT=true when the matched synth PR is a draft", async () => {
mocked.listActivePullRequestsBySourceRef.mockResolvedValue([
{
pullRequestId: 1,
Expand All @@ -173,7 +236,7 @@ describe("exec-context-pr-synth main", () => {
]);
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");
expect(output).toContain("AW_PR_IS_DRAFT;isOutput=true]true");
});

it("skips path-filter API calls when paths.include and exclude are both empty", async () => {
Expand Down
Loading
Loading