refactor(ci): derive the audit list and stop shelling out through bunx

Review pass over the audit runner and the tool guards.

The audit list was hand-maintained alongside package.json with nothing linking
them, and it had already drifted: check:cron-parity exists, passes, and ran in no
CI step at all. The list is now derived from the check:* scripts with an explicit
exclusion map, so a new audit is opted out deliberately rather than forgotten.
That picks up cron-parity — 22 audits now, not 21.

check-realtime-prune-graph.ts still shelled out through `bunx turbo`, the same
pattern that took the bridge audit from 1s to 39s once the audits ran
concurrently. Both now go through scripts/local-bin.ts, which resolves
node_modules/.bin — the same path check:native-typecheck asserts is the native
TypeScript 7 compiler, so the one guarded path is the one that runs.

Audits are spawned as their script rather than `bun run <name>`, which started a
bun process only to read package.json and start a second one.

Tool detection is memoized per process; it was re-spawning python3 on each of the
5 call sites, in every vitest worker. The CI throw is deliberately NOT memoized —
memoizing it would turn every call after the first into a silent skip, which is
the failure mode the guard exists to prevent. Verified it still throws for all
three guarded tests, not just the first.

Also: dropped the environment module from the @sim/testing barrel so
node:child_process stays out of unrelated consumers' module graphs, restored the
per-audit reporting the 21 separate steps used to give (collapsible groups, error
annotations, and a timing table they never had), and trimmed comments that
restated their code or duplicated the runner's own docs.

Author: waleedlatif1

.github/workflows/test-build.yml

@@ -117,20 +117,9 @@ jobs:
       - name: Lint code
         run: bun run lint:check
 
-      # These 21 audits are independent read-only passes over the tree, so running
-      # them as 21 sequential steps spent the whole time waiting on single-threaded
-      # file walks (~28s serial, ~5s at 13-way locally). scripts/run-audits.ts owns
-      # the list; it buffers each audit's output and replays only the failures, so a
-      # red run still names the audit and shows why. Audits that need a git base ref
-      # or write files stay as their own steps below.
-      #
-      # Notable members: the desktop IPC audit complements the bridge audit by
-      # deriving every fact from the source both sides execute, so it has no
-      # snapshot blind spot; the import-specifier audit catches specifiers that only
-      # webpack (which this job uses) resolves and that break every dev server on
-      # Turbopack; and the native-typecheck audit catches a bare `tsc` falling back
-      # to the ~10x slower JavaScript TypeScript 6 compiler, which otherwise still
-      # passes and just burns minutes.
+      # Every zero-argument `check:*` script, run concurrently. The list is derived in
+      # scripts/run-audits.ts, which also writes the per-audit timing table to the job
+      # summary and annotates failures. Audits needing a base ref stay separate below.
       - name: Repo audits
         run: bun run check:audits
 

packages/testing/src/environment/index.ts

@@ -1,75 +1,63 @@
 /**
- * Detects external command-line tools that a handful of tests shell out to.
+ * Detects external command-line tools that a few suites shell out to.
  *
- * A few suites deliberately execute the real thing rather than a mock — the cloud-review
- * helper's path and read-size bounds, and the code-placeholder compiler's generated Python.
- * That is the point of those tests, but it makes them depend on tools the repo does not
- * vendor, and the failure mode is a raw `SyntaxError` or `ENOENT` from a subprocess with
- * nothing tying it back to a missing tool.
- *
- * Locally these report `false` and print one actionable line, so the affected tests skip.
- * Under `CI` they throw instead: a missing tool there means the gate silently stopped
- * covering a security boundary, which is strictly worse than a red build.
+ * Those suites deliberately execute the real thing rather than a mock, so they depend on tools
+ * the repo does not vendor. Locally a missing tool skips them with a reason; under `CI` it
+ * throws, because a silently-skipped security boundary is worse than a red build.
  */
 import { spawnSync } from 'node:child_process'
 
