From 1836eb53ed34ab77a82056b3c6edb56d83eef3cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 18 Aug 2026 19:26:52 +0200 Subject: [PATCH 1/5] ci: give check:affected real Apple ownership rules and route ios.yml on them (#1781 A9-2) Device-lane ownership by platform family in the affected selector (scripts/check-affected/device-lanes.ts): a TypeScript-only Apple change now carries replay-ios/replay-ios-device/replay-macos in a narrow plan, other families own only their own lanes, shared runtime surface owns every lane, unit tests own none. Golden tables (contracts/fixtures) own the parity unit test and both runner builds instead of failing open. ios.yml pull_request paths-ignore is routed on that ownership; the gate manifest asserts the list against the selector over every tracked path both ways (scripts/gate/routing.ts, ROUTED_LANES). push to main is unfiltered. Path coverage exempts declared manual-only checks the way owned does. --- .github/workflows/ios.yml | 30 ++++ docs/agents/testing.md | 25 ++- package.json | 2 +- scripts/check-affected/device-lanes.test.ts | 178 ++++++++++++++++++++ scripts/check-affected/device-lanes.ts | 169 +++++++++++++++++++ scripts/check-affected/model.test.ts | 33 +++- scripts/check-affected/model.ts | 29 +++- scripts/gate/audit-coverage.test.ts | 16 ++ scripts/gate/audit.ts | 18 +- scripts/gate/declarations.ts | 24 ++- scripts/gate/routing.test.ts | 121 +++++++++++++ scripts/gate/routing.ts | 78 +++++++++ 12 files changed, 709 insertions(+), 14 deletions(-) create mode 100644 scripts/check-affected/device-lanes.test.ts create mode 100644 scripts/check-affected/device-lanes.ts create mode 100644 scripts/gate/routing.test.ts create mode 100644 scripts/gate/routing.ts diff --git a/.github/workflows/ios.yml b/.github/workflows/ios.yml index 25469a626..bd555a44c 100644 --- a/.github/workflows/ios.yml +++ b/.github/workflows/ios.yml @@ -16,6 +16,36 @@ on: - '.github/workflows/deploy.yml' - '.github/workflows/pr-preview.yml' - '.github/workflows/pr-preview-cleanup.yml' + # Routing (#1781 A9-2). Everything below is derived from the affected selector's + # device-lane ownership (scripts/check-affected/device-lanes.ts): unit tests no device + # lane runs, and trees another platform family owns. `pnpm check:gate-manifest` asserts + # this list against the selector over every tracked path, both ways — a path the selector + # routes to the iOS lane (or fails open on: tooling, workflows, package manifests) must not + # appear here, and a path it classifies as another family's or as a unit test must. The + # `push` trigger has no filter: main runs every lane unconditionally. + - '**/*.md' + - 'src/**/*.test.ts' + - 'src/**/__tests__/**/*.ts' + - 'packages/*/src/**/*.test.ts' + - 'packages/*/src/**/__tests__/**/*.ts' + - 'src/platforms/android/**' + - 'src/platforms/harmonyos/**' + - 'src/platforms/linux/**' + - 'src/platforms/vega/**' + - 'src/platforms/web/**' + - 'packages/platform-android/src/**' + - 'packages/platform-harmonyos/src/**' + - 'packages/platform-linux/src/**' + - 'packages/platform-vega/src/**' + - 'packages/platform-web/src/**' + - 'android/**' + - 'linux/**' + - 'test/integration/replays/android/**' + - 'test/integration/replays/linux/**' + - 'test/integration/replays/macos/**' + - 'test/integration/android-**' + - 'test/integration/smoke-android-**' + - 'test/integration/smoke-web-**' push: branches: - main diff --git a/docs/agents/testing.md b/docs/agents/testing.md index 79e31d299..63beb623b 100644 --- a/docs/agents/testing.md +++ b/docs/agents/testing.md @@ -195,7 +195,19 @@ hand-maintained path map: script fails loudly instead of dropping a gate. - A **small explicit build-ownership layer** covers the paths whose owning build cannot be derived: Swift runner, Android helpers, macOS helper, MCP metadata, - and the public package surface (itself derived from `package.json` `exports`). + the TS/Swift golden tables (`contracts/fixtures/`), and the public package + surface (itself derived from `package.json` `exports`). +- **Device lanes** (`replay-ios`, `replay-ios-device`, `replay-macos`, + `replay-android`, `replay-linux`, `web-smoke`) are owned by platform family + (`scripts/check-affected/device-lanes.ts`): a path under a family-tagged tree + (`packages/platform-/`, `src/platforms//`, `android/`, + `test/integration/replays//`, the lane-prefixed `test/integration/` + smoke files) owns that family's lanes; untagged runtime surface owns every + lane; unit tests under `src/` and `packages/*/src/` own none. The tags are + directory-level only — `src/daemon/android-system-dialog.ts` is a naming + convention, not a boundary, and stays shared. `ios.yml`'s `pull_request` + `paths-ignore` is routed on this ownership and held to it both ways by the + gate manifest (below); `push` to main runs every lane unconditionally. Changed-file discovery folds working-tree state into the local plan: in the default local mode (`--head HEAD`) it unions the committed `base..HEAD` diff with @@ -284,9 +296,14 @@ step executes on every run. Raw shell can still run project code, but it cannot manifest. The check proves the smaller structural claim that every registered gate has an explicit CI owner and every affected path can reach one. -The three facts the manifest cannot derive live together in `scripts/gate/declarations.ts`: one -coverage wrapper, one reporting-only `test:*` script, and the Android replay owner hidden inside a -third-party action's `script:` input. +The facts the manifest cannot derive live together in `scripts/gate/declarations.ts`: opaque +runners, reporting-only `test:*` scripts, unprovable and manual-only owners, and the **routed +lanes** — a `pull_request` lane whose `paths-ignore` list is asserted against the selector over +every tracked path, both ways: a path the selector fails open on or routes to one of the lane's +declared or sampled checks must start the lane, and a path it classifies as another family's +device-lane surface or as a unit test must not (`scripts/gate/routing.ts`). GitHub evaluates +`paths-ignore` before a runner is allocated, so this is routing with no job on the critical +path; the assertion is what keeps the hand-written glob list a derived artifact. ## Mutation report over decision kernels diff --git a/package.json b/package.json index 687284a30..de211d53e 100644 --- a/package.json +++ b/package.json @@ -130,7 +130,7 @@ "fallow:baseline": "fallow dead-code --save-baseline fallow-baselines/dead-code.json --summary && fallow health --report-only --save-baseline fallow-baselines/health.json --summary", "check:fallow": "fallow audit", "check:affected": "node --experimental-strip-types scripts/check-affected/run.ts", - "check:affected:test": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/check-affected/model.test.ts scripts/check-affected/platform-packages.test.ts scripts/check-affected/run.test.ts", + "check:affected:test": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/check-affected/model.test.ts scripts/check-affected/platform-packages.test.ts scripts/check-affected/device-lanes.test.ts scripts/check-affected/run.test.ts", "gate": "node --experimental-strip-types scripts/gate/run.ts", "check:gate-manifest": "node --experimental-strip-types scripts/gate/check.ts", "check:gate-manifest:test": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/gate/*.test.ts", diff --git a/scripts/check-affected/device-lanes.test.ts b/scripts/check-affected/device-lanes.test.ts new file mode 100644 index 000000000..f6519af95 --- /dev/null +++ b/scripts/check-affected/device-lanes.test.ts @@ -0,0 +1,178 @@ +// Device-lane ownership (#1781 A9-2): planted paths per family, and the two properties the +// ios.yml routing leans on — an Apple-owned change carries its iOS checks in a narrow plan, and +// a tooling change beside it still fails open to the full set. + +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { deviceLaneLeaf, deviceLanesFor, isDeviceLaneSurface, isUnitTest } from './device-lanes.ts'; +import { selectChecks, type CheckId } from './model.ts'; + +const IOS: readonly CheckId[] = ['replay-ios', 'replay-ios-device']; + +function lanes(file: string): CheckId[] { + const plan = selectChecks({ changedFiles: [file] }); + assert.equal(plan.failOpen, false, `${file} must not fail open`); + return plan.checks.filter((id) => + /^replay-|^web-smoke$|^swift-runner-|^macos-helper$|^android-helpers$/.test(id), + ); +} + +test('a TypeScript-only Apple change selects the iOS and macOS lanes without a Swift build', () => { + for (const file of [ + 'src/platforms/apple/core/apps.ts', + 'packages/platform-apple/src/runtime.ts', + 'src/platforms/apple/os/macos/desktop.ts', + ]) { + assert.deepEqual(lanes(file), ['replay-ios', 'replay-ios-device', 'replay-macos'], file); + assert.ok( + selectChecks({ changedFiles: [file] }).reasons.some( + (reason) => reason.rule === 'own:device-lane:apple', + ), + ); + } + assert.deepEqual(lanes('src/daemon/snapshot-presentation/ios/action-shelf.ts'), [...IOS]); + assert.deepEqual(lanes('test/integration/replays/macos/01-desktop.ad'), ['replay-macos']); +}); + +test('a Swift runner change selects both builds and every Apple lane', () => { + assert.deepEqual(lanes('apple/runner/AgentDeviceRunner/Sources/Foo.swift'), [ + 'swift-runner-ios', + 'swift-runner-macos', + 'replay-ios', + 'replay-ios-device', + 'replay-macos', + ]); +}); + +test('an iOS replay script and the iOS smoke files own the iOS lanes', () => { + for (const file of [ + 'test/integration/replays/ios/simulator/01-settings.ad', + 'test/integration/replays/ios/fixture/02-checkout-release.ad', + 'test/integration/smoke-ios-simulator.test.ts', + 'test/integration/ios-simulator-e2e/live-runner.ts', + 'test/integration/ios-simulator-e2e-cleanup.test.ts', + ]) { + assert.deepEqual(lanes(file), [...IOS], file); + } +}); + +test('shared runtime surface owns every device lane', () => { + for (const file of [ + 'src/daemon/handlers/session.ts', + 'packages/kernel/src/errors.ts', + 'src/daemon/android-system-dialog.ts', // naming convention in a shared dir, not a boundary + 'test/integration/smoke-daemon-clean.test.ts', + 'src/platforms/install-source.ts', + ]) { + assert.equal(deviceLaneLeaf(file), 'shared', file); + assert.deepEqual(lanes(file), [ + 'web-smoke', + 'replay-ios', + 'replay-ios-device', + 'replay-macos', + 'replay-linux', + 'replay-android', + ]); + } +}); + +test('another family owns only its own lanes, so an Android-only change carries no iOS check', () => { + assert.deepEqual(lanes('src/platforms/android/perf.ts'), ['replay-android']); + assert.deepEqual(lanes('packages/platform-android/src/inventory.ts'), ['replay-android']); + assert.deepEqual(lanes('android/snapshot-helper/src/main/java/X.java'), [ + 'android-helpers', + 'replay-android', + ]); + assert.deepEqual(lanes('test/integration/replays/android/emulator/01-settings.ad'), [ + 'replay-android', + ]); + assert.deepEqual(lanes('test/integration/smoke-android-emulator.test.ts'), ['replay-android']); + assert.deepEqual(lanes('test/integration/android-emulator-e2e/live-runner.ts'), [ + 'replay-android', + ]); + assert.deepEqual(lanes('src/platforms/linux/snapshot.ts'), ['replay-linux']); + assert.deepEqual(lanes('linux/atspi-dump.py'), ['replay-linux']); + assert.deepEqual(lanes('src/platforms/web/provider.ts'), ['web-smoke']); + assert.deepEqual(lanes('test/integration/smoke-web-platform.test.ts'), ['web-smoke']); + // Families with no CI lane fall through to the static gates only. + assert.deepEqual(lanes('src/platforms/harmonyos/hdc.ts'), []); + assert.deepEqual(lanes('packages/platform-vega/src/index.ts'), []); +}); + +test('the fixture app owns the mobile lanes whichever subtree changes', () => { + assert.deepEqual(lanes('examples/test-app/app/index.tsx'), [ + 'replay-ios', + 'replay-ios-device', + 'replay-android', + ]); + assert.equal(deviceLaneLeaf('examples/test-app/modules/lab/android/build.gradle'), 'fixture-app'); +}); + +test('unit tests under src/ and packages/*/src own no lane', () => { + for (const file of [ + 'src/daemon/selectors.test.ts', + 'src/__tests__/contracts/interaction-guarantees.test.ts', + 'src/platforms/apple/__tests__/apps.test.ts', + 'packages/platform-apple/src/runtime.test.ts', + ]) { + assert.ok(isUnitTest(file), file); + assert.ok(!isDeviceLaneSurface(file), file); + assert.deepEqual(lanes(file), [], file); + } + // Non-TypeScript under __tests__ is a fixture the selector cannot place, not a unit test. + assert.ok(!isUnitTest('src/__tests__/test-utils/android-ime-capture.raw.json')); + // Package tests outside src/ have no owner and keep failing open. + assert.ok(!isUnitTest('packages/maestro/test/conformance/verify.test.ts')); + assert.equal( + selectChecks({ changedFiles: ['packages/maestro/test/conformance/verify.test.ts'] }).failOpen, + true, + ); +}); + +test('a mixed tree is shared, and two Apple leaves are still Apple', () => { + assert.equal(deviceLaneLeaf('src/platforms/apple/os/macos/x.ts'), 'apple'); + assert.equal(deviceLaneLeaf('test/integration/android-emulator-e2e/ios/x.ts'), 'shared'); + assert.deepEqual(deviceLanesFor('src/platforms/apple/os/macos/x.ts').lanes, [ + 'replay-ios', + 'replay-ios-device', + 'replay-macos', + ]); +}); + +test('the surface never narrows a file the selector could not otherwise place', () => { + for (const file of [ + 'src/global.d.ts', + 'src/foo.json', + 'test/integration/ios-simulator-e2e/a.png', + ]) { + assert.ok(!isDeviceLaneSurface(file), file); + assert.equal(selectChecks({ changedFiles: [file] }).failOpen, true, file); + } +}); + +test('a golden table owns the parity unit test and both runner builds', () => { + const plan = selectChecks({ changedFiles: ['contracts/fixtures/tap-point-policy.json'] }); + assert.equal(plan.failOpen, false); + for (const id of ['unit', 'swift-runner-ios', 'swift-runner-macos'] as const) { + assert.ok(plan.checks.includes(id), id); + } + assert.ok(plan.reasons.some((reason) => reason.rule === 'own:golden-table')); +}); + +test('a tooling change beside an Android-only change still fails open to the full set', () => { + for (const tooling of [ + '.github/workflows/ios.yml', + '.github/actions/setup-apple-runner-build/action.yml', + 'package.json', + 'pnpm-lock.yaml', + 'tsconfig.json', + 'vitest.config.ts', + 'scripts/check-affected/device-lanes.ts', + 'packages/platform-android/package.json', + ]) { + const plan = selectChecks({ changedFiles: ['src/platforms/android/perf.ts', tooling] }); + assert.equal(plan.failOpen, true, tooling); + assert.ok(plan.checks.includes('swift-runner-ios'), tooling); + assert.ok(plan.checks.includes('replay-ios'), tooling); + } +}); diff --git a/scripts/check-affected/device-lanes.ts b/scripts/check-affected/device-lanes.ts new file mode 100644 index 000000000..ba4d8d926 --- /dev/null +++ b/scripts/check-affected/device-lanes.ts @@ -0,0 +1,169 @@ +// Device-lane ownership: which live device lanes a changed path can break (#1781 A9-2). +// +// The live lanes (`replay-ios`, `replay-ios-device`, `replay-macos`, `replay-android`, +// `replay-linux`, `web-smoke`) drive the CLI and daemon against a real simulator, emulator, +// desktop, or browser. Before this rule the selector could reach them only through a full +// fail-open, so a TypeScript-only Apple change (`src/platforms/apple/**`) produced a plan +// with no iOS check in it at all, and nothing could route `ios.yml` on the plan. +// +// What enumerates the surface. The import graph cannot: the daemon value-imports every +// platform module (`src/platform-runtime.ts` composes all six families; `src/core/interactors/*` +// import each family's actions; `src/daemon/session-teardown.ts` calls Android cleanup on every +// session close), so reachability from `src/bin.ts` is the whole tree, and the layering graph +// does not resolve relative imports inside `packages/`. The repo's *enforced* platform partition +// can: `CANONICAL_PLATFORM_FAMILIES` names the families, layering R13 pins each family's runtime +// to `packages/platform-/` and `src/platforms//`, and the remaining family-owned +// trees are named by a family or Apple-leaf directory segment (`android/`, `linux/`, +// `test/integration/replays//`, `src/daemon/snapshot-presentation/ios/`) or, under +// `test/integration/`, by the lane prefix of the smoke file. A path tagged with exactly one +// family owns that family's lanes; a path tagged with none — or with two — is shared runtime +// surface and owns every lane. Unit tests (`*.test.ts`, `__tests__/`) under `src/` and +// `packages/` own no lane: no device lane runs Vitest, and the build excludes them. +// +// What this deliberately does not claim: that a family-tagged file is never executed by +// another family's lane. Cross-family lifecycle calls exist (see session-teardown above); the +// unit and provider suites are the backstop for those, `push` to main runs every lane +// unconditionally, and the routing decision on `ios.yml` was taken against a 90-day +// retrospective replay of merged PRs (issue #1781), not against this rule alone. + +import { CANONICAL_PLATFORM_FAMILIES } from '../layering/platform-package-policy.ts'; +import type { CheckId, SelectionReason } from './model.ts'; + +export type PlatformFamily = (typeof CANONICAL_PLATFORM_FAMILIES)[number]; + +// A family, an Apple leaf that owns a subset of the family's lanes, or the fixture app. +export type Leaf = PlatformFamily | 'ios' | 'macos' | 'fixture-app'; + +// The one place a family (or Apple leaf) is mapped to the lanes that exercise it. Families +// with no CI lane (harmonyos, vega — see the HarmonyOS hardware policy in +// docs/agents/testing.md) own nothing here and fall through to the static gates only. +const LEAF_LANES: Readonly> = { + ios: ['replay-ios', 'replay-ios-device'], + macos: ['replay-macos'], + apple: ['replay-ios', 'replay-ios-device', 'replay-macos'], + android: ['replay-android'], + linux: ['replay-linux'], + web: ['web-smoke'], + harmonyos: [], + vega: [], + // Not a family: the Expo fixture app (`examples/test-app/`) that the mobile smokes install. + 'fixture-app': ['replay-ios', 'replay-ios-device', 'replay-android'], +}; + +// Directory segments that tag a path with a family. Apple's leaves are separate tags so a +// macOS-only tree does not own the iOS lane; a tree tagged `apple` (or `tvos`/`visionos`/ +// `xcuitest`, whose only lanes today are the Apple family's) owns every Apple lane. +const APPLE_LEAF_TAGS: Readonly> = { + ios: 'ios', + macos: 'macos', + apple: 'apple', + tvos: 'apple', + visionos: 'apple', + xcuitest: 'apple', +}; + +const OTHER_FAMILIES: readonly PlatformFamily[] = CANONICAL_PLATFORM_FAMILIES.filter( + (family) => family !== 'apple', +); + +export const ALL_DEVICE_LANES: readonly CheckId[] = [...new Set(Object.values(LEAF_LANES).flat())]; + +function leafOfSegment(segment: string): Leaf | null { + const tag = /^platform-([^/]+)$/.exec(segment)?.[1] ?? segment; + if (tag in APPLE_LEAF_TAGS) return APPLE_LEAF_TAGS[tag]!; + return OTHER_FAMILIES.find((family) => family === tag) ?? null; +} + +// Under test/integration/ the smoke files carry their lane in the leading tokens of the first +// segment (`smoke-android-emulator.test.ts`, `ios-simulator-e2e/`, `android-emulator-e2e/`). +// Elsewhere only directory segments tag: `src/daemon/android-system-dialog.ts` is a naming +// convention in a shared directory, not an enforced boundary, so it stays shared. +function tagSegments(file: string): string[] { + const segments = file.split('/'); + const directories = segments.slice(0, -1); + const rest = file.startsWith('test/integration/') ? segments.slice(2, 3) : []; + return [ + ...directories, + ...rest.flatMap((segment) => + segment + .replace(/^smoke-/, '') + .split(/[-.]/) + .slice(0, 1), + ), + ]; +} + +// The family root of `file`, `'shared'` when it is untagged or tagged by more than one leaf +// (a mixed tree is not provably any single family's). +export function deviceLaneLeaf(file: string): Leaf | 'shared' { + // The Expo fixture app is consumed by exactly the mobile lanes, whichever subtree changes. + if (file.startsWith('examples/test-app/')) return 'fixture-app'; + const leaves = new Set( + tagSegments(file) + .map(leafOfSegment) + .filter((leaf): leaf is Leaf => leaf !== null), + ); + if (leaves.size === 0) return 'shared'; + if (leaves.size === 1) return [...leaves][0]!; + // Two Apple leaves (ios + macos) still resolve to the Apple family; two families are shared. + return [...leaves].every((leaf) => leaf in APPLE_LEAF_TAGS) ? 'apple' : 'shared'; +} + +export function deviceLanesFor(file: string): { leaf: Leaf | 'shared'; lanes: readonly CheckId[] } { + const leaf = deviceLaneLeaf(file); + return { leaf, lanes: leaf === 'shared' ? ALL_DEVICE_LANES : LEAF_LANES[leaf] }; +} + +// The runtime surface a device lane can observe. Listed by root AND extension class so this +// rule never turns a file the selector could not otherwise place (a `.json` under `src/`, a +// screenshot under `test/integration/`) into a narrow plan: those keep failing open through +// the ambiguous-path guard in model.ts, exactly as before. +const RUNTIME_SURFACE: ReadonlyArray<{ root: string; owns: (file: string) => boolean }> = [ + // Production TypeScript; unit tests own no lane (no device lane runs Vitest, the build + // excludes them). + { root: 'src/', owns: (file) => isProductionTs(file) }, + { + root: 'packages/', + owns: (file) => /^packages\/[^/]+\/src\//.test(file) && isProductionTs(file), + }, + // Integration tests, their support modules, and the replay scripts the lanes execute. + { root: 'test/integration/', owns: (file) => file.endsWith('.ts') || file.endsWith('.ad') }, + // The Expo fixture app the iOS/Android smoke drives (same extension class as own:test-app). + { root: 'examples/test-app/', owns: (file) => /\.(?:[cm]?[jt]sx?|json)$/.test(file) }, + // Native runner and helper sources — build inputs of the lanes that boot them. + { root: 'apple/', owns: () => true }, + { root: 'android/', owns: () => true }, + { root: 'linux/', owns: () => true }, + // TS/Swift golden tables (own:golden-table in model.ts): the parity XCTest runs on the lane. + { root: 'contracts/fixtures/', owns: () => true }, +]; + +// Unit tests under src/ and packages/*/src/ — Vitest's, which no device lane runs and the +// build excludes. TypeScript only: a `.json` capture under `__tests__/` is a fixture the +// selector cannot place, and it fails open like any other unowned file. (`test/integration/**` +// tests are the lanes' own smoke files and stay on the surface.) +export function isUnitTest(file: string): boolean { + return ( + /^(?:src|packages\/[^/]+\/src)\//.test(file) && + file.endsWith('.ts') && + (/\.test\.ts$/.test(file) || /(?:^|\/)__tests__\//.test(file)) + ); +} + +function isProductionTs(file: string): boolean { + return file.endsWith('.ts') && !file.endsWith('.d.ts') && !isUnitTest(file); +} + +export function isDeviceLaneSurface(file: string): boolean { + return RUNTIME_SURFACE.some((entry) => file.startsWith(entry.root) && entry.owns(file)); +} + +export function deviceLaneOwnership(file: string): SelectionReason[] { + if (!isDeviceLaneSurface(file)) return []; + const { leaf, lanes } = deviceLanesFor(file); + const detail = + leaf === 'shared' + ? 'shared runtime surface: every device lane drives the CLI and daemon through it' + : `${leaf}-owned tree: only the ${leaf} device lanes execute it`; + return lanes.map((check) => ({ check, path: file, rule: `own:device-lane:${leaf}`, detail })); +} diff --git a/scripts/check-affected/model.test.ts b/scripts/check-affected/model.test.ts index dad58e18d..c9811b878 100644 --- a/scripts/check-affected/model.test.ts +++ b/scripts/check-affected/model.test.ts @@ -75,9 +75,13 @@ test('android-adb stub test delegates project ownership to Vitest', () => { test('Swift runner change selects both XCUITest platform builds', () => { // Each platform build is its own gate in its own lane, so a Swift change owns both. + // (The Apple device lanes ride along: the runner is what those lanes boot — device-lanes.ts.) assert.deepEqual(ids(['apple/runner/Sources/Runner/Main.swift']), [ 'swift-runner-ios', 'swift-runner-macos', + 'replay-ios', + 'replay-ios-device', + 'replay-macos', ]); assert.ok(ids(['src/platforms/apple/core/runner/Support.swift']).includes('swift-runner-ios')); }); @@ -88,7 +92,14 @@ test('a runner XCTest source also selects the test-list and package-source check // (#1781 A7), and the platform builds cannot see that — they compile fine either way. assert.deepEqual( ids(['apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Alert.swift']), - ['swift-runner-ios', 'swift-runner-macos', 'xctest-selection'], + [ + 'swift-runner-ios', + 'swift-runner-macos', + 'xctest-selection', + 'replay-ios', + 'replay-ios-device', + 'replay-macos', + ], ); // The bug the file filter used to have: membership is the directory, not the name. assert.ok( @@ -101,8 +112,14 @@ test('a runner XCTest source also selects the test-list and package-source check }); test('Android helper change selects the android-helpers build', () => { - assert.deepEqual(ids(['android/snapshot-helper/src/Main.kt']), ['android-helpers']); - assert.deepEqual(ids(['android/ime-helper/AndroidManifest.xml']), ['android-helpers']); + assert.deepEqual(ids(['android/snapshot-helper/src/Main.kt']), [ + 'android-helpers', + 'replay-android', + ]); + assert.deepEqual(ids(['android/ime-helper/AndroidManifest.xml']), [ + 'android-helpers', + 'replay-android', + ]); }); test('MCP metadata change selects the mcp-metadata check', () => { @@ -128,7 +145,15 @@ test('docs-only change selects no checks and records the docs paths', () => { test('test app source selects root lint and format plus its isolated typecheck', () => { const result = plan(['examples/test-app/app/index.tsx']); assert.equal(result.failOpen, false); - assert.deepEqual(result.checks, ['format', 'lint', 'test-app-typecheck']); + // Plus the mobile lanes that install the fixture app it builds (device-lanes.ts). + assert.deepEqual(result.checks, [ + 'format', + 'lint', + 'test-app-typecheck', + 'replay-ios', + 'replay-ios-device', + 'replay-android', + ]); }); test('unknown path fails open to the full check set', () => { diff --git a/scripts/check-affected/model.ts b/scripts/check-affected/model.ts index c40480e70..5cc9e2145 100644 --- a/scripts/check-affected/model.ts +++ b/scripts/check-affected/model.ts @@ -20,6 +20,7 @@ // remains authoritative; this only optimizes local/agent feedback. import { WIRE_SURFACE_FILES } from '../../test/wire-compat/surface.ts'; +import { deviceLaneOwnership } from './device-lanes.ts'; // The canonical gate universe. Every gate CI runs is one of these — including the // ones that drive their own runner (fuzz, mutation, the Maestro differential) and @@ -209,7 +210,7 @@ function isWorkflowTooling(file: string): boolean { ); } -function isDocs(file: string): boolean { +export function isDocs(file: string): boolean { // skills/ Markdown is agent guidance prose with no owning suite (the // SkillGym harness was removed), so it classifies as docs like the rest. return ( @@ -481,6 +482,27 @@ const BUILD_OWNERSHIP: ReadonlyArray<{ detail: 'MCP registry metadata must stay in sync', owns: (file) => file === 'server.json' || file === 'smithery.yaml', }, + // TS/Swift golden tables (`contracts/fixtures/*.json`): the vitest parity test and the + // runner XCTest twin both read them, so a table edit owns the unit lane and both runner + // builds. Without this a `.json` under contracts/ has no derivable owner and fails open. + { + check: 'unit', + rule: 'own:golden-table', + detail: 'the vitest parity twin asserts the TS rule against the golden table', + owns: (file) => file.startsWith('contracts/fixtures/'), + }, + { + check: 'swift-runner-ios', + rule: 'own:golden-table', + detail: 'the runner XCTest twin asserts the Swift rule against the golden table', + owns: (file) => file.startsWith('contracts/fixtures/'), + }, + { + check: 'swift-runner-macos', + rule: 'own:golden-table', + detail: 'the runner XCTest twin asserts the Swift rule against the golden table', + owns: (file) => file.startsWith('contracts/fixtures/'), + }, ]; const buildOwnership: OwnershipRule = ({ file }, input) => { @@ -520,6 +542,10 @@ const docsOwnership: OwnershipRule = ({ file }) => ] : []; +// Live device lanes, by platform family (device-lanes.ts). Selected here alongside the +// static gates so a TypeScript-only Apple change carries its iOS/macOS lanes in the plan. +const deviceLaneRule: OwnershipRule = ({ file }) => deviceLaneOwnership(file); + const OWNERSHIP_RULES: readonly OwnershipRule[] = [ formatGate, staticTsGates, @@ -532,6 +558,7 @@ const OWNERSHIP_RULES: readonly OwnershipRule[] = [ replayCompatOwnership, daemonWireCompatOwnership, buildOwnership, + deviceLaneRule, ]; function fileFacts(file: string): FileFacts { diff --git a/scripts/gate/audit-coverage.test.ts b/scripts/gate/audit-coverage.test.ts index d45a2e31a..e3d2df430 100644 --- a/scripts/gate/audit-coverage.test.ts +++ b/scripts/gate/audit-coverage.test.ts @@ -140,3 +140,19 @@ test('a `test:*` script that is a suite by name, not by shape, needs an owner', 'a new test:* script with no catalog entry must fail `registered`', ); }); + +// The device-lane rules (#1781 A9-2) route Apple and Android paths to the parked replay lanes. +// Path coverage exempts a declared manual-only check the way `owned` does — the gap is already +// printed by name — but only while it is declared: drop the declaration and every path that +// selects the check reports it. +test('a parked check selected by a path is exempt from path-coverage only while declared', () => { + const declared = audit(base).filter((failure) => failure.assertion === 'path-coverage'); + assert.deepEqual(declared, []); + const undeclared = audit(base, { manualOnly: {}, unprovable: {} }).filter( + (failure) => failure.assertion === 'path-coverage', + ); + assert.ok( + undeclared.some((failure) => /selects "replay-ios"/.test(failure.message)), + 'without the declaration, the iOS replay lane must surface as unreachable from its paths', + ); +}); diff --git a/scripts/gate/audit.ts b/scripts/gate/audit.ts index 2955f6460..7c01f0715 100644 --- a/scripts/gate/audit.ts +++ b/scripts/gate/audit.ts @@ -6,9 +6,12 @@ import { MANUAL_ONLY_OWNERS, type ManualOnlyOwner, REPORTING_SCRIPTS, + ROUTED_LANES, + type RoutedLane, UNPROVABLE_OWNERS, } from './declarations.ts'; import { categories, checkUnits, covered, scriptUnits, type Model } from './model.ts'; +import { routing } from './routing.ts'; export type Failure = { readonly assertion: string; readonly message: string }; @@ -16,11 +19,13 @@ export type Failure = { readonly assertion: string; readonly message: string }; export type GateDeclarations = { readonly manualOnly: Readonly>; readonly unprovable: Readonly>; + readonly routed?: readonly RoutedLane[]; }; const DECLARED: GateDeclarations = { manualOnly: MANUAL_ONLY_OWNERS, unprovable: UNPROVABLE_OWNERS, + routed: ROUTED_LANES, }; const HEADINGS: Readonly> = { @@ -29,6 +34,7 @@ const HEADINGS: Readonly> = { gate: 'Gate ids that name no registered check', surface: 'Execution surfaces the manifest does not model', 'path-coverage': 'Paths whose selected checks no triggered lane runs', + routing: 'Routed lanes whose paths-ignore disagrees with the selector', registered: 'Suites and projects no registered check covers', }; @@ -169,11 +175,16 @@ function laneSurfaces(model: Model): Failure[] { ); } -function pathCoverage(model: Model): Failure[] { +// A parked (manual-only) or loader-invisible check is exempt here for the same reason it is +// exempt in `unowned`: no path can reach a lane nothing starts, and check.ts already prints +// those checks by name on every run. Repeating the gap once per path that selects them would +// only bury the real path-coverage findings. +function pathCoverage(model: Model, declarations: GateDeclarations): Failure[] { + const exempt = { ...declarations.unprovable, ...declarations.manualOnly }; return categories(model).flatMap((category) => category.checks.flatMap((id) => { const spec = CHECK_CATALOG.find((entry) => entry.id === id); - if (!spec) return []; + if (!spec || id in exempt) return []; const result = covered(spec, category.path, model); if (result.covered) return []; return [ @@ -234,7 +245,8 @@ export function audit(model: Model, declarations: GateDeclarations = DECLARED): ...manualOnly(model, declarations.manualOnly), ...gateIds(model), ...laneSurfaces(model), - ...pathCoverage(model), + ...pathCoverage(model, declarations), + ...routing(model, declarations.routed ?? []), ...unregisteredSuites(model), ...orphanProjects(model), ]; diff --git a/scripts/gate/declarations.ts b/scripts/gate/declarations.ts index 75ab15dad..d49d98db7 100644 --- a/scripts/gate/declarations.ts +++ b/scripts/gate/declarations.ts @@ -1,4 +1,4 @@ -// The four small facts the manifest cannot derive from package scripts and workflow YAML. +// The five small facts the manifest cannot derive from package scripts and workflow YAML. // A script whose Vitest/node-test invocation the loader cannot read, mapped to the units it // really runs. Empty right now: the one entry was a coverage wrapper, and `test:coverage:ci` @@ -70,3 +70,25 @@ export const MANUAL_ONLY_OWNERS: Readonly> = { ].join(' '), }, }; + +/** + * A `pull_request` lane whose `paths-ignore` list is routing, not hygiene (#1781 A9-2): the + * list is asserted against the affected selector over every tracked path, both ways. A path + * the selector fails open on, or routes to one of the lane's declared gates or to `sampled` + * (checks the lane runs a slice of through raw shell, invisible to the loader), must start the + * lane; a path the selector places on another family's device-lane surface, or classifies as + * a unit test, must not. GitHub evaluates the list before a runner is allocated, so the + * decision costs no macOS time and adds no job to the critical path. + */ +export type RoutedLane = { + /** Lane label, exactly as the loader builds it: ` / `. */ + readonly lane: string; + /** Checks the lane samples without declaring, in addition to its run-gate declarations. */ + readonly sampled: readonly string[]; +}; + +export const ROUTED_LANES: readonly RoutedLane[] = [ + // ios.yml declares swift-runner-ios; its raw-shell steps run one iOS simulator replay and, when + // IOS_UDID is set, one physical-device replay — a slice of the parked replay-ios lanes. + { lane: 'iOS / Smoke Tests', sampled: ['replay-ios', 'replay-ios-device'] }, +]; diff --git a/scripts/gate/routing.test.ts b/scripts/gate/routing.test.ts new file mode 100644 index 000000000..30a3128f5 --- /dev/null +++ b/scripts/gate/routing.test.ts @@ -0,0 +1,121 @@ +// Routed-lane witnesses (#1781 A9-2): the live ios.yml agrees with the selector, and each +// planted disagreement below is reported — an ignore that hides an Apple path or a tooling +// glob, and a missing ignore for a tree the selector says is another family's. + +import { execFileSync } from 'node:child_process'; +import assert from 'node:assert/strict'; +import path from 'node:path'; +import test from 'node:test'; +import { ROUTED_LANES } from './declarations.ts'; +import { loadModel, type Model } from './model.ts'; +import { routing } from './routing.ts'; +import type { Lane } from './workflows.ts'; + +const repoRoot = path.resolve(import.meta.dirname, '../..'); +const tracked = execFileSync('git', ['ls-files'], { cwd: repoRoot, encoding: 'utf8' }) + .split('\n') + .filter(Boolean); +const base = loadModel(repoRoot, tracked); +const IOS = ROUTED_LANES.find((routed) => routed.lane === 'iOS / Smoke Tests'); +assert.ok(IOS, 'the iOS lane is declared routed'); +const iosLane = base.lanes.find((lane) => lane.label === IOS.lane); +assert.ok(iosLane, 'ios.yml defines the routed lane'); + +function withIos(change: (lane: Lane) => Lane): Model { + return { + ...base, + lanes: base.lanes.map((lane) => (lane.label === IOS.lane ? change(lane) : lane)), + }; +} + +const messages = (model: Model) => routing(model, ROUTED_LANES).map((failure) => failure.message); + +test('the live ios.yml paths-ignore agrees with the selector over every tracked path', () => { + assert.deepEqual(messages(base), []); +}); + +test('the routed lane derives its needs from its declared gate plus the sampled checks', () => { + assert.ok(iosLane.gates.includes('swift-runner-ios'), 'ios.yml still declares the runner build'); + assert.deepEqual([...IOS.sampled], ['replay-ios', 'replay-ios-device']); +}); + +test('ignoring an Apple-owned tree is reported with the checks the selector routes it to', () => { + const model = withIos((lane) => ({ + ...lane, + pathsIgnore: [...lane.pathsIgnore, 'src/platforms/apple/**'], + })); + const found = messages(model); + assert.ok(found.length > 0); + assert.ok( + found.some( + (message) => + /ignores src\/platforms\/apple\//.test(message) && + /routes it to "replay-ios"/.test(message), + ), + found.slice(0, 3).join('\n'), + ); +}); + +test('ignoring a tooling glob is reported as fail-open, even under .github/', () => { + const model = withIos((lane) => ({ + ...lane, + pathsIgnore: [...lane.pathsIgnore, '.github/actions/**', 'package.json'], + })); + const found = messages(model); + assert.ok( + found.some( + (message) => + /ignores \.github\/actions\/setup-apple-runner-build\/action\.yml/.test(message) && + /fails open on it \(workflow-tooling\)/.test(message), + ), + found.slice(0, 3).join('\n'), + ); + assert.ok( + found.some((message) => /ignores package\.json, but the selector fails open/.test(message)), + ); +}); + +test('a .github path ignored by exact name stays the workflow’s own call', () => { + // The live list names deploy.yml and the docs preview workflows explicitly; the live tree + // is green above, so this only pins that the exemption is by exact path, not by prefix. + const model = withIos((lane) => ({ + ...lane, + pathsIgnore: [...lane.pathsIgnore, '.github/workflows/size.yml'], + })); + assert.deepEqual(messages(model), []); +}); + +test('dropping a family root from the ignore list fails the routing claim for that tree', () => { + const model = withIos((lane) => ({ + ...lane, + pathsIgnore: lane.pathsIgnore.filter((pattern) => pattern !== 'src/platforms/android/**'), + })); + const found = messages(model); + assert.ok( + found.some( + (message) => + /starts on src\/platforms\/android\//.test(message) && + /android-owned \(lanes: replay-android\)/.test(message), + ), + found.slice(0, 3).join('\n'), + ); +}); + +test('dropping the unit-test ignore fails the routing claim for a unit test', () => { + const model = withIos((lane) => ({ + ...lane, + pathsIgnore: lane.pathsIgnore.filter((pattern) => pattern !== 'src/**/*.test.ts'), + })); + assert.ok( + messages(model).some((message) => + /\.test\.ts, which the selector classifies as a unit test/.test(message), + ), + ); +}); + +test('a routed lane that loses its pull_request trigger, or its job, is reported', () => { + const gone = { ...base, lanes: base.lanes.filter((lane) => lane.label !== IOS.lane) }; + assert.ok(messages(gone).some((message) => /not defined by any workflow/.test(message))); + const pushOnly = withIos((lane) => ({ ...lane, triggers: ['push'] })); + assert.ok(messages(pushOnly).some((message) => /no pull_request trigger/.test(message))); +}); diff --git a/scripts/gate/routing.ts b/scripts/gate/routing.ts new file mode 100644 index 000000000..db4dfb2c6 --- /dev/null +++ b/scripts/gate/routing.ts @@ -0,0 +1,78 @@ +// Routed lanes: a `paths-ignore` list held to the affected selector, both ways (#1781 A9-2). +// +// GitHub evaluates `paths-ignore` before it allocates a runner, so it is the one routing +// mechanism that costs no macOS time and adds no job to the critical path. Its weakness is +// that it is a hand-written glob list next to a derived selector — the two drift the first +// time someone adds a platform root or a unit-test convention. This assertion is what makes +// the YAML a derived artifact: over every tracked path, the lane must start whenever the +// selector says the change can reach it, and must not start on a path the selector places on +// another family's device-lane surface or classifies as a unit test. + +import { deviceLanesFor, isDeviceLaneSurface, isUnitTest } from '../check-affected/device-lanes.ts'; +import { isDocs, selectChecks, type CheckId } from '../check-affected/model.ts'; +import type { RoutedLane } from './declarations.ts'; +import type { Model } from './model.ts'; +import { type Lane, triggersOnPath } from './workflows.ts'; + +export type RoutingFailure = { readonly assertion: 'routing'; readonly message: string }; + +function failure(message: string): RoutingFailure { + return { assertion: 'routing', message }; +} + +// A `.github/**` path the lane ignores by its exact name is the workflow's own knowledge of a +// sibling workflow it does not use (deploy, docs preview); the selector's fail-open on +// `.github/**` is about *local* checks and cannot see that. A glob there is not exempt: it +// could hide the composite action the lane itself runs. +function ignoredByExactName(lane: Lane, file: string): boolean { + return file.startsWith('.github/') && lane.pathsIgnore.includes(file); +} + +export function routingFor(model: Model, routed: RoutedLane): RoutingFailure[] { + const lane = model.lanes.find((candidate) => candidate.label === routed.lane); + if (!lane) { + return [failure(`routed lane "${routed.lane}" is not defined by any workflow.`)]; + } + if (!lane.triggers.includes('pull_request')) { + return [failure(`routed lane "${routed.lane}" has no pull_request trigger to route.`)]; + } + const needs = new Set([...lane.gates, ...routed.sampled]); + const failures: RoutingFailure[] = []; + for (const file of [...model.trackedFiles].sort()) { + if (isDocs(file)) continue; + const plan = selectChecks({ changedFiles: [file], packageEntryFiles: model.packageEntryFiles }); + const routedTo = plan.checks.filter((id) => needs.has(id)); + const needsLane = plan.failOpen || routedTo.length > 0; + const starts = triggersOnPath(lane, file); + if (needsLane && !starts && !ignoredByExactName(lane, file)) { + const why = plan.failOpen + ? `fails open on it (${plan.failOpenReasons.map((reason) => reason.rule).join(', ')})` + : `routes it to ${routedTo.map((id) => `"${id}"`).join(', ')}`; + failures.push( + failure( + `${lane.workflow} ignores ${file}, but the selector ${why}. Remove the ignore entry.`, + ), + ); + continue; + } + if (needsLane || !starts) continue; + const surface = isDeviceLaneSurface(file) ? deviceLanesFor(file) : null; + const claim = isUnitTest(file) + ? 'a unit test no device lane runs' + : surface && !surface.lanes.some((id: CheckId) => needs.has(id)) + ? `${surface.leaf}-owned (lanes: ${surface.lanes.join(', ') || 'none'})` + : null; + if (claim === null) continue; + failures.push( + failure( + `${lane.workflow} starts on ${file}, which the selector classifies as ${claim}. ` + + `Add it to paths-ignore, or the routing claim is false for that path.`, + ), + ); + } + return failures; +} + +export function routing(model: Model, routedLanes: readonly RoutedLane[]): RoutingFailure[] { + return routedLanes.flatMap((routed) => routingFor(model, routed)); +} From 64f9458befbdc9b7a38067b2d203733ca817dfed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 18 Aug 2026 19:29:04 +0200 Subject: [PATCH 2/5] ci: tighten routing assertion shape (fallow: unused exports, complexity) --- scripts/check-affected/device-lanes.ts | 2 +- scripts/gate/routing.ts | 81 +++++++++++++++----------- 2 files changed, 47 insertions(+), 36 deletions(-) diff --git a/scripts/check-affected/device-lanes.ts b/scripts/check-affected/device-lanes.ts index ba4d8d926..21bc54095 100644 --- a/scripts/check-affected/device-lanes.ts +++ b/scripts/check-affected/device-lanes.ts @@ -66,7 +66,7 @@ const OTHER_FAMILIES: readonly PlatformFamily[] = CANONICAL_PLATFORM_FAMILIES.fi (family) => family !== 'apple', ); -export const ALL_DEVICE_LANES: readonly CheckId[] = [...new Set(Object.values(LEAF_LANES).flat())]; +const ALL_DEVICE_LANES: readonly CheckId[] = [...new Set(Object.values(LEAF_LANES).flat())]; function leafOfSegment(segment: string): Leaf | null { const tag = /^platform-([^/]+)$/.exec(segment)?.[1] ?? segment; diff --git a/scripts/gate/routing.ts b/scripts/gate/routing.ts index db4dfb2c6..b4ed79ff3 100644 --- a/scripts/gate/routing.ts +++ b/scripts/gate/routing.ts @@ -28,7 +28,27 @@ function ignoredByExactName(lane: Lane, file: string): boolean { return file.startsWith('.github/') && lane.pathsIgnore.includes(file); } -export function routingFor(model: Model, routed: RoutedLane): RoutingFailure[] { +// Why the selector says the lane must start on `file`, or null when it need not. +function mustStart(model: Model, needs: ReadonlySet, file: string): string | null { + const plan = selectChecks({ changedFiles: [file], packageEntryFiles: model.packageEntryFiles }); + if (plan.failOpen) { + return `fails open on it (${plan.failOpenReasons.map((reason) => reason.rule).join(', ')})`; + } + const routedTo = plan.checks.filter((id) => needs.has(id)); + return routedTo.length > 0 ? `routes it to ${routedTo.map((id) => `"${id}"`).join(', ')}` : null; +} + +// How the selector classifies a path the lane need not start, or null when it makes no claim +// (a path outside the device-lane surface is simply not the routing's business). +function mustNotStart(needs: ReadonlySet, file: string): string | null { + if (isUnitTest(file)) return 'a unit test no device lane runs'; + if (!isDeviceLaneSurface(file)) return null; + const { leaf, lanes } = deviceLanesFor(file); + if (lanes.some((id: CheckId) => needs.has(id))) return null; + return `${leaf}-owned (lanes: ${lanes.join(', ') || 'none'})`; +} + +function routingFor(model: Model, routed: RoutedLane): RoutingFailure[] { const lane = model.lanes.find((candidate) => candidate.label === routed.lane); if (!lane) { return [failure(`routed lane "${routed.lane}" is not defined by any workflow.`)]; @@ -37,40 +57,31 @@ export function routingFor(model: Model, routed: RoutedLane): RoutingFailure[] { return [failure(`routed lane "${routed.lane}" has no pull_request trigger to route.`)]; } const needs = new Set([...lane.gates, ...routed.sampled]); - const failures: RoutingFailure[] = []; - for (const file of [...model.trackedFiles].sort()) { - if (isDocs(file)) continue; - const plan = selectChecks({ changedFiles: [file], packageEntryFiles: model.packageEntryFiles }); - const routedTo = plan.checks.filter((id) => needs.has(id)); - const needsLane = plan.failOpen || routedTo.length > 0; - const starts = triggersOnPath(lane, file); - if (needsLane && !starts && !ignoredByExactName(lane, file)) { - const why = plan.failOpen - ? `fails open on it (${plan.failOpenReasons.map((reason) => reason.rule).join(', ')})` - : `routes it to ${routedTo.map((id) => `"${id}"`).join(', ')}`; - failures.push( - failure( - `${lane.workflow} ignores ${file}, but the selector ${why}. Remove the ignore entry.`, - ), - ); - continue; - } - if (needsLane || !starts) continue; - const surface = isDeviceLaneSurface(file) ? deviceLanesFor(file) : null; - const claim = isUnitTest(file) - ? 'a unit test no device lane runs' - : surface && !surface.lanes.some((id: CheckId) => needs.has(id)) - ? `${surface.leaf}-owned (lanes: ${surface.lanes.join(', ') || 'none'})` - : null; - if (claim === null) continue; - failures.push( - failure( - `${lane.workflow} starts on ${file}, which the selector classifies as ${claim}. ` + - `Add it to paths-ignore, or the routing claim is false for that path.`, - ), - ); - } - return failures; + return [...model.trackedFiles] + .sort() + .filter((file) => !isDocs(file)) + .flatMap((file) => { + const starts = triggersOnPath(lane, file); + const why = mustStart(model, needs, file); + if (why !== null) { + return starts || ignoredByExactName(lane, file) + ? [] + : [ + failure( + `${lane.workflow} ignores ${file}, but the selector ${why}. Remove the ignore entry.`, + ), + ]; + } + const claim = starts ? mustNotStart(needs, file) : null; + return claim === null + ? [] + : [ + failure( + `${lane.workflow} starts on ${file}, which the selector classifies as ${claim}. ` + + `Add it to paths-ignore, or the routing claim is false for that path.`, + ), + ]; + }); } export function routing(model: Model, routedLanes: readonly RoutedLane[]): RoutingFailure[] { From 2e2d23aee9427aab4b3f702cb4a6198b099806d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 18 Aug 2026 19:31:08 +0200 Subject: [PATCH 3/5] ci: name parked checks in check:affected --run skips --- scripts/check-affected/run.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/scripts/check-affected/run.ts b/scripts/check-affected/run.ts index c01197c9c..469a0e7f7 100644 --- a/scripts/check-affected/run.ts +++ b/scripts/check-affected/run.ts @@ -19,6 +19,7 @@ import { resolveCommand, type CheckSpec, } from './checks.ts'; +import { MANUAL_ONLY_OWNERS } from '../gate/declarations.ts'; import { loadModel, owningLanes } from '../gate/model.ts'; import { ALL_CHECKS, selectChecks, type CheckId, type CheckPlan } from './model.ts'; @@ -193,8 +194,7 @@ export async function runChecks( const coverageSelected = plan.checks.includes('coverage'); const ciJobs = skipped.length > 0 ? ciJobsByCheck() : new Map(); for (const spec of skipped) { - const jobs = ciJobs.get(spec.id) ?? []; - process.stdout.write(`\n[skip] ${spec.id} — GitHub-authoritative (jobs: ${jobs.join(', ')})\n`); + process.stdout.write(`\n[skip] ${spec.id} — ${describeOwner(spec.id, ciJobs)}\n`); } for (const spec of runnable) { if (isCoveredByAffectedCoverage(spec, coverageSelected)) { @@ -215,6 +215,14 @@ export async function runChecks( return 0; } +// Where a check the local run skips is authoritative. A parked check has no automatic +// lane; say so instead of printing an empty job list. +function describeOwner(id: CheckId, ciJobs: ReadonlyMap): string { + const parked = MANUAL_ONLY_OWNERS[id]; + if (parked) return `parked, workflow_dispatch only (${parked.lane})`; + return `GitHub-authoritative (jobs: ${(ciJobs.get(id) ?? []).join(', ')})`; +} + function isCoveredByAffectedCoverage(spec: CheckSpec, coverageSelected: boolean): boolean { return ( coverageSelected && From 04920732af6993eee1c916d6e7b29d697a121c1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 18 Aug 2026 20:01:17 +0200 Subject: [PATCH 4/5] ci: bound the routed-lane exemption to sibling workflows (review of #1857) The exact-name .github exemption was unbounded: naming the lane's own setup-apple-runner-build or boot-ios-test-simulator action skipped the lane that runs them and the manifest stayed green. Lane now carries the transitive composite-action closure plus its own workflow file (Lane.uses, same walk declaredGates does), and the exemption refuses anything in it. Also: an unowned path under an ignored root (a non-TS fixture under a family root) asked for the ignore entry to be removed, which would un-route every sibling in that tree; it now asks for a selector owner. Both cases pinned, both proven red against the pre-fix code. Documents GitHub's 300-changed-file path-filter limit in docs/agents/testing.md. --- docs/agents/testing.md | 7 ++++++ scripts/gate/routing.test.ts | 38 +++++++++++++++++++++++++++++++ scripts/gate/routing.ts | 43 +++++++++++++++++++++++++----------- scripts/gate/workflows.ts | 27 ++++++++++++++++++++++ 4 files changed, 102 insertions(+), 13 deletions(-) diff --git a/docs/agents/testing.md b/docs/agents/testing.md index 63beb623b..44d5e6841 100644 --- a/docs/agents/testing.md +++ b/docs/agents/testing.md @@ -305,6 +305,13 @@ device-lane surface or as a unit test must not (`scripts/gate/routing.ts`). GitH `paths-ignore` before a runner is allocated, so this is routing with no job on the critical path; the assertion is what keeps the hand-written glob list a derived artifact. +Two limits of the mechanism, both inherent to `paths-ignore` rather than to the assertion: +GitHub's path filters examine only the **first 300 changed files**, so a PR larger than that can +skip a routed lane on the strength of its first 300 paths alone (`push` to `main` has no filter +and is the backstop); and a lane may name a *sibling* workflow file exactly to say it does not +use it, but never a file in its own `uses:` closure — the composite actions its steps run, plus +its own definition — which the assertion refuses. + ## Mutation report over decision kernels Mutation score is the mechanical answer to "is this test load-bearing or decorative". A full-suite diff --git a/scripts/gate/routing.test.ts b/scripts/gate/routing.test.ts index 30a3128f5..a3229c495 100644 --- a/scripts/gate/routing.test.ts +++ b/scripts/gate/routing.test.ts @@ -85,6 +85,44 @@ test('a .github path ignored by exact name stays the workflow’s own call', () assert.deepEqual(messages(model), []); }); +// Review finding on #1857: the exact-name exemption was unbounded, so naming the lane's OWN +// build and boot actions skipped the lane that runs them and the manifest stayed green. The +// exemption now stops at `lane.uses` — the transitive composite-action closure plus the +// workflow file — and these are the reviewer's two planted cases. +test('the exemption cannot name an action the lane itself runs', () => { + for (const own of [ + '.github/actions/setup-apple-runner-build/action.yml', + '.github/actions/boot-ios-test-simulator/action.yml', + '.github/actions/run-gate/action.yml', // reached only through a composite action + '.github/workflows/ios.yml', // the lane's own definition + ]) { + assert.ok(iosLane.uses.includes(own), `${own} must be in the lane's uses closure`); + const model = withIos((lane) => ({ ...lane, pathsIgnore: [...lane.pathsIgnore, own] })); + assert.ok( + messages(model).some( + (message) => message.includes(`ignores ${own}`) && /fails open/.test(message), + ), + `naming ${own} exactly must still fail the routing assertion`, + ); + } +}); + +// The trap fires correctly for a tracked non-TS file under an ignored family root, but the +// remedy is not "remove the ignore entry" — that would un-route every sibling `.ts` in the +// tree. The selector gap is the fix, and the message has to say so. +test('an unowned path under an ignored root asks for an owner, not for the entry’s removal', () => { + const planted = 'src/platforms/android/probe-fixture.json'; + const model = { + ...base, + trackedFiles: new Set([...base.trackedFiles, planted]), + }; + const found = routing(model, ROUTED_LANES).map((failure) => failure.message); + assert.equal(found.length, 1, found.join('\n')); + assert.match(found[0] ?? '', /fails open on it \(ambiguous-path\)/); + assert.match(found[0] ?? '', /Give it an owning check in scripts\/check-affected\//); + assert.ok(!/Remove the ignore entry/.test(found[0] ?? '')); +}); + test('dropping a family root from the ignore list fails the routing claim for that tree', () => { const model = withIos((lane) => ({ ...lane, diff --git a/scripts/gate/routing.ts b/scripts/gate/routing.ts index b4ed79ff3..de641c511 100644 --- a/scripts/gate/routing.ts +++ b/scripts/gate/routing.ts @@ -22,20 +22,41 @@ function failure(message: string): RoutingFailure { // A `.github/**` path the lane ignores by its exact name is the workflow's own knowledge of a // sibling workflow it does not use (deploy, docs preview); the selector's fail-open on -// `.github/**` is about *local* checks and cannot see that. A glob there is not exempt: it -// could hide the composite action the lane itself runs. -function ignoredByExactName(lane: Lane, file: string): boolean { - return file.startsWith('.github/') && lane.pathsIgnore.includes(file); +// `.github/**` is about *local* checks and cannot see that. +// +// The exemption stops at the lane's own machinery, and that is enforced rather than asserted in +// prose: `lane.uses` is the transitive closure of the composite actions the job's steps run, +// plus the workflow file itself, so naming `setup-apple-runner-build/action.yml` or +// `boot-ios-test-simulator/action.yml` exactly is refused the way a glob is. Reviewer planted +// both and the manifest stayed green before this check existed. +function exemptSiblingWorkflow(lane: Lane, file: string): boolean { + return ( + file.startsWith('.github/') && lane.pathsIgnore.includes(file) && !lane.uses.includes(file) + ); } -// Why the selector says the lane must start on `file`, or null when it need not. +// Why the selector says the lane must start on `file`, and what to do about it, or null when +// the lane need not start. The remedy differs by cause: a path the selector *routes* to the +// lane, or fails open on because it is tooling, is one the ignore list must not name. A path it +// fails open on because it has no owner at all (`unknown-path`/`ambiguous-path` — a fixture +// under a family root, say) is a selector gap: deleting the ignore entry that happens to match +// it would un-route every sibling in that tree, which is the opposite of the fix. +const UNOWNED_RULES = new Set(['unknown-path', 'ambiguous-path']); + function mustStart(model: Model, needs: ReadonlySet, file: string): string | null { const plan = selectChecks({ changedFiles: [file], packageEntryFiles: model.packageEntryFiles }); if (plan.failOpen) { - return `fails open on it (${plan.failOpenReasons.map((reason) => reason.rule).join(', ')})`; + const rules = plan.failOpenReasons.map((reason) => reason.rule); + const remedy = rules.every((rule) => UNOWNED_RULES.has(rule)) + ? 'Give it an owning check in scripts/check-affected/ — the ignore entry that matches it ' + + 'is load-bearing for the rest of that tree' + : 'Remove the ignore entry'; + return `fails open on it (${rules.join(', ')}). ${remedy}.`; } const routedTo = plan.checks.filter((id) => needs.has(id)); - return routedTo.length > 0 ? `routes it to ${routedTo.map((id) => `"${id}"`).join(', ')}` : null; + return routedTo.length > 0 + ? `routes it to ${routedTo.map((id) => `"${id}"`).join(', ')}. Remove the ignore entry.` + : null; } // How the selector classifies a path the lane need not start, or null when it makes no claim @@ -64,13 +85,9 @@ function routingFor(model: Model, routed: RoutedLane): RoutingFailure[] { const starts = triggersOnPath(lane, file); const why = mustStart(model, needs, file); if (why !== null) { - return starts || ignoredByExactName(lane, file) + return starts || exemptSiblingWorkflow(lane, file) ? [] - : [ - failure( - `${lane.workflow} ignores ${file}, but the selector ${why}. Remove the ignore entry.`, - ), - ]; + : [failure(`${lane.workflow} ignores ${file}, but the selector ${why}`)]; } const claim = starts ? mustNotStart(needs, file) : null; return claim === null diff --git a/scripts/gate/workflows.ts b/scripts/gate/workflows.ts index f6b93b999..b9ae91fc6 100644 --- a/scripts/gate/workflows.ts +++ b/scripts/gate/workflows.ts @@ -20,6 +20,12 @@ export type Lane = { */ readonly triggers: readonly string[]; readonly gates: readonly CheckId[]; + /** + * Repo-relative files of the local composite actions this job's steps use, transitively, + * plus the workflow file itself: the lane's own machinery. Editing one of these changes what + * the lane does, so the lane cannot claim to be unaffected by it (`scripts/gate/routing.ts`). + */ + readonly uses: readonly string[]; readonly verbatim: readonly string[]; readonly paths: readonly string[]; readonly pathsIgnore: readonly string[]; @@ -88,6 +94,26 @@ function declaredGates( return gates; } +// The transitive closure of local composite actions the steps use, as repo-relative action +// files. Same walk `declaredGates` performs, kept separate because it answers a different +// question: not "which gate does this lane declare" but "which files ARE this lane". +function localActionFiles( + steps: readonly RawStep[], + root: string, + chain: readonly string[] = [], +): string[] { + const files: string[] = []; + for (const step of steps) { + const action = readLocalAction(step.uses, root); + if (!action || !step.uses || chain.includes(step.uses)) continue; + files.push( + path.posix.join(step.uses.slice(2), 'action.yml'), + ...localActionFiles(action.runs?.steps ?? [], root, [...chain, step.uses]), + ); + } + return files; +} + function triggerPaths(on: Record) { const pr = on.pull_request ?? {}; return { paths: pr.paths ?? [], pathsIgnore: pr['paths-ignore'] ?? [] }; @@ -111,6 +137,7 @@ function workflowLanes( qualifying, triggers: Object.keys(on), gates: [...new Set(declaredGates(job.steps ?? [], root))], + uses: [...new Set([`.github/workflows/${file}`, ...localActionFiles(job.steps ?? [], root)])], verbatim: (job.steps ?? []).flatMap((step) => typeof step.run === 'string' ? verbatimScripts(step.run, scripts) : [], ), From 17a762f272ad4ddbe23a6ae004ced11e3685d869 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 19 Aug 2026 16:59:23 +0200 Subject: [PATCH 5/5] ci: close the routed-lane exemption over composite-action support files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lane.uses recorded only each composite action's action.yml, so a support file the descriptor executes was exemptible as if it were an unrelated sibling workflow: ios.yml uses setup-fixture-app, whose action.yml runs "$GITHUB_ACTION_PATH/fetch-artifact.sh", and that script runs its siblings resolve-artifact-name.sh and trusted-artifact.mjs — references that exist only inside shell, one level past anything YAML parsing sees. The closure unit is the action's directory now. It needs no shell model and cannot miss a file however deep the reference chain runs; the coarseness is harmless because a file in an action's own directory belongs to that action. All three files pinned, red against the descriptor-only closure. --- scripts/gate/routing.test.ts | 23 +++++++++++++++++++++++ scripts/gate/workflows.ts | 26 +++++++++++++++++++++++--- 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/scripts/gate/routing.test.ts b/scripts/gate/routing.test.ts index a3229c495..9b654de9d 100644 --- a/scripts/gate/routing.test.ts +++ b/scripts/gate/routing.test.ts @@ -107,6 +107,29 @@ test('the exemption cannot name an action the lane itself runs', () => { } }); +// Second review finding on #1857, one level past the first: `Lane.uses` recorded only the +// composite actions' `action.yml` descriptors, so a support file the descriptor *executes* +// (`bash "$GITHUB_ACTION_PATH/fetch-artifact.sh"`) was exemptible as if it were an unrelated +// sibling workflow. The closure is the action's directory now, which also covers the two files +// referenced no closer than inside that shell script. +test('the exemption cannot name a support file of an action the lane runs', () => { + const support = [ + '.github/actions/setup-fixture-app/fetch-artifact.sh', // named by the action.yml + '.github/actions/setup-fixture-app/resolve-artifact-name.sh', // named only inside that script + '.github/actions/setup-fixture-app/trusted-artifact.mjs', // likewise + ]; + for (const file of support) { + assert.ok(iosLane.uses.includes(file), `${file} must be in the lane's uses closure`); + const model = withIos((lane) => ({ ...lane, pathsIgnore: [...lane.pathsIgnore, file] })); + assert.ok( + messages(model).some( + (message) => message.includes(`ignores ${file}`) && /fails open/.test(message), + ), + `naming ${file} exactly must still fail the routing assertion`, + ); + } +}); + // The trap fires correctly for a tracked non-TS file under an ignored family root, but the // remedy is not "remove the ignore entry" — that would un-route every sibling `.ts` in the // tree. The selector gap is the fix, and the message has to say so. diff --git a/scripts/gate/workflows.ts b/scripts/gate/workflows.ts index b9ae91fc6..42ca5a013 100644 --- a/scripts/gate/workflows.ts +++ b/scripts/gate/workflows.ts @@ -94,9 +94,17 @@ function declaredGates( return gates; } -// The transitive closure of local composite actions the steps use, as repo-relative action -// files. Same walk `declaredGates` performs, kept separate because it answers a different +// Every file of every local composite action the steps use, transitively, as repo-relative +// paths. Same walk `declaredGates` performs, kept separate because it answers a different // question: not "which gate does this lane declare" but "which files ARE this lane". +// +// The unit is the action's DIRECTORY, not its `action.yml`. A composite action's descriptor is +// only its entry point: `setup-fixture-app/action.yml` runs +// `bash "$GITHUB_ACTION_PATH/fetch-artifact.sh"`, and that script in turn runs its siblings +// `resolve-artifact-name.sh` and `trusted-artifact.mjs` — references that exist only inside +// shell, one level past anything YAML parsing can see. Collecting the directory needs no shell +// model and cannot miss a file however deep the chain goes; the cost is coarseness, which is +// harmless here because a file inside an action's own directory belongs to that action. function localActionFiles( steps: readonly RawStep[], root: string, @@ -106,14 +114,26 @@ function localActionFiles( for (const step of steps) { const action = readLocalAction(step.uses, root); if (!action || !step.uses || chain.includes(step.uses)) continue; + const dir = step.uses.slice(2); files.push( - path.posix.join(step.uses.slice(2), 'action.yml'), + ...filesUnder(path.join(root, dir)).map((file) => path.posix.join(dir, file)), ...localActionFiles(action.runs?.steps ?? [], root, [...chain, step.uses]), ); } return files; } +/** Every file under `dir`, recursively, as paths relative to it. */ +function filesUnder(dir: string): string[] { + if (!fs.existsSync(dir)) return []; + return fs + .readdirSync(dir, { withFileTypes: true, recursive: true }) + .filter((entry) => entry.isFile()) + .map((entry) => + path.posix.join(path.relative(dir, entry.parentPath).split(path.sep).join('/'), entry.name), + ); +} + function triggerPaths(on: Record) { const pr = on.pull_request ?? {}; return { paths: pr.paths ?? [], pathsIgnore: pr['paths-ignore'] ?? [] };