mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-29 21:07:40 +01:00
feat(skills): require explicit merge modes (#10978)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Agents can select company skills and synchronize them to adapter runtimes > - The skill sync API replaced the complete selection without an explicit destructive choice > - Company package import also replaced conflicting skills by default > - These defaults could remove operator edits during setup and import reruns > - This pull request adds explicit assignment merge modes and safe package conflict handling > - The benefit is that reruns preserve operator work unless the caller explicitly requests replacement ## Linked Issues or Issue Description **What existing behavior does this improve?** This change improves agent skill synchronization and company package import. **Subsystem affected** This is a cross-cutting change across the shared contracts, server, CLI, and UI. **Current behavior** Agent skill synchronization replaces the full desired skill set from a modeless request. Package import replaces a conflicting skill when the caller does not select a conflict mode. **Proposed behavior** Agent skill synchronization requires `add`, `remove`, or `replace`. Package import skips conflicts by default. Each imported skill reports whether it was created, renamed, replaced, or skipped. **Reason and benefit** Setup and import reruns must preserve operator edits by default. Explicit destructive modes make data loss less likely and make each outcome inspectable. **Breaking changes** Callers of the agent skill sync API must now send `mode`. Callers that need the former behavior must send `replace`. Package import now uses `skip` when `onConflict` is absent. ## What Changed - Added required `add`, `remove`, and `replace` modes to the shared agent skill sync contract. - Added actionable `422` validation for missing or invalid modes. - Updated first-party UI and CLI callers with explicit modes. - Changed package skill conflict handling to use `skip` by default. - Kept plugin-owned and built-in stock skill imports on explicit `replace`. - Added created, renamed, replaced, and skipped results to company imports. - Added regression coverage for merge modes and package conflict outcomes. ## Verification - `pnpm check:token-gates` - `pnpm -r typecheck` - `pnpm build` - `pnpm test:run:serialized` (128 suites passed) - `pnpm --filter @paperclipai/skills-catalog test` (20 tests passed) - Focused agent skill route, company skill service, portability, CLI, and UI tests passed. - GitHub CI passed build, typecheck, canary, all general and serialized test shards, all browser shards, policy, security, and final verification on commit `2cfbb3e4c5`. - Greptile reviewed the latest commit at 5/5 with zero unresolved threads. ## Risks - This change intentionally rejects modeless agent skill sync requests. - The safe package default can leave an existing skill unchanged where the old default overwrote it. - All first-party callers now select a mode. Regression tests cover each outcome. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI `gpt-5.6-sol` through Codex. The runtime used agentic reasoning, tool use, code execution, and repository editing. The runtime did not expose the context window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
f6c6452b25
commit
5da382fd59
31 files changed
+459
-75
No files matched your search
@@ -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" })]);
|
||||
|
||||
@@ -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");
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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 });
|
||||
|
||||
Reference in new issue
Block a user