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} />