From 1f0f7e8a26517f90741473bff3b402d77431ea3f Mon Sep 17 00:00:00 2001 From: AstroHan Date: Wed, 2 Sep 2026 00:30:58 +0800 Subject: [PATCH 1/3] ci: give the full-suite fallback's biggest buckets their real owners MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A path the classifier does not recognise sets `unknownCode`, which returns the full suite. That fallback is correct as a default and is the largest single source of full-suite runs, so the fix is to give its biggest occupants an owner rather than to weaken it. `native/gitoxide-helper/` and `native/runtime-host-peer/` each have an admission lane that owns `cargo fmt` and `cargo test` for them, and the direct-peer crate additionally reaches CLI packaging, which builds it into the tarball and runs `cargo deny` against `deny.toml`. Nothing under either is read by lint, typecheck, Storybook, or a real window. They are 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 crate added under `native/` has neither a lane nor a JavaScript consumer on the commit that introduces it, and this fallback is the only thing that would build it at all. A regression asserts that an unowned native root still selects full. `.asf.yaml` goes to `RELEASE_CONTRACT_FILES`. It names the required contexts and the release-environment admission rules, and `product-release.test.mjs` is the suite that parses it and asserts them — so selecting the release contract is what proves a change to the merge gate still passes its own policy test. Routing it anywhere else lets that gate change while the test that guards it is skipped. `patches/` deliberately keeps the fallback. 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 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 one exists the cost here is runner minutes rather than a silent regression. Refs #4480 --- scripts/ci-test-plan.mjs | 33 ++++++++++++++++ scripts/ci-test-plan.test.mjs | 71 +++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+) 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); From 8c64bc0678e30681f3857b06a9e4f3717ce5b7dc Mon Sep 17 00:00:00 2001 From: AstroHan Date: Tue, 1 Sep 2026 23:08:06 +0800 Subject: [PATCH 2/3] ci: cancel superseded installed-package validation runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `cli-package-validation` had a concurrency group but no `cancel-in-progress`, which is the setting that makes a group cancel rather than queue. A run that a newer push had already invalidated therefore ran to completion at full price while its replacement waited behind it. It fans out to about fourteen jobs per run across four build targets and four install environments, so it holds more runner slots than any other workflow here — 736 jobs against CI's 523 over one measured day. In that window 14 of its 60 pull request runs were superseded while still executing, spending 275 of 1672 slot-minutes on work that was already obsolete. Scoped to `pull_request` the way `ci.yml` scopes its own group: release callers arrive through `workflow_call`, where `github.event_name` is the caller's, so a publication run is never cancelled. Refs #4480 --- .github/workflows/cli-package-validation.yml | 7 +++++++ scripts/ci-workflow-policy.test.mjs | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+) 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/scripts/ci-workflow-policy.test.mjs b/scripts/ci-workflow-policy.test.mjs index 1297375ab2..8d202ca54b 100644 --- a/scripts/ci-workflow-policy.test.mjs +++ b/scripts/ci-workflow-policy.test.mjs @@ -407,6 +407,25 @@ test('no lane asks for the one runner label that queues', () => { } }); +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'); From 8343c555d9450a6c3c6850ae73a6667f560d2e5a Mon Sep 17 00:00:00 2001 From: AstroHan Date: Wed, 2 Sep 2026 00:31:24 +0800 Subject: [PATCH 3/3] ci: stop selecting the helper admission lane on an unrelated manifest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `gitoxide-helper-admission.yml` filtered on `packages/runtime/package.json`, which selected three runners across three operating systems on 17 of the last 300 first-parent commits. Dropping it takes that to 5. The coupling that entry was added for in #3561 is real and still exists: `gitoxide-helper-invocation-internal.ts` imports `@maka/runtime/child-process-lifecycle`, a subpath that resolves only through the exports map in that manifest, added by the same commit. Editing the map can break the Gitoxide owner. What changed is not the coupling but the question of who covers it. Core CI does. A change to `packages/runtime/package.json` selects `packages/runtime-host` through the reverse dependency closure, so the import is typechecked and tested on the required context whether or not this lane runs. What the three-runner fan-out proves that core CI cannot is the Rust side and the cross-platform invocation contract, and a manifest edit changes neither. The `pull_request` and `push` lists are edited together, and a rule now holds them that way: any lane that filters both triggers must filter them on the same paths. A lane that looks at one set before a merge and another after it reports a verdict about a tree nobody validated, and that mistake is invisible in a diff showing only the list being edited. `dependency-audit` was already drifting this way — its `push` list omitted the workflow's own file — and is corrected here. Leaving one trigger unfiltered stays available, since that is a visible and sometimes deliberate choice: `windows-recovery.yml` does exactly that, so the merged result is checked even when the pull request was green against a stale base. This replaces a test that asserted every filter entry could reach Gitoxide. Every entry names the subject, so it skipped all of them and proved nothing. Refs #4480 --- .github/workflows/dependency-audit.yml | 1 + .../workflows/gitoxide-helper-admission.yml | 2 - scripts/ci-workflow-policy.test.mjs | 51 ++++++++++++++++--- 3 files changed, 45 insertions(+), 9 deletions(-) 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-workflow-policy.test.mjs b/scripts/ci-workflow-policy.test.mjs index 8d202ca54b..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,33 @@ 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'); @@ -757,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;