From 29a743cb9e3601a7896c89875609df3a6968fe8b Mon Sep 17 00:00:00 2001 From: Dotta Date: Thu, 12 Mar 2026 15:57:37 -0500 Subject: [PATCH] fix: keep runtime skills scoped to ./skills Co-Authored-By: Paperclip --- cli/src/commands/client/agent.ts | 31 +++--- packages/adapter-utils/src/server-utils.ts | 102 +++++++++++++----- .../codex-local/src/server/execute.ts | 11 ++ .../cursor-local/src/server/execute.ts | 11 ++ .../gemini-local/src/server/execute.ts | 11 ++ .../adapters/pi-local/src/server/execute.ts | 11 ++ .../__tests__/paperclip-skill-utils.test.ts | 61 +++++++++++ 7 files changed, 195 insertions(+), 43 deletions(-) create mode 100644 server/src/__tests__/paperclip-skill-utils.test.ts diff --git a/cli/src/commands/client/agent.ts b/cli/src/commands/client/agent.ts index a6e86277..2c294628 100644 --- a/cli/src/commands/client/agent.ts +++ b/cli/src/commands/client/agent.ts @@ -1,5 +1,9 @@ import { Command } from "commander"; import type { Agent } from "@paperclipai/shared"; +import { + removeMaintainerOnlySkillSymlinks, + resolvePaperclipSkillsDir, +} from "@paperclipai/adapter-utils/server-utils"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; @@ -34,17 +38,12 @@ interface SkillsInstallSummary { tool: "codex" | "claude"; target: string; linked: string[]; + removed: string[]; skipped: string[]; failed: Array<{ name: string; error: string }>; } const __moduleDir = path.dirname(fileURLToPath(import.meta.url)); -const PAPERCLIP_SKILLS_CANDIDATES = [ - path.resolve(__moduleDir, "../../../../../.agents/skills"), // dev: cli/src/commands/client -> repo root/.agents/skills - path.resolve(__moduleDir, "../../../../../skills"), // dev: cli/src/commands/client -> repo root/skills - path.resolve(process.cwd(), ".agents/skills"), - path.resolve(process.cwd(), "skills"), -]; function codexSkillsHome(): string { const fromEnv = process.env.CODEX_HOME?.trim(); @@ -58,14 +57,6 @@ function claudeSkillsHome(): string { return path.join(base, "skills"); } -async function resolvePaperclipSkillsDir(): Promise { - for (const candidate of PAPERCLIP_SKILLS_CANDIDATES) { - const isDir = await fs.stat(candidate).then((s) => s.isDirectory()).catch(() => false); - if (isDir) return candidate; - } - return null; -} - async function installSkillsForTarget( sourceSkillsDir: string, targetSkillsDir: string, @@ -75,12 +66,17 @@ async function installSkillsForTarget( tool, target: targetSkillsDir, linked: [], + removed: [], skipped: [], failed: [], }; await fs.mkdir(targetSkillsDir, { recursive: true }); const entries = await fs.readdir(sourceSkillsDir, { withFileTypes: true }); + summary.removed = await removeMaintainerOnlySkillSymlinks( + targetSkillsDir, + entries.filter((entry) => entry.isDirectory()).map((entry) => entry.name), + ); for (const entry of entries) { if (!entry.isDirectory()) continue; const source = path.join(sourceSkillsDir, entry.name); @@ -140,7 +136,6 @@ async function installSkillsForTarget( error: err instanceof Error ? err.message : String(err), }); } - } } return summary; @@ -253,10 +248,10 @@ export function registerAgentCommands(program: Command): void { const installSummaries: SkillsInstallSummary[] = []; if (opts.installSkills !== false) { - const skillsDir = await resolvePaperclipSkillsDir(); + const skillsDir = await resolvePaperclipSkillsDir(__moduleDir, [path.resolve(process.cwd(), "skills")]); if (!skillsDir) { throw new Error( - "Could not locate local Paperclip skills directory. Expected ./skills or ./.agents/skills in the repo checkout.", + "Could not locate local Paperclip skills directory. Expected ./skills in the repo checkout.", ); } @@ -301,7 +296,7 @@ export function registerAgentCommands(program: Command): void { if (installSummaries.length > 0) { for (const summary of installSummaries) { console.log( - `${summary.tool}: linked=${summary.linked.length} skipped=${summary.skipped.length} failed=${summary.failed.length} target=${summary.target}`, + `${summary.tool}: linked=${summary.linked.length} removed=${summary.removed.length} skipped=${summary.skipped.length} failed=${summary.failed.length} target=${summary.target}`, ); for (const failed of summary.failed) { console.log(` failed ${failed.name}: ${failed.error}`); diff --git a/packages/adapter-utils/src/server-utils.ts b/packages/adapter-utils/src/server-utils.ts index 15f0ada8..30f0c9bd 100644 --- a/packages/adapter-utils/src/server-utils.ts +++ b/packages/adapter-utils/src/server-utils.ts @@ -33,9 +33,7 @@ export const MAX_CAPTURE_BYTES = 4 * 1024 * 1024; export const MAX_EXCERPT_BYTES = 32 * 1024; const SENSITIVE_ENV_KEY = /(key|token|secret|password|passwd|authorization|cookie)/i; const PAPERCLIP_SKILL_ROOT_RELATIVE_CANDIDATES = [ - "../../.agents/skills", "../../skills", - "../../../../../.agents/skills", "../../../../../skills", ]; @@ -44,6 +42,14 @@ export interface PaperclipSkillEntry { source: string; } +function normalizePathSlashes(value: string): string { + return value.replaceAll("\\", "/"); +} + +function isMaintainerOnlySkillTarget(candidate: string): boolean { + return normalizePathSlashes(candidate).includes("/.agents/skills/"); +} + export function parseObject(value: unknown): Record { if (typeof value !== "object" || value === null || Array.isArray(value)) { return {}; @@ -256,36 +262,44 @@ export async function ensureAbsoluteDirectory( } } -export async function listPaperclipSkillEntries(moduleDir: string): Promise { - const entriesByName = new Map(); +export async function resolvePaperclipSkillsDir( + moduleDir: string, + additionalCandidates: string[] = [], +): Promise { + const candidates = [ + ...PAPERCLIP_SKILL_ROOT_RELATIVE_CANDIDATES.map((relativePath) => path.resolve(moduleDir, relativePath)), + ...additionalCandidates.map((candidate) => path.resolve(candidate)), + ]; const seenRoots = new Set(); - for (const relativePath of PAPERCLIP_SKILL_ROOT_RELATIVE_CANDIDATES) { - const root = path.resolve(moduleDir, relativePath); + for (const root of candidates) { if (seenRoots.has(root)) continue; seenRoots.add(root); - const isDirectory = await fs.stat(root).then((stats) => stats.isDirectory()).catch(() => false); - if (!isDirectory) continue; - - let entries: Awaited>; - try { - entries = await fs.readdir(root, { withFileTypes: true }); - } catch { - continue; - } - - for (const entry of entries) { - if (!entry.isDirectory()) continue; - if (entriesByName.has(entry.name)) continue; - entriesByName.set(entry.name, { - name: entry.name, - source: path.join(root, entry.name), - }); - } + if (isDirectory) return root; } - return Array.from(entriesByName.values()); + return null; +} + +export async function listPaperclipSkillEntries( + moduleDir: string, + additionalCandidates: string[] = [], +): Promise { + const root = await resolvePaperclipSkillsDir(moduleDir, additionalCandidates); + if (!root) return []; + + try { + const entries = await fs.readdir(root, { withFileTypes: true }); + return entries + .filter((entry) => entry.isDirectory()) + .map((entry) => ({ + name: entry.name, + source: path.join(root, entry.name), + })); + } catch { + return []; + } } export async function readPaperclipSkillMarkdown( @@ -340,6 +354,44 @@ export async function ensurePaperclipSkillSymlink( return "repaired"; } +export async function removeMaintainerOnlySkillSymlinks( + skillsHome: string, + allowedSkillNames: Iterable, +): Promise { + const allowed = new Set(Array.from(allowedSkillNames)); + try { + const entries = await fs.readdir(skillsHome, { withFileTypes: true }); + const removed: string[] = []; + for (const entry of entries) { + if (allowed.has(entry.name)) continue; + + const target = path.join(skillsHome, entry.name); + const existing = await fs.lstat(target).catch(() => null); + if (!existing?.isSymbolicLink()) continue; + + const linkedPath = await fs.readlink(target).catch(() => null); + if (!linkedPath) continue; + + const resolvedLinkedPath = path.isAbsolute(linkedPath) + ? linkedPath + : path.resolve(path.dirname(target), linkedPath); + if ( + !isMaintainerOnlySkillTarget(linkedPath) && + !isMaintainerOnlySkillTarget(resolvedLinkedPath) + ) { + continue; + } + + await fs.unlink(target); + removed.push(entry.name); + } + + return removed; + } catch { + return []; + } +} + export async function ensureCommandResolvable(command: string, cwd: string, env: NodeJS.ProcessEnv) { const resolved = await resolveCommandPath(command, cwd, env); if (resolved) return; diff --git a/packages/adapters/codex-local/src/server/execute.ts b/packages/adapters/codex-local/src/server/execute.ts index 3afbf09e..c51dc8a1 100644 --- a/packages/adapters/codex-local/src/server/execute.ts +++ b/packages/adapters/codex-local/src/server/execute.ts @@ -16,6 +16,7 @@ import { ensurePaperclipSkillSymlink, ensurePathInEnv, listPaperclipSkillEntries, + removeMaintainerOnlySkillSymlinks, renderTemplate, runChildProcess, } from "@paperclipai/adapter-utils/server-utils"; @@ -80,6 +81,16 @@ export async function ensureCodexSkillsInjected( const skillsHome = options.skillsHome ?? path.join(codexHomeDir(), "skills"); await fs.mkdir(skillsHome, { recursive: true }); + const removedSkills = await removeMaintainerOnlySkillSymlinks( + skillsHome, + skillsEntries.map((entry) => entry.name), + ); + for (const skillName of removedSkills) { + await onLog( + "stderr", + `[paperclip] Removed maintainer-only Codex skill "${skillName}" from ${skillsHome}\n`, + ); + } const linkSkill = options.linkSkill; for (const entry of skillsEntries) { const target = path.join(skillsHome, entry.name); diff --git a/packages/adapters/cursor-local/src/server/execute.ts b/packages/adapters/cursor-local/src/server/execute.ts index ae068c1c..043c3ef1 100644 --- a/packages/adapters/cursor-local/src/server/execute.ts +++ b/packages/adapters/cursor-local/src/server/execute.ts @@ -15,6 +15,7 @@ import { ensurePaperclipSkillSymlink, ensurePathInEnv, listPaperclipSkillEntries, + removeMaintainerOnlySkillSymlinks, renderTemplate, runChildProcess, } from "@paperclipai/adapter-utils/server-utils"; @@ -108,6 +109,16 @@ export async function ensureCursorSkillsInjected( ); return; } + const removedSkills = await removeMaintainerOnlySkillSymlinks( + skillsHome, + skillsEntries.map((entry) => entry.name), + ); + for (const skillName of removedSkills) { + await onLog( + "stderr", + `[paperclip] Removed maintainer-only Cursor skill "${skillName}" from ${skillsHome}\n`, + ); + } const linkSkill = options.linkSkill ?? ((source: string, target: string) => fs.symlink(source, target)); for (const entry of skillsEntries) { const target = path.join(skillsHome, entry.name); diff --git a/packages/adapters/gemini-local/src/server/execute.ts b/packages/adapters/gemini-local/src/server/execute.ts index fa27bffa..2408b425 100644 --- a/packages/adapters/gemini-local/src/server/execute.ts +++ b/packages/adapters/gemini-local/src/server/execute.ts @@ -15,6 +15,7 @@ import { ensurePaperclipSkillSymlink, ensurePathInEnv, listPaperclipSkillEntries, + removeMaintainerOnlySkillSymlinks, parseObject, redactEnvForLogs, renderTemplate, @@ -96,6 +97,16 @@ async function ensureGeminiSkillsInjected( ); return; } + const removedSkills = await removeMaintainerOnlySkillSymlinks( + skillsHome, + skillsEntries.map((entry) => entry.name), + ); + for (const skillName of removedSkills) { + await onLog( + "stderr", + `[paperclip] Removed maintainer-only Gemini skill "${skillName}" from ${skillsHome}\n`, + ); + } for (const entry of skillsEntries) { const target = path.join(skillsHome, entry.name); diff --git a/packages/adapters/pi-local/src/server/execute.ts b/packages/adapters/pi-local/src/server/execute.ts index 6fabe32d..dfb1453b 100644 --- a/packages/adapters/pi-local/src/server/execute.ts +++ b/packages/adapters/pi-local/src/server/execute.ts @@ -15,6 +15,7 @@ import { ensurePaperclipSkillSymlink, ensurePathInEnv, listPaperclipSkillEntries, + removeMaintainerOnlySkillSymlinks, renderTemplate, runChildProcess, } from "@paperclipai/adapter-utils/server-utils"; @@ -54,6 +55,16 @@ async function ensurePiSkillsInjected(onLog: AdapterExecutionContext["onLog"]) { const piSkillsHome = path.join(os.homedir(), ".pi", "agent", "skills"); await fs.mkdir(piSkillsHome, { recursive: true }); + const removedSkills = await removeMaintainerOnlySkillSymlinks( + piSkillsHome, + skillsEntries.map((entry) => entry.name), + ); + for (const skillName of removedSkills) { + await onLog( + "stderr", + `[paperclip] Removed maintainer-only Pi skill "${skillName}" from ${piSkillsHome}\n`, + ); + } for (const entry of skillsEntries) { const target = path.join(piSkillsHome, entry.name); diff --git a/server/src/__tests__/paperclip-skill-utils.test.ts b/server/src/__tests__/paperclip-skill-utils.test.ts new file mode 100644 index 00000000..4344dc17 --- /dev/null +++ b/server/src/__tests__/paperclip-skill-utils.test.ts @@ -0,0 +1,61 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { + listPaperclipSkillEntries, + removeMaintainerOnlySkillSymlinks, +} from "@paperclipai/adapter-utils/server-utils"; + +async function makeTempDir(prefix: string): Promise { + return fs.mkdtemp(path.join(os.tmpdir(), prefix)); +} + +describe("paperclip skill utils", () => { + const cleanupDirs = new Set(); + + afterEach(async () => { + await Promise.all(Array.from(cleanupDirs).map((dir) => fs.rm(dir, { recursive: true, force: true }))); + cleanupDirs.clear(); + }); + + it("lists runtime skills from ./skills without pulling in .agents/skills", async () => { + const root = await makeTempDir("paperclip-skill-roots-"); + cleanupDirs.add(root); + + const moduleDir = path.join(root, "a", "b", "c", "d", "e"); + await fs.mkdir(moduleDir, { recursive: true }); + await fs.mkdir(path.join(root, "skills", "paperclip"), { recursive: true }); + await fs.mkdir(path.join(root, ".agents", "skills", "release"), { recursive: true }); + + const entries = await listPaperclipSkillEntries(moduleDir); + + expect(entries.map((entry) => entry.name)).toEqual(["paperclip"]); + expect(entries[0]?.source).toBe(path.join(root, "skills", "paperclip")); + }); + + it("removes stale maintainer-only symlinks from a shared skills home", async () => { + const root = await makeTempDir("paperclip-skill-cleanup-"); + cleanupDirs.add(root); + + const skillsHome = path.join(root, "skills-home"); + const runtimeSkill = path.join(root, "skills", "paperclip"); + const customSkill = path.join(root, "custom", "release-notes"); + const staleMaintainerSkill = path.join(root, ".agents", "skills", "release"); + + await fs.mkdir(skillsHome, { recursive: true }); + await fs.mkdir(runtimeSkill, { recursive: true }); + await fs.mkdir(customSkill, { recursive: true }); + + await fs.symlink(runtimeSkill, path.join(skillsHome, "paperclip")); + await fs.symlink(customSkill, path.join(skillsHome, "release-notes")); + await fs.symlink(staleMaintainerSkill, path.join(skillsHome, "release")); + + const removed = await removeMaintainerOnlySkillSymlinks(skillsHome, ["paperclip"]); + + expect(removed).toEqual(["release"]); + await expect(fs.lstat(path.join(skillsHome, "release"))).rejects.toThrow(); + expect((await fs.lstat(path.join(skillsHome, "paperclip"))).isSymbolicLink()).toBe(true); + expect((await fs.lstat(path.join(skillsHome, "release-notes"))).isSymbolicLink()).toBe(true); + }); +});