RomanNum3ral
/
paperclip
mirror of https://github.com/paperclipai/paperclip.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix(adapter-utils): prune a Gemini managed skill only when its shape on disk is still lane-owned 

The prune step removed any entry the managed-skills manifest named, with
no check of what the entry currently is. The manifest records what the
lane owned at the end of the last run, not what the entry is now. A user
could remove a managed symbolic link and write their own directory at
the same name between runs; the next prune then deleted that directory.

Add a shape check at the point of removal. A manifest-named entry is
still lane-owned only when it is a symbolic link, or a directory that
carries a valid materialized-skill sentinel. Every other shape survives
the prune, and Paperclip logs one line to record why.

Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
nickyleach 2026-09-11 15:17:40 +00:00
parent 4f1bb2817d
commit 57fd03312f
2 changed files with 136 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

76
packages/adapter-utils/src/server-utils.test.ts
View File

@ -362,16 +362,18 @@ describe("removeMaintainerOnlySkillSymlinks", () => {
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.
// A skill this lane materialized as an owned copy (the shape the
// Agent Client Protocol lane's permission-error fallback writes) and
// no longer selects.
const staleManagedDir = path.join(skillsHome, "old-skill");
await fs.mkdir(staleManagedDir, { recursive: true });
const staleSource = path.join(root, "old-skill-source");
await fs.mkdir(staleSource, { recursive: true });
await fs.writeFile(
path.join(staleManagedDir, "SKILL.md"),
path.join(staleSource, "SKILL.md"),
"# old\n",
"utf8",
);
await materializePaperclipSkillCopy(staleSource, staleManagedDir);
await writeManagedGeminiSkillsManifest(skillsHome, ["old-skill"]);
// An entry the manifest never named: a skill the user put in their
@ -485,6 +487,70 @@ describe("removeMaintainerOnlySkillSymlinks", () => {
await fs.rm(root, { recursive: true, force: true });
}
});
it("keeps a user's own directory that replaced a formerly managed entry between runs", 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"), "# notes\n", "utf8");
// Run 1: the lane links the selected skill and records it as managed.
const target = path.join(skillsHome, "notes");
await ensurePaperclipSkillSymlink(source, target);
await writeManagedGeminiSkillsManifest(skillsHome, ["notes"]);
// Between runs, the user removes the link by hand and writes their
// own directory at the same name.
await fs.unlink(target);
await fs.mkdir(target, { recursive: true });
await fs.writeFile(
path.join(target, "SKILL.md"),
"# my own notes\n",
"utf8",
);
// Run 2: the skill is no longer selected. The manifest still names
// "notes", but the entry on disk is now the user's own directory, not
// a lane-owned shape. The prune must keep it and must not report it
// as removed.
const removed = await removeMaintainerOnlySkillSymlinks(skillsHome, []);
expect(removed).toEqual([]);
await expect(
fs.readFile(path.join(target, "SKILL.md"), "utf8"),
).resolves.toBe("# my own notes\n");
} finally {
await fs.rm(root, { recursive: true, force: true });
}
});
it("keeps a manifest-named entry that is a regular file, not a lane-owned shape", 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 target = path.join(skillsHome, "stray-file");
await fs.writeFile(target, "not a skill\n", "utf8");
await writeManagedGeminiSkillsManifest(skillsHome, ["stray-file"]);
const removed = await removeMaintainerOnlySkillSymlinks(skillsHome, []);
expect(removed).toEqual([]);
await expect(fs.readFile(target, "utf8")).resolves.toBe(
"not a skill\n",
);
} finally {
await fs.rm(root, { recursive: true, force: true });
}
});
});
describe("Gemini managed-skills manifest records only owned entries", () => {

84
packages/adapter-utils/src/server-utils.ts
View File

@ -4505,6 +4505,29 @@ export async function writeManagedGeminiSkillsManifest(
);
}
/**
* Test if `target` carries a valid materialized-skill sentinel, the marker
* `materializePaperclipSkillCopy` writes into a directory it owns.
*/
async function hasValidMaterializedSkillSentinel(
target: string,
): Promise<boolean> {
try {
const raw = JSON.parse(
await fs.readFile(
path.join(target, MATERIALIZED_SKILL_SENTINEL),
"utf8",
),
) as unknown;
const parsed = parseObject(raw);
return (
parsed.version === 1 && typeof parsed.sourceFingerprint === "string"
);
} catch {
return false;
}
}
/**
* Test if the Gemini lane owns the entry at `target` after it materializes
* `source` there. Only an owned entry may enter the managed-skills
@ -4537,25 +4560,37 @@ export async function isManagedGeminiSkillEntry(
}
if (existing.isDirectory()) {
try {
const raw = JSON.parse(
await fs.readFile(
path.join(target, MATERIALIZED_SKILL_SENTINEL),
"utf8",
),
) as unknown;
const parsed = parseObject(raw);
return (
parsed.version === 1 && typeof parsed.sourceFingerprint === "string"
);
} catch {
return false;
}
return hasValidMaterializedSkillSentinel(target);
}
return false;
}
/**
* Test if `target` carries a shape the Gemini lane could have created, with
* no check against a specific skill source. `removeMaintainerOnlySkillSymlinks`
* calls this at the point of removal, because the manifest only records what
* the lane owned at the end of the last run — it does not prove the entry is
* still the lane's now. A lane-owned shape is one of:
*
* - a symbolic link, of any target; or
* - a directory that carries a valid materialized-skill sentinel.
*
* A plain directory with no sentinel, a regular file, and every other entry
* type are not a lane-owned shape, even when the manifest names the entry.
*/
export async function isLaneOwnedSkillEntryShape(
target: string,
): Promise<boolean> {
const existing = await fs.lstat(target).catch(() => null);
if (!existing) return false;
if (existing.isSymbolicLink()) return true;
if (existing.isDirectory()) {
return hasValidMaterializedSkillSentinel(target);
}
return false;
}
export async function removeMaintainerOnlySkillSymlinks(
skillsHome: string,
allowedSkillNames: Iterable<string>,
@ -4570,12 +4605,23 @@ export async function removeMaintainerOnlySkillSymlinks(
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.
// A name in the manifest is a skill this lane owned at the end of the
// last run. That does not prove the lane still owns it now: a user
// action between runs can replace the entry with their own directory
// at the same name. Remove it only when its shape on disk is still
// lane-owned — a symbolic link, or a directory with a valid
// materialized-skill sentinel.
if (managed.has(entry.name)) {
await fs.rm(target, { recursive: true, force: true }).catch(() => {});
removed.push(entry.name);
if (await isLaneOwnedSkillEntryShape(target)) {
await fs.rm(target, { recursive: true, force: true }).catch(() => {});
removed.push(entry.name);
} else {
// eslint-disable-next-line no-console
console.warn(
`[paperclip] kept Gemini managed-skill entry "${entry.name}" — ` +
"its shape on disk is not lane-owned.",
);
}
continue;
}