From b4a19193fe3baf9075f8ac419517021ea0c71ea3 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sat, 10 Oct 2026 01:59:12 +0800 Subject: [PATCH] fix(setup): install the skill for Claude Code --- docs/installation.md | 12 +- src/application/Doctor.fixture.ts | 1 - src/application/Doctor.test.ts | 1 - src/application/Doctor.ts | 73 +++----- src/application/Setup.fixture.ts | 2 +- src/application/Setup.planning.test.ts | 40 +++++ src/application/Setup.recovery.test.ts | 4 +- src/application/Setup.ts | 31 +++- src/application/SetupHost.ts | 18 +- src/application/SetupPlan.ts | 50 +++--- src/application/SetupSkill.ts | 136 +++++++++++--- src/application/SetupTypes.ts | 7 +- src/application/Uninstall.ts | 49 ++++- .../filesystem/clientLifecycle.test.ts | 39 +++- tests/boundary/filesystem/setupSkill.test.ts | 169 +++++++++++++++--- 15 files changed, 484 insertions(+), 148 deletions(-) diff --git a/docs/installation.md b/docs/installation.md index ad141737b..fecbe4aa0 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -192,6 +192,13 @@ default. Hopper is a separate optional choice: setup shows its proposed installation or connection and requires its own explicit approval. It can also save verified paths for an existing Ghidra installation. +The bundled skill is installed where each selected client discovers personal +skills: Claude Code uses `~/.claude/skills` (or +`$CLAUDE_CONFIG_DIR/skills` when configured), while other supported clients +use the shared `~/.agents/skills` directory. A mixed selection plans both +paths. Selecting the skill without a client uses the shared directory. Setup +leaves existing skill copies in other locations untouched. + After selection, review the plan's exact paths and changes and approve before REA writes files or installs Hopper. You can cancel at any prompt. @@ -199,7 +206,7 @@ Before applying changes, REA checks your current configuration. The plan lists: - an existing Hopper installation, a verified existing Ghidra installation, or the official Hopper package it proposes to install; - each detected agent configuration path; -- the REA skill destination; +- each selected REA skill destination; - external software, network origins, integrity evidence, and package-manager commands. @@ -658,7 +665,8 @@ and other MCP servers. Purging removes only REA's cache and state under `~/.rea`. A client configuration that is malformed, unreadable, or at an unsafe path stops the operation before anything is removed, and a client that fails while being updated stops the remaining removals. A purge path that is a -symbolic link is retained and reported rather than followed. See the +symbolic link is retained and reported rather than followed. Uninstall checks +REA's managed shared and Claude Code personal skill locations. See the [CLI guide](cli.md#output-and-exit-status) for exit statuses. ## MCP Registry diff --git a/src/application/Doctor.fixture.ts b/src/application/Doctor.fixture.ts index 375439098..770b78fe6 100644 --- a/src/application/Doctor.fixture.ts +++ b/src/application/Doctor.fixture.ts @@ -27,7 +27,6 @@ export const createDoctorHostFixture = ( Promise.resolve({ version: PRODUCT_IDENTITY.skillVersion, toolCount: CATALOG_IDENTITY.counts.mcp_tools, - catalogDigest: null, canonical: true, }), ...overrides, diff --git a/src/application/Doctor.test.ts b/src/application/Doctor.test.ts index 70b85bf6a..afc0b5ef1 100644 --- a/src/application/Doctor.test.ts +++ b/src/application/Doctor.test.ts @@ -176,7 +176,6 @@ describe("doctor installation identity", () => { Promise.resolve({ version: "10", toolCount: null, - catalogDigest: null, canonical: false, }), }), diff --git a/src/application/Doctor.ts b/src/application/Doctor.ts index 5426a6453..88dfda4c8 100644 --- a/src/application/Doctor.ts +++ b/src/application/Doctor.ts @@ -1,5 +1,5 @@ import { constants } from "node:fs"; -import { access, readFile, readdir } from "node:fs/promises"; +import { access, readdir } from "node:fs/promises"; import { homeDirectoryFromEnvironment } from "../config/homeDirectory.js"; import { join } from "node:path"; @@ -15,7 +15,11 @@ import { readLinuxDistribution, type LinuxDistribution, } from "./LinuxHopper.js"; -import { canonicalSkillNeedsInstall } from "./SetupSkill.js"; +import { + claudeCodeSkillsDirectory, + readInstalledSkillIdentity, + type InstalledSkillIdentity, +} from "./SetupSkill.js"; import { CATALOG_IDENTITY } from "../catalogIdentity.js"; import { PRODUCT_IDENTITY, SDK_IDENTITY } from "../identity.js"; import { @@ -92,20 +96,13 @@ export interface DoctorHost { manualHopperPaths(): Promise; providerInspections?(): Promise; installationPaths?(): Promise; - installedSkillIdentity?(): Promise; + installedSkillIdentity?( + clientIds: readonly string[], + ): Promise; clientRegistrations?(): Promise; ilspyCmdVersion?(path: string): Promise; } -/** Observed skill metadata and comparison with the packaged instruction bundle. */ -interface InstalledSkillIdentity { - readonly version: string | null; - readonly toolCount: number | null; - readonly catalogDigest: string | null; - /** Whether all managed instruction and reference files match this package. */ - readonly canonical: boolean; -} - /** Structured result returned by the read-only doctor workflow. */ export interface DoctorReport { readonly healthy: boolean; @@ -143,8 +140,6 @@ interface DoctorIdentity { readonly skill: { readonly installed_version: string | null; readonly installed_tool_count: number | null; - /** Legacy catalog digest observed in older bundles; current bundles omit it. */ - readonly installed_catalog_digest: string | null; readonly state: "aligned" | "stale" | "missing"; readonly remediation: string | null; }; @@ -161,12 +156,13 @@ export const runDoctor = async ( host: DoctorHost = systemDoctorHost(), requestedScope?: DoctorScope, ): Promise => { + const scope = normalizeDoctorScope(requestedScope, target); const { checks: diagnosticChecks, hopperPath, providerInspections, } = await collectDoctorDiagnostics(target, host); - const identity = await collectDoctorIdentity(host); + const identity = await collectDoctorIdentity(host, scope); const checks = [...diagnosticChecks, ...identity.checks]; const providers = { hopperAvailable: @@ -178,7 +174,6 @@ export const runDoctor = async ( providerInspections, }; const environmentHealthy = doctorHealthy(checks, providers); - const scope = normalizeDoctorScope(requestedScope, target); const scoped = scopeDoctorChecks({ checks, registrations: identity.value.registrations, @@ -202,14 +197,20 @@ export const runDoctor = async ( const collectDoctorIdentity = async ( host: DoctorHost, + scope: DoctorScopeReport, ): Promise<{ readonly checks: readonly DoctorCheck[]; readonly value: DoctorIdentity; }> => { const installationPaths = (await host.installationPaths?.()) ?? []; - const installedSkillIdentity = await host.installedSkillIdentity?.(); - const skillAligned = skillIdentityAligned(installedSkillIdentity); const registrations = (await host.clientRegistrations?.()) ?? []; + const skillClientIds = + scope.mode === "audit-wide" + ? registrations.map(({ client }) => client) + : scope.clients; + const installedSkillIdentity = + await host.installedSkillIdentity?.(skillClientIds); + const skillAligned = skillIdentityAligned(installedSkillIdentity); return { checks: [ ...(skillAligned ? [] : [skillIdentityCheck(installedSkillIdentity)]), @@ -237,7 +238,6 @@ const collectDoctorIdentity = async ( skill: { installed_version: installedSkillIdentity?.version ?? null, installed_tool_count: installedSkillIdentity?.toolCount ?? null, - installed_catalog_digest: installedSkillIdentity?.catalogDigest ?? null, state: installedSkillIdentity === undefined ? "missing" @@ -437,7 +437,12 @@ export const systemDoctorHost = ( return []; } }, - installedSkillIdentity: () => installedSkillIdentity(homeDirectory), + installedSkillIdentity: (clientIds) => + readInstalledSkillIdentity( + homeDirectory, + clientIds, + claudeCodeSkillsDirectory(homeDirectory, environment), + ), clientRegistrations: () => readClientRegistrationStatuses(homeDirectory, undefined, { platform, @@ -502,34 +507,6 @@ const manualHopperPaths = async (home: string): Promise => { return paths; }; -const readInstalledSkill = (home: string): Promise => - readFile( - join(home, ".agents/skills", PRODUCT_IDENTITY.skillName, "SKILL.md"), - "utf8", - ); - -const installedSkillIdentity = async ( - home: string, -): Promise => { - try { - const content = await readInstalledSkill(home); - const countText = /^\s{2}tool_count:\s*(\d+)\s*$/mu.exec(content)?.[1]; - return { - canonical: !(await canonicalSkillNeedsInstall(home)), - version: /^\s{2}version:\s*"([^"]+)"\s*$/mu.exec(content)?.[1] ?? null, - toolCount: - countText === undefined ? null : Number.parseInt(countText, 10), - catalogDigest: - /^\s{2}catalog_digest:\s*"([a-f0-9]{64})"\s*$/mu.exec(content)?.[1] ?? - null, - }; - } catch (cause: unknown) { - // best-effort cleanup: optional skill probing; failure means unknown. - void cause; - return undefined; - } -}; - const uniqueLines = (value: string): string[] => [ ...new Set( value diff --git a/src/application/Setup.fixture.ts b/src/application/Setup.fixture.ts index 078239e40..b15b7940e 100644 --- a/src/application/Setup.fixture.ts +++ b/src/application/Setup.fixture.ts @@ -18,6 +18,7 @@ import { setupRegistrationCommand } from "./SetupHost.js"; export class FakeSetupHost implements SetupHost { readonly platform: NodeJS.Platform; readonly homeDirectory = "/fixture/home"; + readonly claudeCodeSkillsDirectory = "/fixture/home/.claude/skills"; readonly registrationCommand: readonly string[]; nodeVersion = "24.18.0"; version: string | undefined = "14.5"; @@ -186,7 +187,6 @@ export class FakeSetupHost implements SetupHost { skill: { installed_version: null, installed_tool_count: null, - installed_catalog_digest: null, state: this.skill === "unchanged" ? "aligned" : "missing", remediation: null, }, diff --git a/src/application/Setup.planning.test.ts b/src/application/Setup.planning.test.ts index 41b2a7178..d1ca6baea 100644 --- a/src/application/Setup.planning.test.ts +++ b/src/application/Setup.planning.test.ts @@ -4,6 +4,46 @@ import { runSetup } from "./Setup.js"; import type { SetupProgressEvent } from "./SetupTypes.js"; import { FakeSetupHost, options } from "./Setup.fixture.js"; +describe("skill plan destinations", () => { + it("plans a Claude-only skill in its personal location", async () => { + const host = new FakeSetupHost(); + host.clients = [{ name: "claude_code", configPath: "/claude.json" }]; + + const result = await runSetup( + { ...options(false), clientIds: ["claude_code"] }, + host, + ); + + expect( + result.plannedActions + .filter(({ kind }) => kind === "install_skill") + .map(({ target }) => target), + ).toEqual(["/fixture/home/.claude/skills/reverse-engineer-anything"]); + }); + + it("plans both shared and personal skill roots for mixed clients", async () => { + const host = new FakeSetupHost(); + host.clients = [ + { name: "claude_code", configPath: "/claude.json" }, + { name: "codex", configPath: "/codex.toml", format: "toml" }, + ]; + + const result = await runSetup( + { ...options(false), clientIds: ["claude_code", "codex"] }, + host, + ); + + expect( + result.plannedActions + .filter(({ kind }) => kind === "install_skill") + .map(({ target }) => target), + ).toEqual([ + "/fixture/home/.agents/skills/reverse-engineer-anything", + "/fixture/home/.claude/skills/reverse-engineer-anything", + ]); + }); +}); + describe("setup workflow", () => { it("returns a complete plan without mutation", async () => { const host = new FakeSetupHost(); diff --git a/src/application/Setup.recovery.test.ts b/src/application/Setup.recovery.test.ts index 57765ec23..95a479400 100644 --- a/src/application/Setup.recovery.test.ts +++ b/src/application/Setup.recovery.test.ts @@ -60,7 +60,7 @@ describe("setup workflow", () => { ); expect(result.status).toBe("needs_human"); expect(result.remediation).toBe( - "REA analysis skill could not be installed or verified. Check permissions for `~/.agents/skills`, then rerun setup.", + "REA analysis skill could not be installed or verified. Check permissions for its planned skill directories, then rerun setup.", ); }); @@ -194,7 +194,7 @@ describe("setup scoped readiness", () => { expect(result.status).toBe("needs_human"); expect(host.hopperInstalls).toBe(0); expect(result.remediation).toBe( - "Automated Hopper setup supports Ubuntu 24.04+, Fedora 41+, 64-bit Arch Linux, and CachyOS; configure an existing supported provider instead.", + "Automated Hopper setup supports Ubuntu 24.04+, Fedora 41+, Nobara 44+, 64-bit Arch Linux, and CachyOS; configure an existing supported provider instead.", ); }); diff --git a/src/application/Setup.ts b/src/application/Setup.ts index 2d6a0ac90..ea99de6c7 100644 --- a/src/application/Setup.ts +++ b/src/application/Setup.ts @@ -67,20 +67,31 @@ export const runSetup = async ( : (options.clientIds ?? options.readinessScope?.clients ?? discovery.defaultClientIds); + let skillNeedsInstall = await host.skillNeedsInstall(selectedClientIds); let skillSelected = options.installSkill === true || (options.installSkill !== false && selectedClientIds.length > 0); - let installSkill = skillSelected && discovery.skillNeedsInstall; + let installSkill = skillSelected && skillNeedsInstall; let planDiscovery = discovery; if (interactiveSelection) { + const selectionSkillClientIds = [ + ...new Set([ + ...discovery.defaultClientIds, + ...discovery.detectedClientIds, + ]), + ]; + const selectionSkillNeedsInstall = await host.skillNeedsInstall( + selectionSkillClientIds, + ); const offerSkillAction = - options.installSkill !== false && discovery.skillNeedsInstall; + options.installSkill !== false && selectionSkillNeedsInstall; const selectionActions = await planSetupActions({ discovery, host, providerEnvironment, command: host.registrationCommand, clientIds: [], + skillClientIds: selectionSkillClientIds, installSkill: offerSkillAction, }); const decision = await confirm(selectionActions.plannedActions, { @@ -111,6 +122,7 @@ export const runSetup = async ( selectedClientIds = [...selectedActionIds] .filter((id) => id.startsWith("configure_client:")) .map((id) => id.slice("configure_client:".length)); + skillNeedsInstall = await host.skillNeedsInstall(selectedClientIds); const proposedInstallAction = selectionActions.plannedActions.some( ({ id }) => id === "install_hopper", ); @@ -125,7 +137,7 @@ export const runSetup = async ( (selectedActionIds.has("install_skill") && selectedClientIds.length === 0) || (options.installSkill !== false && selectedClientIds.length > 0); - installSkill = skillSelected && discovery.skillNeedsInstall; + installSkill = skillSelected && skillNeedsInstall; } const planned = await planSetupActions({ discovery: planDiscovery, @@ -133,6 +145,7 @@ export const runSetup = async ( providerEnvironment, command: host.registrationCommand, clientIds: selectedClientIds, + skillClientIds: selectedClientIds, installSkill, }); plannedActions = planned.plannedActions; @@ -239,10 +252,15 @@ export const runSetup = async ( if (clientFailure !== undefined) return fail(clientFailure); if ( planSelection.installSkill && - !(await installSkillAction(host, options, appliedActions)) + !(await installSkillAction( + host, + options, + appliedActions, + selectedClientIds, + )) ) return fail( - "REA analysis skill could not be installed or verified. Check permissions for `~/.agents/skills`, then rerun setup.", + "REA analysis skill could not be installed or verified. Check permissions for its planned skill directories, then rerun setup.", ); const doctor = summarizeDoctor(await host.doctor(readinessScope)); const remediation = combineRemediation( @@ -330,6 +348,7 @@ const installSkillAction = async ( host: SetupHost, options: SetupOptions, appliedActions: string[], + skillClientIds: readonly string[], ): Promise => { const label = "REA reverse-engineering skill"; emitProgress(options, { @@ -337,7 +356,7 @@ const installSkillAction = async ( label, state: "started", }); - const skill = await host.installSkill(); + const skill = await host.installSkill(skillClientIds); if (skill === "failed") { emitProgress(options, { actionId: "install_skill", diff --git a/src/application/SetupHost.ts b/src/application/SetupHost.ts index 3e2d64e1a..068b8c012 100644 --- a/src/application/SetupHost.ts +++ b/src/application/SetupHost.ts @@ -16,6 +16,7 @@ import { import { installMacHopper, systemMacHopperInstallHost } from "./MacHopper.js"; import { supportedClients, type SetupClient } from "./SupportedClients.js"; import { + claudeCodeSkillsDirectory as resolveClaudeCodeSkillsDirectory, canonicalSkillNeedsInstall, installCanonicalSkill, } from "./SetupSkill.js"; @@ -78,7 +79,7 @@ export const hostRemediation = async ( : undefined; } if ((await host.linuxDistribution())?.supported === true) return undefined; - return "Automated Hopper setup supports Ubuntu 24.04+, Fedora 41+, 64-bit Arch Linux, and CachyOS; configure an existing supported provider instead."; + return "Automated Hopper setup supports Ubuntu 24.04+, Fedora 41+, Nobara 44+, 64-bit Arch Linux, and CachyOS; configure an existing supported provider instead."; }; /** Production setup effects for Hopper, agent configuration, and the canonical skill directory. */ @@ -89,9 +90,14 @@ export const systemSetupHost = ( const doctorHost = selectedDoctorHost ?? systemDoctorHost({ environment }); const platform = doctorHost.platform; const { homeDirectory } = doctorHost; + const claudeSkillsDirectory = resolveClaudeCodeSkillsDirectory( + homeDirectory, + environment, + ); return { platform, homeDirectory, + claudeCodeSkillsDirectory: claudeSkillsDirectory, registrationCommand: setupRegistrationCommand( platform, environment.npm_command === "exec", @@ -143,8 +149,14 @@ export const systemSetupHost = ( (aligned) => !aligned, ), inspectClientConfiguration: inspectClientConfiguration, - skillNeedsInstall: () => canonicalSkillNeedsInstall(homeDirectory), - installSkill: () => installCanonicalSkill(homeDirectory), + skillNeedsInstall: (clientIds) => + canonicalSkillNeedsInstall( + homeDirectory, + clientIds, + claudeSkillsDirectory, + ), + installSkill: (clientIds) => + installCanonicalSkill(homeDirectory, clientIds, claudeSkillsDirectory), doctor: (scope) => runDoctor(undefined, doctorHost, scope), }; }; diff --git a/src/application/SetupPlan.ts b/src/application/SetupPlan.ts index 87b741309..fc0d978a3 100644 --- a/src/application/SetupPlan.ts +++ b/src/application/SetupPlan.ts @@ -1,11 +1,11 @@ import { join } from "node:path"; -import { PRODUCT_IDENTITY } from "../identity.js"; import { isOwnedClientRegistrationCommand } from "./ClientRegistrationIdentity.js"; import { linuxHopperInstallDisclosure, type LinuxPackageFamily, } from "./LinuxHopper.js"; +import { skillDestinations } from "./SetupSkill.js"; import { macHopperInstallDisclosure } from "./MacHopper.js"; import type { SetupAction, @@ -20,7 +20,6 @@ import type { DoctorReport } from "./Doctor.js"; export interface SetupDiscovery { readonly initialDoctor: Awaited>; readonly installHopper: boolean; - readonly skillNeedsInstall: boolean; readonly clients: readonly SetupClient[]; readonly detectedClientIds: readonly string[]; readonly defaultClientIds: readonly string[]; @@ -57,10 +56,9 @@ export const discoverSetupState = async (input: { input.proposeHopper && (Object.keys(input.providerEnvironment).length === 0 || linuxHopperRepairNeeded)); - const [clients, detectedClients, skillNeedsInstall] = await Promise.all([ + const [clients, detectedClients] = await Promise.all([ input.host.supportedClients(), input.host.detectedClients(), - input.host.skillNeedsInstall(), ]); const detectedIds = new Set(detectedClients.map(({ name }) => name)); const registrations = initialDoctor.identity?.registrations ?? []; @@ -100,7 +98,6 @@ export const discoverSetupState = async (input: { return { initialDoctor, installHopper, - skillNeedsInstall, clients, detectedClientIds: [...detectedIds], defaultClientIds, @@ -118,6 +115,7 @@ export const planSetupActions = async (input: { readonly providerEnvironment: SetupProviderEnvironment; readonly command: readonly string[]; readonly clientIds: readonly string[]; + readonly skillClientIds: readonly string[]; readonly installSkill: boolean; }): Promise<{ readonly plannedActions: readonly SetupAction[]; @@ -189,6 +187,8 @@ export const planSetupActions = async (input: { homeDirectory: input.host.homeDirectory, installHopper: input.discovery.installHopper, installSkill: input.installSkill, + skillClientIds: input.skillClientIds, + claudeCodeSkillsDirectory: input.host.claudeCodeSkillsDirectory, clients: clientPlans, providerEnvironment: input.providerEnvironment, ...(input.discovery.linuxPackageFamily === undefined @@ -209,6 +209,8 @@ const setupPlan = (input: { readonly homeDirectory: string; readonly installHopper: boolean; readonly installSkill: boolean; + readonly skillClientIds: readonly string[]; + readonly claudeCodeSkillsDirectory: string; readonly clients: readonly { readonly client: SetupClient; readonly operation: "create" | "update"; @@ -266,22 +268,28 @@ const setupPlan = (input: { ...(backupPath === undefined ? {} : { backupPath }), })), ...(input.installSkill - ? [ - { - id: "install_skill", - kind: "install_skill" as const, - label: "REA reverse-engineering skill", - target: join( - input.homeDirectory, - ".agents/skills", - PRODUCT_IDENTITY.skillName, - ), - detail: - "Install or update the bundled REA reverse-engineering skill and on-demand references.", - external: false, - operation: "install" as const, - }, - ] + ? skillDestinations( + input.homeDirectory, + input.skillClientIds, + input.claudeCodeSkillsDirectory, + ).map(({ client, path }): SetupAction => ({ + id: + client === "claude_code" + ? "install_skill:claude_code" + : "install_skill", + kind: "install_skill", + label: + client === "claude_code" + ? "REA skill for Claude Code" + : "REA reverse-engineering skill", + target: path, + detail: + client === "claude_code" + ? "Install or update the REA skill in Claude Code's personal skill directory." + : "Install or update the bundled REA reverse-engineering skill and on-demand references.", + external: false, + operation: "install", + })) : []), ]; diff --git a/src/application/SetupSkill.ts b/src/application/SetupSkill.ts index 346a1ace7..19a306f9a 100644 --- a/src/application/SetupSkill.ts +++ b/src/application/SetupSkill.ts @@ -19,8 +19,57 @@ interface CanonicalSkillFile { readonly original: string | undefined; } -const skillRoot = (home: string): string => - join(home, ".agents/skills", PRODUCT_IDENTITY.skillName); +/** Managed skill location selected for a client family. */ +export interface SkillDestination { + readonly client: "shared" | "claude_code"; + readonly path: string; +} + +/** Metadata and byte alignment observed in selected skill installations. */ +export interface InstalledSkillIdentity { + readonly version: string | null; + readonly toolCount: number | null; + readonly canonical: boolean; +} + +/** Resolve Claude Code personal skills from the selected environment. */ +export const claudeCodeSkillsDirectory = ( + home: string, + environment: { readonly CLAUDE_CONFIG_DIR?: string } = {}, +): string => + join(environment.CLAUDE_CONFIG_DIR ?? join(home, ".claude"), "skills"); + +/** Resolve the skill roots required by a client selection, or all owned roots. */ +export const skillDestinations = ( + home: string, + clientIds: readonly string[] | undefined, + claudeSkillsDirectory = claudeCodeSkillsDirectory(home), +): readonly SkillDestination[] => { + const includeClaude = + clientIds === undefined || clientIds.includes("claude_code"); + const includeShared = + clientIds === undefined || + clientIds.length === 0 || + clientIds.some((clientId) => clientId !== "claude_code"); + return [ + ...(includeShared + ? [ + { + client: "shared" as const, + path: join(home, ".agents/skills", PRODUCT_IDENTITY.skillName), + }, + ] + : []), + ...(includeClaude + ? [ + { + client: "claude_code" as const, + path: join(claudeSkillsDirectory, PRODUCT_IDENTITY.skillName), + }, + ] + : []), + ]; +}; const readOptionalText = async (path: string): Promise => { try { @@ -34,32 +83,39 @@ const readOptionalText = async (path: string): Promise => { const canonicalSkillFiles = async ( home: string, + clientIds: readonly string[], + claudeSkillsDirectory?: string, ): Promise => Promise.all( - SKILL_FILES.map(async (relativePath) => { - const destination = join(skillRoot(home), relativePath); - return { - destination, - content: await readFile( - new URL( - `../../skills/${PRODUCT_IDENTITY.skillName}/${relativePath}`, - import.meta.url, - ), - "utf8", - ), - original: await readOptionalText(destination), - }; - }), + skillDestinations(home, clientIds, claudeSkillsDirectory).flatMap( + ({ path: root }) => + SKILL_FILES.map(async (relativePath) => { + const destination = join(root, relativePath); + return { + destination, + content: await readFile( + new URL( + `../../skills/${PRODUCT_IDENTITY.skillName}/${relativePath}`, + import.meta.url, + ), + "utf8", + ), + original: await readOptionalText(destination), + }; + }), + ), ); /** Report whether setup would change any file in the managed REA skill bundle. */ export const canonicalSkillNeedsInstall = async ( home: string, + clientIds: readonly string[] = [], + claudeSkillsDirectory?: string, ): Promise => { try { - return (await canonicalSkillFiles(home)).some( - ({ content, original }) => original !== content, - ); + return ( + await canonicalSkillFiles(home, clientIds, claudeSkillsDirectory) + ).some(({ content, original }) => original !== content); } catch (cause: unknown) { // Unreadable skill state fails open to install so setup can repair it. void cause; @@ -82,10 +138,16 @@ const restoreSkillFiles = async ( /** Transactionally install or upgrade the canonical REA skill and references. */ export const installCanonicalSkill = async ( home: string, + clientIds: readonly string[] = [], + claudeSkillsDirectory?: string, ): Promise<"installed" | "unchanged" | "failed"> => { let changed: readonly CanonicalSkillFile[] = []; try { - const canonical = await canonicalSkillFiles(home); + const canonical = await canonicalSkillFiles( + home, + clientIds, + claudeSkillsDirectory, + ); changed = canonical.filter(({ content, original }) => original !== content); if (changed.length === 0) return "unchanged"; @@ -112,3 +174,37 @@ export const installCanonicalSkill = async ( return "failed"; } }; + +/** Read installed skill metadata and compare every managed location. */ +export const readInstalledSkillIdentity = async ( + home: string, + clientIds: readonly string[], + claudeSkillsDirectory?: string, +): Promise => { + const destinations = skillDestinations( + home, + clientIds, + claudeSkillsDirectory, + ); + let content: string | undefined; + for (const destination of destinations) { + try { + content = await readFile(join(destination.path, "SKILL.md"), "utf8"); + break; + } catch { + // Another selected copy may provide metadata; canonical validation below + // still marks missing or unreadable copies as stale. + } + } + if (content === undefined) return undefined; + const toolCount = /^\s{2}tool_count:\s*(\d+)\s*$/mu.exec(content)?.[1]; + return { + canonical: !(await canonicalSkillNeedsInstall( + home, + clientIds, + claudeSkillsDirectory, + )), + version: /^\s{2}version:\s*"([^"]+)"\s*$/mu.exec(content)?.[1] ?? null, + toolCount: toolCount === undefined ? null : Number.parseInt(toolCount, 10), + }; +}; diff --git a/src/application/SetupTypes.ts b/src/application/SetupTypes.ts index 75aed01bd..08eb0df99 100644 --- a/src/application/SetupTypes.ts +++ b/src/application/SetupTypes.ts @@ -42,6 +42,7 @@ export type ClientConfigurationInspection = export interface SetupHost { readonly platform: NodeJS.Platform; readonly homeDirectory: string; + readonly claudeCodeSkillsDirectory: string; readonly registrationCommand: readonly string[]; readonly nodeVersion: string; macosVersion(): Promise; @@ -65,8 +66,10 @@ export interface SetupHost { providerEnvironment: SetupProviderEnvironment, command: readonly string[], ): Promise; - skillNeedsInstall(): Promise; - installSkill(): Promise<"installed" | "unchanged" | "failed">; + skillNeedsInstall(clientIds: readonly string[]): Promise; + installSkill( + clientIds: readonly string[], + ): Promise<"installed" | "unchanged" | "failed">; doctor(scope?: DoctorScope): Promise>>; } /** Structured setup outcome carrying remediation instead of prompting. */ diff --git a/src/application/Uninstall.ts b/src/application/Uninstall.ts index f05224adb..9848e91da 100644 --- a/src/application/Uninstall.ts +++ b/src/application/Uninstall.ts @@ -15,6 +15,7 @@ import writeFileAtomic from "write-file-atomic"; import { z } from "zod"; import { PRODUCT_IDENTITY } from "../identity.js"; +import { claudeCodeSkillsDirectory, skillDestinations } from "./SetupSkill.js"; import { isOwnedClientRegistrationCommand } from "./ClientRegistrationIdentity.js"; import { resolveClientConfigTransactionPath } from "./ClientConfigPath.js"; import { @@ -178,7 +179,7 @@ export const systemUninstallHost = ( : undefined; }, removeClient: (client) => removeClient(client, fileSystem), - removeSkill: () => removeManagedSkills(home, fileSystem), + removeSkill: () => removeManagedSkills(home, fileSystem, environment), purgeData: async () => [ await removeManagedPath(join(home, ".rea/cache"), "cache", fileSystem), await removeManagedPath(join(home, ".rea/state"), "state", fileSystem), @@ -402,12 +403,48 @@ const removeManagedPath = async ( const removeManagedSkills = async ( home: string, fileSystem: UninstallFileSystem, -): Promise => - removeManagedPath( - join(home, ".agents/skills", PRODUCT_IDENTITY.skillName), - "skill", - fileSystem, + environment: Readonly, +): Promise => { + const results = await Promise.all( + skillDestinations( + home, + undefined, + claudeCodeSkillsDirectory(home, environment), + ).map(({ client, path }) => + removeManagedPath( + path, + client === "claude_code" ? "Claude Code skill" : "skill", + fileSystem, + ), + ), ); + const failed = results.find(({ status }) => status === "failed"); + if (failed !== undefined) + return item( + "skill", + "failed", + results + .map(({ name, status, detail }) => `${name}: ${status} (${detail})`) + .join(" "), + ); + const status = results.some( + ({ status: resultStatus }) => resultStatus === "removed", + ) + ? "removed" + : results.some(({ status: resultStatus }) => resultStatus === "retained") + ? "retained" + : "skipped"; + return item( + "skill", + status, + results + .map( + ({ name, status: resultStatus, detail }) => + `${name}: ${resultStatus} (${detail})`, + ) + .join(" "), + ); +}; const item = ( name: string, diff --git a/tests/boundary/filesystem/clientLifecycle.test.ts b/tests/boundary/filesystem/clientLifecycle.test.ts index 0a649610b..88eab9953 100644 --- a/tests/boundary/filesystem/clientLifecycle.test.ts +++ b/tests/boundary/filesystem/clientLifecycle.test.ts @@ -1629,14 +1629,41 @@ describe("client configuration filesystem failure reporting", () => { remove: () => Promise.reject(new Error("SECRET removal failure")), }), ); - expect(result.items).toContainEqual({ - name: "skill", - status: "failed", - detail: - "This item could not be removed. Check file permissions, then rerun uninstall.", - }); + expect(result.items).toContainEqual( + expect.objectContaining({ + name: "skill", + status: "failed", + detail: expect.stringContaining( + "This item could not be removed. Check file permissions, then rerun uninstall.", + ), + }), + ); expect(JSON.stringify(result)).not.toContain("SECRET"); }); + + it("uninstalls all managed skill roots, including a custom Claude Code directory", async () => { + const home = await createTestTempDirectory("rea-uninstall-skills-"); + roots.push(home); + const customClaudeDirectory = join(home, "claude-config"); + const rootsToRemove = [ + join(home, ".agents/skills/reverse-engineer-anything"), + join(customClaudeDirectory, "skills/reverse-engineer-anything"), + ]; + await Promise.all( + rootsToRemove.map((path) => mkdir(path, { recursive: true })), + ); + + const result = await systemUninstallHost(home, testFileSystem, { + CLAUDE_CONFIG_DIR: customClaudeDirectory, + }).removeSkill(); + + expect(result).toMatchObject({ status: "removed" }); + await Promise.all( + rootsToRemove.map(async (path) => { + await expect(lstat(path)).rejects.toMatchObject({ code: "ENOENT" }); + }), + ); + }); }); const testFileSystem: UninstallFileSystem = { diff --git a/tests/boundary/filesystem/setupSkill.test.ts b/tests/boundary/filesystem/setupSkill.test.ts index e90adeb1b..0d1caa93a 100644 --- a/tests/boundary/filesystem/setupSkill.test.ts +++ b/tests/boundary/filesystem/setupSkill.test.ts @@ -1,11 +1,19 @@ import { createHash } from "node:crypto"; -import { mkdir, readFile, rm, writeFile } from "node:fs/promises"; +import { + access, + chmod, + mkdir, + readFile, + rm, + writeFile, +} from "node:fs/promises"; import { dirname, join } from "node:path"; import { describe, expect, it } from "vitest"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { canonicalSkillNeedsInstall, + claudeCodeSkillsDirectory, installCanonicalSkill, } from "../../../src/application/SetupSkill.js"; import { @@ -18,6 +26,127 @@ import { PRODUCT_IDENTITY } from "../../../src/identity.js"; import { z } from "zod"; import { skillReferenceIssues } from "../../../scripts/lib/docs-facts.mjs"; +describe("canonical skill transaction", () => { + it.skipIf(process.platform === "win32" || process.getuid?.() === 0)( + "preserves an unreadable existing skill instead of treating it as absent", + async () => { + const home = await createTestTempDirectory("rea-skill-unreadable-"); + const destination = join( + home, + ".agents/skills/reverse-engineer-anything/SKILL.md", + ); + const original = "existing private skill bytes\n"; + await mkdir(dirname(destination), { recursive: true }); + await writeFile(destination, original, { mode: 0o200 }); + try { + await chmod(destination, 0o200); + expect(await installCanonicalSkill(home)).toBe("failed"); + } finally { + await chmod(destination, 0o600); + } + expect(await readFile(destination, "utf8")).toBe(original); + await expect( + readFile(`${destination}.rea.backup`, "utf8"), + ).rejects.toMatchObject({ + code: "ENOENT", + }); + }, + ); + + it("installs only the selected Claude Code personal skill", async () => { + const home = await createTestTempDirectory("rea-claude-skill-"); + const configDirectory = join(home, "claude-config"); + const skillsDirectory = claudeCodeSkillsDirectory(home, { + CLAUDE_CONFIG_DIR: configDirectory, + }); + const claudeSkill = join( + skillsDirectory, + "reverse-engineer-anything/SKILL.md", + ); + await mkdir(configDirectory, { recursive: true }); + await writeFile( + join(configDirectory, ".claude.json"), + JSON.stringify({ + mcpServers: { + rea: { + command: "npx", + args: ["-y", PRODUCT_IDENTITY.registrationPackageSpecifier, "mcp"], + }, + }, + }), + ); + const host = systemDoctorHost({ + environment: { + HOME: home, + USERPROFILE: home, + CLAUDE_CONFIG_DIR: configDirectory, + }, + }); + + expect( + await canonicalSkillNeedsInstall(home, ["claude_code"], skillsDirectory), + ).toBe(true); + expect( + await installCanonicalSkill(home, ["claude_code"], skillsDirectory), + ).toBe("installed"); + expect( + await canonicalSkillNeedsInstall(home, ["claude_code"], skillsDirectory), + ).toBe(false); + await expect(access(claudeSkill)).resolves.toBeUndefined(); + await expect( + access(join(home, ".agents/skills/reverse-engineer-anything")), + ).rejects.toMatchObject({ code: "ENOENT" }); + expect( + ( + await runDoctor(undefined, host, { + clients: ["claude_code"], + skill: true, + }) + ).identity?.skill.state, + ).toBe("aligned"); + }); + + it("checks selected skill locations for independent drift", async () => { + const home = await createTestTempDirectory("rea-mixed-skill-"); + const sharedSkill = join( + home, + ".agents/skills/reverse-engineer-anything/SKILL.md", + ); + const claudeSkills = claudeCodeSkillsDirectory(home); + const claudeSkill = join( + claudeSkills, + "reverse-engineer-anything/SKILL.md", + ); + const host = systemDoctorHost({ + environment: { HOME: home, USERPROFILE: home }, + }); + const doctorScope = { clients: ["claude_code", "codex"], skill: true }; + + expect(await installCanonicalSkill(home, ["claude_code", "codex"])).toBe( + "installed", + ); + await rm(sharedSkill); + expect( + (await runDoctor(undefined, host, doctorScope)).identity?.skill.state, + ).toBe("stale"); + expect(await installCanonicalSkill(home, ["claude_code", "codex"])).toBe( + "installed", + ); + for (const skill of [sharedSkill, claudeSkill]) { + await writeFile(skill, "selected copy changed\n"); + expect( + (await runDoctor(undefined, host, doctorScope)).identity?.skill.state, + ).toBe("stale"); + expect(await installCanonicalSkill(home, ["claude_code", "codex"])).toBe( + "installed", + ); + expect( + (await runDoctor(undefined, host, doctorScope)).identity?.skill.state, + ).toBe("aligned"); + } + }); +}); + describe("canonical skill transaction", () => { it("backs up and upgrades a stale managed skill without touching siblings", async () => { const home = await createTestTempDirectory("rea-skill-test-"); @@ -90,7 +219,6 @@ describe("canonical skill transaction", () => { expect(installedSkill).toContain( `tool_count: ${String(TOOL_CONTRACTS.length)}`, ); - expect(installedSkill).not.toContain("catalog_digest:"); expect(await readFile(sibling, "utf8")).toBe("unrelated skill\n"); expect( await readFile( @@ -160,8 +288,9 @@ it("doctor verifies installed instructions and references even when metadata is environment: { HOME: home, USERPROFILE: home }, }); const host = createDoctorHostFixture({ - installedSkillIdentity: () => - system.installedSkillIdentity?.() ?? Promise.resolve(undefined), + installedSkillIdentity: (registrations) => + system.installedSkillIdentity?.(registrations) ?? + Promise.resolve(undefined), }); expect((await runDoctor(undefined, host)).identity?.skill.state).toBe( "missing", @@ -169,24 +298,14 @@ it("doctor verifies installed instructions and references even when metadata is expect(await installCanonicalSkill(home)).toBe("installed"); expect((await runDoctor(undefined, host)).identity?.skill).toMatchObject({ state: "aligned", - installed_catalog_digest: null, }); - const legacyDigest = "0".repeat(64); - await writeFile( - destination, - (await readFile(destination, "utf8")).replace( - ` tool_count: ${String(TOOL_CONTRACTS.length)}`, - ` tool_count: ${String(TOOL_CONTRACTS.length)}\n catalog_digest: "${legacyDigest}"`, - ), - ); - expect((await runDoctor(undefined, host)).identity?.skill).toMatchObject({ - state: "stale", - installed_catalog_digest: legacyDigest, - }); - expect(await installCanonicalSkill(home)).toBe("installed"); - for (const path of [destination, reference]) { - const canonical = await readFile(path, "utf8"); - await writeFile(path, `${canonical}\nLocally changed instructions.\n`); + for (const [path, remove] of [ + [destination, false], + [reference, false], + [reference, true], + ] as const) { + if (remove) await rm(path); + else await writeFile(path, "Locally changed instructions.\n"); expect((await runDoctor(undefined, host)).identity?.skill.state).toBe( "stale", ); @@ -195,12 +314,4 @@ it("doctor verifies installed instructions and references even when metadata is "aligned", ); } - await rm(reference); - expect((await runDoctor(undefined, host)).identity?.skill.state).toBe( - "stale", - ); - expect(await installCanonicalSkill(home)).toBe("installed"); - expect((await runDoctor(undefined, host)).identity?.skill.state).toBe( - "aligned", - ); });