From fcf66f3a91e6d1e5b81200f5ebf945b0d6ecad9d Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Thu, 30 Jul 2026 15:29:31 -0700 Subject: [PATCH] feat(skills): add managed skill rename API (#9688) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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 --- packages/shared/src/index.ts | 4 + packages/shared/src/types/company-skill.ts | 13 + packages/shared/src/types/index.ts | 2 + .../shared/src/validators/company-skill.ts | 14 + packages/shared/src/validators/index.ts | 3 + .../__tests__/company-skills-routes.test.ts | 110 +++++++ .../__tests__/company-skills-service.test.ts | 301 +++++++++++++++++- server/src/routes/company-skills.ts | 44 +++ server/src/routes/openapi.ts | 22 ++ server/src/services/company-skills.ts | 206 +++++++++++- 10 files changed, 717 insertions(+), 2 deletions(-) diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 1027fa4ab5..6ebfd0c14d 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -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, diff --git a/packages/shared/src/types/company-skill.ts b/packages/shared/src/types/company-skill.ts index 9498329ddd..4d72d2e674 100644 --- a/packages/shared/src/types/company-skill.ts +++ b/packages/shared/src/types/company-skill.ts @@ -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; diff --git a/packages/shared/src/types/index.ts b/packages/shared/src/types/index.ts index 95ef134711..34a8bdf50e 100644 --- a/packages/shared/src/types/index.ts +++ b/packages/shared/src/types/index.ts @@ -141,6 +141,8 @@ export type { CompanySkillForkReassignment, CompanySkillForkResult, CompanySkillForkPrecheckResult, + CompanySkillRenameRequest, + CompanySkillRenameResult, CompanySkillUpdateRequest, CompanySkillUpdateStatus, CompanySkillAuditSeverity, diff --git a/packages/shared/src/validators/company-skill.ts b/packages/shared/src/validators/company-skill.ts index ac3a61fd1a..bc9c7bd292 100644 --- a/packages/shared/src/validators/company-skill.ts +++ b/packages/shared/src/validators/company-skill.ts @@ -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; export type CompanySkillCommentUpdate = z.infer; export type CompanySkillFork = z.infer; +export type CompanySkillRename = z.infer; export type CompanySkillUpdate = z.infer; export type CatalogSkillListQuery = z.infer; export type CompanySkillInstallCatalog = z.infer; diff --git a/packages/shared/src/validators/index.ts b/packages/shared/src/validators/index.ts index bd577033e2..0dca441108 100644 --- a/packages/shared/src/validators/index.ts +++ b/packages/shared/src/validators/index.ts @@ -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, diff --git a/server/src/__tests__/company-skills-routes.test.ts b/server/src/__tests__/company-skills-routes.test.ts index 4246b8c149..66d243b2e1 100644 --- a/server/src/__tests__/company-skills-routes.test.ts +++ b/server/src/__tests__/company-skills-routes.test.ts @@ -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" }); diff --git a/server/src/__tests__/company-skills-service.test.ts b/server/src/__tests__/company-skills-service.test.ts index 0725237c84..4697b67f0c 100644 --- a/server/src/__tests__/company-skills-service.test.ts +++ b/server/src/__tests__/company-skills-service.test.ts @@ -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; + const loose = after.find((agent) => agent.id === looseAgentId)!.adapterConfig as Record; + const other = after.find((agent) => agent.id === otherAgentId)!.adapterConfig as Record; + 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" }); + }); }); diff --git a/server/src/routes/company-skills.ts b/server/src/routes/company-skills.ts index bcc3151ffb..80966a15a8 100644 --- a/server/src/routes/company-skills.ts +++ b/server/src/routes/company-skills.ts @@ -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; diff --git a/server/src/routes/openapi.ts b/server/src/routes/openapi.ts index e619f5f3d2..87255b8bd7 100644 --- a/server/src/routes/openapi.ts +++ b/server/src/routes/openapi.ts @@ -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", diff --git a/server/src/services/company-skills.ts b/server/src/services/company-skills.ts index 5a1f666a09..f5b03985c1 100644 --- a/server/src/services/company-skills.ts +++ b/server/src/services/company-skills.ts @@ -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, metadata: Record | 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. `/`). 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 { + 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) + .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; + 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 { 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,