diff --git a/.gitignore b/.gitignore index 6d87318e3..489453d79 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ *.pyc* __pycache__/ .venv/ +.venv .vscode/ .env .env.local diff --git a/Dockerfile b/Dockerfile index be358ac53..b4ebd0936 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 14311d8c0..4f2e3f138 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 diff --git a/agent/anthropic_adapter.py b/agent/anthropic_adapter.py index 3a2d3f68e..4a586d7f0 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/codex_responses_adapter.py b/agent/codex_responses_adapter.py index b8479141d..e9b6ace9b 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 d5469a1b3..318e67d0f 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 45722d265..ef69ac683 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -474,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, ) -> Dict[str, Any]: """ Run a complete conversation with tool calling until completion. @@ -489,6 +490,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: @@ -510,6 +513,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, @@ -3752,8 +3756,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 62630ce45..0ceebecbf 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 944886d72..ddf8699e9 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 240595a4e..dcd50a299 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 e31fcdea4..4493eae5f 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 b11cade39..bbae3c9a7 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 @@ -957,6 +958,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 +1538,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 +1599,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 +1626,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 +1645,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 +1664,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 +1700,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 +1720,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 +1737,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 269c2fdd2..18264c44b 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 6f68d3041..9f16534a4 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 76f57dfcd..b3f39123f 100644 --- a/agent/system_prompt.py +++ b/agent/system_prompt.py @@ -40,6 +40,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 +83,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 +102,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 @@ -333,7 +345,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 +413,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/transports/anthropic.py b/agent/transports/anthropic.py index aad491613..98721f7c5 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 1d24ac335..1ce449eea 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 e94d43279..8041eabdb 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 40d136f96..a42838293 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 19096c613..c8e31becf 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) { diff --git a/apps/desktop/electron/session-windows.cjs b/apps/desktop/electron/session-windows.cjs index 929bf3ea9..5e2f3d4c6 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 8261809db..78f19b859 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 000000000..ec8a94831 --- /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 000000000..623effa4d --- /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 08080188a..70d35fb7b 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 38315b9c6..b88c28121 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 000000000..100d6c346 --- /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/index.tsx b/apps/desktop/src/app/chat/index.tsx index 63983caaa..4ae3817c8 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' @@ -86,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 { @@ -121,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 } @@ -272,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) @@ -296,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) @@ -315,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({ @@ -432,6 +453,7 @@ export function ChatView({ loading={threadLoading} onBranchInNewChat={onBranchInNewChat} onCancel={onCancel} + onDismissError={onDismissError} onRestoreToMessage={onRestoreToMessage} sessionId={activeSessionId} sessionKey={threadKey} @@ -465,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 45251ceef..05dfbbc76 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) @@ -736,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() @@ -845,6 +893,8 @@ export function DesktopController() { gatewayState, locationPathname: location.pathname, resumeSession, + resumeFailedSessionId, + resumeExhaustedSessionId, routedSessionId, runtimeIdByStoredSessionIdRef, selectedStoredSessionId, @@ -994,6 +1044,7 @@ export function DesktopController() { void removeSession(selectedStoredSessionId) } }} + onDismissError={dismissError} onEdit={editMessage} onPasteClipboardImage={() => void composer.pasteClipboardImage()} onPickFiles={() => void composer.pickContextPaths('file')} @@ -1002,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/session/hooks/use-message-stream.ts b/apps/desktop/src/app/session/hooks/use-message-stream.ts index c07222c68..3ee52ec8e 100644 --- a/apps/desktop/src/app/session/hooks/use-message-stream.ts +++ b/apps/desktop/src/app/session/hooks/use-message-stream.ts @@ -1102,8 +1102,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-route-resume.test.tsx b/apps/desktop/src/app/session/hooks/use-route-resume.test.tsx index e0d984c37..e05f8b748 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 ad7677cc4..1be8da90c 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 739e8b937..a84a854de 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 6f7a779e8..36dfea759 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions.ts +++ b/apps/desktop/src/app/session/hooks/use-session-actions.ts @@ -38,6 +38,8 @@ import { setFreshDraftReady, setIntroSeed, setMessages, + setResumeExhaustedSessionId, + setResumeFailedSessionId, setSelectedStoredSessionId, setSessions, setSessionStartedAt, @@ -579,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) @@ -769,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 e2a973582..681334aa2 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 a08eb1f16..1445dd17a 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/model-settings.test.tsx b/apps/desktop/src/app/settings/model-settings.test.tsx index a0b1afdc9..afe267b5f 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 c55fa6b47..e88938def 100644 --- a/apps/desktop/src/app/settings/model-settings.tsx +++ b/apps/desktop/src/app/settings/model-settings.tsx @@ -3,11 +3,14 @@ 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, getGlobalModelOptions, + getHermesConfigRecord, getRecommendedDefaultModel, + saveHermesConfig, setEnvVar, setModelAssignment } from '@/hermes' @@ -15,11 +18,26 @@ import type { AuxiliaryModelsResponse, ModelOptionProvider, StaleAuxAssignment } 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` @@ -97,6 +115,9 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) { const [selectedProvider, setSelectedProvider] = useState('') const [selectedModel, setSelectedModel] = useState('') const [auxiliary, setAuxiliary] = useState(null) + // 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: '' }) @@ -113,10 +134,11 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) { setError('') try { - const [modelInfo, modelOptions, auxiliaryModels] = await Promise.all([ + const [modelInfo, modelOptions, auxiliaryModels, cfg] = await Promise.all([ getGlobalModelInfo(), getGlobalModelOptions(), - getAuxiliaryModels() + getAuxiliaryModels(), + getHermesConfigRecord() ]) setMainModel({ model: modelInfo.model, provider: modelInfo.provider }) @@ -124,6 +146,7 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) { setSelectedProvider(prev => prev || modelInfo.provider) setSelectedModel(prev => prev || modelInfo.model) setAuxiliary(auxiliaryModels) + setConfig(cfg) } catch (err) { setError(err instanceof Error ? err.message : String(err)) } finally { @@ -181,6 +204,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. @@ -433,6 +492,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/shell/app-shell.tsx b/apps/desktop/src/app/shell/app-shell.tsx index ade1f8a3c..7cbcaacfb 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/model-menu-panel.tsx b/apps/desktop/src/app/shell/model-menu-panel.tsx index a9795564a..c3d20ebd8 100644 --- a/apps/desktop/src/app/shell/model-menu-panel.tsx +++ b/apps/desktop/src/app/shell/model-menu-panel.tsx @@ -18,7 +18,7 @@ import { Skeleton } from '@/components/ui/skeleton' import type { HermesGateway } from '@/hermes' import { getGlobalModelOptions } from '@/hermes' import { useI18n } from '@/i18n' -import { displayModelName, modelDisplayParts, reasoningEffortLabel } from '@/lib/model-status-label' +import { currentPickerSelection, displayModelName, modelDisplayParts, reasoningEffortLabel } from '@/lib/model-status-label' import { cn } from '@/lib/utils' import { $modelPresets, applyModelPreset, modelPresetKey } from '@/store/model-presets' import { @@ -84,8 +84,12 @@ export function ModelMenuPanel({ gateway, onSelectModel, requestGateway }: Model } }) - 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 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 000000000..a206e8e84 --- /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 b870913b0..097b10628 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 fad994474..b3ea416d0 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 2c87f6d0c..3da29aebb 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 e3faf6454..8c98b88a5 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)-0.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. +