diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f21753d24e..9ec149dca5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,8 +22,13 @@ concurrency: cancel-in-progress: true jobs: - ios-runner-swift-compat: - name: iOS Runner Swift Compatibility + # Two single-`rg`-assertion jobs (formerly `ios-runner-swift-compat`, + # `no-test-di-seams`, added independently in b79bd8601 / 9eb060406) folded + # into steps here: each was checkout + one grep, paying full job + # scheduling/checkout overhead and its own PR status-check line for what is + # a single assertion. Each step keeps its own failure message. See #1462. + static-checks: + name: Static Checks runs-on: ubuntu-latest timeout-minutes: 10 steps: @@ -37,6 +42,13 @@ jobs: exit 1 fi + - name: Fail if test-only DI seams reappear in production code + run: | + if rg '\?\s*:\s*typeof\s+' src/ --glob '!**/__tests__/**' --glob '!*.test.ts'; then + echo "Found test-only DI seams (optional typeof params) in production code." + exit 1 + fi + swift-runner-unit-compile: name: Swift Runner Unit Compile runs-on: macos-26 @@ -57,21 +69,6 @@ jobs: xcuitest-platform: macos xcuitest-destination: platform=macOS,arch=arm64 - no-test-di-seams: - name: No test-only DI seams - runs-on: ubuntu-latest - timeout-minutes: 5 - steps: - - name: Checkout - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - - - name: Fail if test-only DI seams reappear in production code - run: | - if rg '\?\s*:\s*typeof\s+' src/ --glob '!**/__tests__/**' --glob '!*.test.ts'; then - echo "Found test-only DI seams (optional typeof params) in production code." - exit 1 - fi - lint: name: Lint & Format runs-on: ubuntu-latest @@ -110,16 +107,14 @@ jobs: # Generalizes the former inline commands/-import grep into a structured # import-direction lint over the resolved graph. See scripts/layering/check.ts # and CONTEXT.md (Architecture: folder DAG + layering lint). - run: | - node --experimental-strip-types --test scripts/layering/model.test.ts scripts/layering/zone-policy.test.ts - node --experimental-strip-types scripts/layering/check.ts + run: pnpm check:layering - name: Check the depgraph report agrees with the gate # scripts/depgraph reads the same model as the gate, so its inversion count must # reproduce TYPE_INVERSION_BASELINE. Free two-sources check: if the tree changes # and only one side is updated, this fails and names the difference. Runs here # rather than in its own job so the two can never be green independently. - run: node --experimental-strip-types --test scripts/depgraph/model.test.ts + run: pnpm depgraph:test affected-selector: name: Affected-check Selector @@ -135,10 +130,12 @@ jobs: install-deps: false # The selector is fail-open and advisory (GitHub CI stays authoritative), - # so the gate only guards the derivation model. Invoked directly with no - # deps, mirroring the layering guard. + # so the gate only guards the derivation model. Invoked with no deps, + # mirroring the layering guard. R8 (scripts/layering/zero-dep-jobs.ts) + # resolves this bare pnpm script name back to package.json's own command + # to find its entry scripts, so this is not a `pnpm install` for the job. - name: Check affected-selector model - run: node --experimental-strip-types --test scripts/check-affected/model.test.ts scripts/check-affected/run.test.ts + run: pnpm check:affected:test maestro-conformance: name: Maestro Conformance Oracle diff --git a/.github/workflows/conformance-differential.yml b/.github/workflows/conformance-differential.yml index 10e78e0e6a..44a7b3d1c5 100644 --- a/.github/workflows/conformance-differential.yml +++ b/.github/workflows/conformance-differential.yml @@ -126,7 +126,7 @@ jobs: # burning its budget) — outcome parity alone cannot see that. - name: Run differential run: | - node --experimental-strip-types scripts/maestro-conformance/differential/run.ts \ + pnpm maestro:conformance:differential \ --platform ios \ --out-dir "${{ github.workspace }}/.tmp/conformance-differential" \ --trace-root "${{ github.workspace }}/.agent-device" \ diff --git a/.github/workflows/conformance-regenerate.yml b/.github/workflows/conformance-regenerate.yml index abeaaebc9e..9f6c753fda 100644 --- a/.github/workflows/conformance-regenerate.yml +++ b/.github/workflows/conformance-regenerate.yml @@ -49,7 +49,7 @@ jobs: # Verifies the pinned jar SHA-256s against pinned-upstream.json, runs the # harness over the corpus, and rewrites fixtures/ + corpus/manifest.json. - name: Regenerate fixtures from pinned upstream - run: node scripts/maestro-conformance/regenerate.mjs + run: pnpm maestro:conformance:regenerate - name: Fail if regeneration changed anything run: | diff --git a/.github/workflows/linux.yml b/.github/workflows/linux.yml index de5f4cd301..7e7d71044a 100644 --- a/.github/workflows/linux.yml +++ b/.github/workflows/linux.yml @@ -104,7 +104,7 @@ jobs: - name: Run Linux replay smoke test run: | pnpm clean:daemon - node --experimental-strip-types src/bin.ts test test/integration/replays/linux \ + pnpm test:replay:linux \ --retries 2 \ --report-junit test/artifacts/replays-linux.junit.xml diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml index fafdcd55c1..a03691ebfb 100644 --- a/.github/workflows/macos.yml +++ b/.github/workflows/macos.yml @@ -49,7 +49,7 @@ jobs: run: pnpm build:macos-helper - name: Run macOS integration test - run: node --experimental-strip-types src/bin.ts test test/integration/replays/macos --retries 2 --report-junit test/artifacts/replays-macos.junit.xml + run: pnpm test:replay:macos --retries 2 --report-junit test/artifacts/replays-macos.junit.xml - name: Upload macOS artifacts if: always() diff --git a/.github/workflows/perf-nightly.yml b/.github/workflows/perf-nightly.yml index 3ccf4213db..8ce31e9c08 100644 --- a/.github/workflows/perf-nightly.yml +++ b/.github/workflows/perf-nightly.yml @@ -68,7 +68,7 @@ jobs: - name: Run iOS command perf benchmark run: | - node --experimental-strip-types scripts/perf/run.ts \ + pnpm perf \ --platform ios \ --device "iPhone 17 Pro" \ --n "$PERF_ROUNDS" --warmup 1 \ @@ -112,7 +112,7 @@ jobs: adb -s emulator-5554 shell settings put global window_animation_scale 0 || true; adb -s emulator-5554 shell settings put global transition_animation_scale 0 || true; adb -s emulator-5554 shell settings put global animator_duration_scale 0 || true; - node --experimental-strip-types scripts/perf/run.ts --platform android --serial emulator-5554 --n "$PERF_ROUNDS" --warmup 1 --out-dir "$GITHUB_WORKSPACE/perf-results" + pnpm perf:android --serial emulator-5554 --n "$PERF_ROUNDS" --warmup 1 --out-dir "$GITHUB_WORKSPACE/perf-results" - name: Upload Android perf report if: always() diff --git a/.github/workflows/publish-mcp-registry.yml b/.github/workflows/publish-mcp-registry.yml index 666e147409..db90001062 100644 --- a/.github/workflows/publish-mcp-registry.yml +++ b/.github/workflows/publish-mcp-registry.yml @@ -54,6 +54,10 @@ jobs: echo "version=$VERSION" >> "$GITHUB_OUTPUT" shell: bash + # This job provisions pnpm via plain actions/setup-node, not the repo's + # ./.github/actions/setup-node-pnpm — so pnpm itself is never installed here, + # and `pnpm check:mcp-metadata` would fail before publishing. Keep the direct + # node invocation; it duplicates check:mcp-metadata's command deliberately. - name: Check MCP metadata sync run: node scripts/sync-mcp-metadata.mjs --check diff --git a/.github/workflows/replays-nightly.yml b/.github/workflows/replays-nightly.yml index f517043b1a..b4f063eac4 100644 --- a/.github/workflows/replays-nightly.yml +++ b/.github/workflows/replays-nightly.yml @@ -114,7 +114,7 @@ jobs: profile: pixel_7 target: google_apis_playstore emulator-options: -no-window -gpu swiftshader_indirect -no-snapshot -noaudio -no-boot-anim -no-metrics - script: node --experimental-strip-types src/bin.ts test test/integration/replays/android --retries 2 --report-junit test/artifacts/replays-android.junit.xml + script: pnpm test:replay:android --retries 2 --report-junit test/artifacts/replays-android.junit.xml - name: Upload Android artifacts if: always() @@ -168,7 +168,7 @@ jobs: pnpm clean:daemon - name: Run iOS simulator replay suite - run: node --experimental-strip-types src/bin.ts test test/integration/replays/ios/simulator --udid "${{ steps.ios-simulator.outputs.simulator-udid }}" --retries 2 --artifacts-dir test/artifacts/replays-ios-simulator --report-junit test/artifacts/replays-ios-simulator.junit.xml + run: pnpm test:replay:ios --udid "${{ steps.ios-simulator.outputs.simulator-udid }}" --retries 2 --artifacts-dir test/artifacts/replays-ios-simulator --report-junit test/artifacts/replays-ios-simulator.junit.xml - name: Fetch current fixture app id: fixture-app @@ -195,7 +195,7 @@ jobs: if: env.IOS_UDID != '' env: IOS_UDID: ${{ vars.IOS_UDID }} - run: node --experimental-strip-types src/bin.ts test test/integration/replays/ios/device --udid "$IOS_UDID" --retries 2 --artifacts-dir test/artifacts/replays-ios-device --report-junit test/artifacts/replays-ios-device.junit.xml + run: pnpm test:replay:ios-device --udid "$IOS_UDID" --retries 2 --artifacts-dir test/artifacts/replays-ios-device --report-junit test/artifacts/replays-ios-device.junit.xml - name: Upload iOS artifacts if: always() @@ -232,7 +232,7 @@ jobs: run: pnpm build:macos-helper - name: Run macOS replay suite - run: node --experimental-strip-types src/bin.ts test test/integration/replays/macos --retries 2 --report-junit test/artifacts/replays-macos.junit.xml + run: pnpm test:replay:macos --retries 2 --report-junit test/artifacts/replays-macos.junit.xml - name: Upload macOS artifacts if: always() diff --git a/.github/workflows/size.yml b/.github/workflows/size.yml index 974161e736..150d3e11db 100644 --- a/.github/workflows/size.yml +++ b/.github/workflows/size.yml @@ -71,7 +71,7 @@ jobs: git checkout --detach "${{ github.event.pull_request.head.sha }}" pnpm install --frozen-lockfile pnpm build - node scripts/size-report.mjs \ + pnpm size \ --compare /tmp/agent-device-size-base.json \ --startup-runs 7 \ --json .tmp/size-report.json \ @@ -84,4 +84,4 @@ jobs: env: GITHUB_TOKEN: ${{ github.token }} GITHUB_PR_NUMBER: ${{ github.event.pull_request.number }} - run: node scripts/size-report.mjs --post-comment .tmp/size-report.md + run: pnpm size --post-comment .tmp/size-report.md diff --git a/scripts/layering/check.ts b/scripts/layering/check.ts index 098cfaf907..241ab642c2 100644 --- a/scripts/layering/check.ts +++ b/scripts/layering/check.ts @@ -400,9 +400,13 @@ function checkZeroDepJobs(): Violation[] { .split('\n') .filter(Boolean), ); + const packageJson = JSON.parse(fs.readFileSync(path.join(repoRoot, 'package.json'), 'utf8')) as { + scripts?: Record; + }; + const packageScripts = new Map(Object.entries(packageJson.scripts ?? {})); const violations: Violation[] = []; - for (const job of zeroDepJobs(workflows, fileExists)) { + for (const job of zeroDepJobs(workflows, fileExists, packageScripts)) { // Fail closed: a zero-dep job whose commands the entry scan cannot recognize would // otherwise be silently exempt from the rule it is the whole reason for. if (job.entries.length === 0) { diff --git a/scripts/layering/model.test.ts b/scripts/layering/model.test.ts index 6d10b243b7..1544a92695 100644 --- a/scripts/layering/model.test.ts +++ b/scripts/layering/model.test.ts @@ -332,6 +332,132 @@ test('a job with no recognizable entry script is reported rather than exempted', assert.deepEqual(jobs, [{ workflow: 'w.yml', job: 'zero-dep', entries: [] }]); }); +test('a bare pnpm script name resolves through package.json to its entry scripts', () => { + // The workflow names no path at all — only the pnpm script name package.json maps to the + // real command. A zero-dep job may call its script this way (#1462) without R8 losing the + // entries it needs to check: the resolution reads the same paths out of the mapped command. + const workflow = ` +name: CI +jobs: + zero-dep: + steps: + - uses: ./.github/actions/setup-node-pnpm + with: + install-deps: false + - run: pnpm check:affected:test +`; + const present = new Set([ + 'scripts/check-affected/model.test.ts', + 'scripts/check-affected/run.test.ts', + ]); + const packageScripts = new Map([ + [ + 'check:affected:test', + 'node --experimental-strip-types --test scripts/check-affected/model.test.ts scripts/check-affected/run.test.ts', + ], + ]); + const jobs = zeroDepJobs( + new Map([['w.yml', workflow]]), + (file) => present.has(file), + packageScripts, + ); + assert.deepEqual(jobs, [ + { + workflow: 'w.yml', + job: 'zero-dep', + entries: ['scripts/check-affected/model.test.ts', 'scripts/check-affected/run.test.ts'], + }, + ]); +}); + +test('a resolved script that itself runs a named script is expanded too', () => { + // A chained alias (`outer` runs `pnpm inner`) is one hop further from the workflow text + // than the direct case above. If resolution stopped at one level, inner's entry would be + // invisible to R8 even though the job genuinely depends on it at runtime. + const workflow = ` +name: CI +jobs: + zero-dep: + steps: + - uses: ./.github/actions/setup-node-pnpm + with: + install-deps: false + - run: pnpm outer +`; + const present = new Set(['scripts/outer/entry.ts', 'scripts/inner/entry.ts']); + const packageScripts = new Map([ + ['outer', 'node scripts/outer/entry.ts && pnpm inner'], + ['inner', 'node scripts/inner/entry.ts'], + ]); + const jobs = zeroDepJobs( + new Map([['w.yml', workflow]]), + (file) => present.has(file), + packageScripts, + ); + assert.deepEqual(jobs, [ + { + workflow: 'w.yml', + job: 'zero-dep', + entries: ['scripts/inner/entry.ts', 'scripts/outer/entry.ts'], + }, + ]); +}); + +test('an alias cycle does not hang, and still collects every non-cyclic entry', () => { + // `a` runs `pnpm b`, `b` runs `pnpm a` back — resolution must stop re-expanding a name it + // has already walked on this chain, not recurse until the stack overflows. Each script's + // own direct entry is still found before the cycle closes. + const workflow = ` +name: CI +jobs: + zero-dep: + steps: + - uses: ./.github/actions/setup-node-pnpm + with: + install-deps: false + - run: pnpm a +`; + const present = new Set(['scripts/a/entry.ts', 'scripts/b/entry.ts']); + const packageScripts = new Map([ + ['a', 'node scripts/a/entry.ts && pnpm b'], + ['b', 'node scripts/b/entry.ts && pnpm a'], + ]); + const jobs = zeroDepJobs( + new Map([['w.yml', workflow]]), + (file) => present.has(file), + packageScripts, + ); + assert.deepEqual(jobs, [ + { + workflow: 'w.yml', + job: 'zero-dep', + entries: ['scripts/a/entry.ts', 'scripts/b/entry.ts'], + }, + ]); +}); + +test('a pnpm word that names no real package.json script resolves to nothing', () => { + // `pnpm install` (or any other non-script pnpm subcommand) must not be treated as a script + // name just because it follows `pnpm` — it is absent from packageScripts, same as a shell + // word that merely looks like a path is absent from the tree. + const workflow = ` +name: CI +jobs: + zero-dep: + steps: + - uses: ./.github/actions/setup-node-pnpm + with: + install-deps: false + - run: pnpm install --frozen-lockfile +`; + const jobs = zeroDepJobs( + new Map([['w.yml', workflow]]), + () => true, + new Map([['check:affected:test', 'node scripts/check-affected/run.test.ts']]), + ); + assert.deepEqual(jobs, [{ workflow: 'w.yml', job: 'zero-dep', entries: [] }]); +}); + test('a package import anywhere in a zero-dep closure is rejected, builtins are not', () => { const tree = new Map([ [ @@ -394,10 +520,13 @@ test("the repo's own zero-dep jobs resolve without node_modules", () => { : null; }; const exists = (file: string): boolean => read(file) !== null; + const packageJson = JSON.parse(read('package.json')!) as { scripts?: Record }; + const packageScripts = new Map(Object.entries(packageJson.scripts ?? {})); const jobs = zeroDepJobs( new Map([['.github/workflows/ci.yml', read('.github/workflows/ci.yml')!]]), exists, + packageScripts, ); assert.ok(jobs.length > 0, 'expected ci.yml to still declare at least one zero-dep job'); for (const job of jobs) { diff --git a/scripts/layering/zero-dep-jobs.ts b/scripts/layering/zero-dep-jobs.ts index 1edf6b2e2f..3af9bc9854 100644 --- a/scripts/layering/zero-dep-jobs.ts +++ b/scripts/layering/zero-dep-jobs.ts @@ -51,6 +51,47 @@ function runBlockEntries(run: string): string[] { ); } +/** + * `pnpm ` invocations in a `run:` block. Matched by shape only — the caller resolves + * each name against package.json's own scripts, so a word that is not a real script (`pnpm + * install`, `pnpm build`) drops out there rather than here. + */ +function pnpmScriptNames(run: string): string[] { + return [...run.matchAll(/(?:^|\s)pnpm\s+(?:run\s+)?([\w:.-]+)/g)].map((match) => match[1]!); +} + +/** + * Entry paths reachable from a `run:` block, following `pnpm ` aliases into + * package.json and back into `runBlockEntries` however deep the chain goes — a script + * that itself runs another named script must not hide that script's entries from R8. + * `chain` is the set of script names already expanded on this path: an alias that names + * one of them is a cycle, and is dropped rather than walked again, so a cycle contributes + * whatever entries its non-cyclic edges found instead of recursing forever. + */ +function resolveRunEntries( + run: string, + packageScripts: ReadonlyMap, + fileExists: (file: string) => boolean, + entries: Set, + chain: ReadonlySet = new Set(), +): void { + for (const entry of runBlockEntries(run)) { + if (fileExists(entry)) entries.add(entry); + } + for (const scriptName of pnpmScriptNames(run)) { + if (chain.has(scriptName)) continue; + const resolved = packageScripts.get(scriptName); + if (resolved === undefined) continue; + resolveRunEntries( + resolved, + packageScripts, + fileExists, + entries, + new Set([...chain, scriptName]), + ); + } +} + function stepsOf(job: unknown): Record[] { if (job === null || typeof job !== 'object') return []; const steps = (job as Record)['steps']; @@ -72,10 +113,21 @@ function skipsInstall(step: Record): boolean { /** * Every job across `workflows` that sets `install-deps: false`, with the entry scripts its * steps invoke. `fileExists` filters candidate paths down to files that are really there. + * + * `packageScripts` resolves a bare `pnpm ` invocation back to the command string + * package.json names it, so a zero-dep job may call its script by its pnpm name — R8 reads + * the same entry paths out of the resolved command rather than requiring the literal path + * inline in the workflow. A `pnpm` word that names no real script (`pnpm install`) resolves + * to nothing and is silently ignored, same as a shell word that merely looks like a path. + * Resolution recurses through chained aliases (a script that itself runs another named + * script), so a nested entry is never invisible to the closure just because it is one hop + * further away; a cycle in that chain stops re-expanding the repeated name rather than + * recursing forever. */ export function zeroDepJobs( workflows: ReadonlyMap, fileExists: (file: string) => boolean, + packageScripts: ReadonlyMap = new Map(), ): ZeroDepJob[] { const jobs: ZeroDepJob[] = []; for (const [workflow, source] of workflows) { @@ -90,9 +142,7 @@ export function zeroDepJobs( for (const step of steps) { const run = step['run']; if (typeof run !== 'string') continue; - for (const entry of runBlockEntries(run)) { - if (fileExists(entry)) entries.add(entry); - } + resolveRunEntries(run, packageScripts, fileExists, entries); } jobs.push({ workflow, job, entries: [...entries].sort() }); }