-/**
- * Python 3.12 is the floor, set by PEP 701 f-strings rather than by `match` statements.
- *
- * The compiler suite generates `match` (3.10) but also f-strings that reuse the outer quote
- * and embed `#`. On 3.11 those raise `f-string: unmatched '('` and `f-string expression part
- * cannot include '#'` — exactly the raw SyntaxError this guard exists to prevent — so a 3.10
- * floor would have let two of the three guarded tests through and failed anyway.
- */
-export const MIN_PYTHON: readonly [number, number] = [3, 12]
+/** PEP 701 f-strings set this floor, not the `match` statements (3.10) the suite also generates. */
+const MIN_PYTHON: readonly [number, number] = [3, 12]
 
-/** Reasons to pass to vitest's `ctx.skip(...)` so the report says why, not just that. */
-export const PYTHON_SKIP_REASON = 'needs python3 >= 3.12 (PEP 701 f-strings); macOS ships 3.9'
+/** Reasons for vitest's `ctx.skip(...)`, so the report says why and not just that. */
+export const PYTHON_SKIP_REASON = `needs python3 >= ${MIN_PYTHON.join('.')} (PEP 701 f-strings); macOS ships 3.9`
 export const RIPGREP_SKIP_REASON = 'needs ripgrep (`rg`) on PATH'
 
