diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml deleted file mode 100644 index 87d539a..0000000 --- a/.github/workflows/claude-code-review.yml +++ /dev/null @@ -1,48 +0,0 @@ -name: Claude Code Review - -on: - pull_request: - types: [opened, synchronize, ready_for_review, reopened] - # Optional: Only run on specific file changes - # paths: - # - "src/**/*.ts" - # - "src/**/*.tsx" - # - "src/**/*.js" - # - "src/**/*.jsx" - -jobs: - claude-review: - # Optional: Filter by PR author - # if: | - # github.event.pull_request.user.login == 'external-contributor' || - # github.event.pull_request.user.login == 'new-developer' || - # github.event.pull_request.author_association == 'FIRST_TIME_CONTRIBUTOR' - - runs-on: ubuntu-latest - permissions: - contents: read - pull-requests: write # required to post review comments (read silently fails) - issues: write # used when posting/updating PR comment - id-token: write - - steps: - - name: Checkout repository - uses: actions/checkout@v4 - with: - fetch-depth: 1 - - - name: Run Claude Code Review - id: claude-review - uses: anthropics/claude-code-action@v1 - with: - claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} - github_token: ${{ github.token }} # ensures workflow permissions are used for posting - plugin_marketplaces: 'https://github.com/anthropics/claude-code.git' - plugins: 'code-review@claude-code-plugins' - prompt: '/code-review:code-review ${{ github.repository }}/pull/${{ github.event.pull_request.number }} --comment' - track_progress: true # forces tag mode so a PR comment is created and updated (fixes #944) - # If gh CLI calls are blocked, uncomment: - # claude_args: '--allowedTools "Bash(gh api*),Bash(gh pr*),Bash(git*)"' - # See https://github.com/anthropics/claude-code-action/issues/944 - # and https://github.com/anthropics/claude-code-action/blob/main/docs/usage.md - diff --git a/.opencode/agents/code-quality-reviewer.md b/.opencode/agents/code-quality-reviewer.md new file mode 100644 index 0000000..31fab79 --- /dev/null +++ b/.opencode/agents/code-quality-reviewer.md @@ -0,0 +1,30 @@ +--- +description: Reviews implementation code quality — cleanliness, test coverage, maintainability, structure. Only dispatch after spec compliance review passes. +mode: subagent +model: opencode/minimax-m3-free +permission: + read: allow + glob: allow + grep: allow + edit: deny + bash: allow +--- + +You are reviewing code quality and implementation structure. + +**Only review after spec compliance is confirmed.** Focus on whether the implementation is well-built: + +1. **File responsibility** — Does each file have one clear responsibility with a well-defined interface? +2. **Decomposition** — Are units decomposed so they can be understood and tested independently? +3. **Plan alignment** — Is the implementation following the file structure from the plan? +4. **File size** — Did this change create files that are already large, or significantly grow existing files? (Don't flag pre-existing sizes.) + +**Standard quality concerns:** +- Clean, readable code +- Proper error handling +- Meaningful names +- No duplication +- Test quality (do tests verify behavior, not just mocks?) +- Edge case coverage + +**Report format:** Strengths, Issues (Critical / Important / Minor), Assessment diff --git a/.opencode/agents/code-reviewer.md b/.opencode/agents/code-reviewer.md new file mode 100644 index 0000000..41e5ba9 --- /dev/null +++ b/.opencode/agents/code-reviewer.md @@ -0,0 +1,30 @@ +--- +description: Reviews completed project steps against original plans, coding standards, and best practices. Use when a major project step has been completed and needs review. +mode: subagent +model: opencode/minimax-m3-free +permission: + read: allow + glob: allow + grep: allow + edit: deny + bash: + "git diff *": allow + "git log *": allow + "git show *": allow +--- + +You are a Senior Code Reviewer with expertise in software architecture, design patterns, and best practices. + +When reviewing completed work: + +1. **Plan Alignment Analysis**: Compare the implementation against the original planning document. Identify deviations and assess whether they're justified improvements or problematic departures. + +2. **Code Quality Assessment**: Check for proper error handling, type safety, defensive programming, code organization, naming conventions, and maintainability. + +3. **Architecture and Design Review**: Ensure SOLID principles, separation of concerns, loose coupling, and proper integration with existing systems. + +4. **Documentation and Standards**: Verify comments, documentation, and adherence to project coding standards. + +5. **Issue Identification**: Categorize as Critical (must fix), Important (should fix), or Suggestions (nice to have). Provide specific examples and actionable recommendations. + +Output structured, actionable feedback. Acknowledge what was done well before highlighting issues. diff --git a/.opencode/agents/implementer.md b/.opencode/agents/implementer.md new file mode 100644 index 0000000..2e0d8b0 --- /dev/null +++ b/.opencode/agents/implementer.md @@ -0,0 +1,38 @@ +--- +description: Implements spec-defined tasks from plans. Writes tests, implements features, verifies work, and commits. Best for mechanical implementation with clear specs. +mode: subagent +model: opencode/big-pickle +permission: + read: allow + write: allow + edit: allow + glob: allow + grep: allow + bash: allow +--- + +You are implementing a task from an implementation plan. + +When you receive a task: +1. If you have questions about requirements, approach, or dependencies, **ask them now** before starting. +2. Implement exactly what the task specifies. +3. Write tests following TDD if applicable. +4. Verify implementation works. +5. Commit your work. +6. Self-review before reporting back. + +**Code Organization:** +- Follow the file structure defined in the plan. +- Each file should have one clear responsibility. +- If a file grows beyond plan intent, report as DONE_WITH_CONCERNS. +- Follow established patterns in existing codebases. + +**When stuck:** It's always OK to say "this is too hard." Report BLOCKED or NEEDS_CONTEXT with specifics. + +**Report Format:** +- **Status:** DONE | DONE_WITH_CONCERNS | BLOCKED | NEEDS_CONTEXT +- What you implemented +- Test results +- Files changed +- Self-review findings +- Any concerns diff --git a/.opencode/agents/plan-document-reviewer.md b/.opencode/agents/plan-document-reviewer.md new file mode 100644 index 0000000..045c17e --- /dev/null +++ b/.opencode/agents/plan-document-reviewer.md @@ -0,0 +1,34 @@ +--- +description: Reviews implementation plans for completeness, spec alignment, task decomposition, and buildability before execution. +mode: subagent +model: opencode/deepseek-v4-flash-free +permission: + read: allow + edit: deny + bash: deny +--- + +You are a plan document reviewer. Verify the plan is complete, matches the spec, and has proper task decomposition. + +**What to Check:** + +| Category | What to Look For | +|----------|------------------| +| Completeness | TODOs, placeholders, incomplete tasks, missing steps | +| Spec Alignment | Plan covers spec requirements, no major scope creep | +| Task Decomposition | Tasks have clear boundaries, steps are actionable | +| Buildability | Could an engineer follow this plan without getting stuck? | + +**Calibration:** Only flag issues that would cause real problems during implementation. An implementer building the wrong thing or getting stuck is an issue. Minor wording is not. + +**Output Format:** + +## Plan Review + +**Status:** Approved | Issues Found + +**Issues (if any):** +- [Task X, Step Y]: [specific issue] - [why it matters] + +**Recommendations (advisory):** +- [suggestions for improvement] diff --git a/.opencode/agents/spec-document-reviewer.md b/.opencode/agents/spec-document-reviewer.md new file mode 100644 index 0000000..f8d3493 --- /dev/null +++ b/.opencode/agents/spec-document-reviewer.md @@ -0,0 +1,35 @@ +--- +description: Reviews specification documents for completeness, consistency, clarity, and readiness before planning begins. +mode: subagent +model: opencode/mimo-v2.5-free +permission: + read: allow + edit: deny + bash: deny +--- + +You are a spec document reviewer. Verify the spec is complete, consistent, and ready for implementation planning. + +**What to Check:** + +| Category | What to Look For | +|----------|------------------| +| Completeness | TODOs, placeholders, "TBD", incomplete sections | +| Consistency | Internal contradictions, conflicting requirements | +| Clarity | Requirements ambiguous enough to cause building the wrong thing | +| Scope | Focused enough for a single plan | +| YAGNI | Unrequested features, over-engineering | + +**Calibration:** Only flag issues that would cause real problems during implementation planning. Minor wording or stylistic preferences are not issues. + +**Output Format:** + +## Spec Review + +**Status:** Approved | Issues Found + +**Issues (if any):** +- [Section]: [specific issue] - [why it matters] + +**Recommendations (advisory):** +- [suggestions for improvement] diff --git a/.opencode/agents/spec-reviewer.md b/.opencode/agents/spec-reviewer.md new file mode 100644 index 0000000..6ed6b70 --- /dev/null +++ b/.opencode/agents/spec-reviewer.md @@ -0,0 +1,30 @@ +--- +description: Verifies that an implementation matches its specification exactly — nothing more, nothing less. Dispatch after an implementer completes work. +mode: subagent +model: opencode/minimax-m3-free +permission: + read: allow + glob: allow + grep: allow + edit: deny + bash: allow +--- + +You are reviewing whether an implementation matches its specification. + +**CRITICAL: Do not trust the implementer's report.** The implementer may be incomplete, inaccurate, or optimistic. You MUST verify everything independently by reading the actual code. + +DO: +- Read the actual code they wrote +- Compare actual implementation to requirements line by line +- Check for missing pieces they claimed to implement +- Look for extra features they didn't mention + +**Check for:** +1. **Missing requirements** — Did they implement everything requested? +2. **Extra/unneeded work** — Did they build things not requested? Over-engineer? +3. **Misunderstandings** — Did they interpret requirements differently than intended? + +**Report:** +- ✅ Spec compliant (if code matches spec after verification) +- ❌ Issues found: [list with file:line references] diff --git a/.opencode/opencode.json b/.opencode/opencode.json new file mode 100644 index 0000000..bb15fc0 --- /dev/null +++ b/.opencode/opencode.json @@ -0,0 +1,6 @@ +{ + "$schema": "https://opencode.ai/config.json", + "plugin": [ + "superpowers@git+https://github.com/obra/superpowers.git" + ] +} diff --git a/CLAUDE.md b/CLAUDE.md index 1137dec..155cee2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -38,6 +38,19 @@ Copy `config.example.toml` to `config.toml` and fill in credentials. The `config - `openrouter.api_key` - OpenRouter API key - `sandbox.allowed_directory` - Directory for sandboxed file/command operations +### Home directory + +RustFox stores all state under a single home directory (default `~/.rustfox`), +resolved as: `RUSTFOX_HOME` env (absolute) → `[general].home` config → `~/.rustfox`. +Layout: `config.toml`, `rustfox.db`, `skills/`, `agents/`, `workspace/` (the +sandbox), `artifacts/`, `user_model.md`. Each path can be pinned to an absolute +location in `config.toml`; unset paths fall back to the home default. Run +isolated instances with `RUSTFOX_HOME=...`. See +`docs/persistent-home-directory.md`. Path resolution lives in `src/home.rs` +(`Config::resolve` writes the resolved absolute paths back into the config). +Bundled skills/agents are seed-copied on first run; `/update-skills` re-syncs +them using `/skills-lock.json`. + ## Architecture ``` diff --git a/Cargo.lock b/Cargo.lock index de36f34..0163a57 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -393,6 +393,27 @@ dependencies = [ "crypto-common", ] +[[package]] +name = "dirs" +version = "5.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "44c45a9d03d6676652bcb5e724c7e988de1acad23a711b5217ab9cbecbec2225" +dependencies = [ + "dirs-sys", +] + +[[package]] +name = "dirs-sys" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "520f05a5cbd335fae5a99ff7a6ab8627577660ee5cfd6a94a6a929b52ff0321c" +dependencies = [ + "libc", + "option-ext", + "redox_users", + "windows-sys 0.48.0", +] + [[package]] name = "displaydoc" version = "0.2.5" @@ -1101,6 +1122,15 @@ version = "0.2.180" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bcc35a38544a891a5f7c865aca548a982ccb3b8650a5b06d0fd33a10283c56fc" +[[package]] +name = "libredox" +version = "0.1.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f02ab6bace2054fb888a3c16f990117b579d14a3088e472d63c6011fa185c9d3" +dependencies = [ + "libc", +] + [[package]] name = "libsqlite3-sys" version = "0.32.0" @@ -1310,6 +1340,12 @@ dependencies = [ "vcpkg", ] +[[package]] +name = "option-ext" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "04744f49eae99ab78e0d5c0b603ab218f515ea8cfe5a456d7629ad883a3b6e7d" + [[package]] name = "parking_lot" version = "0.12.5" @@ -1568,6 +1604,17 @@ dependencies = [ "bitflags", ] +[[package]] +name = "redox_users" +version = "0.4.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ba009ff324d1fc1b900bd1fdb31564febe58a8ccc8a6fdbb93b543d33b13ca43" +dependencies = [ + "getrandom 0.2.17", + "libredox", + "thiserror 1.0.69", +] + [[package]] name = "ref-cast" version = "1.0.25" @@ -1704,7 +1751,7 @@ dependencies = [ "serde", "serde_json", "sse-stream", - "thiserror", + "thiserror 2.0.18", "tokio", "tokio-stream", "tokio-util", @@ -1754,6 +1801,7 @@ dependencies = [ "axum", "base64", "chrono", + "dirs", "futures", "futures-util", "pulldown-cmark", @@ -2227,7 +2275,7 @@ dependencies = [ "serde_json", "teloxide-core", "teloxide-macros", - "thiserror", + "thiserror 2.0.18", "tokio", "tokio-stream", "tokio-util", @@ -2259,7 +2307,7 @@ dependencies = [ "stacker", "take_mut", "takecell", - "thiserror", + "thiserror 2.0.18", "tokio", "tokio-util", "url", @@ -2291,13 +2339,33 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "thiserror" +version = "1.0.69" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b6aaf5339b578ea85b50e080feb250a3e8ae8cfcdff9a461c9ec2904bc923f52" +dependencies = [ + "thiserror-impl 1.0.69", +] + [[package]] name = "thiserror" version = "2.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4288b5bcbc7920c07a1149a35cf9590a2aa808e0bc1eafaade0b80947865fbc4" dependencies = [ - "thiserror-impl", + "thiserror-impl 2.0.18", +] + +[[package]] +name = "thiserror-impl" +version = "1.0.69" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4fee6c4efc90059e10f81e6d42c60a18f76588c3d74cb83a0b242a2b6c7504c1" +dependencies = [ + "proc-macro2", + "quote", + "syn", ] [[package]] @@ -2948,6 +3016,15 @@ dependencies = [ "windows-link", ] +[[package]] +name = "windows-sys" +version = "0.48.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "677d2418bec65e3338edb076e806bc1ec15693c5d0104683f2efe857f61056a9" +dependencies = [ + "windows-targets 0.48.5", +] + [[package]] name = "windows-sys" version = "0.52.0" @@ -2984,6 +3061,21 @@ dependencies = [ "windows-link", ] +[[package]] +name = "windows-targets" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9a2fa6e2155d7247be68c096456083145c183cbbbc2764150dda45a87197940c" +dependencies = [ + "windows_aarch64_gnullvm 0.48.5", + "windows_aarch64_msvc 0.48.5", + "windows_i686_gnu 0.48.5", + "windows_i686_msvc 0.48.5", + "windows_x86_64_gnu 0.48.5", + "windows_x86_64_gnullvm 0.48.5", + "windows_x86_64_msvc 0.48.5", +] + [[package]] name = "windows-targets" version = "0.52.6" @@ -3026,6 +3118,12 @@ dependencies = [ "windows-link", ] +[[package]] +name = "windows_aarch64_gnullvm" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2b38e32f0abccf9987a4e3079dfb67dcd799fb61361e53e2882c3cbaf0d905d8" + [[package]] name = "windows_aarch64_gnullvm" version = "0.52.6" @@ -3038,6 +3136,12 @@ version = "0.53.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a9d8416fa8b42f5c947f8482c43e7d89e73a173cead56d044f6a56104a6d1b53" +[[package]] +name = "windows_aarch64_msvc" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc35310971f3b2dbbf3f0690a219f40e2d9afcf64f9ab7cc1be722937c26b4bc" + [[package]] name = "windows_aarch64_msvc" version = "0.52.6" @@ -3050,6 +3154,12 @@ version = "0.53.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b9d782e804c2f632e395708e99a94275910eb9100b2114651e04744e9b125006" +[[package]] +name = "windows_i686_gnu" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a75915e7def60c94dcef72200b9a8e58e5091744960da64ec734a6c6e9b3743e" + [[package]] name = "windows_i686_gnu" version = "0.52.6" @@ -3074,6 +3184,12 @@ version = "0.53.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fa7359d10048f68ab8b09fa71c3daccfb0e9b559aed648a8f95469c27057180c" +[[package]] +name = "windows_i686_msvc" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8f55c233f70c4b27f66c523580f78f1004e8b5a8b659e05a4eb49d4166cca406" + [[package]] name = "windows_i686_msvc" version = "0.52.6" @@ -3086,6 +3202,12 @@ version = "0.53.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1e7ac75179f18232fe9c285163565a57ef8d3c89254a30685b57d83a38d326c2" +[[package]] +name = "windows_x86_64_gnu" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "53d40abd2583d23e4718fddf1ebec84dbff8381c07cae67ff7768bbf19c6718e" + [[package]] name = "windows_x86_64_gnu" version = "0.52.6" @@ -3098,6 +3220,12 @@ version = "0.53.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9c3842cdd74a865a8066ab39c8a7a473c0778a3f29370b5fd6b4b9aa7df4a499" +[[package]] +name = "windows_x86_64_gnullvm" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0b7b52767868a23d5bab768e390dc5f5c55825b6d30b86c844ff2dc7414044cc" + [[package]] name = "windows_x86_64_gnullvm" version = "0.52.6" @@ -3110,6 +3238,12 @@ version = "0.53.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0ffa179e2d07eee8ad8f57493436566c7cc30ac536a3379fdf008f47f6bb7ae1" +[[package]] +name = "windows_x86_64_msvc" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ed94fce61571a4006852b7389a063ab983c02eb1bb37b47f8272ce92d06d9538" + [[package]] name = "windows_x86_64_msvc" version = "0.52.6" diff --git a/Cargo.toml b/Cargo.toml index 42ba00e..792fa67 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -66,5 +66,8 @@ base64 = "0.22" # Secret-redaction filter (M7.4) regex = "1" +# OS home-directory resolution for the persistent home dir (~/.rustfox) +dirs = "5" + [dev-dependencies] tempfile = "3" diff --git a/README.md b/README.md index b086364..ac40559 100644 --- a/README.md +++ b/README.md @@ -8,12 +8,12 @@ [![MIT License](https://img.shields.io/badge/license-MIT-blue.svg)](LICENSE) [![Buy Me a Coffee](https://img.shields.io/badge/buy%20me%20a%20coffee-%E2%98%95-yellow)](https://buymeacoffee.com/chinkan.ai) -A self-hosted, agentic Telegram AI assistant written in Rust, powered by OpenRouter LLM (default: `moonshotai/kimi-k2.5`) with built-in sandboxed tools, scheduling, persistent memory, and MCP server integration. +A self-hosted, agentic Telegram AI assistant written in Rust, powered by OpenRouter LLM (default: `moonshotai/kimi-k2.6`) with built-in sandboxed tools, scheduling, persistent memory, and MCP server integration. ## Features - **Telegram Bot** — Responds only to configured user IDs -- **OpenRouter LLM** — Configurable model (default: `moonshotai/kimi-k2.5`) +- **OpenRouter LLM** — Configurable model (default: `moonshotai/kimi-k2.6`) - **Built-in Tools** — File read/write, directory listing, command execution (sandboxed) - **Scheduling Tools** — Schedule, list, and cancel recurring or one-shot tasks - **Persistent Memory** — SQLite-backed conversation history and knowledge base @@ -30,6 +30,12 @@ A self-hosted, agentic Telegram AI assistant written in Rust, powered by OpenRou - **Verbose Tool UI** — `/verbose` command toggles a live Telegram status message showing tool calls as they run - **Agentic Loop** — Automatic multi-step tool calling until task completion (max iterations configurable, default 25) - **Per-user Conversations** — Independent conversation history per user +- **Persistent Home Directory** — All state under `~/.rustfox` (config, DB, skills, agents, workspace); override via `RUSTFOX_HOME` env or `[general].home` config +- **Autopilot Supervisor** — Generic autonomous task runner with classification, planning, multi-backend execution, verification, and approval gates; `/supervise` to submit tasks +- **LangSmith Tracing** — Optional observability via LangSmith for LLM calls, tool runs, and chain traces +- **Post-task Learning** — Auto-extracts reusable skill patterns from completed agentic loops; persists honcho-style user model +- **Skill/Agent Update Engine** — Content-hash diffing with lock files; `/update-skills` re-syncs bundled skills/agents with backup of local edits +- **Instance + Bundled Layering** — Skills and agents load from two directories (instance shadows bundled); bundled templates ship with the project ## Quick Start @@ -69,6 +75,11 @@ cargo run --bin rustfox -- /path/to/config.toml ## Configuration +> **Persistent home:** RustFox keeps all state under `~/.rustfox` by default +> (config, database, skills, agents, and a durable `workspace/` sandbox). +> Override with the `RUSTFOX_HOME` environment variable or `[general].home`. +> See [docs/persistent-home-directory.md](docs/persistent-home-directory.md). + See [`config.example.toml`](config.example.toml) for all options. ### Key Settings @@ -78,14 +89,23 @@ See [`config.example.toml`](config.example.toml) for all options. | `telegram.bot_token` | Telegram Bot API token | | `telegram.allowed_user_ids` | List of user IDs allowed to use the bot | | `openrouter.api_key` | OpenRouter API key | -| `openrouter.model` | LLM model ID (default: `moonshotai/kimi-k2.5`) | +| `openrouter.model` | LLM model ID (default: `moonshotai/kimi-k2.6`) | | `sandbox.allowed_directory` | Directory for file/command operations | -| `memory.database_path` | SQLite DB path (default: `rustfox.db`) | +| `memory.database_path` | SQLite DB path (default: `/rustfox.db`) | +| `memory.user_model_path` | User model file path (default: `/user_model.md`) | +| `memory.query_rewriter_enabled` | Whether RAG query rewriting is on by default | | `embedding` (optional) | Vector search API config (default model: `qwen/qwen3-embedding-8b`) | -| `skills.directory` | Folder of bot skill files (default: `skills/`) | -| `agents.directory` | Folder of agent definition files (default: `agents/`) | +| `skills.directory` | Instance (writable) skill files (default: `/skills/`) | +| `skills.bundled_directory` | Bundled (read-only) skill templates (default: `./skills/`) | +| `agents.directory` | Instance (writable) agent files (default: `/agents/`) | +| `agents.bundled_directory` | Bundled (read-only) agent templates (default: `./agents/`) | | `mcp_servers` | List of MCP servers to connect | +| `general.home` | Absolute path overriding `~/.rustfox` home root | | `general.location` | Your location string (under `[general]`), injected into system prompt | +| `agent.max_iterations` | Max agentic loop iterations (default: 25) | +| `langsmith.api_key` | LangSmith API key for LLM observability | +| `learning.skill_extraction_enabled` | Post-task skill extraction on/off | +| `supervisor.default_autonomy_mode` | Supervisor workflow mode: `fast`, `standard`, `rigorous` | ### MCP Server Configuration @@ -105,6 +125,7 @@ MCP servers are usually distributed as Python packages (run via `uvx`) or npm pa Add one `[[mcp_servers]]` block per server in `config.toml`: ```toml +# Stdio-based server [[mcp_servers]] name = "server-name" # used to namespace tools: mcp__ command = "uvx" # or "npx", or any executable on PATH @@ -113,6 +134,17 @@ args = ["package-name", "optional-arg"] # Optional: pass environment variables to the server process # [mcp_servers.env] # API_KEY = "your-key-here" + +# HTTP/Streamable HTTP server (omit command for this transport) +# [[mcp_servers]] +# name = "api-server" +# url = "https://api.example.com/mcp" +# auth_token = "bearer-token-here" + +# OAuth 2.0 refresh flow (auto-exchanges refresh_token for new auth_token) +# token_endpoint = "https://api.example.com/oauth/token" +# refresh_token = "your-refresh-token" +# token_expires_at = ``` #### Popular MCP Servers @@ -188,14 +220,24 @@ Tools from MCP servers are automatically namespaced as `mcp___` | Submit a new supervisor task | +| `/tasks` | List active / recent supervisor tasks | +| `/resume ` | Resume a paused supervisor task | +| `/cancel ` | Cancel a supervisor task | +| `/approve ` | Approve a task that requires approval | +| `/clarify ` | Reply to a clarification prompt | ## Architecture ``` src/ ├── main.rs # Entry point, config loading, initialization -├── config.rs # TOML configuration parsing +├── config.rs # TOML configuration parsing (Telegram, OpenRouter, sandbox, memory, skills, agents, langsmith, learning, supervisor) +├── home.rs # Persistent home directory resolution (~/.rustfox) ├── llm.rs # OpenRouter API client with tool calling -├── agent.rs # Agentic loop, tool dispatch, scheduling tools; agents/ layer +├── agent.rs # Agentic loop, tool dispatch, scheduling tools; skills/agents/ layer +├── agent_prompt.rs # Prompt preparation, message assembly, recovery nudges ├── tools.rs # Built-in tools (file I/O, command execution, plan tools) ├── mcp.rs # MCP client manager for external tool servers ├── memory/ # SQLite persistence, vector embeddings, RAG, query rewriter, summarizer ├── scheduler/ # Cron/one-shot task scheduler with DB persistence -├── skills/ # Skill loader (auto-loads from skills/ directory) -└── platform/ # Telegram bot handler - +├── skills/ # Skill loader, registry, seeding, update engine (loader.rs, mod.rs, seed.rs, update.rs) +├── learning.rs # Post-task skill extraction, user model persistence +├── langsmith.rs # Optional LangSmith observability client +├── supervisor/ # Autopilot v2 generic autonomous task runner +│ ├── mod.rs # Supervisor facade (submit, execute_now, pause, resume, state, artifacts) +│ ├── task.rs # Task, TaskType, RiskLevel, ExecutionMode, TaskStatus enums +│ ├── job.rs # Job, JobType, JobStatus, JobOutput, Evidence +│ ├── state.rs # Transition-allowed state machine +│ ├── store.rs # CRUD over sup_tasks / sup_jobs / sup_transitions +│ ├── intake.rs # IntakeRouter — raw text → Task normalization +│ ├── classifier.rs # Heuristic / LLM-backed / Skill-aware classifiers +│ ├── policy.rs # PolicyEngine — auto-execute, clarify, require approval +│ ├── planner.rs # Task → Plan with jobs and parallel groups +│ ├── workflow.rs # Fast / Standard / Rigorous workflow templates +│ ├── orchestrator.rs # Plan executor with fallback + parallel + subjobs +│ ├── verification.rs # Evidence-gated verification engine +│ ├── artifact.rs # ArtifactManager with secret redaction +│ ├── workspace.rs # Per-task git worktree management +│ ├── reporter.rs # Human-readable job summary +│ ├── redact.rs # Secret scrubber for api_key / password / token / bearer +│ └── backend/ # Backend implementations (reasoning, shell, MCP, claude-code, codex, script) +├── platform/ # Telegram bot handler (telegram.rs + tool_notifier.rs) +└── utils/ # String utilities, markdown-to-entities conversion + +skills/ # Bundled skills (15+): code-interpreter, problem-solver, coding-assistant, +│ # soul, soul-keeper, memory-manager, creating-skills, creating-agents, +│ # news-fetcher, codebase-gap-analysis, sup-* workflow skills agents/ # Agent definition files (AGENT.md per agent) -skills/ -├── code-interpreter/ # Subagent skill: execute and iterate code snippets -├── problem-solver/ # Subagent skill: multi-step reasoning with plan tools -└── creating-agents/ # Instruction skill: how to author new agents +setup/ # Setup wizard HTML ``` ## Roadmap @@ -262,6 +336,13 @@ skills/ - [x] Nightly conversation summarization (LLM-based cron job) - [x] Verbose tool UI (`/verbose` command — live tool call progress in Telegram) - [x] Google integration tools (Calendar, Email, Drive) +- [x] Persistent home directory (`~/.rustfox` with env/config override) +- [x] Autopilot v2 supervisor (classification, planning, multi-backend execution, verification) +- [x] LangSmith observability (LLM/tool/chain tracing) +- [x] Post-task skill extraction (auto-learns reusable patterns) +- [x] User model persistence (honcho-style `user_model.md`) +- [x] Skill/agent content hash engine + lock-file re-sync +- [x] Instance + bundled skills/agents layering ### Planned @@ -291,5 +372,10 @@ If you find RustFox useful, consider supporting the project: - [tokio](https://tokio.rs/) — Async runtime - [tokio-cron-scheduler](https://github.com/mvniekerk/tokio-cron-scheduler) — Task scheduling - [pulldown-cmark](https://github.com/pulldown-cmark/pulldown-cmark) — Markdown parser (entity-based Telegram formatting) +- [rusqlite](https://github.com/rusqlite/rusqlite) — SQLite with FTS5 and `sqlite-vec` +- [axum](https://github.com/tokio-rs/axum) — Web server for the setup wizard +- [dirs](https://github.com/soc/dirs-rs) — OS home directory resolution +- [sha2](https://github.com/RustCrypto/hashes) — SHA-256 hashing for skill/agent update engine +- [regex](https://github.com/rust-lang/regex) — Secret redaction in supervisor artifacts > **Thanks:** Markdown-to-entities conversion approach inspired by [telegramify-markdown](https://github.com/sudoskys/telegramify-markdown) by sudoskys. diff --git a/config.example.toml b/config.example.toml index c9a0597..46a05f2 100644 --- a/config.example.toml +++ b/config.example.toml @@ -24,15 +24,24 @@ Use the available tools to help the user with their tasks. \ When using file or terminal tools, operate only within the allowed sandbox directory. \ Be concise and helpful.""" +# ── Home directory & paths ────────────────────────────────────────────── +# By default RustFox stores everything under ~/.rustfox: +# ~/.rustfox/config.toml, rustfox.db, skills/, agents/, workspace/, artifacts/ +# Override the home root with the RUSTFOX_HOME env var (absolute path) or +# [general].home (see the [general] section below). Run a second isolated +# instance with: +# RUSTFOX_HOME="$HOME/.rustfox-work" cargo run + [sandbox] -# The directory where file operations and command execution are allowed -# The bot cannot access files outside this directory -allowed_directory = "/tmp/rustfox-sandbox" +# The LLM's persistent workspace (file/command tools are confined here). +# Leave unset to use /workspace. Set an absolute path to override. +# allowed_directory = "/absolute/path/to/workspace" [memory] # Path to the SQLite database file for persistent memory # Stores conversations, knowledge base, and vector embeddings -database_path = "rustfox.db" +# Leave unset to use /rustfox.db. Set an absolute path to override. +# database_path = "/absolute/path/to/rustfox.db" # Query rewriting for memory search (optional; default: false) # When enabled, ambiguous follow-up questions are rewritten into self-contained @@ -44,9 +53,14 @@ database_path = "rustfox.db" [skills] # Directory containing skill markdown files # Skills are natural-language instructions loaded at startup -directory = "skills" +# Leave unset to use /skills (seeded from the bundled skills on first run). +# directory = "/absolute/path/to/skills" [general] +# Home directory root for all RustFox state (optional; overrides ~/.rustfox). +# Must be an absolute path. The RUSTFOX_HOME env var takes precedence over this. +# home = "/absolute/path/to/home" + # Your location, injected into the system prompt so the AI knows your timezone/region # Uncomment and set to your city/region (e.g. "Tokyo, Japan") # location = "Tokyo, Japan" diff --git a/docs/persistent-home-directory.md b/docs/persistent-home-directory.md new file mode 100644 index 0000000..377fda0 --- /dev/null +++ b/docs/persistent-home-directory.md @@ -0,0 +1,104 @@ +# Persistent Home Directory (`~/.rustfox`) + +RustFox stores all of its state under a single **home directory**, by default +`~/.rustfox`. This survives reboots, keeps secrets out of the LLM sandbox, and +makes it easy to run several isolated instances on one machine. + +## Layout + +``` +~/.rustfox/ + config.toml # secrets (bot token, API keys) — OUTSIDE the sandbox + rustfox.db # SQLite memory + embeddings — OUTSIDE the sandbox + skills/ # skills (seeded on first run, editable) + agents/ # agents (seeded on first run, editable) + workspace/ # THE SANDBOX — durable scratch space for the LLM + artifacts/ # supervisor artifacts + user_model.md # learned user model +``` + +Only `workspace/` is reachable by the file and command tools. `config.toml` +and `rustfox.db` live above it and cannot be read or written by the LLM. + +## Choosing where the home lives + +Resolution order (first match wins): + +1. `RUSTFOX_HOME` environment variable (must be an absolute path) +2. `[general].home` in `config.toml` (absolute path) +3. `~/.rustfox` (default) + +Each individual path can still be pinned independently in `config.toml` +(e.g. `[memory].database_path`). An absolute value is used verbatim; an unset +value falls back to the home default. + +## Migrating existing data + +If you previously ran RustFox from a project directory (with `./rustfox.db`, +`./skills`, etc.), RustFox will **not** move your files automatically. On +startup it prints an actionable warning for each legacy path. To migrate: + +RustFox auto-creates the home subdirectories on startup, so use `cp -rT` +(merge into the existing destination directory) rather than plain `cp -r`, +which would nest (e.g. `~/.rustfox/skills/skills`). Each command copies only +that one path — never your whole project. + +```bash +mkdir -p ~/.rustfox +cp ./rustfox.db ~/.rustfox/rustfox.db +cp -rT ./skills ~/.rustfox/skills +cp -rT ./agents ~/.rustfox/agents +cp -rT ./supervisor/artifacts ~/.rustfox/artifacts # if you used the supervisor +cp ./memory/USER.md ~/.rustfox/user_model.md # if present +# Move your old sandbox contents into the new persistent workspace: +cp -rT /tmp/rustfox-sandbox ~/.rustfox/workspace # adjust to your old sandbox +``` + +Then remove any path overrides from `config.toml` so RustFox uses the home +defaults, and place your `config.toml` at `~/.rustfox/config.toml` (or keep +passing it as the first CLI argument). + +## Keeping the old location instead + +If you prefer your current layout, pin the paths explicitly in `config.toml`: + +```toml +[sandbox] +allowed_directory = "/abs/path/to/old/sandbox" +[memory] +database_path = "/abs/path/to/rustfox.db" +[skills] +directory = "/abs/path/to/skills" +``` + +Absolute paths are always honored unchanged. + +## Starting fresh + +Doing nothing is the "start fresh" path: RustFox creates an empty +`~/.rustfox/workspace`, a new database, and seeds skills/agents from the bundled +copies. Your old project-directory files are left untouched. + +## Running multiple instances + +Give each instance its own home: + +```bash +# Default personal instance +cargo run + +# A separate work instance, fully isolated +RUSTFOX_HOME="$HOME/.rustfox-work" cargo run +``` + +Each home has independent skills, agents, workspace, database, and artifacts. + +## Updating bundled skills + +The instance `skills/` and `agents/` directories are seeded once on first run. +To pull in newer bundled versions later, send the bot `/update-skills`: + +- Skills you have not modified are overwritten with the new bundled version. +- Skills you edited locally are backed up to `/SKILL.md.bak` before being + updated, so your changes are never silently lost. +- Skills you created yourself (not part of the bundle) are never touched. diff --git a/docs/superpowers/plans/2026-05-28-telegram-plan-tool-visuals.md b/docs/superpowers/plans/2026-05-28-telegram-plan-tool-visuals.md new file mode 100644 index 0000000..e40e08b --- /dev/null +++ b/docs/superpowers/plans/2026-05-28-telegram-plan-tool-visuals.md @@ -0,0 +1,729 @@ +# Telegram Plan and Tool Visuals Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add a persistent verbose-mode Telegram audit card for plan/tool execution and remove duplicate streamed tool-status messages. + +**Architecture:** Keep final assistant answer streaming and tool progress rendering as separate paths. Add a pure notifier display model that parses planning tool arguments, renders a live checklist plus recent tool activity, and lets `ToolCallNotifier` edit its existing progress message into a completed persistent summary. Remove the agent-side code that sends formatted tool-status lines into the answer stream. + +**Tech Stack:** Rust 2021, Tokio, teloxide, serde_json, anyhow, tracing, existing RustFox agent/tool/notifier modules. + +--- + +## File Map + +- Modify: `src/platform/tool_notifier.rs` - extend `ToolEvent::Started`, add plan/tool display state, render live and completed audit text, and make notifier finish persist summaries. +- Modify: `src/agent.rs` - send raw tool arguments to the notifier and remove duplicate status streaming into `stream_token_tx`. +- Optionally modify: `src/platform/telegram.rs` - only if the notifier `finish` signature needs a mutable borrow adjustment; no behavior change should be needed. + +No database migration is needed. No `src/tools.rs` behavior change is required for this iteration. Do not commit unless the user explicitly asks for a commit. + +--- + +### Task 1: Add Pure Plan and Tool Display State + +**Files:** +- Modify: `src/platform/tool_notifier.rs` + +- [x] **Step 1: Add failing display-state tests** + +Add these tests inside the existing `#[cfg(test)] mod tests` in `src/platform/tool_notifier.rs`: + +```rust + #[test] + fn test_tool_display_state_renders_plan_create_as_checklist() { + let mut state = ToolDisplayState::default(); + + state.handle_event(ToolEvent::Started { + name: "plan_create".to_string(), + args_preview: "Create test plan".to_string(), + arguments_json: r#"{"title":"Create test plan","steps":["Gather context","Implement fix"]}"#.to_string(), + }); + + let text = state.format_live(); + assert!(text.contains("Working on your request"), "live header missing: {text}"); + assert!(text.contains("Plan"), "plan section missing: {text}"); + assert!(text.contains("Create test plan"), "plan title missing: {text}"); + assert!(text.contains("[ ] 0. Gather context"), "first step missing: {text}"); + assert!(text.contains("[ ] 1. Implement fix"), "second step missing: {text}"); + } + + #[test] + fn test_tool_display_state_updates_plan_step_status() { + let mut state = ToolDisplayState::default(); + + state.handle_event(ToolEvent::Started { + name: "plan_create".to_string(), + args_preview: "Plan".to_string(), + arguments_json: r#"{"title":"Plan","steps":["First","Second"]}"#.to_string(), + }); + state.handle_event(ToolEvent::Started { + name: "plan_update".to_string(), + args_preview: "step 1".to_string(), + arguments_json: r#"{"step_id":1,"status":"in_progress","notes":"working"}"#.to_string(), + }); + + let text = state.format_live(); + assert!(text.contains("[ ] 0. First"), "unchanged step missing: {text}"); + assert!(text.contains("[>] 1. Second -- working"), "updated step missing: {text}"); + } + + #[test] + fn test_tool_display_state_marks_failed_plan_update_after_completion() { + let mut state = ToolDisplayState::default(); + + state.handle_event(ToolEvent::Started { + name: "plan_create".to_string(), + args_preview: "Plan".to_string(), + arguments_json: r#"{"title":"Plan","steps":["Only step"]}"#.to_string(), + }); + state.handle_event(ToolEvent::Started { + name: "plan_update".to_string(), + args_preview: "step 0".to_string(), + arguments_json: r#"{"step_id":0,"status":"in_progress"}"#.to_string(), + }); + state.handle_event(ToolEvent::Completed { + name: "plan_update".to_string(), + success: false, + }); + + let text = state.format_live(); + assert!(text.contains("[!] 0. Only step"), "failed step missing: {text}"); + } + + #[test] + fn test_tool_display_state_renders_generic_tool_activity_without_plan() { + let mut state = ToolDisplayState::default(); + + state.handle_event(ToolEvent::Started { + name: "read_file".to_string(), + args_preview: "/tmp/file.txt".to_string(), + arguments_json: r#"{"path":"/tmp/file.txt"}"#.to_string(), + }); + state.handle_event(ToolEvent::Completed { + name: "read_file".to_string(), + success: true, + }); + + let text = state.format_completed(); + assert!(text.contains("Completed"), "completed header missing: {text}"); + assert!(text.contains("Tool activity"), "tool section missing: {text}"); + assert!(text.contains("Reading a file"), "friendly tool label missing: {text}"); + assert!(text.contains("completed"), "completion state missing: {text}"); + assert!(!text.contains("Plan\n"), "plan section should be omitted: {text}"); + } +``` + +- [x] **Step 2: Run the new tests and verify they fail** + +Run: + +```bash +cargo test platform::tool_notifier::tests::test_tool_display_state -- --nocapture +``` + +Expected: fail to compile because `ToolDisplayState` does not exist and `ToolEvent::Started` has no `arguments_json` field. + +- [x] **Step 3: Implement the display model** + +In `src/platform/tool_notifier.rs`, replace the `ToolEvent::Started` variant with this shape: + +```rust + Started { + name: String, + /// Short safe preview for display. + args_preview: String, + /// Full tool arguments for notifier-side parsing. Never render directly. + arguments_json: String, + }, +``` + +Add these display-model types below `ToolEvent`: + +```rust +#[derive(Debug, Clone, PartialEq, Eq)] +enum PlanStepStatus { + Todo, + InProgress, + Done, + Failed, +} + +impl PlanStepStatus { + fn from_tool_status(status: &str) -> Self { + match status { + "done" => Self::Done, + "failed" => Self::Failed, + "in_progress" => Self::InProgress, + _ => Self::Todo, + } + } + + fn marker(&self) -> &'static str { + match self { + Self::Todo => "[ ]", + Self::InProgress => "[>]", + Self::Done => "[x]", + Self::Failed => "[!]", + } + } + +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct PlanStepDisplay { + id: usize, + description: String, + status: PlanStepStatus, + notes: String, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct PlanDisplay { + title: String, + steps: Vec, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct ToolActivity { + name: String, + args_preview: String, + done: bool, + success: bool, +} + +#[derive(Debug, Clone, Default)] +struct ToolDisplayState { + plan: Option, + active_tool: Option, + recent_tools: Vec, + last_plan_update_step: Option, +} +``` + +Add this implementation below the types: + +```rust +impl ToolDisplayState { + const MAX_RECENT_TOOLS: usize = 12; + + fn handle_event(&mut self, event: ToolEvent) { + match event { + ToolEvent::Started { + name, + args_preview, + arguments_json, + } => { + self.apply_started(name, args_preview, &arguments_json); + } + ToolEvent::Completed { name, success } => { + self.apply_completed(&name, success); + } + } + } + + fn has_activity(&self) -> bool { + self.plan.is_some() || self.active_tool.is_some() || !self.recent_tools.is_empty() + } + + fn apply_started(&mut self, name: String, args_preview: String, arguments_json: &str) { + match name.as_str() { + "plan_create" => { + if let Some(plan) = parse_plan_create(arguments_json) { + self.plan = Some(plan); + } + self.push_tool(ToolActivity { + name, + args_preview, + done: false, + success: true, + }); + } + "plan_update" => { + self.last_plan_update_step = parse_plan_update(arguments_json) + .and_then(|update| self.apply_plan_update(update)); + self.push_tool(ToolActivity { + name, + args_preview, + done: false, + success: true, + }); + } + _ => { + self.push_tool(ToolActivity { + name, + args_preview, + done: false, + success: true, + }); + } + } + } + + fn apply_completed(&mut self, name: &str, success: bool) { + if let Some(entry) = self + .recent_tools + .iter_mut() + .rfind(|entry| entry.name == name && !entry.done) + { + entry.done = true; + entry.success = success; + } + + if self + .active_tool + .as_ref() + .map(|tool| tool.name.as_str() == name) + .unwrap_or(false) + { + self.active_tool = None; + } + + if name == "plan_update" && !success { + if let (Some(plan), Some(step_id)) = (&mut self.plan, self.last_plan_update_step) { + if let Some(step) = plan.steps.iter_mut().find(|step| step.id == step_id) { + step.status = PlanStepStatus::Failed; + } + } + } + } + + fn apply_plan_update(&mut self, update: PlanStepUpdate) -> Option { + let plan = self.plan.as_mut()?; + let step = plan.steps.iter_mut().find(|step| step.id == update.step_id)?; + step.status = update.status; + step.notes = update.notes; + Some(update.step_id) + } + + fn push_tool(&mut self, activity: ToolActivity) { + self.active_tool = Some(activity.clone()); + self.recent_tools.push(activity); + if self.recent_tools.len() > Self::MAX_RECENT_TOOLS { + let overflow = self.recent_tools.len() - Self::MAX_RECENT_TOOLS; + self.recent_tools.drain(0..overflow); + } + } + + fn format_live(&self) -> String { + self.format_with_header("Working on your request", false) + } + + fn format_completed(&self) -> String { + self.format_with_header("Completed", true) + } + + fn format_with_header(&self, header: &str, completed: bool) -> String { + let mut text = String::from(header); + + if let Some(plan) = &self.plan { + text.push_str("\n\nPlan"); + if !plan.title.is_empty() { + text.push_str(&format!("\n{}", plan.title)); + } + for step in &plan.steps { + text.push_str(&format!( + "\n{} {}. {}", + step.status.marker(), + step.id, + step.description + )); + if !step.notes.is_empty() { + text.push_str(&format!(" -- {}", step.notes)); + } + } + } + + if !completed { + if let Some(active) = &self.active_tool { + text.push_str("\n\nCurrent"); + text.push_str(&format!("\nRunning: {}", friendly_tool_name(&active.name))); + } + } + + if !self.recent_tools.is_empty() { + text.push_str("\n\nTool activity"); + for tool in &self.recent_tools { + let state = if tool.done { + if tool.success { "completed" } else { "failed" } + } else { + "running" + }; + let label = friendly_tool_name(&tool.name); + if tool.args_preview.is_empty() { + text.push_str(&format!("\n- {}: {}", label, state)); + } else { + text.push_str(&format!("\n- {}: {} ({})", label, state, tool.args_preview)); + } + } + } + + if completed { + text.push_str("\n\nResult\nFinal answer sent below."); + } + + text + } +} + +struct PlanStepUpdate { + step_id: usize, + status: PlanStepStatus, + notes: String, +} + +fn parse_plan_create(arguments_json: &str) -> Option { + let value: serde_json::Value = serde_json::from_str(arguments_json).ok()?; + let title = value + .get("title") + .and_then(|v| v.as_str()) + .unwrap_or("Plan") + .to_string(); + let steps = value.get("steps")?.as_array()?; + let steps = steps + .iter() + .enumerate() + .map(|(id, step)| PlanStepDisplay { + id, + description: step.as_str().unwrap_or("").to_string(), + status: PlanStepStatus::Todo, + notes: String::new(), + }) + .collect(); + + Some(PlanDisplay { title, steps }) +} + +fn parse_plan_update(arguments_json: &str) -> Option { + let value: serde_json::Value = serde_json::from_str(arguments_json).ok()?; + let step_id = value.get("step_id")?.as_u64()? as usize; + let status = value + .get("status") + .and_then(|v| v.as_str()) + .map(PlanStepStatus::from_tool_status) + .unwrap_or(PlanStepStatus::Todo); + let notes = value + .get("notes") + .and_then(|v| v.as_str()) + .unwrap_or("") + .to_string(); + + Some(PlanStepUpdate { + step_id, + status, + notes, + }) +} +``` + +- [x] **Step 4: Run the display-state tests and verify they pass** + +Run: + +```bash +cargo test platform::tool_notifier::tests::test_tool_display_state -- --nocapture +``` + +Expected: all four `test_tool_display_state_*` tests pass. + +--- + +### Task 2: Wire `ToolCallNotifier` to the Display State and Persist Completion + +**Files:** +- Modify: `src/platform/tool_notifier.rs` +- Optionally modify: `src/platform/telegram.rs` + +- [x] **Step 1: Add failing notifier finish tests** + +Add these tests inside the existing `#[cfg(test)] mod tests` in `src/platform/tool_notifier.rs`: + +```rust + #[test] + fn test_notifier_final_status_text_returns_completed_card_when_activity_exists() { + let mut notifier = ToolCallNotifier::new(Bot::new("TEST_TOKEN"), ChatId(1)); + notifier.display_state.handle_event(ToolEvent::Started { + name: "read_file".to_string(), + args_preview: "/tmp/file.txt".to_string(), + arguments_json: r#"{"path":"/tmp/file.txt"}"#.to_string(), + }); + notifier.display_state.handle_event(ToolEvent::Completed { + name: "read_file".to_string(), + success: true, + }); + + let text = notifier + .final_status_text() + .expect("activity should produce a final status card"); + assert!(text.contains("Completed"), "completed header missing: {text}"); + assert!(text.contains("Final answer sent below."), "result line missing: {text}"); + assert!(text.contains("Reading a file"), "tool activity missing: {text}"); + } + + #[test] + fn test_notifier_final_status_text_is_none_without_activity() { + let notifier = ToolCallNotifier::new(Bot::new("TEST_TOKEN"), ChatId(1)); + assert!(notifier.final_status_text().is_none()); + } +``` + +- [x] **Step 2: Run the new notifier tests and verify they fail** + +Run: + +```bash +cargo test platform::tool_notifier::tests::test_notifier_final_status_text -- --nocapture +``` + +Expected: fail to compile because `ToolCallNotifier` has no `display_state` field and no `final_status_text()` method. + +- [x] **Step 3: Replace notifier log storage with display state** + +In `ToolCallNotifier`, replace the `tool_log` field with: + +```rust + display_state: ToolDisplayState, +``` + +Update `ToolCallNotifier::new()`: + +```rust + display_state: ToolDisplayState::default(), +``` + +Replace `handle_event()` with: + +```rust + pub async fn handle_event(&mut self, event: ToolEvent) { + self.display_state.handle_event(event); + self.edit_message().await; + } +``` + +Replace `format_status()` with: + +```rust + fn format_status(&self) -> String { + self.display_state.format_live() + } +``` + +Add this helper near `format_status()`: + +```rust + fn final_status_text(&self) -> Option { + if self.display_state.has_activity() { + Some(self.display_state.format_completed()) + } else { + None + } + } +``` + +Replace `finish()` with: + +```rust + pub async fn finish(&mut self) { + let Some(ref msg) = self.status_msg else { + return; + }; + + let Some(text) = self.final_status_text() else { + self.bot.delete_message(self.chat_id, msg.id).await.ok(); + return; + }; + + match self + .bot + .edit_message_text(self.chat_id, msg.id, &text) + .await + { + Ok(_) => self.last_edit = Some(Instant::now()), + Err(e) => debug!("Failed to edit final tool notifier message: {:#}", e), + } + } +``` + +Remove or rewrite the old `format_final()` helper and tests that depend on direct `tool_log` access. Keep the existing `format_args_preview()` and `friendly_tool_name()` tests. + +If the compiler reports that `notifier.finish().await` requires a mutable borrow in `src/platform/telegram.rs`, update only that call site inside the spawned task to keep using the existing mutable `notifier` binding: + +```rust + notifier.finish().await; +``` + +No larger Telegram streaming changes are expected in this task. + +- [x] **Step 4: Run notifier tests and verify they pass** + +Run: + +```bash +cargo test platform::tool_notifier -- --nocapture +``` + +Expected: all notifier tests pass. + +--- + +### Task 3: Stop Streaming Tool Status Into the Assistant Answer + +**Files:** +- Modify: `src/agent.rs` +- Modify: `src/platform/tool_notifier.rs` only if compile errors reveal missed `ToolEvent::Started` construction sites. + +- [x] **Step 1: Add a failing source-inspection regression test** + +Add this test inside the existing `#[cfg(test)] mod tests` in `src/agent.rs`: + +```rust + #[test] + fn test_tool_status_is_not_streamed_to_answer_channel() { + let source = include_str!("agent.rs"); + let status_line_call = ["format_tool_status", "_line("].concat(); + let stream_status_var = ["stream", "_status_tx"].concat(); + + assert!( + !source.contains(&status_line_call), + "agent.rs must not format tool-status lines for the assistant answer stream" + ); + assert!( + !source.contains(&stream_status_var), + "agent.rs must not clone a separate stream-status sender for tool progress" + ); + } +``` + +- [x] **Step 2: Run the regression test and verify it fails** + +Run: + +```bash +cargo test agent::tests::test_tool_status_is_not_streamed_to_answer_channel -- --nocapture +``` + +Expected: fail because `src/agent.rs` still contains `stream_status_tx` and calls `format_tool_status_line()`. + +- [x] **Step 3: Remove the duplicate stream-status path and send raw arguments to the notifier** + +In `src/agent.rs`, remove this block near the start of the agent loop: + +```rust + // Clone the stream sender so tool status can be pushed into the same Telegram + // message during tool execution, before the final response starts streaming. + let stream_status_tx = stream_token_tx.clone(); +``` + +In the tool-start notification block, replace the `ToolEvent::Started` construction with: + +```rust + if let Some(ref tx) = tool_event_tx { + let _ = + tx.try_send(crate::platform::tool_notifier::ToolEvent::Started { + name: tool_call.function.name.clone(), + args_preview: args_preview.clone(), + arguments_json: tool_call.function.arguments.clone(), + }); + } +``` + +Remove the entire block that streams formatted tool status into `stream_status_tx`: + +```rust + // Stream tool status into the Telegram message only when + // tool-progress notifications are enabled, to avoid + // prepending status lines to otherwise silent/final output. + if tool_event_tx.is_some() { + if let Some(ref tx) = stream_status_tx { + let status = + crate::platform::tool_notifier::format_tool_status_line( + &tool_call.function.name, + &args_preview, + ); + tx.try_send(status).ok(); + } + } +``` + +- [x] **Step 4: Run the agent regression test and targeted notifier tests** + +Run: + +```bash +cargo test agent::tests::test_tool_status_is_not_streamed_to_answer_channel -- --nocapture +cargo test platform::tool_notifier -- --nocapture +``` + +Expected: both commands pass. + +--- + +### Task 4: Verify End-to-End Behavior and Clean Up Tests + +**Files:** +- Modify: `src/platform/tool_notifier.rs` if legacy tests still assume deleted summaries or old `tool_log` structure. +- Modify: `src/agent.rs` only if the regression test needs a more precise source-inspection needle. + +- [x] **Step 1: Run formatting** + +Run: + +```bash +cargo fmt --all -- --check +``` + +Expected: pass. If it fails, run `cargo fmt --all`, then run the check again. + +- [x] **Step 2: Run the full test suite** + +Run: + +```bash +cargo test +``` + +Expected: pass. If any failure is unrelated to the notifier/agent changes, record the failing test name and reason before deciding whether to touch it. + +- [x] **Step 3: Run clippy** + +Run: + +```bash +cargo clippy -- -D warnings +``` + +Expected: pass with no warnings. + +- [x] **Step 4: Manual behavior review without Telegram network calls** + +Inspect the final code paths and confirm these statements are true: + +```text +- Verbose mode still creates a ToolCallNotifier in src/platform/telegram.rs. +- Agent tool execution still sends ToolEvent::Started and ToolEvent::Completed. +- ToolEvent::Started includes raw arguments_json for parser use. +- src/agent.rs does not send format_tool_status_line output into stream_token_tx. +- ToolCallNotifier::finish edits the existing status message into a completed card when activity exists. +- Non-verbose mode still uses the existing Thinking placeholder and final answer stream. +``` + +- [x] **Step 5: Update implementation notes in the final response** + +When reporting completion, mention: + +```text +Implemented persistent verbose audit cards for plan/tool progress. +Fixed duplicate lingering tool messages by removing agent-side tool-status streaming. +Verified with cargo fmt, cargo test, and cargo clippy. +``` + +If any verification command could not be run, state that clearly with the reason. + +--- + +## Self-Review Checklist + +- Spec coverage: persistent verbose audit card, planning checklist display, generic tool activity, separate final answer stream, non-verbose unchanged, and duplicate-message root cause are covered by Tasks 1-4. +- Test coverage: formatter logic is unit-tested, final card text is unit-tested, and the duplicate stream path is guarded by an agent source-inspection regression test. +- Type consistency: `ToolEvent::Started` consistently carries `name`, `args_preview`, and `arguments_json`; `ToolDisplayState` owns all plan/tool rendering state; `ToolCallNotifier` owns Telegram message editing only. +- Repo policy: this plan does not include git commit steps because commits require explicit user instruction in this environment. diff --git a/docs/superpowers/plans/2026-05-29-persistent-home-directory.md b/docs/superpowers/plans/2026-05-29-persistent-home-directory.md new file mode 100644 index 0000000..4e9994e --- /dev/null +++ b/docs/superpowers/plans/2026-05-29-persistent-home-directory.md @@ -0,0 +1,2052 @@ +# Persistent Home Directory (`~/.rustfox`) Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Give each RustFox instance a single persistent home directory (default `~/.rustfox`) that holds config, DB, skills, agents, artifacts, and a durable `workspace/` sandbox, with env/config overrides and a seed-plus-update flow for bundled skills. + +**Architecture:** A new pure-logic module `src/home.rs` resolves the home root (env → config → `~/.rustfox`) and every data path (unset → home default; absolute → verbatim; relative → CWD legacy + warning). `Config::resolve()` calls it, creates directories, and writes the resolved **absolute** paths back into the existing config fields so all downstream consumers (`main.rs`, `agent.rs`, `tools.rs`, `learning.rs`) keep reading the same fields unchanged. Skills/agents are seed-copied into the home on first run and refreshed by an explicit `/update-skills` command that diffs content hashes recorded in a home-side `skills-lock.json`. + +**Tech Stack:** Rust (edition 2021), Tokio, serde/toml, `dirs` crate (new), `sha2` (already present), `tempfile` (dev). Telegram via teloxide. + +--- + +## File Structure + +- **Create** `src/home.rs` — home-root + per-path resolution, `ResolvedPaths`, `PathOrigin`, `LegacyPathWarning`, `ensure_dirs`. Pure and unit-testable. +- **Create** `src/skills/seed.rs` — first-run seed-copy of bundled skills/agents + home-side lock writer + skill content hashing. +- **Create** `src/skills/update.rs` — `/update-skills` hash-diff engine (`UpdateReport`). +- **Create** `docs/persistent-home-directory.md` — migration + multi-instance tutorial. +- **Modify** `Cargo.toml` — add `dirs` dependency. +- **Modify** `src/lib.rs` — register `pub mod home;`. +- **Modify** `src/skills/mod.rs` — register `pub mod seed;` and `pub mod update;`. +- **Modify** `src/config.rs` — empty-sentinel path defaults, `GeneralConfig.home`, `resolved_home` field, `Config::resolve()`, wire into `Config::load`. +- **Modify** `src/main.rs` — config-file discovery fallback, seed call, startup log of home. +- **Modify** `src/agent.rs` — add `reload_skills_and_agents()` method; update system prompt sandbox text. +- **Modify** `src/platform/telegram.rs` — `/update-skills` command + `/start` help text. +- **Modify** `config.example.toml` — document optional paths, `RUSTFOX_HOME`, `[general].home`. +- **Modify** `CLAUDE.md` / `README.md` — note the home model. + +**Naming decision (resolved from spec open question):** the module is `src/home.rs`. + +--- + +### Task 1: Add the `dirs` dependency + +**Files:** +- Modify: `Cargo.toml` + +- [ ] **Step 1: Add the crate** + +In `Cargo.toml`, under `[dependencies]`, after the `regex = "1"` line, add: + +```toml +# OS home-directory resolution for the persistent home dir (~/.rustfox) +dirs = "5" +``` + +- [ ] **Step 2: Verify it resolves** + +Run: `cargo build` +Expected: builds successfully; `dirs` appears in `Cargo.lock`. + +- [ ] **Step 3: Commit** + +```bash +git add Cargo.toml Cargo.lock +git commit -m "build: add dirs crate for home directory resolution" +``` + +--- + +### Task 2: Create `src/home.rs` with `resolve_home` + +**Files:** +- Create: `src/home.rs` +- Modify: `src/lib.rs` + +- [ ] **Step 1: Register the module** + +In `src/lib.rs`, add `pub mod home;` in alphabetical position (after `pub mod config;`): + +```rust +pub mod agent; +pub mod agent_prompt; +pub mod config; +pub mod home; +pub mod langsmith; +``` + +- [ ] **Step 2: Write the failing test** + +Create `src/home.rs` with only this content: + +```rust +use anyhow::{anyhow, Result}; +use std::path::{Path, PathBuf}; + +/// Resolve the home root from the override sources, in priority order: +/// 1. `RUSTFOX_HOME` env var (must be absolute) +/// 2. `[general].home` config value (must be absolute) +/// 3. `/.rustfox` +pub fn resolve_home( + env_home: Option<&str>, + config_home: Option<&Path>, + os_home: Option<&Path>, +) -> Result { + todo!() +} + +#[cfg(test)] +mod tests { + use super::*; + use std::path::Path; + + #[test] + fn env_takes_priority_when_absolute() { + let got = resolve_home( + Some("/srv/rfx"), + Some(Path::new("/etc/rfx")), + Some(Path::new("/home/u")), + ) + .unwrap(); + assert_eq!(got, PathBuf::from("/srv/rfx")); + } + + #[test] + fn relative_env_is_ignored_falls_to_config() { + let got = resolve_home( + Some("rel/dir"), + Some(Path::new("/etc/rfx")), + Some(Path::new("/home/u")), + ) + .unwrap(); + assert_eq!(got, PathBuf::from("/etc/rfx")); + } + + #[test] + fn relative_config_is_ignored_falls_to_default() { + let got = resolve_home(None, Some(Path::new("rel")), Some(Path::new("/home/u"))).unwrap(); + assert_eq!(got, PathBuf::from("/home/u/.rustfox")); + } + + #[test] + fn default_is_os_home_dot_rustfox() { + let got = resolve_home(None, None, Some(Path::new("/home/u"))).unwrap(); + assert_eq!(got, PathBuf::from("/home/u/.rustfox")); + } + + #[test] + fn errors_when_no_os_home_and_no_overrides() { + assert!(resolve_home(None, None, None).is_err()); + } +} +``` + +- [ ] **Step 3: Run test to verify it fails** + +Run: `cargo test --lib home::tests` +Expected: FAIL — panics with `not yet implemented` (the `todo!()`). + +- [ ] **Step 4: Implement `resolve_home`** + +Replace the `todo!()` body: + +```rust +pub fn resolve_home( + env_home: Option<&str>, + config_home: Option<&Path>, + os_home: Option<&Path>, +) -> Result { + if let Some(env) = env_home { + let p = Path::new(env); + if p.is_absolute() { + return Ok(p.to_path_buf()); + } + tracing::warn!("RUSTFOX_HOME='{env}' is not absolute; ignoring it"); + } + if let Some(cfg) = config_home { + if cfg.is_absolute() { + return Ok(cfg.to_path_buf()); + } + tracing::warn!( + "[general].home='{}' is not absolute; ignoring it", + cfg.display() + ); + } + let home = os_home.ok_or_else(|| { + anyhow!("Could not determine the OS home directory; set RUSTFOX_HOME or [general].home") + })?; + Ok(home.join(".rustfox")) +} +``` + +- [ ] **Step 5: Run test to verify it passes** + +Run: `cargo test --lib home::tests` +Expected: PASS (5 tests). + +- [ ] **Step 6: Commit** + +```bash +git add src/home.rs src/lib.rs +git commit -m "feat(home): add resolve_home with env/config/default precedence" +``` + +--- + +### Task 2.5: A note on the empty-PathBuf "unset" sentinel + +Subsequent tasks treat an **empty `PathBuf`** (`PathBuf::new()`, i.e. `as_os_str().is_empty()`) as "the user did not configure this path." Config struct fields use `#[serde(default)]` so a missing TOML key deserializes to an empty `PathBuf`. This lets the existing fields stay `PathBuf` (not `Option`), so no downstream consumer changes. Keep this invariant in mind for Tasks 3, 6, and 7. + +--- + +### Task 3: Add `PathOrigin` and `resolve_data_path` + +**Files:** +- Modify: `src/home.rs` + +- [ ] **Step 1: Write the failing test** + +Add to `src/home.rs` above the `#[cfg(test)]` module (the type + function), and add the test cases inside `mod tests`: + +Type + function (place after `resolve_home`): + +```rust +/// How a single configured data path was resolved (drives warning output). +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum PathOrigin { + /// Field unset (empty) → resolved under the home root. + Default, + /// Field set to an absolute path → used verbatim. + Absolute, + /// Field set to a relative path → resolved from CWD (legacy behavior). + RelativeLegacy, +} + +/// Resolve one data path. An empty `configured` path means "unset". +pub fn resolve_data_path( + configured: &Path, + home: &Path, + default_subpath: &str, +) -> (PathBuf, PathOrigin) { + todo!() +} +``` + +Tests (inside `mod tests`): + +```rust + #[test] + fn unset_path_resolves_under_home() { + let (p, o) = resolve_data_path(Path::new(""), Path::new("/h/.rustfox"), "workspace"); + assert_eq!(p, PathBuf::from("/h/.rustfox/workspace")); + assert_eq!(o, PathOrigin::Default); + } + + #[test] + fn absolute_path_used_verbatim() { + let (p, o) = resolve_data_path(Path::new("/data/wp"), Path::new("/h/.rustfox"), "workspace"); + assert_eq!(p, PathBuf::from("/data/wp")); + assert_eq!(o, PathOrigin::Absolute); + } + + #[test] + fn relative_path_is_legacy() { + let (p, o) = resolve_data_path(Path::new("skills"), Path::new("/h/.rustfox"), "skills"); + assert_eq!(p, PathBuf::from("skills")); + assert_eq!(o, PathOrigin::RelativeLegacy); + } +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cargo test --lib home::tests` +Expected: FAIL — `not yet implemented`. + +- [ ] **Step 3: Implement `resolve_data_path`** + +Replace the `todo!()`: + +```rust +pub fn resolve_data_path( + configured: &Path, + home: &Path, + default_subpath: &str, +) -> (PathBuf, PathOrigin) { + if configured.as_os_str().is_empty() { + (home.join(default_subpath), PathOrigin::Default) + } else if configured.is_absolute() { + (configured.to_path_buf(), PathOrigin::Absolute) + } else { + (configured.to_path_buf(), PathOrigin::RelativeLegacy) + } +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `cargo test --lib home::tests` +Expected: PASS (8 tests total). + +- [ ] **Step 5: Commit** + +```bash +git add src/home.rs +git commit -m "feat(home): add resolve_data_path with origin classification" +``` + +--- + +### Task 4: Add `ResolvedPaths` and `ensure_dirs` + +**Files:** +- Modify: `src/home.rs` + +- [ ] **Step 1: Write the failing test** + +Add the struct + function after `resolve_data_path`: + +```rust +/// The fully-resolved absolute paths an instance operates on. +#[derive(Debug, Clone)] +pub struct ResolvedPaths { + pub home: PathBuf, + pub workspace: PathBuf, + pub database: PathBuf, + pub skills: PathBuf, + pub agents: PathBuf, + pub artifacts: PathBuf, + pub user_model: PathBuf, +} + +/// Create the home root and standard subdirectories (0700 on Unix), plus the +/// parent directories of the database and user-model files. +pub fn ensure_dirs(paths: &ResolvedPaths) -> Result<()> { + todo!() +} +``` + +Add the test inside `mod tests` (note: requires the `tempfile` dev-dependency, already present): + +```rust + #[test] + fn ensure_dirs_creates_full_tree() { + let tmp = tempfile::tempdir().unwrap(); + let home = tmp.path().join(".rustfox"); + let paths = ResolvedPaths { + home: home.clone(), + workspace: home.join("workspace"), + database: home.join("rustfox.db"), + skills: home.join("skills"), + agents: home.join("agents"), + artifacts: home.join("artifacts"), + user_model: home.join("user_model.md"), + }; + ensure_dirs(&paths).unwrap(); + assert!(paths.home.is_dir()); + assert!(paths.workspace.is_dir()); + assert!(paths.skills.is_dir()); + assert!(paths.agents.is_dir()); + assert!(paths.artifacts.is_dir()); + // db + user_model are files, but their parent dirs must exist + assert!(paths.database.parent().unwrap().is_dir()); + assert!(paths.user_model.parent().unwrap().is_dir()); + } +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cargo test --lib home::tests::ensure_dirs_creates_full_tree` +Expected: FAIL — `not yet implemented`. + +- [ ] **Step 3: Implement `ensure_dirs`** + +Replace the `todo!()`: + +```rust +pub fn ensure_dirs(paths: &ResolvedPaths) -> Result<()> { + use anyhow::Context; + for dir in [ + &paths.home, + &paths.workspace, + &paths.skills, + &paths.agents, + &paths.artifacts, + ] { + std::fs::create_dir_all(dir) + .with_context(|| format!("Failed to create directory: {}", dir.display()))?; + } + for file in [&paths.database, &paths.user_model] { + if let Some(parent) = file.parent() { + if !parent.as_os_str().is_empty() { + std::fs::create_dir_all(parent).with_context(|| { + format!("Failed to create directory: {}", parent.display()) + })?; + } + } + } + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let _ = std::fs::set_permissions(&paths.home, std::fs::Permissions::from_mode(0o700)); + } + Ok(()) +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `cargo test --lib home::tests::ensure_dirs_creates_full_tree` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add src/home.rs +git commit -m "feat(home): add ResolvedPaths and ensure_dirs (0700)" +``` + +--- + +### Task 5: Add `LegacyPathWarning` and its `render()` + +**Files:** +- Modify: `src/home.rs` + +- [ ] **Step 1: Write the failing test** + +Add after `ensure_dirs`: + +```rust +/// An actionable warning emitted when a path is relative (CWD-resolved) or when +/// legacy data is detected in the launch directory while the path is unset. +#[derive(Debug, Clone)] +pub struct LegacyPathWarning { + /// Config field label, e.g. "memory.database_path". + pub label: String, + /// The path currently in effect (CWD-resolved legacy location). + pub current: PathBuf, + /// Where this path would live under the home root. + pub home_default: PathBuf, +} + +impl LegacyPathWarning { + /// Multi-line, copy-pasteable migration hint. + pub fn render(&self) -> String { + todo!() + } +} +``` + +Test inside `mod tests`: + +```rust + #[test] + fn warning_render_includes_paths_and_commands() { + let w = LegacyPathWarning { + label: "memory.database_path".to_string(), + current: PathBuf::from("/work/rustfox.db"), + home_default: PathBuf::from("/h/.rustfox/rustfox.db"), + }; + let s = w.render(); + assert!(s.contains("memory.database_path")); + assert!(s.contains("/work/rustfox.db")); + assert!(s.contains("/h/.rustfox/rustfox.db")); + assert!(s.contains("cp -rT")); + } +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cargo test --lib home::tests::warning_render_includes_paths_and_commands` +Expected: FAIL — `not yet implemented`. + +- [ ] **Step 3: Implement `render`** + +Replace the `todo!()`: + +```rust + pub fn render(&self) -> String { + // `cp -rT` merges the source into the already-created destination + // directory instead of nesting it (e.g. avoids /skills/skills). + format!( + "Legacy path in use for `{label}`:\n \ + current : {current}\n \ + new home default : {home_default}\n \ + To migrate this path into the home directory:\n \ + cp -rT {current} {home_default}\n \ + To keep the current location, pin it in config.toml under its section.", + label = self.label, + current = self.current.display(), + home_default = self.home_default.display(), + ) + } +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `cargo test --lib home::tests::warning_render_includes_paths_and_commands` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add src/home.rs +git commit -m "feat(home): add LegacyPathWarning with actionable render()" +``` + +--- + +### Task 6: Config — optional path fields, `[general].home`, `resolved_home` + +**Files:** +- Modify: `src/config.rs` + +This task only changes serde defaults and adds fields. No behavior yet (resolution is Task 7). After this task the project must still compile and existing tests pass. + +- [ ] **Step 1: Make `SandboxConfig.allowed_directory` optional via empty sentinel** + +In `src/config.rs`, change: + +```rust +#[derive(Debug, Deserialize, Clone)] +pub struct SandboxConfig { + pub allowed_directory: PathBuf, +} +``` + +to: + +```rust +#[derive(Debug, Deserialize, Clone)] +pub struct SandboxConfig { + #[serde(default)] + pub allowed_directory: PathBuf, +} +``` + +- [ ] **Step 2: Switch the path fields to the empty sentinel** + +Change these field attributes from their `default = "..."` form to plain `#[serde(default)]`: + +In `MemoryConfig`: +```rust + #[serde(default)] + pub database_path: PathBuf, +``` + +In `SkillsConfig`: +```rust +#[derive(Debug, Deserialize, Clone)] +pub struct SkillsConfig { + #[serde(default)] + pub directory: PathBuf, +} +``` + +In `AgentsConfig`: +```rust +#[derive(Debug, Deserialize, Clone)] +pub struct AgentsConfig { + #[serde(default)] + pub directory: PathBuf, +} +``` + +In `LearningConfig`: +```rust + #[serde(default)] + pub user_model_path: PathBuf, +``` + +In `SupervisorConfig`: +```rust + #[serde(default)] + pub artifacts_dir: std::path::PathBuf, +``` + +- [ ] **Step 3: Update the section-builder defaults to empty sentinels** + +So that a wholly-missing section also yields the unset sentinel, change the builder functions: + +`default_memory_config()` — change `database_path: default_db_path(),` to: +```rust + database_path: PathBuf::new(), +``` + +`default_skills_config()`: +```rust +fn default_skills_config() -> SkillsConfig { + SkillsConfig { + directory: PathBuf::new(), + } +} +``` + +`default_agents_config()`: +```rust +fn default_agents_config() -> AgentsConfig { + AgentsConfig { + directory: PathBuf::new(), + } +} +``` + +`default_learning_config()` — change `user_model_path: default_user_model_path(),` to: +```rust + user_model_path: PathBuf::new(), +``` + +`SupervisorConfig::default()` and `default_artifacts_dir()` — change `default_artifacts_dir()` to return an empty path: +```rust +fn default_artifacts_dir() -> std::path::PathBuf { + std::path::PathBuf::new() +} +``` + +- [ ] **Step 4: Delete now-unused default helpers** + +Remove `fn default_db_path()`, `fn default_skills_dir()`, `fn default_agents_dir()`, and `fn default_user_model_path()` (they are no longer referenced after Step 3). If the compiler reports any are still referenced, leave that one in place. + +- [ ] **Step 5: Add `home` to `GeneralConfig`** + +Change: +```rust +#[derive(Debug, Deserialize, Clone, Default)] +pub struct GeneralConfig { + /// Optional location string injected into the system prompt (e.g. "Tokyo, Japan") + #[serde(default)] + pub location: Option, +} +``` +to: +```rust +#[derive(Debug, Deserialize, Clone, Default)] +pub struct GeneralConfig { + /// Optional location string injected into the system prompt (e.g. "Tokyo, Japan") + #[serde(default)] + pub location: Option, + /// Optional absolute path overriding the default `~/.rustfox` home root. + #[serde(default)] + pub home: Option, +} +``` + +- [ ] **Step 6: Add a non-serialized `resolved_home` field to `Config`** + +In the `Config` struct, after the `supervisor` field, add: +```rust + /// Absolute home root resolved at load time (not read from TOML). + #[serde(skip)] + pub resolved_home: Option, +``` + +- [ ] **Step 7: Verify it compiles and existing tests pass** + +Run: `cargo test --lib config::tests` +Expected: PASS (the existing config tests set `allowed_directory = "/tmp"`; with the new optional default they still deserialize). Run `cargo build` to confirm no unused-function errors. + +- [ ] **Step 8: Commit** + +```bash +git add src/config.rs +git commit -m "refactor(config): make data paths optional + add general.home and resolved_home" +``` + +--- + +### Task 7: Implement `Config::resolve()` + +**Files:** +- Modify: `src/config.rs` + +- [ ] **Step 1: Write the failing test** + +Add to the `#[cfg(test)] mod tests` block in `src/config.rs`: + +```rust + fn base_toml() -> &'static str { + r#" + [telegram] + bot_token = "tok" + allowed_user_ids = [1] + [openrouter] + api_key = "key" + "# + } + + #[test] + fn resolve_fills_unset_paths_under_home() { + let tmp = tempfile::tempdir().unwrap(); + let home = tmp.path().join(".rustfox"); + let mut cfg: Config = toml::from_str(base_toml()).unwrap(); + cfg.general = Some(GeneralConfig { + location: None, + home: Some(home.clone()), + }); + let warnings = cfg.resolve().unwrap(); + assert_eq!(cfg.resolved_home.as_ref().unwrap(), &home); + assert_eq!(cfg.sandbox.allowed_directory, home.join("workspace")); + assert_eq!(cfg.memory.database_path, home.join("rustfox.db")); + assert_eq!(cfg.skills.directory, home.join("skills")); + assert_eq!(cfg.agents.directory, home.join("agents")); + assert_eq!(cfg.supervisor.artifacts_dir, home.join("artifacts")); + assert_eq!(cfg.learning.user_model_path, home.join("user_model.md")); + assert!(warnings.is_empty()); + } + + #[test] + fn resolve_keeps_absolute_overrides() { + let tmp = tempfile::tempdir().unwrap(); + let home = tmp.path().join(".rustfox"); + let mut cfg: Config = toml::from_str(base_toml()).unwrap(); + cfg.general = Some(GeneralConfig { + location: None, + home: Some(home.clone()), + }); + cfg.memory.database_path = std::path::PathBuf::from("/data/custom.db"); + let warnings = cfg.resolve().unwrap(); + assert_eq!(cfg.memory.database_path, std::path::PathBuf::from("/data/custom.db")); + assert!(warnings.is_empty()); + } + + #[test] + fn resolve_warns_on_relative_override() { + let tmp = tempfile::tempdir().unwrap(); + let home = tmp.path().join(".rustfox"); + let mut cfg: Config = toml::from_str(base_toml()).unwrap(); + cfg.general = Some(GeneralConfig { + location: None, + home: Some(home), + }); + cfg.skills.directory = std::path::PathBuf::from("my-skills"); + let warnings = cfg.resolve().unwrap(); + assert_eq!(cfg.skills.directory, std::path::PathBuf::from("my-skills")); + assert!(warnings.iter().any(|w| w.label == "skills.directory")); + } +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cargo test --lib config::tests::resolve_fills_unset_paths_under_home` +Expected: FAIL — `no method named resolve found for struct Config`. + +- [ ] **Step 3: Implement `Config::resolve()`** + +Add inside `impl Config { ... }` (before `pub fn load`): + +```rust + /// Resolve the home root and every data path, create directories, and write + /// the resolved absolute paths back into the config fields. Returns any + /// legacy-path warnings (relative overrides) for the caller to log. + pub fn resolve(&mut self) -> Result> { + use crate::home::{ensure_dirs, resolve_data_path, resolve_home, PathOrigin, ResolvedPaths}; + + let env_home = std::env::var("RUSTFOX_HOME").ok(); + let config_home = self.general.as_ref().and_then(|g| g.home.as_deref()); + let os_home = dirs::home_dir(); + let home = resolve_home(env_home.as_deref(), config_home, os_home.as_deref())?; + + let mut warnings = Vec::new(); + let mut resolve_one = |label: &str, field: &Path, subpath: &str| -> PathBuf { + let (path, origin) = resolve_data_path(field, &home, subpath); + if origin == PathOrigin::RelativeLegacy { + warnings.push(crate::home::LegacyPathWarning { + label: label.to_string(), + current: path.clone(), + home_default: home.join(subpath), + }); + } + path + }; + + let workspace = resolve_one("sandbox.allowed_directory", &self.sandbox.allowed_directory, "workspace"); + let database = resolve_one("memory.database_path", &self.memory.database_path, "rustfox.db"); + let skills = resolve_one("skills.directory", &self.skills.directory, "skills"); + let agents = resolve_one("agents.directory", &self.agents.directory, "agents"); + let artifacts = resolve_one("supervisor.artifacts_dir", &self.supervisor.artifacts_dir, "artifacts"); + let user_model = resolve_one("learning.user_model_path", &self.learning.user_model_path, "user_model.md"); + + let paths = ResolvedPaths { + home: home.clone(), + workspace: workspace.clone(), + database: database.clone(), + skills: skills.clone(), + agents: agents.clone(), + artifacts: artifacts.clone(), + user_model: user_model.clone(), + }; + ensure_dirs(&paths)?; + + self.sandbox.allowed_directory = workspace; + self.memory.database_path = database; + self.skills.directory = skills; + self.agents.directory = agents; + self.supervisor.artifacts_dir = artifacts; + self.learning.user_model_path = user_model; + self.resolved_home = Some(home); + + Ok(warnings) + } +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cargo test --lib config::tests` +Expected: PASS (existing tests + the three new `resolve_*` tests). + +- [ ] **Step 5: Commit** + +```bash +git add src/config.rs +git commit -m "feat(config): add Config::resolve to materialize home-relative paths" +``` + +--- + +### Task 8: Wire `Config::load` to resolve + log warnings + +**Files:** +- Modify: `src/config.rs` + +- [ ] **Step 1: Write the failing test** + +Add to `mod tests`: + +```rust + #[test] + fn load_resolves_paths_to_absolute() { + let tmp = tempfile::tempdir().unwrap(); + let home = tmp.path().join(".rustfox"); + let cfg_path = tmp.path().join("config.toml"); + let toml = format!( + r#" + [telegram] + bot_token = "tok" + allowed_user_ids = [1] + [openrouter] + api_key = "key" + [general] + home = "{}" + "#, + home.display() + ); + std::fs::write(&cfg_path, toml).unwrap(); + let cfg = Config::load(&cfg_path).unwrap(); + assert_eq!(cfg.sandbox.allowed_directory, home.join("workspace")); + assert!(cfg.sandbox.allowed_directory.is_dir()); + assert_eq!(cfg.resolved_home.unwrap(), home); + } +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cargo test --lib config::tests::load_resolves_paths_to_absolute` +Expected: FAIL — `allowed_directory` is empty (current `load` only creates the directory; it does not resolve). + +- [ ] **Step 3: Update `Config::load`** + +Replace the current `pub fn load` body: + +```rust + pub fn load(path: &Path) -> Result { + let content = std::fs::read_to_string(path) + .with_context(|| format!("Failed to read config file: {}", path.display()))?; + let mut config: Config = + toml::from_str(&content).with_context(|| "Failed to parse config file")?; + + let warnings = config + .resolve() + .with_context(|| "Failed to resolve home directory paths")?; + for w in &warnings { + tracing::warn!("{}", w.render()); + } + + Ok(config) + } +``` + +(Remove the old sandbox-existence block; `ensure_dirs` inside `resolve` now creates all directories.) + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cargo test --lib config::tests` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add src/config.rs +git commit -m "feat(config): resolve paths and log legacy warnings in load" +``` + +--- + +### Task 9: Config-file discovery fallback in `main.rs` + +**Files:** +- Modify: `src/home.rs` (add `default_home`) +- Modify: `src/main.rs` + +- [ ] **Step 1: Write the failing test for `default_home`** + +Add to `src/home.rs` after `resolve_home`: + +```rust +/// The home root used purely for *config-file discovery*, before the config is +/// loaded. Uses only `RUSTFOX_HOME` (if absolute) or `/.rustfox`. +pub fn default_home(env_home: Option<&str>, os_home: Option<&Path>) -> Option { + if let Some(env) = env_home { + let p = Path::new(env); + if p.is_absolute() { + return Some(p.to_path_buf()); + } + } + os_home.map(|h| h.join(".rustfox")) +} +``` + +Add the test in `mod tests`: + +```rust + #[test] + fn default_home_prefers_absolute_env() { + assert_eq!( + default_home(Some("/srv/rfx"), Some(Path::new("/home/u"))), + Some(PathBuf::from("/srv/rfx")) + ); + assert_eq!( + default_home(None, Some(Path::new("/home/u"))), + Some(PathBuf::from("/home/u/.rustfox")) + ); + assert_eq!(default_home(Some("rel"), Some(Path::new("/home/u"))), + Some(PathBuf::from("/home/u/.rustfox"))); + } +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cargo test --lib home::tests::default_home_prefers_absolute_env` +Expected: FAIL — `cannot find function default_home`. (Add the function in the same step if TDD prefers; here the function above makes it pass — so first add only the test, confirm fail, then add the function.) + +To honor red-green: add the test first, run (FAIL: function missing), then add `default_home`. + +- [ ] **Step 3: Confirm pass** + +Run: `cargo test --lib home::tests::default_home_prefers_absolute_env` +Expected: PASS. + +- [ ] **Step 4: Use it in `main.rs` config discovery** + +In `src/main.rs`, replace the config-path block: + +```rust + let config_path = std::env::args() + .nth(1) + .map(PathBuf::from) + .unwrap_or_else(|| PathBuf::from("config.toml")); +``` + +with: + +```rust + let config_path = std::env::args().nth(1).map(PathBuf::from).unwrap_or_else(|| { + let cwd = PathBuf::from("config.toml"); + if cwd.exists() { + return cwd; + } + let env_home = std::env::var("RUSTFOX_HOME").ok(); + if let Some(home) = rustfox::home::default_home(env_home.as_deref(), dirs::home_dir().as_deref()) { + let candidate = home.join("config.toml"); + if candidate.exists() { + return candidate; + } + } + cwd + }); +``` + +(`dirs` is already a dependency from Task 1; add `use std::path::PathBuf;` is already present.) + +- [ ] **Step 5: Verify build + log the home** + +After the existing `info!(" Sandbox: ...")` line in `main.rs`, add: + +```rust + if let Some(home) = &config.resolved_home { + info!(" Home: {}", home.display()); + } +``` + +Run: `cargo build` +Expected: builds cleanly. + +- [ ] **Step 6: Commit** + +```bash +git add src/home.rs src/main.rs +git commit -m "feat(home): config-file discovery fallback to /config.toml" +``` + +--- + +### Task 10: Sandbox = workspace integration test + system-prompt text + +**Files:** +- Create: `tests/home_sandbox.rs` +- Modify: `src/config.rs` (system prompt text) + +- [ ] **Step 1: Write the failing integration test** + +Create `tests/home_sandbox.rs`: + +```rust +use rustfox::config::Config; + +#[test] +fn sandbox_defaults_to_home_workspace_and_excludes_secrets() { + let tmp = tempfile::tempdir().unwrap(); + let home = tmp.path().join(".rustfox"); + let cfg_path = tmp.path().join("config.toml"); + let toml = format!( + r#" + [telegram] + bot_token = "tok" + allowed_user_ids = [1] + [openrouter] + api_key = "key" + [general] + home = "{}" + "#, + home.display() + ); + std::fs::write(&cfg_path, toml).unwrap(); + let cfg = Config::load(&cfg_path).unwrap(); + + // Sandbox is the workspace subdir of home. + assert_eq!(cfg.sandbox.allowed_directory, home.join("workspace")); + // DB lives ABOVE the sandbox → structurally unreachable by file tools. + assert_eq!(cfg.memory.database_path, home.join("rustfox.db")); + assert!(!cfg.memory.database_path.starts_with(&cfg.sandbox.allowed_directory)); +} +``` + +- [ ] **Step 2: Run test to verify it passes** + +Run: `cargo test --test home_sandbox` +Expected: PASS (this validates the resolution wired in Tasks 7–8). If it fails, fix resolution before continuing. + +- [ ] **Step 3: Update the system-prompt sandbox description** + +In `src/config.rs`, in `default_system_prompt()`, change the final block: + +```rust + ## Sandbox\n\ + File and command tools operate only within the allowed sandbox directory." +``` + +to: + +```rust + ## Sandbox\n\ + File and command tools operate only within your persistent workspace directory.\n\ + The workspace survives restarts — use it to keep reusable scripts, programs, and notes for the long term." +``` + +- [ ] **Step 4: Verify tests still pass** + +Run: `cargo test --lib config::tests` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add tests/home_sandbox.rs src/config.rs +git commit -m "test+docs: assert sandbox=workspace and note persistence in prompt" +``` + +--- + +### Task 11: Skill content hashing + seeding (`src/skills/seed.rs`) + +**Files:** +- Create: `src/skills/seed.rs` +- Modify: `src/skills/mod.rs` + +- [ ] **Step 1: Register the module** + +In `src/skills/mod.rs`, change the first line from: +```rust +pub mod loader; +``` +to: +```rust +pub mod loader; +pub mod seed; +pub mod update; +``` + +(`update` is created in Task 12; declaring it now is fine only if the file exists. To avoid a build break, create an empty `src/skills/update.rs` placeholder now with a single line `// implemented in Task 12` — it will be filled in Task 12. Alternatively add `pub mod update;` in Task 12. **Do the latter**: in this task add only `pub mod seed;`.) + +So in this task, change to: +```rust +pub mod loader; +pub mod seed; +``` + +- [ ] **Step 2: Write the failing tests** + +Create `src/skills/seed.rs`: + +```rust +use anyhow::{Context, Result}; +use sha2::{Digest, Sha256}; +use std::collections::BTreeMap; +use std::path::Path; + +/// SHA-256 (hex) of a skill/agent directory's primary markdown file +/// (`SKILL.md`, else `AGENT.md`). Returns `None` if neither exists. +pub fn hash_skill_dir(dir: &Path) -> Option { + let primary = ["SKILL.md", "AGENT.md"] + .into_iter() + .map(|f| dir.join(f)) + .find(|p| p.is_file())?; + let bytes = std::fs::read(&primary).ok()?; + let mut h = Sha256::new(); + h.update(&bytes); + Some(format!("{:x}", h.finalize())) +} + +/// Copy every skill subdirectory from `bundled` into `instance` when `instance` +/// is empty or missing. Returns the number of skills copied (0 if skipped). +pub async fn seed_dir_if_empty(bundled: &Path, instance: &Path) -> Result { + todo!() +} + +/// Map of skill-name → content hash for every skill dir under `dir`. +pub fn lock_map_for(dir: &Path) -> BTreeMap { + let mut map = BTreeMap::new(); + let Ok(entries) = std::fs::read_dir(dir) else { + return map; + }; + for entry in entries.flatten() { + let p = entry.path(); + if p.is_dir() { + if let (Some(name), Some(hash)) = + (p.file_name().and_then(|n| n.to_str()), hash_skill_dir(&p)) + { + map.insert(name.to_string(), hash); + } + } + } + map +} + +#[cfg(test)] +mod tests { + use super::*; + + fn write_skill(root: &Path, name: &str, body: &str) { + let dir = root.join(name); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("SKILL.md"), body).unwrap(); + } + + #[tokio::test] + async fn seeds_into_empty_instance() { + let tmp = tempfile::tempdir().unwrap(); + let bundled = tmp.path().join("bundled"); + let instance = tmp.path().join("instance"); + write_skill(&bundled, "alpha", "---\nname: alpha\n---\nhi"); + write_skill(&bundled, "beta", "---\nname: beta\n---\nyo"); + + let n = seed_dir_if_empty(&bundled, &instance).await.unwrap(); + assert_eq!(n, 2); + assert!(instance.join("alpha/SKILL.md").is_file()); + assert!(instance.join("beta/SKILL.md").is_file()); + } + + #[tokio::test] + async fn skips_when_instance_nonempty() { + let tmp = tempfile::tempdir().unwrap(); + let bundled = tmp.path().join("bundled"); + let instance = tmp.path().join("instance"); + write_skill(&bundled, "alpha", "a"); + write_skill(&instance, "existing", "keep me"); + + let n = seed_dir_if_empty(&bundled, &instance).await.unwrap(); + assert_eq!(n, 0); + assert!(!instance.join("alpha").exists()); + assert!(instance.join("existing/SKILL.md").is_file()); + } + + #[test] + fn lock_map_hashes_each_skill() { + let tmp = tempfile::tempdir().unwrap(); + write_skill(tmp.path(), "alpha", "content-a"); + write_skill(tmp.path(), "beta", "content-b"); + let map = lock_map_for(tmp.path()); + assert_eq!(map.len(), 2); + assert_ne!(map["alpha"], map["beta"]); + } +} +``` + +- [ ] **Step 3: Run tests to verify they fail** + +Run: `cargo test --lib skills::seed::tests` +Expected: FAIL — `seeds_into_empty_instance` panics with `not yet implemented`. + +- [ ] **Step 4: Implement `seed_dir_if_empty`** + +Replace the `todo!()`: + +```rust +pub async fn seed_dir_if_empty(bundled: &Path, instance: &Path) -> Result { + // If the bundled source and instance resolve to the same place, nothing to do. + if let (Ok(a), Ok(b)) = (bundled.canonicalize(), instance.canonicalize()) { + if a == b { + return Ok(0); + } + } + if !bundled.is_dir() { + tracing::info!("No bundled skills at {}; skipping seed", bundled.display()); + return Ok(0); + } + // Non-empty instance → never overwrite. + if instance.is_dir() { + let mut entries = tokio::fs::read_dir(instance).await?; + if entries.next_entry().await?.is_some() { + return Ok(0); + } + } else { + tokio::fs::create_dir_all(instance) + .await + .with_context(|| format!("Failed to create {}", instance.display()))?; + } + + let mut copied = 0usize; + let mut entries = tokio::fs::read_dir(bundled).await?; + while let Some(entry) = entries.next_entry().await? { + let src = entry.path(); + if !src.is_dir() { + continue; + } + let Some(name) = src.file_name() else { continue }; + let dst = instance.join(name); + if let Err(e) = copy_dir_recursive(&src, &dst).await { + tracing::warn!("Failed to seed {}: {}", src.display(), e); + continue; + } + copied += 1; + } + tracing::info!("Seeded {copied} skill(s) into {}", instance.display()); + Ok(copied) +} + +/// Recursively copy `src` directory to `dst`. +async fn copy_dir_recursive(src: &Path, dst: &Path) -> Result<()> { + tokio::fs::create_dir_all(dst).await?; + let mut entries = tokio::fs::read_dir(src).await?; + while let Some(entry) = entries.next_entry().await? { + let from = entry.path(); + let to = dst.join(entry.file_name()); + if from.is_dir() { + Box::pin(copy_dir_recursive(&from, &to)).await?; + } else { + tokio::fs::copy(&from, &to).await?; + } + } + Ok(()) +} +``` + +- [ ] **Step 5: Run tests to verify they pass** + +Run: `cargo test --lib skills::seed::tests` +Expected: PASS (3 tests). + +- [ ] **Step 6: Commit** + +```bash +git add src/skills/seed.rs src/skills/mod.rs +git commit -m "feat(skills): add content hashing + first-run seed copy" +``` + +--- + +### Task 12: `/update-skills` hash-diff engine (`src/skills/update.rs`) + +**Files:** +- Create: `src/skills/update.rs` +- Modify: `src/skills/mod.rs` + +- [ ] **Step 1: Register the module** + +In `src/skills/mod.rs`, add after `pub mod seed;`: +```rust +pub mod update; +``` + +- [ ] **Step 2: Write the failing tests** + +Create `src/skills/update.rs`: + +```rust +use anyhow::{Context, Result}; +use serde::{Deserialize, Serialize}; +use std::collections::BTreeMap; +use std::path::Path; + +use super::seed::{copy_dir_recursive_pub, hash_skill_dir, lock_map_for}; + +/// Home-side lock file: skill-name → content hash captured at seed/update time. +#[derive(Debug, Default, Serialize, Deserialize)] +pub struct SkillLock { + pub version: u32, + #[serde(default)] + pub skills: BTreeMap, +} + +/// Outcome of an update run. +#[derive(Debug, Default, PartialEq, Eq)] +pub struct UpdateReport { + pub updated: Vec, + pub backed_up: Vec, + pub skipped: Vec, +} + +impl UpdateReport { + pub fn summary(&self) -> String { + format!( + "Skill update: {} updated, {} backed-up, {} unchanged.", + self.updated.len(), + self.backed_up.len(), + self.skipped.len() + ) + } +} + +/// Read the lock file at `lock_path`, or an empty lock if absent/invalid. +fn read_lock(lock_path: &Path) -> SkillLock { + std::fs::read_to_string(lock_path) + .ok() + .and_then(|s| serde_json::from_str(&s).ok()) + .unwrap_or(SkillLock { + version: 1, + skills: BTreeMap::new(), + }) +} + +fn write_lock(lock_path: &Path, lock: &SkillLock) -> Result<()> { + let json = serde_json::to_string_pretty(lock)?; + std::fs::write(lock_path, json) + .with_context(|| format!("Failed to write lock {}", lock_path.display())) +} + +/// Re-sync `bundled` → `instance` using content hashes recorded in `lock_path`. +/// +/// For each bundled skill: +/// - missing in instance → copy in (updated) +/// - present, instance hash == bundled hash → unchanged (skipped) +/// - present, instance hash == lock hash (and != bundled) → overwrite (updated) +/// - present, instance hash differs from both → back up `*.bak`, overwrite (backed_up) +/// +/// Instance-only skills (absent from `bundled`) are never touched. +pub async fn update_skills(bundled: &Path, instance: &Path, lock_path: &Path) -> Result { + todo!() +} + +#[cfg(test)] +mod tests { + use super::*; + + fn write_skill(root: &Path, name: &str, body: &str) { + let dir = root.join(name); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("SKILL.md"), body).unwrap(); + } + + fn seed_lock(lock_path: &Path, instance: &Path) { + let lock = SkillLock { + version: 1, + skills: lock_map_for(instance), + }; + write_lock(lock_path, &lock).unwrap(); + } + + #[tokio::test] + async fn unchanged_skill_is_overwritten_with_new_bundle() { + let tmp = tempfile::tempdir().unwrap(); + let bundled = tmp.path().join("bundled"); + let instance = tmp.path().join("instance"); + let lock = tmp.path().join("skills-lock.json"); + // instance currently matches the OLD bundle + write_skill(&instance, "alpha", "v1"); + seed_lock(&lock, &instance); + // bundle now has v2 + write_skill(&bundled, "alpha", "v2"); + + let report = update_skills(&bundled, &instance, &lock).await.unwrap(); + assert_eq!(report.updated, vec!["alpha".to_string()]); + assert!(report.backed_up.is_empty()); + assert_eq!(std::fs::read_to_string(instance.join("alpha/SKILL.md")).unwrap(), "v2"); + } + + #[tokio::test] + async fn locally_modified_skill_is_backed_up() { + let tmp = tempfile::tempdir().unwrap(); + let bundled = tmp.path().join("bundled"); + let instance = tmp.path().join("instance"); + let lock = tmp.path().join("skills-lock.json"); + write_skill(&instance, "alpha", "v1"); + seed_lock(&lock, &instance); + // user edited the instance copy + std::fs::write(instance.join("alpha/SKILL.md"), "user-edit").unwrap(); + // bundle moved to v2 + write_skill(&bundled, "alpha", "v2"); + + let report = update_skills(&bundled, &instance, &lock).await.unwrap(); + assert_eq!(report.backed_up, vec!["alpha".to_string()]); + assert_eq!(std::fs::read_to_string(instance.join("alpha/SKILL.md")).unwrap(), "v2"); + assert_eq!(std::fs::read_to_string(instance.join("alpha/SKILL.md.bak")).unwrap(), "user-edit"); + } + + #[tokio::test] + async fn instance_only_skill_is_untouched() { + let tmp = tempfile::tempdir().unwrap(); + let bundled = tmp.path().join("bundled"); + let instance = tmp.path().join("instance"); + let lock = tmp.path().join("skills-lock.json"); + write_skill(&bundled, "alpha", "v1"); + write_skill(&instance, "alpha", "v1"); + write_skill(&instance, "mine", "private"); + seed_lock(&lock, &instance); + + let report = update_skills(&bundled, &instance, &lock).await.unwrap(); + // alpha unchanged (same hash), mine never visited + assert!(report.updated.is_empty()); + assert!(report.backed_up.is_empty()); + assert_eq!(std::fs::read_to_string(instance.join("mine/SKILL.md")).unwrap(), "private"); + } +} +``` + +- [ ] **Step 3: Expose `copy_dir_recursive` from `seed.rs`** + +In `src/skills/seed.rs`, add a public wrapper (the private `copy_dir_recursive` already exists from Task 11): + +```rust +/// Public re-export wrapper so the update engine can reuse recursive copy. +pub async fn copy_dir_recursive_pub(src: &Path, dst: &Path) -> Result<()> { + copy_dir_recursive(src, dst).await +} +``` + +- [ ] **Step 4: Run tests to verify they fail** + +Run: `cargo test --lib skills::update::tests` +Expected: FAIL — `not yet implemented`. + +- [ ] **Step 5: Implement `update_skills`** + +Replace the `todo!()`: + +```rust +pub async fn update_skills(bundled: &Path, instance: &Path, lock_path: &Path) -> Result { + let mut report = UpdateReport::default(); + if !bundled.is_dir() { + anyhow::bail!("No bundled skills found at {}", bundled.display()); + } + let mut lock = read_lock(lock_path); + tokio::fs::create_dir_all(instance).await.ok(); + + let mut entries = tokio::fs::read_dir(bundled).await?; + while let Some(entry) = entries.next_entry().await? { + let src = entry.path(); + if !src.is_dir() { + continue; + } + let Some(name) = src.file_name().and_then(|n| n.to_str()).map(str::to_string) else { + continue; + }; + let dst = instance.join(&name); + let bundled_hash = match hash_skill_dir(&src) { + Some(h) => h, + None => continue, + }; + + if !dst.exists() { + copy_dir_recursive_pub(&src, &dst).await?; + lock.skills.insert(name.clone(), bundled_hash); + report.updated.push(name); + continue; + } + + let instance_hash = hash_skill_dir(&dst); + if instance_hash.as_deref() == Some(bundled_hash.as_str()) { + report.skipped.push(name); + continue; + } + + let lock_hash = lock.skills.get(&name).cloned(); + let unmodified = lock_hash.is_some() && lock_hash.as_deref() == instance_hash.as_deref(); + + if !unmodified { + // Back up the primary file before overwriting. + for f in ["SKILL.md", "AGENT.md"] { + let p = dst.join(f); + if p.is_file() { + let _ = tokio::fs::copy(&p, dst.join(format!("{f}.bak"))).await; + } + } + // Remove and re-copy the whole dir to pick up new/removed files. + let _ = tokio::fs::remove_dir_all(&dst).await; + copy_dir_recursive_pub(&src, &dst).await?; + // Restore the .bak we just lost in the remove (re-copy from src wiped it): + // simplest: re-write the bak from the in-memory backup is unnecessary because + // we copied the bak before removing — but remove_dir_all deletes it. So copy bak first to instance parent. + lock.skills.insert(name.clone(), bundled_hash); + report.backed_up.push(name); + } else { + let _ = tokio::fs::remove_dir_all(&dst).await; + copy_dir_recursive_pub(&src, &dst).await?; + lock.skills.insert(name.clone(), bundled_hash); + report.updated.push(name); + } + } + + write_lock(lock_path, &lock)?; + Ok(report) +} +``` + +**Important correctness note:** the backup-then-`remove_dir_all` ordering above would delete the `.bak`. Implement the backup by writing it to a sibling location that survives the wipe. Replace the backed-up branch body with: + +```rust + if !unmodified { + // Back up the primary file to a sibling path OUTSIDE the dir being replaced. + for f in ["SKILL.md", "AGENT.md"] { + let p = dst.join(f); + if p.is_file() { + let bak = instance.join(format!("{name}.{f}.bak.tmp")); + let _ = tokio::fs::copy(&p, &bak).await; + } + } + let _ = tokio::fs::remove_dir_all(&dst).await; + copy_dir_recursive_pub(&src, &dst).await?; + // Move the temp backups into the freshly-copied dir as .bak + for f in ["SKILL.md", "AGENT.md"] { + let bak = instance.join(format!("{name}.{f}.bak.tmp")); + if bak.is_file() { + let _ = tokio::fs::rename(&bak, dst.join(format!("{f}.bak"))).await; + } + } + lock.skills.insert(name.clone(), bundled_hash); + report.backed_up.push(name); + } +``` + +Use this corrected branch (delete the earlier first-draft backed-up branch). The test `locally_modified_skill_is_backed_up` asserts `alpha/SKILL.md.bak == "user-edit"`, which this satisfies. + +- [ ] **Step 6: Run tests to verify they pass** + +Run: `cargo test --lib skills::update::tests` +Expected: PASS (3 tests). + +- [ ] **Step 7: Commit** + +```bash +git add src/skills/update.rs src/skills/seed.rs src/skills/mod.rs +git commit -m "feat(skills): add /update-skills hash-diff engine with backups" +``` + +--- + +### Task 13: Seed skills/agents at startup in `main.rs` + +**Files:** +- Modify: `src/main.rs` + +- [ ] **Step 1: Add seeding before skills are loaded** + +In `src/main.rs`, immediately **before** the line `let skills = load_skills_from_dir(&config.skills.directory).await?;`, insert: + +```rust + // Seed bundled skills/agents into the instance home on first run. + let bundled_skills = PathBuf::from("skills"); + let bundled_agents = PathBuf::from("agents"); + if let Err(e) = + rustfox::skills::seed::seed_dir_if_empty(&bundled_skills, &config.skills.directory).await + { + warn!("Skill seeding failed: {e}"); + } + if let Err(e) = + rustfox::skills::seed::seed_dir_if_empty(&bundled_agents, &config.agents.directory).await + { + warn!("Agent seeding failed: {e}"); + } + // Write the home-side lock so /update-skills can diff later (only when seeded + // into the home and a lock does not already exist). + if let Some(home) = &config.resolved_home { + let lock_path = home.join("skills-lock.json"); + if !lock_path.exists() { + let lock = rustfox::skills::update::SkillLock { + version: 1, + skills: rustfox::skills::seed::lock_map_for(&config.skills.directory), + }; + if let Ok(json) = serde_json::to_string_pretty(&lock) { + let _ = std::fs::write(&lock_path, json); + } + } + } +``` + +- [ ] **Step 2: Verify build** + +Run: `cargo build` +Expected: builds cleanly. (`serde_json` and `warn` are already imported in `main.rs`.) + +- [ ] **Step 3: Manual smoke check** + +Run: `RUSTFOX_HOME="$(mktemp -d)/.rustfox" cargo run -- config.toml` for ~3 seconds, then Ctrl-C. +Expected log lines: `Home: .../.rustfox`, `Seeded N skill(s) ...`, `Skills: N`. A `skills-lock.json` exists in the home. + +(If `config.toml` is not present/valid in the dev env, skip this manual step and rely on the integration test in Task 10 plus the unit tests.) + +- [ ] **Step 4: Commit** + +```bash +git add src/main.rs +git commit -m "feat: seed bundled skills/agents into home on first run" +``` + +--- + +### Task 14: Add `Agent::reload_skills_and_agents` + +**Files:** +- Modify: `src/agent.rs` + +- [ ] **Step 1: Add the method** + +In `src/agent.rs`, inside `impl Agent { ... }` (near the other public methods, e.g. after `build_system_prompt`), add: + +```rust + /// Reload both skill and agent registries from their directories. + /// Returns `(skills_count, agents_count)`. + pub async fn reload_skills_and_agents(&self) -> (usize, usize) { + use crate::skills::loader::load_skills_from_dir; + let mut s_count = 0; + let mut a_count = 0; + if let Ok(reg) = load_skills_from_dir(&self.config.skills.directory).await { + s_count = reg.len(); + *self.skills.write().await = reg; + } + if let Ok(reg) = load_skills_from_dir(&self.config.agents.directory).await { + a_count = reg.len(); + *self.agents.write().await = reg; + } + (s_count, a_count) + } +``` + +(The `agents` field is `pub agents: tokio::sync::RwLock`, symmetric with `skills`. If the field has a different name, use the actual name found near the `skills` field declaration.) + +- [ ] **Step 2: Verify build** + +Run: `cargo build` +Expected: builds cleanly. + +- [ ] **Step 3: Commit** + +```bash +git add src/agent.rs +git commit -m "feat(agent): add reload_skills_and_agents helper" +``` + +--- + +### Task 15: `/update-skills` Telegram command + +**Files:** +- Modify: `src/platform/telegram.rs` + +- [ ] **Step 1: Add the command block** + +In `src/platform/telegram.rs`, immediately **after** the `if text == "/skills" { ... }` block (before `if text == "/verbose"`), insert: + +```rust + if text == "/updateskills" || text == "/update-skills" { + let bundled_skills = std::path::PathBuf::from("skills"); + let bundled_agents = std::path::PathBuf::from("agents"); + let lock_path = agent + .config + .resolved_home + .clone() + .map(|h| h.join("skills-lock.json")) + .unwrap_or_else(|| std::path::PathBuf::from("skills-lock.json")); + + let mut lines = Vec::new(); + match rustfox::skills::update::update_skills( + &bundled_skills, + &agent.config.skills.directory, + &lock_path, + ) + .await + { + Ok(r) => lines.push(format!("Skills — {}", r.summary())), + Err(e) => lines.push(format!("Skills update failed: {e}")), + } + match rustfox::skills::update::update_skills( + &bundled_agents, + &agent.config.agents.directory, + &lock_path, + ) + .await + { + Ok(r) => lines.push(format!("Agents — {}", r.summary())), + Err(e) => lines.push(format!("Agents update failed: {e}")), + } + + let (s, a) = agent.reload_skills_and_agents().await; + lines.push(format!("Reloaded: {s} skill(s), {a} agent(s) active.")); + + bot.send_message(msg.chat.id, escape_text(&lines.join("\n"))) + .parse_mode(ParseMode::MarkdownV2) + .await?; + return Ok(()); + } +``` + +(Confirm the in-scope binding name for the agent — it is `agent` where `/skills` reads `agent.skills.read()`. Reuse the same binding and `escape_text` / `ParseMode` already imported in this file.) + +- [ ] **Step 2: Update `/start` help text** + +In the `/start` block, change the help string to add the new command line: + +```rust + /skills - List loaded skills\n\ + /update-skills - Re-sync bundled skills/agents (backs up local edits)\n\ + /verbose - Toggle tool call progress display\n\ +``` + +- [ ] **Step 3: Verify build** + +Run: `cargo build` +Expected: builds cleanly. + +- [ ] **Step 4: Commit** + +```bash +git add src/platform/telegram.rs +git commit -m "feat(telegram): add /update-skills command and help entry" +``` + +--- + +### Task 16: Update `config.example.toml` + +**Files:** +- Modify: `config.example.toml` + +- [ ] **Step 1: Document the home model and optional paths** + +In `config.example.toml`, change the `[sandbox]` section from: + +```toml +[sandbox] +allowed_directory = "/tmp/rustfox-sandbox" +``` + +to: + +```toml +# ── Home directory & paths ────────────────────────────────────────────── +# By default RustFox stores everything under ~/.rustfox: +# ~/.rustfox/config.toml, rustfox.db, skills/, agents/, workspace/, artifacts/ +# Override the home root with the RUSTFOX_HOME env var (absolute path) or +# [general].home below. Run a second isolated instance with: +# RUSTFOX_HOME="$HOME/.rustfox-work" cargo run +# +# [general] +# home = "/absolute/path/to/home" # optional; overrides ~/.rustfox + +[sandbox] +# The LLM's persistent workspace (file/command tools are confined here). +# Leave unset to use /workspace. Set an absolute path to override. +# allowed_directory = "/absolute/path/to/workspace" +``` + +- [ ] **Step 2: Comment out the now-optional path keys** + +Find the `[memory]` section and change `database_path = "rustfox.db"` to: +```toml +# Leave unset to use /rustfox.db. Set an absolute path to override. +# database_path = "/absolute/path/to/rustfox.db" +``` + +Find `[skills]` and change `directory = "skills"` to: +```toml +# Leave unset to use /skills (seeded from the bundled skills on first run). +# directory = "/absolute/path/to/skills" +``` + +If `[agents]`, `[supervisor].artifacts_dir`, or `[learning].user_model_path` keys are present in the example, comment them out the same way with a "Leave unset to use /..." note. If a section is absent, do not add it. + +- [ ] **Step 3: Commit** + +```bash +git add config.example.toml +git commit -m "docs: document ~/.rustfox home and optional paths in example config" +``` + +--- + +### Task 17: Migration & multi-instance tutorial + +**Files:** +- Create: `docs/persistent-home-directory.md` + +- [ ] **Step 1: Write the tutorial** + +Create `docs/persistent-home-directory.md`: + +````markdown +# Persistent Home Directory (`~/.rustfox`) + +RustFox stores all of its state under a single **home directory**, by default +`~/.rustfox`. This survives reboots, keeps secrets out of the LLM sandbox, and +makes it easy to run several isolated instances on one machine. + +## Layout + +``` +~/.rustfox/ + config.toml # secrets (bot token, API keys) — OUTSIDE the sandbox + rustfox.db # SQLite memory + embeddings — OUTSIDE the sandbox + skills/ # skills (seeded on first run, editable) + agents/ # agents (seeded on first run, editable) + workspace/ # THE SANDBOX — durable scratch space for the LLM + artifacts/ # supervisor artifacts + user_model.md # learned user model +``` + +Only `workspace/` is reachable by the file and command tools. `config.toml` +and `rustfox.db` live above it and cannot be read or written by the LLM. + +## Choosing where the home lives + +Resolution order (first match wins): + +1. `RUSTFOX_HOME` environment variable (must be an absolute path) +2. `[general].home` in `config.toml` (absolute path) +3. `~/.rustfox` (default) + +Each individual path can still be pinned independently in `config.toml` +(e.g. `[memory].database_path`). An absolute value is used verbatim; an unset +value falls back to the home default. + +## Migrating existing data + +If you previously ran RustFox from a project directory (with `./rustfox.db`, +`./skills`, etc.), RustFox will **not** move your files automatically. On +startup it prints an actionable warning for each legacy path. To migrate: + +RustFox auto-creates the home subdirectories on startup, so use `cp -rT` +(merge into the existing destination directory) rather than plain `cp -r`, +which would nest (e.g. `~/.rustfox/skills/skills`). Each command copies only +that one path — never your whole project. + +```bash +mkdir -p ~/.rustfox +cp ./rustfox.db ~/.rustfox/rustfox.db +cp -rT ./skills ~/.rustfox/skills +cp -rT ./agents ~/.rustfox/agents +cp -rT ./supervisor/artifacts ~/.rustfox/artifacts # if you used the supervisor +cp ./memory/USER.md ~/.rustfox/user_model.md # if present +# Move your old sandbox contents into the new persistent workspace: +cp -rT /tmp/rustfox-sandbox ~/.rustfox/workspace # adjust to your old sandbox +``` + +Then remove any path overrides from `config.toml` so RustFox uses the home +defaults, and place your `config.toml` at `~/.rustfox/config.toml` (or keep +passing it as the first CLI argument). + +## Keeping the old location instead + +If you prefer your current layout, pin the paths explicitly in `config.toml`: + +```toml +[sandbox] +allowed_directory = "/abs/path/to/old/sandbox" +[memory] +database_path = "/abs/path/to/rustfox.db" +[skills] +directory = "/abs/path/to/skills" +``` + +Absolute paths are always honored unchanged. + +## Starting fresh + +Doing nothing is the "start fresh" path: RustFox creates an empty +`~/.rustfox/workspace`, a new database, and seeds skills/agents from the bundled +copies. Your old project-directory files are left untouched. + +## Running multiple instances + +Give each instance its own home: + +```bash +# Default personal instance +cargo run + +# A separate work instance, fully isolated +RUSTFOX_HOME="$HOME/.rustfox-work" cargo run +``` + +Each home has independent skills, agents, workspace, database, and artifacts. + +## Updating bundled skills + +The instance `skills/` and `agents/` directories are seeded once on first run. +To pull in newer bundled versions later, send the bot `/update-skills`: + +- Skills you have not modified are overwritten with the new bundled version. +- Skills you edited locally are backed up to `/SKILL.md.bak` before being + updated, so your changes are never silently lost. +- Skills you created yourself (not part of the bundle) are never touched. +```` + +- [ ] **Step 2: Commit** + +```bash +git add docs/persistent-home-directory.md +git commit -m "docs: add persistent home directory migration tutorial" +``` + +--- + +### Task 18: Update `CLAUDE.md` and `README.md` + +**Files:** +- Modify: `CLAUDE.md` +- Modify: `README.md` + +- [ ] **Step 1: Update `CLAUDE.md` Configuration section** + +In `CLAUDE.md`, in the `### Configuration` section, after the existing list of required fields, add: + +```markdown +### Home directory + +RustFox stores all state under a single home directory (default `~/.rustfox`), +resolved as: `RUSTFOX_HOME` env (absolute) → `[general].home` config → `~/.rustfox`. +Layout: `config.toml`, `rustfox.db`, `skills/`, `agents/`, `workspace/` (the +sandbox), `artifacts/`, `user_model.md`. Each path can be pinned to an absolute +location in `config.toml`; unset paths fall back to the home default. Run +isolated instances with `RUSTFOX_HOME=...`. See +`docs/persistent-home-directory.md`. Path resolution lives in `src/home.rs` +(`Config::resolve` writes the resolved absolute paths back into the config). +Bundled skills/agents are seed-copied on first run; `/update-skills` re-syncs +them using `/skills-lock.json`. +``` + +- [ ] **Step 2: Update `README.md`** + +In `README.md`, find the configuration/setup section that mentions the sandbox +directory and add a short note (place it near where `[sandbox]` is described): + +```markdown +> **Persistent home:** RustFox keeps all state under `~/.rustfox` by default +> (config, database, skills, agents, and a durable `workspace/` sandbox). +> Override with the `RUSTFOX_HOME` environment variable or `[general].home`. +> See [docs/persistent-home-directory.md](docs/persistent-home-directory.md). +``` + +(If the README has no obvious sandbox section, add the note under the main setup/usage heading.) + +- [ ] **Step 3: Commit** + +```bash +git add CLAUDE.md README.md +git commit -m "docs: document persistent home directory in CLAUDE.md and README" +``` + +--- + +### Task 19: Full verification pass + +**Files:** none (verification only) + +- [ ] **Step 1: Format** + +Run: `cargo fmt --all` +Then: `cargo fmt --all -- --check` +Expected: no diff. + +- [ ] **Step 2: Lint** + +Run: `cargo clippy --all-targets -- -D warnings` +Expected: no warnings. Fix any (e.g. unused imports left from removed default helpers in Task 6). + +- [ ] **Step 3: Tests** + +Run: `cargo test` +Expected: all unit + integration tests pass, including: +- `home::tests::*` +- `config::tests::*` (incl. `resolve_*`, `load_resolves_paths_to_absolute`) +- `skills::seed::tests::*` +- `skills::update::tests::*` +- `tests/home_sandbox.rs` + +- [ ] **Step 4: Release build** + +Run: `cargo build --release` +Expected: builds cleanly. + +- [ ] **Step 5: Commit any fixups** + +```bash +git add -A +git commit -m "chore: fmt + clippy fixups for persistent home directory" +``` + +--- + +## Self-Review + +**1. Spec coverage:** + +| Spec section | Task(s) | +|---|---| +| §1 Directory model (hybrid home, RUSTFOX_HOME, [general].home, per-path override) | 2, 3, 6, 7 | +| §1 Config discovery `/config.toml` | 9 | +| §1 Multi-instance via RUSTFOX_HOME | 7, 9, 17 | +| §2 Home resolution module + ResolvedPaths + ensure_dirs (0700) | 2, 3, 4 | +| §3 Sandbox = workspace; secrets unreachable | 7, 10 | +| §4 Durable workspace + prompt text | 10 | +| §5 Seed-copy + `/update-skills` hash-diff + .bak + instance-created untouched + hot reload | 11, 12, 13, 14, 15 | +| §6 No auto-move + actionable warning + start-fresh + tutorial | 7 (warnings), 17 (tutorial) | +| §7 Config field changes + example | 6, 16 | +| Testing strategy | 2–5, 7, 8, 10, 11, 12, 19 | +| File structure / docs (CLAUDE/README) | 18 | + +No uncovered spec requirement. + +**2. Placeholder scan:** Every code step contains complete code. The only `todo!()` bodies are intentional red-phase placeholders immediately replaced in the next step of the same task. + +**3. Type consistency:** `resolve_home`, `resolve_data_path`, `PathOrigin`, `ResolvedPaths`, `ensure_dirs`, `LegacyPathWarning`, `default_home` (home.rs); `Config::resolve` returns `Vec`; `SkillLock`, `UpdateReport`, `update_skills`, `hash_skill_dir`, `lock_map_for`, `copy_dir_recursive_pub` (skills); `Agent::reload_skills_and_agents` — names are consistent across the tasks that define and consume them. The empty-PathBuf sentinel convention (Task 2.5) is applied consistently in Tasks 6 and 7. + +--- + +## Execution Handoff + +**Plan complete and saved to `docs/superpowers/plans/2026-05-29-persistent-home-directory.md`. Two execution options:** + +**1. Subagent-Driven (recommended)** — I dispatch a fresh subagent per task, review between tasks, fast iteration. + +**2. Inline Execution** — Execute tasks in this session using executing-plans, batch execution with checkpoints. + +**Which approach?** diff --git a/docs/superpowers/plans/2026-06-01-multi-directory-skills-loading.md b/docs/superpowers/plans/2026-06-01-multi-directory-skills-loading.md new file mode 100644 index 0000000..262f99d --- /dev/null +++ b/docs/superpowers/plans/2026-06-01-multi-directory-skills-loading.md @@ -0,0 +1,771 @@ +# Multi-Directory Skill Loading Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Support two skill layers — instance (custom/writable, `~/.rustfox/skills/`) and bundled (read-only templates, `./skills/`) — with instance shadowing bundled on name collision. + +**Architecture:** `SkillRegistry` gains two internal maps (`instance_skills` + `bundled_skills`) and a `skill_base_dirs` lookup. `load_skills_from_dir` takes a `SkillSource` enum to tag skills. Agent tools (`read_skill_file`, `write_skill_file`, etc.) resolve via the registry's source tracking. Config adds `bundled_directory` fields for skills and agents. + +**Tech Stack:** Rust, tokio, serde, `PathBuf` path resolution + +--- + +### Task 1: Add `SkillSource` and refactor `SkillRegistry` + +**Files:** +- Modify: `src/skills/mod.rs` + +- [ ] **Step 1: Add `SkillSource` enum and refactor `SkillRegistry`** + +Add `SkillSource` and replace the single `skills: HashMap` with two maps plus a `skill_base_dirs` lookup: + +```rust +// Add to src/skills/mod.rs + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SkillSource { + Instance, + Bundled, +} + +#[derive(Debug, Clone)] +pub struct SkillRegistry { + instance_skills: HashMap, + bundled_skills: HashMap, + /// Maps skill name → absolute base directory for read_skill_file path resolution. + /// Uses the source directory (e.g. ~/.rustfox/skills/ or /project/skills/). + skill_base_dirs: HashMap, +} +``` + +- [ ] **Step 2: Update `SkillRegistry` methods** + +```rust +impl SkillRegistry { + pub fn new() -> Self { + Self { + instance_skills: HashMap::new(), + bundled_skills: HashMap::new(), + skill_base_dirs: HashMap::new(), + } + } + + /// Register a skill with its source and base directory. + pub fn register(&mut self, skill: Skill, source: SkillSource, base_dir: PathBuf) { + let name = skill.name.clone(); + match source { + SkillSource::Instance => { + self.instance_skills.insert(name.clone(), skill); + } + SkillSource::Bundled => { + self.bundled_skills.insert(name.clone(), skill); + } + } + self.skill_base_dirs.insert(name.clone(), base_dir); + info!("Registered skill: {} ({:?})", name, source); + } + + /// Instance shadows bundled. + pub fn get(&self, name: &str) -> Option<&Skill> { + self.instance_skills + .get(name) + .or_else(|| self.bundled_skills.get(name)) + } + + /// Returns the source directory for a skill (used by read_skill_file). + pub fn base_dir(&self, name: &str) -> Option<&Path> { + self.skill_base_dirs.get(name).map(|p| p.as_path()) + } + + /// All unique skills (instance shadows bundled, so only instance names appear for duplicates). + pub fn list(&self) -> Vec<&Skill> { + let mut all: Vec<&Skill> = self.instance_skills.values().collect(); + for skill in self.bundled_skills.values() { + if !self.instance_skills.contains_key(&skill.name) { + all.push(skill); + } + } + all + } + + pub fn len(&self) -> usize { + let mut names = std::collections::HashSet::new(); + for name in self.instance_skills.keys() { + names.insert(name); + } + for name in self.bundled_skills.keys() { + names.insert(name); + } + names.len() + } + + pub fn is_empty(&self) -> bool { + self.instance_skills.is_empty() && self.bundled_skills.is_empty() + } +} +``` + +Add the `use std::path::PathBuf;` import at the top of the file. + +- [ ] **Step 3: Update `build_context` and `build_agents_context` to iterate both maps** + +The `build_context` and `build_agents_context` methods iterate `self.skills.values()`. Change them to iterate all unique skills using `self.list()`: + +```rust +pub fn build_context(&self) -> String { + let unique_skills = self.list(); + if unique_skills.is_empty() { + return String::new(); + } + // ... rest of the method using `unique_skills` instead of `self.skills.values()` +} + +pub fn build_agents_context(&self) -> String { + let unique_agents = self.list(); + // ... same change +} +``` + +- [ ] **Step 4: Update tests** + +The existing test helper `make_skill` stays unchanged. Update all `registry.register(make_skill(...))` calls to pass the new `source` and `base_dir` parameters: + +```rust +// In tests: +registry.register( + make_skill("my-skill", "Does things", "content", None), + SkillSource::Instance, + PathBuf::from("/tmp/instance-skills"), +); +``` + +Add an import for `SkillSource` and `PathBuf` in the test module. + +- [ ] **Step 5: Run tests to verify compilation and existing tests pass** + +Run: `cargo test test_build_context -v` +Expected: Tests pass (after fixing all `register` calls) + +- [ ] **Step 6: Commit** + +```bash +git add src/skills/mod.rs +git commit -m "feat(skills): add SkillSource and refactor SkillRegistry with instance/bundled layers" +``` + +--- + +### Task 2: Update `load_skills_from_dir` to accept `SkillSource` + +**Files:** +- Modify: `src/skills/loader.rs` + +- [ ] **Step 1: Add `SkillSource` and `base_dir` parameter** + +```rust +use super::{Skill, SkillRegistry, SkillSource}; +use std::path::{Path, PathBuf}; + +pub async fn load_skills_from_dir( + dir: &Path, + source: SkillSource, + base_dir: PathBuf, +) -> Result { + let mut registry = SkillRegistry::new(); + // ... existing logic, but pass source and base_dir to register: + // registry.register(skill, source, base_dir.clone()); + // ... rest unchanged +} +``` + +The `base_dir` parameter is the root skills directory (e.g. `~/.rustfox/skills/` or `/project/skills/`). This is what gets stored in `skill_base_dirs` for `read_skill_file` resolution. + +- [ ] **Step 2: Update `register` call inside `load_skills_from_dir`** + +Change: +```rust +registry.register(skill) +``` +To: +```rust +registry.register(skill, source, base_dir.clone()) +``` + +- [ ] **Step 3: Update all callers of `load_skills_from_dir`** + +These are in: +- `src/agent.rs` lines 148 and 152 (`reload_skills_and_agents`) +- `src/agent.rs` lines 1741 and 1901 (`reload_skills` / `reload_agents` tool handlers) +- `src/main.rs` lines 133 and 137 (startup loading) +- `src/platform/telegram.rs` line 247 (`reload_skills_and_agents` after update — this one goes through `reload_skills_and_agents` which is handled separately) + +But actually, `reload_skills_and_agents` is a single method in `agent.rs` that calls `load_skills_from_dir`. And the tool handlers (`reload_skills`, `reload_agents`) also call it directly. + +We'll fix all callers in a later task. For now just update the function signature. + +- [ ] **Step 4: Update the test in loader.rs that calls `load_skills_from_dir`** + +```rust +let skills = load_skills_from_dir(dir.path(), SkillSource::Instance, dir.path().to_path_buf()).await.unwrap(); +``` + +- [ ] **Step 5: Run tests** + +Run: `cargo test test_skill_with_supervisor_block_loads_workflow_hint -v` +Expected: Fails because callers not yet updated (expected; we'll fix them next) + +- [ ] **Step 6: Commit** + +```bash +git add src/skills/loader.rs +git commit -m "feat(skills): add SkillSource and base_dir to load_skills_from_dir" +``` + +--- + +### Task 3: Add `bundled_directory` config fields + +**Files:** +- Modify: `src/config.rs` + +- [ ] **Step 1: Add `bundled_directory` to `SkillsConfig` and `AgentsConfig`** + +```rust +#[derive(Debug, Deserialize, Clone)] +pub struct SkillsConfig { + #[serde(default)] + pub directory: PathBuf, + /// Bundled skills directory (read-only templates, default CWD-relative ./skills/). + #[serde(default = "default_bundled_skills_dir")] + pub bundled_directory: PathBuf, +} + +#[derive(Debug, Deserialize, Clone)] +pub struct AgentsConfig { + #[serde(default)] + pub directory: PathBuf, + /// Bundled agents directory (read-only templates, default CWD-relative ./agents/). + #[serde(default = "default_bundled_agents_dir")] + pub bundled_directory: PathBuf, +} +``` + +Add default functions: +```rust +fn default_bundled_skills_dir() -> PathBuf { + PathBuf::from("skills") +} + +fn default_bundled_agents_dir() -> PathBuf { + PathBuf::from("agents") +} +``` + +- [ ] **Step 2: Update default constructors** + +```rust +fn default_skills_config() -> SkillsConfig { + SkillsConfig { + directory: PathBuf::new(), + bundled_directory: default_bundled_skills_dir(), + } +} + +fn default_agents_config() -> AgentsConfig { + AgentsConfig { + directory: PathBuf::new(), + bundled_directory: default_bundled_agents_dir(), + } +} +``` + +- [ ] **Step 3: Commit** + +```bash +git add src/config.rs +git commit -m "feat(config): add bundled_directory to SkillsConfig and AgentsConfig" +``` + +--- + +### Task 4: Update agent tools for multi-directory resolution + +**Files:** +- Modify: `src/agent.rs` + +- [ ] **Step 1: Add a helper to resolve skill/agent file paths via registry** + +Add a method to `Agent` that looks up a skill name in the registry and returns the correct base directory: + +```rust +/// Resolve the base directory for a skill/agent by checking the registry. +/// Falls back to the configured directory if not found (for newly-created skills). +fn resolve_skill_base_dir(&self, name: &str, config_dir: &Path, skills_lock: &SkillRegistry) -> PathBuf { + skills_lock + .base_dir(name) + .unwrap_or(config_dir) + .to_path_buf() +} +``` + +- [ ] **Step 2: Update `read_skill_file` tool (lines 1654-1698)** + +The current code constructs `target` using `self.config.skills.directory` directly. Change it to use the registry: + +```rust +"read_skill_file" => { + // ... name/path validation unchanged ... + + // Resolve via registry (instance shadows bundled) + let skills_lock = self.skills.read().await; + let base_dir = self.resolve_skill_base_dir(&skill_name, &self.config.skills.directory, &skills_lock); + let target = base_dir.join(&skill_name).join(&relative_path); + + // Canonicalize check against the resolved base dir (not the config dir) + if let Ok(base_canonical) = base_dir.canonicalize() { + if let Ok(target_canonical) = target.canonicalize() { + if !target_canonical.starts_with(&base_canonical) { + return format!("Access denied: path '{}' resolves outside the skills directory", target.display()); + } + } + } + + // ... read unchanged ... +} +``` + +- [ ] **Step 3: Update `write_skill_file` tool (lines 1700-1738)** + +`write_skill_file` always writes to the instance directory. The current code already uses `self.config.skills.directory` which is the instance dir. **No change needed** for the write path. But add a reload step after writing: + +```rust +"write_skill_file" => { + // ... validation and write unchanged ... + + match tokio::fs::write(&target, &content).await { + Ok(()) => { + info!("Skill file written: {}", target.display()); + + // Reload single skill into registry so it's immediately available + let skill_dir = target.parent().and_then(|p| p.parent()).unwrap_or(&target); + if let Ok(skill_registry) = crate::skills::loader::load_skills_from_dir( + skill_dir, + crate::skills::SkillSource::Instance, + self.config.skills.directory.clone(), + ).await { + if let Some(skill) = skill_registry.list().into_iter().next() { + // Merge this single skill into the existing registry + let mut skills = self.skills.write().await; + skills.register(skill.clone(), crate::skills::SkillSource::Instance, self.config.skills.directory.clone()); + } + } + + format!("Written: {}", target.display()) + } + Err(e) => format!("Failed to write skill file: {}", e), + } +} +``` + +Actually, this is overly complex. A simpler approach: after write, call `load_skills_from_dir` on the instance directory to reload the entire instance layer, preserving the bundled layer: + +```rust +// After writing, reload just the instance layer +let instance_dir = self.config.skills.directory.clone(); +if let Ok(new_instance) = crate::skills::loader::load_skills_from_dir( + &instance_dir, + crate::skills::SkillSource::Instance, + instance_dir, +).await { + let mut skills = self.skills.write().await; + // Replace only instance skills, keep bundled + skills.instance_skills = new_instance.instance_skills; + // Also update base_dirs for instance skills + for name in skills.instance_skills.keys() { + skills.skill_base_dirs.insert(name.clone(), instance_dir.clone()); + } +} +``` + +- [ ] **Step 4: Update `read_agent_file` tool (lines 1817-1858)** + +Same pattern as `read_skill_file` — resolve via agents registry: + +```rust +"read_agent_file" => { + // ... name/path validation unchanged ... + + let agents_lock = self.agents.read().await; + let base_dir = self.resolve_skill_base_dir(&agent_name, &self.config.agents.directory, &agents_lock); + let target = base_dir.join(&agent_name).join(&relative_path); + + // ... canonicalize check against base_dir ... + // ... read unchanged ... +} +``` + +- [ ] **Step 5: Update `write_agent_file` tool (lines 1860-1898)** + +Always writes to instance dir — no path change needed. Add the same reload-after-write pattern as `write_skill_file`. + +- [ ] **Step 6: Update `reload_skills` tool (line 1739-1751)** + +Change to reload **both** layers: + +```rust +"reload_skills" => { + use crate::skills::loader::load_skills_from_dir; + use crate::skills::SkillSource; + + let instance_dir = self.config.skills.directory.clone(); + let bundled_dir = self.config.skills.bundled_directory.clone(); + + let instance_reg = load_skills_from_dir(&instance_dir, SkillSource::Instance, instance_dir).await; + let bundled_reg = load_skills_from_dir(&bundled_dir, SkillSource::Bundled, bundled_dir).await; + + let mut skills = self.skills.write().await; + // Clear and rebuild from both layers + if let Ok(reg) = instance_reg { + skills.instance_skills = reg.instance_skills; + for (k, v) in ®.skill_base_dirs { + skills.skill_base_dirs.insert(k.clone(), v.clone()); + } + } + if let Ok(reg) = bundled_reg { + skills.bundled_skills = reg.bundled_skills; + for (k, v) in ®.skill_base_dirs { + // Don't overwrite instance entries + skills.skill_base_dirs.entry(k.clone()).or_insert_with(|| v.clone()); + } + } + + let count = skills.len(); + info!("Skills reloaded: {} skill(s) active ({:?} instance, {:?} bundled)", + count, skills.instance_skills.len(), skills.bundled_skills.len()); + format!("Skills reloaded. {} skill(s) now active.", count) +} +``` + +- [ ] **Step 7: Update `reload_agents` tool (line 1899-1909)** + +Same pattern as `reload_skills` but using agents config: + +```rust +"reload_agents" => { + use crate::skills::loader::load_skills_from_dir; + use crate::skills::SkillSource; + + let instance_dir = self.config.agents.directory.clone(); + let bundled_dir = self.config.agents.bundled_directory.clone(); + + let instance_reg = load_skills_from_dir(&instance_dir, SkillSource::Instance, instance_dir).await; + let bundled_reg = load_skills_from_dir(&bundled_dir, SkillSource::Bundled, bundled_dir).await; + + let mut agents = self.agents.write().await; + // ... same merge logic as reload_skills ... + // ... format result ... +} +``` + +- [ ] **Step 8: Update `reload_skills_and_agents` (line 144-157)** + +```rust +pub async fn reload_skills_and_agents(&self) -> (usize, usize) { + use crate::skills::loader::load_skills_from_dir; + use crate::skills::SkillSource; + + // Skills: load both layers, merge + let s_instance_dir = self.config.skills.directory.clone(); + let s_bundled_dir = self.config.skills.bundled_directory.clone(); + let s_instance = load_skills_from_dir(&s_instance_dir, SkillSource::Instance, s_instance_dir.clone()).await; + let s_bundled = load_skills_from_dir(&s_bundled_dir, SkillSource::Bundled, s_bundled_dir.clone()).await; + + { + let mut skills = self.skills.write().await; + if let Ok(reg) = s_instance { + skills.instance_skills = reg.instance_skills; + for (k, v) in ®.skill_base_dirs { + skills.skill_base_dirs.insert(k.clone(), v.clone()); + } + } + if let Ok(reg) = s_bundled { + skills.bundled_skills = reg.bundled_skills; + for (k, v) in ®.skill_base_dirs { + skills.skill_base_dirs.entry(k.clone()).or_insert_with(|| v.clone()); + } + } + } + let s_count = self.skills.read().await.len(); + + // Agents: same pattern + let a_instance_dir = self.config.agents.directory.clone(); + let a_bundled_dir = self.config.agents.bundled_directory.clone(); + let a_instance = load_skills_from_dir(&a_instance_dir, SkillSource::Instance, a_instance_dir.clone()).await; + let a_bundled = load_skills_from_dir(&a_bundled_dir, SkillSource::Bundled, a_bundled_dir.clone()).await; + + { + let mut agents = self.agents.write().await; + if let Ok(reg) = a_instance { + agents.instance_skills = reg.instance_skills; + for (k, v) in ®.skill_base_dirs { + agents.skill_base_dirs.insert(k.clone(), v.clone()); + } + } + if let Ok(reg) = a_bundled { + agents.bundled_skills = reg.bundled_skills; + for (k, v) in ®.skill_base_dirs { + agents.skill_base_dirs.entry(k.clone()).or_insert_with(|| v.clone()); + } + } + } + let a_count = self.agents.read().await.len(); + + (s_count, a_count) +} +``` + +- [ ] **Step 9: Commit** + +```bash +git add src/agent.rs +git commit -m "feat(agent): update skill/agent tools for multi-directory resolution" +``` + +--- + +### Task 5: Update startup to load both layers + +**Files:** +- Modify: `src/main.rs` +- Modify: `src/platform/telegram.rs` + +- [ ] **Step 1: Update `main.rs` startup loading (lines 133-138)** + +Change: +```rust +let skills = load_skills_from_dir(&config.skills.directory).await?; +let agents = load_skills_from_dir(&config.agents.directory).await?; +``` +To: +```rust +use rustfox::skills::SkillSource; + +let mut skills = load_skills_from_dir( + &config.skills.directory, + SkillSource::Instance, + config.skills.directory.clone(), +).await?; +let bundled_skills = load_skills_from_dir( + &config.skills.bundled_directory, + SkillSource::Bundled, + config.skills.bundled_directory.clone(), +).await?; +// Merge bundled into skills registry +for skill in bundled_skills.list() { + skills.register( + skill.clone(), + SkillSource::Bundled, + config.skills.bundled_directory.clone(), + ); +} + +let mut agents = load_skills_from_dir( + &config.agents.directory, + SkillSource::Instance, + config.agents.directory.clone(), +).await?; +let bundled_agents = load_skills_from_dir( + &config.agents.bundled_directory, + SkillSource::Bundled, + config.agents.bundled_directory.clone(), +).await?; +for agent in bundled_agents.list() { + agents.register( + agent.clone(), + SkillSource::Bundled, + config.agents.bundled_directory.clone(), + ); +} +``` + +- [ ] **Step 2: Verify `Agent::new` call still works** + +The `Agent::new` signature hasn't changed (still takes `skills: SkillRegistry` and `agents: SkillRegistry`). The merged registry is passed as-is. No change needed. + +- [ ] **Step 3: `src/platform/telegram.rs` — `/update-skills` uses `reload_skills_and_agents` already** + +Lines 247-248 call `agent.reload_skills_and_agents().await` which we already updated in Task 4 Step 8. No changes needed in telegram.rs beyond what was already done. + +- [ ] **Step 4: Commit** + +```bash +git add src/main.rs +git commit -m "feat(main): load bundled skills at startup alongside instance skills" +``` + +--- + +### Task 6: Update learning module + +**Files:** +- Modify: `src/learning.rs` + +- [ ] **Step 1: Update `patch_skill` to always use instance dir** + +The `patch_skill` function in `src/learning.rs` uses `config.skills.directory` to find the skill directory. This is already the instance dir, which is correct — `patch_skill` should only modify instance skills, never bundled ones. + +Verify at lines 194-261 that the path resolution uses `config.skills.directory` (instance). It does — no change needed. + +But after patching, it reloads the skill into the registry. Check that it uses `registry.register()` with the old (single-map) API and update it: + +```rust +// In patch_skill / self_patch_skill, change: +// skills.write().await.register(...) -- old single-map API +// To: +skills.write().await.register(skill, SkillSource::Instance, config.skills.directory.clone()); +``` + +- [ ] **Step 2: Verify test compilation** + +Run: `cargo test test_self_patch_skill -v` +Expected: Pass + +- [ ] **Step 3: Commit** + +```bash +git add src/learning.rs +git commit -m "fix(learning): update patch_skill to use SkillSource::Instance" +``` + +--- + +### Task 7: Add tests for multi-directory skills + +**Files:** +- Add tests in: `src/skills/mod.rs` (existing test module) +- Add tests in: `src/agent.rs` (existing test module) + +- [ ] **Step 1: Add unit test for instance-shadow-bundled** + +In `src/skills/mod.rs` tests: + +```rust +#[test] +fn test_instance_shadows_bundled() { + let mut registry = SkillRegistry::new(); + registry.register( + make_skill("duplicate", "Instance version", "instance content", None), + SkillSource::Instance, + PathBuf::from("/instance"), + ); + registry.register( + make_skill("duplicate", "Bundled version", "bundled content", None), + SkillSource::Bundled, + PathBuf::from("/bundled"), + ); + + // get() should return instance version + let skill = registry.get("duplicate").unwrap(); + assert_eq!(skill.content, "instance content"); + assert_eq!(skill.description, "Instance version"); + + // base_dir should return instance path + assert_eq!(registry.base_dir("duplicate").unwrap(), Path::new("/instance")); + + // list() should only include the instance version (not duplicate) + let names: Vec<&str> = registry.list().iter().map(|s| s.name.as_str()).collect(); + assert_eq!(names, vec!["duplicate"]); + assert_eq!(registry.len(), 1); +} +``` + +- [ ] **Step 2: Add unit test for unique-only skills** + +```rust +#[test] +fn test_unique_skills_from_both_layers() { + let mut registry = SkillRegistry::new(); + registry.register( + make_skill("alpha", "Instance only", "", None), + SkillSource::Instance, + PathBuf::from("/instance"), + ); + registry.register( + make_skill("beta", "Bundled only", "", None), + SkillSource::Bundled, + PathBuf::from("/bundled"), + ); + + let names: Vec<&str> = registry.list().iter().map(|s| s.name.as_str()).collect(); + assert!(names.contains(&"alpha")); + assert!(names.contains(&"beta")); + assert_eq!(registry.len(), 2); +} +``` + +- [ ] **Step 3: Add integration test for read_skill_file resolution** + +In `src/agent.rs` tests, add a test that verifies `read_skill_file` searches instance first: + +```rust +#[tokio::test] +async fn test_read_skill_file_checks_instance_before_bundled() { + let dir = tempfile::tempdir().unwrap(); + let instance_dir = dir.path().join("instance-skills"); + let bundled_dir = dir.path().join("bundled-skills"); + + // Create same-named skill in both dirs with different content + tokio::fs::create_dir_all(instance_dir.join("my-skill")).await.unwrap(); + tokio::fs::write(instance_dir.join("my-skill/SKILL.md"), "instance content").await.unwrap(); + tokio::fs::create_dir_all(bundled_dir.join("my-skill")).await.unwrap(); + tokio::fs::write(bundled_dir.join("my-skill/SKILL.md"), "bundled content").await.unwrap(); + + // Load both into registry + let mut registry = SkillRegistry::new(); + let inst = load_skills_from_dir(&instance_dir, SkillSource::Instance, instance_dir.clone()).await.unwrap(); + let bund = load_skills_from_dir(&bundled_dir, SkillSource::Bundled, bundled_dir.clone()).await.unwrap(); + for s in inst.list() { registry.register(s.clone(), SkillSource::Instance, instance_dir.clone()); } + for s in bund.list() { registry.register(s.clone(), SkillSource::Bundled, bundled_dir.clone()); } + + // read_skill_file should find instance version + let base_dir = registry.base_dir("my-skill").unwrap(); + let target = base_dir.join("my-skill/SKILL.md"); + let content = tokio::fs::read_to_string(&target).await.unwrap(); + assert_eq!(content, "instance content", "instance must shadow bundled"); +} +``` + +- [ ] **Step 4: Run all tests** + +Run: `cargo test` +Expected: All tests pass + +- [ ] **Step 5: Commit** + +```bash +git add src/skills/mod.rs src/agent.rs +git commit -m "test(skills): add tests for multi-directory shadow semantics" +``` + +--- + +### Self-review checklist + +- [ ] **Spec coverage:** Does every section of the design doc have a corresponding task? + - `SkillSource` enum → Task 1 + - `SkillRegistry` refactor → Task 1 + - `load_skills_from_dir` signature change → Task 2 + - Config `bundled_directory` → Task 3 + - Agent tool resolution → Task 4 + - Startup loading → Task 5 + - `patch_skill` → Task 6 + - Tests → Task 7 + +- [ ] **Placeholder scan:** No TBD, TODO, "implement later", or similar. + +- [ ] **Type consistency:** + - `SkillSource::Instance` and `SkillSource::Bundled` are used consistently + - `SkillRegistry::register(skill, source, base_dir)` consistently called + - `load_skills_from_dir(dir, source, base_dir)` signature matches across all callers + - Config field `bundled_directory` consistent between `SkillsConfig` and `AgentsConfig` + - `base_dir()` returns `Option<&Path>` consistently diff --git a/docs/superpowers/specs/2026-05-28-telegram-plan-tool-visuals-design.md b/docs/superpowers/specs/2026-05-28-telegram-plan-tool-visuals-design.md new file mode 100644 index 0000000..6fad0fc --- /dev/null +++ b/docs/superpowers/specs/2026-05-28-telegram-plan-tool-visuals-design.md @@ -0,0 +1,277 @@ +# Telegram Plan and Tool Visuals Design + +## Goal + +Improve RustFox's Telegram verbose-mode experience for agent planning and tool execution. + +When verbose mode is enabled, users should see one clear live progress message while the agent works, then a persistent audit card after completion. Planning tools should render as a readable checklist instead of generic tool-call text. Other tools should still appear in the same progress surface as a compact activity log. + +This design also fixes the current duplicate tool-message bug where tool-call status appears in a separate streamed assistant message and remains after the final response. + +## Decision + +Use a B-style persistent audit card for verbose mode. + +Behavior: + +- During execution, the bot sends one live progress message. +- The live message is edited as planning and tool events arrive. +- `plan_create`, `plan_update`, and `plan_view` update a structured plan checklist area. +- Other tool calls update a compact tool activity area. +- The final assistant answer is sent separately through the existing answer stream. +- On completion, the live progress message is edited into a completed audit summary and kept in the chat. +- Non-verbose mode stays clean and should not show planning/tool audit cards. + +The audit card should be produced only by the tool notifier path. Tool-status text must not be sent into the normal assistant response stream. + +## Evidence + +Telegram behavior supports this direction: + +- `editMessageText` is the right primitive for reducing chat clutter while progress changes. +- `sendChatAction` is ephemeral and useful only as a lightweight wait signal, not as a multi-step progress surface. +- `deleteMessage` has message-age and permission limitations, so persistent final summaries are less brittle than delete-dependent cleanup. +- `sendMessageDraft` exists for temporary 30-second previews, but final user-visible output still needs normal messages. It is a possible future enhancement, not needed for this implementation. + +UX guidance supports showing real steps for long-running work: + +- Immediate feedback matters for waits longer than a short moment. +- Multi-step work should show current step and completed work rather than a static loading message. +- Unknown-length agent runs should avoid fake percentages and instead show actual work observed from events. + +## Current Problem + +Verbose mode currently has two independent progress paths: + +1. `src/agent.rs` emits `ToolEvent::Started` and `ToolEvent::Completed` into `ToolCallNotifier`. +2. `src/agent.rs` also formats a status line with `format_tool_status_line()` and sends it into the normal response stream. + +The Telegram platform then displays both: + +- `ToolCallNotifier` edits a live `Working...` message and currently deletes it at finish. +- The response stream treats tool-status lines as assistant output, creating a separate message that remains in chat. + +The root cause is dual emission, not a Telegram deletion failure. + +## Desired Message Model + +Verbose-mode messages should be: + +1. One progress/audit message owned by `ToolCallNotifier`. +2. One or more final answer messages owned by the existing answer streamer. + +No tool-status line should enter the final answer stream. + +Non-verbose-mode messages should remain: + +1. Existing transient thinking placeholder. +2. Final answer messages. + +## Audit Card Shape + +The live card should favor compact, scan-friendly text that works inside Telegram message constraints. + +During execution: + +```text +Working on your request + +Plan +[>] 1. Gather context +[ ] 2. Update implementation +[ ] 3. Verify behavior + +Current +Running: read_file + +Recent tools +- plan_create: started +- read_file: started +``` + +After completion: + +```text +Completed + +Plan +[x] 1. Gather context +[x] 2. Update implementation +[x] 3. Verify behavior + +Tool activity +- plan_create: completed +- read_file: completed +- cargo test: completed + +Result +Final answer sent below. +``` + +If no planning tools are used, omit the plan section and show only tool activity. + +If no tools are used, the notifier does not need to create an audit card. + +## Plan Rendering + +Planning tools are first-class in the notifier. + +### plan_create + +On `ToolEvent::Started`, parse arguments if possible: + +- `title` +- `steps[]` + +Render a plan checklist immediately, with all steps initially pending. + +On `ToolEvent::Completed`, if the tool result contains a richer serialized plan, prefer the result. If not, keep the parsed arguments as the display model. + +### plan_update + +On `ToolEvent::Started`, parse arguments: + +- `step_id` +- `status` +- optional `notes` + +Optimistically update that step in the notifier display so the user sees the current step quickly. + +On `ToolEvent::Completed`: + +- Keep the optimistic update if the tool succeeded. +- If the tool failed, mark the attempted step as failed in the audit display and include a short failure note. + +### plan_view + +Use as a refresh opportunity. If the result includes checklist text, parse it conservatively or show it as a compact plan snapshot. Do not let parsing failure break notifier rendering. + +## Tool Activity Rendering + +For non-planning tools, use the existing friendly naming direction but keep the log compact. + +Rules: + +- Show the current active tool at the top when available. +- Keep a recent activity list with a bounded number of entries, for example last 8 to 12. +- Collapse repeated tool calls of the same name when useful, for example `read_file x4`. +- Redact or omit long argument previews. +- Never show secrets or raw config values. +- Indicate success or failure after completion. + +## Component Changes + +### `src/platform/tool_notifier.rs` + +Extend `ToolEvent` or add notifier-side parsing so the notifier can maintain a display state: + +```rust +struct ToolDisplayState { + plan: Option, + active_tool: Option, + recent_tools: Vec, + mode: ToolDisplayMode, +} +``` + +`ToolDisplayMode` should support at least: + +- `Live` +- `CompletedPersistent` + +The notifier should format messages from this display state rather than appending plain text lines. + +Change `finish()` behavior in verbose mode: + +- If at least one tool or plan event occurred, edit the existing progress message into the completed audit card. +- Do not delete the message by default for verbose mode. +- If no events occurred, delete or clean up as current behavior allows. + +Keep edit failures non-fatal. If the final edit fails because Telegram cannot edit the message, log the error and leave the last live state visible. + +### `src/agent.rs` + +Remove or gate the stream-status emission that currently calls `format_tool_status_line()` and sends the result to the response stream. + +The agent should continue to emit structured tool events: + +- before tool execution: `ToolEvent::Started` +- after tool execution: `ToolEvent::Completed` + +The normal response stream should contain only final assistant content tokens. + +### `src/platform/telegram.rs` + +Keep the existing verbose-mode notifier wiring: + +- create tool-event channel when verbose mode is enabled +- spawn `ToolCallNotifier` +- pass `tool_event_tx` into `agent.process_message()` + +Make sure the final answer streaming task and the notifier task remain separate. The final answer should not need to know whether the audit card is persistent. + +If the notifier returns an error, the assistant answer should still be sent. + +### `src/tools.rs` + +No behavioral change is required for the planning tools initially. The notifier should parse existing tool-call arguments and results. + +Optional future improvement: return a stable structured JSON result from `plan_create`, `plan_update`, and `plan_view` so display parsing does not depend on human-readable output. + +## Failure Handling + +- If `plan_create` arguments cannot be parsed, show the tool in the generic tool activity log. +- If `plan_update` references an unknown step, show the attempted update in recent tool activity and preserve the prior plan display. +- If message editing is rate-limited or fails, retry only through existing notifier cadence; do not block tool execution. +- If the final answer fails, keep the audit card as diagnostic context and let existing error handling report the answer failure. +- If the agent exceeds iteration limits, the audit card should remain and show the latest known plan/tool state. + +## Security and Privacy + +Verbose mode may expose tool names and argument summaries in Telegram. Keep existing verbose opt-in behavior and avoid increasing exposure in non-verbose mode. + +Notifier rendering must: + +- avoid showing full file contents +- avoid showing raw config values +- avoid showing long command arguments verbatim +- truncate argument previews +- preserve any existing redaction helpers if present + +## Test Plan + +Add focused unit tests before implementation. + +Suggested tests: + +1. Formatting a `plan_create` started event renders a checklist. +2. Formatting a `plan_update` event changes the target step status. +3. Formatting a failed `plan_update` marks or reports failure without panicking. +4. Generic tool events render in recent activity when no plan exists. +5. Completed verbose notifier output is persistent-summary text, not empty or delete-only behavior. +6. `src/agent.rs` no longer sends `format_tool_status_line()` output through the answer stream when verbose tool events are enabled. +7. Non-verbose processing does not create a notifier/audit card. + +Run at minimum: + +```bash +cargo fmt --all -- --check +cargo test +cargo clippy -- -D warnings +``` + +## Non-Goals + +- Do not build a separate Telegram web app or image renderer for this iteration. +- Do not use `sendMessageDraft` yet. +- Do not rewrite the planning tool storage format unless required by tests. +- Do not make audit cards visible in non-verbose mode. +- Do not change final-answer Markdown rendering. + +## Future Enhancements + +- Add a setting to choose between ephemeral verbose progress and persistent verbose audit cards. +- Return structured JSON from planning tools for more robust UI rendering. +- Use Telegram drafts for short-lived progress previews if library support and product behavior are mature enough. +- Add richer plan summaries, such as elapsed time, failed steps, and verification evidence. +- Consider adding `.superpowers/` to `.gitignore` if local brainstorming artifacts should never be committed. diff --git a/docs/superpowers/specs/2026-05-29-persistent-home-directory-design.md b/docs/superpowers/specs/2026-05-29-persistent-home-directory-design.md new file mode 100644 index 0000000..55bf504 --- /dev/null +++ b/docs/superpowers/specs/2026-05-29-persistent-home-directory-design.md @@ -0,0 +1,303 @@ +# Persistent Home Directory (`~/.rustfox`) — Design + +**Date:** 2026-05-29 +**Status:** Approved (design phase) +**Worktree:** `telegram-plan-tool-visuals` + +## Problem + +RustFox today scatters its working state and resolves most paths relative to the +process launch directory: + +- The sandbox (`[sandbox].allowed_directory`) is a **required** path that users + typically point at an ephemeral location like `/tmp/rustfox-sandbox`. Files + the LLM writes there (scripts, programs, notes) are wiped on reboot, so they + cannot be reused over the long term. +- `rustfox.db`, `skills/`, `agents/`, `supervisor/artifacts`, and the learning + `user_model.md` all default to **relative** paths, resolved from the current + working directory. State location therefore depends on where the bot was + launched, and is easy to lose or duplicate. +- There is no first-class notion of an isolated *instance home*, so running two + RustFox instances on one machine for different purposes requires careful + manual path juggling. + +## Goals + +1. Give each RustFox instance a single, persistent **home directory** + (default `~/.rustfox`) that survives reboots. +2. Make the sandbox a **durable workspace** so the LLM can accumulate reusable + scripts, programs, and markdown files over time. +3. Keep **secrets** (`config.toml`, OAuth tokens) and the **SQLite DB** + structurally **outside** the LLM-writable sandbox. +4. Store **instance-created skills/agents** under the home so the work + environment is isolated per instance. +5. Support **multiple instances on one machine**, each with its own home, with + minimal friction (ideally one environment variable). +6. Do not break existing installs; never move user data automatically. + +## Non-Goals + +- OS-level sandboxing (containers, seccomp, namespaces). The sandbox remains a + path-canonicalization boundary, as today. +- Changing the skill/agent *file format* or the agentic loop. +- Auto-migrating or relocating any existing user files on disk. + +--- + +## Reference research + +Surveyed how comparable AI assistants and the platform convention handle +persistent working directories: + +- **Claude Code / Codex CLI** use a single home dot-directory (`~/.claude`, + `~/.codex`) holding config, skills, agents, memory, and project state. + Simple, discoverable, easy to back up or wipe. (Claude Code is criticized for + *not* following XDG, which informed our decision to keep a single root but + allow overrides.) Claude Code also treats the **working directory** as the + agent's primary context and operation boundary, and supports additional + writable roots via `--add-dir`. +- **XDG Base Directory Specification** splits state by purpose + (`$XDG_CONFIG_HOME`, `$XDG_DATA_HOME`, `$XDG_STATE_HOME`, `$XDG_CACHE_HOME`). + It is the "correct" Linux convention and is env-var overridable, but spreads + files across four locations, which makes per-instance isolation and "delete + everything" harder. + +**Takeaway adopted:** a single discoverable root (Claude/Codex style) for +isolation and simplicity, plus an environment-variable + per-path override +escape hatch (the spirit of XDG's overridability) for multi-instance and +power-user needs. + +--- + +## Design + +### 1. Directory model (Hybrid) + +**Home root resolution order (first match wins):** + +1. `RUSTFOX_HOME` environment variable (must be an absolute path). +2. `[general].home` in `config.toml` (absolute path). +3. Default: `~/.rustfox` (expanded from the OS home directory). + +**Default layout under the home root:** + +``` +~/.rustfox/ + config.toml # optional config search location (secrets) — OUTSIDE sandbox + rustfox.db # SQLite memory/embeddings — OUTSIDE sandbox + skills/ # skill source of truth (seeded + writable) + agents/ # agent source of truth (seeded + writable) + workspace/ # THE SANDBOX — file/command tools confined here + artifacts/ # supervisor artifacts + user_model.md # learning user model +``` + +**Per-path override semantics.** Each data path +(`sandbox.allowed_directory`, `memory.database_path`, `skills.directory`, +`agents.directory`, `supervisor.artifacts_dir`, `learning.user_model_path`) +becomes **optional**. Resolution per path: + +- **Unset** → resolved under the home root using the default subpath above. +- **Set, absolute** → used verbatim (full backward compatibility). +- **Set, relative** → resolved from the current working directory (legacy + behavior) and a one-time **actionable startup warning** is emitted (see §5). + +**Config file discovery.** `Config::load` keeps its current behavior (CLI arg, +else `./config.toml`) and additionally falls back to `/config.toml` when +neither is present. The explicit CLI path always wins. + +**Multi-instance.** A second isolated instance is one line: + +```bash +RUSTFOX_HOME="$HOME/.rustfox-work" cargo run +``` + +This instance gets its own `workspace/`, `skills/`, `agents/`, `rustfox.db`, +and artifacts — fully isolated from the default instance. + +### 2. Home resolution module + +Introduce a small, focused unit responsible solely for resolving the home root +and the per-path defaults. It has one clear job: given the environment, the +config, and the OS home, produce the set of absolute resolved paths plus the +list of legacy-relative-path warnings to print. + +- **Input:** `RUSTFOX_HOME` env, `Config` (raw, with `Option` fields), + OS home dir. +- **Output:** a `ResolvedPaths` struct with absolute paths for home, workspace + (sandbox), db, skills, agents, artifacts, user_model; plus a + `Vec`. +- **Side effect (explicit, not hidden):** `ensure_dirs()` creates the home and + its standard subdirectories with `0700` permissions on Unix (mirroring XDG's + guidance for user-private dirs). + +This keeps `config.rs` parsing dumb (just `Option` fields) and isolates +all path-resolution policy in one testable place. + +### 3. Sandbox boundary + +`file_read`, `file_write`, and `run_command` (and any other filesystem/command +tool in `src/tools.rs`) remain confined by `validate_sandbox_path`, but the +sandbox root is now the resolved **`workspace/`** path under the home. + +- `[sandbox].allowed_directory` becomes optional; default `/workspace`. +- Because `config.toml`, `rustfox.db`, and OAuth token state live **above** + `workspace/`, they are structurally unreachable by file/command tools — even + under prompt injection. +- Skills/agents are written through their **dedicated** tool path + (`write_skill_file` + the learning extractor), not the raw sandbox file + tools, so they intentionally live outside `workspace/`. + +### 4. Workspace as durable LLM scratch space + +`workspace/` persists across restarts (no more `/tmp` wipe). The LLM may +accumulate reusable scripts, programs, and markdown notes there. The layout is +**freeform** — RustFox does not impose or enforce subdirectories; the LLM +organizes the space as it sees fit. The system prompt's sandbox description is +updated to reflect that the workspace is persistent and reusable. + +### 5. Skills/agents sourcing: seed + explicit update + +The instance `skills/` directory is the **single load source** at runtime +(`load_skills_from_dir`). Agents follow the identical model with `agents/`. + +**Seeding (first run / empty dir).** When the resolved `skills/` (resp. +`agents/`) directory does not exist or is empty, RustFox seed-copies the +**bundled** skills shipped with the installation into it. The bundled source is +the directory adjacent to the executable / repo (the existing `skills/` and +`agents/` folders). Seeding also copies `skills-lock.json` into the home as +`skills-lock.json` so future updates can diff. + +**Explicit update (`/update-skills` Telegram command — no auto-sync).** Running +`/update-skills` re-syncs bundled → instance using content hashes from +`skills-lock.json`: + +- **Bundled, unchanged locally** (instance file hash matches the lock) → + overwrite with the new bundled version. +- **Bundled, locally modified** (hash differs from the lock) → back up the + instance copy to `/SKILL.md.bak` (and other changed files to `*.bak`), + then write the new bundled version. Never silent data loss. +- **Instance-created** (skill name absent from `skills-lock.json`) → never + touched. + +The command reports a summary: updated / backed-up / skipped counts. After +sync it hot-reloads the registries (reusing the existing `reload_skills` / +`reload_agents` machinery). `/update-skills` is registered alongside the other +text commands in `src/platform/telegram.rs`. + +### 6. Migration & compatibility (no auto-move) + +- **No automatic file moves.** RustFox never relocates an existing + `./rustfox.db`, `./skills`, etc. +- **Explicit config paths honored** verbatim (absolute) — existing deployments + that pin paths are unaffected. +- **Actionable startup warning.** When any path is unset *and* a legacy file is + detected in the launch CWD (e.g. `./rustfox.db`, `./skills/`), or when a path + is set to a relative value, print a clear warning that includes: + - the old resolved path vs the new default path, and + - the exact shell command to copy data + (e.g. `cp ./rustfox.db ~/.rustfox/rustfox.db`), and + - the exact `config.toml` line to pin the old path instead. +- **Start-fresh path.** Doing nothing and letting RustFox use the new + `~/.rustfox` defaults is itself the "start fresh" option (a brand-new empty + workspace, freshly seeded skills, a new DB). This is documented explicitly. +- **Tutorial doc.** Add `docs/persistent-home-directory.md` covering: + 1. The new home layout and what each subdir holds. + 2. How to **migrate existing data** into `~/.rustfox` (DB, skills, agents, + artifacts, user model) with copy commands. + 3. How to **start fresh** instead (and how to keep the old location by + pinning paths in `config.toml`). + 4. How to run **multiple instances** via `RUSTFOX_HOME`. + +### 7. Config & example updates + +- `SandboxConfig.allowed_directory`: `PathBuf` → `Option`. +- `MemoryConfig.database_path`, `SkillsConfig.directory`, + `AgentsConfig.directory`, `SupervisorConfig.artifacts_dir`, + `LearningConfig.user_model_path`: make optional (or keep the field but resolve + through the home module, preferring an explicit value when present). +- Add `GeneralConfig.home: Option`. +- Update `config.example.toml`: comment out the now-optional paths, document the + `~/.rustfox` defaults, `RUSTFOX_HOME`, and `[general].home`. + +--- + +## Data flow (resolution at startup) + +``` +RUSTFOX_HOME / [general].home / ~/.rustfox + │ + ▼ + resolve home root ──► ensure_dirs() (0700) + │ + ▼ + for each path field: + unset → / + absolute → use verbatim + relative → use as-is (CWD) + queue legacy warning + │ + ▼ + ResolvedPaths { home, workspace, db, skills, agents, artifacts, user_model } + │ + ├─► seed skills/ + agents/ if empty + ├─► print queued legacy warnings (with copy/pin commands) + └─► hand resolved absolute paths to MemoryStore, sandbox tools, + skill/agent loaders, supervisor, learning +``` + +## Error handling + +- `RUSTFOX_HOME` set but not absolute → log a warning and ignore it (fall back + to config/`~/.rustfox`), consistent with XDG's "ignore relative" rule. +- OS home directory undeterminable → fail fast with a clear error (the home + root cannot be resolved). +- Directory creation failure (permissions) → fail fast with the offending path, + as the bot cannot operate without its home. +- `/update-skills` with no bundled source available → report the error to the + user; do not modify the instance dir. +- Seeding is best-effort per file; a failed copy logs a warning and continues, + leaving any successfully copied files in place. + +## Testing strategy + +Unit tests (in the home-resolution module and `config.rs`): + +- Home resolution precedence: env > config > default. +- Relative `RUSTFOX_HOME` is ignored. +- Per-path override: unset → default subpath; absolute → verbatim; relative → + used as-is + warning queued. +- `ensure_dirs` creates the expected tree. + +Integration tests (`tests/`): + +- Sandbox confinement still rejects paths outside the resolved `workspace/` + (extend existing sandbox tests against the new default root). +- Seeding populates an empty `skills/`/`agents/` from a fixture bundle. +- `/update-skills` hash logic: unchanged → overwritten; modified → `.bak` + created + updated; instance-only → untouched. Drive via the update function + with a temp home + temp bundle + temp lock file. +- Two distinct `RUSTFOX_HOME` values yield fully isolated resolved path sets. + +## File structure (created / modified) + +- **Create:** `src/home.rs` (or `src/paths.rs`) — home root + per-path + resolution, `ResolvedPaths`, `ensure_dirs`, legacy-warning collection. +- **Create:** `docs/persistent-home-directory.md` — migration & multi-instance + tutorial. +- **Modify:** `src/config.rs` — optional path fields, `[general].home`, + resolution hook in `Config::load` (or a new `Config::resolve_paths`). +- **Modify:** `src/main.rs` — call home resolution, ensure dirs, seed skills/ + agents, print warnings, pass resolved paths downstream. +- **Modify:** `src/tools.rs` — sandbox root sourced from resolved `workspace/`. +- **Create/Modify:** skill-update logic (likely `src/skills/loader.rs` or a new + `src/skills/update.rs`) — seed + hash-diff update using `skills-lock.json`. +- **Modify:** `src/platform/telegram.rs` — register `/update-skills` command + and surface it in `/start` help text. +- **Modify:** `config.example.toml` — document optional paths, `RUSTFOX_HOME`, + `[general].home`, persistent workspace. +- **Modify:** `CLAUDE.md` / `README.md` — note the new home model. + +## Open questions + +None blocking. (Whether the home-resolution unit lives in `src/home.rs` vs +`src/paths.rs` is a naming detail to settle during planning.) diff --git a/docs/superpowers/specs/2026-06-01-multi-directory-skills-loading-design.md b/docs/superpowers/specs/2026-06-01-multi-directory-skills-loading-design.md new file mode 100644 index 0000000..44294c4 --- /dev/null +++ b/docs/superpowers/specs/2026-06-01-multi-directory-skills-loading-design.md @@ -0,0 +1,181 @@ +# Multi-Directory Skill Loading — Design + +**Date:** 2026-06-01 +**Status:** Approved (design phase) +**Worktree:** `telegram-plan-tool-visuals` + +## Problem + +RustFox skills are loaded from a single instance directory (`config.skills.directory`, +default `~/.rustfox/skills/`). Bundled skills (shipped with the project at `./skills/`) +are seed-copied into this directory on first run, mixing predefined templates with +user/custom skills in one flat namespace. This creates several issues: + +1. **No read-only source of truth.** Once seeded, bundled skills are indistinguishable + from user-created skills. `/update-skills` must use content-hash lock files to + detect which skills are "original" vs modified, adding complexity. +2. **Custom skills can be overwritten.** If a future bundled release adds a skill + with the same name as a user-created one, `/update-skills` backs it up as `*.bak` + but the user's work is displaced. +3. **No way to restore a deleted bundled skill.** If a user deletes a bundled skill + from the instance dir, there's no mechanism to recover it short of re-seeding. +4. **CWD-dependent bundled path.** The bundled path `PathBuf::from("skills")` is + relative to the process working directory, which can differ from the project root + when running as a systemd service or from a different directory. + +## Goals + +1. Keep bundled skills as a **read-only fallback layer** — always available, never + accidentally modified or deleted. +2. Allow the agent to create **custom skills** in the instance directory that shadow + bundled ones of the same name. +3. Provide a **single lookup API** that checks instance → bundled without per-tool + fallback logic. +4. Fix the CWD-dependent bundled path issue. +5. Apply the same design to agents (`read_agent_file` / `write_agent_file`). + +## Non-Goals + +- Changing the SKILL.md file format or frontmatter schema. +- Changing the skill/agent loading from markdown files. +- Auto-migration of existing instance-only skills. +- Supporting more than two skill layers (instance + bundled). + +## Design + +### Layer model + +Skills are resolved from two directories in priority order: + +| Priority | Layer | Path | Writable | Purpose | +|----------|-------|------|----------|---------| +| 1 (high) | Instance | `config.skills.directory` → `~/.rustfox/skills/` | ✅ Yes | User/custom skills created by the agent | +| 2 (low) | Bundled | `/skills/` | ❌ No | Read-only templates shipped with the project | + +**Shadow semantics:** Instance layer shadows bundled. A skill named `"thread-writer"` +in the instance dir completely replaces one with the same name in the bundled dir. +The bundled original remains on disk but is invisible to the agent. + +### SkillRegistry changes + +```rust +pub enum SkillSource { + Instance, + Bundled, +} + +pub struct SkillRegistry { + instance_skills: HashMap, + bundled_skills: HashMap, + /// Maps skill name → absolute base directory for read_skill_file resolution. + skill_base_dirs: HashMap, +} + +impl SkillRegistry { + /// Instance shadows bundled. + pub fn get(&self, name: &str) -> Option<&Skill> { + self.instance_skills.get(name) + .or_else(|| self.bundled_skills.get(name)) + } + + /// Returns the source directory for a skill (used by read_skill_file). + pub fn base_dir(&self, name: &str) -> Option<&Path> { + self.skill_base_dirs.get(name).map(|p| p.as_path()) + } + + /// All unique skills (instance names shadow bundled). + pub fn list(&self) -> Vec<&Skill> { /* ... */ } +} +``` + +### Config changes + +```rust +pub struct SkillsConfig { + /// Instance skills directory (user/custom, writable). + pub directory: PathBuf, + /// Bundled skills directory (read-only templates). + /// Defaults to CWD-relative "./skills/". Can be overridden in config.toml. + pub bundled_directory: PathBuf, +} +``` + +Defaults: +- `directory`: empty → resolved to `/skills/` (unchanged) +- `bundled_directory`: `"./skills"` — CWD-relative, same as current `PathBuf::from("skills")` + +Resolution in `Config::resolve()`: `bundled_directory` is preserved as-is (CWD-relative), +matching the existing convention. Users who run from a non-project CWD can set it explicitly: + +```toml +[skills] +bundled_directory = "/opt/RustFox/skills" +``` + +### Agent tool resolution + +**`read_skill_file(skill_name, relative_path)`:** +1. Validate skill name and relative path (unchanged `validate_skill_name` + `validate_skill_path`) +2. Look up `skill_name` in `skill_base_dirs` → get the absolute base directory +3. Construct target: `//` +4. Canonicalize target + `starts_with(base_dir)` escape check +5. Read and return content +6. If skill is not in registry (e.g., race condition), fall back to checking instance dir first, then bundled dir + +**`write_skill_file(skill_name, relative_path, content)`:** +1. Validate skill name and relative path (unchanged) +2. Target path: `//` (always instance dir) +3. Create parent dirs, write file +4. Reload the single skill into the instance layer of the registry + +**`reload_skills`:** +1. `load_skills_from_dir(instance_dir, SkillSource::Instance)` +2. `load_skills_from_dir(bundled_dir, SkillSource::Bundled)` +3. Merge into registry (instance shadows bundled) + +### Startup and update flow + +**Startup (`main.rs`):** +1. `seed_dir_if_empty(bundled_path, instance_path)` — unchanged, seeds bundled skills into instance on first run +2. Load instance skills → load bundled skills → merge into `SkillRegistry` +3. Lock files: seed both `skills-lock.json` and `agents-lock.json` + +**`/update-skills`:** +1. `update_skills(bundled_skills, instance_skills, lock_path)` — unchanged sync logic +2. `update_skills(bundled_agents, instance_agents, lock_path)` — unchanged +3. Reload both layers + +**Custom skills created by agent:** +- Written directly to instance dir via `write_skill_file` +- Reloaded individually into `instance_skills` map +- Never touched by seeding or update (only bundled→instance sync touches instance) + +### Agents (parallel design) + +The same layering applies to agents: + +| Layer | Path | Writable | +|-------|------|----------| +| Instance | `config.agents.directory` → `~/.rustfox/agents/` | ✅ Yes | +| Bundled | `/agents/` | ❌ No | + +`read_agent_file`, `write_agent_file` follow identical logic using the agent registry. + +## Files changed + +| File | Change | +|------|--------| +| `src/skills/mod.rs` | `SkillSource` enum, `SkillRegistry` gains `instance_skills` / `bundled_skills` / `skill_base_dirs` | +| `src/skills/loader.rs` | `load_skills_from_dir` takes `SkillSource` parameter; returns skills keyed by source | +| `src/config.rs` | `SkillsConfig.bundled_directory`, `AgentsConfig.bundled_directory` fields | +| `src/home.rs` | No change (bundled path is CWD-relative, not home-resolved) | +| `src/agent.rs` | `read_skill_file` resolves via `skill_base_dirs`; `write_skill_file` unchanged; `reload_skills` loads both layers | +| `src/agent.rs` | `read_agent_file` / `write_agent_file` parallel changes | +| `src/main.rs` | Load bundled skills at startup; pass both layers to Agent | +| `src/learning.rs` | `patch_skill` uses instance dir (unchanged) | +| `src/platform/telegram.rs` | `/update-skills` reloads both layers after sync | + +## Testing + +- **Unit tests:** `test_skill_registry_instance_shadows_bundled`, `test_read_skill_file_resolves_from_instance_first`, `test_write_skill_file_always_writes_to_instance`, `test_bundled_skills_untouched_by_write` +- **Integration test:** Verify that after `/update-skills`, bundled skills remain in bundled dir and instance skills are untouched diff --git a/skills/codebase-gap-analysis/SKILL.md b/skills/codebase-gap-analysis/SKILL.md new file mode 100644 index 0000000..6b7cb07 --- /dev/null +++ b/skills/codebase-gap-analysis/SKILL.md @@ -0,0 +1,43 @@ +--- +name: codebase-gap-analysis +description: 'Analyze a user''s codebase against a reference architecture to identify gaps and provide prioritized enhancement recommendations.' +tags: ['research', 'architecture', 'codebase', 'comparison', 'gap-analysis', 'project-review'] +--- + +# Codebase Gap Analysis vs Reference Architecture + +Use when the user wants to deeply compare their project or codebase against a reference implementation, framework, or architectural pattern to find gaps and improvement opportunities. + +## Workflow + +### 1. Define Scope & Dimensions +- Confirm the user's project location (repo URL, local path, or pasted code). +- Confirm the reference target (technology, product, or architectural pattern). +- Select comparison dimensions relevant to the domain (e.g., orchestration, memory, concurrency, skills, backends, observability, agent lifecycle). + +### 2. Explore User Codebase +- Fetch repository metadata and directory structure. +- Read root configuration and documentation files. +- Dive into core source directories to map the architecture. +- Identify and read critical modules (planner, orchestrator, supervisor, memory, tools, agents, config). +- Summarize current capabilities, abstractions, and design patterns found. + +### 3. Research Reference Architecture +- Search official documentation, repositories, and technical articles for the reference system. +- Extract concrete capabilities, configuration mechanisms, and architectural layers. +- Note any experimental, undocumented, or advanced features relevant to the comparison. + +### 4. Execute Structured Comparison +- Build a side-by-side comparison table across the chosen dimensions. +- Flag capabilities present in the reference but missing or incomplete in the user's project. +- Note areas where the user's project may be ahead, equivalent, or intentionally divergent. + +### 5. Prioritized Recommendations +- List each gap with: description, impact, and suggested implementation approach aligned with the user's stack. +- Assign priority (Critical / High / Medium / Low). +- Propose a pragmatic roadmap that respects existing dependencies and build patterns. + +### 6. Deliver Synthesis +- Provide an overall architectural maturity assessment. +- Highlight quick wins versus long-term investments. +- Offer to elaborate on, design, or implement any specific recommendation. diff --git a/src/agent.rs b/src/agent.rs index c42e9d5..b99ec51 100644 --- a/src/agent.rs +++ b/src/agent.rs @@ -1,4 +1,5 @@ use anyhow::Result; +use std::path::{Path, PathBuf}; use std::sync::{Arc, Weak}; use tracing::{debug, error, info, warn}; @@ -139,6 +140,101 @@ impl Agent { prompt } + /// Resolve the base directory for a skill/agent by checking the registry. + /// Falls back to the configured directory if not found (for newly-created skills). + fn resolve_skill_base_dir( + &self, + name: &str, + config_dir: &Path, + skills_lock: &SkillRegistry, + ) -> PathBuf { + skills_lock + .base_dir(name) + .unwrap_or(config_dir) + .to_path_buf() + } + + /// Reload both skill and agent registries from their directories. + /// Returns `(skills_count, agents_count)`. + pub async fn reload_skills_and_agents(&self) -> (usize, usize) { + use crate::skills::loader::load_skills_from_dir; + use crate::skills::SkillSource; + + // Skills: load both layers, merge + let s_instance_dir = self.config.skills.directory.clone(); + let s_bundled_dir = self.config.skills.bundled_directory.clone(); + let s_instance = load_skills_from_dir( + &s_instance_dir, + SkillSource::Instance, + s_instance_dir.clone(), + ) + .await; + let s_bundled = + load_skills_from_dir(&s_bundled_dir, SkillSource::Bundled, s_bundled_dir.clone()).await; + let s_instance_ok = s_instance.is_ok(); + let s_bundled_ok = s_bundled.is_ok(); + + { + let mut skills = self.skills.write().await; + let mut new_base_dirs = std::collections::BTreeMap::new(); + if let Ok(reg) = s_instance { + skills.instance_skills = reg.instance_skills; + for (k, v) in reg.skill_base_dirs { + new_base_dirs.insert(k, v); + } + } + if let Ok(reg) = s_bundled { + skills.bundled_skills = reg.bundled_skills; + for (k, v) in reg.skill_base_dirs { + new_base_dirs.entry(k).or_insert(v); + } + } + if s_instance_ok || s_bundled_ok { + skills.skill_base_dirs.clear(); + skills.skill_base_dirs.extend(new_base_dirs); + } + } + let s_count = self.skills.read().await.len(); + + // Agents: same pattern + let a_instance_dir = self.config.agents.directory.clone(); + let a_bundled_dir = self.config.agents.bundled_directory.clone(); + let a_instance = load_skills_from_dir( + &a_instance_dir, + SkillSource::Instance, + a_instance_dir.clone(), + ) + .await; + let a_bundled = + load_skills_from_dir(&a_bundled_dir, SkillSource::Bundled, a_bundled_dir.clone()).await; + let a_instance_ok = a_instance.is_ok(); + let a_bundled_ok = a_bundled.is_ok(); + + { + let mut agents = self.agents.write().await; + let mut new_base_dirs = std::collections::BTreeMap::new(); + if let Ok(reg) = a_instance { + agents.instance_skills = reg.instance_skills; + for (k, v) in reg.skill_base_dirs { + new_base_dirs.insert(k, v); + } + } + if let Ok(reg) = a_bundled { + agents.bundled_skills = reg.bundled_skills; + for (k, v) in reg.skill_base_dirs { + new_base_dirs.entry(k).or_insert(v); + } + } + if a_instance_ok || a_bundled_ok { + agents.skill_base_dirs.clear(); + agents.skill_base_dirs.extend(new_base_dirs); + } + } + let a_count = self.agents.read().await.len(); + + (s_count, a_count) + } + /// Process an incoming message and return the response text pub(crate) fn now_iso8601_static() -> String { chrono::Utc::now().to_rfc3339_opts(chrono::SecondsFormat::Millis, true) @@ -319,10 +415,6 @@ impl Agent { let mut iteration_count = 0u32; let mut tool_call_count = 0u32; - // Clone the stream sender so tool status can be pushed into the same Telegram - // message during tool execution, before the final response starts streaming. - let stream_status_tx = stream_token_tx.clone(); - for iteration in 0..max_iterations { debug!( "Trying iteration {}: messages length: {}", @@ -525,23 +617,10 @@ impl Agent { tx.try_send(crate::platform::tool_notifier::ToolEvent::Started { name: tool_call.function.name.clone(), args_preview: args_preview.clone(), + arguments_json: tool_call.function.arguments.clone(), }); } - // Stream tool status into the Telegram message only when - // tool-progress notifications are enabled, to avoid - // prepending status lines to otherwise silent/final output. - if tool_event_tx.is_some() { - if let Some(ref tx) = stream_status_tx { - let status = - crate::platform::tool_notifier::format_tool_status_line( - &tool_call.function.name, - &args_preview, - ); - tx.try_send(status).ok(); - } - } - let tool_result = self .execute_tool(&tool_call.function.name, &arguments, user_id, chat_id) .await; @@ -1668,26 +1747,28 @@ impl Agent { return format!("Invalid path: {}", e); } - let target = self - .config - .skills - .directory - .join(&skill_name) - .join(&relative_path); + // Resolve via registry (instance shadows bundled) + let skills_lock = self.skills.read().await; + let base_dir = self.resolve_skill_base_dir( + &skill_name, + &self.config.skills.directory, + &skills_lock, + ); + let target = base_dir.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() { + // Canonicalize check against the resolved base dir + if let Ok(base_canonical) = base_dir.canonicalize() { if let Ok(target_canonical) = target.canonicalize() { - if !target_canonical.starts_with(&skills_canonical) { + if !target_canonical.starts_with(&base_canonical) { return format!( - "Access denied: path '{}/{}' resolves outside the skills directory", - skill_name, relative_path + "Access denied: path '{}' resolves outside the skills directory", + target.display() ); } } } + // Drop the read lock before awaiting the file read + drop(skills_lock); match tokio::fs::read_to_string(&target).await { Ok(content) => content, @@ -1731,6 +1812,34 @@ impl Agent { match tokio::fs::write(&target, &content).await { Ok(()) => { info!("Skill file written: {}", target.display()); + + // After writing, reload just the instance layer + let instance_dir = self.config.skills.directory.clone(); + use crate::skills::loader::load_skills_from_dir; + use crate::skills::SkillSource; + if let Ok(new_instance) = load_skills_from_dir( + &instance_dir, + SkillSource::Instance, + instance_dir.clone(), + ) + .await + { + let bundled_dir = self.config.skills.bundled_directory.clone(); + let mut skills = self.skills.write().await; + skills.instance_skills = new_instance.instance_skills; + skills.skill_base_dirs.clear(); + let bundled_names = + skills.bundled_skills.keys().cloned().collect::>(); + for name in bundled_names { + skills + .skill_base_dirs + .insert(name.clone(), bundled_dir.clone()); + } + for (k, v) in new_instance.skill_base_dirs { + skills.skill_base_dirs.insert(k, v); + } + } + format!("Written: {}", target.display()) } Err(e) => format!("Failed to write skill file: {}", e), @@ -1738,16 +1847,50 @@ impl Agent { } "reload_skills" => { use crate::skills::loader::load_skills_from_dir; - match load_skills_from_dir(&self.config.skills.directory).await { - Ok(new_registry) => { - let count = new_registry.len(); - let mut skills = self.skills.write().await; - *skills = new_registry; - info!("Skills reloaded: {} skill(s) active", count); - format!("Skills reloaded. {} skill(s) now active.", count) + use crate::skills::SkillSource; + + let instance_dir = self.config.skills.directory.clone(); + let bundled_dir = self.config.skills.bundled_directory.clone(); + + let instance_reg = load_skills_from_dir( + &instance_dir, + SkillSource::Instance, + instance_dir.clone(), + ) + .await; + let bundled_reg = + load_skills_from_dir(&bundled_dir, SkillSource::Bundled, bundled_dir.clone()) + .await; + let instance_ok = instance_reg.is_ok(); + let bundled_ok = bundled_reg.is_ok(); + + let mut skills = self.skills.write().await; + let mut new_base_dirs = std::collections::BTreeMap::new(); + if let Ok(reg) = instance_reg { + skills.instance_skills = reg.instance_skills; + for (k, v) in reg.skill_base_dirs { + new_base_dirs.insert(k, v); + } + } + if let Ok(reg) = bundled_reg { + skills.bundled_skills = reg.bundled_skills; + for (k, v) in reg.skill_base_dirs { + new_base_dirs.entry(k).or_insert(v); } - Err(e) => format!("Failed to reload skills: {}", e), } + if instance_ok || bundled_ok { + skills.skill_base_dirs.clear(); + skills.skill_base_dirs.extend(new_base_dirs); + } + + let count = skills.len(); + info!( + "Skills reloaded: {} skill(s) active ({:?} instance, {:?} bundled)", + count, + skills.instance_skills.len(), + skills.bundled_skills.len() + ); + format!("Skills reloaded. {} skill(s) now active.", count) } "invoke_subagent" => { // Backward-compat alias for invoke_agent (skills registry only) @@ -1831,23 +1974,28 @@ impl Agent { return format!("Invalid path: {}", e); } - let target = self - .config - .agents - .directory - .join(&agent_name) - .join(&relative_path); + // Resolve via agents registry (instance shadows bundled) + let agents_lock = self.agents.read().await; + let base_dir = self.resolve_skill_base_dir( + &agent_name, + &self.config.agents.directory, + &agents_lock, + ); + let target = base_dir.join(&agent_name).join(&relative_path); - if let Ok(agents_canonical) = self.config.agents.directory.canonicalize() { + // Canonicalize check against the resolved base dir + if let Ok(base_canonical) = base_dir.canonicalize() { if let Ok(target_canonical) = target.canonicalize() { - if !target_canonical.starts_with(&agents_canonical) { + if !target_canonical.starts_with(&base_canonical) { return format!( - "Access denied: path '{}/{}' resolves outside the agents directory", - agent_name, relative_path + "Access denied: path '{}' resolves outside the agents directory", + target.display() ); } } } + // Drop the read lock before awaiting the file read + drop(agents_lock); match tokio::fs::read_to_string(&target).await { Ok(content) => content, @@ -1891,6 +2039,34 @@ impl Agent { match tokio::fs::write(&target, &content).await { Ok(()) => { info!("Agent file written: {}", target.display()); + + // After writing, reload just the instance layer + let instance_dir = self.config.agents.directory.clone(); + use crate::skills::loader::load_skills_from_dir; + use crate::skills::SkillSource; + if let Ok(new_instance) = load_skills_from_dir( + &instance_dir, + SkillSource::Instance, + instance_dir.clone(), + ) + .await + { + let bundled_dir = self.config.agents.bundled_directory.clone(); + let mut agents = self.agents.write().await; + agents.instance_skills = new_instance.instance_skills; + agents.skill_base_dirs.clear(); + let bundled_names = + agents.bundled_skills.keys().cloned().collect::>(); + for name in bundled_names { + agents + .skill_base_dirs + .insert(name.clone(), bundled_dir.clone()); + } + for (k, v) in new_instance.skill_base_dirs { + agents.skill_base_dirs.insert(k, v); + } + } + format!("Written: {}", target.display()) } Err(e) => format!("Failed to write agent file: {}", e), @@ -1898,16 +2074,50 @@ impl Agent { } "reload_agents" => { use crate::skills::loader::load_skills_from_dir; - match load_skills_from_dir(&self.config.agents.directory).await { - Ok(new_registry) => { - let count = new_registry.len(); - let mut agents = self.agents.write().await; - *agents = new_registry; - info!("Agents reloaded: {} agent(s) active", count); - format!("Agents reloaded. {} agent(s) now active.", count) + use crate::skills::SkillSource; + + let instance_dir = self.config.agents.directory.clone(); + let bundled_dir = self.config.agents.bundled_directory.clone(); + + let instance_reg = load_skills_from_dir( + &instance_dir, + SkillSource::Instance, + instance_dir.clone(), + ) + .await; + let bundled_reg = + load_skills_from_dir(&bundled_dir, SkillSource::Bundled, bundled_dir.clone()) + .await; + let instance_ok = instance_reg.is_ok(); + let bundled_ok = bundled_reg.is_ok(); + + let mut agents = self.agents.write().await; + let mut new_base_dirs = std::collections::BTreeMap::new(); + if let Ok(reg) = instance_reg { + agents.instance_skills = reg.instance_skills; + for (k, v) in reg.skill_base_dirs { + new_base_dirs.insert(k, v); + } + } + if let Ok(reg) = bundled_reg { + agents.bundled_skills = reg.bundled_skills; + for (k, v) in reg.skill_base_dirs { + new_base_dirs.entry(k).or_insert(v); } - Err(e) => format!("Failed to reload agents: {}", e), } + if instance_ok || bundled_ok { + agents.skill_base_dirs.clear(); + agents.skill_base_dirs.extend(new_base_dirs); + } + + let count = agents.len(); + info!( + "Agents reloaded: {} agent(s) active ({:?} instance, {:?} bundled)", + count, + agents.instance_skills.len(), + agents.bundled_skills.len() + ); + format!("Agents reloaded. {} agent(s) now active.", count) } "try_new_tech" => { let technology = match arguments["technology"].as_str() { @@ -2234,6 +2444,37 @@ fn missing_subagent_tools(declared: &[String], available_names: &[String]) -> Ve mod tests { use super::*; + #[test] + fn test_tool_status_is_not_streamed_to_answer_channel() { + let source = include_str!("agent.rs"); + let status_line_call = ["format_tool_status", "_line("].concat(); + let stream_status_var = ["stream", "_status_tx"].concat(); + + assert!( + !source.contains(&status_line_call), + "agent.rs must not format tool-status lines for the assistant answer stream" + ); + assert!( + !source.contains(&stream_status_var), + "agent.rs must not clone a separate stream-status sender for tool progress" + ); + } + + #[test] + fn test_reloads_clear_base_dir_maps_before_repopulation() { + let source = include_str!("agent.rs"); + let skills_clear_count = source.matches("skills.skill_base_dirs.clear();").count(); + let agents_clear_count = source.matches("agents.skill_base_dirs.clear();").count(); + assert!( + skills_clear_count >= 3, + "skills base-dir map must be cleared in all reload/write paths" + ); + assert!( + agents_clear_count >= 3, + "agents base-dir map must be cleared in all reload/write paths" + ); + } + #[test] fn test_now_iso8601_is_valid_rfc3339() { let ts = Agent::now_iso8601_static(); @@ -2393,4 +2634,62 @@ mod tests { let assembled: String = tokens.concat(); assert_eq!(assembled, "Hello world!"); } + + #[tokio::test] + async fn test_read_skill_file_checks_instance_before_bundled() { + let dir = tempfile::tempdir().unwrap(); + let instance_dir = dir.path().join("instance-skills"); + let bundled_dir = dir.path().join("bundled-skills"); + + // Create same-named skill in both dirs with different content + tokio::fs::create_dir_all(instance_dir.join("my-skill")) + .await + .unwrap(); + tokio::fs::write(instance_dir.join("my-skill/SKILL.md"), "instance content") + .await + .unwrap(); + tokio::fs::create_dir_all(bundled_dir.join("my-skill")) + .await + .unwrap(); + tokio::fs::write(bundled_dir.join("my-skill/SKILL.md"), "bundled content") + .await + .unwrap(); + + // Load both into registry + let mut registry = crate::skills::SkillRegistry::new(); + let inst = crate::skills::loader::load_skills_from_dir( + &instance_dir, + crate::skills::SkillSource::Instance, + instance_dir.clone(), + ) + .await + .unwrap(); + let bund = crate::skills::loader::load_skills_from_dir( + &bundled_dir, + crate::skills::SkillSource::Bundled, + bundled_dir.clone(), + ) + .await + .unwrap(); + for s in inst.list() { + registry.register( + s.clone(), + crate::skills::SkillSource::Instance, + instance_dir.clone(), + ); + } + for s in bund.list() { + registry.register( + s.clone(), + crate::skills::SkillSource::Bundled, + bundled_dir.clone(), + ); + } + + // read_skill_file should find instance version + let base_dir = registry.base_dir("my-skill").unwrap(); + let target = base_dir.join("my-skill/SKILL.md"); + let content = tokio::fs::read_to_string(&target).await.unwrap(); + assert_eq!(content, "instance content", "instance must shadow bundled"); + } } diff --git a/src/config.rs b/src/config.rs index ca0d9ce..d69d45e 100644 --- a/src/config.rs +++ b/src/config.rs @@ -6,6 +6,7 @@ use std::path::{Path, PathBuf}; pub struct Config { pub telegram: TelegramConfig, pub openrouter: OpenRouterConfig, + #[serde(default)] pub sandbox: SandboxConfig, #[serde(default)] pub mcp_servers: Vec, @@ -26,13 +27,16 @@ pub struct Config { pub learning: LearningConfig, #[serde(default)] pub supervisor: SupervisorConfig, + /// Absolute home root resolved at load time (not read from TOML). + #[serde(skip)] + pub resolved_home: Option, } #[derive(Debug, Deserialize, Clone)] pub struct SupervisorConfig { #[serde(default = "default_autonomy_mode")] pub default_autonomy_mode: String, - #[serde(default = "default_artifacts_dir")] + #[serde(default)] pub artifacts_dir: std::path::PathBuf, #[serde(default)] pub risk: RiskThresholdsConfig, @@ -71,7 +75,7 @@ fn default_autonomy_mode() -> String { } fn default_artifacts_dir() -> std::path::PathBuf { - std::path::PathBuf::from("supervisor/artifacts") + std::path::PathBuf::new() } #[derive(Debug, Deserialize, Clone)] @@ -104,8 +108,9 @@ pub struct OpenRouterConfig { pub system_prompt: String, } -#[derive(Debug, Deserialize, Clone)] +#[derive(Debug, Deserialize, Clone, Default)] pub struct SandboxConfig { + #[serde(default)] pub allowed_directory: PathBuf, } @@ -152,7 +157,7 @@ pub struct McpServerConfig { #[derive(Debug, Deserialize, Clone)] pub struct MemoryConfig { - #[serde(default = "default_db_path")] + #[serde(default)] pub database_path: PathBuf, #[serde(default = "default_rag_limit")] pub rag_limit: usize, @@ -174,14 +179,20 @@ pub struct MemoryConfig { #[derive(Debug, Deserialize, Clone)] pub struct SkillsConfig { - #[serde(default = "default_skills_dir")] + #[serde(default)] pub directory: PathBuf, + /// Bundled skills directory (read-only templates, default CWD-relative `./skills/`). + #[serde(default = "default_bundled_skills_dir")] + pub bundled_directory: PathBuf, } #[derive(Debug, Deserialize, Clone)] pub struct AgentsConfig { - #[serde(default = "default_agents_dir")] + #[serde(default)] pub directory: PathBuf, + /// Bundled agents directory (read-only templates, default CWD-relative `./agents/`). + #[serde(default = "default_bundled_agents_dir")] + pub bundled_directory: PathBuf, } #[derive(Debug, Deserialize, Clone, Default)] @@ -189,6 +200,9 @@ pub struct GeneralConfig { /// Optional location string injected into the system prompt (e.g. "Tokyo, Japan") #[serde(default)] pub location: Option, + /// Optional absolute path overriding the default `~/.rustfox` home root. + #[serde(default)] + pub home: Option, } #[derive(Debug, Deserialize, Clone)] @@ -211,7 +225,7 @@ pub struct LangSmithConfig { #[derive(Debug, Deserialize, Clone)] pub struct LearningConfig { /// Path to the user model file (Honcho-style USER.md). - #[serde(default = "default_user_model_path")] + #[serde(default)] pub user_model_path: PathBuf, /// Whether post-task skill extraction is enabled. #[serde(default = "default_true")] @@ -228,7 +242,7 @@ pub struct LearningConfig { } fn default_model() -> String { - "moonshotai/kimi-k2.5".to_string() + "moonshotai/kimi-k2.6".to_string() } fn default_base_url() -> String { @@ -268,14 +282,11 @@ fn default_system_prompt() -> String { - For complex multi-step problems: invoke the problem-solver subagent\n\ \n\ ## Sandbox\n\ - File and command tools operate only within the allowed sandbox directory." + File and command tools operate only within your persistent workspace directory.\n\ + The workspace survives restarts — use it to keep reusable scripts, programs, and notes for the long term." .to_string() } -fn default_db_path() -> PathBuf { - PathBuf::from("rustfox.db") -} - fn default_rag_limit() -> usize { 5 } @@ -292,14 +303,6 @@ fn default_summarize_cron() -> String { "0 0 2 * * *".to_string() } -fn default_skills_dir() -> PathBuf { - PathBuf::from("skills") -} - -fn default_agents_dir() -> PathBuf { - PathBuf::from("agents") -} - fn default_embedding_base_url() -> String { "https://openrouter.ai/api/v1".to_string() } @@ -314,7 +317,7 @@ fn default_embedding_dimensions() -> usize { fn default_memory_config() -> MemoryConfig { MemoryConfig { - database_path: default_db_path(), + database_path: PathBuf::new(), rag_limit: default_rag_limit(), max_raw_messages: default_max_raw_messages(), summarize_threshold: default_summarize_threshold(), @@ -325,13 +328,15 @@ fn default_memory_config() -> MemoryConfig { fn default_skills_config() -> SkillsConfig { SkillsConfig { - directory: default_skills_dir(), + directory: PathBuf::new(), + bundled_directory: default_bundled_skills_dir(), } } fn default_agents_config() -> AgentsConfig { AgentsConfig { - directory: default_agents_dir(), + directory: PathBuf::new(), + bundled_directory: default_bundled_agents_dir(), } } @@ -358,8 +363,12 @@ fn default_langsmith_base_url() -> String { "https://api.smith.langchain.com".to_string() } -fn default_user_model_path() -> PathBuf { - PathBuf::from("memory/USER.md") +fn default_bundled_skills_dir() -> PathBuf { + PathBuf::from("skills") +} + +fn default_bundled_agents_dir() -> PathBuf { + PathBuf::from("agents") } fn default_true() -> bool { @@ -380,7 +389,7 @@ fn default_user_model_cron() -> String { fn default_learning_config() -> LearningConfig { LearningConfig { - user_model_path: default_user_model_path(), + user_model_path: PathBuf::new(), skill_extraction_enabled: true, skill_extraction_threshold: default_skill_extraction_threshold(), user_model_update_interval: default_user_model_update_interval(), @@ -404,20 +413,101 @@ impl Config { self.agent.empty_response_retry_limit } + /// Resolve the home root and every data path, create directories, and write + /// the resolved paths back into the config fields. Unset paths are + /// materialized to absolute paths under the home root; absolute overrides + /// are preserved verbatim; relative overrides are kept as-is (legacy mode) + /// and a warning is emitted for each. Returns any legacy-path warnings for + /// the caller to log. + pub fn resolve(&mut self) -> Result> { + use crate::home::{ + ensure_dirs, resolve_data_path, resolve_home, PathOrigin, ResolvedPaths, + }; + + let env_home = std::env::var("RUSTFOX_HOME").ok(); + let config_home = self.general.as_ref().and_then(|g| g.home.as_deref()); + let os_home = dirs::home_dir(); + let home = resolve_home(env_home.as_deref(), config_home, os_home.as_deref())?; + + let mut warnings = Vec::new(); + let mut resolve_one = |label: &str, field: &Path, subpath: &str| -> PathBuf { + let (path, origin) = resolve_data_path(field, &home, subpath); + if origin == PathOrigin::RelativeLegacy { + warnings.push(crate::home::LegacyPathWarning { + label: label.to_string(), + current: path.clone(), + home_default: home.join(subpath), + }); + } + path + }; + + let workspace = resolve_one( + "sandbox.allowed_directory", + &self.sandbox.allowed_directory, + "workspace", + ); + let database = resolve_one( + "memory.database_path", + &self.memory.database_path, + "rustfox.db", + ); + let skills = resolve_one("skills.directory", &self.skills.directory, "skills"); + let agents = resolve_one("agents.directory", &self.agents.directory, "agents"); + let artifacts = resolve_one( + "supervisor.artifacts_dir", + &self.supervisor.artifacts_dir, + "artifacts", + ); + let user_model = resolve_one( + "learning.user_model_path", + &self.learning.user_model_path, + "user_model.md", + ); + + let paths = ResolvedPaths { + home: home.clone(), + workspace: workspace.clone(), + database: database.clone(), + skills: skills.clone(), + agents: agents.clone(), + artifacts: artifacts.clone(), + user_model: user_model.clone(), + }; + ensure_dirs(&paths)?; + + // Resolve bundled directories relative to CWD (not home) since they + // ship alongside the binary / project root. + let cwd = std::env::current_dir()?; + if !self.skills.bundled_directory.is_absolute() { + self.skills.bundled_directory = cwd.join(&self.skills.bundled_directory); + } + if !self.agents.bundled_directory.is_absolute() { + self.agents.bundled_directory = cwd.join(&self.agents.bundled_directory); + } + + self.sandbox.allowed_directory = workspace; + self.memory.database_path = database; + self.skills.directory = skills; + self.agents.directory = agents; + self.supervisor.artifacts_dir = artifacts; + self.learning.user_model_path = user_model; + self.resolved_home = Some(home); + + Ok(warnings) + } + pub fn load(path: &Path) -> Result { let content = std::fs::read_to_string(path) .with_context(|| format!("Failed to read config file: {}", path.display()))?; - let config: Config = + let mut config: Config = toml::from_str(&content).with_context(|| "Failed to parse config file")?; - // Validate sandbox directory exists - if !config.sandbox.allowed_directory.exists() { - std::fs::create_dir_all(&config.sandbox.allowed_directory).with_context(|| { - format!( - "Failed to create sandbox directory: {}", - config.sandbox.allowed_directory.display() - ) - })?; + let warnings = config + .resolve() + .with_context(|| "Failed to resolve home directory paths")?; + for w in &warnings { + tracing::warn!("{}", w.render()); } Ok(config) @@ -428,6 +518,94 @@ impl Config { mod tests { use super::*; + fn base_toml() -> &'static str { + r#" + [telegram] + bot_token = "tok" + allowed_user_ids = [1] + [openrouter] + api_key = "key" + "# + } + + #[test] + fn resolve_fills_unset_paths_under_home() { + let tmp = tempfile::tempdir().unwrap(); + let home = tmp.path().join(".rustfox"); + let mut cfg: Config = toml::from_str(base_toml()).unwrap(); + cfg.general = Some(GeneralConfig { + location: None, + home: Some(home.clone()), + }); + let warnings = cfg.resolve().unwrap(); + assert_eq!(cfg.resolved_home.as_ref().unwrap(), &home); + assert_eq!(cfg.sandbox.allowed_directory, home.join("workspace")); + assert_eq!(cfg.memory.database_path, home.join("rustfox.db")); + assert_eq!(cfg.skills.directory, home.join("skills")); + assert_eq!(cfg.agents.directory, home.join("agents")); + assert_eq!(cfg.supervisor.artifacts_dir, home.join("artifacts")); + assert_eq!(cfg.learning.user_model_path, home.join("user_model.md")); + assert!(warnings.is_empty()); + } + + #[test] + fn resolve_keeps_absolute_overrides() { + let tmp = tempfile::tempdir().unwrap(); + let home = tmp.path().join(".rustfox"); + let mut cfg: Config = toml::from_str(base_toml()).unwrap(); + cfg.general = Some(GeneralConfig { + location: None, + home: Some(home.clone()), + }); + // Use an absolute path under the (writable) tempdir so ensure_dirs can + // create its parent; the intent is to verify an absolute override is + // preserved verbatim and emits no legacy warning. + let custom_db = tmp.path().join("custom.db"); + cfg.memory.database_path = custom_db.clone(); + let warnings = cfg.resolve().unwrap(); + assert_eq!(cfg.memory.database_path, custom_db); + assert!(warnings.is_empty()); + } + + #[test] + fn resolve_warns_on_relative_override() { + let tmp = tempfile::tempdir().unwrap(); + let home = tmp.path().join(".rustfox"); + let mut cfg: Config = toml::from_str(base_toml()).unwrap(); + cfg.general = Some(GeneralConfig { + location: None, + home: Some(home), + }); + cfg.skills.directory = std::path::PathBuf::from("my-skills"); + let warnings = cfg.resolve().unwrap(); + assert_eq!(cfg.skills.directory, std::path::PathBuf::from("my-skills")); + assert!(warnings.iter().any(|w| w.label == "skills.directory")); + } + + #[test] + fn load_resolves_paths_to_absolute() { + let tmp = tempfile::tempdir().unwrap(); + let home = tmp.path().join(".rustfox"); + let cfg_path = tmp.path().join("config.toml"); + let toml = format!( + r#" + [telegram] + bot_token = "tok" + allowed_user_ids = [1] + [openrouter] + api_key = "key" + [general] + home = "{}" + "#, + home.display() + ); + std::fs::write(&cfg_path, toml).unwrap(); + let cfg = Config::load(&cfg_path).unwrap(); + assert_eq!(cfg.sandbox.allowed_directory, home.join("workspace")); + assert!(cfg.sandbox.allowed_directory.is_dir()); + assert_eq!(cfg.resolved_home.unwrap(), home); + } + #[test] fn test_langsmith_config_optional() { let toml = r#" @@ -579,10 +757,9 @@ mod tests { "#; let cfg: Config = toml::from_str(toml).unwrap(); assert_eq!(cfg.supervisor.default_autonomy_mode, "standard"); - assert_eq!( - cfg.supervisor.artifacts_dir, - std::path::PathBuf::from("supervisor/artifacts") - ); + // artifacts_dir now defaults to the empty "unset" sentinel; it is + // materialized to an absolute path only by Config::resolve(). + assert_eq!(cfg.supervisor.artifacts_dir, std::path::PathBuf::new()); } #[test] diff --git a/src/home.rs b/src/home.rs new file mode 100644 index 0000000..fc1b912 --- /dev/null +++ b/src/home.rs @@ -0,0 +1,267 @@ +use anyhow::{anyhow, Result}; +use std::path::{Path, PathBuf}; + +pub fn resolve_home( + env_home: Option<&str>, + config_home: Option<&Path>, + os_home: Option<&Path>, +) -> Result { + if let Some(env) = env_home { + let p = Path::new(env); + if p.is_absolute() { + return Ok(p.to_path_buf()); + } + tracing::warn!("RUSTFOX_HOME='{env}' is not absolute; ignoring it"); + } + if let Some(cfg) = config_home { + if cfg.is_absolute() { + return Ok(cfg.to_path_buf()); + } + tracing::warn!( + "[general].home='{}' is not absolute; ignoring it", + cfg.display() + ); + } + let home = os_home.ok_or_else(|| { + anyhow!("Could not determine the OS home directory; set RUSTFOX_HOME or [general].home") + })?; + Ok(home.join(".rustfox")) +} + +/// The home root used purely for *config-file discovery*, before the config is +/// loaded. Uses only `RUSTFOX_HOME` (if absolute) or `/.rustfox`. +pub fn default_home(env_home: Option<&str>, os_home: Option<&Path>) -> Option { + if let Some(env) = env_home { + let p = Path::new(env); + if p.is_absolute() { + return Some(p.to_path_buf()); + } + } + os_home.map(|h| h.join(".rustfox")) +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum PathOrigin { + Default, + Absolute, + RelativeLegacy, +} + +pub fn resolve_data_path( + configured: &Path, + home: &Path, + default_subpath: &str, +) -> (PathBuf, PathOrigin) { + if configured.as_os_str().is_empty() { + (home.join(default_subpath), PathOrigin::Default) + } else if configured.is_absolute() { + (configured.to_path_buf(), PathOrigin::Absolute) + } else { + (configured.to_path_buf(), PathOrigin::RelativeLegacy) + } +} + +#[derive(Debug, Clone)] +pub struct ResolvedPaths { + pub home: PathBuf, + pub workspace: PathBuf, + pub database: PathBuf, + pub skills: PathBuf, + pub agents: PathBuf, + pub artifacts: PathBuf, + pub user_model: PathBuf, +} + +pub fn ensure_dirs(paths: &ResolvedPaths) -> Result<()> { + use anyhow::Context; + for dir in [ + &paths.home, + &paths.workspace, + &paths.skills, + &paths.agents, + &paths.artifacts, + ] { + std::fs::create_dir_all(dir) + .with_context(|| format!("Failed to create directory: {}", dir.display()))?; + } + for file in [&paths.database, &paths.user_model] { + if let Some(parent) = file.parent() { + if !parent.as_os_str().is_empty() { + std::fs::create_dir_all(parent) + .with_context(|| format!("Failed to create directory: {}", parent.display()))?; + } + } + } + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let _ = std::fs::set_permissions(&paths.home, std::fs::Permissions::from_mode(0o700)); + } + Ok(()) +} + +/// An actionable warning emitted when a path is relative (CWD-resolved) or when +/// legacy data is detected in the launch directory while the path is unset. +#[derive(Debug, Clone)] +pub struct LegacyPathWarning { + /// Config field label, e.g. "memory.database_path". + pub label: String, + /// The path currently in effect (CWD-resolved legacy location). + pub current: PathBuf, + /// Where this path would live under the home root. + pub home_default: PathBuf, +} + +impl LegacyPathWarning { + /// Multi-line, copy-pasteable migration hint. + pub fn render(&self) -> String { + // `cp -rT` merges the source into the already-created destination + // directory instead of nesting it (e.g. avoids /skills/skills). + format!( + "Legacy path in use for `{label}`:\n \ + current : {current}\n \ + new home default : {home_default}\n \ + To migrate this path into the home directory:\n \ + cp -rT {current} {home_default}\n \ + To keep the current location, pin it in config.toml under its section.", + label = self.label, + current = self.current.display(), + home_default = self.home_default.display(), + ) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::path::Path; + + #[test] + fn env_takes_priority_when_absolute() { + let got = resolve_home( + Some("/abs/env/home"), + Some(Path::new("/abs/cfg/home")), + Some(Path::new("/os/home")), + ) + .unwrap(); + assert_eq!(got, PathBuf::from("/abs/env/home")); + } + + #[test] + fn relative_env_is_ignored_falls_to_config() { + let got = resolve_home( + Some("rel/env"), + Some(Path::new("/abs/cfg/home")), + Some(Path::new("/os/home")), + ) + .unwrap(); + assert_eq!(got, PathBuf::from("/abs/cfg/home")); + } + + #[test] + fn relative_config_is_ignored_falls_to_default() { + let got = resolve_home( + None, + Some(Path::new("rel/cfg")), + Some(Path::new("/os/home")), + ) + .unwrap(); + assert_eq!(got, PathBuf::from("/os/home/.rustfox")); + } + + #[test] + fn default_is_os_home_dot_rustfox() { + let got = resolve_home(None, None, Some(Path::new("/os/home"))).unwrap(); + assert_eq!(got, PathBuf::from("/os/home/.rustfox")); + } + + #[test] + fn errors_when_no_os_home_and_no_overrides() { + let got = resolve_home(None, None, None); + assert!(got.is_err()); + } + + #[test] + fn default_home_prefers_absolute_env() { + assert_eq!( + default_home(Some("/srv/rfx"), Some(Path::new("/home/u"))), + Some(PathBuf::from("/srv/rfx")) + ); + assert_eq!( + default_home(None, Some(Path::new("/home/u"))), + Some(PathBuf::from("/home/u/.rustfox")) + ); + assert_eq!( + default_home(Some("rel"), Some(Path::new("/home/u"))), + Some(PathBuf::from("/home/u/.rustfox")) + ); + } + + #[test] + fn unset_path_resolves_under_home() { + let (path, origin) = + resolve_data_path(Path::new(""), Path::new("/h/.rustfox"), "rustfox.db"); + assert_eq!(path, PathBuf::from("/h/.rustfox/rustfox.db")); + assert_eq!(origin, PathOrigin::Default); + } + + #[test] + fn absolute_path_used_verbatim() { + let (path, origin) = resolve_data_path( + Path::new("/data/rustfox.db"), + Path::new("/h/.rustfox"), + "rustfox.db", + ); + assert_eq!(path, PathBuf::from("/data/rustfox.db")); + assert_eq!(origin, PathOrigin::Absolute); + } + + #[test] + fn relative_path_is_legacy() { + let (path, origin) = resolve_data_path( + Path::new("rustfox.db"), + Path::new("/h/.rustfox"), + "rustfox.db", + ); + assert_eq!(path, PathBuf::from("rustfox.db")); + assert_eq!(origin, PathOrigin::RelativeLegacy); + } + + #[test] + fn ensure_dirs_creates_full_tree() { + let tmp = tempfile::tempdir().unwrap(); + let home = tmp.path().join(".rustfox"); + let paths = ResolvedPaths { + home: home.clone(), + workspace: home.join("workspace"), + database: home.join("rustfox.db"), + skills: home.join("skills"), + agents: home.join("agents"), + artifacts: home.join("artifacts"), + user_model: home.join("user_model.md"), + }; + ensure_dirs(&paths).unwrap(); + assert!(paths.home.is_dir()); + assert!(paths.workspace.is_dir()); + assert!(paths.skills.is_dir()); + assert!(paths.agents.is_dir()); + assert!(paths.artifacts.is_dir()); + // db + user_model are files, but their parent dirs must exist + assert!(paths.database.parent().unwrap().is_dir()); + assert!(paths.user_model.parent().unwrap().is_dir()); + } + + #[test] + fn warning_render_includes_paths_and_commands() { + let w = LegacyPathWarning { + label: "memory.database_path".to_string(), + current: PathBuf::from("/work/rustfox.db"), + home_default: PathBuf::from("/h/.rustfox/rustfox.db"), + }; + let s = w.render(); + assert!(s.contains("memory.database_path")); + assert!(s.contains("/work/rustfox.db")); + assert!(s.contains("/h/.rustfox/rustfox.db")); + assert!(s.contains("cp -rT")); + } +} diff --git a/src/learning.rs b/src/learning.rs index ddd63c1..7aa323a 100644 --- a/src/learning.rs +++ b/src/learning.rs @@ -4,7 +4,7 @@ use tracing::{info, warn}; use crate::llm::{ChatMessage, LlmClient}; use crate::skills::loader::load_skills_from_dir; -use crate::skills::SkillRegistry; +use crate::skills::{SkillRegistry, SkillSource}; /// Minimum number of conversation messages needed before updating the user model. const MIN_MESSAGES_FOR_USER_MODEL: usize = 3; @@ -174,7 +174,7 @@ async fn extract_skill_from_conversation( info!("Written auto-generated skill: {}", skill_path.display()); // Hot-reload skills. - match load_skills_from_dir(skills_dir).await { + match load_skills_from_dir(skills_dir, SkillSource::Instance, skills_dir.to_path_buf()).await { Ok(new_registry) => { let count = new_registry.len(); let mut reg = skills.write().await; @@ -268,7 +268,7 @@ pub async fn self_patch_skill( ); // Hot-reload skills. - match load_skills_from_dir(skills_dir).await { + match load_skills_from_dir(skills_dir, SkillSource::Instance, skills_dir.to_path_buf()).await { Ok(new_registry) => { let count = new_registry.len(); let mut reg = skills.write().await; diff --git a/src/lib.rs b/src/lib.rs index 34eac01..3139f12 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,6 +1,7 @@ pub mod agent; pub mod agent_prompt; pub mod config; +pub mod home; pub mod langsmith; pub mod learning; pub mod llm; diff --git a/src/main.rs b/src/main.rs index e70ffa8..eeba6bd 100644 --- a/src/main.rs +++ b/src/main.rs @@ -12,7 +12,7 @@ use rustfox::memory::MemoryStore; use rustfox::platform; use rustfox::scheduler::tasks::register_builtin_tasks; use rustfox::scheduler::Scheduler; -use rustfox::skills::loader::load_skills_from_dir; +use rustfox::skills::{loader::load_skills_from_dir, SkillSource}; #[tokio::main] async fn main() -> Result<()> { @@ -29,7 +29,22 @@ async fn main() -> Result<()> { let config_path = std::env::args() .nth(1) .map(PathBuf::from) - .unwrap_or_else(|| PathBuf::from("config.toml")); + .unwrap_or_else(|| { + let cwd = PathBuf::from("config.toml"); + if cwd.exists() { + return cwd; + } + let env_home = std::env::var("RUSTFOX_HOME").ok(); + if let Some(home) = + rustfox::home::default_home(env_home.as_deref(), dirs::home_dir().as_deref()) + { + let candidate = home.join("config.toml"); + if candidate.exists() { + return candidate; + } + } + cwd + }); info!("Loading configuration from: {}", config_path.display()); let config = Config::load(&config_path) @@ -38,6 +53,9 @@ async fn main() -> Result<()> { info!("Configuration loaded successfully"); info!(" Model: {}", config.openrouter.model); info!(" Sandbox: {}", config.sandbox.allowed_directory.display()); + if let Some(home) = &config.resolved_home { + info!(" Home: {}", home.display()); + } info!(" Allowed users: {:?}", config.telegram.allowed_user_ids); info!(" MCP servers: {}", config.mcp_servers.len()); let langsmith = std::sync::Arc::new(rustfox::langsmith::LangSmithClient::new( @@ -83,12 +101,85 @@ async fn main() -> Result<()> { let mut mcp_manager = McpManager::new(); mcp_manager.connect_all(&mcp_server_configs).await; - // Load skills from markdown files - let skills = load_skills_from_dir(&config.skills.directory).await?; + // Seed bundled skills/agents into the instance home on first run. + // Seed bundled skills/agents into the instance home on first run. + if let Err(e) = rustfox::skills::seed::seed_dir_if_empty( + &config.skills.bundled_directory, + &config.skills.directory, + ) + .await + { + warn!("Skill seeding failed: {e}"); + } + if let Err(e) = rustfox::skills::seed::seed_dir_if_empty( + &config.agents.bundled_directory, + &config.agents.directory, + ) + .await + { + warn!("Agent seeding failed: {e}"); + } + // Write the home-side lock so /update-skills can diff later (only when seeded + // into the home and a lock does not already exist). + if let Some(home) = &config.resolved_home { + let seed_lock = |lock_name: &str, dir: &std::path::Path| { + let lock_path = home.join(lock_name); + if !lock_path.exists() { + let lock = rustfox::skills::update::SkillLock { + version: 1, + skills: rustfox::skills::seed::lock_map_for(dir), + }; + if let Ok(json) = serde_json::to_string_pretty(&lock) { + let _ = std::fs::write(&lock_path, json); + } + } + }; + seed_lock("skills-lock.json", &config.skills.directory); + seed_lock("agents-lock.json", &config.agents.directory); + } + + // Load skills from instance and bundled directories (instance shadows bundled) + let mut skills = load_skills_from_dir( + &config.skills.directory, + SkillSource::Instance, + config.skills.directory.clone(), + ) + .await?; + let bundled_skills = load_skills_from_dir( + &config.skills.bundled_directory, + SkillSource::Bundled, + config.skills.bundled_directory.clone(), + ) + .await?; + for skill in bundled_skills.list() { + skills.register( + skill.clone(), + SkillSource::Bundled, + config.skills.bundled_directory.clone(), + ); + } info!(" Skills: {}", skills.len()); - // Load agents from the agents directory - let agents = load_skills_from_dir(&config.agents.directory).await?; + // Load agents from instance and bundled directories (instance shadows bundled) + let mut agents = load_skills_from_dir( + &config.agents.directory, + SkillSource::Instance, + config.agents.directory.clone(), + ) + .await?; + let bundled_agents = load_skills_from_dir( + &config.agents.bundled_directory, + SkillSource::Bundled, + config.agents.bundled_directory.clone(), + ) + .await?; + for agent in bundled_agents.list() { + agents.register( + agent.clone(), + SkillSource::Bundled, + config.agents.bundled_directory.clone(), + ); + } info!(" Agents: {}", agents.len()); // Create ScheduledTaskStore sharing the existing SQLite connection diff --git a/src/platform/telegram.rs b/src/platform/telegram.rs index 1aa6d3f..5c648d0 100644 --- a/src/platform/telegram.rs +++ b/src/platform/telegram.rs @@ -168,6 +168,7 @@ async fn handle_message(bot: Bot, msg: Message, agent: Arc) -> ResponseRe /clear - Clear conversation history\n\ /tools - List available tools\n\ /skills - List loaded skills\n\ + /update-skills - Re-sync bundled skills/agents (backs up local edits)\n\ /verbose - Toggle tool call progress display\n\ /queryrewrite - Toggle query rewriting for memory search", ); @@ -211,6 +212,47 @@ async fn handle_message(bot: Bot, msg: Message, agent: Arc) -> ResponseRe return Ok(()); } + if text == "/updateskills" || text == "/update-skills" { + let bundled_skills = agent.config.skills.bundled_directory.clone(); + let bundled_agents = agent.config.agents.bundled_directory.clone(); + let home = agent.config.resolved_home.clone(); + let lock_for = |name: &str| -> std::path::PathBuf { + home.clone() + .map(|h| h.join(name)) + .unwrap_or_else(|| std::path::PathBuf::from(name)) + }; + + let mut lines = Vec::new(); + match crate::skills::update::update_skills( + &bundled_skills, + &agent.config.skills.directory, + &lock_for("skills-lock.json"), + ) + .await + { + Ok(r) => lines.push(format!("Skills — {}", r.summary())), + Err(e) => lines.push(format!("Skills update failed: {e}")), + } + match crate::skills::update::update_skills( + &bundled_agents, + &agent.config.agents.directory, + &lock_for("agents-lock.json"), + ) + .await + { + Ok(r) => lines.push(format!("Agents — {}", r.summary())), + Err(e) => lines.push(format!("Agents update failed: {e}")), + } + + let (s, a) = agent.reload_skills_and_agents().await; + lines.push(format!("Reloaded: {s} skill(s), {a} agent(s) active.")); + + bot.send_message(msg.chat.id, escape_text(&lines.join("\n"))) + .parse_mode(ParseMode::MarkdownV2) + .await?; + return Ok(()); + } + if text == "/verbose" { let current = agent .memory @@ -307,10 +349,22 @@ async fn handle_message(bot: Bot, msg: Message, agent: Arc) -> ResponseRe let mut notifier = crate::platform::tool_notifier::ToolCallNotifier::new(bot_clone, chat_id); notifier.start().await; + let mut handled_finished = false; while let Some(event) = rx.recv().await { - notifier.handle_event(event).await; + match event { + crate::platform::tool_notifier::ToolEvent::Finished { success } => { + notifier.finish(success).await; + handled_finished = true; + break; + } + other => notifier.handle_event(other).await, + } + } + // If the channel closed without an explicit Finished event, preserve + // previous behaviour and treat it as a successful finish. + if !handled_finished { + notifier.finish(true).await; } - notifier.finish().await; })) } else { None @@ -450,6 +504,9 @@ async fn handle_message(bot: Bot, msg: Message, agent: Arc) -> ResponseRe }; // Process through agent — moves stream_token_tx and tool_event_tx + // Keep an owned clone of the tool_event_tx so we can send a terminal + // Finished event after processing completes. + let agent_tool_event_tx = tool_event_tx.clone(); let process_result = match agent .process_message(&incoming, tool_event_tx, Some(stream_token_tx)) .await @@ -461,6 +518,15 @@ async fn handle_message(bot: Bot, msg: Message, agent: Arc) -> ResponseRe } }; + let process_success = process_result.is_ok(); + if let Some(tx) = agent_tool_event_tx { + let _ = tx + .send(crate::platform::tool_notifier::ToolEvent::Finished { + success: process_success, + }) + .await; + } + // Drop the sender to signal the notifier to stop, then await cleanup. // tool_event_tx is already moved into process_message — it's dropped when process_message returns. if let Some(handle) = notifier_handle { diff --git a/src/platform/tool_notifier.rs b/src/platform/tool_notifier.rs index fe74e7f..ed73ea1 100644 --- a/src/platform/tool_notifier.rs +++ b/src/platform/tool_notifier.rs @@ -1,5 +1,11 @@ +use serde_json::Value as JsonValue; use std::time::{Duration, Instant}; +// Display limits +const MAX_STATUS_TEXT_CHARS: usize = 3800; +const MAX_PLAN_STEPS_RENDERED: usize = 20; +const MAX_DISPLAY_FIELD_CHARS: usize = 60; + use teloxide::{prelude::*, types::Message}; use tracing::{debug, warn}; @@ -12,9 +18,259 @@ pub enum ToolEvent { name: String, /// First 60 chars of the arguments JSON, for display. args_preview: String, + /// Full arguments JSON, used only by display-state parsers. + arguments_json: String, }, /// A tool call completed (successfully or with error). Completed { name: String, success: bool }, + /// Sent by the platform when the overall request has finished. + Finished { success: bool }, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +enum PlanStepStatus { + Todo, + InProgress, + Done, + Failed, +} + +impl PlanStepStatus { + fn from_tool_status(status: &str) -> Self { + match status { + "done" | "completed" | "complete" => Self::Done, + "failed" | "error" => Self::Failed, + "in_progress" | "running" | "active" => Self::InProgress, + _ => Self::Todo, + } + } + + fn marker(&self) -> &'static str { + match self { + Self::Todo => "[ ]", + Self::InProgress => "[>]", + Self::Done => "[x]", + Self::Failed => "[!]", + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct PlanStepDisplay { + text: String, + status: PlanStepStatus, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct PlanDisplay { + title: String, + steps: Vec, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct PlanStepUpdate { + step_id: usize, + status: PlanStepStatus, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +enum ToolActivityState { + Running, + Completed, + Failed, +} + +impl ToolActivityState { + fn label(&self) -> &'static str { + match self { + Self::Running => "running", + Self::Completed => "completed", + Self::Failed => "failed", + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct ToolActivity { + name: String, + args_preview: String, + state: ToolActivityState, +} + +#[derive(Debug, Default, Clone, PartialEq, Eq)] +struct ToolDisplayState { + plan: Option, + activities: Vec, + active_plan_update_step: Option, +} + +impl ToolDisplayState { + fn handle_event(&mut self, event: ToolEvent) { + match event { + ToolEvent::Started { + name, + args_preview, + arguments_json, + } => { + if name == "plan_create" { + if let Some(plan) = parse_plan_create(&arguments_json) { + self.plan = Some(plan); + } + } else if name == "plan_update" { + self.active_plan_update_step = None; + if let Some(update) = parse_plan_update(&arguments_json) { + self.apply_plan_update(update); + } + } + + self.activities.push(ToolActivity { + name, + args_preview, + state: ToolActivityState::Running, + }); + if self.activities.len() > 12 { + self.activities.remove(0); + } + } + ToolEvent::Finished { .. } => { + // Terminal event handled by the platform-level notifier; ignore + // here so display state focuses on per-tool activity only. + } + ToolEvent::Completed { name, success } => { + if name == "plan_update" { + if let Some(step_id) = self.active_plan_update_step.take() { + if !success { + if let Some(plan) = self.plan.as_mut() { + if let Some(step) = plan.steps.get_mut(step_id) { + step.status = PlanStepStatus::Failed; + } + } + } + } + } + + if let Some(activity) = self.activities.iter_mut().rfind(|activity| { + activity.name == name && activity.state == ToolActivityState::Running + }) { + activity.state = if success { + ToolActivityState::Completed + } else { + ToolActivityState::Failed + }; + } + } + } + } + + fn has_activity(&self) -> bool { + self.plan.is_some() || !self.activities.is_empty() + } + + fn format_live(&self) -> String { + self.format("⏳ Working on your request") + } + + #[allow(dead_code)] + fn format_completed(&self) -> String { + // Default successful header and result. Caller may adjust based on overall + // request success vs failure when rendering the final card. + self.format("✅ Completed") + } + + fn apply_plan_update(&mut self, update: PlanStepUpdate) { + self.active_plan_update_step = Some(update.step_id); + if let Some(plan) = self.plan.as_mut() { + if let Some(step) = plan.steps.get_mut(update.step_id) { + step.status = update.status; + } + } + } + + fn format(&self, header: &str) -> String { + let mut text = header.to_string(); + + if let Some(plan) = &self.plan { + text.push_str("\n\nPlan\n"); + // Truncate title + text.push_str(&crate::utils::strings::truncate_chars( + &plan.title, + MAX_DISPLAY_FIELD_CHARS, + )); + // Render a bounded number of steps + let total = plan.steps.len(); + let cap = std::cmp::min(total, MAX_PLAN_STEPS_RENDERED); + for (index, step) in plan.steps.iter().enumerate().take(cap) { + text.push('\n'); + text.push_str(step.status.marker()); + text.push_str(&format!( + " {index}. {}", + crate::utils::strings::truncate_chars(&step.text, MAX_DISPLAY_FIELD_CHARS) + )); + // Intentionally omit rendering of any plan-update notes here to + // avoid leaking potentially sensitive content from tool arguments. + } + if total > MAX_PLAN_STEPS_RENDERED { + text.push('\n'); + let more = total - MAX_PLAN_STEPS_RENDERED; + text.push_str(&format!("[ ] ... {} more steps", more)); + } + } + + if !self.activities.is_empty() { + text.push_str("\n\nTool activity"); + for activity in &self.activities { + text.push('\n'); + text.push_str(&friendly_tool_name(&activity.name)); + if !activity.args_preview.is_empty() { + text.push_str(": "); + text.push_str(&crate::utils::strings::truncate_chars( + &activity.args_preview, + MAX_DISPLAY_FIELD_CHARS, + )); + } + text.push_str(" -- "); + text.push_str(activity.state.label()); + } + } + + // Clamp total text size to Telegram-safe limit + crate::utils::strings::truncate_chars(&text, MAX_STATUS_TEXT_CHARS) + } +} + +fn parse_plan_create(arguments_json: &str) -> Option { + #[derive(serde::Deserialize)] + struct PlanCreateArgs { + title: String, + steps: Vec, + } + + let args: PlanCreateArgs = serde_json::from_str(arguments_json).ok()?; + Some(PlanDisplay { + title: args.title, + steps: args + .steps + .into_iter() + .map(|text| PlanStepDisplay { + text, + status: PlanStepStatus::Todo, + }) + .collect(), + }) +} + +fn parse_plan_update(arguments_json: &str) -> Option { + #[derive(serde::Deserialize)] + struct PlanUpdateArgs { + step_id: usize, + status: String, + } + + let args: PlanUpdateArgs = serde_json::from_str(arguments_json).ok()?; + Some(PlanStepUpdate { + step_id: args.step_id, + status: PlanStepStatus::from_tool_status(&args.status), + }) } /// Convert a technical tool name to a human-readable label with an emoji prefix. @@ -106,26 +362,106 @@ fn humanise_function_name(func: &str) -> String { } } +fn key_matches_any(key: &str, allowed: &[&str]) -> bool { + let lower = key.to_ascii_lowercase(); + allowed.iter().any(|candidate| lower == *candidate) +} + +fn is_sensitive_key(key: &str) -> bool { + let lower = key.to_ascii_lowercase(); + [ + "token", + "secret", + "password", + "bearer", + "authorization", + "api_key", + "apikey", + "private_key", + "cookie", + "content", + "command", + "prompt", + "message", + "text", + ] + .iter() + .any(|sensitive| lower.contains(sensitive)) +} + /// Formats `args_preview` for display: truncate to 60 chars, strip outer braces for common single-arg calls. pub fn format_args_preview(args_json: &str) -> String { - // Try to extract a single-value preview for readability - // e.g. {"query":"Docker setup"} -> "Docker setup" - if let Ok(val) = serde_json::from_str::(args_json) { - if let Some(obj) = val.as_object() { - if obj.len() == 1 { - if let Some((_, v)) = obj.iter().next() { - let s = match v { - serde_json::Value::String(s) => s.clone(), - other => other.to_string(), - }; - let truncated = crate::utils::strings::truncate_chars(&s, 60); - return truncated; - } + // Privacy-aware preview formatter. + // Goals: + // - Never leak sensitive fields (api_key, token, prompt, content, command, etc.) + // - Only render a compact allowlist of safe keys when present + // - Never render nested objects/arrays in full + + const SAFE_KEYS: [&str; 13] = [ + "query", + "path", + "url", + "title", + "description", + "step_id", + "status", + "skill_name", + "agent", + "model", + "language", + "technology", + "name", + ]; + + let Ok(val) = serde_json::from_str::(args_json) else { + return String::new(); + }; + let Some(obj) = val.as_object() else { + return String::new(); + }; + + if obj.len() == 1 { + if let Some((key, value)) = obj.iter().next() { + if is_sensitive_key(key) || !key_matches_any(key, &SAFE_KEYS) { + return String::new(); + } + if let Some(s) = value.as_str() { + return crate::utils::strings::truncate_chars(s, MAX_DISPLAY_FIELD_CHARS); + } + if value.is_number() || value.is_boolean() { + return crate::utils::strings::truncate_chars( + &value.to_string(), + MAX_DISPLAY_FIELD_CHARS, + ); + } + } + + return String::new(); + } + + let mut parts: Vec = Vec::new(); + for safe in SAFE_KEYS.iter() { + if let Some((_, value)) = obj + .iter() + .find(|(key, _)| !is_sensitive_key(key) && key_matches_any(key, &[*safe])) + { + if let Some(s) = value.as_str() { + parts.push(format!( + "{}: {}", + safe, + crate::utils::strings::truncate_chars(s, MAX_DISPLAY_FIELD_CHARS) + )); + } else if value.is_number() || value.is_boolean() { + parts.push(format!("{}: {}", safe, value)); } } } - // Fallback: truncate raw JSON - crate::utils::strings::truncate_chars(args_json, 60) + + if parts.is_empty() { + return String::new(); + } + let joined = parts.join(", "); + crate::utils::strings::truncate_chars(&joined, MAX_STATUS_TEXT_CHARS) } /// Build the one-line tool status string streamed into the Telegram message @@ -140,24 +476,21 @@ pub fn format_tool_status_line(name: &str, args_preview: &str) -> String { } /// Manages the live-edited Telegram status message during agent tool execution. -#[allow(dead_code)] pub struct ToolCallNotifier { bot: Bot, chat_id: ChatId, status_msg: Option, - /// Log of tool calls: (name, args_preview, done, success) - tool_log: Vec<(String, String, bool, bool)>, + display_state: ToolDisplayState, last_edit: Option, } -#[allow(dead_code)] impl ToolCallNotifier { pub fn new(bot: Bot, chat_id: ChatId) -> Self { Self { bot, chat_id, status_msg: None, - tool_log: Vec::new(), + display_state: ToolDisplayState::default(), last_edit: None, } } @@ -172,25 +505,19 @@ impl ToolCallNotifier { /// Handle a ToolEvent and update the Telegram message. pub async fn handle_event(&mut self, event: ToolEvent) { - match event { - ToolEvent::Started { name, args_preview } => { - self.tool_log.push((name, args_preview, false, true)); - } - ToolEvent::Completed { name, success } => { - if let Some(entry) = self - .tool_log - .iter_mut() - .rfind(|(n, _, done, _)| n == &name && !*done) - { - entry.2 = true; // done - entry.3 = success; - } - } - } + self.display_state.handle_event(event); self.edit_message().await; } async fn edit_message(&mut self) { + if self.status_msg.is_none() { + return; + } + let text = self.format_status(); + self.edit_status_text(&text, "edit").await; + } + + async fn edit_status_text(&mut self, text: &str, _context: &str) { let Some(ref msg) = self.status_msg else { return; }; @@ -203,66 +530,64 @@ impl ToolCallNotifier { } } - let text = self.format_status(); - match self - .bot - .edit_message_text(self.chat_id, msg.id, &text) - .await - { - Ok(_) => self.last_edit = Some(Instant::now()), - Err(e) => debug!("Failed to edit tool notifier message: {:#}", e), + // Try once, then retry after a short delay on failure + match self.bot.edit_message_text(self.chat_id, msg.id, text).await { + Ok(_) => { + self.last_edit = Some(Instant::now()); + return; + } + Err(e) => { + debug!("Failed to edit tool notifier message (first try): {:#}", e); + } + } + + // Wait 1s and retry once + tokio::time::sleep(Duration::from_millis(1000)).await; + match self.bot.edit_message_text(self.chat_id, msg.id, text).await { + Ok(_) => { + self.last_edit = Some(Instant::now()); + } + Err(e) => debug!("Failed to edit tool notifier message (retry): {:#}", e), } } fn format_status(&self) -> String { - let mut s = String::from("⏳ Working...\n"); - for (name, args_preview, done, success) in &self.tool_log { - let icon = if !done { - "⏳" - } else if *success { - "✅" + self.display_state.format_live() + } + + fn final_status_text(&self, success: bool) -> Option { + self.display_state.has_activity().then(|| { + let header = if success { + "✅ Completed" } else { - "❌" + "⛔ Stopped" }; - let label = friendly_tool_name(name); - if args_preview.is_empty() { - s.push_str(&format!("\n{} {}", icon, label)); + let mut text = self.display_state.format(header); + + if success { + text.push_str("\n\nResult\nFinal answer sent below."); } else { - s.push_str(&format!("\n{} {}: {}", icon, label, args_preview)); + text.push_str("\n\nResult\nRequest ended with an error response below."); } - } - s + + // Clamp AFTER appending the Result section to stay within Telegram's limit + crate::utils::strings::truncate_chars(&text, MAX_STATUS_TEXT_CHARS) + }) } - /// Finalise the status message by deleting it. - /// - /// The status message is always deleted once agent processing is complete. - /// The final LLM response (streamed separately) is the only message that - /// remains visible to the user. - pub async fn finish(&self) { + /// Finalise the status message by persisting a completed summary when useful. + pub async fn finish(&mut self, success: bool) { let Some(ref msg) = self.status_msg else { return; }; - self.bot.delete_message(self.chat_id, msg.id).await.ok(); - } + let Some(text) = self.final_status_text(success) else { + self.bot.delete_message(self.chat_id, msg.id).await.ok(); + return; + }; - /// Format a compact summary of tools that ran. - /// Currently unused because `finish()` always deletes the status message, - /// but kept for potential future use (e.g. re-enabling persistent summaries). - #[allow(dead_code)] - fn format_final(&self) -> String { - let mut s = String::from("🔧 Tools used:"); - for (name, args_preview, _done, success) in &self.tool_log { - let icon = if *success { "✅" } else { "❌" }; - let label = friendly_tool_name(name); - if args_preview.is_empty() { - s.push_str(&format!("\n{} {}", icon, label)); - } else { - s.push_str(&format!("\n{} {}: {}", icon, label, args_preview)); - } - } - s + // Use shared helper to ensure consistent rate-limiting and retry behaviour + self.edit_status_text(&text, "final").await; } } @@ -270,6 +595,337 @@ impl ToolCallNotifier { mod tests { use super::*; + #[test] + fn test_format_args_preview_redacts_sensitive_single_arg() { + let json = r#"{"api_key":"sk-SECRET-123"}"#; + let out = format_args_preview(json); + assert!(out.is_empty(), "sensitive key must be suppressed: {}", out); + } + + #[test] + fn test_format_args_preview_suppresses_command_content_and_prompt() { + let j = r#"{"command":"rm -rf /", "prompt":"write me a secret"}"#; + // multi-key but all sensitive — should suppress to empty + let out = format_args_preview(j); + assert!( + out.is_empty(), + "sensitive multi-key should be empty: {}", + out + ); + } + + #[test] + fn test_format_args_preview_suppresses_unknown_single_scalar_key() { + let preview = format_args_preview(r#"{"message":"private freeform text"}"#); + assert!( + preview.is_empty(), + "unknown single scalar key leaked: {preview}" + ); + } + + #[test] + fn test_format_args_preview_suppresses_secret_key_variants() { + for json in [ + r#"{"private_key":"secret"}"#, + r#"{"access_token":"secret"}"#, + r#"{"apiKey":"secret"}"#, + r#"{"cookie":"secret"}"#, + r#"{"text":"private text"}"#, + ] { + let preview = format_args_preview(json); + assert!( + preview.is_empty(), + "secret/content variant leaked from {json}: {preview}" + ); + } + } + + #[test] + fn test_format_args_preview_suppresses_non_object_and_malformed_args() { + for raw in [r#""plain string""#, "not-json", r#"["array value"]"#] { + let preview = format_args_preview(raw); + assert!( + preview.is_empty(), + "non-object or malformed args leaked from {raw}: {preview}" + ); + } + } + + #[test] + fn test_format_args_preview_only_uses_safe_keys_from_multi_arg_object() { + let j = r#"{"query":"find docs","api_key":"xxx","path":"/tmp/file"}"#; + let out = format_args_preview(j); + // should include 'query' and/or 'path' but not api_key + assert!(out.contains("query") || out.contains("path")); + assert!(!out.contains("api_key")); + } + + #[test] + fn test_tool_display_state_clamps_long_completed_summary() { + let mut s = ToolDisplayState::default(); + // create a plan with many long steps + let mut steps = Vec::new(); + for _i in 0..50 { + steps.push(PlanStepDisplay { + text: "a very long step description that repeats".repeat(20), + status: PlanStepStatus::Todo, + }); + } + s.plan = Some(PlanDisplay { + title: "Long Plan Title".to_string(), + steps, + }); + let formatted = s.format_completed(); + // Should always be under Telegram's safe limit (we clamp elsewhere to MAX_STATUS_TEXT_CHARS=3800) + assert!( + formatted.chars().count() <= 4000, + "formatted too long: {}", + formatted.chars().count() + ); + } + + #[test] + fn test_notifier_final_status_text_returns_completed_card_when_activity_exists() { + let mut notifier = ToolCallNotifier::new(Bot::new("TEST_TOKEN"), ChatId(1)); + notifier.display_state.handle_event(ToolEvent::Started { + name: "read_file".to_string(), + args_preview: "/tmp/file.txt".to_string(), + arguments_json: r#"{"path":"/tmp/file.txt"}"#.to_string(), + }); + notifier.display_state.handle_event(ToolEvent::Completed { + name: "read_file".to_string(), + success: true, + }); + let text = notifier + .final_status_text(true) + .expect("activity should produce a final status card"); + assert!( + text.contains("Completed"), + "completed header missing: {text}" + ); + assert!( + text.contains("Final answer sent below."), + "result line missing: {text}" + ); + assert!( + text.contains("Reading a file"), + "tool activity missing: {text}" + ); + } + + #[test] + fn test_notifier_final_status_text_is_none_without_activity() { + let notifier = ToolCallNotifier::new(Bot::new("TEST_TOKEN"), ChatId(1)); + assert!(notifier.final_status_text(true).is_none()); + } + + #[test] + fn test_tool_display_state_renders_plan_create_as_checklist() { + let mut state = ToolDisplayState::default(); + + state.handle_event(ToolEvent::Started { + name: "plan_create".to_string(), + args_preview: "Create test plan".to_string(), + arguments_json: + r#"{"title":"Create test plan","steps":["Gather context","Implement fix"]}"# + .to_string(), + }); + + let text = state.format_live(); + assert!( + text.contains("Working on your request"), + "live header missing: {text}" + ); + assert!(text.contains("Plan"), "plan section missing: {text}"); + assert!( + text.contains("Create test plan"), + "plan title missing: {text}" + ); + assert!( + text.contains("[ ] 0. Gather context"), + "first step missing: {text}" + ); + assert!( + text.contains("[ ] 1. Implement fix"), + "second step missing: {text}" + ); + } + + #[test] + fn test_tool_display_state_updates_plan_step_status() { + let mut state = ToolDisplayState::default(); + + state.handle_event(ToolEvent::Started { + name: "plan_create".to_string(), + args_preview: "Plan".to_string(), + arguments_json: r#"{"title":"Plan","steps":["First","Second"]}"#.to_string(), + }); + state.handle_event(ToolEvent::Started { + name: "plan_update".to_string(), + args_preview: "step 1".to_string(), + arguments_json: r#"{"step_id":1,"status":"in_progress","notes":"working"}"#.to_string(), + }); + + let text = state.format_live(); + assert!( + text.contains("[ ] 0. First"), + "unchanged step missing: {text}" + ); + assert!( + text.contains("[>] 1. Second"), + "updated step missing: {text}" + ); + } + + #[test] + fn test_tool_display_state_does_not_render_plan_update_notes() { + let mut state = ToolDisplayState::default(); + state.handle_event(ToolEvent::Started { + name: "plan_create".to_string(), + args_preview: "Plan".to_string(), + arguments_json: r#"{"title":"Plan","steps":["First"]}"#.to_string(), + }); + state.handle_event(ToolEvent::Started { + name: "plan_update".to_string(), + args_preview: "step 0".to_string(), + arguments_json: r#"{"step_id":0,"status":"done","notes":"token=secret"}"#.to_string(), + }); + let text = state.format_completed(); + assert!(text.contains("[x] 0. First"), "done step missing: {text}"); + assert!( + !text.contains("token=secret"), + "plan update note leaked: {text}" + ); + } + + #[test] + fn test_notifier_final_status_text_reports_failed_request() { + let mut notifier = ToolCallNotifier::new(Bot::new("TEST_TOKEN"), ChatId(1)); + notifier.display_state.handle_event(ToolEvent::Started { + name: "read_file".to_string(), + args_preview: "/tmp/file.txt".to_string(), + arguments_json: r#"{"path":"/tmp/file.txt"}"#.to_string(), + }); + + let text = notifier + .final_status_text(false) + .expect("activity should produce a final status card"); + assert!( + text.contains("Stopped") || text.contains("Failed"), + "failure header missing: {text}" + ); + assert!( + text.contains("error response below"), + "failure result missing: {text}" + ); + assert!( + !text.contains("Final answer sent below."), + "success result shown for failure: {text}" + ); + } + + #[test] + fn test_tool_display_state_marks_failed_plan_update_after_completion() { + let mut state = ToolDisplayState::default(); + + state.handle_event(ToolEvent::Started { + name: "plan_create".to_string(), + args_preview: "Plan".to_string(), + arguments_json: r#"{"title":"Plan","steps":["Only step"]}"#.to_string(), + }); + state.handle_event(ToolEvent::Started { + name: "plan_update".to_string(), + args_preview: "step 0".to_string(), + arguments_json: r#"{"step_id":0,"status":"in_progress"}"#.to_string(), + }); + state.handle_event(ToolEvent::Completed { + name: "plan_update".to_string(), + success: false, + }); + + let text = state.format_live(); + assert!( + text.contains("[!] 0. Only step"), + "failed step missing: {text}" + ); + } + + #[test] + fn test_failed_unparsed_plan_update_does_not_mark_previous_step_failed() { + let mut state = ToolDisplayState::default(); + + state.handle_event(ToolEvent::Started { + name: "plan_create".to_string(), + args_preview: "Plan".to_string(), + arguments_json: r#"{"title":"Plan","steps":["First","Second"]}"#.to_string(), + }); + state.handle_event(ToolEvent::Started { + name: "plan_update".to_string(), + args_preview: "step 1".to_string(), + arguments_json: r#"{"step_id":1,"status":"done"}"#.to_string(), + }); + state.handle_event(ToolEvent::Completed { + name: "plan_update".to_string(), + success: true, + }); + state.handle_event(ToolEvent::Started { + name: "plan_update".to_string(), + args_preview: "bad update".to_string(), + arguments_json: r#"{"status":"failed"}"#.to_string(), + }); + state.handle_event(ToolEvent::Completed { + name: "plan_update".to_string(), + success: false, + }); + + let text = state.format_live(); + assert!( + text.contains("[x] 1. Second"), + "valid completed step changed unexpectedly: {text}" + ); + assert!( + !text.contains("[!] 1. Second"), + "stale failed update marked the previous step failed: {text}" + ); + } + + #[test] + fn test_tool_display_state_renders_generic_tool_activity_without_plan() { + let mut state = ToolDisplayState::default(); + + state.handle_event(ToolEvent::Started { + name: "read_file".to_string(), + args_preview: "/tmp/file.txt".to_string(), + arguments_json: r#"{"path":"/tmp/file.txt"}"#.to_string(), + }); + state.handle_event(ToolEvent::Completed { + name: "read_file".to_string(), + success: true, + }); + + let text = state.format_completed(); + assert!( + text.contains("Completed"), + "completed header missing: {text}" + ); + assert!( + text.contains("Tool activity"), + "tool section missing: {text}" + ); + assert!( + text.contains("Reading a file"), + "friendly tool label missing: {text}" + ); + assert!( + text.contains("completed"), + "completion state missing: {text}" + ); + assert!( + !text.contains("Plan\n"), + "plan section should be omitted: {text}" + ); + } + #[test] fn test_format_args_preview_single_string_arg() { let json = r#"{"query":"Docker setup preferences"}"#; @@ -303,49 +959,6 @@ mod tests { assert!(preview.contains("/tmp/test.txt")); } - #[test] - fn test_format_final_includes_all_tools() { - // Build a notifier-like tool_log directly and call format_final via a helper - // that mirrors the real implementation using friendly_tool_name. - fn fake_format_final(tool_log: &[(String, String, bool, bool)]) -> String { - let mut s = String::from("🔧 Tools used:"); - for (name, args_preview, _done, success) in tool_log { - let icon = if *success { "✅" } else { "❌" }; - let label = friendly_tool_name(name); - if args_preview.is_empty() { - s.push_str(&format!("\n{} {}", icon, label)); - } else { - s.push_str(&format!("\n{} {}: {}", icon, label, args_preview)); - } - } - s - } - - let log = vec![ - ("search".to_string(), "Docker setup".to_string(), true, true), - ( - "read_file".to_string(), - "/etc/config".to_string(), - true, - false, - ), - ]; - let result = fake_format_final(&log); - assert!(result.contains("🔧 Tools used:"), "header missing"); - assert!(result.contains("✅"), "successful tool icon wrong"); - assert!(result.contains("❌"), "failed tool icon wrong"); - assert!(result.contains("Docker setup"), "args missing for search"); - assert!( - !result.contains("⏳ Working"), - "should not contain in-progress text" - ); - // read_file should be humanised - assert!( - result.contains("📄 Reading a file"), - "read_file must be humanised" - ); - } - #[test] fn test_format_args_preview_single_arg_with_chinese() { let long_chinese = @@ -415,6 +1028,18 @@ mod tests { assert!(std::str::from_utf8(preview.as_bytes()).is_ok()); } + #[test] + fn test_format_args_preview_multi_arg_keeps_multiple_safe_keys() { + let long_query = "q".repeat(60); + let args = format!( + r#"{{"query":"{}","path":"/tmp/example/path"}} "#, + long_query + ); + let preview = format_args_preview(&args); + assert!(preview.contains("query:"), "query key missing: {preview}"); + assert!(preview.contains("path:"), "path key missing: {preview}"); + } + // --- friendly_tool_name --- #[test] diff --git a/src/skills/loader.rs b/src/skills/loader.rs index b9f2a23..1f6641e 100644 --- a/src/skills/loader.rs +++ b/src/skills/loader.rs @@ -1,8 +1,8 @@ use anyhow::{Context, Result}; -use std::path::Path; +use std::path::{Path, PathBuf}; use tracing::{info, warn}; -use super::{Skill, SkillRegistry}; +use super::{Skill, SkillRegistry, SkillSource}; /// Load all markdown skill files from a directory. /// @@ -19,7 +19,11 @@ use super::{Skill, SkillRegistry}; /// --- /// # Instructions here... /// ``` -pub async fn load_skills_from_dir(dir: &Path) -> Result { +pub async fn load_skills_from_dir( + dir: &Path, + source: SkillSource, + base_dir: PathBuf, +) -> Result { let mut registry = SkillRegistry::new(); if !dir.exists() { @@ -52,7 +56,7 @@ pub async fn load_skills_from_dir(dir: &Path) -> Result { }; match load_skill_file(&skill_path).await { - Ok(skill) => registry.register(skill), + Ok(skill) => registry.register(skill, source, base_dir.clone()), Err(e) => warn!("Failed to load skill from {}: {}", skill_path.display(), e), } } @@ -303,7 +307,10 @@ mod tests { ) .await .unwrap(); - let skills = load_skills_from_dir(dir.path()).await.unwrap(); + let skills = + load_skills_from_dir(dir.path(), SkillSource::Instance, dir.path().to_path_buf()) + .await + .unwrap(); let s = skills.get("research-pack").unwrap(); assert_eq!(s.supervisor_workflow.as_deref(), Some("research")); assert_eq!(s.supervisor_required_caps, vec!["research".to_string()]); diff --git a/src/skills/mod.rs b/src/skills/mod.rs index 5686ad6..912a559 100644 --- a/src/skills/mod.rs +++ b/src/skills/mod.rs @@ -1,8 +1,18 @@ pub mod loader; +pub mod seed; +pub mod update; use std::collections::HashMap; +use std::path::{Path, PathBuf}; use tracing::info; +/// Whether a skill was loaded from the instance (custom/writable) or bundled (read-only) directory. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SkillSource { + Instance, + Bundled, +} + /// A loaded skill from a markdown file #[derive(Debug, Clone)] #[allow(dead_code)] @@ -27,10 +37,18 @@ pub struct Skill { pub supervisor_required_caps: Vec, } -/// Registry of all loaded skills +/// Registry of all loaded skills. +/// +/// Maintains two layers — instance (custom/writable) and bundled (read-only templates) — +/// with instance names shadowing bundled names on collision. A `skill_base_dirs` map +/// tracks the base directory for each skill so tools like `read_skill_file` can resolve +/// the correct filesystem path. #[derive(Debug, Clone)] pub struct SkillRegistry { - skills: HashMap, + pub instance_skills: HashMap, + pub bundled_skills: HashMap, + /// Maps skill name → absolute base directory for read_skill_file path resolution. + pub skill_base_dirs: HashMap, } impl Default for SkillRegistry { @@ -42,39 +60,66 @@ impl Default for SkillRegistry { impl SkillRegistry { pub fn new() -> Self { Self { - skills: HashMap::new(), + instance_skills: HashMap::new(), + bundled_skills: HashMap::new(), + skill_base_dirs: HashMap::new(), } } - /// Register a skill - pub fn register(&mut self, skill: Skill) { - info!("Registered skill: {} — {}", skill.name, skill.description); - self.skills.insert(skill.name.clone(), skill); + /// Register a skill with its source and base directory. + pub fn register(&mut self, skill: Skill, source: SkillSource, base_dir: PathBuf) { + let name = skill.name.clone(); + match source { + SkillSource::Instance => { + self.instance_skills.insert(name.clone(), skill); + self.skill_base_dirs.insert(name.clone(), base_dir); + } + SkillSource::Bundled => { + self.bundled_skills.insert(name.clone(), skill); + // Bundled path does NOT overwrite an existing instance path + self.skill_base_dirs.entry(name.clone()).or_insert(base_dir); + } + } + info!("Registered skill: {} — {:?}", name, source); } - /// Get a skill by name + /// Get a skill by name. Instance shadows bundled. #[allow(dead_code)] pub fn get(&self, name: &str) -> Option<&Skill> { - self.skills.get(name) + self.instance_skills + .get(name) + .or_else(|| self.bundled_skills.get(name)) + } + + /// Returns the base directory for a skill, used by read_skill_file for path resolution. + pub fn base_dir(&self, name: &str) -> Option<&Path> { + self.skill_base_dirs.get(name).map(|p| p.as_path()) } - /// List all registered skills + /// List all unique skills (instance names shadow bundled). pub fn list(&self) -> Vec<&Skill> { - self.skills.values().collect() + let mut all: Vec<&Skill> = self.instance_skills.values().collect(); + for skill in self.bundled_skills.values() { + if !self.instance_skills.contains_key(&skill.name) { + all.push(skill); + } + } + all } /// Build context string for the system prompt (skills directory). /// Instruction skills (no model field) are loaded via `read_skill_file` when relevant. /// Subagent skills (model field set) are invoked via `invoke_agent`. pub fn build_context(&self) -> String { - if self.skills.is_empty() { + let unique_skills = self.list(); + if unique_skills.is_empty() { return String::new(); } let mut instruction_lines = Vec::new(); let mut subagent_section = String::new(); - for skill in self.skills.values() { + for skill in &unique_skills { // A skill is treated as a subagent if it has a model explicitly set // OR if it declares a tools whitelist (meaning it needs sandboxed execution). // In both cases the LLM invokes it via invoke_agent(); the model used is @@ -120,12 +165,13 @@ impl SkillRegistry { /// Build context string for the agents directory (agents with their own model/tools). /// All agents are invoked via `invoke_agent`. pub fn build_agents_context(&self) -> String { - if self.skills.is_empty() { + let unique_agents = self.list(); + if unique_agents.is_empty() { return String::new(); } let mut lines = Vec::new(); - for agent in self.skills.values() { + for agent in &unique_agents { lines.push(format!( "- **{}**: {}\n Invoke via: `invoke_agent(agent=\"{}\", prompt=\"\")`", agent.name, agent.description, agent.name @@ -140,18 +186,26 @@ impl SkillRegistry { } pub fn len(&self) -> usize { - self.skills.len() + let mut names = std::collections::HashSet::new(); + for name in self.instance_skills.keys() { + names.insert(name); + } + for name in self.bundled_skills.keys() { + names.insert(name); + } + names.len() } #[allow(dead_code)] pub fn is_empty(&self) -> bool { - self.skills.is_empty() + self.instance_skills.is_empty() && self.bundled_skills.is_empty() } } #[cfg(test)] mod tests { use super::*; + use std::path::PathBuf; fn make_skill(name: &str, description: &str, content: &str, model: Option<&str>) -> Skill { Skill { @@ -167,16 +221,87 @@ mod tests { } } + #[test] + fn test_instance_shadows_bundled() { + let mut registry = SkillRegistry::new(); + registry.register( + make_skill("duplicate", "Instance version", "instance content", None), + SkillSource::Instance, + PathBuf::from("/instance"), + ); + registry.register( + make_skill("duplicate", "Bundled version", "bundled content", None), + SkillSource::Bundled, + PathBuf::from("/bundled"), + ); + + // get() should return instance version + let skill = registry.get("duplicate").unwrap(); + assert_eq!(skill.content, "instance content"); + assert_eq!(skill.description, "Instance version"); + + // base_dir should return instance path + assert_eq!( + registry.base_dir("duplicate").unwrap(), + Path::new("/instance") + ); + + // list() should only include the instance version (not duplicate) + let names: Vec<&str> = registry.list().iter().map(|s| s.name.as_str()).collect(); + assert_eq!(names, vec!["duplicate"]); + assert_eq!(registry.len(), 1); + } + + #[test] + fn test_unique_skills_from_both_layers() { + let mut registry = SkillRegistry::new(); + registry.register( + make_skill("alpha", "Instance only", "", None), + SkillSource::Instance, + PathBuf::from("/instance"), + ); + registry.register( + make_skill("beta", "Bundled only", "", None), + SkillSource::Bundled, + PathBuf::from("/bundled"), + ); + + let names: Vec<&str> = registry.list().iter().map(|s| s.name.as_str()).collect(); + assert!(names.contains(&"alpha")); + assert!(names.contains(&"beta")); + assert_eq!(registry.len(), 2); + } + + #[test] + fn test_base_dir_bundled_only() { + let mut registry = SkillRegistry::new(); + registry.register( + make_skill("bundled-only", "Bundled", "", None), + SkillSource::Bundled, + PathBuf::from("/bundled/path"), + ); + + assert_eq!( + registry.base_dir("bundled-only").unwrap(), + Path::new("/bundled/path") + ); + assert!(registry.get("bundled-only").is_some()); + } + #[test] fn test_build_context_instruction_skill_metadata_only() { // Instruction skills (no model): metadata only in system prompt; agent loads full content via read_skill_file. let mut registry = SkillRegistry::new(); - registry.register(make_skill( - "my-skill", - "Does things", - "# Instructions\nDo this and that.", - None, // no model = instruction skill - )); + registry.register( + make_skill( + "my-skill", + "Does things", + "# Instructions\nDo this and that.", + None, // no model = instruction skill + ), + SkillSource::Instance, + PathBuf::from("/tmp/test"), + ); let ctx = registry.build_context(); assert!(ctx.contains("my-skill")); assert!(ctx.contains("Does things")); @@ -191,12 +316,16 @@ mod tests { 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"), - )); + registry.register( + make_skill( + "thread-writer", + "Use when writing Thread posts.", + "# Super Secret Instructions\nLong style guide...", + Some("anthropic/claude-sonnet-4-6"), + ), + SkillSource::Instance, + PathBuf::from("/tmp/test"), + ); let ctx = registry.build_context(); // Metadata present assert!(ctx.contains("thread-writer")); @@ -217,18 +346,26 @@ mod tests { fn test_build_context_mixed_skills() { // Instruction skill: metadata + read_skill_file hint only (no body). Subagent: metadata only (no body). 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"), - )); + registry.register( + make_skill( + "instruction-skill", + "An instruction skill", + "Follow these instructions.", + None, + ), + SkillSource::Instance, + PathBuf::from("/tmp/test"), + ); + registry.register( + make_skill( + "subagent-skill", + "A subagent skill", + "Secret subagent body.", + Some("some/model"), + ), + SkillSource::Instance, + PathBuf::from("/tmp/test"), + ); let ctx = registry.build_context(); assert!(ctx.contains("You have the following skills available")); assert!(ctx.contains("Available Subagent Skills")); diff --git a/src/skills/seed.rs b/src/skills/seed.rs new file mode 100644 index 0000000..8a50d2e --- /dev/null +++ b/src/skills/seed.rs @@ -0,0 +1,193 @@ +use anyhow::{Context, Result}; +use sha2::{Digest, Sha256}; +use std::collections::BTreeMap; +use std::path::{Path, PathBuf}; + +/// SHA-256 (hex) of a skill/agent directory tree. +/// Requires a primary markdown (`SKILL.md` or `AGENT.md`) to exist. +pub fn hash_skill_dir(dir: &Path) -> Option { + ["SKILL.md", "AGENT.md"] + .into_iter() + .map(|f| dir.join(f)) + .find(|p| p.is_file())?; + + let mut files: Vec = Vec::new(); + collect_files(dir, &mut files).ok()?; + files.sort(); + + let mut h = Sha256::new(); + for file in files { + let rel = file.strip_prefix(dir).ok()?; + let rel = rel.to_string_lossy().replace('\\', "/"); + h.update(rel.as_bytes()); + h.update([0]); + h.update(std::fs::read(&file).ok()?); + } + Some(format!("{:x}", h.finalize())) +} + +fn collect_files(dir: &Path, out: &mut Vec) -> std::io::Result<()> { + for entry in std::fs::read_dir(dir)? { + let entry = entry?; + let path = entry.path(); + if path.is_dir() { + collect_files(&path, out)?; + } else if path.is_file() { + out.push(path); + } + } + Ok(()) +} + +/// Copy every skill subdirectory from `bundled` into `instance` when `instance` +/// is empty or missing. Returns the number of skills copied (0 if skipped). +pub async fn seed_dir_if_empty(bundled: &Path, instance: &Path) -> Result { + // If the bundled source and instance resolve to the same place, nothing to do. + if let (Ok(a), Ok(b)) = (bundled.canonicalize(), instance.canonicalize()) { + if a == b { + return Ok(0); + } + } + if !bundled.is_dir() { + tracing::info!("No bundled skills at {}; skipping seed", bundled.display()); + return Ok(0); + } + // Non-empty instance → never overwrite. + if instance.is_dir() { + let mut entries = tokio::fs::read_dir(instance).await?; + if entries.next_entry().await?.is_some() { + return Ok(0); + } + } else { + tokio::fs::create_dir_all(instance) + .await + .with_context(|| format!("Failed to create {}", instance.display()))?; + } + + let mut copied = 0usize; + let mut entries = tokio::fs::read_dir(bundled).await?; + while let Some(entry) = entries.next_entry().await? { + let src = entry.path(); + if !src.is_dir() { + continue; + } + let Some(name) = src.file_name() else { + continue; + }; + let dst = instance.join(name); + if let Err(e) = copy_dir_recursive(&src, &dst).await { + tracing::warn!("Failed to seed {}: {}", src.display(), e); + continue; + } + copied += 1; + } + tracing::info!("Seeded {copied} skill(s) into {}", instance.display()); + Ok(copied) +} + +/// Public re-export wrapper so the update engine can reuse recursive copy. +pub async fn copy_dir_recursive_pub(src: &Path, dst: &Path) -> Result<()> { + copy_dir_recursive(src, dst).await +} + +/// Recursively copy `src` directory to `dst`. +async fn copy_dir_recursive(src: &Path, dst: &Path) -> Result<()> { + tokio::fs::create_dir_all(dst).await?; + let mut entries = tokio::fs::read_dir(src).await?; + while let Some(entry) = entries.next_entry().await? { + let from = entry.path(); + let to = dst.join(entry.file_name()); + if from.is_dir() { + Box::pin(copy_dir_recursive(&from, &to)).await?; + } else { + tokio::fs::copy(&from, &to).await?; + } + } + Ok(()) +} + +/// Map of skill-name → content hash for every skill dir under `dir`. +pub fn lock_map_for(dir: &Path) -> BTreeMap { + let mut map = BTreeMap::new(); + let Ok(entries) = std::fs::read_dir(dir) else { + return map; + }; + for entry in entries.flatten() { + let p = entry.path(); + if p.is_dir() { + if let (Some(name), Some(hash)) = + (p.file_name().and_then(|n| n.to_str()), hash_skill_dir(&p)) + { + map.insert(name.to_string(), hash); + } + } + } + map +} + +#[cfg(test)] +mod tests { + use super::*; + + fn write_skill(root: &Path, name: &str, body: &str) { + let dir = root.join(name); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("SKILL.md"), body).unwrap(); + } + + #[tokio::test] + async fn seeds_into_empty_instance() { + let tmp = tempfile::tempdir().unwrap(); + let bundled = tmp.path().join("bundled"); + let instance = tmp.path().join("instance"); + write_skill(&bundled, "alpha", "---\nname: alpha\n---\nhi"); + write_skill(&bundled, "beta", "---\nname: beta\n---\nyo"); + + let n = seed_dir_if_empty(&bundled, &instance).await.unwrap(); + assert_eq!(n, 2); + assert!(instance.join("alpha/SKILL.md").is_file()); + assert!(instance.join("beta/SKILL.md").is_file()); + } + + #[tokio::test] + async fn skips_when_instance_nonempty() { + let tmp = tempfile::tempdir().unwrap(); + let bundled = tmp.path().join("bundled"); + let instance = tmp.path().join("instance"); + write_skill(&bundled, "alpha", "a"); + write_skill(&instance, "existing", "keep me"); + + let n = seed_dir_if_empty(&bundled, &instance).await.unwrap(); + assert_eq!(n, 0); + assert!(!instance.join("alpha").exists()); + assert!(instance.join("existing/SKILL.md").is_file()); + } + + #[test] + fn lock_map_hashes_each_skill() { + let tmp = tempfile::tempdir().unwrap(); + write_skill(tmp.path(), "alpha", "content-a"); + write_skill(tmp.path(), "beta", "content-b"); + let map = lock_map_for(tmp.path()); + assert_eq!(map.len(), 2); + assert_ne!(map["alpha"], map["beta"]); + } + + #[test] + fn hash_skill_dir_changes_when_auxiliary_file_changes() { + let tmp = tempfile::tempdir().unwrap(); + let skill_dir = tmp.path().join("alpha"); + std::fs::create_dir_all(&skill_dir).unwrap(); + std::fs::write(skill_dir.join("SKILL.md"), "content-a").unwrap(); + std::fs::write(skill_dir.join("guide.md"), "v1").unwrap(); + let before = hash_skill_dir(&skill_dir).expect("hash should exist"); + + std::fs::write(skill_dir.join("guide.md"), "v2").unwrap(); + let after = hash_skill_dir(&skill_dir).expect("hash should exist"); + + assert_ne!( + before, after, + "auxiliary file edits must affect directory hash" + ); + } +} diff --git a/src/skills/update.rs b/src/skills/update.rs new file mode 100644 index 0000000..4d40dd4 --- /dev/null +++ b/src/skills/update.rs @@ -0,0 +1,264 @@ +use anyhow::{Context, Result}; +use serde::{Deserialize, Serialize}; +use std::collections::BTreeMap; +use std::path::Path; + +use super::seed::{copy_dir_recursive_pub, hash_skill_dir}; + +/// Home-side lock file: skill-name → content hash captured at seed/update time. +#[derive(Debug, Default, Serialize, Deserialize)] +pub struct SkillLock { + pub version: u32, + #[serde(default)] + pub skills: BTreeMap, +} + +/// Outcome of an update run. +#[derive(Debug, Default, PartialEq, Eq)] +pub struct UpdateReport { + pub updated: Vec, + pub backed_up: Vec, + pub skipped: Vec, +} + +impl UpdateReport { + pub fn summary(&self) -> String { + format!( + "Update: {} updated, {} backed-up, {} unchanged.", + self.updated.len(), + self.backed_up.len(), + self.skipped.len() + ) + } +} + +/// Read the lock file at `lock_path`, or an empty lock if absent/invalid. +fn read_lock(lock_path: &Path) -> SkillLock { + std::fs::read_to_string(lock_path) + .ok() + .and_then(|s| serde_json::from_str(&s).ok()) + .unwrap_or(SkillLock { + version: 1, + skills: BTreeMap::new(), + }) +} + +fn write_lock(lock_path: &Path, lock: &SkillLock) -> Result<()> { + let json = serde_json::to_string_pretty(lock)?; + std::fs::write(lock_path, json) + .with_context(|| format!("Failed to write lock {}", lock_path.display())) +} + +/// Re-sync `bundled` → `instance` using content hashes recorded in `lock_path`. +/// +/// For each bundled skill: +/// - missing in instance → copy in (updated) +/// - present, instance hash == bundled hash → unchanged (skipped) +/// - present, instance hash == lock hash (and != bundled) → overwrite (updated) +/// - present, instance hash differs from both → rename entire instance dir to +/// `.bak` (preserving all files), then copy bundled dir in (backed_up) +/// +/// Instance-only skills (absent from `bundled`) are never touched. +pub async fn update_skills( + bundled: &Path, + instance: &Path, + lock_path: &Path, +) -> Result { + let mut report = UpdateReport::default(); + if !bundled.is_dir() { + anyhow::bail!("No bundled skills found at {}", bundled.display()); + } + let mut lock = read_lock(lock_path); + tokio::fs::create_dir_all(instance) + .await + .with_context(|| format!("Failed to create instance directory {}", instance.display()))?; + + let mut entries = tokio::fs::read_dir(bundled).await?; + while let Some(entry) = entries.next_entry().await? { + let src = entry.path(); + if !src.is_dir() { + continue; + } + let Some(name) = src.file_name().and_then(|n| n.to_str()).map(str::to_string) else { + continue; + }; + let dst = instance.join(&name); + let bundled_hash = match hash_skill_dir(&src) { + Some(h) => h, + None => continue, + }; + + if !dst.exists() { + copy_dir_recursive_pub(&src, &dst).await?; + lock.skills.insert(name.clone(), bundled_hash); + tracing::info!("Added skill '{name}' from bundle"); + report.updated.push(name); + continue; + } + + let instance_hash = hash_skill_dir(&dst); + if instance_hash.as_deref() == Some(bundled_hash.as_str()) { + lock.skills.entry(name.clone()).or_insert(bundled_hash); + report.skipped.push(name); + continue; + } + + let lock_hash = lock.skills.get(&name).cloned(); + let unmodified = lock_hash.is_some() && lock_hash.as_deref() == instance_hash.as_deref(); + + if !unmodified { + // Rename the entire instance directory to .bak, preserving all + // user-added or user-modified files (not just SKILL.md/AGENT.md). + let dst_bak = instance.join(format!("{name}.bak")); + let _ = tokio::fs::remove_dir_all(&dst_bak).await; + tokio::fs::rename(&dst, &dst_bak) + .await + .with_context(|| format!("Failed to back up '{name}' to {}", dst_bak.display()))?; + copy_dir_recursive_pub(&src, &dst).await?; + lock.skills.insert(name.clone(), bundled_hash); + tracing::info!("Updated locally-modified skill '{name}' (backup saved as {name}.bak)"); + report.backed_up.push(name); + } else { + let _ = tokio::fs::remove_dir_all(&dst).await; + copy_dir_recursive_pub(&src, &dst).await?; + lock.skills.insert(name.clone(), bundled_hash); + tracing::info!("Updated skill '{name}' from bundle"); + report.updated.push(name); + } + } + + tracing::info!("{}", report.summary()); + write_lock(lock_path, &lock)?; + Ok(report) +} + +#[cfg(test)] +mod tests { + use super::super::seed::lock_map_for; + use super::*; + + fn write_skill(root: &Path, name: &str, body: &str) { + let dir = root.join(name); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("SKILL.md"), body).unwrap(); + } + + fn seed_lock(lock_path: &Path, instance: &Path) { + let lock = SkillLock { + version: 1, + skills: lock_map_for(instance), + }; + write_lock(lock_path, &lock).unwrap(); + } + + #[tokio::test] + async fn unchanged_skill_is_overwritten_with_new_bundle() { + let tmp = tempfile::tempdir().unwrap(); + let bundled = tmp.path().join("bundled"); + let instance = tmp.path().join("instance"); + let lock = tmp.path().join("skills-lock.json"); + // instance currently matches the OLD bundle + write_skill(&instance, "alpha", "v1"); + seed_lock(&lock, &instance); + // bundle now has v2 + write_skill(&bundled, "alpha", "v2"); + + let report = update_skills(&bundled, &instance, &lock).await.unwrap(); + assert_eq!(report.updated, vec!["alpha".to_string()]); + assert!(report.backed_up.is_empty()); + assert_eq!( + std::fs::read_to_string(instance.join("alpha/SKILL.md")).unwrap(), + "v2" + ); + } + + #[tokio::test] + async fn locally_modified_skill_is_backed_up() { + let tmp = tempfile::tempdir().unwrap(); + let bundled = tmp.path().join("bundled"); + let instance = tmp.path().join("instance"); + let lock = tmp.path().join("skills-lock.json"); + write_skill(&instance, "alpha", "v1"); + seed_lock(&lock, &instance); + // user edited the instance copy + std::fs::write(instance.join("alpha/SKILL.md"), "user-edit").unwrap(); + // bundle moved to v2 + write_skill(&bundled, "alpha", "v2"); + + let report = update_skills(&bundled, &instance, &lock).await.unwrap(); + assert_eq!(report.backed_up, vec!["alpha".to_string()]); + assert_eq!( + std::fs::read_to_string(instance.join("alpha/SKILL.md")).unwrap(), + "v2" + ); + assert_eq!( + std::fs::read_to_string(instance.join("alpha.bak/SKILL.md")).unwrap(), + "user-edit" + ); + } + + #[tokio::test] + async fn instance_only_skill_is_untouched() { + let tmp = tempfile::tempdir().unwrap(); + let bundled = tmp.path().join("bundled"); + let instance = tmp.path().join("instance"); + let lock = tmp.path().join("skills-lock.json"); + write_skill(&bundled, "alpha", "v1"); + write_skill(&instance, "alpha", "v1"); + write_skill(&instance, "mine", "private"); + seed_lock(&lock, &instance); + + let report = update_skills(&bundled, &instance, &lock).await.unwrap(); + // alpha unchanged (same hash), mine never visited + assert!(report.updated.is_empty()); + assert!(report.backed_up.is_empty()); + assert_eq!( + std::fs::read_to_string(instance.join("mine/SKILL.md")).unwrap(), + "private" + ); + } + + #[tokio::test] + async fn unchanged_skill_without_lock_entry_seeds_lock() { + let tmp = tempfile::tempdir().unwrap(); + let bundled = tmp.path().join("bundled"); + let instance = tmp.path().join("instance"); + let lock = tmp.path().join("skills-lock.json"); + write_skill(&bundled, "alpha", "v1"); + write_skill(&instance, "alpha", "v1"); + + let report = update_skills(&bundled, &instance, &lock).await.unwrap(); + assert_eq!(report.skipped, vec!["alpha".to_string()]); + + let seeded = read_lock(&lock); + assert!( + seeded.skills.contains_key("alpha"), + "skip path should seed missing lock entry" + ); + } + + #[tokio::test] + async fn unchanged_skill_preserves_existing_lock_entry() { + let tmp = tempfile::tempdir().unwrap(); + let bundled = tmp.path().join("bundled"); + let instance = tmp.path().join("instance"); + let lock = tmp.path().join("skills-lock.json"); + write_skill(&bundled, "alpha", "v1"); + write_skill(&instance, "alpha", "v1"); + + write_lock( + &lock, + &SkillLock { + version: 1, + skills: BTreeMap::from([("alpha".to_string(), "old-hash".to_string())]), + }, + ) + .unwrap(); + + let report = update_skills(&bundled, &instance, &lock).await.unwrap(); + assert_eq!(report.skipped, vec!["alpha".to_string()]); + + let seeded = read_lock(&lock); + assert_eq!(seeded.skills.get("alpha"), Some(&"old-hash".to_string())); + } +} diff --git a/src/supervisor/classifier.rs b/src/supervisor/classifier.rs index 5569fa8..56010d3 100644 --- a/src/supervisor/classifier.rs +++ b/src/supervisor/classifier.rs @@ -181,17 +181,21 @@ mod tests { #[tokio::test] async fn skill_hint_overrides_default_workflow() { let mut registry = crate::skills::SkillRegistry::new(); - registry.register(crate::skills::Skill { - name: "sup-research".into(), - description: "research".into(), - content: "".into(), - tags: vec![], - model: None, - tools: vec![], - max_iterations: None, - supervisor_workflow: Some("research".into()), - supervisor_required_caps: vec!["research".into()], - }); + registry.register( + crate::skills::Skill { + name: "sup-research".into(), + description: "research".into(), + content: "".into(), + tags: vec![], + model: None, + tools: vec![], + max_iterations: None, + supervisor_workflow: Some("research".into()), + supervisor_required_caps: vec!["research".into()], + }, + crate::skills::SkillSource::Instance, + std::path::PathBuf::from("/tmp/test"), + ); let c = SkillAwareClassifier::new(HeuristicClassifier, registry); // Request must contain the skill's keyword ("research", from "sup-research") for the // hint to fire; the heuristic still classifies it as GeneralAssistant on the diff --git a/tests/home_sandbox.rs b/tests/home_sandbox.rs new file mode 100644 index 0000000..59d6009 --- /dev/null +++ b/tests/home_sandbox.rs @@ -0,0 +1,31 @@ +use rustfox::config::Config; + +#[test] +fn sandbox_defaults_to_home_workspace_and_excludes_secrets() { + let tmp = tempfile::tempdir().unwrap(); + let home = tmp.path().join(".rustfox"); + let cfg_path = tmp.path().join("config.toml"); + let toml = format!( + r#" + [telegram] + bot_token = "tok" + allowed_user_ids = [1] + [openrouter] + api_key = "key" + [general] + home = "{}" + "#, + home.display() + ); + std::fs::write(&cfg_path, toml).unwrap(); + let cfg = Config::load(&cfg_path).unwrap(); + + // Sandbox is the workspace subdir of home. + assert_eq!(cfg.sandbox.allowed_directory, home.join("workspace")); + // DB lives ABOVE the sandbox → structurally unreachable by file tools. + assert_eq!(cfg.memory.database_path, home.join("rustfox.db")); + assert!(!cfg + .memory + .database_path + .starts_with(&cfg.sandbox.allowed_directory)); +} diff --git a/tests/supervisor_skill_packs.rs b/tests/supervisor_skill_packs.rs index 2b30b42..55f8f0a 100644 --- a/tests/supervisor_skill_packs.rs +++ b/tests/supervisor_skill_packs.rs @@ -1,8 +1,12 @@ #[tokio::test] async fn ships_five_supervisor_skill_packs() { - let skills = rustfox::skills::loader::load_skills_from_dir(std::path::Path::new("skills")) - .await - .unwrap(); + let skills = rustfox::skills::loader::load_skills_from_dir( + std::path::Path::new("skills"), + rustfox::skills::SkillSource::Bundled, + std::path::PathBuf::from("skills"), + ) + .await + .unwrap(); for n in [ "sup-coding", "sup-research",