From 2f96463fbc5db797ed37119210cf3be4d184123a Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sat, 29 Aug 2026 06:02:22 -0700 Subject: [PATCH 1/4] fix(ci): keep the changed-code quality gate runnable on huge changed sets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Oxlint takes paths as positional arguments only, so `check:code-quality:changed` spawned one process carrying every changed file. A lane's `.orca-task-*` scratch tree holding seven nested repo checkouts put 39,079 untracked source files in scope — 3.5 MB of argv against a 1 MiB ARG_MAX — and the gate died with E2BIG before Oxlint ran. Two fixes: ignore the per-task scratch trees so a foreign checkout is no longer counted as your changed code, and batch the argument list under 256 KiB per invocation, concatenating the diagnostics so every batch reaches the exit status. --- .gitignore | 6 ++ config/scripts/check-changed-code-quality.mjs | 44 +++++++++- .../check-changed-code-quality.test.mjs | 82 ++++++++++++++++++- 3 files changed, 128 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index 176206bacfb..9b247e50924 100644 --- a/.gitignore +++ b/.gitignore @@ -130,6 +130,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/ diff --git a/config/scripts/check-changed-code-quality.mjs b/config/scripts/check-changed-code-quality.mjs index 615a2402bcf..da19398c745 100644 --- a/config/scripts/check-changed-code-quality.mjs +++ b/config/scripts/check-changed-code-quality.mjs @@ -263,9 +263,37 @@ function printDiagnostic(diagnostic, root) { console.error(`${file}:${line} ${code}: ${diagnostic.message}`) } -function runOxlintScan(root, scan, 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. +// Stay well under the smallest platform limit (macOS ARG_MAX is 1 MiB, shared with the +// environment) so a batch cannot overflow it. +const MAX_BATCH_ARGUMENT_BYTES = 256 * 1024 + +export function batchFilesByArgumentBytes(files, limit = MAX_BATCH_ARGUMENT_BYTES) { + 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') + 1 + 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 +} + +function spawnOxlintBatch(root, scan, batch) { const pnpm = process.platform === 'win32' ? 'pnpm.cmd' : 'pnpm' - const result = spawnSync(pnpm, ['exec', 'oxlint', ...scan.args, '--format', 'json', ...files], { + const result = spawnSync(pnpm, ['exec', 'oxlint', ...scan.args, '--format', 'json', ...batch], { cwd: root, encoding: 'utf8', maxBuffer: 128 * 1024 * 1024 @@ -277,7 +305,17 @@ function runOxlintScan(root, scan, files) { 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 diagnostics = [] + for (const batch of batchFilesByArgumentBytes(files)) { + diagnostics.push( + ...(parseOxlintOutput(spawnBatch(root, scan, batch), scan.label).diagnostics ?? []) + ) + } + return diagnostics } export function main( diff --git a/config/scripts/check-changed-code-quality.test.mjs b/config/scripts/check-changed-code-quality.test.mjs index 76a25802e5c..f4122481c7a 100644 --- a/config/scripts/check-changed-code-quality.test.mjs +++ b/config/scripts/check-changed-code-quality.test.mjs @@ -1,10 +1,12 @@ import { describe, expect, it } from 'vitest' import { OXLINT_SCANS, + batchFilesByArgumentBytes, diagnosticTouchesAddedLines, isMovedCode, overlapsAddedLines, - parseAddedLineRanges + parseAddedLineRanges, + runOxlintScan } from './check-changed-code-quality.mjs' describe('changed-code quality line matching', () => { @@ -104,3 +106,81 @@ describe('moved-code exemption', () => { expect(isMovedCode(['', ' '], [['a()']])).toBe(false) }) }) + +describe('argument batching', () => { + const argumentBytes = (batch) => batch.reduce((total, file) => total + file.length + 1, 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` + ) + + 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('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(argumentBytes(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'], 18)).toEqual([ + ['src/\u00e9.ts'], + ['src/b.ts'] + ]) + }) +}) + +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`) + + it('keeps the diagnostics of every batch, not just the last one', () => { + const spawnBatch = (_root, _scan, batch) => + JSON.stringify({ diagnostics: batch.map(diagnosticFor) }) + + expect(batchFilesByArgumentBytes(files).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 = batchFilesByArgumentBytes(files) + const failing = batches.at(Math.floor(batches.length / 2)).at(0) + const spawnBatch = (_root, _scan, batch) => + JSON.stringify({ diagnostics: batch.includes(failing) ? [diagnosticFor(failing)] : [] }) + + expect(batches.at(0)).not.toContain(failing) + expect(batches.at(-1)).not.toContain(failing) + expect(runOxlintScan('/repo', scan, files, spawnBatch)).toEqual([diagnosticFor(failing)]) + }) + + // 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 = [] + const spawnBatch = (_root, _scan, batch) => { + batchSizes.push(batch.length) + return JSON.stringify({ diagnostics: [] }) + } + runOxlintScan('/repo', scan, files, spawnBatch) + + expect(batchSizes.length).toBeGreaterThan(1) + expect(Math.max(...batchSizes)).toBeLessThan(files.length) + }) +}) From e991d8e59a7690a34325c1c7cc4f34c70fdd3cef Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sat, 29 Aug 2026 06:19:40 -0700 Subject: [PATCH 2/4] fix(ci): keep changed-code batches within Windows limits --- config/scripts/check-changed-code-quality.mjs | 30 ++++++++++++------- .../check-changed-code-quality.test.mjs | 7 +++++ 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/config/scripts/check-changed-code-quality.mjs b/config/scripts/check-changed-code-quality.mjs index da19398c745..b5da5a6cfb9 100644 --- a/config/scripts/check-changed-code-quality.mjs +++ b/config/scripts/check-changed-code-quality.mjs @@ -265,11 +265,16 @@ function printDiagnostic(diagnostic, root) { // 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. -// Stay well under the smallest platform limit (macOS ARG_MAX is 1 MiB, shared with the -// environment) so a batch cannot overflow it. -const MAX_BATCH_ARGUMENT_BYTES = 256 * 1024 +// Windows CreateProcess caps the whole command at 32,767 characters. Leave room for Node, +// the Oxlint entry point, fixed flags, and libuv quoting without shrinking Unix batches. +const POSIX_MAX_BATCH_ARGUMENT_BYTES = 256 * 1024 +const WINDOWS_MAX_BATCH_ARGUMENT_BYTES = 24 * 1024 -export function batchFilesByArgumentBytes(files, limit = MAX_BATCH_ARGUMENT_BYTES) { +export function maxBatchArgumentBytes(platform = process.platform) { + return platform === 'win32' ? WINDOWS_MAX_BATCH_ARGUMENT_BYTES : POSIX_MAX_BATCH_ARGUMENT_BYTES +} + +export function batchFilesByArgumentBytes(files, limit = maxBatchArgumentBytes()) { const batches = [] let batch = [] let bytes = 0 @@ -292,12 +297,17 @@ export function batchFilesByArgumentBytes(files, limit = MAX_BATCH_ARGUMENT_BYTE } function spawnOxlintBatch(root, scan, batch) { - const pnpm = process.platform === 'win32' ? 'pnpm.cmd' : 'pnpm' - const result = spawnSync(pnpm, ['exec', 'oxlint', ...scan.args, '--format', 'json', ...batch], { - cwd: root, - encoding: 'utf8', - maxBuffer: 128 * 1024 * 1024 - }) + // Bypass pnpm.cmd so Windows gets CreateProcess' limit instead of cmd.exe's 8,191 chars. + const oxlintCli = path.join(root, 'node_modules', 'oxlint', 'bin', 'oxlint') + const result = spawnSync( + process.execPath, + [oxlintCli, ...scan.args, '--format', 'json', ...batch], + { + cwd: root, + encoding: 'utf8', + maxBuffer: 128 * 1024 * 1024 + } + ) if (result.error) { throw result.error } diff --git a/config/scripts/check-changed-code-quality.test.mjs b/config/scripts/check-changed-code-quality.test.mjs index f4122481c7a..c4708b94faa 100644 --- a/config/scripts/check-changed-code-quality.test.mjs +++ b/config/scripts/check-changed-code-quality.test.mjs @@ -4,6 +4,7 @@ import { batchFilesByArgumentBytes, diagnosticTouchesAddedLines, isMovedCode, + maxBatchArgumentBytes, overlapsAddedLines, parseAddedLineRanges, runOxlintScan @@ -121,6 +122,12 @@ describe('argument batching', () => { expect(batchFilesByArgumentBytes(files)).toEqual([files]) }) + it('uses a Windows-safe budget without shrinking Unix batches', () => { + expect(maxBatchArgumentBytes('win32')).toBe(24 * 1024) + expect(maxBatchArgumentBytes('linux')).toBe(256 * 1024) + expect(maxBatchArgumentBytes('darwin')).toBe(256 * 1024) + }) + it('splits an oversized set into batches that each fit the limit', () => { const batches = batchFilesByArgumentBytes(oversizedSet) From 3039f50341cfd673c148f239fdd816da3b62323d Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sat, 29 Aug 2026 06:46:03 -0700 Subject: [PATCH 3/4] fix(ci): budget the changed-code batches from the whole spawn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The batch budget counted only the file list, but execve() charges argv, the inherited environment and one pointer per entry against a single ceiling. A 256 KiB batch beside a large environment failed the spawn outright, so the gate reported nothing instead of linting. Compute the budget from everything the spawn carries: the Node binary, the Oxlint entry point, the fixed flags and — on POSIX only, since Windows ships it in a separate block — the environment. Budget against half of macOS' kern.argmax rather than all of it: a Node child SIGSEGVs (24) or throws a stack-overflow RangeError (26) near 970 KiB, well before E2BIG at 1,048 KiB. Resolve the Oxlint launcher from its own manifest instead of assuming a flat node_modules layout, and name the scan and batch when a spawn fails so it reads as a launch failure rather than a crash in the gate. --- config/scripts/check-changed-code-quality.mjs | 117 +++++++++++++--- .../check-changed-code-quality.test.mjs | 127 +++++++++++++++--- 2 files changed, 207 insertions(+), 37 deletions(-) diff --git a/config/scripts/check-changed-code-quality.mjs b/config/scripts/check-changed-code-quality.mjs index b5da5a6cfb9..c2f9619ecfd 100644 --- a/config/scripts/check-changed-code-quality.mjs +++ b/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' @@ -265,13 +266,66 @@ function printDiagnostic(diagnostic, root) { // 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. -// Windows CreateProcess caps the whole command at 32,767 characters. Leave room for Node, -// the Oxlint entry point, fixed flags, and libuv quoting without shrinking Unix batches. -const POSIX_MAX_BATCH_ARGUMENT_BYTES = 256 * 1024 -const WINDOWS_MAX_BATCH_ARGUMENT_BYTES = 24 * 1024 +// 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 -export function maxBatchArgumentBytes(platform = process.platform) { - return platform === 'win32' ? WINDOWS_MAX_BATCH_ARGUMENT_BYTES : POSIX_MAX_BATCH_ARGUMENT_BYTES +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()) { @@ -281,7 +335,7 @@ export function batchFilesByArgumentBytes(files, limit = maxBatchArgumentBytes() 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') + 1 + const cost = Buffer.byteLength(file, 'utf8') + PER_ENTRY_OVERHEAD_BYTES if (batch.length > 0 && bytes + cost > limit) { batches.push(batch) batch = [] @@ -296,20 +350,41 @@ export function batchFilesByArgumentBytes(files, limit = maxBatchArgumentBytes() 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) { - // Bypass pnpm.cmd so Windows gets CreateProcess' limit instead of cmd.exe's 8,191 chars. - const oxlintCli = path.join(root, 'node_modules', 'oxlint', 'bin', 'oxlint') - const result = spawnSync( - process.execPath, - [oxlintCli, ...scan.args, '--format', 'json', ...batch], - { - cwd: root, - encoding: 'utf8', - maxBuffer: 128 * 1024 * 1024 - } - ) + 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) @@ -319,8 +394,10 @@ function spawnOxlintBatch(root, scan, batch) { } 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)) { + for (const batch of batchFilesByArgumentBytes(files, limit)) { diagnostics.push( ...(parseOxlintOutput(spawnBatch(root, scan, batch), scan.label).diagnostics ?? []) ) diff --git a/config/scripts/check-changed-code-quality.test.mjs b/config/scripts/check-changed-code-quality.test.mjs index c4708b94faa..c2a2c2004ce 100644 --- a/config/scripts/check-changed-code-quality.test.mjs +++ b/config/scripts/check-changed-code-quality.test.mjs @@ -3,6 +3,7 @@ import { OXLINT_SCANS, batchFilesByArgumentBytes, diagnosticTouchesAddedLines, + environmentEntries, isMovedCode, maxBatchArgumentBytes, overlapsAddedLines, @@ -109,12 +110,28 @@ describe('moved-code exemption', () => { }) describe('argument batching', () => { - const argumentBytes = (batch) => batch.reduce((total, file) => total + file.length + 1, 0) + // 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`) @@ -123,9 +140,9 @@ describe('argument batching', () => { }) it('uses a Windows-safe budget without shrinking Unix batches', () => { - expect(maxBatchArgumentBytes('win32')).toBe(24 * 1024) - expect(maxBatchArgumentBytes('linux')).toBe(256 * 1024) - expect(maxBatchArgumentBytes('darwin')).toBe(256 * 1024) + 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', () => { @@ -134,7 +151,7 @@ describe('argument batching', () => { expect(batches.length).toBeGreaterThan(1) expect(batches.flat()).toEqual(oversizedSet) for (const batch of batches) { - expect(argumentBytes(batch)).toBeLessThanOrEqual(256 * 1024) + expect(spawnBytes(batch)).toBeLessThanOrEqual(256 * 1024) } }) @@ -147,45 +164,121 @@ describe('argument batching', () => { }) it('counts multi-byte paths by their byte length, not their character count', () => { - expect(batchFilesByArgumentBytes(['src/\u00e9.ts', 'src/b.ts'], 18)).toEqual([ + 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(batchFilesByArgumentBytes(files).length).toBeGreaterThan(1) + 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 = batchFilesByArgumentBytes(files) + const batches = observeBatches() + expect(batches.length).toBeGreaterThan(2) const failing = batches.at(Math.floor(batches.length / 2)).at(0) - const spawnBatch = (_root, _scan, batch) => - JSON.stringify({ diagnostics: batch.includes(failing) ? [diagnosticFor(failing)] : [] }) expect(batches.at(0)).not.toContain(failing) expect(batches.at(-1)).not.toContain(failing) - expect(runOxlintScan('/repo', scan, files, spawnBatch)).toEqual([diagnosticFor(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: 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 = [] - const spawnBatch = (_root, _scan, batch) => { - batchSizes.push(batch.length) - return JSON.stringify({ diagnostics: [] }) - } - runOxlintScan('/repo', scan, files, spawnBatch) + const batchSizes = observeBatches().map((batch) => batch.length) expect(batchSizes.length).toBeGreaterThan(1) expect(Math.max(...batchSizes)).toBeLessThan(files.length) From 0a8da47f401bc707f6b35e03e2614c3f2187abcd Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sat, 29 Aug 2026 07:08:08 -0700 Subject: [PATCH 4/4] fix(ci): surface what Oxlint printed when the changed-code gate cannot parse it Oxlint writes configuration failures to stdout, and the slice from the first brace to the last one takes a wrapper's own warning for the report, so the gate died with "did not return Oxlint JSON output" and discarded the only explanation of why. --- config/scripts/check-changed-code-quality.mjs | 15 +++++++++++--- .../check-changed-code-quality.test.mjs | 20 +++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/config/scripts/check-changed-code-quality.mjs b/config/scripts/check-changed-code-quality.mjs index c2f9619ecfd..adf9c9f365a 100644 --- a/config/scripts/check-changed-code-quality.mjs +++ b/config/scripts/check-changed-code-quality.mjs @@ -115,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) { diff --git a/config/scripts/check-changed-code-quality.test.mjs b/config/scripts/check-changed-code-quality.test.mjs index c2a2c2004ce..e85ddc8f880 100644 --- a/config/scripts/check-changed-code-quality.test.mjs +++ b/config/scripts/check-changed-code-quality.test.mjs @@ -276,6 +276,26 @@ describe('diagnostic collection across batches', () => { ]) }) + // 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)