From e345023db808da81c3df4627c7943e6733acf6b1 Mon Sep 17 00:00:00 2001 From: Charan Jagwani Date: Sat, 12 Sep 2026 22:07:43 -0700 Subject: [PATCH 1/5] perf(test): parallelize collaborator permission cases --- .../e2e-collaborator-permission-retry.test.ts | 96 ++++++++++++------- 1 file changed, 63 insertions(+), 33 deletions(-) diff --git a/test/e2e/support/e2e-collaborator-permission-retry.test.ts b/test/e2e/support/e2e-collaborator-permission-retry.test.ts index 726ef275e12..63e16e718dd 100644 --- a/test/e2e/support/e2e-collaborator-permission-retry.test.ts +++ b/test/e2e/support/e2e-collaborator-permission-retry.test.ts @@ -1,12 +1,13 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { spawnSync } from "node:child_process"; +import assert from "node:assert/strict"; +import { execFile } from "node:child_process"; import { chmodSync, existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { describe, expect, it } from "vitest"; +import { describe, it, vi } from "vitest"; import { readWorkflow } from "../../helpers/e2e-workflow-contract"; @@ -32,6 +33,29 @@ const AUTHORIZATION_STEPS: AuthorizationStep[] = [ }, ]; +type RunProcessResult = { + status: number | null; + signal: NodeJS.Signals | null; + stdout: string; + stderr: string; +}; + +function runProcess(file: string, args: readonly string[], env: NodeJS.ProcessEnv) { + return new Promise((resolve) => { + execFile(file, [...args], { encoding: "utf8", env }, (error, stdout, stderr) => { + const signal = error?.signal ?? null; + resolve({ + status: signal ? null : Number(error?.code) || (error ? -1 : 0), + signal, + stdout, + stderr, + }); + }); + }); +} + +vi.setConfig({ maxConcurrency: 8 }); + function authorizationScript(stepName: string): string { const workflow = readWorkflow() as { jobs: Record }>; @@ -39,7 +63,7 @@ function authorizationScript(stepName: string): string { const step = workflow.jobs["generate-matrix"]!.steps!.find( (candidate) => candidate.name === stepName, ); - expect(step?.run).toEqual(expect.any(String)); + assert.equal(typeof step?.run, "string", `${stepName} script is missing`); return step!.run!; } @@ -48,7 +72,7 @@ function lines(path: string): string[] { return value === "" ? [] : value.split("\n"); } -function runAuthorization( +async function runAuthorization( stepName: string, scenario: PermissionScenario, options: { actor?: string; status?: string } = {}, @@ -113,9 +137,10 @@ printf '%s\n' "$1" >>"$SLEEP_LOG" chmodSync(sleepPath, 0o755); const workflowSha = "c".repeat(40); - const result = spawnSync("bash", ["--noprofile", "--norc", "-c", authorizationScript(stepName)], { - encoding: "utf8", - env: { + const result = await runProcess( + "bash", + ["--noprofile", "--norc", "-c", authorizationScript(stepName)], + { ...process.env, ACTOR: options.actor ?? "dispatch-admin", ALLOW_JETSON_DISPATCH: "false", @@ -143,7 +168,7 @@ printf '%s\n' "$1" >>"$SLEEP_LOG" WORKFLOW_REF: "refs/heads/main", WORKFLOW_SHA: workflowSha, }, - }); + ); const permissionAttempts = existsSync(attemptFile) ? Number.parseInt(readFileSync(attemptFile, "utf8"), 10) : 0; @@ -153,13 +178,13 @@ printf '%s\n' "$1" >>"$SLEEP_LOG" return { ...result, curlOperations, permissionAttempts, sleeps }; } -describe.each(AUTHORIZATION_STEPS)( +describe.concurrent.each(AUTHORIZATION_STEPS)( "$name collaborator permission read", ({ deniedMessage, mismatchMessage, name }) => { - it.each(["408", "429", "503"])( + it.for(["408", "429", "503"])( "retries HTTP %s once before authorization succeeds (#9337)", - (status) => { - const result = runAuthorization(name, "transient-then-success", { status }); + async (status, { expect }) => { + const result = await runAuthorization(name, "transient-then-success", { status }); expect(result.status, result.stderr).toBe(0); expect(result.permissionAttempts).toBe(2); @@ -175,8 +200,8 @@ describe.each(AUTHORIZATION_STEPS)( }, ); - it("stops after three transient transport failures (#9337)", () => { - const result = runAuthorization(name, "transport-exhaustion"); + it("stops after three transient transport failures (#9337)", async ({ expect }) => { + const result = await runAuthorization(name, "transport-exhaustion"); expect(result.status).not.toBe(0); expect(result.permissionAttempts).toBe(3); @@ -187,21 +212,24 @@ describe.each(AUTHORIZATION_STEPS)( ); }); - it.each(["401", "403", "404", "422"])("does not retry HTTP %s (#9337)", (status) => { - const result = runAuthorization(name, "terminal-http", { status }); + it.for(["401", "403", "404", "422"])( + "does not retry HTTP %s (#9337)", + async (status, { expect }) => { + const result = await runAuthorization(name, "terminal-http", { status }); - expect(result.status).not.toBe(0); - expect(result.permissionAttempts).toBe(1); - expect(result.sleeps).toEqual([]); - expect(result.curlOperations).toEqual(["permission"]); - expect(result.stderr).toContain( - `Collaborator permission read attempt 1/3 failed: HTTP ${status}`, - ); - expect(result.stderr).not.toContain("private-response-body"); - }); + expect(result.status).not.toBe(0); + expect(result.permissionAttempts).toBe(1); + expect(result.sleeps).toEqual([]); + expect(result.curlOperations).toEqual(["permission"]); + expect(result.stderr).toContain( + `Collaborator permission read attempt 1/3 failed: HTTP ${status}`, + ); + expect(result.stderr).not.toContain("private-response-body"); + }, + ); - it("does not retry a malformed HTTP 200 response (#9337)", () => { - const result = runAuthorization(name, "malformed-success"); + it("does not retry a malformed HTTP 200 response (#9337)", async ({ expect }) => { + const result = await runAuthorization(name, "malformed-success"); expect(result.status).not.toBe(0); expect(result.permissionAttempts).toBe(1); @@ -213,8 +241,8 @@ describe.each(AUTHORIZATION_STEPS)( expect(result.stderr).not.toContain("private-response-body"); }); - it("does not retry a valid response with an unauthorized role (#9337)", () => { - const result = runAuthorization(name, "denied"); + it("does not retry a valid response with an unauthorized role (#9337)", async ({ expect }) => { + const result = await runAuthorization(name, "denied"); expect(result.status).not.toBe(0); expect(result.permissionAttempts).toBe(1); @@ -223,8 +251,8 @@ describe.each(AUTHORIZATION_STEPS)( expect(result.stderr).toContain(deniedMessage); }); - it("does not retry a permission response for a different actor (#9337)", () => { - const result = runAuthorization(name, "mismatched-actor"); + it("does not retry a permission response for a different actor (#9337)", async ({ expect }) => { + const result = await runAuthorization(name, "mismatched-actor"); expect(result.status).not.toBe(0); expect(result.permissionAttempts).toBe(1); @@ -233,8 +261,10 @@ describe.each(AUTHORIZATION_STEPS)( expect(result.stderr).toContain(mismatchMessage); }); - it("rejects an invalid actor before the permission read (#9337)", () => { - const result = runAuthorization(name, "transient-then-success", { actor: "invalid actor" }); + it("rejects an invalid actor before the permission read (#9337)", async ({ expect }) => { + const result = await runAuthorization(name, "transient-then-success", { + actor: "invalid actor", + }); expect(result.status).not.toBe(0); expect(result.permissionAttempts).toBe(0); From 721906228d1711a771548ee5d791704b7d4be955 Mon Sep 17 00:00:00 2001 From: Charan Jagwani Date: Sat, 12 Sep 2026 22:55:12 -0700 Subject: [PATCH 2/5] test(e2e): bound collaborator permission processes Signed-off-by: Charan Jagwani --- .../e2e-collaborator-permission-retry.test.ts | 237 +++++++++++++----- 1 file changed, 169 insertions(+), 68 deletions(-) diff --git a/test/e2e/support/e2e-collaborator-permission-retry.test.ts b/test/e2e/support/e2e-collaborator-permission-retry.test.ts index 63e16e718dd..e25706f38be 100644 --- a/test/e2e/support/e2e-collaborator-permission-retry.test.ts +++ b/test/e2e/support/e2e-collaborator-permission-retry.test.ts @@ -2,14 +2,15 @@ // SPDX-License-Identifier: Apache-2.0 import assert from "node:assert/strict"; -import { execFile } from "node:child_process"; +import { spawn } from "node:child_process"; import { chmodSync, existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { describe, it, vi } from "vitest"; +import { describe, it, type TestContext, vi } from "vitest"; import { readWorkflow } from "../../helpers/e2e-workflow-contract"; +import { superviseChild } from "../../helpers/process-supervisor"; type AuthorizationStep = { deniedMessage: string; @@ -18,6 +19,7 @@ type AuthorizationStep = { }; type PermissionScenario = + | "blocking" | "denied" | "malformed-success" | "mismatched-actor" @@ -40,18 +42,44 @@ type RunProcessResult = { stderr: string; }; -function runProcess(file: string, args: readonly string[], env: NodeJS.ProcessEnv) { - return new Promise((resolve) => { - execFile(file, [...args], { encoding: "utf8", env }, (error, stdout, stderr) => { - const signal = error?.signal ?? null; - resolve({ - status: signal ? null : Number(error?.code) || (error ? -1 : 0), - signal, - stdout, - stderr, - }); - }); +function runProcess( + file: string, + args: readonly string[], + env: NodeJS.ProcessEnv, + owner: Pick, +) { + owner.signal.throwIfAborted(); + let stdout = ""; + let stderr = ""; + const child = spawn(file, [...args], { + detached: true, + env, + stdio: ["ignore", "pipe", "pipe"], + }); + const finishController = new AbortController(); + const resultPromise = superviseChild(child, { + killGraceMs: 0, + onStderr: (chunk) => { + stderr += chunk; + }, + onStdout: (chunk) => { + stdout += chunk; + }, + signal: AbortSignal.any([owner.signal, finishController.signal]), + timeoutMs: 10_000, + }).then((result): RunProcessResult => ({ + signal: result.signal, + status: result.signal + ? null + : (result.exitCode ?? (result.spawnError || result.cleanupError ? -1 : null)), + stderr, + stdout, + })); + owner.onTestFinished(async () => { + finishController.abort(); + await resultPromise; }); + return resultPromise; } vi.setConfig({ maxConcurrency: 8 }); @@ -73,13 +101,17 @@ function lines(path: string): string[] { } async function runAuthorization( + owner: Pick, stepName: string, scenario: PermissionScenario, - options: { actor?: string; status?: string } = {}, + options: { actor?: string; onFixtureCreated?: (fixture: string) => void; status?: string } = {}, ) { const fixture = mkdtempSync(join(tmpdir(), "nemoclaw-collaborator-permission-")); + options.onFixtureCreated?.(fixture); const attemptFile = join(fixture, "attempts"); const curlLog = join(fixture, "curl.log"); + const permissionProcessFile = join(fixture, "permission-process"); + const permissionReadyFile = join(fixture, "permission-ready"); const sleepLog = join(fixture, "sleep.log"); const curlPath = join(fixture, "curl"); const sleepPath = join(fixture, "sleep"); @@ -109,6 +141,11 @@ if [[ "$url" == *"/collaborators/"*"/permission" ]]; then curl_exit=0 printf -v body '{"user":{"login":"%s"},"role_name":"admin"}' "$actor" case "$PERMISSION_SCENARIO" in + blocking) + printf '%s\n' "$$" >"$PERMISSION_PROCESS_FILE" + : >"$PERMISSION_READY_FILE" + exec /bin/sleep 60 + ;; transient-then-success) if (( attempt == 1 )); then status="$PERMISSION_TEST_STATUS"; body="private-response-body"; fi ;; @@ -136,46 +173,66 @@ printf '%s\n' "$1" >>"$SLEEP_LOG" chmodSync(curlPath, 0o755); chmodSync(sleepPath, 0o755); - const workflowSha = "c".repeat(40); - const result = await runProcess( - "bash", - ["--noprofile", "--norc", "-c", authorizationScript(stepName)], - { - ...process.env, - ACTOR: options.actor ?? "dispatch-admin", - ALLOW_JETSON_DISPATCH: "false", - BASE_SHA: "b".repeat(40), - CHECKOUT_REPOSITORY: "contributor/NemoClaw", - CHECKOUT_SHA: "", - CURL_LOG: curlLog, - EXPECTED_WORKFLOW_SHA: workflowSha, - GITHUB_REPOSITORY: "NVIDIA/NemoClaw", - GITHUB_TOKEN: "private-test-token", - INCLUDE_LAUNCHABLE: "true", - JOBS: "", - PATH: `${fixture}:${process.env.PATH ?? ""}`, - PERMISSION_ATTEMPT_FILE: attemptFile, - PERMISSION_SCENARIO: scenario, - PERMISSION_TEST_STATUS: options.status ?? "503", - PR_NUMBER: "42", - REVIEW_REASON: "Reviewed latest PR commit", - RUN_ATTEMPT: "1", - RUNNER_TEMP: fixture, - SLEEP_LOG: sleepLog, - TARGETS: "", - TRIGGERING_ACTOR: "dispatch-admin", - WORKFLOW_EVENT: "workflow_dispatch", - WORKFLOW_REF: "refs/heads/main", - WORKFLOW_SHA: workflowSha, - }, - ); - const permissionAttempts = existsSync(attemptFile) - ? Number.parseInt(readFileSync(attemptFile, "utf8"), 10) - : 0; - const curlOperations = lines(curlLog); - const sleeps = lines(sleepLog); - rmSync(fixture, { force: true, recursive: true }); - return { ...result, curlOperations, permissionAttempts, sleeps }; + try { + const workflowSha = "c".repeat(40); + const result = await runProcess( + "bash", + ["--noprofile", "--norc", "-c", authorizationScript(stepName)], + { + ...process.env, + ACTOR: options.actor ?? "dispatch-admin", + ALLOW_JETSON_DISPATCH: "false", + BASE_SHA: "b".repeat(40), + CHECKOUT_REPOSITORY: "contributor/NemoClaw", + CHECKOUT_SHA: "", + CURL_LOG: curlLog, + EXPECTED_WORKFLOW_SHA: workflowSha, + GITHUB_REPOSITORY: "NVIDIA/NemoClaw", + GITHUB_TOKEN: "private-test-token", + INCLUDE_LAUNCHABLE: "true", + JOBS: "", + PATH: `${fixture}:${process.env.PATH ?? ""}`, + PERMISSION_ATTEMPT_FILE: attemptFile, + PERMISSION_PROCESS_FILE: permissionProcessFile, + PERMISSION_READY_FILE: permissionReadyFile, + PERMISSION_SCENARIO: scenario, + PERMISSION_TEST_STATUS: options.status ?? "503", + PR_NUMBER: "42", + REVIEW_REASON: "Reviewed latest PR commit", + RUN_ATTEMPT: "1", + RUNNER_TEMP: fixture, + SLEEP_LOG: sleepLog, + TARGETS: "", + TRIGGERING_ACTOR: "dispatch-admin", + WORKFLOW_EVENT: "workflow_dispatch", + WORKFLOW_REF: "refs/heads/main", + WORKFLOW_SHA: workflowSha, + }, + owner, + ); + const permissionAttempts = existsSync(attemptFile) + ? Number.parseInt(readFileSync(attemptFile, "utf8"), 10) + : 0; + const permissionPid = existsSync(permissionProcessFile) + ? Number.parseInt(readFileSync(permissionProcessFile, "utf8"), 10) + : undefined; + let permissionProcessRunning = false; + try { + process.kill(permissionPid ?? Number.NaN, 0); + permissionProcessRunning = true; + } catch { + // No permission process was started, or the cancelled process is gone as required. + } + return { + ...result, + curlOperations: lines(curlLog), + permissionAttempts, + permissionProcessRunning, + sleeps: lines(sleepLog), + }; + } finally { + rmSync(fixture, { force: true, recursive: true }); + } } describe.concurrent.each(AUTHORIZATION_STEPS)( @@ -183,8 +240,9 @@ describe.concurrent.each(AUTHORIZATION_STEPS)( ({ deniedMessage, mismatchMessage, name }) => { it.for(["408", "429", "503"])( "retries HTTP %s once before authorization succeeds (#9337)", - async (status, { expect }) => { - const result = await runAuthorization(name, "transient-then-success", { status }); + async (status, context) => { + const { expect } = context; + const result = await runAuthorization(context, name, "transient-then-success", { status }); expect(result.status, result.stderr).toBe(0); expect(result.permissionAttempts).toBe(2); @@ -200,8 +258,9 @@ describe.concurrent.each(AUTHORIZATION_STEPS)( }, ); - it("stops after three transient transport failures (#9337)", async ({ expect }) => { - const result = await runAuthorization(name, "transport-exhaustion"); + it("stops after three transient transport failures (#9337)", async (context) => { + const { expect } = context; + const result = await runAuthorization(context, name, "transport-exhaustion"); expect(result.status).not.toBe(0); expect(result.permissionAttempts).toBe(3); @@ -214,8 +273,9 @@ describe.concurrent.each(AUTHORIZATION_STEPS)( it.for(["401", "403", "404", "422"])( "does not retry HTTP %s (#9337)", - async (status, { expect }) => { - const result = await runAuthorization(name, "terminal-http", { status }); + async (status, context) => { + const { expect } = context; + const result = await runAuthorization(context, name, "terminal-http", { status }); expect(result.status).not.toBe(0); expect(result.permissionAttempts).toBe(1); @@ -228,8 +288,9 @@ describe.concurrent.each(AUTHORIZATION_STEPS)( }, ); - it("does not retry a malformed HTTP 200 response (#9337)", async ({ expect }) => { - const result = await runAuthorization(name, "malformed-success"); + it("does not retry a malformed HTTP 200 response (#9337)", async (context) => { + const { expect } = context; + const result = await runAuthorization(context, name, "malformed-success"); expect(result.status).not.toBe(0); expect(result.permissionAttempts).toBe(1); @@ -241,8 +302,9 @@ describe.concurrent.each(AUTHORIZATION_STEPS)( expect(result.stderr).not.toContain("private-response-body"); }); - it("does not retry a valid response with an unauthorized role (#9337)", async ({ expect }) => { - const result = await runAuthorization(name, "denied"); + it("does not retry a valid response with an unauthorized role (#9337)", async (context) => { + const { expect } = context; + const result = await runAuthorization(context, name, "denied"); expect(result.status).not.toBe(0); expect(result.permissionAttempts).toBe(1); @@ -251,8 +313,9 @@ describe.concurrent.each(AUTHORIZATION_STEPS)( expect(result.stderr).toContain(deniedMessage); }); - it("does not retry a permission response for a different actor (#9337)", async ({ expect }) => { - const result = await runAuthorization(name, "mismatched-actor"); + it("does not retry a permission response for a different actor (#9337)", async (context) => { + const { expect } = context; + const result = await runAuthorization(context, name, "mismatched-actor"); expect(result.status).not.toBe(0); expect(result.permissionAttempts).toBe(1); @@ -261,8 +324,9 @@ describe.concurrent.each(AUTHORIZATION_STEPS)( expect(result.stderr).toContain(mismatchMessage); }); - it("rejects an invalid actor before the permission read (#9337)", async ({ expect }) => { - const result = await runAuthorization(name, "transient-then-success", { + it("rejects an invalid actor before the permission read (#9337)", async (context) => { + const { expect } = context; + const result = await runAuthorization(context, name, "transient-then-success", { actor: "invalid actor", }); @@ -272,5 +336,42 @@ describe.concurrent.each(AUTHORIZATION_STEPS)( expect(result.sleeps).toEqual([]); expect(result.stderr).toContain("actor is invalid"); }); + + it("kills a blocked permission read and removes its fixture when cancelled", async (context) => { + const deadline = new AbortController(); + let fixture = ""; + const resultPromise = runAuthorization( + { + onTestFinished: (handler) => context.onTestFinished(handler), + signal: AbortSignal.any([context.signal, deadline.signal]), + }, + name, + "blocking", + { + onFixtureCreated: (createdFixture) => { + fixture = createdFixture; + }, + }, + ); + try { + await vi.waitFor( + () => context.expect(existsSync(join(fixture, "permission-ready"))).toBe(true), + { + interval: 10, + timeout: 5_000, + }, + ); + deadline.abort(); + const result = await resultPromise; + + context.expect(result.status).toBeNull(); + context.expect(result.signal).toMatch(/^SIG(?:TERM|KILL)$/); + context.expect(result.permissionProcessRunning).toBe(false); + context.expect(existsSync(fixture)).toBe(false); + } finally { + deadline.abort(); + await resultPromise; + } + }); }, ); From 5d6fa2bde03656908ccfe06242cffdef5b52e6fb Mon Sep 17 00:00:00 2001 From: Charan Jagwani Date: Sat, 12 Sep 2026 23:44:49 -0700 Subject: [PATCH 3/5] test(e2e): bound collaborator fixture output Signed-off-by: Charan Jagwani --- .../e2e-collaborator-permission-retry.test.ts | 69 +++++++++++++++++-- 1 file changed, 65 insertions(+), 4 deletions(-) diff --git a/test/e2e/support/e2e-collaborator-permission-retry.test.ts b/test/e2e/support/e2e-collaborator-permission-retry.test.ts index e25706f38be..6800c1bd173 100644 --- a/test/e2e/support/e2e-collaborator-permission-retry.test.ts +++ b/test/e2e/support/e2e-collaborator-permission-retry.test.ts @@ -36,42 +36,60 @@ const AUTHORIZATION_STEPS: AuthorizationStep[] = [ ]; type RunProcessResult = { + error?: Error; status: number | null; signal: NodeJS.Signals | null; stdout: string; stderr: string; }; +const MAX_PROCESS_OUTPUT_BYTES = 10 * 1024 * 1024; + function runProcess( file: string, args: readonly string[], env: NodeJS.ProcessEnv, owner: Pick, + maxOutputBytes = MAX_PROCESS_OUTPUT_BYTES, ) { owner.signal.throwIfAborted(); let stdout = ""; let stderr = ""; + let outputError: Error | undefined; const child = spawn(file, [...args], { detached: true, env, stdio: ["ignore", "pipe", "pipe"], }); const finishController = new AbortController(); + const append = (current: string, chunk: string, stream: string): string => { + const next = current + chunk; + const limitError = + !outputError && Buffer.byteLength(next, "utf8") > maxOutputBytes + ? new Error(`${stream} exceeded the process output limit`) + : undefined; + outputError ??= limitError; + void (limitError ? finishController.abort() : undefined); + return outputError ? current : next; + }; const resultPromise = superviseChild(child, { killGraceMs: 0, onStderr: (chunk) => { - stderr += chunk; + stderr = append(stderr, chunk, "stderr"); }, onStdout: (chunk) => { - stdout += chunk; + stdout = append(stdout, chunk, "stdout"); }, signal: AbortSignal.any([owner.signal, finishController.signal]), timeoutMs: 10_000, }).then((result): RunProcessResult => ({ + ...(result.spawnError || result.cleanupError || outputError + ? { error: result.spawnError ?? result.cleanupError ?? outputError } + : {}), signal: result.signal, status: result.signal ? null - : (result.exitCode ?? (result.spawnError || result.cleanupError ? -1 : null)), + : (result.exitCode ?? (result.spawnError || result.cleanupError || outputError ? -1 : null)), stderr, stdout, })); @@ -104,7 +122,11 @@ async function runAuthorization( owner: Pick, stepName: string, scenario: PermissionScenario, - options: { actor?: string; onFixtureCreated?: (fixture: string) => void; status?: string } = {}, + options: { + actor?: string; + onFixtureCreated?: (fixture: string) => void; + status?: string; + } = {}, ) { const fixture = mkdtempSync(join(tmpdir(), "nemoclaw-collaborator-permission-")); options.onFixtureCreated?.(fixture); @@ -373,5 +395,44 @@ describe.concurrent.each(AUTHORIZATION_STEPS)( await resultPromise; } }); + + it.sequential("bounds child output, reaps its process group, and removes its fixture", async (context) => { + const fixture = mkdtempSync(join(tmpdir(), "nemoclaw-collaborator-output-")); + const processFile = join(fixture, "process"); + let result: RunProcessResult | undefined; + let outputProcessPid = Number.NaN; + try { + result = await runProcess( + "bash", + [ + "--noprofile", + "--norc", + "-c", + `printf '%s\\n' "$$" >"$PERMISSION_PROCESS_FILE" +/usr/bin/head -c 65537 /dev/zero >&2 +exec /bin/sleep 60`, + ], + { ...process.env, PERMISSION_PROCESS_FILE: processFile }, + context, + 64 * 1024, + ); + outputProcessPid = Number.parseInt(readFileSync(processFile, "utf8"), 10); + } finally { + rmSync(fixture, { force: true, recursive: true }); + } + let outputProcessRunning = false; + try { + process.kill(outputProcessPid, 0); + outputProcessRunning = true; + } catch { + // The output-limited process is gone as required. + } + + context.expect(result?.error?.message).toBe("stderr exceeded the process output limit"); + context.expect(result?.status).toBeNull(); + context.expect(result?.signal).toMatch(/^SIG(?:TERM|KILL)$/); + context.expect(outputProcessRunning).toBe(false); + context.expect(existsSync(fixture)).toBe(false); + }); }, ); From 7775924dc213a7ce26984961b46fe6f1fb912bb6 Mon Sep 17 00:00:00 2001 From: Charan Jagwani Date: Sun, 13 Sep 2026 00:21:20 -0700 Subject: [PATCH 4/5] refactor(test): share supervised process runner Signed-off-by: Charan Jagwani --- .../cli-artifact-workflow-boundary.test.ts | 59 +++---------- .../e2e-collaborator-permission-retry.test.ts | 65 ++------------ .../e2e/support/openshell-sdk-install.test.ts | 71 +++------------- test/helpers/supervised-process.ts | 84 +++++++++++++++++++ 4 files changed, 118 insertions(+), 161 deletions(-) create mode 100644 test/helpers/supervised-process.ts diff --git a/test/e2e/support/cli-artifact-workflow-boundary.test.ts b/test/e2e/support/cli-artifact-workflow-boundary.test.ts index dd965270193..aa1b602ba5b 100644 --- a/test/e2e/support/cli-artifact-workflow-boundary.test.ts +++ b/test/e2e/support/cli-artifact-workflow-boundary.test.ts @@ -1,7 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { execFileSync, spawn } from "node:child_process"; +import { execFileSync } from "node:child_process"; import assert from "node:assert/strict"; import { createHash } from "node:crypto"; import fs from "node:fs"; @@ -9,7 +9,10 @@ import os from "node:os"; import path from "node:path"; import { describe, it, type TestContext, vi } from "vitest"; -import { superviseChild } from "../../helpers/process-supervisor.ts"; +import { + runSupervisedProcess, + type SupervisedProcessOwner, +} from "../../helpers/supervised-process.ts"; const CANDIDATE_SHA = execFileSync("git", ["rev-parse", "HEAD"], { encoding: "utf8", @@ -19,7 +22,7 @@ const PROCESS_OUTPUT_LIMIT = 1024 * 1024; const IDENTITY_SCRIPT = path.resolve("scripts/e2e/validate-cli-artifact-identity.sh"); const RESTORE_SCRIPT = path.resolve("scripts/e2e/restore-cli-artifact.sh"); -type ProcessOwner = Pick; +type ProcessOwner = SupervisedProcessOwner; type RunProcessOptions = { cwd?: string; @@ -40,56 +43,18 @@ async function runProcess( args: readonly string[], options: RunProcessOptions = {}, ): Promise { - options.owner?.signal.throwIfAborted(); - let stdout = ""; - let stderr = ""; - let outputError: Error | undefined; - const child = spawn(file, [...args], { + const result = await runSupervisedProcess(file, args, { cwd: options.cwd, - detached: true, env: options.env, - stdio: ["ignore", "pipe", "pipe"], - }); - const finishController = new AbortController(); - const append = (current: string, chunk: string, stream: string): string => { - const next = current + chunk; - const limitError = - !outputError && Buffer.byteLength(next, "utf8") > PROCESS_OUTPUT_LIMIT - ? new Error(`${stream} exceeded the process output limit`) - : undefined; - outputError ??= limitError; - void (limitError ? finishController.abort() : undefined); - return outputError ? current : next; - }; - const signal = options.owner - ? AbortSignal.any([options.owner.signal, finishController.signal]) - : finishController.signal; - const resultPromise = superviseChild(child, { - killGraceMs: 0, - onStderr: (chunk) => { - stderr = append(stderr, chunk, "stderr"); - }, - onStdout: (chunk) => { - stdout = append(stdout, chunk, "stdout"); - }, - signal, + maxOutputBytesPerStream: PROCESS_OUTPUT_LIMIT, + owner: options.owner, timeoutMs: options.timeoutMs ?? 20_000, }); - options.owner?.onTestFinished(async () => { - finishController.abort(); - await resultPromise; - }); - const result = await resultPromise; - const processError = outputError ?? result.spawnError ?? result.cleanupError; return { - status: processError - ? -1 - : result.signal - ? null - : (result.exitCode ?? (result.spawnError ? -1 : null)), + status: result.error ? -1 : result.status, signal: result.signal, - stdout, - stderr: processError ? `${stderr}${processError.message}\n` : stderr, + stdout: result.stdout, + stderr: result.error ? `${result.stderr}${result.error.message}\n` : result.stderr, }; } diff --git a/test/e2e/support/e2e-collaborator-permission-retry.test.ts b/test/e2e/support/e2e-collaborator-permission-retry.test.ts index 6800c1bd173..6474767760d 100644 --- a/test/e2e/support/e2e-collaborator-permission-retry.test.ts +++ b/test/e2e/support/e2e-collaborator-permission-retry.test.ts @@ -2,7 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 import assert from "node:assert/strict"; -import { spawn } from "node:child_process"; import { chmodSync, existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -10,7 +9,10 @@ import { join } from "node:path"; import { describe, it, type TestContext, vi } from "vitest"; import { readWorkflow } from "../../helpers/e2e-workflow-contract"; -import { superviseChild } from "../../helpers/process-supervisor"; +import { + runSupervisedProcess, + type SupervisedProcessResult, +} from "../../helpers/supervised-process"; type AuthorizationStep = { deniedMessage: string; @@ -35,14 +37,6 @@ const AUTHORIZATION_STEPS: AuthorizationStep[] = [ }, ]; -type RunProcessResult = { - error?: Error; - status: number | null; - signal: NodeJS.Signals | null; - stdout: string; - stderr: string; -}; - const MAX_PROCESS_OUTPUT_BYTES = 10 * 1024 * 1024; function runProcess( @@ -52,52 +46,12 @@ function runProcess( owner: Pick, maxOutputBytes = MAX_PROCESS_OUTPUT_BYTES, ) { - owner.signal.throwIfAborted(); - let stdout = ""; - let stderr = ""; - let outputError: Error | undefined; - const child = spawn(file, [...args], { - detached: true, + return runSupervisedProcess(file, args, { env, - stdio: ["ignore", "pipe", "pipe"], - }); - const finishController = new AbortController(); - const append = (current: string, chunk: string, stream: string): string => { - const next = current + chunk; - const limitError = - !outputError && Buffer.byteLength(next, "utf8") > maxOutputBytes - ? new Error(`${stream} exceeded the process output limit`) - : undefined; - outputError ??= limitError; - void (limitError ? finishController.abort() : undefined); - return outputError ? current : next; - }; - const resultPromise = superviseChild(child, { - killGraceMs: 0, - onStderr: (chunk) => { - stderr = append(stderr, chunk, "stderr"); - }, - onStdout: (chunk) => { - stdout = append(stdout, chunk, "stdout"); - }, - signal: AbortSignal.any([owner.signal, finishController.signal]), + maxOutputBytesPerStream: maxOutputBytes, + owner, timeoutMs: 10_000, - }).then((result): RunProcessResult => ({ - ...(result.spawnError || result.cleanupError || outputError - ? { error: result.spawnError ?? result.cleanupError ?? outputError } - : {}), - signal: result.signal, - status: result.signal - ? null - : (result.exitCode ?? (result.spawnError || result.cleanupError || outputError ? -1 : null)), - stderr, - stdout, - })); - owner.onTestFinished(async () => { - finishController.abort(); - await resultPromise; }); - return resultPromise; } vi.setConfig({ maxConcurrency: 8 }); @@ -396,10 +350,10 @@ describe.concurrent.each(AUTHORIZATION_STEPS)( } }); - it.sequential("bounds child output, reaps its process group, and removes its fixture", async (context) => { + it.sequential("bounds child output and reaps its process group", async (context) => { const fixture = mkdtempSync(join(tmpdir(), "nemoclaw-collaborator-output-")); const processFile = join(fixture, "process"); - let result: RunProcessResult | undefined; + let result: SupervisedProcessResult | undefined; let outputProcessPid = Number.NaN; try { result = await runProcess( @@ -432,7 +386,6 @@ exec /bin/sleep 60`, context.expect(result?.status).toBeNull(); context.expect(result?.signal).toMatch(/^SIG(?:TERM|KILL)$/); context.expect(outputProcessRunning).toBe(false); - context.expect(existsSync(fixture)).toBe(false); }); }, ); diff --git a/test/e2e/support/openshell-sdk-install.test.ts b/test/e2e/support/openshell-sdk-install.test.ts index 70ebcef3ebb..29746b5d9c9 100644 --- a/test/e2e/support/openshell-sdk-install.test.ts +++ b/test/e2e/support/openshell-sdk-install.test.ts @@ -2,15 +2,18 @@ // SPDX-License-Identifier: Apache-2.0 import assert from "node:assert/strict"; -import { spawn } from "node:child_process"; import { createHash } from "node:crypto"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { describe, it, type TestContext, vi } from "vitest"; +import { describe, it, vi } from "vitest"; import YAML from "yaml"; import { testTimeoutOptions } from "../../helpers/timeouts.ts"; -import { superviseChild } from "../../helpers/process-supervisor.ts"; +import { + runSupervisedProcess, + type SupervisedProcessOwner, + type SupervisedProcessResult, +} from "../../helpers/supervised-process.ts"; const profile = YAML.parse( fs.readFileSync(".github/workflows/e2e-standard-profile.yaml", "utf8"), @@ -31,77 +34,29 @@ const externalGatewayInstallScript = externalGateway.jobs["external-gateway-heal type RunProcessOptions = { cwd?: string; env?: NodeJS.ProcessEnv; - owner: Pick; + owner: SupervisedProcessOwner; timeoutMs: number; }; -type RunProcessResult = { - error?: Error; - signal: NodeJS.Signals | null; - status: number | null; - stderr: string; - stdout: string; -}; - function runProcess( file: string, args: readonly string[], options: RunProcessOptions, -): Promise { - options.owner.signal.throwIfAborted(); - let stdout = ""; - let stderr = ""; - let outputError: Error | undefined; - const child = spawn(file, [...args], { +): Promise { + return runSupervisedProcess(file, args, { cwd: options.cwd, - detached: true, env: options.env, - stdio: ["ignore", "pipe", "pipe"], - }); - const finishController = new AbortController(); - const append = (current: string, chunk: string, stream: string): string => { - const next = current + chunk; - const limitError = - !outputError && Buffer.byteLength(next, "utf8") > 10 * 1024 * 1024 - ? new Error(`${stream} exceeded the 10 MiB process output limit`) - : undefined; - outputError ??= limitError; - void (limitError ? finishController.abort() : undefined); - return outputError ? current : next; - }; - const resultPromise = superviseChild(child, { - killGraceMs: 0, - onStderr: (chunk) => { - stderr = append(stderr, chunk, "stderr"); - }, - onStdout: (chunk) => { - stdout = append(stdout, chunk, "stdout"); - }, - signal: AbortSignal.any([options.owner.signal, finishController.signal]), + maxOutputBytesPerStream: 10 * 1024 * 1024, + owner: options.owner, timeoutMs: options.timeoutMs, }); - options.owner.onTestFinished(async () => { - finishController.abort(); - await resultPromise; - }); - return resultPromise.then((result) => ({ - ...(result.spawnError || result.cleanupError || outputError - ? { error: result.spawnError ?? result.cleanupError ?? outputError } - : {}), - signal: result.signal, - status: result.signal - ? null - : (result.exitCode ?? (result.spawnError || result.cleanupError || outputError ? -1 : null)), - stderr, - stdout, - })); } async function runSuccessfulProcess( file: string, args: readonly string[], options: RunProcessOptions, -): Promise { +): Promise { const result = await runProcess(file, args, options); assert.equal(result.error, undefined, result.error?.message); assert.equal(result.status, 0, `${result.stdout}\n${result.stderr}`); @@ -131,7 +86,7 @@ type PackageDefinition = { async function writePackageArchives( root: string, packages: readonly PackageDefinition[], - owner: Pick, + owner: SupervisedProcessOwner, ) { const sources = packages.map(({ dependencies = {}, name, version = "1.0.0" }) => { const source = path.join(root, `${name.replaceAll("/", "-")}-${version}`); diff --git a/test/helpers/supervised-process.ts b/test/helpers/supervised-process.ts new file mode 100644 index 00000000000..34a935a4d1c --- /dev/null +++ b/test/helpers/supervised-process.ts @@ -0,0 +1,84 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { spawn } from "node:child_process"; +import type { TestContext } from "vitest"; + +import { superviseChild } from "./process-supervisor.ts"; + +export type SupervisedProcessOwner = Pick; + +export interface RunSupervisedProcessOptions { + cwd?: string; + env?: NodeJS.ProcessEnv; + maxOutputBytesPerStream: number; + owner?: SupervisedProcessOwner; + timeoutMs: number; +} + +export interface SupervisedProcessResult { + error?: Error; + signal: NodeJS.Signals | null; + status: number | null; + stderr: string; + stdout: string; + timedOut: boolean; +} + +/** Runs a detached test fixture with bounded output and process-group cleanup. */ +export function runSupervisedProcess( + file: string, + args: readonly string[], + options: RunSupervisedProcessOptions, +): Promise { + options.owner?.signal.throwIfAborted(); + let stdout = ""; + let stderr = ""; + let outputError: Error | undefined; + const child = spawn(file, [...args], { + cwd: options.cwd, + detached: true, + env: options.env, + stdio: ["ignore", "pipe", "pipe"], + }); + const finishController = new AbortController(); + const append = (current: string, chunk: string, stream: string): string => { + const next = current + chunk; + const limitError = + !outputError && Buffer.byteLength(next, "utf8") > options.maxOutputBytesPerStream + ? new Error(`${stream} exceeded the process output limit`) + : undefined; + outputError ??= limitError; + if (limitError) finishController.abort(); + return outputError ? current : next; + }; + const signal = options.owner + ? AbortSignal.any([options.owner.signal, finishController.signal]) + : finishController.signal; + const resultPromise = superviseChild(child, { + killGraceMs: 0, + onStderr: (chunk) => { + stderr = append(stderr, chunk, "stderr"); + }, + onStdout: (chunk) => { + stdout = append(stdout, chunk, "stdout"); + }, + signal, + timeoutMs: options.timeoutMs, + }).then((result): SupervisedProcessResult => { + const error = outputError ?? result.spawnError ?? result.cleanupError; + return { + ...(error ? { error } : {}), + signal: result.signal, + status: result.signal ? null : (result.exitCode ?? (error ? -1 : null)), + stderr, + stdout, + timedOut: result.timedOut, + }; + }); + options.owner?.onTestFinished(async () => { + finishController.abort(); + await resultPromise; + }); + return resultPromise; +} From 3ebf7ca1da58d869c364ef73a3ec2f0c39828e10 Mon Sep 17 00:00:00 2001 From: Charan Jagwani Date: Sun, 13 Sep 2026 00:55:18 -0700 Subject: [PATCH 5/5] fix(test): preserve supervised process failures Signed-off-by: Charan Jagwani --- .../e2e-collaborator-permission-retry.test.ts | 19 +++++++++++++++++++ test/helpers/supervised-process.ts | 5 +++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/test/e2e/support/e2e-collaborator-permission-retry.test.ts b/test/e2e/support/e2e-collaborator-permission-retry.test.ts index 6474767760d..541d4a7ab0c 100644 --- a/test/e2e/support/e2e-collaborator-permission-retry.test.ts +++ b/test/e2e/support/e2e-collaborator-permission-retry.test.ts @@ -387,5 +387,24 @@ exec /bin/sleep 60`, context.expect(result?.signal).toMatch(/^SIG(?:TERM|KILL)$/); context.expect(outputProcessRunning).toBe(false); }); + + it.sequential("reports oversized output as failure after a zero exit", async (context) => { + const result = await runProcess( + "bash", + [ + "--noprofile", + "--norc", + "-c", + "(sleep 0.05; /usr/bin/head -c 2048 /dev/zero >&2) & exit 0", + ], + process.env, + context, + 1024, + ); + + context.expect(result.error?.message).toBe("stderr exceeded the process output limit"); + context.expect(result.status).toBe(-1); + context.expect(result.signal).toBeNull(); + }); }, ); diff --git a/test/helpers/supervised-process.ts b/test/helpers/supervised-process.ts index 34a935a4d1c..ab0b8397044 100644 --- a/test/helpers/supervised-process.ts +++ b/test/helpers/supervised-process.ts @@ -11,6 +11,7 @@ export type SupervisedProcessOwner = Pick { stderr = append(stderr, chunk, "stderr"); }, @@ -70,7 +71,7 @@ export function runSupervisedProcess( return { ...(error ? { error } : {}), signal: result.signal, - status: result.signal ? null : (result.exitCode ?? (error ? -1 : null)), + status: result.signal ? null : error ? -1 : result.exitCode, stderr, stdout, timedOut: result.timedOut,