mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-11 05:31:46 +02:00
## 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>
509 lines
14 KiB
TypeScript
509 lines
14 KiB
TypeScript
import { Command } from "commander";
|
|
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
|
import { registerSkillsCommands } from "../commands/client/skills.js";
|
|
import { resolveCompanySkillReference } from "../commands/client/skills.js";
|
|
|
|
const ORIGINAL_ENV = { ...process.env };
|
|
|
|
function makeProgram(): Command {
|
|
const program = new Command();
|
|
program.exitOverride();
|
|
program.configureOutput({
|
|
writeOut: () => undefined,
|
|
writeErr: () => undefined,
|
|
});
|
|
registerSkillsCommands(program);
|
|
return program;
|
|
}
|
|
|
|
async function runCommand(args: string[]): Promise<void> {
|
|
await makeProgram().parseAsync(args, { from: "user" });
|
|
}
|
|
|
|
function jsonResponse(body: unknown, status = 200): Response {
|
|
return new Response(JSON.stringify(body), {
|
|
status,
|
|
headers: { "content-type": "application/json" },
|
|
});
|
|
}
|
|
|
|
function skill(overrides: Record<string, unknown> = {}) {
|
|
return {
|
|
id: "11111111-1111-1111-1111-111111111111",
|
|
companyId: "company-1",
|
|
key: "paperclip/review-prs",
|
|
slug: "review-prs",
|
|
name: "Review PRs",
|
|
description: "Review pull requests",
|
|
markdown: "# Review PRs",
|
|
sourceType: "local_path",
|
|
sourceLocator: null,
|
|
sourceRef: null,
|
|
trustLevel: "markdown_only",
|
|
compatibility: "compatible",
|
|
fileInventory: [{ path: "SKILL.md", kind: "skill" }],
|
|
metadata: null,
|
|
createdAt: "2026-05-26T00:00:00.000Z",
|
|
updatedAt: "2026-05-26T00:00:00.000Z",
|
|
attachedAgentCount: 2,
|
|
editable: true,
|
|
editableReason: null,
|
|
sourceLabel: null,
|
|
sourceBadge: "local",
|
|
sourcePath: null,
|
|
...overrides,
|
|
};
|
|
}
|
|
|
|
function catalogSkill(overrides: Record<string, unknown> = {}) {
|
|
return {
|
|
id: "paperclipai:bundled:software-development:github-pr-workflow",
|
|
key: "paperclipai/bundled/software-development/github-pr-workflow",
|
|
kind: "bundled",
|
|
category: "software-development",
|
|
slug: "github-pr-workflow",
|
|
name: "github-pr-workflow",
|
|
description: "Prepare pull requests, review responses, and verification notes.",
|
|
path: "catalog/bundled/software-development/github-pr-workflow",
|
|
entrypoint: "SKILL.md",
|
|
trustLevel: "markdown_only",
|
|
compatibility: "compatible",
|
|
defaultInstall: false,
|
|
recommendedForRoles: ["engineer"],
|
|
requires: [],
|
|
tags: ["github", "pull-requests"],
|
|
files: [{ path: "SKILL.md", kind: "skill", sizeBytes: 128, sha256: "sha256:abc" }],
|
|
contentHash: "sha256:catalog",
|
|
...overrides,
|
|
};
|
|
}
|
|
|
|
function agent(overrides: Record<string, unknown> = {}) {
|
|
return {
|
|
id: "agent-1",
|
|
companyId: "company-1",
|
|
name: "Coder",
|
|
role: "engineer",
|
|
status: "active",
|
|
reportsTo: null,
|
|
budgetMonthlyCents: 0,
|
|
spentMonthlyCents: 0,
|
|
adapterType: "codex_local",
|
|
adapterConfig: {},
|
|
runtimeConfig: {},
|
|
permissions: {},
|
|
createdAt: "2026-05-26T00:00:00.000Z",
|
|
updatedAt: "2026-05-26T00:00:00.000Z",
|
|
...overrides,
|
|
};
|
|
}
|
|
|
|
describe("skills CLI helpers", () => {
|
|
it("resolves skill refs by id, key, or unique normalized slug", () => {
|
|
const rows = [
|
|
skill({ id: "skill-a", key: "paperclip/a", slug: "alpha", name: "Alpha" }),
|
|
skill({ id: "skill-b", key: "paperclip/b", slug: "beta-skill", name: "Beta" }),
|
|
];
|
|
|
|
expect(resolveCompanySkillReference(rows, "skill-a").key).toBe("paperclip/a");
|
|
expect(resolveCompanySkillReference(rows, "paperclip/b").id).toBe("skill-b");
|
|
expect(resolveCompanySkillReference(rows, "Beta Skill").id).toBe("skill-b");
|
|
});
|
|
|
|
it("rejects ambiguous slug refs", () => {
|
|
const rows = [
|
|
skill({ id: "skill-a", key: "paperclip/a", slug: "same", name: "A" }),
|
|
skill({ id: "skill-b", key: "paperclip/b", slug: "same", name: "B" }),
|
|
];
|
|
|
|
expect(() => resolveCompanySkillReference(rows, "same")).toThrow(/Ambiguous skill slug/);
|
|
});
|
|
});
|
|
|
|
describe("skills CLI commands", () => {
|
|
let fetchMock: ReturnType<typeof vi.fn>;
|
|
let logSpy: ReturnType<typeof vi.spyOn>;
|
|
let writeChunks: unknown[];
|
|
|
|
beforeEach(() => {
|
|
process.env = { ...ORIGINAL_ENV };
|
|
delete process.env.PAPERCLIP_API_URL;
|
|
delete process.env.PAPERCLIP_API_KEY;
|
|
delete process.env.PAPERCLIP_COMPANY_ID;
|
|
fetchMock = vi.fn();
|
|
vi.stubGlobal("fetch", fetchMock);
|
|
logSpy = vi.spyOn(console, "log").mockImplementation(() => undefined);
|
|
writeChunks = [];
|
|
vi.spyOn(process.stdout, "write").mockImplementation((chunk: string | Uint8Array) => {
|
|
writeChunks.push(chunk);
|
|
return true;
|
|
});
|
|
});
|
|
|
|
afterEach(() => {
|
|
process.env = { ...ORIGINAL_ENV };
|
|
vi.unstubAllGlobals();
|
|
vi.restoreAllMocks();
|
|
});
|
|
|
|
it("lists company skills as JSON through the shared client context", async () => {
|
|
const rows = [skill()];
|
|
fetchMock.mockResolvedValueOnce(jsonResponse(rows));
|
|
|
|
await runCommand([
|
|
"skills",
|
|
"list",
|
|
"--company-id",
|
|
"company-1",
|
|
"--api-base",
|
|
"http://paperclip.test",
|
|
"--api-key",
|
|
"token",
|
|
"--json",
|
|
]);
|
|
|
|
expect(fetchMock).toHaveBeenCalledWith(
|
|
"http://paperclip.test/api/companies/company-1/skills",
|
|
expect.objectContaining({
|
|
method: "GET",
|
|
headers: expect.objectContaining({ authorization: "Bearer token" }),
|
|
}),
|
|
);
|
|
expect(JSON.parse(String(logSpy.mock.calls[0]?.[0]))).toEqual(rows);
|
|
});
|
|
|
|
it("resolves a skill slug before reading detail", async () => {
|
|
fetchMock
|
|
.mockResolvedValueOnce(jsonResponse([skill()]))
|
|
.mockResolvedValueOnce(jsonResponse({ ...skill(), usedByAgents: [] }));
|
|
|
|
await runCommand([
|
|
"skills",
|
|
"show",
|
|
"Review PRs",
|
|
"--company-id",
|
|
"company-1",
|
|
"--api-base",
|
|
"http://paperclip.test",
|
|
"--api-key",
|
|
"token",
|
|
"--json",
|
|
]);
|
|
|
|
expect(fetchMock).toHaveBeenNthCalledWith(
|
|
2,
|
|
"http://paperclip.test/api/companies/company-1/skills/11111111-1111-1111-1111-111111111111",
|
|
expect.objectContaining({ method: "GET" }),
|
|
);
|
|
});
|
|
|
|
it("prints skill files as raw pipeable content in human mode", async () => {
|
|
fetchMock
|
|
.mockResolvedValueOnce(jsonResponse([skill()]))
|
|
.mockResolvedValueOnce(jsonResponse({
|
|
skillId: "11111111-1111-1111-1111-111111111111",
|
|
path: "SKILL.md",
|
|
kind: "skill",
|
|
content: "# Review PRs",
|
|
language: "markdown",
|
|
markdown: true,
|
|
editable: true,
|
|
}));
|
|
|
|
await runCommand([
|
|
"skills",
|
|
"file",
|
|
"review-prs",
|
|
"--company-id",
|
|
"company-1",
|
|
"--api-base",
|
|
"http://paperclip.test",
|
|
"--api-key",
|
|
"token",
|
|
]);
|
|
|
|
expect(logSpy).not.toHaveBeenCalled();
|
|
expect(writeChunks.join("")).toBe("# Review PRs\n");
|
|
});
|
|
|
|
it("browses catalog skills with filters in table output", async () => {
|
|
fetchMock.mockResolvedValueOnce(jsonResponse([catalogSkill()]));
|
|
|
|
await runCommand([
|
|
"skills",
|
|
"browse",
|
|
"--kind",
|
|
"bundled",
|
|
"--category",
|
|
"software-development",
|
|
"--query",
|
|
"github",
|
|
"--api-base",
|
|
"http://paperclip.test",
|
|
"--api-key",
|
|
"token",
|
|
]);
|
|
|
|
expect(fetchMock).toHaveBeenCalledWith(
|
|
"http://paperclip.test/api/skills/catalog?kind=bundled&category=software-development&q=github",
|
|
expect.objectContaining({ method: "GET" }),
|
|
);
|
|
const rendered = logSpy.mock.calls.map((call: unknown[]) => String(call[0])).join("\n");
|
|
expect(rendered).toContain("id");
|
|
expect(rendered).toContain("paperclipai:bundled:software-development:github-pr-workflow");
|
|
expect(rendered).toContain("roles");
|
|
});
|
|
|
|
it("searches catalog skills as JSON", async () => {
|
|
const rows = [catalogSkill()];
|
|
fetchMock.mockResolvedValueOnce(jsonResponse(rows));
|
|
|
|
await runCommand([
|
|
"skills",
|
|
"search",
|
|
"pull requests",
|
|
"--kind",
|
|
"bundled",
|
|
"--api-base",
|
|
"http://paperclip.test",
|
|
"--api-key",
|
|
"token",
|
|
"--json",
|
|
]);
|
|
|
|
expect(fetchMock).toHaveBeenCalledWith(
|
|
"http://paperclip.test/api/skills/catalog?kind=bundled&q=pull+requests",
|
|
expect.objectContaining({ method: "GET" }),
|
|
);
|
|
expect(JSON.parse(String(logSpy.mock.calls[0]?.[0]))).toEqual(rows);
|
|
});
|
|
|
|
it("inspects catalog skill detail by query ref so keys with slashes work", async () => {
|
|
const detail = catalogSkill();
|
|
fetchMock.mockResolvedValueOnce(jsonResponse(detail));
|
|
|
|
await runCommand([
|
|
"skills",
|
|
"inspect",
|
|
"paperclipai/bundled/software-development/github-pr-workflow",
|
|
"--api-base",
|
|
"http://paperclip.test",
|
|
"--api-key",
|
|
"token",
|
|
"--json",
|
|
]);
|
|
|
|
expect(fetchMock).toHaveBeenCalledWith(
|
|
"http://paperclip.test/api/skills/catalog/ref?ref=paperclipai%2Fbundled%2Fsoftware-development%2Fgithub-pr-workflow",
|
|
expect.objectContaining({ method: "GET" }),
|
|
);
|
|
expect(JSON.parse(String(logSpy.mock.calls[0]?.[0]))).toEqual(detail);
|
|
});
|
|
|
|
it("installs catalog skills into the company library without agent sync", async () => {
|
|
const result = {
|
|
action: "created",
|
|
skill: skill({
|
|
key: "paperclipai/bundled/software-development/github-pr-workflow",
|
|
slug: "pr-flow",
|
|
sourceType: "catalog",
|
|
}),
|
|
catalogSkill: catalogSkill(),
|
|
warnings: [],
|
|
};
|
|
fetchMock.mockResolvedValueOnce(jsonResponse(result, 201));
|
|
|
|
await runCommand([
|
|
"skills",
|
|
"install",
|
|
"github-pr-workflow",
|
|
"--as",
|
|
"pr-flow",
|
|
"--force",
|
|
"--company-id",
|
|
"company-1",
|
|
"--api-base",
|
|
"http://paperclip.test",
|
|
"--api-key",
|
|
"token",
|
|
"--json",
|
|
]);
|
|
|
|
expect(fetchMock).toHaveBeenCalledWith(
|
|
"http://paperclip.test/api/companies/company-1/skills/install-catalog",
|
|
expect.objectContaining({
|
|
method: "POST",
|
|
body: JSON.stringify({
|
|
catalogSkillId: "github-pr-workflow",
|
|
slug: "pr-flow",
|
|
force: true,
|
|
}),
|
|
}),
|
|
);
|
|
expect(JSON.parse(String(logSpy.mock.calls[0]?.[0]))).toEqual(result);
|
|
});
|
|
|
|
it("passes force to skill updates", async () => {
|
|
fetchMock
|
|
.mockResolvedValueOnce(jsonResponse([skill()]))
|
|
.mockResolvedValueOnce(jsonResponse(skill({ sourceRef: "sha256:new" })));
|
|
|
|
await runCommand([
|
|
"skills",
|
|
"update",
|
|
"review-prs",
|
|
"--force",
|
|
"--company-id",
|
|
"company-1",
|
|
"--api-base",
|
|
"http://paperclip.test",
|
|
"--api-key",
|
|
"token",
|
|
"--json",
|
|
]);
|
|
|
|
expect(fetchMock).toHaveBeenNthCalledWith(
|
|
2,
|
|
"http://paperclip.test/api/companies/company-1/skills/11111111-1111-1111-1111-111111111111/install-update",
|
|
expect.objectContaining({
|
|
method: "POST",
|
|
body: JSON.stringify({ force: true }),
|
|
}),
|
|
);
|
|
});
|
|
|
|
it("audits installed skill bytes through the server", async () => {
|
|
const audit = {
|
|
skillId: "11111111-1111-1111-1111-111111111111",
|
|
installedHash: "sha256:installed",
|
|
originHash: "sha256:origin",
|
|
verdict: "warning",
|
|
codes: ["network_reference"],
|
|
findings: [{
|
|
code: "network_reference",
|
|
severity: "warning",
|
|
message: "Skill content references network-capable commands or URLs.",
|
|
path: "SKILL.md",
|
|
}],
|
|
scannedAt: "2026-05-26T00:00:00.000Z",
|
|
scanVersion: "skills-audit-v1",
|
|
};
|
|
fetchMock
|
|
.mockResolvedValueOnce(jsonResponse([skill()]))
|
|
.mockResolvedValueOnce(jsonResponse(audit));
|
|
|
|
await runCommand([
|
|
"skills",
|
|
"audit",
|
|
"review-prs",
|
|
"--company-id",
|
|
"company-1",
|
|
"--api-base",
|
|
"http://paperclip.test",
|
|
"--api-key",
|
|
"token",
|
|
"--json",
|
|
]);
|
|
|
|
expect(fetchMock).toHaveBeenNthCalledWith(
|
|
2,
|
|
"http://paperclip.test/api/companies/company-1/skills/11111111-1111-1111-1111-111111111111/audit",
|
|
expect.objectContaining({
|
|
method: "POST",
|
|
body: JSON.stringify({}),
|
|
}),
|
|
);
|
|
expect(JSON.parse(String(logSpy.mock.calls[0]?.[0]))).toEqual(audit);
|
|
});
|
|
|
|
it("requires confirmation for reset and sends force when confirmed", async () => {
|
|
fetchMock
|
|
.mockResolvedValueOnce(jsonResponse([skill({ sourceType: "catalog" })]))
|
|
.mockResolvedValueOnce(jsonResponse(skill({ sourceType: "catalog" })));
|
|
|
|
await runCommand([
|
|
"skills",
|
|
"reset",
|
|
"review-prs",
|
|
"--yes",
|
|
"--force",
|
|
"--company-id",
|
|
"company-1",
|
|
"--api-base",
|
|
"http://paperclip.test",
|
|
"--api-key",
|
|
"token",
|
|
"--json",
|
|
]);
|
|
|
|
expect(fetchMock).toHaveBeenNthCalledWith(
|
|
2,
|
|
"http://paperclip.test/api/companies/company-1/skills/11111111-1111-1111-1111-111111111111/reset",
|
|
expect.objectContaining({
|
|
method: "POST",
|
|
body: JSON.stringify({ force: true }),
|
|
}),
|
|
);
|
|
});
|
|
|
|
it("syncs desired company skill refs to an agent and returns the runtime snapshot", async () => {
|
|
const snapshot = {
|
|
adapterType: "codex_local",
|
|
supported: true,
|
|
mode: "persistent",
|
|
desiredSkills: ["paperclip/review-prs"],
|
|
entries: [
|
|
{
|
|
key: "paperclip/review-prs",
|
|
runtimeName: "review-prs",
|
|
desired: true,
|
|
managed: true,
|
|
required: false,
|
|
state: "installed",
|
|
origin: "company_managed",
|
|
detail: null,
|
|
},
|
|
],
|
|
warnings: [],
|
|
};
|
|
fetchMock
|
|
.mockResolvedValueOnce(jsonResponse(agent()))
|
|
.mockResolvedValueOnce(jsonResponse(snapshot));
|
|
|
|
await runCommand([
|
|
"skills",
|
|
"agent",
|
|
"sync",
|
|
"coder",
|
|
"--skill",
|
|
"review-prs",
|
|
"--skill",
|
|
"paperclip/qa",
|
|
"--mode",
|
|
"add",
|
|
"--company-id",
|
|
"company-1",
|
|
"--api-base",
|
|
"http://paperclip.test",
|
|
"--api-key",
|
|
"token",
|
|
"--json",
|
|
]);
|
|
|
|
expect(fetchMock).toHaveBeenNthCalledWith(
|
|
1,
|
|
"http://paperclip.test/api/agents/coder?companyId=company-1",
|
|
expect.objectContaining({ method: "GET" }),
|
|
);
|
|
expect(fetchMock).toHaveBeenNthCalledWith(
|
|
2,
|
|
"http://paperclip.test/api/agents/agent-1/skills/sync",
|
|
expect.objectContaining({
|
|
method: "POST",
|
|
body: JSON.stringify({ desiredSkills: ["review-prs", "paperclip/qa"], mode: "add" }),
|
|
}),
|
|
);
|
|
expect(JSON.parse(String(logSpy.mock.calls[0]?.[0]))).toEqual(snapshot);
|
|
});
|
|
});
|