feat(company-skills): implement skill deletion with agent usage check

Added functionality to prevent deletion of skills that are still in use by agents. Updated the company skill service to throw an unprocessable error if a skill is attempted to be deleted while still referenced by agents. Enhanced the UI to include a delete button and confirmation dialog, displaying relevant messages based on agent usage. Updated tests to cover the new deletion logic and error handling.
This commit is contained in:
Daniel Sousa 2026-04-01 17:18:01 +01:00
parent 5b479652f2
commit 77f854c081
No known key found for this signature in database
4 changed files with 189 additions and 31 deletions

View file

@ -3,6 +3,7 @@ import request from "supertest";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { companySkillRoutes } from "../routes/company-skills.js";
import { errorHandler } from "../middleware/index.js";
import { unprocessable } from "../errors.js";
const mockAgentService = vi.hoisted(() => ({
getById: vi.fn(),
@ -15,6 +16,7 @@ const mockAccessService = vi.hoisted(() => ({
const mockCompanySkillService = vi.hoisted(() => ({
importFromSource: vi.fn(),
deleteSkill: vi.fn(),
}));
const mockLogActivity = vi.hoisted(() => vi.fn());
@ -45,6 +47,11 @@ describe("company skill mutation permissions", () => {
imported: [],
warnings: [],
});
mockCompanySkillService.deleteSkill.mockResolvedValue({
id: "skill-1",
slug: "find-skills",
name: "Find Skills",
});
mockLogActivity.mockResolvedValue(undefined);
mockAccessService.canUser.mockResolvedValue(true);
mockAccessService.hasPermission.mockResolvedValue(false);
@ -110,4 +117,28 @@ describe("company skill mutation permissions", () => {
"https://github.com/vercel-labs/agent-browser",
);
});
it("returns a blocking error when attempting to delete a skill still used by agents", async () => {
mockCompanySkillService.deleteSkill.mockRejectedValue(
unprocessable(
'Cannot delete skill "Find Skills" while it is still used by Builder, Reviewer. Detach it from those agents first.',
),
);
const res = await request(createApp({
type: "board",
userId: "local-board",
companyIds: ["company-1"],
source: "local_implicit",
isInstanceAdmin: false,
}))
.delete("/api/companies/company-1/skills/skill-1");
expect(res.status, JSON.stringify(res.body)).toBe(422);
expect(res.body).toEqual({
error: 'Cannot delete skill "Find Skills" while it is still used by Builder, Reviewer. Detach it from those agents first.',
});
expect(mockCompanySkillService.deleteSkill).toHaveBeenCalledWith("company-1", "skill-1");
expect(mockLogActivity).not.toHaveBeenCalled();
});
});

View file

@ -5,7 +5,7 @@ import { fileURLToPath } from "node:url";
import { and, asc, eq } from "drizzle-orm";
import type { Db } from "@paperclipai/db";
import { companySkills } from "@paperclipai/db";
import { readPaperclipSkillSyncPreference, writePaperclipSkillSyncPreference } from "@paperclipai/adapter-utils/server-utils";
import { readPaperclipSkillSyncPreference } from "@paperclipai/adapter-utils/server-utils";
import type { PaperclipSkillEntry } from "@paperclipai/adapter-utils/server-utils";
import type {
CompanySkill,
@ -2298,26 +2298,23 @@ export function companySkillService(db: Db) {
if (!row) return null;
const skill = toCompanySkill(row);
const usedByAgents = await usage(companyId, skill.key);
// Remove from any agent desiredSkills that reference this skill
const agentRows = await agents.list(companyId);
const allSkills = await listFull(companyId);
for (const agent of agentRows) {
const config = agent.adapterConfig as Record<string, unknown>;
const preference = readPaperclipSkillSyncPreference(config);
const referencesSkill = preference.desiredSkills.some((ref) => {
const resolved = resolveSkillReference(allSkills, ref);
return resolved.skill?.id === skillId;
});
if (referencesSkill) {
const filtered = preference.desiredSkills.filter((ref) => {
const resolved = resolveSkillReference(allSkills, ref);
return resolved.skill?.id !== skillId;
});
await agents.update(agent.id, {
adapterConfig: writePaperclipSkillSyncPreference(config, filtered),
});
}
if (usedByAgents.length > 0) {
const agentNames = usedByAgents.map((agent) => agent.name).sort((left, right) => left.localeCompare(right));
throw unprocessable(
`Cannot delete skill "${skill.name}" while it is still used by ${agentNames.join(", ")}. Detach it from those agents first.`,
{
skillId: skill.id,
skillKey: skill.key,
usedByAgents: usedByAgents.map((agent) => ({
id: agent.id,
name: agent.name,
urlKey: agent.urlKey,
adapterType: agent.adapterType,
})),
},
);
}
// Delete DB row

View file

@ -51,4 +51,8 @@ export const companySkillsApi = {
`/companies/${encodeURIComponent(companyId)}/skills/${encodeURIComponent(skillId)}/install-update`,
{},
),
delete: (companyId: string, skillId: string) =>
api.delete<CompanySkill>(
`/companies/${encodeURIComponent(companyId)}/skills/${encodeURIComponent(skillId)}`,
),
};

View file

@ -52,6 +52,7 @@ import {
RefreshCw,
Save,
Search,
Trash2,
} from "lucide-react";
type SkillTreeNode = {
@ -503,6 +504,8 @@ function SkillPane({
checkUpdatesPending,
onInstallUpdate,
installUpdatePending,
onDelete,
deletePending,
onSave,
savePending,
}: {
@ -522,6 +525,8 @@ function SkillPane({
checkUpdatesPending: boolean;
onInstallUpdate: () => void;
installUpdatePending: boolean;
onDelete: () => void;
deletePending: boolean;
onSave: () => void;
savePending: boolean;
}) {
@ -545,6 +550,10 @@ function SkillPane({
const body = file?.markdown ? stripFrontmatter(file.content) : file?.content ?? "";
const currentPin = shortRef(detail.sourceRef);
const latestPin = shortRef(updateStatus?.latestRef);
const removeBlocked = usedBy.length > 0;
const removeDisabledReason = removeBlocked
? "Detach this skill from all agents before removing it."
: null;
return (
<div className="min-w-0">
@ -559,17 +568,29 @@ function SkillPane({
<p className="mt-2 max-w-3xl text-sm text-muted-foreground">{detail.description}</p>
)}
</div>
{detail.editable ? (
<button
className="inline-flex items-center gap-2 text-sm text-muted-foreground hover:text-foreground"
onClick={() => setEditMode(!editMode)}
<div className="flex flex-wrap items-center justify-end gap-2">
<Button
variant="ghost"
size="sm"
onClick={onDelete}
disabled={deletePending}
title={removeDisabledReason ?? undefined}
>
<Pencil className="h-3.5 w-3.5" />
{editMode ? "Stop editing" : "Edit"}
</button>
) : (
<div className="text-sm text-muted-foreground">{detail.editableReason}</div>
)}
<Trash2 className="mr-1.5 h-3.5 w-3.5" />
{deletePending ? "Removing..." : "Remove"}
</Button>
{detail.editable ? (
<button
className="inline-flex items-center gap-2 text-sm text-muted-foreground hover:text-foreground"
onClick={() => setEditMode(!editMode)}
>
<Pencil className="h-3.5 w-3.5" />
{editMode ? "Stop editing" : "Edit"}
</button>
) : (
<div className="text-sm text-muted-foreground">{detail.editableReason}</div>
)}
</div>
</div>
<div className="mt-4 space-y-3 border-t border-border pt-4 text-sm">
@ -723,7 +744,7 @@ function SkillPane({
) : file.markdown && viewMode === "preview" ? (
<MarkdownBody>{body}</MarkdownBody>
) : (
<pre className="overflow-x-auto whitespace-pre-wrap break-words border-0 bg-transparent p-0 font-mono text-sm text-foreground">
<pre className="overflow-x-auto whitespace-pre-wrap wrap-break-word border-0 bg-transparent p-0 font-mono text-sm text-foreground">
<code>{file.content}</code>
</pre>
)}
@ -751,6 +772,9 @@ export function CompanySkills() {
const [displayedDetail, setDisplayedDetail] = useState<CompanySkillDetail | null>(null);
const [displayedFile, setDisplayedFile] = useState<CompanySkillFileDetail | null>(null);
const [scanStatusMessage, setScanStatusMessage] = useState<string | null>(null);
const [deleteOpen, setDeleteOpen] = useState(false);
const [deleteTargetSkillId, setDeleteTargetSkillId] = useState<string | null>(null);
const [deleteTargetDetail, setDeleteTargetDetail] = useState<CompanySkillDetail | null>(null);
const parsedRoute = useMemo(() => parseSkillRoute(routePath), [routePath]);
const routeSkillId = parsedRoute.skillId;
const selectedPath = parsedRoute.filePath;
@ -848,6 +872,20 @@ export function CompanySkills() {
const activeDetail = detailQuery.data ?? displayedDetail;
const activeFile = fileQuery.data ?? displayedFile;
function openDeleteDialog() {
setDeleteTargetSkillId(selectedSkillId);
setDeleteTargetDetail(activeDetail ?? null);
setDeleteOpen(true);
}
function closeDeleteDialog(open: boolean) {
setDeleteOpen(open);
if (!open) {
setDeleteTargetSkillId(null);
setDeleteTargetDetail(null);
}
}
const importSkill = useMutation({
mutationFn: (importSource: string) => companySkillsApi.importFromSource(selectedCompanyId!, importSource),
onSuccess: async (result) => {
@ -987,6 +1025,44 @@ export function CompanySkills() {
},
});
const deleteSkill = useMutation({
mutationFn: () => companySkillsApi.delete(selectedCompanyId!, deleteTargetSkillId!),
onSuccess: async (skill) => {
closeDeleteDialog(false);
setDisplayedDetail(null);
setDisplayedFile(null);
await Promise.all([
queryClient.invalidateQueries({ queryKey: queryKeys.companySkills.list(selectedCompanyId!) }),
...(deleteTargetSkillId ? [
queryClient.invalidateQueries({ queryKey: queryKeys.companySkills.detail(selectedCompanyId!, deleteTargetSkillId) }),
queryClient.invalidateQueries({ queryKey: queryKeys.companySkills.updateStatus(selectedCompanyId!, deleteTargetSkillId) }),
] : []),
...(deleteTargetSkillId ? [
queryClient.invalidateQueries({
queryKey: queryKeys.companySkills.file(selectedCompanyId!, deleteTargetSkillId, selectedPath),
}),
] : []),
]);
await queryClient.refetchQueries({
queryKey: queryKeys.companySkills.list(selectedCompanyId!),
type: "active",
});
navigate("/skills", { replace: true });
pushToast({
tone: "success",
title: "Skill removed",
body: `${skill.name} was removed from the company skill library.`,
});
},
onError: (error) => {
pushToast({
tone: "error",
title: "Remove failed",
body: error instanceof Error ? error.message : "Failed to remove skill.",
});
},
});
if (!selectedCompanyId) {
return <EmptyState icon={Boxes} message="Select a company to manage skills." />;
}
@ -1002,6 +1078,54 @@ export function CompanySkills() {
return (
<>
<Dialog open={deleteOpen} onOpenChange={closeDeleteDialog}>
<DialogContent className="sm:max-w-md">
<DialogHeader>
<DialogTitle>Remove skill</DialogTitle>
<DialogDescription>
Remove this skill from the company library. If any agents still use it, removal will be blocked until it is detached.
</DialogDescription>
</DialogHeader>
<div className="space-y-3 text-sm">
<p>
{deleteTargetDetail
? `You are about to remove ${deleteTargetDetail.name}.`
: "You are about to remove this skill."}
</p>
{deleteTargetDetail?.usedByAgents?.length ? (
<div className="rounded-md border border-border px-3 py-3 text-muted-foreground">
Currently used by {deleteTargetDetail.usedByAgents.map((agent) => agent.name).join(", ")}.
</div>
) : null}
{(deleteTargetDetail?.usedByAgents.length ?? 0) > 0 ? (
<p className="text-muted-foreground">
Detach this skill from all agents to enable removal.
</p>
) : null}
</div>
<DialogFooter>
{(deleteTargetDetail?.usedByAgents.length ?? 0) > 0 ? (
<Button variant="ghost" onClick={() => closeDeleteDialog(false)}>
Close
</Button>
) : (
<>
<Button variant="ghost" onClick={() => closeDeleteDialog(false)} disabled={deleteSkill.isPending}>
Cancel
</Button>
<Button
variant="destructive"
onClick={() => deleteSkill.mutate()}
disabled={deleteSkill.isPending || !deleteTargetSkillId}
>
{deleteSkill.isPending ? "Removing..." : "Remove skill"}
</Button>
</>
)}
</DialogFooter>
</DialogContent>
</Dialog>
<Dialog open={emptySourceHelpOpen} onOpenChange={setEmptySourceHelpOpen}>
<DialogContent className="sm:max-w-md">
<DialogHeader>
@ -1160,6 +1284,8 @@ export function CompanySkills() {
checkUpdatesPending={updateStatusQuery.isFetching}
onInstallUpdate={() => installUpdate.mutate()}
installUpdatePending={installUpdate.isPending}
onDelete={openDeleteDialog}
deletePending={deleteSkill.isPending}
onSave={() => saveFile.mutate()}
savePending={saveFile.isPending}
/>