-const warned = new Set<string>()
-
-function unavailable(tool: string, hint: string): false {
-  if (process.env.CI) {
-    throw new Error(
-      `${tool} is required to run this suite and was not found. CI must never skip these tests — they cover behavior that is only observable by running the real tool. ${hint}`
-    )
-  }
-  if (!warned.has(tool)) {
-    warned.add(tool)
-    console.warn(`[@sim/testing] Skipping tests that require ${tool}. ${hint}`)
+/**
+ * Wraps a probe so it runs at most once per process, warns at most once, and always throws
+ * under CI — memoizing the throw would turn every call after the first into a silent skip.
+ */
+function toolGuard(label: string, detect: () => { ok: boolean; hint: string }): () => boolean {
+  let result: { ok: boolean; hint: string } | undefined
+  let warned = false
+  return () => {
+    result ??= detect()
+    if (result.ok) return true
+    if (process.env.CI) {
+      throw new Error(
+        `${label} is required to run this suite and was not found. CI must never skip these tests — they cover behavior that is only observable by running the real tool. ${result.hint}`
+      )
+    }
+    if (!warned) {
+      warned = true
+      console.warn(`[@sim/testing] Skipping tests that require ${label}. ${result.hint}`)
+    }
+    return false
   }
-  return false
-}
-
-/** Parses `python3 --version`, returning null when the interpreter is missing or unreadable. */
-export function detectPython3(): { major: number; minor: number } | null {
-  const result = spawnSync('python3', ['--version'], { encoding: 'utf8' })
-  const match = /Python (\d+)\.(\d+)/.exec(`${result.stdout ?? ''}${result.stderr ?? ''}`)
-  if (!match) return null
-  return { major: Number(match[1]), minor: Number(match[2]) }
 }
 
 /**
  * True when `python3` resolves to at least {@link MIN_PYTHON}.
  *
- * macOS ships 3.9 as the system `python3` and Homebrew's newer builds are not linked as
- * `python3`, so this is false on a stock Mac even when a modern Python is installed.
+ * False on a stock Mac: macOS ships 3.9 as the system `python3`, and Homebrew's newer builds
+ * are not linked under that name.
  */
-export function hasPython3(): boolean {
-  const version = detectPython3()
+export const hasPython3 = toolGuard(`python3 >= ${MIN_PYTHON.join('.')}`, () => {
+  const probe = spawnSync('python3', ['--version'], { encoding: 'utf8' })
+  const match = /Python (\d+)\.(\d+)/.exec(`${probe.stdout ?? ''}${probe.stderr ?? ''}`)
+  const found = match ? { major: Number(match[1]), minor: Number(match[2]) } : null
   const [minMajor, minMinor] = MIN_PYTHON
-  const label = `python3 >= ${minMajor}.${minMinor}`
-  const hint = `Found ${version ? `${version.major}.${version.minor}` : 'no python3 on PATH'}. Install a newer Python (e.g. \`brew install python@3.13\`) and put it on PATH ahead of /usr/bin.`
-  if (!version) return unavailable(label, hint)
-  if (version.major > minMajor) return true
-  if (version.major === minMajor && version.minor >= minMinor) return true
-  return unavailable(label, hint)
-}
+  return {
+    ok: Boolean(
+      found && (found.major > minMajor || (found.major === minMajor && found.minor >= minMinor))
+    ),
+    hint: `Found ${found ? `${found.major}.${found.minor}` : 'no python3 on PATH'}. Install a newer Python (e.g. \`brew install python@3.13\`) and put it on PATH ahead of /usr/bin.`,
+  }
+})
 
 /** True when `rg` is an executable on PATH. */
-export function hasRipgrep(): boolean {
-  const result = spawnSync('rg', ['--version'], { encoding: 'utf8' })
-  if (result.status === 0) return true
-  return unavailable('ripgrep (`rg`)', 'Install it with `brew install ripgrep`.')
-}
+export const hasRipgrep = toolGuard('ripgrep (`rg`)', () => ({
+  ok: spawnSync('rg', ['--version'], { encoding: 'utf8' }).status === 0,
+  hint: 'Install it with `brew install ripgrep`.',
+}))

packages/testing/src/index.ts

@@ -43,7 +43,6 @@
 
 export * from './assertions'
 export * from './builders'
-export * from './environment'
 export * from './factories'
 export * from './mocks'
 export * from './types'

scripts/check-desktop-bridge-contract.ts

@@ -31,21 +31,14 @@
 import { spawnSync } from 'node:child_process'
 import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'
 import { readFile, writeFile } from 'node:fs/promises'
-import { createRequire } from 'node:module'
 import { tmpdir } from 'node:os'
 import { dirname, join, resolve } from 'node:path'
 import { fileURLToPath } from 'node:url'
 import { formatGeneratedSource } from './format-generated-source'
+import { localBin } from './local-bin'
 
 const SCRIPT_DIR = dirname(fileURLToPath(import.meta.url))
 const ROOT = resolve(SCRIPT_DIR, '..')
-
-/** The native TypeScript 7 compiler entry point, resolved once. */
-const TSC_ENTRY = join(
-  dirname(createRequire(import.meta.url).resolve('typescript/package.json')),
-  'bin',
-  'tsc'
-)
 const BRIDGE_SOURCE_PATH = resolve(ROOT, 'packages/desktop-bridge/src/index.ts')
 const PROTOCOL_SOURCE_PATH = resolve(ROOT, 'packages/browser-protocol/src/index.ts')
 const TERMINAL_PROTOCOL_SOURCE_PATH = resolve(ROOT, 'packages/terminal-protocol/src/index.ts')
@@ -181,12 +174,7 @@ function checkCompatibility(): { compatible: boolean; output: string } {
   try {
     writeFileSync(join(dir, 'compat.ts'), compatSource)
     writeFileSync(join(dir, 'tsconfig.json'), JSON.stringify(tsconfig, null, 2))
-    // Spawned as a resolved module path rather than via `bunx`. `bunx` re-resolves the
-    // package on every call against the shared install cache, which on CI is a network-backed
-    // sticky-disk mount — cheap when this audit runs alone, but it serialized behind the
-    // other audits once they started running concurrently and took this step from 1s to 39s,
-    // making it the entire wall clock of the batch.
-    const result = spawnSync(process.execPath, [TSC_ENTRY, '-p', dir, '--pretty', 'false'], {
+    const result = spawnSync(localBin('tsc'), ['-p', dir, '--pretty', 'false'], {
       cwd: ROOT,
       encoding: 'utf8',
     })

scripts/check-realtime-prune-graph.ts

@@ -3,6 +3,7 @@ import { mkdtemp, readdir, rm, stat } from 'node:fs/promises'
 import { tmpdir } from 'node:os'
 import path from 'node:path'
 import { $ } from 'bun'
+import { localBin } from './local-bin'
 
 const MAX_PRUNED_PACKAGE_COUNT = 25
 
@@ -27,7 +28,7 @@ async function main() {
   const scratch = await mkdtemp(path.join(tmpdir(), 'sim-realtime-prune-'))
   try {
     console.log(`Pruning @sim/realtime into ${scratch}`)
-    await $`bunx turbo prune @sim/realtime --docker --out-dir=${scratch}`.quiet()
+    await $`${localBin('turbo')} prune @sim/realtime --docker --out-dir=${scratch}`.quiet()
 
     const apps = await listPackages(path.join(scratch, 'json', 'apps'))
     const packages = await listPackages(path.join(scratch, 'json', 'packages'))

scripts/local-bin.ts

@@ -0,0 +1,19 @@
+import path from 'node:path'
+
+const ROOT = path.resolve(import.meta.dir, '..')
+
+/**
+ * Absolute path to a locally-installed executable.
+ *
+ * Scripts spawn these instead of shelling out through `bunx`, which re-resolves the package on
+ * every call against the shared install cache — a network-backed sticky-disk mount on CI. That
+ * is cheap for a lone caller and serializes badly once the audits run concurrently: it took the
+ * desktop-bridge audit from 1s to 39s and made it the entire wall clock of the batch.
+ *
+ * `tsc` resolves here to the native TypeScript 7 compiler, which `check:native-typecheck`
+ * asserts — so this is the one path guarded against the bin-shadowing that otherwise silently
+ * selects the ~10x slower JavaScript compiler.
+ */
+export function localBin(name: string): string {
+  return path.join(ROOT, 'node_modules', '.bin', name)
+}

scripts/run-audits.ts

@@ -1,52 +1,65 @@
 #!/usr/bin/env bun
 /**
- * Runs the independent repo audits concurrently.
+ * Runs the repo's independent audits concurrently.
  *
  * Each audit is a self-contained read-only pass over the tree, so running them as 20-odd
  * sequential CI steps spent most of its wall clock waiting on single-threaded file walks.
- * Only audits that need no extra arguments, working directory, or git base ref belong here —
- * the ones that diff against a base ref (block registry, migration safety) or write files
- * (drizzle generate) stay as their own steps.
  *
- * Output is buffered per audit and replayed only for failures, so a green run stays quiet
- * and a red one still shows exactly which audit failed and why.
+ * The list is derived from the `check:*` scripts in package.json rather than restated here,
+ * so a new audit is picked up by default and has to be opted *out* deliberately. The previous
+ * hand-maintained list had already drifted: `check:cron-parity` existed, passed, and ran
+ * nowhere. Audits that need a git base ref or write files stay excluded and keep their own
+ * workflow step.
  */
-const AUDITS = [
-  'check:boundaries',
-  'check:api-validation:strict',
-  'check:desktop-bridge',
-  'check:desktop-ipc',
-  'check:utils',
-  'check:zustand-v5',
-  'check:react-query',
-  'check:client-boundary',
-  'check:bare-icons',
-  'check:icon-paths',
-  'check:realtime-prune',
-  'check:tool-registry-boundary',
-  'check:tool-request-boundary',
-  'check:trigger-block-cycle',
-  'check:import-specifiers',
-  'check:sql-date-binding',
-  'check:native-typecheck',
+import path from 'node:path'
+
+/** `check:*` scripts this runner deliberately does not own, and why. */
+const EXCLUDED: Record<string, string> = {
+  'check:audits': 'this runner',
+  'check:migrations': 'needs a git base ref argument',
+  'check:api-validation': 'superseded by the :strict variant, which this runner does run',
+}
+
+/**
+ * Generated-artifact checks that live outside the `check:*` namespace. Listed explicitly
+ * because the `*:check` namespace also holds checks that need a sibling repo or network.
+ */
+const EXTRA_AUDITS = [
   'tool-metadata:check',
   'integration-catalog:check',
   'skills:check',
   'agent-stream-docs:check',
 ] as const
 
+const ROOT = path.resolve(import.meta.dir, '..')
+
 interface AuditResult {
   script: string
   ok: boolean
   durationMs: number
   output: string
 }
 
-const CONCURRENCY = Math.max(2, navigator.hardwareConcurrency - 1)
+async function auditScripts(): Promise<string[]> {
+  const manifest = await Bun.file(path.join(ROOT, 'package.json')).json()
+  const scripts = manifest.scripts as Record<string, string>
+  const derived = Object.keys(scripts).filter(
+    (name) => name.startsWith('check:') && !(name in EXCLUDED)
+  )
+  return [...derived, ...EXTRA_AUDITS]
+}
 
-async function runAudit(script: string): Promise<AuditResult> {
+/**
+ * Runs one audit, capturing its output.
+ *
+ * Spawns the script directly rather than `bun run <name>`, which would start a bun process
+ * only to have it read package.json and start a second one.
+ */
+async function runAudit(script: string, command: string): Promise<AuditResult> {
   const startedAt = performance.now()
-  const proc = Bun.spawn(['bun', 'run', script], {
+  const argv = command.replace(/^bun run /, '').split(/\s+/)
+  const proc = Bun.spawn([process.execPath, ...argv], {
+    cwd: ROOT,
     stdout: 'pipe',
     stderr: 'pipe',
     env: { ...process.env, FORCE_COLOR: '0' },
@@ -64,33 +77,63 @@ async function runAudit(script: string): Promise<AuditResult> {
   }
 }
 
-const queue = [...AUDITS]
+const manifest = await Bun.file(path.join(ROOT, 'package.json')).json()
+const commands = manifest.scripts as Record<string, string>
+const queue = await auditScripts()
+const total = queue.length
+// The coordinator only awaits, so it does not need a core reserved for it.
+const workers = Math.min(Math.max(2, navigator.hardwareConcurrency), total)
 const results: AuditResult[] = []
 
 async function worker(): Promise<void> {
-  for (let script = queue.shift(); script; script = queue.shift()) {
-    const result = await runAudit(script)
+  let script: string | undefined
+  while ((script = queue.shift())) {
+    const result = await runAudit(script, commands[script])
     results.push(result)
     console.log(`${result.ok ? '✓' : '✗'} ${result.script} (${Math.round(result.durationMs)}ms)`)
   }
 }
 
 const startedAt = performance.now()
-await Promise.all(Array.from({ length: Math.min(CONCURRENCY, AUDITS.length) }, worker))
+await Promise.all(Array.from({ length: workers }, worker))
 const wallMs = performance.now() - startedAt
-
-const failures = results.filter((result) => !result.ok)
-const serialMs = results.reduce((total, result) => total + result.durationMs, 0)
+const serialMs = results.reduce((sum, result) => sum + result.durationMs, 0)
 
 console.log(
-  `\n${results.length} audits in ${(wallMs / 1000).toFixed(1)}s wall (${(serialMs / 1000).toFixed(1)}s serial, ${CONCURRENCY}-way)`
+  `\n${total} audits in ${(wallMs / 1000).toFixed(1)}s wall (${(serialMs / 1000).toFixed(1)}s serial, ${workers}-way)`
 )
 
+/**
+ * Restores what the per-step workflow gave up: collapsible per-audit output and inline
+ * failure annotations in the GitHub UI, plus a timing table the separate steps never had.
+ */
+if (process.env.GITHUB_ACTIONS) {
+  const summary = [
+    '| Audit | Result | Duration |',
+    '| --- | --- | --- |',
+    ...[...results]
+      .sort((a, b) => b.durationMs - a.durationMs)
+      .map((r) => `| \`${r.script}\` | ${r.ok ? '✓' : '✗'} | ${Math.round(r.durationMs)}ms |`),
+    '',
+    `${total} audits in ${(wallMs / 1000).toFixed(1)}s wall (${(serialMs / 1000).toFixed(1)}s serial, ${workers}-way)`,
+  ].join('\n')
+  if (process.env.GITHUB_STEP_SUMMARY) {
+    await Bun.write(process.env.GITHUB_STEP_SUMMARY, `${summary}\n`)
+  }
+}
+
+const failures = results.filter((result) => !result.ok)
 if (failures.length > 0) {
   for (const failure of failures) {
-    console.error(`\n${'─'.repeat(72)}\n✗ ${failure.script}\n${'─'.repeat(72)}`)
-    console.error(failure.output || '(no output)')
+    if (process.env.GITHUB_ACTIONS) {
+      console.error(`::group::✗ ${failure.script}`)
+      console.error(failure.output || '(no output)')
+      console.error('::endgroup::')
+      console.error(`::error title=${failure.script}::audit failed — see the group above`)
+    } else {
+      console.error(`\n${'─'.repeat(72)}\n✗ ${failure.script}\n${'─'.repeat(72)}`)
+      console.error(failure.output || '(no output)')
+    }
   }
-  console.error(`\n${failures.length} audit(s) failed: ${failures.map((f) => f.script).join(', ')}`)
   process.exit(1)
 }