From b4a7efa8d2c1e2f57c042d50fda0157e025feac6 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Thu, 25 Jun 2026 10:03:41 -0500 Subject: [PATCH] Add skill category editing in settings (#8615) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Skills can be installed, inspected, filtered, and grouped inside company settings > - Skill category metadata already exists in the data model and list filters, but users could not edit categories after a skill was created or imported > - That made category filters and counts drift from the way operators actually want to organize their skills > - This pull request adds category editing to the existing skill settings dialog and sends those edits through the existing company-scoped skill update API > - The server mutation now includes category information in the activity log so settings changes are auditable > - The benefit is that operators can keep installed skills organized without reinstalling or recreating them ## Linked Issues or Issue Description No duplicate or closely related public GitHub issues or PRs were found for `skill categories settings`. Feature request fields: **Subsystem affected** Cross-cutting: `server/` REST API routes/services and `ui/` React board settings. **Problem or motivation** Company operators can create or import skills with categories, and Paperclip already exposes category filters and category counts. After installation, though, operators could not edit a skill's categories from the skill detail settings screen. That made it difficult to keep skills grouped correctly as workflows evolved. **Proposed solution** Add category editing to the existing skill settings dialog. The category field accepts comma-separated values, normalizes them into slugs, deduplicates repeated categories, allows clearing all categories, and saves categories together with the existing sharing setting through the company-scoped skill update API. **Alternatives considered** One alternative was to keep categories editable only during create/import flows, but that forces users to recreate or reinstall skills just to adjust grouping metadata. Another was a separate categories-only action, but batching settings into one explicit Save action keeps the dialog predictable. **Roadmap alignment** This supports the completed Skills Manager roadmap area by making installed skills easier to organize and maintain inside company settings. **Additional context** The server already persisted skill categories and supported category list filters/counts. This PR wires the existing metadata into the settings editing path and adds focused route, service, and UI tests. ## What Changed - Added category editing to the skill detail settings dialog, including comma-separated input, normalized deduplication, reset, dirty-state handling, and save feedback. - Updated the skill settings mutation path to save categories and sharing scope together, then refresh detail/list cache entries. - Included updated categories in `company.skill_updated` activity details. - Added server route/service coverage for category updates, normalization, filtering, counts, clearing, and activity logging. - Added UI coverage for saving category edits, clearing categories, reordered no-op category sets, saving sharing changes together, and preserving draft input after a failed save. - Updated the Storybook skill detail harness for the renamed settings callback props. ## Verification - `pnpm run preflight:workspace-links && pnpm exec vitest run server/src/__tests__/company-skills-routes.test.ts server/src/__tests__/company-skills-service.test.ts ui/src/pages/CompanySkills.test.tsx` - Latest-head GitHub checks are green for typecheck, build, e2e, general tests, serialized server suites, policy, commitperclip review, Socket Security, Snyk status, and Canary Dry Run. - Greptile Review succeeded on the latest head with zero unresolved review threads. ## Risks Low risk. The change uses the existing company skill update API and category normalization path. The main behavioral change is that the settings dialog now batches sharing and category edits behind an explicit Save button instead of saving sharing immediately on select change. > 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 based on GPT-5, with tool-enabled repository inspection, shell execution, git, and GitHub CLI access. ## 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 --- .../__tests__/company-skills-routes.test.ts | 33 ++++ .../__tests__/company-skills-service.test.ts | 40 ++++ server/src/routes/company-skills.ts | 1 + ui/src/pages/CompanySkills.test.tsx | 172 +++++++++++++++++- ui/src/pages/CompanySkills.tsx | 110 +++++++++-- .../stories/skills-store-detail.stories.tsx | 4 +- 6 files changed, 332 insertions(+), 28 deletions(-) diff --git a/server/src/__tests__/company-skills-routes.test.ts b/server/src/__tests__/company-skills-routes.test.ts index bac7a475fc..ca8abc4d21 100644 --- a/server/src/__tests__/company-skills-routes.test.ts +++ b/server/src/__tests__/company-skills-routes.test.ts @@ -299,6 +299,7 @@ describe("company skill mutation permissions", () => { mockCompanySkillService.updateSkill.mockResolvedValue({ id: "skill-1", slug: "review", + categories: ["memory", "review"], sharingScope: "company", }); mockCompanySkillService.updateFile.mockResolvedValue({ @@ -773,6 +774,38 @@ describe("company skill mutation permissions", () => { expect(mockCompanySkillService.categoryCounts).toHaveBeenCalledWith("company-1"); }); + it("accepts category updates and logs the skill mutation", async () => { + const app = await createApp({ type: "board", source: "local_implicit", userId: "user-1" }); + + const res = await request(app) + .patch("/api/companies/company-1/skills/skill-1") + .send({ categories: ["memory", "review"], sharingScope: "company" }) + .expect(200); + + expect(res.body).toMatchObject({ + id: "skill-1", + categories: ["memory", "review"], + sharingScope: "company", + }); + expect(mockCompanySkillService.updateSkill).toHaveBeenCalledWith("company-1", "skill-1", { + categories: ["memory", "review"], + sharingScope: "company", + }); + expect(mockLogActivity).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ + companyId: "company-1", + actorType: "user", + actorId: "user-1", + action: "company.skill_updated", + entityType: "company_skill", + entityId: "skill-1", + details: { + slug: "review", + categories: ["memory", "review"], + sharingScope: "company", + }, + })); + }); + it("creates skill versions and logs the mutation", async () => { const app = await createApp({ type: "board", source: "local_implicit", userId: "user-1" }); diff --git a/server/src/__tests__/company-skills-service.test.ts b/server/src/__tests__/company-skills-service.test.ts index 26a479567f..81d9158950 100644 --- a/server/src/__tests__/company-skills-service.test.ts +++ b/server/src/__tests__/company-skills-service.test.ts @@ -303,6 +303,46 @@ describeEmbeddedPostgres("companySkillService.list", () => { }); }); + it("updates categories, normalizes values, and reflects them in list filters and counts", 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: "Category Skill", + tagline: "A categorized skill", + categories: ["engineering"], + }); + + const updated = await svc.updateSkill(companyId, skill.id, { + categories: ["Memory", "review", "memory", " "], + }); + + expect(updated.categories).toEqual(["memory", "review"]); + await expect(svc.detail(companyId, skill.id)).resolves.toMatchObject({ + id: skill.id, + categories: ["memory", "review"], + }); + await expect(svc.list(companyId, { categories: ["review"] })).resolves.toEqual([ + expect.objectContaining({ id: skill.id, categories: ["memory", "review"] }), + ]); + await expect(svc.list(companyId, { categories: ["engineering"] })).resolves.toEqual([]); + await expect(svc.categoryCounts(companyId)).resolves.toEqual([ + { slug: "memory", count: 1 }, + { slug: "review", count: 1 }, + ]); + + await expect(svc.updateSkill(companyId, skill.id, { categories: [] })).resolves.toMatchObject({ + id: skill.id, + categories: [], + }); + await expect(svc.categoryCounts(companyId)).resolves.toEqual([]); + }); + it("creates a fork from the creation flow with copied files and lineage", async () => { const companyId = randomUUID(); const sourceSkillId = randomUUID(); diff --git a/server/src/routes/company-skills.ts b/server/src/routes/company-skills.ts index e2e176e4ca..a58e7a6c24 100644 --- a/server/src/routes/company-skills.ts +++ b/server/src/routes/company-skills.ts @@ -446,6 +446,7 @@ export function companySkillRoutes(db: Db) { entityId: result.id, details: { slug: result.slug, + categories: result.categories, sharingScope: result.sharingScope, }, }); diff --git a/ui/src/pages/CompanySkills.test.tsx b/ui/src/pages/CompanySkills.test.tsx index df199276c4..988d088aee 100644 --- a/ui/src/pages/CompanySkills.test.tsx +++ b/ui/src/pages/CompanySkills.test.tsx @@ -124,7 +124,7 @@ function makeVersion(revisionNumber: number, content: string): CompanySkillVersi }; } -function makeDetail(currentVersion: CompanySkillVersion): CompanySkillDetail { +function makeDetail(currentVersion: CompanySkillVersion, overrides: Partial = {}): CompanySkillDetail { return { id: "skill-1", companyId: "company-1", @@ -165,20 +165,25 @@ function makeDetail(currentVersion: CompanySkillVersion): CompanySkillDetail { sourcePath: null, currentVersion, starredByCurrentActor: false, + ...overrides, }; } -async function renderSkillDetail(versions: CompanySkillVersion[]) { +async function renderSkillDetail( + versions: CompanySkillVersion[], + props: Partial> = {}, +) { container = document.createElement("div"); document.body.appendChild(container); root = createRoot(container); + const detail = props.detail ?? makeDetail(versions[0]!); await act(async () => { root?.render( , ); }); @@ -229,6 +235,22 @@ async function click(button: HTMLButtonElement) { }); } +async function inputValue(input: HTMLInputElement, value: string) { + await act(async () => { + const setter = Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, "value")?.set; + setter?.call(input, value); + input.dispatchEvent(new Event("input", { bubbles: true })); + }); +} + +async function selectValue(select: HTMLSelectElement, value: string) { + await act(async () => { + const setter = Object.getOwnPropertyDescriptor(HTMLSelectElement.prototype, "value")?.set; + setter?.call(select, value); + select.dispatchEvent(new Event("change", { bubbles: true })); + }); +} + describe("getSkillVersionDiffSelection", () => { it("selects the previous saved revision or the initial baseline for row diffs", () => { const v1 = makeVersion(1, "first"); @@ -274,3 +296,141 @@ describe("SkillDetailPage versions tab", () => { expect(dialog.textContent).not.toContain("Both sides are the same version"); }); }); + +describe("SkillDetailPage settings", () => { + it("saves normalized category edits from the settings dialog", async () => { + const v1 = makeVersion(1, "# Demo Skill"); + const onUpdateSettings = vi.fn(); + const node = await renderSkillDetail([v1], { + activeTab: "overview", + detail: makeDetail(v1, { + categories: ["engineering"], + sharingScope: "company", + }), + onUpdateSettings, + }); + + await click(buttonsNamed(node, "Settings")[0] as HTMLButtonElement); + const dialog = node.querySelector('[role="dialog"]') as HTMLElement; + const categoryInput = dialog.querySelector("input") as HTMLInputElement; + const saveButton = buttonsNamed(dialog, "Save settings")[0] as HTMLButtonElement; + + expect(categoryInput.value).toBe("engineering"); + + await inputValue(categoryInput, " Memory, review, memory ,,"); + await click(saveButton); + + expect(onUpdateSettings).toHaveBeenCalledWith({ + sharingScope: "company", + categories: ["memory", "review"], + }); + }); + + it("allows clearing categories and saving sharing together", async () => { + const v1 = makeVersion(1, "# Demo Skill"); + const onUpdateSettings = vi.fn(); + const node = await renderSkillDetail([v1], { + activeTab: "overview", + detail: makeDetail(v1, { + categories: ["engineering"], + sharingScope: "company", + }), + onUpdateSettings, + }); + + await click(buttonsNamed(node, "Settings")[0] as HTMLButtonElement); + const dialog = node.querySelector('[role="dialog"]') as HTMLElement; + + await inputValue(dialog.querySelector("input") as HTMLInputElement, ""); + await selectValue(dialog.querySelector("select") as HTMLSelectElement, "private"); + await click(buttonsNamed(dialog, "Save settings")[0] as HTMLButtonElement); + + expect(onUpdateSettings).toHaveBeenCalledWith({ + sharingScope: "private", + categories: [], + }); + }); + + it("does not treat reordered categories as dirty", async () => { + const v1 = makeVersion(1, "# Demo Skill"); + const node = await renderSkillDetail([v1], { + activeTab: "overview", + detail: makeDetail(v1, { + categories: ["memory", "review"], + sharingScope: "company", + }), + }); + + await click(buttonsNamed(node, "Settings")[0] as HTMLButtonElement); + const dialog = node.querySelector('[role="dialog"]') as HTMLElement; + + await inputValue(dialog.querySelector("input") as HTMLInputElement, "review, memory"); + + expect((buttonsNamed(dialog, "Save settings")[0] as HTMLButtonElement).disabled).toBe(true); + }); + + it("keeps the category draft visible while a failed save leaves detail unchanged", async () => { + const v1 = makeVersion(1, "# Demo Skill"); + const detail = makeDetail(v1, { + categories: ["engineering"], + sharingScope: "company", + }); + const onUpdateSettings = vi.fn(); + const node = await renderSkillDetail([v1], { + activeTab: "overview", + detail, + onUpdateSettings, + }); + + await click(buttonsNamed(node, "Settings")[0] as HTMLButtonElement); + const categoryInput = node.querySelector('[role="dialog"] input') as HTMLInputElement; + + await inputValue(categoryInput, "memory"); + await click(buttonsNamed(node.querySelector('[role="dialog"]') as HTMLElement, "Save settings")[0] as HTMLButtonElement); + + await act(async () => { + root?.render( + , + ); + }); + + expect((node.querySelector('[role="dialog"] input') as HTMLInputElement).value).toBe("memory"); + }); +}); diff --git a/ui/src/pages/CompanySkills.tsx b/ui/src/pages/CompanySkills.tsx index edf5ba5f00..0e1b12b010 100644 --- a/ui/src/pages/CompanySkills.tsx +++ b/ui/src/pages/CompanySkills.tsx @@ -17,6 +17,7 @@ import type { CompanySkillSharingScope, CompanySkillSourceBadge, CompanySkillTrustLevel, + CompanySkillUpdateRequest, CompanySkillUpdateStatus, CompanySkillVersion, } from "@paperclipai/shared"; @@ -614,10 +615,22 @@ function normalizeSkillDraftSlug(value: string) { } function splitCategoryDraft(value: string) { - return value - .split(",") - .map((entry) => normalizeSkillDraftSlug(entry)) - .filter(Boolean); + return Array.from( + new Set(value + .split(",") + .map((entry) => normalizeSkillDraftSlug(entry)) + .filter(Boolean)), + ); +} + +function categorySetKey(categories: string[]) { + return [...categories].sort().join(","); +} + +function skillSettingsToastBody(skill: Pick) { + const sharing = skill.sharingScope === "private" ? "Sharing: private" : "Sharing: company"; + const categories = skill.categories.length ? `Categories: ${skill.categories.join(", ")}` : "Categories: none"; + return `${sharing} | ${categories}`; } function defaultSkillMarkdown(name: string, tagline: string) { @@ -2577,8 +2590,8 @@ export function SkillDetailPage({ onToggleStar, starPending, onFork, - onUpdateSharingScope, - updateSharingPending, + onUpdateSettings, + updateSettingsPending, onDelete, deletePending, }: { @@ -2616,13 +2629,15 @@ export function SkillDetailPage({ onToggleStar: () => void; starPending: boolean; onFork: () => void; - onUpdateSharingScope: (scope: Exclude) => void; - updateSharingPending: boolean; + onUpdateSettings: (payload: Pick) => void; + updateSettingsPending: boolean; onDelete: () => void; deletePending: boolean; }) { const [diffOpen, setDiffOpen] = useState(false); const [settingsOpen, setSettingsOpen] = useState(false); + const [settingsSharingScope, setSettingsSharingScope] = useState>("company"); + const [settingsCategoryDraft, setSettingsCategoryDraft] = useState(""); // Top-level description is clamped to four lines; "View all" expands it. We // only surface the toggle when the text actually overflows the clamp. const descriptionRef = useRef(null); @@ -2636,6 +2651,11 @@ export function SkillDetailPage({ useEffect(() => { setDescExpanded(false); }, [detail?.id]); + useEffect(() => { + if (!detail || settingsOpen) return; + setSettingsSharingScope(detail.sharingScope === "public_link" ? "company" : detail.sharingScope); + setSettingsCategoryDraft(detail.categories.join(", ")); + }, [detail, settingsOpen]); const sortedVersions = [...versions].sort((a, b) => b.revisionNumber - a.revisionNumber); const [leftVersionId, setLeftVersionId] = useState(null); const [rightVersionId, setRightVersionId] = useState(null); @@ -2673,6 +2693,10 @@ export function SkillDetailPage({ const latestPin = shortRef(updateStatus?.latestRef); const selectedVersion = versions.find((version) => version.id === currentVersionSelection(skill)) ?? null; const subtitleText = resolveSkillSummaryText(skill) ?? source.label; + const settingsCategories = splitCategoryDraft(settingsCategoryDraft); + const settingsCategoriesDirty = categorySetKey(settingsCategories) !== categorySetKey(skill.categories); + const settingsSharingDirty = settingsSharingScope !== (skill.sharingScope === "public_link" ? "company" : skill.sharingScope); + const settingsDirty = settingsCategoriesDirty || settingsSharingDirty; // Look up the richer agent record (icon, paused) for agents using this skill. const attachAgentMetaById = new Map(attachAgents.map((agent) => [agent.id, agent])); @@ -3221,7 +3245,11 @@ export function SkillDetailPage({
+ + {detail.editable ? (
Danger zone
@@ -4025,20 +4087,28 @@ export function CompanySkills() { }); const updateSkillSettings = useMutation({ - mutationFn: (payload: { skillId: string; sharingScope: Exclude }) => - companySkillsApi.update(selectedCompanyId!, payload.skillId, { sharingScope: payload.sharingScope }), + mutationFn: (payload: { skillId: string; updates: Pick }) => + companySkillsApi.update(selectedCompanyId!, payload.skillId, payload.updates), onSuccess: async (skill) => { + queryClient.setQueryData( + queryKeys.companySkills.detail(selectedCompanyId!, skill.id), + (current) => current ? { ...current, ...skill } : current, + ); + queryClient.setQueryData( + queryKeys.companySkills.list(selectedCompanyId!), + (current) => current?.map((entry) => entry.id === skill.id ? { ...entry, ...skill } : entry), + ); await Promise.all([ queryClient.invalidateQueries({ queryKey: queryKeys.companySkills.list(selectedCompanyId!) }), queryClient.invalidateQueries({ queryKey: queryKeys.companySkills.detail(selectedCompanyId!, skill.id) }), ]); - pushToast({ tone: "success", title: "Sharing updated", body: skill.sharingScope === "private" ? "Private" : "Company" }); + pushToast({ tone: "success", title: "Skill settings updated", body: skillSettingsToastBody(skill) }); }, onError: (error) => { pushToast({ tone: "error", - title: "Sharing update failed", - body: error instanceof Error ? error.message : "Failed to update sharing scope.", + title: "Skill settings update failed", + body: error instanceof Error ? error.message : "Failed to update skill settings.", }); }, }); @@ -4624,8 +4694,8 @@ export function CompanySkills() { onToggleStar={() => toggleStar.mutate()} starPending={toggleStar.isPending} onFork={() => activeDetail && openCreateWizard(buildForkSkillDraft(activeDetail))} - onUpdateSharingScope={(sharingScope) => activeDetail && updateSkillSettings.mutate({ skillId: activeDetail.id, sharingScope })} - updateSharingPending={updateSkillSettings.isPending} + onUpdateSettings={(updates) => activeDetail && updateSkillSettings.mutate({ skillId: activeDetail.id, updates })} + updateSettingsPending={updateSkillSettings.isPending} onDelete={openDeleteDialog} deletePending={deleteSkill.isPending} /> diff --git a/ui/storybook/stories/skills-store-detail.stories.tsx b/ui/storybook/stories/skills-store-detail.stories.tsx index a592dfbecd..b0b95ecbfc 100644 --- a/ui/storybook/stories/skills-store-detail.stories.tsx +++ b/ui/storybook/stories/skills-store-detail.stories.tsx @@ -181,8 +181,8 @@ function SkillDetailHarness({ initialTab = "overview" as DetailTab }: { initialT onToggleStar={() => {}} starPending={false} onFork={() => {}} - onUpdateSharingScope={() => {}} - updateSharingPending={false} + onUpdateSettings={() => {}} + updateSettingsPending={false} onDelete={() => {}} deletePending={false} />