diff --git a/src/domain/runtime.ts b/src/domain/runtime.ts index 2bd6ffe..e606e58 100644 --- a/src/domain/runtime.ts +++ b/src/domain/runtime.ts @@ -14,6 +14,7 @@ export type OnboardFailureCode = | "missing_tty" | "unsupported_platform" | "hermes_not_found" + | "hermes_unavailable" | "unsupported_hermes_version" | "managed_install" | "write_blocked" @@ -109,6 +110,7 @@ export const ONBOARD_FAILURE_FAMILY_BY_CODE = { missing_tty: "runtime", unsupported_platform: "runtime", hermes_not_found: "runtime", + hermes_unavailable: "runtime", unsupported_hermes_version: "runtime", managed_install: "runtime", write_blocked: "runtime", diff --git a/src/hermes/cli.ts b/src/hermes/cli.ts index 00ae15d..a43aabf 100644 --- a/src/hermes/cli.ts +++ b/src/hermes/cli.ts @@ -1,21 +1,32 @@ import type { OnboardDependencies } from "../runtime/dependencies.js"; export const HERMES_COMMAND = "hermes"; +export const HERMES_COMMAND_TIMEOUT_MS = 10_000; export type HermesCliFailure = | { args: readonly string[]; - cause: unknown; + cause?: unknown; + errorCode?: string; kind: "not_found"; } | { args: readonly string[]; + errorCode?: string; exitCode: number; signal: NodeJS.Signals | null; stderr: string; stdout: string; kind: "nonzero_exit"; } + | { + args: readonly string[]; + signal: NodeJS.Signals | null; + stderr: string; + stdout: string; + timeoutMs: number; + kind: "timed_out"; + } | { args: readonly string[]; reason: string; @@ -49,12 +60,39 @@ export async function runHermesCommand( const result = await dependencies.commands.execFile(HERMES_COMMAND, args, { cwd: dependencies.runtime.cwd, env: dependencies.runtime.env, + timeoutMs: HERMES_COMMAND_TIMEOUT_MS, }); + if (result.errorCode === "ENOENT") { + return { + failure: { + args, + errorCode: result.errorCode, + kind: "not_found", + }, + ok: false, + }; + } + + if (result.timedOut === true) { + return { + failure: { + args, + kind: "timed_out", + signal: result.signal, + stderr: result.stderr, + stdout: result.stdout, + timeoutMs: HERMES_COMMAND_TIMEOUT_MS, + }, + ok: false, + }; + } + if (result.exitCode !== 0) { return { failure: { args, + errorCode: result.errorCode, exitCode: result.exitCode, kind: "nonzero_exit", signal: result.signal, diff --git a/src/runtime/dependencies.ts b/src/runtime/dependencies.ts index 6f8ef97..067395a 100644 --- a/src/runtime/dependencies.ts +++ b/src/runtime/dependencies.ts @@ -27,10 +27,12 @@ export interface OnboardRuntimeEnvironment { } export interface OnboardCommandResult { + errorCode?: string; exitCode: number; signal: NodeJS.Signals | null; stderr: string; stdout: string; + timedOut?: boolean; } export interface OnboardCommandRunner { @@ -40,6 +42,7 @@ export interface OnboardCommandRunner { options?: { cwd?: string; env?: NodeJS.ProcessEnv; + timeoutMs?: number; }, ): Promise; } @@ -156,6 +159,7 @@ const NODE_COMMAND_RUNNER: OnboardCommandRunner = { cwd: options?.cwd, encoding: "utf8", env: options?.env, + timeout: options?.timeoutMs, windowsHide: true, }); @@ -166,12 +170,14 @@ const NODE_COMMAND_RUNNER: OnboardCommandRunner = { stdout: result.stdout, }; } catch (error) { - if (isExecFileExitError(error)) { + if (isExecFileError(error)) { return { - exitCode: error.code ?? 1, + ...(typeof error.code === "string" ? { errorCode: error.code } : {}), + exitCode: typeof error.code === "number" ? error.code : 1, signal: error.signal ?? null, stderr: toText(error.stderr), stdout: toText(error.stdout), + ...(error.killed === true ? { timedOut: true } : {}), }; } @@ -226,14 +232,19 @@ export function createNodeOnboardDependencies( }; } -function isExecFileExitError(error: unknown): error is Error & { - code?: number; +function isExecFileError(error: unknown): error is Error & { + code?: number | string | null; + killed?: boolean; signal?: NodeJS.Signals | null; stderr?: string | Buffer; stdout?: string | Buffer; } { return ( - error instanceof Error && "code" in error && typeof error.code === "number" + error instanceof Error && + ("code" in error || + "signal" in error || + "stderr" in error || + "stdout" in error) ); } diff --git a/src/runtime/preconditions.ts b/src/runtime/preconditions.ts index 25923b3..905a1cd 100644 --- a/src/runtime/preconditions.ts +++ b/src/runtime/preconditions.ts @@ -9,7 +9,7 @@ import { type OnboardPreflightSuccessResult, type PreflightReport, } from "../domain/runtime.js"; -import { ensureHermesAvailable } from "../hermes/cli.js"; +import { ensureHermesAvailable, type HermesCliFailure } from "../hermes/cli.js"; import { resolveHermesContext } from "../hermes/path-resolution.js"; import type { OnboardDependencies } from "./dependencies.js"; @@ -55,12 +55,7 @@ export async function runPreflightChecks( const hermesPresence = await ensureHermesAvailable(dependencies); if (!hermesPresence.ok) { - return createOnboardFailure("hermes_not_found", { - guidance: - "Install `hermes` so it is available on PATH, then rerun the helper.", - message: - "Hermes Agent was not found on PATH, so onboarding cannot resolve the active config context yet.", - }); + return createHermesAvailabilityFailure(hermesPresence.failure); } const hermesVersionCheck = validateHermesVersion(hermesPresence.stdout); @@ -137,6 +132,69 @@ export async function runPreflightChecks( }; } +function createHermesAvailabilityFailure( + failure: HermesCliFailure, +): OnboardFailure { + const hermesArgs = failure.args.join(" "); + + if (failure.kind === "not_found") { + return createOnboardFailure("hermes_not_found", { + details: { + hermesArgs, + hermesFailureKind: failure.kind, + }, + guidance: + "Install `hermes` so it is available on PATH, then rerun the helper.", + message: + "Hermes Agent was not found on PATH, so onboarding cannot resolve the active config context yet.", + }); + } + + if (failure.kind === "timed_out") { + return createOnboardFailure("hermes_unavailable", { + details: { + hermesArgs, + hermesFailureKind: failure.kind, + timeoutMs: failure.timeoutMs, + }, + guidance: + "Check that `hermes --version` completes in this shell, repair or reinstall Hermes Agent if needed, then rerun the helper.", + message: `Hermes Agent did not respond to \`hermes ${hermesArgs}\` within ${failure.timeoutMs} ms, so onboarding cannot safely continue.`, + }); + } + + if (failure.kind === "nonzero_exit") { + return createOnboardFailure("hermes_unavailable", { + details: { + exitCode: failure.exitCode, + hermesArgs, + hermesFailureKind: failure.kind, + ...(failure.errorCode === undefined + ? {} + : { + errorCode: failure.errorCode, + }), + }, + guidance: + "Check that `hermes --version` completes successfully in this shell, repair or reinstall Hermes Agent if needed, then rerun the helper.", + message: + "Hermes Agent returned a non-zero status for `hermes --version`, so onboarding cannot resolve the active config context yet.", + }); + } + + return createOnboardFailure("hermes_unavailable", { + details: { + hermesArgs, + hermesFailureKind: failure.kind, + reason: failure.reason, + }, + guidance: + "Check that `hermes --version` prints a usable version in this shell, repair or reinstall Hermes Agent if needed, then rerun the helper.", + message: + "Hermes Agent did not return usable output for `hermes --version`, so onboarding cannot resolve the active config context yet.", + }); +} + type HermesVersionCheckResult = | { ok: true; diff --git a/test/cli.test.ts b/test/cli.test.ts index dce394e..3fddc75 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -189,6 +189,42 @@ test("missing Hermes on PATH aborts before the command can continue", async () = assert.equal(stderr.contents, ""); }); +test("Hermes version command signal renders a domain failure", async () => { + const harness = await createHermesIntegrationHarness({ + fixture: "clean-home", + }); + const stdout = createBufferWriter(); + const stderr = createBufferWriter(); + + try { + await harness.installFakeHermesOnPath({ + versionSignal: "SIGTERM", + }); + + const result = await run([], { + dependencies: harness.createDependencies({ + runtime: { + osRelease: "6.8.0", + platform: "linux", + stdinIsTTY: true, + stdoutIsTTY: true, + }, + }), + stderr, + stdout, + }); + + assert.equal(result.exitCode, 1); + assert.equal(result.result?.status, "failure"); + assert.equal(result.result?.code, "hermes_unavailable"); + assert.match(stdout.contents, /Hermes Agent returned a non-zero status/i); + assert.doesNotMatch(stdout.contents, /Command failed: hermes --version/i); + assert.equal(stderr.contents, ""); + } finally { + await harness.cleanup(); + } +}); + test("missing TTY aborts before the helper calls Hermes", async () => { const harness = await createHermesIntegrationHarness({ fixture: "clean-home", diff --git a/test/helpers/fake-hermes.mjs b/test/helpers/fake-hermes.mjs index 5c4efe8..382d5ac 100644 --- a/test/helpers/fake-hermes.mjs +++ b/test/helpers/fake-hermes.mjs @@ -7,6 +7,14 @@ const env = process.env; recordInvocation(args); if (args.length === 1 && args[0] === "--version") { + const versionSignal = normalizeString( + env.GONKAGATE_FAKE_HERMES_VERSION_SIGNAL, + ); + + if (versionSignal !== undefined) { + process.kill(process.pid, versionSignal); + } + respond({ exitCode: Number(env.GONKAGATE_FAKE_HERMES_VERSION_EXIT_CODE ?? "0"), stderr: env.GONKAGATE_FAKE_HERMES_VERSION_STDERR ?? "", diff --git a/test/helpers/harness.ts b/test/helpers/harness.ts index 51e9b8b..963e382 100644 --- a/test/helpers/harness.ts +++ b/test/helpers/harness.ts @@ -38,6 +38,7 @@ export interface FakeHermesOptions { profilePathOverrides?: Record; versionExitCode?: number; versionOutput?: string; + versionSignal?: NodeJS.Signals; versionStderr?: string; } @@ -294,6 +295,7 @@ import ${JSON.stringify(fakeHermesFixturePath)}; ), GONKAGATE_FAKE_HERMES_VERSION_OUTPUT: options.versionOutput ?? "hermes-agent 0.14.0", + GONKAGATE_FAKE_HERMES_VERSION_SIGNAL: options.versionSignal ?? "", GONKAGATE_FAKE_HERMES_VERSION_STDERR: options.versionStderr ?? "", ...(options.configPathOutput === undefined ? {} @@ -480,6 +482,7 @@ async function runExecFile( options?: { cwd?: string; env?: NodeJS.ProcessEnv; + timeoutMs?: number; }, ) { try { @@ -487,6 +490,7 @@ async function runExecFile( cwd: options?.cwd, encoding: "utf8", env: options?.env, + timeout: options?.timeoutMs, windowsHide: true, }); @@ -497,12 +501,14 @@ async function runExecFile( stdout: result.stdout, }; } catch (error) { - if (isExecFileExitError(error)) { + if (isExecFileError(error)) { return { - exitCode: error.code ?? 1, + ...(typeof error.code === "string" ? { errorCode: error.code } : {}), + exitCode: typeof error.code === "number" ? error.code : 1, signal: error.signal ?? null, stderr: toText(error.stderr), stdout: toText(error.stdout), + ...(error.killed === true ? { timedOut: true } : {}), }; } @@ -510,14 +516,19 @@ async function runExecFile( } } -function isExecFileExitError(error: unknown): error is Error & { - code?: number; +function isExecFileError(error: unknown): error is Error & { + code?: number | string | null; + killed?: boolean; signal?: NodeJS.Signals | null; stderr?: string | Buffer; stdout?: string | Buffer; } { return ( - error instanceof Error && "code" in error && typeof error.code === "number" + error instanceof Error && + ("code" in error || + "signal" in error || + "stderr" in error || + "stdout" in error) ); } diff --git a/test/preconditions.test.ts b/test/preconditions.test.ts index 63cc23a..f2410f2 100644 --- a/test/preconditions.test.ts +++ b/test/preconditions.test.ts @@ -2,6 +2,7 @@ import { access as accessPath } from "node:fs/promises"; import assert from "node:assert/strict"; import { resolve } from "node:path"; import test from "node:test"; +import { HERMES_COMMAND_TIMEOUT_MS } from "../src/hermes/cli.js"; import { runPreflightChecks } from "../src/runtime/preconditions.js"; import { createHermesIntegrationHarness } from "./helpers/harness.js"; @@ -157,6 +158,96 @@ test("unparseable Hermes version output aborts before path resolution", async () } }); +test("timed out Hermes version command aborts before path resolution", async () => { + const harness = await createHermesIntegrationHarness({ + fixture: "clean-home", + }); + const hermesInvocations: string[][] = []; + + try { + const result = await runPreflightChecks( + {}, + harness.createDependencies({ + commands: { + async execFile(file, args, options) { + assert.equal(file, "hermes"); + assert.deepEqual(args, ["--version"]); + assert.equal(options?.timeoutMs, HERMES_COMMAND_TIMEOUT_MS); + hermesInvocations.push([...args]); + + return { + exitCode: 1, + signal: "SIGTERM", + stderr: "", + stdout: "", + timedOut: true, + }; + }, + }, + runtime: { + osRelease: "6.8.0", + platform: "linux", + stdinIsTTY: true, + stdoutIsTTY: true, + }, + }), + ); + + assert.equal(result.status, "failure"); + + if (result.status !== "failure") { + return; + } + + assert.equal(result.code, "hermes_unavailable"); + assert.equal(result.details?.hermesFailureKind, "timed_out"); + assert.equal(result.details?.timeoutMs, HERMES_COMMAND_TIMEOUT_MS); + assert.match(result.message, /did not respond/i); + assert.deepEqual(hermesInvocations, [["--version"]]); + } finally { + await harness.cleanup(); + } +}); + +test("signalled Hermes version command aborts before path resolution", async () => { + const harness = await createHermesIntegrationHarness({ + fixture: "clean-home", + }); + + try { + await harness.installFakeHermesOnPath({ + versionSignal: "SIGTERM", + }); + + const result = await runPreflightChecks( + {}, + harness.createDependencies({ + runtime: { + osRelease: "6.8.0", + platform: "linux", + stdinIsTTY: true, + stdoutIsTTY: true, + }, + }), + ); + + assert.equal(result.status, "failure"); + + if (result.status !== "failure") { + return; + } + + assert.equal(result.code, "hermes_unavailable"); + assert.equal(result.details?.hermesFailureKind, "nonzero_exit"); + assert.match(result.message, /non-zero status/i); + assert.deepEqual(await harness.readFakeHermesInvocations(), [ + ["--version"], + ]); + } finally { + await harness.cleanup(); + } +}); + test("unsupported win32 aborts before Hermes is invoked", async () => { const harness = await createHermesIntegrationHarness({ fixture: "clean-home",