From 010c9925e30620c83ccbbcf2bf5c314ce8071e10 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 19 Aug 2026 15:38:01 -0700 Subject: [PATCH 01/24] fix(bot-mode): roster age, pulse, unread, and sort now see canonical Bot Chat activity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The canonical Bot Chat is hidden from session lists by design, so profiles.list's last_session never advances when you message a bot there. PR #88690 moved the roster PREVIEW to preferred_session but left every activity signal on last_session — a bot you just messaged showed '6d ago', never pulsed, never badged, and sorted below stale bots. New botActivitySession(bot) helper returns the fresher of preferred_session (the pinned Bot Chat, resolved precisely by the backend) and last_session (newest visible conversation). All four activity sites key off it now: - row age label (relativeTime) - active-now pulse dot + activeBots strip - unread watermark + activity toast preview - roster recency sort (activityOf) Older gateways without the preferred_session resolver degrade to last_session exactly as before. Backend untouched. Tests: extracted the real helper into the vm harnesses (no stub drift), 5 new behavior tests; sabotage-verified they fail against the old code. --- .../desktop/src/plugins/hermes-bots/plugin.js | 57 ++++++++++++---- .../tests/active-now-strip.test.mjs | 65 ++++++++++++++++++- .../tests/activity-toasts.test.mjs | 24 ++++++- .../tests/profile-prewarm.test.mjs | 10 ++- 4 files changed, 137 insertions(+), 19 deletions(-) diff --git a/apps/desktop/src/plugins/hermes-bots/plugin.js b/apps/desktop/src/plugins/hermes-bots/plugin.js index 712042ef53..ce66a17929 100644 --- a/apps/desktop/src/plugins/hermes-bots/plugin.js +++ b/apps/desktop/src/plugins/hermes-bots/plugin.js @@ -120,13 +120,17 @@ function setActivityToasts(enabled) { } /** Detect new inbound activity from a fresh roster: last_active moved past - * the watermark for a bot whose chat isn't on screen -> unread + toast. */ + * the watermark for a bot whose chat isn't on screen -> unread + toast. + * Watermarks follow botActivitySession (canonical Bot Chat included) — + * last_session alone never sees the hidden Bot Chat, so DMs delivered + * there would neither badge nor toast. */ function trackInboundActivity(roster) { const seeding = !watermarksSeeded watermarksSeeded = true for (const bot of roster) { - const ts = bot.last_session?.last_active || 0 + const activity = botActivitySession(bot) + const ts = activity?.last_active || 0 const prev = rosterWatermarks.get(bot.name) || 0 rosterWatermarks.set(bot.name, Math.max(prev, ts)) @@ -153,7 +157,7 @@ function trackInboundActivity(roster) { if ($activityToasts.get()) { const meta = $botMeta.get()[bot.name] const label = displayName(bot, meta) - const preview = (bot.last_session?.preview || '').trim() + const preview = (activity?.preview || '').trim() const inbound = /^Message from/i.test(preview) host.notify({ @@ -4762,6 +4766,28 @@ function generatedSessionTitle(session, preview) { * seconds is treated as "active now" (pulsing dot in its row). */ const ACTIVE_WINDOW_S = 90 +/** The session whose activity best represents this bot — the FRESHER of the + * pinned canonical Bot Chat (preferred_session) and the profile's newest + * visible conversation (last_session). + * + * Canonical Bot Chats are hidden from the session list by design, so + * last_session alone never sees them: a bot you talk to all day through its + * Bot Chat reads "6d ago" because its newest VISIBLE session is a week old. + * #88690 moved the preview text to preferred_session but left every activity + * signal (age label, pulse dot, unread watermark, recency sort) on + * last_session. All of them key off this helper now. Older gateways without + * the preferred_session resolver degrade to last_session unchanged. */ +function botActivitySession(bot) { + const preferred = bot?.preferred_session + const last = bot?.last_session + + if (!preferred || !last) { + return preferred || last || null + } + + return (preferred.last_active || 0) >= (last.last_active || 0) ? preferred : last +} + /** Bots that are working right now: the profile the gateway is running a * turn for (busy), plus any bot whose last message landed inside the * liveness window. Pure — output follows the input roster's order, so @@ -4769,7 +4795,7 @@ const ACTIVE_WINDOW_S = 90 function activeBots(roster, activeProfile, gatewayState, now = Date.now()) { return (roster || []).filter(bot => { const busyTurn = !bot.remoteSource && bot.name === activeProfile && gatewayState === 'busy' - const last = bot.last_session?.last_active || 0 + const last = botActivitySession(bot)?.last_active || 0 const inWindow = Boolean(last && now / 1000 - last < ACTIVE_WINDOW_S) return busyTurn || inWindow @@ -4797,7 +4823,17 @@ function BotRow({ bot, onDelete, onEdit, onGroup }) { // Keep user photos/pets. Drop the 160px SVG backfill so the math face can move. const photo = Boolean(image && !isBackfilledFacePng(image)) const gatewayState = useValue(host.state.gateway) - const activeNow = Boolean(last?.last_active && Date.now() / 1000 - last.last_active < ACTIVE_WINDOW_S) + // Preview identity must match click identity (#88200): when the backend + // resolved the pinned canonical chat, preview THAT session — not the + // profile's most recent (but unrelated) activity. Activity signals + // (age label, pulse dot) follow the same rule via botActivitySession: + // the canonical Bot Chat is hidden from last_session, so keying age off + // last_session alone shows "6d ago" on a bot you just messaged. + const previewSession = bot.preferred_session || last + const activitySession = botActivitySession(bot) + const activeNow = Boolean( + activitySession?.last_active && Date.now() / 1000 - activitySession.last_active < ACTIVE_WINDOW_S + ) // Work pose only when this bot is actually doing something: the active // profile while the gateway is busy, or a bot that wrote within the // liveness window. Not every bot whenever the gateway is busy. @@ -4808,11 +4844,6 @@ function BotRow({ bot, onDelete, onEdit, onGroup }) { const unread = !bot.remoteSource && Boolean(unreadByName[bot.name]) // WHO sent the last message (bot-to-bot DM vs human) — the full stored // history lives in the Sessions workspace (context menu), not inline. - // Preview identity must match click identity (#88200): when the backend - // resolved the pinned canonical chat, preview THAT session — not the - // profile's most recent (but unrelated) activity. Liveness checks above - // keep last_session semantics: any recent activity means the bot is alive. - const previewSession = bot.preferred_session || last const { fromBot } = previewKind(previewSession?.preview) // DM previews read like DMs: strip the delivery prefix, keep the message. const displayPreview = stripPreviewMarkdown( @@ -4983,10 +5014,10 @@ function BotRow({ bot, onDelete, onEdit, onGroup }) { title: 'Active in the last 90s' }) : null, - last + activitySession ? jsx('span', { className: 'shrink-0 text-[0.6875rem] text-(--ui-text-quaternary)', - children: relativeTime(last.last_active * 1000) + children: relativeTime(activitySession.last_active * 1000) }) : null ] @@ -9541,7 +9572,7 @@ function BotsPane() { // No special slot for the primary bot — it competes on recency too. const activityOf = bot => { const created = botRosterMeta(bot, allMeta)?.created || bot.ui_meta?.['hermes-bots']?.created || 0 - const lastMsg = (bot.last_session?.last_active || 0) * 1000 + const lastMsg = (botActivitySession(bot)?.last_active || 0) * 1000 return Math.max(created, lastMsg) } diff --git a/apps/desktop/src/plugins/hermes-bots/tests/active-now-strip.test.mjs b/apps/desktop/src/plugins/hermes-bots/tests/active-now-strip.test.mjs index c56c587bac..10d57932f6 100644 --- a/apps/desktop/src/plugins/hermes-bots/tests/active-now-strip.test.mjs +++ b/apps/desktop/src/plugins/hermes-bots/tests/active-now-strip.test.mjs @@ -7,7 +7,7 @@ const source = readFileSync(new URL('../plugin.js', import.meta.url), 'utf8') // The activeBots helper must stay a self-contained slice between the // liveness-window constant and the BotRow section so tests can extract it. -function loadActiveBots() { +function loadActiveBotsSlice() { const start = source.indexOf('const ACTIVE_WINDOW_S') const end = source.indexOf('// ── bot row ─') @@ -15,11 +15,19 @@ function loadActiveBots() { const context = {} vm.runInNewContext( - `${source.slice(start, end)}\nglobalThis.__activeBots = activeBots;`, + `${source.slice(start, end)}\nglobalThis.__activeBots = activeBots;\nglobalThis.__botActivitySession = botActivitySession;`, context ) - return context.__activeBots + return context +} + +function loadActiveBots() { + return loadActiveBotsSlice().__activeBots +} + +function loadBotActivitySession() { + return loadActiveBotsSlice().__botActivitySession } // Fixed clock so "inside the window" vs "stale" is deterministic. @@ -74,6 +82,57 @@ test('roster without profiles never throws', () => { assert.equal(activeBots([], 'default', 'open', NOW).length, 0) }) +// ── botActivitySession: canonical Bot Chat activity counts (hermes-agent "6d ago" bug) ── + +test('botActivitySession picks the fresher preferred_session over a stale last_session', () => { + const botActivitySession = loadBotActivitySession() + const bot = { + // Canonical Bot Chat (hidden from session lists): messaged seconds ago. + preferred_session: { id: 'bot-chat', last_active: NOW / 1000 - 5, preview: 'fresh DM' }, + // Newest VISIBLE session: 6 days old — what last_session alone reports. + last_session: { id: 'old-scratch', last_active: NOW / 1000 - 6 * 86400, preview: 'ancient' } + } + assert.equal(botActivitySession(bot).id, 'bot-chat') +}) + +test('botActivitySession keeps last_session when it is the fresher one', () => { + const botActivitySession = loadBotActivitySession() + const bot = { + preferred_session: { id: 'bot-chat', last_active: NOW / 1000 - 3600 }, + last_session: { id: 'scratch', last_active: NOW / 1000 - 10 } + } + assert.equal(botActivitySession(bot).id, 'scratch') +}) + +test('botActivitySession degrades to whichever side exists (older gateways / no pin)', () => { + const botActivitySession = loadBotActivitySession() + assert.equal(botActivitySession({ last_session: { id: 'only', last_active: 1 } }).id, 'only') + assert.equal(botActivitySession({ preferred_session: { id: 'pin', last_active: 1 } }).id, 'pin') + assert.equal(botActivitySession({}), null) + assert.equal(botActivitySession(null), null) +}) + +test('activeBots counts Bot Chat activity that last_session cannot see', () => { + const activeBots = loadActiveBots() + const bots = [ + { + name: 'default', + preferred_session: { last_active: NOW / 1000 - 5 }, + last_session: { last_active: NOW / 1000 - 6 * 86400 } + } + ] + const names = activeBots(bots, 'other', 'open', NOW).map(bot => bot.name) + assert.ok(names.includes('default'), 'fresh canonical-chat activity must light the pulse dot') +}) + +test('row age label and recency sort key off botActivitySession, not last_session', () => { + // The "6d ago" regression: the timestamp/sort sites must not read + // bot.last_session directly anymore. + assert.match(source, /relativeTime\(activitySession\.last_active \* 1000\)/) + assert.match(source, /const lastMsg = \(botActivitySession\(bot\)\?\.last_active \|\| 0\) \* 1000/) + assert.doesNotMatch(source, /relativeTime\(last\.last_active \* 1000\)/) +}) + test('ActiveNowStrip renders above the roster, is a live region, and is click-accessible', () => { // Strip is placed between the pane header and the search field. const headerEnd = source.indexOf("children: 'Bots'") diff --git a/apps/desktop/src/plugins/hermes-bots/tests/activity-toasts.test.mjs b/apps/desktop/src/plugins/hermes-bots/tests/activity-toasts.test.mjs index cd89ff4942..3ba2d2968e 100644 --- a/apps/desktop/src/plugins/hermes-bots/tests/activity-toasts.test.mjs +++ b/apps/desktop/src/plugins/hermes-bots/tests/activity-toasts.test.mjs @@ -13,6 +13,11 @@ const source = readFileSync(new URL('../plugin.js', import.meta.url), 'utf8') function loadTracker(toastsEnabled) { const start = source.indexOf('const rosterWatermarks = new Map()') const end = source.indexOf('/** Last good cron list', start) + // The tracker keys watermarks off the REAL botActivitySession helper + // (defined later in plugin.js) — extract it so the harness can't drift. + const helperStart = source.indexOf('function botActivitySession(') + const helperEnd = source.indexOf('/** Bots that are working', helperStart) + assert.ok(helperStart >= 0 && helperEnd > helperStart, 'botActivitySession must remain extractable') const notifications = [] const context = { pluginCtx: null, @@ -30,7 +35,8 @@ function loadTracker(toastsEnabled) { displayName: bot => bot.name } const section = source - .slice(start, end) + .slice(helperStart, helperEnd) + .concat('\n', source.slice(start, end)) .concat('\nglobalThis.__t = { trackInboundActivity, $activityToasts, setActivityToasts };\n') vm.runInNewContext(section, context, { filename: 't.js' }) if (toastsEnabled) { @@ -68,3 +74,19 @@ test('pref defaults OFF and persists via ctx.storage under activity-toasts', () ) assert.match(source, /storage\?\.get\?\.\('activity-toasts'\)/) }) + +test('activity in the hidden canonical Bot Chat still badges (the "6d ago" class)', () => { + // The canonical Bot Chat is hidden from session lists, so last_session + // never advances when a DM lands there — only preferred_session does. + const t = loadTracker(false) + const at = ts => [ + { + name: 'researcher', + last_session: { last_active: 100, preview: 'ancient scratch chat' }, + preferred_session: { last_active: ts, preview: 'Message from writer: hi' } + } + ] + t.trackInboundActivity(at(150)) // seeding poll + t.trackInboundActivity(at(250)) // Bot Chat got a DM; last_session unchanged + assert.equal(t.$botUnread.get().researcher, true, 'hidden Bot Chat activity must set unread') +}) diff --git a/apps/desktop/src/plugins/hermes-bots/tests/profile-prewarm.test.mjs b/apps/desktop/src/plugins/hermes-bots/tests/profile-prewarm.test.mjs index 57ade31cd8..971ba37980 100644 --- a/apps/desktop/src/plugins/hermes-bots/tests/profile-prewarm.test.mjs +++ b/apps/desktop/src/plugins/hermes-bots/tests/profile-prewarm.test.mjs @@ -15,6 +15,12 @@ function sourceBetween(start, end) { return source.slice(from, to) } +// BotRow's activity resolver — extract the REAL helper so the harness can't +// drift from production behavior. +function activitySessionSource() { + return sourceBetween('function botActivitySession(', '/** Bots that are working') +} + function renderBotRow(input = 'alpha') { const bot = typeof input === 'string' ? { name: input } : input const name = bot.name @@ -93,7 +99,7 @@ function renderBotRow(input = 'alpha') { useValue: store => store.get() } - vm.runInNewContext(`${prepareSource}\n${botRowSource}\nglobalThis.BotRow = BotRow`, context) + vm.runInNewContext(`${activitySessionSource()}\n${prepareSource}\n${botRowSource}\nglobalThis.BotRow = BotRow`, context) const tree = context.BotRow({ bot, onEdit: context.onEdit }) const row = tree.type === 'button' ? tree : tree.props.children[0].props.children @@ -215,7 +221,7 @@ test('behavior: remote default does not open this-device chat when the source di useValue: store => store.get() } - vm.runInNewContext(`${prepareSource}\n${botRowSource}\nglobalThis.BotRow = BotRow`, context) + vm.runInNewContext(`${activitySessionSource()}\n${prepareSource}\n${botRowSource}\nglobalThis.BotRow = BotRow`, context) const tree = context.BotRow({ bot, onEdit: context.onEdit }) const row = tree.type === 'button' ? tree : tree.props.children[0].props.children From a664429192c2a6cdb31ba5654ba80025e42a8859 Mon Sep 17 00:00:00 2001 From: liuhao1024 Date: Wed, 19 Aug 2026 06:01:39 +0800 Subject: [PATCH 02/24] fix(agent): cap the ultra reasoning level at the wire vocabulary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hermes' internal effort vocabulary extends the wire set with ultra (documented by /reasoning as none..xhigh|max|ultra). OpenAI-compatible wires — OpenRouter chief among them — accept exactly max|xhigh|high|medium|low|minimal|none and reject the extension with HTTP 400, so an ultra configured while the default model was Anthropic worked (the Anthropic adapter maps its own levels) but leaked untranslated the moment a per-job override pinned a non-Anthropic model, failing every call for that job. The wire-compat chokepoint for this transport previously mapped ultra to max only for gpt-5.6; generalize the cap to every model. --- agent/transports/chat_completions.py | 18 +++++-- .../test_reasoning_effort_wire_translation.py | 54 +++++++++++++++++++ 2 files changed, 67 insertions(+), 5 deletions(-) create mode 100644 tests/agent/test_reasoning_effort_wire_translation.py diff --git a/agent/transports/chat_completions.py b/agent/transports/chat_completions.py index 939ef6f446..c2b11e7be9 100644 --- a/agent/transports/chat_completions.py +++ b/agent/transports/chat_completions.py @@ -84,13 +84,21 @@ def _add_prompt_cache_key( def _reasoning_config_for_model(model: str, reasoning_config: dict | None) -> dict | None: - """Return the model's wire-compatible reasoning config.""" + """Return the model's wire-compatible reasoning config. + + Hermes' internal effort set extends the wire vocabulary with ``ultra`` + (the /reasoning command documents none..xhigh|max|ultra). OpenAI- + compatible wires — OpenRouter chief among them — accept exactly + max|xhigh|high|medium|low|minimal|none and reject the extension with + HTTP 400, so an ``ultra`` configured for an Anthropic default leaks + untranslated when a per-job/per-turn override pins a non-Anthropic + model on this transport and the whole call fails (#89503). Map the + extension to its wire cap for every model on this path; the Anthropic + adapter keeps its own richer mapping. + """ if not isinstance(reasoning_config, dict): return reasoning_config - if ( - "gpt-5.6" in (model or "").lower() - and str(reasoning_config.get("effort") or "").strip().lower() == "ultra" - ): + if str(reasoning_config.get("effort") or "").strip().lower() == "ultra": normalized = dict(reasoning_config) normalized["effort"] = "max" return normalized diff --git a/tests/agent/test_reasoning_effort_wire_translation.py b/tests/agent/test_reasoning_effort_wire_translation.py new file mode 100644 index 0000000000..d126b57412 --- /dev/null +++ b/tests/agent/test_reasoning_effort_wire_translation.py @@ -0,0 +1,54 @@ +"""Wire translation for Hermes' extended reasoning-effort vocabulary (#89503). + +Hermes' internal effort set extends the wire vocabulary with ``ultra`` (the +/reasoning command documents none..xhigh|max|ultra). OpenAI-compatible wires — +OpenRouter chief among them — accept exactly max|xhigh|high|medium|low|minimal| +none and reject the extension with HTTP 400: + + reasoning.effort: Invalid option: expected one of "max"|"xhigh"|"high"| + "medium"|"low"|"minimal"|"none" + +An ``ultra`` configured while the default model was Anthropic worked (the +Anthropic adapter maps its own levels), but the moment a per-job override +pinned an OpenRouter model the extension leaked through the OpenAI-compatible +transport untranslated and every call failed. ``_reasoning_config_for_model`` +is the wire-compat chokepoint for this transport: it must cap the extension +for every model, not just the one vendor prefix that happened to be fixed +first. +""" + +from agent.transports.chat_completions import _reasoning_config_for_model + + +class TestUltraEffortWireTranslation: + def test_ultra_maps_to_max_for_any_model(self): + """The extension level caps at the wire vocabulary for every model — + including the OpenRouter vendor prefixes a per-job override pins + (#89503's nvidia/ case) and models with no vendor prefix at all.""" + for model in ( + "nvidia/nemotron-3.5-lightning:free", + "deepseek/deepseek-v4", + "qwen/qwen3.5-coder", + "some-internal-model", + ): + out = _reasoning_config_for_model( + model, {"enabled": True, "effort": "ultra"} + ) + assert out == {"enabled": True, "effort": "max"}, model + + def test_gpt_56_ultra_still_maps(self): + """The original pre-existing mapping (gpt-5.6 + ultra → max) is + preserved by the generalized one.""" + out = _reasoning_config_for_model( + "gpt-5.6", {"enabled": True, "effort": "ultra"} + ) + assert out == {"enabled": True, "effort": "max"} + + def test_wire_native_levels_pass_through_untouched(self): + for level in ("none", "minimal", "low", "medium", "high", "xhigh", "max"): + cfg = {"enabled": True, "effort": level} + assert _reasoning_config_for_model("any/model", cfg) == cfg + + def test_non_dict_and_none_pass_through(self): + assert _reasoning_config_for_model("m", None) is None + assert _reasoning_config_for_model("m", "not-a-dict") == "not-a-dict" From 9ec5750aca33bf6e977cfde468868b0f0d66e339 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 19 Aug 2026 15:33:41 -0700 Subject: [PATCH 03/24] fix: widen reasoning-effort wire translation to sibling sites (#89503 class) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The chat_completions chokepoint fix (ultra->max for every model, cherry-picked from #89509) has siblings with the same bug shape: - codex.py: ultra->max was gated on gpt-5.6 only; now baseline for all Responses-API models (backend-specific branches still override). - Kimi top-level reasoning_effort: K3 accepts low/high/max only — 'medium' and upper-ladder levels were dropped to the medium default (400s on K3, ladder inversion on K2). Full ladder mapped per family, mirroring the kimi-coding plugin's K3 map. - TokenHub: 'minimal' fell through to the 'high' default (asked least, got most); full ladder now mapped onto low/medium/high. - auxiliary_client Responses path: ultra->max alongside the existing minimal->low clamp. - custom provider plugin: ultra capped at max instead of forwarded verbatim to GLM/vLLM/SGLang backends that reject it. - copilot plugin: ad-hoc downgrade rules replaced with the shared clamp_reasoning_effort_to_supported ladder walk so ultra/max resolve to the strongest supported level instead of medium (#74295). Sabotage-verified: new sibling-site tests fail 6/10 without the fixes. --- agent/auxiliary_client.py | 7 +- agent/transports/chat_completions.py | 46 ++++++- agent/transports/codex.py | 12 +- plugins/model-providers/copilot/__init__.py | 38 +++--- plugins/model-providers/custom/__init__.py | 8 +- .../test_reasoning_effort_sibling_sites.py | 121 ++++++++++++++++++ 6 files changed, 205 insertions(+), 27 deletions(-) create mode 100644 tests/agent/transports/test_reasoning_effort_sibling_sites.py diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py index ca6bea2d26..21a0a7533b 100644 --- a/agent/auxiliary_client.py +++ b/agent/auxiliary_client.py @@ -1552,9 +1552,14 @@ class _CodexCompletionsAdapter: # with a 400. effort = reasoning_cfg.get("effort") or "medium" # Codex backend rejects "minimal"; clamp to "low" to - # match the main-agent Codex transport behavior. + # match the main-agent Codex transport behavior. "ultra" + # is Hermes-internal ladder vocabulary with no wire + # equivalent anywhere on this API; cap it at "max" + # (same class as #89503). if effort == "minimal": effort = "low" + elif effort == "ultra": + effort = "max" resp_kwargs["reasoning"] = { "effort": effort, "summary": "auto", diff --git a/agent/transports/chat_completions.py b/agent/transports/chat_completions.py index c2b11e7be9..b1f9a8b0a4 100644 --- a/agent/transports/chat_completions.py +++ b/agent/transports/chat_completions.py @@ -567,11 +567,36 @@ class ChatCompletionsTransport(ProviderTransport): and reasoning_config.get("enabled") is False ) if not _kimi_thinking_off: - _kimi_effort = "medium" + # K3 accepts low/high/max only (default high) — "medium" and + # Hermes' upper-ladder levels 400 or silently degrade. Mirror + # the kimi-coding plugin's _K3_EFFORT_MAP; older Kimi models + # keep the low/medium/high vocabulary with the stronger + # Hermes levels capped at high instead of being dropped + # (dropping them inverted the ladder: ultra sent the + # "medium" default, weaker than an explicit high). + _e = "" if reasoning_config and isinstance(reasoning_config, dict): _e = (reasoning_config.get("effort") or "").strip().lower() - if _e in {"low", "medium", "high"}: - _kimi_effort = _e + if "k3" in (model or "").lower(): + _kimi_effort = { + "minimal": "low", + "low": "low", + "medium": "high", + "high": "high", + "xhigh": "max", + "max": "max", + "ultra": "max", + }.get(_e, "high") + else: + _kimi_effort = { + "minimal": "low", + "low": "low", + "medium": "medium", + "high": "high", + "xhigh": "high", + "max": "high", + "ultra": "high", + }.get(_e, "medium") api_kwargs["reasoning_effort"] = _kimi_effort # Tencent TokenHub: top-level reasoning_effort (unless thinking disabled) @@ -582,11 +607,22 @@ class ChatCompletionsTransport(ProviderTransport): and reasoning_config.get("enabled") is False ) if not _tokenhub_thinking_off: + # TokenHub accepts low/medium/high. Map Hermes' full ladder + # onto that set instead of dropping unknown levels to the + # "high" default — dropping inverted the ladder for + # "minimal" (asked for the least, got the most). _tokenhub_effort = "high" if reasoning_config and isinstance(reasoning_config, dict): _e = (reasoning_config.get("effort") or "").strip().lower() - if _e in {"low", "medium", "high"}: - _tokenhub_effort = _e + _tokenhub_effort = { + "minimal": "low", + "low": "low", + "medium": "medium", + "high": "high", + "xhigh": "high", + "max": "high", + "ultra": "high", + }.get(_e, "high") api_kwargs["reasoning_effort"] = _tokenhub_effort # LM Studio: top-level reasoning_effort. Only emit when the model diff --git a/agent/transports/codex.py b/agent/transports/codex.py index 9a866225b0..247cc18dfe 100644 --- a/agent/transports/codex.py +++ b/agent/transports/codex.py @@ -432,10 +432,14 @@ class ResponsesApiTransport(ProviderTransport): elif reasoning_config.get("effort"): reasoning_effort = reasoning_config["effort"] - _effort_clamp = {"minimal": "low"} - if "gpt-5.6" in (model or "").lower(): - # Ultra is the Codex product tier; the Responses API wire value is max. - _effort_clamp["ultra"] = "max" + # "ultra" is Hermes-internal ladder vocabulary (the Codex product + # tier); no Responses-API backend accepts it verbatim, so the + # baseline maps it to its wire cap "max" for EVERY model — the old + # gpt-5.6-only guard leaked "ultra" untranslated to sibling models + # and the request 400'd (same class as #89503 on the + # chat-completions transport). Backend-specific branches below + # override the baseline where the ceiling is narrower. + _effort_clamp = {"minimal": "low", "ultra": "max"} if params.get("is_xai_responses", False): from agent.model_metadata import is_grok_46_family diff --git a/plugins/model-providers/copilot/__init__.py b/plugins/model-providers/copilot/__init__.py index 781a96c5cc..f8b5912781 100644 --- a/plugins/model-providers/copilot/__init__.py +++ b/plugins/model-providers/copilot/__init__.py @@ -37,23 +37,29 @@ class CopilotProfile(ProviderProfile): effort = reasoning_config.get("effort", "medium") # Honor the requested level when the live Copilot catalog # lists it as supported: gpt-5.5/gpt-5.4 DO support - # ``xhigh``. Only downgrade levels the catalog does NOT - # list (e.g. ``xhigh``/``max`` on models capped lower, or - # ``minimal`` where unsupported), choosing the nearest - # weaker supported level rather than forwarding verbatim. - # - # (Previously this unconditionally mapped xhigh->high, a - # stale guard that silently capped models which do support - # the higher level.) + # ``xhigh``. Otherwise clamp to the nearest WEAKER + # supported level via the shared ladder helper — the old + # ad-hoc rules dropped everything unrecognized to + # ``medium``, which inverted the ladder: ``ultra`` (the + # strongest ask) resolved weaker than an explicit + # ``high`` (#74295). if effort not in supported_efforts: - if effort == "xhigh" and "high" in supported_efforts: - effort = "high" - elif effort == "minimal" and "low" in supported_efforts: - effort = "low" - elif "medium" in supported_efforts: - effort = "medium" - else: - effort = supported_efforts[0] + from hermes_cli.models import ( + clamp_reasoning_effort_to_supported, + ) + + effort = clamp_reasoning_effort_to_supported( + effort, list(supported_efforts) + ) + if effort not in supported_efforts: + # Unrecognized/bespoke level the ladder can't + # place — fall back to medium, then to the + # catalog's first entry. + effort = ( + "medium" + if "medium" in supported_efforts + else supported_efforts[0] + ) if effort in supported_efforts: extra_body["reasoning"] = {"effort": effort} elif supported_efforts: diff --git a/plugins/model-providers/custom/__init__.py b/plugins/model-providers/custom/__init__.py index de744d5091..6353b59f59 100644 --- a/plugins/model-providers/custom/__init__.py +++ b/plugins/model-providers/custom/__init__.py @@ -64,7 +64,13 @@ class CustomProfile(ProviderProfile): top_level["reasoning_effort"] = "none" extra_body["think"] = False elif _effort: - top_level["reasoning_effort"] = _effort + # "ultra" is Hermes-internal ladder vocabulary — no known + # OpenAI-compatible backend accepts it verbatim (GLM/ARK, + # vLLM and SGLang all top out at "max"); cap it at the wire + # ceiling instead of forwarding a guaranteed 400 (#89503). + top_level["reasoning_effort"] = ( + "max" if _effort == "ultra" else _effort + ) return extra_body, top_level diff --git a/tests/agent/transports/test_reasoning_effort_sibling_sites.py b/tests/agent/transports/test_reasoning_effort_sibling_sites.py new file mode 100644 index 0000000000..6e754c29ca --- /dev/null +++ b/tests/agent/transports/test_reasoning_effort_sibling_sites.py @@ -0,0 +1,121 @@ +"""Sibling-site coverage for the reasoning-effort wire-vocabulary class (#89503). + +The chat-completions chokepoint fix (ultra → max for every model) is covered +by tests/agent/test_reasoning_effort_wire_translation.py. These tests pin the +sibling sites fixed in the same sweep: + +- Kimi/Moonshot top-level ``reasoning_effort``: K3 accepts low/high/max only + (docs: default high); K2-era models accept low/medium/high. Previously the + transport forwarded only {low,medium,high} and silently dropped everything + else to "medium", so K3 400'd on "medium" requests and ultra resolved + WEAKER than an explicit high (ladder inversion). +- Tencent TokenHub: accepts low/medium/high; upper-ladder levels previously + dropped to the "high" default (accidentally right) but "minimal" also + dropped to high — asked for the least, got the most. +- Codex/Responses transport: ultra → max for EVERY model, not just gpt-5.6. +""" + +from agent.transports import get_transport +import agent.transports.chat_completions # noqa: F401 +import agent.transports.codex # noqa: F401 + + +def _cc(): + return get_transport("chat_completions") + + +def _kimi_kwargs(model, reasoning_config): + return _cc().build_kwargs( + model=model, + messages=[{"role": "user", "content": "hi"}], + is_kimi=True, + reasoning_config=reasoning_config, + ) + + +class TestKimiEffortVocabulary: + def test_k3_maps_full_hermes_ladder(self): + expected = { + "minimal": "low", + "low": "low", + "medium": "high", + "high": "high", + "xhigh": "max", + "max": "max", + "ultra": "max", + } + for hermes_level, wire_level in expected.items(): + kw = _kimi_kwargs( + "kimi-k3", {"enabled": True, "effort": hermes_level} + ) + assert kw["reasoning_effort"] == wire_level, hermes_level + + def test_k3_default_is_high(self): + kw = _kimi_kwargs("kimi-k3", None) + assert kw["reasoning_effort"] == "high" + + def test_k2_upper_ladder_caps_at_high_not_medium(self): + """Pre-fix, ultra/max/xhigh on K2-era models silently dropped to the + 'medium' default — the strongest ask resolved weaker than an explicit + high (ladder inversion, same class as #74295).""" + for level in ("xhigh", "max", "ultra"): + kw = _kimi_kwargs( + "moonshotai/kimi-k2.6", {"enabled": True, "effort": level} + ) + assert kw["reasoning_effort"] == "high", level + + def test_k2_native_levels_pass_through(self): + for level in ("low", "medium", "high"): + kw = _kimi_kwargs( + "moonshotai/kimi-k2.6", {"enabled": True, "effort": level} + ) + assert kw["reasoning_effort"] == level + + def test_k2_minimal_maps_to_low(self): + kw = _kimi_kwargs( + "moonshotai/kimi-k2.6", {"enabled": True, "effort": "minimal"} + ) + assert kw["reasoning_effort"] == "low" + + def test_disabled_omits_effort(self): + kw = _kimi_kwargs("kimi-k3", {"enabled": False}) + assert "reasoning_effort" not in kw + + +class TestTokenHubEffortVocabulary: + def _kwargs(self, reasoning_config): + return _cc().build_kwargs( + model="hunyuan-t2", + messages=[{"role": "user", "content": "hi"}], + is_tokenhub=True, + reasoning_config=reasoning_config, + ) + + def test_upper_ladder_caps_at_high(self): + for level in ("xhigh", "max", "ultra"): + kw = self._kwargs({"enabled": True, "effort": level}) + assert kw["reasoning_effort"] == "high", level + + def test_minimal_maps_to_low_not_high(self): + """Pre-fix, 'minimal' fell through to the 'high' default — asked for + the least reasoning, got the most.""" + kw = self._kwargs({"enabled": True, "effort": "minimal"}) + assert kw["reasoning_effort"] == "low" + + def test_native_levels_pass_through(self): + for level in ("low", "medium", "high"): + kw = self._kwargs({"enabled": True, "effort": level}) + assert kw["reasoning_effort"] == level + + +class TestCodexUltraForEveryModel: + def test_ultra_maps_to_max_for_non_gpt56_models(self): + transport = get_transport("codex_responses") + for model in ("gpt-5.6-codex", "o5-pro", "some-responses-model"): + kw = transport.build_kwargs( + model=model, + messages=[{"role": "user", "content": "hi"}], + tools=[], + reasoning_config={"enabled": True, "effort": "ultra"}, + ) + assert kw["reasoning"]["effort"] == "max", model From 2dea073a1c50da45948c9b45c96fbac3e8078616 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 19 Aug 2026 15:24:12 -0700 Subject: [PATCH 04/24] fix(tools): dispatch image/video FAL strictly on the stored hermes tools selection Add read_selection()/selection_exists()/selection_error() to tool_backend_helpers: one provider string per category ('nous' = managed Nous Tool Gateway, vendor name = direct with the user's own credentials, no key ever written = legacy credential autodetect). Legacy configs are interpreted at read time only (use_gateway: true => nous); nothing is migrated on disk, and the DEFAULT_CONFIG-seeded stt.provider: local is treated as never-configured. _resolve_managed_fal_gateway / _resolve_managed_fal_video_gateway now switch on that string: 'nous' routes managed only (unentitled => error naming the selection), a stored vendor routes direct only (missing FAL_KEY => error naming FAL_KEY and the selection, no silent managed reroute), and FAL_KEY presence no longer selects the route. Krea's model-driven managed interception now requires no stored provider (or the managed selection) instead of merely provider != krea, and the image/video registries map the 'nous' selection to the FAL plugin. --- agent/image_gen_registry.py | 12 +++ agent/video_gen_registry.py | 12 +++ plugins/image_gen/krea/__init__.py | 25 ++++-- plugins/video_gen/fal/__init__.py | 76 ++++++++++++++++-- tools/image_generation_tool.py | 80 ++++++++++++++++--- tools/tool_backend_helpers.py | 120 +++++++++++++++++++++++++++++ 6 files changed, 299 insertions(+), 26 deletions(-) diff --git a/agent/image_gen_registry.py b/agent/image_gen_registry.py index 82393e88db..6239bbe891 100644 --- a/agent/image_gen_registry.py +++ b/agent/image_gen_registry.py @@ -139,6 +139,18 @@ def get_active_provider() -> Optional[ImageGenProvider]: except Exception as exc: logger.debug("Could not read image_gen.provider from config: %s", exc) + # The managed "Nous Subscription" selection is serviced by the FAL + # plugin through the managed fal-queue gateway (the legacy FAL pipeline + # routes managed when the stored selection is "nous"). + if configured: + try: + from tools.tool_backend_helpers import NOUS_MANAGED_PROVIDER + + if configured.lower() == NOUS_MANAGED_PROVIDER: + configured = "fal" + except Exception: # pragma: no cover — helpers are in-repo + pass + with _lock: snapshot = dict(_providers) snapshot.update(_scoped_providers.get(hermes_home_key(), {})) diff --git a/agent/video_gen_registry.py b/agent/video_gen_registry.py index ff46b087c0..5b4c9cea54 100644 --- a/agent/video_gen_registry.py +++ b/agent/video_gen_registry.py @@ -132,6 +132,18 @@ def get_active_provider() -> Optional[VideoGenProvider]: except Exception as exc: logger.debug("Could not read video_gen.provider from config: %s", exc) + # The managed "Nous Subscription" selection is serviced by the FAL + # plugin through the managed fal-queue gateway (the plugin's resolver + # routes managed when the stored selection is "nous"). + if configured: + try: + from tools.tool_backend_helpers import NOUS_MANAGED_PROVIDER + + if configured.lower() == NOUS_MANAGED_PROVIDER: + configured = "fal" + except Exception: # pragma: no cover — helpers are in-repo + pass + with _lock: snapshot = dict(_providers) snapshot.update(_scoped_providers.get(hermes_home_key(), {})) diff --git a/plugins/image_gen/krea/__init__.py b/plugins/image_gen/krea/__init__.py index 07e4ecaf34..0027a8f9b1 100644 --- a/plugins/image_gen/krea/__init__.py +++ b/plugins/image_gen/krea/__init__.py @@ -178,20 +178,31 @@ def _resolve_model(explicit: Optional[str] = None) -> Tuple[str, Dict[str, Any]] def _resolve_managed_krea_gateway(): """Return managed Krea gateway config when the user is on the managed path. - Mirrors ``_resolve_managed_fal_gateway`` in ``tools/image_generation_tool.py``: - the Nous-hosted Krea gateway wins when it is resolvable AND either no direct - ``KREA_API_KEY`` is configured or the user explicitly opted into the gateway - for ``image_gen``. Returns ``None`` (direct/BYO path) otherwise, and never - raises — plugin discovery and availability scans must stay robust. + Strict selection model: the managed Krea gateway is used when the stored + ``image_gen`` selection is ``nous`` (or legacy ``use_gateway: true``), or + on a never-configured install when no direct ``KREA_API_KEY`` exists. + An explicit vendor selection (``krea``, ``fal``, ...) pins the direct + path. Returns ``None`` (direct/BYO path) otherwise, and never raises — + plugin discovery and availability scans must stay robust. """ try: from tools.managed_tool_gateway import resolve_managed_tool_gateway - from tools.tool_backend_helpers import prefers_gateway + from tools.tool_backend_helpers import ( + NOUS_MANAGED_PROVIDER, + read_selection, + ) except Exception as exc: # noqa: BLE001 logger.debug("Managed Krea gateway resolution unavailable: %s", exc) return None - if get_secret("KREA_API_KEY") and not prefers_gateway("image_gen"): + try: + selected = read_selection("image_gen") + except Exception: # noqa: BLE001 + selected = None + if selected is not None and selected != NOUS_MANAGED_PROVIDER: + # Explicit vendor selection: direct credentials only. + return None + if selected is None and get_secret("KREA_API_KEY"): return None try: diff --git a/plugins/video_gen/fal/__init__.py b/plugins/video_gen/fal/__init__.py index 3d5714d781..abc8285cb8 100644 --- a/plugins/video_gen/fal/__init__.py +++ b/plugins/video_gen/fal/__init__.py @@ -530,14 +530,44 @@ _managed_fal_video_client_lock = threading.Lock() def _resolve_managed_fal_video_gateway(): - """Return managed fal-queue gateway config when the user prefers the gateway - or direct FAL credentials are absent.""" - from tools.tool_backend_helpers import fal_key_is_configured, prefers_gateway + """Resolve the FAL video route from the stored selection. - if fal_key_is_configured() and not prefers_gateway("video_gen"): - return None + Plain switch on the stored ``video_gen`` provider string — mirrors the + image FAL resolver: ``"nous"`` (or legacy ``use_gateway: true``) → + managed only (unentitled ⇒ selection-naming error); any other stored + provider → direct only (missing FAL_KEY ⇒ selection-naming error); + never-configured category → legacy credential autodetect. + """ from tools.managed_tool_gateway import resolve_managed_tool_gateway + from tools.tool_backend_helpers import ( + NOUS_MANAGED_PROVIDER, + fal_key_is_configured, + read_selection, + selection_error, + ) + selected = read_selection("video_gen") + if selected == NOUS_MANAGED_PROVIDER: + gateway = resolve_managed_tool_gateway("fal-queue") + if gateway is None: + raise ValueError(selection_error( + "video_gen", + NOUS_MANAGED_PROVIDER, + "the Nous Tool Gateway is not available (not entitled or " + "unreachable)", + )) + return gateway + if selected is not None: + if not fal_key_is_configured(): + raise ValueError(selection_error( + "video_gen", + selected, + "FAL_KEY is not set", + )) + return None + # Never-configured category: legacy credential autodetect (do NOT persist). + if fal_key_is_configured(): + return None return resolve_managed_tool_gateway("fal-queue") @@ -598,12 +628,28 @@ def _submit_fal_video_request(endpoint: str, arguments: Dict[str, Any]): def _check_fal_video_available() -> bool: - """True if the FAL.ai video backend is reachable (direct key or managed gateway).""" - from tools.tool_backend_helpers import fal_key_is_configured + """True if the FAL video backend selected via `hermes tools` (or, on a + never-configured install, any FAL backend) is reachable. + Never raises — a stored-but-broken selection reports False here; the + honest selection-naming error surfaces at call time from + ``_resolve_managed_fal_video_gateway``. + """ + from tools.managed_tool_gateway import resolve_managed_tool_gateway + from tools.tool_backend_helpers import ( + NOUS_MANAGED_PROVIDER, + fal_key_is_configured, + read_selection, + ) + + selected = read_selection("video_gen") + if selected == NOUS_MANAGED_PROVIDER: + return resolve_managed_tool_gateway("fal-queue") is not None + if selected is not None: + return fal_key_is_configured() if fal_key_is_configured(): return True - return _resolve_managed_fal_video_gateway() is not None + return resolve_managed_tool_gateway("fal-queue") is not None # --------------------------------------------------------------------------- @@ -766,6 +812,20 @@ class FALVideoGenProvider(VideoGenProvider): **kwargs: Any, ) -> Dict[str, Any]: if not _check_fal_video_available(): + from tools.tool_backend_helpers import read_selection + + if read_selection("video_gen") is not None: + # A stored selection that cannot run gets the honest + # selection-naming error from the strict resolver. + try: + _resolve_managed_fal_video_gateway() + except ValueError as exc: + return error_response( + error=str(exc), + error_type="auth_required", + provider="fal", + prompt=prompt, + ) return error_response( error=( "No FAL backend available. Either set FAL_KEY " diff --git a/tools/image_generation_tool.py b/tools/image_generation_tool.py index f794ca6268..5f1d0e17f5 100644 --- a/tools/image_generation_tool.py +++ b/tools/image_generation_tool.py @@ -66,10 +66,12 @@ from tools.fal_common import ( ) from tools.managed_tool_gateway import resolve_managed_tool_gateway from tools.tool_backend_helpers import ( + NOUS_MANAGED_PROVIDER, fal_key_is_configured, managed_nous_tools_enabled, nous_tool_gateway_unavailable_message, - prefers_gateway, + read_selection, + selection_error, ) logger = logging.getLogger(__name__) @@ -732,9 +734,43 @@ _managed_fal_client_lock = threading.Lock() # Managed FAL gateway (Nous Subscription) # --------------------------------------------------------------------------- def _resolve_managed_fal_gateway(): - """Return managed fal-queue gateway config when the user prefers the gateway - or direct FAL credentials are absent.""" - if fal_key_is_configured() and not prefers_gateway("image_gen"): + """Resolve the FAL route from the stored `hermes tools` selection. + + Dispatch is a plain switch on the stored ``image_gen`` provider string: + - ``"nous"`` (or legacy ``use_gateway: true``) → managed fal-queue + gateway ONLY; unentitled/unreachable is a selection-naming error + (never a silent fall back to FAL_KEY). + - any other stored provider (``"fal"``, ...) → direct FAL ONLY; a + missing FAL_KEY is an error naming FAL_KEY and the selection (never a + silent managed reroute). + - no selection ever written → legacy credential autodetect: direct when + FAL_KEY is set, else the managed gateway when resolvable, else None. + + Returns the managed gateway config, or ``None`` for the direct route. + Raises ``ValueError`` with the honest error contract when the stored + selection cannot run. + """ + selected = read_selection("image_gen") + if selected == NOUS_MANAGED_PROVIDER: + gateway = resolve_managed_tool_gateway("fal-queue") + if gateway is None: + raise ValueError(selection_error( + "image_gen", + NOUS_MANAGED_PROVIDER, + "the Nous Tool Gateway is not available (not entitled or " + "unreachable)", + )) + return gateway + if selected is not None: + if not fal_key_is_configured(): + raise ValueError(selection_error( + "image_gen", + selected, + "FAL_KEY is not set", + )) + return None + # Never-configured category: legacy credential autodetect (do NOT persist). + if fal_key_is_configured(): return None return resolve_managed_tool_gateway("fal-queue") @@ -1228,6 +1264,9 @@ def image_generate_tool( if not prompt or not isinstance(prompt, str) or len(prompt.strip()) == 0: raise ValueError("Prompt is required and must be a non-empty string") + # Strict selection check: a stored-but-broken selection raises the + # honest selection-naming error from _resolve_managed_fal_gateway(); + # only the never-configured path can report "no backend at all". if not (fal_key_is_configured() or _resolve_managed_fal_gateway()): raise ValueError(_build_no_backend_setup_message()) @@ -1372,8 +1411,19 @@ def image_generate_tool( def check_fal_api_key() -> bool: - """True if the FAL.ai API key (direct or managed gateway) is available.""" - return bool(fal_key_is_configured() or _resolve_managed_fal_gateway()) + """True if the FAL backend selected via `hermes tools` (or, on a + never-configured install, any FAL backend) is available. + + A stored-but-broken selection reports False here (registry gating); + the honest selection-naming error surfaces at call time from + ``_resolve_managed_fal_gateway``. + """ + selected = read_selection("image_gen") + if selected == NOUS_MANAGED_PROVIDER: + return bool(resolve_managed_tool_gateway("fal-queue")) + if selected is not None: + return fal_key_is_configured() + return bool(fal_key_is_configured() or resolve_managed_tool_gateway("fal-queue")) def _build_no_backend_setup_message() -> str: @@ -1431,7 +1481,7 @@ def check_image_generation_requirements() -> bool: pass configured = _read_configured_image_provider() - if not configured or configured == "fal": + if not configured or configured in ("fal", NOUS_MANAGED_PROVIDER): return False # Probe only the explicitly selected plugin. Merely possessing a cloud @@ -1629,8 +1679,11 @@ def _dispatch_to_plugin_provider( ignore it via their ``**kwargs`` (the ABC contract). """ configured = _read_configured_image_provider() - if not configured or configured == "fal": - return None # unset/explicit FAL keeps the legacy FAL path + if not configured or configured in ("fal", NOUS_MANAGED_PROVIDER): + # Unset/explicit FAL keeps the legacy FAL path; "nous" (managed + # Nous Subscription selection) also runs the legacy pipeline, which + # routes through the managed fal-queue gateway. + return None # Also read configured model so we can pass it to the plugin configured_model = _read_configured_image_model() @@ -1785,8 +1838,13 @@ def _maybe_route_managed_krea( Direct/BYO users (no managed gateway) fall through untouched. """ - # ``provider == "krea"`` is already handled by the standard plugin dispatch. - if _read_configured_image_provider() == "krea": + # Strict selection rule: an explicitly stored ``image_gen.provider`` + # (other than the managed "nous" selection, which IS a managed-mode + # opt-in) disables the model-driven managed interception — the user's + # picker choice dispatches normally. Interception is permitted only on + # never-configured installs or under the managed selection. + configured_provider = _read_configured_image_provider() + if configured_provider is not None and configured_provider != NOUS_MANAGED_PROVIDER: return None normalized = _normalize_krea_model(_read_configured_image_model()) diff --git a/tools/tool_backend_helpers.py b/tools/tool_backend_helpers.py index b00814c0ad..f3c23c9c85 100644 --- a/tools/tool_backend_helpers.py +++ b/tools/tool_backend_helpers.py @@ -290,6 +290,126 @@ def prefers_gateway(config_section: str) -> bool: return False +# The provider value the managed "Nous Subscription" picker rows write for +# every category (image_gen.provider: nous, web.backend: nous, +# browser.cloud_provider: nous, ...). Runtime dispatch is a plain switch on +# the stored string: "nous" → managed gateway client; any vendor name → that +# vendor direct with the user's own credentials; no key ever written → +# legacy credential autodetect. +NOUS_MANAGED_PROVIDER = "nous" + +# Per-capability keys that also count as "this category has been configured". +_EXTRA_SELECTION_KEYS = { + "web": ("search_backend", "extract_backend"), +} + +# Which key(s) carry the category's provider selection. ``browser.backend`` +# is deliberately excluded for the browser section — it is the DRIVER choice +# ("browser-use" CLI vs built-in tools), not the cloud provider selection. +_SELECTION_NAME_KEYS = { + "browser": ("cloud_provider",), + "web": ("backend",), +} +_DEFAULT_NAME_KEYS = ("provider", "backend", "cloud_provider") + + +def read_selection(section: str) -> str | None: + """Return the stored `hermes tools` provider string for a config section. + + THE single runtime read of the persisted selection. Returns: + - ``"nous"`` — the managed Nous Tool Gateway row was selected, + - a vendor name (``"fal"``, ``"openai"``, ``"firecrawl"``, ...) — that + vendor, direct, with the user's own credentials, + - ``None`` — the category has NEVER been configured; the legacy + credential autodetect ladder is permitted (and must not be persisted). + + Reads the RAW config.yaml (not the DEFAULT_CONFIG-merged view) so key + presence means "a selection was actually written", not "the schema has a + default". Never raises; an unreadable config reports ``None``. + + Legacy interpretation (read-time only — nothing is migrated on disk): + older picker versions wrote ``
.use_gateway`` beside the name + key. ``use_gateway: true`` was only ever written by the managed "Nous + Subscription" row, so it maps to ``"nous"`` regardless of the name key; + ``use_gateway: false`` beside a name key maps to that name. + """ + try: + from hermes_cli.config import read_raw_config_readonly + + cfg = read_raw_config_readonly() or {} + raw = cfg.get(section) if isinstance(cfg, dict) else None + except Exception: + raw = None + if not isinstance(raw, dict): + return None + + def _str_or_none(key: str) -> str | None: + value = raw.get(key) + if value is None: + return None + text = str(value).strip().lower() + return text or None + + name = None + for key in _SELECTION_NAME_KEYS.get(section, _DEFAULT_NAME_KEYS): + name = _str_or_none(key) + if name: + break + + # Legacy shim: a truthy use_gateway means the managed row was picked + # (it was the only writer of use_gateway: true). + if "use_gateway" in raw and is_truthy_value(raw.get("use_gateway"), default=False): + return NOUS_MANAGED_PROVIDER + + # Migration shim: DEFAULT_CONFIG historically seeded ``stt.provider: + # local`` on every install, so that exact value with no picker-written + # use_gateway key is ambiguous. Treat it as never-configured — the + # autodetect ladder prefers local first anyway, and hard-pinning would + # error every seeded install that lacks faster-whisper. + if section == "stt" and name == "local" and "use_gateway" not in raw: + return None + + if name: + return name + + # use_gateway: false with no name key is not a usable selection shape; + # per-capability web keys still count as configured elsewhere via + # selection_exists(). Fall to autodetect. + return None + + +def selection_exists(section: str) -> bool: + """True when ANY selection signal has ever been written for the section. + + Wider than ``read_selection() is not None``: per-capability web keys + (``search_backend``/``extract_backend``) mark the category as configured + even when the shared backend name is empty. + """ + if read_selection(section) is not None: + return True + extra = _EXTRA_SELECTION_KEYS.get(section, ()) + if not extra: + return False + try: + from hermes_cli.config import read_raw_config_readonly + + cfg = read_raw_config_readonly() or {} + raw = cfg.get(section) if isinstance(cfg, dict) else None + except Exception: + return False + if not isinstance(raw, dict): + return False + return any(str(raw.get(key) or "").strip() for key in extra) + + +def selection_error(section: str, selection_name: str, failure: str) -> str: + """The uniform honest-error contract for a selected-but-broken provider.""" + return ( + f"{section} is configured to use {selection_name} (set via hermes " + f"tools), but {failure}. Run 'hermes tools' to change it." + ) + + def fal_key_is_configured() -> bool: """Return True when FAL_KEY is set to a non-whitespace value. From d7119ea2a641a39a52bdbef139e290dbb90caf5b Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 19 Aug 2026 15:24:26 -0700 Subject: [PATCH 05/24] fix(web): honor the stored web backend selection; no silent backend swaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _get_backend returns the stored web.backend verbatim (mapping the managed 'nous' selection to the firecrawl provider) — unknown names surface the honest selection-naming error at dispatch instead of silently rerouting through the credential ladder, which now runs only on never-configured installs. _get_capability_backend no longer discards an explicit search/extract backend when its availability probe fails. The firecrawl client resolves strictly: 'nous' => managed gateway only (unavailable => selection-naming error), stored vendor => direct only (no FIRECRAWL key => error, never a silent managed fallback billed to Nous). --- plugins/web/firecrawl/provider.py | 108 +++++++++++++++++++++++------- tools/web_tools.py | 103 ++++++++++++++++++++++++---- 2 files changed, 175 insertions(+), 36 deletions(-) diff --git a/plugins/web/firecrawl/provider.py b/plugins/web/firecrawl/provider.py index ae02b43a6e..e18e78ea48 100644 --- a/plugins/web/firecrawl/provider.py +++ b/plugins/web/firecrawl/provider.py @@ -48,7 +48,7 @@ from __future__ import annotations import asyncio import logging import os -from typing import Any, Dict, List, Optional, TYPE_CHECKING +from typing import Any, Dict, List, NoReturn, Optional, TYPE_CHECKING from agent.web_search_provider import WebSearchProvider from tools.url_safety import is_safe_url @@ -168,11 +168,22 @@ def _has_direct_firecrawl_config() -> bool: def check_firecrawl_api_key() -> bool: - """Return True when Firecrawl backend (direct or gateway) is usable. + """Return True when the Firecrawl backend selected via `hermes tools` + (or, on a never-configured install, either route) is usable. Re-exported by :mod:`tools.web_tools` for backward compatibility with existing tests and the ``hermes tools`` setup flow. """ + from tools.tool_backend_helpers import ( + NOUS_MANAGED_PROVIDER, + read_selection, + ) + + selected = read_selection("web") + if selected == NOUS_MANAGED_PROVIDER: + return _is_tool_gateway_ready() + if selected is not None: + return _has_direct_firecrawl_config() return _has_direct_firecrawl_config() or _is_tool_gateway_ready() @@ -188,7 +199,7 @@ def _firecrawl_backend_help_suffix() -> str: ) -def _raise_web_backend_configuration_error() -> None: +def _raise_web_backend_configuration_error() -> "NoReturn": """Raise a clear error for unsupported web backend configuration.""" import tools.web_tools as _wt @@ -212,46 +223,95 @@ def _raise_web_backend_configuration_error() -> None: def _get_firecrawl_client() -> Any: """Get or create the cached Firecrawl client. - When ``web.use_gateway`` is set in config, the managed Tool Gateway is - preferred even if direct Firecrawl credentials are present. Otherwise - direct Firecrawl takes precedence when explicitly configured. + Strict selection semantics (switch on the stored ``web`` selection): + - ``"nous"`` (or legacy ``use_gateway: true``) → managed Tool Gateway + ONLY; unavailable is a selection-naming error (a present + FIRECRAWL_API_KEY does not reroute). + - any other stored web backend → direct Firecrawl ONLY; missing config + is a selection-naming error — never a silent managed fallback billed + to Nous. + - never-configured web section → legacy behavior: direct config when + present, else the managed gateway. - Raises ValueError when neither path is usable. + Raises ValueError when the resolved path is unusable. The cached client is stored on :mod:`tools.web_tools` (as ``_firecrawl_client`` and ``_firecrawl_client_config``) rather than on this plugin module so that unit tests that reset the cache via ``tools.web_tools._firecrawl_client = None`` keep working. Helper - functions (``prefers_gateway``, ``resolve_managed_tool_gateway``, - ``_read_nous_access_token``, ``Firecrawl``) are also looked up via - :mod:`tools.web_tools` for the same reason — see - :func:`_is_tool_gateway_ready`. + functions (``resolve_managed_tool_gateway``, ``_read_nous_access_token``, + ``Firecrawl``) are also looked up via :mod:`tools.web_tools` for the same + reason — see :func:`_is_tool_gateway_ready`. """ import tools.web_tools as _wt + from tools.tool_backend_helpers import ( + NOUS_MANAGED_PROVIDER, + read_selection, + selection_error, + selection_exists, + ) + + selected = read_selection("web") direct_config = _get_direct_firecrawl_config() - if direct_config is not None and not _wt.prefers_gateway("web"): - kwargs, client_config = direct_config - else: + + def _managed_kwargs(): managed_gateway = _wt.resolve_managed_tool_gateway( "firecrawl", token_reader=_wt._read_nous_access_token ) if managed_gateway is None: + return None + kwargs = { + "api_key": managed_gateway.nous_user_token, + "api_url": managed_gateway.gateway_origin, + } + return kwargs, ( + "tool-gateway", + kwargs["api_url"], + managed_gateway.nous_user_token, + ) + + if selected == NOUS_MANAGED_PROVIDER: + managed = _managed_kwargs() + if managed is None: + logger.error( + "Firecrawl client initialization failed: the Nous " + "Subscription web selection is stored but the tool gateway " + "is unavailable." + ) + raise ValueError(selection_error( + "web", + NOUS_MANAGED_PROVIDER, + "the Nous Tool Gateway is not available (not entitled or " + "unreachable)", + )) + kwargs, client_config = managed + elif selected is not None or selection_exists("web"): + # Stored vendor selection (or per-capability web keys routing to + # firecrawl): direct Firecrawl only. + if direct_config is None: + logger.error( + "Firecrawl client initialization failed: direct Firecrawl " + "selected but FIRECRAWL_API_KEY/FIRECRAWL_API_URL is not set." + ) + raise ValueError(selection_error( + "web", + selected or "firecrawl", + "neither FIRECRAWL_API_KEY nor FIRECRAWL_API_URL is set", + )) + kwargs, client_config = direct_config + elif direct_config is not None: + kwargs, client_config = direct_config + else: + # Never-configured web section: legacy managed fallback. + managed = _managed_kwargs() + if managed is None: logger.error( "Firecrawl client initialization failed: " "missing direct config and tool-gateway auth." ) _raise_web_backend_configuration_error() - - kwargs = { - "api_key": managed_gateway.nous_user_token, - "api_url": managed_gateway.gateway_origin, - } - client_config = ( - "tool-gateway", - kwargs["api_url"], - managed_gateway.nous_user_token, - ) + kwargs, client_config = managed cached = getattr(_wt, "_firecrawl_client", None) cached_config = getattr(_wt, "_firecrawl_client_config", None) diff --git a/tools/web_tools.py b/tools/web_tools.py index e8c62142af..0634cedc4f 100644 --- a/tools/web_tools.py +++ b/tools/web_tools.py @@ -223,16 +223,36 @@ def _list_registered_web_providers(): def _get_backend() -> str: """Determine which web backend to use (shared fallback). - Reads ``web.backend`` from config.yaml (set by ``hermes tools``). - Falls back to whichever API key is present for users who configured - keys manually without running setup. + Reads ``web.backend`` from config.yaml (set by ``hermes tools``). A + stored backend name is returned as-is — no availability probe, no + fallback — so the vendor path can raise its own honest error when the + selection is broken. The credential/entitlement autodetect ladder runs + ONLY when no web selection has ever been stored. """ configured = (_load_web_config().get("backend") or "").lower().strip() - if configured in _LEGACY_WEB_BACKENDS or _registered_web_provider(configured) is not None: + if configured: + # Strict: the stored selection is final, known name or not — an + # unknown/typoed name surfaces as the vendor path's honest error + # rather than silently rerouting through the credential ladder. + # The managed "Nous Subscription" selection ("nous") is serviced by + # the firecrawl provider, whose client resolver routes it through + # the managed Tool Gateway. + from tools.tool_backend_helpers import NOUS_MANAGED_PROVIDER + + if configured == NOUS_MANAGED_PROVIDER: + return "firecrawl" return configured - # Fallback for manual / legacy config — pick the highest-priority - # available backend. Explicit user credentials (TAVILY_API_KEY etc.) + from tools.tool_backend_helpers import selection_exists + + if selection_exists("web"): + # A web selection exists (e.g. use_gateway key or per-capability + # backends) but the shared backend name is empty — keep the + # firecrawl default rather than credential-laddering. + return "firecrawl" + + # Never-configured install — pick the highest-priority available + # backend. Explicit user credentials (TAVILY_API_KEY etc.) # beat the managed-tool-gateway probe so a deliberate setup is not # pre-empted by a Nous OAuth token whose subscription tier may not # actually grant web-search access (the gateway then fails at runtime @@ -298,12 +318,16 @@ def _get_extract_backend() -> str: def _get_capability_backend(capability: str) -> str: """Shared helper for per-capability backend selection. - Reads ``web.{capability}_backend`` from config; if set and available, - uses it. Otherwise falls through to the shared ``_get_backend()``. + Reads ``web.{capability}_backend`` from config; a stored value is + returned unconditionally (strict selection — no availability probe). + A selected-but-broken backend surfaces the vendor path's honest error + instead of being silently replaced by whatever the credential ladder + finds. Falls through to the shared ``_get_backend()`` only when no + per-capability override is stored. """ cfg = _load_web_config() specific = (cfg.get(f"{capability}_backend") or "").lower().strip() - if specific and _is_backend_available(specific): + if specific: return specific return _get_backend() @@ -692,9 +716,35 @@ def web_search_tool(query: str, limit: int = 5) -> str: backend = _get_search_backend() provider = _wsp_get_provider(backend) if backend else None if provider is None or not provider.supports_search(): - # Fall back to availability-walked active provider when the - # configured backend isn't a registered search provider (typo, - # uninstalled plugin, or capability mismatch). + from tools.tool_backend_helpers import ( + selection_error, + selection_exists, + ) + + if provider is None and backend and selection_exists("web"): + disabled_key = _disabled_web_plugin_for(capability="search") + if disabled_key: + _vendor = disabled_key.split("/", 1)[-1] + error_text = ( + f"web.search_backend is set to '{_vendor}', but its " + f"plugin ('{disabled_key}') is disabled in config. " + f"Re-enable it with `hermes plugins enable {disabled_key}` " + "(or remove it from plugins.disabled)." + ) + else: + error_text = selection_error( + "web", + f"'{backend}'", + "no registered web search provider has that name", + ) + response_data = {"success": False, "error": error_text} + result_json = json.dumps(response_data, indent=2, ensure_ascii=False) + debug_call_data["error"] = error_text + _debug.log_call("web_search_tool", debug_call_data) + _debug.save() + return result_json + # Never-configured install: fall back to the availability-walked + # active provider (legacy autodetect behavior). provider = get_active_search_provider() if provider is None: @@ -898,6 +948,35 @@ async def web_extract_tool( }, ensure_ascii=False, ) + from tools.tool_backend_helpers import ( + selection_error, + selection_exists, + ) + + if backend and selection_exists("web"): + # Strict selection: a stored-but-unregistered backend + # errors by name instead of silently switching to + # whatever the availability walk finds. + disabled_key = _disabled_web_plugin_for(capability="extract") + if disabled_key: + _vendor = disabled_key.split("/", 1)[-1] + error_text = ( + f"web.extract_backend is set to '{_vendor}', but " + f"its plugin ('{disabled_key}') is disabled in " + f"config. Re-enable it with `hermes plugins " + f"enable {disabled_key}` (or remove it from " + "plugins.disabled)." + ) + else: + error_text = selection_error( + "web", + f"'{backend}'", + "no registered web extract provider has that name", + ) + return json.dumps( + {"success": False, "error": error_text}, + ensure_ascii=False, + ) provider = get_active_extract_provider() if provider is None: # If the configured backend is a bundled web plugin the From 099258ef488e0a3aede6ad374c919ff08ad4261d Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 19 Aug 2026 15:24:26 -0700 Subject: [PATCH 06/24] fix(voice): route TTS/STT OpenAI audio on the stored selection, not credentials MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both _resolve_openai_audio_client_config resolvers now switch on the stored provider string: 'nous' (or legacy use_gateway: true) => managed openai-audio gateway only, erroring by selection name when unentitled — the STT twin previously never read the stored gateway intent at all, so a direct OPENAI_API_KEY silently overrode the Nous Subscription pick; stored vendor => direct credentials only with a selection-naming error on missing keys (no silent managed fallback); never-configured keeps the legacy ladder. DEFAULT_CONFIG stops seeding stt.provider: local, and the seeded value on existing configs is treated as no-selection so autodetect keeps working for that installed base. --- hermes_cli/config_defaults.py | 6 ++- tools/transcription_tools.py | 73 +++++++++++++++++++++++++++++++- tools/tts_tool.py | 79 ++++++++++++++++++++++++++++------- 3 files changed, 140 insertions(+), 18 deletions(-) diff --git a/hermes_cli/config_defaults.py b/hermes_cli/config_defaults.py index e55b3d923d..c1dd275401 100644 --- a/hermes_cli/config_defaults.py +++ b/hermes_cli/config_defaults.py @@ -1667,7 +1667,11 @@ DEFAULT_CONFIG = { # the raw transcript is also echoed back to the user as a 🎙️ message. # Set false to keep STT for the agent while suppressing that user-facing echo. "echo_transcripts": True, - "provider": "local", # "local" (free, faster-whisper) | "groq" | "openai" (Whisper API) | "mistral" (Voxtral Transcribe) | "elevenlabs" (Scribe) | "deepinfra" + # NOTE: no seeded "provider" key. Strict selection semantics treat a + # stored stt.provider as an explicit user pick; seeding "local" here + # made a fresh install indistinguishable from a user choice. The + # autodetect ladder covers unset. Valid values when set: + # "local" (free, faster-whisper) | "groq" | "openai" (Whisper API) | "mistral" (Voxtral Transcribe) | "elevenlabs" (Scribe) | "deepinfra" # Global language hint applied to EVERY provider unless a per-provider # language overrides it. Defaults to "en" — Whisper auto-detection # frequently misidentifies short/accented clips, which reads as diff --git a/tools/transcription_tools.py b/tools/transcription_tools.py index c02a35a934..77f4cca296 100644 --- a/tools/transcription_tools.py +++ b/tools/transcription_tools.py @@ -1025,6 +1025,27 @@ def _get_provider(stt_config: dict) -> str: explicit = "provider" in stt_config provider = stt_config.get("provider", DEFAULT_PROVIDER) + # The managed "Nous Subscription" selection (stt.provider: nous) is + # serviced by the OpenAI provider implementation, routed through the + # managed openai-audio gateway by _resolve_openai_audio_client_config. + if isinstance(provider, str) and provider.strip().lower() == "nous": + provider = "openai" + + if explicit and provider == "local": + # Legacy DEFAULT_CONFIG seeded ``stt.provider: local`` on every + # install, so a merged-config "local" is not proof of a user pick. + # ``read_selection`` reads the raw config.yaml and applies the + # seeded-value migration shim; when the raw file holds no stt + # selection, take the autodetect branch (which prefers local first + # anyway, so a genuine local user is unaffected when it's available). + try: + from tools.tool_backend_helpers import read_selection + + if read_selection("stt") is None: + explicit = False + except Exception: # pragma: no cover — helpers are in-repo + pass + # --- Explicit provider: respect the user's choice ---------------------- if explicit: @@ -3260,11 +3281,61 @@ def transcribe_audio_local_fallback( def _resolve_openai_audio_client_config() -> tuple[str, str]: - """Return direct OpenAI audio config or a managed gateway fallback.""" + """Return ``(api_key, base_url)`` for the OpenAI STT client. + + Strict selection semantics (switch on the stored ``stt`` provider + string; previously this resolver never read the stored gateway intent): + - ``"nous"`` (or legacy ``use_gateway: true``) → managed gateway ONLY; + unentitled/unreachable is a selection-naming error (a direct + OPENAI_API_KEY must NOT override it). + - any other stored stt provider → direct credentials ONLY; missing + credentials is a selection-naming error — no silent managed fallback. + - never-configured stt section → legacy ladder: config key → local + base_url → env key → managed gateway. + """ + from tools.tool_backend_helpers import ( + NOUS_MANAGED_PROVIDER, + read_selection, + selection_error, + ) + stt_config = _load_stt_config() openai_cfg = stt_config.get("openai") or {} cfg_api_key = openai_cfg.get("api_key", "") cfg_base_url = openai_cfg.get("base_url", "") + + selected = read_selection("stt") + + if selected == NOUS_MANAGED_PROVIDER: + managed_gateway = resolve_managed_tool_gateway("openai-audio") + if managed_gateway is None: + raise ValueError(selection_error( + "stt", + NOUS_MANAGED_PROVIDER, + "the Nous Tool Gateway is not available (not entitled or " + "unreachable)", + )) + return managed_gateway.nous_user_token, urljoin( + f"{managed_gateway.gateway_origin.rstrip('/')}/", "v1" + ) + + if selected is not None: + # Stored vendor selection: direct credentials only. + if cfg_api_key: + return cfg_api_key, (cfg_base_url or OPENAI_BASE_URL) + if cfg_base_url and _is_local_or_private_url(cfg_base_url): + return "not-needed", cfg_base_url + direct_api_key = resolve_openai_audio_api_key() + if direct_api_key: + return direct_api_key, OPENAI_BASE_URL + raise ValueError(selection_error( + "stt", + selected, + "neither stt.openai.api_key in config nor " + "VOICE_TOOLS_OPENAI_KEY/OPENAI_API_KEY is set", + )) + + # Never-configured stt section: legacy credential ladder. if cfg_api_key: return cfg_api_key, (cfg_base_url or OPENAI_BASE_URL) diff --git a/tools/tts_tool.py b/tools/tts_tool.py index fba661fd5b..3aa4875922 100644 --- a/tools/tts_tool.py +++ b/tools/tts_tool.py @@ -93,10 +93,12 @@ def _resolve_provider_key(env_var: str, provider_id: str) -> str: from tools.managed_tool_gateway import resolve_managed_tool_gateway from tools.tool_backend_helpers import ( + NOUS_MANAGED_PROVIDER, managed_nous_tools_enabled, nous_tool_gateway_unavailable_message, - prefers_gateway, + read_selection, resolve_openai_audio_api_key, + selection_error, ) from tools.xai_http import hermes_xai_user_agent @@ -651,8 +653,15 @@ def _get_provider(tts_config: Dict[str, Any]) -> str: Inference credentials do not imply consent to paid speech generation. Users opt into cloud TTS by setting ``tts.provider`` (normally through ``hermes tools``); otherwise the historical Edge backend remains active. + + The managed "Nous Subscription" selection (``tts.provider: nous``) is + serviced by the OpenAI provider implementation, routed through the + managed openai-audio gateway by ``_resolve_openai_audio_client_config``. """ - return (tts_config.get("provider") or DEFAULT_PROVIDER).lower().strip() + provider = (tts_config.get("provider") or DEFAULT_PROVIDER).lower().strip() + if provider == NOUS_MANAGED_PROVIDER: + return "openai" + return provider @dataclass(frozen=True) @@ -3785,24 +3794,61 @@ def _resolve_openai_audio_client_config() -> tuple[str, str, bool]: ``is_managed`` is True when the config resolves to the Nous managed audio gateway (a restricted proxy), so callers can coerce the request to what the - gateway supports. When ``tts.use_gateway`` is set the gateway is preferred - even if direct OpenAI credentials are present. + gateway supports. - Resolution order (mirrors the STT resolver): - 1. ``tts.openai.api_key`` / ``tts.openai.base_url`` from ``config.yaml`` - 2. ``VOICE_TOOLS_OPENAI_KEY`` / ``OPENAI_API_KEY`` environment variables - (still honoring ``tts.openai.base_url`` when set) - 3. Managed OpenAI audio tool gateway + Strict selection semantics (switch on the stored ``tts`` provider + string): + - ``"nous"`` (or legacy ``use_gateway: true``) → managed gateway ONLY; + unentitled/unreachable is a selection-naming error. + - any other stored tts provider → direct credentials ONLY + (``tts.openai.api_key`` then ``VOICE_TOOLS_OPENAI_KEY``/ + ``OPENAI_API_KEY``); missing credentials is a selection-naming error — + no silent managed fallback. + - never-configured tts section → legacy ladder: config key → env key → + managed gateway. """ tts_config = _load_tts_config() openai_cfg = (tts_config.get("openai") if isinstance(tts_config, dict) else None) or {} cfg_api_key = openai_cfg.get("api_key") or "" cfg_base_url = openai_cfg.get("base_url") or "" - if cfg_api_key and not prefers_gateway("tts"): + + selected = read_selection("tts") + + if selected == NOUS_MANAGED_PROVIDER: + managed_gateway = resolve_managed_tool_gateway("openai-audio") + if managed_gateway is None: + raise ValueError(selection_error( + "tts", + NOUS_MANAGED_PROVIDER, + "the Nous Tool Gateway is not available (not entitled or " + "unreachable)", + )) + return ( + managed_gateway.nous_user_token, + urljoin(f"{managed_gateway.gateway_origin.rstrip('/')}/", "v1"), + True, + ) + + if selected is not None: + # Stored vendor selection: direct credentials only. + if cfg_api_key: + return cfg_api_key, (cfg_base_url or DEFAULT_OPENAI_BASE_URL), False + direct_api_key = resolve_openai_audio_api_key() + if direct_api_key: + return direct_api_key, (cfg_base_url or DEFAULT_OPENAI_BASE_URL), False + raise ValueError(selection_error( + "tts", + selected, + "neither tts.openai.api_key in config nor " + "VOICE_TOOLS_OPENAI_KEY/OPENAI_API_KEY is set", + )) + + # Never-configured tts section: legacy credential ladder. + if cfg_api_key: return cfg_api_key, (cfg_base_url or DEFAULT_OPENAI_BASE_URL), False direct_api_key = resolve_openai_audio_api_key() - if direct_api_key and not prefers_gateway("tts"): + if direct_api_key: return direct_api_key, (cfg_base_url or DEFAULT_OPENAI_BASE_URL), False managed_gateway = resolve_managed_tool_gateway("openai-audio") @@ -3811,7 +3857,7 @@ def _resolve_openai_audio_client_config() -> tuple[str, str, bool]: "Neither tts.openai.api_key in config nor " "VOICE_TOOLS_OPENAI_KEY/OPENAI_API_KEY is set" ) - if managed_nous_tools_enabled() or prefers_gateway("tts"): + if managed_nous_tools_enabled(): message += ( ". " + nous_tool_gateway_unavailable_message( @@ -3828,11 +3874,12 @@ def _resolve_openai_audio_client_config() -> tuple[str, str, bool]: def _has_openai_audio_backend() -> bool: - """Return True when OpenAI audio can use config/env credentials or the managed gateway.""" - openai_cfg = (_load_tts_config().get("openai") or {}) - if openai_cfg.get("api_key"): + """Return True when the selected OpenAI audio route is usable.""" + try: + _resolve_openai_audio_client_config() return True - return bool(resolve_openai_audio_api_key() or resolve_managed_tool_gateway("openai-audio")) + except ValueError: + return False # =========================================================================== From 7f83d3808c13dcda96c12f488ba3d819c44c9437 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 19 Aug 2026 15:24:39 -0700 Subject: [PATCH 07/24] fix(browser): strict cloud-provider selection; camofox becomes a selection An explicitly stored browser.cloud_provider that names no registered plugin now raises the honest selection-naming error instead of warning and silently auto-detecting; the auto-detect walk (including the managed gateway entitlement probe) runs only when no cloud_provider key was ever written. The 'nous' selection routes to the Browser Use provider, whose config resolver is now a strict switch: 'nous' => managed only, stored vendor => direct BROWSER_USE_API_KEY only with a selection-naming error when missing. Camofox is selected via browser.cloud_provider: camofox; CAMOFOX_URL stays the server ADDRESS only and can no longer override an explicit different selection (never-configured installs keep the legacy env-var activation). --- plugins/browser/browser_use/provider.py | 75 ++++++++++++++++++------- tools/browser_camofox.py | 26 +++++++-- tools/browser_tool.py | 46 +++++++++------ 3 files changed, 106 insertions(+), 41 deletions(-) diff --git a/plugins/browser/browser_use/provider.py b/plugins/browser/browser_use/provider.py index f4008d440c..bfc2709e5e 100644 --- a/plugins/browser/browser_use/provider.py +++ b/plugins/browser/browser_use/provider.py @@ -134,37 +134,74 @@ class BrowserUseBrowserProvider(BrowserProvider): peek_nous_access_token, resolve_managed_tool_gateway, ) - from tools.tool_backend_helpers import prefers_gateway + from tools.tool_backend_helpers import ( + NOUS_MANAGED_PROVIDER, + read_selection, + ) + + def _managed_config() -> Optional[Dict[str, Any]]: + # Keep availability scans off the synchronous OAuth refresh path. + managed = resolve_managed_tool_gateway( + "browser-use", + token_reader=None if refresh_token else peek_nous_access_token, + ) + if managed is None: + return None + return { + "api_key": managed.nous_user_token, + "base_url": managed.gateway_origin.rstrip("/"), + "managed_mode": True, + } - # Direct API key wins unless the user has explicitly opted into the - # managed Nous gateway via ``tool_gateway.browser: gateway``. api_key = get_secret("BROWSER_USE_API_KEY") - if api_key and not prefers_gateway("browser"): + selected = read_selection("browser") + + # Strict selection: "nous" (or legacy use_gateway: true) → managed + # gateway ONLY; any other stored browser selection → direct API key + # ONLY (no silent managed fallback); never-configured → legacy + # behavior (direct key when present, else managed gateway). + if selected == NOUS_MANAGED_PROVIDER: + return _managed_config() + if selected is not None: + if api_key: + return { + "api_key": api_key, + "base_url": _BASE_URL, + "managed_mode": False, + } + return None + if api_key: return { "api_key": api_key, "base_url": _BASE_URL, "managed_mode": False, } - - # Keep availability scans off the synchronous OAuth refresh path. - managed = resolve_managed_tool_gateway( - "browser-use", - token_reader=None if refresh_token else peek_nous_access_token, - ) - if managed is None: - return None - - return { - "api_key": managed.nous_user_token, - "base_url": managed.gateway_origin.rstrip("/"), - "managed_mode": True, - } + return _managed_config() def _get_config(self) -> Dict[str, Any]: - from tools.tool_backend_helpers import managed_nous_tools_enabled + from tools.tool_backend_helpers import ( + NOUS_MANAGED_PROVIDER, + managed_nous_tools_enabled, + read_selection, + selection_error, + ) config = self._get_config_or_none() if config is None: + selected = read_selection("browser") + if selected == NOUS_MANAGED_PROVIDER: + raise ValueError(selection_error( + "browser", + NOUS_MANAGED_PROVIDER, + "the Nous Tool Gateway is not available (not entitled or " + "unreachable)", + )) + if selected is not None: + raise ValueError(selection_error( + "browser", + selected, + "BROWSER_USE_API_KEY is not set", + )) message = ( "Browser Use requires a direct BROWSER_USE_API_KEY credential." ) diff --git a/tools/browser_camofox.py b/tools/browser_camofox.py index ef68112f55..432ca85f0b 100644 --- a/tools/browser_camofox.py +++ b/tools/browser_camofox.py @@ -113,21 +113,39 @@ def _config_cdp_url() -> str: def is_camofox_mode() -> bool: - """True when Camofox backend is configured and no CDP override is active. + """True when the Camofox backend is selected and no CDP override is active. + + Camofox is a selection: ``browser.cloud_provider: camofox`` (set via + ``hermes tools``). ``CAMOFOX_URL`` is the server ADDRESS only — its + presence no longer selects the backend when a different + ``browser.cloud_provider`` is stored. Legacy read-time interpretation: + when NO cloud provider selection was ever written, a set ``CAMOFOX_URL`` + keeps activating Camofox exactly as before (nothing is migrated/written + to config). A CDP override takes priority over Camofox so the browser tools operate on the real CDP browser (and a CDP backend is treated as non-local for SSRF checks) instead of being silently routed to Camofox. The override may come from the ``BROWSER_CDP_URL`` env var (set by ``/browser connect``) OR a persistent ``browser.cdp_url`` in config.yaml — both are honored, matching - ``browser_tool._get_cdp_override()``'s precedence. (Previously only the env - var suppressed Camofox, so ``CAMOFOX_URL`` + a config CDP override still - routed navigation through Camofox.) + ``browser_tool._get_cdp_override()``'s precedence. """ if os.getenv("BROWSER_CDP_URL", "").strip(): return False if _config_cdp_url(): return False + try: + from tools.tool_backend_helpers import read_selection + + selected = read_selection("browser") + except Exception: # pragma: no cover — helpers are in-repo + selected = None + if selected == "camofox": + return True + if selected is not None: + # An explicit different browser selection wins: CAMOFOX_URL is just + # an address, not a choice. + return False return bool(get_camofox_url()) diff --git a/tools/browser_tool.py b/tools/browser_tool.py index 544a06d512..34f38baa1e 100644 --- a/tools/browser_tool.py +++ b/tools/browser_tool.py @@ -815,19 +815,26 @@ def _resolve_cloud_provider_uncached() -> Optional[CloudBrowserProvider]: global _cached_cloud_provider, _cloud_provider_resolved resolved: Optional[CloudBrowserProvider] = None + provider_key = None try: from hermes_cli.config import read_raw_config cfg = read_raw_config() browser_cfg = cfg.get("browser", {}) - provider_key = None if isinstance(browser_cfg, dict) and "cloud_provider" in browser_cfg: provider_key = normalize_browser_cloud_provider( browser_cfg.get("cloud_provider") ) - if provider_key == "local": + if provider_key in ("local", "camofox"): + # Camofox runs through the built-in browser tools + # (is_camofox_mode() dispatch), not a cloud provider. _cached_cloud_provider = None _cloud_provider_resolved = True return None + if provider_key == "nous": + # Managed "Nous Subscription" selection is serviced by the + # Browser Use provider, whose config resolver routes it + # through the managed browser-use gateway. + provider_key = "browser-use" if provider_key: try: if _is_legacy_provider_registry_overridden(): @@ -841,20 +848,20 @@ def _resolve_cloud_provider_uncached() -> Optional[CloudBrowserProvider]: # populated. Idempotent — cheap on subsequent calls. _ensure_browser_plugins_loaded() resolved = _registry_get_browser_provider(provider_key) - if resolved is None: - # Explicit config name unknown to the registry — - # might be a typo, an uninstalled plugin, or a - # registry-population failure. Warn the user - # (legacy code would have surfaced a typed - # credentials error via direct class instantiation; - # post-migration we surface this WARNING instead). - logger.warning( - "browser.cloud_provider=%r is not a registered " - "browser plugin; falling back to auto-detect " - "(install the corresponding plugin or fix the " - "config key spelling).", - provider_key, - ) + if resolved is None: + # Strict selection: a stored-but-unregistered name is an + # honest error, never a silent reroute to auto-detect. + from tools.tool_backend_helpers import selection_error + + raise ValueError(selection_error( + "browser", + f"'{provider_key}'", + "no registered browser plugin has that name (install " + "the corresponding plugin or fix the config key " + "spelling)", + )) + except ValueError: + raise except Exception: logger.warning( "Failed to instantiate explicit cloud_provider %r; will retry on next call", @@ -862,13 +869,16 @@ def _resolve_cloud_provider_uncached() -> Optional[CloudBrowserProvider]: exc_info=True, ) return None + except ValueError: + raise except Exception as e: # Config file may be temporarily unreadable; still try auto-detect so # env-based / managed-gateway credentials can resolve. Don't pin cache. logger.debug("Could not read cloud_provider from config: %s", e) - if resolved is None: - # Auto-detect path: Browser Use first (managed Nous gateway or + if resolved is None and provider_key is None: + # Auto-detect path — permitted ONLY when no cloud_provider selection + # was ever written: Browser Use first (managed Nous gateway or # direct API key), then Browserbase (direct credentials). Uses # the legacy class names imported at the top of this module so # tests that ``monkeypatch.setattr(browser_tool, "BrowserUseProvider", ...)`` From b10c5a8084b453595e796281249a70d39bb7c8c3 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 19 Aug 2026 15:24:39 -0700 Subject: [PATCH 08/24] fix(cli): persist one provider string per picker row; mirror strict routing in status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every hermes tools row now writes exactly one selection value per category — managed 'Nous Subscription' rows write 'nous', BYOK rows the vendor name (including the historically-unset BYOK-FAL image row) — and use_gateway is no longer written; fresh picks drop any legacy key so the read-time shim cannot override them. The non-managed clear now resolves the category from the row's own markers, covering plugin-injected rows the TOOL_CATEGORIES loop missed. Setup-flow writers (managed defaults, gateway enablement) store 'nous', and the feature-state mirrors in nous_subscription.py compute per-category selections with the same legacy interpretation so hermes status matches runtime: a stored vendor selection pins direct (managed availability no longer lights it up) and an explicit non-camofox selection beats a stray CAMOFOX_URL. --- hermes_cli/nous_subscription.py | 181 +++++++++++++++++------- hermes_cli/tools_config.py | 234 +++++++++++++++++++++----------- 2 files changed, 284 insertions(+), 131 deletions(-) diff --git a/hermes_cli/nous_subscription.py b/hermes_cli/nous_subscription.py index 5289cabce0..882956fe31 100644 --- a/hermes_cli/nous_subscription.py +++ b/hermes_cli/nous_subscription.py @@ -54,6 +54,26 @@ def _uses_gateway(section: object) -> bool: return is_truthy_value(section.get("use_gateway"), default=False) +def _selected_provider(section: object, name_key: str = "provider") -> Optional[str]: + """Return the stored provider string for a config section dict. + + Mirrors :func:`tools.tool_backend_helpers.read_selection`'s semantics on + an in-memory section dict: ``"nous"`` for the managed selection (stored + ``nous`` value or legacy ``use_gateway: true``), a vendor name for BYOK + picks, or ``None`` when no selection is stored. Keeping this in lockstep + with the runtime resolver is what stops ``hermes status`` from lying. + """ + if not isinstance(section, dict): + return None + if is_truthy_value(section.get("use_gateway"), default=False): + return "nous" + value = section.get(name_key) + if value is None: + return None + name = str(value).strip().lower() + return name or None + + @dataclass(frozen=True) class NousFeatureState: key: str @@ -315,11 +335,14 @@ def _resolve_browser_feature_state( on the latter, or setup/status advertise a browser that fails on first use when Chromium is missing. """ - if direct_camofox: - return "camofox", True, bool(browser_tool_enabled), False - if browser_provider_explicit: current_provider = browser_provider or "local" + if current_provider == "camofox": + # Camofox is now a stored selection (browser.cloud_provider: + # camofox); CAMOFOX_URL is only the server address. + available = bool(direct_camofox) + active = bool(browser_tool_enabled and available) + return current_provider, available, active, False if current_provider == "browserbase": available = bool(browser_local_available and direct_browserbase) active = bool(browser_tool_enabled and available) @@ -347,6 +370,11 @@ def _resolve_browser_feature_state( active = bool(browser_tool_enabled and available) return current_provider, available, active, False + # Never-configured autodetect: CAMOFOX_URL keeps activating Camofox + # exactly as before when no cloud_provider selection was ever stored. + if direct_camofox: + return "camofox", True, bool(browser_tool_enabled), False + if managed_browser_available or direct_browser_use: available = bool(browser_local_available) managed = bool( @@ -435,17 +463,48 @@ def get_nous_subscription_features( terminal_cfg.get("modal_mode") ) - # use_gateway flags — when True, the user explicitly opted into the - # Tool Gateway via `hermes model`, so direct credentials should NOT - # prevent gateway routing. - web_use_gateway = _uses_gateway(web_cfg) - tts_use_gateway = _uses_gateway(tts_cfg) - stt_use_gateway = _uses_gateway(stt_cfg) - browser_use_gateway = _uses_gateway(browser_cfg) + # Stored selections (strict model): one provider string per category. + # "nous" (stored value or legacy use_gateway: true) = managed gateway; + # vendor name = that vendor direct; None = never configured (autodetect). image_gen_cfg = config.get("image_gen") if isinstance(config.get("image_gen"), dict) else {} - image_use_gateway = _uses_gateway(image_gen_cfg) video_gen_cfg = config.get("video_gen") if isinstance(config.get("video_gen"), dict) else {} - video_use_gateway = _uses_gateway(video_gen_cfg) + web_selected = _selected_provider(web_cfg, "backend") + tts_selected = _selected_provider(tts_cfg) + stt_selected = _selected_provider(stt_cfg) + browser_selected = _selected_provider(browser_cfg, "cloud_provider") + image_selected = _selected_provider(image_gen_cfg) + video_selected = _selected_provider(video_gen_cfg) + + # Same seeded-value shim as tools.tool_backend_helpers.read_selection: + # legacy DEFAULT_CONFIG seeded ``stt.provider: local`` on every install, + # so that exact value with no picker-written use_gateway key is treated + # as never-configured. + if ( + stt_selected == "local" + and isinstance(stt_cfg, dict) + and "use_gateway" not in stt_cfg + ): + stt_selected = None + + # Managed selection flags (replace the legacy use_gateway reads — + # use_gateway is now interpreted only inside _selected_provider). + web_use_gateway = web_selected == "nous" + tts_use_gateway = tts_selected == "nous" + stt_use_gateway = stt_selected == "nous" + browser_use_gateway = browser_selected == "nous" + image_use_gateway = image_selected == "nous" + video_use_gateway = video_selected == "nous" + + # The "nous" selection is serviced by a concrete vendor implementation — + # normalize the current-provider labels so downstream vendor checks hold. + if web_backend == "nous" or web_use_gateway: + web_backend = "firecrawl" + if tts_provider == "nous" or tts_use_gateway: + tts_provider = "openai" + if stt_provider == "nous" or stt_use_gateway: + stt_provider = "openai" + if browser_provider == "nous" or browser_use_gateway: + browser_provider = "browser-use" direct_exa = bool(get_env_value("EXA_API_KEY")) direct_firecrawl = bool(get_env_value("FIRECRAWL_API_KEY") or get_env_value("FIRECRAWL_API_URL")) @@ -549,6 +608,27 @@ def get_nous_subscription_features( managed_enabled=managed_tools_flag, ) + # Strict selection: a stored VENDOR selection pins the category to direct + # credentials — managed availability must not light the feature up (the + # runtime will error, not reroute), and camofox/local selections must not + # be pre-empted by env credentials for other providers. + if web_selected is not None and not web_use_gateway: + managed_web_available = False + if image_selected is not None and not image_use_gateway: + managed_image_available = False + if video_selected is not None and not video_use_gateway: + managed_video_available = False + if tts_selected is not None and not tts_use_gateway: + managed_tts_available = False + if stt_selected is not None and not stt_use_gateway: + managed_stt_available = False + if browser_selected is not None and not browser_use_gateway: + managed_browser_available = False + if browser_selected is not None and browser_selected != "camofox": + # CAMOFOX_URL is the server address, not a selection: an explicit + # different browser choice wins over the env var. + direct_camofox = False + web_managed = web_backend == "firecrawl" and managed_web_available and not direct_firecrawl web_active = bool( web_tool_enabled @@ -664,17 +744,10 @@ def get_nous_subscription_features( modal_active = False modal_direct_override = False - tts_explicit_configured = False - raw_tts_cfg = config.get("tts") - if isinstance(raw_tts_cfg, dict) and "provider" in raw_tts_cfg: - tts_explicit_configured = tts_provider not in {"", "edge"} - - # STT considers any non-default provider explicit. "local" is the - # DEFAULT_CONFIG seed, so seeing it doesn't mean the user picked it. - stt_explicit_configured = False - raw_stt_cfg = config.get("stt") - if isinstance(raw_stt_cfg, dict) and "provider" in raw_stt_cfg: - stt_explicit_configured = stt_provider not in {"", "local"} + # Explicit-configured mirrors the stored selections computed above so + # status/picker markers stay in lockstep with runtime dispatch. + tts_explicit_configured = tts_selected is not None and tts_selected != "edge" + stt_explicit_configured = stt_selected is not None features = { "web": NousFeatureState( @@ -698,8 +771,8 @@ def get_nous_subscription_features( managed_by_nous=image_managed, direct_override=image_active and not image_managed, toolset_enabled=image_tool_enabled, - current_provider="FAL" if direct_fal else ("Nous Subscription" if image_managed else ""), - explicit_configured=direct_fal, + current_provider="FAL" if (image_selected not in (None, "nous") or (image_selected is None and direct_fal)) else ("Nous Subscription" if (image_managed or image_use_gateway) else ""), + explicit_configured=image_selected is not None or direct_fal, ), "video_gen": NousFeatureState( key="video_gen", @@ -710,8 +783,8 @@ def get_nous_subscription_features( managed_by_nous=video_managed, direct_override=video_active and not video_managed, toolset_enabled=video_tool_enabled, - current_provider="FAL" if direct_fal_video else ("Nous Subscription" if video_managed else ""), - explicit_configured=direct_fal_video, + current_provider="FAL" if (video_selected not in (None, "nous") or (video_selected is None and direct_fal_video)) else ("Nous Subscription" if (video_managed or video_use_gateway) else ""), + explicit_configured=video_selected is not None or direct_fal_video, ), "tts": NousFeatureState( key="tts", @@ -823,24 +896,26 @@ def apply_nous_managed_defaults( or get_env_value("FIRECRAWL_API_KEY") or get_env_value("FIRECRAWL_API_URL") ): - web_cfg["backend"] = "firecrawl" + web_cfg["backend"] = "nous" + web_cfg.pop("use_gateway", None) changed.add("web") if "tts" in selected_toolsets and not features.tts.explicit_configured and not ( resolve_openai_audio_api_key() or get_env_value("ELEVENLABS_API_KEY") ): - tts_cfg["provider"] = "openai" + tts_cfg["provider"] = "nous" + tts_cfg.pop("use_gateway", None) changed.add("tts") # STT: same pattern as TTS. The DEFAULT_CONFIG seed is "local" # (requires `pip install faster-whisper`); for Nous subscribers we - # flip it to "openai" so the managed audio gateway handles transcription - # via the same auth as TTS. Skipped when the user has explicitly - # configured STT, has direct credentials for a non-managed provider, - # has a working local backend (faster-whisper installed or a custom - # local command — strong intent signal that "local" was a choice, not - # just the DEFAULT_CONFIG seed), or isn't entitled to the managed + # flip it to the managed selection so the managed audio gateway handles + # transcription via the same auth as TTS. Skipped when the user has + # explicitly configured STT, has direct credentials for a non-managed + # provider, has a working local backend (faster-whisper installed or a + # custom local command — strong intent signal that "local" was a choice, + # not just the DEFAULT_CONFIG seed), or isn't entitled to the managed # "openai-audio" category (flipping would point at a gateway that # refuses them, silently breaking voice transcription). if ( @@ -854,14 +929,16 @@ def apply_nous_managed_defaults( and features.account_info is not None and features.account_info.tool_gateway_entitled_for("openai-audio") ): - stt_cfg["provider"] = "openai" + stt_cfg["provider"] = "nous" + stt_cfg.pop("use_gateway", None) changed.add("stt") if "browser" in selected_toolsets and not features.browser.explicit_configured and not ( get_env_value("BROWSER_USE_API_KEY") or get_env_value("BROWSERBASE_API_KEY") ): - browser_cfg["cloud_provider"] = "browser-use" + browser_cfg["cloud_provider"] = "nous" + browser_cfg.pop("use_gateway", None) changed.add("browser") if "image_gen" in selected_toolsets and not fal_key_is_configured(): @@ -869,7 +946,8 @@ def apply_nous_managed_defaults( if not isinstance(image_cfg, dict): image_cfg = {} config["image_gen"] = image_cfg - image_cfg["use_gateway"] = True + image_cfg["provider"] = "nous" + image_cfg.pop("use_gateway", None) changed.add("image_gen") # Video gen is not funded by the free tool pool, so only wire managed video @@ -883,8 +961,8 @@ def apply_nous_managed_defaults( if not isinstance(video_cfg, dict): video_cfg = {} config["video_gen"] = video_cfg - video_cfg["provider"] = "fal" - video_cfg["use_gateway"] = True + video_cfg["provider"] = "nous" + video_cfg.pop("use_gateway", None) changed.add("video_gen") return changed @@ -1050,23 +1128,23 @@ def apply_gateway_defaults( config["browser"] = browser_cfg if "web" in tool_keys: - web_cfg["backend"] = "firecrawl" - web_cfg["use_gateway"] = True + web_cfg["backend"] = "nous" + web_cfg.pop("use_gateway", None) changed.add("web") if "tts" in tool_keys: - tts_cfg["provider"] = "openai" - tts_cfg["use_gateway"] = True + tts_cfg["provider"] = "nous" + tts_cfg.pop("use_gateway", None) changed.add("tts") if "stt" in tool_keys: - stt_cfg["provider"] = "openai" - stt_cfg["use_gateway"] = True + stt_cfg["provider"] = "nous" + stt_cfg.pop("use_gateway", None) changed.add("stt") if "browser" in tool_keys: - browser_cfg["cloud_provider"] = "browser-use" - browser_cfg["use_gateway"] = True + browser_cfg["cloud_provider"] = "nous" + browser_cfg.pop("use_gateway", None) changed.add("browser") if "image_gen" in tool_keys: @@ -1074,7 +1152,8 @@ def apply_gateway_defaults( if not isinstance(image_cfg, dict): image_cfg = {} config["image_gen"] = image_cfg - image_cfg["use_gateway"] = True + image_cfg["provider"] = "nous" + image_cfg.pop("use_gateway", None) changed.add("image_gen") if "video_gen" in tool_keys: @@ -1082,8 +1161,8 @@ def apply_gateway_defaults( if not isinstance(video_cfg, dict): video_cfg = {} config["video_gen"] = video_cfg - video_cfg["provider"] = "fal" - video_cfg["use_gateway"] = True + video_cfg["provider"] = "nous" + video_cfg.pop("use_gateway", None) changed.add("video_gen") return changed diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index 599ec3abf1..a724b4888f 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -31,7 +31,7 @@ from hermes_cli.nous_subscription import ( get_nous_subscription_features, ) from hermes_cli.nous_account import format_nous_portal_entitlement_message -from tools.tool_backend_helpers import fal_key_is_configured +from tools.tool_backend_helpers import NOUS_MANAGED_PROVIDER, fal_key_is_configured from utils import base_url_hostname, is_truthy_value logger = logging.getLogger(__name__) @@ -3823,39 +3823,55 @@ def _is_provider_active( image_cfg = config.get("image_gen", {}) if isinstance(image_cfg, dict): configured_provider = image_cfg.get("provider") - if configured_provider not in {None, "", "fal"}: + if configured_provider not in {None, "", "fal", NOUS_MANAGED_PROVIDER}: return False - if image_cfg.get("use_gateway") is not None and not is_truthy_value(image_cfg.get("use_gateway"), default=False): + if ( + configured_provider != NOUS_MANAGED_PROVIDER + and image_cfg.get("use_gateway") is not None + and not is_truthy_value(image_cfg.get("use_gateway"), default=False) + ): return False return feature.managed_by_nous if managed_feature == "video_gen": video_cfg = config.get("video_gen", {}) if isinstance(video_cfg, dict): configured_provider = video_cfg.get("provider") - if configured_provider not in {None, "", "fal"}: + if configured_provider not in {None, "", "fal", NOUS_MANAGED_PROVIDER}: return False - if video_cfg.get("use_gateway") is not None and not is_truthy_value(video_cfg.get("use_gateway"), default=False): + if ( + configured_provider != NOUS_MANAGED_PROVIDER + and video_cfg.get("use_gateway") is not None + and not is_truthy_value(video_cfg.get("use_gateway"), default=False) + ): return False return feature.managed_by_nous if provider.get("tts_provider"): return ( feature.managed_by_nous - and cfg_get(config, "tts", "provider") == provider["tts_provider"] + and cfg_get(config, "tts", "provider") + in {provider["tts_provider"], NOUS_MANAGED_PROVIDER} ) if provider.get("stt_provider"): return ( feature.managed_by_nous - and cfg_get(config, "stt", "provider") == provider["stt_provider"] + and cfg_get(config, "stt", "provider") + in {provider["stt_provider"], NOUS_MANAGED_PROVIDER} ) if "browser_provider" in provider: # Browser Use mode is a driver on top of the provider (it attaches # to the provider's CDP endpoint), so the provider row stays # active alongside the Browser Use row. current = cfg_get(config, "browser", "cloud_provider") - return feature.managed_by_nous and provider["browser_provider"] == current + return feature.managed_by_nous and current in { + provider["browser_provider"], + NOUS_MANAGED_PROVIDER, + } if provider.get("web_backend"): current = cfg_get(config, "web", "backend") - return feature.managed_by_nous and current == provider["web_backend"] + return feature.managed_by_nous and current in { + provider["web_backend"], + NOUS_MANAGED_PROVIDER, + } return feature.managed_by_nous if provider.get("tts_provider"): @@ -4168,20 +4184,19 @@ def _configure_xai_imagine_storage(section_name: str, config: dict) -> None: def _select_plugin_image_gen_provider(plugin_name: str, config: dict, *, use_gateway: bool = False) -> None: """Persist a plugin-backed image generation provider selection. - ``use_gateway`` mirrors :func:`_select_plugin_video_gen_provider`: a - provider picked through the Nous-managed flow must keep routing through - the gateway. Hardcoding ``False`` here silently flipped Nous-managed FAL - picks onto the user's personal FAL_KEY — _write_provider_config sets - ``image_gen.use_gateway = True`` for a managed pick, and this function - runs AFTER it, so the hardcoded value clobbered the managed flag. + ``use_gateway=True`` marks a provider picked through the Nous-managed + flow: the stored selection becomes ``image_gen.provider: nous`` (the + single provider string the runtime switches on). BYOK picks store the + plugin name. Any legacy ``use_gateway`` key is removed so old-config + read-time shims cannot override the fresh selection. """ img_cfg = config.setdefault("image_gen", {}) if not isinstance(img_cfg, dict): img_cfg = {} config["image_gen"] = img_cfg - img_cfg["provider"] = plugin_name - img_cfg["use_gateway"] = use_gateway - _print_success(f" image_gen.provider set to: {plugin_name}") + img_cfg["provider"] = NOUS_MANAGED_PROVIDER if use_gateway else plugin_name + img_cfg.pop("use_gateway", None) + _print_success(f" image_gen.provider set to: {img_cfg['provider']}") _configure_imagegen_model_for_plugin(plugin_name, config) if plugin_name == "xai": _configure_xai_imagine_storage("image_gen", config) @@ -4321,14 +4336,19 @@ def _configure_stt_model(stt_provider: str, config: dict) -> None: def _select_plugin_video_gen_provider(plugin_name: str, config: dict, *, use_gateway: bool = False) -> None: - """Persist a plugin-backed video generation provider selection.""" + """Persist a plugin-backed video generation provider selection. + + Mirrors :func:`_select_plugin_image_gen_provider`: managed picks store + ``video_gen.provider: nous``; BYOK picks store the plugin name; any + legacy ``use_gateway`` key is removed. + """ vid_cfg = config.setdefault("video_gen", {}) if not isinstance(vid_cfg, dict): vid_cfg = {} config["video_gen"] = vid_cfg - vid_cfg["provider"] = plugin_name - vid_cfg["use_gateway"] = use_gateway - _print_success(f" video_gen.provider set to: {plugin_name}") + vid_cfg["provider"] = NOUS_MANAGED_PROVIDER if use_gateway else plugin_name + vid_cfg.pop("use_gateway", None) + _print_success(f" video_gen.provider set to: {vid_cfg['provider']}") _configure_videogen_model_for_plugin(plugin_name, config) if plugin_name == "xai": _configure_xai_imagine_storage("video_gen", config) @@ -4339,32 +4359,45 @@ def _write_provider_config(provider: dict, config: dict, *, managed_feature) -> This is the pure, non-interactive core of :func:`_configure_provider` — it writes ``tts.provider`` / ``browser.cloud_provider`` / ``web.backend`` - and the ``use_gateway`` flags based on the provider's markers, but does - NOT prompt for env vars, run post-setup hooks, gate on Nous auth, or run - interactive model pickers. Both the CLI configurator and the desktop GUI - ``PUT .../provider`` endpoint call through here so there is one code path. + based on the provider's markers, but does NOT prompt for env vars, run + post-setup hooks, gate on Nous auth, or run interactive model pickers. + Both the CLI configurator and the desktop GUI ``PUT .../provider`` + endpoint call through here so there is one code path. + + Selection model: every row writes exactly ONE provider string per + category. Managed "Nous Subscription" rows write ``nous``; BYOK rows + write the vendor name. ``use_gateway`` is no longer written — a fresh + pick removes any legacy key from the touched section so the read-time + legacy shim (use_gateway: true ⇒ nous) cannot override the new choice. """ + def _set_selection(section_key: str, name_key: str, vendor_value) -> None: + section = config.setdefault(section_key, {}) + if not isinstance(section, dict): + section = {} + config[section_key] = section + section[name_key] = ( + NOUS_MANAGED_PROVIDER if managed_feature else vendor_value + ) + section.pop("use_gateway", None) + # Set TTS provider in config if applicable if provider.get("tts_provider"): - tts_cfg = config.setdefault("tts", {}) - tts_cfg["provider"] = provider["tts_provider"] - tts_cfg["use_gateway"] = bool(managed_feature) + _set_selection("tts", "provider", provider["tts_provider"]) # Set STT provider in config if applicable if provider.get("stt_provider"): - stt_cfg = config.setdefault("stt", {}) - stt_cfg["provider"] = provider["stt_provider"] - stt_cfg["use_gateway"] = bool(managed_feature) + _set_selection("stt", "provider", provider["stt_provider"]) # Set browser cloud provider in config if applicable if "browser_provider" in provider: bp = provider["browser_provider"] browser_cfg = config.setdefault("browser", {}) - if bp: - browser_cfg["cloud_provider"] = bp - # Browser Use mode (browser.backend) composes with the provider — - # switching providers keeps the driver choice intact. - browser_cfg["use_gateway"] = bool(managed_feature) + if bp or managed_feature: + # Browser Use mode (browser.backend) composes with the provider — + # switching providers keeps the driver choice intact. + _set_selection("browser", "cloud_provider", bp) + else: + browser_cfg.pop("use_gateway", None) if provider.get("browser_backend"): browser_cfg = config.setdefault("browser", {}) @@ -4372,28 +4405,51 @@ def _write_provider_config(provider: dict, config: dict, *, managed_feature) -> # Set web search backend in config if applicable if provider.get("web_backend"): - web_cfg = config.setdefault("web", {}) - web_cfg["backend"] = provider["web_backend"] - web_cfg["use_gateway"] = bool(managed_feature) + _set_selection("web", "backend", provider["web_backend"]) # Set computer_use backend in config if applicable if provider.get("computer_use_backend"): cu_cfg = config.setdefault("computer_use", {}) cu_cfg["backend"] = provider["computer_use_backend"] - # For tools without a specific config key (e.g. image_gen), still - # track use_gateway so the runtime knows the user's intent. + # Managed rows for categories without a marker handled above (e.g. the + # image_gen/video_gen "Nous Subscription" rows carry only + # managed_nous_feature) still persist the "nous" selection. if managed_feature and managed_feature not in {"web", "tts", "stt", "browser"}: - config.setdefault(managed_feature, {})["use_gateway"] = True + section = config.setdefault(managed_feature, {}) + if isinstance(section, dict): + section["provider"] = NOUS_MANAGED_PROVIDER + section.pop("use_gateway", None) elif not managed_feature: - # User picked a non-gateway provider — find which category this - # belongs to and clear use_gateway if it was previously set. - for cat_key, cat in TOOL_CATEGORIES.items(): - if provider in cat.get("providers", []): - section = config.get(cat_key) - if isinstance(section, dict) and section.get("use_gateway"): - section["use_gateway"] = False - break + # User picked a non-gateway provider — clear any stale legacy + # use_gateway key on the category so the read-time shim cannot + # override the fresh selection. Resolve the category from the + # provider's own markers first (plugin-injected rows are NOT in + # TOOL_CATEGORIES' hardcoded provider lists and previously skipped + # this clear), then fall back to the category-membership walk. + marker_sections = { + "tts_provider": "tts", + "stt_provider": "stt", + "browser_provider": "browser", + "web_backend": "web", + "image_gen_plugin_name": "image_gen", + "imagegen_backend": "image_gen", + "video_gen_plugin_name": "video_gen", + } + cleared = False + for marker, section_key in marker_sections.items(): + if provider.get(marker) or marker in provider: + section = config.get(section_key) + if isinstance(section, dict): + section.pop("use_gateway", None) + cleared = True + if not cleared: + for cat_key, cat in TOOL_CATEGORIES.items(): + if provider in cat.get("providers", []): + section = config.get(cat_key) + if isinstance(section, dict): + section.pop("use_gateway", None) + break def apply_provider_selection(ts_key: str, provider_name: str, config: dict) -> None: @@ -4425,15 +4481,17 @@ def apply_provider_selection(ts_key: str, provider_name: str, config: dict) -> N # Plugin-registered image/video gen backends record the provider name in # their own config section. Write that here (without the interactive # model picker the CLI runs afterwards — model choice is a separate GUI - # flow). + # flow). Managed picks store the "nous" selection. plugin_name = provider.get("image_gen_plugin_name") if plugin_name: img_cfg = config.setdefault("image_gen", {}) if not isinstance(img_cfg, dict): img_cfg = {} config["image_gen"] = img_cfg - img_cfg["provider"] = plugin_name - img_cfg["use_gateway"] = bool(managed_feature) + img_cfg["provider"] = ( + NOUS_MANAGED_PROVIDER if managed_feature else plugin_name + ) + img_cfg.pop("use_gateway", None) video_plugin = provider.get("video_gen_plugin_name") if video_plugin: @@ -4441,15 +4499,22 @@ def apply_provider_selection(ts_key: str, provider_name: str, config: dict) -> N if not isinstance(vid_cfg, dict): vid_cfg = {} config["video_gen"] = vid_cfg - vid_cfg["provider"] = video_plugin - vid_cfg["use_gateway"] = bool(managed_feature) + vid_cfg["provider"] = ( + NOUS_MANAGED_PROVIDER if managed_feature else video_plugin + ) + vid_cfg.pop("use_gateway", None) - # In-tree FAL imagegen backend: keep image_gen.provider on the legacy - # path (mirrors _configure_provider). - if provider.get("imagegen_backend"): + # In-tree FAL imagegen backend (BYOK): always persist the explicit + # ``image_gen.provider: fal`` selection — historically this row could + # leave the provider key unset, making a deliberate BYOK pick + # indistinguishable from a never-configured install. + if provider.get("imagegen_backend") and not managed_feature: img_cfg = config.setdefault("image_gen", {}) - if isinstance(img_cfg, dict) and img_cfg.get("provider") not in {None, "", "fal"}: - img_cfg["provider"] = "fal" + if not isinstance(img_cfg, dict): + img_cfg = {} + config["image_gen"] = img_cfg + img_cfg["provider"] = "fal" + img_cfg.pop("use_gateway", None) def _configure_provider( @@ -4503,8 +4568,10 @@ def _configure_provider( # Set TTS provider in config if applicable if provider.get("tts_provider"): tts_cfg = config.setdefault("tts", {}) - tts_cfg["provider"] = provider["tts_provider"] - tts_cfg["use_gateway"] = bool(managed_feature) + tts_cfg["provider"] = ( + NOUS_MANAGED_PROVIDER if managed_feature else provider["tts_provider"] + ) + tts_cfg.pop("use_gateway", None) # Set STT provider in config if applicable if provider.get("stt_provider"): @@ -4552,13 +4619,15 @@ def _configure_provider( backend = provider.get("imagegen_backend") if backend: _configure_imagegen_model(backend, config) - # In-tree FAL is the only non-plugin backend today. Keep - # image_gen.provider clear so the dispatch shim falls through - # to the legacy FAL path. + # In-tree FAL is the only non-plugin backend today. Persist the + # explicit selection: "nous" for a managed row, "fal" for BYOK. img_cfg = config.setdefault("image_gen", {}) - if isinstance(img_cfg, dict) and img_cfg.get("provider") not in {None, "", "fal"}: - img_cfg["provider"] = "fal" - # STT providers prompt for model selection after provider pick + if isinstance(img_cfg, dict): + img_cfg["provider"] = ( + NOUS_MANAGED_PROVIDER if managed_feature else "fal" + ) + img_cfg.pop("use_gateway", None) + # STT providers prompt for model selection after backend pick # (skipped for managed rows — the gateway pins the model). if provider.get("stt_provider") and not managed_feature: _configure_stt_model(provider["stt_provider"], config) @@ -4636,8 +4705,11 @@ def _configure_provider( if backend: _configure_imagegen_model(backend, config) img_cfg = config.setdefault("image_gen", {}) - if isinstance(img_cfg, dict) and img_cfg.get("provider") not in {None, "", "fal"}: - img_cfg["provider"] = "fal" + if isinstance(img_cfg, dict): + img_cfg["provider"] = ( + NOUS_MANAGED_PROVIDER if managed_feature else "fal" + ) + img_cfg.pop("use_gateway", None) # STT providers prompt for model selection after env vars are in. if provider.get("stt_provider") and not managed_feature: _configure_stt_model(provider["stt_provider"], config) @@ -5091,14 +5163,14 @@ def _reconfigure_provider( if backend == "fal": img_cfg = config.setdefault("image_gen", {}) if isinstance(img_cfg, dict): - img_cfg["provider"] = "fal" # A managed (Nous Subscription) row also carries - # imagegen_backend="fal" — the model picker runs AFTER - # _write_provider_config set use_gateway=True, so an - # unconditional False here silently flipped managed - # picks onto the user's personal FAL_KEY (same class - # as fe63353cb, which fixed the plugin-provider path). - img_cfg["use_gateway"] = bool(managed_feature) + # imagegen_backend="fal" — store the "nous" selection + # for it, "fal" for BYOK, and drop any legacy + # use_gateway key. + img_cfg["provider"] = ( + NOUS_MANAGED_PROVIDER if managed_feature else "fal" + ) + img_cfg.pop("use_gateway", None) # STT providers prompt for model selection on reconfig too. if provider.get("stt_provider") and not managed_feature: _configure_stt_model(provider["stt_provider"], config) @@ -5140,10 +5212,12 @@ def _reconfigure_provider( if backend == "fal": img_cfg = config.setdefault("image_gen", {}) if isinstance(img_cfg, dict): - img_cfg["provider"] = "fal" # Same managed-row guard as the no-env-vars branch above: # never clobber a Nous-managed pick back onto direct keys. - img_cfg["use_gateway"] = bool(managed_feature) + img_cfg["provider"] = ( + NOUS_MANAGED_PROVIDER if managed_feature else "fal" + ) + img_cfg.pop("use_gateway", None) # STT providers prompt for model selection on reconfig too. if provider.get("stt_provider") and not managed_feature: From 89e75f4770705f12dabfb61bd713c8fbceb7643e Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 19 Aug 2026 15:24:46 -0700 Subject: [PATCH 09/24] test(tools): pin strict provider-string selection per category New tests/tools/test_strict_provider_selection.py covers read_selection semantics (legacy use_gateway interpretation, seeded stt local, empty strings, browser.backend vs cloud_provider) and the three strict behaviors per category: managed 'nous' selection wins over present direct keys, a vendor selection with missing credentials raises the selection-naming error with NO managed call, and never-configured installs keep today's autodetect. Updated the tests that pinned the old credential-first precedence (TTS resolver gateway override, STT silent managed fallback, web invalid-backend reroute, video_gen picker writes). Sabotage-verified: reverting the image FAL strict switch makes the new managed-selection tests fail. --- tests/hermes_cli/test_nous_subscription.py | 9 +- tests/hermes_cli/test_tools_config.py | 4 +- tests/tools/test_managed_media_gateways.py | 6 +- tests/tools/test_strict_provider_selection.py | 357 ++++++++++++++++++ tests/tools/test_transcription_tools.py | 14 + tests/tools/test_tts_openai_config.py | 38 +- tests/tools/test_web_tools_config.py | 23 +- 7 files changed, 434 insertions(+), 17 deletions(-) create mode 100644 tests/tools/test_strict_provider_selection.py diff --git a/tests/hermes_cli/test_nous_subscription.py b/tests/hermes_cli/test_nous_subscription.py index b71a0fb284..73f680d3d1 100644 --- a/tests/hermes_cli/test_nous_subscription.py +++ b/tests/hermes_cli/test_nous_subscription.py @@ -150,9 +150,8 @@ def test_prompt_enable_tool_gateway_pool_offers_covered_tools_only(monkeypatch): def test_apply_nous_managed_defaults_writes_video_gen_config(monkeypatch): - """apply_nous_managed_defaults must write video_gen.provider and - video_gen.use_gateway when a Nous subscriber selects video_gen - without a direct FAL_KEY.""" + """apply_nous_managed_defaults must store the managed 'nous' selection + when a Nous subscriber selects video_gen without a direct FAL_KEY.""" monkeypatch.setattr(ns, "managed_nous_tools_enabled", lambda **kw: True) monkeypatch.delenv("FAL_KEY", raising=False) monkeypatch.setattr(ns, "fal_key_is_configured", lambda: False) @@ -167,8 +166,8 @@ def test_apply_nous_managed_defaults_writes_video_gen_config(monkeypatch): ) assert "video_gen" in changed - assert config["video_gen"]["provider"] == "fal" - assert config["video_gen"]["use_gateway"] is True + assert config["video_gen"]["provider"] == "nous" + assert "use_gateway" not in config["video_gen"] # --------------------------------------------------------------------------- diff --git a/tests/hermes_cli/test_tools_config.py b/tests/hermes_cli/test_tools_config.py index f978a490c0..3212eb6146 100644 --- a/tests/hermes_cli/test_tools_config.py +++ b/tests/hermes_cli/test_tools_config.py @@ -256,8 +256,8 @@ def test_first_install_nous_auto_configures_video_gen(monkeypatch): tools_command(first_install=True, config=config) - assert config["video_gen"]["provider"] == "fal" - assert config["video_gen"]["use_gateway"] is True + assert config["video_gen"]["provider"] == "nous" + assert "use_gateway" not in config["video_gen"] # video_gen should NOT appear in the manual configure list — it's auto-configured assert "video_gen" not in configured diff --git a/tests/tools/test_managed_media_gateways.py b/tests/tools/test_managed_media_gateways.py index 01343140b6..fa73128b6a 100644 --- a/tests/tools/test_managed_media_gateways.py +++ b/tests/tools/test_managed_media_gateways.py @@ -247,7 +247,9 @@ def test_transcription_uses_model_specific_response_formats(monkeypatch, tmp_pat _install_fake_tools_package() _install_fake_openai_module(whisper_capture, transcription_response="hello from whisper") monkeypatch.setenv("HERMES_HOME", str(tmp_path)) - (tmp_path / "config.yaml").write_text("stt:\n provider: openai\n") + # The managed audio route is the stored "nous" selection (strict model); + # a stored "openai" selection now means direct credentials only. + (tmp_path / "config.yaml").write_text("stt:\n provider: nous\n") monkeypatch.delenv("VOICE_TOOLS_OPENAI_KEY", raising=False) monkeypatch.delenv("OPENAI_API_KEY", raising=False) monkeypatch.setenv("TOOL_GATEWAY_DOMAIN", "nousresearch.com") @@ -257,7 +259,7 @@ def test_transcription_uses_model_specific_response_formats(monkeypatch, tmp_pat "tools.transcription_tools", "transcription_tools.py", ) - transcription_tools._load_stt_config = lambda: {"provider": "openai"} + transcription_tools._load_stt_config = lambda: {"provider": "nous"} audio_path = tmp_path / "audio.wav" audio_path.write_bytes(b"RIFF0000WAVEfmt ") diff --git a/tests/tools/test_strict_provider_selection.py b/tests/tools/test_strict_provider_selection.py new file mode 100644 index 0000000000..cbb158e47a --- /dev/null +++ b/tests/tools/test_strict_provider_selection.py @@ -0,0 +1,357 @@ +"""Strict tool-provider selection: the `hermes tools` choice always wins. + +Policy (owner decision): the provider string stored in config.yaml is what +runs at call time. "nous" → managed Nous Tool Gateway only; a vendor name → +that vendor direct with the user's own credentials; no key ever written → +today's credential autodetect. Credential presence must NEVER select or +reroute; a selected-but-broken provider produces an honest error naming the +selection and pointing at `hermes tools`. + +Per category these tests pin the three strict behaviors: + (a) managed selection + direct key present ⇒ managed route (key ignored) + (b) vendor selection + key missing ⇒ selection-naming error, NO managed call + (c) never-configured ⇒ legacy autodetect unchanged +""" + +from types import SimpleNamespace +from unittest.mock import patch + +import pytest + +from tools import tool_backend_helpers as tbh + + +MANAGED = SimpleNamespace( + nous_user_token="managed-token", + gateway_origin="https://gateway.nousresearch.com", +) + + +# --------------------------------------------------------------------------- +# read_selection — the shared helper +# --------------------------------------------------------------------------- + + +class TestReadSelection: + def _with_raw(self, raw): + return patch( + "hermes_cli.config.read_raw_config_readonly", + return_value=raw, + ) + + def test_never_configured_returns_none(self): + with self._with_raw({}): + assert tbh.read_selection("image_gen") is None + + def test_vendor_provider_returned(self): + with self._with_raw({"image_gen": {"provider": "fal"}}): + assert tbh.read_selection("image_gen") == "fal" + + def test_nous_provider_returned(self): + with self._with_raw({"image_gen": {"provider": "nous"}}): + assert tbh.read_selection("image_gen") == "nous" + + def test_legacy_use_gateway_true_maps_to_nous(self): + """Old configs stored use_gateway: true beside a vendor name — only + the managed picker row ever wrote it, so it means 'nous'.""" + with self._with_raw({"video_gen": {"provider": "fal", "use_gateway": True}}): + assert tbh.read_selection("video_gen") == "nous" + + def test_legacy_use_gateway_false_keeps_vendor(self): + with self._with_raw({"tts": {"provider": "openai", "use_gateway": False}}): + assert tbh.read_selection("tts") == "openai" + + def test_empty_string_backend_is_no_selection(self): + """DEFAULT_CONFIG's seeded empty strings are not selections.""" + with self._with_raw({"web": {"backend": ""}}): + assert tbh.read_selection("web") is None + + def test_seeded_stt_local_is_no_selection(self): + """Legacy DEFAULT_CONFIG seeded stt.provider: local on every + install; that value alone must be treated as never-configured.""" + with self._with_raw({"stt": {"provider": "local"}}): + assert tbh.read_selection("stt") is None + + def test_stt_local_with_use_gateway_key_is_a_selection(self): + """A picker-written stt section (use_gateway key present) means + local was a genuine choice.""" + with self._with_raw({"stt": {"provider": "local", "use_gateway": False}}): + assert tbh.read_selection("stt") == "local" + + def test_browser_backend_key_is_not_the_cloud_selection(self): + """browser.backend is the driver choice (browser-use CLI vs built-in + tools), not the cloud provider selection.""" + with self._with_raw({"browser": {"backend": "browser-use"}}): + assert tbh.read_selection("browser") is None + + def test_web_per_capability_keys_mark_configured(self): + with self._with_raw({"web": {"search_backend": "searxng"}}): + assert tbh.read_selection("web") is None + assert tbh.selection_exists("web") is True + + +# --------------------------------------------------------------------------- +# Image generation (FAL) +# --------------------------------------------------------------------------- + + +class TestImageFalStrictSelection: + def test_nous_selection_routes_managed_even_with_fal_key(self): + from tools import image_generation_tool as it + + with patch.object(it, "read_selection", return_value="nous"), \ + patch.object(it, "fal_key_is_configured", return_value=True), \ + patch.object(it, "resolve_managed_tool_gateway", return_value=MANAGED) as gw: + assert it._resolve_managed_fal_gateway() is MANAGED + gw.assert_called_once_with("fal-queue") + + def test_nous_selection_unentitled_raises_selection_error(self): + from tools import image_generation_tool as it + + with patch.object(it, "read_selection", return_value="nous"), \ + patch.object(it, "fal_key_is_configured", return_value=True), \ + patch.object(it, "resolve_managed_tool_gateway", return_value=None): + with pytest.raises(ValueError) as exc: + it._resolve_managed_fal_gateway() + assert "image_gen is configured to use nous" in str(exc.value) + assert "hermes tools" in str(exc.value) + + def test_fal_selection_missing_key_errors_without_managed_call(self): + from tools import image_generation_tool as it + + with patch.object(it, "read_selection", return_value="fal"), \ + patch.object(it, "fal_key_is_configured", return_value=False), \ + patch.object(it, "resolve_managed_tool_gateway") as gw: + with pytest.raises(ValueError) as exc: + it._resolve_managed_fal_gateway() + gw.assert_not_called() + assert "FAL_KEY" in str(exc.value) + assert "image_gen is configured to use fal" in str(exc.value) + assert "hermes tools" in str(exc.value) + + def test_fal_selection_with_key_routes_direct(self): + from tools import image_generation_tool as it + + with patch.object(it, "read_selection", return_value="fal"), \ + patch.object(it, "fal_key_is_configured", return_value=True), \ + patch.object(it, "resolve_managed_tool_gateway") as gw: + assert it._resolve_managed_fal_gateway() is None + gw.assert_not_called() + + def test_never_configured_autodetect_direct_when_key_present(self): + from tools import image_generation_tool as it + + with patch.object(it, "read_selection", return_value=None), \ + patch.object(it, "fal_key_is_configured", return_value=True): + assert it._resolve_managed_fal_gateway() is None + + def test_never_configured_autodetect_managed_when_no_key(self): + from tools import image_generation_tool as it + + with patch.object(it, "read_selection", return_value=None), \ + patch.object(it, "fal_key_is_configured", return_value=False), \ + patch.object(it, "resolve_managed_tool_gateway", return_value=MANAGED): + assert it._resolve_managed_fal_gateway() is MANAGED + + def test_check_fal_api_key_reflects_selection(self): + from tools import image_generation_tool as it + + with patch.object(it, "read_selection", return_value="fal"), \ + patch.object(it, "fal_key_is_configured", return_value=False), \ + patch.object(it, "resolve_managed_tool_gateway", return_value=MANAGED): + # Broken vendor selection reports unavailable even though the + # managed gateway would resolve. + assert it.check_fal_api_key() is False + + +# --------------------------------------------------------------------------- +# Video generation (FAL plugin) +# --------------------------------------------------------------------------- + + +class TestVideoFalStrictSelection: + def test_nous_selection_routes_managed_even_with_fal_key(self): + from plugins.video_gen import fal as vf + + with patch("tools.tool_backend_helpers.read_selection", return_value="nous"), \ + patch("tools.tool_backend_helpers.fal_key_is_configured", return_value=True), \ + patch("tools.managed_tool_gateway.resolve_managed_tool_gateway", return_value=MANAGED): + assert vf._resolve_managed_fal_video_gateway() is MANAGED + + def test_fal_selection_missing_key_errors_without_managed_call(self): + from plugins.video_gen import fal as vf + + with patch("tools.tool_backend_helpers.read_selection", return_value="fal"), \ + patch("tools.tool_backend_helpers.fal_key_is_configured", return_value=False), \ + patch("tools.managed_tool_gateway.resolve_managed_tool_gateway") as gw: + with pytest.raises(ValueError) as exc: + vf._resolve_managed_fal_video_gateway() + gw.assert_not_called() + assert "video_gen is configured to use fal" in str(exc.value) + assert "FAL_KEY" in str(exc.value) + + def test_never_configured_autodetect_unchanged(self): + from plugins.video_gen import fal as vf + + with patch("tools.tool_backend_helpers.read_selection", return_value=None), \ + patch("tools.tool_backend_helpers.fal_key_is_configured", return_value=True): + assert vf._resolve_managed_fal_video_gateway() is None + + +# --------------------------------------------------------------------------- +# STT (OpenAI audio resolver — previously ignored the stored intent entirely) +# --------------------------------------------------------------------------- + + +class TestSttStrictSelection: + def test_nous_selection_beats_direct_openai_key(self): + from tools import transcription_tools as tt + + with patch.object(tt, "_load_stt_config", return_value={"openai": {"api_key": "sk-direct"}}), \ + patch("tools.tool_backend_helpers.read_selection", return_value="nous"), \ + patch.object(tt, "resolve_managed_tool_gateway", return_value=MANAGED): + api_key, base_url = tt._resolve_openai_audio_client_config() + assert api_key == "managed-token" + assert base_url.startswith("https://gateway.nousresearch.com") + + def test_vendor_selection_missing_key_errors_without_managed_call(self): + from tools import transcription_tools as tt + + with patch.object(tt, "_load_stt_config", return_value={}), \ + patch("tools.tool_backend_helpers.read_selection", return_value="openai"), \ + patch.object(tt, "resolve_openai_audio_api_key", return_value=""), \ + patch.object(tt, "resolve_managed_tool_gateway") as gw: + with pytest.raises(ValueError) as exc: + tt._resolve_openai_audio_client_config() + gw.assert_not_called() + assert "stt is configured to use openai" in str(exc.value) + assert "hermes tools" in str(exc.value) + + def test_never_configured_keeps_legacy_ladder(self): + from tools import transcription_tools as tt + + with patch.object(tt, "_load_stt_config", return_value={}), \ + patch("tools.tool_backend_helpers.read_selection", return_value=None), \ + patch.object(tt, "resolve_openai_audio_api_key", return_value="sk-env"): + api_key, base_url = tt._resolve_openai_audio_client_config() + assert api_key == "sk-env" + + +# --------------------------------------------------------------------------- +# Browser Use provider +# --------------------------------------------------------------------------- + + +class TestBrowserUseStrictSelection: + def _provider(self): + from plugins.browser.browser_use.provider import BrowserUseBrowserProvider + + return BrowserUseBrowserProvider() + + def test_nous_selection_routes_managed_even_with_direct_key(self): + provider = self._provider() + with patch("plugins.browser.browser_use.provider.get_secret", return_value="bu-key"), \ + patch("tools.tool_backend_helpers.read_selection", return_value="nous"), \ + patch("tools.managed_tool_gateway.resolve_managed_tool_gateway", return_value=MANAGED): + config = provider._get_config_or_none() + assert config["managed_mode"] is True + assert config["api_key"] == "managed-token" + + def test_vendor_selection_missing_key_errors_without_managed_call(self): + provider = self._provider() + with patch("plugins.browser.browser_use.provider.get_secret", return_value=""), \ + patch("tools.tool_backend_helpers.read_selection", return_value="browser-use"), \ + patch("tools.managed_tool_gateway.resolve_managed_tool_gateway") as gw: + with pytest.raises(ValueError) as exc: + provider._get_config() + gw.assert_not_called() + assert "browser is configured to use browser-use" in str(exc.value) + assert "BROWSER_USE_API_KEY" in str(exc.value) + + def test_never_configured_key_still_routes_direct(self): + provider = self._provider() + with patch("plugins.browser.browser_use.provider.get_secret", return_value="bu-key"), \ + patch("tools.tool_backend_helpers.read_selection", return_value=None): + config = provider._get_config_or_none() + assert config["managed_mode"] is False + assert config["api_key"] == "bu-key" + + +# --------------------------------------------------------------------------- +# Camofox: selection over env var +# --------------------------------------------------------------------------- + + +class TestCamofoxSelection: + def test_camofox_selection_activates_mode(self, monkeypatch): + from tools import browser_camofox as bc + + monkeypatch.delenv("BROWSER_CDP_URL", raising=False) + with patch.object(bc, "_config_cdp_url", return_value=""), \ + patch("tools.tool_backend_helpers.read_selection", return_value="camofox"): + assert bc.is_camofox_mode() is True + + def test_other_selection_beats_camofox_url_env(self, monkeypatch): + """CAMOFOX_URL is the ADDRESS, not the choice: an explicit different + browser selection wins.""" + from tools import browser_camofox as bc + + monkeypatch.delenv("BROWSER_CDP_URL", raising=False) + with patch.object(bc, "_config_cdp_url", return_value=""), \ + patch.object(bc, "get_camofox_url", return_value="http://localhost:9377"), \ + patch("tools.tool_backend_helpers.read_selection", return_value="local"): + assert bc.is_camofox_mode() is False + + def test_never_configured_env_url_still_activates(self, monkeypatch): + from tools import browser_camofox as bc + + monkeypatch.delenv("BROWSER_CDP_URL", raising=False) + with patch.object(bc, "_config_cdp_url", return_value=""), \ + patch.object(bc, "get_camofox_url", return_value="http://localhost:9377"), \ + patch("tools.tool_backend_helpers.read_selection", return_value=None): + assert bc.is_camofox_mode() is True + + +# --------------------------------------------------------------------------- +# tools_config writers: one provider string per row, no use_gateway writes +# --------------------------------------------------------------------------- + + +class TestWriteProviderConfig: + def test_managed_row_writes_nous_and_clears_legacy_flag(self): + from hermes_cli.tools_config import _write_provider_config + + config = {"tts": {"provider": "edge", "use_gateway": False}} + provider = {"name": "Nous Subscription", "tts_provider": "openai"} + _write_provider_config(provider, config, managed_feature="tts") + assert config["tts"]["provider"] == "nous" + assert "use_gateway" not in config["tts"] + + def test_byok_row_writes_vendor_and_clears_legacy_flag(self): + from hermes_cli.tools_config import _write_provider_config + + config = {"web": {"backend": "nous", "use_gateway": True}} + provider = {"name": "Tavily", "web_backend": "tavily"} + _write_provider_config(provider, config, managed_feature=None) + assert config["web"]["backend"] == "tavily" + assert "use_gateway" not in config["web"] + + def test_managed_image_row_persists_nous_provider(self): + from hermes_cli.tools_config import _write_provider_config + + config = {} + provider = {"name": "Nous Subscription", "imagegen_backend": "fal"} + _write_provider_config(provider, config, managed_feature="image_gen") + assert config["image_gen"]["provider"] == "nous" + assert "use_gateway" not in config["image_gen"] + + def test_plugin_injected_byok_row_clears_stale_use_gateway(self): + """Plugin-injected rows are not in TOOL_CATEGORIES' hardcoded + provider lists; the legacy clear-loop skipped them.""" + from hermes_cli.tools_config import _write_provider_config + + config = {"stt": {"provider": "nous", "use_gateway": True}} + provider = {"name": "Groq Whisper", "stt_provider": "groq"} + _write_provider_config(provider, config, managed_feature=None) + assert config["stt"]["provider"] == "groq" + assert "use_gateway" not in config["stt"] diff --git a/tests/tools/test_transcription_tools.py b/tests/tools/test_transcription_tools.py index a2b586a9cd..97177ff027 100644 --- a/tests/tools/test_transcription_tools.py +++ b/tests/tools/test_transcription_tools.py @@ -131,11 +131,25 @@ class TestExplicitProviderRespected: monkeypatch.delenv("GROQ_API_KEY", raising=False) with patch("tools.transcription_tools._HAS_FASTER_WHISPER", False), \ patch("tools.transcription_tools._has_local_command", return_value=False), \ + patch("tools.tool_backend_helpers.read_selection", return_value="local"), \ patch("tools.transcription_tools._HAS_OPENAI", True): from tools.transcription_tools import _get_provider result = _get_provider({"provider": "local"}) assert result == "none", f"Expected 'none' but got {result!r}" + def test_seeded_local_without_stored_selection_autodetects(self, monkeypatch): + """The DEFAULT_CONFIG-seeded stt.provider: local (no raw-config + selection) is treated as never-configured: autodetect runs instead of + hard-pinning to a missing local backend.""" + monkeypatch.setenv("GROQ_API_KEY", "gsk-test") + with patch("tools.transcription_tools._HAS_FASTER_WHISPER", False), \ + patch("tools.transcription_tools._has_local_command", return_value=False), \ + patch("tools.transcription_tools._try_lazy_install_stt", return_value=False), \ + patch("tools.tool_backend_helpers.read_selection", return_value=None), \ + patch("tools.transcription_tools._HAS_OPENAI", True): + from tools.transcription_tools import _get_provider + assert _get_provider({"provider": "local"}) == "groq" + def test_explicit_local_uses_local_command_fallback(self, monkeypatch): """Local-to-local_command fallback is fine — both are local.""" monkeypatch.setenv( diff --git a/tests/tools/test_tts_openai_config.py b/tests/tools/test_tts_openai_config.py index f489ab8560..8aacce7ac3 100644 --- a/tests/tools/test_tts_openai_config.py +++ b/tests/tools/test_tts_openai_config.py @@ -25,7 +25,7 @@ class TestResolveOpenaiAudioClientConfig: } with patch.object(tts_tool, "_load_tts_config", return_value=config), \ - patch.object(tts_tool, "prefers_gateway", return_value=False), \ + patch.object(tts_tool, "read_selection", return_value="openai"), \ patch.object(tts_tool, "resolve_openai_audio_api_key", return_value="env-key"), \ patch.object(tts_tool, "resolve_managed_tool_gateway", return_value=None): assert tts_tool._resolve_openai_audio_client_config() == ( @@ -38,7 +38,7 @@ class TestResolveOpenaiAudioClientConfig: config = {"openai": {"api_key": "cfg-key"}} with patch.object(tts_tool, "_load_tts_config", return_value=config), \ - patch.object(tts_tool, "prefers_gateway", return_value=False): + patch.object(tts_tool, "read_selection", return_value=None): assert tts_tool._resolve_openai_audio_client_config() == ( "cfg-key", tts_tool.DEFAULT_OPENAI_BASE_URL, @@ -46,7 +46,9 @@ class TestResolveOpenaiAudioClientConfig: ) - def test_use_gateway_overrides_config_credentials(self): + def test_nous_selection_overrides_config_credentials(self): + """A stored 'nous' selection (or legacy use_gateway: true) routes + managed even when direct credentials are present.""" config = {"openai": {"api_key": "cfg-key", "base_url": "http://localhost:4003/v1"}} managed = SimpleNamespace( nous_user_token="managed-token", @@ -54,7 +56,7 @@ class TestResolveOpenaiAudioClientConfig: ) with patch.object(tts_tool, "_load_tts_config", return_value=config), \ - patch.object(tts_tool, "prefers_gateway", return_value=True), \ + patch.object(tts_tool, "read_selection", return_value="nous"), \ patch.object(tts_tool, "resolve_openai_audio_api_key", return_value="env-key"), \ patch.object(tts_tool, "resolve_managed_tool_gateway", return_value=managed): assert tts_tool._resolve_openai_audio_client_config() == ( @@ -63,9 +65,35 @@ class TestResolveOpenaiAudioClientConfig: True, ) + def test_nous_selection_unentitled_raises_selection_error(self): + """Selected managed route + unavailable gateway = honest error naming + the selection, never a silent fall back to direct credentials.""" + config = {"openai": {"api_key": "cfg-key"}} + with patch.object(tts_tool, "_load_tts_config", return_value=config), \ + patch.object(tts_tool, "read_selection", return_value="nous"), \ + patch.object(tts_tool, "resolve_openai_audio_api_key", return_value="env-key"), \ + patch.object(tts_tool, "resolve_managed_tool_gateway", return_value=None): + with pytest.raises(ValueError) as exc: + tts_tool._resolve_openai_audio_client_config() + assert "nous" in str(exc.value) + assert "hermes tools" in str(exc.value) + + def test_vendor_selection_missing_key_raises_selection_error(self): + """A stored vendor selection with no credentials errors by name — + NO managed gateway call is attempted.""" + with patch.object(tts_tool, "_load_tts_config", return_value={"provider": "openai"}), \ + patch.object(tts_tool, "read_selection", return_value="openai"), \ + patch.object(tts_tool, "resolve_openai_audio_api_key", return_value=""), \ + patch.object(tts_tool, "resolve_managed_tool_gateway") as gateway_mock: + with pytest.raises(ValueError) as exc: + tts_tool._resolve_openai_audio_client_config() + gateway_mock.assert_not_called() + assert "openai" in str(exc.value) + assert "hermes tools" in str(exc.value) + def test_missing_config_and_env_raises_updated_error(self): with patch.object(tts_tool, "_load_tts_config", return_value={}), \ - patch.object(tts_tool, "prefers_gateway", return_value=False), \ + patch.object(tts_tool, "read_selection", return_value=None), \ patch.object(tts_tool, "resolve_openai_audio_api_key", return_value=""), \ patch.object(tts_tool, "resolve_managed_tool_gateway", return_value=None), \ patch.object(tts_tool, "managed_nous_tools_enabled", return_value=False): diff --git a/tests/tools/test_web_tools_config.py b/tests/tools/test_web_tools_config.py index 237037a22f..56cbaad0bd 100644 --- a/tests/tools/test_web_tools_config.py +++ b/tests/tools/test_web_tools_config.py @@ -222,12 +222,29 @@ class TestBackendSelection: patch("tools.web_tools._ddgs_package_importable", return_value=False): assert _get_backend() == "firecrawl" - def test_invalid_config_falls_through_to_fallback(self): - """web.backend=invalid → ignored, uses key-based fallback.""" + def test_invalid_config_is_returned_verbatim(self): + """Strict selection: web.backend=nonexistent is returned as-is so the + dispatch path raises the honest selection-naming error — never + silently rerouted through the credential ladder.""" from tools.web_tools import _get_backend with patch("tools.web_tools._load_web_config", return_value={"backend": "nonexistent"}), \ patch.dict(os.environ, {"PARALLEL_API_KEY": "test-key"}): - assert _get_backend() == "parallel" + assert _get_backend() == "nonexistent" + + def test_stored_backend_wins_over_other_credentials(self): + """Strict selection: a stored web.backend beats env keys for other + vendors — no availability probe, no credential override.""" + from tools.web_tools import _get_backend + with patch("tools.web_tools._load_web_config", return_value={"backend": "firecrawl"}), \ + patch.dict(os.environ, {"TAVILY_API_KEY": "tvly-test"}): + assert _get_backend() == "firecrawl" + + def test_nous_backend_maps_to_firecrawl(self): + """The managed 'nous' selection is serviced by the firecrawl + provider (whose client resolver routes managed).""" + from tools.web_tools import _get_backend + with patch("tools.web_tools._load_web_config", return_value={"backend": "nous"}): + assert _get_backend() == "firecrawl" def test_managed_gateway_does_not_preempt_explicit_tavily(self): """Regression: a Nous OAuth token (managed gateway "ready") must NOT From aa3c5e59d3e837580892bac7165a84facf7c7b4d Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 19 Aug 2026 15:49:32 -0700 Subject: [PATCH 10/24] fix(tools): honor raw stt.provider: local; finish _reconfigure_provider provider-string migration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two real gaps the CI-red sibling tests exposed: - read_selection() treated EVERY raw stt.provider: local as the legacy DEFAULT_CONFIG seed and reported no-selection — but the seed never reached config.yaml (save_config strips schema defaults), so a picker- or hand-written local pick was silently discarded and the autodetect ladder could route an explicit local user to cloud STT. A raw 'local' is now a genuine selection; the merged-view ambiguity note replaces the over-broad shim (mirror comment updated in nous_subscription._selected_provider and _get_provider). - _reconfigure_provider was half-migrated: the tts/stt/browser/web branches and the managed-category fallthrough still wrote use_gateway flags and vendor names for managed rows. They now write the single provider string ('nous' for managed rows) and pop the legacy key, matching _write_provider_config. --- hermes_cli/nous_subscription.py | 15 +++----- hermes_cli/tools_config.py | 36 +++++++++++++------ tests/tools/test_strict_provider_selection.py | 11 +++--- tools/tool_backend_helpers.py | 14 ++++---- tools/transcription_tools.py | 7 ++-- 5 files changed, 48 insertions(+), 35 deletions(-) diff --git a/hermes_cli/nous_subscription.py b/hermes_cli/nous_subscription.py index 882956fe31..01d10d12f4 100644 --- a/hermes_cli/nous_subscription.py +++ b/hermes_cli/nous_subscription.py @@ -475,16 +475,11 @@ def get_nous_subscription_features( image_selected = _selected_provider(image_gen_cfg) video_selected = _selected_provider(video_gen_cfg) - # Same seeded-value shim as tools.tool_backend_helpers.read_selection: - # legacy DEFAULT_CONFIG seeded ``stt.provider: local`` on every install, - # so that exact value with no picker-written use_gateway key is treated - # as never-configured. - if ( - stt_selected == "local" - and isinstance(stt_cfg, dict) - and "use_gateway" not in stt_cfg - ): - stt_selected = None + # Lockstep with tools.tool_backend_helpers.read_selection: these are + # merged-config sections, so the legacy DEFAULT_CONFIG-seeded + # ``stt.provider: local`` COULD appear here without a user pick on old + # versions. Current DEFAULT_CONFIG no longer seeds it, so a merged + # ``local`` implies the raw file holds it — a genuine selection. # Managed selection flags (replace the legacy use_gateway reads — # use_gateway is now interpreted only inside _selected_provider). diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index a724b4888f..cea0d5f792 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -5084,22 +5084,33 @@ def _reconfigure_provider( ) return + # Selection model (mirrors _write_provider_config): every row writes ONE + # provider string per category — "nous" for managed rows, the vendor name + # for BYOK rows — and drops any legacy use_gateway key so the read-time + # shim (use_gateway: true ⇒ nous) cannot override the fresh pick. if provider.get("tts_provider"): tts_cfg = config.setdefault("tts", {}) - tts_cfg["provider"] = provider["tts_provider"] - tts_cfg["use_gateway"] = bool(managed_feature) + tts_cfg["provider"] = ( + NOUS_MANAGED_PROVIDER if managed_feature else provider["tts_provider"] + ) + tts_cfg.pop("use_gateway", None) _print_success(f" TTS provider set to: {provider['tts_provider']}") if provider.get("stt_provider"): stt_cfg = config.setdefault("stt", {}) - stt_cfg["provider"] = provider["stt_provider"] - stt_cfg["use_gateway"] = bool(managed_feature) + stt_cfg["provider"] = ( + NOUS_MANAGED_PROVIDER if managed_feature else provider["stt_provider"] + ) + stt_cfg.pop("use_gateway", None) _print_success(f" STT provider set to: {provider['stt_provider']}") if "browser_provider" in provider: bp = provider["browser_provider"] browser_cfg = config.setdefault("browser", {}) - if bp == "local": + if managed_feature: + browser_cfg["cloud_provider"] = NOUS_MANAGED_PROVIDER + _print_success(f" Browser cloud provider set to: {bp or 'nous'}") + elif bp == "local": browser_cfg["cloud_provider"] = "local" _print_success(" Browser set to local mode") elif bp: @@ -5107,7 +5118,7 @@ def _reconfigure_provider( _print_success(f" Browser cloud provider set to: {bp}") # Browser Use mode (browser.backend) composes with the provider — # switching providers keeps the driver choice intact. - browser_cfg["use_gateway"] = bool(managed_feature) + browser_cfg.pop("use_gateway", None) if provider.get("browser_backend"): browser_cfg = config.setdefault("browser", {}) @@ -5117,8 +5128,10 @@ def _reconfigure_provider( # Set web search backend in config if applicable if provider.get("web_backend"): web_cfg = config.setdefault("web", {}) - web_cfg["backend"] = provider["web_backend"] - web_cfg["use_gateway"] = bool(managed_feature) + web_cfg["backend"] = ( + NOUS_MANAGED_PROVIDER if managed_feature else provider["web_backend"] + ) + web_cfg.pop("use_gateway", None) _print_success(f" Web backend set to: {provider['web_backend']}") # Set computer_use backend in config if applicable @@ -5132,13 +5145,14 @@ def _reconfigure_provider( if not isinstance(section, dict): section = {} config[managed_feature] = section - section["use_gateway"] = True + section["provider"] = NOUS_MANAGED_PROVIDER + section.pop("use_gateway", None) elif not managed_feature: for cat_key, cat in TOOL_CATEGORIES.items(): if provider in cat.get("providers", []): section = config.get(cat_key) - if isinstance(section, dict) and section.get("use_gateway"): - section["use_gateway"] = False + if isinstance(section, dict): + section.pop("use_gateway", None) break if not env_vars: diff --git a/tests/tools/test_strict_provider_selection.py b/tests/tools/test_strict_provider_selection.py index cbb158e47a..aed8ae9e32 100644 --- a/tests/tools/test_strict_provider_selection.py +++ b/tests/tools/test_strict_provider_selection.py @@ -66,11 +66,14 @@ class TestReadSelection: with self._with_raw({"web": {"backend": ""}}): assert tbh.read_selection("web") is None - def test_seeded_stt_local_is_no_selection(self): - """Legacy DEFAULT_CONFIG seeded stt.provider: local on every - install; that value alone must be treated as never-configured.""" + def test_raw_stt_local_is_a_selection(self): + """A raw config.yaml ``stt.provider: local`` is a genuine pick: the + DEFAULT_CONFIG seed never reached disk (save_config strips schema + defaults), and the current picker's Local Whisper row writes exactly + this shape (provider only, legacy use_gateway popped). Treating it + as no-selection would silently discard the user's choice.""" with self._with_raw({"stt": {"provider": "local"}}): - assert tbh.read_selection("stt") is None + assert tbh.read_selection("stt") == "local" def test_stt_local_with_use_gateway_key_is_a_selection(self): """A picker-written stt section (use_gateway key present) means diff --git a/tools/tool_backend_helpers.py b/tools/tool_backend_helpers.py index f3c23c9c85..0096c72fdf 100644 --- a/tools/tool_backend_helpers.py +++ b/tools/tool_backend_helpers.py @@ -361,13 +361,13 @@ def read_selection(section: str) -> str | None: if "use_gateway" in raw and is_truthy_value(raw.get("use_gateway"), default=False): return NOUS_MANAGED_PROVIDER - # Migration shim: DEFAULT_CONFIG historically seeded ``stt.provider: - # local`` on every install, so that exact value with no picker-written - # use_gateway key is ambiguous. Treat it as never-configured — the - # autodetect ladder prefers local first anyway, and hard-pinning would - # error every seeded install that lacks faster-whisper. - if section == "stt" and name == "local" and "use_gateway" not in raw: - return None + # NOTE on the legacy DEFAULT_CONFIG ``stt.provider: local`` seed: it never + # reached the raw config.yaml (``save_config`` strips schema defaults), + # and the old picker's Local Whisper row always wrote ``use_gateway: + # False`` beside it. A raw ``local`` here therefore IS a user selection — + # hand-written or picker-written — and is honored like any other vendor + # name. The seeded-value ambiguity only exists in DEFAULT_CONFIG-merged + # views, which this function never reads. if name: return name diff --git a/tools/transcription_tools.py b/tools/transcription_tools.py index 77f4cca296..9a38929d84 100644 --- a/tools/transcription_tools.py +++ b/tools/transcription_tools.py @@ -1034,9 +1034,10 @@ def _get_provider(stt_config: dict) -> str: if explicit and provider == "local": # Legacy DEFAULT_CONFIG seeded ``stt.provider: local`` on every # install, so a merged-config "local" is not proof of a user pick. - # ``read_selection`` reads the raw config.yaml and applies the - # seeded-value migration shim; when the raw file holds no stt - # selection, take the autodetect branch (which prefers local first + # ``read_selection`` reads the raw config.yaml: when the raw file + # holds an stt selection (picker- or hand-written ``local``) it is + # honored; when the merged "local" came only from a legacy default + # merge, take the autodetect branch (which prefers local first # anyway, so a genuine local user is unaffected when it's available). try: from tools.tool_backend_helpers import read_selection From 19113b34d1b52dfdebe8b3fd46349b4f74c0731f Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 19 Aug 2026 15:49:32 -0700 Subject: [PATCH 11/24] test(tools): repin selector/picker tests to the provider-string contract Update the sibling tests that pinned the old use_gateway-writing contract: image/video selector and reconfigure rows now assert the single provider string ('nous' managed / 'fal' BYOK) plus legacy-key popping, the stt/video picker writes drop the use_gateway expectation, the web_server managed-browser select asserts the persisted 'nous' cloud_provider, and explicit-local STT pins no-cloud-fallback against a stored raw-config selection. --- .../test_imagegen_managed_gateway.py | 84 +++++++++++-------- tests/hermes_cli/test_stt_picker.py | 5 +- tests/hermes_cli/test_video_gen_picker.py | 3 +- tests/hermes_cli/test_web_server.py | 5 +- tests/tools/test_transcription.py | 3 +- 5 files changed, 59 insertions(+), 41 deletions(-) diff --git a/tests/hermes_cli/test_imagegen_managed_gateway.py b/tests/hermes_cli/test_imagegen_managed_gateway.py index 7886439842..7daf364112 100644 --- a/tests/hermes_cli/test_imagegen_managed_gateway.py +++ b/tests/hermes_cli/test_imagegen_managed_gateway.py @@ -1,15 +1,18 @@ -"""Regression tests for image_gen use_gateway persistence (managed FAL clobber). +"""Regression tests for image_gen provider persistence (managed FAL clobber). -Bug: ``_select_plugin_image_gen_provider`` hardcoded -``image_gen.use_gateway = False``. When a user picked FAL through the -Nous-subscription managed flow, ``_write_provider_config`` first set -``use_gateway = True`` — then the image selector ran and clobbered it back -to False, silently routing every generation through the user's personal -FAL_KEY instead of the Nous Tool Gateway (real incident: personal key -drained to zero while the subscription sat unused). +Historical bug: ``_select_plugin_image_gen_provider`` hardcoded the direct +(non-managed) routing. When a user picked FAL through the Nous-subscription +managed flow, the managed write landed first — then the image selector ran +and clobbered it back to direct, silently routing every generation through +the user's personal FAL_KEY instead of the Nous Tool Gateway (real incident: +personal key drained to zero while the subscription sat unused). -The video twin (``_select_plugin_video_gen_provider``) already accepted a -``use_gateway`` kwarg; these tests pin the image path to the same contract. +Current contract (strict provider-string selection): each picker row writes +exactly ONE provider string per category — ``image_gen.provider: nous`` for +the managed "Nous Subscription" row, ``image_gen.provider: fal`` for the +BYOK FAL row — and any legacy ``use_gateway`` key is popped so the +read-time shim (use_gateway: true ⇒ nous) cannot override the fresh pick. +The video twin (``_select_plugin_video_gen_provider``) shares the contract. """ from hermes_cli.tools_config import ( @@ -28,43 +31,50 @@ def _quiet(monkeypatch): monkeypatch.setattr(tc, "_configure_videogen_model_for_plugin", lambda *a, **k: None) -def test_image_gen_selector_preserves_managed_gateway_flag(monkeypatch): - """Managed pick: use_gateway=True must survive the selector.""" +def test_image_gen_selector_preserves_managed_selection(monkeypatch): + """Managed pick: the 'nous' provider string must survive the selector.""" _quiet(monkeypatch) config = {} - # The managed flow first writes the managed flag... + # The managed flow first persists the managed selection... _write_provider_config( {"image_gen_plugin_name": "fal"}, config, managed_feature="image_gen" ) - assert config["image_gen"]["use_gateway"] is True + assert config["image_gen"]["provider"] == "nous" + assert "use_gateway" not in config["image_gen"] - # ...then the selector runs; passing the managed flag must NOT clobber it. + # ...then the selector runs; the managed kwarg must NOT clobber it + # back onto the vendor name (direct-key routing). _select_plugin_image_gen_provider("fal", config, use_gateway=True) - assert config["image_gen"]["provider"] == "fal" - assert config["image_gen"]["use_gateway"] is True + assert config["image_gen"]["provider"] == "nous" + assert "use_gateway" not in config["image_gen"] -def test_image_gen_selector_direct_key_pick_clears_gateway(monkeypatch): - """Non-managed pick keeps the historical default: direct key, no gateway.""" +def test_image_gen_selector_direct_key_pick_writes_vendor(monkeypatch): + """Non-managed pick writes the vendor name and pops the legacy flag.""" _quiet(monkeypatch) config = {"image_gen": {"use_gateway": True}} _select_plugin_image_gen_provider("fal", config) assert config["image_gen"]["provider"] == "fal" - assert config["image_gen"]["use_gateway"] is False + assert "use_gateway" not in config["image_gen"] -def test_image_and_video_selectors_share_the_gateway_contract(monkeypatch): +def test_image_and_video_selectors_share_the_selection_contract(monkeypatch): """The two selectors are twins: same kwarg, same persistence behavior.""" _quiet(monkeypatch) - for use_gateway in (True, False): - config = {} + for use_gateway, expected in ((True, "nous"), (False, "fal")): + config = { + "image_gen": {"use_gateway": not use_gateway}, + "video_gen": {"use_gateway": not use_gateway}, + } _select_plugin_image_gen_provider("fal", config, use_gateway=use_gateway) _select_plugin_video_gen_provider("fal", config, use_gateway=use_gateway) - assert config["image_gen"]["use_gateway"] is use_gateway - assert config["video_gen"]["use_gateway"] is use_gateway + assert config["image_gen"]["provider"] == expected + assert config["video_gen"]["provider"] == expected + assert "use_gateway" not in config["image_gen"] + assert "use_gateway" not in config["video_gen"] def _quiet_reconfigure(monkeypatch): @@ -82,11 +92,12 @@ def _quiet_reconfigure(monkeypatch): monkeypatch.setattr(ns, "ensure_nous_portal_access", lambda **k: True) -def test_reconfigure_managed_fal_row_keeps_gateway_flag(monkeypatch): +def test_reconfigure_managed_fal_row_keeps_managed_selection(monkeypatch): """The sibling bug of fe63353cb: the legacy-backend model-pick step in - _reconfigure_provider hardcoded use_gateway=False AFTER the managed - branch wrote True — a Nous Subscription user re-entering the picker to - change models was silently flipped onto their personal FAL_KEY.""" + _reconfigure_provider hardcoded the direct selection AFTER the managed + branch wrote the managed one — a Nous Subscription user re-entering the + picker to change models was silently flipped onto their personal + FAL_KEY.""" _quiet_reconfigure(monkeypatch) import hermes_cli.tools_config as tc @@ -98,17 +109,17 @@ def test_reconfigure_managed_fal_row_keeps_gateway_flag(monkeypatch): "override_env_vars": ["FAL_KEY"], "imagegen_backend": "fal", } - config = {"image_gen": {"model": "fal-ai/gpt-image-2"}} + config = {"image_gen": {"model": "fal-ai/gpt-image-2", "use_gateway": True}} tc._reconfigure_provider(managed_row, config) - assert config["image_gen"]["provider"] == "fal" - assert config["image_gen"]["use_gateway"] is True + assert config["image_gen"]["provider"] == "nous" + assert "use_gateway" not in config["image_gen"] -def test_reconfigure_direct_fal_row_clears_gateway_flag(monkeypatch): - """Direct-key FAL reconfig must still clear the flag (historical - behavior for genuinely non-managed picks).""" +def test_reconfigure_direct_fal_row_writes_vendor_selection(monkeypatch): + """Direct-key FAL reconfig writes the vendor name and pops any stale + legacy use_gateway key so the read-time shim can't resurrect 'nous'.""" _quiet_reconfigure(monkeypatch) import hermes_cli.tools_config as tc @@ -121,4 +132,5 @@ def test_reconfigure_direct_fal_row_clears_gateway_flag(monkeypatch): tc._reconfigure_provider(direct_row, config) - assert config["image_gen"]["use_gateway"] is False + assert config["image_gen"]["provider"] == "fal" + assert "use_gateway" not in config["image_gen"] diff --git a/tests/hermes_cli/test_stt_picker.py b/tests/hermes_cli/test_stt_picker.py index 198362c450..0fec1c813d 100644 --- a/tests/hermes_cli/test_stt_picker.py +++ b/tests/hermes_cli/test_stt_picker.py @@ -56,11 +56,12 @@ class TestSttCategory: class TestConfigWrites: def test_write_provider_config_sets_stt_provider(self): - config = {} + config = {"stt": {"use_gateway": True}} prov = _stt_provider_named("Groq") _write_provider_config(prov, config, managed_feature=None) assert config["stt"]["provider"] == "groq" - assert config["stt"]["use_gateway"] is False + # Legacy key is popped so the read-time shim can't override the pick. + assert "use_gateway" not in config["stt"] def test_apply_provider_selection_stt(self): diff --git a/tests/hermes_cli/test_video_gen_picker.py b/tests/hermes_cli/test_video_gen_picker.py index 8c2ebeb17e..c740ef3942 100644 --- a/tests/hermes_cli/test_video_gen_picker.py +++ b/tests/hermes_cli/test_video_gen_picker.py @@ -113,7 +113,8 @@ class TestReconfigureWritesProvider: assert config["video_gen"]["provider"] == "xai_fake" assert config["video_gen"]["model"] == "xai_fake-video-v1" - assert config["video_gen"]["use_gateway"] is False + # Non-managed pick: no legacy use_gateway key is written. + assert "use_gateway" not in config["video_gen"] class TestPluginVideoProvidersRow: diff --git a/tests/hermes_cli/test_web_server.py b/tests/hermes_cli/test_web_server.py index e8487de5fb..5c1fb1e985 100644 --- a/tests/hermes_cli/test_web_server.py +++ b/tests/hermes_cli/test_web_server.py @@ -2671,9 +2671,12 @@ class TestNewEndpoints: assert data["needs_nous_auth"] is True assert data["feature"] == "browser" # The selection is still persisted — activation is what's gated. + # Managed rows store the single 'nous' provider string (the runtime + # maps it to the Browser Use cloud through the Nous Tool Gateway). from hermes_cli.config import load_config cfg = load_config() - assert cfg["browser"]["cloud_provider"] == "browser-use" + assert cfg["browser"]["cloud_provider"] == "nous" + assert "use_gateway" not in cfg["browser"] # -- Web capability split (search vs extract backends) ------------------ diff --git a/tests/tools/test_transcription.py b/tests/tools/test_transcription.py index 2d8b8b04e9..a6a81b0149 100644 --- a/tests/tools/test_transcription.py +++ b/tests/tools/test_transcription.py @@ -43,7 +43,8 @@ class TestGetProvider: monkeypatch.delenv("GROQ_API_KEY", raising=False) with patch("tools.transcription_tools._HAS_FASTER_WHISPER", False), \ patch("tools.transcription_tools._HAS_OPENAI", True), \ - patch("tools.transcription_tools._has_local_command", return_value=False): + patch("tools.transcription_tools._has_local_command", return_value=False), \ + patch("tools.tool_backend_helpers.read_selection", return_value="local"): from tools.transcription_tools import _get_provider assert _get_provider({"provider": "local"}) == "none" From 6ad587236fd3298f414e2b868be7a7a6c1e48c39 Mon Sep 17 00:00:00 2001 From: hukla <129692708+huklaa@users.noreply.github.com> Date: Wed, 19 Aug 2026 11:59:27 +0300 Subject: [PATCH 12/24] fix(sdk): return registered connection list --- apps/desktop/src/sdk/index.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/desktop/src/sdk/index.ts b/apps/desktop/src/sdk/index.ts index ffafa03f0a..200c7e7ddf 100644 --- a/apps/desktop/src/sdk/index.ts +++ b/apps/desktop/src/sdk/index.ts @@ -464,7 +464,9 @@ export const host = { throw new Error('This Desktop build has no connection registry. Update Hermes Desktop.') } - return bridge.list() + const registry = await bridge.list() + + return Array.isArray(registry) ? registry : Array.isArray(registry?.connections) ? registry.connections : [] }, /** The union agent roster across every registered connection: one row per From c825be4c770495be56b9830ad2f0db8c91198dc5 Mon Sep 17 00:00:00 2001 From: hukla <129692708+huklaa@users.noreply.github.com> Date: Wed, 19 Aug 2026 14:05:31 +0300 Subject: [PATCH 13/24] test(sdk): cover connection registry list contract --- apps/desktop/src/sdk/index.test.ts | 48 ++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/apps/desktop/src/sdk/index.test.ts b/apps/desktop/src/sdk/index.test.ts index 83c68d9a7b..1b3f004e8b 100644 --- a/apps/desktop/src/sdk/index.test.ts +++ b/apps/desktop/src/sdk/index.test.ts @@ -107,3 +107,51 @@ describe('host.state turn flags', () => { $sessionTiles.set([]) }) }) + +describe('host.connections', () => { + const desktopWindow = window as unknown as { hermesDesktop?: Window['hermesDesktop'] } + const originalDesktop = desktopWindow.hermesDesktop + + afterEach(() => { + desktopWindow.hermesDesktop = originalDesktop + }) + + const stubList = (value: unknown) => { + desktopWindow.hermesDesktop = { + ...originalDesktop, + connections: { + ...originalDesktop?.connections, + list: async () => value + } + } as Window['hermesDesktop'] + } + + it('returns connection rows from the desktop registry envelope (#89823)', async () => { + stubList({ + connections: [ + { id: 'local', kind: 'local', label: 'This device' }, + { id: 'remote', kind: 'remote', label: 'Remote gateway' } + ], + primary: 'local', + secureTokenStorage: true, + version: 2 + }) + + const connections = await host.connections() + + expect(Array.isArray(connections)).toBe(true) + expect(connections.map(connection => connection.id)).toEqual(['local', 'remote']) + }) + + it('falls back to an empty list when the registry has no connection rows', async () => { + stubList({ primary: '', secureTokenStorage: true, version: 1 }) + + await expect(host.connections()).resolves.toEqual([]) + }) + + it('rejects on desktop builds without the connection registry', async () => { + desktopWindow.hermesDesktop = undefined + + await expect(host.connections()).rejects.toThrow('This Desktop build has no connection registry') + }) +}) From aa2cec721faccd15a6f0156a82041b0938cac062 Mon Sep 17 00:00:00 2001 From: hukla <129692708+huklaa@users.noreply.github.com> Date: Wed, 19 Aug 2026 14:40:52 +0300 Subject: [PATCH 14/24] test(sdk): cover registry primary connection mapping --- apps/desktop/src/sdk/index.test.ts | 60 +++++++++++++++++++----------- 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/apps/desktop/src/sdk/index.test.ts b/apps/desktop/src/sdk/index.test.ts index 1b3f004e8b..f7f879d4e4 100644 --- a/apps/desktop/src/sdk/index.test.ts +++ b/apps/desktop/src/sdk/index.test.ts @@ -112,44 +112,62 @@ describe('host.connections', () => { const desktopWindow = window as unknown as { hermesDesktop?: Window['hermesDesktop'] } const originalDesktop = desktopWindow.hermesDesktop + const connection = (id: string, label: string) => ({ + id, + kind: 'remote' as const, + label, + tokenPreview: null, + tokenSet: true, + url: `https://${id}.example` + }) + + const stubBridge = (list: () => Promise) => { + desktopWindow.hermesDesktop = { + ...originalDesktop, + connections: { list } + } as unknown as Window['hermesDesktop'] + } + afterEach(() => { desktopWindow.hermesDesktop = originalDesktop }) - const stubList = (value: unknown) => { - desktopWindow.hermesDesktop = { - ...originalDesktop, - connections: { - ...originalDesktop?.connections, - list: async () => value - } - } as Window['hermesDesktop'] - } - - it('returns connection rows from the desktop registry envelope (#89823)', async () => { - stubList({ - connections: [ - { id: 'local', kind: 'local', label: 'This device' }, - { id: 'remote', kind: 'remote', label: 'Remote gateway' } - ], + it('returns the registry rows, not the envelope that carries them (#89823)', async () => { + stubBridge(async () => ({ + connections: [connection('local', 'This Mac'), connection('homelab', 'Homelab')], primary: 'local', secureTokenStorage: true, version: 2 - }) + })) const connections = await host.connections() expect(Array.isArray(connections)).toBe(true) - expect(connections.map(connection => connection.id)).toEqual(['local', 'remote']) + expect(connections.map(entry => entry.id)).toEqual(['local', 'homelab']) + expect(connections[1]).toMatchObject({ kind: 'remote', label: 'Homelab', url: 'https://homelab.example' }) }) - it('falls back to an empty list when the registry has no connection rows', async () => { - stubList({ primary: '', secureTokenStorage: true, version: 1 }) + it('folds the envelope-level primary id down onto the row that owns it', async () => { + stubBridge(async () => ({ + connections: [connection('local', 'This Mac'), connection('homelab', 'Homelab')], + primary: 'homelab', + secureTokenStorage: true, + version: 2 + })) + + expect((await host.connections()).map(entry => [entry.id, entry.primary])).toEqual([ + ['local', false], + ['homelab', true] + ]) + }) + + it('reads as a single-source desktop when the payload carries no rows', async () => { + stubBridge(async () => ({ primary: '', secureTokenStorage: true, version: 1 })) await expect(host.connections()).resolves.toEqual([]) }) - it('rejects on desktop builds without the connection registry', async () => { + it('still rejects on a Desktop build without the connection registry', async () => { desktopWindow.hermesDesktop = undefined await expect(host.connections()).rejects.toThrow('This Desktop build has no connection registry') From b40d2019e8728fcb945d216a00e84ff1292bd79b Mon Sep 17 00:00:00 2001 From: hukla <129692708+huklaa@users.noreply.github.com> Date: Wed, 19 Aug 2026 14:42:37 +0300 Subject: [PATCH 15/24] fix(sdk): preserve primary in registered connections --- apps/desktop/src/sdk/index.ts | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/apps/desktop/src/sdk/index.ts b/apps/desktop/src/sdk/index.ts index 200c7e7ddf..6f502c51f1 100644 --- a/apps/desktop/src/sdk/index.ts +++ b/apps/desktop/src/sdk/index.ts @@ -464,9 +464,10 @@ export const host = { throw new Error('This Desktop build has no connection registry. Update Hermes Desktop.') } - const registry = await bridge.list() + const registryPayload = await bridge.list() + const rows = Array.isArray(registryPayload?.connections) ? registryPayload.connections : [] - return Array.isArray(registry) ? registry : Array.isArray(registry?.connections) ? registry.connections : [] + return rows.map(connection => ({ ...connection, primary: connection.id === registryPayload.primary })) }, /** The union agent roster across every registered connection: one row per @@ -498,6 +499,22 @@ export const host = { ensureAgent: async (connectionId: null | string, profile: string): Promise => ensureGatewayAgent(connectionId, (profile ?? '').trim() || 'default'), + /** Open a stored session that belongs to an agent on ANY registered source. + * The connection id + profile are the durable route; the store handles + * dialing / source activation, then the regular session-open path owns + * navigation + hydration. */ + openAgentSession: async ( + connectionId: null | string, + profile: string, + storedSessionId: string, + options: Omit = {} + ): Promise => { + await ensureGatewayAgent(connectionId, (profile ?? '').trim() || 'default') + + return host.openSession(storedSessionId, { ...options, profile }) + }, + + /** Open a stored session — optionally pre-activating its profile first. */ openSession: async (storedSessionId: string, options: PluginOpenSessionOptions = {}): Promise => { const generation = ++openSessionGeneration const profile = (options.profile ?? '').trim() @@ -949,4 +966,4 @@ export { Blobatar } from 'blobatar/react' export { atom, computed } from 'nanostores' /** Markdown renderer (same pipeline core chat surfaces use) so plugins render * message text as a preview instead of raw Markdown source. */ -export { Streamdown } from 'streamdown' +export { Streamdown } from 'streamdown' \ No newline at end of file From a46fe01251b87e299707033a495dd63e95fa7c63 Mon Sep 17 00:00:00 2001 From: hukla Date: Wed, 19 Aug 2026 22:48:22 +0300 Subject: [PATCH 16/24] chore(sdk): remove unrelated session helper from #89893 --- apps/desktop/src/sdk/index.ts | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/apps/desktop/src/sdk/index.ts b/apps/desktop/src/sdk/index.ts index 6f502c51f1..88019d6ef7 100644 --- a/apps/desktop/src/sdk/index.ts +++ b/apps/desktop/src/sdk/index.ts @@ -499,21 +499,6 @@ export const host = { ensureAgent: async (connectionId: null | string, profile: string): Promise => ensureGatewayAgent(connectionId, (profile ?? '').trim() || 'default'), - /** Open a stored session that belongs to an agent on ANY registered source. - * The connection id + profile are the durable route; the store handles - * dialing / source activation, then the regular session-open path owns - * navigation + hydration. */ - openAgentSession: async ( - connectionId: null | string, - profile: string, - storedSessionId: string, - options: Omit = {} - ): Promise => { - await ensureGatewayAgent(connectionId, (profile ?? '').trim() || 'default') - - return host.openSession(storedSessionId, { ...options, profile }) - }, - /** Open a stored session — optionally pre-activating its profile first. */ openSession: async (storedSessionId: string, options: PluginOpenSessionOptions = {}): Promise => { const generation = ++openSessionGeneration @@ -966,4 +951,4 @@ export { Blobatar } from 'blobatar/react' export { atom, computed } from 'nanostores' /** Markdown renderer (same pipeline core chat surfaces use) so plugins render * message text as a preview instead of raw Markdown source. */ -export { Streamdown } from 'streamdown' \ No newline at end of file +export { Streamdown } from 'streamdown' From 271e49a8ffbaf6bf006013c2247546bccf3997a8 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 19 Aug 2026 15:14:01 -0700 Subject: [PATCH 17/24] fix(bot-mode): accept both host.connections() shapes in the Create-on picker normalize The SDK now returns the registry rows per its documented contract (salvaged #89893), while desktops predating the SDK unwrap resolve the raw registry envelope. The plugin normalize accepts both, so the picker works across the transition; regression test updated to pin the dual-shape normalize. --- apps/desktop/src/plugins/hermes-bots/plugin.js | 5 ++++- .../tests/cross-connection-bots.test.mjs | 18 ++++++++---------- contributors/emails/hukla25@gmail.com | 1 + 3 files changed, 13 insertions(+), 11 deletions(-) create mode 100644 contributors/emails/hukla25@gmail.com diff --git a/apps/desktop/src/plugins/hermes-bots/plugin.js b/apps/desktop/src/plugins/hermes-bots/plugin.js index ce66a17929..df073db64b 100644 --- a/apps/desktop/src/plugins/hermes-bots/plugin.js +++ b/apps/desktop/src/plugins/hermes-bots/plugin.js @@ -6189,7 +6189,10 @@ function CreateAgentDialog({ open, onClose, roster }) { host .connections() - .then(value => setConnections(Array.isArray(value?.connections) ? value.connections : [])) + // host.connections() returns the registry ROWS on current SDKs, but the + // envelope object ({version, primary, connections: [...]}) on desktops + // that predate the SDK-side unwrap — accept both shapes. + .then(value => setConnections(Array.isArray(value) ? value : Array.isArray(value?.connections) ? value.connections : [])) .catch(() => setConnections([])) }, [open, connections]) diff --git a/apps/desktop/src/plugins/hermes-bots/tests/cross-connection-bots.test.mjs b/apps/desktop/src/plugins/hermes-bots/tests/cross-connection-bots.test.mjs index 5092884e08..5c7573d508 100644 --- a/apps/desktop/src/plugins/hermes-bots/tests/cross-connection-bots.test.mjs +++ b/apps/desktop/src/plugins/hermes-bots/tests/cross-connection-bots.test.mjs @@ -163,17 +163,15 @@ test('source contract: group chat turns route through requestForBot on the membe assert.match(pluginSource, /members: Array\.isArray\(room\.members\) \? room\.members : \[\]/) }) -test('regression: host.connections() registry object is unwrapped before the picker gate', () => { - // host.connections() resolves the IPC handler hermes:connections:list, which - // returns the registry OBJECT ({version, primary, connections: [...]}) — not - // a bare array. The picker gate (Array.isArray(connections) && length > 1) - // never fired, so the "Create on" picker stayed hidden on multi-connection - // desktops. The plugin must unwrap .connections before storing. +test('regression: host.connections() result is normalized for BOTH SDK shapes before the picker gate', () => { + // Current SDKs return the registry ROWS from host.connections() (the + // documented contract); desktops that predate the SDK-side unwrap resolve + // the raw IPC payload — the registry OBJECT ({version, primary, + // connections: [...]}). The picker gate (Array.isArray(connections) && + // length > 1) needs rows either way, so the plugin must accept both + // shapes. Pinning one shape only reopens #89823 on the other. assert.match( pluginSource, - /setConnections\(Array\.isArray\(value\?\.connections\) \? value\.connections : \[\]\)/ + /setConnections\(Array\.isArray\(value\) \? value : Array\.isArray\(value\?\.connections\) \? value\.connections : \[\]\)/ ) - // The built-in Connections UI consumes the registry object, so the IPC - // handler contract must NOT change to a bare array. - assert.doesNotMatch(pluginSource, /setConnections\(Array\.isArray\(value\) \? value : \[\]\)/) }) diff --git a/contributors/emails/hukla25@gmail.com b/contributors/emails/hukla25@gmail.com new file mode 100644 index 0000000000..3655bceda9 --- /dev/null +++ b/contributors/emails/hukla25@gmail.com @@ -0,0 +1 @@ +huklaa From d762ed9b3c79326cee1cfcd357cdc5aabbc523bf Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 19 Aug 2026 15:38:28 -0700 Subject: [PATCH 18/24] feat: execution-discipline guidance now reaches all tool-capable models (config model.execution_guidance) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Un-fences OPENAI_MODEL_EXECUTION_GUIDANCE from the gpt/codex/grok substring check and gives it its own injection gate, independent of tool_use_enforcement, controlled by config.yaml `agent.execution_guidance` (auto/true/false/list — same semantics as tool_use_enforcement). The "auto" list (EXECUTION_GUIDANCE_MODELS) now also covers deepseek, kimi, qwen, glm, minimax, mimo, and mistral. Composio agentic-eval traces showed Hermes+DeepSeek/Kimi failing where competitors passed: financial math done in prose, no read-back after external writes, malformed identifiers "repaired", completeness claimed despite count mismatches. The discipline block existed but those models never received it. The block is extended with compact clauses distilled from that analysis: - external-write read-back (tool-call success is not task success; internal file edits already confirmed by the tool are not re-verified) - count reconciliation (declared totals/has_more are hard assertions) - literal preservation (never normalize identifiers that fail a stated format; lookup success does not validate a malformed token) - retry-differently (empty/partial/suspiciously narrow results get a broader retry before concluding) - completion gated on verification (done = every named acceptance criterion verified, never a plausible subset) The todo tool description now encourages enumeration-as-checklist for "all N items" tasks and gates completed status on verified work, never intent. Guidance is chosen once at session start keyed on model name, so the system prompt stays byte-stable for the life of a conversation. Supersedes/absorbs prior contributor proposals: #20588, #35087, #41874 (MiMo), #53847 (GLM tool-calls-as-text stall). Co-authored-by: Mat-London <56627804+Mat-London@users.noreply.github.com> Co-authored-by: intelac <8803887+intelac@users.noreply.github.com> Co-authored-by: 6ylqq <51219463+6ylqq@users.noreply.github.com> Co-authored-by: tauros1983 <267660491+tauros1983@users.noreply.github.com> --- agent/agent_init.py | 6 ++ agent/prompt_builder.py | 54 +++++++++++++++- agent/system_prompt.py | 38 +++++++++-- hermes_cli/config_defaults.py | 9 +++ hermes_cli/dump.py | 1 + tests/agent/test_prompt_builder.py | 45 +++++++++++++ tests/agent/test_system_prompt.py | 81 ++++++++++++++++++++++++ tests/run_agent/test_run_agent.py | 66 +++++++++++++++++++ tools/todo_tool.py | 5 +- website/docs/user-guide/configuration.md | 33 ++++++++-- 10 files changed, 324 insertions(+), 14 deletions(-) diff --git a/agent/agent_init.py b/agent/agent_init.py index 20e44607bb..60683fcafc 100644 --- a/agent/agent_init.py +++ b/agent/agent_init.py @@ -1896,6 +1896,12 @@ def init_agent( _agent_section = {} agent._tool_use_enforcement = _agent_section.get("tool_use_enforcement", "auto") + # Execution-discipline guidance gate: "auto" (default — matches + # EXECUTION_GUIDANCE_MODELS), true (always), false (never), or list of + # model-name substrings. Independent of tool_use_enforcement — see + # agent/system_prompt.py for the injection gate. + agent._execution_guidance = _agent_section.get("execution_guidance", "auto") + # Empty-response retry guard config (NS-503): additive # ``agent.empty_response_guard`` subsection. Resolution is tolerant — # a malformed section falls back to the schema defaults (guard on, diff --git a/agent/prompt_builder.py b/agent/prompt_builder.py index 998e5606a0..ed1533748b 100644 --- a/agent/prompt_builder.py +++ b/agent/prompt_builder.py @@ -358,6 +358,25 @@ TOOL_USE_ENFORCEMENT_GUIDANCE = ( # Add new patterns here when a model family needs explicit steering. TOOL_USE_ENFORCEMENT_MODELS = ("gpt", "codex", "gemini", "gemma", "grok", "glm", "qwen", "deepseek") +# Model name substrings whose sessions receive OPENAI_MODEL_EXECUTION_GUIDANCE +# (execution discipline: tool persistence, mandatory tool use for arithmetic, +# external-write read-back, count reconciliation, literal preservation, +# verification-gated completion) when agent.execution_guidance is "auto". +# +# gpt/codex/grok are the historical set; deepseek/kimi/qwen/glm/minimax/ +# mimo/mistral were added after Composio agentic-eval traces showed the same +# failure modes on those families (financial math in prose, no read-back after +# external writes, identifier "repair", completeness claims despite count +# mismatches). GLM's tool-calls-as-plain-text stall (#53847) and MiMo (#41874) +# are covered here too. Gemini/Gemma are excluded — they get the more specific +# GOOGLE_MODEL_OPERATIONAL_GUIDANCE block instead. Claude is excluded because +# it does not exhibit these failure modes; users can opt any model in via +# config.yaml `agent.execution_guidance: true` or a substring list. +EXECUTION_GUIDANCE_MODELS = ( + "gpt", "codex", "grok", + "deepseek", "kimi", "qwen", "glm", "minimax", "mimo", "mistral", +) + # Universal "finish the job" guidance — applied to ALL models, not gated # by model family. Addresses two cross-model failure modes: # 1. Stopping after a stub: writing a tiny file or running one command @@ -438,13 +457,22 @@ PARALLEL_TOOL_CALL_GUIDANCE = ( # without tool calls, suggests workarounds instead of using existing tools, # replies with plans/suggestions instead of executing). The body is # family-agnostic; the OPENAI_ prefix reflects origin, not exclusivity. +# +# As of the Composio agentic-eval follow-up, the block is no longer fenced to +# gpt/codex/grok: eval traces showed DeepSeek/Kimi doing financial math in +# prose, skipping read-back verification after external writes, "repairing" +# malformed identifiers, and claiming completeness despite count mismatches — +# exactly the failure modes this block targets. The injection gate lives in +# agent/system_prompt.py and is controlled by config.yaml +# ``agent.execution_guidance`` (auto/true/false/list); "auto" matches the +# EXECUTION_GUIDANCE_MODELS substring tuple below. OPENAI_MODEL_EXECUTION_GUIDANCE = ( "# Execution discipline\n" "\n" "- Use tools whenever they improve correctness, completeness, or grounding.\n" "- Do not stop early when another tool call would materially improve the result.\n" - "- If a tool returns empty or partial results, retry with a different query or " - "strategy before giving up.\n" + "- If a tool returns empty, partial, or suspiciously narrow results, retry " + "with a broader or different query or strategy before concluding.\n" "- Keep calling tools until: (1) the task is complete, AND (2) you have verified " "the result.\n" "\n" @@ -487,8 +515,30 @@ OPENAI_MODEL_EXECUTION_GUIDANCE = ( "- Formatting: does the output match the requested format or schema?\n" "- Safety: if the next step has side effects (file writes, commands, API calls), " "confirm scope before executing.\n" + "- Completion: 'done' means every named acceptance criterion is verified — " + "never a plausible subset. Completing your plan is not itself the answer; " + "the requested output must appear in your response.\n" "\n" "\n" + "\n" + "- After any state-changing write to an external system (API call, message " + "post, record update), verify the effect by reading back the exact target " + "before claiming success — a successful tool call is not a successful task. " + "Do NOT re-verify internal file edits a tool already confirmed.\n" + "- Declared totals in responses (total, reply_count, has_more, '...N more') " + "are hard assertions. If your enumerated count disagrees, re-fetch or parse " + "programmatically — never finalize on 'go with what I have'.\n" + "- When building write payloads, set fields explicitly rather than relying " + "on provider defaults that could contradict intent.\n" + "\n" + "\n" + "\n" + "- Preserve identifiers, commands, and values exactly as given — never " + "'repair' or normalize a token that fails a stated format. A successful " + "lookup does not validate a malformed source token; validate format first, " + "then look up.\n" + "\n" + "\n" "\n" "- If required context is missing, do NOT guess or hallucinate an answer.\n" "- Use the appropriate lookup tool when missing information is retrievable " diff --git a/agent/system_prompt.py b/agent/system_prompt.py index 2e2579a79c..7d5bba76b3 100644 --- a/agent/system_prompt.py +++ b/agent/system_prompt.py @@ -33,6 +33,7 @@ from typing import Any, Dict, List, Optional from agent.prompt_builder import ( DEFAULT_AGENT_IDENTITY, + EXECUTION_GUIDANCE_MODELS, GOOGLE_MODEL_OPERATIONAL_GUIDANCE, HERMES_AGENT_HELP_GUIDANCE, KANBAN_GUIDANCE, @@ -477,13 +478,36 @@ def build_system_prompt_parts(agent: Any, system_message: Optional[str] = None) # paths, parallel tool calls, verify-before-edit, etc.) if "gemini" in _model_lower or "gemma" in _model_lower: stable_parts.append(GOOGLE_MODEL_OPERATIONAL_GUIDANCE) - # OpenAI GPT/Codex execution discipline (tool persistence, - # prerequisite checks, verification, anti-hallucination). - # Also applied to xAI Grok — same failure modes (claims completion - # without tool calls, suggests workarounds instead of using - # existing tools, replies with plans instead of executing). - if "gpt" in _model_lower or "codex" in _model_lower or "grok" in _model_lower: - stable_parts.append(OPENAI_MODEL_EXECUTION_GUIDANCE) + + # Execution-discipline guidance (tool persistence, mandatory tool use + # for arithmetic, external-write read-back, count reconciliation, + # literal preservation, verification-gated completion). Historically + # nested inside the tool-use-enforcement branch and fenced to + # gpt/codex/grok; now an independent gate so DeepSeek/Kimi/Qwen-class + # models receive it even when tool_use_enforcement is off. Controlled + # by config.yaml agent.execution_guidance: + # "auto" (default) — matches EXECUTION_GUIDANCE_MODELS + # true — always inject (all models) + # false — never inject + # list — custom model-name substrings to match + # Resolved once at session start keyed on the (fixed) model name, so + # the system prompt stays byte-stable for the life of the conversation. + if agent.valid_tool_names: + _exec_guidance = getattr(agent, "_execution_guidance", "auto") + _exec_inject = False + if _exec_guidance is True or (isinstance(_exec_guidance, str) and _exec_guidance.lower() in {"true", "always", "yes", "on"}): + _exec_inject = True + elif _exec_guidance is False or (isinstance(_exec_guidance, str) and _exec_guidance.lower() in {"false", "never", "no", "off"}): + _exec_inject = False + elif isinstance(_exec_guidance, list): + model_lower = (agent.model or "").lower() + _exec_inject = any(p.lower() in model_lower for p in _exec_guidance if isinstance(p, str)) + else: + # "auto" or any unrecognised value — use hardcoded defaults + model_lower = (agent.model or "").lower() + _exec_inject = any(p in model_lower for p in EXECUTION_GUIDANCE_MODELS) + if _exec_inject: + stable_parts.append(OPENAI_MODEL_EXECUTION_GUIDANCE) has_skills_tools = any(name in agent.valid_tool_names for name in ['skills_list', 'skill_view', 'skill_manage']) if has_skills_tools: diff --git a/hermes_cli/config_defaults.py b/hermes_cli/config_defaults.py index c1dd275401..5a6173d6d2 100644 --- a/hermes_cli/config_defaults.py +++ b/hermes_cli/config_defaults.py @@ -146,6 +146,15 @@ DEFAULT_CONFIG = { # (force on/off for all models), or a list of model-name substrings # to match (e.g. ["gpt", "codex", "gemini", "qwen"]). "tool_use_enforcement": "auto", + # Execution-discipline guidance: injects a system prompt block covering + # tool persistence, mandatory tool use for arithmetic/system facts, + # external-write read-back, count reconciliation, literal preservation + # of identifiers, and verification-gated completion. Chosen once at + # session start keyed on model name (prompt stays byte-stable). + # Values: "auto" (default — applies to gpt/codex/grok/deepseek/kimi/ + # qwen/glm/minimax/mimo/mistral models), true/false (force on/off for + # all models), or a list of model-name substrings to match. + "execution_guidance": "auto", # Intent-ack continuation: when the model opens a turn by narrating an # action it will take ("I'll go check the logs...") but emits no tool # call, intercept the turn-end, inject a "continue now, execute the diff --git a/hermes_cli/dump.py b/hermes_cli/dump.py index a8d3e992bd..e29675ce13 100644 --- a/hermes_cli/dump.py +++ b/hermes_cli/dump.py @@ -238,6 +238,7 @@ def _config_overrides(config: dict) -> dict[str, str]: ("agent", "gateway_timeout"), ("agent", "session_stall_timeout"), ("agent", "tool_use_enforcement"), + ("agent", "execution_guidance"), ("terminal", "backend"), ("terminal", "docker_image"), ("terminal", "persistent_shell"), diff --git a/tests/agent/test_prompt_builder.py b/tests/agent/test_prompt_builder.py index 38bc36efba..4d94272023 100644 --- a/tests/agent/test_prompt_builder.py +++ b/tests/agent/test_prompt_builder.py @@ -986,6 +986,51 @@ class TestOpenAIModelExecutionGuidance: assert isinstance(OPENAI_MODEL_EXECUTION_GUIDANCE, str) assert len(OPENAI_MODEL_EXECUTION_GUIDANCE) > 100 + def test_guidance_covers_external_write_readback(self): + text = OPENAI_MODEL_EXECUTION_GUIDANCE.lower() + assert "read" in text and "back" in text + assert "successful tool call is not a successful task" in text + + def test_guidance_covers_count_reconciliation(self): + text = OPENAI_MODEL_EXECUTION_GUIDANCE.lower() + assert "has_more" in text + assert "hard assertions" in text + + def test_guidance_covers_literal_preservation(self): + text = OPENAI_MODEL_EXECUTION_GUIDANCE.lower() + assert "normalize" in text + assert "malformed" in text + + def test_guidance_covers_retry_differently(self): + text = OPENAI_MODEL_EXECUTION_GUIDANCE.lower() + assert "suspiciously narrow" in text + assert "retry" in text + + def test_guidance_gates_completion_on_verification(self): + text = OPENAI_MODEL_EXECUTION_GUIDANCE.lower() + assert "plausible subset" in text + + +class TestExecutionGuidanceModels: + """Behavior contracts for the default auto-match model list.""" + + def test_includes_historical_families(self): + from agent.prompt_builder import EXECUTION_GUIDANCE_MODELS + for fam in ("gpt", "codex", "grok"): + assert fam in EXECUTION_GUIDANCE_MODELS + + def test_includes_composio_eval_families(self): + from agent.prompt_builder import EXECUTION_GUIDANCE_MODELS + for fam in ("deepseek", "kimi", "qwen", "glm", "minimax", "mimo", "mistral"): + assert fam in EXECUTION_GUIDANCE_MODELS + + def test_excludes_google_and_claude(self): + # Gemini/Gemma get GOOGLE_MODEL_OPERATIONAL_GUIDANCE instead; + # Claude doesn't exhibit the targeted failure modes. + from agent.prompt_builder import EXECUTION_GUIDANCE_MODELS + for fam in ("gemini", "gemma", "claude"): + assert fam not in EXECUTION_GUIDANCE_MODELS + class TestParallelToolCallGuidance: """Behavior contracts for the universal parallel-tool-call guidance block. diff --git a/tests/agent/test_system_prompt.py b/tests/agent/test_system_prompt.py index aac357bd75..56dc09f1a1 100644 --- a/tests/agent/test_system_prompt.py +++ b/tests/agent/test_system_prompt.py @@ -116,6 +116,87 @@ class TestCodingContextBlock: assert "coding agent" not in _stable_prompt(agent) +class TestExecutionGuidanceInjection: + """Injection gate for OPENAI_MODEL_EXECUTION_GUIDANCE via + ``agent.execution_guidance`` (auto/true/false/list). + + Background — Composio agentic-eval traces (2026-08): the block was + historically fenced to gpt/codex/grok AND nested inside the + tool-use-enforcement branch, so DeepSeek/Kimi/Qwen-class models + received no execution discipline at all. The gate is now independent + of tool_use_enforcement and defaults to a broader family list. + """ + + def _prompt(self, model, execution_guidance="auto", *, + tool_use_enforcement=False, + valid_tool_names=("terminal", "read_file")): + agent = _make_agent( + valid_tool_names=list(valid_tool_names), + model=model, + _tool_use_enforcement=tool_use_enforcement, + _execution_guidance=execution_guidance, + ) + return _stable_prompt(agent) + + def test_deepseek_gets_guidance_by_default(self): + stable = self._prompt("deepseek/deepseek-v4-pro") + assert "Execution discipline" in stable + assert "" in stable + + def test_kimi_gets_guidance_by_default(self): + assert "Execution discipline" in self._prompt("moonshotai/kimi-k3") + + def test_qwen_glm_minimax_mimo_mistral_get_guidance_by_default(self): + for model in ("qwen/qwen-3-max", "z-ai/glm-5.2", + "minimax/minimax-m2", "xiaomi/mimo-v2", + "mistralai/mistral-large-3"): + assert "Execution discipline" in self._prompt(model), model + + def test_gpt_still_gets_guidance(self): + assert "Execution discipline" in self._prompt("openai/gpt-5.5") + + def test_grok_still_gets_guidance(self): + assert "Execution discipline" in self._prompt("xai/grok-4") + + def test_independent_of_tool_use_enforcement(self): + # The gate must not require tool-use enforcement to be on. + stable = self._prompt("deepseek/deepseek-v4-flash", + tool_use_enforcement=False) + assert "Execution discipline" in stable + assert "Tool-use enforcement" not in stable + + def test_claude_does_not_get_guidance_by_default(self): + assert "Execution discipline" not in self._prompt( + "anthropic/claude-opus-4.8") + + def test_gemini_does_not_get_guidance_by_default(self): + assert "Execution discipline" not in self._prompt( + "google/gemini-2.5-pro") + + def test_config_false_suppresses(self): + assert "Execution discipline" not in self._prompt( + "openai/gpt-5.5", execution_guidance=False) + assert "Execution discipline" not in self._prompt( + "deepseek/deepseek-v4-pro", execution_guidance="off") + + def test_config_true_forces_for_any_model(self): + assert "Execution discipline" in self._prompt( + "anthropic/claude-opus-4.8", execution_guidance=True) + + def test_config_list_matches_substring(self): + stable = self._prompt("mycorp/custom-llm-7b", + execution_guidance=["custom-llm", "gpt"]) + assert "Execution discipline" in stable + + def test_config_list_non_match_suppresses(self): + assert "Execution discipline" not in self._prompt( + "openai/gpt-5.5", execution_guidance=["deepseek"]) + + def test_no_tools_no_guidance(self): + assert "Execution discipline" not in self._prompt( + "deepseek/deepseek-v4-pro", valid_tool_names=()) + + class TestNamedProfileHintIntegration: """The same defect through the REAL resolution chain (#72894). diff --git a/tests/run_agent/test_run_agent.py b/tests/run_agent/test_run_agent.py index 135e37e83f..cd4836c575 100644 --- a/tests/run_agent/test_run_agent.py +++ b/tests/run_agent/test_run_agent.py @@ -1092,6 +1092,72 @@ class TestToolUseEnforcementConfig: assert TOOL_USE_ENFORCEMENT_GUIDANCE not in prompt +class TestExecutionGuidanceConfig: + """End-to-end tests for the agent.execution_guidance config option — + from config.yaml through agent_init to the built system prompt.""" + + def _make_agent(self, model="deepseek/deepseek-v4-pro", execution_guidance=None): + agent_cfg = {"tool_use_enforcement": False} + if execution_guidance is not None: + agent_cfg["execution_guidance"] = execution_guidance + with ( + patch( + "run_agent.get_tool_definitions", + return_value=_make_tool_defs("terminal", "web_search"), + ), + patch("run_agent.check_toolset_requirements", return_value={}), + patch("run_agent.OpenAI"), + patch( + "hermes_cli.config.load_config", + return_value={"agent": agent_cfg}, + ), patch( + "hermes_cli.config.load_config_readonly", + return_value={"agent": agent_cfg}, + ), + ): + a = AIAgent( + model=model, + api_key="test-key-1234567890", + base_url="https://openrouter.ai/api/v1", + quiet_mode=True, + skip_context_files=True, + skip_memory=True, + ) + a.client = MagicMock() + return a + + def test_deepseek_gets_guidance_by_default(self): + from agent.prompt_builder import OPENAI_MODEL_EXECUTION_GUIDANCE + agent = self._make_agent(model="deepseek/deepseek-v4-pro") + assert OPENAI_MODEL_EXECUTION_GUIDANCE in agent._build_system_prompt() + + def test_gpt_still_gets_guidance(self): + from agent.prompt_builder import OPENAI_MODEL_EXECUTION_GUIDANCE + agent = self._make_agent(model="openai/gpt-4.1") + assert OPENAI_MODEL_EXECUTION_GUIDANCE in agent._build_system_prompt() + + def test_config_false_suppresses(self): + from agent.prompt_builder import OPENAI_MODEL_EXECUTION_GUIDANCE + agent = self._make_agent( + model="deepseek/deepseek-v4-pro", execution_guidance=False + ) + assert OPENAI_MODEL_EXECUTION_GUIDANCE not in agent._build_system_prompt() + + def test_config_list_matches(self): + from agent.prompt_builder import OPENAI_MODEL_EXECUTION_GUIDANCE + agent = self._make_agent( + model="moonshotai/kimi-k3", execution_guidance=["kimi"] + ) + assert OPENAI_MODEL_EXECUTION_GUIDANCE in agent._build_system_prompt() + + def test_config_list_non_match_suppresses(self): + from agent.prompt_builder import OPENAI_MODEL_EXECUTION_GUIDANCE + agent = self._make_agent( + model="openai/gpt-4.1", execution_guidance=["kimi"] + ) + assert OPENAI_MODEL_EXECUTION_GUIDANCE not in agent._build_system_prompt() + + class TestTaskCompletionGuidance: """Tests for the universal task-completion / no-fabrication guidance (config.yaml ``agent.task_completion_guidance``). diff --git a/tools/todo_tool.py b/tools/todo_tool.py index 1eea334f43..cc80fb92fb 100644 --- a/tools/todo_tool.py +++ b/tools/todo_tool.py @@ -296,6 +296,8 @@ TODO_SCHEMA = { "description": ( "Manage your task list for the current session. Use for complex tasks " "with 3+ steps or when the user provides multiple tasks. " + "For 'all N items' tasks, enumerate every instance as its own checklist " + "item so none are silently dropped. " "Call with no parameters to read the current list.\n\n" "Writing:\n" "- Provide 'todos' array to create/update items\n" @@ -304,7 +306,8 @@ TODO_SCHEMA = { "Each item: {id: string, content: string, " "status: pending|in_progress|completed|cancelled}\n" "List order is priority. Only ONE item in_progress at a time.\n" - "Mark items completed immediately when done. If something fails, " + "Mark an item completed only after the work is verified done, never " + "based on intent. If something fails, " "cancel it and add a revised item.\n\n" "Always returns the full current list." ), diff --git a/website/docs/user-guide/configuration.md b/website/docs/user-guide/configuration.md index 0eccf2d5aa..e031cb639e 100644 --- a/website/docs/user-guide/configuration.md +++ b/website/docs/user-guide/configuration.md @@ -1629,13 +1629,11 @@ agent: ### What it injects -When enabled, three layers of guidance may be added to the system prompt: +When enabled, two layers of guidance may be added to the system prompt: 1. **General tool-use enforcement** (all matched models) — instructs the model to make tool calls immediately instead of describing intentions, keep working until the task is complete, and never end a turn with a promise of future action. -2. **OpenAI execution discipline** (GPT, Codex, and Grok models) — additional guidance addressing GPT-specific failure modes: abandoning work on partial results, skipping prerequisite lookups, hallucinating instead of using tools, and declaring "done" without verification. - -3. **Google operational guidance** (Gemini and Gemma models only) — conciseness, absolute paths, parallel tool calls, and verify-before-edit patterns. +2. **Google operational guidance** (Gemini and Gemma models only) — conciseness, absolute paths, parallel tool calls, and verify-before-edit patterns. These are transparent to the user and only affect the system prompt. Models that already use tools reliably (like Claude) don't need this guidance, which is why `"auto"` excludes them. @@ -1648,6 +1646,33 @@ agent: tool_use_enforcement: ["gpt", "codex", "gemini", "grok", "my-custom-model"] ``` +## Execution-Discipline Guidance + +Separately from tool-use enforcement, Hermes injects an **execution-discipline** block for model families that share a set of agentic failure modes observed in eval traces: doing arithmetic in prose instead of code, skipping read-back verification after external writes, "repairing" malformed identifiers, claiming completeness despite count mismatches, and declaring "done" without verifying every acceptance criterion. + +```yaml +agent: + execution_guidance: "auto" # "auto" | true | false | ["model-substring", ...] +``` + +| Value | Behavior | +|-------|----------| +| `"auto"` (default) | Enabled for models matching: `gpt`, `codex`, `grok`, `deepseek`, `kimi`, `qwen`, `glm`, `minimax`, `mimo`, `mistral`. | +| `true` | Always enabled, regardless of model. | +| `false` | Always disabled, regardless of model. | +| `["deepseek", "my-custom-model"]` | Enabled only when the model name contains one of the listed substrings (case-insensitive). | + +The injected block covers: + +- **Tool persistence** — keep calling tools until the task is complete *and* verified; retry empty, partial, or suspiciously narrow lookup results with a broader or different query before concluding. +- **Mandatory tool use** — arithmetic, hashes, dates, system state, and file facts always come from a tool, never from mental computation. +- **External-write read-back** — after any state-changing write to an external system, read back the exact target before claiming success (internal file edits a tool already confirmed are not re-verified). +- **Count reconciliation** — declared totals (`total`, `reply_count`, `has_more`) are hard assertions; on mismatch, re-fetch or parse programmatically. +- **Literal preservation** — never normalize or "repair" identifiers that fail a stated format; a successful lookup does not validate a malformed source token. +- **Verification-gated completion** — "done" means every named acceptance criterion is verified, never a plausible subset. + +The gate is independent of `tool_use_enforcement` — either can be on without the other. The guidance is chosen once at session start keyed on the model name, so the system prompt stays byte-stable (and prompt-cache-friendly) for the life of the conversation. Gemini/Gemma are excluded from the auto list because they receive the more specific Google operational guidance; Claude is excluded because it doesn't exhibit these failure modes — opt any model in with `true` or a substring list. + ## Tool-Loop Guardrails Hermes detects when the agent is stuck in an unproductive tool-calling loop — the same tool call failing repeatedly, the same tool failing over and over, or an idempotent call returning the same result with no progress. By default it injects a **warning** into the tool result so the model self-corrects; it does not hard-stop, since a person watching the CLI/TUI can intervene. From 09e657793eb9dd0b508ee25861a1d1271e67869b Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 19 Aug 2026 15:40:30 -0700 Subject: [PATCH 19/24] feat: MCP tool results spill at 50K and carry upstream-elision warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Composio-style MCP servers return un-paginated 22-47K-char payloads that sail under the generic 100K per-result spillover threshold, bloating context and ballooning per-turn reasoning time on long conversations. Competitors cap harder (OpenCode/pi 50KB, Claude Code 30K, Codex ~10K tokens). Three changes: - mcp_* tools spill at a tighter 50K default (BudgetConfig.mcp_result_size, config-overridable via tool_budget.mcp_result_size_chars; pinned and per-tool overrides still win; capped by the context-scaled default). - The persisted-output preview now teaches recovery: page the saved file with read_file or process with execute_code instead of re-requesting the same data from the remote API. - Untrusted/MCP string results are scanned (bounded, first 64KB) for provider-side elision markers ('...N more items', "has_more": true, 'saved to sandbox', data_preview) and get ONE cache-safe incompleteness notice appended at result-construction time, before untrusted wrapping — so the model stops treating provider-elided enumerations as complete. - Hard 2M-char allocation cap in mcp_tool.py (text, error, and structuredContent paths) so a pathological multi-MB server payload is bounded before it propagates, while ordinary large results reach spillover intact. Distilled from #56060/#56072/#56511 (issue #56059); supersedes their 50K lossy truncation with spillover-friendly semantics. Docs: configuration.md spillover-budget section + cli-config.yaml.example. Co-authored-by: Stoltemberg <215755014+Stoltemberg@users.noreply.github.com> Co-authored-by: AlexFucuson9 <295703459+AlexFucuson9@users.noreply.github.com> Co-authored-by: Tranquil-Flow <66773372+Tranquil-Flow@users.noreply.github.com> --- agent/tool_dispatch_helpers.py | 72 +++++++++++++++++- agent/tool_executor.py | 5 +- cli-config.yaml.example | 13 ++++ tests/agent/test_tool_dispatch_helpers.py | 90 +++++++++++++++++++++++ tests/tools/test_budget_config.py | 65 ++++++++++++++++ tests/tools/test_mcp_result_size_limit.py | 69 +++++++++++++++++ tests/tools/test_tool_result_storage.py | 19 +++++ tools/budget_config.py | 69 ++++++++++++++++- tools/mcp_tool.py | 55 +++++++++++++- tools/tool_result_storage.py | 7 +- website/docs/user-guide/configuration.md | 15 ++++ 11 files changed, 473 insertions(+), 6 deletions(-) create mode 100644 tests/tools/test_mcp_result_size_limit.py diff --git a/agent/tool_dispatch_helpers.py b/agent/tool_dispatch_helpers.py index af0970accb..0c4259b656 100644 --- a/agent/tool_dispatch_helpers.py +++ b/agent/tool_dispatch_helpers.py @@ -557,7 +557,11 @@ def make_tool_result_message( The outer list itself is rebuilt rather than returned by identity, so callers should compare by value, not by ``is``. """ - wrapped = _maybe_wrap_untrusted(name, content) + # Order matters: detect provider-side elision on the RAW content and + # append the notice first, THEN wrap — so the notice lives inside the + # untrusted block next to the data it describes, appended exactly once + # at construction time (cache-safe). + wrapped = _maybe_wrap_untrusted(name, _maybe_append_elision_notice(name, content)) message = stamp_message_timestamp({ "role": "tool", "name": name, @@ -608,6 +612,70 @@ def _is_untrusted_tool(name: Optional[str]) -> bool: return any(name.startswith(p) for p in _UNTRUSTED_TOOL_PREFIXES) +# --- Upstream-elision detection -------------------------------------------- +# +# Some MCP servers elide data SERVER-SIDE and mark the elision inside the +# payload itself (e.g. Composio: '...13 more items' inside a JSON array, +# '"has_more": true', 'Complete response was large (N tokens). Full data +# saved to sandbox in /mnt/files/...', 'data_preview' envelopes). Because the +# result looks structurally complete, models treat the visible slice as the +# whole dataset and falsely claim completeness. When one of these markers is +# present, we append ONE compact notice at result-construction time — before +# the message enters history, never mutated later, so prompt caching is safe. + +# Conservative patterns only: each one is an explicit provider-side "there is +# more data than what you can see" signal, not a generic truncation heuristic. +_UPSTREAM_ELISION_PATTERNS = ( + re.compile(r"\.\.\.\s*\d+\s+more\s+items?", re.IGNORECASE), + re.compile(r'"has_more"\s*:\s*true', re.IGNORECASE), + re.compile(r"saved to sandbox", re.IGNORECASE), + re.compile(r"data_preview", re.IGNORECASE), +) + +# Results smaller than this can't meaningfully hide an elided enumeration — +# skip the scan entirely so tiny results pay nothing. +_ELISION_SCAN_MIN_CHARS = 1_000 + +# Bound the regex scan: markers appear near the elided structure, which for +# the payload sizes that matter (20-50K) is always inside the first 64KB. +_ELISION_SCAN_MAX_CHARS = 65_536 + +_UPSTREAM_ELISION_NOTICE = ( + '\n[hermes note: this result contains provider-side elision markers ' + '(e.g. "...N more items" / has_more:true). The data shown is INCOMPLETE ' + '— page/fetch the remainder before treating any enumeration as complete.]' +) + + +def _detect_upstream_elision(content: Any) -> bool: + """True when a string tool result carries provider-side elision markers. + + Cheap and safe by construction: non-string content is never scanned, + results under ``_ELISION_SCAN_MIN_CHARS`` short-circuit, and the regex + scan is capped at the first ``_ELISION_SCAN_MAX_CHARS`` chars. + """ + if not isinstance(content, str): + return False + if len(content) < _ELISION_SCAN_MIN_CHARS: + return False + window = content[:_ELISION_SCAN_MAX_CHARS] + return any(p.search(window) for p in _UPSTREAM_ELISION_PATTERNS) + + +def _maybe_append_elision_notice(name: str, content: Any) -> Any: + """Append the incompleteness notice to untrusted string results that + embed upstream elision markers. Returns ``content`` unchanged otherwise. + + Runs on the RAW result before untrusted-wrapping so the notice sits with + the data it describes, and only at result-construction time (cache-safe). + """ + if not _is_untrusted_tool(name): + return content + if _detect_upstream_elision(content): + return content + _UPSTREAM_ELISION_NOTICE + return content + + def _tool_output_risk_metadata(name: str, content: Any) -> Optional[Dict[str, Any]]: """Classify textual attacker-controlled output without retaining a copy. @@ -729,5 +797,7 @@ __all__ = [ "_extract_landed_file_mutation_paths", "_extract_error_preview", "_trajectory_normalize_msg", + "_detect_upstream_elision", + "_maybe_append_elision_notice", "make_tool_result_message", ] diff --git a/agent/tool_executor.py b/agent/tool_executor.py index 90226b02e1..2eb69c6124 100644 --- a/agent/tool_executor.py +++ b/agent/tool_executor.py @@ -87,7 +87,10 @@ def _budget_for_agent(agent) -> BudgetConfig: """ try: ctx = getattr(getattr(agent, "context_compressor", None), "context_length", None) - return budget_for_context_window(int(ctx)) if ctx else DEFAULT_BUDGET + # budget_for_context_window(None) (rather than DEFAULT_BUDGET) so the + # config-driven MCP threshold override still applies when the context + # length isn't resolvable. + return budget_for_context_window(int(ctx) if ctx else None) except Exception: return DEFAULT_BUDGET diff --git a/cli-config.yaml.example b/cli-config.yaml.example index d2228c76a7..87aad0ae94 100644 --- a/cli-config.yaml.example +++ b/cli-config.yaml.example @@ -671,6 +671,19 @@ compression: # To pin a specific model/provider for compression summaries, use the # auxiliary section below (auxiliary.compression.provider / model). +# ============================================================================= +# Tool-result budget (optional) +# ============================================================================= +# Controls when a large tool result is spilled to disk (full output saved to +# $HERMES_HOME/cache/spillover, preview + path kept in context). MCP tool +# results (tools named mcp_*) spill at a tighter 50,000-char threshold than +# the generic 100K default: MCP servers routinely return un-paginated 20-50K +# payloads that bloat context and slow every subsequent turn. Nothing is +# lost — the full result is on disk and readable with read_file. +# +# tool_budget: +# mcp_result_size_chars: 50000 # per-result spillover threshold for mcp_* tools + # ============================================================================= # Anthropic prompt caching TTL # ============================================================================= diff --git a/tests/agent/test_tool_dispatch_helpers.py b/tests/agent/test_tool_dispatch_helpers.py index 34c0a6dda6..c56c362d36 100644 --- a/tests/agent/test_tool_dispatch_helpers.py +++ b/tests/agent/test_tool_dispatch_helpers.py @@ -215,3 +215,93 @@ class TestFileMutationTargets: }, ) assert targets == ["old/name.py", "new/name.py"] + + +class TestUpstreamElisionDetection: + """Provider-side elision markers get a one-line incompleteness notice.""" + + def _payload(self, marker: str) -> str: + return '{"items": ["' + "x" * 1_200 + '"], ' + marker + "}" + + def test_more_items_marker_detected(self): + from agent.tool_dispatch_helpers import _detect_upstream_elision + assert _detect_upstream_elision(self._payload('"note": "... 13 more items"')) + + def test_has_more_true_detected(self): + from agent.tool_dispatch_helpers import _detect_upstream_elision + assert _detect_upstream_elision(self._payload('"has_more": true')) + + def test_saved_to_sandbox_detected(self): + from agent.tool_dispatch_helpers import _detect_upstream_elision + assert _detect_upstream_elision( + "y" * 1_100 + " Complete response was large. Full data saved to sandbox in /mnt/files/x.json" + ) + + def test_data_preview_detected(self): + from agent.tool_dispatch_helpers import _detect_upstream_elision + assert _detect_upstream_elision(self._payload('"data_preview": {}')) + + def test_has_more_false_not_detected(self): + from agent.tool_dispatch_helpers import _detect_upstream_elision + assert not _detect_upstream_elision(self._payload('"has_more": false')) + + def test_plain_large_result_not_detected(self): + from agent.tool_dispatch_helpers import _detect_upstream_elision + assert not _detect_upstream_elision("z" * 5_000) + + def test_non_string_content_skipped(self): + from agent.tool_dispatch_helpers import _detect_upstream_elision + assert not _detect_upstream_elision(None) + assert not _detect_upstream_elision({"has_more": True}) + assert not _detect_upstream_elision([{"type": "text", "text": "... 5 more items"}]) + + def test_short_results_short_circuit(self): + from agent.tool_dispatch_helpers import _detect_upstream_elision + # Marker present but under the 1K scan floor -> skipped. + assert not _detect_upstream_elision('"has_more": true') + + def test_marker_beyond_scan_cap_not_matched(self): + from agent.tool_dispatch_helpers import ( + _ELISION_SCAN_MAX_CHARS, + _detect_upstream_elision, + ) + content = "a" * (_ELISION_SCAN_MAX_CHARS + 10) + '"has_more": true' + assert not _detect_upstream_elision(content) + + +class TestElisionNoticeWiring: + """Notice appended once at construction time, before untrusted wrapping.""" + + def _elided(self) -> str: + return '{"items": ["' + "x" * 1_200 + '"], "has_more": true}' + + def test_notice_appended_for_mcp_tool(self): + from agent.tool_dispatch_helpers import ( + _UPSTREAM_ELISION_NOTICE, + _maybe_append_elision_notice, + ) + out = _maybe_append_elision_notice("mcp_composio_search", self._elided()) + assert out.endswith(_UPSTREAM_ELISION_NOTICE) + + def test_trusted_tool_never_annotated(self): + from agent.tool_dispatch_helpers import _maybe_append_elision_notice + content = self._elided() + assert _maybe_append_elision_notice("terminal", content) is content + + def test_untrusted_without_markers_unchanged(self): + from agent.tool_dispatch_helpers import _maybe_append_elision_notice + content = "y" * 2_000 + assert _maybe_append_elision_notice("mcp_x", content) is content + + def test_notice_inside_untrusted_wrapper(self): + """Order: detect on raw -> append notice -> wrap. The notice must sit + INSIDE the untrusted block, and the message is built once (cache-safe).""" + from agent.tool_dispatch_helpers import make_tool_result_message + msg = make_tool_result_message("mcp_composio_search", self._elided(), "call_1") + content = msg["content"] + assert content.startswith("") + assert "INCOMPLETE" in content + assert content.index("hermes note") < content.index("") + # Exactly one notice. + assert content.count("hermes note") == 1 diff --git a/tests/tools/test_budget_config.py b/tests/tools/test_budget_config.py index 118bca3ecb..2ef47369c3 100644 --- a/tests/tools/test_budget_config.py +++ b/tests/tools/test_budget_config.py @@ -173,3 +173,68 @@ class TestBudgetForContextWindow: threshold = cfg.resolve_threshold("mcp_firecrawl_firecrawl_search") assert threshold < huge_len assert cfg.default_result_size < huge_len + + +# --------------------------------------------------------------------------- +# MCP-prefix threshold (mcp_result_size) +# --------------------------------------------------------------------------- + + +class TestMcpPrefixThreshold: + """mcp_* tools get the tighter 50K default, config-overridable.""" + + def test_default_mcp_threshold_is_50k(self): + from tools.budget_config import DEFAULT_MCP_RESULT_SIZE_CHARS + assert DEFAULT_MCP_RESULT_SIZE_CHARS == 50_000 + assert DEFAULT_BUDGET.resolve_threshold("mcp_composio_search_tools") == 50_000 + + def test_non_mcp_tools_keep_generic_default(self): + assert DEFAULT_BUDGET.resolve_threshold("some_random_tool") == DEFAULT_RESULT_SIZE_CHARS + + def test_pinned_wins_over_mcp_prefix(self): + with patch.dict(PINNED_THRESHOLDS, {"mcp_pinned_tool": float("inf")}): + assert DEFAULT_BUDGET.resolve_threshold("mcp_pinned_tool") == float("inf") + + def test_tool_override_wins_over_mcp_prefix(self): + cfg = BudgetConfig(tool_overrides={"mcp_special": 75_000}) + assert cfg.resolve_threshold("mcp_special") == 75_000 + + def test_mcp_threshold_capped_by_scaled_default(self): + """On a small model the scaled default_result_size caps the MCP value.""" + cfg = BudgetConfig(default_result_size=20_000, mcp_result_size=50_000) + assert cfg.resolve_threshold("mcp_anything") == 20_000 + + def test_mcp_threshold_never_exceeds_default_result_size(self): + cfg = BudgetConfig(default_result_size=100_000, mcp_result_size=999_999) + assert cfg.resolve_threshold("mcp_anything") == 100_000 + + def test_config_override_via_hermes_home(self, tmp_path, monkeypatch): + (tmp_path / "config.yaml").write_text( + "tool_budget:\n mcp_result_size_chars: 30000\n" + ) + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + cfg = budget_for_context_window(None) + assert cfg.resolve_threshold("mcp_composio_multi_execute") == 30_000 + # Generic tools are untouched by the MCP knob. + assert cfg.default_result_size == DEFAULT_RESULT_SIZE_CHARS + + def test_config_override_survives_window_scaling(self, tmp_path, monkeypatch): + (tmp_path / "config.yaml").write_text( + "tool_budget:\n mcp_result_size_chars: 30000\n" + ) + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + cfg = budget_for_context_window(200_000) + assert cfg.mcp_result_size == 30_000 + + def test_malformed_config_falls_back_to_default(self, tmp_path, monkeypatch): + (tmp_path / "config.yaml").write_text("tool_budget: not-a-mapping\n") + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + cfg = budget_for_context_window(None) + assert cfg.resolve_threshold("mcp_x_y") == 50_000 + + def test_scaled_small_window_caps_mcp_threshold(self, tmp_path, monkeypatch): + """A tiny model's scaled default_result_size caps even the MCP value.""" + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) # no config.yaml + cfg = budget_for_context_window(16_384) # scaled default < 50K + assert cfg.default_result_size < 50_000 + assert cfg.resolve_threshold("mcp_tool") == cfg.default_result_size diff --git a/tests/tools/test_mcp_result_size_limit.py b/tests/tools/test_mcp_result_size_limit.py new file mode 100644 index 0000000000..da9e11a308 --- /dev/null +++ b/tests/tools/test_mcp_result_size_limit.py @@ -0,0 +1,69 @@ +"""Regression tests for the MCP hard result cap (#56059). + +MCP tool results had no allocation bound — a buggy or malicious MCP server +could return multi-megabyte text that floods memory and context before the +budget/spillover layer sees it. The hard cap truncates only pathological +payloads (over 2M chars by default) with a 40% head / 60% tail split; +ordinary large results pass through untouched so the 50K MCP spillover +threshold (tools/budget_config.py) can preserve them in full on disk. + +Test shape adapted from PR #56511 (Tranquil-Flow); cap semantics differ — +see _MCP_HARD_RESULT_CAP_CHARS in tools/mcp_tool.py. +""" + +from __future__ import annotations + +from tools.mcp_tool import _MCP_HARD_RESULT_CAP_CHARS, _truncate_mcp_text_result + + +class TestTruncateMcpTextResult: + def test_short_result_unchanged(self): + text = "x" * 100 + assert _truncate_mcp_text_result(text) == text + + def test_exact_limit_unchanged(self): + text = "y" * 100 + assert _truncate_mcp_text_result(text, max_chars=100) == text + + def test_spillover_sized_result_passes_untouched(self): + """A 60K result (over the 50K spillover threshold) is NOT truncated + here — the budget layer must receive it intact so spillover can + preserve the full payload on disk.""" + text = "z" * 60_000 + assert _truncate_mcp_text_result(text) == text + + def test_pathological_result_is_truncated(self): + text = "z" * (_MCP_HARD_RESULT_CAP_CHARS + 500_000) + result = _truncate_mcp_text_result(text) + assert len(result) < len(text) + assert "TRUNCATED" in result + + def test_truncation_preserves_head_and_tail(self): + head_marker = "HEAD_MARKER_START" + tail_marker = "TAIL_MARKER_END" + text = head_marker + "x" * 5000 + tail_marker + result = _truncate_mcp_text_result(text, max_chars=200) + assert result.startswith(head_marker) + assert result.endswith(tail_marker) + + def test_truncation_includes_omitted_count(self): + text = "a" * 5000 + result = _truncate_mcp_text_result(text, max_chars=100) + assert "4,900" in result # 5000 - 100 omitted + assert "5,000" in result # total original length + + def test_truncation_uses_40_60_head_tail_split(self): + text = "H" * 40 + "M" * 5000 + "T" * 60 + result = _truncate_mcp_text_result(text, max_chars=100) + assert result[:40] == "H" * 40 + assert result[-60:] == "T" * 60 + + def test_empty_result_unchanged(self): + assert _truncate_mcp_text_result("") == "" + + def test_hard_cap_sits_above_spillover_threshold(self): + """The hard cap must stay far above the MCP spillover threshold so + spillover, not lossy truncation, handles ordinary large results.""" + from tools.budget_config import DEFAULT_MCP_RESULT_SIZE_CHARS + + assert _MCP_HARD_RESULT_CAP_CHARS > DEFAULT_MCP_RESULT_SIZE_CHARS * 10 diff --git a/tests/tools/test_tool_result_storage.py b/tests/tools/test_tool_result_storage.py index bdb50a7361..c72d8f933f 100644 --- a/tests/tools/test_tool_result_storage.py +++ b/tests/tools/test_tool_result_storage.py @@ -480,3 +480,22 @@ class TestSpillover: assert not old.exists() assert (spill_dir / "tc_prune_1.txt").exists() + + +# ── recovery hint in the persisted preview ──────────────────────────── + +class TestRecoveryHint: + def test_preview_teaches_recovery_not_refetch(self): + msg = _build_persisted_message( + preview="preview text", + has_more=True, + original_size=60_000, + file_path="/tmp/hermes-results/r.txt", + ) + assert "Recovery:" in msg + assert "execute_code" in msg + assert "re-request" in msg + # Structure preserved: tag, size, path, read_file guidance all intact. + assert msg.startswith(PERSISTED_OUTPUT_TAG) + assert msg.endswith(PERSISTED_OUTPUT_CLOSING_TAG) + assert "read_file" in msg diff --git a/tools/budget_config.py b/tools/budget_config.py index 8e47479446..c746bdb3b3 100644 --- a/tools/budget_config.py +++ b/tools/budget_config.py @@ -18,6 +18,55 @@ DEFAULT_RESULT_SIZE_CHARS: int = 100_000 DEFAULT_TURN_BUDGET_CHARS: int = 200_000 DEFAULT_PREVIEW_SIZE_CHARS: int = 1_500 +# Tighter default per-result threshold for MCP tools (name prefix ``mcp_``). +# +# MCP servers routinely return un-paginated 20-50K-char payloads (tool +# discovery catalogs, batched executions) that sail under the generic 100K +# threshold and silently bloat context — in agentic evals this measurably +# ballooned per-turn reasoning time on long conversations. Competitor +# harnesses cap harder (OpenCode 50KB, pi 50KB, Claude Code 30K chars, +# Codex ~10K tokens); 50K chars keeps parity with the strictest general- +# purpose caps while spillover (unlike truncation) preserves the full +# payload on disk. Overridable via ``tool_budget.mcp_result_size_chars`` +# in config.yaml. +DEFAULT_MCP_RESULT_SIZE_CHARS: int = 50_000 + +# Tool-name prefix that identifies MCP-served tools (same prefix the +# untrusted-content wrapper keys on in agent/tool_dispatch_helpers.py). +MCP_TOOL_PREFIX: str = "mcp_" + + +def _configured_mcp_result_size() -> int: + """Read ``tool_budget.mcp_result_size_chars`` from the active config.yaml. + + Reads ``$HERMES_HOME/config.yaml`` (falling back to ``~/.hermes/config.yaml`` + when HERMES_HOME is unset) so the value is hermetically testable. Fully + guarded: any error, missing file, missing key, or non-positive value + returns the built-in default. The ``tool_budget:`` block name is shared + with the wider configurable-caps proposal (#80508) so the two can merge + without a key rename. + """ + import os + + try: + import yaml + + home = os.environ.get("HERMES_HOME") or os.path.expanduser("~/.hermes") + path = os.path.join(home, "config.yaml") + if os.path.isfile(path): + with open(path, encoding="utf-8") as fh: + data = yaml.safe_load(fh) or {} + block = data.get("tool_budget") + if isinstance(block, dict): + raw = block.get("mcp_result_size_chars") + if raw is not None: + value = int(raw) + if value > 0: + return value + except Exception: + pass + return DEFAULT_MCP_RESULT_SIZE_CHARS + @dataclass(frozen=True) class BudgetConfig: @@ -32,12 +81,21 @@ class BudgetConfig: default_result_size: int = DEFAULT_RESULT_SIZE_CHARS turn_budget: int = DEFAULT_TURN_BUDGET_CHARS preview_size: int = DEFAULT_PREVIEW_SIZE_CHARS + mcp_result_size: int = DEFAULT_MCP_RESULT_SIZE_CHARS tool_overrides: Dict[str, int] = field(default_factory=dict) def resolve_threshold(self, tool_name: str) -> int | float: """Resolve the persistence threshold for a tool. - Priority: pinned -> tool_overrides -> registry per-tool -> default. + Priority: pinned -> tool_overrides -> mcp_ prefix -> registry + per-tool -> default. + + MCP tools (``mcp_`` prefix) get a tighter default threshold + (``mcp_result_size``, 50K chars) because MCP servers return + un-paginated payloads with no per-tool registry entry to constrain + them. The value is additionally capped at ``default_result_size`` + so a context-scaled budget for a small model still constrains MCP + results the same way it constrains registry values. The registry per-tool value is capped at ``default_result_size`` so a context-scaled budget (small model) actually constrains tools that @@ -50,6 +108,8 @@ class BudgetConfig: return PINNED_THRESHOLDS[tool_name] if tool_name in self.tool_overrides: return self.tool_overrides[tool_name] + if tool_name.startswith(MCP_TOOL_PREFIX): + return min(self.mcp_result_size, self.default_result_size) from tools.registry import registry registry_value = registry.get_max_result_size(tool_name, default=self.default_result_size) if registry_value == float("inf"): @@ -95,8 +155,12 @@ def budget_for_context_window(context_length: int | None) -> BudgetConfig: small models proportionally to their window, floored so a usable preview always survives. """ + mcp_result_size = _configured_mcp_result_size() + if not context_length or context_length <= 0: - return DEFAULT_BUDGET + if mcp_result_size == DEFAULT_MCP_RESULT_SIZE_CHARS: + return DEFAULT_BUDGET + return BudgetConfig(mcp_result_size=mcp_result_size) window_chars = context_length * _CHARS_PER_TOKEN per_result = int(window_chars * _PER_RESULT_WINDOW_FRACTION) @@ -111,4 +175,5 @@ def budget_for_context_window(context_length: int | None) -> BudgetConfig: default_result_size=per_result, turn_budget=per_turn, preview_size=DEFAULT_PREVIEW_SIZE_CHARS, + mcp_result_size=mcp_result_size, ) diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py index 93b4f2585c..08b15dcd42 100644 --- a/tools/mcp_tool.py +++ b/tools/mcp_tool.py @@ -121,6 +121,41 @@ from tools.ansi_strip import strip_unicode_tags logger = logging.getLogger(__name__) + +# Hard allocation ceiling for a single MCP text payload (chars). This is the +# FIRST line of defense against a buggy or malicious MCP server returning +# multi-megabyte text: without it the full payload is allocated, JSON-encoded +# and handed downstream before the budget/spillover layer ever sees it +# (#56059). It deliberately sits far ABOVE the budget layer's 50K MCP +# spillover threshold (tools/budget_config.py) so ordinary large results +# reach spillover INTACT — spilled to disk in full, preview in context — +# while only pathological multi-MB floods are lossy-truncated here. +# +# Distilled from #56060 (Stoltemberg), #56072 (AlexFucuson9) and #56511 +# (Tranquil-Flow), which capped at get_max_bytes() (50K) — correct +# protection, but at that level it would truncate before spillover could +# preserve the data. The 40% head / 60% tail split is #56511's shape. +_MCP_HARD_RESULT_CAP_CHARS = 2_000_000 + + +def _truncate_mcp_text_result(text: str, max_chars: int = _MCP_HARD_RESULT_CAP_CHARS) -> str: + """Bound pathological MCP text before it propagates (#56059). + + Results at or under ``max_chars`` pass through unchanged; oversized text + keeps a 40% head / 60% tail split with an omission notice in between. + """ + if len(text) <= max_chars: + return text + head_chars = int(max_chars * 0.4) + tail_chars = max_chars - head_chars + omitted = len(text) - head_chars - tail_chars + return ( + text[:head_chars] + + f"\n\n... [MCP RESULT TRUNCATED - {omitted:,} chars omitted " + f"out of {len(text):,} total] ...\n\n" + + text[-tail_chars:] + ) + # Upper bound for the OSV malware preflight during stdio MCP startup. The # check makes a blocking urllib HTTPS call whose own timeout can fail to # interrupt a stalled SSL handshake, which froze the asyncio event loop and @@ -5816,7 +5851,9 @@ def _make_tool_handler(server_name: str, tool_name: str, tool_timeout: float): if res_text: error_text += str(res_text) return tool_error(_sanitize_error( - error_text or "MCP tool returned an error" + _truncate_mcp_text_result( + error_text or "MCP tool returned an error" + ) )) # Collect text from content blocks. MCP tool results can also @@ -5868,6 +5905,10 @@ def _make_tool_handler(server_name: str, tool_name: str, tool_timeout: float): ) text_result = "\n".join(parts) if parts else "" + # Hard-cap pathological payloads before they propagate (#56059); + # ordinary large results pass untouched to the spillover layer. + text_result = _truncate_mcp_text_result(text_result) + # Combine content + structuredContent when both are present. # MCP spec: content is model-oriented (text), structuredContent # is machine-oriented (JSON metadata). For an AI agent, content @@ -5885,6 +5926,18 @@ def _make_tool_handler(server_name: str, tool_name: str, tool_timeout: float): # vendor-namespaced keys (`com.example.mcp/...`) pass through — # their semantics belong to the server. structured = mcp_field(result, "structured_content", "structuredContent") + # Cap structuredContent too — a malicious server could flood + # context via a multi-MB JSON payload (#56059). When the + # serialized form exceeds the hard cap, replace it with the + # truncated string (head + tail preserved) so it degrades + # gracefully instead of flooding downstream. + if structured is not None: + try: + _structured_json = json.dumps(structured, ensure_ascii=False, default=str) + except (TypeError, ValueError): + _structured_json = None + if _structured_json is not None and len(_structured_json) > _MCP_HARD_RESULT_CAP_CHARS: + structured = _truncate_mcp_text_result(_structured_json) meta = _strip_reserved_meta_keys(mcp_field(result, "meta", "meta")) if structured is not None or meta is not None: payload: Dict[str, Any] = {} diff --git a/tools/tool_result_storage.py b/tools/tool_result_storage.py index 47bf3799f8..ff731a5b8f 100644 --- a/tools/tool_result_storage.py +++ b/tools/tool_result_storage.py @@ -281,7 +281,12 @@ def _build_persisted_message( msg = f"{PERSISTED_OUTPUT_TAG}\n" msg += f"This tool result was too large ({original_size:,} characters, {size_str}).\n" msg += f"Full output saved to: {file_path}\n" - msg += "Use the read_file tool with offset and limit to access specific sections of this output.\n\n" + msg += "Use the read_file tool with offset and limit to access specific sections of this output.\n" + msg += ( + "Recovery: page through the saved file with read_file (offset/limit) or " + "process it with execute_code — do NOT re-request the same data from the " + "remote API; the full result is already on disk.\n\n" + ) msg += f"Preview (first {len(preview)} chars):\n" msg += preview if has_more: diff --git a/website/docs/user-guide/configuration.md b/website/docs/user-guide/configuration.md index e031cb639e..7b9b43e455 100644 --- a/website/docs/user-guide/configuration.md +++ b/website/docs/user-guide/configuration.md @@ -746,6 +746,21 @@ tool_output: max_lines: 500 ``` +### Tool-Result Spillover Budget + +Separately from truncation, oversized tool *results* are spilled to disk rather than cut: the full output is saved under `$HERMES_HOME/cache/spillover/` and the in-context content is replaced by a preview plus the saved file's path (readable with `read_file` using `offset`/`limit`, or processable with `execute_code`). The generic per-result spillover threshold is 100,000 chars, scaled down automatically for small-context models. + +MCP tool results (tools named `mcp_*`) spill at a tighter **50,000-char** default: MCP servers routinely return large un-paginated payloads (tool-discovery catalogs, batched executions) that would otherwise sit under the generic threshold and bloat context on every subsequent turn. Nothing is lost — the full result is preserved on disk. Override the threshold via: + +```yaml +tool_budget: + mcp_result_size_chars: 50000 # per-result spillover threshold for mcp_* tools +``` + +The MCP threshold is always capped at the (possibly context-scaled) generic per-result threshold, so raising it cannot exceed what the active model's window allows. + +Hermes also flags **provider-side elision**: when an MCP or web tool result embeds its own truncation markers (`...N more items`, `"has_more": true`, "saved to sandbox" notes), a one-line notice is appended to the result warning that the visible data is incomplete and should be paged/fetched before treating any enumeration as complete. + ## Global Toolset Disable To suppress specific toolsets across the CLI and every gateway platform in one From 4a83b03c3ad83ff8770711284b0f059636be3dda Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 19 Aug 2026 16:19:36 -0700 Subject: [PATCH 20/24] fix: read tool_budget config via load_config_readonly (config read guard) --- tools/budget_config.py | 37 ++++++++++++++++--------------------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/tools/budget_config.py b/tools/budget_config.py index c746bdb3b3..ec02576a76 100644 --- a/tools/budget_config.py +++ b/tools/budget_config.py @@ -37,32 +37,27 @@ MCP_TOOL_PREFIX: str = "mcp_" def _configured_mcp_result_size() -> int: - """Read ``tool_budget.mcp_result_size_chars`` from the active config.yaml. + """Read ``tool_budget.mcp_result_size_chars`` from the active config. - Reads ``$HERMES_HOME/config.yaml`` (falling back to ``~/.hermes/config.yaml`` - when HERMES_HOME is unset) so the value is hermetically testable. Fully - guarded: any error, missing file, missing key, or non-positive value - returns the built-in default. The ``tool_budget:`` block name is shared - with the wider configurable-caps proposal (#80508) so the two can merge + Goes through :func:`hermes_cli.config.load_config_readonly` (the + sanctioned read path — raw config.yaml parsing outside owner modules + is guarded by tests/hermes_cli/test_config_read_guard.py). Fully + guarded: any error, missing key, or non-positive value returns the + built-in default. The ``tool_budget:`` block name is shared with the + wider configurable-caps proposal (#80508) so the two can merge without a key rename. """ - import os - try: - import yaml + from hermes_cli.config import load_config_readonly - home = os.environ.get("HERMES_HOME") or os.path.expanduser("~/.hermes") - path = os.path.join(home, "config.yaml") - if os.path.isfile(path): - with open(path, encoding="utf-8") as fh: - data = yaml.safe_load(fh) or {} - block = data.get("tool_budget") - if isinstance(block, dict): - raw = block.get("mcp_result_size_chars") - if raw is not None: - value = int(raw) - if value > 0: - return value + data = load_config_readonly() + block = data.get("tool_budget") if isinstance(data, dict) else None + if isinstance(block, dict): + raw = block.get("mcp_result_size_chars") + if raw is not None: + value = int(raw) + if value > 0: + return value except Exception: pass return DEFAULT_MCP_RESULT_SIZE_CHARS From 2473e568592f3ab06fd8d4b69462303a0d8373bf Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 19 Aug 2026 18:31:57 -0500 Subject: [PATCH 21/24] fix(desktop): stand the terminal overlay down when its tab loses focus MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The persistent terminal is a position:fixed overlay that chases its slot's rect, and the whole tracker — visibility included — was gated behind the renderer pause. Switching tabs while the window is unfocused therefore left the overlay parked over the zone at full opacity with pointerEvents:auto, so the chat underneath was unreachable until something refocused the window. Visibility is correctness rather than perf, so sample it on every wake even while paused; the rect chase, which is the part that forces layout, stays gated. --- .../app/right-sidebar/terminal/persistent.tsx | 33 ++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/apps/desktop/src/app/right-sidebar/terminal/persistent.tsx b/apps/desktop/src/app/right-sidebar/terminal/persistent.tsx index 9f7180a036..f40fcedf6c 100644 --- a/apps/desktop/src/app/right-sidebar/terminal/persistent.tsx +++ b/apps/desktop/src/app/right-sidebar/terminal/persistent.tsx @@ -103,8 +103,25 @@ export function PersistentTerminal({ onAddSelectionToChat }: PersistentTerminalP } } + // Visibility is CORRECTNESS, not perf. The rect chase below forces layout, + // so it stays gated on the renderer pause — but `hidden` is a cheap + // attribute walk, and skipping it while paused strands the overlay over a + // tab the user has already switched away from: the terminal keeps covering + // the chat, opaque and pointer-interactive, until something refocuses the + // window. Sample it on every wake, paused or not. + const syncHidden = () => { + const hidden = isElementInHiddenPane(slot) + + if (prev && prev.hidden !== hidden) { + prev = { ...prev, hidden } + setRect(prev) + } + } + const measure = (reason: string): boolean => { if (rendererPaused()) { + syncHidden() + return false } @@ -140,7 +157,20 @@ export function PersistentTerminal({ onAddSelectionToChat }: PersistentTerminalP } const scheduleMeasure = (reason = 'unknown') => { - if (stopped || rendererPaused() || frame !== 0) { + if (stopped) { + return + } + + // Paused: no frame is coming (and none is wanted — the rect chase is the + // expensive half). Still settle visibility synchronously so a tab switch + // while the window is unfocused can't leave the overlay stranded. + if (rendererPaused()) { + syncHidden() + + return + } + + if (frame !== 0) { return } @@ -158,6 +188,7 @@ export function PersistentTerminal({ onAddSelectionToChat }: PersistentTerminalP const handleVisibilityChange = () => { if (rendererPaused()) { cancelFrame() + syncHidden() return } From 7f3d2559312fcb6991bc98037f20c3f5f33f7444 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 19 Aug 2026 18:31:57 -0500 Subject: [PATCH 22/24] test(desktop): cover the terminal overlay hiding on an unfocused tab switch --- .../terminal/persistent.test.tsx | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/apps/desktop/src/app/right-sidebar/terminal/persistent.test.tsx b/apps/desktop/src/app/right-sidebar/terminal/persistent.test.tsx index 5cf40b6ee6..ab4155823b 100644 --- a/apps/desktop/src/app/right-sidebar/terminal/persistent.test.tsx +++ b/apps/desktop/src/app/right-sidebar/terminal/persistent.test.tsx @@ -365,4 +365,41 @@ describe('PersistentTerminal rect tracking', () => { expect(overlay.style.pointerEvents).toBe('auto') expect(mount.container!.querySelector('[data-testid="terminal-workspace"]')).toBe(workspace) }) + + it('hides the overlay on a tab switch that happens while the window is unfocused', () => { + // The trap: the terminal is a tab in the main zone and the user clicks + // another tab without the window being focused (or right as it blurs). + // The rect chase is paused then — but visibility is correctness, not perf, + // so the overlay must still stand down instead of covering the chat with + // an opaque, pointer-interactive surface until something refocuses. + installRaf() + vi.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockReturnValue(rect(10, 20, 200, 100)) + $terminalTakeover.set(true) + + mount.render(