From d3daab7da3a0f1ec8d7990d29024190bcd7b8e31 Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Sun, 21 Dec 2025 22:06:11 -0500 Subject: [PATCH 1/3] Add skill packs and model routing Phase 1: Remove Legacy Skills System - Removed skills: HashMap from Config - Removed skill: Option from AgentSpec - Added default_target: Option to Config - Updated cli.rs to use current_target instead of current_skill - Simplified target resolution in agent.rs and subagent.rs Phase 2: Skill Packs Implementation - Created src/skillpacks/ module with: - parser.rs - SKILL.md parsing with YAML frontmatter - index.rs - Discovery from .yo/skills/ and ~/.yo/skills/ - activation.rs - Active skill lifecycle management - Created ActivateSkill tool in src/tools/activate_skill.rs - Added /skillpacks and /skillpack REPL commands - Integrated skill prompt injection and allowed-tools filtering in agent.rs - Added transcript events for skill lifecycle Phase 3: Model Routing - Created src/model_routing.rs with: - RouteCategory enum (Planning, Coding, Exploration, Testing, Documentation, Fast, Default) - Category inference from agent name/description - Hardcoded defaults with config override capability - Integrated ModelRouter into subagent.rs for automatic target selection - Added [model_routing.routes] section to example-yo.toml Key Features: - Subagents are automatically routed to appropriate models based on task type - Skill packs provide reusable instructions with tool restrictions - All 32 tests pass, clippy is clean, no warnings --- Cargo.lock | 20 ++++ Cargo.toml | 1 + README.md | 115 +++++++++++++++++-- example-yo.toml | 42 +++++-- src/agent.rs | 142 ++++++++++++++++++++--- src/cli.rs | 183 ++++++++++++++++++++++-------- src/config.rs | 36 +++--- src/main.rs | 47 +++++--- src/model_routing.rs | 213 +++++++++++++++++++++++++++++++++++ src/skillpacks/activation.rs | 157 ++++++++++++++++++++++++++ src/skillpacks/index.rs | 133 ++++++++++++++++++++++ src/skillpacks/mod.rs | 12 ++ src/skillpacks/parser.rs | 170 ++++++++++++++++++++++++++++ src/subagent.rs | 32 ++++-- src/tools/activate_skill.rs | 28 +++++ src/tools/mod.rs | 4 +- src/transcript.rs | 43 +++++++ 17 files changed, 1245 insertions(+), 133 deletions(-) create mode 100644 src/model_routing.rs create mode 100644 src/skillpacks/activation.rs create mode 100644 src/skillpacks/index.rs create mode 100644 src/skillpacks/mod.rs create mode 100644 src/skillpacks/parser.rs create mode 100644 src/tools/activate_skill.rs diff --git a/Cargo.lock b/Cargo.lock index 16a357b..aab7538 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -914,6 +914,19 @@ dependencies = [ "serde", ] +[[package]] +name = "serde_yaml" +version = "0.9.34+deprecated" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6a8b1a1a2ebf674015cc02edccce75287f1a0130d394307b36743c2f5d504b47" +dependencies = [ + "indexmap", + "itoa", + "ryu", + "serde", + "unsafe-libyaml", +] + [[package]] name = "sha2" version = "0.10.9" @@ -1084,6 +1097,12 @@ version = "0.1.14" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7dd6e30e90baa6f72411720665d41d89b9a3d039dc45b8faea1ddd07f617f6af" +[[package]] +name = "unsafe-libyaml" +version = "0.2.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "673aac59facbab8a9007c7f6108d11f63b603f7cabff99fabf650fea5c32b861" + [[package]] name = "untrusted" version = "0.9.0" @@ -1507,6 +1526,7 @@ dependencies = [ "rustyline", "serde", "serde_json", + "serde_yaml", "sha2", "shell-words", "toml", diff --git a/Cargo.toml b/Cargo.toml index 7d931d7..5427537 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,6 +19,7 @@ regex = "1" rustyline = "14" serde = { version = "1", features = ["derive"] } serde_json = "1" +serde_yaml = "0.9" sha2 = "0.10" toml = "0.8" ureq = { version = "2", features = ["json"] } diff --git a/README.md b/README.md index d73c444..4f69e6b 100644 --- a/README.md +++ b/README.md @@ -9,8 +9,9 @@ An open source, local agentic butler for software development. `yo` orchestrates - **Built-in tools** - Read, Write, Edit, Grep, Glob, Bash - **MCP integration** - Connect external tool servers via Model Context Protocol - **Subagents** - Delegate tasks to specialized agents with restricted tools +- **Skill Packs** - Reusable instruction sets with tool restrictions (Claude Code compatible) +- **Model Routing** - Automatic model selection based on task type - **Permission system** - Granular allow/ask/deny rules for tool access -- **Skill routing** - Map named skills to specific model@backend targets - **Session transcripts** - JSONL audit logs of all interactions - **Context management** - Automatic compaction when conversation grows large @@ -52,7 +53,7 @@ yo -p "refactor main.rs" --yes | `--mode` | Permission mode: default, acceptEdits, bypassPermissions | | `--max-turns` | Max agent iterations per turn (default: 12) | | `--trace` | Enable detailed tracing | -| `--list-targets` | Show configured backends and skills | +| `--list-targets` | Show configured backends and default target | ## REPL Commands @@ -64,15 +65,17 @@ yo -p "refactor main.rs" --yes | `/session` | Show session ID and transcript path | | `/context` | Show context usage stats | | `/backends` | List configured backends | -| `/skills` | List available skills | -| `/skill [name]` | Get or set current skill | -| `/target [model@backend]` | Override current target | +| `/target [model@backend]` | Show or set current target | | `/mode [name]` | Get or set permission mode | | `/permissions` | Show permission rules | | `/permissions add [allow\|ask\|deny] "pattern"` | Add rule | | `/trace` | Toggle tracing | | `/agents` | List available subagents | | `/task ` | Run a subagent with the given prompt | +| `/skillpacks` | List available skill packs | +| `/skillpack use ` | Activate a skill pack | +| `/skillpack drop ` | Deactivate a skill pack | +| `/skillpack active` | List active skill packs | | `/mcp list` | List MCP servers | | `/mcp connect ` | Connect to MCP server | | `/mcp disconnect ` | Disconnect MCP server | @@ -94,9 +97,7 @@ Configuration hierarchy (highest to lowest priority): base_url = "https://api.venice.ai/api/v1" api_key_env = "VENICE_API_KEY" -[skills] -default = "qwen3-235b-a22b-instruct-2507@venice" -fast = "gpt-4o-mini@chatgpt" +default_target = "qwen3-235b-a22b-instruct-2507@venice" [permissions] mode = "default" @@ -116,6 +117,11 @@ auto_compact_enabled = true command = "/path/to/mcp-calc" args = [] auto_start = false + +[model_routing.routes] +planning = "qwen3-235b-a22b-instruct-2507@venice" +coding = "claude-3-5-sonnet-latest@claude" +exploration = "gpt-4o-mini@chatgpt" ``` See `example-yo.toml` for complete reference. @@ -162,8 +168,7 @@ You are Scout, a read-only exploration agent. Use Glob to find files, Grep to search, Read to examine. """ -# Optional: override skill or target -# skill = "fast" +# Optional: override target for this agent # target = "gpt-4o-mini@chatgpt" ``` @@ -200,6 +205,87 @@ The main agent can delegate using the `Task` tool: - Tool access is restricted to `allowed_tools` list - Subagent activity is logged to transcripts +## Skill Packs + +Skill packs are reusable instruction sets that guide the agent for specific tasks. They use the Claude Code compatible SKILL.md format with YAML frontmatter. + +### SKILL.md Format + +Skill packs are stored in `.yo/skills//SKILL.md` or `~/.yo/skills//SKILL.md`: + +```markdown +--- +name: safe-file-reader +description: Read files without making changes +allowed-tools: Read, Grep, Glob +--- + +You are in safe-file-reader mode. Only inspect files; do not modify anything. +Use Glob to find files, Grep to search content, Read to examine. +``` + +### Frontmatter Fields + +| Field | Required | Description | +|-------|----------|-------------| +| `name` | Yes | Lowercase letters, numbers, hyphens (max 64 chars) | +| `description` | Yes | Brief description (max 1024 chars) | +| `allowed-tools` | No | Restrict to specific tools (CSV or YAML list) | + +### Using Skill Packs + +**Via REPL:** +``` +/skillpacks # List available skill packs +/skillpack use reader # Activate a skill pack +/skillpack active # Show active skill packs +/skillpack drop reader # Deactivate +``` + +**Via LLM (ActivateSkill tool):** +The agent can activate skills using the `ActivateSkill` tool, or by mentioning `$skill-name` in conversation. + +### Tool Restrictions + +When multiple skills are active, their `allowed-tools` are intersected. Only tools allowed by all active skills can be used. + +## Model Routing + +Model routing automatically selects the best model for each subagent based on task type. Different models excel at different tasks—planning, coding, exploration, etc. + +### Route Categories + +| Category | Keywords | Default Target | +|----------|----------|----------------| +| `planning` | plan, architect, design | qwen3-235b@venice | +| `coding` | patch, edit, code, implement | claude-3-5-sonnet@claude | +| `exploration` | scout, explore, find, search | gpt-4o-mini@chatgpt | +| `testing` | test, verify, check | gpt-4o-mini@chatgpt | +| `documentation` | doc, readme, comment | gpt-4o-mini@chatgpt | +| `fast` | (explicit) | gpt-4o-mini@chatgpt | +| `default` | (fallback) | gpt-4o-mini@chatgpt | + +### How It Works + +1. Subagent name/description is analyzed for keywords +2. Category is inferred from keywords +3. Target is resolved: explicit spec > config route > hardcoded default +4. Subagent runs on the selected model + +### Configuration + +Override defaults in config: + +```toml +[model_routing.routes] +planning = "qwen3-235b-a22b-instruct-2507@venice" +coding = "claude-3-5-sonnet-latest@claude" +exploration = "gpt-4o-mini@chatgpt" +testing = "gpt-4o-mini@chatgpt" +``` + +Explicit `target` in agent specs always takes priority over routing. + ## Architecture ``` @@ -256,7 +342,13 @@ User Input | `tools/glob.rs` | File pattern matching | | `tools/task.rs` | Subagent delegation tool | | `tools/mcp_dispatch.rs` | Route MCP tool calls | +| `tools/activate_skill.rs` | Skill pack activation tool | | `subagent.rs` | Subagent runtime, tool filtering, mode clamping | +| `skillpacks/mod.rs` | Skill pack module exports | +| `skillpacks/parser.rs` | SKILL.md file parser | +| `skillpacks/index.rs` | Skill pack discovery and indexing | +| `skillpacks/activation.rs` | Active skill lifecycle | +| `model_routing.rs` | Task-based model selection | | `mcp/client.rs` | MCP JSON-RPC client | | `mcp/manager.rs` | MCP server lifecycle | | `mcp/transport.rs` | Stdio transport layer | @@ -265,7 +357,7 @@ User Input 1. User input received (REPL or one-shot) 2. Agent adds message to conversation -3. Agent resolves skill → target (model@backend) +3. Agent resolves target (model@backend) 4. Agent collects tool schemas (built-in + MCP) 5. LLM request sent with messages + tools 6. Response parsed for text and tool calls @@ -281,5 +373,6 @@ Sessions logged to `.yo/sessions/.jsonl` with events: - Tool calls and results - Permission decisions - Subagent lifecycle (start, end, tool calls) +- Skill pack lifecycle (index built, activate, deactivate, parse errors) - MCP server lifecycle - Errors and metadata diff --git a/example-yo.toml b/example-yo.toml index ee3f7aa..cf43f40 100644 --- a/example-yo.toml +++ b/example-yo.toml @@ -44,18 +44,12 @@ base_url = "http://localhost:11434/v1" # Ollama typically doesn't require an API key # ============================================================================= -# SKILLS +# DEFAULT TARGET # ============================================================================= -# Skills map task categories to targets. A target is "model@backend". -# -# The "default" skill is used when no specific skill is selected. -# Switch skills in REPL with: /skill +# The default target to use when no --target flag is provided. +# Format: model@backend -[skills] -default = "qwen3-235b-a22b-instruct-2507@venice" -# planning = "gpt-4@chatgpt" -# coding = "claude-3-5-sonnet-latest@claude" -# fast = "llama3.2@ollama" +default_target = "qwen3-235b-a22b-instruct-2507@venice" # ============================================================================= # PERMISSIONS @@ -146,6 +140,33 @@ auto_compact_enabled = true # Number of recent turns to keep after compaction keep_last_turns = 10 +# ============================================================================= +# MODEL ROUTING +# ============================================================================= +# Configure automatic model selection based on task type. +# Subagents are automatically routed to different models based on their +# name/description. Override defaults here. +# +# Route categories: +# planning - Architecture and design tasks (keywords: plan, architect, design) +# coding - Code generation and refactoring (keywords: patch, edit, code, implement) +# exploration - Finding files, understanding codebase (keywords: scout, explore, find) +# testing - Running tests, verification (keywords: test, verify, check) +# documentation - Writing docs, READMEs (keywords: doc, readme, comment) +# fast - Quick operations that need low latency +# default - Fallback for unmatched categories +# +# Format: category = "model@backend" + +# [model_routing.routes] +# planning = "qwen3-235b-a22b-instruct-2507@venice" +# coding = "claude-3-5-sonnet-latest@claude" +# exploration = "gpt-4o-mini@chatgpt" +# testing = "gpt-4o-mini@chatgpt" +# documentation = "gpt-4o-mini@chatgpt" +# fast = "gpt-4o-mini@chatgpt" +# default = "gpt-4o-mini@chatgpt" + # ============================================================================= # MCP (Model Context Protocol) SERVERS # ============================================================================= @@ -209,7 +230,6 @@ keep_last_turns = 10 # permission_mode - "default", "acceptEdits", or "bypassPermissions" # max_turns - Maximum iterations (default: 8) # system_prompt - Custom system prompt -# skill - Optional skill to use for LLM target # target - Optional explicit target (model@backend) # # Use agents in REPL with: diff --git a/src/agent.rs b/src/agent.rs index 653814f..5521dc0 100644 --- a/src/agent.rs +++ b/src/agent.rs @@ -33,17 +33,45 @@ pub fn run_turn(ctx: &Context, user_input: &str, messages: &mut Vec) -> R "content": user_input })); - // Resolve target for current skill - let skill = ctx.current_skill.borrow().clone(); - let config = ctx.config.borrow(); - let target = config - .resolve_skill(&skill) - .or_else(|| config.default_target()) - .ok_or_else(|| anyhow::anyhow!("No target configured for skill: {}", skill))?; - let bash_config = config.bash.clone(); - drop(config); - - trace(ctx, "TARGET", &format!("{} (skill: {})", target, skill)); + // Resolve target: override > config default + let target = { + let current = ctx.current_target.borrow(); + if let Some(t) = current.as_ref() { + t.clone() + } else { + ctx.config + .borrow() + .get_default_target() + .ok_or_else(|| anyhow::anyhow!("No target configured. Use --target or /target"))? + } + }; + let bash_config = ctx.config.borrow().bash.clone(); + + trace(ctx, "TARGET", &target.to_string()); + + // Check for $skill-name mentions and auto-activate + for word in user_input.split_whitespace() { + if word.starts_with('$') && word.len() > 1 { + let skill_name = + &word[1..].trim_end_matches(|c: char| !c.is_alphanumeric() && c != '-'); + let index = ctx.skill_index.borrow(); + if index.get(skill_name).is_some() { + let active = ctx.active_skills.borrow(); + if active.get(skill_name).is_none() { + drop(active); + let mut active = ctx.active_skills.borrow_mut(); + if let Ok(activation) = active.activate(skill_name, &index) { + let _ = ctx.transcript.borrow_mut().skill_activate( + &activation.name, + Some("auto-activated from $mention"), + activation.allowed_tools.as_ref(), + ); + trace(ctx, "SKILL", &format!("Auto-activated: {}", skill_name)); + } + } + } + } + } // Get built-in tool schemas (including Task for main agent) and add MCP tools let mut tool_schemas = tools::schemas_with_task(); @@ -57,6 +85,33 @@ pub fn run_turn(ctx: &Context, user_input: &str, messages: &mut Vec) -> R } } + // Apply allowed-tools restriction from active skills + let active_skills = ctx.active_skills.borrow(); + let effective_allowed = active_skills.effective_allowed_tools(); + drop(active_skills); + + if let Some(allowed) = &effective_allowed { + tool_schemas.retain(|schema| { + if let Some(name) = schema + .get("function") + .and_then(|f| f.get("name")) + .and_then(|n| n.as_str()) + { + // ActivateSkill is always available + if name == "ActivateSkill" { + return true; + } + // Task is always available for subagent delegation + if name == "Task" { + return true; + } + allowed.iter().any(|a| a == name) + } else { + false + } + }); + } + // Use max_turns from CLI if provided, otherwise default let max_iterations = ctx.args.max_turns.unwrap_or(MAX_ITERATIONS); @@ -68,9 +123,29 @@ pub fn run_turn(ctx: &Context, user_input: &str, messages: &mut Vec) -> R let mut backends = ctx.backends.borrow_mut(); let client = backends.get_client(&target.backend)?; + // Build system prompt with skill pack info + let mut system_prompt = SYSTEM_PROMPT.to_string(); + + // Add skill pack index + let skill_index = ctx.skill_index.borrow(); + let skill_prompt = skill_index.format_for_prompt(50); + drop(skill_index); + if !skill_prompt.is_empty() { + system_prompt.push_str("\n\n"); + system_prompt.push_str(&skill_prompt); + } + + // Add active skill instructions + let active_skills = ctx.active_skills.borrow(); + if !active_skills.is_empty() { + system_prompt.push_str("\n\n"); + system_prompt.push_str(&active_skills.format_for_conversation()); + } + drop(active_skills); + let mut req_messages = vec![json!({ "role": "system", - "content": SYSTEM_PROMPT + "content": system_prompt })]; req_messages.extend(messages.clone()); @@ -169,7 +244,48 @@ pub fn run_turn(ctx: &Context, user_input: &str, messages: &mut Vec) -> R ); let result = if allowed { - if name == "Task" { + if name == "ActivateSkill" { + // Execute ActivateSkill tool + let skill_name = args["name"].as_str().unwrap_or(""); + let reason = args["reason"].as_str(); + + if skill_name.is_empty() { + json!({ + "error": { + "code": "missing_name", + "message": "Missing required 'name' parameter" + } + }) + } else { + let skill_index = ctx.skill_index.borrow(); + let mut active_skills = ctx.active_skills.borrow_mut(); + match active_skills.activate(skill_name, &skill_index) { + Ok(activation) => { + let _ = ctx.transcript.borrow_mut().skill_activate( + &activation.name, + reason, + activation.allowed_tools.as_ref(), + ); + json!({ + "ok": true, + "name": activation.name, + "description": activation.description, + "allowed_tools": activation.allowed_tools, + "instructions_loaded": true, + "message": format!("Skill '{}' activated. Instructions loaded.", activation.name) + }) + } + Err(e) => { + json!({ + "error": { + "code": "activation_failed", + "message": e.to_string() + } + }) + } + } + } + } else if name == "Task" { // Execute Task tool (subagent delegation) tools::task::execute(args.clone(), ctx)? } else if name.starts_with("mcp.") { diff --git a/src/cli.rs b/src/cli.rs index 7ee4929..e7d06f6 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,6 +1,15 @@ use crate::{ - agent, backend::BackendRegistry, config::Config, config::PermissionMode, - mcp::manager::McpManager, policy::PolicyEngine, transcript::Transcript, Args, + agent, + backend::BackendRegistry, + config::Config, + config::PermissionMode, + config::Target, + mcp::manager::McpManager, + model_routing::ModelRouter, + policy::PolicyEngine, + skillpacks::{ActiveSkills, SkillIndex}, + transcript::Transcript, + Args, }; use anyhow::Result; use rustyline::error::ReadlineError; @@ -16,9 +25,12 @@ pub struct Context { pub tracing: RefCell, pub config: RefCell, pub backends: RefCell, - pub current_skill: RefCell, + pub current_target: RefCell>, pub policy: RefCell, pub mcp_manager: RefCell, + pub skill_index: RefCell, + pub active_skills: RefCell, + pub model_router: RefCell, } pub fn run_once(ctx: &Context, prompt: &str) -> Result<()> { @@ -76,9 +88,7 @@ fn handle_command(ctx: &Context, cmd: &str, messages: &mut Vec - set current skill"); - println!(" /target - override current target"); + println!(" /target [t] - show/set current target (model@backend)"); println!("Permissions:"); println!(" /mode [name] - get/set permission mode (default|acceptEdits|bypassPermissions)"); println!(" /permissions - show permission rules"); @@ -94,6 +104,12 @@ fn handle_command(ctx: &Context, cmd: &str, messages: &mut Vec - connect to an MCP server"); println!(" /mcp disconnect - disconnect from an MCP server"); println!(" /mcp tools - list tools from an MCP server"); + println!("Skill Packs:"); + println!(" /skillpacks - list all skill packs"); + println!(" /skillpack show - show skill details"); + println!(" /skillpack use - activate skill"); + println!(" /skillpack drop - deactivate skill"); + println!(" /skillpack active - list active skills"); } "/session" => { println!("Session: {}", ctx.session_id); @@ -114,49 +130,13 @@ fn handle_command(ctx: &Context, cmd: &str, messages: &mut Vec { - let current = ctx.current_skill.borrow(); - let config = ctx.config.borrow(); - println!("Skills → Targets:"); - for (skill, target) in &config.skills { - let marker = if skill == &*current { " *" } else { "" }; - println!(" {}: {}{}", skill, target, marker); - } - } - "/skill" => { - if parts.len() > 1 { - let skill = parts[1].trim(); - let config = ctx.config.borrow(); - if config.skills.contains_key(skill) { - drop(config); - *ctx.current_skill.borrow_mut() = skill.to_string(); - if let Some(target) = ctx.config.borrow().resolve_skill(skill) { - println!("Switched to skill: {} ({})", skill, target); - } - } else { - println!( - "Unknown skill: {}. Available: {:?}", - skill, - config.skills.keys().collect::>() - ); - } - } else { - let skill = ctx.current_skill.borrow(); - let config = ctx.config.borrow(); - if let Some(target) = config.resolve_skill(&skill) { - println!("Current skill: {} ({})", skill, target); - } else { - println!("Current skill: {}", skill); - } - } - } "/target" => { if parts.len() > 1 { let target_str = parts[1].trim(); - if let Some(target) = crate::config::Target::parse(target_str) { + if let Some(target) = Target::parse(target_str) { if ctx.backends.borrow().has_backend(&target.backend) { - // Override the default skill's target - println!("Target override: {} (use /skill to switch skills)", target); + *ctx.current_target.borrow_mut() = Some(target.clone()); + println!("Target set: {}", target); } else { println!( "Unknown backend: {}. Use /backends to list.", @@ -167,12 +147,16 @@ fn handle_command(ctx: &Context, cmd: &str, messages: &mut Vec { handle_task_command(ctx, if parts.len() > 1 { parts[1] } else { "" }); } + "/skillpacks" => { + handle_skillpacks_command(ctx); + } + "/skillpack" => { + handle_skillpack_command(ctx, if parts.len() > 1 { parts[1] } else { "" }); + } _ => println!("Unknown command: {}", parts[0]), } false @@ -477,3 +467,96 @@ fn handle_task_command(ctx: &Context, args: &str) { } } } + +fn handle_skillpacks_command(ctx: &Context) { + use crate::skillpacks::index::SkillSource; + + let index = ctx.skill_index.borrow(); + if index.count() == 0 { + println!("No skill packs found."); + println!("Add skills to .yo/skills//SKILL.md"); + } else { + println!("Skill Packs ({}):", index.count()); + for meta in index.all() { + let source = match meta.source { + SkillSource::Project => "[project]", + SkillSource::User => "[user]", + }; + println!(" {} {} - {}", meta.name, source, meta.description); + } + } + + // Show parse errors if any + for (path, error) in index.errors() { + eprintln!(" [error] {}: {}", path.display(), error); + } +} + +fn handle_skillpack_command(ctx: &Context, args: &str) { + let parts: Vec<&str> = args.splitn(2, ' ').collect(); + + if parts.is_empty() || parts[0].is_empty() { + println!("Usage:"); + println!(" /skillpack show - show skill details"); + println!(" /skillpack use - activate skill"); + println!(" /skillpack drop - deactivate skill"); + println!(" /skillpack active - list active skills"); + return; + } + + match parts[0] { + "show" if parts.len() > 1 => { + let name = parts[1].trim(); + let index = ctx.skill_index.borrow(); + if let Some(meta) = index.get(name) { + println!("Skill: {}", meta.name); + println!("Description: {}", meta.description); + if let Some(tools) = &meta.allowed_tools { + println!("Allowed tools: {}", tools.join(", ")); + } + println!("Path: {}", meta.path.display()); + } else { + println!("Skill '{}' not found", name); + } + } + "use" if parts.len() > 1 => { + let name = parts[1].trim(); + let index = ctx.skill_index.borrow(); + let mut active = ctx.active_skills.borrow_mut(); + match active.activate(name, &index) { + Ok(activation) => { + println!("Activated skill: {}", activation.name); + let _ = ctx.transcript.borrow_mut().skill_activate( + &activation.name, + None, + activation.allowed_tools.as_ref(), + ); + } + Err(e) => println!("Error: {}", e), + } + } + "drop" if parts.len() > 1 => { + let name = parts[1].trim(); + let mut active = ctx.active_skills.borrow_mut(); + match active.deactivate(name) { + Ok(()) => { + println!("Deactivated skill: {}", name); + let _ = ctx.transcript.borrow_mut().skill_deactivate(name); + } + Err(e) => println!("Error: {}", e), + } + } + "active" => { + let active = ctx.active_skills.borrow(); + let names = active.list(); + if names.is_empty() { + println!("No active skills"); + } else { + println!("Active skills: {}", names.join(", ")); + } + } + _ => { + println!("Unknown subcommand. Use: show, use, drop, active"); + } + } +} diff --git a/src/config.rs b/src/config.rs index 86974d9..8265ea6 100644 --- a/src/config.rs +++ b/src/config.rs @@ -97,8 +97,6 @@ pub struct AgentSpec { #[serde(default)] pub description: String, #[serde(default)] - pub skill: Option, - #[serde(default)] pub target: Option, #[serde(default = "default_allowed_tools")] pub allowed_tools: Vec, @@ -264,13 +262,15 @@ impl BackendConfig { } } +use crate::model_routing::ModelRoutingConfig; + /// Main configuration structure #[derive(Debug, Clone, Deserialize, Default)] pub struct Config { #[serde(default)] pub backends: HashMap, #[serde(default)] - pub skills: HashMap, + pub default_target: Option, #[serde(default)] pub permissions: PermissionsConfig, #[serde(default)] @@ -279,6 +279,8 @@ pub struct Config { pub context: ContextConfig, #[serde(default)] pub mcp: McpConfig, + #[serde(default)] + pub model_routing: ModelRoutingConfig, #[serde(skip)] pub agents: HashMap, } @@ -327,11 +329,12 @@ impl Config { Config { backends, - skills: HashMap::new(), + default_target: None, permissions: PermissionsConfig::default(), bash: BashConfig::default(), context: ContextConfig::default(), mcp: McpConfig::default(), + model_routing: ModelRoutingConfig::default(), agents: HashMap::new(), } } @@ -392,12 +395,14 @@ impl Config { /// For permissions: arrays are concatenated, mode is overridden if non-default /// For bash/context: scalars are overridden if set pub fn merge(&mut self, other: Config) { - // Merge backends and skills + // Merge backends for (name, backend) in other.backends { self.backends.insert(name, backend); } - for (skill, target) in other.skills { - self.skills.insert(skill, target); + + // Override default_target if set in other + if other.default_target.is_some() { + self.default_target = other.default_target; } // Merge permissions: concatenate arrays, override mode if non-default @@ -427,14 +432,9 @@ impl Config { } } - /// Resolve a skill to its target - pub fn resolve_skill(&self, skill: &str) -> Option { - self.skills.get(skill).and_then(|s| Target::parse(s)) - } - - /// Get the default target (from "default" skill) - pub fn default_target(&self) -> Option { - self.resolve_skill("default") + /// Get the default target + pub fn get_default_target(&self) -> Option { + self.default_target.as_ref().and_then(|s| Target::parse(s)) } /// Create config from CLI arguments, starting with built-in backends @@ -466,10 +466,8 @@ impl Config { }, ); - // Set default skill to use CLI-provided model - config - .skills - .insert("default".to_string(), format!("{}@{}", model, backend_name)); + // Set default target to use CLI-provided model + config.default_target = Some(format!("{}@{}", model, backend_name)); config } diff --git a/src/main.rs b/src/main.rs index f2eb74a..b11dc84 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4,7 +4,9 @@ mod cli; mod config; mod llm; mod mcp; +mod model_routing; mod policy; +mod skillpacks; mod subagent; mod tools; mod transcript; @@ -103,23 +105,18 @@ fn main() -> Result<()> { // Apply --target override if provided if let Some(target_str) = &args.target { - cfg.skills.insert("default".to_string(), target_str.clone()); + cfg.default_target = Some(target_str.clone()); } - // If no default skill is set, try to set one based on available API keys + // If no default target is set, try to set one based on available API keys // Priority: Venice (our default) > ChatGPT > Claude > Ollama - if cfg.resolve_skill("default").is_none() { + if cfg.default_target.is_none() { if std::env::var("VENICE_API_KEY").is_ok() || std::env::var("venice_api_key").is_ok() { - cfg.skills - .insert("default".to_string(), format!("{}@venice", args.model)); + cfg.default_target = Some(format!("{}@venice", args.model)); } else if std::env::var("OPENAI_API_KEY").is_ok() { - cfg.skills - .insert("default".to_string(), "gpt-4o-mini@chatgpt".to_string()); + cfg.default_target = Some("gpt-4o-mini@chatgpt".to_string()); } else if std::env::var("ANTHROPIC_API_KEY").is_ok() { - cfg.skills.insert( - "default".to_string(), - "claude-3-5-sonnet-latest@claude".to_string(), - ); + cfg.default_target = Some("claude-3-5-sonnet-latest@claude".to_string()); } // Ollama doesn't need a key, but user should explicitly set --target } @@ -142,9 +139,10 @@ fn main() -> Result<()> { }) ); } - println!("\nSkills → Targets:"); - for (skill, target) in &cfg.skills { - println!(" {}: {}", skill, target); + if let Some(target) = &cfg.default_target { + println!("\nDefault target: {}", target); + } else { + println!("\nNo default target configured."); } return Ok(()); } @@ -190,7 +188,7 @@ fn main() -> Result<()> { let session_id = uuid::Uuid::new_v4().to_string(); let transcript_path = transcripts_dir.join(format!("{}.jsonl", session_id)); - let transcript = transcript::Transcript::new(&transcript_path, &session_id, &root)?; + let mut transcript = transcript::Transcript::new(&transcript_path, &session_id, &root)?; let trace = args.trace; let backends = backend::BackendRegistry::new(&cfg); @@ -203,6 +201,20 @@ fn main() -> Result<()> { // Create MCP manager from config let mcp_manager = mcp::manager::McpManager::new(cfg.mcp.servers.clone()); + // Build skill pack index + let skill_index = skillpacks::SkillIndex::build(&root); + + // Log skill index built + let _ = transcript.skill_index_built(skill_index.count()); + + // Log parse errors + for (path, error) in skill_index.errors() { + let _ = transcript.skill_parse_error(path, error); + } + + // Create model router + let model_router = model_routing::ModelRouter::new(cfg.model_routing.clone()); + let ctx = cli::Context { args, root, @@ -211,9 +223,12 @@ fn main() -> Result<()> { tracing: RefCell::new(trace), config: RefCell::new(cfg), backends: RefCell::new(backends), - current_skill: RefCell::new("default".to_string()), + current_target: RefCell::new(None), policy: RefCell::new(policy_engine), mcp_manager: RefCell::new(mcp_manager), + skill_index: RefCell::new(skill_index), + active_skills: RefCell::new(skillpacks::ActiveSkills::new()), + model_router: RefCell::new(model_router), }; if let Some(prompt) = &ctx.args.prompt { diff --git a/src/model_routing.rs b/src/model_routing.rs new file mode 100644 index 0000000..55975c2 --- /dev/null +++ b/src/model_routing.rs @@ -0,0 +1,213 @@ +//! Model routing for situational model selection. +//! +//! This module enables automatic model selection based on: +//! - Subagent type (inferred from name/description) +//! - Hardcoded defaults with config overrides + +use crate::config::Target; +use serde::{Deserialize, Serialize}; +use std::collections::HashMap; + +/// Route categories for model selection +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Deserialize, Serialize)] +#[serde(rename_all = "lowercase")] +pub enum RouteCategory { + Planning, + Coding, + Exploration, + Testing, + Documentation, + Fast, + Default, +} + +impl RouteCategory { + /// Infer category from agent name/description + pub fn from_agent_name(name: &str, description: &str) -> Self { + let combined = format!("{} {}", name, description).to_lowercase(); + + if combined.contains("plan") + || combined.contains("architect") + || combined.contains("design") + { + RouteCategory::Planning + } else if combined.contains("patch") + || combined.contains("edit") + || combined.contains("refactor") + || combined.contains("code") + || combined.contains("implement") + { + RouteCategory::Coding + } else if combined.contains("scout") + || combined.contains("explore") + || combined.contains("find") + || combined.contains("search") + { + RouteCategory::Exploration + } else if combined.contains("test") + || combined.contains("verify") + || combined.contains("check") + { + RouteCategory::Testing + } else if combined.contains("doc") + || combined.contains("readme") + || combined.contains("comment") + { + RouteCategory::Documentation + } else { + RouteCategory::Default + } + } +} + +/// Configuration for model routing +#[derive(Debug, Clone, Default, Deserialize, Serialize)] +pub struct ModelRoutingConfig { + #[serde(default)] + pub routes: HashMap, // category -> target string +} + +/// Hardcoded default routes (sensible defaults) +fn default_routes() -> HashMap { + let mut routes = HashMap::new(); + + // These are sensible defaults - users can override in config + // Format: model@backend + + // Planning tasks benefit from strong reasoning + routes.insert( + RouteCategory::Planning, + "qwen3-235b-a22b-instruct-2507@venice".to_string(), + ); + + // Coding needs strong code generation + routes.insert( + RouteCategory::Coding, + "claude-3-5-sonnet-latest@claude".to_string(), + ); + + // Exploration can use faster models + routes.insert( + RouteCategory::Exploration, + "gpt-4o-mini@chatgpt".to_string(), + ); + + // Testing needs reliable execution + routes.insert(RouteCategory::Testing, "gpt-4o-mini@chatgpt".to_string()); + + // Documentation + routes.insert( + RouteCategory::Documentation, + "gpt-4o-mini@chatgpt".to_string(), + ); + + // Fast operations + routes.insert(RouteCategory::Fast, "gpt-4o-mini@chatgpt".to_string()); + + // Default fallback + routes.insert(RouteCategory::Default, "gpt-4o-mini@chatgpt".to_string()); + + routes +} + +/// Model router that resolves targets based on context +pub struct ModelRouter { + config: ModelRoutingConfig, + defaults: HashMap, +} + +impl ModelRouter { + pub fn new(config: ModelRoutingConfig) -> Self { + Self { + config, + defaults: default_routes(), + } + } + + /// Resolve target for a route category + pub fn resolve(&self, category: RouteCategory, fallback: &Target) -> Target { + // Check user config first + if let Some(target_str) = self.config.routes.get(&category) { + if let Some(target) = Target::parse(target_str) { + return target; + } + } + + // Check defaults + if let Some(target_str) = self.defaults.get(&category) { + if let Some(target) = Target::parse(target_str) { + return target; + } + } + + // Fallback to provided default + fallback.clone() + } + + /// Resolve target for an agent spec + pub fn resolve_for_agent( + &self, + agent_name: &str, + agent_description: &str, + explicit_target: Option<&str>, + fallback: &Target, + ) -> Target { + // Explicit target takes priority + if let Some(target_str) = explicit_target { + if let Some(target) = Target::parse(target_str) { + return target; + } + } + + // Infer category and route + let category = RouteCategory::from_agent_name(agent_name, agent_description); + self.resolve(category, fallback) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_category_inference() { + assert_eq!( + RouteCategory::from_agent_name("planner", "Plan the architecture"), + RouteCategory::Planning + ); + assert_eq!( + RouteCategory::from_agent_name("patch", "Apply code edits"), + RouteCategory::Coding + ); + assert_eq!( + RouteCategory::from_agent_name("scout", "Find files"), + RouteCategory::Exploration + ); + assert_eq!( + RouteCategory::from_agent_name("test-runner", "Run tests"), + RouteCategory::Testing + ); + assert_eq!( + RouteCategory::from_agent_name("docs", "Write documentation"), + RouteCategory::Documentation + ); + assert_eq!( + RouteCategory::from_agent_name("unknown", "Some agent"), + RouteCategory::Default + ); + } + + #[test] + fn test_router_explicit_target_priority() { + let router = ModelRouter::new(ModelRoutingConfig::default()); + let fallback = Target { + model: "fallback".to_string(), + backend: "test".to_string(), + }; + + let result = + router.resolve_for_agent("scout", "Find files", Some("explicit@backend"), &fallback); + assert_eq!(result.model, "explicit"); + assert_eq!(result.backend, "backend"); + } +} diff --git a/src/skillpacks/activation.rs b/src/skillpacks/activation.rs new file mode 100644 index 0000000..0272e65 --- /dev/null +++ b/src/skillpacks/activation.rs @@ -0,0 +1,157 @@ +//! Skill activation and lifecycle management. + +use super::{parser::parse_skill_md, SkillIndex, SkillPack}; +use anyhow::{anyhow, Result}; +use std::collections::{HashMap, HashSet}; + +/// Result of skill activation +#[derive(Debug)] +pub struct SkillActivation { + pub name: String, + pub description: String, + pub allowed_tools: Option>, + #[allow(dead_code)] + pub instructions: String, +} + +/// Manages the set of active skills +#[derive(Debug, Default)] +pub struct ActiveSkills { + active: HashMap, +} + +impl ActiveSkills { + pub fn new() -> Self { + Self::default() + } + + /// Activate a skill by name + pub fn activate(&mut self, name: &str, index: &SkillIndex) -> Result { + // Check if already active + if self.active.contains_key(name) { + return Err(anyhow!("Skill '{}' is already active", name)); + } + + // Find in index + let meta = index + .get(name) + .ok_or_else(|| anyhow!("Skill '{}' not found", name))?; + + // Load full SKILL.md + let pack = parse_skill_md(&meta.path)?; + + let activation = SkillActivation { + name: pack.name.clone(), + description: pack.description.clone(), + allowed_tools: pack.allowed_tools.clone(), + instructions: pack.instructions.clone(), + }; + + self.active.insert(pack.name.clone(), pack); + + Ok(activation) + } + + /// Deactivate a skill + pub fn deactivate(&mut self, name: &str) -> Result<()> { + if self.active.remove(name).is_none() { + return Err(anyhow!("Skill '{}' is not active", name)); + } + Ok(()) + } + + /// Get list of active skill names + pub fn list(&self) -> Vec<&str> { + self.active.keys().map(|s| s.as_str()).collect() + } + + /// Check if any skills are active + pub fn is_empty(&self) -> bool { + self.active.is_empty() + } + + /// Get active skill by name + pub fn get(&self, name: &str) -> Option<&SkillPack> { + self.active.get(name) + } + + /// Compute effective allowed tools (intersection of all active skills) + /// Returns None if no restrictions (no active skills specify allowed-tools) + pub fn effective_allowed_tools(&self) -> Option> { + let restrictions: Vec<&Vec> = self + .active + .values() + .filter_map(|p| p.allowed_tools.as_ref()) + .collect(); + + if restrictions.is_empty() { + return None; + } + + // Start with first set, intersect with rest + let mut effective: HashSet<&String> = restrictions[0].iter().collect(); + + for r in &restrictions[1..] { + let other: HashSet<&String> = r.iter().collect(); + effective = effective.intersection(&other).cloned().collect(); + } + + Some(effective.into_iter().cloned().collect()) + } + + /// Format active skills for conversation injection + pub fn format_for_conversation(&self) -> String { + let mut parts = Vec::new(); + + for pack in self.active.values() { + parts.push(format!( + "## Active Skill: {}\n\n{}", + pack.name, pack.instructions + )); + } + + parts.join("\n\n---\n\n") + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_effective_allowed_tools_intersection() { + // Simulate two skill packs with overlapping allowed_tools + let mut active = ActiveSkills::new(); + + // Manually insert packs for testing + active.active.insert( + "skill1".to_string(), + SkillPack { + name: "skill1".to_string(), + description: "Skill 1".to_string(), + allowed_tools: Some(vec![ + "Read".to_string(), + "Grep".to_string(), + "Glob".to_string(), + ]), + instructions: "".to_string(), + root_path: std::path::PathBuf::new(), + }, + ); + + active.active.insert( + "skill2".to_string(), + SkillPack { + name: "skill2".to_string(), + description: "Skill 2".to_string(), + allowed_tools: Some(vec!["Bash".to_string(), "Read".to_string()]), + instructions: "".to_string(), + root_path: std::path::PathBuf::new(), + }, + ); + + let effective = active.effective_allowed_tools().unwrap(); + assert_eq!(effective.len(), 1); + assert!(effective.contains(&"Read".to_string())); + } +} diff --git a/src/skillpacks/index.rs b/src/skillpacks/index.rs new file mode 100644 index 0000000..13a973d --- /dev/null +++ b/src/skillpacks/index.rs @@ -0,0 +1,133 @@ +//! Skill pack discovery and indexing. + +use super::parser::parse_frontmatter; +use anyhow::Result; +use std::collections::HashMap; +use std::path::{Path, PathBuf}; + +/// Source of a skill pack +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SkillSource { + Project, // .yo/skills/ + User, // ~/.yo/skills/ +} + +/// Minimal metadata for a skill (for progressive disclosure) +#[derive(Debug, Clone)] +pub struct SkillMetadata { + pub name: String, + pub description: String, + pub allowed_tools: Option>, + pub path: PathBuf, + pub source: SkillSource, +} + +/// Index of all discovered skills +#[derive(Debug, Default)] +pub struct SkillIndex { + skills: HashMap, + parse_errors: Vec<(PathBuf, String)>, +} + +impl SkillIndex { + /// Build index from search paths + pub fn build(project_root: &Path) -> Self { + let mut index = SkillIndex::default(); + + // User skills (lower priority - loaded first, overwritten by project) + if let Some(home) = dirs::home_dir() { + let user_skills = home.join(".yo").join("skills"); + index.scan_dir(&user_skills, SkillSource::User); + } + + // Project skills (higher priority) + let project_skills = project_root.join(".yo").join("skills"); + index.scan_dir(&project_skills, SkillSource::Project); + + index + } + + fn scan_dir(&mut self, dir: &Path, source: SkillSource) { + if !dir.exists() { + return; + } + + let Ok(entries) = std::fs::read_dir(dir) else { + return; + }; + + for entry in entries.flatten() { + let path = entry.path(); + if !path.is_dir() { + continue; + } + + let skill_md = path.join("SKILL.md"); + if !skill_md.exists() { + continue; + } + + match self.index_skill(&skill_md, source) { + Ok(meta) => { + self.skills.insert(meta.name.clone(), meta); + } + Err(e) => { + self.parse_errors.push((skill_md, e.to_string())); + } + } + } + } + + fn index_skill(&self, path: &Path, source: SkillSource) -> Result { + let content = std::fs::read_to_string(path)?; + let frontmatter = parse_frontmatter(&content)?; + + Ok(SkillMetadata { + name: frontmatter.name.clone(), + description: frontmatter.description, + allowed_tools: frontmatter.allowed_tools.map(|at| at.to_vec()), + path: path.to_path_buf(), + source, + }) + } + + /// Get all skill metadata + pub fn all(&self) -> impl Iterator { + self.skills.values() + } + + /// Get skill by name + pub fn get(&self, name: &str) -> Option<&SkillMetadata> { + self.skills.get(name) + } + + /// Get parse errors + pub fn errors(&self) -> &[(PathBuf, String)] { + &self.parse_errors + } + + /// Count of indexed skills + pub fn count(&self) -> usize { + self.skills.len() + } + + /// Format skill list for system prompt injection + pub fn format_for_prompt(&self, max_entries: usize) -> String { + if self.skills.is_empty() { + return String::new(); + } + + let mut lines = vec!["Available skill packs:".to_string()]; + + for (count, meta) in self.skills.values().enumerate() { + if count >= max_entries { + let remaining = self.skills.len() - max_entries; + lines.push(format!(" (+{} more; use /skillpacks to view)", remaining)); + break; + } + lines.push(format!("- {}: {}", meta.name, meta.description)); + } + + lines.join("\n") + } +} diff --git a/src/skillpacks/mod.rs b/src/skillpacks/mod.rs new file mode 100644 index 0000000..c48e6dc --- /dev/null +++ b/src/skillpacks/mod.rs @@ -0,0 +1,12 @@ +//! Skill Packs: portable, repo-checkable skill system. +//! +//! Skill packs are SKILL.md files with YAML frontmatter that provide +//! reusable instructions to the agent. + +pub mod activation; +pub mod index; +pub mod parser; + +pub use activation::ActiveSkills; +pub use index::SkillIndex; +pub use parser::SkillPack; diff --git a/src/skillpacks/parser.rs b/src/skillpacks/parser.rs new file mode 100644 index 0000000..8e4859d --- /dev/null +++ b/src/skillpacks/parser.rs @@ -0,0 +1,170 @@ +//! SKILL.md file parser. + +use anyhow::{anyhow, Result}; +use serde::Deserialize; +use std::path::Path; + +/// Validation constants +const MAX_NAME_LEN: usize = 64; +const MAX_DESCRIPTION_LEN: usize = 1024; + +/// Parsed SKILL.md frontmatter +#[derive(Debug, Clone, Deserialize)] +pub struct SkillFrontmatter { + pub name: String, + pub description: String, + #[serde(default, rename = "allowed-tools")] + pub allowed_tools: Option, +} + +/// Allowed tools can be CSV string or YAML list +#[derive(Debug, Clone, Deserialize)] +#[serde(untagged)] +pub enum AllowedTools { + Csv(String), + List(Vec), +} + +impl AllowedTools { + /// Convert to a list of tool names + pub fn to_vec(&self) -> Vec { + match self { + AllowedTools::Csv(s) => s.split(',').map(|t| t.trim().to_string()).collect(), + AllowedTools::List(v) => v.clone(), + } + } +} + +/// Complete skill pack with frontmatter and body +#[derive(Debug, Clone)] +pub struct SkillPack { + pub name: String, + pub description: String, + pub allowed_tools: Option>, + pub instructions: String, + #[allow(dead_code)] + pub root_path: std::path::PathBuf, +} + +/// Parse only the frontmatter from a SKILL.md file (for indexing) +pub fn parse_frontmatter(content: &str) -> Result { + // Find YAML frontmatter between --- markers + if !content.starts_with("---") { + return Err(anyhow!("SKILL.md must start with YAML frontmatter (---)")); + } + + let rest = &content[3..]; + let end = rest + .find("\n---") + .ok_or_else(|| anyhow!("Missing closing --- for frontmatter"))?; + + let yaml = &rest[..end]; + let frontmatter: SkillFrontmatter = serde_yaml::from_str(yaml)?; + + // Validate + validate_name(&frontmatter.name)?; + validate_description(&frontmatter.description)?; + + Ok(frontmatter) +} + +/// Parse complete SKILL.md file (for activation) +pub fn parse_skill_md(path: &Path) -> Result { + let content = std::fs::read_to_string(path)?; + let frontmatter = parse_frontmatter(&content)?; + + // Extract body after frontmatter + // Find the second --- and take everything after + let rest = &content[3..]; + let fm_end = rest + .find("\n---") + .ok_or_else(|| anyhow!("Missing closing --- for frontmatter"))?; + let body_start = 3 + fm_end + 4; // skip "---" + "\n---" + let instructions = if body_start < content.len() { + content[body_start..].trim().to_string() + } else { + String::new() + }; + + let root_path = path.parent().unwrap_or(Path::new(".")).to_path_buf(); + + Ok(SkillPack { + name: frontmatter.name, + description: frontmatter.description, + allowed_tools: frontmatter.allowed_tools.map(|at| at.to_vec()), + instructions, + root_path, + }) +} + +fn validate_name(name: &str) -> Result<()> { + if name.len() > MAX_NAME_LEN { + return Err(anyhow!("Skill name exceeds {} chars", MAX_NAME_LEN)); + } + if !name + .chars() + .all(|c| c.is_ascii_lowercase() || c.is_ascii_digit() || c == '-') + { + return Err(anyhow!( + "Skill name must be lowercase letters, numbers, hyphens only" + )); + } + Ok(()) +} + +fn validate_description(desc: &str) -> Result<()> { + if desc.len() > MAX_DESCRIPTION_LEN { + return Err(anyhow!("Description exceeds {} chars", MAX_DESCRIPTION_LEN)); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_parse_frontmatter() { + let content = r#"--- +name: safe-file-reader +description: Read files without making changes +allowed-tools: Read, Grep, Glob +--- + +Only inspect files; do not modify. +"#; + let fm = parse_frontmatter(content).unwrap(); + assert_eq!(fm.name, "safe-file-reader"); + assert_eq!(fm.description, "Read files without making changes"); + let tools = fm.allowed_tools.unwrap().to_vec(); + assert_eq!(tools, vec!["Read", "Grep", "Glob"]); + } + + #[test] + fn test_parse_frontmatter_yaml_list() { + let content = r#"--- +name: test-skill +description: A test skill +allowed-tools: + - Read + - Write +--- + +Instructions here. +"#; + let fm = parse_frontmatter(content).unwrap(); + let tools = fm.allowed_tools.unwrap().to_vec(); + assert_eq!(tools, vec!["Read", "Write"]); + } + + #[test] + fn test_invalid_name() { + let content = r#"--- +name: Invalid_Name +description: Bad name +--- +"#; + let result = parse_frontmatter(content); + assert!(result.is_err()); + } +} diff --git a/src/subagent.rs b/src/subagent.rs index 1749d5e..36f26fe 100644 --- a/src/subagent.rs +++ b/src/subagent.rs @@ -178,20 +178,28 @@ pub fn run_subagent( PolicyEngine::new(subagent_config, true, false) }; - // Resolve target for subagent + // Resolve target using model routing: + // 1. If spec.target is set explicitly, use it + // 2. Otherwise, use ModelRouter to select based on agent name/description + // 3. Fallback to parent's current target or config default let config = ctx.config.borrow(); - let target = if let Some(target_str) = &spec.target { - crate::config::Target::parse(target_str) - } else if let Some(skill) = &spec.skill { - config.resolve_skill(skill) - } else { - // Inherit from current skill - let skill = ctx.current_skill.borrow().clone(); - config - .resolve_skill(&skill) - .or_else(|| config.default_target()) + let fallback = { + let current = ctx.current_target.borrow(); + current + .as_ref() + .cloned() + .or_else(|| config.get_default_target()) + .ok_or_else(|| anyhow::anyhow!("No target configured for subagent"))? + }; + let target = { + let router = ctx.model_router.borrow(); + router.resolve_for_agent( + &spec.name, + &spec.description, + spec.target.as_deref(), + &fallback, + ) }; - let target = target.ok_or_else(|| anyhow::anyhow!("No target configured for subagent"))?; let bash_config = config.bash.clone(); drop(config); diff --git a/src/tools/activate_skill.rs b/src/tools/activate_skill.rs new file mode 100644 index 0000000..eec749f --- /dev/null +++ b/src/tools/activate_skill.rs @@ -0,0 +1,28 @@ +//! ActivateSkill tool for model-invoked skill activation. + +use serde_json::{json, Value}; + +/// Get the ActivateSkill tool schema +pub fn schema() -> Value { + json!({ + "type": "function", + "function": { + "name": "ActivateSkill", + "description": "Activate a skill pack to gain specialized instructions and optionally restrict available tools. Use when the task matches a skill's description. View available skills in the 'Available skill packs' section of the system prompt.", + "parameters": { + "type": "object", + "properties": { + "name": { + "type": "string", + "description": "Name of the skill pack to activate" + }, + "reason": { + "type": "string", + "description": "Brief reason for activating this skill (optional)" + } + }, + "required": ["name"] + } + } + }) +} diff --git a/src/tools/mod.rs b/src/tools/mod.rs index 224b877..637c145 100644 --- a/src/tools/mod.rs +++ b/src/tools/mod.rs @@ -1,3 +1,4 @@ +pub mod activate_skill; pub mod bash; pub mod edit; mod glob; @@ -24,7 +25,7 @@ pub fn schemas() -> Vec { ] } -/// Get all tool schemas including Task (used by main agent) +/// Get all tool schemas including Task and ActivateSkill (used by main agent) pub fn schemas_with_task() -> Vec { vec![ read::schema(), @@ -34,6 +35,7 @@ pub fn schemas_with_task() -> Vec { glob::schema(), bash::schema(), task::schema(), + activate_skill::schema(), ] } diff --git a/src/transcript.rs b/src/transcript.rs index cdfaf6c..1b24b40 100644 --- a/src/transcript.rs +++ b/src/transcript.rs @@ -229,4 +229,47 @@ impl Transcript { }), ) } + + /// Log skill index built + pub fn skill_index_built(&mut self, count: usize) -> Result<()> { + self.log( + "skill_index_built", + serde_json::json!({ + "count": count, + }), + ) + } + + /// Log skill activation + pub fn skill_activate( + &mut self, + name: &str, + reason: Option<&str>, + allowed_tools: Option<&Vec>, + ) -> Result<()> { + self.log( + "skill_activate", + serde_json::json!({ + "name": name, + "reason": reason, + "allowed_tools": allowed_tools, + }), + ) + } + + /// Log skill deactivation + pub fn skill_deactivate(&mut self, name: &str) -> Result<()> { + self.log("skill_deactivate", serde_json::json!({ "name": name })) + } + + /// Log skill parse error + pub fn skill_parse_error(&mut self, path: &std::path::Path, error: &str) -> Result<()> { + self.log( + "skill_parse_error", + serde_json::json!({ + "path": path.display().to_string(), + "error": error, + }), + ) + } } From d4a65f1b0b7a6e4f85f792ef764763e5a02b877b Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Sun, 21 Dec 2025 22:13:03 -0500 Subject: [PATCH 2/3] add GH CI/CD workflow --- .github/workflows/TestingCI.yml | 48 +++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 .github/workflows/TestingCI.yml diff --git a/.github/workflows/TestingCI.yml b/.github/workflows/TestingCI.yml new file mode 100644 index 0000000..e334570 --- /dev/null +++ b/.github/workflows/TestingCI.yml @@ -0,0 +1,48 @@ +name: Rust + +on: + push: + pull_request: + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +env: + CARGO_TERM_COLOR: always + RUST_BACKTRACE: 1 + +jobs: + linux-ubuntu: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Build + run: cargo build --release --verbose + - name: Run tests + run: cargo test --release --verbose + + macos-homebrew: + runs-on: macos-latest + steps: + - uses: actions/checkout@v4 + - uses: sfackler/actions/rustup@master + - run: echo "version=$(rustc --version)" >> $GITHUB_OUTPUT + id: rust-version + - uses: actions/cache@v4 + with: + path: ~/.cargo/registry/index + key: index-${{ runner.os }}-${{ github.run_number }} + restore-keys: | + index-${{ runner.os }}- + - run: cargo generate-lockfile + - uses: actions/cache@v4 + with: + path: ~/.cargo/registry/cache + key: registry-${{ runner.os }}-${{ steps.rust-version.outputs.version }}-${{ hashFiles('Cargo.lock') }} + - name: Fetch + run: cargo fetch + - name: Build + run: cargo build --release --verbose + - name: Run tests + run: cargo test --release --verbose From dffafb3acd7c851c2038a7f1983a7d10387da52f Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Sun, 21 Dec 2025 22:29:02 -0500 Subject: [PATCH 3/3] minor review fixes --- Cargo.lock | 28 +++++++++++++++++----------- Cargo.toml | 2 +- src/skillpacks/activation.rs | 9 +++------ src/skillpacks/index.rs | 6 +++++- src/skillpacks/parser.rs | 6 ++++++ src/tools/activate_skill.rs | 2 +- 6 files changed, 33 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index aab7538..88eda00 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -586,6 +586,16 @@ dependencies = [ "libc", ] +[[package]] +name = "libyml" +version = "0.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3302702afa434ffa30847a83305f0a69d6abd74293b6554c18ec85c7ef30c980" +dependencies = [ + "anyhow", + "version_check", +] + [[package]] name = "linux-raw-sys" version = "0.11.0" @@ -915,16 +925,18 @@ dependencies = [ ] [[package]] -name = "serde_yaml" -version = "0.9.34+deprecated" +name = "serde_yml" +version = "0.0.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6a8b1a1a2ebf674015cc02edccce75287f1a0130d394307b36743c2f5d504b47" +checksum = "59e2dd588bf1597a252c3b920e0143eb99b0f76e4e082f4c92ce34fbc9e71ddd" dependencies = [ "indexmap", "itoa", + "libyml", + "memchr", "ryu", "serde", - "unsafe-libyaml", + "version_check", ] [[package]] @@ -1097,12 +1109,6 @@ version = "0.1.14" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7dd6e30e90baa6f72411720665d41d89b9a3d039dc45b8faea1ddd07f617f6af" -[[package]] -name = "unsafe-libyaml" -version = "0.2.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "673aac59facbab8a9007c7f6108d11f63b603f7cabff99fabf650fea5c32b861" - [[package]] name = "untrusted" version = "0.9.0" @@ -1526,7 +1532,7 @@ dependencies = [ "rustyline", "serde", "serde_json", - "serde_yaml", + "serde_yml", "sha2", "shell-words", "toml", diff --git a/Cargo.toml b/Cargo.toml index 5427537..9357404 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,7 +19,7 @@ regex = "1" rustyline = "14" serde = { version = "1", features = ["derive"] } serde_json = "1" -serde_yaml = "0.9" +serde_yaml = { package = "serde_yml", version = "0.0.12" } sha2 = "0.10" toml = "0.8" ureq = { version = "2", features = ["json"] } diff --git a/src/skillpacks/activation.rs b/src/skillpacks/activation.rs index 0272e65..3759314 100644 --- a/src/skillpacks/activation.rs +++ b/src/skillpacks/activation.rs @@ -84,14 +84,11 @@ impl ActiveSkills { .filter_map(|p| p.allowed_tools.as_ref()) .collect(); - if restrictions.is_empty() { - return None; - } - // Start with first set, intersect with rest - let mut effective: HashSet<&String> = restrictions[0].iter().collect(); + let first = restrictions.first()?; + let mut effective: HashSet<&String> = first.iter().collect(); - for r in &restrictions[1..] { + for r in restrictions.iter().skip(1) { let other: HashSet<&String> = r.iter().collect(); effective = effective.intersection(&other).cloned().collect(); } diff --git a/src/skillpacks/index.rs b/src/skillpacks/index.rs index 13a973d..4e99523 100644 --- a/src/skillpacks/index.rs +++ b/src/skillpacks/index.rs @@ -117,9 +117,13 @@ impl SkillIndex { return String::new(); } + // Sort by name for deterministic output + let mut sorted: Vec<_> = self.skills.values().collect(); + sorted.sort_by_key(|m| &m.name); + let mut lines = vec!["Available skill packs:".to_string()]; - for (count, meta) in self.skills.values().enumerate() { + for (count, meta) in sorted.into_iter().enumerate() { if count >= max_entries { let remaining = self.skills.len() - max_entries; lines.push(format!(" (+{} more; use /skillpacks to view)", remaining)); diff --git a/src/skillpacks/parser.rs b/src/skillpacks/parser.rs index 8e4859d..e639609 100644 --- a/src/skillpacks/parser.rs +++ b/src/skillpacks/parser.rs @@ -98,6 +98,9 @@ pub fn parse_skill_md(path: &Path) -> Result { } fn validate_name(name: &str) -> Result<()> { + if name.is_empty() { + return Err(anyhow!("Skill name cannot be empty")); + } if name.len() > MAX_NAME_LEN { return Err(anyhow!("Skill name exceeds {} chars", MAX_NAME_LEN)); } @@ -113,6 +116,9 @@ fn validate_name(name: &str) -> Result<()> { } fn validate_description(desc: &str) -> Result<()> { + if desc.is_empty() { + return Err(anyhow!("Skill description cannot be empty")); + } if desc.len() > MAX_DESCRIPTION_LEN { return Err(anyhow!("Description exceeds {} chars", MAX_DESCRIPTION_LEN)); } diff --git a/src/tools/activate_skill.rs b/src/tools/activate_skill.rs index eec749f..4a7612a 100644 --- a/src/tools/activate_skill.rs +++ b/src/tools/activate_skill.rs @@ -8,7 +8,7 @@ pub fn schema() -> Value { "type": "function", "function": { "name": "ActivateSkill", - "description": "Activate a skill pack to gain specialized instructions and optionally restrict available tools. Use when the task matches a skill's description. View available skills in the 'Available skill packs' section of the system prompt.", + "description": "Activate a skill pack to gain specialized instructions and optionally restrict available tools. Use when the task matches a skill's description. View available skills in the 'Available skill packs:' section of the system prompt.", "parameters": { "type": "object", "properties": {