1413729a06
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agents increasingly depend on reusable skills, so the control plane needs a first-class way to browse, inspect, install, version, and attach those skills. > - The old skills surface was mostly operational plumbing; it did not give operators a store-like discovery flow, canonical detail URLs, rich source/version context, or creation paths. > - The backend also needed stronger contracts around company skill metadata, versions, install counts, runtime materialization, and adapter skill preferences. > - This pull request builds the Skills Store foundation across DB, shared contracts, server routes/services, UI, and Storybook. > - The benefit is a more inspectable, operator-friendly skill workflow that still preserves company-scoped control-plane boundaries and agent runtime behavior. ## Linked Issues or Issue Description No GitHub issue exists for this Paperclip work item. Paperclip task refs: PAP-10846 and PAP-10921. Feature request: Paperclip operators need a single Skills Store experience where company skills can be discovered, inspected, created, versioned, installed, and attached to agents without relying on scattered operational screens or implicit runtime state. Related PR search: - Searched GitHub for `Skills Store`, `company skills`, and `skill detail`. - Found several open skills-related PRs such as #7809 and #4409, but no duplicate PR for this end-to-end Skills Store branch. ## What Changed - Added the Skills Store backend foundation: company skill schema fields, migrations, shared types/validators, and expanded server skill routes/services. - Added skill discovery, category navigation, canonical skill detail routes, tabs, source attribution, version snapshots/diffs, install count backfill, and creation flows. - Updated agent skill preference handling so version selections survive runtime mention injection and runtime skill materialization honors pinned versions. - Preserved unversioned skill assignments as live/current selections instead of silently pinning them to the current version at assignment time. - Added focused regression coverage for company skill routes/services, route helpers, UI behavior, skill version diffs, and runtime skill version pins. - Added Storybook coverage for Skills Store discovery/detail states and updated the main layout navigation. - Addressed Greptile findings around version creation races, soft-deleted comments, fork metadata scoping, GitHub skill directory fallback, runtime snapshot materialization, shared runtime skill-selection helpers, and version-assignment semantics. ## Verification - `pnpm exec vitest run server/src/__tests__/heartbeat-project-env.test.ts server/src/__tests__/heartbeat-runtime-skills.test.ts` - `pnpm exec vitest run packages/shared/src/validators/company-skill.test.ts` - `pnpm exec vitest run server/src/__tests__/company-portability.test.ts server/src/__tests__/company-skills-service.test.ts` - `pnpm exec vitest run cli/src/__tests__/company-import-export-e2e.test.ts` - `pnpm exec vitest run server/src/__tests__/agent-skills-routes.test.ts` - `pnpm exec vitest run server/src/__tests__/openapi-routes.test.ts` - `pnpm exec vitest run server/src/__tests__/company-skills-service.test.ts server/src/__tests__/company-skills-routes.test.ts server/src/__tests__/heartbeat-runtime-skills.test.ts` - `pnpm exec vitest run server/src/__tests__/company-skills-service.test.ts` - `pnpm exec vitest run ui/src/components/IssueProperties.test.tsx -t "edits existing custom assignee model options from the properties pane"` - `pnpm --filter @paperclipai/server typecheck` - GitHub checks are green on `0823957a2`: Build, Canary Dry Run, General tests, Typecheck + Release Registry, serialized server suites, e2e, policy/review, Socket, Snyk, and aggregate `verify`. - Greptile Review succeeded on `0823957a2` with `40 files reviewed, 0 comments added`; GitHub unresolved review threads: 0. Not run in this heartbeat: - Browser screenshot capture for the UI changes. This PR intentionally omits screenshots per the Paperclip task direction not to add design screenshots/images. ## Risks - Broad feature branch touching DB, shared contracts, server, and UI; reviewers should still scan merge conflicts carefully if `master` moves again before landing. - Skill version/runtime behavior is sensitive: pinned skill versions must stay pinned while default selections should continue following the current version. - UI polish should get normal reviewer/browser attention before merge because this PR includes a large Skills Store surface and screenshots were intentionally omitted. > 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 Codex, GPT-5-based coding agent with tool use and local command execution. ## 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 run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] If this change affects the UI, I have included before/after screenshots (intentionally omitted per PAP-10921 direction) - [ ] 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> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
362 lines
11 KiB
TypeScript
362 lines
11 KiB
TypeScript
import { describe, expect, it, vi } from "vitest";
|
|
import { buildSkillMentionHref } from "@paperclipai/shared";
|
|
import {
|
|
LOW_TRUST_REVIEW_PRESET,
|
|
applyRunScopedMentionedSkillKeys,
|
|
extractMentionedSkillIdsFromSources,
|
|
resolveExecutionRunAdapterConfig,
|
|
} from "../services/heartbeat.ts";
|
|
|
|
describe("resolveExecutionRunAdapterConfig", () => {
|
|
it("overlays project and routine env on top of agent env and unions secret keys", async () => {
|
|
const resolveAdapterConfigForRuntime = vi.fn().mockResolvedValue({
|
|
config: {
|
|
env: {
|
|
SHARED_KEY: "agent",
|
|
AGENT_ONLY: "agent-only",
|
|
},
|
|
other: "value",
|
|
},
|
|
secretKeys: new Set(["AGENT_SECRET"]),
|
|
manifest: [
|
|
{
|
|
configPath: "env.AGENT_SECRET",
|
|
envKey: "AGENT_SECRET",
|
|
secretId: "secret-agent",
|
|
secretKey: "agent-secret",
|
|
version: 1,
|
|
provider: "local_encrypted",
|
|
outcome: "success",
|
|
},
|
|
],
|
|
});
|
|
const resolveEnvBindings = vi
|
|
.fn()
|
|
.mockResolvedValueOnce({
|
|
env: {
|
|
SHARED_KEY: "project",
|
|
PROJECT_ONLY: "project-only",
|
|
},
|
|
secretKeys: new Set(["PROJECT_SECRET"]),
|
|
manifest: [
|
|
{
|
|
configPath: "env.PROJECT_SECRET",
|
|
envKey: "PROJECT_SECRET",
|
|
secretId: "secret-project",
|
|
secretKey: "project-secret",
|
|
version: 1,
|
|
provider: "local_encrypted",
|
|
outcome: "success",
|
|
},
|
|
],
|
|
})
|
|
.mockResolvedValueOnce({
|
|
env: {
|
|
SHARED_KEY: "routine",
|
|
ROUTINE_ONLY: "routine-only",
|
|
},
|
|
secretKeys: new Set(["ROUTINE_SECRET"]),
|
|
manifest: [
|
|
{
|
|
configPath: "env.ROUTINE_SECRET",
|
|
envKey: "ROUTINE_SECRET",
|
|
secretId: "secret-routine",
|
|
secretKey: "routine-secret",
|
|
version: 1,
|
|
provider: "local_encrypted",
|
|
outcome: "success",
|
|
},
|
|
],
|
|
});
|
|
|
|
const result = await resolveExecutionRunAdapterConfig({
|
|
companyId: "company-1",
|
|
executionRunConfig: { env: { SHARED_KEY: "agent" } },
|
|
projectEnv: { SHARED_KEY: "project" },
|
|
routineEnv: { SHARED_KEY: "routine" },
|
|
routineId: "routine-1",
|
|
secretsSvc: {
|
|
resolveAdapterConfigForRuntime,
|
|
resolveEnvBindings,
|
|
} as any,
|
|
});
|
|
|
|
expect(result.resolvedConfig).toMatchObject({
|
|
other: "value",
|
|
env: {
|
|
SHARED_KEY: "routine",
|
|
AGENT_ONLY: "agent-only",
|
|
PROJECT_ONLY: "project-only",
|
|
ROUTINE_ONLY: "routine-only",
|
|
},
|
|
});
|
|
expect(Array.from(result.secretKeys).sort()).toEqual(["AGENT_SECRET", "PROJECT_SECRET", "ROUTINE_SECRET"]);
|
|
expect(result.secretManifest.map((entry) => entry.secretId).sort()).toEqual([
|
|
"secret-agent",
|
|
"secret-project",
|
|
"secret-routine",
|
|
]);
|
|
expect(JSON.stringify(result.secretManifest)).not.toContain("agent-only");
|
|
expect(JSON.stringify(result.secretManifest)).not.toContain("project-only");
|
|
expect(JSON.stringify(result.secretManifest)).not.toContain("routine-only");
|
|
expect(resolveEnvBindings.mock.calls[1]?.[2]).toMatchObject({
|
|
consumerType: "routine",
|
|
consumerId: "routine-1",
|
|
});
|
|
});
|
|
|
|
it("drops Paperclip runtime-owned env before resolving agent, project, and routine overlays", async () => {
|
|
const resolveAdapterConfigForRuntime = vi.fn(async (_companyId, config: Record<string, unknown>) => ({
|
|
config: {
|
|
...config,
|
|
env: { ...(config.env as Record<string, unknown>) },
|
|
},
|
|
secretKeys: new Set<string>(),
|
|
manifest: [],
|
|
}));
|
|
const resolveEnvBindings = vi.fn(async (_companyId, env: Record<string, unknown>) => ({
|
|
env: Object.fromEntries(
|
|
Object.entries(env).filter((entry): entry is [string, string] => typeof entry[1] === "string"),
|
|
),
|
|
secretKeys: new Set<string>(),
|
|
manifest: [],
|
|
}));
|
|
|
|
const result = await resolveExecutionRunAdapterConfig({
|
|
companyId: "company-1",
|
|
agentId: "agent-1",
|
|
executionRunConfig: {
|
|
env: {
|
|
PAPERCLIP_API_KEY: { type: "secret_ref", secretId: "secret-api-key", version: "latest" },
|
|
PAPERCLIP_AGENT_ID: "spoofed-agent",
|
|
AGENT_ONLY: "agent-only",
|
|
},
|
|
},
|
|
projectEnv: {
|
|
PAPERCLIP_API_KEY: "project-api-key",
|
|
PAPERCLIP_COMPANY_ID: "spoofed-company",
|
|
PROJECT_ONLY: "project-only",
|
|
},
|
|
routineEnv: {
|
|
PAPERCLIP_API_KEY: "routine-api-key",
|
|
PAPERCLIP_RUN_ID: "spoofed-run",
|
|
ROUTINE_ONLY: "routine-only",
|
|
},
|
|
routineId: "routine-1",
|
|
secretsSvc: {
|
|
resolveAdapterConfigForRuntime,
|
|
resolveEnvBindings,
|
|
} as any,
|
|
});
|
|
|
|
expect(resolveAdapterConfigForRuntime.mock.calls[0]?.[1]).toEqual({
|
|
env: {
|
|
AGENT_ONLY: "agent-only",
|
|
},
|
|
});
|
|
expect(resolveEnvBindings.mock.calls[0]?.[1]).toEqual({
|
|
PROJECT_ONLY: "project-only",
|
|
});
|
|
expect(resolveEnvBindings.mock.calls[1]?.[1]).toEqual({
|
|
ROUTINE_ONLY: "routine-only",
|
|
});
|
|
expect(result.resolvedConfig.env).toEqual({
|
|
AGENT_ONLY: "agent-only",
|
|
PROJECT_ONLY: "project-only",
|
|
ROUTINE_ONLY: "routine-only",
|
|
});
|
|
expect(JSON.stringify(result.resolvedConfig.env)).not.toContain("PAPERCLIP_");
|
|
});
|
|
|
|
it("skips project env resolution when the project has no bindings", async () => {
|
|
const resolveAdapterConfigForRuntime = vi.fn().mockResolvedValue({
|
|
config: { env: { AGENT_ONLY: "agent-only" } },
|
|
secretKeys: new Set<string>(),
|
|
manifest: [],
|
|
});
|
|
const resolveEnvBindings = vi.fn();
|
|
|
|
const result = await resolveExecutionRunAdapterConfig({
|
|
companyId: "company-1",
|
|
executionRunConfig: { env: { AGENT_ONLY: "agent-only" } },
|
|
projectEnv: null,
|
|
secretsSvc: {
|
|
resolveAdapterConfigForRuntime,
|
|
resolveEnvBindings,
|
|
} as any,
|
|
});
|
|
|
|
expect(result.resolvedConfig.env).toEqual({ AGENT_ONLY: "agent-only" });
|
|
expect(result.secretManifest).toEqual([]);
|
|
expect(resolveEnvBindings).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("passes low-trust allowed secret binding ids into all runtime secret contexts", async () => {
|
|
const resolveAdapterConfigForRuntime = vi.fn().mockResolvedValue({
|
|
config: { env: {} },
|
|
secretKeys: new Set<string>(),
|
|
manifest: [],
|
|
});
|
|
const resolveEnvBindings = vi.fn().mockResolvedValue({
|
|
env: {},
|
|
secretKeys: new Set<string>(),
|
|
manifest: [],
|
|
});
|
|
|
|
await resolveExecutionRunAdapterConfig({
|
|
companyId: "company-1",
|
|
agentId: "agent-1",
|
|
issueId: "issue-1",
|
|
heartbeatRunId: "run-1",
|
|
projectId: "project-1",
|
|
routineId: "routine-1",
|
|
executionRunConfig: { env: {} },
|
|
projectEnv: { PROJECT_FLAG: "plain" },
|
|
routineEnv: { ROUTINE_FLAG: "plain" },
|
|
trustPreset: {
|
|
kind: "low_trust_review",
|
|
preset: LOW_TRUST_REVIEW_PRESET,
|
|
boundary: {
|
|
mode: LOW_TRUST_REVIEW_PRESET,
|
|
companyId: "company-1",
|
|
issueIds: ["issue-1"],
|
|
allowedSecretBindingIds: ["binding-1"],
|
|
},
|
|
sourcePresets: {},
|
|
},
|
|
secretsSvc: {
|
|
resolveAdapterConfigForRuntime,
|
|
resolveEnvBindings,
|
|
} as any,
|
|
});
|
|
|
|
expect(resolveAdapterConfigForRuntime.mock.calls[0]?.[2]).toMatchObject({
|
|
allowedBindingIds: ["binding-1"],
|
|
});
|
|
expect(resolveEnvBindings.mock.calls[0]?.[2]).toMatchObject({
|
|
allowedBindingIds: ["binding-1"],
|
|
});
|
|
expect(resolveEnvBindings.mock.calls[1]?.[2]).toMatchObject({
|
|
allowedBindingIds: ["binding-1"],
|
|
});
|
|
});
|
|
|
|
it("rejects inline sensitive env values for low-trust runs", async () => {
|
|
await expect(resolveExecutionRunAdapterConfig({
|
|
companyId: "company-1",
|
|
agentId: "agent-1",
|
|
issueId: "issue-1",
|
|
executionRunConfig: {
|
|
env: {
|
|
OPENAI_API_KEY: "inline-secret",
|
|
},
|
|
},
|
|
projectEnv: null,
|
|
trustPreset: {
|
|
kind: "low_trust_review",
|
|
preset: LOW_TRUST_REVIEW_PRESET,
|
|
boundary: {
|
|
mode: LOW_TRUST_REVIEW_PRESET,
|
|
companyId: "company-1",
|
|
issueIds: ["issue-1"],
|
|
},
|
|
sourcePresets: {},
|
|
},
|
|
secretsSvc: {
|
|
resolveAdapterConfigForRuntime: vi.fn(),
|
|
resolveEnvBindings: vi.fn(),
|
|
} as any,
|
|
})).rejects.toMatchObject({
|
|
status: 422,
|
|
details: { code: "low_trust_inline_sensitive_env_denied" },
|
|
});
|
|
});
|
|
});
|
|
|
|
describe("extractMentionedSkillIdsFromSources", () => {
|
|
it("collects UUID skill mention ids across issue sources", () => {
|
|
const releaseSkillId = "11111111-1111-4111-8111-111111111111";
|
|
const browserSkillId = "22222222-2222-4222-8222-222222222222";
|
|
const releaseHref = buildSkillMentionHref(releaseSkillId, "release-changelog");
|
|
const browserHref = buildSkillMentionHref(browserSkillId, "agent-browser");
|
|
|
|
expect(
|
|
extractMentionedSkillIdsFromSources([
|
|
`Please use [/release-changelog](${releaseHref})`,
|
|
`And also [/agent-browser](${browserHref})`,
|
|
`Duplicate mention [/release-changelog](${releaseHref})`,
|
|
]),
|
|
).toEqual([releaseSkillId, browserSkillId]);
|
|
});
|
|
|
|
it("ignores legacy non-UUID skill mention ids before runtime database lookup", () => {
|
|
const validSkillId = "33333333-3333-4333-8333-333333333333";
|
|
const validHref = buildSkillMentionHref(validSkillId, "greploop");
|
|
const legacyHref = buildSkillMentionHref("skill-greploop", "greploop");
|
|
|
|
expect(
|
|
extractMentionedSkillIdsFromSources([
|
|
`Use [/greploop](${legacyHref}) and [/prcheckloop](${validHref})`,
|
|
]),
|
|
).toEqual([validSkillId]);
|
|
});
|
|
});
|
|
|
|
describe("applyRunScopedMentionedSkillKeys", () => {
|
|
it("adds mentioned skills without mutating the original config", () => {
|
|
const originalConfig = {
|
|
command: "codex",
|
|
paperclipSkillSync: {
|
|
desiredSkills: ["paperclipai/paperclip/paperclip"],
|
|
},
|
|
};
|
|
|
|
const updatedConfig = applyRunScopedMentionedSkillKeys(originalConfig, [
|
|
"company/company-1/release-changelog",
|
|
"paperclipai/paperclip/paperclip",
|
|
"company/company-1/release-changelog",
|
|
]);
|
|
|
|
expect(updatedConfig).toEqual({
|
|
command: "codex",
|
|
paperclipSkillSync: {
|
|
desiredSkills: [
|
|
"paperclipai/paperclip/paperclip",
|
|
"company/company-1/release-changelog",
|
|
],
|
|
},
|
|
});
|
|
expect(originalConfig).toEqual({
|
|
command: "codex",
|
|
paperclipSkillSync: {
|
|
desiredSkills: ["paperclipai/paperclip/paperclip"],
|
|
},
|
|
});
|
|
});
|
|
|
|
it("preserves existing version pins when adding mentioned skills", () => {
|
|
const originalConfig = {
|
|
command: "codex",
|
|
paperclipSkillSync: {
|
|
desiredSkills: [
|
|
{ key: "company/company-1/release-changelog", versionId: "version-1" },
|
|
],
|
|
},
|
|
};
|
|
|
|
const updatedConfig = applyRunScopedMentionedSkillKeys(originalConfig, [
|
|
"company/company-1/security-review",
|
|
]);
|
|
|
|
expect(updatedConfig).toEqual({
|
|
command: "codex",
|
|
paperclipSkillSync: {
|
|
desiredSkills: [
|
|
{ key: "company/company-1/release-changelog", versionId: "version-1" },
|
|
{ key: "company/company-1/security-review", versionId: null },
|
|
],
|
|
},
|
|
});
|
|
});
|
|
});
|