test(npm): one bounded, diagnosable way to run npm, and quiet npm env on every test spawn (#1130)

docs/CI.md

@@ -205,6 +205,18 @@ stack: Node runs separate `t.after()` hooks in registration order, so an earlier
 directory-removal hook would run while those later resources are still open.
 Filesystem retries cannot repair that ordering.
 
+Tests and scripts start npm through `scripts/npm-command.ts`, never through
+`npm.cmd` and a shell: `npmCommand` runs npm's JavaScript entry under the
+current Node, and `runNpmSync` adds a bounded synchronous run with no stdin,
+no update check, funding or audit request, and a failure message that names
+the exit status, signal, whether the timeout fired, the elapsed time and the
+tail of both streams
+([#1130](https://github.com/jimhoyd-com/urlcode/issues/1130)). To repeat a
+suspected Windows npm hang, rerun the failing job, or run the test file in a
+loop on a Windows machine, for example
+`for /L %i in (1,1,50) do npm run test:shard -- test/prepare.test.ts`; each
+npm call logs its elapsed time as a test diagnostic.
+
 The store suite uses a two-minute test-file timeout.
 Keep large suites split into focused files so a file can finish within its budget
 on supported CI runners, with fixtures isolated between files. Platform-sensitive Windows coverage is Node 24

scripts/npm-command.ts

@@ -1,4 +1,6 @@
 import assert from 'node:assert/strict';
+import { spawnSync } from 'node:child_process';
+import type { SpawnSyncOptionsWithStringEncoding } from 'node:child_process';
 import { existsSync } from 'node:fs';
 import { dirname, join } from 'node:path';
 
@@ -17,3 +19,79 @@ export function npmCommand(args: string[], options: {
   assert((options.exists ?? existsSync)(adjacent), 'Cannot locate npm-cli.js. Run this helper through npm run so npm_execpath is available.');
   return { command: execPath, args: [adjacent, ...args] };
 }
+
+/**
+ * Settings forced on every npm a test or script runs synchronously (#1130): no update check, funding or audit request
+ * and no progress output, so nothing but the command itself is left for the child to finish before it exits.
+ */
+export const quietNpmEnv: Readonly<Record<string, string>> = {
+  npm_config_update_notifier: 'false',
+  npm_config_fund: 'false',
+  npm_config_audit: 'false',
+  npm_config_progress: 'false',
+};
+
+export interface NpmSyncOptions {
+  cwd: string;
+  /** The child's whole environment before the quiet settings are layered over it; defaults to this process's. */
+  env?: NodeJS.ProcessEnv;
+  timeoutMs: number;
+}
+
+/**
+ * The exact spawn of one synchronous npm run: npm's JavaScript entry under this Node (never npm.cmd through a shell,
+ * whose cmd.exe wrapper can outlive a timeout kill), no stdin to hold open or read, and no console window on Windows.
+ */
+export function npmSpawn(args: string[], options: NpmSyncOptions & Parameters<typeof npmCommand>[1]): {
+  command: string; args: string[]; options: SpawnSyncOptionsWithStringEncoding;
+} {
+  const { cwd, env = process.env, timeoutMs, ...locate } = options;
+  const command = npmCommand(args, locate);
+  return {
+    ...command,
+    options: {
+      cwd, encoding: 'utf8', timeout: timeoutMs, maxBuffer: 64 * 1024 * 1024, windowsHide: true,
+      stdio: ['ignore', 'pipe', 'pipe'], env: { ...env, ...quietNpmEnv },
+    },
+  };
+}
+
+export interface NpmRun {
+  command: string; args: string[]; cwd: string;
+  status: number | null; signal: NodeJS.Signals | null; timedOut: boolean;
+  elapsedMs: number; timeoutMs: number; stdout: string; stderr: string; error?: string;
+}
+
+/** Runs npm once, synchronously, and records what a failure needs to be diagnosed: signal, timeout and elapsed time. */
+export function runNpmSync(args: string[], options: NpmSyncOptions): NpmRun {
+  const spawn = npmSpawn(args, options);
+  const started = performance.now();
+  const result = spawnSync(spawn.command, spawn.args, spawn.options);
+  const elapsedMs = Math.round(performance.now() - started);
+  const code = (result.error as NodeJS.ErrnoException | undefined)?.code;
+  return {
+    command: spawn.command, args: spawn.args, cwd: options.cwd, status: result.status, signal: result.signal,
+    timedOut: code === 'ETIMEDOUT', elapsedMs, timeoutMs: options.timeoutMs,
+    stdout: result.stdout ?? '', stderr: result.stderr ?? '', ...(result.error ? { error: result.error.message } : {}),
+  };
+}
+
+const tail = (text: string, lines = 20): string => {
+  const kept = text.trimEnd().split('\n');
+  return kept.length > lines ? [`... [${kept.length - lines} earlier lines]`, ...kept.slice(-lines)].join('\n') : kept.join('\n');
+};
+
+/** One message naming the command, how it ended, how long it took against its limit, and the end of both streams. */
+export function describeNpmRun(run: NpmRun): string {
+  return [
+    `${JSON.stringify([run.command, ...run.args])} in ${run.cwd}`,
+    `exited with status ${run.status}, signal ${run.signal}, timedOut ${run.timedOut} after ${run.elapsedMs} ms (timeout ${run.timeoutMs} ms)`,
+    ...(run.error ? [`spawn error: ${run.error}`] : []),
+    `stderr (tail):\n${tail(run.stderr) || '(empty)'}`,
+    `stdout (tail):\n${tail(run.stdout) || '(empty)'}`,
+  ].join('\n');
+}
+
+export function assertNpmSucceeded(run: NpmRun): asserts run is NpmRun & { status: 0 } {
+  assert.equal(run.status, 0, describeNpmRun(run));
+}

scripts/workspaces.ts

@@ -10,6 +10,7 @@ import { readFile, readdir } from 'node:fs/promises';
 import { spawnSync } from 'node:child_process';
 import { join, resolve } from 'node:path';
 import { fileURLToPath, pathToFileURL } from 'node:url';
+import { npmCommand } from './npm-command.ts';
 
 export interface Addon { name: string; kind: 'extension' | 'artifact'; directory: string; packageName: string; version: string; description: string; requires: string[]; uses: string[]; testsWith: string[]; scripts: Record<string, string> }
 export const repositoryRoot = resolve(fileURLToPath(new URL('..', import.meta.url)));
@@ -44,10 +45,10 @@ if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href)
   const list = await addons();
   if (command === 'list') process.stdout.write(JSON.stringify(list.map(({ scripts: _scripts, ...addon }) => addon), null, 2) + '\n');
   else if (command === 'run' && script) {
-    const npm = process.platform === 'win32' ? 'npm.cmd' : 'npm';
     for (const addon of list) {
       if (!addon.scripts[script]) continue;
-      const result = spawnSync(npm, ['run', script, '--workspace', addon.packageName], { stdio: 'inherit', shell: process.platform === 'win32' });
+      const npm = npmCommand(['run', script, '--workspace', addon.packageName]);
+      const result = spawnSync(npm.command, npm.args, { stdio: 'inherit' });
       if (result.status !== 0) process.exit(result.status ?? 1);
     }
   } else { process.stderr.write('Use: node scripts/workspaces.ts list | run <script>\n'); process.exit(2); }

test/addons.integration.ts

@@ -12,14 +12,13 @@ import { pathToFileURL } from 'node:url';
 import type { TestContext } from 'node:test';
 import { addons, repositoryRoot } from '../scripts/workspaces.ts';
 import { loadDocument } from '../packages/core/src/config.ts';
+import { npmCommand } from '../scripts/npm-command.ts';
 import { packAddons, shippedManifest } from '../scripts/pack-addons.ts';
 import type { PackedAddons } from '../scripts/pack-addons.ts';
 
 const cli = join(repositoryRoot, 'dist', 'cli.js');
 /** npm's own CLI under this Node when a script started the test, so no shell is needed on Windows. */
-const npmRun = (args: string[], cwd: string) => process.env.npm_execpath
-  ? spawnSync(process.execPath, [process.env.npm_execpath, ...args], { cwd, encoding: 'utf8' })
-  : spawnSync(process.platform === 'win32' ? 'npm.cmd' : 'npm', args, { cwd, encoding: 'utf8', shell: process.platform === 'win32' });
+const npmRun = (args: string[], cwd: string) => { const command = npmCommand(args); return spawnSync(command.command, command.args, { cwd, encoding: 'utf8' }); };
 
 let packed: Promise<PackedAddons> | undefined;
 /** Packs once per run: core and every add-on, with an addons.json pinning the add-on tarballs by sha512. */
@@ -275,9 +274,7 @@ test('an auth site\'s generated validate script reviews locally with no context,
   assert.equal(added.status, 0, added.stderr);
   operatorCli(t, dir, 'urlcode-auth', ['migrate'], {});
   const { PROJECT_SHA256: _pin, URLCODE_ORIGIN: _origin, URLCODE_POLICY: _policy, ...ambient } = process.env;
-  const npmScript = (script: string, env: Record<string, string>) => process.env.npm_execpath
-    ? spawnSync(process.execPath, [process.env.npm_execpath, 'run', '--silent', script], { cwd: dir, encoding: 'utf8', timeout: 300000, env: { ...ambient, ...env } })
-    : spawnSync(process.platform === 'win32' ? 'npm.cmd' : 'npm', ['run', '--silent', script], { cwd: dir, encoding: 'utf8', timeout: 300000, env: { ...ambient, ...env }, shell: process.platform === 'win32' });
+  const npmScript = (script: string, env: Record<string, string>) => { const command = npmCommand(['run', '--silent', script]); return spawnSync(command.command, command.args, { cwd: dir, encoding: 'utf8', timeout: 300000, env: { ...ambient, ...env } }); };
   const refusal = (stderr: string) => JSON.parse(stderr.trim().split('\n').filter(line => line.startsWith('{')).at(-1)!) as { message: string; code: string; command: string };
 
   // No context at all: the generated validate script reviews the current revision locally (#932), with no pin or origin.

test/authjs-provider.integration.ts

@@ -13,21 +13,21 @@ import { tmpdir } from 'node:os';
 import { join } from 'node:path';
 import type { TestContext } from 'node:test';
 import { parse } from 'yaml';
+import { npmCommand } from '../scripts/npm-command.ts';
 import { packAddons } from '../scripts/pack-addons.ts';
 import { repositoryRoot } from '../scripts/workspaces.ts';
 
 const proofs = join(repositoryRoot, 'proofs');
 const proof = join(proofs, 'private-requests-authjs'), betterAuthProof = join(proofs, 'private-requests'), provider = join(proofs, 'authjs-provider');
-const npmCli = process.env.npm_execpath;
-const npmCommand = npmCli ? process.execPath : process.platform === 'win32' ? 'npm.cmd' : 'npm';
 
 interface Run { status: number | null; stdout: string; stderr: string }
 function run(t: TestContext, cwd: string, command: string, args: string[], env: Record<string, string> = {}): Run {
-  const result = spawnSync(command, args, { cwd, encoding: 'utf8', timeout: 600000, env: { ...process.env, ...env }, shell: command === 'npm.cmd' });
+  const result = spawnSync(command, args, { cwd, encoding: 'utf8', timeout: 600000, env: { ...process.env, ...env } });
   t.diagnostic(`${command === process.execPath ? 'node' : command} ${args.join(' ').slice(0, 160)} -> ${result.status}`);
   return { status: result.status, stdout: result.stdout, stderr: result.stderr };
 }
-const npm = (t: TestContext, cwd: string, args: string[], env: Record<string, string> = {}): Run => run(t, cwd, npmCommand, npmCli ? [npmCli, ...args] : args, env);
+// npm's own CLI under this Node (scripts/npm-command.ts): no shell, so Windows needs no npm.cmd quoting.
+const npm = (t: TestContext, cwd: string, args: string[], env: Record<string, string> = {}): Run => { const command = npmCommand(args); return run(t, cwd, command.command, command.args, env); };
 const urlcode = (t: TestContext, site: string, args: string[], env: Record<string, string> = {}): Run => run(t, site, process.execPath, [join(site, 'node_modules', '@jimhoyd', 'urlcode', 'dist', 'cli.js'), ...args], env);
 const freePort = (): Promise<number> => new Promise((resolve, reject) => {
   const server = createServer().listen(0, '127.0.0.1', () => { const { port } = server.address() as { port: number }; server.close(() => resolve(port)); }).on('error', reject);

test/docs-search-package.test.ts

@@ -7,6 +7,7 @@ import { tmpdir } from 'node:os';
 import { join, resolve } from 'node:path';
 import { fileURLToPath } from 'node:url';
 import { parsePackJson } from '../scripts/pack-json.ts';
+import { assertNpmSucceeded, runNpmSync } from '../scripts/npm-command.ts';
 
 // agent-context.ts's searchDocs (the urlcode docs search CLI and the MCP
 // search_docs tool) reads docs/*.md from the package root at runtime, but
@@ -21,29 +22,20 @@ interface PackReport { name: string; filename: string; version: string }
 const root = resolve(fileURLToPath(new URL('../', import.meta.url)));
 
 test('urlcode docs search works against the published npm tarball', async t => {
-  const npm = process.env.npm_execpath;
-  assert(npm, 'Run this test through npm');
+  assert(process.env.npm_execpath, 'Run this test through npm');
   const cache = await mkdtemp(join(tmpdir(), 'urlcode-docs-search-'));
   t.after(async () => { await rm(cache, { recursive: true, force: true }); });
 
-  const pack = spawnSync(process.execPath, [npm, 'pack', '--ignore-scripts', '--json', '--pack-destination', cache], {
-    cwd: root,
-    encoding: 'utf8',
-    env: { ...process.env, npm_config_cache: cache },
-    timeout: 120_000,
-  });
-  assert.equal(pack.status, 0, pack.stderr || pack.stdout || pack.error?.message || 'npm pack failed');
+  const pack = runNpmSync(['pack', '--ignore-scripts', '--json', '--pack-destination', cache], { cwd: root, env: { ...process.env, npm_config_cache: cache }, timeoutMs: 120_000 });
+  assertNpmSucceeded(pack);
   const [report] = parsePackJson<PackReport>(pack.stdout, pack.stderr);
   assert(report, 'npm pack reported no package');
 
   const install = join(cache, 'install');
   await mkdir(install);
-  const installResult = spawnSync(process.execPath, [npm, 'install', '--omit=dev', '--omit=optional', '--ignore-scripts', '--no-audit', '--no-fund', '--prefix', install, join(cache, report.filename)], {
-    encoding: 'utf8',
-    env: { ...process.env, npm_config_cache: cache },
-    timeout: 120_000,
-  });
-  assert.equal(installResult.status, 0, installResult.stderr || installResult.stdout || installResult.error?.message || 'npm install failed');
+  assertNpmSucceeded(runNpmSync(['install', '--omit=dev', '--omit=optional', '--ignore-scripts', '--no-audit', '--no-fund', '--prefix', install, join(cache, report.filename)], {
+    cwd: process.cwd(), env: { ...process.env, npm_config_cache: cache }, timeoutMs: 120_000,
+  }));
 
   const cli = join(install, 'node_modules', ...report.name.split('/'), 'dist', 'cli.js');
   assert.ok(existsSync(cli), 'Installed package did not ship the built CLI');

test/npm-command.test.ts

@@ -1,7 +1,8 @@
 import test from 'node:test';
 import assert from 'node:assert/strict';
 import { join } from 'node:path';
-import { npmCommand } from '../scripts/npm-command.ts';
+import { assertNpmSucceeded, describeNpmRun, npmCommand, npmSpawn, quietNpmEnv, runNpmSync } from '../scripts/npm-command.ts';
+import type { NpmRun } from '../scripts/npm-command.ts';
 
 test('npm helper preserves arguments with spaces without invoking a shell', () => {
   assert.deepEqual(npmCommand(['install', '--cache=C:/a b/cache'], { platform: 'win32', execPath: 'C:/Node/node.exe', npmExecPath: 'C:/Node/npm-cli.js' }), {
@@ -15,3 +16,48 @@ test('direct Windows invocation locates npm JavaScript instead of executing npm.
   assert.deepEqual(result.args, [join('/node', 'node_modules', 'npm', 'bin', 'npm-cli.js'), 'ci']);
   assert.throws(() => npmCommand(['ci'], { platform: 'win32', npmExecPath: '', exists: () => false }), /Run this helper through npm run/);
 });
+
+test('a synchronous npm run gets no stdin, no shell, no window, and quiet settings layered over the given env (#1130)', () => {
+  const spawn = npmSpawn(['pack', '--json'], {
+    cwd: 'C:/fixture', timeoutMs: 30_000, platform: 'win32', execPath: 'C:/Node/node.exe', npmExecPath: 'C:/Node/npm-cli.js',
+    env: { PATH: 'C:/bin', npm_config_cache: 'C:/cache', npm_config_update_notifier: 'true' },
+  });
+  assert.equal(spawn.command, 'C:/Node/node.exe');
+  assert.deepEqual(spawn.args, ['C:/Node/npm-cli.js', 'pack', '--json']);
+  assert.deepEqual(spawn.options.stdio, ['ignore', 'pipe', 'pipe']);
+  assert.equal(spawn.options.shell, undefined);
+  assert.equal(spawn.options.windowsHide, true);
+  assert.equal(spawn.options.timeout, 30_000);
+  assert.equal(spawn.options.cwd, 'C:/fixture');
+  assert.deepEqual(spawn.options.env, {
+    PATH: 'C:/bin', npm_config_cache: 'C:/cache',
+    npm_config_update_notifier: 'false', npm_config_fund: 'false', npm_config_audit: 'false', npm_config_progress: 'false',
+  });
+  assert.deepEqual(quietNpmEnv, { npm_config_update_notifier: 'false', npm_config_fund: 'false', npm_config_audit: 'false', npm_config_progress: 'false' });
+});
+
+test('a timed-out npm run names its signal, timeout, elapsed time and the tail of both streams (#1130)', () => {
+  const stdout = Array.from({ length: 30 }, (_, index) => `out ${index}`).join('\n');
+  const run: NpmRun = {
+    command: 'node', args: ['npm-cli.js', 'pack', '--json'], cwd: '/fixture', status: null, signal: 'SIGTERM', timedOut: true,
+    elapsedMs: 30_037, timeoutMs: 30_000, stdout, stderr: '', error: 'spawnSync node ETIMEDOUT',
+  };
+  const message = describeNpmRun(run);
+  assert.match(message, /\["node","npm-cli\.js","pack","--json"\] in \/fixture/);
+  assert.match(message, /status null, signal SIGTERM, timedOut true after 30037 ms \(timeout 30000 ms\)/);
+  assert.match(message, /spawn error: spawnSync node ETIMEDOUT/);
+  assert.match(message, /stderr \(tail\):\n\(empty\)/);
+  assert.match(message, /\.\.\. \[10 earlier lines\]\nout 10\n/);
+  assert.match(message, /out 29$/);
+  assert.doesNotMatch(message, /out 9\n/);
+  assert.throws(() => assertNpmSucceeded(run), (error: Error) => error.message.includes('timedOut true after 30037 ms'));
+});
+
+test('runNpmSync records status, signal and elapsed time for a real npm run', { skip: !process.env.npm_execpath && 'needs npm_execpath' }, () => {
+  const run = runNpmSync(['--version'], { cwd: process.cwd(), timeoutMs: 60_000 });
+  assertNpmSucceeded(run);
+  assert.equal(run.timedOut, false);
+  assert.equal(run.signal, null);
+  assert.match(run.stdout.trim(), /^\d+\.\d+\.\d+/);
+  assert.ok(run.elapsedMs >= 0);
+});

test/prepare.test.ts

@@ -1,13 +1,12 @@
 import assert from 'node:assert/strict';
 import test from 'node:test';
-import { spawnSync } from 'node:child_process';
 import { copyFile, mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises';
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';
+import { assertNpmSucceeded, runNpmSync } from '../scripts/npm-command.ts';
 
 test('packing with ignored scripts preserves built output, while prepare still builds a checkout', async t => {
-  const npm = process.env.npm_execpath;
-  assert(npm, 'Run this test through npm');
+  assert(process.env.npm_execpath, 'Run this test through npm');
   const root = await mkdtemp(join(tmpdir(), 'urlcode-prepare-'));
   t.after(() => rm(root, { recursive: true, force: true }));
   await mkdir(join(root, 'scripts'));
@@ -18,9 +17,11 @@ test('packing with ignored scripts preserves built output, while prepare still b
   await writeFile(join(root, 'scripts', 'build.ts'), "import { writeFile } from 'node:fs/promises'; await writeFile('dist/addons.json', 'rebuilt');\n");
   const manifest = join(root, 'dist', 'addons.json');
   await writeFile(manifest, 'existing build');
+  // A timeout reports signal, elapsed time and both stream tails (#1130), and every run's time is logged as evidence.
   const run = (...args: string[]) => {
-    const result = spawnSync(process.execPath, [npm, ...args], { cwd: root, encoding: 'utf8', timeout: 30_000, env: { ...process.env, npm_config_cache: join(root, 'cache') } });
-    assert.equal(result.status, 0, result.stderr || result.stdout || result.error?.message || 'npm failed');
+    const result = runNpmSync(args, { cwd: root, timeoutMs: 30_000, env: { ...process.env, npm_config_cache: join(root, 'cache') } });
+    t.diagnostic(`npm ${args.join(' ')}: status ${result.status} after ${result.elapsedMs} ms`);
+    assertNpmSucceeded(result);
   };
   // npm 10 invokes prepare even here; newer npm versions skip the lifecycle.
   run('pack', '--ignore-scripts', '--json');