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
|
||||
|
||||
### 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)
|
||||
|
||||
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 { describeModel, compactSessionContext, COMPACTION_FALLBACK_INSTRUCTIONS } from "./pi.js";
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { describeModel, compactSessionContext, COMPACTION_FALLBACK_INSTRUCTIONS, createKbAgent, type AgentOptions } from "./pi.js";
|
||||
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", () => {
|
||||
it('returns "provider/modelId" when session has a model', () => {
|
||||
const fakeSession = {
|
||||
@@ -115,3 +180,133 @@ describe("compactSessionContext", () => {
|
||||
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
|
||||
* (all discovered skills included). */
|
||||
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(
|
||||
@@ -482,19 +486,30 @@ export async function createKbAgent(options: AgentOptions): Promise<AgentResult>
|
||||
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
|
||||
let skillsOverrideFn: ReturnType<typeof createSkillsOverrideFromSelection> | undefined;
|
||||
if (options.skillSelection) {
|
||||
const selectionResult = resolveSessionSkills(options.skillSelection);
|
||||
if (effectiveSkillSelection) {
|
||||
const selectionResult = resolveSessionSkills(effectiveSkillSelection);
|
||||
if (selectionResult.diagnostics.length > 0) {
|
||||
const purpose = options.skillSelection.sessionPurpose ?? "skills";
|
||||
const purpose = effectiveSkillSelection.sessionPurpose ?? "skills";
|
||||
for (const diag of selectionResult.diagnostics) {
|
||||
console.error(`[pi] [skills] [${purpose}] ${diag.type}: ${diag.message}`);
|
||||
}
|
||||
}
|
||||
skillsOverrideFn = createSkillsOverrideFromSelection(selectionResult, {
|
||||
requestedSkillNames: options.skillSelection.requestedSkillNames,
|
||||
sessionPurpose: options.skillSelection.sessionPurpose,
|
||||
requestedSkillNames: effectiveSkillSelection.requestedSkillNames,
|
||||
sessionPurpose: effectiveSkillSelection.sessionPurpose,
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user