sim: merge PR #17201

.gitignore

@@ -132,6 +132,12 @@ validation-screenshots/
 /notes/
 /pr-evidence/
 
+# Per-task agent scratch trees, named for the Orca task id. They hold evidence and sometimes whole
+# nested repo checkouts; a foreign checkout is not this repo's changed code, and 39,079 files from
+# one overflowed the argument list of `check:code-quality:changed`. Unanchored because a lane can
+# create one from any working directory.
+.orca-task-*/
+
 # Playwright
 test-results/
 playwright-report/

config/scripts/check-changed-code-quality.mjs

@@ -1,5 +1,6 @@
 import { execFileSync, spawnSync } from 'node:child_process'
 import { existsSync, readFileSync } from 'node:fs'
+import { createRequire } from 'node:module'
 import path from 'node:path'
 import process from 'node:process'
 import { pathToFileURL } from 'node:url'
@@ -114,10 +115,19 @@ export function collectAddedLineRanges(root, requestedBase) {
 function parseOxlintOutput(stdout, label) {
   const start = stdout.indexOf('{')
   const end = stdout.lastIndexOf('}')
-  if (start === -1 || end === -1) {
-    throw new Error(`${label} did not return Oxlint JSON output.`)
+  if (start !== -1 && end !== -1) {
+    try {
+      return JSON.parse(stdout.slice(start, end + 1))
+    } catch {
+      // Fall through so the caller sees what Oxlint actually printed.
+    }
   }
-  return JSON.parse(stdout.slice(start, end + 1))
+  // Why echo it: Oxlint writes configuration failures to stdout, and a wrapper's own
+  // warning can carry braces that this slice mistakes for the report, so discarding the
+  // output leaves the gate dying with no reason anywhere in the log.
+  throw new Error(
+    `${label} did not return Oxlint JSON output. Oxlint printed:\n${stdout.trim().slice(0, 2000)}`
+  )
 }
 
 function normalizedDiagnosticPath(root, filename) {
@@ -263,21 +273,145 @@ function printDiagnostic(diagnostic, root) {
   console.error(`${file}:${line} ${code}: ${diagnostic.message}`)
 }
 
-function runOxlintScan(root, scan, files) {
-  const pnpm = process.platform === 'win32' ? 'pnpm.cmd' : 'pnpm'
-  const result = spawnSync(pnpm, ['exec', 'oxlint', ...scan.args, '--format', 'json', ...files], {
+// Why: Oxlint takes paths as positional arguments only — no stdin, no @file — so one
+// oversized changed set makes the spawn itself fail with E2BIG and the gate never runs.
+// The file list is only part of what the spawn carries, so budget against the whole of it.
+//
+// POSIX execve() charges argv strings, the inherited environment, AND one pointer per
+// entry against a single ceiling: macOS caps that at kern.argmax (1 MiB), Linux at
+// max(min(6 MiB, RLIMIT_STACK/4), 128 KiB) — 2 MiB at the default 8 MiB stack.
+//
+// Budgeting just under the kernel's own ceiling is not enough. macOS copies argv and the
+// environment into the child's stack, and a Node child dies from the squeeze well before
+// the kernel refuses the exec: measured here, a total near 970 KiB SIGSEGVs on Node 24 and
+// throws a stack-overflow RangeError on Node 26, while E2BIG only starts around 1,048 KiB.
+// Half of kern.argmax leaves that cliff ~450 KiB away.
+//
+// Windows is a different shape entirely: CreateProcess caps only the command line, at
+// 32,767 UTF-16 units, and passes the environment in a separate block that does not count.
+// Counting UTF-8 bytes against a UTF-16 ceiling over-estimates, which is the safe direction.
+const POSIX_SPAWN_CEILING_BYTES = 512 * 1024
+const WINDOWS_COMMAND_LINE_CEILING_BYTES = 32767
+// Every entry costs a pointer beside its bytes, and libuv may wrap a Windows argument in
+// quotes. Measured on macOS: 6,096 paths cost ~48 KiB in pointers alone.
+const PER_ENTRY_OVERHEAD_BYTES = 12
+// Slack for kernel padding and the exec path the kernel copies alongside argv.
+const SPAWN_HEADROOM_BYTES = 8 * 1024
+// A full ceiling's worth of paths would be a single enormous batch; hold the file list to
+// what the gate already used so an ordinary changed set still spawns once per scan.
+const MAX_BATCH_ARGUMENT_BYTES = 256 * 1024
+// A pathological environment must not drive the budget to zero, which would spawn Oxlint
+// once per path.
+const MIN_BATCH_ARGUMENT_BYTES = 8 * 1024
+
+function spawnEntryBytes(entries) {
+  let total = 0
+  for (const entry of entries) {
+    total += Buffer.byteLength(entry, 'utf8') + PER_ENTRY_OVERHEAD_BYTES
+  }
+  return total
+}
+
+export function environmentEntries(env = process.env) {
+  const entries = []
+  for (const [key, value] of Object.entries(env)) {
+    if (value !== undefined) {
+      entries.push(`${key}=${value}`)
+    }
+  }
+  return entries
+}
+
+export function maxBatchArgumentBytes({
+  platform = process.platform,
+  fixedArguments = [],
+  env = process.env
+} = {}) {
+  const windows = platform === 'win32'
+  const ceiling = windows ? WINDOWS_COMMAND_LINE_CEILING_BYTES : POSIX_SPAWN_CEILING_BYTES
+  const carried =
+    spawnEntryBytes(fixedArguments) + (windows ? 0 : spawnEntryBytes(environmentEntries(env)))
+  return Math.min(
+    MAX_BATCH_ARGUMENT_BYTES,
+    Math.max(MIN_BATCH_ARGUMENT_BYTES, ceiling - SPAWN_HEADROOM_BYTES - carried)
+  )
+}
+
+export function batchFilesByArgumentBytes(files, limit = maxBatchArgumentBytes()) {
+  const batches = []
+  let batch = []
+  let bytes = 0
+  for (const file of files) {
+    // A single path over the limit still gets its own batch: an empty argument list
+    // would make Oxlint lint the whole working directory instead.
+    const cost = Buffer.byteLength(file, 'utf8') + PER_ENTRY_OVERHEAD_BYTES
+    if (batch.length > 0 && bytes + cost > limit) {
+      batches.push(batch)
+      batch = []
+      bytes = 0
+    }
+    batch.push(file)
+    bytes += cost
+  }
+  if (batch.length > 0) {
+    batches.push(batch)
+  }
+  return batches
+}
+
+// Why resolve the manifest rather than joining node_modules: pnpm's store is not a flat
+// tree, and `bin` is the launcher the package itself declares. Not `pnpm exec`, which on
+// Windows routes through pnpm.cmd and cmd.exe, whose command line caps at 8,191 characters
+// instead of CreateProcess' 32,767.
+let cachedOxlintCliPath = null
+function oxlintCliPath() {
+  if (cachedOxlintCliPath === null) {
+    const manifestPath = createRequire(import.meta.url).resolve('oxlint/package.json')
+    const manifest = JSON.parse(readFileSync(manifestPath, 'utf8'))
+    cachedOxlintCliPath = path.join(path.dirname(manifestPath), manifest.bin.oxlint)
+  }
+  return cachedOxlintCliPath
+}
+
+function oxlintCommand(scan) {
+  return {
+    command: process.execPath,
+    fixedArguments: [oxlintCliPath(), ...scan.args, '--format', 'json']
+  }
+}
+
+function spawnOxlintBatch(root, scan, batch) {
+  const { command, fixedArguments } = oxlintCommand(scan)
+  const result = spawnSync(command, [...fixedArguments, ...batch], {
     cwd: root,
     encoding: 'utf8',
     maxBuffer: 128 * 1024 * 1024
   })
   if (result.error) {
-    throw result.error
+    // Why restate it: a bare spawn error reads as a crash in the gate rather than a
+    // failure to launch Oxlint over this batch.
+    throw new Error(
+      `${scan.label} could not run Oxlint over ${batch.length} file(s): ${result.error.message}`,
+      { cause: result.error }
+    )
   }
   if (!result.stdout.trim()) {
     process.stderr.write(result.stderr)
     throw new Error(`${scan.label} failed before producing diagnostics.`)
   }
-  return parseOxlintOutput(result.stdout, scan.label).diagnostics ?? []
+  return result.stdout
+}
+
+export function runOxlintScan(root, scan, files, spawnBatch = spawnOxlintBatch) {
+  const { command, fixedArguments } = oxlintCommand(scan)
+  const limit = maxBatchArgumentBytes({ fixedArguments: [command, ...fixedArguments] })
+  const diagnostics = []
+  for (const batch of batchFilesByArgumentBytes(files, limit)) {
+    diagnostics.push(
+      ...(parseOxlintOutput(spawnBatch(root, scan, batch), scan.label).diagnostics ?? [])
+    )
+  }
+  return diagnostics
 }
 
 export function main(

config/scripts/check-changed-code-quality.test.mjs

@@ -1,10 +1,14 @@
 import { describe, expect, it } from 'vitest'
 import {
   OXLINT_SCANS,
+  batchFilesByArgumentBytes,
   diagnosticTouchesAddedLines,
+  environmentEntries,
   isMovedCode,
+  maxBatchArgumentBytes,
   overlapsAddedLines,
-  parseAddedLineRanges
+  parseAddedLineRanges,
+  runOxlintScan
 } from './check-changed-code-quality.mjs'
 
 describe('changed-code quality line matching', () => {
@@ -104,3 +108,199 @@ describe('moved-code exemption', () => {
     expect(isMovedCode(['', '   '], [['a()']])).toBe(false)
   })
 })
+
+describe('argument batching', () => {
+  // Mirrors the script's own accounting: every argv/envp entry costs a pointer beside its bytes.
+  const spawnBytes = (entries) =>
+    entries.reduce((total, entry) => total + Buffer.byteLength(entry, 'utf8') + 12, 0)
+  // Large enough to exceed the real byte budget, so the batching that ships is what runs.
+  const oversizedSet = Array.from(
+    { length: 20000 },
+    (_, index) => `src/generated/module-${index}.ts`
+  )
+  const fixedArguments = [
+    '/usr/local/bin/node',
+    '/repo/node_modules/oxlint/bin/oxlint',
+    '--format',
+    'json'
+  ]
+  // Big enough that the budget must shrink below the cap, small enough to still batch.
+  const largeEnvironment = { PATH: '/usr/bin', ORCA_LARGE_VARIABLE: 'x'.repeat(300 * 1024) }
+  const fatEnvironment = { PATH: '/usr/bin', ORCA_FAT_VARIABLE: 'x'.repeat(900 * 1024) }
+  // Half of macOS kern.argmax: spawning Node fails from stack pressure near 970 KiB, well
+  // before the kernel's own 1 MiB E2BIG.
+  const POSIX_CEILING = 512 * 1024
+  // CreateProcess' documented lpCommandLine maximum.
+  const WINDOWS_CEILING = 32767
+
+  it('runs a normal changed set as a single invocation', () => {
+    const files = Array.from({ length: 50 }, (_, index) => `src/renderer/src/module-${index}.tsx`)
+
+    expect(batchFilesByArgumentBytes(files)).toEqual([files])
+  })
+
+  it('uses a Windows-safe budget without shrinking Unix batches', () => {
+    expect(maxBatchArgumentBytes({ platform: 'win32', env: {} })).toBe(32767 - 8 * 1024)
+    expect(maxBatchArgumentBytes({ platform: 'linux', env: {} })).toBe(256 * 1024)
+    expect(maxBatchArgumentBytes({ platform: 'darwin', env: {} })).toBe(256 * 1024)
+  })
+
+  it('splits an oversized set into batches that each fit the limit', () => {
+    const batches = batchFilesByArgumentBytes(oversizedSet)
+
+    expect(batches.length).toBeGreaterThan(1)
+    expect(batches.flat()).toEqual(oversizedSet)
+    for (const batch of batches) {
+      expect(spawnBytes(batch)).toBeLessThanOrEqual(256 * 1024)
+    }
+  })
+
+  // Why: an empty argument list makes Oxlint lint the whole working directory.
+  it('never emits an empty batch, even when the first path is longer than the limit', () => {
+    const batches = batchFilesByArgumentBytes(['src/very-long-path.ts', 'src/a.ts'], 10)
+
+    expect(batches.every((batch) => batch.length > 0)).toBe(true)
+    expect(batches.flat()).toEqual(['src/very-long-path.ts', 'src/a.ts'])
+  })
+
+  it('counts multi-byte paths by their byte length, not their character count', () => {
+    expect(batchFilesByArgumentBytes(['src/\u00e9.ts', 'src/b.ts'], 30)).toEqual([
+      ['src/\u00e9.ts'],
+      ['src/b.ts']
+    ])
+  })
+
+  // Why: execve() charges argv and the inherited environment against one ceiling, so a
+  // budget computed from the file list alone stacks 256 KiB of paths on top of whatever the
+  // shell already carries, and the spawn fails before Oxlint ever starts.
+  it('keeps the whole POSIX spawn — arguments and environment — under the ceiling', () => {
+    const limit = maxBatchArgumentBytes({
+      platform: 'darwin',
+      fixedArguments,
+      env: largeEnvironment
+    })
+    const batches = batchFilesByArgumentBytes(oversizedSet, limit)
+    const environmentCost = spawnBytes(environmentEntries(largeEnvironment))
+
+    expect(batches.length).toBeGreaterThan(1)
+    for (const batch of batches) {
+      expect(spawnBytes([...fixedArguments, ...batch]) + environmentCost).toBeLessThanOrEqual(
+        POSIX_CEILING
+      )
+    }
+  })
+
+  it('shrinks the POSIX budget as the environment grows', () => {
+    const small = maxBatchArgumentBytes({ platform: 'darwin', fixedArguments, env: { A: 'a' } })
+    const large = maxBatchArgumentBytes({
+      platform: 'darwin',
+      fixedArguments,
+      env: largeEnvironment
+    })
+
+    expect(large).toBeLessThan(small)
+  })
+
+  // Why: CreateProcess caps the command line only; the environment ships in its own block.
+  it('budgets Windows from the command line alone and stays inside CreateProcess', () => {
+    const limit = maxBatchArgumentBytes({
+      platform: 'win32',
+      fixedArguments,
+      env: fatEnvironment
+    })
+
+    expect(limit).toBe(maxBatchArgumentBytes({ platform: 'win32', fixedArguments, env: {} }))
+    for (const batch of batchFilesByArgumentBytes(oversizedSet, limit)) {
+      expect(spawnBytes([...fixedArguments, ...batch])).toBeLessThanOrEqual(WINDOWS_CEILING)
+    }
+  })
+
+  // Why: a zero budget would spawn Oxlint once per path instead of failing usefully.
+  it('floors the budget when the environment alone exceeds the ceiling', () => {
+    const limit = maxBatchArgumentBytes({
+      platform: 'darwin',
+      fixedArguments,
+      env: { ORCA_FAT_VARIABLE: 'x'.repeat(4 * 1024 * 1024) }
+    })
+
+    expect(limit).toBe(8 * 1024)
+  })
+
+  it('ignores environment entries that carry no value', () => {
+    expect(environmentEntries({ A: 'a', B: undefined })).toEqual(['A=a'])
+  })
+})
+
+describe('diagnostic collection across batches', () => {
+  const scan = { label: 'code quality', args: [] }
+  const diagnosticFor = (file) => ({ filename: file, message: `finding in ${file}` })
+  const files = Array.from({ length: 20000 }, (_, index) => `src/generated/module-${index}.ts`)
+  const observeBatches = () => {
+    const batches = []
+    runOxlintScan('/repo', scan, files, (_root, _scan, batch) => {
+      batches.push(batch)
+      return JSON.stringify({ diagnostics: [] })
+    })
+    return batches
+  }
+  const reportOnly = (failing) => (_root, _scan, batch) =>
+    JSON.stringify({ diagnostics: batch.includes(failing) ? [diagnosticFor(failing)] : [] })
+
+  it('keeps the diagnostics of every batch, not just the last one', () => {
+    const spawnBatch = (_root, _scan, batch) =>
+      JSON.stringify({ diagnostics: batch.map(diagnosticFor) })
+
+    expect(observeBatches().length).toBeGreaterThan(1)
+    expect(runOxlintScan('/repo', scan, files, spawnBatch)).toEqual(files.map(diagnosticFor))
+  })
+
+  it('reports a finding that only a middle batch produces', () => {
+    const batches = observeBatches()
+    expect(batches.length).toBeGreaterThan(2)
+    const failing = batches.at(Math.floor(batches.length / 2)).at(0)
+
+    expect(batches.at(0)).not.toContain(failing)
+    expect(batches.at(-1)).not.toContain(failing)
+    expect(runOxlintScan('/repo', scan, files, reportOnly(failing))).toEqual([
+      diagnosticFor(failing)
+    ])
+  })
+
+  it('reports a finding that only the last batch produces', () => {
+    const batches = observeBatches()
+    const failing = batches.at(-1).at(-1)
+
+    expect(batches.at(0)).not.toContain(failing)
+    expect(runOxlintScan('/repo', scan, files, reportOnly(failing))).toEqual([
+      diagnosticFor(failing)
+    ])
+  })
+
+  // Why: Oxlint writes configuration failures to stdout, so swallowing it leaves the gate
+  // dying with no reason in the log.
+  it('surfaces what Oxlint printed when the output is not a report', () => {
+    const failure = 'Failed to parse oxlint configuration file.\n\n  x Rule not found\n'
+
+    expect(() => runOxlintScan('/repo', scan, ['src/a.ts'], () => failure)).toThrow(
+      /Rule not found/
+    )
+  })
+
+  // Why: the slice from the first brace to the last one takes a wrapper's own warning for
+  // the report, and the raw SyntaxError names neither the scan nor the warning.
+  it('surfaces a wrapper warning whose braces shadow the report', () => {
+    const polluted = ` WARN  Unsupported engine: wanted: {"node":"24"}\n${JSON.stringify({ diagnostics: [] })}`
+
+    expect(() => runOxlintScan('/repo', scan, ['src/a.ts'], () => polluted)).toThrow(
+      /Unsupported engine/
+    )
+  })
+
+  // Why: the whole point is that no single invocation carries the full argument list.
+  it('never hands the whole oversized set to one invocation', () => {
+    const batchSizes = observeBatches().map((batch) => batch.length)
+
+    expect(batchSizes.length).toBeGreaterThan(1)
+    expect(Math.max(...batchSizes)).toBeLessThan(files.length)
+  })
+})