mirror of
https://github.com/morluto/rea
synced 2026-10-25 22:08:11 -04:00
fix(ghidra): skip the Java probe without an installation (#6282)
Setup and doctor inspect every provider. With no GHIDRA_INSTALL_DIR and no Ghidra launcher on PATH, the Ghidra inspection still ran the host `java` from PATH. Its result was discarded because the configuration check already rejects Ghidra, but its owned-process cleanup could still fail closed. In CI, that cleanup reported cleanup_incomplete for an unrelated live process whose /proc/<pid>/environ was unreadable, so `rea setup --client hermes` exited 1. Run Java only for a selected installation, matching the existing rule for a relative JAVA_HOME. Hermes setup tests now report the CLI JSON when an exit code assertion fails. Fixes #6200 Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -1040,6 +1040,51 @@ describe("Ghidra configuration paths", () => {
|
||||
},
|
||||
);
|
||||
|
||||
it.each([
|
||||
["linux", undefined],
|
||||
["linux", "/opt/jdk-21"],
|
||||
["darwin", undefined],
|
||||
["win32", "C:\\Java\\jdk-21"],
|
||||
] as const)(
|
||||
"does not run Java without a Ghidra installation (%s %o)",
|
||||
async (platform, javaHome) => {
|
||||
const probed: string[] = [];
|
||||
const result = await inspectGhidraInstallation(
|
||||
{
|
||||
resourceOwner: inspectionResources.owner,
|
||||
environment: { PATH: "/usr/bin" },
|
||||
...(javaHome === undefined ? {} : { javaHome }),
|
||||
platform,
|
||||
architecture: "x64",
|
||||
},
|
||||
host({
|
||||
discoverInstallDirOnPath: () => undefined,
|
||||
probeJava: (command) => {
|
||||
probed.push(command);
|
||||
return javaProbeOk<GhidraJavaObservation>(JAVA);
|
||||
},
|
||||
}),
|
||||
);
|
||||
expect(probed).toEqual([]);
|
||||
expect(result).toMatchObject({
|
||||
status: "unavailable",
|
||||
installDir: null,
|
||||
javaVersion: null,
|
||||
rejection: { code: "not_configured" },
|
||||
});
|
||||
expect(projectGhidraDoctorInspection(result)).toMatchObject({
|
||||
available: false,
|
||||
registrationEnvironment: {},
|
||||
checks: [
|
||||
expect.objectContaining({
|
||||
name: "configuration",
|
||||
code: "not_configured",
|
||||
}),
|
||||
],
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it("judges absolute paths by the inspected platform", async () => {
|
||||
const result = await inspectGhidraInstallation(
|
||||
{
|
||||
|
||||
@@ -530,9 +530,12 @@ const installationCoordinates = async (
|
||||
.find((candidate) => host.executable(candidate)) ?? "java"
|
||||
);
|
||||
})();
|
||||
// Relative JAVA_HOME is refused by configuration validation without execution.
|
||||
// Java is a prerequisite of a selected installation. Without one, or with a
|
||||
// relative JAVA_HOME, configuration validation refuses Ghidra before Java can
|
||||
// matter, so callers such as setup and doctor must not execute a host JVM.
|
||||
const javaProbe =
|
||||
options.javaHome === undefined || path.isAbsolute(options.javaHome)
|
||||
installDir !== null &&
|
||||
(options.javaHome === undefined || path.isAbsolute(options.javaHome))
|
||||
? await host.probeJava(
|
||||
javaCommand,
|
||||
ghidraJavaEnvironment(
|
||||
|
||||
@@ -133,7 +133,7 @@ cliTest(
|
||||
"--dry-run",
|
||||
"--json",
|
||||
]);
|
||||
expect(planned.exitCode).toBe(0);
|
||||
expect(planned.exitCode, planned.stdout).toBe(0);
|
||||
expect(planned.json).toMatchObject({
|
||||
status: "planned",
|
||||
plannedActions: expect.arrayContaining([
|
||||
@@ -152,7 +152,7 @@ cliTest(
|
||||
"--yes",
|
||||
"--json",
|
||||
]);
|
||||
expect(applied.exitCode).toBe(0);
|
||||
expect(applied.exitCode, applied.stdout).toBe(0);
|
||||
expect(applied.json).toMatchObject({ status: "ready" });
|
||||
expect(parse(await readFile(config, "utf8"))).toMatchObject({
|
||||
model: "custom",
|
||||
@@ -175,7 +175,7 @@ cliTest(
|
||||
"--skill",
|
||||
"--json",
|
||||
]);
|
||||
expect(doctor.exitCode).toBe(0);
|
||||
expect(doctor.exitCode, doctor.stdout).toBe(0);
|
||||
expect(doctor.json).toMatchObject({
|
||||
healthy: true,
|
||||
identity: { skill: { state: "aligned" } },
|
||||
@@ -189,7 +189,7 @@ cliTest(
|
||||
]);
|
||||
expect(repeat.json).toMatchObject({ status: "ready", appliedActions: [] });
|
||||
const uninstall = await run(["uninstall", "--json"]);
|
||||
expect(uninstall.exitCode).toBe(0);
|
||||
expect(uninstall.exitCode, uninstall.stdout).toBe(0);
|
||||
expect(parse(await readFile(config, "utf8"))).toEqual(parse(original));
|
||||
await expect(access(skill)).rejects.toMatchObject({ code: "ENOENT" });
|
||||
},
|
||||
@@ -243,7 +243,7 @@ for (const selection of ["default", "custom", "suffix", "explicit"] as const) {
|
||||
"--dry-run",
|
||||
"--json",
|
||||
]);
|
||||
expect(plan.exitCode).toBe(0);
|
||||
expect(plan.exitCode, plan.stdout).toBe(0);
|
||||
expect(plan.json).toMatchObject({
|
||||
status: "planned",
|
||||
plannedActions: expect.arrayContaining([
|
||||
@@ -253,10 +253,14 @@ for (const selection of ["default", "custom", "suffix", "explicit"] as const) {
|
||||
}),
|
||||
]),
|
||||
});
|
||||
expect(
|
||||
(await run(["setup", "--client", "hermes", "--yes", "--json"]))
|
||||
.exitCode,
|
||||
).toBe(0);
|
||||
const applied = await run([
|
||||
"setup",
|
||||
"--client",
|
||||
"hermes",
|
||||
"--yes",
|
||||
"--json",
|
||||
]);
|
||||
expect(applied.exitCode, applied.stdout).toBe(0);
|
||||
expect(parse(await readFile(config, "utf8"))).toMatchObject({
|
||||
mcp_servers: {
|
||||
rea: { enabled: true },
|
||||
@@ -281,7 +285,7 @@ for (const selection of ["default", "custom", "suffix", "explicit"] as const) {
|
||||
"--skill",
|
||||
"--json",
|
||||
]);
|
||||
expect(doctor.exitCode).toBe(0);
|
||||
expect(doctor.exitCode, doctor.stdout).toBe(0);
|
||||
expect(doctor.json).toMatchObject({
|
||||
healthy: true,
|
||||
identity: {
|
||||
@@ -382,7 +386,7 @@ for (const name of ["missing", "deleted", "ghost", "../escape", "root"]) {
|
||||
"--yes",
|
||||
"--json",
|
||||
]);
|
||||
expect(unrelated.exitCode).toBe(0);
|
||||
expect(unrelated.exitCode, unrelated.stdout).toBe(0);
|
||||
},
|
||||
);
|
||||
}
|
||||
@@ -418,7 +422,7 @@ cliTest.skipIf(process.platform === "win32")(
|
||||
"--dry-run",
|
||||
"--json",
|
||||
]);
|
||||
expect(planned.exitCode).toBe(0);
|
||||
expect(planned.exitCode, planned.stdout).toBe(0);
|
||||
expect(planned.json).toMatchObject({
|
||||
status: "planned",
|
||||
plannedActions: expect.arrayContaining([
|
||||
|
||||
@@ -61,3 +61,63 @@ cliTest.skipIf(process.platform === "win32").for(["blocked", "ready"] as const)(
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
cliTest.skipIf(process.platform === "win32")(
|
||||
"setup and doctor do not execute host Java while Ghidra is unconfigured",
|
||||
async ({ cli }) => {
|
||||
const root = await createTestTempDirectory("rea-setup-unconfigured-java-");
|
||||
const home = join(root, "home");
|
||||
const bin = join(root, "bin");
|
||||
const marker = join(root, "java-executed");
|
||||
await mkdir(home);
|
||||
await mkdir(bin);
|
||||
// A host JVM is unrelated to an unconfigured Ghidra provider; running it
|
||||
// exposes setup to that process's cleanup outcome.
|
||||
await writeFile(
|
||||
join(bin, "java"),
|
||||
`#!/bin/sh\nprintf '%s\\n' "$*" >> '${marker}'\nexit 1\n`,
|
||||
{ mode: 0o755 },
|
||||
);
|
||||
const environment = {
|
||||
HOME: home,
|
||||
USERPROFILE: home,
|
||||
// The recording java shadows any host JVM in the system directories.
|
||||
PATH: `${bin}:/usr/bin:/bin`,
|
||||
HOPPER_LAUNCHER_PATH: join(root, "missing-hopper"),
|
||||
};
|
||||
const setup = await cli.run({
|
||||
arguments: ["setup", "--client", "claude_code", "--dry-run", "--json"],
|
||||
environment,
|
||||
timeoutMs: 20_000,
|
||||
});
|
||||
expect(setup.exitCode, setup.stdout).toBe(0);
|
||||
expect(setup.json).toMatchObject({
|
||||
status: "planned",
|
||||
doctor: {
|
||||
providerInspections: expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
id: "ghidra",
|
||||
configured: false,
|
||||
checks: [
|
||||
expect.objectContaining({
|
||||
name: "configuration",
|
||||
code: "not_configured",
|
||||
}),
|
||||
],
|
||||
}),
|
||||
]),
|
||||
},
|
||||
});
|
||||
const doctor = await cli.run({
|
||||
arguments: ["doctor", "--json"],
|
||||
environment,
|
||||
timeoutMs: 20_000,
|
||||
});
|
||||
expect(doctor.json).toMatchObject({
|
||||
providerInspections: expect.arrayContaining([
|
||||
expect.objectContaining({ id: "ghidra", configured: false }),
|
||||
]),
|
||||
});
|
||||
await expect(access(marker)).rejects.toMatchObject({ code: "ENOENT" });
|
||||
},
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user