From 2984fa74acfb6f7cbb504677a348e20bff85b806 Mon Sep 17 00:00:00 2001 From: N0zoM1z0 <1705004398@qq.com> Date: Tue, 14 Jul 2026 20:14:58 +0000 Subject: [PATCH] fix(setup): omit aligned skill from plan --- src/application/Setup.ts | 71 +++++++++++++++++++++++++---------- src/application/SetupPlan.ts | 17 ++++++--- src/application/SetupSkill.ts | 45 +++++++++++++++++----- tests/setup.test.ts | 20 +++++++++- tests/setupSkill.test.ts | 7 +++- 5 files changed, 123 insertions(+), 37 deletions(-) diff --git a/src/application/Setup.ts b/src/application/Setup.ts index 639f71d72..ae8aff502 100644 --- a/src/application/Setup.ts +++ b/src/application/Setup.ts @@ -25,7 +25,10 @@ import { installMacHopper } from "./MacHopper.js"; import { configureDetectedClients } from "./SetupClients.js"; import { supportedClients, type SetupClient } from "./SupportedClients.js"; export type { SetupClient } from "./SupportedClients.js"; -import { installCanonicalSkill } from "./SetupSkill.js"; +import { + canonicalSkillNeedsInstall, + installCanonicalSkill, +} from "./SetupSkill.js"; import { setupPlan } from "./SetupPlan.js"; import { setupInstallFailure, @@ -34,7 +37,10 @@ import { } from "./SetupInstallFailure.js"; export type { SetupHopperInstallResult } from "./SetupInstallFailure.js"; -export { installCanonicalSkill } from "./SetupSkill.js"; +export { + canonicalSkillNeedsInstall, + installCanonicalSkill, +} from "./SetupSkill.js"; const registrationCommand = (): readonly string[] => process.env.npm_command === "exec" @@ -69,6 +75,7 @@ export interface SetupHost { hopperPath: string | undefined, command: readonly string[], ): Promise; + skillNeedsInstall(): Promise; installSkill(): Promise<"installed" | "unchanged" | "failed">; doctor(): Promise>>; } @@ -131,17 +138,10 @@ export const runSetup = async ( const unsupported = await hostRemediation(host); if (unsupported !== undefined) return fail(unsupported); let hopperPath = await host.hopperPath(); - const initialDoctor = await host.doctor(); - const linuxHopperRepairNeeded = initialDoctor.checks.some( - ({ name, ok, detail }) => - !ok && - (name === "hopper-demo-runtime" || - (name === "hopper-version" && detail === "/opt/hopper/bin/Hopper")), - ); - const installHopper = hopperPath === undefined || linuxHopperRepairNeeded; - const detectedClients = await host.detectedClients(); - plannedActions = setupPlan(host.platform, installHopper, detectedClients); - let approved = options.approved; + const discovery = await discoverSetupActions(host, hopperPath); + const { detectedClients, installHopper, installSkill } = discovery; + plannedActions = discovery.plannedActions; + let approved = options.approved || plannedActions.length === 0; let interactiveApproval = false; if (!approved && confirm !== undefined && !options.structured) { interactiveApproval = await confirm(plannedActions); @@ -182,12 +182,14 @@ export const runSetup = async ( appliedActions, }); if (clientFailure !== undefined) return fail(clientFailure); - const skill = await host.installSkill(); - if (skill === "failed") - return fail( - "REA analysis skill could not be installed or verified. Check permissions for `~/.agents/skills`, then rerun setup.", - ); - if (skill === "installed") appliedActions.push("installed_skill"); + if (installSkill) { + const skill = await host.installSkill(); + if (skill === "failed") + return fail( + "REA analysis skill could not be installed or verified. Check permissions for `~/.agents/skills`, then rerun setup.", + ); + if (skill === "installed") appliedActions.push("installed_skill"); + } const doctor = await host.doctor(); const remediation = finalSetupRemediation( host.platform, @@ -205,6 +207,36 @@ export const runSetup = async ( }; }; +const discoverSetupActions = async ( + host: SetupHost, + hopperPath: string | undefined, +) => { + const initialDoctor = await host.doctor(); + const linuxHopperRepairNeeded = initialDoctor.checks.some( + ({ name, ok, detail }) => + !ok && + (name === "hopper-demo-runtime" || + (name === "hopper-version" && detail === "/opt/hopper/bin/Hopper")), + ); + const installHopper = hopperPath === undefined || linuxHopperRepairNeeded; + const [detectedClients, installSkill] = await Promise.all([ + host.detectedClients(), + host.skillNeedsInstall(), + ]); + const plannedActions = setupPlan( + host.platform, + installHopper, + installSkill, + detectedClients, + ); + return { + installHopper, + installSkill, + detectedClients, + plannedActions, + }; +}; + const finalSetupRemediation = ( platform: NodeJS.Platform, installedHopper: boolean, @@ -261,6 +293,7 @@ const systemSetupHost = (): SetupHost => { : client.format === "toml" ? configureTomlClient(client, hopperPath, command) : configureJsonClient(client, hopperPath, command), + skillNeedsInstall: () => canonicalSkillNeedsInstall(homedir()), installSkill: () => installCanonicalSkill(homedir()), doctor: () => runDoctor(undefined, doctorHost), }; diff --git a/src/application/SetupPlan.ts b/src/application/SetupPlan.ts index 38aba05e4..7c6789886 100644 --- a/src/application/SetupPlan.ts +++ b/src/application/SetupPlan.ts @@ -4,6 +4,7 @@ import type { SetupAction, SetupClient } from "./Setup.js"; export const setupPlan = ( platform: NodeJS.Platform, installHopper: boolean, + installSkill: boolean, clients: readonly SetupClient[], ): readonly SetupAction[] => [ ...(installHopper @@ -32,10 +33,14 @@ export const setupPlan = ( external: false, }), ), - { - kind: "install_skill", - target: "~/.agents/skills/rea-analysis/SKILL.md", - detail: "Install or update the bundled REA analysis skill.", - external: false, - }, + ...(installSkill + ? [ + { + kind: "install_skill" as const, + target: "~/.agents/skills/rea-analysis/SKILL.md", + detail: "Install or update the bundled REA analysis skill.", + external: false, + }, + ] + : []), ]; diff --git a/src/application/SetupSkill.ts b/src/application/SetupSkill.ts index 5df937780..c8907cd58 100644 --- a/src/application/SetupSkill.ts +++ b/src/application/SetupSkill.ts @@ -4,26 +4,53 @@ import writeFileAtomic from "write-file-atomic"; import { PRODUCT_IDENTITY } from "../identity.js"; +const canonicalSkillFiles = async ( + home: string, +): Promise<{ readonly content: string; readonly destination: string }> => ({ + destination: join( + home, + ".agents/skills", + PRODUCT_IDENTITY.skillName, + "SKILL.md", + ), + content: await readFile( + new URL( + `../../skills/${PRODUCT_IDENTITY.skillName}/SKILL.md`, + import.meta.url, + ), + "utf8", + ), +}); + +/** Report whether setup would change the managed REA skill. */ +export const canonicalSkillNeedsInstall = async ( + home: string, +): Promise => { + try { + const { content, destination } = await canonicalSkillFiles(home); + return (await readFile(destination, "utf8")) !== content; + } catch { + return true; + } +}; + /** Transactionally install or upgrade the versioned canonical REA skill. */ export const installCanonicalSkill = async ( home: string, ): Promise<"installed" | "unchanged" | "failed"> => { - const destination = join( + let destination = join( home, ".agents/skills", PRODUCT_IDENTITY.skillName, "SKILL.md", ); - const backup = `${destination}.rea.backup`; + let backup = `${destination}.rea.backup`; let original: string | undefined; try { - const content = await readFile( - new URL( - `../../skills/${PRODUCT_IDENTITY.skillName}/SKILL.md`, - import.meta.url, - ), - "utf8", - ); + const canonical = await canonicalSkillFiles(home); + destination = canonical.destination; + backup = `${destination}.rea.backup`; + const { content } = canonical; original = await readFile(destination, "utf8").catch(() => undefined); if (original === content) return "unchanged"; await mkdir(dirname(destination), { recursive: true }); diff --git a/tests/setup.test.ts b/tests/setup.test.ts index 11c79dfb1..2a7dd5fed 100644 --- a/tests/setup.test.ts +++ b/tests/setup.test.ts @@ -23,6 +23,7 @@ class FakeSetupHost implements SetupHost { clientResults = new Map(); hopperInstalls = 0; configurations = 0; + skillInstalls = 0; doctorHealthy: boolean | undefined; linuxDemoRuntimeMissing = false; unsupportedHopperVersion = false; @@ -62,8 +63,12 @@ class FakeSetupHost implements SetupHost { this.clientResults.get(client.name) ?? { status: "configured" }, ); }; - installSkill = (): Promise<"installed" | "unchanged" | "failed"> => - Promise.resolve(this.skill); + skillNeedsInstall = (): Promise => + Promise.resolve(this.skill !== "unchanged"); + installSkill = (): Promise<"installed" | "unchanged" | "failed"> => { + this.skillInstalls += 1; + return Promise.resolve(this.skill); + }; doctor = (): Promise<{ healthy: boolean; hopperPath?: string; @@ -218,6 +223,17 @@ describe("setup workflow", () => { expect(host.hopperInstalls).toBe(0); }); + it("omits an aligned managed skill from an otherwise empty plan", async () => { + const host = new FakeSetupHost(); + host.hopper = "/Applications/Hopper"; + host.skill = "unchanged"; + const result = await runSetup(options(false), host); + expect(result.status).toBe("ready"); + expect(result.plannedActions).toEqual([]); + expect(result.appliedActions).toEqual([]); + expect(host.skillInstalls).toBe(0); + }); + it("installs missing Linux demo dependencies for existing Hopper", async () => { const host = new FakeSetupHost("linux"); host.distribution = { diff --git a/tests/setupSkill.test.ts b/tests/setupSkill.test.ts index 9e27a8d8a..559084c1e 100644 --- a/tests/setupSkill.test.ts +++ b/tests/setupSkill.test.ts @@ -2,7 +2,10 @@ import { mkdtemp, mkdir, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { afterEach, describe, expect, it } from "vitest"; -import { installCanonicalSkill } from "../src/application/Setup.js"; +import { + canonicalSkillNeedsInstall, + installCanonicalSkill, +} from "../src/application/Setup.js"; import { TOOL_CONTRACTS } from "../src/contracts/toolContracts.js"; const roots: string[] = []; @@ -23,6 +26,7 @@ describe("canonical skill transaction", () => { await writeFile(destination, "stale managed skill\n"); await writeFile(sibling, "unrelated skill\n"); + expect(await canonicalSkillNeedsInstall(home)).toBe(true); expect(await installCanonicalSkill(home)).toBe("installed"); expect(await readFile(`${destination}.rea.backup`, "utf8")).toBe( "stale managed skill\n", @@ -32,6 +36,7 @@ describe("canonical skill transaction", () => { `tool_count: ${String(TOOL_CONTRACTS.length)}`, ); expect(await readFile(sibling, "utf8")).toBe("unrelated skill\n"); + expect(await canonicalSkillNeedsInstall(home)).toBe(false); expect(await installCanonicalSkill(home)).toBe("unchanged"); }); });