From 52976ac5cdc08c76ca48ae0800b61024e4ae1af8 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Mon, 26 Oct 2026 13:56:28 +0800 Subject: [PATCH] fix(doctor): bound and cancel diagnostic subprocesses --- src/application/Doctor.ts | 66 ++++++-- .../doctorSystemHostFilesystem.test.ts | 155 +++++++++++++++++- 2 files changed, 210 insertions(+), 11 deletions(-) diff --git a/src/application/Doctor.ts b/src/application/Doctor.ts index 9b56c8190e..0277b32e02 100644 --- a/src/application/Doctor.ts +++ b/src/application/Doctor.ts @@ -22,7 +22,12 @@ import { join } from "node:path"; import { analysisErrorRemediationAction } from "../domain/analysisErrorPresentation.js"; import { BinaryTargetError } from "../domain/configurationErrors.js"; import { parseBinaryTarget } from "./BinaryTargetResolver.js"; -import { execFileOutput } from "../process/ExecFileOutput.js"; +import { + execFileOutput, + ExecFileOutputCancelledError, + execFileOutputFailure, + execFileFailureCapturedOutput, +} from "../process/ExecFileOutput.js"; import { jsonValueSchema, type JsonValue } from "../domain/jsonValue.js"; import { javaScriptRuntime, @@ -97,8 +102,8 @@ export interface DoctorProviderInspection { readonly configured: boolean; readonly available: boolean; readonly providerVersion: string | null; - /** Adapter-validated non-secret settings, independent of runtime readiness. - * Invalid installations publish no settings. */ + /** Validated discoveries used as registration defaults. Setup preserves + * caller-selected configuration separately from these readiness facts. */ readonly registrationEnvironment: Readonly>; readonly checks: readonly DoctorProviderCheck[]; } @@ -545,7 +550,47 @@ export const systemDoctorHost = ( const nativeConfiguration = inspectNativeAnalysisConfiguration( snapshotEnvironment(environment, platform), ); - const hostExecFileOutput = options.execFileOutput ?? execFileOutput; + const execute = options.execFileOutput ?? execFileOutput; + // These commands report short host facts, not analysis output. Reuse the + // ILSpy probe's ten-second allowance and bound each captured stream to 64 KiB. + // Baseline diagnostics must work without provider ownership prerequisites. + const hostExecFileOutput: typeof execFileOutput = (command, args, input) => + resources.run(async () => { + try { + const output = await execute(command, args, { + ...input, + timeout: 10_000, + maxBuffer: 64 * 1024, + stopSignal: "SIGTERM", + signal: inspectionSignal, + }); + if (inspectionSignal.aborted) + throw new AnalysisCancelledError("doctor", { + cause: inspectionSignal.reason, + capturedOutput: { + stdout: output.stdout, + stderr: output.stderr, + stdout_bytes: output.stdoutBytes, + stderr_bytes: output.stderrBytes, + exit_code: 0, + signal: null, + truncated: false, + }, + }); + return output; + } catch (cause: unknown) { + if (cause instanceof ExecFileOutputCancelledError) { + const output = execFileOutputFailure(cause); + throw new AnalysisCancelledError("doctor", { + cause: cause.cause, + ...(output === undefined + ? {} + : { capturedOutput: execFileFailureCapturedOutput(output) }), + }); + } + throw cause; + } + }); const homeDirectory = homeDirectoryFromEnvironment(environment, platform); const commandEnvironment = { env: environment }; const resourceScope: RetainedResourceScope = { @@ -652,6 +697,7 @@ export const systemDoctorHost = ( ) ).stdout.trim(); } catch (cause: unknown) { + if (cause instanceof AnalysisCancelledError) throw cause; // best-effort cleanup: optional Homebrew probing; absence means uninstalled. void cause; return undefined; @@ -689,14 +735,11 @@ export const systemDoctorHost = ( try { await access(path, constants.X_OK); return firstIlspyVersionLine( - ( - await hostExecFileOutput(path, ["--version"], { - timeout: 10_000, - ...commandEnvironment, - }) - ).stdout, + (await hostExecFileOutput(path, ["--version"], commandEnvironment)) + .stdout, ); } catch (cause: unknown) { + if (cause instanceof AnalysisCancelledError) throw cause; // best-effort cleanup: optional version probing; failure means unavailable. void cause; return undefined; @@ -711,6 +754,7 @@ export const systemDoctorHost = ( .stdout, ); } catch (cause: unknown) { + if (cause instanceof AnalysisCancelledError) throw cause; // best-effort cleanup: optional path probing; failure means none found. void cause; return []; @@ -768,6 +812,7 @@ const readMacosVersion = async ( await run("/usr/bin/sw_vers", ["-productVersion"], options) ).stdout.trim(); } catch (cause: unknown) { + if (cause instanceof AnalysisCancelledError) throw cause; // best-effort cleanup: optional OS version probing; failure means unknown. void cause; return undefined; @@ -787,6 +832,7 @@ const executableAvailable = async ( const linked = await run("ldd", [path], options); return linuxSharedLibrariesAvailable(`${linked.stdout}\n${linked.stderr}`); } catch (cause: unknown) { + if (cause instanceof AnalysisCancelledError) throw cause; // best-effort cleanup: optional executable probing; failure means unavailable. void cause; return false; diff --git a/tests/boundary/filesystem/doctorSystemHostFilesystem.test.ts b/tests/boundary/filesystem/doctorSystemHostFilesystem.test.ts index cf1396f0c8..201a4a568f 100644 --- a/tests/boundary/filesystem/doctorSystemHostFilesystem.test.ts +++ b/tests/boundary/filesystem/doctorSystemHostFilesystem.test.ts @@ -1,10 +1,12 @@ -import { mkdir, writeFile } from "node:fs/promises"; +import { access, mkdir, readFile, writeFile } from "node:fs/promises"; import { dirname, join } from "node:path"; import { expect, it } from "vitest"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { systemDoctorHost } from "../../../src/application/Doctor.js"; +import { AnalysisCancelledError } from "../../../src/domain/analysisErrorCore.js"; +import { ExecFileOutputCancelledError } from "../../../src/process/ExecFileOutput.js"; it("threads the selected host and environment into system diagnostics", async () => { const environment = { @@ -22,6 +24,12 @@ it("threads the selected host and environment into system diagnostics", async () architecture: "x64", environment, execFileOutput: async (command, arguments_, options) => { + expect(options).toMatchObject({ + timeout: 10_000, + maxBuffer: 64 * 1024, + stopSignal: "SIGTERM", + signal: expect.any(AbortSignal), + }); calls.push({ command, arguments: arguments_, environment: options?.env }); if (command === "/usr/bin/sw_vers") return { @@ -65,6 +73,151 @@ it("threads the selected host and environment into system diagnostics", async () ).toBe(true); }); +it.each([ + "macosVersion", + "installationPaths", + "brewHopperPath", + "executable", + "ilspyCmdVersion", +] as const)( + "preserves cancellation through the %s fallback", + async (operation) => { + const reason = { request: "stop diagnostics" }; + const host = systemDoctorHost({ + platform: "linux", + environment: {}, + execFileOutput: async () => { + throw new ExecFileOutputCancelledError(reason); + }, + }); + try { + await expect(host[operation]?.(process.execPath)).rejects.toMatchObject({ + _tag: "AnalysisCancelledError", + cause: reason, + }); + } finally { + await host.close?.(); + } + }, +); + +it.each(["complete", "failed"] as const)( + "retains a %s probe's outcome when cancellation arrives at completion", + async (outcome) => { + const controller = new AbortController(); + const reason = { request: "late cancellation" }; + const host = systemDoctorHost({ + signal: controller.signal, + environment: {}, + execFileOutput: async () => { + controller.abort(reason); + if (outcome === "failed") + throw Object.assign(new Error("probe timed out"), { + code: "ETIMEDOUT", + }); + return { + stdout: "observed", + stderr: "warning", + stdoutBytes: 8, + stderrBytes: 7, + }; + }, + }); + try { + const pending = host.installationPaths?.(); + if (outcome === "failed") await expect(pending).resolves.toEqual([]); + else + await expect(pending).rejects.toMatchObject({ + _tag: "AnalysisCancelledError", + cause: reason, + capturedOutput: { + stdout: "observed", + stderr: "warning", + stdout_bytes: 8, + stderr_bytes: 7, + exit_code: 0, + }, + }); + } finally { + await host.close?.(); + } + }, +); + +it.skipIf(process.platform === "win32").each(["abort", "close"] as const)( + "%s joins an active diagnostic probe's exit and preserves cancellation", + async (action) => { + const home = await createTestTempDirectory("rea-doctor-probe-stop-"); + const started = join(home, "started"); + const cleaned = join(home, "cleaned"); + await writeFile( + join(home, "which"), + `#!${process.execPath} +const fs = require("node:fs"); +const timer = setTimeout(() => process.exit(91), 8000); +process.on("SIGTERM", () => { + clearTimeout(timer); + setTimeout(() => { fs.writeFileSync(${JSON.stringify(cleaned)}, "cleaned"); process.exit(0); }, 50); +}); +fs.writeFileSync(${JSON.stringify(started)}, "ready"); +`, + { mode: 0o700 }, + ); + const controller = new AbortController(); + const reason = { request: "cancel running probe" }; + const host = systemDoctorHost({ + environment: { HOME: home, PATH: home }, + signal: controller.signal, + }); + const pending = host.installationPaths!().then( + (value) => value, + (cause: unknown) => cause, + ); + try { + await expect.poll(async () => readFile(started, "utf8")).toBe("ready"); + if (action === "abort") controller.abort(reason); + else await host.close?.(); + const failure = await pending; + expect(failure).toBeInstanceOf(AnalysisCancelledError); + if (action === "abort") expect((failure as Error).cause).toBe(reason); + expect(await readFile(cleaned, "utf8")).toBe("cleaned"); + } finally { + controller.abort(reason); + await pending; + await host.close?.(); + } + }, +); + +it.skipIf(process.platform === "win32")( + "does not launch a diagnostic for an already cancelled request", + async () => { + const home = await createTestTempDirectory("rea-doctor-pre-abort-"); + const marker = join(home, "launched"); + await writeFile( + join(home, "which"), + `#!${process.execPath}\nrequire("node:fs").writeFileSync(${JSON.stringify(marker)}, "launched");\n`, + { mode: 0o700 }, + ); + const controller = new AbortController(); + const reason = { request: "already cancelled" }; + controller.abort(reason); + const host = systemDoctorHost({ + environment: { HOME: home, PATH: home }, + signal: controller.signal, + }); + try { + await expect(host.installationPaths?.()).rejects.toMatchObject({ + _tag: "AnalysisCancelledError", + cause: reason, + }); + await expect(access(marker)).rejects.toMatchObject({ code: "ENOENT" }); + } finally { + await host.close?.(); + } + }, +); + it("does not report a directory as an available executable", async () => { const host = systemDoctorHost({ platform: "darwin",