mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-26 02:00:52 +01:00
Fix skills store file reads and version snapshots (PAP-10910)
- readFile: fall back to stored markdown when a local/catalog source file is missing instead of 500ing on ENOENT; treat an explicit "."/"" github repoSkillDir as repo root instead of appending the slug; fall back to stored markdown when the github fetch fails for SKILL.md - updateFile: snapshot a version after each changed save (skips no-op saves) and thread the acting user/agent through the route - createLocalSkill: seed an "Initial version" so the Versions tab is populated from creation - migration 0100: backfill null install_count and enforce default/not-null Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
1 parent
e8d2920977
commit
94c27257ec
5 files changed
+203
-8
No files matched your search
@@ -0,0 +1,3 @@
|
||||
UPDATE "company_skills" SET "install_count" = 0 WHERE "install_count" IS NULL;--> statement-breakpoint
|
||||
ALTER TABLE "company_skills" ALTER COLUMN "install_count" SET DEFAULT 0;--> statement-breakpoint
|
||||
ALTER TABLE "company_skills" ALTER COLUMN "install_count" SET NOT NULL;
|
||||
@@ -701,6 +701,13 @@
|
||||
"when": 1781130000000,
|
||||
"tag": "0099_skills_store_foundation",
|
||||
"breakpoints": true
|
||||
},
|
||||
{
|
||||
"idx": 100,
|
||||
"version": "7",
|
||||
"when": 1781480000000,
|
||||
"tag": "0100_skill_install_count_backfill",
|
||||
"breakpoints": true
|
||||
}
|
||||
]
|
||||
}
|
||||
@@ -2,7 +2,7 @@ import { randomUUID } from "node:crypto";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { promises as fs } from "node:fs";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest";
|
||||
import { agents, companies, companySkills, createDb } from "@paperclipai/db";
|
||||
import {
|
||||
getEmbeddedPostgresTestSupport,
|
||||
@@ -408,6 +408,16 @@ describeEmbeddedPostgres("companySkillService.list", () => {
|
||||
]);
|
||||
const version = await svc.createVersion(companyId, skillId, {}, { type: "user", userId: "board" });
|
||||
|
||||
await expect(svc.resolveRequestedSkillEntries(companyId, [
|
||||
"pinned-skill",
|
||||
])).resolves.toEqual([
|
||||
{ key: `company/${companyId}/pinned-skill`, versionId: version.id },
|
||||
]);
|
||||
await expect(svc.resolveRequestedSkillEntries(companyId, [
|
||||
{ key: "pinned-skill", versionId: null },
|
||||
])).resolves.toEqual([
|
||||
{ key: `company/${companyId}/pinned-skill`, versionId: version.id },
|
||||
]);
|
||||
await expect(svc.resolveRequestedSkillEntries(companyId, [
|
||||
{ key: "pinned-skill", versionId: version.id },
|
||||
])).resolves.toEqual([
|
||||
@@ -759,4 +769,150 @@ describeEmbeddedPostgres("companySkillService.list", () => {
|
||||
"# Runtime Coach\n\nRecovered from DB.\n",
|
||||
);
|
||||
});
|
||||
|
||||
it("falls back to stored markdown when reading SKILL.md from a missing local source", async () => {
|
||||
const companyId = randomUUID();
|
||||
const skillId = randomUUID();
|
||||
const skillKey = `company/${companyId}/missing-reader`;
|
||||
const missingSkillDir = path.join(await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-missing-read-skill-")), "gone");
|
||||
cleanupDirs.add(path.dirname(missingSkillDir));
|
||||
|
||||
await db.insert(companies).values({
|
||||
id: companyId,
|
||||
name: "Paperclip",
|
||||
issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`,
|
||||
requireBoardApprovalForNewAgents: false,
|
||||
});
|
||||
await db.insert(companySkills).values({
|
||||
id: skillId,
|
||||
companyId,
|
||||
key: skillKey,
|
||||
slug: "missing-reader",
|
||||
name: "Missing Reader",
|
||||
description: null,
|
||||
markdown: "# Missing Reader\n\nRecovered from DB.\n",
|
||||
sourceType: "local_path",
|
||||
sourceLocator: missingSkillDir,
|
||||
trustLevel: "markdown_only",
|
||||
compatibility: "compatible",
|
||||
fileInventory: [
|
||||
{ path: "SKILL.md", kind: "skill" },
|
||||
{ path: "references/guide.md", kind: "reference" },
|
||||
],
|
||||
metadata: { sourceKind: "local_path" },
|
||||
});
|
||||
await db.insert(agents).values({
|
||||
id: randomUUID(),
|
||||
companyId,
|
||||
name: "Reader",
|
||||
role: "engineer",
|
||||
status: "active",
|
||||
adapterType: "codex_local",
|
||||
adapterConfig: {
|
||||
paperclipSkillSync: {
|
||||
desiredSkills: [skillKey],
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
await expect(svc.readFile(companyId, skillId, "SKILL.md")).resolves.toMatchObject({
|
||||
path: "SKILL.md",
|
||||
content: "# Missing Reader\n\nRecovered from DB.\n",
|
||||
});
|
||||
await expect(svc.readFile(companyId, skillId, "references/guide.md")).rejects.toMatchObject({
|
||||
status: 404,
|
||||
});
|
||||
});
|
||||
|
||||
it("reads root-level SKILL.md for github skills with a '.' repoSkillDir", async () => {
|
||||
const companyId = randomUUID();
|
||||
const skillId = randomUUID();
|
||||
await db.insert(companies).values({
|
||||
id: companyId,
|
||||
name: "Paperclip",
|
||||
issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`,
|
||||
requireBoardApprovalForNewAgents: false,
|
||||
});
|
||||
await db.insert(companySkills).values({
|
||||
id: skillId,
|
||||
companyId,
|
||||
key: `company/${companyId}/root-skill`,
|
||||
slug: "root-skill",
|
||||
name: "Root Skill",
|
||||
description: null,
|
||||
markdown: "# Root Skill (stored)\n",
|
||||
sourceType: "github",
|
||||
sourceLocator: "https://github.com/acme/root-skill",
|
||||
sourceRef: "main",
|
||||
trustLevel: "markdown_only",
|
||||
compatibility: "compatible",
|
||||
fileInventory: [{ path: "SKILL.md", kind: "skill" }],
|
||||
metadata: { owner: "acme", repo: "root-skill", ref: "main", repoSkillDir: "." },
|
||||
});
|
||||
|
||||
const requestedUrls: string[] = [];
|
||||
vi.stubGlobal("fetch", async (url: string | URL) => {
|
||||
requestedUrls.push(String(url));
|
||||
return new Response("# Root Skill (remote)\n", { status: 200 });
|
||||
});
|
||||
try {
|
||||
await expect(svc.readFile(companyId, skillId, "SKILL.md")).resolves.toMatchObject({
|
||||
content: "# Root Skill (remote)\n",
|
||||
});
|
||||
expect(requestedUrls).toEqual([
|
||||
"https://raw.githubusercontent.com/acme/root-skill/main/SKILL.md",
|
||||
]);
|
||||
|
||||
vi.stubGlobal("fetch", async () => {
|
||||
throw new Error("network down");
|
||||
});
|
||||
await expect(svc.readFile(companyId, skillId, "SKILL.md")).resolves.toMatchObject({
|
||||
content: "# Root Skill (stored)\n",
|
||||
});
|
||||
} finally {
|
||||
vi.unstubAllGlobals();
|
||||
}
|
||||
});
|
||||
|
||||
it("seeds an initial version on create and snapshots a version on each changed save", async () => {
|
||||
const companyId = randomUUID();
|
||||
await db.insert(companies).values({
|
||||
id: companyId,
|
||||
name: "Paperclip",
|
||||
issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`,
|
||||
requireBoardApprovalForNewAgents: false,
|
||||
});
|
||||
|
||||
const skill = await svc.createLocalSkill(
|
||||
companyId,
|
||||
{ name: "Versioned Editor", description: "Edits with history" },
|
||||
{ type: "user", userId: "board" },
|
||||
);
|
||||
expect(skill.currentVersionId).not.toBeNull();
|
||||
let versions = await svc.listVersions(companyId, skill.id);
|
||||
expect(versions).toHaveLength(1);
|
||||
expect(versions[0]).toMatchObject({
|
||||
revisionNumber: 1,
|
||||
label: "Initial version",
|
||||
authorUserId: "board",
|
||||
});
|
||||
expect(skill.currentVersionId).toBe(versions[0]!.id);
|
||||
|
||||
const editedMarkdown = "---\nname: Versioned Editor\n---\n\n# Versioned Editor\n\nEdited body.\n";
|
||||
await expect(svc.updateFile(companyId, skill.id, "SKILL.md", editedMarkdown, { type: "user", userId: "board" }))
|
||||
.resolves.toMatchObject({ path: "SKILL.md", content: editedMarkdown });
|
||||
versions = await svc.listVersions(companyId, skill.id);
|
||||
expect(versions).toHaveLength(2);
|
||||
expect(versions[0]).toMatchObject({ revisionNumber: 2, authorUserId: "board" });
|
||||
expect(versions[0]!.fileInventory).toEqual([
|
||||
expect.objectContaining({ path: "SKILL.md", content: editedMarkdown }),
|
||||
]);
|
||||
await expect(svc.getById(companyId, skill.id)).resolves.toMatchObject({
|
||||
currentVersionId: versions[0]!.id,
|
||||
});
|
||||
|
||||
await svc.updateFile(companyId, skill.id, "SKILL.md", editedMarkdown, { type: "user", userId: "board" });
|
||||
versions = await svc.listVersions(companyId, skill.id);
|
||||
expect(versions).toHaveLength(2);
|
||||
});
|
||||
});
|
||||
@@ -458,6 +458,7 @@ export function companySkillRoutes(db: Db) {
|
||||
skillId,
|
||||
String(req.body.path ?? ""),
|
||||
String(req.body.content ?? ""),
|
||||
skillActor(req),
|
||||
);
|
||||
|
||||
const actor = getActorInfo(req);
|
||||
|
||||
@@ -3011,12 +3011,15 @@ export function companySkillService(db: Db) {
|
||||
|
||||
if (skill.sourceType === "local_path" || skill.sourceType === "catalog") {
|
||||
const absolutePath = resolveLocalSkillFilePath(skill, normalizedPath);
|
||||
if (absolutePath) {
|
||||
content = await fs.readFile(absolutePath, "utf8");
|
||||
const diskContent = absolutePath
|
||||
? await fs.readFile(absolutePath, "utf8").catch(() => null)
|
||||
: null;
|
||||
if (diskContent !== null) {
|
||||
content = diskContent;
|
||||
} else if (normalizedPath === "SKILL.md") {
|
||||
content = skill.markdown;
|
||||
} else {
|
||||
throw notFound("Skill file not found");
|
||||
throw notFound("Skill file is unavailable: the skill source directory is missing.");
|
||||
}
|
||||
} else if (skill.sourceType === "github" || skill.sourceType === "skills_sh") {
|
||||
const metadata = getSkillMeta(skill);
|
||||
@@ -3024,12 +3027,25 @@ export function companySkillService(db: Db) {
|
||||
const repo = asString(metadata.repo);
|
||||
const hostname = asString(metadata.hostname) || "github.com";
|
||||
const ref = skill.sourceRef ?? asString(metadata.ref) ?? "main";
|
||||
const repoSkillDir = normalizeGitHubSkillDirectory(asString(metadata.repoSkillDir), skill.slug);
|
||||
const rawRepoSkillDir = asString(metadata.repoSkillDir);
|
||||
// An explicit "."/"" repoSkillDir means SKILL.md lives at the repo root;
|
||||
// only fall back to the slug subdirectory when metadata is absent.
|
||||
const repoSkillDir = rawRepoSkillDir == null
|
||||
? normalizeGitHubSkillDirectory(rawRepoSkillDir, skill.slug)
|
||||
: normalizeGitHubSkillDirectory(rawRepoSkillDir, "");
|
||||
if (!owner || !repo) {
|
||||
throw unprocessable("Skill source metadata is incomplete.");
|
||||
}
|
||||
const repoPath = normalizePortablePath(path.posix.join(repoSkillDir, normalizedPath));
|
||||
content = await fetchText(resolveRawGitHubUrl(hostname, owner, repo, ref, repoPath));
|
||||
try {
|
||||
content = await fetchText(resolveRawGitHubUrl(hostname, owner, repo, ref, repoPath));
|
||||
} catch (error) {
|
||||
if (normalizedPath === "SKILL.md" && skill.markdown) {
|
||||
content = skill.markdown;
|
||||
} else {
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
} else if (skill.sourceType === "url") {
|
||||
if (normalizedPath !== "SKILL.md") {
|
||||
throw notFound("This skill source only exposes SKILL.md");
|
||||
@@ -3211,10 +3227,17 @@ export function companySkillService(db: Db) {
|
||||
})
|
||||
.where(and(eq(companySkills.id, forkSource.id), eq(companySkills.companyId, companyId)));
|
||||
}
|
||||
return row ? toCompanySkill(row) : created;
|
||||
await createVersion(companyId, created.id, { label: "Initial version" }, actor);
|
||||
return (await getById(companyId, created.id)) ?? (row ? toCompanySkill(row) : created);
|
||||
}
|
||||
|
||||
async function updateFile(companyId: string, skillId: string, relativePath: string, content: string): Promise<CompanySkillFileDetail> {
|
||||
async function updateFile(
|
||||
companyId: string,
|
||||
skillId: string,
|
||||
relativePath: string,
|
||||
content: string,
|
||||
actor: SkillActor | null = null,
|
||||
): Promise<CompanySkillFileDetail> {
|
||||
await ensureSkillInventoryCurrent(companyId);
|
||||
const skill = await getById(companyId, skillId);
|
||||
if (!skill) throw notFound("Skill not found");
|
||||
@@ -3228,6 +3251,7 @@ export function companySkillService(db: Db) {
|
||||
const absolutePath = resolveLocalSkillFilePath(skill, normalizedPath);
|
||||
if (!absolutePath) throw notFound("Skill file not found");
|
||||
|
||||
const previousContent = await fs.readFile(absolutePath, "utf8").catch(() => null);
|
||||
await fs.mkdir(path.dirname(absolutePath), { recursive: true });
|
||||
await fs.writeFile(absolutePath, content, "utf8");
|
||||
|
||||
@@ -3250,6 +3274,10 @@ export function companySkillService(db: Db) {
|
||||
.where(eq(companySkills.id, skill.id));
|
||||
}
|
||||
|
||||
if (previousContent !== content) {
|
||||
await createVersion(companyId, skillId, {}, actor);
|
||||
}
|
||||
|
||||
const detail = await readFile(companyId, skillId, normalizedPath);
|
||||
if (!detail) throw notFound("Skill file not found");
|
||||
return detail;
|
||||
|
||||
Reference in new issue
Block a user