From 02f12b927a17615150d6e1126d1b841c5dca648b Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 15 Jul 2026 06:34:25 +0000 Subject: [PATCH 1/3] fix: address setup, upgrade, and process test regressions --- scripts/verify-package.mjs | 5 ++-- src/application/Setup.ts | 44 +++++++++++++++++++++++++++--------- src/application/Upgrade.ts | 3 +-- tests/processCapture.test.ts | 4 ++-- tests/setup.test.ts | 29 ++++++++++++++++++++++-- tests/setupConfig.test.ts | 6 +++++ 6 files changed, 72 insertions(+), 19 deletions(-) diff --git a/scripts/verify-package.mjs b/scripts/verify-package.mjs index c28fdee8a..65598341f 100644 --- a/scripts/verify-package.mjs +++ b/scripts/verify-package.mjs @@ -421,10 +421,11 @@ try { alignedExecution.status !== (status === "ready" ? 0 : 1) || aligned.plannedActions.length !== 0 || aligned.appliedActions.length !== 0 || - second.appliedActions.length !== 0 + !second.plannedActions.some(({ kind }) => kind === "install_hopper") || + !second.appliedActions.includes("installed_hopper") ) throw new Error( - `packaged setup was not ready and idempotent without confirmation: ${JSON.stringify({ first, aligned, second })}`, + `packaged setup did not preserve idempotent defaults and explicit Hopper reinstall behavior: ${JSON.stringify({ first, aligned, second })}`, ); for (const configPath of [claudeConfig, cursorConfig]) { const config = json(await readFile(configPath, "utf8")); diff --git a/src/application/Setup.ts b/src/application/Setup.ts index 80dc20b9d..4d06275d6 100644 --- a/src/application/Setup.ts +++ b/src/application/Setup.ts @@ -6,6 +6,7 @@ import { rm, writeFile, } from "node:fs/promises"; +import { constants as fsConstants } from "node:fs"; import { homedir } from "node:os"; import { dirname, resolve } from "node:path"; import { z } from "zod"; @@ -143,8 +144,13 @@ export const runSetup = async ( const unsupported = await hostRemediation(host); if (unsupported !== undefined) return fail(unsupported); let hopperPath = await host.hopperPath(); - const discovery = await discoverSetupActions(host, hopperPath); - const { detectedClients, installHopper, installSkill } = discovery; + const discovery = await discoverSetupActions( + host, + hopperPath, + options.installHopper, + ); + const { installHopper, installSkill } = discovery; + let { detectedClients } = discovery; plannedActions = discovery.plannedActions; let approved = options.approved || plannedActions.length === 0; let interactiveApproval = false; @@ -177,6 +183,12 @@ export const runSetup = async ( }; hopperPath = installed.launcherPath; appliedActions.push("installed_hopper"); + detectedClients = await filterClientsNeedingConfigure( + host, + detectedClients, + hopperPath, + registrationCommand(), + ); } const clientFailure = await configureDetectedClients({ host, @@ -215,6 +227,7 @@ export const runSetup = async ( const discoverSetupActions = async ( host: SetupHost, hopperPath: string | undefined, + forceHopperInstall: boolean, ) => { const initialDoctor = await host.doctor(); const linuxHopperRepairNeeded = initialDoctor.checks.some( @@ -223,7 +236,8 @@ const discoverSetupActions = async ( (name === "hopper-demo-runtime" || (name === "hopper-version" && detail === "/opt/hopper/bin/Hopper")), ); - const installHopper = hopperPath === undefined || linuxHopperRepairNeeded; + const installHopper = + forceHopperInstall || hopperPath === undefined || linuxHopperRepairNeeded; const [detectedClients, installSkill] = await Promise.all([ host.detectedClients(), host.skillNeedsInstall(), @@ -387,11 +401,8 @@ export const configureJsonClient = async ( let backupPath: string | undefined; if (original !== undefined) { backupPath = `${client.configPath}.rea.backup`; - try { - await copyFile(transactionPath, backupPath); - } catch { + if (!(await preserveConfigBackup(transactionPath, backupPath))) return { status: "failed", reason: "backup" }; - } } document.mcpServers = { ...servers, @@ -470,11 +481,11 @@ export const configureTomlClient = async ( return { status: "unchanged" }; const backupPath = original === undefined ? undefined : `${client.configPath}.rea.backup`; - try { - if (backupPath !== undefined) await copyFile(transactionPath, backupPath); - } catch { + if ( + backupPath !== undefined && + !(await preserveConfigBackup(transactionPath, backupPath)) + ) return { status: "failed", reason: "backup" }; - } document.mcp_servers = { ...servers, [PRODUCT_IDENTITY.mcpServerKey]: desired, @@ -516,6 +527,17 @@ const exists = async (path: string): Promise => { return false; } }; +const preserveConfigBackup = async ( + source: string, + destination: string, +): Promise => { + try { + await copyFile(source, destination, fsConstants.COPYFILE_EXCL); + return true; + } catch (cause: unknown) { + return cause instanceof Error && "code" in cause && cause.code === "EEXIST"; + } +}; const objectSchema = z.record(z.string(), z.unknown()); const parseOptionalObject = (value: unknown): Record => value === undefined ? {} : objectSchema.parse(value); diff --git a/src/application/Upgrade.ts b/src/application/Upgrade.ts index 756c5d5e4..f3d51abf3 100644 --- a/src/application/Upgrade.ts +++ b/src/application/Upgrade.ts @@ -208,8 +208,7 @@ const runCommand = ( ): Promise => new Promise((resolveResult) => { const child = spawn(command, [...arguments_], { - stdio: - output === "human" ? "inherit" : ["inherit", process.stderr, "inherit"], + stdio: output === "human" ? "inherit" : ["inherit", "ignore", "inherit"], }); child.once("error", () => resolveResult(false)); child.once("exit", (code) => resolveResult(code === 0)); diff --git a/tests/processCapture.test.ts b/tests/processCapture.test.ts index 6309d9c24..5d64a20e6 100644 --- a/tests/processCapture.test.ts +++ b/tests/processCapture.test.ts @@ -967,7 +967,7 @@ describe("process capture adapter", () => { } finally { await rm(root, { recursive: true, force: true }); } - }); + }, 20_000); it("does not follow or disclose symlink targets outside declared roots", async () => { const root = await mkdtemp(join(tmpdir(), "rea-symlink-test-")); @@ -1186,5 +1186,5 @@ describe("process capture adapter", () => { const { stdout } = await execFileAsync("ps", ["-axo", "command="]); expect(stdout).not.toContain(`${processFixture} tree-child`); expect(stdout).not.toContain(`${processFixture} tree-grandchild`); - }); + }, 20_000); }); diff --git a/tests/setup.test.ts b/tests/setup.test.ts index c3e1830aa..670c79912 100644 --- a/tests/setup.test.ts +++ b/tests/setup.test.ts @@ -24,6 +24,7 @@ class FakeSetupHost implements SetupHost { hopperInstalls = 0; configurations = 0; skillInstalls = 0; + checkedHopperPaths: Array = []; doctorHealthy: boolean | undefined; linuxDemoRuntimeMissing = false; unsupportedHopperVersion = false; @@ -63,10 +64,15 @@ class FakeSetupHost implements SetupHost { this.clientResults.get(client.name) ?? { status: "configured" }, ); }; - clientNeedsConfigure = (client: SetupClient): Promise => - Promise.resolve( + clientNeedsConfigure = ( + client: SetupClient, + hopperPath: string | undefined, + ): Promise => { + this.checkedHopperPaths.push(hopperPath); + return Promise.resolve( this.clientResults.get(client.name)?.status !== "unchanged", ); + }; skillNeedsInstall = (): Promise => Promise.resolve(this.skill !== "unchanged"); installSkill = (): Promise<"installed" | "unchanged" | "failed"> => { @@ -227,6 +233,25 @@ describe("setup workflow", () => { expect(host.hopperInstalls).toBe(0); }); + it("reinstalls existing Hopper when explicitly requested", async () => { + const host = new FakeSetupHost(); + host.hopper = "/Applications/Hopper"; + host.skill = "unchanged"; + host.clients = [{ name: "codex", configPath: "/codex.toml" }]; + host.clientResults.set("codex", { status: "unchanged" }); + + const result = await runSetup(options(true, true), host); + + expect(result.plannedActions.map(({ kind }) => kind)).toEqual([ + "install_hopper", + "configure_client", + ]); + expect(result.appliedActions).toEqual(["installed_hopper"]); + expect(host.hopperInstalls).toBe(1); + expect(host.configurations).toBe(0); + expect(host.checkedHopperPaths).toEqual(["/manual/Hopper"]); + }); + it("omits an aligned managed skill from an otherwise empty plan", async () => { const host = new FakeSetupHost(); host.hopper = "/Applications/Hopper"; diff --git a/tests/setupConfig.test.ts b/tests/setupConfig.test.ts index 9dd9516ed..292d05ddd 100644 --- a/tests/setupConfig.test.ts +++ b/tests/setupConfig.test.ts @@ -123,6 +123,8 @@ describe("JSON client configuration transaction", () => { const configPath = join(directory, "mcp.json"); const hopperPath = "/Applications/Hopper v6.app/Contents/MacOS/hopper"; const client = { name: "cursor", configPath }; + const original = "{}\n"; + await writeFile(configPath, original); expect(await configureJsonClient(client, hopperPath)).toMatchObject({ status: "configured", }); @@ -138,6 +140,10 @@ describe("JSON client configuration transaction", () => { expect(await configureJsonClient(client, hopperPath)).toEqual({ status: "unchanged", }); + expect( + await configureJsonClient(client, "/opt/hopper/bin/Hopper"), + ).toMatchObject({ status: "configured" }); + expect(await readFile(`${configPath}.rea.backup`, "utf8")).toBe(original); }); it("refuses malformed existing JSON without overwriting it", async () => { From acaf03056d6b457d85c6b7744565d6c7d56c424c Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 15 Jul 2026 06:40:23 +0000 Subject: [PATCH 2/3] test: keep package setup verification read-only --- scripts/verify-package.mjs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/verify-package.mjs b/scripts/verify-package.mjs index 65598341f..dc34a0a91 100644 --- a/scripts/verify-package.mjs +++ b/scripts/verify-package.mjs @@ -406,7 +406,7 @@ try { const aligned = json(alignedExecution.stdout); const secondExecution = await runWithStatus( cli, - ["setup", "--yes", "--install-hopper", "--json"], + ["setup", "--install-hopper", "--json"], environment, ); const second = json(secondExecution.stdout); @@ -414,15 +414,15 @@ try { const status = process.platform === "linux" ? "needs_human" : "ready"; if ( first.status !== status || - second.status !== status || + second.status !== "needs_confirmation" || firstExecution.status !== (status === "ready" ? 0 : 1) || - secondExecution.status !== (status === "ready" ? 0 : 1) || + secondExecution.status !== 1 || aligned.status !== status || alignedExecution.status !== (status === "ready" ? 0 : 1) || aligned.plannedActions.length !== 0 || aligned.appliedActions.length !== 0 || !second.plannedActions.some(({ kind }) => kind === "install_hopper") || - !second.appliedActions.includes("installed_hopper") + second.appliedActions.length !== 0 ) throw new Error( `packaged setup did not preserve idempotent defaults and explicit Hopper reinstall behavior: ${JSON.stringify({ first, aligned, second })}`, From 0ca93f9779bc6c576479561c488982963fbb9167 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 15 Jul 2026 06:52:52 +0000 Subject: [PATCH 3/3] fix(setup): replace managed Hopper on reinstall --- src/application/MacHopper.ts | 23 ++++++++++++++++++++--- src/application/Setup.ts | 8 ++++---- tests/macHopper.test.ts | 29 ++++++++++++++++++++++++----- tests/setup.test.ts | 7 ++++++- 4 files changed, 54 insertions(+), 13 deletions(-) diff --git a/src/application/MacHopper.ts b/src/application/MacHopper.ts index 62e217719..17286aa91 100644 --- a/src/application/MacHopper.ts +++ b/src/application/MacHopper.ts @@ -55,6 +55,7 @@ export interface MacHopperInstallHost { source: string, destination: string, stage: string, + replaceExisting: boolean, ): Promise; launcherReady(path: string): Promise; openApplication(path: string): Promise; @@ -64,6 +65,7 @@ export interface MacHopperInstallHost { /** Download, verify, and install Hopper into the current user's Applications directory. */ export const installMacHopper = async ( + options: { readonly replaceExisting?: boolean } = {}, host: MacHopperInstallHost = systemMacHopperInstallHost(), ): Promise => { let temporary: string | undefined; @@ -83,7 +85,8 @@ export const installMacHopper = async ( return { status: "failed", reason: "integrity" }; const destination = host.destination(); - if (await host.destinationExists(destination)) + const destinationExists = await host.destinationExists(destination); + if (destinationExists && options.replaceExisting !== true) return { status: "failed", reason: "destination_exists" }; temporary = await host.createTemporaryDirectory(); const packagePath = join(temporary, "hopper.dmg"); @@ -97,7 +100,9 @@ export const installMacHopper = async ( if (source === undefined || !(await host.validBundle(source))) return { status: "failed", reason: "bundle" }; const stage = `${destination}.rea-stage`; - if (!(await host.installBundle(source, destination, stage))) + if ( + !(await host.installBundle(source, destination, stage, destinationExists)) + ) return { status: "failed", reason: "copy" }; const launcherPath = join(destination, "Contents/MacOS/hopper"); if (!(await host.launcherReady(launcherPath))) @@ -181,15 +186,27 @@ const systemMacHopperInstallHost = (): MacHopperInstallHost => ({ return false; } }, - async installBundle(source, destination, stage) { + async installBundle(source, destination, stage, replaceExisting) { + const previous = `${stage}-previous`; try { await mkdir(join(homedir(), "Applications"), { recursive: true }); await rm(stage, { recursive: true, force: true }); + await rm(previous, { recursive: true, force: true }); await execFileAsync("ditto", [source, stage]); + await access(join(stage, "Contents/MacOS/hopper")); + if (replaceExisting) await rename(destination, previous); await rename(stage, destination); + if (replaceExisting) await rm(previous, { recursive: true, force: true }); return true; } catch { await rm(stage, { recursive: true, force: true }); + if (replaceExisting) { + try { + await rename(previous, destination); + } catch { + // The original destination was not moved, or rollback is impossible. + } + } return false; } }, diff --git a/src/application/Setup.ts b/src/application/Setup.ts index 4d06275d6..fb4fca45c 100644 --- a/src/application/Setup.ts +++ b/src/application/Setup.ts @@ -69,7 +69,7 @@ export interface SetupHost { macosVersion(): Promise; linuxDistribution(): Promise; hopperPath(): Promise; - installHopper(): Promise; + installHopper(replaceExisting: boolean): Promise; detectedClients(): Promise; configureClient( client: SetupClient, @@ -170,7 +170,7 @@ export const runSetup = async ( }; if (installHopper && (interactiveApproval || options.installHopper)) { - const installed = await host.installHopper(); + const installed = await host.installHopper(options.installHopper); if (installed.status === "failed") return { status: "needs_human", @@ -320,11 +320,11 @@ const systemSetupHost = (): SetupHost => { macosVersion: () => doctorHost.macosVersion(), linuxDistribution: readLinuxDistribution, hopperPath: async () => (await runDoctor(undefined, doctorHost)).hopperPath, - installHopper: async () => { + installHopper: async (replaceExisting) => { const result = process.platform === "linux" ? await installLinuxHopper() - : await installMacHopper(); + : await installMacHopper({ replaceExisting }); if (result.status === "installed") return result; return setupInstallFailure(result.reason); }, diff --git a/tests/macHopper.test.ts b/tests/macHopper.test.ts index 7647f274f..61bc1f26d 100644 --- a/tests/macHopper.test.ts +++ b/tests/macHopper.test.ts @@ -19,6 +19,7 @@ class FakeMacHopperHost implements MacHopperInstallHost { unmounts = 0; cleanups = 0; opens = 0; + replaces = false; download = (url: string) => Promise.resolve( @@ -51,7 +52,15 @@ class FakeMacHopperHost implements MacHopperInstallHost { appBundle = () => Promise.resolve("/mounted/Hopper Disassembler.app"); validBundle = () => Promise.resolve(this.valid); destinationExists = () => Promise.resolve(this.existing); - installBundle = () => Promise.resolve(this.copySucceeds); + installBundle = ( + _source: string, + _destination: string, + _stage: string, + replaceExisting: boolean, + ) => { + this.replaces = replaceExisting; + return Promise.resolve(this.copySucceeds); + }; launcherReady = () => Promise.resolve(this.launcherSucceeds); openApplication = () => { this.opens += 1; @@ -67,7 +76,7 @@ class FakeMacHopperHost implements MacHopperInstallHost { describe("macOS Hopper installation", () => { it("installs, verifies, opens, and cleans the official package", async () => { const host = new FakeMacHopperHost(); - await expect(installMacHopper(host)).resolves.toEqual({ + await expect(installMacHopper({}, host)).resolves.toEqual({ status: "installed", launcherPath: "/home/user/Applications/Hopper Disassembler.app/Contents/MacOS/hopper", @@ -80,7 +89,7 @@ describe("macOS Hopper installation", () => { it("rejects an integrity mismatch before mounting", async () => { const host = new FakeMacHopperHost(); host.badHash = true; - await expect(installMacHopper(host)).resolves.toEqual({ + await expect(installMacHopper({}, host)).resolves.toEqual({ status: "failed", reason: "integrity", }); @@ -91,16 +100,26 @@ describe("macOS Hopper installation", () => { it("refuses to overwrite an existing application", async () => { const host = new FakeMacHopperHost(); host.existing = true; - await expect(installMacHopper(host)).resolves.toEqual({ + await expect(installMacHopper({}, host)).resolves.toEqual({ status: "failed", reason: "destination_exists", }); }); + it("replaces the managed application when explicitly requested", async () => { + const host = new FakeMacHopperHost(); + host.existing = true; + + await expect( + installMacHopper({ replaceExisting: true }, host), + ).resolves.toMatchObject({ status: "installed" }); + expect(host.replaces).toBe(true); + }); + it("cleans and detaches after bundle validation fails", async () => { const host = new FakeMacHopperHost(); host.valid = false; - await expect(installMacHopper(host)).resolves.toEqual({ + await expect(installMacHopper({}, host)).resolves.toEqual({ status: "failed", reason: "bundle", }); diff --git a/tests/setup.test.ts b/tests/setup.test.ts index 670c79912..23c0e22c2 100644 --- a/tests/setup.test.ts +++ b/tests/setup.test.ts @@ -22,6 +22,7 @@ class FakeSetupHost implements SetupHost { clients: readonly SetupClient[] = []; clientResults = new Map(); hopperInstalls = 0; + hopperReplaceRequests: boolean[] = []; configurations = 0; skillInstalls = 0; checkedHopperPaths: Array = []; @@ -38,8 +39,11 @@ class FakeSetupHost implements SetupHost { linuxDistribution = (): Promise => Promise.resolve(this.distribution); hopperPath = (): Promise => Promise.resolve(this.hopper); - installHopper = (): Promise => { + installHopper = ( + replaceExisting: boolean, + ): Promise => { this.hopperInstalls += 1; + this.hopperReplaceRequests.push(replaceExisting); this.linuxDemoRuntimeMissing = false; if (this.hopperInstallSucceeds) { this.hopper = "/manual/Hopper"; @@ -248,6 +252,7 @@ describe("setup workflow", () => { ]); expect(result.appliedActions).toEqual(["installed_hopper"]); expect(host.hopperInstalls).toBe(1); + expect(host.hopperReplaceRequests).toEqual([true]); expect(host.configurations).toBe(0); expect(host.checkedHopperPaths).toEqual(["/manual/Hopper"]); });