feat(ui): offer enabling a skill for agents at install time (#12136)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The company skill library lets operators install skills, and each agent has its own enabled-skill set > - Installing a skill only writes the library row; no agent receives the skill, and the UI says "Skill installed" with no attach step > - Operators install a skill, ask an agent to use it, and the agent truthfully reports the skill as not available — the install felt broken > - This pull request adds an "Enable for agents" step to the install dialog and enables the skill for the selected agents right after install > - The benefit is that "install" defaults to a state where agents can actually use the skill, and the toast is honest when they cannot ## Linked Issues or Issue Description **What existing behavior does this improve?** Installing a skill from the catalog in the company Skills page. **Subsystem affected** Web UI — company skills catalog install flow (`ui/src/pages/CompanySkills.tsx`). **Current behavior** Install writes a `company_skills` row and shows a "Skill installed" toast. No agent is enabled for the skill. Agents resolve their skills from their own desired-skill set, so they report the skill as not installed. The operator has to find the separate "Add to agent" control to make the install effective. **Proposed behavior** The install dialog shows an "Enable for agents" section for fresh installs. It pre-selects every agent whose adapter supports skills. After install, the page enables the skill for each selected agent (skills sync with mode `add`). The success toast reports how many agents received the skill, and warns when the skill is in the library with no agents enabled. **Breaking changes** None. Updates and replacements of an existing skill do not show the new section and behave as before. ## What Changed - `InstallPreviewDialog` gains an "Enable for agents" section (fresh installs only) built on the existing `AgentMultiSelect`, with agents whose adapter lacks skills support disabled. - New exported helper `defaultInstallAgentSelection` pre-selects every skills-capable, non-required agent. - The install mutation enables the skill for each selected agent via `agentsApi.syncSkills(..., "add")` before invalidating queries, and reports per-agent failures in a warning toast without failing the install. - Toast copy now distinguishes "enabled for N agents" from "in the library but not enabled for any agent yet". ## Verification - `cd ui && npx vitest run src/pages/CompanySkills.test.tsx` — 23 tests pass, including three new ones: default-selection helper, confirm payload carries the pre-selected agents, update/replace path skips the section. - `cd ui && pnpm run typecheck` — clean. - Manual: install a catalog skill with two agents in the company; both are pre-selected; after install the skill page lists both under "Used by agents". ## Risks - Low risk. Enablement uses the existing per-agent skills sync route with mode `add`, so concurrent edits to an agent's desired set are not overwritten. A per-agent sync failure surfaces as a warning toast and never fails the install itself. ## Model Used - Claude Fable 5 (`claude-fable-5`, Anthropic) with extended thinking and tool use, via Claude Code. ## 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 - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge
This commit is contained in:
Devin Foley
GitHub
parent
3e28d64a72
commit
d2b9765cc8
|
|
@ -3,11 +3,13 @@
|
|||
import type { ComponentProps, ReactNode } from "react";
|
||||
import { flushSync } from "react-dom";
|
||||
import { createRoot, type Root } from "react-dom/client";
|
||||
import type { CompanySkillDetail, CompanySkillVersion, FolderListResult } from "@paperclipai/shared";
|
||||
import type { CatalogSkill, CompanySkillDetail, CompanySkillVersion, FolderListResult } from "@paperclipai/shared";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
DiscoveryGrid,
|
||||
InstallPreviewDialog,
|
||||
SkillDetailPage,
|
||||
defaultInstallAgentSelection,
|
||||
getSkillVersionDiffSelection,
|
||||
resolveDiscoveryTab,
|
||||
withDiscoveryTab,
|
||||
|
|
@ -837,3 +839,161 @@ describe("SkillDetailPage settings", () => {
|
|||
expect((node.querySelector('[role="dialog"] input') as HTMLInputElement).value).toBe("memory");
|
||||
});
|
||||
});
|
||||
|
||||
describe("install-time agent enablement", () => {
|
||||
const agentOptions = [
|
||||
{ id: "agent-ceo", name: "CEO", adapterType: "claude_local", supportsSkills: true, required: false, icon: null, paused: false },
|
||||
{ id: "agent-designer", name: "Designer", adapterType: "claude_local", supportsSkills: true, required: false, icon: null, paused: true },
|
||||
{ id: "agent-gateway", name: "Gateway", adapterType: "openclaw_gateway", supportsSkills: false, required: false, icon: null, paused: false },
|
||||
{ id: "agent-builtin", name: "Summarizer", adapterType: "claude_local", supportsSkills: true, required: true, icon: null, paused: false },
|
||||
];
|
||||
|
||||
function makeCatalogSkill(): CatalogSkill {
|
||||
return {
|
||||
id: "catalog-1",
|
||||
key: "paperclipai/bundled/product/wireframe",
|
||||
kind: "bundled",
|
||||
category: "product",
|
||||
slug: "wireframe",
|
||||
name: "wireframe",
|
||||
description: "Draw wireframes.",
|
||||
path: "catalog/bundled/product/wireframe",
|
||||
entrypoint: "SKILL.md",
|
||||
trustLevel: "markdown_only",
|
||||
compatibility: "compatible",
|
||||
defaultInstall: false,
|
||||
recommendedForRoles: [],
|
||||
requires: [],
|
||||
tags: [],
|
||||
files: [{ path: "SKILL.md", kind: "skill", sizeBytes: 128, sha256: "abc" }],
|
||||
contentHash: "sha256:abc",
|
||||
};
|
||||
}
|
||||
|
||||
it("defaults to every skills-capable, non-required agent", () => {
|
||||
expect(defaultInstallAgentSelection(agentOptions)).toEqual(new Set(["agent-ceo", "agent-designer"]));
|
||||
});
|
||||
|
||||
it("passes the default agent selection through onConfirm for fresh installs", async () => {
|
||||
const onConfirm = vi.fn();
|
||||
container = document.createElement("div");
|
||||
document.body.appendChild(container);
|
||||
root = createRoot(container);
|
||||
|
||||
await act(async () => {
|
||||
root?.render(
|
||||
<InstallPreviewDialog
|
||||
open
|
||||
onOpenChange={vi.fn()}
|
||||
skill={makeCatalogSkill()}
|
||||
packageName={null}
|
||||
packageVersion={null}
|
||||
conflict={null}
|
||||
defaultSlug="wireframe"
|
||||
defaultForce={false}
|
||||
defaultAction="install"
|
||||
agents={agentOptions}
|
||||
isPending={false}
|
||||
error={null}
|
||||
onConfirm={onConfirm}
|
||||
/>,
|
||||
);
|
||||
});
|
||||
// The dialog seeds its slug/agent state in passive effects; give them a
|
||||
// macrotask to flush before interacting.
|
||||
await act(async () => {
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
});
|
||||
|
||||
const node = container as ParentNode;
|
||||
expect(node.textContent).toContain("Enable for agents");
|
||||
|
||||
await click(buttonsNamed(node, "Install skill")[0] as HTMLButtonElement);
|
||||
|
||||
expect(onConfirm).toHaveBeenCalledWith({
|
||||
slug: "wireframe",
|
||||
force: false,
|
||||
agentIds: expect.arrayContaining(["agent-ceo", "agent-designer"]),
|
||||
});
|
||||
expect(onConfirm.mock.calls[0][0].agentIds).toHaveLength(2);
|
||||
});
|
||||
|
||||
it("keeps tracking the agent default when agents load after the dialog opens", async () => {
|
||||
const onConfirm = vi.fn();
|
||||
container = document.createElement("div");
|
||||
document.body.appendChild(container);
|
||||
root = createRoot(container);
|
||||
|
||||
const renderDialog = (agents: typeof agentOptions) =>
|
||||
root?.render(
|
||||
<InstallPreviewDialog
|
||||
open
|
||||
onOpenChange={vi.fn()}
|
||||
skill={makeCatalogSkill()}
|
||||
packageName={null}
|
||||
packageVersion={null}
|
||||
conflict={null}
|
||||
defaultSlug="wireframe"
|
||||
defaultForce={false}
|
||||
defaultAction="install"
|
||||
agents={agents}
|
||||
isPending={false}
|
||||
error={null}
|
||||
onConfirm={onConfirm}
|
||||
/>,
|
||||
);
|
||||
|
||||
// Dialog opens before the agents query resolves: nothing to select yet.
|
||||
await act(async () => renderDialog([]));
|
||||
await act(async () => {
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
});
|
||||
|
||||
// The agents arrive later; the untouched selection must pick them up.
|
||||
await act(async () => renderDialog(agentOptions));
|
||||
await act(async () => {
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
});
|
||||
|
||||
await click(buttonsNamed(container as ParentNode, "Install skill")[0] as HTMLButtonElement);
|
||||
|
||||
expect(onConfirm.mock.calls[0][0].agentIds).toHaveLength(2);
|
||||
});
|
||||
|
||||
it("skips agent enablement for updates and replacements", async () => {
|
||||
const onConfirm = vi.fn();
|
||||
container = document.createElement("div");
|
||||
document.body.appendChild(container);
|
||||
root = createRoot(container);
|
||||
|
||||
await act(async () => {
|
||||
root?.render(
|
||||
<InstallPreviewDialog
|
||||
open
|
||||
onOpenChange={vi.fn()}
|
||||
skill={makeCatalogSkill()}
|
||||
packageName={null}
|
||||
packageVersion={null}
|
||||
conflict={null}
|
||||
defaultSlug="wireframe"
|
||||
defaultForce={false}
|
||||
defaultAction="update"
|
||||
agents={agentOptions}
|
||||
isPending={false}
|
||||
error={null}
|
||||
onConfirm={onConfirm}
|
||||
/>,
|
||||
);
|
||||
});
|
||||
await act(async () => {
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
});
|
||||
|
||||
const node = container as ParentNode;
|
||||
expect(node.textContent).not.toContain("Enable for agents");
|
||||
|
||||
await click(buttonsNamed(node, "Install update")[0] as HTMLButtonElement);
|
||||
|
||||
expect(onConfirm).toHaveBeenCalledWith({ slug: "wireframe", force: false, agentIds: [] });
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -2018,7 +2018,17 @@ function CatalogDetailPane({
|
|||
);
|
||||
}
|
||||
|
||||
function InstallPreviewDialog({
|
||||
// Installing only adds a skill to the company library; an agent can use it only
|
||||
// once it is also enabled for that agent. Pre-select every agent that can
|
||||
// receive the skill so "install" defaults to a state where the skill is
|
||||
// actually usable, instead of a library row no agent has.
|
||||
export function defaultInstallAgentSelection(
|
||||
agents: Array<Pick<AttachAgentOption, "id" | "supportsSkills" | "required">>,
|
||||
): Set<string> {
|
||||
return new Set(agents.filter((agent) => agent.supportsSkills && !agent.required).map((agent) => agent.id));
|
||||
}
|
||||
|
||||
export function InstallPreviewDialog({
|
||||
open,
|
||||
onOpenChange,
|
||||
skill,
|
||||
|
|
@ -2028,6 +2038,7 @@ function InstallPreviewDialog({
|
|||
defaultSlug,
|
||||
defaultForce,
|
||||
defaultAction,
|
||||
agents,
|
||||
isPending,
|
||||
error,
|
||||
onConfirm,
|
||||
|
|
@ -2041,13 +2052,21 @@ function InstallPreviewDialog({
|
|||
defaultSlug: string | null;
|
||||
defaultForce: boolean;
|
||||
defaultAction: "install" | "update" | "replace";
|
||||
agents: AttachAgentOption[];
|
||||
isPending: boolean;
|
||||
error: string | null;
|
||||
onConfirm: (input: { slug: string | null; force: boolean }) => void;
|
||||
onConfirm: (input: { slug: string | null; force: boolean; agentIds: string[] }) => void;
|
||||
}) {
|
||||
const [slug, setSlug] = useState<string>("");
|
||||
const [force, setForce] = useState(false);
|
||||
const [advancedOpen, setAdvancedOpen] = useState(false);
|
||||
const [selectedAgentIds, setSelectedAgentIds] = useState<Set<string>>(new Set());
|
||||
// Whether the user changed the agent selection this open. Until then the
|
||||
// selection keeps tracking the default: the agents query may resolve after
|
||||
// the dialog opens, and a one-shot seed would freeze an empty selection and
|
||||
// install the skill for nobody.
|
||||
const [selectionTouched, setSelectionTouched] = useState(false);
|
||||
const wasOpenRef = useRef(false);
|
||||
|
||||
useEffect(() => {
|
||||
if (!open) return;
|
||||
|
|
@ -2056,6 +2075,18 @@ function InstallPreviewDialog({
|
|||
setAdvancedOpen(defaultAction === "replace" || defaultForce);
|
||||
}, [open, defaultSlug, defaultForce, defaultAction]);
|
||||
|
||||
useEffect(() => {
|
||||
if (open && !wasOpenRef.current) setSelectionTouched(false);
|
||||
wasOpenRef.current = open;
|
||||
}, [open]);
|
||||
|
||||
// Track the default selection while the dialog is open and untouched; the
|
||||
// user's first change takes over and background refetches never clobber it.
|
||||
useEffect(() => {
|
||||
if (!open || selectionTouched) return;
|
||||
setSelectedAgentIds(defaultAction === "install" ? defaultInstallAgentSelection(agents) : new Set());
|
||||
}, [open, selectionTouched, defaultAction, agents]);
|
||||
|
||||
if (!skill) return null;
|
||||
|
||||
let confirmLabel = "Install skill";
|
||||
|
|
@ -2140,6 +2171,33 @@ function InstallPreviewDialog({
|
|||
</div>
|
||||
) : null}
|
||||
|
||||
{defaultAction === "install" ? (
|
||||
<div className="rounded-md border border-border p-3">
|
||||
<div className="mb-1 text-xs uppercase tracking-wide text-muted-foreground">Enable for agents</div>
|
||||
<p className="mb-2 text-xs text-muted-foreground">
|
||||
Installing adds the skill to the company library. Agents can only use it once it is enabled for them.
|
||||
</p>
|
||||
<AgentMultiSelect
|
||||
agents={agents}
|
||||
selectedAgentIds={selectedAgentIds}
|
||||
onChange={(next) => {
|
||||
setSelectionTouched(true);
|
||||
setSelectedAgentIds(next);
|
||||
}}
|
||||
showSelectionPreview={false}
|
||||
emptyMessage="No agents in this company support skills yet."
|
||||
isAgentDisabled={(agent) => {
|
||||
const option = agent as AttachAgentOption;
|
||||
return option.required || !option.supportsSkills;
|
||||
}}
|
||||
getDescription={(agent) => {
|
||||
const option = agent as AttachAgentOption;
|
||||
return `${option.adapterType}${option.required ? " · required" : ""}${!option.supportsSkills ? " · skills not supported" : ""}`;
|
||||
}}
|
||||
/>
|
||||
</div>
|
||||
) : null}
|
||||
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => setAdvancedOpen((value) => !value)}
|
||||
|
|
@ -2174,7 +2232,13 @@ function InstallPreviewDialog({
|
|||
</Button>
|
||||
<Button
|
||||
variant={confirmVariant}
|
||||
onClick={() => onConfirm({ slug: slug.trim().length > 0 ? slug.trim() : null, force })}
|
||||
onClick={() =>
|
||||
onConfirm({
|
||||
slug: slug.trim().length > 0 ? slug.trim() : null,
|
||||
force,
|
||||
agentIds: defaultAction === "install" ? Array.from(selectedAgentIds) : [],
|
||||
})
|
||||
}
|
||||
disabled={isPending}
|
||||
>
|
||||
{confirmLabel}
|
||||
|
|
@ -4583,13 +4647,29 @@ export function CompanySkills() {
|
|||
return counts;
|
||||
}, [installedSkills]);
|
||||
const installCatalog = useMutation({
|
||||
mutationFn: (payload: { catalogSkillId: string; slug: string | null; force: boolean }) =>
|
||||
mutationFn: (payload: { catalogSkillId: string; slug: string | null; force: boolean; agentIds: string[] }) =>
|
||||
companySkillsApi.installCatalog(selectedCompanyId!, {
|
||||
catalogSkillId: payload.catalogSkillId,
|
||||
slug: payload.slug,
|
||||
force: payload.force,
|
||||
}),
|
||||
onSuccess: async (result) => {
|
||||
onSuccess: async (result, payload) => {
|
||||
// Enable the skill for the agents chosen in the install dialog before any
|
||||
// invalidation, so the refetched skill detail already reflects the
|
||||
// attachments. Mode "add" appends to each agent's desired set without
|
||||
// clobbering concurrent edits. A per-agent failure must not fail the
|
||||
// install itself — the skill is in the library either way.
|
||||
const enableTargets = result.action === "created" ? payload.agentIds : [];
|
||||
let enabledCount = 0;
|
||||
let enableFailures = 0;
|
||||
for (const agentId of enableTargets) {
|
||||
try {
|
||||
await agentsApi.syncSkills(agentId, [{ key: result.skill.key, versionId: null }], "add", selectedCompanyId ?? undefined);
|
||||
enabledCount += 1;
|
||||
} catch {
|
||||
enableFailures += 1;
|
||||
}
|
||||
}
|
||||
await Promise.all([
|
||||
queryClient.invalidateQueries({ queryKey: queryKeys.companySkills.list(selectedCompanyId!) }),
|
||||
queryClient.invalidateQueries({ queryKey: queryKeys.companySkills.detail(selectedCompanyId!, result.skill.id) }),
|
||||
|
|
@ -4598,8 +4678,19 @@ export function CompanySkills() {
|
|||
pushToast({
|
||||
tone: "success",
|
||||
title: result.action === "created" ? "Skill installed" : result.action === "updated" ? "Skill updated" : "Skill is up to date",
|
||||
body: result.skill.name,
|
||||
body: result.action === "created"
|
||||
? enabledCount > 0
|
||||
? `${result.skill.name} — enabled for ${enabledCount} agent${enabledCount === 1 ? "" : "s"}.`
|
||||
: `${result.skill.name} is in the library but not enabled for any agent yet. Use "Add to agent" to enable it.`
|
||||
: result.skill.name,
|
||||
});
|
||||
if (enableFailures > 0) {
|
||||
pushToast({
|
||||
tone: "warn",
|
||||
title: "Skill installed, but enabling failed",
|
||||
body: `Could not enable ${result.skill.name} for ${enableFailures} agent${enableFailures === 1 ? "" : "s"}. Use "Add to agent" on the skill page.`,
|
||||
});
|
||||
}
|
||||
if (result.warnings[0]) {
|
||||
pushToast({ tone: "warn", title: "Install warnings", body: result.warnings[0] });
|
||||
}
|
||||
|
|
@ -5165,14 +5256,16 @@ export function CompanySkills() {
|
|||
defaultSlug={installDialogState.defaultSlug}
|
||||
defaultForce={installDialogState.defaultForce}
|
||||
defaultAction={installDialogState.defaultAction}
|
||||
agents={eligibleAgentsForAttach}
|
||||
isPending={installCatalog.isPending}
|
||||
error={installDialogState.error}
|
||||
onConfirm={({ slug, force }) => {
|
||||
onConfirm={({ slug, force, agentIds }) => {
|
||||
if (!installDialogState.catalogSkill) return;
|
||||
installCatalog.mutate({
|
||||
catalogSkillId: installDialogState.catalogSkill.id,
|
||||
slug,
|
||||
force,
|
||||
agentIds,
|
||||
});
|
||||
}}
|
||||
/>
|
||||
|
|
|
|||
Loading…
Reference in New Issue