From e6f22409589e4d6c154ad7609fd11dd2cd36bba5 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Thu, 29 Oct 2026 00:58:08 -0400 Subject: [PATCH] fix(scripts): read gh replies over 1 MiB in release:notes and the sweeps run() in scripts/release-notes.mjs called spawnSync with Node's default 1 MiB maxBuffer, so the src/ tree query at v2.2.0's merge commit (1,087,532 bytes) aborted the run with ENOBUFS. Pass a 256 MiB maxBuffer. scripts/lib/sweep.mjs's gh() gets the same: the Dependabot alert listing is about 7 KB per alert, so ~150 open alerts would overflow it. Tests drive the real spawnSync with a >1 MiB reply, and check a genuine spawn error is still rethrown. Closes #5339 Co-Authored-By: Claude Opus 5.5 Signed-off-by: cliffhall --- scripts/lib/sweep.mjs | 9 +++++++++ scripts/lib/sweep.test.mjs | 30 ++++++++++++++++++++++++++++++ scripts/release-notes.mjs | 13 ++++++++++++- scripts/release-notes.test.mjs | 28 ++++++++++++++++++++++++++++ 4 files changed, 79 insertions(+), 1 deletion(-) diff --git a/scripts/lib/sweep.mjs b/scripts/lib/sweep.mjs index 5daea0a0aa..9e380110a7 100644 --- a/scripts/lib/sweep.mjs +++ b/scripts/lib/sweep.mjs @@ -48,6 +48,14 @@ export const AUTOMATION_LOGIN = "github-actions"; /** Labels on every sweep issue. `chore` and `dependencies` need write access. */ export const SWEEP_LABELS = ["v2", "chore", "dependencies"]; +/** + * The most stdout `gh()` reads from one call. Node's default is 1 MiB, and + * past it `spawnSync` throws ENOBUFS. The alert listing is the reply that can + * cross it: each Dependabot alert is about 7 KB of JSON, so some 150 open + * alerts overflow the default and abort the sweep (#5339). + */ +export const GH_MAX_BUFFER = 256 * 1024 * 1024; + /** The two labels the trust rule reads; `v2` is applied by an issue form. */ const TRUST_LABELS = ["chore", "dependencies"]; @@ -223,6 +231,7 @@ export function isDryRun(argv = process.argv.slice(2)) { export function gh(spawn, args, input) { const result = spawn("gh", args, { encoding: "utf8", + maxBuffer: GH_MAX_BUFFER, ...(input === undefined ? {} : { input }), }); if (result.error) throw result.error; diff --git a/scripts/lib/sweep.test.mjs b/scripts/lib/sweep.test.mjs index 949906f78b..0e8d7f60bb 100644 --- a/scripts/lib/sweep.test.mjs +++ b/scripts/lib/sweep.test.mjs @@ -5,7 +5,9 @@ import { test } from "node:test"; import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; import { + GH_MAX_BUFFER, SWEEP_LABELS, cell, currentMilestone, @@ -176,6 +178,34 @@ test("gh throws on a spawn error, and passes stdin only when given", () => { assert.equal(calls[1].input, "body"); }); +// The real spawnSync, with "gh" swapped for a node that writes `bytes` of +// stdout, so the options gh() passes are the ones that decide the outcome. +const bigReply = (bytes) => (_cmd, _args, opts) => + spawnSync( + process.execPath, + ["-e", `process.stdout.write("x".repeat(${bytes}))`], + opts, + ); + +test("gh reads a reply over Node's 1 MiB default without ENOBUFS (#5339)", () => { + const bytes = 2 * 1024 * 1024; + assert.ok(GH_MAX_BUFFER > bytes); + const result = gh(bigReply(bytes), ["api", "x"]); + assert.equal(result.status, 0); + assert.equal(result.stdout.length, bytes); +}); + +test("gh still rethrows a genuine spawn error from the real spawnSync", () => { + assert.throws( + () => + gh( + (_cmd, args, opts) => spawnSync("/nonexistent/gh-5339", args, opts), + ["api", "x"], + ), + (err) => err.code === "ENOENT", + ); +}); + test("currentMilestone reads open milestones and fails loudly", () => { const { spawn, calls } = fakeSpawn(() => ({ status: 0, diff --git a/scripts/release-notes.mjs b/scripts/release-notes.mjs index fe5eb39362..84e0780328 100644 --- a/scripts/release-notes.mjs +++ b/scripts/release-notes.mjs @@ -68,6 +68,13 @@ import { parseArgs } from "node:util"; import { readProject } from "./prepare-python-release.mjs"; export const REPO = "modelcontextprotocol/servers"; + +// The most stdout `run()` reads from one call. Node's default is 1 MiB, and +// past it `spawnSync` throws ENOBUFS before any notes are produced: the tree +// under `src/` at v2.2.0's merge commit came back at 1,087,532 bytes (#5339), +// and it only grows. 256 MiB is far past any reply this script reads while +// still bounding a runaway one. +export const MAX_BUFFER = 256 * 1024 * 1024; const [OWNER, NAME] = REPO.split("/"); // What `collaborators/{user}/permission` reports in its `permission` field. @@ -183,7 +190,11 @@ export function parseNotesArgs(argv) { * @param {string} [input] */ function run(spawn, cmd, args, input) { - const result = spawn(cmd, args, { encoding: "utf8", input }); + const result = spawn(cmd, args, { + encoding: "utf8", + input, + maxBuffer: MAX_BUFFER, + }); if (result.error) { throw result.error; } diff --git a/scripts/release-notes.test.mjs b/scripts/release-notes.test.mjs index e4f8ed3497..c3333bba88 100644 --- a/scripts/release-notes.test.mjs +++ b/scripts/release-notes.test.mjs @@ -10,9 +10,11 @@ // `npm run test:scripts`. import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; import { test } from "node:test"; import { ADVISORY_LEAD_IN, + MAX_BUFFER, MIXED_LEAD_IN, REPO, THANKS_LEAD_IN, @@ -20,6 +22,7 @@ import { bucket, closingKeywordIssues, dirsTouched, + filesOf, formatKnownIssues, formatPackages, formatThanks, @@ -282,6 +285,31 @@ test("bucket: one, several and no servers; unpublished and empty fall to Reposit ); }); +test("a gh reply over Node's 1 MiB default gets through without ENOBUFS (#5339)", () => { + // The real spawnSync, with "gh" swapped for a node that prints 2 MiB of + // file names, so the options run() passes are what decide the outcome. + const lines = 128 * 1024; + const spawn = (_cmd, _args, opts) => + spawnSync( + process.execPath, + ["-e", `process.stdout.write("src/everything/f.ts\\n".repeat(${lines}))`], + opts, + ); + assert.ok(MAX_BUFFER > lines * 20); + const files = filesOf(spawn, 1); + assert.equal(files.length, lines); + assert.equal(files[0], "src/everything/f.ts"); +}); + +test("a genuine spawn error is still rethrown", () => { + const spawn = (_cmd, args, opts) => + spawnSync("/nonexistent/gh-5339", args, opts); + assert.throws( + () => filesOf(spawn, 1), + (err) => err.code === "ENOENT", + ); +}); + test("closingKeywordIssues reads every closing keyword, never a cross-repo ref", () => { const body = [ "Closes #1",