diff --git a/docs/client-configuration.md b/docs/client-configuration.md index 38af2e6d74..a898f3ae59 100644 --- a/docs/client-configuration.md +++ b/docs/client-configuration.md @@ -334,7 +334,11 @@ Setup, doctor and uninstall follow the root's `active_profile` selection, as plain Hermes CLI commands do. An explicit `HERMES_HOME` pointing to `profiles/` selects that named profile directly. An invalid, missing or deleted sticky profile is reported instead of writing into the default home; -repair the profile or run `hermes profile use default` before retrying. +it also remains an unresolved skill selection, so setup cannot report a +successful empty installation. Overrides nested beneath the native Hermes home +use that native home's sticky selection and profile root together, matching +[Hermes's root rules](https://github.com/NousResearch/hermes-agent/blob/748c79a1c6873124d84071cdabb4062a75913e1f/hermes_constants.py#L83). +Repair the profile or run `hermes profile use default` before retrying. REA follows `HERMES_HOME` whitespace trimming, environment-variable expansion, and home expansion before resolving that profile. Setup previews the resolved configuration and skill destinations; doctor and uninstall use those same paths. @@ -495,3 +499,19 @@ npx -y rea-agents@ mcp Do not put credentials in that command or its arguments. `rea doctor --client grok_bot` reports this manual step. `rea uninstall` does not remove the account connector. + +## Skill selection and maintenance + +With no client selected, the interactive skill choice targets the shared +`~/.agents/skills` copy. Selecting a client targets its personal destination +when supported. Use `rea setup --skill-client shared --skill` for the shared +copy, or `--skill-client claude_code --skill` for that client's copy. Setup +resolves the final selection before deciding which files need installation. +Doctor retains each selected stale or missing path and names the matching +repair command; a current copy elsewhere does not establish freshness. + +Unresolved profile or directory selections remain diagnostics and never become +write targets. Update maintenance considers only known enabled, owned REA +registrations and owned skills. If identity or a selected skill root is +unresolved, it reports maintenance as unavailable with the known scope and +remaining diagnostics. An empty writable scope alone does not prove current. diff --git a/src/application/Doctor.test.ts b/src/application/Doctor.test.ts index 1372a632ed..683634ed08 100644 --- a/src/application/Doctor.test.ts +++ b/src/application/Doctor.test.ts @@ -582,6 +582,47 @@ describe("doctor", () => { }); describe("doctor installation identity", () => { + it("names each stale or missing skill destination and its exact repair selection", async () => { + const result = await runDoctor( + undefined, + host({ + installedSkillIdentity: () => + Promise.resolve({ + version: "current-first-copy", + toolCount: 0, + canonical: false, + issues: [ + { + client: "claude_code", + path: "/home/.claude/skills/rea", + kind: "stale", + detail: "Selected skill copy is stale", + }, + { + client: "shared", + path: "/home/.agents/skills/rea", + kind: "missing", + detail: "Selected skill copy is missing", + }, + ], + }), + }), + ); + expect(result.identity?.skill).toMatchObject({ state: "stale" }); + expect(result.identity?.skill.remediation).toContain( + "rea setup --skill-client claude_code --skill", + ); + expect(result.identity?.skill.remediation).toContain( + "rea setup --skill-client shared --skill", + ); + expect(result.identity?.skill.remediation).toContain( + "/home/.claude/skills/rea", + ); + expect(result.identity?.skill.remediation).toContain( + "/home/.agents/skills/rea", + ); + }); + it("distinguishes stale skills, path shadowing, and unobservable live state", async () => { const result = await runDoctor( undefined, diff --git a/src/application/Doctor.ts b/src/application/Doctor.ts index 6b828c2b0a..143729dbf5 100644 --- a/src/application/Doctor.ts +++ b/src/application/Doctor.ts @@ -428,9 +428,21 @@ const skillIdentityAligned = ( const skillIdentityState = ( identity: InstalledSkillIdentity | undefined, ): DoctorSkillState => { - if (identity?.issues?.some(({ kind }) => kind === "unreadable")) + if ( + identity?.issues?.some( + ({ kind }) => kind === "unreadable" || kind === "unresolved", + ) + ) return "unknown"; - if ((identity?.issues?.length ?? 0) > 0) return "unmanaged"; + if ( + identity?.issues?.some( + ({ kind }) => kind === "symbolic_link" || kind === "not_directory", + ) + ) + return "unmanaged"; + if (identity?.issues?.some(({ kind }) => kind === "stale")) return "stale"; + if (identity?.issues?.some(({ kind }) => kind === "missing")) + return "missing"; return identity === undefined ? "missing" : skillIdentityAligned(identity) @@ -443,7 +455,15 @@ const skillIdentityRemediation = ( ): string => { const issues = identity?.issues; if (issues !== undefined && issues.length > 0) - return `${issues.map(({ path, detail }) => `${detail} (${path})`).join(" ")} Verify the reported paths and use regular skill directories, or run rea setup with --skill=false to configure the client without installing the skill.`; + return issues + .map(({ path, detail, kind, client }) => { + const repair = + kind === "stale" || kind === "missing" + ? `Run rea setup --skill-client ${client ?? "shared"} --skill to repair this copy.` + : "Repair the selected path/profile before rerunning setup, or use --skill=false to configure clients without installing the skill."; + return `${detail} (${path}). ${repair}`; + }) + .join(" "); return "Run rea setup to update the installed REA skill."; }; diff --git a/src/application/HermesClientHome.ts b/src/application/HermesClientHome.ts index 96861aa5e4..11e7f1b6d3 100644 --- a/src/application/HermesClientHome.ts +++ b/src/application/HermesClientHome.ts @@ -168,7 +168,9 @@ export const resolveHermesClientHome = ( join(stickyRoot, "active_profile"), `Invalid saved Hermes profile ${JSON.stringify(name)}. Run 'hermes profile use default' or repair active_profile.`, ); - const selected = join(root, "profiles", canonical); + // Hermes resolves both sticky selection and named profiles from its default + // root (including overrides beneath the native home), never mixed roots. + const selected = join(stickyRoot, "profiles", canonical); if (!liveProfile(selected)) throw new HermesClientPathError( selected, diff --git a/src/application/Setup.configuration.test.ts b/src/application/Setup.configuration.test.ts index b4293abb08..3c073231ce 100644 --- a/src/application/Setup.configuration.test.ts +++ b/src/application/Setup.configuration.test.ts @@ -286,6 +286,38 @@ describe("configured Linux Hopper selection", () => { }); }); +it("offers shared skill intent when the detected personal copy is current", async () => { + const host = new FakeSetupHost(); + host.hopper = "/Applications/Hopper"; + host.clients = [{ name: "claude_code", configPath: "/claude.json" }]; + host.skillNeedsInstall = (clients) => Promise.resolve(clients.length === 0); + const result = await runSetup( + { ...options(false), structured: false }, + host, + async (actions, context) => { + if (context?.stage !== "select") return true; + expect(actions).toContainEqual( + expect.objectContaining({ id: "install_skill" }), + ); + return { approved: false, selectedActionIds: ["install_skill"] }; + }, + ); + expect(result.status).toBe("ready"); + expect(result.plannedActions).toEqual([ + expect.objectContaining({ + id: "install_skill", + target: join( + host.homeDirectory, + ".agents", + "skills", + "reverse-engineer-anything", + ), + }), + ]); + expect(host.configurations).toBe(0); + expect(host.skillInstalls).toBe(1); +}); + describe("setup workflow action selection", () => { it("executes only actions selected by the interactive adapter", async () => { const host = new FakeSetupHost(); diff --git a/src/application/Setup.fixture.ts b/src/application/Setup.fixture.ts index 1836a53073..4f42035eb0 100644 --- a/src/application/Setup.fixture.ts +++ b/src/application/Setup.fixture.ts @@ -141,7 +141,7 @@ export class FakeSetupHost implements SetupHost { Promise.resolve( this.clientInspections.get(client.name) ?? { status: "update" }, ); - skillNeedsInstall = (): Promise => + skillNeedsInstall: SetupHost["skillNeedsInstall"] = (): Promise => Promise.resolve(this.skill !== "unchanged"); validateSkillDirectories = ( clientIds: readonly string[], diff --git a/src/application/Setup.planning.test.ts b/src/application/Setup.planning.test.ts index ace77e337f..527f213856 100644 --- a/src/application/Setup.planning.test.ts +++ b/src/application/Setup.planning.test.ts @@ -105,7 +105,7 @@ describe("selected skill admission", () => { { decision: "the offered skill ID", acceptAll: false }, { decision: "accept all", acceptAll: true }, ])( - "keeps a CLI-only skill choice offered under a client-specific ID ($decision)", + "keeps a CLI-only skill choice independent of detected personal copies ($decision)", async ({ acceptAll }) => { const host = new FakeSetupHost(); host.hopper = "/Applications/Hopper"; @@ -127,7 +127,7 @@ describe("selected skill admission", () => { return true; }, ); - expect(offered).toEqual(["install_skill:claude_code"]); + expect(offered).toEqual(["install_skill"]); expect(result.status).toBe("ready"); expect( result.plannedActions.map(({ id, target }) => ({ id, target })), diff --git a/src/application/Setup.ts b/src/application/Setup.ts index aed4a7dada..b92fd3323b 100644 --- a/src/application/Setup.ts +++ b/src/application/Setup.ts @@ -154,17 +154,10 @@ const runSetupWorkflow = async ( 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 && selectionSkillNeedsInstall; + // The skill choice is intent; its final destination is determined after + // selecting clients. An empty client selection names the shared CLI copy. + const selectionSkillClientIds: readonly string[] = []; + const offerSkillAction = options.installSkill !== false; const selectionActions = await planSetupActions({ discovery, host, diff --git a/src/application/SetupPlan.ts b/src/application/SetupPlan.ts index 7a8405c123..f15ff531be 100644 --- a/src/application/SetupPlan.ts +++ b/src/application/SetupPlan.ts @@ -1,3 +1,4 @@ +import { SetupSkillInstallError, type SkillDestination } from "./SetupSkill.js"; import { classifyClientRegistrationCommand } from "./ClientRegistrationIdentity.js"; import type { LinuxPackageFamily } from "./HopperReleaseFeed.js"; import { @@ -5,7 +6,6 @@ import { LINUX_HOPPER_SETUP_HOSTS, type LinuxDistribution, } from "./LinuxHopper.js"; -import type { SkillDestination } from "./SetupSkill.js"; import { macHopperInstallDisclosure } from "./MacHopper.js"; import { macHopperUserApplicationPath } from "./MacHopperPaths.js"; import type { @@ -211,6 +211,19 @@ export const planSetupActions = async (input: { blocker: `${invalid.client.displayName ?? invalid.client.name}: ${invalid.inspection.remediation}`, }; } + let skillDestinations: readonly SkillDestination[] = []; + try { + if (input.installSkill) + skillDestinations = input.host.skillDestinations(input.skillClientIds); + } catch (cause: unknown) { + if (!(cause instanceof SetupSkillInstallError)) throw cause; + return { + plannedActions: [], + selectedClients, + clientsToConfigure: [], + blocker: cause.remediation, + }; + } const clientPlans = plannedClientWrites( inspections, input.discovery.installHopper, @@ -224,7 +237,7 @@ export const planSetupActions = async (input: { installHopper: input.discovery.installHopper, installSkill: input.installSkill, skillClientIds: input.skillClientIds, - skillDestinations: input.host.skillDestinations(input.skillClientIds), + skillDestinations, clients: clientPlans, providerEnvironment: input.providerEnvironment, ...(input.discovery.linuxPackageFamily === undefined diff --git a/src/application/SetupSkill.ts b/src/application/SetupSkill.ts index d1137214e9..f7f2723b80 100644 --- a/src/application/SetupSkill.ts +++ b/src/application/SetupSkill.ts @@ -173,23 +173,64 @@ export interface InstalledSkillIdentity { /** Known management blockers and unresolved filesystem observations. */ export interface SkillPathIssue { readonly path: string; - readonly kind: "symbolic_link" | "not_directory" | "unreadable"; + readonly kind: + | "symbolic_link" + | "not_directory" + | "unreadable" + | "unresolved" + | "stale" + | "missing"; + readonly client?: string; readonly detail: string; } -/** Resolve the skill roots required by a client selection, or all owned roots. */ +/** Keep unresolved selected roots as diagnostics, never as writable destinations. */ +export const skillDestinationSelection = ( + home: string, + clientIds: readonly string[] | undefined, + environment: Readonly = {}, + platform: NodeJS.Platform = process.platform, +): { + readonly destinations: readonly SkillDestination[]; + readonly issues: readonly SkillPathIssue[]; +} => { + const selection = clientSkillDirectories( + home, + clientIds, + environment, + platform, + ); + return { + destinations: selection.directories.map(({ client, directory }) => ({ + client, + path: join(directory, PRODUCT_IDENTITY.skillName), + })), + issues: selection.unresolved.map(({ client, path, detail }) => ({ + client, + path, + detail, + kind: "unresolved", + })), + }; +}; + +/** Writable destinations are available only after all selected roots resolve. */ export const skillDestinations = ( home: string, clientIds: readonly string[] | undefined, environment: Readonly = {}, platform: NodeJS.Platform = process.platform, -): readonly SkillDestination[] => - clientSkillDirectories(home, clientIds, environment, platform).map( - ({ client, directory }) => ({ - client, - path: join(directory, PRODUCT_IDENTITY.skillName), - }), +): readonly SkillDestination[] => { + const selection = skillDestinationSelection( + home, + clientIds, + environment, + platform, ); + if (selection.issues.length > 0) + throw skillDirectoryFailure(selection.issues); + return selection.destinations; +}; /** Resolve existing REA-owned skill roots using setup's canonical destinations. */ export const existingSkillDestinations = async ( @@ -197,19 +238,26 @@ export const existingSkillDestinations = async ( environment: Readonly = {}, platform: NodeJS.Platform = process.platform, resources?: ArtifactResourceScope, -): Promise => { - const existing: SkillDestination[] = []; - for (const destination of skillDestinations( +): Promise<{ + readonly destinations: readonly SkillDestination[]; + readonly issues: readonly SkillPathIssue[]; +}> => { + const selection = skillDestinationSelection( home, undefined, environment, platform, - )) { + ); + const issues: SkillPathIssue[] = [...selection.issues]; + const existing: SkillDestination[] = []; + for (const destination of selection.destinations) { try { - const issues = await skillDirectoryIssues([destination]); - if (issues.some(({ kind }) => kind === "unreadable")) - throw skillDirectoryFailure(issues); - if (issues.length > 0) continue; + const pathIssues = await skillDirectoryIssues([destination]); + if (pathIssues.some(({ kind }) => kind === "unreadable")) { + issues.push(...pathIssues); + continue; + } + if (pathIssues.length > 0) continue; const manifest = join(destination.path, "SKILL.md"); const content = await readSkillFile(manifest, resources); if (isManagedSkillManifest(content)) existing.push(destination); @@ -222,10 +270,15 @@ export const existingSkillDestinations = async ( ) throw cause; if (cause instanceof NonRegularFileReadError) continue; - throw cause; + issues.push({ + client: destination.client, + path: destination.path, + kind: "unreadable", + detail: causeReason(cause), + }); } } - return existing; + return { destinations: existing, issues }; }; const readOptionalSkillState = async ( @@ -305,9 +358,16 @@ export const validateCanonicalSkillDirectories = async ( environment: Readonly = {}, platform: NodeJS.Platform = process.platform, ): Promise> => { - const issues = await skillDirectoryIssues( - skillDestinations(home, clientIds, environment, platform), + const selection = skillDestinationSelection( + home, + clientIds, + environment, + platform, ); + const issues = [ + ...selection.issues, + ...(await skillDirectoryIssues(selection.destinations)), + ]; return issues.length === 0 ? ok(undefined) : err(skillDirectoryFailure(issues)); @@ -367,31 +427,21 @@ const inspectCanonicalSkill = async ( platform: NodeJS.Platform, resources?: ArtifactResourceScope, ): Promise> => { - const destinations = skillDestinations( + const selection = skillDestinationSelection( home, clientIds, environment, platform, ); - const issues = - clientIds === undefined - ? [] - : [...(await skillDirectoryIssues(destinations))]; - if (issues.length > 0) - return err({ - _tag: "SkillInspectionFailure", - issues, - cause: issues, - manifest: undefined, - }); + const destinations = selection.destinations; + const issues: SkillPathIssue[] = [...selection.issues]; let canonical = true; let manifest: string | undefined; - let failureCause: unknown; + let failureCause: unknown = issues.length > 0 ? issues : undefined; let installedDestination = false; for (const destination of destinations) { const { path: root } = destination; - const pathIssues = - clientIds === undefined ? await skillDirectoryIssues([destination]) : []; + const pathIssues = await skillDirectoryIssues([destination]); if (pathIssues.length > 0) { issues.push(...pathIssues); if (failureCause === undefined) failureCause = pathIssues; @@ -437,6 +487,13 @@ const inspectCanonicalSkill = async ( installedDestination ||= rootHasFiles; canonical = canonical && rootCanonical; issues.push(...rootIssues); + if (!rootCanonical && rootIssues.length === 0) + issues.push({ + client: destination.client, + path: root, + kind: rootHasFiles ? "stale" : "missing", + detail: `Selected skill copy is ${rootHasFiles ? "stale" : "missing"}`, + }); } } return issues.length === 0 diff --git a/src/application/SupportedClients.ts b/src/application/SupportedClients.ts index f531f01108..d2e7251eab 100644 --- a/src/application/SupportedClients.ts +++ b/src/application/SupportedClients.ts @@ -809,13 +809,24 @@ export const supportedClients = ( }); }; -/** Resolve personal skill roots using the same client environment as setup. */ +/** Resolved destinations and unresolved selections share one path-resolution owner. */ export const clientSkillDirectories = ( home: string, clientIds: readonly string[] | undefined, environment: Readonly = {}, platform: NodeJS.Platform = process.platform, -): readonly { readonly client: string; readonly directory: string }[] => { +): { + readonly directories: readonly { + readonly client: string; + readonly directory: string; + }[]; + readonly unresolved: readonly { + readonly client: string; + readonly path: string; + readonly detail: string; + }[]; +} => { + const unresolved: { client: string; path: string; detail: string }[] = []; const definitions: readonly ClientDefinition[] = SUPPORTED_CLIENT_DEFINITIONS; const selected = clientIds === undefined @@ -831,6 +842,14 @@ export const clientSkillDirectories = ( const destinations = new Map(); for (const client of selected) { const definition = definitions.find(({ name }) => name === client); + if (definition === undefined && client !== "shared") { + unresolved.push({ + client, + path: client, + detail: `Unsupported skill client: ${client}`, + }); + continue; + } let directory: string; try { const resolved = @@ -841,11 +860,20 @@ export const clientSkillDirectories = ( platform, env: environment, }); - if (!resolved.ok) continue; + if (!resolved.ok) { + unresolved.push({ + client, + path: resolved.error.path, + detail: resolved.error.message, + }); + continue; + } directory = resolved.value; } catch (cause: unknown) { - // Registration diagnostics own an unresolved profile; never fall back to another skill root. - if (cause instanceof HermesClientPathError) continue; + if (cause instanceof HermesClientPathError) { + unresolved.push({ client, path: cause.path, detail: cause.message }); + continue; + } throw cause; } if (!destinations.has(directory)) @@ -855,7 +883,10 @@ export const clientSkillDirectories = ( directory, }); } - return [...destinations.values()].sort((left, right) => - compareUnicodeCodePoints(left.directory, right.directory), - ); + return { + directories: [...destinations.values()].sort((left, right) => + compareUnicodeCodePoints(left.directory, right.directory), + ), + unresolved, + }; }; diff --git a/src/application/Uninstall.ts b/src/application/Uninstall.ts index 2233396e6a..a59620aa89 100644 --- a/src/application/Uninstall.ts +++ b/src/application/Uninstall.ts @@ -26,7 +26,7 @@ import { isManagedSkillManifest, MANAGED_SKILL_FILES, readSkillFile, - skillDestinations, + skillDestinationSelection, } from "./SetupSkill.js"; import { classifyClientRegistrationCommand } from "./ClientRegistrationIdentity.js"; import { CURRENT_ENTRYPOINT_PATH } from "../process/CurrentEntrypointPath.js"; @@ -312,12 +312,21 @@ export const systemUninstallHost = ( ), ), inspectSkill: async () => { - for (const destination of skillDestinations( + const selection = skillDestinationSelection( home, undefined, environment, platform, - )) { + ); + if (selection.issues.length > 0) + return item( + "skill", + "failed", + selection.issues + .map(({ path, detail }) => `${detail} (${path})`) + .join(" "), + ); + for (const destination of selection.destinations) { const inspected = await inspectManagedSkill( destination.path, destination.client, @@ -810,16 +819,24 @@ const removeManagedSkills = async ( platform: NodeJS.Platform, writeBlocker?: (path: string) => string | undefined, ): Promise => { + const selection = skillDestinationSelection( + home, + undefined, + environment, + platform, + ); + if (selection.issues.length > 0) + return item( + "skill", + "failed", + selection.issues + .map(({ path, detail }) => `${detail} (${path})`) + .join(" "), + ); // Every destination shares one summary item; preserve both client and path. const results = await Promise.all( - skillDestinations(home, undefined, environment, platform).map( - ({ client, path }) => - removeManagedSkill( - path, - `${client} (${path})`, - fileSystem, - writeBlocker, - ), + selection.destinations.map(({ client, path }) => + removeManagedSkill(path, `${client} (${path})`, fileSystem, writeBlocker), ), ); const failed = results.find(({ status }) => status === "failed"); diff --git a/src/application/UpdateMaintenance.ts b/src/application/UpdateMaintenance.ts index fa1d4023e9..9ea7e5e4c1 100644 --- a/src/application/UpdateMaintenance.ts +++ b/src/application/UpdateMaintenance.ts @@ -2,8 +2,14 @@ import { z } from "zod"; import { RegularFileCleanupFailure } from "../filesystem/OwnedRegularFile.js"; import { classifyClientRegistrationCommand } from "./ClientRegistrationIdentity.js"; -import { readClientRegistrationInspections } from "./ClientRegistrationStatus.js"; -import { existingSkillDestinations } from "./SetupSkill.js"; +import { + readClientRegistrationInspections, + type ClientRegistrationStatus, +} from "./ClientRegistrationStatus.js"; +import { + existingSkillDestinations, + type SkillPathIssue, +} from "./SetupSkill.js"; import type { SetupAction } from "./SetupTypes.js"; import type { Result } from "../domain/result.js"; import type { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; @@ -12,6 +18,8 @@ import type { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.j export interface MaintenanceScope { readonly clients: readonly string[]; readonly skill: boolean; + readonly unresolved?: readonly ClientRegistrationStatus[]; + readonly skillIssues?: readonly SkillPathIssue[]; readonly skillDestinations?: readonly { readonly client: string; readonly path: string; @@ -27,7 +35,11 @@ export type IntegrationMaintenance = readonly command: readonly string[]; readonly plannedActions: readonly SetupAction[]; } - | { readonly status: "unavailable"; readonly remediation: string }; + | { + readonly status: "unavailable"; + readonly remediation: string; + readonly scope?: MaintenanceScope; + }; const setupPlanSchema = z.object({ status: z.literal("planned"), @@ -62,7 +74,12 @@ export const existingMaintenanceScope = async ( }, ); const clients: string[] = []; + const unresolved: ClientRegistrationStatus[] = []; for (const { status: registration, activation } of registrations) { + if (registration.state === "unknown" || registration.state === "invalid") { + unresolved.push(registration); + continue; + } if ( activation !== "enabled" || (registration.state !== "aligned" && registration.state !== "stale") || @@ -72,7 +89,7 @@ export const existingMaintenanceScope = async ( continue; clients.push(registration.client); } - const skillDestinations = await existingSkillDestinations( + const skills = await existingSkillDestinations( home, environment, process.platform, @@ -80,8 +97,10 @@ export const existingMaintenanceScope = async ( ); return { clients, - skill: skillDestinations.length > 0, - skillDestinations, + skill: skills.destinations.length > 0, + skillDestinations: skills.destinations, + ...(skills.issues.length === 0 ? {} : { skillIssues: skills.issues }), + ...(unresolved.length === 0 ? {} : { unresolved }), }; }; @@ -100,6 +119,24 @@ export const planIntegrationMaintenance = async ( environment, resources, ); + if ( + (scope.unresolved?.length ?? 0) > 0 || + (scope.skillIssues?.length ?? 0) > 0 + ) + return { + status: "unavailable", + scope, + remediation: `Integration maintenance is unresolved: ${[ + ...(scope.unresolved ?? []).map( + ({ client, config_path, remediation }) => + `${client} (${config_path}): ${remediation ?? "Registration identity is unresolved"}`, + ), + ...(scope.skillIssues ?? []).map( + ({ client, path, detail }) => + `${client ?? "skill"} (${path}): ${detail}`, + ), + ].join(" ")}`, + }; const skillDestinations = scope.skillDestinations ?? []; if (scope.clients.length === 0 && !scope.skill) return { status: "current", plannedActions: [] }; diff --git a/tests/boundary/filesystem/dshClientConfiguration.test.ts b/tests/boundary/filesystem/dshClientConfiguration.test.ts index 0142716b64..b1301c5ba2 100644 --- a/tests/boundary/filesystem/dshClientConfiguration.test.ts +++ b/tests/boundary/filesystem/dshClientConfiguration.test.ts @@ -62,9 +62,9 @@ describe("DeepSeek Harness configuration paths", () => { configPath: join(home, dshRoot, "cordis.patch.yml"), markerPath: join(home, dshRoot), }); - expect(clientSkillDirectories(home, ["dsh"], environment)).toEqual([ - { client, directory: join(home, agentsRoot, "skills") }, - ]); + expect( + clientSkillDirectories(home, ["dsh"], environment).directories, + ).toEqual([{ client, directory: join(home, agentsRoot, "skills") }]); }, ); @@ -82,9 +82,9 @@ describe("DeepSeek Harness configuration paths", () => { configPath: join(root, "cordis.patch.yml"), markerPath: root, }); - expect(clientSkillDirectories(home, ["dsh"], environment)).toEqual([ - { client: "dsh", directory: join(root, "skills") }, - ]); + expect( + clientSkillDirectories(home, ["dsh"], environment).directories, + ).toEqual([{ client: "dsh", directory: join(root, "skills") }]); }, ); @@ -103,7 +103,7 @@ describe("DeepSeek Harness configuration paths", () => { expect( clientSkillDirectories(join("/home/a"), ["dsh"], { DSH_AGENTS_HOME: "/custom/agents", - }), + }).directories, ).toEqual([ { client: "dsh", directory: resolve("/custom/agents", "skills") }, ]); diff --git a/tests/boundary/filesystem/kimiClientConfiguration.test.ts b/tests/boundary/filesystem/kimiClientConfiguration.test.ts index 077c24d3ad..462b336877 100644 --- a/tests/boundary/filesystem/kimiClientConfiguration.test.ts +++ b/tests/boundary/filesystem/kimiClientConfiguration.test.ts @@ -71,7 +71,7 @@ describe("Kimi Code configuration paths and skills", () => { }); it("uses the existing shared skill destination without a separate skill root", () => { - expect(clientSkillDirectories("/home/a", ["kimi"])).toEqual([ + expect(clientSkillDirectories("/home/a", ["kimi"]).directories).toEqual([ { client: "shared", directory: "/home/a/.agents/skills" }, ]); }); diff --git a/tests/boundary/filesystem/setupSkill.test.ts b/tests/boundary/filesystem/setupSkill.test.ts index d3b16bb125..1ca405548f 100644 --- a/tests/boundary/filesystem/setupSkill.test.ts +++ b/tests/boundary/filesystem/setupSkill.test.ts @@ -178,7 +178,10 @@ describe("skill directory admission", () => { references, process.platform === "win32" ? "junction" : "dir", ); - expect(await existingSkillDestinations(directory)).toEqual([]); + expect(await existingSkillDestinations(directory)).toEqual({ + destinations: [], + issues: [], + }); expect((await lstat(references)).isSymbolicLink()).toBe(true); await access(join(retained, "native-and-artifacts.md")); }); diff --git a/tests/boundary/filesystem/skillSelectionProvenance.test.ts b/tests/boundary/filesystem/skillSelectionProvenance.test.ts new file mode 100644 index 0000000000..3ea5a92512 --- /dev/null +++ b/tests/boundary/filesystem/skillSelectionProvenance.test.ts @@ -0,0 +1,107 @@ +import { mkdir, readFile, readdir, writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { expect, it } from "vitest"; + +import { clientSkillDirectories } from "../../../src/application/SupportedClients.js"; +import { resolveHermesClientHome } from "../../../src/application/HermesClientHome.js"; +import { + canonicalSkillNeedsInstall, + readInstalledSkillIdentity, + skillDestinations, + validateCanonicalSkillDirectories, +} from "../../../src/application/SetupSkill.js"; +import { PRODUCT_IDENTITY } from "../../../src/identity.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; + +it("uses the same Hermes root for sticky selection and named profile resolution", async () => { + const home = await createTestTempDirectory("rea-hermes-nested-root-"); + const native = join(home, ".hermes"); + const nested = join(native, "nested"); + const profile = join(native, "profiles", "coder"); + await mkdir(profile, { recursive: true }); + await mkdir(nested); + await writeFile(join(profile, "config.yaml"), "version: 1\n"); + await writeFile(join(native, "active_profile"), "coder\n"); + await writeFile(join(nested, "active_profile"), "other\n"); + expect(resolveHermesClientHome(home, "linux", { HERMES_HOME: nested })).toBe( + profile, + ); + expect( + clientSkillDirectories(home, ["hermes"], { HERMES_HOME: nested }) + .directories, + ).toEqual([{ client: "hermes", directory: join(profile, "skills") }]); +}); + +it("retains an explicitly selected missing Hermes profile as unresolved without writing elsewhere", async () => { + const home = await createTestTempDirectory("rea-skill-profile-selection-"); + const root = join(home, ".hermes"); + await mkdir(root); + await writeFile(join(root, "active_profile"), "missing-profile\n"); + const environment = { HERMES_HOME: root }; + const selection = clientSkillDirectories(home, ["hermes"], environment); + expect(selection).toMatchObject({ + directories: [], + unresolved: [ + { + client: "hermes", + path: join(root, "profiles", "missing-profile"), + }, + ], + }); + expect(() => skillDestinations(home, ["hermes"], environment)).toThrow( + "missing", + ); + expect(await canonicalSkillNeedsInstall(home, ["hermes"], environment)).toBe( + true, + ); + expect( + await validateCanonicalSkillDirectories(home, ["hermes"], environment), + ).toMatchObject({ ok: false }); + expect( + await readInstalledSkillIdentity(home, ["hermes"], environment), + ).toMatchObject({ + canonical: false, + issues: [ + { + client: "hermes", + kind: "unresolved", + path: join(root, "profiles", "missing-profile"), + }, + ], + }); + expect(await readdir(home)).toEqual([".hermes"]); + expect(await readFile(join(root, "active_profile"), "utf8")).toBe( + "missing-profile\n", + ); +}); + +it("retains freshness for each selected personal and shared skill copy", async () => { + const home = await createTestTempDirectory("rea-skill-copy-selection-"); + const roots = [ + join(home, ".claude", "skills", PRODUCT_IDENTITY.skillName), + join(home, ".agents", "skills", PRODUCT_IDENTITY.skillName), + ]; + for (const root of roots) { + await mkdir(root, { recursive: true }); + await writeFile(join(root, "SKILL.md"), "stale local copy\n"); + } + const identity = await readInstalledSkillIdentity(home, [ + "claude_code", + "shared", + ]); + expect(identity).toMatchObject({ + canonical: false, + issues: expect.arrayContaining([ + expect.objectContaining({ + client: "claude_code", + path: roots[0], + kind: "stale", + }), + expect.objectContaining({ + client: "shared", + path: roots[1], + kind: "stale", + }), + ]), + }); +}); diff --git a/tests/process-global/updateMaintenance.test.ts b/tests/process-global/updateMaintenance.test.ts index 5c3341ba95..48f44bc711 100644 --- a/tests/process-global/updateMaintenance.test.ts +++ b/tests/process-global/updateMaintenance.test.ts @@ -130,6 +130,42 @@ describe("OMP integration maintenance", () => { }); describe("maintenance activation and layered configuration", () => { + it("keeps unresolved registrations out of writes without calling them current", async () => { + await writeClient("codex", staleCodex); + const cursor = await writeClient( + "cursor", + JSON.stringify({ + mcpServers: { + rea: { command: join(home, "removed-release", "rea"), args: ["mcp"] }, + }, + }), + ); + const before = await readFile(cursor, "utf8"); + const result = await planIntegrationMaintenance( + home, + entryPoint, + process.env, + () => { + throw new Error("An unresolved maintenance scope must not be executed"); + }, + ); + expect(result).toMatchObject({ + status: "unavailable", + scope: { + clients: ["codex"], + unresolved: [ + expect.objectContaining({ + client: "cursor", + state: "unknown", + config_path: cursor, + }), + ], + }, + remediation: expect.stringContaining("cursor"), + }); + expect(await readFile(cursor, "utf8")).toBe(before); + }); + it("leaves an inherited disabled OpenCode V2 registration outside maintenance", async () => { const upper = await writeClient( "opencode",