diff --git a/docs/installation.md b/docs/installation.md index 44ae73c7bc..ab85fe0b16 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -736,7 +736,9 @@ and skips directories. On Linux, it prefers executable `/opt/hopper/bin/Hopper`, then executable `~/.local/share/rea/hopper/bin/Hopper`, followed by `/opt/hopper-v4/bin/hopper` and `/usr/bin/hopper`; if none exists, the former remains the diagnostic -fallback. `HOPPER_LAUNCHER_PATH` overrides those choices: +fallback. `rea doctor` checks only the current platform's candidates, in the +same order; other platforms have no automatic candidates. +`HOPPER_LAUNCHER_PATH` overrides those choices: ```bash export HOPPER_LAUNCHER_PATH=/absolute/path/to/Hopper diff --git a/src/application/Doctor.test.ts b/src/application/Doctor.test.ts index 0b29bbdc42..1372a632ed 100644 --- a/src/application/Doctor.test.ts +++ b/src/application/Doctor.test.ts @@ -1,11 +1,16 @@ +import { mkdir } from "node:fs/promises"; +import { join, posix } from "node:path"; import { describe, expect, it } from "vitest"; +import { defaultHopperLauncherPath } from "../config/parseConfig.js"; import { SUPPORTED_NODE_VERSION_PROSE } from "../domain/runtimeVersion.js"; import type { ClientRegistrationStatus } from "./ClientRegistrationStatus.js"; import { createDoctorHostFixture as host } from "./Doctor.fixture.js"; import { runDoctor, systemDoctorHost, type DoctorHost } from "./Doctor.js"; import { err } from "../domain/result.js"; import { HopperReleaseError } from "./HopperReleaseFeed.js"; +import { MAC_HOPPER_SYSTEM_LAUNCHER } from "./MacHopperPaths.js"; +import { createTestTempDirectory } from "../../tests/fixtures/temporaryDirectory.js"; describe("doctor Hopper startup timeout", () => { const timeoutHost = ( @@ -1042,18 +1047,87 @@ describe("doctor installed Hopper discovery", () => { ); expect(result.hopperPath).toBe(path); }); +}); - it("detects a manually installed Hopper application", async () => { - const path = "/Applications/Hopper v6.app/Contents/MacOS/hopper"; - const result = await runDoctor( - undefined, - host({ - executable: (candidate) => Promise.resolve(candidate === path), - manualHopperPaths: () => Promise.resolve([path]), - }), - ); - expect(result.hopperPath).toBe(path); +describe("doctor Hopper discovery matches runtime selection", () => { + // Model launchers with runtime's POSIX joins; filesystem work stays native. + const launchers = (home: string) => ({ + macSystem: MAC_HOPPER_SYSTEM_LAUNCHER, + renamed: posix.join( + home, + "Applications/Hopper 5.app/Contents/MacOS/hopper", + ), + linuxSystem: "/opt/hopper-v4/bin/hopper", }); + type Launcher = keyof ReturnType; + + it.each<[NodeJS.Platform, readonly Launcher[], Launcher | undefined]>([ + ["darwin", ["linuxSystem"], undefined], + ["darwin", ["linuxSystem", "renamed"], "renamed"], + ["darwin", ["linuxSystem", "macSystem"], "macSystem"], + ["darwin", ["renamed", "macSystem"], "macSystem"], + ["linux", ["macSystem", "renamed"], undefined], + ["linux", ["macSystem", "linuxSystem"], "linuxSystem"], + ["win32", ["macSystem", "linuxSystem", "renamed"], undefined], + ])( + "on %s with %j executable, selects %s", + async (platform, executable, expected) => { + // Runtime enumerates renamed bundles from disk; doctor reads its host scan. + const home = await createTestTempDirectory("rea-doctor-hopper-"); + await mkdir(join(home, "Applications/Hopper 5.app"), { recursive: true }); + const paths = launchers(home); + const executables = new Set(executable.map((name) => paths[name])); + const probed: string[] = []; + let renamedScans = 0; + let brewProbes = 0; + const result = await runDoctor( + undefined, + host({ + platform, + homeDirectory: home, + executable: (path) => { + probed.push(path); + return Promise.resolve(executables.has(path)); + }, + manualHopperPaths: () => { + renamedScans += 1; + return Promise.resolve([paths.renamed]); + }, + brewHopperPath: () => { + brewProbes += 1; + return Promise.resolve(undefined); + }, + }), + ); + const runtime = defaultHopperLauncherPath(platform, home, (path) => + executables.has(path), + ); + + expect(result.hopperPath).toBe( + expected === undefined ? undefined : paths[expected], + ); + // Hopper has no automatic launcher on other platforms; runtime's fixed + // fallback there is never probed. + if (platform === "win32") expect(probed).toEqual([]); + else + expect(result.hopperPath).toBe( + executables.has(runtime) ? runtime : undefined, + ); + expect(renamedScans).toBe( + platform === "darwin" && expected !== "macSystem" ? 1 : 0, + ); + expect(brewProbes).toBe( + platform === "darwin" && expected === undefined ? 1 : 0, + ); + if (expected === undefined) + expect( + result.checks.find(({ name }) => name === "hopper"), + ).toMatchObject({ + ok: false, + classification: "missing_analysis_engine", + }); + }, + ); }); describe("doctor target admission on Linux", () => { diff --git a/src/application/Doctor.ts b/src/application/Doctor.ts index 735258b0d5..6b828c2b0a 100644 --- a/src/application/Doctor.ts +++ b/src/application/Doctor.ts @@ -8,7 +8,7 @@ import { ArtifactReaderFailure } from "../artifacts/ArtifactReader.js"; import { AnalysisCleanupLease } from "../artifacts/AnalysisCleanupLease.js"; import { STARTUP_OWNER_CLOSED } from "../process/SharedStartupAttempt.js"; import { constants } from "node:fs"; -import { access, readdir, stat } from "node:fs/promises"; +import { access, stat } from "node:fs/promises"; import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; import type { RetainedResourceScope } from "./AnalysisProvider.js"; import { nativeAnalysisConfigurationError } from "../config/parseConfig.js"; @@ -27,10 +27,7 @@ import { type JavaScriptRuntime, } from "../domain/runtimeVersion.js"; import { installedCaskAppdir, probeHomebrew } from "./homebrew.js"; -import { - macHopperApplicationRoots, - macHopperRenamedLauncherCandidates, -} from "./MacHopperPaths.js"; +import { readMacHopperRenamedLauncherCandidates } from "./MacHopperPaths.js"; import { inspectLinuxHopperLauncher, linuxSharedLibrariesAvailable, @@ -128,7 +125,9 @@ export interface DoctorHost { path: string, ): Promise>; linuxDemoRuntimeCheck(): Promise; + /** Homebrew cask launcher; consulted only where macOS bundles are probed. */ brewHopperPath(): Promise; + /** Renamed `Hopper*.app` launchers; consulted only where macOS bundles are probed. */ manualHopperPaths(): Promise; providerInspections?(): Promise; /** Resolve a parsed upstream request budget from the effective client environment. */ @@ -645,7 +644,8 @@ export const systemDoctorHost = ( : join(appdir, "Hopper Disassembler.app/Contents/MacOS/hopper"); }); }, - manualHopperPaths: () => manualHopperPaths(homeDirectory), + manualHopperPaths: () => + readMacHopperRenamedLauncherCandidates(homeDirectory), ...(options.providerInspections === undefined ? {} : { @@ -777,25 +777,6 @@ const uncomposedLinuxDemoRuntimeCheck = (): Promise => remediation: "Run rea doctor through the production CLI adapter.", }); -const manualHopperPaths = async (home: string): Promise => { - const roots = macHopperApplicationRoots(home); - const bundleNamesByRoot = new Map(); - for (const root of roots) { - try { - bundleNamesByRoot.set( - root, - (await readdir(root, { withFileTypes: true })) - .filter((entry) => entry.isDirectory()) - .map(({ name }) => name), - ); - } catch (cause: unknown) { - // A missing or unreadable optional application directory is not fatal. - void cause; - } - } - return macHopperRenamedLauncherCandidates(home, bundleNamesByRoot); -}; - const uniqueLines = (value: string): string[] => [ ...new Set( value diff --git a/src/application/DoctorDiagnostics.ts b/src/application/DoctorDiagnostics.ts index 46236bd778..270fcc00c9 100644 --- a/src/application/DoctorDiagnostics.ts +++ b/src/application/DoctorDiagnostics.ts @@ -3,16 +3,8 @@ import { supportsNodeVersion, } from "../domain/runtimeVersion.js"; import { providerStartupTimeoutSchema } from "../config/providerStartupTimeout.js"; -import { - LINUX_HOPPER_SETUP_HOSTS, - linuxHopperLauncherCandidates, - systemLinuxHopperLauncherCandidates, - supportsReaHost, -} from "./LinuxHopper.js"; -import { - MAC_HOPPER_SYSTEM_LAUNCHER, - macHopperCanonicalLauncherCandidates, -} from "./MacHopperPaths.js"; +import { hopperLauncherDiscovery } from "./HopperLauncherDiscovery.js"; +import { LINUX_HOPPER_SETUP_HOSTS, supportsReaHost } from "./LinuxHopper.js"; import type { DoctorCheck, DoctorHost, @@ -154,26 +146,14 @@ const hostCheck = async (host: DoctorHost): Promise => { }; const findHopper = async (host: DoctorHost): Promise => { - const candidates = - host.configuredHopperPath === undefined - ? [ - ...(host.platform === "darwin" - ? macHopperCanonicalLauncherCandidates(host.homeDirectory) - : [MAC_HOPPER_SYSTEM_LAUNCHER]), - ...(host.platform === "linux" - ? linuxHopperLauncherCandidates(host.homeDirectory) - : [...systemLinuxHopperLauncherCandidates]), - ...(host.platform === "darwin" - ? [] - : macHopperCanonicalLauncherCandidates(host.homeDirectory).slice( - 1, - )), - ...(await host.manualHopperPaths()), - ] - : [host.configuredHopperPath]; - const hopperPath = await firstExecutable(host, candidates); - if (hopperPath !== undefined || host.configuredHopperPath !== undefined) - return hopperPath; + if (host.configuredHopperPath !== undefined) + return firstExecutable(host, [host.configuredHopperPath]); + const discovery = hopperLauncherDiscovery(host.platform, host.homeDirectory); + const fixed = await firstExecutable(host, discovery.candidates); + if (fixed !== undefined || !discovery.probesMacApplicationBundles) + return fixed; + const renamed = await firstExecutable(host, await host.manualHopperPaths()); + if (renamed !== undefined) return renamed; const brewCandidate = await host.brewHopperPath(); return firstExecutable( host, diff --git a/src/application/HopperLauncherDiscovery.ts b/src/application/HopperLauncherDiscovery.ts new file mode 100644 index 0000000000..2d9ba3cd34 --- /dev/null +++ b/src/application/HopperLauncherDiscovery.ts @@ -0,0 +1,49 @@ +import { + linuxHopperLauncherCandidates, + REA_MANAGED_LINUX_HOPPER_LAUNCHER, +} from "./LinuxHopper.js"; +import { + MAC_HOPPER_SYSTEM_LAUNCHER, + macHopperCanonicalLauncherCandidates, +} from "./MacHopperPaths.js"; + +/** + * Where automatic Hopper discovery looks on one platform. Runtime configuration + * and doctor both read it, so doctor checks the same candidates, in the same + * order, as CLI/MCP startup. + */ +export interface HopperLauncherDiscovery { + /** Fixed launcher paths, probed in order. */ + readonly candidates: readonly string[]; + /** + * Whether renamed `Hopper*.app` bundles are probed after the fixed paths. + * Doctor also reports the Homebrew cask application only when this is set. + */ + readonly probesMacApplicationBundles: boolean; + /** Launcher runtime configures when no candidate is executable. */ + readonly fallback: string; +} + +/** Select the automatic Hopper launcher candidates for one platform. */ +export const hopperLauncherDiscovery = ( + platform: NodeJS.Platform, + homeDirectory: string, +): HopperLauncherDiscovery => { + if (platform === "darwin") + return { + candidates: macHopperCanonicalLauncherCandidates(homeDirectory), + probesMacApplicationBundles: true, + fallback: MAC_HOPPER_SYSTEM_LAUNCHER, + }; + if (platform === "linux") + return { + candidates: linuxHopperLauncherCandidates(homeDirectory), + probesMacApplicationBundles: false, + fallback: REA_MANAGED_LINUX_HOPPER_LAUNCHER, + }; + return { + candidates: [], + probesMacApplicationBundles: false, + fallback: MAC_HOPPER_SYSTEM_LAUNCHER, + }; +}; diff --git a/src/application/LinuxHopper.ts b/src/application/LinuxHopper.ts index c2175f8c2c..b9525d9235 100644 --- a/src/application/LinuxHopper.ts +++ b/src/application/LinuxHopper.ts @@ -463,7 +463,7 @@ export const linuxHopperLauncherPath = (home: string): string => export const REA_MANAGED_LINUX_HOPPER_LAUNCHER = "/opt/hopper/bin/Hopper"; /** Other system-wide Hopper paths used by package installations. */ -export const systemLinuxHopperLauncherCandidates: readonly string[] = [ +const systemLinuxHopperLauncherCandidates: readonly string[] = [ "/opt/hopper-v4/bin/hopper", "/usr/bin/hopper", ]; diff --git a/src/application/MacHopperPaths.test.ts b/src/application/MacHopperPaths.test.ts index 6cab55309a..eb33acabc6 100644 --- a/src/application/MacHopperPaths.test.ts +++ b/src/application/MacHopperPaths.test.ts @@ -1,12 +1,13 @@ -import { mkdtemp, mkdir, rm } from "node:fs/promises"; +import { mkdtemp, mkdir, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, posix, relative } from "node:path"; import { describe, expect, it } from "vitest"; import { + localMacHopperRenamedLauncherCandidates, macHopperCanonicalLauncherCandidates, - macHopperLauncherCandidates, macHopperRenamedLauncherCandidates, + readMacHopperRenamedLauncherCandidates, } from "./MacHopperPaths.js"; describe("macOS Hopper launcher paths", () => { @@ -35,13 +36,7 @@ describe("macOS Hopper launcher paths", () => { ]); }); - it("always includes the installer's canonical user destination", () => { - expect(macHopperLauncherCandidates(home)).toContain( - "/fixture/home/Applications/Hopper Disassembler.app/Contents/MacOS/hopper", - ); - }); - - it("finds renamed user bundles through the runtime filesystem scan", async () => { + it("finds the same renamed user bundles through runtime and doctor scans", async () => { // Keep native ownership for all I/O, including UNC temporary directories. // On Windows, a checkout-relative home models POSIX separators without // discarding the drive/share or normalizing a UNC prefix through posix.join. @@ -53,14 +48,22 @@ describe("macOS Hopper launcher paths", () => { process.platform === "win32" ? relative(process.cwd(), temporary).replaceAll("\\", "/") : temporary; - const appDirectory = join(temporary, "Applications", "Hopper Preview.app"); + const applications = join(temporary, "Applications"); try { - await mkdir(appDirectory, { recursive: true }); - expect([...macHopperLauncherCandidates(home)]).toContain( + await mkdir(join(applications, "Hopper Preview.app"), { + recursive: true, + }); + await mkdir(join(applications, "Hopper Disassembler.app")); + await writeFile(join(applications, "Hopper File.app"), ""); + const runtime = [...localMacHopperRenamedLauncherCandidates(home)]; + expect(runtime.filter((path) => path.startsWith(home))).toEqual([ posix.join( home, "Applications/Hopper Preview.app/Contents/MacOS/hopper", ), + ]); + expect(await readMacHopperRenamedLauncherCandidates(home)).toEqual( + runtime, ); } finally { await rm(temporary, { recursive: true, force: true }); diff --git a/src/application/MacHopperPaths.ts b/src/application/MacHopperPaths.ts index b65712f0a6..b63762f833 100644 --- a/src/application/MacHopperPaths.ts +++ b/src/application/MacHopperPaths.ts @@ -1,4 +1,5 @@ -import { readdirSync } from "node:fs"; +import { readdirSync, type Dirent } from "node:fs"; +import { readdir } from "node:fs/promises"; import { posix } from "node:path"; export const MAC_HOPPER_APPLICATION_DIRECTORY = "/Applications"; @@ -52,13 +53,12 @@ export const macHopperRenamedLauncherCandidates = ( .map((name) => posix.join(root, name, "Contents/MacOS/hopper")), ); +const directoryNames = (entries: readonly Dirent[]): readonly string[] => + entries.filter((entry) => entry.isDirectory()).map(({ name }) => name); + const localBundleNames = (root: string): readonly string[] => { try { - return readdirSync(root, { withFileTypes: true }) - .filter( - (entry) => entry.isDirectory() && /^Hopper.*\.app$/iu.test(entry.name), - ) - .map(({ name }) => name); + return directoryNames(readdirSync(root, { withFileTypes: true })); } catch (cause: unknown) { // Missing or unreadable optional app directories contain no candidates. void cause; @@ -66,14 +66,32 @@ const localBundleNames = (root: string): readonly string[] => { } }; -/** Yield canonical paths first; enumerate renamed bundles only if needed. */ -export function* macHopperLauncherCandidates( +/** Yield renamed bundles from the local filesystem, reading each root only if needed. */ +export function* localMacHopperRenamedLauncherCandidates( homeDirectory: string, ): Generator { - yield* macHopperCanonicalLauncherCandidates(homeDirectory); for (const root of macHopperApplicationRoots(homeDirectory)) yield* macHopperRenamedLauncherCandidates( homeDirectory, new Map([[root, localBundleNames(root)]]), ); } + +/** Read the same renamed bundles without blocking the event loop, for doctor. */ +export const readMacHopperRenamedLauncherCandidates = async ( + homeDirectory: string, +): Promise => { + const bundleNamesByRoot = new Map(); + for (const root of macHopperApplicationRoots(homeDirectory)) { + try { + bundleNamesByRoot.set( + root, + directoryNames(await readdir(root, { withFileTypes: true })), + ); + } catch (cause: unknown) { + // Missing or unreadable optional app directories contain no candidates. + void cause; + } + } + return macHopperRenamedLauncherCandidates(homeDirectory, bundleNamesByRoot); +}; diff --git a/src/config/parseConfig.ts b/src/config/parseConfig.ts index c3d4a9fef0..067d09c7a7 100644 --- a/src/config/parseConfig.ts +++ b/src/config/parseConfig.ts @@ -8,14 +8,8 @@ import { DEFAULT_BRIDGE_MAX_REQUEST } from "./bridgeMaxRequestBytes.js"; import { DEFAULT_REGEX_MATCHING_DEADLINE } from "./regexMatchingDeadline.js"; import { DEFAULT_REGEX_WORKER_HEAP_REQUEST } from "./regexWorkerHeapRequest.js"; import type { AppConfig } from "./types.js"; -import { - linuxHopperLauncherCandidates, - REA_MANAGED_LINUX_HOPPER_LAUNCHER, -} from "../application/LinuxHopper.js"; -import { - MAC_HOPPER_SYSTEM_LAUNCHER, - macHopperLauncherCandidates, -} from "../application/MacHopperPaths.js"; +import { hopperLauncherDiscovery } from "../application/HopperLauncherDiscovery.js"; +import { localMacHopperRenamedLauncherCandidates } from "../application/MacHopperPaths.js"; const executableAvailable = (path: string): boolean => { try { accessSync(path, constants.X_OK); @@ -30,16 +24,15 @@ export const defaultHopperLauncherPath = ( homeDirectory: string, executable: (path: string) => boolean = executableAvailable, ): string => { - if (platform === "darwin") { - for (const candidate of macHopperLauncherCandidates(homeDirectory)) + const discovery = hopperLauncherDiscovery(platform, homeDirectory); + for (const candidate of discovery.candidates) + if (executable(candidate)) return candidate; + if (discovery.probesMacApplicationBundles) + for (const candidate of localMacHopperRenamedLauncherCandidates( + homeDirectory, + )) if (executable(candidate)) return candidate; - return MAC_HOPPER_SYSTEM_LAUNCHER; - } - if (platform !== "linux") return MAC_HOPPER_SYSTEM_LAUNCHER; - return ( - linuxHopperLauncherCandidates(homeDirectory).find(executable) ?? - REA_MANAGED_LINUX_HOPPER_LAUNCHER - ); + return discovery.fallback; }; /**