From b616d887f6dcbcb11e5727a4fe1d3b6e31c3d0f4 Mon Sep 17 00:00:00 2001 From: Dotta Date: Thu, 6 Aug 2026 14:29:31 +0000 Subject: [PATCH 1/2] feat(skills): require explicit assignment merge modes Make agent skill sync use required add, remove, or replace semantics. Default imported skill conflicts to skip and expose per-skill import outcomes across the API, UI, and CLI. Co-Authored-By: Paperclip --- cli/src/__tests__/agent-lifecycle.test.ts | 10 +- cli/src/__tests__/company.test.ts | 13 +++ cli/src/__tests__/skills.test.ts | 4 +- cli/src/commands/client/agent.ts | 10 +- cli/src/commands/client/company.ts | 21 ++++ cli/src/commands/client/skills.ts | 42 ++++--- doc/CLI.md | 17 +-- docs/cli/control-plane-commands.md | 2 +- packages/shared/src/index.ts | 2 + packages/shared/src/types/adapter-skills.ts | 3 + .../shared/src/types/company-portability.ts | 9 ++ packages/shared/src/types/index.ts | 1 + .../shared/src/validators/adapter-skills.ts | 7 ++ packages/shared/src/validators/index.ts | 1 + .../src/__tests__/agent-skills-routes.test.ts | 103 +++++++++++++++++- .../src/__tests__/company-portability.test.ts | 29 ++++- .../__tests__/company-skills-service.test.ts | 66 +++++++++++ server/src/routes/agents.ts | 86 ++++++++++++--- server/src/routes/openapi.ts | 2 +- server/src/services/company-portability.ts | 13 ++- server/src/services/company-skills.ts | 44 +++++--- ui/src/api/agents.ts | 9 +- .../skill-studio/AgentsUsingSkillDialog.tsx | 2 +- ui/src/pages/CompanyImport.test.tsx | 1 + ui/src/pages/CompanyImport.tsx | 21 ++++ ui/src/pages/CompanySkills.tsx | 2 +- ui/src/pages/agent-skills/AgentSkillsTab.tsx | 2 +- ui/storybook/stories/team-catalog.stories.tsx | 1 + 28 files changed, 450 insertions(+), 73 deletions(-) diff --git a/cli/src/__tests__/agent-lifecycle.test.ts b/cli/src/__tests__/agent-lifecycle.test.ts index 4e2917ce83e..41817823db9 100644 --- a/cli/src/__tests__/agent-lifecycle.test.ts +++ b/cli/src/__tests__/agent-lifecycle.test.ts @@ -83,7 +83,15 @@ describe("agent lifecycle commands", () => { await run(["agent", "runtime-state:reset-session", AGENT_ID, "--task-key", "task-1"]); await run(["agent", "task-sessions", AGENT_ID]); await run(["agent", "skills", AGENT_ID]); - await run(["agent", "skills:sync", AGENT_ID, "--desired-skills", "paperclip,github"]); + await run([ + "agent", + "skills:sync", + AGENT_ID, + "--desired-skills", + "paperclip,github", + "--mode", + "replace", + ]); await run(["agent", "instructions-path:update", AGENT_ID, "--payload-json", JSON.stringify({ path: "/tmp/AGENTS.md" })]); await run(["agent", "instructions-bundle", AGENT_ID]); await run(["agent", "instructions-bundle:update", AGENT_ID, "--payload-json", JSON.stringify({ mode: "managed" })]); diff --git a/cli/src/__tests__/company.test.ts b/cli/src/__tests__/company.test.ts index 1532047f6f8..a548c8821a4 100644 --- a/cli/src/__tests__/company.test.ts +++ b/cli/src/__tests__/company.test.ts @@ -504,6 +504,17 @@ describe("renderCompanyImportResult", () => { { slug: "cto", id: "agent-2", action: "updated", name: "CTO", reason: "replace strategy" }, { slug: "ops", id: null, action: "skipped", name: "Ops", reason: "skip strategy" }, ], + skills: [ + { + originalKey: "company/source/review", + originalSlug: "review", + key: "company/target/review-2", + slug: "review-2", + id: "skill-1", + action: "renamed", + reason: "rename strategy", + }, + ], projects: [ { slug: "app", id: "project-1", action: "created", name: "App", reason: null }, { slug: "ops", id: "project-2", action: "updated", name: "Operations", reason: "replace strategy" }, @@ -525,8 +536,10 @@ describe("renderCompanyImportResult", () => { expect(rendered).toContain("Company"); expect(rendered).toContain("https://paperclip.example/PAP/dashboard"); expect(rendered).toContain("3 agents total (1 created, 1 updated, 1 skipped)"); + expect(rendered).toContain("1 skill total (1 renamed)"); expect(rendered).toContain("3 projects total (1 created, 1 updated, 1 skipped)"); expect(rendered).toContain("Agent results"); + expect(rendered).toContain("Skill results"); expect(rendered).toContain("Project results"); expect(rendered).toContain("Using claude-local adapter"); expect(rendered).toContain("Review API keys"); diff --git a/cli/src/__tests__/skills.test.ts b/cli/src/__tests__/skills.test.ts index 038b9812f1b..80d0dd3c791 100644 --- a/cli/src/__tests__/skills.test.ts +++ b/cli/src/__tests__/skills.test.ts @@ -479,6 +479,8 @@ describe("skills CLI commands", () => { "review-prs", "--skill", "paperclip/qa", + "--mode", + "add", "--company-id", "company-1", "--api-base", @@ -498,7 +500,7 @@ describe("skills CLI commands", () => { "http://paperclip.test/api/agents/agent-1/skills/sync", expect.objectContaining({ method: "POST", - body: JSON.stringify({ desiredSkills: ["review-prs", "paperclip/qa"] }), + body: JSON.stringify({ desiredSkills: ["review-prs", "paperclip/qa"], mode: "add" }), }), ); expect(JSON.parse(String(logSpy.mock.calls[0]?.[0]))).toEqual(snapshot); diff --git a/cli/src/commands/client/agent.ts b/cli/src/commands/client/agent.ts index 8144352c491..6d6677e4244 100644 --- a/cli/src/commands/client/agent.ts +++ b/cli/src/commands/client/agent.ts @@ -71,6 +71,7 @@ interface AgentResetSessionOptions extends BaseClientOptions { interface AgentSkillsSyncOptions extends BaseClientOptions { desiredSkills: string; + mode: string; } interface AgentInstructionsFileOptions extends BaseClientOptions { @@ -589,10 +590,17 @@ export function registerAgentCommands(program: Command): void { .description("Sync desired skills onto an agent") .argument("", "Agent ID") .requiredOption("--desired-skills ", "Desired skill names") + .requiredOption( + "--mode ", + "Merge mode: add keeps other skills; remove deletes only named skills; replace destructively overwrites the complete set", + ) .action(async (agentId: string, opts: AgentSkillsSyncOptions) => { try { const ctx = resolveCommandContext(opts); - const payload = agentSkillSyncSchema.parse({ desiredSkills: parseCsv(opts.desiredSkills) }); + const payload = agentSkillSyncSchema.parse({ + desiredSkills: parseCsv(opts.desiredSkills), + mode: opts.mode, + }); const result = await ctx.api.post(apiPath`/api/agents/${agentId}/skills/sync`, payload); printOutput(result, { json: ctx.json }); } catch (err) { diff --git a/cli/src/commands/client/company.ts b/cli/src/commands/client/company.ts index bd0d162a502..72864f6482c 100644 --- a/cli/src/commands/client/company.ts +++ b/cli/src/commands/client/company.ts @@ -548,6 +548,16 @@ function summarizeImportAgentResults(agents: CompanyPortabilityImportResult["age return `${agents.length} ${pluralize(agents.length, "agent")} total (${parts.join(", ")})`; } +function summarizeImportSkillResults(skills: CompanyPortabilityImportResult["skills"]): string { + if (skills.length === 0) return "0 skills changed"; + const actions = ["created", "renamed", "replaced", "skipped"] as const; + const parts = actions.flatMap((action) => { + const count = skills.filter((skill) => skill.action === action).length; + return count > 0 ? [`${count} ${action}`] : []; + }); + return `${skills.length} ${pluralize(skills.length, "skill")} total (${parts.join(", ")})`; +} + function summarizeImportProjectResults(projects: CompanyPortabilityImportResult["projects"]): string { if (projects.length === 0) return "0 projects changed"; const created = projects.filter((project) => project.action === "created").length; @@ -681,10 +691,12 @@ export function renderCompanyImportResult( result: CompanyPortabilityImportResult, meta: { targetLabel: string; companyUrl?: string; infoMessages?: string[] }, ): string { + const skills = result.skills ?? []; const lines: string[] = [ `${pc.bold("Target")} ${meta.targetLabel}`, `${pc.bold("Company")} ${result.company.name} (${actionChip(result.company.action)})`, `${pc.bold("Agents")} ${summarizeImportAgentResults(result.agents)}`, + `${pc.bold("Skills")} ${summarizeImportSkillResults(skills)}`, `${pc.bold("Projects")} ${summarizeImportProjectResults(result.projects)}`, ]; @@ -701,6 +713,15 @@ export function renderCompanyImportResult( reason: agent.reason, })), ); + appendPreviewExamples( + lines, + "Skill results", + skills.map((skill) => ({ + action: skill.action, + label: `${skill.originalSlug} -> ${skill.slug}`, + reason: skill.reason, + })), + ); appendPreviewExamples( lines, "Project results", diff --git a/cli/src/commands/client/skills.ts b/cli/src/commands/client/skills.ts index c97ebd18df1..f24316c9c49 100644 --- a/cli/src/commands/client/skills.ts +++ b/cli/src/commands/client/skills.ts @@ -1,17 +1,19 @@ import { Command } from "commander"; -import type { - Agent, - AgentSkillSnapshot, - CatalogSkill, - CompanySkill, - CompanySkillAuditResult, - CompanySkillDetail, - CompanySkillFileDetail, - CompanySkillImportResult, - CompanySkillInstallCatalogResult, - CompanySkillListItem, - CompanySkillProjectScanResult, - CompanySkillUpdateStatus, +import { + agentSkillAssignmentModeSchema, + type AgentSkillAssignmentMode, + type Agent, + type AgentSkillSnapshot, + type CatalogSkill, + type CompanySkill, + type CompanySkillAuditResult, + type CompanySkillDetail, + type CompanySkillFileDetail, + type CompanySkillImportResult, + type CompanySkillInstallCatalogResult, + type CompanySkillListItem, + type CompanySkillProjectScanResult, + type CompanySkillUpdateStatus, } from "@paperclipai/shared"; import { readFile } from "node:fs/promises"; import { stdin as input, stdout as output } from "node:process"; @@ -69,6 +71,7 @@ interface ConfirmedSkillOptions extends SkillsOptions { interface AgentSkillSyncOptions extends SkillsOptions { skill?: string[]; + mode: AgentSkillAssignmentMode; } type CompanySkillReferenceTarget = Pick; @@ -502,9 +505,13 @@ function registerAgentSkillCommands(skills: Command): void { addCommonClientOptions( agent .command("sync") - .description("Replace an agent's desired company skills and sync runtime state") + .description("Merge an agent's desired company skills and sync runtime state") .argument("", "Agent ID or shortname/url-key") .option("--skill ", "Desired company skill ID, key, or slug; may be repeated", collectOptionValue, [] as string[]) + .requiredOption( + "--mode ", + "Merge mode: add keeps other skills; remove deletes only named skills; replace destructively overwrites the complete set", + ) .action(async (agentRef: string, opts: AgentSkillSyncOptions) => { try { const desiredSkills = opts.skill ?? []; @@ -513,16 +520,17 @@ function registerAgentSkillCommands(skills: Command): void { } const ctx = resolveCommandContext(opts, { requireCompany: true }); const agentRow = await resolveAgent(ctx, agentRef); + const mode = agentSkillAssignmentModeSchema.parse(opts.mode); const snapshot = await ctx.api.post( `/api/agents/${encodeURIComponent(agentRow.id)}/skills/sync`, - { desiredSkills }, + { desiredSkills, mode }, ); if (ctx.json) { printOutput(snapshot, { json: true }); return; } console.log( - `Desired company skills replaced for ${agentRow.name} (${agentRow.id}); runtime sync returned ${snapshot?.entries.length ?? 0} entrie(s).`, + `Desired company skills updated with ${mode} mode for ${agentRow.name} (${agentRow.id}); runtime sync returned ${snapshot?.entries.length ?? 0} entrie(s).`, ); printAgentSkillSnapshot(snapshot, agentRow); } catch (err) { @@ -548,7 +556,7 @@ function registerAgentSkillCommands(skills: Command): void { ); const snapshot = await ctx.api.post( `/api/agents/${encodeURIComponent(agentRow.id)}/skills/sync`, - { desiredSkills: [] }, + { desiredSkills: [], mode: "replace" }, ); if (ctx.json) { printOutput(snapshot, { json: true }); diff --git a/doc/CLI.md b/doc/CLI.md index 5b36f799589..5c75aeb4e85 100644 --- a/doc/CLI.md +++ b/doc/CLI.md @@ -365,7 +365,7 @@ pnpm paperclipai agent runtime-state pnpm paperclipai agent runtime-state:reset-session [--task-key ] pnpm paperclipai agent task-sessions pnpm paperclipai agent skills -pnpm paperclipai agent skills:sync --desired-skills paperclip,github +pnpm paperclipai agent skills:sync --desired-skills paperclip,github --mode add pnpm paperclipai agent instructions-path:update --payload-json '{"path":"/path/to/AGENTS.md"}' pnpm paperclipai agent instructions-bundle pnpm paperclipai agent instructions-bundle:update --payload-json '{"mode":"managed"}' @@ -465,9 +465,10 @@ By default the command creates a `todo` issue assigned to the target agent and w 1. **Company install** — adds or updates a row in `company_skills` for the whole company. This is what `skills install`, `skills import`, `skills create`, and `skills scan-projects` do. -2. **Agent attach** — replaces an agent's *desired* company skill set - (`skills agent sync`/`clear`). This is a desired-state operation on the - agent's adapter config; it does not change the company library. +2. **Agent attach** — merges an agent's *desired* company skill set with an + explicit `add`, `remove`, or `replace` mode (`skills agent sync`/`clear`). + This is a desired-state operation on the agent's adapter config; it does not + change the company library. 3. **Adapter runtime sync** — the adapter reconciles the desired skill set with files on disk and reports an `AgentSkillSnapshot` (`skills agent list`). `skills agent sync` triggers this automatically after updating desired state. @@ -564,12 +565,14 @@ maintenance loop for catalog-installed skills: ```sh pnpm paperclipai skills agent list --company-id -pnpm paperclipai skills agent sync --skill [--skill ...] --company-id +pnpm paperclipai skills agent sync --skill [--skill ...] --mode --company-id pnpm paperclipai skills agent clear --yes --company-id ``` -`skills agent sync` replaces the agent's non-required desired skill set (it is -not additive) and returns the resulting adapter `AgentSkillSnapshot`. +`skills agent sync` requires a merge mode and returns the resulting adapter +`AgentSkillSnapshot`. `add` preserves all unnamed assignments, `remove` deletes +only named assignments, and `replace` destructively overwrites the complete +non-required desired skill set. `skills agent clear` sends an empty desired list. Required Paperclip skills are still enforced by the server in both cases. diff --git a/docs/cli/control-plane-commands.md b/docs/cli/control-plane-commands.md index 406b3190bd7..70adf99ad24 100644 --- a/docs/cli/control-plane-commands.md +++ b/docs/cli/control-plane-commands.md @@ -91,7 +91,7 @@ pnpm paperclipai skills import ./skills/my-skill --company-id pnpm paperclipai skills import owner/repo/path/to/skill --company-id # Attach desired company skills to an agent after install/import -pnpm paperclipai skills agent sync --skill github-pr-workflow --company-id +pnpm paperclipai skills agent sync --skill github-pr-workflow --mode add --company-id ``` ## Approval Commands diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index cbb2b9e49bf..b4ce278490c 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -764,6 +764,7 @@ export type { CatalogTeamInstallResult, InstalledCatalogTeam, AgentSkillSyncMode, + AgentSkillAssignmentMode, AgentSkillState, AgentSkillOrigin, AgentDesiredSkillEntry, @@ -1595,6 +1596,7 @@ export { type ProbeEnvironmentConfig, agentSkillStateSchema, agentSkillSyncModeSchema, + agentSkillAssignmentModeSchema, agentDesiredSkillEntrySchema, agentDesiredSkillSelectionSchema, agentSkillEntrySchema, diff --git a/packages/shared/src/types/adapter-skills.ts b/packages/shared/src/types/adapter-skills.ts index 998dda9a54d..ac77f7e013e 100644 --- a/packages/shared/src/types/adapter-skills.ts +++ b/packages/shared/src/types/adapter-skills.ts @@ -1,5 +1,7 @@ export type AgentSkillSyncMode = "unsupported" | "persistent" | "ephemeral"; +export type AgentSkillAssignmentMode = "add" | "remove" | "replace"; + export type AgentSkillState = | "available" | "configured" @@ -46,5 +48,6 @@ export interface AgentSkillSnapshot { } export interface AgentSkillSyncRequest { + mode: AgentSkillAssignmentMode; desiredSkills: Array; } diff --git a/packages/shared/src/types/company-portability.ts b/packages/shared/src/types/company-portability.ts index b43afdda2e3..3cc0eb4a6f0 100644 --- a/packages/shared/src/types/company-portability.ts +++ b/packages/shared/src/types/company-portability.ts @@ -402,6 +402,15 @@ export interface CompanyPortabilityImportResult { name: string; reason: string | null; }[]; + skills: { + originalKey: string; + originalSlug: string; + key: string; + slug: string; + id: string; + action: "created" | "renamed" | "replaced" | "skipped"; + reason: string | null; + }[]; projects: { slug: string; id: string | null; diff --git a/packages/shared/src/types/index.ts b/packages/shared/src/types/index.ts index 6027c15d39e..e23ae3daeb7 100644 --- a/packages/shared/src/types/index.ts +++ b/packages/shared/src/types/index.ts @@ -250,6 +250,7 @@ export type { } from "./teams-catalog.js"; export type { AgentSkillSyncMode, + AgentSkillAssignmentMode, AgentSkillState, AgentSkillOrigin, AgentDesiredSkillEntry, diff --git a/packages/shared/src/validators/adapter-skills.ts b/packages/shared/src/validators/adapter-skills.ts index 96c0c4998d5..d0befc91103 100644 --- a/packages/shared/src/validators/adapter-skills.ts +++ b/packages/shared/src/validators/adapter-skills.ts @@ -21,6 +21,12 @@ export const agentSkillSyncModeSchema = z.enum([ "ephemeral", ]); +export const agentSkillAssignmentModeSchema = z.enum([ + "add", + "remove", + "replace", +]); + export const agentDesiredSkillEntrySchema = z.object({ key: z.string().min(1), versionId: z.string().uuid().nullable(), @@ -59,6 +65,7 @@ export const agentSkillSnapshotSchema = z.object({ }); export const agentSkillSyncSchema = z.object({ + mode: agentSkillAssignmentModeSchema, desiredSkills: z.array(agentDesiredSkillSelectionSchema), }); diff --git a/packages/shared/src/validators/index.ts b/packages/shared/src/validators/index.ts index d93c6649dd6..b0923b60848 100644 --- a/packages/shared/src/validators/index.ts +++ b/packages/shared/src/validators/index.ts @@ -282,6 +282,7 @@ export { export { agentSkillStateSchema, agentSkillSyncModeSchema, + agentSkillAssignmentModeSchema, agentDesiredSkillEntrySchema, agentDesiredSkillSelectionSchema, agentSkillEntrySchema, diff --git a/server/src/__tests__/agent-skills-routes.test.ts b/server/src/__tests__/agent-skills-routes.test.ts index db6df76cc17..6dd77801ed1 100644 --- a/server/src/__tests__/agent-skills-routes.test.ts +++ b/server/src/__tests__/agent-skills-routes.test.ts @@ -488,7 +488,7 @@ describe.sequential("agent skill routes", () => { await createApp(), (baseUrl) => request(baseUrl) .post("/api/agents/11111111-1111-4111-8111-111111111111/skills/sync?companyId=company-1") - .send({ desiredSkills: ["paperclip"] }), + .send({ desiredSkills: ["paperclip"], mode: "replace" }), ); expect(syncRes.status, JSON.stringify(syncRes.body)).toBe(200); const syncCall = mockSecretService.resolveAdapterConfigForRuntime.mock.calls.at(-1); @@ -588,7 +588,7 @@ describe.sequential("agent skill routes", () => { const res = await requestApp(await createApp(), (baseUrl) => request(baseUrl) .post("/api/agents/11111111-1111-4111-8111-111111111111/skills/sync?companyId=company-1") - .send({ desiredSkills: ["paperclip"] })); + .send({ desiredSkills: ["paperclip"], mode: "replace" })); expect(res.status, JSON.stringify(res.body)).toBe(200); expect(mockAgentService.update).toHaveBeenCalledWith( @@ -615,12 +615,102 @@ describe.sequential("agent skill routes", () => { ); }); + it("requires an explicit actionable merge mode for skill sync", async () => { + const res = await requestApp(await createApp(), (baseUrl) => request(baseUrl) + .post("/api/agents/11111111-1111-4111-8111-111111111111/skills/sync?companyId=company-1") + .send({ desiredSkills: ["paperclip"] })); + + expect(res.status, JSON.stringify(res.body)).toBe(422); + expect(res.body.error).toContain('"add", "remove", or "replace"'); + expect(res.body.error).toContain('"replace" only to overwrite'); + expect(mockAgentService.update).not.toHaveBeenCalled(); + }); + + it("adds only named desired skills while preserving existing assignments", async () => { + mockAgentService.getById.mockResolvedValue({ + ...makeAgent("claude_local"), + adapterConfig: { + paperclipSkillSync: { desiredSkills: ["company-1/keep"] }, + }, + }); + + const res = await requestApp(await createApp(), (baseUrl) => request(baseUrl) + .post("/api/agents/11111111-1111-4111-8111-111111111111/skills/sync?companyId=company-1") + .send({ desiredSkills: ["paperclip"], mode: "add" })); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(mockAgentService.update).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + adapterConfig: expect.objectContaining({ + paperclipSkillSync: { + desiredSkills: ["company-1/keep", "paperclipai/paperclip/paperclip"], + }, + }), + }), + expect.any(Object), + ); + }); + + it("removes only named desired skills while preserving other assignments", async () => { + mockAgentService.getById.mockResolvedValue({ + ...makeAgent("claude_local"), + adapterConfig: { + paperclipSkillSync: { + desiredSkills: ["company-1/keep", "paperclipai/paperclip/paperclip"], + }, + }, + }); + + const res = await requestApp(await createApp(), (baseUrl) => request(baseUrl) + .post("/api/agents/11111111-1111-4111-8111-111111111111/skills/sync?companyId=company-1") + .send({ desiredSkills: ["paperclip"], mode: "remove" })); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(mockAgentService.update).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + adapterConfig: expect.objectContaining({ + paperclipSkillSync: { desiredSkills: ["company-1/keep"] }, + }), + }), + expect.any(Object), + ); + }); + + it("replaces the complete desired skill set only when explicitly requested", async () => { + mockAgentService.getById.mockResolvedValue({ + ...makeAgent("claude_local"), + adapterConfig: { + paperclipSkillSync: { desiredSkills: ["company-1/keep"] }, + }, + }); + + const res = await requestApp(await createApp(), (baseUrl) => request(baseUrl) + .post("/api/agents/11111111-1111-4111-8111-111111111111/skills/sync?companyId=company-1") + .send({ desiredSkills: ["paperclip"], mode: "replace" })); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(mockAgentService.update).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + adapterConfig: expect.objectContaining({ + paperclipSkillSync: { + desiredSkills: ["paperclipai/paperclip/paperclip"], + }, + }), + }), + expect.any(Object), + ); + }); + it("rejects version pins while beta skills are disabled", async () => { mockAgentService.getById.mockResolvedValue(makeAgent("claude_local")); const res = await requestApp(await createApp(), (baseUrl) => request(baseUrl) .post("/api/agents/11111111-1111-4111-8111-111111111111/skills/sync?companyId=company-1") .send({ + mode: "replace", desiredSkills: [{ key: "paperclipai/paperclip/paperclip", versionId: "22222222-2222-4222-8222-222222222222", @@ -640,6 +730,7 @@ describe.sequential("agent skill routes", () => { const res = await requestApp(await createApp(), (baseUrl) => request(baseUrl) .post("/api/agents/11111111-1111-4111-8111-111111111111/skills/sync?companyId=company-1") .send({ + mode: "replace", desiredSkills: [{ key: "paperclipai/paperclip/paperclip", versionId }], })); @@ -687,7 +778,7 @@ describe.sequential("agent skill routes", () => { const res = await requestApp(await createApp(), (baseUrl) => request(baseUrl) .post("/api/agents/11111111-1111-4111-8111-111111111111/skills/sync?companyId=company-1") - .send({ desiredSkills: ["paperclip", "stale/removed/skill"] })); + .send({ desiredSkills: ["paperclip", "stale/removed/skill"], mode: "replace" })); expect(res.status, JSON.stringify(res.body)).toBe(200); // Stale key preserved in the persisted config alongside the resolved skill. @@ -741,7 +832,7 @@ describe.sequential("agent skill routes", () => { const res = await requestApp(await createApp(), (baseUrl) => request(baseUrl) .post("/api/agents/11111111-1111-4111-8111-111111111111/skills/sync?companyId=company-1") - .send({ desiredSkills: ["paperclipai/paperclip/paperclip"] })); + .send({ desiredSkills: ["paperclipai/paperclip/paperclip"], mode: "replace" })); expect(res.status, JSON.stringify(res.body)).toBe(200); expect(mockAdapter.syncSkills).toHaveBeenCalled(); @@ -796,7 +887,7 @@ describe.sequential("agent skill routes", () => { const res = await requestApp(await createApp(), (baseUrl) => request(baseUrl) .post("/api/agents/11111111-1111-4111-8111-111111111111/skills/sync?companyId=company-1") - .send({ desiredSkills: ["paperclipai/paperclip/paperclip"] })); + .send({ desiredSkills: ["paperclipai/paperclip/paperclip"], mode: "replace" })); expect(res.status, JSON.stringify(res.body)).toBe(200); expect(mockAdapter.syncSkills).toHaveBeenCalledWith( @@ -816,7 +907,7 @@ describe.sequential("agent skill routes", () => { const res = await requestApp(await createApp(), (baseUrl) => request(baseUrl) .post("/api/agents/11111111-1111-4111-8111-111111111111/skills/sync?companyId=company-1") - .send({ desiredSkills: ["paperclip"] })); + .send({ desiredSkills: ["paperclip"], mode: "replace" })); expect(res.status, JSON.stringify(res.body)).toBe(200); expect(mockAgentService.update).toHaveBeenCalledWith( diff --git a/server/src/__tests__/company-portability.test.ts b/server/src/__tests__/company-portability.test.ts index 63c3a149438..9631ddd3b7b 100644 --- a/server/src/__tests__/company-portability.test.ts +++ b/server/src/__tests__/company-portability.test.ts @@ -3005,6 +3005,18 @@ describe("company portability", () => { id: "agent-created", name: "ClaudeCoder", }); + companySkillSvc.importPackageFiles.mockResolvedValueOnce([{ + skill: { + id: "skill-imported", + key: paperclipKey, + slug: "paperclip", + }, + action: "renamed", + originalKey: "paperclip", + originalSlug: "paperclip", + requestedRefs: ["paperclip"], + reason: "Existing skill matched; renamed to paperclip-2.", + }]); const exported = await portability.exportBundle("company-1", { include: { @@ -3017,7 +3029,7 @@ describe("company portability", () => { agentSvc.list.mockResolvedValue([]); - await portability.importBundle({ + const result = await portability.importBundle({ source: { type: "inline", rootPath: exported.rootPath, @@ -3039,8 +3051,17 @@ describe("company portability", () => { const textOnlyFiles = Object.fromEntries(Object.entries(exported.files).filter(([, v]) => typeof v === "string")); expect(companySkillSvc.importPackageFiles).toHaveBeenCalledWith("company-imported", textOnlyFiles, { - onConflict: "replace", + onConflict: "rename", }); + expect(result.skills).toEqual([{ + originalKey: "paperclip", + originalSlug: "paperclip", + key: paperclipKey, + slug: "paperclip", + id: "skill-imported", + action: "renamed", + reason: "Existing skill matched; renamed to paperclip-2.", + }]); expect(agentSvc.create).toHaveBeenCalledWith("company-imported", expect.objectContaining({ adapterConfig: expect.objectContaining({ paperclipSkillSync: { @@ -3301,7 +3322,7 @@ describe("company portability", () => { "agents/cmo/AGENTS.md": expect.any(String), }), { - onConflict: "replace", + onConflict: "rename", }, ); expect(companySkillSvc.importPackageFiles).toHaveBeenCalledWith( @@ -3310,7 +3331,7 @@ describe("company portability", () => { "agents/claudecoder/AGENTS.md": expect.any(String), }), { - onConflict: "replace", + onConflict: "rename", }, ); expect(agentSvc.create).toHaveBeenCalledTimes(1); diff --git a/server/src/__tests__/company-skills-service.test.ts b/server/src/__tests__/company-skills-service.test.ts index c2c3541bfcc..1de80114abb 100644 --- a/server/src/__tests__/company-skills-service.test.ts +++ b/server/src/__tests__/company-skills-service.test.ts @@ -1605,6 +1605,72 @@ describeEmbeddedPostgres("companySkillService.list", () => { ]); }); + it("defaults package conflicts to skip and reports skip, rename, and explicit replace outcomes", 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 original = await svc.createLocalSkill(companyId, { + name: "Conflict Skill", + slug: "conflict-skill", + markdown: "---\nname: Conflict Skill\n---\n\n# Original\n", + }); + const packageFiles = { + "skills/conflict-skill/SKILL.md": [ + "---", + "name: Imported Conflict Skill", + "slug: conflict-skill", + "description: Incoming package version", + "---", + "", + "# Imported", + "", + ].join("\n"), + }; + + const skipped = await svc.importPackageFiles(companyId, packageFiles); + expect(skipped).toEqual([ + expect.objectContaining({ + action: "skipped", + originalSlug: "conflict-skill", + skill: expect.objectContaining({ id: original.id, name: "Conflict Skill" }), + }), + ]); + await expect(svc.getById(companyId, original.id)).resolves.toMatchObject({ + name: "Conflict Skill", + markdown: expect.stringContaining("# Original"), + }); + + const renamed = await svc.importPackageFiles(companyId, packageFiles, { onConflict: "rename" }); + expect(renamed).toEqual([ + expect.objectContaining({ + action: "renamed", + originalSlug: "conflict-skill", + skill: expect.objectContaining({ + name: "Imported Conflict Skill", + slug: "conflict-skill-2", + }), + }), + ]); + expect((await svc.list(companyId)).filter((skill) => skill.slug.startsWith("conflict-skill"))).toHaveLength(2); + + const replaced = await svc.importPackageFiles(companyId, packageFiles, { onConflict: "replace" }); + expect(replaced).toEqual([ + expect.objectContaining({ + action: "replaced", + originalSlug: "conflict-skill", + skill: expect.objectContaining({ id: original.id, name: "Imported Conflict Skill" }), + }), + ]); + await expect(svc.getById(companyId, original.id)).resolves.toMatchObject({ + name: "Imported Conflict Skill", + markdown: expect.stringContaining("# Imported"), + }); + }); + it("rejects executable external package skills before persistence", async () => { const companyId = randomUUID(); await db.insert(companies).values({ diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index f8d991895f0..97754e352d8 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -1,4 +1,4 @@ -import { Router, type Request, type Response } from "express"; +import { Router, type NextFunction, type Request, type Response } from "express"; import { generateKeyPairSync, randomUUID } from "node:crypto"; import path from "node:path"; import type { Db } from "@paperclipai/db"; @@ -18,6 +18,7 @@ import { resetAgentSessionSchema, testAdapterEnvironmentSchema, type AgentDesiredSkillEntry, + type AgentSkillAssignmentMode, type AgentSkillSnapshot, type InstanceSchedulerHeartbeatAgent, upsertAgentInstructionsFileSchema, @@ -117,6 +118,35 @@ import { touchesAgentProfileChangeConsentFields, } from "../services/change-consent-gate.js"; +const AGENT_SKILL_ASSIGNMENT_MODES = ["add", "remove", "replace"] as const; + +function requireAgentSkillAssignmentMode(req: Request, _res: Response, next: NextFunction) { + if (!AGENT_SKILL_ASSIGNMENT_MODES.includes(req.body?.mode)) { + throw unprocessable( + 'Skill sync requires mode: "add", "remove", or "replace". ' + + 'Use "replace" only to overwrite the complete desired skill set.', + ); + } + next(); +} + +function mergeDesiredSkillEntries( + current: AgentDesiredSkillEntry[], + requested: AgentDesiredSkillEntry[], + mode: AgentSkillAssignmentMode, +) { + if (mode === "replace") return requested; + + const requestedKeys = new Set(requested.map((entry) => entry.key)); + if (mode === "remove") { + return current.filter((entry) => !requestedKeys.has(entry.key)); + } + + const merged = new Map(current.map((entry) => [entry.key, entry])); + for (const entry of requested) merged.set(entry.key, entry); + return Array.from(merged.values()); +} + const RUN_LOG_DEFAULT_LIMIT_BYTES = 256_000; const RUN_LOG_MAX_LIMIT_BYTES = 1024 * 1024; @@ -1625,6 +1655,7 @@ export function agentRoutes( adapterType: string, adapterConfig: Record, requestedDesiredSkills: AgentDesiredSkillEntry[] | undefined, + mode: AgentSkillAssignmentMode, options: { tolerateUnknownDesiredSkills?: boolean } = {}, ) { if (!requestedDesiredSkills) { @@ -1647,22 +1678,44 @@ export function agentRoutes( await companySkills.resolveRequestedSkillEntries(companyId, requestedDesiredSkills, { tolerateUnknownReferences: options.tolerateUnknownDesiredSkills, }); - // Runtime materialization + version selection only ever consider skills that - // actually resolve to the company library; stale keys can't be materialized. - const runtimeSkillEntries = await companySkills.listRuntimeSkillEntries(companyId, { - materializeMissing: shouldMaterializeRuntimeSkillsForAdapter(adapterType), - versionSelections: skillVersionSelectionMap(resolvedRequestedSkillEntries), - }); - const resolvedDesiredSkillEntries = resolvedRequestedSkillEntries.filter( + const requestedSkillEntries = [ + ...resolvedRequestedSkillEntries, + ...unresolvedDesiredSkillKeys.map((key) => ({ key, versionId: null })), + ].filter( (entry, index, entries) => entries.findIndex((candidate) => candidate.key === entry.key) === index, ); - // Preserve stale/unresolvable keys in the persisted desired set so they stay - // visible (and explicitly removable) instead of vanishing on the next save. - const desiredSkillEntries: AgentDesiredSkillEntry[] = [ - ...resolvedDesiredSkillEntries, - ...unresolvedDesiredSkillKeys.map((key) => ({ key, versionId: null })), - ]; + + const currentPreference = readPaperclipSkillSyncPreference(adapterConfig); + const { resolved: resolvedCurrentSkillEntries, unresolved: unresolvedCurrentSkillKeys } = + currentPreference.desiredSkillEntries.length > 0 + ? await companySkills.resolveRequestedSkillEntries( + companyId, + currentPreference.desiredSkillEntries, + { tolerateUnknownReferences: true }, + ) + : { resolved: [], unresolved: [] }; + const currentSkillEntries = [ + ...resolvedCurrentSkillEntries, + ...unresolvedCurrentSkillKeys.map((key) => ({ key, versionId: null })), + ].filter( + (entry, index, entries) => entries.findIndex((candidate) => candidate.key === entry.key) === index, + ); + + const desiredSkillEntries = mergeDesiredSkillEntries(currentSkillEntries, requestedSkillEntries, mode); const desiredSkills = desiredSkillEntries.map((entry) => entry.key); + const resolvedKeys = new Set([ + ...resolvedCurrentSkillEntries.map((entry) => entry.key), + ...resolvedRequestedSkillEntries.map((entry) => entry.key), + ]); + // Runtime materialization + version selection only ever consider final + // assignments that resolve to the company library; stale keys remain + // persisted and explicitly removable without reaching adapter runtimes. + const runtimeSkillEntries = await companySkills.listRuntimeSkillEntries(companyId, { + materializeMissing: shouldMaterializeRuntimeSkillsForAdapter(adapterType), + versionSelections: skillVersionSelectionMap( + desiredSkillEntries.filter((entry) => resolvedKeys.has(entry.key)), + ), + }); return { adapterConfig: writePaperclipSkillSyncPreference(adapterConfig, desiredSkillEntries), @@ -1992,6 +2045,7 @@ export function agentRoutes( router.post( "/agents/:id/skills/sync", + requireAgentSkillAssignmentMode, validate(agentSkillSyncSchema), async (req, res) => { const id = req.params.id as string; @@ -2010,6 +2064,7 @@ export function agentRoutes( agent.adapterType, agent.adapterConfig as Record, requestedSkills, + req.body.mode, // Toggling a resolvable skill must not fail just because the agent // already carries stale desired keys (e.g. a skill removed from the // library). Preserve those keys so they remain visible/removable. @@ -2074,6 +2129,7 @@ export function agentRoutes( adapterType: updated.adapterType, desiredSkills, desiredSkillEntries, + assignmentMode: req.body.mode, mode: snapshot.mode, supported: snapshot.supported, entryCount: snapshot.entries.length, @@ -2494,6 +2550,7 @@ export function agentRoutes( hireInput.adapterType, requestedAdapterConfig, normalizeDesiredSkillSelections(Array.isArray(requestedDesiredSkills) ? requestedDesiredSkills : undefined), + "add", ); const normalizedAdapterConfig = await normalizeMediatedAdapterConfigForPersistence({ companyId, @@ -2689,6 +2746,7 @@ export function agentRoutes( createInput.adapterType, requestedAdapterConfig, normalizeDesiredSkillSelections(Array.isArray(requestedDesiredSkills) ? requestedDesiredSkills : undefined), + "add", ); const normalizedAdapterConfig = await normalizeMediatedAdapterConfigForPersistence({ companyId, diff --git a/server/src/routes/openapi.ts b/server/src/routes/openapi.ts index a94702fb767..e11a2f4edff 100644 --- a/server/src/routes/openapi.ts +++ b/server/src/routes/openapi.ts @@ -1975,7 +1975,7 @@ registry.registerPath({ params: z.object({ id: z.string() }), body: jsonBody(agentSkillSyncSchema), }, - responses: { 200: r.ok(), 400: r.badRequest, 401: r.unauthorized, 404: r.notFound }, + responses: { 200: r.ok(), 400: r.badRequest, 401: r.unauthorized, 404: r.notFound, 422: r.unprocessable }, }); registry.registerPath({ diff --git a/server/src/services/company-portability.ts b/server/src/services/company-portability.ts index 8818598ec4a..cdf4cb56303 100644 --- a/server/src/services/company-portability.ts +++ b/server/src/services/company-portability.ts @@ -203,7 +203,7 @@ function assertInlineSourceComplete(source: CompanyPortabilityImport["source"]) } function resolveSkillConflictStrategy(mode: ImportMode, collisionStrategy: CompanyPortabilityCollisionStrategy) { - if (mode === "board_full") return "replace" as const; + if (mode === "board_full") return collisionStrategy; return collisionStrategy === "skip" ? "skip" as const : "rename" as const; } @@ -5329,7 +5329,7 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { desiredSkillRefMap.set(importedSkill.originalSlug, importedSkill.skill.key); if (importedSkill.action === "skipped") { warnings.push(`Skipped skill ${importedSkill.originalSlug}; existing skill ${importedSkill.skill.slug} was kept.`); - } else if (importedSkill.originalKey !== importedSkill.skill.key) { + } else if (importedSkill.action === "renamed") { warnings.push(`Imported skill ${importedSkill.originalSlug} as ${importedSkill.skill.slug} to avoid overwriting an existing skill.`); } } @@ -6148,6 +6148,15 @@ export function companyPortabilityService(db: Db, storage?: StorageService) { action: companyAction, }, agents: resultAgents, + skills: importedSkills.map((result) => ({ + originalKey: result.originalKey, + originalSlug: result.originalSlug, + key: result.skill.key, + slug: result.skill.slug, + id: result.skill.id, + action: result.action, + reason: result.reason, + })), projects: resultProjects, routines: resultRoutines, envInputs: sourceManifest.envInputs ?? [], diff --git a/server/src/services/company-skills.ts b/server/src/services/company-skills.ts index 4776fadfa24..22348d87647 100644 --- a/server/src/services/company-skills.ts +++ b/server/src/services/company-skills.ts @@ -257,7 +257,7 @@ type PackageSkillConflictStrategy = "replace" | "rename" | "skip"; export type ImportPackageSkillResult = { skill: CompanySkill; - action: "created" | "updated" | "skipped"; + action: "created" | "renamed" | "replaced" | "skipped"; originalKey: string; originalSlug: string; requestedRefs: string[]; @@ -5826,6 +5826,14 @@ export function companySkillService(db: Db) { const importedSkills = readInlineSkillImports(companyId, normalizedFiles); if (importedSkills.length === 0) return []; + // Conflict handling must never bypass source and reserved-key checks. In + // particular, the safe default "skip" path still validates the incoming + // package before returning the existing skill. + for (const skill of importedSkills) { + assertImportedSkillKeyAllowed(skill); + assertImportedSkillSourceAllowed(skill); + } + for (const skill of importedSkills) { if (skill.sourceType !== "catalog") continue; const materializedDir = await materializeCatalogSkillFiles(companyId, skill, normalizedFiles); @@ -5834,7 +5842,7 @@ export function companySkillService(db: Db) { } } - const conflictStrategy = options?.onConflict ?? "replace"; + const conflictStrategy = options?.onConflict ?? "skip"; const existingSkills = await listFull(companyId); const existingByKey = new Map(existingSkills.map((skill) => [skill.key, skill])); const existingBySlug = new Map( @@ -5848,8 +5856,7 @@ export function companySkillService(db: Db) { skill: ImportedSkill; originalKey: string; originalSlug: string; - existingBefore: CompanySkill | null; - actionHint: "created" | "updated"; + actionHint: "created" | "renamed" | "replaced"; reason: string | null; }> = []; const out: ImportPackageSkillResult[] = []; @@ -5863,17 +5870,29 @@ export function companySkillService(db: Db) { const conflict = existingByIncomingKey ?? existingByIncomingSlug; if (!conflict || conflictStrategy === "replace") { - toPersist.push(importedSkill); + const skillToPersist = conflict && conflict.key !== importedSkill.key + ? { + ...importedSkill, + key: conflict.key, + slug: conflict.slug, + metadata: { + ...(importedSkill.metadata ?? {}), + skillKey: conflict.key, + importedFromSkillKey: originalKey, + importedFromSkillSlug: originalSlug, + }, + } + : importedSkill; + toPersist.push(skillToPersist); prepared.push({ - skill: importedSkill, + skill: skillToPersist, originalKey, originalSlug, - existingBefore: existingByIncomingKey, - actionHint: existingByIncomingKey ? "updated" : "created", - reason: existingByIncomingKey ? "Existing skill key matched; replace strategy." : null, + actionHint: conflict ? "replaced" : "created", + reason: conflict ? "Existing skill matched; explicit replace strategy." : null, }); - usedSlugs.add(normalizedSlug); - usedKeys.add(importedSkill.key); + usedSlugs.add(normalizeSkillSlug(skillToPersist.slug) ?? skillToPersist.slug); + usedKeys.add(skillToPersist.key); continue; } @@ -5907,8 +5926,7 @@ export function companySkillService(db: Db) { skill: renamedSkill, originalKey, originalSlug, - existingBefore: null, - actionHint: "created", + actionHint: "renamed", reason: `Existing skill matched; renamed to ${renamedSlug}.`, }); usedSlugs.add(renamedSlug); diff --git a/ui/src/api/agents.ts b/ui/src/api/agents.ts index 45e134c3cd1..6b42b1f581f 100644 --- a/ui/src/api/agents.ts +++ b/ui/src/api/agents.ts @@ -1,6 +1,7 @@ import type { Agent, AgentDesiredSkillEntry, + AgentSkillAssignmentMode, AgentPermissions, AgentDetail, AgentInstructionsBundle, @@ -178,8 +179,12 @@ export const agentsApi = { listKeys: (id: string, companyId?: string) => api.get(agentPath(id, companyId, "/keys")), skills: (id: string, companyId?: string) => api.get(agentPath(id, companyId, "/skills")), - syncSkills: (id: string, desiredSkills: Array, companyId?: string) => - api.post(agentPath(id, companyId, "/skills/sync"), { desiredSkills }), + syncSkills: ( + id: string, + desiredSkills: Array, + mode: AgentSkillAssignmentMode, + companyId?: string, + ) => api.post(agentPath(id, companyId, "/skills/sync"), { desiredSkills, mode }), createKey: (id: string, name: string, companyId?: string, scope?: AgentApiKeyScope) => api.post(agentPath(id, companyId, "/keys"), { name, ...(scope ? { scope } : {}) }), revokeKey: (agentId: string, keyId: string, companyId?: string) => diff --git a/ui/src/components/skill-studio/AgentsUsingSkillDialog.tsx b/ui/src/components/skill-studio/AgentsUsingSkillDialog.tsx index aa88f29119b..439e0ce92c1 100644 --- a/ui/src/components/skill-studio/AgentsUsingSkillDialog.tsx +++ b/ui/src/components/skill-studio/AgentsUsingSkillDialog.tsx @@ -189,7 +189,7 @@ export function AgentsUsingSkillDialog({ if (desiredSetsEqual(currentEntries, nextEntries)) { return { agentId: agent.id, changed: false }; } - await agentsApi.syncSkills(agent.id, nextEntries, companyId); + await agentsApi.syncSkills(agent.id, nextEntries, "replace", companyId); return { agentId: agent.id, changed: true }; }, onSuccess: async ({ agentId }) => { diff --git a/ui/src/pages/CompanyImport.test.tsx b/ui/src/pages/CompanyImport.test.tsx index ac531c8ab51..dd2fefcb97b 100644 --- a/ui/src/pages/CompanyImport.test.tsx +++ b/ui/src/pages/CompanyImport.test.tsx @@ -160,6 +160,7 @@ function buildImportResult(): CompanyPortabilityImportResult { return { company: { id: "company-2", name: "Imported Test", action: "created" }, agents: [{ slug: "coder", id: "agent-1", action: "created", name: "Coder", reason: null }], + skills: [], projects: [], routines: [{ slug: "weekly-report", id: "routine-1", action: "created", title: "Weekly Report", status: "paused" }], envInputs: [], diff --git a/ui/src/pages/CompanyImport.tsx b/ui/src/pages/CompanyImport.tsx index 5aecd4a4374..3339867d6de 100644 --- a/ui/src/pages/CompanyImport.tsx +++ b/ui/src/pages/CompanyImport.tsx @@ -1455,6 +1455,7 @@ export function CompanyImport() { if (importOutcome) { const { result, dashboardPath } = importOutcome; + const skillResults = result.skills ?? []; const activationItems = importOutcome.pausedAutomations ? buildActivationItems(result) : []; const pendingCount = activationItems.filter( (item) => activationChecked.has(item.key) && !activatedKeys.has(item.key), @@ -1465,11 +1466,31 @@ export function CompanyImport() {

Import complete

{result.company.name}: {result.agents.length} agent{result.agents.length === 1 ? "" : "s"},{" "} + {skillResults.length} skill{skillResults.length === 1 ? "" : "s"},{" "} {result.projects.length} project{result.projects.length === 1 ? "" : "s"}, and{" "} {result.routines.length} routine{result.routines.length === 1 ? "" : "s"} processed.

+ {skillResults.length > 0 && ( +
+
+

Skill import results

+
+
+ {skillResults.map((skill) => ( +
+ {skill.originalSlug} + {skill.action} + {skill.slug !== skill.originalSlug && ( + as {skill.slug} + )} +
+ ))} +
+
+ )} + {result.warnings.length > 0 && (
{result.warnings.map((w) => ( diff --git a/ui/src/pages/CompanySkills.tsx b/ui/src/pages/CompanySkills.tsx index be8a9f41ad8..2bb0859f07f 100644 --- a/ui/src/pages/CompanySkills.tsx +++ b/ui/src/pages/CompanySkills.tsx @@ -4850,7 +4850,7 @@ export function CompanySkills() { const attachAgentsMutation = useMutation({ mutationFn: async (input: { agentId: string; desiredSkills: Array }) => { - return agentsApi.syncSkills(input.agentId, input.desiredSkills, selectedCompanyId ?? undefined); + return agentsApi.syncSkills(input.agentId, input.desiredSkills, "replace", selectedCompanyId ?? undefined); }, onSuccess: async () => { await Promise.all([ diff --git a/ui/src/pages/agent-skills/AgentSkillsTab.tsx b/ui/src/pages/agent-skills/AgentSkillsTab.tsx index 44adb974373..4ec4d2b82ee 100644 --- a/ui/src/pages/agent-skills/AgentSkillsTab.tsx +++ b/ui/src/pages/agent-skills/AgentSkillsTab.tsx @@ -112,7 +112,7 @@ export function AgentSkillsTab({ agent, companyId }: { agent: Agent; companyId?: const syncSkills = useMutation({ mutationFn: (desiredSkills: Array) => - agentsApi.syncSkills(agent.id, desiredSkills, companyId), + agentsApi.syncSkills(agent.id, desiredSkills, "replace", companyId), onSuccess: async (snapshot) => { queryClient.setQueryData(queryKeys.agents.skills(agent.id), snapshot); lastSavedSkillsRef.current = snapshot.desiredSkills; diff --git a/ui/storybook/stories/team-catalog.stories.tsx b/ui/storybook/stories/team-catalog.stories.tsx index c30ee006cf4..7fd0d1e4c0d 100644 --- a/ui/storybook/stories/team-catalog.stories.tsx +++ b/ui/storybook/stories/team-catalog.stories.tsx @@ -352,6 +352,7 @@ export const InstallSuccess: Story = { { slug: "cto", id: "a2", action: "created", name: "CTO", reason: null }, { slug: "cmo", id: "a3", action: "created", name: "CMO (from Core Exec Team)", reason: null }, ], + skills: [], projects: [{ slug: "launch", id: "p1", action: "created", name: "Launch", reason: null }], routines: [], envInputs: [], From 2cfbb3e4c54c154ec65c815dc0a62062c57619cf Mon Sep 17 00:00:00 2001 From: Dotta Date: Thu, 6 Aug 2026 14:35:46 +0000 Subject: [PATCH 2/2] docs(skills): require explicit assignment modes Update shipped and operational skill-sync examples for the required merge-mode contract. Co-Authored-By: Paperclip --- .../bundled/paperclip-operations/reflection-coach/SKILL.md | 2 +- skills/paperclip/SKILL.md | 2 +- skills/paperclip/references/company-skills.md | 7 +++++++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/skills-catalog/catalog/bundled/paperclip-operations/reflection-coach/SKILL.md b/packages/skills-catalog/catalog/bundled/paperclip-operations/reflection-coach/SKILL.md index b5167838920..205a12494e4 100644 --- a/packages/skills-catalog/catalog/bundled/paperclip-operations/reflection-coach/SKILL.md +++ b/packages/skills-catalog/catalog/bundled/paperclip-operations/reflection-coach/SKILL.md @@ -176,7 +176,7 @@ Server-enforced mutation target keys: When the interaction resolves **accepted**, apply the change in a *separate* run: - **AGENTS.md** — update the target's managed instruction file exactly as the accepted diff specified. -- **Skill** — install/update the skill in the company library, then `POST /api/agents//skills/sync` when the target should receive it. +- **Skill** — install/update the skill in the company library, then `POST /api/agents//skills/sync` with `{"mode":"add","desiredSkills":[""]}` when the target should receive it. Use `remove` only for the named assignments. Use `replace` only after explicit confirmation to overwrite the complete desired skill set. - **Tool description** — update the target agent's description/profile field that the accepted diff named. The server rejects Reflection Coach mutations unless the accepted `request_confirmation` was created by Reflection Coach in a previous run, has a displayed diff, and is bound to the resource by one of the target keys above. If the interaction was rejected or is still pending, apply nothing. If you were asked to apply without a reviewed diff and an accepted interaction, refuse and name the gate — no-same-run-apply is load-bearing. diff --git a/skills/paperclip/SKILL.md b/skills/paperclip/SKILL.md index 981afa05f26..728395f5bac 100644 --- a/skills/paperclip/SKILL.md +++ b/skills/paperclip/SKILL.md @@ -421,7 +421,7 @@ or linking cases through the agent-facing cases API. Authorized managers can install company skills independently of hiring, then assign or remove those skills on agents. - Install and inspect company skills with the company skills API. -- Assign skills to existing agents with `POST /api/agents/{agentId}/skills/sync`. +- Assign skills to existing agents with `POST /api/agents/{agentId}/skills/sync` and an explicit `add`, `remove`, or `replace` mode. Prefer `add`; `replace` overwrites the complete desired skill set. - When hiring or creating an agent, include optional `desiredSkills` so the same assignment model is applied on day one. If you are asked to install a skill for the company or an agent you MUST read: diff --git a/skills/paperclip/references/company-skills.md b/skills/paperclip/references/company-skills.md index 37ba590ab8f..696653cb9d1 100644 --- a/skills/paperclip/references/company-skills.md +++ b/skills/paperclip/references/company-skills.md @@ -188,11 +188,18 @@ curl -sS "$PAPERCLIP_API_URL/api/companies/$PAPERCLIP_COMPANY_ID/skills//skills/sync" \ -H "Authorization: Bearer $PAPERCLIP_API_KEY" \ -H "Content-Type: application/json" \ -d '{ + "mode": "add", "desiredSkills": [ "vercel-labs/agent-browser/agent-browser" ]