From cc388282347d5dac725a9f5dfcbdbe9fe9e0a1a1 Mon Sep 17 00:00:00 2001 From: RoboClaw Date: Thu, 1 Oct 2026 06:23:57 -0700 Subject: [PATCH] fix(tooling): bound formatter arguments for large changed checks (#162703) Reuse the docs formatter batching owner so changed-file checks retain every selected path without overflowing the pnpm shell command. Preserve formatting flags and serial failure propagation. Co-authored-by: steipete <58493+steipete@users.noreply.github.com> --- scripts/check-changed.mts | 13 ++---- scripts/format-docs.mts | 40 +++---------------- scripts/lib/format-command-batches.mts | 33 +++++++++++++++ .../scripts/changed-lanes.format-argv.test.ts | 27 +++++++++++++ 4 files changed, 70 insertions(+), 43 deletions(-) create mode 100644 scripts/lib/format-command-batches.mts create mode 100644 test/scripts/changed-lanes.format-argv.test.ts diff --git a/scripts/check-changed.mts b/scripts/check-changed.mts index 1b16b8e6dd26..17888ce2c358 100644 --- a/scripts/check-changed.mts +++ b/scripts/check-changed.mts @@ -36,6 +36,7 @@ import { getChangedPathFacts, normalizeChangedPath } from "./lib/changed-path-fa import { printTimingSummary } from "./lib/check-timing-summary.mts"; import { isDirectRunUrl } from "./lib/direct-run.mjs"; import { runWithFailedTrailer } from "./lib/failed-trailer.mts"; +import { chunkFormatFilesForCommand } from "./lib/format-command-batches.mts"; import { resolveLocalCheckEnv } from "./lib/local-check-runtime.mts"; import { runManagedCommand } from "./lib/managed-child-process.mts"; import { readNativeTypeScriptConfig } from "./lib/native-typescript-config.mts"; @@ -785,15 +786,9 @@ export function createChangedCheckPlan( add("duplicate scan target coverage", ["dup:check:coverage"]); broadAudits.add(add("coercion helper declaration guard", ["check:coercion-helpers"])); add("dependency pin guard", ["deps:pins:check"]); - if (result.paths.length > 0) { - lintChecks.add( - add("format changed files", [ - "format:check", - "--no-error-on-unmatched-pattern", - "--", - ...result.paths, - ]), - ); + const formatPrefix = ["format:check", "--no-error-on-unmatched-pattern", "--"]; + for (const files of chunkFormatFilesForCommand(result.paths, formatPrefix)) { + lintChecks.add(add("format changed files", [...formatPrefix, ...files])); } const npmLockGuardCommand = createNpmLockGuardCommand(result.paths); if (npmLockGuardCommand) { diff --git a/scripts/format-docs.mts b/scripts/format-docs.mts index 212dbc3b342b..09b8c0ea7f1b 100644 --- a/scripts/format-docs.mts +++ b/scripts/format-docs.mts @@ -6,6 +6,10 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { pathToFileURL } from "node:url"; +import { + chunkFormatFilesForCommand, + FORMAT_MAX_COMMAND_LINE_BYTES, +} from "./lib/format-command-batches.mts"; import { resolveRepoToolBinPath } from "./lib/local-check-runtime.mts"; import { outputTail, spawnOutputText } from "./lib/output-tail.mts"; import { resolveRepoRoot } from "./lib/repo-root.mjs"; @@ -13,7 +17,6 @@ import { buildCmdExeCommandLine, resolveWindowsCmdExePath } from "./windows-cmd- const ROOT = resolveRepoRoot(import.meta.url); const CHECK = process.argv.includes("--check"); const DOCS_FORMAT_MAX_BUFFER_BYTES = 1024 * 1024 * 16; -const DOCS_FORMAT_MAX_COMMAND_LINE_BYTES = 24 * 1024; const FAILURE_OUTPUT_TAIL_BYTES = 16 * 1024; type CommandResult = { @@ -91,37 +94,6 @@ export function docsFiles(root = ROOT, deps: FormatDeps = {}) { .filter((relativePath) => (deps.existsSync ?? fs.existsSync)(path.join(root, relativePath))); } -function commandLineBytes(args: string[]) { - return args.reduce((total, arg) => total + Buffer.byteLength(arg, "utf8") + 3, 0); -} - -function chunkFilesForCommand( - files: string[], - prefixArgs: string[], - maxBytes = DOCS_FORMAT_MAX_COMMAND_LINE_BYTES, -) { - const chunks: string[][] = []; - let chunk: string[] = []; - let chunkBytes = commandLineBytes(prefixArgs); - - for (const file of files) { - const fileBytes = Buffer.byteLength(file, "utf8") + 3; - if (chunk.length > 0 && chunkBytes + fileBytes > maxBytes) { - chunks.push(chunk); - chunk = []; - chunkBytes = commandLineBytes(prefixArgs); - } - chunk.push(file); - chunkBytes += fileBytes; - } - - if (chunk.length > 0) { - chunks.push(chunk); - } - - return chunks; -} - export function resolveOxfmtInvocation(args: string[], params: OxfmtParams = {}) { const repoRoot = params.repoRoot ?? ROOT; const platform = params.platform ?? process.platform; @@ -160,10 +132,10 @@ export function runOxfmt(files: string[], params: OxfmtParams = {}, deps: Format const repoRoot = params.repoRoot ?? ROOT; const spawnSyncImpl = deps.spawnSync ?? spawnSync; const prefixArgs = ["--write", "--threads=1", "--config", path.join(repoRoot, ".oxfmtrc.jsonc")]; - for (const chunk of chunkFilesForCommand( + for (const chunk of chunkFormatFilesForCommand( files, prefixArgs, - params.maxCommandLineBytes ?? DOCS_FORMAT_MAX_COMMAND_LINE_BYTES, + params.maxCommandLineBytes ?? FORMAT_MAX_COMMAND_LINE_BYTES, )) { const invocation = resolveOxfmtInvocation([...prefixArgs, ...chunk], { comSpec: params.comSpec, diff --git a/scripts/lib/format-command-batches.mts b/scripts/lib/format-command-batches.mts new file mode 100644 index 000000000000..517d85bbf5c9 --- /dev/null +++ b/scripts/lib/format-command-batches.mts @@ -0,0 +1,33 @@ +// Keep formatter arguments bounded even when pnpm joins them into one shell command. +export const FORMAT_MAX_COMMAND_LINE_BYTES = 24 * 1024; + +function commandLineBytes(args: string[]) { + return args.reduce((total, arg) => total + Buffer.byteLength(arg, "utf8") + 3, 0); +} + +export function chunkFormatFilesForCommand( + files: string[], + prefixArgs: string[], + maxBytes = FORMAT_MAX_COMMAND_LINE_BYTES, +) { + const chunks: string[][] = []; + let chunk: string[] = []; + let chunkBytes = commandLineBytes(prefixArgs); + + for (const file of files) { + const fileBytes = Buffer.byteLength(file, "utf8") + 3; + if (chunk.length > 0 && chunkBytes + fileBytes > maxBytes) { + chunks.push(chunk); + chunk = []; + chunkBytes = commandLineBytes(prefixArgs); + } + chunk.push(file); + chunkBytes += fileBytes; + } + + if (chunk.length > 0) { + chunks.push(chunk); + } + + return chunks; +} diff --git a/test/scripts/changed-lanes.format-argv.test.ts b/test/scripts/changed-lanes.format-argv.test.ts new file mode 100644 index 000000000000..5c76dcf3d67a --- /dev/null +++ b/test/scripts/changed-lanes.format-argv.test.ts @@ -0,0 +1,27 @@ +import { expect, it } from "vitest"; +import { detectChangedLanes } from "../../scripts/changed-lanes.mts"; +import { createChangedCheckPlan } from "../../scripts/check-changed.mts"; + +it.each([false, true])( + "keeps every formatter path within the argv budget (lintOnly=%s)", + (lintOnly) => { + const paths = Array.from( + { length: 1_600 }, + (_, index) => `docs/${"long目录_".repeat(8)}file-${index} name's.md`, + ).toSorted((left, right) => left.localeCompare(right)); + const commands = createChangedCheckPlan(detectChangedLanes(paths), { + lintOnly, + }).commands.filter((command) => command.args[0] === "format:check"); + + expect(commands.length).toBeGreaterThan(1); + expect(commands.flatMap((command) => command.args.slice(3))).toEqual(paths); + for (const command of commands) { + expect(command.args.slice(0, 3)).toEqual([ + "format:check", + "--no-error-on-unmatched-pattern", + "--", + ]); + expect(Buffer.byteLength(command.args.join(" "), "utf8")).toBeLessThanOrEqual(24 * 1024); + } + }, +);