diff --git a/packages/adapter-utils/src/acpx-engine/execute.ts b/packages/adapter-utils/src/acpx-engine/execute.ts index 3f537fedd1c..a80f0d39496 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.ts @@ -68,6 +68,7 @@ import { selectPaperclipTaskMarkdown, resolveLegacyPaperclipDesiredSkillNames, removeMaintainerOnlySkillSymlinks, + writeManagedGeminiSkillsManifest, rewriteWorkspaceCwdEnvVarsForExecution, shapePaperclipWorkspaceEnvForExecution, stringifyPaperclipWakePayload, @@ -1313,7 +1314,7 @@ async function prepareGeminiSkillRuntime(input: { const allowedSkillNames = selectedSkills.map((entry) => entry.runtimeName); const removedSkills = await removeMaintainerOnlySkillSymlinks(skillsHome, allowedSkillNames); for (const skillName of removedSkills) { - await input.onLog("stdout", `[paperclip] Removed maintainer-only ACPX Gemini skill "${skillName}" from ${skillsHome}\n`); + await input.onLog("stdout", `[paperclip] Removed stale ACPX Gemini skill "${skillName}" from ${skillsHome}\n`); } for (const entry of selectedSkills) { @@ -1341,6 +1342,7 @@ async function prepareGeminiSkillRuntime(input: { ); } } + await writeManagedGeminiSkillsManifest(skillsHome, allowedSkillNames); return { identity: { diff --git a/packages/adapter-utils/src/server-utils.test.ts b/packages/adapter-utils/src/server-utils.test.ts index dc9c99d1ecd..e48cdc91879 100644 --- a/packages/adapter-utils/src/server-utils.test.ts +++ b/packages/adapter-utils/src/server-utils.test.ts @@ -20,12 +20,14 @@ import { materializePaperclipSkillCopy, PAPERCLIP_OPERATIONAL_SKILL_KEY, refreshPaperclipWorkspaceEnvForExecution, + removeMaintainerOnlySkillSymlinks, renderPaperclipWakePrompt, resolveLegacyPaperclipDesiredSkillNames, resolvePaperclipDesiredSkillNames, selectPaperclipTaskMarkdown, runningProcesses, runChildProcess, + writeManagedGeminiSkillsManifest, sanitizeSshRemoteEnv, signalRunningProcess, shapePaperclipWorkspaceEnvForExecution, @@ -349,6 +351,140 @@ describe("materializePaperclipSkillCopy", () => { }); }); +describe("removeMaintainerOnlySkillSymlinks", () => { + it("prunes a stale managed directory the manifest names, and keeps an unmanaged directory", async () => { + const root = await fs.mkdtemp( + path.join(os.tmpdir(), "paperclip-gemini-skills-"), + ); + try { + const skillsHome = path.join(root, "skills"); + await fs.mkdir(skillsHome, { recursive: true }); + + // A skill this lane materialized as a plain directory (the shape an + // earlier build without the manifest could leave behind) and no + // longer selects. + const staleManagedDir = path.join(skillsHome, "old-skill"); + await fs.mkdir(staleManagedDir, { recursive: true }); + await fs.writeFile( + path.join(staleManagedDir, "SKILL.md"), + "# old\n", + "utf8", + ); + await writeManagedGeminiSkillsManifest(skillsHome, ["old-skill"]); + + // An entry the manifest never named: a skill the user put in their + // own Gemini skills home, not one Paperclip materialized. + const unmanagedDir = path.join(skillsHome, "users-own-skill"); + await fs.mkdir(unmanagedDir, { recursive: true }); + await fs.writeFile( + path.join(unmanagedDir, "SKILL.md"), + "# mine\n", + "utf8", + ); + + const removed = await removeMaintainerOnlySkillSymlinks(skillsHome, []); + + expect(removed).toEqual(["old-skill"]); + await expect(fs.stat(staleManagedDir)).rejects.toThrow(); + await expect( + fs.readFile(path.join(unmanagedDir, "SKILL.md"), "utf8"), + ).resolves.toBe("# mine\n"); + } finally { + await fs.rm(root, { recursive: true, force: true }); + } + }); + + it("prunes a stale managed symlink the manifest names", async () => { + const root = await fs.mkdtemp( + path.join(os.tmpdir(), "paperclip-gemini-skills-"), + ); + try { + const skillsHome = path.join(root, "skills"); + const source = path.join(root, "source-skill"); + await fs.mkdir(skillsHome, { recursive: true }); + await fs.mkdir(source, { recursive: true }); + await fs.writeFile(path.join(source, "SKILL.md"), "# skill\n", "utf8"); + + const target = path.join(skillsHome, "linked-skill"); + await fs.symlink(source, target); + await writeManagedGeminiSkillsManifest(skillsHome, ["linked-skill"]); + + const removed = await removeMaintainerOnlySkillSymlinks(skillsHome, []); + + expect(removed).toEqual(["linked-skill"]); + await expect(fs.lstat(target)).rejects.toThrow(); + // The link is gone, but the skill source it pointed to must survive — + // this lane only ever prunes its own target directory. + await expect( + fs.readFile(path.join(source, "SKILL.md"), "utf8"), + ).resolves.toBe("# skill\n"); + } finally { + await fs.rm(root, { recursive: true, force: true }); + } + }); + + it("keeps a non-symlink entry the manifest never named, even when it is not selected", async () => { + const root = await fs.mkdtemp( + path.join(os.tmpdir(), "paperclip-gemini-skills-"), + ); + try { + const skillsHome = path.join(root, "skills"); + await fs.mkdir(skillsHome, { recursive: true }); + const unmanagedDir = path.join(skillsHome, "users-own-skill"); + await fs.mkdir(unmanagedDir, { recursive: true }); + await fs.writeFile( + path.join(unmanagedDir, "SKILL.md"), + "# mine\n", + "utf8", + ); + // No manifest is written at all: this skills home predates the + // manifest, or the entry was never Paperclip's to manage. + + const removed = await removeMaintainerOnlySkillSymlinks(skillsHome, []); + + expect(removed).toEqual([]); + await expect( + fs.readFile(path.join(unmanagedDir, "SKILL.md"), "utf8"), + ).resolves.toBe("# mine\n"); + } finally { + await fs.rm(root, { recursive: true, force: true }); + } + }); + + it("still removes a legacy maintainer-only symlink absent from any manifest", async () => { + const root = await fs.mkdtemp( + path.join(os.tmpdir(), "paperclip-gemini-skills-"), + ); + try { + const skillsHome = path.join(root, "skills"); + const maintainerSource = path.join( + root, + ".agents", + "skills", + "maintainer-skill", + ); + await fs.mkdir(skillsHome, { recursive: true }); + await fs.mkdir(maintainerSource, { recursive: true }); + await fs.writeFile( + path.join(maintainerSource, "SKILL.md"), + "# maintainer\n", + "utf8", + ); + + const target = path.join(skillsHome, "maintainer-skill"); + await fs.symlink(maintainerSource, target); + // No manifest is written: this covers the pre-manifest legacy case. + + const removed = await removeMaintainerOnlySkillSymlinks(skillsHome, []); + + expect(removed).toEqual(["maintainer-skill"]); + await expect(fs.lstat(target)).rejects.toThrow(); + } finally { + await fs.rm(root, { recursive: true, force: true }); + } + }); +}); + describe("adapter skill snapshots", () => { const requiredEntry = { key: "paperclipai/paperclip/paperclip", diff --git a/packages/adapter-utils/src/server-utils.ts b/packages/adapter-utils/src/server-utils.ts index e7abb155ac4..827a32d7efa 100644 --- a/packages/adapter-utils/src/server-utils.ts +++ b/packages/adapter-utils/src/server-utils.ts @@ -174,6 +174,7 @@ const PAPERCLIP_SKILL_ROOT_RELATIVE_CANDIDATES = [ const MATERIALIZED_SKILL_SENTINEL = ".paperclip-materialized-skill.json"; const MATERIALIZED_SKILL_LOCK_OWNER = "owner.json"; const MATERIALIZED_SKILL_LOCK_STALE_MS = 30_000; +const MANAGED_GEMINI_SKILLS_MANIFEST = ".paperclip-managed-skills.json"; function expandHomePrefix(value: string): string { if (value === "~") return os.homedir(); @@ -4453,11 +4454,63 @@ export async function materializePaperclipSkillCopy( } } +/** + * Read the set of skill names the Gemini lane manages in `skillsHome`. A + * managed name is a name `writeManagedGeminiSkillsManifest` wrote after this + * lane materialized it. A missing or unreadable manifest yields an empty + * set, so a skills home from before this manifest existed manages nothing + * yet — the legacy symlink check in `removeMaintainerOnlySkillSymlinks` + * still covers that case. + */ +export async function readManagedGeminiSkillsManifest( + skillsHome: string, +): Promise> { + try { + const raw = JSON.parse( + await fs.readFile( + path.join(skillsHome, MANAGED_GEMINI_SKILLS_MANIFEST), + "utf8", + ), + ) as unknown; + const parsed = parseObject(raw); + const names = Array.isArray(parsed.managedSkillNames) + ? parsed.managedSkillNames.filter( + (value): value is string => + typeof value === "string" && value.trim().length > 0, + ) + : []; + return new Set(names); + } catch { + return new Set(); + } +} + +/** + * Record the skill names the Gemini lane just materialized into + * `skillsHome`. Call this after every materialize pass, so the manifest + * always names exactly the entries this lane owns. Only a name in this + * manifest is a valid prune target for `removeMaintainerOnlySkillSymlinks` — + * a name never written here is a skill the user or another tool put in + * their own Gemini skills home, and it must survive. + */ +export async function writeManagedGeminiSkillsManifest( + skillsHome: string, + skillNames: Iterable, +): Promise { + const managedSkillNames = Array.from(new Set(skillNames)).sort(); + await fs.writeFile( + path.join(skillsHome, MANAGED_GEMINI_SKILLS_MANIFEST), + `${JSON.stringify({ version: 1, managedSkillNames }, null, 2)}\n`, + "utf8", + ); +} + export async function removeMaintainerOnlySkillSymlinks( skillsHome: string, allowedSkillNames: Iterable, ): Promise { const allowed = new Set(Array.from(allowedSkillNames)); + const managed = await readManagedGeminiSkillsManifest(skillsHome); try { const entries = await fs.readdir(skillsHome, { withFileTypes: true }); const removed: string[] = []; @@ -4465,6 +4518,20 @@ export async function removeMaintainerOnlySkillSymlinks( if (allowed.has(entry.name)) continue; const target = path.join(skillsHome, entry.name); + + // A name in the manifest is a skill this lane owns. Prune it in full — + // a directory, a file, or a symlink to any source — because the lane + // itself wrote it and no longer selects it. + if (managed.has(entry.name)) { + await fs.rm(target, { recursive: true, force: true }).catch(() => {}); + removed.push(entry.name); + continue; + } + + // A name outside the manifest predates it, or belongs to the user. + // Only remove it when it is a symlink into a maintainer-only source — + // the narrow legacy case a skills home can carry from before this + // manifest existed. Every other unmanaged entry survives untouched. const existing = await fs.lstat(target).catch(() => null); if (!existing?.isSymbolicLink()) continue; diff --git a/packages/adapters/gemini-local/src/server/execute.ts b/packages/adapters/gemini-local/src/server/execute.ts index 5ecfd95b438..288e144bb4b 100644 --- a/packages/adapters/gemini-local/src/server/execute.ts +++ b/packages/adapters/gemini-local/src/server/execute.ts @@ -44,6 +44,7 @@ import { readPaperclipIssueWorkModeFromContext, resolveLegacyPaperclipDesiredSkillNames, removeMaintainerOnlySkillSymlinks, + writeManagedGeminiSkillsManifest, parseObject, renderTemplate, renderPaperclipWakePrompt, @@ -164,7 +165,7 @@ async function ensureGeminiSkillsInjected( for (const skillName of removedSkills) { await onLog( "stderr", - `[paperclip] Removed maintainer-only Gemini skill "${skillName}" from ${skillsHome}\n`, + `[paperclip] Removed stale Gemini skill "${skillName}" from ${skillsHome}\n`, ); } @@ -185,6 +186,10 @@ async function ensureGeminiSkillsInjected( ); } } + await writeManagedGeminiSkillsManifest( + skillsHome, + selectedEntries.map((entry) => entry.runtimeName), + ); } async function buildGeminiSkillsDir(