diff --git a/agent/anthropic_adapter.py b/agent/anthropic_adapter.py index d9bbe2d8e3a8..a4a211843eed 100644 --- a/agent/anthropic_adapter.py +++ b/agent/anthropic_adapter.py @@ -1783,11 +1783,25 @@ def _strip_orphaned_tool_blocks(result: List[Dict[str, Any]]) -> None: tool_result_ids.add(block.get("tool_use_id")) for m in result: if m["role"] == "assistant" and isinstance(m["content"], list): - m["content"] = [ + kept = [ b for b in m["content"] if b.get("type") != "tool_use" or b.get("id") in tool_result_ids ] + # If stripping an orphaned tool_use mutated a turn that also carries a + # signed thinking block, that block's Anthropic signature was computed + # against the ORIGINAL (un-stripped) turn content and is now invalid. + # Anthropic rejects the replayed turn with HTTP 400 "thinking blocks in + # the latest assistant message cannot be modified". Flag the turn so + # _manage_thinking_signatures can demote the dead signature instead of + # replaying it verbatim. See hermes-agent: extended-thinking + parallel + # tool batch interrupted mid-flight → non-retryable 400 crash-loop. + if len(kept) != len(m["content"]) and any( + isinstance(b, dict) and b.get("type") in {"thinking", "redacted_thinking"} + for b in m["content"] + ): + m["_thinking_signature_invalidated"] = True + m["content"] = kept if not m["content"]: m["content"] = [{"type": "text", "text": "(tool call removed)"}] @@ -1832,6 +1846,10 @@ def _merge_consecutive_roles(result: List[Dict[str, Any]]) -> List[Dict[str, Any fixed[-1]["content"] = prev_content + curr_content else: # Consecutive assistant messages — merge text content. + # Propagate the orphan-strip signature-invalidation flag onto the + # surviving (prev) dict so _manage_thinking_signatures still sees it. + if m.get("_thinking_signature_invalidated"): + fixed[-1]["_thinking_signature_invalidated"] = True # Drop thinking blocks from the *second* message: their # signature was computed against a different turn boundary # and becomes invalid once merged. @@ -1920,11 +1938,26 @@ def _manage_thinking_signatures( else: # Latest assistant on direct Anthropic: keep signed, downgrade unsigned # to text so the reasoning isn't lost. + # + # Exception: if orphan-stripping (or another structural mutation) removed + # a tool_use block from THIS turn, every thinking signature on it was + # computed against the original turn content and is now dead. Anthropic + # rejects the turn either way — replaying the signed block 400s with + # "thinking blocks in the latest assistant message cannot be modified", + # and a bare signed block with no following tool_use is also invalid. + # Demote ALL thinking blocks on this turn to text so the turn replays + # cleanly and the model can re-plan from the surviving tool results. + signature_dead = bool(m.get("_thinking_signature_invalidated")) new_content = [] for b in m["content"]: if not isinstance(b, dict) or b.get("type") not in _THINKING_TYPES: new_content.append(b) continue + if signature_dead: + thinking_text = b.get("thinking", "") + if thinking_text: + new_content.append({"type": "text", "text": thinking_text}) + continue if b.get("type") == "redacted_thinking": # Redacted blocks use 'data' for the signature payload — # drop the block when 'data' is missing (can't be validated). @@ -1944,6 +1977,9 @@ def _manage_thinking_signatures( if isinstance(b, dict) and b.get("type") in _THINKING_TYPES: b.pop("cache_control", None) + # Drop the internal bookkeeping flag — it must never reach the API payload. + m.pop("_thinking_signature_invalidated", None) + def _evict_old_screenshots(result: List[Dict[str, Any]]) -> None: """Keep only the most recent ``_MAX_KEEP_IMAGES`` computer-use screenshots. diff --git a/agent/background_review.py b/agent/background_review.py index bf99ee528458..9125e208c773 100644 --- a/agent/background_review.py +++ b/agent/background_review.py @@ -67,6 +67,12 @@ "from. Capture it.\n" " • A skill that got loaded or consulted this session turned out " "to be wrong, missing a step, or outdated. Patch it NOW.\n\n" + "Evidence discipline: do not encode the foreground assistant's own " + "unverified diagnosis as a durable rule. For operational/debugging " + "lessons, prefer timestamp-matched tool output, logs, tests, or a user " + "correction. If the turn ended with an error sentinel, malformed final " + "response, interrupted stream, or retry/fallback exhaustion, capture the " + "recovery pattern only, not the failed hypothesis.\n\n" "Preference order — prefer the earliest action that fits, but do " "pick one when a signal above fired:\n" " 1. UPDATE A CURRENTLY-LOADED SKILL. Look back through the " @@ -169,6 +175,12 @@ "emerged.\n" " • A skill that was loaded or consulted turned out wrong, " "missing, or outdated — patch it now.\n\n" + "Evidence discipline: do not encode the foreground assistant's own " + "unverified diagnosis as a durable rule. For operational/debugging " + "lessons, prefer timestamp-matched tool output, logs, tests, or a user " + "correction. If the turn ended with an error sentinel, malformed final " + "response, interrupted stream, or retry/fallback exhaustion, capture the " + "recovery pattern only, not the failed hypothesis.\n\n" "Preference order for skills — pick the earliest that fits:\n" " 1. UPDATE A CURRENTLY-LOADED SKILL. Check what skills were " "loaded via /skill-name or skill_view in the conversation. If one " diff --git a/agent/conversation_compression.py b/agent/conversation_compression.py index ba8678cc7231..a0b077e3e589 100644 --- a/agent/conversation_compression.py +++ b/agent/conversation_compression.py @@ -644,6 +644,12 @@ def try_shrink_image_parts_in_messages(api_messages: list) -> bool: # after a confirmed provider rejection, so the alternative is failure. target_bytes = 4 * 1024 * 1024 changed_count = 0 + # Track parts that are over the target but could NOT be shrunk under it. + # If any survive, retrying is pointless — the same oversized payload will + # be re-sent and rejected again, wasting the single retry budget. We only + # report success (caller retries) when every over-threshold image was + # actually brought under the target. + unshrinkable_oversized = 0 def _shrink_data_url(url: str) -> Optional[str]: """Return a smaller data URL, or None if shrink can't help.""" @@ -710,17 +716,34 @@ def _shrink_data_url(url: str) -> Optional[str]: if resized: image_value["url"] = resized changed_count += 1 + elif isinstance(url, str) and url.startswith("data:") \ + and len(url) > target_bytes: + unshrinkable_oversized += 1 elif isinstance(image_value, str): resized = _shrink_data_url(image_value) if resized: part["image_url"] = resized changed_count += 1 + elif image_value.startswith("data:") \ + and len(image_value) > target_bytes: + unshrinkable_oversized += 1 if changed_count: logger.info( "image-shrink recovery: re-encoded %d image part(s) to fit under %.0f MB", changed_count, target_bytes / (1024 * 1024), ) + if unshrinkable_oversized: + # At least one oversized image could not be shrunk under the target. + # Retrying would re-send it and fail identically, so signal "no + # progress" even if other parts shrank — the caller will surface the + # original error rather than burning its single retry on a no-op. + logger.warning( + "image-shrink recovery: %d oversized image part(s) could not be " + "shrunk under %.0f MB — not retrying (would re-send rejected payload)", + unshrinkable_oversized, target_bytes / (1024 * 1024), + ) + return False return changed_count > 0 diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index bb6c6229cdb7..d7bde8e5fba3 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -444,6 +444,7 @@ def run_conversation( agent._incomplete_scratchpad_retries = 0 agent._codex_incomplete_retries = 0 agent._thinking_prefill_retries = 0 + agent._malformed_final_retries = 0 agent._post_tool_empty_retried = False agent._last_content_with_tools = None agent._last_content_tools_all_housekeeping = False @@ -672,6 +673,7 @@ def run_conversation( # context loss. agent._empty_content_retries = 0 agent._thinking_prefill_retries = 0 + agent._malformed_final_retries = 0 agent._last_content_with_tools = None agent._last_content_tools_all_housekeeping = False agent._mute_post_response = False @@ -3817,6 +3819,7 @@ def _stop_spinner(): if _had_prefill: agent._thinking_prefill_retries = 0 agent._empty_content_retries = 0 + agent._malformed_final_retries = 0 # Successful tool execution — reset the post-tool nudge # flag so it can fire again if the model goes empty on # a LATER tool round. @@ -4209,6 +4212,76 @@ def _stop_spinner(): # Reset retry counter/signature on successful content agent._empty_content_retries = 0 agent._thinking_prefill_retries = 0 + + malformed_final_reason = agent._detect_malformed_tool_final_response( + final_response, + finish_reason, + messages, + ) + if malformed_final_reason: + agent._malformed_final_retries += 1 + logger.warning( + "Malformed final response after tool calls (%s) — " + "recovery attempt %d (model=%s provider=%s)", + malformed_final_reason, + agent._malformed_final_retries, + agent.model, + agent.provider, + ) + agent._buffer_status( + "⚠️ Model returned a malformed final response after " + f"tool calls — retrying ({agent._malformed_final_retries}/2)" + ) + if agent._malformed_final_retries == 1: + recovery_msg = agent._build_assistant_message( + assistant_message, + finish_reason, + ) + recovery_msg["content"] = "[malformed final response omitted]" + recovery_msg["_malformed_final_recovery_synthetic"] = True + messages.append(recovery_msg) + messages.append({ + "role": "user", + "content": ( + "The previous final response was malformed. " + "Regenerate a concise, complete final answer " + "from the tool results above. Do not repeat " + "punctuation or stop mid-word." + ), + "_malformed_final_recovery_synthetic": True, + }) + agent._session_messages = messages + continue + + if agent._fallback_chain: + agent._buffer_status( + "⚠️ Malformed final response repeated — " + "switching to fallback provider..." + ) + if agent._try_activate_fallback(): + agent._buffer_status( + f"↻ Switched to fallback: {agent.model} " + f"({agent.provider})" + ) + continue + + agent._flush_status_buffer() + _turn_exit_reason = "malformed_final_exhausted" + assistant_msg = agent._build_assistant_message( + assistant_message, + finish_reason, + ) + assistant_msg["content"] = "(malformed final response)" + assistant_msg["_malformed_final_recovery_synthetic"] = True + messages.append(assistant_msg) + final_response = ( + "Model returned a malformed final response after " + "tool calls and recovery was exhausted. Try again " + "or switch providers." + ) + break + + agent._malformed_final_retries = 0 # Successful content reached — drop any buffered retry # status from earlier failed attempts in this turn. agent._clear_status_buffer() @@ -4261,6 +4334,7 @@ def _stop_spinner(): messages[-1].get("_thinking_prefill") or messages[-1].get("_empty_recovery_synthetic") or messages[-1].get("_empty_terminal_sentinel") + or messages[-1].get("_malformed_final_recovery_synthetic") ) ): messages.pop() diff --git a/cli.py b/cli.py index baf033920a15..e6bd7167435f 100644 --- a/cli.py +++ b/cli.py @@ -3577,8 +3577,17 @@ def _get_status_bar_snapshot(self) -> Dict[str, Any]: compressor = getattr(agent, "context_compressor", None) if compressor: + # last_prompt_tokens is parked at the -1 sentinel right after a + # compression, until the next real API call reports a prompt count + # (awaiting_real_usage_after_compression). The status bar must not + # render that sentinel verbatim — it produced "-1/200K" / "-1%". + # Clamp it to 0 so the one transitional turn reads as empty context. context_tokens = getattr(compressor, "last_prompt_tokens", 0) or 0 + if context_tokens < 0: + context_tokens = 0 context_length = getattr(compressor, "context_length", 0) or 0 + if context_length < 0: + context_length = 0 snapshot["context_tokens"] = context_tokens snapshot["context_length"] = context_length or None snapshot["compressions"] = getattr(compressor, "compression_count", 0) or 0 @@ -15074,6 +15083,96 @@ def new_event_loop(self): # Main Entry Point # ============================================================================ +def _run_kanban_goal_loop_q(cli: "HermesCLI", first_response: str) -> None: + """Drive a kanban goal_mode worker through the Ralph-style goal loop. + + Called from the quiet single-query path AFTER the worker's first turn, + only when ``HERMES_KANBAN_GOAL_MODE`` is set (dispatcher-spawned + goal_mode card). Wires the worker's ``run_conversation`` and the kanban + DB into ``goals.run_kanban_goal_loop``. All errors are swallowed by the + caller — a broken goal loop must never wedge a worker, the dispatcher's + claim TTL / crash detection is the backstop. + """ + import os as _os + + task_id = (_os.environ.get("HERMES_KANBAN_TASK") or "").strip() + if not task_id: + return + + from hermes_cli import kanban_db as _kb + from hermes_cli.goals import run_kanban_goal_loop as _run_loop, DEFAULT_MAX_TURNS as _DEF_TURNS + + # Resolve goal text from the card (title + body = the acceptance + # criteria the judge evaluates against). + conn = _kb.connect() + try: + task = _kb.get_task(conn, task_id) + finally: + try: + conn.close() + except Exception: + pass + if task is None: + return + + goal_parts = [task.title or ""] + if task.body: + goal_parts.append(task.body) + goal_text = "\n\n".join(p for p in goal_parts if p).strip() + if not goal_text: + return + + max_turns = task.goal_max_turns or _DEF_TURNS + + def _run_turn(prompt: str) -> str: + result = cli.agent.run_conversation( + user_message=prompt, + conversation_history=cli.conversation_history, + ) + # Keep session_id in sync if mid-run compression rotated it. + if ( + getattr(cli.agent, "session_id", None) + and cli.agent.session_id != cli.session_id + ): + cli.session_id = cli.agent.session_id + resp = result.get("final_response", "") if isinstance(result, dict) else str(result) + if resp: + print(resp) + return resp or "" + + def _task_status() -> "str | None": + c = _kb.connect() + try: + t = _kb.get_task(c, task_id) + return t.status if t is not None else None + finally: + try: + c.close() + except Exception: + pass + + def _block(reason: str) -> None: + c = _kb.connect() + try: + _kb.block_task(c, task_id, reason=reason) + finally: + try: + c.close() + except Exception: + pass + + _run_loop( + task_id=task_id, + goal_text=goal_text, + run_turn=_run_turn, + task_status_fn=_task_status, + block_fn=_block, + max_turns=max_turns, + first_response=first_response or "", + log=lambda m: logger.info("%s", m), + ) + + def main( query: str = None, q: str = None, @@ -15471,6 +15570,20 @@ def _signal_handler_q(signum, frame): print(f"Error: {result['error']}", file=sys.stderr) elif response: print(response) + + # Kanban goal-loop mode: a worker spawned for a + # goal_mode card keeps working in THIS session until an + # auxiliary judge agrees the card is done, the worker + # terminates the task itself, or the turn budget runs + # out (→ sticky block). Gated on the env vars the + # dispatcher sets in `_default_spawn`; a no-op for every + # normal worker and every non-kanban `-q` run. + if os.environ.get("HERMES_KANBAN_GOAL_MODE") == "1": + try: + _run_kanban_goal_loop_q(cli, response) + except Exception as _goal_exc: + logger.debug("kanban goal loop failed: %s", _goal_exc) + # Session ID goes to stderr so piped stdout is clean. print(f"\nsession_id: {cli.session_id}", file=sys.stderr) diff --git a/gateway/platforms/bluebubbles.py b/gateway/platforms/bluebubbles.py index ec852e3d6107..2fc4102b6666 100644 --- a/gateway/platforms/bluebubbles.py +++ b/gateway/platforms/bluebubbles.py @@ -14,6 +14,7 @@ import os import re import uuid +from collections import OrderedDict from datetime import datetime from typing import Any, Dict, List, Optional from urllib.parse import quote @@ -60,6 +61,8 @@ _PHONE_RE = re.compile(r"\+?\d{7,15}") _EMAIL_RE = re.compile(r"[\w.+-]+@[\w-]+\.[\w.]+") +_GUID_CACHE_SIZE = 500 # LRU cap for resolved chat-GUID lookups + def _redact(text: str) -> str: """Redact phone numbers and emails from log output.""" @@ -128,7 +131,7 @@ def __init__(self, config: PlatformConfig): self._runner = None self._private_api_enabled: Optional[bool] = None self._helper_connected: bool = False - self._guid_cache: Dict[str, str] = {} + self._guid_cache: OrderedDict[str, str] = OrderedDict() # ------------------------------------------------------------------ # API helpers @@ -365,6 +368,7 @@ async def _resolve_chat_guid(self, target: str) -> Optional[str]: if ";" in target: return target if target in self._guid_cache: + self._guid_cache.move_to_end(target) return self._guid_cache[target] try: payload = await self._api_post( @@ -377,10 +381,14 @@ async def _resolve_chat_guid(self, target: str) -> Optional[str]: if identifier == target: if guid: self._guid_cache[target] = guid + while len(self._guid_cache) > _GUID_CACHE_SIZE: + self._guid_cache.popitem(last=False) return guid for part in chat.get("participants", []) or []: if (part.get("address") or "").strip() == target and guid: self._guid_cache[target] = guid + while len(self._guid_cache) > _GUID_CACHE_SIZE: + self._guid_cache.popitem(last=False) return guid except Exception: pass diff --git a/gateway/platforms/feishu.py b/gateway/platforms/feishu.py index 10ddbb17d21c..12ad62b5a7e9 100644 --- a/gateway/platforms/feishu.py +++ b/gateway/platforms/feishu.py @@ -240,6 +240,7 @@ # drain on completion; the cap is a safeguard against unbounded growth from # delete-failures, not a capacity plan. _FEISHU_PROCESSING_REACTION_CACHE_SIZE = 1024 +_FEISHU_MESSAGE_TEXT_CACHE_SIZE = 512 # LRU cap for reply-context message text lookups # QR onboarding constants _ONBOARD_ACCOUNTS_URLS = { @@ -1452,7 +1453,7 @@ def __init__(self, config: PlatformConfig): self._sent_message_ids_to_chat: Dict[str, str] = {} # message_id → chat_id (for reaction routing) self._sent_message_id_order: List[str] = [] # LRU order for _sent_message_ids_to_chat self._chat_info_cache: Dict[str, Dict[str, Any]] = {} - self._message_text_cache: Dict[str, Optional[str]] = {} + self._message_text_cache: "OrderedDict[str, Optional[str]]" = OrderedDict() self._app_lock_identity: Optional[str] = None self._text_batch_state = FeishuBatchState() self._pending_text_batches = self._text_batch_state.events @@ -3959,6 +3960,7 @@ async def _fetch_message_text(self, message_id: str) -> Optional[str]: if not self._client or not message_id: return None if message_id in self._message_text_cache: + self._message_text_cache.move_to_end(message_id) return self._message_text_cache[message_id] try: request = self._build_get_message_request(message_id) @@ -3980,6 +3982,8 @@ async def _fetch_message_text(self, message_id: str) -> Optional[str]: mentions=parent_mentions, ) self._message_text_cache[message_id] = text + while len(self._message_text_cache) > _FEISHU_MESSAGE_TEXT_CACHE_SIZE: + self._message_text_cache.popitem(last=False) return text except Exception: logger.warning("[Feishu] Failed to fetch parent message %s", message_id, exc_info=True) diff --git a/gateway/run.py b/gateway/run.py index 6adb98b8e209..dbe3742ca257 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -3543,9 +3543,13 @@ async def _notify_active_sessions_of_shutdown(self) -> None: ) continue - # Include thread_id if present so the message lands in the - # correct forum topic / thread. - metadata = {"thread_id": thread_id} if thread_id else None + metadata = self._thread_metadata_for_target( + platform, + chat_id, + thread_id, + chat_type=getattr(source, "chat_type", None) if source is not None else None, + adapter=adapter, + ) result = await adapter.send(chat_id, msg, metadata=metadata) if result is not None and getattr(result, "success", True) is False: @@ -3591,7 +3595,12 @@ async def _notify_active_sessions_of_shutdown(self) -> None: continue try: - metadata = {"thread_id": home.thread_id} if home.thread_id else None + metadata = self._thread_metadata_for_target( + platform, + home.chat_id, + home.thread_id, + adapter=adapter, + ) if metadata: result = await adapter.send(str(home.chat_id), msg, metadata=metadata) else: @@ -4465,10 +4474,19 @@ async def start(self) -> bool: # Drain any recovered process watchers (from crash recovery checkpoint) try: from tools.process_registry import process_registry - while process_registry.pending_watchers: - watcher = process_registry.pending_watchers.pop(0) + # Detach the current batch atomically: reassigning to a fresh list + # takes ownership of exactly the watchers present now, so any watcher + # appended concurrently during the yield below isn't silently dropped + # by a clear() on the shared list. + watchers = process_registry.pending_watchers + process_registry.pending_watchers = [] + # Process in batches of 100 with event-loop yield points to avoid + # O(n^2) event-loop blocking when recovering thousands of watchers. + for i, watcher in enumerate(watchers): asyncio.create_task(self._run_process_watcher(watcher)) logger.info("Resumed watcher for recovered process %s", watcher.get("session_id")) + if i % 100 == 99: + await asyncio.sleep(0) except Exception as e: logger.error("Recovered watcher setup error: %s", e) @@ -7938,7 +7956,7 @@ async def _do_undo(): result = await result return str(result) if result else None except Exception as e: - logger.debug("Plugin command dispatch failed (non-fatal): %s", e) + logger.warning("Plugin command dispatch failed: %s", e) # Skill slash commands: /skill-name loads the skill and sends to agent. # resolve_skill_command_key() handles the Telegram underscore/hyphen @@ -7970,7 +7988,7 @@ async def _do_undo(): ) # Fall through to normal message processing with bundle content except Exception as exc: - logger.debug("Bundle dispatch failed (non-fatal): %s", exc) + logger.warning("Bundle dispatch failed: %s", exc) if command and not locals().get("_bundle_handled", False): try: @@ -9161,9 +9179,15 @@ async def _handle_message_with_agent(self, event, source, _quick_key: str, run_g # Check for pending process watchers (check_interval on background processes) try: from tools.process_registry import process_registry - while process_registry.pending_watchers: - watcher = process_registry.pending_watchers.pop(0) + # Detach the current batch atomically (see crash-recovery drain + # above): reassign to a fresh list so a watcher appended by a + # concurrent session during the yield isn't dropped by clear(). + watchers = process_registry.pending_watchers + process_registry.pending_watchers = [] + for i, watcher in enumerate(watchers): asyncio.create_task(self._run_process_watcher(watcher)) + if i % 100 == 99: + await asyncio.sleep(0) except Exception as e: logger.error("Process watcher setup error: %s", e) @@ -10109,6 +10133,45 @@ async def _handle_agents_command(self, event: MessageEvent) -> str: return "\n".join(lines) + def _sibling_thread_run_keys(self, source: SessionSource, own_key: str) -> list: + """Find running-agent keys for OTHER participants in the same thread. + + Only applies when the message originates in a thread. In per-user + thread mode (``thread_sessions_per_user=True``) each participant gets + an isolated session key of the form + ``agent:main:{platform}:{chat_type}:{chat_id}:{thread_id}:{user_id}``, + so a run started by another user is invisible to the caller's own + ``/stop``. This returns the keys of any *actually running* agents + (not the pending sentinel, not the caller's own key) whose key shares + the caller's ``{chat_id}:{thread_id}`` prefix. + + Returns an empty list when the source is not in a thread, or when no + sibling runs exist — callers must still gate on authorization. + """ + thread_id = getattr(source, "thread_id", None) + chat_id = getattr(source, "chat_id", None) + if not thread_id or not chat_id: + return [] + platform = source.platform.value + chat_type = getattr(source, "chat_type", None) or "" + # Prefix that every per-user key in this thread shares, up to and + # including the thread_id segment. Matching either the exact + # shared-thread key or any key with a further (user_id) segment + # (prefix + ":") avoids cross-matching an unrelated thread whose id + # merely starts with this one. + prefix = ":".join( + ["agent:main", platform, chat_type, str(chat_id), str(thread_id)] + ) + matches = [] + for key, agent in list(self._running_agents.items()): + if key == own_key: + continue + if agent is _AGENT_PENDING_SENTINEL or not agent: + continue + if key == prefix or key.startswith(prefix + ":"): + matches.append(key) + return matches + async def _handle_stop_command(self, event: MessageEvent) -> Union[str, EphemeralReply]: """Handle /stop command - interrupt a running agent. @@ -10145,8 +10208,31 @@ async def _handle_stop_command(self, event: MessageEvent) -> Union[str, Ephemera invalidation_reason="stop_command_handler", ) return EphemeralReply(t("gateway.stop.stopped")) - else: - return t("gateway.stop.no_active") + + # No run under the caller's own session key. In a per-user thread + # (thread_sessions_per_user=True) each participant is isolated even + # inside one shared thread, so a run another user started lives under + # a different key. Authorized users should still be able to /stop it + # (#bernard-thread-stop). Fall back to interrupting any running + # agent(s) that share this thread, gated on authorization. + sibling_keys = self._sibling_thread_run_keys(source, session_key) + if sibling_keys and self._is_user_authorized(source): + for sibling_key in sibling_keys: + await self._interrupt_and_clear_session( + sibling_key, + source, + interrupt_reason=_INTERRUPT_REASON_STOP, + invalidation_reason="stop_command_thread_sibling", + ) + logger.info( + "STOP (thread sibling) by %s — interrupted %d run(s) in thread: %s", + session_key, + len(sibling_keys), + ", ".join(sibling_keys), + ) + return EphemeralReply(t("gateway.stop.stopped")) + + return t("gateway.stop.no_active") async def _handle_platform_command(self, event: MessageEvent) -> str: """Handle ``/platform list|pause|resume [name]`` — surface and @@ -10275,6 +10361,7 @@ async def _handle_restart_command(self, event: MessageEvent) -> Union[str, Ephem notify_data = { "platform": event.source.platform.value if event.source.platform else None, "chat_id": event.source.chat_id, + "chat_type": event.source.chat_type, } if event.source.thread_id: notify_data["thread_id"] = event.source.thread_id @@ -14120,13 +14207,34 @@ def _thread_metadata_for_source( reply_to_message_id: Optional[str] = None, ) -> Optional[Dict[str, Any]]: """Build the metadata dict platforms need for thread-aware replies.""" - thread_id = getattr(source, "thread_id", None) + return self._thread_metadata_for_target( + getattr(source, "platform", None), + getattr(source, "chat_id", None), + getattr(source, "thread_id", None), + chat_type=getattr(source, "chat_type", None), + reply_to_message_id=reply_to_message_id or getattr(source, "message_id", None), + ) + + def _thread_metadata_for_target( + self, + platform: Optional[Platform], + chat_id: Optional[str], + thread_id: Optional[str], + *, + chat_type: Optional[str] = None, + reply_to_message_id: Optional[str] = None, + adapter: Optional[Any] = None, + ) -> Optional[Dict[str, Any]]: + """Build thread metadata for synthetic sends that only have routing state.""" if thread_id is None: return None metadata: Dict[str, Any] = {"thread_id": thread_id} - if ( - getattr(source, "platform", None) == Platform.TELEGRAM - and getattr(source, "chat_type", None) == "dm" + if self._is_telegram_dm_topic_target( + platform, + chat_id, + thread_id, + chat_type=chat_type, + adapter=adapter, ): metadata["telegram_dm_topic_reply_fallback"] = True # Telegram DM topic lanes need direct_messages_topic_id in metadata @@ -14135,11 +14243,42 @@ def _thread_metadata_for_source( tid = str(thread_id) if tid and tid not in {"", "1"}: metadata["direct_messages_topic_id"] = tid - anchor = reply_to_message_id or getattr(source, "message_id", None) - if anchor is not None: - metadata["telegram_reply_to_message_id"] = str(anchor) + if reply_to_message_id is not None: + metadata["telegram_reply_to_message_id"] = str(reply_to_message_id) return metadata + @staticmethod + def _is_telegram_dm_topic_target( + platform: Optional[Platform], + chat_id: Optional[str], + thread_id: Optional[str], + *, + chat_type: Optional[str] = None, + adapter: Optional[Any] = None, + ) -> bool: + """Return True when a target is a Telegram private DM topic lane.""" + if platform != Platform.TELEGRAM or thread_id is None: + return False + if chat_type == "dm": + return True + # Inspect operator-declared DM topics via the adapter's lookup. Resolve + # the method on the CLASS, not the instance: getattr() on a MagicMock + # auto-creates a callable child for any attribute, so an instance-level + # lookup would report a DM topic for every test double. Only a + # dict-shaped return counts as an operator-declared topic — a bare + # MagicMock or other sentinel must not. Mirrors the guard in + # _rename_telegram_topic_for_session_title. + if adapter is not None and chat_id: + get_dm_topic_info = getattr(type(adapter), "_get_dm_topic_info", None) + if callable(get_dm_topic_info): + try: + topic_info = get_dm_topic_info(adapter, str(chat_id), str(thread_id)) + except Exception: + logger.debug("Failed to inspect Telegram DM topic metadata", exc_info=True) + else: + return isinstance(topic_info, dict) + return False + @staticmethod def _reply_anchor_for_event(event: MessageEvent) -> Optional[str]: """Return the platform-specific reply anchor for GatewayRunner sends.""" @@ -14348,6 +14487,7 @@ async def _handle_update_command(self, event: MessageEvent) -> str: pending = { "platform": event.source.platform.value, "chat_id": event.source.chat_id, + "chat_type": event.source.chat_type, "user_id": event.source.user_id, "session_key": session_key, "timestamp": datetime.now().isoformat(), @@ -14498,12 +14638,19 @@ async def _watch_update_progress( pending = json.loads(path.read_text()) platform_str = pending.get("platform") chat_id = pending.get("chat_id") + chat_type = pending.get("chat_type") session_key = pending.get("session_key") thread_id = pending.get("thread_id") - metadata = {"thread_id": thread_id} if thread_id else None if platform_str and chat_id: platform = Platform(platform_str) adapter = self.adapters.get(platform) + metadata = self._thread_metadata_for_target( + platform, + chat_id, + thread_id, + chat_type=chat_type, + adapter=adapter, + ) # Fallback session key if not stored (old pending files) if not session_key: session_key = f"{platform_str}:{chat_id}" @@ -14707,6 +14854,7 @@ async def _send_update_notification(self) -> bool: pending = json.loads(claimed_path.read_text()) platform_str = pending.get("platform") chat_id = pending.get("chat_id") + chat_type = pending.get("chat_type") thread_id = pending.get("thread_id") if not exit_code_path.exists(): @@ -14729,7 +14877,13 @@ async def _send_update_notification(self) -> bool: adapter = self.adapters.get(platform) if adapter and chat_id: - metadata = {"thread_id": thread_id} if thread_id else None + metadata = self._thread_metadata_for_target( + platform, + chat_id, + thread_id, + chat_type=chat_type, + adapter=adapter, + ) # Strip ANSI escape codes for clean display output = re.sub(r'\x1b\[[0-9;]*m', '', output).strip() if output: @@ -14771,6 +14925,7 @@ async def _send_restart_notification(self) -> Optional[tuple[str, str, Optional[ data = json.loads(notify_path.read_text()) platform_str = data.get("platform") chat_id = data.get("chat_id") + chat_type = data.get("chat_type") thread_id = data.get("thread_id") if not platform_str or not chat_id: @@ -14793,7 +14948,13 @@ async def _send_restart_notification(self) -> Optional[tuple[str, str, Optional[ ) return None - metadata = {"thread_id": thread_id} if thread_id else None + metadata = self._thread_metadata_for_target( + platform, + chat_id, + thread_id, + chat_type=chat_type, + adapter=adapter, + ) result = await adapter.send( str(chat_id), "♻ Gateway restarted successfully. Your session continues.", @@ -14857,7 +15018,12 @@ async def _send_home_channel_startup_notifications( continue try: - metadata = {"thread_id": home.thread_id} if home.thread_id else None + metadata = self._thread_metadata_for_target( + platform, + home.chat_id, + home.thread_id, + adapter=adapter, + ) if metadata: result = await adapter.send(str(home.chat_id), message, metadata=metadata) else: diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index 4fc59d926daf..506e7499999e 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -6126,55 +6126,56 @@ def _label(mid): _DIM = "\033[2m" _RESET = "\033[0m" - # Try arrow-key menu first, fall back to number input + # Try arrow-key menu first, fall back to number input. + # Uses the shared curses radiolist (ESC/arrow-key handling that works + # across terminals, incl. those that emit raw escape sequences) instead + # of simple_term_menu, which conflicts with /dev/tty and left ESC/arrow + # keys unreliable in the setup model picker. try: - from simple_term_menu import TerminalMenu + from hermes_cli.curses_ui import curses_radiolist - choices = [f" {_label(mid)}" for mid in ordered] - choices.append(" Enter custom model name") - choices.append(" Skip (keep current)") + choices = [_label(mid) for mid in ordered] + choices.append("Enter custom model name") + choices.append("Skip (keep current)") _upgrade_url = (portal_url or DEFAULT_NOUS_PORTAL_URL).rstrip("/") unavailable_footer = unavailable_message.strip() if not unavailable_footer and _unavailable: unavailable_footer = f"Upgrade at {_upgrade_url} for paid models" - # Print the unavailable block BEFORE the menu via regular print(). - # simple_term_menu pads title lines to terminal width (causes wrapping), - # so we keep the title minimal and use stdout for the static block. - # clear_screen=False means our printed output stays visible above. + # The pricing column header (and any unavailable-models block) is shown + # as a multi-line description above the list so it survives the curses + # screen clear. menu_title already embeds the aligned price header. + desc_lines: list[str] = [] + if has_pricing: + # menu_title is "Select default model:\n
/Mtok" + # Keep only the header portion for the description. + header_part = menu_title.split("\n", 1) + if len(header_part) > 1: + desc_lines.extend(header_part[1].splitlines()) if _unavailable: - print(menu_title) - print() for mid in _unavailable: - print(f"{_DIM} {_label(mid)}{_RESET}") - print() - print(f"{_DIM} ── {unavailable_footer} ──{_RESET}") - print() - effective_title = "Available free models:" - else: - effective_title = menu_title + desc_lines.append(f" {_label(mid)}") + desc_lines.append(f" ── {unavailable_footer} ──") + description = "\n".join(desc_lines) if desc_lines else None - menu = TerminalMenu( + idx = curses_radiolist( + "Select default model:", choices, - cursor_index=default_idx, - menu_cursor="-> ", - menu_cursor_style=("fg_green", "bold"), - menu_highlight_style=("fg_green",), - cycle_cursor=True, - clear_screen=False, - title=effective_title, + selected=default_idx, + cancel_returns=-1, + description=description, ) - idx = menu.show() - from hermes_cli.curses_ui import flush_stdin - flush_stdin() - if idx is None: + if idx < 0: return None print() if idx < len(ordered): return ordered[idx] elif idx == len(ordered): - custom = input("Enter model name: ").strip() + try: + custom = input("Enter model name: ").strip() + except (EOFError, KeyboardInterrupt): + return None return custom if custom else None return None except (ImportError, NotImplementedError, OSError, subprocess.SubprocessError): diff --git a/hermes_cli/config.py b/hermes_cli/config.py index f5985556c679..bb004d9445ad 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -1903,7 +1903,7 @@ def _ensure_hermes_home_managed(home: Path): # Disk cache TTL in hours. Beyond this, the CLI refetches on the # next /model or `hermes model` invocation; network failures # silently fall back to the stale cache. - "ttl_hours": 24, + "ttl_hours": 1, # Optional per-provider override URLs for third parties that want # to self-host their own curation list using the same schema. # Example: @@ -2151,7 +2151,7 @@ def _ensure_hermes_home_managed(home: Path): # Config schema version - bump this when adding new required fields - "_config_version": 24, + "_config_version": 25, } # ============================================================================= @@ -4344,6 +4344,22 @@ def migrate_config(interactive: bool = True, quiet: bool = False) -> Dict[str, A f"{', '.join(added_aux)}" ) + # ── Version 24 → 25: lower model_catalog TTL 24h → 1h ── + # The model picker now refreshes its curated list hourly so freshly + # published model-catalog.json deploys reach users without a day-long + # stale window. Only rewrite the OLD default (24) — never clobber a + # value the user deliberately customized. + if current_ver < 25: + config = read_raw_config() + raw_mc = config.get("model_catalog") + if isinstance(raw_mc, dict) and raw_mc.get("ttl_hours") == 24: + raw_mc["ttl_hours"] = 1 + config["model_catalog"] = raw_mc + save_config(config) + results["config_added"].append("model_catalog.ttl_hours 24→1") + if not quiet: + print(" ✓ Lowered model_catalog.ttl_hours to 1 (hourly picker refresh)") + if current_ver < latest_ver and not quiet: print(f"Config version: {current_ver} → {latest_ver}") diff --git a/hermes_cli/curses_ui.py b/hermes_cli/curses_ui.py index f0e991c0ae2c..ee31183b7125 100644 --- a/hermes_cli/curses_ui.py +++ b/hermes_cli/curses_ui.py @@ -32,37 +32,135 @@ def flush_stdin() -> None: pass -def curses_checklist( - title: str, - items: List[str], - selected: Set[int], +# Normalized menu actions returned by ``read_menu_key``. Using sentinels keeps +# every menu's key-handling branch identical and free of raw escape-byte logic. +NAV_UP = "up" +NAV_DOWN = "down" +NAV_SELECT = "select" +NAV_TOGGLE = "toggle" +NAV_CANCEL = "cancel" +NAV_NONE = "none" + + +def read_menu_key(stdscr) -> str: + """Read one keypress and normalize it to a menu action. + + Decodes raw arrow-key escape sequences in addition to the translated + ``curses.KEY_*`` values. Even with ``keypad(True)`` (which + ``curses.wrapper`` sets), some terminals/terminfo entries deliver cursor + keys as raw CSI/SS3 byte sequences — ``getch()`` then returns ``27`` (ESC) + followed by e.g. ``[`` ``A``. Treating that leading ``27`` as a cancel is + what made the setup wizard's provider/model pickers bail to the numbered + fallback the moment a user pressed up/down. + + Returns one of the ``NAV_*`` constants. A lone ESC (no continuation byte + within a short window) is the only thing that maps to ``NAV_CANCEL`` via + the escape path; ``q`` also cancels. Unknown sequences map to + ``NAV_NONE`` so the caller simply ignores them rather than misfiring. + """ + import curses + + key = stdscr.getch() + + if key in (curses.KEY_UP, ord("k")): + return NAV_UP + if key in (curses.KEY_DOWN, ord("j")): + return NAV_DOWN + if key in (curses.KEY_ENTER, 10, 13): + return NAV_SELECT + if key == ord(" "): + return NAV_TOGGLE + if key == ord("q"): + return NAV_CANCEL + + if key == 27: # ESC — could be a lone ESC (cancel) or an escape sequence. + # Wait briefly for a continuation byte. On slow PTYs (SSH/tmux) the + # bytes of an arrow key can arrive across separate reads, so a tiny + # timeout avoids misreading a split sequence as a bare ESC. + try: + stdscr.timeout(60) + nxt = stdscr.getch() + finally: + stdscr.timeout(-1) # restore blocking mode + + if nxt == -1: + return NAV_CANCEL # genuine lone ESC + + if nxt in (ord("["), ord("O")): # CSI / SS3 introducer + final = stdscr.getch() + if final in (ord("A"), ord("k")): + return NAV_UP + if final in (ord("B"), ord("j")): + return NAV_DOWN + # Consume the tail of any other CSI sequence (e.g. ``[3~`` Delete, + # ``[H`` Home) up to its terminator so stray bytes don't leak into + # the next input() and corrupt it. + while 0x20 <= final <= 0x3F: # CSI parameter/intermediate bytes + final = stdscr.getch() + return NAV_NONE + # ESC followed by some other byte we don't handle — swallow it. + return NAV_NONE + + return NAV_NONE + + +# Sentinel: an on_action reducer returns this to mean "keep looping" (the +# keypress changed cursor/selection state but didn't resolve the menu). +_KEEP = object() + + +def _run_curses_menu( *, - cancel_returns: Set[int] | None = None, - status_fn: Optional[Callable[[Set[int]], str]] = None, -) -> Set[int]: - """Curses multi-select checklist. Returns set of selected indices. - - Args: - title: Header line displayed above the checklist. - items: Display labels for each row. - selected: Indices that start checked (pre-selected). - cancel_returns: Returned on ESC/q. Defaults to the original *selected*. - status_fn: Optional callback ``f(chosen_indices) -> str`` whose return - value is rendered on the bottom row of the terminal. Use this for - live aggregate info (e.g. estimated token counts). + initial_cursor, + item_count, + draw_header, + draw_row, + on_action, + reserve_bottom=1, + draw_footer=None, + extra_color_pairs=False, + fallback, + cancel_value, +): + """Shared curses single-/multi-select event loop. + + Owns every piece the three public menus used to duplicate verbatim: + the non-TTY guard, ``curses.wrapper`` setup (cursor hide + color pairs), + the per-frame ``clear``/``getmaxyx``/``refresh`` cycle, scroll-offset math, + row iteration, the ``read_menu_key`` dispatch with ``NAV_UP``/``NAV_DOWN`` + cursor wrap, ``flush_stdin``, and the ``KeyboardInterrupt`` / curses- + unavailable fallback. Per-menu behavior is supplied as callbacks so the + rendered output stays byte-identical to the old hand-rolled loops. + + Callbacks / params: + draw_header(stdscr, max_y, max_x) -> int + Draw the title/hint/description rows. Returns the first screen row + index where the scrollable item list should start. + draw_row(stdscr, y, idx, is_cursor, max_x) -> None + Draw one item row. + on_action(action, cursor) -> value + Reducer for SELECT/TOGGLE/CANCEL. Return ``_KEEP`` to continue the + loop; return anything else to resolve the menu with that value. + (UP/DOWN cursor movement is handled by the driver itself.) + reserve_bottom: number of bottom screen rows kept clear of items + (1 = leave the final row blank, matching the old loops). + draw_footer(stdscr, max_y, max_x) -> None + Optional bottom-row painter (e.g. a status bar). Drawn after the + item rows; its row budget must be included in ``reserve_bottom``. + extra_color_pairs: also init pair 3 (dim gray) for status bars. + fallback() -> value + Called when curses errors out on a real TTY (curses unavailable). + cancel_value: returned on non-TTY stdin, ESC/cancel, or KeyboardInterrupt. """ - if cancel_returns is None: - cancel_returns = set(selected) - - # Safety: curses and input() both hang or spin when stdin is not a - # terminal (e.g. subprocess pipe). Return defaults immediately. + # Non-TTY (piped/redirected stdin): curses and input() both hang or spin, + # so return the cancel value directly — matching the pre-refactor guard in + # each menu (the numbered fallback is only for curses errors on a real TTY). if not sys.stdin.isatty(): - return cancel_returns + return cancel_value try: import curses - chosen = set(selected) - result_holder: list = [None] + result_holder = [_KEEP] def _draw(stdscr): curses.curs_set(0) @@ -71,95 +169,148 @@ def _draw(stdscr): curses.use_default_colors() curses.init_pair(1, curses.COLOR_GREEN, -1) curses.init_pair(2, curses.COLOR_YELLOW, -1) - curses.init_pair(3, 8 if curses.COLORS > 8 else curses.COLOR_WHITE, -1) # dim gray - cursor = 0 + if extra_color_pairs: + curses.init_pair( + 3, 8 if curses.COLORS > 8 else curses.COLOR_WHITE, -1 + ) + cursor = initial_cursor scroll_offset = 0 while True: stdscr.clear() max_y, max_x = stdscr.getmaxyx() - # Reserve bottom row for status bar when status_fn provided - footer_rows = 1 if status_fn else 0 - - # Header - try: - hattr = curses.A_BOLD - if curses.has_colors(): - hattr |= curses.color_pair(2) - stdscr.addnstr(0, 0, title, max_x - 1, hattr) - stdscr.addnstr( - 1, 0, - " ↑↓ navigate SPACE toggle ENTER confirm ESC cancel", - max_x - 1, curses.A_DIM, - ) - except curses.error: - pass + items_start = draw_header(stdscr, max_y, max_x) - # Scrollable item list - visible_rows = max_y - 3 - footer_rows + visible_rows = max_y - items_start - reserve_bottom if cursor < scroll_offset: scroll_offset = cursor elif cursor >= scroll_offset + visible_rows: scroll_offset = cursor - visible_rows + 1 for draw_i, i in enumerate( - range(scroll_offset, min(len(items), scroll_offset + visible_rows)) + range(scroll_offset, min(item_count, scroll_offset + visible_rows)) ): - y = draw_i + 3 - if y >= max_y - 1 - footer_rows: + y = draw_i + items_start + if y >= max_y - reserve_bottom: break - check = "✓" if i in chosen else " " - arrow = "→" if i == cursor else " " - line = f" {arrow} [{check}] {items[i]}" - attr = curses.A_NORMAL - if i == cursor: - attr = curses.A_BOLD - if curses.has_colors(): - attr |= curses.color_pair(1) - try: - stdscr.addnstr(y, 0, line, max_x - 1, attr) - except curses.error: - pass - - # Status bar (bottom row, right-aligned) - if status_fn: - try: - status_text = status_fn(chosen) - if status_text: - # Right-align on the bottom row - sx = max(0, max_x - len(status_text) - 1) - sattr = curses.A_DIM - if curses.has_colors(): - sattr |= curses.color_pair(3) - stdscr.addnstr(max_y - 1, sx, status_text, max_x - sx - 1, sattr) - except curses.error: - pass + draw_row(stdscr, y, i, i == cursor, max_x) + + if draw_footer is not None: + draw_footer(stdscr, max_y, max_x) stdscr.refresh() - key = stdscr.getch() - - if key in {curses.KEY_UP, ord("k")}: - cursor = (cursor - 1) % len(items) - elif key in {curses.KEY_DOWN, ord("j")}: - cursor = (cursor + 1) % len(items) - elif key == ord(" "): - chosen.symmetric_difference_update({cursor}) - elif key in {curses.KEY_ENTER, 10, 13}: - result_holder[0] = set(chosen) - return - elif key in {27, ord("q")}: - result_holder[0] = cancel_returns - return + action = read_menu_key(stdscr) + + if action == NAV_UP: + cursor = (cursor - 1) % item_count + elif action == NAV_DOWN: + cursor = (cursor + 1) % item_count + elif action in (NAV_SELECT, NAV_TOGGLE, NAV_CANCEL): + outcome = on_action(action, cursor) + if outcome is not _KEEP: + result_holder[0] = outcome + return curses.wrapper(_draw) flush_stdin() - return result_holder[0] if result_holder[0] is not None else cancel_returns + return result_holder[0] if result_holder[0] is not _KEEP else cancel_value except KeyboardInterrupt: - return cancel_returns + return cancel_value except Exception: - return _numbered_fallback(title, items, selected, cancel_returns, status_fn) + return fallback() + + +def curses_checklist( + title: str, + items: List[str], + selected: Set[int], + *, + cancel_returns: Set[int] | None = None, + status_fn: Optional[Callable[[Set[int]], str]] = None, +) -> Set[int]: + """Curses multi-select checklist. Returns set of selected indices. + + Args: + title: Header line displayed above the checklist. + items: Display labels for each row. + selected: Indices that start checked (pre-selected). + cancel_returns: Returned on ESC/q. Defaults to the original *selected*. + status_fn: Optional callback ``f(chosen_indices) -> str`` whose return + value is rendered on the bottom row of the terminal. Use this for + live aggregate info (e.g. estimated token counts). + """ + if cancel_returns is None: + cancel_returns = set(selected) + + chosen = set(selected) + + def _draw_header(stdscr, max_y, max_x): + import curses + try: + hattr = curses.A_BOLD + if curses.has_colors(): + hattr |= curses.color_pair(2) + stdscr.addnstr(0, 0, title, max_x - 1, hattr) + stdscr.addnstr( + 1, 0, + " ↑↓ navigate SPACE toggle ENTER confirm ESC cancel", + max_x - 1, curses.A_DIM, + ) + except curses.error: + pass + return 3 + + def _draw_row(stdscr, y, i, is_cursor, max_x): + import curses + check = "✓" if i in chosen else " " + arrow = "→" if is_cursor else " " + line = f" {arrow} [{check}] {items[i]}" + attr = curses.A_NORMAL + if is_cursor: + attr = curses.A_BOLD + if curses.has_colors(): + attr |= curses.color_pair(1) + try: + stdscr.addnstr(y, 0, line, max_x - 1, attr) + except curses.error: + pass + + def _draw_footer(stdscr, max_y, max_x): + import curses + try: + status_text = status_fn(chosen) + if status_text: + # Right-align on the bottom row + sx = max(0, max_x - len(status_text) - 1) + sattr = curses.A_DIM + if curses.has_colors(): + sattr |= curses.color_pair(3) + stdscr.addnstr(max_y - 1, sx, status_text, max_x - sx - 1, sattr) + except curses.error: + pass + + def _on_action(action, cursor): + if action == NAV_TOGGLE: + chosen.symmetric_difference_update({cursor}) + return _KEEP + if action == NAV_SELECT: + return set(chosen) + return cancel_returns # NAV_CANCEL + + return _run_curses_menu( + initial_cursor=0, + item_count=len(items), + draw_header=_draw_header, + draw_row=_draw_row, + on_action=_on_action, + reserve_bottom=(2 if status_fn else 1), + draw_footer=_draw_footer if status_fn else None, + extra_color_pairs=bool(status_fn), + fallback=lambda: _numbered_fallback(title, items, selected, cancel_returns, status_fn), + cancel_value=cancel_returns, + ) def curses_radiolist( @@ -184,106 +335,68 @@ def curses_radiolist( if cancel_returns is None: cancel_returns = selected - if not sys.stdin.isatty(): - return cancel_returns - desc_lines: list[str] = [] if description: desc_lines = description.splitlines() - try: + def _draw_header(stdscr, max_y, max_x): import curses - result_holder: list = [None] - - def _draw(stdscr): - curses.curs_set(0) + row = 0 + try: + hattr = curses.A_BOLD if curses.has_colors(): - curses.start_color() - curses.use_default_colors() - curses.init_pair(1, curses.COLOR_GREEN, -1) - curses.init_pair(2, curses.COLOR_YELLOW, -1) - cursor = selected - scroll_offset = 0 - - while True: - stdscr.clear() - max_y, max_x = stdscr.getmaxyx() - - row = 0 - - # Header - try: - hattr = curses.A_BOLD - if curses.has_colors(): - hattr |= curses.color_pair(2) - stdscr.addnstr(row, 0, title, max_x - 1, hattr) - row += 1 - - # Description lines - for dline in desc_lines: - if row >= max_y - 1: - break - stdscr.addnstr(row, 0, dline, max_x - 1, curses.A_NORMAL) - row += 1 - - stdscr.addnstr( - row, 0, - " \u2191\u2193 navigate ENTER/SPACE select ESC cancel", - max_x - 1, curses.A_DIM, - ) - row += 1 - except curses.error: - pass - - # Scrollable item list - items_start = row + 1 - visible_rows = max_y - items_start - 1 - if cursor < scroll_offset: - scroll_offset = cursor - elif cursor >= scroll_offset + visible_rows: - scroll_offset = cursor - visible_rows + 1 - - for draw_i, i in enumerate( - range(scroll_offset, min(len(items), scroll_offset + visible_rows)) - ): - y = draw_i + items_start - if y >= max_y - 1: - break - radio = "\u25cf" if i == selected else "\u25cb" - arrow = "\u2192" if i == cursor else " " - line = f" {arrow} ({radio}) {items[i]}" - attr = curses.A_NORMAL - if i == cursor: - attr = curses.A_BOLD - if curses.has_colors(): - attr |= curses.color_pair(1) - try: - stdscr.addnstr(y, 0, line, max_x - 1, attr) - except curses.error: - pass - - stdscr.refresh() - key = stdscr.getch() - - if key in {curses.KEY_UP, ord("k")}: - cursor = (cursor - 1) % len(items) - elif key in {curses.KEY_DOWN, ord("j")}: - cursor = (cursor + 1) % len(items) - elif key in {ord(" "), curses.KEY_ENTER, 10, 13}: - result_holder[0] = cursor - return - elif key in {27, ord("q")}: - result_holder[0] = cancel_returns - return - - curses.wrapper(_draw) - flush_stdin() - return result_holder[0] if result_holder[0] is not None else cancel_returns - - except KeyboardInterrupt: - return cancel_returns - except Exception: - return _radio_numbered_fallback(title, items, selected, cancel_returns) + hattr |= curses.color_pair(2) + stdscr.addnstr(row, 0, title, max_x - 1, hattr) + row += 1 + + # Description lines + for dline in desc_lines: + if row >= max_y - 1: + break + stdscr.addnstr(row, 0, dline, max_x - 1, curses.A_NORMAL) + row += 1 + + stdscr.addnstr( + row, 0, + " \u2191\u2193 navigate ENTER/SPACE select ESC cancel", + max_x - 1, curses.A_DIM, + ) + row += 1 + except curses.error: + pass + # One blank row between the hint and the item list. + return row + 1 + + def _draw_row(stdscr, y, i, is_cursor, max_x): + import curses + radio = "\u25cf" if i == selected else "\u25cb" + arrow = "\u2192" if is_cursor else " " + line = f" {arrow} ({radio}) {items[i]}" + attr = curses.A_NORMAL + if is_cursor: + attr = curses.A_BOLD + if curses.has_colors(): + attr |= curses.color_pair(1) + try: + stdscr.addnstr(y, 0, line, max_x - 1, attr) + except curses.error: + pass + + def _on_action(action, cursor): + if action in (NAV_SELECT, NAV_TOGGLE): + return cursor + return cancel_returns # NAV_CANCEL + + return _run_curses_menu( + initial_cursor=selected, + item_count=len(items), + draw_header=_draw_header, + draw_row=_draw_row, + on_action=_on_action, + reserve_bottom=1, + fallback=lambda: _radio_numbered_fallback(title, items, selected, cancel_returns), + cancel_value=cancel_returns, + ) def _radio_numbered_fallback( @@ -324,93 +437,58 @@ def curses_single_select( Works inside prompt_toolkit because curses.wrapper() restores the terminal safely, unlike simple_term_menu which conflicts with /dev/tty. """ - if not sys.stdin.isatty(): - return None + all_items = list(items) + [cancel_label] + cancel_idx = len(items) - try: + def _draw_header(stdscr, max_y, max_x): import curses - result_holder: list = [None] - - all_items = list(items) + [cancel_label] - cancel_idx = len(items) - - def _draw(stdscr): - curses.curs_set(0) + try: + hattr = curses.A_BOLD if curses.has_colors(): - curses.start_color() - curses.use_default_colors() - curses.init_pair(1, curses.COLOR_GREEN, -1) - curses.init_pair(2, curses.COLOR_YELLOW, -1) - cursor = min(default_index, len(all_items) - 1) - scroll_offset = 0 - - while True: - stdscr.clear() - max_y, max_x = stdscr.getmaxyx() - - try: - hattr = curses.A_BOLD - if curses.has_colors(): - hattr |= curses.color_pair(2) - stdscr.addnstr(0, 0, title, max_x - 1, hattr) - stdscr.addnstr( - 1, 0, - " ↑↓ navigate ENTER confirm ESC/q cancel", - max_x - 1, curses.A_DIM, - ) - except curses.error: - pass - - visible_rows = max_y - 3 - if cursor < scroll_offset: - scroll_offset = cursor - elif cursor >= scroll_offset + visible_rows: - scroll_offset = cursor - visible_rows + 1 - - for draw_i, i in enumerate( - range(scroll_offset, min(len(all_items), scroll_offset + visible_rows)) - ): - y = draw_i + 3 - if y >= max_y - 1: - break - arrow = "→" if i == cursor else " " - line = f" {arrow} {all_items[i]}" - attr = curses.A_NORMAL - if i == cursor: - attr = curses.A_BOLD - if curses.has_colors(): - attr |= curses.color_pair(1) - try: - stdscr.addnstr(y, 0, line, max_x - 1, attr) - except curses.error: - pass - - stdscr.refresh() - key = stdscr.getch() - - if key in {curses.KEY_UP, ord("k")}: - cursor = (cursor - 1) % len(all_items) - elif key in {curses.KEY_DOWN, ord("j")}: - cursor = (cursor + 1) % len(all_items) - elif key in {curses.KEY_ENTER, 10, 13}: - result_holder[0] = cursor - return - elif key in {27, ord("q")}: - result_holder[0] = None - return - - curses.wrapper(_draw) - flush_stdin() - if result_holder[0] is not None and result_holder[0] >= cancel_idx: + hattr |= curses.color_pair(2) + stdscr.addnstr(0, 0, title, max_x - 1, hattr) + stdscr.addnstr( + 1, 0, + " ↑↓ navigate ENTER confirm ESC/q cancel", + max_x - 1, curses.A_DIM, + ) + except curses.error: + pass + return 3 + + def _draw_row(stdscr, y, i, is_cursor, max_x): + import curses + arrow = "→" if is_cursor else " " + line = f" {arrow} {all_items[i]}" + attr = curses.A_NORMAL + if is_cursor: + attr = curses.A_BOLD + if curses.has_colors(): + attr |= curses.color_pair(1) + try: + stdscr.addnstr(y, 0, line, max_x - 1, attr) + except curses.error: + pass + + def _on_action(action, cursor): + if action == NAV_SELECT: + # Selecting the synthetic cancel row resolves to None, mirroring + # the old post-loop ``>= cancel_idx`` guard. + return None if cursor >= cancel_idx else cursor + if action == NAV_CANCEL: return None - return result_holder[0] - - except KeyboardInterrupt: - return None - except Exception: - all_items = list(items) + [cancel_label] - cancel_idx = len(items) - return _numbered_single_fallback(title, all_items, cancel_idx) + return _KEEP # NAV_TOGGLE — no-op for this menu + + return _run_curses_menu( + initial_cursor=min(default_index, len(all_items) - 1), + item_count=len(all_items), + draw_header=_draw_header, + draw_row=_draw_row, + on_action=_on_action, + reserve_bottom=1, + fallback=lambda: _numbered_single_fallback(title, all_items, cancel_idx), + cancel_value=None, + ) def _numbered_single_fallback( diff --git a/hermes_cli/goals.py b/hermes_cli/goals.py index d6a139419a71..a6a28deaf95d 100644 --- a/hermes_cli/goals.py +++ b/hermes_cli/goals.py @@ -747,6 +747,153 @@ def next_continuation_prompt(self) -> Optional[str]: return CONTINUATION_PROMPT_TEMPLATE.format(goal=self._state.goal) +# ────────────────────────────────────────────────────────────────────── +# Kanban worker goal loop +# ────────────────────────────────────────────────────────────────────── + +# Continuation prompt fed back to a kanban goal-mode worker that has not +# yet completed/blocked its task. The card's own acceptance criteria are +# the goal — the worker already has the full task body in its first turn, +# so we keep this short and point it back at the lifecycle contract. +KANBAN_GOAL_CONTINUATION_TEMPLATE = ( + "[Continuing toward this kanban task — judge says it is not done yet]\n" + "Reason: {reason}\n\n" + "Take the next concrete step toward completing the task. When the work " + "is genuinely finished, call kanban_complete with a summary. If you are " + "blocked and need human input, call kanban_block with a reason. Do not " + "stop without calling one of them." +) + +# Fed when the judge believes the work is done but the worker never called +# kanban_complete / kanban_block. One explicit nudge to terminate the task +# the right way before the loop gives up. +KANBAN_GOAL_FINALIZE_TEMPLATE = ( + "[The work looks complete, but the task is still open]\n" + "Reason: {reason}\n\n" + "If the task is genuinely done, call kanban_complete now with a short " + "summary of what you did. If something still blocks completion, call " + "kanban_block with the reason instead." +) + + +def run_kanban_goal_loop( + *, + task_id: str, + goal_text: str, + run_turn, + task_status_fn, + block_fn, + max_turns: int = DEFAULT_MAX_TURNS, + first_response: str = "", + log=None, +) -> Dict[str, Any]: + """Drive a kanban worker through a Ralph-style goal loop. + + The dispatcher spawns a goal-mode worker exactly like a normal worker + (``hermes -p chat -q "work kanban task "``). The worker's + first turn has already run by the time this is called; ``first_response`` + is that turn's reply. From here we: + + 1. Check whether the worker already terminated the task (called + ``kanban_complete`` / ``kanban_block``). If so, stop — nothing to do. + 2. Otherwise judge the latest response against ``goal_text`` (the card's + title + body). ``continue`` → feed a continuation prompt and run + another turn IN THE SAME SESSION via ``run_turn``. ``done`` but the + task is still open → one explicit "call kanban_complete" nudge. + 3. When the turn budget is exhausted and the worker still hasn't + terminated the task, ``block_fn`` is invoked so the card lands in a + sticky ``blocked`` state for human review (NOT a silent exit). + + This function performs NO SessionDB persistence — a worker process is + ephemeral, so the turn budget lives in a local counter. It is fully + decoupled from the CLI for testability: callers inject ``run_turn`` + (str -> str), ``task_status_fn`` (() -> str|None), and ``block_fn`` + (reason: str -> None). + + Returns a decision dict: ``{"outcome", "turns_used", "reason"}`` where + outcome is one of ``"completed_by_worker"``, ``"blocked_budget"``, + ``"blocked_by_worker"``, or ``"stopped"``. + """ + + def _log(msg: str) -> None: + if log is not None: + try: + log(msg) + except Exception: + pass + + max_turns = int(max_turns or DEFAULT_MAX_TURNS) + if max_turns < 1: + max_turns = DEFAULT_MAX_TURNS + + last_response = first_response or "" + # The first turn already consumed one unit of budget. + turns_used = 1 + nudged_to_finalize = False + + while True: + # Did the worker terminate the task itself this turn? + try: + status = task_status_fn() + except Exception as exc: + _log(f"kanban goal loop: status check failed ({exc}); stopping") + return {"outcome": "stopped", "turns_used": turns_used, "reason": "status check failed"} + + if status == "done": + _log(f"kanban goal loop: task {task_id} completed by worker after {turns_used} turn(s)") + return {"outcome": "completed_by_worker", "turns_used": turns_used, "reason": "worker completed the task"} + if status == "blocked": + _log(f"kanban goal loop: task {task_id} blocked by worker after {turns_used} turn(s)") + return {"outcome": "blocked_by_worker", "turns_used": turns_used, "reason": "worker blocked the task"} + if status not in ("running", "ready"): + # Reclaimed / archived / unexpected — let the dispatcher own it. + _log(f"kanban goal loop: task {task_id} status={status!r}; stopping") + return {"outcome": "stopped", "turns_used": turns_used, "reason": f"status={status}"} + + # Still open — judge whether the latest response satisfies the card. + verdict, reason, _parse_failed = judge_goal(goal_text, last_response) + _log(f"kanban goal loop: turn {turns_used}/{max_turns} verdict={verdict} reason={_truncate(reason, 120)}") + + if verdict == "done": + if nudged_to_finalize: + # Already asked once to call kanban_complete and it still + # didn't — block for review rather than spin. + _log(f"kanban goal loop: task {task_id} judged done but worker won't finalize; blocking") + try: + block_fn( + f"Goal-mode worker's output looked complete but it never " + f"called kanban_complete after a finalize nudge ({reason})." + ) + except Exception as exc: + _log(f"kanban goal loop: block_fn failed ({exc})") + return {"outcome": "blocked_budget", "turns_used": turns_used, "reason": "judged done, never finalized"} + prompt = KANBAN_GOAL_FINALIZE_TEMPLATE.format(reason=_truncate(reason, 400)) + nudged_to_finalize = True + else: + prompt = KANBAN_GOAL_CONTINUATION_TEMPLATE.format(reason=_truncate(reason, 400)) + + # Budget check BEFORE spending another turn. + if turns_used >= max_turns: + _log(f"kanban goal loop: task {task_id} exhausted {turns_used}/{max_turns} turns; blocking") + try: + block_fn( + f"Goal-mode worker exhausted its turn budget " + f"({turns_used}/{max_turns}) without completing the task. " + f"Last judge verdict: {_truncate(reason, 300)}" + ) + except Exception as exc: + _log(f"kanban goal loop: block_fn failed ({exc})") + return {"outcome": "blocked_budget", "turns_used": turns_used, "reason": "turn budget exhausted"} + + # Run another turn in the same session. + try: + last_response = run_turn(prompt) or "" + except Exception as exc: + _log(f"kanban goal loop: run_turn failed ({exc}); stopping") + return {"outcome": "stopped", "turns_used": turns_used, "reason": f"run_turn error: {type(exc).__name__}"} + turns_used += 1 + + __all__ = [ "GoalState", "GoalManager", @@ -754,9 +901,12 @@ def next_continuation_prompt(self) -> Optional[str]: "CONTINUATION_PROMPT_WITH_SUBGOALS_TEMPLATE", "JUDGE_USER_PROMPT_TEMPLATE", "JUDGE_USER_PROMPT_WITH_SUBGOALS_TEMPLATE", + "KANBAN_GOAL_CONTINUATION_TEMPLATE", + "KANBAN_GOAL_FINALIZE_TEMPLATE", "DEFAULT_MAX_TURNS", "load_goal", "save_goal", "clear_goal", "judge_goal", + "run_kanban_goal_loop", ] diff --git a/hermes_cli/kanban.py b/hermes_cli/kanban.py index a6e76fe35a46..8b67ebc3d99d 100644 --- a/hermes_cli/kanban.py +++ b/hermes_cli/kanban.py @@ -341,6 +341,19 @@ def build_parser(parent_subparsers: argparse._SubParsersAction) -> argparse.Argu "two retries. Omit to use the dispatcher's " "kanban.failure_limit config " f"(default {kb.DEFAULT_FAILURE_LIMIT}).") + p_create.add_argument("--goal", action="store_true", dest="goal_mode", + help="Run the worker in a goal loop: after each " + "turn a judge checks the response against the " + "card title/body and, if not done, the worker " + "keeps going in the same session until the " + "judge agrees it's complete (or the turn " + "budget runs out, which blocks the card for " + "review). Best for open-ended cards one shot " + "rarely finishes.") + p_create.add_argument("--goal-max-turns", type=int, default=None, + metavar="N", dest="goal_max_turns", + help="Turn budget for --goal workers (default 20). " + "Ignored without --goal.") p_create.add_argument("--initial-status", choices=sorted(kb.VALID_INITIAL_STATUSES), default="running", @@ -1343,6 +1356,8 @@ def _cmd_create(args: argparse.Namespace) -> int: max_runtime_seconds=max_runtime, skills=getattr(args, "skills", None) or None, max_retries=max_retries, + goal_mode=bool(getattr(args, "goal_mode", False)), + goal_max_turns=getattr(args, "goal_max_turns", None), initial_status=getattr(args, "initial_status", "running"), ) task = kb.get_task(conn, task_id) diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index 4711655249d2..3bb14573e9e2 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -725,6 +725,19 @@ class Task: # ``kanban.failure_limit`` config, and then to ``DEFAULT_FAILURE_LIMIT``. # Name matches the ``--max-retries`` CLI flag on ``kanban create``. max_retries: Optional[int] = None + # When True, the dispatched worker runs in a Ralph-style goal loop + # (the same engine behind the ``/goal`` slash command): after each + # turn an auxiliary judge model evaluates the worker's response + # against this card's title/body (treated as the goal). If the judge + # says "not done" and budget remains, the worker is fed a + # continuation prompt IN THE SAME SESSION and keeps working until the + # judge agrees, the goal-turn budget is exhausted (→ kanban_block), + # or the worker explicitly blocks/completes. ``False`` (default) = + # the classic single-shot worker. ``goal_max_turns`` bounds the loop. + goal_mode: bool = False + # Goal-loop turn budget for ``goal_mode`` workers. ``None`` falls + # through to the goals engine default (``goals.DEFAULT_MAX_TURNS``). + goal_max_turns: Optional[int] = None # Originating chat/agent session id, when the task was created from # within an agent loop that propagated ``HERMES_SESSION_ID``. NULL for # tasks created from the CLI, the dashboard, or any path that doesn't @@ -797,6 +810,12 @@ def from_row(cls, row: sqlite3.Row) -> "Task": max_retries=( row["max_retries"] if "max_retries" in keys else None ), + goal_mode=( + bool(row["goal_mode"]) if "goal_mode" in keys and row["goal_mode"] else False + ), + goal_max_turns=( + row["goal_max_turns"] if "goal_max_turns" in keys and row["goal_max_turns"] else None + ), session_id=( row["session_id"] if "session_id" in keys else None ), @@ -946,6 +965,16 @@ class Event: -- case) falls through to the dispatcher-level ``kanban.failure_limit`` -- config and then ``DEFAULT_FAILURE_LIMIT``. max_retries INTEGER, + -- When 1, the dispatched worker runs in a Ralph-style goal loop: an + -- auxiliary judge re-evaluates the worker's response against the + -- card title/body after each turn and feeds a continuation prompt + -- back into the SAME session until the judge agrees the work is done + -- or ``goal_max_turns`` is exhausted. NULL/0 = classic single-shot + -- worker (the default). + goal_mode INTEGER NOT NULL DEFAULT 0, + -- Goal-loop turn budget for ``goal_mode`` workers. NULL = use the + -- goals-engine default. + goal_max_turns INTEGER, -- Originating chat/agent session id when the task was created from -- inside an agent loop that propagated ``HERMES_SESSION_ID``. NULL -- for tasks created from the CLI, dashboard, or any path that doesn't @@ -1584,6 +1613,20 @@ def _migrate_add_optional_columns(conn: sqlite3.Connection) -> None: if "model_override" not in cols: conn.execute("ALTER TABLE tasks ADD COLUMN model_override TEXT") + if "goal_mode" not in cols: + # Ralph-style goal loop toggle for the dispatched worker. 0 (the + # default) = classic single-shot worker, preserving the behaviour + # existing rows had before the column existed. + _add_column_if_missing( + conn, "tasks", "goal_mode", "goal_mode INTEGER NOT NULL DEFAULT 0" + ) + + if "goal_max_turns" not in cols: + # Per-task goal-loop turn budget. NULL = goals-engine default. + _add_column_if_missing( + conn, "tasks", "goal_max_turns", "goal_max_turns INTEGER" + ) + if "session_id" not in cols: # Originating agent/chat session id, populated when the task is # created from within an agent loop that propagated @@ -1967,6 +2010,8 @@ def create_task( max_runtime_seconds: Optional[int] = None, skills: Optional[Iterable[str]] = None, max_retries: Optional[int] = None, + goal_mode: bool = False, + goal_max_turns: Optional[int] = None, initial_status: str = "running", session_id: Optional[str] = None, board: Optional[str] = None, @@ -2134,8 +2179,8 @@ def create_task( id, title, body, assignee, status, priority, created_by, created_at, workspace_kind, workspace_path, branch_name, tenant, idempotency_key, max_runtime_seconds, - skills, max_retries, session_id - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + skills, max_retries, goal_mode, goal_max_turns, session_id + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) """, ( task_id, @@ -2154,6 +2199,8 @@ def create_task( int(max_runtime_seconds) if max_runtime_seconds is not None else None, json.dumps(skills_list) if skills_list is not None else None, int(max_retries) if max_retries is not None else None, + 1 if goal_mode else 0, + int(goal_max_turns) if goal_max_turns is not None else None, session_id, ), ) @@ -2173,6 +2220,7 @@ def create_task( "tenant": tenant, "branch_name": branch_name, "skills": list(skills_list) if skills_list else None, + "goal_mode": bool(goal_mode) or None, }, ) return task_id @@ -6412,6 +6460,13 @@ def _default_spawn( env["HERMES_KANBAN_RUN_ID"] = str(task.current_run_id) if task.claim_lock: env["HERMES_KANBAN_CLAIM_LOCK"] = task.claim_lock + # Goal-loop mode: the worker reads these and wraps its run in the + # Ralph-style /goal judge loop (see cli.py quiet-mode path). Only set + # when enabled so non-goal tasks keep a clean env. + if task.goal_mode: + env["HERMES_KANBAN_GOAL_MODE"] = "1" + if task.goal_max_turns is not None: + env["HERMES_KANBAN_GOAL_MAX_TURNS"] = str(int(task.goal_max_turns)) terminal_timeout = _worker_terminal_timeout_env( task.max_runtime_seconds, env.get("TERMINAL_TIMEOUT"), diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 1cb4bd3d6b89..1941dc2af313 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -4455,23 +4455,17 @@ def _remove_custom_provider(config): choices.append("Cancel") try: - from simple_term_menu import TerminalMenu - - menu = TerminalMenu( - [f" {c}" for c in choices], - cursor_index=0, - menu_cursor="-> ", - menu_cursor_style=("fg_red", "bold"), - menu_highlight_style=("fg_red",), - cycle_cursor=True, - clear_screen=False, - title="Select provider to remove:", - ) - idx = menu.show() - from hermes_cli.curses_ui import flush_stdin + from hermes_cli.curses_ui import curses_radiolist - flush_stdin() + idx = curses_radiolist( + "Select provider to remove:", + list(choices), + selected=0, + cancel_returns=-1, + ) print() + if idx < 0: + idx = None except (ImportError, NotImplementedError, OSError, subprocess.SubprocessError): for i, c in enumerate(choices, 1): print(f" {i}. {c}") @@ -4538,27 +4532,19 @@ def _model_flow_named_custom(config, provider_info): print(f"Found {len(models)} model(s):\n") try: - from simple_term_menu import TerminalMenu + from hermes_cli.curses_ui import curses_radiolist menu_items = [ - f" {m} (current)" if m == saved_model else f" {m}" for m in models - ] + [" Cancel"] - menu = TerminalMenu( + f"{m} (current)" if m == saved_model else m for m in models + ] + ["Cancel"] + idx = curses_radiolist( + f"Select model from {name}:", menu_items, - cursor_index=default_idx, - menu_cursor="-> ", - menu_cursor_style=("fg_green", "bold"), - menu_highlight_style=("fg_green",), - cycle_cursor=True, - clear_screen=False, - title=f"Select model from {name}:", + selected=default_idx, + cancel_returns=-1, ) - idx = menu.show() - from hermes_cli.curses_ui import flush_stdin - - flush_stdin() print() - if idx is None or idx >= len(models): + if idx < 0 or idx >= len(models): print("Cancelled.") return model_name = models[idx] @@ -4735,26 +4721,18 @@ def _label(effort): default_idx = 0 try: - from simple_term_menu import TerminalMenu + from hermes_cli.curses_ui import curses_radiolist - choices = [f" {_label(effort)}" for effort in ordered] - choices.append(f" {disable_label}") - choices.append(f" {skip_label}") - menu = TerminalMenu( + choices = [_label(effort) for effort in ordered] + choices.append(disable_label) + choices.append(skip_label) + idx = curses_radiolist( + "Select reasoning effort:", choices, - cursor_index=default_idx, - menu_cursor="-> ", - menu_cursor_style=("fg_green", "bold"), - menu_highlight_style=("fg_green",), - cycle_cursor=True, - clear_screen=False, - title="Select reasoning effort:", + selected=default_idx, + cancel_returns=-1, ) - idx = menu.show() - from hermes_cli.curses_ui import flush_stdin - - flush_stdin() - if idx is None: + if idx < 0: return None print() if idx < len(ordered): diff --git a/hermes_cli/model_catalog.py b/hermes_cli/model_catalog.py index 703d958402ca..f69791340dc1 100644 --- a/hermes_cli/model_catalog.py +++ b/hermes_cli/model_catalog.py @@ -73,7 +73,7 @@ DEFAULT_CATALOG_FALLBACK_URLS: tuple[str, ...] = ( "https://raw.githubusercontent.com/NousResearch/hermes-agent/main/website/static/api/model-catalog.json", ) -DEFAULT_TTL_HOURS = 24 +DEFAULT_TTL_HOURS = 1 DEFAULT_FETCH_TIMEOUT = 8.0 SUPPORTED_SCHEMA_VERSION = 1 diff --git a/hermes_cli/nous_subscription.py b/hermes_cli/nous_subscription.py index f19393337bd4..abc79bbf7cfa 100644 --- a/hermes_cli/nous_subscription.py +++ b/hermes_cli/nous_subscription.py @@ -7,7 +7,11 @@ from typing import Dict, Iterable, Optional, Set from hermes_cli.config import get_env_value, load_config -from hermes_cli.nous_account import NousPortalAccountInfo, get_nous_portal_account_info +from hermes_cli.nous_account import ( + NousPortalAccountInfo, + format_nous_portal_entitlement_message, + get_nous_portal_account_info, +) from tools.managed_tool_gateway import is_managed_tool_gateway_ready from utils import is_truthy_value from tools.tool_backend_helpers import ( @@ -882,3 +886,136 @@ def prompt_enable_tool_gateway( if already_managed and not newly_switched: print(" (all tools already using Tool Gateway)") return changed + + +# --------------------------------------------------------------------------- +# Inline Nous Portal login for the Tool Gateway picker (`hermes tools`) +# --------------------------------------------------------------------------- + + +def ensure_nous_portal_access(*, capability: str = "the Nous Tool Gateway") -> bool: + """Make sure the user has paid Nous Portal access, logging in if needed. + + Used by ``hermes tools`` when a user selects a Nous-managed Tool Gateway + backend (e.g. "Firecrawl (Nous Portal)"). Unlike ``hermes model``'s Nous + login, this: + + - does NOT change the inference provider (``model.provider`` is untouched), + - does NOT run model selection, and + - does NOT offer the bulk "enable for all tools" Tool Gateway prompt. + + It only performs the Nous Portal device-code OAuth (when the user isn't + already logged in) and refreshes entitlement, so the caller can enable the + single tool the user picked. + + Returns ``True`` when the account has paid service access after the flow, + ``False`` otherwise (declined login, login failed, or no paid entitlement). + """ + # Fast path: already entitled. + try: + info = get_nous_portal_account_info(force_fresh=True) + except Exception: + info = None + if info is not None and info.paid_service_access is True: + return True + + # If not logged in at all, run the device-code login (auth only). + if info is None or not info.logged_in: + if not _run_nous_portal_login_only(capability=capability): + return False + try: + info = get_nous_portal_account_info(force_fresh=True) + except Exception: + info = None + + if info is not None and info.paid_service_access is True: + return True + + # Logged in but no paid access — surface billing guidance, do not enable. + message = format_nous_portal_entitlement_message(info, capability=capability) + if message: + for line in message.splitlines(): + print(f" {line}") + return False + + +def _run_nous_portal_login_only(*, capability: str) -> bool: + """Run the Nous Portal device-code OAuth and persist credentials only. + + No model selection, no provider switch, no Tool Gateway bulk prompt. + Returns ``True`` on a successful login, ``False`` if the user declined or + the flow failed. + """ + try: + from hermes_cli.auth import ( + _auth_store_lock, + _load_auth_store, + _nous_device_code_login, + _read_shared_nous_state, + _save_auth_store, + _save_provider_state, + _sync_nous_pool_from_auth_store, + _try_import_shared_nous_state, + _write_shared_nous_state, + ) + except Exception as exc: # pragma: no cover - defensive + print(f" Could not start Nous Portal login: {exc}") + return False + + print() + print(f" {capability} requires a Nous Portal login.") + try: + proceed = input(" Log in to Nous Portal now? [Y/n]: ").strip().lower() + except (EOFError, KeyboardInterrupt): + print() + return False + if proceed not in {"", "y", "yes"}: + print(" Skipped Nous Portal login.") + return False + + try: + # Snapshot the active_provider so a tool-config login never silently + # switches the user's inference provider to Nous. + with _auth_store_lock(): + prior_active_provider = _load_auth_store().get("active_provider") + + auth_state = None + shared = _read_shared_nous_state() + if shared: + try: + do_import = input( + " Found existing Nous OAuth credentials. Import them? [Y/n]: " + ).strip().lower() + except (EOFError, KeyboardInterrupt): + do_import = "y" + if do_import in {"", "y", "yes"}: + auth_state = _try_import_shared_nous_state(timeout_seconds=15.0) + + if auth_state is None: + auth_state = _nous_device_code_login() + + with _auth_store_lock(): + auth_store = _load_auth_store() + _save_provider_state(auth_store, "nous", auth_state) + # Preserve the user's existing inference provider — this login is + # for tool entitlement only, not a provider switch. + if prior_active_provider: + auth_store["active_provider"] = prior_active_provider + else: + auth_store.pop("active_provider", None) + _save_auth_store(auth_store) + + _write_shared_nous_state(auth_state) + _sync_nous_pool_from_auth_store() + print(" Nous Portal login successful.") + return True + except KeyboardInterrupt: + print("\n Login cancelled.") + return False + except SystemExit: + # _nous_device_code_login raises SystemExit on subscription_required; + # it already printed billing guidance. + return False + except Exception as exc: + print(f" Nous Portal login failed: {exc}") + return False diff --git a/hermes_cli/setup.py b/hermes_cli/setup.py index b65bffabf1c5..5753ab83c5e6 100644 --- a/hermes_cli/setup.py +++ b/hermes_cli/setup.py @@ -305,7 +305,7 @@ def prompt_checklist(title: str, items: list, pre_selected: list = None) -> list appended at the end — the user toggles items with Space and confirms with Enter on "Continue →". - Falls back to a numbered toggle interface when simple_term_menu is + Falls back to a numbered toggle interface when curses is unavailable. Returns: @@ -737,175 +737,15 @@ def setup_model_provider(config: dict, *, quick: bool = False): if isinstance(_m, dict): selected_provider = _m.get("provider") - # ── Same-provider fallback & rotation setup (full setup only) ── - if not quick and _supports_same_provider_pool_setup(selected_provider): - try: - from types import SimpleNamespace - from agent.credential_pool import load_pool - from hermes_cli.auth_commands import auth_add_command - - pool = load_pool(selected_provider) - entries = pool.entries() - entry_count = len(entries) - manual_count = sum(1 for entry in entries if str(getattr(entry, "source", "")).startswith("manual")) - auto_count = entry_count - manual_count - print() - print_header("Same-Provider Fallback & Rotation") - print_info( - "Hermes can keep multiple credentials for one provider and rotate between" - ) - print_info( - "them when a credential is exhausted or rate-limited. This preserves" - ) - print_info( - "your primary provider while reducing interruptions from quota issues." - ) - print() - if auto_count > 0: - print_info( - f"Current pooled credentials for {selected_provider}: {entry_count} " - f"({manual_count} manual, {auto_count} auto-detected from env/shared auth)" - ) - else: - print_info(f"Current pooled credentials for {selected_provider}: {entry_count}") - - while prompt_yes_no("Add another credential for same-provider fallback?", False): - auth_add_command( - SimpleNamespace( - provider=selected_provider, - auth_type="", - label=None, - api_key=None, - portal_url=None, - inference_url=None, - client_id=None, - scope=None, - no_browser=False, - timeout=15.0, - insecure=False, - ca_bundle=None, - ) - ) - pool = load_pool(selected_provider) - entry_count = len(pool.entries()) - print_info(f"Provider pool now has {entry_count} credential(s).") - - if entry_count > 1: - strategy_labels = [ - "Fill-first / sticky — keep using the first healthy credential until it is exhausted", - "Round robin — rotate to the next healthy credential after each selection", - "Random — pick a random healthy credential each time", - ] - current_strategy = _get_credential_pool_strategies(config).get(selected_provider, "fill_first") - default_strategy_idx = { - "fill_first": 0, - "round_robin": 1, - "random": 2, - }.get(current_strategy, 0) - strategy_idx = prompt_choice( - "Select same-provider rotation strategy:", - strategy_labels, - default_strategy_idx, - ) - strategy_value = ["fill_first", "round_robin", "random"][strategy_idx] - _set_credential_pool_strategy(config, selected_provider, strategy_value) - print_success(f"Saved {selected_provider} rotation strategy: {strategy_value}") - except Exception as exc: - logger.debug("Could not configure same-provider fallback in setup: %s", exc) - - # ── Vision & Image Analysis Setup (full setup only) ── - if quick: - _vision_needs_setup = False - else: - try: - from agent.auxiliary_client import get_available_vision_backends - _vision_backends = set(get_available_vision_backends()) - except Exception: - _vision_backends = set() - - _vision_needs_setup = not bool(_vision_backends) - - if selected_provider in _vision_backends: - _vision_needs_setup = False - - if _vision_needs_setup: - _prov_names = { - "nous-api": "Nous Portal API key", - "copilot": "GitHub Copilot", - "copilot-acp": "GitHub Copilot ACP", - "zai": "Z.AI / GLM", - "kimi-coding": "Kimi / Moonshot", - "kimi-coding-cn": "Kimi / Moonshot (China)", - "stepfun": "StepFun Step Plan", - "minimax": "MiniMax", - "minimax-cn": "MiniMax CN", - "anthropic": "Anthropic", - "custom": "your custom endpoint", - } - _prov_display = _prov_names.get(selected_provider, selected_provider or "your provider") - - print() - print_header("Vision & Image Analysis (optional)") - print_info(f"Vision uses a separate multimodal backend. {_prov_display}") - print_info("doesn't currently provide one Hermes can auto-use for vision,") - print_info("so choose a backend now or skip and configure later.") - print() - - _vision_choices = [ - "OpenRouter — uses Gemini (free tier at openrouter.ai/keys)", - "OpenAI-compatible endpoint — base URL, API key, and vision model", - "Skip for now", - ] - _vision_idx = prompt_choice("Configure vision:", _vision_choices, 2) - - if _vision_idx == 0: # OpenRouter - _or_key = prompt(" OpenRouter API key", password=True).strip() - if _or_key: - save_env_value("OPENROUTER_API_KEY", _or_key) - print_success("OpenRouter key saved — vision will use Gemini") - else: - print_info("Skipped — vision won't be available") - elif _vision_idx == 1: # OpenAI-compatible endpoint - _base_url = prompt(" Base URL (blank for OpenAI)").strip() or "https://api.openai.com/v1" - _api_key_label = " API key" - _is_native_openai = base_url_hostname(_base_url) == "api.openai.com" - if _is_native_openai: - _api_key_label = " OpenAI API key" - _oai_key = prompt(_api_key_label, password=True).strip() - if _oai_key: - save_env_value("OPENAI_API_KEY", _oai_key) - # Save vision base URL to config (not .env — only secrets go there) - _vaux = config.setdefault("auxiliary", {}).setdefault("vision", {}) - _vaux["base_url"] = _base_url - if _is_native_openai: - _oai_vision_models = ["gpt-4o", "gpt-4o-mini", "gpt-4.1", "gpt-4.1-mini", "gpt-4.1-nano"] - _vm_choices = _oai_vision_models + ["Use default (gpt-4o-mini)"] - _vm_idx = prompt_choice("Select vision model:", _vm_choices, 0) - _selected_vision_model = ( - _oai_vision_models[_vm_idx] - if _vm_idx < len(_oai_vision_models) - else "gpt-4o-mini" - ) - else: - _selected_vision_model = prompt(" Vision model (blank = use main/custom default)").strip() - if _selected_vision_model: - save_env_value("AUXILIARY_VISION_MODEL", _selected_vision_model) - print_success( - f"Vision configured with {_base_url}" - + (f" ({_selected_vision_model})" if _selected_vision_model else "") - ) - else: - print_info("Skipped — vision won't be available") - else: - print_info("Skipped — add later with 'hermes setup' or configure AUXILIARY_VISION_* settings") - + # Credential rotation, vision-backend selection, and TTS provider are no + # longer prompted here. They have safe defaults (rotation off, vision + # auto-detected from the main provider, TTS = Edge) and are configurable + # on demand via `hermes auth add`, `hermes setup` vision, and + # `hermes setup tts`. This keeps both quick and full setup thin. # Tool Gateway prompt is already shown by _model_flow_nous() above. save_config(config) - if not quick and selected_provider != "nous": - _setup_tts_provider(config) - # ============================================================================= # Section 1b: TTS Provider Configuration @@ -1341,29 +1181,9 @@ def setup_terminal_backend(config: dict): if selected_backend == "local": print_success("Terminal backend: Local") print_info("Commands run directly on this machine.") - - # Gateway/cron working directory - print() - print_info("Gateway working directory:") - print_info(" Used by Telegram/Discord/cron sessions.") - print_info(" CLI/TUI always uses your launch directory instead.") - current_cwd = cfg_get(config, "terminal", "cwd", default="") - cwd = prompt(" Gateway working directory", current_cwd or str(Path.home())) - if cwd: - config["terminal"]["cwd"] = cwd - - # Sudo support - print() - existing_sudo = get_env_value("SUDO_PASSWORD") - if existing_sudo: - print_info("Sudo password: configured") - elif prompt_yes_no( - "Enable sudo support? (stores password for apt install, etc.)", False - ): - sudo_pass = prompt(" Sudo password", password=True) - if sudo_pass: - save_env_value("SUDO_PASSWORD", sudo_pass) - print_success("Sudo password saved") + # Gateway working directory defaults to home; sudo stays off. Both are + # configurable later via `hermes setup terminal` / config.yaml. + config["terminal"].setdefault("cwd", str(Path.home())) elif selected_backend == "docker": print_success("Terminal backend: Docker") @@ -1376,13 +1196,10 @@ def setup_terminal_backend(config: dict): else: print_info(f"Docker found: {docker_bin}") - # Docker image - current_image = cfg_get(config, "terminal", "docker_image", default="nikolaik/python-nodejs:python3.11-nodejs20") - image = prompt(" Docker image", current_image) - config["terminal"]["docker_image"] = image - save_env_value("TERMINAL_DOCKER_IMAGE", image) - - _prompt_container_resources(config) + # Image and resource limits use defaults; tune via `hermes setup terminal`. + config["terminal"].setdefault( + "docker_image", "nikolaik/python-nodejs:python3.11-nodejs20" + ) elif selected_backend == "singularity": print_success("Terminal backend: Singularity/Apptainer") @@ -1397,12 +1214,11 @@ def setup_terminal_backend(config: dict): else: print_info(f"Found: {sing_bin}") - current_image = cfg_get(config, "terminal", "singularity_image", default="docker://nikolaik/python-nodejs:python3.11-nodejs20") - image = prompt(" Container image", current_image) - config["terminal"]["singularity_image"] = image - save_env_value("TERMINAL_SINGULARITY_IMAGE", image) - - _prompt_container_resources(config) + # Image and resource limits use defaults; tune via `hermes setup terminal`. + config["terminal"].setdefault( + "singularity_image", + "docker://nikolaik/python-nodejs:python3.11-nodejs20", + ) elif selected_backend == "modal": print_success("Terminal backend: Modal") @@ -1501,8 +1317,6 @@ def setup_terminal_backend(config: dict): if token_secret: save_env_value("MODAL_TOKEN_SECRET", token_secret) - _prompt_container_resources(config) - elif selected_backend == "daytona": print_success("Terminal backend: Daytona") print_info("Persistent cloud development environments.") @@ -1552,13 +1366,10 @@ def setup_terminal_backend(config: dict): save_env_value("DAYTONA_API_KEY", api_key) print_success(" Configured") - # Daytona image - current_image = cfg_get(config, "terminal", "daytona_image", default="nikolaik/python-nodejs:python3.11-nodejs20") - image = prompt(" Sandbox image", current_image) - config["terminal"]["daytona_image"] = image - save_env_value("TERMINAL_DAYTONA_IMAGE", image) - - _prompt_container_resources(config) + # Image and resource limits use defaults; tune via `hermes setup terminal`. + config["terminal"].setdefault( + "daytona_image", "nikolaik/python-nodejs:python3.11-nodejs20" + ) elif selected_backend == "ssh": print_success("Terminal backend: SSH") @@ -1625,7 +1436,7 @@ def setup_terminal_backend(config: dict): def _apply_default_agent_settings(config: dict): """Apply recommended defaults for all agent settings without prompting.""" - config.setdefault("agent", {})["max_turns"] = 90 + config.setdefault("agent", {})["max_turns"] = 150 # config.yaml is the authoritative source for max_turns; the gateway # bridges it into HERMES_MAX_ITERATIONS at startup. We no longer write # to .env to avoid the dual-source inconsistency that caused the @@ -1637,18 +1448,17 @@ def _apply_default_agent_settings(config: dict): config.setdefault("compression", {})["enabled"] = True config["compression"]["threshold"] = 0.50 - config.setdefault("session_reset", {}).update({ - "mode": "both", - "idle_minutes": 1440, - "at_hour": 4, - }) + # Default to never auto-resetting sessions. The gateway treats absent + # session_reset as "both", so we must write "none" explicitly to make + # the no-auto-reset default actually take effect. + config.setdefault("session_reset", {})["mode"] = "none" save_config(config) print_success("Applied recommended defaults:") - print_info(" Max iterations: 90") + print_info(" Max iterations: 150") print_info(" Tool progress: all") print_info(" Compression threshold: 0.50") - print_info(" Session reset: inactivity (1440 min) + daily (4:00)") + print_info(" Session reset: never (use /reset or compression)") print_info(" Run `hermes setup agent` later to customize.") @@ -3197,7 +3007,7 @@ def run_setup_wizard(args): config = load_config() setup_mode = prompt_choice("How would you like to set up Hermes?", [ - "Quick setup — provider, model & messaging (recommended)", + "Quick Setup (Nous Portal) — OAuth login, model & messaging (recommended)", "Full setup — configure everything", ], 0) @@ -3228,9 +3038,11 @@ def run_setup_wizard(args): if not (migration_ran and _skip_configured_section(config, "terminal", "Terminal Backend")): setup_terminal_backend(config) - # Section 3: Agent Settings - if not (migration_ran and _skip_configured_section(config, "agent", "Agent Settings")): - setup_agent_settings(config) + # Section 3: Agent Settings — no longer prompted. First installs get the + # recommended defaults silently; existing installs keep whatever they have. + # Tune later with `hermes setup agent`. + if not is_existing: + _apply_default_agent_settings(config) # Section 4: Messaging Platforms if not (migration_ran and _skip_configured_section(config, "gateway", "Messaging Platforms")): @@ -3250,13 +3062,43 @@ def run_setup_wizard(args): def _run_first_time_quick_setup(config: dict, hermes_home, is_existing: bool): - """Streamlined first-time setup: provider, model, terminal & messaging. + """Streamlined first-time setup via Nous Portal: OAuth, model, terminal & messaging. - Applies sensible defaults for TTS (Edge), agent settings, and tools — - the user can customize later via ``hermes setup
``. + Routes straight to the Nous Portal provider — runs the device-code OAuth + login, picks a Nous model, then configures the terminal backend and (optionally) + a messaging platform. Applies sensible defaults for everything else (agent + settings, tools); the user can customize later via ``hermes setup
`` + or switch providers with ``hermes model``. """ - # Step 1: Model & Provider (essential — skips rotation/vision/TTS) - setup_model_provider(config, quick=True) + from hermes_cli.config import load_config + + # Step 1: Nous Portal — OAuth login + model selection. + # _model_flow_nous() handles both the logged-out path (device-code OAuth, + # which selects a model internally) and the already-logged-in path (curated + # Nous model picker). Provider is set to "nous" by the login/model save. + print() + print_header("Nous Portal") + print_info("One subscription, 300+ models, plus the Tool Gateway:") + print_info(" web search, image generation, TTS, browser automation.") + print_info("Sign up: https://portal.nousresearch.com/manage-subscription") + print() + try: + from hermes_cli.main import _model_flow_nous + _model_flow_nous(config) + except (KeyboardInterrupt, EOFError): + print() + print_info("Nous Portal setup cancelled.") + except Exception as exc: + logger.debug("_model_flow_nous error during quick setup: %s", exc) + print_warning(f"Nous Portal setup encountered an error: {exc}") + print_info("You can try again later with: hermes model") + + # Re-sync the wizard's config dict from disk — _model_flow_nous (and the + # underlying login/model save) write via their own load/save cycle, and the + # wizard's later save_config(config) must not clobber those values (#4172). + _refreshed = load_config() + config.clear() + config.update(_refreshed) # Step 2: Terminal Backend — where commands run is a core decision setup_terminal_backend(config) diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index 4b495d4d3a0e..8322ff0d8e10 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -1876,18 +1876,26 @@ def _visible_providers( *, force_fresh: bool = False, ) -> list[dict]: - """Return provider entries visible for the current auth/config state.""" + """Return provider entries visible for the current auth/config state. + + Nous-managed Tool Gateway rows (``managed_nous_feature``) are always + shown — even to logged-out / unentitled users — so the picker advertises + that the capability exists. Selecting one drives an inline Nous Portal + login + entitlement check (see ``_configure_provider``); the row only + *activates* the gateway once paid access is confirmed. + """ features = get_nous_subscription_features(config, force_fresh=force_fresh) - managed_available = bool( - features.account_info - and features.account_info.logged_in - and features.account_info.paid_service_access is True - ) visible = [] for provider in cat.get("providers", []): - if provider.get("managed_nous_feature") and not managed_available: - continue - if provider.get("requires_nous_auth") and not features.nous_auth_present: + # Nous-managed Tool Gateway rows stay visible regardless of auth — + # selecting one drives an inline Portal login. A `requires_nous_auth` + # row that is NOT a managed gateway feature (pure pre-auth UX) is + # still hidden until the user is logged in. + if ( + provider.get("requires_nous_auth") + and not provider.get("managed_nous_feature") + and not features.nous_auth_present + ): continue visible.append(provider) @@ -1933,22 +1941,16 @@ def _hidden_nous_gateway_message( *, force_fresh: bool = False, ) -> str: - """Return a reason when a category's Nous provider is hidden.""" - features = get_nous_subscription_features(config, force_fresh=force_fresh) - managed_available = bool( - features.account_info - and features.account_info.logged_in - and features.account_info.paid_service_access is True - ) - if managed_available: - return "" - if not any(p.get("managed_nous_feature") for p in cat.get("providers", [])): - return "" - message = format_nous_portal_entitlement_message( - features.account_info, - capability=capability, - ) - return message or "" + """Deprecated: Nous Tool Gateway rows are no longer hidden. + + Previously this returned a "log in / upgrade" banner shown above a + category when its Nous-managed rows were filtered out for unentitled + users. Those rows are now always listed (see ``_visible_providers``), and + the login + entitlement guidance happens inline when the user selects one + (``ensure_nous_portal_access``). Kept as a no-op so call sites stay simple; + always returns an empty string. + """ + return "" _POST_SETUP_INSTALLED: dict = { @@ -2132,14 +2134,17 @@ def _configure_tool_category( configured = "" else: configured = " [configured]" - # Highlight Nous-managed entries when the user has Portal auth. - # curses_radiolist can't render ANSI inside item strings, so we - # use a plain unicode star + parenthetical phrase. Suppressed - # when no Portal auth is present so non-subscribers see the - # picker unchanged. + # Mark Nous-managed entries. Logged-in paid subscribers get the + # "included" star; everyone else gets a "via Nous Portal" hint so + # it's clear selecting the row triggers a Portal login. The rows + # are always shown now (see _visible_providers) — selecting one + # drives an inline login + entitlement check. sub_marker = "" - if _nous_logged_in and p.get("managed_nous_feature"): - sub_marker = " ★ Included with your Nous subscription" + if p.get("managed_nous_feature"): + if _nous_logged_in: + sub_marker = " ★ Included with your Nous subscription" + else: + sub_marker = " ★ via Nous Portal (login on select)" provider_choices.append(f"{p['name']}{badge}{tag}{configured}{sub_marker}") # Add skip option @@ -2558,7 +2563,26 @@ def _configure_provider( env_vars = provider.get("env_vars", []) managed_feature = provider.get("managed_nous_feature") - if provider.get("requires_nous_auth"): + # Nous-managed Tool Gateway backends are always listed (see + # _visible_providers), but only *activate* once the user has paid Nous + # Portal access. Selecting one runs an inline Portal login when needed — + # auth + entitlement only, no inference-provider switch and no bulk + # "enable all tools" prompt (that lives in `hermes model`). + if managed_feature: + from hermes_cli.nous_subscription import ensure_nous_portal_access + + if not ensure_nous_portal_access( + capability=f"{provider.get('name', 'the Nous Tool Gateway')}" + ): + _print_warning( + " Not enabled — Nous Portal paid access is required for this backend." + ) + return + + # Pure pre-auth UX rows (requires_nous_auth without a managed gateway + # feature) keep the old gate. Managed rows are handled by the inline + # login above, so don't double-check them here. + if provider.get("requires_nous_auth") and not managed_feature: features = get_nous_subscription_features(config, force_fresh=force_fresh) entitled = bool( features.account_info and features.account_info.paid_service_access is True @@ -2922,7 +2946,22 @@ def _reconfigure_provider( env_vars = provider.get("env_vars", []) managed_feature = provider.get("managed_nous_feature") - if provider.get("requires_nous_auth"): + # Same inline Nous Portal login + entitlement gate as _configure_provider: + # managed Tool Gateway backends only activate with paid Portal access. + if managed_feature: + from hermes_cli.nous_subscription import ensure_nous_portal_access + + if not ensure_nous_portal_access( + capability=f"{provider.get('name', 'the Nous Tool Gateway')}" + ): + _print_warning( + " Not enabled — Nous Portal paid access is required for this backend." + ) + return + + # Pure pre-auth UX rows keep the old gate; managed rows already handled + # by the inline login above. + if provider.get("requires_nous_auth") and not managed_feature: features = get_nous_subscription_features(config, force_fresh=force_fresh) entitled = bool( features.account_info and features.account_info.paid_service_access is True diff --git a/plugins/kanban/dashboard/dist/index.js b/plugins/kanban/dashboard/dist/index.js index c22c06c12934..451f3a0118c1 100644 --- a/plugins/kanban/dashboard/dist/index.js +++ b/plugins/kanban/dashboard/dist/index.js @@ -2600,6 +2600,13 @@ // input here to save vertical space in the common `scratch` case. const [workspaceKind, setWorkspaceKind] = useState("scratch"); const [workspacePath, setWorkspacePath] = useState(""); + // Goal-mode: when on, the dispatched worker runs the Ralph-style /goal + // loop — a judge re-checks the card after each turn and the worker keeps + // going in the same session until done, or the turn budget runs out + // (which blocks the card for review). goalMaxTurns is optional; blank + // = backend default. + const [goalMode, setGoalMode] = useState(false); + const [goalMaxTurns, setGoalMaxTurns] = useState(""); const submit = function () { const trimmed = title.trim(); @@ -2626,9 +2633,17 @@ } const wpTrim = workspacePath.trim(); if (wpTrim) body.workspace_path = wpTrim; + // Goal-mode toggle. Only send the keys when enabled so the request + // shape stays small and old dispatchers ignore it cleanly. + if (goalMode) { + body.goal_mode = true; + const gmt = parseInt(goalMaxTurns, 10); + if (Number.isFinite(gmt) && gmt > 0) body.goal_max_turns = gmt; + } props.onSubmit(body); setTitle(""); setAssignee(""); setPriority(0); setParent(""); setSkills(""); setWorkspaceKind("scratch"); setWorkspacePath(""); + setGoalMode(false); setGoalMaxTurns(""); }; const showPathInput = workspaceKind !== "scratch"; @@ -2685,6 +2700,29 @@ title: "Force-load these skills into the worker (in addition to the built-in kanban-worker).", className: "h-7 text-xs", }), + h("div", { className: "flex gap-2 items-center" }, + h("label", { + className: "flex items-center gap-1.5 text-xs cursor-pointer select-none", + title: "Goal mode: the worker keeps going in the same session until a judge agrees the card is done (or the turn budget runs out, which blocks it for review). Best for open-ended cards one shot rarely finishes.", + }, + h("input", { + type: "checkbox", + checked: goalMode, + onChange: function (e) { setGoalMode(!!e.target.checked); }, + className: "h-3.5 w-3.5 accent-current", + }), + tx(t, "goalMode", "goal mode"), + ), + goalMode ? h(Input, { + type: "number", + value: goalMaxTurns, + onChange: function (e) { setGoalMaxTurns(e.target.value); }, + placeholder: tx(t, "goalMaxTurns", "max turns (default 20)"), + className: "h-7 text-xs w-40", + title: "Turn budget for the goal loop. Blank = backend default (20).", + min: 1, + }) : null, + ), h("div", { className: "flex gap-2" }, h(Select, Object.assign({ value: workspaceKind, @@ -3161,6 +3199,12 @@ label: tx(i18n, "skills", "Skills"), value: t.skills.join(", "), }) : null, + t.goal_mode ? h(MetaRow, { + label: tx(i18n, "goalMode", "Goal mode"), + value: t.goal_max_turns + ? `on (max ${t.goal_max_turns} turns)` + : "on", + }) : null, t.created_by ? h(MetaRow, { label: tx(i18n, "createdBy", "Created by"), value: t.created_by }) : null, ), h(StatusActions, { diff --git a/plugins/kanban/dashboard/plugin_api.py b/plugins/kanban/dashboard/plugin_api.py index 0c2122c2a11c..2d792622fce0 100644 --- a/plugins/kanban/dashboard/plugin_api.py +++ b/plugins/kanban/dashboard/plugin_api.py @@ -581,6 +581,8 @@ class CreateTaskBody(BaseModel): idempotency_key: Optional[str] = None max_runtime_seconds: Optional[int] = None skills: Optional[list[str]] = None + goal_mode: bool = False + goal_max_turns: Optional[int] = None @router.post("/tasks") @@ -603,6 +605,8 @@ def create_task(payload: CreateTaskBody, board: Optional[str] = Query(None)): idempotency_key=payload.idempotency_key, max_runtime_seconds=payload.max_runtime_seconds, skills=payload.skills, + goal_mode=payload.goal_mode, + goal_max_turns=payload.goal_max_turns, ) task = kanban_db.get_task(conn, task_id) body: dict[str, Any] = {"task": _task_dict(task) if task else None} diff --git a/run_agent.py b/run_agent.py index 18ca748908d0..06bc71c2b71d 100644 --- a/run_agent.py +++ b/run_agent.py @@ -1295,6 +1295,42 @@ def _should_treat_stop_as_truncated( return not self._has_natural_response_ending(visible_text) + def _detect_malformed_tool_final_response( + self, + content: str, + finish_reason: str, + messages: Optional[list] = None, + ) -> Optional[str]: + """Detect non-empty final answers that are structurally broken. + + Empty/thinking-only finals already have a dedicated recovery path. This + catches a different local-model failure mode: after tool results, a + backend can report ``finish_reason=stop`` with visible text that ends in + a degenerate character run, making the loop treat a corrupted answer as + successful. + """ + if finish_reason != "stop" or self.api_mode != "chat_completions": + return None + recent_messages = messages or [] + if not any( + isinstance(msg, dict) and msg.get("role") == "tool" + for msg in recent_messages[-12:] + ): + return None + if not content: + return None + + visible_text = self._strip_think_blocks(content).strip() + if len(visible_text) < 80: + return None + + tail = visible_text[-160:] + repeated_tail = re.search(r"([^\s\w])\1{11,}\s*$", tail) + punctuation_tail = re.search(r"[!?.,;:_=+\-*/#~`|\\]{16,}\s*$", tail) + if repeated_tail or punctuation_tail: + return "degenerate repeated punctuation at final-response tail" + return None + def _looks_like_codex_intermediate_ack( self, user_message: str, @@ -1422,6 +1458,7 @@ def _drop_trailing_empty_response_scaffolding(self, messages: List[Dict]) -> Non and ( messages[-1].get("_empty_recovery_synthetic") or messages[-1].get("_empty_terminal_sentinel") + or messages[-1].get("_malformed_final_recovery_synthetic") ) ): messages.pop() diff --git a/scripts/release.py b/scripts/release.py index 30d0d84d6a07..87d3ad2b1421 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -49,6 +49,7 @@ "mathijs.vd.hurk@gmail.com": "mathijsvandenhurk", "drpelagik@gmail.com": "SeaXen", "lengr@users.noreply.github.com": "LengR", + "17255546+CharZhou@users.noreply.github.com": "CharZhou", "metalclaudbot@gmail.com": "HashClawAI", "tonybear55665566@gmail.com": "TonyPepeBear", "kaspersniels@gmail.com": "nielskaspers", @@ -80,6 +81,7 @@ "interstellar.consulting@gmail.com": "Interstellar-code", "33978413+Interstellar-code@users.noreply.github.com": "Interstellar-code", "tillfalko@gmail.com": "tillfalko", + "hi@fesalfayed.com": "fesalfayed", # teknium (multiple emails) "teknium1@gmail.com": "teknium1", "kenyon1977@gmail.com": "kenyonxu", @@ -1406,6 +1408,9 @@ "peter.yuqin@gmail.com": "WuKongAI-CMU", # PR #10082 (reject symlinked audio inputs) "sunil.nitie@gmail.com": "Sunil123135", # PR #31031 (Windows Docker Desktop compose) "weichangyuwcy@gmail.com": "ChyuWei", # PR #30987 (TUI TTS env var on voice off) + # batch salvage PR #35758 (perf micro-fixes) + "116212274+amathxbt@users.noreply.github.com": "amathxbt", # PR #22155 (cache tool_output_limits) + "takis312@hotmail.com": "ErnestHysa", # PRs #32636/#32708 (MCP asyncio.sleep + O(n^2) watcher drain) } diff --git a/skills/devops/kanban-orchestrator/SKILL.md b/skills/devops/kanban-orchestrator/SKILL.md index 25f634205c84..760f830710d6 100644 --- a/skills/devops/kanban-orchestrator/SKILL.md +++ b/skills/devops/kanban-orchestrator/SKILL.md @@ -178,6 +178,30 @@ Tell them what you created in plain prose, naming the actual profiles you used: **Tenant inheritance.** If `HERMES_TENANT` is set in your env, pass `tenant=os.environ.get("HERMES_TENANT")` on every `kanban_create` call so child tasks stay in the same namespace. +## Goal-mode cards (persistent workers) + +By default a dispatched worker gets **one shot** at its card: it does its work, calls `kanban_complete`/`kanban_block`, and exits. For open-ended cards where one turn rarely finishes the job, pass `goal_mode=True` to wrap that worker in a Ralph-style goal loop — the same engine behind the `/goal` slash command: + +```python +kanban_create( + title="Translate the full docs site to French", + body="Acceptance: every page translated, no English left, links intact.", + assignee="", + goal_mode=True, # judge re-checks the card after each turn + goal_max_turns=15, # optional budget (default 20) +)["task_id"] +``` + +How it behaves: +- After each worker turn, an auxiliary judge evaluates the worker's response against the card's **title + body** (treated as the acceptance criteria). +- Not done + budget remains → the worker keeps going **in the same session** (full context retained — not a fresh respawn). +- Worker calls `kanban_complete`/`kanban_block` itself → loop stops, normal lifecycle. +- Budget exhausted without completion → the card is **blocked** for human review (sticky), never a silent exit. + +When to use it: long, multi-step, or "keep going until X is true" cards. When NOT to: cheap one-shot cards (translation of a single string, a quick lookup) — the judge overhead isn't worth it, and the dispatcher's existing retry/circuit-breaker already handles transient worker failures. + +Write the body as **explicit acceptance criteria** — the judge is only as good as the goal text. "Translate the README" is weaker than "Translate every section of the README to French; no English sentences remain." + ## Recovering stuck workers When a worker profile keeps crashing, hallucinating, or getting blocked by its own mistakes (usually: wrong model, missing skill, broken credential), the kanban dashboard flags the task with a ⚠ badge and opens a **Recovery** section in the drawer. Three primary actions: diff --git a/tests/agent/test_anthropic_adapter.py b/tests/agent/test_anthropic_adapter.py index 7c7e8e333734..818a016f4ea5 100644 --- a/tests/agent/test_anthropic_adapter.py +++ b/tests/agent/test_anthropic_adapter.py @@ -1827,6 +1827,79 @@ def test_multi_turn_conversation_preserves_only_last(self): assert len(last_thinking) == 1 assert last_thinking[0]["signature"] == "sig_3" + def test_orphan_stripped_tool_use_demotes_dead_signed_thinking(self): + """Regression: extended-thinking + interrupted parallel tool batch. + + An assistant turn with a signed thinking block fires several parallel + tool_use blocks, but the batch is interrupted before every tool_result + comes back. On replay, the orphaned tool_use is stripped — which mutates + the turn and invalidates the thinking-block signature (it was computed + against the original, un-stripped content). Anthropic then rejects the + turn with HTTP 400 "thinking blocks in the latest assistant message + cannot be modified", a non-retryable error that crash-loops the gateway. + + The signed thinking block on the mutated latest turn must be demoted to + a plain text block so the turn replays cleanly. + """ + messages = [ + { + "role": "assistant", + "content": "", + "tool_calls": [ + {"id": "tc_kept", "function": {"name": "tool_a", "arguments": "{}"}}, + {"id": "tc_orphan", "function": {"name": "tool_b", "arguments": "{}"}}, + ], + "reasoning_details": [ + {"type": "thinking", "thinking": "Plan: call A and B.", "signature": "sig_dead"}, + ], + }, + # Only one of the two parallel tool_use blocks got a result back. + {"role": "tool", "tool_call_id": "tc_kept", "content": "result A"}, + ] + _, result = convert_messages_to_anthropic(messages) + assistant = next(m for m in result if m["role"] == "assistant") + blocks = assistant["content"] + + # No signed thinking block survives — the signature is dead. + assert not any( + isinstance(b, dict) and b.get("type") in {"thinking", "redacted_thinking"} + for b in blocks + ) + # The reasoning text is preserved as a text block (not silently lost). + text_contents = [b.get("text", "") for b in blocks if b.get("type") == "text"] + assert "Plan: call A and B." in text_contents + # The orphaned tool_use is gone; the answered one survives. + tool_use_ids = [b.get("id") for b in blocks if b.get("type") == "tool_use"] + assert tool_use_ids == ["tc_kept"] + # Internal bookkeeping flag must never leak into the API payload. + assert "_thinking_signature_invalidated" not in assistant + + def test_signed_thinking_preserved_when_no_tool_use_stripped(self): + """Control: an intact latest turn keeps its signed thinking verbatim. + + This guards against the orphan-strip fix over-firing — when no tool_use + is removed, the signature is still valid and must be replayed as-is. + """ + messages = [ + { + "role": "assistant", + "content": "", + "tool_calls": [ + {"id": "tc_1", "function": {"name": "tool_a", "arguments": "{}"}}, + ], + "reasoning_details": [ + {"type": "thinking", "thinking": "Valid plan.", "signature": "sig_live"}, + ], + }, + {"role": "tool", "tool_call_id": "tc_1", "content": "result A"}, + ] + _, result = convert_messages_to_anthropic(messages) + assistant = next(m for m in result if m["role"] == "assistant") + thinking = [b for b in assistant["content"] if b.get("type") == "thinking"] + assert len(thinking) == 1 + assert thinking[0]["signature"] == "sig_live" + assert "_thinking_signature_invalidated" not in assistant + # --------------------------------------------------------------------------- # Tool choice diff --git a/tests/cli/test_cli_status_bar.py b/tests/cli/test_cli_status_bar.py index f62287f622bd..47a78c570443 100644 --- a/tests/cli/test_cli_status_bar.py +++ b/tests/cli/test_cli_status_bar.py @@ -81,6 +81,29 @@ def test_build_status_bar_text_for_wide_terminal(self): assert "$0.06" not in text # cost hidden by default assert "15m" in text + def test_post_compression_sentinel_does_not_render_negative(self): + """Right after a compression, last_prompt_tokens is parked at the -1 + sentinel until the next API call reports real usage. The status bar + must clamp it to 0 instead of rendering "-1/200K" / "-1%". + """ + cli_obj = _attach_agent( + _make_cli(), + prompt_tokens=10_230, + completion_tokens=2_220, + total_tokens=12_450, + api_calls=7, + context_tokens=-1, + context_length=200_000, + ) + + snapshot = cli_obj._get_status_bar_snapshot() + assert snapshot["context_tokens"] == 0 + assert snapshot["context_percent"] == 0 + + text = cli_obj._build_status_bar_text(width=120) + assert "-1" not in text + assert "0/200K" in text + def test_input_height_counts_wide_characters_using_cell_width(self): cli_obj = _make_cli() diff --git a/tests/gateway/test_feishu.py b/tests/gateway/test_feishu.py index 0f65fd052be2..56770f55d761 100644 --- a/tests/gateway/test_feishu.py +++ b/tests/gateway/test_feishu.py @@ -6,6 +6,7 @@ import tempfile import time import unittest +from collections import OrderedDict from pathlib import Path from types import SimpleNamespace from typing import Dict @@ -4603,7 +4604,7 @@ def _build_adapter(self): adapter._bot_open_id = "ou_bot" adapter._bot_user_id = "" adapter._bot_name = "Hermes" - adapter._message_text_cache = {} + adapter._message_text_cache = OrderedDict() adapter._client = Mock() adapter._build_get_message_request = Mock(return_value=object()) return adapter diff --git a/tests/gateway/test_restart_notification.py b/tests/gateway/test_restart_notification.py index e7a931f8f8ad..56be2337031c 100644 --- a/tests/gateway/test_restart_notification.py +++ b/tests/gateway/test_restart_notification.py @@ -59,6 +59,7 @@ async def test_restart_command_writes_notify_file(tmp_path, monkeypatch): data = json.loads(notify_path.read_text()) assert data["platform"] == "telegram" assert data["chat_id"] == "42" + assert data["chat_type"] == "dm" assert "thread_id" not in data # no thread → omitted @@ -112,8 +113,7 @@ async def test_restart_command_preserves_thread_id(tmp_path, monkeypatch): runner, _adapter = make_restart_runner() runner.request_restart = MagicMock(return_value=True) - source = make_restart_source(chat_id="99") - source.thread_id = "topic_7" + source = make_restart_source(chat_id="99", thread_id="777") event = MessageEvent( text="/restart", @@ -125,7 +125,8 @@ async def test_restart_command_preserves_thread_id(tmp_path, monkeypatch): await runner._handle_restart_command(event) data = json.loads((tmp_path / ".restart_notify.json").read_text()) - assert data["thread_id"] == "topic_7" + assert data["chat_type"] == "dm" + assert data["thread_id"] == "777" @pytest.mark.asyncio @@ -258,17 +259,31 @@ async def test_send_home_channel_startup_notification_preserves_thread_metadata( platform=Platform.TELEGRAM, chat_id="parent-42", name="Ops Topic", - thread_id="topic-7", + thread_id="777", ) + # Declare the DM-topic lookup on the adapter CLASS, not the instance. + # _is_telegram_dm_topic_target resolves _get_dm_topic_info via type(adapter) + # so a MagicMock auto-attribute (instance-level) is intentionally ignored; + # a real adapter exposes the method on its class. Mirrors the fake-adapter + # pattern in test_telegram_topic_mode.py. + class _DmTopicAdapter(type(adapter)): + def _get_dm_topic_info(self, chat_id, thread_id): + return {"name": "Ops Topic"} + + adapter.__class__ = _DmTopicAdapter adapter.send = AsyncMock(return_value=SendResult(success=True, message_id="home")) delivered = await runner._send_home_channel_startup_notifications() - assert delivered == {("telegram", "parent-42", "topic-7")} + assert delivered == {("telegram", "parent-42", "777")} adapter.send.assert_called_once_with( "parent-42", "♻️ Gateway online — Hermes is back and ready.", - metadata={"thread_id": "topic-7"}, + metadata={ + "thread_id": "777", + "telegram_dm_topic_reply_fallback": True, + "direct_messages_topic_id": "777", + }, ) @@ -373,7 +388,8 @@ async def test_send_restart_notification_with_thread(tmp_path, monkeypatch): notify_path.write_text(json.dumps({ "platform": "telegram", "chat_id": "99", - "thread_id": "topic_7", + "chat_type": "dm", + "thread_id": "777", })) runner, adapter = make_restart_runner() @@ -381,9 +397,13 @@ async def test_send_restart_notification_with_thread(tmp_path, monkeypatch): delivered_target = await runner._send_restart_notification() - assert delivered_target == ("telegram", "99", "topic_7") + assert delivered_target == ("telegram", "99", "777") call_args = adapter.send.call_args - assert call_args[1]["metadata"] == {"thread_id": "topic_7"} + assert call_args[1]["metadata"] == { + "thread_id": "777", + "telegram_dm_topic_reply_fallback": True, + "direct_messages_topic_id": "777", + } assert not notify_path.exists() diff --git a/tests/gateway/test_stop_thread_sibling.py b/tests/gateway/test_stop_thread_sibling.py new file mode 100644 index 000000000000..d8076ba6e550 --- /dev/null +++ b/tests/gateway/test_stop_thread_sibling.py @@ -0,0 +1,158 @@ +"""Regression tests: /stop can interrupt a sibling participant's run in a +per-user thread. + +When ``thread_sessions_per_user=True``, each participant in a thread gets an +isolated session key (``...:{thread_id}:{user_id}``). A run another user +started lives under a different key, so the caller's own ``/stop`` used to find +nothing and reply "no active task to stop". Authorized users should be able to +stop any run in the same thread. +""" + +import pytest + +from gateway.run import GatewayRunner, _AGENT_PENDING_SENTINEL, _INTERRUPT_REASON_STOP +from gateway.session import SessionSource, build_session_key +from gateway.platforms.base import Platform, MessageEvent, MessageType + + +class _FakeAgent: + pass + + +def _thread_source(uid, thread_id="thr1", chat_id="chan1"): + return SessionSource( + platform=Platform.DISCORD, + chat_type="forum", + chat_id=chat_id, + thread_id=thread_id, + user_id=uid, + ) + + +def _per_user_key(uid, thread_id="thr1", chat_id="chan1"): + return build_session_key( + _thread_source(uid, thread_id, chat_id), + thread_sessions_per_user=True, + ) + + +# --------------------------------------------------------------------------- +# _sibling_thread_run_keys +# --------------------------------------------------------------------------- + + +def test_sibling_finds_other_users_run_in_same_thread(): + runner = object.__new__(GatewayRunner) + key_a = _per_user_key("userA") + key_b = _per_user_key("userB") + runner._running_agents = {key_b: _FakeAgent()} + assert runner._sibling_thread_run_keys(_thread_source("userA"), key_a) == [key_b] + + +def test_sibling_excludes_callers_own_key(): + runner = object.__new__(GatewayRunner) + key_a = _per_user_key("userA") + key_b = _per_user_key("userB") + runner._running_agents = {key_a: _FakeAgent(), key_b: _FakeAgent()} + assert runner._sibling_thread_run_keys(_thread_source("userA"), key_a) == [key_b] + + +def test_sibling_skips_pending_sentinel(): + runner = object.__new__(GatewayRunner) + key_a = _per_user_key("userA") + key_b = _per_user_key("userB") + runner._running_agents = {key_b: _AGENT_PENDING_SENTINEL} + assert runner._sibling_thread_run_keys(_thread_source("userA"), key_a) == [] + + +def test_sibling_does_not_match_different_thread_same_chat(): + # thr1 caller must not match a run in thr11 (prefix-collision guard). + runner = object.__new__(GatewayRunner) + key_a = _per_user_key("userA", thread_id="thr1") + key_b_other = _per_user_key("userB", thread_id="thr11") + runner._running_agents = {key_b_other: _FakeAgent()} + assert runner._sibling_thread_run_keys(_thread_source("userA"), key_a) == [] + + +def test_sibling_returns_empty_for_non_thread_source(): + # Non-thread group/channel must NOT trigger the cross-user fallback. + runner = object.__new__(GatewayRunner) + nonthread = SessionSource( + platform=Platform.DISCORD, chat_type="group", chat_id="chan1", user_id="userA" + ) + grp_b = build_session_key( + SessionSource( + platform=Platform.DISCORD, chat_type="group", chat_id="chan1", user_id="userB" + ) + ) + runner._running_agents = {grp_b: _FakeAgent()} + assert runner._sibling_thread_run_keys(nonthread, "agent:main:discord:group:chan1:userA") == [] + + +# --------------------------------------------------------------------------- +# _handle_stop_command fallback path +# --------------------------------------------------------------------------- + + +class _StoreEntry: + def __init__(self, session_key): + self.session_key = session_key + + +class _FakeStore: + def __init__(self, session_key): + self._key = session_key + + def get_or_create_session(self, source): + return _StoreEntry(self._key) + + +@pytest.mark.asyncio +async def test_stop_interrupts_sibling_thread_run_when_authorized(monkeypatch): + runner = object.__new__(GatewayRunner) + key_a = _per_user_key("userA") + key_b = _per_user_key("userB") + runner._running_agents = {key_b: _FakeAgent()} + runner.session_store = _FakeStore(key_a) + + interrupted = [] + + async def _fake_interrupt(session_key, source, *, interrupt_reason, invalidation_reason): + interrupted.append((session_key, interrupt_reason, invalidation_reason)) + + runner._interrupt_and_clear_session = _fake_interrupt + runner._is_user_authorized = lambda source: True + + event = MessageEvent( + text="/stop", message_type=MessageType.TEXT, source=_thread_source("userA") + ) + result = await runner._handle_stop_command(event) + + assert interrupted == [(key_b, _INTERRUPT_REASON_STOP, "stop_command_thread_sibling")] + # EphemeralReply or str — both carry the "stopped" message, not "no_active". + assert "no active" not in str(getattr(result, "text", result)).lower() + + +@pytest.mark.asyncio +async def test_stop_does_not_interrupt_sibling_when_unauthorized(monkeypatch): + runner = object.__new__(GatewayRunner) + key_a = _per_user_key("userA") + key_b = _per_user_key("userB") + runner._running_agents = {key_b: _FakeAgent()} + runner.session_store = _FakeStore(key_a) + + interrupted = [] + + async def _fake_interrupt(session_key, source, *, interrupt_reason, invalidation_reason): + interrupted.append(session_key) + + runner._interrupt_and_clear_session = _fake_interrupt + runner._is_user_authorized = lambda source: False + + event = MessageEvent( + text="/stop", message_type=MessageType.TEXT, source=_thread_source("userA") + ) + result = await runner._handle_stop_command(event) + + assert interrupted == [] + assert "no active" in str(getattr(result, "text", result)).lower() diff --git a/tests/gateway/test_update_command.py b/tests/gateway/test_update_command.py index 154603898b34..e3f74694bdf5 100644 --- a/tests/gateway/test_update_command.py +++ b/tests/gateway/test_update_command.py @@ -210,6 +210,7 @@ async def test_writes_pending_marker(self, tmp_path): data = json.loads(pending_path.read_text()) assert data["platform"] == "telegram" assert data["chat_id"] == "99999" + assert data["chat_type"] == "dm" assert "timestamp" in data assert not (hermes_home / ".update_exit_code").exists() @@ -469,6 +470,7 @@ async def test_sends_notification_with_thread_metadata(self, tmp_path): pending = { "platform": "telegram", "chat_id": "67890", + "chat_type": "dm", "thread_id": "777", "user_id": "12345", } @@ -482,7 +484,11 @@ async def test_sends_notification_with_thread_metadata(self, tmp_path): with patch("gateway.run._hermes_home", hermes_home): await runner._send_update_notification() - assert mock_adapter.send.call_args.kwargs["metadata"] == {"thread_id": "777"} + assert mock_adapter.send.call_args.kwargs["metadata"] == { + "thread_id": "777", + "telegram_dm_topic_reply_fallback": True, + "direct_messages_topic_id": "777", + } @pytest.mark.asyncio async def test_strips_ansi_codes(self, tmp_path): diff --git a/tests/hermes_cli/test_curses_arrow_keys.py b/tests/hermes_cli/test_curses_arrow_keys.py new file mode 100644 index 000000000000..c1bafbd8c3d4 --- /dev/null +++ b/tests/hermes_cli/test_curses_arrow_keys.py @@ -0,0 +1,102 @@ +"""Regression tests for arrow-key decoding in the curses menus. + +Root cause these guard against: on many terminals/terminfo entries, cursor +keys are delivered to ``getch()`` as raw CSI/SS3 escape byte sequences +(``27, 91, 66`` for arrow-down) even when ``keypad(True)`` is set. The menus +used to treat the leading ``27`` as ESC/cancel, which dumped the setup wizard's +provider/model picker into its numbered "Select [1-N]" fallback the instant a +user pressed up or down. +""" +import curses + +from hermes_cli.curses_ui import ( + NAV_CANCEL, + NAV_DOWN, + NAV_NONE, + NAV_SELECT, + NAV_UP, + read_menu_key, +) + + +class FakeStdscr: + """Minimal stdscr stand-in that replays a queue of getch() byte returns. + + ``getch`` pops from ``keys``; an empty queue yields ``-1`` (matching curses + non-blocking behavior). ``timeout`` is recorded but otherwise inert. + """ + + def __init__(self, keys): + self.keys = list(keys) + self.timeouts = [] + + def getch(self): + return self.keys.pop(0) if self.keys else -1 + + def timeout(self, ms): + self.timeouts.append(ms) + + +def test_raw_csi_arrow_down_decodes_to_down(): + # ESC [ B -> down, NOT cancel + assert read_menu_key(FakeStdscr([27, ord("["), ord("B")])) == NAV_DOWN + + +def test_raw_csi_arrow_up_decodes_to_up(): + # ESC [ A -> up + assert read_menu_key(FakeStdscr([27, ord("["), ord("A")])) == NAV_UP + + +def test_raw_ss3_arrow_keys_decode(): + # Application cursor mode: ESC O B / ESC O A + assert read_menu_key(FakeStdscr([27, ord("O"), ord("B")])) == NAV_DOWN + assert read_menu_key(FakeStdscr([27, ord("O"), ord("A")])) == NAV_UP + + +def test_translated_key_constants_still_work(): + assert read_menu_key(FakeStdscr([curses.KEY_DOWN])) == NAV_DOWN + assert read_menu_key(FakeStdscr([curses.KEY_UP])) == NAV_UP + + +def test_vim_keys(): + assert read_menu_key(FakeStdscr([ord("j")])) == NAV_DOWN + assert read_menu_key(FakeStdscr([ord("k")])) == NAV_UP + + +def test_lone_escape_is_cancel(): + # ESC with no continuation byte (getch returns -1) -> genuine cancel. + assert read_menu_key(FakeStdscr([27])) == NAV_CANCEL + + +def test_q_is_cancel(): + assert read_menu_key(FakeStdscr([ord("q")])) == NAV_CANCEL + + +def test_enter_variants_select(): + assert read_menu_key(FakeStdscr([10])) == NAV_SELECT + assert read_menu_key(FakeStdscr([13])) == NAV_SELECT + assert read_menu_key(FakeStdscr([curses.KEY_ENTER])) == NAV_SELECT + + +def test_unhandled_csi_sequence_is_consumed_and_ignored(): + # Delete key (ESC [ 3 ~): must be swallowed whole and map to NAV_NONE so + # its tail bytes don't leak into a subsequent input() call. + fake = FakeStdscr([27, ord("["), ord("3"), ord("~"), ord("X")]) + assert read_menu_key(fake) == NAV_NONE + # The trailing 'X' (a genuinely separate keypress) must remain unconsumed. + assert fake.keys == [ord("X")] + + +def test_home_end_csi_sequences_ignored(): + # ESC [ H (Home) and ESC [ F (End) -> NAV_NONE, fully consumed. + assert read_menu_key(FakeStdscr([27, ord("["), ord("H")])) == NAV_NONE + assert read_menu_key(FakeStdscr([27, ord("["), ord("F")])) == NAV_NONE + + +def test_escape_uses_short_timeout_then_restores_blocking(): + fake = FakeStdscr([27, ord("["), ord("B")]) + read_menu_key(fake) + # A short positive timeout is set to wait for the continuation byte, then + # blocking mode (-1) is restored. + assert fake.timeouts[0] > 0 + assert fake.timeouts[-1] == -1 diff --git a/tests/hermes_cli/test_custom_provider_model_switch.py b/tests/hermes_cli/test_custom_provider_model_switch.py index 0f3a76a1ab95..45415f50fc6a 100644 --- a/tests/hermes_cli/test_custom_provider_model_switch.py +++ b/tests/hermes_cli/test_custom_provider_model_switch.py @@ -45,7 +45,7 @@ def test_saved_model_still_probes_endpoint(self, config_home): } with patch("hermes_cli.models.fetch_api_models", return_value=["model-A", "model-B"]) as mock_fetch, \ - patch.dict("sys.modules", {"simple_term_menu": None}), \ + patch("hermes_cli.curses_ui.curses_radiolist", side_effect=ImportError), \ patch("builtins.input", return_value="2"), \ patch("builtins.print"): _model_flow_named_custom({}, provider_info) @@ -70,7 +70,7 @@ def test_can_switch_to_different_model(self, config_home): } with patch("hermes_cli.models.fetch_api_models", return_value=["model-A", "model-B"]), \ - patch.dict("sys.modules", {"simple_term_menu": None}), \ + patch("hermes_cli.curses_ui.curses_radiolist", side_effect=ImportError), \ patch("builtins.input", return_value="2"), \ patch("builtins.print"): _model_flow_named_custom({}, provider_info) @@ -116,7 +116,7 @@ def test_no_saved_model_still_works(self, config_home): } with patch("hermes_cli.models.fetch_api_models", return_value=["model-X"]), \ - patch.dict("sys.modules", {"simple_term_menu": None}), \ + patch("hermes_cli.curses_ui.curses_radiolist", side_effect=ImportError), \ patch("builtins.input", return_value="1"), \ patch("builtins.print"): _model_flow_named_custom({}, provider_info) @@ -140,7 +140,7 @@ def test_api_mode_set_from_provider_info(self, config_home): } with patch("hermes_cli.models.fetch_api_models", return_value=["claude-3"]) as mock_fetch, \ - patch.dict("sys.modules", {"simple_term_menu": None}), \ + patch("hermes_cli.curses_ui.curses_radiolist", side_effect=ImportError), \ patch("builtins.input", return_value="1"), \ patch("builtins.print"): _model_flow_named_custom({}, provider_info) @@ -173,7 +173,7 @@ def test_api_mode_cleared_when_not_specified(self, config_home): } with patch("hermes_cli.models.fetch_api_models", return_value=["llama-3"]), \ - patch.dict("sys.modules", {"simple_term_menu": None}), \ + patch("hermes_cli.curses_ui.curses_radiolist", side_effect=ImportError), \ patch("builtins.input", return_value="1"), \ patch("builtins.print"): _model_flow_named_custom({}, provider_info) @@ -210,7 +210,7 @@ def test_env_template_api_key_is_preserved_in_model_config(self, config_home, mo } with patch("hermes_cli.models.fetch_api_models", return_value=["qwen3.6-35b-fast"]) as mock_fetch, \ - patch.dict("sys.modules", {"simple_term_menu": None}), \ + patch("hermes_cli.curses_ui.curses_radiolist", side_effect=ImportError), \ patch("builtins.input", return_value="1"), \ patch("builtins.print"): _model_flow_named_custom({}, provider_info) @@ -251,7 +251,7 @@ def test_key_env_custom_provider_persists_reference_not_secret(self, config_home } with patch("hermes_cli.models.fetch_api_models", return_value=["qwen3.6-35b-fast"]), \ - patch.dict("sys.modules", {"simple_term_menu": None}), \ + patch("hermes_cli.curses_ui.curses_radiolist", side_effect=ImportError), \ patch("builtins.input", return_value="1"), \ patch("builtins.print"): _model_flow_named_custom({}, provider_info) @@ -309,7 +309,7 @@ def _pick_neuralwatt(labels, default=0): side_effect=_pick_neuralwatt), \ patch("hermes_cli.models.fetch_api_models", return_value=["qwen3.6-35b-fast"]) as mock_fetch, \ - patch.dict("sys.modules", {"simple_term_menu": None}), \ + patch("hermes_cli.curses_ui.curses_radiolist", side_effect=ImportError), \ patch("builtins.input", return_value="1"), \ patch("builtins.print"): select_provider_and_model() @@ -422,7 +422,7 @@ def _pick_neuralwatt(labels, default=0): side_effect=_pick_neuralwatt), \ patch("hermes_cli.models.fetch_api_models", return_value=["qwen3.6-35b-fast"]) as mock_fetch, \ - patch.dict("sys.modules", {"simple_term_menu": None}), \ + patch("hermes_cli.curses_ui.curses_radiolist", side_effect=ImportError), \ patch("builtins.input", return_value="1"), \ patch("builtins.print"): select_provider_and_model() @@ -486,7 +486,7 @@ def test_key_env_providers_dict_entry_does_not_add_api_key( "hermes_cli.models.fetch_api_models", return_value=["claude-opus-4-7"], ) as mock_fetch, \ - patch.dict("sys.modules", {"simple_term_menu": None}), \ + patch("hermes_cli.curses_ui.curses_radiolist", side_effect=ImportError), \ patch("builtins.input", return_value="1"), \ patch("builtins.print"): _model_flow_named_custom({}, provider_info) @@ -551,7 +551,7 @@ def test_key_env_providers_dict_preserves_existing_api_key( "hermes_cli.models.fetch_api_models", return_value=["claude-opus-4-7"], ), \ - patch.dict("sys.modules", {"simple_term_menu": None}), \ + patch("hermes_cli.curses_ui.curses_radiolist", side_effect=ImportError), \ patch("builtins.input", return_value="1"), \ patch("builtins.print"): _model_flow_named_custom({}, provider_info) diff --git a/tests/hermes_cli/test_kanban_goal_mode.py b/tests/hermes_cli/test_kanban_goal_mode.py new file mode 100644 index 000000000000..173174374831 --- /dev/null +++ b/tests/hermes_cli/test_kanban_goal_mode.py @@ -0,0 +1,300 @@ +"""Tests for kanban goal_mode — per-card Ralph-style goal loop. + +Covers three layers: + +1. DB: goal_mode / goal_max_turns persist through create_task + from_row, + and a legacy DB (without the columns) migrates cleanly. +2. Spawn: _default_spawn sets the HERMES_KANBAN_GOAL_MODE env vars only + when the card opts in. +3. Loop: goals.run_kanban_goal_loop continuation / completion / budget + behaviour, driven entirely through injected callbacks (no live model). +""" + +from __future__ import annotations + +import sqlite3 +from pathlib import Path + +import pytest + +from hermes_cli import kanban_db as kb +from hermes_cli import goals + + +@pytest.fixture +def kanban_home(tmp_path, monkeypatch): + home = tmp_path / ".hermes" + home.mkdir() + monkeypatch.setenv("HERMES_HOME", str(home)) + monkeypatch.setattr(Path, "home", lambda: tmp_path) + kb.init_db() + return home + + +# --------------------------------------------------------------------------- +# DB layer +# --------------------------------------------------------------------------- + +def test_goal_mode_defaults_off(kanban_home): + with kb.connect() as conn: + tid = kb.create_task(conn, title="plain task", assignee="worker") + task = kb.get_task(conn, tid) + assert task.goal_mode is False + assert task.goal_max_turns is None + + +def test_goal_mode_persists(kanban_home): + with kb.connect() as conn: + tid = kb.create_task( + conn, + title="open-ended task", + assignee="worker", + goal_mode=True, + goal_max_turns=7, + ) + task = kb.get_task(conn, tid) + assert task.goal_mode is True + assert task.goal_max_turns == 7 + + +def test_goal_mode_without_max_turns(kanban_home): + with kb.connect() as conn: + tid = kb.create_task( + conn, title="t", assignee="worker", goal_mode=True + ) + task = kb.get_task(conn, tid) + assert task.goal_mode is True + assert task.goal_max_turns is None + + +def test_legacy_db_migrates_goal_columns(tmp_path, monkeypatch): + """A tasks table created without goal columns must gain them on init.""" + home = tmp_path / ".hermes" + home.mkdir() + monkeypatch.setenv("HERMES_HOME", str(home)) + monkeypatch.setattr(Path, "home", lambda: tmp_path) + + db_path = kb.kanban_db_path() + db_path.parent.mkdir(parents=True, exist_ok=True) + # Minimal legacy schema: tasks table missing goal_mode / goal_max_turns. + legacy = sqlite3.connect(db_path) + legacy.execute( + """ + CREATE TABLE tasks ( + id TEXT PRIMARY KEY, + title TEXT NOT NULL, + body TEXT, + assignee TEXT, + status TEXT NOT NULL DEFAULT 'ready', + priority INTEGER NOT NULL DEFAULT 0, + created_by TEXT, + created_at INTEGER NOT NULL, + started_at INTEGER, + completed_at INTEGER, + workspace_kind TEXT NOT NULL DEFAULT 'scratch', + workspace_path TEXT, + claim_lock TEXT, + claim_expires INTEGER + ) + """ + ) + legacy.execute( + "INSERT INTO tasks (id, title, status, priority, created_at, workspace_kind) " + "VALUES ('legacy1', 'old', 'ready', 0, 1, 'scratch')" + ) + legacy.commit() + legacy.close() + + # init_db runs the additive migration. + kb.init_db() + with kb.connect() as conn: + cols = {r["name"] for r in conn.execute("PRAGMA table_info(tasks)")} + assert "goal_mode" in cols + assert "goal_max_turns" in cols + task = kb.get_task(conn, "legacy1") + # Existing row keeps the safe default. + assert task.goal_mode is False + assert task.goal_max_turns is None + + +# --------------------------------------------------------------------------- +# Spawn env +# --------------------------------------------------------------------------- + +def test_spawn_sets_goal_env_only_when_enabled(kanban_home, monkeypatch): + captured = {} + + class _FakeProc: + pid = 4242 + + def _fake_popen(cmd, **kwargs): + captured["env"] = kwargs.get("env", {}) + return _FakeProc() + + monkeypatch.setattr("subprocess.Popen", _fake_popen) + # Avoid the kanban-worker skill probe touching the real skills dir. + monkeypatch.setattr(kb, "_kanban_worker_skill_available", lambda home: False) + + with kb.connect() as conn: + tid = kb.create_task( + conn, + title="goal task", + assignee="default", + goal_mode=True, + goal_max_turns=5, + ) + task = kb.get_task(conn, tid) + + kb._default_spawn(task, str(kanban_home)) + env = captured["env"] + assert env.get("HERMES_KANBAN_GOAL_MODE") == "1" + assert env.get("HERMES_KANBAN_GOAL_MAX_TURNS") == "5" + + +def test_spawn_no_goal_env_for_plain_task(kanban_home, monkeypatch): + captured = {} + + class _FakeProc: + pid = 4243 + + def _fake_popen(cmd, **kwargs): + captured["env"] = kwargs.get("env", {}) + return _FakeProc() + + monkeypatch.setattr("subprocess.Popen", _fake_popen) + monkeypatch.setattr(kb, "_kanban_worker_skill_available", lambda home: False) + + with kb.connect() as conn: + tid = kb.create_task(conn, title="plain", assignee="default") + task = kb.get_task(conn, tid) + + kb._default_spawn(task, str(kanban_home)) + env = captured["env"] + assert "HERMES_KANBAN_GOAL_MODE" not in env + assert "HERMES_KANBAN_GOAL_MAX_TURNS" not in env + + +# --------------------------------------------------------------------------- +# Goal loop logic (callback-injected, no live model) +# --------------------------------------------------------------------------- + +def _patch_judge(monkeypatch, verdicts): + """Make judge_goal return a scripted sequence of verdicts.""" + seq = list(verdicts) + + def _fake_judge(goal, response, subgoals=None): + v = seq.pop(0) if seq else "done" + return v, f"scripted:{v}", False + + monkeypatch.setattr(goals, "judge_goal", _fake_judge) + + +def test_loop_stops_when_worker_already_completed(monkeypatch): + # Worker called kanban_complete on its first turn — no judging needed. + _patch_judge(monkeypatch, ["continue"]) # should never be consulted + turns = [] + + res = goals.run_kanban_goal_loop( + task_id="t1", + goal_text="do the thing", + run_turn=lambda p: turns.append(p) or "x", + task_status_fn=lambda: "done", + block_fn=lambda r: pytest.fail("should not block"), + first_response="done already", + ) + assert res["outcome"] == "completed_by_worker" + assert turns == [] # no extra turns + + +def test_loop_continues_then_worker_completes(monkeypatch): + _patch_judge(monkeypatch, ["continue", "continue"]) + statuses = iter(["running", "running", "done"]) + turns = [] + + res = goals.run_kanban_goal_loop( + task_id="t2", + goal_text="ship feature", + run_turn=lambda p: turns.append(p) or f"turn{len(turns)}", + task_status_fn=lambda: next(statuses), + block_fn=lambda r: pytest.fail("should not block"), + max_turns=10, + first_response="started", + ) + assert res["outcome"] == "completed_by_worker" + # Two continuation turns fed before the worker completed. + assert len(turns) == 2 + assert all("not done yet" in p for p in turns) + + +def test_loop_blocks_on_budget_exhaustion(monkeypatch): + _patch_judge(monkeypatch, ["continue"] * 10) + blocked = {} + + def _block(reason): + blocked["reason"] = reason + + res = goals.run_kanban_goal_loop( + task_id="t3", + goal_text="endless task", + run_turn=lambda p: "still going", + task_status_fn=lambda: "running", + block_fn=_block, + max_turns=3, + first_response="turn1", + ) + assert res["outcome"] == "blocked_budget" + assert res["turns_used"] == 3 + assert "turn budget" in blocked["reason"].lower() + + +def test_loop_finalize_nudge_when_judge_done_but_open(monkeypatch): + # Judge says done, but worker never terminated → one finalize nudge, + # then worker completes. + _patch_judge(monkeypatch, ["done", "done"]) + statuses = iter(["running", "done"]) + turns = [] + + res = goals.run_kanban_goal_loop( + task_id="t4", + goal_text="task", + run_turn=lambda p: turns.append(p) or "ok", + task_status_fn=lambda: next(statuses), + block_fn=lambda r: pytest.fail("should not block"), + max_turns=10, + first_response="looks done", + ) + assert res["outcome"] == "completed_by_worker" + assert len(turns) == 1 + assert "still open" in turns[0] + + +def test_loop_blocks_when_judge_done_but_never_finalizes(monkeypatch): + # Judge keeps saying done, worker never calls kanban_complete → block + # after the single finalize nudge. + _patch_judge(monkeypatch, ["done", "done"]) + blocked = {} + + res = goals.run_kanban_goal_loop( + task_id="t5", + goal_text="task", + run_turn=lambda p: "still not finalizing", + task_status_fn=lambda: "running", + block_fn=lambda r: blocked.update(reason=r), + max_turns=10, + first_response="looks done", + ) + assert res["outcome"] == "blocked_budget" + assert "finalize" in blocked["reason"].lower() + + +def test_loop_stops_if_task_reclaimed(monkeypatch): + _patch_judge(monkeypatch, ["continue"]) + res = goals.run_kanban_goal_loop( + task_id="t6", + goal_text="task", + run_turn=lambda p: pytest.fail("should not run a turn"), + task_status_fn=lambda: "archived", + block_fn=lambda r: pytest.fail("should not block"), + first_response="x", + ) + assert res["outcome"] == "stopped" diff --git a/tests/hermes_cli/test_model_provider_persistence.py b/tests/hermes_cli/test_model_provider_persistence.py index aef758f099ec..75eb5b8dc708 100644 --- a/tests/hermes_cli/test_model_provider_persistence.py +++ b/tests/hermes_cli/test_model_provider_persistence.py @@ -191,14 +191,12 @@ def test_named_custom_provider_preserves_explicit_api_mode(self, config_home): } # Patch fetch_api_models so the named custom flow returns one model; - # patch simple_term_menu to force the input() fallback; patch input to - # auto-select the first model from the fallback prompt. - fake_menu_module = MagicMock() - fake_menu_module.TerminalMenu.side_effect = OSError("no tty in test") + # force the curses menu to error so the input() fallback runs; patch + # input to auto-select the first model from the fallback prompt. with patch("hermes_cli.auth._save_model_choice"), \ patch("hermes_cli.auth.deactivate_provider"), \ patch("hermes_cli.models.fetch_api_models", return_value=["gpt-5.4"]), \ - patch.dict("sys.modules", {"simple_term_menu": fake_menu_module}), \ + patch("hermes_cli.curses_ui.curses_radiolist", side_effect=OSError("no tty in test")), \ patch("builtins.input", return_value="1"): _model_flow_named_custom({}, provider_info) diff --git a/tests/hermes_cli/test_nous_subscription.py b/tests/hermes_cli/test_nous_subscription.py index 561602c0ac69..e25fd86f147b 100644 --- a/tests/hermes_cli/test_nous_subscription.py +++ b/tests/hermes_cli/test_nous_subscription.py @@ -321,3 +321,70 @@ def test_apply_nous_managed_defaults_preserves_existing_video_gen_section(monkey assert config["video_gen"]["use_gateway"] is True # Pre-existing keys should be preserved assert config["video_gen"]["model"] == "pixverse-v6" + + +# --------------------------------------------------------------------------- +# ensure_nous_portal_access — inline login gate for `hermes tools` +# --------------------------------------------------------------------------- + + +def test_ensure_nous_portal_access_fast_path_when_already_paid(monkeypatch): + """Already-entitled users return True without any login prompt.""" + login_called = {"v": False} + + monkeypatch.setattr( + ns, "get_nous_portal_account_info", + lambda **kw: _account(logged_in=True, paid=True), + ) + + def _login(**kw): + login_called["v"] = True + return True + + monkeypatch.setattr(ns, "_run_nous_portal_login_only", _login) + + assert ns.ensure_nous_portal_access() is True + assert login_called["v"] is False + + +def test_ensure_nous_portal_access_logs_in_then_grants(monkeypatch): + """Logged-out user logs in, then entitlement re-check shows paid access.""" + states = iter([ + _account(logged_in=False, paid=None), # initial check + _account(logged_in=True, paid=True), # after login + ]) + monkeypatch.setattr( + ns, "get_nous_portal_account_info", lambda **kw: next(states), + ) + monkeypatch.setattr(ns, "_run_nous_portal_login_only", lambda **kw: True) + + assert ns.ensure_nous_portal_access() is True + + +def test_ensure_nous_portal_access_returns_false_when_login_declined(monkeypatch): + monkeypatch.setattr( + ns, "get_nous_portal_account_info", + lambda **kw: _account(logged_in=False, paid=None), + ) + monkeypatch.setattr(ns, "_run_nous_portal_login_only", lambda **kw: False) + + assert ns.ensure_nous_portal_access() is False + + +def test_ensure_nous_portal_access_false_when_logged_in_but_unpaid(monkeypatch): + """Logged in already but no paid access — no login attempt, returns False.""" + login_called = {"v": False} + monkeypatch.setattr( + ns, "get_nous_portal_account_info", + lambda **kw: _account(logged_in=True, paid=False), + ) + + def _login(**kw): + login_called["v"] = True + return True + + monkeypatch.setattr(ns, "_run_nous_portal_login_only", _login) + + assert ns.ensure_nous_portal_access() is False + # Already logged in, so no device-code login should be attempted. + assert login_called["v"] is False diff --git a/tests/hermes_cli/test_reasoning_effort_menu.py b/tests/hermes_cli/test_reasoning_effort_menu.py index 3d360a4f2f6f..79063587f0b9 100644 --- a/tests/hermes_cli/test_reasoning_effort_menu.py +++ b/tests/hermes_cli/test_reasoning_effort_menu.py @@ -1,24 +1,15 @@ -import sys -import types - - from hermes_cli.main import _prompt_reasoning_effort_selection -class _FakeTerminalMenu: - last_choices = None - - def __init__(self, choices, **kwargs): - _FakeTerminalMenu.last_choices = choices - self._cursor_index = kwargs.get("cursor_index") - - def show(self): - return self._cursor_index +def test_reasoning_menu_orders_minimal_before_low(monkeypatch): + captured = {} + def _fake_radiolist(title, items, *, selected=0, cancel_returns=None, description=None): + captured["items"] = items + captured["selected"] = selected + return selected # pick the pre-selected (current) entry -def test_reasoning_menu_orders_minimal_before_low(monkeypatch): - fake_module = types.SimpleNamespace(TerminalMenu=_FakeTerminalMenu) - monkeypatch.setitem(sys.modules, "simple_term_menu", fake_module) + monkeypatch.setattr("hermes_cli.curses_ui.curses_radiolist", _fake_radiolist) selected = _prompt_reasoning_effort_selection( ["low", "minimal", "medium", "high"], @@ -26,9 +17,9 @@ def test_reasoning_menu_orders_minimal_before_low(monkeypatch): ) assert selected == "medium" - assert _FakeTerminalMenu.last_choices[:4] == [ - " minimal", - " low", - " medium ← currently in use", - " high", + assert captured["items"][:4] == [ + "minimal", + "low", + "medium ← currently in use", + "high", ] diff --git a/tests/hermes_cli/test_setup_menu_curses_migration.py b/tests/hermes_cli/test_setup_menu_curses_migration.py new file mode 100644 index 000000000000..9f6560b1e62d --- /dev/null +++ b/tests/hermes_cli/test_setup_menu_curses_migration.py @@ -0,0 +1,84 @@ +"""Regression tests confirming the setup model/provider/reasoning pickers route +through the shared curses radiolist (ESC + arrow-key handling that works across +terminals, incl. Ghostty) instead of simple_term_menu. + +Guards against silently regressing back to simple_term_menu, whose ESC/arrow +handling was unreliable in `hermes setup` (the provider->model sub-menu). +""" +from unittest.mock import patch + + +def test_prompt_model_selection_uses_curses_radiolist(): + from hermes_cli.auth import _prompt_model_selection + + seen = {} + + def _fake(title, items, *, selected=0, cancel_returns=None, description=None): + seen["title"] = title + seen["items"] = items + return 1 # pick second model + + with patch("hermes_cli.curses_ui.curses_radiolist", side_effect=_fake), \ + patch("builtins.print"): + result = _prompt_model_selection(["model-a", "model-b"]) + + assert result == "model-b" + assert seen["title"] == "Select default model:" + # Items are the models plus the custom/skip entries. + assert seen["items"][:2] == ["model-a", "model-b"] + assert "Skip (keep current)" in seen["items"] + + +def test_prompt_model_selection_esc_cancels(): + from hermes_cli.auth import _prompt_model_selection + + # curses_radiolist returns the cancel sentinel (-1) on ESC. + with patch("hermes_cli.curses_ui.curses_radiolist", return_value=-1), \ + patch("builtins.print"): + result = _prompt_model_selection(["model-a", "model-b"]) + + assert result is None + + +def test_reasoning_effort_uses_curses_radiolist(): + from hermes_cli.main import _prompt_reasoning_effort_selection + + with patch("hermes_cli.curses_ui.curses_radiolist", return_value=2), \ + patch("builtins.print"): + result = _prompt_reasoning_effort_selection(["low", "medium", "high"], current_effort="") + + assert result == "high" + + +def test_reasoning_effort_esc_cancels(): + from hermes_cli.main import _prompt_reasoning_effort_selection + + with patch("hermes_cli.curses_ui.curses_radiolist", return_value=-1), \ + patch("builtins.print"): + result = _prompt_reasoning_effort_selection(["low", "medium", "high"], current_effort="") + + assert result is None + + +def test_model_selection_with_pricing_passes_description(): + """When pricing is supplied, the aligned header is passed as the curses + description (multi-line text above the list), not lost.""" + from hermes_cli.auth import _prompt_model_selection + + seen = {} + + def _fake(title, items, *, selected=0, cancel_returns=None, description=None): + seen["description"] = description + return len(items) - 1 # Skip + + pricing = { + "model-a": {"prompt": "0.000001", "completion": "0.000002"}, + "model-b": {"prompt": "0.000003", "completion": "0.000004"}, + } + with patch("hermes_cli.curses_ui.curses_radiolist", side_effect=_fake), \ + patch("builtins.print"): + _prompt_model_selection(["model-a", "model-b"], pricing=pricing) + + # The description should carry the In/Out price header. + assert seen["description"] is not None + assert "In" in seen["description"] and "Out" in seen["description"] diff --git a/tests/hermes_cli/test_setup_model_provider.py b/tests/hermes_cli/test_setup_model_provider.py index aa8a9c182ba5..099f4eb94b27 100644 --- a/tests/hermes_cli/test_setup_model_provider.py +++ b/tests/hermes_cli/test_setup_model_provider.py @@ -146,203 +146,6 @@ def fake_select(): assert reloaded["model"]["provider"] == "zai" -def test_setup_same_provider_rotation_strategy_saved_for_multi_credential_pool(tmp_path, monkeypatch): - monkeypatch.setenv("HERMES_HOME", str(tmp_path)) - _clear_provider_env(monkeypatch) - save_env_value("OPENROUTER_API_KEY", "or-key") - - # Pre-write config so the pool step sees provider="openrouter" - _write_model_config("openrouter", "", "anthropic/claude-opus-4.6") - - config = load_config() - - class _Entry: - def __init__(self, label): - self.label = label - - class _Pool: - def entries(self): - return [_Entry("primary"), _Entry("secondary")] - - def fake_select(): - pass # no-op — config already has provider set - - def fake_prompt_choice(question, choices, default=0): - if "rotation strategy" in question: - return 1 # round robin - tts_idx = _maybe_keep_current_tts(question, choices) - if tts_idx is not None: - return tts_idx - return default - - def fake_prompt_yes_no(question, default=True): - return False - - # Patch directly on the module objects to ensure local imports pick them up. - import hermes_cli.main as _main_mod - import hermes_cli.setup as _setup_mod - import agent.credential_pool as _pool_mod - import agent.auxiliary_client as _aux_mod - - monkeypatch.setattr(_main_mod, "select_provider_and_model", fake_select) - # NOTE: _stub_tts overwrites prompt_choice, so set our mock AFTER it. - _stub_tts(monkeypatch) - monkeypatch.setattr(_setup_mod, "prompt_choice", fake_prompt_choice) - monkeypatch.setattr(_setup_mod, "prompt_yes_no", fake_prompt_yes_no) - monkeypatch.setattr(_setup_mod, "prompt", lambda *args, **kwargs: "") - monkeypatch.setattr(_pool_mod, "load_pool", lambda provider: _Pool()) - monkeypatch.setattr(_aux_mod, "get_available_vision_backends", lambda: []) - - setup_model_provider(config) - - # The pool has 2 entries, so the strategy prompt should fire - strategy = config.get("credential_pool_strategies", {}).get("openrouter") - assert strategy == "round_robin", f"Expected round_robin but got {strategy}" - - -def test_setup_same_provider_fallback_can_add_another_credential(tmp_path, monkeypatch): - monkeypatch.setenv("HERMES_HOME", str(tmp_path)) - _clear_provider_env(monkeypatch) - save_env_value("OPENROUTER_API_KEY", "or-key") - - # Pre-write config so the pool step sees provider="openrouter" - _write_model_config("openrouter", "", "anthropic/claude-opus-4.6") - - config = load_config() - pool_sizes = iter([1, 2]) - add_calls = [] - - class _Entry: - def __init__(self, label): - self.label = label - - class _Pool: - def __init__(self, size): - self._size = size - - def entries(self): - return [_Entry(f"cred-{idx}") for idx in range(self._size)] - - def fake_load_pool(provider): - return _Pool(next(pool_sizes)) - - def fake_auth_add_command(args): - add_calls.append(args.provider) - - def fake_select(): - pass # no-op — config already has provider set - - def fake_prompt_choice(question, choices, default=0): - if question == "Select same-provider rotation strategy:": - return 0 - tts_idx = _maybe_keep_current_tts(question, choices) - if tts_idx is not None: - return tts_idx - return default - - yes_no_answers = iter([True, False]) - - def fake_prompt_yes_no(question, default=True): - if question == "Add another credential for same-provider fallback?": - return next(yes_no_answers) - return False - - monkeypatch.setattr("hermes_cli.main.select_provider_and_model", fake_select) - _stub_tts(monkeypatch) - monkeypatch.setattr("hermes_cli.setup.prompt_choice", fake_prompt_choice) - monkeypatch.setattr("hermes_cli.setup.prompt_yes_no", fake_prompt_yes_no) - monkeypatch.setattr("hermes_cli.setup.prompt", lambda *args, **kwargs: "") - monkeypatch.setattr("agent.credential_pool.load_pool", fake_load_pool) - monkeypatch.setattr("hermes_cli.auth_commands.auth_add_command", fake_auth_add_command) - monkeypatch.setattr("agent.auxiliary_client.get_available_vision_backends", lambda: []) - - setup_model_provider(config) - - assert add_calls == ["openrouter"] - assert config.get("credential_pool_strategies", {}).get("openrouter") == "fill_first" - - -def test_setup_same_provider_single_credential_keeps_existing_rotation_strategy(tmp_path, monkeypatch): - monkeypatch.setenv("HERMES_HOME", str(tmp_path)) - _clear_provider_env(monkeypatch) - save_env_value("OPENROUTER_API_KEY", "or-key") - - _write_model_config("openrouter", "", "anthropic/claude-opus-4.6") - - config = load_config() - config["credential_pool_strategies"] = {"openrouter": "round_robin"} - save_config(config) - - class _Entry: - def __init__(self, label): - self.label = label - - class _Pool: - def entries(self): - return [_Entry("primary")] - - def fake_select(): - pass - - monkeypatch.setattr("hermes_cli.main.select_provider_and_model", fake_select) - _stub_tts(monkeypatch) - monkeypatch.setattr("hermes_cli.setup.prompt", lambda *args, **kwargs: "") - monkeypatch.setattr("agent.credential_pool.load_pool", lambda provider: _Pool()) - monkeypatch.setattr("agent.auxiliary_client.get_available_vision_backends", lambda: []) - - setup_model_provider(config) - - assert config.get("credential_pool_strategies", {}).get("openrouter") == "round_robin" - - -def test_setup_pool_step_shows_manual_vs_auto_detected_counts(tmp_path, monkeypatch, capsys): - monkeypatch.setenv("HERMES_HOME", str(tmp_path)) - _clear_provider_env(monkeypatch) - save_env_value("OPENROUTER_API_KEY", "or-key") - - # Pre-write config so the pool step sees provider="openrouter" - _write_model_config("openrouter", "", "anthropic/claude-opus-4.6") - - config = load_config() - - class _Entry: - def __init__(self, label, source): - self.label = label - self.source = source - - class _Pool: - def entries(self): - return [ - _Entry("primary", "manual"), - _Entry("secondary", "manual"), - _Entry("OPENROUTER_API_KEY", "env:OPENROUTER_API_KEY"), - ] - - def fake_select(): - pass # no-op — config already has provider set - - def fake_prompt_choice(question, choices, default=0): - if "rotation strategy" in question: - return 0 - tts_idx = _maybe_keep_current_tts(question, choices) - if tts_idx is not None: - return tts_idx - return default - - monkeypatch.setattr("hermes_cli.main.select_provider_and_model", fake_select) - _stub_tts(monkeypatch) - monkeypatch.setattr("hermes_cli.setup.prompt_choice", fake_prompt_choice) - monkeypatch.setattr("hermes_cli.setup.prompt_yes_no", lambda *args, **kwargs: False) - monkeypatch.setattr("hermes_cli.setup.prompt", lambda *args, **kwargs: "") - monkeypatch.setattr("agent.credential_pool.load_pool", lambda provider: _Pool()) - monkeypatch.setattr("agent.auxiliary_client.get_available_vision_backends", lambda: []) - - setup_model_provider(config) - - out = capsys.readouterr().out - assert "Current pooled credentials for openrouter: 3 (2 manual, 1 auto-detected from env/shared auth)" in out - - def test_setup_copilot_acp_skips_same_provider_pool_step(tmp_path, monkeypatch): monkeypatch.setenv("HERMES_HOME", str(tmp_path)) _clear_provider_env(monkeypatch) diff --git a/tests/hermes_cli/test_setup_reconfigure.py b/tests/hermes_cli/test_setup_reconfigure.py index 6ed49e54ae4a..73e9bfcfa45e 100644 --- a/tests/hermes_cli/test_setup_reconfigure.py +++ b/tests/hermes_cli/test_setup_reconfigure.py @@ -122,10 +122,11 @@ def test_bare_setup_runs_full_reconfigure_without_menu(self, existing_install): m["prompt_choice"].assert_not_called() # Quick-setup path NOT taken. m["quick"].assert_not_called() - # All five sections ran. + # Model/terminal/gateway/tools run; agent settings are no longer + # prompted on existing installs (they keep their tuned values). m["model"].assert_called_once() m["terminal"].assert_called_once() - m["agent"].assert_called_once() + m["agent"].assert_not_called() m["gateway"].assert_called_once() m["tools"].assert_called_once() @@ -149,7 +150,7 @@ def test_reconfigure_flag_is_backwards_compat_noop(self, existing_install): m["prompt_choice"].assert_not_called() m["model"].assert_called_once() m["terminal"].assert_called_once() - m["agent"].assert_called_once() + m["agent"].assert_not_called() m["gateway"].assert_called_once() m["tools"].assert_called_once() diff --git a/tests/hermes_cli/test_terminal_menu_fallbacks.py b/tests/hermes_cli/test_terminal_menu_fallbacks.py index a1283049950f..626858af4ce9 100644 --- a/tests/hermes_cli/test_terminal_menu_fallbacks.py +++ b/tests/hermes_cli/test_terminal_menu_fallbacks.py @@ -1,25 +1,21 @@ -"""Regression tests for numbered fallbacks when TerminalMenu cannot initialize.""" +"""Regression tests for numbered fallbacks when the interactive curses menu +cannot initialize (e.g. non-TTY, curses unavailable, terminal error).""" import subprocess -import sys -import types from hermes_cli.config import load_config, save_config -class _BrokenTerminalMenu: - def __init__(self, *args, **kwargs): - raise subprocess.CalledProcessError(2, ["tput", "clear"]) +def _raise_menu(*args, **kwargs): + # Mimic curses_radiolist hitting an unrecoverable terminal error so the + # caller's except clause routes to the numbered-input fallback. + raise subprocess.CalledProcessError(2, ["tput", "clear"]) -def test_prompt_model_selection_falls_back_on_terminalmenu_runtime_error(monkeypatch): +def test_prompt_model_selection_falls_back_on_menu_runtime_error(monkeypatch): from hermes_cli.auth import _prompt_model_selection - monkeypatch.setitem( - sys.modules, - "simple_term_menu", - types.SimpleNamespace(TerminalMenu=_BrokenTerminalMenu), - ) + monkeypatch.setattr("hermes_cli.curses_ui.curses_radiolist", _raise_menu) responses = iter(["2"]) monkeypatch.setattr("builtins.input", lambda _prompt="": next(responses)) @@ -28,14 +24,10 @@ def test_prompt_model_selection_falls_back_on_terminalmenu_runtime_error(monkeyp assert selected == "model-b" -def test_prompt_reasoning_effort_falls_back_on_terminalmenu_runtime_error(monkeypatch): +def test_prompt_reasoning_effort_falls_back_on_menu_runtime_error(monkeypatch): from hermes_cli.main import _prompt_reasoning_effort_selection - monkeypatch.setitem( - sys.modules, - "simple_term_menu", - types.SimpleNamespace(TerminalMenu=_BrokenTerminalMenu), - ) + monkeypatch.setattr("hermes_cli.curses_ui.curses_radiolist", _raise_menu) responses = iter(["3"]) monkeypatch.setattr("builtins.input", lambda _prompt="": next(responses)) @@ -44,15 +36,11 @@ def test_prompt_reasoning_effort_falls_back_on_terminalmenu_runtime_error(monkey assert selected == "high" -def test_remove_custom_provider_falls_back_on_terminalmenu_runtime_error(tmp_path, monkeypatch): +def test_remove_custom_provider_falls_back_on_menu_runtime_error(tmp_path, monkeypatch): from hermes_cli.main import _remove_custom_provider monkeypatch.setenv("HERMES_HOME", str(tmp_path)) - monkeypatch.setitem( - sys.modules, - "simple_term_menu", - types.SimpleNamespace(TerminalMenu=_BrokenTerminalMenu), - ) + monkeypatch.setattr("hermes_cli.curses_ui.curses_radiolist", _raise_menu) cfg = load_config() cfg["custom_providers"] = [ @@ -72,15 +60,11 @@ def test_remove_custom_provider_falls_back_on_terminalmenu_runtime_error(tmp_pat ] -def test_named_custom_provider_model_picker_falls_back_on_terminalmenu_runtime_error(tmp_path, monkeypatch): +def test_named_custom_provider_model_picker_falls_back_on_menu_runtime_error(tmp_path, monkeypatch): from hermes_cli.main import _model_flow_named_custom monkeypatch.setenv("HERMES_HOME", str(tmp_path)) - monkeypatch.setitem( - sys.modules, - "simple_term_menu", - types.SimpleNamespace(TerminalMenu=_BrokenTerminalMenu), - ) + monkeypatch.setattr("hermes_cli.curses_ui.curses_radiolist", _raise_menu) monkeypatch.setattr("hermes_cli.models.fetch_api_models", lambda *args, **kwargs: ["model-a", "model-b"]) monkeypatch.setattr("hermes_cli.auth.deactivate_provider", lambda: None) diff --git a/tests/hermes_cli/test_tools_config.py b/tests/hermes_cli/test_tools_config.py index e93ad8fcaf30..fc2906d73472 100644 --- a/tests/hermes_cli/test_tools_config.py +++ b/tests/hermes_cli/test_tools_config.py @@ -612,6 +612,52 @@ def test_visible_providers_include_nous_subscription_when_logged_in(monkeypatch) assert providers[0]["name"].startswith("Nous Subscription") +def test_visible_providers_show_nous_subscription_when_logged_out(monkeypatch): + """Nous-managed Tool Gateway rows are always listed, even logged out. + + Selecting one triggers an inline Portal login (entitlement is checked at + selection time, not visibility time). + """ + config = {"model": {"provider": "openrouter"}} + + monkeypatch.setattr( + "hermes_cli.nous_subscription.get_nous_portal_account_info", + lambda: NousPortalAccountInfo( + logged_in=False, + source="none", + fresh=False, + paid_service_access=None, + ), + ) + + providers = _visible_providers(TOOL_CATEGORIES["browser"], config) + + assert any(p["name"].startswith("Nous Subscription") for p in providers) + + +def test_visible_providers_show_nous_subscription_when_paid_access_is_false(monkeypatch): + """Logged-in-but-unpaid users still see the managed rows. + + The paid-access gate moved from visibility to selection time — the row is + shown; ``ensure_nous_portal_access`` blocks activation if still unpaid. + """ + config = {"model": {"provider": "nous"}} + + monkeypatch.setattr( + "hermes_cli.nous_subscription.get_nous_portal_account_info", + lambda: NousPortalAccountInfo( + logged_in=True, + source="jwt", + fresh=False, + paid_service_access=False, + ), + ) + + providers = _visible_providers(TOOL_CATEGORIES["browser"], config) + + assert any(p["name"].startswith("Nous Subscription") for p in providers) + + def test_visible_providers_force_fresh_shows_nous_subscription_after_upgrade(monkeypatch): calls = [] @@ -643,24 +689,6 @@ def fake_subscription_features(config, *, force_fresh=False): assert ("features", True) in calls -def test_visible_providers_hide_nous_subscription_when_paid_access_is_false(monkeypatch): - config = {"model": {"provider": "nous"}} - - monkeypatch.setattr( - "hermes_cli.nous_subscription.get_nous_portal_account_info", - lambda: NousPortalAccountInfo( - logged_in=True, - source="jwt", - fresh=False, - paid_service_access=False, - ), - ) - - providers = _visible_providers(TOOL_CATEGORIES["browser"], config) - - assert all(not provider["name"].startswith("Nous Subscription") for provider in providers) - - def test_local_browser_provider_is_saved_explicitly(monkeypatch): config = {} local_provider = next( @@ -669,7 +697,6 @@ def test_local_browser_provider_is_saved_explicitly(monkeypatch): if provider.get("browser_provider") == "local" ) monkeypatch.setattr("hermes_cli.tools_config._run_post_setup", lambda key: None) - _configure_provider(local_provider, config) assert config["browser"]["cloud_provider"] == "local" @@ -1265,7 +1292,13 @@ def test_get_effective_configurable_toolsets_dedupes_bundled_plugins(): ({"name": "B", "browser_provider": "browserbase", "env_vars": []}, "browser", False), ({"name": "W", "web_backend": "tavily", "env_vars": []}, "web", False), ]) -def test_reconfigure_provider_syncs_use_gateway(provider, config_key, expected): +def test_reconfigure_provider_syncs_use_gateway(monkeypatch, provider, config_key, expected): + # Managed providers run the inline Portal entitlement gate; treat the user + # as already entitled so the test exercises the use_gateway sync. + monkeypatch.setattr( + "hermes_cli.nous_subscription.ensure_nous_portal_access", + lambda **kwargs: True, + ) config = {} _reconfigure_provider(provider, config) assert config[config_key]["use_gateway"] is expected @@ -1301,3 +1334,69 @@ def test_reconfigure_provider_runs_post_setup_for_env_var_providers( _reconfigure_provider(provider, {}) assert called == [post_setup_key] + + +# --------------------------------------------------------------------------- +# Inline Nous Portal login gate on managed-provider selection +# --------------------------------------------------------------------------- + + +def test_configure_managed_provider_blocks_when_not_entitled(monkeypatch): + """Selecting a Nous-managed backend without paid access writes no config.""" + monkeypatch.setattr( + "hermes_cli.nous_subscription.ensure_nous_portal_access", + lambda **kwargs: False, + ) + provider = { + "name": "Nous Subscription (Firecrawl)", + "web_backend": "firecrawl", + "managed_nous_feature": "web", + "env_vars": [], + } + config = {} + + _configure_provider(provider, config) + + # No use_gateway / backend written — the gate returned before any mutation. + assert "web" not in config + + +def test_configure_managed_provider_enables_when_entitled(monkeypatch): + """Once entitled, selecting the managed backend sets use_gateway=True.""" + monkeypatch.setattr( + "hermes_cli.nous_subscription.ensure_nous_portal_access", + lambda **kwargs: True, + ) + provider = { + "name": "Nous Subscription (Firecrawl)", + "web_backend": "firecrawl", + "managed_nous_feature": "web", + "env_vars": [], + } + config = {} + + _configure_provider(provider, config) + + assert config["web"]["backend"] == "firecrawl" + assert config["web"]["use_gateway"] is True + + +def test_configure_non_managed_provider_skips_portal_gate(monkeypatch): + """A self-hosted provider must never trigger the Nous Portal login gate.""" + called = {"gate": False} + + def _boom(**kwargs): + called["gate"] = True + return False + + monkeypatch.setattr( + "hermes_cli.nous_subscription.ensure_nous_portal_access", _boom + ) + provider = {"name": "Tavily", "web_backend": "tavily", "env_vars": []} + config = {} + + _configure_provider(provider, config) + + assert called["gate"] is False + assert config["web"]["backend"] == "tavily" + assert config["web"]["use_gateway"] is False diff --git a/tests/run_agent/test_empty_response_recovery_persistence.py b/tests/run_agent/test_empty_response_recovery_persistence.py index 27e6c23d2d47..a3009a596c51 100644 --- a/tests/run_agent/test_empty_response_recovery_persistence.py +++ b/tests/run_agent/test_empty_response_recovery_persistence.py @@ -92,3 +92,36 @@ def test_persist_session_strips_marked_terminal_empty_sentinel(): assert messages == [{"role": "user", "content": "continue"}] assert agent.flushed_session_db_messages[-1] == messages assert all(not msg.get("_empty_terminal_sentinel") for msg in messages) + + +def test_persist_session_strips_trailing_malformed_final_recovery_scaffolding(): + agent = _agent_with_stubbed_persistence() + messages = [ + {"role": "user", "content": "run the task"}, + { + "role": "assistant", + "content": "", + "tool_calls": [{"id": "call_1", "type": "function", + "function": {"name": "x", "arguments": "{}"}}], + }, + {"role": "tool", "content": "{}", "tool_call_id": "call_1"}, + { + "role": "assistant", + "content": "[malformed final response omitted]", + "_malformed_final_recovery_synthetic": True, + }, + { + "role": "user", + "content": "The previous final response was malformed.", + "_malformed_final_recovery_synthetic": True, + }, + ] + + AIAgent._persist_session(agent, messages, conversation_history=[]) + + assert messages == [{"role": "user", "content": "run the task"}] + assert agent.flushed_session_db_messages[-1] == messages + assert all( + not msg.get("_malformed_final_recovery_synthetic") + for msg in messages + ) diff --git a/tests/run_agent/test_image_shrink_recovery.py b/tests/run_agent/test_image_shrink_recovery.py index c5114ffef04c..86a3e6abf641 100644 --- a/tests/run_agent/test_image_shrink_recovery.py +++ b/tests/run_agent/test_image_shrink_recovery.py @@ -273,3 +273,51 @@ def test_shrink_that_makes_it_bigger_rejected(self, monkeypatch): assert agent._try_shrink_image_parts_in_messages(msgs) is False # Original URL still in place, not replaced by the bigger one. assert msgs[0]["content"][0]["image_url"]["url"] == oversized_url + + def test_mixed_one_shrinkable_one_not_returns_false(self, monkeypatch): + """Regression for the wedged-session incident (May 2026). + + When one oversized image shrinks but another oversized image can't, + the helper must return False — retrying would re-send the surviving + oversized payload and fail identically, burning the single retry on a + no-op. The original bug returned True after shrinking *any* part, + which is what permanently wedged a session whose history held a 12 MB + tool-result image alongside a freshly-loaded shrinkable one. + """ + agent = _make_agent() + shrinkable = _big_png_data_url(5000) + unshrinkable = _big_png_data_url(6000) + small = "data:image/jpeg;base64," + "C" * 500 + + # _resize_image_for_vision returns small for the shrinkable input but + # echoes the oversized payload back for the unshrinkable one. + def fake_resize(path, *a, **kw): + # The temp file written by the helper contains the decoded bytes; + # distinguish by size — the 6000 KB source stays "big". + try: + size = path.stat().st_size + except Exception: + size = 0 + if size > 5500 * 1024: + return unshrinkable # can't reduce — echo oversized back + return small + + monkeypatch.setattr( + "tools.vision_tools._resize_image_for_vision", + fake_resize, + raising=False, + ) + + msgs = [{ + "role": "tool", + "content": [ + {"type": "image_url", "image_url": {"url": shrinkable}}, + {"type": "image_url", "image_url": {"url": unshrinkable}}, + ], + }] + # One part shrank, one survived oversized → must NOT retry. + assert agent._try_shrink_image_parts_in_messages(msgs) is False + # The shrinkable one was still re-encoded (mutated in place). + assert msgs[0]["content"][0]["image_url"]["url"] == small + # The unshrinkable one is left as-is (caller surfaces original error). + assert msgs[0]["content"][1]["image_url"]["url"] == unshrinkable diff --git a/tests/run_agent/test_malformed_final_recovery.py b/tests/run_agent/test_malformed_final_recovery.py new file mode 100644 index 000000000000..238a2b9304e1 --- /dev/null +++ b/tests/run_agent/test_malformed_final_recovery.py @@ -0,0 +1,126 @@ +"""Regression tests for malformed non-empty final response recovery.""" + +from __future__ import annotations + +from unittest.mock import MagicMock, patch + +from run_agent import AIAgent +from tests.run_agent.test_run_agent import ( + _mock_response, + _mock_tool_call, +) + + +def _tool_schema(name: str = "terminal") -> dict: + return { + "type": "function", + "function": { + "name": name, + "description": "test tool", + "parameters": {"type": "object", "properties": {}}, + }, + } + + +def _make_agent(): + with ( + patch("run_agent.get_tool_definitions", return_value=[_tool_schema()]), + patch("run_agent.check_toolset_requirements", return_value={}), + patch("run_agent.OpenAI"), + ): + agent = AIAgent( + api_key="test-key-1234567890", + base_url="http://127.0.0.1:9090/v1", + model="dflash", + quiet_mode=True, + skip_context_files=True, + skip_memory=True, + ) + agent.client = MagicMock() + agent._cached_system_prompt = "You are helpful." + agent._use_prompt_caching = False + agent.tool_delay = 0 + agent.compression_enabled = False + agent.save_trajectories = False + return agent + + +def test_degenerate_final_after_tool_calls_gets_regeneration_prompt(): + agent = _make_agent() + tool_call = _mock_tool_call(name="terminal", arguments="{}") + malformed = ( + "Now I have a clear picture. The key pattern is:\n\n" + "1. signal: killed, exit code: -1\n" + "2. process exited but not StateStopping\n" + "3. no valid JSON data found in stream\n" + "4. No d!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!" + ) + + agent.client.chat.completions.create.side_effect = [ + _mock_response(content="", tool_calls=[tool_call], finish_reason="tool_calls"), + _mock_response(content=malformed, finish_reason="stop"), + _mock_response(content="The backend did not die at that timestamp.", finish_reason="stop"), + ] + + with ( + patch("run_agent.handle_function_call", return_value='{"ok": true}'), + patch.object(agent, "_persist_session"), + patch.object(agent, "_save_trajectory"), + patch.object(agent, "_cleanup_task_resources"), + ): + result = agent.run_conversation("diagnose dflash") + + assert agent.client.chat.completions.create.call_count == 3 + second_retry_kwargs = agent.client.chat.completions.create.call_args_list[2].kwargs + retry_messages = second_retry_kwargs["messages"] + assert retry_messages[-1]["role"] == "user" + assert "Regenerate a concise, complete final answer" in retry_messages[-1]["content"] + assert retry_messages[-2]["content"] == "[malformed final response omitted]" + assert result["final_response"] == "The backend did not die at that timestamp." + assert "No d!!!!" not in result["final_response"] + + +def test_degenerate_final_repeats_activates_fallback_provider(): + agent = _make_agent() + agent._fallback_chain = [ + { + "provider": "openrouter", + "model": "openai/gpt-test-fallback", + "base_url": "https://openrouter.ai/api/v1", + "api_key": "fallback-key", + } + ] + tool_call = _mock_tool_call(name="terminal", arguments="{}") + malformed = ( + "I found the issue after reviewing logs and state. " + "The final line is broken!!!!!!!!!!!!!!!!!!!!!!!!" + ) + + agent.client.chat.completions.create.side_effect = [ + _mock_response(content="", tool_calls=[tool_call], finish_reason="tool_calls"), + _mock_response(content=malformed, finish_reason="stop"), + _mock_response(content=malformed, finish_reason="stop"), + _mock_response(content="Fallback produced a clean answer.", finish_reason="stop"), + ] + fallback_client = MagicMock() + fallback_client.api_key = "fallback-key" + fallback_client.base_url = "https://openrouter.ai/api/v1" + fallback_client.chat = agent.client.chat + + with ( + patch("run_agent.handle_function_call", return_value='{"ok": true}'), + patch.object(agent, "_persist_session"), + patch.object(agent, "_save_trajectory"), + patch.object(agent, "_cleanup_task_resources"), + patch( + "agent.auxiliary_client.resolve_provider_client", + return_value=(fallback_client, "openai/gpt-test-fallback"), + ), + patch.object(agent, "_create_request_openai_client", return_value=fallback_client), + ): + result = agent.run_conversation("diagnose dflash") + + assert agent.client.chat.completions.create.call_count == 4 + assert agent.provider == "openrouter" + assert agent.model == "openai/gpt-test-fallback" + assert result["final_response"] == "Fallback produced a clean answer." diff --git a/tests/tools/test_process_registry.py b/tests/tools/test_process_registry.py index bc1ec06d66c7..8e3426b2713f 100644 --- a/tests/tools/test_process_registry.py +++ b/tests/tools/test_process_registry.py @@ -481,8 +481,8 @@ def __init__(self): def get_temp_dir(self): return "/data/data/com.termux/files/usr/tmp" - def execute(self, command, timeout=None): - self.commands.append((command, timeout)) + def execute(self, command, **kwargs): + self.commands.append((command, kwargs)) return {"output": "4321\n"} env = FakeEnv() @@ -501,6 +501,52 @@ def execute(self, command, timeout=None): assert "cat /tmp/hermes_bg_" not in bg_command fake_thread.start.assert_called_once() + def test_spawn_via_env_checks_returncode_when_wrapper_fails(self, registry): + class FakeEnv: + def __init__(self): + self.commands = [] + + def execute(self, command, **kwargs): + self.commands.append((command, kwargs)) + return {"output": "syntax error", "returncode": 2} + + env = FakeEnv() + fake_thread = MagicMock() + + with patch("tools.process_registry.threading.Thread", return_value=fake_thread), \ + patch.object(registry, "_write_checkpoint"): + session = registry.spawn_via_env(env, "echo hello") + + assert session.exited is True + assert session.exit_code == 2 + assert session.pid is None + assert session.output_buffer == "syntax error" + fake_thread.start.assert_not_called() + # A failed launch must not be exposed as a running/tracked session. + assert session.id not in registry._running + + def test_spawn_via_env_disables_rewrite_for_bg_wrapper(self, registry): + class FakeEnv: + def __init__(self): + self.commands = [] + + def get_temp_dir(self): + return "/tmp" + + def execute(self, command, **kwargs): + self.commands.append((command, kwargs)) + return {"output": "4321\n", "returncode": 0} + + env = FakeEnv() + fake_thread = MagicMock() + + with patch("tools.process_registry.threading.Thread", return_value=fake_thread), \ + patch.object(registry, "_write_checkpoint"): + registry.spawn_via_env(env, "echo hello") + + args, kwargs = env.commands[0] + assert kwargs.get("rewrite_compound_background") is False + def test_env_poller_quotes_temp_paths_with_spaces(self, registry): session = _make_session(sid="proc_space") session.exited = False @@ -514,8 +560,8 @@ def __init__(self): {"output": "0\n"}, ]) - def execute(self, command, timeout=None): - self.commands.append((command, timeout)) + def execute(self, command, **kwargs): + self.commands.append((command, kwargs)) return next(self._responses) env = FakeEnv() diff --git a/tests/tools/test_terminal_task_cwd.py b/tests/tools/test_terminal_task_cwd.py index 8c8ff867c361..1947836fb476 100644 --- a/tests/tools/test_terminal_task_cwd.py +++ b/tests/tools/test_terminal_task_cwd.py @@ -1,6 +1,7 @@ """Regression tests for task/session cwd propagation in terminal_tool.""" import json +from types import SimpleNamespace import tools.terminal_tool as terminal_tool @@ -10,6 +11,7 @@ def _minimal_terminal_config(cwd="/default"): "env_type": "local", "cwd": cwd, "timeout": 60, + "lifetime_seconds": 3600, } @@ -72,3 +74,147 @@ def execute(self, command, **kwargs): assert result["exit_code"] == 0 assert calls == [{"timeout": 60, "cwd": "/explicit/workdir"}] + + +def test_foreground_command_prefers_live_env_cwd_over_init_time_cwd(monkeypatch): + """A prior `cd` updates env.cwd; terminal_tool must honor that live cwd.""" + calls = [] + + class FakeEnv: + env = {} + cwd = "/workspace/live" + + def execute(self, command, **kwargs): + calls.append((command, kwargs)) + return {"output": "ok", "returncode": 0} + + task_id = "session-live-cwd" + monkeypatch.setattr(terminal_tool, "_active_environments", {task_id: FakeEnv()}) + monkeypatch.setattr(terminal_tool, "_last_activity", {}) + monkeypatch.setattr(terminal_tool, "_task_env_overrides", {task_id: {"cwd": "/workspace/init"}}) + monkeypatch.setattr(terminal_tool, "_get_env_config", lambda: _minimal_terminal_config(cwd="/workspace/init")) + monkeypatch.setattr(terminal_tool, "_start_cleanup_thread", lambda: None) + monkeypatch.setattr(terminal_tool, "_resolve_container_task_id", lambda value: value or "default") + monkeypatch.setattr( + terminal_tool, + "_check_all_guards", + lambda command, env_type: {"approved": True}, + ) + + result = json.loads(terminal_tool.terminal_tool(command="pwd", task_id=task_id)) + + assert result["exit_code"] == 0 + assert calls == [("pwd", {"timeout": 60, "cwd": "/workspace/live"})] + + +def test_background_command_prefers_live_env_cwd_over_init_time_cwd(monkeypatch): + """Background process launches must also use the live session cwd.""" + + class FakeEnv: + env = {} + cwd = "/workspace/live" + + class FakeRegistry: + def __init__(self): + self.calls = [] + self.pending_watchers = [] + + def spawn_local(self, **kwargs): + self.calls.append(kwargs) + return SimpleNamespace(id="proc_test", pid=1234) + + import tools.process_registry as process_registry_mod + + registry = FakeRegistry() + task_id = "session-live-cwd-bg" + monkeypatch.setattr(terminal_tool, "_active_environments", {task_id: FakeEnv()}) + monkeypatch.setattr(terminal_tool, "_last_activity", {}) + monkeypatch.setattr(terminal_tool, "_task_env_overrides", {task_id: {"cwd": "/workspace/init"}}) + monkeypatch.setattr(terminal_tool, "_get_env_config", lambda: _minimal_terminal_config(cwd="/workspace/init")) + monkeypatch.setattr(terminal_tool, "_start_cleanup_thread", lambda: None) + monkeypatch.setattr(terminal_tool, "_resolve_container_task_id", lambda value: value or "default") + monkeypatch.setattr( + terminal_tool, + "_check_all_guards", + lambda command, env_type: {"approved": True}, + ) + monkeypatch.setattr(process_registry_mod, "process_registry", registry) + + result = json.loads( + terminal_tool.terminal_tool( + command="sleep 1", + task_id=task_id, + background=True, + ) + ) + + assert result["exit_code"] == 0 + assert registry.calls == [{ + "command": "sleep 1", + "cwd": "/workspace/live", + "task_id": task_id, + "session_key": "", + "env_vars": {}, + "use_pty": False, + }] + + +def test_registering_cwd_override_updates_live_env_cwd(monkeypatch): + """An ACP ``update_cwd`` (re-)registered mid-session must win over a + previously ``cd``-ed live ``env.cwd``. + + Preferring live ``env.cwd`` (so session-local ``cd`` survives) means a + freshly registered ``cwd`` override would otherwise sit *below* the + already-set ``env.cwd`` and be silently ignored. ``register_task_env_overrides`` + syncs the new cwd onto the live cached env so an explicit ACP project-root + change takes effect, as the editor client expects. + """ + + class FakeEnv: + env = {} + cwd = "/workspace/old" + + task_id = "acp-session-update" + fake_env = FakeEnv() + monkeypatch.setattr(terminal_tool, "_active_environments", {task_id: fake_env}) + monkeypatch.setattr(terminal_tool, "_task_env_overrides", {}) + + terminal_tool.register_task_env_overrides(task_id, {"cwd": "/workspace/new"}) + + # The live env now reflects the editor's new project root. + assert fake_env.cwd == "/workspace/new" + + # A subsequent command resolves to the new cwd (env.cwd precedence). + assert terminal_tool._resolve_command_cwd( + workdir=None, env=fake_env, default_cwd="/workspace/config" + ) == "/workspace/new" + + +def test_registering_cwd_override_noop_when_no_live_env(monkeypatch): + """Registering an override before the env exists must not crash; the cwd + is applied at env creation time instead.""" + monkeypatch.setattr(terminal_tool, "_active_environments", {}) + monkeypatch.setattr(terminal_tool, "_task_env_overrides", {}) + + # Should not raise even though no env is cached yet. + terminal_tool.register_task_env_overrides("acp-session-pending", {"cwd": "/workspace/new"}) + + assert terminal_tool._task_env_overrides["acp-session-pending"] == {"cwd": "/workspace/new"} + + +def test_registering_non_cwd_override_leaves_live_env_cwd_untouched(monkeypatch): + """A non-cwd override (e.g. a per-task Modal image) must not disturb the + live env's cwd.""" + + class FakeEnv: + env = {} + cwd = "/workspace/keep" + + task_id = "rl-rollout-1" + fake_env = FakeEnv() + monkeypatch.setattr(terminal_tool, "_active_environments", {task_id: fake_env}) + monkeypatch.setattr(terminal_tool, "_task_env_overrides", {}) + + terminal_tool.register_task_env_overrides(task_id, {"modal_image": "custom:latest"}) + + assert fake_env.cwd == "/workspace/keep" diff --git a/tests/tools/test_tool_output_limits.py b/tests/tools/test_tool_output_limits.py index 19fa3fc05a1b..b18f7f3ad0b1 100644 --- a/tests/tools/test_tool_output_limits.py +++ b/tests/tools/test_tool_output_limits.py @@ -22,6 +22,16 @@ from tools import tool_output_limits as tol +@pytest.fixture(autouse=True) +def _reset_limits_cache(): + """get_tool_output_limits() now memoizes its result for the process + lifetime, so each test must start from a clean cache to observe the + config value it patches in.""" + tol._reset_tool_output_limits_cache() + yield + tol._reset_tool_output_limits_cache() + + class TestDefaults: def test_defaults_match_previous_hardcoded_values(self): assert tol.DEFAULT_MAX_BYTES == 50_000 diff --git a/tests/tools/test_vision_native_fast_path.py b/tests/tools/test_vision_native_fast_path.py index 9916ca369d50..bb396c05dc36 100644 --- a/tests/tools/test_vision_native_fast_path.py +++ b/tests/tools/test_vision_native_fast_path.py @@ -139,6 +139,44 @@ def test_file_url_scheme_resolves(self, tmp_path): assert isinstance(result, dict) assert result.get("_multimodal") is True + def test_oversized_image_resized_under_embed_cap(self, tmp_path): + """Regression for the wedged-session incident (May 2026). + + A vision tool-result image is baked into conversation history and + re-sent on every subsequent turn. Anthropic rejects any single + base64 image over 5 MB with a 400, and immutable history means the + bad bytes can't be cleared by retrying — the session is permanently + wedged. The native fast path must proactively resize down to the + embed cap (well under 5 MB) BEFORE embedding, not just at the 20 MB + hard ceiling. Skips if Pillow isn't available (resize is a no-op). + """ + pytest = __import__("pytest") + try: + from PIL import Image + except ImportError: + pytest.skip("Pillow not installed — proactive resize is a no-op") + + from tools.vision_tools import _EMBED_TARGET_BYTES + + # Noisy PNG that base64-encodes to well over 5 MB (won't compress much). + big = tmp_path / "big.png" + Image.effect_noise((2600, 2600), 80).convert("RGB").save(big, format="PNG") + assert big.stat().st_size * 4 // 3 > 5 * 1024 * 1024, "test image not big enough" + + result = asyncio.get_event_loop().run_until_complete( + _vision_analyze_native(str(big), "describe") + ) + assert isinstance(result, dict) and result.get("_multimodal") is True + url = next( + p["image_url"]["url"] + for p in result["content"] + if p.get("type") == "image_url" + ) + assert len(url) <= _EMBED_TARGET_BYTES, ( + f"embedded image {len(url) / 1024 / 1024:.1f} MB exceeds embed cap " + f"{_EMBED_TARGET_BYTES / 1024 / 1024:.0f} MB — would wedge sessions on Anthropic" + ) + # ─── _handle_vision_analyze fast-path gating ───────────────────────────────── diff --git a/tests/tools/test_voice_mode.py b/tests/tools/test_voice_mode.py index 2a2b77bae2ee..8f6a8e67784e 100644 --- a/tests/tools/test_voice_mode.py +++ b/tests/tools/test_voice_mode.py @@ -72,6 +72,62 @@ def _fake_import_audio(): # detect_audio_environment — WSL / SSH / Docker detection # ============================================================================ +class TestPulseSocketReachable: + def test_no_env_no_socket(self, monkeypatch): + monkeypatch.delenv("PULSE_SERVER", raising=False) + monkeypatch.delenv("PULSE_RUNTIME_PATH", raising=False) + monkeypatch.delenv("XDG_RUNTIME_DIR", raising=False) + from tools.voice_mode import _pulse_socket_reachable + assert _pulse_socket_reachable() is False + + def test_stale_socket_file_not_reachable(self, monkeypatch, tmp_path): + """A socket file with no listener should not count as reachable.""" + import socket as _socket + sock_path = tmp_path / "pulse" / "native" + sock_path.parent.mkdir(parents=True) + # Create + bind, then close so the path is a stale socket file. + s = _socket.socket(_socket.AF_UNIX, _socket.SOCK_STREAM) + s.bind(str(sock_path)) + s.close() + monkeypatch.delenv("PULSE_SERVER", raising=False) + monkeypatch.delenv("PULSE_RUNTIME_PATH", raising=False) + monkeypatch.setenv("XDG_RUNTIME_DIR", str(tmp_path)) + from tools.voice_mode import _pulse_socket_reachable + assert _pulse_socket_reachable() is False + + def test_listening_socket_reachable_via_xdg_runtime(self, monkeypatch, tmp_path): + """A live PulseAudio-style socket under XDG_RUNTIME_DIR is reachable (#35622).""" + import socket as _socket + sock_path = tmp_path / "pulse" / "native" + sock_path.parent.mkdir(parents=True) + server = _socket.socket(_socket.AF_UNIX, _socket.SOCK_STREAM) + server.bind(str(sock_path)) + server.listen(1) + try: + monkeypatch.delenv("PULSE_SERVER", raising=False) + monkeypatch.delenv("PULSE_RUNTIME_PATH", raising=False) + monkeypatch.setenv("XDG_RUNTIME_DIR", str(tmp_path)) + from tools.voice_mode import _pulse_socket_reachable + assert _pulse_socket_reachable() is True + finally: + server.close() + + def test_listening_socket_reachable_via_pulse_server_env(self, monkeypatch, tmp_path): + import socket as _socket + sock_path = tmp_path / "native" + server = _socket.socket(_socket.AF_UNIX, _socket.SOCK_STREAM) + server.bind(str(sock_path)) + server.listen(1) + try: + monkeypatch.delenv("PULSE_RUNTIME_PATH", raising=False) + monkeypatch.delenv("XDG_RUNTIME_DIR", raising=False) + monkeypatch.setenv("PULSE_SERVER", f"unix:{sock_path}") + from tools.voice_mode import _pulse_socket_reachable + assert _pulse_socket_reachable() is True + finally: + server.close() + + class TestDetectAudioEnvironment: def test_clean_environment_is_available(self, monkeypatch): """No SSH, Docker, or WSL — should be available.""" @@ -88,8 +144,11 @@ def test_clean_environment_is_available(self, monkeypatch): assert result["warnings"] == [] def test_ssh_blocks_voice(self, monkeypatch): - """SSH environment should block voice mode.""" + """SSH environment without a reachable sound server should block voice mode.""" monkeypatch.setenv("SSH_CLIENT", "1.2.3.4 54321 22") + monkeypatch.delenv("PULSE_SERVER", raising=False) + monkeypatch.delenv("PIPEWIRE_REMOTE", raising=False) + monkeypatch.setattr("tools.voice_mode._pulse_socket_reachable", lambda: False) monkeypatch.setattr("tools.voice_mode._import_audio", lambda: (MagicMock(), MagicMock())) @@ -98,12 +157,46 @@ def test_ssh_blocks_voice(self, monkeypatch): assert result["available"] is False assert any("SSH" in w for w in result["warnings"]) + def test_ssh_with_pulse_server_allows_voice(self, monkeypatch): + """SSH with PULSE_SERVER set should NOT block voice mode (#35622).""" + monkeypatch.setenv("SSH_CLIENT", "1.2.3.4 54321 22") + monkeypatch.setenv("PULSE_SERVER", "unix:/run/user/1002/pulse/native") + monkeypatch.delenv("PIPEWIRE_REMOTE", raising=False) + monkeypatch.setattr("tools.voice_mode._import_audio", + lambda: (MagicMock(), MagicMock())) + monkeypatch.setattr("builtins.open", _non_wsl_proc_version(open)) + + from tools.voice_mode import detect_audio_environment + result = detect_audio_environment() + assert result["available"] is True + assert result["warnings"] == [] + assert any("SSH" in n for n in result.get("notices", [])) + + def test_ssh_with_reachable_pulse_socket_allows_voice(self, monkeypatch): + """SSH with a reachable PulseAudio socket (no env vars) allows voice (#35622).""" + monkeypatch.setenv("SSH_CLIENT", "1.2.3.4 54321 22") + monkeypatch.delenv("PULSE_SERVER", raising=False) + monkeypatch.delenv("PIPEWIRE_REMOTE", raising=False) + # User runs `pulseaudio &` locally on the SSH host: the default socket + # is reachable even though PULSE_SERVER is unset. + monkeypatch.setattr("tools.voice_mode._pulse_socket_reachable", lambda: True) + monkeypatch.setattr("tools.voice_mode._import_audio", + lambda: (MagicMock(), MagicMock())) + monkeypatch.setattr("builtins.open", _non_wsl_proc_version(open)) + + from tools.voice_mode import detect_audio_environment + result = detect_audio_environment() + assert result["available"] is True + assert result["warnings"] == [] + assert any("SSH" in n for n in result.get("notices", [])) + def test_wsl_without_pulse_blocks_voice(self, monkeypatch, tmp_path): """WSL without PULSE_SERVER should block voice mode.""" monkeypatch.delenv("SSH_CLIENT", raising=False) monkeypatch.delenv("SSH_TTY", raising=False) monkeypatch.delenv("SSH_CONNECTION", raising=False) monkeypatch.delenv("PULSE_SERVER", raising=False) + monkeypatch.setattr("tools.voice_mode._pulse_socket_reachable", lambda: False) monkeypatch.setattr("tools.voice_mode._import_audio", lambda: (MagicMock(), MagicMock())) @@ -184,6 +277,7 @@ def test_device_query_fails_without_pulse_blocks(self, monkeypatch): monkeypatch.delenv("SSH_TTY", raising=False) monkeypatch.delenv("SSH_CONNECTION", raising=False) monkeypatch.delenv("PULSE_SERVER", raising=False) + monkeypatch.setattr("tools.voice_mode._pulse_socket_reachable", lambda: False) mock_sd = MagicMock() mock_sd.query_devices.side_effect = Exception("device query failed") @@ -312,6 +406,7 @@ def test_docker_without_audio_forwarding_blocks_voice(self, monkeypatch): monkeypatch.delenv("SSH_CONNECTION", raising=False) monkeypatch.delenv("PULSE_SERVER", raising=False) monkeypatch.delenv("PIPEWIRE_REMOTE", raising=False) + monkeypatch.setattr("tools.voice_mode._pulse_socket_reachable", lambda: False) monkeypatch.setattr("hermes_constants.is_container", lambda: True) monkeypatch.setattr("tools.voice_mode._import_audio", lambda: (MagicMock(), MagicMock())) diff --git a/tools/environments/base.py b/tools/environments/base.py index 618ea2bb9224..251bb18f1425 100644 --- a/tools/environments/base.py +++ b/tools/environments/base.py @@ -833,18 +833,18 @@ def execute( *, timeout: int | None = None, stdin_data: str | None = None, + rewrite_compound_background: bool = True, ) -> dict: """Execute a command, return {"output": str, "returncode": int}.""" self._before_execute() exec_command, sudo_stdin = self._prepare_command(command) - # Guard against the `A && B &` subshell-wait trap: bash forks a - # subshell for the compound that then waits for an infinite B (a - # server, `yes > /dev/null`, etc.), leaking the subshell forever. - # Rewriting to `A && { B & }` runs B as a plain background in the - # current shell — no subshell wait. - from tools.terminal_tool import _rewrite_compound_background - exec_command = _rewrite_compound_background(exec_command) + # Guard against the `A && B &` subshell-wait trap by default. + # Some callers (spawn_via_env) already produce shell-safe wrappers and + # pass rewrite_compound_background=False. + if rewrite_compound_background: + from tools.terminal_tool import _rewrite_compound_background + exec_command = _rewrite_compound_background(exec_command) effective_timeout = timeout or self.timeout effective_cwd = cwd or self.cwd @@ -893,4 +893,3 @@ def _prepare_command(self, command: str) -> tuple[str, str | None]: from tools.terminal_tool import _transform_sudo_command return _transform_sudo_command(command) - diff --git a/tools/environments/modal_utils.py b/tools/environments/modal_utils.py index 4d68399e4165..f83c0075ee3e 100644 --- a/tools/environments/modal_utils.py +++ b/tools/environments/modal_utils.py @@ -79,7 +79,12 @@ def execute( *, timeout: int | None = None, stdin_data: str | None = None, + rewrite_compound_background: bool = True, ) -> dict: + # Managed/remote modal transports execute commands via explicit transport + # and do not rely on shell background rewriters. Keep parameter for + # compatibility with BaseEnvironment callers. + _ = rewrite_compound_background self._before_execute() prepared = self._prepare_modal_exec( command, diff --git a/tools/kanban_tools.py b/tools/kanban_tools.py index d3493f0f8e39..3b4ede304c5c 100644 --- a/tools/kanban_tools.py +++ b/tools/kanban_tools.py @@ -759,6 +759,10 @@ def _handle_create(args: dict, **kw) -> str: return tool_error( f"skills must be a list of skill names, got {type(skills).__name__}" ) + goal_mode, goal_bool_error = _parse_bool_arg(args, "goal_mode") + if goal_bool_error: + return tool_error(goal_bool_error) + goal_max_turns = args.get("goal_max_turns") if isinstance(parents, str): parents = [parents] if not isinstance(parents, (list, tuple)): @@ -786,6 +790,10 @@ def _handle_create(args: dict, **kw) -> str: if max_runtime_seconds is not None else None ), skills=skills, + goal_mode=goal_mode, + goal_max_turns=( + int(goal_max_turns) if goal_max_turns is not None else None + ), initial_status=str(initial_status), created_by=os.environ.get("HERMES_PROFILE") or "worker", session_id=session_id, @@ -1250,6 +1258,29 @@ def _board_schema_prop() -> dict[str, str]: "assignee's profile." ), }, + "goal_mode": { + "type": "boolean", + "description": ( + "Run the dispatched worker in a goal loop. When true, " + "after each turn an auxiliary judge checks the worker's " + "response against this card's title/body; if the work " + "isn't done and budget remains, the worker keeps going " + "in the same session until the judge agrees it's " + "complete (or the goal-turn budget is exhausted, which " + "blocks the task for human review). Use this for " + "open-ended cards where one shot rarely finishes the " + "work. Defaults to false (classic single-shot worker)." + ), + }, + "goal_max_turns": { + "type": "integer", + "description": ( + "Turn budget for goal_mode workers. Caps how many " + "continuation turns the worker may take before the task " + "is blocked for review. Ignored unless goal_mode is " + "true. Defaults to the goal-engine default (20)." + ), + }, "board": _board_schema_prop(), }, "required": ["title", "assignee"], diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py index 9794b5e85921..b3a7cd2d5ce6 100644 --- a/tools/mcp_tool.py +++ b/tools/mcp_tool.py @@ -2040,14 +2040,27 @@ async def _recover(): loop = _mcp_loop if loop is not None and loop.is_running(): loop.call_soon_threadsafe(srv._reconnect_event.set) + # Wait briefly for the session to come back ready. Bounded # so that a stuck reconnect falls through to the error - # path rather than hanging the caller. - deadline = time.monotonic() + 15 - while time.monotonic() < deadline: - if srv.session is not None and srv._ready.is_set(): - break - time.sleep(0.25) + # path rather than hanging the caller. The async helper + # runs on the MCP event loop via _run_on_mcp_loop so it + # does NOT block the event loop during the poll interval. + async def _await_ready() -> bool: + deadline = time.monotonic() + 15 + while time.monotonic() < deadline: + if srv.session is not None and srv._ready.is_set(): + return True + await asyncio.sleep(0.25) + return False + + try: + _run_on_mcp_loop(_await_ready(), timeout=15) + except Exception as exc: + logger.warning( + "MCP OAuth '%s': ready poll failed: %s", + server_name, exc, + ) # A successful OAuth recovery is independent evidence that the # server is viable again, so close the circuit breaker here — diff --git a/tools/process_registry.py b/tools/process_registry.py index f739b51ea2ce..6679d74029c3 100644 --- a/tools/process_registry.py +++ b/tools/process_registry.py @@ -697,7 +697,11 @@ def spawn_via_env( ) try: - result = env.execute(bg_command, timeout=timeout) + result = env.execute( + bg_command, + timeout=timeout, + rewrite_compound_background=False, + ) output = result.get("output", "").strip() # Try to extract the PID from the output for line in output.splitlines(): @@ -705,6 +709,15 @@ def spawn_via_env( if line.isdigit(): session.pid = int(line) break + # If the wrapper couldn't produce a PID (for example, syntax + # error or broken redirect), treat it as a failed launch instead + # of exposing a fake running session. + if session.pid is None: + session.exited = True + session.exit_code = int(result.get("returncode", -1)) + if session.exit_code == 0: + session.exit_code = -1 + session.output_buffer = result.get("output", "").strip() except Exception as e: session.exited = True session.exit_code = -1 @@ -723,9 +736,12 @@ def spawn_via_env( with self._lock: self._prune_if_needed() - self._running[session.id] = session + if not session.exited: + self._running[session.id] = session + + if not session.exited: + self._write_checkpoint() - self._write_checkpoint() return session # ----- Reader / Poller Threads ----- diff --git a/tools/terminal_tool.py b/tools/terminal_tool.py index 8351d61eb93d..1a7c32170ef7 100644 --- a/tools/terminal_tool.py +++ b/tools/terminal_tool.py @@ -962,6 +962,23 @@ def register_task_env_overrides(task_id: str, overrides: Dict[str, Any]): """ _task_env_overrides[task_id] = overrides + # If a live environment already exists for this task, a freshly registered + # ``cwd`` override (e.g. the ACP client switching the editor's project root + # mid-session via ``session/load`` / ``session/resume``) must take effect on + # the cached env too. ``terminal_tool`` resolves the per-command cwd as + # ``workdir > env.cwd > config/override cwd`` so that ordinary in-session + # ``cd`` state is preserved; without syncing here the override would sit + # below the (already-set) ``env.cwd`` and be silently ignored once any + # command has run. Pushing it onto the live env keeps ``cd`` tracking intact + # while letting an explicit ACP cwd change win, as the client expects. + new_cwd = overrides.get("cwd") + if isinstance(new_cwd, str) and new_cwd.strip(): + container_id = _resolve_container_task_id(task_id) + with _env_lock: + env = _active_environments.get(container_id) + if env is not None and getattr(env, "cwd", None) is not None: + env.cwd = new_cwd + def clear_task_env_overrides(task_id: str): """ @@ -1718,6 +1735,30 @@ def _resolve_notification_flag_conflict( return watch_patterns, "" +def _resolve_command_cwd( + *, + workdir: Optional[str], + env: Any, + default_cwd: str, +) -> str: + """Return the cwd for a command, preferring the live session cwd. + + ``terminal_tool`` historically re-sent the init-time/config cwd on every + call. That broke session-local ``cd`` state: the environment tracked the + new directory in ``env.cwd``, but foreground/background calls kept forcing + the old cwd back through ``env.execute(..., cwd=...)``. Explicit + ``workdir=`` must still override everything. + """ + if workdir: + return workdir + + live_cwd = getattr(env, "cwd", None) + if isinstance(live_cwd, str) and live_cwd.strip(): + return live_cwd + + return default_cwd + + def terminal_tool( command: str, background: bool = False, @@ -1990,7 +2031,11 @@ def terminal_tool( from tools.process_registry import process_registry session_key = get_current_session_key(default="") - effective_cwd = workdir or cwd + effective_cwd = _resolve_command_cwd( + workdir=workdir, + env=env, + default_cwd=cwd, + ) try: if env_type == "local": proc_session = process_registry.spawn_local( @@ -2207,7 +2252,11 @@ def terminal_tool( try: execute_kwargs = { "timeout": effective_timeout, - "cwd": workdir or cwd, + "cwd": _resolve_command_cwd( + workdir=workdir, + env=env, + default_cwd=cwd, + ), } result = env.execute(command, **execute_kwargs) except Exception as e: diff --git a/tools/tool_output_limits.py b/tools/tool_output_limits.py index fd24a2da352a..0a7cc156460d 100644 --- a/tools/tool_output_limits.py +++ b/tools/tool_output_limits.py @@ -40,6 +40,10 @@ DEFAULT_MAX_LINES = 2000 # file_operations.MAX_LINES DEFAULT_MAX_LINE_LENGTH = 2000 # file_operations.MAX_LINE_LENGTH +# Module-level cache — populated on first call. +# Avoids repeated config file I/O on every tool call. +_cached_limits: dict | None = None + def _coerce_positive_int(value: Any, default: int) -> int: """Return ``value`` as a positive int, or ``default`` on any issue.""" @@ -58,7 +62,14 @@ def get_tool_output_limits() -> Dict[str, int]: Keys: ``max_bytes``, ``max_lines``, ``max_line_length``. Missing or invalid entries fall through to the ``DEFAULT_*`` constants. This function NEVER raises. + + Result is cached for the process lifetime to avoid repeated disk I/O + on every tool call. Call ``_reset_tool_output_limits_cache()`` in + tests that need a fresh read after config changes. """ + global _cached_limits + if _cached_limits is not None: + return _cached_limits try: from hermes_cli.config import load_config cfg = load_config() or {} @@ -68,13 +79,20 @@ def get_tool_output_limits() -> Dict[str, int]: except Exception: section = {} - return { + _cached_limits = { "max_bytes": _coerce_positive_int(section.get("max_bytes"), DEFAULT_MAX_BYTES), "max_lines": _coerce_positive_int(section.get("max_lines"), DEFAULT_MAX_LINES), "max_line_length": _coerce_positive_int( section.get("max_line_length"), DEFAULT_MAX_LINE_LENGTH ), } + return _cached_limits + + +def _reset_tool_output_limits_cache() -> None: + """Reset the cached limits — for tests or after config hot-reload.""" + global _cached_limits + _cached_limits = None def get_max_bytes() -> int: diff --git a/tools/vision_tools.py b/tools/vision_tools.py index 23a0508fed1d..39a4921f1a9a 100644 --- a/tools/vision_tools.py +++ b/tools/vision_tools.py @@ -311,10 +311,21 @@ def _image_to_base64_data_url(image_path: Path, mime_type: Optional[str] = None) return data_url -# Hard limit for vision API payloads (20 MB) — matches the most restrictive -# major provider (Gemini inline data limit). Images above this are rejected. +# Absolute hard ceiling for vision API payloads (20 MB) — above this, no major +# provider accepts the image and we reject outright. _MAX_BASE64_BYTES = 20 * 1024 * 1024 +# Proactive embed cap (4 MB). This is the size we resize an image DOWN to +# before embedding it into conversation history, regardless of the 20 MB hard +# ceiling. Anthropic's per-image base64 limit is 5 MB; once an oversized image +# is baked into history (e.g. a vision tool-result), it is re-sent on every +# subsequent turn and permanently wedges the session with a 400 that retries +# can't clear (the bad bytes are immutable history). Capping at embed time — +# with headroom under 5 MB — is the only durable fix. Matches the post-failure +# shrink target in agent.conversation_compression so behaviour is consistent +# whether we resize proactively or reactively. +_EMBED_TARGET_BYTES = 4 * 1024 * 1024 + # Target size when auto-resizing on API failure (5 MB). After a provider # rejects an image, we downscale to this target and retry once. _RESIZE_TARGET_BYTES = 5 * 1024 * 1024 @@ -656,11 +667,21 @@ async def _vision_analyze_native( temp_image_path, mime_type=detected_mime_type, ) - # Honour the same hard cap as the legacy path. Resize if needed. - if len(image_data_url) > _MAX_BASE64_BYTES: + # Proactive embed cap: this image gets baked into conversation + # history and re-sent on every subsequent turn. Anthropic rejects + # any single base64 image over 5 MB with a 400, and because history + # is immutable, an oversized embed permanently wedges the session — + # retries can't clear bytes that are already in the request. Resize + # DOWN to the embed target (4 MB, headroom under 5 MB) whenever the + # payload exceeds it, not just at the 20 MB hard ceiling. + if len(image_data_url) > _EMBED_TARGET_BYTES: image_data_url = _resize_image_for_vision( temp_image_path, mime_type=detected_mime_type, + max_base64_bytes=_EMBED_TARGET_BYTES, ) + # If even resizing can't get under the absolute hard ceiling, + # there's nothing more we can do — reject rather than embed a + # session-wedging payload. if len(image_data_url) > _MAX_BASE64_BYTES: return tool_error( f"Image too large for vision API: base64 payload is " diff --git a/tools/voice_mode.py b/tools/voice_mode.py index e98fcef8857b..5d75f3c2068c 100644 --- a/tools/voice_mode.py +++ b/tools/voice_mode.py @@ -85,6 +85,59 @@ def _termux_voice_capture_available() -> bool: return _termux_microphone_command() is not None and _termux_api_app_installed() +def _pulse_socket_reachable() -> bool: + """Return True if a PulseAudio/PipeWire socket is reachable on disk. + + Covers the common case where a sound server runs locally (e.g. on a + remote SSH host) without ``PULSE_SERVER``/``PIPEWIRE_REMOTE`` being set -- + the client just connects to the default socket under the runtime dir. + We look at ``PULSE_SERVER`` unix paths, ``PULSE_RUNTIME_PATH``, and + ``XDG_RUNTIME_DIR`` for a ``pulse/native`` or ``pipewire-0`` socket + (issue #35622). + """ + import socket + import stat + + candidates: List[str] = [] + + pulse_server = os.environ.get('PULSE_SERVER', '') + # PULSE_SERVER may be "unix:/path", "unix:/path;..." or a bare path. + for part in pulse_server.split(';'): + part = part.strip() + if part.startswith('unix:'): + candidates.append(part[len('unix:'):]) + + pulse_runtime = os.environ.get('PULSE_RUNTIME_PATH') + if pulse_runtime: + candidates.append(os.path.join(pulse_runtime, 'native')) + + xdg_runtime = os.environ.get('XDG_RUNTIME_DIR') + if xdg_runtime: + candidates.append(os.path.join(xdg_runtime, 'pulse', 'native')) + candidates.append(os.path.join(xdg_runtime, 'pipewire-0')) + + for path in candidates: + if not path: + continue + try: + if not stat.S_ISSOCK(os.stat(path).st_mode): + continue + except OSError: + continue + # Confirm the socket actually accepts a connection -- a stale socket + # file left by a dead server should not count as reachable. + sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + try: + sock.settimeout(0.5) + sock.connect(path) + return True + except OSError: + continue + finally: + sock.close() + return False + + def detect_audio_environment() -> dict: """Detect if the current environment supports audio I/O. @@ -98,12 +151,25 @@ def detect_audio_environment() -> dict: termux_app_installed = _termux_api_app_installed() termux_capture = bool(termux_mic_cmd and termux_app_installed) has_forwarded_audio = bool( - os.environ.get('PULSE_SERVER') or os.environ.get('PIPEWIRE_REMOTE') + os.environ.get('PULSE_SERVER') + or os.environ.get('PIPEWIRE_REMOTE') + or _pulse_socket_reachable() ) - # SSH detection + # SSH detection -- normally no audio devices, but honor a reachable + # sound server (PulseAudio/PipeWire socket or forwarding env vars), which + # works fine over SSH (issue #35622). if any(os.environ.get(v) for v in ('SSH_CLIENT', 'SSH_TTY', 'SSH_CONNECTION')): - warnings.append("Running over SSH -- no audio devices available") + if has_forwarded_audio: + notices.append("Running over SSH with a reachable PulseAudio/PipeWire sound server") + else: + warnings.append( + "Running over SSH -- no audio devices available.\n" + " If a sound server (PulseAudio/PipeWire) is running on this host,\n" + " point Hermes at it, e.g.:\n" + " export XDG_RUNTIME_DIR=/run/user/$(id -u)\n" + " # or: export PULSE_SERVER=unix:$XDG_RUNTIME_DIR/pulse/native" + ) # Docker/Podman container detection — honor host audio forwarding. # When the user mounts a PulseAudio/PipeWire socket into the container diff --git a/ui-tui/src/__tests__/createGatewayEventHandler.test.ts b/ui-tui/src/__tests__/createGatewayEventHandler.test.ts index 897875b2c032..afebc4d10aca 100644 --- a/ui-tui/src/__tests__/createGatewayEventHandler.test.ts +++ b/ui-tui/src/__tests__/createGatewayEventHandler.test.ts @@ -681,6 +681,31 @@ describe('createGatewayEventHandler', () => { expect(resumeById).not.toHaveBeenCalled() }) + it('on gateway.ready after a crash, resumes the recovered session once and skips forge', async () => { + const appended: Msg[] = [] + const newSession = vi.fn() + const resumeById = vi.fn() + const ctx = buildCtx(appended) + + ctx.session.newSession = newSession + // Mimic resumeById's synchronous status write so the test proves the + // "recovering session…" label is applied *after* (and survives) it. + ctx.session.resumeById = resumeById.mockImplementation(() => patchUiState({ status: 'resuming…' })) + ctx.session.STARTUP_RESUME_ID = '' + ctx.session.recoverSidRef = ref('sess-crashed') + + const onEvent = createGatewayEventHandler(ctx) + + onEvent({ payload: {}, type: 'gateway.ready' } as any) + + await vi.waitFor(() => expect(resumeById).toHaveBeenCalledWith('sess-crashed')) + expect(newSession).not.toHaveBeenCalled() + // One-shot: the ref is consumed so a later ordinary restart forges/resumes + // per config instead of re-resuming the recovered session. + expect(ctx.session.recoverSidRef.current).toBeNull() + expect(getUiState().status).toBe('recovering session…') + }) + it('on gateway.ready with auto_resume on and a recent session, resumes it', async () => { const appended: Msg[] = [] const newSession = vi.fn() diff --git a/ui-tui/src/__tests__/gatewayRecovery.test.ts b/ui-tui/src/__tests__/gatewayRecovery.test.ts new file mode 100644 index 000000000000..5ede200a9bbf --- /dev/null +++ b/ui-tui/src/__tests__/gatewayRecovery.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, it } from 'vitest' + +import { GATEWAY_RECOVERY_LIMIT, GATEWAY_RECOVERY_WINDOW_MS, planGatewayRecovery } from '../app/gatewayRecovery.js' + +describe('planGatewayRecovery', () => { + it('recovers the live session and records the attempt', () => { + const plan = planGatewayRecovery('sess-1', null, [], 1000) + + expect(plan).toEqual({ attempts: [1000], recover: true, sid: 'sess-1' }) + }) + + it('does not recover when there is no session to resume', () => { + expect(planGatewayRecovery(null, null, [], 1000)).toEqual({ attempts: [], recover: false, sid: null }) + }) + + it('keeps retrying the recovery target through a startup crash-loop, bounded by the budget', () => { + // First exit: live sid present. + let attempts: number[] = [] + let plan = planGatewayRecovery('sess-1', null, attempts, 0) + + expect(plan.recover).toBe(true) + expect(plan.sid).toBe('sess-1') + attempts = plan.attempts + + // Respawn crash-loops before gateway.ready: live sid is now null, but the + // recovery target carries it forward so we keep trying up to the budget. + for (let i = 1; i < GATEWAY_RECOVERY_LIMIT; i++) { + plan = planGatewayRecovery(null, 'sess-1', attempts, i) + expect(plan.recover).toBe(true) + expect(plan.sid).toBe('sess-1') + attempts = plan.attempts + } + + // Budget exhausted: fall back to the inert state instead of spawn-storming. + plan = planGatewayRecovery(null, 'sess-1', attempts, GATEWAY_RECOVERY_LIMIT) + expect(plan.recover).toBe(false) + expect(plan.sid).toBe('sess-1') + }) + + it('prunes attempts older than the window so recovery re-arms', () => { + const old = Array.from({ length: GATEWAY_RECOVERY_LIMIT }, (_, i) => i) + const plan = planGatewayRecovery('sess-1', null, old, GATEWAY_RECOVERY_WINDOW_MS + 100) + + expect(plan.attempts).toEqual([GATEWAY_RECOVERY_WINDOW_MS + 100]) + expect(plan.recover).toBe(true) + }) +}) diff --git a/ui-tui/src/__tests__/parentLog.test.ts b/ui-tui/src/__tests__/parentLog.test.ts new file mode 100644 index 000000000000..2a910c7cfd91 --- /dev/null +++ b/ui-tui/src/__tests__/parentLog.test.ts @@ -0,0 +1,75 @@ +import { mkdtempSync, readFileSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +// parentLog gates itself off under VITEST so unit tests can't pollute a real +// ~/.hermes. To exercise the real persistence path we clear that gate, point +// HERMES_HOME at a temp dir, and re-import the module fresh (path + enabled +// flag are captured at module load). +const loadFresh = async (home: string) => { + vi.resetModules() + vi.stubEnv('VITEST', '') + vi.stubEnv('HERMES_HOME', home) + + return import('../lib/parentLog.js') +} + +describe('recordParentLifecycle', () => { + let home: string + + beforeEach(() => { + home = mkdtempSync(join(tmpdir(), 'hermes-parentlog-')) + }) + + afterEach(() => { + vi.unstubAllEnvs() + rmSync(home, { force: true, recursive: true }) + }) + + it('appends a timestamped breadcrumb to logs/tui_gateway_crash.log', async () => { + const { recordParentLifecycle } = await loadFresh(home) + + recordParentLifecycle('graceful-exit received signal=SIGHUP → killing gateway') + + const contents = readFileSync(join(home, 'logs', 'tui_gateway_crash.log'), 'utf8') + + expect(contents).toContain('[tui-parent]') + expect(contents).toContain('graceful-exit received signal=SIGHUP → killing gateway') + expect(contents).toMatch(/\d{4}-\d{2}-\d{2}T/) + }) + + it('collapses embedded newlines so a value stays one breadcrumb', async () => { + const { recordParentLifecycle } = await loadFresh(home) + + recordParentLifecycle('uncaughtException: boom\n at foo()\r\n at bar()') + + const lines = readFileSync(join(home, 'logs', 'tui_gateway_crash.log'), 'utf8').trimEnd().split('\n') + + expect(lines).toHaveLength(1) + expect(lines[0]).toContain('boom ↵ at foo() ↵ at bar()') + }) + + it('caps an oversized breadcrumb so it cannot bloat the shared crash log', async () => { + const { recordParentLifecycle } = await loadFresh(home) + + recordParentLifecycle('x'.repeat(10_000)) + + const line = readFileSync(join(home, 'logs', 'tui_gateway_crash.log'), 'utf8') + + expect(line).toContain('[truncated 10000 chars]') + expect(line.length).toBeLessThan(4_500) + }) + + it('is a no-op under VITEST so tests stay hermetic', async () => { + vi.resetModules() + vi.stubEnv('VITEST', 'true') + vi.stubEnv('HERMES_HOME', home) + + const { recordParentLifecycle } = await import('../lib/parentLog.js') + + expect(() => recordParentLifecycle('should not be written')).not.toThrow() + expect(() => readFileSync(join(home, 'logs', 'tui_gateway_crash.log'), 'utf8')).toThrow() + }) +}) diff --git a/ui-tui/src/app/createGatewayEventHandler.ts b/ui-tui/src/app/createGatewayEventHandler.ts index 70264b0c7f93..987518a4460c 100644 --- a/ui-tui/src/app/createGatewayEventHandler.ts +++ b/ui-tui/src/app/createGatewayEventHandler.ts @@ -76,7 +76,7 @@ const normalizeSubagentStatus = (status: unknown, fallback: SubagentStatus): Sub export function createGatewayEventHandler(ctx: GatewayEventHandlerContext): (ev: GatewayEvent) => void { const { rpc } = ctx.gateway - const { STARTUP_RESUME_ID, newSession, resumeById, setCatalog } = ctx.session + const { STARTUP_RESUME_ID, newSession, recoverSidRef, resumeById, setCatalog } = ctx.session const { bellOnComplete, stdout, sys } = ctx.system const { appendMessage, panel, setHistoryItems } = ctx.transcript const { setInput } = ctx.composer @@ -303,6 +303,23 @@ export function createGatewayEventHandler(ctx: GatewayEventHandlerContext): (ev: }) .catch((e: unknown) => turnController.pushActivity(`command catalog unavailable: ${rpcErrorMessage(e)}`, 'info')) + // Crash recovery: a respawn triggered by an unexpected gateway death + // resumes the session that was live, not a brand-new one. One-shot — the + // ref is cleared so an ordinary later restart still forges/resumes per + // config. No startup prompt here (this is mid-session, not a cold boot). + const recoverSid = recoverSidRef?.current + + if (recoverSidRef && recoverSid) { + recoverSidRef.current = null + resumeById(recoverSid) + // After resumeById: it synchronously sets status to 'resuming…' on entry, + // so override it here to keep the distinct "recovering" label visible for + // the duration of the resume RPC (which later flips status to 'ready'). + patchUiState({ status: 'recovering session…' }) + + return + } + if (STARTUP_RESUME_ID) { patchUiState({ status: 'resuming…' }) resumeById(STARTUP_RESUME_ID) diff --git a/ui-tui/src/app/gatewayRecovery.ts b/ui-tui/src/app/gatewayRecovery.ts new file mode 100644 index 000000000000..f68cc5acfb17 --- /dev/null +++ b/ui-tui/src/app/gatewayRecovery.ts @@ -0,0 +1,35 @@ +// Crash-recovery budget for the gateway exit handler. A gateway that +// crash-loops on startup must not let the TUI spawn-storm, so respawn+resume +// attempts are capped to GATEWAY_RECOVERY_LIMIT within a sliding +// GATEWAY_RECOVERY_WINDOW_MS; past the budget the app falls back to the inert +// "gateway exited" state. Kept pure (no refs/UI) so the bound — including the +// crash-loop case — is unit-testable. +export const GATEWAY_RECOVERY_LIMIT = 3 +export const GATEWAY_RECOVERY_WINDOW_MS = 60_000 + +export interface RecoveryPlan { + // Attempt timestamps to persist (the pruned window, plus `now` iff recovering). + attempts: number[] + recover: boolean + // Session to resume — the live sid, or the not-yet-consumed recovery target + // when the live sid was already cleared by a prior exit. + sid: null | string +} + +// Decide whether to respawn+resume after a gateway death. `liveSid` is the +// current session (nulled on the first exit); `recoverSid` is a pending +// recovery target carried across a respawn that died before gateway.ready — +// so a startup crash-loop keeps retrying the same session up to the budget +// instead of stranding it after one attempt. +export function planGatewayRecovery( + liveSid: null | string, + recoverSid: null | string, + attempts: number[], + now: number +): RecoveryPlan { + const sid = liveSid ?? recoverSid + const recent = attempts.filter(t => now - t < GATEWAY_RECOVERY_WINDOW_MS) + const recover = Boolean(sid) && recent.length < GATEWAY_RECOVERY_LIMIT + + return { attempts: recover ? [...recent, now] : recent, recover, sid } +} diff --git a/ui-tui/src/app/interfaces.ts b/ui-tui/src/app/interfaces.ts index 991b69faba48..cbedac59c16c 100644 --- a/ui-tui/src/app/interfaces.ts +++ b/ui-tui/src/app/interfaces.ts @@ -239,6 +239,10 @@ export interface GatewayEventHandlerContext { STARTUP_RESUME_ID: string colsRef: MutableRefObject newSession: (msg?: string, title?: string) => void + // Set by useMainApp's exit handler to the session that was live when the + // gateway died unexpectedly; consumed once by the next `gateway.ready` so a + // respawn resumes that session instead of forging a fresh one. + recoverSidRef?: MutableRefObject resetSession: () => void resumeById: (id: string) => void setCatalog: StateSetter diff --git a/ui-tui/src/app/useMainApp.ts b/ui-tui/src/app/useMainApp.ts index cfa45438399f..43e8a2ed628c 100644 --- a/ui-tui/src/app/useMainApp.ts +++ b/ui-tui/src/app/useMainApp.ts @@ -30,6 +30,7 @@ import type { Msg, PanelSection, SlashCatalog } from '../types.js' import { createGatewayEventHandler } from './createGatewayEventHandler.js' import { createSlashHandler } from './createSlashHandler.js' +import { planGatewayRecovery } from './gatewayRecovery.js' import { getInputSelection } from './inputSelectionStore.js' import { type GatewayRpc, type TranscriptRow } from './interfaces.js' import { $overlayState, patchOverlayState } from './overlayStore.js' @@ -200,6 +201,8 @@ export function useMainApp(gw: GatewayClient) { const terminalHintsShownRef = useRef(new Set()) const historyItemsRef = useRef(historyItems) const lastUserMsgRef = useRef(lastUserMsg) + const recoverSidRef = useRef(null) + const recoveryAtRef = useRef([]) const msgIdsRef = useRef(new WeakMap()) const msgIdSeqRef = useRef(0) const heightCachesRef = useRef(new Map>()) @@ -694,6 +697,7 @@ export function useMainApp(gw: GatewayClient) { STARTUP_RESUME_ID, colsRef, newSession: session.newSession, + recoverSidRef, resetSession: session.resetSession, resumeById: session.resumeById, setCatalog @@ -734,7 +738,34 @@ export function useMainApp(gw: GatewayClient) { const exitHandler = () => { turnController.reset() + + // A still-owned child dying while the TUI is alive is an *unexpected* + // death — a user /quit exits Node before this fires, and a replaced child + // is identity-skipped in GatewayClient. Rather than stranding a long + // session (the user's complaint), respawn the gateway and resume the + // persisted session via the next gateway.ready, so a single crash / OOM / + // signal doesn't lose their work. planGatewayRecovery bounds the attempts + // so a gateway that crash-loops on startup can't spawn-storm, and falls + // back to recoverSidRef when sid was already cleared by a prior exit. + const plan = planGatewayRecovery(getUiState().sid, recoverSidRef.current, recoveryAtRef.current, Date.now()) + + // Clear sid immediately: while the gateway is down, sid-guarded effects + // (session.active_list poll, queue drain) would otherwise fire RPCs at a + // dead/respawning gateway. recoverSidRef carries the session forward, and + // resumeById restores sid once the fresh gateway is ready. + recoveryAtRef.current = plan.attempts patchUiState({ busy: false, sid: null, status: 'gateway exited' }) + + if (plan.recover && plan.sid) { + recoverSidRef.current = plan.sid + turnController.pushActivity('gateway exited · recovering session…', 'warn') + sys('gateway exited — recovering your session (any in-flight reply was lost)') + gw.start() + + return + } + + recoverSidRef.current = null turnController.pushActivity('gateway exited · /logs to inspect', 'error') sys('error: gateway exited') } diff --git a/ui-tui/src/entry.tsx b/ui-tui/src/entry.tsx index 787f738f9f3c..45b9f564cf57 100644 --- a/ui-tui/src/entry.tsx +++ b/ui-tui/src/entry.tsx @@ -11,6 +11,7 @@ import { setupGracefulExit } from './lib/gracefulExit.js' import { formatBytes, type HeapDumpResult, performHeapDump } from './lib/memory.js' import { type MemorySnapshot, startMemoryMonitor } from './lib/memoryMonitor.js' import { openExternalUrl } from './lib/openExternalUrl.js' +import { recordParentLifecycle } from './lib/parentLog.js' import { clampStdoutDimensions } from './lib/terminalDimensions.js' import { resetTerminalModes } from './lib/terminalModes.js' @@ -56,9 +57,14 @@ setupGracefulExit({ onError: (scope, err) => { const message = err instanceof Error ? `${err.name}: ${err.message}\n${err.stack ?? ''}` : String(err) + recordParentLifecycle(`${scope}: ${message.split('\n')[0]?.slice(0, 400) ?? ''}`) process.stderr.write(`hermes-tui lifecycle ${scope}: ${message.slice(0, 2000)}\n`) }, onSignal: signal => { + // The next line in the crash log is the child's `=== SIGTERM received ===` + // (gw.kill forwards SIGTERM regardless of which signal hit us) — this is + // what tells SIGHUP (terminal/SSH dropped) apart from a real SIGTERM. + recordParentLifecycle(`graceful-exit received signal=${signal} → killing gateway`) resetTerminalModes() process.stderr.write(`hermes-tui lifecycle: received ${signal}\n`) } @@ -66,6 +72,10 @@ setupGracefulExit({ const stopMemoryMonitor = startMemoryMonitor({ onCritical: (snap, dump) => { + // process.exit(137) closes the child's stdin → the gateway logs a clean + // EOF, NOT SIGTERM. Recording it here is the only way a crash report can + // attribute a death to Node OOM rather than a signal-driven kill. + recordParentLifecycle(`memory-critical process.exit(137) heap=${formatBytes(snap.heapUsed)} rss=${formatBytes(snap.rss)} dump=${dump?.heapPath ?? 'failed'}`) resetTerminalModes() process.stderr.write(`hermes-tui lifecycle: memory critical exit heap=${formatBytes(snap.heapUsed)} rss=${formatBytes(snap.rss)}\n`) process.stderr.write(dumpNotice(snap, dump)) diff --git a/ui-tui/src/gatewayClient.ts b/ui-tui/src/gatewayClient.ts index f3121152c906..37bfa881aba5 100644 --- a/ui-tui/src/gatewayClient.ts +++ b/ui-tui/src/gatewayClient.ts @@ -6,6 +6,7 @@ import { createInterface } from 'node:readline' import type { GatewayEvent } from './gatewayTypes.js' import { CircularBuffer } from './lib/circularBuffer.js' +import { recordParentLifecycle } from './lib/parentLog.js' const MAX_GATEWAY_LOG_LINES = 200 const MAX_LOG_LINE_BYTES = 4096 @@ -237,7 +238,7 @@ export class GatewayClient extends EventEmitter { // readable on slow boots. const stderrTail = this.getLogTail(20) - this.pushLog(`[startup] timed out waiting for gateway.ready (python=${python}, cwd=${cwd})`) + this.lifecycle(`[startup] timed out waiting for gateway.ready (python=${python}, cwd=${cwd})`) this.publish({ type: 'gateway.start_timeout', payload: { cwd, python, stderr_tail: stderrTail } @@ -248,7 +249,7 @@ export class GatewayClient extends EventEmitter { private handleTransportExit(code: null | number, reason?: string) { this.clearReadyTimer() this.closeSidecarSocket() - this.pushLog(`[lifecycle] transport exit code=${code ?? 'null'} reason=${reason ?? 'none'}`) + this.lifecycle(`[lifecycle] transport exit code=${code ?? 'null'} reason=${reason ?? 'none'}`) this.rejectPending(new Error(reason || `gateway exited${code === null ? '' : ` (${code})`}`)) if (this.subscribed) { @@ -335,7 +336,7 @@ export class GatewayClient extends EventEmitter { env.PYTHONPATH = pyPath ? `${root}${delimiter}${pyPath}` : root this.startReadyTimer(python, cwd) this.proc = spawn(python, ['-m', 'tui_gateway.entry'], { cwd, env, stdio: ['pipe', 'pipe', 'pipe'] }) - this.pushLog(`[lifecycle] spawned gateway child ${describeChild(this.proc)} python=${python} cwd=${cwd}`) + this.lifecycle(`[lifecycle] spawned gateway child ${describeChild(this.proc)} python=${python} cwd=${cwd}`) this.stdoutRl = createInterface({ input: this.proc.stdout! }) this.stdoutRl.on('line', raw => { @@ -372,7 +373,7 @@ export class GatewayClient extends EventEmitter { const line = `[spawn] ${err.message}` - this.pushLog(`[lifecycle] child error ${describeChild(ownedProc)} message=${err.message}`) + this.lifecycle(`[lifecycle] child error ${describeChild(ownedProc)} message=${err.message}`) this.pushLog(line) this.publish({ type: 'gateway.stderr', payload: { line } }) // Detach the reference up front so the late `exit` event for @@ -396,7 +397,7 @@ export class GatewayClient extends EventEmitter { return } - this.pushLog(`[lifecycle] child exit ${describeChild(ownedProc)} code=${code ?? 'null'} signal=${signal ?? 'null'}`) + this.lifecycle(`[lifecycle] child exit ${describeChild(ownedProc)} code=${code ?? 'null'} signal=${signal ?? 'null'}`) this.handleTransportExit(code) }) } @@ -507,7 +508,7 @@ export class GatewayClient extends EventEmitter { this.resetStartupState() if (this.proc && !this.proc.killed && this.proc.exitCode === null) { - this.pushLog(`[lifecycle] replacing live gateway child ${describeChild(this.proc)}`) + this.lifecycle(`[lifecycle] replacing live gateway child ${describeChild(this.proc)}`) this.proc.kill() } @@ -564,6 +565,14 @@ export class GatewayClient extends EventEmitter { this.logs.push(truncateLine(line)) } + // Death-explaining breadcrumbs (spawn / exit / kill / replace) — kept in the + // in-memory tail for /logs AND persisted to the gateway crash log so the + // reason survives a parent exit and lands next to the child's SIGTERM panic. + private lifecycle(line: string) { + this.pushLog(line) + recordParentLifecycle(line) + } + private rejectPending(err: Error) { for (const p of this.pending.values()) { clearTimeout(p.timeout) @@ -717,7 +726,7 @@ export class GatewayClient extends EventEmitter { const proc = this.proc const killed = proc?.kill() - this.pushLog(`[lifecycle] GatewayClient.kill reason=${reason} ${describeChild(proc)} killResult=${killed ?? 'none'}`) + this.lifecycle(`[lifecycle] GatewayClient.kill reason=${reason} ${describeChild(proc)} killResult=${killed ?? 'none'}`) this.closeGatewaySocket() this.closeSidecarSocket() this.clearReadyTimer() diff --git a/ui-tui/src/lib/parentLog.ts b/ui-tui/src/lib/parentLog.ts new file mode 100644 index 000000000000..24f45855239b --- /dev/null +++ b/ui-tui/src/lib/parentLog.ts @@ -0,0 +1,57 @@ +import { appendFileSync, mkdirSync } from 'node:fs' +import { homedir } from 'node:os' +import { join } from 'node:path' + +// Mirror the Python gateway's panic log (tui_gateway/server.py::_CRASH_LOG) from +// the Node parent so lifecycle breadcrumbs interleave, by timestamp, with the +// child's `=== SIGTERM received ===` / `=== gateway exit ===` entries. +// +// A backend SIGTERM is *usually* a parent action — `gw.kill()` (graceful-exit on +// a signal to Node, or an explicit /quit) or `start()` replacing a live child — +// but it can also come straight from an external supervisor (s6, a cgroup OOM +// reaper, a stray `kill`) signalling the child directly. Telling those apart is +// exactly the point: #31051 left these breadcrumbs in an in-memory CircularBuffer +// that dies with the process, so SIGTERM crash reports arrived with no parent +// context. A `[tui-parent]` line immediately before the child's panic means a +// parent kill; its absence *suggests* an external signal — not definitive, +// since this logger is best-effort (disabled under VITEST, and a failed append +// is swallowed). Persisting the death-explaining events here is what makes that +// distinction (and a memory-critical `process.exit(137)`, which closes stdin → +// clean EOF, not SIGTERM) diagnosable after the fact. +const logDir = join(process.env.HERMES_HOME?.trim() || join(homedir(), '.hermes'), 'logs') +const CRASH_LOG = join(logDir, 'tui_gateway_crash.log') + +// Skipped under vitest so unit tests exercising start()/kill() can't write into +// a real ~/.hermes (tests must stay hermetic — see AGENTS.md). +const enabled = !process.env.VITEST +// Slice a single breadcrumb's value to MAX_BREADCRUMB chars (a short +// "[truncated …]" marker is appended, so the written line is slightly longer) +// so a pathological value (e.g. a giant error) can't bloat the shared crash log +// or add noticeable blocking on the synchronous append. Mirrors the spirit of +// GatewayClient's in-memory log-line cap. +const MAX_BREADCRUMB = 4096 +let warned = false + +export function recordParentLifecycle(line: string): void { + if (!enabled) { + return + } + + try { + // Collapse embedded newlines so a multi-line value (e.g. an error message) + // stays one breadcrumb and can't masquerade as a separate log entry or as + // the child's panic output sharing this file. + const oneLine = line.replace(/[\r\n]+/g, ' ↵ ') + + const capped = + oneLine.length > MAX_BREADCRUMB ? `${oneLine.slice(0, MAX_BREADCRUMB)}… [truncated ${oneLine.length} chars]` : oneLine + + mkdirSync(logDir, { recursive: true }) + appendFileSync(CRASH_LOG, `[tui-parent] ${new Date().toISOString()} ${capped}\n`) + } catch { + if (!warned) { + warned = true + process.stderr.write('hermes-tui: parent lifecycle log unavailable\n') + } + } +} diff --git a/website/docs/guides/run-hermes-with-nous-portal.md b/website/docs/guides/run-hermes-with-nous-portal.md index a8ac20478e59..c810d1e1ccdc 100644 --- a/website/docs/guides/run-hermes-with-nous-portal.md +++ b/website/docs/guides/run-hermes-with-nous-portal.md @@ -136,6 +136,8 @@ hermes tools # → TTS → "Nous Subscription" (recommended) ``` +These rows appear in `hermes tools` even before you've logged into Nous Portal — if you pick "Nous Subscription" without an active session, Hermes runs the Portal login inline (without changing your inference provider or your other tools). + Verify your mix with: ```bash diff --git a/website/docs/integrations/nous-portal.md b/website/docs/integrations/nous-portal.md index ddf688d87524..24e914793082 100644 --- a/website/docs/integrations/nous-portal.md +++ b/website/docs/integrations/nous-portal.md @@ -188,7 +188,7 @@ hermes tools # → TTS → "Nous Subscription" ``` -The Tool Gateway is opt-in per tool, not all-or-nothing. See the [Tool Gateway docs](/user-guide/features/tool-gateway) for the full per-tool configuration matrix. +The Tool Gateway is opt-in per tool, not all-or-nothing. The managed backends show up in `hermes tools` whether or not you're logged into Nous Portal — if you pick "Nous Subscription" before authenticating, Hermes runs the Portal login inline (it won't change your inference provider or touch your other tools). See the [Tool Gateway docs](/user-guide/features/tool-gateway) for the full per-tool configuration matrix. ### Subscription management diff --git a/website/docs/reference/toolsets-reference.md b/website/docs/reference/toolsets-reference.md index a9c3d6b8d84f..831416dd0269 100644 --- a/website/docs/reference/toolsets-reference.md +++ b/website/docs/reference/toolsets-reference.md @@ -67,7 +67,7 @@ Or in-session: | `computer_use` | `computer_use` | Background macOS desktop control via cua-driver — does not steal cursor/focus. Works with any tool-capable model. macOS only; requires `cua-driver` on `$PATH`. | | `image_gen` | `image_generate` | Text-to-image generation via FAL.ai (with opt-in OpenAI / xAI backends). | | `video_gen` | `video_generate` | Text-to-video and image-to-video via plugin-registered backends (xAI Grok-Imagine, FAL.ai Veo 3.1 / Pixverse v6 / Kling O3). Pass `image_url` to animate an image; omit it for text-to-video. | -| `kanban` | `kanban_block`, `kanban_comment`, `kanban_complete`, `kanban_create`, `kanban_heartbeat`, `kanban_link`, `kanban_list`, `kanban_show`, `kanban_unblock` | Multi-agent coordination tools. Registered for dispatcher-spawned task workers (`HERMES_KANBAN_TASK`) and for profiles that explicitly enable the `kanban` toolset. Workers mark tasks done, block, heartbeat, comment, and create/link follow-up tasks; orchestrator profiles additionally get board-routing tools like list/unblock. | +| `kanban` | `kanban_block`, `kanban_comment`, `kanban_complete`, `kanban_create`, `kanban_heartbeat`, `kanban_link`, `kanban_list`, `kanban_show`, `kanban_unblock` | Multi-agent coordination tools. Registered for dispatcher-spawned task workers (`HERMES_KANBAN_TASK`) and for profiles that explicitly list the `kanban` toolset by name (the `all`/`*` wildcard does **not** enable it). Workers mark tasks done, block, heartbeat, comment, and create/link follow-up tasks; orchestrator profiles additionally get board-routing tools like list/unblock. | | `memory` | `memory` | Persistent cross-session memory management. | | `messaging` | `send_message` | Send messages to other platforms (Telegram, Discord, etc.) from within a session. | | `moa` | `mixture_of_agents` | Multi-model consensus via Mixture of Agents. | @@ -156,6 +156,11 @@ custom_toolsets: - `all` or `*` — expands to every registered toolset (built-in + dynamic + plugin) +A handful of tools have an additional availability check on top of toolset membership and are **not** turned on by `all`/`*` alone: + +- **Capability-gated** tools (browser, `computer_use`, `code_execution`, Feishu, Home Assistant, cronjob) appear only when their backend/credential prerequisite is configured. +- **Workflow-gated** tools — the `kanban` toolset — are deliberately opt-in. `all`/`*` does **not** enable kanban; you must list `kanban` explicitly (or be a dispatcher-spawned worker with `HERMES_KANBAN_TASK` set). Kanban tools mutate shared board state, so they stay off by default even under `all`. + ## Relationship to `hermes tools` The `hermes tools` command provides a curses-based UI for toggling individual tools on or off per platform. This operates at the tool level (finer than toolsets) and persists to `config.yaml`. Disabled tools are filtered out even if their toolset is enabled. diff --git a/website/docs/user-guide/features/kanban.md b/website/docs/user-guide/features/kanban.md index 0192f9c6461c..4c8ae55e8fc4 100644 --- a/website/docs/user-guide/features/kanban.md +++ b/website/docs/user-guide/features/kanban.md @@ -428,6 +428,20 @@ hermes kanban create "audit auth flow" \ These skills are **additive** to the built-in `kanban-worker` — the dispatcher emits one `--skills ` flag for each (and for the built-in), so the worker spawns with all of them loaded. The skill names must match skills that are actually installed on the assignee's profile (run `hermes skills list` to see what's available); there's no runtime install. +### Goal-mode cards (`--goal`) + +By default each worker gets **one shot** at its card — do the work, call `kanban_complete`/`kanban_block`, exit. Pass `--goal` (CLI) or `goal_mode=True` (the `kanban_create` tool / dashboard) to instead run that worker in a **goal loop**, the same Ralph-style engine behind the `/goal` slash command: after every turn an auxiliary judge checks the worker's output against the card's title + body (treated as the acceptance criteria), and if the work isn't done — and the turn budget remains — the worker keeps going **in the same session** until the judge agrees, the worker terminates the task itself, or the budget runs out (which **blocks** the card for human review rather than exiting silently). + +```bash +hermes kanban create "Translate the docs site to French" \ + --body "Acceptance: every page translated, no English left, links intact." \ + --assignee linguist \ + --goal \ + --goal-max-turns 15 # optional; default 20 +``` + +Use it for open-ended, multi-step, or "keep going until X is true" cards. Skip it for cheap one-shot work — the per-turn judge overhead isn't worth it, and the dispatcher's existing retry/circuit-breaker already handles transient worker failures. The judge is only as good as your goal text, so write the body as **explicit acceptance criteria**. + ### The orchestrator skill A **well-behaved orchestrator does not do the work itself.** It decomposes the user's goal into tasks, links them, assigns each to one of the profiles you've set up, and steps back. The `kanban-orchestrator` skill encodes this as tool-call patterns: anti-temptation rules, a Step-0 profile-discovery prompt (the dispatcher silently fails on unknown assignee names, so the orchestrator must ground every card in profiles that actually exist on your machine), and a decomposition playbook keyed on `kanban_create` / `kanban_link` / `kanban_comment`. @@ -632,6 +646,7 @@ hermes kanban create "" [--body ...] [--assignee <profile>] [--priority N] [--triage] [--idempotency-key KEY] [--max-runtime 30m|2h|1d|<seconds>] [--max-retries N] + [--goal] [--goal-max-turns N] [--skill <name>]... [--json] hermes kanban list [--mine] [--assignee P] [--status S] [--tenant T] [--archived] diff --git a/website/docs/user-guide/features/tool-gateway.md b/website/docs/user-guide/features/tool-gateway.md index 6e7a528d736c..edb93b0f6c6f 100644 --- a/website/docs/user-guide/features/tool-gateway.md +++ b/website/docs/user-guide/features/tool-gateway.md @@ -39,19 +39,23 @@ Bring your own keys anytime — per-tool, whenever you want to. The gateway isn' ## Get started -The fastest path for a fresh install: +There are three ways in — pick whichever fits where you are: ```bash -hermes setup --portal # Nous OAuth, set Nous as provider, and turn on the Tool Gateway in one go +hermes setup --portal # Fresh install: Nous OAuth + set Nous as provider + turn on the Tool Gateway in one go ``` -Already have Hermes configured? Just switch your provider: +```bash +hermes model # Switch your inference provider to Nous Portal — Hermes then offers to turn on the gateway for all tools +``` ```bash -hermes model # Pick Nous Portal — Hermes will offer to turn on the Tool Gateway +hermes tools # Enable the gateway per-tool — pick "Nous Subscription" for any tool you want ``` -When you select Nous Portal, Hermes offers to turn on the Tool Gateway. Accept, and you're done — every supported tool is live on the next run. +`hermes setup --portal` and `hermes model` are the all-at-once paths: log in once, optionally flip every tool to the gateway. `hermes tools` is the à la carte path — turn on just the tools you want, one at a time. + +**You don't have to log in first.** With `hermes tools`, the Nous-managed backends (Web search, Image, Video, TTS, Browser) are always listed, even if you've never signed into Nous Portal. Select one and Hermes runs the Portal login right there if you aren't already authenticated — no need to run `hermes model` beforehand. If your Nous OAuth is already active, selecting the backend enables it immediately with no extra prompt. This path only logs you in and turns on the one tool you picked — it does **not** switch your inference provider, and it does **not** prompt you to enable the gateway for every other tool. Check what's active at any time: @@ -92,7 +96,7 @@ Switch any tool at any time via: hermes tools # Interactive picker for each tool category ``` -Select the tool, pick **Nous Subscription** as the provider (or any direct provider you prefer). No config editing required. +Select the tool, pick **Nous Subscription** as the provider (or any direct provider you prefer). No config editing required. If you aren't logged into Nous Portal yet, picking **Nous Subscription** kicks off the Portal login inline — you don't need to authenticate through `hermes model` first. ## Using individual image models