diff --git a/docs/testing.md b/docs/testing.md index d9d4775d35..946db07bf1 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -791,6 +791,13 @@ behavior requires the applicable `verify:*` lanes; packaging and installation changes also require `npm run verify:package`. The full-gate benchmark measures that explicit lane, not the routine iteration requirement. +The package-update registry fixture retains its first proxy/body failure and +aborts the owned updater command. It joins the helper and installer descendants +before workspace removal. Incomplete process cleanup retains the exact owner +and workspace rather than reporting successful removal. The controlled +`packageUpdateRegistryFailure` regression needs built process modules, uses no +public registry, and checks both the primary failure and live descendant absence. + ## Real-toolchain verification lanes Linux Rizin debugger ownership changes require a separate real-provider gate on diff --git a/scripts/verify-package-update.mjs b/scripts/verify-package-update.mjs index 0741e3fe5f..f9af1d3c43 100644 --- a/scripts/verify-package-update.mjs +++ b/scripts/verify-package-update.mjs @@ -1,10 +1,107 @@ -import { createHash } from "node:crypto"; +import { createHash, randomUUID } from "node:crypto"; import { createServer } from "node:http"; import { readFile, writeFile, mkdir } from "node:fs/promises"; import { dirname, join } from "node:path"; import { pathToFileURL } from "node:url"; import { exec, json } from "./lib/verify-package-core.mjs"; +import { closeVerifierResources } from "./lib/verifier-cleanup.mjs"; + +/** Keep the live updater owner and its workspace when process cleanup is unproven. */ +export class PackageUpdateCleanupFailure extends AggregateError { + constructor(primary, cleanupOwner, result, workspace) { + const cleanup = new Error(result.reason); + super( + primary === undefined ? [cleanup] : [primary, cleanup], + `Package updater cleanup incomplete; retained ${workspace}`, + { cause: primary }, + ); + this.cleanupOwner = cleanupOwner; + this.workspace = workspace; + } +} + +/** Own the helper and installer descendants until output and process cleanup finish. */ +async function runUpdater({ + harness, + environment, + signal, + workspace, + retainWorkspace, +}) { + // Only updater execution needs built process ownership; importing this fixture + // does not make unrelated source-only verifier helpers depend on dist. + const { ProviderProcessSupervisor, spawnOwnedProviderProcess } = + await import("../dist/process/ProviderProcess.js"); + const { cleanupOwnedProcessGroup } = + await import("../dist/process/ProcessOwnership.js"); + const launch = await spawnOwnedProviderProcess({ + command: process.execPath, + arguments: [harness], + expectedCommand: null, + hostEnvironment: environment, + cwd: workspace, + runId: randomUUID(), + sweepTokenOwnedProcesses: true, + signal, + }); + let collectionFailure; + const supervisor = new ProviderProcessSupervisor( + { + ...launch, + ownsProcessLifetime: true, + cleanup: + launch.cleanup ?? (() => cleanupOwnedProcessGroup(launch.ownership)), + }, + { + maxDiagnosticBytes: 16 * 1024 * 1024, + onDiagnostic(event) { + if (event.type === "output" && event.truncated) + collectionFailure ??= new Error( + "Package updater exceeded its diagnostic capacity", + ); + if (event.type === "error") + collectionFailure ??= new Error(event.message, { + cause: event.cause, + }); + }, + }, + ); + let failure; + let stdout; + try { + while (!(await supervisor.waitForOutputClose(50))) { + signal.throwIfAborted(); + if (collectionFailure !== undefined) throw collectionFailure; + } + signal.throwIfAborted(); + if (collectionFailure !== undefined) throw collectionFailure; + const snapshot = supervisor.snapshot(); + if ( + snapshot.diagnosticTruncated || + snapshot.exitCode !== 0 || + snapshot.signal !== null + ) + throw new Error( + `Package updater failed: ${snapshot.stdout.text}\n${snapshot.stderr.text}`, + ); + stdout = snapshot.stdout.text; + } catch (cause) { + failure = { cause }; + } + const stopped = await supervisor.stop(); + if (stopped.status === "incomplete") { + retainWorkspace?.(); + throw new PackageUpdateCleanupFailure( + failure?.cause, + supervisor, + stopped, + workspace, + ); + } + if (failure !== undefined) throw failure.cause; + return stdout; +} /** Exercise the packaged updater with real npm and a local release registry fixture. */ export async function verifyPackageUpdate({ @@ -13,6 +110,7 @@ export async function verifyPackageUpdate({ tarball, workspace, environment, + retainWorkspace, }) { const metadataPath = join(packageRoot, "package.json"); const metadata = json(await readFile(metadataPath, "utf8")); @@ -25,9 +123,11 @@ export async function verifyPackageUpdate({ await mkdir(dirname(codexPath), { recursive: true }); await writeFile(codexPath, originalConfig); let fixtureFailure; + const fixture = new AbortController(); const server = createServer((request, response) => { void serveRegistry(request, response).catch((error) => { fixtureFailure ??= error; + fixture.abort(fixtureFailure); if (response.headersSent) { response.destroy(); return; @@ -75,6 +175,7 @@ export async function verifyPackageUpdate({ server.once("error", reject); server.listen(0, "127.0.0.1", resolveListen); }); + let primary; try { const address = server.address(); if (typeof address !== "object" || address === null) @@ -140,12 +241,13 @@ if (result.status === "failed") process.exitCode = 1; `, ); const result = json( - ( - await exec(process.execPath, [harness], { - env: runtimeEnvironment, - maxBuffer: 16 * 1024 * 1024, - }) - ).stdout, + await runUpdater({ + harness, + environment: runtimeEnvironment, + signal: fixture.signal, + workspace, + retainWorkspace, + }), ); if ( result.status !== "updated" || @@ -185,13 +287,24 @@ if (result.status === "failed") process.exitCode = 1; maintenance_read_only: true, }; } catch (error) { - throw fixtureFailure ?? error; + primary = + error instanceof PackageUpdateCleanupFailure + ? error + : (fixtureFailure ?? error); + throw primary; } finally { - server.closeAllConnections(); - await new Promise((resolveClose, reject) => - server.close((error) => - error === undefined ? resolveClose() : reject(error), - ), + await closeVerifierResources( + [ + async () => { + server.closeAllConnections(); + await new Promise((resolveClose, reject) => + server.close((error) => + error === undefined ? resolveClose() : reject(error), + ), + ); + }, + ], + primary, ); } } diff --git a/scripts/verify-package.mjs b/scripts/verify-package.mjs index a20ecd487d..219b38dd62 100644 --- a/scripts/verify-package.mjs +++ b/scripts/verify-package.mjs @@ -48,6 +48,7 @@ let repositorySkill; let report; let retainedPackage; let retentionMetadata; +let updateWorkspaceRetained = false; const runStage = async (name, action) => { const started = performance.now(); @@ -115,6 +116,9 @@ try { tarball, workspace, environment: environmentData.environment, + retainWorkspace() { + updateWorkspaceRetained = true; + }, }), ); mcpStartup = await runStage("MCP startup", () => @@ -255,9 +259,14 @@ try { }); } } finally { - await runStage("workspace cleanup", () => - rm(workspace, { recursive: true, force: true }), - ); + if (updateWorkspaceRetained) + process.stderr.write( + `Package updater cleanup incomplete; retained ${workspace}\n`, + ); + else + await runStage("workspace cleanup", () => + rm(workspace, { recursive: true, force: true }), + ); } const verifierLineage = await runStage("verifier lineage", () => completeVerifierRun(verifierRun), diff --git a/tests/boundary/process/packageUpdateRegistryFailure.test.ts b/tests/boundary/process/packageUpdateRegistryFailure.test.ts index 303e8ac82b..0f6e5afc4d 100644 --- a/tests/boundary/process/packageUpdateRegistryFailure.test.ts +++ b/tests/boundary/process/packageUpdateRegistryFailure.test.ts @@ -11,13 +11,14 @@ import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; const execFileAsync = promisify(execFile); const injectedFailure = "injected upstream body failure"; -it("reports the upstream body failure instead of a second header write", async () => { +it("retains upstream body failure and joins the active updater and installer child", async () => { const testRoot = await createTestTempDirectory("rea-update-registry-"); const packageRoot = join(testRoot, "package"); const workspace = join(testRoot, "workspace"); const prefix = join(testRoot, "prefix"); const tarball = join(testRoot, "release.tgz"); const application = join(packageRoot, "dist", "application"); + const descendants = join(workspace, "updater-pids.json"); await Promise.all([ mkdir(application, { recursive: true }), mkdir(join(packageRoot, "scripts"), { recursive: true }), @@ -46,13 +47,19 @@ it("reports the upstream body failure instead of a second header write", async ( writeFile( join(application, "Update.js"), [ + 'import { spawn } from "node:child_process";', + 'import { writeFile } from "node:fs/promises";', "export async function runUpdate() {", + ' const child = spawn(process.execPath, ["--eval", "setInterval(() => {}, 1000)"], { stdio: "ignore" });', + ' await new Promise((resolve, reject) => { child.once("spawn", resolve); child.once("error", reject); });', + ` await writeFile(${JSON.stringify(descendants)}, JSON.stringify([process.pid, child.pid]));`, " const registry = process.env.npm_config_registry;", " const response = await fetch(`${registry}/unlisted`, {", " signal: AbortSignal.timeout(5000),", " });", " const body = await response.text();", - " throw new Error(`registry fixture answered ${response.status}: ${body}`);", + " setInterval(() => {}, 1000);", + " await new Promise(() => {});", "}", "", ].join("\n"), @@ -86,7 +93,20 @@ try { } catch (cause) { failure = cause instanceof Error ? cause.message : String(cause); } -process.stdout.write(JSON.stringify({ failure })); +const { readFile } = await import("node:fs/promises"); +const pids = JSON.parse(await readFile(${JSON.stringify(descendants)}, "utf8")); +let livePids; +if (process.platform === "win32") { + livePids = pids.filter((pid) => { + try { process.kill(pid, 0); return true; } + catch (cause) { if (cause.code === "ESRCH") return false; throw cause; } + }); +} else { + const { systemProcessOwnershipHost } = await import(${JSON.stringify(pathToFileURL(resolve("dist/process/ProcessOwnershipObservation.js")).href)}); + const { liveProcesses } = await import(${JSON.stringify(pathToFileURL(resolve("dist/process/ProcessOwnershipProcessTree.js")).href)}); + livePids = liveProcesses(await systemProcessOwnershipHost.listProcesses()).filter(({pid}) => pids.includes(pid)).map(({pid}) => pid); +} +process.stdout.write(JSON.stringify({ failure, observedPids: pids.length, livePids })); `; const { stdout } = await execFileAsync( process.execPath, @@ -94,5 +114,9 @@ process.stdout.write(JSON.stringify({ failure })); { env: { ...process.env }, maxBuffer: 1_024 * 1_024, timeout: 30_000 }, ); - expect(JSON.parse(stdout)).toEqual({ failure: injectedFailure }); + expect(JSON.parse(stdout)).toEqual({ + failure: injectedFailure, + observedPids: 2, + livePids: [], + }); }, 45_000);