From 627316b93367e7c27bffce8b493ed0c0834cbeb4 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 12:04:22 +0000 Subject: [PATCH 1/2] fix(devx): bound vitest's inner worker pool from the invocation layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit turbo's outer `--concurrency=50%` (#11954) bounds how many package `test` tasks run at once, but not vitest's own pool inside each of them. 40 of the 41 `vitest.config.ts` files say nothing about pool sizing — the single mention, in `packages/cli`, is a comment recording a REJECTED lever — so every package takes vitest's default of `max(cores - 1, 1)`, which scales with the host rather than with the shard it was given. Peak workers is the product of the two, and both terms grow with core count. Measured on a 4-CPU/15GB container, the product law holds exactly: 2x3=6, 4x3=12, 4x2=8, 4x1=4 concurrent workers observed. The bound goes at the invocation layer, per #10149's recorded reasoning that worker allocation is a property of the shard rather than of any one package's config. No `vitest.config.ts` is touched. Two traps this shape exists to avoid, both measured rather than assumed: - turbo filters task environments, so `VITEST_MAX_WORKERS` alone does NOTHING. Through turbo it spawned 3 workers (the unbounded default) while the same variable on a direct `vitest run` spawned 1. The `globalPassThroughEnv` entry is what makes the lever real. - vitest's `maxWorkers` is a PIN, not a ceiling — `resolveMaxWorkers()` returns the configured value outright. A flat `4` produced 8 workers at outer=2 where the default produces 6, i.e. a flat number RAISES the count on small boxes. So the cap is computed against the host's own cores and only ever lowers. A no-op on any host with <= 5 cores, today's CI runners included. In the regime where it binds (outer=2, inner 8 -> 4 on the 7-package fleet) it cut worker RSS 5700MB -> 2475MB for 93s -> 95s of wall, inside this box's noise. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UjM2ia8Av1v5NqfqQEQmC6 --- .github/workflows/ci.yml | 14 +++ .github/workflows/rerun-safety-nightly.yml | 14 +++ package.json | 2 +- scripts/vitest-worker-cap.mjs | 103 +++++++++++++++++++++ turbo.json | 1 + 5 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 scripts/vitest-worker-cap.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fef3f452af..03f401b309 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -556,6 +556,13 @@ jobs: echo "No packages on this shard — nothing to test." exit 0 fi + # Bound vitest's INNER worker pool (#11958). A no-op on a runner with + # <= 5 cores (it only ever LOWERS vitest's own `cores - 1` default); + # it exists so a larger runner cannot multiply turbo's outer + # --concurrency by a host-sized inner pool. Empty on failure, which is + # vitest's own "use the default" signal. Needs turbo.json's + # globalPassThroughEnv entry or turbo strips it — see the script header. + export VITEST_MAX_WORKERS="$(node scripts/vitest-worker-cap.mjs)" FILTERS=$(sed 's/^/--filter=/' "$RUNNER_TEMP/shard-packages.txt" | tr '\n' ' ') mkdir -p "$RUNNER_TEMP/stall-reports" node scripts/run-with-stall-guard.mjs --log "$RUNNER_TEMP/test-core.log" --stall-minutes 10 \ @@ -1129,6 +1136,13 @@ jobs: env: NODE_OPTIONS: --report-on-signal --report-signal=SIGUSR2 --report-directory=${{ runner.temp }}/stall-reports run: | + # Bound vitest's INNER worker pool (#11958). A no-op on a runner with + # <= 5 cores (it only ever LOWERS vitest's own `cores - 1` default); + # it exists so a larger runner cannot multiply turbo's outer + # --concurrency by a host-sized inner pool. Empty on failure, which is + # vitest's own "use the default" signal. Needs turbo.json's + # globalPassThroughEnv entry or turbo strips it — see the script header. + export VITEST_MAX_WORKERS="$(node scripts/vitest-worker-cap.mjs)" mkdir -p "$RUNNER_TEMP/stall-reports" node scripts/run-with-stall-guard.mjs --log "$RUNNER_TEMP/dogfood.log" --stall-minutes 10 \ --report-dir "$RUNNER_TEMP/stall-reports" -- \ diff --git a/.github/workflows/rerun-safety-nightly.yml b/.github/workflows/rerun-safety-nightly.yml index 41e5b73acc..25990c9d57 100644 --- a/.github/workflows/rerun-safety-nightly.yml +++ b/.github/workflows/rerun-safety-nightly.yml @@ -89,6 +89,13 @@ jobs: # frozen while healthy. The guard refuses a turbo run without it. - name: Test suite — pass 1 run: | + # Bound vitest's INNER worker pool (#11958). A no-op on a runner with + # <= 5 cores (it only ever LOWERS vitest's own `cores - 1` default); + # it exists so a larger runner cannot multiply turbo's outer + # --concurrency by a host-sized inner pool. Empty on failure, which is + # vitest's own "use the default" signal. Needs turbo.json's + # globalPassThroughEnv entry or turbo strips it — see the script header. + export VITEST_MAX_WORKERS="$(node scripts/vitest-worker-cap.mjs)" node scripts/run-with-stall-guard.mjs --log "$RUNNER_TEMP/rerun-pass1.log" --stall-minutes 15 -- \ pnpm turbo run test --concurrency=4 --force --log-order=stream @@ -112,6 +119,13 @@ jobs: # there. Keep the two verdicts apart. - name: Test suite — pass 2 (same working tree) run: | + # Bound vitest's INNER worker pool (#11958). A no-op on a runner with + # <= 5 cores (it only ever LOWERS vitest's own `cores - 1` default); + # it exists so a larger runner cannot multiply turbo's outer + # --concurrency by a host-sized inner pool. Empty on failure, which is + # vitest's own "use the default" signal. Needs turbo.json's + # globalPassThroughEnv entry or turbo strips it — see the script header. + export VITEST_MAX_WORKERS="$(node scripts/vitest-worker-cap.mjs)" status=0 node scripts/run-with-stall-guard.mjs --log "$RUNNER_TEMP/rerun-pass2.log" --stall-minutes 15 -- \ pnpm turbo run test --concurrency=4 --force --log-order=stream || status=$? diff --git a/package.json b/package.json index f1a1937b1d..92bc76c478 100644 --- a/package.json +++ b/package.json @@ -10,7 +10,7 @@ "dev:crm": "node scripts/check-dev-prereqs.mjs && pnpm check:console-sha && pnpm --filter @objectstack/example-crm dev", "dev:todo": "node scripts/check-dev-prereqs.mjs && pnpm check:console-sha && pnpm --filter @objectstack/example-todo dev", "spec:rebuild": "turbo run build --filter=...@objectstack/spec", - "test": "turbo run test --concurrency=50%", + "test": "VITEST_MAX_WORKERS=$(node scripts/vitest-worker-cap.mjs) turbo run test --concurrency=50%", "test:e2e": "turbo run test:e2e", "typecheck": "turbo run typecheck", "clean": "turbo run clean && rm -rf dist", diff --git a/scripts/vitest-worker-cap.mjs b/scripts/vitest-worker-cap.mjs new file mode 100644 index 0000000000..b790feb1f0 --- /dev/null +++ b/scripts/vitest-worker-cap.mjs @@ -0,0 +1,103 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Prints the value for `VITEST_MAX_WORKERS` — the bound on vitest's INNER + * worker pool — for the root `test` script to export into `turbo run test`. + * + * ## Why this exists at all (#11958) + * + * Two fan-outs multiply and neither bounds the other: + * + * OUTER `turbo run test --concurrency=50%` — how many package `test` tasks + * run at once. Bounded as a share of the host's cores (#11954/#11938). + * INNER vitest's own pool inside EACH of those tasks. Unbounded: 40 of this + * repo's 41 `vitest.config.ts` files say nothing about pool sizing + * (the one mention, in `packages/cli`, is a COMMENT recording a + * rejected lever), so every package takes vitest's default, which is + * `max(availableParallelism() - 1, 1)` — i.e. it scales with the + * HOST's core count, not with the shard it was given. + * + * Peak concurrent test-worker processes is therefore `outer × inner`, and both + * terms grow with core count, so the product grows QUADRATICALLY. Measured on + * a 4-CPU/15GB container, the product law holds exactly — 2×3=6, 4×3=12, + * 4×2=8, 4×1=4 workers observed for those combinations. + * + * ## Why a CAP, computed here, and not a flat pinned number + * + * ⚠️ vitest's `maxWorkers` is a PIN, not a ceiling: `resolveMaxWorkers()` + * returns the configured value outright rather than `min()`-ing it with the + * default. Measured: `VITEST_MAX_WORKERS=4` on this 4-core box produced 8 + * concurrent workers at outer=2, where the DEFAULT produces 6. A flat number + * small enough to protect a 64-core box would tax every small box, and a flat + * number chosen for comfort would RAISE the count on small boxes. So the cap is + * applied here, against this host's own core count, and only ever lowers. + * + * The ceiling is 4 rather than 1-2 because oversubscription is what makes this + * suite fast — its cost is dominated by module IMPORT, not CPU. Holding the + * ceiling at 4 keeps today's oversubscription ratio roughly constant as core + * count grows (total ≈ 2 × cores) instead of letting it grow with the box. + * + * ## What it buys, measured in the regime where it binds + * + * On the 7-package fleet at outer=2, emulating a larger box by setting the + * inner pool explicitly (peak RSS is of the vitest processes only): + * + * inner=8 (a 9-core box's default) 16 workers 5700 MB workers 93s + * inner=4 (this cap) 8 workers 2475 MB workers 95s + * + * -57% worker RSS for ~0 wall-clock (93s vs 95s is inside this box's run-to-run + * noise; two same-config repeats differed by 17s). On a host with <= 5 cores + * this file returns the default unchanged, so it is a NO-OP for every box the + * project runs on today, CI runners included — which is the point: it bounds + * growth without taxing anyone now. + * + * ## The silent no-op this is paired with + * + * ⚠️ Exporting this variable does NOTHING on its own. Turbo filters task + * environments, so the variable must also be declared in `turbo.json` + * (`globalPassThroughEnv`). Measured before that line existed: + * `VITEST_MAX_WORKERS=1` through turbo spawned 3 workers — the unbounded + * default — while the same variable on a direct `vitest run` spawned 1. If you + * change either half, verify by OBSERVING the worker count (`ps` for + * `--experimental-import-meta-resolve` children), never by the value being + * accepted without error. + * + * ⚠️ The value must be a plain integer. vitest reads this variable with + * `Number.parseInt`, so a percentage — the spelling turbo's `--concurrency` + * accepts — is silently truncated: `VITEST_MAX_WORKERS=50%` means FIFTY + * workers, not half the box. + */ + +import os from 'node:os'; + +/** vitest's own default: `max(availableParallelism() - 1, 1)` (non-watch). */ +export function vitestDefaultWorkers(cores) { + return Math.max(cores - 1, 1); +} + +/** The ceiling. Only ever lowers vitest's default — never raises it. */ +export const WORKER_CEILING = 4; + +export function workerCap(cores) { + return Math.min(vitestDefaultWorkers(cores), WORKER_CEILING); +} + +// An explicit value from the environment wins: a developer profiling one +// package, or a CI job that knows its own runner, is a better judge of its +// shard than this file's host-relative guess. Only a positive integer is +// honoured — anything else falls through to the computed cap rather than +// reaching vitest as NaN. +const override = Number.parseInt(process.env.VITEST_MAX_WORKERS ?? '', 10); +if (Number.isInteger(override) && override > 0) { + process.stdout.write(String(override)); + process.exit(0); +} + +const cores = + typeof os.availableParallelism === 'function' ? os.availableParallelism() : os.cpus().length; + +// A non-integer here would reach vitest as NaN and break the pool, so the +// output is validated rather than trusted. An empty/absent value is vitest's +// own "use the default" signal, which is the safe way to fail. +const value = workerCap(cores); +process.stdout.write(Number.isInteger(value) && value > 0 ? String(value) : ''); diff --git a/turbo.json b/turbo.json index da7121812e..3d4dc9f53d 100644 --- a/turbo.json +++ b/turbo.json @@ -2,6 +2,7 @@ "$schema": "https://turbo.build/schema.json", "globalDependencies": ["tsconfig.json", "tsup.config.ts"], "globalEnv": ["OS_SKIP_DTS"], + "globalPassThroughEnv": ["VITEST_MAX_WORKERS"], "tasks": { "build": { "dependsOn": ["^build"], From a3dc8414007f36ff68687f617760bb7f9994e153 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 12:06:24 +0000 Subject: [PATCH 2/2] fix(devx): guard the worker-cap script's top level per check:entry-guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The file exports `workerCap`/`WORKER_CEILING` for tests and callers, so its top-level dispatch must not run inside an importer — `check:entry-guard` measured 8 of 39 unguarded exporters ending the importer mid-import, five of them exit 0. Value resolution moves into an exported `resolveValue()` and the write sits behind `isEntrypoint`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UjM2ia8Av1v5NqfqQEQmC6 --- scripts/vitest-worker-cap.mjs | 40 ++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/scripts/vitest-worker-cap.mjs b/scripts/vitest-worker-cap.mjs index b790feb1f0..54237fd0b0 100644 --- a/scripts/vitest-worker-cap.mjs +++ b/scripts/vitest-worker-cap.mjs @@ -69,6 +69,7 @@ */ import os from 'node:os'; +import { isEntrypoint } from './invoked-as.mjs'; /** vitest's own default: `max(availableParallelism() - 1, 1)` (non-watch). */ export function vitestDefaultWorkers(cores) { @@ -82,22 +83,27 @@ export function workerCap(cores) { return Math.min(vitestDefaultWorkers(cores), WORKER_CEILING); } -// An explicit value from the environment wins: a developer profiling one -// package, or a CI job that knows its own runner, is a better judge of its -// shard than this file's host-relative guess. Only a positive integer is -// honoured — anything else falls through to the computed cap rather than -// reaching vitest as NaN. -const override = Number.parseInt(process.env.VITEST_MAX_WORKERS ?? '', 10); -if (Number.isInteger(override) && override > 0) { - process.stdout.write(String(override)); - process.exit(0); +/** + * Resolves the value to print. An explicit value from the environment wins: a + * developer profiling one package, or a CI job that knows its own runner, is a + * better judge of its shard than this file's host-relative guess. Only a + * positive integer is honoured — anything else falls through to the computed + * cap rather than reaching vitest as NaN. + */ +export function resolveValue(env = process.env) { + const override = Number.parseInt(env.VITEST_MAX_WORKERS ?? '', 10); + if (Number.isInteger(override) && override > 0) return override; + const cores = + typeof os.availableParallelism === 'function' ? os.availableParallelism() : os.cpus().length; + return workerCap(cores); } -const cores = - typeof os.availableParallelism === 'function' ? os.availableParallelism() : os.cpus().length; - -// A non-integer here would reach vitest as NaN and break the pool, so the -// output is validated rather than trusted. An empty/absent value is vitest's -// own "use the default" signal, which is the safe way to fail. -const value = workerCap(cores); -process.stdout.write(Number.isInteger(value) && value > 0 ? String(value) : ''); +// Guarded per `check:entry-guard`: this file exports bindings, so its top level +// must not run inside an importer. +if (isEntrypoint(import.meta.url)) { + // A non-integer here would reach vitest as NaN and break the pool, so the + // output is validated rather than trusted. An empty value is vitest's own + // "use the default" signal, which is the safe way to fail. + const value = resolveValue(); + process.stdout.write(Number.isInteger(value) && value > 0 ? String(value) : ''); +}