From 29b3c1b0ca3aedad750282620aa912d26acc15f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 22 Aug 2026 13:58:21 +0200 Subject: [PATCH 1/4] dx(test): opt-in worker-count override for solo local vitest runs resolveVitestMaxWorkers() caps local runs at 2 workers so parallel worktrees and spawn-heavy tests keep headroom, but a solo run that owns the machine pays 6x on a 12-core host for no benefit. Add AGENT_DEVICE_VITEST_MAX_WORKERS to opt in to a higher cap. It is clamped to os.cpus().length so a runaway value can't oversubscribe the host, and it is a no-op in CI (CI already derives its own worker count). A missing, blank, non-numeric, non-integer, or non-positive value falls through to the existing default cap rather than throwing. Default (unset) behavior is unchanged. Closes #1962 --- docs/agents/testing.md | 3 ++ scripts/lib/vitest-concurrency.test.ts | 57 ++++++++++++++++++++++++++ scripts/lib/vitest-concurrency.ts | 27 +++++++++++- vitest.config.ts | 3 ++ 4 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 scripts/lib/vitest-concurrency.test.ts diff --git a/docs/agents/testing.md b/docs/agents/testing.md index 3d8d10bc2..ddf798c3a 100644 --- a/docs/agents/testing.md +++ b/docs/agents/testing.md @@ -180,3 +180,6 @@ There is no unit-test retry layer—fix or remove flakes. - Keep Vitest isolation enabled and the pool on forks. Both alternatives were measured and did not improve the suite; importing the module under test rather than a platform barrel is the useful optimization. +- A solo local run that owns the machine can raise the default two-worker cap with + `AGENT_DEVICE_VITEST_MAX_WORKERS=`; it is clamped to the host's CPU count and ignored in CI, + which derives its own count. diff --git a/scripts/lib/vitest-concurrency.test.ts b/scripts/lib/vitest-concurrency.test.ts new file mode 100644 index 000000000..8983a66e0 --- /dev/null +++ b/scripts/lib/vitest-concurrency.test.ts @@ -0,0 +1,57 @@ +import os from 'node:os'; +import assert from 'node:assert/strict'; +import { test } from 'vitest'; +import { + DEFAULT_VITEST_MAX_WORKERS, + resolveVitestMaxWorkers, + VITEST_MAX_WORKERS_OVERRIDE_ENV, +} from './vitest-concurrency.ts'; + +test('an unset override preserves the existing default local cap', () => { + assert.equal(resolveVitestMaxWorkers({}), DEFAULT_VITEST_MAX_WORKERS); +}); + +test('CI ignores the override even when both signals are present', () => { + assert.equal( + resolveVitestMaxWorkers({ CI: 'true', [VITEST_MAX_WORKERS_OVERRIDE_ENV]: '8' }), + undefined, + ); +}); + +test('a valid override below the core count is honored as-is', () => { + // Below any real host's core count, so the resolver takes the override + // branch rather than clamping it back down. + assert.equal(resolveVitestMaxWorkers({ [VITEST_MAX_WORKERS_OVERRIDE_ENV]: '1' }), 1); +}); + +test('an override above the available core count is clamped down to it', () => { + assert.equal( + resolveVitestMaxWorkers({ [VITEST_MAX_WORKERS_OVERRIDE_ENV]: '999' }), + os.cpus().length, + ); +}); + +test('a non-numeric override falls through to the default cap', () => { + assert.equal( + resolveVitestMaxWorkers({ [VITEST_MAX_WORKERS_OVERRIDE_ENV]: 'not-a-number' }), + DEFAULT_VITEST_MAX_WORKERS, + ); +}); + +test('a zero or negative override falls through to the default cap', () => { + assert.equal( + resolveVitestMaxWorkers({ [VITEST_MAX_WORKERS_OVERRIDE_ENV]: '0' }), + DEFAULT_VITEST_MAX_WORKERS, + ); + assert.equal( + resolveVitestMaxWorkers({ [VITEST_MAX_WORKERS_OVERRIDE_ENV]: '-4' }), + DEFAULT_VITEST_MAX_WORKERS, + ); +}); + +test('a non-integer override falls through to the default cap', () => { + assert.equal( + resolveVitestMaxWorkers({ [VITEST_MAX_WORKERS_OVERRIDE_ENV]: '2.5' }), + DEFAULT_VITEST_MAX_WORKERS, + ); +}); diff --git a/scripts/lib/vitest-concurrency.ts b/scripts/lib/vitest-concurrency.ts index 5c5635159..54982e504 100644 --- a/scripts/lib/vitest-concurrency.ts +++ b/scripts/lib/vitest-concurrency.ts @@ -1,3 +1,5 @@ +import os from 'node:os'; + /** * Keep one Vitest invocation modest enough to coexist with two other Codex * worktrees on a 12-core development host: 3 agents + (3 suites * 2 workers) @@ -5,6 +7,29 @@ */ export const DEFAULT_VITEST_MAX_WORKERS = 2; +/** + * Opt-in escape hatch for a solo local run that owns the whole machine (see + * docs/agents/testing.md). Ignored in CI, which already derives its own + * worker count from the isolated runner's CPU pool. + */ +export const VITEST_MAX_WORKERS_OVERRIDE_ENV = 'AGENT_DEVICE_VITEST_MAX_WORKERS'; + export function resolveVitestMaxWorkers(env: NodeJS.ProcessEnv = process.env): number | undefined { - return env.CI === 'true' ? undefined : DEFAULT_VITEST_MAX_WORKERS; + if (env.CI === 'true') return undefined; + + const override = parsePositiveInt(env[VITEST_MAX_WORKERS_OVERRIDE_ENV]); + // Clamp rather than trust the override literally: a typo like `999` must not + // oversubscribe the host the way the default cap above exists to prevent. + if (override !== undefined) return Math.min(override, os.cpus().length); + + return DEFAULT_VITEST_MAX_WORKERS; +} + +// A missing, blank, non-numeric, non-integer, or non-positive value falls +// through to the default cap instead of throwing or coercing to something +// surprising (e.g. `Number('')` is 0, not NaN). +function parsePositiveInt(value: string | undefined): number | undefined { + if (value === undefined || value.trim() === '') return undefined; + const parsed = Number(value); + return Number.isInteger(parsed) && parsed > 0 ? parsed : undefined; } diff --git a/vitest.config.ts b/vitest.config.ts index 26ab5634e..f41c04618 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -61,6 +61,9 @@ export default defineConfig({ include: [ 'src/**/*.test.ts', 'packages/*/src/**/*.test.ts', + // Pure resolver logic (no subprocess, no device), imported by this + // config itself and by scripts/check-affected — belongs in the fast lane. + 'scripts/lib/vitest-concurrency.test.ts', // The validation fuzz generators' expectation gates (#1781 B2): in-process, no // subprocess or worker, so they ride the fast lane unlike their serialized siblings. 'scripts/fuzz/validation-arbitraries.test.ts', From 0098bd8cd41afe6af620ca79ab26fddc889e4ce5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 22 Aug 2026 14:04:04 +0200 Subject: [PATCH 2/4] docs: tighten the worker-override note to fit the agent-guidance budget docs/agents/testing.md sits at a 10,000-byte per-file ceiling enforced by check:agent-guidance, and the first phrasing pushed it to 10,065. Restate the override in one tighter bullet that leads with the "solo run only" caveat, which is the constraint a reader most needs. --- docs/agents/testing.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/agents/testing.md b/docs/agents/testing.md index ddf798c3a..bda12b188 100644 --- a/docs/agents/testing.md +++ b/docs/agents/testing.md @@ -180,6 +180,5 @@ There is no unit-test retry layer—fix or remove flakes. - Keep Vitest isolation enabled and the pool on forks. Both alternatives were measured and did not improve the suite; importing the module under test rather than a platform barrel is the useful optimization. -- A solo local run that owns the machine can raise the default two-worker cap with - `AGENT_DEVICE_VITEST_MAX_WORKERS=`; it is clamped to the host's CPU count and ignored in CI, - which derives its own count. +- Raise the two-worker local cap only for a solo run: `AGENT_DEVICE_VITEST_MAX_WORKERS=`, + clamped to host CPUs, ignored in CI. From 4117550f27466fbd63d113f1d4c6d79ed9936b6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 22 Aug 2026 14:05:49 +0200 Subject: [PATCH 3/4] fix(test): clamp the worker override with os.availableParallelism() Node documents cpus().length as unfit for sizing application parallelism: it ignores CPU affinity and cgroup limits, so it can report a pool wider than the process may actually use. Clamping against it would inflate the very ceiling this override's safety clamp exists to enforce. availableParallelism() honors those constraints, so the clamp now means what it claims on constrained hosts. Test updated to match. --- scripts/lib/vitest-concurrency.test.ts | 12 +++++++----- scripts/lib/vitest-concurrency.ts | 5 ++++- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/scripts/lib/vitest-concurrency.test.ts b/scripts/lib/vitest-concurrency.test.ts index 8983a66e0..89f472cfc 100644 --- a/scripts/lib/vitest-concurrency.test.ts +++ b/scripts/lib/vitest-concurrency.test.ts @@ -18,16 +18,18 @@ test('CI ignores the override even when both signals are present', () => { ); }); -test('a valid override below the core count is honored as-is', () => { - // Below any real host's core count, so the resolver takes the override - // branch rather than clamping it back down. +test('a valid override within the available parallelism is honored as-is', () => { + // 1 is <= availableParallelism() on every host, so the resolver takes the + // override branch rather than clamping it back down. assert.equal(resolveVitestMaxWorkers({ [VITEST_MAX_WORKERS_OVERRIDE_ENV]: '1' }), 1); }); -test('an override above the available core count is clamped down to it', () => { +// availableParallelism() is the clamp ceiling on purpose: it honors CPU affinity +// and cgroup limits that cpus().length reports straight past. +test('an override above the available parallelism is clamped down to it', () => { assert.equal( resolveVitestMaxWorkers({ [VITEST_MAX_WORKERS_OVERRIDE_ENV]: '999' }), - os.cpus().length, + os.availableParallelism(), ); }); diff --git a/scripts/lib/vitest-concurrency.ts b/scripts/lib/vitest-concurrency.ts index 54982e504..827370ad7 100644 --- a/scripts/lib/vitest-concurrency.ts +++ b/scripts/lib/vitest-concurrency.ts @@ -20,7 +20,10 @@ export function resolveVitestMaxWorkers(env: NodeJS.ProcessEnv = process.env): n const override = parsePositiveInt(env[VITEST_MAX_WORKERS_OVERRIDE_ENV]); // Clamp rather than trust the override literally: a typo like `999` must not // oversubscribe the host the way the default cap above exists to prevent. - if (override !== undefined) return Math.min(override, os.cpus().length); + // availableParallelism(), not cpus().length: Node documents the latter as + // unfit for sizing parallelism because it ignores CPU affinity and cgroup + // limits, which would inflate the ceiling this clamp exists to enforce. + if (override !== undefined) return Math.min(override, os.availableParallelism()); return DEFAULT_VITEST_MAX_WORKERS; } From 8360daa710c4a0180fabf7d322c7bc6bbdf5e197 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 22 Aug 2026 16:01:25 +0200 Subject: [PATCH 4/4] test: keep the resolver cases in the already-included setup test Review feedback: a new test file beside the resolver, plus its entry in vitest.config.ts's unit-core include list, is a change to test discovery that the mutation lane's `vitest related` graph reads. Fold the override cases into src/__tests__/hermetic-env-setup.test.ts, which is already in the unit suite and already imports the resolver, and drop the config edit entirely so this PR no longer touches test discovery at all. Same six assertions, no coverage lost. --- scripts/lib/vitest-concurrency.test.ts | 59 ------------------------ src/__tests__/hermetic-env-setup.test.ts | 30 ++++++++++++ vitest.config.ts | 3 -- 3 files changed, 30 insertions(+), 62 deletions(-) delete mode 100644 scripts/lib/vitest-concurrency.test.ts diff --git a/scripts/lib/vitest-concurrency.test.ts b/scripts/lib/vitest-concurrency.test.ts deleted file mode 100644 index 89f472cfc..000000000 --- a/scripts/lib/vitest-concurrency.test.ts +++ /dev/null @@ -1,59 +0,0 @@ -import os from 'node:os'; -import assert from 'node:assert/strict'; -import { test } from 'vitest'; -import { - DEFAULT_VITEST_MAX_WORKERS, - resolveVitestMaxWorkers, - VITEST_MAX_WORKERS_OVERRIDE_ENV, -} from './vitest-concurrency.ts'; - -test('an unset override preserves the existing default local cap', () => { - assert.equal(resolveVitestMaxWorkers({}), DEFAULT_VITEST_MAX_WORKERS); -}); - -test('CI ignores the override even when both signals are present', () => { - assert.equal( - resolveVitestMaxWorkers({ CI: 'true', [VITEST_MAX_WORKERS_OVERRIDE_ENV]: '8' }), - undefined, - ); -}); - -test('a valid override within the available parallelism is honored as-is', () => { - // 1 is <= availableParallelism() on every host, so the resolver takes the - // override branch rather than clamping it back down. - assert.equal(resolveVitestMaxWorkers({ [VITEST_MAX_WORKERS_OVERRIDE_ENV]: '1' }), 1); -}); - -// availableParallelism() is the clamp ceiling on purpose: it honors CPU affinity -// and cgroup limits that cpus().length reports straight past. -test('an override above the available parallelism is clamped down to it', () => { - assert.equal( - resolveVitestMaxWorkers({ [VITEST_MAX_WORKERS_OVERRIDE_ENV]: '999' }), - os.availableParallelism(), - ); -}); - -test('a non-numeric override falls through to the default cap', () => { - assert.equal( - resolveVitestMaxWorkers({ [VITEST_MAX_WORKERS_OVERRIDE_ENV]: 'not-a-number' }), - DEFAULT_VITEST_MAX_WORKERS, - ); -}); - -test('a zero or negative override falls through to the default cap', () => { - assert.equal( - resolveVitestMaxWorkers({ [VITEST_MAX_WORKERS_OVERRIDE_ENV]: '0' }), - DEFAULT_VITEST_MAX_WORKERS, - ); - assert.equal( - resolveVitestMaxWorkers({ [VITEST_MAX_WORKERS_OVERRIDE_ENV]: '-4' }), - DEFAULT_VITEST_MAX_WORKERS, - ); -}); - -test('a non-integer override falls through to the default cap', () => { - assert.equal( - resolveVitestMaxWorkers({ [VITEST_MAX_WORKERS_OVERRIDE_ENV]: '2.5' }), - DEFAULT_VITEST_MAX_WORKERS, - ); -}); diff --git a/src/__tests__/hermetic-env-setup.test.ts b/src/__tests__/hermetic-env-setup.test.ts index adabb34cc..dd1e04128 100644 --- a/src/__tests__/hermetic-env-setup.test.ts +++ b/src/__tests__/hermetic-env-setup.test.ts @@ -5,6 +5,7 @@ import assert from 'node:assert/strict'; import { DEFAULT_VITEST_MAX_WORKERS, resolveVitestMaxWorkers, + VITEST_MAX_WORKERS_OVERRIDE_ENV, } from '../../scripts/lib/vitest-concurrency.ts'; import vitestConfig from '../../vitest.config.ts'; @@ -23,6 +24,35 @@ test('vitest caps aggregate worker concurrency for parallel worktrees', () => { assert.equal(resolveVitestMaxWorkers({ CI: 'true' }), undefined); }); +// The opt-in solo-run escape hatch (#1962). These live here rather than beside the +// resolver so the mutation lane's `vitest related` graph is not widened by a new +// test file: this one is already in the unit-core suite and already imports it. +test('a solo run may raise the local worker cap, clamped and CI-ignored', () => { + // Clamped to availableParallelism(), never honored literally: cpus().length would + // ignore CPU affinity and cgroup limits and inflate the ceiling this enforces. + assert.equal( + resolveVitestMaxWorkers({ [VITEST_MAX_WORKERS_OVERRIDE_ENV]: '999' }), + os.availableParallelism(), + ); + // 1 is <= availableParallelism() on every host, so this takes the override branch. + assert.equal(resolveVitestMaxWorkers({ [VITEST_MAX_WORKERS_OVERRIDE_ENV]: '1' }), 1); + // CI derives its own count, so the override is inert there even when both are set. + assert.equal( + resolveVitestMaxWorkers({ CI: 'true', [VITEST_MAX_WORKERS_OVERRIDE_ENV]: '8' }), + undefined, + ); +}); + +test('an unusable worker override falls through to the default cap', () => { + for (const value of ['not-a-number', '0', '-4', '2.5', '', ' ']) { + assert.equal( + resolveVitestMaxWorkers({ [VITEST_MAX_WORKERS_OVERRIDE_ENV]: value }), + DEFAULT_VITEST_MAX_WORKERS, + `${JSON.stringify(value)} must degrade to the default cap rather than throw`, + ); + } +}); + // Wiring: the scrub only helps if every project loads it as a setup file. CI runs with the // vars unset, so a dropped wiring is otherwise invisible — assert it structurally instead. test('every vitest project wires the hermetic-env setup', () => { diff --git a/vitest.config.ts b/vitest.config.ts index f41c04618..26ab5634e 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -61,9 +61,6 @@ export default defineConfig({ include: [ 'src/**/*.test.ts', 'packages/*/src/**/*.test.ts', - // Pure resolver logic (no subprocess, no device), imported by this - // config itself and by scripts/check-affected — belongs in the fast lane. - 'scripts/lib/vitest-concurrency.test.ts', // The validation fuzz generators' expectation gates (#1781 B2): in-process, no // subprocess or worker, so they ride the fast lane unlike their serialized siblings. 'scripts/fuzz/validation-arbitraries.test.ts',