From 83bacfcc949146477a101087a3a55c68f93e8228 Mon Sep 17 00:00:00 2001 From: Derek Pearson <32114370+dpearson2699@users.noreply.github.com> Date: Fri, 20 Mar 2026 17:42:28 -0400 Subject: [PATCH] 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. --- .../pi-coding-agent/src/core/agent-session.ts | 145 +++++++++++++++--- .../src/core/skill-tool.test.ts | 89 +++++++++++ packages/pi-coding-agent/src/core/skills.ts | 3 +- .../src/modes/interactive/interactive-mode.ts | 6 +- .../gsd/tests/skill-activation.test.ts | 8 +- 5 files changed, 219 insertions(+), 32 deletions(-) create mode 100644 packages/pi-coding-agent/src/core/skill-tool.test.ts diff --git a/packages/pi-coding-agent/src/core/agent-session.ts b/packages/pi-coding-agent/src/core/agent-session.ts index acd234702..859ab1a7f 100644 --- a/packages/pi-coding-agent/src/core/agent-session.ts +++ b/packages/pi-coding-agent/src/core/agent-session.ts @@ -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 = `\nReferences are relative to ${skill.baseDir}.\n\n${body}\n`; + 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 = {}; @@ -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 = `\nReferences are relative to ${skill.baseDir}.\n\n${body}\n`; - 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 { 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, diff --git a/packages/pi-coding-agent/src/core/skill-tool.test.ts b/packages/pi-coding-agent/src/core/skill-tool.test.ts new file mode 100644 index 000000000..e8a3b2964 --- /dev/null +++ b/packages/pi-coding-agent/src/core/skill-tool.test.ts @@ -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 : "", + `\nReferences are relative to ${join(testDir, ".pi", "skills", "swift-testing")}.\n\n# Swift Testing\nUse this skill.\n`, + ); + }); + + 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/); + }); +}); diff --git a/packages/pi-coding-agent/src/core/skills.ts b/packages/pi-coding-agent/src/core/skills.ts index ba59cf7fa..9868b1546 100644 --- a/packages/pi-coding-agent/src/core/skills.ts +++ b/packages/pi-coding-agent/src/core/skills.ts @@ -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 when the task matches its description.", + "If the Skill tool reports an unknown skill, do not guess: use an exact name from 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.", "", "", diff --git a/packages/pi-coding-agent/src/modes/interactive/interactive-mode.ts b/packages/pi-coding-agent/src/modes/interactive/interactive-mode.ts index beacaebe1..6795d2064 100644 --- a/packages/pi-coding-agent/src/modes/interactive/interactive-mode.ts +++ b/packages/pi-coding-agent/src/modes/interactive/interactive-mode.ts @@ -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); } /** diff --git a/src/resources/extensions/gsd/tests/skill-activation.test.ts b/src/resources/extensions/gsd/tests/skill-activation.test.ts index 23df394ca..e2c6c7be0 100644 --- a/src/resources/extensions/gsd/tests/skill-activation.test.ts +++ b/src/resources/extensions/gsd/tests/skill-activation.test.ts @@ -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, "Call Skill('swift-testing')."); } finally { cleanup(base); }