Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/domain/runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export type OnboardFailureCode =
| "missing_tty"
| "unsupported_platform"
| "hermes_not_found"
| "hermes_unavailable"
| "unsupported_hermes_version"
| "managed_install"
| "write_blocked"
Expand Down Expand Up @@ -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",
Expand Down
40 changes: 39 additions & 1 deletion src/hermes/cli.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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,
Expand Down
21 changes: 16 additions & 5 deletions src/runtime/dependencies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -40,6 +42,7 @@ export interface OnboardCommandRunner {
options?: {
cwd?: string;
env?: NodeJS.ProcessEnv;
timeoutMs?: number;
},
): Promise<OnboardCommandResult>;
}
Expand Down Expand Up @@ -156,6 +159,7 @@ const NODE_COMMAND_RUNNER: OnboardCommandRunner = {
cwd: options?.cwd,
encoding: "utf8",
env: options?.env,
timeout: options?.timeoutMs,
windowsHide: true,
});

Expand All @@ -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 } : {}),
};
}

Expand Down Expand Up @@ -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)
);
}

Expand Down
72 changes: 65 additions & 7 deletions src/runtime/preconditions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
36 changes: 36 additions & 0 deletions test/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
8 changes: 8 additions & 0 deletions test/helpers/fake-hermes.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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 ?? "",
Expand Down
21 changes: 16 additions & 5 deletions test/helpers/harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export interface FakeHermesOptions {
profilePathOverrides?: Record<string, FakeHermesProfileOverride>;
versionExitCode?: number;
versionOutput?: string;
versionSignal?: NodeJS.Signals;
versionStderr?: string;
}

Expand Down Expand Up @@ -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
? {}
Expand Down Expand Up @@ -480,13 +482,15 @@ async function runExecFile(
options?: {
cwd?: string;
env?: NodeJS.ProcessEnv;
timeoutMs?: number;
},
) {
try {
const result = await execFileAsync(file, [...args], {
cwd: options?.cwd,
encoding: "utf8",
env: options?.env,
timeout: options?.timeoutMs,
windowsHide: true,
});

Expand All @@ -497,27 +501,34 @@ 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 } : {}),
};
}

throw error;
}
}

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)
);
}

Expand Down
Loading
Loading