fix(setup): omit aligned skill from plan

This commit is contained in:
N0zoM1z0 2026-07-14 20:14:58 +00:00
parent 528c05e2fa
commit 2984fa74ac
5 changed files with 123 additions and 37 deletions

View File

@ -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<ClientConfigurationResult>;
skillNeedsInstall(): Promise<boolean>;
installSkill(): Promise<"installed" | "unchanged" | "failed">;
doctor(): Promise<Awaited<ReturnType<typeof runDoctor>>>;
}
@ -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),
};

View File

@ -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,
},
]
: []),
];

View File

@ -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<boolean> => {
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 });

View File

@ -23,6 +23,7 @@ class FakeSetupHost implements SetupHost {
clientResults = new Map<string, ClientConfigurationResult>();
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<boolean> =>
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 = {

View File

@ -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");
});
});