From f8161ad7b31e636dd393e6b9cc1fe64a3de8a075 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Mon, 26 Oct 2026 18:11:54 +0800 Subject: [PATCH] fix(setup): preserve selected home paths throughout skill installation --- src/application/SetupSkill.ts | 15 +-- src/application/SupportedClients.ts | 6 +- .../setup/selectedClientHomePaths.test.ts | 93 +++++++++++++++++++ 3 files changed, 105 insertions(+), 9 deletions(-) diff --git a/src/application/SetupSkill.ts b/src/application/SetupSkill.ts index 9ffc2bfaec..4253e302f6 100644 --- a/src/application/SetupSkill.ts +++ b/src/application/SetupSkill.ts @@ -1,6 +1,7 @@ import { copyFile, lstat, mkdir, readFile } from "node:fs/promises"; import { constants as fsConstants } from "node:fs"; -import { dirname, join } from "node:path"; +import { dirname } from "node:path"; +import { appendCallerPath } from "../filesystem/callerPath.js"; import { err, ok, type Result } from "../domain/result.js"; import { parseSkillManifestMetadata } from "../domain/skillManifest.js"; import { readBoundedFileBytes } from "../process/BoundedFileBytes.js"; @@ -205,7 +206,7 @@ export const skillDestinationSelection = ( return { destinations: selection.directories.map(({ client, directory }) => ({ client, - path: join(directory, PRODUCT_IDENTITY.skillName), + path: appendCallerPath(directory, PRODUCT_IDENTITY.skillName, platform), })), issues: selection.unresolved.map(({ client, path, detail }) => ({ client, @@ -260,7 +261,7 @@ export const existingSkillDestinations = async ( continue; } if (pathIssues.length > 0) continue; - const manifest = join(destination.path, "SKILL.md"); + const manifest = appendCallerPath(destination.path, "SKILL.md"); const content = await readSkillFile(manifest, resources); if (isManagedSkillManifest(content)) existing.push(destination); } catch (cause: unknown) { @@ -335,7 +336,9 @@ const skillDirectoryIssues = async ( issues.push(root.error); continue; } - const references = await inspectSkillDirectory(join(path, "references")); + const references = await inspectSkillDirectory( + appendCallerPath(path, "references"), + ); if (!references.ok) issues.push(references.error); } return issues; @@ -398,7 +401,7 @@ const canonicalSkillFiles = async ( return Promise.all( destinations.flatMap(({ path: root }) => MANAGED_SKILL_FILES.map(async (relativePath) => { - const destination = join(root, relativePath); + const destination = appendCallerPath(root, relativePath); return { destination, content: await canonicalSkillContent(relativePath), @@ -455,7 +458,7 @@ const inspectCanonicalSkill = async ( let rootHasFiles = false; const rootIssues: SkillPathIssue[] = []; for (const relativePath of MANAGED_SKILL_FILES) { - const destination = join(root, relativePath); + const destination = appendCallerPath(root, relativePath); let original: SkillFileSnapshot | undefined; try { original = await readOptionalSkillState(destination, resources); diff --git a/src/application/SupportedClients.ts b/src/application/SupportedClients.ts index 198f909afd..bbe1559fe2 100644 --- a/src/application/SupportedClients.ts +++ b/src/application/SupportedClients.ts @@ -930,6 +930,7 @@ export const clientSkillDirectories = ( : clientIds.length === 0 ? ["shared"] : clientIds; + const sharedDirectory = appendCallerPath(home, ".agents/skills", platform); const destinations = new Map(); for (const client of selected) { const definition = definitions.find(({ name }) => name === client); @@ -945,7 +946,7 @@ export const clientSkillDirectories = ( try { const resolved = definition?.skillPath === undefined - ? ok(join(home, ".agents", "skills")) + ? ok(sharedDirectory) : resolvePath(definition.skillPath, { home, platform, @@ -969,8 +970,7 @@ export const clientSkillDirectories = ( } if (!destinations.has(directory)) destinations.set(directory, { - client: - directory === join(home, ".agents", "skills") ? "shared" : client, + client: directory === sharedDirectory ? "shared" : client, directory, }); } diff --git a/tests/acceptance/setup/selectedClientHomePaths.test.ts b/tests/acceptance/setup/selectedClientHomePaths.test.ts index dc69c88a72..e71af9e6a3 100644 --- a/tests/acceptance/setup/selectedClientHomePaths.test.ts +++ b/tests/acceptance/setup/selectedClientHomePaths.test.ts @@ -109,3 +109,96 @@ cliTest.skipIf(process.platform === "win32")( ).toEqual([]); }, ); + +cliTest.skipIf(process.platform === "win32")( + "installs shared skills in the native HOME selection with backups and repeat setup", + async ({ cli }) => { + const root = await createTestTempDirectory("rea-selected-skill-home-"); + const selected = join(root, "selected"); + await mkdir(join(selected, "nested"), { recursive: true }); + await symlink(join(selected, "nested"), join(root, "alias")); + const home = `${root}/alias/..`; + const relative = ".agents/skills/reverse-engineer-anything"; + const skill = join(selected, relative); + const lexicalSkill = join(root, relative); + const original = "previous selected instructions\n"; + const oldReference = "previous selected reference\n"; + const sentinel = "unselected instructions\n"; + const sentinelReference = "unselected reference\n"; + const reference = "references/connection-and-recovery.md"; + for (const directory of [skill, lexicalSkill]) + await mkdir(join(directory, "references"), { recursive: true }); + await writeFile(join(skill, "SKILL.md"), original); + await writeFile(join(skill, reference), oldReference); + await writeFile(join(lexicalSkill, "SKILL.md"), sentinel); + await writeFile(join(lexicalSkill, reference), sentinelReference); + const run = (arguments_: readonly string[]) => + cli.run({ + arguments: [...arguments_, "--json"], + cwd: root, + environment: { HOME: home, USERPROFILE: home }, + }); + const configured = await run([ + "setup", + "--skill-client", + "shared", + "--yes", + ]); + expect(configured.exitCode, configured.stdout + configured.stderr).toBe(0); + expect(configured.json).toMatchObject({ + status: "ready", + appliedActions: ["installed_skill"], + }); + const installed = await readFile(join(skill, "SKILL.md"), "utf8"); + expect(installed).toContain("name: reverse-engineer-anything"); + const installedReference = await readFile(join(skill, reference), "utf8"); + expect(installedReference).not.toBe(oldReference); + expect(await readFile(join(skill, "SKILL.md.rea.backup"), "utf8")).toBe( + original, + ); + expect(await readFile(`${join(skill, reference)}.rea.backup`, "utf8")).toBe( + oldReference, + ); + expect(await readFile(join(lexicalSkill, "SKILL.md"), "utf8")).toBe( + sentinel, + ); + expect(await readFile(join(lexicalSkill, reference), "utf8")).toBe( + sentinelReference, + ); + for (const path of [ + join(lexicalSkill, "SKILL.md.rea.backup"), + `${join(lexicalSkill, reference)}.rea.backup`, + ]) + await expect(access(path)).rejects.toMatchObject({ code: "ENOENT" }); + const repeated = await run(["setup", "--skill-client", "shared", "--yes"]); + expect(repeated.exitCode, repeated.stdout + repeated.stderr).toBe(0); + expect(repeated.json).toMatchObject({ + status: "ready", + appliedActions: [], + }); + expect(await readFile(join(skill, "SKILL.md"), "utf8")).toBe(installed); + expect(await readFile(join(skill, reference), "utf8")).toBe( + installedReference, + ); + const doctor = await run(["doctor", "--skill"]); + // Doctor also discovers client-specific locations. Gemini uses the lexical + // HOME sibling, whose deliberately stale sentinel must remain untouched. + expect(doctor.exitCode, doctor.stdout + doctor.stderr).toBe(1); + expect(doctor.json).toMatchObject({ + identity: { + skill: { + state: "stale", + issues: [{ client: "gemini_cli", path: lexicalSkill, kind: "stale" }], + }, + }, + }); + expect( + (await readdir(skill)).filter((name) => name.startsWith(".rea-skill-")), + ).toEqual([]); + expect( + (await readdir(join(skill, "references"))).filter((name) => + name.startsWith(".rea-skill-"), + ), + ).toEqual([]); + }, +);