diff --git a/.asf.yaml b/.asf.yaml index abb77a54e5..afff5f4c74 100644 --- a/.asf.yaml +++ b/.asf.yaml @@ -67,12 +67,12 @@ github: # Storybook. Renaming it there, or adding a paths filter that stops # ci.yml from running, freezes every pull request: the check never # reports and no committer can override it. + # A required context must report on every pull request, so a lane + # behind a paths filter cannot be listed here: the filter would keep + # the workflow from starting and the check would stay pending forever. + # windows_recovery is filtered and therefore deliberately absent. contexts: - test - # Windows recovery is a separate native crash/owner-death boundary. - # The workflow runs on every PR and main push so this context can be - # required without leaving unrelated pull requests pending forever. - - windows_recovery rulesets: - name: Immutable release tags diff --git a/.github/workflows/windows-baseline.yml b/.github/workflows/windows-baseline.yml index a2f812c7a3..06f2dafaaa 100644 --- a/.github/workflows/windows-baseline.yml +++ b/.github/workflows/windows-baseline.yml @@ -149,7 +149,7 @@ jobs: # Full packages/storage test:dist is ~10 minutes on windows-latest and # mostly duplicates the Linux unit lane. Baseline keeps process/path/ - # lock-sensitive gates here. Release-blocking crash evidence belongs to + # lock-sensitive gates here. Crash and owner-death evidence belongs to # windows-recovery.yml, so this diagnostic lane does not duplicate it. # Bump concurrency carefully — several suites spawn child processes and # fight for disk under high fan-out. diff --git a/.github/workflows/windows-recovery.yml b/.github/workflows/windows-recovery.yml index 91de576062..8f5c4966bf 100644 --- a/.github/workflows/windows-recovery.yml +++ b/.github/workflows/windows-recovery.yml @@ -17,16 +17,73 @@ name: Windows recovery +# The paths below are a pre-filter, not this lane's real input. The real input +# is the import closure of the crash and owner-death recovery authorities in +# storage, runtime and Runtime Host, which reaches well past any list worth +# hand-maintaining. So they name those workspaces and every workspace they hold +# a TypeScript project reference to, plus the manifests, patches and scripts +# the unconditional install and clean steps consume. That keeps a change there +# reported before merge, and the nightly run covers the transitive edits the +# list cannot match. +# +# GitHub evaluates a path filter against the first 300 files of the diff only, +# so a pull request wider than that can skip the filter outright. A repository-wide sweep is exactly the change that touches every +# recovery authority at once, which is one more reason the main push below +# carries no filter at all. on: pull_request: branches: [main] + paths: + - 'package.json' + - 'package-lock.json' + - 'patches/**' + - 'scripts/apply-dependency-patches.mjs' + - 'scripts/install-electron-with-retry.mjs' + - 'scripts/run-electron-installer.cjs' + - 'scripts/clean-build.mjs' + - 'scripts/clean-paths.mjs' + - 'scripts/windows-runtime-host-local-ipc-trust.ps1' + - 'tsconfig.base.json' + - 'tsconfig.lib.json' + - 'packages/core/package.json' + - 'packages/core/tsconfig.json' + - 'packages/core/src/**' + - 'packages/storage/package.json' + - 'packages/storage/tsconfig.json' + - 'packages/storage/src/**' + - 'packages/runtime/package.json' + - 'packages/runtime/tsconfig.json' + - 'packages/runtime/src/**' + - 'packages/runtime/scripts/**' + - 'packages/runtime-host/package.json' + - 'packages/runtime-host/tsconfig.json' + - 'packages/runtime-host/src/**' + - '.github/workflows/windows-recovery.yml' + # Unfiltered on purpose: required_status_checks is `strict: false`, so a pull + # request goes green against a stale base and only the merged result proves + # two independently green halves still agree. It is also the backstop for the + # 300-file case above. The nightly alone would find either a day later, + # against a batch of commits instead of one. push: branches: [main] + schedule: + # Offset from windows-sandbox-w0 so the Windows lanes do not overlap. + - cron: '17 8 * * *' workflow_dispatch: +# Pull request pushes supersede each other, keyed on the pull request number +# because github.head_ref is a bare branch name two forks can share: a second +# contributor pushing to their own `main` would otherwise cancel the first +# contributor's run, and a cancelled check is not a failed one. +# Scheduled, manual and main-push runs fall back to the run id, since +# github.ref is refs/heads/main for all three and one shared group would let a +# dispatch queue behind the nightly and then be discarded while still pending. +# That deliberately stops main pushes from superseding each other: this lane is +# the only place a merged Windows regression is observed, so every merge needs +# its own evidence rather than only the newest one surviving. concurrency: - group: windows-recovery-${{ github.ref }} - cancel-in-progress: ${{ github.event_name == 'pull_request' }} + group: windows-recovery-${{ github.event.pull_request.number || github.run_id }} + cancel-in-progress: true permissions: contents: read diff --git a/scripts/ci-test-plan.test.mjs b/scripts/ci-test-plan.test.mjs index e0e9e9c852..974b4c24c4 100644 --- a/scripts/ci-test-plan.test.mjs +++ b/scripts/ci-test-plan.test.mjs @@ -21,7 +21,7 @@ import assert from 'node:assert/strict'; import { readdirSync, readFileSync } from 'node:fs'; import test from 'node:test'; -import { formatGitHubOutputs, planTests } from './ci-test-plan.mjs'; +import { formatGitHubOutputs, loadWorkspaceGraph, planTests } from './ci-test-plan.mjs'; const dirs = [ 'packages/core', @@ -352,14 +352,113 @@ test('pull request triggers stay on an explicit allowlist', () => { ]); }); -test('Windows recovery publishes one stable PR and main check for ruleset enforcement', () => { +test('the recovery lane pairs its path filter with a nightly run and a main push', () => { + // Read from the `on:` block with comments stripped, so documenting a trigger + // cannot break its contract. + const triggers = triggerBlock('windows-recovery.yml'); + + // Same contract as the sandbox lane: the filter is a pre-filter, not the + // lane's import closure, so dropping the schedule would silently lose every + // transitive edit it cannot match, and dropping the filter would put every + // Windows recovery run back on every pull request. The main push carries no + // filter because `strict: false` lets a stale-base pull request go green, + // and because a paths filter only sees the first 300 files of a diff. + // Stripped comment lines survive as blank ones, so the gap between the + // trigger and its list is any mix of blank and four-space lines. + assert.match(triggers, /\n {2}pull_request:\n(?:(?: {4}[^\n]*)?\n)* {4}paths:/u); + assert.match(triggers, /\n {2}push:\n {4}branches: \[main\]\n/u); + assert.doesNotMatch( + triggers.match(/\n {2}push:\n(?:(?: {4}[^\n]*)?\n)*/u)?.[0] ?? '', + /\bpaths(-ignore)?:/u, + ); + assert.match(triggers, /\n {2}schedule:\n/u); + assert.match(triggers, /\n {2}workflow_dispatch:/u); + assert.match(readWorkflow('windows-recovery.yml'), /\n {4}name: windows_recovery/u); +}); + +test('the recovery lane keeps every run kind out of one shared concurrency group', () => { + const workflow = readWorkflow('windows-recovery.yml'); + + // github.head_ref is a bare branch name, so two forks pushing their own + // `main` would share a group and cancel each other; github.ref is + // refs/heads/main for the nightly, a dispatch and a main push alike, so a + // ref-keyed group made a dispatch queue behind the nightly and let the next + // dispatch discard it while pending. + assert.match( + workflow, + /group: windows-recovery-\$\{\{ github\.event\.pull_request\.number \|\| github\.run_id \}\}/u, + ); + assert.match(workflow, /\n {2}cancel-in-progress: true/u); +}); + +test('the recovery lane filters pull requests by the workspaces its steps execute', () => { const workflow = readWorkflow('windows-recovery.yml'); + const filtered = new Set(pullRequestPathFilter('windows-recovery.yml')); + + // Derived from the dist paths the steps run, then widened along the workspace + // dependency graph the planner selects with. The separator class matches the + // backslash form too, because these steps run under pwsh where both are + // legal. A new workspace on this lane, or a new dependency under one of them, + // fails here until the filter admits its sources and project file. + const executed = [ + ...new Set( + [...workflow.matchAll(/packages[/\\]([^/\\]+)[/\\]dist[/\\]/gu)].map((match) => match[1]), + ), + ].sort(); + assert.deepEqual(executed, ['runtime', 'runtime-host', 'storage']); + + const closure = dependencyClosure(executed.map((workspace) => `packages/${workspace}`)); + assert.ok(closure.includes('packages/core'), 'dependency closure must reach core'); + for (const dir of closure) { + assert.ok(filtered.has(`${dir}/src/**`), `${dir}: sources`); + assert.ok(filtered.has(`${dir}/tsconfig.json`), `${dir}: project file`); + assert.ok(filtered.has(`${dir}/package.json`), `${dir}: manifest`); + } +}); + +test('the recovery lane filter follows the postinstall launcher chain', () => { + const filtered = new Set(pullRequestPathFilter('windows-recovery.yml')); + const manifest = JSON.parse(readFileSync(new URL('../package.json', import.meta.url), 'utf8')); + // Derived from postinstall itself, then one hop into whatever those entry + // points launch, because a launcher the filter cannot see still decides what + // `npm ci` produces on Windows. A restated list missed exactly that hop. + const entrypoints = [...manifest.scripts.postinstall.matchAll(/node (scripts\/[\w.-]+)/gu)].map( + (match) => match[1], + ); + assert.ok(entrypoints.length > 0, 'postinstall runs no script'); + + for (const entrypoint of entrypoints) { + assert.ok(filtered.has(entrypoint), entrypoint); + const source = readFileSync(new URL(`../${entrypoint}`, import.meta.url), 'utf8'); + for (const launched of source.matchAll(/new URL\('\.\/([\w.-]+)'/gu)) { + assert.ok(filtered.has(`scripts/${launched[1]}`), `${entrypoint} launches ${launched[1]}`); + } + } +}); + +test('the recovery lane filters pull requests by what its install and clean steps consume', () => { + const filtered = new Set(pullRequestPathFilter('windows-recovery.yml')); - assert.match(workflow, /\n {2}pull_request:\n {4}branches: \[main\]/u); - assert.match(workflow, /\n {2}push:\n {4}branches: \[main\]/u); - assert.match(workflow, /\n {2}workflow_dispatch:/u); - assert.match(workflow, /\n {4}name: windows_recovery/u); - assert.match(workflow, /cancel-in-progress: \$\{\{ github\.event_name == 'pull_request' \}\}/u); + // `npm.cmd ci` and `npm.cmd run build:test` run unconditionally, so these are + // first-class inputs of the lane rather than transitive edits the nightly can + // be left to cover. A grouped dependabot bump touches only the manifests, and + // the crash gates sit on a native file lock the Linux `test` lane never sees. + for (const path of [ + 'package.json', + 'package-lock.json', + 'patches/**', + 'scripts/apply-dependency-patches.mjs', + 'scripts/install-electron-with-retry.mjs', + 'scripts/clean-build.mjs', + 'scripts/clean-paths.mjs', + 'scripts/windows-runtime-host-local-ipc-trust.ps1', + 'tsconfig.base.json', + 'tsconfig.lib.json', + 'packages/runtime/scripts/**', + '.github/workflows/windows-recovery.yml', + ]) { + assert.ok(filtered.has(path), path); + } }); test('the sandbox lane pairs its path filter with a nightly run', () => { @@ -494,6 +593,56 @@ test('core CI runs the live Eval proxy lifecycle when Eval is selected', () => { const WORKFLOW_DIR = new URL('../.github/workflows/', import.meta.url); +/** + * Reads the `paths` list belonging to a workflow's `pull_request` trigger. + * Anchoring to the trigger, instead of matching entry text anywhere in the + * file, is what makes the filter assertions fail when entries move under + * `paths-ignore`, under another trigger, or out of `on:` altogether. + */ +function pullRequestPathFilter(name) { + // Reads the `on:` block with comments already stripped, so a comment between + // the trigger and its list cannot end the scan, and accepts the quoting and + // spacing YAML allows, so a legal rewrite reports the entries it really has + // instead of an empty list that reads as a missing filter. + const lines = triggerBlock(name).split('\n'); + const start = lines.findIndex((line) => /^ {2}pull_request:\s*$/u.test(line)); + assert.ok(start >= 0, `${name}: no pull_request trigger`); + + const paths = []; + let inPaths = false; + for (const line of lines.slice(start + 1)) { + if (line.trim() === '') continue; + if (/^ {0,2}\S/u.test(line)) break; + if (/^ {4}\S/u.test(line)) { + inPaths = /^ {4}paths:\s*$/u.test(line); + continue; + } + const entry = inPaths ? /^\s+-\s+['"]?(.+?)['"]?\s*$/u.exec(line) : null; + if (entry) paths.push(entry[1]); + } + return paths; +} + +/** + * Workspace dirs `seeds` depend on, transitively, read off the same graph the + * planner selects with rather than a second definition of the same edges. The + * graph stores dependents, so a dependency is any dir listing one of ours. + */ +function dependencyClosure(seeds) { + const graph = loadWorkspaceGraph(); + const selected = new Set(seeds); + const pending = [...seeds]; + while (pending.length > 0) { + const dir = pending.shift(); + for (const [dependency, dependents] of graph.dependents) { + if (!dependents.has(dir) || selected.has(dependency)) continue; + selected.add(dependency); + pending.push(dependency); + } + } + return [...selected].sort(); +} + function readWorkflow(name) { return readFileSync(new URL(name, WORKFLOW_DIR), 'utf8'); } @@ -502,11 +651,14 @@ function readWorkflow(name) { * Reads the `on:` block only, so a workflow cannot escape a trigger contract by * writing `on: [pull_request]`, and prose elsewhere in the file cannot fake one. */ -function hasPullRequestTrigger(name) { +function triggerBlock(name) { const withoutComments = readWorkflow(name).replaceAll(/^[ \t]*#.*$/gmu, ''); - const triggers = withoutComments.match(/^on:(.*(?:\n(?![^\s#]).*)*)/mu)?.[1] ?? ''; - return /\bpull_request(_target)?\b/u.test(triggers); + return withoutComments.match(/^on:(.*(?:\n(?![^\s#]).*)*)/mu)?.[1] ?? ''; +} + +function hasPullRequestTrigger(name) { + return /\bpull_request(_target)?\b/u.test(triggerBlock(name)); } /** diff --git a/scripts/product-release.test.mjs b/scripts/product-release.test.mjs index 0a3754e071..4a8bfc4fd6 100644 --- a/scripts/product-release.test.mjs +++ b/scripts/product-release.test.mjs @@ -724,10 +724,7 @@ test('one product workflow gates one draft release on every required artifact', test('repository control plane admits only reviewed immutable release tags', async () => { const config = parseYaml(await readFile(new URL('../.asf.yaml', import.meta.url), 'utf8')); - assert.deepEqual(config.github.protected_branches.main.required_status_checks.contexts, [ - 'test', - 'windows_recovery', - ]); + assert.deepEqual(config.github.protected_branches.main.required_status_checks.contexts, ['test']); const environments = config.github.environments; for (const [name, tagPattern] of [ ['release', 'v*-incubating-rc*'],