From ff078738ea0108548fc9c147140942fbeab7c833 Mon Sep 17 00:00:00 2001 From: wysie Date: Mon, 18 May 2026 12:39:50 +0800 Subject: [PATCH 01/30] fix(skills): load symlinked skill slash commands --- agent/skill_commands.py | 26 ++++++++++++++++++++++++-- tests/agent/test_skill_commands.py | 26 ++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/agent/skill_commands.py b/agent/skill_commands.py index 42e7c85743..018d84865c 100644 --- a/agent/skill_commands.py +++ b/agent/skill_commands.py @@ -58,13 +58,35 @@ def _load_skill_payload(skill_identifier: str, task_id: str | None = None) -> tu try: from tools.skills_tool import SKILLS_DIR, skill_view + from agent.skill_utils import get_external_skills_dirs identifier_path = Path(raw_identifier).expanduser() if identifier_path.is_absolute(): + normalized = None + trusted_roots = [SKILLS_DIR] try: - normalized = str(identifier_path.resolve().relative_to(SKILLS_DIR.resolve())) + trusted_roots.extend(get_external_skills_dirs()) except Exception: - normalized = raw_identifier + pass + + # Prefer the lexical path under a trusted skill root before + # resolving symlinks. Slash-command discovery can legitimately + # find a skill via ~/.hermes/skills/ where is a + # symlink to a checked-out skill elsewhere. Resolving first turns + # that trusted visible path into an arbitrary absolute path that + # skill_view() refuses to load. + for root in trusted_roots: + try: + normalized = str(identifier_path.relative_to(root)) + break + except ValueError: + continue + + if normalized is None: + try: + normalized = str(identifier_path.resolve().relative_to(SKILLS_DIR.resolve())) + except Exception: + normalized = raw_identifier else: normalized = raw_identifier.lstrip("/") diff --git a/tests/agent/test_skill_commands.py b/tests/agent/test_skill_commands.py index c11976ef97..a206348c0d 100644 --- a/tests/agent/test_skill_commands.py +++ b/tests/agent/test_skill_commands.py @@ -4,6 +4,8 @@ import os from pathlib import Path from unittest.mock import patch +import pytest + import tools.skills_tool as skills_tool_module from agent.skill_commands import ( build_preloaded_skills_prompt, @@ -125,6 +127,30 @@ class TestScanSkillCommands: assert "/knowledge-brain" in result assert result["/knowledge-brain"]["name"] == "knowledge-brain" + def test_loads_skill_invocation_from_symlinked_skill_dir(self, tmp_path): + """Slash commands should load skills symlinked under the local skills dir.""" + external_root = tmp_path / "external" + skills_root = tmp_path / "skills" + skills_root.mkdir() + real_skill_dir = _make_skill( + external_root, + "impeccable", + body="Apply impeccable design craft.", + ) + symlink_path = skills_root / "impeccable" + try: + symlink_path.symlink_to(real_skill_dir, target_is_directory=True) + except (OSError, NotImplementedError) as exc: + pytest.skip(f"symlinks unavailable in test environment: {exc}") + + with patch("tools.skills_tool.SKILLS_DIR", skills_root): + result = scan_skill_commands() + message = build_skill_invocation_message("/impeccable") + + assert "/impeccable" in result + assert message is not None + assert "Apply impeccable design craft." in message + def test_get_skill_commands_rescans_when_platform_scope_changes(self, tmp_path): """Platform-specific disabled-skill caches must not leak across platforms. From 94c523f0c5c8f717c5294f9048d02dee2774b469 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Mon, 18 May 2026 00:36:17 -0700 Subject: [PATCH 02/30] docs(session_search): update all docs for the single-shape rewrite (#27840) Companion PR to #27590. Sweeps remaining stale references to the LLM-summary path that landed in main with #27590 but weren't fully caught in the followup cleanup commit. Real rewrites: - user-guide/sessions.md: 'Session Search Tool' section rewritten to describe the three calling shapes (discovery / scroll / browse) with worked examples. Adds the 'Optional parameters' subsection covering sort and role_filter. - user-guide/features/memory.md: 'Session Search' overview rewritten, comparison table updated (speed: ms instead of LLM summarization, added explicit free-cost row, link to sessions.md for details). Stale-claim sweeps: - user-guide/configuring-models.md: drop the 'Session Search' row from the aux-model override table (no aux model anymore), drop session search from the auxiliary-models list. - user-guide/features/codex-app-server-runtime.md: drop session_search from the ChatGPT-subscription cost note, drop the session_search block from the per-task override config example. - developer-guide/provider-runtime.md: drop 'session search summarization' from the auxiliary tasks list. - developer-guide/agent-loop.md: drop session search from the auxiliary fallback chain list. - user-guide/skills/.../autonomous-ai-agents-hermes-agent.md: drop session_search from the 'auxiliary models not working' debug step. Untouched (still accurate as tool-name mentions, not behavioral claims): - features/tools.md, features/honcho.md, features/acp.md - cli.md, sessions.md (other sections) - developer-guide/tools-runtime.md, agent-loop.md (line 157) - acp-internals.md, adding-tools.md, prompt-assembly.md - reference/toolsets-reference.md, reference/tools-reference.md --- website/docs/developer-guide/agent-loop.md | 2 +- .../docs/developer-guide/provider-runtime.md | 1 - website/docs/user-guide/configuring-models.md | 3 +- .../features/codex-app-server-runtime.md | 5 +- website/docs/user-guide/features/memory.md | 8 ++- website/docs/user-guide/sessions.md | 57 ++++++++++++++++--- .../autonomous-ai-agents-hermes-agent.md | 2 +- 7 files changed, 59 insertions(+), 19 deletions(-) diff --git a/website/docs/developer-guide/agent-loop.md b/website/docs/developer-guide/agent-loop.md index cf9cb1c1ef..fdc0cc3c8f 100644 --- a/website/docs/developer-guide/agent-loop.md +++ b/website/docs/developer-guide/agent-loop.md @@ -194,7 +194,7 @@ When the primary model fails (429 rate limit, 5xx server error, 401/403 auth err 3. On success, continue the conversation with the new provider 4. On 401/403, attempt credential refresh before failing over -The fallback system also covers auxiliary tasks independently — vision, compression, web extraction, and session search each have their own fallback chain configurable via the `auxiliary.*` config section. +The fallback system also covers auxiliary tasks independently — vision, compression, and web extraction each have their own fallback chain configurable via the `auxiliary.*` config section. ## Compression and Persistence diff --git a/website/docs/developer-guide/provider-runtime.md b/website/docs/developer-guide/provider-runtime.md index 830382479f..67c86b01c2 100644 --- a/website/docs/developer-guide/provider-runtime.md +++ b/website/docs/developer-guide/provider-runtime.md @@ -150,7 +150,6 @@ Auxiliary tasks such as: - vision - web extraction summarization - context compression summaries -- session search summarization - skills hub operations - MCP helper operations - memory flushes diff --git a/website/docs/user-guide/configuring-models.md b/website/docs/user-guide/configuring-models.md index 4c12fa7e7d..a4ce79eea3 100644 --- a/website/docs/user-guide/configuring-models.md +++ b/website/docs/user-guide/configuring-models.md @@ -7,7 +7,7 @@ sidebar_position: 3 Hermes uses two kinds of model slots: - **Main model** — what the agent thinks with. Every user message, every tool-call loop, every streamed response goes through this model. -- **Auxiliary models** — smaller side-jobs the agent offloads. Context compression, vision (image analysis), web-page summarization, session search, approval scoring, MCP tool routing, session-title generation, and skill search. Each has its own slot and can be overridden independently. +- **Auxiliary models** — smaller side-jobs the agent offloads. Context compression, vision (image analysis), web-page summarization, approval scoring, MCP tool routing, session-title generation, and skill search. Each has its own slot and can be overridden independently. This page covers configuring both from the dashboard. If you prefer config files or the CLI, jump to [Alternative methods](#alternative-methods) at the bottom. @@ -52,7 +52,6 @@ Every auxiliary task defaults to `auto` — meaning Hermes uses your main model | **Title Gen** | Almost always. A $0.10/M flash model writes session titles as well as Opus. Default config sets this to `google/gemini-3-flash-preview` on OpenRouter. | | **Vision** | When your main model is a coding model without vision (e.g. Kimi, DeepSeek). Point it at `google/gemini-2.5-flash` or `gpt-4o-mini`. | | **Compression** | When you're burning reasoning tokens on Opus/M2.7 just to summarize context. A fast chat model does the job at 1/50th the cost. | -| **Session Search** | When recall queries fan out — default max_concurrency is 3. A cheap model keeps the bill predictable. | | **Approval** | For `approval_mode: smart` — a fast/cheap model (haiku, flash, gpt-5-mini) decides whether to auto-approve low-risk commands. Expensive models here are waste. | | **Web Extract** | When you use `web_extract` heavily. Same logic as compression — summarization doesn't need reasoning. | | **Skills Hub** | `hermes skills search` uses this. Usually fine at `auto`. | diff --git a/website/docs/user-guide/features/codex-app-server-runtime.md b/website/docs/user-guide/features/codex-app-server-runtime.md index 575250d9b0..130e790f06 100644 --- a/website/docs/user-guide/features/codex-app-server-runtime.md +++ b/website/docs/user-guide/features/codex-app-server-runtime.md @@ -242,7 +242,7 @@ default_permissions = ":read-only" ## Auxiliary tasks and ChatGPT subscription token cost -When this runtime is on with the `openai-codex` provider, **auxiliary tasks (title generation, context compression, vision auto-detect, session search summarization, the background self-improvement review fork) also flow through your ChatGPT subscription by default**, because Hermes' auxiliary client uses the main provider/model when no per-task override is set. +When this runtime is on with the `openai-codex` provider, **auxiliary tasks (title generation, context compression, vision auto-detect, the background self-improvement review fork) also flow through your ChatGPT subscription by default**, because Hermes' auxiliary client uses the main provider/model when no per-task override is set. This isn't specific to `codex_app_server` — it's true for the existing `codex_responses` path too — but it's more visible here because you're explicitly opting in for the subscription billing. @@ -259,9 +259,6 @@ auxiliary: vision_detect: provider: openrouter model: google/gemini-3-flash-preview - session_search: - provider: openrouter - model: google/gemini-3-flash-preview goal_judge: provider: openrouter model: google/gemini-3-flash-preview diff --git a/website/docs/user-guide/features/memory.md b/website/docs/user-guide/features/memory.md index 77f74d28a8..5c07df6357 100644 --- a/website/docs/user-guide/features/memory.md +++ b/website/docs/user-guide/features/memory.md @@ -177,19 +177,23 @@ Memory entries are scanned for injection and exfiltration patterns before being Beyond MEMORY.md and USER.md, the agent can search its past conversations using the `session_search` tool: - All CLI and messaging sessions are stored in SQLite (`~/.hermes/state.db`) with FTS5 full-text search -- Search queries return relevant past conversations with Gemini Flash summarization +- Search queries return actual messages from the DB — no LLM summarization, no truncation - The agent can find things it discussed weeks ago, even if they're not in its active memory +- The agent can also scroll forward/backward inside any session it finds ```bash hermes sessions list # Browse past sessions ``` +See [Session Search Tool](/docs/user-guide/sessions#session-search-tool) for the three calling shapes (discovery / scroll / browse) and the response format. + ### session_search vs memory | Feature | Persistent Memory | Session Search | |---------|------------------|----------------| | **Capacity** | ~1,300 tokens total | Unlimited (all sessions) | -| **Speed** | Instant (in system prompt) | Requires search + LLM summarization | +| **Speed** | Instant (in system prompt) | ~20ms FTS5 query, ~1ms scroll | +| **Cost** | Token cost in every prompt | Free — no LLM calls | | **Use case** | Key facts always available | Finding specific past conversations | | **Management** | Manually curated by agent | Automatic — all sessions stored | | **Token cost** | Fixed per session (~1,300 tokens) | On-demand (searched when needed) | diff --git a/website/docs/user-guide/sessions.md b/website/docs/user-guide/sessions.md index e90c3f60bc..2a663bf5ac 100644 --- a/website/docs/user-guide/sessions.md +++ b/website/docs/user-guide/sessions.md @@ -366,25 +366,66 @@ For deeper analytics — token usage, cost estimates, tool breakdown, and activi ## Session Search Tool -The agent has a built-in `session_search` tool that performs full-text search across all past conversations using SQLite's FTS5 engine. +The agent has a built-in `session_search` tool that performs full-text search across all past conversations using SQLite's FTS5 engine — and lets the agent scroll through any session it finds. No LLM calls, no summarization, no truncation. Every shape returns actual messages from the DB. -### How It Works +### Three calling shapes -1. FTS5 searches matching messages ranked by relevance -2. Groups results by session, takes the top N unique sessions (default 3) -3. Loads each session's conversation, truncates to ~100K chars centered on matches -4. Sends to a fast summarization model for focused summaries -5. Returns per-session summaries with metadata and surrounding context +The tool infers what you want from which arguments you set. There's no `mode` parameter. + +**1. Discovery — pass `query`:** + +```python +session_search(query="auth refactor", limit=3) +``` + +Runs FTS5, dedupes hits by session lineage, returns the top N sessions. Each result carries: + +- `session_id`, `title`, `when`, `source` +- `snippet` — FTS5-highlighted match excerpt +- `bookend_start` — first 3 user+assistant messages of the session (the goal/kickoff) +- `messages` — ±5 messages around the FTS5 match, with the anchor message flagged (the hit in context) +- `bookend_end` — last 3 user+assistant messages of the session (the resolution/decisions) +- `match_message_id`, `messages_before`, `messages_after` + +Bookends + window together reconstruct goal → match → resolution without paying for the whole transcript. Typical wall time: 15–50ms on a real session DB. + +**2. Scroll — pass `session_id` + `around_message_id`:** + +```python +session_search(session_id="20260510_174648_805cc2", around_message_id=590803, window=10) +``` + +Returns a window of ±`window` messages centered on the anchor. No FTS5, no bookends — just the slice. Use after a discovery call when you need more context than the ±5 default window. + +- To scroll **forward**: pass `messages[-1].id` back as `around_message_id` +- To scroll **backward**: pass `messages[0].id` back as `around_message_id` +- The boundary message appears in both windows as an orientation marker +- When `messages_before` or `messages_after` is less than `window`, you're at the start or end of the session + +Typical wall time: 1–2ms per scroll call. + +**3. Browse — no args:** + +```python +session_search() +``` + +Returns recent sessions chronologically (titles, previews, timestamps). Useful when the user asks "what was I working on" without naming a topic. ### FTS5 Query Syntax The search supports standard FTS5 query syntax: -- Simple keywords: `docker deployment` +- Simple keywords: `docker deployment` (FTS5 defaults to AND) - Phrases: `"exact phrase"` - Boolean: `docker OR kubernetes`, `python NOT java` - Prefix: `deploy*` +### Optional parameters + +- `sort` — `newest` or `oldest`, on top of FTS5 ranking. Omit for relevance-only ordering (the default; suitable for exploratory recall). Use `newest` for "where did we leave X" questions, `oldest` for "how did X start" questions. +- `role_filter` — comma-separated roles to include. Discovery defaults to `user,assistant` (tool output is usually noise). Pass `user,assistant,tool` to include tool output (debugging tool behaviour) or `tool` to search tool output only. + ### When It's Used The agent is prompted to use session search automatically: diff --git a/website/docs/user-guide/skills/bundled/autonomous-ai-agents/autonomous-ai-agents-hermes-agent.md b/website/docs/user-guide/skills/bundled/autonomous-ai-agents/autonomous-ai-agents-hermes-agent.md index 5f2c8d16a2..ec0a4a9250 100644 --- a/website/docs/user-guide/skills/bundled/autonomous-ai-agents/autonomous-ai-agents-hermes-agent.md +++ b/website/docs/user-guide/skills/bundled/autonomous-ai-agents/autonomous-ai-agents-hermes-agent.md @@ -853,7 +853,7 @@ Common gateway problems: - **Windows-specific issues** (`Alt+Enter` newline, WinError 10106, UTF-8 BOM config, test suite, line endings): see the dedicated **Windows-Specific Quirks** section above. ### Auxiliary models not working -If `auxiliary` tasks (vision, compression, session_search) fail silently, the `auto` provider can't find a backend. Either set `OPENROUTER_API_KEY` or `GOOGLE_API_KEY`, or explicitly configure each auxiliary task's provider: +If `auxiliary` tasks (vision, compression) fail silently, the `auto` provider can't find a backend. Either set `OPENROUTER_API_KEY` or `GOOGLE_API_KEY`, or explicitly configure each auxiliary task's provider: ```bash hermes config set auxiliary.vision.provider hermes config set auxiliary.vision.model From 41f1eddee30a01a7b3dd2c2efad6f0e3dca681aa Mon Sep 17 00:00:00 2001 From: kshitij <82637225+kshitijk4poor@users.noreply.github.com> Date: Mon, 18 May 2026 00:45:25 -0700 Subject: [PATCH 03/30] refactor(doctor): extract section banner + fail-and-issue helpers (#27830) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `hermes_cli/doctor.py` had two recurring patterns: 1. **15 section headers** of the form `print() ; print(color("◆ Name", Colors.CYAN, Colors.BOLD))` bracketed by 3-line `# =====` / `# Check: X` / `# =====` comment banners. 2. **Paired `check_fail(...) ; issues.append(...)`** for every diagnostic that emits both a user-visible failure and an auto-fix instruction. Add two helpers and collapse the patterns: def _section(title): print() print(color(f"◆ {title}", Colors.CYAN, Colors.BOLD)) def _fail_and_issue(text, detail, fix, issues): check_fail(text, detail) issues.append(fix) Replacements: - 15 `# =====/# X/# =====` banner triples + section header pairs compressed to `_section(...)` - All 18 `check_fail + issues.append` pairs collapsed to `_fail_and_issue(...)` (single-line where the call fits under 120 chars, multi-line where it doesn't) - Net -5 LOC (`+128 / -133`) The LOC delta is modest after wrapping long calls onto multi-line form for readability — the real win is uniform call shape and removal of two parallel-pattern footguns. There is now exactly one way to emit a diagnostic that pairs a user-visible failure with a fix instruction. Behavior is byte-identical. `_section` produces the same blank line + bold-cyan output the inline two prints did, and `_fail_and_issue` does the same `check_fail + issues.append` sequence in the same order. Verified empirically by diffing live `run_doctor()` stdout from this branch against `origin/main` — `diff -q` reports zero differences. Test plan: - All 69 tests across test_doctor.py, test_doctor_command_install.py, and test_doctor_dedicated_provider_skip.py pass - `ruff check hermes_cli/doctor.py` clean - Live `run_doctor()` output byte-identical to origin/main Refs #23972 (Phase 2 tracker — dedup-only refactor in line with the "net-LOC-negative" discipline). --- hermes_cli/doctor.py | 261 +++++++++++++++++++++---------------------- 1 file changed, 128 insertions(+), 133 deletions(-) diff --git a/hermes_cli/doctor.py b/hermes_cli/doctor.py index 87043bc261..4440b38682 100644 --- a/hermes_cli/doctor.py +++ b/hermes_cli/doctor.py @@ -195,6 +195,18 @@ def check_info(text: str): print(f" {color('→', Colors.CYAN)} {text}") +def _section(title: str) -> None: + """Print a doctor section banner: blank line + bold cyan ◆ title.""" + print() + print(color(f"◆ {title}", Colors.CYAN, Colors.BOLD)) + + +def _fail_and_issue(text: str, detail: str, fix: str, issues: list[str]) -> None: + """Emit a check_fail and append the corresponding fix instruction.""" + check_fail(text, detail) + issues.append(fix) + + def _check_gateway_service_linger(issues: list[str]) -> None: """Warn when a systemd user gateway service will stop after logout.""" try: @@ -214,9 +226,7 @@ def _check_gateway_service_linger(issues: list[str]) -> None: if not unit_path.exists(): return - print() - print(color("◆ Gateway Service", Colors.CYAN, Colors.BOLD)) - + _section("Gateway Service") linger_enabled, linger_detail = get_systemd_linger_status() if linger_enabled is True: check_ok("Systemd linger enabled", "(gateway service survives logout)") @@ -373,11 +383,7 @@ def run_doctor(args): print(color("│ 🩺 Hermes Doctor │", Colors.CYAN)) print(color("└─────────────────────────────────────────────────────────┘", Colors.CYAN)) - # ========================================================================= - # Check: Security advisories (RUNS FIRST — these are the most urgent) - # ========================================================================= - print() - print(color("◆ Security Advisories", Colors.CYAN, Colors.BOLD)) + _section("Security Advisories") try: from hermes_cli.security_advisories import ( detect_compromised, @@ -423,12 +429,7 @@ def run_doctor(args): # Never let a bug in the advisory check block the rest of doctor. check_warn(f"Security advisory check failed: {e}") - # ========================================================================= - # Check: Python version - # ========================================================================= - print() - print(color("◆ Python Environment", Colors.CYAN, Colors.BOLD)) - + _section("Python Environment") py_version = sys.version_info if py_version >= (3, 11): check_ok(f"Python {py_version.major}.{py_version.minor}.{py_version.micro}") @@ -438,8 +439,12 @@ def run_doctor(args): elif py_version >= (3, 8): check_warn(f"Python {py_version.major}.{py_version.minor}.{py_version.micro}", "(3.10+ recommended)") else: - check_fail(f"Python {py_version.major}.{py_version.minor}.{py_version.micro}", "(3.10+ required)") - issues.append("Upgrade Python to 3.10+") + _fail_and_issue( + f"Python {py_version.major}.{py_version.minor}.{py_version.micro}", + "(3.10+ required)", + "Upgrade Python to 3.10+", + issues, + ) # Check if in virtual environment in_venv = sys.prefix != sys.base_prefix @@ -448,12 +453,7 @@ def run_doctor(args): else: check_warn("Not in virtual environment", "(recommended)") - # ========================================================================= - # Check: Required packages - # ========================================================================= - print() - print(color("◆ Required Packages", Colors.CYAN, Colors.BOLD)) - + _section("Required Packages") required_packages = [ ("openai", "OpenAI SDK"), ("rich", "Rich (terminal UI)"), @@ -473,8 +473,7 @@ def run_doctor(args): __import__(module) check_ok(name) except ImportError: - check_fail(name, "(missing)") - issues.append(f"Install {name}: {_python_install_cmd()} {module}") + _fail_and_issue(name, "(missing)", f"Install {name}: {_python_install_cmd()} {module}", issues) for module, name in optional_packages: try: @@ -483,12 +482,7 @@ def run_doctor(args): except ImportError: check_warn(name, "(optional, not installed)") - # ========================================================================= - # Check: Configuration files - # ========================================================================= - print() - print(color("◆ Configuration Files", Colors.CYAN, Colors.BOLD)) - + _section("Configuration Files") # Check ~/.hermes/.env (primary location for user config) env_path = HERMES_HOME / '.env' if env_path.exists(): @@ -611,14 +605,15 @@ def run_doctor(args): and not (provider_ids_to_accept & valid_provider_ids) ): known_list = ", ".join(sorted(known_providers)) if known_providers else "(unavailable)" - check_fail( + _fail_and_issue( f"model.provider '{provider_raw}' is not a recognised provider", f"(known: {known_list})", - ) - issues.append( - f"model.provider '{provider_raw}' is unknown. " - f"Valid providers: {known_list}. " - f"Fix: run 'hermes config set model.provider '" + ( + f"model.provider '{provider_raw}' is unknown. " + f"Valid providers: {known_list}. " + f"Fix: run 'hermes config set model.provider '" + ), + issues, ) # Warn if model is set to a provider-prefixed name on a provider that doesn't use them @@ -677,14 +672,15 @@ def run_doctor(args): or status.get("api_key") ) if not configured: - check_fail( + _fail_and_issue( f"model.provider '{runtime_provider}' is set but no API key is configured", "(check ~/.hermes/.env or run 'hermes setup')", - ) - issues.append( - f"No credentials found for provider '{runtime_provider}'. " - f"Run 'hermes setup' or set the provider's API key in {_DHH}/.env, " - f"or switch providers with 'hermes config set model.provider '" + ( + f"No credentials found for provider '{runtime_provider}'. " + f"Run 'hermes setup' or set the provider's API key in {_DHH}/.env, " + f"or switch providers with 'hermes config set model.provider '" + ), + issues, ) except Exception: pass @@ -768,8 +764,7 @@ def run_doctor(args): from hermes_cli.config import validate_config_structure config_issues = validate_config_structure() if config_issues: - print() - print(color("◆ Config Structure", Colors.CYAN, Colors.BOLD)) + _section("Config Structure") for ci in config_issues: if ci.severity == "error": check_fail(ci.message) @@ -782,12 +777,7 @@ def run_doctor(args): except Exception: pass - # ========================================================================= - # Check: Auth providers - # ========================================================================= - print() - print(color("◆ Auth Providers", Colors.CYAN, Colors.BOLD)) - + _section("Auth Providers") try: from hermes_cli.auth import ( get_nous_auth_status, @@ -859,12 +849,7 @@ def run_doctor(args): "(optional — only required to import tokens from an existing Codex CLI login)" ) - # ========================================================================= - # Check: Directory structure - # ========================================================================= - print() - print(color("◆ Directory Structure", Colors.CYAN, Colors.BOLD)) - + _section("Directory Structure") hermes_home = HERMES_HOME if hermes_home.exists(): check_ok(f"{_DHH} directory exists") @@ -976,13 +961,8 @@ def run_doctor(args): _check_gateway_service_linger(issues) - # ========================================================================= - # Check: Command installation (hermes bin symlink) - # ========================================================================= if sys.platform != "win32": - print() - print(color("◆ Command Installation", Colors.CYAN, Colors.BOLD)) - + _section("Command Installation") # Determine the venv entry point location _venv_bin = None for _venv_name in ("venv", ".venv"): @@ -1056,12 +1036,7 @@ def run_doctor(args): else: issues.append(f"Missing {_cmd_link_display}/hermes symlink — run 'hermes doctor --fix'") - # ========================================================================= - # Check: External tools - # ========================================================================= - print() - print(color("◆ External Tools", Colors.CYAN, Colors.BOLD)) - + _section("External Tools") # Git if _safe_which("git"): check_ok("git") @@ -1087,11 +1062,14 @@ def run_doctor(args): if result is not None and result.returncode == 0: check_ok("docker", "(daemon running)") else: - check_fail("docker daemon not running") - issues.append("Start Docker daemon") + _fail_and_issue("docker daemon not running", "", "Start Docker daemon", issues) else: - check_fail("docker not found", "(required for TERMINAL_ENV=docker)") - issues.append("Install Docker or change TERMINAL_ENV") + _fail_and_issue( + "docker not found", + "(required for TERMINAL_ENV=docker)", + "Install Docker or change TERMINAL_ENV", + issues, + ) elif _safe_which("docker"): check_ok("docker", "(optional)") elif _is_termux(): @@ -1126,11 +1104,14 @@ def run_doctor(args): if result is not None and result.returncode == 0: check_ok(f"SSH connection to {ssh_host}") else: - check_fail(f"SSH connection to {ssh_host}") - issues.append(f"Check SSH configuration for {ssh_host}") + _fail_and_issue(f"SSH connection to {ssh_host}", "", f"Check SSH configuration for {ssh_host}", issues) else: - check_fail("TERMINAL_SSH_HOST not set", "(required for TERMINAL_ENV=ssh)") - issues.append("Set TERMINAL_SSH_HOST in .env") + _fail_and_issue( + "TERMINAL_SSH_HOST not set", + "(required for TERMINAL_ENV=ssh)", + "Set TERMINAL_SSH_HOST in .env", + issues, + ) # Daytona (if using daytona backend) if terminal_env == "daytona": @@ -1138,14 +1119,22 @@ def run_doctor(args): if daytona_key: check_ok("Daytona API key", "(configured)") else: - check_fail("DAYTONA_API_KEY not set", "(required for TERMINAL_ENV=daytona)") - issues.append("Set DAYTONA_API_KEY environment variable") + _fail_and_issue( + "DAYTONA_API_KEY not set", + "(required for TERMINAL_ENV=daytona)", + "Set DAYTONA_API_KEY environment variable", + issues, + ) try: from daytona import Daytona # noqa: F401 — SDK presence check check_ok("daytona SDK", "(installed)") except ImportError: - check_fail("daytona SDK not installed", "(pip install daytona)") - issues.append("Install daytona SDK: pip install daytona") + _fail_and_issue( + "daytona SDK not installed", + "(pip install daytona)", + "Install daytona SDK: pip install daytona", + issues, + ) # Vercel Sandbox (if using vercel_sandbox backend) if terminal_env == "vercel_sandbox": @@ -1155,32 +1144,50 @@ def run_doctor(args): check_ok("Vercel runtime", f"({runtime})") else: supported = ", ".join(_SUPPORTED_VERCEL_RUNTIMES) - check_fail("Vercel runtime unsupported", f"({runtime}; use {supported})") - issues.append(f"Set TERMINAL_VERCEL_RUNTIME to one of: {supported}") + _fail_and_issue( + "Vercel runtime unsupported", + f"({runtime}; use {supported})", + f"Set TERMINAL_VERCEL_RUNTIME to one of: {supported}", + issues, + ) disk = os.getenv("TERMINAL_CONTAINER_DISK", "51200").strip() if disk in {"", "0", "51200"}: check_ok("Vercel disk setting", "(uses platform default)") else: - check_fail("Vercel custom disk unsupported", "(reset terminal.container_disk to 51200)") - issues.append("Vercel Sandbox does not support custom container_disk; use the shared default 51200") + _fail_and_issue( + "Vercel custom disk unsupported", + "(reset terminal.container_disk to 51200)", + "Vercel Sandbox does not support custom container_disk; use the shared default 51200", + issues, + ) if importlib.util.find_spec("vercel") is not None: check_ok("vercel SDK", "(installed)") else: - check_fail("vercel SDK not installed", "(pip install 'hermes-agent[vercel]')") - issues.append("Install the Vercel optional dependency: pip install 'hermes-agent[vercel]'") + _fail_and_issue( + "vercel SDK not installed", + "(pip install 'hermes-agent[vercel]')", + "Install the Vercel optional dependency: pip install 'hermes-agent[vercel]'", + issues, + ) auth_status = describe_vercel_auth() if auth_status.ok: check_ok("Vercel auth", f"({auth_status.label})") elif auth_status.label.startswith("partial"): - check_fail("Vercel auth incomplete", f"({auth_status.label})") - issues.append("Set VERCEL_TOKEN, VERCEL_PROJECT_ID, and VERCEL_TEAM_ID together") + _fail_and_issue( + "Vercel auth incomplete", + f"({auth_status.label})", + "Set VERCEL_TOKEN, VERCEL_PROJECT_ID, and VERCEL_TEAM_ID together", + issues, + ) else: - check_fail("Vercel auth not configured", f"({auth_status.label})") - issues.append( - "Configure Vercel Sandbox auth with VERCEL_TOKEN, VERCEL_PROJECT_ID, and VERCEL_TEAM_ID" + _fail_and_issue( + "Vercel auth not configured", + f"({auth_status.label})", + "Configure Vercel Sandbox auth with VERCEL_TOKEN, VERCEL_PROJECT_ID, and VERCEL_TEAM_ID", + issues, ) for line in auth_status.detail_lines: check_info(f"Vercel auth {line}") @@ -1320,12 +1327,7 @@ def run_doctor(args): for note in _termux_install_all_fallback_notes(): check_info(note) - # ========================================================================= - # Check: API connectivity - # ========================================================================= - print() - print(color("◆ API Connectivity", Colors.CYAN, Colors.BOLD)) - + _section("API Connectivity") # Refactor: every connectivity probe below is HTTP-bound and fully # independent. Running them in series spent ~5s wall on a typical # workstation (2s of that was boto3's IMDS lookup for AWS credentials, @@ -1673,12 +1675,7 @@ def run_doctor(args): for _issue in _issues_to_add: issues.append(_issue) - # ========================================================================= - # Check: Tool Availability - # ========================================================================= - print() - print(color("◆ Tool Availability", Colors.CYAN, Colors.BOLD)) - + _section("Tool Availability") try: # Add project root to path for imports sys.path.insert(0, str(PROJECT_ROOT)) @@ -1706,12 +1703,7 @@ def run_doctor(args): except Exception as e: check_warn("Could not check tool availability", f"({e})") - # ========================================================================= - # Check: Skills Hub - # ========================================================================= - print() - print(color("◆ Skills Hub", Colors.CYAN, Colors.BOLD)) - + _section("Skills Hub") hub_dir = HERMES_HOME / "skills" / ".hub" if hub_dir.exists(): check_ok("Skills Hub directory exists") @@ -1752,12 +1744,7 @@ def run_doctor(args): else: check_warn("No GITHUB_TOKEN", f"(60 req/hr rate limit — set in {_DHH}/.env for better rates)") - # ========================================================================= - # Memory Provider (only check the active provider, if any) - # ========================================================================= - print() - print(color("◆ Memory Provider", Colors.CYAN, Colors.BOLD)) - + _section("Memory Provider") _active_memory_provider = "" try: import yaml as _yaml @@ -1782,8 +1769,12 @@ def run_doctor(args): elif not hcfg.enabled: check_info(f"Honcho disabled (set enabled: true in {_honcho_cfg_path} to activate)") elif not (hcfg.api_key or hcfg.base_url): - check_fail("Honcho API key or base URL not set", "run: hermes memory setup") - issues.append("No Honcho API key — run 'hermes memory setup'") + _fail_and_issue( + "Honcho API key or base URL not set", + "run: hermes memory setup", + "No Honcho API key — run 'hermes memory setup'", + issues, + ) else: from plugins.memory.honcho.client import get_honcho_client, reset_honcho_client reset_honcho_client() @@ -1794,11 +1785,14 @@ def run_doctor(args): f"workspace={hcfg.workspace_id} mode={hcfg.recall_mode} freq={hcfg.write_frequency}", ) except Exception as _e: - check_fail("Honcho connection failed", str(_e)) - issues.append(f"Honcho unreachable: {_e}") + _fail_and_issue("Honcho connection failed", str(_e), f"Honcho unreachable: {_e}", issues) except ImportError: - check_fail("honcho-ai not installed", "pip install honcho-ai") - issues.append("Honcho is set as memory provider but honcho-ai is not installed") + _fail_and_issue( + "honcho-ai not installed", + "pip install honcho-ai", + "Honcho is set as memory provider but honcho-ai is not installed", + issues, + ) except Exception as _e: check_warn("Honcho check failed", str(_e)) elif _active_memory_provider == "mem0": @@ -1810,11 +1804,19 @@ def run_doctor(args): check_ok("Mem0 API key configured") check_info(f"user_id={mem0_cfg.get('user_id', '?')} agent_id={mem0_cfg.get('agent_id', '?')}") else: - check_fail("Mem0 API key not set", "(set MEM0_API_KEY in .env or run hermes memory setup)") - issues.append("Mem0 is set as memory provider but API key is missing") + _fail_and_issue( + "Mem0 API key not set", + "(set MEM0_API_KEY in .env or run hermes memory setup)", + "Mem0 is set as memory provider but API key is missing", + issues, + ) except ImportError: - check_fail("Mem0 plugin not loadable", "pip install mem0ai") - issues.append("Mem0 is set as memory provider but mem0ai is not installed") + _fail_and_issue( + "Mem0 plugin not loadable", + "pip install mem0ai", + "Mem0 is set as memory provider but mem0ai is not installed", + issues, + ) except Exception as _e: check_warn("Mem0 check failed", str(_e)) else: @@ -1831,17 +1833,13 @@ def run_doctor(args): except Exception as _e: check_warn(f"{_active_memory_provider} check failed", str(_e)) - # ========================================================================= - # Profiles - # ========================================================================= try: from hermes_cli.profiles import list_profiles, _get_wrapper_dir, profile_exists import re as _re named_profiles = [p for p in list_profiles() if not p.is_default] if named_profiles: - print() - print(color("◆ Profiles", Colors.CYAN, Colors.BOLD)) + _section("Profiles") check_ok(f"{len(named_profiles)} profile(s) found") wrapper_dir = _get_wrapper_dir() for p in named_profiles: @@ -1878,9 +1876,6 @@ def run_doctor(args): except Exception: pass - # ========================================================================= - # Summary - # ========================================================================= print() remaining_issues = issues + manual_issues if should_fix and fixed_count > 0: From 0fa46c613b364e435ea8a8ea6c3cb31c1a01ab50 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Mon, 18 May 2026 01:19:16 -0700 Subject: [PATCH 04/30] fix(yuanbao): persist message_id on @bot user transcript writes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Yuanbao's QuoteContextMiddleware has a transcript-lookup fallback for when quote.desc is empty: it scans the session transcript for the quoted message_id and pulls ybres anchors out of its content. That fallback works for observed (silent) group messages because the platform writer attaches message_id (yuanbao.py:2091). It silently fails for @bot agent-processed messages because gateway/run.py wrote them as {role:user, content, timestamp} with no message_id, so quoting an earlier @bot turn that contained an image/file couldn't be resolved. Fix: attach event.message_id to the user transcript entry at all three write sites in gateway/run.py — the agent_failed_early branch, the no-new-messages edge case, and the normal agent path (first user-role entry in new_messages). Surfaces gap reported in #27425 (loongfay) using the existing fallback already on main; no new caches needed. Co-authored-by: loongfay --- gateway/run.py | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/gateway/run.py b/gateway/run.py index 818bd282dd..623d238af3 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -8072,9 +8072,12 @@ class GatewayRunner: # message so the next message can load a transcript that # reflects what was said. Skip the assistant error text since # it's a gateway-generated hint, not model output. (#7100) + _user_entry = {"role": "user", "content": message_text, "timestamp": ts} + if event.message_id: + _user_entry["message_id"] = str(event.message_id) self.session_store.append_to_transcript( session_entry.session_id, - {"role": "user", "content": message_text, "timestamp": ts}, + _user_entry, ) else: history_len = agent_result.get("history_offset", len(history)) @@ -8082,9 +8085,12 @@ class GatewayRunner: # If no new messages found (edge case), fall back to simple user/assistant if not new_messages: + _user_entry = {"role": "user", "content": message_text, "timestamp": ts} + if event.message_id: + _user_entry["message_id"] = str(event.message_id) self.session_store.append_to_transcript( session_entry.session_id, - {"role": "user", "content": message_text, "timestamp": ts} + _user_entry, ) if response: self.session_store.append_to_transcript( @@ -8097,12 +8103,25 @@ class GatewayRunner: # to prevent the duplicate-write bug (#860). We still write # to JSONL for backward compatibility and as a backup. agent_persisted = self._session_db is not None + # Attach the inbound platform message_id to the first user + # entry written this turn so platform-level quote-resolution + # (e.g. Yuanbao QuoteContextMiddleware's transcript fallback) + # can find earlier @bot messages by their original message_id. + _user_msg_id_attached = False for msg in new_messages: # Skip system messages (they're rebuilt each run) if msg.get("role") == "system": continue # Add timestamp to each message for debugging entry = {**msg, "timestamp": ts} + if ( + not _user_msg_id_attached + and msg.get("role") == "user" + and event.message_id + and "message_id" not in entry + ): + entry["message_id"] = str(event.message_id) + _user_msg_id_attached = True self.session_store.append_to_transcript( session_entry.session_id, entry, skip_db=agent_persisted, From 060ec02858eb9e441da234476a2356708605fcc4 Mon Sep 17 00:00:00 2001 From: HenkDz Date: Fri, 15 May 2026 19:32:31 +0100 Subject: [PATCH 05/30] docs: add ACP Zed edit approval diffs plan --- .../2026-05-15-acp-zed-edit-approval-diffs.md | 152 ++++++++++++++++++ 1 file changed, 152 insertions(+) create mode 100644 docs/plans/2026-05-15-acp-zed-edit-approval-diffs.md diff --git a/docs/plans/2026-05-15-acp-zed-edit-approval-diffs.md b/docs/plans/2026-05-15-acp-zed-edit-approval-diffs.md new file mode 100644 index 0000000000..4946291d4b --- /dev/null +++ b/docs/plans/2026-05-15-acp-zed-edit-approval-diffs.md @@ -0,0 +1,152 @@ +# ACP Zed Pre-Edit Approval Diffs Implementation Plan + +> **For Hermes:** Use subagent-driven-development skill to implement this plan task-by-task. + +**Goal:** Gate file mutations in ACP/Zed behind explicit pre-edit approval with a structured diff, similar to Codex/Kimi edit review behavior. + +**Architecture:** Hermes already renders edit diffs after tools run. This PR adds a pre-mutation permission gate for file mutation tools. Intercept `write_file`, `patch`, and eventually `skill_manage` before they mutate disk; compute proposed old/new content; send ACP `session/request_permission` with `kind="edit"` and diff content; only execute the mutation after approval. Rejections return a clear tool result and leave files unchanged. + +**Tech Stack:** Python, ACP `request_permission`, `FileEditToolCallContent` / `acp.tool_diff_content`, Hermes file tools, pytest with temp files. + +--- + +### Task 1: Confirm current ACP diff/permission schema + +Run: + +```bash +/home/nour/.hermes/hermes-agent/venv/bin/python - <<'PY' +from acp.schema import RequestPermissionRequest, ToolCallUpdate +import acp, inspect +print(RequestPermissionRequest.model_fields) +print(ToolCallUpdate.model_fields) +print(inspect.signature(acp.tool_diff_content)) +PY +``` + +Record actual field names. Do not rely on stale examples. + +### Task 2: Add denied-write test + +**Objective:** A rejected `write_file` must not mutate disk. + +**Files:** +- Create/modify: `tests/acp/test_edit_approval.py` + +Test shape: + +```python +def test_write_file_rejected_by_acp_permission_does_not_mutate(tmp_path): + path = tmp_path / "demo.txt" + path.write_text("old") + + # Install fake ACP edit approval callback returning reject_once. + # Invoke the same interception function that the terminal/tool path will call. + + result = maybe_gate_file_edit( + tool_name="write_file", + args={"path": str(path), "content": "new"}, + approval_requester=fake_reject, + ) + + assert path.read_text() == "old" + assert "rejected" in result.lower() +``` + +The exact function name will be created in Task 4. + +### Task 3: Add approved-write test + +**Objective:** Approved writes proceed and include diff content in permission request. + +Assert: + +- fake requester received tool call `kind == "edit"` +- content includes diff block for `demo.txt` +- after approval, file content is changed + +### Task 4: Implement edit proposal computation + +**Files:** +- Create: `acp_adapter/edit_approval.py` + +Add pure helpers first: + +```python +@dataclass +class EditProposal: + path: str + old_text: str | None + new_text: str + title: str + + +def proposal_for_write_file(args: dict[str, Any]) -> EditProposal: + path = str(args["path"]) + old_text = Path(path).read_text(encoding="utf-8") if Path(path).exists() else None + new_text = str(args.get("content", "")) + return EditProposal(path=path, old_text=old_text, new_text=new_text, title=f"Edit {path}") +``` + +For `patch`, start with replace-mode only. V4A/multi-file patches can be a second task or second PR if too risky. + +### Task 5: Implement ACP permission requester + +**Files:** +- Modify: `acp_adapter/permissions.py` or new `acp_adapter/edit_approval.py` + +Build request with: + +```python +acp.tool_diff_content(path=proposal.path, old_text=proposal.old_text, new_text=proposal.new_text) +``` + +Options: + +- allow once +- reject once +- optionally allow always/reject always only after policy storage exists + +Default deny on exception/cancel/timeout. + +### Task 6: Intercept file mutation tools before execution + +**Objective:** Ensure mutation cannot happen before approval. + +**Files:** +- Likely modify: `model_tools.py` or `acp_adapter/server.py` session-context tool wrapper + +Do not bury this inside post-execution `acp_adapter/events.py`; that is too late. + +Preferred design: + +- set an ACP session contextvar around `agent.run_conversation(...)` +- in the central tool execution path, before dispatching `write_file`/`patch`, call the ACP edit approval gate if contextvar exists +- if rejected, return a normal tool result string like `{"success": false, "error": "Edit rejected by user"}` +- if approved, continue to original tool implementation + +### Task 7: Expand patch coverage + +Add tests for: + +- `patch` replace mode approved/rejected +- creating a new file via `write_file` +- missing old string -> should fail before approval or return normal patch error, but must not mutate +- permission requester exception -> deny and no mutation + +### Task 8: Verification + +Run: + +```bash +scripts/run_tests.sh tests/acp/test_edit_approval.py tests/acp/test_events.py tests/acp/test_tools.py -q +``` + +Then run manual Zed verification: + +1. Ask Hermes ACP to edit a small file. +2. Confirm Zed shows a diff before mutation. +3. Reject and verify file unchanged. +4. Approve and verify file changed. + +**Do not merge** without manual reject-path verification. From 9592e595a26b77754e9d538ad41272e88f1b9d30 Mon Sep 17 00:00:00 2001 From: HenkDz Date: Fri, 15 May 2026 23:28:44 +0100 Subject: [PATCH 06/30] feat(acp): require approval for editor file edits --- acp_adapter/edit_approval.py | 228 ++++++++++++++++++++++++++++++++ acp_adapter/server.py | 28 +++- model_tools.py | 14 ++ tests/acp/test_edit_approval.py | 179 +++++++++++++++++++++++++ 4 files changed, 448 insertions(+), 1 deletion(-) create mode 100644 acp_adapter/edit_approval.py create mode 100644 tests/acp/test_edit_approval.py diff --git a/acp_adapter/edit_approval.py b/acp_adapter/edit_approval.py new file mode 100644 index 0000000000..ebeab0bc7e --- /dev/null +++ b/acp_adapter/edit_approval.py @@ -0,0 +1,228 @@ +"""Pre-execution ACP edit approval helpers. + +This module is intentionally isolated from the generic tool registry. ACP binds +an edit approval requester in a ContextVar for the duration of one ACP agent run; +CLI, gateway, and other sessions leave it unset and therefore bypass this guard. +""" + +from __future__ import annotations + +import asyncio +import json +import logging +from concurrent.futures import TimeoutError as FutureTimeout +from contextvars import ContextVar, Token +from dataclasses import dataclass +from itertools import count +from pathlib import Path +from typing import Any, Callable + +logger = logging.getLogger(__name__) + + +@dataclass(frozen=True) +class EditProposal: + """A proposed single-file edit that can be shown to an ACP client.""" + + tool_name: str + path: str + old_text: str | None + new_text: str + arguments: dict[str, Any] + + +EditApprovalRequester = Callable[[EditProposal], bool] + +_EDIT_APPROVAL_REQUESTER: ContextVar[EditApprovalRequester | None] = ContextVar( + "ACP_EDIT_APPROVAL_REQUESTER", + default=None, +) +_PERMISSION_REQUEST_IDS = count(1) + + +def set_edit_approval_requester(requester: EditApprovalRequester | None) -> Token: + """Bind an ACP edit approval requester for the current context.""" + + return _EDIT_APPROVAL_REQUESTER.set(requester) + + +def reset_edit_approval_requester(token: Token) -> None: + """Restore a previous edit approval requester binding.""" + + _EDIT_APPROVAL_REQUESTER.reset(token) + + +def clear_edit_approval_requester() -> None: + """Clear the current requester; primarily used by tests.""" + + _EDIT_APPROVAL_REQUESTER.set(None) + + +def get_edit_approval_requester() -> EditApprovalRequester | None: + return _EDIT_APPROVAL_REQUESTER.get() + + +def _read_text_if_exists(path: str) -> str | None: + p = Path(path).expanduser() + if not p.exists(): + return None + if not p.is_file(): + raise OSError(f"Cannot edit non-file path: {path}") + return p.read_text(encoding="utf-8", errors="replace") + + +def _proposal_for_write_file(arguments: dict[str, Any]) -> EditProposal: + path = str(arguments.get("path") or "") + if not path: + raise ValueError("path required") + content = arguments.get("content") + if content is None: + raise ValueError("content required") + return EditProposal( + tool_name="write_file", + path=path, + old_text=_read_text_if_exists(path), + new_text=str(content), + arguments=dict(arguments), + ) + + +def _proposal_for_patch_replace(arguments: dict[str, Any]) -> EditProposal: + path = str(arguments.get("path") or "") + if not path: + raise ValueError("path required") + old_string = arguments.get("old_string") + new_string = arguments.get("new_string") + if old_string is None or new_string is None: + raise ValueError("old_string and new_string required") + + old_text = _read_text_if_exists(path) + if old_text is None: + raise ValueError(f"Failed to read file: {path}") + + from tools.fuzzy_match import fuzzy_find_and_replace + + new_text, match_count, _strategy, error = fuzzy_find_and_replace( + old_text, + str(old_string), + str(new_string), + bool(arguments.get("replace_all", False)), + ) + if error or match_count == 0: + raise ValueError(error or f"Could not find match for old_string in {path}") + + return EditProposal( + tool_name="patch", + path=path, + old_text=old_text, + new_text=new_text, + arguments=dict(arguments), + ) + + +def build_edit_proposal(tool_name: str, arguments: dict[str, Any]) -> EditProposal | None: + """Return an edit proposal for supported file mutation calls.""" + + if tool_name == "write_file": + return _proposal_for_write_file(arguments) + if tool_name == "patch" and arguments.get("mode", "replace") == "replace": + return _proposal_for_patch_replace(arguments) + return None + + +def maybe_require_edit_approval(tool_name: str, arguments: dict[str, Any]) -> str | None: + """Run ACP edit approval if bound. + + Returns a JSON tool-error string when the edit must be blocked, otherwise + ``None`` so dispatch can continue. Requester exceptions deny by default. + """ + + requester = get_edit_approval_requester() + if requester is None: + return None + + try: + proposal = build_edit_proposal(tool_name, arguments) + except Exception as exc: + logger.warning("Could not build ACP edit approval proposal for %s: %s", tool_name, exc) + return json.dumps({"error": f"Edit approval denied: could not prepare diff ({exc})"}, ensure_ascii=False) + + if proposal is None: + return None + + try: + approved = bool(requester(proposal)) + except Exception as exc: + logger.warning("ACP edit approval requester failed: %s", exc) + approved = False + + if approved: + return None + return json.dumps({"error": "Edit approval denied by ACP client; file was not modified."}, ensure_ascii=False) + + +def build_acp_edit_tool_call(proposal: EditProposal): + """Build the ToolCallUpdate payload for ACP request_permission.""" + + import acp + + tool_call_id = f"edit-approval-{next(_PERMISSION_REQUEST_IDS)}" + return acp.update_tool_call( + tool_call_id, + title=f"Approve edit: {proposal.path}", + kind="edit", + status="pending", + content=[ + acp.tool_diff_content( + path=proposal.path, + old_text=proposal.old_text, + new_text=proposal.new_text, + ) + ], + raw_input={"tool": proposal.tool_name, "arguments": proposal.arguments}, + ) + + +def make_acp_edit_approval_requester( + request_permission_fn: Callable, + loop: asyncio.AbstractEventLoop, + session_id: str, + timeout: float = 60.0, +) -> EditApprovalRequester: + """Return a sync requester that bridges edit proposals to ACP permissions.""" + + def _requester(proposal: EditProposal) -> bool: + from acp.schema import PermissionOption + from agent.async_utils import safe_schedule_threadsafe + + options = [ + PermissionOption(option_id="allow_once", kind="allow_once", name="Allow edit"), + PermissionOption(option_id="deny", kind="reject_once", name="Deny"), + ] + tool_call = build_acp_edit_tool_call(proposal) + coro = request_permission_fn( + session_id=session_id, + tool_call=tool_call, + options=options, + ) + future = safe_schedule_threadsafe( + coro, + loop, + logger=logger, + log_message="Edit approval request: failed to schedule on loop", + ) + if future is None: + return False + try: + response = future.result(timeout=timeout) + except (FutureTimeout, Exception) as exc: + future.cancel() + logger.warning("Edit approval request timed out or failed: %s", exc) + return False + outcome = getattr(response, "outcome", None) + return ( + getattr(outcome, "outcome", None) == "selected" + and getattr(outcome, "option_id", None) == "allow_once" + ) + + return _requester diff --git a/acp_adapter/server.py b/acp_adapter/server.py index 3031de161f..ebec969205 100644 --- a/acp_adapter/server.py +++ b/acp_adapter/server.py @@ -1243,6 +1243,7 @@ class HermesACPAgent(acp.Agent): tool_call_ids: dict[str, Deque[str]] = defaultdict(deque) tool_call_meta: dict[str, dict[str, Any]] = {} previous_approval_cb = None + edit_approval_requester = None streamed_message = False @@ -1259,6 +1260,16 @@ class HermesACPAgent(acp.Agent): message_cb(text) approval_cb = make_approval_callback(conn.request_permission, loop, session_id) + try: + from acp_adapter.edit_approval import make_acp_edit_approval_requester + + edit_approval_requester = make_acp_edit_approval_requester( + conn.request_permission, + loop, + session_id, + ) + except Exception: + logger.debug("Could not create ACP edit approval requester", exc_info=True) else: tool_progress_cb = None reasoning_cb = None @@ -1288,9 +1299,10 @@ class HermesACPAgent(acp.Agent): # which requires a notify_cb registered in _gateway_notify_cbs. previous_approval_cb = None previous_interactive = None + edit_approval_token = None def _run_agent() -> dict: - nonlocal previous_approval_cb, previous_interactive + nonlocal previous_approval_cb, previous_interactive, edit_approval_token # Bind HERMES_SESSION_KEY for this session so per-session caches # (e.g. the interactive sudo password cache in tools.terminal_tool) # scope to the ACP session rather than leaking across sessions @@ -1314,6 +1326,13 @@ class HermesACPAgent(acp.Agent): _terminal_tool.set_approval_callback(approval_cb) except Exception: logger.debug("Could not set ACP approval callback", exc_info=True) + if edit_approval_requester: + try: + from acp_adapter.edit_approval import set_edit_approval_requester + + edit_approval_token = set_edit_approval_requester(edit_approval_requester) + except Exception: + logger.debug("Could not set ACP edit approval requester", exc_info=True) # Signal to tools.approval that we have an interactive callback # and the non-interactive auto-approve path must not fire. previous_interactive = os.environ.get("HERMES_INTERACTIVE") @@ -1341,6 +1360,13 @@ class HermesACPAgent(acp.Agent): _terminal_tool.set_approval_callback(previous_approval_cb) except Exception: logger.debug("Could not restore approval callback", exc_info=True) + if edit_approval_token is not None: + try: + from acp_adapter.edit_approval import reset_edit_approval_requester + + reset_edit_approval_requester(edit_approval_token) + except Exception: + logger.debug("Could not restore ACP edit approval requester", exc_info=True) if session_tokens is not None and clear_session_vars is not None: try: clear_session_vars(session_tokens) diff --git a/model_tools.py b/model_tools.py index 1cbc83096a..ad938b5f18 100644 --- a/model_tools.py +++ b/model_tools.py @@ -788,6 +788,20 @@ def handle_function_call( if block_message is not None: return json.dumps({"error": block_message}, ensure_ascii=False) + # ACP/Zed edit approval runs before any file mutation. The requester + # is bound via ContextVar only for ACP sessions, so CLI/gateway paths + # are unaffected when it is unset. + try: + from acp_adapter.edit_approval import maybe_require_edit_approval + + edit_block_message = maybe_require_edit_approval(function_name, function_args) + if edit_block_message is not None: + return edit_block_message + except Exception as _edit_approval_err: + logger.debug("ACP edit approval guard error: %s", _edit_approval_err) + if function_name in {"write_file", "patch"}: + return json.dumps({"error": "Edit approval denied: approval guard failed"}, ensure_ascii=False) + # Notify the read-loop tracker when a non-read/search tool runs, # so the *consecutive* counter resets (reads after other work are fine). if function_name not in _READ_SEARCH_TOOLS: diff --git a/tests/acp/test_edit_approval.py b/tests/acp/test_edit_approval.py new file mode 100644 index 0000000000..2d68e22045 --- /dev/null +++ b/tests/acp/test_edit_approval.py @@ -0,0 +1,179 @@ +"""Tests for ACP pre-edit approval gating.""" + +from __future__ import annotations + +import json + +from acp_adapter.edit_approval import ( + EditProposal, + build_acp_edit_tool_call, + clear_edit_approval_requester, + set_edit_approval_requester, +) +from model_tools import handle_function_call + + +def teardown_function() -> None: + clear_edit_approval_requester() + + +def test_acp_permission_tool_call_uses_edit_kind_and_diff_content(): + proposal = EditProposal( + tool_name="write_file", + path="demo.txt", + old_text="old\n", + new_text="new\n", + arguments={"path": "demo.txt", "content": "new\n"}, + ) + + tool_call = build_acp_edit_tool_call(proposal) + + assert tool_call.kind == "edit" + assert tool_call.status == "pending" + assert tool_call.rawInput == {"tool": "write_file", "arguments": proposal.arguments} + assert len(tool_call.content) == 1 + diff = tool_call.content[0] + assert diff.path == "demo.txt" + assert diff.oldText == "old\n" + assert diff.newText == "new\n" + + +def test_write_file_rejection_does_not_mutate_existing_file(tmp_path): + target = tmp_path / "sample.txt" + target.write_text("before\n", encoding="utf-8") + + set_edit_approval_requester(lambda _proposal: False) + + result = json.loads( + handle_function_call( + "write_file", + {"path": str(target), "content": "after\n"}, + task_id="acp-edit-reject", + ) + ) + + assert "error" in result + assert "Edit approval denied" in result["error"] + assert target.read_text(encoding="utf-8") == "before\n" + + +def test_write_file_approval_mutates_and_request_includes_diff(tmp_path): + target = tmp_path / "sample.txt" + target.write_text("before\n", encoding="utf-8") + proposals = [] + + def approve(proposal): + proposals.append(proposal) + return True + + set_edit_approval_requester(approve) + + result = json.loads( + handle_function_call( + "write_file", + {"path": str(target), "content": "after\n"}, + task_id="acp-edit-approve", + ) + ) + + assert result.get("bytes_written") == len("after\n") + assert target.read_text(encoding="utf-8") == "after\n" + assert len(proposals) == 1 + proposal = proposals[0] + assert proposal.tool_name == "write_file" + assert proposal.path == str(target) + assert proposal.old_text == "before\n" + assert proposal.new_text == "after\n" + + +def test_write_file_new_file_request_has_empty_old_text(tmp_path): + target = tmp_path / "new.txt" + proposals = [] + + set_edit_approval_requester(lambda proposal: proposals.append(proposal) or True) + + result = json.loads( + handle_function_call( + "write_file", + {"path": str(target), "content": "created\n"}, + task_id="acp-edit-new-file", + ) + ) + + assert result.get("bytes_written") == len("created\n") + assert target.read_text(encoding="utf-8") == "created\n" + assert proposals[0].old_text is None + assert proposals[0].new_text == "created\n" + + +def test_requester_exception_denies_and_does_not_mutate(tmp_path): + target = tmp_path / "sample.txt" + target.write_text("before\n", encoding="utf-8") + + def boom(_proposal): + raise RuntimeError("zed disconnected") + + set_edit_approval_requester(boom) + + result = json.loads( + handle_function_call( + "write_file", + {"path": str(target), "content": "after\n"}, + task_id="acp-edit-exception", + ) + ) + + assert "error" in result + assert "Edit approval denied" in result["error"] + assert target.read_text(encoding="utf-8") == "before\n" + + +def test_patch_replace_rejection_does_not_mutate(tmp_path): + target = tmp_path / "sample.txt" + target.write_text("alpha\nbeta\n", encoding="utf-8") + + set_edit_approval_requester(lambda _proposal: False) + + result = json.loads( + handle_function_call( + "patch", + { + "mode": "replace", + "path": str(target), + "old_string": "beta\n", + "new_string": "gamma\n", + }, + task_id="acp-patch-reject", + ) + ) + + assert "error" in result + assert "Edit approval denied" in result["error"] + assert target.read_text(encoding="utf-8") == "alpha\nbeta\n" + + +def test_patch_replace_approval_request_includes_full_file_diff(tmp_path): + target = tmp_path / "sample.txt" + target.write_text("alpha\nbeta\n", encoding="utf-8") + proposals = [] + + set_edit_approval_requester(lambda proposal: proposals.append(proposal) or True) + + result = json.loads( + handle_function_call( + "patch", + { + "mode": "replace", + "path": str(target), + "old_string": "beta\n", + "new_string": "gamma\n", + }, + task_id="acp-patch-approve", + ) + ) + + assert result.get("success") is True + assert target.read_text(encoding="utf-8") == "alpha\ngamma\n" + assert proposals[0].tool_name == "patch" + assert proposals[0].old_text == "alpha\nbeta\n" + assert proposals[0].new_text == "alpha\ngamma\n" From 49b28d1646286a1bae20afb93ee3532dfba35888 Mon Sep 17 00:00:00 2001 From: HenkDz Date: Sat, 16 May 2026 11:55:49 +0100 Subject: [PATCH 07/30] fix(acp): avoid duplicate edit approval diffs --- acp_adapter/tools.py | 15 ++++--------- tests/acp/test_mcp_e2e.py | 13 ++++------- tests/acp/test_tools.py | 45 ++++++++++++++++++--------------------- 3 files changed, 29 insertions(+), 44 deletions(-) diff --git a/acp_adapter/tools.py b/acp_adapter/tools.py index 77a62e243b..e9ea747324 100644 --- a/acp_adapter/tools.py +++ b/acp_adapter/tools.py @@ -895,7 +895,7 @@ def _build_tool_complete_content( if len(display_result) > 5000: display_result = display_result[:4900] + f"\n... ({len(result)} chars total, truncated)" - if tool_name in {"write_file", "patch", "skill_manage"}: + if tool_name == "skill_manage": try: from agent.display import extract_edit_diff @@ -936,22 +936,15 @@ def build_tool_start( if tool_name == "patch": mode = arguments.get("mode", "replace") - if mode == "replace": - path = arguments.get("path", "") - old = arguments.get("old_string", "") - new = arguments.get("new_string", "") - content = [acp.tool_diff_content(path=path, new_text=new, old_text=old)] - else: - patch_text = arguments.get("patch", "") - content = _build_patch_mode_content(patch_text) + path = arguments.get("path") or "patch input" + content = [_text(f"Preparing {mode} edit for {path}. Approval prompt shows the diff.")] return acp.start_tool_call( tool_call_id, title, kind=kind, content=content, locations=locations, ) if tool_name == "write_file": path = arguments.get("path", "") - file_content = arguments.get("content", "") - content = [acp.tool_diff_content(path=path, new_text=file_content)] + content = [_text(f"Preparing write to {path}. Approval prompt shows the diff." if path else "Preparing file write. Approval prompt shows the diff.")] return acp.start_tool_call( tool_call_id, title, kind=kind, content=content, locations=locations, ) diff --git a/tests/acp/test_mcp_e2e.py b/tests/acp/test_mcp_e2e.py index dab4607198..00bf53b21f 100644 --- a/tests/acp/test_mcp_e2e.py +++ b/tests/acp/test_mcp_e2e.py @@ -183,7 +183,7 @@ class TestMcpRegistrationE2E: assert "hello" in complete_event.content[0].content.text assert complete_event.raw_output is None - def test_patch_mode_tool_start_emits_diff_blocks_for_v4a_patch(self): + def test_patch_mode_tool_start_defers_diff_to_edit_approval_prompt(self): update = build_tool_start( "tc-1", "patch", @@ -193,14 +193,9 @@ class TestMcpRegistrationE2E: }, ) - assert len(update.content) == 2 - assert update.content[0].type == "diff" - assert update.content[0].path == "src/app.py" - assert update.content[0].old_text == "old line" - assert update.content[0].new_text == "new line" - assert update.content[1].type == "diff" - assert update.content[1].path == "src/new.py" - assert update.content[1].new_text == "hello" + assert len(update.content) == 1 + assert update.content[0].type == "content" + assert "Approval prompt shows the diff" in update.content[0].content.text @pytest.mark.asyncio async def test_prompt_tool_results_paired_by_call_id(self, acp_agent, mock_manager): diff --git a/tests/acp/test_tools.py b/tests/acp/test_tools.py index dc62b296c6..11a427591d 100644 --- a/tests/acp/test_tools.py +++ b/tests/acp/test_tools.py @@ -147,7 +147,7 @@ class TestBuildToolTitle: class TestBuildToolStart: def test_build_tool_start_for_patch(self): - """patch should produce a FileEditToolCallContent (diff).""" + """patch start should not duplicate the edit-approval diff.""" args = { "path": "src/main.py", "old_string": "print('hello')", @@ -156,24 +156,23 @@ class TestBuildToolStart: result = build_tool_start("tc-1", "patch", args) assert isinstance(result, ToolCallStart) assert result.kind == "edit" - # The first content item should be a diff assert len(result.content) >= 1 - diff_item = result.content[0] - assert isinstance(diff_item, FileEditToolCallContent) - assert diff_item.path == "src/main.py" - assert diff_item.new_text == "print('world')" - assert diff_item.old_text == "print('hello')" + item = result.content[0] + assert isinstance(item, ContentToolCallContent) + assert "Approval prompt shows the diff" in item.content.text + assert "src/main.py" in item.content.text def test_build_tool_start_for_write_file(self): - """write_file should produce a FileEditToolCallContent (diff).""" + """write_file start should not duplicate the edit-approval diff.""" args = {"path": "new_file.py", "content": "print('hello')"} result = build_tool_start("tc-w1", "write_file", args) assert isinstance(result, ToolCallStart) assert result.kind == "edit" assert len(result.content) >= 1 - diff_item = result.content[0] - assert isinstance(diff_item, FileEditToolCallContent) - assert diff_item.path == "new_file.py" + item = result.content[0] + assert isinstance(item, ContentToolCallContent) + assert "Approval prompt shows the diff" in item.content.text + assert "new_file.py" in item.content.text def test_build_tool_start_for_terminal(self): """terminal should produce text content with the command.""" @@ -452,8 +451,8 @@ class TestBuildToolComplete: assert len(display_text) < 6000 assert "truncated" in display_text - def test_build_tool_complete_for_patch_uses_diff_blocks(self): - """Completed patch calls should keep structured diff content for Zed.""" + def test_build_tool_complete_for_patch_summarizes_without_repeating_diff(self): + """Completed patch calls should not duplicate the edit-approval diff.""" patch_result = ( '{"success": true, "diff": "--- a/README.md\\n+++ b/README.md\\n@@ -1 +1,2 @@\\n old line\\n+new line\\n", ' '"files_modified": ["README.md"]}' @@ -461,18 +460,17 @@ class TestBuildToolComplete: result = build_tool_complete("tc-p1", "patch", patch_result) assert isinstance(result, ToolCallProgress) assert len(result.content) == 1 - diff_item = result.content[0] - assert isinstance(diff_item, FileEditToolCallContent) - assert diff_item.path == "README.md" - assert diff_item.old_text == "old line" - assert diff_item.new_text == "old line\nnew line" + item = result.content[0] + assert isinstance(item, ContentToolCallContent) + assert "✅ patch completed" in item.content.text + assert "README.md" in item.content.text def test_build_tool_complete_for_patch_falls_back_to_text_when_no_diff(self): result = build_tool_complete("tc-p2", "patch", '{"success": true}') assert isinstance(result, ToolCallProgress) assert isinstance(result.content[0], ContentToolCallContent) - def test_build_tool_complete_for_write_file_uses_snapshot_diff(self, tmp_path): + def test_build_tool_complete_for_write_file_summarizes_without_repeating_diff(self, tmp_path): target = tmp_path / "diff-test.txt" snapshot = type("Snapshot", (), {"paths": [target], "before": {str(target): None}})() target.write_text("hello from hermes\n", encoding="utf-8") @@ -486,11 +484,10 @@ class TestBuildToolComplete: ) assert isinstance(result, ToolCallProgress) assert len(result.content) == 1 - diff_item = result.content[0] - assert isinstance(diff_item, FileEditToolCallContent) - assert diff_item.path.endswith("diff-test.txt") - assert diff_item.old_text is None - assert diff_item.new_text == "hello from hermes" + item = result.content[0] + assert isinstance(item, ContentToolCallContent) + assert "✅ write_file completed" in item.content.text + assert "diff-test.txt" in item.content.text # --------------------------------------------------------------------------- From f70e0b85dd483d1c2b37e8bebe7a4241796726d9 Mon Sep 17 00:00:00 2001 From: HenkDz Date: Sat, 16 May 2026 12:06:21 +0100 Subject: [PATCH 08/30] feat(acp): add session-scoped edit auto-approval --- acp_adapter/edit_approval.py | 50 ++++++++++++++++++++++++ acp_adapter/server.py | 68 +++++++++++++++++++++++++++++++-- tests/acp/test_edit_approval.py | 24 ++++++++++++ tests/acp/test_server.py | 29 +++++++++++++- 4 files changed, 166 insertions(+), 5 deletions(-) diff --git a/acp_adapter/edit_approval.py b/acp_adapter/edit_approval.py index ebeab0bc7e..7c5fcaefd2 100644 --- a/acp_adapter/edit_approval.py +++ b/acp_adapter/edit_approval.py @@ -40,6 +40,12 @@ _EDIT_APPROVAL_REQUESTER: ContextVar[EditApprovalRequester | None] = ContextVar( _PERMISSION_REQUEST_IDS = count(1) +SENSITIVE_AUTO_APPROVE_NAMES = {".env", ".env.local", ".env.production", "id_rsa", "id_ed25519"} +AUTO_APPROVE_ASK = "ask" +AUTO_APPROVE_WORKSPACE = "workspace_session" +AUTO_APPROVE_SESSION = "session" + + def set_edit_approval_requester(requester: EditApprovalRequester | None) -> Token: """Bind an ACP edit approval requester for the current context.""" @@ -130,6 +136,40 @@ def build_edit_proposal(tool_name: str, arguments: dict[str, Any]) -> EditPropos return None +def _is_sensitive_auto_approve_path(path: str) -> bool: + parts = Path(path).expanduser().parts + lowered = {part.lower() for part in parts} + if ".git" in lowered or ".ssh" in lowered: + return True + return Path(path).name.lower() in SENSITIVE_AUTO_APPROVE_NAMES + + +def should_auto_approve_edit(proposal: EditProposal, policy: str, cwd: str | None = None) -> bool: + """Return whether an ACP edit proposal may bypass the prompt for this session. + + This is intentionally session-scoped and conservative: sensitive paths still + ask even under autonomous policies. + """ + + policy = str(policy or AUTO_APPROVE_ASK).strip() + if policy == AUTO_APPROVE_ASK or _is_sensitive_auto_approve_path(proposal.path): + return False + path = Path(proposal.path).expanduser().resolve(strict=False) + if policy == AUTO_APPROVE_SESSION: + return True + if policy == AUTO_APPROVE_WORKSPACE: + if str(path).startswith("/tmp/"): + return True + if cwd: + root = Path(cwd).expanduser().resolve(strict=False) + try: + path.relative_to(root) + return True + except ValueError: + return False + return False + + def maybe_require_edit_approval(tool_name: str, arguments: dict[str, Any]) -> str | None: """Run ACP edit approval if bound. @@ -188,6 +228,7 @@ def make_acp_edit_approval_requester( loop: asyncio.AbstractEventLoop, session_id: str, timeout: float = 60.0, + auto_approve_getter: Callable[[], tuple[str, str | None]] | None = None, ) -> EditApprovalRequester: """Return a sync requester that bridges edit proposals to ACP permissions.""" @@ -195,6 +236,15 @@ def make_acp_edit_approval_requester( from acp.schema import PermissionOption from agent.async_utils import safe_schedule_threadsafe + if auto_approve_getter is not None: + try: + policy, cwd = auto_approve_getter() + if should_auto_approve_edit(proposal, policy, cwd): + logger.info("Auto-approved ACP edit under policy %s: %s", policy, proposal.path) + return True + except Exception: + logger.debug("ACP edit auto-approval policy check failed", exc_info=True) + options = [ PermissionOption(option_id="allow_once", kind="allow_once", name="Allow edit"), PermissionOption(option_id="deny", kind="reject_once", name="Deny"), diff --git a/acp_adapter/server.py b/acp_adapter/server.py index ebec969205..62f8eafe6f 100644 --- a/acp_adapter/server.py +++ b/acp_adapter/server.py @@ -45,6 +45,8 @@ from acp.schema import ( SetSessionModeResponse, ResourceContentBlock, SessionCapabilities, + SessionConfigOptionSelect, + SessionConfigSelectOption, SessionForkCapabilities, SessionListCapabilities, SessionModelState, @@ -495,6 +497,9 @@ class HermesACPAgent(acp.Agent): }, ) + _EDIT_APPROVAL_POLICY_CONFIG_ID = "edit_approval_policy" + _EDIT_APPROVAL_POLICY_DEFAULT = "ask" + def __init__(self, session_manager: SessionManager | None = None): super().__init__() self.session_manager = session_manager or SessionManager() @@ -507,6 +512,49 @@ class HermesACPAgent(acp.Agent): self._conn = conn logger.info("ACP client connected") + + def _session_config_options(self, state: SessionState) -> list[Any]: + values = getattr(state, "config_options", None) + if not isinstance(values, dict): + values = {} + current = str(values.get(self._EDIT_APPROVAL_POLICY_CONFIG_ID) or self._EDIT_APPROVAL_POLICY_DEFAULT) + allowed = {"ask", "workspace_session", "session"} + if current not in allowed: + current = self._EDIT_APPROVAL_POLICY_DEFAULT + return [ + SessionConfigOptionSelect( + id=self._EDIT_APPROVAL_POLICY_CONFIG_ID, + name="Edit approvals", + description="Control ACP edit approvals for this session.", + category="permissions", + type="select", + current_value=current, + options=[ + SessionConfigSelectOption( + value="ask", + name="Ask before edits", + description="Require approval for every file edit.", + ), + SessionConfigSelectOption( + value="workspace_session", + name="Auto-allow workspace edits", + description="Allow workspace and /tmp edits for this session; still asks for sensitive paths.", + ), + SessionConfigSelectOption( + value="session", + name="Auto-allow all edits this session", + description="Allow file edits for this session except sensitive paths.", + ), + ], + ) + ] + + def _edit_approval_policy_for_state(self, state: SessionState) -> tuple[str, str | None]: + values = getattr(state, "config_options", None) + if not isinstance(values, dict): + values = {} + return str(values.get(self._EDIT_APPROVAL_POLICY_CONFIG_ID) or self._EDIT_APPROVAL_POLICY_DEFAULT), state.cwd + @staticmethod def _encode_model_choice(provider: str | None, model: str | None) -> str: """Encode a model selection so ACP clients can keep provider context.""" @@ -992,6 +1040,7 @@ class HermesACPAgent(acp.Agent): return NewSessionResponse( session_id=state.session_id, models=self._build_model_state(state), + config_options=self._session_config_options(state), ) async def load_session( @@ -1033,7 +1082,10 @@ class HermesACPAgent(acp.Agent): ) self._schedule_available_commands_update(session_id) self._schedule_usage_update(state) - return LoadSessionResponse(models=self._build_model_state(state)) + return LoadSessionResponse( + models=self._build_model_state(state), + config_options=self._session_config_options(state), + ) async def resume_session( self, @@ -1062,7 +1114,10 @@ class HermesACPAgent(acp.Agent): ) self._schedule_available_commands_update(state.session_id) self._schedule_usage_update(state) - return ResumeSessionResponse(models=self._build_model_state(state)) + return ResumeSessionResponse( + models=self._build_model_state(state), + config_options=self._session_config_options(state), + ) async def cancel(self, session_id: str, **kwargs: Any) -> None: state = self.session_manager.get_session(session_id) @@ -1092,7 +1147,11 @@ class HermesACPAgent(acp.Agent): logger.info("Forked session %s -> %s", session_id, new_id) if new_id: self._schedule_available_commands_update(new_id) - return ForkSessionResponse(session_id=new_id) + return ForkSessionResponse( + session_id=new_id, + models=self._build_model_state(state) if state is not None else None, + config_options=self._session_config_options(state) if state is not None else None, + ) async def list_sessions( self, @@ -1267,6 +1326,7 @@ class HermesACPAgent(acp.Agent): conn.request_permission, loop, session_id, + auto_approve_getter=lambda: self._edit_approval_policy_for_state(state), ) except Exception: logger.debug("Could not create ACP edit approval requester", exc_info=True) @@ -1810,4 +1870,4 @@ class HermesACPAgent(acp.Agent): setattr(state, "config_options", options) self.session_manager.save_session(session_id) logger.info("Session %s: config option %s updated", session_id, config_id) - return SetSessionConfigOptionResponse(config_options=[]) + return SetSessionConfigOptionResponse(config_options=self._session_config_options(state)) diff --git a/tests/acp/test_edit_approval.py b/tests/acp/test_edit_approval.py index 2d68e22045..1b6660c3b2 100644 --- a/tests/acp/test_edit_approval.py +++ b/tests/acp/test_edit_approval.py @@ -3,12 +3,14 @@ from __future__ import annotations import json +from pathlib import Path from acp_adapter.edit_approval import ( EditProposal, build_acp_edit_tool_call, clear_edit_approval_requester, set_edit_approval_requester, + should_auto_approve_edit, ) from model_tools import handle_function_call @@ -177,3 +179,25 @@ def test_patch_replace_approval_request_includes_full_file_diff(tmp_path): assert proposals[0].tool_name == "patch" assert proposals[0].old_text == "alpha\nbeta\n" assert proposals[0].new_text == "alpha\ngamma\n" + + +def test_workspace_auto_approval_allows_workspace_and_tmp_but_not_sensitive(tmp_path): + workspace_file = tmp_path / "src.py" + tmp_file = Path("/tmp/hermes-acp-auto-approve-test.txt") + env_file = tmp_path / ".env" + + assert should_auto_approve_edit( + EditProposal("write_file", str(workspace_file), None, "x", {}), + "workspace_session", + str(tmp_path), + ) + assert should_auto_approve_edit( + EditProposal("write_file", str(tmp_file), None, "x", {}), + "workspace_session", + str(tmp_path), + ) + assert not should_auto_approve_edit( + EditProposal("write_file", str(env_file), None, "SECRET=x", {}), + "session", + str(tmp_path), + ) diff --git a/tests/acp/test_server.py b/tests/acp/test_server.py index 65dd6fd6b7..e17d9c618c 100644 --- a/tests/acp/test_server.py +++ b/tests/acp/test_server.py @@ -52,6 +52,32 @@ def agent(mock_manager): """HermesACPAgent backed by a mock session manager.""" return HermesACPAgent(session_manager=mock_manager) + @pytest.mark.asyncio + async def test_new_session_includes_edit_approval_config_option(self, agent): + resp = await agent.new_session(cwd="/tmp") + + assert resp.config_options + option = resp.config_options[0] + assert option.id == "edit_approval_policy" + assert option.current_value == "ask" + assert {choice.value for choice in option.options} == { + "ask", + "workspace_session", + "session", + } + + @pytest.mark.asyncio + async def test_set_config_option_persists_edit_approval_policy(self, agent): + resp = await agent.new_session(cwd="/tmp") + update = await agent.set_config_option( + "edit_approval_policy", + resp.session_id, + "workspace_session", + ) + + assert isinstance(update, SetSessionConfigOptionResponse) + assert update.config_options[0].current_value == "workspace_session" + # --------------------------------------------------------------------------- # initialize @@ -892,7 +918,8 @@ class TestSessionConfiguration: ) assert mode_result == {} - assert config_result == {"configOptions": []} + assert config_result["configOptions"] + assert config_result["configOptions"][0]["id"] == "edit_approval_policy" @pytest.mark.asyncio async def test_router_accepts_unstable_model_switch_when_enabled(self, agent): From 029239860426a3b12c3c7e0a7ac1a5634ff7b0ce Mon Sep 17 00:00:00 2001 From: HenkDz Date: Sat, 16 May 2026 19:15:08 +0100 Subject: [PATCH 09/30] fix(acp): use modes for edit auto-approval --- acp_adapter/events.py | 16 ++++- acp_adapter/server.py | 129 +++++++++++++++++++++++---------------- acp_adapter/tools.py | 30 +++++++-- tests/acp/test_server.py | 57 +++++++++-------- tests/acp/test_tools.py | 20 ++++++ 5 files changed, 165 insertions(+), 87 deletions(-) diff --git a/acp_adapter/events.py b/acp_adapter/events.py index 00e940b9ee..ab82c0e7e3 100644 --- a/acp_adapter/events.py +++ b/acp_adapter/events.py @@ -117,6 +117,7 @@ def make_tool_progress_cb( loop: asyncio.AbstractEventLoop, tool_call_ids: Dict[str, Deque[str]], tool_call_meta: Dict[str, Dict[str, Any]], + edit_approval_policy_getter: Callable[[], tuple[str, str | None]] | None = None, ) -> Callable: """Create a ``tool_progress_callback`` for AIAgent. @@ -162,7 +163,20 @@ def make_tool_progress_cb( logger.debug("Failed to capture ACP edit snapshot for %s", name, exc_info=True) tool_call_meta[tc_id] = {"args": args, "snapshot": snapshot} - update = build_tool_start(tc_id, name, args) + edit_diff = None + if name in {"write_file", "patch"} and edit_approval_policy_getter is not None: + try: + from acp_adapter.edit_approval import build_edit_proposal, should_auto_approve_edit + + proposal = build_edit_proposal(name, args) + if proposal is not None: + policy, cwd = edit_approval_policy_getter() + if should_auto_approve_edit(proposal, policy, cwd): + edit_diff = proposal + except Exception: + logger.debug("Failed to prepare auto-approved ACP edit diff for %s", name, exc_info=True) + + update = build_tool_start(tc_id, name, args, edit_diff=edit_diff) _send_update(conn, session_id, loop, update) return _tool_progress diff --git a/acp_adapter/server.py b/acp_adapter/server.py index 62f8eafe6f..e4fc336b66 100644 --- a/acp_adapter/server.py +++ b/acp_adapter/server.py @@ -45,10 +45,10 @@ from acp.schema import ( SetSessionModeResponse, ResourceContentBlock, SessionCapabilities, - SessionConfigOptionSelect, - SessionConfigSelectOption, SessionForkCapabilities, SessionListCapabilities, + SessionMode, + SessionModeState, SessionModelState, SessionResumeCapabilities, SessionInfo, @@ -499,6 +499,17 @@ class HermesACPAgent(acp.Agent): _EDIT_APPROVAL_POLICY_CONFIG_ID = "edit_approval_policy" _EDIT_APPROVAL_POLICY_DEFAULT = "ask" + _MODE_DEFAULT = "default" + _MODE_ACCEPT_EDITS = "accept_edits" + _MODE_DONT_ASK = "dont_ask" + _MODE_TO_EDIT_APPROVAL_POLICY = { + _MODE_DEFAULT: "ask", + _MODE_ACCEPT_EDITS: "workspace_session", + _MODE_DONT_ASK: "session", + } + _EDIT_APPROVAL_POLICY_TO_MODE = { + value: key for key, value in _MODE_TO_EDIT_APPROVAL_POLICY.items() + } def __init__(self, session_manager: SessionManager | None = None): super().__init__() @@ -513,47 +524,43 @@ class HermesACPAgent(acp.Agent): logger.info("ACP client connected") - def _session_config_options(self, state: SessionState) -> list[Any]: - values = getattr(state, "config_options", None) - if not isinstance(values, dict): - values = {} - current = str(values.get(self._EDIT_APPROVAL_POLICY_CONFIG_ID) or self._EDIT_APPROVAL_POLICY_DEFAULT) - allowed = {"ask", "workspace_session", "session"} - if current not in allowed: - current = self._EDIT_APPROVAL_POLICY_DEFAULT - return [ - SessionConfigOptionSelect( - id=self._EDIT_APPROVAL_POLICY_CONFIG_ID, - name="Edit approvals", - description="Control ACP edit approvals for this session.", - category="permissions", - type="select", - current_value=current, - options=[ - SessionConfigSelectOption( - value="ask", - name="Ask before edits", - description="Require approval for every file edit.", - ), - SessionConfigSelectOption( - value="workspace_session", - name="Auto-allow workspace edits", - description="Allow workspace and /tmp edits for this session; still asks for sensitive paths.", - ), - SessionConfigSelectOption( - value="session", - name="Auto-allow all edits this session", - description="Allow file edits for this session except sensitive paths.", - ), - ], - ) - ] + def _session_modes(self, state: SessionState) -> SessionModeState: + """Return ACP session modes while preserving Zed's separate model picker. + + Zed renders ``config_options`` in the prominent selector slot where the + model picker was visible. Claude/Codex expose policy-like controls as ACP + modes, which coexist with the model picker, so Hermes maps edit approval + policy onto modes instead of advertising config options. + """ + + current = str(getattr(state, "mode", "") or self._MODE_DEFAULT) + if current not in self._MODE_TO_EDIT_APPROVAL_POLICY: + current = self._MODE_DEFAULT + return SessionModeState( + current_mode_id=current, + available_modes=[ + SessionMode( + id=self._MODE_DEFAULT, + name="Default", + description="Ask before edits.", + ), + SessionMode( + id=self._MODE_ACCEPT_EDITS, + name="Accept Edits", + description="Auto-allow workspace and /tmp edits; still asks for sensitive paths.", + ), + SessionMode( + id=self._MODE_DONT_ASK, + name="Don't Ask", + description="Auto-allow file edits for this session except sensitive paths.", + ), + ], + ) def _edit_approval_policy_for_state(self, state: SessionState) -> tuple[str, str | None]: - values = getattr(state, "config_options", None) - if not isinstance(values, dict): - values = {} - return str(values.get(self._EDIT_APPROVAL_POLICY_CONFIG_ID) or self._EDIT_APPROVAL_POLICY_DEFAULT), state.cwd + mode = str(getattr(state, "mode", "") or self._MODE_DEFAULT) + policy = self._MODE_TO_EDIT_APPROVAL_POLICY.get(mode, self._EDIT_APPROVAL_POLICY_DEFAULT) + return policy, state.cwd @staticmethod def _encode_model_choice(provider: str | None, model: str | None) -> str: @@ -1040,7 +1047,7 @@ class HermesACPAgent(acp.Agent): return NewSessionResponse( session_id=state.session_id, models=self._build_model_state(state), - config_options=self._session_config_options(state), + modes=self._session_modes(state), ) async def load_session( @@ -1084,7 +1091,7 @@ class HermesACPAgent(acp.Agent): self._schedule_usage_update(state) return LoadSessionResponse( models=self._build_model_state(state), - config_options=self._session_config_options(state), + modes=self._session_modes(state), ) async def resume_session( @@ -1116,7 +1123,7 @@ class HermesACPAgent(acp.Agent): self._schedule_usage_update(state) return ResumeSessionResponse( models=self._build_model_state(state), - config_options=self._session_config_options(state), + modes=self._session_modes(state), ) async def cancel(self, session_id: str, **kwargs: Any) -> None: @@ -1150,7 +1157,7 @@ class HermesACPAgent(acp.Agent): return ForkSessionResponse( session_id=new_id, models=self._build_model_state(state) if state is not None else None, - config_options=self._session_config_options(state) if state is not None else None, + modes=self._session_modes(state) if state is not None else None, ) async def list_sessions( @@ -1307,7 +1314,14 @@ class HermesACPAgent(acp.Agent): streamed_message = False if conn: - tool_progress_cb = make_tool_progress_cb(conn, session_id, loop, tool_call_ids, tool_call_meta) + tool_progress_cb = make_tool_progress_cb( + conn, + session_id, + loop, + tool_call_ids, + tool_call_meta, + edit_approval_policy_getter=lambda: self._edit_approval_policy_for_state(state), + ) reasoning_cb = make_thinking_cb(conn, session_id, loop) step_cb = make_step_cb(conn, session_id, loop, tool_call_ids, tool_call_meta) message_cb = make_message_cb(conn, session_id, loop) @@ -1849,9 +1863,12 @@ class HermesACPAgent(acp.Agent): if state is None: logger.warning("Session %s: mode switch requested for missing session", session_id) return None - setattr(state, "mode", mode_id) + normalized_mode = str(mode_id or "").strip() + if normalized_mode not in self._MODE_TO_EDIT_APPROVAL_POLICY: + normalized_mode = self._MODE_DEFAULT + setattr(state, "mode", normalized_mode) self.session_manager.save_session(session_id) - logger.info("Session %s: mode switched to %s", session_id, mode_id) + logger.info("Session %s: mode switched to %s", session_id, normalized_mode) return SetSessionModeResponse() async def set_config_option( @@ -1863,11 +1880,15 @@ class HermesACPAgent(acp.Agent): logger.warning("Session %s: config update requested for missing session", session_id) return None - options = getattr(state, "config_options", None) - if not isinstance(options, dict): - options = {} - options[str(config_id)] = value - setattr(state, "config_options", options) + if str(config_id) == self._EDIT_APPROVAL_POLICY_CONFIG_ID: + mode = self._EDIT_APPROVAL_POLICY_TO_MODE.get(str(value), self._MODE_DEFAULT) + setattr(state, "mode", mode) + else: + options = getattr(state, "config_options", None) + if not isinstance(options, dict): + options = {} + options[str(config_id)] = value + setattr(state, "config_options", options) self.session_manager.save_session(session_id) logger.info("Session %s: config option %s updated", session_id, config_id) - return SetSessionConfigOptionResponse(config_options=self._session_config_options(state)) + return SetSessionConfigOptionResponse(config_options=[]) diff --git a/acp_adapter/tools.py b/acp_adapter/tools.py index e9ea747324..6513f1bb55 100644 --- a/acp_adapter/tools.py +++ b/acp_adapter/tools.py @@ -928,6 +928,8 @@ def build_tool_start( tool_call_id: str, tool_name: str, arguments: Dict[str, Any], + *, + edit_diff: Any = None, ) -> ToolCallStart: """Create a ToolCallStart event for the given hermes tool invocation.""" kind = get_tool_kind(tool_name) @@ -935,16 +937,34 @@ def build_tool_start( locations = extract_locations(arguments) if tool_name == "patch": - mode = arguments.get("mode", "replace") - path = arguments.get("path") or "patch input" - content = [_text(f"Preparing {mode} edit for {path}. Approval prompt shows the diff.")] + if edit_diff is not None: + content = [ + acp.tool_diff_content( + path=edit_diff.path, + old_text=edit_diff.old_text, + new_text=edit_diff.new_text, + ) + ] + else: + mode = arguments.get("mode", "replace") + path = arguments.get("path") or "patch input" + content = [_text(f"Preparing {mode} edit for {path}. Approval prompt shows the diff.")] return acp.start_tool_call( tool_call_id, title, kind=kind, content=content, locations=locations, ) if tool_name == "write_file": - path = arguments.get("path", "") - content = [_text(f"Preparing write to {path}. Approval prompt shows the diff." if path else "Preparing file write. Approval prompt shows the diff.")] + if edit_diff is not None: + content = [ + acp.tool_diff_content( + path=edit_diff.path, + old_text=edit_diff.old_text, + new_text=edit_diff.new_text, + ) + ] + else: + path = arguments.get("path", "") + content = [_text(f"Preparing write to {path}. Approval prompt shows the diff." if path else "Preparing file write. Approval prompt shows the diff.")] return acp.start_tool_call( tool_call_id, title, kind=kind, content=content, locations=locations, ) diff --git a/tests/acp/test_server.py b/tests/acp/test_server.py index e17d9c618c..79b7e56f2b 100644 --- a/tests/acp/test_server.py +++ b/tests/acp/test_server.py @@ -24,6 +24,7 @@ from acp.schema import ( PromptResponse, ResumeSessionResponse, SessionModelState, + SessionModeState, SetSessionConfigOptionResponse, SetSessionModelResponse, SetSessionModeResponse, @@ -52,31 +53,34 @@ def agent(mock_manager): """HermesACPAgent backed by a mock session manager.""" return HermesACPAgent(session_manager=mock_manager) - @pytest.mark.asyncio - async def test_new_session_includes_edit_approval_config_option(self, agent): - resp = await agent.new_session(cwd="/tmp") - assert resp.config_options - option = resp.config_options[0] - assert option.id == "edit_approval_policy" - assert option.current_value == "ask" - assert {choice.value for choice in option.options} == { - "ask", - "workspace_session", - "session", - } +@pytest.mark.asyncio +async def test_new_session_exposes_edit_approvals_as_modes_not_config_options(agent): + resp = await agent.new_session(cwd="/tmp") - @pytest.mark.asyncio - async def test_set_config_option_persists_edit_approval_policy(self, agent): - resp = await agent.new_session(cwd="/tmp") - update = await agent.set_config_option( - "edit_approval_policy", - resp.session_id, - "workspace_session", - ) + assert resp.config_options is None + assert isinstance(resp.modes, SessionModeState) + assert resp.modes.current_mode_id == "default" + assert [(mode.id, mode.name) for mode in resp.modes.available_modes] == [ + ("default", "Default"), + ("accept_edits", "Accept Edits"), + ("dont_ask", "Don't Ask"), + ] - assert isinstance(update, SetSessionConfigOptionResponse) - assert update.config_options[0].current_value == "workspace_session" + +@pytest.mark.asyncio +async def test_set_config_option_persists_edit_approval_policy_without_advertising_config(agent): + resp = await agent.new_session(cwd="/tmp") + update = await agent.set_config_option( + "edit_approval_policy", + resp.session_id, + "workspace_session", + ) + state = agent.session_manager.get_session(resp.session_id) + + assert isinstance(update, SetSessionConfigOptionResponse) + assert update.config_options == [] + assert getattr(state, "mode", None) == "accept_edits" # --------------------------------------------------------------------------- @@ -891,11 +895,11 @@ class TestSessionConfiguration: @pytest.mark.asyncio async def test_set_session_mode_returns_response(self, agent): new_resp = await agent.new_session(cwd="/tmp") - resp = await agent.set_session_mode(mode_id="chat", session_id=new_resp.session_id) + resp = await agent.set_session_mode(mode_id="accept_edits", session_id=new_resp.session_id) state = agent.session_manager.get_session(new_resp.session_id) assert isinstance(resp, SetSessionModeResponse) - assert getattr(state, "mode", None) == "chat" + assert getattr(state, "mode", None) == "accept_edits" @pytest.mark.asyncio async def test_router_accepts_stable_session_config_methods(self, agent): @@ -904,7 +908,7 @@ class TestSessionConfiguration: mode_result = await router( "session/set_mode", - {"modeId": "chat", "sessionId": new_resp.session_id}, + {"modeId": "accept_edits", "sessionId": new_resp.session_id}, False, ) config_result = await router( @@ -918,8 +922,7 @@ class TestSessionConfiguration: ) assert mode_result == {} - assert config_result["configOptions"] - assert config_result["configOptions"][0]["id"] == "edit_approval_policy" + assert config_result["configOptions"] == [] @pytest.mark.asyncio async def test_router_accepts_unstable_model_switch_when_enabled(self, agent): diff --git a/tests/acp/test_tools.py b/tests/acp/test_tools.py index 11a427591d..004b1f32f8 100644 --- a/tests/acp/test_tools.py +++ b/tests/acp/test_tools.py @@ -2,6 +2,7 @@ import pytest +from acp_adapter.edit_approval import EditProposal from acp_adapter.tools import ( TOOL_KIND_MAP, build_tool_complete, @@ -174,6 +175,25 @@ class TestBuildToolStart: assert "Approval prompt shows the diff" in item.content.text assert "new_file.py" in item.content.text + def test_auto_approved_edit_start_shows_diff_content(self): + """Auto-approved edit starts need the diff because no approval card exists.""" + args = {"path": "/tmp/acp.txt", "old_string": "old", "new_string": "new"} + result = build_tool_start( + "tc-auto-edit", + "patch", + args, + edit_diff=EditProposal("patch", "/tmp/acp.txt", "old\n", "new\n", args), + ) + + assert isinstance(result, ToolCallStart) + assert result.kind == "edit" + assert len(result.content) == 1 + item = result.content[0] + assert isinstance(item, FileEditToolCallContent) + assert item.path == "/tmp/acp.txt" + assert item.old_text == "old\n" + assert item.new_text == "new\n" + def test_build_tool_start_for_terminal(self): """terminal should produce text content with the command.""" args = {"command": "ls -la /tmp"} From 8831eb5c70e2e99cda9983919100a335a9bd86b8 Mon Sep 17 00:00:00 2001 From: qWaitCrypto Date: Fri, 15 May 2026 14:46:33 +0800 Subject: [PATCH 10/30] fix(kanban): align worker terminal timeout with task runtime --- hermes_cli/kanban_db.py | 55 ++++++++ .../test_kanban_core_functionality.py | 118 ++++++++++++++++++ 2 files changed, 173 insertions(+) diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index 4bd4827e38..bad382c339 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -3067,6 +3067,10 @@ DEFAULT_SPAWN_FAILURE_LIMIT = DEFAULT_FAILURE_LIMIT # and rotates on spawn if the file is larger than this at spawn time. DEFAULT_LOG_ROTATE_BYTES = 2 * 1024 * 1024 # 2 MiB +# Keep a little wall-clock budget for the worker to observe a terminal timeout +# and call kanban_block/kanban_complete before max_runtime_seconds kills it. +KANBAN_TERMINAL_TIMEOUT_GRACE_SECONDS = 30 + @dataclass class DispatchResult: @@ -4077,6 +4081,36 @@ def _resolve_hermes_argv() -> list[str]: return [sys.executable, "-m", "hermes_cli.main"] +def _worker_terminal_timeout_env( + max_runtime_seconds: Optional[int], + current_timeout: Optional[str], +) -> Optional[str]: + """Return a worker-scoped TERMINAL_TIMEOUT override, if needed. + + Kanban's ``max_runtime_seconds`` bounds the whole worker attempt. The + terminal tool has its own default timeout via ``TERMINAL_TIMEOUT``; when + the worker runtime is longer, raise only the child process default so a + long command is not killed by the generic terminal default first. + """ + if max_runtime_seconds is None: + return None + try: + runtime = int(max_runtime_seconds) + except (TypeError, ValueError): + return None + if runtime <= 0: + return None + + desired = max(1, runtime - KANBAN_TERMINAL_TIMEOUT_GRACE_SECONDS) + try: + existing = int(str(current_timeout).strip()) if current_timeout else 0 + except (TypeError, ValueError): + existing = 0 + if existing >= desired: + return None + return str(desired) + + def _default_spawn( task: Task, workspace: str, @@ -4132,6 +4166,18 @@ def _default_spawn( env["HERMES_KANBAN_RUN_ID"] = str(task.current_run_id) if task.claim_lock: env["HERMES_KANBAN_CLAIM_LOCK"] = task.claim_lock + terminal_timeout = _worker_terminal_timeout_env( + task.max_runtime_seconds, + env.get("TERMINAL_TIMEOUT"), + ) + if terminal_timeout is not None: + env["TERMINAL_TIMEOUT"] = terminal_timeout + foreground_timeout = _worker_terminal_timeout_env( + task.max_runtime_seconds, + env.get("TERMINAL_MAX_FOREGROUND_TIMEOUT"), + ) + if foreground_timeout is not None: + env["TERMINAL_MAX_FOREGROUND_TIMEOUT"] = foreground_timeout # Pin the shared board + workspaces root the dispatcher resolved, so # that even when the worker activates a profile (`hermes -p ` # rewrites HERMES_HOME), its kanban paths still match the @@ -4322,6 +4368,15 @@ def build_worker_context(conn: sqlite3.Connection, task_id: str) -> str: if task.tenant: lines.append(f"Tenant: {task.tenant}") lines.append(f"Workspace: {task.workspace_kind} @ {task.workspace_path or '(unresolved)'}") + if task.max_runtime_seconds is not None: + terminal_timeout = _worker_terminal_timeout_env( + task.max_runtime_seconds, + os.environ.get("TERMINAL_TIMEOUT"), + ) + effective_terminal_timeout = terminal_timeout or os.environ.get("TERMINAL_TIMEOUT") + lines.append(f"Max runtime: {task.max_runtime_seconds}s") + if effective_terminal_timeout: + lines.append(f"Terminal timeout: {effective_terminal_timeout}s") lines.append("") if task.body and task.body.strip(): diff --git a/tests/hermes_cli/test_kanban_core_functionality.py b/tests/hermes_cli/test_kanban_core_functionality.py index 35dc7ace95..879a74dee5 100644 --- a/tests/hermes_cli/test_kanban_core_functionality.py +++ b/tests/hermes_cli/test_kanban_core_functionality.py @@ -2679,6 +2679,124 @@ def test_default_spawn_auto_loads_kanban_worker_skill(kanban_home, monkeypatch): assert env.get("HERMES_PROFILE") == "some-profile" +def test_default_spawn_raises_terminal_timeout_to_task_runtime(kanban_home, monkeypatch): + """A task runtime cap should raise the worker's terminal default. + + This is worker-scoped env only: normal CLI/gateway terminal settings stay + untouched, but long kanban tasks no longer inherit a short generic + TERMINAL_TIMEOUT that kills their foreground command first. + """ + captured = {} + + class FakeProc: + pid = 123 + + def fake_popen(cmd, **kwargs): + captured["env"] = kwargs.get("env", {}) + return FakeProc() + + monkeypatch.setattr("subprocess.Popen", fake_popen) + monkeypatch.setenv("TERMINAL_TIMEOUT", "180") + monkeypatch.delenv("TERMINAL_MAX_FOREGROUND_TIMEOUT", raising=False) + + conn = kb.connect() + try: + tid = kb.create_task( + conn, + title="long worker", + assignee="ops", + max_runtime_seconds=3600, + ) + task = kb.get_task(conn, tid) + workspace = kb.resolve_workspace(task) + kb._default_spawn(task, str(workspace)) + finally: + conn.close() + + assert captured["env"]["TERMINAL_TIMEOUT"] == "3570" + assert captured["env"]["TERMINAL_MAX_FOREGROUND_TIMEOUT"] == "3570" + assert os.environ["TERMINAL_TIMEOUT"] == "180" + + +def test_default_spawn_preserves_longer_terminal_timeout(kanban_home, monkeypatch): + """Kanban should never lower an explicitly larger terminal timeout.""" + captured = {} + + class FakeProc: + pid = 124 + + def fake_popen(cmd, **kwargs): + captured["env"] = kwargs.get("env", {}) + return FakeProc() + + monkeypatch.setattr("subprocess.Popen", fake_popen) + monkeypatch.setenv("TERMINAL_TIMEOUT", "7200") + monkeypatch.setenv("TERMINAL_MAX_FOREGROUND_TIMEOUT", "7200") + + conn = kb.connect() + try: + tid = kb.create_task( + conn, + title="already tuned", + assignee="ops", + max_runtime_seconds=3600, + ) + task = kb.get_task(conn, tid) + workspace = kb.resolve_workspace(task) + kb._default_spawn(task, str(workspace)) + finally: + conn.close() + + assert captured["env"]["TERMINAL_TIMEOUT"] == "7200" + assert captured["env"]["TERMINAL_MAX_FOREGROUND_TIMEOUT"] == "7200" + + +def test_default_spawn_leaves_terminal_timeout_without_runtime_cap(kanban_home, monkeypatch): + """Uncapped tasks keep the existing terminal timeout behavior.""" + captured = {} + + class FakeProc: + pid = 125 + + def fake_popen(cmd, **kwargs): + captured["env"] = kwargs.get("env", {}) + return FakeProc() + + monkeypatch.setattr("subprocess.Popen", fake_popen) + monkeypatch.setenv("TERMINAL_TIMEOUT", "180") + monkeypatch.delenv("TERMINAL_MAX_FOREGROUND_TIMEOUT", raising=False) + + conn = kb.connect() + try: + tid = kb.create_task(conn, title="uncapped", assignee="ops") + task = kb.get_task(conn, tid) + workspace = kb.resolve_workspace(task) + kb._default_spawn(task, str(workspace)) + finally: + conn.close() + + assert captured["env"]["TERMINAL_TIMEOUT"] == "180" + assert "TERMINAL_MAX_FOREGROUND_TIMEOUT" not in captured["env"] + + +def test_build_worker_context_includes_runtime_timeout_budget(kanban_home, monkeypatch): + monkeypatch.setenv("TERMINAL_TIMEOUT", "180") + conn = kb.connect() + try: + tid = kb.create_task( + conn, + title="long context", + assignee="ops", + max_runtime_seconds=3600, + ) + ctx = kb.build_worker_context(conn, tid) + finally: + conn.close() + + assert "Max runtime: 3600s" in ctx + assert "Terminal timeout: 3570s" in ctx + + # --------------------------------------------------------------------------- # Per-task force-loaded skills From 6e60a8a09225d7395a3bd68246a39654251d7458 Mon Sep 17 00:00:00 2001 From: qWaitCrypto <119617223+qWaitCrypto@users.noreply.github.com> Date: Thu, 14 May 2026 17:46:58 +0800 Subject: [PATCH 11/30] feat(kanban): make worker log retention configurable --- hermes_cli/config.py | 5 ++ hermes_cli/kanban_db.py | 81 ++++++++++++++++--- .../test_kanban_core_functionality.py | 27 +++++++ 3 files changed, 103 insertions(+), 10 deletions(-) diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 6510532a7c..84898623fb 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -1486,6 +1486,11 @@ DEFAULT_CONFIG = { # same task/profile (spawn_failed, timed_out, or crashed). Reassignment # resets the streak for the new profile. "failure_limit": 2, + # Worker stdout/stderr logs rotate at spawn time. Defaults preserve + # the historical 2 MiB + one-backup behavior; long-running workers can + # raise these to keep more early failure evidence. + "worker_log_rotate_bytes": 2 * 1024 * 1024, + "worker_log_backup_count": 1, # Profile that decomposes tasks in the Triage column. When unset, # falls back to the default profile (the one `hermes` launches with # no -p flag). Set this to a dedicated 'orchestrator' profile if you diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index bad382c339..5b5fe456c9 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -3066,6 +3066,7 @@ DEFAULT_SPAWN_FAILURE_LIMIT = DEFAULT_FAILURE_LIMIT # Max bytes to keep in a single worker log file. The dispatcher truncates # and rotates on spawn if the file is larger than this at spawn time. DEFAULT_LOG_ROTATE_BYTES = 2 * 1024 * 1024 # 2 MiB +DEFAULT_LOG_BACKUP_COUNT = 1 # Keep a little wall-clock budget for the worker to observe a terminal timeout # and call kanban_block/kanban_complete before max_runtime_seconds kills it. @@ -4029,25 +4030,84 @@ def dispatch_once( return result -def _rotate_worker_log(log_path: Path, max_bytes: int) -> None: - """Rotate ```` to ``.1`` if it exceeds ``max_bytes``. +def _positive_int(value: Any, default: int, *, minimum: int = 1) -> int: + try: + parsed = int(value) + except (TypeError, ValueError): + return default + return parsed if parsed >= minimum else default - Single-generation rotation — one old file kept, newer one replaces it. - Keeps disk usage bounded while still giving the user a chance to grab - the prior run's output. + +def worker_log_rotation_config(kanban_cfg: Optional[dict] = None) -> tuple[int, int]: + """Return ``(rotate_bytes, backup_count)`` for worker log rotation. + + Defaults preserve the historical behavior: rotate at 2 MiB and keep one + backup generation (``.log.1``). Operators with long-running workers can + raise either value from ``config.yaml`` without changing dispatcher code. + """ + if kanban_cfg is None: + try: + from hermes_cli.config import load_config + + kanban_cfg = (load_config().get("kanban") or {}) + except Exception: + kanban_cfg = {} + max_bytes = _positive_int( + (kanban_cfg or {}).get("worker_log_rotate_bytes"), + DEFAULT_LOG_ROTATE_BYTES, + minimum=1, + ) + backup_count = _positive_int( + (kanban_cfg or {}).get("worker_log_backup_count"), + DEFAULT_LOG_BACKUP_COUNT, + minimum=0, + ) + return max_bytes, backup_count + + +def _rotated_log_path(log_path: Path, generation: int) -> Path: + return log_path.with_suffix(log_path.suffix + f".{generation}") + + +def _rotate_worker_log( + log_path: Path, + max_bytes: int, + backup_count: int = DEFAULT_LOG_BACKUP_COUNT, +) -> None: + """Rotate ```` when it exceeds ``max_bytes``. + + ``backup_count=1`` preserves the legacy single-generation behavior: + ```` moves to ``.1`` and any previous ``.1`` is replaced. + Higher values shift older generations up to ``backup_count``. """ try: if not log_path.exists(): return if log_path.stat().st_size <= max_bytes: return - rotated = log_path.with_suffix(log_path.suffix + ".1") + backup_count = _positive_int( + backup_count, + DEFAULT_LOG_BACKUP_COUNT, + minimum=0, + ) + if backup_count == 0: + log_path.unlink() + return + oldest = _rotated_log_path(log_path, backup_count) try: - if rotated.exists(): - rotated.unlink() + if oldest.exists(): + oldest.unlink() except OSError: pass - log_path.rename(rotated) + for generation in range(backup_count - 1, 0, -1): + src = _rotated_log_path(log_path, generation) + if not src.exists(): + continue + try: + src.rename(_rotated_log_path(log_path, generation + 1)) + except OSError: + pass + log_path.rename(_rotated_log_path(log_path, 1)) except OSError: pass @@ -4232,7 +4292,8 @@ def _default_spawn( log_dir = worker_logs_dir(board=board) log_dir.mkdir(parents=True, exist_ok=True) log_path = log_dir / f"{task.id}.log" - _rotate_worker_log(log_path, DEFAULT_LOG_ROTATE_BYTES) + rotate_bytes, backup_count = worker_log_rotation_config() + _rotate_worker_log(log_path, rotate_bytes, backup_count) # Use 'a' so a re-run on unblock appends rather than overwrites. log_f = open(log_path, "ab") diff --git a/tests/hermes_cli/test_kanban_core_functionality.py b/tests/hermes_cli/test_kanban_core_functionality.py index 879a74dee5..f9e05f99ba 100644 --- a/tests/hermes_cli/test_kanban_core_functionality.py +++ b/tests/hermes_cli/test_kanban_core_functionality.py @@ -679,6 +679,33 @@ def test_worker_log_rotation_keeps_one_generation(kanban_home, tmp_path): assert (log_dir / "t_aaaa.log.1").exists() +def test_worker_log_rotation_keeps_configured_generations(kanban_home): + log_dir = kanban_home / "kanban" / "logs" + log_dir.mkdir(parents=True, exist_ok=True) + target = log_dir / "t_multi.log" + target.write_text("current") + (log_dir / "t_multi.log.1").write_text("one") + (log_dir / "t_multi.log.2").write_text("two") + + kb._rotate_worker_log(target, max_bytes=1, backup_count=3) + + assert not target.exists() + assert (log_dir / "t_multi.log.1").read_text() == "current" + assert (log_dir / "t_multi.log.2").read_text() == "one" + assert (log_dir / "t_multi.log.3").read_text() == "two" + + +def test_worker_log_rotation_config_defaults_and_overrides(): + assert kb.worker_log_rotation_config({}) == ( + kb.DEFAULT_LOG_ROTATE_BYTES, + kb.DEFAULT_LOG_BACKUP_COUNT, + ) + assert kb.worker_log_rotation_config({ + "worker_log_rotate_bytes": 10, + "worker_log_backup_count": 4, + }) == (10, 4) + + def test_read_worker_log_tail(kanban_home): log_dir = kanban_home / "kanban" / "logs" log_dir.mkdir(parents=True, exist_ok=True) From d9fef0c8ab308a6c4258eb1449b40a685925bd67 Mon Sep 17 00:00:00 2001 From: qWaitCrypto <119617223+qWaitCrypto@users.noreply.github.com> Date: Thu, 14 May 2026 17:07:57 +0800 Subject: [PATCH 12/30] fix(kanban): align failure diagnostics with retry limit --- hermes_cli/kanban.py | 13 +++- hermes_cli/kanban_diagnostics.py | 70 +++++++++++++++++---- plugins/kanban/dashboard/plugin_api.py | 6 ++ tests/hermes_cli/test_kanban_diagnostics.py | 60 +++++++++++++++++- 4 files changed, 134 insertions(+), 15 deletions(-) diff --git a/hermes_cli/kanban.py b/hermes_cli/kanban.py index 55b1d4125a..12e3e71e9e 100644 --- a/hermes_cli/kanban.py +++ b/hermes_cli/kanban.py @@ -1393,6 +1393,11 @@ def _cmd_diagnostics(args: argparse.Namespace) -> int: the dashboard uses, so CLI output matches what the UI shows. """ from hermes_cli import kanban_diagnostics as kd + from hermes_cli.config import load_config + + diag_config = kd.config_from_kanban_config( + (load_config().get("kanban") or {}) + ) with kb.connect() as conn: # Either one-task mode or fleet mode. @@ -1406,6 +1411,7 @@ def _cmd_diagnostics(args: argparse.Namespace) -> int: task, kb.list_events(conn, args.task), kb.list_runs(conn, args.task), + config=diag_config, ) } else: @@ -1433,7 +1439,12 @@ def _cmd_diagnostics(args: argparse.Namespace) -> int: diags_by_task = {} for r in rows: tid = r["id"] - dl = kd.compute_task_diagnostics(r, ev_by.get(tid, []), run_by.get(tid, [])) + dl = kd.compute_task_diagnostics( + r, + ev_by.get(tid, []), + run_by.get(tid, []), + config=diag_config, + ) if dl: diags_by_task[tid] = dl diff --git a/hermes_cli/kanban_diagnostics.py b/hermes_cli/kanban_diagnostics.py index 42c0c2043f..2f8b7c8ed0 100644 --- a/hermes_cli/kanban_diagnostics.py +++ b/hermes_cli/kanban_diagnostics.py @@ -230,6 +230,14 @@ def _generic_recovery_actions(task: Any, *, running: bool) -> list[DiagnosticAct RuleFn = Callable[[Any, list[Any], list[Any], int, dict], list[Diagnostic]] +def _positive_int(value: Any, default: int) -> int: + try: + parsed = int(value) + except (TypeError, ValueError): + return default + return parsed if parsed >= 1 else default + + def _rule_hallucinated_cards(task, events, runs, now, cfg) -> list[Diagnostic]: """Blocked-hallucination gate fires: a worker called kanban_complete with created_cards that didn't exist or weren't created by the @@ -319,18 +327,19 @@ def _rule_repeated_failures(task, events, runs, now, cfg) -> list[Diagnostic]: all look the same: the kernel keeps retrying and the operator needs to intervene. - Threshold: cfg["failure_threshold"] (default 3). A threshold of 3 - is one below the circuit-breaker's default (5), so the diagnostic - surfaces BEFORE the breaker trips — giving operators a window to - fix the problem while the dispatcher's still retrying. + Threshold: cfg["failure_threshold"]. Runtime callers should derive + this from ``kanban.failure_limit`` unless the user explicitly set a + diagnostics threshold, so the signal does not lag behind the + dispatcher's circuit breaker. Accepts the legacy ``spawn_failure_threshold`` config key for back-compat. """ - threshold = int(cfg.get( + threshold = _positive_int(cfg.get( "failure_threshold", cfg.get("spawn_failure_threshold", 3), - )) + ), 3) + failure_limit = _positive_int(cfg.get("failure_limit"), threshold) # Read the new unified counter name, with a fallback to the legacy # column name so this rule keeps working against old DB rows the # caller somehow materialised without running the migration. @@ -402,10 +411,9 @@ def _rule_repeated_failures(task, events, runs, now, cfg) -> list[Diagnostic]: f"This task has failed {failures} times in a row " f"(most recent: {outcome_label}). Full last error:\n\n" f"{err_snippet}\n\n" - f"The dispatcher will keep retrying until the consecutive-" - f"failures counter trips the circuit breaker (default 5), " - f"at which point the task auto-blocks. Fix the root cause " - f"and reclaim to retry." + f"The dispatcher circuit breaker is configured for " + f"{failure_limit} consecutive non-success attempts. Fix the " + f"root cause and reclaim or unblock the task to retry." ) else: title = f"Agent {outcome_label} x{failures} (no error recorded)" @@ -427,6 +435,8 @@ def _rule_repeated_failures(task, events, runs, now, cfg) -> list[Diagnostic]: "consecutive_failures": failures, "most_recent_outcome": most_recent_outcome, "last_error": last_err, + "failure_threshold": threshold, + "failure_limit": failure_limit, }, )] @@ -716,9 +726,11 @@ DIAGNOSTIC_KINDS = ( DEFAULT_CONFIG = { - "failure_threshold": 3, + # Match the dispatcher default (kanban.failure_limit) so repeated-failure + # diagnostics do not lag behind the default auto-block threshold. + "failure_threshold": 2, # Legacy alias accepted at read time by _rule_repeated_failures. - "spawn_failure_threshold": 3, + "spawn_failure_threshold": 2, "crash_threshold": 2, "blocked_stale_hours": 24, # Stranded-task threshold. 30 min by default — below that, the @@ -728,6 +740,28 @@ DEFAULT_CONFIG = { } +def config_from_kanban_config(kanban_cfg: Optional[dict]) -> dict: + """Build diagnostics config from the runtime ``kanban`` config section. + + ``kanban.diagnostics.failure_threshold`` remains an explicit override. + Otherwise, derive the repeated-failure threshold from + ``kanban.failure_limit`` so CLI/dashboard diagnostics match the + dispatcher's actual circuit-breaker threshold. + """ + kanban_cfg = kanban_cfg or {} + diag_cfg = dict(kanban_cfg.get("diagnostics") or {}) + diag_cfg.setdefault( + "failure_limit", + kanban_cfg.get("failure_limit", DEFAULT_CONFIG["failure_threshold"]), + ) + if ( + "failure_threshold" not in diag_cfg + and "spawn_failure_threshold" not in diag_cfg + ): + diag_cfg["failure_threshold"] = diag_cfg["failure_limit"] + return diag_cfg + + def compute_task_diagnostics( task, events: list, @@ -743,7 +777,17 @@ def compute_task_diagnostics( most-recent ``last_seen_at``. """ now_ts = int(now if now is not None else time.time()) - cfg = {**DEFAULT_CONFIG, **(config or {})} + config = config or {} + cfg = {**DEFAULT_CONFIG, **config} + if ( + "failure_threshold" not in config + and "spawn_failure_threshold" not in config + and "failure_limit" in config + ): + cfg["failure_threshold"] = _positive_int( + config.get("failure_limit"), + DEFAULT_CONFIG["failure_threshold"], + ) out: list[Diagnostic] = [] for rule in _RULES: try: diff --git a/plugins/kanban/dashboard/plugin_api.py b/plugins/kanban/dashboard/plugin_api.py index 16e6066385..0a4685b4a5 100644 --- a/plugins/kanban/dashboard/plugin_api.py +++ b/plugins/kanban/dashboard/plugin_api.py @@ -224,6 +224,11 @@ def _compute_task_diagnostics( rule definitions. """ from hermes_cli import kanban_diagnostics as kd + from hermes_cli.config import load_config + + diag_config = kd.config_from_kanban_config( + (load_config().get("kanban") or {}) + ) # Build the candidate task list. We need each task's row + its # events + its runs. Doing N separate queries works but scales @@ -270,6 +275,7 @@ def _compute_task_diagnostics( r, events_by_task.get(tid, []), runs_by_task.get(tid, []), + config=diag_config, ) if diags: out[tid] = [d.to_dict() for d in diags] diff --git a/tests/hermes_cli/test_kanban_diagnostics.py b/tests/hermes_cli/test_kanban_diagnostics.py index ad00e4136a..53fdf4fc34 100644 --- a/tests/hermes_cli/test_kanban_diagnostics.py +++ b/tests/hermes_cli/test_kanban_diagnostics.py @@ -177,10 +177,68 @@ def test_repeated_failures_escalates_to_critical(): def test_repeated_failures_below_threshold_silent(): - task = _task(consecutive_failures=2) + task = _task(consecutive_failures=1) assert kd.compute_task_diagnostics(task, [], []) == [] +def test_repeated_failures_default_matches_dispatcher_failure_limit(): + """Default dispatcher auto-blocks at 2 failures, so diagnostics must + also surface at 2 instead of waiting for the stale threshold of 3. + """ + task = _task(status="blocked", consecutive_failures=2, + last_failure_error="elapsed 600s > limit 300s") + runs = [_run(outcome="timed_out", run_id=1)] + diags = kd.compute_task_diagnostics(task, [], runs) + repeated = [d for d in diags if d.kind == "repeated_failures"] + assert len(repeated) == 1 + d = repeated[0] + assert d.data["failure_threshold"] == 2 + assert d.data["failure_limit"] == 2 + assert "default 5" not in d.detail + assert "configured for 2" in d.detail + + +def test_repeated_failures_derives_threshold_from_kanban_failure_limit(): + task = _task(status="ready", consecutive_failures=2, + last_failure_error="Profile 'debugger' does not exist") + runs = [_run(outcome="spawn_failed", run_id=1)] + assert kd.compute_task_diagnostics( + task, [], runs, config={"failure_limit": 4} + ) == [] + + task = _task(status="blocked", consecutive_failures=4, + last_failure_error="Profile 'debugger' does not exist") + diags = kd.compute_task_diagnostics( + task, [], runs, config={"failure_limit": 4} + ) + repeated = [d for d in diags if d.kind == "repeated_failures"] + assert len(repeated) == 1 + assert repeated[0].data["failure_threshold"] == 4 + assert repeated[0].data["failure_limit"] == 4 + + +def test_repeated_failures_explicit_threshold_overrides_failure_limit(): + task = _task(status="ready", consecutive_failures=3, + last_failure_error="Profile 'debugger' does not exist") + runs = [_run(outcome="spawn_failed", run_id=1)] + diags = kd.compute_task_diagnostics( + task, [], runs, config={"failure_limit": 5, "failure_threshold": 3} + ) + repeated = [d for d in diags if d.kind == "repeated_failures"] + assert len(repeated) == 1 + assert repeated[0].data["failure_threshold"] == 3 + assert repeated[0].data["failure_limit"] == 5 + + +def test_config_from_kanban_config_preserves_explicit_diagnostics_threshold(): + cfg = kd.config_from_kanban_config({ + "failure_limit": 5, + "diagnostics": {"failure_threshold": 3}, + }) + assert cfg["failure_threshold"] == 3 + assert cfg["failure_limit"] == 5 + + def test_repeated_crashes_counts_trailing_streak_only(): task = _task(status="ready", assignee="crashy") runs = [ From dadc8aa25580ac1ecc65d6185dfc6bd0e1d6d279 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Mon, 18 May 2026 01:27:06 -0700 Subject: [PATCH 13/30] fix(kanban): surface unusable triage auxiliary model (auto-decompose aware) (#27871) Adds a 'triage_aux_unavailable' diagnostic for tasks stuck in triage when neither the active aux helper slot nor the main-model auto fallback is usable. Auto-decompose aware: - kanban.auto_decompose=True (default): primary is auxiliary.kanban_decomposer, triage_specifier is the fanout=false fallback. - kanban.auto_decompose=False: primary is auxiliary.triage_specifier (manual 'hermes kanban specify' path). Default aux slots use 'provider: auto' which falls back to the main model, so this rule only fires when both the explicit slot config AND the main-model auto fallback are absent. Quiet by default; informative when there is a real config gap. Also adds kd.config_from_runtime_config() that carries kanban + auxiliary + model keys through to diagnostics, and updates CLI/dashboard call sites to use it. config_from_kanban_config() is preserved for back-compat. Reworks the original PR #25640 idea (@qWaitCrypto) to align with the new auto-decompose dispatcher path landed in #27572. The original PR pointed only at auxiliary.triage_specifier, which is now the fallback rather than the primary helper. Co-authored-by: qWaitCrypto --- hermes_cli/kanban.py | 4 +- hermes_cli/kanban_diagnostics.py | 229 ++++++++++++++++++++ plugins/kanban/dashboard/plugin_api.py | 4 +- tests/hermes_cli/test_kanban_diagnostics.py | 124 +++++++++++ 4 files changed, 355 insertions(+), 6 deletions(-) diff --git a/hermes_cli/kanban.py b/hermes_cli/kanban.py index 12e3e71e9e..edaee42f88 100644 --- a/hermes_cli/kanban.py +++ b/hermes_cli/kanban.py @@ -1395,9 +1395,7 @@ def _cmd_diagnostics(args: argparse.Namespace) -> int: from hermes_cli import kanban_diagnostics as kd from hermes_cli.config import load_config - diag_config = kd.config_from_kanban_config( - (load_config().get("kanban") or {}) - ) + diag_config = kd.config_from_runtime_config(load_config()) with kb.connect() as conn: # Either one-task mode or fleet mode. diff --git a/hermes_cli/kanban_diagnostics.py b/hermes_cli/kanban_diagnostics.py index 2f8b7c8ed0..8acd6dd932 100644 --- a/hermes_cli/kanban_diagnostics.py +++ b/hermes_cli/kanban_diagnostics.py @@ -230,6 +230,98 @@ def _generic_recovery_actions(task: Any, *, running: bool) -> list[DiagnosticAct RuleFn = Callable[[Any, list[Any], list[Any], int, dict], list[Diagnostic]] +def _aux_slot_explicit(slot: Any) -> bool: + """Return True if the auxiliary slot has user-supplied non-default fields. + + Defaults from ``DEFAULT_CONFIG`` use ``provider: "auto"`` with empty + model/base_url/api_key — that path falls through to the main model. An + "explicit" config is one where the user actively set a provider (not + "auto"), or supplied a model / base_url / api_key. + """ + if not isinstance(slot, dict): + return False + provider = str(slot.get("provider") or "").strip().lower() + if provider and provider != "auto": + return True + for key in ("model", "base_url", "api_key"): + if str(slot.get(key) or "").strip(): + return True + return False + + +def _main_model_visible(raw_config: Any) -> bool: + """Best-effort check that a main model is configured. + + Diagnostics runs in the dashboard process which may not share the CLI's + runtime state, so we read the raw config dict. If we cannot prove the + main model is set, we err on the side of NOT firing the diagnostic. + """ + if not isinstance(raw_config, dict): + return False + model_cfg = raw_config.get("model") + if isinstance(model_cfg, dict): + provider = str(model_cfg.get("provider") or "").strip() + model = str( + model_cfg.get("default") + or model_cfg.get("model") + or model_cfg.get("name") + or "" + ).strip() + return bool(provider and model) + return bool(str(model_cfg or "").strip()) + + +def triage_aux_status(config: Optional[dict]) -> Optional[dict]: + """Inspect raw config and report whether triage paths look configured. + + Returns ``None`` when config context is unavailable (suppress diagnostic + to avoid noisy false positives in tests / low-level callers). Otherwise + returns a dict with: + + - ``auto_decompose``: bool — whether the dispatcher auto-runs decompose + - ``decomposer_explicit``: bool — user-supplied decomposer slot + - ``specifier_explicit``: bool — user-supplied specifier slot + - ``main_model_visible``: bool — main model can serve as auto fallback + """ + if not isinstance(config, dict): + return None + + explicit = config.get("triage_aux_status") + if isinstance(explicit, dict): + return explicit + + aux = config.get("auxiliary") + kanban_cfg = config.get("kanban") if isinstance(config.get("kanban"), dict) else {} + + # Have we been handed any config context at all? When neither auxiliary + # nor kanban nor model keys are present, the caller is a low-level test + # passing {} — stay silent. + if ( + not isinstance(aux, dict) + and not kanban_cfg + and "model" not in config + ): + return None + + decomposer_explicit = False + specifier_explicit = False + if isinstance(aux, dict): + decomposer_explicit = _aux_slot_explicit(aux.get("kanban_decomposer")) + specifier_explicit = _aux_slot_explicit(aux.get("triage_specifier")) + + # ``auto_decompose`` defaults to True per kanban DEFAULT_CONFIG. + auto_decompose = True + if isinstance(kanban_cfg, dict) and "auto_decompose" in kanban_cfg: + auto_decompose = bool(kanban_cfg.get("auto_decompose")) + + return { + "auto_decompose": auto_decompose, + "decomposer_explicit": decomposer_explicit, + "specifier_explicit": specifier_explicit, + "main_model_visible": _main_model_visible(config), + } + + def _positive_int(value: Any, default: int) -> int: try: parsed = int(value) @@ -285,6 +377,118 @@ def _rule_hallucinated_cards(task, events, runs, now, cfg) -> list[Diagnostic]: )] +def _rule_triage_aux_unavailable(task, events, runs, now, cfg) -> list[Diagnostic]: + """A triage task cannot leave triage without an auxiliary helper. + + With the auto-decompose dispatcher (kanban.auto_decompose, default True), + triage tasks fan out via ``auxiliary.kanban_decomposer`` and fall back to + ``auxiliary.triage_specifier`` when the decomposer returns ``fanout=false``. + With auto-decompose off, the user must run ``hermes kanban specify``, + which only needs ``auxiliary.triage_specifier``. + + The default slot is ``provider: auto`` → auto-falls back to the main model, + so this rule only fires when: + + - the relevant slot is explicitly set to something broken, OR + - the auto fallback has no main model to fall back to. + + Config context is required; pass {} from tests to keep the rule silent. + """ + if _task_field(task, "status") != "triage": + return [] + + status = triage_aux_status(cfg) + if status is None: + return [] + + auto_decompose = bool(status.get("auto_decompose")) + decomposer_explicit = bool(status.get("decomposer_explicit")) + specifier_explicit = bool(status.get("specifier_explicit")) + main_visible = bool(status.get("main_model_visible")) + + # Determine the primary slot and whether it is usable. + if auto_decompose: + primary_slot = "auxiliary.kanban_decomposer" + primary_explicit = decomposer_explicit + fallback_slot = "auxiliary.triage_specifier" + fallback_explicit = specifier_explicit + primary_desc = "decomposer" + detail_path = ( + "Auto-decompose is on, so the dispatcher needs " + "auxiliary.kanban_decomposer (with auxiliary.triage_specifier as " + "a fallback for non-fan-out tasks)." + ) + else: + primary_slot = "auxiliary.triage_specifier" + primary_explicit = specifier_explicit + fallback_slot = "auxiliary.kanban_decomposer" + fallback_explicit = decomposer_explicit + primary_desc = "specifier" + detail_path = ( + "Auto-decompose is off, so triage tasks need " + "`hermes kanban specify`, which uses auxiliary.triage_specifier." + ) + + # The primary slot is usable when either: it was explicitly configured by + # the user, OR the default `provider: auto` can fall back to the main + # model. If both fail, we have a real configuration gap. + if primary_explicit or main_visible: + return [] + + task_id = _task_field(task, "id") or "" + actions = [ + DiagnosticAction( + kind="cli_hint", + label=f"Configure {primary_slot}", + payload={ + "command": ( + f"hermes config set {primary_slot}.provider auto" + ) + }, + suggested=True, + ), + ] + if not fallback_explicit and not main_visible: + actions.append(DiagnosticAction( + kind="cli_hint", + label=f"Or configure fallback {fallback_slot}", + payload={ + "command": ( + f"hermes config set {fallback_slot}.provider auto" + ) + }, + )) + if not auto_decompose: + actions.append(DiagnosticAction( + kind="cli_hint", + label=f"Specify manually: hermes kanban specify {task_id}", + payload={"command": f"hermes kanban specify {task_id}"}, + )) + + return [Diagnostic( + kind="triage_aux_unavailable", + severity="warning", + title=f"Triage {primary_desc} has no usable model", + detail=( + f"This task is still in triage and no working auxiliary model is " + f"visible to the dispatcher. {detail_path} The default slot uses " + f"`provider: auto` which falls back to the main model, but no main " + f"model is configured either. Configure the slot directly or set a " + f"main model so the auto fallback can take over." + ), + actions=actions, + first_seen_at=now, + last_seen_at=now, + count=1, + data={ + "task_id": task_id, + "auto_decompose": auto_decompose, + "primary_slot": primary_slot, + "main_model_visible": main_visible, + }, + )] + + def _rule_prose_phantom_refs(task, events, runs, now, cfg) -> list[Diagnostic]: """Advisory prose-scan: the completion summary mentions ``t_`` ids that don't resolve. Non-blocking; surfaced as a warning only. @@ -705,6 +909,7 @@ def _rule_stranded_in_ready(task, events, runs, now, cfg) -> list[Diagnostic]: # severity ties. Add new rules here. _RULES: list[RuleFn] = [ _rule_hallucinated_cards, + _rule_triage_aux_unavailable, _rule_prose_phantom_refs, _rule_repeated_failures, _rule_repeated_crashes, @@ -717,6 +922,7 @@ _RULES: list[RuleFn] = [ # rules are added. DIAGNOSTIC_KINDS = ( "hallucinated_cards", + "triage_aux_unavailable", "prose_phantom_refs", "repeated_failures", "repeated_crashes", @@ -762,6 +968,29 @@ def config_from_kanban_config(kanban_cfg: Optional[dict]) -> dict: return diag_cfg +def config_from_runtime_config(raw_config: Optional[dict]) -> dict: + """Build diagnostics config from the full Hermes runtime config. + + Carries through ``kanban``, ``auxiliary``, and ``model`` keys so triage- + aware rules can inspect the active aux-helper and main-model state. + Folds the ``kanban`` block through ``config_from_kanban_config`` so the + repeated-failure threshold derivation still applies. + """ + raw_config = raw_config or {} + if not isinstance(raw_config, dict): + return {} + cfg: dict = {} + kanban_cfg = raw_config.get("kanban") + if isinstance(kanban_cfg, dict): + cfg.update(config_from_kanban_config(kanban_cfg)) + cfg["kanban"] = kanban_cfg + for key in ("auxiliary", "model"): + value = raw_config.get(key) + if value is not None: + cfg[key] = value + return cfg + + def compute_task_diagnostics( task, events: list, diff --git a/plugins/kanban/dashboard/plugin_api.py b/plugins/kanban/dashboard/plugin_api.py index 0a4685b4a5..92a9d75366 100644 --- a/plugins/kanban/dashboard/plugin_api.py +++ b/plugins/kanban/dashboard/plugin_api.py @@ -226,9 +226,7 @@ def _compute_task_diagnostics( from hermes_cli import kanban_diagnostics as kd from hermes_cli.config import load_config - diag_config = kd.config_from_kanban_config( - (load_config().get("kanban") or {}) - ) + diag_config = kd.config_from_runtime_config(load_config()) # Build the candidate task list. We need each task's row + its # events + its runs. Doing N separate queries works but scales diff --git a/tests/hermes_cli/test_kanban_diagnostics.py b/tests/hermes_cli/test_kanban_diagnostics.py index 53fdf4fc34..6329825ce1 100644 --- a/tests/hermes_cli/test_kanban_diagnostics.py +++ b/tests/hermes_cli/test_kanban_diagnostics.py @@ -613,3 +613,127 @@ def test_stranded_in_ready_works_on_real_db_row(kanban_home): assert stranded[0].data["assignee"] == "ghost" finally: conn.close() + + + +# --------------------------------------------------------------------------- +# triage_aux_unavailable rule — auto-decompose aware +# --------------------------------------------------------------------------- + + +def _triage_task(): + return _task(id="t_triage1", status="triage") + + +def test_triage_aux_unavailable_silent_without_config_context(): + """Low-level callers passing no config dict should not see this rule.""" + diags = kd.compute_task_diagnostics(_triage_task(), [], []) + assert [d for d in diags if d.kind == "triage_aux_unavailable"] == [] + + +def test_triage_aux_unavailable_silent_when_main_model_visible(): + """Default `provider: auto` falls back to the main model — no warning.""" + config = { + "auxiliary": {}, + "model": {"provider": "openrouter", "default": "qwen/qwen3"}, + "kanban": {"auto_decompose": True}, + } + diags = kd.compute_task_diagnostics(_triage_task(), [], [], config=config) + assert [d for d in diags if d.kind == "triage_aux_unavailable"] == [] + + +def test_triage_aux_unavailable_silent_when_decomposer_explicit(): + """User explicitly configured decomposer → no warning, even without main.""" + config = { + "auxiliary": { + "kanban_decomposer": {"provider": "openrouter", "model": "qwen/qwen3"}, + }, + "kanban": {"auto_decompose": True}, + } + diags = kd.compute_task_diagnostics(_triage_task(), [], [], config=config) + assert [d for d in diags if d.kind == "triage_aux_unavailable"] == [] + + +def test_triage_aux_unavailable_fires_auto_decompose_on_no_fallback(): + """auto_decompose=True, no decomposer, no main model → warn about decomposer.""" + config = { + "auxiliary": {}, + "kanban": {"auto_decompose": True}, + } + diags = kd.compute_task_diagnostics(_triage_task(), [], [], config=config) + triage = [d for d in diags if d.kind == "triage_aux_unavailable"] + assert len(triage) == 1 + d = triage[0] + assert d.severity == "warning" + assert "decomposer" in d.title.lower() + assert d.data["auto_decompose"] is True + assert d.data["primary_slot"] == "auxiliary.kanban_decomposer" + suggested = [a for a in d.actions if a.suggested] + assert suggested + assert "auxiliary.kanban_decomposer" in suggested[0].payload["command"] + + +def test_triage_aux_unavailable_fires_auto_decompose_off_points_at_specifier(): + """auto_decompose=False → primary is specifier, not decomposer.""" + config = { + "auxiliary": {}, + "kanban": {"auto_decompose": False}, + } + diags = kd.compute_task_diagnostics(_triage_task(), [], [], config=config) + triage = [d for d in diags if d.kind == "triage_aux_unavailable"] + assert len(triage) == 1 + d = triage[0] + assert "specifier" in d.title.lower() + assert d.data["auto_decompose"] is False + assert d.data["primary_slot"] == "auxiliary.triage_specifier" + # And it should offer the manual specify command as an action + labels = [a.label for a in d.actions] + assert any("hermes kanban specify" in l for l in labels) + + +def test_triage_aux_unavailable_skips_non_triage_tasks(): + config = {"auxiliary": {}, "kanban": {"auto_decompose": True}} + task = _task(status="todo") + diags = kd.compute_task_diagnostics(task, [], [], config=config) + assert [d for d in diags if d.kind == "triage_aux_unavailable"] == [] + + +def test_triage_aux_status_recognises_auto_default_as_not_explicit(): + """Default `provider: auto` with empty fields → not 'explicit'.""" + status = kd.triage_aux_status({ + "auxiliary": { + "kanban_decomposer": {"provider": "auto", "model": ""}, + }, + "kanban": {}, + }) + assert status is not None + assert status["decomposer_explicit"] is False + + +def test_triage_aux_status_recognises_explicit_model_only(): + """Even with provider=auto, a non-empty model counts as explicit.""" + status = kd.triage_aux_status({ + "auxiliary": { + "kanban_decomposer": {"provider": "auto", "model": "qwen/qwen3"}, + }, + "kanban": {}, + }) + assert status is not None + assert status["decomposer_explicit"] is True + + +def test_config_from_runtime_config_carries_aux_and_model(): + cfg = kd.config_from_runtime_config({ + "kanban": {"failure_limit": 5, "auto_decompose": False}, + "auxiliary": {"kanban_decomposer": {"provider": "openrouter"}}, + "model": {"provider": "openrouter", "default": "qwen/qwen3"}, + }) + assert cfg["failure_threshold"] == 5 + assert cfg["kanban"]["auto_decompose"] is False + assert cfg["auxiliary"]["kanban_decomposer"]["provider"] == "openrouter" + assert cfg["model"]["default"] == "qwen/qwen3" + + +def test_config_from_runtime_config_handles_empty_input(): + assert kd.config_from_runtime_config(None) == {} + assert kd.config_from_runtime_config({}) == {} From f2fdb9a178a0b646d0803ab0789914657dc8c361 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Mon, 18 May 2026 02:14:43 -0700 Subject: [PATCH 14/30] =?UTF-8?q?feat(gateway):=20deliverable=20mode=20?= =?UTF-8?q?=E2=80=94=20ship=20artifacts=20as=20native=20uploads=20from=20a?= =?UTF-8?q?ny=20agent=20surface=20(#27813)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The agent can now produce a chart, PDF, spreadsheet, or any other supported file type and have it land in Slack / Discord / Telegram / WhatsApp / etc. as a native attachment, just by mentioning the absolute path in its response. Same primitive works for kanban-worker completions: workers attach artifacts via kanban_complete(artifacts=[...]) and the gateway notifier uploads them alongside the completion message. Changes: - gateway/platforms/base.py: extract_local_files now covers PDFs, docx, spreadsheets (xlsx/csv/json/yaml), presentations (pptx), archives (zip/tar/gz), audio (mp3/wav/...), and html — not just images and video. Image/video extensions still embed inline; everything else routes to send_document via the existing dispatch partition in gateway/run.py. - tools/kanban_tools.py + hermes_cli/kanban_db.py: kanban_complete gains an explicit ``artifacts`` parameter. The handler stashes it in metadata.artifacts (for downstream workers) and the kernel promotes it onto the completed-event payload so the notifier can find it without a second SQL round-trip. - gateway/run.py: _kanban_notifier_watcher now calls a new helper _deliver_kanban_artifacts after sending the completion text. The helper reads payload.artifacts (preferred), falls back to scanning the payload summary and task.result with extract_local_files, then partitions images / videos / documents and uploads each via send_multiple_images / send_video / send_document. - website/docs/user-guide/features/deliverable-mode.md + sidebars.ts: user-facing docs page covering the extension list, the kanban artifacts pattern, and the MCP-for-connector-breadth recommendation. Tests: - tests/gateway/test_extract_local_files.py: 7 new test cases (documents, spreadsheets, presentations, audio, archives, html, chart-pdf canonical case). 44 passing, 0 regressions. - tests/tools/test_kanban_tools.py: 4 new cases covering the artifacts arg shape (list / string / merge with existing metadata / type rejection). 17 passing. - tests/hermes_cli/test_kanban_notify.py: 2 new cases covering full notifier → artifact-upload path and missing-file silent-skip. 12 passing. - E2E (real files, real kanban kernel, real BasePlatformAdapter): worker calls kanban_complete(artifacts=[png,pdf,csv]) → metadata + event payload land → notifier helper partitions correctly → send_multiple_images called once with the PNG, send_document called twice with PDF + CSV. What's NOT in this PR (deferred to follow-ups): - Ad-hoc "research this for two hours, ping the thread when done" slash command — covered today by kanban subscriptions; a dedicated slash command can ride a follow-up PR if needed. - Setup-wizard prompt for recommended MCP servers (Notion, GitHub, Linear, etc.) — docs page lists them; UI is a separate change. Plan and rationale captured in ~/.hermes/docs/perplexity-computer-parity.pdf (local doc, not shipped). --- gateway/platforms/base.py | 32 +++- gateway/run.py | 127 ++++++++++++++ hermes_cli/kanban_db.py | 14 ++ tests/gateway/test_extract_local_files.py | 63 ++++++- tests/hermes_cli/test_kanban_notify.py | 159 ++++++++++++++++++ tests/tools/test_kanban_tools.py | 87 ++++++++++ tools/kanban_tools.py | 66 +++++++- .../user-guide/features/deliverable-mode.md | 130 ++++++++++++++ website/sidebars.ts | 1 + 9 files changed, 671 insertions(+), 8 deletions(-) create mode 100644 website/docs/user-guide/features/deliverable-mode.md diff --git a/gateway/platforms/base.py b/gateway/platforms/base.py index 96b56d29cc..34ebc385fa 100644 --- a/gateway/platforms/base.py +++ b/gateway/platforms/base.py @@ -2157,12 +2157,20 @@ class BasePlatformAdapter(ABC): @staticmethod def extract_local_files(content: str) -> Tuple[List[str], str]: """ - Detect bare local file paths in response text for native media delivery. + Detect bare local file paths in response text for native delivery. Matches absolute paths (/...) and tilde paths (~/) ending in common - image or video extensions. Validates each candidate with - ``os.path.isfile()`` to avoid false positives from URLs or - non-existent paths. + image, video, audio, or document extensions. Validates each + candidate with ``os.path.isfile()`` to avoid false positives from + URLs or non-existent paths. + + The extension list is broader than just images/video so the agent + can produce arbitrary artifacts (charts, PDFs, spreadsheets, code + archives, CSVs) and have them ship to the user as native uploads + without needing an explicit ``MEDIA:`` tag. Image / video + extensions still embed inline where the platform supports it; + document extensions route through ``send_document``. The dispatch + partition lives in ``gateway/run.py``. Paths inside fenced code blocks (``` ... ```) and inline code (`...`) are ignored so that code samples are never mutilated. @@ -2172,8 +2180,22 @@ class BasePlatformAdapter(ABC): raw path strings removed). """ _LOCAL_MEDIA_EXTS = ( - '.png', '.jpg', '.jpeg', '.gif', '.webp', + # Images (embed inline) + '.png', '.jpg', '.jpeg', '.gif', '.webp', '.bmp', '.tiff', '.svg', + # Video (embed inline where supported) '.mp4', '.mov', '.avi', '.mkv', '.webm', + # Audio (delivered as voice/audio where supported) + '.mp3', '.wav', '.ogg', '.m4a', '.flac', + # Documents (uploaded as file attachments) + '.pdf', '.docx', '.doc', '.odt', '.rtf', '.txt', '.md', + # Spreadsheets / data + '.xlsx', '.xls', '.ods', '.csv', '.tsv', '.json', '.xml', '.yaml', '.yml', + # Presentations + '.pptx', '.ppt', '.odp', '.key', + # Archives + '.zip', '.tar', '.gz', '.tgz', '.bz2', '.xz', '.7z', '.rar', + # Web / rendered output + '.html', '.htm', ) ext_part = '|'.join(e.lstrip('.') for e in _LOCAL_MEDIA_EXTS) diff --git a/gateway/run.py b/gateway/run.py index 623d238af3..e36acf444c 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -4474,6 +4474,29 @@ class GatewayRunner: "kanban notifier: delivered %s event for %s to %s/%s on board %s", kind, sub["task_id"], platform_str, sub["chat_id"], board_slug, ) + # After delivering the text notification, surface + # any artifact paths the worker referenced in + # ``kanban_complete(summary=..., artifacts=[...])`` + # (or the legacy ``result`` field) as native + # uploads. ``extract_local_files`` finds bare + # absolute paths in the summary; + # ``send_document`` / ``send_image_file`` uploads + # them. Only fires on the ``completed`` event so + # we never spam attachments on retries. + if kind == "completed": + try: + await self._deliver_kanban_artifacts( + adapter=adapter, + chat_id=sub["chat_id"], + metadata=metadata, + event_payload=getattr(ev, "payload", None), + task=task, + ) + except Exception as art_exc: + logger.debug( + "kanban notifier: artifact delivery for %s failed: %s", + sub["task_id"], art_exc, + ) # Reset the failure counter on success. sub_fail_counts.pop(sub_key, None) except Exception as exc: @@ -4591,6 +4614,110 @@ class GatewayRunner: finally: conn.close() + async def _deliver_kanban_artifacts( + self, + *, + adapter, + chat_id: str, + metadata: dict, + event_payload: Optional[dict], + task, + ) -> None: + """Upload artifact files referenced by a completed kanban task. + + Workers passing ``kanban_complete(artifacts=[...])`` ship absolute + file paths through the completion event so downstream humans get + the deliverable as a native upload instead of a path printed in + chat. + + Sources scanned, in priority order: + 1. ``event_payload['artifacts']`` (explicit list — preferred) + 2. ``event_payload['summary']`` (truncated first line) + 3. ``task.result`` (legacy fallback) + + Files are deduplicated, missing files are silently skipped (the + path may have been mentioned for reference only), and delivery + errors are logged but do not break the notifier loop. + """ + from pathlib import Path as _Path + + candidates: list[str] = [] + seen: set[str] = set() + + def _add(path: str) -> None: + if not path: + return + expanded = os.path.expanduser(path) + if expanded in seen: + return + if not os.path.isfile(expanded): + return + seen.add(expanded) + candidates.append(expanded) + + # 1. Explicit artifacts list in payload. + if isinstance(event_payload, dict): + raw = event_payload.get("artifacts") + if isinstance(raw, (list, tuple)): + for item in raw: + if isinstance(item, str): + _add(item) + + # 2. Paths embedded in the payload summary. + summary = event_payload.get("summary") + if isinstance(summary, str) and summary: + paths, _ = adapter.extract_local_files(summary) + for p in paths: + _add(p) + + # 3. Legacy: paths embedded in task.result. + if task is not None and getattr(task, "result", None): + result_text = str(task.result) + paths, _ = adapter.extract_local_files(result_text) + for p in paths: + _add(p) + + if not candidates: + return + + _IMAGE_EXTS = {".png", ".jpg", ".jpeg", ".gif", ".webp", ".bmp", ".tiff", ".svg"} + _VIDEO_EXTS = {".mp4", ".mov", ".avi", ".mkv", ".webm", ".3gp"} + + from urllib.parse import quote as _quote + + # Partition images so they ride a single send_multiple_images call + # on platforms that support batch image uploads (Signal/Slack RPCs). + image_paths = [p for p in candidates if _Path(p).suffix.lower() in _IMAGE_EXTS] + other_paths = [p for p in candidates if _Path(p).suffix.lower() not in _IMAGE_EXTS] + + if image_paths: + try: + batch = [(f"file://{_quote(p)}", "") for p in image_paths] + await adapter.send_multiple_images( + chat_id=chat_id, images=batch, metadata=metadata, + ) + except Exception as exc: + logger.warning( + "kanban notifier: image batch upload failed: %s", exc, + ) + + for path in other_paths: + ext = _Path(path).suffix.lower() + try: + if ext in _VIDEO_EXTS: + await adapter.send_video( + chat_id=chat_id, video_path=path, metadata=metadata, + ) + else: + await adapter.send_document( + chat_id=chat_id, file_path=path, metadata=metadata, + ) + except Exception as exc: + logger.warning( + "kanban notifier: artifact upload (%s) failed: %s", + path, exc, + ) + async def _kanban_dispatcher_watcher(self) -> None: """Embedded kanban dispatcher — one tick every `dispatch_interval_seconds`. diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index 5b5fe456c9..4def6fc5d5 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -2479,6 +2479,20 @@ def complete_task( } if verified_cards: completed_payload["verified_cards"] = verified_cards + # Carry artifact paths in the event payload so the gateway + # notifier can upload them as native attachments alongside the + # completion message. Workers pass these via + # ``kanban_complete(artifacts=[...])`` which stashes the list in + # ``metadata["artifacts"]`` — we promote it onto the event so + # consumers don't have to fetch the run row to find it. + if isinstance(metadata, dict): + md_artifacts = metadata.get("artifacts") + if isinstance(md_artifacts, (list, tuple)): + cleaned_artifacts = [ + str(p).strip() for p in md_artifacts if isinstance(p, str) and str(p).strip() + ] + if cleaned_artifacts: + completed_payload["artifacts"] = cleaned_artifacts _append_event( conn, task_id, "completed", completed_payload, diff --git a/tests/gateway/test_extract_local_files.py b/tests/gateway/test_extract_local_files.py index dd93e6370f..568b311cb9 100644 --- a/tests/gateway/test_extract_local_files.py +++ b/tests/gateway/test_extract_local_files.py @@ -74,6 +74,58 @@ class TestBasicDetection: assert len(paths) == 1, f"Failed for {ext}" assert paths[0] == f"/tmp/pic{ext}" + def test_document_extensions(self): + """Documents (PDF, Word, plain text, etc.) ship as file uploads.""" + for ext in (".pdf", ".docx", ".doc", ".odt", ".rtf", ".txt", ".md"): + text = f"Report at /tmp/report{ext} attached" + paths, _ = _extract(text) + assert len(paths) == 1, f"Failed for {ext}" + assert paths[0] == f"/tmp/report{ext}" + + def test_spreadsheet_and_data_extensions(self): + """Spreadsheets and structured data ship as file uploads.""" + for ext in (".xlsx", ".xls", ".csv", ".tsv", ".json", ".xml", ".yaml", ".yml"): + text = f"Data at /tmp/data{ext} ready" + paths, _ = _extract(text) + assert len(paths) == 1, f"Failed for {ext}" + assert paths[0] == f"/tmp/data{ext}" + + def test_presentation_extensions(self): + """Presentations ship as file uploads.""" + for ext in (".pptx", ".ppt", ".odp"): + text = f"Deck at /tmp/deck{ext} done" + paths, _ = _extract(text) + assert len(paths) == 1, f"Failed for {ext}" + assert paths[0] == f"/tmp/deck{ext}" + + def test_audio_extensions(self): + """Audio files are detected and routed by the gateway dispatch.""" + for ext in (".mp3", ".wav", ".ogg", ".m4a", ".flac"): + text = f"Audio at /tmp/sound{ext} ready" + paths, _ = _extract(text) + assert len(paths) == 1, f"Failed for {ext}" + assert paths[0] == f"/tmp/sound{ext}" + + def test_archive_extensions(self): + """Archives ship as file uploads.""" + for ext in (".zip", ".tar", ".gz", ".tgz", ".bz2", ".7z"): + text = f"Archive at /tmp/bundle{ext} ready" + paths, _ = _extract(text) + assert len(paths) == 1, f"Failed for {ext}" + assert paths[0] == f"/tmp/bundle{ext}" + + def test_html_extension(self): + paths, _ = _extract("Open /tmp/report.html in browser") + assert paths == ["/tmp/report.html"] + + def test_chart_pdf_path(self): + """Common case: agent renders a chart via matplotlib and references the file.""" + text = "Here is the comparison chart: /tmp/q3-sales.pdf" + paths, cleaned = _extract(text) + assert paths == ["/tmp/q3-sales.pdf"] + assert "/tmp/q3-sales.pdf" not in cleaned + assert "comparison chart" in cleaned + def test_case_insensitive_extension(self): paths, _ = _extract("See /tmp/PHOTO.PNG and /tmp/vid.MP4 now") assert len(paths) == 2 @@ -269,8 +321,15 @@ class TestEdgeCases: assert cleaned == "" def test_no_media_extensions(self): - """Non-media extensions should not be matched.""" - paths, _ = _extract("See /tmp/data.csv and /tmp/script.py and /tmp/notes.txt") + """Extensions outside the supported list should not be matched. + + ``.py`` and ``.log`` are intentionally excluded because (a) most + source files are quoted in inline code or fenced blocks anyway, + and (b) auto-shipping arbitrary source files would be a + surprise. Documents (.pdf, .docx), data (.csv, .json), + archives (.zip), and presentations (.pptx) ARE matched. + """ + paths, _ = _extract("See /tmp/script.py and /tmp/server.log here") assert paths == [] def test_path_with_spaces_not_matched(self): diff --git a/tests/hermes_cli/test_kanban_notify.py b/tests/hermes_cli/test_kanban_notify.py index ddfa4b40aa..1ebf92705d 100644 --- a/tests/hermes_cli/test_kanban_notify.py +++ b/tests/hermes_cli/test_kanban_notify.py @@ -479,3 +479,162 @@ async def test_gateway_create_autosubscribes_on_explicit_board(kanban_home): assert kb.list_notify_subs(conn) == [] finally: conn.close() + + +@pytest.mark.asyncio +async def test_notifier_uploads_artifacts_on_completion(kanban_home, tmp_path): + """When a completed event carries ``artifacts`` in its payload, the + notifier uploads each file to the subscribed chat as a native + attachment. Images batch through send_multiple_images; documents + route through send_document. See the artifacts wiring in + gateway/run.py._deliver_kanban_artifacts. + """ + import hermes_cli.kanban_db as kb + from gateway.run import GatewayRunner + from gateway.config import Platform + from tools import kanban_tools as kt + + # Materialize real files so os.path.isfile passes inside the helper. + chart_path = tmp_path / "q3-revenue.png" + chart_path.write_bytes(b"PNG-fake-bytes") + report_path = tmp_path / "report.pdf" + report_path.write_bytes(b"%PDF-fake") + + conn = kb.connect() + try: + tid = kb.create_task(conn, title="render q3 chart", assignee="worker1") + kb.add_notify_sub(conn, task_id=tid, platform="telegram", chat_id="chat1") + finally: + conn.close() + + # Use the production handler so we exercise the full path: tool args + # → metadata.artifacts → event payload promotion. + import os + os.environ["HERMES_KANBAN_TASK"] = tid + try: + out = kt._handle_complete({ + "summary": "rendered the chart", + "artifacts": [str(chart_path), str(report_path)], + }) + finally: + os.environ.pop("HERMES_KANBAN_TASK", None) + import json as _json + assert _json.loads(out)["ok"] is True + + runner = object.__new__(GatewayRunner) + runner._running = True + runner._kanban_sub_fail_counts = {} + + fake_adapter = MagicMock() + fake_adapter.name = "telegram" + + sends: list = [] + images_uploaded: list = [] + documents_uploaded: list = [] + + async def _send(chat_id, msg, metadata=None): + sends.append((chat_id, msg)) + runner._running = False + + async def _send_images(chat_id, images, metadata=None, **_kw): + images_uploaded.extend(p for p, _ in images) + + async def _send_document(chat_id, file_path, metadata=None, **_kw): + documents_uploaded.append(file_path) + + fake_adapter.send = AsyncMock(side_effect=_send) + fake_adapter.send_multiple_images = AsyncMock(side_effect=_send_images) + fake_adapter.send_document = AsyncMock(side_effect=_send_document) + # extract_local_files is used internally for legacy path fallback; + # the real BasePlatformAdapter implementation lives there, so wire it. + from gateway.platforms.base import BasePlatformAdapter + fake_adapter.extract_local_files = BasePlatformAdapter.extract_local_files + + runner.adapters = {Platform.TELEGRAM: fake_adapter} + + _orig_sleep = asyncio.sleep + + async def _fast_sleep(_): + await _orig_sleep(0) + + with patch("gateway.run.asyncio.sleep", side_effect=_fast_sleep): + await asyncio.wait_for( + runner._kanban_notifier_watcher(interval=1), + timeout=10.0, + ) + + # The text completion notification fired. + assert len(sends) == 1 + # The PNG rode the image-batch path. + assert any("q3-revenue.png" in p for p in images_uploaded), images_uploaded + # The PDF rode the document path. + assert any("report.pdf" in p for p in documents_uploaded), documents_uploaded + + +@pytest.mark.asyncio +async def test_notifier_artifact_delivery_skips_missing_files(kanban_home, tmp_path): + """Missing artifact paths are silently skipped — they may have been + referenced by name only. The notifier must not crash and must still + deliver any artifacts that do exist.""" + import hermes_cli.kanban_db as kb + from gateway.run import GatewayRunner + from gateway.config import Platform + from tools import kanban_tools as kt + + real_pdf = tmp_path / "real.pdf" + real_pdf.write_bytes(b"%PDF-fake") + + conn = kb.connect() + try: + tid = kb.create_task(conn, title="t", assignee="worker1") + kb.add_notify_sub(conn, task_id=tid, platform="telegram", chat_id="chat1") + finally: + conn.close() + + import os + os.environ["HERMES_KANBAN_TASK"] = tid + try: + kt._handle_complete({ + "summary": "one real, one ghost", + "artifacts": [str(real_pdf), "/tmp/definitely-does-not-exist.pdf"], + }) + finally: + os.environ.pop("HERMES_KANBAN_TASK", None) + + runner = object.__new__(GatewayRunner) + runner._running = True + runner._kanban_sub_fail_counts = {} + + fake_adapter = MagicMock() + fake_adapter.name = "telegram" + + documents_uploaded: list = [] + + async def _send(chat_id, msg, metadata=None): + runner._running = False + + async def _send_document(chat_id, file_path, metadata=None, **_kw): + documents_uploaded.append(file_path) + + fake_adapter.send = AsyncMock(side_effect=_send) + fake_adapter.send_document = AsyncMock(side_effect=_send_document) + fake_adapter.send_multiple_images = AsyncMock() + from gateway.platforms.base import BasePlatformAdapter + fake_adapter.extract_local_files = BasePlatformAdapter.extract_local_files + + runner.adapters = {Platform.TELEGRAM: fake_adapter} + + _orig_sleep = asyncio.sleep + + async def _fast_sleep(_): + await _orig_sleep(0) + + with patch("gateway.run.asyncio.sleep", side_effect=_fast_sleep): + await asyncio.wait_for( + runner._kanban_notifier_watcher(interval=1), + timeout=10.0, + ) + + # Only the real file was uploaded. + assert len(documents_uploaded) == 1 + assert "real.pdf" in documents_uploaded[0] diff --git a/tests/tools/test_kanban_tools.py b/tests/tools/test_kanban_tools.py index c31ae6f08b..1dbd72ad93 100644 --- a/tests/tools/test_kanban_tools.py +++ b/tests/tools/test_kanban_tools.py @@ -318,6 +318,93 @@ def test_complete_with_result_only(worker_env): assert d["ok"] is True +def test_complete_with_artifacts_lands_in_event_payload(worker_env): + """``artifacts=[...]`` rides into the completed event payload so the + gateway notifier can upload them as native attachments. See the + kanban notifier in gateway/run.py for the consumer side.""" + from hermes_cli import kanban_db as kb + from tools import kanban_tools as kt + + out = kt._handle_complete({ + "summary": "rendered the chart", + "artifacts": ["/tmp/q3-revenue.png", "/tmp/q3-report.pdf"], + }) + assert json.loads(out)["ok"] is True + + conn = kb.connect() + try: + events = kb.list_events(conn, worker_env) + # Find the completion event + completed = [e for e in events if e.kind == "completed"] + assert len(completed) == 1 + payload = completed[0].payload or {} + assert payload.get("artifacts") == [ + "/tmp/q3-revenue.png", + "/tmp/q3-report.pdf", + ] + # And the artifacts also live on metadata for downstream workers + run = kb.latest_run(conn, worker_env) + assert run.metadata.get("artifacts") == [ + "/tmp/q3-revenue.png", + "/tmp/q3-report.pdf", + ] + finally: + conn.close() + + +def test_complete_artifacts_accepts_single_string(worker_env): + """A bare string is auto-promoted to a single-element list for convenience.""" + from hermes_cli import kanban_db as kb + from tools import kanban_tools as kt + + out = kt._handle_complete({ + "summary": "one chart", + "artifacts": "/tmp/chart.png", + }) + assert json.loads(out)["ok"] is True + + conn = kb.connect() + try: + run = kb.latest_run(conn, worker_env) + assert run.metadata.get("artifacts") == ["/tmp/chart.png"] + finally: + conn.close() + + +def test_complete_artifacts_merges_with_explicit_metadata_field(worker_env): + """If the worker passes metadata.artifacts AND the top-level artifacts + param, merge the two without duplicates.""" + from hermes_cli import kanban_db as kb + from tools import kanban_tools as kt + + out = kt._handle_complete({ + "summary": "merged", + "metadata": {"artifacts": ["/tmp/a.png"], "other": "fact"}, + "artifacts": ["/tmp/b.pdf", "/tmp/a.png"], + }) + assert json.loads(out)["ok"] is True + + conn = kb.connect() + try: + run = kb.latest_run(conn, worker_env) + # Order: existing entries first, then new ones, deduplicated. + assert run.metadata.get("artifacts") == ["/tmp/a.png", "/tmp/b.pdf"] + assert run.metadata.get("other") == "fact" + finally: + conn.close() + + +def test_complete_rejects_non_list_artifacts(worker_env): + """Non-list, non-string artifacts should be rejected with a clear error.""" + from tools import kanban_tools as kt + out = kt._handle_complete({ + "summary": "bad shape", + "artifacts": {"not": "a list"}, + }) + err = json.loads(out).get("error", "") + assert "artifacts must be a list" in err + + def test_complete_rejects_no_handoff(worker_env): from tools import kanban_tools as kt out = kt._handle_complete({}) diff --git a/tools/kanban_tools.py b/tools/kanban_tools.py index fab0a68c92..eaf32a3a37 100644 --- a/tools/kanban_tools.py +++ b/tools/kanban_tools.py @@ -371,6 +371,7 @@ def _handle_complete(args: dict, **kw) -> str: metadata = args.get("metadata") result = args.get("result") created_cards = args.get("created_cards") + artifacts = args.get("artifacts") if created_cards is not None: if isinstance(created_cards, str): # Accept a single id as a string for convenience. @@ -384,6 +385,45 @@ def _handle_complete(args: dict, **kw) -> str: created_cards = [ str(c).strip() for c in created_cards if str(c).strip() ] + if artifacts is not None: + if isinstance(artifacts, str): + # Accept a single path as a string for convenience. + artifacts = [artifacts] + if not isinstance(artifacts, (list, tuple)): + return tool_error( + f"artifacts must be a list of file paths, got " + f"{type(artifacts).__name__}" + ) + artifacts = [ + str(p).strip() for p in artifacts if str(p).strip() + ] + # Carry the artifact list inside metadata so it rides the + # existing completed-event payload without a schema change at + # the DB layer. The gateway notifier reads payload['artifacts'] + # off the completion event and uploads each path as a native + # attachment. + if artifacts: + if metadata is None: + metadata = {} + elif not isinstance(metadata, dict): + return tool_error( + f"metadata must be an object/dict, got " + f"{type(metadata).__name__}" + ) + # Don't overwrite an existing metadata.artifacts the worker + # passed manually — merge instead. + existing = metadata.get("artifacts") + if isinstance(existing, (list, tuple)): + merged: list[str] = [] + seen: set[str] = set() + for item in list(existing) + artifacts: + s = str(item).strip() + if s and s not in seen: + seen.add(s) + merged.append(s) + metadata["artifacts"] = merged + else: + metadata["artifacts"] = artifacts if not (summary or result): return tool_error( "provide at least one of: summary (preferred), result" @@ -760,7 +800,12 @@ KANBAN_COMPLETE_SCHEMA = { "tasks via ``kanban_create`` during this run, list their ids " "in ``created_cards`` — the kernel verifies them so phantom " "references are caught before they leak into downstream " - "automation." + "automation. If you produced deliverable files (charts, PDFs, " + "spreadsheets, generated images), list their absolute paths " + "in ``artifacts`` — the gateway notifier will upload them as " + "native attachments to the human who subscribed to the task, " + "so the deliverable lands in their chat alongside the summary " + "instead of being a path they have to fetch by hand." ), "parameters": { "type": "object", @@ -811,6 +856,25 @@ KANBAN_COMPLETE_SCHEMA = { "did not create any cards." ), }, + "artifacts": { + "type": "array", + "items": {"type": "string"}, + "description": ( + "Optional list of absolute paths to deliverable " + "files you produced during this run — generated " + "charts, PDFs, spreadsheets, images, archives. " + "Examples: [\"/tmp/q3-revenue.png\", " + "\"/tmp/report.pdf\"]. The gateway notifier " + "uploads each path as a native attachment to the " + "subscribed chat (images embed inline, everything " + "else uploads as a file) so the deliverable " + "lands with the completion notification. Skip " + "intermediate scratch files and references that " + "are not the deliverable. The path must exist " + "on disk when the notifier runs; missing files " + "are silently skipped." + ), + }, }, "required": [], }, diff --git a/website/docs/user-guide/features/deliverable-mode.md b/website/docs/user-guide/features/deliverable-mode.md new file mode 100644 index 0000000000..e08e3966fa --- /dev/null +++ b/website/docs/user-guide/features/deliverable-mode.md @@ -0,0 +1,130 @@ +--- +title: Deliverable Mode (Artifacts in Chat) +sidebar_label: Deliverable Mode +description: How the agent ships generated charts, PDFs, spreadsheets, and other files as native attachments in messaging platforms. +--- + +# Deliverable Mode + +When Hermes Agent runs inside a messaging gateway (Slack, Discord, Telegram, +WhatsApp, Signal, etc.), it can deliver generated files directly into the +chat — not as paths the user has to copy, but as native attachments. + +A chart shows up as an inline image. A PDF report shows up as a file +download. A spreadsheet uploads as `.xlsx`. The agent does not need to +write a `MEDIA:` tag or do anything special — it just generates the file +and mentions its absolute path in the response. The gateway picks the path +out of the text, removes it from the visible message, and uploads the +file natively. + +## How it works + +Three pieces fit together: + +1. **The agent has tools that produce files.** `execute_code` for charts via + matplotlib, the `latex-pdf-report` skill for PDFs, the `powerpoint` skill + for decks, `image_generate` for images, `text_to_speech` for audio, and so + on. + +2. **The gateway scans agent responses for file paths.** Any absolute path + (`/tmp/...`) or home-relative path (`~/...`) ending in a supported + extension gets extracted. Paths inside code blocks and inline code are + ignored so code samples are never mutilated. + +3. **The gateway dispatches by file type.** Images embed inline where the + platform supports it; videos embed inline; audio routes to voice/audio + attachments; everything else uploads as a file attachment. + +## Supported file extensions + +| Category | Extensions | Delivery | +|---|---|---| +| Images | `.png .jpg .jpeg .gif .webp .bmp .tiff .svg` | Inline embed | +| Video | `.mp4 .mov .avi .mkv .webm` | Inline embed (where supported) | +| Audio | `.mp3 .wav .ogg .m4a .flac` | Voice / audio attachment | +| Documents | `.pdf .docx .doc .odt .rtf .txt .md` | File upload | +| Data | `.xlsx .xls .csv .tsv .json .xml .yaml .yml` | File upload | +| Presentations | `.pptx .ppt .odp` | File upload | +| Archives | `.zip .tar .gz .tgz .bz2 .7z` | File upload | +| Web | `.html .htm` | File upload | + +`.py`, `.log`, and other source-file extensions are intentionally excluded so +the agent doesn't auto-ship arbitrary source files; if you want to send code +to the user, use a code block. + +## Encouraging the agent to produce artifacts + +The agent doesn't reach for artifacts by default — it has to know to. +Two ways to nudge it: + +**Per-session:** ask explicitly ("send me the comparison as a chart", +"return the data as a CSV") or write your own custom-instructions / +personality entry that biases toward artifact-style replies on +messaging platforms. + +**Project-level:** add the bias to `AGENTS.md` / `CLAUDE.md` / +`.cursorrules` in a project the agent works from, or to your global +custom instructions in `~/.hermes/config.yaml` under `agent.custom_instructions`. + +The mechanic the agent has to use is simple: render the file to an +absolute path (e.g. `/tmp/q3-revenue.png`) and mention that path as +plain text in the reply. The gateway does the rest. Paths inside +fenced code blocks or backticks are ignored so code samples are never +mutilated. + +## Kanban: artifacts ride completion notifications + +If you use Hermes' kanban multi-agent workflow, workers can attach +deliverable files to their `kanban_complete` call: + +```python +kanban_complete( + summary="rendered Q3 revenue chart and report", + artifacts=[ + "/tmp/q3-revenue.png", + "/tmp/q3-report.pdf", + ], +) +``` + +When the gateway notifier delivers the "task completed" message to whoever +subscribed to the task in Slack/Telegram/etc., it also uploads each artifact +as a native attachment to that chat. The human gets the deliverable and the +summary in one place. + +Files that don't exist on disk when the notifier runs are silently skipped. + +## Connecting more services with MCP + +Beyond the artifact-delivery pipeline, the agent can reach into other +services via MCP (Model Context Protocol). The MCP ecosystem ships +community servers for most popular tools — install whichever you need: + +| Service | What it unlocks | +|---|---| +| **Notion** | Read/write Notion pages, databases, query workspace | +| **GitHub** | Issues, PRs, comments, repo search beyond the gh CLI | +| **Linear** | Tickets, projects, cycles | +| **Slack** | Workspace-wide search, read other channels | +| **Gmail** | Inbox triage, send mail, label management | +| **Salesforce** | Leads, opportunities, account data | +| **Snowflake / BigQuery** | SQL against data warehouses | +| **Google Drive** | File search, contents, share management | + +Install MCP servers via `~/.hermes/config.yaml` under the `mcp_servers` +section. See [MCP integration](./mcp.md) for the full setup guide. + +## Comparison to Perplexity Computer in Slack + +Perplexity Computer's Slack integration is built around the same idea: +the agent generates a deliverable (chart, PDF, slide deck) and posts it +back into the thread as a native attachment. Hermes Agent's deliverable +mode provides the same user-facing pattern locally: + +- Generation happens in the user's own venv / sandbox (no remote tenant). +- Files land in the chat via the same Slack `files.uploadV2` API. +- Connector breadth comes via MCP rather than a curated catalog of 400 + hosted integrations — install the ones you actually use. + +OAuth tokens stay on the user's machine in `auth.json` / `.env`. No hosted +token storage. No multi-tenant microVM. Same end result. diff --git a/website/sidebars.ts b/website/sidebars.ts index 1a0aa6fb0b..7ca300c9d5 100644 --- a/website/sidebars.ts +++ b/website/sidebars.ts @@ -89,6 +89,7 @@ const sidebars: SidebarsConfig = { 'user-guide/features/vision', 'user-guide/features/image-generation', 'user-guide/features/tts', + 'user-guide/features/deliverable-mode', ], }, { From 6f5ec929a187739b0b06d2935cac4dc7537ac22c Mon Sep 17 00:00:00 2001 From: Siddharth Balyan <52913345+alt-glitch@users.noreply.github.com> Date: Mon, 18 May 2026 16:34:10 +0530 Subject: [PATCH 15/30] feat(config): add install-method stamping + Docker detection (#27843) * feat(config): add install-method stamping + Docker detection Dockerfile stamps "docker", install.sh stamps "git", and cmd_postinstall stamps "pip" into ~/.hermes/.install_method. detect_install_method() reads the stamp first, then falls back to managed-system / container / .git heuristics. Adds Docker upgrade guidance. Tracking: #27826 * fix(stamp): move Docker stamp to entrypoint, install.sh stamp after print_success The Dockerfile stamp was overwritten by the VOLUME overlay at container start. Moving it to entrypoint.sh ensures it persists. The install.sh stamp now writes after print_success so it only lands on full success. --- Dockerfile | 1 + docker/entrypoint.sh | 3 ++ hermes_cli/config.py | 41 +++++++++++++++++-- hermes_cli/main.py | 3 ++ scripts/install.sh | 2 + .../hermes_cli/test_pip_install_detection.py | 31 ++++++++++++-- 6 files changed, 74 insertions(+), 7 deletions(-) diff --git a/Dockerfile b/Dockerfile index bde3412ed7..6e8f020963 100644 --- a/Dockerfile +++ b/Dockerfile @@ -115,5 +115,6 @@ RUN uv pip install --no-cache-dir --no-deps -e "." ENV HERMES_WEB_DIST=/opt/hermes/hermes_cli/web_dist ENV HERMES_HOME=/opt/data ENV PATH="/opt/data/.local/bin:${PATH}" +RUN mkdir -p /opt/data VOLUME [ "/opt/data" ] ENTRYPOINT [ "/usr/bin/tini", "-g", "--", "/opt/hermes/docker/entrypoint.sh" ] diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh index 09e870543a..9af045e226 100755 --- a/docker/entrypoint.sh +++ b/docker/entrypoint.sh @@ -61,6 +61,9 @@ fi # --- Running as hermes from here --- source "${INSTALL_DIR}/.venv/bin/activate" +# Stamp install method for detect_install_method() +echo "docker" > "${HERMES_HOME:=/opt/data}/.install_method" 2>/dev/null || true + # Create essential directory structure. Cache and platform directories # (cache/images, cache/audio, platforms/whatsapp, etc.) are created on # demand by the application — don't pre-create them here so new installs diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 84898623fb..e69c51a4d3 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -188,21 +188,42 @@ def is_managed() -> bool: return get_managed_system() is not None +_NIX_UPDATE_MSG = "Update your Nix flake input and rebuild (e.g. nix flake update, nixos-rebuild, or home-manager switch)" + + def get_managed_update_command() -> Optional[str]: """Return the preferred upgrade command for a managed install.""" managed_system = get_managed_system() if managed_system == "Homebrew": return "brew upgrade hermes-agent" if managed_system == "NixOS": - return "sudo nixos-rebuild switch" + return _NIX_UPDATE_MSG return None def detect_install_method(project_root: Optional[Path] = None) -> str: - """Detect how Hermes was installed: 'nixos', 'homebrew', 'git', or 'pip'.""" + """Detect how Hermes was installed: 'docker', 'nixos', 'homebrew', 'git', or 'pip'. + + Resolution order: + 1. Stamped ``~/.hermes/.install_method`` file (written by installers) + 2. HERMES_MANAGED env / .managed marker (NixOS, Homebrew) + 3. Container detection (/.dockerenv, /run/.containerenv, cgroup) + 4. .git directory presence -> 'git' + 5. Fallback -> 'pip' + """ + stamp = get_hermes_home() / ".install_method" + try: + method = stamp.read_text(encoding="utf-8").strip().lower() + if method: + return method + except OSError: + pass managed = get_managed_system() if managed: return managed.lower().replace(" ", "-") + from hermes_constants import is_container + if is_container(): + return "docker" if project_root is None: project_root = Path(__file__).parent.parent.resolve() if (project_root / ".git").is_dir(): @@ -210,12 +231,24 @@ def detect_install_method(project_root: Optional[Path] = None) -> str: return "pip" +def stamp_install_method(method: str) -> None: + """Write the install method to ~/.hermes/.install_method.""" + stamp = get_hermes_home() / ".install_method" + try: + stamp.parent.mkdir(parents=True, exist_ok=True) + stamp.write_text(method + "\n", encoding="utf-8") + except OSError: + pass + + def recommended_update_command_for_method(method: str) -> str: - """Return the update command for a given install method.""" + """Return the update command or guidance for a given install method.""" if method == "nixos": - return "sudo nixos-rebuild switch" + return _NIX_UPDATE_MSG if method == "homebrew": return "brew upgrade hermes-agent" + if method == "docker": + return "docker pull nousresearch/hermes-agent:latest" if method == "pip": import shutil uv = shutil.which("uv") diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 575835b2c7..fe28754367 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -1735,8 +1735,11 @@ def cmd_setup(args): def cmd_postinstall(args): """One-shot bootstrap for pip users: install non-Python deps + run setup.""" + from hermes_cli.config import stamp_install_method from hermes_cli.dep_ensure import ensure_dependency + stamp_install_method("pip") + print("⚕ Hermes post-install bootstrap") print() diff --git a/scripts/install.sh b/scripts/install.sh index 9b1b7469bb..c34c64267c 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -1996,6 +1996,8 @@ main() { maybe_start_gateway print_success + + echo "git" > "$HERMES_HOME/.install_method" } if [ -n "$ENSURE_DEPS" ]; then diff --git a/tests/hermes_cli/test_pip_install_detection.py b/tests/hermes_cli/test_pip_install_detection.py index b0f4cbd75a..da3dd35e32 100644 --- a/tests/hermes_cli/test_pip_install_detection.py +++ b/tests/hermes_cli/test_pip_install_detection.py @@ -4,7 +4,8 @@ from unittest.mock import patch def test_pip_install_detected_when_no_git_dir(tmp_path): """When PROJECT_ROOT has no .git, detect as pip install.""" - with patch("hermes_cli.config.get_managed_system", return_value=None): + with patch("hermes_cli.config.get_managed_system", return_value=None), \ + patch("hermes_cli.config.get_hermes_home", return_value=tmp_path): from hermes_cli.config import detect_install_method method = detect_install_method(project_root=tmp_path) assert method == "pip" @@ -13,7 +14,8 @@ def test_pip_install_detected_when_no_git_dir(tmp_path): def test_git_install_detected_when_git_dir_exists(tmp_path): """When PROJECT_ROOT has .git, detect as git install.""" (tmp_path / ".git").mkdir() - with patch("hermes_cli.config.get_managed_system", return_value=None): + with patch("hermes_cli.config.get_managed_system", return_value=None), \ + patch("hermes_cli.config.get_hermes_home", return_value=tmp_path): from hermes_cli.config import detect_install_method method = detect_install_method(project_root=tmp_path) assert method == "git" @@ -22,7 +24,8 @@ def test_git_install_detected_when_git_dir_exists(tmp_path): def test_managed_install_takes_precedence(tmp_path): """When HERMES_MANAGED is set, that takes precedence over git detection.""" (tmp_path / ".git").mkdir() - with patch("hermes_cli.config.get_managed_system", return_value="NixOS"): + with patch("hermes_cli.config.get_managed_system", return_value="NixOS"), \ + patch("hermes_cli.config.get_hermes_home", return_value=tmp_path): from hermes_cli.config import detect_install_method method = detect_install_method(project_root=tmp_path) assert method == "nixos" @@ -35,3 +38,25 @@ def test_recommended_update_command_pip(): assert "pip install" in cmd or "uv pip install" in cmd assert "--upgrade" in cmd assert "hermes-agent" in cmd + + +def test_stamp_file_takes_precedence(tmp_path): + (tmp_path / ".git").mkdir() + (tmp_path / ".install_method").write_text("docker\n") + with patch("hermes_cli.config.get_managed_system", return_value=None), \ + patch("hermes_cli.config.get_hermes_home", return_value=tmp_path): + from hermes_cli.config import detect_install_method + assert detect_install_method(project_root=tmp_path) == "docker" + + +def test_docker_detected_via_dockerenv(tmp_path): + with patch("hermes_cli.config.get_managed_system", return_value=None), \ + patch("hermes_cli.config.get_hermes_home", return_value=tmp_path), \ + patch("hermes_constants.is_container", return_value=True): + from hermes_cli.config import detect_install_method + assert detect_install_method(project_root=tmp_path) == "docker" + + +def test_recommended_update_command_docker(): + from hermes_cli.config import recommended_update_command_for_method + assert "docker pull" in recommended_update_command_for_method("docker") From e3a254d65b1b83d9ee75d4591113fa65a7f3a13d Mon Sep 17 00:00:00 2001 From: Siddharth Balyan <52913345+alt-glitch@users.noreply.github.com> Date: Mon, 18 May 2026 16:34:24 +0530 Subject: [PATCH 16/30] =?UTF-8?q?feat(dep=5Fensure):=20complete=20Windows?= =?UTF-8?q?=20bootstrap=20=E2=80=94=20dep=5Fensure=20+=20install.ps1=20+?= =?UTF-8?q?=20detection=20(#27845)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(dep_ensure): complete Windows bootstrap — dep_ensure + install.ps1 + detection dep_ensure.py gains Windows awareness: PowerShell invocation, platform- specific browser detection, (path, shell) tuple returns. install.ps1 gains -Ensure/-PostInstall modes using npm -g --prefix (aligned with install.sh) and agent-browser install for Chromium. browser_tool.py gains node/ in candidate dirs for Windows .cmd shims. Both install scripts bundled in pip wheel. Tracking: #27826 * fix(install.ps1): add --ignore-scripts to npm install for camofox @askjo/camofox-browser has a dependency (impit) whose postinstall script runs `npx only-allow pnpm`, which fails under npm. Adding --ignore-scripts avoids the spurious failure without affecting functionality. Tracking: #27826 * fix: remove duplicate install scripts from git CI already copies scripts/install.{sh,ps1} into hermes_cli/scripts/ during wheel build. No need to commit copies — .gitignore keeps them out, _find_install_script() falls back to scripts/ for git-clone users. Tracking: #27826 * fix: address review — remove env_extra, fix ps1 error handling - Remove unused env_extra parameter from ensure_dependency() - Invoke-EnsureMode node case now uses Test-Node consistently - Install-AgentBrowser uses throw instead of exit 1 --- .github/workflows/upload_to_pypi.yml | 3 +- hermes_cli/dep_ensure.py | 89 ++++++++++++--- pyproject.toml | 2 +- scripts/install.ps1 | 160 ++++++++++++++++++++++++++- tests/hermes_cli/test_dep_ensure.py | 134 ++++++++++++++++++++-- tools/browser_tool.py | 9 +- 6 files changed, 368 insertions(+), 29 deletions(-) diff --git a/.github/workflows/upload_to_pypi.yml b/.github/workflows/upload_to_pypi.yml index 95477ccf01..86e7ae477b 100644 --- a/.github/workflows/upload_to_pypi.yml +++ b/.github/workflows/upload_to_pypi.yml @@ -71,10 +71,11 @@ jobs: test -f hermes_cli/web_dist/index.html || { echo "ERROR: web_dist not built"; exit 1; } test -f hermes_cli/tui_dist/entry.js || { echo "ERROR: tui_dist not built"; exit 1; } - - name: Bundle install.sh into wheel + - name: Bundle install scripts into wheel run: | mkdir -p hermes_cli/scripts cp scripts/install.sh hermes_cli/scripts/install.sh + cp scripts/install.ps1 hermes_cli/scripts/install.ps1 - name: Build wheel and sdist run: uv build --sdist --wheel diff --git a/hermes_cli/dep_ensure.py b/hermes_cli/dep_ensure.py index 1067b428f7..848e402396 100644 --- a/hermes_cli/dep_ensure.py +++ b/hermes_cli/dep_ensure.py @@ -16,11 +16,14 @@ browser tool needs agent-browser). from __future__ import annotations import os +import platform import shutil import subprocess import sys from pathlib import Path +_IS_WINDOWS = platform.system() == "Windows" + _DEP_CHECKS = { "node": lambda: shutil.which("node") is not None, "browser": lambda: ( @@ -41,7 +44,11 @@ _DEP_DESCRIPTIONS = { def _has_system_browser() -> bool: - for name in ("google-chrome", "google-chrome-stable", "chromium", "chromium-browser", "chrome"): + if _IS_WINDOWS: + names = ("chrome", "msedge", "chromium") + else: + names = ("google-chrome", "google-chrome-stable", "chromium", "chromium-browser", "chrome") + for name in names: if shutil.which(name): return True return False @@ -49,39 +56,67 @@ def _has_system_browser() -> bool: def _has_hermes_agent_browser() -> bool: from hermes_constants import get_hermes_home - return (get_hermes_home() / "node_modules" / ".bin" / "agent-browser").is_file() + home = get_hermes_home() + if _IS_WINDOWS: + # npm -g --prefix puts .cmd shims directly in the prefix dir on Windows + return (home / "node" / "agent-browser.cmd").is_file() + # install.sh installs globally into $HERMES_HOME/node/bin/ via npm -g --prefix + # Also check legacy node_modules/.bin/ path for git-clone installs. + return ( + (home / "node" / "bin" / "agent-browser").is_file() + or (home / "node_modules" / ".bin" / "agent-browser").is_file() + ) def _find_install_script( package_dir: Path | None = None, repo_root: Path | None = None, -) -> Path | None: - """Locate install.sh — bundled in wheel or in git checkout.""" +) -> tuple[Path | None, str | None]: + """Locate the install script — bundled in wheel or in git checkout. + + On Windows, prefers install.ps1; on POSIX, prefers install.sh. + Returns a (path, shell) tuple, or (None, None) if neither is found. + """ if package_dir is None: package_dir = Path(__file__).parent if repo_root is None: repo_root = package_dir.parent - bundled = package_dir / "scripts" / "install.sh" - if bundled.is_file(): - return bundled - repo = repo_root / "scripts" / "install.sh" - if repo.is_file(): - return repo - return None + if _IS_WINDOWS: + preferred = ("install.ps1", "powershell") + fallback = ("install.sh", "bash") + else: + preferred = ("install.sh", "bash") + fallback = ("install.ps1", "powershell") + + for script_name, shell in (preferred, fallback): + bundled = package_dir / "scripts" / script_name + if bundled.is_file(): + return bundled, shell + repo = repo_root / "scripts" / script_name + if repo.is_file(): + return repo, shell + + return None, None -def ensure_dependency(dep: str, interactive: bool = True) -> bool: +def ensure_dependency( + dep: str, + interactive: bool = True, +) -> bool: """Ensure a non-Python dependency is available. Returns True if available.""" check = _DEP_CHECKS.get(dep) - if check and check(): + if check is None: + # Unknown dep — don't silently forward to install script. + return False + if check(): return True - script = _find_install_script() + script, shell = _find_install_script() if script is None: if interactive: desc = _DEP_DESCRIPTIONS.get(dep, dep) - print(f" {desc} is not installed and install.sh was not found.") + print(f" {desc} is not installed and no install script was found.") print(f" Install {dep} manually and try again.") return False @@ -91,12 +126,30 @@ def ensure_dependency(dep: str, interactive: bool = True) -> bool: reply = input(f"{desc} is not installed. Install now? [Y/n] ").strip().lower() except (EOFError, KeyboardInterrupt): return False - if reply not in {"", "y", "yes"}: + if reply not in ("", "y", "yes"): return False + if shell == "powershell": + from hermes_constants import get_hermes_home + ps_bin = shutil.which("powershell") or shutil.which("pwsh") + if not ps_bin: + if interactive: + print(" PowerShell not found. Install PowerShell or run install.ps1 manually.") + return False + cmd = [ + ps_bin, + "-ExecutionPolicy", "Bypass", + "-File", str(script), + "-Ensure", dep, + "-HermesHome", str(get_hermes_home()), + ] + else: + cmd = ["bash", str(script), "--ensure", dep] + + run_env = {**os.environ, "IS_INTERACTIVE": "false"} result = subprocess.run( - ["bash", str(script), "--ensure", dep], - env={**os.environ, "IS_INTERACTIVE": "false"}, + cmd, + env=run_env, ) if result.returncode != 0: return False diff --git a/pyproject.toml b/pyproject.toml index ba66d0da71..cb3c515e02 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -210,7 +210,7 @@ hermes-acp = "acp_adapter.entry:main" py-modules = ["run_agent", "model_tools", "toolsets", "batch_runner", "trajectory_compressor", "toolset_distributions", "cli", "hermes_bootstrap", "hermes_constants", "hermes_state", "hermes_time", "hermes_logging", "utils"] [tool.setuptools.package-data] -hermes_cli = ["web_dist/**/*"] +hermes_cli = ["web_dist/**/*", "tui_dist/**/*", "scripts/install.sh", "scripts/install.ps1"] gateway = ["assets/**/*"] [tool.setuptools.packages.find] diff --git a/scripts/install.ps1 b/scripts/install.ps1 index c774e9a860..7fb618eca6 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -28,7 +28,11 @@ param( [string]$Stage, [switch]$ProtocolVersion, [switch]$NonInteractive, - [switch]$Json + [switch]$Json, + + # --- Ensure mode (dep_ensure.py entry point) --- + [string]$Ensure = "", + [switch]$PostInstall ) $ErrorActionPreference = "Stop" @@ -108,6 +112,105 @@ function Write-Err { Write-Host "[X] $Message" -ForegroundColor Red } +# --- Ensure-mode helpers --- + +function Resolve-NpmCmd { + $npmCmd = Get-Command npm -ErrorAction SilentlyContinue + if (-not $npmCmd) { return $null } + $npmExe = $npmCmd.Source + if ($npmExe -like "*.ps1") { + $npmCmdSibling = Join-Path (Split-Path $npmExe -Parent) "npm.cmd" + if (Test-Path $npmCmdSibling) { return $npmCmdSibling } + } + return $npmExe +} + +function Find-SystemBrowser { + $candidates = @( + "${env:ProgramFiles}\Google\Chrome\Application\chrome.exe", + "${env:ProgramFiles(x86)}\Google\Chrome\Application\chrome.exe", + "${env:LOCALAPPDATA}\Google\Chrome\Application\chrome.exe", + "${env:ProgramFiles}\Microsoft\Edge\Application\msedge.exe", + "${env:ProgramFiles(x86)}\Microsoft\Edge\Application\msedge.exe", + "${env:ProgramFiles}\Chromium\Application\chrome.exe", + "${env:LOCALAPPDATA}\Chromium\Application\chrome.exe" + ) + foreach ($p in $candidates) { + if (Test-Path $p) { return $p } + } + return $null +} + +function Write-BrowserEnv { + param([string]$BrowserPath) + if (-not (Test-Path $HermesHome)) { + New-Item -ItemType Directory -Force -Path $HermesHome | Out-Null + } + $envFile = Join-Path $HermesHome ".env" + if (-not (Test-Path $envFile)) { + Set-Content -Path $envFile -Value "AGENT_BROWSER_EXECUTABLE_PATH=$BrowserPath" -Encoding UTF8 + return + } + $content = Get-Content $envFile -Raw -ErrorAction SilentlyContinue + if ($content -and $content -match "AGENT_BROWSER_EXECUTABLE_PATH=") { return } + Add-Content -Path $envFile -Value "AGENT_BROWSER_EXECUTABLE_PATH=$BrowserPath" -Encoding UTF8 +} + +function Install-AgentBrowser { + param([switch]$SkipChromium) + $npm = Resolve-NpmCmd + if (-not $npm) { + Write-Err "npm not found -- install Node.js first" + throw "npm not found" + } + + Write-Info "Installing agent-browser via npm -g --prefix..." + $prefixDir = Join-Path $HermesHome "node" + if (-not (Test-Path $prefixDir)) { + New-Item -ItemType Directory -Path $prefixDir -Force | Out-Null + } + $npmLog = [System.IO.Path]::GetTempFileName() + $prevEAP = $ErrorActionPreference + $ErrorActionPreference = "Continue" + & $npm install -g --prefix $prefixDir --silent --ignore-scripts "agent-browser@^0.26.0" "@askjo/camofox-browser@^1.5.2" 2>&1 | Tee-Object -FilePath $npmLog | Out-Null + $npmExit = $LASTEXITCODE + $ErrorActionPreference = $prevEAP + if ($npmExit -ne 0) { + $npmDetail = Get-Content $npmLog -Raw -ErrorAction SilentlyContinue + Remove-Item $npmLog -Force -ErrorAction SilentlyContinue + Write-Err "npm install -g failed (exit $npmExit): $npmDetail" + throw "npm install failed" + } + Remove-Item $npmLog -Force -ErrorAction SilentlyContinue + + if (-not $SkipChromium) { + $sysBrowser = Find-SystemBrowser + if ($sysBrowser) { + Write-BrowserEnv -BrowserPath $sysBrowser + Write-Info "System browser detected -- skipping Chromium download" + } else { + $abExe = Join-Path $prefixDir "agent-browser.cmd" + if (Test-Path $abExe) { + Write-Info "Installing Chromium via agent-browser install..." + $abLog = [System.IO.Path]::GetTempFileName() + $prevEAP = $ErrorActionPreference + $ErrorActionPreference = "Continue" + & $abExe install 2>&1 | Tee-Object -FilePath $abLog | Out-Null + $abExit = $LASTEXITCODE + $ErrorActionPreference = $prevEAP + if ($abExit -ne 0) { + $abDetail = Get-Content $abLog -Raw -ErrorAction SilentlyContinue + Write-Warn "Chromium install failed (exit $abExit): $abDetail" + } + Remove-Item $abLog -Force -ErrorAction SilentlyContinue + } else { + Write-Warn "agent-browser.cmd not found at $abExe" + } + } + } + Write-Success "Agent-browser ready" +} + # ============================================================================ # Dependency checks # ============================================================================ @@ -2043,6 +2146,48 @@ function Invoke-AllStages { } } +function Invoke-EnsureMode { + param([string]$Deps) + $depList = $Deps -split "," + foreach ($dep in $depList) { + $dep = $dep.Trim() + switch ($dep) { + "node" { + [void](Test-Node) + if (-not $script:HasNode) { + Write-Err "Node.js could not be installed" + exit 1 + } + } + "browser" { + [void](Test-Node) + if ($script:HasNode) { + Install-AgentBrowser + } else { + Write-Err "Node.js is required for browser tools but could not be installed" + exit 1 + } + } + "ripgrep" { + Write-Info "ripgrep: install manually on Windows (scoop install ripgrep)" + } + "ffmpeg" { + Write-Info "ffmpeg: install manually on Windows (scoop install ffmpeg)" + } + default { + Write-Err "Unknown dependency: $dep" + exit 1 + } + } + } +} + +function Invoke-PostInstallMode { + Write-Info "Running post-install setup..." + Invoke-EnsureMode -Deps "node,browser" + Write-Info "Post-install complete" +} + function Main { Write-Banner Invoke-AllStages @@ -2062,6 +2207,19 @@ function Main { # structured JSON error frame instead of a bare exception. try { + if ($Ensure -ne "") { + if ($PSBoundParameters.ContainsKey("Stage")) { + Write-Err "Cannot use -Ensure and -Stage simultaneously" + exit 1 + } + Invoke-EnsureMode -Deps $Ensure + exit 0 + } + if ($PostInstall) { + Invoke-PostInstallMode + exit 0 + } + if ($ProtocolVersion) { Write-Output $InstallStageProtocolVersion exit 0 diff --git a/tests/hermes_cli/test_dep_ensure.py b/tests/hermes_cli/test_dep_ensure.py index c980c29009..77fee5b7ec 100644 --- a/tests/hermes_cli/test_dep_ensure.py +++ b/tests/hermes_cli/test_dep_ensure.py @@ -16,7 +16,7 @@ def test_ensure_dependency_returns_false_when_missing_noninteractive(): from hermes_cli.dep_ensure import ensure_dependency with patch("hermes_cli.dep_ensure.shutil") as mock_shutil: mock_shutil.which.return_value = None - with patch("hermes_cli.dep_ensure._find_install_script", return_value=None): + with patch("hermes_cli.dep_ensure._find_install_script", return_value=(None, None)): result = ensure_dependency("node", interactive=False) assert result is False @@ -27,9 +27,11 @@ def test_find_install_script_from_checkout(tmp_path): scripts_dir = tmp_path / "scripts" scripts_dir.mkdir() (scripts_dir / "install.sh").write_text("#!/bin/bash", encoding="utf-8") - result = _find_install_script(package_dir=tmp_path / "hermes_cli", repo_root=tmp_path) - assert result is not None - assert result.name == "install.sh" + with patch("hermes_cli.dep_ensure._IS_WINDOWS", False): + path, shell = _find_install_script(package_dir=tmp_path / "hermes_cli", repo_root=tmp_path) + assert path is not None + assert path.name == "install.sh" + assert shell == "bash" def test_find_install_script_from_wheel(tmp_path): @@ -38,6 +40,124 @@ def test_find_install_script_from_wheel(tmp_path): bundled = tmp_path / "hermes_cli" / "scripts" bundled.mkdir(parents=True) (bundled / "install.sh").write_text("#!/bin/bash", encoding="utf-8") - result = _find_install_script(package_dir=tmp_path / "hermes_cli", repo_root=tmp_path) - assert result is not None - assert result.name == "install.sh" + with patch("hermes_cli.dep_ensure._IS_WINDOWS", False): + path, shell = _find_install_script(package_dir=tmp_path / "hermes_cli", repo_root=tmp_path) + assert path is not None + assert path.name == "install.sh" + assert shell == "bash" + + +def test_find_install_script_prefers_ps1_on_windows(tmp_path): + """On Windows, _find_install_script should find install.ps1.""" + scripts_dir = tmp_path / "hermes_cli" / "scripts" + scripts_dir.mkdir(parents=True) + (scripts_dir / "install.ps1").write_text("# fake") + (scripts_dir / "install.sh").write_text("# fake") + from hermes_cli.dep_ensure import _find_install_script + with patch("hermes_cli.dep_ensure._IS_WINDOWS", True): + path, shell = _find_install_script(package_dir=tmp_path / "hermes_cli") + assert path == scripts_dir / "install.ps1" + assert shell == "powershell" + + +def test_find_install_script_returns_sh_on_posix(tmp_path): + """On POSIX, _find_install_script should find install.sh.""" + scripts_dir = tmp_path / "hermes_cli" / "scripts" + scripts_dir.mkdir(parents=True) + (scripts_dir / "install.ps1").write_text("# fake") + (scripts_dir / "install.sh").write_text("# fake") + from hermes_cli.dep_ensure import _find_install_script + with patch("hermes_cli.dep_ensure._IS_WINDOWS", False): + path, shell = _find_install_script(package_dir=tmp_path / "hermes_cli") + assert path == scripts_dir / "install.sh" + assert shell == "bash" + + +def test_find_install_script_falls_back_to_repo_root(tmp_path): + """When no bundled script, check repo root.""" + repo_root = tmp_path / "repo" + (repo_root / "scripts").mkdir(parents=True) + (repo_root / "scripts" / "install.sh").write_text("# fake") + from hermes_cli.dep_ensure import _find_install_script + with patch("hermes_cli.dep_ensure._IS_WINDOWS", False): + path, shell = _find_install_script(package_dir=tmp_path / "hermes_cli", repo_root=repo_root) + assert path == repo_root / "scripts" / "install.sh" + assert shell == "bash" + + +def test_find_install_script_returns_none_when_missing(tmp_path): + from hermes_cli.dep_ensure import _find_install_script + with patch("hermes_cli.dep_ensure._IS_WINDOWS", False): + result = _find_install_script(package_dir=tmp_path / "x", repo_root=tmp_path / "y") + assert result == (None, None) + + +def test_has_system_browser_checks_windows_names(): + from hermes_cli.dep_ensure import _has_system_browser + with patch("hermes_cli.dep_ensure._IS_WINDOWS", True), \ + patch("hermes_cli.dep_ensure.shutil") as mock_shutil: + mock_shutil.which.side_effect = lambda name: "/fake/msedge.exe" if name == "msedge" else None + assert _has_system_browser() is True + + +def test_has_system_browser_checks_posix_names(): + from hermes_cli.dep_ensure import _has_system_browser + with patch("hermes_cli.dep_ensure._IS_WINDOWS", False), \ + patch("hermes_cli.dep_ensure.shutil") as mock_shutil: + mock_shutil.which.return_value = None + assert _has_system_browser() is False + + +def test_has_hermes_agent_browser_windows_path(tmp_path): + node_dir = tmp_path / "node" + node_dir.mkdir(parents=True) + (node_dir / "agent-browser.cmd").write_text("@echo off") + from hermes_cli.dep_ensure import _has_hermes_agent_browser + with patch("hermes_cli.dep_ensure._IS_WINDOWS", True), \ + patch("hermes_constants.get_hermes_home", return_value=tmp_path): + assert _has_hermes_agent_browser() is True + + +def test_has_hermes_agent_browser_posix_path(tmp_path): + bin_dir = tmp_path / "node" / "bin" + bin_dir.mkdir(parents=True) + (bin_dir / "agent-browser").write_text("#!/bin/sh") + from hermes_cli.dep_ensure import _has_hermes_agent_browser + with patch("hermes_cli.dep_ensure._IS_WINDOWS", False), \ + patch("hermes_constants.get_hermes_home", return_value=tmp_path): + assert _has_hermes_agent_browser() is True + + +def test_has_hermes_agent_browser_legacy_node_modules_path(tmp_path): + """Legacy git-clone installs put agent-browser in $HERMES_HOME/node_modules/.bin/.""" + bin_dir = tmp_path / "node_modules" / ".bin" + bin_dir.mkdir(parents=True) + (bin_dir / "agent-browser").write_text("#!/bin/sh") + from hermes_cli.dep_ensure import _has_hermes_agent_browser + with patch("hermes_cli.dep_ensure._IS_WINDOWS", False), \ + patch("hermes_constants.get_hermes_home", return_value=tmp_path): + assert _has_hermes_agent_browser() is True + + +def test_ensure_dependency_uses_powershell_on_windows(tmp_path): + from hermes_cli.dep_ensure import ensure_dependency + scripts_dir = tmp_path / "scripts" + scripts_dir.mkdir(parents=True) + (scripts_dir / "install.ps1").write_text("# fake") + with patch("hermes_cli.dep_ensure._IS_WINDOWS", True), \ + patch("hermes_cli.dep_ensure._DEP_CHECKS", {"node": lambda: False}), \ + patch("hermes_cli.dep_ensure._find_install_script", return_value=(scripts_dir / "install.ps1", "powershell")), \ + patch("hermes_cli.dep_ensure.shutil") as mock_shutil, \ + patch("hermes_constants.get_hermes_home", return_value=tmp_path / "fakehome"), \ + patch("subprocess.run") as mock_run, \ + patch("sys.stdin") as mock_stdin: + mock_shutil.which.side_effect = lambda name: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe" if name == "powershell" else None + mock_stdin.isatty.return_value = False + mock_run.return_value = type("R", (), {"returncode": 0})() + ensure_dependency("node", interactive=False) + cmd = mock_run.call_args[0][0] + assert "powershell" in cmd[0].lower() + assert "-Ensure" in cmd + assert cmd[cmd.index("-Ensure") + 1] == "node" + assert "-HermesHome" in cmd + assert str(tmp_path / "fakehome") in cmd diff --git a/tools/browser_tool.py b/tools/browser_tool.py index fb96649cb3..447f650071 100644 --- a/tools/browser_tool.py +++ b/tools/browser_tool.py @@ -158,8 +158,9 @@ def _browser_candidate_path_dirs() -> list[str]: """Return ordered browser CLI PATH candidates shared by discovery and execution.""" hermes_home = get_hermes_home() hermes_node_bin = str(hermes_home / "node" / "bin") + hermes_node_root = str(hermes_home / "node") hermes_nm_bin = str(hermes_home / "node_modules" / ".bin") - return [hermes_node_bin, hermes_nm_bin, *list(_discover_homebrew_node_dirs()), *_SANE_PATH_DIRS] + return [hermes_node_bin, hermes_node_root, hermes_nm_bin, *list(_discover_homebrew_node_dirs()), *_SANE_PATH_DIRS] def _merge_browser_path(existing_path: str = "") -> str: @@ -1827,6 +1828,12 @@ def _find_agent_browser() -> str: if not recheck: hermes_nm = str(get_hermes_home() / "node_modules" / ".bin") recheck = shutil.which("agent-browser", path=hermes_nm) + if not recheck: + hermes_node_bin = str(get_hermes_home() / "node" / "bin") + recheck = shutil.which("agent-browser", path=hermes_node_bin) + if not recheck: + hermes_node_root = str(get_hermes_home() / "node") + recheck = shutil.which("agent-browser", path=hermes_node_root) if recheck: _cached_agent_browser = recheck _agent_browser_resolved = True From d9b6f75c0b0ffa3cdb3cbe63de3a8e1a5aa44e8f Mon Sep 17 00:00:00 2001 From: Siddharth Balyan <52913345+alt-glitch@users.noreply.github.com> Date: Mon, 18 May 2026 16:36:26 +0530 Subject: [PATCH 17/30] refactor(bootstrap): consolidate ACP browser bootstrap into install.{sh,ps1} (#27851) * refactor(bootstrap): consolidate ACP browser bootstrap into install.{sh,ps1} Delete 687 lines of duplicated browser bootstrap code from acp_adapter/bootstrap/. All browser installation now routes through dep_ensure -> install.{sh,ps1} --ensure, using agent-browser install for Chromium. install.sh gains ensure_browser() with macOS app-bundle detection and per-distro guidance. Tracking: #27826 * fix(install.sh): add --ignore-scripts to npm install for camofox @askjo/camofox-browser has a dependency (impit) whose postinstall script runs `npx only-allow pnpm`, which fails under npm. Adding --ignore-scripts avoids the spurious failure without affecting functionality. Tracking: #27826 * fix: add explicit return in ensure_browser, narrow exception in entry.py ensure_browser() now returns 0 explicitly on all success paths. _run_setup_browser() catches OSError instead of broad Exception, letting ImportError propagate as a real packaging bug. --- acp_adapter/bootstrap/__init__.py | 0 .../bootstrap/bootstrap_browser_tools.ps1 | 288 ------------- .../bootstrap/bootstrap_browser_tools.sh | 399 ------------------ acp_adapter/entry.py | 63 +-- scripts/install.sh | 110 +++-- tests/acp/test_entry.py | 111 ++--- 6 files changed, 140 insertions(+), 831 deletions(-) delete mode 100644 acp_adapter/bootstrap/__init__.py delete mode 100644 acp_adapter/bootstrap/bootstrap_browser_tools.ps1 delete mode 100755 acp_adapter/bootstrap/bootstrap_browser_tools.sh diff --git a/acp_adapter/bootstrap/__init__.py b/acp_adapter/bootstrap/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/acp_adapter/bootstrap/bootstrap_browser_tools.ps1 b/acp_adapter/bootstrap/bootstrap_browser_tools.ps1 deleted file mode 100644 index f840fd2d55..0000000000 --- a/acp_adapter/bootstrap/bootstrap_browser_tools.ps1 +++ /dev/null @@ -1,288 +0,0 @@ -# bootstrap_browser_tools.ps1 — install agent-browser + Playwright Chromium -# into ~/.hermes/node/ for use by Hermes Agent's browser tools on Windows. -# -# Targets the registry-install path: users who got Hermes via -# `uvx --from 'hermes-agent[acp]==X' hermes-acp` don't have a repo clone, -# so the install.ps1 `npm install`-in-repo flow doesn't apply. This script -# is a self-contained, idempotent slice of install.ps1's browser block. -# -# Usage: -# .\bootstrap_browser_tools.ps1 # use defaults -# .\bootstrap_browser_tools.ps1 -Yes # accept Chromium download -# .\bootstrap_browser_tools.ps1 -SkipChromium # Node + agent-browser only -# -# Idempotent: re-running this is safe and fast. - -[CmdletBinding()] -param( - [switch]$Yes, - [switch]$SkipChromium -) - -$ErrorActionPreference = "Stop" -$NodeVersion = "22" - -# ───────────────────────────────────────────────────────────────────────── -# Logging -# ───────────────────────────────────────────────────────────────────────── - -function Write-Info { param([string]$msg) Write-Host "[*] $msg" -ForegroundColor Cyan } -function Write-Success { param([string]$msg) Write-Host "[+] $msg" -ForegroundColor Green } -function Write-Warn { param([string]$msg) Write-Host "[!] $msg" -ForegroundColor Yellow } -function Write-Err { param([string]$msg) Write-Host "[x] $msg" -ForegroundColor Red } - -# ───────────────────────────────────────────────────────────────────────── -# Paths -# ───────────────────────────────────────────────────────────────────────── - -$HermesHome = $env:HERMES_HOME -if (-not $HermesHome) { - $HermesHome = Join-Path $env:USERPROFILE ".hermes" -} -$NodePrefix = Join-Path $HermesHome "node" - -# ───────────────────────────────────────────────────────────────────────── -# Step 1: Node.js -# ───────────────────────────────────────────────────────────────────────── - -function Resolve-NpmExe { - # Same gotcha as install.ps1: prefer npm.cmd over npm.ps1 so the - # PowerShell execution policy doesn't block us. - $cmd = Get-Command npm -ErrorAction SilentlyContinue - if (-not $cmd) { return $null } - $npmExe = $cmd.Source - if ($npmExe -like "*.ps1") { - $sibling = Join-Path (Split-Path $npmExe -Parent) "npm.cmd" - if (Test-Path $sibling) { return $sibling } - } - return $npmExe -} - -function Resolve-NpxExe { - $cmd = Get-Command npx -ErrorAction SilentlyContinue - if (-not $cmd) { return $null } - $npxExe = $cmd.Source - if ($npxExe -like "*.ps1") { - $sibling = Join-Path (Split-Path $npxExe -Parent) "npx.cmd" - if (Test-Path $sibling) { return $sibling } - } - return $npxExe -} - -function Ensure-Node { - # System Node on PATH? - $sysNode = Get-Command node -ErrorAction SilentlyContinue - if ($sysNode) { - try { - $v = & $sysNode.Source --version - $major = [int]($v -replace '^v(\d+).*', '$1') - if ($major -ge 20) { - Write-Success "Node.js $v found on PATH" - return - } - Write-Warn "Node.js $v is older than v20 — installing managed Node." - } catch { - Write-Warn "Failed to query Node version: $_" - } - } - - # Hermes-managed Node? - $managedNode = Join-Path $NodePrefix "node.exe" - if (Test-Path $managedNode) { - $v = & $managedNode --version - Write-Success "Node.js $v found (Hermes-managed at $NodePrefix)" - # Prepend to current-process PATH so subsequent npm/npx calls find it. - $env:PATH = "$NodePrefix;$env:PATH" - return - } - - Write-Info "Installing Node.js $NodeVersion LTS into $NodePrefix ..." - - $arch = if ([Environment]::Is64BitOperatingSystem) { "x64" } else { "x86" } - $indexUrl = "https://nodejs.org/dist/latest-v${NodeVersion}.x/" - - try { - $indexPage = Invoke-WebRequest -Uri $indexUrl -UseBasicParsing - $matches = [regex]::Matches($indexPage.Content, "node-v${NodeVersion}\.\d+\.\d+-win-${arch}\.zip") - if ($matches.Count -eq 0) { - Write-Err "Could not locate Node.js $NodeVersion zip for win-$arch" - throw "no tarball" - } - $zipName = $matches[0].Value - $zipUrl = "$indexUrl$zipName" - - $tmpDir = Join-Path $env:TEMP "hermes-node-$([guid]::NewGuid().ToString('N'))" - New-Item -ItemType Directory -Force -Path $tmpDir | Out-Null - $zipPath = Join-Path $tmpDir $zipName - - Write-Info "Downloading $zipName ..." - Invoke-WebRequest -Uri $zipUrl -OutFile $zipPath -UseBasicParsing - - Expand-Archive -Path $zipPath -DestinationPath $tmpDir -Force - $extracted = Get-ChildItem -Path $tmpDir -Directory | Where-Object { $_.Name -like "node-v*" } | Select-Object -First 1 - - if (-not $extracted) { Write-Err "Node.js extraction failed"; throw "extract" } - - if (Test-Path $NodePrefix) { Remove-Item -Recurse -Force $NodePrefix } - New-Item -ItemType Directory -Force -Path $HermesHome | Out-Null - Move-Item -Path $extracted.FullName -Destination $NodePrefix - - Remove-Item -Recurse -Force $tmpDir -ErrorAction SilentlyContinue - - $env:PATH = "$NodePrefix;$env:PATH" - $v = & "$NodePrefix\node.exe" --version - Write-Success "Node.js $v installed to $NodePrefix" - } catch { - Write-Err "Node.js install failed: $_" - Write-Info "Install Node 20+ manually from https://nodejs.org/en/download/ and re-run." - throw - } -} - -# ───────────────────────────────────────────────────────────────────────── -# Step 2: agent-browser -# ───────────────────────────────────────────────────────────────────────── - -function Ensure-AgentBrowser { - $npmExe = Resolve-NpmExe - if (-not $npmExe) { - Write-Err "npm not on PATH after Node install — aborting" - throw "npm missing" - } - - # Already installed? - $existing = Get-Command agent-browser -ErrorAction SilentlyContinue - if ($existing) { - Write-Success "agent-browser already installed at $($existing.Source)" - return - } - - # When the user has system Node (winget / installer-based), `npm install - # -g` writes to a directory that may require admin rights. Force the - # prefix to the user-writable Hermes-managed Node directory so we never - # need elevation and the agent can always find the result. Mirrors the - # bash bootstrap's `--prefix $NODE_PREFIX` strategy. - New-Item -ItemType Directory -Force -Path $NodePrefix | Out-Null - - Write-Info "Installing agent-browser (npm, prefix=$NodePrefix)..." - & $npmExe install -g --prefix $NodePrefix --silent ` - "agent-browser@^0.26.0" "@askjo/camofox-browser@^1.5.2" - if ($LASTEXITCODE -ne 0) { - Write-Err "npm install -g agent-browser failed (exit $LASTEXITCODE)" - throw "npm install" - } - - # Windows npm global installs drop shims at $NodePrefix\ root (not bin/). - # Prepend to PATH so any subsequent npx call resolves them. - $env:PATH = "$NodePrefix;$env:PATH" - - Write-Success "agent-browser installed to $NodePrefix" -} - -# ───────────────────────────────────────────────────────────────────────── -# Step 3: Playwright Chromium -# ───────────────────────────────────────────────────────────────────────── - -function Find-SystemBrowser { - $candidates = @( - "C:\Program Files\Google\Chrome\Application\chrome.exe", - "C:\Program Files (x86)\Google\Chrome\Application\chrome.exe", - "C:\Program Files\Chromium\Application\chromium.exe", - "${env:LOCALAPPDATA}\Google\Chrome\Application\chrome.exe", - "${env:LOCALAPPDATA}\Chromium\Application\chromium.exe" - ) - foreach ($p in $candidates) { - if (Test-Path $p) { return $p } - } - # Edge — Chromium-based, agent-browser can use it - foreach ($p in @( - "C:\Program Files (x86)\Microsoft\Edge\Application\msedge.exe", - "C:\Program Files\Microsoft\Edge\Application\msedge.exe" - )) { - if (Test-Path $p) { return $p } - } - return $null -} - -function Write-BrowserEnv { - param([string]$BrowserPath) - $envFile = Join-Path $HermesHome ".env" - New-Item -ItemType Directory -Force -Path $HermesHome | Out-Null - if (Test-Path $envFile) { - $existing = Get-Content $envFile -Raw -ErrorAction SilentlyContinue - if ($existing -and ($existing -match "(?m)^AGENT_BROWSER_EXECUTABLE_PATH=")) { - return - } - } - Add-Content -Path $envFile -Value "" - Add-Content -Path $envFile -Value "# Hermes Agent browser tools — use the system Chrome/Chromium/Edge binary." - Add-Content -Path $envFile -Value "AGENT_BROWSER_EXECUTABLE_PATH=$BrowserPath" - Write-Success "Configured browser tools to use $BrowserPath" -} - -function Confirm-ChromiumDownload { - if ($Yes) { return $true } - if (-not [Environment]::UserInteractive) { - Write-Warn "Non-interactive shell — skipping Chromium prompt." - Write-Info "Re-run with -Yes to install Chromium (~400 MB download)." - return $false - } - $reply = Read-Host "Install Playwright Chromium (~400 MB download)? [y/N]" - return ($reply -match "^(y|yes)$") -} - -function Ensure-Chromium { - if ($SkipChromium) { - Write-Info "Skipping Chromium install (-SkipChromium)" - return - } - - # agent-browser on Windows expects a Playwright-managed Chromium under - # %LOCALAPPDATA%\ms-playwright. The system-browser shortcut from the - # Linux/macOS path doesn't apply the same way on Windows — Playwright's - # default launch path won't pick up a stock Chrome install without an - # explicit AGENT_BROWSER_EXECUTABLE_PATH. We still offer it as a - # fallback when the user doesn't want the download. - - if (-not (Confirm-ChromiumDownload)) { - $sys = Find-SystemBrowser - if ($sys) { - Write-Info "Using system browser at $sys (Chromium download skipped)." - Write-BrowserEnv -BrowserPath $sys - } else { - Write-Info "Chromium install skipped. Browser tools won't launch until" - Write-Info "Chromium is installed or AGENT_BROWSER_EXECUTABLE_PATH is set." - } - return - } - - $npxExe = Resolve-NpxExe - if (-not $npxExe) { - Write-Err "npx not on PATH — cannot install Playwright Chromium" - throw "npx missing" - } - - Write-Info "Installing Playwright Chromium (~400 MB) ..." - & $npxExe --yes playwright install chromium - if ($LASTEXITCODE -ne 0) { - Write-Err "Playwright Chromium install failed (exit $LASTEXITCODE)" - Write-Info "Try again later: npx --yes playwright install chromium" - throw "playwright" - } - Write-Success "Playwright Chromium installed" -} - -# ───────────────────────────────────────────────────────────────────────── -# Main -# ───────────────────────────────────────────────────────────────────────── - -Write-Info "Hermes Agent: bootstrapping browser tools" -Write-Info " HERMES_HOME = $HermesHome" -Write-Info " OS = Windows" - -Ensure-Node -Ensure-AgentBrowser -Ensure-Chromium - -Write-Success "Browser tools setup complete." -Write-Info "Hermes Agent will pick up agent-browser from $NodePrefix on next launch." diff --git a/acp_adapter/bootstrap/bootstrap_browser_tools.sh b/acp_adapter/bootstrap/bootstrap_browser_tools.sh deleted file mode 100755 index 9981069a6a..0000000000 --- a/acp_adapter/bootstrap/bootstrap_browser_tools.sh +++ /dev/null @@ -1,399 +0,0 @@ -#!/usr/bin/env bash -# -# bootstrap_browser_tools.sh — install agent-browser + Playwright Chromium -# into ~/.hermes/node/ for use by Hermes Agent's browser tools. -# -# Targets the registry-install path: users who got Hermes via -# `uvx --from 'hermes-agent[acp]==X' hermes-acp` don't have a repo clone, -# so the install.sh `npm install`-in-repo flow doesn't apply. This script -# is a self-contained, idempotent slice of install.sh's browser block — -# safe to run from `hermes-acp --setup-browser`, from a fresh terminal, -# or from install.sh itself (it's a no-op when everything is already in place). -# -# Usage: -# bootstrap_browser_tools.sh # use defaults -# bootstrap_browser_tools.sh --yes # accept the ~400MB Chromium download -# bootstrap_browser_tools.sh --skip-chromium # only install Node + agent-browser -# HERMES_HOME=/custom/path bootstrap_browser_tools.sh -# -# Idempotent: re-running this is safe and fast. Each step checks whether -# the work is already done. - -set -euo pipefail - -# ───────────────────────────────────────────────────────────────────────── -# Config -# ───────────────────────────────────────────────────────────────────────── - -NODE_VERSION="22" -HERMES_HOME="${HERMES_HOME:-$HOME/.hermes}" -NODE_PREFIX="$HERMES_HOME/node" - -SKIP_CHROMIUM=false -ASSUME_YES=false - -# ───────────────────────────────────────────────────────────────────────── -# Logging -# ───────────────────────────────────────────────────────────────────────── - -if [ -t 1 ]; then - C_GREEN='\033[0;32m' - C_YELLOW='\033[0;33m' - C_BLUE='\033[0;34m' - C_RED='\033[0;31m' - C_RESET='\033[0m' -else - C_GREEN='' ; C_YELLOW='' ; C_BLUE='' ; C_RED='' ; C_RESET='' -fi - -log_info() { printf "${C_BLUE}[*]${C_RESET} %s\n" "$*"; } -log_success() { printf "${C_GREEN}[✓]${C_RESET} %s\n" "$*"; } -log_warn() { printf "${C_YELLOW}[!]${C_RESET} %s\n" "$*" >&2; } -log_error() { printf "${C_RED}[✗]${C_RESET} %s\n" "$*" >&2; } - -# ───────────────────────────────────────────────────────────────────────── -# Arg parsing -# ───────────────────────────────────────────────────────────────────────── - -while [ $# -gt 0 ]; do - case "$1" in - --skip-chromium) SKIP_CHROMIUM=true ;; - --yes|-y) ASSUME_YES=true ;; - -h|--help) - cat </dev/null 2>&1; then - local found_ver major - found_ver=$(node --version 2>/dev/null) - major=$(echo "$found_ver" | sed -E 's/^v([0-9]+).*/\1/') - if [ -n "$major" ] && [ "$major" -ge 20 ]; then - log_success "Node.js $found_ver found on PATH" - return 0 - fi - log_warn "Node.js $found_ver is older than v20 — installing managed Node." - fi - - if [ -x "$NODE_PREFIX/bin/node" ]; then - local found_ver - found_ver=$("$NODE_PREFIX/bin/node" --version 2>/dev/null || echo "?") - export PATH="$NODE_PREFIX/bin:$PATH" - log_success "Node.js $found_ver found (Hermes-managed at $NODE_PREFIX)" - return 0 - fi - - log_info "Installing Node.js $NODE_VERSION LTS into $NODE_PREFIX ..." - - local index_url="https://nodejs.org/dist/latest-v${NODE_VERSION}.x/" - local tarball_name - tarball_name=$(curl -fsSL "$index_url" \ - | grep -oE "node-v${NODE_VERSION}\.[0-9]+\.[0-9]+-${NODE_OS}-${NODE_ARCH}\.tar\.xz" \ - | head -1) - - if [ -z "$tarball_name" ]; then - tarball_name=$(curl -fsSL "$index_url" \ - | grep -oE "node-v${NODE_VERSION}\.[0-9]+\.[0-9]+-${NODE_OS}-${NODE_ARCH}\.tar\.gz" \ - | head -1) - fi - - if [ -z "$tarball_name" ]; then - log_error "Could not locate Node.js $NODE_VERSION tarball for $NODE_OS-$NODE_ARCH" - log_info "Install Node 20+ manually: https://nodejs.org/en/download/" - return 1 - fi - - local tmp_dir - tmp_dir=$(mktemp -d) - trap 'rm -rf "$tmp_dir"' RETURN - - log_info "Downloading $tarball_name ..." - if ! curl -fsSL "${index_url}${tarball_name}" -o "$tmp_dir/$tarball_name"; then - log_error "Node.js download failed" - return 1 - fi - - if [[ "$tarball_name" == *.tar.xz ]]; then - tar xf "$tmp_dir/$tarball_name" -C "$tmp_dir" - else - tar xzf "$tmp_dir/$tarball_name" -C "$tmp_dir" - fi - - local extracted_dir - extracted_dir=$(ls -d "$tmp_dir"/node-v* 2>/dev/null | head -1) - if [ ! -d "$extracted_dir" ]; then - log_error "Node.js extraction failed" - return 1 - fi - - mkdir -p "$HERMES_HOME" - rm -rf "$NODE_PREFIX" - mv "$extracted_dir" "$NODE_PREFIX" - - export PATH="$NODE_PREFIX/bin:$PATH" - - local installed_ver - installed_ver=$("$NODE_PREFIX/bin/node" --version 2>/dev/null || echo "?") - log_success "Node.js $installed_ver installed to $NODE_PREFIX" -} - -# ───────────────────────────────────────────────────────────────────────── -# Step 2: agent-browser + @askjo/camofox-browser via global npm install -# ───────────────────────────────────────────────────────────────────────── - -ensure_agent_browser() { - if ! command -v npm >/dev/null 2>&1; then - log_error "npm not on PATH after Node install — aborting" - return 1 - fi - - # _find_agent_browser() in tools/browser_tool.py walks ~/.hermes/node/bin - # plus a few standard prefixes, so installing globally into the managed - # Node prefix is enough — no PATH manipulation needed from the agent side. - if [ -x "$NODE_PREFIX/bin/agent-browser" ] || command -v agent-browser >/dev/null 2>&1; then - log_success "agent-browser already installed" - return 0 - fi - - # When the system's `npm` resolves to a root-owned prefix (e.g. - # /usr/lib/node_modules), `npm install -g` fails with EACCES without - # sudo. Force the prefix to the user-writable Hermes-managed Node - # directory so we never need sudo and the agent can always find the - # result. If we installed Node ourselves above, this is a no-op - # (managed Node already uses $NODE_PREFIX). If the user has system - # Node, we still drop agent-browser under $NODE_PREFIX/bin/ — which - # is exactly where _browser_candidate_path_dirs() looks first. - mkdir -p "$NODE_PREFIX" - - log_info "Installing agent-browser (npm, prefix=$NODE_PREFIX)..." - if ! npm install -g --prefix "$NODE_PREFIX" --silent \ - agent-browser@^0.26.0 \ - "@askjo/camofox-browser@^1.5.2"; then - log_error "npm install -g agent-browser failed" - return 1 - fi - - # macOS/Linux global installs place the shim into $NODE_PREFIX/bin/. - # Add it to PATH for any subsequent steps (npx playwright). - export PATH="$NODE_PREFIX/bin:$PATH" - - log_success "agent-browser installed to $NODE_PREFIX/bin/" -} - -# ───────────────────────────────────────────────────────────────────────── -# Step 3: Playwright Chromium -# ───────────────────────────────────────────────────────────────────────── - -confirm_chromium_download() { - if [ "$ASSUME_YES" = true ]; then return 0; fi - if [ ! -t 0 ]; then - log_warn "Non-interactive shell — skipping Chromium prompt." - log_info "Re-run with --yes to install Chromium (~400 MB download)." - return 1 - fi - printf "Install Playwright Chromium (~400 MB download)? [y/N] " - local reply="" - read -r reply || reply="" - case "$reply" in - y|Y|yes|YES) return 0 ;; - *) return 1 ;; - esac -} - -# Detect a usable system Chrome/Chromium. agent-browser's Chrome engine can -# use it instead of downloading Playwright's bundled Chromium, saving the -# download cost. Returns the path or empty string. -find_system_browser() { - local candidate - for candidate in google-chrome google-chrome-stable chromium chromium-browser chrome; do - if command -v "$candidate" >/dev/null 2>&1; then - command -v "$candidate" - return 0 - fi - done - # macOS app-bundle locations - if [ "$OS" = "macos" ]; then - for candidate in \ - "/Applications/Google Chrome.app/Contents/MacOS/Google Chrome" \ - "/Applications/Chromium.app/Contents/MacOS/Chromium" ; do - if [ -x "$candidate" ]; then - echo "$candidate" - return 0 - fi - done - fi - return 1 -} - -write_browser_env() { - local browser_path="$1" - local env_file="$HERMES_HOME/.env" - mkdir -p "$HERMES_HOME" - if [ -f "$env_file" ] && grep -q "^AGENT_BROWSER_EXECUTABLE_PATH=" "$env_file"; then - return 0 - fi - { - echo "" - echo "# Hermes Agent browser tools — use the system Chrome/Chromium binary." - echo "AGENT_BROWSER_EXECUTABLE_PATH=$browser_path" - } >> "$env_file" - log_success "Configured browser tools to use $browser_path" -} - -ensure_chromium() { - if [ "$SKIP_CHROMIUM" = true ]; then - log_info "Skipping Chromium install (--skip-chromium)" - return 0 - fi - - local system_browser - system_browser="$(find_system_browser 2>/dev/null || true)" - if [ -n "$system_browser" ]; then - log_success "Found system browser: $system_browser" - log_info "Skipping Playwright Chromium download; agent-browser will use it." - write_browser_env "$system_browser" - return 0 - fi - - if ! confirm_chromium_download; then - log_info "Chromium install skipped. Browser tools will only work if you" - log_info "set AGENT_BROWSER_EXECUTABLE_PATH or install Chromium later." - return 0 - fi - - if ! command -v npx >/dev/null 2>&1; then - log_error "npx not on PATH — cannot install Playwright Chromium" - return 1 - fi - - log_info "Installing Playwright Chromium (~400 MB) ..." - - # On apt-based distros, --with-deps requires sudo. Try non-interactively - # only — never prompt — and fall back to the bare browser-only install. - local installed=false - if [ "$OS" = "linux" ]; then - case "$DISTRO" in - ubuntu|debian|raspbian|pop|linuxmint|elementary|zorin|kali|parrot) - if [ "$(id -u)" -eq 0 ] || (command -v sudo >/dev/null 2>&1 && sudo -n true 2>/dev/null); then - log_info "Installing system deps with --with-deps (sudo available)" - if npx --yes playwright install --with-deps chromium; then - installed=true - fi - else - log_warn "sudo not available non-interactively — installing Chromium without system deps." - log_info "If browser tools fail to launch, an administrator should run:" - log_info " sudo npx playwright install-deps chromium" - fi - ;; - arch|manjaro|cachyos|endeavouros|garuda) - log_info "Arch-family system dependencies are not auto-installed." - log_info "If launch fails, run: sudo pacman -S nss atk at-spi2-core cups libdrm libxkbcommon mesa pango cairo alsa-lib" - ;; - fedora|rhel|centos|rocky|alma) - log_info "Fedora/RHEL system dependencies are not auto-installed." - log_info "If launch fails, run: sudo dnf install nss atk at-spi2-core cups-libs libdrm libxkbcommon mesa-libgbm pango cairo alsa-lib" - ;; - opensuse*|sles) - log_info "openSUSE system dependencies are not auto-installed." - ;; - esac - fi - - if [ "$installed" = false ]; then - if npx --yes playwright install chromium; then - installed=true - fi - fi - - if [ "$installed" = true ]; then - log_success "Playwright Chromium installed" - else - log_error "Playwright Chromium install failed" - log_info "Try again later: npx --yes playwright install chromium" - return 1 - fi -} - -# ───────────────────────────────────────────────────────────────────────── -# Main -# ───────────────────────────────────────────────────────────────────────── - -main() { - log_info "Hermes Agent: bootstrapping browser tools" - log_info " HERMES_HOME = $HERMES_HOME" - log_info " OS / arch = $NODE_OS-$NODE_ARCH ${DISTRO:+($DISTRO)}" - - ensure_node - ensure_agent_browser - ensure_chromium - - log_success "Browser tools setup complete." - log_info "Hermes Agent will pick up agent-browser from $NODE_PREFIX/bin/ on next launch." -} - -main diff --git a/acp_adapter/entry.py b/acp_adapter/entry.py index cf5c2ba9cf..9ce6281824 100644 --- a/acp_adapter/entry.py +++ b/acp_adapter/entry.py @@ -182,56 +182,31 @@ def _run_setup() -> None: def _run_setup_browser(assume_yes: bool = False) -> int: - """Bootstrap agent-browser + Playwright Chromium for the registry-install path. + """Bootstrap agent-browser + Chromium. - Shells out to the bundled platform-specific bootstrap script - (acp_adapter/bootstrap/bootstrap_browser_tools.{sh,ps1}) so the install - logic lives in one place — readable, debuggable, and shareable with - install.sh / install.ps1 if we ever want to call it from there too. + Routes through dep_ensure -> install.{sh,ps1} --ensure, sharing code + with ``hermes postinstall`` and the runtime lazy installer. - Returns the script's exit code (0 on success). + Returns 0 on success, 1 on failure. """ - import platform - import subprocess + from hermes_cli.dep_ensure import ensure_dependency - bootstrap_dir = Path(__file__).resolve().parent / "bootstrap" - - if platform.system() == "Windows": - script = bootstrap_dir / "bootstrap_browser_tools.ps1" - if not script.is_file(): - print( - f"Bootstrap script not found at {script} — wheel may be incomplete.", - file=sys.stderr, - ) - return 1 - cmd = [ - "powershell.exe", - "-NoProfile", - "-ExecutionPolicy", "Bypass", - "-File", str(script), - ] - if assume_yes: - cmd.append("-Yes") - else: - script = bootstrap_dir / "bootstrap_browser_tools.sh" - if not script.is_file(): - print( - f"Bootstrap script not found at {script} — wheel may be incomplete.", - file=sys.stderr, - ) - return 1 - cmd = ["bash", str(script)] - if assume_yes: - cmd.append("--yes") - - # stdio is inherited so the user sees the bootstrap's progress live. try: - result = subprocess.run(cmd, check=False) - except FileNotFoundError as exc: - # bash / powershell.exe not on PATH - print(f"Could not launch browser bootstrap: {exc}", file=sys.stderr) + node_ok = ensure_dependency("node", interactive=not assume_yes) + if not node_ok: + print("Node.js installation failed — cannot proceed with browser tools.", + file=sys.stderr) + return 1 + + browser_ok = ensure_dependency("browser", interactive=not assume_yes) + if not browser_ok: + print("Browser tools installation failed.", file=sys.stderr) + return 1 + + return 0 + except OSError as exc: + print(f"Browser bootstrap failed: {exc}", file=sys.stderr) return 1 - return result.returncode def main(argv: list[str] | None = None) -> None: diff --git a/scripts/install.sh b/scripts/install.sh index c34c64267c..3ece561a86 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -1512,6 +1512,17 @@ find_system_browser() { fi done + if [ "$(uname)" = "Darwin" ]; then + for app in \ + "/Applications/Google Chrome.app/Contents/MacOS/Google Chrome" \ + "/Applications/Chromium.app/Contents/MacOS/Chromium"; do + if [ -x "$app" ]; then + echo "$app" + return 0 + fi + done + fi + return 1 } @@ -1534,10 +1545,15 @@ configure_browser_env_from_system_browser() { browser_path="$(find_system_browser 2>/dev/null || true)" fi - if [ -z "$browser_path" ] || [ ! -f "$env_file" ]; then + if [ -z "$browser_path" ]; then return 0 fi + mkdir -p "$HERMES_HOME" + if [ ! -f "$env_file" ]; then + touch "$env_file" + fi + if grep -q '^AGENT_BROWSER_EXECUTABLE_PATH=' "$env_file" 2>/dev/null; then log_info "AGENT_BROWSER_EXECUTABLE_PATH already configured" return 0 @@ -1888,6 +1904,73 @@ print_success() { fi } +ensure_browser() { + if ! command -v node >/dev/null 2>&1; then + local node_bin="$HERMES_HOME/node/bin/node" + if [ -x "$node_bin" ]; then + export PATH="$HERMES_HOME/node/bin:$PATH" + else + log_error "Node.js not found. Run with --ensure node first." + return 1 + fi + fi + + local npm_bin + npm_bin="$(command -v npm 2>/dev/null || echo "$HERMES_HOME/node/bin/npm")" + if [ ! -x "$npm_bin" ]; then + log_error "npm not found" + return 1 + fi + + log_info "Installing agent-browser..." + local log_file + log_file="$(mktemp)" + if ! "$npm_bin" install -g --prefix "$HERMES_HOME/node" --silent --ignore-scripts \ + "agent-browser@^0.26.0" \ + "@askjo/camofox-browser@^1.5.2" \ + >"$log_file" 2>&1; then + log_error "npm install failed:" + cat "$log_file" >&2 + rm -f "$log_file" + return 1 + fi + rm -f "$log_file" + export PATH="$HERMES_HOME/node/bin:$PATH" + + local sys_browser + sys_browser="$(find_system_browser 2>/dev/null || true)" + if [ -n "$sys_browser" ]; then + configure_browser_env_from_system_browser "$sys_browser" + log_info "System browser detected -- skipping Chromium download" + return 0 + fi + + log_info "Installing Chromium via agent-browser install..." + local ab_bin="$HERMES_HOME/node/bin/agent-browser" + if [ -x "$ab_bin" ]; then + "$ab_bin" install 2>/dev/null || { + log_warn "Chromium install failed. Browser tools may not work without a system browser." + + # OS-specific hints (detect_os sets $DISTRO) + case "${DISTRO:-unknown}" in + ubuntu|debian) + log_info "Try: sudo apt-get install -y chromium-browser" + ;; + arch) + log_info "Try: sudo pacman -S chromium" + ;; + fedora|rhel|centos) + log_info "Try: sudo dnf install -y chromium" + ;; + esac + } + else + log_warn "agent-browser not found at $ab_bin" + fi + + return 0 +} + ensure_mode() { detect_os @@ -1901,19 +1984,7 @@ ensure_mode() { browser) check_node if [ "$HAS_NODE" = true ]; then - DETECTED_BROWSER_EXECUTABLE="$(find_system_browser 2>/dev/null || true)" - if [ -z "$DETECTED_BROWSER_EXECUTABLE" ]; then - log_info "Installing agent-browser + Chromium..." - npm_bin="$(command -v npm 2>/dev/null || echo "")" - if [ -n "$npm_bin" ]; then - local agent_browser_dir="$HERMES_HOME/node_modules" - mkdir -p "$agent_browser_dir" - "$npm_bin" install --prefix "$HERMES_HOME" agent-browser 2>/dev/null || true - npx playwright install chromium 2>/dev/null || true - fi - else - log_success "System browser found: $DETECTED_BROWSER_EXECUTABLE" - fi + ensure_browser fi ;; ripgrep) @@ -1948,16 +2019,7 @@ postinstall_mode() { install_system_packages if [ "$HAS_NODE" = true ] && [ "$SKIP_BROWSER" = false ]; then - DETECTED_BROWSER_EXECUTABLE="$(find_system_browser 2>/dev/null || true)" - if [ -z "$DETECTED_BROWSER_EXECUTABLE" ]; then - log_info "Installing browser engine..." - npm_bin="$(command -v npm 2>/dev/null || echo "")" - if [ -n "$npm_bin" ]; then - npx playwright install chromium 2>/dev/null || true - fi - else - log_success "System browser found: $DETECTED_BROWSER_EXECUTABLE" - fi + ensure_browser fi HERMES_CMD="$(command -v hermes 2>/dev/null || echo "")" diff --git a/tests/acp/test_entry.py b/tests/acp/test_entry.py index 81d30cd868..1d881565bd 100644 --- a/tests/acp/test_entry.py +++ b/tests/acp/test_entry.py @@ -94,103 +94,62 @@ def test_main_setup_skips_browser_prompt_on_no(monkeypatch): assert called == [] -def test_main_setup_browser_invokes_bundled_script(monkeypatch): - """`hermes-acp --setup-browser` must shell out to the bundled bootstrap - script — never reimplement the install logic inline.""" - monkeypatch.setattr("platform.system", lambda: "Linux") +def test_main_setup_browser_calls_ensure_dependency(monkeypatch): + """`hermes-acp --setup-browser` routes through dep_ensure.ensure_dependency.""" + calls = [] - captured = {} + def fake_ensure(dep, interactive=True): + calls.append((dep, interactive)) + return True - def fake_run(cmd, check=False): - captured["cmd"] = cmd - - class _R: - returncode = 0 - - return _R() - - monkeypatch.setattr("subprocess.run", fake_run) + monkeypatch.setattr("hermes_cli.dep_ensure.ensure_dependency", fake_ensure) entry.main(["--setup-browser"]) - assert captured["cmd"][0] == "bash" - assert captured["cmd"][1].endswith("bootstrap_browser_tools.sh") - # --yes is NOT passed when the flag is absent. - assert "--yes" not in captured["cmd"] + assert ("node", True) in calls + assert ("browser", True) in calls def test_main_setup_browser_forwards_yes_flag(monkeypatch): - monkeypatch.setattr("platform.system", lambda: "Linux") + """--yes suppresses interactive prompts in ensure_dependency.""" + calls = [] - captured = {} + def fake_ensure(dep, interactive=True): + calls.append((dep, interactive)) + return True - def fake_run(cmd, check=False): - captured["cmd"] = cmd - - class _R: - returncode = 0 - - return _R() - - monkeypatch.setattr("subprocess.run", fake_run) + monkeypatch.setattr("hermes_cli.dep_ensure.ensure_dependency", fake_ensure) entry.main(["--setup-browser", "--yes"]) - assert "--yes" in captured["cmd"] + assert ("node", False) in calls + assert ("browser", False) in calls -def test_main_setup_browser_uses_powershell_on_windows(monkeypatch): - monkeypatch.setattr("platform.system", lambda: "Windows") +def test_main_setup_browser_stops_on_node_failure(monkeypatch): + """If node install fails, browser install is not attempted.""" + calls = [] - captured = {} + def fake_ensure(dep, interactive=True): + calls.append(dep) + return dep != "node" # node fails - def fake_run(cmd, check=False): - captured["cmd"] = cmd - - class _R: - returncode = 0 - - return _R() - - monkeypatch.setattr("subprocess.run", fake_run) - - entry.main(["--setup-browser", "--yes"]) - - assert captured["cmd"][0] == "powershell.exe" - assert any(part.endswith("bootstrap_browser_tools.ps1") for part in captured["cmd"]) - assert "-Yes" in captured["cmd"] - - -def test_main_setup_browser_propagates_failure(monkeypatch): - monkeypatch.setattr("platform.system", lambda: "Linux") - - class _R: - returncode = 7 - - monkeypatch.setattr("subprocess.run", lambda cmd, check=False: _R()) + monkeypatch.setattr("hermes_cli.dep_ensure.ensure_dependency", fake_ensure) with pytest.raises(SystemExit) as excinfo: entry.main(["--setup-browser"]) - assert excinfo.value.code == 7 + assert excinfo.value.code == 1 + assert "node" in calls + assert "browser" not in calls -def test_bootstrap_scripts_ship_with_package(): - """The package-data wiring (pyproject.toml) must include the bootstrap - scripts — otherwise `--setup-browser` 404s at runtime.""" - from pathlib import Path +def test_main_setup_browser_propagates_browser_failure(monkeypatch): + """If browser install fails, exit code is 1.""" + def fake_ensure(dep, interactive=True): + return dep != "browser" # browser fails - bootstrap_dir = Path(entry.__file__).resolve().parent / "bootstrap" - sh = bootstrap_dir / "bootstrap_browser_tools.sh" - ps1 = bootstrap_dir / "bootstrap_browser_tools.ps1" + monkeypatch.setattr("hermes_cli.dep_ensure.ensure_dependency", fake_ensure) - assert sh.is_file(), f"missing bundled script: {sh}" - assert ps1.is_file(), f"missing bundled script: {ps1}" - - sh_text = sh.read_text(encoding="utf-8") - ps1_text = ps1.read_text(encoding="utf-8") - - # Sanity: scripts know how to find the Hermes-managed Node prefix. - assert "HERMES_HOME" in sh_text - assert "agent-browser" in sh_text - assert "HermesHome" in ps1_text - assert "agent-browser" in ps1_text + with pytest.raises(SystemExit) as excinfo: + entry.main(["--setup-browser"]) + assert excinfo.value.code == 1 From 609c485fc6d0a0c24a023cd1349ebd6ddbf60315 Mon Sep 17 00:00:00 2001 From: Austin Pickett Date: Mon, 18 May 2026 08:42:33 -0400 Subject: [PATCH 18/30] Merge pull request #27971 from NousResearch/austin/fix/goal-statusbar fix(tui): keep /goal verdict out of compact status row --- .../createGatewayEventHandler.test.ts | 42 ++++++++++++++++++- ui-tui/src/app/createGatewayEventHandler.ts | 17 ++++++-- 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/ui-tui/src/__tests__/createGatewayEventHandler.test.ts b/ui-tui/src/__tests__/createGatewayEventHandler.test.ts index cd278eecdf..0c7ec3b06a 100644 --- a/ui-tui/src/__tests__/createGatewayEventHandler.test.ts +++ b/ui-tui/src/__tests__/createGatewayEventHandler.test.ts @@ -4,7 +4,7 @@ import { createGatewayEventHandler } from '../app/createGatewayEventHandler.js' import { getOverlayState, resetOverlayState } from '../app/overlayStore.js' import { turnController } from '../app/turnController.js' import { getTurnState, resetTurnState } from '../app/turnStore.js' -import { patchUiState, resetUiState } from '../app/uiStore.js' +import { getUiState, patchUiState, resetUiState } from '../app/uiStore.js' import { estimateTokensRough } from '../lib/text.js' import type { Msg } from '../types.js' @@ -132,6 +132,46 @@ describe('createGatewayEventHandler', () => { expect(ctx.system.sys).toHaveBeenCalledWith('compressing 968 messages (~123,400 tok)…') }) + it('keeps goal verdict text in transcript but shows a brief idle status (#goal statusbar)', () => { + const appended: Msg[] = [] + const ctx = buildCtx(appended) + const onEvent = createGatewayEventHandler(ctx) + const verdict = '✓ Goal achieved: long judge reason goes only in transcript, not merged with cwd label.' + + vi.useFakeTimers() + try { + onEvent({ + payload: { kind: 'goal', text: verdict }, + type: 'status.update' + } as any) + + expect(ctx.system.sys).toHaveBeenCalledWith(verdict) + expect(getUiState().status).toBe('✓ goal complete') + + vi.advanceTimersByTime(6001) + expect(getUiState().status).toBe('ready') + } finally { + vi.useRealTimers() + } + }) + + it('maps goal status.update prefixes to short status strings', () => { + const ctx = buildCtx([]) + const onEvent = createGatewayEventHandler(ctx) + + onEvent({ + payload: { kind: 'goal', text: '↻ Continuing toward goal (1/10): reason' }, + type: 'status.update' + } as any) + expect(getUiState().status).toBe('↻ goal continuing') + + onEvent({ + payload: { kind: 'goal', text: '⏸ Goal paused — budget exhausted.' }, + type: 'status.update' + } as any) + expect(getUiState().status).toBe('⏸ goal paused') + }) + it('surfaces self-improvement review summaries as a persistent system line', () => { const appended: Msg[] = [] const ctx = buildCtx(appended) diff --git a/ui-tui/src/app/createGatewayEventHandler.ts b/ui-tui/src/app/createGatewayEventHandler.ts index ca269a131b..267334bfd7 100644 --- a/ui-tui/src/app/createGatewayEventHandler.ts +++ b/ui-tui/src/app/createGatewayEventHandler.ts @@ -338,14 +338,23 @@ export function createGatewayEventHandler(ctx: GatewayEventHandlerContext): (ev: return } - setStatus(p.text) - - if (p.kind === 'compressing') { + if (p.kind === 'goal') { sys(p.text) + const brief = p.text.startsWith('✓') + ? '✓ goal complete' + : p.text.startsWith('↻') + ? '↻ goal continuing' + : p.text.startsWith('⏸') + ? '⏸ goal paused' + : 'ready' + setStatus(brief) + restoreStatusAfter(6000) return } - if (p.kind === 'goal') { + setStatus(p.text) + + if (p.kind === 'compressing') { sys(p.text) return } From ac1536b19f5765e082650615e0a5748f731f8c58 Mon Sep 17 00:00:00 2001 From: duyua9 Date: Mon, 18 May 2026 22:03:25 +0800 Subject: [PATCH 19/30] fix(web): render object config values structurally (#10949) --- web/src/components/AutoField.tsx | 95 +++++++++++++++++++++++++------- 1 file changed, 75 insertions(+), 20 deletions(-) diff --git a/web/src/components/AutoField.tsx b/web/src/components/AutoField.tsx index f7afd150b0..0f96d42042 100644 --- a/web/src/components/AutoField.tsx +++ b/web/src/components/AutoField.tsx @@ -17,6 +17,71 @@ function FieldHint({ schema, schemaKey }: { schema: Record; sch ); } +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function formatScalar(value: unknown): string { + if (value === undefined || value === null) return ""; + if (typeof value === "string") return value; + if (typeof value === "number" || typeof value === "boolean") return String(value); + return JSON.stringify(value); +} + +function NestedValueEditor({ + fieldKey, + value, + onChange, +}: { + fieldKey: string; + value: unknown; + onChange: (v: unknown) => void; +}) { + if (isRecord(value)) { + return ( +
+ {Object.entries(value).map(([subKey, subVal]) => ( +
+ + onChange({ ...value, [subKey]: next })} + /> +
+ ))} +
+ ); + } + + if (Array.isArray(value)) { + return ( +
+ {value.map((item, index) => ( +
+ + + onChange(value.map((existing, i) => (i === index ? next : existing))) + } + /> +
+ ))} +
+ ); + } + + return ( + onChange(e.target.value)} + className="text-xs" + /> + ); +} + export function AutoField({ schemaKey, schema, @@ -26,6 +91,16 @@ export function AutoField({ const rawLabel = schemaKey.split(".").pop() ?? schemaKey; const label = rawLabel.replace(/_/g, " ").replace(/\b\w/g, (c) => c.toUpperCase()); + if (isRecord(value) || (Array.isArray(value) && value.some((item) => isRecord(item)))) { + return ( +
+ + + +
+ ); + } + if (schema.type === "boolean") { return (
@@ -114,26 +189,6 @@ export function AutoField({ ); } - if (typeof value === "object" && value !== null && !Array.isArray(value)) { - const obj = value as Record; - return ( -
- - - {Object.entries(obj).map(([subKey, subVal]) => ( -
- - onChange({ ...obj, [subKey]: e.target.value })} - className="text-xs" - /> -
- ))} -
- ); - } - return (
From 6a20ad6c0a6cf9b078da4dd3710fe6cbf37241d2 Mon Sep 17 00:00:00 2001 From: "Brian D. Evans" Date: Mon, 18 May 2026 15:23:03 +0100 Subject: [PATCH 20/30] fix(dashboard): constrain theme picker dropdown height so themes are scrollable (#25213) (#25220) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The header theme picker (`ThemeSwitcher`) renders a `role="listbox"` popup with no `max-height` or overflow. With 20+ community themes installed under `~/.hermes/dashboard-themes/`, the list extends past the viewport and themes at the top or bottom are unreachable — the user reports only 15 of 26 themes visible, with no scrollbar to access the rest. Sibling switchers (`LanguageSwitcher`, `SlashPopover`) already cap their listboxes (`max-h-80 overflow-y-auto` / `max-h-64 overflow-y-auto`); this just brings the theme picker into line. Scoped to the component instead of a global `div[role="listbox"]` CSS rule so other dropdowns aren't affected. `70dvh` matches the user's tested workaround and the `dvh` unit handles mobile browser UI chrome correctly (unlike `vh`). Fixes #25213. Co-authored-by: briandevans <252620095+briandevans@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) --- web/src/components/ThemeSwitcher.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/components/ThemeSwitcher.tsx b/web/src/components/ThemeSwitcher.tsx index 462ccaacfc..90a3d11ebd 100644 --- a/web/src/components/ThemeSwitcher.tsx +++ b/web/src/components/ThemeSwitcher.tsx @@ -79,7 +79,7 @@ export function ThemeSwitcher({ dropUp = false }: ThemeSwitcherProps) { role="listbox" aria-label={t.theme?.title ?? "Theme"} className={cn( - "absolute z-50 min-w-[240px]", + "absolute z-50 min-w-[240px] max-h-[70dvh] overflow-y-auto", dropUp ? "left-0 bottom-full mb-1" : "right-0 top-full mt-1", "border border-current/20 bg-background-base/95 backdrop-blur-sm", "shadow-[0_12px_32px_-8px_rgba(0,0,0,0.6)]", From 4414a99d8c3ec4c38c816ad7a33d5c0ee0d61962 Mon Sep 17 00:00:00 2001 From: LeonSGP <154585401+LeonSGP43@users.noreply.github.com> Date: Mon, 18 May 2026 22:35:18 +0800 Subject: [PATCH 21/30] fix(kanban): stop forcing dashboard text to all caps (#26413) --- plugins/kanban/dashboard/dist/index.js | 2 +- plugins/kanban/dashboard/dist/style.css | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/plugins/kanban/dashboard/dist/index.js b/plugins/kanban/dashboard/dist/index.js index 3f6def61ce..fb4d346582 100644 --- a/plugins/kanban/dashboard/dist/index.js +++ b/plugins/kanban/dashboard/dist/index.js @@ -1701,7 +1701,7 @@ return h("div", { className: "hermes-kanban-boardswitcher" }, h("div", { className: "hermes-kanban-boardswitcher-inner" }, h("div", { className: "flex flex-col gap-0.5" }, - h("div", { className: "text-[11px] uppercase tracking-wider text-muted-foreground" }, + h("div", { className: "text-[11px] tracking-wider text-muted-foreground" }, tx(t, "board", "Board")), h("div", { className: "flex items-center gap-2" }, h(Select, Object.assign({ diff --git a/plugins/kanban/dashboard/dist/style.css b/plugins/kanban/dashboard/dist/style.css index f3d66a8859..afbe591550 100644 --- a/plugins/kanban/dashboard/dist/style.css +++ b/plugins/kanban/dashboard/dist/style.css @@ -465,7 +465,6 @@ .hermes-kanban-section-head { font-size: 0.72rem; font-weight: 600; - text-transform: uppercase; letter-spacing: 0.07em; color: var(--color-muted-foreground); } @@ -611,7 +610,6 @@ } .hermes-kanban-deps-label { font-size: 0.68rem; - text-transform: uppercase; letter-spacing: 0.08em; color: var(--color-muted-foreground); min-width: 4rem; @@ -691,7 +689,6 @@ border: 0; color: var(--color-muted-foreground); font-size: 0.7rem; - text-transform: uppercase; letter-spacing: 0.05em; cursor: pointer; padding: 0; @@ -869,7 +866,6 @@ .hermes-kanban-run-outcome { font-family: var(--font-mono, ui-monospace, monospace); font-weight: 600; - text-transform: uppercase; letter-spacing: 0.05em; color: var(--color-foreground); } @@ -929,7 +925,6 @@ .hermes-kanban-run-meta-label { font-size: 0.65rem; font-weight: 600; - text-transform: uppercase; letter-spacing: 0.06em; color: var(--color-muted-foreground); padding-bottom: 0.15rem; From 16abb74eab2d0bd34efc0e94a17846507a0c952c Mon Sep 17 00:00:00 2001 From: Wesley Simplicio Date: Mon, 18 May 2026 11:48:21 -0300 Subject: [PATCH 22/30] fix(kanban): use selectChangeHandler for workspace, parent, and bulk-reassign selects (#24547) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SDK Select fires onValueChange(value) not onChange({target:{value}}), so all three bare onChange handlers silently received undefined from e.target. Replace raw onChange with selectChangeHandler() — the existing helper that wires both onValueChange and a guarded onChange — so selections register regardless of which event the SDK Select dispatches. Closes #24520 Co-authored-by: Claude Sonnet 4.6 --- plugins/kanban/dashboard/dist/index.js | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/plugins/kanban/dashboard/dist/index.js b/plugins/kanban/dashboard/dist/index.js index fb4d346582..9ed0d4ef22 100644 --- a/plugins/kanban/dashboard/dist/index.js +++ b/plugins/kanban/dashboard/dist/index.js @@ -2027,11 +2027,10 @@ ), h("div", { className: "hermes-kanban-bulk-reassign", title: "Reassign selected tasks to a different Hermes profile. Pick a profile (or unassign) and click Apply." }, - h(Select, { + h(Select, Object.assign({ value: assignee, - onChange: function (e) { setAssignee(e.target.value); }, className: "h-7 text-xs", - }, + }, selectChangeHandler(setAssignee)), h(SelectOption, { value: "" }, "— reassign —"), h(SelectOption, { value: "__none__" }, "(unassign)"), props.assignees.map(function (a) { @@ -2542,12 +2541,11 @@ className: "h-7 text-xs", }), h("div", { className: "flex gap-2" }, - h(Select, { + h(Select, Object.assign({ value: workspaceKind, - onChange: function (e) { setWorkspaceKind(e.target.value); }, title: "scratch: isolated temp dir (default). worktree: git worktree on the assignee profile. dir: exact path (required below).", className: "h-7 text-xs w-28", - }, + }, selectChangeHandler(setWorkspaceKind)), h(SelectOption, { value: "scratch" }, "scratch"), h(SelectOption, { value: "worktree" }, "worktree"), h(SelectOption, { value: "dir" }, "dir"), @@ -2559,12 +2557,11 @@ className: "h-7 text-xs flex-1", }) : null, ), - h(Select, { + h(Select, Object.assign({ value: parent, - onChange: function (e) { setParent(e.target.value); }, className: "h-7 text-xs", title: "Optional parent task. A child stays blocked in its current column until the parent is marked done.", - }, + }, selectChangeHandler(setParent)), h(SelectOption, { value: "" }, tx(t, "noParent", "— no parent —")), (props.allTasks || []).map(function (task) { return h(SelectOption, { key: task.id, value: task.id }, From 73407b1e303a13782812675869047dfab60ca650 Mon Sep 17 00:00:00 2001 From: sharziki Date: Sat, 16 May 2026 13:03:31 -0400 Subject: [PATCH 23/30] fix(auth): send Bearer auth for Azure Foundry anthropic_messages endpoints Azure AI Foundry's Anthropic-style endpoint requires `Authorization: Bearer` instead of `x-api-key`. Add `azure.com` to `_requires_bearer_auth()` so the existing Bearer path at line 586 fires before the generic third-party branch sets `api_key` (x-api-key). Fixes #26970 --- agent/anthropic_adapter.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/agent/anthropic_adapter.py b/agent/anthropic_adapter.py index e7e1a8acb6..469b0fc9bb 100644 --- a/agent/anthropic_adapter.py +++ b/agent/anthropic_adapter.py @@ -471,14 +471,18 @@ def _requires_bearer_auth(base_url: str | None) -> bool: """Return True for Anthropic-compatible providers that require Bearer auth. Some third-party /anthropic endpoints implement Anthropic's Messages API but - require Authorization: Bearer *** of Anthropic's native x-api-key header. - MiniMax's global and China Anthropic-compatible endpoints follow this pattern. + require Authorization: Bearer instead of Anthropic's native x-api-key header. + MiniMax's global and China Anthropic-compatible endpoints, and Azure AI + Foundry's Anthropic-style endpoint follow this pattern. """ normalized = _normalize_base_url_text(base_url) if not normalized: return False normalized = normalized.rstrip("/").lower() - return normalized.startswith(("https://api.minimax.io/anthropic", "https://api.minimaxi.com/anthropic")) + return ( + normalized.startswith(("https://api.minimax.io/anthropic", "https://api.minimaxi.com/anthropic")) + or "azure.com" in normalized + ) def _base_url_needs_context_1m_beta(base_url: str | None) -> bool: From f0c6d591488aa6df3940b8e5791e1c872f4b2888 Mon Sep 17 00:00:00 2001 From: teknium <127238744+teknium1@users.noreply.github.com> Date: Mon, 18 May 2026 09:23:50 -0700 Subject: [PATCH 24/30] fix(anthropic): scope MiniMax beta-strip to MiniMax only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cherry-pick of @sharziki's #27022 routed Azure Foundry through _requires_bearer_auth, which also triggered the MiniMax-specific beta-strip in _common_betas_for_base_url — dropping the 1M-context beta from Azure even though Azure needs it for 1M context. Split the strip predicate: introduce _is_minimax_anthropic_endpoint so the fine-grained-tool-streaming and context-1m strips only fire for MiniMax hosts, leaving Azure's bearer-auth header swap intact without losing 1M context. Also add a regression test that asserts Azure gets Bearer auth, the api-version query param, and the context-1m-2025-08-07 beta. --- agent/anthropic_adapter.py | 21 +++++++++++++++++++-- tests/agent/test_anthropic_adapter.py | 21 +++++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/agent/anthropic_adapter.py b/agent/anthropic_adapter.py index 469b0fc9bb..de9b7dd586 100644 --- a/agent/anthropic_adapter.py +++ b/agent/anthropic_adapter.py @@ -493,6 +493,21 @@ def _base_url_needs_context_1m_beta(base_url: str | None) -> bool: return "azure.com" in normalized +def _is_minimax_anthropic_endpoint(base_url: str | None) -> bool: + """Return True for MiniMax's Anthropic-compatible endpoints. + + MiniMax rejects the fine-grained-tool-streaming and context-1m betas; + those need to be stripped even though MiniMax also uses Bearer auth. + """ + normalized = _normalize_base_url_text(base_url) + if not normalized: + return False + normalized = normalized.rstrip("/").lower() + return normalized.startswith( + ("https://api.minimax.io/anthropic", "https://api.minimaxi.com/anthropic") + ) + + def _common_betas_for_base_url( base_url: str | None, *, @@ -502,7 +517,9 @@ def _common_betas_for_base_url( MiniMax's Anthropic-compatible endpoints (Bearer-auth) reject requests that include Anthropic's ``fine-grained-tool-streaming`` beta — every - tool-use message triggers a connection error. + tool-use message triggers a connection error. They also reject the + 1M-context beta. Azure AI Foundry's Anthropic endpoint also uses + Bearer auth but keeps both betas (it needs the 1M beta for 1M context). The ``context-1m-2025-08-07`` beta is not sent to native Anthropic by default because some subscriptions reject it. Add it only for endpoint @@ -515,7 +532,7 @@ def _common_betas_for_base_url( betas = list(_COMMON_BETAS) if _base_url_needs_context_1m_beta(base_url) and not drop_context_1m_beta: betas.append(_CONTEXT_1M_BETA) - if _requires_bearer_auth(base_url): + if _is_minimax_anthropic_endpoint(base_url): _stripped = {_TOOL_STREAMING_BETA, _CONTEXT_1M_BETA} return [b for b in betas if b not in _stripped] if drop_context_1m_beta: diff --git a/tests/agent/test_anthropic_adapter.py b/tests/agent/test_anthropic_adapter.py index c7119dfd3b..3d19c32dca 100644 --- a/tests/agent/test_anthropic_adapter.py +++ b/tests/agent/test_anthropic_adapter.py @@ -155,6 +155,27 @@ class TestBuildAnthropicClient: "anthropic-beta": "interleaved-thinking-2025-05-14" } + def test_azure_foundry_anthropic_endpoint_uses_bearer_auth(self): + """Azure AI Foundry's /anthropic endpoint requires Authorization: Bearer. + + Regression test for #26970: without this, builds set api_key (x-api-key) + and the endpoint returns HTTP 401. Also verifies that Azure retains the + 1M-context beta even though it now matches `_requires_bearer_auth`. + """ + with patch("agent.anthropic_adapter._anthropic_sdk") as mock_sdk: + build_anthropic_client( + "azure-foundry-secret-123", + base_url="https://my-resource.openai.azure.com/anthropic", + ) + kwargs = mock_sdk.Anthropic.call_args[1] + assert kwargs["auth_token"] == "azure-foundry-secret-123" + assert "api_key" not in kwargs + # Azure endpoints still get the api-version query param plumbing. + assert kwargs.get("default_query") == {"api-version": "2025-04-15"} + # Azure keeps the 1M-context beta (it's not MiniMax). + betas = kwargs["default_headers"]["anthropic-beta"] + assert "context-1m-2025-08-07" in betas + class TestReadClaudeCodeCredentials: @pytest.fixture(autouse=True) From a86d2ad5574147f527c5cf998751ccb46af47745 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Mon, 18 May 2026 09:31:08 -0700 Subject: [PATCH 25/30] fix(kanban-dashboard): wire onValueChange on OrchestrationPanel Selects (#27893) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dashboard SDK's