diff --git a/.gitignore b/.gitignore index 6d87318e35..489453d79b 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ *.pyc* __pycache__/ .venv/ +.venv .vscode/ .env .env.local diff --git a/Dockerfile b/Dockerfile index be358ac534..b4ebd09369 100644 --- a/Dockerfile +++ b/Dockerfile @@ -9,8 +9,11 @@ FROM ghcr.io/astral-sh/uv:0.11.6-python3.13-trixie@sha256:b3c543b6c4f23a5f2df228 FROM node:22-bookworm-slim@sha256:7af03b14a13c8cdd38e45058fd957bf00a72bbe17feac43b1c15a689c029c732 AS node_source FROM debian:13.4 -# Disable Python stdout buffering to ensure logs are printed immediately +# Disable Python stdout buffering to ensure logs are printed immediately. +# Do not write .pyc files at runtime: /opt/hermes is immutable in the +# published container and writable state belongs under /opt/data. ENV PYTHONUNBUFFERED=1 +ENV PYTHONDONTWRITEBYTECODE=1 # Store Playwright browsers outside the volume mount so the build-time # install survives the /opt/data volume overlay at runtime. @@ -186,36 +189,38 @@ RUN cd web && npm run build && \ # ---------- Source code ---------- # .dockerignore excludes node_modules, so the installs above survive. -COPY --chown=hermes:hermes . . +COPY . . # ---------- Permissions ---------- -# Make install dir world-readable so any HERMES_UID can read it at runtime. -# The venv needs to be traversable too. -# node_modules trees additionally need to be writable by the hermes user -# so the runtime `npm install` triggered by _tui_need_npm_install() in -# hermes_cli/main.py succeeds (see #18800). /opt/hermes/web is build-time -# only (HERMES_WEB_DIST points at hermes_cli/web_dist) and is intentionally -# not chowned here. -# /opt/hermes/gateway is runtime-writable: Python may create __pycache__ and -# gateway state artifacts beneath the package after services drop privileges, -# especially when the hermes UID is remapped at boot (#27221). -# The .venv MUST remain hermes-writable so lazy_deps.py can install -# remaining optional platform packages and future pin bumps at first use. -# Without this, `uv pip install` fails with EACCES and adapters silently -# fail to load. See tools/lazy_deps.py. +# Link hermes-agent itself (editable). Deps are already installed in the +# cached layer above; `--no-deps` makes this a fast egg-link creation with no +# resolution or downloads. +RUN uv pip install --no-cache-dir --no-deps -e "." + +# Keep /opt/hermes immutable for the runtime hermes user. Hosted/container +# instances must not be able to self-edit the installed source or venv; user +# data, skills, plugins, config, logs, and dashboard uploads live under +# /opt/data instead. Root can still repair the image during build/boot, but +# supervised Hermes processes drop to the non-root hermes user. USER root -RUN chmod -R a+rX /opt/hermes && \ - chown -R hermes:hermes /opt/hermes/.venv /opt/hermes/ui-tui /opt/hermes/gateway /opt/hermes/node_modules +RUN mkdir -p /opt/hermes/bin && \ + cp /opt/hermes/docker/hermes-exec-shim.sh /opt/hermes/bin/hermes && \ + chmod 0755 /opt/hermes/bin/hermes && \ + printf 'docker\n' > /opt/hermes/.install_method && \ + chown -R root:root /opt/hermes && \ + chmod -R a+rX /opt/hermes && \ + chmod -R a-w /opt/hermes +# The ``.install_method`` stamp is baked next to the running code (the install +# tree), NOT into $HERMES_HOME. $HERMES_HOME (/opt/data) is a shared data +# volume that is commonly bind-mounted from the host and even shared with a +# host-side Desktop/CLI install; stamping it at boot used to clobber that +# host install's marker and wrongly block its ``hermes update``. A code-scoped +# stamp is read first by detect_install_method() and is immune to the share. # Start as root so the s6-overlay stage2 hook can usermod/groupmod and chown # the data volume. Each supervised service then drops to the hermes user via # `s6-setuidgid hermes` in its run script. If HERMES_UID is unset, services # run as the default hermes user (UID 10000). -# ---------- Link hermes-agent itself (editable) ---------- -# Deps are already installed in the cached layer above; `--no-deps` makes -# this a fast (~1s) egg-link creation with no resolution or downloads. -RUN uv pip install --no-cache-dir --no-deps -e "." - # ---------- Bake build-time git revision ---------- # .dockerignore excludes .git, so `git rev-parse HEAD` from inside the # container always returns nothing — meaning `hermes dump` reports @@ -235,8 +240,9 @@ RUN uv pip install --no-cache-dir --no-deps -e "." # every published image has it. ARG HERMES_GIT_SHA= RUN if [ -n "${HERMES_GIT_SHA}" ]; then \ + chmod u+w /opt/hermes && \ printf '%s\n' "${HERMES_GIT_SHA}" > /opt/hermes/.hermes_build_sha && \ - chown hermes:hermes /opt/hermes/.hermes_build_sha; \ + chmod a-w /opt/hermes /opt/hermes/.hermes_build_sha; \ fi # ---------- s6-overlay service wiring ---------- @@ -282,6 +288,8 @@ ENV HERMES_WEB_DIST=/opt/hermes/hermes_cli/web_dist # check. (A separate launcher hardening is tracked independently.) ENV HERMES_TUI_DIR=/opt/hermes/ui-tui ENV HERMES_HOME=/opt/data +ENV HERMES_WRITE_SAFE_ROOT=/opt/data +ENV HERMES_DISABLE_LAZY_INSTALLS=1 # `docker exec` privilege-drop shim. When operators run # `docker exec hermes ...` they default to root, and any file the @@ -294,7 +302,6 @@ ENV HERMES_HOME=/opt/data # Recursion is impossible because the shim exec's the venv binary by # absolute path (/opt/hermes/.venv/bin/hermes). See the shim source for # the opt-out env var (HERMES_DOCKER_EXEC_AS_ROOT=1). -COPY --chmod=0755 docker/hermes-exec-shim.sh /opt/hermes/bin/hermes # Pre-s6 entrypoint.sh did `source .venv/bin/activate` which exported # the venv bin onto PATH; Architecture B's main-wrapper.sh does the diff --git a/agent/agent_init.py b/agent/agent_init.py index 7ce839b087..f5015de83f 100644 --- a/agent/agent_init.py +++ b/agent/agent_init.py @@ -27,7 +27,7 @@ import threading import time import uuid from datetime import datetime -from typing import Any, Dict, List, Optional +from typing import Any, Callable, Dict, List, Optional from urllib.parse import urlparse, parse_qs, urlunparse from agent.context_compressor import ContextCompressor @@ -195,6 +195,7 @@ def init_agent( status_callback: callable = None, notice_callback: callable = None, notice_clear_callback: callable = None, + event_callback: Optional[Callable[[str, dict], None]] = None, max_tokens: int = None, reasoning_config: Dict[str, Any] = None, service_tier: str = None, @@ -426,6 +427,7 @@ def init_agent( agent.status_callback = status_callback agent.notice_callback = notice_callback agent.notice_clear_callback = notice_clear_callback + agent.event_callback = event_callback agent.tool_gen_callback = tool_gen_callback @@ -597,6 +599,7 @@ def init_agent( # (e.g. CLI voice mode adds a temporary prefix for the live call only). agent._persist_user_message_idx = None agent._persist_user_message_override = None + agent._persist_user_message_timestamp = None # Cache anthropic image-to-text fallbacks per image payload/URL so a # single tool loop does not repeatedly re-run auxiliary vision on the @@ -1162,6 +1165,9 @@ def init_agent( "hermes_home": str(get_hermes_home()), "agent_context": "primary", } + if _init_kwargs["platform"] == "cli": + _init_kwargs["warning_callback"] = agent._emit_warning + _init_kwargs["status_callback"] = agent._emit_status # Thread session title for memory provider scoping # (e.g. honcho uses this to derive chat-scoped session keys) if agent._session_db: @@ -1230,6 +1236,12 @@ def init_agent( # targets. agent._task_completion_guidance = bool(_agent_section.get("task_completion_guidance", True)) + # Universal parallel-tool-call guidance toggle. Default True. Separate + # flag from task_completion_guidance because a user may want one but not + # the other. Steers the model to batch independent tool calls into a + # single turn; the runtime already executes such batches concurrently. + agent._parallel_tool_call_guidance = bool(_agent_section.get("parallel_tool_call_guidance", True)) + # Local Python toolchain probe toggle. Default True. When False, # the probe is skipped entirely (no subprocess calls, no system-prompt # line). Useful for users on exotic setups where the probe heuristics diff --git a/agent/agent_runtime_helpers.py b/agent/agent_runtime_helpers.py index 884866dc11..4a267f9559 100644 --- a/agent/agent_runtime_helpers.py +++ b/agent/agent_runtime_helpers.py @@ -1839,28 +1839,42 @@ def invoke_tool(agent, function_name: str, function_args: dict, effective_task_i elif function_name == "memory": def _execute(next_args: dict) -> Any: target = next_args.get("target", "memory") + operations = next_args.get("operations") from tools.memory_tool import memory_tool as _memory_tool result = _memory_tool( action=next_args.get("action"), target=target, content=next_args.get("content"), old_text=next_args.get("old_text"), + operations=operations, store=agent._memory_store, ) - # Bridge: notify external memory provider of built-in memory writes - if agent._memory_manager and next_args.get("action") in {"add", "replace"}: - try: - agent._memory_manager.on_memory_write( - next_args.get("action", ""), - target, - next_args.get("content", ""), - metadata=agent._build_memory_write_metadata( - task_id=effective_task_id, - tool_call_id=tool_call_id, - ), + # Bridge: notify external memory provider of built-in memory writes. + # Covers both the single-op shape and each add/replace inside a batch. + if agent._memory_manager: + if operations: + _mem_ops = [ + op for op in operations + if isinstance(op, dict) and op.get("action") in {"add", "replace"} + ] + else: + _mem_ops = ( + [{"action": next_args.get("action"), "content": next_args.get("content")}] + if next_args.get("action") in {"add", "replace"} else [] ) - except Exception: - pass + for _op in _mem_ops: + try: + agent._memory_manager.on_memory_write( + _op.get("action", ""), + target, + _op.get("content", "") or "", + metadata=agent._build_memory_write_metadata( + task_id=effective_task_id, + tool_call_id=tool_call_id, + ), + ) + except Exception: + pass return _finish_agent_tool(result, next_args) elif agent._memory_manager and agent._memory_manager.has_tool(function_name): def _execute(next_args: dict) -> Any: diff --git a/agent/anthropic_adapter.py b/agent/anthropic_adapter.py index 3a2d3f68e1..4a586d7f0f 100644 --- a/agent/anthropic_adapter.py +++ b/agent/anthropic_adapter.py @@ -372,7 +372,7 @@ def _detect_claude_code_version() -> str: _CLAUDE_CODE_SYSTEM_PREFIX = "You are Claude Code, Anthropic's official CLI for Claude." -_MCP_TOOL_PREFIX = "mcp_" +_MCP_TOOL_PREFIX = "mcp__" def _get_claude_code_version() -> str: @@ -2349,25 +2349,46 @@ def build_anthropic_kwargs( text = text.replace("Nous Research", "Anthropic") block["text"] = text - # 3. Prefix tool names with mcp_ (Claude Code convention) - # Skip names that already begin with the marker — native MCP server - # tools (from mcp_servers: in config.yaml) are registered under their - # full mcp__ name and would double-prefix otherwise, - # breaking round-trip registry lookup in normalize_response. GH-25255. + # 3. Normalize tool names so NOTHING goes on the OAuth wire with a + # single-underscore ``mcp_`` prefix. Anthropic's subscription/OAuth + # billing classifier treats a single-underscore ``mcp_`` tool name as + # a third-party-app fingerprint and rejects the request with HTTP 400 + # "Third-party apps now draw from extra usage, not plan limits" + # (verified empirically: a single ``mcp_foo`` tool flips a request + # from plan-billing to the extra-usage lane; ``mcp__foo`` is accepted). + # + # Two cases, both must land on the double-underscore ``mcp__`` form: + # a) bare Hermes-native tools (``read_file``) -> ``mcp__read_file`` + # b) native MCP server tools registered under their full + # single-underscore ``mcp__`` name + # (``mcp_linear_get_issue``) -> ``mcp__linear_get_issue`` + # Case (b) is the gap that the bare ``mcp_``->``mcp__`` constant swap + # left open: those tools were *skipped* and stayed single-underscore, + # so any session with an MCP server configured still tripped the + # classifier. normalize_response reverses both forms via registry + # lookup so the dispatcher still sees the original name. GH-25255. + def _to_oauth_wire_name(name: str) -> str: + if name.startswith("mcp__"): + return name # already correct, don't double-prefix + if name.startswith("mcp_"): + # single-underscore native MCP tool -> promote to double + return "mcp__" + name[len("mcp_"):] + return _MCP_TOOL_PREFIX + name # bare name -> mcp__ + if anthropic_tools: for tool in anthropic_tools: - if "name" in tool and not tool["name"].startswith(_MCP_TOOL_PREFIX): - tool["name"] = _MCP_TOOL_PREFIX + tool["name"] + if "name" in tool: + tool["name"] = _to_oauth_wire_name(tool["name"]) - # 4. Prefix tool names in message history (tool_use and tool_result blocks) + # 4. Apply the same normalization to tool names in message history + # (tool_use blocks) so replayed turns match the wire names above. for msg in anthropic_messages: content = msg.get("content") if isinstance(content, list): for block in content: if isinstance(block, dict): if block.get("type") == "tool_use" and "name" in block: - if not block["name"].startswith(_MCP_TOOL_PREFIX): - block["name"] = _MCP_TOOL_PREFIX + block["name"] + block["name"] = _to_oauth_wire_name(block["name"]) elif block.get("type") == "tool_result" and "tool_use_id" in block: pass # tool_result uses ID, not name diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py index 01ea45d7be..86a1c765a7 100644 --- a/agent/auxiliary_client.py +++ b/agent/auxiliary_client.py @@ -3079,23 +3079,20 @@ def _try_configured_fallback_chain( if not fb_provider or fb_provider.lower() == skip: continue fb_model = str(entry.get("model", "")).strip() or None - fb_base_url = str(entry.get("base_url", "")).strip() or None - fb_api_key = str(entry.get("api_key", "")).strip() or None label = f"fallback_chain[{i}]({fb_provider})" try: - fb_client = _resolve_single_provider( - fb_provider, fb_model, fb_base_url, fb_api_key) + fb_client, resolved_model = _resolve_fallback_entry(entry) except Exception: - fb_client = None + fb_client, resolved_model = None, None if fb_client is not None: logger.info( "Auxiliary %s: %s on %s — configured fallback to %s (%s)", - task, reason, failed_provider, label, fb_model or "default", + task, reason, failed_provider, label, resolved_model or fb_model or "default", ) - return fb_client, fb_model, label + return fb_client, resolved_model or fb_model, label tried.append(label) if tried: @@ -3106,6 +3103,103 @@ def _try_configured_fallback_chain( return None, None, "" +def _fallback_entry_api_key(entry: Dict[str, Any]) -> Optional[str]: + """Resolve inline or env-backed API key from a fallback-chain entry.""" + explicit = str(entry.get("api_key") or "").strip() + if explicit: + return explicit + key_env = str(entry.get("key_env") or entry.get("api_key_env") or "").strip() + if key_env: + return os.getenv(key_env, "").strip() or None + return None + + +def _resolve_fallback_entry(entry: Dict[str, Any]) -> Tuple[Optional[Any], Optional[str]]: + """Resolve one fallback entry through the central provider router.""" + provider = str(entry.get("provider") or "").strip() + model = str(entry.get("model") or "").strip() or None + if not provider or not model: + return None, None + base_url = str(entry.get("base_url") or "").strip() or None + api_key = _fallback_entry_api_key(entry) + api_mode = str(entry.get("api_mode") or entry.get("transport") or "").strip() or None + return resolve_provider_client( + provider, + model=model, + explicit_base_url=base_url, + explicit_api_key=api_key, + api_mode=api_mode, + ) + + +def _try_main_fallback_chain( + task: Optional[str], + failed_provider: str = "", + reason: str = "error", +) -> Tuple[Optional[Any], Optional[str], str]: + """Try the top-level main-agent fallback chain for an auxiliary call. + + ``provider: auto`` auxiliary tasks should respect the user's declared + main fallback policy before dropping into Hermes' built-in discovery + chain. The top-level chain is read through ``get_fallback_chain`` so + both modern ``fallback_providers`` and legacy ``fallback_model`` entries + participate in the same order as the main agent. + """ + try: + from hermes_cli.config import load_config + from hermes_cli.fallback_config import get_fallback_chain + + chain = get_fallback_chain(load_config()) + except Exception as exc: + logger.debug("Auxiliary %s: could not load main fallback chain: %s", task or "call", exc) + return None, None, "" + + if not chain: + return None, None, "" + + failed_norm = (failed_provider or "").strip().lower() + main_norm = (_read_main_provider() or "").strip().lower() + skip = {p for p in (failed_norm, main_norm, "auto") if p} + tried: List[str] = [] + + for i, entry in enumerate(chain): + if not isinstance(entry, dict): + continue + fb_provider = str(entry.get("provider") or "").strip() + fb_model = str(entry.get("model") or "").strip() + if not fb_provider or not fb_model: + continue + fb_norm = fb_provider.lower() + label = f"fallback_providers[{i}]({fb_provider})" + if fb_norm in skip: + tried.append(f"{label} (skipped)") + continue + if _is_provider_unhealthy(fb_norm): + _log_skip_unhealthy(fb_norm, task) + tried.append(f"{label} (unhealthy)") + continue + try: + fb_client, resolved_model = _resolve_fallback_entry(entry) + except Exception as exc: + logger.debug("Auxiliary %s: main fallback %s failed to resolve: %s", task or "call", label, exc) + fb_client, resolved_model = None, None + if fb_client is not None: + logger.info( + "Auxiliary %s: %s on %s — main fallback chain to %s (%s)", + task or "call", reason, failed_provider or "auto", label, + resolved_model or fb_model, + ) + return fb_client, resolved_model or fb_model, fb_provider + tried.append(label) + + if tried: + logger.debug( + "Auxiliary %s: main fallback chain exhausted (tried: %s)", + task or "call", ", ".join(tried), + ) + return None, None, "" + + def _resolve_single_provider( provider: str, model: Optional[str] = None, @@ -3116,16 +3210,19 @@ def _resolve_single_provider( Uses the existing provider resolution infrastructure where possible. """ - # Reuse resolve_provider_client which handles provider→client mapping + # Reuse resolve_provider_client which handles provider→client mapping. client, resolved_model = resolve_provider_client( provider=provider, model=model, - base_url=base_url, - api_key=api_key, + explicit_base_url=base_url, + explicit_api_key=api_key, ) return client -def _resolve_auto(main_runtime: Optional[Dict[str, Any]] = None) -> Tuple[Optional[OpenAI], Optional[str]]: +def _resolve_auto( + main_runtime: Optional[Dict[str, Any]] = None, + task: Optional[str] = None, +) -> Tuple[Optional[OpenAI], Optional[str]]: """Full auto-detection chain. Priority: @@ -3223,7 +3320,22 @@ def _resolve_auto(main_runtime: Optional[Dict[str, Any]] = None) -> Tuple[Option main_provider, resolved or main_model) return client, resolved or main_model - # ── Step 2: aggregator / fallback chain ────────────────────────────── + # ── Step 2: user-configured fallback policy ───────────────────────── + # In auto mode, respect the task-specific fallback chain first, then the + # main agent's top-level fallback_providers/fallback_model chain. The + # hardcoded provider discovery chain below is only the convenience default + # for users who have not declared a fallback policy. + if task: + fb_client, fb_model, _fb_label = _try_configured_fallback_chain( + task, main_provider or "auto", reason="main provider unavailable") + if fb_client is not None: + return fb_client, fb_model + fb_client, fb_model, _fb_label = _try_main_fallback_chain( + task, main_provider or "auto", reason="main provider unavailable") + if fb_client is not None: + return fb_client, fb_model + + # ── Step 3: aggregator / fallback chain ────────────────────────────── tried = [] for label, try_fn in _get_provider_chain(): if _is_provider_unhealthy(label): @@ -3344,6 +3456,7 @@ def resolve_provider_client( api_mode: str = None, main_runtime: Optional[Dict[str, Any]] = None, is_vision: bool = False, + task: Optional[str] = None, ) -> Tuple[Optional[Any], Optional[str]]: """Central router: given a provider name and optional model, return a configured client with the correct auth, base URL, and API format. @@ -3464,7 +3577,7 @@ def resolve_provider_client( # ── Auto: try all providers in priority order ──────────────────── if provider == "auto": - client, resolved = _resolve_auto(main_runtime=main_runtime) + client, resolved = _resolve_auto(main_runtime=main_runtime, task=task) if client is None: return None, None # When auto-detection lands on a non-OpenRouter provider (e.g. a @@ -4357,11 +4470,16 @@ def _client_cache_key( api_mode: Optional[str] = None, main_runtime: Optional[Dict[str, Any]] = None, is_vision: bool = False, + task: Optional[str] = None, ) -> tuple: runtime = _normalize_main_runtime(main_runtime) runtime_key = tuple(runtime.get(field, "") for field in _MAIN_RUNTIME_FIELDS) if provider == "auto" else () + # `auto` can now resolve through task-specific or main fallback policy, + # so the task participates in the cache key. Non-auto providers keep the + # old cache shape because the explicit provider/model tuple is sufficient. + task_key = (task or "") if provider == "auto" else "" pool_hint = _pool_cache_hint(provider, main_runtime=main_runtime) - return (provider, async_mode, base_url or "", api_key or "", api_mode or "", runtime_key, is_vision, pool_hint) + return (provider, async_mode, base_url or "", api_key or "", api_mode or "", runtime_key, is_vision, task_key, pool_hint) def _store_cached_client(cache_key: tuple, client: Any, default_model: Optional[str], *, bound_loop: Any = None) -> None: @@ -4554,6 +4672,7 @@ def _get_cached_client( api_mode: str = None, main_runtime: Optional[Dict[str, Any]] = None, is_vision: bool = False, + task: Optional[str] = None, ) -> Tuple[Optional[Any], Optional[str]]: """Get or create a cached client for the given provider. @@ -4591,6 +4710,7 @@ def _get_cached_client( api_mode=api_mode, main_runtime=main_runtime, is_vision=is_vision, + task=task, ) with _client_cache_lock: if cache_key in _client_cache: @@ -4635,6 +4755,7 @@ def _get_cached_client( api_mode=api_mode, main_runtime=runtime, is_vision=is_vision, + task=task, ) if client is not None: # For async clients, remember which loop they were created on so we @@ -5140,7 +5261,7 @@ def call_llm( if not resolved_base_url: logger.info("Auxiliary %s: provider %s unavailable, trying auto-detection chain", task or "call", resolved_provider) - client, final_model = _get_cached_client("auto", main_runtime=main_runtime) + client, final_model = _get_cached_client("auto", main_runtime=main_runtime, task=task) if client is None: raise RuntimeError( f"No LLM provider configured for task={task} provider={resolved_provider}. " @@ -5466,14 +5587,19 @@ def call_llm( # Fallback order (#26882, #26803): # 1. User-configured fallback_chain (per-task) if set - # 2. Main agent model (last-resort safety net) - # For auto users (no explicit aux provider), use the full - # auto-detection chain instead — its Step 1 IS the main agent - # model, so users on `auto` already get main-model fallback. + # 2. For auto: top-level main fallback_providers/fallback_model + # 3. For auto: built-in auxiliary discovery chain + # 4. For explicit aux providers: main agent model safety net fb_client, fb_model, fb_label = (None, None, "") if is_auto: - fb_client, fb_model, fb_label = _try_payment_fallback( - resolved_provider, task, reason=reason) + fb_client, fb_model, fb_label = _try_configured_fallback_chain( + task, resolved_provider or "auto", reason=reason) + if fb_client is None: + fb_client, fb_model, fb_label = _try_main_fallback_chain( + task, resolved_provider or "auto", reason=reason) + if fb_client is None: + fb_client, fb_model, fb_label = _try_payment_fallback( + resolved_provider, task, reason=reason) else: fb_client, fb_model, fb_label = _try_configured_fallback_chain( task, resolved_provider or "auto", reason=reason) @@ -5636,7 +5762,7 @@ async def async_call_llm( if not resolved_base_url: logger.info("Auxiliary %s: provider %s unavailable, trying auto-detection chain", task or "call", resolved_provider) - client, final_model = _get_cached_client("auto", async_mode=True) + client, final_model = _get_cached_client("auto", async_mode=True, main_runtime=main_runtime, task=task) if client is None: raise RuntimeError( f"No LLM provider configured for task={task} provider={resolved_provider}. " @@ -5904,13 +6030,19 @@ async def async_call_llm( # Fallback order (#26882, #26803): # 1. User-configured fallback_chain (per-task) if set - # 2. Main agent model (last-resort safety net) - # Auto users get the full auto-detection chain instead — its - # Step 1 IS the main agent model. + # 2. For auto: top-level main fallback_providers/fallback_model + # 3. For auto: built-in auxiliary discovery chain + # 4. For explicit aux providers: main agent model safety net fb_client, fb_model, fb_label = (None, None, "") if is_auto: - fb_client, fb_model, fb_label = _try_payment_fallback( - resolved_provider, task, reason=reason) + fb_client, fb_model, fb_label = _try_configured_fallback_chain( + task, resolved_provider or "auto", reason=reason) + if fb_client is None: + fb_client, fb_model, fb_label = _try_main_fallback_chain( + task, resolved_provider or "auto", reason=reason) + if fb_client is None: + fb_client, fb_model, fb_label = _try_payment_fallback( + resolved_provider, task, reason=reason) else: fb_client, fb_model, fb_label = _try_configured_fallback_chain( task, resolved_provider or "auto", reason=reason) diff --git a/agent/background_review.py b/agent/background_review.py index 2c9703ba68..ee4791d98d 100644 --- a/agent/background_review.py +++ b/agent/background_review.py @@ -300,6 +300,7 @@ def summarize_background_review_actions( "target": args.get("target", "memory"), "content": args.get("content", ""), "old_text": args.get("old_text", ""), + "operations": args.get("operations") or [], "name": args.get("name", ""), "old_string": args.get("old_string", ""), "new_string": args.get("new_string", ""), @@ -353,6 +354,7 @@ def summarize_background_review_actions( content = detail.get("content", "") old_text = detail.get("old_text", "") skill_name = detail.get("name", "") + operations = detail.get("operations") or [] max_preview = 120 if is_skill: change = data.get("_change", {}) @@ -376,6 +378,21 @@ def summarize_background_review_actions( actions.append(f"📝 Skill '{skill_name}' rewritten: {description}") else: actions.append(f"📝 {message}" if message else f"Skill {action}") + elif operations: + for op in operations: + op = op or {} + op_act = op.get("action", "") + op_content = (op.get("content") or "") + op_old = (op.get("old_text") or "") + if op_act == "add" and op_content: + preview = op_content[:max_preview] + ("…" if len(op_content) > max_preview else "") + actions.append(f"{label} ➕ {preview}") + elif op_act == "replace" and op_content: + preview = op_content[:max_preview] + ("…" if len(op_content) > max_preview else "") + actions.append(f"{label} ✏️ {preview}") + elif op_act == "remove" and op_old: + preview = op_old[:60] + ("…" if len(op_old) > 60 else "") + actions.append(f"{label} ➖ {preview}") elif action == "add" and content: preview = content[:max_preview] + ("…" if len(content) > max_preview else "") actions.append(f"{label} ➕ {preview}") @@ -391,6 +408,7 @@ def summarize_background_review_actions( "added" in message_lower or "replaced" in message_lower or "removed" in message_lower + or "applied" in message_lower or (target and "add" in message.lower()) or "Entry added" in message ): diff --git a/agent/codex_responses_adapter.py b/agent/codex_responses_adapter.py index b8479141db..e9b6ace9b8 100644 --- a/agent/codex_responses_adapter.py +++ b/agent/codex_responses_adapter.py @@ -262,6 +262,26 @@ def _responses_tools(tools: Optional[List[Dict[str, Any]]] = None) -> Optional[L return converted or None +# Provider-executed built-in tool *declaration* types accepted on the +# Responses ``tools`` array. These are declared by ``type`` alone (no +# client-side name/parameters schema) and run server-side — the provider +# owns the implementation and reports progress via the matching ``*_call`` +# output items. Hermes injects xAI's native ``web_search`` for the xAI +# transport (see agent/transports/codex.py); the rest are listed so the +# preflight validator passes them through rather than rejecting them as +# "unsupported type". Mirrors the ``*_call`` item-type set used in +# _normalize_codex_response. +_RESPONSES_BUILTIN_TOOL_TYPES = { + "web_search", + "web_search_preview", + "file_search", + "code_interpreter", + "image_generation", + "computer_use_preview", + "local_shell", +} + + # --------------------------------------------------------------------------- # Message format conversion # --------------------------------------------------------------------------- @@ -802,7 +822,22 @@ def _preflight_codex_api_kwargs( for idx, tool in enumerate(tools): if not isinstance(tool, dict): raise ValueError(f"Codex Responses tools[{idx}] must be an object.") - if tool.get("type") != "function": + + tool_type = tool.get("type") + + # Provider-executed built-in tools (xAI native web_search, code + # interpreter, etc.) are declared by ``type`` alone and carry no + # ``name``/``parameters`` schema — the provider owns the + # implementation. Pass them through verbatim instead of forcing + # them through the function-tool validation below (which would + # otherwise reject them with "unsupported type"). See + # agent/transports/codex.py for where xAI's native web_search is + # injected. + if tool_type in _RESPONSES_BUILTIN_TOOL_TYPES: + normalized_tools.append(dict(tool)) + continue + + if tool_type != "function": raise ValueError(f"Codex Responses tools[{idx}] has unsupported type {tool.get('type')!r}.") name = tool.get("name") @@ -1086,6 +1121,33 @@ def _normalize_codex_response( saw_final_answer_phase = False saw_reasoning_item = False + # Server-side built-in tool calls (xAI's native web_search, code + # interpreter, etc.) are executed by the provider and reported as + # discrete ``*_call`` output items. xAI's /v1/responses surface + # (e.g. grok-composer-2.5-fast on SuperGrok OAuth) routinely leaves + # these items at ``status="in_progress"`` even when the overall + # ``response.status == "completed"`` — the search ran to completion + # server-side, the per-item status simply isn't reconciled. These + # are NOT a signal that the model's turn is unfinished, so they must + # not flip ``has_incomplete_items``. Only the response-level status + # and genuine model output items (message/reasoning/function_call) + # govern the incomplete verdict. Without this guard, any turn where + # grok-composer invokes server-side search is misclassified as + # ``finish_reason="incomplete"`` and burns 3 fruitless continuation + # retries before failing with "Codex response remained incomplete + # after 3 continuation attempts". client-side function/custom tool + # calls keep their own in_progress handling below (they are skipped, + # not awaited). + _SERVER_SIDE_TOOL_CALL_TYPES = { + "web_search_call", + "file_search_call", + "code_interpreter_call", + "image_generation_call", + "computer_call", + "local_shell_call", + "mcp_call", + } + for item in output: item_type = getattr(item, "type", None) item_status = getattr(item, "status", None) @@ -1094,7 +1156,10 @@ def _normalize_codex_response( else: item_status = None - if item_status in {"queued", "in_progress", "incomplete"}: + if ( + item_status in {"queued", "in_progress", "incomplete"} + and item_type not in _SERVER_SIDE_TOOL_CALL_TYPES + ): has_incomplete_items = True saw_streaming_or_item_incomplete = True diff --git a/agent/conversation_compression.py b/agent/conversation_compression.py index d5469a1b34..318e67d0fa 100644 --- a/agent/conversation_compression.py +++ b/agent/conversation_compression.py @@ -603,6 +603,20 @@ def compress_context( force=True, ) + # Emit session:compress event so hooks (e.g. MemPalace sync) can ingest + # the completed old session before its details are lost. + _old_sid_for_event = locals().get("old_session_id") + if getattr(agent, "event_callback", None): + try: + agent.event_callback("session:compress", { + "platform": agent.platform or "", + "session_id": agent.session_id, + "old_session_id": _old_sid_for_event or "", + "compression_count": agent.context_compressor.compression_count, + }) + except Exception as e: + logger.debug("event_callback error on session:compress: %s", e) + # Keep the post-compression rough estimate for diagnostics, but do not # treat it as provider-reported prompt usage. Schema-heavy rough estimates # can remain above threshold even after the next real API request fits. diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index fadc2fdfd8..d33936fca3 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -300,11 +300,20 @@ def _restore_or_build_system_prompt(agent, system_message, conversation_history) agent.session_id, exc, ) - if stored_prompt: + if stored_prompt and _stored_prompt_matches_runtime(agent, stored_prompt): # Continuing session — reuse the exact system prompt from the # previous turn so the Anthropic cache prefix matches. agent._cached_system_prompt = stored_prompt return + if stored_prompt: + stored_state = "stale_runtime" + logger.info( + "Stored system prompt for session %s has stale runtime identity; " + "rebuilding for model=%s provider=%s.", + agent.session_id, + getattr(agent, "model", "") or "", + getattr(agent, "provider", "") or "", + ) if conversation_history and stored_state in ("null", "empty"): # Continuing session whose stored prompt is unusable. The @@ -366,6 +375,30 @@ def _restore_or_build_system_prompt(agent, system_message, conversation_history) ) +def _stored_prompt_matches_runtime(agent, prompt: str) -> bool: + """Return False when the persisted Model/Provider lines are stale.""" + + def line_value(label: str) -> str: + prefix = f"{label}:" + value = "" + for line in prompt.splitlines(): + if line.startswith(prefix): + value = line[len(prefix):].strip() + return value + + stored_model = line_value("Model") + current_model = str(getattr(agent, "model", "") or "").strip() + if stored_model and current_model and stored_model != current_model: + return False + + stored_provider = line_value("Provider") + current_provider = str(getattr(agent, "provider", "") or "").strip() + if stored_provider and current_provider and stored_provider != current_provider: + return False + + return True + + def _get_continuation_prompt(is_partial_stub: bool, dropped_tools: Optional[List[str]] = None) -> str: if is_partial_stub and dropped_tools: tool_list = ", ".join(dropped_tools[:3]) @@ -441,6 +474,7 @@ def run_conversation( task_id: str = None, stream_callback: Optional[callable] = None, persist_user_message: Optional[str] = None, + persist_user_timestamp: Optional[float] = None, moa_config: Optional[dict[str, Any]] = None, ) -> Dict[str, Any]: """ @@ -457,6 +491,8 @@ def run_conversation( persist_user_message: Optional clean user message to store in transcripts/history when user_message contains API-only synthetic prefixes. + persist_user_timestamp: Optional platform event timestamp to store + as metadata on that persisted user message. or queuing follow-up prefetch work. Returns: @@ -491,6 +527,7 @@ def run_conversation( task_id, stream_callback, persist_user_message, + persist_user_timestamp, restore_or_build_system_prompt=_restore_or_build_system_prompt, install_safe_stdio=_install_safe_stdio, sanitize_surrogates=_sanitize_surrogates, @@ -3756,8 +3793,30 @@ def run_conversation( assistant_msg = agent._build_assistant_message(assistant_message, finish_reason) messages.append(assistant_msg) for tc in assistant_message.tool_calls: - if tc.function.name not in agent.valid_tool_names: - content = f"Tool '{tc.function.name}' does not exist. Available tools: {available}" + _tc_name = tc.function.name + if _tc_name not in agent.valid_tool_names: + # A blank/whitespace-only name is not a typo the + # model can fuzzy-correct toward a real tool — it is + # almost always a weak open model echoing tool-call + # XML/JSON it saw in file or tool output (#47967: + # / payloads in a file + # prime mimo/nemotron-class models to emit empty + # structured calls). Dumping the full tool catalog + # in that case feeds the priming loop more names to + # mimic and inflates context 3-4x across retries, so + # send a terse error that tells the model in-context + # tool-call syntax is DATA, not a call to make. + if not (_tc_name or "").strip(): + content = ( + "Tool call rejected: the tool name was empty. " + "If tool-call XML or JSON appeared in file " + "contents or tool output, that is data — do " + "not re-emit it as a tool call. To call a " + "tool, use a valid name from your tool list; " + "otherwise reply in plain text." + ) + else: + content = f"Tool '{_tc_name}' does not exist. Available tools: {available}" else: content = "Skipped: another tool call in this turn used an invalid name. Please retry this tool call." messages.append({ diff --git a/agent/curator.py b/agent/curator.py index 62630ce453..0ceebecbff 100644 --- a/agent/curator.py +++ b/agent/curator.py @@ -57,6 +57,11 @@ DEFAULT_INTERVAL_HOURS = 24 * 7 # 7 days DEFAULT_MIN_IDLE_HOURS = 2 DEFAULT_STALE_AFTER_DAYS = 30 DEFAULT_ARCHIVE_AFTER_DAYS = 90 +# Consolidation (the LLM umbrella-building fork) is OFF by default. The +# deterministic inactivity prune (apply_automatic_transitions) still runs +# whenever the curator is enabled; only the opinionated, aux-model-cost +# consolidation pass is opt-in. +DEFAULT_CONSOLIDATE = False # --------------------------------------------------------------------------- @@ -182,6 +187,22 @@ def get_prune_builtins() -> bool: return bool(cfg.get("prune_builtins", True)) +def get_consolidate() -> bool: + """Whether the curator runs its LLM consolidation (umbrella-building) pass. + + OFF by default. When off, a curator run does ONLY the deterministic + inactivity prune (mark stale / archive long-unused skills) and skips the + forked aux-model review entirely — no consolidation, no umbrella-building, + no aux-model cost. Set ``curator.consolidate: true`` to opt back into the + LLM pass that merges overlapping skills into class-level umbrellas. + + The explicit ``hermes curator run --consolidate`` flag overrides this for + a single invocation regardless of the config value. + """ + cfg = _load_config() + return bool(cfg.get("consolidate", DEFAULT_CONSOLIDATE)) + + # --------------------------------------------------------------------------- # Idle / interval check # --------------------------------------------------------------------------- @@ -1408,25 +1429,38 @@ def run_curator_review( on_summary: Optional[Callable[[str], None]] = None, synchronous: bool = False, dry_run: bool = False, + consolidate: Optional[bool] = None, ) -> Dict[str, Any]: """Execute a single curator review pass. Steps: 1. Apply automatic state transitions (pure, no LLM). - 2. If there are agent-created skills, spawn a forked AIAgent that runs - the LLM review prompt against the current candidate list. + 2. If consolidation is enabled AND there are agent-created skills, spawn + a forked AIAgent that runs the LLM review prompt against the current + candidate list. 3. Update .curator_state with last_run_at and a one-line summary. 4. Invoke *on_summary* with a user-visible description. If *synchronous* is True, the LLM review runs in the calling thread; the default is to spawn a daemon thread so the caller returns immediately. + *consolidate* gates the LLM umbrella-building pass. ``None`` (the default) + reads ``curator.consolidate`` from config (OFF by default). Passing + ``True``/``False`` overrides the config for this invocation — used by the + ``hermes curator run --consolidate`` flag. When consolidation is off, only + the deterministic inactivity prune runs and the forked aux-model review is + skipped entirely (no aux-model cost). + If *dry_run* is True, the automatic stale/archive transitions are SKIPPED and the LLM review pass is instructed to produce a report only — no skill_manage mutations, no terminal archive moves. The REPORT.md still gets written and ``state.last_report_path`` still records it so users - can read what the curator WOULD have done. + can read what the curator WOULD have done. A dry-run also honors + *consolidate*: when consolidation is off, the preview only reports the + deterministic prune candidates. """ + if consolidate is None: + consolidate = get_consolidate() start = datetime.now(timezone.utc) if dry_run: # Count candidates without mutating state. @@ -1489,6 +1523,53 @@ def run_curator_review( before_report = [] before_names = {r.get("name") for r in before_report if isinstance(r, dict)} + # Consolidation gate. When off (the default), the curator does ONLY the + # deterministic inactivity prune above — no forked aux-model review, no + # umbrella-building, no aux-model cost. Record the run, write a report + # reflecting the prune-only outcome, and return without spawning a fork. + if not consolidate: + final_summary = ( + f"{prefix}{auto_summary}; llm: skipped (consolidation off)" + ) + llm_meta = { + "final": "", + "summary": "skipped (consolidation off)", + "model": "", + "provider": "", + "tool_calls": [], + "error": None, + } + elapsed = (datetime.now(timezone.utc) - start).total_seconds() + state2 = load_state() + state2["last_run_duration_seconds"] = elapsed + state2["last_run_summary"] = final_summary + try: + after_report = skill_usage.agent_created_report() + except Exception: + after_report = [] + try: + report_path = _write_run_report( + started_at=start, + elapsed_seconds=elapsed, + auto_counts=counts, + auto_summary=auto_summary, + before_report=before_report, + before_names=before_names, + after_report=after_report, + llm_meta=llm_meta, + ) + if report_path is not None: + state2["last_report_path"] = str(report_path) + except Exception as e: + logger.debug("Curator report write failed: %s", e, exc_info=True) + save_state(state2) + if on_summary: + try: + on_summary(f"curator: {final_summary}") + except Exception: + pass + return + llm_meta: Dict[str, Any] = {} try: candidate_list = _render_candidate_list() diff --git a/agent/curator_backup.py b/agent/curator_backup.py index 944886d729..ddf8699e9b 100644 --- a/agent/curator_backup.py +++ b/agent/curator_backup.py @@ -46,7 +46,7 @@ import shutil import tarfile from datetime import datetime, timezone from pathlib import Path -from typing import Any, Dict, List, Optional, Tuple +from typing import Any, Dict, List, Optional, Set, Tuple from hermes_constants import get_hermes_home from agent.skill_utils import is_excluded_skill_path @@ -208,13 +208,17 @@ def _write_manifest(dest: Path, reason: str, archive_path: Path, ) -def snapshot_skills(reason: str = "manual") -> Optional[Path]: +def snapshot_skills(reason: str = "manual", *, protect_ids: Optional[Set[str]] = None) -> Optional[Path]: """Create a tar.gz snapshot of ``~/.hermes/skills/`` and prune old ones. Returns the snapshot directory path, or ``None`` if the snapshot was skipped (backup disabled, skills dir missing, or an IO error occurred — in which case we log at debug and return None so the curator never aborts a pass because of a backup failure). + + ``protect_ids`` is forwarded to the prune step so callers can guarantee + specific snapshot ids survive even when they fall outside the keep + window (rollback passes the id it is about to restore from). """ if not is_enabled(): logger.debug("Curator backup disabled by config; skipping snapshot") @@ -276,15 +280,19 @@ def snapshot_skills(reason: str = "manual") -> Optional[Path]: pass return None - _prune_old(keep=get_keep()) + _prune_old(keep=get_keep(), protect=protect_ids) logger.info("Curator snapshot created: %s (%s)", snap_id, reason) return dest -def _prune_old(keep: int) -> List[str]: +def _prune_old(keep: int, protect: Optional[Set[str]] = None) -> List[str]: """Delete regular snapshots beyond the newest *keep*. Returns deleted - ids. Staging dirs (``.rollback-staging-*``) are implementation detail - and pruned independently on every call.""" + ids. Snapshot ids in *protect* are never deleted even when they fall + outside the keep window — rollback() uses this so the mandatory + pre-rollback safety snapshot can never evict the very snapshot being + restored. Staging dirs (``.rollback-staging-*``) are implementation + detail and pruned independently on every call.""" + protect = protect or set() backups = _backups_dir() if not backups.exists(): return [] @@ -305,6 +313,8 @@ def _prune_old(keep: int) -> List[str]: entries.sort(key=lambda t: t[0], reverse=True) deleted: List[str] = [] for _, path in entries[keep:]: + if path.name in protect: + continue try: shutil.rmtree(path) deleted.append(path.name) @@ -564,7 +574,13 @@ def rollback(backup_id: Optional[str] = None) -> Tuple[bool, str, Optional[Path] # out before touching anything — otherwise a failed extract could leave # the user with no skills. try: - snapshot_skills(reason=f"pre-rollback to {target.name}") + # Protect the target from this snapshot's prune step: at the steady + # keep limit, pruning the oldest snapshot would otherwise delete the + # very snapshot we are about to extract from. + snapshot_skills( + reason=f"pre-rollback to {target.name}", + protect_ids={target.name}, + ) except Exception as e: return (False, f"pre-rollback safety snapshot failed: {e}", None) diff --git a/agent/memory_manager.py b/agent/memory_manager.py index 240595a4eb..dcd50a2997 100644 --- a/agent/memory_manager.py +++ b/agent/memory_manager.py @@ -33,6 +33,7 @@ from concurrent.futures import ThreadPoolExecutor from typing import Any, Dict, List, Optional from agent.memory_provider import MemoryProvider +from agent.skill_commands import extract_user_instruction_from_skill_message from tools.registry import tool_error logger = logging.getLogger(__name__) @@ -430,16 +431,37 @@ class MemoryManager: # -- Prefetch / recall --------------------------------------------------- + @staticmethod + def _strip_skill_scaffolding(text: str) -> Optional[str]: + """Return memory-worthy user text, or None to skip the turn. + + When a user invokes a /skill or /bundle, Hermes expands the turn into + a model-facing message that embeds the entire skill body. Feeding that + verbatim to memory providers pollutes their stores/embeddings with + prompt scaffolding instead of what the user actually asked. We recover + just the user's instruction here, once, for every provider — so this + is fixed for the whole provider fan-out, not per backend. + + - Non-skill messages pass through unchanged. + - Skill turns with a user instruction return that instruction. + - Bare skill invocations (no instruction) return None → callers skip + the turn, since there is no user content worth remembering. + """ + return extract_user_instruction_from_skill_message(text) + def prefetch_all(self, query: str, *, session_id: str = "") -> str: """Collect prefetch context from all providers. Returns merged context text labeled by provider. Empty providers are skipped. Failures in one provider don't block others. """ + clean_query = self._strip_skill_scaffolding(query) + if not clean_query: + return "" parts = [] for provider in self._providers: try: - result = provider.prefetch(query, session_id=session_id) + result = provider.prefetch(clean_query, session_id=session_id) if result and result.strip(): parts.append(result) except Exception as e: @@ -460,10 +482,14 @@ class MemoryManager: if not providers: return + clean_query = self._strip_skill_scaffolding(query) + if not clean_query: + return + def _run() -> None: for provider in providers: try: - provider.queue_prefetch(query, session_id=session_id) + provider.queue_prefetch(clean_query, session_id=session_id) except Exception as e: logger.debug( "Memory provider '%s' queue_prefetch failed (non-fatal): %s", @@ -515,6 +541,11 @@ class MemoryManager: if not providers: return + clean_user_content = self._strip_skill_scaffolding(user_content) + if not clean_user_content: + return + user_content = clean_user_content + def _run() -> None: for provider in providers: try: diff --git a/agent/model_metadata.py b/agent/model_metadata.py index e31fcdea48..4493eae5f1 100644 --- a/agent/model_metadata.py +++ b/agent/model_metadata.py @@ -275,6 +275,11 @@ DEFAULT_CONTEXT_LENGTHS = { # via a custom provider. Values sourced from models.dev (2026-04). # Keys use substring matching (longest-first), so e.g. "grok-4.20" # matches "grok-4.20-0309-reasoning" / "-non-reasoning" / "-multi-agent-0309". + # OAuth-only slug; absent from GET /v1/models. xAI publishes a 200k + # usable context window for Composer 2.5 on Grok Build (SuperGrok / + # Premium+); /v1/responses additionally enforces a ~262144 input+output + # budget, but the usable context (what we track here) is 200k. + "grok-composer": 200000, # grok-composer-2.5-fast (Grok Build CLI) "grok-build": 256000, # grok-build-0.1 "grok-code-fast": 256000, # grok-code-fast-1 "grok-2-vision": 8192, # grok-2-vision, -1212, -latest diff --git a/agent/prompt_builder.py b/agent/prompt_builder.py index b11cade39b..97836f27b0 100644 --- a/agent/prompt_builder.py +++ b/agent/prompt_builder.py @@ -8,6 +8,7 @@ import json import logging import os import threading +import contextvars from collections import OrderedDict from pathlib import Path @@ -304,6 +305,47 @@ TASK_COMPLETION_GUIDANCE = ( "is always better than inventing a result." ) +# Universal parallel-tool-call guidance — applied to ALL models. +# +# Why this matters for cost: every assistant turn resends the entire +# accumulated conversation (and, on cache-friendly providers, re-reads the +# cached prefix and pays for the newly-appended turn). A model that issues +# one tool call per turn multiplies the number of round-trips — and therefore +# the resent context — for any task that needs several independent reads, +# searches, or safe lookups. Batching independent calls into a single +# assistant response collapses N turns into one, cutting both latency and the +# resent-context cost that compounds over a long conversation. +# +# The hermes-agent runtime already executes a batch of tool calls +# concurrently when they are independent (read-only tools always; path-scoped +# file ops when their targets don't overlap — see +# run_agent._execute_tool_calls / tool_dispatch_helpers). The missing piece +# was telling the *model* to emit those calls together in the first place. +# Until now the only batching steer in the prompt lived in +# GOOGLE_MODEL_OPERATIONAL_GUIDANCE — Gemini/Gemma got it, every other model +# got nothing. This block makes the steer universal; the now-redundant +# Google-only bullet has been dropped so no model receives it twice. +# +# Short on purpose — shipped in the cached system prompt to every user, every +# session. Token cost is paid once at install and amortised across all +# sessions via prefix caching. Keep it tight. +# +# Ported from cline/cline#11514 ("encourage parallel tool calls"), adapted +# from Cline's TypeScript tool-surface guidance to hermes-agent's Python +# prompt-assembly architecture. +PARALLEL_TOOL_CALL_GUIDANCE = ( + "# Parallel tool calls\n" + "When you need several pieces of information that don't depend on each " + "other, request them together in a single response instead of one tool " + "call per turn. Independent reads, searches, web fetches, and read-only " + "commands should be batched into the same assistant turn — the runtime " + "executes independent calls concurrently, and batching avoids resending " + "the whole conversation on every extra round-trip.\n" + "Only serialize calls when a later call genuinely depends on an earlier " + "call's result (e.g. you must read a file before you can patch it). When " + "in doubt and the calls are independent, batch them." +) + # OpenAI GPT/Codex-specific execution guidance. Addresses known failure modes # where GPT models abandon work on partial results, skip prerequisite lookups, # hallucinate instead of using tools, and declare "done" without verification. @@ -385,9 +427,10 @@ GOOGLE_MODEL_OPERATIONAL_GUIDANCE = ( "package.json, requirements.txt, Cargo.toml, etc. before importing.\n" "- **Conciseness:** Keep explanatory text brief — a few sentences, not " "paragraphs. Focus on actions and results over narration.\n" - "- **Parallel tool calls:** When you need to perform multiple independent " - "operations (e.g. reading several files), make all the tool calls in a " - "single response rather than sequentially.\n" + # Parallel-tool-call steering now lives in the universal + # PARALLEL_TOOL_CALL_GUIDANCE block (injected for all models), so it is no + # longer duplicated here — keeping it would send Gemini/Gemma the same + # instruction twice. "- **Non-interactive commands:** Use flags like -y, --yes, --non-interactive " "to prevent CLI tools from hanging on prompts.\n" "- **Keep going:** Work autonomously until the task is fully resolved. " @@ -957,6 +1000,80 @@ CONTEXT_FILE_MAX_CHARS = 20_000 CONTEXT_TRUNCATE_HEAD_RATIO = 0.7 CONTEXT_TRUNCATE_TAIL_RATIO = 0.2 +# Dynamic-cap parameters (used when no explicit context_file_max_chars is set). +# The cap scales with the model's context window so large-context models rarely +# truncate a project doc, while small-context models stay at the historical +# 20K floor. ~4 chars/token is the usual English heuristic; we spend a small +# slice of the window on context files since they share the cached prefix with +# the system prompt, tools, memory, and the whole conversation. +_CONTEXT_FILE_CHARS_PER_TOKEN = 4 +_CONTEXT_FILE_WINDOW_FRACTION = 0.06 +_CONTEXT_FILE_DYNAMIC_CEILING = 500_000 + + +def _dynamic_context_file_max_chars(context_length: Optional[int]) -> int: + """Derive a char cap from the model's context window. + + Returns at least ``CONTEXT_FILE_MAX_CHARS`` (the historical 20K floor) and + at most ``_CONTEXT_FILE_DYNAMIC_CEILING``. When ``context_length`` is + unknown/invalid, returns the flat default so behavior is unchanged. + """ + if not isinstance(context_length, int) or context_length <= 0: + return CONTEXT_FILE_MAX_CHARS + budget = int( + context_length * _CONTEXT_FILE_CHARS_PER_TOKEN * _CONTEXT_FILE_WINDOW_FRACTION + ) + return max(CONTEXT_FILE_MAX_CHARS, min(budget, _CONTEXT_FILE_DYNAMIC_CEILING)) + + +def _get_context_file_max_chars(context_length: Optional[int] = None) -> int: + """Return the context-file truncation limit. + + Resolution order: + 1. Explicit ``context_file_max_chars`` in config.yaml — user knows best, + always wins (including over the dynamic cap). + 2. Dynamic cap derived from the model's ``context_length`` when provided + (scales the budget to the window; floor 20K, ceiling 500K). + 3. ``CONTEXT_FILE_MAX_CHARS`` (20K) as the upstream-compatible fallback. + """ + try: + from hermes_cli.config import load_config + + val = load_config().get("context_file_max_chars") + if isinstance(val, (int, float)) and val > 0: + return int(val) + except Exception as e: + logger.debug("Could not read context_file_max_chars from config: %s", e) + return _dynamic_context_file_max_chars(context_length) + +# Collect truncation warnings so the caller (run_agent) can surface them. +# A ContextVar (not a module-global list) isolates accumulation per thread / +# per async task, so concurrent gateway-session prompt builds can't drain or +# clear each other's pending warnings (cross-session leak). Each build runs in +# its own context, collects its own warnings, and drains them synchronously. +_truncation_warnings: "contextvars.ContextVar[Optional[list]]" = contextvars.ContextVar( + "context_file_truncation_warnings", default=None +) + + +def _record_truncation_warning(msg: str) -> None: + """Append a truncation warning to the current context's accumulator.""" + warnings = _truncation_warnings.get() + if warnings is None: + warnings = [] + _truncation_warnings.set(warnings) + warnings.append(msg) + + +def drain_truncation_warnings() -> list: + """Return and clear any truncation warnings accumulated in this context.""" + warnings = _truncation_warnings.get() + if not warnings: + return [] + drained = list(warnings) + warnings.clear() + return drained + # ========================================================================= # Skills prompt cache @@ -1463,19 +1580,47 @@ def build_nous_subscription_prompt(valid_tool_names: "set[str] | None" = None) - # Context files (SOUL.md, AGENTS.md, .cursorrules) # ========================================================================= -def _truncate_content(content: str, filename: str, max_chars: int = CONTEXT_FILE_MAX_CHARS) -> str: - """Head/tail truncation with a marker in the middle.""" +def _truncate_content( + content: str, + filename: str, + max_chars: Optional[int] = None, + context_length: Optional[int] = None, + read_path: Optional[str] = None, +) -> str: + """Head/tail truncation with a marker in the middle. + + ``filename`` is the human label used in warnings. ``read_path`` is the + concrete path the agent should ``read_file`` to recover the full content + (defaults to ``filename`` when not supplied). ``context_length`` lets the + cap scale to the model's window when no explicit config override is set. + """ + if max_chars is None: + max_chars = _get_context_file_max_chars(context_length) if len(content) <= max_chars: return content + target = read_path or filename + msg = ( + f"⚠️ Context file {filename} TRUNCATED: " + f"{len(content)} chars exceeds limit of {max_chars} — " + f"trim the file, pin a larger context_file_max_chars, or use a " + f"larger-context model!" + ) + logger.warning(msg) + _record_truncation_warning(msg) head_chars = int(max_chars * CONTEXT_TRUNCATE_HEAD_RATIO) tail_chars = int(max_chars * CONTEXT_TRUNCATE_TAIL_RATIO) head = content[:head_chars] tail = content[-tail_chars:] - marker = f"\n\n[...truncated {filename}: kept {head_chars}+{tail_chars} of {len(content)} chars. Use file tools to read the full file.]\n\n" + marker = ( + f"\n\n[...truncated {filename}: kept {head_chars}+{tail_chars} of " + f"{len(content)} chars. The middle is omitted — if you need the full " + f"instructions, read the complete file with the read_file tool: " + f"{target}]\n\n" + ) return head + marker + tail -def load_soul_md() -> Optional[str]: +def load_soul_md(context_length: Optional[int] = None) -> Optional[str]: """Load SOUL.md from HERMES_HOME and return its content, or None. Used as the agent identity (slot #1 in the system prompt). When this @@ -1496,14 +1641,17 @@ def load_soul_md() -> Optional[str]: if not content: return None content = _scan_context_content(content, "SOUL.md") - content = _truncate_content(content, "SOUL.md") + content = _truncate_content( + content, "SOUL.md", context_length=context_length, + read_path=str(soul_path), + ) return content except Exception as e: logger.debug("Could not read SOUL.md from %s: %s", soul_path, e) return None -def _load_hermes_md(cwd_path: Path) -> str: +def _load_hermes_md(cwd_path: Path, context_length: Optional[int] = None) -> str: """.hermes.md / HERMES.md — walk to git root.""" hermes_md_path = _find_hermes_md(cwd_path) if not hermes_md_path: @@ -1520,13 +1668,16 @@ def _load_hermes_md(cwd_path: Path) -> str: pass content = _scan_context_content(content, rel) result = f"## {rel}\n\n{content}" - return _truncate_content(result, ".hermes.md") + return _truncate_content( + result, ".hermes.md", context_length=context_length, + read_path=str(hermes_md_path), + ) except Exception as e: logger.debug("Could not read %s: %s", hermes_md_path, e) return "" -def _load_agents_md(cwd_path: Path) -> str: +def _load_agents_md(cwd_path: Path, context_length: Optional[int] = None) -> str: """AGENTS.md — top-level only (no recursive walk).""" for name in ["AGENTS.md", "agents.md"]: candidate = cwd_path / name @@ -1536,13 +1687,16 @@ def _load_agents_md(cwd_path: Path) -> str: if content: content = _scan_context_content(content, name) result = f"## {name}\n\n{content}" - return _truncate_content(result, "AGENTS.md") + return _truncate_content( + result, "AGENTS.md", context_length=context_length, + read_path=str(candidate), + ) except Exception as e: logger.debug("Could not read %s: %s", candidate, e) return "" -def _load_claude_md(cwd_path: Path) -> str: +def _load_claude_md(cwd_path: Path, context_length: Optional[int] = None) -> str: """CLAUDE.md / claude.md — cwd only.""" for name in ["CLAUDE.md", "claude.md"]: candidate = cwd_path / name @@ -1552,13 +1706,16 @@ def _load_claude_md(cwd_path: Path) -> str: if content: content = _scan_context_content(content, name) result = f"## {name}\n\n{content}" - return _truncate_content(result, "CLAUDE.md") + return _truncate_content( + result, "CLAUDE.md", context_length=context_length, + read_path=str(candidate), + ) except Exception as e: logger.debug("Could not read %s: %s", candidate, e) return "" -def _load_cursorrules(cwd_path: Path) -> str: +def _load_cursorrules(cwd_path: Path, context_length: Optional[int] = None) -> str: """.cursorrules + .cursor/rules/*.mdc — cwd only.""" cursorrules_content = "" cursorrules_file = cwd_path / ".cursorrules" @@ -1585,10 +1742,17 @@ def _load_cursorrules(cwd_path: Path) -> str: if not cursorrules_content: return "" - return _truncate_content(cursorrules_content, ".cursorrules") + return _truncate_content( + cursorrules_content, ".cursorrules", context_length=context_length, + read_path=str(cwd_path / ".cursorrules"), + ) -def build_context_files_prompt(cwd: Optional[str] = None, skip_soul: bool = False) -> str: +def build_context_files_prompt( + cwd: Optional[str] = None, + skip_soul: bool = False, + context_length: Optional[int] = None, +) -> str: """Discover and load context files for the system prompt. Priority (first found wins — only ONE project context type is loaded): @@ -1598,7 +1762,11 @@ def build_context_files_prompt(cwd: Optional[str] = None, skip_soul: bool = Fals 4. .cursorrules / .cursor/rules/*.mdc (cwd only) SOUL.md from HERMES_HOME is independent and always included when present. - Each context source is capped at 20,000 chars. + + Each context source is capped before injection. The cap defaults to the + model's context window (scaled — see ``_dynamic_context_file_max_chars``) + when *context_length* is provided, falling back to 20,000 chars otherwise. + An explicit ``context_file_max_chars`` in config.yaml always wins. When *skip_soul* is True, SOUL.md is not included here (it was already loaded via ``load_soul_md()`` for the identity slot). @@ -1611,17 +1779,17 @@ def build_context_files_prompt(cwd: Optional[str] = None, skip_soul: bool = Fals # Priority-based project context: first match wins project_context = ( - _load_hermes_md(cwd_path) - or _load_agents_md(cwd_path) - or _load_claude_md(cwd_path) - or _load_cursorrules(cwd_path) + _load_hermes_md(cwd_path, context_length) + or _load_agents_md(cwd_path, context_length) + or _load_claude_md(cwd_path, context_length) + or _load_cursorrules(cwd_path, context_length) ) if project_context: sections.append(project_context) # SOUL.md from HERMES_HOME only — skip when already loaded as identity if not skip_soul: - soul_content = load_soul_md() + soul_content = load_soul_md(context_length) if soul_content: sections.append(soul_content) diff --git a/agent/skill_commands.py b/agent/skill_commands.py index 269c2fdd25..18264c44bd 100644 --- a/agent/skill_commands.py +++ b/agent/skill_commands.py @@ -26,6 +26,91 @@ _skill_commands_platform: Optional[str] = None _SKILL_INVALID_CHARS = re.compile(r"[^a-z0-9-]") _SKILL_MULTI_HYPHEN = re.compile(r"-{2,}") +# --------------------------------------------------------------------------- +# Skill-scaffolding markers and the canonical extractor. +# +# When a user invokes a /skill (or /bundle), Hermes expands the turn into a +# model-facing message that embeds the full skill body plus scaffolding. That +# expanded text is what flows into the agent loop — and into memory providers +# via MemoryManager. Providers that store or embed the raw user turn (mem0, +# openviking, hindsight, retaindb, byterover, honcho, supermemory) would +# otherwise capture the entire skill body instead of what the user actually +# asked. ``extract_user_instruction_from_skill_message`` recovers just the +# user's instruction so memory stays clean. +# +# These markers MUST stay byte-identical to the builders below +# (``_build_skill_message`` here, ``build_bundle_invocation_message`` in +# agent/skill_bundles.py). They are co-located with the single-skill builder +# on purpose, and the bundle markers are asserted against the bundle builder in +# tests/openviking_plugin/test_openviking.py::test_skill_markers_match_hermes_scaffolding. +# --------------------------------------------------------------------------- +_SKILL_INVOCATION_PREFIX = "[IMPORTANT: The user has invoked the " +_SINGLE_SKILL_MARKER = "The full skill content is loaded below.]" +_SINGLE_SKILL_INSTRUCTION = ( + "The user has provided the following instruction alongside the skill invocation: " +) +_RUNTIME_NOTE = "\n\n[Runtime note:" +_BUNDLE_MARKER = " skill bundle," +_BUNDLE_USER_INSTRUCTION = "\nUser instruction: " +_BUNDLE_FIRST_SKILL_BLOCK = "\n\n[Loaded as part of the " + + +def extract_user_instruction_from_skill_message(content: Any) -> Optional[str]: + """Recover the user's instruction from a slash-skill-expanded turn. + + Returns: + - The original string unchanged when it is NOT skill scaffolding + (a normal user message passes straight through). + - The extracted user instruction when the scaffolding carried one. + - ``None`` when the content is skill scaffolding with no user + instruction (i.e. a bare ``/skill`` invocation). Callers that feed + memory providers should skip the turn in that case — there is no + user content worth storing. + """ + if not isinstance(content, str): + return None + + if not content.startswith(_SKILL_INVOCATION_PREFIX): + return content + + if _BUNDLE_MARKER in content: + return _extract_bundle_user_instruction(content) + + if _SINGLE_SKILL_MARKER in content: + return _extract_single_skill_user_instruction(content) + + return None + + +def _extract_single_skill_user_instruction(message: str) -> Optional[str]: + # Single-skill format appends the user instruction after the skill body, so + # the last occurrence is the user-provided one; the body may quote this text. + marker_idx = message.rfind(_SINGLE_SKILL_INSTRUCTION) + if marker_idx < 0: + return None + + instruction = message[marker_idx + len(_SINGLE_SKILL_INSTRUCTION):] + runtime_idx = instruction.find(_RUNTIME_NOTE) + if runtime_idx >= 0: + instruction = instruction[:runtime_idx] + instruction = instruction.strip() + return instruction or None + + +def _extract_bundle_user_instruction(message: str) -> Optional[str]: + # Bundle format puts the user instruction before the loaded skills, so the + # first occurrence is the user-provided one. + marker_idx = message.find(_BUNDLE_USER_INSTRUCTION) + if marker_idx < 0: + return None + + instruction = message[marker_idx + len(_BUNDLE_USER_INSTRUCTION):] + first_skill_idx = instruction.find(_BUNDLE_FIRST_SKILL_BLOCK) + if first_skill_idx >= 0: + instruction = instruction[:first_skill_idx] + instruction = instruction.strip() + return instruction or None + def _resolve_skill_commands_platform() -> Optional[str]: """Return the current platform scope used for disabled-skill filtering. diff --git a/agent/skill_utils.py b/agent/skill_utils.py index 6f68d3041b..9f16534a45 100644 --- a/agent/skill_utils.py +++ b/agent/skill_utils.py @@ -43,14 +43,20 @@ EXCLUDED_SKILL_DIRS = frozenset( ) ) +# Supporting files live inside a skill package and are loaded explicitly via +# skill_view(skill, file_path=...). They are not standalone skills and must not +# be scanned for active SKILL.md/DESCRIPTION.md entries, even if a Curator or +# archive workflow preserves a complete old skill package under references/. +SKILL_SUPPORT_DIRS = frozenset(("references", "templates", "assets", "scripts")) + def is_excluded_skill_path(path) -> bool: - """True if any component of *path* is in EXCLUDED_SKILL_DIRS. + """True if *path* should be skipped by active skill scanners. - Use this on every SKILL.md path produced by ``rglob`` to prune - dependency, virtualenv, VCS, and cache directories. Centralising the - check here keeps every skill-scanning site in sync with the shared - exclusion set. + Use this on every ``SKILL.md`` path produced by direct ``rglob`` scans to + prune dependency, virtualenv, VCS, cache, and progressive-disclosure + support-package paths. Centralising the check here keeps every + skill-scanning site in sync with the shared exclusion set. Accepts a Path or string. """ @@ -59,7 +65,36 @@ def is_excluded_skill_path(path) -> bool: except AttributeError: from pathlib import PurePath parts = PurePath(str(path)).parts - return any(part in EXCLUDED_SKILL_DIRS for part in parts) + return any(part in EXCLUDED_SKILL_DIRS for part in parts) or is_skill_support_path( + path + ) + + +def is_skill_support_path(path) -> bool: + """True if *path* is under a support dir of an actual skill root. + + ``references/``, ``templates/``, ``assets/``, and ``scripts/`` are + progressive-disclosure support areas when they sit directly inside a skill + directory containing ``SKILL.md``. They are not active discovery roots for + standalone skills. A preserved package such as + ``some-skill/references/old-skill-package/SKILL.md`` is documentation data + unless the caller explicitly loads it via ``file_path``. + + Legitimate categories or skill names such as ``skills/scripts/foo`` remain + discoverable because their ``scripts`` component is not directly under a + directory that contains ``SKILL.md``. + """ + path_obj = path if isinstance(path, Path) else Path(str(path)) + parts = path_obj.parts + # Last component may be a file or candidate skill directory name. Only + # components before the leaf can be containing support directories. + for idx, part in enumerate(parts[:-1]): + if part not in SKILL_SUPPORT_DIRS or idx == 0: + continue + skill_root = Path(*parts[:idx]) + if (skill_root / "SKILL.md").exists(): + return True + return False # ── Lazy YAML loader ───────────────────────────────────────────────────── @@ -661,12 +696,21 @@ def extract_skill_description(frontmatter: Dict[str, Any]) -> str: def iter_skill_index_files(skills_dir: Path, filename: str): """Walk skills_dir yielding sorted paths matching *filename*. - Excludes Hermes metadata, VCS, virtualenv/dependency, and cache - directories so dependencies cannot register nested skills. + Excludes Hermes metadata, VCS, virtualenv/dependency, cache, and skill + support directories. Support directories (references/templates/assets/ + scripts) can contain arbitrary markdown and even archived package + ``SKILL.md`` files, but they are progressive-disclosure data loaded through + ``skill_view(..., file_path=...)`` rather than active skill roots. """ matches = [] for root, dirs, files in os.walk(skills_dir, followlinks=True): - dirs[:] = [d for d in dirs if d not in EXCLUDED_SKILL_DIRS] + has_skill_md = "SKILL.md" in files + dirs[:] = [ + d + for d in dirs + if d not in EXCLUDED_SKILL_DIRS + and not (has_skill_md and d in SKILL_SUPPORT_DIRS) + ] if filename in files: matches.append(Path(root) / filename) for path in sorted(matches, key=lambda p: str(p.relative_to(skills_dir))): diff --git a/agent/system_prompt.py b/agent/system_prompt.py index 76f57dfcdb..281f01399b 100644 --- a/agent/system_prompt.py +++ b/agent/system_prompt.py @@ -33,6 +33,7 @@ from agent.prompt_builder import ( KANBAN_GUIDANCE, MEMORY_GUIDANCE, OPENAI_MODEL_EXECUTION_GUIDANCE, + PARALLEL_TOOL_CALL_GUIDANCE, PLATFORM_HINTS, SESSION_SEARCH_GUIDANCE, SKILLS_GUIDANCE, @@ -40,6 +41,7 @@ from agent.prompt_builder import ( TASK_COMPLETION_GUIDANCE, TOOL_USE_ENFORCEMENT_GUIDANCE, TOOL_USE_ENFORCEMENT_MODELS, + drain_truncation_warnings, ) from agent.runtime_cwd import resolve_context_cwd @@ -82,6 +84,17 @@ def build_system_prompt_parts(agent: Any, system_message: Optional[str] = None) # we resolve through ``_ra()`` to honor those patches. _r = _ra() + # Resolve the model's context window once so context-file caps can scale + # to it (dynamic cap — see prompt_builder._dynamic_context_file_max_chars). + # None falls back to the historical flat default. This value is stable for + # the life of the conversation, so it does not threaten prompt caching. + _ctx_len: Optional[int] = None + _cc = getattr(agent, "context_compressor", None) + if _cc is not None: + _cc_len = getattr(_cc, "context_length", None) + if isinstance(_cc_len, int) and _cc_len > 0: + _ctx_len = _cc_len + # ── Stable tier ──────────────────────────────────────────────── stable_parts: List[str] = [] @@ -90,7 +103,7 @@ def build_system_prompt_parts(agent: Any, system_message: Optional[str] = None) # cwd project instructions disabled. _soul_loaded = False if agent.load_soul_identity or not agent.skip_context_files: - _soul_content = _r.load_soul_md() + _soul_content = _r.load_soul_md(_ctx_len) if _soul_content: stable_parts.append(_soul_content) _soul_loaded = True @@ -111,6 +124,17 @@ def build_system_prompt_parts(agent: Any, system_message: Optional[str] = None) if getattr(agent, "_task_completion_guidance", True) and agent.valid_tool_names: stable_parts.append(TASK_COMPLETION_GUIDANCE) + # Universal parallel-tool-call guidance. Tells the model to batch + # independent tool calls into one assistant turn rather than emitting one + # call per turn — the runtime already runs independent calls concurrently + # (read-only tools always; non-overlapping path-scoped file ops), so the + # only thing missing was steering the model to produce the batch. Cuts + # round-trips and the resent-context cost that compounds over a long + # conversation. Gated by config.yaml ``agent.parallel_tool_call_guidance`` + # (default True) and only injected when tools are actually loaded. + if getattr(agent, "_parallel_tool_call_guidance", True) and agent.valid_tool_names: + stable_parts.append(PARALLEL_TOOL_CALL_GUIDANCE) + # Tool-aware behavioral guidance: only inject when the tools are loaded tool_guidance = [] if "memory" in agent.valid_tool_names: @@ -333,7 +357,8 @@ def build_system_prompt_parts(agent: Any, system_message: Optional[str] = None) # dir — the user's real cwd there, but the install dir for the gateway # daemon, which is why the gateway sets TERMINAL_CWD. context_files_prompt = _r.build_context_files_prompt( - cwd=resolve_context_cwd(), skip_soul=_soul_loaded) + cwd=resolve_context_cwd(), skip_soul=_soul_loaded, + context_length=_ctx_len) if context_files_prompt: context_parts.append(context_files_prompt) @@ -400,7 +425,14 @@ def build_system_prompt(agent: Any, system_message: Optional[str] = None) -> str warm across turns. """ parts = build_system_prompt_parts(agent, system_message=system_message) - return "\n\n".join(p for p in (parts["stable"], parts["context"], parts["volatile"]) if p) + joined = "\n\n".join(p for p in (parts["stable"], parts["context"], parts["volatile"]) if p) + + # Surface context-file truncation warnings through the normal agent status + # channel so gateway/CLI users see them in chat instead of only in logs. + for warning in drain_truncation_warnings(): + agent._emit_status(warning) + + return joined def invalidate_system_prompt(agent: Any) -> None: diff --git a/agent/tool_executor.py b/agent/tool_executor.py index 144a292978..e7ba79db8b 100644 --- a/agent/tool_executor.py +++ b/agent/tool_executor.py @@ -1012,28 +1012,42 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe elif function_name == "memory": def _execute(next_args: dict) -> Any: target = next_args.get("target", "memory") + operations = next_args.get("operations") from tools.memory_tool import memory_tool as _memory_tool result = _memory_tool( action=next_args.get("action"), target=target, content=next_args.get("content"), old_text=next_args.get("old_text"), + operations=operations, store=agent._memory_store, ) - # Bridge: notify external memory provider of built-in memory writes - if agent._memory_manager and next_args.get("action") in {"add", "replace"}: - try: - agent._memory_manager.on_memory_write( - next_args.get("action", ""), - target, - next_args.get("content", ""), - metadata=agent._build_memory_write_metadata( - task_id=effective_task_id, - tool_call_id=getattr(tool_call, "id", None), - ), + # Bridge: notify external memory provider of built-in memory writes. + # Covers both the single-op shape and each add/replace inside a batch. + if agent._memory_manager: + if operations: + _mem_ops = [ + op for op in operations + if isinstance(op, dict) and op.get("action") in {"add", "replace"} + ] + else: + _mem_ops = ( + [{"action": next_args.get("action"), "content": next_args.get("content")}] + if next_args.get("action") in {"add", "replace"} else [] ) - except Exception: - pass + for _op in _mem_ops: + try: + agent._memory_manager.on_memory_write( + _op.get("action", ""), + target, + _op.get("content", "") or "", + metadata=agent._build_memory_write_metadata( + task_id=effective_task_id, + tool_call_id=getattr(tool_call, "id", None), + ), + ) + except Exception: + pass return result function_result, function_args = _run_agent_tool_execution_middleware( agent, diff --git a/agent/transports/anthropic.py b/agent/transports/anthropic.py index aad4916138..98721f7c5e 100644 --- a/agent/transports/anthropic.py +++ b/agent/transports/anthropic.py @@ -88,7 +88,7 @@ class AnthropicTransport(ProviderTransport): from agent.transports.types import ToolCall strip_tool_prefix = kwargs.get("strip_tool_prefix", False) - _MCP_PREFIX = "mcp_" + _MCP_PREFIX = "mcp__" text_parts = [] reasoning_parts = [] @@ -132,17 +132,25 @@ class AnthropicTransport(ProviderTransport): elif block.type == "tool_use": name = block.name if strip_tool_prefix and name.startswith(_MCP_PREFIX): - stripped = name[len(_MCP_PREFIX):] - # Only strip the mcp_ prefix for OAuth-injected tools - # (where Hermes adds the prefix when sending to Anthropic - # and must remove it on the way back). Native MCP server - # tools (from mcp_servers: in config.yaml) are registered - # in the tool registry under their FULL mcp__ - # name and must NOT be stripped. GH-25255. + # On the OAuth wire every tool carries a double-underscore + # ``mcp__`` prefix (added in build_anthropic_kwargs to avoid + # Anthropic's single-underscore third-party classifier). + # Reverse it back to the name the registry/dispatcher knows. + # Two original forms map onto the same ``mcp__`` wire name: + # ``mcp__read_file`` <- bare native tool ``read_file`` + # ``mcp__linear_get_issue`` <- MCP server tool + # ``mcp_linear_get_issue`` + # Resolve by registry lookup, preferring whichever original + # is actually registered; never rewrite a name the LLM used + # that already resolves natively. GH-25255. from tools.registry import registry as _tool_registry - if (_tool_registry.get_entry(stripped) - and not _tool_registry.get_entry(name)): - name = stripped + if not _tool_registry.get_entry(name): + bare = name[len(_MCP_PREFIX):] # read_file + single = "mcp_" + bare # mcp_read_file / mcp_linear_get_issue + if _tool_registry.get_entry(single): + name = single + elif _tool_registry.get_entry(bare): + name = bare tool_calls.append( ToolCall( id=block.id, diff --git a/agent/transports/codex.py b/agent/transports/codex.py index 1d24ac3355..1ce449eeaa 100644 --- a/agent/transports/codex.py +++ b/agent/transports/codex.py @@ -128,6 +128,65 @@ class ResponsesApiTransport(ProviderTransport): reasoning_effort = _effort_clamp.get(reasoning_effort, reasoning_effort) response_tools = _responses_tools(tools) + + # xAI server-side web search. + # + # grok models on xAI's /v1/responses surface (notably + # grok-composer-2.5-fast on SuperGrok OAuth) have a *native*, + # server-executed web search. When the model is handed a + # client-side function literally named ``web_search``, it routes + # the intent to that native engine — but because the tool is + # declared as a plain ``function`` rather than xAI's first-class + # ``{"type": "web_search"}`` built-in, the server-side search is + # dispatched but never reconciled: the response streams reasoning + # + ``web_search_call`` progress items, the searches never reach + # ``status="completed"`` in the assembled output, no final + # message is emitted, and ``_normalize_codex_response`` correctly + # sees reasoning-with-no-answer and reports ``incomplete``. The + # turn then burns 3 continuation retries and fails with "Codex + # response remained incomplete after 3 continuation attempts". + # Verified live against grok-composer-2.5-fast (2026-06). + # + # Fix: when the agent HAS a client-side ``web_search`` function (i.e. + # the user enabled the web toolset), declare xAI's native + # ``web_search`` built-in instead so the search actually runs to + # completion server-side and the model streams a real answer. The + # Responses API rejects two tools sharing the name ``web_search`` + # (HTTP 400 "Duplicate tool names"), so we drop the client-side + # ``web_search`` function for the xAI path and let the native tool + # satisfy it. All other client-side tools (read_file, terminal, + # web_extract, MCP tools, …) are untouched and continue to dispatch + # through Hermes's agent loop. + # + # Scope: we ONLY swap in the native built-in when the client + # ``web_search`` was actually present. We do NOT force-enable Grok + # server-side search on turns where the user never had web enabled — + # that would silently route around Hermes's web-provider config and + # tool-trace/citation plumbing for every xai-oauth turn. The swap is + # a 1:1 replacement of an already-requested capability, not an + # additive grant. + # + # NOTE: for the swapped case this routes ``web_search`` to Grok's + # native search engine for xAI sessions instead of Hermes's + # configured web provider (Tavily/etc.), and those results bypass + # Hermes's tool-trace / citation plumbing (they arrive baked into the + # model's answer rather than as a tool result the loop observes). + # Scoped to ``is_xai_responses`` deliberately; narrow to specific + # models if a future grok variant should keep the client-side + # function. + if is_xai_responses and response_tools: + has_client_web_search = any( + isinstance(t, dict) and t.get("name") == "web_search" + for t in response_tools + ) + if has_client_web_search: + filtered = [ + t for t in response_tools + if not (isinstance(t, dict) and t.get("name") == "web_search") + ] + filtered.append({"type": "web_search"}) + response_tools = filtered + # ``tools`` MUST be omitted entirely when there are no functions to # expose: the openai SDK's ``responses.stream()`` / ``responses.parse()`` # eagerly call ``_make_tools(tools)`` which does ``for tool in tools`` @@ -218,10 +277,28 @@ class ResponsesApiTransport(ProviderTransport): kwargs.pop("timeout", None) if is_codex_backend: - # chatgpt.com/backend-api/codex rejects body-level - # ``extra_headers`` with HTTP 400. Correlation/cache routing for - # this backend must not be sent through the Responses payload. - kwargs.pop("extra_headers", None) + # The Codex backend rejects body-level ``extra_headers`` with + # HTTP 400, but the OpenAI SDK's ``extra_headers`` kwarg maps + # to actual HTTP request headers (not body fields). We need + # these headers for cache-scope routing so prompt cache hits + # remain high. Send session_id / x-client-request-id as HTTP + # headers while keeping ``prompt_cache_key`` in the body for + # standard OpenAI routing as a belt-and-braces fallback. + cache_scope_id = str(session_id or "").strip() + if cache_scope_id: + existing_extra_headers = kwargs.get("extra_headers") + merged_extra_headers: Dict[str, str] = {} + if isinstance(existing_extra_headers, dict): + merged_extra_headers.update( + { + str(key): str(value) + for key, value in existing_extra_headers.items() + if key and value is not None + } + ) + merged_extra_headers["session_id"] = cache_scope_id + merged_extra_headers["x-client-request-id"] = cache_scope_id + kwargs["extra_headers"] = merged_extra_headers max_tokens = params.get("max_tokens") if max_tokens is not None and not is_codex_backend: diff --git a/agent/turn_context.py b/agent/turn_context.py index e94d43279a..8041eabdb7 100644 --- a/agent/turn_context.py +++ b/agent/turn_context.py @@ -69,6 +69,7 @@ def build_turn_context( task_id: Optional[str], stream_callback, persist_user_message: Optional[str], + persist_user_timestamp: Optional[float] = None, *, restore_or_build_system_prompt, install_safe_stdio, @@ -121,6 +122,7 @@ def build_turn_context( agent._stream_callback = stream_callback agent._persist_user_message_idx = None agent._persist_user_message_override = persist_user_message + agent._persist_user_message_timestamp = persist_user_timestamp # Generate unique task_id if not provided to isolate VMs between tasks. effective_task_id = task_id or str(uuid.uuid4()) agent._current_task_id = effective_task_id diff --git a/apps/bootstrap-installer/src-tauri/src/update.rs b/apps/bootstrap-installer/src-tauri/src/update.rs index 40d136f960..a42838293a 100644 --- a/apps/bootstrap-installer/src-tauri/src/update.rs +++ b/apps/bootstrap-installer/src-tauri/src/update.rs @@ -286,7 +286,7 @@ async fn run_update(app: AppHandle) -> Result<()> { emit_stage(&app, "rebuild", StageState::Running, None, None); let started = Instant::now(); let rebuild_args: Vec = vec!["desktop".into(), "--build-only".into()]; - let rebuild = run_streamed( + let mut rebuild = run_streamed( &app, &hermes, &rebuild_args, @@ -295,6 +295,33 @@ async fn run_update(app: AppHandle) -> Result<()> { Some("rebuild"), ) .await?; + + // Retry-once: the first `--build-only` can return nonzero on a still-settling + // post-update tree or a network-blocked Electron fetch that our self-heal + // repaired mid-run. A second attempt then builds clean off the healed dist + // (the content-hash stamp makes it a near-no-op when the first actually + // succeeded). Without this the updater bails here and never reaches the + // relaunch below — the app updates but doesn't restart. Matches the + // retry-once `hermes update` already does above, and `hermes update`'s own + // desktop rebuild in cmd_update. + if rebuild_needs_retry(rebuild.exit_code) { + emit_log( + &app, + Some("rebuild"), + LogStream::Stdout, + "[rebuild] first desktop rebuild failed; retrying once (a self-healed \ + Electron download builds clean on the second run)…", + ); + rebuild = run_streamed( + &app, + &hermes, + &rebuild_args, + &install_root, + &child_env, + Some("rebuild"), + ) + .await?; + } let rebuild_ms = started.elapsed().as_millis() as u64; if rebuild.exit_code != Some(0) { @@ -533,6 +560,14 @@ fn is_locked(path: &Path) -> bool { } } +/// Whether the `desktop --build-only` rebuild should be retried once. Any +/// non-success exit qualifies: the common cause is a transient first-attempt +/// failure (still-settling tree / self-healed Electron download) that a clean +/// second run resolves. +fn rebuild_needs_retry(exit_code: Option) -> bool { + exit_code != Some(0) +} + /// Spawn `hermes ` from `cwd`, stream stdout/stderr as Log events on the /// bootstrap channel, and return the exit code. Mirrors powershell::run_script /// but for an arbitrary command (no install.ps1 -File wrapping). @@ -970,6 +1005,16 @@ mod tests { assert_eq!(update_branch_from_args(["--update"]), None); } + #[test] + fn rebuild_retries_only_on_failure() { + assert!(!rebuild_needs_retry(Some(0)), "a clean rebuild must not retry"); + assert!(rebuild_needs_retry(Some(1)), "a failed rebuild retries once"); + assert!( + rebuild_needs_retry(None), + "a killed/signalled rebuild (no exit code) retries once" + ); + } + #[test] fn parses_only_app_targets() { assert_eq!( diff --git a/apps/desktop/electron/main.cjs b/apps/desktop/electron/main.cjs index 19096c6135..be89c6c91c 100644 --- a/apps/desktop/electron/main.cjs +++ b/apps/desktop/electron/main.cjs @@ -28,6 +28,7 @@ const { detectRemoteDisplay, isWindowsBinaryPathInWsl, isWslEnvironment } = requ const { runBootstrap } = require('./bootstrap-runner.cjs') const { buildSessionWindowUrl, + chatWindowWebPreferences, createSessionWindowRegistry, SESSION_WINDOW_MIN_HEIGHT, SESSION_WINDOW_MIN_WIDTH @@ -44,6 +45,7 @@ const { readDirForIpc } = require('./fs-read-dir.cjs') const { gitRootForIpc } = require('./git-root.cjs') const { worktreesForIpc } = require('./git-worktrees.cjs') const { OFFICIAL_REPO_HTTPS_URL, isOfficialSshRemote } = require('./update-remote.cjs') +const { runRebuildWithRetry } = require('./update-rebuild.cjs') const { buildPosixCleanupScript, buildWindowsCleanupScript, @@ -2008,10 +2010,14 @@ async function applyUpdatesPosixInApp() { } emitUpdateProgress({ stage: 'rebuild', message: 'Rebuilding the desktop app…', percent: 60 }) - const rebuilt = await runStreamedUpdate(hermes, ['desktop', '--build-only'], { - cwd: updateRoot, - env, - stage: 'rebuild' + // Retry-once: a first rebuild can fail on a still-settling tree or a + // self-healed (network-blocked) Electron download; a second run builds clean + // off the healed dist so we reach the swap+relaunch below instead of bailing. + const rebuilt = await runRebuildWithRetry(attempt => { + if (attempt > 0) { + emitUpdateProgress({ stage: 'rebuild', message: 'Retrying the desktop rebuild…', percent: 60 }) + } + return runStreamedUpdate(hermes, ['desktop', '--build-only'], { cwd: updateRoot, env, stage: 'rebuild' }) }) if (rebuilt.code !== 0) { emitUpdateProgress({ @@ -5106,14 +5112,7 @@ function spawnSecondaryWindow({ sessionId, watch, newSession } = {}) { // themes/context.tsx, so the window appears already themed. show: false, backgroundColor: getWindowBackgroundColor(), - webPreferences: { - preload: path.join(__dirname, 'preload.cjs'), - contextIsolation: true, - webviewTag: true, - sandbox: true, - nodeIntegration: false, - devTools: true - } + webPreferences: chatWindowWebPreferences(path.join(__dirname, 'preload.cjs')) }) if (IS_MAC) { @@ -5180,23 +5179,11 @@ function createWindow() { // material before the renderer paints the app theme. See createSessionWindow. show: false, backgroundColor: getWindowBackgroundColor(), - webPreferences: { - preload: path.join(__dirname, 'preload.cjs'), - contextIsolation: true, - webviewTag: true, - sandbox: true, - nodeIntegration: false, - devTools: true, - // Keep timers + requestAnimationFrame running at full speed when the - // window is blurred/occluded. The chat transcript streams to the screen - // through a requestAnimationFrame-gated flush (useSessionStateCache), - // so with Chromium's default background throttling the live answer - // stalls whenever this window isn't focused (e.g. you switch to your - // editor mid-turn, or open detached devtools) and only appears once you - // refocus or refresh. A streaming chat app must render in the - // background, so opt out — matching the secondary windows above. - backgroundThrottling: false - } + // Shared with the secondary session windows (chatWindowWebPreferences) so + // both keep `backgroundThrottling: false` — the chat transcript streams via + // a requestAnimationFrame-gated flush that Chromium pauses for blurred + // windows, stalling the live answer until refocus. See session-windows.cjs. + webPreferences: chatWindowWebPreferences(path.join(__dirname, 'preload.cjs')) }) if (IS_MAC) { @@ -6564,6 +6551,12 @@ app.on('before-quit', () => { flushDesktopLogBufferSync() closePreviewWatchers() + // Kill open PTYs before environment teardown to avoid the node-pty#904 + // ThreadSafeFunction SIGABRT race. + for (const id of [...terminalSessions.keys()]) { + disposeTerminalSession(id) + } + if (hermesProcess && !hermesProcess.killed) { hermesProcess.kill('SIGTERM') } diff --git a/apps/desktop/electron/session-windows.cjs b/apps/desktop/electron/session-windows.cjs index 929bf3ea9a..5e2f3d4c68 100644 --- a/apps/desktop/electron/session-windows.cjs +++ b/apps/desktop/electron/session-windows.cjs @@ -10,6 +10,29 @@ const { pathToFileURL } = require('node:url') const SESSION_WINDOW_MIN_WIDTH = 420 const SESSION_WINDOW_MIN_HEIGHT = 620 +// Shared webPreferences for every window that renders the chat transcript — the +// primary window AND the secondary session windows. Keeping it in one place is +// the whole point: the two BrowserWindow definitions in main.cjs used to be +// hand-copied, and the secondary windows silently lost `backgroundThrottling: +// false`, so a streamed answer stalled until the window regained focus. +// +// `backgroundThrottling: false` is load-bearing: the transcript streams to the +// screen through a requestAnimationFrame-gated flush, which Chromium pauses for +// blurred/occluded windows. A streaming chat app must keep painting in the +// background, so every chat window opts out. The preload path is injected +// because it depends on the Electron entry's __dirname. +function chatWindowWebPreferences(preloadPath) { + return { + preload: preloadPath, + contextIsolation: true, + webviewTag: true, + sandbox: true, + nodeIntegration: false, + devTools: true, + backgroundThrottling: false + } +} + // Build the renderer URL for a secondary window. The renderer uses a // HashRouter, so the session route lives after the '#'. The `?win=secondary` // flag MUST sit in the query string BEFORE the '#': anything after the '#' is @@ -94,6 +117,7 @@ function createSessionWindowRegistry() { module.exports = { buildSessionWindowUrl, + chatWindowWebPreferences, createSessionWindowRegistry, SESSION_WINDOW_MIN_HEIGHT, SESSION_WINDOW_MIN_WIDTH diff --git a/apps/desktop/electron/session-windows.test.cjs b/apps/desktop/electron/session-windows.test.cjs index 8261809dbf..78f19b859e 100644 --- a/apps/desktop/electron/session-windows.test.cjs +++ b/apps/desktop/electron/session-windows.test.cjs @@ -1,7 +1,11 @@ const assert = require('node:assert/strict') const test = require('node:test') -const { buildSessionWindowUrl, createSessionWindowRegistry } = require('./session-windows.cjs') +const { + buildSessionWindowUrl, + chatWindowWebPreferences, + createSessionWindowRegistry +} = require('./session-windows.cjs') // A minimal fake BrowserWindow: tracks listeners + destroyed state and lets a // test fire the 'closed' event, mirroring the slice of the Electron API the @@ -175,3 +179,21 @@ test('registry trims the session id before keying', () => { assert.equal(registry.has('s1'), true) }) + +test('chatWindowWebPreferences disables background throttling so streaming paints while blurred', () => { + // Regression: secondary session windows used to omit this flag, so a streamed + // answer stalled until the window regained focus (Chromium pauses the + // requestAnimationFrame-gated transcript flush for backgrounded windows). + const prefs = chatWindowWebPreferences('/tmp/preload.cjs') + + assert.equal(prefs.backgroundThrottling, false) +}) + +test('chatWindowWebPreferences passes the preload path through and keeps the hardened defaults', () => { + const prefs = chatWindowWebPreferences('/some/preload.cjs') + + assert.equal(prefs.preload, '/some/preload.cjs') + assert.equal(prefs.contextIsolation, true) + assert.equal(prefs.sandbox, true) + assert.equal(prefs.nodeIntegration, false) +}) diff --git a/apps/desktop/electron/update-rebuild.cjs b/apps/desktop/electron/update-rebuild.cjs new file mode 100644 index 0000000000..ec8a948316 --- /dev/null +++ b/apps/desktop/electron/update-rebuild.cjs @@ -0,0 +1,29 @@ +'use strict' + +/** + * Retry-once policy for the desktop `--build-only` rebuild during self-update. + * + * The first rebuild can return nonzero on a still-settling post-update tree or a + * network-blocked Electron fetch that the installer's self-heal repaired mid-run. + * A second attempt then builds clean off the healed dist (the content-hash stamp + * makes it a near-no-op when the first actually succeeded). Without the retry the + * updater bails before the relaunch step — the app updates but doesn't restart. + */ + +function shouldRetryRebuild(code) { + return code !== 0 +} + +/** + * Run `rebuild()` (async, resolves `{ code, ... }`), retrying once on failure. + * Returns the final result. + */ +async function runRebuildWithRetry(rebuild) { + let result = await rebuild(0) + if (shouldRetryRebuild(result.code)) { + result = await rebuild(1) + } + return result +} + +module.exports = { shouldRetryRebuild, runRebuildWithRetry } diff --git a/apps/desktop/electron/update-rebuild.test.cjs b/apps/desktop/electron/update-rebuild.test.cjs new file mode 100644 index 0000000000..623effa4d1 --- /dev/null +++ b/apps/desktop/electron/update-rebuild.test.cjs @@ -0,0 +1,55 @@ +/** + * Tests for electron/update-rebuild.cjs — the retry-once policy for the desktop + * `--build-only` rebuild during self-update. + * + * Run with: node --test electron/update-rebuild.test.cjs + * (Wired into npm test:desktop:platforms in package.json.) + * + * Why this matters: a first rebuild can return nonzero on a still-settling tree + * or a self-healed (network-blocked) Electron download. Without a second attempt + * the updater bails before the relaunch step — the app updates but never restarts + * (the field report behind this fix). The retry must fire on failure, not on + * success, and must run at most twice. + */ + +const test = require('node:test') +const assert = require('node:assert/strict') + +const { shouldRetryRebuild, runRebuildWithRetry } = require('./update-rebuild.cjs') + +test('shouldRetryRebuild retries only on a non-success exit', () => { + assert.equal(shouldRetryRebuild(0), false) + assert.equal(shouldRetryRebuild(1), true) + assert.equal(shouldRetryRebuild(null), true) +}) + +test('a clean first rebuild runs once and does not retry', async () => { + const codes = [] + const result = await runRebuildWithRetry(attempt => { + codes.push(attempt) + return Promise.resolve({ code: 0 }) + }) + assert.deepEqual(codes, [0]) + assert.equal(result.code, 0) +}) + +test('a failed first rebuild retries once and succeeds', async () => { + const codes = [] + const result = await runRebuildWithRetry(attempt => { + codes.push(attempt) + return Promise.resolve({ code: attempt === 0 ? 1 : 0 }) + }) + assert.deepEqual(codes, [0, 1]) + assert.equal(result.code, 0) +}) + +test('a rebuild that keeps failing runs at most twice and reports the failure', async () => { + const codes = [] + const result = await runRebuildWithRetry(attempt => { + codes.push(attempt) + return Promise.resolve({ code: 1, error: 'rebuild-failed' }) + }) + assert.deepEqual(codes, [0, 1]) + assert.equal(result.code, 1) + assert.equal(result.error, 'rebuild-failed') +}) diff --git a/apps/desktop/package.json b/apps/desktop/package.json index 08080188a5..70d35fb7bb 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -21,7 +21,7 @@ "build": "node scripts/assert-root-install.cjs && node scripts/write-build-stamp.cjs && node scripts/stage-native-deps.cjs && tsc -b && vite build && npm run postbuild", "postbuild": "node scripts/assert-dist-built.cjs", "prebuilder": "node scripts/patch-electron-builder-mac-binary.cjs", - "builder": "cross-env NODE_OPTIONS=--max-old-space-size=16384 electron-builder", + "builder": "cross-env NODE_OPTIONS=--max-old-space-size=16384 node scripts/run-electron-builder.cjs", "pack": "npm run build && npm run builder -- --dir", "dist": "npm run build && npm run builder", "dist:mac": "npm run build && npm run builder -- --mac", @@ -37,7 +37,7 @@ "test:desktop:nsis": "node scripts/test-desktop.mjs nsis", "test:desktop:existing": "node scripts/test-desktop.mjs existing", "test:desktop:fresh": "node scripts/test-desktop.mjs fresh", - "test:desktop:platforms": "node --test electron/bootstrap-platform.test.cjs electron/hardening.test.cjs electron/backend-env.test.cjs electron/backend-probes.test.cjs electron/bootstrap-runner.test.cjs electron/connection-config.test.cjs electron/dashboard-token.test.cjs electron/gateway-ws-probe.test.cjs electron/oauth-net-request.test.cjs electron/desktop-uninstall.test.cjs electron/session-windows.test.cjs electron/workspace-cwd.test.cjs electron/fs-read-dir.test.cjs electron/git-root.test.cjs electron/windows-child-process.test.cjs electron/update-remote.test.cjs electron/windows-user-env.test.cjs", + "test:desktop:platforms": "node --test electron/bootstrap-platform.test.cjs electron/hardening.test.cjs electron/backend-env.test.cjs electron/backend-probes.test.cjs electron/bootstrap-runner.test.cjs electron/connection-config.test.cjs electron/dashboard-token.test.cjs electron/gateway-ws-probe.test.cjs electron/oauth-net-request.test.cjs electron/desktop-uninstall.test.cjs electron/session-windows.test.cjs electron/workspace-cwd.test.cjs electron/fs-read-dir.test.cjs electron/git-root.test.cjs electron/windows-child-process.test.cjs electron/update-remote.test.cjs electron/update-rebuild.test.cjs electron/windows-user-env.test.cjs", "typecheck": "tsc -p . --noEmit", "lint": "eslint src/ electron/", "lint:fix": "eslint src/ electron/ --fix", @@ -117,7 +117,7 @@ "@vitejs/plugin-react": "^6.0.1", "concurrently": "^10.0.3", "cross-env": "^10.1.0", - "electron": "^40.9.3", + "electron": "40.10.2", "electron-builder": "^26.8.1", "eslint": "^9.39.4", "eslint-plugin-perfectionist": "^5.9.0", @@ -134,8 +134,7 @@ "wait-on": "^9.0.5" }, "build": { - "electronVersion": "40.9.3", - "electronDist": "../../node_modules/electron/dist", + "electronVersion": "40.10.2", "appId": "com.nousresearch.hermes", "productName": "Hermes", "executableName": "Hermes", diff --git a/apps/desktop/scripts/patch-electron-builder-mac-binary.cjs b/apps/desktop/scripts/patch-electron-builder-mac-binary.cjs index 38315b9c65..b88c281219 100644 --- a/apps/desktop/scripts/patch-electron-builder-mac-binary.cjs +++ b/apps/desktop/scripts/patch-electron-builder-mac-binary.cjs @@ -24,6 +24,11 @@ const replacement = ` // ${marker}: electron-builder 26.8.x can sometimes cop if (!fs.existsSync(bundledElectronBinary)) { const candidates = [ path.join(packager.info.framework.distMacOsAppName, "Contents", "MacOS", electronBranding.productName), + // npm may nest the workspace-only electron devDep under + // apps/desktop/node_modules (process.cwd() during pack), or hoist + // it to the repo root. Try the workspace-local install first, then + // the root hoist, so the fallback works under either layout. + path.join(process.cwd(), "node_modules", "electron", "dist", "Electron.app", "Contents", "MacOS", electronBranding.productName), path.join(process.cwd(), "..", "..", "node_modules", "electron", "dist", "Electron.app", "Contents", "MacOS", electronBranding.productName), ]; const sourceBinary = candidates.find(candidate => fs.existsSync(candidate)); diff --git a/apps/desktop/scripts/run-electron-builder.cjs b/apps/desktop/scripts/run-electron-builder.cjs new file mode 100644 index 0000000000..100d6c346e --- /dev/null +++ b/apps/desktop/scripts/run-electron-builder.cjs @@ -0,0 +1,57 @@ +"use strict" + +// Resolve electronDist at runtime (#38673, #47917): electron-builder 26.8.x can +// re-unpack a broken Electron.app; reusing the installed dist dodges that. +// npm workspace hoisting is non-deterministic — require.resolve finds electron +// wherever it landed. Dist present → -c.electronDist=/dist; absent → let +// electron-builder fetch via @electron/get (electronVersion + ELECTRON_MIRROR). + +const fs = require("node:fs") +const path = require("node:path") +const { spawnSync } = require("node:child_process") + +function electronDistDir() { + try { + return path.join(path.dirname(require.resolve("electron/package.json")), "dist") + } catch { + return null + } +} + +function distBinary(dist) { + if (process.platform === "darwin") { + return path.join(dist, "Electron.app", "Contents", "MacOS", "Electron") + } + if (process.platform === "win32") { + return path.join(dist, "electron.exe") + } + return path.join(dist, "electron") +} + +function electronBuilderCli() { + const pkgJson = require.resolve("electron-builder/package.json") + const bin = require(pkgJson).bin + const rel = typeof bin === "string" ? bin : bin["electron-builder"] + return path.join(path.dirname(pkgJson), rel) +} + +const dist = electronDistDir() +const args = [] +if (dist && fs.existsSync(distBinary(dist))) { + args.push(`-c.electronDist=${dist}`) +} else { + console.warn( + "[run-electron-builder] no local electron dist; electron-builder will fetch " + + "via @electron/get (electronVersion + ELECTRON_MIRROR)." + ) +} +args.push(...process.argv.slice(2)) + +const result = spawnSync(process.execPath, [electronBuilderCli(), ...args], { + stdio: "inherit", +}) +if (result.error) { + console.error(`[run-electron-builder] spawn failed: ${result.error.message}`) + process.exit(1) +} +process.exit(result.status == null ? 1 : result.status) diff --git a/apps/desktop/src/app/chat/composer/controls.tsx b/apps/desktop/src/app/chat/composer/controls.tsx index 8bc1a2b7cf..6d748c73b5 100644 --- a/apps/desktop/src/app/chat/composer/controls.tsx +++ b/apps/desktop/src/app/chat/composer/controls.tsx @@ -9,6 +9,7 @@ import { formatCombo } from '@/lib/keybinds/combo' import { cn } from '@/lib/utils' import type { ConversationStatus } from './hooks/use-voice-conversation' +import { ModelPill } from './model-pill' import type { ChatBarState, VoiceStatus } from './types' export const ICON_BTN = 'size-(--composer-control-size) shrink-0 rounded-md' @@ -66,6 +67,7 @@ export function ComposerControls({ const c = t.composer const steerCombo = formatCombo('mod+enter') const steerLabel = `${c.steer} (${steerCombo})` + const steerTip = ( {c.steer} @@ -81,8 +83,10 @@ export function ComposerControls({ return (
- - {canSteer && ( + + {/* While the agent runs and the user is typing, steer takes over the mic's + slot rather than crowding the row with an extra button. */} + {canSteer ? ( + ) : ( + )} {showVoicePrimary ? ( diff --git a/apps/desktop/src/app/chat/composer/model-pill.tsx b/apps/desktop/src/app/chat/composer/model-pill.tsx new file mode 100644 index 0000000000..f04b6e2302 --- /dev/null +++ b/apps/desktop/src/app/chat/composer/model-pill.tsx @@ -0,0 +1,86 @@ +import { useStore } from '@nanostores/react' +import { useState } from 'react' + +import { ModelMenuCloseContext } from '@/app/shell/model-menu-panel' +import { Button } from '@/components/ui/button' +import { DropdownMenu, DropdownMenuContent, DropdownMenuTrigger } from '@/components/ui/dropdown-menu' +import { GlyphSpinner } from '@/components/ui/glyph-spinner' +import { useI18n } from '@/i18n' +import { ChevronDown } from '@/lib/icons' +import { formatModelStatusLabel } from '@/lib/model-status-label' +import { cn } from '@/lib/utils' +import { + $currentFastMode, + $currentModel, + $currentProvider, + $currentReasoningEffort, + setModelPickerOpen +} from '@/store/session' + +import type { ChatBarState } from './types' + +const PILL = cn( + 'h-(--composer-control-size) max-w-40 shrink-0 gap-1 rounded-md px-2 text-xs font-normal', + 'text-(--ui-text-tertiary) hover:bg-(--chrome-action-hover) hover:text-foreground' +) + +/** + * Composer model selector — the relocated status-bar pill. Reuses the live + * `model.options` dropdown (`modelMenuContent`) verbatim; falls back to the + * full picker when the gateway is closed and no live menu exists. + */ +export function ModelPill({ disabled, model }: { disabled: boolean; model: ChatBarState['model'] }) { + const copy = useI18n().t.shell.statusbar + const currentModel = useStore($currentModel) + const currentProvider = useStore($currentProvider) + const fastMode = useStore($currentFastMode) + const reasoningEffort = useStore($currentReasoningEffort) + const [open, setOpen] = useState(false) + + // The model resolves a beat after the gateway/session comes up. Rather than + // flash a literal "No model", show a quiet loader (inherits the pill text + // color at half opacity) until a model lands. + const label = ( + <> + {currentModel.trim() ? ( + {formatModelStatusLabel(currentModel, { fastMode, reasoningEffort })} + ) : ( + + )} + + + ) + + const title = currentProvider ? copy.modelTitle(currentProvider, currentModel || copy.modelNone) : copy.switchModel + + if (!model.modelMenuContent) { + return ( + + ) + } + + return ( + + + + + + setOpen(false)}> + {model.modelMenuContent} + + + + ) +} diff --git a/apps/desktop/src/app/chat/composer/types.ts b/apps/desktop/src/app/chat/composer/types.ts index 36b3b8e6d3..6d9444a6d9 100644 --- a/apps/desktop/src/app/chat/composer/types.ts +++ b/apps/desktop/src/app/chat/composer/types.ts @@ -1,3 +1,5 @@ +import type { ReactNode } from 'react' + import type { HermesGateway } from '@/hermes' import type { ComposerAttachment } from '@/store/composer' @@ -22,6 +24,8 @@ export interface ChatBarState { canSwitch: boolean loading?: boolean quickModels?: QuickModelOption[] + /** Reused status-bar dropdown (built with gateway + selectModel upstream). */ + modelMenuContent?: ReactNode } tools: { enabled: boolean; label: string; suggestions?: ContextSuggestion[] } voice: { enabled: boolean; active: boolean } diff --git a/apps/desktop/src/app/chat/index.tsx b/apps/desktop/src/app/chat/index.tsx index c9f525653e..4ae3817c88 100644 --- a/apps/desktop/src/app/chat/index.tsx +++ b/apps/desktop/src/app/chat/index.tsx @@ -15,7 +15,9 @@ import { Backdrop } from '@/components/Backdrop' import { PromptOverlays } from '@/components/prompt-overlays' import { Button } from '@/components/ui/button' import { Codicon } from '@/components/ui/codicon' +import { ErrorState } from '@/components/ui/error-state' import { getGlobalModelOptions, type HermesGateway } from '@/hermes' +import { useI18n } from '@/i18n' import type { ChatMessage } from '@/lib/chat-messages' import { quickModelOptions, sessionTitle, toRuntimeMessage } from '@/lib/chat-runtime' import { useIncrementalExternalStoreRuntime } from '@/lib/incremental-external-store-runtime' @@ -38,11 +40,12 @@ import { $lastVisibleMessageIsUser, $messages, $messagesEmpty, + $resumeExhaustedSessionId, $selectedStoredSessionId, $sessions, sessionPinId } from '@/store/session' -import { isNewSessionWindow, isSecondaryWindow } from '@/store/windows' +import { isSecondaryWindow } from '@/store/windows' import type { ModelOptionsResponse } from '@/types/hermes' import { routeSessionId } from '../routes' @@ -62,6 +65,7 @@ import { threadLoadingState } from './thread-loading' interface ChatViewProps extends Omit, 'onSubmit'> { gateway: HermesGateway | null + modelMenuContent?: React.ReactNode onToggleSelectedPin: () => void onDeleteSelectedSession: () => void onCancel: () => Promise | void @@ -85,7 +89,9 @@ interface ChatViewProps extends Omit, 'onSubmit'> { onEdit: (message: AppendMessage) => Promise onReload: (parentId: string | null) => Promise onRestoreToMessage?: (messageId: string) => Promise + onRetryResume: (sessionId: string) => void onTranscribeAudio?: (audio: Blob) => Promise + onDismissError?: (messageId: string) => void } interface ChatHeaderProps { @@ -120,10 +126,10 @@ function ChatHeader({ ? pinnedSessionIds.includes(selectedSessionId) : false - // A brand-new session has no session to pin/delete/rename, so the header is - // just a dead "New session" label + chevron. Drop it (and its border) - // entirely until there's a real session to act on. - if (isNewSessionWindow() || (!selectedSessionId && !activeSessionId && !isRoutedSessionView)) { + // Secondary windows (new-session scratch, subagent watch, cmd-click pop-out) + // are compact side panels — they drop the session-actions header + border + // entirely. A brand-new draft has nothing to pin/delete/rename either. + if (isSecondaryWindow() || (!selectedSessionId && !activeSessionId && !isRoutedSessionView)) { return null } @@ -250,6 +256,7 @@ function ChatRuntimeBoundary({ export function ChatView({ className, gateway, + modelMenuContent, onToggleSelectedPin, onDeleteSelectedSession, onCancel, @@ -270,9 +277,12 @@ export function ChatView({ onEdit, onReload, onRestoreToMessage, - onTranscribeAudio + onRetryResume, + onTranscribeAudio, + onDismissError }: ChatViewProps) { const location = useLocation() + const { t } = useI18n() const activeSessionId = useStore($activeSessionId) const awaitingResponse = useStore($awaitingResponse) const busy = useStore($busy) @@ -294,6 +304,7 @@ export function ChatView({ const messagesEmpty = useStore($messagesEmpty) const lastVisibleIsUser = useStore($lastVisibleMessageIsUser) const selectedSessionId = useStore($selectedStoredSessionId) + const resumeExhaustedSessionId = useStore($resumeExhaustedSessionId) const routedSessionId = routeSessionId(location.pathname) const isRoutedSessionView = Boolean(routedSessionId) @@ -313,9 +324,21 @@ export function ChatView({ // session exists — even if it has zero messages (a brand-new routed // session). The flicker where `busy` flips true briefly during hydrate // is handled by `threadLoadingState`'s last-visible-user gate. - const loadingSession = isRoutedSessionView && (routeSessionMismatch || (messagesEmpty && !activeSessionId)) + // + // resumeExhausted: the bounded auto-retry in use-route-resume gave up on this + // routed session (gateway RPC + REST fallback failed through every attempt). + // Suppress the loader and show an explicit error + manual Retry instead of + // spinning forever. Gated on the route matching so a stale latch from another + // session can't blank the current one. + const resumeExhausted = isRoutedSessionView && resumeExhaustedSessionId === routedSessionId + + const loadingSession = + !resumeExhausted && isRoutedSessionView && (routeSessionMismatch || (messagesEmpty && !activeSessionId)) + const threadLoading = threadLoadingState(loadingSession, busy, awaitingResponse, lastVisibleIsUser) - const showChatBar = !loadingSession + // Hide the composer in the exhausted error state too: there's no live runtime + // to send to until a retry rebinds one. + const showChatBar = !loadingSession && !resumeExhausted const threadKey = selectedSessionId || activeSessionId || (isRoutedSessionView ? location.pathname : 'new') const modelOptionsQuery = useQuery({ @@ -346,6 +369,7 @@ export function ChatView({ provider: currentProvider, canSwitch: gatewayOpen, loading: !gatewayOpen || (!currentModel && !currentProvider), + modelMenuContent, quickModels }, tools: { @@ -358,7 +382,7 @@ export function ChatView({ active: false } }), - [contextSuggestions, currentModel, currentProvider, gatewayOpen, quickModels] + [contextSuggestions, currentModel, currentProvider, gatewayOpen, modelMenuContent, quickModels] ) // Drop files anywhere in the conversation area, not just on the composer @@ -429,6 +453,7 @@ export function ChatView({ loading={threadLoading} onBranchInNewChat={onBranchInNewChat} onCancel={onCancel} + onDismissError={onDismissError} onRestoreToMessage={onRestoreToMessage} sessionId={activeSessionId} sessionKey={threadKey} @@ -462,6 +487,21 @@ export function ChatView({ )} + {resumeExhausted && routedSessionId && ( +
+ +
+ +
+ +
+ )} {showChatBar && } diff --git a/apps/desktop/src/app/desktop-controller.tsx b/apps/desktop/src/app/desktop-controller.tsx index 5ff162a2ca..05dfbbc764 100644 --- a/apps/desktop/src/app/desktop-controller.tsx +++ b/apps/desktop/src/app/desktop-controller.tsx @@ -13,7 +13,7 @@ import { useSkinCommand } from '@/themes/use-skin-command' import { formatRefValue } from '../components/assistant-ui/directive-text' import { getCronJobs, getSessionMessages, listAllProfileSessions, type SessionInfo, triggerCronJob } from '../hermes' -import { preserveLocalAssistantErrors, toChatMessages } from '../lib/chat-messages' +import { type ChatMessage, chatMessageText, preserveLocalAssistantErrors, toChatMessages } from '../lib/chat-messages' import { isMessagingSource, LOCAL_SESSION_SOURCE_IDS, @@ -52,7 +52,10 @@ import { $currentCwd, $freshDraftReady, $gatewayState, + $messages, $messagingSessions, + $resumeFailedSessionId, + $resumeExhaustedSessionId, $selectedStoredSessionId, $sessions, $workingSessionIds, @@ -199,6 +202,8 @@ export function DesktopController() { const activeSessionId = useStore($activeSessionId) const currentCwd = useStore($currentCwd) const freshDraftReady = useStore($freshDraftReady) + const resumeFailedSessionId = useStore($resumeFailedSessionId) + const resumeExhaustedSessionId = useStore($resumeExhaustedSessionId) const filePreviewTarget = useStore($filePreviewTarget) const previewTarget = useStore($previewTarget) const selectedStoredSessionId = useStore($selectedStoredSessionId) @@ -711,7 +716,9 @@ export function DesktopController() { } lastGatewayProfileRef.current = activeGatewayProfile - void refreshCurrentModel() + // Force: the new profile has its own default, so reseed even if the composer + // already shows the previous profile's model. + void refreshCurrentModel(true) void refreshActiveProfile() }, [activeGatewayProfile, refreshCurrentModel]) @@ -734,6 +741,49 @@ export function DesktopController() { [branchCurrentSession, refreshSessions] ) + // Clear a failed turn's red error banner from the transcript. Errors are + // renderer-local state (never persisted), so dismissing is purely a view + + // session-cache edit. A message that errored before emitting any visible + // text is a bare error placeholder → drop it entirely; one that streamed + // partial output then failed keeps its content and just sheds the error. + // Both the per-runtime cache AND the live $messages view must be updated: + // `preserveLocalAssistantErrors` re-grafts any still-errored message it + // finds in the view onto the next session.info flush, so clearing only the + // cache would let the heartbeat resurrect the banner. + const dismissError = useCallback( + (messageId: string) => { + const runtimeSessionId = activeSessionIdRef.current + + if (!runtimeSessionId) { + return + } + + const clearErrorIn = (messages: ChatMessage[]): ChatMessage[] => + messages.flatMap(message => { + if (message.id !== messageId || !message.error) { + return [message] + } + + if (!chatMessageText(message).trim() && !message.parts.some(part => part.type !== 'text')) { + return [] + } + + return [{ ...message, error: undefined, pending: false }] + }) + + // View first: the flush below reads $messages as the "current" baseline + // for error preservation, so the banner must be gone from it before the + // cache update triggers a re-sync. + setMessages(clearErrorIn($messages.get())) + + updateSessionState(runtimeSessionId, state => ({ + ...state, + messages: clearErrorIn(state.messages) + })) + }, + [activeSessionIdRef, updateSessionState] + ) + const startSessionInWorkspace = useCallback( (path: null | string) => { startFreshSessionDraft() @@ -843,6 +893,8 @@ export function DesktopController() { gatewayState, locationPathname: location.pathname, resumeSession, + resumeFailedSessionId, + resumeExhaustedSessionId, routedSessionId, runtimeIdByStoredSessionIdRef, selectedStoredSessionId, @@ -859,7 +911,6 @@ export function DesktopController() { gatewayLogLines, gatewayState, inferenceStatus, - modelMenuContent, openAgents, freshDraftReady, openCommandCenterSection, @@ -981,6 +1032,7 @@ export function DesktopController() { composer.addContextRefAttachment(`@url:${formatRefValue(url)}`, url)} onAttachDroppedItems={composer.attachDroppedItems} @@ -992,6 +1044,7 @@ export function DesktopController() { void removeSession(selectedStoredSessionId) } }} + onDismissError={dismissError} onEdit={editMessage} onPasteClipboardImage={() => void composer.pasteClipboardImage()} onPickFiles={() => void composer.pickContextPaths('file')} @@ -1000,6 +1053,7 @@ export function DesktopController() { onReload={reloadFromMessage} onRemoveAttachment={id => void composer.removeAttachment(id)} onRestoreToMessage={restoreToMessage} + onRetryResume={sessionId => void resumeSession(sessionId, true)} onSteer={steerPrompt} onSubmit={submitText} onThreadMessagesChange={handleThreadMessagesChange} diff --git a/apps/desktop/src/app/right-sidebar/store.ts b/apps/desktop/src/app/right-sidebar/store.ts index 8c07f08245..b0e26f0388 100644 --- a/apps/desktop/src/app/right-sidebar/store.ts +++ b/apps/desktop/src/app/right-sidebar/store.ts @@ -9,3 +9,22 @@ export const $terminalTakeover = atom(storedBoolean(TAKEOVER_KEY, false)) $terminalTakeover.subscribe(active => persistBoolean(TAKEOVER_KEY, active)) export const setTerminalTakeover = (active: boolean) => $terminalTakeover.set(active) + +/** A command queued to run in the embedded terminal. The terminal pane flushes + * (and clears) it once its session is live, so a value set before the pane + * mounts still runs. Cleared after flush so a later remount can't replay it. */ +export const $terminalInjection = atom(null) + +/** Open the terminal pane and run a command in it. Used to disconnect external + * (CLI-managed) providers, which Hermes can't clear via the API — the user + * sees exactly what runs instead of Hermes silently deleting their creds. */ +export const runInTerminal = (command: string) => { + const trimmed = command.trim() + + if (!trimmed) { + return + } + + setTerminalTakeover(true) + $terminalInjection.set(trimmed) +} diff --git a/apps/desktop/src/app/right-sidebar/terminal/use-terminal-session.ts b/apps/desktop/src/app/right-sidebar/terminal/use-terminal-session.ts index 1e5d4d275b..3479ed6db2 100644 --- a/apps/desktop/src/app/right-sidebar/terminal/use-terminal-session.ts +++ b/apps/desktop/src/app/right-sidebar/terminal/use-terminal-session.ts @@ -10,6 +10,8 @@ import { triggerHaptic } from '@/lib/haptics' import { $filePreviewTarget, $previewTarget } from '@/store/preview' import { useTheme } from '@/themes/context' +import { $terminalInjection } from '../store' + import { makeTerminalReader, setActiveTerminalReader } from './buffer' import { isAddSelectionShortcut, @@ -675,6 +677,28 @@ export function useTerminalSession({ cwd, onAddSelectionToChat }: UseTerminalSes return () => cancelAnimationFrame(raf) }, [activeTheme, themeName]) + // Flush a queued command (e.g. a provider-disconnect) into the live session. + // Only active while open; the subscribe fires immediately, so a command set + // before this pane mounted runs as soon as the session is ready. Clearing the + // atom after writing stops a later remount from replaying a stale command. + useEffect(() => { + if (status !== 'open') { + return + } + + return $terminalInjection.subscribe(command => { + const id = sessionIdRef.current + + if (!command || !id) { + return + } + + void window.hermesDesktop?.terminal?.write(id, `${command}\r`) + $terminalInjection.set(null) + termRef.current?.focus() + }) + }, [status]) + return { addSelectionToChat, hostRef, diff --git a/apps/desktop/src/app/session/hooks/use-message-stream.ts b/apps/desktop/src/app/session/hooks/use-message-stream.ts index c07222c689..909c142479 100644 --- a/apps/desktop/src/app/session/hooks/use-message-stream.ts +++ b/apps/desktop/src/app/session/hooks/use-message-stream.ts @@ -13,6 +13,7 @@ import { type GatewayEventPayload, reasoningPart, renderMediaTags, + textPart, upsertToolPart } from '@/lib/chat-messages' import { coerceGatewayText, coerceThinkingText, normalizePersonalityValue } from '@/lib/chat-runtime' @@ -1080,6 +1081,32 @@ export function useMessageStream({ // completions / watch matches here — re-sync the status stack. void refreshBackgroundProcesses(sessionId) } + } else if (event.type === 'review.summary') { + // Self-improvement background review saved something to memory/skills + // and emitted a persistent summary (Python formats it as + // "💾 Self-improvement review: …"). The CLI prints this via + // prompt_toolkit and the Ink TUI renders it as a system line; the + // desktop has neither, so without this handler the skill/memory + // change happens silently. Surface it as a persistent system message + // in the transcript so the user is always informed — it must not be a + // transient toast that can be missed. + const text = coerceGatewayText(payload?.text).trim() + + if (text && sessionId) { + flushQueuedDeltas(sessionId) + updateSessionState(sessionId, state => ({ + ...state, + messages: [ + ...state.messages, + { + id: `review-summary-${Date.now()}`, + role: 'system', + parts: [textPart(text)], + timestamp: Math.floor(Date.now() / 1000) + } + ] + })) + } } else if (event.type === 'error') { const errorMessage = payload?.message || 'Hermes reported an error' const looksLikeProviderSetup = isProviderSetupErrorMessage(errorMessage) @@ -1102,8 +1129,13 @@ export function useMessageStream({ if (looksLikeProviderSetup) { requestDesktopOnboarding(errorMessage) - } else if (isActiveEvent) { + } else { + // Toast globally, not just when the failing thread is focused: a + // turn-ending error (e.g. out of funds) blocks every thread, so the + // inline error alone is too easy to miss. The stable id collapses the + // same error from multiple blocked threads into one toast. notify({ + id: `gateway-error:${errorMessage}`, kind: 'error', title: 'Hermes error', message: errorMessage diff --git a/apps/desktop/src/app/session/hooks/use-model-controls.test.tsx b/apps/desktop/src/app/session/hooks/use-model-controls.test.tsx index 612290800e..f7765de04c 100644 --- a/apps/desktop/src/app/session/hooks/use-model-controls.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-model-controls.test.tsx @@ -130,7 +130,6 @@ describe('useModelControls', () => { await expect( controls.selectModel({ model: 'claude-sonnet-4.6', - persistGlobal: false, provider: 'anthropic' }) ).resolves.toBe(true) @@ -143,26 +142,57 @@ describe('useModelControls', () => { expect(requestGateway).not.toHaveBeenCalledWith('slash.exec', expect.anything()) }) - it('keeps the global path on setGlobalModel when there is no active session', async () => { - setGlobalModel.mockResolvedValue(undefined) + it('stores a no-session pick as UI state with no gateway or global write', async () => { + const requestGateway = vi.fn() let controls!: Controls render( (controls = value)} - requestGateway={vi.fn()} + requestGateway={requestGateway} /> ) await expect( controls.selectModel({ model: 'claude-sonnet-4.6', - persistGlobal: false, provider: 'anthropic' }) ).resolves.toBe(true) - expect(setGlobalModel).toHaveBeenCalledWith('anthropic', 'claude-sonnet-4.6') + // The pick is plain UI state; session.create ships it later. Nothing touches + // the gateway or the profile default here. + expect($currentModel.get()).toBe('claude-sonnet-4.6') + expect($currentProvider.get()).toBe('anthropic') + expect(requestGateway).not.toHaveBeenCalled() + expect(setGlobalModel).not.toHaveBeenCalled() + }) + + it('seeds an empty composer model from global but never clobbers a pick', async () => { + vi.mocked(getGlobalModelInfo).mockResolvedValue({ model: 'openai/gpt-5.5', provider: 'openai-codex' }) + + const { result } = renderHook(() => + useModelControls({ + activeSessionId: null, + queryClient: new QueryClient(), + requestGateway: vi.fn() + }) + ) + + // Empty → seeds the default. + await result.current.refreshCurrentModel() + expect($currentModel.get()).toBe('openai/gpt-5.5') + + // A user pick must survive the lifecycle refreshes that fire on boot / fresh + // draft / session events. + setCurrentModel('anthropic/claude-sonnet-4.6') + setCurrentProvider('anthropic') + await result.current.refreshCurrentModel() + expect($currentModel.get()).toBe('anthropic/claude-sonnet-4.6') + + // A profile swap forces a reseed to the new profile's default. + await result.current.refreshCurrentModel(true) + expect($currentModel.get()).toBe('openai/gpt-5.5') }) }) diff --git a/apps/desktop/src/app/session/hooks/use-model-controls.ts b/apps/desktop/src/app/session/hooks/use-model-controls.ts index 681eac871a..50788b1e0b 100644 --- a/apps/desktop/src/app/session/hooks/use-model-controls.ts +++ b/apps/desktop/src/app/session/hooks/use-model-controls.ts @@ -1,7 +1,7 @@ import { type QueryClient } from '@tanstack/react-query' import { useCallback } from 'react' -import { getGlobalModelInfo, setGlobalModel } from '@/hermes' +import { getGlobalModelInfo } from '@/hermes' import { useI18n } from '@/i18n' import { notifyError } from '@/store/notifications' import { @@ -15,7 +15,6 @@ import type { ModelOptionsResponse } from '@/types/hermes' interface ModelSelection { model: string - persistGlobal: boolean provider: string } @@ -28,6 +27,7 @@ interface ModelControlsOptions { export function useModelControls({ activeSessionId, queryClient, requestGateway }: ModelControlsOptions) { const { t } = useI18n() const copy = t.desktop + const updateModelOptionsCache = useCallback( (provider: string, model: string, includeGlobal: boolean) => { const patch = (prev: ModelOptionsResponse | undefined) => ({ ...(prev ?? {}), provider, model }) @@ -41,14 +41,24 @@ export function useModelControls({ activeSessionId, queryClient, requestGateway [activeSessionId, queryClient] ) - const refreshCurrentModel = useCallback(async () => { + // Seed the composer's model state from the profile default. `force` reseeds + // for a profile swap (the new profile has its own default); otherwise this + // only fills an EMPTY selection so a user's pick (plain UI state in + // $currentModel) survives the lifecycle refreshes that fire on boot / fresh + // draft / session events. A live session owns the footer, so skip entirely. + const refreshCurrentModel = useCallback(async (force = false) => { try { + if ($activeSessionId.get()) { + return + } + + if (!force && $currentModel.get()) { + return + } + const result = await getGlobalModelInfo() - // A resumed/live session owns the footer model state. Global config - // refreshes (gateway boot, profile swap, settings save) must not clobber - // the active chat's runtime model/provider in the status bar. - if ($activeSessionId.get()) { + if ($activeSessionId.get() || (!force && $currentModel.get())) { return } @@ -64,12 +74,14 @@ export function useModelControls({ activeSessionId, queryClient, requestGateway } }, []) - // Returns whether the switch succeeded so callers can await it before - // applying follow-up changes (e.g. editing a model's reasoning/fast must land - // on the right active model — bail rather than write to the previous one). + // Returns whether the switch succeeded so callers can await it before applying + // follow-up changes. The composer model is plain UI state: with no live + // session it's just stored (and shipped on the next session.create); with one + // it's scoped to that session via config.set. It NEVER writes the profile + // default — that lives in Settings → Model — so picking a model here can't + // silently mutate global config. const selectModel = useCallback( async (selection: ModelSelection): Promise => { - const includeGlobal = selection.persistGlobal || !activeSessionId // Snapshot for rollback: the switch is applied optimistically, so a // failure must restore the prior model/provider (store + query cache) // rather than leave the UI showing a model the backend never selected. @@ -78,42 +90,34 @@ export function useModelControls({ activeSessionId, queryClient, requestGateway setCurrentModel(selection.model) setCurrentProvider(selection.provider) - updateModelOptionsCache(selection.provider, selection.model, includeGlobal) + updateModelOptionsCache(selection.provider, selection.model, !activeSessionId) + + // No live session yet: the pick is pure UI state. session.create reads + // $currentModel/$currentProvider and applies it as that session's override. + if (!activeSessionId) { + return true + } try { - if (activeSessionId) { - await requestGateway('config.set', { - session_id: activeSessionId, - key: 'model', - value: `${selection.model} --provider ${selection.provider}${selection.persistGlobal ? ' --global' : ''}` - }) + await requestGateway('config.set', { + session_id: activeSessionId, + key: 'model', + value: `${selection.model} --provider ${selection.provider}` + }) - if (selection.persistGlobal) { - void refreshCurrentModel() - } - - void queryClient.invalidateQueries({ - queryKey: selection.persistGlobal ? ['model-options'] : ['model-options', activeSessionId] - }) - - return true - } - - await setGlobalModel(selection.provider, selection.model) - void refreshCurrentModel() - void queryClient.invalidateQueries({ queryKey: ['model-options'] }) + void queryClient.invalidateQueries({ queryKey: ['model-options', activeSessionId] }) return true } catch (err) { setCurrentModel(prevModel) setCurrentProvider(prevProvider) - updateModelOptionsCache(prevProvider, prevModel, includeGlobal) + updateModelOptionsCache(prevProvider, prevModel, !activeSessionId) notifyError(err, copy.modelSwitchFailed) return false } }, - [activeSessionId, copy.modelSwitchFailed, queryClient, refreshCurrentModel, requestGateway, updateModelOptionsCache] + [activeSessionId, copy.modelSwitchFailed, queryClient, requestGateway, updateModelOptionsCache] ) return { refreshCurrentModel, selectModel, updateModelOptionsCache } diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions.ts index 4c1b50b83a..829119f65b 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions.ts @@ -58,6 +58,7 @@ import { clearSessionTodos } from '@/store/todos' import type { ClientSessionState, + BrowserManageResponse, FileAttachResponse, HandoffFailResponse, HandoffRequestResponse, @@ -1141,6 +1142,81 @@ export function usePromptActions({ } catch (err) { renderSlashOutput(`error: ${err instanceof Error ? err.message : String(err)}`) } + }, + // /browser connect|disconnect|status manages the live CDP connection on + // the gateway host, mirroring the TUI's browser.manage RPC. It mutates + // BROWSER_CDP_URL (and may launch Chrome) in the gateway process — only + // meaningful when that process runs on this machine, so it's gated to + // local connections. A remote gateway would act on the wrong host. + browser: async ctx => { + const resolved = await withSlashOutput(ctx) + + if (!resolved) { + return + } + + const { render: renderSlashOutput, sessionId } = resolved + + if ($connection.get()?.mode === 'remote') { + renderSlashOutput( + '/browser manages a Chromium-family browser on the gateway host — only available when connected to a local gateway.' + ) + + return + } + + const [rawAction = 'status', ...rest] = ctx.arg.trim().split(/\s+/).filter(Boolean) + const cmdAction = rawAction.toLowerCase() + + if (!['connect', 'disconnect', 'status'].includes(cmdAction)) { + renderSlashOutput( + 'usage: /browser [connect|disconnect|status] [url] · persistent: set browser.cdp_url in config.yaml' + ) + + return + } + + const url = cmdAction === 'connect' ? rest.join(' ').trim() || 'http://127.0.0.1:9222' : undefined + + if (url) { + renderSlashOutput(`checking Chromium-family browser remote debugging at ${url}...`) + } + + try { + const result = await requestGateway('browser.manage', { + action: cmdAction, + session_id: sessionId, + ...(url && { url }) + }) + + // Without a streamed session subscription, the gateway bundles its + // progress lines into `messages` — flush them inline. + result?.messages?.forEach(message => renderSlashOutput(message)) + + if (cmdAction === 'status') { + renderSlashOutput( + result?.connected + ? `browser connected: ${result.url || '(url unavailable)'}` + : 'browser not connected (try /browser connect or set browser.cdp_url in config.yaml)' + ) + + return + } + + if (cmdAction === 'disconnect') { + renderSlashOutput('browser disconnected') + + return + } + + if (result?.connected) { + renderSlashOutput('Browser connected to live Chromium-family browser via CDP') + renderSlashOutput(`Endpoint: ${result.url || '(url unavailable)'}`) + renderSlashOutput('next browser tool call will use this CDP endpoint') + } + } catch (err) { + renderSlashOutput(`error: ${err instanceof Error ? err.message : String(err)}`) + } } } diff --git a/apps/desktop/src/app/session/hooks/use-route-resume.test.tsx b/apps/desktop/src/app/session/hooks/use-route-resume.test.tsx index e0d984c37f..e05f8b748d 100644 --- a/apps/desktop/src/app/session/hooks/use-route-resume.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-route-resume.test.tsx @@ -2,6 +2,8 @@ import { cleanup, render } from '@testing-library/react' import type { MutableRefObject } from 'react' import { afterEach, describe, expect, it, vi } from 'vitest' +import { $resumeExhaustedSessionId, setResumeExhaustedSessionId } from '@/store/session' + import { useRouteResume } from './use-route-resume' interface HarnessProps { @@ -13,6 +15,8 @@ interface HarnessProps { gatewayState: string locationPathname: string resumeSession: (sessionId: string, focus: boolean) => Promise + resumeFailedSessionId?: null | string + resumeExhaustedSessionId?: null | string routedSessionId: null | string runtimeIdByStoredSessionIdRef: MutableRefObject> selectedStoredSessionId: null | string @@ -20,8 +24,12 @@ interface HarnessProps { startFreshSessionDraft: (focus: boolean) => unknown } -function RouteResumeHarness(props: HarnessProps) { - useRouteResume(props) +function RouteResumeHarness({ + resumeFailedSessionId = null, + resumeExhaustedSessionId = null, + ...props +}: HarnessProps) { + useRouteResume({ ...props, resumeExhaustedSessionId, resumeFailedSessionId }) return null } @@ -256,3 +264,212 @@ describe('useRouteResume', () => { expect(resumeSession).toHaveBeenCalledWith('session-1', true) }) }) + +describe('useRouteResume bounded auto-retry after a failed resume', () => { + afterEach(() => { + cleanup() + vi.useRealTimers() + vi.restoreAllMocks() + setResumeExhaustedSessionId(null) + }) + + // Common stranded-window props: gateway open, route on the session, no runtime + // yet, and the ref already synced to the route (resumeSession sets it at entry + // before failing) — the exact state that defeats the main effect's self-heal. + function strandedProps(resumeSession: (sid: string, focus: boolean) => Promise) { + return { + activeSessionId: null, + activeSessionIdRef: { current: null } as MutableRefObject, + creatingSessionRef: { current: false }, + currentView: 'chat', + freshDraftReady: false, + gatewayState: 'open', + locationPathname: '/session-1', + resumeSession, + routedSessionId: 'session-1', + runtimeIdByStoredSessionIdRef: { current: new Map() }, + selectedStoredSessionId: 'session-1', + // Synced to the route by the failed resume's synchronous entry-write. + selectedStoredSessionIdRef: { current: 'session-1' } as MutableRefObject, + startFreshSessionDraft: vi.fn() + } + } + + it('retries the resume on backoff when the routed session is flagged as failed', () => { + vi.useFakeTimers() + const resumeSession = vi.fn(async () => undefined) + + render() + + // The main effect fires one resume on mount (pathname-changed). Clear it so + // we assert purely the bounded-retry effect's scheduled retry below. + resumeSession.mockClear() + + // No immediate fire — the retry is scheduled behind the backoff timer. + expect(resumeSession).not.toHaveBeenCalled() + + // First backoff window (1s) elapses → one retry. + vi.advanceTimersByTime(1_000) + expect(resumeSession).toHaveBeenCalledTimes(1) + expect(resumeSession).toHaveBeenCalledWith('session-1', true) + }) + + it('does NOT retry a failed session that is not the routed one', () => { + vi.useFakeTimers() + const resumeSession = vi.fn(async () => undefined) + + // The failure flag points at a different session than the route. + render() + resumeSession.mockClear() // drop the mount resume + + vi.advanceTimersByTime(10_000) + expect(resumeSession).not.toHaveBeenCalled() + }) + + it('skips the scheduled retry if the session already recovered when the timer fires', () => { + vi.useFakeTimers() + const resumeSession = vi.fn(async () => undefined) + const props = strandedProps(resumeSession) + + render() + resumeSession.mockClear() // drop the mount resume + + // A resume landed while we waited: runtime is now bound. + props.activeSessionIdRef.current = 'runtime-1' + + vi.advanceTimersByTime(8_000) + expect(resumeSession).not.toHaveBeenCalled() + }) + + it('stops retrying after MAX_RESUME_RETRIES consecutive failures', () => { + vi.useFakeTimers() + const resumeSession = vi.fn(async () => undefined) + const props = strandedProps(resumeSession) + + // Model the real re-arm loop: resumeSession clears $resumeFailedSessionId at + // entry (null) and a repeat failure re-sets it ('session-1'). That null->id + // toggle is what re-runs the effect and advances the bounded counter. The + // routed session never changes, so the counter is NOT reset between cycles. + const { rerender } = render() + resumeSession.mockClear() // drop the mount resume; count only the retries + + for (let i = 0; i < 8; i += 1) { + vi.advanceTimersByTime(8_000) // fire the scheduled retry (if any) + rerender() // cleared at entry + rerender() // re-armed on failure + } + + // Capped at MAX_RESUME_RETRIES (4): a persistently dead backend can't + // hot-loop the resume forever. + expect(resumeSession.mock.calls.length).toBe(4) + + // Once auto-retry gives up, the exhausted latch is armed for the routed + // session so the chat view can swap the perpetual loader for an explicit + // error + manual Retry instead of spinning forever. + expect($resumeExhaustedSessionId.get()).toBe('session-1') + }) + + it('does not arm the exhausted latch while retries remain', () => { + vi.useFakeTimers() + const resumeSession = vi.fn(async () => undefined) + const props = strandedProps(resumeSession) + + const { rerender } = render() + resumeSession.mockClear() + + // Two failure cycles — still under the 4-retry cap, so the latch must stay + // clear and the loader keeps spinning (auto-recovery hasn't given up yet). + for (let i = 0; i < 2; i += 1) { + vi.advanceTimersByTime(8_000) + rerender() + rerender() + } + + expect($resumeExhaustedSessionId.get()).toBeNull() + }) + + it('clears a stale exhausted latch when the route moves off the stranded session', () => { + vi.useFakeTimers() + const resumeSession = vi.fn(async () => undefined) + const props = strandedProps(resumeSession) + + // Pre-arm the latch as if this session had exhausted its retries. + setResumeExhaustedSessionId('session-1') + + // Route is now on a different, healthy session that is not flagged as + // failed — the retry effect's "route moved off" branch clears the latch. + render( + + ) + + expect($resumeExhaustedSessionId.get()).toBeNull() + }) + + it('resets the retry counter for a fresh backoff cycle when the exhausted latch clears (manual retry, same session)', () => { + vi.useFakeTimers() + const resumeSession = vi.fn(async () => undefined) + const props = strandedProps(resumeSession) + + // Phase A — exhaust the bounded auto-retry (counter → MAX) like a dead + // backend. The resumeExhaustedSessionId prop stays null here: the hook sets + // the store, which doesn't feed back into the prop in this harness. + const { rerender } = render() + resumeSession.mockClear() + for (let i = 0; i < 8; i += 1) { + vi.advanceTimersByTime(8_000) + rerender() + rerender() + } + expect(resumeSession.mock.calls.length).toBe(4) // capped + expect($resumeExhaustedSessionId.get()).toBe('session-1') + + // Phase B — user clicks Retry on the SAME stranded session. resumeSession + // clears both latches at entry; the exhausted latch's armed->cleared edge + // must reset the attempt counter so a fresh bounded cycle runs, not a single + // one-shot attempt that immediately re-arms the error. Model the prop + // transitions: reflect the armed latch, then clear it (retry), then re-arm + // the failure latch on the fresh failure. + resumeSession.mockClear() + rerender() + rerender() + rerender() + + // A real retry fires again instead of staying pinned at MAX (which would + // dispatch nothing). Without the reset the counter stays >= MAX and this + // advance dispatches zero resumes. + vi.advanceTimersByTime(8_000) + expect(resumeSession.mock.calls.length).toBeGreaterThan(0) + }) + + it('does not burn retry attempts on unrelated re-renders during the backoff window', () => { + vi.useFakeTimers() + const props = strandedProps(vi.fn()) + + // Mount schedules the first backoff timer. Then re-render repeatedly with a + // fresh resumeSession identity (referential instability — a real dep change + // for the retry effect) WITHOUT ever letting the timer fire. The old code + // incremented the attempt counter at schedule time, so >= MAX re-renders + // armed the exhausted error with zero resumes actually dispatched. The fix + // only advances the counter when a timer truly fires, so the latch stays + // clear no matter how many spurious re-renders happen mid-backoff. + const { rerender } = render( + undefined)} /> + ) + for (let j = 0; j < 8; j += 1) { + rerender( + undefined)} /> + ) + } + + expect($resumeExhaustedSessionId.get()).toBeNull() + }) +}) diff --git a/apps/desktop/src/app/session/hooks/use-route-resume.ts b/apps/desktop/src/app/session/hooks/use-route-resume.ts index ad7677cc4b..1be8da90c6 100644 --- a/apps/desktop/src/app/session/hooks/use-route-resume.ts +++ b/apps/desktop/src/app/session/hooks/use-route-resume.ts @@ -1,6 +1,7 @@ import { type MutableRefObject, useEffect, useRef } from 'react' import { isNewChatRoute } from '@/app/routes' +import { setResumeExhaustedSessionId } from '@/store/session' interface RouteResumeOptions { activeSessionId: string | null @@ -11,6 +12,17 @@ interface RouteResumeOptions { gatewayState: string | undefined locationPathname: string resumeSession: (sessionId: string, focus: boolean) => Promise + // Stored-session id whose most recent resume failed terminally (set by + // useSessionActions, mirrored from $resumeFailedSessionId). While this equals + // routedSessionId the window would otherwise latch on the loader forever, so + // the bounded-retry effect below re-attempts the resume. + resumeFailedSessionId: string | null + // Stored-session id whose bounded auto-retry has EXHAUSTED (mirrored from + // $resumeExhaustedSessionId). Only resumeSession clears this latch (manual + // Retry / reconnect / reselect) — the auto-retry loop never does — so its + // armed->cleared edge is an unambiguous "give me a fresh backoff cycle" + // signal the effect below uses to reset the attempt counter. + resumeExhaustedSessionId: string | null routedSessionId: string | null runtimeIdByStoredSessionIdRef: MutableRefObject> selectedStoredSessionId: string | null @@ -18,6 +30,19 @@ interface RouteResumeOptions { startFreshSessionDraft: (focus: boolean) => unknown } +// Bounded auto-retry for a stranded session window. A resume can fail terminally +// (gateway RPC reject + REST fallback failure) on a transiently wedged backend — +// dead provider key, a runaway turn hogging the dispatcher, flaky DNS. Without a +// retry the loader latches forever. We retry with backoff, capped, so a +// genuinely dead backend doesn't hot-loop the resume. +const MAX_RESUME_RETRIES = 4 +const RESUME_RETRY_BASE_MS = 1_000 +const RESUME_RETRY_MAX_MS = 8_000 + +function resumeRetryDelayMs(attempt: number): number { + return Math.min(RESUME_RETRY_MAX_MS, RESUME_RETRY_BASE_MS * 2 ** attempt) +} + // HashRouter boot edge case: pathname briefly reads `/` before the hash is // parsed. If the hash references a real session, defer; resume picks it up // next tick. Without this, ctrl+R on `#/:sessionId` flashes 5 loading states. @@ -49,6 +74,8 @@ export function useRouteResume({ gatewayState, locationPathname, resumeSession, + resumeFailedSessionId, + resumeExhaustedSessionId, routedSessionId, runtimeIdByStoredSessionIdRef, selectedStoredSessionId, @@ -58,6 +85,16 @@ export function useRouteResume({ const lastPathnameRef = useRef(null) const seenGatewayStateRef = useRef(false) const wasGatewayOpenRef = useRef(false) + // Per-session retry bookkeeping for the bounded auto-retry effect below. Keyed + // by the session id we're retrying so switching chats resets the counter. + const retrySessionIdRef = useRef(null) + const retryAttemptRef = useRef(0) + // Tracks the previous exhausted-latch value so we can detect its armed->cleared + // edge. resumeSession clears $resumeExhaustedSessionId on a manual Retry / + // reconnect / reselect; that transition is our cue to reset the attempt counter + // for a fresh backoff cycle on the SAME session (the auto-retry loop itself + // never touches this latch, so it can't spuriously trigger the reset). + const prevResumeExhaustedRef = useRef(null) useEffect(() => { const gatewayOpen = gatewayState === 'open' @@ -139,4 +176,111 @@ export function useRouteResume({ selectedStoredSessionIdRef, startFreshSessionDraft ]) + + // Bounded auto-retry: when the routed session's resume failed terminally + // (resumeFailedSessionId matches the route), schedule a backoff retry so the + // window recovers on its own instead of latching the loader forever. This is + // the safety net the main effect above can't provide: after a failed resume, + // selectedStoredSessionIdRef.current already equals the route (resumeSession + // sets it synchronously at entry) and the pathname/gateway are unchanged, so + // none of stuckOnRoutedSession / pathnameChanged / gatewayBecameOpen fire + // again. resumeSession clears resumeFailedSessionId on its next attempt; a + // success keeps it clear (the effect's guard then no-ops), a repeat failure + // re-arms it and we back off further, capped at MAX_RESUME_RETRIES. + useEffect(() => { + // Detect the exhausted-latch armed->cleared edge for the current route. Only + // resumeSession clears $resumeExhaustedSessionId (manual Retry / reconnect / + // reselect) — the auto-retry loop never touches it — so this transition + // uniquely means "the user asked for another go." Reset the attempt counter + // for a fresh bounded backoff cycle on the SAME session. Without this, + // retryAttemptRef stays pinned at MAX after exhaustion (the !stranded reset + // below only fires on a route CHANGE to a different session), so a manual + // retry on the same stranded session would get exactly ONE attempt and then + // immediately re-arm the exhausted error — never the renewed backoff cycle + // the store/session.ts + use-session-actions.ts comments promise. (Point 2) + const wasExhausted = prevResumeExhaustedRef.current + prevResumeExhaustedRef.current = resumeExhaustedSessionId + if (wasExhausted && wasExhausted === routedSessionId && resumeExhaustedSessionId !== wasExhausted) { + retrySessionIdRef.current = routedSessionId + retryAttemptRef.current = 0 + } + + if (currentView !== 'chat' || gatewayState !== 'open') { + return + } + + const stranded = + Boolean(routedSessionId) && + resumeFailedSessionId === routedSessionId && + !creatingSessionRef.current + + if (!stranded) { + // Route moved off the stranded session (or it recovered) — reset the + // counter so a future failure on another session starts fresh, and clear + // any exhausted-latch armed for a session we're no longer viewing (never + // the current route: that's the error state we want to keep showing). + // resumeSession also clears it on a fresh attempt; this covers a plain + // route-change away from the stranded window. + if (retrySessionIdRef.current !== routedSessionId) { + retrySessionIdRef.current = null + retryAttemptRef.current = 0 + setResumeExhaustedSessionId(current => (current && current !== routedSessionId ? null : current)) + } + + return + } + + // New stranded session id → reset the attempt counter. + if (retrySessionIdRef.current !== routedSessionId) { + retrySessionIdRef.current = routedSessionId + retryAttemptRef.current = 0 + } + + if (retryAttemptRef.current >= MAX_RESUME_RETRIES) { + // Give up auto-retrying a persistently dead backend; the user can still + // reconnect / reselect (which resets the counter via the branch above). + // Surface an explicit error + manual Retry in the chat view instead of + // spinning the loader forever — resumeSession (manual Retry / reconnect / + // reselect) clears this latch and resets the counter for a fresh cycle. + setResumeExhaustedSessionId(routedSessionId) + + return + } + + const attempt = retryAttemptRef.current + const sessionId = routedSessionId as string + + const timer = setTimeout(() => { + // Re-check liveness at fire time: a resume may have landed while we waited. + if ( + creatingSessionRef.current || + selectedStoredSessionIdRef.current !== sessionId || + activeSessionIdRef.current !== null + ) { + return + } + + // Consume an attempt ONLY now that a resume is actually dispatching. + // Incrementing at schedule time (the old behavior) let unrelated dep + // changes during the 1s–8s backoff window — a transient gatewayState + // flip, a non-referentially-stable resumeSession — clear the pending + // timer and re-run the effect, burning an attempt without any resume + // having fired. A flapping backend could then hit MAX in a couple of + // re-renders with far fewer than MAX real attempts. (Point 3) + retryAttemptRef.current += 1 + void resumeSession(sessionId, true) + }, resumeRetryDelayMs(attempt)) + + return () => clearTimeout(timer) + }, [ + activeSessionIdRef, + creatingSessionRef, + currentView, + gatewayState, + resumeSession, + resumeFailedSessionId, + resumeExhaustedSessionId, + routedSessionId, + selectedStoredSessionIdRef + ]) } diff --git a/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx b/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx index 739e8b9375..a84a854ded 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx @@ -3,8 +3,9 @@ import type { MutableRefObject } from 'react' import { useEffect } from 'react' import { afterEach, describe, expect, it, vi } from 'vitest' +import { getSessionMessages } from '@/hermes' import { $activeGatewayProfile, $newChatProfile } from '@/store/profile' -import { $currentCwd } from '@/store/session' +import { $currentCwd, $messages, $resumeFailedSessionId, setMessages, setResumeFailedSessionId } from '@/store/session' import type { ClientSessionState } from '../../types' @@ -117,3 +118,142 @@ describe('createBackendSessionForSend profile routing', () => { expect(params).toMatchObject({ profile: 'default' }) }) }) + +// ── Resume failure recovery (the "stuck loading session window" bug) ────────── +// When session.resume rejects AND the REST transcript fallback ALSO fails, the +// hook must (a) not throw out of the fallback (which stranded the loader), and +// (b) arm $resumeFailedSessionId so use-route-resume can retry. A resume that +// succeeds must NOT leave the flag armed. +function ResumeHarness({ + onReady, + requestGateway +}: { + onReady: (resume: (storedSessionId: string, replaceRoute?: boolean) => Promise) => void + requestGateway: (method: string, params?: Record) => Promise +}) { + const ref = (value: T): MutableRefObject => ({ current: value }) + + const actions = useSessionActions({ + activeSessionId: null, + activeSessionIdRef: ref(null), + busyRef: ref(false), + creatingSessionRef: ref(false), + ensureSessionState: () => ({}) as ClientSessionState, + getRouteToken: () => 'token', + navigate: vi.fn() as never, + requestGateway, + runtimeIdByStoredSessionIdRef: ref(new Map()), + selectedStoredSessionId: null, + selectedStoredSessionIdRef: ref(null), + sessionStateByRuntimeIdRef: ref(new Map()), + syncSessionStateToView: vi.fn(), + updateSessionState: (_sessionId, updater) => updater({} as ClientSessionState) + }) + + useEffect(() => { + onReady(actions.resumeSession) + }, [actions.resumeSession, onReady]) + + return null +} + +describe('resumeSession failure recovery', () => { + afterEach(() => { + cleanup() + setResumeFailedSessionId(null) + setMessages([]) + vi.restoreAllMocks() + }) + + async function runResume( + requestGateway: (method: string, params?: Record) => Promise + ): Promise { + let resume: ((storedSessionId: string, replaceRoute?: boolean) => Promise) | null = null + render( (resume = r)} requestGateway={requestGateway} />) + await waitFor(() => expect(resume).not.toBeNull()) + await resume!('stored-1', true) + } + + it('arms $resumeFailedSessionId when resume RPC and REST fallback both fail', async () => { + // session.resume rejects (e.g. timeout against a wedged backend)... + const requestGateway = vi.fn(async (method: string) => { + if (method === 'session.resume') { + throw new Error('request timed out: session.resume') + } + + return {} as never + }) + + // ...and the REST transcript fallback also rejects (backend unreachable). + vi.mocked(getSessionMessages).mockRejectedValue(new Error('network down')) + + await runResume(requestGateway) + + // The window is no longer silently stranded: the failure latch is armed for + // the stored session, which use-route-resume consumes to retry. + expect($resumeFailedSessionId.get()).toBe('stored-1') + }) + + it('does NOT arm the failure latch when the resume RPC fails but the REST fallback paints history', async () => { + // session.resume rejects, but the REST transcript fallback succeeds and + // hydrates a readable transcript — the window is NOT stranded. + const requestGateway = vi.fn(async (method: string) => { + if (method === 'session.resume') { + throw new Error('request timed out: session.resume') + } + + return {} as never + }) + + vi.mocked(getSessionMessages).mockResolvedValue({ + messages: [ + { content: 'hello', role: 'user', timestamp: 1 }, + { content: 'hi there', role: 'assistant', timestamp: 2 } + ], + session_id: 'stored-1' + } as never) + + await runResume(requestGateway) + + // Arming here would auto-retry a window that already shows history and, + // on exhaustion, blank that transcript behind the error overlay — a + // regression vs. plain fallback-success. The latch must stay clear. + expect($resumeFailedSessionId.get()).toBeNull() + // The fallback transcript is visible. + expect($messages.get().length).toBeGreaterThan(0) + }) + + it('does NOT throw out of the fallback when REST also fails (no unhandled rejection)', async () => { + const requestGateway = vi.fn(async (method: string) => { + if (method === 'session.resume') { + throw new Error('request timed out: session.resume') + } + + return {} as never + }) + + vi.mocked(getSessionMessages).mockRejectedValue(new Error('network down')) + + // resumeSession must resolve (swallow the fallback failure), not reject. + await expect(runResume(requestGateway)).resolves.toBeUndefined() + }) + + it('leaves the failure latch clear when resume succeeds', async () => { + // Pre-arm to prove a successful resume clears it (entry-clear path). + setResumeFailedSessionId('stored-1') + + const requestGateway = vi.fn(async (method: string, params?: Record) => { + if (method === 'session.resume') { + return { session_id: 'runtime-1', resumed: params?.session_id, messages: [], info: {} } as never + } + + return {} as never + }) + + vi.mocked(getSessionMessages).mockResolvedValue({ messages: [] } as never) + + await runResume(requestGateway) + + expect($resumeFailedSessionId.get()).toBeNull() + }) +}) diff --git a/apps/desktop/src/app/session/hooks/use-session-actions.ts b/apps/desktop/src/app/session/hooks/use-session-actions.ts index 50b6bb0d27..36dfea759f 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions.ts +++ b/apps/desktop/src/app/session/hooks/use-session-actions.ts @@ -15,6 +15,10 @@ import { requestDesktopOnboarding } from '@/store/onboarding' import { $activeGatewayProfile, $newChatProfile, $profiles, ensureGatewayProfile, normalizeProfileKey } from '@/store/profile' import { $currentCwd, + $currentFastMode, + $currentModel, + $currentProvider, + $currentReasoningEffort, $messages, $sessions, $yoloActive, @@ -34,6 +38,8 @@ import { setFreshDraftReady, setIntroSeed, setMessages, + setResumeExhaustedSessionId, + setResumeFailedSessionId, setSelectedStoredSessionId, setSessions, setSessionStartedAt, @@ -407,13 +413,13 @@ export function useSessionActions({ }) setSessionStartedAt(null) setTurnStartedAt(null) - // New chats start in the configured default project dir when set, - // otherwise the sticky last-used workspace (PR #37586). - setCurrentModel('') - setCurrentProvider('') - setCurrentReasoningEffort('') + // The composer's model/effort/fast is sticky UI state (persisted in + // localStorage) — a new chat FOLLOWS your last pick instead of snapping + // back to the profile default, so we deliberately don't reset it here. The + // profile default still owns first-run seeding and profile switches (see + // refreshCurrentModel). Only $currentServiceTier (a live-session mirror) + // is cleared. setCurrentServiceTier('') - setCurrentFastMode(false) setYoloActive(false) setCurrentCwd(workspaceCwdForNewSession()) setCurrentBranch('') @@ -443,11 +449,23 @@ export function useSessionActions({ const newChatProfile = $newChatProfile.get() ?? normalizeProfileKey($activeGatewayProfile.get()) await ensureGatewayProfile(newChatProfile) const cwd = $currentCwd.get().trim() || workspaceCwdForNewSession() + // The composer's model/effort/fast is sticky UI state ($currentModel, + // $currentProvider, $currentReasoningEffort, $currentFastMode). Ship it + // with every session.create so the new chat opens on whatever the picker + // shows — applied as per-session overrides, never written to the profile + // default (that lives in Settings → Model). + const uiModel = $currentModel.get().trim() + const uiProvider = $currentProvider.get().trim() + const uiEffort = $currentReasoningEffort.get().trim() + const uiFast = $currentFastMode.get() const created = await requestGateway('session.create', { cols: 96, ...(cwd && { cwd }), - ...(newChatProfile ? { profile: newChatProfile } : {}) + ...(newChatProfile ? { profile: newChatProfile } : {}), + ...(uiModel ? { model: uiModel, ...(uiProvider ? { provider: uiProvider } : {}) } : {}), + ...(uiEffort ? { reasoning_effort: uiEffort } : {}), + ...(uiFast ? { fast: true } : {}) }) const stored = created.stored_session_id ?? null @@ -563,6 +581,15 @@ export function useSessionActions({ clearNotifications() setSelectedStoredSessionId(storedSessionId) selectedStoredSessionIdRef.current = storedSessionId + // Optimistically clear any prior resume-failure latch for this session: + // we're attempting a fresh resume, so the self-heal in use-route-resume + // must not keep treating it as stranded. It's re-armed below only if THIS + // attempt fails terminally (RPC reject + REST fallback failure). + setResumeFailedSessionId(current => (current === storedSessionId ? null : current)) + // Also clear the exhausted-latch: a fresh attempt (manual Retry, reconnect, + // reselect) gives the bounded auto-retry counter a clean cycle, so the + // chat view drops the error state and shows the loader again. + setResumeExhaustedSessionId(current => (current === storedSessionId ? null : current)) const warmRuntimeId = runtimeIdByStoredSessionIdRef.current.get(storedSessionId) @@ -753,13 +780,41 @@ export function useSessionActions({ return } - const fallback = await getSessionMessages(storedSessionId, sessionProfile) + // The gateway resume RPC failed. Try the REST transcript as a fallback + // so the window at least shows history. CRITICAL: this fallback must be + // wrapped in its own try — if it ALSO throws (wedged/unreachable backend, + // the common case when resume failed in the first place), an unguarded + // throw here skips setMessages AND leaves activeSessionId null with an + // empty transcript. That is the exact state the thread loader latches on + // forever (messagesEmpty && !activeSessionId) with no recovery path — + // the "open in new window stays stuck loading, even after a nap" bug. + try { + const fallback = await getSessionMessages(storedSessionId, sessionProfile) - if (!isCurrentResume()) { - return + if (!isCurrentResume()) { + return + } + + setMessages(preserveLocalAssistantErrors(toChatMessages(fallback.messages), $messages.get())) + } catch { + // Fallback also failed: nothing to paint. Leave whatever messages are + // already shown and fall through to arm the resume-failure latch so + // use-route-resume re-attempts the resume on the next render / window + // focus / gateway reconnect instead of stranding the loader. + } + + if (isCurrentResume() && $messages.get().length === 0) { + // Arm the self-heal ONLY when the window is still empty: the gateway + // resume rejected AND the REST fallback failed to paint a transcript. + // That is the exact stranded state the loader latches on + // (messagesEmpty && !activeSessionId), and matches $resumeFailedSessionId's + // documented contract. If the REST fallback DID paint history, the + // window is readable — arming here would needlessly auto-retry and, + // once retries exhaust, blank that visible transcript behind the + // exhausted-state error overlay (a regression vs. plain fallback success). + setResumeFailedSessionId(storedSessionId) } - setMessages(preserveLocalAssistantErrors(toChatMessages(fallback.messages), $messages.get())) notifyError(err, copy.resumeFailed) } finally { if (isCurrentResume()) { diff --git a/apps/desktop/src/app/session/hooks/use-session-state-cache.test.tsx b/apps/desktop/src/app/session/hooks/use-session-state-cache.test.tsx index e2a9735827..681334aa2d 100644 --- a/apps/desktop/src/app/session/hooks/use-session-state-cache.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-session-state-cache.test.tsx @@ -2,12 +2,14 @@ import { act, cleanup, render } from '@testing-library/react' import type { MutableRefObject } from 'react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { ChatMessage } from '@/lib/chat-messages' import { $currentFastMode, $currentModel, $currentProvider, $currentReasoningEffort, $currentServiceTier, + $messages, $turnStartedAt, setCurrentFastMode, setCurrentModel, @@ -213,3 +215,113 @@ describe('useSessionStateCache — per-session turn timer', () => { expect($currentFastMode.get()).toBe(false) }) }) + +function userMessage(id: string, text: string): ChatMessage { + return { id, role: 'user', parts: [{ type: 'text', text }] } +} + +function assistantText(id: string, text: string): ChatMessage { + return { id, role: 'assistant', parts: [{ type: 'text', text }] } +} + +function assistantError(id: string, error: string): ChatMessage { + return { id, role: 'assistant', parts: [], error, pending: false } +} + +interface ViewHarnessProps { + activeSessionId: string | null + onReady: (cache: Cache) => void +} + +function ViewHarness({ activeSessionId, onReady }: ViewHarnessProps) { + const busyRef: MutableRefObject = { current: false } + const cache = useSessionStateCache({ + activeSessionId, + busyRef, + selectedStoredSessionId: null, + setAwaitingResponse: () => undefined, + setBusy: () => undefined, + // Wire the published view back into the real $messages atom the flush + // reads from, so the round-trip matches production. + setMessages: messages => $messages.set(messages) + }) + + onReady(cache) + + return null +} + +describe('useSessionStateCache — cross-thread error isolation', () => { + afterEach(() => { + cleanup() + $messages.set([]) + }) + + it('does not leak a failed turn into another thread on switch', () => { + $messages.set([]) + let cache!: Cache + const { rerender } = render( (cache = c)} />) + + // Thread A ends its turn with an out-of-funds error and is on screen. + act(() => { + cache.updateSessionState( + 'thread-A', + state => ({ + ...state, + busy: false, + messages: [userMessage('user-a', 'do the thing'), assistantError('assistant-a-error', 'Out of funds')] + }), + 'stored-A' + ) + }) + + expect($messages.get().some(message => message.error === 'Out of funds')).toBe(true) + + // Switch to thread B (which completed cleanly). Its cached state syncs to + // the view while $messages still holds thread A's transcript. + rerender( (cache = c)} />) + act(() => { + cache.updateSessionState( + 'thread-B', + state => ({ + ...state, + busy: false, + messages: [userMessage('user-b', 'hello'), assistantText('assistant-b', 'hi there')] + }), + 'stored-B' + ) + }) + + expect($messages.get().map(message => message.id)).toEqual(['user-b', 'assistant-b']) + expect($messages.get().some(message => message.error === 'Out of funds')).toBe(false) + }) + + it('still preserves a same-session local error a heartbeat dropped', () => { + $messages.set([]) + let cache!: Cache + render( (cache = c)} />) + + // First paint establishes thread A as the on-screen session. + act(() => { + cache.updateSessionState( + 'thread-A', + state => ({ ...state, busy: false, messages: [userMessage('user-a', 'do the thing')] }), + 'stored-A' + ) + }) + + // A local error lands in the view (e.g. failAssistantMessage wrote it). + $messages.set([userMessage('user-a', 'do the thing'), assistantError('assistant-a-error', 'OpenRouter 403')]) + + // A later same-session heartbeat carries cached state that lost the error. + act(() => { + cache.updateSessionState('thread-A', state => ({ + ...state, + busy: false, + messages: [userMessage('user-a', 'do the thing')] + })) + }) + + expect($messages.get().some(message => message.error === 'OpenRouter 403')).toBe(true) + }) +}) diff --git a/apps/desktop/src/app/session/hooks/use-session-state-cache.ts b/apps/desktop/src/app/session/hooks/use-session-state-cache.ts index a08eb1f16c..1445dd17a7 100644 --- a/apps/desktop/src/app/session/hooks/use-session-state-cache.ts +++ b/apps/desktop/src/app/session/hooks/use-session-state-cache.ts @@ -79,6 +79,9 @@ export function useSessionStateCache({ const runtimeIdByStoredSessionIdRef = useRef(new Map()) const pendingViewStateRef = useRef<{ sessionId: string; state: ClientSessionState } | null>(null) const viewSyncRafRef = useRef(null) + // Runtime id whose transcript currently occupies `$messages` — lets the + // flush below tell a same-session refresh from a thread switch. + const viewSessionIdRef = useRef(null) useEffect(() => { activeSessionIdRef.current = activeSessionId @@ -142,12 +145,22 @@ export function useSessionStateCache({ // jerks the scroll position while the user is reading. Skip the publish when // the merged result is content-identical to what's already on screen. const currentMessages = $messages.get() - const nextMessages = preserveLocalAssistantErrors(pending.state.messages, currentMessages) + // On a thread switch `$messages` still holds the *previous* thread, so + // preserving its local errors would graft that thread's failed turn (e.g. + // an out-of-funds error) onto this one — then cascade it everywhere as the + // polluted view becomes the next switch's baseline. Only carry errors + // across a same-session refresh; our cached state already keeps its own. + const nextMessages = + viewSessionIdRef.current === pending.sessionId + ? preserveLocalAssistantErrors(pending.state.messages, currentMessages) + : pending.state.messages if (!sameMessageList(nextMessages, currentMessages)) { setMessages(nextMessages) } + viewSessionIdRef.current = pending.sessionId + syncRuntimeMetadataToView(pending.state) setBusy(pending.state.busy) setMutableRef(busyRef, pending.state.busy) diff --git a/apps/desktop/src/app/settings/index.tsx b/apps/desktop/src/app/settings/index.tsx index 6c832799eb..ecf0f29377 100644 --- a/apps/desktop/src/app/settings/index.tsx +++ b/apps/desktop/src/app/settings/index.tsx @@ -228,7 +228,7 @@ export function SettingsView({ gateway, onClose, onConfigSaved, onMainModelChang onMainModelChanged={onMainModelChanged} /> ) : activeView === 'providers' ? ( - + ) : activeView === 'keys' ? ( ) : activeView === 'mcp' ? ( diff --git a/apps/desktop/src/app/settings/model-settings.test.tsx b/apps/desktop/src/app/settings/model-settings.test.tsx index a0b1afdc95..afe267b5fd 100644 --- a/apps/desktop/src/app/settings/model-settings.test.tsx +++ b/apps/desktop/src/app/settings/model-settings.test.tsx @@ -16,6 +16,8 @@ const getAuxiliaryModels = vi.fn() const setModelAssignment = vi.fn() const getRecommendedDefaultModel = vi.fn() const setEnvVar = vi.fn() +const getHermesConfigRecord = vi.fn() +const saveHermesConfig = vi.fn() const startManualProviderOAuth = vi.fn() vi.mock('@/hermes', () => ({ @@ -24,7 +26,9 @@ vi.mock('@/hermes', () => ({ getAuxiliaryModels: () => getAuxiliaryModels(), setModelAssignment: (body: unknown) => setModelAssignment(body), getRecommendedDefaultModel: (slug: string) => getRecommendedDefaultModel(slug), - setEnvVar: (key: string, value: string) => setEnvVar(key, value) + setEnvVar: (key: string, value: string) => setEnvVar(key, value), + getHermesConfigRecord: () => getHermesConfigRecord(), + saveHermesConfig: (config: unknown) => saveHermesConfig(config) })) vi.mock('@/store/onboarding', () => ({ @@ -35,7 +39,13 @@ beforeEach(() => { getGlobalModelInfo.mockResolvedValue({ provider: 'nous', model: 'hermes-4' }) getGlobalModelOptions.mockResolvedValue({ providers: [ - { name: 'Nous', slug: 'nous', models: ['hermes-4', 'hermes-4-mini'], authenticated: true }, + { + name: 'Nous', + slug: 'nous', + models: ['hermes-4', 'hermes-4-mini'], + authenticated: true, + capabilities: { 'hermes-4': { reasoning: true, fast: true } } + }, // An unconfigured api_key provider — surfaced by the full-universe payload. { name: 'DeepSeek', slug: 'deepseek', models: [], authenticated: false, auth_type: 'api_key', key_env: 'DEEPSEEK_API_KEY' } ] @@ -47,6 +57,8 @@ beforeEach(() => { setModelAssignment.mockResolvedValue({ provider: 'nous', model: 'hermes-4', gateway_tools: [] }) getRecommendedDefaultModel.mockResolvedValue({ provider: 'deepseek', model: 'deepseek-chat', free_tier: null }) setEnvVar.mockResolvedValue({ ok: true }) + getHermesConfigRecord.mockResolvedValue({ agent: { reasoning_effort: 'medium', service_tier: 'normal' } }) + saveHermesConfig.mockResolvedValue({ ok: true }) }) afterEach(() => { @@ -100,6 +112,31 @@ describe('ModelSettings', () => { await waitFor(() => expect(setEnvVar).toHaveBeenCalledWith('DEEPSEEK_API_KEY', 'sk-test-123')) }) + it('writes the profile default speed (service_tier) when the fast switch is toggled', async () => { + await renderModelSettings() + await waitFor(() => expect(getHermesConfigRecord).toHaveBeenCalled()) + + const fastSwitch = await screen.findByRole('switch') + fireEvent.click(fastSwitch) + + await waitFor(() => + expect(saveHermesConfig).toHaveBeenCalledWith( + expect.objectContaining({ agent: expect.objectContaining({ service_tier: 'fast' }) }) + ) + ) + }) + + it('hides the reasoning/speed defaults when the main model reports no capabilities', async () => { + getGlobalModelOptions.mockResolvedValueOnce({ + providers: [{ name: 'Nous', slug: 'nous', models: ['hermes-4'], authenticated: true, capabilities: { 'hermes-4': { reasoning: false, fast: false } } }] + }) + + await renderModelSettings() + await waitFor(() => expect(getHermesConfigRecord).toHaveBeenCalled()) + + expect(screen.queryByRole('switch')).toBeNull() + }) + it('renders the auxiliary task rows', async () => { await renderModelSettings() diff --git a/apps/desktop/src/app/settings/model-settings.tsx b/apps/desktop/src/app/settings/model-settings.tsx index 4402693f34..d8b29adc65 100644 --- a/apps/desktop/src/app/settings/model-settings.tsx +++ b/apps/desktop/src/app/settings/model-settings.tsx @@ -3,6 +3,7 @@ import { useCallback, useEffect, useMemo, useState } from 'react' import { Button } from '@/components/ui/button' import { Input } from '@/components/ui/input' import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@/components/ui/select' +import { Switch } from '@/components/ui/switch' import { getAuxiliaryModels, getGlobalModelInfo, @@ -10,6 +11,8 @@ import { getMoaModels, getRecommendedDefaultModel, saveMoaModels, + getHermesConfigRecord, + saveHermesConfig, setEnvVar, setModelAssignment } from '@/hermes' @@ -17,11 +20,26 @@ import type { AuxiliaryModelsResponse, MoaConfigResponse, MoaModelSlot, ModelOpt import { useI18n } from '@/i18n' import { AlertTriangle, Cpu, Loader2 } from '@/lib/icons' import { cn } from '@/lib/utils' +import { notifyError } from '@/store/notifications' import { startManualLocalEndpoint, startManualProviderOAuth } from '@/store/onboarding' +import type { HermesConfigRecord } from '@/types/hermes' import { CONTROL_TEXT } from './constants' +import { getNested, setNested } from './helpers' import { ListRow, LoadingState, Pill, SectionHeading } from './primitives' +// Hermes' reasoning levels (VALID_REASONING_EFFORTS); `none` = thinking off. +// Empty config = Hermes default (medium), shown as Medium. +const EFFORT_VALUES = ['none', 'minimal', 'low', 'medium', 'high', 'xhigh'] as const + +// agent.service_tier stores "fast"/"priority"/"on" for fast; anything else is +// normal (mirrors tui_gateway _load_service_tier). +const isFastTier = (tier: unknown): boolean => + ['fast', 'priority', 'on'].includes(String(tier ?? '').trim().toLowerCase()) + +// Reuse the composer's effort labels (`xhigh` shows as "Max", else 1:1). +const effortLabelKey = (v: string) => (v === 'xhigh' ? 'max' : v) as 'high' | 'low' | 'max' | 'medium' | 'minimal' + // A provider row is "ready" to pick a model from when it reports models. The // backend now surfaces the full `hermes model` universe (every canonical // provider), so unconfigured providers come back with `authenticated:false` @@ -102,6 +120,9 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) { const [moa, setMoa] = useState(null) const [selectedMoaPreset, setSelectedMoaPreset] = useState('') const [newMoaPresetName, setNewMoaPresetName] = useState('') + // Full profile config, kept so the reasoning/speed defaults round-trip + // (read agent.* → write back the whole record) like the generic config page. + const [config, setConfig] = useState(null) const [applying, setApplying] = useState(false) const [editingAuxTask, setEditingAuxTask] = useState(null) const [auxDraft, setAuxDraft] = useState<{ model: string; provider: string }>({ model: '', provider: '' }) @@ -118,11 +139,12 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) { setError('') try { - const [modelInfo, modelOptions, auxiliaryModels, moaModels] = await Promise.all([ + const [modelInfo, modelOptions, auxiliaryModels, moaModels, cfg] = await Promise.all([ getGlobalModelInfo(), getGlobalModelOptions(), getAuxiliaryModels(), - getMoaModels().catch(() => null) + getMoaModels().catch(() => null), + getHermesConfigRecord() ]) setMainModel({ model: modelInfo.model, provider: modelInfo.provider }) @@ -135,6 +157,7 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) { if (moaModels) { setSelectedMoaPreset(prev => prev && moaModels.presets[prev] ? prev : moaModels.default_preset) } + setConfig(cfg) } catch (err) { setError(err instanceof Error ? err.message : String(err)) } finally { @@ -248,6 +271,42 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) { .map(entry => ({ task: entry.task, provider: entry.provider, model: entry.model })) }, [auxiliary, mainModel]) + // Capabilities of the APPLIED main model — gates the profile-default + // reasoning/speed controls the same way the composer picker gates per-model + // edits (reasoning defaults on, fast defaults off when unreported). + const mainCaps = useMemo(() => { + const row = providers.find(provider => provider.slug === mainModel?.provider) + + return mainModel ? row?.capabilities?.[mainModel.model] : undefined + }, [providers, mainModel]) + + const reasoningSupported = mainCaps?.reasoning ?? true + const fastSupported = mainCaps?.fast ?? false + const effortValue = String(getNested(config ?? {}, 'agent.reasoning_effort') ?? '').trim().toLowerCase() || 'medium' + const fastOn = isFastTier(getNested(config ?? {}, 'agent.service_tier')) + + // Persist a single agent.* default by round-tripping the whole config record + // (PUT /api/config replaces it) — optimistic, with rollback on failure. + const writeAgentDefault = useCallback( + async (key: string, value: string) => { + if (!config) { + return + } + + const prev = config + const next = setNested(config, key, value) + setConfig(next) + + try { + await saveHermesConfig(next) + } catch (err) { + setConfig(prev) + notifyError(err, m.defaultsFailed) + } + }, + [config, m.defaultsFailed] + ) + // Paste an API key for the selected `api_key` provider, persist it, then // refresh so the now-authenticated provider's models populate. Auto-selects // the recommended default model so the user can Apply in one more click. @@ -500,6 +559,38 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) { : `${selectedProviderRow?.name} signs in through your browser — Hermes runs the flow for you.`}

)} + {config && mainModel && (reasoningSupported || fastSupported) && ( +
+ {m.defaultsLabel} + {reasoningSupported && ( +
+ {m.reasoning} + +
+ )} + {fastSupported && ( + + )} +
+ )} {error &&
{error}
} {switchStaleAux.length > 0 && (
diff --git a/apps/desktop/src/app/settings/providers-settings.test.tsx b/apps/desktop/src/app/settings/providers-settings.test.tsx index 8379d203f6..27c029b442 100644 --- a/apps/desktop/src/app/settings/providers-settings.test.tsx +++ b/apps/desktop/src/app/settings/providers-settings.test.tsx @@ -55,7 +55,7 @@ afterEach(() => { async function renderProvidersSettings() { const { ProvidersSettings } = await import('./providers-settings') - return render() + return render() } describe('ProvidersSettings', () => { @@ -95,6 +95,6 @@ describe('ProvidersSettings', () => { expect(await screen.findByText('Qwen Code')).toBeTruthy() expect(screen.queryByRole('button', { name: 'Remove Qwen Code' })).toBeNull() - expect(screen.getByText(/managed outside Hermes/)).toBeTruthy() + expect(screen.getByText(/managed by its own CLI/)).toBeTruthy() }) }) diff --git a/apps/desktop/src/app/settings/providers-settings.tsx b/apps/desktop/src/app/settings/providers-settings.tsx index f1132e6c33..2585e13995 100644 --- a/apps/desktop/src/app/settings/providers-settings.tsx +++ b/apps/desktop/src/app/settings/providers-settings.tsx @@ -1,6 +1,8 @@ import { useStore } from '@nanostores/react' +import type { ReactNode } from 'react' import { useCallback, useEffect, useMemo, useState } from 'react' +import { runInTerminal } from '@/app/right-sidebar/store' import { FEATURED_ID, FeaturedProviderRow, @@ -23,6 +25,20 @@ import { SettingsCategoryHeading, useEnvCredentials } from './env-credentials' import { providerGroup, providerMeta, providerPriority } from './helpers' import { LoadingState, SettingsContent } from './primitives' +// The embedded terminal (and thus the "run disconnect command" path) only +// exists in the Electron desktop shell, not the web dashboard. +const canRunInTerminal = () => typeof window !== 'undefined' && Boolean(window.hermesDesktop?.terminal) + +// Parallel group headers ("Connected", "Other providers") so the expanded list +// reads as its own section instead of bleeding into the connected group. +function GroupLabel({ children }: { children: ReactNode }) { + return ( +

+ {children} +

+ ) +} + // Sub-views surfaced as a sidebar subnav: account sign-in vs raw API keys. export const PROVIDER_VIEWS = ['accounts', 'keys'] as const @@ -90,11 +106,13 @@ function buildProviderKeyGroups(vars: Record): ProviderKeyGr function OAuthPicker({ disconnecting, onDisconnect, + onTerminalDisconnect, onWantApiKey, providers }: { disconnecting: null | string onDisconnect: (provider: OAuthProvider) => void + onTerminalDisconnect: (provider: OAuthProvider) => void onWantApiKey: () => void providers: OAuthProvider[] }) { @@ -138,15 +156,14 @@ function OAuthPicker({ {featured && } {connected.length > 0 && ( <> -

- {p.connected} -

+ {p.connected} {connected.map(p => ( ))} @@ -154,6 +171,7 @@ function OAuthPicker({ )} {showOthers && ( <> + {connected.length > 0 && {p.otherProviders}} {others.map(p => ( ))} @@ -180,21 +198,26 @@ function ConnectedProviderRow({ disconnecting, onDisconnect, onSelect, + onTerminalDisconnect, provider }: { disconnecting: boolean onDisconnect: (provider: OAuthProvider) => void onSelect: (provider: OAuthProvider) => void + onTerminalDisconnect: (provider: OAuthProvider) => void provider: OAuthProvider }) { const { t } = useI18n() + const copy = t.settings.providers const title = providerTitle(provider) const Trail = provider.flow === 'external' ? Terminal : ChevronRight + // Hermes can clear this provider's creds via the API. const canDisconnect = provider.disconnectable ?? provider.flow !== 'external' - - const disconnectHint = provider.flow === 'external' - ? t.settings.providers.removeExternal(title, provider.cli_command) - : t.settings.providers.removeKeyManaged(title) + // External (CLI-managed) provider Hermes can't clear via the API, but ships a + // command we can run in the embedded terminal (Electron shell only). + const terminalDisconnect = !canDisconnect && Boolean(provider.disconnect_command) && canRunInTerminal() + // Only fall back to a static "remove it elsewhere" hint when we offer no button. + const showHint = !canDisconnect && !terminalDisconnect return (
@@ -203,13 +226,13 @@ function ConnectedProviderRow({ {title} - {t.settings.providers.connected} + {copy.connected}

{t.onboarding.flowSubtitles[provider.flow]}

- {!canDisconnect && ( + {showHint && (

- {disconnectHint} + {provider.flow === 'external' ? copy.removeExternalGeneric(title) : copy.removeKeyManaged(title)}

)} @@ -228,6 +251,18 @@ function ConnectedProviderRow({ {disconnecting ? : } )} + {terminalDisconnect && ( + + )}
) @@ -243,7 +278,7 @@ function NoProviderKeys() { ) } -export function ProvidersSettings({ onViewChange, view }: ProvidersSettingsProps) { +export function ProvidersSettings({ onClose, onViewChange, view }: ProvidersSettingsProps) { const { t } = useI18n() const { rowProps, vars } = useEnvCredentials() const [oauthProviders, setOauthProviders] = useState([]) @@ -282,6 +317,29 @@ export function ProvidersSettings({ onViewChange, view }: ProvidersSettingsProps return () => void (cancelled = true) }, [onboardingActive]) + // External (CLI-managed) providers can't be cleared via the API by design — + // Hermes never deletes creds another tool owns behind a silent API call. + // Instead we run the documented removal command in the embedded terminal so + // the user sees exactly what executes, then return them to chat to watch it. + function handleTerminalDisconnect(provider: OAuthProvider) { + const command = provider.disconnect_command + + if (!command) { + return + } + + const name = providerTitle(provider) + + if (!window.confirm(t.settings.providers.removeTerminalConfirm(name, command))) { + return + } + + // Leave the settings overlay so the terminal pane (chat-only) is visible. + onClose() + runInTerminal(command) + notify({ kind: 'info', title: t.settings.providers.removedTitle, message: t.settings.providers.removeTerminalRunning(name) }) + } + async function handleDisconnect(provider: OAuthProvider) { const name = providerTitle(provider) @@ -341,6 +399,7 @@ export function ProvidersSettings({ onViewChange, view }: ProvidersSettingsProps void handleDisconnect(provider)} + onTerminalDisconnect={handleTerminalDisconnect} onWantApiKey={() => onViewChange('keys')} providers={oauthProviders} /> @@ -359,6 +418,7 @@ interface ProviderKeyGroup { } interface ProvidersSettingsProps { + onClose: () => void onViewChange: (view: ProviderView) => void view: ProviderView } diff --git a/apps/desktop/src/app/shell/app-shell.tsx b/apps/desktop/src/app/shell/app-shell.tsx index ade1f8a3c3..7cbcaacfb4 100644 --- a/apps/desktop/src/app/shell/app-shell.tsx +++ b/apps/desktop/src/app/shell/app-shell.tsx @@ -16,7 +16,7 @@ import { } from '@/store/layout' import { $paneWidthOverride } from '@/store/panes' import { $connection } from '@/store/session' -import { isNewSessionWindow, isSecondaryWindow } from '@/store/windows' +import { isSecondaryWindow } from '@/store/windows' import { SIDEBAR_COLLAPSE_MEDIA_QUERY } from '../layout-constants' @@ -80,7 +80,10 @@ export function AppShell({ const connection = useStore($connection) const viewportFullscreen = useSyncExternalStore(subscribeWindowSize, viewportIsFullscreen, () => false) const isFullscreen = Boolean(connection?.isFullscreen) || viewportFullscreen - const hideTitlebarControls = isNewSessionWindow() + // Every secondary window (new-session scratch, subagent watch, cmd-click + // pop-out) is a compact side panel — none of them carry the full titlebar + // tool cluster. Gate on isSecondaryWindow, never the narrower new-session flag. + const hideTitlebarControls = isSecondaryWindow() const titlebarControls = titlebarControlsPosition(connection?.windowButtonPosition, isFullscreen) // Width Windows/Linux reserve for the OS-painted min/max/close overlay (zero // on macOS, where window controls sit on the left and are reported via diff --git a/apps/desktop/src/app/shell/hooks/use-statusbar-items.tsx b/apps/desktop/src/app/shell/hooks/use-statusbar-items.tsx index 53ce2dcc15..b9a2d71545 100644 --- a/apps/desktop/src/app/shell/hooks/use-statusbar-items.tsx +++ b/apps/desktop/src/app/shell/hooks/use-statusbar-items.tsx @@ -1,5 +1,4 @@ import { useStore } from '@nanostores/react' -import type { ReactNode } from 'react' import { useCallback, useMemo } from 'react' import type { CommandCenterSection } from '@/app/command-center' @@ -9,7 +8,6 @@ import { useI18n } from '@/i18n' import { Activity, AlertCircle, - ChevronDown, Clock, Command, Hash, @@ -19,7 +17,6 @@ import { Zap, ZapFilled } from '@/lib/icons' -import { formatModelStatusLabel } from '@/lib/model-status-label' import type { RuntimeReadinessResult } from '@/lib/runtime-readiness' import { contextBarLabel, LiveDuration, usageContextLabel } from '@/lib/statusbar' import { cn } from '@/lib/utils' @@ -30,16 +27,11 @@ import { $activeSessionId, $busy, $connection, - $currentFastMode, - $currentModel, - $currentProvider, - $currentReasoningEffort, $currentUsage, $sessionStartedAt, $turnStartedAt, $workingSessionIds, $yoloActive, - setModelPickerOpen, setYoloActive } from '@/store/session' import { $subagentsBySession, activeSubagentCount } from '@/store/subagents' @@ -65,7 +57,6 @@ interface StatusbarItemsOptions { gatewayLogLines: readonly string[] gatewayState: string inferenceStatus: RuntimeReadinessResult | null - modelMenuContent?: ReactNode openAgents: () => void openCommandCenterSection: (section: CommandCenterSection) => void freshDraftReady: boolean @@ -83,7 +74,6 @@ export function useStatusbarItems({ gatewayLogLines, gatewayState, inferenceStatus, - modelMenuContent, openAgents, openCommandCenterSection, freshDraftReady, @@ -97,10 +87,6 @@ export function useStatusbarItems({ const terminalTakeover = useStore($terminalTakeover) const yoloActive = useStore($yoloActive) const busy = useStore($busy) - const currentFastMode = useStore($currentFastMode) - const currentModel = useStore($currentModel) - const currentProvider = useStore($currentProvider) - const currentReasoningEffort = useStore($currentReasoningEffort) const currentUsage = useStore($currentUsage) const desktopActionTasks = useStore($desktopActionTasks) const previewServerRestartStatus = useStore($previewServerRestartStatus) @@ -416,37 +402,6 @@ export function useStatusbarItems({ title: yoloActive ? copy.yoloOn : copy.yoloOff, variant: 'action' }, - { - id: 'model-summary', - label: ( - - - {formatModelStatusLabel(currentModel, { - fastMode: currentFastMode, - reasoningEffort: currentReasoningEffort - })} - - - - ), - ...(modelMenuContent - ? { - menuAlign: 'end' as const, - menuClassName: 'w-64', - menuContent: modelMenuContent, - title: currentProvider - ? copy.modelTitle(currentProvider, currentModel || copy.modelNone) - : copy.switchModel, - variant: 'menu' as const - } - : { - onSelect: () => setModelPickerOpen(true), - title: currentProvider - ? copy.providerModelTitle(currentProvider, currentModel || copy.noModel) - : copy.openModelPicker, - variant: 'action' as const - }) - }, { className: `w-7 justify-center px-0${terminalTakeover ? ' bg-accent/55 text-foreground' : ''}`, hidden: !chatOpen, @@ -465,11 +420,6 @@ export function useStatusbarItems({ contextBar, contextUsage, copy, - currentFastMode, - currentModel, - currentProvider, - currentReasoningEffort, - modelMenuContent, sessionStartedAt, showYoloToggle, terminalTakeover, diff --git a/apps/desktop/src/app/shell/model-edit-submenu.test.tsx b/apps/desktop/src/app/shell/model-edit-submenu.test.tsx new file mode 100644 index 0000000000..e2493c6002 --- /dev/null +++ b/apps/desktop/src/app/shell/model-edit-submenu.test.tsx @@ -0,0 +1,84 @@ +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' + +import { DropdownMenu, DropdownMenuContent, DropdownMenuSub, DropdownMenuSubTrigger } from '@/components/ui/dropdown-menu' +import { $modelPresets, getModelPreset } from '@/store/model-presets' +import { $activeSessionId } from '@/store/session' + +import { type FastControl, ModelEditSubmenu } from './model-edit-submenu' + +// Radix calls these on open; jsdom doesn't implement them. +beforeAll(() => { + Element.prototype.scrollIntoView = vi.fn() + Element.prototype.hasPointerCapture = vi.fn(() => false) + Element.prototype.releasePointerCapture = vi.fn() +}) + +beforeEach(() => { + $modelPresets.set({}) + $activeSessionId.set(null) +}) + +afterEach(() => { + cleanup() + vi.clearAllMocks() +}) + +// Render the submenu inside an open menu/sub so its content (switches) mounts. +function renderSubmenu(opts: { fastControl: FastControl; reasoning: boolean; requestGateway: () => Promise }) { + return render( + + + + edit + + + + + ) +} + +// Regression: editing the active row before a live session exists must stay +// preset-only — the gateway's config.set falls back to global config when no +// session matches, so it must not be called. (Caught in the second review.) +describe('ModelEditSubmenu no-session guard', () => { + it('param fast: records the preset but skips the gateway without a session', () => { + const requestGateway = vi.fn().mockResolvedValue({}) + renderSubmenu({ fastControl: { kind: 'param', on: false }, reasoning: false, requestGateway }) + + fireEvent.click(screen.getByRole('switch')) + + expect(getModelPreset('p1', 'm1').fast).toBe(true) + expect(requestGateway).not.toHaveBeenCalled() + }) + + it('reasoning: records the preset but skips the gateway without a session', () => { + const requestGateway = vi.fn().mockResolvedValue({}) + renderSubmenu({ fastControl: { kind: 'none' }, reasoning: true, requestGateway }) + + // Thinking starts on (medium); toggling it off routes through patchReasoning. + fireEvent.click(screen.getByRole('switch')) + + expect(getModelPreset('p1', 'm1').effort).toBe('none') + expect(requestGateway).not.toHaveBeenCalled() + }) + + it('param fast: pushes to the gateway once a session is active', async () => { + const requestGateway = vi.fn().mockResolvedValue({}) + $activeSessionId.set('sess1') + renderSubmenu({ fastControl: { kind: 'param', on: false }, reasoning: false, requestGateway }) + + fireEvent.click(screen.getByRole('switch')) + + expect(requestGateway).toHaveBeenCalledWith('config.set', { key: 'fast', session_id: 'sess1', value: 'fast' }) + }) +}) diff --git a/apps/desktop/src/app/shell/model-edit-submenu.tsx b/apps/desktop/src/app/shell/model-edit-submenu.tsx index 6872cca7f5..881e33cab0 100644 --- a/apps/desktop/src/app/shell/model-edit-submenu.tsx +++ b/apps/desktop/src/app/shell/model-edit-submenu.tsx @@ -12,13 +12,9 @@ import { } from '@/components/ui/dropdown-menu' import { Switch } from '@/components/ui/switch' import { useI18n } from '@/i18n' +import { setModelPreset } from '@/store/model-presets' import { notifyError } from '@/store/notifications' -import { - $activeSessionId, - $currentReasoningEffort, - setCurrentFastMode, - setCurrentReasoningEffort -} from '@/store/session' +import { $activeSessionId, setCurrentFastMode, setCurrentReasoningEffort } from '@/store/session' // Hermes' real reasoning levels (see VALID_REASONING_EFFORTS); `none` is owned // by the Thinking toggle, not the radio. @@ -76,96 +72,104 @@ export function resolveFastControl( } interface ModelEditSubmenuProps { + /** This row's effective reasoning effort (live for the active model, else its + * preset) — the submenu shows and edits from this, never the raw session. */ + effort: string /** How fast mode is offered for this model (param toggle vs. variant swap). */ fastControl: FastControl /** Whether this row's model is the active one. */ isActive: boolean - /** Switch to this model (resolves false on failure). Awaited before applying - * edits when not active so a failed switch doesn't write to the old model. */ - onActivate: () => Promise | void + /** This row's model id — edits persist as its global preset. */ + model: string /** Switch to a specific model id (used to swap base ⇄ -fast variant). */ onSelectModel: (model: string) => Promise | void + /** This row's provider slug — edits persist as its global preset. */ + provider: string /** Whether this model supports reasoning effort. */ reasoning: boolean requestGateway: (method: string, params?: Record) => Promise } export function ModelEditSubmenu({ + effort, fastControl, isActive, - onActivate, + model, onSelectModel, + provider, reasoning, requestGateway }: ModelEditSubmenuProps) { const { t } = useI18n() const copy = t.shell.modelOptions - // Reactive session state comes straight from the stores rather than being - // drilled through the panel, so editing it re-renders only this submenu. const activeSessionId = useStore($activeSessionId) - const currentReasoningEffort = useStore($currentReasoningEffort) - const effort = normalizeEffort(currentReasoningEffort) - const thinkingOn = isThinkingEnabled(currentReasoningEffort) + const effortValue = normalizeEffort(effort) + const thinkingOn = isThinkingEnabled(effort) - // Reasoning/fast are session-scoped (they apply to the active model), so - // editing a non-active model first switches to it. Returns false if the - // switch failed, so callers skip applying to the wrong (previous) model. - const ensureActive = async (): Promise => { - if (isActive) { - return true + // Editing always records the model's global preset; the active model also gets + // it pushed onto the live session. Non-active edits stay preset-only — they do + // not switch you to that model. + const patchReasoning = async (next: string) => { + setModelPreset(provider, model, { effort: next }) + + if (!isActive) { + return } - return (await onActivate()) !== false - } - - const patchReasoning = async (next: string, rollback: string) => { setCurrentReasoningEffort(next) + // Preset-only without a session: `isActive` holds for the global/default + // row pre-session, and the gateway's `config.set` falls back to global + // config when none matches — so don't reach it (preset + optimistic store + // are the whole effect). Same guard in applyModelPreset / toggleFast. + if (!activeSessionId) { + return + } + try { - if (!(await ensureActive())) { - setCurrentReasoningEffort(rollback) - - return - } - - await requestGateway('config.set', { - key: 'reasoning', - session_id: activeSessionId ?? '', - value: next - }) + await requestGateway('config.set', { key: 'reasoning', session_id: activeSessionId, value: next }) } catch (err) { - setCurrentReasoningEffort(rollback) + setCurrentReasoningEffort(effort) + setModelPreset(provider, model, { effort }) notifyError(err, copy.updateFailed) } } const toggleFast = (enabled: boolean) => { if (fastControl.kind === 'variant') { - // Fast is a separate model id — swap to it (or back to the base). - void onSelectModel(enabled ? fastControl.fastId : fastControl.baseId) + // Fast is a separate model id. Record the choice on the base model's + // preset (selectFamily picks the `-fast` sibling later when set), and + // only swap models now if this is the active row — inactive edits must + // stay preset-only, same as the param path below. + setModelPreset(provider, fastControl.baseId, { fast: enabled }) + + if (isActive) { + void onSelectModel(enabled ? fastControl.fastId : fastControl.baseId) + } return } if (fastControl.kind === 'param') { + setModelPreset(provider, model, { fast: enabled }) + + if (!isActive) { + return + } + setCurrentFastMode(enabled) + // Preset-only without a session (see patchReasoning). + if (!activeSessionId) { + return + } void (async () => { try { - if (!(await ensureActive())) { - setCurrentFastMode(!enabled) - - return - } - - await requestGateway('config.set', { - key: 'fast', - session_id: activeSessionId ?? '', - value: enabled ? 'fast' : 'normal' - }) + await requestGateway('config.set', { key: 'fast', session_id: activeSessionId, value: enabled ? 'fast' : 'normal' }) } catch (err) { setCurrentFastMode(!enabled) + setModelPreset(provider, model, { fast: !enabled }) notifyError(err, copy.fastFailed) } })() @@ -188,9 +192,7 @@ export function ModelEditSubmenu({ - void patchReasoning(checked ? effort || 'medium' : 'none', currentReasoningEffort) - } + onCheckedChange={checked => void patchReasoning(checked ? effortValue || 'medium' : 'none')} size="xs" /> @@ -205,10 +207,7 @@ export function ModelEditSubmenu({ <> {copy.effort} - void patchReasoning(value, currentReasoningEffort)} - value={effort} - > + void patchReasoning(value)} value={effortValue}> {EFFORT_OPTIONS.map(option => ( void>(() => {}) + interface ModelMenuPanelProps { gateway?: HermesGateway - onSelectModel: (selection: { model: string; persistGlobal: boolean; provider: string }) => Promise | void + onSelectModel: (selection: { model: string; provider: string }) => Promise | void requestGateway: (method: string, params?: Record) => Promise } @@ -54,6 +60,7 @@ interface ProviderGroup { export function ModelMenuPanel({ gateway, onSelectModel, requestGateway }: ModelMenuPanelProps) { const { t } = useI18n() const copy = t.shell.modelMenu + const closeMenu = useContext(ModelMenuCloseContext) const [search, setSearch] = useState('') const [activeMoaPreset, setActiveMoaPreset] = useState('') // Reactive session state is read from the stores here (not drilled in), so @@ -64,6 +71,7 @@ export function ModelMenuPanel({ gateway, onSelectModel, requestGateway }: Model const currentModel = useStore($currentModel) const currentProvider = useStore($currentProvider) const currentReasoningEffort = useStore($currentReasoningEffort) + const modelPresets = useStore($modelPresets) const visibleModels = useStore($visibleModels) const modelOptions = useQuery({ @@ -82,8 +90,12 @@ export function ModelMenuPanel({ gateway, onSelectModel, requestGateway }: Model queryFn: (): Promise => getMoaModels() }) - const optionsModel = String(modelOptions.data?.model ?? currentModel ?? '') - const optionsProvider = String(modelOptions.data?.provider ?? currentProvider ?? '') + const { model: optionsModel, provider: optionsProvider } = currentPickerSelection( + !!activeSessionId, + { model: currentModel, provider: currentProvider }, + modelOptions.data + ) + const loading = modelOptions.isPending && !modelOptions.data const error = modelOptions.error @@ -99,8 +111,35 @@ export function ModelMenuPanel({ gateway, onSelectModel, requestGateway }: Model [visibleModels, providers] ) - const switchTo = (model: string, provider: string) => - onSelectModel({ model, persistGlobal: !activeSessionId, provider }) + // The composer picker never persists the profile default. With a session it + // scopes the switch to that session; with none it's UI state shipped on the + // next session.create (see selectModel). The default lives in Settings → Model. + const switchTo = (model: string, provider: string) => onSelectModel({ model, provider }) + + // Selecting a model row restores that model's remembered preset onto the + // session (effort/fast), gated by capability. Unset → Hermes defaults. + const selectFamily = async (family: ModelFamily, provider: ModelOptionProvider) => { + const caps = provider.capabilities?.[family.id] + const preset = modelPresets[modelPresetKey(provider.slug, family.id)] ?? {} + + // Variant-fast models (no speed param) express "fast" as a separate `-fast` + // id, so honor the saved preset by selecting that sibling. Param-fast is + // applied via applyModelPreset below instead. + const variantFast = !(caps?.fast ?? false) && !!family.fastId + const targetId = variantFast && preset.fast === true ? family.fastId! : family.id + + if ((await switchTo(targetId, provider.slug)) === false) { + return + } + + await applyModelPreset( + { + effort: (caps?.reasoning ?? true) ? (preset.effort ?? 'medium') : undefined, + fast: (caps?.fast ?? false) ? (preset.fast ?? false) : undefined + }, + { failMessage: t.shell.modelOptions.updateFailed, request: requestGateway, sessionId: activeSessionId } + ) + } const toggleMoaPreset = async (preset: string) => { if (!activeSessionId) { @@ -168,37 +207,42 @@ export function ModelMenuPanel({ gateway, onSelectModel, requestGateway }: Model // -fast variant carries the same param support as its base. const caps = group.provider.capabilities?.[family.id] - // Single source of truth for the active row's fast state — keeps - // the row label in lock-step with the submenu's Fast toggle and - // handles the standalone `-fast` id case. + // Effective settings for this row: live session state when it's + // the active model, otherwise its remembered preset (Hermes + // defaults when unset). Row label AND submenu read from these so + // they never disagree. + const preset = modelPresets[modelPresetKey(group.provider.slug, family.id)] ?? {} + const effEffort = isCurrent ? currentReasoningEffort : preset.effort ?? '' + const effFast = isCurrent ? currentFastMode : preset.fast ?? false + const fastControl = resolveFastControl( activeId ?? family.id, group.provider.models ?? [], caps?.fast ?? false, - currentFastMode + effFast ) - // Grayed text is live session state only. Do not label inactive - // rows as "Fast" just because they have a fast-capable sibling: - // that makes an off Fast toggle look like it is already on. - const meta = isCurrent - ? [ - fastControl.kind !== 'none' && fastControl.on ? copy.fast : null, - reasoningEffortLabel(currentReasoningEffort) || copy.medium - ] - .filter(Boolean) - .join(' ') - : '' + const meta = [ + fastControl.kind !== 'none' && fastControl.on ? copy.fast : null, + (caps?.reasoning ?? true) ? reasoningEffortLabel(effEffort) || copy.medium : null + ] + .filter(Boolean) + .join(' ') // Every row is a hover-Edit submenu trigger. Activating it - // (pointer or keyboard) switches to the family's base model; - // the Fast toggle inside swaps to the -fast sibling (or flips - // the speed param). The sub-trigger has no `onSelect`, so wire - // both click and Enter/Space for keyboard parity. + // (pointer or keyboard) switches to the family's base model and + // restores its preset; the Fast toggle inside swaps to the -fast + // sibling (or flips the speed param). The sub-trigger has no + // `onSelect`, so wire both click and Enter/Space for keyboard parity. + // Clicking the row commits the model and closes the picker; the + // edit submenu (reasoning/fast) is reached by HOVER, so you can + // still tweak those without the click dismissing everything. const activate = () => { if (!isCurrent) { - void switchTo(family.id, group.provider.slug) + void selectFamily(family, group.provider) } + + closeMenu() } return ( @@ -220,10 +264,12 @@ export function ModelMenuPanel({ gateway, onSelectModel, requestGateway }: Model {isCurrent ? : null} switchTo(family.id, group.provider.slug)} + model={family.id} onSelectModel={nextModel => switchTo(nextModel, group.provider.slug)} + provider={group.provider.slug} reasoning={caps?.reasoning ?? true} requestGateway={requestGateway} /> diff --git a/apps/desktop/src/app/types.ts b/apps/desktop/src/app/types.ts index 5082b70406..9500468482 100644 --- a/apps/desktop/src/app/types.ts +++ b/apps/desktop/src/app/types.ts @@ -46,6 +46,12 @@ export interface SlashExecResponse { warning?: string } +export interface BrowserManageResponse { + connected?: boolean + url?: string + messages?: string[] +} + export interface SessionSteerResponse { // 'queued' == accepted into the live turn's steer slot (injected at the next // tool-result boundary); 'rejected' == no live tool window, caller queues. diff --git a/apps/desktop/src/components/assistant-ui/block-direction.test.tsx b/apps/desktop/src/components/assistant-ui/block-direction.test.tsx new file mode 100644 index 0000000000..a206e8e847 --- /dev/null +++ b/apps/desktop/src/components/assistant-ui/block-direction.test.tsx @@ -0,0 +1,129 @@ +// Lists and blockquotes have chrome beside the text (markers, the quote +// border) whose side is driven by the box's CSS direction, which the +// unicode-bidi:plaintext rules never touch. These tests pin the split of +// responsibilities: ul/ol/blockquote carry dir="auto" so the browser +// resolves their box direction from content, inline code carries dir="ltr" +// so it neither votes in that resolution nor reorders, and plain prose +// blocks stay attribute-free (the plaintext CSS owns them). jsdom does not +// resolve dir="auto", so the contract is asserted at the attribute level. +import { AssistantRuntimeProvider, type ThreadMessage, useExternalStoreRuntime } from '@assistant-ui/react' +import { render, screen } from '@testing-library/react' +import { describe, expect, it, vi } from 'vitest' + +import { Thread } from './thread' + +const createdAt = new Date('2026-06-01T00:00:00.000Z') + +class TestResizeObserver { + observe() {} + unobserve() {} + disconnect() {} +} + +vi.stubGlobal('ResizeObserver', TestResizeObserver) +vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => + window.setTimeout(() => callback(performance.now()), 0) +) +vi.stubGlobal('cancelAnimationFrame', (id: number) => window.clearTimeout(id)) + +Element.prototype.scrollTo = function scrollTo() {} + +function stubOffsetDimension( + prop: 'offsetHeight' | 'offsetWidth', + clientProp: 'clientHeight' | 'clientWidth', + fallback: number +) { + const previous = Object.getOwnPropertyDescriptor(HTMLElement.prototype, prop) + + Object.defineProperty(HTMLElement.prototype, prop, { + configurable: true, + get() { + return previous?.get?.call(this) || (this as HTMLElement)[clientProp] || fallback + } + }) +} + +stubOffsetDimension('offsetWidth', 'clientWidth', 800) +stubOffsetDimension('offsetHeight', 'clientHeight', 600) + +function userMessage(): ThreadMessage { + return { + id: 'user-1', + role: 'user', + content: [{ type: 'text', text: 'hi' }], + attachments: [], + createdAt, + metadata: { custom: {} } + } as ThreadMessage +} + +function assistantMessage(text: string): ThreadMessage { + return { + id: 'assistant-1', + role: 'assistant', + content: [{ type: 'text', text }], + status: { type: 'complete', reason: 'stop' }, + createdAt, + metadata: { + unstable_state: null, + unstable_annotations: [], + unstable_data: [], + steps: [], + custom: {} + } + } as ThreadMessage +} + +function Harness({ text }: { text: string }) { + const runtime = useExternalStoreRuntime({ + messages: [userMessage(), assistantMessage(text)], + isRunning: false, + onNew: async () => {} + }) + + return ( + + + + ) +} + +describe('block-level direction chrome', () => { + it('lists carry dir="auto" so markers follow the resolved direction', async () => { + render() + + const item = await screen.findByText(/חוף גורדון/) + + expect(item.closest('ol')?.getAttribute('dir')).toBe('auto') + + const bullet = await screen.findByText(/פריט/) + + expect(bullet.closest('ul')?.getAttribute('dir')).toBe('auto') + }) + + it('blockquotes carry dir="auto" so the border follows the resolved direction', async () => { + render( ציטוט קצר בעברית'} />) + + const quote = await screen.findByText(/ציטוט קצר/) + + expect(quote.closest('blockquote')?.getAttribute('dir')).toBe('auto') + }) + + it('inline code carries dir="ltr" so it does not vote in dir="auto" resolution', async () => { + render() + + const code = await screen.findByText('npm install') + + expect(code.tagName).toBe('CODE') + expect(code.getAttribute('dir')).toBe('ltr') + expect(code.closest('ol')?.getAttribute('dir')).toBe('auto') + }) + + it('plain prose blocks stay attribute-free (plaintext CSS owns them)', async () => { + render() + + const paragraph = await screen.findByText(/שלום לכולם/) + + expect(paragraph.closest('p')?.hasAttribute('dir')).toBe(false) + }) +}) diff --git a/apps/desktop/src/components/assistant-ui/directive-text.tsx b/apps/desktop/src/components/assistant-ui/directive-text.tsx index b870913b01..097b106281 100644 --- a/apps/desktop/src/components/assistant-ui/directive-text.tsx +++ b/apps/desktop/src/components/assistant-ui/directive-text.tsx @@ -322,13 +322,29 @@ function shortLabel(type: HermesRefType, id: string): string { return tail || id } +function safeEmbeddedImages(text: string) { + try { + return extractEmbeddedImages(text) + } catch { + return { cleanedText: text, images: [] as string[] } + } +} + +function safeDirectiveSegments(text: string): Unstable_DirectiveSegment[] { + try { + return [...hermesDirectiveFormatter.parse(text)] + } catch { + return [{ kind: 'text', text }] + } +} + /** * Renders text containing Hermes directives (`@file:...`, `@image:...`) as * inline chips. Embedded MEDIA images render below as a thumbnail row. */ export function DirectiveContent({ text }: { text: string }) { - const { cleanedText, images } = useMemo(() => extractEmbeddedImages(text ?? ''), [text]) - const segments = useMemo(() => hermesDirectiveFormatter.parse(cleanedText), [cleanedText]) + const { cleanedText, images } = useMemo(() => safeEmbeddedImages(text ?? ''), [text]) + const segments = useMemo(() => safeDirectiveSegments(cleanedText), [cleanedText]) return ( diff --git a/apps/desktop/src/components/assistant-ui/markdown-text.test.ts b/apps/desktop/src/components/assistant-ui/markdown-text.test.ts index fad9944741..b3ea416d06 100644 --- a/apps/desktop/src/components/assistant-ui/markdown-text.test.ts +++ b/apps/desktop/src/components/assistant-ui/markdown-text.test.ts @@ -201,4 +201,13 @@ describe('preprocessMarkdown', () => { expect(output).toContain('') }) + + it('handles a fenced block larger than V8 spread-argument limit', () => { + // A single huge code block (e.g. a logged minified bundle) used to throw + // `RangeError: Maximum call stack size exceeded` via `out.push(...lines)`. + const body = Array.from({ length: 200_000 }, (_, i) => `line ${i}`).join('\n') + const input = `\`\`\`js\n${body}\n\`\`\`` + + expect(() => preprocessMarkdown(input)).not.toThrow() + }) }) diff --git a/apps/desktop/src/components/assistant-ui/markdown-text.tsx b/apps/desktop/src/components/assistant-ui/markdown-text.tsx index 2c87f6d0c3..3da29aebbc 100644 --- a/apps/desktop/src/components/assistant-ui/markdown-text.tsx +++ b/apps/desktop/src/components/assistant-ui/markdown-text.tsx @@ -19,8 +19,9 @@ import { useState } from 'react' +import { ExpandableBlock } from '@/components/chat/expandable-block' import { PreviewAttachment } from '@/components/chat/preview-attachment' -import { SyntaxHighlighter } from '@/components/chat/shiki-highlighter' +import { chunkByLines, SyntaxHighlighter } from '@/components/chat/shiki-highlighter' import { ZoomableImage } from '@/components/chat/zoomable-image' import { normalizeExternalUrl, openExternalLink, PrettyLink } from '@/lib/external-link' import { createMemoizedMathPlugin } from '@/lib/katex-memo' @@ -57,7 +58,11 @@ const mathPlugin = createMemoizedMathPlugin({ singleDollarTextMath: true }) // flush) with a tail-bounded repair — see lib/remend-tail.ts. Must stay // module-scope so the prop identity is stable across renders. function preprocessWithTailRepair(text: string): string { - return tailBoundedRemend(preprocessMarkdown(text)) + try { + return tailBoundedRemend(preprocessMarkdown(text)) + } catch { + return text + } } // Memoized block splitter. Streamdown calls `parseMarkdownIntoBlocks` (a full @@ -453,8 +458,35 @@ const MARKDOWN_CONTAINER_CLASS_NAME = cn( '[&>*:first-child]:mt-0 [&>*:last-child]:mb-0 [&>*+*]:mt-(--paragraph-gap)' ) +const MAX_MARKDOWN_CHARS = 200_000 + +function HugeTextFallback({ containerClassName, text }: { containerClassName?: string; text: string }) { + const chunks = useMemo(() => chunkByLines(text, 200), [text]) + + return ( +
+ + {chunks.map((chunk, index) => ( +
+ {chunk.text} +
+ ))} + +
+ ) +} + function MarkdownTextSurface({ containerClassName, containerProps }: MarkdownTextSurfaceProps) { - const { status } = useMessagePartText() + const { status, text } = useMessagePartText() const isStreaming = status.type === 'running' // Keep code parsing enabled while streaming so incomplete fenced blocks still @@ -484,19 +516,37 @@ function MarkdownTextSurface({ containerClassName, containerProps }: MarkdownTex

), a: MarkdownLink, + // Inline code must not vote when an ancestor resolves `dir="auto"` + // (HTML's algorithm skips descendants that carry their own dir), + // mirroring the CSS isolate that already keeps it out of the + // plaintext scan. Fenced code never reaches this override; it goes + // through the code plugin's CodeCard path. + inlineCode: ({ className, ...props }: ComponentProps<'code'>) => ( + + ), // `---` as quiet spacing, not a heavy full-width rule. hr: (_props: ComponentProps<'hr'>) =>

, + // Lists and blockquotes have chrome that sits *beside* the text + // (markers, the quote border), and that side is driven by the CSS + // `direction` of the box, which `unicode-bidi: plaintext` never + // touches — an RTL list otherwise renders its numbers stranded at + // the far left. `dir="auto"` lets the browser resolve the box + // direction from content; the plaintext rules in styles.css keep + // owning per-line text direction. Inline code carries `dir="ltr"` + // (see the `code` override) so it doesn't vote here either, same + // contract as the CSS isolate. blockquote: ({ className, ...props }: ComponentProps<'blockquote'>) => (
), ul: ({ className, ...props }: ComponentProps<'ul'>) => ( -
    +
      ), ol: ({ className, ...props }: ComponentProps<'ol'>) => ( -
        +
          ), li: ({ className, ...props }: ComponentProps<'li'>) => (
        1. @@ -533,6 +583,10 @@ function MarkdownTextSurface({ containerClassName, containerProps }: MarkdownTex [isStreaming] ) + if (text.length > MAX_MARKDOWN_CHARS) { + return + } + return ( void }) { + const runtime = useExternalStoreRuntime({ + messages: [assistantErrorMessage('OpenRouter rejected the request (403).')], + isRunning: false, + onNew: async () => {} + }) + + return ( + + + + ) +} + describe('assistant-ui streaming renderer', () => { beforeEach(() => { resizeObservers.clear() @@ -421,6 +435,23 @@ describe('assistant-ui streaming renderer', () => { expect(screen.getByRole('alert').textContent).toContain('OpenRouter rejected the request (403).') }) + it('omits the dismiss control when no onDismissError handler is supplied', () => { + render() + + expect(screen.queryByRole('button', { name: 'Dismiss error' })).toBeNull() + }) + + it('invokes onDismissError with the errored message id when the dismiss control is clicked', () => { + const onDismissError = vi.fn() + render() + + const dismiss = screen.getByRole('button', { name: 'Dismiss error' }) + fireEvent.click(dismiss) + + expect(onDismissError).toHaveBeenCalledTimes(1) + expect(onDismissError).toHaveBeenCalledWith('assistant-error-1') + }) + // Scroll behavior (follow-at-bottom, escape-on-scroll-up, re-engage) is owned // by the use-stick-to-bottom library and covered by its own test suite. We // don't re-assert its scrollTop mechanics here — doing so in jsdom (no real diff --git a/apps/desktop/src/components/assistant-ui/thread-list.tsx b/apps/desktop/src/components/assistant-ui/thread-list.tsx index 0a4be83961..8c98b88a59 100644 --- a/apps/desktop/src/components/assistant-ui/thread-list.tsx +++ b/apps/desktop/src/components/assistant-ui/thread-list.tsx @@ -22,7 +22,7 @@ import { resetThreadScroll, setThreadAtBottom } from '@/store/thread-scroll' -import { isNewSessionWindow, isSecondaryWindow } from '@/store/windows' +import { isSecondaryWindow } from '@/store/windows' import { MessageRenderBoundary } from './message-render-boundary' @@ -134,13 +134,20 @@ const ThreadMessageListInner: FC = ({ const hiddenCount = firstVisible const visibleGroups = hiddenCount > 0 ? groups.slice(hiddenCount) : groups const restoreFromBottomRef = useRef(null) - const newSessionWindow = isNewSessionWindow() - const newSessionTitlebarGap = 'calc(var(--titlebar-height)+0.75rem)' - const threadContentTopPad = newSessionWindow + // Secondary windows (new-session scratch, subagent watch, cmd-click pop-out) + // hide the titlebar tool cluster + session header, but the OS traffic lights + // still sit in the top-left, so reserve the titlebar gap above the transcript. + const secondaryWindow = isSecondaryWindow() + // NB: CSS calc() requires whitespace around the +/- operator. This string is + // assigned verbatim to the --sticky-human-top inline style below (it does not + // go through Tailwind, which would auto-space it), so the spaces are load- + // bearing — without them the declaration is invalid, gets dropped, and the + // sticky user bubble falls back to its ~4px default and slides under the OS + // traffic lights. + const secondaryTitlebarGap = 'calc(var(--titlebar-height) + 0.75rem)' + const threadContentTopPad = secondaryWindow ? 'pt-[calc(var(--titlebar-height)+0.75rem)]' - : isSecondaryWindow() - ? 'pt-6' - : 'pt-[calc(var(--titlebar-height)+1.5rem)]' + : 'pt-[calc(var(--titlebar-height)-0.5rem)]' useEffect(() => setThreadAtBottom(isAtBottom), [isAtBottom]) useEffect(() => () => resetThreadScroll(), []) @@ -247,10 +254,21 @@ const ThreadMessageListInner: FC = ({ style={ { height: clampToComposer ? 'var(--thread-viewport-height)' : '100%', - ...(newSessionWindow ? { '--sticky-human-top': newSessionTitlebarGap } : {}) + ...(secondaryWindow ? { '--sticky-human-top': secondaryTitlebarGap } : {}) } as CSSProperties } > + {secondaryWindow && ( + // Secondary windows hide the titlebar chrome, so the scroller runs to + // the window's top edge and streamed text slides up under the OS + // traffic lights. Content padding alone scrolls away with the text — a + // fixed opaque strip (the titlebar's drag region) masks anything behind + // it and keeps the window draggable, matching the main window's header. +