From 9589ce177c57b2a600665fc37c7c6ebe2785273e Mon Sep 17 00:00:00 2001 From: Cameron Date: Thu, 18 Dec 2025 08:32:26 -0800 Subject: [PATCH] fix: Use size-based threshold for skills memory block format (#226) --- src/agent/prompts/skills.mdx | 2 +- src/agent/skills.ts | 110 ++++++++++++++++- src/auth/setup-ui.tsx | 2 +- src/cli/components/FeedbackDialog.tsx | 2 +- src/tests/agent/skills-format.test.ts | 167 ++++++++++++++++++++++++++ 5 files changed, 277 insertions(+), 6 deletions(-) create mode 100644 src/tests/agent/skills-format.test.ts diff --git a/src/agent/prompts/skills.mdx b/src/agent/prompts/skills.mdx index 9c94f3c..252512a 100644 --- a/src/agent/prompts/skills.mdx +++ b/src/agent/prompts/skills.mdx @@ -1,6 +1,6 @@ --- label: skills -description: A memory block listing all available skills with their metadata (name and description). This block is auto-generated based on the `.skills` directory. Do not manually edit this block. +description: A memory block listing all available skills. Auto-generated from the `.skills` directory - do not manually edit. When there are few skills, shows full metadata (name, description). When there are many skills, shows a compact directory tree structure to save space. To use a skill, load it into memory or read the SKILL.md file directly. --- [CURRENTLY EMPTY] diff --git a/src/agent/skills.ts b/src/agent/skills.ts index 380ba84..ee9adeb 100644 --- a/src/agent/skills.ts +++ b/src/agent/skills.ts @@ -51,6 +51,12 @@ export interface SkillDiscoveryError { export const SKILLS_DIR = ".skills"; /** + * Skills block character limit. + * If formatted skills exceed this, fall back to compact tree format. + */ +const SKILLS_BLOCK_CHAR_LIMIT = 20000; + +/** origin/main * Discovers skills by recursively searching for SKILL.MD files * @param skillsPath - The directory to search for skills (default: .skills in current directory) * @returns A result containing discovered skills and any errors @@ -204,12 +210,82 @@ async function parseSkillFile( } /** - * Formats discovered skills as a string for the skills memory block + * Formats skills as a compact directory tree structure * @param skills - Array of discovered skills * @param skillsDirectory - Absolute path to the skills directory - * @returns Formatted string representation of skills + * @returns Tree-structured string representation */ -export function formatSkillsForMemory( +function formatSkillsAsTree(skills: Skill[], skillsDirectory: string): string { + let output = `Skills Directory: ${skillsDirectory}\n\n`; + + if (skills.length === 0) { + return `${output}[NO SKILLS AVAILABLE]`; + } + + output += `Note: Many skills available - showing directory structure only. For each skill path shown below, you can either:\n`; + output += `- Load it persistently into memory using the path (e.g., "ai/tools/mcp-builder")\n`; + output += `- Read ${skillsDirectory}/{path}/SKILL.md directly to preview without loading\n\n`; + + // Build tree structure from skill IDs + interface TreeNode { + [key: string]: TreeNode | null; + } + + const tree: TreeNode = {}; + + // Parse all skill IDs into tree structure + for (const skill of skills) { + const parts = skill.id.split("/"); + let current = tree; + + for (let i = 0; i < parts.length; i++) { + const part = parts[i]; + if (!part) continue; + + // Last part is the skill name (leaf node) + if (i === parts.length - 1) { + current[part] = null; + } else { + // Intermediate directory + if (!current[part]) { + current[part] = {}; + } + current = current[part] as TreeNode; + } + } + } + + // Render tree with indentation + function renderTree(node: TreeNode, indent: string = ""): string { + let result = ""; + const entries = Object.entries(node).sort(([a], [b]) => a.localeCompare(b)); + + for (const [name, children] of entries) { + if (children === null) { + // Leaf node (skill) + result += `${indent}${name}\n`; + } else { + // Directory node + result += `${indent}${name}/\n`; + result += renderTree(children, `${indent} `); + } + } + + return result; + } + + output += renderTree(tree); + + return output.trim(); +} + +/** + * Formats discovered skills with full metadata + * @param skills - Array of discovered skills + * @param skillsDirectory - Absolute path to the skills directory + * @returns Full metadata string representation + */ +function formatSkillsWithMetadata( skills: Skill[], skillsDirectory: string, ): string { @@ -272,3 +348,31 @@ function formatSkill(skill: Skill): string { output += "\n"; return output; } + +/** + * Formats discovered skills as a string for the skills memory block. + * Tries full metadata format first, falls back to compact tree if it exceeds limit. + * @param skills - Array of discovered skills + * @param skillsDirectory - Absolute path to the skills directory + * @returns Formatted string representation of skills + */ +export function formatSkillsForMemory( + skills: Skill[], + skillsDirectory: string, +): string { + // Handle empty case + if (skills.length === 0) { + return `Skills Directory: ${skillsDirectory}\n\n[NO SKILLS AVAILABLE]`; + } + + // Try full metadata format first + const fullFormat = formatSkillsWithMetadata(skills, skillsDirectory); + + // If within limit, use full format + if (fullFormat.length <= SKILLS_BLOCK_CHAR_LIMIT) { + return fullFormat; + } + + // Otherwise fall back to compact tree format + return formatSkillsAsTree(skills, skillsDirectory); +} diff --git a/src/auth/setup-ui.tsx b/src/auth/setup-ui.tsx index 15a0722..1f660c9 100644 --- a/src/auth/setup-ui.tsx +++ b/src/auth/setup-ui.tsx @@ -66,7 +66,7 @@ export function SetupUI({ onComplete }: SetupUIProps) { subprocess.on("error", () => { // Silently ignore - user can still manually visit the URL shown above }); - } catch (openErr) { + } catch (_openErr) { // If auto-open fails, user can still manually visit the URL // This handles cases like missing opener commands in containers } diff --git a/src/cli/components/FeedbackDialog.tsx b/src/cli/components/FeedbackDialog.tsx index da97984..09f6748 100644 --- a/src/cli/components/FeedbackDialog.tsx +++ b/src/cli/components/FeedbackDialog.tsx @@ -12,7 +12,7 @@ export function FeedbackDialog({ onSubmit, onCancel }: FeedbackDialogProps) { const [feedbackText, setFeedbackText] = useState(""); const [error, setError] = useState(""); - useInput((input, key) => { + useInput((_input, key) => { if (key.escape) { onCancel(); } diff --git a/src/tests/agent/skills-format.test.ts b/src/tests/agent/skills-format.test.ts new file mode 100644 index 0000000..fec941b --- /dev/null +++ b/src/tests/agent/skills-format.test.ts @@ -0,0 +1,167 @@ +import { describe, expect, test } from "bun:test"; +import { formatSkillsForMemory, type Skill } from "../../agent/skills"; + +describe("Skills formatting", () => { + test("shows full metadata for small skill collections", () => { + const skills: Skill[] = [ + { + id: "testing", + name: "Testing", + description: "Unit testing patterns and conventions", + path: "/test/.skills/testing/SKILL.md", + }, + { + id: "deployment", + name: "Deployment", + description: "Deployment workflows and scripts", + path: "/test/.skills/deployment/SKILL.md", + }, + ]; + + const result = formatSkillsForMemory(skills, "/test/.skills"); + + // Should contain full metadata + expect(result).toContain("Available Skills:"); + expect(result).toContain("### Testing"); + expect(result).toContain("ID: `testing`"); + expect(result).toContain("Description: Unit testing patterns"); + expect(result).toContain("### Deployment"); + + // Should NOT contain tree format markers + expect(result).not.toContain("Note: Many skills available"); + }); + + test("shows tree format when full metadata exceeds limit", () => { + // Create enough skills with long descriptions to exceed 20k chars + const skills: Skill[] = []; + const longDescription = "A".repeat(500); // 500 chars per description + + for (let i = 0; i < 50; i++) { + skills.push({ + id: `category-${i}/skill-${i}`, + name: `Skill ${i}`, + description: longDescription, + path: `/test/.skills/category-${i}/skill-${i}/SKILL.md`, + }); + } + + const result = formatSkillsForMemory(skills, "/test/.skills"); + + // Should contain tree format markers + expect(result).toContain("Note: Many skills available"); + expect(result).toContain("showing directory structure only"); + + // Should NOT contain full metadata markers + expect(result).not.toContain("Available Skills:"); + expect(result).not.toContain("Description:"); + + // Should show directory structure + expect(result).toContain("category-"); + expect(result).toContain("skill-"); + }); + + test("tree format shows nested directory structure", () => { + const skills: Skill[] = []; + const longDescription = "A".repeat(500); + + // Create nested skills to exceed limit + for (let i = 0; i < 50; i++) { + skills.push({ + id: `ai/tools/tool-${i}`, + name: `Tool ${i}`, + description: longDescription, + path: `/test/.skills/ai/tools/tool-${i}/SKILL.md`, + }); + } + + const result = formatSkillsForMemory(skills, "/test/.skills"); + + // Should show hierarchical structure + expect(result).toContain("ai/"); + expect(result).toContain(" tools/"); + expect(result).toContain(" tool-"); + }); + + test("handles empty skill list", () => { + const result = formatSkillsForMemory([], "/test/.skills"); + + expect(result).toContain("Skills Directory: /test/.skills"); + expect(result).toContain("[NO SKILLS AVAILABLE]"); + }); + + test("tree format includes helper message", () => { + const skills: Skill[] = []; + const longDescription = "A".repeat(500); + + for (let i = 0; i < 50; i++) { + skills.push({ + id: `skill-${i}`, + name: `Skill ${i}`, + description: longDescription, + path: `/test/.skills/skill-${i}/SKILL.md`, + }); + } + + const result = formatSkillsForMemory(skills, "/test/.skills"); + + // Should include usage instructions + expect(result).toContain("Load it persistently into memory"); + expect(result).toContain("Read"); + expect(result).toContain("SKILL.md"); + expect(result).toContain("preview without loading"); + }); + + test("full format respects character limit boundary", () => { + // Create skills that are just under the limit + const skills: Skill[] = []; + + // Each skill formatted is roughly 100 chars, so ~190 skills should be under 20k + for (let i = 0; i < 10; i++) { + skills.push({ + id: `skill-${i}`, + name: `Skill ${i}`, + description: "Short description", + path: `/test/.skills/skill-${i}/SKILL.md`, + }); + } + + const result = formatSkillsForMemory(skills, "/test/.skills"); + + // With short descriptions, should still use full format + expect(result).toContain("Available Skills:"); + expect(result.length).toBeLessThan(20000); + }); + + test("tree format groups skills by directory correctly", () => { + const skills: Skill[] = []; + const longDescription = "A".repeat(500); + + for (let i = 0; i < 30; i++) { + skills.push({ + id: `ai/agents/agent-${i}`, + name: `Agent ${i}`, + description: longDescription, + path: `/test/.skills/ai/agents/agent-${i}/SKILL.md`, + }); + } + + for (let i = 0; i < 30; i++) { + skills.push({ + id: `development/patterns/pattern-${i}`, + name: `Pattern ${i}`, + description: longDescription, + path: `/test/.skills/development/patterns/pattern-${i}/SKILL.md`, + }); + } + + const result = formatSkillsForMemory(skills, "/test/.skills"); + + // Should show both top-level directories + expect(result).toContain("ai/"); + expect(result).toContain("development/"); + + // Should show nested structure + expect(result).toContain(" agents/"); + expect(result).toContain(" patterns/"); + }); +});