diff --git a/CLAUDE.md b/CLAUDE.md index 6fecd74..329cbd6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -178,6 +178,10 @@ tags: [tag1, tag2] # optional: for organization 2. The skill is auto-loaded at startup — no code changes needed 3. Configure the skills directory in `config.toml`: `[skills] directory = "skills"` +Skills can be **instruction skills** (no `model` in frontmatter; full body injected into the system prompt) or **subagent skills** (`model` set; only name + description shown; invoke via `invoke_subagent(skill="name", prompt="...")`). The orchestration skill teaches the agent when to call which subagent and when to override the model (e.g. `model="anthropic/claude-sonnet-4-6"` for thread-writer-hk). + +**Daily News to Threads flow:** The `daily-news-to-threads` orchestration skill (instruction) directs the main agent to: (1) call the `news-fetcher` subagent (default model) to get AI news from Gmail Google 快訊, (2) call the `thread-writer-hk` subagent with model override to write a HK-style Threads thread with verified links, (3) post the thread via Threads MCP and report success. Requires Gmail (google-workspace), fetch, and threads MCP servers in config. + ## Files Not to Commit - `config.toml` - Contains API keys and tokens diff --git a/README.md b/README.md index 0a08311..182141e 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ A self-hosted, agentic Telegram AI assistant written in Rust, powered by OpenRou - **Persistent Memory** — SQLite-backed conversation history and knowledge base - **Vector Embedding Search** — Hybrid vector + FTS5 search using `qwen/qwen3-embedding-8b` - **MCP Integration** — Connect any MCP-compatible server to extend capabilities -- **Bot Skills** — Folder-based natural-language skill instructions auto-loaded at startup +- **Bot Skills** — Folder-based natural-language skill instructions auto-loaded at startup; orchestration and subagent skills (e.g. **daily-news-to-threads**) let the main agent delegate to specialized subagents and override models per task - **Agentic Loop** — Automatic multi-step tool calling until task completion (max iterations configurable, default 25) - **Per-user Conversations** — Independent conversation history per user diff --git a/docs/plans/2026-02-23-subagent-model-selection-design.md b/docs/plans/2026-02-23-subagent-model-selection-design.md new file mode 100644 index 0000000..ef304e1 --- /dev/null +++ b/docs/plans/2026-02-23-subagent-model-selection-design.md @@ -0,0 +1,222 @@ +# Design: Subagent Model Selection — Skill-Scoped Subagents with Model Overrides + +**Date:** 2026-02-23 +**Status:** Approved + +--- + +## Goal + +Enable the RustFox agent to: + +1. **Delegate tasks to a subagent** — a temporary, isolated mini-agentic loop that runs a skill's instructions with its own model, tool whitelist, and iteration budget +2. **Select the model per-skill** — each skill's SKILL.md frontmatter can declare which LLM to use, separate from the main agent's default model +3. **Give subagents only the tools they need** — tool whitelist declared in frontmatter; subagent cannot access memory, scheduling, or invoke further subagents +4. **Allow subagents to read their own skill files** — a new `read_skill_file` tool lets any agent read files from the skills directory without sandbox constraints + +**Primary motivating use case:** A cron-triggered "post daily thread" workflow where the main agent (cheap/fast model) orchestrates — fetching Gmail content, invoking the subagent, posting to Threads — while the subagent (high-quality writing model, e.g. Claude Sonnet) runs the actual post-writing loop following detailed style instructions. + +--- + +## Architecture Overview + +``` +Main Agent (default model from config, e.g. kimi-k2.5) +│ +│ system prompt = base prompt +│ + instruction skills: full body (unchanged) +│ + subagent skills: metadata only + invoke_subagent hint +│ +│ tools = [existing tools] + read_skill_file + invoke_subagent +│ +│ [cron fires] "Write daily thread post" +│ → mcp_gmail_fetch → raw content +│ → invoke_subagent(skill="thread-writer", prompt="") +│ +└──► Subagent (model from skill frontmatter, e.g. claude-sonnet-4-6) + │ + │ system prompt = "You are the thread-writer subagent. + │ Start by calling read_skill_file." + │ tools = [read_skill_file, ...declared in skill frontmatter] + │ isolated message history — no shared conversation or memory + │ + ├── iter 0: read_skill_file("thread-writer", "SKILL.md") → full instructions + ├── iter 1: read_skill_file("thread-writer", "style-guide.md") → style doc + ├── iter 2..N: composes post using instructions + style + └── returns: polished post text + ▲ +Main Agent receives post as tool result + → mcp_threads_post(post) + → "Posted! Here's what was published: ..." +``` + +**Key principles:** +- **Isolated context** — subagent has no access to conversation history, memory, or scheduling +- **Progressive disclosure** — skill body only enters context when the subagent reads it at runtime +- **Composable** — any skill with a `model` field in frontmatter becomes a subagent skill +- **Safe** — subagent tool access is limited to its declared whitelist; no recursive `invoke_subagent` +- **Non-breaking** — existing instruction skills (no `model` field) continue to work exactly as before + +--- + +## SKILL.md Frontmatter Extension + +Three new optional fields: + +```yaml +--- +name: thread-writer +description: Use when writing daily Thread posts from fetched source content. + Invoke via invoke_subagent, not directly. +model: anthropic/claude-sonnet-4-6 # optional — model for this subagent +tools: [read_skill_file, mcp_threads_post] # optional — allowed tool whitelist +max_iterations: 8 # optional — cap for this subagent's loop +--- + +# Thread Writer + +You are a specialized subagent. Your full instructions are in SKILL.md which +you have already read. Write engaging daily Thread posts... +[full instructions — only loaded when subagent reads this at runtime] +``` + +| Field | Type | Default | Notes | +|-------|------|---------|-------| +| `model` | string | config default | If present, skill becomes a subagent skill | +| `tools` | list | `[read_skill_file]` | Tool whitelist for subagent; `read_skill_file` always added | +| `max_iterations` | integer | config `max_iterations` | Capped at global value | + +Caller can override `model` and `tools` at `invoke_subagent` call time (per-invocation override). + +--- + +## Skill Injection Change + +Skills with a `model` field in frontmatter are treated as **subagent skills**. `build_context()` emits them differently: + +**Before (instruction skill — unchanged):** +``` +## Skill: creating-skills +Use when the user asks to create, write, or add a new bot skill... + +[full body injected] +``` + +**After (subagent skill — metadata only):** +``` +## Subagent skill: thread-writer +Use when writing daily Thread posts from fetched source content. +Invoke via: invoke_subagent(skill="thread-writer", prompt="") +``` + +This keeps the main agent's context lean and avoids injecting potentially long style guides into every conversation. + +--- + +## New Tools + +### `read_skill_file` + +Reads a file from the skills directory. Available to both main agent and subagents. + +| Parameter | Type | Required | Description | +|-----------|------|----------|-------------| +| `skill_name` | string | yes | Skill directory name (validated: `^[a-z0-9-]{1,64}$`) | +| `path` | string | yes | Relative path within skill dir, e.g. `SKILL.md`, `style-guide.md` | + +- Resolves to `config.skills.directory / skill_name / path` +- Validates against traversal: no `..` components, not absolute +- Not sandbox-restricted (skills directory is separate from sandbox) + +### `invoke_subagent` + +Boots a subagent mini-loop and returns its final text response. Available to main agent only (not exposed to subagents — prevents infinite nesting). + +| Parameter | Type | Required | Description | +|-----------|------|----------|-------------| +| `skill` | string | yes | Skill name to run as subagent | +| `prompt` | string | yes | Task content to pass to the subagent | +| `model` | string | no | Overrides skill's declared model | +| `tools` | list | no | Overrides skill's declared tool whitelist | + +--- + +## Subagent Mini-Loop (`run_subagent`) + +``` +run_subagent(skill_name, prompt, model_override, tools_override) + 1. Load skill metadata (model, tools, max_iterations) + 2. Resolve final model: model_override → skill.model → config.model + 3. Resolve final tools: tools_override → skill.tools → [read_skill_file] + Always prepend read_skill_file regardless + 4. Build subagent tool definitions (filtered to allowed list only) + 5. Bootstrap messages: + system: "You are the subagent. Start by calling + read_skill_file(skill_name='', path='SKILL.md')." + user: + 6. Mini agentic loop (up to resolved max_iterations): + - call llm.chat_with_model(messages, tools, model) + - if tool_calls: execute via execute_subagent_tool(), append, continue + - else: return content + 7. On max iterations: return error message string +``` + +`execute_subagent_tool` handles only safe, stateless tools: +- `read_skill_file` — always available +- Built-in tools (`read_file`, `write_file`, `run_command`) — if in whitelist +- MCP tools — if in whitelist +- Memory/scheduling tools — **never** available to subagents + +--- + +## LLM Client Change + +Add model-override support to `LlmClient`: + +```rust +// Existing (unchanged) +pub async fn chat(&self, messages: &[ChatMessage], tools: &[ToolDefinition]) -> Result + +// New +pub async fn chat_with_model( + &self, + messages: &[ChatMessage], + tools: &[ToolDefinition], + model: &str, +) -> Result +``` + +`chat()` becomes a thin wrapper calling `chat_with_model(&self.config.model, ...)`. + +--- + +## Files Touched + +| File | Change | +|------|--------| +| `src/llm.rs` | Add `chat_with_model(messages, tools, model: &str)` | +| `src/skills/mod.rs` | Add `model`, `tools`, `max_iterations` to `Skill` struct; update `build_context()` to separate instruction vs subagent skills | +| `src/skills/loader.rs` | Parse `model`, `tools`, `max_iterations` from frontmatter | +| `src/agent.rs` | Add `read_skill_file` + `invoke_subagent` tool definitions; implement `run_subagent()`; handle both in `execute_tool()` | +| `skills/thread-writer/SKILL.md` | New example subagent skill (demonstrates the feature) | + +No new Cargo dependencies. No `config.toml` schema changes. + +--- + +## Security Notes + +- `read_skill_file` validates `skill_name` with strict regex and checks `path` for `..` traversal +- Subagents cannot call `invoke_subagent` — no recursive subagent nesting +- Subagents cannot call memory or scheduling tools — isolated and stateless +- The tool whitelist is enforced at `execute_subagent_tool` dispatch — unknown tools return an error string (not a crash) +- Subagent has no access to conversation history or persistent state + +--- + +## Open Questions (resolved) + +- **Model fallback**: subagent without `model` frontmatter gets `config.openrouter.model` — uses main model, but still isolated context +- **Subagent accessing sandbox**: allowed if `read_file`/`write_file`/`run_command` declared in `tools` whitelist — same sandbox as main agent +- **Max iterations for subagent**: resolved as `min(skill.max_iterations, config.max_iterations)` — cannot exceed global cap +- **Skill injection for subagent skills**: metadata-only in main system prompt; full body loaded lazily by subagent at runtime diff --git a/docs/plans/2026-02-23-subagent-model-selection.md b/docs/plans/2026-02-23-subagent-model-selection.md new file mode 100644 index 0000000..c989431 --- /dev/null +++ b/docs/plans/2026-02-23-subagent-model-selection.md @@ -0,0 +1,1051 @@ +# Subagent Model Selection Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. + +**Goal:** Add an `invoke_subagent` tool that lets the main agent delegate tasks to isolated mini-agents that run a named skill using a skill-specific model and tool whitelist. + +**Architecture:** Each skill's SKILL.md can declare `model`, `tools`, and `max_iterations` in its YAML frontmatter. When the main agent calls `invoke_subagent(skill, prompt)`, a fresh agentic loop boots with that skill's model (via a new `chat_with_model` LLM method), a restricted tool list, and an isolated message history. Subagents read their own instructions at runtime using a new `read_skill_file` tool. Subagent skills appear in the main agent's system prompt as metadata-only (name + description) rather than with full bodies. + +**Tech Stack:** Rust 2021, Tokio, `serde_json`, `anyhow`. No new dependencies. + +--- + +## Reference files + +Read these before starting. They contain all the patterns you'll be repeating: + +- `src/llm.rs` — `LlmClient::chat()` you will refactor (68 lines total) +- `src/skills/mod.rs` — `Skill` struct and `build_context()` you will extend (75 lines) +- `src/skills/loader.rs` — `extract_field` / `extract_list_field` helpers you will follow (164 lines) +- `src/agent.rs` — `execute_tool()`, `skill_tool_definitions()`, `validate_skill_name()`, `validate_skill_path()` you will extend (~1000 lines — read carefully before Task 4) +- `docs/plans/2026-02-23-subagent-model-selection-design.md` — approved design doc + +--- + +## Task 1: Add `chat_with_model` to `LlmClient` + +**Files:** +- Modify: `src/llm.rs` + +The `ChatRequest` struct (line 46) and `LlmClient::chat()` (line 80) are the only things to touch. The goal is to make the model a runtime parameter so the subagent loop can call any model without creating a new `LlmClient`. + +**Step 1: Write the failing test** + +Add this to the bottom of `src/llm.rs` (after the `impl LlmClient` block): + +```rust +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_chat_request_serializes_model_field() { + // Verifies the model string will appear in the JSON POST body + let req = ChatRequest { + model: "anthropic/claude-sonnet-4-6".to_string(), + messages: vec![], + tools: None, + tool_choice: None, + max_tokens: 100, + }; + let json = serde_json::to_value(&req).unwrap(); + assert_eq!(json["model"], "anthropic/claude-sonnet-4-6"); + } + + #[test] + fn test_chat_request_default_model_is_different_from_override() { + // Ensures chat_with_model can use a different model than the config default + let default_req = ChatRequest { + model: "moonshotai/kimi-k2.5".to_string(), + messages: vec![], + tools: None, + tool_choice: None, + max_tokens: 100, + }; + let override_req = ChatRequest { + model: "anthropic/claude-sonnet-4-6".to_string(), + messages: vec![], + tools: None, + tool_choice: None, + max_tokens: 100, + }; + let json_default = serde_json::to_value(&default_req).unwrap(); + let json_override = serde_json::to_value(&override_req).unwrap(); + assert_ne!(json_default["model"], json_override["model"]); + } +} +``` + +**Step 2: Run to verify tests pass (they test existing serialization)** + +```bash +cargo test --lib llm 2>&1 | tail -20 +``` + +Expected: PASS (these only test `ChatRequest` serialization which already works) + +**Step 3: Refactor `chat()` into `chat_with_model()`** + +Replace the existing `chat()` method body. The full `impl LlmClient` block becomes: + +```rust +impl LlmClient { + pub fn new(config: OpenRouterConfig) -> Self { + Self { + client: reqwest::Client::new(), + config, + } + } + + /// Chat with an explicit model string (used by subagents to override the default). + pub async fn chat_with_model( + &self, + messages: &[ChatMessage], + tools: &[ToolDefinition], + model: &str, + ) -> Result { + let tools_param = if tools.is_empty() { + None + } else { + Some(tools.to_vec()) + }; + + let tool_choice = if tools_param.is_some() { + Some("auto".to_string()) + } else { + None + }; + + let request = ChatRequest { + model: model.to_string(), + messages: messages.to_vec(), + tools: tools_param, + tool_choice, + max_tokens: self.config.max_tokens, + }; + + let url = format!("{}/chat/completions", self.config.base_url); + + debug!("Sending request to OpenRouter: {}", url); + + let response = self + .client + .post(&url) + .header("Authorization", format!("Bearer {}", self.config.api_key)) + .header("Content-Type", "application/json") + .json(&request) + .send() + .await + .context("Failed to send request to OpenRouter")?; + + let status = response.status(); + if !status.is_success() { + let error_body = response.text().await.unwrap_or_default(); + anyhow::bail!("OpenRouter API error ({}): {}", status, error_body); + } + + let chat_response: ChatResponse = response + .json() + .await + .context("Failed to parse OpenRouter response")?; + + chat_response + .choices + .into_iter() + .next() + .map(|c| c.message) + .context("No response from OpenRouter") + } + + /// Chat using the model configured in config.toml (delegates to chat_with_model). + pub async fn chat( + &self, + messages: &[ChatMessage], + tools: &[ToolDefinition], + ) -> Result { + self.chat_with_model(messages, tools, &self.config.model).await + } +} +``` + +**Step 4: Verify compilation and tests** + +```bash +cargo check 2>&1 | grep -E "^error" | head -20 +cargo test --lib llm 2>&1 | tail -20 +``` + +Expected: 0 errors, 2 tests pass. + +**Step 5: Commit** + +```bash +git add src/llm.rs +git commit -m "feat(llm): add chat_with_model for per-call model override" +``` + +--- + +## Task 2: Extend `Skill` struct with subagent fields + +**Files:** +- Modify: `src/skills/mod.rs` + +The `Skill` struct (line 9) and `build_context()` (line 52) both need changing. Skills with a `model` field are treated as subagent skills — their full body is NOT injected into the main agent's system prompt, only metadata. + +**Step 1: Write the failing tests** + +Add a `#[cfg(test)] mod tests` block at the bottom of `src/skills/mod.rs`: + +```rust +#[cfg(test)] +mod tests { + use super::*; + + fn make_skill(name: &str, description: &str, content: &str, model: Option<&str>) -> Skill { + Skill { + name: name.to_string(), + description: description.to_string(), + content: content.to_string(), + tags: vec![], + model: model.map(str::to_string), + tools: vec![], + max_iterations: None, + } + } + + #[test] + fn test_build_context_instruction_skill_injects_full_body() { + // Skills without a model field get their full content injected + let mut registry = SkillRegistry::new(); + registry.register(make_skill( + "my-skill", + "Does things", + "# Instructions\nDo this and that.", + None, // no model = instruction skill + )); + let ctx = registry.build_context(); + assert!(ctx.contains("# Instructions")); + assert!(ctx.contains("Do this and that.")); + } + + #[test] + fn test_build_context_subagent_skill_injects_metadata_only() { + // Skills with a model field get only name + description + invoke hint + let mut registry = SkillRegistry::new(); + registry.register(make_skill( + "thread-writer", + "Use when writing Thread posts.", + "# Super Secret Instructions\nLong style guide...", + Some("anthropic/claude-sonnet-4-6"), + )); + let ctx = registry.build_context(); + // Metadata present + assert!(ctx.contains("thread-writer")); + assert!(ctx.contains("Use when writing Thread posts.")); + assert!(ctx.contains("invoke_subagent")); + // Body NOT present + assert!(!ctx.contains("Super Secret Instructions")); + assert!(!ctx.contains("Long style guide")); + } + + #[test] + fn test_build_context_empty_registry() { + let registry = SkillRegistry::new(); + assert_eq!(registry.build_context(), String::new()); + } + + #[test] + fn test_build_context_mixed_skills() { + // Both skill types can coexist + let mut registry = SkillRegistry::new(); + registry.register(make_skill( + "instruction-skill", + "An instruction skill", + "Follow these instructions.", + None, + )); + registry.register(make_skill( + "subagent-skill", + "A subagent skill", + "Secret subagent body.", + Some("some/model"), + )); + let ctx = registry.build_context(); + assert!(ctx.contains("Follow these instructions.")); + assert!(!ctx.contains("Secret subagent body.")); + assert!(ctx.contains("invoke_subagent")); + } +} +``` + +**Step 2: Run to verify tests fail (fields don't exist yet)** + +```bash +cargo test --lib skills::mod 2>&1 | grep -E "^error" | head -10 +``` + +Expected: compile errors about missing fields `model`, `tools`, `max_iterations`. + +**Step 3: Add fields to `Skill` struct** + +Replace the `Skill` struct (lines 7-18) with: + +```rust +/// A loaded skill from a markdown file +#[derive(Debug, Clone)] +#[allow(dead_code)] +pub struct Skill { + /// Skill name (derived from filename or frontmatter) + pub name: String, + /// Short description + pub description: String, + /// Full markdown content (the instructions) + pub content: String, + /// Category/tags for organization + pub tags: Vec, + /// If set, this skill runs as a subagent using this model + pub model: Option, + /// Tool whitelist for the subagent (empty = read_skill_file only) + pub tools: Vec, + /// Max loop iterations for the subagent (None = use global config default) + pub max_iterations: Option, +} +``` + +**Step 4: Update `build_context()` to split instruction vs subagent skills** + +Replace `build_context()` (lines 52-65) with: + +```rust +/// Build context string for the system prompt. +/// Instruction skills (no model field): full body injected. +/// Subagent skills (have model field): metadata only + invoke_subagent hint. +pub fn build_context(&self) -> String { + if self.skills.is_empty() { + return String::new(); + } + + let mut instruction_section = String::new(); + let mut subagent_section = String::new(); + + for skill in self.skills.values() { + if skill.model.is_some() { + // Subagent skill — metadata only + subagent_section.push_str(&format!( + "- **{}**: {}\n Invoke via: `invoke_subagent(skill=\"{}\", prompt=\"\")`\n", + skill.name, skill.description, skill.name + )); + } else { + // Instruction skill — full body + instruction_section.push_str(&format!("## Skill: {}\n", skill.name)); + instruction_section.push_str(&format!("{}\n\n", skill.content)); + } + } + + let mut context = String::new(); + + if !instruction_section.is_empty() { + context.push_str("You have the following skills available. When relevant, follow these instructions:\n\n"); + context.push_str(&instruction_section); + } + + if !subagent_section.is_empty() { + context.push_str("\n## Available Subagent Skills\n\n"); + context.push_str("Delegate these tasks using `invoke_subagent`:\n\n"); + context.push_str(&subagent_section); + } + + context +} +``` + +**Step 5: Fix loader.rs — `Skill::new` calls need the new fields** + +After this step, `loader.rs` will fail to compile because it constructs `Skill { ... }` without the new fields. Fix each `Ok(Skill { ... })` in `src/skills/loader.rs` by adding: + +```rust +model: None, +tools: vec![], +max_iterations: None, +``` + +to both construction sites (the frontmatter path at line ~78 and the no-frontmatter path at line ~91). This is a temporary stub — Task 3 replaces it with real parsing. + +**Step 6: Run tests** + +```bash +cargo test --lib 2>&1 | tail -30 +``` + +Expected: all tests pass including the 4 new ones. + +**Step 7: Commit** + +```bash +git add src/skills/mod.rs src/skills/loader.rs +git commit -m "feat(skills): add model/tools/max_iterations fields; subagent skills show metadata-only in system prompt" +``` + +--- + +## Task 3: Parse new frontmatter fields in skill loader + +**Files:** +- Modify: `src/skills/loader.rs` + +`extract_field` and `extract_list_field` already exist. You need to add `extract_u32_field` and wire up the three new fields. + +**Step 1: Write the failing tests** + +Add a `#[cfg(test)] mod tests` block at the bottom of `src/skills/loader.rs`: + +```rust +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_extract_u32_field_present() { + let fm = "name: my-skill\nmax_iterations: 8\n"; + assert_eq!(extract_u32_field(fm, "max_iterations"), Some(8)); + } + + #[test] + fn test_extract_u32_field_absent() { + let fm = "name: my-skill\n"; + assert_eq!(extract_u32_field(fm, "max_iterations"), None); + } + + #[test] + fn test_extract_u32_field_invalid_value() { + let fm = "max_iterations: not-a-number\n"; + assert_eq!(extract_u32_field(fm, "max_iterations"), None); + } + + #[test] + fn test_load_skill_parses_model_field() { + let frontmatter = "name: thread-writer\ndescription: Write posts\nmodel: anthropic/claude-sonnet-4-6\n"; + let model = extract_field(frontmatter, "model"); + assert_eq!(model.as_deref(), Some("anthropic/claude-sonnet-4-6")); + } + + #[test] + fn test_load_skill_parses_tools_field() { + let frontmatter = "tools: [read_skill_file, mcp_threads_post]\n"; + let tools = extract_list_field(frontmatter, "tools"); + assert_eq!(tools, vec!["read_skill_file", "mcp_threads_post"]); + } + + #[test] + fn test_load_skill_defaults_when_fields_absent() { + let frontmatter = "name: plain-skill\ndescription: Simple skill\n"; + assert_eq!(extract_field(frontmatter, "model"), None); + assert!(extract_list_field(frontmatter, "tools").is_empty()); + assert_eq!(extract_u32_field(frontmatter, "max_iterations"), None); + } +} +``` + +**Step 2: Run to verify `test_extract_u32_field_*` fail (function doesn't exist)** + +```bash +cargo test --lib skills::loader 2>&1 | grep -E "^error" | head -5 +``` + +Expected: compile error — `extract_u32_field` not found. + +**Step 3: Add `extract_u32_field`** + +Add this function after `extract_list_field` in `src/skills/loader.rs`: + +```rust +/// Extract a `key: N` unsigned integer field from YAML-like frontmatter +fn extract_u32_field(frontmatter: &str, key: &str) -> Option { + extract_field(frontmatter, key)?.parse().ok() +} +``` + +**Step 4: Wire up new fields in `load_skill_file`** + +In `load_skill_file`, replace the frontmatter branch return (currently around line 78) with: + +```rust +return Ok(Skill { + name: skill_name, + description: description.unwrap_or_else(|| first_line_or_heading(&body)), + content: body, + tags, + model: extract_field(frontmatter, "model"), + tools: extract_list_field(frontmatter, "tools"), + max_iterations: extract_u32_field(frontmatter, "max_iterations"), +}); +``` + +The no-frontmatter path (line ~91) keeps `model: None, tools: vec![], max_iterations: None` — no skills without frontmatter can be subagents. + +**Step 5: Run tests** + +```bash +cargo test --lib 2>&1 | tail -20 +``` + +Expected: all tests pass including the 6 new loader tests. + +**Step 6: Commit** + +```bash +git add src/skills/loader.rs +git commit -m "feat(skills/loader): parse model, tools, max_iterations from SKILL.md frontmatter" +``` + +--- + +## Task 4: Add `read_skill_file` tool + +**Files:** +- Modify: `src/agent.rs` + +This tool reads a file from the skills directory (not sandbox-restricted). It reuses the existing `validate_skill_name` and `validate_skill_path` helpers already in `agent.rs` (lines 886-919). + +**Step 1: Write the failing tests** + +In `src/agent.rs`, find the existing `#[cfg(test)] mod tests` block (line 922). Add these tests to it: + +```rust +#[test] +fn test_read_skill_file_validates_skill_name() { + // validate_skill_name is reused — just verify the boundary + assert!(validate_skill_name("valid-skill").is_ok()); + assert!(validate_skill_name("../evil").is_err()); + assert!(validate_skill_name("").is_err()); +} + +#[test] +fn test_read_skill_file_validates_relative_path() { + assert!(validate_skill_path("SKILL.md").is_ok()); + assert!(validate_skill_path("style-guide.md").is_ok()); + assert!(validate_skill_path("../other-skill/SKILL.md").is_err()); + assert!(validate_skill_path("/etc/passwd").is_err()); + assert!(validate_skill_path("").is_err()); +} +``` + +**Step 2: Run to verify they pass (reuses existing validated functions)** + +```bash +cargo test --lib agent::tests 2>&1 | tail -20 +``` + +Expected: all pass (these functions already exist and are already tested; we're just adding clarity tests). + +**Step 3: Add `read_skill_file` tool definition** + +In `skill_tool_definitions()` (around line 465), add a third `ToolDefinition` entry after `reload_skills`: + +```rust +ToolDefinition { + tool_type: "function".to_string(), + function: FunctionDefinition { + name: "read_skill_file".to_string(), + description: concat!( + "Read a file from a skill directory. Use this to load a skill's full instructions ", + "or supporting files (style guides, templates, reference docs). ", + "Available to both the main agent and subagents." + ).to_string(), + parameters: json!({ + "type": "object", + "properties": { + "skill_name": { + "type": "string", + "description": "Skill directory name (e.g. 'thread-writer')" + }, + "path": { + "type": "string", + "description": "Relative path within the skill directory (e.g. 'SKILL.md', 'style-guide.md')" + } + }, + "required": ["skill_name", "path"] + }), + }, +}, +``` + +**Step 4: Add `read_skill_file` handler in `execute_tool()`** + +In `execute_tool()`, add a match arm **before** the `"write_skill_file"` arm (around line 748): + +```rust +"read_skill_file" => { + let skill_name = match arguments["skill_name"].as_str() { + Some(n) => n.to_string(), + None => return "Missing skill_name".to_string(), + }; + let relative_path = match arguments["path"].as_str() { + Some(p) => p.to_string(), + None => return "Missing path".to_string(), + }; + + if let Err(e) = validate_skill_name(&skill_name) { + return format!("Invalid skill_name: {}", e); + } + if let Err(e) = validate_skill_path(&relative_path) { + return format!("Invalid path: {}", e); + } + + let target = self + .config + .skills + .directory + .join(&skill_name) + .join(&relative_path); + + match tokio::fs::read_to_string(&target).await { + Ok(content) => content, + Err(e) => format!( + "Failed to read skill file '{}/{}': {}", + skill_name, relative_path, e + ), + } +} +``` + +**Step 5: Verify compilation** + +```bash +cargo check 2>&1 | grep -E "^error" | head -10 +``` + +Expected: 0 errors. + +**Step 6: Run all tests** + +```bash +cargo test --lib 2>&1 | tail -20 +``` + +Expected: all pass. + +**Step 7: Commit** + +```bash +git add src/agent.rs +git commit -m "feat(agent): add read_skill_file tool for reading skill files from skills directory" +``` + +--- + +## Task 5: Add `invoke_subagent` tool and `run_subagent` mini-loop + +**Files:** +- Modify: `src/agent.rs` + +This is the main feature. `run_subagent` is a new `async fn` on `Agent` that boots an isolated agentic loop with the skill's model and tool whitelist. + +**Step 1: Write the failing tests** + +Add to the `#[cfg(test)] mod tests` block in `src/agent.rs`: + +```rust +#[test] +fn test_subagent_tool_whitelist_always_includes_read_skill_file() { + // read_skill_file is always available to subagents regardless of whitelist + let declared: Vec = vec!["mcp_threads_post".to_string()]; + let effective = effective_subagent_tools(&declared); + assert!(effective.contains(&"read_skill_file".to_string())); + assert!(effective.contains(&"mcp_threads_post".to_string())); +} + +#[test] +fn test_subagent_tool_whitelist_empty_gets_read_skill_file() { + let declared: Vec = vec![]; + let effective = effective_subagent_tools(&declared); + assert_eq!(effective, vec!["read_skill_file".to_string()]); +} + +#[test] +fn test_subagent_tool_whitelist_deduplicates_read_skill_file() { + // If the skill already lists read_skill_file, it shouldn't appear twice + let declared = vec!["read_skill_file".to_string(), "mcp_something".to_string()]; + let effective = effective_subagent_tools(&declared); + let count = effective.iter().filter(|t| *t == "read_skill_file").count(); + assert_eq!(count, 1); +} +``` + +**Step 2: Run to verify tests fail (function doesn't exist)** + +```bash +cargo test --lib agent::tests 2>&1 | grep -E "^error" | head -5 +``` + +Expected: compile error — `effective_subagent_tools` not found. + +**Step 3: Add `effective_subagent_tools` helper (free function)** + +Add this **after** the `validate_skill_path` function (around line 919), before the `#[cfg(test)]` block: + +```rust +/// Build the effective tool whitelist for a subagent. +/// Always includes `read_skill_file`; deduplicates. +fn effective_subagent_tools(declared: &[String]) -> Vec { + let mut tools = vec!["read_skill_file".to_string()]; + for t in declared { + if t != "read_skill_file" { + tools.push(t.clone()); + } + } + tools +} +``` + +**Step 4: Run new tests to verify they pass** + +```bash +cargo test --lib agent::tests 2>&1 | tail -20 +``` + +Expected: all pass. + +**Step 5: Implement `run_subagent` method on `Agent`** + +Add this method to `impl Agent` in `src/agent.rs`, just before `execute_tool`: + +```rust +/// Run a named skill as an isolated subagent mini-loop. +/// Returns the subagent's final text response (or an error string). +async fn run_subagent( + &self, + skill_name: &str, + prompt: &str, + model_override: Option<&str>, + tools_override: Option>, +) -> String { + // Resolve model and tool list from skill metadata (or overrides) + let (resolved_model, declared_tools, max_iter) = { + let skills = self.skills.read().await; + let skill = skills.get(skill_name); + let model = model_override + .map(str::to_string) + .or_else(|| skill.and_then(|s| s.model.clone())) + .unwrap_or_else(|| self.config.openrouter.model.clone()); + let tools = tools_override + .or_else(|| skill.map(|s| s.tools.clone())) + .unwrap_or_default(); + let max_i = skill + .and_then(|s| s.max_iterations) + .unwrap_or_else(|| self.config.max_iterations()) + .min(self.config.max_iterations()); + (model, tools, max_i) + }; + + let allowed_tools = effective_subagent_tools(&declared_tools); + + // Build the subagent tool definitions (filtered to whitelist only) + let all_possible_tools: Vec = { + let mut t = tools::builtin_tool_definitions(); + t.extend(self.mcp.tool_definitions()); + t.extend(self.skill_tool_definitions()); // includes read_skill_file + t + }; + let subagent_tools: Vec = all_possible_tools + .into_iter() + .filter(|td| allowed_tools.contains(&td.function.name)) + .collect(); + + // Bootstrap messages — system prompt instructs subagent to read its SKILL.md first + let system_content = format!( + "You are the '{}' subagent. Your first action MUST be to call \ + read_skill_file with skill_name='{}' and path='SKILL.md' to load your instructions.", + skill_name, skill_name + ); + let mut messages = vec![ + ChatMessage { + role: "system".to_string(), + content: Some(system_content), + tool_calls: None, + tool_call_id: None, + }, + ChatMessage { + role: "user".to_string(), + content: Some(prompt.to_string()), + tool_calls: None, + tool_call_id: None, + }, + ]; + + // Mini agentic loop (isolated — no memory, no scheduling) + for iteration in 0..max_iter { + let response = match self + .llm + .chat_with_model(&messages, &subagent_tools, &resolved_model) + .await + { + Ok(r) => r, + Err(e) => return format!("Subagent '{}' error: {}", skill_name, e), + }; + + if let Some(tool_calls) = &response.tool_calls { + if !tool_calls.is_empty() { + info!( + "Subagent '{}' requested {} tool call(s) (iteration {})", + skill_name, + tool_calls.len(), + iteration + ); + + messages.push(response.clone()); + + for tool_call in tool_calls { + let arguments: serde_json::Value = + serde_json::from_str(&tool_call.function.arguments) + .unwrap_or(serde_json::Value::Object(serde_json::Map::new())); + + // Only allow whitelisted tools + let result = if allowed_tools.contains(&tool_call.function.name) { + self.execute_tool( + &tool_call.function.name, + &arguments, + "", // subagent has no user_id context + "", // subagent has no chat_id context + ) + .await + } else { + format!( + "Tool '{}' is not available to this subagent.", + tool_call.function.name + ) + }; + + messages.push(ChatMessage { + role: "tool".to_string(), + content: Some(result), + tool_calls: None, + tool_call_id: Some(tool_call.id.clone()), + }); + } + + continue; + } + } + + // Final response — no tool calls + return response.content.unwrap_or_default(); + } + + format!( + "Subagent '{}' reached the maximum number of iterations ({}).", + skill_name, max_iter + ) +} +``` + +**Step 6: Add `invoke_subagent` tool definition** + +In `skill_tool_definitions()`, add a fourth entry after `read_skill_file`: + +```rust +ToolDefinition { + tool_type: "function".to_string(), + function: FunctionDefinition { + name: "invoke_subagent".to_string(), + description: concat!( + "Delegate a task to a named skill running as an isolated subagent. ", + "The subagent uses its own model and tool whitelist declared in its SKILL.md frontmatter. ", + "Use this for skills listed under 'Available Subagent Skills' in the system prompt. ", + "The subagent runs an isolated agentic loop and returns its final text response." + ).to_string(), + parameters: json!({ + "type": "object", + "properties": { + "skill": { + "type": "string", + "description": "Name of the skill to run as a subagent (e.g. 'thread-writer')" + }, + "prompt": { + "type": "string", + "description": "The task content to pass to the subagent" + }, + "model": { + "type": "string", + "description": "Optional: override the skill's declared model for this invocation" + }, + "tools": { + "type": "array", + "items": { "type": "string" }, + "description": "Optional: override the skill's declared tool whitelist" + } + }, + "required": ["skill", "prompt"] + }), + }, +}, +``` + +**Step 7: Add `invoke_subagent` handler in `execute_tool()`** + +Add a match arm after the `"reload_skills"` arm (around line 799): + +```rust +"invoke_subagent" => { + let skill = match arguments["skill"].as_str() { + Some(s) => s.to_string(), + None => return "Missing skill".to_string(), + }; + let prompt = match arguments["prompt"].as_str() { + Some(p) => p.to_string(), + None => return "Missing prompt".to_string(), + }; + let model_override = arguments["model"].as_str().map(str::to_string); + let tools_override = arguments["tools"].as_array().map(|arr| { + arr.iter() + .filter_map(|v| v.as_str().map(str::to_string)) + .collect::>() + }); + + info!( + "Invoking subagent '{}' (model_override: {:?})", + skill, model_override + ); + + self.run_subagent( + &skill, + &prompt, + model_override.as_deref(), + tools_override, + ) + .await +} +``` + +**Step 8: Verify compilation and tests** + +```bash +cargo check 2>&1 | grep -E "^error" | head -20 +cargo test --lib 2>&1 | tail -20 +``` + +Expected: 0 errors, all tests pass. + +**Step 9: Commit** + +```bash +git add src/agent.rs +git commit -m "feat(agent): add invoke_subagent tool and run_subagent mini-loop with model override" +``` + +--- + +## Task 6: Add example subagent skill and run full CI checks + +**Files:** +- Create: `skills/thread-writer/SKILL.md` + +**Step 1: Create the example subagent skill** + +Create `skills/thread-writer/SKILL.md`: + +```markdown +--- +name: thread-writer +description: Use when writing daily Thread posts from fetched source content. Invoke via invoke_subagent, not directly. +model: anthropic/claude-sonnet-4-6 +tools: [read_skill_file, mcp_threads_post] +max_iterations: 8 +--- + +# Thread Writer + +You are a specialized subagent that writes engaging daily Thread posts. + +## Your Task + +Given source content (e.g. email summaries, articles, notes), write a compelling Thread post that: +- Opens with a strong hook in the first post +- Breaks content into short, punchy posts (max 500 chars each) +- Uses a consistent voice: direct, insightful, no hype +- Ends with a clear takeaway or call to action +- Avoids filler phrases ("As an AI...", "In conclusion...") + +## Format + +Return the posts as a numbered list: +1. [first post — hook] +2. [second post] +... +N. [final post — takeaway] + +## Style Notes + +- Short sentences. Active voice. +- No hashtags unless the content is specifically about a trending topic. +- Emojis are optional but use sparingly (max 1 per post). +``` + +**Step 2: Run `cargo fmt`** + +```bash +cargo fmt --all 2>&1 +``` + +Expected: exits cleanly (no output means no formatting issues). + +**Step 3: Run `cargo clippy`** + +```bash +cargo clippy -- -D warnings 2>&1 | grep -E "^error|warning\[" | head -20 +``` + +Expected: 0 errors. Fix any warnings before proceeding. + +**Step 4: Run the full test suite** + +```bash +cargo test 2>&1 | tail -30 +``` + +Expected: all tests pass. + +**Step 5: Run `cargo build --release`** + +```bash +cargo build --release 2>&1 | grep -E "^error" | head -10 +``` + +Expected: 0 errors. (This takes a minute — it validates the release build.) + +**Step 6: Final commit** + +```bash +git add skills/thread-writer/SKILL.md +git commit -m "feat(skills): add thread-writer as example subagent skill" +``` + +**Step 7: Push to remote** + +```bash +git push -u origin claude/subagent-model-selection-5OTEa +``` + +Expected: pushed successfully. + +--- + +## Verification Checklist + +Before calling this done, confirm: + +- [ ] `cargo fmt --all -- --check` exits 0 +- [ ] `cargo clippy -- -D warnings` exits 0 +- [ ] `cargo test` passes all tests +- [ ] `cargo build --release` succeeds +- [ ] `git log --oneline` shows 6 commits on `claude/subagent-model-selection-5OTEa` + +--- + +## What Was NOT Built (intentional scope limits) + +- **Recursive subagents**: `invoke_subagent` is not in any subagent's tool definitions — no nesting by design +- **Subagent memory access**: subagents cannot call `remember`, `recall`, or `search_memory` — stateless by design +- **Subagent scheduling**: subagents cannot schedule tasks — orchestration stays in the main agent +- **Config-level subagent defaults**: all configuration lives in SKILL.md frontmatter, not `config.toml` diff --git a/skills/thread-writer/SKILL.md b/skills/thread-writer/SKILL.md new file mode 100644 index 0000000..beddfe3 --- /dev/null +++ b/skills/thread-writer/SKILL.md @@ -0,0 +1,34 @@ +--- +name: thread-writer +description: Use when writing daily Thread posts from fetched source content. Invoke via invoke_subagent, not directly. +model: anthropic/claude-sonnet-4-6 +tools: [read_skill_file, mcp_threads_post] +max_iterations: 8 +--- + +# Thread Writer + +You are a specialized subagent that writes engaging daily Thread posts. + +## Your Task + +Given source content (e.g. email summaries, articles, notes), write a compelling Thread post that: +- Opens with a strong hook in the first post +- Breaks content into short, punchy posts (max 500 chars each) +- Uses a consistent voice: direct, insightful, no hype +- Ends with a clear takeaway or call to action +- Avoids filler phrases ("As an AI...", "In conclusion...") + +## Format + +Return the posts as a numbered list: +1. [first post — hook] +2. [second post] +... +N. [final post — takeaway] + +## Style Notes + +- Short sentences. Active voice. +- No hashtags unless the content is specifically about a trending topic. +- Emojis are optional but use sparingly (max 1 per post). diff --git a/src/agent.rs b/src/agent.rs index 4002966..2efadf3 100644 --- a/src/agent.rs +++ b/src/agent.rs @@ -1,6 +1,6 @@ use anyhow::Result; use std::sync::{Arc, Weak}; -use tracing::info; +use tracing::{debug, info}; use teloxide::Bot; @@ -155,6 +155,9 @@ impl Agent { // Agentic loop — keep calling LLM until we get a non-tool response let max_iterations = self.config.max_iterations(); for iteration in 0..max_iterations { + + debug!("Trying iteration {}: {:?}", iteration, messages); + let response = self.llm.chat(&messages, &all_tools).await?; if let Some(tool_calls) = &response.tool_calls { @@ -187,6 +190,8 @@ impl Agent { tool_result.len() ); + debug!("Tool '{}' result: {}", tool_call.function.name, tool_result); + let tool_msg = ChatMessage { role: "tool".to_string(), content: Some(tool_result), @@ -507,9 +512,209 @@ impl Agent { parameters: json!({ "type": "object", "properties": {} }), }, }, + ToolDefinition { + tool_type: "function".to_string(), + function: FunctionDefinition { + name: "read_skill_file".to_string(), + description: concat!( + "Read a file from a skill directory. Use this to load a skill's full instructions ", + "when you decide a skill is relevant (call with relative_path='SKILL.md'), then follow the loaded content. ", + "Also use for supporting files (style guides, templates, reference docs)." + ).to_string(), + parameters: json!({ + "type": "object", + "properties": { + "skill_name": { + "type": "string", + "description": "Skill directory name (e.g. 'thread-writer')" + }, + "relative_path": { + "type": "string", + "description": "Path within the skill directory, e.g. 'SKILL.md', 'reference.md', 'scripts/helper.py'" + } + }, + "required": ["skill_name", "relative_path"] + }), + }, + }, + ToolDefinition { + tool_type: "function".to_string(), + function: FunctionDefinition { + name: "invoke_subagent".to_string(), + description: concat!( + "Delegate a task to a named skill running as an isolated subagent. ", + "The subagent uses its own model and tool whitelist declared in its SKILL.md frontmatter. ", + "Use this for skills listed under 'Available Subagent Skills' in the system prompt. ", + "The subagent runs an isolated agentic loop and returns its final text response." + ).to_string(), + parameters: json!({ + "type": "object", + "properties": { + "skill": { + "type": "string", + "description": "Name of the skill to run as a subagent (e.g. 'thread-writer')" + }, + "prompt": { + "type": "string", + "description": "The task content to pass to the subagent" + }, + "model": { + "type": "string", + "description": "Optional: override the skill's declared model for this invocation" + }, + "tools": { + "type": "array", + "items": { "type": "string" }, + "description": "Optional: override the skill's declared tool whitelist" + } + }, + "required": ["skill", "prompt"] + }), + }, + }, ] } + /// Run a named skill as an isolated subagent mini-loop. + /// Returns the subagent's final text response (or an error string). + async fn run_subagent( + &self, + skill_name: &str, + prompt: &str, + model_override: Option<&str>, + tools_override: Option>, + ) -> String { + // Resolve model and tool list from skill metadata (or overrides) + let (resolved_model, declared_tools, max_iter) = { + let skills = self.skills.read().await; + let skill = skills.get(skill_name); + let model = model_override + .map(str::to_string) + .or_else(|| skill.and_then(|s| s.model.clone())) + .unwrap_or_else(|| self.config.openrouter.model.clone()); + let tools = tools_override + .or_else(|| skill.map(|s| s.tools.clone())) + .unwrap_or_default(); + let max_i = skill + .and_then(|s| s.max_iterations) + .unwrap_or_else(|| self.config.max_iterations()) + .min(self.config.max_iterations()); + (model, tools, max_i) + }; + + let allowed_tools = effective_subagent_tools(&declared_tools); + + info!( + "Subagent '{}' using model: {} (allowed_tools: {} tools)", + skill_name, + resolved_model, + allowed_tools.len() + ); + + // Build the subagent tool definitions (filtered to whitelist only) + let all_possible_tools: Vec = { + let mut t = tools::builtin_tool_definitions(); + t.extend(self.mcp.tool_definitions()); + t.extend(self.skill_tool_definitions()); // includes read_skill_file + t + }; + let subagent_tools: Vec = all_possible_tools + .into_iter() + .filter(|td| allowed_tools.contains(&td.function.name)) + .collect(); + + // Bootstrap messages — system prompt instructs subagent to read its SKILL.md first + let system_content = format!( + "You are the '{}' subagent. Your first action MUST be to call \ + read_skill_file with skill_name='{}' and relative_path='SKILL.md' to load your instructions.", + skill_name, skill_name + ); + let mut messages = vec![ + ChatMessage { + role: "system".to_string(), + content: Some(system_content), + tool_calls: None, + tool_call_id: None, + }, + ChatMessage { + role: "user".to_string(), + content: Some(prompt.to_string()), + tool_calls: None, + tool_call_id: None, + }, + ]; + + // Mini agentic loop (isolated — no memory, no scheduling) + for iteration in 0..max_iter { + let response = match self + .llm + .chat_with_model(&messages, &subagent_tools, &resolved_model) + .await + { + Ok(r) => r, + Err(e) => return format!("Subagent '{}' error: {}", skill_name, e), + }; + + if let Some(tool_calls) = &response.tool_calls { + if !tool_calls.is_empty() { + info!( + "Subagent '{}' requested {} tool call(s) (iteration {})", + skill_name, + tool_calls.len(), + iteration + ); + + messages.push(response.clone()); + + for tool_call in tool_calls { + let arguments: serde_json::Value = + serde_json::from_str(&tool_call.function.arguments) + .unwrap_or(serde_json::Value::Object(serde_json::Map::new())); + + // Only allow whitelisted tools + let result = if allowed_tools.contains(&tool_call.function.name) { + self.execute_tool( + &tool_call.function.name, + &arguments, + "", // subagent has no user_id context + "", // subagent has no chat_id context + ) + .await + } else { + info!( + "Subagent '{}' denied tool '{}' (allowed: {:?})", + skill_name, + tool_call.function.name, + allowed_tools + ); + format!( + "Tool '{}' is not available to this subagent.", + tool_call.function.name + ) + }; + + messages.push(ChatMessage { + role: "tool".to_string(), + content: Some(result), + tool_calls: None, + tool_call_id: Some(tool_call.id.clone()), + }); + } + + continue; + } + } + + // Final response — no tool calls + return response.content.unwrap_or_default(); + } + + format!( + "Subagent '{}' reached the maximum number of iterations ({}).", + skill_name, max_iter + ) + } + /// Execute a tool call by routing to the right handler async fn execute_tool( &self, @@ -745,6 +950,52 @@ impl Agent { Err(e) => format!("Failed to update task status: {}", e), } } + "read_skill_file" => { + let skill_name = match arguments["skill_name"].as_str() { + Some(n) => n.to_string(), + None => return "Missing skill_name".to_string(), + }; + let relative_path = match arguments["relative_path"].as_str() { + Some(p) => p.to_string(), + None => return "Missing relative_path".to_string(), + }; + + if let Err(e) = validate_skill_name(&skill_name) { + return format!("Invalid skill_name: {}", e); + } + if let Err(e) = validate_skill_path(&relative_path) { + return format!("Invalid path: {}", e); + } + + let target = self + .config + .skills + .directory + .join(&skill_name) + .join(&relative_path); + + // Canonicalize to detect symlink escapes (same pattern as validate_sandbox_path). + // If either path doesn't exist yet, canonicalize returns Err and we skip the + // check — read_to_string will fail with not-found in that case. + if let Ok(skills_canonical) = self.config.skills.directory.canonicalize() { + if let Ok(target_canonical) = target.canonicalize() { + if !target_canonical.starts_with(&skills_canonical) { + return format!( + "Access denied: path '{}/{}' resolves outside the skills directory", + skill_name, relative_path + ); + } + } + } + + match tokio::fs::read_to_string(&target).await { + Ok(content) => content, + Err(e) => format!( + "Failed to read skill file '{}/{}': {}", + skill_name, relative_path, e + ), + } + } "write_skill_file" => { let skill_name = match arguments["skill_name"].as_str() { Some(n) => n.to_string(), @@ -797,6 +1048,35 @@ impl Agent { Err(e) => format!("Failed to reload skills: {}", e), } } + "invoke_subagent" => { + let skill = match arguments["skill"].as_str() { + Some(s) => s.to_string(), + None => return "Missing skill".to_string(), + }; + let prompt = match arguments["prompt"].as_str() { + Some(p) => p.to_string(), + None => return "Missing prompt".to_string(), + }; + let model_override = arguments["model"].as_str().map(str::to_string); + let tools_override = arguments["tools"].as_array().map(|arr| { + arr.iter() + .filter_map(|v| v.as_str().map(str::to_string)) + .collect::>() + }); + + info!( + "Invoking subagent '{}' (model_override: {:?})", + skill, model_override + ); + + Box::pin(self.run_subagent( + &skill, + &prompt, + model_override.as_deref(), + tools_override, + )) + .await + } _ if self.mcp.is_mcp_tool(name) => match self.mcp.call_tool(name, arguments).await { Ok(result) => result, Err(e) => format!("MCP tool error: {}", e), @@ -918,6 +1198,18 @@ fn validate_skill_path(path: &str) -> Result<(), String> { Ok(()) } +/// Build the effective tool whitelist for a subagent. +/// Always includes `read_skill_file`; deduplicates. +fn effective_subagent_tools(declared: &[String]) -> Vec { + let mut tools = vec!["read_skill_file".to_string()]; + for t in declared { + if t != "read_skill_file" { + tools.push(t.clone()); + } + } + tools +} + #[cfg(test)] mod tests { use super::*; @@ -1004,4 +1296,46 @@ mod tests { assert!(validate_skill_path("/etc/passwd").is_err()); assert!(validate_skill_path("/SKILL.md").is_err()); } + + #[test] + fn test_read_skill_file_validates_skill_name() { + // validate_skill_name is reused — just verify the boundary + assert!(validate_skill_name("valid-skill").is_ok()); + assert!(validate_skill_name("../evil").is_err()); + assert!(validate_skill_name("").is_err()); + } + + #[test] + fn test_read_skill_file_validates_relative_path() { + assert!(validate_skill_path("SKILL.md").is_ok()); + assert!(validate_skill_path("style-guide.md").is_ok()); + assert!(validate_skill_path("../other-skill/SKILL.md").is_err()); + assert!(validate_skill_path("/etc/passwd").is_err()); + assert!(validate_skill_path("").is_err()); + } + + #[test] + fn test_subagent_tool_whitelist_always_includes_read_skill_file() { + // read_skill_file is always available to subagents regardless of whitelist + let declared: Vec = vec!["mcp_threads_post".to_string()]; + let effective = effective_subagent_tools(&declared); + assert!(effective.contains(&"read_skill_file".to_string())); + assert!(effective.contains(&"mcp_threads_post".to_string())); + } + + #[test] + fn test_subagent_tool_whitelist_empty_gets_read_skill_file() { + let declared: Vec = vec![]; + let effective = effective_subagent_tools(&declared); + assert_eq!(effective, vec!["read_skill_file".to_string()]); + } + + #[test] + fn test_subagent_tool_whitelist_deduplicates_read_skill_file() { + // If the skill already lists read_skill_file, it shouldn't appear twice + let declared = vec!["read_skill_file".to_string(), "mcp_something".to_string()]; + let effective = effective_subagent_tools(&declared); + let count = effective.iter().filter(|t| *t == "read_skill_file").count(); + assert_eq!(count, 1); + } } diff --git a/src/llm.rs b/src/llm.rs index 15c85d1..f47c832 100644 --- a/src/llm.rs +++ b/src/llm.rs @@ -77,10 +77,12 @@ impl LlmClient { } } - pub async fn chat( + /// Chat with an explicit model string (used by subagents to override the default). + pub async fn chat_with_model( &self, messages: &[ChatMessage], tools: &[ToolDefinition], + model: &str, ) -> Result { let tools_param = if tools.is_empty() { None @@ -95,7 +97,7 @@ impl LlmClient { }; let request = ChatRequest { - model: self.config.model.clone(), + model: model.to_string(), messages: messages.to_vec(), tools: tools_param, tool_choice, @@ -104,7 +106,7 @@ impl LlmClient { let url = format!("{}/chat/completions", self.config.base_url); - debug!("Sending request to OpenRouter: {}", url); + debug!("Sending request to OpenRouter: {} model={}", url, request.model); let response = self .client @@ -134,4 +136,55 @@ impl LlmClient { .map(|c| c.message) .context("No response from OpenRouter") } + + /// Chat using the model configured in config.toml (delegates to chat_with_model). + pub async fn chat( + &self, + messages: &[ChatMessage], + tools: &[ToolDefinition], + ) -> Result { + self.chat_with_model(messages, tools, &self.config.model) + .await + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_chat_request_serializes_model_field() { + // Verifies the model string will appear in the JSON POST body + let req = ChatRequest { + model: "anthropic/claude-sonnet-4-6".to_string(), + messages: vec![], + tools: None, + tool_choice: None, + max_tokens: 100, + }; + let json = serde_json::to_value(&req).unwrap(); + assert_eq!(json["model"], "anthropic/claude-sonnet-4-6"); + } + + #[test] + fn test_chat_request_default_model_is_different_from_override() { + // Ensures chat_with_model can use a different model than the config default + let default_req = ChatRequest { + model: "moonshotai/kimi-k2.5".to_string(), + messages: vec![], + tools: None, + tool_choice: None, + max_tokens: 100, + }; + let override_req = ChatRequest { + model: "anthropic/claude-sonnet-4-6".to_string(), + messages: vec![], + tools: None, + tool_choice: None, + max_tokens: 100, + }; + let json_default = serde_json::to_value(&default_req).unwrap(); + let json_override = serde_json::to_value(&override_req).unwrap(); + assert_ne!(json_default["model"], json_override["model"]); + } } diff --git a/src/skills/loader.rs b/src/skills/loader.rs index b8d3a05..f9e748c 100644 --- a/src/skills/loader.rs +++ b/src/skills/loader.rs @@ -80,6 +80,9 @@ async fn load_skill_file(path: &Path) -> Result { description: description.unwrap_or_else(|| first_line_or_heading(&body)), content: body, tags, + model: extract_field(frontmatter, "model"), + tools: extract_list_field(frontmatter, "tools"), + max_iterations: extract_u32_field(frontmatter, "max_iterations"), }); } } @@ -93,6 +96,9 @@ async fn load_skill_file(path: &Path) -> Result { description, content: content.to_string(), tags: Vec::new(), + model: None, + tools: vec![], + max_iterations: None, }) } @@ -130,6 +136,11 @@ fn extract_list_field(frontmatter: &str, key: &str) -> Vec { Vec::new() } +/// Extract a `key: N` unsigned integer field from YAML-like frontmatter +fn extract_u32_field(frontmatter: &str, key: &str) -> Option { + extract_field(frontmatter, key)?.parse().ok() +} + /// Derive skill name from file path fn name_from_path(path: &Path) -> String { // If it's SKILL.md inside a directory, use the directory name @@ -161,3 +172,49 @@ fn first_line_or_heading(content: &str) -> String { } "No description".to_string() } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_extract_u32_field_present() { + let fm = "name: my-skill\nmax_iterations: 8\n"; + assert_eq!(extract_u32_field(fm, "max_iterations"), Some(8)); + } + + #[test] + fn test_extract_u32_field_absent() { + let fm = "name: my-skill\n"; + assert_eq!(extract_u32_field(fm, "max_iterations"), None); + } + + #[test] + fn test_extract_u32_field_invalid_value() { + let fm = "max_iterations: not-a-number\n"; + assert_eq!(extract_u32_field(fm, "max_iterations"), None); + } + + #[test] + fn test_load_skill_parses_model_field() { + let frontmatter = + "name: thread-writer\ndescription: Write posts\nmodel: anthropic/claude-sonnet-4-6\n"; + let model = extract_field(frontmatter, "model"); + assert_eq!(model.as_deref(), Some("anthropic/claude-sonnet-4-6")); + } + + #[test] + fn test_load_skill_parses_tools_field() { + let frontmatter = "tools: [read_skill_file, mcp_threads_post]\n"; + let tools = extract_list_field(frontmatter, "tools"); + assert_eq!(tools, vec!["read_skill_file", "mcp_threads_post"]); + } + + #[test] + fn test_load_skill_defaults_when_fields_absent() { + let frontmatter = "name: plain-skill\ndescription: Simple skill\n"; + assert_eq!(extract_field(frontmatter, "model"), None); + assert!(extract_list_field(frontmatter, "tools").is_empty()); + assert_eq!(extract_u32_field(frontmatter, "max_iterations"), None); + } +} diff --git a/src/skills/mod.rs b/src/skills/mod.rs index 51d7756..84fea4f 100644 --- a/src/skills/mod.rs +++ b/src/skills/mod.rs @@ -15,6 +15,12 @@ pub struct Skill { pub content: String, /// Category/tags for organization pub tags: Vec, + /// If set, this skill runs as a subagent using this model + pub model: Option, + /// Tool whitelist for the subagent (empty = read_skill_file only) + pub tools: Vec, + /// Max loop iterations for the subagent (None = use global config default) + pub max_iterations: Option, } /// Registry of all loaded skills @@ -48,19 +54,51 @@ impl SkillRegistry { } /// Build context string for the system prompt. - /// Gives the LLM awareness of all available skills. + /// All skills: metadata only (name + description). Instruction skills get a hint to load + /// full content via read_skill_file(SKILL.md) when relevant; subagent skills get invoke_subagent hint. pub fn build_context(&self) -> String { if self.skills.is_empty() { return String::new(); } - let mut context = String::from( - "You have the following skills available. When relevant, follow these instructions:\n\n", - ); + let mut instruction_lines = String::new(); + let mut subagent_section = String::new(); + for skill in self.skills.values() { - context.push_str(&format!("## Skill: {}\n", skill.name)); - context.push_str(&format!("{}\n\n", skill.content)); + if skill.model.is_some() { + // Subagent skill — metadata only + invoke_subagent hint + subagent_section.push_str(&format!( + "- **{}**: {}\n Invoke via: `invoke_subagent(skill=\"{}\", prompt=\"\")`\n", + skill.name, skill.description, skill.name + )); + } else { + // Instruction skill — metadata only + read_skill_file hint (no full body) + instruction_lines.push_str(&format!( + "- **{}** (instruction): {}. Load with: read_skill_file(skill_name=\"{}\", relative_path=\"SKILL.md\") when relevant.\n", + skill.name, skill.description, skill.name + )); + } + } + + let mut context = String::new(); + + if !instruction_lines.is_empty() { + context.push_str( + "When an instruction skill is relevant, load its full instructions with read_skill_file(skill_name=\"\", relative_path=\"SKILL.md\"), then follow them. For subagent skills use invoke_subagent.\n\n", + ); + context.push_str("You have the following skills available:\n\n"); + context.push_str(&instruction_lines); + } + + if !subagent_section.is_empty() { + if !instruction_lines.is_empty() { + context.push('\n'); + } + context.push_str("## Available Subagent Skills\n\n"); + context.push_str("Delegate these tasks using `invoke_subagent`:\n\n"); + context.push_str(&subagent_section); } + context } @@ -73,3 +111,92 @@ impl SkillRegistry { self.skills.is_empty() } } + +#[cfg(test)] +mod tests { + use super::*; + + fn make_skill(name: &str, description: &str, content: &str, model: Option<&str>) -> Skill { + Skill { + name: name.to_string(), + description: description.to_string(), + content: content.to_string(), + tags: vec![], + model: model.map(str::to_string), + tools: vec![], + max_iterations: None, + } + } + + #[test] + fn test_build_context_instruction_skill_metadata_only() { + // Instruction skills: metadata only + read_skill_file hint; full body NOT injected + let mut registry = SkillRegistry::new(); + registry.register(make_skill( + "my-skill", + "Does things", + "# Instructions\nDo this and that.", + None, // no model = instruction skill + )); + let ctx = registry.build_context(); + assert!(ctx.contains("my-skill")); + assert!(ctx.contains("Does things")); + assert!(ctx.contains("read_skill_file")); + assert!(ctx.contains("SKILL.md")); + assert!(!ctx.contains("# Instructions")); + assert!(!ctx.contains("Do this and that.")); + } + + #[test] + fn test_build_context_subagent_skill_injects_metadata_only() { + // Skills with a model field get only name + description + invoke hint + let mut registry = SkillRegistry::new(); + registry.register(make_skill( + "thread-writer", + "Use when writing Thread posts.", + "# Super Secret Instructions\nLong style guide...", + Some("anthropic/claude-sonnet-4-6"), + )); + let ctx = registry.build_context(); + // Metadata present + assert!(ctx.contains("thread-writer")); + assert!(ctx.contains("Use when writing Thread posts.")); + assert!(ctx.contains("invoke_subagent")); + // Body NOT present + assert!(!ctx.contains("Super Secret Instructions")); + assert!(!ctx.contains("Long style guide")); + } + + #[test] + fn test_build_context_empty_registry() { + let registry = SkillRegistry::new(); + assert_eq!(registry.build_context(), String::new()); + } + + #[test] + fn test_build_context_mixed_skills() { + // Both skill types: metadata only; instruction body and subagent body NOT in context + let mut registry = SkillRegistry::new(); + registry.register(make_skill( + "instruction-skill", + "An instruction skill", + "Follow these instructions.", + None, + )); + registry.register(make_skill( + "subagent-skill", + "A subagent skill", + "Secret subagent body.", + Some("some/model"), + )); + let ctx = registry.build_context(); + assert!(ctx.contains("instruction-skill")); + assert!(ctx.contains("An instruction skill")); + assert!(ctx.contains("read_skill_file")); + assert!(!ctx.contains("Follow these instructions.")); + assert!(!ctx.contains("Secret subagent body.")); + assert!(ctx.contains("invoke_subagent")); + assert!(ctx.contains("You have the following skills available")); + assert!(ctx.contains("Available Subagent Skills")); + } +}