diff --git a/docs/context_providers.md b/docs/context_providers.md index e673349..c4fde2c 100644 --- a/docs/context_providers.md +++ b/docs/context_providers.md @@ -83,4 +83,4 @@ | `navi/context_providers/` | Built-in providers (server code) | | `context_providers/` | User-written providers (hot-reloadable) | | `context_providers/_template.py` | Format reference (not loaded) | -| `manuals/write_context_provider.md` | Navi's how-to guide | +| `manuals/guides/write_context_provider.md` | Navi's how-to guide | diff --git a/manuals/code_exec.md b/manuals/code_exec.md new file mode 100644 index 0000000..f910f25 --- /dev/null +++ b/manuals/code_exec.md @@ -0,0 +1,46 @@ +# code_exec — Manual + +## What it does +Runs Python in a subprocess and returns stdout (and stderr, prefixed `[stderr]`). The interpreter is **fresh on every call** — no state persists between calls, so import everything you need in each script. + +Success means exit code 0. A non-zero exit is returned as `success: false` with the exit code and the full output, which is usually the traceback: read it, it is your own bug, not a tool failure. + +## Parameters + +| Parameter | Required | Description | +|-----------|----------|-------------| +| `code` | yes | Python source to execute. | +| `working_dir` | no | Directory to run in (see [Limits](#limits)). Defaults to the session's working directory. | +| `timeout` | no | Seconds, default 30, clamped to 1–300. | +| `background` | no | Detach; returns a `task_id` immediately. | + +```json +{"code": "import json, pathlib\nprint(json.loads(pathlib.Path('package.json').read_text())['version'])"} +{"code": "print(sum(len(l) for l in open('data.csv')))", "working_dir": "/home/user/project"} +{"code": "import time\nfor i in range(5):\n print(i); time.sleep(1)", "timeout": 120} +``` + +## When to use it + +Use `code_exec` for anything that is properly a *program*: parsing and transforming data, calculation, text munging, calling a Python library, a quick check of a value. Use `terminal` instead when the task is shell-native — pipes, `grep`/`awk`, system commands, a CLI tool. + +A script whose output you then have to read back is often slower than one `filesystem`/`terminal` call. Reach for Python when the alternative is a long chain of tool calls, not by reflex. + +## Detaching long runs + +Pass `"background": true` for a suite, a large computation or anything that outlives the turn. You get a `task_id` back immediately and the real result arrives as a note at the start of a later turn; check it with the `tasks` tool. A detached call **without** an explicit `timeout` gets 300 s from the executor — so pass `timeout` when the work legitimately needs the ceiling, and remember 300 s is the maximum either way. + +## Limits + +- Timeout is clamped: `0` becomes 1, `10000` becomes 300, and a non-numeric value falls back to 30. You cannot get more than 300 s in the foreground — use a persistent terminal for genuinely long jobs. +- For a non-admin user the working directory is confined to `user_data//`; an absolute `working_dir` outside it is silently replaced with the sandbox root, and relative paths are resolved inside it. Admin and single-user runs use the client's working directory. +- Temp scripts are written inside that directory and deleted afterwards. +- On timeout the process is killed and you get `Code execution timed out after Ns` with `error: timeout` — a timeout is not a partial result; nothing from the run is returned. + +## Common mistakes + +- Assuming imports or variables survive between calls. They do not — every call starts clean. +- `print()`-less scripts: nothing is written to stdout means `(no output)`, which looks like a silent failure but is a successful run. +- Using `code_exec` to run `subprocess.run("ls")` when `terminal` or `filesystem list` was the tool. +- Leaving the default 30 s on a test suite that needs 90, then reading the timeout as a broken test. +- Writing to a path outside the sandbox and getting a file that is not where you expected. diff --git a/manuals/compile_scad.md b/manuals/compile_scad.md new file mode 100644 index 0000000..753a660 --- /dev/null +++ b/manuals/compile_scad.md @@ -0,0 +1,92 @@ +# mcp__navi-3d__compile_scad + +## Что делает + +Компилирует существующий OpenSCAD-скрипт (`.scad`) в binary STL-файл. + +**Требует:** OpenSCAD установлен в системе (`openscad` в PATH). + +## Предпосылки + +1. **Файл `.scad` уже должен существовать.** Напишите его заранее через `filesystem write`. +2. **OpenSCAD должен быть установлен.** Если нет — инструмент вернёт ошибку `openscad_not_found`. + +## Формат вызова + +```python +mcp__navi-3d__compile_scad( + session_id="...", + source_path="handle.scad", + output_path="handle.stl" +) +``` + +## Параметры + +| Параметр | Обязательно | Описание | +|---|---|---| +| `session_id` | Да | UUID текущей сессии Navi. Файлы разрешаются внутри `session_files//`. | +| `source_path` | Да | Путь к существующему `.scad`-файлу (внутри сессии или абсолютный) | +| `output_path` | Да | Путь, куда записать сгенерированный `.stl`. Родительские директории создаются автоматически. | + +## Workflow + +### 1. Написать OpenSCAD-скрипт + +``` +filesystem write session_files/sess-abc/bracket.scad ' +difference() { + cube([40, 20, 5], center=true); + translate([15, 0, 0]) cylinder(h=6, d=4, center=true); + translate([-15, 0, 0]) cylinder(h=6, d=4, center=true); +} +' +``` + +### 2. Проверить линтингом (рекомендуется) + +``` +mcp__navi-3d__lint_scad( + session_id="sess-abc", + source_path="bracket.scad" +) +``` + +### 3. Скомпилировать в STL + +``` +mcp__navi-3d__compile_scad( + session_id="sess-abc", + source_path="bracket.scad", + output_path="bracket.stl" +) +``` + +### 4. Показать пользователю + +``` +content_publish(filename="bracket.stl", title="Bracket") +``` + +## Что возвращает + +При успехе: +``` +Generated: bracket.stl +Path: /home/.../session_files/sess-abc/bracket.stl +Size: 12.4 KB +``` + +При ошибке: +- `openscad_not_found` — OpenSCAD не установлен +- `scad_not_found` — исходный файл не найден +- `openscad_compile_error` — ошибка компиляции (невалидный CSG, деление на ноль и т.д.) +- `no_output` — OpenSCAD завершился без ошибок, но файл не создался +- `wrong_session_dir` — файл находится вне разрешённой сессии + +## Важные правила + +1. **Всегда binary STL** — нет параметра ASCII/binary, всегда используется `--export-format binstl`. +2. **Скрипт должен существовать до вызова** — `compile_scad` не пишет `.scad`, только компилирует. +3. **Всегда передавайте `session_id`** — без него инструмент не знает, в какой директории искать файлы. +4. **Ошибки OpenSCAD** читаемы: строка, символ, тип ошибки — всё в stderr. diff --git a/manuals/create_mcp_server.md b/manuals/create_mcp_server.md new file mode 100644 index 0000000..7e3dd07 --- /dev/null +++ b/manuals/create_mcp_server.md @@ -0,0 +1,224 @@ +# Writing MCP Servers for Navi + +This manual describes how to create, test, register, and maintain MCP servers that extend Navi's capabilities. Read this **before** you start building a new server. + +## 1. Philosophy: Why MCP instead of user tools? + +MCP servers run in isolated processes and communicate via the Model Context Protocol. They **cannot** crash Navi's core, they can be reloaded without restarting the server, and they scale to complex external integrations (APIs, databases, browsers, etc.). The trade-off is slightly more boilerplate than a single `tools/foo.py` file. + +**Rule:** Every new capability that is not trivial (more than a simple datetime or notes lookup) should be built as an MCP server. + +## 2. Directory structure + +MCP servers live under: + +``` +mcp-servers// +├── pyproject.toml +├── README.md +└── app/ + ├── __init__.py + └── mcp_server.py +``` + +- `` — snake_case or kebab-case. Must match the key you will use in `mcp_servers.d/.json`. +- `pyproject.toml` — Python package metadata and dependencies. +- `app/mcp_server.py` — the actual server code (FastMCP). + +## 3. Creating a new server from the template + +Use the built-in tool `create_mcp_server` (preferred) or copy the template manually: + +```bash +cp -r mcp-servers/_template mcp-servers/my_server +cd mcp-servers/my_server +# Edit pyproject.toml: change name, description, add dependencies +# Edit app/mcp_server.py: add your tools and instructions +``` + +### 3.1 pyproject.toml + +Minimal required fields: + +```toml +[build-system] +requires = ["setuptools>=61.0"] +build-backend = "setuptools.build_meta" + +[project] +name = "mcp-server-myserver" +version = "0.1.0" +description = "What this server does" +requires-python = ">=3.11" +dependencies = [ + "mcp>=1.27", + "pydantic>=2.0", + # add your own: httpx, asyncpg, playwright, etc. +] + +[project.scripts] +mcp-server-myserver = "app.mcp_server:main" + +[tool.setuptools.packages.find] +where = ["."] +include = ["app*"] +``` + +### 3.2 app/mcp_server.py + +Read the template at `mcp-servers/_template/app/mcp_server.py` first. It contains a working hello-world server with extensive inline comments. + +Key sections you must edit: + +1. **`INSTRUCTIONS`** — These are injected into Navi's system prompt. Describe: + - What this server does and when to use it. + - Recommended workflow (order of tool calls). + - ABSOLUTE RULE about never bypassing these tools with filesystem/terminal. + +2. **`mcp = FastMCP("name", instructions=INSTRUCTIONS)`** — The name should match the directory key. + +3. **Tool functions** — Each tool: + - Is an `async def`. + - Uses `@mcp.tool(name="tool_name")`. + - Parameters use `Annotated[T, Field(description="...")]` — never plain types. + - Returns a plain `str` (JSON string for structured data is fine). + - Raises on real errors. + - Validates required params explicitly. + +Example: + +```python +@mcp.tool(name="search_docs") +async def search_docs_tool( + query: Annotated[str, Field(description="Search query string.")], + limit: Annotated[int, Field(description="Max results.")] = 10, +) -> str: + """Search the documentation index.""" + if not query.strip(): + raise ValueError("query is required and cannot be empty.") + results = await _do_search(query, limit) + return json.dumps(results, ensure_ascii=False, indent=2) +``` + +## 4. Environment and installation + +After creating files, you **must**: + +1. Create a virtual environment: + ```bash + python -m venv .venv + source .venv/bin/activate + pip install -e . + ``` + +2. Verify the server starts without crashing: + ```bash + timeout 5 python -m app.mcp_server || true + ``` + If it prints a traceback, fix the code before proceeding. + +## 5. Registering the server in Navi + +Create a file `mcp_servers.d/.json` in the project root. The filename (without `.json`) becomes the server name. Example for a server named `my_server`: + +```json +{ + "transport": "stdio", + "command": "/absolute/path/to/mcp-servers/my_server/.venv/bin/python", + "args": ["-m", "app.mcp_server"], + "cwd": "/absolute/path/to/mcp-servers/my_server", + "env": { + "MCP_TRANSPORT": "stdio" + }, + "groups": { + "default": ["search_docs", "read_doc"] + }, + "instructions": "Optional extra instructions merged with the server's own INSTRUCTIONS." +} +``` + +**Critical fields:** +- `command` — absolute path to the venv's Python binary. +- `cwd` — absolute path to the server directory. +- The filename must be `.json` (e.g. `my_server.json`). +- `args` — usually `["-m", "app.mcp_server"]`. +- `groups` — organize tools into named groups so profiles can reference them cleanly. + +After editing `mcp_servers.d/.json`, call `reload_tools` to connect the server and register its tools. + +## 6. Testing + +### 6.1 Check connection + +Call `mcp_status`. You should see your server as `connected` with the correct tool count. + +### 6.2 Test each tool + +Call `test_mcp_tool` for every tool your server exposes: + +``` +test_mcp_tool(server_name="my_server", tool_name="search_docs", arguments={"query": "hello", "limit": 3}) +``` + +If any tool fails, read the error output, fix the code in `app/mcp_server.py`, and repeat. + +### 6.3 Manual stderr inspection + +If `mcp_status` shows `disconnected` but the code looks correct, inspect stderr manually: + +```bash +cd mcp-servers/my_server +.venv/bin/python -m app.mcp_server 2>&1 | head -n 20 +``` + +## 7. Updating an MCP server + +1. Edit the code in `mcp-servers//app/mcp_server.py`. +2. (Optional) If you added new dependencies, edit `pyproject.toml` and run `pip install -e .` inside the venv. +3. Call `reload_tools` to reconnect the server and re-register tools. +4. Call `test_mcp_tool` to verify. + +## 8. Deleting an MCP server + +1. Remove the server directory or move it to a backup location. +2. Remove the entry from `mcp_servers.d/.json`. +3. Call `reload_tools`. + +## 9. Connecting an external MCP server + +If the server was written by someone else: + +1. Clone or place the server code on disk. +2. Create its venv and install dependencies. +3. Read its README to learn tool names and required environment variables. +4. Add an entry to `mcp_servers.d/.json` with the correct `command`, `cwd`, `args`, and `env`. +5. Define `groups` mapping the tools into logical sets. +6. Call `reload_tools`. +7. Call `test_mcp_tool` for a representative tool. + +## 10. Common mistakes and debugging + +| Symptom | Cause | Fix | +|---|---|---| +| `mcp_status` shows `disconnected` | Wrong `command` or `cwd` path | Double-check absolute paths | +| Traceback on startup | Syntax error or missing import | Run `python -m py_compile app/mcp_server.py` | +| `test_mcp_tool` returns `is_error=True` | Tool raised an exception | Fix the tool logic; check parameter validation | +| Tool schema missing descriptions | Used plain types instead of `Annotated[..., Field(...)]` | Add `Field(description=...)` to every parameter | +| Navi never calls the server | Profile does not map the server in `mcp_servers` | Edit the profile's `config.json` and add the server groups | +| Navi bypasses MCP with filesystem | `INSTRUCTIONS` missing ABSOLUTE RULE | Add explicit rule in server INSTRUCTIONS | + +## 11. Workflow checklist for Navi + +When asked to create a new MCP server: + +1. Read this manual (`manuals/create_mcp_server.md`). +2. Read the template (`mcp-servers/_template/app/mcp_server.py`). +3. Call `create_mcp_server(name=..., description=...)` to scaffold the directory. +4. Edit `app/mcp_server.py` iteratively using `filesystem`. +5. Validate syntax: `code_exec` or `terminal` with `python -m py_compile ...`. +6. Test startup: `terminal` with `timeout 5 python -m app.mcp_server`. +7. Edit `mcp_servers.d/.json` via `filesystem` to register the server. +8. Call `reload_tools`. +9. Call `test_mcp_tool` for every tool. +10. Write good `INSTRUCTIONS` inside `mcp_server.py`. +11. Report results to the user. diff --git a/manuals/filesystem.md b/manuals/filesystem.md new file mode 100644 index 0000000..96f56e8 --- /dev/null +++ b/manuals/filesystem.md @@ -0,0 +1,66 @@ +# filesystem — Manual + +## What it does +Reads and edits files and directories. The action that matters most is picking the cheapest deterministic way to make a change: an AI-assisted edit is the last resort, never the first. + +## Editing policy — in this order + +1. **`edit`** — exact text. Pass `old` (which must occur **exactly once** in the file) and `new`. Read the file first and copy `old` verbatim, including indentation. This is the default for almost every change, and it either applies exactly or fails loudly — it cannot quietly edit the wrong line. +2. **`edit_lines`** — line numbers. Pass an `operations` array when you know the exact lines ("change line 15"). Deterministic, no AI call. +3. **`smart_edit`** — AI-assisted, for changes that genuinely cannot be expressed as text or line numbers: "rename this symbol everywhere", "add type hints to every function". It costs an LLM call and reads the whole file. Try the two above first, always. +4. **`write`** — create a file or rewrite it entirely (pass `content`). +5. **`query`** — ask a question about a file's content and get an answer instead of the file. +6. Everything else — `read`, `append`, `list`, `find`, `find_up`, `grep`, `diff`, `info`, `copy`, `move`, `delete`, `exists`, `mkdir`. + +## Parameters + +| Parameter | Used by | Description | +|-----------|---------|-------------| +| `action` | required | One of the actions listed above. | +| `path` | required | File or directory. `~` is expanded. | +| `content` | `write`, `append` | Text to write or append. | +| `old` / `new` | `edit` | Exact text to replace, and its replacement. | +| `operations` | `edit_lines` | Array of `{"op": "replace"\|"delete"\|"insert", "start", "end", "after", "content"}`. Lines are 1-based and inclusive; `insert` uses `after`. | +| `destination` | `move`, `copy`, `diff` | Target path, or the second file to compare against. Missing parent directories are created. | +| `pattern` | `find`, `find_up`, `grep` | Glob for `find` (e.g. `*.log`), exact filename for `find_up`, search text for `grep`. | +| `glob` | `grep` | Optional filename filter for a recursive search, e.g. `*.py`. | +| `regex` | `grep` | `true` for pattern matching, default `false` = literal substring. | +| `question` | `query` | The question to answer from the file. | +| `instruction` | `smart_edit` | The change to make, in natural language. | +| `offset` / `limit` | `read` | First line (1-based) and how many to return. | +| `numbered` | `read` | Default `true`, prefixed with 1-based line numbers. Set `false` for raw content you are about to copy into an edit. | +| `recursive` | `list` | Full tree instead of the top level. | + +## Actions worth knowing + +```json +{"action": "info", "path": "src/app.py"} +{"action": "read", "path": "src/app.py", "offset": 40, "limit": 30} +{"action": "read", "path": "src/app.py", "numbered": false} + +{"action": "edit", "path": "src/app.py", "old": " return None", "new": " return {}"} +{"action": "edit_lines", "path": "src/app.py", + "operations": [{"op": "replace", "start": 15, "end": 15, "content": " limit = 50"}]} + +{"action": "grep", "path": "src", "pattern": "TODO", "glob": "*.py"} +{"action": "find", "path": ".", "pattern": "*.log"} +{"action": "find_up", "path": "src/deep/file.py", "pattern": "pyproject.toml"} +{"action": "diff", "path": "src/app.py", "destination": "src/app.py.bak"} +``` + +- **`info` before `read`** on an unknown file — it reports the size, which tells you whether a full read is going to flood the context. +- **`find_up`** walks from `path` towards the root looking for an exact filename — the way to locate `pyproject.toml`/`.git` from a nested directory. +- **`diff`** is a unified diff between two **files** — `path` against `destination` (both required; directories are rejected). + +## Access + +In multi-user mode every path is resolved inside `user_data//`; a path that escapes it returns `Access denied: ... outside allowed paths`. Do not work around it by writing to `/tmp` or an absolute path — ask for the file to be placed in the workspace instead. In single-user/admin mode the allowed roots are the working directory and the configured paths. + +## Common mistakes + +- `edit` with an `old` string that appears twice — the call is refused, correctly. Include more surrounding context until it is unique. +- `old` copied from a numbered `read` — the line-number column is not part of the file. Read with `numbered: false` when copying text into an edit. +- `write` to change one line: it replaces the whole file, and anything you did not retype is gone. Use `edit`. +- `smart_edit` for something `edit` could do — an LLM call to change a line is a waste, and it can touch more than you asked. +- `delete` on a directory — it removes the whole tree recursively, with no prompt and no undo. +- `mkdir` for a path whose parent does not exist: it does not create intermediate directories. `write` does. diff --git a/manuals/guides/write_context_provider.md b/manuals/guides/write_context_provider.md new file mode 100644 index 0000000..3a2fe9a --- /dev/null +++ b/manuals/guides/write_context_provider.md @@ -0,0 +1,68 @@ +# Writing a Context Provider + +Context providers inject dynamic runtime data as system messages into every LLM call. Use them when information is needed frequently and fetching it on-demand every time would be wasteful (current URL, hostname, server status, etc.). + +## When to use a context provider vs a tool + +- **Context provider**: data that is useful on *every* call without the model asking — server address, current environment, active config values. +- **Tool**: data that is needed occasionally on demand — current weather, a file's contents, search results. + +## File format + +Create a file in `context_providers/` at the project root. File name must not start with `_`. + +```python +# context_providers/my_provider.py + +name = "my_provider" +description = "One-line description of what this injects." +global_provider = False # True = injected in ALL profiles automatically + +async def get_context() -> str | None: + """Return a string or None. None means skip silently.""" + return "[System] My info: ..." +``` + +### Required fields +| Field | Type | Description | +|---|---|---| +| `name` | `str` | Unique identifier | +| `description` | `str` | What data this provides | +| `get_context` | `async def` | Returns `str` to inject or `None` to skip | + +### Optional fields +| Field | Type | Default | Description | +|---|---|---|---| +| `global_provider` | `bool` | `False` | `True` = inject in every profile; `False` = only in profiles that list this name in `context_providers` | + +## Enabling per-profile + +For non-global providers, add the name to the profile's `config.json`: +```json +"context_providers": ["my_provider"] +``` + +## Activation + +After writing, call `reload_tools` — it reloads both tools and context providers. The new data will appear in the next LLM call. + +## Example: inject current server hostname + +```python +# context_providers/hostname.py +import socket + +name = "hostname" +description = "Injects the current server hostname." +global_provider = True + +async def get_context() -> str | None: + return f"[System] Server hostname: {socket.gethostname()}" +``` + +## Notes + +- Return value is injected as `role="system"` right after the memory summary, before conversation history. +- Exceptions in `get_context()` are caught and logged — they never crash the agent. +- `global_provider = True` providers are loaded for every profile; no config change needed. +- Built-in providers live in `navi/context_providers/` and cannot be overwritten by user providers. diff --git a/manuals/lint_scad.md b/manuals/lint_scad.md index a0808ff..89b533c 100644 --- a/manuals/lint_scad.md +++ b/manuals/lint_scad.md @@ -1,4 +1,4 @@ -# mcp__navi_3d__lint_scad +# mcp__navi-3d__lint_scad ## Что делает @@ -11,7 +11,7 @@ ## Формат вызова ```python -mcp__navi_3d__lint_scad( +mcp__navi-3d__lint_scad( session_id="...", source_path="bracket.scad" ) @@ -40,7 +40,7 @@ ### 2. Запустить линтинг ``` -mcp__navi_3d__lint_scad( +mcp__navi-3d__lint_scad( session_id="sess-abc", source_path="bracket.scad" ) @@ -55,7 +55,7 @@ После чистого линта: ``` -mcp__navi_3d__compile_scad( +mcp__navi-3d__compile_scad( session_id="sess-abc", source_path="bracket.scad", output_path="bracket.stl" diff --git a/manuals/memory.md b/manuals/memory.md new file mode 100644 index 0000000..1c2186e --- /dev/null +++ b/manuals/memory.md @@ -0,0 +1,82 @@ +# memory — Manual + +## What it does +Stores facts about the user that outlive the session: who they are, what they prefer, which hosts and tools they run, what they are working on. Anything true tomorrow but not worth rediscovering belongs here; anything true only for this turn belongs in `scratchpad`. + +Facts are scoped to the user. A save is an **upsert** on `(category, key)` — writing the same key again overwrites it rather than adding a second fact, so correct a wrong fact by saving over it. + +## Parameters + +| Parameter | Used by | Description | +|-----------|---------|-------------| +| `action` | required | `save` \| `search` \| `forget` \| `list`. | +| `query` | `search` | Keywords describing what to look for. | +| `category` | `save`, `forget` | `profile` \| `preferences` \| `technical` \| `projects` \| `other`. | +| `key` | `save`, `forget` | `snake_case` identifier, unique within the category. | +| `value` | `save` | The fact, one concise plain-text statement. | +| `source` | `save` | `conversation` (default) \| `tool_call` \| `auto_discovery` \| `user_explicit`. | +| `confidence` | `save` | 0–100, default 70. | +| `expires_days` | `save` | Days until the fact expires. Omit for "never". | +| `source_context` | `save` | Provenance, e.g. `found via ip addr on localhost`. | + +Categories: `profile` = who they are, `preferences` = likes/dislikes, `technical` = OS/tools/servers, `projects` = ongoing work, `other` = everything else. + +## Actions + +### `save` +Requires `category`, `key` and `value`; omitting any of the three fails the call with a specific message. An invalid `category` is rejected; an invalid `source` is silently coerced to `conversation`, and `confidence` is clamped to 0–100. + +```json +{"action": "save", "category": "preferences", "key": "response_language", + "value": "Prefers all answers in Russian", "source": "user_explicit", + "confidence": 95, "source_context": "user asked directly in session"} + +{"action": "save", "category": "technical", "key": "prod_server_ip", + "value": "Production runs on 192.168.1.168 (branch master, docker postgres)", + "source": "tool_call", "confidence": 95, "expires_days": 7, + "source_context": "found via ssh_exec hostname/ip addr"} +``` + +Calibrate the two numbers honestly — they are what makes a fact worth trusting later: + +| How you learned it | `source` | `confidence` | +|---|---|---| +| Ran a tool and read the output | `tool_call` | 95 | +| The user told you | `user_explicit` | 80–95 | +| Extracted from the conversation | `conversation` | 70 | +| Read it on the web | `auto_discovery` | 50 | +| Inferred / guessed | `conversation` | 30 | + +**System facts expire.** An IP, a running service, a host's disk layout — set `expires_days: 7`. A stale host address is worse than no address, because it is trusted. Durable preferences and identity never expire. + +### `search` +Requires `query`. Returns up to 15 matching facts, each with its category, key, value and provenance (`src`, `conf`, `ctx`). This is the read path — search before saving, so you update an existing key instead of inventing a near-duplicate. + +```json +{"action": "search", "query": "server ip"} +{"action": "search", "query": "language preference"} +``` + +### `forget` +Requires `key`; `category` optionally narrows it. Returns how many facts were deleted; deleting nothing is an error (`not found`), not a silent success. + +```json +{"action": "forget", "key": "prod_server_ip", "category": "technical"} +``` + +### `list` +Returns the **categories** that hold facts, not the facts themselves: + +```json +{"action": "list"} +``` + +Use it to see the shape of what is stored; use `search` to read the content. + +## Rules + +- Do not store what the code or the session already says — no facts about the current conversation's files, and nothing recoverable from one tool call. +- One fact per key. `home_server_ip` and `prod_server_ip` are different facts; `server` holding three sentences is one unusable blob. +- `value` is a statement, not a field name: "Home server is 192.168.1.168", not "192.168.1.168". +- Search before saving. The upsert only overwrites when the key matches exactly. +- Never save secrets — passwords, tokens, keys. Record where they live, not what they are. diff --git a/manuals/model_3d.md b/manuals/model_3d.md deleted file mode 100644 index 4eafde2..0000000 --- a/manuals/model_3d.md +++ /dev/null @@ -1,92 +0,0 @@ -# mcp__navi_3d__compile_scad - -## Что делает - -Компилирует существующий OpenSCAD-скрипт (`.scad`) в binary STL-файл. - -**Требует:** OpenSCAD установлен в системе (`openscad` в PATH). - -## Предпосылки - -1. **Файл `.scad` уже должен существовать.** Напишите его заранее через `filesystem write`. -2. **OpenSCAD должен быть установлен.** Если нет — инструмент вернёт ошибку `openscad_not_found`. - -## Формат вызова - -```python -mcp__navi_3d__compile_scad( - session_id="...", - source_path="handle.scad", - output_path="handle.stl" -) -``` - -## Параметры - -| Параметр | Обязательно | Описание | -|---|---|---| -| `session_id` | Да | UUID текущей сессии Navi. Файлы разрешаются внутри `session_files//`. | -| `source_path` | Да | Путь к существующему `.scad`-файлу (внутри сессии или абсолютный) | -| `output_path` | Да | Путь, куда записать сгенерированный `.stl`. Родительские директории создаются автоматически. | - -## Workflow - -### 1. Написать OpenSCAD-скрипт - -``` -filesystem write session_files/sess-abc/bracket.scad ' -difference() { - cube([40, 20, 5], center=true); - translate([15, 0, 0]) cylinder(h=6, d=4, center=true); - translate([-15, 0, 0]) cylinder(h=6, d=4, center=true); -} -' -``` - -### 2. Проверить линтингом (рекомендуется) - -``` -mcp__navi_3d__lint_scad( - session_id="sess-abc", - source_path="bracket.scad" -) -``` - -### 3. Скомпилировать в STL - -``` -mcp__navi_3d__compile_scad( - session_id="sess-abc", - source_path="bracket.scad", - output_path="bracket.stl" -) -``` - -### 4. Показать пользователю - -``` -content_publish(filename="bracket.stl", title="Bracket") -``` - -## Что возвращает - -При успехе: -``` -Generated: bracket.stl -Path: /home/.../session_files/sess-abc/bracket.stl -Size: 12.4 KB -``` - -При ошибке: -- `openscad_not_found` — OpenSCAD не установлен -- `scad_not_found` — исходный файл не найден -- `openscad_compile_error` — ошибка компиляции (невалидный CSG, деление на ноль и т.д.) -- `no_output` — OpenSCAD завершился без ошибок, но файл не создался -- `wrong_session_dir` — файл находится вне разрешённой сессии - -## Важные правила - -1. **Всегда binary STL** — нет параметра ASCII/binary, всегда используется `--export-format binstl`. -2. **Скрипт должен существовать до вызова** — `compile_scad` не пишет `.scad`, только компилирует. -3. **Всегда передавайте `session_id`** — без него инструмент не знает, в какой директории искать файлы. -4. **Ошибки OpenSCAD** читаемы: строка, символ, тип ошибки — всё в stderr. diff --git a/manuals/notify.md b/manuals/notify.md new file mode 100644 index 0000000..81be0dc --- /dev/null +++ b/manuals/notify.md @@ -0,0 +1,48 @@ +# notify — Manual + +## What it does +Pushes a notification to the user's device — application web push, a Synapse event, or both, depending on the user's notification settings. It exists to reach the user **when the turn is not enough**: a background task finished, something broke, or a decision is needed before work can continue. + +It is not a progress log. A notification interrupts; each one should be something the user would want to know now. + +## Parameters + +| Parameter | Required | Description | +|-----------|----------|-------------| +| `message` | yes | One plain sentence or two. | +| `level` | no | `info` (default) \| `warning` \| `intervention`. | + +```json +{"message": "Backup finished: 4.2 GB, no errors.", "level": "info"} +{"message": "Deploy failed at the migration step — the DB is still on the old schema.", "level": "warning"} +{"message": "Delete the 12 archived sessions on prod? Two of them are referenced by open tasks. Need an answer before I continue.", "level": "intervention"} +``` + +Levels map to delivery priority and to the push title: + +| `level` | Meaning | Priority | +|---|---|---| +| `info` | FYI — the result is in, nothing needed | normal | +| `warning` | Something went wrong or degraded | high | +| `intervention` | The user's decision is needed now | critical | + +The notification titles are localised by level ("Navi: уведомление" / "Navi: предупреждение" / "Navi: нужно ваше решение") — that is the app's wording, not yours; the `message` is what you write. + +## Where it goes + +Delivery follows the user's `push_target` setting, and the result says exactly what happened: + +- **app** — web push to the running client. Skipped if push is not configured on this server. +- **app_synapse** — both legs. +- **synapse** — a `navi-notification` event through Synapse, with the level as the action and a dedup key derived from the message. Skipped if the Synapse source key is not configured. + +A successful call returns `success: true` and a report such as `Delivered: app, synapse.` or `Skipped: synapse (source key not configured).` **A skipped leg is not a failure** — the notification went where it could. Do not retry to force the other leg; the configuration is a user choice. + +The call fails only when there is genuinely nothing to send: no user context in the run (*No user context*), an empty `message`, or a `level` outside the three above. + +## Rules + +- **`intervention` must state the decision.** Say what you need decided and what happens next, so the user can answer from the notification without opening the session. "Please advise" is not a request. +- One notification per event. A background task that finished gets one note — not one when it starts, one at 50%, and one at the end. +- Do not notify for work the user is watching anyway, and do not notify to report your own progress. The push is for what they are *not* looking at. +- When a background task completes and the user is already in the session, a note in the reply is enough. diff --git a/manuals/plan.md b/manuals/plan.md new file mode 100644 index 0000000..7b4c1ca --- /dev/null +++ b/manuals/plan.md @@ -0,0 +1,59 @@ +# plan — Manual + +## What it does +Runs the planner over the live session: it decomposes the goal into milestones and steps (each tagged with an executor — `TOOL`, `AGENT` or `SELF`) and **populates the todo with those steps**. It reads the conversation, memory, the todo and the scratchpad's `findings`/`errors` sections, so a re-plan sees what you have already learned. + +It costs **2 LLM calls** (complexity analysis + execution plan). Use it selectively, like `reflect`. + +Requires an active agent run — outside one the call fails with *plan is not available in this context*. + +## Parameters + +| Parameter | Required | Description | +|-----------|----------|-------------| +| `reason` | for re-planning | What changed — the discovery that makes the current plan stale, in one or two concrete sentences. | +| `updated_goal` | no | The new success criterion, only meaningful together with `reason`. | + +```json +{} +{"reason": "the config is TOML, not JSON, so the parser step is wrong"} +{"reason": "the API needs pagination after all", "updated_goal": "sync all 12k records, not just the first page"} +``` + +## When to call it + +Call it **before** starting execution when the task is non-trivial: + +- multiple steps, or several files/systems involved; +- research or an unknown codebase, where the shape of the work is itself unclear; +- real risk — production, data loss, irreversible operations; +- the work needs sub-agent scoping decided up front. + +Skip it for trivial work: a single-file edit, a one-off command, a question, casual chat. Planning a one-liner wastes two LLM calls and produces a plan of one step. + +## Re-planning + +Pass `reason` when the plan's **overall structure** is wrong — a step turned out unnecessary, the real problem differs from the assumed one, or new constraints appeared. The new plan replaces the todo; completed work survives in the conversation and the scratchpad. + +Do **not** re-plan for a single failed step, or to drop/merge/reorder one or two steps — edit the `todo` directly with `add`/`update`. The choice between the two tools: + +| Situation | Tool | +|---|---| +| One step failed, or the order of a couple of steps changed | `todo` | +| You are unsure *what* is wrong; the approach needs questioning | `reflect` | +| The remaining plan's overall shape is wrong, or there is no plan | `plan` | + +## What you get back + +`# Plan` (fresh) or `# Revised plan` (with `reason`), followed by the milestones and steps, followed by an instruction that depends on the assessed complexity: + +- **complex** — present the plan to the user in a few sentences and **wait for confirmation** before executing. Do not start yet. +- **not complex** — the todo has been populated; proceed from step 1, tracking progress with `todo`. + +When planning produces no plan at all, the call fails rather than inventing one: proceed directly (fresh) or keep the current plan and revise the todo inline (re-plan). Neither is an error to retry. + +## Rules + +- One plan per task. Calling it again mid-task without `reason` throws away the current plan and its progress. +- After a re-plan the todo reflects the new plan — the old steps are gone; do not `update` an index that no longer means what it did. +- The plan is not a substitute for doing the work: nothing in it is executed by calling it. diff --git a/manuals/reload_tools.md b/manuals/reload_tools.md new file mode 100644 index 0000000..f3e6321 --- /dev/null +++ b/manuals/reload_tools.md @@ -0,0 +1,65 @@ +# reload_tools — Manual + +## What it does +Reloads the things that can be changed without restarting the server: tool files from `tools/`, context providers from `context_providers/`, and every configured MCP server (reconnected, its tools re-registered). Call it after writing or editing any of them. + +It takes **no parameters**. + +```json +{} +``` + +## Reading the report + +The output is one line per fact, and the call's `success` is `false` if any of them reported an error: + +``` +Tools (4): get_current_datetime, gmail, weather, my_tool. +Tool errors (1): + broken.py: SyntaxError: invalid syntax (broken.py, line 3) +Context providers (2): hostname, server_status. +MCP tools (37): mcp__tgclient__..., mcp__navi-web__... +``` + +- **`Tool errors` / `Context provider errors`** — file by file. Errors are isolated: a broken file does not stop the others from loading, and does not stop the call from being useful. Fix the named file and reload again. +- **`Warning: enabled.json names N tool(s) that do not exist...`** — `tools/enabled.json` lists a name no tool answers to. Those names are silently dropped from every profile, so the tool you thought you enabled is simply absent. Check the name matches the tool's `name` exactly. +- **`MCP reload error`** — the MCP leg failed as a whole; the tools above it were still reloaded. + +## Writing a tool for it to load + +Put a `.py` file in `tools/`. Files starting with `_` are ignored (`_template.py` is a scaffold, not a tool). + +**Module style** — the simple form: + +```python +name = "notes" +description = "Save and retrieve short text notes by key. Actions: save, get, list." +parameters = { + "type": "object", + "properties": { + "action": {"type": "string", "enum": ["save", "get", "list"]}, + "key": {"type": "string", "description": "Note identifier"}, + "value": {"type": "string", "description": "Note content (for save)"}, + }, + "required": ["action"], +} + +async def execute(params: dict) -> str: + if params["action"] == "list": + return "No notes saved yet." + return f"Saved: {params['key']}" +``` + +All four names are required (`name`, `description`, `parameters`, `execute`), `execute` must be `async`, it takes `params` only, and it returns **a string** — a dict or `None` becomes `str(...)`. Raise an exception to signal failure: it surfaces as a failed tool result with the exception type. `description` is what the model reads when deciding whether to call the tool, so write it for that decision, not as a title. + +**Class style** — a subclass of `Tool` needs class-level `name`, `description`, `parameters`, and `execute(self, params)` or `execute(self, params, ctx=None)`. Other signatures are rejected with a "wrong signature" error, and the class must be instantiable with no arguments. + +To make the tool visible to a profile, its `name` must be declared — in the profile config, or in `tools/enabled.json`. A file that loads but is named nowhere is registered and still uncallable; `list_tools` is where that shows up. + +## Rules + +- Reload **after** the file is complete. A half-written file loads as a `Tool errors` line, and the report is then partly about your editor. +- Reloading MCP servers reconnects them — an in-flight MCP call is not something to reload through. Do it between steps, not mid-call. +- Context providers inject text into **every** system prompt. Reload after editing one, but change them deliberately: a bad provider costs tokens on every call, not just one. +- Do not call it in a loop. One reload after a batch of edits is enough. +- A reload is not needed for MCP tool *arguments* or for data files a tool reads — only for tool/provider/server definitions. diff --git a/manuals/render_3d.md b/manuals/render_3d.md deleted file mode 100644 index 98b4ac0..0000000 --- a/manuals/render_3d.md +++ /dev/null @@ -1,107 +0,0 @@ -# mcp__navi_3d__render_stl - -## Что делает - -Рендерит PNG-скриншоты ракурсов из STL-файла через OpenSCAD CLI. - -**Требует:** OpenSCAD установлен в системе (`openscad` в PATH). - -## Предпосылки - -1. **STL-файл уже должен существовать.** Сгенерируйте его через `mcp__navi_3d__compile_scad` заранее. -2. **OpenSCAD должен быть установлен.** - -## Формат вызова - -```python -mcp__navi_3d__render_stl( - session_id="...", - source_path="bracket.stl", - views=["iso", "front", "top"] -) -``` - -## Параметры - -| Параметр | Обязательно | Описание | -|---|---|---| -| `session_id` | Да | UUID текущей сессии Navi. Файлы разрешаются внутри `session_files//`. | -| `source_path` | Да | Путь к существующему `.stl`-файлу (внутри сессии или абсолютный) | -| `views` | Нет | Список ракурсов (макс. 3). Доступные: `front`, `back`, `top`, `bottom`, `left`, `right`, `iso`. По умолчанию `["iso"]`. | - -## Доступные ракурсы - -| Ракурс | Описание | -|---|---| -| `front` | Вид спереди | -| `back` | Вид сзади | -| `top` | Вид сверху | -| `bottom` | Вид снизу | -| `left` | Вид слева | -| `right` | Вид справа | -| `iso` | Изометрия (по умолчанию) | - -## Workflow - -### 1. Сгенерировать STL - -``` -mcp__navi_3d__compile_scad( - session_id="sess-abc", - source_path="bracket.scad", - output_path="bracket.stl" -) -``` - -### 2. Отрендерить ракурсы - -``` -mcp__navi_3d__render_stl( - session_id="sess-abc", - source_path="bracket.stl", - views=["iso", "front", "top"] -) -``` - -### 3. Проверить рендеры - -Каждый PNG сохраняется рядом с STL с суффиксом вида: -- `bracket.iso.png` -- `bracket.front.png` -- `bracket.top.png` - -Обычно эти PNG нужны Нави для внутренней проверки геометрии перед публикацией STL. Откройте их через `image_view` и проверьте форму, пропорции и физическую корректность. - -Не публикуйте PNG пользователю, если пользователь явно не попросил preview-картинки. - -``` -image_view(source="session_files/sess-abc/bracket.iso.png") -image_view(source="session_files/sess-abc/bracket.front.png") -image_view(source="session_files/sess-abc/bracket.top.png") -``` - -## Что возвращает - -При успехе: -``` -Generated 3 image(s): - bracket.iso.png - bracket.front.png - bracket.top.png -``` - -При ошибке: -- `openscad_not_found` — OpenSCAD не установлен -- `stl_not_found` — исходный файл не найден -- `too_many_views` — больше 3 ракурсов за раз -- `invalid_views` — неизвестное имя ракурса -- `render_failed` — все рендеры не удались - -## Важные правила - -1. **Максимум 3 ракурса за вызов** — если нужно больше, разбейте на несколько вызовов. -2. **Фиксированное разрешение** — 400×300, не настраивается. -3. **PNG сохраняются рядом с STL** — в той же директории, с суффиксом ракурса. -4. **Всегда preview mode** — быстрый рендер, не полный CSG. -5. **Не склеивает в сетку** — каждый ракурс — отдельный файл. PNG обычно нужны для внутренней проверки; публикуйте их только по явной просьбе пользователя. -6. **Всегда передавайте `session_id`** — без него инструмент не знает, в какой директории искать файлы. diff --git a/manuals/render_stl.md b/manuals/render_stl.md new file mode 100644 index 0000000..43f7542 --- /dev/null +++ b/manuals/render_stl.md @@ -0,0 +1,107 @@ +# mcp__navi-3d__render_stl + +## Что делает + +Рендерит PNG-скриншоты ракурсов из STL-файла через OpenSCAD CLI. + +**Требует:** OpenSCAD установлен в системе (`openscad` в PATH). + +## Предпосылки + +1. **STL-файл уже должен существовать.** Сгенерируйте его через `mcp__navi-3d__compile_scad` заранее. +2. **OpenSCAD должен быть установлен.** + +## Формат вызова + +```python +mcp__navi-3d__render_stl( + session_id="...", + source_path="bracket.stl", + views=["iso", "front", "top"] +) +``` + +## Параметры + +| Параметр | Обязательно | Описание | +|---|---|---| +| `session_id` | Да | UUID текущей сессии Navi. Файлы разрешаются внутри `session_files//`. | +| `source_path` | Да | Путь к существующему `.stl`-файлу (внутри сессии или абсолютный) | +| `views` | Нет | Список ракурсов (макс. 3). Доступные: `front`, `back`, `top`, `bottom`, `left`, `right`, `iso`. По умолчанию `["iso"]`. | + +## Доступные ракурсы + +| Ракурс | Описание | +|---|---| +| `front` | Вид спереди | +| `back` | Вид сзади | +| `top` | Вид сверху | +| `bottom` | Вид снизу | +| `left` | Вид слева | +| `right` | Вид справа | +| `iso` | Изометрия (по умолчанию) | + +## Workflow + +### 1. Сгенерировать STL + +``` +mcp__navi-3d__compile_scad( + session_id="sess-abc", + source_path="bracket.scad", + output_path="bracket.stl" +) +``` + +### 2. Отрендерить ракурсы + +``` +mcp__navi-3d__render_stl( + session_id="sess-abc", + source_path="bracket.stl", + views=["iso", "front", "top"] +) +``` + +### 3. Проверить рендеры + +Каждый PNG сохраняется рядом с STL с суффиксом вида: +- `bracket.iso.png` +- `bracket.front.png` +- `bracket.top.png` + +Обычно эти PNG нужны Нави для внутренней проверки геометрии перед публикацией STL. Откройте их через `image_view` и проверьте форму, пропорции и физическую корректность. + +Не публикуйте PNG пользователю, если пользователь явно не попросил preview-картинки. + +``` +image_view(source="session_files/sess-abc/bracket.iso.png") +image_view(source="session_files/sess-abc/bracket.front.png") +image_view(source="session_files/sess-abc/bracket.top.png") +``` + +## Что возвращает + +При успехе: +``` +Generated 3 image(s): + bracket.iso.png + bracket.front.png + bracket.top.png +``` + +При ошибке: +- `openscad_not_found` — OpenSCAD не установлен +- `stl_not_found` — исходный файл не найден +- `too_many_views` — больше 3 ракурсов за раз +- `invalid_views` — неизвестное имя ракурса +- `render_failed` — все рендеры не удались + +## Важные правила + +1. **Максимум 3 ракурса за вызов** — если нужно больше, разбейте на несколько вызовов. +2. **Фиксированное разрешение** — 400×300, не настраивается. +3. **PNG сохраняются рядом с STL** — в той же директории, с суффиксом ракурса. +4. **Всегда preview mode** — быстрый рендер, не полный CSG. +5. **Не склеивает в сетку** — каждый ракурс — отдельный файл. PNG обычно нужны для внутренней проверки; публикуйте их только по явной просьбе пользователя. +6. **Всегда передавайте `session_id`** — без него инструмент не знает, в какой директории искать файлы. diff --git a/manuals/ssh_exec.md b/manuals/ssh_exec.md new file mode 100644 index 0000000..d7a8a50 --- /dev/null +++ b/manuals/ssh_exec.md @@ -0,0 +1,81 @@ +# ssh_exec — Manual + +## What it does +Runs a shell command on a remote host over SSH (`action: "exec"`, the default), or moves a file with SCP (`action: "scp"`). Both credential styles work out of the box: inline `host`/`username`/`password`/`key_path` for a one-off, or a named `connection` from `ssh_hosts.json` for a host you visit repeatedly. + +Success is `exit_status == 0`. Any other status returns `success: false` with the full output preserved in the error — read it, it is the remote command's own stderr, not a Navi failure. + +## Parameters + +| Parameter | Required | Description | +|-----------|----------|-------------| +| `action` | no | `exec` (default) or `scp`. | +| `command` | for `exec` | Shell command to run on the remote host. | +| `host` | for a direct connection | Hostname or IP. | +| `username` | no | SSH user. Defaults to `$USER` on the Navi host, so pass it for anything but the obvious case. | +| `password` | no | Password auth. | +| `port` | no | Default 22. | +| `key_path` | no | Private key, e.g. `~/.ssh/id_rsa`. `~` is expanded. | +| `connection` | no | Named entry from `ssh_hosts.json`. | +| `local_path` / `remote_path` / `direction` | for `scp` | `upload` (local→remote) or `download` (remote→local). | +| `timeout` | no | Seconds, default 60. | +| `background` | no | Detach; returns a `task_id` immediately. See [Detaching](#detaching-long-commands). | + +## Connecting + +**Inline** — anything ad-hoc: + +```json +{"command": "uptime", "host": "192.168.1.168", "username": "ubuntu"} +{"command": "df -h /", "host": "10.0.0.5", "username": "root", "key_path": "~/.ssh/id_ed25519"} +{"command": "whoami", "host": "10.0.0.5", "username": "root", "password": "..."} +``` + +**Named connection** — the host is in `ssh_hosts.json`. Pass `connection` instead of the host triple: + +```json +{"command": "systemctl status navi", "connection": "prod"} +``` + +`connection` takes precedence, and inline `host`/`username`/`password`/`port`/`key_path` override the stored values one by one — useful for the same host under a different user. + +With neither a `connection` that exists nor a `host`, the call fails with *No SSH target specified* before any network attempt. + +## Credentials and host keys + +- **Key auth** — `key_path` (or `client_keys` in `ssh_hosts.json`) is expanded and used first; a `password` given alongside it becomes a fallback. +- **Password-only** — key lookup is disabled so asyncssh does not try your local keys first. This is the difference between a clean login and *too many authentication failures*. +- **No credentials at all** — asyncssh tries the default keys under `~/.ssh/`. +- **Host key verification** is decided by `known_hosts`: + - key absent from the config → verification is **skipped** (the default for ad-hoc calls); + - `"known_hosts": null` (as in the template) → the system `~/.ssh/known_hosts` is used; + - a path → that file is used. + + To verify a host you connect to often, put the path (or an explicit `null`) in its `ssh_hosts.json` entry. Skipping is fine for a fresh VPS and wrong for production. + +## Connections are pooled + +A connection is reused within a session (keyed by session + host + port + user, 20-minute TTL). Two sessions touching the same server keep separate pools and never interfere. If the server drops the connection (`DisconnectError`, `ConnectionLost`, EOF) the entry is evicted and the command is retried **once** on a fresh connection; a second failure is returned as-is. `PermissionDenied` is never retried — fix the credentials instead. + +## Transferring files + +```json +{"action": "scp", "direction": "upload", "connection": "prod", + "local_path": "/tmp/build.tar.gz", "remote_path": "/opt/app/build.tar.gz"} +{"action": "scp", "direction": "download", "connection": "prod", + "remote_path": "/var/log/navi.log", "local_path": "/tmp/navi.log"} +``` + +Both paths are required; the result says `Uploaded:` or `Downloaded:` with both ends. SCP honours the same `timeout` (default 60 s) — raise it for a large file. + +## Detaching long commands + +Pass `"background": true` for builds, package installs, backups and anything else that outlives the turn. You get a `task_id` back immediately and the result arrives as a note at the start of a later turn; inspect it with the `tasks` tool. When you detach **without** passing `timeout`, the executor raises the limit to 300 s for that run — do not leave a detached command at the 60 s default and assume it finished. + +## Common mistakes + +- Omitting `username` and landing as the Navi host's user, not the intended one. +- Putting the host in `command` (`ssh root@10.0.0.5 uptime`) — that depends on local SSH config and key forwarding. Use `host`/`username`, or `connection`. +- Reading a non-zero `exit_status` as a broken tool. The tool worked; the command failed. +- `scp` without `direction` — it defaults to `upload`, so a download silently needs it. +- Expecting host-key verification. It is off unless the connection says otherwise. diff --git a/manuals/todo.md b/manuals/todo.md new file mode 100644 index 0000000..eea4253 --- /dev/null +++ b/manuals/todo.md @@ -0,0 +1,64 @@ +# todo — Manual + +## What it does +Tracks the steps of the current task and their status. The list is **already populated** when the task starts — the planner fills the todo with the plan's steps — so you normally never call `set`; you call `update` as you go and `view` when you lose the thread. + +The tool is the record of *what you actually verified*. Marking a step `done` without a `validation` field is rejected outright, which is the point: it stops "done" from meaning "I moved on". + +Tasks are per-session and do not survive a server restart. + +## Parameters + +| Parameter | Required | Description | +|-----------|----------|-------------| +| `op` | yes | `set` \| `view` \| `update` \| `add` \| `clear`. | +| `tasks` | for `set`/`add` | Ordered list of step descriptions (strings). | +| `index` | for `update` | 1-based step number. | +| `status` | for `update` | `pending` \| `in_progress` \| `done` \| `failed` \| `skipped`. | +| `validation` | for `status: "done"` | How you verified the result. | + +An omitted `op` is **inferred** from the arguments you did send — `{"index": 1, "status": "in_progress"}` is read as an `update`, `{"tasks": [...]}` as a `set` — so a missing discriminator costs you nothing. An explicitly *wrong* `op` is rejected with the list of valid ones. A `status` outside the five is stored as sent rather than rejected, which means a typo (`"Done"`) silently stops the step from ever counting as done — spell them lower-case exactly as listed. + +## Actions + +### `update` — the one you use constantly +```json +{"op": "update", "index": 1, "status": "in_progress"} +{"op": "update", "index": 1, "status": "done", "validation": "ran uv run pytest tests/unit/tools -q — 38 passed"} +{"op": "update", "index": 2, "status": "failed", "validation": "ssh timeout after 60s; host unreachable, tried both key and password"} +``` + +- `index` is 1-based and must be within the plan — an out-of-range index returns the plan's length so you can correct it. +- **`done` requires `validation`.** Without it the call fails with `validation_required` and nothing changes. Say what you checked, not what you did: *"ran X — output matched Y"*, *"read the file and confirmed the block is gone"*. +- `failed` without `validation` is accepted, with a tip. Provide one anyway — it is what makes re-planning possible. +- Set `in_progress` when you **start** a step, and move it to `done`/`failed` **before starting the next one**. A status that lags reality turns the todo into decoration. + +### `view` +Re-orients you: current statuses plus the validation notes. Call it after a sub-agent returns, after a long tool chain, or whenever the plan in your head may have drifted from the plan in the store. "No plan set for this session." means exactly that — no plan exists yet. + +### `set` +Creates or **replaces** the Master Plan. Use it only when the plan must be rebuilt mid-task. It takes the whole list and discards every existing status, so it is the expensive, destructive option. + +### `add` +Appends steps discovered mid-task and **preserves existing steps and their statuses**. This is what you want for a newly surfaced subtask — never rebuild the list with `set` to add one line. Requires a plan to already exist. + +### `clear` +Resets the plan. Rarely right: a finished plan is the record of what was verified, and the final answer should be able to point at every step marked `done`. + +## The pattern + +1. Start a step → `{"op": "update", "index": N, "status": "in_progress"}`. +2. Do the work. +3. Verify it — actually run the check, read the output. +4. `{"op": "update", "index": N, "status": "done", "validation": "..."}`. +5. Repeat. Nothing is left `in_progress` when you answer. + +Before your final message, every completed step — including the last one — must be `done` with validation. If you discover work the plan does not cover, `add` it rather than silently doing it. + +## Common mistakes + +- `set` instead of `add`, wiping the statuses of steps already finished. +- Marking several steps `done` at the end in one sweep, with validations written from memory. Verify per step; the todo is only worth reading if it tracked reality while it happened. +- Leaving the last step `in_progress` in the final answer. +- Using a 0-based index. It is 1-based, like the rendered list. +- Treating `view` as optional after a sub-agent run — its steps are not in your head. diff --git a/manuals/write_context_provider.md b/manuals/write_context_provider.md deleted file mode 100644 index 3a2fe9a..0000000 --- a/manuals/write_context_provider.md +++ /dev/null @@ -1,68 +0,0 @@ -# Writing a Context Provider - -Context providers inject dynamic runtime data as system messages into every LLM call. Use them when information is needed frequently and fetching it on-demand every time would be wasteful (current URL, hostname, server status, etc.). - -## When to use a context provider vs a tool - -- **Context provider**: data that is useful on *every* call without the model asking — server address, current environment, active config values. -- **Tool**: data that is needed occasionally on demand — current weather, a file's contents, search results. - -## File format - -Create a file in `context_providers/` at the project root. File name must not start with `_`. - -```python -# context_providers/my_provider.py - -name = "my_provider" -description = "One-line description of what this injects." -global_provider = False # True = injected in ALL profiles automatically - -async def get_context() -> str | None: - """Return a string or None. None means skip silently.""" - return "[System] My info: ..." -``` - -### Required fields -| Field | Type | Description | -|---|---|---| -| `name` | `str` | Unique identifier | -| `description` | `str` | What data this provides | -| `get_context` | `async def` | Returns `str` to inject or `None` to skip | - -### Optional fields -| Field | Type | Default | Description | -|---|---|---|---| -| `global_provider` | `bool` | `False` | `True` = inject in every profile; `False` = only in profiles that list this name in `context_providers` | - -## Enabling per-profile - -For non-global providers, add the name to the profile's `config.json`: -```json -"context_providers": ["my_provider"] -``` - -## Activation - -After writing, call `reload_tools` — it reloads both tools and context providers. The new data will appear in the next LLM call. - -## Example: inject current server hostname - -```python -# context_providers/hostname.py -import socket - -name = "hostname" -description = "Injects the current server hostname." -global_provider = True - -async def get_context() -> str | None: - return f"[System] Server hostname: {socket.gethostname()}" -``` - -## Notes - -- Return value is injected as `role="system"` right after the memory summary, before conversation history. -- Exceptions in `get_context()` are caught and logged — they never crash the agent. -- `global_provider = True` providers are loaded for every profile; no config change needed. -- Built-in providers live in `navi/context_providers/` and cannot be overwritten by user providers. diff --git a/manuals/write_mcp_server.md b/manuals/write_mcp_server.md deleted file mode 100644 index a0e8ec0..0000000 --- a/manuals/write_mcp_server.md +++ /dev/null @@ -1,224 +0,0 @@ -# Writing MCP Servers for Navi - -This manual describes how to create, test, register, and maintain MCP servers that extend Navi's capabilities. Read this **before** you start building a new server. - -## 1. Philosophy: Why MCP instead of user tools? - -MCP servers run in isolated processes and communicate via the Model Context Protocol. They **cannot** crash Navi's core, they can be reloaded without restarting the server, and they scale to complex external integrations (APIs, databases, browsers, etc.). The trade-off is slightly more boilerplate than a single `tools/foo.py` file. - -**Rule:** Every new capability that is not trivial (more than a simple datetime or notes lookup) should be built as an MCP server. - -## 2. Directory structure - -MCP servers live under: - -``` -mcp-servers// -├── pyproject.toml -├── README.md -└── app/ - ├── __init__.py - └── mcp_server.py -``` - -- `` — snake_case or kebab-case. Must match the key you will use in `mcp_servers.d/.json`. -- `pyproject.toml` — Python package metadata and dependencies. -- `app/mcp_server.py` — the actual server code (FastMCP). - -## 3. Creating a new server from the template - -Use the built-in tool `create_mcp_server` (preferred) or copy the template manually: - -```bash -cp -r mcp-servers/_template mcp-servers/my_server -cd mcp-servers/my_server -# Edit pyproject.toml: change name, description, add dependencies -# Edit app/mcp_server.py: add your tools and instructions -``` - -### 3.1 pyproject.toml - -Minimal required fields: - -```toml -[build-system] -requires = ["setuptools>=61.0"] -build-backend = "setuptools.build_meta" - -[project] -name = "mcp-server-myserver" -version = "0.1.0" -description = "What this server does" -requires-python = ">=3.11" -dependencies = [ - "mcp>=1.27", - "pydantic>=2.0", - # add your own: httpx, asyncpg, playwright, etc. -] - -[project.scripts] -mcp-server-myserver = "app.mcp_server:main" - -[tool.setuptools.packages.find] -where = ["."] -include = ["app*"] -``` - -### 3.2 app/mcp_server.py - -Read the template at `mcp-servers/_template/app/mcp_server.py` first. It contains a working hello-world server with extensive inline comments. - -Key sections you must edit: - -1. **`INSTRUCTIONS`** — These are injected into Navi's system prompt. Describe: - - What this server does and when to use it. - - Recommended workflow (order of tool calls). - - ABSOLUTE RULE about never bypassing these tools with filesystem/terminal. - -2. **`mcp = FastMCP("name", instructions=INSTRUCTIONS)`** — The name should match the directory key. - -3. **Tool functions** — Each tool: - - Is an `async def`. - - Uses `@mcp.tool(name="tool_name")`. - - Parameters use `Annotated[T, Field(description="...")]` — never plain types. - - Returns a plain `str` (JSON string for structured data is fine). - - Raises on real errors. - - Validates required params explicitly. - -Example: - -```python -@mcp.tool(name="search_docs") -async def search_docs_tool( - query: Annotated[str, Field(description="Search query string.")], - limit: Annotated[int, Field(description="Max results.")] = 10, -) -> str: - """Search the documentation index.""" - if not query.strip(): - raise ValueError("query is required and cannot be empty.") - results = await _do_search(query, limit) - return json.dumps(results, ensure_ascii=False, indent=2) -``` - -## 4. Environment and installation - -After creating files, you **must**: - -1. Create a virtual environment: - ```bash - python -m venv .venv - source .venv/bin/activate - pip install -e . - ``` - -2. Verify the server starts without crashing: - ```bash - timeout 5 python -m app.mcp_server || true - ``` - If it prints a traceback, fix the code before proceeding. - -## 5. Registering the server in Navi - -Create a file `mcp_servers.d/.json` in the project root. The filename (without `.json`) becomes the server name. Example for a server named `my_server`: - -```json -{ - "transport": "stdio", - "command": "/absolute/path/to/mcp-servers/my_server/.venv/bin/python", - "args": ["-m", "app.mcp_server"], - "cwd": "/absolute/path/to/mcp-servers/my_server", - "env": { - "MCP_TRANSPORT": "stdio" - }, - "groups": { - "default": ["search_docs", "read_doc"] - }, - "instructions": "Optional extra instructions merged with the server's own INSTRUCTIONS." -} -``` - -**Critical fields:** -- `command` — absolute path to the venv's Python binary. -- `cwd` — absolute path to the server directory. -- The filename must be `.json` (e.g. `my_server.json`). -- `args` — usually `["-m", "app.mcp_server"]`. -- `groups` — organize tools into named groups so profiles can reference them cleanly. - -After editing `mcp_servers.d/.json`, call `reload_tools` to connect the server and register its tools. - -## 6. Testing - -### 6.1 Check connection - -Call `mcp_status`. You should see your server as `connected` with the correct tool count. - -### 6.2 Test each tool - -Call `test_mcp_tool` for every tool your server exposes: - -``` -test_mcp_tool(server_name="my_server", tool_name="search_docs", arguments={"query": "hello", "limit": 3}) -``` - -If any tool fails, read the error output, fix the code in `app/mcp_server.py`, and repeat. - -### 6.3 Manual stderr inspection - -If `mcp_status` shows `disconnected` but the code looks correct, inspect stderr manually: - -```bash -cd mcp-servers/my_server -.venv/bin/python -m app.mcp_server 2>&1 | head -n 20 -``` - -## 7. Updating an MCP server - -1. Edit the code in `mcp-servers//app/mcp_server.py`. -2. (Optional) If you added new dependencies, edit `pyproject.toml` and run `pip install -e .` inside the venv. -3. Call `reload_tools` to reconnect the server and re-register tools. -4. Call `test_mcp_tool` to verify. - -## 8. Deleting an MCP server - -1. Remove the server directory or move it to a backup location. -2. Remove the entry from `mcp_servers.d/.json`. -3. Call `reload_tools`. - -## 9. Connecting an external MCP server - -If the server was written by someone else: - -1. Clone or place the server code on disk. -2. Create its venv and install dependencies. -3. Read its README to learn tool names and required environment variables. -4. Add an entry to `mcp_servers.d/.json` with the correct `command`, `cwd`, `args`, and `env`. -5. Define `groups` mapping the tools into logical sets. -6. Call `reload_tools`. -7. Call `test_mcp_tool` for a representative tool. - -## 10. Common mistakes and debugging - -| Symptom | Cause | Fix | -|---|---|---| -| `mcp_status` shows `disconnected` | Wrong `command` or `cwd` path | Double-check absolute paths | -| Traceback on startup | Syntax error or missing import | Run `python -m py_compile app/mcp_server.py` | -| `test_mcp_tool` returns `is_error=True` | Tool raised an exception | Fix the tool logic; check parameter validation | -| Tool schema missing descriptions | Used plain types instead of `Annotated[..., Field(...)]` | Add `Field(description=...)` to every parameter | -| Navi never calls the server | Profile does not map the server in `mcp_servers` | Edit the profile's `config.json` and add the server groups | -| Navi bypasses MCP with filesystem | `INSTRUCTIONS` missing ABSOLUTE RULE | Add explicit rule in server INSTRUCTIONS | - -## 11. Workflow checklist for Navi - -When asked to create a new MCP server: - -1. Read this manual (`manuals/write_mcp_server.md`). -2. Read the template (`mcp-servers/_template/app/mcp_server.py`). -3. Call `create_mcp_server(name=..., description=...)` to scaffold the directory. -4. Edit `app/mcp_server.py` iteratively using `filesystem`. -5. Validate syntax: `code_exec` or `terminal` with `python -m py_compile ...`. -6. Test startup: `terminal` with `timeout 5 python -m app.mcp_server`. -7. Edit `mcp_servers.d/.json` via `filesystem` to register the server. -8. Call `reload_tools`. -9. Call `test_mcp_tool` for every tool. -10. Write good `INSTRUCTIONS` inside `mcp_server.py`. -11. Report results to the user. diff --git a/manuals/write_tool.md b/manuals/write_tool.md deleted file mode 100644 index 017fad2..0000000 --- a/manuals/write_tool.md +++ /dev/null @@ -1,115 +0,0 @@ -# write_tool — Manual - -## What it does -Writes Python source code to `tools/.py` and immediately reloads it into the system. -The new tool becomes a permanent part of your capabilities — available in every future session. - -## Parameters -- `name` (string, required) — tool filename without `.py`, e.g. `"task_manager"` -- `code` (string, required) — full Python source code (see format below) - -## Required code format -Every tool file must define exactly these four things at module level, in this order: - -```python -name = "tool_name" # must match the filename -description = "When and why to use this tool. Be specific — this is what you read to decide whether to call it." -parameters = { - "type": "object", - "properties": { - "param1": {"type": "string", "description": "What this parameter is for"}, - "param2": {"type": "integer", "description": "..."}, - }, - "required": ["param1"], -} - -async def execute(params: dict) -> str: - value = params["param1"] - # your implementation here - return "result as a plain string" -``` - -Rules: -- NO classes -- NO print() at module level -- `execute` MUST be `async` -- `execute` MUST return a plain `str` — not dict, not None -- Raise an exception to signal failure (do not return error dicts) -- Put imports inside `execute()` or at the top of the file — both are fine - -## Data persistence -If the tool needs to store data between calls, use a JSON file inside the `tools/` directory: - -```python -import json, os - -DATA_FILE = os.path.join(os.path.dirname(__file__), "my_tool_data.json") - -def _load(): - if os.path.exists(DATA_FILE): - with open(DATA_FILE) as f: - return json.load(f) - return {} - -def _save(data): - with open(DATA_FILE, "w") as f: - json.dump(data, f, ensure_ascii=False, indent=2) -``` - -## Full example — a simple note-taking tool - -```python -import json, os - -name = "notes" -description = ( - "Save and retrieve short text notes by key. " - "Use this to remember things across sessions: facts, preferences, reminders. " - "Actions: save (store a note), get (retrieve by key), list (show all keys)." -) -parameters = { - "type": "object", - "properties": { - "action": {"type": "string", "enum": ["save", "get", "list"]}, - "key": {"type": "string", "description": "Note identifier"}, - "value": {"type": "string", "description": "Note content (for save)"}, - }, - "required": ["action"], -} - -_FILE = os.path.join(os.path.dirname(__file__), "notes_data.json") - -async def execute(params: dict) -> str: - action = params["action"] - data = json.loads(open(_FILE).read()) if os.path.exists(_FILE) else {} - - if action == "save": - key, value = params["key"], params["value"] - data[key] = value - open(_FILE, "w").write(json.dumps(data, ensure_ascii=False, indent=2)) - return f"Saved: {key}" - - if action == "get": - key = params["key"] - if key not in data: - raise KeyError(f"Note '{key}' not found. Use action=list to see all keys.") - return data[key] - - if action == "list": - if not data: - return "No notes saved yet." - return "Saved notes: " + ", ".join(data.keys()) - - raise ValueError(f"Unknown action: {action}") -``` - -## What write_tool checks before writing -- `name`, `description`, `parameters`, `async def execute` must all be present in the code -- Name must not start with `_` - -If the check fails, you get a clear error with the list of what's missing. -If the file loads but has a Python error, you get the exact traceback. - -## After a successful call -The tool is registered and added to `tools/enabled.json`. -It will be available starting from the **next user message** — not the current one. diff --git a/navi/core/registry.py b/navi/core/registry.py index 201d228..ca72c94 100644 --- a/navi/core/registry.py +++ b/navi/core/registry.py @@ -230,7 +230,12 @@ reload_tool = ReloadToolsTool(registry=tools, cp_registry=cp_registry, mcp_manager=mcp_manager) list_tool = ListToolsTool(registry=tools, profile_registry=profiles, mcp_manager=mcp_manager) - manual_tool = ToolManualTool(registry=tools) + # profile_registry + mcp_manager let it resolve a name against the profile's own + # toolset the same way the executor will — and say "registered but not enabled + # for this profile" instead of a bare "no manual found". + manual_tool = ToolManualTool( + registry=tools, profile_registry=profiles, mcp_manager=mcp_manager + ) memory_tool = MemoryTool(memory_store) if memory_store else None mcp_status_tool = McpStatusTool() create_mcp_server_tool = CreateMcpServerTool() diff --git a/navi/core/tool_executor.py b/navi/core/tool_executor.py index 45a077b..1dc0999 100644 --- a/navi/core/tool_executor.py +++ b/navi/core/tool_executor.py @@ -9,6 +9,8 @@ from navi.llm.base import Message, ToolCallRequest from navi.tools._internal.base import Tool, ToolResult, current_session_id +from .tool_utils import resolve_tool as _resolve_tool + if TYPE_CHECKING: from navi.core.events import ToolEvent @@ -25,63 +27,6 @@ return {t.strip() for t in settings.backgroundable_tools.split(",") if t.strip()} -def _resolve_tool(tool_map: dict[str, Tool], name: str) -> tuple[str, Tool | None]: - """Resolve exact tool names plus common MCP alias mistakes.""" - from navi.mcp.tools import is_mcp_tool, parse_mcp_name - - tool = tool_map.get(name) - if tool is not None: - return name, tool - - # Support bare tool name when the full MCP name ends with it - # e.g. "web_search" -> "mcp__navi-web__web_search" - bare_matches = [ - (candidate_name, candidate) - for candidate_name, candidate in tool_map.items() - if is_mcp_tool(candidate_name) and candidate_name.endswith(f"__{name}") - ] - if len(bare_matches) == 1: - return bare_matches[0] - - # Normalized variant (dash vs underscore) - normalized = name.replace("-", "_") - normalized_matches = [ - (candidate_name, candidate) - for candidate_name, candidate in tool_map.items() - if is_mcp_tool(candidate_name) and candidate_name.replace("-", "_") == normalized - ] - if len(normalized_matches) == 1: - return normalized_matches[0] - - # Fallback: old underscore format like mcp_server_tool -> mcp__server__tool - old_format_matches = [ - (candidate_name, candidate) - for candidate_name, candidate in tool_map.items() - if is_mcp_tool(candidate_name) and name.startswith("mcp_") - ] - for candidate_name, candidate in old_format_matches: - parsed = parse_mcp_name(candidate_name) - if parsed is None: - continue - server_name, tool_name = parsed - # mcp_navi_web_search -> navi_web_search -> split into navi, web, search - # We try matching by removing the mcp_ prefix and comparing - expected_old = f"mcp_{server_name}_{tool_name}" - if expected_old == name: - return candidate_name, candidate - - # Extra fallback: legacy colon format mcp:server:tool (from old sessions) - legacy_matches = [ - (candidate_name, candidate) - for candidate_name, candidate in tool_map.items() - if is_mcp_tool(candidate_name) and name.replace(":", "__") == candidate_name - ] - if len(legacy_matches) == 1: - return legacy_matches[0] - - return name, None - - class ToolExecutor: """Runs tool calls and builds ToolEvent / Message results.""" diff --git a/navi/core/tool_utils.py b/navi/core/tool_utils.py index 4041b40..dba6d38 100644 --- a/navi/core/tool_utils.py +++ b/navi/core/tool_utils.py @@ -1,4 +1,10 @@ -"""Tool list construction shared between Agent and SubAgentRunner.""" +"""Tool list construction and name resolution shared across the core. + +`build_tool_list` is what the Agent and the SubAgentRunner use to turn a +profile's config into concrete tools; `resolve_tool` is how a model-written +tool name finds one of them. Both live here so every consumer — the executor +and tool_manual — answers "which tool did the model mean" the same way. +""" import logging from pathlib import Path @@ -19,6 +25,71 @@ return [] +def resolve_tool(tool_map: dict[str, Tool], name: str) -> tuple[str, Tool | None]: + """Resolve a requested name against a tool map, tolerating the ways models + mangle MCP names. + + Returns the key to report ('requested name' when nothing matched) and the + tool. The variants below are all observed in stored sessions: a bare tool + name, a dash/underscore swap, the old `mcp__` spelling, and + the legacy `mcp:server:tool` one. Every consumer must share this, or the + same call resolves in the executor and 404s in the tool that documents it. + """ + from navi.mcp.tools import is_mcp_tool, parse_mcp_name + + tool = tool_map.get(name) + if tool is not None: + return name, tool + + # Support bare tool name when the full MCP name ends with it + # e.g. "web_search" -> "mcp__navi-web__web_search" + bare_matches = [ + (candidate_name, candidate) + for candidate_name, candidate in tool_map.items() + if is_mcp_tool(candidate_name) and candidate_name.endswith(f"__{name}") + ] + if len(bare_matches) == 1: + return bare_matches[0] + + # Normalized variant (dash vs underscore) + normalized = name.replace("-", "_") + normalized_matches = [ + (candidate_name, candidate) + for candidate_name, candidate in tool_map.items() + if is_mcp_tool(candidate_name) and candidate_name.replace("-", "_") == normalized + ] + if len(normalized_matches) == 1: + return normalized_matches[0] + + # Fallback: old underscore format like mcp_server_tool -> mcp__server__tool + old_format_matches = [ + (candidate_name, candidate) + for candidate_name, candidate in tool_map.items() + if is_mcp_tool(candidate_name) and name.startswith("mcp_") + ] + for candidate_name, candidate in old_format_matches: + parsed = parse_mcp_name(candidate_name) + if parsed is None: + continue + server_name, tool_name = parsed + # mcp_navi_web_search -> navi_web_search -> split into navi, web, search + # We try matching by removing the mcp_ prefix and comparing + expected_old = f"mcp_{server_name}_{tool_name}" + if expected_old == name: + return candidate_name, candidate + + # Extra fallback: legacy colon format mcp:server:tool (from old sessions) + legacy_matches = [ + (candidate_name, candidate) + for candidate_name, candidate in tool_map.items() + if is_mcp_tool(candidate_name) and name.replace(":", "__") == candidate_name + ] + if len(legacy_matches) == 1: + return legacy_matches[0] + + return name, None + + def build_tool_list( enabled: list[str], mcp_servers: dict[str, list[str]] | None, diff --git a/navi/profiles/tool_developer/system_prompt.txt b/navi/profiles/tool_developer/system_prompt.txt index a119623..9dd5df4 100644 --- a/navi/profiles/tool_developer/system_prompt.txt +++ b/navi/profiles/tool_developer/system_prompt.txt @@ -11,7 +11,7 @@ ## Prerequisites — read BEFORE building Every time you are asked to create a new MCP server: -1. Call `tool_manual("write_mcp_server")` to read the full manual. +1. Call `tool_manual("create_mcp_server")` to read the full manual. 2. Read `mcp-servers/_template/app/mcp_server.py` to see the annotated template. 3. Then proceed to implementation. @@ -128,7 +128,7 @@ **Sub-agent briefing:** - Give the exact server name, directory path, and file to edit. - Specify every tool name, description, parameter schema, and expected return format. -- Include: "Read `manuals/write_mcp_server.md` and `mcp-servers/_template/app/mcp_server.py` first." +- Include: "Read `manuals/create_mcp_server.md` and `mcp-servers/_template/app/mcp_server.py` first." - Write the context the sub-agent needs (server name, paths, tool specs, how to verify) into the `context_transfer` scratchpad section before spawning — it's injected into the sub-agent automatically. The sub-agent does NOT inherit your short-term memory. - The sub-agent's toolset is restricted: no `memory`, `switch_profile`, `spawn_agent`, `schedule_recall`/`manage_recall`, `reload_tools`, `test_mcp_tool`, or `mcp_status` — it cannot register or test the server itself; run those inline. Record findings in `scratchpad`, not `memory`. - End with: "Complete all assigned work. Return: summary of changes, test output." diff --git a/navi/tools/list_tools.py b/navi/tools/list_tools.py index 202e1a2..c0b66c7 100644 --- a/navi/tools/list_tools.py +++ b/navi/tools/list_tools.py @@ -15,6 +15,15 @@ ) +def _manual_names() -> set[str]: + """Tools with a hand-written manual. Imported lazily — tool_manual owns the + manuals directory, and neither module should pull the other in at import time. + """ + from navi.tools.tool_manual import manual_names + + return manual_names() + + class ListToolsTool(Tool): name = "list_tools" description = ( @@ -156,6 +165,16 @@ lines.append(f"Not registered ({len(missing)}): {', '.join(missing)}") lines.append(_MISSING_HINT) + # One line, not one per tool: which of this profile's tools have a real + # hand-written manual is the difference between "read tool_manual" being a + # useful suggestion and a name the agent has no reason to pick over another. + documented = sorted( + name for names in available.values() for name in names if name in _manual_names() + ) + if documented: + lines.append("") + lines.append(f"Hand-written manuals available: {', '.join(documented)}") + if not query and not verbose: lines.append("") lines.append( diff --git a/navi/tools/tool_manual.py b/navi/tools/tool_manual.py index 07ecfb1..efae9d3 100644 --- a/navi/tools/tool_manual.py +++ b/navi/tools/tool_manual.py @@ -1,77 +1,397 @@ -"""Built-in tool that returns the detailed manual for a given tool.""" +"""Built-in tool that returns the detailed manual for a given tool. +Two ways of answering: a hand-written ``manuals/.md`` when one exists, and +a manual generated from the tool's live schema otherwise. Resolution goes through +the same ``resolve_tool`` the executor uses, so a name that works when the agent +*calls* the tool also works when it *asks about* it — the two used to disagree and +a manual the file system held was unreachable under its MCP spelling. +""" + +from __future__ import annotations + +import difflib from pathlib import Path -from ._internal.base import Tool, ToolContext, ToolResult +from ._internal.base import Tool, ToolContext, ToolResult, current_profile_id MANUALS_DIR = Path(__file__).parent.parent.parent / "manuals" +GUIDES_DIR = MANUALS_DIR / "guides" + +# Same spelling list_tools buckets by, so the two agree on where a tool comes from. +NATIVE_SOURCE = "native" +_MCP_PREFIX = "mcp__" +_MCP_SEP = "__" + +_MAX_SUGGESTIONS = 5 +_MAX_DEPTH = 5 class ToolManualTool(Tool): name = "tool_manual" description = ( - "Returns the detailed manual for a tool: full usage instructions, parameter reference, and examples. " - "Call this before using an unfamiliar tool, or when you are unsure about the correct format or parameters." + "Returns the detailed manual for a tool: full usage instructions, parameter " + "reference, and examples. Call this before using an unfamiliar tool, or when you " + "are unsure about the correct format or parameters. Name the tool bare " + "('compile_scad') or by its full MCP name ('mcp__navi-3d__compile_scad')." ) parameters = { "type": "object", "properties": { "tool_name": { "type": "string", - "description": "Name of the tool to look up, e.g. 'filesystem'", + "description": ( + "Name of the tool to look up, e.g. 'filesystem', 'todo', " + "'mcp__navi-3d__compile_scad'." + ), } }, "required": ["tool_name"], } - def __init__(self, registry=None) -> None: + def __init__(self, registry=None, profile_registry=None, mcp_manager=None) -> None: self._registry = registry + self._profile_registry = profile_registry + self._mcp_manager = mcp_manager async def execute(self, params: dict, ctx: ToolContext | None = None) -> ToolResult: - tool_name = params["tool_name"].strip() + # Deferred: navi.core builds the registry, which imports this module. + from navi.core.tool_utils import resolve_tool - manual_file = MANUALS_DIR / f"{tool_name}.md" - if manual_file.exists(): - return ToolResult(success=True, output=manual_file.read_text(encoding="utf-8")) + requested = params.get("tool_name") + # params["tool_name"] raised KeyError on a missing key, and .strip() raised + # AttributeError on null or a number. Neither is something the agent can act + # on — anything that is not a usable string is "no name given". + tool_name = requested.strip() if isinstance(requested, str) else "" + if not tool_name: + return self._index() - # No .md file — generate a manual from the tool's schema - if self._registry: - try: - tool = self._registry.get(tool_name) - return ToolResult(success=True, output=_auto_manual(tool)) - except Exception: - pass + # A hand-written manual wins, under any spelling the model may use for it. + manual = _read_manual(tool_name) + if manual is not None: + return ToolResult(success=True, output=manual) - available = sorted(f.stem for f in MANUALS_DIR.glob("*.md")) if MANUALS_DIR.exists() else [] - hint = f"\nAvailable manuals: {', '.join(available)}" if available else "" - return ToolResult(success=False, output=f"No manual found for '{tool_name}'.{hint}", error="not_found") + profile_id = self._active_profile_id(ctx) + scoped = self._profile_tool_map(profile_id) + _, tool = resolve_tool(scoped, tool_name) + if tool is not None: + return ToolResult(success=True, output=_auto_manual(tool)) + + # Registered, but not enabled for this profile. Documenting it is still the + # useful answer, and saying so stops the agent from calling it and getting + # "tool not found" back from the executor. A hint, never a refusal. + _, elsewhere = resolve_tool(self._registry_map(), tool_name) + if elsewhere is not None: + where = f"profile '{profile_id}'" if profile_id else "this run" + note = ( + f"'{elsewhere.name}' is registered but not enabled for {where}. Calling it " + "would fail with 'tool not found' — pick an enabled tool, or ask the user " + "to enable it." + ) + return ToolResult(success=True, output=f"[{note}]\n\n{_auto_manual(elsewhere)}") + + return self._not_found(tool_name, scoped) + + # ── helpers ────────────────────────────────────────────────────────── + + @staticmethod + def _active_profile_id(ctx: ToolContext | None) -> str: + return ((ctx.profile_id if ctx else None) or current_profile_id.get() or "").strip() + + def _registry_map(self) -> dict[str, Tool]: + """Every registered tool, as {name: Tool} for resolve_tool().""" + if self._registry is None: + return {} + return {tool.name: tool for tool in self._registry.all()} + + def _profile_tool_map(self, profile_id: str) -> dict[str, Tool]: + """The tools this profile can call, as {name: Tool} for resolve_tool(). + + Union of the agent and subagent scopes — a sub-agent asking about a tool that + only the parent's agent scope enables must not be told it is "not enabled": it + cannot tell which scope it runs in, and half the answers would be wrong. + + Built with build_tool_list, the same builder the agent uses for its real + toolset, so this cannot drift from what the executor will resolve. + """ + if not profile_id or self._registry is None or self._profile_registry is None: + return {} + + # Deferred: see execute(). + from navi.core.tool_utils import build_tool_list + + try: + profile = self._profile_registry.get(profile_id) + except Exception: + return {} + + tools: list[Tool] = [] + for scope_config in (profile.get_agent_tools(), profile.get_subagent_tools()): + tools.extend( + build_tool_list( + scope_config.native, scope_config.mcp, self._registry, self._mcp_manager + ) + ) + return {tool.name: tool for tool in tools} + + def _index(self) -> ToolResult: + """No name given — the catalogue of hand-written manuals, by tool source. + + An agent that does not know which tools have real docs can otherwise only + guess names and collect 'not found' answers. This is the answer to "what is + documented?", so it succeeds: nothing was asked for, nothing is wrong. + """ + by_source: dict[str, list[str]] = {} + registry_map = self._registry_map() + for path in _manual_index().values(): + if path.parent == GUIDES_DIR: + continue # a guide is not a tool + stem = path.stem + by_source.setdefault(_source_for_manual(stem, registry_map), []).append(stem) + + if not by_source: + return ToolResult( + success=False, + output="No hand-written manuals are installed — pass tool_name to get " + "a manual generated from that tool's schema.", + error="no_manuals", + ) + + lines = ["Hand-written manuals, by tool source — call tool_manual(tool_name=…) for one:"] + for source in sorted(by_source, key=lambda s: (s != NATIVE_SOURCE, s)): + names = sorted(by_source[source]) + lines.append(f"{source} ({len(names)}): {', '.join(names)}") + guides = sorted(p.stem for p in _guide_paths()) + if guides: + lines.append(f"guides (not tools; readable by name): {', '.join(guides)}") + lines.append( + "Any other tool still works: tool_manual returns a manual generated from " + "its schema." + ) + return ToolResult(success=True, output="\n".join(lines)) + + def _not_found(self, tool_name: str, scoped: dict[str, Tool]) -> ToolResult: + """Nothing matched — suggest instead of leaving the agent to guess.""" + known = { + name.lower(): name + for name in set(scoped) | set(self._registry_map()) | set(_manual_index()) + } + # A bare MCP name ("web_search") is a legitimate spelling of + # "mcp__navi-web__web_search" — resolve_tool accepts it, so a typo of one has + # to be suggestible too, not filed under "nothing registered". + for full in list(known.values()): + segment = full.rsplit("__", 1)[-1] + if segment != full: + known.setdefault(segment.lower(), full) + suggestions = difflib.get_close_matches( + tool_name.lower(), list(known), n=_MAX_SUGGESTIONS, cutoff=0.6 + ) + lines = [f"No tool named '{tool_name}' — nothing is registered under that name."] + if suggestions: + lines.append(f"Did you mean: {', '.join(known[s] for s in suggestions)}?") + available = sorted(manual_names()) + if available: + lines.append(f"Tools with a hand-written manual: {', '.join(available)}.") + lines.append("list_tools lists every tool this profile can call.") + return ToolResult(success=False, output="\n".join(lines), error="not_found") + + +def _manual_index() -> dict[str, Path]: + """Lower-cased name -> manual file, over manuals/ and manuals/guides/. + + Manuals are named after the tool they document. Guides are not tools, but they + are indexed too so a guide can still be read by name. + """ + index: dict[str, Path] = {} + for base in (MANUALS_DIR, GUIDES_DIR): + if not base.is_dir(): + continue + for path in sorted(base.glob("*.md")): + index.setdefault(path.stem.lower(), path) + return index + + +def _manual_candidates(name: str) -> list[str]: + """Spellings `name` may be filed under, best first, de-duplicated.""" + out = [name] + # The agent calls MCP tools by their full name (mcp__navi-3d__compile_scad) while + # the file is named after the tool alone (compile_scad.md). + for separator in ("__", ":"): + if separator in name: + out.append(name.rsplit(separator, 1)[-1]) + # Dash vs underscore is the other way the same tool gets spelled. + for candidate in list(out): + out += [candidate.replace("-", "_"), candidate.replace("_", "-")] + + seen: set[str] = set() + unique: list[str] = [] + for candidate in out: + key = candidate.lower() + if candidate and key not in seen: + seen.add(key) + unique.append(candidate) + return unique + + +def _read_manual(name: str) -> str | None: + """The hand-written manual for `name`, or None when no file matches.""" + index = _manual_index() + for candidate in _manual_candidates(name): + path = index.get(candidate.lower()) + if path is None: + continue + try: + return path.read_text(encoding="utf-8") + except OSError: + return None + return None + + +def _guide_paths() -> list[Path]: + if not GUIDES_DIR.is_dir(): + return [] + return sorted(GUIDES_DIR.glob("*.md")) + + +def manual_names() -> set[str]: + """Tool names with a hand-written manual (guides excluded — they are not tools).""" + if not MANUALS_DIR.is_dir(): + return set() + return {path.stem for path in MANUALS_DIR.glob("*.md")} + + +def _source_of(name: str) -> str: + """Where a tool comes from, spelled as the agent sees it: 'native' or 'mcp____'.""" + if not name.startswith(_MCP_PREFIX): + return NATIVE_SOURCE + server = name[len(_MCP_PREFIX) :].split(_MCP_SEP, 1)[0] + return f"{_MCP_PREFIX}{server}{_MCP_SEP}" + + +def _source_for_manual(stem: str, registry_map: dict) -> str: + """A manual is filed under the tool's own name, so ask the registry where that + tool actually lives — 'compile_scad.md' documents an MCP tool, not a built-in. + """ + from navi.core.tool_utils import resolve_tool + + _, tool = resolve_tool(registry_map, stem) + return _source_of(tool.name if tool is not None else stem) def _auto_manual(tool) -> str: - """Generate a readable manual from a tool's schema.""" - lines = [f"# {tool.name}", "", tool.description, "", "## Parameters"] + """A manual generated from the tool's live JSON schema. - props = tool.parameters.get("properties", {}) - required = set(tool.parameters.get("required", [])) + The header says which source it is and that this is a schema, not a curated + guide — otherwise the agent cannot tell a thin parameter contract from a real + manual and may read the absence of examples as the tool being simple. + """ + schema = tool.parameters or {} + lines = [ + f"# {tool.name}", + "", + tool.description or "(this tool has no description)", + "", + f"> Generated from the tool's JSON schema (source: {_source_of(tool.name)}). " + "This is the parameter contract, not a hand-written manual.", + "", + "## Parameters", + ] + props = schema.get("properties") or {} if not props: + lines.append("") lines.append("This tool takes no parameters.") - else: - for param_name, spec in props.items(): - req = " (required)" if param_name in required else " (optional)" - ptype = spec.get("type", "any") - desc = spec.get("description", "") - enum = spec.get("enum") - line = f"- `{param_name}` ({ptype}{req}): {desc}" - if enum: - line += f" — one of: {', '.join(repr(e) for e in enum)}" - lines.append(line) + return "\n".join(lines) - # Nested object properties - nested = spec.get("properties", {}) - for nname, nspec in nested.items(): - nreq = " (required)" if nname in spec.get("required", []) else " (optional)" - ndesc = nspec.get("description", "") - lines.append(f" - `{nname}` ({nspec.get('type', 'any')}{nreq}): {ndesc}") - + lines.append("") + _render_properties(props, schema.get("required") or [], lines, "", 0) return "\n".join(lines) + + +def _render_properties(props: dict, required, lines: list[str], indent: str, depth: int) -> None: + """One bullet per parameter, with its nested structure indented underneath.""" + required = set(required) + for param_name, spec in props.items(): + if not isinstance(spec, dict): + spec = {} + lines.append(f"{indent}- `{param_name}`{_describe(spec, param_name in required)}") + _render_nested(spec, lines, indent + " ", depth) + + +def _render_nested(spec: dict, lines: list[str], indent: str, depth: int) -> None: + """Whatever hangs under a parameter: object fields, array items, union branches. + + A model that cannot see the shape of an array of objects guesses it and gets the + call wrong, so object fields and array items are always spelled out. + """ + if depth >= _MAX_DEPTH: + lines.append(f"{indent}… (nested deeper; see the tool's schema)") + return + + nested = spec.get("properties") + if nested: + _render_properties(nested, spec.get("required") or [], lines, indent, depth + 1) + return + + items = spec.get("items") + if isinstance(items, dict) and (items.get("properties") or items.get("oneOf") or items.get("anyOf")): + lines.append(f"{indent}each item:") + _render_nested(items, lines, indent + " ", depth + 1) + + for union in ("oneOf", "anyOf"): + branches = [b for b in (spec.get(union) or []) if isinstance(b, dict)] + if not branches: + continue + lines.append(f"{indent}{union} — one of:") + for branch in branches: + lines.append(f"{indent} • {_branch_summary(branch)}") + _render_nested(branch, lines, indent + " ", depth + 1) + + +def _describe(spec: dict, required: bool) -> str: + """`(string, required): description — one of: …; default: …`""" + label = ", ".join([_type_of(spec), "required" if required else "optional"]) + + notes = [] + enum = spec.get("enum") + if enum: + notes.append("one of: " + ", ".join(repr(v) for v in enum)) + if "const" in spec: + notes.append(f"const: {spec['const']!r}") + if "default" in spec: + notes.append(f"default: {spec['default']!r}") + note = (" — " + "; ".join(notes)) if notes else "" + + return f" ({label}): {spec.get('description') or ''}{note}" + + +def _type_of(spec: dict) -> str: + """The type label, inferred when the schema expresses it as a union or array.""" + declared = spec.get("type") + if isinstance(declared, list): + return " | ".join(str(t) for t in declared) + if declared == "array": + items = spec.get("items") + return f"array of {_type_of(items)}" if isinstance(items, dict) else "array" + if declared: + return str(declared) + + alternatives = [b for b in (spec.get("oneOf") or []) + (spec.get("anyOf") or []) if isinstance(b, dict)] + if alternatives: + return " | ".join(_type_of(branch) for branch in alternatives) + + if spec.get("properties"): + return "object" + return "any" + + +def _branch_summary(branch: dict) -> str: + """One line for a oneOf/anyOf alternative: its type plus whatever pins it down.""" + label = _type_of(branch) + if "const" in branch: + return f"{label} = {branch['const']!r}" + enum = branch.get("enum") + if enum: + return f"{label}: {', '.join(repr(v) for v in enum)}" + if branch.get("description"): + return f"{label} — {branch['description']}" + return label diff --git a/persona_navi_code.txt b/persona_navi_code.txt index 54a8e8b..0f5257f 100644 --- a/persona_navi_code.txt +++ b/persona_navi_code.txt @@ -11,7 +11,7 @@ - Для shell-команд используй `terminal`. - Для работы с файлами — `filesystem`. - Для быстрых проверок кода — `code_exec`. -- Перед созданием нового инструмента всегда читай `tool_manual("write_tool")`. +- Перед созданием нового инструмента всегда читай `tool_manual("reload_tools")`. Параллельность и фоновые задачи: - Всё, что дольше ~45-60 секунд (длинные команды в `terminal`, `ssh_exec`, `peer`, `spawn_agent`, `code_exec`), запускай с `"background": true` — получишь `task_id` и сможешь продолжать работу. diff --git a/tests/unit/tools/test_list_tools.py b/tests/unit/tools/test_list_tools.py index ad72f2b..567800d 100644 --- a/tests/unit/tools/test_list_tools.py +++ b/tests/unit/tools/test_list_tools.py @@ -286,3 +286,32 @@ assert result.success is False assert result.error == "unknown_scope" + + +@pytest.mark.asyncio +async def test_names_the_profiles_tools_that_have_a_hand_written_manual(monkeypatch): + """"Use tool_manual" is only actionable if the agent knows which tools have real + docs. One line, and only for tools this profile can actually call — a manual for + some other profile's tool is noise.""" + monkeypatch.setattr("navi.tools.tool_manual.manual_names", lambda: {"todo", "weather"}) + profiles = make_profiles(make_profile_with("secretary", ["todo", "sync_files"])) + registry = make_registry( + ("todo", "notes and tasks"), ("sync_files", "copy files"), ("weather", "forecast") + ) + + result = await make_tool(registry, profiles).execute({"profile_id": "secretary"}) + + assert "Hand-written manuals available: todo" in result.output + assert "weather" not in result.output + assert "sync_files" not in result.output.split("Hand-written manuals")[1] + + +@pytest.mark.asyncio +async def test_no_manual_line_when_nothing_is_documented(monkeypatch): + monkeypatch.setattr("navi.tools.tool_manual.manual_names", set) + profiles = make_profiles(make_profile_with("secretary", ["todo"])) + registry = make_registry(("todo", "notes")) + + result = await make_tool(registry, profiles).execute({"profile_id": "secretary"}) + + assert "Hand-written manuals" not in result.output diff --git a/tests/unit/tools/test_manual_drift.py b/tests/unit/tools/test_manual_drift.py new file mode 100644 index 0000000..a20ddeb --- /dev/null +++ b/tests/unit/tools/test_manual_drift.py @@ -0,0 +1,285 @@ +"""manuals/ must stay in step with the tools that exist — and with the docs that cite it. + +A manual is filed under the tool's own name: ``compile_scad.md`` documents +``mcp__navi-3d__compile_scad``. That name is the only thing that lets tool_manual +find the file, and nothing enforced it. So three manuals documented tools that are +called something else — ``write_mcp_server.md``/``write_tool.md`` (no such tools at +all), ``model_3d.md`` (the tool is ``compile_scad``), ``render_3d.md`` (the tool is +``render_stl``) — and the hand-written text was dead weight while the agent got a +schema dump in its place. The prompts pointed at those same wrong names, so fixing +the files without fixing the callers would have moved the breakage, not removed it. + +These tests read the real repository tree, not a fixture: they are the guard on the +files as shipped, and they are what fails when the next tool is renamed. +""" + +from __future__ import annotations + +import ast +import difflib +import json +import re +from pathlib import Path + +import pytest + +from navi.core.registry import ToolRegistry +from navi.tools import tool_manual +from navi.tools.tool_manual import ToolManualTool +from tests.conftest_factory import FakeTool + +REPO_ROOT = Path(tool_manual.__file__).resolve().parents[2] + + +# ── where a real tool name can come from ───────────────────────────────── + +def _string_name_assignments(tree: ast.AST) -> set[str]: + """Every ``name = "literal"`` at module or class level — how tools declare themselves.""" + found: set[str] = set() + for node in ast.walk(tree): + if not isinstance(node, (ast.Module, ast.ClassDef)): + continue + for stmt in node.body: + if isinstance(stmt, ast.Assign) and isinstance(stmt.value, ast.Constant): + value = stmt.value.value + targets = [t for t in stmt.targets if isinstance(t, ast.Name)] + elif isinstance(stmt, ast.AnnAssign) and isinstance(stmt.value, ast.Constant): + value = stmt.value.value + targets = [stmt.target] if isinstance(stmt.target, ast.Name) else [] + else: + continue + if isinstance(value, str) and value and any(t.id == "name" for t in targets): + found.add(value) + return found + + +def _builtin_tool_names() -> set[str]: + """Built-ins (navi/tools/) and user tools (tools/) — _-prefixed scaffolds are skipped.""" + names: set[str] = set() + for pattern in ("navi/tools/*.py", "navi/tools/*/*.py", "tools/*.py"): + for path in REPO_ROOT.glob(pattern): + if path.name.startswith("_"): + continue + try: + names |= _string_name_assignments(ast.parse(path.read_text(encoding="utf-8"))) + except SyntaxError: # a file the interpreter would reject is not a tool + continue + return names + + +def _declared_mcp_tool_names() -> set[str]: + """``@mcp.tool(name="x")`` in the MCP server sources — the tools a server ships.""" + names: set[str] = set() + for path in REPO_ROOT.glob("mcp-servers/*/app/mcp_server.py"): + tree = ast.parse(path.read_text(encoding="utf-8")) + for node in ast.walk(tree): + if not isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): + continue + for decorator in node.decorator_list: + if not isinstance(decorator, ast.Call): + continue + func = decorator.func + if not (isinstance(func, ast.Attribute) and func.attr == "tool"): + continue + for kw in decorator.keywords: + if ( + kw.arg == "name" + and isinstance(kw.value, ast.Constant) + and isinstance(kw.value.value, str) + ): + names.add(kw.value.value) + return names + + +def _configured_mcp_tools() -> dict[str, set[str]]: + """server name -> tool names, from the tracked server configs. + + The server name is the config's filename (``navi/mcp/config.py`` takes it from + ``file_path.stem``), which is what makes ``mcp____`` computable here. + """ + servers: dict[str, set[str]] = {} + for path in sorted((REPO_ROOT / "mcp_servers.d").glob("*.json")): + groups = json.loads(path.read_text(encoding="utf-8")).get("groups") or {} + names = { + name + for group in groups.values() + if isinstance(group, list) + for name in group + if isinstance(name, str) + } + if names: + servers[path.stem] = names + return servers + + +NATIVE_TOOLS = _builtin_tool_names() +MCP_BY_SERVER = _configured_mcp_tools() +# The names the agent calls by (mcp____) and the bare ones it may ask +# about instead. Both are real spellings, so both count as "this tool exists". +MCP_FULL_NAMES = {f"mcp__{server}__{tool}" for server, tools in MCP_BY_SERVER.items() for tool in tools} +MCP_BARE_NAMES = ( + {tool for tools in MCP_BY_SERVER.values() for tool in tools} | _declared_mcp_tool_names() +) +KNOWN_TOOLS = NATIVE_TOOLS | MCP_BARE_NAMES | MCP_FULL_NAMES + + +def _full_names_for(tool: str) -> list[str]: + """Every mcp____ a bare name may legitimately mean (usually one).""" + return sorted( + f"mcp__{server}__{tool}" for server, tools in MCP_BY_SERVER.items() if tool in tools + ) + + +def _manuals() -> list[Path]: + return sorted(tool_manual.MANUALS_DIR.glob("*.md")) + + +def _manual_names_on_disk() -> set[str]: + return {path.stem for path in _manuals()} + + +def _closest(name: str) -> str: + matches = difflib.get_close_matches(name, sorted(KNOWN_TOOLS), n=3, cutoff=0.5) + return f" Closest real tool names: {', '.join(matches)}." if matches else "" + + +@pytest.fixture(scope="module") +def registry() -> ToolRegistry: + """Every tool that exists, registered — so the index can bucket by real source. + + Only the names the registry really holds: a bare MCP name is a spelling the agent + may use, not a tool of its own, and registering it would make tool_manual resolve + 'compile_scad' to a native tool that shadows the real MCP one. + """ + reg = ToolRegistry() + for name in sorted(NATIVE_TOOLS | MCP_FULL_NAMES): + reg.register(FakeTool(name), builtin=True) + return reg + + +# ── the manuals name real tools ────────────────────────────────────────── + +def test_the_manuals_directory_is_not_empty(): + """Every assertion below is vacuous on an empty tree — fail loudly instead.""" + assert _manuals(), f"no manuals found under {tool_manual.MANUALS_DIR}" + + +@pytest.mark.parametrize("path", _manuals(), ids=lambda p: p.stem) +def test_every_manual_is_named_after_a_tool_that_exists(path: Path): + """This is what catches a manual for a tool nobody can call — write_tool.md was one.""" + assert path.stem in KNOWN_TOOLS, ( + f"manuals/{path.name} documents '{path.stem}', which is not a tool: no built-in or " + f"tools/*.py declares it, and no mcp_servers.d group lists it.{_closest(path.stem)}" + ) + + +def test_a_guide_is_never_named_after_a_tool(): + """A guide sharing a tool's name would shadow that tool's manual in the index.""" + for path in sorted(tool_manual.GUIDES_DIR.glob("*.md")): + assert path.stem not in KNOWN_TOOLS, ( + f"manuals/guides/{path.name} is named after the real tool '{path.stem}' — a guide " + "is not a tool and must not be filed under a tool's name" + ) + + +# ── the manuals are reachable under the names the agent uses ───────────── + +@pytest.mark.parametrize("path", _manuals(), ids=lambda p: p.stem) +async def test_the_bare_name_returns_the_hand_written_manual(path: Path): + result = await ToolManualTool().execute({"tool_name": path.stem}) + + assert result.success is True + assert result.output == path.read_text(encoding="utf-8"), ( + f"manuals/{path.name} exists but tool_manual('{path.stem}') did not return it" + ) + + +@pytest.mark.parametrize( + "path", [p for p in _manuals() if _full_names_for(p.stem)], ids=lambda p: p.stem +) +async def test_an_mcp_manual_is_reachable_by_its_full_name(path: Path): + """The regression, on the real file: the agent calls the MCP spelling, the file is + the bare one — so the call must reach the hand-written text, not the schema.""" + for full_name in _full_names_for(path.stem): + result = await ToolManualTool().execute({"tool_name": full_name}) + + assert result.success is True + assert result.output == path.read_text(encoding="utf-8") + assert "not a hand-written manual" not in result.output + + +async def test_mcp_manuals_are_indexed_under_their_server(registry: ToolRegistry): + """A manual is filed bare, so its source can only come from the registry.""" + result = await ToolManualTool(registry=registry).execute({}) + + assert result.success is True + for path in _manuals(): + for full_name in _full_names_for(path.stem): + server_source = full_name.rsplit("__", 1)[0] + "__" + assert f"{server_source} (" in result.output, ( + f"manuals/{path.name} documents the tool '{full_name}', but the index does " + f"not file it under {server_source}" + ) + + +def test_the_index_lists_every_manual_on_disk(): + assert tool_manual.manual_names() == _manual_names_on_disk() + + +async def test_the_no_argument_call_succeeds_and_names_them(registry: ToolRegistry): + result = await ToolManualTool(registry=registry).execute({}) + + assert result.success is True + for path in _manuals(): + assert path.stem in result.output + for guide in sorted(tool_manual.GUIDES_DIR.glob("*.md")): + assert guide.stem in result.output + + +# ── the docs and prompts cite manuals that exist ───────────────────────── + +def _reference_sources() -> list[Path]: + """Live docs and prompts. docs/archive/ is history, not a claim about today.""" + sources = sorted((REPO_ROOT / "docs").glob("*.md")) + sources += sorted((REPO_ROOT / "navi/profiles").rglob("*.txt")) + sources += [REPO_ROOT / "persona_navi_code.txt", REPO_ROOT / "CLAUDE.md"] + return [path for path in sources if path.is_file()] + + +_TOOL_MANUAL_CALL = re.compile(r"""tool_manual\(\s*["']([^"']+)["']\s*\)""") +_MANUAL_PATH = re.compile(r"manuals/([A-Za-z0-9_./-]+\.md)") + + +def _citations(pattern: re.Pattern[str]) -> dict[str, list[str]]: + """pattern's capture group -> [':', …], over the live docs.""" + found: dict[str, list[str]] = {} + for path in _reference_sources(): + for number, line in enumerate(path.read_text(encoding="utf-8").splitlines(), 1): + for match in pattern.finditer(line): + found.setdefault(match.group(1), []).append(f"{path.relative_to(REPO_ROOT)}:{number}") + return found + + +def test_every_cited_manual_file_exists(): + """`manuals/.md` in a doc is a promise the file is there — moving the guide + into guides/ broke exactly this, in docs/context_providers.md.""" + missing = { + name: where + for name, where in _citations(_MANUAL_PATH).items() + if not (tool_manual.MANUALS_DIR / name).is_file() + } + assert not missing, f"cited manuals that do not exist: {missing}" + + +def test_every_tool_manual_call_resolves(): + """`tool_manual("x")` must reach something: a hand-written manual, a guide, or — + at worst — a real tool whose schema the tool can render.""" + unresolved = { + name: where + for name, where in _citations(_TOOL_MANUAL_CALL).items() + if tool_manual._read_manual(name) is None and name not in KNOWN_TOOLS + } + assert not unresolved, ( + f"tool_manual() calls that resolve to nothing: {unresolved} — name a real tool, or " + "write the manual the docs promise" + ) diff --git a/tests/unit/tools/test_tool_manual.py b/tests/unit/tools/test_tool_manual.py new file mode 100644 index 0000000..dbede92 --- /dev/null +++ b/tests/unit/tools/test_tool_manual.py @@ -0,0 +1,436 @@ +"""tool_manual — reaching the right manual, and failing usefully. + +The bug this covers: the tool looked up `manuals/.md` by the exact string the +model sent, then fell back to `registry.get(name)`. The agent calls MCP tools by +their full name (`mcp__navi-3d__compile_scad`), but the file is named after the tool +(`compile_scad.md`) — so a hand-written manual that existed was unreachable, and the +agent silently got a thin schema dump instead. The executor resolves such names fine; +the tool that *documents* them did not. +""" + +import pytest + +from navi.core.registry import ProfileRegistry, ToolRegistry +from navi.profiles.base import ToolConfig, ToolScopeConfig +from navi.tools import tool_manual +from navi.tools._internal.base import ToolContext +from navi.tools.tool_manual import ToolManualTool +from tests.conftest_factory import FakeTool, make_profile + + +@pytest.fixture(autouse=True) +def no_user_enabled_tools(monkeypatch): + """Every profile also picks up tools/enabled.json — a real file on the machine + running the tests. Tests must see exactly what they declare. + """ + monkeypatch.setattr("navi.core.tool_utils.load_user_enabled_tools", list) + + +@pytest.fixture +def manuals(tmp_path, monkeypatch): + """A manuals/ tree this test owns, with the guides/ subdir beside it.""" + root = tmp_path / "manuals" + (root / "guides").mkdir(parents=True) + monkeypatch.setattr(tool_manual, "MANUALS_DIR", root) + monkeypatch.setattr(tool_manual, "GUIDES_DIR", root / "guides") + return root + + +class FakeMcpManager: + """Stand-in for McpManager — group name to tool names, as a server config holds them.""" + + def __init__(self, groups: dict[str, dict[str, list[str]]] | None = None) -> None: + self._groups = groups or {} + + def resolve_group(self, server_name: str, group_name: str) -> list[str]: + return list(self._groups.get(server_name, {}).get(group_name, [])) + + +def build(registry=None, profiles=None, mcp_manager=None) -> ToolManualTool: + return ToolManualTool( + registry=registry, profile_registry=profiles, mcp_manager=mcp_manager + ) + + +def registry_with(*tools: FakeTool) -> ToolRegistry: + registry = ToolRegistry() + for tool in tools: + registry.register(tool, builtin=True) + return registry + + +def profiles_with(profile) -> ProfileRegistry: + registry = ProfileRegistry() + registry.register(profile) + return registry + + +def profile_with(profile_id="developer", native=(), mcp=None, subagent_native=None): + """A profile holding exactly the declared tools — no enabled_tools migration on top.""" + subagent = ( + ToolScopeConfig(native=list(subagent_native)) + if subagent_native is not None + else ToolScopeConfig() + ) + return make_profile( + profile_id, + enabled_tools=[], + tools=ToolConfig( + agent=ToolScopeConfig(native=list(native), mcp=dict(mcp or {})), + subagent=subagent, + ), + ) + + +class TestManualFiles: + async def test_a_bare_name_finds_the_manual(self, manuals): + (manuals / "compile_scad.md").write_text("# compile_scad\n\nhand-written\n") + + result = await build().execute({"tool_name": "compile_scad"}) + + assert result.success is True + assert "hand-written" in result.output + + async def test_the_full_mcp_name_reaches_the_hand_written_manual(self, manuals): + """The regression: the agent calls the MCP spelling, the file is the bare one.""" + (manuals / "compile_scad.md").write_text("# compile_scad\n\nhand-written\n") + + result = await build().execute({"tool_name": "mcp__navi-3d__compile_scad"}) + + assert result.success is True + assert "hand-written" in result.output + + async def test_an_underscored_server_name_reaches_it_too(self, manuals): + (manuals / "lint_scad.md").write_text("# lint_scad\n\nhand-written\n") + + result = await build().execute({"tool_name": "mcp__navi_3d__lint_scad"}) + + assert "hand-written" in result.output + + async def test_the_name_is_matched_case_insensitively(self, manuals): + (manuals / "compile_scad.md").write_text("# compile_scad\n\nhand-written\n") + + result = await build().execute({"tool_name": "Compile_SCAD"}) + + assert "hand-written" in result.output + + async def test_a_dash_spelling_finds_the_underscored_file(self, manuals): + (manuals / "compile_scad.md").write_text("# compile_scad\n\nhand-written\n") + + result = await build().execute({"tool_name": "compile-scad"}) + + assert "hand-written" in result.output + + async def test_a_guide_is_reachable_by_name(self, manuals): + """Guides live in guides/ and are not tools, but naming one must still read it.""" + (manuals / "guides" / "write_context_provider.md").write_text("# guide\n\nbody\n") + + result = await build().execute({"tool_name": "write_context_provider"}) + + assert result.success is True + assert "body" in result.output + + async def test_the_hand_written_manual_beats_the_generated_one(self, manuals): + (manuals / "filesystem.md").write_text("# filesystem\n\nhand-written\n") + registry = registry_with(FakeTool("filesystem", description="schema description")) + + result = await build(registry=registry).execute({"tool_name": "filesystem"}) + + assert "hand-written" in result.output + assert "schema description" not in result.output + + +class TestGeneratedManual: + async def test_an_unwritten_manual_falls_back_to_the_schema(self, manuals): + registry = registry_with(FakeTool("todo", description="Manage the todo list")) + + result = await build(registry=registry).execute({"tool_name": "todo"}) + + assert result.success is True + assert "# todo" in result.output + assert "Manage the todo list" in result.output + + async def test_a_bare_mcp_name_resolves_against_the_profiles_toolset(self, manuals): + """The same name the executor would resolve must resolve here — mcp group '*'.""" + registry = registry_with(FakeTool("mcp__navi-web__web_search", description="Search")) + profiles = profiles_with(profile_with("developer", mcp={"navi-web": ["*"]})) + + result = await build(registry=registry, profiles=profiles).execute( + {"tool_name": "web_search"}, ctx=ToolContext(profile_id="developer") + ) + + assert result.success is True + assert "# mcp__navi-web__web_search" in result.output + + +class TestGeneratedRendering: + """The generated manual is what most tools get, so it has to render the whole + schema: a model that cannot see the shape of an array of objects guesses it.""" + + def tool(self, parameters, description="A tool"): + return FakeTool("shape_probe", description=description, parameters=parameters) + + async def render(self, manuals, parameters, **kwargs): + registry = registry_with(self.tool({"type": "object", **parameters})) + result = await build(registry=registry, **kwargs).execute({"tool_name": "shape_probe"}) + return result.output + + async def test_the_header_names_the_source_and_the_schema(self, manuals): + out = await self.render(manuals, {"properties": {}}) + + assert "Generated from the tool's JSON schema (source: native)" in out + assert "not a hand-written manual" in out + + async def test_an_mcp_tool_reports_its_server_as_the_source(self, manuals): + registry = registry_with(FakeTool("mcp__navi-3d__compile_scad", description="Scad")) + profiles = profiles_with(profile_with("developer", mcp={"navi-3d": ["*"]})) + + result = await build(registry=registry, profiles=profiles).execute( + {"tool_name": "compile_scad"}, ctx=ToolContext(profile_id="developer") + ) + + assert "source: mcp__navi-3d__" in result.output + + async def test_a_tool_with_no_parameters_says_so(self, manuals): + out = await self.render(manuals, {}) + + assert "takes no parameters" in out + + async def test_a_nested_object_is_spelled_out(self, manuals): + out = await self.render( + manuals, + { + "properties": { + "filter": { + "type": "object", + "description": "Restrict the search", + "properties": { + "limit": {"type": "integer", "description": "How many"}, + "tag": {"type": "string"}, + }, + "required": ["limit"], + } + } + }, + ) + + assert "- `filter` (object, optional): Restrict the search" in out + assert "`limit` (integer, required): How many" in out + assert "`tag` (string, optional)" in out + + async def test_an_array_of_objects_shows_the_item_shape(self, manuals): + out = await self.render( + manuals, + { + "properties": { + "entries": { + "type": "array", + "description": "Rows to write", + "items": { + "type": "object", + "properties": { + "name": {"type": "string", "description": "Key"}, + "value": {"type": "number"}, + }, + "required": ["name"], + }, + } + } + }, + ) + + assert "(array of object, optional): Rows to write" in out + assert "each item:" in out + assert "`name` (string, required): Key" in out + assert "`value` (number, optional)" in out + + async def test_an_array_of_scalars_names_the_item_type(self, manuals): + out = await self.render(manuals, {"properties": {"tags": {"type": "array", "items": {"type": "string"}}}}) + + assert "(array of string, optional)" in out + + async def test_enum_and_default_are_shown(self, manuals): + out = await self.render( + manuals, + { + "properties": { + "action": { + "type": "string", + "description": "What to do", + "enum": ["add", "remove"], + "default": "add", + } + } + }, + ) + + assert "one of: 'add', 'remove'" in out + assert "default: 'add'" in out + + async def test_a_union_is_rendered_with_its_branches(self, manuals): + out = await self.render( + manuals, + { + "properties": { + "target": { + "description": "Where to write", + "oneOf": [ + {"type": "string", "description": "A path"}, + {"type": "array", "items": {"type": "string"}}, + ], + } + } + }, + ) + + assert "(string | array of string, optional): Where to write" in out + assert "oneOf — one of:" in out + assert "• string — A path" in out + + async def test_deep_nesting_is_capped_not_unbounded(self, manuals): + """A pathological schema must not turn the manual into an infinite tree.""" + spec = {"type": "string", "description": "bottom"} + for level in range(12): + spec = { + "type": "object", + "properties": {"next": spec}, + "description": f"level {level}", + } + + out = await self.render(manuals, {"properties": {"root": spec}}) + + assert "nested deeper" in out + assert out.count("- `next`") <= 6 + + +class TestProfileScope: + async def test_a_registered_tool_warns_it_is_not_enabled_for_the_profile(self, manuals): + registry = registry_with(FakeTool("ssh_exec", description="Run a remote command")) + profiles = profiles_with(profile_with("secretary", native=["filesystem"])) + + result = await build(registry=registry, profiles=profiles).execute( + {"tool_name": "ssh_exec"}, ctx=ToolContext(profile_id="secretary") + ) + + # Still documents it — the hint must not turn into a refusal — but says why + # calling it would fail. + assert result.success is True + assert "not enabled for profile 'secretary'" in result.output + assert "Run a remote command" in result.output + + async def test_a_tool_only_in_the_subagent_scope_is_not_called_disabled(self, manuals): + """A sub-agent asking about its own tool must not be told it lacks it.""" + registry = registry_with(FakeTool("ssh_exec", description="Run a remote command")) + profiles = profiles_with(profile_with("developer", subagent_native=["ssh_exec"])) + + result = await build(registry=registry, profiles=profiles).execute( + {"tool_name": "ssh_exec"}, ctx=ToolContext(profile_id="developer") + ) + + assert "not enabled" not in result.output + assert "Run a remote command" in result.output + + async def test_without_an_active_profile_a_registered_tool_is_still_documented(self, manuals): + registry = registry_with(FakeTool("ssh_exec", description="Run a remote command")) + + result = await build(registry=registry).execute({"tool_name": "ssh_exec"}) + + assert result.success is True + assert "not enabled for this run" in result.output + assert "Run a remote command" in result.output + + +class TestNoArgument: + """The old code did params["tool_name"].strip() — KeyError when the model omits + the key, AttributeError when it sends null or a number. Neither is an answer, so + a missing name now returns the catalogue of what is documented.""" + + async def test_no_name_gives_the_catalogue_of_manuals(self, manuals): + (manuals / "filesystem.md").write_text("# filesystem\n") + (manuals / "todo.md").write_text("# todo\n") + + result = await build().execute({}) + + assert result.success is True + assert "filesystem" in result.output + assert "todo" in result.output + assert "tool_name" in result.output + + @pytest.mark.parametrize("bad", [None, 42, [], {"a": 1}]) + async def test_a_non_string_name_is_not_a_crash(self, manuals, bad): + (manuals / "filesystem.md").write_text("# filesystem\n") + + result = await build().execute({"tool_name": bad}) + + assert result.success is True + assert "filesystem" in result.output + + async def test_a_whitespace_name_is_not_a_crash(self, manuals): + (manuals / "filesystem.md").write_text("# filesystem\n") + + result = await build().execute({"tool_name": " "}) + + assert result.success is True + assert "filesystem" in result.output + + async def test_no_manuals_at_all_is_not_a_crash(self, manuals): + result = await build().execute({}) + + assert result.success is False + assert result.error == "no_manuals" + + +class TestIndex: + async def test_manuals_are_grouped_by_source(self, manuals): + """A manual is filed under the tool's name — which source it belongs to comes + from the registry, not from how the file happens to be spelled.""" + (manuals / "filesystem.md").write_text("# filesystem\n") + (manuals / "web_search.md").write_text("# web_search\n") + registry = registry_with( + FakeTool("filesystem"), FakeTool("mcp__navi-web__web_search") + ) + + result = await build(registry=registry).execute({}) + + assert "native (1): filesystem" in result.output + assert "mcp__navi-web__ (1): web_search" in result.output + + async def test_guides_are_listed_apart_from_tools(self, manuals): + (manuals / "filesystem.md").write_text("# filesystem\n") + (manuals / "guides" / "write_context_provider.md").write_text("# guide\n") + + result = await build().execute({}) + + assert "guides (not tools; readable by name): write_context_provider" in result.output + assert "write_context_provider" not in result.output.split("guides")[0] + + async def test_the_index_says_other_tools_still_have_a_manual(self, manuals): + (manuals / "filesystem.md").write_text("# filesystem\n") + + result = await build().execute({}) + + assert "generated from its schema" in result.output + + +class TestMiss: + async def test_a_typo_gets_a_suggestion(self, manuals): + registry = registry_with(FakeTool("filesystem", description="Files")) + + result = await build(registry=registry).execute({"tool_name": "filesistem"}) + + assert result.success is False + assert result.error == "not_found" + assert "filesystem" in result.output + + async def test_a_typo_in_a_bare_mcp_name_gets_a_suggestion(self, manuals): + registry = registry_with(FakeTool("mcp__navi-web__web_search", description="Search")) + + result = await build(registry=registry).execute({"tool_name": "web_serch"}) + + assert "web_search" in result.output + + async def test_a_hopeless_name_still_points_at_list_tools(self, manuals): + result = await build().execute({"tool_name": "zzzz_qqqq_nonsense"}) + + assert result.success is False + assert "list_tools" in result.output