feat(pi): add Skill tool resolution (#1661)
* fix(gsd extension): detect initialized projects in health widget Use .gsd presence plus project-state detection for the health widget so bootstrapped projects no longer appear as unloaded before metrics exist. * fix(gsd extension): detect initialized projects in health widget Use .gsd presence plus project-state detection for the health widget so bootstrapped projects no longer appear as unloaded before metrics exist. * feat: add Skill tool resolution for Pi agent Expose a built-in Skill tool so dispatched prompts can resolve skill names without guessing file paths. This aligns runtime behavior with skill activation prompts and adds coverage for exact activation and unknown-skill handling.
This commit is contained in:
parent
bb91b05137
commit
83bacfcc94
5 changed files with 219 additions and 32 deletions
|
|
@ -25,6 +25,7 @@ import type {
|
|||
} from "@gsd/pi-agent-core";
|
||||
import type { AssistantMessage, ImageContent, Message, Model, TextContent } from "@gsd/pi-ai";
|
||||
import { modelsAreEqual, resetApiProviders, supportsXhigh } from "@gsd/pi-ai";
|
||||
import { Type } from "@sinclair/typebox";
|
||||
import { getDocsPath } from "../config.js";
|
||||
import { getErrorMessage } from "../utils/error.js";
|
||||
import { theme } from "../modes/interactive/theme/theme.js";
|
||||
|
|
@ -732,9 +733,10 @@ export class AgentSession {
|
|||
* Changes take effect on the next agent turn.
|
||||
*/
|
||||
setActiveToolsByName(toolNames: string[]): void {
|
||||
const requestedToolNames = [...new Set([...toolNames, ...this._getBuiltinToolNames()])];
|
||||
const tools: AgentTool[] = [];
|
||||
const validToolNames: string[] = [];
|
||||
for (const name of toolNames) {
|
||||
for (const name of requestedToolNames) {
|
||||
const tool = this._toolRegistry.get(name);
|
||||
if (tool) {
|
||||
tools.push(tool);
|
||||
|
|
@ -743,6 +745,7 @@ export class AgentSession {
|
|||
}
|
||||
this.agent.setTools(tools);
|
||||
|
||||
|
||||
// Rebuild base system prompt with new tool set
|
||||
this._baseSystemPrompt = this._rebuildSystemPrompt(validToolNames);
|
||||
this.agent.setSystemPrompt(this._baseSystemPrompt);
|
||||
|
|
@ -858,6 +861,48 @@ export class AgentSession {
|
|||
return Array.from(unique);
|
||||
}
|
||||
|
||||
private _findSkillByName(skillName: string) {
|
||||
return this.resourceLoader.getSkills().skills.find((skill) => skill.name === skillName);
|
||||
}
|
||||
|
||||
private _formatMissingSkillMessage(skillName: string): string {
|
||||
const availableSkills = this.resourceLoader.getSkills().skills.map((skill) => skill.name).join(", ") || "(none)";
|
||||
return `Skill "${skillName}" not found. Available skills: ${availableSkills}`;
|
||||
}
|
||||
|
||||
private _emitSkillExpansionError(skillFilePath: string, err: unknown): void {
|
||||
this._extensionRunner?.emitError({
|
||||
extensionPath: skillFilePath,
|
||||
event: "skill_expansion",
|
||||
error: getErrorMessage(err),
|
||||
});
|
||||
}
|
||||
|
||||
private _renderSkillInvocation(skill: { name: string; filePath: string; baseDir: string }, args?: string): string {
|
||||
const content = readFileSync(skill.filePath, "utf-8");
|
||||
const body = stripFrontmatter(content).trim();
|
||||
const skillBlock = `<skill name="${skill.name}" location="${skill.filePath}">\nReferences are relative to ${skill.baseDir}.\n\n${body}\n</skill>`;
|
||||
return args && args.trim() ? `${skillBlock}\n\n${args.trim()}` : skillBlock;
|
||||
}
|
||||
|
||||
private _expandSkillByName(skillName: string, args?: string): string {
|
||||
const skill = this._findSkillByName(skillName);
|
||||
if (!skill) {
|
||||
throw new Error(this._formatMissingSkillMessage(skillName));
|
||||
}
|
||||
|
||||
try {
|
||||
return this._renderSkillInvocation(skill, args);
|
||||
} catch (err) {
|
||||
this._emitSkillExpansionError(skill.filePath, err);
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
private _formatSkillInvocation(skillName: string, args?: string): string {
|
||||
return this._expandSkillByName(skillName, args);
|
||||
}
|
||||
|
||||
private _rebuildSystemPrompt(toolNames: string[]): string {
|
||||
const validToolNames = toolNames.filter((name) => this._toolRegistry.has(name));
|
||||
const toolSnippets: Record<string, string> = {};
|
||||
|
|
@ -1103,25 +1148,78 @@ export class AgentSession {
|
|||
const skillName = spaceIndex === -1 ? text.slice(7) : text.slice(7, spaceIndex);
|
||||
const args = spaceIndex === -1 ? "" : text.slice(spaceIndex + 1).trim();
|
||||
|
||||
const skill = this.resourceLoader.getSkills().skills.find((s) => s.name === skillName);
|
||||
if (!skill) return text; // Unknown skill, pass through
|
||||
if (!this._findSkillByName(skillName)) return text;
|
||||
|
||||
try {
|
||||
const content = readFileSync(skill.filePath, "utf-8");
|
||||
const body = stripFrontmatter(content).trim();
|
||||
const skillBlock = `<skill name="${skill.name}" location="${skill.filePath}">\nReferences are relative to ${skill.baseDir}.\n\n${body}\n</skill>`;
|
||||
return args ? `${skillBlock}\n\n${args}` : skillBlock;
|
||||
} catch (err) {
|
||||
// Emit error like extension commands do
|
||||
this._extensionRunner?.emitError({
|
||||
extensionPath: skill.filePath,
|
||||
event: "skill_expansion",
|
||||
error: getErrorMessage(err),
|
||||
});
|
||||
return text; // Return original on error
|
||||
return this._formatSkillInvocation(skillName, args);
|
||||
} catch {
|
||||
return text;
|
||||
}
|
||||
}
|
||||
|
||||
private _createBuiltInSkillTool(): AgentTool {
|
||||
const skillSchema = Type.Object({
|
||||
skill: Type.String({ description: "The skill name. E.g., 'commit', 'review-pr', or 'pdf'" }),
|
||||
args: Type.Optional(Type.String({ description: "Optional arguments for the skill" })),
|
||||
});
|
||||
|
||||
return {
|
||||
name: "Skill",
|
||||
label: "Skill",
|
||||
description:
|
||||
"Execute a skill within the main conversation. Use this tool when users ask for a slash command or reference a skill by name. Returns the expanded skill block and appends args after it.",
|
||||
parameters: skillSchema,
|
||||
execute: async (_toolCallId, params: unknown) => {
|
||||
const input = params as { skill: string; args?: string };
|
||||
try {
|
||||
return {
|
||||
content: [
|
||||
{
|
||||
type: "text",
|
||||
text: this._expandSkillByName(input.skill, input.args),
|
||||
},
|
||||
],
|
||||
details: undefined,
|
||||
};
|
||||
} catch (err) {
|
||||
return {
|
||||
content: [{ type: "text", text: getErrorMessage(err) }],
|
||||
details: undefined,
|
||||
};
|
||||
}
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
private _getBuiltinToolNames(): string[] {
|
||||
return this._getBuiltinTools().map((tool) => tool.name);
|
||||
}
|
||||
|
||||
private _getBuiltinTools(): AgentTool[] {
|
||||
return [this._createBuiltInSkillTool()];
|
||||
}
|
||||
|
||||
private _getRegisteredToolDefinitions(): ToolDefinition[] {
|
||||
const registeredTools = this._extensionRunner?.getAllRegisteredTools() ?? [];
|
||||
return registeredTools.map((tool) => tool.definition);
|
||||
}
|
||||
|
||||
private _getBuiltinToolDefinitions(): ToolDefinition[] {
|
||||
return this._getBuiltinTools().map((tool) => ({
|
||||
name: tool.name,
|
||||
label: tool.label,
|
||||
description: tool.description,
|
||||
parameters: tool.parameters,
|
||||
execute: async () => ({ content: [], details: undefined }),
|
||||
}));
|
||||
}
|
||||
|
||||
getRenderableToolDefinition(toolName: string): ToolDefinition | undefined {
|
||||
return [...this._getBuiltinToolDefinitions(), ...this._getRegisteredToolDefinitions()].find(
|
||||
(tool) => tool.name === toolName,
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Queue a steering message to interrupt the agent mid-run.
|
||||
* Delivered after current tool execution, skips remaining tools.
|
||||
|
|
@ -1967,8 +2065,12 @@ export class AgentSession {
|
|||
const wrappedExtensionTools = this._extensionRunner
|
||||
? wrapRegisteredTools(allCustomTools, this._extensionRunner)
|
||||
: [];
|
||||
const builtinTools = this._getBuiltinTools();
|
||||
|
||||
const toolRegistry = new Map(this._baseToolRegistry);
|
||||
for (const tool of builtinTools) {
|
||||
toolRegistry.set(tool.name, tool);
|
||||
}
|
||||
for (const tool of wrappedExtensionTools as AgentTool[]) {
|
||||
toolRegistry.set(tool.name, tool);
|
||||
}
|
||||
|
|
@ -2694,14 +2796,11 @@ export class AgentSession {
|
|||
async exportToHtml(outputPath?: string): Promise<string> {
|
||||
const themeName = this.settingsManager.getTheme();
|
||||
|
||||
// Create tool renderer if we have an extension runner (for custom tool HTML rendering)
|
||||
let toolRenderer: ToolHtmlRenderer | undefined;
|
||||
if (this._extensionRunner) {
|
||||
toolRenderer = createToolHtmlRenderer({
|
||||
getToolDefinition: (name) => this._extensionRunner!.getToolDefinition(name),
|
||||
theme,
|
||||
});
|
||||
}
|
||||
// Create tool renderer for extension and built-in tool HTML rendering
|
||||
const toolRenderer = createToolHtmlRenderer({
|
||||
getToolDefinition: (name) => this.getRenderableToolDefinition(name),
|
||||
theme,
|
||||
});
|
||||
|
||||
return await exportSessionToHtml(this.sessionManager, this.state, {
|
||||
outputPath,
|
||||
|
|
|
|||
89
packages/pi-coding-agent/src/core/skill-tool.test.ts
Normal file
89
packages/pi-coding-agent/src/core/skill-tool.test.ts
Normal file
|
|
@ -0,0 +1,89 @@
|
|||
import assert from "node:assert/strict";
|
||||
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { afterEach, beforeEach, describe, it } from "node:test";
|
||||
|
||||
import { Agent } from "@gsd/pi-agent-core";
|
||||
import { AuthStorage } from "./auth-storage.js";
|
||||
import { AgentSession } from "./agent-session.js";
|
||||
import { ModelRegistry } from "./model-registry.js";
|
||||
import { DefaultResourceLoader } from "./resource-loader.js";
|
||||
import { SessionManager } from "./session-manager.js";
|
||||
import { SettingsManager } from "./settings-manager.js";
|
||||
|
||||
let testDir: string;
|
||||
|
||||
function writeSkill(cwd: string, name: string, description: string, body = `# ${name}\n`): string {
|
||||
const skillDir = join(cwd, ".pi", "skills", name);
|
||||
mkdirSync(skillDir, { recursive: true });
|
||||
const skillPath = join(skillDir, "SKILL.md");
|
||||
writeFileSync(skillPath, `---\nname: ${name}\ndescription: ${description}\n---\n\n${body}`);
|
||||
return skillPath;
|
||||
}
|
||||
|
||||
describe("Skill tool", () => {
|
||||
beforeEach(() => {
|
||||
testDir = mkdtempSync(join(tmpdir(), "skill-tool-test-"));
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
rmSync(testDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
async function createSession() {
|
||||
const agentDir = join(testDir, "agent-home");
|
||||
const authStorage = AuthStorage.inMemory({});
|
||||
const modelRegistry = new ModelRegistry(authStorage, join(agentDir, "models.json"));
|
||||
const settingsManager = SettingsManager.inMemory();
|
||||
const resourceLoader = new DefaultResourceLoader({
|
||||
cwd: testDir,
|
||||
agentDir,
|
||||
settingsManager,
|
||||
noExtensions: true,
|
||||
noPromptTemplates: true,
|
||||
noThemes: true,
|
||||
});
|
||||
await resourceLoader.reload();
|
||||
|
||||
return new AgentSession({
|
||||
agent: new Agent(),
|
||||
sessionManager: SessionManager.inMemory(testDir),
|
||||
settingsManager,
|
||||
cwd: testDir,
|
||||
resourceLoader,
|
||||
modelRegistry,
|
||||
});
|
||||
}
|
||||
|
||||
it("resolves a project-level skill to the exact skill block format", async () => {
|
||||
const skillPath = writeSkill(
|
||||
testDir,
|
||||
"swift-testing",
|
||||
"Use for Swift Testing assertions and verification patterns.",
|
||||
"# Swift Testing\nUse this skill.\n",
|
||||
);
|
||||
const session = await createSession();
|
||||
|
||||
const tool = session.state.tools.find((entry) => entry.name === "Skill");
|
||||
assert.ok(tool, "Skill tool should be registered");
|
||||
|
||||
const result = await tool.execute("call-1", { skill: "swift-testing" });
|
||||
assert.equal(
|
||||
result.content[0]?.type === "text" ? result.content[0].text : "",
|
||||
`<skill name="swift-testing" location="${skillPath}">\nReferences are relative to ${join(testDir, ".pi", "skills", "swift-testing")}.\n\n# Swift Testing\nUse this skill.\n</skill>`,
|
||||
);
|
||||
});
|
||||
|
||||
it("returns a helpful error for unknown skills", async () => {
|
||||
writeSkill(testDir, "swift-testing", "Use for Swift Testing assertions and verification patterns.");
|
||||
const session = await createSession();
|
||||
const tool = session.state.tools.find((entry) => entry.name === "Skill");
|
||||
assert.ok(tool, "Skill tool should be registered");
|
||||
|
||||
const result = await tool.execute("call-2", { skill: "nonexistent" });
|
||||
const message = result.content[0]?.type === "text" ? result.content[0].text : "";
|
||||
assert.match(message, /^Skill "nonexistent" not found\. Available skills: /);
|
||||
assert.match(message, /swift-testing/);
|
||||
});
|
||||
});
|
||||
|
|
@ -299,7 +299,8 @@ export function formatSkillsForPrompt(skills: Skill[]): string {
|
|||
|
||||
const lines = [
|
||||
"\n\nThe following skills provide specialized instructions for specific tasks.",
|
||||
"Use the read tool to load a skill's file when the task matches its description.",
|
||||
"Use the Skill tool with the exact skill name from <available_skills> when the task matches its description.",
|
||||
"If the Skill tool reports an unknown skill, do not guess: use an exact name from <available_skills> or tell the user the skill is unavailable.",
|
||||
"When a skill file references a relative path, resolve it against the skill directory (parent of SKILL.md / dirname of the path) and use that absolute path in tool commands.",
|
||||
"",
|
||||
"<available_skills>",
|
||||
|
|
|
|||
|
|
@ -1184,12 +1184,10 @@ export class InteractiveMode {
|
|||
}
|
||||
|
||||
/**
|
||||
* Get a registered tool definition by name (for custom rendering).
|
||||
* Get a tool definition by name (for custom rendering).
|
||||
*/
|
||||
private getRegisteredToolDefinition(toolName: string) {
|
||||
const tools = this.session.extensionRunner?.getAllRegisteredTools() ?? [];
|
||||
const registeredTool = tools.find((t) => t.definition.name === toolName);
|
||||
return registeredTool?.definition;
|
||||
return this.session.getRenderableToolDefinition(toolName);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -60,17 +60,17 @@ test("buildSkillActivationBlock matches installed skills from task context", ()
|
|||
}
|
||||
});
|
||||
|
||||
test("buildSkillActivationBlock includes always_use_skills from preferences", () => {
|
||||
test("buildSkillActivationBlock includes always_use_skills from preferences using exact Skill tool format", () => {
|
||||
const base = makeTempBase();
|
||||
try {
|
||||
writeSkill(base, "testing", "Use for test setup, assertions, and verification patterns.");
|
||||
writeSkill(base, "swift-testing", "Use for Swift Testing assertions and verification patterns.");
|
||||
loadOnlyTestSkills(base);
|
||||
|
||||
const result = buildBlock(base, { taskTitle: "Unrelated task title" }, {
|
||||
always_use_skills: ["testing"],
|
||||
always_use_skills: ["swift-testing"],
|
||||
});
|
||||
|
||||
assert.match(result, /Call Skill\('testing'\)/);
|
||||
assert.equal(result, "<skill_activation>Call Skill('swift-testing').</skill_activation>");
|
||||
} finally {
|
||||
cleanup(base);
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue