fix: keep runtime skills scoped to ./skills
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
@@ -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<string | null> {
|
||||
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}`);
|
||||
|
||||
@@ -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<string, unknown> {
|
||||
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<PaperclipSkillEntry[]> {
|
||||
const entriesByName = new Map<string, PaperclipSkillEntry>();
|
||||
export async function resolvePaperclipSkillsDir(
|
||||
moduleDir: string,
|
||||
additionalCandidates: string[] = [],
|
||||
): Promise<string | null> {
|
||||
const candidates = [
|
||||
...PAPERCLIP_SKILL_ROOT_RELATIVE_CANDIDATES.map((relativePath) => path.resolve(moduleDir, relativePath)),
|
||||
...additionalCandidates.map((candidate) => path.resolve(candidate)),
|
||||
];
|
||||
const seenRoots = new Set<string>();
|
||||
|
||||
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<ReturnType<typeof fs.readdir>>;
|
||||
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<PaperclipSkillEntry[]> {
|
||||
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<string>,
|
||||
): Promise<string[]> {
|
||||
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;
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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);
|
||||
|
||||
61
server/src/__tests__/paperclip-skill-utils.test.ts
Normal file
61
server/src/__tests__/paperclip-skill-utils.test.ts
Normal file
@@ -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<string> {
|
||||
return fs.mkdtemp(path.join(os.tmpdir(), prefix));
|
||||
}
|
||||
|
||||
describe("paperclip skill utils", () => {
|
||||
const cleanupDirs = new Set<string>();
|
||||
|
||||
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);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user