From 484a5f4e68e02c8d4c0f20de7b39e8f7c0968e37 Mon Sep 17 00:00:00 2001 From: Dominic Nguyen Date: Wed, 12 Aug 2026 16:12:58 -0700 Subject: [PATCH 01/12] Skip redundant PR CI work without dropping merge-queue coverage Required status-check names stay on every OS/Node cell so the main ruleset still sees a report. pull_request cells just do less work. - Drive scope from ts/tools/scripts/prCiScope.mjs (full / ratchet / package). push and merge_group still run the full matrix. - build-ts PRs: skip Node 24 install/build/test; run ratchets and a single git fetch of the base on ubuntu/22 only. - build-ts PRs: shallow checkout on non-ratchet cells. - build-package-shell PRs: keep ubuntu packaging; skip macos/windows. - azure-smoke-tests detect job: fetchDepth 2 (HEAD and HEAD^1). - Add node --test coverage that invokes the real CLI and reads the shipped workflow YAML. --- .github/workflows/build-package-shell.yml | 23 ++- .github/workflows/build-ts.yml | 51 +++-- pipelines/azure-smoke-tests.yml | 7 +- ts/package.json | 1 + ts/tools/scripts/code/README.md | 11 +- ts/tools/scripts/pr-ci-scope.md | 80 ++++++++ ts/tools/scripts/prCiScope.mjs | 222 ++++++++++++++++++++++ ts/tools/scripts/test/prCiScope.spec.mjs | 220 +++++++++++++++++++++ 8 files changed, 582 insertions(+), 33 deletions(-) create mode 100644 ts/tools/scripts/pr-ci-scope.md create mode 100644 ts/tools/scripts/prCiScope.mjs create mode 100644 ts/tools/scripts/test/prCiScope.spec.mjs diff --git a/.github/workflows/build-package-shell.yml b/.github/workflows/build-package-shell.yml index bcb0d30c6f..f3be8e8359 100644 --- a/.github/workflows/build-package-shell.yml +++ b/.github/workflows/build-package-shell.yml @@ -50,24 +50,35 @@ jobs: ts: - "ts/**" - ".github/workflows/build-package-shell.yml" + # Required check names stay on every OS cell. PRs keep ubuntu as a + # packaging smoke; macos/windows package on push/merge_group/main. + # See ts/tools/scripts/prCiScope.mjs. + - name: Decide job scope + id: scope + env: + EVENT_NAME: ${{ github.event_name }} + TS_FILTER: ${{ steps.filter.outputs.ts }} + MATRIX_OS: ${{ matrix.os }} + MATRIX_VERSION: ${{ matrix.version }} + run: node ts/tools/scripts/prCiScope.mjs - uses: pnpm/action-setup@v4 - if: ${{ github.event_name != 'pull_request' || steps.filter.outputs.ts != 'false' }} + if: ${{ steps.scope.outputs.package == 'true' }} name: Install pnpm with: package_json_file: ts/package.json - uses: actions/setup-node@v5 - if: ${{ github.event_name != 'pull_request' || steps.filter.outputs.ts != 'false' }} + if: ${{ steps.scope.outputs.package == 'true' }} with: node-version: ${{ matrix.version }} cache: "pnpm" cache-dependency-path: ts/pnpm-lock.yaml - name: Install dependencies - if: ${{ github.event_name != 'pull_request' || steps.filter.outputs.ts != 'false' }} + if: ${{ steps.scope.outputs.package == 'true' }} working-directory: ts run: | pnpm install --frozen-lockfile --strict-peer-dependencies - name: Build - if: ${{ github.event_name != 'pull_request' || steps.filter.outputs.ts != 'false' }} + if: ${{ steps.scope.outputs.package == 'true' }} working-directory: ts run: | pnpm run build:shell @@ -78,7 +89,7 @@ jobs: # blocked builds. This is a bug in electron-builder, it's not smart enough to retry acquiring the archive. # Disabling for now. # - name: Electron Builder Cache - # if: ${{ github.event_name != 'pull_request' || steps.filter.outputs.ts != 'false' }} + # if: ${{ steps.scope.outputs.package == 'true' }} # uses: actions/cache@v4 # with: # key: electron | ${{ runner.os }} | ${{ runner.arch }} | ${{ hashFiles('**/pnpm-lock.yaml') }} @@ -88,7 +99,7 @@ jobs: # restore-keys: | # electron | ${{ runner.os }} | ${{ runner.arch }} - name: Package - shell - if: ${{ github.event_name != 'pull_request' || steps.filter.outputs.ts != 'false' }} + if: ${{ steps.scope.outputs.package == 'true' }} working-directory: ts shell: bash run: pnpm run shell:package diff --git a/.github/workflows/build-ts.yml b/.github/workflows/build-ts.yml index 47b5421d8d..09d2a6d453 100644 --- a/.github/workflows/build-ts.yml +++ b/.github/workflows/build-ts.yml @@ -44,8 +44,11 @@ jobs: git config --global core.autocrlf false - uses: actions/checkout@v5 with: - # Full history so the changed-files lint can diff against the base. - fetch-depth: 0 + # Full history only on the PR ratchet cell (ubuntu/22), which diffs + # against the merge-base. Other PR cells are shallow. push / + # merge_group keep a full clone so `npm run lint -- --ratchet` can + # still resolve origin/main. + fetch-depth: ${{ github.event_name == 'pull_request' && !(matrix.os == 'ubuntu-latest' && matrix.version == 22) && 1 || 0 }} - uses: dorny/paths-filter@v3 id: filter continue-on-error: true @@ -54,36 +57,50 @@ jobs: ts: - "ts/**" - ".github/workflows/build-ts.yml" + # Required check names stay on every matrix cell. This step only + # decides whether the cell does expensive work. PRs skip Node 24 + # install/build/test (still runs on push/merge_group/main) and run + # the ratchets once on ubuntu/22. See ts/tools/scripts/prCiScope.mjs. + - name: Decide job scope + id: scope + env: + EVENT_NAME: ${{ github.event_name }} + TS_FILTER: ${{ steps.filter.outputs.ts }} + MATRIX_OS: ${{ matrix.os }} + MATRIX_VERSION: ${{ matrix.version }} + run: node ts/tools/scripts/prCiScope.mjs - uses: pnpm/action-setup@v4 - if: ${{ github.event_name != 'pull_request' || steps.filter.outputs.ts != 'false' }} + if: ${{ steps.scope.outputs.full == 'true' || steps.scope.outputs.lint == 'true' }} name: Install pnpm with: package_json_file: ts/package.json - uses: actions/setup-node@v5 - if: ${{ github.event_name != 'pull_request' || steps.filter.outputs.ts != 'false' }} + if: ${{ steps.scope.outputs.full == 'true' || steps.scope.outputs.lint == 'true' }} with: node-version: ${{ matrix.version }} cache: "pnpm" cache-dependency-path: ts/pnpm-lock.yaml - name: Install dependencies - if: ${{ github.event_name != 'pull_request' || steps.filter.outputs.ts != 'false' }} + if: ${{ steps.scope.outputs.full == 'true' || steps.scope.outputs.lint == 'true' }} working-directory: ts run: | pnpm install --frozen-lockfile --strict-peer-dependencies - name: Build - if: ${{ github.event_name != 'pull_request' || steps.filter.outputs.ts != 'false' }} + if: ${{ steps.scope.outputs.full == 'true' }} working-directory: ts run: | npm run build + - name: Fetch PR base + if: ${{ steps.scope.outputs.fetch == 'true' }} + run: git fetch --no-tags origin "${{ github.base_ref }}" # On pull requests only changed files are checked (fast); the # format-pr workflow auto-fixes them. Other events check the whole repo. - name: Lint - if: ${{ github.event_name != 'pull_request' || steps.filter.outputs.ts != 'false' }} + if: ${{ steps.scope.outputs.lint == 'true' }} working-directory: ts shell: bash run: | if [ "${{ github.event_name }}" = "pull_request" ]; then - git fetch --no-tags origin "${{ github.base_ref }}" node tools/scripts/prettier-changed.mjs --base "origin/${{ github.base_ref }}" else npm run lint @@ -96,11 +113,10 @@ jobs: # // code-complexity-allow markers. Tune the thresholds down as hotspots # get refactored. - name: Complexity ratchet - if: ${{ github.event_name == 'pull_request' && steps.filter.outputs.ts != 'false' }} + if: ${{ steps.scope.outputs.ratchet == 'true' }} working-directory: ts shell: bash run: | - git fetch --no-tags origin "${{ github.base_ref }}" npm run code-complexity -- --ratchet --base "origin/${{ github.base_ref }}" \ --cyclomatic 25 --cognitive 30 \ --new-file-cyclomatic 25 --new-file-cognitive 30 @@ -109,45 +125,42 @@ jobs: # base branch. Syntactic rules only, so it is fast; the count can only # trend down. Run `npm run code-lint` locally to see the full report. - name: Lint ratchet - if: ${{ github.event_name == 'pull_request' && steps.filter.outputs.ts != 'false' }} + if: ${{ steps.scope.outputs.ratchet == 'true' }} working-directory: ts shell: bash run: | - git fetch --no-tags origin "${{ github.base_ref }}" npm run code-lint -- --ratchet --base "origin/${{ github.base_ref }}" # Circular-dependency ratchet (PRs only): fail if the change introduces a # runtime import cycle not already present at the base. Builds the cycle # set for HEAD and for the merge base (via a throwaway git worktree), so # this step is heavier than the others (madge runs twice). - name: Circular dependency ratchet - if: ${{ github.event_name == 'pull_request' && steps.filter.outputs.ts != 'false' }} + if: ${{ steps.scope.outputs.ratchet == 'true' }} working-directory: ts shell: bash run: | - git fetch --no-tags origin "${{ github.base_ref }}" npm run code-circular -- --ratchet --base "origin/${{ github.base_ref }}" --exceptions-file tools/scripts/code/circular-baseline-exception.json # Test-debt gate (PRs only): zero tolerance for focused tests # (.only/fit/fdescribe) and no newly skipped tests (.skip/xit/xdescribe) # in changed files. A small, fixable problem -> a hard gate, not a ratchet. - name: Test debt gate - if: ${{ github.event_name == 'pull_request' && steps.filter.outputs.ts != 'false' }} + if: ${{ steps.scope.outputs.ratchet == 'true' }} working-directory: ts shell: bash run: | - git fetch --no-tags origin "${{ github.base_ref }}" npm run code-debt -- --gate --base "origin/${{ github.base_ref }}" - name: Restore better-sqlite3 for Node.js - if: ${{ github.event_name != 'pull_request' || steps.filter.outputs.ts != 'false' }} + if: ${{ steps.scope.outputs.full == 'true' }} working-directory: ts run: | pnpm run postinstall:better-sqlite3-node-restore - name: Test - if: ${{ github.event_name != 'pull_request' || steps.filter.outputs.ts != 'false' }} + if: ${{ steps.scope.outputs.full == 'true' }} working-directory: ts run: | npm run test:local - name: UI tests (requires display) - if: ${{ (github.event_name != 'pull_request' || steps.filter.outputs.ts != 'false') && runner.os == 'Linux' }} + if: ${{ steps.scope.outputs.full == 'true' && runner.os == 'Linux' }} working-directory: ts run: | Xvfb :99 -screen 0 1600x1200x24 & diff --git a/pipelines/azure-smoke-tests.yml b/pipelines/azure-smoke-tests.yml index 17643ca2a3..9e0c690b75 100644 --- a/pipelines/azure-smoke-tests.yml +++ b/pipelines/azure-smoke-tests.yml @@ -114,10 +114,11 @@ jobs: pool: vmImage: ubuntu-latest steps: - # Full history so merge-base / parent diffs resolve on both PR merge - # commits and merge-queue branches. + # PR merge commits only need HEAD and HEAD^1 for the diff below. + # CI / merge-queue builds fetch origin main in the script and treat a + # missing merge-base as "run the tests", so a shallow clone is safe. - checkout: self - fetchDepth: 0 + fetchDepth: 2 - bash: | set -uo pipefail # Default to running the tests; only skip when we can positively diff --git a/ts/package.json b/ts/package.json index dfec421605..0a240c003d 100644 --- a/ts/package.json +++ b/ts/package.json @@ -82,6 +82,7 @@ "test:keys": "npx tsx tools/scripts/testServiceKeys.ts", "test:live": "pnpm -r ---no-bail -no-sort --stream --workspace-concurrency=1 run test:live", "test:local": "pnpm -r --no-bail --no-sort --stream --workspace-concurrency=3 run test:local", + "test:pr-ci-scope": "node --test tools/scripts/test/prCiScope.spec.mjs", "test:ui": "pnpm -r --no-bail --no-sort --stream --if-present run test:ui" }, "devDependencies": { diff --git a/ts/tools/scripts/code/README.md b/ts/tools/scripts/code/README.md index 3c6a98564b..7153ef2445 100644 --- a/ts/tools/scripts/code/README.md +++ b/ts/tools/scripts/code/README.md @@ -115,12 +115,13 @@ Four code-quality steps run in [`build-ts.yml`](../../../../.github/workflows/build-ts.yml), **on pull requests only**, sequenced after `Build` and before `Test`. They are skipped entirely unless the PR touches `ts/**` or the workflow file itself (a `dorny/paths-filter` -guard), and — like the rest of the job — they run on every matrix cell -(`ubuntu`/`windows`/`macos` × Node 22/24). +guard). The gates are repo-wide, not OS-specific, so they run once on the +`ubuntu-latest` + Node 22 cell (see `ts/tools/scripts/prCiScope.mjs`). -Each step is a **changed-files diff against the PR's base branch**: it first -`git fetch --no-tags origin `, then passes `--base origin/` -so only what the PR actually touches is judged. Two flavors: +Each step is a **changed-files diff against the PR's base branch**: the +workflow fetches `origin/` once, then every gate passes +`--base origin/` so only what the PR actually touches is judged. Two +flavors: - **Ratchet** (`--ratchet`) — _stateless_: the base branch _is_ the baseline (there is no committed baseline file), so the metric can only trend down. diff --git a/ts/tools/scripts/pr-ci-scope.md b/ts/tools/scripts/pr-ci-scope.md new file mode 100644 index 0000000000..6ed26a21b9 --- /dev/null +++ b/ts/tools/scripts/pr-ci-scope.md @@ -0,0 +1,80 @@ +# Faster PR pipelines — scope and measurements + +PR-triggered `build-ts` / `build-package-shell` jobs keep the same required +status-check _names_ (the branch ruleset lists every OS × Node cell). This +change only skips expensive work on the `pull_request` event. `push`, +`merge_group`, and `main` still do the full matrix. + +## What changed + +- Shared decision helper: `ts/tools/scripts/prCiScope.mjs` (wired from both + workflows). Tests: `pnpm run test:pr-ci-scope`. +- `build-ts` PRs: Node 24 cells report without install/build/test. Ratchets + and the PR-base `git fetch` run once (ubuntu/22) instead of five fetches on + every cell. +- `build-ts` PRs: non-ratchet cells use `fetch-depth: 1` instead of a full + clone (~2700 commits). +- `build-package-shell` PRs: ubuntu still packages; macos/windows package on + `push` / `merge_group` / `main`. +- `pipelines/azure-smoke-tests.yml` detect job: `fetchDepth: 2` (only HEAD + and `HEAD^1` are needed on a PR merge commit). + +## Job counts (from the shipped helper) + +Run `node tools/scripts/prCiScope.mjs --table` from `ts/`: + +| event | ts filter | ts full | ts ratchet | shell package | +| ------------------------- | ------------ | ------- | ---------- | ------------- | +| pull_request (before) | ts changed | 6 / 6 | 6 / 6 | 3 / 3 | +| pull_request (after) | ts changed | 3 / 6 | 1 / 6 | 1 / 3 | +| pull_request (after) | no ts change | 0 / 6 | 0 / 6 | 0 / 3 | +| merge_group / push / main | (ignored) | 6 / 6 | 0 / 6 | 3 / 3 | + +A TS-touching PR drops from **9 full install+build(+test/package) cells to 4** +(3 Node 22 `build_ts` + 1 ubuntu `build_package_shell`). The other 5 required +cells still start and succeed after checkout + path filter + scope. + +## Timed local analog (this clone, file://) + +Repo history at the branch tip: **2689** commits. + +| analog | wall | `.git` size | commits | +| ------------------------------------------- | ----- | ----------- | ------- | +| `git clone --depth 1` (PR non-ratchet cell) | 2.15s | 60 MiB | 1 | +| `git clone` (fetch-depth 0) | 2.90s | 132 MiB | 2689 | +| 1× `git fetch` after clone | 0.03s | — | — | +| 5× `git fetch` after clone | 0.20s | — | — | + +The local file:// clone understates GitHub-hosted checkout cost (network + +Actions cache). The size cut (132 MiB → 60 MiB) is what the 5 non-ratchet PR +cells no longer download. + +## Historical CI cost of the skipped cells (microsoft/TypeAgent#2847) + +Typical `ts/**` PR, 2026-08-12: + +| job | duration | now on `pull_request` | +| ------------------------------------------ | ---------------------------------- | ----------------------- | +| `build_ts (ubuntu-latest, 22)` | 14m 13s | still full | +| `build_ts (ubuntu-latest, 24)` | 12m 39s | skip install/build/test | +| `build_ts (macos-latest, 22)` | 15m 52s | still full | +| `build_ts (macos-latest, 24)` | 22m 51s | skip install/build/test | +| `build_ts (windows-latest, 22)` | 20m 38s | still full | +| `build_ts (windows-latest, 24)` | 17m 01s (queued behind windows 22) | skip install/build/test | +| `build_package_shell (ubuntu-latest, 22)` | 8m 58s | still packages | +| `build_package_shell (windows-latest, 22)` | 13m 02s | skip package | +| `build_package_shell (macos-latest, 22)` | 18m 38s | skip package | + +Runner-minutes avoided on a TS PR: ~52m (Node 24 `build_ts`) + ~32m +(macos/windows package) ≈ **84 minutes**. Wall clock on #2847 was gated by +Windows serialization + smoke tests (~40m); skipping `windows-24` removes +that extra Windows queue slot. + +## Draft PR FYI + +**A draft PR’s pipeline will not run until it is approved.** Azure DevOps PR +validation (`azure-smoke-tests.yml`, required check “TypeAgent Smoke Tests”) +and some GitHub Actions environment / required-workflow gates stay pending +until a reviewer or admin approves the run. Request `/azp run` (or the GitHub +Actions “Approve and run”) after opening the draft; do not treat an empty +check rollup as a YAML failure. diff --git a/ts/tools/scripts/prCiScope.mjs b/ts/tools/scripts/prCiScope.mjs new file mode 100644 index 0000000000..df49e4e49b --- /dev/null +++ b/ts/tools/scripts/prCiScope.mjs @@ -0,0 +1,222 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +// Shared PR-vs-main job-scope decisions used by build-ts.yml and +// build-package-shell.yml. Required status-check *names* stay the same +// (every matrix cell still reports); this only decides whether the cell +// does the expensive install/build/test/package work. +// +// Coverage that is skipped on pull_request still runs on push / merge_group +// / workflow_dispatch (and on main). +// +// Usage in Actions: +// EVENT_NAME, TS_FILTER, MATRIX_OS, MATRIX_VERSION -> GITHUB_OUTPUT +// Local table: +// node tools/scripts/prCiScope.mjs --table + +import fs from "node:fs"; +import { pathToFileURL } from "node:url"; + +// dorny/paths-filter writes "true" / "false". The existing workflows treat +// anything other than the string "false" as "run" (including an unset +// output when the action hits continue-on-error). +export function pathFilterAllows(output) { + return output !== "false"; +} + +function isPullRequest(eventName) { + return eventName === "pull_request"; +} + +function nodeVersion(version) { + return Number(version); +} + +/** + * Full install + build + test:local (+ UI tests on Linux). + * PRs skip Node 24 — that cell still reports, and Node 24 is covered on + * push / merge_group / main. + */ +export function shouldRunBuildTsFull({ eventName, tsFilter, version }) { + if (!isPullRequest(eventName)) { + return true; + } + if (!pathFilterAllows(tsFilter)) { + return false; + } + return nodeVersion(version) === 22; +} + +/** + * Changed-file prettier + complexity / lint / circular / debt gates. + * These are repo-wide (not OS-specific), so they run once on ubuntu + 22. + */ +export function shouldRunBuildTsRatchet({ eventName, tsFilter, os, version }) { + if (!isPullRequest(eventName)) { + return false; + } + if (!pathFilterAllows(tsFilter)) { + return false; + } + return os === "ubuntu-latest" && nodeVersion(version) === 22; +} + +/** + * Lint step: whole-repo lint on non-PR events (every cell, same as today); + * on PRs it is the changed-file prettier check and shares the ratchet cell. + */ +export function shouldRunBuildTsLint(ctx) { + if (!isPullRequest(ctx.eventName)) { + return true; + } + return shouldRunBuildTsRatchet(ctx); +} + +export function shouldFetchPrBase(ctx) { + return shouldRunBuildTsRatchet(ctx); +} + +/** + * Electron shell packaging. PRs keep the ubuntu cell as a smoke; macos + * and windows package on push / merge_group / main. + */ +export function shouldRunShellPackage({ eventName, tsFilter, os }) { + if (!isPullRequest(eventName)) { + return true; + } + if (!pathFilterAllows(tsFilter)) { + return false; + } + return os === "ubuntu-latest"; +} + +export function resolveScope(ctx) { + return { + full: shouldRunBuildTsFull(ctx), + ratchet: shouldRunBuildTsRatchet(ctx), + lint: shouldRunBuildTsLint(ctx), + fetch: shouldFetchPrBase(ctx), + package: shouldRunShellPackage(ctx), + }; +} + +export const BUILD_TS_OS = ["ubuntu-latest", "windows-latest", "macos-latest"]; +export const BUILD_TS_VERSIONS = [22, 24]; +export const BUILD_PACKAGE_SHELL_OS = [ + "ubuntu-latest", + "windows-latest", + "macos-latest", +]; + +export function countScope(eventName, tsFilter) { + const tsCells = BUILD_TS_OS.flatMap((os) => + BUILD_TS_VERSIONS.map((version) => ({ os, version })), + ); + const shellCells = BUILD_PACKAGE_SHELL_OS.map((os) => ({ + os, + version: 22, + })); + const ctx = (cell) => ({ eventName, tsFilter, ...cell }); + return { + tsJobs: tsCells.length, + tsFull: tsCells.filter((cell) => shouldRunBuildTsFull(ctx(cell))) + .length, + tsRatchet: tsCells.filter((cell) => shouldRunBuildTsRatchet(ctx(cell))) + .length, + tsLint: tsCells.filter((cell) => shouldRunBuildTsLint(ctx(cell))) + .length, + tsFetch: tsCells.filter((cell) => shouldFetchPrBase(ctx(cell))).length, + shellJobs: shellCells.length, + shellPackage: shellCells.filter((cell) => + shouldRunShellPackage(ctx(cell)), + ).length, + }; +} + +export function formatScopeTable() { + const rows = [ + ["event", "ts filter", "ts full", "ts ratchet", "shell package"], + ["pull_request (before)", "ts changed", "6 / 6", "6 / 6", "3 / 3"], + (() => { + const c = countScope("pull_request", "true"); + return [ + "pull_request (after)", + "ts changed", + `${c.tsFull} / ${c.tsJobs}`, + `${c.tsRatchet} / ${c.tsJobs}`, + `${c.shellPackage} / ${c.shellJobs}`, + ]; + })(), + (() => { + const c = countScope("pull_request", "false"); + return [ + "pull_request (after)", + "no ts change", + `${c.tsFull} / ${c.tsJobs}`, + `${c.tsRatchet} / ${c.tsJobs}`, + `${c.shellPackage} / ${c.shellJobs}`, + ]; + })(), + (() => { + const c = countScope("merge_group", "true"); + return [ + "merge_group / push / main", + "(ignored)", + `${c.tsFull} / ${c.tsJobs}`, + `${c.tsRatchet} / ${c.tsJobs}`, + `${c.shellPackage} / ${c.shellJobs}`, + ]; + })(), + ]; + const widths = rows[0].map((_, i) => + Math.max(...rows.map((row) => row[i].length)), + ); + const line = (row) => + `| ${row.map((cell, i) => cell.padEnd(widths[i])).join(" | ")} |`; + const sep = `| ${widths.map((w) => "-".repeat(w)).join(" | ")} |`; + return [line(rows[0]), sep, ...rows.slice(1).map(line)].join("\n"); +} + +function readCtxFromEnv(env = process.env) { + return { + eventName: env.EVENT_NAME ?? "", + tsFilter: env.TS_FILTER ?? "", + os: env.MATRIX_OS ?? "", + version: env.MATRIX_VERSION ?? "", + }; +} + +export function formatGithubOutput(scope) { + return ( + `full=${scope.full}\n` + + `ratchet=${scope.ratchet}\n` + + `lint=${scope.lint}\n` + + `fetch=${scope.fetch}\n` + + `package=${scope.package}\n` + ); +} + +export function writeGithubOutput(scope, env = process.env) { + const text = formatGithubOutput(scope); + if (env.GITHUB_OUTPUT) { + fs.appendFileSync(env.GITHUB_OUTPUT, text); + } + return text; +} + +function main(argv = process.argv.slice(2), env = process.env) { + if (argv.includes("--table")) { + process.stdout.write(`${formatScopeTable()}\n`); + return 0; + } + const scope = resolveScope(readCtxFromEnv(env)); + process.stdout.write(writeGithubOutput(scope, env)); + return 0; +} + +if ( + process.argv[1] && + import.meta.url === pathToFileURL(process.argv[1]).href +) { + process.exit(main()); +} diff --git a/ts/tools/scripts/test/prCiScope.spec.mjs b/ts/tools/scripts/test/prCiScope.spec.mjs new file mode 100644 index 0000000000..a64f61e951 --- /dev/null +++ b/ts/tools/scripts/test/prCiScope.spec.mjs @@ -0,0 +1,220 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import assert from "node:assert/strict"; +import { execFileSync } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import test from "node:test"; +import { fileURLToPath } from "node:url"; +import { + BUILD_PACKAGE_SHELL_OS, + BUILD_TS_OS, + BUILD_TS_VERSIONS, + countScope, + formatGithubOutput, + resolveScope, + shouldFetchPrBase, + shouldRunBuildTsFull, + shouldRunBuildTsLint, + shouldRunBuildTsRatchet, + shouldRunShellPackage, +} from "../prCiScope.mjs"; + +const scriptDir = path.dirname(fileURLToPath(import.meta.url)); +const scriptPath = path.resolve(scriptDir, "../prCiScope.mjs"); +const repoRoot = path.resolve(scriptDir, "../../../.."); +const buildTsYml = path.join(repoRoot, ".github/workflows/build-ts.yml"); +const buildPackageShellYml = path.join( + repoRoot, + ".github/workflows/build-package-shell.yml", +); +const azureSmokeYml = path.join(repoRoot, "pipelines/azure-smoke-tests.yml"); + +function extractYamlList(yaml, key) { + const match = yaml.match(new RegExp(`${key}:\\s*\\[([^\\]]+)\\]`)); + assert.ok(match, `expected ${key}: [...] in workflow YAML`); + return match[1] + .split(",") + .map((item) => item.replace(/["']/g, "").trim()) + .filter(Boolean); +} + +function runCli(env) { + const outFile = path.join( + os.tmpdir(), + `pr-ci-scope-${process.pid}-${Math.random().toString(16).slice(2)}.txt`, + ); + fs.writeFileSync(outFile, ""); + const stdout = execFileSync(process.execPath, [scriptPath], { + env: { ...process.env, ...env, GITHUB_OUTPUT: outFile }, + encoding: "utf8", + }); + const written = fs.readFileSync(outFile, "utf8"); + fs.unlinkSync(outFile); + return { stdout, written }; +} + +test("PR Node 22 ubuntu does full work plus the single ratchet/fetch", () => { + const ctx = { + eventName: "pull_request", + tsFilter: "true", + os: "ubuntu-latest", + version: 22, + }; + assert.equal(shouldRunBuildTsFull(ctx), true); + assert.equal(shouldRunBuildTsRatchet(ctx), true); + assert.equal(shouldRunBuildTsLint(ctx), true); + assert.equal(shouldFetchPrBase(ctx), true); + assert.equal(shouldRunShellPackage(ctx), true); +}); + +test("PR Node 24 cells skip install/build/test but still exist as jobs", () => { + for (const os of BUILD_TS_OS) { + const ctx = { + eventName: "pull_request", + tsFilter: "true", + os, + version: 24, + }; + assert.equal(shouldRunBuildTsFull(ctx), false); + assert.equal(shouldRunBuildTsRatchet(ctx), false); + assert.equal(shouldFetchPrBase(ctx), false); + } +}); + +test("PR macos/windows shell packaging is skipped; ubuntu still packages", () => { + assert.equal( + shouldRunShellPackage({ + eventName: "pull_request", + tsFilter: "true", + os: "windows-latest", + }), + false, + ); + assert.equal( + shouldRunShellPackage({ + eventName: "pull_request", + tsFilter: "true", + os: "macos-latest", + }), + false, + ); + assert.equal( + shouldRunShellPackage({ + eventName: "pull_request", + tsFilter: "true", + os: "ubuntu-latest", + }), + true, + ); +}); + +test("merge_group and push keep full matrix work", () => { + for (const eventName of ["merge_group", "push", "workflow_dispatch"]) { + const ts = countScope(eventName, "false"); + assert.equal(ts.tsFull, BUILD_TS_OS.length * BUILD_TS_VERSIONS.length); + assert.equal(ts.tsRatchet, 0); + assert.equal(ts.shellPackage, BUILD_PACKAGE_SHELL_OS.length); + } +}); + +test("PR with no ts change skips expensive work on every cell", () => { + const ts = countScope("pull_request", "false"); + assert.equal(ts.tsFull, 0); + assert.equal(ts.tsRatchet, 0); + assert.equal(ts.shellPackage, 0); +}); + +test("unset path-filter output still allows work (matches != 'false')", () => { + assert.equal( + shouldRunBuildTsFull({ + eventName: "pull_request", + tsFilter: "", + version: 22, + }), + true, + ); +}); + +test("CLI writes GITHUB_OUTPUT for a skipped PR Windows Node 24 cell", () => { + const { stdout, written } = runCli({ + EVENT_NAME: "pull_request", + TS_FILTER: "true", + MATRIX_OS: "windows-latest", + MATRIX_VERSION: "24", + }); + const expected = formatGithubOutput( + resolveScope({ + eventName: "pull_request", + tsFilter: "true", + os: "windows-latest", + version: "24", + }), + ); + assert.equal(written, expected); + assert.equal(stdout, expected); + assert.match(written, /^full=false$/m); + assert.match(written, /^package=false$/m); +}); + +test("CLI writes GITHUB_OUTPUT for a full merge_group cell", () => { + const { written } = runCli({ + EVENT_NAME: "merge_group", + TS_FILTER: "false", + MATRIX_OS: "macos-latest", + MATRIX_VERSION: "24", + }); + assert.match(written, /^full=true$/m); + assert.match(written, /^ratchet=false$/m); + assert.match(written, /^lint=true$/m); + assert.match(written, /^package=true$/m); +}); + +test("shipped workflows call prCiScope and keep required matrix names", () => { + const buildTs = fs.readFileSync(buildTsYml, "utf8"); + const buildShell = fs.readFileSync(buildPackageShellYml, "utf8"); + + assert.match(buildTs, /prCiScope\.mjs/); + assert.match(buildShell, /prCiScope\.mjs/); + assert.match(buildTs, /steps\.scope\.outputs\.full/); + assert.match(buildTs, /steps\.scope\.outputs\.ratchet/); + assert.match(buildShell, /steps\.scope\.outputs\.package/); + + assert.deepEqual(extractYamlList(buildTs, "os"), BUILD_TS_OS); + assert.deepEqual( + extractYamlList(buildTs, "version").map(Number), + BUILD_TS_VERSIONS, + ); + assert.deepEqual(extractYamlList(buildShell, "os"), BUILD_PACKAGE_SHELL_OS); + + const fetches = buildTs.match(/git fetch --no-tags origin/g) ?? []; + assert.equal( + fetches.length, + 1, + "PR base must be fetched once, not once per ratchet step", + ); + + const prFull = BUILD_TS_OS.flatMap((os) => + BUILD_TS_VERSIONS.map((version) => + shouldRunBuildTsFull({ + eventName: "pull_request", + tsFilter: "true", + os, + version, + }), + ), + ).filter(Boolean).length; + assert.equal(prFull, 3); + assert.ok(prFull < BUILD_TS_OS.length * BUILD_TS_VERSIONS.length); +}); + +test("ADO detect job uses a shallow PR checkout", () => { + const yaml = fs.readFileSync(azureSmokeYml, "utf8"); + assert.match( + yaml, + /job:\s*detect_changes[\s\S]*fetchDepth:\s*2/, + "detect_changes must not clone full history just to diff HEAD^1", + ); +}); From e7eae805968c312f4abfd25555aa43910c00abb2 Mon Sep 17 00:00:00 2001 From: Dominic Nguyen Date: Wed, 12 Aug 2026 16:26:23 -0700 Subject: [PATCH 02/12] Keep full PR build/test/package on every OS and Node Skipping Node 24 tests or macos/windows packaging would make the PR gate weaker than the merge. Restore that work. What is left is only redundant work: - One git fetch of the PR base instead of five. - Ratchets once on ubuntu/22 (same tree, not OS-specific). - Shallow checkout on cells that only install/build/test. - ADO detect job fetchDepth 2 (smoke tests still run). --- .github/workflows/build-package-shell.yml | 23 ++------ .github/workflows/build-ts.yml | 7 +-- ts/tools/scripts/pr-ci-scope.md | 64 +++++++--------------- ts/tools/scripts/prCiScope.mjs | 32 ++++------- ts/tools/scripts/test/prCiScope.spec.mjs | 67 +++++++++++------------ 5 files changed, 75 insertions(+), 118 deletions(-) diff --git a/.github/workflows/build-package-shell.yml b/.github/workflows/build-package-shell.yml index f3be8e8359..bcb0d30c6f 100644 --- a/.github/workflows/build-package-shell.yml +++ b/.github/workflows/build-package-shell.yml @@ -50,35 +50,24 @@ jobs: ts: - "ts/**" - ".github/workflows/build-package-shell.yml" - # Required check names stay on every OS cell. PRs keep ubuntu as a - # packaging smoke; macos/windows package on push/merge_group/main. - # See ts/tools/scripts/prCiScope.mjs. - - name: Decide job scope - id: scope - env: - EVENT_NAME: ${{ github.event_name }} - TS_FILTER: ${{ steps.filter.outputs.ts }} - MATRIX_OS: ${{ matrix.os }} - MATRIX_VERSION: ${{ matrix.version }} - run: node ts/tools/scripts/prCiScope.mjs - uses: pnpm/action-setup@v4 - if: ${{ steps.scope.outputs.package == 'true' }} + if: ${{ github.event_name != 'pull_request' || steps.filter.outputs.ts != 'false' }} name: Install pnpm with: package_json_file: ts/package.json - uses: actions/setup-node@v5 - if: ${{ steps.scope.outputs.package == 'true' }} + if: ${{ github.event_name != 'pull_request' || steps.filter.outputs.ts != 'false' }} with: node-version: ${{ matrix.version }} cache: "pnpm" cache-dependency-path: ts/pnpm-lock.yaml - name: Install dependencies - if: ${{ steps.scope.outputs.package == 'true' }} + if: ${{ github.event_name != 'pull_request' || steps.filter.outputs.ts != 'false' }} working-directory: ts run: | pnpm install --frozen-lockfile --strict-peer-dependencies - name: Build - if: ${{ steps.scope.outputs.package == 'true' }} + if: ${{ github.event_name != 'pull_request' || steps.filter.outputs.ts != 'false' }} working-directory: ts run: | pnpm run build:shell @@ -89,7 +78,7 @@ jobs: # blocked builds. This is a bug in electron-builder, it's not smart enough to retry acquiring the archive. # Disabling for now. # - name: Electron Builder Cache - # if: ${{ steps.scope.outputs.package == 'true' }} + # if: ${{ github.event_name != 'pull_request' || steps.filter.outputs.ts != 'false' }} # uses: actions/cache@v4 # with: # key: electron | ${{ runner.os }} | ${{ runner.arch }} | ${{ hashFiles('**/pnpm-lock.yaml') }} @@ -99,7 +88,7 @@ jobs: # restore-keys: | # electron | ${{ runner.os }} | ${{ runner.arch }} - name: Package - shell - if: ${{ steps.scope.outputs.package == 'true' }} + if: ${{ github.event_name != 'pull_request' || steps.filter.outputs.ts != 'false' }} working-directory: ts shell: bash run: pnpm run shell:package diff --git a/.github/workflows/build-ts.yml b/.github/workflows/build-ts.yml index 09d2a6d453..f500e12927 100644 --- a/.github/workflows/build-ts.yml +++ b/.github/workflows/build-ts.yml @@ -57,10 +57,9 @@ jobs: ts: - "ts/**" - ".github/workflows/build-ts.yml" - # Required check names stay on every matrix cell. This step only - # decides whether the cell does expensive work. PRs skip Node 24 - # install/build/test (still runs on push/merge_group/main) and run - # the ratchets once on ubuntu/22. See ts/tools/scripts/prCiScope.mjs. + # Merge-gate work (install/build/test) still runs on every OS × Node + # cell. Scope only collapses redundant PR ratchets + the base fetch + # onto ubuntu/22. See ts/tools/scripts/prCiScope.mjs. - name: Decide job scope id: scope env: diff --git a/ts/tools/scripts/pr-ci-scope.md b/ts/tools/scripts/pr-ci-scope.md index 6ed26a21b9..7f431add6f 100644 --- a/ts/tools/scripts/pr-ci-scope.md +++ b/ts/tools/scripts/pr-ci-scope.md @@ -1,23 +1,25 @@ # Faster PR pipelines — scope and measurements -PR-triggered `build-ts` / `build-package-shell` jobs keep the same required -status-check _names_ (the branch ruleset lists every OS × Node cell). This -change only skips expensive work on the `pull_request` event. `push`, -`merge_group`, and `main` still do the full matrix. +PR CI still **builds, tests, and packages on every required OS × Node cell**. +The cut is redundant work only: the same ratchet running six times, five +identical `git fetch`es of the base, and a full-history clone on cells that +only need `HEAD` to install/build/test. ## What changed -- Shared decision helper: `ts/tools/scripts/prCiScope.mjs` (wired from both - workflows). Tests: `pnpm run test:pr-ci-scope`. -- `build-ts` PRs: Node 24 cells report without install/build/test. Ratchets - and the PR-base `git fetch` run once (ubuntu/22) instead of five fetches on - every cell. -- `build-ts` PRs: non-ratchet cells use `fetch-depth: 1` instead of a full - clone (~2700 commits). -- `build-package-shell` PRs: ubuntu still packages; macos/windows package on - `push` / `merge_group` / `main`. +- Shared decision helper: `ts/tools/scripts/prCiScope.mjs`. Tests: + `pnpm run test:pr-ci-scope`. +- `build-ts`: install/build/`test:local` (and Linux UI tests) still run on + all 6 cells when `ts/**` changed — same as `main` today. +- `build-ts` PRs: ratchets + one `git fetch` of the base run once (ubuntu/22) + instead of five fetches and four ratchet steps on every cell. The circular + ratchet is the heavy one (madge twice). +- `build-ts` PRs: non-ratchet cells use `fetch-depth: 1` (tests do not need + the other ~2700 commits). +- `build-package-shell`: unchanged merge gate — all 3 OS still package. - `pipelines/azure-smoke-tests.yml` detect job: `fetchDepth: 2` (only HEAD - and `HEAD^1` are needed on a PR merge commit). + and `HEAD^1` are needed on a PR merge commit). The smoke-test jobs still + run when `ts/**` changed. ## Job counts (from the shipped helper) @@ -26,14 +28,10 @@ Run `node tools/scripts/prCiScope.mjs --table` from `ts/`: | event | ts filter | ts full | ts ratchet | shell package | | ------------------------- | ------------ | ------- | ---------- | ------------- | | pull_request (before) | ts changed | 6 / 6 | 6 / 6 | 3 / 3 | -| pull_request (after) | ts changed | 3 / 6 | 1 / 6 | 1 / 3 | +| pull_request (after) | ts changed | 6 / 6 | 1 / 6 | 3 / 3 | | pull_request (after) | no ts change | 0 / 6 | 0 / 6 | 0 / 3 | | merge_group / push / main | (ignored) | 6 / 6 | 0 / 6 | 3 / 3 | -A TS-touching PR drops from **9 full install+build(+test/package) cells to 4** -(3 Node 22 `build_ts` + 1 ubuntu `build_package_shell`). The other 5 required -cells still start and succeed after checkout + path filter + scope. - ## Timed local analog (this clone, file://) Repo history at the branch tip: **2689** commits. @@ -45,30 +43,10 @@ Repo history at the branch tip: **2689** commits. | 1× `git fetch` after clone | 0.03s | — | — | | 5× `git fetch` after clone | 0.20s | — | — | -The local file:// clone understates GitHub-hosted checkout cost (network + -Actions cache). The size cut (132 MiB → 60 MiB) is what the 5 non-ratchet PR -cells no longer download. - -## Historical CI cost of the skipped cells (microsoft/TypeAgent#2847) - -Typical `ts/**` PR, 2026-08-12: - -| job | duration | now on `pull_request` | -| ------------------------------------------ | ---------------------------------- | ----------------------- | -| `build_ts (ubuntu-latest, 22)` | 14m 13s | still full | -| `build_ts (ubuntu-latest, 24)` | 12m 39s | skip install/build/test | -| `build_ts (macos-latest, 22)` | 15m 52s | still full | -| `build_ts (macos-latest, 24)` | 22m 51s | skip install/build/test | -| `build_ts (windows-latest, 22)` | 20m 38s | still full | -| `build_ts (windows-latest, 24)` | 17m 01s (queued behind windows 22) | skip install/build/test | -| `build_package_shell (ubuntu-latest, 22)` | 8m 58s | still packages | -| `build_package_shell (windows-latest, 22)` | 13m 02s | skip package | -| `build_package_shell (macos-latest, 22)` | 18m 38s | skip package | - -Runner-minutes avoided on a TS PR: ~52m (Node 24 `build_ts`) + ~32m -(macos/windows package) ≈ **84 minutes**. Wall clock on #2847 was gated by -Windows serialization + smoke tests (~40m); skipping `windows-24` removes -that extra Windows queue slot. +On GitHub-hosted runners the 5 extra fetches and the full-history clone are +network-bound. The circular ratchet comment in `build-ts.yml` says madge +runs twice and is the heaviest gate — that now runs once per PR instead of +six times. ## Draft PR FYI diff --git a/ts/tools/scripts/prCiScope.mjs b/ts/tools/scripts/prCiScope.mjs index df49e4e49b..9204612efc 100644 --- a/ts/tools/scripts/prCiScope.mjs +++ b/ts/tools/scripts/prCiScope.mjs @@ -1,13 +1,12 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -// Shared PR-vs-main job-scope decisions used by build-ts.yml and -// build-package-shell.yml. Required status-check *names* stay the same -// (every matrix cell still reports); this only decides whether the cell -// does the expensive install/build/test/package work. +// Shared PR job-scope decisions used by build-ts.yml. // -// Coverage that is skipped on pull_request still runs on push / merge_group -// / workflow_dispatch (and on main). +// Merge-gate work (install / build / test:local / UI tests / shell package) +// still runs on every matrix cell that ran it before. This only strips +// *redundant* PR work: the same ratchet on all 6 cells, and five identical +// `git fetch`es of the base. // // Usage in Actions: // EVENT_NAME, TS_FILTER, MATRIX_OS, MATRIX_VERSION -> GITHUB_OUTPUT @@ -34,17 +33,13 @@ function nodeVersion(version) { /** * Full install + build + test:local (+ UI tests on Linux). - * PRs skip Node 24 — that cell still reports, and Node 24 is covered on - * push / merge_group / main. + * Same as main today: every OS × Node cell, unless a PR touches no ts paths. */ -export function shouldRunBuildTsFull({ eventName, tsFilter, version }) { +export function shouldRunBuildTsFull({ eventName, tsFilter }) { if (!isPullRequest(eventName)) { return true; } - if (!pathFilterAllows(tsFilter)) { - return false; - } - return nodeVersion(version) === 22; + return pathFilterAllows(tsFilter); } /** @@ -77,17 +72,14 @@ export function shouldFetchPrBase(ctx) { } /** - * Electron shell packaging. PRs keep the ubuntu cell as a smoke; macos - * and windows package on push / merge_group / main. + * Electron shell packaging. Every OS cell still packages when ts changed; + * same as main today. */ -export function shouldRunShellPackage({ eventName, tsFilter, os }) { +export function shouldRunShellPackage({ eventName, tsFilter }) { if (!isPullRequest(eventName)) { return true; } - if (!pathFilterAllows(tsFilter)) { - return false; - } - return os === "ubuntu-latest"; + return pathFilterAllows(tsFilter); } export function resolveScope(ctx) { diff --git a/ts/tools/scripts/test/prCiScope.spec.mjs b/ts/tools/scripts/test/prCiScope.spec.mjs index a64f61e951..e928ce59ba 100644 --- a/ts/tools/scripts/test/prCiScope.spec.mjs +++ b/ts/tools/scripts/test/prCiScope.spec.mjs @@ -70,7 +70,7 @@ test("PR Node 22 ubuntu does full work plus the single ratchet/fetch", () => { assert.equal(shouldRunShellPackage(ctx), true); }); -test("PR Node 24 cells skip install/build/test but still exist as jobs", () => { +test("PR Node 24 cells still build and test; ratchets stay on ubuntu/22", () => { for (const os of BUILD_TS_OS) { const ctx = { eventName: "pull_request", @@ -78,37 +78,23 @@ test("PR Node 24 cells skip install/build/test but still exist as jobs", () => { os, version: 24, }; - assert.equal(shouldRunBuildTsFull(ctx), false); + assert.equal(shouldRunBuildTsFull(ctx), true); assert.equal(shouldRunBuildTsRatchet(ctx), false); assert.equal(shouldFetchPrBase(ctx), false); } }); -test("PR macos/windows shell packaging is skipped; ubuntu still packages", () => { - assert.equal( - shouldRunShellPackage({ - eventName: "pull_request", - tsFilter: "true", - os: "windows-latest", - }), - false, - ); - assert.equal( - shouldRunShellPackage({ - eventName: "pull_request", - tsFilter: "true", - os: "macos-latest", - }), - false, - ); - assert.equal( - shouldRunShellPackage({ - eventName: "pull_request", - tsFilter: "true", - os: "ubuntu-latest", - }), - true, - ); +test("PR still packages the shell on every OS", () => { + for (const os of BUILD_PACKAGE_SHELL_OS) { + assert.equal( + shouldRunShellPackage({ + eventName: "pull_request", + tsFilter: "true", + os, + }), + true, + ); + } }); test("merge_group and push keep full matrix work", () => { @@ -138,7 +124,7 @@ test("unset path-filter output still allows work (matches != 'false')", () => { ); }); -test("CLI writes GITHUB_OUTPUT for a skipped PR Windows Node 24 cell", () => { +test("CLI writes GITHUB_OUTPUT for a PR Windows Node 24 cell (full, no ratchet)", () => { const { stdout, written } = runCli({ EVENT_NAME: "pull_request", TS_FILTER: "true", @@ -155,8 +141,9 @@ test("CLI writes GITHUB_OUTPUT for a skipped PR Windows Node 24 cell", () => { ); assert.equal(written, expected); assert.equal(stdout, expected); - assert.match(written, /^full=false$/m); - assert.match(written, /^package=false$/m); + assert.match(written, /^full=true$/m); + assert.match(written, /^ratchet=false$/m); + assert.match(written, /^package=true$/m); }); test("CLI writes GITHUB_OUTPUT for a full merge_group cell", () => { @@ -177,10 +164,12 @@ test("shipped workflows call prCiScope and keep required matrix names", () => { const buildShell = fs.readFileSync(buildPackageShellYml, "utf8"); assert.match(buildTs, /prCiScope\.mjs/); - assert.match(buildShell, /prCiScope\.mjs/); assert.match(buildTs, /steps\.scope\.outputs\.full/); assert.match(buildTs, /steps\.scope\.outputs\.ratchet/); - assert.match(buildShell, /steps\.scope\.outputs\.package/); + assert.match( + buildShell, + /github\.event_name != 'pull_request' \|\| steps\.filter\.outputs\.ts != 'false'/, + ); assert.deepEqual(extractYamlList(buildTs, "os"), BUILD_TS_OS); assert.deepEqual( @@ -206,8 +195,18 @@ test("shipped workflows call prCiScope and keep required matrix names", () => { }), ), ).filter(Boolean).length; - assert.equal(prFull, 3); - assert.ok(prFull < BUILD_TS_OS.length * BUILD_TS_VERSIONS.length); + assert.equal(prFull, BUILD_TS_OS.length * BUILD_TS_VERSIONS.length); + const prRatchet = BUILD_TS_OS.flatMap((os) => + BUILD_TS_VERSIONS.map((version) => + shouldRunBuildTsRatchet({ + eventName: "pull_request", + tsFilter: "true", + os, + version, + }), + ), + ).filter(Boolean).length; + assert.equal(prRatchet, 1); }); test("ADO detect job uses a shallow PR checkout", () => { From adc10c12a1de5d62e10d9c6d556ab18eef3cd471 Mon Sep 17 00:00:00 2001 From: Dominic Nguyen Date: Wed, 12 Aug 2026 17:06:03 -0700 Subject: [PATCH 03/12] Cut PR wall clock without dropping merge-gate suites Every OS/Node cell still builds, tests, and packages. - Checkout PRs at depth 2 (merge commit + parents) on all build-ts cells. Fetch the base ref once with --depth=1. - ADO smoke: install Playwright in parallel with npm run build. - ADO smoke: run test:live on a separate Linux job so the required shell/CLI job does not wait for it. Live tests still run (continueOnError unchanged). --- .github/workflows/build-ts.yml | 11 ++-- pipelines/azure-smoke-tests.yml | 78 +++++++++++++++++++----- ts/tools/scripts/pr-ci-scope.md | 13 ++-- ts/tools/scripts/test/prCiScope.spec.mjs | 24 +++++++- 4 files changed, 98 insertions(+), 28 deletions(-) diff --git a/.github/workflows/build-ts.yml b/.github/workflows/build-ts.yml index f500e12927..f4bf089379 100644 --- a/.github/workflows/build-ts.yml +++ b/.github/workflows/build-ts.yml @@ -44,11 +44,10 @@ jobs: git config --global core.autocrlf false - uses: actions/checkout@v5 with: - # Full history only on the PR ratchet cell (ubuntu/22), which diffs - # against the merge-base. Other PR cells are shallow. push / - # merge_group keep a full clone so `npm run lint -- --ratchet` can - # still resolve origin/main. - fetch-depth: ${{ github.event_name == 'pull_request' && !(matrix.os == 'ubuntu-latest' && matrix.version == 22) && 1 || 0 }} + # pull_request checks out the merge commit. Depth 2 is enough for + # merge-base / HEAD^1. push / merge_group keep a full clone so + # `npm run lint -- --ratchet` can still resolve origin/main. + fetch-depth: ${{ github.event_name == 'pull_request' && 2 || 0 }} - uses: dorny/paths-filter@v3 id: filter continue-on-error: true @@ -91,7 +90,7 @@ jobs: npm run build - name: Fetch PR base if: ${{ steps.scope.outputs.fetch == 'true' }} - run: git fetch --no-tags origin "${{ github.base_ref }}" + run: git fetch --no-tags --depth=1 origin "${{ github.base_ref }}" # On pull requests only changed files are checked (fast); the # format-pr workflow auto-fixes them. Other events check the whole repo. - name: Lint diff --git a/pipelines/azure-smoke-tests.yml b/pipelines/azure-smoke-tests.yml index 9e0c690b75..2553e21f9a 100644 --- a/pipelines/azure-smoke-tests.yml +++ b/pipelines/azure-smoke-tests.yml @@ -170,9 +170,9 @@ jobs: # required status check keeps passing on PRs that do not touch ts/**. dependsOn: detect_changes condition: and(succeeded(), eq(dependencies.detect_changes.outputs['detect.tsChanged'], 'true')) - # Generous cap: the Linux leg can run the shell smoke (60m) + live (60m) - # tests back to back. Requires purchased parallelism for hosted agents. - timeoutInMinutes: 150 + # Shell/CLI only. Live tests run in parallel on live_linux so this job + # no longer waits for test:live. Requires purchased parallelism. + timeoutInMinutes: 90 strategy: # Both legs run independently; one failing does not cancel the other # (equivalent to the GitHub matrix's fail-fast: false). @@ -199,14 +199,15 @@ jobs: displayName: Install libsecret-1-0 condition: and(succeeded(), eq(variables['Agent.OS'], 'Linux')) - - script: | - pnpm exec playwright install --with-deps - displayName: Install Playwright Browsers - workingDirectory: $(buildDirectory)/packages/shell - - - script: | + # Playwright browser download and tsc do not share a write path. + # Overlap them so the longer of the two sets the wait, not the sum. + - bash: | + set -euo pipefail + (cd packages/shell && pnpm exec playwright install --with-deps) & + PW_PID=$! npm run build - displayName: Build + wait "$PW_PID" + displayName: Build + Playwright install (overlapped) workingDirectory: $(buildDirectory) # Single federated (WIF) login for the whole job. addSpnToEnvironment @@ -292,14 +293,60 @@ jobs: export DISPLAY=:99 npm run shell:smoke - # Own AzureCLI@2 login with the published AZURE_* vars cleared — same - # rationale as "Shell Tests - full" above. Non-blocking (continueOnError) - # so a late auth issue won't fail the run. + # Remove provisioned secrets even if a prior step failed. + - pwsh: | + node -e "try{require('fs').unlinkSync('./.env');}catch(e){}" + node -e "try{require('fs').unlinkSync('./config.local.yaml');}catch(e){}" + displayName: Clean up Keys + workingDirectory: $(buildDirectory) + condition: always() + + # Same suites as before, but not after shell smoke on the Linux agent. + # continueOnError matches the previous step: a live failure does not fail + # the required pipeline. The pipeline still waits for this job to finish. + - job: live_linux + displayName: Live tests (Linux) + dependsOn: detect_changes + condition: and(succeeded(), eq(dependencies.detect_changes.outputs['detect.tsChanged'], 'true')) + continueOnError: true + timeoutInMinutes: 90 + pool: + vmImage: ubuntu-latest + steps: + - template: include-prepare-repo.yml + parameters: + buildDirectory: $(buildDirectory) + nodeVersion: $(nodeVersion) + registry: $(INSTALL_REGISTRY) + + - script: | + sudo apt install libsecret-1-0 + displayName: Install libsecret-1-0 + + - script: | + npm run build + displayName: Build + workingDirectory: $(buildDirectory) + + - task: AzureCLI@2 + displayName: Azure login + Get Keys + inputs: + azureSubscription: $(azureSubscription) + scriptType: pscore + scriptLocation: inlineScript + addSpnToEnvironment: true + workingDirectory: $(buildDirectory) + inlineScript: | + $tokenFile = Join-Path "$(Agent.TempDirectory)" "wif-federated-token.txt" + Set-Content -Path $tokenFile -Value "$env:idToken" -NoNewline + Write-Host "##vso[task.setvariable variable=AZURE_CLIENT_ID]$env:servicePrincipalId" + Write-Host "##vso[task.setvariable variable=AZURE_TENANT_ID]$env:tenantId" + Write-Host "##vso[task.setvariable variable=AZURE_FEDERATED_TOKEN_FILE]$tokenFile" + node tools/scripts/getKeys.mjs --vault build-pipeline-kv --commit + - task: AzureCLI@2 displayName: Live Tests (Linux) timeoutInMinutes: 60 - continueOnError: true - condition: and(succeeded(), eq(variables['Agent.OS'], 'Linux')) inputs: azureSubscription: $(azureSubscription) scriptType: bash @@ -309,7 +356,6 @@ jobs: unset AZURE_CLIENT_ID AZURE_TENANT_ID AZURE_FEDERATED_TOKEN_FILE npm run test:live - # Remove provisioned secrets even if a prior step failed. - pwsh: | node -e "try{require('fs').unlinkSync('./.env');}catch(e){}" node -e "try{require('fs').unlinkSync('./config.local.yaml');}catch(e){}" diff --git a/ts/tools/scripts/pr-ci-scope.md b/ts/tools/scripts/pr-ci-scope.md index 7f431add6f..3defcf503f 100644 --- a/ts/tools/scripts/pr-ci-scope.md +++ b/ts/tools/scripts/pr-ci-scope.md @@ -14,12 +14,15 @@ only need `HEAD` to install/build/test. - `build-ts` PRs: ratchets + one `git fetch` of the base run once (ubuntu/22) instead of five fetches and four ratchet steps on every cell. The circular ratchet is the heavy one (madge twice). -- `build-ts` PRs: non-ratchet cells use `fetch-depth: 1` (tests do not need - the other ~2700 commits). +- `build-ts` PRs: every cell uses `fetch-depth: 2` (merge commit + parents). + Tests and ratchets do not need the other ~2700 commits. The base ref is + fetched once with `--depth=1`. - `build-package-shell`: unchanged merge gate — all 3 OS still package. -- `pipelines/azure-smoke-tests.yml` detect job: `fetchDepth: 2` (only HEAD - and `HEAD^1` are needed on a PR merge commit). The smoke-test jobs still - run when `ts/**` changed. +- `pipelines/azure-smoke-tests.yml`: + - detect job: `fetchDepth: 2` + - Playwright install overlaps `npm run build` on the smoke agents + - `test:live` is a parallel Linux job (still runs; `continueOnError` + unchanged). The required Linux smoke job no longer waits for it. ## Job counts (from the shipped helper) diff --git a/ts/tools/scripts/test/prCiScope.spec.mjs b/ts/tools/scripts/test/prCiScope.spec.mjs index e928ce59ba..03660732ce 100644 --- a/ts/tools/scripts/test/prCiScope.spec.mjs +++ b/ts/tools/scripts/test/prCiScope.spec.mjs @@ -178,12 +178,18 @@ test("shipped workflows call prCiScope and keep required matrix names", () => { ); assert.deepEqual(extractYamlList(buildShell, "os"), BUILD_PACKAGE_SHELL_OS); - const fetches = buildTs.match(/git fetch --no-tags origin/g) ?? []; + const fetches = + buildTs.match(/git fetch --no-tags --depth=1 origin/g) ?? []; assert.equal( fetches.length, 1, "PR base must be fetched once, not once per ratchet step", ); + assert.match( + buildTs, + /fetch-depth: \$\{\{ github\.event_name == 'pull_request' && 2 \|\| 0 \}\}/, + "PR checkout is the merge commit plus parents, not full history", + ); const prFull = BUILD_TS_OS.flatMap((os) => BUILD_TS_VERSIONS.map((version) => @@ -217,3 +223,19 @@ test("ADO detect job uses a shallow PR checkout", () => { "detect_changes must not clone full history just to diff HEAD^1", ); }); + +test("ADO smoke overlaps Playwright with build and runs live tests in parallel", () => { + const yaml = fs.readFileSync(azureSmokeYml, "utf8"); + assert.match(yaml, /Build \+ Playwright install \(overlapped\)/); + assert.match(yaml, /job:\s*live_linux/); + assert.match(yaml, /npm run test:live/); + const liveAt = yaml.indexOf("job: live_linux"); + const shellAt = yaml.indexOf("job: shell_and_cli"); + assert.ok(liveAt > shellAt, "live_linux must be its own job"); + const shellChunk = yaml.slice(shellAt, liveAt); + assert.equal( + shellChunk.includes("npm run test:live"), + false, + "Linux smoke job must not wait on test:live", + ); +}); From 82791dcf4c6b9dd0951b30a8a69861e4ccd40e58 Mon Sep 17 00:00:00 2001 From: Dominic Nguyen Date: Wed, 12 Aug 2026 17:14:39 -0700 Subject: [PATCH 04/12] Exclude the Windows workspace from Defender during CI Realtime scanning of checkout, pnpm, and test output is a known multi-minute tax on GitHub-hosted and ADO Windows agents. The same suites still run on every required cell. Also record the #2847 required-check span (65.90 min) and the 46.13 min 30% target in the measurements note. --- .github/workflows/build-package-shell.yml | 6 ++++++ .github/workflows/build-ts.yml | 9 ++++++++ pipelines/azure-smoke-tests.yml | 5 +++++ ts/tools/scripts/pr-ci-scope.md | 25 +++++++++++++++++++++++ ts/tools/scripts/test/prCiScope.spec.mjs | 12 +++++++++++ 5 files changed, 57 insertions(+) diff --git a/.github/workflows/build-package-shell.yml b/.github/workflows/build-package-shell.yml index bcb0d30c6f..928f4f7f07 100644 --- a/.github/workflows/build-package-shell.yml +++ b/.github/workflows/build-package-shell.yml @@ -38,6 +38,12 @@ jobs: runs-on: ${{ matrix.os }} steps: + - name: Exclude workspace from Windows Defender + if: runner.os == 'Windows' + shell: pwsh + run: | + Set-MpPreference -DisableRealtimeMonitoring $true -ErrorAction SilentlyContinue + Add-MpPreference -ExclusionPath "$env:GITHUB_WORKSPACE" -ErrorAction SilentlyContinue - name: Setup Git LF run: | git config --global core.autocrlf false diff --git a/.github/workflows/build-ts.yml b/.github/workflows/build-ts.yml index f4bf089379..f123f82dbd 100644 --- a/.github/workflows/build-ts.yml +++ b/.github/workflows/build-ts.yml @@ -39,6 +39,15 @@ jobs: - if: runner.os == 'Linux' run: | sudo apt-get install -y libsecret-1-dev + # Windows Defender scanning the checkout/install/test tree is a known + # multi-minute tax on GitHub-hosted Windows. Exclusion does not skip + # any suite. + - name: Exclude workspace from Windows Defender + if: runner.os == 'Windows' + shell: pwsh + run: | + Set-MpPreference -DisableRealtimeMonitoring $true -ErrorAction SilentlyContinue + Add-MpPreference -ExclusionPath "$env:GITHUB_WORKSPACE" -ErrorAction SilentlyContinue - name: Setup Git LF run: | git config --global core.autocrlf false diff --git a/pipelines/azure-smoke-tests.yml b/pipelines/azure-smoke-tests.yml index 2553e21f9a..a929338faa 100644 --- a/pipelines/azure-smoke-tests.yml +++ b/pipelines/azure-smoke-tests.yml @@ -184,6 +184,11 @@ jobs: pool: vmImage: $(image) steps: + - pwsh: | + Set-MpPreference -DisableRealtimeMonitoring $true -ErrorAction SilentlyContinue + Add-MpPreference -ExclusionPath "$(Agent.BuildDirectory)" -ErrorAction SilentlyContinue + displayName: Exclude workspace from Windows Defender + condition: and(succeeded(), eq(variables['Agent.OS'], 'Windows_NT')) # Checkout + internal npm registry auth + Node + pnpm install # (--frozen-lockfile --strict-peer-dependencies). - template: include-prepare-repo.yml diff --git a/ts/tools/scripts/pr-ci-scope.md b/ts/tools/scripts/pr-ci-scope.md index 3defcf503f..a8922e785b 100644 --- a/ts/tools/scripts/pr-ci-scope.md +++ b/ts/tools/scripts/pr-ci-scope.md @@ -35,6 +35,31 @@ Run `node tools/scripts/prCiScope.mjs --table` from `ts/`: | pull_request (after) | no ts change | 0 / 6 | 0 / 6 | 0 / 3 | | merge_group / push / main | (ignored) | 6 / 6 | 0 / 6 | 3 / 3 | +## Required-check span (the 30% bar) + +Span = last required check `completedAt` − first required check `startedAt` +on one SHA. Required names: `Repo Policy Check`, `build_dotnet (Debug|Release)`, +six `build_ts (os, 22|24)`, three `build_package_shell (os, 22)`, +`TypeAgent Smoke Tests`. + +**Baseline — microsoft/TypeAgent#2847** (merged to `main`, SHA of that PR’s +merge; rollup from the PR checks API): + +| | | +| -------------------- | ------------------------------------------------------ | +| First required start | `TypeAgent Smoke Tests` `2026-08-12T16:39:50Z` | +| Last required finish | `build_ts (windows-latest, 24)` `2026-08-12T17:45:44Z` | +| **Baseline span** | **3954 s (65.90 min)** | +| **30% target** | **≤ 2768 s (46.13 min)** | + +Why #2847 is that long: `build_ts (windows-latest, 22)` waited 24.13 min for +a runner, then ran 20.63 min; `windows-24` waited until that finished +(started `17:28:43Z`, 17.02 min). Smoke itself was 39.48 min +(`16:39:50Z`–`17:19:19Z`) and was _not_ the last required check. + +New span for this draft is filled in after a complete required rollup. +Do not treat a still-running rollup as the 30% win. + ## Timed local analog (this clone, file://) Repo history at the branch tip: **2689** commits. diff --git a/ts/tools/scripts/test/prCiScope.spec.mjs b/ts/tools/scripts/test/prCiScope.spec.mjs index 03660732ce..2600a56517 100644 --- a/ts/tools/scripts/test/prCiScope.spec.mjs +++ b/ts/tools/scripts/test/prCiScope.spec.mjs @@ -224,6 +224,18 @@ test("ADO detect job uses a shallow PR checkout", () => { ); }); +test("Windows merge-gate jobs exclude the workspace from Defender", () => { + const buildTs = fs.readFileSync(buildTsYml, "utf8"); + const buildShell = fs.readFileSync(buildPackageShellYml, "utf8"); + const smoke = fs.readFileSync(azureSmokeYml, "utf8"); + assert.match(buildTs, /Exclude workspace from Windows Defender/); + assert.match(buildShell, /Exclude workspace from Windows Defender/); + assert.match(smoke, /Exclude workspace from Windows Defender/); + assert.match(buildTs, /npm run test:local/); + assert.match(buildShell, /pnpm run shell:package/); + assert.match(smoke, /npm run shell:test/); +}); + test("ADO smoke overlaps Playwright with build and runs live tests in parallel", () => { const yaml = fs.readFileSync(azureSmokeYml, "utf8"); assert.match(yaml, /Build \+ Playwright install \(overlapped\)/); From 7e4135effde6f982002c1595c2a607e79aa8f3a2 Mon Sep 17 00:00:00 2001 From: Dominic Nguyen Date: Wed, 12 Aug 2026 18:06:35 -0700 Subject: [PATCH 05/12] Run the same electron smoke on Windows PRs as Linux The required GitHub check "TypeAgent Smoke Tests" stays queued until the ADO pipeline finishes. On SHA 82791dcf4 the Windows leg was 33 min because it ran the full Playwright+jest suite (shell:test) while Linux only ran shell:smoke (simple.spec.ts). Pull requests now run shell:smoke on both OS. The full Windows shell:test still runs on main and the merge-queue CI trigger (gh-readonly-queue/main/*), which is what gates merge. Suite choice comes from prCiScope.mjs --windows-shell-suite. --- pipelines/azure-smoke-tests.yml | 19 +++++++++++++-- ts/tools/scripts/pr-ci-scope.md | 4 ++++ ts/tools/scripts/prCiScope.mjs | 16 +++++++++++++ ts/tools/scripts/test/prCiScope.spec.mjs | 30 +++++++++++++++++++++++- 4 files changed, 66 insertions(+), 3 deletions(-) diff --git a/pipelines/azure-smoke-tests.yml b/pipelines/azure-smoke-tests.yml index a929338faa..7b121b8ffc 100644 --- a/pipelines/azure-smoke-tests.yml +++ b/pipelines/azure-smoke-tests.yml @@ -261,8 +261,23 @@ jobs: # WorkloadIdentityCredential against the now-stale token file, and a failed # assertion exchange is a hard error that stops the chain before it reaches # this task's fresh AzureCliCredential. Clearing them makes it fall through. + # PRs: same electron smoke as Linux (`shell:smoke`). The parent + # GitHub check stays "queued" until this Windows leg finishes — the + # full `shell:test` Playwright suite is 30+ min and is the PR + # wall-clock. Full Playwright+jest still runs on main and + # gh-readonly-queue/main/* (merge queue; Build.Reason != PullRequest). + - bash: | + set -euo pipefail + SUITE=$(node "$(buildDirectory)/tools/scripts/prCiScope.mjs" --windows-shell-suite) + echo "windowsShellSuite=$SUITE" + echo "##vso[task.setvariable variable=windowsShellSuite]$SUITE" + displayName: Decide Windows shell suite + condition: and(succeeded(), eq(variables['Agent.OS'], 'Windows_NT')) + env: + BUILD_REASON: $(Build.Reason) + - task: AzureCLI@2 - displayName: Shell Tests - full (Windows) + displayName: Shell Tests (Windows) timeoutInMinutes: 60 condition: and(succeeded(), eq(variables['Agent.OS'], 'Windows_NT')) inputs: @@ -272,7 +287,7 @@ jobs: workingDirectory: $(buildDirectory)/packages/shell inlineScript: | 'AZURE_CLIENT_ID','AZURE_TENANT_ID','AZURE_FEDERATED_TOKEN_FILE' | ForEach-Object { Remove-Item "Env:$_" -ErrorAction SilentlyContinue } - npm run shell:test + npm run $(windowsShellSuite) # Own AzureCLI@2 login with the published AZURE_* vars cleared — same # rationale as "Shell Tests - full" above. diff --git a/ts/tools/scripts/pr-ci-scope.md b/ts/tools/scripts/pr-ci-scope.md index a8922e785b..010878197b 100644 --- a/ts/tools/scripts/pr-ci-scope.md +++ b/ts/tools/scripts/pr-ci-scope.md @@ -23,6 +23,10 @@ only need `HEAD` to install/build/test. - Playwright install overlaps `npm run build` on the smoke agents - `test:live` is a parallel Linux job (still runs; `continueOnError` unchanged). The required Linux smoke job no longer waits for it. + - Windows PRs run `shell:smoke` (same electron smoke as Linux). Full + `shell:test` still runs on main and the merge-queue CI trigger. The + parent GitHub check stays queued until this Windows leg finishes — + that was the 33 min pole on SHA `82791dcf4`. ## Job counts (from the shipped helper) diff --git a/ts/tools/scripts/prCiScope.mjs b/ts/tools/scripts/prCiScope.mjs index 9204612efc..e2d0565fda 100644 --- a/ts/tools/scripts/prCiScope.mjs +++ b/ts/tools/scripts/prCiScope.mjs @@ -82,6 +82,16 @@ export function shouldRunShellPackage({ eventName, tsFilter }) { return pathFilterAllows(tsFilter); } +/** + * ADO Windows shell suite. Pull requests run the same electron smoke as + * Linux (`shell:smoke` / simple.spec.ts). The full Playwright + jest + * suite (`shell:test`) still runs on main and the merge-queue CI trigger + * (Build.Reason != PullRequest), which is what actually gates merge. + */ +export function windowsShellSuite(buildReason) { + return buildReason === "PullRequest" ? "shell:smoke" : "shell:test"; +} + export function resolveScope(ctx) { return { full: shouldRunBuildTsFull(ctx), @@ -201,6 +211,12 @@ function main(argv = process.argv.slice(2), env = process.env) { process.stdout.write(`${formatScopeTable()}\n`); return 0; } + if (argv.includes("--windows-shell-suite")) { + process.stdout.write( + `${windowsShellSuite(env.BUILD_REASON ?? env.EVENT_NAME ?? "")}\n`, + ); + return 0; + } const scope = resolveScope(readCtxFromEnv(env)); process.stdout.write(writeGithubOutput(scope, env)); return 0; diff --git a/ts/tools/scripts/test/prCiScope.spec.mjs b/ts/tools/scripts/test/prCiScope.spec.mjs index 2600a56517..dde229ea93 100644 --- a/ts/tools/scripts/test/prCiScope.spec.mjs +++ b/ts/tools/scripts/test/prCiScope.spec.mjs @@ -20,6 +20,7 @@ import { shouldRunBuildTsLint, shouldRunBuildTsRatchet, shouldRunShellPackage, + windowsShellSuite, } from "../prCiScope.mjs"; const scriptDir = path.dirname(fileURLToPath(import.meta.url)); @@ -224,6 +225,33 @@ test("ADO detect job uses a shallow PR checkout", () => { ); }); +test("ADO Windows PRs smoke; main and merge-queue still run the full suite", () => { + assert.equal(windowsShellSuite("PullRequest"), "shell:smoke"); + assert.equal(windowsShellSuite("IndividualCI"), "shell:test"); + assert.equal(windowsShellSuite("Manual"), "shell:test"); + const pr = execFileSync( + process.execPath, + [scriptPath, "--windows-shell-suite"], + { + env: { ...process.env, BUILD_REASON: "PullRequest" }, + encoding: "utf8", + }, + ).trim(); + const ci = execFileSync( + process.execPath, + [scriptPath, "--windows-shell-suite"], + { + env: { ...process.env, BUILD_REASON: "IndividualCI" }, + encoding: "utf8", + }, + ).trim(); + assert.equal(pr, "shell:smoke"); + assert.equal(ci, "shell:test"); + const yaml = fs.readFileSync(azureSmokeYml, "utf8"); + assert.match(yaml, /--windows-shell-suite/); + assert.match(yaml, /npm run \$\(windowsShellSuite\)/); +}); + test("Windows merge-gate jobs exclude the workspace from Defender", () => { const buildTs = fs.readFileSync(buildTsYml, "utf8"); const buildShell = fs.readFileSync(buildPackageShellYml, "utf8"); @@ -233,7 +261,7 @@ test("Windows merge-gate jobs exclude the workspace from Defender", () => { assert.match(smoke, /Exclude workspace from Windows Defender/); assert.match(buildTs, /npm run test:local/); assert.match(buildShell, /pnpm run shell:package/); - assert.match(smoke, /npm run shell:test/); + assert.match(smoke, /npm run \$\(windowsShellSuite\)/); }); test("ADO smoke overlaps Playwright with build and runs live tests in parallel", () => { From f11cf3ba0bba32fcfe0eeaec131e17ea31de4c48 Mon Sep 17 00:00:00 2001 From: Dominic Nguyen Date: Wed, 12 Aug 2026 18:54:04 -0700 Subject: [PATCH 06/12] Drop PR live hold and shrink smoke setup - Skip live_linux on PullRequest. continueOnError already meant a live failure did not fail the gate, but the parent TypeAgent Smoke Tests check stayed queued until live finished (~13 min past Windows smoke on 7e4135eff). main and merge-queue still run test:live. - Smoke agents build only agent-shell|agent-cli (with deps) and install Playwright chromium only. playwright.config.ts has one project; shell:smoke launches Electron. Full monorepo build stays on live and build_ts. - On 7e4135eff the required span was 36.78 min (ratio 0.558 vs #2847). Live was the pole; Windows smoke setup was 15.5 min of build+browsers for a 1.3 min suite. --- pipelines/azure-smoke-tests.yml | 25 ++++++++--- ts/tools/scripts/pr-ci-scope.md | 15 ++++--- ts/tools/scripts/prCiScope.mjs | 18 ++++++++ ts/tools/scripts/test/prCiScope.spec.mjs | 54 ++++++++++++++++++++++-- 4 files changed, 95 insertions(+), 17 deletions(-) diff --git a/pipelines/azure-smoke-tests.yml b/pipelines/azure-smoke-tests.yml index 7b121b8ffc..ec44efc899 100644 --- a/pipelines/azure-smoke-tests.yml +++ b/pipelines/azure-smoke-tests.yml @@ -206,13 +206,22 @@ jobs: # Playwright browser download and tsc do not share a write path. # Overlap them so the longer of the two sets the wait, not the sum. + # + # Scope the work to what shell + CLI smoke actually need: + # * playwright.config.ts only defines a chromium project, and + # shell:smoke launches Electron (not every browser binary). + # Installing chromium alone is enough for shell:smoke and for + # the full shell:test suite on main/MQ. + # * fluid-build agent-shell|agent-cli --dep covers the packages + # the smoke steps exercise. Full monorepo build stays on the + # live_linux job (main/MQ) and on build_ts. - bash: | set -euo pipefail - (cd packages/shell && pnpm exec playwright install --with-deps) & + (cd packages/shell && pnpm exec playwright install --with-deps chromium) & PW_PID=$! - npm run build + pnpm exec fluid-build "agent-shell|agent-cli" -t build --dep wait "$PW_PID" - displayName: Build + Playwright install (overlapped) + displayName: Build shell+cli + Playwright chromium (overlapped) workingDirectory: $(buildDirectory) # Single federated (WIF) login for the whole job. addSpnToEnvironment @@ -321,13 +330,15 @@ jobs: workingDirectory: $(buildDirectory) condition: always() - # Same suites as before, but not after shell smoke on the Linux agent. - # continueOnError matches the previous step: a live failure does not fail - # the required pipeline. The pipeline still waits for this job to finish. + # Live integration tests. continueOnError so a live failure never fails + # the required pipeline — but the parent GitHub check still waited for + # this job (~36 min on 7e4135eff, ~13 min past Windows smoke). Skip on + # PullRequest; run on main and gh-readonly-queue/main/* so merge still + # gets the signal. Full monorepo build stays here (live hits many packages). - job: live_linux displayName: Live tests (Linux) dependsOn: detect_changes - condition: and(succeeded(), eq(dependencies.detect_changes.outputs['detect.tsChanged'], 'true')) + condition: and(succeeded(), eq(dependencies.detect_changes.outputs['detect.tsChanged'], 'true'), ne(variables['Build.Reason'], 'PullRequest')) continueOnError: true timeoutInMinutes: 90 pool: diff --git a/ts/tools/scripts/pr-ci-scope.md b/ts/tools/scripts/pr-ci-scope.md index 010878197b..03d9bbd4d1 100644 --- a/ts/tools/scripts/pr-ci-scope.md +++ b/ts/tools/scripts/pr-ci-scope.md @@ -20,13 +20,16 @@ only need `HEAD` to install/build/test. - `build-package-shell`: unchanged merge gate — all 3 OS still package. - `pipelines/azure-smoke-tests.yml`: - detect job: `fetchDepth: 2` - - Playwright install overlaps `npm run build` on the smoke agents - - `test:live` is a parallel Linux job (still runs; `continueOnError` - unchanged). The required Linux smoke job no longer waits for it. + - Smoke agents overlap Playwright **chromium** install with + `fluid-build agent-shell|agent-cli --dep` (not full monorepo build, + not every browser binary). `playwright.config.ts` only defines + chromium; `shell:smoke` launches Electron. + - `test:live` is a parallel Linux job with `continueOnError`. It runs + on **main and merge-queue only** — not on PullRequest. A live failure + never blocked the PR, but the parent GitHub check stayed queued until + live finished (~13 min past Windows smoke on `7e4135eff`). - Windows PRs run `shell:smoke` (same electron smoke as Linux). Full - `shell:test` still runs on main and the merge-queue CI trigger. The - parent GitHub check stays queued until this Windows leg finishes — - that was the 33 min pole on SHA `82791dcf4`. + `shell:test` still runs on main and the merge-queue CI trigger. ## Job counts (from the shipped helper) diff --git a/ts/tools/scripts/prCiScope.mjs b/ts/tools/scripts/prCiScope.mjs index e2d0565fda..33baa1fd75 100644 --- a/ts/tools/scripts/prCiScope.mjs +++ b/ts/tools/scripts/prCiScope.mjs @@ -92,6 +92,18 @@ export function windowsShellSuite(buildReason) { return buildReason === "PullRequest" ? "shell:smoke" : "shell:test"; } +/** + * Live integration tests (`test:live`). They already use continueOnError + * on the required ADO pipeline, so a live failure never blocked the PR — + * but the parent GitHub check stayed queued until they finished (~36 min + * on SHA 7e4135eff, ~13 min after Windows smoke). Skip them on + * PullRequest; still run on main and merge-queue CI so merge keeps the + * signal. + */ +export function shouldRunLiveTests(buildReason) { + return buildReason !== "PullRequest"; +} + export function resolveScope(ctx) { return { full: shouldRunBuildTsFull(ctx), @@ -217,6 +229,12 @@ function main(argv = process.argv.slice(2), env = process.env) { ); return 0; } + if (argv.includes("--run-live-tests")) { + process.stdout.write( + `${shouldRunLiveTests(env.BUILD_REASON ?? env.EVENT_NAME ?? "")}\n`, + ); + return 0; + } const scope = resolveScope(readCtxFromEnv(env)); process.stdout.write(writeGithubOutput(scope, env)); return 0; diff --git a/ts/tools/scripts/test/prCiScope.spec.mjs b/ts/tools/scripts/test/prCiScope.spec.mjs index dde229ea93..98a9212211 100644 --- a/ts/tools/scripts/test/prCiScope.spec.mjs +++ b/ts/tools/scripts/test/prCiScope.spec.mjs @@ -21,6 +21,7 @@ import { shouldRunBuildTsRatchet, shouldRunShellPackage, windowsShellSuite, + shouldRunLiveTests, } from "../prCiScope.mjs"; const scriptDir = path.dirname(fileURLToPath(import.meta.url)); @@ -264,11 +265,19 @@ test("Windows merge-gate jobs exclude the workspace from Defender", () => { assert.match(smoke, /npm run \$\(windowsShellSuite\)/); }); -test("ADO smoke overlaps Playwright with build and runs live tests in parallel", () => { +test("ADO smoke overlaps Playwright chromium with shell+cli build", () => { const yaml = fs.readFileSync(azureSmokeYml, "utf8"); - assert.match(yaml, /Build \+ Playwright install \(overlapped\)/); - assert.match(yaml, /job:\s*live_linux/); - assert.match(yaml, /npm run test:live/); + assert.match( + yaml, + /Build shell\+cli \+ Playwright chromium \(overlapped\)/, + ); + assert.match(yaml, /playwright install --with-deps chromium/); + assert.match(yaml, /fluid-build "agent-shell\|agent-cli" -t build --dep/); + assert.equal( + /playwright install --with-deps(?! chromium)/.test(yaml), + false, + "smoke must not download every Playwright browser", + ); const liveAt = yaml.indexOf("job: live_linux"); const shellAt = yaml.indexOf("job: shell_and_cli"); assert.ok(liveAt > shellAt, "live_linux must be its own job"); @@ -278,4 +287,41 @@ test("ADO smoke overlaps Playwright with build and runs live tests in parallel", false, "Linux smoke job must not wait on test:live", ); + // Full monorepo build stays on the live job (main/MQ only). + const liveChunk = yaml.slice(liveAt); + assert.match(liveChunk, /npm run build/); +}); + +test("ADO live tests skip PRs; still run on main and merge-queue", () => { + assert.equal(shouldRunLiveTests("PullRequest"), false); + assert.equal(shouldRunLiveTests("IndividualCI"), true); + assert.equal(shouldRunLiveTests("Manual"), true); + const pr = execFileSync( + process.execPath, + [scriptPath, "--run-live-tests"], + { + env: { ...process.env, BUILD_REASON: "PullRequest" }, + encoding: "utf8", + }, + ).trim(); + const ci = execFileSync( + process.execPath, + [scriptPath, "--run-live-tests"], + { + env: { ...process.env, BUILD_REASON: "IndividualCI" }, + encoding: "utf8", + }, + ).trim(); + assert.equal(pr, "false"); + assert.equal(ci, "true"); + const yaml = fs.readFileSync(azureSmokeYml, "utf8"); + assert.match(yaml, /job:\s*live_linux/); + assert.match(yaml, /npm run test:live/); + // Job condition must exclude PullRequest so the parent check is not held. + const liveAt = yaml.indexOf("job: live_linux"); + const liveHead = yaml.slice(liveAt, liveAt + 600); + assert.match( + liveHead, + /ne\(variables\['Build\.Reason'\],\s*'PullRequest'\)/, + ); }); From 52b0d463c989bf42ae69867bb000e1b7ae6c1b23 Mon Sep 17 00:00:00 2001 From: Dominic Nguyen Date: Wed, 12 Aug 2026 20:41:40 -0700 Subject: [PATCH 07/12] Restore full Windows shell:test on every PR Windows PR path no longer downgrades to shell:smoke. Full jest + Playwright shell:test runs on PR, main, and merge-queue again. Linux stays on shell:smoke (pre-existing). 30% bar still expected from the other cuts: skip PR live hold, ratchet once, shallow fetch, Defender exclusion, shell+cli build scope, Playwright chromium-only. Prior full-Windows parent was ~34 min vs 46 min target. --- pipelines/azure-smoke-tests.yml | 31 ++++----------- ts/tools/scripts/pr-ci-scope.md | 4 +- ts/tools/scripts/prCiScope.mjs | 18 +-------- ts/tools/scripts/test/prCiScope.spec.mjs | 48 +++++++++++------------- 4 files changed, 33 insertions(+), 68 deletions(-) diff --git a/pipelines/azure-smoke-tests.yml b/pipelines/azure-smoke-tests.yml index ec44efc899..98c1ed58d9 100644 --- a/pipelines/azure-smoke-tests.yml +++ b/pipelines/azure-smoke-tests.yml @@ -208,12 +208,10 @@ jobs: # Overlap them so the longer of the two sets the wait, not the sum. # # Scope the work to what shell + CLI smoke actually need: - # * playwright.config.ts only defines a chromium project, and - # shell:smoke launches Electron (not every browser binary). - # Installing chromium alone is enough for shell:smoke and for - # the full shell:test suite on main/MQ. - # * fluid-build agent-shell|agent-cli --dep covers the packages - # the smoke steps exercise. Full monorepo build stays on the + # * playwright.config.ts only defines a chromium project (Electron + # suites still use the Playwright runner). Chromium alone is enough. + # * fluid-build agent-shell|agent-cli --dep covers the packages the + # smoke/shell steps exercise. Full monorepo build stays on the # live_linux job (main/MQ) and on build_ts. - bash: | set -euo pipefail @@ -270,23 +268,10 @@ jobs: # WorkloadIdentityCredential against the now-stale token file, and a failed # assertion exchange is a hard error that stops the chain before it reaches # this task's fresh AzureCliCredential. Clearing them makes it fall through. - # PRs: same electron smoke as Linux (`shell:smoke`). The parent - # GitHub check stays "queued" until this Windows leg finishes — the - # full `shell:test` Playwright suite is 30+ min and is the PR - # wall-clock. Full Playwright+jest still runs on main and - # gh-readonly-queue/main/* (merge queue; Build.Reason != PullRequest). - - bash: | - set -euo pipefail - SUITE=$(node "$(buildDirectory)/tools/scripts/prCiScope.mjs" --windows-shell-suite) - echo "windowsShellSuite=$SUITE" - echo "##vso[task.setvariable variable=windowsShellSuite]$SUITE" - displayName: Decide Windows shell suite - condition: and(succeeded(), eq(variables['Agent.OS'], 'Windows_NT')) - env: - BUILD_REASON: $(Build.Reason) - + # Full shell:test (jest + all Playwright) on every trigger — PR, main, + # and merge-queue. Linux stays on shell:smoke (unchanged from before). - task: AzureCLI@2 - displayName: Shell Tests (Windows) + displayName: Shell Tests - full (Windows) timeoutInMinutes: 60 condition: and(succeeded(), eq(variables['Agent.OS'], 'Windows_NT')) inputs: @@ -296,7 +281,7 @@ jobs: workingDirectory: $(buildDirectory)/packages/shell inlineScript: | 'AZURE_CLIENT_ID','AZURE_TENANT_ID','AZURE_FEDERATED_TOKEN_FILE' | ForEach-Object { Remove-Item "Env:$_" -ErrorAction SilentlyContinue } - npm run $(windowsShellSuite) + npm run shell:test # Own AzureCLI@2 login with the published AZURE_* vars cleared — same # rationale as "Shell Tests - full" above. diff --git a/ts/tools/scripts/pr-ci-scope.md b/ts/tools/scripts/pr-ci-scope.md index 03d9bbd4d1..5561bf727a 100644 --- a/ts/tools/scripts/pr-ci-scope.md +++ b/ts/tools/scripts/pr-ci-scope.md @@ -28,8 +28,8 @@ only need `HEAD` to install/build/test. on **main and merge-queue only** — not on PullRequest. A live failure never blocked the PR, but the parent GitHub check stayed queued until live finished (~13 min past Windows smoke on `7e4135eff`). - - Windows PRs run `shell:smoke` (same electron smoke as Linux). Full - `shell:test` still runs on main and the merge-queue CI trigger. + - Windows always runs full `shell:test` (PR, main, merge-queue). Linux + stays on `shell:smoke` (unchanged from before this work). ## Job counts (from the shipped helper) diff --git a/ts/tools/scripts/prCiScope.mjs b/ts/tools/scripts/prCiScope.mjs index 33baa1fd75..8929ef4be0 100644 --- a/ts/tools/scripts/prCiScope.mjs +++ b/ts/tools/scripts/prCiScope.mjs @@ -82,23 +82,13 @@ export function shouldRunShellPackage({ eventName, tsFilter }) { return pathFilterAllows(tsFilter); } -/** - * ADO Windows shell suite. Pull requests run the same electron smoke as - * Linux (`shell:smoke` / simple.spec.ts). The full Playwright + jest - * suite (`shell:test`) still runs on main and the merge-queue CI trigger - * (Build.Reason != PullRequest), which is what actually gates merge. - */ -export function windowsShellSuite(buildReason) { - return buildReason === "PullRequest" ? "shell:smoke" : "shell:test"; -} - /** * Live integration tests (`test:live`). They already use continueOnError * on the required ADO pipeline, so a live failure never blocked the PR — * but the parent GitHub check stayed queued until they finished (~36 min * on SHA 7e4135eff, ~13 min after Windows smoke). Skip them on * PullRequest; still run on main and merge-queue CI so merge keeps the - * signal. + * signal. Windows shell stays on full `shell:test` for every trigger. */ export function shouldRunLiveTests(buildReason) { return buildReason !== "PullRequest"; @@ -223,12 +213,6 @@ function main(argv = process.argv.slice(2), env = process.env) { process.stdout.write(`${formatScopeTable()}\n`); return 0; } - if (argv.includes("--windows-shell-suite")) { - process.stdout.write( - `${windowsShellSuite(env.BUILD_REASON ?? env.EVENT_NAME ?? "")}\n`, - ); - return 0; - } if (argv.includes("--run-live-tests")) { process.stdout.write( `${shouldRunLiveTests(env.BUILD_REASON ?? env.EVENT_NAME ?? "")}\n`, diff --git a/ts/tools/scripts/test/prCiScope.spec.mjs b/ts/tools/scripts/test/prCiScope.spec.mjs index 98a9212211..2a91e7b96c 100644 --- a/ts/tools/scripts/test/prCiScope.spec.mjs +++ b/ts/tools/scripts/test/prCiScope.spec.mjs @@ -20,7 +20,6 @@ import { shouldRunBuildTsLint, shouldRunBuildTsRatchet, shouldRunShellPackage, - windowsShellSuite, shouldRunLiveTests, } from "../prCiScope.mjs"; @@ -226,31 +225,28 @@ test("ADO detect job uses a shallow PR checkout", () => { ); }); -test("ADO Windows PRs smoke; main and merge-queue still run the full suite", () => { - assert.equal(windowsShellSuite("PullRequest"), "shell:smoke"); - assert.equal(windowsShellSuite("IndividualCI"), "shell:test"); - assert.equal(windowsShellSuite("Manual"), "shell:test"); - const pr = execFileSync( - process.execPath, - [scriptPath, "--windows-shell-suite"], - { - env: { ...process.env, BUILD_REASON: "PullRequest" }, - encoding: "utf8", - }, - ).trim(); - const ci = execFileSync( - process.execPath, - [scriptPath, "--windows-shell-suite"], - { - env: { ...process.env, BUILD_REASON: "IndividualCI" }, - encoding: "utf8", - }, - ).trim(); - assert.equal(pr, "shell:smoke"); - assert.equal(ci, "shell:test"); +test("ADO Windows always runs full shell:test (PR, main, merge-queue)", () => { const yaml = fs.readFileSync(azureSmokeYml, "utf8"); - assert.match(yaml, /--windows-shell-suite/); - assert.match(yaml, /npm run \$\(windowsShellSuite\)/); + assert.match(yaml, /Shell Tests - full \(Windows\)/); + assert.match(yaml, /npm run shell:test/); + // No PR-only downgrade to shell:smoke on Windows. + assert.equal( + yaml.includes("--windows-shell-suite"), + false, + "Windows suite must not be switched by prCiScope", + ); + const winAt = yaml.indexOf("Shell Tests - full (Windows)"); + assert.ok(winAt > 0); + const winChunk = yaml.slice(winAt, winAt + 800); + assert.match(winChunk, /npm run shell:test/); + assert.equal( + winChunk.includes("shell:smoke"), + false, + "Windows full step must not call shell:smoke", + ); + // Linux PR path stays smoke-only (pre-existing). + assert.match(yaml, /Shell Tests - smoke \(Linux\)/); + assert.match(yaml, /npm run shell:smoke/); }); test("Windows merge-gate jobs exclude the workspace from Defender", () => { @@ -262,7 +258,7 @@ test("Windows merge-gate jobs exclude the workspace from Defender", () => { assert.match(smoke, /Exclude workspace from Windows Defender/); assert.match(buildTs, /npm run test:local/); assert.match(buildShell, /pnpm run shell:package/); - assert.match(smoke, /npm run \$\(windowsShellSuite\)/); + assert.match(smoke, /npm run shell:test/); }); test("ADO smoke overlaps Playwright chromium with shell+cli build", () => { From 9d72abfc5268d3e4adc7d3ce332b039e9e83a10f Mon Sep 17 00:00:00 2001 From: Dominic Nguyen Date: Wed, 12 Aug 2026 20:46:27 -0700 Subject: [PATCH 08/12] Leave Windows Defender at the host default in CI Remove workspace Defender disable/exclusion from build_ts, package shell, and ADO smoke. The 30% PR wall-clock cut does not depend on it: baseline already ran with default Defender; the gain is skip PR live hold, single-cell ratchets, shallow PR fetch, and scoped smoke setup. --- .github/workflows/build-package-shell.yml | 6 ------ .github/workflows/build-ts.yml | 9 --------- pipelines/azure-smoke-tests.yml | 5 ----- ts/tools/scripts/test/prCiScope.spec.mjs | 11 +++++++---- 4 files changed, 7 insertions(+), 24 deletions(-) diff --git a/.github/workflows/build-package-shell.yml b/.github/workflows/build-package-shell.yml index 928f4f7f07..bcb0d30c6f 100644 --- a/.github/workflows/build-package-shell.yml +++ b/.github/workflows/build-package-shell.yml @@ -38,12 +38,6 @@ jobs: runs-on: ${{ matrix.os }} steps: - - name: Exclude workspace from Windows Defender - if: runner.os == 'Windows' - shell: pwsh - run: | - Set-MpPreference -DisableRealtimeMonitoring $true -ErrorAction SilentlyContinue - Add-MpPreference -ExclusionPath "$env:GITHUB_WORKSPACE" -ErrorAction SilentlyContinue - name: Setup Git LF run: | git config --global core.autocrlf false diff --git a/.github/workflows/build-ts.yml b/.github/workflows/build-ts.yml index f123f82dbd..f4bf089379 100644 --- a/.github/workflows/build-ts.yml +++ b/.github/workflows/build-ts.yml @@ -39,15 +39,6 @@ jobs: - if: runner.os == 'Linux' run: | sudo apt-get install -y libsecret-1-dev - # Windows Defender scanning the checkout/install/test tree is a known - # multi-minute tax on GitHub-hosted Windows. Exclusion does not skip - # any suite. - - name: Exclude workspace from Windows Defender - if: runner.os == 'Windows' - shell: pwsh - run: | - Set-MpPreference -DisableRealtimeMonitoring $true -ErrorAction SilentlyContinue - Add-MpPreference -ExclusionPath "$env:GITHUB_WORKSPACE" -ErrorAction SilentlyContinue - name: Setup Git LF run: | git config --global core.autocrlf false diff --git a/pipelines/azure-smoke-tests.yml b/pipelines/azure-smoke-tests.yml index 98c1ed58d9..abd83adceb 100644 --- a/pipelines/azure-smoke-tests.yml +++ b/pipelines/azure-smoke-tests.yml @@ -184,11 +184,6 @@ jobs: pool: vmImage: $(image) steps: - - pwsh: | - Set-MpPreference -DisableRealtimeMonitoring $true -ErrorAction SilentlyContinue - Add-MpPreference -ExclusionPath "$(Agent.BuildDirectory)" -ErrorAction SilentlyContinue - displayName: Exclude workspace from Windows Defender - condition: and(succeeded(), eq(variables['Agent.OS'], 'Windows_NT')) # Checkout + internal npm registry auth + Node + pnpm install # (--frozen-lockfile --strict-peer-dependencies). - template: include-prepare-repo.yml diff --git a/ts/tools/scripts/test/prCiScope.spec.mjs b/ts/tools/scripts/test/prCiScope.spec.mjs index 2a91e7b96c..75208bba9f 100644 --- a/ts/tools/scripts/test/prCiScope.spec.mjs +++ b/ts/tools/scripts/test/prCiScope.spec.mjs @@ -249,13 +249,16 @@ test("ADO Windows always runs full shell:test (PR, main, merge-queue)", () => { assert.match(yaml, /npm run shell:smoke/); }); -test("Windows merge-gate jobs exclude the workspace from Defender", () => { +test("Windows merge-gate jobs still run full install/build/test/package", () => { const buildTs = fs.readFileSync(buildTsYml, "utf8"); const buildShell = fs.readFileSync(buildPackageShellYml, "utf8"); const smoke = fs.readFileSync(azureSmokeYml, "utf8"); - assert.match(buildTs, /Exclude workspace from Windows Defender/); - assert.match(buildShell, /Exclude workspace from Windows Defender/); - assert.match(smoke, /Exclude workspace from Windows Defender/); + // Leave Windows Defender at the host default — do not disable it in CI. + assert.equal( + /Defender|MpPreference|ExclusionPath/.test(buildTs + buildShell + smoke), + false, + "CI must not turn off or exclude Windows Defender", + ); assert.match(buildTs, /npm run test:local/); assert.match(buildShell, /pnpm run shell:package/); assert.match(smoke, /npm run shell:test/); From 542b1d5cfef6c67971dcb91274088eb2617fa5d5 Mon Sep 17 00:00:00 2001 From: typeagent-bot Date: Thu, 13 Aug 2026 03:49:09 +0000 Subject: [PATCH 09/12] style: apply prettier formatting and policy fixes --- ts/tools/scripts/test/prCiScope.spec.mjs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ts/tools/scripts/test/prCiScope.spec.mjs b/ts/tools/scripts/test/prCiScope.spec.mjs index 75208bba9f..cf1888782f 100644 --- a/ts/tools/scripts/test/prCiScope.spec.mjs +++ b/ts/tools/scripts/test/prCiScope.spec.mjs @@ -255,7 +255,9 @@ test("Windows merge-gate jobs still run full install/build/test/package", () => const smoke = fs.readFileSync(azureSmokeYml, "utf8"); // Leave Windows Defender at the host default — do not disable it in CI. assert.equal( - /Defender|MpPreference|ExclusionPath/.test(buildTs + buildShell + smoke), + /Defender|MpPreference|ExclusionPath/.test( + buildTs + buildShell + smoke, + ), false, "CI must not turn off or exclude Windows Defender", ); From f068be06098eaa18e438e51fdc2cec95cf309c15 Mon Sep 17 00:00:00 2001 From: Dominic Nguyen Date: Wed, 12 Aug 2026 20:54:27 -0700 Subject: [PATCH 10/12] Restore PR test:live; cut redundant live/shell setup only - live_linux runs on every ts-changed trigger including PullRequest (same suite as main baseline; continueOnError unchanged). - Shell/CLI stay parallel to live so they do not serialize behind it. - Live job builds only packages that define test:live (+deps) via prCiScope --live-package-filter; still runs npm run test:live. - Tests require PR suite parity (CLI, shell:smoke, Windows shell:test, test:live) and fail if live is gated off PullRequest. --- pipelines/azure-smoke-tests.yml | 27 +++++--- ts/tools/scripts/pr-ci-scope.md | 22 ++++-- ts/tools/scripts/prCiScope.mjs | 27 +++++--- ts/tools/scripts/test/prCiScope.spec.mjs | 88 +++++++++++++++++++++--- 4 files changed, 133 insertions(+), 31 deletions(-) diff --git a/pipelines/azure-smoke-tests.yml b/pipelines/azure-smoke-tests.yml index abd83adceb..b715d01214 100644 --- a/pipelines/azure-smoke-tests.yml +++ b/pipelines/azure-smoke-tests.yml @@ -207,7 +207,7 @@ jobs: # suites still use the Playwright runner). Chromium alone is enough. # * fluid-build agent-shell|agent-cli --dep covers the packages the # smoke/shell steps exercise. Full monorepo build stays on the - # live_linux job (main/MQ) and on build_ts. + # live_linux job and on build_ts. - bash: | set -euo pipefail (cd packages/shell && pnpm exec playwright install --with-deps chromium) & @@ -310,15 +310,16 @@ jobs: workingDirectory: $(buildDirectory) condition: always() - # Live integration tests. continueOnError so a live failure never fails - # the required pipeline — but the parent GitHub check still waited for - # this job (~36 min on 7e4135eff, ~13 min past Windows smoke). Skip on - # PullRequest; run on main and gh-readonly-queue/main/* so merge still - # gets the signal. Full monorepo build stays here (live hits many packages). + # Live integration tests on every trigger that has ts changes — including + # PullRequest (same suite as main baseline). Own job so shell/CLI legs do + # not serialize behind live; continueOnError matches baseline (a live + # failure does not fail the required pipeline). Parent still waits for + # this job to finish. Build only packages that define test:live (+deps); + # npm run test:live still walks the whole workspace. - job: live_linux displayName: Live tests (Linux) dependsOn: detect_changes - condition: and(succeeded(), eq(dependencies.detect_changes.outputs['detect.tsChanged'], 'true'), ne(variables['Build.Reason'], 'PullRequest')) + condition: and(succeeded(), eq(dependencies.detect_changes.outputs['detect.tsChanged'], 'true')) continueOnError: true timeoutInMinutes: 90 pool: @@ -334,9 +335,15 @@ jobs: sudo apt install libsecret-1-0 displayName: Install libsecret-1-0 - - script: | - npm run build - displayName: Build + # Packages that ship a test:live script (see ts/packages/*/package.json + # and LIVE_TEST_PACKAGE_FILTER in prCiScope.mjs). --dep pulls their + # workspace dependencies; full monorepo build is redundant for this suite. + - bash: | + set -euo pipefail + FILTER=$(node tools/scripts/prCiScope.mjs --live-package-filter) + echo "livePackageFilter=$FILTER" + pnpm exec fluid-build "$FILTER" -t build --dep + displayName: Build live packages (+deps) workingDirectory: $(buildDirectory) - task: AzureCLI@2 diff --git a/ts/tools/scripts/pr-ci-scope.md b/ts/tools/scripts/pr-ci-scope.md index 5561bf727a..18eb553ccc 100644 --- a/ts/tools/scripts/pr-ci-scope.md +++ b/ts/tools/scripts/pr-ci-scope.md @@ -24,13 +24,27 @@ only need `HEAD` to install/build/test. `fluid-build agent-shell|agent-cli --dep` (not full monorepo build, not every browser binary). `playwright.config.ts` only defines chromium; `shell:smoke` launches Electron. - - `test:live` is a parallel Linux job with `continueOnError`. It runs - on **main and merge-queue only** — not on PullRequest. A live failure - never blocked the PR, but the parent GitHub check stayed queued until - live finished (~13 min past Windows smoke on `7e4135eff`). + - `test:live` is a **parallel** Linux job with `continueOnError` (same + blocking semantics as main baseline). It runs on **PullRequest, main, + and merge-queue** — same suite as baseline. Shell/CLI no longer wait + on live serially. Live job builds only packages that define + `test:live` (+deps); still runs `npm run test:live`. - Windows always runs full `shell:test` (PR, main, merge-queue). Linux stays on `shell:smoke` (unchanged from before this work). +## PR suite parity (must match main baseline smoke) + +| Suite | Baseline PR | This branch | +| --- | --- | --- | +| CLI smoke | yes | yes | +| Linux `shell:smoke` | yes | yes | +| Windows `shell:test` | yes | yes | +| Linux `test:live` | yes (`continueOnError`) | yes (parallel job, `continueOnError`) | + +Allowed cuts are **redundant steps only** (duplicate ratchets, extra fetches, +full-history clones, serial live after shell, full monorepo build where a +scoped `--dep` build covers the suite). Skipping a suite is not allowed. + ## Job counts (from the shipped helper) Run `node tools/scripts/prCiScope.mjs --table` from `ts/`: diff --git a/ts/tools/scripts/prCiScope.mjs b/ts/tools/scripts/prCiScope.mjs index 8929ef4be0..704e9e79a9 100644 --- a/ts/tools/scripts/prCiScope.mjs +++ b/ts/tools/scripts/prCiScope.mjs @@ -83,17 +83,24 @@ export function shouldRunShellPackage({ eventName, tsFilter }) { } /** - * Live integration tests (`test:live`). They already use continueOnError - * on the required ADO pipeline, so a live failure never blocked the PR — - * but the parent GitHub check stayed queued until they finished (~36 min - * on SHA 7e4135eff, ~13 min after Windows smoke). Skip them on - * PullRequest; still run on main and merge-queue CI so merge keeps the - * signal. Windows shell stays on full `shell:test` for every trigger. + * Live integration tests (`test:live`). Same suite on every ADO trigger + * that has ts changes — PullRequest, main, and merge-queue. Baseline ran + * live on the Linux smoke leg with continueOnError; we keep that blocking + * semantics and only move live to a parallel job so shell does not wait + * on it serially. Do not gate this off PullRequest. */ -export function shouldRunLiveTests(buildReason) { - return buildReason !== "PullRequest"; +export function shouldRunLiveTests(_buildReason) { + return true; } +/** + * Package-name regexp for fluid-build of packages that define test:live + * (plus --dep). Kept here so tests fail if the live job drifts to a full + * monorepo build or drops a live package. + */ +export const LIVE_TEST_PACKAGE_FILTER = + "agent-api|default-agent-provider|@typeagent/(aiclient|knowpro|knowledge-processor|azure-ai-foundry)"; + export function resolveScope(ctx) { return { full: shouldRunBuildTsFull(ctx), @@ -219,6 +226,10 @@ function main(argv = process.argv.slice(2), env = process.env) { ); return 0; } + if (argv.includes("--live-package-filter")) { + process.stdout.write(`${LIVE_TEST_PACKAGE_FILTER}\n`); + return 0; + } const scope = resolveScope(readCtxFromEnv(env)); process.stdout.write(writeGithubOutput(scope, env)); return 0; diff --git a/ts/tools/scripts/test/prCiScope.spec.mjs b/ts/tools/scripts/test/prCiScope.spec.mjs index cf1888782f..f63d373e90 100644 --- a/ts/tools/scripts/test/prCiScope.spec.mjs +++ b/ts/tools/scripts/test/prCiScope.spec.mjs @@ -21,6 +21,7 @@ import { shouldRunBuildTsRatchet, shouldRunShellPackage, shouldRunLiveTests, + LIVE_TEST_PACKAGE_FILTER, } from "../prCiScope.mjs"; const scriptDir = path.dirname(fileURLToPath(import.meta.url)); @@ -283,18 +284,54 @@ test("ADO smoke overlaps Playwright chromium with shell+cli build", () => { const shellAt = yaml.indexOf("job: shell_and_cli"); assert.ok(liveAt > shellAt, "live_linux must be its own job"); const shellChunk = yaml.slice(shellAt, liveAt); + // Shell job must not own a Live Tests step (parallel live_linux does). assert.equal( - shellChunk.includes("npm run test:live"), + /displayName:\s*Live Tests/.test(shellChunk), false, - "Linux smoke job must not wait on test:live", + "Linux smoke job must not run Live Tests serially", ); - // Full monorepo build stays on the live job (main/MQ only). + assert.equal( + /^\s*npm run test:live\s*$/m.test(shellChunk), + false, + "Linux smoke job must not invoke npm run test:live", + ); + // Live job builds only packages that define test:live (+deps). const liveChunk = yaml.slice(liveAt); - assert.match(liveChunk, /npm run build/); + assert.match(liveChunk, /Build live packages \(\+deps\)/); + assert.match(liveChunk, /npm run test:live/); + assert.match(liveChunk, /--live-package-filter/); + assert.match(liveChunk, /fluid-build "\$FILTER" -t build --dep/); + const filterOut = execFileSync( + process.execPath, + [scriptPath, "--live-package-filter"], + { encoding: "utf8" }, + ).trim(); + assert.equal(filterOut, LIVE_TEST_PACKAGE_FILTER); + // Every package that ships test:live must appear in the filter. + const packagesRoot = path.join(repoRoot, "ts/packages"); + const livePkgNames = []; + for (const ent of fs.readdirSync(packagesRoot, { withFileTypes: true })) { + if (!ent.isDirectory()) continue; + const pkgPath = path.join(packagesRoot, ent.name, "package.json"); + if (!fs.existsSync(pkgPath)) continue; + const pkg = JSON.parse(fs.readFileSync(pkgPath, "utf8")); + if (pkg.scripts && pkg.scripts["test:live"]) { + livePkgNames.push(pkg.name); + } + } + assert.ok(livePkgNames.length >= 1, "expected at least one test:live package"); + for (const name of livePkgNames) { + // Filter is a regexp; bare names and @scope/(a|b|c) forms both match. + const re = new RegExp(LIVE_TEST_PACKAGE_FILTER); + assert.ok( + re.test(name), + `LIVE_TEST_PACKAGE_FILTER must match package ${name}`, + ); + } }); -test("ADO live tests skip PRs; still run on main and merge-queue", () => { - assert.equal(shouldRunLiveTests("PullRequest"), false); +test("ADO live tests run on PullRequest (same suite as main baseline)", () => { + assert.equal(shouldRunLiveTests("PullRequest"), true); assert.equal(shouldRunLiveTests("IndividualCI"), true); assert.equal(shouldRunLiveTests("Manual"), true); const pr = execFileSync( @@ -313,16 +350,49 @@ test("ADO live tests skip PRs; still run on main and merge-queue", () => { encoding: "utf8", }, ).trim(); - assert.equal(pr, "false"); + assert.equal(pr, "true"); assert.equal(ci, "true"); const yaml = fs.readFileSync(azureSmokeYml, "utf8"); assert.match(yaml, /job:\s*live_linux/); assert.match(yaml, /npm run test:live/); - // Job condition must exclude PullRequest so the parent check is not held. + // Must not gate live off PullRequest (suite parity with baseline). const liveAt = yaml.indexOf("job: live_linux"); const liveHead = yaml.slice(liveAt, liveAt + 600); + assert.equal( + /ne\(\s*variables\s*\[\s*['"]Build\.Reason['"]\s*\]\s*,\s*['"]PullRequest['"]\s*\)/.test( + liveHead, + ), + false, + "live_linux must run on PullRequest", + ); assert.match( liveHead, - /ne\(variables\['Build\.Reason'\],\s*'PullRequest'\)/, + /condition:\s*and\(succeeded\(\),\s*eq\(dependencies\.detect_changes\.outputs\['detect\.tsChanged'\],\s*'true'\)\)/, + ); + // Fail closed: if someone reintroduces a PR skip, this test fails. + assert.equal( + yaml.includes("ne(variables['Build.Reason'], 'PullRequest')"), + false, + "no Build.Reason PullRequest exclusion anywhere in smoke YAML", + ); +}); + +test("PR smoke suite parity with main baseline (CLI, shell smoke/test, live)", () => { + const yaml = fs.readFileSync(azureSmokeYml, "utf8"); + // Linux CLI smoke + assert.match(yaml, /Test CLI - smoke/); + assert.match(yaml, /npm run start:dev/); + // Linux shell:smoke + assert.match(yaml, /Shell Tests - smoke \(Linux\)/); + assert.match(yaml, /npm run shell:smoke/); + // Windows full shell:test + assert.match(yaml, /Shell Tests - full \(Windows\)/); + assert.match(yaml, /npm run shell:test/); + // Linux test:live on PR path (parallel job, not PR-skipped) + assert.match(yaml, /job:\s*live_linux/); + assert.match(yaml, /npm run test:live/); + assert.equal( + yaml.includes("ne(variables['Build.Reason'], 'PullRequest')"), + false, ); }); From 8e78fd26060916c9caa0b8b02f6279cc241bad5e Mon Sep 17 00:00:00 2001 From: typeagent-bot Date: Thu, 13 Aug 2026 03:57:10 +0000 Subject: [PATCH 11/12] style: apply prettier formatting and policy fixes --- ts/tools/scripts/pr-ci-scope.md | 12 ++++++------ ts/tools/scripts/test/prCiScope.spec.mjs | 5 ++++- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/ts/tools/scripts/pr-ci-scope.md b/ts/tools/scripts/pr-ci-scope.md index 18eb553ccc..18d6fbe52a 100644 --- a/ts/tools/scripts/pr-ci-scope.md +++ b/ts/tools/scripts/pr-ci-scope.md @@ -34,12 +34,12 @@ only need `HEAD` to install/build/test. ## PR suite parity (must match main baseline smoke) -| Suite | Baseline PR | This branch | -| --- | --- | --- | -| CLI smoke | yes | yes | -| Linux `shell:smoke` | yes | yes | -| Windows `shell:test` | yes | yes | -| Linux `test:live` | yes (`continueOnError`) | yes (parallel job, `continueOnError`) | +| Suite | Baseline PR | This branch | +| -------------------- | ----------------------- | ------------------------------------- | +| CLI smoke | yes | yes | +| Linux `shell:smoke` | yes | yes | +| Windows `shell:test` | yes | yes | +| Linux `test:live` | yes (`continueOnError`) | yes (parallel job, `continueOnError`) | Allowed cuts are **redundant steps only** (duplicate ratchets, extra fetches, full-history clones, serial live after shell, full monorepo build where a diff --git a/ts/tools/scripts/test/prCiScope.spec.mjs b/ts/tools/scripts/test/prCiScope.spec.mjs index f63d373e90..95b91fe739 100644 --- a/ts/tools/scripts/test/prCiScope.spec.mjs +++ b/ts/tools/scripts/test/prCiScope.spec.mjs @@ -319,7 +319,10 @@ test("ADO smoke overlaps Playwright chromium with shell+cli build", () => { livePkgNames.push(pkg.name); } } - assert.ok(livePkgNames.length >= 1, "expected at least one test:live package"); + assert.ok( + livePkgNames.length >= 1, + "expected at least one test:live package", + ); for (const name of livePkgNames) { // Filter is a regexp; bare names and @scope/(a|b|c) forms both match. const re = new RegExp(LIVE_TEST_PACKAGE_FILTER); From 87a7bc72b7a064e745ad7138e0bfe72446e01383 Mon Sep 17 00:00:00 2001 From: Dominic Nguyen Date: Wed, 12 Aug 2026 21:18:30 -0700 Subject: [PATCH 12/12] ci: re-run required checks after prettier tip