From 5b70e00760ad2ef47540d35857e8f02720eb8b71 Mon Sep 17 00:00:00 2001 From: Aiden Bai Date: Thu, 30 Jul 2026 03:01:22 +0000 Subject: [PATCH] fix(core): cap workspace oxlint subprocesses --- .changeset/calm-workers-share.md | 5 + packages/api/src/diagnose.ts | 29 +++- .../src/dead-code/dead-code-worker-slots.ts | 67 ++------- packages/core/src/index.ts | 2 + packages/core/src/refs.ts | 8 ++ packages/core/src/run-oxlint.ts | 4 + .../core/src/runners/oxlint/spawn-batches.ts | 51 +++++-- .../core/src/runners/oxlint/spawn-oxlint.ts | 2 + packages/core/src/services/linter.ts | 3 + .../src/utils/create-oxlint-spawn-slots.ts | 15 ++ .../core/src/utils/create-worker-slots.ts | 65 +++++++++ .../core/tests/create-worker-slots.test.ts | 98 +++++++++++++ packages/core/tests/spawn-batches.test.ts | 134 ++++++++++++++++++ .../react-doctor/src/cli/commands/inspect.ts | 7 +- .../react-doctor/src/cli/ink/run-scan-app.tsx | 25 +++- .../src/cli/utils/build-runtime-layers.ts | 22 ++- packages/react-doctor/src/inspect.ts | 45 +++++- .../tests/ink/run-scan-app.test.ts | 12 +- .../tests/inspect-action-exit-code.test.ts | 12 +- .../tests/inspect-action-setup-prompt.test.ts | 12 +- .../tests/inspect-action-staged-guard.test.ts | 12 +- .../inspect-action-staged-materialize.test.ts | 8 +- 22 files changed, 532 insertions(+), 106 deletions(-) create mode 100644 .changeset/calm-workers-share.md create mode 100644 packages/core/src/utils/create-oxlint-spawn-slots.ts create mode 100644 packages/core/src/utils/create-worker-slots.ts create mode 100644 packages/core/tests/create-worker-slots.test.ts diff --git a/.changeset/calm-workers-share.md b/.changeset/calm-workers-share.md new file mode 100644 index 0000000000..d8f8d5e18d --- /dev/null +++ b/.changeset/calm-workers-share.md @@ -0,0 +1,5 @@ +--- +"react-doctor": patch +--- + +Cap Oxlint subprocesses across concurrent workspace project scans so project-level parallelism cannot multiply the configured worker budget. diff --git a/packages/api/src/diagnose.ts b/packages/api/src/diagnose.ts index 3b3d2ccb86..0be96788e5 100644 --- a/packages/api/src/diagnose.ts +++ b/packages/api/src/diagnose.ts @@ -3,6 +3,7 @@ import * as Layer from "effect/Layer"; import { buildSkippedChecks, Config, + createOxlintSpawnSlots, DEFAULT_PROJECT_SCAN_CONCURRENCY, DEFAULT_SHOW_WARNINGS, DeadCode, @@ -15,6 +16,8 @@ import { LintPartialFailures, mapWithConcurrency, mergeReactDoctorConfigs, + OxlintConcurrency, + OxlintSpawnSlots, Progress, Project, Reporter, @@ -25,6 +28,7 @@ import { SupplyChain, type InspectOutput, type ResolvedScanTarget, + type WorkerSlots, } from "@react-doctor/core"; import type { DiagnoseOptions, @@ -53,6 +57,8 @@ interface DiagnoseLayerInput { readonly config: ReactDoctorConfig | null; readonly shouldRunLint: boolean; readonly shouldRunDeadCode: boolean; + readonly oxlintConcurrency: number; + readonly oxlintSpawnSlots: WorkerSlots; readonly configOverrideTarget?: Pick< ResolvedScanTarget, "resolvedDirectory" | "configSourceDirectory" @@ -86,6 +92,8 @@ const buildDiagnoseLayer = (input: DiagnoseLayerInput) => { Git.layerNode, input.shouldRunLint ? Linter.layerOxlint : Linter.layerOf([]), LintPartialFailures.layerLive, + Layer.succeed(OxlintConcurrency, input.oxlintConcurrency), + Layer.succeed(OxlintSpawnSlots, input.oxlintSpawnSlots), Progress.layerNoop, Reporter.layerNoop, Score.layerHttp, @@ -154,6 +162,8 @@ const diagnoseDirectory = async ( const program = buildInspectProgram(scanTarget, options); const shouldRunLint = resolveShouldRunLint(options, scanTarget.userConfig); const shouldRunDeadCode = resolveShouldRunDeadCode(options, scanTarget.userConfig); + const oxlintConcurrency = Effect.runSync(OxlintConcurrency); + const oxlintSpawnSlots = createOxlintSpawnSlots(oxlintConcurrency); const output: InspectOutput = await Effect.runPromise( restoreLegacyThrow( @@ -163,6 +173,8 @@ const diagnoseDirectory = async ( config: scanTarget.userConfig, shouldRunLint, shouldRunDeadCode, + oxlintConcurrency, + oxlintSpawnSlots, }), ), Effect.provide(layerOtlp), @@ -190,6 +202,8 @@ const diagnoseProject = async ( projectDefinition: ProjectDefinition, baseOptions: DiagnoseOptions, batchConfig: ReactDoctorConfig | undefined, + oxlintConcurrency: number, + oxlintSpawnSlots: WorkerSlots, ): Promise => { const startTime = globalThis.performance.now(); @@ -220,6 +234,8 @@ const diagnoseProject = async ( config: effectiveConfig, shouldRunLint, shouldRunDeadCode, + oxlintConcurrency, + oxlintSpawnSlots, configOverrideTarget: { resolvedDirectory: scanTarget.resolvedDirectory, configSourceDirectory: didOverridePlugins ? null : scanTarget.configSourceDirectory, @@ -229,6 +245,8 @@ const diagnoseProject = async ( config: effectiveConfig, shouldRunLint, shouldRunDeadCode, + oxlintConcurrency, + oxlintSpawnSlots, }; const layer = buildDiagnoseLayer(diagnoseLayerInput); @@ -256,13 +274,22 @@ const diagnoseProjectBatch = async ( warnIfAiTrainingEnvironment(); const startTime = globalThis.performance.now(); const { projects, concurrency, config: batchConfig, ...baseOptions } = input; + const oxlintConcurrency = Effect.runSync(OxlintConcurrency); + const oxlintSpawnSlots = createOxlintSpawnSlots(oxlintConcurrency); // `diagnoseProject` never rejects (failures come back as `ok: false`), // so the pool always drains every project. const projectResults = await mapWithConcurrency( projects, concurrency ?? DEFAULT_PROJECT_SCAN_CONCURRENCY, - (projectDefinition) => diagnoseProject(projectDefinition, baseOptions, batchConfig), + (projectDefinition) => + diagnoseProject( + projectDefinition, + baseOptions, + batchConfig, + oxlintConcurrency, + oxlintSpawnSlots, + ), ); const succeededProjects = projectResults.filter((projectResult) => projectResult.ok); diff --git a/packages/core/src/dead-code/dead-code-worker-slots.ts b/packages/core/src/dead-code/dead-code-worker-slots.ts index 5ce426cc75..19e912a044 100644 --- a/packages/core/src/dead-code/dead-code-worker-slots.ts +++ b/packages/core/src/dead-code/dead-code-worker-slots.ts @@ -1,67 +1,16 @@ import { resolveDeadCodeConcurrency } from "../utils/resolve-dead-code-concurrency.js"; +import { createWorkerSlots } from "../utils/create-worker-slots.js"; +import type { WorkerSlots } from "../utils/create-worker-slots.js"; -// A process-global counting semaphore bounding how many real deslop dead-code -// child processes run at once, to the memory budget (`resolveDeadCodeConcurrency`). -// -// It's process-global on purpose: the CLI scans the projects of a workspace in -// concurrent `runInspect` fibers within ONE process, and each spawns its own -// dead-code worker — without a shared cap, N concurrent projects could -// oversubscribe memory with N simultaneous children on a small runner. This -// gates only HOW MANY start; each worker still self-terminates via the proven -// one-shot lifecycle (spawn → analyze → exit), so the semaphore adds no -// process-lifecycle surface — it's plain in-process bookkeeping. -// -// `-1` is the un-initialized sentinel; the first acquirer reads the budget once -// (after which the cap is fixed for the process). -let availableSlots = -1; -const waiters: Array<() => void> = []; +let deadCodeWorkerSlots: WorkerSlots | null = null; -const releaseSlot = (): void => { - const nextWaiter = waiters.shift(); - // Hand the slot straight to the next waiter (no increment); only return it to - // the pool when nobody is waiting. Keeps the count balanced either way. - if (nextWaiter !== undefined) nextWaiter(); - else availableSlots += 1; -}; - -/** - * Runs `task` once a dead-code worker slot is free, releasing the slot when the - * task settles (success or failure). With a high cap (roomy machine) every - * caller proceeds immediately; with a low cap (constrained runner) callers - * queue and run as slots free. - * - * `abortSignal` short-circuits the WAIT: if it's already aborted, or fires while - * this caller is queued, the call rejects without acquiring a slot or running - * `task` — so a cancelled scan (e.g. lint failed) doesn't sit in the queue and - * then spawn a child only to tear it down. A queued caller that aborts removes - * its own waiter so a later release never hands a slot to a dead request. - */ export const withDeadCodeWorkerSlot = async ( task: () => Promise, abortSignal?: AbortSignal, ): Promise => { - if (abortSignal?.aborted) throw new Error("Dead-code worker aborted."); - if (availableSlots < 0) availableSlots = resolveDeadCodeConcurrency(); - if (availableSlots > 0) { - availableSlots -= 1; - } else { - await new Promise((resolve, reject) => { - const waiter = (): void => { - abortSignal?.removeEventListener("abort", onAbort); - resolve(); - }; - const onAbort = (): void => { - const queuedIndex = waiters.indexOf(waiter); - if (queuedIndex !== -1) waiters.splice(queuedIndex, 1); - reject(new Error("Dead-code worker aborted.")); - }; - waiters.push(waiter); - abortSignal?.addEventListener("abort", onAbort, { once: true }); - }); - } - try { - return await task(); - } finally { - releaseSlot(); - } + deadCodeWorkerSlots ??= createWorkerSlots({ + slotCount: resolveDeadCodeConcurrency(), + createAbortError: () => new Error("Dead-code worker aborted."), + }); + return deadCodeWorkerSlots.run(task, abortSignal); }; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 8d8a829745..a6a70b616e 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -89,6 +89,8 @@ export * from "./utils/assign-fix-groups.js"; export * from "./utils/build-rule-docs-url.js"; export * from "./utils/classify-package-role.js"; export * from "./utils/compute-config-fingerprint.js"; +export * from "./utils/create-oxlint-spawn-slots.js"; +export * from "./utils/create-worker-slots.js"; export * from "./utils/dedupe-diagnostics.js"; export * from "./utils/define-config.js"; export * from "./utils/detect-ai-training-environment.js"; diff --git a/packages/core/src/refs.ts b/packages/core/src/refs.ts index ab42473eb9..c01667079b 100644 --- a/packages/core/src/refs.ts +++ b/packages/core/src/refs.ts @@ -12,6 +12,7 @@ import { readPositiveEnvMs } from "./utils/read-positive-env-ms.js"; import { resolveAutoScanConcurrency } from "./utils/resolve-auto-scan-concurrency.js"; import { resolveLintBatchOrdering } from "./utils/resolve-lint-batch-ordering.js"; import { resolveScanConcurrency } from "./utils/resolve-scan-concurrency.js"; +import type { WorkerSlots } from "./utils/create-worker-slots.js"; /** * Per-batch oxlint wall-clock budget. Reads from the env var on @@ -126,6 +127,13 @@ export class OxlintConcurrency extends Context.Reference("react-doctor/O }, }) {} +export class OxlintSpawnSlots extends Context.Reference( + "react-doctor/OxlintSpawnSlots", + { + defaultValue: () => null, + }, +) {} + /** * Three-state control for overlapping the dead-code pass with the lint pass — * forking dead-code as a child fiber that runs DURING lint instead of strictly diff --git a/packages/core/src/run-oxlint.ts b/packages/core/src/run-oxlint.ts index 24ef540bb5..1f7b0899ba 100644 --- a/packages/core/src/run-oxlint.ts +++ b/packages/core/src/run-oxlint.ts @@ -28,6 +28,7 @@ import type { SidecarDependencyProbe, SidecarLintCache, } from "./runners/oxlint/sidecar-lint-cache.js"; +import type { WorkerSlots } from "./utils/create-worker-slots.js"; import { resolveUserPlugins } from "./runners/oxlint/plugin-resolution.js"; import { resolveOxlintToolchainVersions } from "./runners/oxlint/resolve-toolchain-versions.js"; import { @@ -133,6 +134,7 @@ interface RunOxlintOptions { * exhaustion (see `spawnLintBatches`). */ concurrency?: number; + spawnSlots?: WorkerSlots; /** * Aborted when the orchestrator's lint-phase timeout fires; forwarded to * `spawnLintBatches` so in-flight oxlint subprocesses are torn down instead @@ -634,6 +636,7 @@ export const runOxlint = async (options: RunOxlintOptions): Promise => { // Past the --max-duration budget: skip instead of spawning, even inside a // binary-split retry, so a batch that started just before the deadline @@ -228,14 +237,31 @@ export const spawnLintBatches = async (input: SpawnLintBatchesInput): Promise => { + if (isPastDeadline()) { + deadlineSkippedFiles.push(...batch); + batchState.deadlineSkippedFileCount += batch.length; + return Promise.resolve(null); + } + return spawnOxlint( + batchArgs, + rootDirectory, + nodeBinaryPath, + spawnTimeoutMs, + outputMaxBytes, + signal, + () => { + if (batchState.didStart) return; + batchState.didStart = true; + startedFileCount += batchState.initialFileCount; + }, + ); + }; + const stdout = + input.spawnSlots === undefined + ? await spawnBatch() + : await input.spawnSlots.run(spawnBatch, signal); + if (stdout === null) return []; return parseOxlintOutput(stdout, project, rootDirectory, sourcePathByLintPath); } catch (error) { if (!isSplittableReactDoctorError(error)) throw error; @@ -302,16 +328,17 @@ export const spawnLintBatches = async (input: SpawnLintBatchesInput): Promise void, ): Promise => new Promise((resolve, reject) => { if (abortSignal?.aborted) { @@ -52,6 +53,7 @@ export const spawnOxlint = ( ); return; } + onSpawn?.(); const child = spawn( nodeBinaryPath, buildProfiledNodeArguments({ diff --git a/packages/core/src/services/linter.ts b/packages/core/src/services/linter.ts index e2cec40dcc..6f75912ac5 100644 --- a/packages/core/src/services/linter.ts +++ b/packages/core/src/services/linter.ts @@ -9,6 +9,7 @@ import { LintBatchOrdering, OxlintConcurrency, OxlintOutputMaxBytes, + OxlintSpawnSlots, OxlintSpawnTimeoutMs, PerFileLintCacheEnabled, SidecarLintCacheEnabled, @@ -129,6 +130,7 @@ export class Linter extends Context.Service< const spawnTimeoutMs = yield* OxlintSpawnTimeoutMs; const outputMaxBytes = yield* OxlintOutputMaxBytes; const concurrency = yield* OxlintConcurrency; + const spawnSlots = yield* OxlintSpawnSlots; const lintBatchOrdering = yield* LintBatchOrdering; const perFileLintCacheEnabled = yield* PerFileLintCacheEnabled; const sidecarLintCacheEnabled = yield* SidecarLintCacheEnabled; @@ -162,6 +164,7 @@ export class Linter extends Context.Service< onSidecarStats: input.onSidecarStats, spawnTimeoutMs, outputMaxBytes, + spawnSlots: spawnSlots ?? undefined, concurrency, signal, lintBatchOrdering, diff --git a/packages/core/src/utils/create-oxlint-spawn-slots.ts b/packages/core/src/utils/create-oxlint-spawn-slots.ts new file mode 100644 index 0000000000..dcb37fdfee --- /dev/null +++ b/packages/core/src/utils/create-oxlint-spawn-slots.ts @@ -0,0 +1,15 @@ +import { OxlintSpawnFailed, ReactDoctorError } from "../errors.js"; +import { createWorkerSlots } from "./create-worker-slots.js"; +import type { WorkerSlots } from "./create-worker-slots.js"; +import { resolveScanConcurrency } from "./resolve-scan-concurrency.js"; + +const createLintPhaseAbortError = (): ReactDoctorError => + new ReactDoctorError({ + reason: new OxlintSpawnFailed({ cause: "lint phase aborted" }), + }); + +export const createOxlintSpawnSlots = (concurrency: number): WorkerSlots => + createWorkerSlots({ + slotCount: resolveScanConcurrency(concurrency), + createAbortError: createLintPhaseAbortError, + }); diff --git a/packages/core/src/utils/create-worker-slots.ts b/packages/core/src/utils/create-worker-slots.ts new file mode 100644 index 0000000000..a2c20262b9 --- /dev/null +++ b/packages/core/src/utils/create-worker-slots.ts @@ -0,0 +1,65 @@ +export interface WorkerSlots { + readonly run: (task: () => Promise, abortSignal?: AbortSignal) => Promise; +} + +interface WorkerSlotWaiter { + readonly resolve: () => void; + readonly abortSignal: AbortSignal | undefined; + readonly onAbort: () => void; +} + +interface CreateWorkerSlotsInput { + readonly slotCount: number; + readonly createAbortError: () => Error; +} + +export const createWorkerSlots = (input: CreateWorkerSlotsInput): WorkerSlots => { + let availableSlotCount = input.slotCount; + const waiters: WorkerSlotWaiter[] = []; + + const releaseSlot = (): void => { + const nextWaiter = waiters.shift(); + if (nextWaiter === undefined) { + availableSlotCount += 1; + return; + } + nextWaiter.abortSignal?.removeEventListener("abort", nextWaiter.onAbort); + nextWaiter.resolve(); + }; + + const acquireSlot = async (abortSignal?: AbortSignal): Promise => { + if (abortSignal?.aborted) throw input.createAbortError(); + if (availableSlotCount > 0) { + availableSlotCount -= 1; + return; + } + await new Promise((resolve, reject) => { + const onAbort = (): void => { + const waiterIndex = waiters.indexOf(waiter); + if (waiterIndex !== -1) waiters.splice(waiterIndex, 1); + reject(input.createAbortError()); + }; + const waiter: WorkerSlotWaiter = { + resolve, + abortSignal, + onAbort, + }; + waiters.push(waiter); + abortSignal?.addEventListener("abort", onAbort, { once: true }); + }); + }; + + return { + run: async ( + task: () => Promise, + abortSignal?: AbortSignal, + ): Promise => { + await acquireSlot(abortSignal); + try { + return await task(); + } finally { + releaseSlot(); + } + }, + }; +}; diff --git a/packages/core/tests/create-worker-slots.test.ts b/packages/core/tests/create-worker-slots.test.ts new file mode 100644 index 0000000000..0c9f7b468d --- /dev/null +++ b/packages/core/tests/create-worker-slots.test.ts @@ -0,0 +1,98 @@ +import { describe, expect, it } from "vite-plus/test"; +import { createWorkerSlots } from "../src/utils/create-worker-slots.js"; + +interface Deferred { + readonly promise: Promise; + readonly resolve: () => void; +} + +const createDeferred = (): Deferred => { + let resolvePromise = (): void => {}; + const promise = new Promise((resolve) => { + resolvePromise = resolve; + }); + return { promise, resolve: resolvePromise }; +}; + +const flushTasks = (): Promise => new Promise((resolve) => setImmediate(resolve)); + +const createTestWorkerSlots = (slotCount: number) => + createWorkerSlots({ + slotCount, + createAbortError: () => new Error("aborted"), + }); + +describe("createWorkerSlots", () => { + it("enforces the peak slot count and admits queued tasks in FIFO order", async () => { + const workerSlots = createTestWorkerSlots(2); + const firstRelease = createDeferred(); + const secondRelease = createDeferred(); + const thirdRelease = createDeferred(); + const fourthRelease = createDeferred(); + const startedTasks: string[] = []; + let runningTaskCount = 0; + let peakRunningTaskCount = 0; + + const runTask = (name: string, release: Deferred): Promise => + workerSlots.run(async () => { + startedTasks.push(name); + runningTaskCount += 1; + peakRunningTaskCount = Math.max(peakRunningTaskCount, runningTaskCount); + await release.promise; + runningTaskCount -= 1; + return name; + }); + + const results = Promise.all([ + runTask("first", firstRelease), + runTask("second", secondRelease), + runTask("third", thirdRelease), + runTask("fourth", fourthRelease), + ]); + await flushTasks(); + expect(startedTasks).toEqual(["first", "second"]); + + secondRelease.resolve(); + await flushTasks(); + expect(startedTasks).toEqual(["first", "second", "third"]); + + firstRelease.resolve(); + await flushTasks(); + expect(startedTasks).toEqual(["first", "second", "third", "fourth"]); + + thirdRelease.resolve(); + fourthRelease.resolve(); + expect(await results).toEqual(["first", "second", "third", "fourth"]); + expect(peakRunningTaskCount).toBe(2); + }); + + it("releases slots after rejection", async () => { + const workerSlots = createTestWorkerSlots(1); + await expect( + workerSlots.run(async () => { + throw new Error("failed"); + }), + ).rejects.toThrow("failed"); + await expect(workerSlots.run(async () => "after")).resolves.toBe("after"); + }); + + it("removes an aborted waiter without running it or leaking a slot", async () => { + const workerSlots = createTestWorkerSlots(1); + const heldRelease = createDeferred(); + const heldTask = workerSlots.run(() => heldRelease.promise); + await flushTasks(); + + const abortController = new AbortController(); + let didRunAbortedTask = false; + const abortedTask = workerSlots.run(async () => { + didRunAbortedTask = true; + }, abortController.signal); + abortController.abort(); + + await expect(abortedTask).rejects.toThrow("aborted"); + expect(didRunAbortedTask).toBe(false); + heldRelease.resolve(); + await heldTask; + await expect(workerSlots.run(async () => "after")).resolves.toBe("after"); + }); +}); diff --git a/packages/core/tests/spawn-batches.test.ts b/packages/core/tests/spawn-batches.test.ts index 85039bd482..37d9aa4c05 100644 --- a/packages/core/tests/spawn-batches.test.ts +++ b/packages/core/tests/spawn-batches.test.ts @@ -17,6 +17,7 @@ import * as path from "node:path"; import { describe, expect, it } from "vite-plus/test"; import type { ProjectInfo } from "@react-doctor/core"; import { spawnLintBatches } from "../src/runners/oxlint/spawn-batches.js"; +import { createOxlintSpawnSlots } from "../src/utils/create-oxlint-spawn-slots.js"; const project: ProjectInfo = { rootDirectory: "/tmp/app", @@ -155,6 +156,64 @@ describe("spawnLintBatches concurrency", () => { const peak = await runMarkedBatches(4, 1); expect(peak).toBe(1); }); + + it("shares one subprocess cap across concurrent project batch runners", async () => { + const markDirectory = fs.mkdtempSync(path.join(os.tmpdir(), "rd-shared-parallel-")); + const markFile = path.join(markDirectory, "marks.txt"); + fs.writeFileSync(markFile, ""); + const script = [ + 'const fs = require("fs");', + `const markFile = ${JSON.stringify(markFile)};`, + 'fs.appendFileSync(markFile, "+");', + "const files = process.argv.slice(1);", + "setTimeout(() => {", + ' fs.appendFileSync(markFile, "-");', + " const diagnostics = files.map((filename) => ({", + ' message: "Array index used as a key",', + ' code: "react-doctor(no-array-index-as-key)",', + ' severity: "warning",', + ' causes: [], url: "", help: "",', + " filename,", + ' labels: [{ label: "", span: { offset: 0, length: 1, line: 1, column: 1 } }],', + " related: [],", + " }));", + " process.stdout.write(JSON.stringify({ diagnostics, number_of_files: files.length, number_of_rules: 1 }));", + `}, ${SLEEP_MS});`, + ].join("\n"); + const spawnSlots = createOxlintSpawnSlots(2); + const runProjectBatches = (projectName: string) => + spawnLintBatches({ + baseArgs: ["-e", script], + fileBatches: Array.from({ length: 3 }, (_unused, index) => [ + `src/${projectName}-${index}.tsx`, + ]), + rootDirectory: process.cwd(), + nodeBinaryPath: process.execPath, + project, + concurrency: 3, + spawnSlots, + }); + + try { + const [firstDiagnostics, secondDiagnostics] = await Promise.all([ + runProjectBatches("first"), + runProjectBatches("second"), + ]); + expect(computePeakConcurrency(fs.readFileSync(markFile, "utf8"))).toBe(2); + expect(firstDiagnostics.map((diagnostic) => diagnostic.filePath)).toEqual([ + "src/first-0.tsx", + "src/first-1.tsx", + "src/first-2.tsx", + ]); + expect(secondDiagnostics.map((diagnostic) => diagnostic.filePath)).toEqual([ + "src/second-0.tsx", + "src/second-1.tsx", + "src/second-2.tsx", + ]); + } finally { + fs.rmSync(markDirectory, { recursive: true, force: true }); + } + }); }); /** @@ -178,6 +237,81 @@ const EMIT_ONE_DIAGNOSTIC_PER_FILE_SCRIPT = [ "process.stdout.write(JSON.stringify({ diagnostics, number_of_files: files.length, number_of_rules: 1 }));", ].join("\n"); +describe("spawnLintBatches shared slot timing", () => { + it("starts the subprocess timeout only after a queued slot is acquired", async () => { + const spawnSlots = createOxlintSpawnSlots(1); + const progressUpdates: Array = []; + let releaseHeldSlot = (): void => {}; + const heldSlot = spawnSlots.run( + () => + new Promise((resolve) => { + releaseHeldSlot = resolve; + }), + ); + await Promise.resolve(); + + const diagnosticsPromise = spawnLintBatches({ + baseArgs: ["-e", EMIT_ONE_DIAGNOSTIC_PER_FILE_SCRIPT], + fileBatches: [["src/queued-a.tsx", "src/queued-b.tsx"]], + rootDirectory: process.cwd(), + nodeBinaryPath: process.execPath, + project, + spawnTimeoutMs: 2_000, + spawnSlots, + onFileProgress: (scannedFileCount, totalFileCount) => { + progressUpdates.push([scannedFileCount, totalFileCount]); + }, + }); + await new Promise((resolve) => setTimeout(resolve, 2_200)); + expect(progressUpdates).toEqual([]); + releaseHeldSlot(); + await heldSlot; + + await expect(diagnosticsPromise).resolves.toMatchObject([ + { filePath: "src/queued-a.tsx" }, + { filePath: "src/queued-b.tsx" }, + ]); + expect(progressUpdates.at(-1)).toEqual([2, 2]); + }); + + it("rechecks the scan deadline after a queued slot is acquired", async () => { + const spawnSlots = createOxlintSpawnSlots(1); + let releaseHeldSlot = (): void => {}; + const heldSlot = spawnSlots.run( + () => + new Promise((resolve) => { + releaseHeldSlot = resolve; + }), + ); + await Promise.resolve(); + const partialFailures: string[] = []; + const progressUpdates: Array = []; + + const diagnosticsPromise = spawnLintBatches({ + baseArgs: ["-e", EMIT_ONE_DIAGNOSTIC_PER_FILE_SCRIPT], + fileBatches: [["src/deadline-a.tsx", "src/deadline-b.tsx"]], + rootDirectory: process.cwd(), + nodeBinaryPath: process.execPath, + project, + deadlineEpochMs: Date.now() + 50, + spawnSlots, + onPartialFailure: (reason) => partialFailures.push(reason), + onFileProgress: (scannedFileCount, totalFileCount) => { + progressUpdates.push([scannedFileCount, totalFileCount]); + }, + }); + await new Promise((resolve) => setTimeout(resolve, 100)); + releaseHeldSlot(); + await heldSlot; + + await expect(diagnosticsPromise).resolves.toEqual([]); + expect(partialFailures).toHaveLength(1); + expect(partialFailures[0]).toContain("2 file(s) skipped"); + expect(partialFailures[0]).toContain("max scan duration reached"); + expect(progressUpdates).toEqual([]); + }); +}); + const lintFileBatches = (fileBatches: string[][]) => spawnLintBatches({ baseArgs: ["-e", EMIT_ONE_DIAGNOSTIC_PER_FILE_SCRIPT], diff --git a/packages/react-doctor/src/cli/commands/inspect.ts b/packages/react-doctor/src/cli/commands/inspect.ts index abe91fa235..586ffd3d36 100644 --- a/packages/react-doctor/src/cli/commands/inspect.ts +++ b/packages/react-doctor/src/cli/commands/inspect.ts @@ -14,7 +14,7 @@ import { resolveScanTarget, toRelativePath, } from "@react-doctor/core"; -import { inspect } from "../../inspect.js"; +import { createInvocationInspect } from "../../inspect.js"; import { flushSentry } from "../../instrument.js"; import type { DiffInfo, @@ -382,6 +382,7 @@ export const inspectAction = async ( } const scanOptions: CliInspectOptions = resolveCliInspectOptions(flags, userConfig); + const inspectProject = createInvocationInspect(scanOptions.concurrency); // One `--max-duration` budget per invocation, shared by every project of a // workspace scan: fix the absolute deadline once here and hand it to each // project's `inspect()` (rather than restarting the budget per project). @@ -668,7 +669,7 @@ export const inspectAction = async ( snapshot.tempDirectory, projectScan.treeRelativeDirectory, ); - const scanResult = await inspect(projectTempDirectory, { + const scanResult = await inspectProject(projectTempDirectory, { ...scanOptions, deadlineEpochMs: scanDeadlineEpochMs, includePaths: [...includePaths], @@ -1006,7 +1007,7 @@ export const inspectAction = async ( if (!isQuiet && !isMultiProject) { logger.dim(" "); } - const scanResult = await inspect(scanDirectory, { + const scanResult = await inspectProject(scanDirectory, { ...scanOptions, deadlineEpochMs: scanDeadlineEpochMs, includePaths, diff --git a/packages/react-doctor/src/cli/ink/run-scan-app.tsx b/packages/react-doctor/src/cli/ink/run-scan-app.tsx index 0b4eabef27..f0de1b52cb 100644 --- a/packages/react-doctor/src/cli/ink/run-scan-app.tsx +++ b/packages/react-doctor/src/cli/ink/run-scan-app.tsx @@ -19,7 +19,7 @@ import type { ScoreResult, WorkspacePackage, } from "@react-doctor/core"; -import { inspect } from "../../inspect.js"; +import { createInvocationInspect } from "../../inspect.js"; import type { ReactDoctorInspectOptions } from "../../inspect.js"; import { buildNoScoreMessage } from "../utils/build-no-score-message.js"; import { computeProjectedScore } from "../utils/compute-score-projection.js"; @@ -379,11 +379,12 @@ const runSingleProjectScan = async ( projectDirectory: string, input: RunScanAppInput, blockingLevel: BlockingLevel, + inspectProject: ReturnType, ): Promise => { const projectScan = await resolveProjectScan(rootScanTarget, projectDirectory); const presentation = resolveScanPresentation(input, [projectScan]); return runMountedScan(projectScan.directory, presentation, blockingLevel, async (context) => { - const result = await inspect(projectScan.directory, { + const result = await inspectProject(projectScan.directory, { ...resolveTuiInspectOptions(input, projectScan.config), isCi: isCiEnvironment(), configOverride: projectScan.config, @@ -421,6 +422,7 @@ const runMultiProjectScan = async ( directories: ReadonlyArray, input: RunScanAppInput, blockingLevel: BlockingLevel, + inspectProject: ReturnType, ): Promise => { const rootDirectory = rootScanTarget.resolvedDirectory; const projectScans = await mapWithConcurrency( @@ -437,7 +439,7 @@ const runMultiProjectScan = async ( projectScans, DEFAULT_PROJECT_SCAN_CONCURRENCY, async (projectScan) => { - const result = await inspect(projectScan.directory, { + const result = await inspectProject(projectScan.directory, { ...resolveTuiInspectOptions(input, projectScan.config), isCi: isCiEnvironment(), configOverride: projectScan.config, @@ -514,6 +516,7 @@ export const runScanApp = async (input: RunScanAppInput): Promise; readonly progressLayer?: Layer.Layer; } @@ -161,7 +168,14 @@ export const buildRuntimeLayers = (input: BuildRuntimeLayersInput) => { // resolved a concrete worker count (today: `--no-parallel` → serial); // otherwise leave the env-seeded default (parallel) so // `REACT_DOCTOR_PARALLEL` still applies to flag-less runs. - return input.oxlintConcurrency === undefined - ? baseLayers - : Layer.mergeAll(baseLayers, Layer.succeed(OxlintConcurrency, input.oxlintConcurrency)); + const layersWithConcurrency = + input.oxlintConcurrency === undefined + ? baseLayers + : Layer.mergeAll(baseLayers, Layer.succeed(OxlintConcurrency, input.oxlintConcurrency)); + return input.oxlintSpawnSlots === undefined + ? layersWithConcurrency + : Layer.mergeAll( + layersWithConcurrency, + Layer.succeed(OxlintSpawnSlots, input.oxlintSpawnSlots), + ); }; diff --git a/packages/react-doctor/src/inspect.ts b/packages/react-doctor/src/inspect.ts index 5db7537795..9ca94b896b 100644 --- a/packages/react-doctor/src/inspect.ts +++ b/packages/react-doctor/src/inspect.ts @@ -7,19 +7,22 @@ import * as Effect from "effect/Effect"; import { buildSkippedChecks, computeDiagnosticDelta, + createOxlintSpawnSlots, DEFAULT_SHOW_WARNINGS, filterDiagnosticsForSurface, filterSourceFiles, highlighter, OXLINT_NODE_REQUIREMENT, + OxlintConcurrency, PerFileLintCacheEnabled, resolveScanTarget, + resolveScanConcurrency, restoreLegacyThrow, runInspect as runInspectEffect, SidecarLintCacheEnabled, } from "@react-doctor/core"; import type * as Layer from "effect/Layer"; -import type { Progress, Reporter } from "@react-doctor/core"; +import type { Progress, Reporter, WorkerSlots } from "@react-doctor/core"; import { applyObservability } from "./cli/utils/apply-observability.js"; import { buildRuntimeLayers } from "./cli/utils/build-runtime-layers.js"; import { @@ -92,6 +95,11 @@ import { VERSION } from "./cli/utils/version.js"; const silentConsole = makeNoopConsole(); +interface OxlintInvocationRuntime { + readonly concurrency: number; + readonly spawnSlots: WorkerSlots; +} + const runConsole = (effect: Effect.Effect): void => { Effect.runSync(effect); }; @@ -325,9 +333,10 @@ const buildRunEventConfig = ( }; }; -export const inspect = async ( +const inspectWithOxlintRuntime = async ( directory: string, - inputOptions: ReactDoctorInspectOptions = {}, + inputOptions: ReactDoctorInspectOptions, + oxlintRuntime: OxlintInvocationRuntime, ): Promise => { const startTime = performance.now(); // The CLI passes an absolute `deadlineEpochMs` shared across a workspace @@ -403,6 +412,7 @@ export const inspect = async ( startTime, deadlineEpochMs, rootSentrySpan, + oxlintRuntime, ); } catch (error) { // Emit the canonical wide event on the failure path too: the scan threw @@ -434,6 +444,26 @@ export const inspect = async ( } }; +export const createInvocationInspect = ( + requestedOxlintConcurrency?: number, +): ((directory: string, inputOptions?: ReactDoctorInspectOptions) => Promise) => { + const concurrency = resolveScanConcurrency( + requestedOxlintConcurrency ?? Effect.runSync(OxlintConcurrency), + ); + const oxlintRuntime: OxlintInvocationRuntime = { + concurrency, + spawnSlots: createOxlintSpawnSlots(concurrency), + }; + return (directory, inputOptions = {}) => + inspectWithOxlintRuntime(directory, inputOptions, oxlintRuntime); +}; + +export const inspect = async ( + directory: string, + inputOptions: ReactDoctorInspectOptions = {}, +): Promise => + createInvocationInspect(inputOptions.concurrency)(directory, inputOptions); + interface BaselineComparison { displayDiagnostics: ReadonlyArray; baselineDelta: NonNullable; @@ -468,6 +498,7 @@ interface RunBaselineComparisonInput { headAnalyzedFiles: ReadonlyArray; /** Shared invocation deadline; bounds the base-ref lint like the head scan. */ deadlineEpochMs: number | null; + oxlintRuntime: OxlintInvocationRuntime; } /** @@ -524,7 +555,8 @@ const runBaselineComparison = async ( shouldRunSupplyChain: params.options.supplyChain, shouldComputeScore: false, shouldShowProgressSpinners: false, - oxlintConcurrency: params.options.concurrency, + oxlintConcurrency: params.oxlintRuntime.concurrency, + oxlintSpawnSlots: params.oxlintRuntime.spawnSlots, }); const baseProgram = runInspectEffect( { @@ -612,6 +644,7 @@ const runInspectWithRuntime = async ( startTime: number, deadlineEpochMs: number | null, rootSentrySpan: SentryRootSpan, + oxlintRuntime: OxlintInvocationRuntime, ): Promise => { const isDiffMode = options.includePaths.length > 0; // Pre-check oxlint native binding the same way the legacy entry @@ -689,7 +722,8 @@ const runInspectWithRuntime = async ( shouldRunSupplyChain: options.supplyChain, shouldComputeScore: !options.noScore, shouldShowProgressSpinners, - oxlintConcurrency: options.concurrency, + oxlintConcurrency: oxlintRuntime.concurrency, + oxlintSpawnSlots: oxlintRuntime.spawnSlots, reporterLayer: options.uiLayers?.reporter, progressLayer: options.uiLayers?.progress, }); @@ -820,6 +854,7 @@ const runInspectWithRuntime = async ( headFiles: options.baseline.headFiles, headAnalyzedFiles: output.analyzedFiles, deadlineEpochMs, + oxlintRuntime, }); if (comparison) { inspectDiagnostics = comparison.displayDiagnostics; diff --git a/packages/react-doctor/tests/ink/run-scan-app.test.ts b/packages/react-doctor/tests/ink/run-scan-app.test.ts index c66ada8fe9..be92559368 100644 --- a/packages/react-doctor/tests/ink/run-scan-app.test.ts +++ b/packages/react-doctor/tests/ink/run-scan-app.test.ts @@ -62,13 +62,17 @@ vi.mock("@react-doctor/core", async (importOriginal) => { }; }); -vi.mock("../../src/inspect.js", () => ({ - inspect: vi.fn(async (directory: string): Promise => { +vi.mock("../../src/inspect.js", () => { + const inspect = vi.fn(async (directory: string): Promise => { const result = mockState.inspectResults.get(directory); if (!result) throw new Error(`Missing inspect result for ${directory}`); return result; - }), -})); + }); + return { + inspect, + createInvocationInspect: vi.fn(() => inspect), + }; +}); vi.mock("../../src/cli/utils/select-projects.js", () => ({ discoverWorkspacePackages: vi.fn(() => []), diff --git a/packages/react-doctor/tests/inspect-action-exit-code.test.ts b/packages/react-doctor/tests/inspect-action-exit-code.test.ts index 45c7464e62..e61a0fd2ef 100644 --- a/packages/react-doctor/tests/inspect-action-exit-code.test.ts +++ b/packages/react-doctor/tests/inspect-action-exit-code.test.ts @@ -42,12 +42,16 @@ vi.mock("@react-doctor/core", async (importOriginal) => { }; }); -vi.mock("../src/inspect.js", () => ({ - inspect: vi.fn(async (): Promise => { +vi.mock("../src/inspect.js", () => { + const inspect = vi.fn(async (): Promise => { if (mockState.result === undefined) throw new Error("mockState.result not set"); return mockState.result; - }), -})); + }); + return { + inspect, + createInvocationInspect: vi.fn(() => inspect), + }; +}); vi.mock("../src/cli/utils/select-projects.js", () => ({ selectProjects: vi.fn(async () => mockState.projectDirectories), diff --git a/packages/react-doctor/tests/inspect-action-setup-prompt.test.ts b/packages/react-doctor/tests/inspect-action-setup-prompt.test.ts index a335c3441d..8723a40d06 100644 --- a/packages/react-doctor/tests/inspect-action-setup-prompt.test.ts +++ b/packages/react-doctor/tests/inspect-action-setup-prompt.test.ts @@ -54,8 +54,8 @@ vi.mock("@react-doctor/core", async (importOriginal) => { }; }); -vi.mock("../src/inspect.js", () => ({ - inspect: vi.fn( +vi.mock("../src/inspect.js", () => { + const inspect = vi.fn( async (directory: string): Promise => ({ diagnostics: [], score: null, @@ -89,8 +89,12 @@ vi.mock("../src/inspect.js", () => ({ }, elapsedMilliseconds: 1, }), - ), -})); + ); + return { + inspect, + createInvocationInspect: vi.fn(() => inspect), + }; +}); vi.mock("../src/cli/utils/select-projects.js", () => ({ selectProjects: vi.fn(async () => mockState.projectDirectories), diff --git a/packages/react-doctor/tests/inspect-action-staged-guard.test.ts b/packages/react-doctor/tests/inspect-action-staged-guard.test.ts index 9bafed0513..83c3eaec61 100644 --- a/packages/react-doctor/tests/inspect-action-staged-guard.test.ts +++ b/packages/react-doctor/tests/inspect-action-staged-guard.test.ts @@ -16,8 +16,8 @@ vi.mock("../src/cli/utils/handle-error.js", () => ({ handleUserError: vi.fn(), })); -vi.mock("../src/inspect.js", () => ({ - inspect: vi.fn( +vi.mock("../src/inspect.js", () => { + const inspect = vi.fn( async (directory: string): Promise => ({ diagnostics: [], score: null, @@ -48,8 +48,12 @@ vi.mock("../src/inspect.js", () => ({ }, elapsedMilliseconds: 1, }), - ), -})); + ); + return { + inspect, + createInvocationInspect: vi.fn(() => inspect), + }; +}); const temporaryDirectories: string[] = []; diff --git a/packages/react-doctor/tests/inspect-action-staged-materialize.test.ts b/packages/react-doctor/tests/inspect-action-staged-materialize.test.ts index 5482a8fd72..d4223f6485 100644 --- a/packages/react-doctor/tests/inspect-action-staged-materialize.test.ts +++ b/packages/react-doctor/tests/inspect-action-staged-materialize.test.ts @@ -17,7 +17,13 @@ vi.mock("../src/cli/utils/handle-error.js", () => ({ handleUserError: vi.fn(), })); -vi.mock("../src/inspect.js", () => ({ inspect: vi.fn() })); +vi.mock("../src/inspect.js", () => { + const inspect = vi.fn(); + return { + inspect, + createInvocationInspect: vi.fn(() => inspect), + }; +}); vi.mock("../src/cli/utils/get-staged-files.js", () => ({ getStagedSourceFiles: vi.fn(),