[nexus] feat(19-01): unit tests for adapter-aware install/uninstall and Hermes dual-source

- skill-registry-adapter-install.test.ts: 9 tests covering install/uninstall/rollback/assignGroup/removeGroup
- hermes-dual-source.test.ts: 7 tests covering syncHermesNativeSkills idempotency and listAgentSkills object shape
- Fix skill-registry-install.test.ts: update uninstall() callers to pass agentSkillsDir (new required param)
- Fix removeGroup() bug: removed incorrect 'individualSkills' guard that prevented file removal for group-installed skills
  (rule 1 auto-fix: group-installed skills were never removed because they appeared in agentSkills with no way to distinguish from direct installs)
- All 16 new tests pass, all existing tests still pass
This commit is contained in:
Mikkel Georgsen 2026-04-01 11:19:57 +02:00
parent 256e263019
commit e3f9302360
4 changed files with 513 additions and 11 deletions

View file

@ -0,0 +1,214 @@
/**
* Tests for Hermes dual-source skill tracking.
* Verifies syncHermesNativeSkills and listAgentSkills typed return shape.
*
* Uses real temp directories instead of mocking readdir (ESM limitation prevents
* direct spying on node:fs/promises named exports in Vitest).
*
* Covers: HERM-01, HERM-02, HERM-03
*/
import { describe, it, expect, beforeEach, afterEach } from "vitest";
import { mkdtemp, rm, mkdir } from "node:fs/promises";
import path from "node:path";
import os from "node:os";
import { getSkillRegistryDb, resetSkillRegistryDb } from "../services/skill-registry-db.js";
import { skills, agentSkills } from "../services/skill-registry-schema.js";
import { skillRegistryService } from "../services/skill-registry.js";
import { skillGroupService } from "../services/skill-registry-groups.js";
import { eq } from "drizzle-orm";
// ---------------------------------------------------------------------------
// Test helpers
// ---------------------------------------------------------------------------
let tmpHome: string;
let fakeHermesSkillsDir: string;
let originalHome: string | undefined;
beforeEach(async () => {
tmpHome = await mkdtemp(path.join(os.tmpdir(), "nexus-hermes-test-"));
process.env.PAPERCLIP_HOME = tmpHome;
// Override HOME so syncHermesNativeSkills reads our fake ~/.hermes/skills/
originalHome = process.env.HOME;
process.env.HOME = tmpHome;
fakeHermesSkillsDir = path.join(tmpHome, ".hermes", "skills");
resetSkillRegistryDb();
});
afterEach(async () => {
resetSkillRegistryDb();
delete process.env.PAPERCLIP_HOME;
if (originalHome !== undefined) {
process.env.HOME = originalHome;
} else {
delete process.env.HOME;
}
await rm(tmpHome, { recursive: true, force: true });
});
/** Create fake skill directories inside ~/.hermes/skills/ */
async function createFakeHermesSkills(skillNames: string[]): Promise<void> {
await mkdir(fakeHermesSkillsDir, { recursive: true });
for (const name of skillNames) {
await mkdir(path.join(fakeHermesSkillsDir, name), { recursive: true });
}
}
// ---------------------------------------------------------------------------
// syncHermesNativeSkills
// ---------------------------------------------------------------------------
describe("syncHermesNativeSkills()", () => {
it("Test 1: creates skills stub rows with sourceId 'hermes-native'", async () => {
await createFakeHermesSkills(["code-reviewer", "test-writer"]);
const svc = skillRegistryService();
await svc.syncHermesNativeSkills("agent-hermes-1");
const db = await getSkillRegistryDb();
const skillRows = await db.select().from(skills)
.where(eq(skills.sourceId, "hermes-native"));
expect(skillRows.length).toBe(2);
expect(skillRows.map((r) => r.id).sort()).toEqual([
"hermes-native/code-reviewer",
"hermes-native/test-writer",
].sort());
expect(skillRows.every((r) => r.sourceId === "hermes-native")).toBe(true);
});
it("Test 2: creates agentSkills rows with source 'native'", async () => {
await createFakeHermesSkills(["code-reviewer"]);
const svc = skillRegistryService();
await svc.syncHermesNativeSkills("agent-hermes-2");
const db = await getSkillRegistryDb();
const agentSkillRows = await db.select().from(agentSkills)
.where(eq(agentSkills.agentId, "agent-hermes-2"));
expect(agentSkillRows.length).toBe(1);
expect(agentSkillRows[0]!.skillId).toBe("hermes-native/code-reviewer");
expect(agentSkillRows[0]!.source).toBe("native");
});
it("Test 3: is idempotent — running twice does not duplicate rows", async () => {
await createFakeHermesSkills(["code-reviewer"]);
const svc = skillRegistryService();
// Run twice
await svc.syncHermesNativeSkills("agent-hermes-3");
await svc.syncHermesNativeSkills("agent-hermes-3");
const db = await getSkillRegistryDb();
const agentSkillRows = await db.select().from(agentSkills)
.where(eq(agentSkills.agentId, "agent-hermes-3"));
// Should only have ONE row despite calling sync twice
expect(agentSkillRows.length).toBe(1);
});
it("Test 4: handles missing ~/.hermes/skills/ gracefully (no throw)", async () => {
// Do NOT create the hermes skills dir — simulate it not existing
const svc = skillRegistryService();
// Must not throw
await expect(svc.syncHermesNativeSkills("agent-hermes-4")).resolves.toBeUndefined();
const db = await getSkillRegistryDb();
const agentSkillRows = await db.select().from(agentSkills)
.where(eq(agentSkills.agentId, "agent-hermes-4"));
expect(agentSkillRows.length).toBe(0);
});
});
// ---------------------------------------------------------------------------
// listAgentSkills — typed object shape
// ---------------------------------------------------------------------------
describe("listAgentSkills() — returns objects with source field", () => {
it("Test 5: returns Array<{skillId, source, installedAt}> not string[]", async () => {
const db = await getSkillRegistryDb();
const agentId = "agent-shape-test";
const now = Date.now();
// Insert a managed skill directly
await db.insert(agentSkills).values({
agentId,
skillId: "nexus-skills/code-review",
installedAt: now,
source: "managed",
});
const grpSvc = skillGroupService();
const result = await grpSvc.listAgentSkills(agentId);
expect(Array.isArray(result)).toBe(true);
expect(result.length).toBe(1);
const entry = result[0]!;
// Must have the 3-field object shape
expect(typeof entry.skillId).toBe("string");
expect(typeof entry.source).toBe("string");
expect(typeof entry.installedAt).toBe("number");
// Must NOT be a plain string (old API)
expect(typeof entry).toBe("object");
});
it("Test 6: returns skills with correct source values", async () => {
const db = await getSkillRegistryDb();
const agentId = "agent-source-values";
const now = Date.now();
await db.insert(agentSkills).values([
{ agentId, skillId: "nexus-skills/managed-skill", installedAt: now, source: "managed" },
{ agentId, skillId: "hermes-native/native-skill", installedAt: now, source: "native" },
]);
const grpSvc = skillGroupService();
const result = await grpSvc.listAgentSkills(agentId);
expect(result.length).toBe(2);
const managedEntry = result.find((r) => r.skillId === "nexus-skills/managed-skill");
const nativeEntry = result.find((r) => r.skillId === "hermes-native/native-skill");
expect(managedEntry?.source).toBe("managed");
expect(nativeEntry?.source).toBe("native");
});
it("Test 7: listAgentSkills includes both managed and native skills for a Hermes agent", async () => {
await createFakeHermesSkills(["native-skill"]);
const agentId = "agent-hermes-combined";
const db = await getSkillRegistryDb();
const now = Date.now();
// Insert a managed skill
await db.insert(agentSkills).values({
agentId,
skillId: "nexus-skills/managed",
installedAt: now,
source: "managed",
});
// Sync native skills
const svc = skillRegistryService();
await svc.syncHermesNativeSkills(agentId);
const grpSvc = skillGroupService();
const result = await grpSvc.listAgentSkills(agentId);
expect(result.length).toBe(2);
const managed = result.filter((r) => r.source === "managed");
const native = result.filter((r) => r.source === "native");
expect(managed.length).toBe(1);
expect(native.length).toBe(1);
expect(managed[0]!.skillId).toBe("nexus-skills/managed");
expect(native[0]!.skillId).toBe("hermes-native/native-skill");
});
});

View file

@ -0,0 +1,295 @@
/**
* Tests for adapter-aware install/uninstall/rollback/assignGroup/removeGroup.
* Verifies that all file operations use the caller-supplied agentSkillsDir
* rather than any hardcoded path.
*
* Covers: INST-01, INST-02, INST-03, INST-04
*/
import { describe, it, expect, beforeEach, afterEach } from "vitest";
import { mkdtemp, rm, mkdir, writeFile, readdir } from "node:fs/promises";
import { existsSync } from "node:fs";
import path from "node:path";
import os from "node:os";
import { getSkillRegistryDb, resetSkillRegistryDb } from "../services/skill-registry-db.js";
import { skills, skillVersions, skillFiles, agentSkills } from "../services/skill-registry-schema.js";
import { skillRegistryService } from "../services/skill-registry.js";
import { skillGroupService } from "../services/skill-registry-groups.js";
// ---------------------------------------------------------------------------
// Test helpers
// ---------------------------------------------------------------------------
let tmpHome: string;
let tmpAgentSkillsDir: string;
beforeEach(async () => {
tmpHome = await mkdtemp(path.join(os.tmpdir(), "nexus-adapter-install-test-"));
tmpAgentSkillsDir = path.join(tmpHome, "agent-skills");
await mkdir(tmpAgentSkillsDir, { recursive: true });
process.env.PAPERCLIP_HOME = tmpHome;
resetSkillRegistryDb();
});
afterEach(async () => {
resetSkillRegistryDb();
delete process.env.PAPERCLIP_HOME;
await rm(tmpHome, { recursive: true, force: true });
});
async function seedSkillWithVersion(opts: {
skillId: string;
sourceId: string;
versionId: string;
cacheDir?: string;
fileKind?: string;
}): Promise<void> {
const db = await getSkillRegistryDb();
const now = Date.now();
await db.insert(skills).values({
id: opts.skillId,
sourceId: opts.sourceId,
name: "Test Skill",
description: null,
sourceUrl: null,
activeVersionId: null,
removedAt: null,
createdAt: now,
updatedAt: now,
});
await db.insert(skillVersions).values({
id: opts.versionId,
skillId: opts.skillId,
version: "abc123",
fetchedAt: now,
cacheDir: opts.cacheDir ?? null,
});
await db.insert(skillFiles).values({
id: `file-${opts.versionId}`,
versionId: opts.versionId,
path: "SKILL.md",
kind: opts.fileKind ?? "skill",
sizeBytes: 100,
});
}
async function createFakeCacheDir(cacheDir: string, content = "# Test Skill"): Promise<void> {
await mkdir(cacheDir, { recursive: true });
await writeFile(path.join(cacheDir, "SKILL.md"), content, "utf-8");
}
// ---------------------------------------------------------------------------
// install() — writes to provided agentSkillsDir
// ---------------------------------------------------------------------------
describe("install() — adapter-aware path", () => {
it("Test 1: writes skill files to the provided agentSkillsDir, not a hardcoded path", async () => {
const skillId = "nexus-skills/typescript-review";
const versionId = `${skillId}@abc123`;
const cacheDir = path.join(tmpHome, "cache", "typescript-review");
await createFakeCacheDir(cacheDir);
await seedSkillWithVersion({ skillId, sourceId: "nexus-skills", versionId, cacheDir });
const svc = skillRegistryService();
const result = await svc.install(skillId, tmpAgentSkillsDir);
expect(result.type).toBe("installed");
if (result.type === "installed") {
// Files must land inside tmpAgentSkillsDir, not any hardcoded path
expect(result.targetDir).toContain(tmpAgentSkillsDir);
expect(existsSync(path.join(result.targetDir, "SKILL.md"))).toBe(true);
}
});
it("Test 2: slug (last segment of skillId) is used as subdirectory name", async () => {
const skillId = "nexus-skills/code-review";
const versionId = `${skillId}@sha1`;
const cacheDir = path.join(tmpHome, "cache", "code-review");
await createFakeCacheDir(cacheDir);
await seedSkillWithVersion({ skillId, sourceId: "nexus-skills", versionId, cacheDir });
const svc = skillRegistryService();
const result = await svc.install(skillId, tmpAgentSkillsDir);
expect(result.type).toBe("installed");
if (result.type === "installed") {
expect(result.targetDir).toBe(path.join(tmpAgentSkillsDir, "code-review"));
}
});
});
// ---------------------------------------------------------------------------
// uninstall() — removes files AND soft-deletes DB row
// ---------------------------------------------------------------------------
describe("uninstall() — file removal + soft-delete", () => {
it("Test 3: removes skill directory from disk", async () => {
const skillId = "nexus-skills/code-review";
const versionId = `${skillId}@abc123`;
const cacheDir = path.join(tmpHome, "cache", "code-review");
await createFakeCacheDir(cacheDir);
await seedSkillWithVersion({ skillId, sourceId: "nexus-skills", versionId, cacheDir });
const svc = skillRegistryService();
// First install so files are on disk
await svc.install(skillId, tmpAgentSkillsDir);
const targetDir = path.join(tmpAgentSkillsDir, "code-review");
expect(existsSync(targetDir)).toBe(true);
// Now uninstall — should remove files
await svc.uninstall(skillId, tmpAgentSkillsDir);
expect(existsSync(targetDir)).toBe(false);
});
it("Test 4: soft-deletes the DB row (sets removedAt)", async () => {
const skillId = "nexus-skills/code-review";
const versionId = `${skillId}@abc123`;
const cacheDir = path.join(tmpHome, "cache", "code-review");
await createFakeCacheDir(cacheDir);
await seedSkillWithVersion({ skillId, sourceId: "nexus-skills", versionId, cacheDir });
const svc = skillRegistryService();
const before = Date.now();
await svc.uninstall(skillId, tmpAgentSkillsDir);
const after = Date.now();
const db = await getSkillRegistryDb();
const { eq } = await import("drizzle-orm");
const rows = await db.select().from(skills).where(eq(skills.id, skillId));
expect(rows[0]?.removedAt).toBeGreaterThanOrEqual(before);
expect(rows[0]?.removedAt).toBeLessThanOrEqual(after);
});
it("Test 5: uninstall with non-existent directory does not throw (force: true)", async () => {
const skillId = "nexus-skills/missing-skill";
await seedSkillWithVersion({
skillId,
sourceId: "nexus-skills",
versionId: `${skillId}@sha1`,
});
const svc = skillRegistryService();
// targetDir never existed — should not throw
await expect(svc.uninstall(skillId, tmpAgentSkillsDir)).resolves.toBeUndefined();
});
it("Test 6: uninstall removes files from the correct path (slug-based subdirectory)", async () => {
const skillId = "my-source/my-skill-name";
const versionId = `${skillId}@abc123`;
const cacheDir = path.join(tmpHome, "cache", "my-skill-name");
await createFakeCacheDir(cacheDir);
await seedSkillWithVersion({ skillId, sourceId: "my-source", versionId, cacheDir });
const svc = skillRegistryService();
await svc.install(skillId, tmpAgentSkillsDir);
// Confirm the correct subdirectory was created
const expectedDir = path.join(tmpAgentSkillsDir, "my-skill-name");
expect(existsSync(expectedDir)).toBe(true);
await svc.uninstall(skillId, tmpAgentSkillsDir);
expect(existsSync(expectedDir)).toBe(false);
});
});
// ---------------------------------------------------------------------------
// rollback() — restores files to provided agentSkillsDir
// ---------------------------------------------------------------------------
describe("rollback() — restores to provided agentSkillsDir", () => {
it("Test 7: restores files to the provided agentSkillsDir (not a hardcoded path)", async () => {
const skillId = "nexus-skills/code-review";
const v1Id = `${skillId}@v1`;
const v2Id = `${skillId}@v2`;
const v1CacheDir = path.join(tmpHome, "cache", skillId, "v1");
const v2CacheDir = path.join(tmpHome, "cache", skillId, "v2");
await createFakeCacheDir(v1CacheDir, "# Version 1");
await createFakeCacheDir(v2CacheDir, "# Version 2");
const db = await getSkillRegistryDb();
const now = Date.now();
await db.insert(skills).values({
id: skillId, sourceId: "nexus-skills", name: "Code Review",
description: null, sourceUrl: null, activeVersionId: v2Id,
removedAt: null, createdAt: now, updatedAt: now,
});
await db.insert(skillVersions).values({ id: v1Id, skillId, version: "v1", fetchedAt: now - 1000, cacheDir: v1CacheDir });
await db.insert(skillVersions).values({ id: v2Id, skillId, version: "v2", fetchedAt: now, cacheDir: v2CacheDir });
await db.insert(skillFiles).values({ id: "f1", versionId: v1Id, path: "SKILL.md", kind: "skill", sizeBytes: 12 });
await db.insert(skillFiles).values({ id: "f2", versionId: v2Id, path: "SKILL.md", kind: "skill", sizeBytes: 12 });
// Pre-install v2
const targetDir = path.join(tmpAgentSkillsDir, "code-review");
await mkdir(targetDir, { recursive: true });
await writeFile(path.join(targetDir, "SKILL.md"), "# Version 2", "utf-8");
// Rollback to v1 using provided agentSkillsDir
const svc = skillRegistryService();
await svc.rollback(skillId, v1Id, tmpAgentSkillsDir);
// Files must be in tmpAgentSkillsDir, not any hardcoded path
const { readFileSync } = await import("node:fs");
const content = readFileSync(path.join(tmpAgentSkillsDir, "code-review", "SKILL.md"), "utf-8");
expect(content).toBe("# Version 1");
});
});
// ---------------------------------------------------------------------------
// assignGroup() / removeGroup() — use provided agentSkillsDir
// ---------------------------------------------------------------------------
describe("assignGroup() — uses provided agentSkillsDir", () => {
it("Test 8: assignGroup installs skills into the provided agentSkillsDir", async () => {
const skillId = "nexus-skills/code-review";
const versionId = `${skillId}@abc123`;
const cacheDir = path.join(tmpHome, "cache", "code-review");
await createFakeCacheDir(cacheDir);
await seedSkillWithVersion({ skillId, sourceId: "nexus-skills", versionId, cacheDir });
const db = await getSkillRegistryDb();
const now = Date.now();
const groupId = "custom/test-group";
await db.insert((await import("../services/skill-registry-schema.js")).skillGroups).values({
id: groupId, name: "Test Group", description: null, isBuiltin: 0, createdAt: now, updatedAt: now,
});
await db.insert((await import("../services/skill-registry-schema.js")).skillGroupMembers).values({
groupId, skillId, addedAt: now,
});
const grpSvc = skillGroupService();
const result = await grpSvc.assignGroup(groupId, "agent-123", tmpAgentSkillsDir);
// The skill should have been installed into our provided dir
expect(result.installed).toContain(skillId);
expect(existsSync(path.join(tmpAgentSkillsDir, "code-review", "SKILL.md"))).toBe(true);
});
});
describe("removeGroup() — uses provided agentSkillsDir", () => {
it("Test 9: removeGroup removes skill files from the provided agentSkillsDir", async () => {
const skillId = "nexus-skills/code-review";
const versionId = `${skillId}@abc123`;
const cacheDir = path.join(tmpHome, "cache", "code-review");
await createFakeCacheDir(cacheDir);
await seedSkillWithVersion({ skillId, sourceId: "nexus-skills", versionId, cacheDir });
const db = await getSkillRegistryDb();
const now = Date.now();
const groupId = "custom/test-group";
const agentId = "agent-xyz";
const { skillGroups: grps, skillGroupMembers: grpMembers } = await import("../services/skill-registry-schema.js");
await db.insert(grps).values({
id: groupId, name: "Test Group", description: null, isBuiltin: 0, createdAt: now, updatedAt: now,
});
await db.insert(grpMembers).values({ groupId, skillId, addedAt: now });
const grpSvc = skillGroupService();
// Assign group (installs skills)
await grpSvc.assignGroup(groupId, agentId, tmpAgentSkillsDir);
expect(existsSync(path.join(tmpAgentSkillsDir, "code-review"))).toBe(true);
// Remove group — should remove files from provided dir
await grpSvc.removeGroup(groupId, agentId, tmpAgentSkillsDir);
expect(existsSync(path.join(tmpAgentSkillsDir, "code-review"))).toBe(false);
});
});

View file

@ -157,7 +157,7 @@ describe("skillRegistryService", () => {
await seedSkillWithVersion({ skillId, sourceId: "schwepps-skills", versionId, cacheDir });
const before = Date.now();
await svc.uninstall(skillId);
await svc.uninstall(skillId, tmpAgentSkillsDir);
const after = Date.now();
const db = await getSkillRegistryDb();
@ -174,7 +174,7 @@ describe("skillRegistryService", () => {
await createFakeCacheDir(cacheDir);
await seedSkillWithVersion({ skillId, sourceId: "schwepps-skills", versionId, cacheDir });
await svc.uninstall(skillId);
await svc.uninstall(skillId, tmpAgentSkillsDir);
// Not visible in normal list
const normalList = await svc.list();

View file

@ -370,19 +370,12 @@ export function skillGroupService() {
for (const sid of effective) stillNeeded.add(sid);
}
// Individually installed skills (not from a group) — these should be preserved
const individualRows = await db
.select()
.from(agentSkills)
.where(eq(agentSkills.agentId, agentId));
const individualSkills = new Set(individualRows.map((r) => r.skillId));
// Find skills that were contributed by the removed group
const removedGroupSkills = await this.resolveEffectiveSkills(groupId);
for (const skillId of removedGroupSkills) {
// Skip if still needed by another group or individually installed
if (stillNeeded.has(skillId) || individualSkills.has(skillId)) continue;
// Skip if still needed by another remaining group
if (stillNeeded.has(skillId)) continue;
// Remove files from agent skills directory
const slug = skillId.split("/").pop() ?? skillId;