diff --git a/src/capabilities/registry.ts b/src/capabilities/registry.ts index 7d98f604..95a5c3e6 100644 --- a/src/capabilities/registry.ts +++ b/src/capabilities/registry.ts @@ -15,6 +15,8 @@ import { createApproveCommandTool } from './shell/approve-command.js'; import { createInstallSkillTool } from './skills/install-skill.js'; import { createListSkillsTool } from './skills/list-skills.js'; import { createUseSkillTool } from './skills/use-skill.js'; +import { createSkillViewTool } from './skills/skill-view.js'; +import { createSkillManageTool } from './skills/skill-manage.js'; import { createScheduleTaskTool } from './scheduler/schedule-task.js'; import { createListTasksTool } from './scheduler/list-tasks.js'; import { createCancelTaskTool } from './scheduler/cancel-task.js'; @@ -139,6 +141,8 @@ export class CapabilityRegistry { this.tools.install_skill = createInstallSkillTool(this.skillLoader); this.tools.list_skills = createListSkillsTool(this.skillLoader); this.tools.use_skill = createUseSkillTool(this.skillLoader, this.permissions); + this.tools.skill_view = createSkillViewTool(this.skillLoader); + this.tools.skill_manage = createSkillManageTool(this.skillLoader); logger.info('Skill tools registered'); } diff --git a/src/capabilities/skills/skill-manage.ts b/src/capabilities/skills/skill-manage.ts new file mode 100644 index 00000000..c31ae2b4 --- /dev/null +++ b/src/capabilities/skills/skill-manage.ts @@ -0,0 +1,619 @@ +/** + * @fileoverview Skill Management Tool - Agent-Managed Skill Creation & Editing + * + * This tool allows the agent to create, update, and delete skills, turning successful + * approaches into reusable procedural knowledge. New skills are created in + * ~/.mercury/skills/. Existing skills can be modified or deleted wherever they live. + * + * Skills are the agent's procedural memory: they capture *how to do a specific + * type of task* based on proven experience. General memory (Second Brain) is + * broad and declarative. Skills are narrow and actionable. + * + * Actions: + * - create -- Create a new skill (SKILL.md + directory structure) + * - edit -- Replace the SKILL.md content of a user skill (full rewrite) + * - patch -- Targeted find-and-replace within SKILL.md or supporting files + * - delete -- Remove a user skill entirely + * - write-file -- Add/overwrite a supporting file (reference, template, script, asset) + * - remove-file-- Remove a supporting file from a user skill + */ + +import { tool, zodSchema } from 'ai'; +import { z } from 'zod'; +import { existsSync, readFileSync, writeFileSync, unlinkSync, mkdirSync, readdirSync, rmdirSync } from 'node:fs'; +import { join, dirname } from 'node:path'; +import { getMercuryHome } from '../../utils/config.js'; +import { logger } from '../../utils/logger.js'; +import type { SkillLoader } from '../../skills/loader.js'; +import { + validateSkillName, + validateCategory, + validateSkillContent, + validateContentSize, + validateFilePath, + isLocalSkill, + findSkill, + resolveSkillDir, + extractFrontmatter, + MAX_NAME_LENGTH, + MAX_DESCRIPTION_LENGTH, + MAX_SKILL_CONTENT_CHARS, + MAX_SKILL_FILE_BYTES, +} from '../../skills/utils.js'; + +const SKILL_FILE = 'SKILL.md'; + +/** Allowed subdirectories for supporting files */ +const ALLOWED_SUBDIRS = ['references', 'templates', 'scripts', 'assets']; + +/** + * Parameters for the skill_manage tool + * @interface SkillManageParams + */ +interface SkillManageParams { + /** The action to perform */ + action: 'create' | 'patch' | 'edit' | 'delete' | 'write-file' | 'remove-file'; + /** Skill name (lowercase, hyphens/underscores, max 64 chars) */ + name: string; + /** Full SKILL.md content for create/edit actions */ + content?: string; + /** Text to find (required for patch) */ + oldString?: string; + /** Replacement text (required for patch) */ + newString?: string; + /** Optional category for organization */ + category?: string; + /** Path to a supporting file */ + filePath?: string; + /** Content for write-file action */ + fileContent?: string; + /** Replace all occurrences (for patch) */ + replaceAll?: boolean; +} + +/** + * Result object for skill management operations + * @interface SkillManageResult + */ +interface SkillManageResult { + success: boolean; + message?: string; + error?: string; + path?: string; + skillMd?: string; + category?: string; + hint?: string; + availableFiles?: string[]; + filePreview?: string; +} + +/** + * Atomic write - writes to temp file first, then replaces target. + * Ensures target is never left in partially-written state. + * @param filePath - Target file path + * @param content - Content to write + * @param encoding - Text encoding (default: utf-8) + */ +function atomicWrite(filePath: string, content: string, encoding: BufferEncoding = 'utf-8'): void { + const tempDir = dirname(filePath); + const fileName = filePath.split(/[/\\]/).pop() || 'tmp'; + const tempPath = join(tempDir, `.${fileName}.tmp.${Date.now()}`); + + try { + writeFileSync(tempPath, content, encoding); + // Atomic rename + try { + const { renameSync } = require('node:fs'); + renameSync(tempPath, filePath); + } catch { + // Fallback: copy content and delete temp + writeFileSync(filePath, content, encoding); + try { unlinkSync(tempPath); } catch { /* ignore */ } + } + } catch (err) { + try { unlinkSync(tempPath); } catch { /* ignore */ } + throw err; + } +} + +/** + * Create a new skill with SKILL.md content. + * @param name - Skill name + * @param content - Full SKILL.md content with YAML frontmatter + * @param category - Optional category for organization + * @param skillsDir - Skills directory path + * @returns SkillManageResult with success/error + */ +function createSkill( + name: string, + content: string, + category: string | null, + skillsDir: string +): SkillManageResult { + // Validate name + const nameErr = validateSkillName(name); + if (nameErr) return { success: false, error: nameErr }; + + // Validate category + const catErr = validateCategory(category); + if (catErr) return { success: false, error: catErr }; + + // Validate content + const contentErr = validateSkillContent(content); + if (contentErr) return { success: false, error: contentErr }; + + // Validate size + const sizeErr = validateContentSize(content); + if (sizeErr) return { success: false, error: sizeErr }; + + // Check for name collisions + const existing = findSkill(name, skillsDir); + if (existing) { + return { success: false, error: `A skill named '${name}' already exists at ${existing}.` }; + } + + // Create the skill directory + const skillDir = resolveSkillDir(name, category, skillsDir); + if (!existsSync(skillDir)) { + mkdirSync(skillDir, { recursive: true }); + } + + // Write SKILL.md + const skillMd = join(skillDir, SKILL_FILE); + try { + atomicWrite(skillMd, content); + } catch (err: any) { + return { success: false, error: `Failed to write skill file: ${err.message}` }; + } + + logger.info({ skill: name, path: skillDir }, 'Skill created'); + + const result: SkillManageResult = { + success: true, + message: `Skill '${name}' created.`, + path: skillDir.replace(/\\/g, '/').replace(skillsDir.replace(/\\/g, '/') + '/', ''), + skillMd, + }; + + if (category) { + result.category = category; + } + + result.hint = `To add reference files, templates, or scripts, use skill_manage(action='write-file', name='${name}', file_path='references/example.md', file_content='...')`; + + return result; +} + +/** + * Edit/replace the SKILL.md of an existing skill. + * @param name - Skill name + * @param content - Full updated SKILL.md content + * @param skillsDir - Skills directory path + * @returns SkillManageResult with success/error + */ +function editSkill(name: string, content: string, skillsDir: string): SkillManageResult { + // Validate content + const contentErr = validateSkillContent(content); + if (contentErr) return { success: false, error: contentErr }; + + // Validate size + const sizeErr = validateContentSize(content); + if (sizeErr) return { success: false, error: sizeErr }; + + const skillDir = findSkill(name, skillsDir); + if (!skillDir) { + return { success: false, error: `Skill '${name}' not found. Use list_skills to see available skills.` }; + } + + if (!isLocalSkill(skillDir, skillsDir)) { + return { success: false, error: `Skill '${name}' is in an external directory and cannot be modified.` }; + } + + const skillMd = join(skillDir, SKILL_FILE); + + try { + atomicWrite(skillMd, content); + } catch (err: any) { + return { success: false, error: `Failed to write skill file: ${err.message}` }; + } + + logger.info({ skill: name }, 'Skill updated'); + + return { + success: true, + message: `Skill '${name}' updated.`, + path: skillDir.replace(/\\/g, '/').replace(skillsDir.replace(/\\/g, '/') + '/', ''), + }; +} + +/** + * Patch a skill file with find-and-replace. + * @param name - Skill name + * @param oldString - Text to find + * @param newString - Replacement text (can be empty to delete) + * @param filePath - Optional file path (defaults to SKILL.md) + * @param replaceAll - Replace all occurrences + * @param skillsDir - Skills directory path + * @returns SkillManageResult with success/error + */ +function patchSkill( + name: string, + oldString: string, + newString: string | null, + filePath: string | null, + replaceAll: boolean, + skillsDir: string +): SkillManageResult { + if (!oldString) { + return { success: false, error: 'old_string is required for patch.' }; + } + + if (newString === undefined) { + return { success: false, error: "new_string is required for patch. Use empty string to delete matched text." }; + } + + const skillDir = findSkill(name, skillsDir); + if (!skillDir) { + return { success: false, error: `Skill '${name}' not found.` }; + } + + if (!isLocalSkill(skillDir, skillsDir)) { + return { success: false, error: `Skill '${name}' is in an external directory and cannot be modified.` }; + } + + let targetPath: string; + if (filePath) { + const pathErr = validateFilePath(filePath); + if (pathErr) return { success: false, error: pathErr }; + + if (filePath.includes('..')) { + return { success: false, error: "Path traversal ('..') is not allowed." }; + } + targetPath = join(skillDir, filePath); + } else { + targetPath = join(skillDir, SKILL_FILE); + } + + if (!existsSync(targetPath)) { + return { success: false, error: `File not found: ${filePath || 'SKILL.md'}` }; + } + + const content = readFileSync(targetPath, 'utf-8'); + let matchCount = 0; + let newContent: string; + + if (replaceAll) { + const regex = new RegExp(escapeRegex(oldString), 'g'); + const matches = content.match(regex); + matchCount = matches ? matches.length : 0; + newContent = content.replace(regex, newString ?? ''); + } else { + const idx = content.indexOf(oldString); + if (idx === -1) { + const preview = content.slice(0, 500) + (content.length > 500 ? '...' : ''); + return { + success: false, + error: `Could not find '${oldString.length > 50 ? oldString.slice(0, 50) + '...' : oldString}' in the file. Make sure the text matches exactly.`, + filePreview: preview, + }; + } + matchCount = 1; + newContent = content.slice(0, idx) + (newString ?? '') + content.slice(idx + oldString.length); + } + + // Validate size + const sizeErr = validateContentSize(newContent, filePath || 'SKILL.md'); + if (sizeErr) return { success: false, error: sizeErr }; + + // If patching SKILL.md, validate frontmatter still intact + if (!filePath) { + const contentErr = validateSkillContent(newContent); + if (contentErr) { + return { success: false, error: `Patch would break SKILL.md structure: ${contentErr}` }; + } + } + + try { + atomicWrite(targetPath, newContent); + } catch (err: any) { + return { success: false, error: `Failed to write file: ${err.message}` }; + } + + logger.info({ skill: name, file: filePath || 'SKILL.md', count: matchCount }, 'Skill patched'); + + return { + success: true, + message: `Patched ${filePath || 'SKILL.md'} in skill '${name}' (${matchCount} replacement${matchCount !== 1 ? 's' : ''}).`, + path: skillDir.replace(/\\/g, '/').replace(skillsDir.replace(/\\/g, '/') + '/', ''), + }; +} + +/** + * Delete a skill. + * @param name - Skill name + * @param skillsDir - Skills directory path + * @returns SkillManageResult with success/error + */ +function deleteSkill(name: string, skillsDir: string): SkillManageResult { + const skillDir = findSkill(name, skillsDir); + if (!skillDir) { + return { success: false, error: `Skill '${name}' not found.` }; + } + + if (!isLocalSkill(skillDir, skillsDir)) { + return { success: false, error: `Skill '${name}' is in an external directory and cannot be deleted.` }; + } + + try { + // Delete all files in the skill directory + const deleteRecursive = (dir: string) => { + if (!existsSync(dir)) return; + const entries = readdirSync(dir, { withFileTypes: true }); + for (const entry of entries) { + const fullPath = join(dir, entry.name); + if (entry.isDirectory()) { + deleteRecursive(fullPath); + try { rmdirSync(fullPath); } catch { /* ignore */ } + } else { + try { unlinkSync(fullPath); } catch { /* ignore */ } + } + } + }; + deleteRecursive(skillDir); + + // Remove the skill directory itself + try { rmdirSync(skillDir); } catch { /* ignore */ } + + // Clean up empty category directories + const parent = dirname(skillDir); + if (parent !== skillsDir && existsSync(parent)) { + try { + if (readdirSync(parent).length === 0) { + rmdirSync(parent); + } + } catch { /* ignore */ } + } + } catch (err: any) { + return { success: false, error: `Failed to delete skill: ${err.message}` }; + } + + logger.info({ skill: name }, 'Skill deleted'); + + return { + success: true, + message: `Skill '${name}' deleted.`, + }; +} + +/** + * Write a supporting file to a skill. + * @param name - Skill name + * @param filePath - Path within skill (references/, templates/, scripts/, assets/) + * @param fileContent - Content to write + * @param skillsDir - Skills directory path + * @returns SkillManageResult with success/error + */ +function writeSkillFile( + name: string, + filePath: string, + fileContent: string, + skillsDir: string +): SkillManageResult { + const pathErr = validateFilePath(filePath); + if (pathErr) return { success: false, error: pathErr }; + + if (fileContent === undefined) { + return { success: false, error: 'file_content is required for write-file.' }; + } + + const skillDir = findSkill(name, skillsDir); + if (!skillDir) { + return { success: false, error: `Skill '${name}' not found. Create it first with action='create'.` }; + } + + if (!isLocalSkill(skillDir, skillsDir)) { + return { success: false, error: `Skill '${name}' is in an external directory and cannot be modified.` }; + } + + // Check size + const contentBytes = Buffer.byteLength(fileContent, 'utf-8'); + if (contentBytes > MAX_SKILL_FILE_BYTES) { + return { + success: false, + error: `File content is ${contentBytes.toLocaleString()} bytes (limit: ${MAX_SKILL_FILE_BYTES.toLocaleString()} bytes / 1 MiB). Consider splitting into smaller files.`, + }; + } + + const sizeErr = validateContentSize(fileContent, filePath); + if (sizeErr) return { success: false, error: sizeErr }; + + const targetPath = join(skillDir, filePath); + try { + mkdirSync(dirname(targetPath), { recursive: true }); + atomicWrite(targetPath, fileContent); + } catch (err: any) { + return { success: false, error: `Failed to write file: ${err.message}` }; + } + + logger.info({ skill: name, file: filePath }, 'Skill file written'); + + return { + success: true, + message: `File '${filePath}' written to skill '${name}'.`, + path: targetPath, + }; +} + +/** + * Remove a supporting file from a skill. + * @param name - Skill name + * @param filePath - Path within skill to remove + * @param skillsDir - Skills directory path + * @returns SkillManageResult with success/error + */ +function removeSkillFile(name: string, filePath: string, skillsDir: string): SkillManageResult { + const pathErr = validateFilePath(filePath); + if (pathErr) return { success: false, error: pathErr }; + + const skillDir = findSkill(name, skillsDir); + if (!skillDir) { + return { success: false, error: `Skill '${name}' not found.` }; + } + + if (!isLocalSkill(skillDir, skillsDir)) { + return { success: false, error: `Skill '${name}' is in an external directory and cannot be modified.` }; + } + + const targetPath = join(skillDir, filePath); + if (!existsSync(targetPath)) { + // List available files for feedback + const available: string[] = []; + for (const subdir of ALLOWED_SUBDIRS) { + const subdirPath = join(skillDir, subdir); + if (existsSync(subdirPath)) { + const files = readdirSync(subdirPath); + for (const f of files) { + available.push(`${subdir}/${f}`); + } + } + } + return { + success: false, + error: `File '${filePath}' not found in skill '${name}'.`, + availableFiles: available.length > 0 ? available : undefined, + }; + } + + try { + unlinkSync(targetPath); + + // Clean up empty subdirectories + const subdirPath = dirname(targetPath); + if (subdirPath !== skillDir && existsSync(subdirPath)) { + try { + if (readdirSync(subdirPath).length === 0) { + rmdirSync(subdirPath); + } + } catch { /* ignore */ } + } + } catch (err: any) { + return { success: false, error: `Failed to remove file: ${err.message}` }; + } + + logger.info({ skill: name, file: filePath }, 'Skill file removed'); + + return { + success: true, + message: `File '${filePath}' removed from skill '${name}'.`, + }; +} + +/** + * Escape special regex characters in a string. + * @param str - String to escape + * @returns Escaped string + */ +function escapeRegex(str: string): string { + return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + +/** + * Creates the skill_manage tool for the capability registry. + * @param skillLoader - The skill loader instance for discovery refresh + * @returns AI tool definition for skill management + */ +export function createSkillManageTool(skillLoader: SkillLoader) { + return tool({ + description: `Manage skills (create, update, delete). Skills are your procedural memory - reusable approaches for recurring task types. Skills go to ~/.mercury/skills/; existing skills can be modified wherever they live. + +Actions: +- create: Create a new skill (SKILL.md + optional category subdirectory) +- patch: Targeted find-and-replace within SKILL.md or supporting files (preferred for fixes) +- edit: Full SKILL.md rewrite (for major overhauls only) +- delete: Remove a user skill entirely +- write-file: Add/overwrite a supporting file (reference, template, script, asset) +- remove-file: Remove a supporting file from a skill + +Create when: complex task succeeded (5+ tool calls), errors overcame, user-corrected approach worked, non-trivial workflow discovered, or user asks to remember a procedure. +Update when: instructions stale/wrong, OS-specific failures, missing steps or pitfalls found during use. If you used a skill and hit issues not covered, patch it immediately. +Prefer updating/patching existing skills over creating new ones. Survey existing skills first (list_skills, then skill_view) before creating. +Confirm with user before creating/deleting skills. +Good skills: trigger conditions, numbered steps with exact commands, pitfalls section, verification steps.`, + + inputSchema: zodSchema(z.object({ + action: z.enum(['create', 'patch', 'edit', 'delete', 'write-file', 'remove-file']).describe('The action to perform.'), + name: z.string().max(MAX_NAME_LENGTH).describe('Skill name (lowercase, hyphens/underscores, max 64 chars). Must match existing skill for patch/edit/delete/write-file/remove-file.'), + content: z.string().optional().describe('Full SKILL.md content (YAML frontmatter + markdown body). Required for create and edit.'), + oldString: z.string().optional().describe('Text to find in the file (required for patch). Include enough context to ensure uniqueness unless replaceAll=true.'), + newString: z.string().optional().describe('Replacement text (required for patch). Can be empty string to delete matched text.'), + replaceAll: z.boolean().optional().describe('For patch: replace all occurrences instead of requiring unique match (default: false).'), + category: z.string().optional().describe('Optional category/domain for organizing the skill (e.g., devops, data-science, mlops). Creates a subdirectory grouping. Only for create.'), + filePath: z.string().optional().describe('Path to a supporting file within the skill directory. For write-file/remove-file: required, must be under references/, templates/, scripts/, or assets/. For patch: optional, defaults to SKILL.md.'), + fileContent: z.string().optional().describe('Content for the file. Required for write-file.'), + })), + + execute: async ({ action, name, content, oldString, newString, replaceAll, category, filePath, fileContent }: SkillManageParams) => { + const skillsDir = join(getMercuryHome(), 'skills'); + + let result: SkillManageResult; + + switch (action) { + case 'create': + if (!content) { + result = { success: false, error: 'content is required for create. Provide the full SKILL.md text (frontmatter + body).' }; + } else { + result = createSkill(name, content, category || null, skillsDir); + } + break; + + case 'edit': + if (!content) { + result = { success: false, error: 'content is required for edit. Provide the full updated SKILL.md text.' }; + } else { + result = editSkill(name, content, skillsDir); + } + break; + + case 'patch': + result = patchSkill(name, oldString || '', newString ?? null, filePath || null, replaceAll || false, skillsDir); + break; + + case 'delete': + result = deleteSkill(name, skillsDir); + break; + + case 'write-file': + if (!filePath) { + result = { success: false, error: "file_path is required for write-file. Example: 'references/example.md'" }; + } else if (fileContent === undefined) { + result = { success: false, error: 'file_content is required for write-file.' }; + } else { + result = writeSkillFile(name, filePath, fileContent, skillsDir); + } + break; + + case 'remove-file': + if (!filePath) { + result = { success: false, error: 'file_path is required for remove-file.' }; + } else { + result = removeSkillFile(name, filePath, skillsDir); + } + break; + + default: + result = { success: false, error: `Unknown action '${action}'. Use: create, patch, edit, delete, write-file, remove-file.` }; + } + + // Refresh skill discovery after successful changes + if (result.success && ['create', 'edit', 'patch', 'delete', 'write-file', 'remove-file'].includes(action)) { + skillLoader.discover(); + } + + if (result.success) { + return JSON.stringify(result, null, 2); + } else { + return `Error: ${result.error}${result.filePreview ? `\n\nFile preview:\n${result.filePreview}` : ''}`; + } + }, + }); +} \ No newline at end of file diff --git a/src/capabilities/skills/skill-view.ts b/src/capabilities/skills/skill-view.ts new file mode 100644 index 00000000..da0b8f20 --- /dev/null +++ b/src/capabilities/skills/skill-view.ts @@ -0,0 +1,59 @@ +/** + * @fileoverview Skill View Tool - View Skill Details + * + * Allows the agent to view a skill's full content including metadata and instructions. + * Use this before creating a similar skill or when updating an existing one. + */ + +import { tool, zodSchema } from 'ai'; +import { z } from 'zod'; +import type { SkillLoader } from '../../skills/loader.js'; +import { MAX_NAME_LENGTH } from '../../skills/utils.js'; + +/** + * Creates the skill_view tool for the capability registry. + * @param skillLoader - The skill loader instance + * @returns AI tool definition for viewing skills + */ +export function createSkillViewTool(skillLoader: SkillLoader) { + return tool({ + description: `View a skill's full content including metadata and instructions. Use this before creating a similar skill or when updating an existing one. Shows the complete SKILL.md content so you can understand the format and decide if you need to create a new skill or update an existing one.`, + + inputSchema: zodSchema(z.object({ + name: z.string().max(MAX_NAME_LENGTH).describe('Name of the skill to view.'), + })), + + execute: async ({ name }) => { + const skill = skillLoader.load(name); + if (!skill) { + return `Skill "${name}" not found. Use list_skills to see available skills.`; + } + + let result = `## Skill: ${skill.name}\n\n`; + result += `**Description:** ${skill.description}\n`; + + if (skill.version) { + result += `**Version:** ${skill.version}\n`; + } + + if (skill['allowed-tools'] && skill['allowed-tools'].length > 0) { + result += `**Allowed Tools:** ${skill['allowed-tools'].join(', ')}\n`; + } + + if (skill['disable-model-invocation']) { + result += `**Model Invocation:** Disabled\n`; + } + + result += `\n---\n\n${skill.instructions}`; + + // Show available supporting files + if (skill.scriptsDir || skill.referencesDir) { + result += '\n\n---\n\n**Supporting Files:**\n'; + if (skill.scriptsDir) result += '- `scripts/` directory available\n'; + if (skill.referencesDir) result += '- `references/` directory available\n'; + } + + return result; + }, + }); +} \ No newline at end of file diff --git a/src/core/agent.ts b/src/core/agent.ts index fa832829..577c8f28 100644 --- a/src/core/agent.ts +++ b/src/core/agent.ts @@ -9,6 +9,7 @@ import type { MercuryConfig } from '../utils/config.js'; import type { TokenBudget } from '../utils/tokens.js'; import type { CapabilityRegistry } from '../capabilities/registry.js'; import type { ScheduledTaskManifest } from './scheduler.js'; +import { SkillTracker, triggerSkillReview } from '../skills/review.js'; import { DeepSeekProvider } from '../providers/deepseek.js'; import { Lifecycle } from './lifecycle.js'; import { Scheduler } from './scheduler.js'; @@ -249,6 +250,7 @@ export class Agent { private messageQueue: ChannelMessage[] = []; private processing = false; private telegramStreaming: boolean; + private skillTracker: SkillTracker; constructor( private config: MercuryConfig, @@ -267,6 +269,7 @@ export class Agent { this.scheduler = scheduler; this.capabilities = capabilities; this.telegramStreaming = config.channels.telegram.streaming ?? true; + this.skillTracker = new SkillTracker({ nudgeInterval: 10 }); this.scheduler.setOnScheduledTask(async (manifest) => this.handleScheduledTask(manifest)); @@ -559,6 +562,11 @@ export class Agent { if (toolCalls.some((tc: any) => tc.toolName === 'use_skill')) { loopDetector.reset(); } + // Track skill management actions to reset review counter + if (toolCalls.some((tc: any) => tc.toolName === 'skill_manage')) { + this.skillTracker.recordSkillAction(); + loopDetector.reset(); + } const hardLoop = loopDetector.detectIdentical(); if (hardLoop) { logger.warn({ tool: hardLoop.tool, count: hardLoop.count }, 'Hard loop detected ā€” aborting'); @@ -724,6 +732,11 @@ export class Agent { if (toolCalls.some((tc: any) => tc.toolName === 'use_skill')) { loopDetector.reset(); } + // Track skill management actions to reset review counter + if (toolCalls.some((tc: any) => tc.toolName === 'skill_manage')) { + this.skillTracker.recordSkillAction(); + loopDetector.reset(); + } const hardLoop = loopDetector.detectIdentical(); if (hardLoop) { logger.warn({ tool: hardLoop.tool, count: hardLoop.count }, 'Hard loop detected ā€” aborting'); @@ -910,6 +923,24 @@ export class Agent { logger.debug('Internal prompt processed, no channel response needed'); } + // Track iterations for skill review triggers + this.skillTracker.recordIteration(); + + // Trigger skill review after configured interval + if (this.skillTracker.shouldTriggerReview() && msg.channelType !== 'internal') { + const recentMessages = this.shortTerm.getRecent(msg.channelId, 8); + if (recentMessages.length > 2) { + triggerSkillReview(this.providers, this.shortTerm as any, recentMessages) + .then(suggestion => { + if (suggestion && channel) { + channel.send(`\nšŸ’” Skill suggestion: ${suggestion}\n`, msg.channelId).catch(() => {}); + } + }) + .catch(() => {}); + this.skillTracker.reset(); + } + } + this.lifecycle.transition('idle'); } catch (err) { logger.error({ err }, 'Error handling message'); @@ -977,6 +1008,30 @@ Always specify owner and repo parameters on GitHub tools. The user's GitHub user prompt += githubHint; } + + // Add skill management guidance + const skillTools = this.capabilities.getToolNames().filter(t => t.startsWith('skill_')); + if (skillTools.includes('skill_manage')) { + prompt += ` + +## Self-Improving Skills +You can create reusable skills from successful task patterns using skill_manage. Consider creating/updating skills when: +- A complex task succeeded (5+ tool calls) +- Errors were overcome through a specific approach +- A user-corrected approach worked better than your initial plan +- A non-trivial workflow was discovered +- User asks you to remember a procedure + +Best practices: +- Survey existing skills first (list_skills, then skill_view) before creating +- Prefer patching existing skills over creating new ones +- Skills should have clear trigger conditions and specific numbered steps +- Confirm with user before creating/deleting skills +- Good skill names: descriptive, lowercase, hyphenated (e.g., 'docker-debug', 'api-error-handling') + +Use skill_view to see skill format examples before creating new ones.`; + } + return prompt; } diff --git a/src/skills/review.ts b/src/skills/review.ts new file mode 100644 index 00000000..14b51b66 --- /dev/null +++ b/src/skills/review.ts @@ -0,0 +1,269 @@ +/** + * @fileoverview Skill Review System - Self-Improving Skills Implementation + * + * Tracks task complexity and triggers skill review suggestions. + * Based on Hermes-Agent's closed learning loop pattern. + * + * Features: + * - Tracks iterations since last skill action + * - Triggers review after configurable interval + * - Generates skill suggestions based on conversation history + * + * Follows Mercury's principles: + * - Token-conscious: lightweight tracking, async generation + * - Self-documenting: clear function documentation + * - Graceful degradation: handles provider failures gracefully + */ + +import { generateText } from 'ai'; +import type { ProviderRegistry } from '../providers/registry.js'; +import type { SkillLoader } from './loader.js'; +import { logger } from '../utils/logger.js'; + +/** Configuration for skill review triggers */ +export interface SkillReviewConfig { + /** How often (in messages) to trigger skill review. Default: 10 */ + nudgeInterval: number; + /** Minimum tool calls before suggesting a skill. Default: 5 */ + minToolCalls: number; +} + +const DEFAULT_CONFIG: SkillReviewConfig = { + nudgeInterval: 10, + minToolCalls: 5, +}; + +/** + * Prompt used to trigger skill review after complex tasks. + * Follows Mercury's token-conscious principle - concise but comprehensive. + */ +const SKILL_REVIEW_PROMPT = `Review the conversation above and consider whether a skill should be saved or updated. + +Guidelines: +1. SURVEY the existing skill landscape first. Call list_skills to see what you have. If anything looks potentially relevant, use the skill to see its instructions before deciding. +2. ONLY CREATE A NEW SKILL when no existing skill reasonably covers the class. +3. PREFER GENERALIZING AN EXISTING SKILL over creating a new one. If a skill already covers the class ā€” even partially ā€” update it (skill_manage action='patch') instead of creating a duplicate. +4. If you notice two existing skills that overlap, note it in your response for future cleanup. +5. If you used a skill and hit issues not covered by it, patch it immediately to include the pitfalls. + +Consider creating/updating a skill when: +- A complex task succeeded (5+ tool calls) +- Errors were overcome through a specific approach +- A user-corrected approach worked better than your initial plan +- A non-trivial workflow was discovered +- User asked you to remember a procedure + +Skip creating skills for: +- Simple one-off tasks +- Trivial greetings or acknowledgments +- Tasks that can be handled by existing skills + +Good skill names: descriptive, lowercase, hyphenated (e.g., 'docker-debug', 'api-error-handling') +Good skill descriptions: clear about when to use this skill + +Output format: +- If no skill needed: briefly explain why not +- If skill should be created/updated: describe what to do with skill_manage +- Always confirm with user before creating/deleting skills`; + +/** System prompt for skill review generation */ +const SKILL_REVIEW_SYSTEM = `You are a skill review assistant. Your job is to evaluate conversations and determine if any approach should become a reusable skill. + +Be conservative ā€” prefer updating existing skills over creating new ones. Only create when there's genuine value in capturing this workflow. +Consider whether the skill would help in future similar situations, not just this one. +Skills should have clear trigger conditions and specific steps, not generic advice.`; + +/** + * Tracks skill-related activity for triggering reviews. + * Lightweight tracker that counts iterations since last skill action. + */ +export class SkillTracker { + private iterationsSinceSkill = 0; + private config: SkillReviewConfig; + + /** + * Create a new SkillTracker + * @param config - Optional configuration overrides + */ + constructor(config: Partial = {}) { + this.config = { ...DEFAULT_CONFIG, ...config }; + } + + /** + * Called when skill_manage is used (create, edit, patch, delete, etc.) + */ + recordSkillAction(): void { + this.iterationsSinceSkill = 0; + logger.debug('Skill action recorded, counter reset'); + } + + /** + * Called after each message/turn processed. + */ + recordIteration(): void { + this.iterationsSinceSkill++; + } + + /** + * Check if we should trigger a skill review. + * @returns True if review should be triggered + */ + shouldTriggerReview(): boolean { + return this.iterationsSinceSkill >= this.config.nudgeInterval; + } + + /** + * Get current iteration count. + * @returns Number of iterations since last skill action + */ + getIterationCount(): number { + return this.iterationsSinceSkill; + } + + /** + * Reset the counter (after a review or skill action). + */ + reset(): void { + this.iterationsSinceSkill = 0; + } + + /** + * Update configuration. + * @param config - Configuration overrides + */ + updateConfig(config: Partial): void { + this.config = { ...this.config, ...config }; + } +} + +/** + * Build a skill review prompt from conversation history. + * @param recentMessages - Recent conversation messages + * @returns Formatted prompt for skill review + */ +export function buildSkillReviewPrompt( + recentMessages: Array<{ role: string; content: string }> +): string { + const conversationText = recentMessages + .map(m => `${m.role}: ${m.content}`) + .join('\n\n'); + + return `Recent conversation:\n${conversationText}\n\n${SKILL_REVIEW_PROMPT}`; +} + +/** + * Trigger a skill review asynchronously. + * Uses the default provider to generate a review suggestion. + * + * @param providers - Provider registry for LLM access + * @param _skillLoader - Skill loader (for future use) + * @param recentMessages - Recent conversation messages + * @returns Review suggestion or null if no skill suggested + */ +export async function triggerSkillReview( + providers: ProviderRegistry, + _skillLoader: SkillLoader, + recentMessages: Array<{ role: string; content: string }> +): Promise { + try { + const provider = providers.getDefault(); + + const result = await generateText({ + model: provider.getModelInstance(), + system: SKILL_REVIEW_SYSTEM, + messages: [ + ...recentMessages.slice(-10).map(m => ({ + role: m.role as 'user' | 'assistant' | 'system', + content: m.content, + })), + { role: 'user' as const, content: SKILL_REVIEW_PROMPT }, + ], + maxOutputTokens: 800, + }); + + const review = result.text.trim(); + + if (review && !review.toLowerCase().includes('no skill needed') && + !review.toLowerCase().includes('no new skill')) { + logger.info({ review: review.slice(0, 200) }, 'Skill review generated suggestion'); + return review; + } + + logger.debug('Skill review: no new skill suggested'); + return null; + } catch (err) { + logger.warn({ err }, 'Skill review generation failed'); + return null; + } +} + +/** + * Check if a conversation is complex enough to warrant skill review. + * @param messages - Conversation messages + * @param minToolCalls - Minimum tool calls to consider complex + * @returns True if conversation is worth reviewing + */ +export function isConversationWorthReviewing( + messages: Array<{ role: string; content: string }>, + minToolCalls = 5 +): boolean { + // Count tool-related patterns in messages + let toolCallCount = 0; + for (const msg of messages) { + // Look for patterns indicating tool usage + const patterns = [ + /\[Using:\s*\w+/g, // [Using: toolname] + /tool.*call/i, // tool call patterns + /executes?|running|calling/i, // execution language + ]; + + for (const pattern of patterns) { + const matches = msg.content.match(pattern); + if (matches) { + toolCallCount += matches.length; + } + } + } + + return toolCallCount >= minToolCalls; +} + +/** + * Generate a good skill name from task description. + * @param task - Task description + * @returns Suggested skill name + */ +export function suggestSkillName(task: string): string { + // Convert to lowercase, replace spaces with hyphens + let name = task.toLowerCase() + .replace(/[^a-z0-9\s-]/g, '') + .replace(/\s+/g, '-') + .replace(/-+/g, '-') + .slice(0, 64); + + // Remove common words + const stopWords = ['how', 'to', 'the', 'a', 'an', 'for', 'and', 'with']; + name = name + .split('-') + .filter(w => !stopWords.includes(w)) + .join('-'); + + return name || 'unnamed-skill'; +} + +/** + * Generate a skill description from task context. + * @param context - Task context + * @returns Suggested skill description + */ +export function suggestSkillDescription(context: string): string { + // Extract first meaningful sentence or phrase + const sentences = context.split(/[.!?]/); + const firstMeaningful = sentences.find(s => s.trim().length > 20); + + if (firstMeaningful) { + return firstMeaningful.trim().slice(0, 200); + } + + return context.slice(0, 200); +} \ No newline at end of file diff --git a/src/skills/utils.ts b/src/skills/utils.ts new file mode 100644 index 00000000..4078af1a --- /dev/null +++ b/src/skills/utils.ts @@ -0,0 +1,232 @@ +/** + * @fileoverview Skill Utilities - Validation and Helper Functions + * + * Provides validation functions and utilities for skill management. + * Follows Mercury's Agentic Expertise principles: + * - Token-conscious: efficient validation + * - Self-documenting: clear function documentation + */ + +import { existsSync, readFileSync, readdirSync, mkdirSync } from 'node:fs'; +import { join } from 'node:path'; +import type { Skill, SkillMeta } from './types.js'; +import { SkillLoader } from './loader.js'; +import { logger } from '../utils/logger.js'; + +const SKILL_FILE = 'SKILL.md'; + +// Characters allowed in skill names (filesystem-safe, URL-friendly) +const VALID_NAME_RE = /^[a-z0-9][a-z0-9._-]*$/; + +// Subdirectories allowed for write_file/remove_file +const ALLOWED_SUBDIRS = new Set(['references', 'templates', 'scripts', 'assets']); + +/** Maximum skill name length */ +export const MAX_NAME_LENGTH = 64; +/** Maximum description length */ +export const MAX_DESCRIPTION_LENGTH = 1024; +/** Maximum SKILL.md content length (~36k tokens at 2.75 chars/token) */ +export const MAX_SKILL_CONTENT_CHARS = 100_000; +/** Maximum supporting file size (1 MiB) */ +export const MAX_SKILL_FILE_BYTES = 1_048_576; + +/** + * Validate a skill name. + * @param name - Skill name to validate + * @returns Error message or null if valid + */ +export function validateSkillName(name: string): string | null { + if (!name) return 'Skill name is required.'; + if (name.length > MAX_NAME_LENGTH) return `Skill name exceeds ${MAX_NAME_LENGTH} characters.`; + if (!VALID_NAME_RE.test(name)) { + return `Invalid skill name '${name}'. Use lowercase letters, numbers, hyphens, dots, and underscores. Must start with a letter or digit.`; + } + return null; +} + +/** + * Validate an optional category name. + * @param category - Category to validate + * @returns Error message or null if valid + */ +export function validateCategory(category: string | null | undefined): string | null { + if (!category) return null; + if (typeof category !== 'string') return 'Category must be a string.'; + + category = category.trim(); + if (!category) return null; + + if (category.includes('/') || category.includes('\\')) { + return `Invalid category '${category}'. Categories must be a single directory name.`; + } + if (category.length > MAX_NAME_LENGTH) return `Category exceeds ${MAX_NAME_LENGTH} characters.`; + if (!VALID_NAME_RE.test(category)) { + return `Invalid category '${category}'. Use lowercase letters, numbers, hyphens, dots, and underscores.`; + } + return null; +} + +/** + * Validate that SKILL.md content has proper frontmatter with required fields. + * @param content - SKILL.md content to validate + * @returns Error message or null if valid + */ +export function validateSkillContent(content: string): string | null { + if (!content.trim()) return 'Content cannot be empty.'; + + if (!content.startsWith('---')) { + return 'SKILL.md must start with YAML frontmatter (---). See existing skills for format.'; + } + + const endMatch = content.slice(3).match(/\n---\s*\n/); + if (!endMatch) { + return "SKILL.md frontmatter is not closed. Ensure you have a closing '---' line."; + } + + const yamlContent = content.slice(3, endMatch.index! + 3); + const body = content.slice(endMatch.index! + endMatch[0].length + 3).trim(); + + if (!body) { + return 'SKILL.md must have content after the frontmatter (instructions, procedures, etc.).'; + } + + return null; +} + +/** + * Validate that content doesn't exceed the character limit. + * @param content - Content to check + * @param label - Label for error messages + * @returns Error message or null if valid + */ +export function validateContentSize(content: string, label = 'SKILL.md'): string | null { + if (content.length > MAX_SKILL_CONTENT_CHARS) { + return `${label} content is ${content.length.toLocaleString()} characters (limit: ${MAX_SKILL_CONTENT_CHARS.toLocaleString()}). Consider splitting into a smaller SKILL.md with supporting files in references/ or templates/.`; + } + return null; +} + +/** + * Validate a file path for write_file/remove_file. + * @param filePath - File path to validate + * @returns Error message or null if valid + */ +export function validateFilePath(filePath: string): string | null { + if (!filePath) return 'file_path is required.'; + + // Prevent path traversal + if (filePath.includes('..')) { + return "Path traversal ('..') is not allowed."; + } + + const parts = filePath.split('/').filter(Boolean); + if (parts.length === 0 || !ALLOWED_SUBDIRS.has(parts[0])) { + return `File must be under one of: ${[...ALLOWED_SUBDIRS].join(', ')}. Got: '${filePath}'`; + } + + if (parts.length < 2) { + return `Provide a file path, not just a directory. Example: '${parts[0]}/myfile.md'`; + } + + return null; +} + +/** + * Check if a skill path is within the local SKILLS_DIR. + * @param skillPath - Skill directory path + * @param skillsDir - Skills base directory + * @returns True if the skill is local (can be modified) + */ +export function isLocalSkill(skillPath: string, skillsDir: string): boolean { + try { + const skillResolved = skillPath.replace(/\\/g, '/'); + const dirResolved = skillsDir.replace(/\\/g, '/'); + return skillResolved.startsWith(dirResolved + '/') || skillResolved === dirResolved; + } catch { + return false; + } +} + +/** + * Get all skill directories (local + external if configured). + * @param skillsDir - Primary skills directory + * @returns Array of skill directory paths + */ +export function getAllSkillsDirs(skillsDir: string): string[] { + const dirs = [skillsDir]; + // Could add support for external_dirs from config in the future + return dirs.filter(d => existsSync(d)); +} + +/** + * Find a skill by name across all skill directories. + * @param name - Skill name to find + * @param skillsDir - Primary skills directory + * @returns Skill directory path or null if not found + */ +export function findSkill(name: string, skillsDir: string): string | null { + for (const dir of getAllSkillsDirs(skillsDir)) { + if (!existsSync(dir)) continue; + + const entries = readdirSync(dir, { withFileTypes: true }); + for (const entry of entries) { + if (!entry.isDirectory() || entry.name.startsWith('_')) continue; + const skillPath = join(dir, entry.name, SKILL_FILE); + if (!existsSync(skillPath)) continue; + + // Load and check the skill's actual name from frontmatter + try { + const content = readFileSync(skillPath, 'utf-8'); + const meta = extractFrontmatter(content); + if (meta && meta.name === name) { + return join(dir, entry.name); + } + // Also check by directory name for backward compatibility + if (entry.name === name) { + return join(dir, entry.name); + } + } catch { + // Fall back to directory name match + if (entry.name === name) { + return join(dir, entry.name); + } + } + } + } + return null; +} + +/** + * Extract YAML frontmatter from SKILL.md content. + * @param content - SKILL.md content + * @returns Parsed frontmatter or null if invalid + */ +export function extractFrontmatter(content: string): SkillMeta | null { + if (!content.startsWith('---')) return null; + + const endMatch = content.slice(3).match(/\n---\s*\n/); + if (!endMatch) return null; + + const yamlContent = content.slice(3, endMatch.index! + 3); + try { + const { parse } = require('yaml'); + const meta = parse(yamlContent) as SkillMeta; + return meta; + } catch { + return null; + } +} + +/** + * Resolve skill directory path. + * @param name - Skill name + * @param category - Optional category + * @param skillsDir - Skills base directory + * @returns Full path to skill directory + */ +export function resolveSkillDir(name: string, category: string | null, skillsDir: string): string { + if (category) { + return join(skillsDir, category, name); + } + return join(skillsDir, name); +} \ No newline at end of file diff --git a/tests/skill-manage.test.ts b/tests/skill-manage.test.ts new file mode 100644 index 00000000..e5c85897 --- /dev/null +++ b/tests/skill-manage.test.ts @@ -0,0 +1,152 @@ +/** + * @fileoverview Tests for Self-Improving Skill System + * + * Tests skill_manage, skill_view tools and SkillTracker. + */ + +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { existsSync, readFileSync, writeFileSync, mkdirSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; + +// Mock skill loader +const mockSkillLoader = { + discover: vi.fn(), +}; + +describe('Skill Utils', () => { + const testSkillsDir = join(tmpdir(), 'mercury-test-skills'); + + beforeEach(() => { + // Clean up test directory + if (existsSync(testSkillsDir)) { + rmSync(testSkillsDir, { recursive: true, force: true }); + } + mkdirSync(testSkillsDir, { recursive: true }); + }); + + describe('validateSkillName', () => { + it('should accept valid skill names', async () => { + const { validateSkillName } = await import('../src/skills/utils.js'); + + expect(validateSkillName('docker-debug')).toBeNull(); + expect(validateSkillName('api_error_handler')).toBeNull(); + expect(validateSkillName('test123')).toBeNull(); + }); + + it('should reject invalid skill names', async () => { + const { validateSkillName } = await import('../src/skills/utils.js'); + + expect(validateSkillName('')).toBe('Skill name is required.'); + expect(validateSkillName('Test-Skill')).toContain('lowercase'); + expect(validateSkillName('-test')).toContain('start with a letter or digit'); + expect(validateSkillName('a'.repeat(65))).toContain('exceeds'); + }); + }); + + describe('validateSkillContent', () => { + it('should accept valid SKILL.md content', async () => { + const { validateSkillContent } = await import('../src/skills/utils.js'); + + const validContent = `--- +name: test-skill +description: A test skill +--- +# Test Skill +Content here.`; + + expect(validateSkillContent(validContent)).toBeNull(); + }); + + it('should reject invalid SKILL.md content', async () => { + const { validateSkillContent } = await import('../src/skills/utils.js'); + + expect(validateSkillContent('')).toBe('Content cannot be empty.'); + expect(validateSkillContent('No frontmatter')).toContain('YAML frontmatter'); + expect(validateSkillContent('---\nname: test\n---\n')).toContain('content after the frontmatter'); + }); + }); + + describe('findSkill', () => { + it('should find existing skills by directory name', async () => { + const { findSkill, resolveSkillDir } = await import('../src/skills/utils.js'); + + // Create a test skill + const skillDir = resolveSkillDir('test-skill', null, testSkillsDir); + mkdirSync(skillDir, { recursive: true }); + writeFileSync(join(skillDir, 'SKILL.md'), `--- +name: test-skill +description: Test +--- +# Test`); + + const found = findSkill('test-skill', testSkillsDir); + expect(found).not.toBeNull(); + expect(found).toContain('test-skill'); + }); + + it('should return null for non-existent skills', async () => { + const { findSkill } = await import('../src/skills/utils.js'); + + const found = findSkill('non-existent', testSkillsDir); + expect(found).toBeNull(); + }); + }); +}); + +describe('SkillTracker', () => { + it('should track iterations', async () => { + const { SkillTracker } = await import('../src/skills/review.js'); + + const tracker = new SkillTracker({ nudgeInterval: 5 }); + + expect(tracker.shouldTriggerReview()).toBe(false); + + tracker.recordIteration(); + tracker.recordIteration(); + tracker.recordIteration(); + + expect(tracker.shouldTriggerReview()).toBe(false); + + tracker.recordIteration(); + tracker.recordIteration(); + + expect(tracker.shouldTriggerReview()).toBe(true); + }); + + it('should reset on skill action', async () => { + const { SkillTracker } = await import('../src/skills/review.js'); + + const tracker = new SkillTracker({ nudgeInterval: 3 }); + + tracker.recordIteration(); + tracker.recordIteration(); + tracker.recordIteration(); + expect(tracker.shouldTriggerReview()).toBe(true); + + tracker.recordSkillAction(); + expect(tracker.shouldTriggerReview()).toBe(false); + expect(tracker.getIterationCount()).toBe(0); + }); +}); + +describe('Skill Review', () => { + it('should suggest skill names from tasks', async () => { + const { suggestSkillName } = await import('../src/skills/review.js'); + + const name1 = suggestSkillName('How to debug Docker issues'); + expect(name1).toMatch(/docker.*debug.*issues|debug.*docker.*issues/); + expect(name1).toMatch(/^[a-z0-9]+(-[a-z0-9]+)*$/); + + const name2 = suggestSkillName('API Error Handling'); + expect(name2).toBe('api-error-handling'); + }); + + it('should suggest descriptions from context', async () => { + const { suggestSkillDescription } = await import('../src/skills/review.js'); + + const desc = suggestSkillDescription('This workflow helps debug Docker container issues step by step.'); + expect(desc).toContain('debug'); + expect(desc).toContain('Docker'); + }); +});