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 <noreply@paperclip.ing>
This commit is contained in:
DottaandPaperclip committed 2026-08-06 14:29:31 +00:00
1 parent 2ea22d6cea
commit b616d887f6
28 files changed
+450 -73

No files matched your search

+9 -1
View File
@@ -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" })]);
+13
View File
@@ -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");
+3 -1
View File
@@ -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);
+9 -1
View File
@@ -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("<agentId>", "Agent ID")
.requiredOption("--desired-skills <csv>", "Desired skill names")
.requiredOption(
"--mode <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) {
+21
View File
@@ -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",
+25 -17
View File
@@ -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<CompanySkillListItem, "id" | "key" | "slug" | "name">;
@@ -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("<agentRef>", "Agent ID or shortname/url-key")
.option("--skill <skillRef>", "Desired company skill ID, key, or slug; may be repeated", collectOptionValue, [] as string[])
.requiredOption(
"--mode <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<AgentSkillSnapshot>(
`/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<AgentSkillSnapshot>(
`/api/agents/${encodeURIComponent(agentRow.id)}/skills/sync`,
{ desiredSkills: [] },
{ desiredSkills: [], mode: "replace" },
);
if (ctx.json) {
printOutput(snapshot, { json: true });
+10 -7
View File
@@ -365,7 +365,7 @@ pnpm paperclipai agent runtime-state <agent-id>
pnpm paperclipai agent runtime-state:reset-session <agent-id> [--task-key <key>]
pnpm paperclipai agent task-sessions <agent-id>
pnpm paperclipai agent skills <agent-id>
pnpm paperclipai agent skills:sync <agent-id> --desired-skills paperclip,github
pnpm paperclipai agent skills:sync <agent-id> --desired-skills paperclip,github --mode add
pnpm paperclipai agent instructions-path:update <agent-id> --payload-json '{"path":"/path/to/AGENTS.md"}'
pnpm paperclipai agent instructions-bundle <agent-id>
pnpm paperclipai agent instructions-bundle:update <agent-id> --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 <agent-id-or-shortname> --company-id <company-id>
pnpm paperclipai skills agent sync <agent-id-or-shortname> --skill <skill-id-or-key-or-slug> [--skill <skill-id-or-key-or-slug>...] --company-id <company-id>
pnpm paperclipai skills agent sync <agent-id-or-shortname> --skill <skill-id-or-key-or-slug> [--skill <skill-id-or-key-or-slug>...] --mode <add|remove|replace> --company-id <company-id>
pnpm paperclipai skills agent clear <agent-id-or-shortname> --yes --company-id <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.
+1 -1
View File
@@ -91,7 +91,7 @@ pnpm paperclipai skills import ./skills/my-skill --company-id <company-id>
pnpm paperclipai skills import owner/repo/path/to/skill --company-id <company-id>
# Attach desired company skills to an agent after install/import
pnpm paperclipai skills agent sync <agent-id> --skill github-pr-workflow --company-id <company-id>
pnpm paperclipai skills agent sync <agent-id> --skill github-pr-workflow --mode add --company-id <company-id>
```
## Approval Commands
+2
View File
@@ -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,
@@ -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<string | AgentDesiredSkillEntry>;
}
@@ -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;
+1
View File
@@ -250,6 +250,7 @@ export type {
} from "./teams-catalog.js";
export type {
AgentSkillSyncMode,
AgentSkillAssignmentMode,
AgentSkillState,
AgentSkillOrigin,
AgentDesiredSkillEntry,
@@ -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),
});
+1
View File
@@ -282,6 +282,7 @@ export {
export {
agentSkillStateSchema,
agentSkillSyncModeSchema,
agentSkillAssignmentModeSchema,
agentDesiredSkillEntrySchema,
agentDesiredSkillSelectionSchema,
agentSkillEntrySchema,
@@ -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(
@@ -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);
@@ -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({
+72 -14
View File
@@ -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<string, unknown>,
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<string, unknown>,
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,
+1 -1
View File
@@ -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({
+11 -2
View File
@@ -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 ?? [],
+31 -13
View File
@@ -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);
+7 -2
View File
@@ -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<AgentKey[]>(agentPath(id, companyId, "/keys")),
skills: (id: string, companyId?: string) =>
api.get<AgentSkillSnapshot>(agentPath(id, companyId, "/skills")),
syncSkills: (id: string, desiredSkills: Array<string | AgentDesiredSkillEntry>, companyId?: string) =>
api.post<AgentSkillSnapshot>(agentPath(id, companyId, "/skills/sync"), { desiredSkills }),
syncSkills: (
id: string,
desiredSkills: Array<string | AgentDesiredSkillEntry>,
mode: AgentSkillAssignmentMode,
companyId?: string,
) => api.post<AgentSkillSnapshot>(agentPath(id, companyId, "/skills/sync"), { desiredSkills, mode }),
createKey: (id: string, name: string, companyId?: string, scope?: AgentApiKeyScope) =>
api.post<AgentKeyCreated>(agentPath(id, companyId, "/keys"), { name, ...(scope ? { scope } : {}) }),
revokeKey: (agentId: string, keyId: string, companyId?: string) =>
@@ -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 }) => {
+1
View File
@@ -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: [],
+21
View File
@@ -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() {
<h2 className="text-base font-semibold">Import complete</h2>
<p className="text-xs text-muted-foreground mt-1">
{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.
</p>
</div>
{skillResults.length > 0 && (
<div className="rounded-md border border-border">
<div className="border-b border-border px-4 py-2.5">
<h3 className="text-sm font-medium">Skill import results</h3>
</div>
<div className="divide-y divide-border">
{skillResults.map((skill) => (
<div key={`${skill.originalKey}:${skill.id}`} className="flex items-center gap-3 px-4 py-2.5 text-sm">
<span className="min-w-0 flex-1 truncate">{skill.originalSlug}</span>
<span className="shrink-0 text-xs text-muted-foreground">{skill.action}</span>
{skill.slug !== skill.originalSlug && (
<span className="shrink-0 text-xs text-muted-foreground">as {skill.slug}</span>
)}
</div>
))}
</div>
</div>
)}
{result.warnings.length > 0 && (
<div className="rounded-md border border-amber-500/30 bg-amber-500/5 px-4 py-3">
{result.warnings.map((w) => (
+1 -1
View File
@@ -4850,7 +4850,7 @@ export function CompanySkills() {
const attachAgentsMutation = useMutation({
mutationFn: async (input: { agentId: string; desiredSkills: Array<string | AgentDesiredSkillEntry> }) => {
return agentsApi.syncSkills(input.agentId, input.desiredSkills, selectedCompanyId ?? undefined);
return agentsApi.syncSkills(input.agentId, input.desiredSkills, "replace", selectedCompanyId ?? undefined);
},
onSuccess: async () => {
await Promise.all([
+1 -1
View File
@@ -112,7 +112,7 @@ export function AgentSkillsTab({ agent, companyId }: { agent: Agent; companyId?:
const syncSkills = useMutation({
mutationFn: (desiredSkills: Array<string | AgentDesiredSkillEntry>) =>
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;
@@ -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: [],