From a86da4ff3f9279e4ca3fc792e6f18542fee9e0ce Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Tue, 14 Jul 2026 22:00:42 +0000 Subject: [PATCH] fix(setup): omit aligned client configurations from plan The setup plan was still advertising configure_client actions for clients whose MCP registration already matched the desired command and Hopper environment. As a result, an otherwise idempotent rerun requested confirmation for changes that would not be applied. - add SetupHost#clientNeedsConfigure to read a client config without mutating it - filter detected clients in discoverSetupActions before building the plan - align the package verifier's "aligned" run with the platform-specific status so Linux (where the fake Hopper path is unsupported) is expected to report needs_human while keeping an empty plan --- scripts/verify-package.mjs | 4 +-- src/application/Setup.ts | 74 ++++++++++++++++++++++++++++++++++++-- tests/setup.test.ts | 4 +++ 3 files changed, 78 insertions(+), 4 deletions(-) diff --git a/scripts/verify-package.mjs b/scripts/verify-package.mjs index c11123bae7..66e7058346 100644 --- a/scripts/verify-package.mjs +++ b/scripts/verify-package.mjs @@ -387,8 +387,8 @@ try { second.status !== status || firstExecution.status !== (status === "ready" ? 0 : 1) || secondExecution.status !== (status === "ready" ? 0 : 1) || - aligned.status !== "ready" || - alignedExecution.status !== 0 || + aligned.status !== status || + alignedExecution.status !== (status === "ready" ? 0 : 1) || aligned.plannedActions.length !== 0 || aligned.appliedActions.length !== 0 || second.appliedActions.length !== 0 diff --git a/src/application/Setup.ts b/src/application/Setup.ts index ae8aff5026..7bcfec14aa 100644 --- a/src/application/Setup.ts +++ b/src/application/Setup.ts @@ -75,6 +75,11 @@ export interface SetupHost { hopperPath: string | undefined, command: readonly string[], ): Promise; + clientNeedsConfigure( + client: SetupClient, + hopperPath: string | undefined, + command: readonly string[], + ): Promise; skillNeedsInstall(): Promise; installSkill(): Promise<"installed" | "unchanged" | "failed">; doctor(): Promise>>; @@ -223,20 +228,43 @@ const discoverSetupActions = async ( host.detectedClients(), host.skillNeedsInstall(), ]); + const command = registrationCommand(); + const clients = installHopper + ? detectedClients + : await filterClientsNeedingConfigure( + host, + detectedClients, + hopperPath, + command, + ); const plannedActions = setupPlan( host.platform, installHopper, installSkill, - detectedClients, + clients, ); return { installHopper, installSkill, - detectedClients, + detectedClients: clients, plannedActions, }; }; +const filterClientsNeedingConfigure = async ( + host: SetupHost, + detectedClients: readonly SetupClient[], + hopperPath: string | undefined, + command: readonly string[], +): Promise => { + const needs = await Promise.all( + detectedClients.map((client) => + host.clientNeedsConfigure(client, hopperPath, command), + ), + ); + return detectedClients.filter((_, index) => needs[index]); +}; + const finalSetupRemediation = ( platform: NodeJS.Platform, installedHopper: boolean, @@ -293,6 +321,10 @@ const systemSetupHost = (): SetupHost => { : client.format === "toml" ? configureTomlClient(client, hopperPath, command) : configureJsonClient(client, hopperPath, command), + clientNeedsConfigure: (client, hopperPath, command) => + clientConfigurationAligned(client, hopperPath, command).then( + (aligned) => !aligned, + ), skillNeedsInstall: () => canonicalSkillNeedsInstall(homedir()), installSkill: () => installCanonicalSkill(homedir()), doctor: () => runDoctor(undefined, doctorHost), @@ -492,3 +524,41 @@ const restoreConfig = async ( // The backup remains available for the remediation reported by setup. } }; + +const clientConfigurationDesired = ( + hopperPath: string | undefined, + command: readonly string[], +) => ({ + command: command[0] ?? PRODUCT_IDENTITY.cliBinary, + args: command.slice(1), + ...(hopperPath === undefined + ? {} + : { env: { HOPPER_LAUNCHER_PATH: hopperPath } }), +}); + +const clientConfigurationAligned = async ( + client: SetupClient, + hopperPath: string | undefined, + command: readonly string[], +): Promise => { + const desired = clientConfigurationDesired(hopperPath, command); + try { + const original = await readFile(client.configPath, "utf8"); + if (client.format === "toml") { + const document = objectSchema.parse(parseToml(original)); + const servers = parseOptionalObject(document.mcp_servers); + return ( + JSON.stringify(servers[PRODUCT_IDENTITY.mcpServerKey]) === + JSON.stringify(desired) + ); + } + const document = parseObject(original); + const servers = parseOptionalObject(document.mcpServers); + return ( + JSON.stringify(servers[PRODUCT_IDENTITY.mcpServerKey]) === + JSON.stringify(desired) + ); + } catch { + return false; + } +}; diff --git a/tests/setup.test.ts b/tests/setup.test.ts index 2a7dd5fed8..efc75d1a3f 100644 --- a/tests/setup.test.ts +++ b/tests/setup.test.ts @@ -63,6 +63,10 @@ class FakeSetupHost implements SetupHost { this.clientResults.get(client.name) ?? { status: "configured" }, ); }; + clientNeedsConfigure = (client: SetupClient): Promise => + Promise.resolve( + this.clientResults.get(client.name)?.status !== "unchanged", + ); skillNeedsInstall = (): Promise => Promise.resolve(this.skill !== "unchanged"); installSkill = (): Promise<"installed" | "unchanged" | "failed"> => {