feat(skills): add managed skill rename API (#9688)
## Thinking Path > - Paperclip is the open source control plane people use to manage AI-agent companies. > - Company skills are reusable capabilities that operators install, edit, assign, and materialize for agents. > - Managed local skills currently lack a safe backend operation for changing their display name and canonical slug/key together. > - Treating rename as an ordinary save can leave duplicate records, stale runtime materializations, or agent assignments pointing at the old key. > - This pull request adds a company-scoped managed-skill rename contract, service operation, and REST endpoint with focused authorization and activity logging. > - The benefit is an atomic-enough, recoverable rename path that keeps disk state, database identity, and agent skill assignments synchronized. ## Linked Issues or Issue Description - Refs #2121 - Problem: managed company skills need a dedicated rename operation rather than save-time duplication behavior. - Expected behavior: renaming a managed skill updates its name, slug, key, source directory, frontmatter, runtime materialization, and assigned-agent references while preserving version pins. ## What Changed - Added shared request/result types and Zod validation for managed skill rename requests. - Added `POST /api/companies/:companyId/skills/:skillId/rename` with `skills.edit` policy checks and `company.skill_renamed` activity logging. - Restricted renames to Paperclip-managed local skills and added slug, key, and target-directory conflict handling. - Moved the managed directory, rewrote only the `SKILL.md` frontmatter name, updated the database row, and rolled filesystem changes back when persistence fails. - Rewrote assigned agents' desired-skill keys while preserving pinned version IDs and removed stale runtime materialization. - Added focused route and service coverage for success, no-op, name-only changes, conflicts, unsupported sources, assignment rewrites, rollback-sensitive behavior, and runtime cleanup. - Rejected multiline rename names before they can inject extra `SKILL.md` frontmatter fields. ## Verification - `pnpm exec vitest run server/src/__tests__/company-skills-service.test.ts server/src/__tests__/company-skills-routes.test.ts` — 106 tests passed. - `pnpm --filter @paperclipai/shared typecheck` — passed. - `pnpm --filter @paperclipai/server typecheck` — passed. ## Risks - Filesystem and database updates cannot share one native transaction; the service stages filesystem changes and explicitly restores the original directory and markdown when the database transaction fails. - Renames intentionally reject catalog, remote, project-scanned, and unmanaged local skills to avoid changing identities owned by external sources. - No database migration is required; the endpoint updates existing company-skill and agent configuration fields. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex coding agent (exact underlying model ID and context-window size were not exposed to this runtime), with reasoning, repository tool use, code execution, and test execution. The rescued source commit also records assistance from Claude Opus 4.8. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Dotta
GitHub
parent
efe8b3b707
commit
fcf66f3a91
|
|
@ -593,6 +593,8 @@ export type {
|
|||
CompanySkillForkReassignment,
|
||||
CompanySkillForkResult,
|
||||
CompanySkillForkPrecheckResult,
|
||||
CompanySkillRenameRequest,
|
||||
CompanySkillRenameResult,
|
||||
CompanySkillUpdateRequest,
|
||||
CompanySkillUpdateStatus,
|
||||
CompanySkillAuditSeverity,
|
||||
|
|
@ -1958,6 +1960,8 @@ export {
|
|||
companySkillCommentCreateSchema,
|
||||
companySkillCommentUpdateSchema,
|
||||
companySkillForkSchema,
|
||||
companySkillRenameSchema,
|
||||
companySkillRenameResultSchema,
|
||||
companySkillUpdateSchema,
|
||||
companySkillUpdateStatusSchema,
|
||||
companySkillAuditFindingSchema,
|
||||
|
|
|
|||
|
|
@ -247,6 +247,19 @@ export interface CompanySkillForkPrecheckResult {
|
|||
existingForks: CompanySkillForkSummary[];
|
||||
}
|
||||
|
||||
export interface CompanySkillRenameRequest {
|
||||
name: string;
|
||||
slug?: string | null;
|
||||
}
|
||||
|
||||
export interface CompanySkillRenameResult {
|
||||
skill: CompanySkill;
|
||||
previousName: string;
|
||||
previousSlug: string;
|
||||
previousKey: string;
|
||||
reassignments: CompanySkillForkReassignment[];
|
||||
}
|
||||
|
||||
export interface CompanySkillUpdateRequest {
|
||||
description?: string | null;
|
||||
iconUrl?: string | null;
|
||||
|
|
|
|||
|
|
@ -141,6 +141,8 @@ export type {
|
|||
CompanySkillForkReassignment,
|
||||
CompanySkillForkResult,
|
||||
CompanySkillForkPrecheckResult,
|
||||
CompanySkillRenameRequest,
|
||||
CompanySkillRenameResult,
|
||||
CompanySkillUpdateRequest,
|
||||
CompanySkillUpdateStatus,
|
||||
CompanySkillAuditSeverity,
|
||||
|
|
|
|||
|
|
@ -200,6 +200,19 @@ export const companySkillForkPrecheckResultSchema = z.object({
|
|||
existingForks: z.array(companySkillForkSummarySchema),
|
||||
});
|
||||
|
||||
export const companySkillRenameSchema = z.object({
|
||||
name: z.string().min(1).regex(/^[^\r\n]+$/, "Name must be a single line"),
|
||||
slug: z.string().min(1).nullable().optional(),
|
||||
});
|
||||
|
||||
export const companySkillRenameResultSchema = z.object({
|
||||
skill: companySkillSchema,
|
||||
previousName: z.string(),
|
||||
previousSlug: z.string(),
|
||||
previousKey: z.string(),
|
||||
reassignments: z.array(companySkillForkReassignmentSchema),
|
||||
});
|
||||
|
||||
export const companySkillUpdateSchema = z.object({
|
||||
description: z.string().nullable().optional(),
|
||||
iconUrl: z.string().nullable().optional(),
|
||||
|
|
@ -569,6 +582,7 @@ export type CompanySkillVersionCreate = z.infer<typeof companySkillVersionCreate
|
|||
export type CompanySkillCommentCreate = z.infer<typeof companySkillCommentCreateSchema>;
|
||||
export type CompanySkillCommentUpdate = z.infer<typeof companySkillCommentUpdateSchema>;
|
||||
export type CompanySkillFork = z.infer<typeof companySkillForkSchema>;
|
||||
export type CompanySkillRename = z.infer<typeof companySkillRenameSchema>;
|
||||
export type CompanySkillUpdate = z.infer<typeof companySkillUpdateSchema>;
|
||||
export type CatalogSkillListQuery = z.infer<typeof catalogSkillListQuerySchema>;
|
||||
export type CompanySkillInstallCatalog = z.infer<typeof companySkillInstallCatalogSchema>;
|
||||
|
|
|
|||
|
|
@ -156,6 +156,8 @@ export {
|
|||
companySkillForkReassignmentSchema,
|
||||
companySkillForkResultSchema,
|
||||
companySkillForkPrecheckResultSchema,
|
||||
companySkillRenameSchema,
|
||||
companySkillRenameResultSchema,
|
||||
companySkillUpdateSchema,
|
||||
companySkillUpdateStatusSchema,
|
||||
companySkillAuditFindingSchema,
|
||||
|
|
@ -207,6 +209,7 @@ export {
|
|||
type CompanySkillCommentCreate,
|
||||
type CompanySkillCommentUpdate,
|
||||
type CompanySkillFork,
|
||||
type CompanySkillRename,
|
||||
type CatalogSkillListQuery,
|
||||
type CompanySkillInstallCatalog,
|
||||
type CompanySkillInstallUpdate,
|
||||
|
|
|
|||
|
|
@ -22,6 +22,7 @@ const mockCompanySkillService = vi.hoisted(() => ({
|
|||
starSkill: vi.fn(),
|
||||
unstarSkill: vi.fn(),
|
||||
forkSkill: vi.fn(),
|
||||
renameSkill: vi.fn(),
|
||||
forkPrecheck: vi.fn(),
|
||||
listComments: vi.fn(),
|
||||
createComment: vi.fn(),
|
||||
|
|
@ -315,6 +316,15 @@ describe("company skill mutation permissions", () => {
|
|||
},
|
||||
reassignments: [],
|
||||
});
|
||||
mockCompanySkillService.renameSkill.mockResolvedValue({
|
||||
skill: { ...forkedSkill, id: "skill-1", name: "Ship PR", slug: "ship-pr", key: "company/company-1/ship-pr" },
|
||||
previousName: "Review",
|
||||
previousSlug: "review",
|
||||
previousKey: "company/company-1/review",
|
||||
reassignments: [
|
||||
{ agentId: "11111111-1111-4111-8111-111111111111", previousSkillKey: "company/company-1/review", nextSkillKey: "company/company-1/ship-pr" },
|
||||
],
|
||||
});
|
||||
mockCompanySkillService.forkPrecheck.mockResolvedValue({
|
||||
skillId: "skill-1",
|
||||
original: {
|
||||
|
|
@ -1641,6 +1651,106 @@ describe("company skill mutation permissions", () => {
|
|||
}));
|
||||
});
|
||||
|
||||
it("renames a skill and logs the rename activity", async () => {
|
||||
const app = await createApp({ type: "board", source: "local_implicit", userId: "user-1" });
|
||||
|
||||
const res = await request(app)
|
||||
.post("/api/companies/company-1/skills/skill-1/rename")
|
||||
.send({ name: "Ship PR", slug: "ship-pr" })
|
||||
.expect(200);
|
||||
|
||||
expect(res.body).toMatchObject({
|
||||
skill: { id: "skill-1", name: "Ship PR", slug: "ship-pr", key: "company/company-1/ship-pr" },
|
||||
previousName: "Review",
|
||||
previousSlug: "review",
|
||||
previousKey: "company/company-1/review",
|
||||
});
|
||||
expect(mockCompanySkillService.renameSkill).toHaveBeenCalledWith(
|
||||
"company-1",
|
||||
"skill-1",
|
||||
{ name: "Ship PR", slug: "ship-pr" },
|
||||
);
|
||||
expect(mockLogActivity).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({
|
||||
action: "company.skill_renamed",
|
||||
entityType: "company_skill",
|
||||
entityId: "skill-1",
|
||||
details: expect.objectContaining({
|
||||
previousSlug: "review",
|
||||
slug: "ship-pr",
|
||||
reassignedAgentIds: ["11111111-1111-4111-8111-111111111111"],
|
||||
}),
|
||||
}));
|
||||
});
|
||||
|
||||
it("does not log rename activity for a normalized no-op", async () => {
|
||||
mockCompanySkillService.renameSkill.mockResolvedValueOnce({
|
||||
skill: {
|
||||
id: "skill-1",
|
||||
name: "Review",
|
||||
slug: "review",
|
||||
key: "company/company-1/review",
|
||||
},
|
||||
previousName: "Review",
|
||||
previousSlug: "review",
|
||||
previousKey: "company/company-1/review",
|
||||
reassignments: [],
|
||||
});
|
||||
const app = await createApp({ type: "board", source: "local_implicit", userId: "user-1" });
|
||||
|
||||
await request(app)
|
||||
.post("/api/companies/company-1/skills/skill-1/rename")
|
||||
.send({ name: "Review", slug: "review" })
|
||||
.expect(200);
|
||||
|
||||
expect(mockLogActivity).not.toHaveBeenCalledWith(expect.anything(), expect.objectContaining({
|
||||
action: "company.skill_renamed",
|
||||
}));
|
||||
});
|
||||
|
||||
it("attributes rename activity to the calling agent API key", async () => {
|
||||
const app = await createApp({
|
||||
type: "agent",
|
||||
agentId: "55555555-5555-4555-8555-555555555555",
|
||||
companyId: "company-1",
|
||||
runId: "run-1",
|
||||
keyId: "agent-key-1",
|
||||
source: "agent_key",
|
||||
});
|
||||
|
||||
await request(app)
|
||||
.post("/api/companies/company-1/skills/skill-1/rename")
|
||||
.send({ name: "Ship PR", slug: "ship-pr" })
|
||||
.expect(200);
|
||||
|
||||
expect(mockLogActivity).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({
|
||||
action: "company.skill_renamed",
|
||||
actorType: "agent",
|
||||
agentId: "55555555-5555-4555-8555-555555555555",
|
||||
runId: "run-1",
|
||||
agentApiKeyId: "agent-key-1",
|
||||
}));
|
||||
});
|
||||
|
||||
it("rejects a rename request with a missing name", async () => {
|
||||
const app = await createApp({ type: "board", source: "local_implicit", userId: "user-1" });
|
||||
|
||||
await request(app)
|
||||
.post("/api/companies/company-1/skills/skill-1/rename")
|
||||
.send({ slug: "ship-pr" })
|
||||
.expect(400);
|
||||
expect(mockCompanySkillService.renameSkill).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects a rename request with a multiline name", async () => {
|
||||
const app = await createApp({ type: "board", source: "local_implicit", userId: "user-1" });
|
||||
|
||||
await request(app)
|
||||
.post("/api/companies/company-1/skills/skill-1/rename")
|
||||
.send({ name: "Ship PR\nslug: injected" })
|
||||
.expect(400);
|
||||
expect(mockCompanySkillService.renameSkill).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not synthesize a shared board user id for board actors without user ids", async () => {
|
||||
const app = await createApp({ type: "board", source: "local_implicit" });
|
||||
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@ import {
|
|||
projects,
|
||||
projectWorkspaces,
|
||||
} from "@paperclipai/db";
|
||||
import { parseFrontmatterMarkdown } from "@paperclipai/shared";
|
||||
import {
|
||||
getEmbeddedPostgresTestSupport,
|
||||
startEmbeddedPostgresTestDatabase,
|
||||
|
|
@ -2523,7 +2524,6 @@ describeEmbeddedPostgres("companySkillService.list", () => {
|
|||
const persisted = await db.select().from(companySkills).where(eq(companySkills.companyId, companyId));
|
||||
expect(persisted.filter((skill) => skill.metadata?.sourceKind === "project_scan")).toEqual([]);
|
||||
});
|
||||
|
||||
it("files new project imports without moving them back on re-import", async () => {
|
||||
const companyId = randomUUID();
|
||||
const projectId = randomUUID();
|
||||
|
|
@ -2579,4 +2579,303 @@ describeEmbeddedPostgres("companySkillService.list", () => {
|
|||
});
|
||||
});
|
||||
|
||||
async function seedCompany(companyId: string) {
|
||||
await db.insert(companies).values({
|
||||
id: companyId,
|
||||
name: "Paperclip",
|
||||
issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`,
|
||||
requireBoardApprovalForNewAgents: false,
|
||||
});
|
||||
}
|
||||
|
||||
function runtimeSkillName(key: string, slug: string) {
|
||||
if (key.startsWith("paperclipai/paperclip/")) return slug;
|
||||
return `${slug}--${createHash("sha256").update(key).digest("hex").slice(0, 10)}`;
|
||||
}
|
||||
|
||||
it("renames a Paperclip-managed skill, moving the directory and rewriting SKILL.md frontmatter", async () => {
|
||||
const companyId = randomUUID();
|
||||
await seedCompany(companyId);
|
||||
const skill = await svc.createLocalSkill(
|
||||
companyId,
|
||||
{ name: "Prepare PR", slug: "prepare-pr", description: "Prep pull requests" },
|
||||
{ type: "user", userId: "board" },
|
||||
);
|
||||
const oldDir = skill.sourceLocator!;
|
||||
const managedRoot = path.dirname(oldDir);
|
||||
|
||||
const result = await svc.renameSkill(companyId, skill.id, { name: "Ship PR", slug: "ship-pr" });
|
||||
|
||||
expect(result).toMatchObject({
|
||||
previousName: "Prepare PR",
|
||||
previousSlug: "prepare-pr",
|
||||
previousKey: `company/${companyId}/prepare-pr`,
|
||||
reassignments: [],
|
||||
skill: {
|
||||
id: skill.id,
|
||||
name: "Ship PR",
|
||||
slug: "ship-pr",
|
||||
key: `company/${companyId}/ship-pr`,
|
||||
},
|
||||
});
|
||||
|
||||
const newDir = path.join(managedRoot, "ship-pr");
|
||||
expect(result.skill.sourceLocator).toBe(newDir);
|
||||
await expect(fs.stat(oldDir)).rejects.toMatchObject({ code: "ENOENT" });
|
||||
const renamedMarkdown = await fs.readFile(path.join(newDir, "SKILL.md"), "utf8");
|
||||
expect(renamedMarkdown).toContain("name: Ship PR");
|
||||
expect(renamedMarkdown).not.toContain("name: Prepare PR");
|
||||
|
||||
// Description and version history are preserved.
|
||||
const persisted = await svc.getById(companyId, skill.id);
|
||||
expect(persisted).toMatchObject({ description: "Prep pull requests" });
|
||||
expect(persisted?.markdown).toBe(renamedMarkdown);
|
||||
const versions = await svc.listVersions(companyId, skill.id);
|
||||
expect(versions).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("quotes YAML-special names while keeping stored and on-disk markdown synchronized", async () => {
|
||||
const companyId = randomUUID();
|
||||
await seedCompany(companyId);
|
||||
const skill = await svc.createLocalSkill(companyId, { name: "Prepare PR", slug: "prepare-pr" });
|
||||
|
||||
const result = await svc.renameSkill(companyId, skill.id, { name: "Ship: PR #1", slug: "ship-pr" });
|
||||
const markdown = await fs.readFile(path.join(result.skill.sourceLocator!, "SKILL.md"), "utf8");
|
||||
const persisted = await svc.getById(companyId, skill.id);
|
||||
|
||||
expect(markdown).toContain('name: "Ship: PR #1"');
|
||||
expect(parseFrontmatterMarkdown(markdown).frontmatter.name).toBe("Ship: PR #1");
|
||||
expect(persisted?.markdown).toBe(markdown);
|
||||
});
|
||||
|
||||
it("supports a name-only rename that keeps the slug/key and directory", async () => {
|
||||
const companyId = randomUUID();
|
||||
await seedCompany(companyId);
|
||||
const skill = await svc.createLocalSkill(
|
||||
companyId,
|
||||
{ name: "Prepare PR", slug: "prepare-pr" },
|
||||
{ type: "user", userId: "board" },
|
||||
);
|
||||
const dir = skill.sourceLocator!;
|
||||
|
||||
// Keeping the slug requires passing it explicitly: an omitted slug is
|
||||
// re-derived from the new name.
|
||||
const result = await svc.renameSkill(companyId, skill.id, { name: "Prepare Pull Request", slug: "prepare-pr" });
|
||||
expect(result.skill).toMatchObject({
|
||||
name: "Prepare Pull Request",
|
||||
slug: "prepare-pr",
|
||||
key: `company/${companyId}/prepare-pr`,
|
||||
sourceLocator: dir,
|
||||
});
|
||||
const markdown = await fs.readFile(path.join(dir, "SKILL.md"), "utf8");
|
||||
expect(markdown).toContain("name: Prepare Pull Request");
|
||||
});
|
||||
|
||||
it("returns the unchanged skill for a normalized no-op rename", async () => {
|
||||
const companyId = randomUUID();
|
||||
await seedCompany(companyId);
|
||||
const skill = await svc.createLocalSkill(
|
||||
companyId,
|
||||
{ name: "Prepare PR", slug: "prepare-pr" },
|
||||
{ type: "user", userId: "board" },
|
||||
);
|
||||
|
||||
const result = await svc.renameSkill(companyId, skill.id, { name: "Prepare PR", slug: "Prepare-PR" });
|
||||
expect(result).toMatchObject({
|
||||
reassignments: [],
|
||||
skill: { id: skill.id, slug: "prepare-pr", key: `company/${companyId}/prepare-pr` },
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects a rename whose slug conflicts with another skill", async () => {
|
||||
const companyId = randomUUID();
|
||||
await seedCompany(companyId);
|
||||
const source = await svc.createLocalSkill(companyId, { name: "Source", slug: "source" });
|
||||
await svc.createLocalSkill(companyId, { name: "Taken", slug: "taken" });
|
||||
|
||||
await expect(svc.renameSkill(companyId, source.id, { name: "Taken", slug: "taken" })).rejects.toMatchObject({
|
||||
status: 409,
|
||||
details: { conflict: "slug", slug: "taken" },
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects a rename whose derived key conflicts with another skill", async () => {
|
||||
const companyId = randomUUID();
|
||||
await seedCompany(companyId);
|
||||
const source = await svc.createLocalSkill(companyId, { name: "Source", slug: "source" });
|
||||
// A sibling whose key already matches the derived target key but whose slug
|
||||
// differs, so only the key-conflict branch fires. It needs a real on-disk
|
||||
// source so inventory reconciliation does not prune it before the check.
|
||||
const squatterDir = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-key-squatter-"));
|
||||
cleanupDirs.add(squatterDir);
|
||||
await fs.writeFile(path.join(squatterDir, "SKILL.md"), "---\nname: Key Squatter\n---\n# Key Squatter\n", "utf8");
|
||||
await db.insert(companySkills).values({
|
||||
id: randomUUID(),
|
||||
companyId,
|
||||
key: `company/${companyId}/renamed`,
|
||||
slug: "different-slug",
|
||||
name: "Key Squatter",
|
||||
description: null,
|
||||
markdown: "# Key Squatter\n",
|
||||
sourceType: "local_path",
|
||||
sourceLocator: squatterDir,
|
||||
trustLevel: "markdown_only",
|
||||
compatibility: "compatible",
|
||||
fileInventory: [{ path: "SKILL.md", kind: "skill" }],
|
||||
metadata: { sourceKind: "managed_local" },
|
||||
});
|
||||
|
||||
await expect(svc.renameSkill(companyId, source.id, { name: "Renamed", slug: "renamed" })).rejects.toMatchObject({
|
||||
status: 409,
|
||||
details: { conflict: "key", key: `company/${companyId}/renamed` },
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects renaming non Paperclip-managed skill sources with 422", async () => {
|
||||
const companyId = randomUUID();
|
||||
await seedCompany(companyId);
|
||||
|
||||
const unmanagedDir = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-unmanaged-skill-"));
|
||||
cleanupDirs.add(unmanagedDir);
|
||||
await fs.writeFile(path.join(unmanagedDir, "SKILL.md"), "---\nname: Unmanaged\n---\n# Unmanaged\n", "utf8");
|
||||
|
||||
const rows = [
|
||||
{
|
||||
slug: "github-skill",
|
||||
sourceType: "github" as const,
|
||||
sourceLocator: "https://github.com/acme/github-skill",
|
||||
metadata: { sourceKind: "github", owner: "acme", repo: "github-skill" },
|
||||
},
|
||||
{
|
||||
slug: "url-skill",
|
||||
sourceType: "url" as const,
|
||||
sourceLocator: "https://example.com/url.md",
|
||||
metadata: { sourceKind: "url" },
|
||||
},
|
||||
{
|
||||
slug: "skills-sh-skill",
|
||||
sourceType: "skills_sh" as const,
|
||||
sourceLocator: "https://github.com/acme/skills-sh-skill",
|
||||
metadata: { sourceKind: "skills_sh", owner: "acme", repo: "skills-sh-skill" },
|
||||
},
|
||||
{
|
||||
slug: "catalog-skill",
|
||||
sourceType: "catalog" as const,
|
||||
sourceLocator: null,
|
||||
metadata: { sourceKind: "catalog" },
|
||||
},
|
||||
{
|
||||
slug: "project-scan-skill",
|
||||
sourceType: "local_path" as const,
|
||||
sourceLocator: unmanagedDir,
|
||||
metadata: { sourceKind: "project_scan" },
|
||||
},
|
||||
{
|
||||
slug: "unmanaged-local-skill",
|
||||
sourceType: "local_path" as const,
|
||||
sourceLocator: unmanagedDir,
|
||||
metadata: { sourceKind: "local_path" },
|
||||
},
|
||||
];
|
||||
const inserted = rows.map((row) => ({ id: randomUUID(), ...row }));
|
||||
await db.insert(companySkills).values(inserted.map((row) => ({
|
||||
id: row.id,
|
||||
companyId,
|
||||
key: `company/${companyId}/${row.slug}`,
|
||||
slug: row.slug,
|
||||
name: row.slug,
|
||||
description: null,
|
||||
markdown: `# ${row.slug}\n`,
|
||||
sourceType: row.sourceType,
|
||||
sourceLocator: row.sourceLocator,
|
||||
trustLevel: "markdown_only" as const,
|
||||
compatibility: "compatible" as const,
|
||||
fileInventory: [{ path: "SKILL.md", kind: "skill" as const }],
|
||||
metadata: row.metadata,
|
||||
})));
|
||||
|
||||
for (const row of inserted) {
|
||||
await expect(svc.renameSkill(companyId, row.id, { name: "Renamed" })).rejects.toMatchObject({
|
||||
status: 422,
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
it("rewrites agent desired-skill keys on rename while preserving version pins", async () => {
|
||||
const companyId = randomUUID();
|
||||
await seedCompany(companyId);
|
||||
const skill = await svc.createLocalSkill(companyId, { name: "Shared", slug: "shared" });
|
||||
const pinnedVersionId = randomUUID();
|
||||
|
||||
const pinnedAgentId = randomUUID();
|
||||
const looseAgentId = randomUUID();
|
||||
const otherAgentId = randomUUID();
|
||||
await db.insert(agents).values([
|
||||
{
|
||||
id: pinnedAgentId,
|
||||
companyId,
|
||||
name: "Pinned",
|
||||
role: "engineer",
|
||||
adapterType: "codex_local",
|
||||
adapterConfig: {
|
||||
paperclipSkillSync: {
|
||||
desiredSkills: [{ key: `company/${companyId}/shared`, versionId: pinnedVersionId }],
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
id: looseAgentId,
|
||||
companyId,
|
||||
name: "Loose",
|
||||
role: "engineer",
|
||||
adapterType: "codex_local",
|
||||
adapterConfig: {
|
||||
paperclipSkillSync: { desiredSkills: [`company/${companyId}/shared`] },
|
||||
},
|
||||
},
|
||||
{
|
||||
id: otherAgentId,
|
||||
companyId,
|
||||
name: "Other",
|
||||
role: "engineer",
|
||||
adapterType: "codex_local",
|
||||
adapterConfig: {
|
||||
paperclipSkillSync: { desiredSkills: [`company/${companyId}/unrelated`] },
|
||||
},
|
||||
},
|
||||
]);
|
||||
|
||||
const result = await svc.renameSkill(companyId, skill.id, { name: "Shared Renamed", slug: "shared-renamed" });
|
||||
expect(result.reassignments).toEqual(
|
||||
expect.arrayContaining([
|
||||
{ agentId: pinnedAgentId, previousSkillKey: `company/${companyId}/shared`, nextSkillKey: `company/${companyId}/shared-renamed` },
|
||||
{ agentId: looseAgentId, previousSkillKey: `company/${companyId}/shared`, nextSkillKey: `company/${companyId}/shared-renamed` },
|
||||
]),
|
||||
);
|
||||
expect(result.reassignments).toHaveLength(2);
|
||||
|
||||
const after = await db.select().from(agents).where(eq(agents.companyId, companyId));
|
||||
const pinned = after.find((agent) => agent.id === pinnedAgentId)!.adapterConfig as Record<string, any>;
|
||||
const loose = after.find((agent) => agent.id === looseAgentId)!.adapterConfig as Record<string, any>;
|
||||
const other = after.find((agent) => agent.id === otherAgentId)!.adapterConfig as Record<string, any>;
|
||||
expect(pinned.paperclipSkillSync.desiredSkills).toEqual([
|
||||
{ key: `company/${companyId}/shared-renamed`, versionId: pinnedVersionId },
|
||||
]);
|
||||
expect(loose.paperclipSkillSync.desiredSkills).toEqual([`company/${companyId}/shared-renamed`]);
|
||||
expect(other.paperclipSkillSync.desiredSkills).toEqual([`company/${companyId}/unrelated`]);
|
||||
});
|
||||
|
||||
it("removes the old runtime materialization when the key/slug changes", async () => {
|
||||
const companyId = randomUUID();
|
||||
await seedCompany(companyId);
|
||||
const skill = await svc.createLocalSkill(companyId, { name: "Runtime Skill", slug: "runtime-skill" });
|
||||
const managedRoot = path.dirname(skill.sourceLocator!);
|
||||
const oldRuntimeDir = path.join(managedRoot, "__runtime__", runtimeSkillName(skill.key, skill.slug));
|
||||
await fs.mkdir(oldRuntimeDir, { recursive: true });
|
||||
await fs.writeFile(path.join(oldRuntimeDir, "SKILL.md"), "# stale\n", "utf8");
|
||||
|
||||
await svc.renameSkill(companyId, skill.id, { name: "Runtime Skill", slug: "runtime-renamed" });
|
||||
|
||||
await expect(fs.stat(oldRuntimeDir)).rejects.toMatchObject({ code: "ENOENT" });
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ import {
|
|||
companySkillInstallUpdateSchema,
|
||||
companySkillListQuerySchema,
|
||||
companySkillProjectScanRequestSchema,
|
||||
companySkillRenameSchema,
|
||||
companySkillResetSchema,
|
||||
companySkillTestInputCreateSchema,
|
||||
companySkillTestInputUpdateSchema,
|
||||
|
|
@ -855,6 +856,49 @@ export function companySkillRoutes(db: Db) {
|
|||
},
|
||||
);
|
||||
|
||||
router.post(
|
||||
"/companies/:companyId/skills/:skillId/rename",
|
||||
validate(companySkillRenameSchema),
|
||||
async (req, res) => {
|
||||
const companyId = req.params.companyId as string;
|
||||
const skillId = req.params.skillId as string;
|
||||
await assertCanMutateCompanySkills(
|
||||
req,
|
||||
companyId,
|
||||
"skills.edit",
|
||||
() => skillPolicyResource({ companyId, skillId }),
|
||||
);
|
||||
const result = await svc.renameSkill(companyId, skillId, req.body);
|
||||
const changed = result.previousName !== result.skill.name
|
||||
|| result.previousSlug !== result.skill.slug
|
||||
|| result.previousKey !== result.skill.key;
|
||||
if (changed) {
|
||||
const actor = getActorInfo(req);
|
||||
await logActivity(db, {
|
||||
companyId,
|
||||
actorType: actor.actorType,
|
||||
actorId: actor.actorId,
|
||||
agentId: actor.agentId,
|
||||
runId: actor.runId,
|
||||
agentApiKeyId: actor.agentApiKeyId,
|
||||
action: "company.skill_renamed",
|
||||
entityType: "company_skill",
|
||||
entityId: result.skill.id,
|
||||
details: {
|
||||
previousName: result.previousName,
|
||||
previousSlug: result.previousSlug,
|
||||
previousKey: result.previousKey,
|
||||
name: result.skill.name,
|
||||
slug: result.skill.slug,
|
||||
key: result.skill.key,
|
||||
reassignedAgentIds: result.reassignments.map((entry: { agentId: string }) => entry.agentId),
|
||||
},
|
||||
});
|
||||
}
|
||||
res.json(result);
|
||||
},
|
||||
);
|
||||
|
||||
router.get("/companies/:companyId/skills/:skillId/comments", async (req, res) => {
|
||||
const companyId = req.params.companyId as string;
|
||||
const skillId = req.params.skillId as string;
|
||||
|
|
|
|||
|
|
@ -110,6 +110,8 @@ import {
|
|||
companySkillImportSchema,
|
||||
companySkillProjectScanRequestSchema,
|
||||
companySkillProjectScanResultSchema,
|
||||
companySkillRenameResultSchema,
|
||||
companySkillRenameSchema,
|
||||
companySkillTestInputCreateSchema,
|
||||
companySkillTestInputUpdateSchema,
|
||||
companySkillTestRunCreateSchema,
|
||||
|
|
@ -4050,6 +4052,26 @@ registry.registerPath({
|
|||
responses: { 200: r.ok(), 401: r.unauthorized },
|
||||
});
|
||||
|
||||
registry.registerPath({
|
||||
method: "post",
|
||||
path: "/api/companies/{companyId}/skills/{skillId}/rename",
|
||||
tags: ["skills"],
|
||||
summary: "Rename a managed company skill",
|
||||
request: {
|
||||
params: z.object({ companyId: z.string(), skillId: z.string() }),
|
||||
body: jsonBody(companySkillRenameSchema),
|
||||
},
|
||||
responses: {
|
||||
200: r.ok(companySkillRenameResultSchema),
|
||||
400: r.badRequest,
|
||||
401: r.unauthorized,
|
||||
403: r.forbidden,
|
||||
404: r.notFound,
|
||||
409: r.conflict,
|
||||
422: r.unprocessable,
|
||||
},
|
||||
});
|
||||
|
||||
registry.registerPath({
|
||||
method: "post",
|
||||
path: "/api/companies/{companyId}/skills",
|
||||
|
|
|
|||
|
|
@ -61,6 +61,8 @@ import type {
|
|||
CompanySkillProjectScanRequest,
|
||||
CompanySkillProjectScanResult,
|
||||
CompanySkillProjectScanSkipped,
|
||||
CompanySkillRenameRequest,
|
||||
CompanySkillRenameResult,
|
||||
CompanySkillSharingScope,
|
||||
CompanySkillSourceBadge,
|
||||
CompanySkillSourceType,
|
||||
|
|
@ -88,7 +90,14 @@ import type {
|
|||
IssueAttachment,
|
||||
IssueDocument,
|
||||
} from "@paperclipai/shared";
|
||||
import { isUuidLike, normalizeAgentUrlKey, parseFrontmatterMarkdown } from "@paperclipai/shared";
|
||||
import {
|
||||
isUuidLike,
|
||||
joinFrontmatterBlock,
|
||||
normalizeAgentUrlKey,
|
||||
parseFrontmatterMarkdown,
|
||||
splitFrontmatterBlock,
|
||||
stringifyFrontmatter,
|
||||
} from "@paperclipai/shared";
|
||||
import { resolvePaperclipInstanceRoot } from "../home-paths.js";
|
||||
import { conflict, forbidden, notFound, unprocessable } from "../errors.js";
|
||||
import { ghFetch, gitHubApiBase, resolveRawGitHubUrl } from "./github-fetch.js";
|
||||
|
|
@ -571,6 +580,26 @@ function buildSkillRuntimeName(key: string, slug: string) {
|
|||
return `${slug}--${hashSkillValue(key)}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Rewrite only the top-level `name:` frontmatter field of a SKILL.md document,
|
||||
* leaving the body and every other field byte-identical. Returns the input
|
||||
* unchanged when there is no frontmatter or no `name:` field to update.
|
||||
*/
|
||||
function rewriteFrontmatterName(markdown: string, newName: string): string {
|
||||
const block = splitFrontmatterBlock(markdown);
|
||||
if (!block.hasFrontmatter) return markdown;
|
||||
let replaced = false;
|
||||
const nextLines = block.frontmatterText.split("\n").map((line) => {
|
||||
if (!replaced && /^name\s*:/.test(line)) {
|
||||
replaced = true;
|
||||
return stringifyFrontmatter({ name: newName });
|
||||
}
|
||||
return line;
|
||||
});
|
||||
if (!replaced) return markdown;
|
||||
return joinFrontmatterBlock({ ...block, frontmatterText: nextLines.join("\n") });
|
||||
}
|
||||
|
||||
function readCanonicalSkillKey(frontmatter: Record<string, unknown>, metadata: Record<string, unknown> | null) {
|
||||
const direct = normalizeSkillKey(
|
||||
asString(frontmatter.key)
|
||||
|
|
@ -2297,6 +2326,27 @@ function resolveManagedSkillsRoot(companyId: string) {
|
|||
return path.resolve(resolvePaperclipInstanceRoot(), "skills", companyId);
|
||||
}
|
||||
|
||||
/**
|
||||
* A rename target must be a true Paperclip-managed local skill: a `local_path`
|
||||
* skill whose `managed_local` source directory lives directly under the
|
||||
* company managed-skills root (e.g. `<managedRoot>/<slug>`). This deliberately
|
||||
* excludes catalog (`__catalog__/...`), runtime (`__runtime__/...`) and other
|
||||
* reserved subtrees, project-scanned skills, unmanaged `local_path` skills, and
|
||||
* all remote source types, which keep their own identity/update semantics.
|
||||
*/
|
||||
function isPaperclipManagedRenameTarget(skill: CompanySkill): boolean {
|
||||
if (skill.sourceType !== "local_path") return false;
|
||||
if (getSkillMeta(skill).sourceKind !== "managed_local") return false;
|
||||
const skillDir = normalizeSkillDirectory(skill);
|
||||
if (!skillDir) return false;
|
||||
const managedRoot = resolveManagedSkillsRoot(skill.companyId);
|
||||
const relative = path.relative(managedRoot, skillDir);
|
||||
if (!relative || relative.startsWith("..") || path.isAbsolute(relative)) return false;
|
||||
const segments = relative.split(path.sep);
|
||||
// Managed skills are a direct child of the root; reject reserved `__*` dirs.
|
||||
return segments.length === 1 && !segments[0]!.startsWith("__");
|
||||
}
|
||||
|
||||
function resolveLocalSkillFilePath(skill: CompanySkill, relativePath: string) {
|
||||
const normalized = normalizePortablePath(relativePath);
|
||||
const skillDir = normalizeSkillDirectory(skill);
|
||||
|
|
@ -3901,6 +3951,159 @@ export function companySkillService(db: Db) {
|
|||
};
|
||||
}
|
||||
|
||||
async function renameSkill(
|
||||
companyId: string,
|
||||
skillId: string,
|
||||
input: CompanySkillRenameRequest,
|
||||
): Promise<CompanySkillRenameResult> {
|
||||
await ensureSkillInventoryCurrent(companyId);
|
||||
const skill = await getById(companyId, skillId);
|
||||
if (!skill) throw notFound("Skill not found");
|
||||
|
||||
if (!isPaperclipManagedRenameTarget(skill)) {
|
||||
throw unprocessable(
|
||||
"Only Paperclip-managed skills can be renamed. Catalog, external, project-scanned, and unmanaged local skills are read-only.",
|
||||
{ skillId: skill.id, sourceType: skill.sourceType, sourceKind: getSkillMeta(skill).sourceKind ?? null },
|
||||
);
|
||||
}
|
||||
|
||||
const newName = input.name.trim();
|
||||
if (!newName) throw unprocessable("Skill name is required.");
|
||||
const newSlug = normalizeSkillSlug(input.slug ?? null) ?? normalizeSkillSlug(newName);
|
||||
if (!newSlug) {
|
||||
throw unprocessable("Skill name must contain at least one letter or number to derive a slug.");
|
||||
}
|
||||
const newKey = `company/${companyId}/${newSlug}`;
|
||||
|
||||
const previousName = skill.name;
|
||||
const previousSlug = skill.slug;
|
||||
const previousKey = skill.key;
|
||||
|
||||
// Normalized no-op: nothing changes, so skip all filesystem/DB work.
|
||||
if (newName === previousName && newSlug === previousSlug && newKey === previousKey) {
|
||||
return { skill, previousName, previousSlug, previousKey, reassignments: [] };
|
||||
}
|
||||
|
||||
// Authoritative conflict detection against slug and key within the company.
|
||||
if (newSlug !== previousSlug || newKey !== previousKey) {
|
||||
const existing = await listFull(companyId);
|
||||
for (const other of existing) {
|
||||
if (other.id === skill.id) continue;
|
||||
if ((normalizeSkillSlug(other.slug) ?? other.slug) === newSlug) {
|
||||
throw conflict(`A company skill with slug "${newSlug}" already exists.`, {
|
||||
conflict: "slug",
|
||||
slug: newSlug,
|
||||
});
|
||||
}
|
||||
if (other.key === newKey) {
|
||||
throw conflict(`A company skill with key "${newKey}" already exists.`, {
|
||||
conflict: "key",
|
||||
key: newKey,
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const managedRoot = resolveManagedSkillsRoot(companyId);
|
||||
const oldDir = normalizeSkillDirectory(skill)!;
|
||||
const newDir = path.resolve(managedRoot, newSlug);
|
||||
const directoryMoved = newDir !== oldDir;
|
||||
|
||||
if (directoryMoved && (await statPath(newDir))) {
|
||||
throw conflict(`A managed skill directory already exists at ${newDir}.`, { conflict: "directory" });
|
||||
}
|
||||
|
||||
// Plan agent reassignments from the old key to the new key, preserving each
|
||||
// pinned versionId. Resolve against the pre-rename reference targets so the
|
||||
// old key still maps cleanly.
|
||||
const referenceSkills = await listReferenceTargets(companyId);
|
||||
const agentRows = await agents.list(companyId, { includeTerminated: true });
|
||||
const plannedReassignments: CompanySkillForkReassignment[] = agentRows
|
||||
.filter((agent) =>
|
||||
resolveDesiredSkillEntries(referenceSkills, agent.adapterConfig as Record<string, unknown>)
|
||||
.some((entry) => entry.key === previousKey))
|
||||
.map((agent) => ({ agentId: agent.id, previousSkillKey: previousKey, nextSkillKey: newKey }));
|
||||
|
||||
// Reversible filesystem stage: capture the original SKILL.md so a failed DB
|
||||
// transaction can be rolled back to the pre-rename on-disk state.
|
||||
const originalMarkdown = await fs.readFile(path.join(oldDir, "SKILL.md"), "utf8").catch(() => null);
|
||||
const rewrittenMarkdown = rewriteFrontmatterName(originalMarkdown ?? skill.markdown, newName);
|
||||
let movedDir = false;
|
||||
try {
|
||||
if (directoryMoved) {
|
||||
await fs.mkdir(path.dirname(newDir), { recursive: true });
|
||||
await fs.rename(oldDir, newDir);
|
||||
movedDir = true;
|
||||
}
|
||||
if (originalMarkdown !== null) {
|
||||
if (rewrittenMarkdown !== originalMarkdown) {
|
||||
await fs.writeFile(path.join(newDir, "SKILL.md"), rewrittenMarkdown, "utf8");
|
||||
}
|
||||
}
|
||||
|
||||
await db.transaction(async (tx) => {
|
||||
const updated = await tx
|
||||
.update(companySkills)
|
||||
.set({
|
||||
name: newName,
|
||||
slug: newSlug,
|
||||
key: newKey,
|
||||
sourceLocator: newDir,
|
||||
markdown: rewrittenMarkdown,
|
||||
updatedAt: new Date(),
|
||||
})
|
||||
.where(and(eq(companySkills.id, skill.id), eq(companySkills.companyId, companyId)))
|
||||
.returning({ id: companySkills.id })
|
||||
.then((rows) => rows[0] ?? null);
|
||||
if (!updated) throw notFound("Skill not found");
|
||||
|
||||
for (const item of plannedReassignments) {
|
||||
const row = await tx
|
||||
.select({ id: agentsTable.id, adapterConfig: agentsTable.adapterConfig })
|
||||
.from(agentsTable)
|
||||
.where(and(eq(agentsTable.companyId, companyId), eq(agentsTable.id, item.agentId)))
|
||||
.for("update")
|
||||
.then((rows) => rows[0] ?? null);
|
||||
if (!row) continue;
|
||||
const adapterConfig = row.adapterConfig as Record<string, unknown>;
|
||||
const nextEntries = resolveDesiredSkillEntries(referenceSkills, adapterConfig).map((entry) =>
|
||||
entry.key === previousKey
|
||||
? { key: newKey, versionId: entry.versionId ?? null }
|
||||
: entry);
|
||||
await tx
|
||||
.update(agentsTable)
|
||||
.set({
|
||||
adapterConfig: writePaperclipSkillSyncPreference(adapterConfig, nextEntries),
|
||||
updatedAt: new Date(),
|
||||
})
|
||||
.where(and(eq(agentsTable.companyId, companyId), eq(agentsTable.id, item.agentId)));
|
||||
}
|
||||
});
|
||||
} catch (error) {
|
||||
// Roll back the filesystem to its pre-rename state.
|
||||
if (originalMarkdown !== null) {
|
||||
await fs
|
||||
.writeFile(path.join(movedDir ? newDir : oldDir, "SKILL.md"), originalMarkdown, "utf8")
|
||||
.catch(() => {});
|
||||
}
|
||||
if (movedDir) {
|
||||
await fs.rename(newDir, oldDir).catch(() => {});
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
|
||||
// Remove the stale runtime materialization so runtime sync recreates it
|
||||
// under the new key/slug.
|
||||
await fs.rm(
|
||||
path.resolve(managedRoot, "__runtime__", buildSkillRuntimeName(previousKey, previousSlug)),
|
||||
{ recursive: true, force: true },
|
||||
);
|
||||
|
||||
const renamed = await getById(companyId, skill.id);
|
||||
if (!renamed) throw notFound("Renamed skill not found");
|
||||
return { skill: renamed, previousName, previousSlug, previousKey, reassignments: plannedReassignments };
|
||||
}
|
||||
|
||||
async function updateStatus(companyId: string, skillId: string): Promise<CompanySkillUpdateStatus | null> {
|
||||
await ensureSkillInventoryCurrent(companyId);
|
||||
const skill = await getById(companyId, skillId);
|
||||
|
|
@ -6606,6 +6809,7 @@ export function companySkillService(db: Db) {
|
|||
updateComment,
|
||||
deleteComment,
|
||||
forkSkill,
|
||||
renameSkill,
|
||||
updateStatus,
|
||||
readFile,
|
||||
updateSkill,
|
||||
|
|
|
|||
Loading…
Reference in New Issue