diff --git a/AGENTS.md b/AGENTS.md index 19c6f27977..8045c3d213 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -210,6 +210,10 @@ registry.register( The registry handles schema collection, dispatch, availability checking, and error wrapping. All handlers MUST return a JSON string. +**Path references in tool schemas**: If the schema description mentions file paths (e.g. default output directories), use `display_hermes_home()` to make them profile-aware. The schema is generated at import time, which is after `_apply_profile_override()` sets `HERMES_HOME`. + +**State files**: If a tool stores persistent state (caches, logs, checkpoints), use `get_hermes_home()` for the base directory — never `Path.home() / ".hermes"`. This ensures each profile gets its own state. + **Agent-level tools** (todo, memory): intercepted by `run_agent.py` before `handle_function_call()`. See `todo_tool.py` for the pattern. --- @@ -358,8 +362,69 @@ in config.yaml (or `HERMES_BACKGROUND_NOTIFICATIONS` env var): --- +## Profiles: Multi-Instance Support + +Hermes supports **profiles** — multiple fully isolated instances, each with its own +`HERMES_HOME` directory (config, API keys, memory, sessions, skills, gateway, etc.). + +The core mechanism: `_apply_profile_override()` in `hermes_cli/main.py` sets +`HERMES_HOME` before any module imports. All 119+ references to `get_hermes_home()` +automatically scope to the active profile. + +### Rules for profile-safe code + +1. **Use `get_hermes_home()` for all HERMES_HOME paths.** Import from `hermes_constants`. + NEVER hardcode `~/.hermes` or `Path.home() / ".hermes"` in code that reads/writes state. + ```python + # GOOD + from hermes_constants import get_hermes_home + config_path = get_hermes_home() / "config.yaml" + + # BAD — breaks profiles + config_path = Path.home() / ".hermes" / "config.yaml" + ``` + +2. **Use `display_hermes_home()` for user-facing messages.** Import from `hermes_constants`. + This returns `~/.hermes` for default or `~/.hermes/profiles/` for profiles. + ```python + # GOOD + from hermes_constants import display_hermes_home + print(f"Config saved to {display_hermes_home()}/config.yaml") + + # BAD — shows wrong path for profiles + print("Config saved to ~/.hermes/config.yaml") + ``` + +3. **Module-level constants are fine** — they cache `get_hermes_home()` at import time, + which is AFTER `_apply_profile_override()` sets the env var. Just use `get_hermes_home()`, + not `Path.home() / ".hermes"`. + +4. **Tests that mock `Path.home()` must also set `HERMES_HOME`** — since code now uses + `get_hermes_home()` (reads env var), not `Path.home() / ".hermes"`: + ```python + with patch.object(Path, "home", return_value=tmp_path), \ + patch.dict(os.environ, {"HERMES_HOME": str(tmp_path / ".hermes")}): + ... + ``` + +5. **Gateway platform adapters should use token locks** — if the adapter connects with + a unique credential (bot token, API key), call `acquire_scoped_lock()` from + `gateway.status` in the `connect()`/`start()` method and `release_scoped_lock()` in + `disconnect()`/`stop()`. This prevents two profiles from using the same credential. + See `gateway/platforms/telegram.py` for the canonical pattern. + +6. **Profile operations are HOME-anchored, not HERMES_HOME-anchored** — `_get_profiles_root()` + returns `Path.home() / ".hermes" / "profiles"`, NOT `get_hermes_home() / "profiles"`. + This is intentional — it lets `hermes -p coder profile list` see all profiles regardless + of which one is active. + ## Known Pitfalls +### DO NOT hardcode `~/.hermes` paths +Use `get_hermes_home()` from `hermes_constants` for code paths. Use `display_hermes_home()` +for user-facing print/log messages. Hardcoding `~/.hermes` breaks profiles — each profile +has its own `HERMES_HOME` directory. This was the source of 5 bugs fixed in PR #3575. + ### DO NOT use `simple_term_menu` for interactive menus Rendering bugs in tmux/iTerm2 — ghosting on scroll. Use `curses` (stdlib) instead. See `hermes_cli/tools_config.py` for the pattern. @@ -375,6 +440,19 @@ Tool schema descriptions must not mention tools from other toolsets by name (e.g ### Tests must not write to `~/.hermes/` The `_isolate_hermes_home` autouse fixture in `tests/conftest.py` redirects `HERMES_HOME` to a temp dir. Never hardcode `~/.hermes/` paths in tests. +**Profile tests**: When testing profile features, also mock `Path.home()` so that +`_get_profiles_root()` and `_get_default_hermes_home()` resolve within the temp dir. +Use the pattern from `tests/hermes_cli/test_profiles.py`: +```python +@pytest.fixture +def profile_env(tmp_path, monkeypatch): + home = tmp_path / ".hermes" + home.mkdir() + monkeypatch.setattr(Path, "home", lambda: tmp_path) + monkeypatch.setenv("HERMES_HOME", str(home)) + return home +``` + --- ## Testing diff --git a/agent/prompt_builder.py b/agent/prompt_builder.py index c8335ef32e..8bc01251bf 100644 --- a/agent/prompt_builder.py +++ b/agent/prompt_builder.py @@ -18,6 +18,7 @@ from typing import Optional from agent.skill_utils import ( extract_skill_conditions, extract_skill_description, + get_all_skills_dirs, get_disabled_skill_names, iter_skill_index_files, parse_frontmatter, @@ -444,16 +445,23 @@ def build_skills_system_prompt( mtime/size manifest — survives process restarts Falls back to a full filesystem scan when both layers miss. + + External skill directories (``skills.external_dirs`` in config.yaml) are + scanned alongside the local ``~/.hermes/skills/`` directory. External dirs + are read-only — they appear in the index but new skills are always created + in the local dir. Local skills take precedence when names collide. """ hermes_home = get_hermes_home() skills_dir = hermes_home / "skills" + external_dirs = get_all_skills_dirs()[1:] # skip local (index 0) - if not skills_dir.exists(): + if not skills_dir.exists() and not external_dirs: return "" # ── Layer 1: in-process LRU cache ───────────────────────────────── cache_key = ( str(skills_dir.resolve()), + tuple(str(d) for d in external_dirs), tuple(sorted(str(t) for t in (available_tools or set()))), tuple(sorted(str(ts) for ts in (available_toolsets or set()))), ) @@ -540,6 +548,56 @@ def build_skills_system_prompt( category_descriptions, ) + # ── External skill directories ───────────────────────────────────── + # Scan external dirs directly (no snapshot caching — they're read-only + # and typically small). Local skills already in skills_by_category take + # precedence: we track seen names and skip duplicates from external dirs. + seen_skill_names: set[str] = set() + for cat_skills in skills_by_category.values(): + for name, _desc in cat_skills: + seen_skill_names.add(name) + + for ext_dir in external_dirs: + if not ext_dir.exists(): + continue + for skill_file in iter_skill_index_files(ext_dir, "SKILL.md"): + try: + is_compatible, frontmatter, desc = _parse_skill_file(skill_file) + if not is_compatible: + continue + entry = _build_snapshot_entry(skill_file, ext_dir, frontmatter, desc) + skill_name = entry["skill_name"] + if skill_name in seen_skill_names: + continue + if entry["frontmatter_name"] in disabled or skill_name in disabled: + continue + if not _skill_should_show( + extract_skill_conditions(frontmatter), + available_tools, + available_toolsets, + ): + continue + seen_skill_names.add(skill_name) + skills_by_category.setdefault(entry["category"], []).append( + (skill_name, entry["description"]) + ) + except Exception as e: + logger.debug("Error reading external skill %s: %s", skill_file, e) + + # External category descriptions + for desc_file in iter_skill_index_files(ext_dir, "DESCRIPTION.md"): + try: + content = desc_file.read_text(encoding="utf-8") + fm, _ = parse_frontmatter(content) + cat_desc = fm.get("description") + if not cat_desc: + continue + rel = desc_file.relative_to(ext_dir) + cat = "/".join(rel.parts[:-1]) if len(rel.parts) > 1 else "general" + category_descriptions.setdefault(cat, str(cat_desc).strip().strip("'\"")) + except Exception as e: + logger.debug("Could not read external skill description %s: %s", desc_file, e) + if not skills_by_category: result = "" else: diff --git a/agent/skill_commands.py b/agent/skill_commands.py index b266ad251c..8a434ea799 100644 --- a/agent/skill_commands.py +++ b/agent/skill_commands.py @@ -128,7 +128,11 @@ def _build_skill_message( supporting.append(rel) if supporting and skill_dir: - skill_view_target = str(skill_dir.relative_to(SKILLS_DIR)) + try: + skill_view_target = str(skill_dir.relative_to(SKILLS_DIR)) + except ValueError: + # Skill is from an external dir — use the skill name instead + skill_view_target = skill_dir.name parts.append("") parts.append("[This skill has supporting files you can load with the skill_view tool:]") for sf in supporting: @@ -158,38 +162,49 @@ def scan_skill_commands() -> Dict[str, Dict[str, Any]]: _skill_commands = {} try: from tools.skills_tool import SKILLS_DIR, _parse_frontmatter, skill_matches_platform, _get_disabled_skill_names - if not SKILLS_DIR.exists(): - return _skill_commands + from agent.skill_utils import get_external_skills_dirs disabled = _get_disabled_skill_names() - for skill_md in SKILLS_DIR.rglob("SKILL.md"): - if any(part in ('.git', '.github', '.hub') for part in skill_md.parts): - continue - try: - content = skill_md.read_text(encoding='utf-8') - frontmatter, body = _parse_frontmatter(content) - # Skip skills incompatible with the current OS platform - if not skill_matches_platform(frontmatter): + seen_names: set = set() + + # Scan local dir first, then external dirs + dirs_to_scan = [] + if SKILLS_DIR.exists(): + dirs_to_scan.append(SKILLS_DIR) + dirs_to_scan.extend(get_external_skills_dirs()) + + for scan_dir in dirs_to_scan: + for skill_md in scan_dir.rglob("SKILL.md"): + if any(part in ('.git', '.github', '.hub') for part in skill_md.parts): continue - name = frontmatter.get('name', skill_md.parent.name) - # Respect user's disabled skills config - if name in disabled: + try: + content = skill_md.read_text(encoding='utf-8') + frontmatter, body = _parse_frontmatter(content) + # Skip skills incompatible with the current OS platform + if not skill_matches_platform(frontmatter): + continue + name = frontmatter.get('name', skill_md.parent.name) + if name in seen_names: + continue + # Respect user's disabled skills config + if name in disabled: + continue + description = frontmatter.get('description', '') + if not description: + for line in body.strip().split('\n'): + line = line.strip() + if line and not line.startswith('#'): + description = line[:80] + break + seen_names.add(name) + cmd_name = name.lower().replace(' ', '-').replace('_', '-') + _skill_commands[f"/{cmd_name}"] = { + "name": name, + "description": description or f"Invoke the {name} skill", + "skill_md_path": str(skill_md), + "skill_dir": str(skill_md.parent), + } + except Exception: continue - description = frontmatter.get('description', '') - if not description: - for line in body.strip().split('\n'): - line = line.strip() - if line and not line.startswith('#'): - description = line[:80] - break - cmd_name = name.lower().replace(' ', '-').replace('_', '-') - _skill_commands[f"/{cmd_name}"] = { - "name": name, - "description": description or f"Invoke the {name} skill", - "skill_md_path": str(skill_md), - "skill_dir": str(skill_md.parent), - } - except Exception: - continue except Exception: pass return _skill_commands diff --git a/agent/skill_utils.py b/agent/skill_utils.py index 5cb2a71050..c11bc5e2d4 100644 --- a/agent/skill_utils.py +++ b/agent/skill_utils.py @@ -158,6 +158,73 @@ def _normalize_string_set(values) -> Set[str]: return {str(v).strip() for v in values if str(v).strip()} +# ── External skills directories ────────────────────────────────────────── + + +def get_external_skills_dirs() -> List[Path]: + """Read ``skills.external_dirs`` from config.yaml and return validated paths. + + Each entry is expanded (``~`` and ``${VAR}``) and resolved to an absolute + path. Only directories that actually exist are returned. Duplicates and + paths that resolve to the local ``~/.hermes/skills/`` are silently skipped. + """ + config_path = get_hermes_home() / "config.yaml" + if not config_path.exists(): + return [] + try: + parsed = yaml_load(config_path.read_text(encoding="utf-8")) + except Exception: + return [] + if not isinstance(parsed, dict): + return [] + + skills_cfg = parsed.get("skills") + if not isinstance(skills_cfg, dict): + return [] + + raw_dirs = skills_cfg.get("external_dirs") + if not raw_dirs: + return [] + if isinstance(raw_dirs, str): + raw_dirs = [raw_dirs] + if not isinstance(raw_dirs, list): + return [] + + local_skills = (get_hermes_home() / "skills").resolve() + seen: Set[Path] = set() + result: List[Path] = [] + + for entry in raw_dirs: + entry = str(entry).strip() + if not entry: + continue + # Expand ~ and environment variables + expanded = os.path.expanduser(os.path.expandvars(entry)) + p = Path(expanded).resolve() + if p == local_skills: + continue + if p in seen: + continue + if p.is_dir(): + seen.add(p) + result.append(p) + else: + logger.debug("External skills dir does not exist, skipping: %s", p) + + return result + + +def get_all_skills_dirs() -> List[Path]: + """Return all skill directories: local ``~/.hermes/skills/`` first, then external. + + The local dir is always first (and always included even if it doesn't exist + yet — callers handle that). External dirs follow in config order. + """ + dirs = [get_hermes_home() / "skills"] + dirs.extend(get_external_skills_dirs()) + return dirs + + # ── Condition extraction ────────────────────────────────────────────────── diff --git a/cli-config.yaml.example b/cli-config.yaml.example index 036efbc337..1f7a515c9c 100644 --- a/cli-config.yaml.example +++ b/cli-config.yaml.example @@ -402,6 +402,15 @@ skills: # Set to 0 to disable. creation_nudge_interval: 15 + # External skill directories — share skills across tools/agents without + # copying them into ~/.hermes/skills/. Each path is expanded (~ and ${VAR}) + # and resolved to an absolute path. External dirs are read-only: skill + # creation always writes to ~/.hermes/skills/. Local skills take precedence + # when names collide. + # external_dirs: + # - ~/.agents/skills + # - /home/shared/team-skills + # ============================================================================= # Agent Behavior # ============================================================================= diff --git a/hermes_cli/config.py b/hermes_cli/config.py index a88433218d..7486f34b98 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -366,6 +366,13 @@ DEFAULT_CONFIG = { # Never saved to sessions, logs, or trajectories. "prefill_messages_file": "", + # Skills — external skill directories for sharing skills across tools/agents. + # Each path is expanded (~, ${VAR}) and resolved. Read-only — skill creation + # always goes to ~/.hermes/skills/. + "skills": { + "external_dirs": [], # e.g. ["~/.agents/skills", "/shared/team-skills"] + }, + # Honcho AI-native memory -- reads ~/.honcho/config.json as single source of truth. # This section is only needed for hermes-specific overrides; everything else # (apiKey, workspace, peerName, sessions, enabled) comes from the global config. diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 79499d8334..3fad92f848 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -1045,6 +1045,7 @@ def _model_flow_openrouter(config, current_model=""): cfg["model"] = model model["provider"] = "openrouter" model["base_url"] = OPENROUTER_BASE_URL + model["api_mode"] = "chat_completions" save_config(cfg) deactivate_provider() print(f"Default model set to: {selected} (via OpenRouter)") @@ -1268,6 +1269,7 @@ def _model_flow_custom(config): cfg["model"] = model model["provider"] = "custom" model["base_url"] = effective_url + model["api_mode"] = "chat_completions" save_config(cfg) deactivate_provider() @@ -2049,6 +2051,7 @@ def _model_flow_kimi(config, current_model=""): cfg["model"] = model model["provider"] = provider_id model["base_url"] = effective_base + model["api_mode"] = "chat_completions" save_config(cfg) deactivate_provider() @@ -2155,6 +2158,7 @@ def _model_flow_api_key_provider(config, provider_id, current_model=""): cfg["model"] = model model["provider"] = provider_id model["base_url"] = effective_base + model["api_mode"] = "chat_completions" save_config(cfg) deactivate_provider() diff --git a/skills/productivity/google-workspace/SKILL.md b/skills/productivity/google-workspace/SKILL.md index 00d91de909..5d1c71bfb5 100644 --- a/skills/productivity/google-workspace/SKILL.md +++ b/skills/productivity/google-workspace/SKILL.md @@ -4,6 +4,11 @@ description: Gmail, Calendar, Drive, Contacts, Sheets, and Docs integration via version: 1.0.0 author: Nous Research license: MIT +required_credential_files: + - path: google_token.json + description: Google OAuth2 token (created by setup script) + - path: google_client_secret.json + description: Google OAuth2 client credentials (downloaded from Google Cloud Console) metadata: hermes: tags: [Google, Gmail, Calendar, Drive, Sheets, Docs, Contacts, Email, OAuth] diff --git a/skills/research/duckduckgo-search/SKILL.md b/skills/research/duckduckgo-search/SKILL.md index 0bfc647396..ea14e6b30f 100644 --- a/skills/research/duckduckgo-search/SKILL.md +++ b/skills/research/duckduckgo-search/SKILL.md @@ -1,7 +1,7 @@ --- name: duckduckgo-search -description: Free web search via DuckDuckGo — text, news, images, videos. No API key needed. Use the Python DDGS library or CLI to search, then web_extract for full content. -version: 1.2.0 +description: Free web search via DuckDuckGo — text, news, images, videos. No API key needed. Prefer the `ddgs` CLI when installed; use the Python DDGS library only after verifying that `ddgs` is available in the current runtime. +version: 1.3.0 author: gamedevCloudy license: MIT metadata: @@ -9,26 +9,96 @@ metadata: tags: [search, duckduckgo, web-search, free, fallback] related_skills: [arxiv] fallback_for_toolsets: [web] -prerequisites: - commands: [ddgs] --- # DuckDuckGo Search Free web search using DuckDuckGo. **No API key required.** -Preferred when `web_search` tool is unavailable or unsuitable (no `FIRECRAWL_API_KEY` set). Can also be used as a standalone search tool. +Preferred when `web_search` is unavailable or unsuitable (for example when `FIRECRAWL_API_KEY` is not set). Can also be used as a standalone search path when DuckDuckGo results are specifically desired. -## Setup +## Detection Flow + +Check what is actually available before choosing an approach: ```bash -# Install the ddgs package (one-time) -pip install ddgs +# Check CLI availability +command -v ddgs >/dev/null && echo "DDGS_CLI=installed" || echo "DDGS_CLI=missing" ``` -## Python API (Primary) +Decision tree: +1. If `ddgs` CLI is installed, prefer `terminal` + `ddgs` +2. If `ddgs` CLI is missing, do not assume `execute_code` can import `ddgs` +3. If the user wants DuckDuckGo specifically, install `ddgs` first in the relevant environment +4. Otherwise fall back to built-in web/browser tools -Use the `DDGS` class in `execute_code` for structured results with typed fields. +Important runtime note: +- Terminal and `execute_code` are separate runtimes +- A successful shell install does not guarantee `execute_code` can import `ddgs` +- Never assume third-party Python packages are preinstalled inside `execute_code` + +## Installation + +Install `ddgs` only when DuckDuckGo search is specifically needed and the runtime does not already provide it. + +```bash +# Python package + CLI entrypoint +pip install ddgs + +# Verify CLI +ddgs --help +``` + +If a workflow depends on Python imports, verify that same runtime can import `ddgs` before using `from ddgs import DDGS`. + +## Method 1: CLI Search (Preferred) + +Use the `ddgs` command via `terminal` when it exists. This is the preferred path because it avoids assuming the `execute_code` sandbox has the `ddgs` Python package installed. + +```bash +# Text search +ddgs text -k "python async programming" -m 5 + +# News search +ddgs news -k "artificial intelligence" -m 5 + +# Image search +ddgs images -k "landscape photography" -m 10 + +# Video search +ddgs videos -k "python tutorial" -m 5 + +# With region filter +ddgs text -k "best restaurants" -m 5 -r us-en + +# Recent results only (d=day, w=week, m=month, y=year) +ddgs text -k "latest AI news" -m 5 -t w + +# JSON output for parsing +ddgs text -k "fastapi tutorial" -m 5 -o json +``` + +### CLI Flags + +| Flag | Description | Example | +|------|-------------|---------| +| `-k` | Keywords (query) — **required** | `-k "search terms"` | +| `-m` | Max results | `-m 5` | +| `-r` | Region | `-r us-en` | +| `-t` | Time limit | `-t w` (week) | +| `-s` | Safe search | `-s off` | +| `-o` | Output format | `-o json` | + +## Method 2: Python API (Only After Verification) + +Use the `DDGS` class in `execute_code` or another Python runtime only after verifying that `ddgs` is installed there. Do not assume `execute_code` includes third-party packages by default. + +Safe wording: +- "Use `execute_code` with `ddgs` after installing or verifying the package if needed" + +Avoid saying: +- "`execute_code` includes `ddgs`" +- "DuckDuckGo search works by default in `execute_code`" **Important:** `max_results` must always be passed as a **keyword argument** — positional usage raises an error on all methods. @@ -76,7 +146,7 @@ from ddgs import DDGS with DDGS() as ddgs: for r in ddgs.images("semiconductor chip", max_results=5): print(r["title"]) - print(r["image"]) # direct image URL + print(r["image"]) print(r.get("thumbnail", "")) print(r.get("source", "")) print() @@ -94,9 +164,9 @@ from ddgs import DDGS with DDGS() as ddgs: for r in ddgs.videos("FastAPI tutorial", max_results=5): print(r["title"]) - print(r.get("content", "")) # video URL - print(r.get("duration", "")) # e.g. "26:03" - print(r.get("provider", "")) # YouTube, etc. + print(r.get("content", "")) + print(r.get("duration", "")) + print(r.get("provider", "")) print(r.get("published", "")) print() ``` @@ -112,50 +182,17 @@ Returns: `title`, `content`, `description`, `duration`, `provider`, `published`, | `images()` | Visuals, diagrams | title, image, thumbnail, url | | `videos()` | Tutorials, demos | title, content, duration, provider | -## CLI (Alternative) - -Use the `ddgs` command via terminal when you don't need structured field access. - -```bash -# Text search -ddgs text -k "python async programming" -m 5 - -# News search -ddgs news -k "artificial intelligence" -m 5 - -# Image search -ddgs images -k "landscape photography" -m 10 - -# Video search -ddgs videos -k "python tutorial" -m 5 - -# With region filter -ddgs text -k "best restaurants" -m 5 -r us-en - -# Recent results only (d=day, w=week, m=month, y=year) -ddgs text -k "latest AI news" -m 5 -t w - -# JSON output for parsing -ddgs text -k "fastapi tutorial" -m 5 -o json -``` - -### CLI Flags - -| Flag | Description | Example | -|------|-------------|---------| -| `-k` | Keywords (query) — **required** | `-k "search terms"` | -| `-m` | Max results | `-m 5` | -| `-r` | Region | `-r us-en` | -| `-t` | Time limit | `-t w` (week) | -| `-s` | Safe search | `-s off` | -| `-o` | Output format | `-o json` | - ## Workflow: Search then Extract -DuckDuckGo returns titles, URLs, and snippets — not full page content. To get full content, follow up with `web_extract`: +DuckDuckGo returns titles, URLs, and snippets — not full page content. To get full page content, search first and then extract the most relevant URL with `web_extract`, browser tools, or curl. -1. **Search** with ddgs to find relevant URLs -2. **Extract** content using the `web_extract` tool (if available) or curl +CLI example: + +```bash +ddgs text -k "fastapi deployment guide" -m 3 -o json +``` + +Python example, only after verifying `ddgs` is installed in that runtime: ```python from ddgs import DDGS @@ -164,25 +201,37 @@ with DDGS() as ddgs: results = list(ddgs.text("fastapi deployment guide", max_results=3)) for r in results: print(r["title"], "->", r["href"]) - -# Then use web_extract tool on the best URL ``` +Then extract the best URL with `web_extract` or another content-retrieval tool. + ## Limitations - **Rate limiting**: DuckDuckGo may throttle after many rapid requests. Add a short delay between searches if needed. -- **No content extraction**: ddgs returns snippets, not full page content. Use `web_extract` or curl for that. +- **No content extraction**: `ddgs` returns snippets, not full page content. Use `web_extract`, browser tools, or curl for the full article/page. - **Results quality**: Generally good but less configurable than Firecrawl's search. - **Availability**: DuckDuckGo may block requests from some cloud IPs. If searches return empty, try different keywords or wait a few seconds. -- **Field variability**: Return fields may vary between results or ddgs versions. Use `.get()` for optional fields to avoid KeyError. +- **Field variability**: Return fields may vary between results or `ddgs` versions. Use `.get()` for optional fields to avoid `KeyError`. +- **Separate runtimes**: A successful `ddgs` install in terminal does not automatically mean `execute_code` can import it. + +## Troubleshooting + +| Problem | Likely Cause | What To Do | +|---------|--------------|------------| +| `ddgs: command not found` | CLI not installed in the shell environment | Install `ddgs`, or use built-in web/browser tools instead | +| `ModuleNotFoundError: No module named 'ddgs'` | Python runtime does not have the package installed | Do not use Python DDGS there until that runtime is prepared | +| Search returns nothing | Temporary rate limiting or poor query | Wait a few seconds, retry, or adjust the query | +| CLI works but `execute_code` import fails | Terminal and `execute_code` are different runtimes | Keep using CLI, or separately prepare the Python runtime | ## Pitfalls - **`max_results` is keyword-only**: `ddgs.text("query", 5)` raises an error. Use `ddgs.text("query", max_results=5)`. +- **Do not assume the CLI exists**: Check `command -v ddgs` before using it. +- **Do not assume `execute_code` can import `ddgs`**: `from ddgs import DDGS` may fail with `ModuleNotFoundError` unless that runtime was prepared separately. +- **Package name**: The package is `ddgs` (previously `duckduckgo-search`). Install with `pip install ddgs`. - **Don't confuse `-k` and `-m`** (CLI): `-k` is for keywords, `-m` is for max results count. -- **Package name**: The package is `ddgs` (was previously `duckduckgo-search`). Install with `pip install ddgs`. -- **Empty results**: If ddgs returns nothing, it may be rate-limited. Wait a few seconds and retry. +- **Empty results**: If `ddgs` returns nothing, it may be rate-limited. Wait a few seconds and retry. ## Validated With -Smoke-tested with `ddgs==9.11.2` on Python 3.13. All four methods (text, news, images, videos) confirmed working with keyword `max_results`. +Validated examples against `ddgs==9.11.2` semantics. Skill guidance now treats CLI availability and Python import availability as separate concerns so the documented workflow matches actual runtime behavior. diff --git a/tests/agent/test_external_skills.py b/tests/agent/test_external_skills.py new file mode 100644 index 0000000000..1a9cd63d58 --- /dev/null +++ b/tests/agent/test_external_skills.py @@ -0,0 +1,157 @@ +"""Tests for external skill directories (skills.external_dirs config).""" + +import json +import os +from pathlib import Path +from unittest.mock import patch + +import pytest + + +@pytest.fixture +def external_skills_dir(tmp_path): + """Create a temp dir with a sample external skill.""" + ext_dir = tmp_path / "external-skills" + skill_dir = ext_dir / "my-external-skill" + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text( + "---\nname: my-external-skill\ndescription: A skill from an external directory\n---\n\n# My External Skill\n\nDo external things.\n" + ) + return ext_dir + + +@pytest.fixture +def hermes_home(tmp_path): + """Create a minimal HERMES_HOME with config.""" + home = tmp_path / ".hermes" + home.mkdir() + (home / "skills").mkdir() + return home + + +class TestGetExternalSkillsDirs: + def test_empty_config(self, hermes_home): + (hermes_home / "config.yaml").write_text("skills:\n external_dirs: []\n") + with patch.dict(os.environ, {"HERMES_HOME": str(hermes_home)}): + from agent.skill_utils import get_external_skills_dirs + result = get_external_skills_dirs() + assert result == [] + + def test_nonexistent_dir_skipped(self, hermes_home): + (hermes_home / "config.yaml").write_text( + "skills:\n external_dirs:\n - /nonexistent/path\n" + ) + with patch.dict(os.environ, {"HERMES_HOME": str(hermes_home)}): + from agent.skill_utils import get_external_skills_dirs + result = get_external_skills_dirs() + assert result == [] + + def test_valid_dir_returned(self, hermes_home, external_skills_dir): + (hermes_home / "config.yaml").write_text( + f"skills:\n external_dirs:\n - {external_skills_dir}\n" + ) + with patch.dict(os.environ, {"HERMES_HOME": str(hermes_home)}): + from agent.skill_utils import get_external_skills_dirs + result = get_external_skills_dirs() + assert len(result) == 1 + assert result[0] == external_skills_dir.resolve() + + def test_duplicate_dirs_deduplicated(self, hermes_home, external_skills_dir): + (hermes_home / "config.yaml").write_text( + f"skills:\n external_dirs:\n - {external_skills_dir}\n - {external_skills_dir}\n" + ) + with patch.dict(os.environ, {"HERMES_HOME": str(hermes_home)}): + from agent.skill_utils import get_external_skills_dirs + result = get_external_skills_dirs() + assert len(result) == 1 + + def test_local_skills_dir_excluded(self, hermes_home): + local_skills = hermes_home / "skills" + (hermes_home / "config.yaml").write_text( + f"skills:\n external_dirs:\n - {local_skills}\n" + ) + with patch.dict(os.environ, {"HERMES_HOME": str(hermes_home)}): + from agent.skill_utils import get_external_skills_dirs + result = get_external_skills_dirs() + assert result == [] + + def test_no_config_file(self, hermes_home): + # No config.yaml at all + with patch.dict(os.environ, {"HERMES_HOME": str(hermes_home)}): + from agent.skill_utils import get_external_skills_dirs + result = get_external_skills_dirs() + assert result == [] + + def test_string_value_converted_to_list(self, hermes_home, external_skills_dir): + (hermes_home / "config.yaml").write_text( + f"skills:\n external_dirs: {external_skills_dir}\n" + ) + with patch.dict(os.environ, {"HERMES_HOME": str(hermes_home)}): + from agent.skill_utils import get_external_skills_dirs + result = get_external_skills_dirs() + assert len(result) == 1 + + +class TestGetAllSkillsDirs: + def test_local_always_first(self, hermes_home, external_skills_dir): + (hermes_home / "config.yaml").write_text( + f"skills:\n external_dirs:\n - {external_skills_dir}\n" + ) + with patch.dict(os.environ, {"HERMES_HOME": str(hermes_home)}): + from agent.skill_utils import get_all_skills_dirs + result = get_all_skills_dirs() + assert result[0] == hermes_home / "skills" + assert result[1] == external_skills_dir.resolve() + + +class TestExternalSkillsInFindAll: + def test_external_skills_found(self, hermes_home, external_skills_dir): + (hermes_home / "config.yaml").write_text( + f"skills:\n external_dirs:\n - {external_skills_dir}\n" + ) + local_skills = hermes_home / "skills" + with ( + patch.dict(os.environ, {"HERMES_HOME": str(hermes_home)}), + patch("tools.skills_tool.SKILLS_DIR", local_skills), + ): + from tools.skills_tool import _find_all_skills + skills = _find_all_skills() + names = [s["name"] for s in skills] + assert "my-external-skill" in names + + def test_local_takes_precedence(self, hermes_home, external_skills_dir): + """If the same skill name exists locally and externally, local wins.""" + local_skills = hermes_home / "skills" + local_skill = local_skills / "my-external-skill" + local_skill.mkdir(parents=True) + (local_skill / "SKILL.md").write_text( + "---\nname: my-external-skill\ndescription: Local version\n---\n\nLocal.\n" + ) + (hermes_home / "config.yaml").write_text( + f"skills:\n external_dirs:\n - {external_skills_dir}\n" + ) + with ( + patch.dict(os.environ, {"HERMES_HOME": str(hermes_home)}), + patch("tools.skills_tool.SKILLS_DIR", local_skills), + ): + from tools.skills_tool import _find_all_skills + skills = _find_all_skills() + matching = [s for s in skills if s["name"] == "my-external-skill"] + assert len(matching) == 1 + assert matching[0]["description"] == "Local version" + + +class TestExternalSkillView: + def test_skill_view_finds_external(self, hermes_home, external_skills_dir): + (hermes_home / "config.yaml").write_text( + f"skills:\n external_dirs:\n - {external_skills_dir}\n" + ) + local_skills = hermes_home / "skills" + with ( + patch.dict(os.environ, {"HERMES_HOME": str(hermes_home)}), + patch("tools.skills_tool.SKILLS_DIR", local_skills), + ): + from tools.skills_tool import skill_view + result = json.loads(skill_view("my-external-skill")) + assert result["success"] is True + assert "external things" in result["content"] diff --git a/tests/agent/test_prompt_builder.py b/tests/agent/test_prompt_builder.py index a5b8be529c..eba85d0338 100644 --- a/tests/agent/test_prompt_builder.py +++ b/tests/agent/test_prompt_builder.py @@ -5,6 +5,8 @@ import importlib import logging import sys +import pytest + from agent.prompt_builder import ( _scan_context_content, _truncate_content, @@ -194,7 +196,7 @@ class TestParseSkillFile: ) from unittest.mock import patch - with patch("tools.skills_tool.sys") as mock_sys: + with patch("agent.skill_utils.sys") as mock_sys: mock_sys.platform = "linux" is_compat, _, _ = _parse_skill_file(skill_file) assert is_compat is False @@ -234,9 +236,6 @@ class TestPromptBuilderImports: # ========================================================================= -import pytest - - class TestBuildSkillsSystemPrompt: @pytest.fixture(autouse=True) def _clear_skills_cache(self): @@ -296,7 +295,7 @@ class TestBuildSkillsSystemPrompt: from unittest.mock import patch - with patch("tools.skills_tool.sys") as mock_sys: + with patch("agent.skill_utils.sys") as mock_sys: mock_sys.platform = "linux" result = build_skills_system_prompt() @@ -574,6 +573,10 @@ class TestBuildContextFilesPrompt: result = build_context_files_prompt(cwd=str(tmp_path)) assert "Lowercase claude rules" in result + @pytest.mark.skipif( + sys.platform == "darwin", + reason="APFS default volume is case-insensitive; CLAUDE.md and claude.md alias the same path", + ) def test_claude_md_uppercase_takes_priority(self, tmp_path): (tmp_path / "CLAUDE.md").write_text("From uppercase.") (tmp_path / "claude.md").write_text("From lowercase.") diff --git a/tests/agent/test_skill_commands.py b/tests/agent/test_skill_commands.py index 1f18d9bf4b..6b3e551e18 100644 --- a/tests/agent/test_skill_commands.py +++ b/tests/agent/test_skill_commands.py @@ -246,20 +246,10 @@ Generate some audio. def test_preserves_remaining_remote_setup_warning(self, tmp_path, monkeypatch): monkeypatch.setenv("TERMINAL_ENV", "ssh") monkeypatch.delenv("TENOR_API_KEY", raising=False) - - def fake_secret_callback(var_name, prompt, metadata=None): - os.environ[var_name] = "stored-in-test" - return { - "success": True, - "stored_as": var_name, - "validated": False, - "skipped": False, - } - monkeypatch.setattr( skills_tool_module, "_secret_capture_callback", - fake_secret_callback, + None, raising=False, ) diff --git a/tests/tools/test_credential_files.py b/tests/tools/test_credential_files.py new file mode 100644 index 0000000000..293e2c6da7 --- /dev/null +++ b/tests/tools/test_credential_files.py @@ -0,0 +1,158 @@ +"""Tests for credential file passthrough registry (tools/credential_files.py).""" + +import os +from pathlib import Path + +import pytest + +from tools.credential_files import ( + clear_credential_files, + get_credential_file_mounts, + register_credential_file, + register_credential_files, + reset_config_cache, +) + + +@pytest.fixture(autouse=True) +def _clean_registry(): + """Reset registry between tests.""" + clear_credential_files() + reset_config_cache() + yield + clear_credential_files() + reset_config_cache() + + +class TestRegisterCredentialFile: + def test_registers_existing_file(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + (tmp_path / "token.json").write_text('{"token": "abc"}') + + result = register_credential_file("token.json") + + assert result is True + mounts = get_credential_file_mounts() + assert len(mounts) == 1 + assert mounts[0]["host_path"] == str(tmp_path / "token.json") + assert mounts[0]["container_path"] == "/root/.hermes/token.json" + + def test_skips_missing_file(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + + result = register_credential_file("nonexistent.json") + + assert result is False + assert get_credential_file_mounts() == [] + + def test_custom_container_base(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + (tmp_path / "cred.json").write_text("{}") + + register_credential_file("cred.json", container_base="/home/user/.hermes") + + mounts = get_credential_file_mounts() + assert mounts[0]["container_path"] == "/home/user/.hermes/cred.json" + + def test_deduplicates_by_container_path(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + (tmp_path / "token.json").write_text("{}") + + register_credential_file("token.json") + register_credential_file("token.json") + + mounts = get_credential_file_mounts() + assert len(mounts) == 1 + + +class TestRegisterCredentialFiles: + def test_string_entries(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + (tmp_path / "a.json").write_text("{}") + (tmp_path / "b.json").write_text("{}") + + missing = register_credential_files(["a.json", "b.json"]) + + assert missing == [] + assert len(get_credential_file_mounts()) == 2 + + def test_dict_entries(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + (tmp_path / "token.json").write_text("{}") + + missing = register_credential_files([ + {"path": "token.json", "description": "OAuth token"}, + ]) + + assert missing == [] + assert len(get_credential_file_mounts()) == 1 + + def test_returns_missing_files(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + (tmp_path / "exists.json").write_text("{}") + + missing = register_credential_files([ + "exists.json", + "missing.json", + {"path": "also_missing.json"}, + ]) + + assert missing == ["missing.json", "also_missing.json"] + assert len(get_credential_file_mounts()) == 1 + + def test_empty_list(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + assert register_credential_files([]) == [] + + +class TestConfigCredentialFiles: + def test_loads_from_config(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + (tmp_path / "oauth.json").write_text("{}") + (tmp_path / "config.yaml").write_text( + "terminal:\n credential_files:\n - oauth.json\n" + ) + + mounts = get_credential_file_mounts() + + assert len(mounts) == 1 + assert mounts[0]["host_path"] == str(tmp_path / "oauth.json") + + def test_config_skips_missing_files(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + (tmp_path / "config.yaml").write_text( + "terminal:\n credential_files:\n - nonexistent.json\n" + ) + + mounts = get_credential_file_mounts() + assert mounts == [] + + def test_combines_skill_and_config(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + (tmp_path / "skill_token.json").write_text("{}") + (tmp_path / "config_token.json").write_text("{}") + (tmp_path / "config.yaml").write_text( + "terminal:\n credential_files:\n - config_token.json\n" + ) + + register_credential_file("skill_token.json") + mounts = get_credential_file_mounts() + + assert len(mounts) == 2 + paths = {m["container_path"] for m in mounts} + assert "/root/.hermes/skill_token.json" in paths + assert "/root/.hermes/config_token.json" in paths + + +class TestGetMountsRechecksExistence: + def test_removed_file_excluded_from_mounts(self, tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + token = tmp_path / "token.json" + token.write_text("{}") + + register_credential_file("token.json") + assert len(get_credential_file_mounts()) == 1 + + # Delete the file after registration + token.unlink() + assert get_credential_file_mounts() == [] diff --git a/tools/credential_files.py b/tools/credential_files.py new file mode 100644 index 0000000000..56c32d572a --- /dev/null +++ b/tools/credential_files.py @@ -0,0 +1,163 @@ +"""Credential file passthrough registry for remote terminal backends. + +Skills that declare ``required_credential_files`` in their frontmatter need +those files available inside sandboxed execution environments (Modal, Docker). +By default remote backends create bare containers with no host files. + +This module provides a session-scoped registry so skill-declared credential +files (and user-configured overrides) are mounted into remote sandboxes. + +Two sources feed the registry: + +1. **Skill declarations** — when a skill is loaded via ``skill_view``, its + ``required_credential_files`` entries are registered here if the files + exist on the host. +2. **User config** — ``terminal.credential_files`` in config.yaml lets users + explicitly list additional files to mount. + +Remote backends (``tools/environments/modal.py``, ``docker.py``) call +:func:`get_credential_file_mounts` at sandbox creation time. + +Each registered entry is a dict:: + + { + "host_path": "/home/user/.hermes/google_token.json", + "container_path": "/root/.hermes/google_token.json", + } +""" + +from __future__ import annotations + +import logging +import os +from pathlib import Path +from typing import Dict, List + +logger = logging.getLogger(__name__) + +# Session-scoped list of credential files to mount. +# Key: container_path (deduplicated), Value: host_path +_registered_files: Dict[str, str] = {} + +# Cache for config-based file list (loaded once per process). +_config_files: List[Dict[str, str]] | None = None + + +def _resolve_hermes_home() -> Path: + return Path(os.environ.get("HERMES_HOME", Path.home() / ".hermes")) + + +def register_credential_file( + relative_path: str, + container_base: str = "/root/.hermes", +) -> bool: + """Register a credential file for mounting into remote sandboxes. + + *relative_path* is relative to ``HERMES_HOME`` (e.g. ``google_token.json``). + Returns True if the file exists on the host and was registered. + """ + hermes_home = _resolve_hermes_home() + host_path = hermes_home / relative_path + if not host_path.is_file(): + logger.debug("credential_files: skipping %s (not found)", host_path) + return False + + container_path = f"{container_base.rstrip('/')}/{relative_path}" + _registered_files[container_path] = str(host_path) + logger.debug("credential_files: registered %s -> %s", host_path, container_path) + return True + + +def register_credential_files( + entries: list, + container_base: str = "/root/.hermes", +) -> List[str]: + """Register multiple credential files from skill frontmatter entries. + + Each entry is either a string (relative path) or a dict with a ``path`` + key. Returns the list of relative paths that were NOT found on the host + (i.e. missing files). + """ + missing = [] + for entry in entries: + if isinstance(entry, str): + rel_path = entry.strip() + elif isinstance(entry, dict): + rel_path = (entry.get("path") or "").strip() + else: + continue + if not rel_path: + continue + if not register_credential_file(rel_path, container_base): + missing.append(rel_path) + return missing + + +def _load_config_files() -> List[Dict[str, str]]: + """Load ``terminal.credential_files`` from config.yaml (cached).""" + global _config_files + if _config_files is not None: + return _config_files + + result: List[Dict[str, str]] = [] + try: + hermes_home = _resolve_hermes_home() + config_path = hermes_home / "config.yaml" + if config_path.exists(): + import yaml + + with open(config_path) as f: + cfg = yaml.safe_load(f) or {} + cred_files = cfg.get("terminal", {}).get("credential_files") + if isinstance(cred_files, list): + for item in cred_files: + if isinstance(item, str) and item.strip(): + host_path = hermes_home / item.strip() + if host_path.is_file(): + container_path = f"/root/.hermes/{item.strip()}" + result.append({ + "host_path": str(host_path), + "container_path": container_path, + }) + except Exception as e: + logger.debug("Could not read terminal.credential_files from config: %s", e) + + _config_files = result + return _config_files + + +def get_credential_file_mounts() -> List[Dict[str, str]]: + """Return all credential files that should be mounted into remote sandboxes. + + Each item has ``host_path`` and ``container_path`` keys. + Combines skill-registered files and user config. + """ + mounts: Dict[str, str] = {} + + # Skill-registered files + for container_path, host_path in _registered_files.items(): + # Re-check existence (file may have been deleted since registration) + if Path(host_path).is_file(): + mounts[container_path] = host_path + + # Config-based files + for entry in _load_config_files(): + cp = entry["container_path"] + if cp not in mounts and Path(entry["host_path"]).is_file(): + mounts[cp] = entry["host_path"] + + return [ + {"host_path": hp, "container_path": cp} + for cp, hp in mounts.items() + ] + + +def clear_credential_files() -> None: + """Reset the skill-scoped registry (e.g. on session reset).""" + _registered_files.clear() + + +def reset_config_cache() -> None: + """Force re-read of config on next access (for testing).""" + global _config_files + _config_files = None diff --git a/tools/environments/docker.py b/tools/environments/docker.py index c5546dbe4b..a24786d17e 100644 --- a/tools/environments/docker.py +++ b/tools/environments/docker.py @@ -312,6 +312,24 @@ class DockerEnvironment(BaseEnvironment): elif workspace_explicitly_mounted: logger.debug("Skipping docker cwd mount: /workspace already mounted by user config") + # Mount credential files (OAuth tokens, etc.) declared by skills. + # Read-only so the container can authenticate but not modify host creds. + try: + from tools.credential_files import get_credential_file_mounts + + for mount_entry in get_credential_file_mounts(): + volume_args.extend([ + "-v", + f"{mount_entry['host_path']}:{mount_entry['container_path']}:ro", + ]) + logger.info( + "Docker: mounting credential %s -> %s", + mount_entry["host_path"], + mount_entry["container_path"], + ) + except Exception as e: + logger.debug("Docker: could not load credential file mounts: %s", e) + logger.info(f"Docker volume_args: {volume_args}") all_run_args = list(_SECURITY_ARGS) + writable_args + resource_args + volume_args logger.info(f"Docker run_args: {all_run_args}") @@ -406,8 +424,17 @@ class DockerEnvironment(BaseEnvironment): if effective_stdin is not None: cmd.append("-i") cmd.extend(["-w", work_dir]) - hermes_env = _load_hermes_env_vars() if self._forward_env else {} - for key in self._forward_env: + # Combine explicit docker_forward_env with skill-declared env_passthrough + # vars so skills that declare required_environment_variables (e.g. Notion) + # have their keys forwarded into the container automatically. + forward_keys = set(self._forward_env) + try: + from tools.env_passthrough import get_all_passthrough + forward_keys |= get_all_passthrough() + except Exception: + pass + hermes_env = _load_hermes_env_vars() if forward_keys else {} + for key in sorted(forward_keys): value = os.getenv(key) if value is None: value = hermes_env.get(key) diff --git a/tools/environments/modal.py b/tools/environments/modal.py index c12ba8b1bd..2842bed114 100644 --- a/tools/environments/modal.py +++ b/tools/environments/modal.py @@ -137,6 +137,28 @@ class ModalEnvironment(BaseEnvironment): ], ) + # Mount credential files (OAuth tokens, etc.) declared by skills. + # These are read-only copies so the sandbox can authenticate with + # external services but can't modify the host's credentials. + cred_mounts = [] + try: + from tools.credential_files import get_credential_file_mounts + + for mount_entry in get_credential_file_mounts(): + cred_mounts.append( + _modal.Mount.from_local_file( + mount_entry["host_path"], + remote_path=mount_entry["container_path"], + ) + ) + logger.info( + "Modal: mounting credential %s -> %s", + mount_entry["host_path"], + mount_entry["container_path"], + ) + except Exception as e: + logger.debug("Modal: could not load credential file mounts: %s", e) + # Start the async worker thread and create sandbox on it # so all gRPC channels are bound to the worker's event loop. self._worker.start() @@ -145,23 +167,90 @@ class ModalEnvironment(BaseEnvironment): app = await _modal.App.lookup.aio( "hermes-agent", create_if_missing=True ) + create_kwargs = dict(sandbox_kwargs) + if cred_mounts: + existing_mounts = list(create_kwargs.pop("mounts", [])) + existing_mounts.extend(cred_mounts) + create_kwargs["mounts"] = existing_mounts sandbox = await _modal.Sandbox.create.aio( "sleep", "infinity", image=effective_image, app=app, - timeout=int(sandbox_kwargs.pop("timeout", 3600)), - **sandbox_kwargs, + timeout=int(create_kwargs.pop("timeout", 3600)), + **create_kwargs, ) return app, sandbox self._app, self._sandbox = self._worker.run_coroutine( _create_sandbox(), timeout=300 ) + # Track synced credential files to avoid redundant pushes. + # Key: container_path, Value: (mtime, size) of last synced version. + self._synced_creds: Dict[str, tuple] = {} logger.info("Modal: sandbox created (task=%s)", self._task_id) + def _sync_credential_files(self) -> None: + """Push credential files into the running sandbox. + + Mounts are set at sandbox creation, but credentials may be created + later (e.g. OAuth setup mid-session). This writes the current file + content into the sandbox via exec(), so new/updated credentials are + available without recreating the sandbox. + """ + try: + from tools.credential_files import get_credential_file_mounts + + mounts = get_credential_file_mounts() + if not mounts: + return + + for entry in mounts: + host_path = entry["host_path"] + container_path = entry["container_path"] + hp = Path(host_path) + try: + stat = hp.stat() + file_key = (stat.st_mtime, stat.st_size) + except OSError: + continue + + # Skip if already synced with same mtime+size + if self._synced_creds.get(container_path) == file_key: + continue + + try: + content = hp.read_text(encoding="utf-8") + except Exception: + continue + + # Write via base64 to avoid shell escaping issues with JSON + import base64 + b64 = base64.b64encode(content.encode("utf-8")).decode("ascii") + container_dir = str(Path(container_path).parent) + cmd = ( + f"mkdir -p {shlex.quote(container_dir)} && " + f"echo {shlex.quote(b64)} | base64 -d > {shlex.quote(container_path)}" + ) + + _cp = container_path # capture for closure + + async def _write(): + proc = await self._sandbox.exec.aio("bash", "-c", cmd) + await proc.wait.aio() + + self._worker.run_coroutine(_write(), timeout=15) + self._synced_creds[container_path] = file_key + logger.debug("Modal: synced credential %s -> %s", host_path, container_path) + except Exception as e: + logger.debug("Modal: credential file sync failed: %s", e) + def execute(self, command: str, cwd: str = "", *, timeout: int | None = None, stdin_data: str | None = None) -> dict: + # Sync credential files before each command so mid-session + # OAuth setups are picked up without requiring a restart. + self._sync_credential_files() + if stdin_data is not None: marker = f"HERMES_EOF_{uuid.uuid4().hex[:8]}" while marker in stdin_data: diff --git a/tools/skills_tool.py b/tools/skills_tool.py index 964b2bef4a..61e045f0d8 100644 --- a/tools/skills_tool.py +++ b/tools/skills_tool.py @@ -494,7 +494,7 @@ def _is_skill_disabled(name: str, platform: str = None) -> bool: def _find_all_skills(*, skip_disabled: bool = False) -> List[Dict[str, Any]]: - """Recursively find all skills in ~/.hermes/skills/. + """Recursively find all skills in ~/.hermes/skills/ and external dirs. Args: skip_disabled: If True, return ALL skills regardless of disabled @@ -504,59 +504,68 @@ def _find_all_skills(*, skip_disabled: bool = False) -> List[Dict[str, Any]]: Returns: List of skill metadata dicts (name, description, category). """ - skills = [] + from agent.skill_utils import get_external_skills_dirs - if not SKILLS_DIR.exists(): - return skills + skills = [] + seen_names: set = set() # Load disabled set once (not per-skill) disabled = set() if skip_disabled else _get_disabled_skill_names() + # Scan local dir first, then external dirs (local takes precedence) + dirs_to_scan = [] + if SKILLS_DIR.exists(): + dirs_to_scan.append(SKILLS_DIR) + dirs_to_scan.extend(get_external_skills_dirs()) - for skill_md in SKILLS_DIR.rglob("SKILL.md"): - if any(part in _EXCLUDED_SKILL_DIRS for part in skill_md.parts): - continue - - skill_dir = skill_md.parent - - try: - content = skill_md.read_text(encoding="utf-8")[:4000] - frontmatter, body = _parse_frontmatter(content) - - if not skill_matches_platform(frontmatter): + for scan_dir in dirs_to_scan: + for skill_md in scan_dir.rglob("SKILL.md"): + if any(part in _EXCLUDED_SKILL_DIRS for part in skill_md.parts): continue - name = frontmatter.get("name", skill_dir.name)[:MAX_NAME_LENGTH] - if name in disabled: + skill_dir = skill_md.parent + + try: + content = skill_md.read_text(encoding="utf-8")[:4000] + frontmatter, body = _parse_frontmatter(content) + + if not skill_matches_platform(frontmatter): + continue + + name = frontmatter.get("name", skill_dir.name)[:MAX_NAME_LENGTH] + if name in seen_names: + continue + if name in disabled: + continue + + description = frontmatter.get("description", "") + if not description: + for line in body.strip().split("\n"): + line = line.strip() + if line and not line.startswith("#"): + description = line + break + + if len(description) > MAX_DESCRIPTION_LENGTH: + description = description[:MAX_DESCRIPTION_LENGTH - 3] + "..." + + category = _get_category_from_path(skill_md) + + seen_names.add(name) + skills.append({ + "name": name, + "description": description, + "category": category, + }) + + except (UnicodeDecodeError, PermissionError) as e: + logger.debug("Failed to read skill file %s: %s", skill_md, e) + continue + except Exception as e: + logger.debug( + "Skipping skill at %s: failed to parse: %s", skill_md, e, exc_info=True + ) continue - - description = frontmatter.get("description", "") - if not description: - for line in body.strip().split("\n"): - line = line.strip() - if line and not line.startswith("#"): - description = line - break - - if len(description) > MAX_DESCRIPTION_LENGTH: - description = description[:MAX_DESCRIPTION_LENGTH - 3] + "..." - - category = _get_category_from_path(skill_md) - - skills.append({ - "name": name, - "description": description, - "category": category, - }) - - except (UnicodeDecodeError, PermissionError) as e: - logger.debug("Failed to read skill file %s: %s", skill_md, e) - continue - except Exception as e: - logger.debug( - "Skipping skill at %s: failed to parse: %s", skill_md, e, exc_info=True - ) - continue return skills @@ -756,7 +765,15 @@ def skill_view(name: str, file_path: str = None, task_id: str = None) -> str: JSON string with skill content or error message """ try: - if not SKILLS_DIR.exists(): + from agent.skill_utils import get_external_skills_dirs + + # Build list of all skill directories to search + all_dirs = [] + if SKILLS_DIR.exists(): + all_dirs.append(SKILLS_DIR) + all_dirs.extend(get_external_skills_dirs()) + + if not all_dirs: return json.dumps( { "success": False, @@ -768,27 +785,37 @@ def skill_view(name: str, file_path: str = None, task_id: str = None) -> str: skill_dir = None skill_md = None - # Try direct path first (e.g., "mlops/axolotl") - direct_path = SKILLS_DIR / name - if direct_path.is_dir() and (direct_path / "SKILL.md").exists(): - skill_dir = direct_path - skill_md = direct_path / "SKILL.md" - elif direct_path.with_suffix(".md").exists(): - skill_md = direct_path.with_suffix(".md") + # Search all dirs: local first, then external (first match wins) + for search_dir in all_dirs: + # Try direct path first (e.g., "mlops/axolotl") + direct_path = search_dir / name + if direct_path.is_dir() and (direct_path / "SKILL.md").exists(): + skill_dir = direct_path + skill_md = direct_path / "SKILL.md" + break + elif direct_path.with_suffix(".md").exists(): + skill_md = direct_path.with_suffix(".md") + break - # Search by directory name + # Search by directory name across all dirs if not skill_md: - for found_skill_md in SKILLS_DIR.rglob("SKILL.md"): - if found_skill_md.parent.name == name: - skill_dir = found_skill_md.parent - skill_md = found_skill_md + for search_dir in all_dirs: + for found_skill_md in search_dir.rglob("SKILL.md"): + if found_skill_md.parent.name == name: + skill_dir = found_skill_md.parent + skill_md = found_skill_md + break + if skill_md: break # Legacy: flat .md files if not skill_md: - for found_md in SKILLS_DIR.rglob(f"{name}.md"): - if found_md.name != "SKILL.md": - skill_md = found_md + for search_dir in all_dirs: + for found_md in search_dir.rglob(f"{name}.md"): + if found_md.name != "SKILL.md": + skill_md = found_md + break + if skill_md: break if not skill_md or not skill_md.exists(): @@ -815,12 +842,21 @@ def skill_view(name: str, file_path: str = None, task_id: str = None) -> str: ensure_ascii=False, ) - # Security: warn if skill is loaded from outside the trusted skills directory + # Security: warn if skill is loaded from outside trusted directories + # (local skills dir + configured external_dirs are all trusted) + _outside_skills_dir = True + _trusted_dirs = [SKILLS_DIR.resolve()] try: - skill_md.resolve().relative_to(SKILLS_DIR.resolve()) - _outside_skills_dir = False - except ValueError: - _outside_skills_dir = True + _trusted_dirs.extend(d.resolve() for d in all_dirs[1:]) + except Exception: + pass + for _td in _trusted_dirs: + try: + skill_md.resolve().relative_to(_td) + _outside_skills_dir = False + break + except ValueError: + continue # Security: detect common prompt injection patterns _INJECTION_PATTERNS = [ @@ -1058,7 +1094,11 @@ def skill_view(name: str, file_path: str = None, task_id: str = None) -> str: if script_files: linked_files["scripts"] = script_files - rel_path = str(skill_md.relative_to(SKILLS_DIR)) + try: + rel_path = str(skill_md.relative_to(SKILLS_DIR)) + except ValueError: + # External skill — use path relative to the skill's own parent dir + rel_path = str(skill_md.relative_to(skill_md.parent.parent)) if skill_md.parent.parent else skill_md.name skill_name = frontmatter.get( "name", skill_md.stem if not skill_dir else skill_dir.name ) @@ -1106,6 +1146,27 @@ def skill_view(name: str, file_path: str = None, task_id: str = None) -> str: exc_info=True, ) + # Register credential files for mounting into remote sandboxes + # (Modal, Docker). Files that exist on the host are registered; + # missing ones are added to the setup_needed indicators. + required_cred_files_raw = frontmatter.get("required_credential_files", []) + if not isinstance(required_cred_files_raw, list): + required_cred_files_raw = [] + missing_cred_files: list = [] + if required_cred_files_raw: + try: + from tools.credential_files import register_credential_files + + missing_cred_files = register_credential_files(required_cred_files_raw) + if missing_cred_files: + setup_needed = True + except Exception: + logger.debug( + "Could not register credential files for skill %s", + skill_name, + exc_info=True, + ) + result = { "success": True, "name": skill_name, @@ -1121,6 +1182,7 @@ def skill_view(name: str, file_path: str = None, task_id: str = None) -> str: "required_environment_variables": required_env_vars, "required_commands": [], "missing_required_environment_variables": remaining_missing_required_envs, + "missing_credential_files": missing_cred_files, "missing_required_commands": [], "setup_needed": setup_needed, "setup_skipped": capture_result["setup_skipped"], @@ -1139,6 +1201,8 @@ def skill_view(name: str, file_path: str = None, task_id: str = None) -> str: if setup_needed: missing_items = [ f"env ${env_name}" for env_name in remaining_missing_required_envs + ] + [ + f"file {path}" for path in missing_cred_files ] setup_note = _build_setup_note( SkillReadinessStatus.SETUP_NEEDED, diff --git a/website/docs/developer-guide/contributing.md b/website/docs/developer-guide/contributing.md index 1d1e24c62d..603b416ac5 100644 --- a/website/docs/developer-guide/contributing.md +++ b/website/docs/developer-guide/contributing.md @@ -90,6 +90,7 @@ pytest tests/ -v - **Comments**: Only when explaining non-obvious intent, trade-offs, or API quirks - **Error handling**: Catch specific exceptions. Use `logger.warning()`/`logger.error()` with `exc_info=True` for unexpected errors - **Cross-platform**: Never assume Unix (see below) +- **Profile-safe paths**: Never hardcode `~/.hermes` — use `get_hermes_home()` from `hermes_constants` for code paths and `display_hermes_home()` for user-facing messages. See [AGENTS.md](https://github.com/NousResearch/hermes-agent/blob/main/AGENTS.md#profiles-multi-instance-support) for full rules. ## Cross-Platform Compatibility diff --git a/website/docs/developer-guide/creating-skills.md b/website/docs/developer-guide/creating-skills.md index f2238d7d56..e5660b61f9 100644 --- a/website/docs/developer-guide/creating-skills.md +++ b/website/docs/developer-guide/creating-skills.md @@ -168,11 +168,38 @@ required_environment_variables: The user can skip setup and keep loading the skill. Hermes never exposes the raw secret value to the model. Gateway and messaging sessions show local setup guidance instead of collecting secrets in-band. :::tip Sandbox Passthrough -When your skill is loaded, any declared `required_environment_variables` that are set are **automatically passed through** to `execute_code` and `terminal` sandboxes. Your skill's scripts can access `$TENOR_API_KEY` (or `os.environ["TENOR_API_KEY"]` in Python) without the user needing to configure anything extra. See [Environment Variable Passthrough](/docs/user-guide/security#environment-variable-passthrough) for details. +When your skill is loaded, any declared `required_environment_variables` that are set are **automatically passed through** to `execute_code` and `terminal` sandboxes — including remote backends like Docker and Modal. Your skill's scripts can access `$TENOR_API_KEY` (or `os.environ["TENOR_API_KEY"]` in Python) without the user needing to configure anything extra. See [Environment Variable Passthrough](/docs/user-guide/security#environment-variable-passthrough) for details. ::: Legacy `prerequisites.env_vars` remains supported as a backward-compatible alias. +### Credential File Requirements (OAuth tokens, etc.) + +Skills that use OAuth or file-based credentials can declare files that need to be mounted into remote sandboxes. This is for credentials stored as **files** (not env vars) — typically OAuth token files produced by a setup script. + +```yaml +required_credential_files: + - path: google_token.json + description: Google OAuth2 token (created by setup script) + - path: google_client_secret.json + description: Google OAuth2 client credentials +``` + +Each entry supports: +- `path` (required) — file path relative to `~/.hermes/` +- `description` (optional) — explains what the file is and how it's created + +When loaded, Hermes checks if these files exist. Missing files trigger `setup_needed`. Existing files are automatically: +- **Mounted into Docker** containers as read-only bind mounts +- **Synced into Modal** sandboxes (at creation + before each command, so mid-session OAuth works) +- Available on **local** backend without any special handling + +:::tip When to use which +Use `required_environment_variables` for simple API keys and tokens (strings stored in `~/.hermes/.env`). Use `required_credential_files` for OAuth token files, client secrets, service account JSON, certificates, or any credential that's a file on disk. +::: + +See the `skills/productivity/google-workspace/SKILL.md` for a complete example using both. + ## Skill Guidelines ### No External Dependencies diff --git a/website/docs/reference/environment-variables.md b/website/docs/reference/environment-variables.md index 493412e107..b60b05982a 100644 --- a/website/docs/reference/environment-variables.md +++ b/website/docs/reference/environment-variables.md @@ -105,7 +105,7 @@ For native Anthropic auth, Hermes prefers Claude Code's own credential files whe |----------|-------------| | `TERMINAL_ENV` | Backend: `local`, `docker`, `ssh`, `singularity`, `modal`, `daytona` | | `TERMINAL_DOCKER_IMAGE` | Docker image (default: `python:3.11`) | -| `TERMINAL_DOCKER_FORWARD_ENV` | JSON array of env var names to explicitly forward into Docker terminal sessions | +| `TERMINAL_DOCKER_FORWARD_ENV` | JSON array of env var names to explicitly forward into Docker terminal sessions. Note: skill-declared `required_environment_variables` are forwarded automatically — you only need this for vars not declared by any skill. | | `TERMINAL_DOCKER_VOLUMES` | Additional Docker volume mounts (comma-separated `host:container` pairs) | | `TERMINAL_DOCKER_MOUNT_CWD_TO_WORKSPACE` | Advanced opt-in: mount the launch cwd into Docker `/workspace` (`true`/`false`, default: `false`) | | `TERMINAL_SINGULARITY_IMAGE` | Singularity image or `.sif` path | diff --git a/website/docs/reference/skills-catalog.md b/website/docs/reference/skills-catalog.md index 8fb22f3974..4a1ecf6295 100644 --- a/website/docs/reference/skills-catalog.md +++ b/website/docs/reference/skills-catalog.md @@ -253,7 +253,7 @@ Skills for academic research, paper discovery, literature review, domain reconna | `arxiv` | Search and retrieve academic papers from arXiv using their free REST API. No API key needed. Search by keyword, author, category, or ID. Combine with web_extract or the ocr-and-documents skill to read full paper content. | `research/arxiv` | | `blogwatcher` | Monitor blogs and RSS/Atom feeds for updates using the blogwatcher CLI. Add blogs, scan for new articles, and track what you've read. | `research/blogwatcher` | | `domain-intel` | Passive domain reconnaissance using Python stdlib. Subdomain discovery, SSL certificate inspection, WHOIS lookups, DNS records, domain availability checks, and bulk multi-domain analysis. No API keys required. | `research/domain-intel` | -| `duckduckgo-search` | Free web search via DuckDuckGo — text, news, images, videos. No API key needed. Use the Python DDGS library or CLI to search, then web_extract for full content. | `research/duckduckgo-search` | +| `duckduckgo-search` | Free web search via DuckDuckGo — text, news, images, videos. No API key needed. Prefer the `ddgs` CLI when installed; use the Python DDGS library only after verifying that `ddgs` is available in the current runtime. | `research/duckduckgo-search` | | `ml-paper-writing` | Write publication-ready ML/AI papers for NeurIPS, ICML, ICLR, ACL, AAAI, COLM. Use when drafting papers from research repos, structuring arguments, verifying citations, or preparing camera-ready submissions. Includes LaTeX templates, reviewer guidelines, and citation verificatio… | `research/ml-paper-writing` | | `polymarket` | Query Polymarket prediction market data — search markets, get prices, orderbooks, and price history. Read-only via public REST APIs, no API key needed. | `research/polymarket` | diff --git a/website/docs/user-guide/features/skills.md b/website/docs/user-guide/features/skills.md index edd61f7396..3d166b9782 100644 --- a/website/docs/user-guide/features/skills.md +++ b/website/docs/user-guide/features/skills.md @@ -8,7 +8,9 @@ description: "On-demand knowledge documents — progressive disclosure, agent-ma Skills are on-demand knowledge documents the agent can load when needed. They follow a **progressive disclosure** pattern to minimize token usage and are compatible with the [agentskills.io](https://agentskills.io/specification) open standard. -All skills live in **`~/.hermes/skills/`** — a single directory that serves as the source of truth. On fresh install, bundled skills are copied from the repo. Hub-installed and agent-created skills also go here. The agent can modify or delete any skill. +All skills live in **`~/.hermes/skills/`** — the primary directory and source of truth. On fresh install, bundled skills are copied from the repo. Hub-installed and agent-created skills also go here. The agent can modify or delete any skill. + +You can also point Hermes at **external skill directories** — additional folders scanned alongside the local one. See [External Skill Directories](#external-skill-directories) below. See also: @@ -164,6 +166,47 @@ Once set, declared env vars are **automatically passed through** to `execute_cod └── .bundled_manifest # Tracks seeded bundled skills ``` +## External Skill Directories + +If you maintain skills outside of Hermes — for example, a shared `~/.agents/skills/` directory used by multiple AI tools — you can tell Hermes to scan those directories too. + +Add `external_dirs` under the `skills` section in `~/.hermes/config.yaml`: + +```yaml +skills: + external_dirs: + - ~/.agents/skills + - /home/shared/team-skills + - ${SKILLS_REPO}/skills +``` + +Paths support `~` expansion and `${VAR}` environment variable substitution. + +### How it works + +- **Read-only**: External dirs are only scanned for skill discovery. When the agent creates or edits a skill, it always writes to `~/.hermes/skills/`. +- **Local precedence**: If the same skill name exists in both the local dir and an external dir, the local version wins. +- **Full integration**: External skills appear in the system prompt index, `skills_list`, `skill_view`, and as `/skill-name` slash commands — no different from local skills. +- **Non-existent paths are silently skipped**: If a configured directory doesn't exist, Hermes ignores it without errors. Useful for optional shared directories that may not be present on every machine. + +### Example + +```text +~/.hermes/skills/ # Local (primary, read-write) +├── devops/deploy-k8s/ +│ └── SKILL.md +└── mlops/axolotl/ + └── SKILL.md + +~/.agents/skills/ # External (read-only, shared) +├── my-custom-workflow/ +│ └── SKILL.md +└── team-conventions/ + └── SKILL.md +``` + +All four skills appear in your skill index. If you create a new skill called `my-custom-workflow` locally, it shadows the external version. + ## Agent-Managed Skills (skill_manage tool) The agent can create, update, and delete its own skills via the `skill_manage` tool. This is the agent's **procedural memory** — when it figures out a non-trivial workflow, it saves the approach as a skill for future reuse. diff --git a/website/docs/user-guide/security.md b/website/docs/user-guide/security.md index 7c59a4aba0..4d51161e1f 100644 --- a/website/docs/user-guide/security.md +++ b/website/docs/user-guide/security.md @@ -278,7 +278,11 @@ required_environment_variables: help: Get a key from https://developers.google.com/tenor ``` -After loading this skill, `TENOR_API_KEY` passes through to both `execute_code` and `terminal` subprocesses — no manual configuration needed. +After loading this skill, `TENOR_API_KEY` passes through to `execute_code`, `terminal` (local), **and remote backends (Docker, Modal)** — no manual configuration needed. + +:::info Docker & Modal +Prior to v0.5.1, Docker's `forward_env` was a separate system from the skill passthrough. They are now merged — skill-declared env vars are automatically forwarded into Docker containers and Modal sandboxes without needing to add them to `docker_forward_env` manually. +::: **2. Config-based passthrough (manual)** @@ -291,17 +295,49 @@ terminal: - ANOTHER_TOKEN ``` +### Credential File Passthrough (OAuth tokens, etc.) {#credential-file-passthrough} + +Some skills need **files** (not just env vars) in the sandbox — for example, Google Workspace stores OAuth tokens as `google_token.json` in `~/.hermes/`. Skills declare these in frontmatter: + +```yaml +required_credential_files: + - path: google_token.json + description: Google OAuth2 token (created by setup script) + - path: google_client_secret.json + description: Google OAuth2 client credentials +``` + +When loaded, Hermes checks if these files exist in `~/.hermes/` and registers them for mounting: + +- **Docker**: Read-only bind mounts (`-v host:container:ro`) +- **Modal**: Mounted at sandbox creation + synced before each command (handles mid-session OAuth setup) +- **Local**: No action needed (files already accessible) + +You can also list credential files manually in `config.yaml`: + +```yaml +terminal: + credential_files: + - google_token.json + - my_custom_oauth_token.json +``` + +Paths are relative to `~/.hermes/`. Files are mounted to `/root/.hermes/` inside the container. + ### What Each Sandbox Filters | Sandbox | Default Filter | Passthrough Override | |---------|---------------|---------------------| | **execute_code** | Blocks vars containing `KEY`, `TOKEN`, `SECRET`, `PASSWORD`, `CREDENTIAL`, `PASSWD`, `AUTH` in name; only allows safe-prefix vars through | ✅ Passthrough vars bypass both checks | | **terminal** (local) | Blocks explicit Hermes infrastructure vars (provider keys, gateway tokens, tool API keys) | ✅ Passthrough vars bypass the blocklist | +| **terminal** (Docker) | No host env vars by default | ✅ Passthrough vars + `docker_forward_env` forwarded via `-e` | +| **terminal** (Modal) | No host env/files by default | ✅ Credential files mounted; env passthrough via sync | | **MCP** | Blocks everything except safe system vars + explicitly configured `env` | ❌ Not affected by passthrough (use MCP `env` config instead) | ### Security Considerations - The passthrough only affects vars you or your skills explicitly declare — the default security posture is unchanged for arbitrary LLM-generated code +- Credential files are mounted **read-only** into Docker containers - Skills Guard scans skill content for suspicious env access patterns before installation - Missing/unset vars are never registered (you can't leak what doesn't exist) - Hermes infrastructure secrets (provider API keys, gateway tokens) should never be added to `env_passthrough` — they have dedicated mechanisms