diff --git a/server/src/__tests__/agent-skills-routes.test.ts b/server/src/__tests__/agent-skills-routes.test.ts new file mode 100644 index 00000000..fde5abd2 --- /dev/null +++ b/server/src/__tests__/agent-skills-routes.test.ts @@ -0,0 +1,187 @@ +import express from "express"; +import request from "supertest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { agentRoutes } from "../routes/agents.js"; +import { errorHandler } from "../middleware/index.js"; + +const mockAgentService = vi.hoisted(() => ({ + getById: vi.fn(), + update: vi.fn(), + resolveByReference: vi.fn(), +})); + +const mockAccessService = vi.hoisted(() => ({ + canUser: vi.fn(), + hasPermission: vi.fn(), +})); + +const mockApprovalService = vi.hoisted(() => ({})); +const mockBudgetService = vi.hoisted(() => ({})); +const mockHeartbeatService = vi.hoisted(() => ({})); +const mockIssueApprovalService = vi.hoisted(() => ({})); +const mockWorkspaceOperationService = vi.hoisted(() => ({})); + +const mockCompanySkillService = vi.hoisted(() => ({ + listRuntimeSkillEntries: vi.fn(), +})); + +const mockSecretService = vi.hoisted(() => ({ + resolveAdapterConfigForRuntime: vi.fn(), +})); + +const mockLogActivity = vi.hoisted(() => vi.fn()); + +const mockAdapter = vi.hoisted(() => ({ + listSkills: vi.fn(), + syncSkills: vi.fn(), +})); + +vi.mock("../services/index.js", () => ({ + agentService: () => mockAgentService, + accessService: () => mockAccessService, + approvalService: () => mockApprovalService, + companySkillService: () => mockCompanySkillService, + budgetService: () => mockBudgetService, + heartbeatService: () => mockHeartbeatService, + issueApprovalService: () => mockIssueApprovalService, + issueService: () => ({}), + logActivity: mockLogActivity, + secretService: () => mockSecretService, + workspaceOperationService: () => mockWorkspaceOperationService, +})); + +vi.mock("../adapters/index.js", () => ({ + findServerAdapter: vi.fn(() => mockAdapter), + listAdapterModels: vi.fn(), +})); + +function createApp() { + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + (req as any).actor = { + type: "board", + userId: "local-board", + companyIds: ["company-1"], + source: "local_implicit", + isInstanceAdmin: false, + }; + next(); + }); + app.use("/api", agentRoutes({} as any)); + app.use(errorHandler); + return app; +} + +function makeAgent(adapterType: string) { + return { + id: "11111111-1111-4111-8111-111111111111", + companyId: "company-1", + name: "Agent", + role: "engineer", + title: "Engineer", + status: "active", + reportsTo: null, + capabilities: null, + adapterType, + adapterConfig: {}, + runtimeConfig: {}, + permissions: null, + updatedAt: new Date(), + }; +} + +describe("agent skill routes", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockAgentService.resolveByReference.mockResolvedValue({ + ambiguous: false, + agent: makeAgent("claude_local"), + }); + mockSecretService.resolveAdapterConfigForRuntime.mockResolvedValue({ config: { env: {} } }); + mockCompanySkillService.listRuntimeSkillEntries.mockResolvedValue([ + { + key: "paperclipai/paperclip/paperclip", + runtimeName: "paperclip", + source: "/tmp/paperclip", + required: true, + requiredReason: "required", + }, + ]); + mockAdapter.listSkills.mockResolvedValue({ + adapterType: "claude_local", + supported: true, + mode: "ephemeral", + desiredSkills: ["paperclipai/paperclip/paperclip"], + entries: [], + warnings: [], + }); + mockAdapter.syncSkills.mockResolvedValue({ + adapterType: "claude_local", + supported: true, + mode: "ephemeral", + desiredSkills: ["paperclipai/paperclip/paperclip"], + entries: [], + warnings: [], + }); + mockAgentService.update.mockImplementation(async (_id: string, patch: Record) => ({ + ...makeAgent("claude_local"), + adapterConfig: patch.adapterConfig ?? {}, + })); + mockLogActivity.mockResolvedValue(undefined); + }); + + it("skips runtime materialization when listing Claude skills", async () => { + mockAgentService.getById.mockResolvedValue(makeAgent("claude_local")); + + const res = await request(createApp()) + .get("/api/agents/11111111-1111-4111-8111-111111111111/skills?companyId=company-1"); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(mockCompanySkillService.listRuntimeSkillEntries).toHaveBeenCalledWith("company-1", { + materializeMissing: false, + }); + expect(mockAdapter.listSkills).toHaveBeenCalledWith( + expect.objectContaining({ + adapterType: "claude_local", + config: expect.objectContaining({ + paperclipRuntimeSkills: expect.any(Array), + }), + }), + ); + }); + + it("keeps runtime materialization for persistent skill adapters", async () => { + mockAgentService.getById.mockResolvedValue(makeAgent("codex_local")); + mockAdapter.listSkills.mockResolvedValue({ + adapterType: "codex_local", + supported: true, + mode: "persistent", + desiredSkills: ["paperclipai/paperclip/paperclip"], + entries: [], + warnings: [], + }); + + const res = await request(createApp()) + .get("/api/agents/11111111-1111-4111-8111-111111111111/skills?companyId=company-1"); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(mockCompanySkillService.listRuntimeSkillEntries).toHaveBeenCalledWith("company-1", { + materializeMissing: true, + }); + }); + + it("skips runtime materialization when syncing Claude skills", async () => { + mockAgentService.getById.mockResolvedValue(makeAgent("claude_local")); + + const res = await request(createApp()) + .post("/api/agents/11111111-1111-4111-8111-111111111111/skills/sync?companyId=company-1") + .send({ desiredSkills: ["paperclipai/paperclip/paperclip"] }); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(mockCompanySkillService.listRuntimeSkillEntries).toHaveBeenCalledWith("company-1", { + materializeMissing: false, + }); + expect(mockAdapter.syncSkills).toHaveBeenCalled(); + }); +}); diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index 0fbe9b59..66bb62b8 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -360,8 +360,18 @@ export function agentRoutes(db: Db) { }; } - async function buildRuntimeSkillConfig(companyId: string, config: Record) { - const runtimeSkillEntries = await companySkills.listRuntimeSkillEntries(companyId); + function shouldMaterializeRuntimeSkillsForAdapter(adapterType: string) { + return adapterType !== "claude_local"; + } + + async function buildRuntimeSkillConfig( + companyId: string, + adapterType: string, + config: Record, + ) { + const runtimeSkillEntries = await companySkills.listRuntimeSkillEntries(companyId, { + materializeMissing: shouldMaterializeRuntimeSkillsForAdapter(adapterType), + }); return { ...config, paperclipRuntimeSkills: runtimeSkillEntries, @@ -507,7 +517,9 @@ export function agentRoutes(db: Db) { const preference = readPaperclipSkillSyncPreference( agent.adapterConfig as Record, ); - const runtimeSkillEntries = await companySkills.listRuntimeSkillEntries(agent.companyId); + const runtimeSkillEntries = await companySkills.listRuntimeSkillEntries(agent.companyId, { + materializeMissing: false, + }); const requiredSkills = runtimeSkillEntries.filter((entry) => entry.required).map((entry) => entry.key); res.json(buildUnsupportedSkillSnapshot(agent.adapterType, Array.from(new Set([...requiredSkills, ...preference.desiredSkills])))); return; @@ -517,7 +529,11 @@ export function agentRoutes(db: Db) { agent.companyId, agent.adapterConfig, ); - const runtimeSkillConfig = await buildRuntimeSkillConfig(agent.companyId, runtimeConfig); + const runtimeSkillConfig = await buildRuntimeSkillConfig( + agent.companyId, + agent.adapterType, + runtimeConfig, + ); const snapshot = await adapter.listSkills({ agentId: agent.id, companyId: agent.companyId, @@ -546,7 +562,9 @@ export function agentRoutes(db: Db) { .filter(Boolean), ), ); - const runtimeSkillEntries = await companySkills.listRuntimeSkillEntries(agent.companyId); + const runtimeSkillEntries = await companySkills.listRuntimeSkillEntries(agent.companyId, { + materializeMissing: shouldMaterializeRuntimeSkillsForAdapter(agent.adapterType), + }); const requiredSkills = runtimeSkillEntries.filter((entry) => entry.required).map((entry) => entry.key); const desiredSkills = Array.from(new Set([...requiredSkills, ...requestedSkills])); const nextAdapterConfig = writePaperclipSkillSyncPreference( diff --git a/server/src/services/company-skills.ts b/server/src/services/company-skills.ts index a899fc41..d528883c 100644 --- a/server/src/services/company-skills.ts +++ b/server/src/services/company-skills.ts @@ -83,6 +83,10 @@ export type ProjectSkillScanTarget = { workspaceCwd: string; }; +type RuntimeSkillEntryOptions = { + materializeMissing?: boolean; +}; + const PROJECT_SCAN_DIRECTORY_ROOTS = [ "skills", "skills/.curated", @@ -1795,7 +1799,15 @@ export function companySkillService(db: Db) { return skillDir; } - async function listRuntimeSkillEntries(companyId: string): Promise { + function resolveRuntimeSkillMaterializedPath(companyId: string, skill: CompanySkill) { + const runtimeRoot = path.resolve(resolveManagedSkillsRoot(companyId), "__runtime__"); + return path.resolve(runtimeRoot, buildSkillRuntimeName(skill.key, skill.slug)); + } + + async function listRuntimeSkillEntries( + companyId: string, + options: RuntimeSkillEntryOptions = {}, + ): Promise { await ensureBundledSkills(companyId); const rows = await db .select() @@ -1809,7 +1821,9 @@ export function companySkillService(db: Db) { const sourceKind = asString(getSkillMeta(skill).sourceKind); let source = normalizeSkillDirectory(skill); if (!source) { - source = await materializeRuntimeSkillFiles(companyId, skill).catch(() => null); + source = options.materializeMissing === false + ? resolveRuntimeSkillMaterializedPath(companyId, skill) + : await materializeRuntimeSkillFiles(companyId, skill).catch(() => null); } if (!source) continue;