feat(FN-1510): add skill resolver and createKbAgent override tests
This commit is contained in:
gsxdsm
@@ -20,7 +20,7 @@ const setFallbackResolverMock = vi.fn();
|
||||
const reloadMock = vi.fn(async () => {});
|
||||
const execSyncMock = vi.fn((_cmd?: any, _opts?: any) => "");
|
||||
const existsSyncMock = vi.fn((_path: PathLike) => false);
|
||||
const readFileSyncMock = vi.fn(() => "{}");
|
||||
const readFileSyncMock = vi.fn((_path?: any) => "{}");
|
||||
|
||||
// Route async `exec` through the `execSync` mock so the promisify bridge works.
|
||||
vi.mock("node:child_process", async () => {
|
||||
@@ -538,4 +538,284 @@ describe("createKbAgent", () => {
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
describe("skill selection", () => {
|
||||
beforeEach(() => {
|
||||
// Reset modules to ensure fresh imports for each test
|
||||
vi.resetModules();
|
||||
});
|
||||
|
||||
it("without skillSelection does not pass skillsOverride to resource loader", async () => {
|
||||
let capturedResourceLoaderOptions: any;
|
||||
vi.doMock("@mariozechner/pi-coding-agent", () => ({
|
||||
AuthStorage: {
|
||||
create: () => ({
|
||||
setFallbackResolver: setFallbackResolverMock,
|
||||
}),
|
||||
},
|
||||
createAgentSession: createAgentSessionMock,
|
||||
createCodingTools: createCodingToolsMock,
|
||||
createExtensionRuntime: createExtensionRuntimeMock,
|
||||
createReadOnlyTools: createReadOnlyToolsMock,
|
||||
DefaultResourceLoader: class {
|
||||
constructor(options: any) {
|
||||
capturedResourceLoaderOptions = options;
|
||||
}
|
||||
async reload() {
|
||||
await reloadMock();
|
||||
}
|
||||
},
|
||||
DefaultPackageManager: class {
|
||||
async resolve() {
|
||||
return packageManagerResolveMock();
|
||||
}
|
||||
},
|
||||
discoverAndLoadExtensions: discoverAndLoadExtensionsMock,
|
||||
getAgentDir: () => "/mock-agent-dir",
|
||||
ModelRegistry: class {
|
||||
find(provider: string, modelId: string) {
|
||||
return findMock(provider, modelId);
|
||||
}
|
||||
getAll() {
|
||||
return getAllMock();
|
||||
}
|
||||
registerProvider(name: string, config: unknown) {
|
||||
return registerProviderMock(name, config);
|
||||
}
|
||||
refresh() {
|
||||
return refreshMock();
|
||||
}
|
||||
},
|
||||
SessionManager: {
|
||||
inMemory: () => ({ kind: "session-manager" }),
|
||||
},
|
||||
SettingsManager: {
|
||||
create: settingsManagerCreateMock,
|
||||
inMemory: settingsManagerInMemoryMock,
|
||||
},
|
||||
}));
|
||||
|
||||
const { createKbAgent: freshCreateKbAgent } = await import("./pi.js");
|
||||
|
||||
await freshCreateKbAgent({
|
||||
cwd: "/tmp",
|
||||
systemPrompt: "test",
|
||||
tools: "coding",
|
||||
});
|
||||
|
||||
// skillsOverride should not be present when skillSelection is not provided
|
||||
expect(capturedResourceLoaderOptions.skillsOverride).toBeUndefined();
|
||||
});
|
||||
|
||||
it("with skillSelection (empty patterns, no requested names) passes through all skills (filter not active)", async () => {
|
||||
// Mock existsSync to return true for settings file
|
||||
existsSyncMock.mockImplementation((path) => {
|
||||
const value = String(path);
|
||||
return value.includes(".fusion/settings.json");
|
||||
});
|
||||
readFileSyncMock.mockImplementation((path) => {
|
||||
const value = String(path);
|
||||
if (value.includes(".fusion/settings.json")) {
|
||||
return JSON.stringify({});
|
||||
}
|
||||
return "{}";
|
||||
});
|
||||
|
||||
const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
|
||||
let capturedResourceLoaderOptions: any;
|
||||
vi.doMock("@mariozechner/pi-coding-agent", () => ({
|
||||
AuthStorage: {
|
||||
create: () => ({
|
||||
setFallbackResolver: setFallbackResolverMock,
|
||||
}),
|
||||
},
|
||||
createAgentSession: createAgentSessionMock,
|
||||
createCodingTools: createCodingToolsMock,
|
||||
createExtensionRuntime: createExtensionRuntimeMock,
|
||||
createReadOnlyTools: createReadOnlyToolsMock,
|
||||
DefaultResourceLoader: class {
|
||||
constructor(options: any) {
|
||||
capturedResourceLoaderOptions = options;
|
||||
}
|
||||
async reload() {
|
||||
await reloadMock();
|
||||
}
|
||||
},
|
||||
DefaultPackageManager: class {
|
||||
async resolve() {
|
||||
return packageManagerResolveMock();
|
||||
}
|
||||
},
|
||||
discoverAndLoadExtensions: discoverAndLoadExtensionsMock,
|
||||
getAgentDir: () => "/mock-agent-dir",
|
||||
ModelRegistry: class {
|
||||
find(provider: string, modelId: string) {
|
||||
return findMock(provider, modelId);
|
||||
}
|
||||
getAll() {
|
||||
return getAllMock();
|
||||
}
|
||||
registerProvider(name: string, config: unknown) {
|
||||
return registerProviderMock(name, config);
|
||||
}
|
||||
refresh() {
|
||||
return refreshMock();
|
||||
}
|
||||
},
|
||||
SessionManager: {
|
||||
inMemory: () => ({ kind: "session-manager" }),
|
||||
},
|
||||
SettingsManager: {
|
||||
create: settingsManagerCreateMock,
|
||||
inMemory: settingsManagerInMemoryMock,
|
||||
},
|
||||
}));
|
||||
|
||||
const { createKbAgent: freshCreateKbAgent } = await import("./pi.js");
|
||||
|
||||
await freshCreateKbAgent({
|
||||
cwd: "/tmp",
|
||||
systemPrompt: "test",
|
||||
tools: "coding",
|
||||
skillSelection: {
|
||||
projectRootDir: "/tmp",
|
||||
},
|
||||
});
|
||||
|
||||
// When filterActive is false, skillsOverride returns base unchanged
|
||||
// The callback should exist but simply return the base skills
|
||||
if (capturedResourceLoaderOptions.skillsOverride) {
|
||||
const result = capturedResourceLoaderOptions.skillsOverride({
|
||||
skills: [{ name: "test", filePath: "/path", description: "", baseDir: "", sourceInfo: {} as any, disableModelInvocation: false }],
|
||||
diagnostics: [],
|
||||
});
|
||||
expect(result.skills).toHaveLength(1); // All skills pass through
|
||||
}
|
||||
|
||||
consoleErrorSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("with skillSelection (specific requested names) activates skill filtering", async () => {
|
||||
const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
|
||||
let capturedResourceLoaderOptions: any;
|
||||
vi.doMock("@mariozechner/pi-coding-agent", () => ({
|
||||
AuthStorage: {
|
||||
create: () => ({
|
||||
setFallbackResolver: setFallbackResolverMock,
|
||||
}),
|
||||
},
|
||||
createAgentSession: createAgentSessionMock,
|
||||
createCodingTools: createCodingToolsMock,
|
||||
createExtensionRuntime: createExtensionRuntimeMock,
|
||||
createReadOnlyTools: createReadOnlyToolsMock,
|
||||
DefaultResourceLoader: class {
|
||||
constructor(options: any) {
|
||||
capturedResourceLoaderOptions = options;
|
||||
}
|
||||
async reload() {
|
||||
await reloadMock();
|
||||
}
|
||||
},
|
||||
DefaultPackageManager: class {
|
||||
async resolve() {
|
||||
return packageManagerResolveMock();
|
||||
}
|
||||
},
|
||||
discoverAndLoadExtensions: discoverAndLoadExtensionsMock,
|
||||
getAgentDir: () => "/mock-agent-dir",
|
||||
ModelRegistry: class {
|
||||
find(provider: string, modelId: string) {
|
||||
return findMock(provider, modelId);
|
||||
}
|
||||
getAll() {
|
||||
return getAllMock();
|
||||
}
|
||||
registerProvider(name: string, config: unknown) {
|
||||
return registerProviderMock(name, config);
|
||||
}
|
||||
refresh() {
|
||||
return refreshMock();
|
||||
}
|
||||
},
|
||||
SessionManager: {
|
||||
inMemory: () => ({ kind: "session-manager" }),
|
||||
},
|
||||
SettingsManager: {
|
||||
create: settingsManagerCreateMock,
|
||||
inMemory: settingsManagerInMemoryMock,
|
||||
},
|
||||
}));
|
||||
|
||||
const { createKbAgent: freshCreateKbAgent } = await import("./pi.js");
|
||||
|
||||
await freshCreateKbAgent({
|
||||
cwd: "/tmp",
|
||||
systemPrompt: "test",
|
||||
tools: "coding",
|
||||
skillSelection: {
|
||||
projectRootDir: "/tmp",
|
||||
requestedSkillNames: ["paperclip"],
|
||||
sessionPurpose: "executor",
|
||||
},
|
||||
});
|
||||
|
||||
// skillsOverride should be present
|
||||
expect(capturedResourceLoaderOptions.skillsOverride).toBeDefined();
|
||||
|
||||
// The override should filter skills
|
||||
const result = capturedResourceLoaderOptions.skillsOverride({
|
||||
skills: [
|
||||
{ name: "paperclip", filePath: "/path/paperclip", description: "", baseDir: "", sourceInfo: {} as any, disableModelInvocation: false },
|
||||
{ name: "lint", filePath: "/path/lint", description: "", baseDir: "", sourceInfo: {} as any, disableModelInvocation: false },
|
||||
],
|
||||
diagnostics: [],
|
||||
});
|
||||
|
||||
// Only paperclip should pass through (matching requested name)
|
||||
expect(result.skills).toHaveLength(1);
|
||||
expect(result.skills[0].name).toBe("paperclip");
|
||||
|
||||
consoleErrorSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("diagnostics are logged via console.error with [pi] [skills] prefix", async () => {
|
||||
const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
|
||||
// Test diagnostics logging by directly calling createSkillsOverrideFromSelection
|
||||
const { createSkillsOverrideFromSelection } = await import("./skill-resolver.js");
|
||||
|
||||
const selection = {
|
||||
allowedSkillPaths: new Set(["/path/nonexistent"]),
|
||||
diagnostics: [],
|
||||
filterActive: true,
|
||||
};
|
||||
|
||||
const override = createSkillsOverrideFromSelection(selection, {
|
||||
sessionPurpose: "executor",
|
||||
});
|
||||
|
||||
// Invoke the override to trigger diagnostics
|
||||
const result = override({
|
||||
skills: [],
|
||||
diagnostics: [],
|
||||
});
|
||||
|
||||
// Check that diagnostics were produced
|
||||
expect(result.diagnostics.length).toBeGreaterThan(0);
|
||||
|
||||
// Check that diagnostics were logged with correct prefix
|
||||
const skillLogs = consoleErrorSpy.mock.calls.filter(call =>
|
||||
String(call[0]).includes("[pi] [skills]")
|
||||
);
|
||||
expect(skillLogs.length).toBeGreaterThan(0);
|
||||
|
||||
// Should include the session purpose
|
||||
const lastLog = skillLogs[skillLogs.length - 1][0] as string;
|
||||
expect(lastLog).toContain("[executor]");
|
||||
|
||||
consoleErrorSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
544
packages/engine/src/skill-resolver.test.ts
Normal file
544
packages/engine/src/skill-resolver.test.ts
Normal file
@@ -0,0 +1,544 @@
|
||||
/**
|
||||
* Unit tests for skill resolver.
|
||||
*/
|
||||
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
resolveSessionSkills,
|
||||
createSkillsOverrideFromSelection,
|
||||
type SkillSelectionResult,
|
||||
} from "./skill-resolver.js";
|
||||
|
||||
// ── Mock Setup ───────────────────────────────────────────────────────────────
|
||||
|
||||
// In-memory file system for tests - using a proxy to intercept fs calls
|
||||
const mockFiles = new Map<string, string>();
|
||||
let mockDirCounter = 0;
|
||||
|
||||
vi.mock("node:fs", async () => {
|
||||
const actual = await vi.importActual<typeof import("node:fs")>("node:fs");
|
||||
return {
|
||||
...actual,
|
||||
existsSync: (path: unknown) => mockFiles.has(String(path)),
|
||||
readFileSync: (path: unknown) => mockFiles.get(String(path)) ?? "{}",
|
||||
mkdtempSync: () => `/tmp/skill-resolver-mock-${++mockDirCounter}`,
|
||||
writeFileSync: (path: unknown, content: unknown) => mockFiles.set(String(path), String(content)),
|
||||
rmSync: (path: unknown) => {
|
||||
const pathStr = String(path);
|
||||
for (const key of mockFiles.keys()) {
|
||||
if (key.startsWith(pathStr)) mockFiles.delete(key);
|
||||
}
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
// ── Test Helpers ─────────────────────────────────────────────────────────────
|
||||
|
||||
function createMockProjectDir(settings: Record<string, unknown> | null): string {
|
||||
const dir = `/tmp/skill-resolver-mock-${++mockDirCounter}`;
|
||||
if (settings !== null) {
|
||||
mockFiles.set(`${dir}/.fusion/settings.json`, JSON.stringify(settings));
|
||||
}
|
||||
return dir;
|
||||
}
|
||||
|
||||
// ── Tests ───────────────────────────────────────────────────────────────────
|
||||
|
||||
describe("resolveSessionSkills", () => {
|
||||
beforeEach(() => {
|
||||
mockFiles.clear();
|
||||
mockDirCounter = 0;
|
||||
});
|
||||
|
||||
describe("returns filterActive: false when no patterns and no requested names", () => {
|
||||
it("returns filterActive: false when no settings file exists", () => {
|
||||
const result = resolveSessionSkills({
|
||||
projectRootDir: "/nonexistent",
|
||||
});
|
||||
|
||||
expect(result.filterActive).toBe(false);
|
||||
expect(result.allowedSkillPaths.size).toBe(0);
|
||||
expect(result.diagnostics).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("returns filterActive: false when settings file is empty", () => {
|
||||
const dir = createMockProjectDir({});
|
||||
const result = resolveSessionSkills({
|
||||
projectRootDir: dir,
|
||||
});
|
||||
|
||||
expect(result.filterActive).toBe(false);
|
||||
expect(result.allowedSkillPaths.size).toBe(0);
|
||||
expect(result.diagnostics).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("returns filterActive: false when settings has no skill configuration", () => {
|
||||
const dir = createMockProjectDir({
|
||||
defaultProvider: "anthropic",
|
||||
defaultModel: "claude-sonnet-4-5",
|
||||
});
|
||||
const result = resolveSessionSkills({
|
||||
projectRootDir: dir,
|
||||
});
|
||||
|
||||
expect(result.filterActive).toBe(false);
|
||||
expect(result.allowedSkillPaths.size).toBe(0);
|
||||
expect(result.diagnostics).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe("returns filterActive: true with + patterns", () => {
|
||||
it("adds skill paths to allowed set with + prefix", () => {
|
||||
const dir = createMockProjectDir({
|
||||
skills: ["+skills/paperclip/SKILL.md", "+skills/lint/SKILL.md"],
|
||||
});
|
||||
|
||||
const result = resolveSessionSkills({
|
||||
projectRootDir: dir,
|
||||
});
|
||||
|
||||
expect(result.filterActive).toBe(true);
|
||||
expect(result.allowedSkillPaths.size).toBe(2);
|
||||
expect(result.allowedSkillPaths.has("skills/paperclip/SKILL.md")).toBe(true);
|
||||
expect(result.allowedSkillPaths.has("skills/lint/SKILL.md")).toBe(true);
|
||||
});
|
||||
|
||||
it("adds skill paths to allowed set without prefix (implicit +)", () => {
|
||||
const dir = createMockProjectDir({
|
||||
skills: ["skills/paperclip/SKILL.md", "skills/lint/SKILL.md"],
|
||||
});
|
||||
|
||||
const result = resolveSessionSkills({
|
||||
projectRootDir: dir,
|
||||
});
|
||||
|
||||
expect(result.filterActive).toBe(true);
|
||||
expect(result.allowedSkillPaths.size).toBe(2);
|
||||
});
|
||||
});
|
||||
|
||||
describe("excludes - pattern skills from allowed set", () => {
|
||||
it("removes skill from allowed set with - prefix", () => {
|
||||
const dir = createMockProjectDir({
|
||||
skills: ["+skills/foo/SKILL.md", "+skills/bar/SKILL.md", "-skills/foo/SKILL.md"],
|
||||
});
|
||||
|
||||
const result = resolveSessionSkills({
|
||||
projectRootDir: dir,
|
||||
});
|
||||
|
||||
expect(result.filterActive).toBe(true);
|
||||
expect(result.allowedSkillPaths.size).toBe(1);
|
||||
expect(result.allowedSkillPaths.has("skills/foo/SKILL.md")).toBe(false);
|
||||
expect(result.allowedSkillPaths.has("skills/bar/SKILL.md")).toBe(true);
|
||||
});
|
||||
|
||||
it("exclusion pattern removes previously added entry", () => {
|
||||
const dir = createMockProjectDir({
|
||||
skills: ["skills/foo/SKILL.md", "-skills/foo/SKILL.md"],
|
||||
});
|
||||
|
||||
const result = resolveSessionSkills({
|
||||
projectRootDir: dir,
|
||||
});
|
||||
|
||||
expect(result.allowedSkillPaths.size).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe("handles mixed + / - patterns correctly", () => {
|
||||
it("last entry wins for duplicate paths", () => {
|
||||
const dir = createMockProjectDir({
|
||||
skills: ["+skills/foo/SKILL.md", "-skills/foo/SKILL.md", "+skills/foo/SKILL.md"],
|
||||
});
|
||||
|
||||
const result = resolveSessionSkills({
|
||||
projectRootDir: dir,
|
||||
});
|
||||
|
||||
// Last + wins
|
||||
expect(result.allowedSkillPaths.has("skills/foo/SKILL.md")).toBe(true);
|
||||
});
|
||||
|
||||
it("last entry wins (exclusion after inclusion)", () => {
|
||||
const dir = createMockProjectDir({
|
||||
skills: ["+skills/foo/SKILL.md", "+skills/foo/SKILL.md", "-skills/foo/SKILL.md"],
|
||||
});
|
||||
|
||||
const result = resolveSessionSkills({
|
||||
projectRootDir: dir,
|
||||
});
|
||||
|
||||
// Last - wins
|
||||
expect(result.allowedSkillPaths.has("skills/foo/SKILL.md")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("handles requestedSkillNames", () => {
|
||||
it("with no patterns, only requested names marks filterActive: true", () => {
|
||||
const result = resolveSessionSkills({
|
||||
projectRootDir: "/nonexistent",
|
||||
requestedSkillNames: ["paperclip", "lint"],
|
||||
});
|
||||
|
||||
expect(result.filterActive).toBe(true);
|
||||
expect(result.diagnostics).toHaveLength(2);
|
||||
expect(result.diagnostics.some(d => d.skillName === "paperclip")).toBe(true);
|
||||
expect(result.diagnostics.some(d => d.skillName === "lint")).toBe(true);
|
||||
});
|
||||
|
||||
it("requestedSkillNames act as info diagnostics when no patterns exist", () => {
|
||||
const result = resolveSessionSkills({
|
||||
projectRootDir: "/nonexistent",
|
||||
requestedSkillNames: ["custom-skill"],
|
||||
});
|
||||
|
||||
expect(result.filterActive).toBe(true);
|
||||
const nameDiags = result.diagnostics.filter(d => d.skillName === "custom-skill");
|
||||
expect(nameDiags).toHaveLength(1);
|
||||
expect(nameDiags[0].type).toBe("info");
|
||||
});
|
||||
});
|
||||
|
||||
describe("package-scoped skill patterns", () => {
|
||||
it("extracts skills from package objects with skills array", () => {
|
||||
const dir = createMockProjectDir({
|
||||
packages: [
|
||||
{
|
||||
source: "@myorg/ai-kit",
|
||||
skills: ["+skills/custom/SKILL.md"],
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
const result = resolveSessionSkills({
|
||||
projectRootDir: dir,
|
||||
});
|
||||
|
||||
expect(result.filterActive).toBe(true);
|
||||
expect(result.allowedSkillPaths.has("skills/custom/SKILL.md")).toBe(true);
|
||||
});
|
||||
|
||||
it("handles mixed top-level and package-scoped patterns", () => {
|
||||
const dir = createMockProjectDir({
|
||||
skills: ["+skills/shared/SKILL.md"],
|
||||
packages: [
|
||||
{
|
||||
source: "@myorg/ai-kit",
|
||||
skills: ["+skills/package-skill/SKILL.md"],
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
const result = resolveSessionSkills({
|
||||
projectRootDir: dir,
|
||||
});
|
||||
|
||||
expect(result.filterActive).toBe(true);
|
||||
expect(result.allowedSkillPaths.size).toBe(2);
|
||||
expect(result.allowedSkillPaths.has("skills/shared/SKILL.md")).toBe(true);
|
||||
expect(result.allowedSkillPaths.has("skills/package-skill/SKILL.md")).toBe(true);
|
||||
});
|
||||
|
||||
it("handles string package entries without crashing", () => {
|
||||
const dir = createMockProjectDir({
|
||||
packages: ["@myorg/ai-kit"],
|
||||
});
|
||||
|
||||
const result = resolveSessionSkills({
|
||||
projectRootDir: dir,
|
||||
});
|
||||
|
||||
// Should not crash, patterns array is undefined for string entries
|
||||
expect(result.filterActive).toBe(false);
|
||||
expect(result.allowedSkillPaths.size).toBe(0);
|
||||
});
|
||||
|
||||
it("handles package objects without skills array", () => {
|
||||
const dir = createMockProjectDir({
|
||||
packages: [
|
||||
{
|
||||
source: "@myorg/ai-kit",
|
||||
extensions: ["dist/index.js"],
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
const result = resolveSessionSkills({
|
||||
projectRootDir: dir,
|
||||
});
|
||||
|
||||
// No skill patterns exist (package has extensions, not skills)
|
||||
expect(result.filterActive).toBe(false);
|
||||
expect(result.allowedSkillPaths.size).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe("reads from .fusion/settings.json primary and .pi/settings.json fallback", () => {
|
||||
it("prefers .fusion/settings.json over .pi/settings.json", () => {
|
||||
const dir = createMockProjectDir(null);
|
||||
|
||||
// Create .pi/settings.json (legacy)
|
||||
mockFiles.set(`${dir}/.pi/settings.json`, JSON.stringify({
|
||||
skills: ["+skills/legacy/SKILL.md"],
|
||||
}));
|
||||
|
||||
// Create .fusion/settings.json (newer, should win)
|
||||
mockFiles.set(`${dir}/.fusion/settings.json`, JSON.stringify({
|
||||
skills: ["+skills/fusion/SKILL.md"],
|
||||
}));
|
||||
|
||||
const result = resolveSessionSkills({
|
||||
projectRootDir: dir,
|
||||
});
|
||||
|
||||
expect(result.allowedSkillPaths.has("skills/fusion/SKILL.md")).toBe(true);
|
||||
expect(result.allowedSkillPaths.has("skills/legacy/SKILL.md")).toBe(false);
|
||||
});
|
||||
|
||||
it("falls back to .pi/settings.json when .fusion/settings.json doesn't exist", () => {
|
||||
const dir = createMockProjectDir(null);
|
||||
|
||||
// Create only .pi/settings.json (legacy)
|
||||
mockFiles.set(`${dir}/.pi/settings.json`, JSON.stringify({
|
||||
skills: ["+skills/legacy/SKILL.md"],
|
||||
}));
|
||||
|
||||
const result = resolveSessionSkills({
|
||||
projectRootDir: dir,
|
||||
});
|
||||
|
||||
expect(result.allowedSkillPaths.has("skills/legacy/SKILL.md")).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("handles missing/empty settings files gracefully", () => {
|
||||
it("handles invalid JSON gracefully", () => {
|
||||
const dir = createMockProjectDir(null);
|
||||
|
||||
// Set invalid JSON
|
||||
mockFiles.set(`${dir}/.fusion/settings.json`, "not valid json {{{");
|
||||
|
||||
// Should not throw
|
||||
const result = resolveSessionSkills({
|
||||
projectRootDir: dir,
|
||||
});
|
||||
|
||||
expect(result.filterActive).toBe(false);
|
||||
});
|
||||
|
||||
it("handles malformed settings object gracefully", () => {
|
||||
const dir = createMockProjectDir(null);
|
||||
|
||||
mockFiles.set(`${dir}/.fusion/settings.json`, JSON.stringify({
|
||||
skills: "not an array",
|
||||
packages: "also not an array",
|
||||
}));
|
||||
|
||||
// Should not throw - malformed data treated as no patterns
|
||||
const result = resolveSessionSkills({
|
||||
projectRootDir: dir,
|
||||
});
|
||||
|
||||
expect(result.filterActive).toBe(false);
|
||||
expect(result.allowedSkillPaths.size).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe("produces info diagnostics for patterns", () => {
|
||||
it("produces info diagnostic for each + pattern", () => {
|
||||
const dir = createMockProjectDir({
|
||||
skills: ["+skills/foo/SKILL.md", "-skills/bar/SKILL.md"],
|
||||
});
|
||||
|
||||
const result = resolveSessionSkills({
|
||||
projectRootDir: dir,
|
||||
});
|
||||
|
||||
const infoDiags = result.diagnostics.filter(d => d.type === "info");
|
||||
expect(infoDiags).toHaveLength(1); // Only + pattern gets info diag
|
||||
expect(infoDiags[0].skillPath).toBe("skills/foo/SKILL.md");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("createSkillsOverrideFromSelection", () => {
|
||||
describe("with filterActive: false", () => {
|
||||
it("returns base unchanged", () => {
|
||||
const selection: SkillSelectionResult = {
|
||||
allowedSkillPaths: new Set(),
|
||||
diagnostics: [],
|
||||
filterActive: false,
|
||||
};
|
||||
|
||||
const override = createSkillsOverrideFromSelection(selection);
|
||||
const base = {
|
||||
skills: [
|
||||
{ name: "foo", filePath: "/path/foo", description: "", baseDir: "", sourceInfo: {} as any, disableModelInvocation: false },
|
||||
{ name: "bar", filePath: "/path/bar", description: "", baseDir: "", sourceInfo: {} as any, disableModelInvocation: false },
|
||||
],
|
||||
diagnostics: [],
|
||||
};
|
||||
|
||||
const result = override(base);
|
||||
|
||||
expect(result.skills).toHaveLength(2);
|
||||
expect(result.diagnostics).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe("with filterActive: true", () => {
|
||||
it("filters skills by allowedSkillPaths", () => {
|
||||
const selection: SkillSelectionResult = {
|
||||
allowedSkillPaths: new Set(["/path/foo"]),
|
||||
diagnostics: [],
|
||||
filterActive: true,
|
||||
};
|
||||
|
||||
const override = createSkillsOverrideFromSelection(selection);
|
||||
const base = {
|
||||
skills: [
|
||||
{ name: "foo", filePath: "/path/foo", description: "", baseDir: "", sourceInfo: {} as any, disableModelInvocation: false },
|
||||
{ name: "bar", filePath: "/path/bar", description: "", baseDir: "", sourceInfo: {} as any, disableModelInvocation: false },
|
||||
],
|
||||
diagnostics: [],
|
||||
};
|
||||
|
||||
const result = override(base);
|
||||
|
||||
expect(result.skills).toHaveLength(1);
|
||||
expect(result.skills[0].name).toBe("foo");
|
||||
});
|
||||
|
||||
it("appends warning diagnostic for allowed paths not matching any skill", () => {
|
||||
const selection: SkillSelectionResult = {
|
||||
allowedSkillPaths: new Set(["/path/nonexistent"]),
|
||||
diagnostics: [],
|
||||
filterActive: true,
|
||||
};
|
||||
|
||||
const override = createSkillsOverrideFromSelection(selection);
|
||||
const base = {
|
||||
skills: [
|
||||
{ name: "foo", filePath: "/path/foo", description: "", baseDir: "", sourceInfo: {} as any, disableModelInvocation: false },
|
||||
],
|
||||
diagnostics: [],
|
||||
};
|
||||
|
||||
const result = override(base);
|
||||
|
||||
expect(result.skills).toHaveLength(0);
|
||||
expect(result.diagnostics).toHaveLength(1);
|
||||
expect(result.diagnostics[0].type).toBe("warning");
|
||||
expect(result.diagnostics[0].message).toContain("not found in discovered skills");
|
||||
});
|
||||
|
||||
it("checks requested names against discovered skills (case-insensitive)", () => {
|
||||
const selection: SkillSelectionResult = {
|
||||
allowedSkillPaths: new Set(),
|
||||
diagnostics: [],
|
||||
filterActive: true,
|
||||
};
|
||||
|
||||
const override = createSkillsOverrideFromSelection(selection, {
|
||||
requestedSkillNames: ["PAPERCLIP", "CustomSkill"],
|
||||
sessionPurpose: "test",
|
||||
});
|
||||
|
||||
const base = {
|
||||
skills: [
|
||||
{ name: "paperclip", filePath: "/path/paperclip", description: "", baseDir: "", sourceInfo: {} as any, disableModelInvocation: false },
|
||||
{ name: "lint", filePath: "/path/lint", description: "", baseDir: "", sourceInfo: {} as any, disableModelInvocation: false },
|
||||
],
|
||||
diagnostics: [],
|
||||
};
|
||||
|
||||
const result = override(base);
|
||||
|
||||
expect(result.diagnostics).toHaveLength(1); // Only CustomSkill not found
|
||||
expect(result.diagnostics[0].type).toBe("warning");
|
||||
expect(result.diagnostics[0].message).toContain("CustomSkill");
|
||||
});
|
||||
|
||||
it("preserves base diagnostics alongside new diagnostics", () => {
|
||||
const selection: SkillSelectionResult = {
|
||||
allowedSkillPaths: new Set(["/path/foo"]),
|
||||
diagnostics: [],
|
||||
filterActive: true,
|
||||
};
|
||||
|
||||
const override = createSkillsOverrideFromSelection(selection);
|
||||
const base = {
|
||||
skills: [
|
||||
{ name: "foo", filePath: "/path/foo", description: "", baseDir: "", sourceInfo: {} as any, disableModelInvocation: false },
|
||||
],
|
||||
diagnostics: [
|
||||
{ type: "warning" as const, message: "base warning" },
|
||||
],
|
||||
};
|
||||
|
||||
const result = override(base);
|
||||
|
||||
expect(result.diagnostics).toHaveLength(1);
|
||||
expect(result.diagnostics[0].message).toBe("base warning");
|
||||
});
|
||||
|
||||
it("logs diagnostics via console.error", () => {
|
||||
const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
|
||||
const selection: SkillSelectionResult = {
|
||||
allowedSkillPaths: new Set(["/path/nonexistent"]),
|
||||
diagnostics: [],
|
||||
filterActive: true,
|
||||
};
|
||||
|
||||
const override = createSkillsOverrideFromSelection(selection, {
|
||||
sessionPurpose: "executor",
|
||||
});
|
||||
|
||||
const base = {
|
||||
skills: [],
|
||||
diagnostics: [],
|
||||
};
|
||||
|
||||
override(base);
|
||||
|
||||
expect(consoleErrorSpy).toHaveBeenCalled();
|
||||
const lastCall = consoleErrorSpy.mock.calls[consoleErrorSpy.mock.calls.length - 1][0] as string;
|
||||
expect(lastCall).toContain("[pi] [skills]");
|
||||
expect(lastCall).toContain("nonexistent");
|
||||
|
||||
consoleErrorSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("includes sessionPurpose in log messages when provided", () => {
|
||||
const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
|
||||
const selection: SkillSelectionResult = {
|
||||
allowedSkillPaths: new Set(["/path/foo"]),
|
||||
diagnostics: [],
|
||||
filterActive: true,
|
||||
};
|
||||
|
||||
const override = createSkillsOverrideFromSelection(selection, {
|
||||
requestedSkillNames: ["missing-skill"],
|
||||
sessionPurpose: "reviewer",
|
||||
});
|
||||
|
||||
const base = {
|
||||
skills: [
|
||||
{ name: "foo", filePath: "/path/foo", description: "", baseDir: "", sourceInfo: {} as any, disableModelInvocation: false },
|
||||
],
|
||||
diagnostics: [],
|
||||
};
|
||||
|
||||
override(base);
|
||||
|
||||
const lastCall = consoleErrorSpy.mock.calls[consoleErrorSpy.mock.calls.length - 1][0] as string;
|
||||
expect(lastCall).toContain("[reviewer]");
|
||||
expect(lastCall).toContain("missing-skill");
|
||||
|
||||
consoleErrorSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -206,22 +206,21 @@ export function resolveSessionSkills(context: SkillSelectionContext): SkillSelec
|
||||
}
|
||||
|
||||
// Build allowed set from patterns
|
||||
const allowedSet = new Set<string>();
|
||||
const excludedPatterns: string[] = [];
|
||||
// Last entry wins for duplicate paths: we track the "final decision" per path
|
||||
const finalDecisions = new Map<string, boolean>(); // true = allowed, false = excluded
|
||||
|
||||
for (const pattern of skillPatterns) {
|
||||
const path = normalizePattern(pattern);
|
||||
if (isExclusionPattern(pattern)) {
|
||||
excludedPatterns.push(path);
|
||||
allowedSet.delete(path); // Remove from allowed set if previously added
|
||||
} else {
|
||||
allowedSet.add(path);
|
||||
}
|
||||
const isExclusion = isExclusionPattern(pattern);
|
||||
finalDecisions.set(path, !isExclusion);
|
||||
}
|
||||
|
||||
// Apply exclusion patterns last (they override inclusions)
|
||||
for (const excluded of excludedPatterns) {
|
||||
allowedSet.delete(excluded);
|
||||
// Build allowed set from final decisions
|
||||
const allowedSet = new Set<string>();
|
||||
for (const [path, allowed] of finalDecisions) {
|
||||
if (allowed) {
|
||||
allowedSet.add(path);
|
||||
}
|
||||
}
|
||||
|
||||
// Determine if filtering is active
|
||||
@@ -303,23 +302,38 @@ export function createSkillsOverrideFromSelection(
|
||||
return base;
|
||||
}
|
||||
|
||||
// Filter skills to only those in the allowed set
|
||||
const filteredSkills = base.skills.filter((skill) => {
|
||||
// Match by filePath (the skill's absolute file path)
|
||||
return allowedSkillPaths.has(skill.filePath);
|
||||
});
|
||||
// Determine the effective filter criteria
|
||||
// When requestedSkillNames is provided without patterns, filter by name
|
||||
// When patterns are provided, filter by file path
|
||||
const hasPatterns = allowedSkillPaths.size > 0;
|
||||
const hasRequestedNames = Boolean(requestedSkillNames && requestedSkillNames.length > 0);
|
||||
|
||||
// Filter skills
|
||||
let filteredSkills: Skill[];
|
||||
if (hasRequestedNames) {
|
||||
// Filter by requested names (case-insensitive match)
|
||||
const requestedNamesLower = new Set(requestedSkillNames!.map((n) => n.toLowerCase()));
|
||||
filteredSkills = base.skills.filter((skill) => requestedNamesLower.has(skill.name.toLowerCase()));
|
||||
} else if (hasPatterns) {
|
||||
// Filter by file path
|
||||
filteredSkills = base.skills.filter((skill) => allowedSkillPaths.has(skill.filePath));
|
||||
} else {
|
||||
// No filter criteria - this shouldn't happen if filterActive is true
|
||||
filteredSkills = base.skills;
|
||||
}
|
||||
|
||||
// Build diagnostics for missing skills
|
||||
const newDiagnostics: ResourceDiagnostic[] = [];
|
||||
|
||||
// Check for configured patterns that don't match any discovered skill
|
||||
// Note: At this point, we have access to base.skills for validation
|
||||
const purpose = sessionPurpose ? ` [${sessionPurpose}]` : "";
|
||||
for (const allowedPath of allowedSkillPaths) {
|
||||
const hasMatch = base.skills.some((skill) => skill.filePath === allowedPath);
|
||||
if (!hasMatch) {
|
||||
newDiagnostics.push({
|
||||
type: "warning",
|
||||
message: `Configured skill pattern '${allowedPath}' not found in discovered skills`,
|
||||
message: `Configured skill pattern '${allowedPath}' not found in discovered skills${purpose}`,
|
||||
path: allowedPath,
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user