From db83eb2a0074dc01e431cb33ad0129e32250d21e Mon Sep 17 00:00:00 2001 From: Mikkel Georgsen Date: Wed, 1 Apr 2026 11:29:03 +0200 Subject: [PATCH] [nexus] test(19-02): route-level integration tests for adapter-aware skill routes - Add 18 supertest tests covering install/uninstall/rollback/group routes - Verify 400 (missing agentId), 404 (unknown agent), 422 (unsupported adapter) - Verify 403 for native skill deletion - Verify hermes_local agents trigger syncHermesNativeSkills before list - Verify group assign resolves dir from URL agentId param - Fix wildcard route syntax from :skillId(*) to *skillId (Express 5 / path-to-regexp v8) resolves pre-existing TS errors for these routes too --- .../skill-registry-routes-adapter.test.ts | 382 ++++++++++++++++++ server/src/routes/skill-registry-groups.ts | 9 +- 2 files changed, 387 insertions(+), 4 deletions(-) create mode 100644 server/src/__tests__/skill-registry-routes-adapter.test.ts diff --git a/server/src/__tests__/skill-registry-routes-adapter.test.ts b/server/src/__tests__/skill-registry-routes-adapter.test.ts new file mode 100644 index 00000000..5b6e7cf9 --- /dev/null +++ b/server/src/__tests__/skill-registry-routes-adapter.test.ts @@ -0,0 +1,382 @@ +/** + * Route-level integration tests for adapter-aware skill registry routes. + * + * Verifies that: + * - Install/uninstall/rollback routes accept agentId (not agentSkillsDir) and resolve paths server-side + * - Group assign/remove routes resolve dir from URL agentId param + * - 400 returned for missing agentId + * - 404 returned for unknown agentId + * - 422 returned for unsupported adapter + * - 403 returned for native skill delete + * - hermes_local agents get syncHermesNativeSkills called before listing + */ +import express from "express"; +import request from "supertest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { skillRegistryRoutes } from "../routes/skill-registry.js"; +import { skillGroupRoutes } from "../routes/skill-registry-groups.js"; +import { errorHandler } from "../middleware/index.js"; + +// --------------------------------------------------------------------------- +// Hoisted mocks +// --------------------------------------------------------------------------- + +const mockAgentService = vi.hoisted(() => ({ + getById: vi.fn(), +})); + +const mockResolveAdapterSkillConfig = vi.hoisted(() => vi.fn()); + +const mockSkillRegistryService = vi.hoisted(() => ({ + list: vi.fn(), + getVersions: vi.fn(), + getById: vi.fn(), + install: vi.fn(), + uninstall: vi.fn(), + rollback: vi.fn(), + fetchAll: vi.fn(), + syncHermesNativeSkills: vi.fn(), +})); + +const mockSkillGroupService = vi.hoisted(() => ({ + listGroups: vi.fn(), + createGroup: vi.fn(), + getGroup: vi.fn(), + updateGroup: vi.fn(), + deleteGroup: vi.fn(), + listMembers: vi.fn(), + addMember: vi.fn(), + removeMember: vi.fn(), + exportGroup: vi.fn(), + importGroup: vi.fn(), + listAgentGroups: vi.fn(), + assignGroup: vi.fn(), + removeGroup: vi.fn(), + listAgentSkills: vi.fn(), +})); + +const mockSkillRatingService = vi.hoisted(() => ({ + rate: vi.fn(), + getRatings: vi.fn(), +})); + +const mockGetSkillRegistryDb = vi.hoisted(() => vi.fn()); +const mockAgentSkillsTable = vi.hoisted(() => ({ agentId: "agent_id", skillId: "skill_id" })); + +// --------------------------------------------------------------------------- +// Module mocks +// --------------------------------------------------------------------------- + +vi.mock("../services/agents.js", () => ({ + agentService: () => mockAgentService, +})); + +vi.mock("@paperclipai/adapter-utils", () => ({ + resolveAdapterSkillConfig: mockResolveAdapterSkillConfig, +})); + +vi.mock("../services/skill-registry.js", () => ({ + skillRegistryService: () => mockSkillRegistryService, +})); + +vi.mock("../services/skill-registry-groups.js", () => ({ + skillGroupService: () => mockSkillGroupService, +})); + +vi.mock("../services/skill-registry-ratings.js", () => ({ + skillRatingService: () => mockSkillRatingService, +})); + +vi.mock("../services/skill-registry-db.js", () => ({ + getSkillRegistryDb: mockGetSkillRegistryDb, +})); + +vi.mock("../services/skill-registry-schema.js", () => ({ + agentSkills: mockAgentSkillsTable, +})); + +// Mock drizzle-orm operators used in the DELETE route +vi.mock("drizzle-orm", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + }; +}); + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function makeAgent(adapterType: string) { + return { + id: "agent-123", + companyId: "company-1", + name: "Test Agent", + adapterType, + status: "active", + }; +} + +function makeDb() { + return {} as any; +} + +function createApp() { + const app = express(); + app.use(express.json()); + // Simulate local_trusted board actor + app.use((req, _res, next) => { + (req as any).actor = { + type: "board", + userId: "local-board", + companyIds: ["company-1"], + source: "local_implicit", + isInstanceAdmin: true, + }; + next(); + }); + const db = makeDb(); + app.use(skillRegistryRoutes(db)); + app.use(skillGroupRoutes(db)); + app.use(errorHandler); + return app; +} + +// Mock the DB returned by getSkillRegistryDb for native skill check +function setupRegistryDb(agentSkillRow?: { source: string } | null) { + const fakeDb = { + select: vi.fn(() => fakeDb), + from: vi.fn(() => fakeDb), + where: vi.fn(async () => agentSkillRow ? [agentSkillRow] : []), + }; + mockGetSkillRegistryDb.mockResolvedValue(fakeDb as any); + return fakeDb; +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe("skill registry routes — adapter-aware install/uninstall", () => { + let app: ReturnType; + + beforeEach(() => { + vi.clearAllMocks(); + app = createApp(); + + // Default: known agent with supported adapter + mockAgentService.getById.mockResolvedValue(makeAgent("claude_local")); + mockResolveAdapterSkillConfig.mockReturnValue({ + supportsInstall: true, + skillDir: "~/.claude/skills", + nativeSkillDir: null, + format: "markdown", + }); + mockSkillRegistryService.install.mockResolvedValue({ + type: "installed", + skillId: "hub/test-skill", + versionId: "v1", + targetDir: "/home/user/.claude/skills/test-skill", + }); + mockSkillRegistryService.uninstall.mockResolvedValue(undefined); + mockSkillRegistryService.rollback.mockResolvedValue(undefined); + mockSkillRegistryService.syncHermesNativeSkills.mockResolvedValue(undefined); + mockSkillGroupService.assignGroup.mockResolvedValue({ groupId: "g1", agentId: "agent-123" }); + mockSkillGroupService.removeGroup.mockResolvedValue(undefined); + mockSkillGroupService.listAgentSkills.mockResolvedValue([ + { skillId: "hub/test-skill", source: "managed", installedAt: 1234567890 }, + ]); + }); + + // --- Install --- + + describe("POST /skill-registry/skills/:sourceId/:slug/install", () => { + it("returns 400 when agentId is missing", async () => { + const res = await request(app) + .post("/skill-registry/skills/hub/test-skill/install") + .send({}); + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/agentId/i); + }); + + it("returns 404 when agentId is unknown", async () => { + mockAgentService.getById.mockResolvedValue(null); + const res = await request(app) + .post("/skill-registry/skills/hub/test-skill/install") + .send({ agentId: "unknown-agent" }); + expect(res.status).toBe(404); + expect(res.body.error).toMatch(/Agent not found/i); + }); + + it("returns 422 when adapter does not support install", async () => { + mockResolveAdapterSkillConfig.mockReturnValue({ + supportsInstall: false, + skillDir: null, + format: "none", + unsupportedReason: "Adapter does not support skill install", + }); + const res = await request(app) + .post("/skill-registry/skills/hub/test-skill/install") + .send({ agentId: "agent-123" }); + expect(res.status).toBe(422); + expect(res.body.error).toMatch(/does not support/i); + }); + + it("resolves the correct agentSkillsDir and installs when agentId is valid", async () => { + const res = await request(app) + .post("/skill-registry/skills/hub/test-skill/install") + .send({ agentId: "agent-123" }); + expect(res.status).toBe(200); + expect(mockAgentService.getById).toHaveBeenCalledWith("agent-123"); + expect(mockResolveAdapterSkillConfig).toHaveBeenCalledWith("claude_local"); + expect(mockSkillRegistryService.install).toHaveBeenCalledWith( + "hub/test-skill", + expect.stringContaining(".claude/skills"), + ); + }); + }); + + // --- Uninstall --- + + describe("DELETE /skill-registry/skills/:sourceId/:slug", () => { + it("returns 400 when agentId query param is missing", async () => { + setupRegistryDb(null); + const res = await request(app) + .delete("/skill-registry/skills/hub/test-skill"); + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/agentId/i); + }); + + it("returns 403 when skill is native", async () => { + setupRegistryDb({ source: "native" }); + const res = await request(app) + .delete("/skill-registry/skills/hub/test-skill") + .query({ agentId: "agent-123" }); + expect(res.status).toBe(403); + expect(res.body.error).toMatch(/native/i); + }); + + it("resolves dir and uninstalls when agentId is valid and skill is managed", async () => { + setupRegistryDb({ source: "managed" }); + const res = await request(app) + .delete("/skill-registry/skills/hub/test-skill") + .query({ agentId: "agent-123" }); + expect(res.status).toBe(200); + expect(mockSkillRegistryService.uninstall).toHaveBeenCalledWith( + "hub/test-skill", + expect.stringContaining(".claude/skills"), + ); + }); + + it("returns 404 when agentId is unknown", async () => { + setupRegistryDb(null); + mockAgentService.getById.mockResolvedValue(null); + const res = await request(app) + .delete("/skill-registry/skills/hub/test-skill") + .query({ agentId: "unknown-agent" }); + expect(res.status).toBe(404); + }); + }); + + // --- Rollback --- + + describe("POST /skill-registry/skills/:sourceId/:slug/rollback", () => { + it("returns 400 when agentId is missing", async () => { + const res = await request(app) + .post("/skill-registry/skills/hub/test-skill/rollback") + .send({ versionId: "v1" }); + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/agentId/i); + }); + + it("returns 404 when agentId is unknown", async () => { + mockAgentService.getById.mockResolvedValue(null); + const res = await request(app) + .post("/skill-registry/skills/hub/test-skill/rollback") + .send({ versionId: "v1", agentId: "unknown-agent" }); + expect(res.status).toBe(404); + }); + + it("resolves dir and rolls back when agentId is valid", async () => { + const res = await request(app) + .post("/skill-registry/skills/hub/test-skill/rollback") + .send({ versionId: "v1", agentId: "agent-123" }); + expect(res.status).toBe(200); + expect(mockSkillRegistryService.rollback).toHaveBeenCalledWith( + "hub/test-skill", + "v1", + expect.stringContaining(".claude/skills"), + ); + }); + }); + + // --- Agent skills list with hermes sync --- + + describe("GET /skill-registry/agents/:agentId/skills", () => { + it("returns skills with source field", async () => { + const res = await request(app) + .get("/skill-registry/agents/agent-123/skills"); + expect(res.status).toBe(200); + expect(Array.isArray(res.body)).toBe(true); + expect(res.body[0]).toHaveProperty("source"); + expect(res.body[0]).toHaveProperty("skillId"); + expect(res.body[0]).toHaveProperty("installedAt"); + }); + + it("calls syncHermesNativeSkills for hermes_local agents before listing", async () => { + mockAgentService.getById.mockResolvedValue(makeAgent("hermes_local")); + const res = await request(app) + .get("/skill-registry/agents/agent-123/skills"); + expect(res.status).toBe(200); + expect(mockSkillRegistryService.syncHermesNativeSkills).toHaveBeenCalledWith("agent-123"); + expect(mockSkillGroupService.listAgentSkills).toHaveBeenCalledWith("agent-123"); + }); + + it("does not call syncHermesNativeSkills for non-hermes agents", async () => { + const res = await request(app) + .get("/skill-registry/agents/agent-123/skills"); + expect(res.status).toBe(200); + expect(mockSkillRegistryService.syncHermesNativeSkills).not.toHaveBeenCalled(); + }); + + it("returns 404 when agent is unknown", async () => { + mockAgentService.getById.mockResolvedValue(null); + const res = await request(app) + .get("/skill-registry/agents/unknown-agent/skills"); + expect(res.status).toBe(404); + }); + }); + + // --- Group assign/remove --- + + describe("POST /skill-registry/agents/:agentId/groups", () => { + it("returns 400 when groupId is missing", async () => { + const res = await request(app) + .post("/skill-registry/agents/agent-123/groups") + .send({}); + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/groupId/i); + }); + + it("returns 404 when agentId is unknown", async () => { + mockAgentService.getById.mockResolvedValue(null); + const res = await request(app) + .post("/skill-registry/agents/unknown-agent/groups") + .send({ groupId: "g1" }); + expect(res.status).toBe(404); + }); + + it("resolves dir from URL agentId and assigns group", async () => { + const res = await request(app) + .post("/skill-registry/agents/agent-123/groups") + .send({ groupId: "g1" }); + expect(res.status).toBe(201); + expect(mockAgentService.getById).toHaveBeenCalledWith("agent-123"); + expect(mockSkillGroupService.assignGroup).toHaveBeenCalledWith( + "g1", + "agent-123", + expect.stringContaining(".claude/skills"), + ); + }); + }); +}); diff --git a/server/src/routes/skill-registry-groups.ts b/server/src/routes/skill-registry-groups.ts index 2e9ea72f..06ade9b7 100644 --- a/server/src/routes/skill-registry-groups.ts +++ b/server/src/routes/skill-registry-groups.ts @@ -148,10 +148,10 @@ export function skillGroupRoutes(db: Db): Router { } }); - router.delete("/skill-registry/groups/:groupId/members/:skillId(*)", async (req, res) => { + router.delete("/skill-registry/groups/:groupId/members/*skillId", async (req, res) => { assertBoard(req); try { - const skillId = req.params.skillId; + const skillId = (req.params as any).skillId as string; await svc.removeMember(req.params.groupId, skillId); res.status(204).end(); } catch (err) { @@ -202,11 +202,12 @@ export function skillGroupRoutes(db: Db): Router { }); // Remove a group from an agent — resolves skill dir from agentId URL param server-side - router.delete("/skill-registry/agents/:agentId/groups/:groupId(*)", async (req, res) => { + router.delete("/skill-registry/agents/:agentId/groups/*groupId", async (req, res) => { assertBoard(req); try { + const groupId = (req.params as any).groupId as string; const agentSkillsDir = await resolveSkillsDirForAgent(db, req.params.agentId); - await svc.removeGroup(req.params.groupId, req.params.agentId, agentSkillsDir); + await svc.removeGroup(groupId, req.params.agentId, agentSkillsDir); res.status(204).end(); } catch (err: any) { const status = err.status ?? 500;