diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py index 01ea45d7be2..86a1c765a78 100644 --- a/agent/auxiliary_client.py +++ b/agent/auxiliary_client.py @@ -3079,23 +3079,20 @@ def _try_configured_fallback_chain( if not fb_provider or fb_provider.lower() == skip: continue fb_model = str(entry.get("model", "")).strip() or None - fb_base_url = str(entry.get("base_url", "")).strip() or None - fb_api_key = str(entry.get("api_key", "")).strip() or None label = f"fallback_chain[{i}]({fb_provider})" try: - fb_client = _resolve_single_provider( - fb_provider, fb_model, fb_base_url, fb_api_key) + fb_client, resolved_model = _resolve_fallback_entry(entry) except Exception: - fb_client = None + fb_client, resolved_model = None, None if fb_client is not None: logger.info( "Auxiliary %s: %s on %s — configured fallback to %s (%s)", - task, reason, failed_provider, label, fb_model or "default", + task, reason, failed_provider, label, resolved_model or fb_model or "default", ) - return fb_client, fb_model, label + return fb_client, resolved_model or fb_model, label tried.append(label) if tried: @@ -3106,6 +3103,103 @@ def _try_configured_fallback_chain( return None, None, "" +def _fallback_entry_api_key(entry: Dict[str, Any]) -> Optional[str]: + """Resolve inline or env-backed API key from a fallback-chain entry.""" + explicit = str(entry.get("api_key") or "").strip() + if explicit: + return explicit + key_env = str(entry.get("key_env") or entry.get("api_key_env") or "").strip() + if key_env: + return os.getenv(key_env, "").strip() or None + return None + + +def _resolve_fallback_entry(entry: Dict[str, Any]) -> Tuple[Optional[Any], Optional[str]]: + """Resolve one fallback entry through the central provider router.""" + provider = str(entry.get("provider") or "").strip() + model = str(entry.get("model") or "").strip() or None + if not provider or not model: + return None, None + base_url = str(entry.get("base_url") or "").strip() or None + api_key = _fallback_entry_api_key(entry) + api_mode = str(entry.get("api_mode") or entry.get("transport") or "").strip() or None + return resolve_provider_client( + provider, + model=model, + explicit_base_url=base_url, + explicit_api_key=api_key, + api_mode=api_mode, + ) + + +def _try_main_fallback_chain( + task: Optional[str], + failed_provider: str = "", + reason: str = "error", +) -> Tuple[Optional[Any], Optional[str], str]: + """Try the top-level main-agent fallback chain for an auxiliary call. + + ``provider: auto`` auxiliary tasks should respect the user's declared + main fallback policy before dropping into Hermes' built-in discovery + chain. The top-level chain is read through ``get_fallback_chain`` so + both modern ``fallback_providers`` and legacy ``fallback_model`` entries + participate in the same order as the main agent. + """ + try: + from hermes_cli.config import load_config + from hermes_cli.fallback_config import get_fallback_chain + + chain = get_fallback_chain(load_config()) + except Exception as exc: + logger.debug("Auxiliary %s: could not load main fallback chain: %s", task or "call", exc) + return None, None, "" + + if not chain: + return None, None, "" + + failed_norm = (failed_provider or "").strip().lower() + main_norm = (_read_main_provider() or "").strip().lower() + skip = {p for p in (failed_norm, main_norm, "auto") if p} + tried: List[str] = [] + + for i, entry in enumerate(chain): + if not isinstance(entry, dict): + continue + fb_provider = str(entry.get("provider") or "").strip() + fb_model = str(entry.get("model") or "").strip() + if not fb_provider or not fb_model: + continue + fb_norm = fb_provider.lower() + label = f"fallback_providers[{i}]({fb_provider})" + if fb_norm in skip: + tried.append(f"{label} (skipped)") + continue + if _is_provider_unhealthy(fb_norm): + _log_skip_unhealthy(fb_norm, task) + tried.append(f"{label} (unhealthy)") + continue + try: + fb_client, resolved_model = _resolve_fallback_entry(entry) + except Exception as exc: + logger.debug("Auxiliary %s: main fallback %s failed to resolve: %s", task or "call", label, exc) + fb_client, resolved_model = None, None + if fb_client is not None: + logger.info( + "Auxiliary %s: %s on %s — main fallback chain to %s (%s)", + task or "call", reason, failed_provider or "auto", label, + resolved_model or fb_model, + ) + return fb_client, resolved_model or fb_model, fb_provider + tried.append(label) + + if tried: + logger.debug( + "Auxiliary %s: main fallback chain exhausted (tried: %s)", + task or "call", ", ".join(tried), + ) + return None, None, "" + + def _resolve_single_provider( provider: str, model: Optional[str] = None, @@ -3116,16 +3210,19 @@ def _resolve_single_provider( Uses the existing provider resolution infrastructure where possible. """ - # Reuse resolve_provider_client which handles provider→client mapping + # Reuse resolve_provider_client which handles provider→client mapping. client, resolved_model = resolve_provider_client( provider=provider, model=model, - base_url=base_url, - api_key=api_key, + explicit_base_url=base_url, + explicit_api_key=api_key, ) return client -def _resolve_auto(main_runtime: Optional[Dict[str, Any]] = None) -> Tuple[Optional[OpenAI], Optional[str]]: +def _resolve_auto( + main_runtime: Optional[Dict[str, Any]] = None, + task: Optional[str] = None, +) -> Tuple[Optional[OpenAI], Optional[str]]: """Full auto-detection chain. Priority: @@ -3223,7 +3320,22 @@ def _resolve_auto(main_runtime: Optional[Dict[str, Any]] = None) -> Tuple[Option main_provider, resolved or main_model) return client, resolved or main_model - # ── Step 2: aggregator / fallback chain ────────────────────────────── + # ── Step 2: user-configured fallback policy ───────────────────────── + # In auto mode, respect the task-specific fallback chain first, then the + # main agent's top-level fallback_providers/fallback_model chain. The + # hardcoded provider discovery chain below is only the convenience default + # for users who have not declared a fallback policy. + if task: + fb_client, fb_model, _fb_label = _try_configured_fallback_chain( + task, main_provider or "auto", reason="main provider unavailable") + if fb_client is not None: + return fb_client, fb_model + fb_client, fb_model, _fb_label = _try_main_fallback_chain( + task, main_provider or "auto", reason="main provider unavailable") + if fb_client is not None: + return fb_client, fb_model + + # ── Step 3: aggregator / fallback chain ────────────────────────────── tried = [] for label, try_fn in _get_provider_chain(): if _is_provider_unhealthy(label): @@ -3344,6 +3456,7 @@ def resolve_provider_client( api_mode: str = None, main_runtime: Optional[Dict[str, Any]] = None, is_vision: bool = False, + task: Optional[str] = None, ) -> Tuple[Optional[Any], Optional[str]]: """Central router: given a provider name and optional model, return a configured client with the correct auth, base URL, and API format. @@ -3464,7 +3577,7 @@ def resolve_provider_client( # ── Auto: try all providers in priority order ──────────────────── if provider == "auto": - client, resolved = _resolve_auto(main_runtime=main_runtime) + client, resolved = _resolve_auto(main_runtime=main_runtime, task=task) if client is None: return None, None # When auto-detection lands on a non-OpenRouter provider (e.g. a @@ -4357,11 +4470,16 @@ def _client_cache_key( api_mode: Optional[str] = None, main_runtime: Optional[Dict[str, Any]] = None, is_vision: bool = False, + task: Optional[str] = None, ) -> tuple: runtime = _normalize_main_runtime(main_runtime) runtime_key = tuple(runtime.get(field, "") for field in _MAIN_RUNTIME_FIELDS) if provider == "auto" else () + # `auto` can now resolve through task-specific or main fallback policy, + # so the task participates in the cache key. Non-auto providers keep the + # old cache shape because the explicit provider/model tuple is sufficient. + task_key = (task or "") if provider == "auto" else "" pool_hint = _pool_cache_hint(provider, main_runtime=main_runtime) - return (provider, async_mode, base_url or "", api_key or "", api_mode or "", runtime_key, is_vision, pool_hint) + return (provider, async_mode, base_url or "", api_key or "", api_mode or "", runtime_key, is_vision, task_key, pool_hint) def _store_cached_client(cache_key: tuple, client: Any, default_model: Optional[str], *, bound_loop: Any = None) -> None: @@ -4554,6 +4672,7 @@ def _get_cached_client( api_mode: str = None, main_runtime: Optional[Dict[str, Any]] = None, is_vision: bool = False, + task: Optional[str] = None, ) -> Tuple[Optional[Any], Optional[str]]: """Get or create a cached client for the given provider. @@ -4591,6 +4710,7 @@ def _get_cached_client( api_mode=api_mode, main_runtime=main_runtime, is_vision=is_vision, + task=task, ) with _client_cache_lock: if cache_key in _client_cache: @@ -4635,6 +4755,7 @@ def _get_cached_client( api_mode=api_mode, main_runtime=runtime, is_vision=is_vision, + task=task, ) if client is not None: # For async clients, remember which loop they were created on so we @@ -5140,7 +5261,7 @@ def call_llm( if not resolved_base_url: logger.info("Auxiliary %s: provider %s unavailable, trying auto-detection chain", task or "call", resolved_provider) - client, final_model = _get_cached_client("auto", main_runtime=main_runtime) + client, final_model = _get_cached_client("auto", main_runtime=main_runtime, task=task) if client is None: raise RuntimeError( f"No LLM provider configured for task={task} provider={resolved_provider}. " @@ -5466,14 +5587,19 @@ def call_llm( # Fallback order (#26882, #26803): # 1. User-configured fallback_chain (per-task) if set - # 2. Main agent model (last-resort safety net) - # For auto users (no explicit aux provider), use the full - # auto-detection chain instead — its Step 1 IS the main agent - # model, so users on `auto` already get main-model fallback. + # 2. For auto: top-level main fallback_providers/fallback_model + # 3. For auto: built-in auxiliary discovery chain + # 4. For explicit aux providers: main agent model safety net fb_client, fb_model, fb_label = (None, None, "") if is_auto: - fb_client, fb_model, fb_label = _try_payment_fallback( - resolved_provider, task, reason=reason) + fb_client, fb_model, fb_label = _try_configured_fallback_chain( + task, resolved_provider or "auto", reason=reason) + if fb_client is None: + fb_client, fb_model, fb_label = _try_main_fallback_chain( + task, resolved_provider or "auto", reason=reason) + if fb_client is None: + fb_client, fb_model, fb_label = _try_payment_fallback( + resolved_provider, task, reason=reason) else: fb_client, fb_model, fb_label = _try_configured_fallback_chain( task, resolved_provider or "auto", reason=reason) @@ -5636,7 +5762,7 @@ async def async_call_llm( if not resolved_base_url: logger.info("Auxiliary %s: provider %s unavailable, trying auto-detection chain", task or "call", resolved_provider) - client, final_model = _get_cached_client("auto", async_mode=True) + client, final_model = _get_cached_client("auto", async_mode=True, main_runtime=main_runtime, task=task) if client is None: raise RuntimeError( f"No LLM provider configured for task={task} provider={resolved_provider}. " @@ -5904,13 +6030,19 @@ async def async_call_llm( # Fallback order (#26882, #26803): # 1. User-configured fallback_chain (per-task) if set - # 2. Main agent model (last-resort safety net) - # Auto users get the full auto-detection chain instead — its - # Step 1 IS the main agent model. + # 2. For auto: top-level main fallback_providers/fallback_model + # 3. For auto: built-in auxiliary discovery chain + # 4. For explicit aux providers: main agent model safety net fb_client, fb_model, fb_label = (None, None, "") if is_auto: - fb_client, fb_model, fb_label = _try_payment_fallback( - resolved_provider, task, reason=reason) + fb_client, fb_model, fb_label = _try_configured_fallback_chain( + task, resolved_provider or "auto", reason=reason) + if fb_client is None: + fb_client, fb_model, fb_label = _try_main_fallback_chain( + task, resolved_provider or "auto", reason=reason) + if fb_client is None: + fb_client, fb_model, fb_label = _try_payment_fallback( + resolved_provider, task, reason=reason) else: fb_client, fb_model, fb_label = _try_configured_fallback_chain( task, resolved_provider or "auto", reason=reason) diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions.ts index 4c1b50b83ad..829119f65b4 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions.ts @@ -58,6 +58,7 @@ import { clearSessionTodos } from '@/store/todos' import type { ClientSessionState, + BrowserManageResponse, FileAttachResponse, HandoffFailResponse, HandoffRequestResponse, @@ -1141,6 +1142,81 @@ export function usePromptActions({ } catch (err) { renderSlashOutput(`error: ${err instanceof Error ? err.message : String(err)}`) } + }, + // /browser connect|disconnect|status manages the live CDP connection on + // the gateway host, mirroring the TUI's browser.manage RPC. It mutates + // BROWSER_CDP_URL (and may launch Chrome) in the gateway process — only + // meaningful when that process runs on this machine, so it's gated to + // local connections. A remote gateway would act on the wrong host. + browser: async ctx => { + const resolved = await withSlashOutput(ctx) + + if (!resolved) { + return + } + + const { render: renderSlashOutput, sessionId } = resolved + + if ($connection.get()?.mode === 'remote') { + renderSlashOutput( + '/browser manages a Chromium-family browser on the gateway host — only available when connected to a local gateway.' + ) + + return + } + + const [rawAction = 'status', ...rest] = ctx.arg.trim().split(/\s+/).filter(Boolean) + const cmdAction = rawAction.toLowerCase() + + if (!['connect', 'disconnect', 'status'].includes(cmdAction)) { + renderSlashOutput( + 'usage: /browser [connect|disconnect|status] [url] · persistent: set browser.cdp_url in config.yaml' + ) + + return + } + + const url = cmdAction === 'connect' ? rest.join(' ').trim() || 'http://127.0.0.1:9222' : undefined + + if (url) { + renderSlashOutput(`checking Chromium-family browser remote debugging at ${url}...`) + } + + try { + const result = await requestGateway('browser.manage', { + action: cmdAction, + session_id: sessionId, + ...(url && { url }) + }) + + // Without a streamed session subscription, the gateway bundles its + // progress lines into `messages` — flush them inline. + result?.messages?.forEach(message => renderSlashOutput(message)) + + if (cmdAction === 'status') { + renderSlashOutput( + result?.connected + ? `browser connected: ${result.url || '(url unavailable)'}` + : 'browser not connected (try /browser connect or set browser.cdp_url in config.yaml)' + ) + + return + } + + if (cmdAction === 'disconnect') { + renderSlashOutput('browser disconnected') + + return + } + + if (result?.connected) { + renderSlashOutput('Browser connected to live Chromium-family browser via CDP') + renderSlashOutput(`Endpoint: ${result.url || '(url unavailable)'}`) + renderSlashOutput('next browser tool call will use this CDP endpoint') + } + } catch (err) { + renderSlashOutput(`error: ${err instanceof Error ? err.message : String(err)}`) + } } } diff --git a/apps/desktop/src/app/types.ts b/apps/desktop/src/app/types.ts index 5082b70406d..9500468482c 100644 --- a/apps/desktop/src/app/types.ts +++ b/apps/desktop/src/app/types.ts @@ -46,6 +46,12 @@ export interface SlashExecResponse { warning?: string } +export interface BrowserManageResponse { + connected?: boolean + url?: string + messages?: string[] +} + export interface SessionSteerResponse { // 'queued' == accepted into the live turn's steer slot (injected at the next // tool-result boundary); 'rejected' == no live tool window, caller queues. diff --git a/apps/desktop/src/lib/desktop-slash-commands.test.ts b/apps/desktop/src/lib/desktop-slash-commands.test.ts index d37738173ce..54f5a6f89df 100644 --- a/apps/desktop/src/lib/desktop-slash-commands.test.ts +++ b/apps/desktop/src/lib/desktop-slash-commands.test.ts @@ -52,6 +52,17 @@ describe('desktop slash command curation', () => { expect(desktopSlashUnavailableMessage('/personality')).toBeNull() }) + it('treats /browser as an executable action command (local-gateway connect)', () => { + // /browser used to be terminal-only; it now resolves to a desktop action + // handler that routes browser.manage RPC when the gateway is local. + expect(isDesktopSlashCommand('/browser')).toBe(true) + expect(isDesktopSlashSuggestion('/browser')).toBe(true) + expect(desktopSlashUnavailableMessage('/browser')).toBeNull() + expect(resolveDesktopCommand('/browser')?.surface).toEqual({ kind: 'action', action: 'browser' }) + // Bare /browser expands to its sub-action options in the popover. + expect(resolveDesktopCommand('/browser')?.args).toBe(true) + }) + it('allows aliases to execute without cluttering the popover', () => { expect(isDesktopSlashSuggestion('/reset')).toBe(false) expect(isDesktopSlashCommand('/reset')).toBe(true) diff --git a/apps/desktop/src/lib/desktop-slash-commands.ts b/apps/desktop/src/lib/desktop-slash-commands.ts index d898a6c83f1..f9ae934edf4 100644 --- a/apps/desktop/src/lib/desktop-slash-commands.ts +++ b/apps/desktop/src/lib/desktop-slash-commands.ts @@ -30,6 +30,7 @@ export interface DesktopThemeCommandOption { */ export type DesktopActionId = | 'branch' + | 'browser' | 'handoff' | 'help' | 'new' @@ -103,6 +104,12 @@ const DESKTOP_COMMAND_SPECS: readonly DesktopCommandSpec[] = [ { name: '/skin', description: 'Switch desktop theme or cycle to the next one', surface: action('skin'), args: true }, { name: '/title', description: 'Rename the current session', surface: action('title') }, { name: '/help', description: 'Show desktop slash commands', aliases: ['/commands'], surface: action('help') }, + { + name: '/browser', + description: 'Manage browser CDP connection [connect|disconnect|status] (local gateway only)', + surface: action('browser'), + args: true + }, // Overlay pickers { name: '/model', description: 'Switch the model for this session', surface: picker('model'), hidden: true }, @@ -142,7 +149,7 @@ const DESKTOP_COMMAND_SPECS: readonly DesktopCommandSpec[] = [ // per reason beats 40 identical object literals. const NO_DESKTOP_SURFACE: Record = { terminal: [ - '/browser', '/busy', '/clear', '/compact', '/config', '/copy', '/cron', '/details', + '/busy', '/clear', '/compact', '/config', '/copy', '/cron', '/details', '/exit', '/footer', '/gateway', '/gquota', '/history', '/image', '/indicator', '/logs', '/mouse', '/paste', '/platforms', '/plugins', '/quit', '/redraw', '/reload', '/restart', '/sb', '/set-home', '/sethome', '/snap', '/snapshot', '/statusbar', '/toolsets', '/update', '/verbose' diff --git a/gateway/platforms/base.py b/gateway/platforms/base.py index 205d9cbf509..cda3acc6e58 100644 --- a/gateway/platforms/base.py +++ b/gateway/platforms/base.py @@ -77,6 +77,13 @@ def _thread_metadata_for_source(source, reply_to_message_id: str | None = None) return metadata +def _mark_notify_metadata(metadata: dict | None) -> dict: + """Clone metadata and mark a user-visible reply as notify-worthy.""" + notify_metadata = dict(metadata) if metadata else {} + notify_metadata["notify"] = True + return notify_metadata + + def _reply_anchor_for_event(event) -> str | None: """Return reply_to id for platforms that need reply semantics. @@ -3889,7 +3896,7 @@ class BasePlatformAdapter(ABC): chat_id=event.source.chat_id, content=_text, reply_to=_reply_anchor_for_event(event), - metadata=thread_meta, + metadata=_mark_notify_metadata(thread_meta), ) if _eph_ttl > 0 and _r.success and _r.message_id: self._schedule_ephemeral_delete( @@ -3995,7 +4002,7 @@ class BasePlatformAdapter(ABC): chat_id=event.source.chat_id, content=_text, reply_to=_reply_anchor_for_event(event), - metadata=_thread_meta, + metadata=_mark_notify_metadata(_thread_meta), ) if _eph_ttl > 0 and _r.success and _r.message_id: self._schedule_ephemeral_delete( @@ -4045,7 +4052,7 @@ class BasePlatformAdapter(ABC): chat_id=event.source.chat_id, content=_text, reply_to=_reply_anchor_for_event(event), - metadata=_thread_meta, + metadata=_mark_notify_metadata(_thread_meta), ) if _eph_ttl > 0 and _r.success and _r.message_id: self._schedule_ephemeral_delete( @@ -4268,6 +4275,12 @@ class BasePlatformAdapter(ABC): ) text_content = _recovered + # Final user-visible content (text, TTS, media, files) gets + # the existing notify=True marker. Clone once so typing/status + # metadata stays unmarked and progress bubbles remain + # thread-strict. + _final_thread_metadata = _mark_notify_metadata(_thread_metadata) + # Auto-TTS: if voice message, generate audio FIRST (before sending text) # Gated via ``_should_auto_tts_for_chat``: fires when the chat has # an explicit ``/voice on|tts`` opt-in OR when ``voice.auto_tts`` is @@ -4307,7 +4320,7 @@ class BasePlatformAdapter(ABC): chat_id=event.source.chat_id, audio_path=_tts_path, caption=telegram_tts_caption, - metadata=_thread_metadata, + metadata=_final_thread_metadata, ) _tts_caption_delivered = bool( telegram_tts_caption and getattr(tts_result, "success", False) @@ -4322,23 +4335,11 @@ class BasePlatformAdapter(ABC): if text_content and not _tts_caption_delivered: logger.info("[%s] Sending response (%d chars) to %s", self.name, len(text_content), event.source.chat_id) _reply_anchor = _reply_anchor_for_event(event) - # Mark final response messages for notification delivery. - # Platform adapters that support per-message notification - # control (e.g. Telegram's disable_notification) use this - # flag to override silent-mode and ensure the final - # response triggers a push notification. - # Clone to avoid mutating the metadata shared with the - # typing-indicator task (which must remain unmarked). - if _thread_metadata is not None: - _thread_metadata = dict(_thread_metadata) - _thread_metadata["notify"] = True - else: - _thread_metadata = {"notify": True} result = await self._send_with_retry( chat_id=event.source.chat_id, content=text_content, reply_to=_reply_anchor, - metadata=_thread_metadata, + metadata=_final_thread_metadata, ) _record_delivery(result) @@ -4367,7 +4368,7 @@ class BasePlatformAdapter(ABC): await self.send_multiple_images( chat_id=event.source.chat_id, images=images, - metadata=_thread_metadata, + metadata=_final_thread_metadata, human_delay=human_delay, ) except Exception as batch_err: @@ -4409,7 +4410,7 @@ class BasePlatformAdapter(ABC): await self.send_multiple_images( chat_id=event.source.chat_id, images=_batch, - metadata=_thread_metadata, + metadata=_final_thread_metadata, human_delay=human_delay, ) except Exception as batch_err: @@ -4424,19 +4425,19 @@ class BasePlatformAdapter(ABC): media_result = await self.send_voice( chat_id=event.source.chat_id, audio_path=media_path, - metadata=_thread_metadata, + metadata=_final_thread_metadata, ) elif ext in _VIDEO_EXTS: media_result = await self.send_video( chat_id=event.source.chat_id, video_path=media_path, - metadata=_thread_metadata, + metadata=_final_thread_metadata, ) else: media_result = await self.send_document( chat_id=event.source.chat_id, file_path=media_path, - metadata=_thread_metadata, + metadata=_final_thread_metadata, ) if not media_result.success: @@ -4454,13 +4455,13 @@ class BasePlatformAdapter(ABC): await self.send_video( chat_id=event.source.chat_id, video_path=file_path, - metadata=_thread_metadata, + metadata=_final_thread_metadata, ) else: await self.send_document( chat_id=event.source.chat_id, file_path=file_path, - metadata=_thread_metadata, + metadata=_final_thread_metadata, ) except Exception as file_err: logger.error("[%s] Error sending local file %s: %s", self.name, file_path, file_err) diff --git a/gateway/platforms/email.py b/gateway/platforms/email.py index 7b247cdda27..d2f7e64ac61 100644 --- a/gateway/platforms/email.py +++ b/gateway/platforms/email.py @@ -678,8 +678,13 @@ class EmailAdapter(BasePlatformAdapter): image_url: str, caption: Optional[str] = None, reply_to: Optional[str] = None, + metadata: Optional[Dict[str, Any]] = None, ) -> SendResult: - """Send an image URL as part of an email body.""" + """Send an image URL as part of an email body. + + ``metadata`` is accepted to honor the base-class contract; the + email body send doesn't use it. + """ text = caption or "" text += f"\n\nImage: {image_url}" return await self.send(chat_id, text.strip(), reply_to) diff --git a/gateway/platforms/whatsapp.py b/gateway/platforms/whatsapp.py index d833d5649a2..00ff2c967e7 100644 --- a/gateway/platforms/whatsapp.py +++ b/gateway/platforms/whatsapp.py @@ -846,13 +846,20 @@ class WhatsAppAdapter(WhatsAppBehaviorMixin, BasePlatformAdapter): image_url: str, caption: Optional[str] = None, reply_to: Optional[str] = None, + metadata: Optional[Dict[str, Any]] = None, ) -> SendResult: - """Download image URL to cache, send natively via bridge.""" + """Download image URL to cache, send natively via bridge. + + ``metadata`` is accepted to honor the base-class contract — the + batch sender ``send_multiple_images`` passes it through to every + send path. The bridge media call doesn't use it, matching the + sibling overrides (send_video / send_voice / send_document). + """ try: local_path = await cache_image_from_url(image_url) return await self._send_media_to_bridge(chat_id, local_path, "image", caption) except Exception: - return await super().send_image(chat_id, image_url, caption, reply_to) + return await super().send_image(chat_id, image_url, caption, reply_to, metadata) async def send_image_file( self, @@ -1136,6 +1143,15 @@ class WhatsAppAdapter(WhatsAppBehaviorMixin, BasePlatformAdapter): body = data.get("body", "") if data.get("isGroup"): body = self._clean_bot_mention_text(body, data) + + # If this is a reply, include the quoted message text so the agent + # knows exactly what the user is responding to (fixes "approve" context issue) + quoted_text = str(data.get("quotedText") or "").strip() + if quoted_text and data.get("hasQuotedMessage"): + # Truncate long quoted text to keep prompts reasonable + if len(quoted_text) > 300: + quoted_text = quoted_text[:297] + "..." + body = f"[Replying to: \"{quoted_text}\"]\n{body}" MAX_TEXT_INJECT_BYTES = 100 * 1024 if msg_type == MessageType.DOCUMENT and cached_urls: for doc_path in cached_urls: diff --git a/gateway/run.py b/gateway/run.py index 470d71906c3..b688f3a3613 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -413,6 +413,57 @@ def _resolve_progress_thread_id(platform: Any, source_thread_id: Any, event_mess return None +def _has_platform_display_override(user_config: dict, platform_key: str, setting: str) -> bool: + """Return True when display.platforms. explicitly sets setting.""" + display = user_config.get("display") if isinstance(user_config, dict) else None + if not isinstance(display, dict): + return False + platforms = display.get("platforms") + if not isinstance(platforms, dict): + return False + platform_cfg = platforms.get(platform_key) + return isinstance(platform_cfg, dict) and setting in platform_cfg + + +def _resolve_gateway_display_bool( + user_config: dict, + platform_key: str, + setting: str, + *, + default: bool = False, + platform: Any = None, + require_platform_override_for: set[Any] | None = None, +) -> bool: + """Resolve a boolean display setting with optional platform-only opt-in. + + Some display features expose assistant scratch text rather than deliberate + user-facing output. For high-noise threaded chat surfaces such as + Mattermost, a global opt-in is too broad: they must be enabled with an + explicit display.platforms.. override. + """ + current_platform = _gateway_platform_value(platform or platform_key) + platform_only = { + _gateway_platform_value(candidate) + for candidate in (require_platform_override_for or set()) + } + if ( + current_platform in platform_only + and not _has_platform_display_override(user_config, platform_key, setting) + ): + return False + + from gateway.display_config import resolve_display_setting + + value = resolve_display_setting(user_config, platform_key, setting, default) + if isinstance(value, bool): + return value + if isinstance(value, str): + return value.strip().lower() in {"true", "yes", "1", "on"} + if value is None: + return bool(default) + return bool(value) + + def _telegramize_command_mentions(text: str, platform: Any) -> str: """Rewrite slash-command mentions to Telegram-valid command names. @@ -8989,17 +9040,24 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew source, session_entry, reason="agent-result-compression", ) - # Prepend reasoning/thinking if display is enabled (per-platform) + # Prepend reasoning/thinking if display is enabled (per-platform). + # Mattermost requires explicit per-platform opt-in because this is + # scratch text, not ordinary final-answer content. try: - from gateway.display_config import resolve_display_setting as _rds - _show_reasoning_effective = _rds( + _show_reasoning_effective = _resolve_gateway_display_bool( _load_gateway_config(), _platform_config_key(source.platform), "show_reasoning", - getattr(self, "_show_reasoning", False), + default=bool(getattr(self, "_show_reasoning", False)), + platform=source.platform, + require_platform_override_for={Platform.MATTERMOST}, ) except Exception: - _show_reasoning_effective = getattr(self, "_show_reasoning", False) + _show_reasoning_effective = ( + False + if source.platform == Platform.MATTERMOST + else getattr(self, "_show_reasoning", False) + ) if _show_reasoning_effective and response and not _intentional_silence: last_reasoning = agent_result.get("last_reasoning") if last_reasoning: @@ -13635,18 +13693,32 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew # in chat platforms while opting into concise mid-turn updates. interim_assistant_messages_enabled = ( source.platform != Platform.WEBHOOK - and bool( - resolve_display_setting( - user_config, - platform_key, - "interim_assistant_messages", - True, - ) + and _resolve_gateway_display_bool( + user_config, + platform_key, + "interim_assistant_messages", + default=True, + platform=source.platform, + require_platform_override_for={Platform.MATTERMOST}, ) ) - + # thinking_progress is independent — if enabled, we need the progress + # queue even when tool_progress is off (thinking relay uses same infra). + # Mattermost requires a per-platform opt-in: global scratch-text display + # is too easy to leak into busy public threads. + _thinking_enabled = _resolve_gateway_display_bool( + user_config, + platform_key, + "thinking_progress", + default=False, + platform=source.platform, + require_platform_override_for={Platform.MATTERMOST}, + ) + needs_progress_queue = tool_progress_enabled or _thinking_enabled + + # Queue for progress messages (thread-safe) - progress_queue = queue.Queue() if tool_progress_enabled else None + progress_queue = queue.Queue() if needs_progress_queue else None last_tool = [None] # Mutable container for tracking in closure last_progress_msg = [None] # Track last message for dedup repeat_count = [0] # How many times the same message repeated @@ -13752,6 +13824,24 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew logger.debug("tool-progress onboarding hint failed: %s", _hint_err) return + # "_thinking" is assistant scratch text between tool calls. It + # is never ordinary tool progress: only relay it when the platform + # explicitly opted into thinking_progress. Handle both legacy + # callback shapes: ("_thinking", text) and + # ("reasoning.available", "_thinking", text, ...). + if event_type == "_thinking" or tool_name == "_thinking": + if not _thinking_enabled: + return + thinking_text = preview if tool_name == "_thinking" else tool_name + msg = f"💬 {thinking_text}" if thinking_text else None + if msg: + progress_queue.put(msg) + return + + # If tool_progress is off, only _thinking passes through (above). + # Regular tool calls are suppressed. + if not tool_progress_enabled: + return # Only act on tool.started events (ignore tool.completed, reasoning.available, etc.) if event_type not in {"tool.started",}: @@ -14783,6 +14873,10 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew agent.clarify_callback = _clarify_callback_sync + # Show assistant thinking between tool calls — independent of + # tool_progress mode. Mattermost needs an explicit per-platform + # opt-in so global scratch-text display does not leak into threads. + agent.thinking_progress = _thinking_enabled # Store agent reference for interrupt support agent_holder[0] = agent # Capture the full tool definitions for transcript logging diff --git a/gateway/stream_consumer.py b/gateway/stream_consumer.py index 7fc9846c49f..f559d7ecd43 100644 --- a/gateway/stream_consumer.py +++ b/gateway/stream_consumer.py @@ -197,6 +197,30 @@ class GatewayStreamConsumer: # this response and route through edit-based for graceful degradation. self._draft_failures = 0 + def _metadata_for_send( + self, + *, + final: bool = False, + expect_edits: bool = False, + ) -> dict | None: + """Return per-send metadata for stream-created messages. + + Mattermost treats notify-worthy sends as user-visible final content + when deciding whether a broken thread root may fall back flat. Preview + and progress sends keep their original metadata and remain thread-strict. + + ``expect_edits`` preserves the upstream Telegram streaming contract: + preview messages that may be edited later must stay on the editable + legacy send path, while fresh/fallback final sends can still use richer + final-message delivery. + """ + meta = dict(self.metadata) if self.metadata else {} + if expect_edits: + meta["expect_edits"] = True + if final: + meta["notify"] = True + return meta or None + @property def already_sent(self) -> bool: """True if at least one message was sent or edited during the run.""" @@ -513,7 +537,11 @@ class GatewayStreamConsumer: chunks_delivered = False reply_to = self._message_id or self._initial_reply_to_id for chunk in chunks: - new_id = await self._send_new_chunk(chunk, reply_to) + new_id = await self._send_new_chunk( + chunk, + reply_to, + final=got_done, + ) if new_id is not None and new_id != reply_to: chunks_delivered = True self._accumulated = "" @@ -749,7 +777,13 @@ class GatewayStreamConsumer: # Strip trailing whitespace/newlines but preserve leading content return cleaned.rstrip() - async def _send_new_chunk(self, text: str, reply_to_id: Optional[str]) -> Optional[str]: + async def _send_new_chunk( + self, + text: str, + reply_to_id: Optional[str], + *, + final: bool = False, + ) -> Optional[str]: """Send a new message chunk, optionally threaded to a previous message. Returns the message_id so callers can thread subsequent chunks. @@ -758,15 +792,11 @@ class GatewayStreamConsumer: if not text.strip(): return reply_to_id try: - meta = dict(self.metadata) if self.metadata else {} - # This chunk becomes the next edit target — adapters that support - # rich final sends (Telegram) must keep it on the editable path. - meta["expect_edits"] = True result = await self.adapter.send( chat_id=self.chat_id, content=text, reply_to=reply_to_id, - metadata=meta, + metadata=self._metadata_for_send(final=final, expect_edits=True), ) if result.success and result.message_id: self._message_id = str(result.message_id) @@ -885,7 +915,7 @@ class GatewayStreamConsumer: result = await self.adapter.send( chat_id=self.chat_id, content=chunk, - metadata=self.metadata, + metadata=self._metadata_for_send(final=True), ) if result.success: break @@ -1242,7 +1272,7 @@ class GatewayStreamConsumer: result = await self.adapter.send( chat_id=self.chat_id, content=text, - metadata=self.metadata, + metadata=self._metadata_for_send(final=True), ) except Exception as e: logger.debug("Fresh-final send failed, falling back to edit: %s", e) @@ -1532,7 +1562,10 @@ class GatewayStreamConsumer: chat_id=self.chat_id, content=text, reply_to=self._initial_reply_to_id, - metadata={**(self.metadata or {}), "expect_edits": True}, + metadata=self._metadata_for_send( + final=finalize, + expect_edits=True, + ), ) if result.success: if result.message_id: diff --git a/scripts/release.py b/scripts/release.py index 3f2823f03d2..25247cff92a 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -90,6 +90,7 @@ AUTHOR_MAP = { "290859878+synapsesx@users.noreply.github.com": "synapsesx", "157689911+itsflownium@users.noreply.github.com": "itsflownium", "dirtyren@users.noreply.github.com": "dirtyren", + "evansrory@gmail.com": "zimigit2020", "237263164+ft-ioxcs@users.noreply.github.com": "ft-ioxcs", "tharushkadinujaya05@gmail.com": "0xneobyte", "138671361+Veritas-7@users.noreply.github.com": "Veritas-7", diff --git a/tests/agent/test_auxiliary_client.py b/tests/agent/test_auxiliary_client.py index 7770b2e8c88..b2960b703c7 100644 --- a/tests/agent/test_auxiliary_client.py +++ b/tests/agent/test_auxiliary_client.py @@ -1653,6 +1653,37 @@ class TestAuxiliaryFallbackLayering: exc.status_code = 402 return exc + def test_auto_provider_uses_task_then_main_chain_before_builtin_chain(self, monkeypatch): + """Auto aux call failures try per-task then top-level fallback before built-ins.""" + primary_client = MagicMock() + primary_client.chat.completions.create.side_effect = self._make_payment_err() + + main_chain_client = MagicMock() + main_chain_client.chat.completions.create.return_value = MagicMock(choices=[ + MagicMock(message=MagicMock(content="from main fallback chain")) + ]) + + with patch("agent.auxiliary_client._get_cached_client", + return_value=(primary_client, "qwen/qwen3.5-122b-a10b")), \ + patch("agent.auxiliary_client._resolve_task_provider_model", + return_value=("auto", None, None, None, None)), \ + patch("agent.auxiliary_client._try_configured_fallback_chain", + return_value=(None, None, "")) as mock_task_chain, \ + patch("agent.auxiliary_client._try_main_fallback_chain", + return_value=(main_chain_client, "inclusionai/ring-2.6-1t:free", "openrouter")) as mock_main_chain, \ + patch("agent.auxiliary_client._try_payment_fallback") as mock_builtin_chain: + result = call_llm( + task="title_generation", + messages=[{"role": "user", "content": "hello"}], + ) + + assert main_chain_client.chat.completions.create.called + mock_task_chain.assert_called_once_with( + "title_generation", "auto", reason="payment error") + mock_main_chain.assert_called_once_with( + "title_generation", "auto", reason="payment error") + mock_builtin_chain.assert_not_called() + def test_explicit_provider_uses_configured_chain_first(self, monkeypatch, caplog): """When a user has fallback_chain configured, it's tried BEFORE the main agent model.""" monkeypatch.setenv("OPENROUTER_API_KEY", "or-key") diff --git a/tests/agent/test_auxiliary_main_first.py b/tests/agent/test_auxiliary_main_first.py index 8913aad537f..f8a681ebfa9 100644 --- a/tests/agent/test_auxiliary_main_first.py +++ b/tests/agent/test_auxiliary_main_first.py @@ -118,6 +118,64 @@ class TestResolveAutoMainFirst: assert client is chain_client assert model == "google/gemini-3-flash-preview" + def test_main_unavailable_uses_task_fallback_chain_before_builtin_chain(self): + """Auto aux resolution honors auxiliary..fallback_chain before built-ins.""" + task_client = MagicMock() + with patch( + "agent.auxiliary_client._read_main_provider", return_value="nvidia", + ), patch( + "agent.auxiliary_client._read_main_model", return_value="qwen/qwen3.5-122b-a10b", + ), patch( + "agent.auxiliary_client.resolve_provider_client", + return_value=(None, None), # main provider has no client + ), patch( + "agent.auxiliary_client._try_configured_fallback_chain", + return_value=(task_client, "task-free-model", "fallback_chain[0](openrouter)"), + ) as mock_task_chain, patch( + "agent.auxiliary_client._try_main_fallback_chain", + ) as mock_main_chain, patch( + "agent.auxiliary_client._try_openrouter", + ) as mock_openrouter: + from agent.auxiliary_client import _resolve_auto + + client, model = _resolve_auto(task="title_generation") + + assert client is task_client + assert model == "task-free-model" + mock_task_chain.assert_called_once_with( + "title_generation", "nvidia", reason="main provider unavailable") + mock_main_chain.assert_not_called() + mock_openrouter.assert_not_called() + + def test_main_unavailable_uses_main_fallback_chain_before_builtin_chain(self): + """Auto aux resolution honors top-level fallback_providers before built-ins.""" + main_fallback_client = MagicMock() + with patch( + "agent.auxiliary_client._read_main_provider", return_value="nvidia", + ), patch( + "agent.auxiliary_client._read_main_model", return_value="qwen/qwen3.5-122b-a10b", + ), patch( + "agent.auxiliary_client.resolve_provider_client", + return_value=(None, None), # main provider has no client + ), patch( + "agent.auxiliary_client._try_configured_fallback_chain", + return_value=(None, None, ""), + ), patch( + "agent.auxiliary_client._try_main_fallback_chain", + return_value=(main_fallback_client, "inclusionai/ring-2.6-1t:free", "openrouter"), + ) as mock_main_chain, patch( + "agent.auxiliary_client._try_openrouter", + ) as mock_openrouter: + from agent.auxiliary_client import _resolve_auto + + client, model = _resolve_auto(task="title_generation") + + assert client is main_fallback_client + assert model == "inclusionai/ring-2.6-1t:free" + mock_main_chain.assert_called_once_with( + "title_generation", "nvidia", reason="main provider unavailable") + mock_openrouter.assert_not_called() + def test_no_main_config_uses_chain_directly(self): """No main provider configured → skip step 1, use chain (no regression).""" chain_client = MagicMock() diff --git a/tests/gateway/test_mattermost.py b/tests/gateway/test_mattermost.py index 9b174a5137a..1fedb30a019 100644 --- a/tests/gateway/test_mattermost.py +++ b/tests/gateway/test_mattermost.py @@ -6,7 +6,10 @@ import pytest from unittest.mock import MagicMock, patch, AsyncMock from gateway.config import Platform, PlatformConfig -from gateway.run import _resolve_progress_thread_id +from gateway.run import ( + _resolve_gateway_display_bool, + _resolve_progress_thread_id, +) class TestMattermostProgressThreadRouting: @@ -32,6 +35,97 @@ class TestMattermostProgressThreadRouting: ) is None +class TestMattermostDisplayHygiene: + def test_mattermost_requires_platform_opt_in_for_interim_assistant_messages(self): + """Global interim commentary must not make Mattermost leak scratch notes.""" + user_config = {"display": {"interim_assistant_messages": True}} + + assert _resolve_gateway_display_bool( + user_config, + "mattermost", + "interim_assistant_messages", + default=True, + platform=Platform.MATTERMOST, + require_platform_override_for={Platform.MATTERMOST}, + ) is False + + def test_mattermost_platform_opt_in_can_enable_interim_assistant_messages(self): + """Mattermost can still opt into commentary explicitly per platform.""" + user_config = { + "display": { + "interim_assistant_messages": False, + "platforms": { + "mattermost": {"interim_assistant_messages": True}, + }, + } + } + + assert _resolve_gateway_display_bool( + user_config, + "mattermost", + "interim_assistant_messages", + default=True, + platform=Platform.MATTERMOST, + require_platform_override_for={Platform.MATTERMOST}, + ) is True + + def test_mattermost_requires_platform_opt_in_for_thinking_progress(self): + """Global thinking_progress must not surface internal analysis in Mattermost.""" + user_config = {"display": {"thinking_progress": True}} + + assert _resolve_gateway_display_bool( + user_config, + "mattermost", + "thinking_progress", + default=False, + platform=Platform.MATTERMOST, + require_platform_override_for={Platform.MATTERMOST}, + ) is False + + def test_mattermost_requires_platform_opt_in_for_show_reasoning(self): + """Global show_reasoning must not prepend scratch reasoning in Mattermost.""" + user_config = {"display": {"show_reasoning": True}} + + assert _resolve_gateway_display_bool( + user_config, + "mattermost", + "show_reasoning", + default=False, + platform=Platform.MATTERMOST, + require_platform_override_for={Platform.MATTERMOST}, + ) is False + + def test_mattermost_platform_opt_in_can_enable_show_reasoning(self): + user_config = { + "display": { + "show_reasoning": False, + "platforms": {"mattermost": {"show_reasoning": True}}, + } + } + + assert _resolve_gateway_display_bool( + user_config, + "mattermost", + "show_reasoning", + default=False, + platform=Platform.MATTERMOST, + require_platform_override_for={Platform.MATTERMOST}, + ) is True + + def test_global_thinking_progress_still_applies_to_other_platforms(self): + """The Mattermost guard must not silently neuter Telegram/other chats.""" + user_config = {"display": {"thinking_progress": True}} + + assert _resolve_gateway_display_bool( + user_config, + "telegram", + "thinking_progress", + default=False, + platform=Platform.TELEGRAM, + require_platform_override_for={Platform.MATTERMOST}, + ) is True + + # --------------------------------------------------------------------------- # Platform & Config # --------------------------------------------------------------------------- @@ -347,6 +441,24 @@ class TestMattermostSend: payload = self.adapter._api_post.call_args_list[0][0][1] assert payload["root_id"] == "root_post" + @pytest.mark.asyncio + async def test_progress_send_with_invalid_thread_root_never_falls_back_flat(self): + """Tool/status/progress bubbles must stay quiet when the thread is broken.""" + self.adapter._reply_mode = "thread" + self.adapter._api_get = AsyncMock(return_value={"id": "bad_root", "root_id": ""}) + self.adapter._api_post = AsyncMock(return_value={}) + + result = await self.adapter.send( + "channel_1", + "⚙️ terminal...", + metadata={"thread_id": "bad_root"}, + ) + + assert result.success is False + assert self.adapter._api_post.call_count == 1 + payload = self.adapter._api_post.call_args_list[0][0][1] + assert payload["root_id"] == "bad_root" + @pytest.mark.asyncio async def test_send_api_failure(self): """When API returns error, send should return failure.""" diff --git a/tests/gateway/test_media_metadata_contract.py b/tests/gateway/test_media_metadata_contract.py new file mode 100644 index 00000000000..7f423e77342 --- /dev/null +++ b/tests/gateway/test_media_metadata_contract.py @@ -0,0 +1,80 @@ +"""Contract: media-send overrides must accept the ``metadata`` kwarg. + +``BasePlatformAdapter.send_multiple_images`` passes ``metadata=metadata`` +to ``send_image`` / ``send_image_file`` / ``send_animation`` on every send. +An override whose signature stops at ``reply_to`` raises ``TypeError: +send_image() got an unexpected keyword argument 'metadata'`` at runtime — +which is exactly how image delivery broke on WhatsApp and email. + +This mirrors ``test_discord_media_metadata.py`` but covers the two +adapters that previously slipped, plus a best-effort sweep over every +adapter that imports cleanly so the next slip is caught at test time. +""" + +from __future__ import annotations + +import importlib +import inspect + +import pytest + + +def _accepts_metadata(method) -> bool: + params = inspect.signature(method).parameters + if "metadata" in params: + return True + # A ``**kwargs`` catch-all also absorbs metadata (the convention used by + # WhatsApp's send_video / send_voice / send_document overrides). + return any(p.kind is inspect.Parameter.VAR_KEYWORD for p in params.values()) + + +# (module, class) for the two adapters this fix targeted. These must import +# in CI, so assert directly rather than skipping. +@pytest.mark.parametrize( + "module_name, class_name", + [ + ("gateway.platforms.whatsapp", "WhatsAppAdapter"), + ("gateway.platforms.email", "EmailAdapter"), + ], +) +def test_send_image_accepts_metadata(module_name, class_name): + cls = getattr(importlib.import_module(module_name), class_name) + assert _accepts_metadata(cls.send_image), ( + f"{class_name}.send_image must accept 'metadata' (or **kwargs) — " + f"send_multiple_images passes it on every send" + ) + + +# Best-effort sweep across all shipped adapters. Modules whose optional +# platform SDK isn't installed are skipped; an adapter that imports but +# whose override drops metadata is a hard failure. +_ALL_ADAPTERS = [ + ("gateway.platforms.bluebubbles", "BlueBubblesAdapter"), + ("gateway.platforms.dingtalk", "DingTalkAdapter"), + ("gateway.platforms.discord", "DiscordAdapter"), + ("gateway.platforms.email", "EmailAdapter"), + ("gateway.platforms.feishu", "FeishuAdapter"), + ("gateway.platforms.matrix", "MatrixAdapter"), + ("gateway.platforms.mattermost", "MattermostAdapter"), + ("gateway.platforms.signal", "SignalAdapter"), + ("gateway.platforms.slack", "SlackAdapter"), + ("gateway.platforms.telegram", "TelegramAdapter"), + ("gateway.platforms.wecom", "WeComAdapter"), + ("gateway.platforms.weixin", "WeixinAdapter"), + ("gateway.platforms.whatsapp", "WhatsAppAdapter"), + ("gateway.platforms.yuanbao", "YuanbaoAdapter"), +] + + +@pytest.mark.parametrize("module_name, class_name", _ALL_ADAPTERS) +def test_all_adapters_send_image_metadata_sweep(module_name, class_name): + try: + module = importlib.import_module(module_name) + except Exception as exc: # optional platform dep not installed + pytest.skip(f"{module_name} not importable: {exc}") + cls = getattr(module, class_name, None) + if cls is None or "send_image" not in cls.__dict__: + pytest.skip(f"{class_name} has no send_image override") + assert _accepts_metadata(cls.send_image), ( + f"{class_name}.send_image drops the 'metadata' kwarg" + ) diff --git a/tests/gateway/test_stream_consumer_thread_routing.py b/tests/gateway/test_stream_consumer_thread_routing.py index b2b7f22ffe5..3c84aef4fa8 100644 --- a/tests/gateway/test_stream_consumer_thread_routing.py +++ b/tests/gateway/test_stream_consumer_thread_routing.py @@ -106,6 +106,42 @@ class TestInitialReplyToId: assert call_kwargs["metadata"] == {**metadata, "expect_edits": True} assert metadata == {"thread_id": "omt_topic789"} + @pytest.mark.asyncio + async def test_final_first_send_marks_metadata_notify_true(self): + """Final streaming sends should use the existing notify=True marker.""" + adapter = _make_adapter() + consumer = GatewayStreamConsumer( + adapter, + "chat_123", + metadata={"thread_id": "root_post_123"}, + initial_reply_to_id="reply_post_456", + ) + + await consumer._send_or_edit("Final answer", finalize=True) + + call_kwargs = adapter.send.call_args[1] + metadata = call_kwargs["metadata"] + assert metadata["thread_id"] == "root_post_123" + assert metadata["notify"] is True + assert "delivery_kind" not in metadata + assert "allow_flat_fallback" not in metadata + + @pytest.mark.asyncio + async def test_nonfinal_first_send_does_not_mark_notify(self): + """Preview/interim streaming sends must not be notify-worthy.""" + adapter = _make_adapter() + consumer = GatewayStreamConsumer( + adapter, + "chat_123", + metadata={"thread_id": "root_post_123"}, + initial_reply_to_id="reply_post_456", + ) + + await consumer._send_or_edit("Preview", finalize=False) + + metadata = adapter.send.call_args[1]["metadata"] + assert metadata == {"thread_id": "root_post_123", "expect_edits": True} + class TestOverflowFirstMessage: """Verify thread routing is preserved when the first message overflows.""" diff --git a/tests/gateway/test_telegram_overflow_partial.py b/tests/gateway/test_telegram_overflow_partial.py index 76e4d16a617..38b10299dc3 100644 --- a/tests/gateway/test_telegram_overflow_partial.py +++ b/tests/gateway/test_telegram_overflow_partial.py @@ -134,7 +134,7 @@ async def test_stream_consumer_fallback_sends_tail_after_partial_overflow(): adapter.send.assert_awaited_once() assert adapter.send.await_args.kwargs["content"] == "world" - assert adapter.send.await_args.kwargs["metadata"] == {"thread_id": "77"} + assert adapter.send.await_args.kwargs["metadata"] == {"thread_id": "77", "notify": True} adapter.delete_message.assert_not_awaited() assert consumer.final_response_sent is True assert consumer.final_content_delivered is True diff --git a/tests/gateway/test_tts_media_routing.py b/tests/gateway/test_tts_media_routing.py index eaf9c592808..016be97ea27 100644 --- a/tests/gateway/test_tts_media_routing.py +++ b/tests/gateway/test_tts_media_routing.py @@ -76,7 +76,7 @@ async def test_base_adapter_routes_telegram_flac_media_tag_to_document_sender(tm adapter.send_document.assert_awaited_once_with( chat_id="chat-1", file_path=str(media_file), - metadata=None, + metadata={"notify": True}, ) adapter.send_voice.assert_not_awaited() @@ -95,7 +95,7 @@ async def test_base_adapter_routes_non_voice_telegram_ogg_media_tag_to_document_ adapter.send_document.assert_awaited_once_with( chat_id="chat-1", file_path=str(media_file), - metadata=None, + metadata={"notify": True}, ) adapter.send_voice.assert_not_awaited() @@ -116,7 +116,7 @@ async def test_base_adapter_routes_voice_tagged_telegram_ogg_media_tag_to_voice_ adapter.send_voice.assert_awaited_once_with( chat_id="chat-1", audio_path=str(media_file), - metadata=None, + metadata={"notify": True}, ) adapter.send_document.assert_not_awaited() diff --git a/tests/run_agent/test_async_httpx_del_neuter.py b/tests/run_agent/test_async_httpx_del_neuter.py index 946d73dbdf1..090e6998269 100644 --- a/tests/run_agent/test_async_httpx_del_neuter.py +++ b/tests/run_agent/test_async_httpx_del_neuter.py @@ -176,11 +176,16 @@ class TestClientCacheBoundedGrowth: """When the loop changes, the old entry should be replaced, not duplicated.""" from agent.auxiliary_client import ( _client_cache, + _client_cache_key, _client_cache_lock, _get_cached_client, ) - key = ("test_replace", True, "", "", "", (), False, "") + key = _client_cache_key( + "test_replace", + async_mode=True, + task="", + ) # Simulate a stale entry from a closed loop old_loop = asyncio.new_event_loop() diff --git a/website/docs/reference/environment-variables.md b/website/docs/reference/environment-variables.md index 2419846a10f..76ce863e661 100644 --- a/website/docs/reference/environment-variables.md +++ b/website/docs/reference/environment-variables.md @@ -687,7 +687,7 @@ For task-specific direct endpoints, Hermes uses the task's configured API key or ## Fallback Providers (config.yaml only) -The primary model fallback chain is configured exclusively through `config.yaml` — there are no environment variables for it. Add a top-level `fallback_providers` list with `provider` and `model` keys to enable automatic failover when your main model encounters errors. +The primary model fallback chain is configured exclusively through `config.yaml` — there are no environment variables for it. Add a top-level `fallback_providers` list with `provider` and `model` keys to enable automatic failover when your main model encounters errors. Auxiliary tasks whose provider is `auto` also consult this chain before Hermes' built-in auxiliary discovery chain. ```yaml fallback_providers: @@ -695,7 +695,7 @@ fallback_providers: model: anthropic/claude-sonnet-4 ``` -The older top-level `fallback_model` single-provider shape is still read for backward compatibility, but new configuration should use `fallback_providers`. +The older top-level `fallback_model` single-provider shape is still read for backward compatibility, but new configuration should use `fallback_providers`. For task-specific auxiliary policy, use `auxiliary..fallback_chain` in `config.yaml`; there is no environment variable equivalent. See [Fallback Providers](/user-guide/features/fallback-providers) for full details. diff --git a/website/docs/user-guide/configuring-models.md b/website/docs/user-guide/configuring-models.md index 3368d5201d8..8d749e15143 100644 --- a/website/docs/user-guide/configuring-models.md +++ b/website/docs/user-guide/configuring-models.md @@ -53,7 +53,7 @@ Click **Show auxiliary** to reveal the 11 task slots: ![Auxiliary panel expanded](/img/docs/dashboard-models/auxiliary-expanded.png) -Every auxiliary task defaults to `auto` — meaning Hermes uses your main model for that job too. Override a specific task when you want a cheaper or faster model for a side-job. +Every auxiliary task defaults to `auto` — meaning Hermes tries your main model for that job too. If that route is unavailable or hits a capacity-style failure, `auto` follows any task-specific `auxiliary..fallback_chain`, then the main `fallback_providers` / `fallback_model` chain, then Hermes' built-in auxiliary discovery chain. Override a specific task when you want a cheaper or faster model for a side-job. ### Common override patterns @@ -129,7 +129,21 @@ auxiliary: # ... other fields unchanged ``` -`provider: auto` with `model: ''` tells Hermes to use the main model for that task. +`provider: auto` with `model: ''` tells Hermes to use the main model for that task, while still honoring fallback policy if the main route cannot serve the auxiliary call. + +Optional task-specific fallback chains live under the same auxiliary task: + +```yaml +auxiliary: + title_generation: + provider: auto + model: '' + fallback_chain: + - provider: openrouter + model: inclusionai/ring-2.6-1t:free +``` + +When `fallback_chain` is absent, `auto` uses the top-level `fallback_providers` chain before the built-in auxiliary discovery chain. ## When does it take effect? diff --git a/website/docs/user-guide/features/fallback-providers.md b/website/docs/user-guide/features/fallback-providers.md index 7eaab0ea8af..dbe431fc1ea 100644 --- a/website/docs/user-guide/features/fallback-providers.md +++ b/website/docs/user-guide/features/fallback-providers.md @@ -168,7 +168,7 @@ fallback_providers: | Messaging gateway (Telegram, Discord, etc.) | ✔ | | Subagent delegation | ✔ (subagents inherit the parent fallback chain) | | Cron jobs | ✔ (cron agents inherit configured fallback providers) | -| Auxiliary tasks (vision, compression) | ✘ (use their own provider chain — see below) | +| Auxiliary tasks on `provider: auto` | ✔ (try per-task fallback, then the main fallback chain before built-in aux discovery) | :::tip There are no environment variables for the primary fallback chain — configure it exclusively through `config.yaml` or `hermes fallback`. This is intentional: fallback configuration is a deliberate choice, not something a stale shell export should override. @@ -195,23 +195,30 @@ Hermes uses separate lightweight models for side tasks. Each task has its own pr ### Auto-Detection Chain -When a task's provider is set to `"auto"` (the default), Hermes tries providers in order until one works: +When a task's provider is set to `"auto"` (the default), Hermes first tries the main provider + main model for that auxiliary task. If that route is unavailable or later fails with a capacity-style error, Hermes now honors user-configured fallback policy before using the built-in discovery chain: -**For text tasks (compression, web extract, etc.):** +```text +Main provider + main model → auxiliary..fallback_chain → +fallback_providers / fallback_model → built-in auxiliary discovery chain +``` + +The task-specific chain is most precise and wins when present. The top-level `fallback_providers` chain is the same policy the main agent uses, so free-only or same-provider fallback rules apply to auxiliary tasks on `auto` as well. + +**Built-in text discovery chain (compression, web extract, title generation, etc.):** ```text OpenRouter → Nous Portal → Custom endpoint → Codex OAuth → API-key providers (z.ai, Kimi, MiniMax, Xiaomi MiMo, Hugging Face, Anthropic) → give up ``` -**For vision tasks:** +**Built-in vision discovery chain:** ```text Main provider (if vision-capable) → OpenRouter → Nous Portal → Codex OAuth → Anthropic → Custom endpoint → give up ``` -If the resolved provider fails at call time, Hermes also has an internal retry: if the provider is not OpenRouter and no explicit `base_url` is set, it tries OpenRouter as a last-resort fallback. +Those built-in chains are a convenience fallback for users who have not declared a task-specific or main fallback policy. ### Configuring Auxiliary Providers @@ -232,6 +239,9 @@ auxiliary: compression: provider: "auto" model: "" + fallback_chain: # optional, task-specific fallback policy + - provider: openrouter + model: inclusionai/ring-2.6-1t:free skills_hub: provider: "auto" @@ -242,7 +252,9 @@ auxiliary: model: "" ``` -Every task above follows the same **provider / model / base_url** pattern. Context compression is configured under `auxiliary.compression`: +Every task above follows the same **provider / model / base_url** pattern. Each task can also declare its own `fallback_chain`; if omitted, `provider: auto` uses the top-level `fallback_providers` chain before Hermes' built-in auxiliary discovery chain. + +Context compression is configured under `auxiliary.compression`: ```yaml auxiliary: