diff --git a/.github/workflows/cli-package-validation.yml b/.github/workflows/cli-package-validation.yml index 7e24e9afaa..efd62c0b0b 100644 --- a/.github/workflows/cli-package-validation.yml +++ b/.github/workflows/cli-package-validation.yml @@ -80,6 +80,13 @@ permissions: concurrency: group: cli-package-validation-${{ github.workflow }}-${{ github.ref }} + # Four build targets and four install environments fan out to about fourteen + # jobs per run, which makes this the largest holder of runner slots in the + # repository. Without this, a run a newer push had already invalidated was not + # cancelled — it ran to completion while the replacement queued behind it in + # this same group. Release callers arrive through `workflow_call`, where + # `github.event_name` is the caller's, so publication runs never cancel. + cancel-in-progress: ${{ github.event_name == 'pull_request' }} jobs: peer-native: diff --git a/.github/workflows/dependency-audit.yml b/.github/workflows/dependency-audit.yml index 633771fa76..9219469ff9 100644 --- a/.github/workflows/dependency-audit.yml +++ b/.github/workflows/dependency-audit.yml @@ -34,6 +34,7 @@ on: push: branches: [main] paths: + - .github/workflows/dependency-audit.yml - scripts/audit-shipped-dependencies.mjs - scripts/third-party-closure.mjs - package.json diff --git a/.github/workflows/gitoxide-helper-admission.yml b/.github/workflows/gitoxide-helper-admission.yml index 2f9b0bfef3..f1b35bcd75 100644 --- a/.github/workflows/gitoxide-helper-admission.yml +++ b/.github/workflows/gitoxide-helper-admission.yml @@ -26,7 +26,6 @@ on: - 'packages/runtime-host/src/__tests__/gitoxide-helper-*.test.ts' - 'packages/runtime-host/src/server/gitoxide-repository-admission-authority-internal.ts' - 'packages/runtime-host/src/__tests__/gitoxide-repository-admission-authority-internal.test.ts' - - 'packages/runtime/package.json' - 'docs/architecture/gitoxide-*.md' push: branches: @@ -38,7 +37,6 @@ on: - 'packages/runtime-host/src/__tests__/gitoxide-helper-*.test.ts' - 'packages/runtime-host/src/server/gitoxide-repository-admission-authority-internal.ts' - 'packages/runtime-host/src/__tests__/gitoxide-repository-admission-authority-internal.test.ts' - - 'packages/runtime/package.json' - 'docs/architecture/gitoxide-*.md' permissions: diff --git a/scripts/ci-test-plan.mjs b/scripts/ci-test-plan.mjs index b66320136c..cbe64c9713 100644 --- a/scripts/ci-test-plan.mjs +++ b/scripts/ci-test-plan.mjs @@ -35,6 +35,11 @@ const FULL_SUITE_FILES = new Set([ ]); const RELEASE_CONTRACT_FILES = new Set([ + // Branch protection. `product-release.test.mjs` parses it and asserts the + // required contexts and the release-environment admission rules, and that + // suite runs behind `check:release` — so this selection is what proves a + // change to the merge gate still satisfies its own policy test. + '.asf.yaml', 'apps/desktop/src/main/app-update-test-context.ts', 'apps/desktop/build/entitlements.mac.inherit.plist', 'apps/desktop/build/entitlements.mac.plist', @@ -160,6 +165,10 @@ const CLI_PACKAGE_WORKSPACES = [ function isCliPackagePath(path) { if (CLI_PACKAGE_FILES.has(path) || path.startsWith('patches/')) return true; + // `release:cli:pack` builds the direct-peer addon into the tarball and runs + // `cargo deny` against that policy, so this crate ships and CLI packaging is + // the JavaScript gate that proves it still does. + if (path === 'deny.toml' || path.startsWith('native/runtime-host-peer/')) return true; if (isDocumentation(path)) return false; if (path.startsWith('scripts/release-cli-')) return true; if (path.startsWith('tsconfig') && path.endsWith('.json')) return true; @@ -444,6 +453,30 @@ export function planTests(changedFiles, options = {}) { code = true; continue; } + // Rust. Each of these crates has an admission lane that owns `cargo fmt` + // and `cargo test` for it, and `native/runtime-host-peer` additionally + // reaches CLI packaging through `isCliPackagePath` above. Nothing under + // either is read by lint, typecheck, Storybook, or a real window, so + // falling through to the guard below made this directory the largest + // single source of full-suite runs. + // + // Named one crate at a time rather than by the `native/` prefix: what + // earns the exemption is having a lane, not being written in Rust. A new + // native root has neither until someone adds one, and until then the + // guard below is the only thing that would test it at all. + // + // `deny.toml` is the lint policy both crates share and travels with them. + if ( + path.startsWith('native/gitoxide-helper/') || + path.startsWith('native/runtime-host-peer/') || + path === 'deny.toml' + ) { + continue; + } + // Branch protection selects no workspace, but it is not unknown either: + // `RELEASE_CONTRACT_FILES` above routes it to `product-release.test.mjs`, + // the suite that parses it and asserts the required contexts. + if (path === '.asf.yaml') continue; if (path.startsWith('.github/')) continue; code = true; unknownCode = true; diff --git a/scripts/ci-test-plan.test.mjs b/scripts/ci-test-plan.test.mjs index a9d03ddeb0..8aca785578 100644 --- a/scripts/ci-test-plan.test.mjs +++ b/scripts/ci-test-plan.test.mjs @@ -371,6 +371,77 @@ test('unknown top-level code fails safe to full selection', () => { assert.equal(planTests(['unknown.config'], { graph }).full, true); }); +// Everything below reached that fail-safe until now. `native/` alone was the +// largest single source of full-suite runs — 14 of the last 300 first-parent +// commits, ahead of `package-lock.json` — because the classifier had no opinion +// about a directory that three dedicated lanes already own. + +test('a Rust crate with its own admission lane selects no JavaScript surface', () => { + // `gitoxide-helper-admission.yml` owns `cargo fmt`, `cargo test`, and the + // JavaScript invocation contract for this crate on three operating systems. + // No step in `ci.yml` reads it, so the plan has nothing to select. + for (const path of ['native/gitoxide-helper/src/main.rs', 'native/gitoxide-helper/Cargo.toml']) { + const plan = planTests([path], { graph }); + assert.equal(plan.full, false, path); + assert.equal(plan.code, false, path); + assert.equal(plan.cliPackage, false, path); + assert.deepEqual(plan.workspaces, [], path); + } +}); + +test('the direct-peer crate and its lint policy select CLI packaging alone', () => { + // `release:cli:pack` builds this addon into the tarball and runs `cargo deny` + // against that policy, so CLI packaging is the one JavaScript gate with a + // stake here. Lint, typecheck, Storybook, and a real window have none. + for (const path of [ + 'native/runtime-host-peer/src/engine.rs', + 'native/runtime-host-peer/Cargo.lock', + 'deny.toml', + ]) { + const plan = planTests([path], { graph }); + assert.equal(plan.full, false, path); + assert.equal(plan.cliPackage, true, path); + assert.equal(plan.releaseContract, true, path); + assert.equal(plan.e2e, false, path); + assert.equal(plan.storybook, false, path); + assert.equal(plan.appIcons, false, path); + assert.deepEqual(plan.workspaces, [], path); + } +}); + +test('a native root without an admission lane still fails safe to full', () => { + // The exemption above is owned by a lane, not by the language. A crate added + // under `native/` has no lane on the commit that introduces it, so nothing + // but this fallback would compile or test it at all. + for (const path of ['native/new-helper/src/main.rs', 'native/new-helper/Cargo.toml']) { + assert.equal(planTests([path], { graph }).full, true, path); + } +}); + +test('branch protection reaches the suite that parses it', () => { + // `.asf.yaml` names the required contexts and the release-environment + // admission rules. `product-release.test.mjs` is the only suite that reads + // it, and it runs behind `check:release` — so selecting the release contract + // is what proves a change to the merge gate still passes its own policy test. + const plan = planTests(['.asf.yaml'], { graph }); + assert.equal(plan.full, false); + assert.equal(plan.releaseContract, true); + assert.equal(plan.code, false); + assert.deepEqual(plan.workspaces, []); +}); + +test('a dependency patch keeps the full suite until a consumer map exists', () => { + // A patch rewrites a dependency's behaviour, and which suite proves that + // behaviour is a property of the patch rather than of the directory. The + // node-pty patch is regressed by + // `packages/runtime/src/__tests__/node-pty-write-lifecycle.test.ts`, while + // the packaging smoke that a build-shaped selection would run only asserts + // that a method name still appears in the tarball. Narrowing this bucket + // needs an explicit patch-to-consumer mapping; until then it stays here, + // where the cost is runner minutes rather than a silent regression. + assert.equal(planTests(['patches/node-pty+1.2.0-beta.15.patch'], { graph }).full, true); +}); + test('full-suite authority files select every surface', () => { for (const path of ['package-lock.json', '.github/workflows/ci.yml']) { assert.equal(planTests([path], { graph }).full, true, path); diff --git a/scripts/ci-workflow-policy.test.mjs b/scripts/ci-workflow-policy.test.mjs index 1297375ab2..508a767016 100644 --- a/scripts/ci-workflow-policy.test.mjs +++ b/scripts/ci-workflow-policy.test.mjs @@ -28,7 +28,7 @@ * this file asserts exactly that, for every suite the install-free steps run. */ import assert from 'node:assert/strict'; -import { existsSync, readdirSync, readFileSync } from 'node:fs'; +import { readdirSync, readFileSync } from 'node:fs'; import test from 'node:test'; import { formatGitHubOutputs, loadWorkspaceGraph, planTests } from './ci-test-plan.mjs'; @@ -407,6 +407,52 @@ test('no lane asks for the one runner label that queues', () => { } }); +test('a lane that filters both triggers filters them on the same paths', () => { + // These lists decide what a lane looks at, and a lane that looks at one set + // on a pull request and another on main reports a verdict about a tree + // nobody validated: green before the merge, or silence after it. Editing + // one list and not its twin is the way that happens, and it is invisible in + // a diff that shows only the list being edited. + // + // Only when both triggers filter. Dropping the filter from one side is a + // different and legitimate decision — `windows-recovery.yml` leaves `push` + // unfiltered on purpose, because `strict: false` lets a pull request go + // green against a stale base and only the merged tree proves two + // independently green halves still agree. That choice is deliberate and + // visible in a diff; a list edited on one side only is neither. + let checked = 0; + + for (const name of readdirSync(WORKFLOW_DIR).filter((file) => file.endsWith('.yml'))) { + const pullRequest = pathFilter(name, 'pull_request'); + const push = pathFilter(name, 'push'); + if (!pullRequest?.length || !push?.length) continue; + + assert.deepEqual(push, pullRequest, `${name}: pull_request and push filter different paths`); + checked += 1; + } + + assert.ok(checked > 0, 'no lane filters both triggers; this rule now checks nothing'); +}); + +test('installed-package validation discards superseded pull request runs', () => { + const workflow = readWorkflow('cli-package-validation.yml'); + + // This lane fans out to about fourteen jobs per run and holds more runner + // slots than any other workflow here. A group without this setting does not + // cancel a superseded run, it queues the replacement behind it, so obsolete + // work finishes at full price. Release callers reach this through + // `workflow_call`, where `github.event_name` belongs to the caller, which is + // what keeps a publication run from ever being cancelled. + assert.match( + workflow, + /group: cli-package-validation-\$\{\{ github\.workflow \}\}-\$\{\{ github\.ref \}\}/u, + ); + assert.match( + workflow, + /\n {2}cancel-in-progress: \$\{\{ github\.event_name == 'pull_request' \}\}/u, + ); +}); + test('the recovery lane keeps every run kind out of one shared concurrency group', () => { const workflow = readWorkflow('windows-recovery.yml'); @@ -738,13 +784,23 @@ const WORKFLOW_DIR = new URL('../.github/workflows/', import.meta.url); * `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 paths = pathFilter(name, 'pull_request'); + assert.ok(paths !== null, `${name}: no pull_request trigger`); + + return paths; +} +// Returns the trigger's `paths:` entries in order, or null when the workflow +// does not carry that trigger at all — which is what lets a caller tell "no +// such trigger" apart from "this trigger runs on everything". +// +// 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. +function pathFilter(name, trigger) { 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 start = lines.findIndex((line) => new RegExp(`^ {2}${trigger}:\\s*$`, 'u').test(line)); + if (start < 0) return null; const paths = []; let inPaths = false;