feat(FN-1811): merge fusion/fn-1811
This commit is contained in:
gsxdsm
243
packages/engine/src/__tests__/agent-skills-flow.test.ts
Normal file
243
packages/engine/src/__tests__/agent-skills-flow.test.ts
Normal file
@@ -0,0 +1,243 @@
|
||||
/**
|
||||
* Integration-style tests for agent skills flow.
|
||||
*
|
||||
* Tests the full metadata → engine flow using mocked AgentStore and in-memory filesystem.
|
||||
*/
|
||||
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { buildSessionSkillContext } from "../session-skill-context.js";
|
||||
import { resolveSessionSkills, createSkillsOverrideFromSelection } from "../skill-resolver.js";
|
||||
import type { Agent, AgentStore } from "@fusion/core";
|
||||
|
||||
// ── Mock Setup ───────────────────────────────────────────────────────────────
|
||||
|
||||
// In-memory file system for tests - using a module-scoped Map
|
||||
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/agent-skills-flow-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/agent-skills-flow-mock-${++mockDirCounter}`;
|
||||
if (settings !== null) {
|
||||
mockFiles.set(`${dir}/.fusion/settings.json`, JSON.stringify(settings));
|
||||
}
|
||||
return dir;
|
||||
}
|
||||
|
||||
// ── Tests ───────────────────────────────────────────────────────────────────
|
||||
|
||||
describe("agent skills flow - full integration", () => {
|
||||
const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
|
||||
beforeEach(() => {
|
||||
mockFiles.clear();
|
||||
mockDirCounter = 0;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
consoleErrorSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("full end-to-end flow: settings patterns + agent metadata + discovered skills produce correct override", async () => {
|
||||
// Step 1: Set up mock filesystem with settings that include review but exclude lint
|
||||
const projectRootDir = createMockProjectDir({
|
||||
skills: ["+skills/review/SKILL.md", "+skills/lint/SKILL.md", "-skills/lint/SKILL.md"],
|
||||
});
|
||||
|
||||
// Step 2: Create mock AgentStore with agent that has review skill
|
||||
const mockAgent: Agent = {
|
||||
id: "agent-001",
|
||||
name: "Test Agent",
|
||||
role: "executor",
|
||||
state: "idle",
|
||||
metadata: { skills: ["review"] },
|
||||
} as unknown as Agent;
|
||||
|
||||
const mockAgentStore = {
|
||||
getAgent: vi.fn().mockResolvedValue(mockAgent),
|
||||
} as unknown as AgentStore;
|
||||
|
||||
// Step 3: Create task with assigned agent
|
||||
const task = { assignedAgentId: "agent-001" };
|
||||
|
||||
// Step 4: Build session skill context from agent metadata
|
||||
const sessionResult = await buildSessionSkillContext({
|
||||
agentStore: mockAgentStore,
|
||||
task,
|
||||
sessionPurpose: "executor",
|
||||
projectRootDir,
|
||||
});
|
||||
|
||||
// Verify skill source and resolved names
|
||||
expect(sessionResult.skillSource).toBe("assigned-agent");
|
||||
expect(sessionResult.resolvedSkillNames).toEqual(["review"]);
|
||||
expect(sessionResult.skillSelectionContext).toBeDefined();
|
||||
expect(sessionResult.skillSelectionContext?.requestedSkillNames).toEqual(["review"]);
|
||||
|
||||
// Step 5: Resolve session skills from project settings
|
||||
const resolvedSkills = resolveSessionSkills(sessionResult.skillSelectionContext!);
|
||||
|
||||
// Verify the settings-based resolution
|
||||
expect(resolvedSkills.filterActive).toBe(true);
|
||||
expect(resolvedSkills.allowedSkillPaths.has("skills/review/SKILL.md")).toBe(true);
|
||||
// lint should NOT be in allowed paths (it was explicitly excluded)
|
||||
expect(resolvedSkills.allowedSkillPaths.has("skills/lint/SKILL.md")).toBe(false);
|
||||
// lint should be in excluded paths
|
||||
expect(resolvedSkills.excludedSkillPaths.has("skills/lint/SKILL.md")).toBe(true);
|
||||
|
||||
// Step 6: Create override callback from selection
|
||||
const override = createSkillsOverrideFromSelection(resolvedSkills, {
|
||||
sessionPurpose: "executor",
|
||||
});
|
||||
|
||||
// Step 7: Apply override to discovered skills (both review and lint exist)
|
||||
const base = {
|
||||
skills: [
|
||||
{ name: "review", filePath: "skills/review/SKILL.md", description: "", baseDir: "", sourceInfo: {} as any, disableModelInvocation: false },
|
||||
{ name: "lint", filePath: "skills/lint/SKILL.md", description: "", baseDir: "", sourceInfo: {} as any, disableModelInvocation: false },
|
||||
],
|
||||
diagnostics: [],
|
||||
};
|
||||
|
||||
const overrideResult = override(base);
|
||||
|
||||
// Step 8: Verify only review passes through (lint is disabled)
|
||||
expect(overrideResult.skills).toHaveLength(1);
|
||||
expect(overrideResult.skills[0].name).toBe("review");
|
||||
|
||||
// Step 9: Verify warning diagnostic for disabled lint skill
|
||||
const disabledLintWarning = overrideResult.diagnostics.find(d =>
|
||||
d.message.includes("disabled") && d.message.includes("lint")
|
||||
);
|
||||
expect(disabledLintWarning).toBeDefined();
|
||||
expect(disabledLintWarning?.type).toBe("warning");
|
||||
|
||||
// Step 10: Verify console.error was called with disabled skill warning
|
||||
const loggedMessages = consoleErrorSpy.mock.calls.map(c => c[0] as string);
|
||||
const hasDisabledLintWarning = loggedMessages.some(m =>
|
||||
m.includes("disabled") && m.includes("lint")
|
||||
);
|
||||
expect(hasDisabledLintWarning).toBe(true);
|
||||
});
|
||||
|
||||
it("flow with no exclusion pattern - both review and lint requested", async () => {
|
||||
// Step 1: Set up mock filesystem with settings that include both skills
|
||||
const projectRootDir = createMockProjectDir({
|
||||
skills: ["+skills/review/SKILL.md", "+skills/lint/SKILL.md"],
|
||||
});
|
||||
|
||||
// Step 2: Create mock AgentStore with agent that has BOTH review and lint skills
|
||||
const mockAgent: Agent = {
|
||||
id: "agent-001",
|
||||
name: "Test Agent",
|
||||
role: "executor",
|
||||
state: "idle",
|
||||
metadata: { skills: ["review", "lint"] },
|
||||
} as unknown as Agent;
|
||||
|
||||
const mockAgentStore = {
|
||||
getAgent: vi.fn().mockResolvedValue(mockAgent),
|
||||
} as unknown as AgentStore;
|
||||
|
||||
// Step 3: Build session skill context
|
||||
const sessionResult = await buildSessionSkillContext({
|
||||
agentStore: mockAgentStore,
|
||||
task: { assignedAgentId: "agent-001" },
|
||||
sessionPurpose: "executor",
|
||||
projectRootDir,
|
||||
});
|
||||
|
||||
expect(sessionResult.skillSource).toBe("assigned-agent");
|
||||
expect(sessionResult.resolvedSkillNames).toEqual(["review", "lint"]);
|
||||
|
||||
// Step 4: Resolve session skills from settings
|
||||
const resolvedSkills = resolveSessionSkills(sessionResult.skillSelectionContext!);
|
||||
|
||||
expect(resolvedSkills.filterActive).toBe(true);
|
||||
expect(resolvedSkills.allowedSkillPaths.has("skills/review/SKILL.md")).toBe(true);
|
||||
expect(resolvedSkills.allowedSkillPaths.has("skills/lint/SKILL.md")).toBe(true);
|
||||
expect(resolvedSkills.excludedSkillPaths.size).toBe(0);
|
||||
|
||||
// Step 5: Create override and apply to discovered skills
|
||||
const override = createSkillsOverrideFromSelection(resolvedSkills, {
|
||||
sessionPurpose: "executor",
|
||||
});
|
||||
|
||||
const base = {
|
||||
skills: [
|
||||
{ name: "review", filePath: "skills/review/SKILL.md", description: "", baseDir: "", sourceInfo: {} as any, disableModelInvocation: false },
|
||||
{ name: "lint", filePath: "skills/lint/SKILL.md", description: "", baseDir: "", sourceInfo: {} as any, disableModelInvocation: false },
|
||||
],
|
||||
diagnostics: [],
|
||||
};
|
||||
|
||||
const overrideResult = override(base);
|
||||
|
||||
// When both skills are requested, both should pass through
|
||||
expect(overrideResult.skills).toHaveLength(2);
|
||||
expect(overrideResult.skills.map(s => s.name)).toEqual(["review", "lint"]);
|
||||
|
||||
// No disabled warnings since neither skill was excluded
|
||||
const disabledWarnings = overrideResult.diagnostics.filter(d =>
|
||||
d.message.includes("disabled")
|
||||
);
|
||||
expect(disabledWarnings).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("flow with role fallback when assigned agent has no skills", async () => {
|
||||
// Step 1: Set up mock filesystem
|
||||
const projectRootDir = createMockProjectDir({
|
||||
skills: ["+skills/executor/SKILL.md"],
|
||||
});
|
||||
|
||||
// Step 2: Create mock AgentStore with agent that has NO skills
|
||||
const mockAgent: Agent = {
|
||||
id: "agent-001",
|
||||
name: "Test Agent",
|
||||
role: "executor",
|
||||
state: "idle",
|
||||
metadata: { skills: [] },
|
||||
} as unknown as Agent;
|
||||
|
||||
const mockAgentStore = {
|
||||
getAgent: vi.fn().mockResolvedValue(mockAgent),
|
||||
} as unknown as AgentStore;
|
||||
|
||||
// Step 3: Build session skill context - should fall back to role
|
||||
const sessionResult = await buildSessionSkillContext({
|
||||
agentStore: mockAgentStore,
|
||||
task: { assignedAgentId: "agent-001" },
|
||||
sessionPurpose: "executor",
|
||||
projectRootDir,
|
||||
});
|
||||
|
||||
// Verify role fallback
|
||||
expect(sessionResult.skillSource).toBe("role-fallback");
|
||||
expect(sessionResult.resolvedSkillNames).toEqual(["executor"]);
|
||||
expect(sessionResult.skillSelectionContext?.requestedSkillNames).toEqual(["executor"]);
|
||||
|
||||
// Step 4: Resolve session skills from settings
|
||||
const resolvedSkills = resolveSessionSkills(sessionResult.skillSelectionContext!);
|
||||
|
||||
expect(resolvedSkills.filterActive).toBe(true);
|
||||
expect(resolvedSkills.allowedSkillPaths.has("skills/executor/SKILL.md")).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -84,6 +84,20 @@ describe("normalizeAgentSkills", () => {
|
||||
it("returns empty array for array of only invalid entries", () => {
|
||||
expect(normalizeAgentSkills([null, undefined, "", 123, {}])).toEqual([]);
|
||||
});
|
||||
|
||||
it("handles object entries with name, deduplicates, trims, and drops invalid entries", () => {
|
||||
const skills = [
|
||||
{ name: " review " },
|
||||
" custom-skill ",
|
||||
{ name: "review" },
|
||||
123,
|
||||
null,
|
||||
{ foo: "bar" },
|
||||
"",
|
||||
{ name: "" },
|
||||
];
|
||||
expect(normalizeAgentSkills(skills)).toEqual(["review", "custom-skill"]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("buildSessionSkillContextSync", () => {
|
||||
@@ -125,6 +139,26 @@ describe("buildSessionSkillContextSync", () => {
|
||||
expect(result.resolvedSkillNames).toEqual(["triage", "executor"]);
|
||||
});
|
||||
|
||||
it("correctly extracts skills from a cached agent object", () => {
|
||||
const agent = {
|
||||
id: "agent-001",
|
||||
name: "Test Agent",
|
||||
role: "executor" as const,
|
||||
state: "idle" as const,
|
||||
metadata: { skills: ["triage", "executor"] },
|
||||
} as unknown as Agent;
|
||||
|
||||
const result = buildSessionSkillContextSync(agent, "executor", projectRootDir);
|
||||
|
||||
expect(result.skillSource).toBe("assigned-agent");
|
||||
expect(result.resolvedSkillNames).toEqual(["triage", "executor"]);
|
||||
expect(result.skillSelectionContext).toEqual({
|
||||
projectRootDir,
|
||||
requestedSkillNames: ["triage", "executor"],
|
||||
sessionPurpose: "executor",
|
||||
});
|
||||
});
|
||||
|
||||
it("falls back to role when agent has empty skills", () => {
|
||||
const agent: Agent = {
|
||||
id: "agent-001",
|
||||
@@ -261,6 +295,56 @@ describe("buildSessionSkillContext", () => {
|
||||
expect(mockAgentStore.getAgent).toHaveBeenCalledWith("agent-001");
|
||||
});
|
||||
|
||||
it("resolves assigned-agent skills when task.assignedAgentId points to an agent with metadata.skills", async () => {
|
||||
const mockAgent: Agent = {
|
||||
id: "agent-001",
|
||||
name: "Test Agent",
|
||||
role: "executor",
|
||||
state: "idle",
|
||||
metadata: { skills: ["review", "custom-skill"] },
|
||||
} as unknown as Agent;
|
||||
|
||||
const mockAgentStore = {
|
||||
getAgent: vi.fn().mockResolvedValue(mockAgent),
|
||||
} as unknown as AgentStore;
|
||||
|
||||
const result = await buildSessionSkillContext({
|
||||
agentStore: mockAgentStore,
|
||||
task: { assignedAgentId: "agent-001" },
|
||||
sessionPurpose: "executor",
|
||||
projectRootDir,
|
||||
});
|
||||
|
||||
expect(result.skillSource).toBe("assigned-agent");
|
||||
expect(result.resolvedSkillNames).toEqual(["review", "custom-skill"]);
|
||||
expect(result.skillSelectionContext?.requestedSkillNames).toEqual(["review", "custom-skill"]);
|
||||
expect(mockAgentStore.getAgent).toHaveBeenCalledWith("agent-001");
|
||||
});
|
||||
|
||||
it("falls back to role fallback skills when assigned agent has no skills", async () => {
|
||||
const mockAgent: Agent = {
|
||||
id: "agent-001",
|
||||
name: "Test Agent",
|
||||
role: "executor",
|
||||
state: "idle",
|
||||
metadata: { skills: [] },
|
||||
} as unknown as Agent;
|
||||
|
||||
const mockAgentStore = {
|
||||
getAgent: vi.fn().mockResolvedValue(mockAgent),
|
||||
} as unknown as AgentStore;
|
||||
|
||||
const result = await buildSessionSkillContext({
|
||||
agentStore: mockAgentStore,
|
||||
task: { assignedAgentId: "agent-001" },
|
||||
sessionPurpose: "executor",
|
||||
projectRootDir,
|
||||
});
|
||||
|
||||
expect(result.skillSource).toBe("role-fallback");
|
||||
expect(result.resolvedSkillNames).toEqual(["executor"]);
|
||||
});
|
||||
|
||||
it("falls back to role when no assignedAgentId", async () => {
|
||||
const mockAgentStore = {
|
||||
getAgent: vi.fn(),
|
||||
|
||||
@@ -358,6 +358,29 @@ describe("resolveSessionSkills", () => {
|
||||
expect(infoDiags).toHaveLength(1); // Only + pattern gets info diag
|
||||
expect(infoDiags[0].skillPath).toBe("skills/foo/SKILL.md");
|
||||
});
|
||||
|
||||
it("with requestedSkillNames intersects with allowed patterns and produces correct diagnostics", () => {
|
||||
const dir = createMockProjectDir({
|
||||
skills: ["+skills/review/SKILL.md", "+skills/lint/SKILL.md"],
|
||||
});
|
||||
|
||||
const result = resolveSessionSkills({
|
||||
projectRootDir: dir,
|
||||
requestedSkillNames: ["review", "missing-skill"],
|
||||
});
|
||||
|
||||
expect(result.filterActive).toBe(true);
|
||||
expect(result.allowedSkillPaths.has("skills/review/SKILL.md")).toBe(true);
|
||||
expect(result.allowedSkillPaths.has("skills/lint/SKILL.md")).toBe(true);
|
||||
|
||||
// Check for info diagnostics
|
||||
const infoDiags = result.diagnostics.filter(d => d.type === "info");
|
||||
// Should have diagnostics for: review (requested), missing-skill (requested), +skills/review (pattern), +skills/lint (pattern)
|
||||
expect(infoDiags.some(d => d.skillName === "review")).toBe(true);
|
||||
expect(infoDiags.some(d => d.skillName === "missing-skill")).toBe(true);
|
||||
expect(infoDiags.some(d => d.skillPath === "skills/review/SKILL.md")).toBe(true);
|
||||
expect(infoDiags.some(d => d.skillPath === "skills/lint/SKILL.md")).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -673,5 +696,99 @@ describe("createSkillsOverrideFromSelection", () => {
|
||||
// Order is preserved from input order (deterministic = consistent)
|
||||
expect(result1.skills.map(s => s.name)).toEqual(["c", "a", "b"]);
|
||||
});
|
||||
|
||||
it("filters discovered Skill[] by requested names and logs warnings for missing skills", () => {
|
||||
const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
|
||||
// Create selection with empty allowed paths (only requested names filtering)
|
||||
const selection: SkillSelectionResult = {
|
||||
allowedSkillPaths: new Set<string>(),
|
||||
excludedSkillPaths: new Set<string>(),
|
||||
diagnostics: [],
|
||||
filterActive: true,
|
||||
};
|
||||
|
||||
const override = createSkillsOverrideFromSelection(selection, {
|
||||
requestedSkillNames: ["found-skill", "missing-skill"],
|
||||
sessionPurpose: "executor",
|
||||
});
|
||||
|
||||
// Only one skill matches the requested names
|
||||
const base = {
|
||||
skills: [
|
||||
{ name: "found-skill", filePath: "/path/found", description: "", baseDir: "", sourceInfo: {} as any, disableModelInvocation: false },
|
||||
],
|
||||
diagnostics: [],
|
||||
};
|
||||
|
||||
const result = override(base);
|
||||
|
||||
// Should only return the found skill
|
||||
expect(result.skills).toHaveLength(1);
|
||||
expect(result.skills[0].name).toBe("found-skill");
|
||||
|
||||
// Should produce warning for missing-skill
|
||||
const missingWarning = result.diagnostics.find(d =>
|
||||
d.type === "warning" && d.message.includes("missing-skill") && d.message.includes("not found in discovered skills")
|
||||
);
|
||||
expect(missingWarning).toBeDefined();
|
||||
|
||||
// Verify console.error logging
|
||||
expect(consoleErrorSpy).toHaveBeenCalled();
|
||||
const loggedMessages = consoleErrorSpy.mock.calls.map(c => c[0] as string);
|
||||
const hasExecutorPrefix = loggedMessages.some(m => m.includes("[executor]") && m.includes("missing-skill"));
|
||||
expect(hasExecutorPrefix).toBe(true);
|
||||
|
||||
consoleErrorSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
describe("end-to-end flow with project settings + agent metadata + discovered skills", () => {
|
||||
beforeEach(() => {
|
||||
mockFiles.clear();
|
||||
mockDirCounter = 0;
|
||||
});
|
||||
|
||||
it("full end-to-end flow: settings patterns + agent skills produce matching override", () => {
|
||||
// Create mock project dir with settings that include review but exclude lint
|
||||
const dir = createMockProjectDir({
|
||||
skills: ["+skills/review/SKILL.md"],
|
||||
});
|
||||
|
||||
// Step 1: Resolve session skills from settings
|
||||
const resolvedSkills = resolveSessionSkills({
|
||||
projectRootDir: dir,
|
||||
requestedSkillNames: ["review"],
|
||||
});
|
||||
|
||||
expect(resolvedSkills.filterActive).toBe(true);
|
||||
expect(resolvedSkills.allowedSkillPaths.has("skills/review/SKILL.md")).toBe(true);
|
||||
|
||||
// Step 2: Create override from selection
|
||||
const override = createSkillsOverrideFromSelection(resolvedSkills, {
|
||||
sessionPurpose: "executor",
|
||||
});
|
||||
|
||||
// Step 3: Apply override to discovered skills (both review and lint exist)
|
||||
const base = {
|
||||
skills: [
|
||||
{ name: "review", filePath: "skills/review/SKILL.md", description: "", baseDir: "", sourceInfo: {} as any, disableModelInvocation: false },
|
||||
{ name: "lint", filePath: "skills/lint/SKILL.md", description: "", baseDir: "", sourceInfo: {} as any, disableModelInvocation: false },
|
||||
],
|
||||
diagnostics: [],
|
||||
};
|
||||
|
||||
const result = override(base);
|
||||
|
||||
// Only review should pass through (lint is not in allowed paths)
|
||||
expect(result.skills).toHaveLength(1);
|
||||
expect(result.skills[0].name).toBe("review");
|
||||
|
||||
// No missing-skill warnings since review exists and matches the pattern
|
||||
const missingWarnings = result.diagnostics.filter(d =>
|
||||
d.type === "warning" && d.message.includes("not found")
|
||||
);
|
||||
expect(missingWarnings).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user