From 188dddf726b50e849978730bb5b00ee0fc814bb6 Mon Sep 17 00:00:00 2001 From: Aiden Bai Date: Wed, 24 Jun 2026 02:30:40 -0700 Subject: [PATCH] fix(skill): prevent re-install from wiping skills when source equals destination When a skill's discovered source path is the same as (or overlaps) the install destination (e.g. authoring skills directly in .agents/skills), the installer cleaned the destination before copying, deleting the source and leaving an empty/stale SKILL.md. Route all copy paths through a syncDirectory helper that no-ops when source and destination resolve to the same path and stages overlapping sources before cleaning. Add a regression test. --- .changeset/safe-skill-reinstall.md | 5 ++ packages/agent-install/src/skill/installer.ts | 53 ++++++++++++++++--- .../test/installer-symlink.test.ts | 33 ++++++++++++ 3 files changed, 83 insertions(+), 8 deletions(-) create mode 100644 .changeset/safe-skill-reinstall.md diff --git a/.changeset/safe-skill-reinstall.md b/.changeset/safe-skill-reinstall.md new file mode 100644 index 0000000..d9283c4 --- /dev/null +++ b/.changeset/safe-skill-reinstall.md @@ -0,0 +1,5 @@ +--- +"agent-install": patch +--- + +Fix skill re-install wiping/not updating files when the discovered source path is the same as (or overlaps) the install destination (e.g. authoring skills directly in `.agents/skills`). The copy now stages overlapping sources and is a no-op when source and destination resolve to the same path, instead of deleting the source before copying. diff --git a/packages/agent-install/src/skill/installer.ts b/packages/agent-install/src/skill/installer.ts index d166542..7905ae6 100644 --- a/packages/agent-install/src/skill/installer.ts +++ b/packages/agent-install/src/skill/installer.ts @@ -1,6 +1,16 @@ import { existsSync } from "node:fs"; -import { cp, lstat, mkdir, readdir, readlink, realpath, rm, symlink } from "node:fs/promises"; -import { homedir, platform } from "node:os"; +import { + cp, + lstat, + mkdir, + mkdtemp, + readdir, + readlink, + realpath, + rm, + symlink, +} from "node:fs/promises"; +import { homedir, platform, tmpdir } from "node:os"; import { basename, dirname, join, relative, resolve, sep } from "node:path"; import { isPathSafe } from "../utils/is-path-safe.ts"; @@ -101,6 +111,36 @@ const copyDirectory = async (src: string, dest: string, rootSrc = src): Promise< ); }; +const resolveExistingPath = async (path: string): Promise => + realpath(path).catch(() => resolve(path)); + +const directoriesOverlap = (source: string, dest: string): boolean => + source === dest || isPathInsideBase(dest, source) || isPathInsideBase(source, dest); + +const syncDirectory = async (source: string, dest: string): Promise => { + const [realSource, realDest] = await Promise.all([ + resolveExistingPath(source), + resolveExistingPath(dest), + ]); + + if (realSource === realDest) return; + + if (directoriesOverlap(realSource, realDest)) { + const staging = await mkdtemp(join(tmpdir(), "agent-install-stage-")); + try { + await copyDirectory(realSource, staging); + await cleanAndCreateDirectory(dest); + await copyDirectory(staging, dest); + } finally { + await rm(staging, { recursive: true, force: true }).catch(() => undefined); + } + return; + } + + await cleanAndCreateDirectory(dest); + await copyDirectory(source, dest); +}; + const isLoopError = (error: unknown): boolean => Boolean( error && @@ -154,8 +194,7 @@ const createSymlink = async (target: string, linkPath: string): Promise }; const installByCopy = async (skill: Skill, destination: string): Promise => { - await cleanAndCreateDirectory(destination); - await copyDirectory(skill.path, destination); + await syncDirectory(skill.path, destination); return { success: true, path: destination, mode: "copy" }; }; @@ -166,8 +205,7 @@ const installBySymlink = async ( agentDir: string, isGlobal: boolean, ): Promise => { - await cleanAndCreateDirectory(canonicalDir); - await copyDirectory(skill.path, canonicalDir); + await syncDirectory(skill.path, canonicalDir); if (isGlobal && isUniversalSkillAgent(agentType)) { return { success: true, path: canonicalDir, canonicalPath: canonicalDir, mode: "symlink" }; @@ -177,8 +215,7 @@ const installBySymlink = async ( return { success: true, path: agentDir, canonicalPath: canonicalDir, mode: "symlink" }; } - await cleanAndCreateDirectory(agentDir); - await copyDirectory(skill.path, agentDir); + await syncDirectory(skill.path, agentDir); return { success: true, path: agentDir, diff --git a/packages/agent-install/test/installer-symlink.test.ts b/packages/agent-install/test/installer-symlink.test.ts index ae4c328..7c753cb 100644 --- a/packages/agent-install/test/installer-symlink.test.ts +++ b/packages/agent-install/test/installer-symlink.test.ts @@ -148,6 +148,39 @@ describe("installer symlink regressions", () => { } }); + it("preserves content when the source path is the canonical destination", async () => { + const root = await mkdtemp(join(tmpdir(), "agent-install-symlink-")); + const projectDir = join(root, "project"); + const skillName = "in-place-skill"; + const canonicalDir = join(projectDir, ".agents", "skills", skillName); + await mkdir(canonicalDir, { recursive: true }); + await writeFile( + join(canonicalDir, "SKILL.md"), + `---\nname: ${skillName}\ndescription: in place\n---\nORIGINAL BODY\n`, + "utf-8", + ); + + try { + const result = await installSkillForAgent( + { name: skillName, description: "test", path: canonicalDir, rawContent: "" }, + "claude-code", + { cwd: projectDir, mode: "symlink", global: false }, + ); + expect(result.success).toBe(true); + + const canonicalContents = await readFile(join(canonicalDir, "SKILL.md"), "utf-8"); + expect(canonicalContents).toContain("ORIGINAL BODY"); + + const claudeContents = await readFile( + join(projectDir, ".claude", "skills", skillName, "SKILL.md"), + "utf-8", + ); + expect(claudeContents).toContain("ORIGINAL BODY"); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + it("rejects unsafe skill names (path traversal)", async () => { const root = await mkdtemp(join(tmpdir(), "agent-install-symlink-")); const projectDir = join(root, "project");