feat(FN-1795): merge fusion/fn-1795
This commit is contained in:
@@ -407,6 +407,16 @@ The token-based approach allows all bottom-layout consumers to be updated togeth
|
|||||||
|
|
||||||
## Agent Skills
|
## Agent Skills
|
||||||
|
|
||||||
|
### Engine Skill Selection (FN-1795)
|
||||||
|
|
||||||
|
The `createKbAgent` function in `packages/engine/src/pi.ts` supports a `skills?: string[]` convenience parameter for skill filtering:
|
||||||
|
|
||||||
|
- **Convenience parameter**: `AgentOptions.skills` accepts an array of skill names and auto-derives a `SkillSelectionContext`
|
||||||
|
- **Precedence**: Explicit `skillSelection` takes precedence over `skills` when both are provided
|
||||||
|
- **Logging**: When using the convenience path, a log message is emitted: `[pi] Using skills from convenience parameter: [skill1, skill2]`
|
||||||
|
- **Engine integration**: All 5 engine paths (executor, triage, reviewer, merger, heartbeat) use `buildSessionSkillContext` to derive skill selection from agent metadata, which then flows through to `createKbAgent` via the `skillSelection` option
|
||||||
|
- **Skill resolver**: `resolveSessionSkills` and `createSkillsOverrideFromSelection` handle the actual skill filtering based on project settings
|
||||||
|
|
||||||
### create-fusion-plugin Skill (FN-1134)
|
### create-fusion-plugin Skill (FN-1134)
|
||||||
|
|
||||||
The `create-fusion-plugin` skill teaches agents how to create Fusion plugins. Located at `.pi/agent/skills/create-fusion-plugin/`.
|
The `create-fusion-plugin` skill teaches agents how to create Fusion plugins. Located at `.pi/agent/skills/create-fusion-plugin/`.
|
||||||
|
|||||||
@@ -1,7 +1,72 @@
|
|||||||
import { describe, it, expect } from "vitest";
|
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||||
import { describeModel, compactSessionContext, COMPACTION_FALLBACK_INSTRUCTIONS } from "./pi.js";
|
import { describeModel, compactSessionContext, COMPACTION_FALLBACK_INSTRUCTIONS, createKbAgent, type AgentOptions } from "./pi.js";
|
||||||
import type { AgentSession } from "@mariozechner/pi-coding-agent";
|
import type { AgentSession } from "@mariozechner/pi-coding-agent";
|
||||||
|
|
||||||
|
// Mock skill resolver functions - define inside factory to avoid hoisting issues
|
||||||
|
vi.mock("./skill-resolver.js", () => {
|
||||||
|
const resolveSessionSkillsMock = vi.fn();
|
||||||
|
const createSkillsOverrideFromSelectionMock = vi.fn();
|
||||||
|
return {
|
||||||
|
resolveSessionSkills: resolveSessionSkillsMock,
|
||||||
|
createSkillsOverrideFromSelection: createSkillsOverrideFromSelectionMock,
|
||||||
|
// Export mock functions for test assertions
|
||||||
|
__getMocks: () => ({
|
||||||
|
resolveSessionSkills: resolveSessionSkillsMock,
|
||||||
|
createSkillsOverrideFromSelection: createSkillsOverrideFromSelectionMock,
|
||||||
|
}),
|
||||||
|
};
|
||||||
|
});
|
||||||
|
|
||||||
|
// Mock pi-coding-agent imports
|
||||||
|
vi.mock("@mariozechner/pi-coding-agent", () => ({
|
||||||
|
AuthStorage: {
|
||||||
|
create: vi.fn(() => ({
|
||||||
|
getCredentials: vi.fn().mockResolvedValue({}),
|
||||||
|
})),
|
||||||
|
},
|
||||||
|
createAgentSession: vi.fn(async () => ({
|
||||||
|
session: {
|
||||||
|
model: { provider: "test", id: "test" },
|
||||||
|
subscribe: vi.fn(),
|
||||||
|
prompt: vi.fn(),
|
||||||
|
sessionFile: undefined,
|
||||||
|
},
|
||||||
|
})),
|
||||||
|
createCodingTools: vi.fn(() => []),
|
||||||
|
createReadOnlyTools: vi.fn(() => []),
|
||||||
|
createExtensionRuntime: vi.fn(),
|
||||||
|
DefaultResourceLoader: vi.fn().mockImplementation(() => ({
|
||||||
|
reload: vi.fn().mockResolvedValue(undefined),
|
||||||
|
skillsOverride: undefined,
|
||||||
|
})),
|
||||||
|
DefaultPackageManager: vi.fn(),
|
||||||
|
discoverAndLoadExtensions: vi.fn().mockResolvedValue({ errors: [], runtime: { pendingProviderRegistrations: [] } }),
|
||||||
|
getAgentDir: vi.fn(() => "/test/agent-dir"),
|
||||||
|
ModelRegistry: vi.fn().mockImplementation(() => ({
|
||||||
|
find: vi.fn().mockReturnValue({ provider: "test", id: "test-model" }),
|
||||||
|
getAll: vi.fn().mockReturnValue([]),
|
||||||
|
registerProvider: vi.fn(),
|
||||||
|
refresh: vi.fn(),
|
||||||
|
})),
|
||||||
|
SessionManager: {
|
||||||
|
inMemory: vi.fn(() => ({})),
|
||||||
|
},
|
||||||
|
SettingsManager: {
|
||||||
|
inMemory: vi.fn(() => ({})),
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
|
||||||
|
// Import mock accessors after mocking (must use dynamic import for hoisted mocks)
|
||||||
|
let resolveSessionSkillsMock: ReturnType<typeof vi.fn>;
|
||||||
|
let createSkillsOverrideFromSelectionMock: ReturnType<typeof vi.fn>;
|
||||||
|
|
||||||
|
// Initialize mocks before first test
|
||||||
|
beforeEach(() => {
|
||||||
|
// Access mocks from the mocked module
|
||||||
|
const mocks = (vi.mocked({ resolveSessionSkills: vi.fn(), createSkillsOverrideFromSelection: vi.fn() }));
|
||||||
|
// We need to re-mock in beforeEach to ensure they're fresh
|
||||||
|
});
|
||||||
|
|
||||||
describe("describeModel", () => {
|
describe("describeModel", () => {
|
||||||
it('returns "provider/modelId" when session has a model', () => {
|
it('returns "provider/modelId" when session has a model', () => {
|
||||||
const fakeSession = {
|
const fakeSession = {
|
||||||
@@ -115,3 +180,133 @@ describe("compactSessionContext", () => {
|
|||||||
expect(result).toEqual({ summary: "", tokensBefore: 0 });
|
expect(result).toEqual({ summary: "", tokensBefore: 0 });
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("createKbAgent skills parameter", () => {
|
||||||
|
let consoleErrorSpy: ReturnType<typeof vi.spyOn>;
|
||||||
|
let mockResolveSessionSkills: ReturnType<typeof vi.fn>;
|
||||||
|
let mockCreateSkillsOverride: ReturnType<typeof vi.fn>;
|
||||||
|
|
||||||
|
beforeEach(async () => {
|
||||||
|
consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||||
|
|
||||||
|
// Access the mocked module to get/set mocks
|
||||||
|
const skillResolver = await import("./skill-resolver.js");
|
||||||
|
mockResolveSessionSkills = vi.mocked(skillResolver.resolveSessionSkills);
|
||||||
|
mockCreateSkillsOverride = vi.mocked(skillResolver.createSkillsOverrideFromSelection);
|
||||||
|
|
||||||
|
mockResolveSessionSkills.mockReturnValue({
|
||||||
|
allowedSkillPaths: new Set(),
|
||||||
|
excludedSkillPaths: new Set(),
|
||||||
|
diagnostics: [],
|
||||||
|
filterActive: true,
|
||||||
|
});
|
||||||
|
mockCreateSkillsOverride.mockReturnValue(() => ({
|
||||||
|
skills: [],
|
||||||
|
diagnostics: [],
|
||||||
|
}));
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
consoleErrorSpy.mockRestore();
|
||||||
|
vi.clearAllMocks();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("skills parameter auto-derives SkillSelectionContext", async () => {
|
||||||
|
const options: AgentOptions = {
|
||||||
|
cwd: "/test/project",
|
||||||
|
systemPrompt: "Test",
|
||||||
|
skills: ["review", "fusion"],
|
||||||
|
};
|
||||||
|
|
||||||
|
await createKbAgent(options);
|
||||||
|
|
||||||
|
// Verify resolveSessionSkills was called with auto-derived context
|
||||||
|
expect(mockResolveSessionSkills).toHaveBeenCalledTimes(1);
|
||||||
|
const callArgs = mockResolveSessionSkills.mock.calls[0]![0];
|
||||||
|
expect(callArgs.projectRootDir).toBe("/test/project");
|
||||||
|
expect(callArgs.requestedSkillNames).toEqual(["review", "fusion"]);
|
||||||
|
expect(callArgs.sessionPurpose).toBe("executor");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("skillSelection takes precedence over skills", async () => {
|
||||||
|
const options: AgentOptions = {
|
||||||
|
cwd: "/test/project",
|
||||||
|
systemPrompt: "Test",
|
||||||
|
skills: ["review"],
|
||||||
|
skillSelection: {
|
||||||
|
projectRootDir: "/other",
|
||||||
|
requestedSkillNames: ["triage"],
|
||||||
|
sessionPurpose: "triage",
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
await createKbAgent(options);
|
||||||
|
|
||||||
|
// Verify resolveSessionSkills was called with explicit skillSelection (not auto-derived)
|
||||||
|
expect(mockResolveSessionSkills).toHaveBeenCalledTimes(1);
|
||||||
|
const callArgs = mockResolveSessionSkills.mock.calls[0]![0];
|
||||||
|
expect(callArgs.projectRootDir).toBe("/other");
|
||||||
|
expect(callArgs.requestedSkillNames).toEqual(["triage"]);
|
||||||
|
expect(callArgs.sessionPurpose).toBe("triage");
|
||||||
|
|
||||||
|
// Verify the convenience log was NOT emitted (skillSelection takes precedence)
|
||||||
|
expect(consoleErrorSpy).not.toHaveBeenCalledWith(
|
||||||
|
expect.stringContaining("Using skills from convenience parameter")
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("empty skills array is treated as unset", async () => {
|
||||||
|
const options: AgentOptions = {
|
||||||
|
cwd: "/test/project",
|
||||||
|
systemPrompt: "Test",
|
||||||
|
skills: [],
|
||||||
|
};
|
||||||
|
|
||||||
|
await createKbAgent(options);
|
||||||
|
|
||||||
|
// Verify no skill resolution occurred
|
||||||
|
expect(mockResolveSessionSkills).not.toHaveBeenCalled();
|
||||||
|
expect(mockCreateSkillsOverride).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("skills auto-derivation logs the convenience parameter", async () => {
|
||||||
|
const options: AgentOptions = {
|
||||||
|
cwd: "/test/project",
|
||||||
|
systemPrompt: "Test",
|
||||||
|
skills: ["review", "fusion"],
|
||||||
|
};
|
||||||
|
|
||||||
|
await createKbAgent(options);
|
||||||
|
|
||||||
|
// Verify the log message includes the skill names
|
||||||
|
expect(consoleErrorSpy).toHaveBeenCalledWith(
|
||||||
|
expect.stringContaining("[pi] Using skills from convenience parameter: [review, fusion]")
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("skills without corresponding discovered skills produces diagnostics", async () => {
|
||||||
|
// Mock to return diagnostics for missing skill
|
||||||
|
mockResolveSessionSkills.mockReturnValue({
|
||||||
|
allowedSkillPaths: new Set(),
|
||||||
|
excludedSkillPaths: new Set(),
|
||||||
|
diagnostics: [
|
||||||
|
{ type: "warning" as const, message: 'Requested skill "nonexistent-skill" not found in discovered skills' },
|
||||||
|
],
|
||||||
|
filterActive: true,
|
||||||
|
});
|
||||||
|
|
||||||
|
const options: AgentOptions = {
|
||||||
|
cwd: "/test/project",
|
||||||
|
systemPrompt: "Test",
|
||||||
|
skills: ["nonexistent-skill"],
|
||||||
|
};
|
||||||
|
|
||||||
|
await createKbAgent(options);
|
||||||
|
|
||||||
|
// The diagnostics should be logged
|
||||||
|
expect(mockResolveSessionSkills).toHaveBeenCalled();
|
||||||
|
expect(consoleErrorSpy).toHaveBeenCalledWith(
|
||||||
|
expect.stringContaining("warning")
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -151,6 +151,10 @@ export interface AgentOptions {
|
|||||||
* caller-requested skill names. Omit to use default skill discovery
|
* caller-requested skill names. Omit to use default skill discovery
|
||||||
* (all discovered skills included). */
|
* (all discovered skills included). */
|
||||||
skillSelection?: SkillSelectionContext;
|
skillSelection?: SkillSelectionContext;
|
||||||
|
/** Convenience: skill names to include in the session. When provided
|
||||||
|
* (and `skillSelection` is not), auto-constructs a SkillSelectionContext
|
||||||
|
* from the cwd and these names. Ignored when `skillSelection` is set. */
|
||||||
|
skills?: string[];
|
||||||
}
|
}
|
||||||
|
|
||||||
function resolveConfiguredModel(
|
function resolveConfiguredModel(
|
||||||
@@ -482,19 +486,30 @@ export async function createKbAgent(options: AgentOptions): Promise<AgentResult>
|
|||||||
options.fallbackModelId,
|
options.fallbackModelId,
|
||||||
);
|
);
|
||||||
|
|
||||||
|
// Resolve skill selection: explicit skillSelection wins over convenience `skills`
|
||||||
|
let effectiveSkillSelection: SkillSelectionContext | undefined = options.skillSelection;
|
||||||
|
if (!effectiveSkillSelection && options.skills && options.skills.length > 0) {
|
||||||
|
console.error(`[pi] Using skills from convenience parameter: [${options.skills.join(", ")}]`);
|
||||||
|
effectiveSkillSelection = {
|
||||||
|
projectRootDir: options.cwd,
|
||||||
|
requestedSkillNames: options.skills,
|
||||||
|
sessionPurpose: "executor",
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
// Resolve skill selection if provided
|
// Resolve skill selection if provided
|
||||||
let skillsOverrideFn: ReturnType<typeof createSkillsOverrideFromSelection> | undefined;
|
let skillsOverrideFn: ReturnType<typeof createSkillsOverrideFromSelection> | undefined;
|
||||||
if (options.skillSelection) {
|
if (effectiveSkillSelection) {
|
||||||
const selectionResult = resolveSessionSkills(options.skillSelection);
|
const selectionResult = resolveSessionSkills(effectiveSkillSelection);
|
||||||
if (selectionResult.diagnostics.length > 0) {
|
if (selectionResult.diagnostics.length > 0) {
|
||||||
const purpose = options.skillSelection.sessionPurpose ?? "skills";
|
const purpose = effectiveSkillSelection.sessionPurpose ?? "skills";
|
||||||
for (const diag of selectionResult.diagnostics) {
|
for (const diag of selectionResult.diagnostics) {
|
||||||
console.error(`[pi] [skills] [${purpose}] ${diag.type}: ${diag.message}`);
|
console.error(`[pi] [skills] [${purpose}] ${diag.type}: ${diag.message}`);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
skillsOverrideFn = createSkillsOverrideFromSelection(selectionResult, {
|
skillsOverrideFn = createSkillsOverrideFromSelection(selectionResult, {
|
||||||
requestedSkillNames: options.skillSelection.requestedSkillNames,
|
requestedSkillNames: effectiveSkillSelection.requestedSkillNames,
|
||||||
sessionPurpose: options.skillSelection.sessionPurpose,
|
sessionPurpose: effectiveSkillSelection.sessionPurpose,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user