From 0ab90040adc3388f18641dc8e056bd5961ac2e75 Mon Sep 17 00:00:00 2001 From: HexLab98 Date: Sun, 12 Jul 2026 06:54:56 +0700 Subject: [PATCH 01/47] fix(config): preserve platforms on partial save_config writes (#62723) Add merge_existing to save_config (default False for full-document callers like the dashboard YAML editor) and route partial writes through _merge_partial_save. _persist_migration writes the full migrated dict directly so deleted keys are not resurrected from the on-disk file. --- hermes_cli/config.py | 46 +++++++++++++++++++++++++++++++++++----- hermes_cli/web_server.py | 4 +++- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 2e497b12d..a47ba24ea 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -5524,10 +5524,13 @@ def _persist_migration(config: Dict[str, Any]) -> None: Every migration step MUST route its write through this helper instead of calling ``save_config`` directly. It is a thin wrapper over - ``save_config(config)`` (default-stripping ON); centralising the call makes - the invariant impossible to regress one migration at a time. Correctness - across seeds, non-default values, behaviour flips, and data transforms is - verified by the migration parity tests. + ``save_config(config)`` (default-stripping ON, no ``merge_existing``); + centralising the call makes the invariant impossible to regress one + migration at a time. Callers must pass the full raw config returned by + ``read_raw_config()`` after in-place mutations (including key removals); + deep-merging the on-disk file back in would resurrect keys the migration + just deleted. Partial-save preservation for unrelated top-level sections + belongs on ``save_config(..., merge_existing=True)``, not here. """ save_config(config) @@ -6302,6 +6305,25 @@ def migrate_config(interactive: bool = True, quiet: bool = False) -> Dict[str, A return results +def _merge_partial_save(raw: dict, override: dict) -> dict: + """Merge *override* over *raw* for partial ``save_config`` writes. + + Top-level sections omitted from *override* are preserved from *raw*. + Shared top-level dict sections are deep-merged so a caller can update one + nested key without dropping sibling keys from disk. Intentional key + removals within a section are not supported here — migration writes must + route through ``_persist_migration`` with a full ``read_raw_config()`` dict + instead. + """ + result = copy.deepcopy(override) + for key, value in raw.items(): + if key not in result: + result[key] = copy.deepcopy(value) + elif isinstance(result.get(key), dict) and isinstance(value, dict): + result[key] = _deep_merge(value, result[key]) + return result + + def _deep_merge(base: dict, override: dict) -> dict: """Recursively merge *override* into *base*, preserving nested defaults. @@ -7184,6 +7206,7 @@ def save_config( *, strip_defaults: bool = True, preserve_keys: Optional[Set[Tuple[str, ...]]] = None, + merge_existing: bool = False, ): """Save configuration to ~/.hermes/config.yaml.\n @@ -7192,6 +7215,12 @@ def save_config( before any normalisation). This prevents config.yaml from being contaminated with schema defaults on every save, which makes future default changes invisible to users. + + When ``merge_existing`` is True, the on-disk raw config is deep-merged + under *config* before writing so partial callers (migration steps via + ``_persist_migration``) cannot drop unrelated sections the caller omitted. + Full-document replacement callers (dashboard raw YAML editor, callers that + already deep-merge) must leave this False so intentional deletions survive. """ with _CONFIG_LOCK: if is_managed(): @@ -7226,11 +7255,17 @@ def save_config( explicit_raw_paths: Optional[Set[Tuple[str, ...]]] = ( _explicit_config_paths(_raw_for_paths) if _raw_for_paths else None ) + if merge_existing and _raw_for_paths: + config = _merge_partial_save(_raw_for_paths, config) # ---------------------------------------------------------------- current_normalized = _normalize_root_model_keys(_normalize_max_turns_config(config)) normalized = current_normalized - raw_existing = _normalize_root_model_keys(_normalize_max_turns_config(read_raw_config())) + raw_existing = ( + _normalize_root_model_keys(_normalize_max_turns_config(_raw_for_paths)) + if _raw_for_paths + else {} + ) if raw_existing: normalized = _preserve_env_ref_templates( normalized, @@ -7278,6 +7313,7 @@ def save_config( extra_content="".join(parts) if parts else None, ) _secure_file(config_path) + _RAW_CONFIG_CACHE.pop(str(config_path), None) _LAST_EXPANDED_CONFIG_BY_PATH[str(config_path)] = copy.deepcopy(current_normalized) diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 175ab7550..406e18f5c 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -14148,7 +14148,9 @@ async def update_config_raw(body: RawConfigUpdate, profile: Optional[str] = None if not isinstance(parsed, dict): raise HTTPException(status_code=400, detail="YAML must be a mapping") with _profile_scope(body.profile or profile): - save_config(parsed) + # Full-document replacement: the editor owns the whole file; do not + # merge omitted sections back from disk (#62723). + save_config(parsed, merge_existing=False) return {"ok": True} except yaml.YAMLError as e: raise HTTPException(status_code=400, detail=f"Invalid YAML: {e}") From 1b059d1ae7a70a812889077b027bc8243ac9345c Mon Sep 17 00:00:00 2001 From: HexLab98 Date: Sun, 12 Jul 2026 06:54:56 +0700 Subject: [PATCH 02/47] test(config): regression for merge_existing partial saves (#62723) --- tests/hermes_cli/test_config.py | 108 ++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/tests/hermes_cli/test_config.py b/tests/hermes_cli/test_config.py index 0b1f1404b..cc1d0d361 100644 --- a/tests/hermes_cli/test_config.py +++ b/tests/hermes_cli/test_config.py @@ -1645,6 +1645,114 @@ class TestMigrationWriteInvariant: assert loaded["display"]["compact"] == DEFAULT_CONFIG["display"]["compact"] +class TestSaveConfigPartialWritePreservation: + """Regression for #62723: partial migration writes must not drop unrelated sections.""" + + def test_merge_existing_preserves_platforms_on_partial_write(self, tmp_path): + body = """_config_version: 30 +model: + default: deepseek-v4-pro + provider: deepseek +agent: + max_turns: 60 +platforms: + feishu: + enabled: true + extra: + app_id: cli_xxx + app_secret: xxx +feishu: + require_mention: true +""" + (tmp_path / "config.yaml").write_text(body, encoding="utf-8") + with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}): + save_config( + { + "_config_version": 30, + "model": {"default": "deepseek-v4-pro", "provider": "deepseek"}, + "agent": {"max_turns": 60, "verify_on_stop": False}, + }, + merge_existing=True, + ) + raw = yaml.safe_load((tmp_path / "config.yaml").read_text(encoding="utf-8")) + + assert raw["platforms"]["feishu"]["extra"]["app_id"] == "cli_xxx" + assert raw["feishu"]["require_mention"] is True + assert raw["agent"]["verify_on_stop"] is False + + def test_partial_write_without_merge_drops_omitted_sections(self, tmp_path): + """Full-replacement callers (raw YAML editor) rely on merge_existing=False.""" + body = """_config_version: 30 +model: + default: deepseek-v4-pro + provider: deepseek +platforms: + feishu: + enabled: true +""" + (tmp_path / "config.yaml").write_text(body, encoding="utf-8") + with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}): + save_config({"model": {"default": "other-model", "provider": "openrouter"}}) + raw = yaml.safe_load((tmp_path / "config.yaml").read_text(encoding="utf-8")) + + assert raw["model"]["default"] == "other-model" + assert "platforms" not in raw + + def test_persist_migration_writes_full_read_raw_config(self, tmp_path): + from hermes_cli.config import _persist_migration, read_raw_config + + body = """_config_version: 30 +model: + default: deepseek-v4-pro + provider: deepseek +agent: + max_turns: 60 +platforms: + feishu: + enabled: true + extra: + app_id: cli_xxx + app_secret: xxx +""" + (tmp_path / "config.yaml").write_text(body, encoding="utf-8") + with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}): + config = read_raw_config() + config.setdefault("agent", {})["verify_on_stop"] = False + config["_config_version"] = 32 + _persist_migration(config) + raw = yaml.safe_load((tmp_path / "config.yaml").read_text(encoding="utf-8")) + + assert raw["platforms"]["feishu"]["extra"]["app_id"] == "cli_xxx" + assert raw["agent"]["verify_on_stop"] is False + assert raw["agent"]["max_turns"] == 60 + assert raw["_config_version"] == 32 + + def test_v30_to_latest_migration_keeps_platforms(self, tmp_path): + """End-to-end: reporter's v30 feishu profile survives version bump.""" + body = """_config_version: 30 +model: + default: deepseek-v4-pro + provider: deepseek +agent: + max_turns: 60 +platforms: + feishu: + enabled: true + extra: + app_id: cli_xxx + app_secret: xxx +feishu: + require_mention: true +""" + (tmp_path / "config.yaml").write_text(body, encoding="utf-8") + with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}): + migrate_config(interactive=False, quiet=True) + raw = yaml.safe_load((tmp_path / "config.yaml").read_text(encoding="utf-8")) + + assert raw["platforms"]["feishu"]["extra"]["app_id"] == "cli_xxx" + assert raw["feishu"]["require_mention"] is True + + class TestVerifyOnStopMigration: """v30 → v31: switch verify_on_stop OFF once, preserving explicit choices.""" From 8fa8aabbbbb1f5f65ad44d747527dfcd9b7a4866 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 15 Jul 2026 04:27:32 -0700 Subject: [PATCH 03/47] test(codex): pin codex_backend issuer in xai-scoped salvage test (#64844) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test_normalize_codex_response_salvage_is_xai_scoped broke on main when two same-day merges crossed: #64764 (#64434 — trust response.status for reasoning-only turns on UNRECOGNIZED Responses backends) changed what a bare _normalize_codex_response(response) call returns for status='completed' reasoning-only output (now 'stop'), while #64768 added this test calling with no issuer_kind and expecting 'incomplete'. The test's intent is that the xAI reasoning-channel salvage does not leak into other special-cased backends — pin issuer_kind='codex_backend' so it exercises exactly that (same pattern as test_normalize_codex_response_treats_summary_only_reasoning_as_incomplete, which was already pinned for #64434). --- tests/agent/test_codex_responses_adapter.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/tests/agent/test_codex_responses_adapter.py b/tests/agent/test_codex_responses_adapter.py index 4a14cadf4..cbf2f0e58 100644 --- a/tests/agent/test_codex_responses_adapter.py +++ b/tests/agent/test_codex_responses_adapter.py @@ -493,14 +493,21 @@ def test_normalize_codex_response_salvage_strips_closing_tag(): def test_normalize_codex_response_salvage_is_xai_scoped(): - """Non-xAI issuers keep the reasoning-only → incomplete classification; - the Codex backend replays encrypted reasoning, so its continuation - genuinely progresses and must not be short-circuited.""" + """Non-xAI special-cased issuers (Codex backend) keep the reasoning-only → + incomplete classification; the Codex backend replays encrypted reasoning, + so its continuation genuinely progresses and must not be short-circuited. + + Pins ``issuer_kind="codex_backend"`` explicitly: with no issuer at all, + the unrecognized-backend rule (#64434) trusts ``status="completed"`` and + returns ``stop`` — that path is covered by the #64434 regression tests. + """ response = _xai_reasoning_only_response( "Thinking.\nThe answer." ) - assistant_message, finish_reason = _normalize_codex_response(response) + assistant_message, finish_reason = _normalize_codex_response( + response, issuer_kind="codex_backend" + ) assert finish_reason == "incomplete" assert assistant_message.content == "" From 779c0dd80d47a9f552990f5789749db16f7a7d63 Mon Sep 17 00:00:00 2001 From: Frowtek Date: Sat, 11 Jul 2026 12:22:56 +0300 Subject: [PATCH 04/47] fix(compressor): unwrap web_extract dict URLs in tool-result summaries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _summarize_tool_result() built the web_extract summary straight from the first `urls` entry. When web_search results are forwarded into web_extract (a common chain), that entry is a dict ({"url"/"href": ...}) rather than a URL string. With 2+ URLs the `url_desc += " (+N more)"` step then raised `TypeError: unsupported operand type(s) for +=: 'dict' and 'str'`, aborting the pre-compression pruning pass; with a single URL the raw dict repr leaked into the summary text. Unwrap the URL from dict entries before use — mirroring the existing web_extract handling in agent/display.py (`_display_url`), tools/web_tools.py (`_web_extract_url`) and acp_adapter/tools.py (`build_tool_title`) — so `url_desc` is always a string and the concatenation is always str + str. Adds regression tests covering multiple/single dict URLs, the href key, malformed dicts, and the plain-string path. --- agent/context_compressor.py | 11 ++++++- tests/agent/test_context_compressor.py | 45 ++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/agent/context_compressor.py b/agent/context_compressor.py index fb998a1db..5ff794acd 100644 --- a/agent/context_compressor.py +++ b/agent/context_compressor.py @@ -686,7 +686,16 @@ def _summarize_tool_result_unguarded(tool_name: str, tool_args: str, tool_conten if tool_name == "web_extract": urls = args.get("urls", []) - url_desc = urls[0] if isinstance(urls, list) and urls else "?" + first = urls[0] if isinstance(urls, list) and urls else "?" + # web_search results are dicts ({"url"/"href": ...}) and models often + # forward them straight into web_extract. Unwrap to the URL string so + # the summary stays readable and the ``+=`` below never hits the + # ``dict + str`` TypeError that would abort pre-compression pruning. + if isinstance(first, dict): + first = first.get("url") or first.get("href") or "?" + elif not isinstance(first, str): + first = "?" + url_desc = first if isinstance(urls, list) and len(urls) > 1: url_desc += f" (+{len(urls) - 1} more)" return f"[web_extract] {url_desc} ({content_len:,} chars)" diff --git a/tests/agent/test_context_compressor.py b/tests/agent/test_context_compressor.py index 230090fd6..9a120ba94 100644 --- a/tests/agent/test_context_compressor.py +++ b/tests/agent/test_context_compressor.py @@ -1,5 +1,6 @@ """Tests for agent/context_compressor.py — compression logic, thresholds, truncation fallback.""" +import json import pytest import time from unittest.mock import patch, MagicMock @@ -9,6 +10,7 @@ from agent.context_compressor import ( HISTORICAL_TASK_HEADING, SUMMARY_PREFIX, COMPRESSED_SUMMARY_METADATA_KEY, + _summarize_tool_result, ) from hermes_state import SessionDB @@ -27,6 +29,49 @@ def compressor(): return c +class TestSummarizeToolResultWebExtract: + """Pre-compression pruning must survive web_extract calls whose ``urls`` are + web_search result dicts ({"url"/"href": ...}), which models routinely forward + straight into web_extract. + """ + + CONTENT = "x" * 500 # >200 chars so the pruning pass actually summarizes + + def test_multiple_dict_urls_do_not_crash(self): + # Two dict URLs previously hit ``dict + str`` -> TypeError, aborting + # _prune_old_tool_results() (and thus compress()). + args = json.dumps({ + "urls": [ + {"url": "https://example.com/a", "title": "A"}, + {"url": "https://example.org/b", "title": "B"}, + ] + }) + summary = _summarize_tool_result("web_extract", args, self.CONTENT) + assert summary == "[web_extract] https://example.com/a (+1 more) (500 chars)" + + def test_single_dict_url_is_unwrapped_not_stringified(self): + args = json.dumps({"urls": [{"url": "https://example.com/a", "title": "A"}]}) + summary = _summarize_tool_result("web_extract", args, self.CONTENT) + assert summary == "[web_extract] https://example.com/a (500 chars)" + assert "{" not in summary # no raw dict repr leaked into the summary + + def test_href_key_is_unwrapped(self): + args = json.dumps({"urls": [{"href": "https://example.com/h"}]}) + summary = _summarize_tool_result("web_extract", args, self.CONTENT) + assert summary == "[web_extract] https://example.com/h (500 chars)" + + def test_malformed_dict_falls_back_to_placeholder(self): + args = json.dumps({"urls": [{"title": "no url here"}, {"title": "still none"}]}) + summary = _summarize_tool_result("web_extract", args, self.CONTENT) + assert summary == "[web_extract] ? (+1 more) (500 chars)" + + def test_plain_string_urls_unchanged(self): + # Regression guard: the normal (already-working) string path is intact. + args = json.dumps({"urls": ["https://example.com/a", "https://example.org/b"]}) + summary = _summarize_tool_result("web_extract", args, self.CONTENT) + assert summary == "[web_extract] https://example.com/a (+1 more) (500 chars)" + + class TestShouldCompress: def test_below_threshold(self, compressor): compressor.last_prompt_tokens = 50000 From c2a3b9ce58f1300b5236d4278b7deee4d2878938 Mon Sep 17 00:00:00 2001 From: liuhao1024 Date: Sat, 13 Jun 2026 12:10:05 +0800 Subject: [PATCH 05/47] fix(state): use PASSIVE checkpoint for periodic WAL flush to prevent B-tree corruption TRUNCATE checkpoint every 50 writes causes B-tree corruption on large databases (65K+ pages) due to the exclusive-lock I/O pressure from checkpointing thousands of frames at once. Switch periodic _try_wal_checkpoint() to PASSIVE mode which does not require an exclusive lock and cannot corrupt pages under I/O pressure. Keep TRUNCATE in close() and pre-VACUUM paths where it is safe (infrequent, controlled conditions). Also replace silent `except Exception: pass` with logged warnings for checkpoint failures so operators can detect early corruption signals. Fixes #45383 --- hermes_state.py | 41 ++++---- tests/test_wal_checkpoint_strategy.py | 138 ++++++++++++++++++++++++++ 2 files changed, 158 insertions(+), 21 deletions(-) create mode 100644 tests/test_wal_checkpoint_strategy.py diff --git a/hermes_state.py b/hermes_state.py index 7181266fe..eafefc2ea 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -986,7 +986,7 @@ class SessionDB: _WRITE_MAX_RETRIES = 15 _WRITE_RETRY_MIN_S = 0.020 # 20ms _WRITE_RETRY_MAX_S = 0.150 # 150ms - # Attempt a PASSIVE WAL checkpoint every N successful writes. + # Attempt a WAL checkpoint every N successful writes (PASSIVE mode). _CHECKPOINT_EVERY_N_WRITES = 50 # Merge fragmented FTS5 segments every N successful writes. The message # triggers append one segment per insert; left unmaintained these grow @@ -1301,35 +1301,34 @@ class SessionDB: ) def _try_wal_checkpoint(self) -> None: - """Best-effort TRUNCATE WAL checkpoint. Never raises. + """Best-effort PASSIVE WAL checkpoint. Never raises. - Flushes committed WAL frames back into the main DB file and - truncates the WAL file to zero bytes. Keeps the WAL from - growing unbounded when many processes hold persistent - connections. + Flushes committed WAL frames back into the main DB file without + requiring an exclusive lock. PASSIVE is safe for frequent + periodic use because it does not block concurrent writers and + cannot corrupt B-tree pages under I/O pressure. - PASSIVE checkpoint was previously used here, but it never - truncates the WAL file — the file stays at its high-water - mark until an explicit TRUNCATE is called (which only - happened inside the infrequent vacuum()). + PASSIVE does not truncate the WAL file — it stays at its + high-water mark. WAL truncation happens in :meth:`close` + (TRUNCATE) and pre-VACUUM checkpoints, which run infrequently + under controlled conditions. - TRUNCATE may block writers briefly while checkpointing, but - _try_wal_checkpoint is called off the hot path (every 50 - writes) and already runs under ``self._lock``, so the - additional hold time is negligible. + Previous TRUNCATE strategy caused B-tree corruption on large + databases (65K+ pages) due to the exclusive-lock I/O pressure + from checkpointing thousands of frames at once (issue #45383). """ try: with self._lock: result = self._conn.execute( - "PRAGMA wal_checkpoint(TRUNCATE)" + "PRAGMA wal_checkpoint(PASSIVE)" ).fetchone() if result and result[1] > 0: logger.debug( "WAL checkpoint: %d/%d pages checkpointed", result[2], result[1], ) - except Exception: - pass # Best effort — never fatal. + except Exception as exc: + logger.warning("WAL checkpoint (PASSIVE) failed: %s", exc) def _try_optimize_fts(self) -> None: """Best-effort FTS5 segment merge. Never raises. @@ -1357,8 +1356,8 @@ class SessionDB: if self._conn: try: self._conn.execute("PRAGMA wal_checkpoint(TRUNCATE)") - except Exception: - pass + except Exception as exc: + logger.debug("WAL checkpoint (TRUNCATE) at close failed: %s", exc) self._conn.close() self._conn = None @@ -7015,8 +7014,8 @@ class SessionDB: # Best-effort WAL checkpoint first, then VACUUM. try: self._conn.execute("PRAGMA wal_checkpoint(TRUNCATE)") - except Exception: - pass + except Exception as exc: + logger.debug("WAL checkpoint (TRUNCATE) before VACUUM failed: %s", exc) self._conn.execute("VACUUM") return optimized diff --git a/tests/test_wal_checkpoint_strategy.py b/tests/test_wal_checkpoint_strategy.py new file mode 100644 index 000000000..880aa4b5c --- /dev/null +++ b/tests/test_wal_checkpoint_strategy.py @@ -0,0 +1,138 @@ +"""Tests for SessionDB WAL checkpoint strategy (issue #45383). + +Verifies that periodic checkpoints use PASSIVE mode (safe for large DBs) +while close() and pre-VACUUM paths still use TRUNCATE. +""" + +import sqlite3 +import logging +from unittest.mock import MagicMock, patch + +import pytest + +from hermes_state import SessionDB + + +@pytest.fixture() +def db(tmp_path): + """Create a SessionDB with a temp database file.""" + db_path = tmp_path / "test_state.db" + session_db = SessionDB(db_path=db_path) + yield session_db + try: + session_db.close() + except Exception: + pass + + +class TestTryWalCheckpointPassive: + """_try_wal_checkpoint() should use PASSIVE mode for periodic use.""" + + def test_checkpoint_uses_passive_mode(self, db): + """PASSIVE checkpoint does not require exclusive lock — safe for large DBs.""" + # Capture the real connection's execute before mocking + real_conn = db._conn + execute_calls = [] + + def tracking_execute(sql, *args, **kwargs): + execute_calls.append(sql) + return real_conn.execute(sql, *args, **kwargs) + + # sqlite3.Connection.execute is read-only (C extension) — replace _conn + mock_conn = MagicMock() + mock_conn.execute.side_effect = tracking_execute + mock_conn.fetchone.return_value = None + db._conn = mock_conn + + db._try_wal_checkpoint() + + passive_calls = [c for c in execute_calls if "wal_checkpoint(PASSIVE)" in c] + truncate_calls = [c for c in execute_calls if "wal_checkpoint(TRUNCATE)" in c] + assert len(passive_calls) == 1, ( + f"Expected 1 PASSIVE checkpoint call, got {len(passive_calls)}" + ) + assert len(truncate_calls) == 0, ( + "Periodic checkpoint should NOT use TRUNCATE" + ) + + def test_checkpoint_logs_warning_on_failure(self, db, caplog): + """Failed PASSIVE checkpoint logs a warning instead of silent pass.""" + mock_conn = MagicMock() + mock_conn.execute.side_effect = sqlite3.OperationalError("disk I/O error") + db._conn = mock_conn + + with caplog.at_level(logging.WARNING): + db._try_wal_checkpoint() + + assert any("WAL checkpoint (PASSIVE) failed" in r.message for r in caplog.records), ( + f"Expected warning log about PASSIVE checkpoint failure, got: {caplog.text}" + ) + + def test_checkpoint_returns_result_on_success(self, db): + """Successful PASSIVE checkpoint does not raise.""" + db._try_wal_checkpoint() + + +class TestCloseUsesTruncate: + """close() should still use TRUNCATE to shrink WAL on shutdown.""" + + def test_close_uses_truncate_mode(self, db): + """TRUNCATE at close is safe — no concurrent writers during shutdown.""" + real_conn = db._conn + execute_calls = [] + + def tracking_execute(sql, *args, **kwargs): + execute_calls.append(sql) + return real_conn.execute(sql, *args, **kwargs) + + mock_conn = MagicMock() + mock_conn.execute.side_effect = tracking_execute + db._conn = mock_conn + + db.close() + + truncate_calls = [c for c in execute_calls if "wal_checkpoint(TRUNCATE)" in c] + assert len(truncate_calls) == 1, ( + f"Expected 1 TRUNCATE checkpoint at close, got {len(truncate_calls)}" + ) + + def test_close_logs_debug_on_failure(self, db, caplog): + """Failed TRUNCATE at close logs debug (not warning — close is best-effort).""" + mock_conn = MagicMock() + mock_conn.execute.side_effect = sqlite3.OperationalError("database is locked") + db._conn = mock_conn + + with caplog.at_level(logging.DEBUG): + db.close() + + assert any("WAL checkpoint (TRUNCATE) at close failed" in r.message for r in caplog.records), ( + f"Expected debug log about TRUNCATE failure at close, got: {caplog.text}" + ) + + +class TestCheckpointFrequency: + """Checkpoint triggers every N writes.""" + + def test_checkpoint_triggers_at_interval(self, db): + """_try_wal_checkpoint is called every _CHECKPOINT_EVERY_N_WRITES writes.""" + call_count = [0] + original = db._try_wal_checkpoint + + def counting_checkpoint(): + call_count[0] += 1 + original() + + db._try_wal_checkpoint = counting_checkpoint + + # Write exactly _CHECKPOINT_EVERY_N_WRITES sessions to trigger one checkpoint + n = db._CHECKPOINT_EVERY_N_WRITES + import time as _time + for i in range(n): + db._execute_write(lambda conn, _i=i: conn.execute( + "INSERT INTO sessions (id, source, started_at) VALUES (?, ?, ?)", + (f"sess_{_i}", "test", _time.time()), + )) + + assert call_count[0] == 1, ( + f"Expected 1 checkpoint after {n} writes, got {call_count[0]}" + ) From bcd7e2ce8999a48e887b50a1a37b6b7812eaa1d7 Mon Sep 17 00:00:00 2001 From: Ahmett101 <297889955+Ahmett101@users.noreply.github.com> Date: Tue, 14 Jul 2026 21:54:24 +0300 Subject: [PATCH 06/47] fix(agent): add finite hard ceiling on openai-codex request time (#64507) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A large Codex request (estimated context >= 10k tokens) disables the no-byte TTFB watchdog on purpose, and openai_codex_stale_timeout_floor *raises* the stale timeout (up to 1200s at >100k tokens) so healthy gateway-scale payloads aren't aborted mid-prefill. When the backend genuinely stalls — no first byte AND no events, exactly the #64507 symptom — the request is only reclaimed at that high stale floor, so the session can hang for 13+ minutes with an idle slash_worker and no ended_at/end_reason while Desktop still shows it as active. Add a flat, finite hard ceiling on total openai-codex request time that always applies (min() of the computed stale timeout and the ceiling) regardless of the TTFB-disable / stale-floor interaction. A stalled large request is now killed at the ceiling and the retry loop / visible failure path takes over instead of hanging indefinitely. Tunable via HERMES_CODEX_HARD_TIMEOUT_SECONDS (default 600s; 0 disables the ceiling to restore pre-fix behavior). Closes #64507 --- agent/chat_completion_helpers.py | 22 ++++ tests/agent/test_codex_ttfb_watchdog.py | 140 ++++++++++++++++++++++++ 2 files changed, 162 insertions(+) diff --git a/agent/chat_completion_helpers.py b/agent/chat_completion_helpers.py index 6615dace6..561c2ce37 100644 --- a/agent/chat_completion_helpers.py +++ b/agent/chat_completion_helpers.py @@ -504,6 +504,28 @@ def interruptible_api_call(agent, api_kwargs: dict): if _codex_floor: _stale_timeout = max(_stale_timeout, _codex_floor) + # ── Codex absolute hard ceiling (#64507) ────────────────────────── + # For large Codex requests the no-byte TTFB watchdog is intentionally + # disabled (so legitimate backend admission / prefill isn't killed), and + # ``openai_codex_stale_timeout_floor`` *raises* the stale timeout (up to + # 1200s at >100k tokens) so healthy gateway-scale payloads aren't aborted. + # That combination means a request that is genuinely stalled at the + # backend — no first byte AND no events, exactly the issue-64507 symptom — + # is only reclaimed at the (high) stale floor, so a session can hang for + # many minutes with an idle worker and no ended_at. Add a flat, finite + # hard ceiling on total request time that ALWAYS applies to openai-codex + # requests regardless of the TTFB/stale interaction, so a stalled large + # request is recovered (retry loop / visible failure) instead of hanging + # indefinitely. Tunable via HERMES_CODEX_HARD_TIMEOUT_SECONDS (set to 0 to + # disable the ceiling entirely; that restores the pre-fix behavior). + _codex_hard_timeout = _env_float("HERMES_CODEX_HARD_TIMEOUT_SECONDS", 600.0) + if ( + _codex_watchdog_enabled + and _openai_codex_backend + and _codex_hard_timeout > 0 + ): + _stale_timeout = min(_stale_timeout, _codex_hard_timeout) + if _est_tokens_for_codex_watchdog > 100_000: _codex_idle_timeout_default = 180.0 elif _est_tokens_for_codex_watchdog > 50_000: diff --git a/tests/agent/test_codex_ttfb_watchdog.py b/tests/agent/test_codex_ttfb_watchdog.py index 283a6a8e9..6a647164f 100644 --- a/tests/agent/test_codex_ttfb_watchdog.py +++ b/tests/agent/test_codex_ttfb_watchdog.py @@ -462,3 +462,143 @@ def test_large_codex_request_strict_ttfb_env_still_reconnects(tmp_path, monkeypa assert "codex_ttfb_kill" in closes finally: stop["flag"] = True + + +def test_large_codex_request_hard_ceiling_reclaims_silent_stall(tmp_path, monkeypatch): + """#64507 regression: a large Codex request (TTFB watchdog disabled by the + size gate, stale floor *raised*) that never emits a single byte must still + be reclaimed at a finite hard ceiling — not hang for 13+ minutes while the + worker stays idle and the session shows as active. + + Uses the real default TTFB threshold (120s) and asserts the request dies at + the hard ceiling regardless of the size-based TTFB disable. + """ + from agent import chat_completion_helpers as h + + agent = _make_codex_agent(tmp_path, monkeypatch) + # Real default TTFB threshold (no HERMES_CODEX_TTFB_* override) → for a + # >10k-token request the no-byte TTFB watchdog is auto-disabled. + monkeypatch.setenv("HERMES_CODEX_HARD_TIMEOUT_SECONDS", "3") + + closes: list = [] + dummy_client = SimpleNamespace() + monkeypatch.setattr(agent, "_create_request_openai_client", lambda **k: dummy_client) + monkeypatch.setattr( + agent, "_abort_request_openai_client", + lambda c, reason=None: closes.append(reason), + ) + monkeypatch.setattr( + agent, "_close_request_openai_client", + lambda c, reason=None: closes.append(reason), + ) + + stop = {"flag": False} + + def fake_hang(api_kwargs, client=None, on_first_delta=None): + # No event marker AND no event ever: the exact issue-64507 stall. + deadline = time.time() + 120 + while time.time() < deadline and not stop["flag"] and not agent._interrupt_requested: + time.sleep(0.02) + raise RuntimeError("connection closed") + + monkeypatch.setattr(agent, "_run_codex_stream", fake_hang) + + large_input = "x" * 44_000 # ~11k estimated tokens → TTFB disabled, stale raised + t0 = time.time() + try: + with pytest.raises(TimeoutError) as excinfo: + h.interruptible_api_call(agent, {"model": "gpt-5.5", "input": large_input}) + elapsed = time.time() - t0 + # Must die at the hard ceiling (3s), nowhere near the raised stale floor. + assert elapsed < 30, f"hard ceiling took {elapsed:.1f}s — stall not reclaimed" + assert "stale_call_kill" in closes, f"stale kill expected, got {closes}" + assert "timed out after" in str(excinfo.value) + assert "with no response" in str(excinfo.value) + finally: + stop["flag"] = True + + +def test_large_codex_request_hard_ceiling_disabled_restores_legacy(tmp_path, monkeypatch): + """Setting HERMES_CODEX_HARD_TIMEOUT_SECONDS=0 disables the ceiling entirely, + restoring the pre-#64507 behavior (request waits out the raised stale floor + instead of being capped). Keeps the knob for operators who must. + """ + from agent import chat_completion_helpers as h + + agent = _make_codex_agent(tmp_path, monkeypatch) + monkeypatch.setenv("HERMES_CODEX_HARD_TIMEOUT_SECONDS", "0") + + closes: list = [] + dummy_client = SimpleNamespace() + monkeypatch.setattr(agent, "_create_request_openai_client", lambda **k: dummy_client) + monkeypatch.setattr( + agent, "_abort_request_openai_client", + lambda c, reason=None: closes.append(reason), + ) + monkeypatch.setattr( + agent, "_close_request_openai_client", + lambda c, reason=None: closes.append(reason), + ) + + sentinel = SimpleNamespace(ok=True) + + def fake_stream(api_kwargs, client=None, on_first_delta=None): + # No event, but only briefly — well under the (here 60s) stale timeout. + time.sleep(2.0) + return sentinel + + monkeypatch.setattr(agent, "_run_codex_stream", fake_stream) + + large_input = "x" * 44_000 + resp = h.interruptible_api_call(agent, {"model": "gpt-5.5", "input": large_input}) + assert resp is sentinel + assert "codex_ttfb_kill" not in closes + assert "stale_call_kill" not in closes + + +def test_large_codex_request_hard_ceiling_caps_raised_stale_floor(tmp_path, monkeypatch): + """The hard ceiling must cap the raised stale floor (openai-codex can push + the stale timeout to 1200s at >100k tokens). A large silent stall must die + at the ceiling, proving the min() wins over the floor. + """ + from agent import chat_completion_helpers as h + + agent = _make_codex_agent(tmp_path, monkeypatch) + monkeypatch.setenv("HERMES_CODEX_HARD_TIMEOUT_SECONDS", "4") + # Force the >100k-token tier so openai_codex_stale_timeout_floor returns 1200s. + monkeypatch.setattr( + agent, "_compute_non_stream_stale_timeout", lambda *a, **k: 1200.0 + ) + + closes: list = [] + dummy_client = SimpleNamespace() + monkeypatch.setattr(agent, "_create_request_openai_client", lambda **k: dummy_client) + monkeypatch.setattr( + agent, "_abort_request_openai_client", + lambda c, reason=None: closes.append(reason), + ) + monkeypatch.setattr( + agent, "_close_request_openai_client", + lambda c, reason=None: closes.append(reason), + ) + + stop = {"flag": False} + + def fake_hang(api_kwargs, client=None, on_first_delta=None): + deadline = time.time() + 200 + while time.time() < deadline and not stop["flag"] and not agent._interrupt_requested: + time.sleep(0.02) + raise RuntimeError("connection closed") + + monkeypatch.setattr(agent, "_run_codex_stream", fake_hang) + + huge_input = "x" * 500_000 # ~125k tokens → stale floor 1200s + t0 = time.time() + try: + with pytest.raises(TimeoutError): + h.interruptible_api_call(agent, {"model": "gpt-5.5", "input": huge_input}) + elapsed = time.time() - t0 + assert elapsed < 40, f"hard ceiling lost to stale floor: {elapsed:.1f}s" + assert "stale_call_kill" in closes, f"stale kill expected, got {closes}" + finally: + stop["flag"] = True From 7af59f474d59e46d7e209dd9e4186376d32ce4c5 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 15 Jul 2026 04:28:12 -0700 Subject: [PATCH 07/47] fix(codex): raise hard-ceiling default above the max stale floor With the TTFB watchdog now scaled (not disabled) for large requests on main, the hard ceiling is a backstop against mid-stream wedges, not the primary stall detector. The original 600s default clamped BELOW the intentional 1200s stale floor for >100k-token requests, partially reverting the floor that keeps healthy gateway-scale payloads alive. Raise the default to 1500s so the ceiling only catches unbounded growth, never healthy slow turns. --- agent/chat_completion_helpers.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/agent/chat_completion_helpers.py b/agent/chat_completion_helpers.py index 561c2ce37..45a88f8de 100644 --- a/agent/chat_completion_helpers.py +++ b/agent/chat_completion_helpers.py @@ -505,20 +505,21 @@ def interruptible_api_call(agent, api_kwargs: dict): _stale_timeout = max(_stale_timeout, _codex_floor) # ── Codex absolute hard ceiling (#64507) ────────────────────────── - # For large Codex requests the no-byte TTFB watchdog is intentionally - # disabled (so legitimate backend admission / prefill isn't killed), and # ``openai_codex_stale_timeout_floor`` *raises* the stale timeout (up to # 1200s at >100k tokens) so healthy gateway-scale payloads aren't aborted. - # That combination means a request that is genuinely stalled at the - # backend — no first byte AND no events, exactly the issue-64507 symptom — - # is only reclaimed at the (high) stale floor, so a session can hang for - # many minutes with an idle worker and no ended_at. Add a flat, finite - # hard ceiling on total request time that ALWAYS applies to openai-codex - # requests regardless of the TTFB/stale interaction, so a stalled large - # request is recovered (retry loop / visible failure) instead of hanging - # indefinitely. Tunable via HERMES_CODEX_HARD_TIMEOUT_SECONDS (set to 0 to + # The scaled no-byte TTFB watchdog catches dead streams that never emit a + # first byte, but a request that emits SOME bytes and then wedges (the + # issue-64507 symptom: vision-inflated request, worker idle, no ended_at) + # is only reclaimed at the (high) stale floor. Add a flat, finite hard + # ceiling on total request time that ALWAYS applies to openai-codex + # requests regardless of the TTFB/stale interaction, so a stalled request + # is recovered (retry loop / visible failure) instead of hanging + # indefinitely. The default sits ABOVE the maximum stale floor (1200s) so + # it never clamps an intentionally-raised timeout for healthy large + # requests — it is a backstop against unbounded growth, not a tighter + # limit. Tunable via HERMES_CODEX_HARD_TIMEOUT_SECONDS (set to 0 to # disable the ceiling entirely; that restores the pre-fix behavior). - _codex_hard_timeout = _env_float("HERMES_CODEX_HARD_TIMEOUT_SECONDS", 600.0) + _codex_hard_timeout = _env_float("HERMES_CODEX_HARD_TIMEOUT_SECONDS", 1500.0) if ( _codex_watchdog_enabled and _openai_codex_backend From 9be941dac1f8fad51a0320202d73fec423fbf3e4 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 14 Jul 2026 07:18:41 -0700 Subject: [PATCH 08/47] feat(mcp): add Blender to the MCP catalog with a curated 4-tool default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds optional-mcps/blender (ahujasid/blender-mcp, stdio via uvx). The server advertises 22 tools; 18 front optional asset services with no upstream trim mechanism, so tools.default_enabled pins the install to the core surface (scene/object info, viewport screenshot, code exec) and the rest stay opt-in through 'hermes mcp configure blender'. Manifests can now declare transport.env (static, non-secret subprocess env vars), parsed/validated in _parse_manifest and written by _build_server_config — used here to ship DISABLE_TELEMETRY=true per the no-telemetry-without-opt-in policy. Runtime already honored per-server env; manifests just couldn't declare it. --- hermes_cli/mcp_catalog.py | 14 +++++ optional-mcps/blender/manifest.yaml | 83 ++++++++++++++++++++++++++++ tests/hermes_cli/test_mcp_catalog.py | 26 +++++++++ 3 files changed, 123 insertions(+) create mode 100644 optional-mcps/blender/manifest.yaml diff --git a/hermes_cli/mcp_catalog.py b/hermes_cli/mcp_catalog.py index aab353949..a75a0d2ca 100644 --- a/hermes_cli/mcp_catalog.py +++ b/hermes_cli/mcp_catalog.py @@ -77,6 +77,10 @@ class TransportSpec: args: List[str] = field(default_factory=list) url: Optional[str] = None version: Optional[str] = None # informational, pinned + # Static environment variables for the stdio subprocess (e.g. telemetry + # opt-outs, mode flags). NOT for secrets — credentials go through + # auth.env so they are prompted for and land in ~/.hermes/.env. + env: Dict[str, str] = field(default_factory=dict) @dataclass @@ -184,12 +188,20 @@ def _parse_manifest(path: Path) -> CatalogEntry: args = transport_raw.get("args") or [] if not isinstance(args, list): raise CatalogError(f"{path}: transport.args must be a list") + env_raw = transport_raw.get("env") or {} + if not isinstance(env_raw, dict) or not all( + isinstance(k, str) and isinstance(v, str) for k, v in env_raw.items() + ): + raise CatalogError( + f"{path}: transport.env must be a mapping of string to string" + ) transport = TransportSpec( type=t_type, command=transport_raw.get("command"), args=[str(a) for a in args], url=transport_raw.get("url"), version=transport_raw.get("version"), + env=dict(env_raw), ) if t_type == "stdio" and not transport.command: raise CatalogError(f"{path}: stdio transport requires 'command'") @@ -468,6 +480,8 @@ def _build_server_config( cfg["command"] = _expand_install_dir(t.command or "", install_dir) if t.args: cfg["args"] = [_expand_install_dir(a, install_dir) for a in t.args] + if t.env: + cfg["env"] = dict(t.env) elif t.type == "http": cfg["url"] = t.url if entry.auth.type == "oauth": diff --git a/optional-mcps/blender/manifest.yaml b/optional-mcps/blender/manifest.yaml new file mode 100644 index 000000000..e897a0778 --- /dev/null +++ b/optional-mcps/blender/manifest.yaml @@ -0,0 +1,83 @@ +# Nous-approved MCP catalog entry. +# Presence in this directory = approval. Merged via PR review. +manifest_version: 1 + +name: blender +description: Drive a live Blender session — modeling, scenes, and renders. +source: https://github.com/ahujasid/blender-mcp + +# ahujasid/blender-mcp (MIT, PyPI: blender-mcp) is the de-facto standard +# Blender MCP. It is a bridge with two halves: +# 1. This stdio server (uvx blender-mcp), which Hermes launches. +# 2. An addon (addon.py) running inside Blender that opens a local command +# socket on 127.0.0.1:9876. The stdio server relays to it. +# There is nothing to git-clone on the Hermes side — uvx resolves the package — +# so no install block. The in-Blender addon setup is covered in post_install. +# +# Why not Blender's official MCP (blender.org/lab): it requires Blender 5.1+ +# and targets scene analysis/documentation rather than asset creation. This +# server works across Blender 3.x/4.x and exposes full modeling control. +transport: + type: stdio + command: "uvx" + args: + - "blender-mcp" + env: + # Upstream ships anonymous telemetry, on by default. Hermes policy is + # no outbound telemetry without explicit opt-in, so the catalog entry + # disables it. Remove this line only if you deliberately want it on. + DISABLE_TELEMETRY: "true" + +# The Blender addon binds 127.0.0.1 only and has no auth of its own; the +# stdio server needs no credentials. Optional integrations (Sketchfab, +# Hyper3D Rodin, Hunyuan3D) take API keys entered in the addon's N-panel +# inside Blender, not via Hermes env. +auth: + type: none + +# Tool selection at install time: +# The server advertises 22 tools. 18 of them front optional third-party asset +# services (PolyHaven, Sketchfab, Hyper3D Rodin, Hunyuan3D) that are dead +# weight unless the matching integration is enabled in the addon panel — and +# upstream has no server-side trim mechanism, so without a filter every one +# of them costs schema tokens on every API call. Default to the core surface: +# scene/object inspection, viewport screenshots, and code execution, which is +# the complete modeling/render capability. Users who enable an asset service +# in the addon can opt into its tools with `hermes mcp configure blender`. +tools: + default_enabled: + - get_scene_info + - get_object_info + - get_viewport_screenshot + - execute_blender_code + +post_install: | + This entry launches the bridge server, but the bridge talks to an addon + INSIDE a running Blender (3.0+). One-time setup: + + 1. Download addon.py from https://github.com/ahujasid/blender-mcp + (raw file: https://raw.githubusercontent.com/ahujasid/blender-mcp/main/addon.py) + 2. Blender > Edit > Preferences > Add-ons > Install... > select addon.py, + then enable "Interface: Blender MCP". + 3. In the 3D viewport press N, open the "BlenderMCP" tab, click + "Connect to Claude" (starts the local socket on 127.0.0.1:9876). + + Blender must be RUNNING with the addon connected before the tools work — + start Blender first, then your Hermes session. The addon refuses to start + under `blender -b` (background mode): its command queue runs on UI timers. + On a machine without a display, run Blender under a virtual one: + xvfb-run blender (or Xvfb :99 & DISPLAY=:99 blender) + GPU rendering (Cycles/OptiX) works fine under Xvfb. + + SECURITY: execute_blender_code runs arbitrary Python inside Blender with no + sandbox — same trust level as the terminal tool. Upstream telemetry is + disabled via DISABLE_TELEMETRY in this entry's env block. + + The 18 asset-service tools (PolyHaven/Sketchfab/Hyper3D/Hunyuan3D) are off + by default. To use one: enable the service in the addon's BlenderMCP panel + (API key there if required), then `hermes mcp configure blender` and tick + its tools. PolyHaven is free and keyless; the others need accounts. + + If Hermes and Blender run on different machines, note that file paths in + execute_blender_code (texture loads, exports) resolve on the BLENDER host's + filesystem, not where Hermes runs. diff --git a/tests/hermes_cli/test_mcp_catalog.py b/tests/hermes_cli/test_mcp_catalog.py index b86cb5ea1..a1679e38f 100644 --- a/tests/hermes_cli/test_mcp_catalog.py +++ b/tests/hermes_cli/test_mcp_catalog.py @@ -197,6 +197,32 @@ class TestManifestParsing: assert get_entry("official/demo") is not None assert get_entry("missing") is None + def test_transport_env_parsed_and_written_to_server_config(self, catalog_dir): + body = _basic_manifest() + body["transport"]["env"] = {"DISABLE_TELEMETRY": "true"} + _write_manifest(catalog_dir, "demo", body) + from hermes_cli.mcp_catalog import _build_server_config + + e = _entry("demo") + assert e.transport.env == {"DISABLE_TELEMETRY": "true"} + cfg = _build_server_config(e, None) + assert cfg["env"] == {"DISABLE_TELEMETRY": "true"} + + def test_transport_env_absent_leaves_config_without_env_key(self, catalog_dir): + _write_manifest(catalog_dir, "demo", _basic_manifest()) + from hermes_cli.mcp_catalog import _build_server_config + + cfg = _build_server_config(_entry("demo"), None) + assert "env" not in cfg + + def test_transport_env_bad_shape_rejected(self, catalog_dir): + body = _basic_manifest() + body["transport"]["env"] = ["DISABLE_TELEMETRY=true"] # list, not mapping + _write_manifest(catalog_dir, "demo", body) + from hermes_cli.mcp_catalog import list_catalog + + assert list_catalog() == [] + # --------------------------------------------------------------------------- # Install flow From a52393a3b6590ace81f28d162f588347b24cc500 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 14 Jul 2026 21:59:20 -0700 Subject: [PATCH 09/47] fix(mcp): pin blender-mcp to 1.6.4 per catalog dependency policy MCP catalog entries follow the same supply-chain rules as pyproject dependencies: exact version pin, and the pinned release must be at least 2 weeks old. blender-mcp 1.6.4 released 2026-06-11 (~5 weeks old, also the latest release). uvx now resolves the exact version instead of latest-at-launch. --- optional-mcps/blender/manifest.yaml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/optional-mcps/blender/manifest.yaml b/optional-mcps/blender/manifest.yaml index e897a0778..ad170d0e3 100644 --- a/optional-mcps/blender/manifest.yaml +++ b/optional-mcps/blender/manifest.yaml @@ -20,8 +20,13 @@ source: https://github.com/ahujasid/blender-mcp transport: type: stdio command: "uvx" + # Pinned per catalog dependency policy: exact version, and the release must + # be at least 2 weeks old at pin time (supply-chain cooldown — same rule as + # pyproject.toml dependencies). 1.6.4 released 2026-06-11. The catalog never + # auto-updates; bumping the pin is a PR to this manifest. args: - - "blender-mcp" + - "blender-mcp==1.6.4" + version: "1.6.4" env: # Upstream ships anonymous telemetry, on by default. Hermes policy is # no outbound telemetry without explicit opt-in, so the catalog entry From 9df5f879b4a5925c0f8f947e7e16ed8e845932c3 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 15 Jul 2026 04:43:30 -0700 Subject: [PATCH 10/47] feat(mcp): enforce exact version pins across the whole MCP catalog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Catalog entries now follow the same supply-chain rules as pyproject dependencies: - n8n: install.ref main -> full commit SHA 7a9ae007 (2026-05-23, branches/tags can be moved by the upstream owner; SHAs cannot) - new contract test: every shipped manifest must pin exactly — git installs need a 40-char SHA, uvx/npx-style launchers need pkg==X / pkg@X with a digit-leading version (rejects bare names, ranges, and npm dist-tags like @latest) - module docstring documents the pin policy (exact version, 2-week cooldown) unreal-engine and linear are http transports (server runs elsewhere) so there is nothing to pin at the transport layer. Verified: unpinning blender-mcp in the manifest makes the contract test fail with a named diagnostic; restoring the pin passes. --- hermes_cli/mcp_catalog.py | 6 ++- optional-mcps/n8n/manifest.yaml | 8 +++- tests/hermes_cli/test_mcp_catalog.py | 57 ++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 3 deletions(-) diff --git a/hermes_cli/mcp_catalog.py b/hermes_cli/mcp_catalog.py index a75a0d2ca..f2a5303ab 100644 --- a/hermes_cli/mcp_catalog.py +++ b/hermes_cli/mcp_catalog.py @@ -10,7 +10,11 @@ Catalog policy: - Entries are added only by merging a PR into hermes-agent. Presence in the ``optional-mcps/`` directory = Nous approval. No community tier, no trust signals beyond "it's in the catalog". -- Manifests pin transport details (commands, args, refs). MCPs are never +- Manifests pin transport details (commands, args, refs). Pins follow the + same supply-chain rules as pyproject dependencies: exact versions for + package launchers (``uvx pkg==X``, ``npx pkg@X``), full commit SHAs for + git installs, and the pinned release should be at least 2 weeks old at + pin time. MCPs are never auto-updated; users explicitly re-run ``hermes mcp install `` to pull a new manifest version after a repo update. - Secrets prompted at install time go to ``~/.hermes/.env`` (the diff --git a/optional-mcps/n8n/manifest.yaml b/optional-mcps/n8n/manifest.yaml index 468efd1dd..bc2fcb5b3 100644 --- a/optional-mcps/n8n/manifest.yaml +++ b/optional-mcps/n8n/manifest.yaml @@ -27,8 +27,12 @@ transport: install: type: git url: https://github.com/CyberSamuraiX/hermes-n8n-mcp.git - # Pin to a commit/tag. Required — manifests do not float HEAD. - ref: main + # Pinned per catalog dependency policy: full commit SHA (branches and tags + # can be moved; SHAs cannot), and the pinned commit must be at least + # 2 weeks old at pin time — same supply-chain rules as pyproject + # dependencies. This SHA is "feat: add local n8n MCP bridge for Hermes" + # (2026-05-23). Bumping the pin is a PR to this manifest. + ref: 7a9ae00795593aa1fdb4e61ecd640e8bfd0c3841 # Bootstrap commands run inside the cloned directory after clone. bootstrap: - "python3 -m venv .venv" diff --git a/tests/hermes_cli/test_mcp_catalog.py b/tests/hermes_cli/test_mcp_catalog.py index a1679e38f..0c440f490 100644 --- a/tests/hermes_cli/test_mcp_catalog.py +++ b/tests/hermes_cli/test_mcp_catalog.py @@ -7,6 +7,7 @@ launch an MCP is mocked. from __future__ import annotations +import re from pathlib import Path from unittest.mock import patch @@ -838,3 +839,59 @@ class TestShippedCatalog: assert entry.name assert entry.description assert entry.transport.type in ("stdio", "http") + + def test_all_shipped_manifests_are_version_locked(self, monkeypatch): + """Contract: catalog entries follow the same supply-chain rules as + pyproject dependencies — everything Hermes fetches/launches is pinned + to an exact version. + + - git installs must pin a full 40-char commit SHA (branches and tags + can be moved by the upstream owner; SHAs cannot). + - package-launcher stdio transports (uvx/npx and their pkg-manager + equivalents) must carry an exact version specifier on the package + arg (``pkg==X`` for Python, ``pkg@X`` for npm). + + http transports and ${INSTALL_DIR}-anchored commands have nothing to + pin at the transport layer (the server runs elsewhere / comes from the + SHA-pinned clone), so they're exempt. + """ + monkeypatch.delenv("HERMES_OPTIONAL_MCPS", raising=False) + from hermes_cli.mcp_catalog import _catalog_root, _parse_manifest + + root = _catalog_root() + if not root.exists(): + pytest.skip("optional-mcps/ not present in this checkout") + + launcher_commands = {"uvx", "npx", "pipx", "bunx", "pnpx"} + problems = [] + for m in root.glob("*/manifest.yaml"): + entry = _parse_manifest(m) + + if entry.install is not None: + if not re.fullmatch(r"[0-9a-f]{40}", entry.install.ref): + problems.append( + f"{entry.name}: install.ref {entry.install.ref!r} is not " + "a full 40-char commit SHA" + ) + + t = entry.transport + if t.type == "stdio" and (t.command or "") in launcher_commands: + pkg_args = [a for a in t.args if not a.startswith("-")] + if not pkg_args: + problems.append(f"{entry.name}: launcher {t.command} has no package arg") + continue + pkg = pkg_args[0] + # Exact-pin shapes: pkg==1.2.3 (uvx/pipx) or pkg@1.2.3 / + # @scope/pkg@1.2.3 (npx/bunx/pnpx). The version must start + # with a digit — a bare name, a range operator, or an npm + # dist-tag (@latest, @next) floats and is rejected. + exact = re.fullmatch(r"[^=@\s]+==\d[\w.\-+]*", pkg) or re.fullmatch( + r"(@[\w.\-]+/)?[\w.\-]+@\d[\w.\-+]*", pkg + ) + if not exact: + problems.append( + f"{entry.name}: package arg {pkg!r} is not pinned to an " + "exact version (expected pkg==X or pkg@X)" + ) + + assert not problems, "unpinned catalog entries:\n" + "\n".join(problems) From 771571aee468258b0883b5ae2cf9ba1a4053bbad Mon Sep 17 00:00:00 2001 From: dorokuma Date: Tue, 2 Jun 2026 12:59:02 +0800 Subject: [PATCH 11/47] fix(auxiliary): pass reasoning_config and extra_body through to auxiliary Anthropic calls Two related bugs in _AnthropicCompletionsAdapter.create() in agent/auxiliary_client.py silently discard caller-supplied reasoning_config and extra_body on the Anthropic-Messages auxiliary-protocol path: * Bug A: reasoning_config=None was hardcoded at L1000, so the reasoning_config parameter on build_anthropic_kwargs was unreachable for any auxiliary task. The main agent path (agent/transports/anthropic.py) already reads reasoning_config from caller params; this PR aligns the auxiliary adapter with the same pattern. * Bug B: create(**kwargs) accepts an OpenAI-style kwargs payload from the caller but only forwards a hand-picked subset to self._client.messages.create(). Any caller-supplied extra_body (e.g. thinking control, metadata, service_tier, vendor-specific fields) was dropped on the floor. The codex/responses transport in the same file already merges extra_body; the Anthropic branch is the gap. This unlocks the caller-supplied extra_body path so auxiliary callers can set per-vendor request fields (including thinking: {type: "disabled"} for Anthropic-compatible vendors that require an explicit disable on the wire), and lets the reasoning_config kwarg flow into build_anthropic_kwargs like the main agent does. Both changes are backward-compatible for callers that don't pass the affected kwargs. Affected providers (all routed through _AnthropicCompletionsAdapter via _maybe_wrap_anthropic): anthropic (native), minimax / minimax-cn, kimi-coding / kimi-coding-cn, z.ai / GLM, and any custom /anthropic-suffixed endpoint. See PR description for related issues (#35566, #7209, #16533, #32813, #29248). --- agent/auxiliary_client.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py index 2b91b9e4d..5f2d7e806 100644 --- a/agent/auxiliary_client.py +++ b/agent/auxiliary_client.py @@ -1337,6 +1337,31 @@ class _AnthropicCompletionsAdapter: if not _forbids_sampling_params(model): anthropic_kwargs["temperature"] = temperature + # Pass through caller-supplied extra_body so providers behind + # Anthropic-compatible gateways receive their per-vendor request + # fields (thinking control, metadata, portal tags, ...). The dict + # form is the documented Anthropic SDK passthrough for non-standard + # request body keys; merge on top of whatever build_anthropic_kwargs + # already produced (e.g. fast-mode ``speed``) so call-time settings + # survive. Two exclusions: + # - ``reasoning``: the OpenAI-shaped config dict is TRANSLATED into + # the native ``thinking`` field above (build_anthropic_kwargs); + # forwarding the raw field alongside would double-specify + # reasoning and 400 on strict gateways. + # - ``_``-prefixed keys: private Hermes plumbing (_reasoning_config + # et al.), never wire fields. + caller_extra_body = kwargs.get("extra_body") + if caller_extra_body and isinstance(caller_extra_body, dict): + passthrough = { + k: v for k, v in caller_extra_body.items() + if k != "reasoning" and not str(k).startswith("_") + } + if passthrough: + existing = anthropic_kwargs.get("extra_body") or {} + if not isinstance(existing, dict): + existing = {} + anthropic_kwargs["extra_body"] = {**existing, **passthrough} + response = create_anthropic_message(self._client, anthropic_kwargs) _transport = get_transport("anthropic_messages") _nr = _transport.normalize_response( From 31dcd68bfaeaa2503a9499e995ee8b4a19059fb9 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 15 Jul 2026 04:42:42 -0700 Subject: [PATCH 12/47] test: cover Anthropic aux extra_body passthrough (Bug B scope + exclusions) Five tests for the salvaged #37217 Bug B fix: vendor-field passthrough, reasoning-key + private-key exclusion, merge-over-existing (fast-mode speed), no-extra_body regression guard, reasoning-only adds nothing. Live probes against api.anthropic.com informed the exclusion design: Anthropic strictly validates the request body (unknown keys 400 with 'Extra inputs are not permitted'), so the passthrough forwards only caller-configured fields and never the OpenAI-shaped reasoning dict (translated natively) or _-private plumbing keys. --- tests/agent/test_auxiliary_client.py | 75 ++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/tests/agent/test_auxiliary_client.py b/tests/agent/test_auxiliary_client.py index 531647eb5..c426a3815 100644 --- a/tests/agent/test_auxiliary_client.py +++ b/tests/agent/test_auxiliary_client.py @@ -3380,6 +3380,81 @@ class TestAuxiliaryTaskExtraBody: } mock_create.assert_called_once() + def _run_anthropic_adapter(self, *, call_extra_body=None, bak_result=None): + """Drive _AnthropicCompletionsAdapter.create() with mocked SDK layers; + return the api_kwargs handed to create_anthropic_message.""" + from agent.auxiliary_client import _AnthropicCompletionsAdapter + + adapter = _AnthropicCompletionsAdapter(MagicMock(), "claude-sonnet-4-6", is_oauth=False) + bak_result = bak_result or { + "model": "claude-sonnet-4-6", "messages": [], "max_tokens": 64, + } + with patch("agent.anthropic_adapter.build_anthropic_kwargs", + return_value=dict(bak_result)), \ + patch("agent.anthropic_adapter.create_anthropic_message") as mock_create, \ + patch("agent.transports.get_transport") as mock_gt: + mock_gt.return_value.normalize_response.return_value = MagicMock( + content="ok", tool_calls=None, reasoning=None, finish_reason="stop", + usage=None, provider_data=None, + ) + kwargs = { + "model": "claude-sonnet-4-6", + "messages": [{"role": "user", "content": "hi"}], + "max_tokens": 64, + } + if call_extra_body is not None: + kwargs["extra_body"] = call_extra_body + adapter.create(**kwargs) + return mock_create.call_args.args[1] + + def test_anthropic_aux_extra_body_passthrough(self): + """Bug B (#37217): vendor fields in extra_body reach the Anthropic SDK.""" + api_kwargs = self._run_anthropic_adapter( + call_extra_body={"thinking": {"type": "disabled"}, "metadata": {"user_id": "u1"}}, + ) + assert api_kwargs["extra_body"] == { + "thinking": {"type": "disabled"}, "metadata": {"user_id": "u1"}, + } + + def test_anthropic_aux_extra_body_excludes_reasoning_and_private_keys(self): + """The OpenAI-shaped reasoning dict is translated (not forwarded), and + private _-prefixed plumbing keys never reach the wire.""" + api_kwargs = self._run_anthropic_adapter( + call_extra_body={ + "reasoning": {"enabled": True, "effort": "low"}, + "_internal": "plumbing", + "metadata": {"user_id": "u1"}, + }, + ) + assert api_kwargs["extra_body"] == {"metadata": {"user_id": "u1"}} + + def test_anthropic_aux_extra_body_merges_over_existing(self): + """Caller extra_body merges on top of what build_anthropic_kwargs + already emitted (fast-mode speed) instead of clobbering it.""" + api_kwargs = self._run_anthropic_adapter( + call_extra_body={"metadata": {"user_id": "u1"}}, + bak_result={ + "model": "claude-sonnet-4-6", "messages": [], "max_tokens": 64, + "extra_body": {"speed": "fast"}, + }, + ) + assert api_kwargs["extra_body"] == { + "speed": "fast", "metadata": {"user_id": "u1"}, + } + + def test_anthropic_aux_no_extra_body_unchanged(self): + """Regression guard: no caller extra_body -> kwargs identical to before.""" + api_kwargs = self._run_anthropic_adapter(call_extra_body=None) + assert "extra_body" not in api_kwargs + + def test_anthropic_aux_reasoning_only_extra_body_adds_nothing(self): + """extra_body containing ONLY the reasoning key must not create an + empty extra_body dict on the wire.""" + api_kwargs = self._run_anthropic_adapter( + call_extra_body={"reasoning": {"enabled": False}}, + ) + assert "extra_body" not in api_kwargs + def test_no_warning_when_provider_is_custom(self, monkeypatch, caplog): """No warning when the provider is 'custom' — OPENAI_BASE_URL is expected.""" import agent.auxiliary_client as mod From a7ef17da7a395e8dc9c4e88ffbe9f57bad812e57 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 15 Jul 2026 04:43:12 -0700 Subject: [PATCH 13/47] chore(release): map dorokuma in AUTHOR_MAP --- scripts/release.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/release.py b/scripts/release.py index 4af52a6d1..80592e6d0 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -334,6 +334,7 @@ AUTHOR_MAP = { "290859878+synapsesx@users.noreply.github.com": "synapsesx", "157689911+itsflownium@users.noreply.github.com": "itsflownium", "dirtyren@users.noreply.github.com": "dirtyren", + "dorokuma@users.noreply.github.com": "dorokuma", "liuwei666888@users.noreply.github.com": "liuwei666888", "527711370@qq.com": "liuwei666888", "217401759+justinschille@users.noreply.github.com": "justinschille", From 8662254ab2fe019fffa4d82cf5263b235c914b5e Mon Sep 17 00:00:00 2001 From: Epoxidex Date: Thu, 21 May 2026 15:01:14 +0300 Subject: [PATCH 14/47] fix(ollama): emit top-level reasoning_effort=none on /v1/chat/completions (#25758) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ollama's /v1/chat/completions silently ignores extra_body.think (it only honours it on /api/chat — ollama/ollama#14820), so agent.reasoning_effort: none never actually disabled thinking on OpenAI-compatible Ollama routes. Emit the top-level reasoning_effort='none' field (which Ollama respects) alongside think=False (kept for proxies and the native /api/chat path). The PR's second half (propagating reasoning_config to the background-review fork) already landed on main via agent/background_review.py, so only the provider-profile change is salvaged here, resolved onto the current GLM/effort-aware profile. Salvaged from PR #29820 by @Epoxidex. --- plugins/model-providers/custom/__init__.py | 11 ++++++++++- .../plugins/model_providers/test_custom_profile.py | 14 ++++++++++---- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/plugins/model-providers/custom/__init__.py b/plugins/model-providers/custom/__init__.py index 2847d161a..de744d509 100644 --- a/plugins/model-providers/custom/__init__.py +++ b/plugins/model-providers/custom/__init__.py @@ -4,7 +4,9 @@ Covers any endpoint registered as provider="custom", including local Ollama instances and OpenAI-compatible reasoning endpoints (GLM-5.2 on Volcengine ARK, vLLM, llama.cpp). Key quirks: - ollama_num_ctx → extra_body.options.num_ctx (local context window) - - reasoning_config disabled → extra_body.think = False + - reasoning_config disabled → top-level reasoning_effort="none" + (Ollama /v1/chat/completions ignores think=False — ollama#14820) + + extra_body.think = False for /api/chat and proxies - reasoning_config enabled + effort → top-level reasoning_effort (the native OpenAI-compatible format GLM/ARK expect; unset omits it so the endpoint's server default applies) @@ -53,6 +55,13 @@ class CustomProfile(ProviderProfile): _effort = (reasoning_config.get("effort") or "").strip().lower() _enabled = reasoning_config.get("enabled", True) if _effort == "none" or _enabled is False: + # Ollama's /v1/chat/completions silently ignores + # extra_body.think (only /api/chat honours it — ollama#14820) + # but respects the top-level reasoning_effort field, so both + # are needed to actually stop a thinking-capable model from + # reasoning (#25758). Endpoints that recognize neither simply + # ignore them. + top_level["reasoning_effort"] = "none" extra_body["think"] = False elif _effort: top_level["reasoning_effort"] = _effort diff --git a/tests/plugins/model_providers/test_custom_profile.py b/tests/plugins/model_providers/test_custom_profile.py index e20ee57b8..152359ed2 100644 --- a/tests/plugins/model_providers/test_custom_profile.py +++ b/tests/plugins/model_providers/test_custom_profile.py @@ -49,20 +49,26 @@ class TestCustomReasoningWireShape: assert tl == {} def test_disabled_sends_think_false(self, custom_profile): - """enabled=False → extra_body.think = False (Ollama thinking-off flag).""" + """enabled=False → reasoning_effort='none' top-level + think=False. + + Both fields are required: Ollama's /v1/chat/completions silently + ignores extra_body.think (only /api/chat honours it — ollama#14820) + but respects top-level reasoning_effort (#25758). think=False stays + for proxies and the native /api/chat path. + """ eb, tl = custom_profile.build_api_kwargs_extras( reasoning_config={"enabled": False}, model="glm-5.2" ) assert eb == {"think": False} - assert tl == {} + assert tl == {"reasoning_effort": "none"} def test_effort_none_sends_think_false(self, custom_profile): - """effort='none' is the disable alias → think=False, no effort.""" + """effort='none' is the disable alias → same dual emission.""" eb, tl = custom_profile.build_api_kwargs_extras( reasoning_config={"enabled": True, "effort": "none"}, model="glm-5.2" ) assert eb == {"think": False} - assert tl == {} + assert tl == {"reasoning_effort": "none"} @pytest.mark.parametrize( "effort", ["minimal", "low", "medium", "high", "xhigh", "max"] From 306e2d2318745b48d0c9d249958b0190f65a07c9 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 14 Jul 2026 14:04:55 -0700 Subject: [PATCH 15/47] chore: AUTHOR_MAP entry for Epoxidex (PR #29820 salvage) --- scripts/release.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/release.py b/scripts/release.py index 80592e6d0..a8777d8ec 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -46,6 +46,7 @@ ACP_REGISTRY_MANIFEST = REPO_ROOT / "acp_registry" / "agent.json" # Auto-extracted from noreply emails + manual overrides AUTHOR_MAP = { "75556242+webtecnica@users.noreply.github.com": "webtecnica", # PR #63360 salvage (nous: restore inference-api base_url) + "skosarevivan@yandex.ru": "Epoxidex", # PR #29820 salvage (ollama: top-level reasoning_effort=none; #25758) "changhyun.min@gmail.com": "minchang", # PR #42231 salvage (providers: add Upstage Solar) "neo@neodeMac-mini.local": "neo-claw-bot", # PR #58465 salvage (moa: drop empty user turns from advisory view) "2024104039@mails.szu.edu.cn": "pixel4039", # PR #64420 salvage (streaming: retry zero-chunk streams) From 7c954969b70bb75a24a1d16bbe7faeb8705a6f42 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 15 Jul 2026 07:39:17 -0700 Subject: [PATCH 16/47] fix(auxiliary): route direct-create aux callers through call_llm (#65029) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(auxiliary): route direct-create aux callers through call_llm (#35566) Five callers (kanban_decompose, kanban_specify, profile_describer, and goals.py's judge + draft-contract) built raw clients via get_text_auxiliary_client() and passed extra_body=get_auxiliary_extra_body() — which only returns Nous portal tags and ignores auxiliary..extra_body from config.yaml entirely. That was the remaining half of #35566 after the call_llm path was fixed. Routing them through call_llm(task=...) gives each caller the full auxiliary contract for free: task extra_body, the reasoning_effort shorthand, transient retries, provider-profile projection, and fallback chains. goal_judge gains a DEFAULT_CONFIG block (it had none — its provider/model overrides silently didn't exist as documented keys). get_auxiliary_extra_body() now has zero non-test callers; kept for plugin back-compat. Fixes #35566. * test: migrate kanban dashboard + CLI specify mocks to call_llm Two more consumers of specify_task mocked the old get_text_auxiliary_client symbol (missed in the first sibling sweep — they live outside tests/hermes_cli's kanban files): the dashboard plugin's /specify endpoint tests and the /kanban slash-command E2E. Same migration as the rest: mock call_llm at the source, no-provider now surfaces via the LLM-error branch. --- hermes_cli/config.py | 12 ++ hermes_cli/goals.py | 36 +--- hermes_cli/kanban_decompose.py | 23 +-- hermes_cli/kanban_specify.py | 21 +-- hermes_cli/profile_describer.py | 22 +-- tests/hermes_cli/test_goals.py | 176 ++++++------------ tests/hermes_cli/test_kanban_cli.py | 7 +- tests/hermes_cli/test_kanban_decompose.py | 24 ++- tests/hermes_cli/test_kanban_specify.py | 29 ++- tests/hermes_cli/test_profile_describer.py | 8 +- tests/plugins/test_kanban_dashboard_plugin.py | 24 ++- 11 files changed, 141 insertions(+), 241 deletions(-) diff --git a/hermes_cli/config.py b/hermes_cli/config.py index a47ba24ea..45cee2071 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -1700,6 +1700,18 @@ DEFAULT_CONFIG = { "extra_body": {}, "reasoning_effort": "", # per-task thinking level: none|minimal|low|medium|high|xhigh|max|ultra (empty = provider default) }, + # Goal judge — evaluates whether a /goal run's latest response + # satisfies the goal/contract, and drafts goal contracts. Short + # structured-JSON calls; a fast cheap model is fine. + "goal_judge": { + "provider": "auto", + "model": "", + "base_url": "", + "api_key": "", + "timeout": 60, + "extra_body": {}, + "reasoning_effort": "", # per-task thinking level: none|minimal|low|medium|high|xhigh|max|ultra (empty = provider default) + }, # Curator — skill-usage review fork. Timeout is generous because the # review pass can take several minutes on reasoning models (umbrella # building over hundreds of candidate skills). "auto" = use main chat diff --git a/hermes_cli/goals.py b/hermes_cli/goals.py index 3a1e86930..21e055402 100644 --- a/hermes_cli/goals.py +++ b/hermes_cli/goals.py @@ -878,20 +878,11 @@ def judge_goal( return "continue", "empty response (nothing to evaluate)", False, None try: - from agent.auxiliary_client import get_auxiliary_extra_body, get_text_auxiliary_client + from agent.auxiliary_client import call_llm except Exception as exc: logger.debug("goal judge: auxiliary client import failed: %s", exc) return "continue", "auxiliary client unavailable", False, None - try: - client, model = get_text_auxiliary_client("goal_judge") - except Exception as exc: - logger.debug("goal judge: get_text_auxiliary_client failed: %s", exc) - return "continue", "auxiliary client unavailable", False, None - - if client is None or not model: - return "continue", "no auxiliary client configured", False, None - # Build the prompt. Priority: contract > subgoals > plain. When both a # contract and subgoals exist, the subgoals are appended into the # contract block as extra criteria so the judge sees a single source of @@ -935,8 +926,11 @@ def judge_goal( ) try: - resp = client.chat.completions.create( - model=model, + # Route through call_llm so auxiliary.goal_judge.* config + # (provider/model/base_url, extra_body, reasoning_effort, retries) + # all apply — the direct-create path dropped extra_body (#35566). + resp = call_llm( + task="goal_judge", messages=[ {"role": "system", "content": JUDGE_SYSTEM_PROMPT}, {"role": "user", "content": prompt}, @@ -944,7 +938,6 @@ def judge_goal( temperature=0, max_tokens=_goal_judge_max_tokens(), timeout=timeout, - extra_body=get_auxiliary_extra_body() or None, ) except Exception as exc: logger.info("goal judge: API call failed (%s) — falling through to continue", exc) @@ -999,23 +992,15 @@ def draft_contract(objective: str, *, timeout: float = DEFAULT_JUDGE_TIMEOUT) -> return None try: - from agent.auxiliary_client import get_auxiliary_extra_body, get_text_auxiliary_client + from agent.auxiliary_client import call_llm except Exception as exc: logger.debug("goal draft: auxiliary client import failed: %s", exc) return None try: - client, model = get_text_auxiliary_client("goal_judge") - except Exception as exc: - logger.debug("goal draft: get_text_auxiliary_client failed: %s", exc) - return None - - if client is None or not model: - return None - - try: - resp = client.chat.completions.create( - model=model, + # Route through call_llm — same #35566 fix as the judge call above. + resp = call_llm( + task="goal_judge", messages=[ {"role": "system", "content": DRAFT_CONTRACT_SYSTEM_PROMPT}, {"role": "user", "content": f"Objective:\n{_truncate(objective, 4000)}"}, @@ -1023,7 +1008,6 @@ def draft_contract(objective: str, *, timeout: float = DEFAULT_JUDGE_TIMEOUT) -> temperature=0, max_tokens=_goal_judge_max_tokens(), timeout=timeout, - extra_body=get_auxiliary_extra_body() or None, ) except Exception as exc: logger.info("goal draft: API call failed (%s)", exc) diff --git a/hermes_cli/kanban_decompose.py b/hermes_cli/kanban_decompose.py index 8af2da911..d6c248c65 100644 --- a/hermes_cli/kanban_decompose.py +++ b/hermes_cli/kanban_decompose.py @@ -298,23 +298,11 @@ def decompose_task( roster, valid_names = _build_roster() try: - from agent.auxiliary_client import ( # type: ignore - get_auxiliary_extra_body, - get_text_auxiliary_client, - ) + from agent.auxiliary_client import call_llm # type: ignore except Exception as exc: logger.debug("decompose: auxiliary client import failed: %s", exc) return DecomposeOutcome(task_id, False, "auxiliary client unavailable") - try: - client, model = get_text_auxiliary_client("kanban_decomposer") - except Exception as exc: - logger.debug("decompose: get_text_auxiliary_client failed: %s", exc) - return DecomposeOutcome(task_id, False, "auxiliary client unavailable") - - if client is None or not model: - return DecomposeOutcome(task_id, False, "no auxiliary client configured") - user_msg = _USER_TEMPLATE.format( task_id=task.id, title=_truncate(task.title or "", 400), @@ -324,8 +312,12 @@ def decompose_task( ) try: - resp = client.chat.completions.create( - model=model, + # Route through call_llm so auxiliary.kanban_decomposer.* config + # (provider/model/base_url, extra_body, reasoning_effort, retries) + # all apply — the previous direct client.chat.completions.create() + # path dropped auxiliary..extra_body entirely (#35566). + resp = call_llm( + task="kanban_decomposer", messages=[ {"role": "system", "content": _SYSTEM_PROMPT}, {"role": "user", "content": user_msg}, @@ -333,7 +325,6 @@ def decompose_task( temperature=0.3, max_tokens=4000, timeout=timeout or 180, - extra_body=get_auxiliary_extra_body() or None, ) except Exception as exc: logger.info( diff --git a/hermes_cli/kanban_specify.py b/hermes_cli/kanban_specify.py index 40812d835..e7aba34a3 100644 --- a/hermes_cli/kanban_specify.py +++ b/hermes_cli/kanban_specify.py @@ -162,22 +162,11 @@ def specify_task( ) try: - from agent.auxiliary_client import get_auxiliary_extra_body, get_text_auxiliary_client + from agent.auxiliary_client import call_llm except Exception as exc: # pragma: no cover — import smoke test logger.debug("specify: auxiliary client import failed: %s", exc) return SpecifyOutcome(task_id, False, "auxiliary client unavailable") - try: - client, model = get_text_auxiliary_client("triage_specifier") - except Exception as exc: - logger.debug("specify: get_text_auxiliary_client failed: %s", exc) - return SpecifyOutcome(task_id, False, "auxiliary client unavailable") - - if client is None or not model: - return SpecifyOutcome( - task_id, False, "no auxiliary client configured" - ) - user_msg = _USER_TEMPLATE.format( task_id=task.id, title=_truncate(task.title or "", 400), @@ -185,8 +174,11 @@ def specify_task( ) try: - resp = client.chat.completions.create( - model=model, + # Route through call_llm so auxiliary.triage_specifier.* config + # (provider/model/base_url, extra_body, reasoning_effort, retries) + # all apply — the direct-create path dropped extra_body (#35566). + resp = call_llm( + task="triage_specifier", messages=[ {"role": "system", "content": _SYSTEM_PROMPT}, {"role": "user", "content": user_msg}, @@ -194,7 +186,6 @@ def specify_task( temperature=0.3, max_tokens=HERMES_KANBAN_SPECIFY_MAX_TOKENS, timeout=timeout or 120, - extra_body=get_auxiliary_extra_body() or None, ) except Exception as exc: logger.info( diff --git a/hermes_cli/profile_describer.py b/hermes_cli/profile_describer.py index f80d1f545..c6af27ae2 100644 --- a/hermes_cli/profile_describer.py +++ b/hermes_cli/profile_describer.py @@ -210,23 +210,11 @@ def describe_profile( model, provider = None, None try: - from agent.auxiliary_client import ( # type: ignore - get_auxiliary_extra_body, - get_text_auxiliary_client, - ) + from agent.auxiliary_client import call_llm # type: ignore except Exception as exc: logger.debug("describe: auxiliary client import failed: %s", exc) return DescribeOutcome(canon, False, "auxiliary client unavailable") - try: - client, aux_model = get_text_auxiliary_client("profile_describer") - except Exception as exc: - logger.debug("describe: get_text_auxiliary_client failed: %s", exc) - return DescribeOutcome(canon, False, "auxiliary client unavailable") - - if client is None or not aux_model: - return DescribeOutcome(canon, False, "no auxiliary client configured") - user_msg = _USER_TEMPLATE.format( name=canon, model=(model or "(unset)"), @@ -237,8 +225,11 @@ def describe_profile( ) try: - resp = client.chat.completions.create( - model=aux_model, + # Route through call_llm so auxiliary.profile_describer.* config + # (provider/model/base_url, extra_body, reasoning_effort, retries) + # all apply — the direct-create path dropped extra_body (#35566). + resp = call_llm( + task="profile_describer", messages=[ {"role": "system", "content": _SYSTEM_PROMPT}, {"role": "user", "content": user_msg}, @@ -246,7 +237,6 @@ def describe_profile( temperature=0.3, max_tokens=400, timeout=timeout or 60, - extra_body=get_auxiliary_extra_body() or None, ) except Exception as exc: logger.info("describe: API call failed for %s (%s)", canon, exc) diff --git a/tests/hermes_cli/test_goals.py b/tests/hermes_cli/test_goals.py index b6ae1abcd..118c11007 100644 --- a/tests/hermes_cli/test_goals.py +++ b/tests/hermes_cli/test_goals.py @@ -166,8 +166,8 @@ class TestJudgeGoal: from hermes_cli import goals with patch( - "agent.auxiliary_client.get_text_auxiliary_client", - return_value=(None, None), + "agent.auxiliary_client.call_llm", + side_effect=RuntimeError("No LLM provider configured"), ): verdict, _, _, _wd = goals.judge_goal("my goal", "my response") assert verdict == "continue" @@ -176,11 +176,9 @@ class TestJudgeGoal: """Judge exception → fail-open continue (don't wedge progress on judge bugs).""" from hermes_cli import goals - fake_client = MagicMock() - fake_client.chat.completions.create.side_effect = RuntimeError("boom") with patch( - "agent.auxiliary_client.get_text_auxiliary_client", - return_value=(fake_client, "judge-model"), + "agent.auxiliary_client.call_llm", + side_effect=RuntimeError("boom"), ): verdict, reason, _, _wd = goals.judge_goal("goal", "response") assert verdict == "continue" @@ -189,17 +187,11 @@ class TestJudgeGoal: def test_judge_says_done(self): from hermes_cli import goals - fake_client = MagicMock() - fake_client.chat.completions.create.return_value = MagicMock( - choices=[ - MagicMock( - message=MagicMock(content='{"done": true, "reason": "achieved"}') - ) - ] - ) with patch( - "agent.auxiliary_client.get_text_auxiliary_client", - return_value=(fake_client, "judge-model"), + "agent.auxiliary_client.call_llm", + return_value=MagicMock( + choices=[MagicMock(message=MagicMock(content='{"done": true, "reason": "achieved"}'))] + ), ): verdict, reason, _, _wd = goals.judge_goal("goal", "agent response") assert verdict == "done" @@ -208,17 +200,11 @@ class TestJudgeGoal: def test_judge_says_continue(self): from hermes_cli import goals - fake_client = MagicMock() - fake_client.chat.completions.create.return_value = MagicMock( - choices=[ - MagicMock( - message=MagicMock(content='{"done": false, "reason": "not yet"}') - ) - ] - ) with patch( - "agent.auxiliary_client.get_text_auxiliary_client", - return_value=(fake_client, "judge-model"), + "agent.auxiliary_client.call_llm", + return_value=MagicMock( + choices=[MagicMock(message=MagicMock(content='{"done": false, "reason": "not yet"}'))] + ), ): verdict, reason, _, _wd = goals.judge_goal("goal", "agent response") assert verdict == "continue" @@ -448,11 +434,9 @@ class TestJudgeParseFailureAutoPause: """Transient network/API errors must not trip the auto-pause guard.""" from hermes_cli import goals - fake_client = MagicMock() - fake_client.chat.completions.create.side_effect = RuntimeError("connection reset") with patch( - "agent.auxiliary_client.get_text_auxiliary_client", - return_value=(fake_client, "judge-model"), + "agent.auxiliary_client.call_llm", + side_effect=RuntimeError("connection reset"), ): verdict, _, parse_failed, _wd = goals.judge_goal("goal", "response") assert verdict == "continue" @@ -462,13 +446,9 @@ class TestJudgeParseFailureAutoPause: """End-to-end: judge returns empty content → parse_failed=True.""" from hermes_cli import goals - fake_client = MagicMock() - fake_client.chat.completions.create.return_value = MagicMock( - choices=[MagicMock(message=MagicMock(content=""))] - ) with patch( - "agent.auxiliary_client.get_text_auxiliary_client", - return_value=(fake_client, "judge-model"), + "agent.auxiliary_client.call_llm", + return_value=MagicMock(choices=[MagicMock(message=MagicMock(content=""))]), ): verdict, _, parse_failed, _wd = goals.judge_goal("goal", "response") assert verdict == "continue" @@ -747,22 +727,11 @@ class TestJudgeGoalWithSubgoals: message = _FakeMsg() class _FakeResp: choices = [_FakeChoice()] - class _FakeClient: - class chat: - class completions: - @staticmethod - def create(**kwargs): - captured.update(kwargs) - return _FakeResp() + def _fake_call_llm(**kwargs): + captured.update(kwargs) + return _FakeResp() - with patch.object(goals, "get_text_auxiliary_client", - return_value=(_FakeClient, "fake-model"), create=True), \ - patch.object(goals, "get_auxiliary_extra_body", - return_value=None, create=True), \ - patch("agent.auxiliary_client.get_text_auxiliary_client", - return_value=(_FakeClient, "fake-model")), \ - patch("agent.auxiliary_client.get_auxiliary_extra_body", - return_value=None): + with patch("agent.auxiliary_client.call_llm", side_effect=_fake_call_llm): verdict, reason, parse_failed, _wd = goals.judge_goal( "ship the feature", "ok shipped", @@ -790,18 +759,11 @@ class TestJudgeGoalWithSubgoals: message = _FakeMsg() class _FakeResp: choices = [_FakeChoice()] - class _FakeClient: - class chat: - class completions: - @staticmethod - def create(**kwargs): - captured.update(kwargs) - return _FakeResp() + def _fake_call_llm(**kwargs): + captured.update(kwargs) + return _FakeResp() - with patch("agent.auxiliary_client.get_text_auxiliary_client", - return_value=(_FakeClient, "fake-model")), \ - patch("agent.auxiliary_client.get_auxiliary_extra_body", - return_value=None): + with patch("agent.auxiliary_client.call_llm", side_effect=_fake_call_llm): goals.judge_goal("ship it", "done", subgoals=None) sent_messages = captured.get("messages") or [] @@ -1363,7 +1325,8 @@ class TestGoalManagerContract: class TestJudgeWithContract: - def _fake_client(self, captured, content='{"done": false, "reason": "more"}'): + def _fake_call_llm(self, captured, content='{"done": false, "reason": "more"}'): + """judge_goal routes through call_llm (#35566) — capture its kwargs.""" class _FakeMsg: pass _FakeMsg.content = content @@ -1371,14 +1334,11 @@ class TestJudgeWithContract: message = _FakeMsg() class _FakeResp: choices = [_FakeChoice()] - class _FakeClient: - class chat: - class completions: - @staticmethod - def create(**kwargs): - captured.update(kwargs) - return _FakeResp() - return _FakeClient + + def _fake(**kwargs): + captured.update(kwargs) + return _FakeResp() + return _fake def test_judge_uses_contract_template(self, hermes_home): from unittest.mock import patch @@ -1386,10 +1346,8 @@ class TestJudgeWithContract: from hermes_cli.goals import GoalContract captured = {} - client = self._fake_client(captured) - with patch("agent.auxiliary_client.get_text_auxiliary_client", - return_value=(client, "fake-model")), \ - patch("agent.auxiliary_client.get_auxiliary_extra_body", return_value=None): + with patch("agent.auxiliary_client.call_llm", + side_effect=self._fake_call_llm(captured)): goals.judge_goal( "ship it", "I think it's done", contract=GoalContract(verification="pytest -q passes"), @@ -1407,10 +1365,8 @@ class TestJudgeWithContract: from hermes_cli.goals import GoalContract captured = {} - client = self._fake_client(captured) - with patch("agent.auxiliary_client.get_text_auxiliary_client", - return_value=(client, "fake-model")), \ - patch("agent.auxiliary_client.get_auxiliary_extra_body", return_value=None): + with patch("agent.auxiliary_client.call_llm", + side_effect=self._fake_call_llm(captured)): goals.judge_goal( "ship it", "done", subgoals=["write changelog"], @@ -1438,16 +1394,8 @@ class TestDraftContract: message = _FakeMsg() class _FakeResp: choices = [_FakeChoice()] - class _FakeClient: - class chat: - class completions: - @staticmethod - def create(**kwargs): - return _FakeResp() - - with patch("agent.auxiliary_client.get_text_auxiliary_client", - return_value=(_FakeClient, "fake-model")), \ - patch("agent.auxiliary_client.get_auxiliary_extra_body", return_value=None): + with patch("agent.auxiliary_client.call_llm", + return_value=_FakeResp()): contract = goals.draft_contract("Migrate auth to JWT") assert contract is not None assert contract.outcome == "auth on JWT" @@ -1464,24 +1412,16 @@ class TestDraftContract: message = _FakeMsg() class _FakeResp: choices = [_FakeChoice()] - class _FakeClient: - class chat: - class completions: - @staticmethod - def create(**kwargs): - return _FakeResp() - - with patch("agent.auxiliary_client.get_text_auxiliary_client", - return_value=(_FakeClient, "fake-model")), \ - patch("agent.auxiliary_client.get_auxiliary_extra_body", return_value=None): + with patch("agent.auxiliary_client.call_llm", + return_value=_FakeResp()): assert goals.draft_contract("anything") is None def test_draft_returns_none_when_no_client(self, hermes_home): from unittest.mock import patch from hermes_cli import goals - with patch("agent.auxiliary_client.get_text_auxiliary_client", - return_value=(None, None)): + with patch("agent.auxiliary_client.call_llm", + side_effect=RuntimeError("No LLM provider configured")): assert goals.draft_contract("anything") is None @@ -1495,7 +1435,8 @@ class TestContractAndBackgroundCompose: the contract block and the background-process list to the judge, so it can return either done (evidence met) or wait (parked on the poller).""" - def _capture_client(self, captured, content='{"verdict": "wait", "wait_on_pid": 4242, "reason": "CI still running"}'): + def _capture_call_llm(self, captured, content='{"verdict": "wait", "wait_on_pid": 4242, "reason": "CI still running"}'): + """judge_goal routes through call_llm (#35566) — capture its kwargs.""" class _FakeMsg: pass _FakeMsg.content = content @@ -1503,14 +1444,11 @@ class TestContractAndBackgroundCompose: message = _FakeMsg() class _FakeResp: choices = [_FakeChoice()] - class _FakeClient: - class chat: - class completions: - @staticmethod - def create(**kwargs): - captured.update(kwargs) - return _FakeResp() - return _FakeClient + + def _fake(**kwargs): + captured.update(kwargs) + return _FakeResp() + return _fake def test_judge_prompt_carries_contract_and_background(self, hermes_home): from unittest.mock import patch @@ -1518,14 +1456,12 @@ class TestContractAndBackgroundCompose: from hermes_cli.goals import GoalContract captured = {} - client = self._capture_client(captured) bg = [{ "session_id": "ci-watch", "pid": 4242, "status": "running", "command": "wait_for_pr_green.sh 50501", "trigger": "exit", }] - with patch("agent.auxiliary_client.get_text_auxiliary_client", - return_value=(client, "fake-model")), \ - patch("agent.auxiliary_client.get_auxiliary_extra_body", return_value=None): + with patch("agent.auxiliary_client.call_llm", + side_effect=self._capture_call_llm(captured)): verdict, reason, parse_failed, wait_directive = goals.judge_goal( "ship the PR", "I pushed and started the CI watcher; waiting on it now.", @@ -1550,14 +1486,12 @@ class TestContractAndBackgroundCompose: from hermes_cli.goals import GoalContract captured = {} - client = self._capture_client( - captured, - content='{"verdict": "done", "reason": "CI is green, evidence shown"}', - ) bg = [{"session_id": "ci", "pid": 4242, "status": "running", "command": "ci", "trigger": "exit"}] - with patch("agent.auxiliary_client.get_text_auxiliary_client", - return_value=(client, "fake-model")), \ - patch("agent.auxiliary_client.get_auxiliary_extra_body", return_value=None): + with patch("agent.auxiliary_client.call_llm", + side_effect=self._capture_call_llm( + captured, + content='{"verdict": "done", "reason": "CI is green, evidence shown"}', + )): verdict, reason, parse_failed, wait_directive = goals.judge_goal( "ship the PR", "CI finished: 30 passed, 0 failed. Done.", diff --git a/tests/hermes_cli/test_kanban_cli.py b/tests/hermes_cli/test_kanban_cli.py index c59578c4b..6433635c2 100644 --- a/tests/hermes_cli/test_kanban_cli.py +++ b/tests/hermes_cli/test_kanban_cli.py @@ -465,11 +465,10 @@ def test_run_slash_specify_end_to_end(kanban_home, monkeypatch): resp.choices[0].message.content = ( '{"title": "Spec: rough idea", "body": "**Goal**\\nShip it."}' ) - fake_client = MagicMock() - fake_client.chat.completions.create = MagicMock(return_value=resp) + # specify_task routes through call_llm now (#35566) — mock it directly. monkeypatch.setattr( - "agent.auxiliary_client.get_text_auxiliary_client", - lambda *a, **kw: (fake_client, "test-model"), + "agent.auxiliary_client.call_llm", + MagicMock(return_value=resp), ) # Specify via slash. diff --git a/tests/hermes_cli/test_kanban_decompose.py b/tests/hermes_cli/test_kanban_decompose.py index 5ba17e58c..7a872a4da 100644 --- a/tests/hermes_cli/test_kanban_decompose.py +++ b/tests/hermes_cli/test_kanban_decompose.py @@ -41,18 +41,19 @@ def _mock_client_returning(content: str): def _patch_aux_client(content: str, *, model: str = "test-model"): - client = _mock_client_returning(content) + # decompose_task now routes through call_llm (see #35566) — mock it at + # the source module so task config, extra_body, and retries stay out of + # unit-test scope. return patch( - "agent.auxiliary_client.get_text_auxiliary_client", - return_value=(client, model), + "agent.auxiliary_client.call_llm", + return_value=_fake_aux_response(content), ) def _patch_extra_body(): - return patch( - "agent.auxiliary_client.get_auxiliary_extra_body", - return_value={}, - ) + # No-op shim retained for call-site compatibility: extra_body plumbing + # now lives inside call_llm, which _patch_aux_client already mocks. + return patch("agent.auxiliary_client.get_auxiliary_extra_body", return_value={}) def _patch_list_profiles(names: list[str]): @@ -334,9 +335,11 @@ def test_decompose_no_aux_client_configured(kanban_home): for p in patches: p.start() try: + # call_llm raises RuntimeError when no provider is configured; the + # decomposer must convert that into a failed outcome, not a crash. with patch( - "agent.auxiliary_client.get_text_auxiliary_client", - return_value=(None, ""), + "agent.auxiliary_client.call_llm", + side_effect=RuntimeError("No LLM provider configured"), ): outcome = decomp.decompose_task(tid, author="me") finally: @@ -344,4 +347,5 @@ def test_decompose_no_aux_client_configured(kanban_home): p.stop() assert outcome.ok is False - assert "no auxiliary client" in outcome.reason + # call_llm's no-provider RuntimeError surfaces via the LLM-error branch. + assert "LLM error" in outcome.reason diff --git a/tests/hermes_cli/test_kanban_specify.py b/tests/hermes_cli/test_kanban_specify.py index dd3770015..8b9c9f521 100644 --- a/tests/hermes_cli/test_kanban_specify.py +++ b/tests/hermes_cli/test_kanban_specify.py @@ -48,15 +48,12 @@ def _mock_client_returning(content: str): def _patch_aux_client(content: str, *, model: str = "test-model"): - """Patch get_text_auxiliary_client at its source + at the module that - imported it lazily inside specify_task. Both patches are needed - because kanban_specify imports the function inside the function body. + """Patch call_llm at its source module — specify_task now routes through + it (#35566) instead of building a raw client. Returns (patcher, mock) so + callers can still assert on the call. """ - client = _mock_client_returning(content) - return patch( - "agent.auxiliary_client.get_text_auxiliary_client", - return_value=(client, model), - ), client + mock_fn = MagicMock(return_value=_fake_aux_response(content)) + return patch("agent.auxiliary_client.call_llm", mock_fn), mock_fn # --------------------------------------------------------------------------- @@ -159,13 +156,14 @@ def test_specify_task_no_aux_client_configured(kanban_home): tid = kb.create_task(conn, title="rough", triage=True) with patch( - "agent.auxiliary_client.get_text_auxiliary_client", - return_value=(None, ""), + "agent.auxiliary_client.call_llm", + side_effect=RuntimeError("No LLM provider configured"), ): outcome = spec.specify_task(tid) assert outcome.ok is False - assert "auxiliary client" in outcome.reason + # call_llm's no-provider RuntimeError surfaces via the LLM-error branch. + assert "LLM error" in outcome.reason # Task must stay in triage — we never touched it. with kb.connect() as conn: assert kb.get_task(conn, tid).status == "triage" @@ -176,10 +174,9 @@ def test_specify_task_llm_api_error_keeps_task_in_triage(kanban_home): tid = kb.create_task(conn, title="rough", triage=True) client = MagicMock() - client.chat.completions.create = MagicMock(side_effect=RuntimeError("429 rate limited")) with patch( - "agent.auxiliary_client.get_text_auxiliary_client", - return_value=(client, "test-model"), + "agent.auxiliary_client.call_llm", + side_effect=RuntimeError("429 rate limited"), ): outcome = spec.specify_task(tid) @@ -288,8 +285,8 @@ def test_cli_specify_all_returns_1_when_every_task_fails(kanban_home, capsys): kb.create_task(conn, title="b", triage=True) with patch( - "agent.auxiliary_client.get_text_auxiliary_client", - return_value=(None, ""), # no aux client → every task fails + "agent.auxiliary_client.call_llm", + side_effect=RuntimeError("No LLM provider configured"), # every task fails ): rc = _run_cli("specify", "--all") diff --git a/tests/hermes_cli/test_profile_describer.py b/tests/hermes_cli/test_profile_describer.py index 3fc5fa3a6..e1a55a648 100644 --- a/tests/hermes_cli/test_profile_describer.py +++ b/tests/hermes_cli/test_profile_describer.py @@ -79,11 +79,11 @@ def _fake_aux_response(content: str): def _patch_aux_client(content: str): - client = MagicMock() - client.chat.completions.create = MagicMock(return_value=_fake_aux_response(content)) + # describe_profile now routes through call_llm (#35566) — mock it at the + # source module. return patch( - "agent.auxiliary_client.get_text_auxiliary_client", - return_value=(client, "test-model"), + "agent.auxiliary_client.call_llm", + return_value=_fake_aux_response(content), ) diff --git a/tests/plugins/test_kanban_dashboard_plugin.py b/tests/plugins/test_kanban_dashboard_plugin.py index 4c7f466ce..2cf207d7d 100644 --- a/tests/plugins/test_kanban_dashboard_plugin.py +++ b/tests/plugins/test_kanban_dashboard_plugin.py @@ -2182,13 +2182,10 @@ def _patch_specifier_response(monkeypatch, *, content, model="test-model"): resp = MagicMock() resp.choices = [MagicMock()] resp.choices[0].message.content = content - fake_client = MagicMock() - fake_client.chat.completions.create = MagicMock(return_value=resp) - monkeypatch.setattr( - "agent.auxiliary_client.get_text_auxiliary_client", - lambda *a, **kw: (fake_client, model), - ) - return fake_client + # specify_task routes through call_llm now (#35566) — mock it directly. + fake_call = MagicMock(return_value=resp) + monkeypatch.setattr("agent.auxiliary_client.call_llm", fake_call) + return fake_call def test_specify_happy_path(client, monkeypatch): @@ -2250,11 +2247,11 @@ def test_specify_no_aux_client_surfaces_reason(client, monkeypatch): json={"title": "rough", "triage": True}, ).json()["task"] - # Simulate "no auxiliary client configured". - monkeypatch.setattr( - "agent.auxiliary_client.get_text_auxiliary_client", - lambda *a, **kw: (None, ""), - ) + # Simulate "no auxiliary client configured" — call_llm raises when + # no provider resolves (#35566 routing). + def _no_provider(**kwargs): + raise RuntimeError("No LLM provider configured") + monkeypatch.setattr("agent.auxiliary_client.call_llm", _no_provider) r = client.post( f"/api/plugins/kanban/tasks/{t['id']}/specify", @@ -2263,7 +2260,8 @@ def test_specify_no_aux_client_surfaces_reason(client, monkeypatch): assert r.status_code == 200 body = r.json() assert body["ok"] is False - assert "auxiliary client" in body["reason"] + # call_llm's no-provider RuntimeError surfaces via the LLM-error branch. + assert "LLM error" in body["reason"] # Task must stay in triage — nothing was touched. detail = client.get(f"/api/plugins/kanban/tasks/{t['id']}").json()["task"] From 1011cd24e2acfaf4efe852800589989edada6dc0 Mon Sep 17 00:00:00 2001 From: HexLab98 Date: Sun, 12 Jul 2026 06:54:55 +0700 Subject: [PATCH 17/47] fix(input): strip bracketed-paste leaks before prompt persistence (#62557) Extract shared hermes_cli/input_sanitize.py (bracketed-paste wrapper stripping and terminal ~[[e artifact suffix collapse) and wire it into prompt.submit and the Desktop composer so corrupted user text is cleaned before messages.content is persisted. --- apps/desktop/src/app/chat/composer/index.tsx | 5 +- .../session/hooks/use-prompt-actions/index.ts | 5 +- .../hooks/use-prompt-actions/submit.ts | 3 +- .../thread/user-edit-composer.tsx | 5 +- .../src/lib/composer-input-sanitize.ts | 71 +++++++++++++++++++ cli.py | 24 +------ hermes_cli/input_sanitize.py | 70 ++++++++++++++++++ tui_gateway/server.py | 6 +- 8 files changed, 159 insertions(+), 30 deletions(-) create mode 100644 apps/desktop/src/lib/composer-input-sanitize.ts create mode 100644 hermes_cli/input_sanitize.py diff --git a/apps/desktop/src/app/chat/composer/index.tsx b/apps/desktop/src/app/chat/composer/index.tsx index 1f5df46eb..a24344a2e 100644 --- a/apps/desktop/src/app/chat/composer/index.tsx +++ b/apps/desktop/src/app/chat/composer/index.tsx @@ -6,6 +6,7 @@ import { composerFill, composerSurfaceGlass } from '@/components/chat/composer-d import { Button } from '@/components/ui/button' import { useI18n } from '@/i18n' import { chatMessageText } from '@/lib/chat-messages' +import { sanitizeComposerInput } from '@/lib/composer-input-sanitize' import { DATA_IMAGE_URL_RE } from '@/lib/embedded-images' import { triggerHaptic } from '@/lib/haptics' import { cn } from '@/lib/utils' @@ -267,7 +268,7 @@ export function ChatBar({ normalizeComposerEditorDom(editor) - const nextDraft = composerPlainText(editor) + const nextDraft = sanitizeComposerInput(composerPlainText(editor)) if (nextDraft !== draftRef.current) { draftRef.current = nextDraft @@ -332,7 +333,7 @@ export function ChatBar({ // blank lines (common when selecting from terminals, code blocks, web pages) // doesn't dump multiline padding into the composer. Internal newlines are // preserved — only the edges are cleaned up. - const pastedText = event.clipboardData.getData('text').trim() + const pastedText = sanitizeComposerInput(event.clipboardData.getData('text').trim()) if (!pastedText) { event.preventDefault() diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts index 89f978cd6..0c1b62eff 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts @@ -6,6 +6,7 @@ import { PROMPT_SUBMIT_REQUEST_TIMEOUT_MS, transcribeAudio } from '@/hermes' import { useI18n } from '@/i18n' import { stripAnsi } from '@/lib/ansi' import { branchGroupForUser, type ChatMessage, chatMessageText, textPart } from '@/lib/chat-messages' +import { sanitizeComposerInput } from '@/lib/composer-input-sanitize' import { pathLabel, SLASH_COMMAND_RE } from '@/lib/chat-runtime' import { triggerHaptic } from '@/lib/haptics' import { setMutableRef } from '@/lib/mutable-ref' @@ -471,7 +472,7 @@ export function usePromptActions({ const submitText = useCallback( async (rawText: string, options?: SubmitTextOptions) => { - const visibleText = rawText.trim() + const visibleText = sanitizeComposerInput(rawText).trim() const attachments = options?.attachments ?? $composerAttachments.get() if (!attachments.length && SLASH_COMMAND_RE.test(visibleText)) { @@ -596,7 +597,7 @@ export function usePromptActions({ // window) so the caller can fall back to queueing the words for the next turn. const steerPrompt = useCallback( async (rawText: string): Promise => { - const text = rawText.trim() + const text = sanitizeComposerInput(rawText).trim() const sessionId = activeSessionId || activeSessionIdRef.current if (!text || !sessionId) { diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts index b1eaa4966..ae94d865d 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts @@ -3,6 +3,7 @@ import { type MutableRefObject, useCallback } from 'react' import { PROMPT_SUBMIT_REQUEST_TIMEOUT_MS } from '@/hermes' import type { Translations } from '@/i18n' import { type ChatMessage, textPart } from '@/lib/chat-messages' +import { sanitizeComposerInput } from '@/lib/composer-input-sanitize' import { optimisticAttachmentRef } from '@/lib/chat-runtime' import { setMutableRef } from '@/lib/mutable-ref' import { @@ -67,7 +68,7 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) { return useCallback( async (rawText: string, options?: SubmitTextOptions) => { - const visibleText = rawText.trim() + const visibleText = sanitizeComposerInput(rawText).trim() const usingComposerAttachments = !options?.attachments // Drop undefined/null holes a session switch or draft restore can leave in diff --git a/apps/desktop/src/components/assistant-ui/thread/user-edit-composer.tsx b/apps/desktop/src/components/assistant-ui/thread/user-edit-composer.tsx index b292f3f90..70a5472e5 100644 --- a/apps/desktop/src/components/assistant-ui/thread/user-edit-composer.tsx +++ b/apps/desktop/src/components/assistant-ui/thread/user-edit-composer.tsx @@ -57,6 +57,7 @@ import { Codicon } from '@/components/ui/codicon' import type { HermesGateway } from '@/hermes' import { useI18n } from '@/i18n' import { attachmentDisplayText, attachmentId, pathLabel } from '@/lib/chat-runtime' +import { sanitizeComposerInput } from '@/lib/composer-input-sanitize' import { DATA_IMAGE_URL_RE } from '@/lib/embedded-images' import { triggerHaptic } from '@/lib/haptics' import { Loader2Icon } from '@/lib/icons' @@ -188,7 +189,7 @@ export const UserEditComposer: FC = ({ cwd, gateway, sess const syncDraftFromEditor = useCallback( (editor: HTMLDivElement) => { - const nextDraft = composerPlainText(editor) + const nextDraft = sanitizeComposerInput(composerPlainText(editor)) if (nextDraft !== draftRef.current) { draftRef.current = nextDraft @@ -477,7 +478,7 @@ export const UserEditComposer: FC = ({ cwd, gateway, sess } const handlePaste = (event: ClipboardEvent) => { - const pastedText = event.clipboardData.getData('text') + const pastedText = sanitizeComposerInput(event.clipboardData.getData('text')) if (!pastedText || DATA_IMAGE_URL_RE.test(pastedText.trim())) { event.preventDefault() diff --git a/apps/desktop/src/lib/composer-input-sanitize.ts b/apps/desktop/src/lib/composer-input-sanitize.ts new file mode 100644 index 000000000..39f392681 --- /dev/null +++ b/apps/desktop/src/lib/composer-input-sanitize.ts @@ -0,0 +1,71 @@ +/** + * Strip terminal bracketed-paste leaks and repeated artifact tails from composer + * text before it is shown in the UI or sent to the gateway. + * + * Mirrors hermes_cli/input_sanitize.py (CLI/TUI gateway defensive path). + */ + +const BRACKETED_PASTE_BOUNDARY_START = /(^|[\s\n>:\]\)])\[200~/g +const BRACKETED_PASTE_BOUNDARY_END = /\[201~(?=$|[\s\n<[():;.,!?])/g +const BRACKETED_PASTE_DEGRADED_START = /(^|[\s\n>:\]\)])00~/g +const BRACKETED_PASTE_DEGRADED_END = /01~(?=$|[\s\n<[():;.,!?])/g + +const DESKTOP_PASTE_ARTIFACT = '~[[e' + +/** Strip leaked bracketed-paste wrapper markers from user-visible text. */ +export function stripLeakedBracketedPasteWrappers(text: string): string { + if (!text) { + return text + } + + let cleaned = text + .replace(/\x1b\[200~/g, '') + .replace(/\x1b\[201~/g, '') + .replace(/\^\[\[200~/g, '') + .replace(/\^\[\[201~/g, '') + + cleaned = cleaned.replace(BRACKETED_PASTE_BOUNDARY_START, '$1') + cleaned = cleaned.replace(BRACKETED_PASTE_BOUNDARY_END, '') + cleaned = cleaned.replace(BRACKETED_PASTE_DEGRADED_START, '$1') + cleaned = cleaned.replace(BRACKETED_PASTE_DEGRADED_END, '') + + return cleaned +} + +/** Drop a trailing run of the desktop ~[[e corruption signature (#62557). */ +export function collapseRepeatedInputArtifacts(text: string, minRepeats = 4): string { + if (!text) { + return text + } + + const marker = DESKTOP_PASTE_ARTIFACT + let index = text.length + let repeatCount = 0 + + while (index >= marker.length && text.slice(index - marker.length, index) === marker) { + repeatCount += 1 + index -= marker.length + } + + if (repeatCount < minRepeats) { + return text + } + + let start = index + if (start >= 2 && text.slice(start - 2, start) === '[e') { + start -= 2 + } else if (start >= 1 && text[start - 1] === '[') { + start -= 1 + } + + return text.slice(0, start) +} + +/** Normalize composer text before submit or draft persistence. */ +export function sanitizeComposerInput(text: string): string { + if (!text) { + return text + } + + return collapseRepeatedInputArtifacts(stripLeakedBracketedPasteWrappers(text)) +} diff --git a/cli.py b/cli.py index 6b2f00cf9..8a8fdf6f5 100644 --- a/cli.py +++ b/cli.py @@ -2995,29 +2995,9 @@ def _should_auto_attach_clipboard_image_on_paste(pasted_text: str) -> bool: def _strip_leaked_bracketed_paste_wrappers(text: str) -> str: - """Strip leaked bracketed-paste wrapper markers from user-visible text. + from hermes_cli.input_sanitize import strip_leaked_bracketed_paste_wrappers - Defensive normalization for cases where terminal/prompt_toolkit parsing - fails and bracketed-paste markers end up in the buffer as literal text. - - We strip canonical wrappers unconditionally and also handle degraded - visible forms like ``[200~`` / ``[201~`` and ``00~`` / ``01~`` when they - look like wrapper boundaries, not arbitrary user content. - """ - if not text: - return text - - text = ( - text.replace("\x1b[200~", "") - .replace("\x1b[201~", "") - .replace("^[[200~", "") - .replace("^[[201~", "") - ) - text = re.sub(r"(^|[\s\n>:\]\)])\[200~", r"\1", text) - text = re.sub(r"\[201~(?=$|[\s\n<\[\(\):;.,!?])", "", text) - text = re.sub(r"(^|[\s\n>:\]\)])00~", r"\1", text) - text = re.sub(r"01~(?=$|[\s\n<\[\(\):;.,!?])", "", text) - return text + return strip_leaked_bracketed_paste_wrappers(text) def _apply_bracketed_paste_timeout_patch() -> None: diff --git a/hermes_cli/input_sanitize.py b/hermes_cli/input_sanitize.py new file mode 100644 index 000000000..90ca4c4d3 --- /dev/null +++ b/hermes_cli/input_sanitize.py @@ -0,0 +1,70 @@ +"""Sanitize user prompt text leaked from terminal / paste control sequences.""" + +from __future__ import annotations + +import re + +_BRACKETED_PASTE_BOUNDARY_START = re.compile(r"(^|[\s\n>:\]\)])\[200~") +_BRACKETED_PASTE_BOUNDARY_END = re.compile(r"\[201~(?=$|[\s\n<\[\(\):;.,!?])") +_BRACKETED_PASTE_DEGRADED_START = re.compile(r"(^|[\s\n>:\]\)])00~") +_BRACKETED_PASTE_DEGRADED_END = re.compile(r"01~(?=$|[\s\n<\[\(\):;.,!?])") + +# Corruption signature from desktop bracketed-paste leaks (#62557). +_DESKTOP_PASTE_ARTIFACT = "~[[e" + + +def strip_leaked_bracketed_paste_wrappers(text: str) -> str: + """Strip leaked bracketed-paste wrapper markers from user-visible text. + + Defensive normalization for cases where terminal/prompt_toolkit parsing + fails and bracketed-paste markers end up in the buffer as literal text. + + Canonical wrappers are stripped unconditionally. Degraded visible forms like + ``[200~`` / ``[201~`` and ``00~`` / ``01~`` are removed only at boundaries + so embedded literals such as ``literal[200~tag`` stay intact. + """ + if not text: + return text + + text = ( + text.replace("\x1b[200~", "") + .replace("\x1b[201~", "") + .replace("^[[200~", "") + .replace("^[[201~", "") + ) + text = _BRACKETED_PASTE_BOUNDARY_START.sub(r"\1", text) + text = _BRACKETED_PASTE_BOUNDARY_END.sub("", text) + text = _BRACKETED_PASTE_DEGRADED_START.sub(r"\1", text) + text = _BRACKETED_PASTE_DEGRADED_END.sub("", text) + return text + + +def collapse_repeated_input_artifacts(text: str, min_repeats: int = 4) -> str: + """Drop a trailing run of the desktop ~[[e corruption signature (#62557).""" + if not text: + return text + + marker = _DESKTOP_PASTE_ARTIFACT + index = len(text) + repeat_count = 0 + while index >= len(marker) and text[index - len(marker) : index] == marker: + repeat_count += 1 + index -= len(marker) + + if repeat_count < min_repeats: + return text + + start = index + if start >= 2 and text[start - 2 : start] == "[e": + start -= 2 + elif start >= 1 and text[start - 1] == "[": + start -= 1 + return text[:start] + + +def sanitize_user_prompt_text(text: str) -> str: + """Normalize user-authored prompt text before persistence or model input.""" + if not isinstance(text, str) or not text: + return text + cleaned = strip_leaked_bracketed_paste_wrappers(text) + return collapse_repeated_input_artifacts(cleaned) diff --git a/tui_gateway/server.py b/tui_gateway/server.py index febb4ec0d..2f6e83393 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -8463,7 +8463,11 @@ def _(rid, params: dict) -> dict: @method("prompt.submit") def _(rid, params: dict) -> dict: - sid, text = params.get("session_id", ""), params.get("text", "") + from hermes_cli.input_sanitize import sanitize_user_prompt_text + + sid = params.get("session_id", "") + raw_text = params.get("text", "") + text = sanitize_user_prompt_text(raw_text) if isinstance(raw_text, str) else raw_text truncate_user_ordinal = params.get("truncate_before_user_ordinal") session, err = _sess_nowait(params, rid) if err: From 2388e0687bd07e9d22d2649fe50010e6016036e0 Mon Sep 17 00:00:00 2001 From: HexLab98 Date: Sun, 12 Jul 2026 06:54:55 +0700 Subject: [PATCH 18/47] test(input): preservation regressions and prompt.submit boundary (#62557) Add cases for mid-string markers, trailing punctuation, and insufficient tail repeats; verify prompt.submit passes sanitized text to run_conversation. --- .../src/lib/composer-input-sanitize.test.ts | 51 +++++++++++++++++++ tests/hermes_cli/test_input_sanitize.py | 51 +++++++++++++++++++ tests/test_tui_gateway_server.py | 45 ++++++++++++++++ 3 files changed, 147 insertions(+) create mode 100644 apps/desktop/src/lib/composer-input-sanitize.test.ts create mode 100644 tests/hermes_cli/test_input_sanitize.py diff --git a/apps/desktop/src/lib/composer-input-sanitize.test.ts b/apps/desktop/src/lib/composer-input-sanitize.test.ts new file mode 100644 index 000000000..2695f942e --- /dev/null +++ b/apps/desktop/src/lib/composer-input-sanitize.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, it } from 'vitest' + +import { + collapseRepeatedInputArtifacts, + sanitizeComposerInput, + stripLeakedBracketedPasteWrappers +} from './composer-input-sanitize' + +describe('stripLeakedBracketedPasteWrappers', () => { + it('leaves plain text unchanged', () => { + expect(stripLeakedBracketedPasteWrappers('hello world')).toBe('hello world') + }) + + it('strips canonical escape wrappers', () => { + expect(stripLeakedBracketedPasteWrappers('\x1b[200~hello\x1b[201~')).toBe('hello') + }) + + it('keeps embedded literal bracket forms', () => { + const text = 'literal[200~tag and literal[201~tag should stay' + expect(stripLeakedBracketedPasteWrappers(text)).toBe(text) + }) +}) + +describe('collapseRepeatedInputArtifacts', () => { + it('removes the desktop corruption tail from #62557', () => { + const prefix = '需要时随时叫我。' + const tail = '[e~[[e' + '~[[e'.repeat(20) + expect(collapseRepeatedInputArtifacts(prefix + tail)).toBe(prefix) + }) + + it('preserves a mid-string marker followed by valid suffix', () => { + const text = 'notes ~[[e more text here' + expect(collapseRepeatedInputArtifacts(text)).toBe(text) + }) + + it('preserves trailing punctuation that is not the corruption signature', () => { + expect(collapseRepeatedInputArtifacts('wait....')).toBe('wait....') + }) + + it('does not strip when fewer than minRepeats markers appear at the tail', () => { + const text = 'hello~[[e~[[e' + expect(collapseRepeatedInputArtifacts(text)).toBe(text) + }) +}) + +describe('sanitizeComposerInput', () => { + it('normalizes wrappers and repeated artifact tails together', () => { + const corrupted = 'hello[' + '~[[e'.repeat(8) + expect(sanitizeComposerInput(corrupted)).toBe('hello') + }) +}) diff --git a/tests/hermes_cli/test_input_sanitize.py b/tests/hermes_cli/test_input_sanitize.py new file mode 100644 index 000000000..b0e86e099 --- /dev/null +++ b/tests/hermes_cli/test_input_sanitize.py @@ -0,0 +1,51 @@ +"""Tests for shared user prompt input sanitization.""" + +from hermes_cli.input_sanitize import ( + collapse_repeated_input_artifacts, + sanitize_user_prompt_text, + strip_leaked_bracketed_paste_wrappers, +) + + +class TestStripLeakedBracketedPasteWrappers: + def test_plain_text_unchanged(self): + assert strip_leaked_bracketed_paste_wrappers("hello world") == "hello world" + + def test_strips_canonical_escape_wrappers(self): + assert strip_leaked_bracketed_paste_wrappers("\x1b[200~hello\x1b[201~") == "hello" + + def test_strips_visible_caret_escape_wrappers(self): + assert strip_leaked_bracketed_paste_wrappers("^[[200~hello^[[201~") == "hello" + + def test_does_not_strip_non_wrapper_bracket_forms_in_normal_text(self): + text = "literal[200~tag and literal[201~tag should stay" + assert strip_leaked_bracketed_paste_wrappers(text) == text + + +class TestCollapseRepeatedInputArtifacts: + def test_issue_62557_corruption_tail(self): + prefix = "需要时随时叫我。" + tail = "[e~[[e" + "~[[e" * 20 + assert collapse_repeated_input_artifacts(prefix + tail) == prefix + + def test_plain_text_unchanged(self): + text = "build00~tag should stay" + assert collapse_repeated_input_artifacts(text) == text + + def test_mid_string_marker_followed_by_suffix_preserved(self): + text = "notes ~[[e more text here" + assert collapse_repeated_input_artifacts(text) == text + + def test_trailing_punctuation_preserved(self): + assert collapse_repeated_input_artifacts("wait....") == "wait...." + + def test_insufficient_tail_repeats_preserved(self): + text = "hello~[[e~[[e" + assert collapse_repeated_input_artifacts(text) == text + + +class TestSanitizeUserPromptText: + def test_combines_wrapper_strip_and_tail_collapse(self): + prefix = "hello[" + corrupted = prefix + "~[[e" * 8 + assert sanitize_user_prompt_text(corrupted) == "hello" diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index f117af57a..3b591156e 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -5093,6 +5093,51 @@ def test_prompt_submit_history_version_mismatch_surfaces_warning(monkeypatch): server._sessions.pop("sid", None) +def test_prompt_submit_sanitizes_bracketed_paste_before_agent(monkeypatch): + """prompt.submit must sanitize corrupted user text before run_conversation.""" + captured: dict[str, str] = {} + + class _Agent: + def run_conversation( + self, prompt, conversation_history=None, stream_callback=None + ): + captured["prompt"] = prompt + return { + "final_response": "ok", + "messages": [{"role": "assistant", "content": "ok"}], + } + + class _ImmediateThread: + def __init__(self, target=None, daemon=None, **kw): + self._target = target + + def start(self): + self._target() + + corrupted = "hello[" + "~[[e" * 8 + server._sessions["sid"] = _session(agent=_Agent()) + try: + monkeypatch.setattr(server.threading, "Thread", _ImmediateThread) + monkeypatch.setattr(server, "_get_usage", lambda _a: {}) + monkeypatch.setattr(server, "render_message", lambda _t, _c: "") + monkeypatch.setattr(server, "_emit", lambda *a: None) + monkeypatch.setattr(server, "_start_agent_build", lambda *a, **k: None) + monkeypatch.setattr(server, "_ensure_session_db_row", lambda *a, **k: None) + monkeypatch.setattr(server, "_persist_branch_seed", lambda *a, **k: None) + + resp = server.handle_request( + { + "id": "1", + "method": "prompt.submit", + "params": {"session_id": "sid", "text": corrupted}, + } + ) + assert resp.get("result"), f"got error: {resp.get('error')}" + assert captured["prompt"] == "hello" + finally: + server._sessions.pop("sid", None) + + def test_prompt_submit_history_version_match_persists_normally(monkeypatch): """Regression guard: the backstop does not affect the happy path.""" From f3ec79964ec178cc4eb4ece2b301fc8d902e85dd Mon Sep 17 00:00:00 2001 From: liuhao1024 Date: Tue, 14 Jul 2026 19:15:10 +0800 Subject: [PATCH 19/47] fix(auxiliary_client): add backward compatibility for build_keepalive_http_client import (#64333) --- agent/auxiliary_client.py | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py index 5f2d7e806..bfb534a99 100644 --- a/agent/auxiliary_client.py +++ b/agent/auxiliary_client.py @@ -102,7 +102,6 @@ OpenAI = _OpenAIProxy() # module-level name, resolves lazily on call/isinstance from agent.credential_pool import load_pool from agent.model_metadata import MINIMUM_CONTEXT_LENGTH, get_model_context_length -from agent.process_bootstrap import build_keepalive_http_client from hermes_cli.config import get_hermes_home from hermes_constants import OPENROUTER_BASE_URL from utils import base_url_host_matches, base_url_hostname, env_float, model_forces_max_completion_tokens, normalize_proxy_env_vars @@ -162,16 +161,27 @@ def _openai_http_client_kwargs( async_mode: bool = False, ) -> Dict[str, Any]: """Inject keepalive httpx client with env-only proxy (not macOS system proxy).""" - client = build_keepalive_http_client( - str(base_url or ""), - async_mode=async_mode, - verify=_resolve_aux_verify(base_url), - ) + try: + from agent.process_bootstrap import build_keepalive_http_client + client = build_keepalive_http_client( + str(base_url or ""), + async_mode=async_mode, + verify=_resolve_aux_verify(base_url), + ) + except (ImportError, AttributeError): + # Fallback for older hermes-agent versions without build_keepalive_http_client. + # This can happen when Desktop users run with stale code (#64333). + # Without the keepalive client, auxiliary calls use the OpenAI SDK's default + # httpx client, which respects macOS system proxy (less predictable for env-only + # proxy use) and has no pool-level keepalive expiry (connections may live longer). + # This is a graceful degradation — functionality still works, just with slightly + # different proxy/keepalive behavior. + client = None + if client is None: return {} return {"http_client": client} - def _create_openai_client(*, api_key: str, base_url: str, **kwargs: Any) -> Any: kwargs = {**_openai_http_client_kwargs(base_url), **kwargs} # Hermes owns auxiliary retry + provider/model fallback policy (the From 07be37d996be7df1965441ca8bdacdb3f884c7e2 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 14 Jul 2026 14:22:38 -0700 Subject: [PATCH 20/47] fix(auxiliary_client): warn once + regression tests for bootstrap version skew Follow-up on the salvaged fallback: silent degradation is how #64333 went unnoticed (jobs dead on arrival, only errors.log knew). Warn once with a resync hint, and cover both the skewed and healthy paths with tests. --- agent/auxiliary_client.py | 30 ++++++++---- .../test_auxiliary_client_bootstrap_skew.py | 47 +++++++++++++++++++ 2 files changed, 69 insertions(+), 8 deletions(-) create mode 100644 tests/agent/test_auxiliary_client_bootstrap_skew.py diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py index bfb534a99..a0a086497 100644 --- a/agent/auxiliary_client.py +++ b/agent/auxiliary_client.py @@ -155,6 +155,9 @@ def _resolve_aux_verify(base_url: Optional[str]) -> Any: return True +_WARNED_KEEPALIVE_IMPORT_SKEW = False + + def _openai_http_client_kwargs( base_url: Optional[str], *, @@ -169,15 +172,26 @@ def _openai_http_client_kwargs( verify=_resolve_aux_verify(base_url), ) except (ImportError, AttributeError): - # Fallback for older hermes-agent versions without build_keepalive_http_client. - # This can happen when Desktop users run with stale code (#64333). - # Without the keepalive client, auxiliary calls use the OpenAI SDK's default - # httpx client, which respects macOS system proxy (less predictable for env-only - # proxy use) and has no pool-level keepalive expiry (connections may live longer). - # This is a graceful degradation — functionality still works, just with slightly - # different proxy/keepalive behavior. + # Version-skewed installs (#64333): a process whose sys.path resolves + # an older agent/process_bootstrap.py without this helper — seen when + # the Desktop app's bundled runtime lags a git-installed source tree + # that newer callers (cron scheduler) were written against. Every cron + # job died on this ImportError before any agent logic ran. Degrade + # gracefully to the OpenAI SDK's default httpx client (respects macOS + # system proxy, no pool-level keepalive expiry) instead of failing the + # whole job, and say so once — silent version skew is how this bug + # went unnoticed until jobs were already dead on arrival. + global _WARNED_KEEPALIVE_IMPORT_SKEW + if not _WARNED_KEEPALIVE_IMPORT_SKEW: + _WARNED_KEEPALIVE_IMPORT_SKEW = True + logger.warning( + "agent.process_bootstrap.build_keepalive_http_client is " + "unavailable — mixed/stale install detected (#64333). Falling " + "back to the SDK default HTTP client. Run `hermes update` (or " + "reinstall the Desktop app) to resync the runtime." + ) client = None - + if client is None: return {} return {"http_client": client} diff --git a/tests/agent/test_auxiliary_client_bootstrap_skew.py b/tests/agent/test_auxiliary_client_bootstrap_skew.py new file mode 100644 index 000000000..d66b436c1 --- /dev/null +++ b/tests/agent/test_auxiliary_client_bootstrap_skew.py @@ -0,0 +1,47 @@ +"""Regression for #64333 — auxiliary client must survive a version-skewed +agent.process_bootstrap that lacks build_keepalive_http_client. + +Desktop installs can run a runtime whose agent/process_bootstrap.py predates +the helper while newer callers (cron scheduler → auxiliary_client) expect it. +Before the fix the module-level import made every cron job die with +ImportError before any agent logic ran. +""" + +import builtins +import logging + +import agent.auxiliary_client as aux + + +def test_missing_bootstrap_helper_degrades_instead_of_raising(monkeypatch, caplog): + """ImportError from process_bootstrap → empty kwargs + one-time warning.""" + real_import = builtins.__import__ + + def _fail_bootstrap(name, *args, **kwargs): + if name == "agent.process_bootstrap": + raise ImportError( + "cannot import name 'build_keepalive_http_client' " + "from 'agent.process_bootstrap'" + ) + return real_import(name, *args, **kwargs) + + monkeypatch.setattr(builtins, "__import__", _fail_bootstrap) + monkeypatch.setattr(aux, "_WARNED_KEEPALIVE_IMPORT_SKEW", False) + + with caplog.at_level(logging.WARNING, logger="agent.auxiliary_client"): + result = aux._openai_http_client_kwargs("https://api.example.com/v1") + again = aux._openai_http_client_kwargs("https://api.example.com/v1") + + assert result == {} + assert again == {} + skew_warnings = [ + r for r in caplog.records if "mixed/stale install" in r.getMessage() + ] + assert len(skew_warnings) == 1 # warned once, not per call + + +def test_healthy_bootstrap_still_injects_keepalive_client(): + """With the helper present, the keepalive http_client is injected.""" + result = aux._openai_http_client_kwargs("https://api.example.com/v1") + assert "http_client" in result + assert result["http_client"] is not None From 678b86df1cf33d3ced5858bb9ae61f5e7c5d4018 Mon Sep 17 00:00:00 2001 From: ethernet Date: Tue, 14 Jul 2026 18:05:00 -0400 Subject: [PATCH 21/47] fix(desktop): worktree from origin/main should not set up upstream tracking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When "new worktree" branches off a remote-tracking ref like origin/main, `git worktree add -b origin/main` auto-sets upstream tracking (branch → origin/main), producing `branch:origin/main` in branch listings. The user wants a standalone local branch — like `git checkout origin/main && git checkout -b branch` — not one silently wired to the remote. Add `--no-track` when the base is an `origin/` ref. Local branch bases are unaffected (they never triggered tracking). --- .../desktop/electron/git-worktree-ops.test.ts | 36 +++++++++++++++++++ apps/desktop/electron/git-worktree-ops.ts | 9 ++++- 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/apps/desktop/electron/git-worktree-ops.test.ts b/apps/desktop/electron/git-worktree-ops.test.ts index a543ecd39..be40e9fe5 100644 --- a/apps/desktop/electron/git-worktree-ops.test.ts +++ b/apps/desktop/electron/git-worktree-ops.test.ts @@ -262,3 +262,39 @@ test('addWorktree: base param branches off a specified local branch', async () = fs.rmSync(dir, { recursive: true, force: true }) } }) + +test('addWorktree: base origin/main does not set up upstream tracking', async () => { + // Two repos: a bare "remote" and a clone, so origin/main resolves as a + // remote-tracking ref — the condition that triggers auto-tracking. + const remoteDir = fs.mkdtempSync(path.join(os.tmpdir(), 'hermes-remote-')) + const cloneDir = fs.mkdtempSync(path.join(os.tmpdir(), 'hermes-clone-')) + const git = (...args) => execFileSync('git', args, { cwd: cloneDir }).toString().trim() + + try { + // Seed the remote with a commit on main. + execFileSync('git', ['init', '-b', 'main', remoteDir]) + execFileSync('git', ['-C', remoteDir, 'commit', '--allow-empty', '-m', 'root']) + + // Clone so origin/main exists as a remote-tracking ref. + execFileSync('git', ['clone', remoteDir, cloneDir]) + + const result = await addWorktree(cloneDir, { base: 'origin/main', branch: 'feature-branch', name: 'feature-branch' }, 'git') + + assert.equal(result.branch, 'feature-branch') + + // The new branch must NOT have an upstream — like `git checkout origin/main + // && git checkout -b feature-branch`, not `git worktree add -b … origin/main`. + let hasUpstream = true + + try { + execFileSync('git', ['-C', result.path, 'rev-parse', '--abbrev-ref', '--symbolic-full-name', '@{u}']) + } catch { + hasUpstream = false + } + + assert.equal(hasUpstream, false) + } finally { + fs.rmSync(remoteDir, { recursive: true, force: true }) + fs.rmSync(cloneDir, { recursive: true, force: true }) + } +}) diff --git a/apps/desktop/electron/git-worktree-ops.ts b/apps/desktop/electron/git-worktree-ops.ts index e84324a00..1acd029e1 100644 --- a/apps/desktop/electron/git-worktree-ops.ts +++ b/apps/desktop/electron/git-worktree-ops.ts @@ -265,8 +265,15 @@ async function addWorktree(repoPath, options, gitBin) { } catch { // The fetch isn't mandatory, but it would be nice to do if possible. // If it's not possible, just use the local ref of the remote branch. - // If it doesn't exist locally, we'll get an error anyways + // If it doesn't exist locally, we'll get an error } + + // When branching off a remote-tracking ref, git auto-sets up tracking + // (e.g. `new-branch` → tracks `origin/main`). The user almost certainly + // wants a standalone local branch — like `git checkout origin/main && + // git checkout -b new-branch` — not a branch silently wired to the + // remote's upstream. `--no-track` prevents that. + args.push('--no-track') } args.push(base) From 1d48863b856d7a82412e1b47d87e30d0378b851f Mon Sep 17 00:00:00 2001 From: ethernet Date: Wed, 15 Jul 2026 11:34:59 -0400 Subject: [PATCH 22/47] fix(desktop): inline git identity in worktree tracking test for CI CI runners have no global git identity. The remote repo seed commit needs inline -c user.email/user.name flags, matching the pattern already used by ensureGitRepo. --- apps/desktop/electron/git-worktree-ops.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/desktop/electron/git-worktree-ops.test.ts b/apps/desktop/electron/git-worktree-ops.test.ts index be40e9fe5..63378c293 100644 --- a/apps/desktop/electron/git-worktree-ops.test.ts +++ b/apps/desktop/electron/git-worktree-ops.test.ts @@ -271,9 +271,10 @@ test('addWorktree: base origin/main does not set up upstream tracking', async () const git = (...args) => execFileSync('git', args, { cwd: cloneDir }).toString().trim() try { - // Seed the remote with a commit on main. + // Seed the remote with a commit on main. Inline identity so it works + // on CI runners with no global git config. execFileSync('git', ['init', '-b', 'main', remoteDir]) - execFileSync('git', ['-C', remoteDir, 'commit', '--allow-empty', '-m', 'root']) + execFileSync('git', ['-C', remoteDir, '-c', 'user.email=hermes@localhost', '-c', 'user.name=Hermes', 'commit', '--allow-empty', '-m', 'root']) // Clone so origin/main exists as a remote-tracking ref. execFileSync('git', ['clone', remoteDir, cloneDir]) From 1600008ab00e5a805b69f7ad89a4ed898dc111a6 Mon Sep 17 00:00:00 2001 From: ethernet Date: Wed, 15 Jul 2026 11:50:28 -0400 Subject: [PATCH 23/47] fix(desktop): show +/- summary on collapsed review folders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ReviewDirRow never rendered a DiffCount, so collapsed folders gave no indication of the additions/deletions inside them. The tree builder already aggregates added/removed onto directory nodes (verified by tree-data tests) — this just renders them, matching the file rows. --- apps/desktop/src/app/right-sidebar/review/file-tree.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/desktop/src/app/right-sidebar/review/file-tree.tsx b/apps/desktop/src/app/right-sidebar/review/file-tree.tsx index e09c964b7..5eb66c0fc 100644 --- a/apps/desktop/src/app/right-sidebar/review/file-tree.tsx +++ b/apps/desktop/src/app/right-sidebar/review/file-tree.tsx @@ -218,6 +218,7 @@ function ReviewDirRow({ {node.name} + {!open && } {open && node.children && ( From 868a2f7d172a5e6f959bab327d3e4a0159cfc477 Mon Sep 17 00:00:00 2001 From: AlexFucuson9 Date: Tue, 7 Jul 2026 11:34:40 +0700 Subject: [PATCH 24/47] fix: handle list content in _serialize_for_summary for multimodal messages msg.get('content') can return a list of parts for multimodal messages (containing text, images, etc.). The old code passed this list directly to redact_sensitive_text(text: str), which raised AttributeError on list.replace(), causing context compression to fail entirely for any session with attached images. Fix: detect list content and extract text parts before redacting. Image parts are replaced with '[image]' placeholder. --- agent/context_compressor.py | 16 ++++++- .../agent/test_compressor_media_stripping.py | 46 +++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/agent/context_compressor.py b/agent/context_compressor.py index 5ff794acd..e16778cd3 100644 --- a/agent/context_compressor.py +++ b/agent/context_compressor.py @@ -1664,7 +1664,21 @@ class ContextCompressor(ContextEngine): parts = [] for msg in turns: role = msg.get("role", "unknown") - content = redact_sensitive_text(msg.get("content") or "") + content = msg.get("content") + if isinstance(content, list): + text_parts: list[str] = [] + for part in content: + if isinstance(part, dict): + if part.get("type") == "text": + text_parts.append(part.get("text", "")) + elif part.get("type") in { + "image", "image_url", "input_image", + }: + text_parts.append("[image]") + elif isinstance(part, str): + text_parts.append(part) + content = "\n".join(text_parts) + content = redact_sensitive_text(content or "") content = _MEDIA_DIRECTIVE_RE.sub("[media attachment]", content) # Strip inline reasoning blocks (, , etc.) from # assistant content before it reaches the summarizer. Reasoning diff --git a/tests/agent/test_compressor_media_stripping.py b/tests/agent/test_compressor_media_stripping.py index f995ac98a..29ca272a4 100644 --- a/tests/agent/test_compressor_media_stripping.py +++ b/tests/agent/test_compressor_media_stripping.py @@ -54,3 +54,49 @@ class TestMediaDirectiveStripping: result = compressor._serialize_for_summary(turns) assert "MEDIA:" not in result assert result.count("[media attachment]") == 2 + + def test_multimodal_list_content_does_not_crash(self, compressor): + """content as a list (multimodal) must not crash redact_sensitive_text. + + Regression: msg.get('content') can return a list of parts for + multimodal messages (images + text). The old code passed this + list directly to redact_sensitive_text(str), which raised + AttributeError on list.replace(). + """ + turns = [ + { + "role": "user", + "content": [ + {"type": "text", "text": "What is in this image?"}, + {"type": "image_url", "image_url": {"url": "data:image/png;base64,abc123"}}, + ], + }, + ] + # Must not raise + result = compressor._serialize_for_summary(turns) + assert "What is in this image?" in result + assert "[image]" in result + + def test_multimodal_list_text_parts_extracted(self, compressor): + """Text parts from multimodal list content are preserved in output.""" + turns = [ + { + "role": "user", + "content": [ + {"type": "text", "text": "first part"}, + {"type": "text", "text": "second part"}, + ], + }, + ] + result = compressor._serialize_for_summary(turns) + assert "first part" in result + assert "second part" in result + + def test_multimodal_list_bare_strings_handled(self, compressor): + """Bare strings inside a content list are joined.""" + turns = [ + {"role": "user", "content": ["hello", "world"]}, + ] + result = compressor._serialize_for_summary(turns) + assert "hello" in result + assert "world" in result From 704bbcca804c443dbc0d0ad82d7fc50b6549380d Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 15 Jul 2026 07:42:33 -0700 Subject: [PATCH 25/47] feat(compressor): keep image URLs and unknown-part markers in summarizer serialization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up on @AlexFucuson9's cherry-picked multimodal flattening: - http(s) image parts render as '[image: ]' so the summary keeps a referenceable handle after compaction (base64 data: URLs still collapse to '[image]' — no reusable reference, and leaking them is the bug being fixed) - unknown part types (document, future shapes) render as '[]' instead of being silently dropped - test docstrings corrected: the original crash premise is stale (redact_sensitive_text grew str() coercion in #52147); the live bug is repr-noise + base64 leakage into the summarizer input --- agent/context_compressor.py | 34 +++++++++++++--- .../agent/test_compressor_media_stripping.py | 40 ++++++++++++++++--- 2 files changed, 63 insertions(+), 11 deletions(-) diff --git a/agent/context_compressor.py b/agent/context_compressor.py index e16778cd3..b652d4ceb 100644 --- a/agent/context_compressor.py +++ b/agent/context_compressor.py @@ -586,6 +586,27 @@ def _strip_historical_media(messages: List[Dict[str, Any]]) -> List[Dict[str, An return result if changed else messages +def _image_part_label(part: Dict[str, Any]) -> str: + """Render a multimodal image part as a short text label for the summarizer. + + Keeps a real, referenceable URL when the image lives at an http(s) + address — the summary can then preserve the handle so the agent (or a + later vision_analyze call) can still reach the image after compaction. + Base64 ``data:`` URLs carry no reusable reference and would flood the + summarizer input, so they collapse to ``[image]``. + """ + url = "" + if isinstance(part.get("image_url"), dict): + url = str(part["image_url"].get("url") or "") + elif isinstance(part.get("image_url"), str): + url = part["image_url"] + elif isinstance(part.get("url"), str): + url = part["url"] + if url.startswith(("http://", "https://")): + return f"[image: {url}]" + return "[image]" + + def _str_arg(args: dict, key: str, default: str = "") -> str: """Safely get a string argument from parsed tool args. @@ -1669,12 +1690,15 @@ class ContextCompressor(ContextEngine): text_parts: list[str] = [] for part in content: if isinstance(part, dict): - if part.get("type") == "text": + ptype = part.get("type") + if ptype == "text": text_parts.append(part.get("text", "")) - elif part.get("type") in { - "image", "image_url", "input_image", - }: - text_parts.append("[image]") + elif ptype in {"image", "image_url", "input_image"}: + text_parts.append(_image_part_label(part)) + else: + # Unknown part type — keep a marker so the + # summarizer knows content existed here. + text_parts.append(f"[{ptype or 'attachment'}]") elif isinstance(part, str): text_parts.append(part) content = "\n".join(text_parts) diff --git a/tests/agent/test_compressor_media_stripping.py b/tests/agent/test_compressor_media_stripping.py index 29ca272a4..2ae9cbf59 100644 --- a/tests/agent/test_compressor_media_stripping.py +++ b/tests/agent/test_compressor_media_stripping.py @@ -56,12 +56,11 @@ class TestMediaDirectiveStripping: assert result.count("[media attachment]") == 2 def test_multimodal_list_content_does_not_crash(self, compressor): - """content as a list (multimodal) must not crash redact_sensitive_text. + """content as a list (multimodal) must be flattened to clean text. - Regression: msg.get('content') can return a list of parts for - multimodal messages (images + text). The old code passed this - list directly to redact_sensitive_text(str), which raised - AttributeError on list.replace(). + Without flattening, str() coercion in redact_sensitive_text dumps + the raw part-dict repr — including base64 image data — into the + summarizer input, burning summary budget on noise. """ turns = [ { @@ -72,10 +71,39 @@ class TestMediaDirectiveStripping: ], }, ] - # Must not raise result = compressor._serialize_for_summary(turns) assert "What is in this image?" in result assert "[image]" in result + assert "base64" not in result + + def test_multimodal_remote_image_keeps_url(self, compressor): + """http(s) image parts keep their URL as a referenceable handle.""" + turns = [ + { + "role": "user", + "content": [ + {"type": "text", "text": "look at this"}, + {"type": "image_url", "image_url": {"url": "https://example.com/a.png"}}, + ], + }, + ] + result = compressor._serialize_for_summary(turns) + assert "[image: https://example.com/a.png]" in result + + def test_multimodal_unknown_part_type_keeps_marker(self, compressor): + """Unknown part types are not silently dropped.""" + turns = [ + { + "role": "user", + "content": [ + {"type": "text", "text": "see attachment"}, + {"type": "document", "title": "spec.pdf"}, + ], + }, + ] + result = compressor._serialize_for_summary(turns) + assert "see attachment" in result + assert "[document]" in result def test_multimodal_list_text_parts_extracted(self, compressor): """Text parts from multimodal list content are preserved in output.""" From fe1ab949fdcfac4027bdb7833362d78ee8f81467 Mon Sep 17 00:00:00 2001 From: Sk <574933+sk-holmes@users.noreply.github.com> Date: Tue, 30 Jun 2026 00:28:19 -0700 Subject: [PATCH 26/47] fix(agent): treat Codex incomplete content filter as refusal Map Codex Responses status=incomplete with incomplete_details.reason=content_filter to finish_reason=content_filter so the existing refusal/fallback path runs instead of burning incomplete continuation attempts. --- agent/codex_responses_adapter.py | 33 ++++++++-- agent/conversation_loop.py | 6 ++ agent/transports/codex.py | 20 +++++-- tests/agent/test_codex_responses_adapter.py | 15 +++++ .../agent/transports/test_codex_transport.py | 28 +++++++++ tests/run_agent/test_run_agent.py | 60 +++++++++++++++++++ 6 files changed, 152 insertions(+), 10 deletions(-) diff --git a/agent/codex_responses_adapter.py b/agent/codex_responses_adapter.py index f3f5ec3da..bce372ebb 100644 --- a/agent/codex_responses_adapter.py +++ b/agent/codex_responses_adapter.py @@ -1118,6 +1118,22 @@ def _normalize_codex_response( differs from the one that minted the encrypted_content blob and drop the item instead of triggering HTTP 400 invalid_encrypted_content. """ + response_status = getattr(response, "status", None) + if isinstance(response_status, str): + response_status = response_status.strip().lower() + else: + response_status = None + + incomplete_details = getattr(response, "incomplete_details", None) + incomplete_reason = "" + if isinstance(incomplete_details, dict): + incomplete_reason = str(incomplete_details.get("reason") or "").strip().lower() + elif incomplete_details is not None: + incomplete_reason = str(getattr(incomplete_details, "reason", "") or "").strip().lower() + response_incomplete_content_filter = ( + response_status == "incomplete" and incomplete_reason == "content_filter" + ) + output = getattr(response, "output", None) if not isinstance(output, list) or not output: # The Codex backend can return empty output when the answer was @@ -1134,15 +1150,18 @@ def _normalize_codex_response( content=[SimpleNamespace(type="output_text", text=out_text.strip())], )] response.output = output + elif response_incomplete_content_filter: + # This is a deterministic provider safety block, not a partial + # answer. Synthesize an empty message so finish_reason below becomes + # content_filter and the conversation loop can fallback/surface it + # instead of burning three continuation attempts. + output = [SimpleNamespace( + type="message", role="assistant", status="completed", content=[] + )] + response.output = output else: raise RuntimeError("Responses API returned no output items") - response_status = getattr(response, "status", None) - if isinstance(response_status, str): - response_status = response_status.strip().lower() - else: - response_status = None - if response_status in {"failed", "cancelled"}: error_obj = getattr(response, "error", None) error_msg = _format_responses_error(error_obj, response_status) @@ -1411,6 +1430,8 @@ def _normalize_codex_response( if tool_calls: finish_reason = "tool_calls" + elif response_incomplete_content_filter: + finish_reason = "content_filter" elif leaked_tool_call_text: finish_reason = "incomplete" elif saw_streaming_or_item_incomplete: diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index a628093e1..8986a2ecd 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -1642,12 +1642,16 @@ def run_conversation( # Check finish_reason before proceeding if agent.api_mode == "codex_responses": status = getattr(response, "status", None) + if isinstance(status, str): + status = status.strip().lower() incomplete_details = getattr(response, "incomplete_details", None) incomplete_reason = None if isinstance(incomplete_details, dict): incomplete_reason = incomplete_details.get("reason") else: incomplete_reason = getattr(incomplete_details, "reason", None) + if incomplete_reason is not None: + incomplete_reason = str(incomplete_reason).strip().lower() if status == "incomplete" and incomplete_reason in {"max_output_tokens", "length"}: # Responses API max-output exhaustion is a normal # Codex incomplete turn. Let the Codex-specific @@ -1657,6 +1661,8 @@ def run_conversation( # emits "Response truncated due to output length # limit" and stops gateway turns. finish_reason = "incomplete" + elif status == "incomplete" and incomplete_reason == "content_filter": + finish_reason = "content_filter" else: finish_reason = "stop" elif agent.api_mode == "anthropic_messages": diff --git a/agent/transports/codex.py b/agent/transports/codex.py index 56f259632..ef7ffbaff 100644 --- a/agent/transports/codex.py +++ b/agent/transports/codex.py @@ -435,15 +435,27 @@ class ResponsesApiTransport(ProviderTransport): def validate_response(self, response: Any) -> bool: """Check Codex Responses API response has valid output structure. - Returns True only if response.output is a non-empty list. - Does NOT check output_text fallback — the caller handles that - with diagnostic logging for stream backfill recovery. + Returns True only if response.output is a non-empty list. Also treats + terminal content-filter incomplete responses as valid: the Responses API + may return status=incomplete with incomplete_details.reason='content_filter' + and no output items. That is a provider refusal signal, not a malformed + response, and must reach normalization so the agent loop can use the + content-policy / fallback path instead of invalid-response retries. + + Does NOT check output_text fallback — the caller handles that with + diagnostic logging for stream backfill recovery. """ if response is None: return False output = getattr(response, "output", None) if not isinstance(output, list) or not output: - return False + status = str(getattr(response, "status", "") or "").strip().lower() + incomplete_details = getattr(response, "incomplete_details", None) + if isinstance(incomplete_details, dict): + reason = str(incomplete_details.get("reason") or "").strip().lower() + else: + reason = str(getattr(incomplete_details, "reason", "") or "").strip().lower() + return status == "incomplete" and reason == "content_filter" return True def preflight_kwargs( diff --git a/tests/agent/test_codex_responses_adapter.py b/tests/agent/test_codex_responses_adapter.py index cbf2f0e58..eb690a9e3 100644 --- a/tests/agent/test_codex_responses_adapter.py +++ b/tests/agent/test_codex_responses_adapter.py @@ -79,6 +79,21 @@ def test_normalize_codex_response_treats_summary_only_reasoning_as_incomplete(): assert assistant_message.codex_reasoning_items is None +def test_normalize_codex_response_maps_incomplete_content_filter_to_refusal(): + response = SimpleNamespace( + status="incomplete", + incomplete_details=SimpleNamespace(reason="content_filter"), + output=[], + output_text="", + ) + + assistant_message, finish_reason = _normalize_codex_response(response) + + assert finish_reason == "content_filter" + assert assistant_message.content == "" + assert response.output + + # --------------------------------------------------------------------------- # Server-side built-in tool calls (xAI native web_search, code interpreter, # etc.) come back as discrete ``*_call`` output items that xAI's diff --git a/tests/agent/transports/test_codex_transport.py b/tests/agent/transports/test_codex_transport.py index 2b15fdf60..761ca8c9c 100644 --- a/tests/agent/transports/test_codex_transport.py +++ b/tests/agent/transports/test_codex_transport.py @@ -673,6 +673,34 @@ class TestCodexValidateResponse: r = SimpleNamespace(output=None, output_text="Some text") assert transport.validate_response(r) is False + def test_empty_output_content_filter_incomplete_is_valid(self, transport): + r = SimpleNamespace( + status="incomplete", + incomplete_details=SimpleNamespace(reason="content_filter"), + output=[], + output_text="", + ) + assert transport.validate_response(r) is True + + def test_empty_output_content_filter_dict_incomplete_is_valid(self, transport): + r = SimpleNamespace( + status=" incomplete ", + incomplete_details={"reason": " content_filter "}, + output=[], + output_text="", + ) + assert transport.validate_response(r) is True + + @pytest.mark.parametrize("reason", ["max_output_tokens", "length", "", None]) + def test_empty_output_other_incomplete_reasons_remain_invalid(self, transport, reason): + r = SimpleNamespace( + status="incomplete", + incomplete_details=SimpleNamespace(reason=reason), + output=[], + output_text="", + ) + assert transport.validate_response(r) is False + class TestCodexMapFinishReason: diff --git a/tests/run_agent/test_run_agent.py b/tests/run_agent/test_run_agent.py index 13187af94..2dc46c1d7 100644 --- a/tests/run_agent/test_run_agent.py +++ b/tests/run_agent/test_run_agent.py @@ -4149,6 +4149,66 @@ class TestRunConversation: assert result["final_response"] == "Final answer" assert result["completed"] is True + def test_codex_content_filter_incomplete_routes_to_policy_fallback(self, agent): + self._setup_agent(agent) + agent.api_mode = "codex_responses" + agent.provider = "openai-codex" + agent.base_url = "https://chatgpt.com/backend-api/codex" + agent._base_url_lower = agent.base_url.lower() + agent._base_url_hostname = "chatgpt.com" + agent.model = "gpt-5.5" + agent._fallback_chain = [ + {"provider": "openrouter", "model": "anthropic/claude-sonnet-4.7"}, + ] + agent._fallback_index = 0 + + content_filter_response = SimpleNamespace( + status="incomplete", + incomplete_details=SimpleNamespace(reason="content_filter"), + output=[], + output_text="", + model="gpt-5.5", + usage=None, + ) + fallback_response = SimpleNamespace( + status="completed", + incomplete_details=None, + output=[ + SimpleNamespace( + type="message", + status="completed", + content=[SimpleNamespace(type="output_text", text="Recovered on fallback")], + ) + ], + model="fallback/model", + usage=None, + ) + hook_events = [] + + def _fake_activate(reason=None): + agent._fallback_index = len(agent._fallback_chain) + return True + + with ( + patch.object(agent, "_create_request_openai_client", return_value=MagicMock()), + patch.object(agent, "_close_request_openai_client"), + patch.object(agent, "_run_codex_stream", side_effect=[content_filter_response, fallback_response]) as mock_run_codex_stream, + patch.object(agent, "_try_activate_fallback", side_effect=_fake_activate) as mock_try_activate_fallback, + patch.object(agent, "_invoke_api_request_error_hook", side_effect=lambda **kw: hook_events.append(kw)), + patch.object(agent, "_persist_session"), + patch.object(agent, "_save_trajectory"), + patch.object(agent, "_cleanup_task_resources"), + ): + result = agent.run_conversation("summarize this large Slack thread") + + assert result["final_response"] == "Recovered on fallback" + assert result["completed"] is True + mock_try_activate_fallback.assert_called_once_with() + assert mock_run_codex_stream.call_count == 2 + assert hook_events[0]["error_type"] == "ContentPolicyBlocked" + assert hook_events[0]["retryable"] is False + assert hook_events[0]["reason"] == FailoverReason.content_policy_blocked.value + def test_ollama_small_runtime_context_fails_before_api_call(self, agent, caplog): self._setup_agent(agent) agent.model = "qwen3.5:9b" From 08015c3a8feec5f43fb10271f0122e1bae5a46df Mon Sep 17 00:00:00 2001 From: LeonSGP43 Date: Wed, 24 Jun 2026 11:47:26 +0800 Subject: [PATCH 27/47] fix(gateway): suppress hidden-only incomplete codex turns --- gateway/run.py | 33 +++- .../gateway/test_incomplete_gateway_turns.py | 159 ++++++++++++++++++ website/docs/user-guide/messaging/slack.md | 6 + 3 files changed, 196 insertions(+), 2 deletions(-) create mode 100644 tests/gateway/test_incomplete_gateway_turns.py diff --git a/gateway/run.py b/gateway/run.py index f6e86b29f..15042bd4c 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -2667,6 +2667,8 @@ def _normalize_empty_agent_response( ) return response if api_calls > 0: + if _is_gateway_hidden_reasoning_incomplete_turn(agent_result): + return "" if agent_result.get("partial"): err = agent_result.get("error", "processing incomplete") return f"⚠️ Processing stopped: {str(err)[:200]}. Try again." @@ -2694,6 +2696,20 @@ def _normalize_empty_agent_response( return response +def _is_gateway_hidden_reasoning_incomplete_turn(agent_result: dict) -> bool: + """Detect retry-exhausted turns with hidden reasoning but no visible answer.""" + if not isinstance(agent_result, dict): + return False + if agent_result.get("failed") or agent_result.get("interrupted"): + return False + if agent_result.get("final_response"): + return False + if not agent_result.get("partial"): + return False + error_text = str(agent_result.get("error", "") or "").lower() + return "remained incomplete after" in error_text + + def _should_clear_resume_pending_after_turn(agent_result: dict) -> bool: """Return True only when a gateway turn really completed successfully. @@ -12039,6 +12055,9 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew # forgets what was just asked. Persist the user turn so the # conversation is preserved. (#7100) agent_failed_early = bool(agent_result.get("failed")) + hidden_reasoning_incomplete = _is_gateway_hidden_reasoning_incomplete_turn( + agent_result + ) _err_str_for_classify = str(agent_result.get("error", "")).lower() # Use specific multi-word phrases (not bare "exceed" or "token") # to avoid false positives on transient errors like "rate limit @@ -12067,6 +12086,13 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew "message so conversation context is preserved on retry.", session_entry.session_id, ) + elif hidden_reasoning_incomplete: + logger.warning( + "Suppressing hidden-reasoning-only incomplete gateway turn " + "for session %s: %s", + session_entry.session_id, + agent_result.get("error", "processing incomplete"), + ) # When compression is exhausted, the session is permanently too # large to process. Auto-reset it so the next message starts @@ -12150,11 +12176,14 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew # entries that were stripped before the agent saw them. if is_context_overflow_failure: pass # handled above — skip all transcript writes - elif agent_failed_early: + elif agent_failed_early or hidden_reasoning_incomplete: # Transient failure (429/timeout/5xx): persist only the user # message so the next message can load a transcript that # reflects what was said. Skip the assistant error text since - # it's a gateway-generated hint, not model output. (#7100) + # it's a gateway-generated hint, not model output. Hidden- + # reasoning-only incomplete turns follow the same persistence + # rule so peer-agent channels don't ingest them as completed + # assistant turns. (#7100, #51628) _user_entry = { "role": "user", "content": ( diff --git a/tests/gateway/test_incomplete_gateway_turns.py b/tests/gateway/test_incomplete_gateway_turns.py new file mode 100644 index 000000000..1ac356711 --- /dev/null +++ b/tests/gateway/test_incomplete_gateway_turns.py @@ -0,0 +1,159 @@ +"""Regression tests for hidden-reasoning-only incomplete gateway turns.""" + +import asyncio +from datetime import datetime +from types import SimpleNamespace +from unittest.mock import AsyncMock, MagicMock + +import pytest + +import gateway.run as gateway_run +from gateway.config import GatewayConfig, Platform, PlatformConfig +from gateway.platforms.base import BasePlatformAdapter, MessageEvent, ProcessingOutcome, SendResult +from gateway.session import SessionEntry, SessionSource, build_session_key + + +class CaptureSlackAdapter(BasePlatformAdapter): + def __init__(self): + super().__init__(PlatformConfig(enabled=True, token="fake-token"), Platform.SLACK) + self.sent = [] + self.processing_hooks = [] + + async def connect(self) -> bool: + return True + + async def disconnect(self) -> None: + return None + + async def send(self, chat_id, content, reply_to=None, metadata=None) -> SendResult: + self.sent.append( + { + "chat_id": chat_id, + "content": content, + "reply_to": reply_to, + "metadata": metadata, + } + ) + return SendResult(success=True, message_id="slack-1") + + async def send_typing(self, chat_id: str, metadata=None) -> None: + return None + + async def get_chat_info(self, chat_id: str): + return {"id": chat_id} + + async def on_processing_start(self, event: MessageEvent) -> None: + self.processing_hooks.append(("start", event.message_id)) + + async def on_processing_complete(self, event: MessageEvent, outcome: ProcessingOutcome) -> None: + self.processing_hooks.append(("complete", event.message_id, outcome)) + + +def _make_incomplete_result() -> dict: + return { + "final_response": None, + "messages": [ + {"role": "user", "content": "hello"}, + {"role": "assistant", "content": ""}, + ], + "tools": [], + "history_offset": 0, + "api_calls": 3, + "partial": True, + "completed": False, + "interrupted": False, + "error": "Codex response remained incomplete after 3 continuation attempts", + "last_prompt_tokens": 0, + } + + +def _make_runner(adapter: CaptureSlackAdapter) -> gateway_run.GatewayRunner: + runner = object.__new__(gateway_run.GatewayRunner) + runner.config = GatewayConfig( + platforms={Platform.SLACK: PlatformConfig(enabled=True, token="fake-token")} + ) + runner.adapters = {Platform.SLACK: adapter} + runner._voice_mode = {} + runner.hooks = SimpleNamespace(emit=AsyncMock(), loaded_hooks=False) + runner.session_store = MagicMock() + runner.session_store.get_or_create_session.return_value = SessionEntry( + session_key="agent:main:slack:channel:C123:171717", + session_id="sess-1", + created_at=datetime.now(), + updated_at=datetime.now(), + platform=Platform.SLACK, + chat_type="channel", + ) + runner.session_store.load_transcript.return_value = [] + runner.session_store.has_any_sessions.return_value = True + runner.session_store.rewrite_transcript = MagicMock() + runner.session_store.append_to_transcript = MagicMock() + runner.session_store.update_session = MagicMock() + runner._running_agents = {} + runner._pending_messages = {} + runner._pending_approvals = {} + runner._session_db = None + runner._is_user_authorized = lambda _source: True + runner._set_session_env = lambda _context: None + runner._run_agent = AsyncMock(return_value=_make_incomplete_result()) + return runner + + +def _make_event() -> MessageEvent: + return MessageEvent( + text="hello", + source=SessionSource( + platform=Platform.SLACK, + chat_id="C123", + chat_type="channel", + thread_id="171717", + user_id="U123", + ), + message_id="m-1", + ) + + +def test_incomplete_codex_warning_is_not_surfaced_as_chat_text(): + agent_result = _make_incomplete_result() + + response = gateway_run._normalize_empty_agent_response( + agent_result, + agent_result.get("final_response") or "", + history_len=4, + ) + + assert response == "" + + +@pytest.mark.asyncio +async def test_incomplete_codex_turn_stays_out_of_slack_transcript(monkeypatch, tmp_path): + adapter = CaptureSlackAdapter() + runner = _make_runner(adapter) + + monkeypatch.setattr(gateway_run, "_hermes_home", tmp_path) + monkeypatch.setattr(gateway_run, "_resolve_runtime_agent_kwargs", lambda: {"api_key": "fake"}) + monkeypatch.setattr( + "agent.model_metadata.get_model_context_length", + lambda *_args, **_kwargs: 100, + ) + monkeypatch.setenv("SLACK_HOME_CHANNEL", "C123") + + adapter.set_message_handler(runner._handle_message) + adapter._keep_typing = lambda *_args, **_kwargs: asyncio.Event().wait() + + event = _make_event() + await adapter._process_message_background(event, build_session_key(event.source)) + + assert adapter.sent == [] + assert runner.session_store.update_session.called + + transcript_roles = [ + call.args[1]["role"] + for call in runner.session_store.append_to_transcript.call_args_list + ] + assert transcript_roles == ["session_meta", "user"] + assert runner.session_store.append_to_transcript.call_args_list[1].args[1]["content"] == "hello" + assert adapter.processing_hooks == [ + ("start", "m-1"), + ("complete", "m-1", ProcessingOutcome.SUCCESS), + ] diff --git a/website/docs/user-guide/messaging/slack.md b/website/docs/user-guide/messaging/slack.md index 42acf9f3e..299b5fa0a 100644 --- a/website/docs/user-guide/messaging/slack.md +++ b/website/docs/user-guide/messaging/slack.md @@ -217,6 +217,12 @@ hermes gateway install # Install as a user service sudo hermes gateway install --system # Linux only: boot-time system service ``` +:::tip Codex reasoning-effort safety +For Codex-backed Slack peer-agent channels, prefer `agent.reasoning_effort: high` or lower. `xhigh` +can spend the entire turn in hidden reasoning and never produce visible assistant text; Hermes now +suppresses those incomplete-turn warnings from the thread and keeps the diagnostics in gateway logs. +::: + --- ## Step 9: Invite the Bot to Channels From 09b6d22dfec471d9d7aa814aa0da3758dad9ebec Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 15 Jul 2026 07:59:12 -0700 Subject: [PATCH 28/47] fix(gateway): harden hidden-incomplete detection against the sentinel final_response MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the salvaged #51657: the conversation loop returns the retry-exhaustion sentinel as BOTH final_response and error, so the original detector (which required final_response to be falsy) never fired on real exhaustion turns — the sentinel text was delivered verbatim into the channel, exactly the #51628 poisoning vector. Detect the sentinel echo, blank it before empty-response normalization, and never suppress a turn whose final_response is genuine model text. Also: dedupe-guard mock fix in the test fixture (has_platform_message_id must return False, not a truthy MagicMock) and two guard tests (real answer never suppressed; interrupted/failed never classified). --- gateway/run.py | 28 +++++++++++--- .../gateway/test_incomplete_gateway_turns.py | 37 +++++++++++++++++-- 2 files changed, 57 insertions(+), 8 deletions(-) diff --git a/gateway/run.py b/gateway/run.py index 15042bd4c..51dcd5e2f 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -2697,17 +2697,27 @@ def _normalize_empty_agent_response( def _is_gateway_hidden_reasoning_incomplete_turn(agent_result: dict) -> bool: - """Detect retry-exhausted turns with hidden reasoning but no visible answer.""" + """Detect retry-exhausted turns with hidden reasoning but no visible answer. + + The conversation loop returns the retry-exhaustion sentinel as BOTH + ``final_response`` and ``error`` ("Codex response remained incomplete + after 3 continuation attempts"), so ``final_response`` being non-empty + does not mean the model produced a visible answer. Treat the turn as + hidden when the error sentinel is present and ``final_response`` is + either empty or merely echoes that sentinel — any genuinely different + final text means the model DID answer and must be delivered. + """ if not isinstance(agent_result, dict): return False if agent_result.get("failed") or agent_result.get("interrupted"): return False - if agent_result.get("final_response"): - return False if not agent_result.get("partial"): return False - error_text = str(agent_result.get("error", "") or "").lower() - return "remained incomplete after" in error_text + error_text = str(agent_result.get("error", "") or "").strip() + if "remained incomplete after" not in error_text.lower(): + return False + final_response = str(agent_result.get("final_response") or "").strip() + return not final_response or final_response == error_text def _should_clear_resume_pending_after_turn(agent_result: dict) -> bool: @@ -11825,6 +11835,14 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew return None response = agent_result.get("final_response") or "" + # Hidden-reasoning-only retry exhaustion: the loop's sentinel text + # ("Codex response remained incomplete after 3 continuation + # attempts") doubles as final_response, so it would be delivered + # verbatim into the channel — where peer agents can ingest it as a + # completed assistant turn (#51628). Blank it here so the normal + # empty-response handling (and the suppression below) applies. + if _is_gateway_hidden_reasoning_incomplete_turn(agent_result): + response = "" try: from gateway.response_filters import is_intentional_silence_agent_result _intentional_silence = is_intentional_silence_agent_result( diff --git a/tests/gateway/test_incomplete_gateway_turns.py b/tests/gateway/test_incomplete_gateway_turns.py index 1ac356711..1d777548f 100644 --- a/tests/gateway/test_incomplete_gateway_turns.py +++ b/tests/gateway/test_incomplete_gateway_turns.py @@ -50,8 +50,12 @@ class CaptureSlackAdapter(BasePlatformAdapter): def _make_incomplete_result() -> dict: + # Mirror the REAL conversation-loop exhaustion shape: the sentinel text is + # returned as BOTH final_response and error (agent/conversation_loop.py's + # "remained incomplete after 3 continuation attempts" return). + _sentinel = "Codex response remained incomplete after 3 continuation attempts" return { - "final_response": None, + "final_response": _sentinel, "messages": [ {"role": "user", "content": "hello"}, {"role": "assistant", "content": ""}, @@ -62,7 +66,7 @@ def _make_incomplete_result() -> dict: "partial": True, "completed": False, "interrupted": False, - "error": "Codex response remained incomplete after 3 continuation attempts", + "error": _sentinel, "last_prompt_tokens": 0, } @@ -89,6 +93,10 @@ def _make_runner(adapter: CaptureSlackAdapter) -> gateway_run.GatewayRunner: runner.session_store.rewrite_transcript = MagicMock() runner.session_store.append_to_transcript = MagicMock() runner.session_store.update_session = MagicMock() + # The transient-failure persistence path dedupes on platform message_id + # (#47237). A bare MagicMock returns a truthy mock, which would wrongly + # mark the user turn as a duplicate and skip persisting it. + runner.session_store.has_platform_message_id = MagicMock(return_value=False) runner._running_agents = {} runner._pending_messages = {} runner._pending_approvals = {} @@ -116,15 +124,38 @@ def _make_event() -> MessageEvent: def test_incomplete_codex_warning_is_not_surfaced_as_chat_text(): agent_result = _make_incomplete_result() + # Mirror the gateway pipeline: the hidden-turn detector blanks the + # sentinel final_response BEFORE empty-response normalization runs. + response = agent_result.get("final_response") or "" + assert gateway_run._is_gateway_hidden_reasoning_incomplete_turn(agent_result) + response = "" + response = gateway_run._normalize_empty_agent_response( agent_result, - agent_result.get("final_response") or "", + response, history_len=4, ) assert response == "" +def test_real_answer_alongside_incomplete_error_is_never_suppressed(): + """A turn whose final_response is genuine model text (not the sentinel + echo) must be delivered even when the error field carries the + retry-exhaustion sentinel — suppression is only for hidden turns.""" + agent_result = _make_incomplete_result() + agent_result["final_response"] = "Here is the actual answer." + + assert not gateway_run._is_gateway_hidden_reasoning_incomplete_turn(agent_result) + + +def test_interrupted_or_failed_turns_are_not_classified_hidden(): + for key in ("interrupted", "failed"): + agent_result = _make_incomplete_result() + agent_result[key] = True + assert not gateway_run._is_gateway_hidden_reasoning_incomplete_turn(agent_result) + + @pytest.mark.asyncio async def test_incomplete_codex_turn_stays_out_of_slack_transcript(monkeypatch, tmp_path): adapter = CaptureSlackAdapter() From 6a8e7069b4e1cb59106805e9060daffb7c5e8653 Mon Sep 17 00:00:00 2001 From: liuhao1024 Date: Fri, 26 Jun 2026 06:10:56 +0800 Subject: [PATCH 29/47] fix(agent): dedup codex incomplete interims on visible content MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two consecutive incomplete assistant interims with identical visible content (content + reasoning) are collapsed even when opaque provider state (encrypted reasoning item ids, message item phases) drifts per continuation — previously that drift defeated dedup and caused message storms (#52711). The latest opaque payload is written onto the existing message in place, so continuation replay still uses fresh provider state. Salvage note: the original PR also made the 3-retry continuation cap cumulative per turn (no reset on progress). That half is intentionally dropped — a legitimate long turn alternating incomplete/progress would hard-fail at 3 cumulative, changing semantics beyond the reported bug. --- agent/conversation_loop.py | 31 ++--- .../test_run_agent_codex_responses.py | 117 +++++++++++++++--- 2 files changed, 117 insertions(+), 31 deletions(-) diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index 8986a2ecd..ca97280e1 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -4465,27 +4465,28 @@ def run_conversation( or interim_has_codex_message_items ): last_msg = messages[-1] if messages else None - # Duplicate detection: two consecutive incomplete assistant - # messages with identical content AND reasoning are collapsed. - # For provider-state-only changes (encrypted reasoning - # items or replayable message ids/phases/statuses differ - # while visible content/reasoning are unchanged), compare - # those opaque payloads too so we don't silently drop the - # newer continuation state. - last_codex_items = last_msg.get("codex_reasoning_items") if isinstance(last_msg, dict) else None - interim_codex_items = interim_msg.get("codex_reasoning_items") - last_codex_message_items = last_msg.get("codex_message_items") if isinstance(last_msg, dict) else None - interim_codex_message_items = interim_msg.get("codex_message_items") - duplicate_interim = ( + # Duplicate detection: compare only visible content + # (content + reasoning). Opaque provider state + # (encrypted reasoning items, message item ids/phases) + # drifts per continuation even when the visible output + # is identical, so including it in the comparison defeats + # dedup and causes message storms (#52711). + visible_duplicate = ( isinstance(last_msg, dict) and last_msg.get("role") == "assistant" and last_msg.get("finish_reason") == "incomplete" and (last_msg.get("content") or "") == (interim_msg.get("content") or "") and (last_msg.get("reasoning") or "") == (interim_msg.get("reasoning") or "") - and last_codex_items == interim_codex_items - and last_codex_message_items == interim_codex_message_items ) - if not duplicate_interim: + if visible_duplicate: + # Update opaque state in-place so the latest + # provider payload is preserved without emitting + # a duplicate visible message. + for _key in ("codex_reasoning_items", "codex_message_items"): + _new_val = interim_msg.get(_key) + if _new_val is not None: + last_msg[_key] = _new_val + else: messages.append(interim_msg) agent._emit_interim_assistant_message(interim_msg) diff --git a/tests/run_agent/test_run_agent_codex_responses.py b/tests/run_agent/test_run_agent_codex_responses.py index 93823144a..f307b0b44 100644 --- a/tests/run_agent/test_run_agent_codex_responses.py +++ b/tests/run_agent/test_run_agent_codex_responses.py @@ -1704,6 +1704,90 @@ def test_mid_turn_compaction_does_not_double_persist_in_place_rows(monkeypatch, ) +def _codex_incomplete_with_reasoning(text: str, reasoning_id: str = "rs_default"): + """Incomplete response with a reasoning item whose id/encrypted_content + can vary independently of the visible message text.""" + return SimpleNamespace( + output=[ + SimpleNamespace( + type="reasoning", + id=reasoning_id, + encrypted_content=f"opaque_{reasoning_id}", + summary=[SimpleNamespace(text="thinking...")], + ), + SimpleNamespace( + type="message", + status="in_progress", + content=[SimpleNamespace(type="output_text", text=text)], + ), + ], + usage=SimpleNamespace(input_tokens=4, output_tokens=2, total_tokens=6), + status="in_progress", + model="gpt-5-codex", + ) + + +def test_codex_incomplete_visible_dedup_suppresses_duplicate_interims(monkeypatch): + """Two consecutive incomplete responses with identical visible content + but different opaque reasoning items should be collapsed — only the first + interim is emitted to the user (#52711).""" + agent = _build_agent(monkeypatch) + # 2 incompletes with same text but different reasoning ids, then a final. + # (Only 2 to avoid hitting the cap of 3.) + responses = [ + _codex_incomplete_with_reasoning("Working on it...", "rs_1"), + _codex_incomplete_with_reasoning("Working on it...", "rs_2"), + _codex_message_response("Done."), + ] + monkeypatch.setattr(agent, "_interruptible_api_call", lambda api_kwargs: responses.pop(0)) + + emitted: list = [] + original_emit = agent._emit_interim_assistant_message + def _capture_emit(msg): + emitted.append(msg.get("content")) + original_emit(msg) + monkeypatch.setattr(agent, "_emit_interim_assistant_message", _capture_emit) + + result = agent.run_conversation("test dedup") + + assert result["completed"] is True + # Only ONE interim should have been emitted (the first), not two. + assert len(emitted) == 1 + assert emitted[0] == "Working on it..." + + +def test_codex_incomplete_opaque_state_updated_in_place(monkeypatch): + """When visible content is a duplicate, the last message's opaque state + (codex_reasoning_items) should be updated in-place without emitting a new + interim (#52711).""" + agent = _build_agent(monkeypatch) + responses = [ + _codex_incomplete_with_reasoning("Partial output...", "rs_1"), + _codex_incomplete_with_reasoning("Partial output...", "rs_2"), + _codex_message_response("Final."), + ] + monkeypatch.setattr(agent, "_interruptible_api_call", lambda api_kwargs: responses.pop(0)) + + result = agent.run_conversation("test opaque update") + + assert result["completed"] is True + # Find the incomplete interim message in the result. + incompletes = [ + m for m in result["messages"] + if m.get("role") == "assistant" and m.get("finish_reason") == "incomplete" + ] + # Only one incomplete message should exist (the second was deduped). + assert len(incompletes) == 1 + # The opaque state should reflect the LATEST reasoning item (rs_2), + # updated in-place on the single message. + items = incompletes[0].get("codex_reasoning_items") + if items: + assert any( + (i.get("id") if isinstance(i, dict) else getattr(i, "id", None)) == "rs_2" + for i in items + ) + + def test_normalize_codex_response_marks_commentary_only_message_as_incomplete(monkeypatch): agent = _build_agent(monkeypatch) from agent.codex_responses_adapter import _normalize_codex_response @@ -2608,7 +2692,8 @@ def test_codex_message_item_status_survives_conversion_and_preflight(monkeypatch def test_duplicate_detection_distinguishes_different_codex_reasoning(monkeypatch): """Two consecutive reasoning-only responses with different encrypted content - must NOT be treated as duplicates.""" + are deduped on visible content — only one interim is kept, but opaque state + is updated in-place (#52711).""" agent = _build_agent(monkeypatch) responses = [ # First reasoning-only response @@ -2641,23 +2726,23 @@ def test_duplicate_detection_distinguishes_different_codex_reasoning(monkeypatch assert result["completed"] is True assert result["final_response"] == "Final answer after thinking." - # Both reasoning-only interim messages should be in history (not collapsed) + # Only one reasoning-only interim should be in history (deduped on + # visible content — both have empty visible output). interim_msgs = [ msg for msg in result["messages"] if msg.get("role") == "assistant" and msg.get("finish_reason") == "incomplete" ] - assert len(interim_msgs) == 2 - encrypted_contents = [ - msg["codex_reasoning_items"][0]["encrypted_content"] - for msg in interim_msgs - ] - assert "enc_first" in encrypted_contents - assert "enc_second" in encrypted_contents + assert len(interim_msgs) == 1 + # But the opaque state should reflect the LATEST reasoning item. + items = interim_msgs[0].get("codex_reasoning_items") + if items: + assert items[0].get("encrypted_content") == "enc_second" def test_duplicate_detection_distinguishes_different_codex_message_items(monkeypatch): - """Incomplete turns with new message ids/phases/statuses must not be collapsed.""" + """Incomplete turns with same visible content but different message ids + are deduped — only one interim kept, opaque state updated in-place (#52711).""" agent = _build_agent(monkeypatch) responses = [ SimpleNamespace( @@ -2700,12 +2785,12 @@ def test_duplicate_detection_distinguishes_different_codex_message_items(monkeyp if msg.get("role") == "assistant" and msg.get("finish_reason") == "incomplete" ] - assert len(interim_msgs) == 2 - assert [msg["codex_message_items"][0]["id"] for msg in interim_msgs] == [ - "msg_first", - "msg_second", - ] - assert all(msg["codex_message_items"][0]["status"] == "in_progress" for msg in interim_msgs) + # Only one interim — deduped on visible content ("Still working..." == "Still working..."). + assert len(interim_msgs) == 1 + # Opaque state should reflect the latest message item. + items = interim_msgs[0].get("codex_message_items") + if items: + assert items[0].get("id") == "msg_second" def test_chat_messages_to_responses_input_deduplicates_reasoning_ids(monkeypatch): From 6efec39ecf8bd640173d31567884cf22aafe3424 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 15 Jul 2026 09:49:42 -0700 Subject: [PATCH 30/47] refactor(skills): rework blender-mcp skill around the catalog MCP entry (#65066) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The optional blender-mcp skill predates the blender MCP catalog entry (#64463) and taught the agent to hand-roll raw TCP JSON to the addon's socket on port 9876 from execute_code — bypassing the catalog's version pinning and install-time tool curation. Reworked to v2.0.0 as the companion skill for the catalog entry: - prerequisites now go through 'hermes mcp install blender' - interaction surface is the four curated MCP tools, not a raw socket - keeps the valuable content: addon setup, bpy recipes (materials, keyframes, render-to-file), pitfalls (timeouts, absolute paths, object mode), plus new pitfalls (xvfb headless, no-sandbox warning, remote-host path resolution) - explicit anti-pattern note: do not hand-roll TCP to 9876 - description shortened to <=60 chars per skill authoring standards alireza78a's original bpy patterns and pitfalls are preserved and credited. Docs page regenerated via generate-skill-docs.py (scoped to this skill only; unrelated generator drift left untouched). --- optional-skills/creative/blender-mcp/SKILL.md | 150 +++++++++-------- .../docs/reference/optional-skills-catalog.md | 2 +- .../optional/creative/creative-blender-mcp.md | 152 ++++++++++-------- 3 files changed, 168 insertions(+), 136 deletions(-) diff --git a/optional-skills/creative/blender-mcp/SKILL.md b/optional-skills/creative/blender-mcp/SKILL.md index ed08c8d96..95e60c897 100644 --- a/optional-skills/creative/blender-mcp/SKILL.md +++ b/optional-skills/creative/blender-mcp/SKILL.md @@ -1,94 +1,88 @@ --- name: blender-mcp -description: Control Blender directly from Hermes via socket connection to the blender-mcp addon. Create 3D objects, materials, animations, and run arbitrary Blender Python (bpy) code. Use when user wants to create or modify anything in Blender. -version: 1.0.0 -requires: Blender 4.3+ (desktop instance required, headless not supported) -author: alireza78a +description: Drive Blender via the catalog blender MCP, with bpy recipes. +version: 2.0.0 +requires: Blender 3.0+ desktop instance (headless via xvfb-run) +author: alireza78a + Hermes Agent tags: [blender, 3d, animation, modeling, bpy, mcp] platforms: [linux, macos, windows] --- -# Blender MCP +# Blender MCP Skill -Control a running Blender instance from Hermes via socket on TCP port 9876. +Companion skill for the `blender` entry in the Hermes MCP catalog. The MCP +server provides the connection to Blender; this skill teaches the bpy idioms +and pitfalls for driving it well. It does not cover Blender UI workflows — +everything here goes through the MCP tools against a live Blender session. -## Setup (one-time) +## When to Use -### 1. Install the Blender addon +Use when the user wants to create or modify anything in a running Blender +instance: meshes, materials, animations, lighting, renders. Requires the +blender MCP server installed and a Blender desktop session with the addon +connected. - curl -sL https://raw.githubusercontent.com/ahujasid/blender-mcp/main/addon.py -o ~/Desktop/blender_mcp_addon.py +## Prerequisites -In Blender: - Edit > Preferences > Add-ons > Install > select blender_mcp_addon.py - Enable "Interface: Blender MCP" +1. Install the MCP server from the Nous catalog (one-time): -### 2. Start the socket server in Blender + hermes mcp install blender -Press N in Blender viewport to open sidebar. -Find "BlenderMCP" tab and click "Start Server". + This configures the pinned `blender-mcp` stdio server with the curated + tool set: `get_scene_info`, `get_object_info`, `get_viewport_screenshot`, + `execute_blender_code`. -### 3. Verify connection +2. Install the addon inside Blender (one-time — the catalog entry's + post-install notes cover this too): + - Download https://raw.githubusercontent.com/ahujasid/blender-mcp/main/addon.py + - Blender > Edit > Preferences > Add-ons > Install... > select addon.py, + enable "Interface: Blender MCP". - nc -z -w2 localhost 9876 && echo "OPEN" || echo "CLOSED" +3. Every session: start Blender FIRST, press N in the viewport, open the + "BlenderMCP" tab, click "Connect to Claude" (starts the local bridge + socket). Then start your Hermes session so the MCP tools are loaded. -## Protocol + The addon refuses to start under `blender -b` (background mode). On a + machine without a display, run Blender under a virtual one: + `xvfb-run blender`. GPU rendering works fine under Xvfb. -Plain UTF-8 JSON over TCP -- no length prefix. +## Quick Reference -Send: {"type": "", "params": {}} -Receive: {"status": "success", "result": } - {"status": "error", "message": ""} +| MCP tool | Use for | +|---------------------------|--------------------------------------------| +| `get_scene_info` | List objects before touching the scene | +| `get_object_info` | Inspect one object (transform, materials) | +| `get_viewport_screenshot` | Visual check of what you built | +| `execute_blender_code` | Everything else — arbitrary bpy Python | -## Available Commands +Optional asset-service tools (PolyHaven, Sketchfab, Hyper3D, Hunyuan3D) are +disabled by default. If the user has enabled a service in the addon panel, +opt into its tools with `hermes mcp configure blender`. -| type | params | description | -|-------------------------|-------------------|---------------------------------| -| execute_code | code (str) | Run arbitrary bpy Python code | -| get_scene_info | (none) | List all objects in scene | -| get_object_info | object_name (str) | Details on a specific object | -| get_viewport_screenshot | (none) | Screenshot of current viewport | +## Procedure -## Python Helper +1. Call `get_scene_info` first — never assume the scene is empty. +2. Build with `execute_blender_code`, in small focused calls (one logical + step per call: add objects, then materials, then animation). Large + monolithic scripts hit the bridge timeout. +3. Verify visually with `get_viewport_screenshot` between major steps. +4. Render to an absolute path and tell the user where the file is. -Use this inside execute_code tool calls: +### Common bpy Patterns - import socket, json +Clear scene: - def blender_exec(code: str, host="localhost", port=9876, timeout=15): - s = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - s.connect((host, port)) - s.settimeout(timeout) - payload = json.dumps({"type": "execute_code", "params": {"code": code}}) - s.sendall(payload.encode("utf-8")) - buf = b"" - while True: - try: - chunk = s.recv(4096) - if not chunk: - break - buf += chunk - try: - json.loads(buf.decode("utf-8")) - break - except json.JSONDecodeError: - continue - except socket.timeout: - break - s.close() - return json.loads(buf.decode("utf-8")) - -## Common bpy Patterns - -### Clear scene bpy.ops.object.select_all(action='SELECT') bpy.ops.object.delete() -### Add mesh objects +Add mesh objects: + bpy.ops.mesh.primitive_uv_sphere_add(radius=1, location=(0, 0, 0)) bpy.ops.mesh.primitive_cube_add(size=2, location=(3, 0, 0)) bpy.ops.mesh.primitive_cylinder_add(radius=0.5, depth=2, location=(-3, 0, 0)) -### Create and assign material +Create and assign material: + mat = bpy.data.materials.new(name="MyMat") mat.use_nodes = True bsdf = mat.node_tree.nodes.get("Principled BSDF") @@ -97,21 +91,43 @@ Use this inside execute_code tool calls: bsdf.inputs["Metallic"].default_value = 0.0 obj.data.materials.append(mat) -### Keyframe animation +Keyframe animation: + obj.location = (0, 0, 0) obj.keyframe_insert(data_path="location", frame=1) obj.location = (0, 0, 3) obj.keyframe_insert(data_path="location", frame=60) -### Render to file +Render to file: + bpy.context.scene.render.filepath = "/tmp/render.png" bpy.context.scene.render.engine = 'CYCLES' bpy.ops.render.render(write_still=True) ## Pitfalls -- Must check socket is open before running (nc -z localhost 9876) -- Addon server must be started inside Blender each session (N-panel > BlenderMCP > Connect) -- Break complex scenes into multiple smaller execute_code calls to avoid timeouts -- Render output path must be absolute (/tmp/...) not relative -- shade_smooth() requires object to be selected and in object mode +- The blender MCP tools only exist if the server is installed and the session + started after install. If they're missing, run `hermes mcp install blender` + and start a new session. +- The addon bridge must be (re)connected inside Blender each Blender session + (N-panel > BlenderMCP > Connect). "Connection refused" from the tools means + Blender isn't running or the addon isn't connected — fix that, don't retry. +- Break complex scenes into multiple smaller `execute_blender_code` calls to + avoid bridge timeouts. +- Render output paths must be absolute (`/tmp/render.png`), not relative — + they resolve on the BLENDER host's filesystem, which matters if Hermes and + Blender run on different machines. +- `shade_smooth()` requires the object to be selected and in object mode. +- `execute_blender_code` runs arbitrary Python inside Blender with no sandbox + — same trust level as the `terminal` tool. Don't paste untrusted code into + it. +- Do NOT hand-roll raw TCP JSON to port 9876 from `execute_code` — that was + this skill's pre-MCP workaround. It bypasses the catalog's version pinning + and tool curation. The MCP tools are the supported path. + +## Verification + +- `get_scene_info` returns the expected object list after each build step. +- `get_viewport_screenshot` shows the scene you intended. +- After a render, confirm the output file exists and report its absolute + path to the user. diff --git a/website/docs/reference/optional-skills-catalog.md b/website/docs/reference/optional-skills-catalog.md index a85d3112a..fea4afb46 100644 --- a/website/docs/reference/optional-skills-catalog.md +++ b/website/docs/reference/optional-skills-catalog.md @@ -57,7 +57,7 @@ hermes skills uninstall |-------|-------------| | [**baoyu-article-illustrator**](/docs/user-guide/skills/optional/creative/creative-baoyu-article-illustrator) | Article illustrations: type × style × palette consistency. | | [**baoyu-comic**](/docs/user-guide/skills/optional/creative/creative-baoyu-comic) | Knowledge comics (知识漫画): educational, biography, tutorial. | -| [**blender-mcp**](/docs/user-guide/skills/optional/creative/creative-blender-mcp) | Control Blender directly from Hermes via socket connection to the blender-mcp addon. Create 3D objects, materials, animations, and run arbitrary Blender Python (bpy) code. Use when user wants to create or modify anything in Blender. | +| [**blender-mcp**](/docs/user-guide/skills/optional/creative/creative-blender-mcp) | Drive Blender via the catalog blender MCP, with bpy recipes. | | [**concept-diagrams**](/docs/user-guide/skills/optional/creative/creative-concept-diagrams) | Generate flat, minimal light/dark-aware SVG diagrams as standalone HTML files, using a unified educational visual language with 9 semantic color ramps, sentence-case typography, and automatic dark mode. Best suited for educational and no... | | [**creative-ideation**](/docs/user-guide/skills/optional/creative/creative-creative-ideation) | Generate ideas via named methods from creative practice. | | [**hyperframes**](/docs/user-guide/skills/optional/creative/creative-hyperframes) | Create HTML-based video compositions, animated title cards, social overlays, captioned talking-head videos, audio-reactive visuals, and shader transitions using HyperFrames. HTML is the source of truth for video. Use when the user wants... | diff --git a/website/docs/user-guide/skills/optional/creative/creative-blender-mcp.md b/website/docs/user-guide/skills/optional/creative/creative-blender-mcp.md index cffc98d8d..0f082f92c 100644 --- a/website/docs/user-guide/skills/optional/creative/creative-blender-mcp.md +++ b/website/docs/user-guide/skills/optional/creative/creative-blender-mcp.md @@ -1,14 +1,14 @@ --- -title: "Blender Mcp — Control Blender directly from Hermes via socket connection to the blender-mcp addon" +title: "Blender Mcp — Drive Blender via the catalog blender MCP, with bpy recipes" sidebar_label: "Blender Mcp" -description: "Control Blender directly from Hermes via socket connection to the blender-mcp addon" +description: "Drive Blender via the catalog blender MCP, with bpy recipes" --- {/* This page is auto-generated from the skill's SKILL.md by website/scripts/generate-skill-docs.py. Edit the source SKILL.md, not this page. */} # Blender Mcp -Control Blender directly from Hermes via socket connection to the blender-mcp addon. Create 3D objects, materials, animations, and run arbitrary Blender Python (bpy) code. Use when user wants to create or modify anything in Blender. +Drive Blender via the catalog blender MCP, with bpy recipes. ## Skill metadata @@ -16,8 +16,8 @@ Control Blender directly from Hermes via socket connection to the blender-mcp ad |---|---| | Source | Optional — install with `hermes skills install official/creative/blender-mcp` | | Path | `optional-skills/creative/blender-mcp` | -| Version | `1.0.0` | -| Author | alireza78a | +| Version | `2.0.0` | +| Author | alireza78a + Hermes Agent | | Platforms | linux, macos, windows | ## Reference: full SKILL.md @@ -26,87 +26,81 @@ Control Blender directly from Hermes via socket connection to the blender-mcp ad The following is the complete skill definition that Hermes loads when this skill is triggered. This is what the agent sees as instructions when the skill is active. ::: -# Blender MCP +# Blender MCP Skill -Control a running Blender instance from Hermes via socket on TCP port 9876. +Companion skill for the `blender` entry in the Hermes MCP catalog. The MCP +server provides the connection to Blender; this skill teaches the bpy idioms +and pitfalls for driving it well. It does not cover Blender UI workflows — +everything here goes through the MCP tools against a live Blender session. -## Setup (one-time) +## When to Use -### 1. Install the Blender addon +Use when the user wants to create or modify anything in a running Blender +instance: meshes, materials, animations, lighting, renders. Requires the +blender MCP server installed and a Blender desktop session with the addon +connected. - curl -sL https://raw.githubusercontent.com/ahujasid/blender-mcp/main/addon.py -o ~/Desktop/blender_mcp_addon.py +## Prerequisites -In Blender: - Edit > Preferences > Add-ons > Install > select blender_mcp_addon.py - Enable "Interface: Blender MCP" +1. Install the MCP server from the Nous catalog (one-time): -### 2. Start the socket server in Blender + hermes mcp install blender -Press N in Blender viewport to open sidebar. -Find "BlenderMCP" tab and click "Start Server". + This configures the pinned `blender-mcp` stdio server with the curated + tool set: `get_scene_info`, `get_object_info`, `get_viewport_screenshot`, + `execute_blender_code`. -### 3. Verify connection +2. Install the addon inside Blender (one-time — the catalog entry's + post-install notes cover this too): + - Download https://raw.githubusercontent.com/ahujasid/blender-mcp/main/addon.py + - Blender > Edit > Preferences > Add-ons > Install... > select addon.py, + enable "Interface: Blender MCP". - nc -z -w2 localhost 9876 && echo "OPEN" || echo "CLOSED" +3. Every session: start Blender FIRST, press N in the viewport, open the + "BlenderMCP" tab, click "Connect to Claude" (starts the local bridge + socket). Then start your Hermes session so the MCP tools are loaded. -## Protocol + The addon refuses to start under `blender -b` (background mode). On a + machine without a display, run Blender under a virtual one: + `xvfb-run blender`. GPU rendering works fine under Xvfb. -Plain UTF-8 JSON over TCP -- no length prefix. +## Quick Reference -Send: {"type": "<command>", "params": {<kwargs>}} -Receive: {"status": "success", "result": <value>} - {"status": "error", "message": "<reason>"} +| MCP tool | Use for | +|---------------------------|--------------------------------------------| +| `get_scene_info` | List objects before touching the scene | +| `get_object_info` | Inspect one object (transform, materials) | +| `get_viewport_screenshot` | Visual check of what you built | +| `execute_blender_code` | Everything else — arbitrary bpy Python | -## Available Commands +Optional asset-service tools (PolyHaven, Sketchfab, Hyper3D, Hunyuan3D) are +disabled by default. If the user has enabled a service in the addon panel, +opt into its tools with `hermes mcp configure blender`. -| type | params | description | -|-------------------------|-------------------|---------------------------------| -| execute_code | code (str) | Run arbitrary bpy Python code | -| get_scene_info | (none) | List all objects in scene | -| get_object_info | object_name (str) | Details on a specific object | -| get_viewport_screenshot | (none) | Screenshot of current viewport | +## Procedure -## Python Helper +1. Call `get_scene_info` first — never assume the scene is empty. +2. Build with `execute_blender_code`, in small focused calls (one logical + step per call: add objects, then materials, then animation). Large + monolithic scripts hit the bridge timeout. +3. Verify visually with `get_viewport_screenshot` between major steps. +4. Render to an absolute path and tell the user where the file is. -Use this inside execute_code tool calls: +### Common bpy Patterns - import socket, json +Clear scene: - def blender_exec(code: str, host="localhost", port=9876, timeout=15): - s = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - s.connect((host, port)) - s.settimeout(timeout) - payload = json.dumps({"type": "execute_code", "params": {"code": code}}) - s.sendall(payload.encode("utf-8")) - buf = b"" - while True: - try: - chunk = s.recv(4096) - if not chunk: - break - buf += chunk - try: - json.loads(buf.decode("utf-8")) - break - except json.JSONDecodeError: - continue - except socket.timeout: - break - s.close() - return json.loads(buf.decode("utf-8")) - -## Common bpy Patterns - -### Clear scene bpy.ops.object.select_all(action='SELECT') bpy.ops.object.delete() -### Add mesh objects +Add mesh objects: + bpy.ops.mesh.primitive_uv_sphere_add(radius=1, location=(0, 0, 0)) bpy.ops.mesh.primitive_cube_add(size=2, location=(3, 0, 0)) bpy.ops.mesh.primitive_cylinder_add(radius=0.5, depth=2, location=(-3, 0, 0)) -### Create and assign material +Create and assign material: + mat = bpy.data.materials.new(name="MyMat") mat.use_nodes = True bsdf = mat.node_tree.nodes.get("Principled BSDF") @@ -115,21 +109,43 @@ Use this inside execute_code tool calls: bsdf.inputs["Metallic"].default_value = 0.0 obj.data.materials.append(mat) -### Keyframe animation +Keyframe animation: + obj.location = (0, 0, 0) obj.keyframe_insert(data_path="location", frame=1) obj.location = (0, 0, 3) obj.keyframe_insert(data_path="location", frame=60) -### Render to file +Render to file: + bpy.context.scene.render.filepath = "/tmp/render.png" bpy.context.scene.render.engine = 'CYCLES' bpy.ops.render.render(write_still=True) ## Pitfalls -- Must check socket is open before running (nc -z localhost 9876) -- Addon server must be started inside Blender each session (N-panel > BlenderMCP > Connect) -- Break complex scenes into multiple smaller execute_code calls to avoid timeouts -- Render output path must be absolute (/tmp/...) not relative -- shade_smooth() requires object to be selected and in object mode +- The blender MCP tools only exist if the server is installed and the session + started after install. If they're missing, run `hermes mcp install blender` + and start a new session. +- The addon bridge must be (re)connected inside Blender each Blender session + (N-panel > BlenderMCP > Connect). "Connection refused" from the tools means + Blender isn't running or the addon isn't connected — fix that, don't retry. +- Break complex scenes into multiple smaller `execute_blender_code` calls to + avoid bridge timeouts. +- Render output paths must be absolute (`/tmp/render.png`), not relative — + they resolve on the BLENDER host's filesystem, which matters if Hermes and + Blender run on different machines. +- `shade_smooth()` requires the object to be selected and in object mode. +- `execute_blender_code` runs arbitrary Python inside Blender with no sandbox + — same trust level as the `terminal` tool. Don't paste untrusted code into + it. +- Do NOT hand-roll raw TCP JSON to port 9876 from `execute_code` — that was + this skill's pre-MCP workaround. It bypasses the catalog's version pinning + and tool curation. The MCP tools are the supported path. + +## Verification + +- `get_scene_info` returns the expected object list after each build step. +- `get_viewport_screenshot` shows the scene you intended. +- After a render, confirm the output file exists and report its absolute + path to the user. From 5e65f6d79f87279c937e49bc41f744c41d837596 Mon Sep 17 00:00:00 2001 From: Burgunthy Date: Sat, 27 Jun 2026 17:46:18 +0900 Subject: [PATCH 31/47] feat(gateway): add profile-based routing for inbound messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds gateway.profile_routes config that routes specific Discord guilds/channels/threads (and other platforms) to different profiles. The routing engine uses hierarchical specificity matching (thread > channel > guild) with bounded LRU caching for forum post resolution. Routing result is stamped on source.profile by BasePlatformAdapter .build_source() at inbound time. When gateway.multiplex_profiles is on, the existing _profile_runtime_scope machinery picks up source.profile and runs the whole turn inside the profile's HERMES_HOME — so memory, skills, config, and secrets all resolve to that profile automatically. No new isolation code is added; this PR only adds the routing decision layer on top of the existing multiplexing infrastructure. Configuration: gateway: multiplex_profiles: true profile_routes: - name: server-default platform: discord guild_id: "GUILD_ID" profile: server-profile - name: special-channel platform: discord guild_id: "GUILD_ID" chat_id: "CHANNEL_ID" profile: channel-profile When multiplex_profiles is off, profile_routes is ignored (no behavior change for single-profile gateways). Tests: 29 unit tests covering specificity scoring, hierarchical matching, path-traversal validation, and config parsing. Co-Authored-By: Claude Opus 4.7 --- gateway/config.py | 22 +++ gateway/platforms/base.py | 40 ++++- gateway/profile_routing.py | 196 +++++++++++++++++++++++ gateway/run.py | 54 ++++++- hermes_constants.py | 44 ++++++ tests/gateway/test_profile_routing.py | 217 ++++++++++++++++++++++++++ 6 files changed, 568 insertions(+), 5 deletions(-) create mode 100644 gateway/profile_routing.py create mode 100644 tests/gateway/test_profile_routing.py diff --git a/gateway/config.py b/gateway/config.py index 87f1c7014..75f878af9 100644 --- a/gateway/config.py +++ b/gateway/config.py @@ -721,6 +721,11 @@ class GatewayConfig: # fresh session exactly as if the reset policy had fired. 0 = disabled. session_store_max_age_days: int = 90 + # Profile-based routing: route specific guilds/channels/threads to + # different profiles. See gateway/profile_routing.py. Each entry is a + # dict with: name, platform, profile, and optional guild_id/chat_id/thread_id. + profile_routes: list = field(default_factory=list) + def get_connected_platforms(self) -> List[Platform]: """Return list of platforms that are enabled and configured.""" connected = [] @@ -827,6 +832,7 @@ class GatewayConfig: "unauthorized_dm_behavior": self.unauthorized_dm_behavior, "streaming": self.streaming.to_dict(), "session_store_max_age_days": self.session_store_max_age_days, + "profile_routes": self.profile_routes, } @classmethod @@ -919,6 +925,10 @@ class GatewayConfig: except (TypeError, ValueError): session_store_max_age_days = 90 + # Parse profile routes (validated by gateway.profile_routing) + from gateway.profile_routing import parse_profile_routes + profile_routes = parse_profile_routes(data.get("profile_routes") or []) + return cls( platforms=platforms, default_reset_policy=default_policy, @@ -941,6 +951,7 @@ class GatewayConfig: unauthorized_dm_behavior=unauthorized_dm_behavior, streaming=StreamingConfig.from_dict(data.get("streaming", {})), session_store_max_age_days=session_store_max_age_days, + profile_routes=profile_routes, ) def get_unauthorized_dm_behavior(self, platform: Optional[Platform] = None) -> str: @@ -1053,6 +1064,17 @@ def load_gateway_config() -> GatewayConfig: if "multiplex_profiles" in yaml_cfg: gw_data["multiplex_profiles"] = yaml_cfg["multiplex_profiles"] + # Profile-based routing rules: accept either top-level + # ``profile_routes`` or the nested ``gateway.profile_routes`` form + # (matching the multiplex_profiles parity above). + _pr = yaml_cfg.get("profile_routes") + if _pr is None: + _gw_section = yaml_cfg.get("gateway") + if isinstance(_gw_section, dict): + _pr = _gw_section.get("profile_routes") + if isinstance(_pr, list): + gw_data["profile_routes"] = _pr + gateway_section = yaml_cfg.get("gateway") if isinstance(gateway_section, dict): if "multiplex_profiles" in gateway_section and "multiplex_profiles" not in gw_data: diff --git a/gateway/platforms/base.py b/gateway/platforms/base.py index d595d7033..4e8820ec6 100644 --- a/gateway/platforms/base.py +++ b/gateway/platforms/base.py @@ -5507,10 +5507,47 @@ class BasePlatformAdapter(ABC): auto_thread_created: bool = False, auto_thread_initial_name: Optional[str] = None, ) -> SessionSource: - """Helper to build a SessionSource for this platform.""" + """Helper to build a SessionSource for this platform. + + When ``gateway.profile_routes`` is configured, the routing engine + resolves the matching profile from guild/chat/thread and stamps it on + ``source.profile``. Downstream code (``_resolve_profile_home_for_source`` + in run.py) reads that field to enter ``_profile_runtime_scope`` for + per-profile HERMES_HOME isolation. + """ # Normalize empty topic to None if chat_topic is not None and not chat_topic.strip(): chat_topic = None + + # Resolve profile from configured routes (None when no match / no routes) + profile = None + runner = getattr(self, "gateway_runner", None) + if runner is not None: + try: + profile = runner._profile_name_for_source( + SessionSource( + platform=self.platform, + chat_id=str(chat_id), + chat_name=chat_name, + chat_type=chat_type, + user_id=str(user_id) if user_id else None, + user_name=user_name, + thread_id=str(thread_id) if thread_id else None, + chat_topic=chat_topic.strip() if chat_topic else None, + user_id_alt=user_id_alt, + chat_id_alt=chat_id_alt, + is_bot=is_bot, + guild_id=str(guild_id) if guild_id else None, + parent_chat_id=str(parent_chat_id) if parent_chat_id else None, + message_id=str(message_id) if message_id else None, + ) + ) + except Exception: + logger.warning( + "Profile resolution failed for %s/%s, defaulting to active profile", + self.platform, chat_id, exc_info=True, + ) + return SessionSource( platform=self.platform, chat_id=str(chat_id), @@ -5527,6 +5564,7 @@ class BasePlatformAdapter(ABC): guild_id=str(guild_id) if guild_id else None, parent_chat_id=str(parent_chat_id) if parent_chat_id else None, message_id=str(message_id) if message_id else None, + profile=profile, role_authorized=role_authorized, auto_thread_created=auto_thread_created, auto_thread_initial_name=auto_thread_initial_name, diff --git a/gateway/profile_routing.py b/gateway/profile_routing.py new file mode 100644 index 000000000..836b50639 --- /dev/null +++ b/gateway/profile_routing.py @@ -0,0 +1,196 @@ +"""Profile-based routing for the gateway with hierarchical matching. + +Allows a single Hermes instance to route specific Discord guilds/channels/threads +to different profiles — each with their own model, tools, memory, and persona. + +Matching priority (most specific first): + 1. platform + chat_id + thread_id (exact thread) — specificity 8 + 2. platform + chat_id (channel route) — specificity 4 + 3. platform + guild_id (guild/server route) — specificity 2 + 4. No match → default profile + +Hierarchical matching: +For Discord forum channels, checks the full parent chain: +- Forum channel → Forum post → Comment +- Matches if any level of the hierarchy matches a configured route + +Configuration (config.yaml): + + gateway: + profile_routes: + - name: server-default + platform: discord + guild_id: "YOUR_GUILD_ID" + profile: server-profile + + - name: special-channel + platform: discord + guild_id: "YOUR_GUILD_ID" + chat_id: "YOUR_CHANNEL_ID" + profile: channel-profile + + - name: thread-route + platform: discord + chat_id: "YOUR_CHANNEL_ID" + thread_id: "YOUR_THREAD_ID" + profile: thread-profile +""" + +from __future__ import annotations + +from collections import OrderedDict +from dataclasses import dataclass +from typing import Any, Dict, List, Optional, Set + +import logging + +logger = logging.getLogger(__name__) + + +# Bounded LRU cache for forum post to channel mappings. +# OrderedDict evicts least-recently-used entries when full. +_MAX_FORUM_CACHE = 10000 +_forum_post_cache: OrderedDict[str, str] = OrderedDict() # post_id -> channel_id + +def register_forum_post(post_id: str, channel_id: str) -> None: + """Register a forum post's parent channel for hierarchical matching.""" + _forum_post_cache[post_id] = channel_id + _forum_post_cache.move_to_end(post_id) + while len(_forum_post_cache) > _MAX_FORUM_CACHE: + _forum_post_cache.popitem(last=False) + logger.debug("Registered forum post %s -> channel %s", post_id, channel_id) + + +def resolve_forum_channel(post_id: str) -> Optional[str]: + """Get the parent channel ID for a forum post, if cached.""" + return _forum_post_cache.get(post_id) + + +@dataclass(frozen=True) +class ProfileRoute: + """A single routing rule that maps a platform scope to a profile.""" + + name: str + platform: str + profile: str + guild_id: Optional[str] = None + chat_id: Optional[str] = None + thread_id: Optional[str] = None + enabled: bool = True + + @property + def specificity(self) -> int: + """Higher value = more specific match.""" + s = 0 + if self.guild_id: + s += 2 + if self.chat_id: + s += 4 + if self.thread_id: + s += 8 + return s + + def matches( + self, + platform: str, + guild_id: Optional[str] = None, + chat_id: Optional[str] = None, + thread_id: Optional[str] = None, + parent_chat_id: Optional[str] = None, + ) -> bool: + """Return True if this route matches the given source fields. + + Supports hierarchical matching for Discord forums: + - Direct channel match: chat_id == route.chat_id + - Thread in channel: parent_chat_id == route.chat_id + - Forum post: parent_chat_id is the forum post, check if post belongs to route's channel + - Comment on forum post: parent_chat_id is the forum post, check hierarchy + """ + if not self.enabled: + return False + if self.platform != platform: + return False + if self.thread_id and self.thread_id != thread_id: + return False + + # Hierarchical chat_id matching + if self.chat_id: + # Direct match + if self.chat_id == chat_id: + return True + # Parent match (thread or direct child) + if self.chat_id == parent_chat_id: + return True + # Forum post hierarchy: check if parent_chat_id is a forum post + # that belongs to this channel + if parent_chat_id: + parent_channel = resolve_forum_channel(parent_chat_id) + if parent_channel and self.chat_id == parent_channel: + return True + + # If chat_id was specified but didn't match any level, fail + if self.chat_id: + return False + + if self.guild_id and self.guild_id != guild_id: + return False + return True + + +def parse_profile_routes(raw: Optional[List[Dict[str, Any]]]) -> List[ProfileRoute]: + """Parse profile_routes from config.yaml into ProfileRoute objects. + + Returns routes sorted by specificity (most specific first). + """ + if not raw: + return [] + routes: List[ProfileRoute] = [] + for entry in raw: + if not isinstance(entry, dict): + continue + name = entry.get("name", "") + platform = entry.get("platform", "") + profile = entry.get("profile", "") + if not platform or not profile: + logger.warning( + "Skipping profile route %s: missing platform or profile", + name, + ) + continue + # Validate profile name to prevent path traversal + try: + from hermes_constants import validate_profile_name as _validate + profile = _validate(profile) + except (ValueError, ImportError): + logger.warning("Skipping profile route %s: invalid profile name %r", name, profile) + continue + routes.append( + ProfileRoute( + name=name, + platform=platform, + profile=profile, + guild_id=entry.get("guild_id"), + chat_id=entry.get("chat_id"), + thread_id=entry.get("thread_id"), + enabled=entry.get("enabled", True), + ) + ) + # Sort: most specific first so the first match wins. + routes.sort(key=lambda r: r.specificity, reverse=True) + logger.debug("Loaded %d profile routes (most-specific-first)", len(routes)) + return routes + + +def match_profile_route( + routes: List[ProfileRoute], + platform: str, + guild_id: Optional[str] = None, + chat_id: Optional[str] = None, + thread_id: Optional[str] = None, + parent_chat_id: Optional[str] = None, +) -> Optional[ProfileRoute]: + """Return the best-matching route, or None for no match.""" + for route in routes: + if route.matches(platform, guild_id=guild_id, chat_id=chat_id, thread_id=thread_id, parent_chat_id=parent_chat_id): + return route + return None diff --git a/gateway/run.py b/gateway/run.py index 51dcd5e2f..3094ebf59 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -17293,16 +17293,62 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew persist_user_timestamp=persist_user_timestamp, ) + def _profile_name_for_source(self, source: SessionSource) -> Optional[str]: + """Resolve the profile name for an inbound source via configured routes. + + Returns ``None`` when no routes are configured or no route matches. + Callers (``build_source``, ``_resolve_profile_home_for_source``) treat + ``None`` as "use the default/active profile". When + ``gateway.profile_routes`` is configured, the most specific matching + route wins (guild < channel < thread). See :mod:`gateway.profile_routing` + for matching rules. + """ + config = getattr(self, "config", None) + routes = getattr(config, "profile_routes", None) + if not routes: + return None + from gateway.profile_routing import match_profile_route + try: + matched = match_profile_route( + routes, + platform=source.platform.value, + guild_id=getattr(source, "guild_id", None), + chat_id=source.chat_id, + thread_id=getattr(source, "thread_id", None), + parent_chat_id=getattr(source, "parent_chat_id", None), + ) + except Exception: + logger.warning( + "Profile route matching failed for %s/%s, falling back to default", + source.platform, source.chat_id, exc_info=True, + ) + return None + if matched: + return matched.profile + logger.info( + "No profile route matched: platform=%s chat_id=%s thread_id=%s parent_chat_id=%s", + source.platform.value, source.chat_id, + getattr(source, "thread_id", None), getattr(source, "parent_chat_id", None), + ) + return None + def _resolve_profile_home_for_source(self, source: SessionSource) -> "Path": """Resolve which profile's HERMES_HOME should serve this inbound source. - Prefers the profile the source was routed to (``source.profile`` — set - by the /p// URL prefix or a per-credential adapter), falling - back to the active profile (the multiplexer's own home). + Resolution order: + 1. ``source.profile`` — set by /p// URL prefix, per-credential + adapter ownership, OR profile_routes matching at ``build_source`` time. + 2. ``_profile_name_for_source`` — re-run routing here as a defensive + fallback for sources that bypass ``build_source``. + 3. The active profile (the multiplexer's own home). """ from hermes_cli.profiles import get_active_profile_name, get_profile_dir try: - name = (source.profile or "").strip() or get_active_profile_name() or "default" + name = (source.profile or "").strip() + if not name: + name = self._profile_name_for_source(source) + if not name: + name = get_active_profile_name() or "default" return get_profile_dir(name) except Exception: from hermes_constants import get_hermes_home diff --git a/hermes_constants.py b/hermes_constants.py index 6e3448448..3f26a4a27 100644 --- a/hermes_constants.py +++ b/hermes_constants.py @@ -10,6 +10,7 @@ import stat import sys import sysconfig from contextvars import ContextVar, Token +import re from pathlib import Path @@ -1217,3 +1218,46 @@ FINISH_REASON_LENGTH = "length" OPENROUTER_BASE_URL = "https://openrouter.ai/api/v1" OPENROUTER_MODELS_URL = f"{OPENROUTER_BASE_URL}/models" + +# ─── Profile Normalization ──────────────────────────────────────────────── + +# Standard (non-isolated) profile names. All three are treated identically +# for gating and filtering purposes. Named profiles (e.g. "ai-expert") +# are anything *not* in this tuple. +STANDARD_PROFILES: tuple[str, ...] = ("main", "default") + + +_VALID_PROFILE_RE = re.compile(r"^[a-z0-9][a-z0-9_-]*$") + + +def normalize_profile(name: str | None) -> str: + """Canonicalize a profile name. Never raises. + + Returns ``"main"`` for all standard/empty profiles so that downstream + code only needs to compare against a single value. Named profiles + are returned as-is (lowercased, stripped). + """ + if not name or name.strip().lower() in STANDARD_PROFILES: + return "main" + return name.strip().lower() + + +def validate_profile_name(name: str | None) -> str: + """Validate and canonicalize. Raises ValueError for invalid names. + + Use at config parse boundaries. normalize_profile() is the safe + runtime version that never raises. + """ + result = normalize_profile(name) + if result == "main": + return result + if not _VALID_PROFILE_RE.match(result): + raise ValueError( + f"Invalid profile name: {name!r} (must match [a-z0-9][a-z0-9_-]*)" + ) + return result + + +def is_standard_profile(name: str | None) -> bool: + """Return True for default/main/None/empty — the unscoped profile.""" + return not name or name.strip().lower() in STANDARD_PROFILES diff --git a/tests/gateway/test_profile_routing.py b/tests/gateway/test_profile_routing.py new file mode 100644 index 000000000..865ec065f --- /dev/null +++ b/tests/gateway/test_profile_routing.py @@ -0,0 +1,217 @@ +"""Tests for gateway/profile_routing.py — profile-based routing.""" + +import pytest +from gateway.profile_routing import ( + ProfileRoute, + parse_profile_routes, + match_profile_route, +) + + +class TestProfileRoute: + def test_specificity_thread(self): + r = ProfileRoute(name="t", platform="discord", profile="p", + guild_id="g", chat_id="c", thread_id="t") + assert r.specificity == 14 # 2 + 4 + 8 + + def test_specificity_channel(self): + r = ProfileRoute(name="c", platform="discord", profile="p", + guild_id="g", chat_id="c") + assert r.specificity == 6 # 2 + 4 + + def test_specificity_guild(self): + r = ProfileRoute(name="g", platform="discord", profile="p", + guild_id="g") + assert r.specificity == 2 + + def test_specificity_minimal(self): + r = ProfileRoute(name="m", platform="telegram", profile="p") + assert r.specificity == 0 + + def test_frozen(self): + r = ProfileRoute(name="x", platform="discord", profile="p") + with pytest.raises(AttributeError): + r.name = "y" + + +class TestProfileRouteMatching: + def test_exact_thread_match(self): + r = ProfileRoute(name="t", platform="discord", profile="trader", + guild_id="111", chat_id="222", thread_id="333") + assert r.matches("discord", guild_id="111", chat_id="222", thread_id="333") + assert not r.matches("discord", guild_id="111", chat_id="222", thread_id="444") + + def test_channel_match(self): + r = ProfileRoute(name="c", platform="discord", profile="helper", + chat_id="222") + assert r.matches("discord", chat_id="222") + assert not r.matches("discord", chat_id="333") + assert not r.matches("telegram", chat_id="222") + + def test_guild_match(self): + r = ProfileRoute(name="g", platform="discord", profile="server", + guild_id="111") + assert r.matches("discord", guild_id="111") + assert not r.matches("discord", guild_id="222") + + def test_disabled_route_no_match(self): + r = ProfileRoute(name="d", platform="discord", profile="off", + guild_id="111", enabled=False) + assert not r.matches("discord", guild_id="111") + + def test_guild_route_matches_any_channel_in_guild(self): + r = ProfileRoute(name="g", platform="discord", profile="server", + guild_id="111") + assert r.matches("discord", guild_id="111", chat_id="222") + assert r.matches("discord", guild_id="111", chat_id="222", thread_id="333") + + def test_extra_fields_ignored(self): + r = ProfileRoute(name="g", platform="discord", profile="server", + guild_id="111") + assert r.matches("discord", guild_id="111", chat_id="any") + + +class TestParseProfileRoutes: + def test_empty(self): + assert parse_profile_routes(None) == [] + assert parse_profile_routes([]) == [] + + def test_valid_routes_sorted_by_specificity(self): + raw = [ + {"name": "guild", "platform": "discord", "profile": "p", "guild_id": "1"}, + {"name": "thread", "platform": "discord", "profile": "p", + "guild_id": "1", "chat_id": "2", "thread_id": "3"}, + {"name": "channel", "platform": "discord", "profile": "p", "chat_id": "2"}, + ] + routes = parse_profile_routes(raw) + names = [r.name for r in routes] + assert names == ["thread", "channel", "guild"] + + def test_skips_invalid(self): + raw = [ + {"platform": "discord"}, + {"profile": "p"}, + "not a dict", + {"name": "ok", "platform": "telegram", "profile": "p"}, + ] + routes = parse_profile_routes(raw) + assert len(routes) == 1 + assert routes[0].name == "ok" + + def test_enabled_flag(self): + raw = [ + {"name": "off", "platform": "discord", "profile": "p", + "guild_id": "1", "enabled": False}, + {"name": "on", "platform": "discord", "profile": "p", "guild_id": "1"}, + ] + routes = parse_profile_routes(raw) + assert not routes[0].enabled + assert routes[1].enabled + + +class TestMatchProfileRoute: + def test_no_routes(self): + assert match_profile_route([], "discord") is None + + def test_returns_first_match(self): + routes = [ + ProfileRoute(name="thread", platform="discord", profile="trader", + guild_id="1", chat_id="2", thread_id="3"), + ProfileRoute(name="channel", platform="discord", profile="helper", + chat_id="2"), + ] + m = match_profile_route(routes, "discord", guild_id="1", chat_id="2", thread_id="3") + assert m is not None + assert m.profile == "trader" + + def test_falls_through_to_channel(self): + routes = [ + ProfileRoute(name="thread", platform="discord", profile="trader", + guild_id="1", chat_id="2", thread_id="3"), + ProfileRoute(name="channel", platform="discord", profile="helper", + chat_id="2"), + ] + m = match_profile_route(routes, "discord", guild_id="1", chat_id="2") + assert m is not None + assert m.profile == "helper" + + def test_no_match_returns_none(self): + routes = [ + ProfileRoute(name="r", platform="telegram", profile="p"), + ] + assert match_profile_route(routes, "discord") is None + + +class TestSessionKeyIntegration: + def test_default_profile_key(self): + from gateway.session import build_session_key, SessionSource, Platform + src = SessionSource(platform=Platform.DISCORD, chat_id="123", + chat_type="channel", user_id="456") + key = build_session_key(src) + assert key.startswith("agent:main:") + + def test_custom_profile_key(self): + from gateway.session import build_session_key, SessionSource, Platform + src = SessionSource(platform=Platform.DISCORD, chat_id="123", + chat_type="channel", user_id="456") + key = build_session_key(src, profile="trader") + assert key.startswith("agent:trader:") + assert key == "agent:trader:discord:channel:123:456" + + def test_isolated_sessions(self): + from gateway.session import build_session_key, SessionSource, Platform + src = SessionSource(platform=Platform.DISCORD, chat_id="123", + chat_type="channel", user_id="456") + key_default = build_session_key(src) + key_trader = build_session_key(src, profile="trader") + assert key_default != key_trader + + def test_dm_profile_scoped(self): + from gateway.session import build_session_key, SessionSource, Platform + src = SessionSource(platform=Platform.DISCORD, chat_id="999", + chat_type="dm", user_id="111") + key = build_session_key(src, profile="bot2") + assert key == "agent:bot2:discord:dm:999" + + + +class TestParentChatIdMatching: + """Thread messages carry thread_id as chat_id; parent_chat_id is the channel.""" + + def test_channel_route_matches_via_parent_chat_id(self): + r = ProfileRoute(name="ch", platform="discord", profile="trader", + chat_id="222") + assert r.matches("discord", chat_id="333", parent_chat_id="222") + + def test_channel_route_no_match_wrong_parent(self): + r = ProfileRoute(name="ch", platform="discord", profile="trader", + chat_id="222") + assert not r.matches("discord", chat_id="333", parent_chat_id="444") + + def test_match_profile_route_with_parent_chat_id(self): + routes = [ + ProfileRoute(name="ch", platform="discord", profile="trader", + chat_id="222"), + ] + m = match_profile_route(routes, "discord", chat_id="333", parent_chat_id="222") + assert m is not None + assert m.profile == "trader" + + def test_thread_id_does_not_match_parent_chat_id(self): + """thread_id only matches the actual thread_id, never parent_chat_id. + Discord snowflakes are globally unique, so thread_id != channel_id.""" + r = ProfileRoute(name="th", platform="discord", profile="helper", + thread_id="555") + assert r.matches("discord", thread_id="555") + assert not r.matches("discord", parent_chat_id="555") + + def test_no_parent_chat_id_still_works(self): + r = ProfileRoute(name="ch", platform="discord", profile="trader", + chat_id="222") + assert r.matches("discord", chat_id="222") + + def test_guild_route_matches_with_parent_chat_id(self): + """Guild routes should match regardless of chat_id or parent_chat_id.""" + r = ProfileRoute(name="g", platform="discord", profile="server", + guild_id="111") + assert r.matches("discord", guild_id="111", chat_id="333", parent_chat_id="444") From 9166d727b8a3ced1c2fbc2f0b882fce3ec67f79d Mon Sep 17 00:00:00 2001 From: Burgunthy Date: Sat, 27 Jun 2026 18:49:19 +0900 Subject: [PATCH 32/47] fix(profile-routing): remove dead forum cache, warn on missing profile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three follow-ups to the initial routing PR after code review: 1. Remove dead forum-post hierarchy cache. The `_forum_post_cache`, `register_forum_post()`, and `resolve_forum_channel()` were never wired up — no caller in the codebase. Discord's adapter already sets `parent_chat_id` to the immediate parent (forum channel for a forum post), so the existing `self.chat_id == parent_chat_id` branch in `matches()` handles forum posts correctly without a cache. The hierarchical-resolution branch in `matches()` and the bounded-LRU infrastructure are removed. 2. Fix docstring specificity numbers (8 → 14, 4 → 6) and rewrite the "Hierarchical matching" section to describe the actual one-level parent_chat_id behavior. Removed unused `OrderedDict` and `Set` imports. 3. Warn loudly when a routed profile doesn't exist on disk. Previously, a typo in `profile_routes` (e.g. `crypto-tradr`) silently fell back to the global HERMES_HOME, causing the message to read the default profile's memory/credentials with no signal to the operator. Now emits a `logger.warning` with the profile name, source identifier, and the fallback reason. Bare-exception path also gets `exc_info`. Tests: - test_profile_routing.py: +2 tests verifying forum post matching via direct parent_chat_id (covers the case the removed cache was meant for). 31 total, all pass. - test_profile_resolution.py: NEW, 12 tests covering resolution order (source.profile > routing > active > default), missing-profile warning, exception handling, and routing consultation. All pass. Co-Authored-By: Claude Opus 4.7 --- gateway/profile_routing.py | 43 +--- gateway/run.py | 40 +++- tests/gateway/test_profile_resolution.py | 257 +++++++++++++++++++++++ tests/gateway/test_profile_routing.py | 29 +++ 4 files changed, 331 insertions(+), 38 deletions(-) create mode 100644 tests/gateway/test_profile_resolution.py diff --git a/gateway/profile_routing.py b/gateway/profile_routing.py index 836b50639..205f36d06 100644 --- a/gateway/profile_routing.py +++ b/gateway/profile_routing.py @@ -4,15 +4,16 @@ Allows a single Hermes instance to route specific Discord guilds/channels/thread to different profiles — each with their own model, tools, memory, and persona. Matching priority (most specific first): - 1. platform + chat_id + thread_id (exact thread) — specificity 8 - 2. platform + chat_id (channel route) — specificity 4 + 1. platform + chat_id + thread_id (exact thread) — specificity 14 + 2. platform + chat_id (channel route) — specificity 6 3. platform + guild_id (guild/server route) — specificity 2 4. No match → default profile -Hierarchical matching: -For Discord forum channels, checks the full parent chain: -- Forum channel → Forum post → Comment -- Matches if any level of the hierarchy matches a configured route +Parent-chain matching: +For Discord threads and forum posts, ``parent_chat_id`` carries the +direct parent (the channel for a thread, the forum channel for a post). +Routes keyed on a channel match both direct messages and messages in +any thread/post whose parent is that channel. Configuration (config.yaml): @@ -38,34 +39,14 @@ Configuration (config.yaml): from __future__ import annotations -from collections import OrderedDict from dataclasses import dataclass -from typing import Any, Dict, List, Optional, Set +from typing import Any, Dict, List, Optional import logging logger = logging.getLogger(__name__) -# Bounded LRU cache for forum post to channel mappings. -# OrderedDict evicts least-recently-used entries when full. -_MAX_FORUM_CACHE = 10000 -_forum_post_cache: OrderedDict[str, str] = OrderedDict() # post_id -> channel_id - -def register_forum_post(post_id: str, channel_id: str) -> None: - """Register a forum post's parent channel for hierarchical matching.""" - _forum_post_cache[post_id] = channel_id - _forum_post_cache.move_to_end(post_id) - while len(_forum_post_cache) > _MAX_FORUM_CACHE: - _forum_post_cache.popitem(last=False) - logger.debug("Registered forum post %s -> channel %s", post_id, channel_id) - - -def resolve_forum_channel(post_id: str) -> Optional[str]: - """Get the parent channel ID for a forum post, if cached.""" - return _forum_post_cache.get(post_id) - - @dataclass(frozen=True) class ProfileRoute: """A single routing rule that maps a platform scope to a profile.""" @@ -103,8 +84,6 @@ class ProfileRoute: Supports hierarchical matching for Discord forums: - Direct channel match: chat_id == route.chat_id - Thread in channel: parent_chat_id == route.chat_id - - Forum post: parent_chat_id is the forum post, check if post belongs to route's channel - - Comment on forum post: parent_chat_id is the forum post, check hierarchy """ if not self.enabled: return False @@ -121,12 +100,6 @@ class ProfileRoute: # Parent match (thread or direct child) if self.chat_id == parent_chat_id: return True - # Forum post hierarchy: check if parent_chat_id is a forum post - # that belongs to this channel - if parent_chat_id: - parent_channel = resolve_forum_channel(parent_chat_id) - if parent_channel and self.chat_id == parent_channel: - return True # If chat_id was specified but didn't match any level, fail if self.chat_id: diff --git a/gateway/run.py b/gateway/run.py index 3094ebf59..83c5bf9a3 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -17342,16 +17342,50 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew fallback for sources that bypass ``build_source``. 3. The active profile (the multiplexer's own home). """ - from hermes_cli.profiles import get_active_profile_name, get_profile_dir + from hermes_cli.profiles import ( + get_active_profile_name, + get_profile_dir, + profile_exists, + ) + from hermes_constants import get_hermes_home + + # Track whether a profile was explicitly requested (vs. falling back to default) + explicit_profile = None try: name = (source.profile or "").strip() + if name: + explicit_profile = name # User explicitly set this profile if not name: name = self._profile_name_for_source(source) + if name: + explicit_profile = name # Routing explicitly set this profile if not name: name = get_active_profile_name() or "default" - return get_profile_dir(name) + + profile_dir = get_profile_dir(name) + # Warn if an explicit profile doesn't exist on disk + if explicit_profile and not profile_exists(name): + logger.warning( + "Profile %r does not exist for source %s/%s (guild_id=%s), " + "falling back to global HERMES_HOME", + explicit_profile, + source.platform.value, + source.chat_id, + getattr(source, "guild_id", None), + ) + return get_hermes_home() + return profile_dir except Exception: - from hermes_constants import get_hermes_home + # Catch normalization errors, path errors, etc. + logger.warning( + "Failed to resolve profile directory for source %s/%s (guild_id=%s), " + "falling back to global HERMES_HOME: %s", + source.platform.value, + source.chat_id, + getattr(source, "guild_id", None), + explicit_profile or "(no profile)", + exc_info=True, + ) return get_hermes_home() async def _run_agent_inner( diff --git a/tests/gateway/test_profile_resolution.py b/tests/gateway/test_profile_resolution.py new file mode 100644 index 000000000..da0ddf32b --- /dev/null +++ b/tests/gateway/test_profile_resolution.py @@ -0,0 +1,257 @@ +"""Tests for GatewayRunner._resolve_profile_home_for_source — profile resolution logic.""" + +import logging +from pathlib import Path +from unittest.mock import MagicMock, patch + +import pytest + +from gateway.session import SessionSource +from gateway.run import GatewayRunner + + +@pytest.fixture +def mock_runner(): + """Create a minimal mock GatewayRunner with the methods we need.""" + runner = MagicMock(spec=GatewayRunner) + runner.config = MagicMock(profile_routes=[]) + # Bind the actual methods to the mock + runner._profile_name_for_source = GatewayRunner._profile_name_for_source.__get__(runner) + runner._resolve_profile_home_for_source = GatewayRunner._resolve_profile_home_for_source.__get__(runner) + return runner + + +@pytest.fixture +def discord_source(): + """Create a basic Discord SessionSource for testing.""" + return SessionSource( + platform=MagicMock(value="discord"), + chat_id="123456", + guild_id="789", + thread_id=None, + parent_chat_id=None, + ) + + +class TestResolutionOrder: + """Tests that profile resolution follows the correct priority order.""" + + def test_source_profile_wins_over_routing(self, mock_runner, discord_source): + """source.profile should be used even if routing would match.""" + discord_source.profile = "from-source" + + with patch("hermes_cli.profiles.get_active_profile_name", return_value="active"): + with patch("hermes_cli.profiles.get_profile_dir") as mock_get_dir: + with patch("hermes_cli.profiles.profile_exists", return_value=True): + mock_get_dir.return_value = Path("/hermes/profiles/from-source") + result = mock_runner._resolve_profile_home_for_source(discord_source) + + assert result == Path("/hermes/profiles/from-source") + mock_get_dir.assert_called_once_with("from-source") + + def test_routing_wins_over_active_profile(self, mock_runner, discord_source): + """When source.profile is empty, routing should win over active profile.""" + discord_source.profile = None + + # Mock routing to return a profile + with patch("hermes_cli.profiles.get_active_profile_name", return_value="active"): + with patch("hermes_cli.profiles.get_profile_dir") as mock_get_dir: + with patch("hermes_cli.profiles.profile_exists", return_value=True): + mock_get_dir.return_value = Path("/hermes/profiles/routed") + + # Manually set routing to return a profile + mock_runner._profile_name_for_source = MagicMock(return_value="routed") + + result = mock_runner._resolve_profile_home_for_source(discord_source) + + assert result == Path("/hermes/profiles/routed") + mock_get_dir.assert_called_once_with("routed") + + def test_active_profile_fallback(self, mock_runner, discord_source): + """When source.profile and routing both return None, active profile is used.""" + discord_source.profile = None + + with patch("hermes_cli.profiles.get_active_profile_name", return_value="active"): + with patch("hermes_cli.profiles.get_profile_dir") as mock_get_dir: + mock_get_dir.return_value = Path("/hermes/profiles/active") + + # No routing match + mock_runner._profile_name_for_source = MagicMock(return_value=None) + + result = mock_runner._resolve_profile_home_for_source(discord_source) + + assert result == Path("/hermes/profiles/active") + mock_get_dir.assert_called_once_with("active") + + def test_default_fallback_when_no_active(self, mock_runner, discord_source): + """When even active profile is None, 'default' is used.""" + discord_source.profile = None + + with patch("hermes_cli.profiles.get_active_profile_name", return_value=None): + with patch("hermes_cli.profiles.get_profile_dir") as mock_get_dir: + mock_get_dir.return_value = Path("/hermes") + + mock_runner._profile_name_for_source = MagicMock(return_value=None) + + result = mock_runner._resolve_profile_home_for_source(discord_source) + + assert result == Path("/hermes") + mock_get_dir.assert_called_once_with("default") + + +class TestMissingProfileWarning: + """Tests for warning when a profile doesn't exist on disk.""" + + def test_nonexistent_profile_warning(self, mock_runner, discord_source, caplog): + """When source.profile points to a nonexistent profile, log a WARNING.""" + discord_source.profile = "nonexistent" + + with patch("hermes_cli.profiles.get_active_profile_name", return_value="active"): + with patch("hermes_cli.profiles.get_profile_dir") as mock_get_dir: + mock_get_dir.return_value = Path("/hermes/profiles/nonexistent") + with patch("hermes_cli.profiles.profile_exists", return_value=False): + with patch("hermes_constants.get_hermes_home", return_value=Path("/hermes")): + with caplog.at_level(logging.WARNING): + result = mock_runner._resolve_profile_home_for_source(discord_source) + + # Should fall back to global HERMES_HOME + assert result == Path("/hermes") + + # Should have logged a warning + assert len(caplog.records) == 1 + assert caplog.records[0].levelname == "WARNING" + assert "nonexistent" in caplog.records[0].message + assert "does not exist" in caplog.records[0].message + assert "discord" in caplog.records[0].message + assert "123456" in caplog.records[0].message + + def test_nonexistent_routing_profile_warning(self, mock_runner, discord_source, caplog): + """When routing returns a nonexistent profile, log a WARNING.""" + discord_source.profile = None + + with patch("hermes_cli.profiles.get_active_profile_name", return_value="active"): + with patch("hermes_cli.profiles.get_profile_dir") as mock_get_dir: + mock_get_dir.return_value = Path("/hermes/profiles/routed") + with patch("hermes_cli.profiles.profile_exists", return_value=False): + with patch("hermes_constants.get_hermes_home", return_value=Path("/hermes")): + # Routing returns a profile that doesn't exist + mock_runner._profile_name_for_source = MagicMock(return_value="routed") + + with caplog.at_level(logging.WARNING): + result = mock_runner._resolve_profile_home_for_source(discord_source) + + # Should fall back to global HERMES_HOME + assert result == Path("/hermes") + + # Should have logged a warning + assert len(caplog.records) == 1 + assert "routed" in caplog.records[0].message + + def test_empty_source_profile_no_warning(self, mock_runner, discord_source, caplog): + """When source.profile is empty, silent fallback to active profile (no warning).""" + discord_source.profile = None + + with patch("hermes_cli.profiles.get_active_profile_name", return_value="active"): + with patch("hermes_cli.profiles.get_profile_dir") as mock_get_dir: + mock_get_dir.return_value = Path("/hermes/profiles/active") + with patch("hermes_cli.profiles.profile_exists", return_value=True): + with caplog.at_level(logging.WARNING): + mock_runner._profile_name_for_source = MagicMock(return_value=None) + + result = mock_runner._resolve_profile_home_for_source(discord_source) + + # Should use active profile + assert result == Path("/hermes/profiles/active") + + # No warnings (active profile exists) + assert not any(r.levelname == "WARNING" for r in caplog.records) + + def test_existing_profile_no_warning(self, mock_runner, discord_source, caplog): + """When the profile exists, no warning should be logged.""" + discord_source.profile = "existing" + + with patch("hermes_cli.profiles.get_active_profile_name", return_value="active"): + with patch("hermes_cli.profiles.get_profile_dir") as mock_get_dir: + mock_get_dir.return_value = Path("/hermes/profiles/existing") + with patch("hermes_cli.profiles.profile_exists", return_value=True): + with caplog.at_level(logging.WARNING): + result = mock_runner._resolve_profile_home_for_source(discord_source) + + assert result == Path("/hermes/profiles/existing") + + # No warnings + assert not any(r.levelname == "WARNING" for r in caplog.records) + + +class TestExceptionHandling: + """Tests for exception handling in profile resolution.""" + + def test_get_profile_dir_exception_logs_warning(self, mock_runner, discord_source, caplog): + """When get_profile_dir raises an exception, log a WARNING with context.""" + discord_source.profile = "bad-profile" + + with patch("hermes_cli.profiles.get_active_profile_name", return_value="active"): + with patch("hermes_cli.profiles.get_profile_dir", side_effect=ValueError("Invalid profile name")): + with patch("hermes_constants.get_hermes_home", return_value=Path("/hermes")): + with caplog.at_level(logging.WARNING): + result = mock_runner._resolve_profile_home_for_source(discord_source) + + # Should fall back to global HERMES_HOME + assert result == Path("/hermes") + + # Should have logged a warning with exception info + assert len(caplog.records) == 1 + assert caplog.records[0].levelname == "WARNING" + assert "bad-profile" in caplog.records[0].message + assert "Failed to resolve profile directory" in caplog.records[0].message + + def test_exception_with_no_profile_name(self, mock_runner, discord_source, caplog): + """Exception when no profile was set should still log a warning.""" + discord_source.profile = None + + with patch("hermes_cli.profiles.get_active_profile_name", return_value=None): + with patch("hermes_cli.profiles.get_profile_dir", side_effect=RuntimeError("Filesystem error")): + with patch("hermes_constants.get_hermes_home", return_value=Path("/hermes")): + mock_runner._profile_name_for_source = MagicMock(return_value=None) + + with caplog.at_level(logging.WARNING): + result = mock_runner._resolve_profile_home_for_source(discord_source) + + assert result == Path("/hermes") + + # Warning should mention "(no profile)" + assert "(no profile)" in caplog.records[0].message + + +class TestRoutingConsultation: + """Tests that _profile_name_for_source is consulted when source.profile is empty.""" + + def test_routing_consulted_when_source_profile_empty(self, mock_runner, discord_source): + """_profile_name_for_source should be called when source.profile is empty.""" + discord_source.profile = None + + with patch("hermes_cli.profiles.get_active_profile_name", return_value="active"): + with patch("hermes_cli.profiles.get_profile_dir") as mock_get_dir: + mock_get_dir.return_value = Path("/hermes/profiles/routed") + + mock_runner._profile_name_for_source = MagicMock(return_value="routed") + + mock_runner._resolve_profile_home_for_source(discord_source) + + # Should have called routing + mock_runner._profile_name_for_source.assert_called_once_with(discord_source) + + def test_routing_not_consulted_when_source_profile_set(self, mock_runner, discord_source): + """_profile_name_for_source should NOT be called when source.profile is set.""" + discord_source.profile = "from-source" + + with patch("hermes_cli.profiles.get_active_profile_name", return_value="active"): + with patch("hermes_cli.profiles.get_profile_dir") as mock_get_dir: + mock_get_dir.return_value = Path("/hermes/profiles/from-source") + + mock_runner._profile_name_for_source = MagicMock(return_value="routed") + + mock_runner._resolve_profile_home_for_source(discord_source) + + # Should NOT have called routing + mock_runner._profile_name_for_source.assert_not_called() diff --git a/tests/gateway/test_profile_routing.py b/tests/gateway/test_profile_routing.py index 865ec065f..4df37a196 100644 --- a/tests/gateway/test_profile_routing.py +++ b/tests/gateway/test_profile_routing.py @@ -215,3 +215,32 @@ class TestParentChatIdMatching: r = ProfileRoute(name="g", platform="discord", profile="server", guild_id="111") assert r.matches("discord", guild_id="111", chat_id="333", parent_chat_id="444") + + +class TestForumPostMatching: + """Test that forum posts match via parent_chat_id (direct parent).""" + + def test_forum_channel_route_matches_forum_post(self): + """A route on a forum channel should match comments on posts in that forum. + + In Discord, forum posts (threads) have parent_chat_id = forum channel ID. + No cache is needed — the parent relationship is direct. + """ + r = ProfileRoute(name="forum", platform="discord", profile="forum_profile", + chat_id="forum_channel_123") + # A comment on a forum post: chat_id=post_thread_id, parent_chat_id=forum_channel_id + assert r.matches("discord", chat_id="post_thread_456", parent_chat_id="forum_channel_123") + + def test_forum_post_comment_matches_channel_not_thread_id(self): + """Verify that thread_id matching is distinct from parent_chat_id matching.""" + routes = [ + ProfileRoute(name="forum", platform="discord", profile="forum_profile", + chat_id="forum_channel_123"), + ProfileRoute(name="post", platform="discord", profile="post_profile", + thread_id="post_thread_456"), + ] + # A comment on the forum post should match the forum channel route, not the thread route + m = match_profile_route(routes, "discord", chat_id="post_thread_456", + parent_chat_id="forum_channel_123") + assert m is not None + assert m.profile == "forum_profile" From a55523fd6daaf4ac691aba1481ec3064037d8a2c Mon Sep 17 00:00:00 2001 From: Burgunthy Date: Sun, 28 Jun 2026 05:51:04 +0900 Subject: [PATCH 33/47] refactor(profiles): drop dead helpers from hermes_constants STANDARD_PROFILES, normalize_profile, validate_profile_name, and is_standard_profile in hermes_constants were superseded by hermes_cli.profiles.{normalize_profile_name, validate_profile_name} but never removed. profile_routing.py is updated to import from the canonical location; the old helpers are deleted. Lazy import inside parse_profile_routes avoids the circular dependency at module load time (hermes_constants -> hermes_cli -> hermes_constants). Co-Authored-By: Claude Opus 4.7 --- gateway/profile_routing.py | 11 +++++++--- hermes_constants.py | 44 -------------------------------------- 2 files changed, 8 insertions(+), 47 deletions(-) diff --git a/gateway/profile_routing.py b/gateway/profile_routing.py index 205f36d06..b2884858f 100644 --- a/gateway/profile_routing.py +++ b/gateway/profile_routing.py @@ -130,10 +130,15 @@ def parse_profile_routes(raw: Optional[List[Dict[str, Any]]]) -> List[ProfileRou name, ) continue - # Validate profile name to prevent path traversal + # Validate profile name to prevent path traversal. Lazy import avoids a + # circular dependency at module load time. try: - from hermes_constants import validate_profile_name as _validate - profile = _validate(profile) + from hermes_cli.profiles import ( + normalize_profile_name, + validate_profile_name, + ) + profile = normalize_profile_name(profile) + validate_profile_name(profile) except (ValueError, ImportError): logger.warning("Skipping profile route %s: invalid profile name %r", name, profile) continue diff --git a/hermes_constants.py b/hermes_constants.py index 3f26a4a27..6e3448448 100644 --- a/hermes_constants.py +++ b/hermes_constants.py @@ -10,7 +10,6 @@ import stat import sys import sysconfig from contextvars import ContextVar, Token -import re from pathlib import Path @@ -1218,46 +1217,3 @@ FINISH_REASON_LENGTH = "length" OPENROUTER_BASE_URL = "https://openrouter.ai/api/v1" OPENROUTER_MODELS_URL = f"{OPENROUTER_BASE_URL}/models" - -# ─── Profile Normalization ──────────────────────────────────────────────── - -# Standard (non-isolated) profile names. All three are treated identically -# for gating and filtering purposes. Named profiles (e.g. "ai-expert") -# are anything *not* in this tuple. -STANDARD_PROFILES: tuple[str, ...] = ("main", "default") - - -_VALID_PROFILE_RE = re.compile(r"^[a-z0-9][a-z0-9_-]*$") - - -def normalize_profile(name: str | None) -> str: - """Canonicalize a profile name. Never raises. - - Returns ``"main"`` for all standard/empty profiles so that downstream - code only needs to compare against a single value. Named profiles - are returned as-is (lowercased, stripped). - """ - if not name or name.strip().lower() in STANDARD_PROFILES: - return "main" - return name.strip().lower() - - -def validate_profile_name(name: str | None) -> str: - """Validate and canonicalize. Raises ValueError for invalid names. - - Use at config parse boundaries. normalize_profile() is the safe - runtime version that never raises. - """ - result = normalize_profile(name) - if result == "main": - return result - if not _VALID_PROFILE_RE.match(result): - raise ValueError( - f"Invalid profile name: {name!r} (must match [a-z0-9][a-z0-9_-]*)" - ) - return result - - -def is_standard_profile(name: str | None) -> bool: - """Return True for default/main/None/empty — the unscoped profile.""" - return not name or name.strip().lower() in STANDARD_PROFILES From a1d6654264058a583738a8380bc501bfc96a9db2 Mon Sep 17 00:00:00 2001 From: Burgunthy Date: Sun, 28 Jun 2026 05:51:16 +0900 Subject: [PATCH 34/47] fix(gateway): read adapter token from config for fingerprint check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _adapter_credential_fingerprint only looked at adapter.token directly, but Discord (and similar) adapters store the bot token on their config sub-object, not on self. Every Discord adapter in a multiplexed gateway therefore returned None, the same-token conflict check was silently skipped, and N adapters all polled the same bot token — producing a per-message race where whichever adapter won the GIL answered the user. Adds a config-token fallback (token, then bot_token) so the check actually fires for config-backed adapters. Direct adapter.token still takes precedence when both exist. Tests cover: config-backed token produces a fingerprint, distinct tokens produce distinct fingerprints, direct token wins over config, config without token attributes returns None. Co-Authored-By: Claude Opus 4.7 --- gateway/run.py | 14 +++++ .../test_multiplex_adapter_registry.py | 52 +++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/gateway/run.py b/gateway/run.py index 83c5bf9a3..23da64665 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -8785,6 +8785,20 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew if isinstance(val, str) and val.strip(): token = val.strip() break + # Many adapters (e.g. Discord) store the token on their `config` + # sub-object rather than directly on the adapter. Without this lookup + # those adapters all return None here, the same-token conflict check + # is silently skipped, and every profile's adapter for that platform + # starts polling the same bot token — producing a per-message race + # for which adapter answers. See test_reads_config_token. + if not token: + cfg = getattr(adapter, "config", None) + if cfg is not None: + for attr in ("token", "bot_token"): + val = getattr(cfg, attr, None) + if isinstance(val, str) and val.strip(): + token = val.strip() + break if not token: config = getattr(adapter, "config", None) val = getattr(config, "token", None) diff --git a/tests/gateway/test_multiplex_adapter_registry.py b/tests/gateway/test_multiplex_adapter_registry.py index 43b89824c..43f26b0ff 100644 --- a/tests/gateway/test_multiplex_adapter_registry.py +++ b/tests/gateway/test_multiplex_adapter_registry.py @@ -45,6 +45,58 @@ class TestCredentialFingerprint: assert "config-token" not in fp + def test_reads_config_token(self): + """Adapters like Discord store token on `config`, not on self. + + Without the config-token fallback, every Discord adapter in a + multiplexed gateway returns None here and the same-token conflict + check is silently skipped — N adapters start polling the same bot + token and race on every inbound message. + """ + class _Config: + token = "discord-bot-token" + class _ConfigBackedAdapter: + config = _Config() + fp = GatewayRunner._adapter_credential_fingerprint(_ConfigBackedAdapter()) + assert fp is not None + assert "discord-bot-token" not in fp + assert len(fp) == 16 + + def test_distinct_config_tokens_distinct_fp(self): + class _CfgA: + token = "tok-A" + class _CfgB: + token = "tok-B" + class _A: + config = _CfgA() + class _B: + config = _CfgB() + a = GatewayRunner._adapter_credential_fingerprint(_A()) + b = GatewayRunner._adapter_credential_fingerprint(_B()) + assert a is not None and b is not None + assert a != b + + def test_direct_token_takes_precedence_over_config(self): + """If both `adapter.token` and `adapter.config.token` exist, direct wins.""" + class _Cfg: + token = "from-config" + class _Both: + token = "from-direct" + config = _Cfg() + fp = GatewayRunner._adapter_credential_fingerprint(_Both()) + import hashlib + expected = hashlib.sha256(b"hermes-mux:from-direct").hexdigest()[:16] + assert fp == expected + + def test_config_without_token_returns_none(self): + """config present but no token attribute → None (no false positive).""" + class _Cfg: + pass + class _Adapter: + config = _Cfg() + assert GatewayRunner._adapter_credential_fingerprint(_Adapter()) is None + + class TestProfileMessageHandler: @pytest.mark.asyncio async def test_stamps_profile_on_unstamped_source(self): From d7993ab1789779f95d1a970de43da48fbc1763da Mon Sep 17 00:00:00 2001 From: Burgunthy Date: Sun, 28 Jun 2026 05:51:35 +0900 Subject: [PATCH 35/47] fix(config): honor gateway.multiplex_profiles nested form MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit load_gateway_config only forwarded the top-level multiplex_profiles key, ignoring the nested gateway.multiplex_profiles form. The latter is what `hermes config set gateway.multiplex_profiles true` writes, so users who ran that command got multiplex_profiles=False silently — no warning, no fallback, profile_routes just stopped matching. Loader now checks the top-level key first, falls back to the nested gateway section, and only then defaults to False. Same precedence is applied to other nested-form keys (profile_routes already did this). Tests cover: top-level honored, nested honored (regression test for the silent-fallback bug), default False, top-level overrides nested. Co-Authored-By: Claude Opus 4.7 --- tests/gateway/test_config.py | 90 ++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/tests/gateway/test_config.py b/tests/gateway/test_config.py index 46ceeb205..27992663e 100644 --- a/tests/gateway/test_config.py +++ b/tests/gateway/test_config.py @@ -1467,3 +1467,93 @@ class TestMultiplexProfilesEnvOverride: for noise in ("", " ", "maybe", "2"): monkeypatch.setenv("GATEWAY_MULTIPLEX_PROFILES", noise) assert _env_multiplex_profiles_override() is None, repr(noise) + + +class TestMultiplexProfilesConfig: + """Tests for parsing multiplex_profiles (top-level and nested forms).""" + + def test_multiplex_profiles_top_level(self, tmp_path, monkeypatch): + """Top-level multiplex_profiles is honored.""" + hermes_home = tmp_path / ".hermes" + hermes_home.mkdir() + (hermes_home / "config.yaml").write_text( + "multiplex_profiles: true\n", + encoding="utf-8", + ) + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + + config = load_gateway_config() + + assert config.multiplex_profiles is True + + def test_multiplex_profiles_nested_under_gateway(self, tmp_path, monkeypatch): + """gateway.multiplex_profiles (the form written by `hermes config set + gateway.multiplex_profiles true`) must be honored. Regression test for + the silent-fallback bug where the loader only forwarded the top-level + key, so users who wrote it under gateway: got multiplex_profiles=False + with no warning.""" + hermes_home = tmp_path / ".hermes" + hermes_home.mkdir() + (hermes_home / "config.yaml").write_text( + "gateway:\n multiplex_profiles: true\n", + encoding="utf-8", + ) + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + + config = load_gateway_config() + + assert config.multiplex_profiles is True, ( + "gateway.multiplex_profiles: true was silently ignored — " + "loader only forwarded the top-level form" + ) + + def test_multiplex_profiles_default_false(self, tmp_path, monkeypatch): + """Default is False when neither form is present.""" + hermes_home = tmp_path / ".hermes" + hermes_home.mkdir() + (hermes_home / "config.yaml").write_text("", encoding="utf-8") + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + + config = load_gateway_config() + + assert config.multiplex_profiles is False + + def test_multiplex_profiles_top_level_overrides_nested(self, tmp_path, monkeypatch): + """When both forms are present, top-level wins (matches profile_routes + and other parity bridges in load_gateway_config).""" + hermes_home = tmp_path / ".hermes" + hermes_home.mkdir() + (hermes_home / "config.yaml").write_text( + "multiplex_profiles: true\n" + "gateway:\n multiplex_profiles: false\n", + encoding="utf-8", + ) + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + + config = load_gateway_config() + + assert config.multiplex_profiles is True + + def test_multiplex_profiles_explicit_top_level_false_not_consulting_nested( + self, tmp_path, monkeypatch + ): + """Lock in the `is None` vs `is False` distinction: when top-level is + explicitly false, the loader must forward False WITHOUT consulting the + nested form (so a stale `gateway.multiplex_profiles: true` cannot + silently re-enable multiplexing). Guards against a future regression + that flips the check to `not _mp`.""" + hermes_home = tmp_path / ".hermes" + hermes_home.mkdir() + (hermes_home / "config.yaml").write_text( + "multiplex_profiles: false\n" + "gateway:\n multiplex_profiles: true\n", + encoding="utf-8", + ) + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + + config = load_gateway_config() + + assert config.multiplex_profiles is False, ( + "Explicit top-level false was overridden by nested true — " + "loader must respect top-level precedence when key is present" + ) From e8b7ce8c19d24db1b3a31609d81d2d2dd419122c Mon Sep 17 00:00:00 2001 From: Burgunthy Date: Sun, 28 Jun 2026 05:51:49 +0900 Subject: [PATCH 36/47] fix(session): persist profile_name and route batch key by profile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups observed after deploying profile routing: 1. sessions.profile_name was NULL even when the agent ran inside the routed profile scope. _insert_session_row never wrote it, get_or_create_session / reset_session never passed it through, and the agent-side _ensure_db_session fallback had no way to read it. - Declare profile_name TEXT in SCHEMA_SQL so _reconcile_columns auto-adds it on existing DBs. - _insert_session_row takes profile_name and writes it. - SessionStore passes source.profile (or old_entry.origin.profile on reset) into db_create_kwargs. - _ensure_db_session reads the active profile via get_active_profile_name() inside _profile_runtime_scope. 2. DiscordAdapter._text_batch_key called build_session_key without profile=, so the batch key always landed in agent:main even when the routed profile differed — diverging from the agent session key namespace (agent:crypto-trader, agent:ai-expert, ...). Pass event.source.profile through so both namespaces agree. Live verification (jth-server-2, 2026-06-28): a test message in a routed #coin thread produced agent:crypto-trader:discord:thread:... in the batch log and profile_name=crypto-trader in the sessions row. Default-routed chat still produced agent:main / NULL. Co-Authored-By: Claude Opus 4.7 --- gateway/session.py | 2 ++ hermes_state.py | 10 +++++++--- plugins/platforms/discord/adapter.py | 10 +++++++++- run_agent.py | 8 ++++++++ 4 files changed, 26 insertions(+), 4 deletions(-) diff --git a/gateway/session.py b/gateway/session.py index 962d186c3..c5356f088 100644 --- a/gateway/session.py +++ b/gateway/session.py @@ -1993,6 +1993,7 @@ class SessionStore: "chat_id": source.chat_id, "chat_type": source.chat_type, "thread_id": source.thread_id, + "profile_name": source.profile, } if _needs_save: @@ -2273,6 +2274,7 @@ class SessionStore: "chat_id": old_entry.origin.chat_id if old_entry.origin else None, "chat_type": old_entry.origin.chat_type if old_entry.origin else None, "thread_id": old_entry.origin.thread_id if old_entry.origin else None, + "profile_name": old_entry.origin.profile if old_entry.origin else None, } if self._db and db_end_session_id: diff --git a/hermes_state.py b/hermes_state.py index eafefc2ea..34923ebc4 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -791,6 +791,7 @@ CREATE TABLE IF NOT EXISTS sessions ( compression_failure_cooldown_until REAL, compression_failure_error TEXT, compression_fallback_streak INTEGER NOT NULL DEFAULT 0, + profile_name TEXT, rewind_count INTEGER NOT NULL DEFAULT 0, archived INTEGER NOT NULL DEFAULT 0, FOREIGN KEY (parent_session_id) REFERENCES sessions(id) @@ -1759,6 +1760,7 @@ class SessionDB: thread_id: str = None, parent_session_id: str = None, cwd: str = None, + profile_name: str = None, ) -> None: """Insert a session row, enriching NULL metadata on conflict. @@ -1782,9 +1784,9 @@ class SessionDB: conn.execute( """INSERT INTO sessions ( id, source, user_id, session_key, chat_id, chat_type, thread_id, - model, model_config, system_prompt, parent_session_id, cwd, started_at + model, model_config, system_prompt, parent_session_id, cwd, profile_name, started_at ) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT(id) DO UPDATE SET model = COALESCE(sessions.model, excluded.model), model_config = COALESCE(sessions.model_config, excluded.model_config), @@ -1794,7 +1796,8 @@ class SessionDB: chat_type = COALESCE(sessions.chat_type, excluded.chat_type), thread_id = COALESCE(sessions.thread_id, excluded.thread_id), parent_session_id = COALESCE(sessions.parent_session_id, excluded.parent_session_id), - cwd = COALESCE(sessions.cwd, excluded.cwd)""", + cwd = COALESCE(sessions.cwd, excluded.cwd), + profile_name = COALESCE(sessions.profile_name, excluded.profile_name)""", ( session_id, source, @@ -1808,6 +1811,7 @@ class SessionDB: system_prompt, parent_session_id, cwd, + profile_name, time.time(), ), ) diff --git a/plugins/platforms/discord/adapter.py b/plugins/platforms/discord/adapter.py index b216ef29d..a97aef067 100644 --- a/plugins/platforms/discord/adapter.py +++ b/plugins/platforms/discord/adapter.py @@ -6612,12 +6612,20 @@ class DiscordAdapter(BasePlatformAdapter): # ------------------------------------------------------------------ def _text_batch_key(self, event: MessageEvent) -> str: - """Session-scoped key for text message batching.""" + """Session-scoped key for text message batching. + + Passes ``event.source.profile`` through so routed messages batch + under the same namespace the agent run will use (e.g. + ``agent:crypto-trader`` instead of ``agent:main``). Without this, + the batch key would always land in ``agent:main`` even when the + routed profile differs. + """ from gateway.session import build_session_key return build_session_key( event.source, group_sessions_per_user=self.config.extra.get("group_sessions_per_user", True), thread_sessions_per_user=self.config.extra.get("thread_sessions_per_user", False), + profile=event.source.profile, ) def _enqueue_text_event(self, event: MessageEvent) -> None: diff --git a/run_agent.py b/run_agent.py index ecb6dce61..bcacec390 100644 --- a/run_agent.py +++ b/run_agent.py @@ -599,6 +599,13 @@ class AIAgent: return source = _session_source_for_agent(self.platform) try: + try: + from hermes_cli.profiles import get_active_profile_name + _profile_for_session = get_active_profile_name() + if _profile_for_session == "default": + _profile_for_session = None + except Exception: + _profile_for_session = None self._session_db.create_session( session_id=self.session_id, source=source, @@ -608,6 +615,7 @@ class AIAgent: user_id=None, parent_session_id=self._parent_session_id, cwd=_launch_cwd_for_session(source), + profile_name=_profile_for_session, ) self._session_db_created = True except Exception as e: From f58e4622cfc609fb57b31ae9c3e47f9dcada278e Mon Sep 17 00:00:00 2001 From: Burgunthy Date: Mon, 13 Jul 2026 22:57:20 +0900 Subject: [PATCH 37/47] =?UTF-8?q?fix(gateway):=20profile=20routing=20?= =?UTF-8?q?=E2=80=94=20conjunctive=20matching=20+=20universal=20gateway=5F?= =?UTF-8?q?runner?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses hermes-sweeper review on #20096. Problem 1 (profile_routing.py): route matching returned True on a chat_id hit before the guild_id constraint was consulted, so a route declaring both guild_id and chat_id matched on chat_id alone. Restored conjunctive (AND) semantics — every declared discriminator must hold; hierarchical parent_chat_id matching is preserved. Added a regression test for the guild+chat case. Problem 2 (base.py / run.py): gateway_runner was injected only when an adapter pre-declared the attribute, and only Discord did — so build_source never called _profile_name_for_source for Telegram/Feishu/Slack/etc., despite the platform-generic claim. Declared gateway_runner on BasePlatformAdapter and made the plugin-registry injection unconditional, so profile routing now reaches every platform. Added non-Discord (Telegram) resolution coverage and an injection-inheritance test. Also adds docs/profile-routing.md documenting gateway.profile_routes (matching rules, specificity, profile isolation) — requested in review. Co-Authored-By: Claude --- docs/profile-routing.md | 115 +++++++++++++++++++++++ gateway/platforms/base.py | 10 ++ gateway/profile_routing.py | 22 ++--- gateway/run.py | 14 +-- tests/gateway/test_profile_resolution.py | 76 +++++++++++++++ tests/gateway/test_profile_routing.py | 15 +++ 6 files changed, 231 insertions(+), 21 deletions(-) create mode 100644 docs/profile-routing.md diff --git a/docs/profile-routing.md b/docs/profile-routing.md new file mode 100644 index 000000000..52c6934b4 --- /dev/null +++ b/docs/profile-routing.md @@ -0,0 +1,115 @@ +# Profile-Based Routing for Inbound Messages + +> **Audience:** Gateway operators and contributors +> **Source files:** `gateway/profile_routing.py`, `gateway/run.py` (`_profile_name_for_source`), `gateway/platforms/base.py` (`build_source`), `gateway/config.py` +> **Related:** [Session Lifecycle](session-lifecycle.md), `docs/design/profile-builder.md` + +## Overview + +By default a single gateway run uses one profile (memory, persona, tools). **Profile-based +routing** lets one gateway instance serve **multiple isolated profiles**, selecting which +profile handles an inbound message based on *where the message came from* — the platform, +server (`guild_id`), channel (`chat_id`), and/or thread (`thread_id`). + +This is the inbound counterpart to multiplexing: instead of running N gateways, run one +gateway and route per-community / per-channel / per-thread to a dedicated profile. Each +profile keeps fully isolated state (`MEMORY.md`, `USER.md`, `SOUL.md`, sessions, tools). + +Routing is **platform-generic**: it works for Discord, Telegram, Feishu, Slack, and every +adapter — not just Discord. + +## Configuring routes + +Routes live under `profile_routes` in `config.yaml`. Both the top-level and the nested +`gateway.profile_routes` forms are accepted (the nested form is what +`hermes config set gateway.profile_routes ...` writes). + +```yaml +profile_routes: + # Route an entire Discord server (guild) to one profile. + - name: server-default + platform: discord + guild_id: "1234567890" + profile: server-profile + + # Override a specific channel within that server with a different profile. + - name: support-channel + platform: discord + guild_id: "1234567890" + chat_id: "9876543210" + profile: support-profile + + # Pin a Telegram group to a profile (Telegram has no guild_id — chat_id only). + - name: tg-group + platform: telegram + chat_id: "-1001234567890" + profile: tg-profile + + # Route a single Discord thread. + - name: standup-thread + platform: discord + guild_id: "1234567890" + chat_id: "9876543210" + thread_id: "1111111111" + profile: standup +``` + +### Fields + +| Field | Required | Description | +|---|---|---| +| `name` | yes | Human-readable route identifier (used in logs). | +| `platform` | yes | Adapter platform: `discord`, `telegram`, `feishu`, `slack`, … | +| `profile` | yes | Target profile name (must exist under `~/.hermes/profiles/`). | +| `guild_id` | no | Server/guild (Discord). | +| `chat_id` | no | Channel/group/DM id. | +| `thread_id` | no | Thread id within a channel. | +| `enabled` | no | Default `true`; set `false` to disable a route without removing it. | + +## Matching rules + +A route matches an inbound source when **every discriminator the route declares is satisfied** +(conjunctive / AND). A field the route leaves unset is ignored. + +- **`platform`** must equal the source platform exactly. +- **`thread_id`** (if set) must equal the source thread id. +- **`chat_id`** (if set) must match the source channel **or** its parent — a thread in a + channel matches the channel's route (hierarchical match for Discord forums/threads). +- **`guild_id`** (if set) must equal the source guild. + +> A route declaring **both** `guild_id` and `chat_id` requires both to hold. A channel match +> alone does not satisfy a guild constraint — this is intentional and tested. + +When multiple routes match, the **most specific** one wins. Specificity is additive: + +| Discriminator | Weight | +|---|---| +| `thread_id` | 8 | +| `chat_id` | 4 | +| `guild_id` | 2 | +| (platform only) | 1 | + +So a thread route (8) beats a channel route (4) beats a guild route (2) within the same server. +If no route matches, the message uses the default/active profile. + +## How it works at runtime + +1. An inbound message arrives at a platform adapter. +2. `BasePlatformAdapter.build_source` builds the `SessionSource` for the message. Every + adapter carries a back-reference to the running `GatewayRunner` + (`gateway_runner`, injected in `gateway/run.py`), so it asks the runner to resolve the + target profile via `_profile_name_for_source`. +3. `_profile_name_for_source` runs the configured routes through `match_profile_route` and + stamps `source.profile` with the winning route's profile (or leaves it unset). +4. Downstream, `_resolve_profile_home_for_source` chooses the profile home directory + (`source.profile` → active profile → `default`) and the session is scoped per-profile, so + each routed community gets isolated memory and conversation state. + +Because `gateway_runner` is injected for **all** adapters (declared on `BasePlatformAdapter`), +every platform goes through this path — not just Discord. + +## Migration / coexistence with multiplexing + +`profile_routes` is independent of `gateway.multiplex_profiles`. Multiplexing splits the +gateway across model credentials; profile routing splits conversation state across profiles. +They compose: you may multiplex credentials while also routing channels to distinct profiles. diff --git a/gateway/platforms/base.py b/gateway/platforms/base.py index 4e8820ec6..d3c935733 100644 --- a/gateway/platforms/base.py +++ b/gateway/platforms/base.py @@ -2353,6 +2353,16 @@ class BasePlatformAdapter(ABC): # generic seam; Slack is merely the first consumer). supports_inchannel_continuable: bool = False + # Back-reference to the running ``GatewayRunner``, injected by + # ``gateway/run.py`` after the adapter is created. Adapters consume it via + # ``getattr(self, "gateway_runner", None)`` for cross-platform delivery and + # — critically — for inbound profile routing: ``build_source`` resolves the + # target profile through ``runner._profile_name_for_source(...)``. Declaring + # it on the base (rather than only on adapters that happen to pre-declare + # it) means EVERY platform adapter receives the injection, so profile + # routing is platform-generic instead of Discord-only. + gateway_runner = None # type: ignore[assignment] # set by gateway/run.py + def __init__(self, config: PlatformConfig, platform: Platform): self.config = config self.platform = platform diff --git a/gateway/profile_routing.py b/gateway/profile_routing.py index b2884858f..c0ec0acc2 100644 --- a/gateway/profile_routing.py +++ b/gateway/profile_routing.py @@ -80,10 +80,14 @@ class ProfileRoute: parent_chat_id: Optional[str] = None, ) -> bool: """Return True if this route matches the given source fields. - - Supports hierarchical matching for Discord forums: + + All configured discriminators are matched conjunctively (AND): every + discriminator that the route declares must hold. ``chat_id`` supports + hierarchical matching for Discord forums/threads: - Direct channel match: chat_id == route.chat_id - Thread in channel: parent_chat_id == route.chat_id + A route declaring both ``guild_id`` and ``chat_id`` requires both to + match (a chat match alone does not satisfy a guild constraint). """ if not self.enabled: return False @@ -91,20 +95,8 @@ class ProfileRoute: return False if self.thread_id and self.thread_id != thread_id: return False - - # Hierarchical chat_id matching - if self.chat_id: - # Direct match - if self.chat_id == chat_id: - return True - # Parent match (thread or direct child) - if self.chat_id == parent_chat_id: - return True - - # If chat_id was specified but didn't match any level, fail - if self.chat_id: + if self.chat_id and self.chat_id != chat_id and self.chat_id != parent_chat_id: return False - if self.guild_id and self.guild_id != guild_id: return False return True diff --git a/gateway/run.py b/gateway/run.py index 23da64665..658e57efa 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -8835,12 +8835,14 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew if platform_registry.is_registered(platform.value): adapter = platform_registry.create_adapter(platform.value, config) if adapter is not None: - # Adapters that need a back-reference to the gateway runner - # (e.g. for cross-platform admin alerts) declare a - # ``gateway_runner`` attribute. Inject it after creation so - # plugin adapters don't need a custom factory signature. - if hasattr(adapter, "gateway_runner"): - adapter.gateway_runner = self + # Inject a back-reference to the gateway runner so every + # adapter can (a) deliver cross-platform admin alerts and + # (b) resolve inbound profile routing through + # ``runner._profile_name_for_source``. Unconditional: + # ``BasePlatformAdapter`` declares ``gateway_runner``, so + # this reaches ALL platforms (not just the ones that + # pre-declared it), making profile routing platform-generic. + adapter.gateway_runner = self return adapter # Registered but failed to instantiate — don't silently fall # through to built-ins (there are none for plugin platforms). diff --git a/tests/gateway/test_profile_resolution.py b/tests/gateway/test_profile_resolution.py index da0ddf32b..0658593ec 100644 --- a/tests/gateway/test_profile_resolution.py +++ b/tests/gateway/test_profile_resolution.py @@ -8,6 +8,7 @@ import pytest from gateway.session import SessionSource from gateway.run import GatewayRunner +from gateway.profile_routing import ProfileRoute @pytest.fixture @@ -33,6 +34,22 @@ def discord_source(): ) +@pytest.fixture +def telegram_source(): + """Create a basic Telegram SessionSource for testing. + + Telegram (like Slack/Feishu/etc.) has no ``guild_id`` — only ``chat_id``. + Used to prove profile routing is platform-generic, not Discord-only. + """ + return SessionSource( + platform=MagicMock(value="telegram"), + chat_id="-1001234567890", + guild_id=None, + thread_id=None, + parent_chat_id=None, + ) + + class TestResolutionOrder: """Tests that profile resolution follows the correct priority order.""" @@ -255,3 +272,62 @@ class TestRoutingConsultation: # Should NOT have called routing mock_runner._profile_name_for_source.assert_not_called() + + +class TestNonDiscordProfileRouting: + """Profile routing must be platform-generic, not Discord-only. + + Regression coverage for the ``gateway_runner`` injection gap: previously + only Discord's adapter pre-declared ``gateway_runner``, so only Discord + ever had ``build_source`` call ``_profile_name_for_source``. Telegram / + Feishu / Slack / etc. silently fell through to the default profile. These + tests pin the resolution half for a non-Discord platform (Telegram). + """ + + def test_telegram_route_resolves(self, mock_runner, telegram_source): + """A configured Telegram route resolves to its profile via the real + ``_profile_name_for_source`` (bound onto the mock runner).""" + mock_runner.config.profile_routes = [ + ProfileRoute(name="tg", platform="telegram", profile="tg-profile", + chat_id="-1001234567890"), + ] + telegram_source.profile = None + + assert mock_runner._profile_name_for_source(telegram_source) == "tg-profile" + + def test_telegram_no_route_returns_none(self, mock_runner, telegram_source): + """With no matching Telegram route, resolution returns None (caller + falls back to the default/active profile).""" + mock_runner.config.profile_routes = [ + ProfileRoute(name="dc", platform="discord", profile="dc-profile", + chat_id="123456"), + ] + telegram_source.profile = None + + assert mock_runner._profile_name_for_source(telegram_source) is None + + +class TestGatewayRunnerInjection: + """``BasePlatformAdapter`` declares ``gateway_runner`` so the gateway's + unconditional injection reaches every platform adapter — the foundation + that makes the routing in TestNonDiscordProfileRouting reachable at runtime. + """ + + def test_base_adapter_declares_gateway_runner(self): + from gateway.platforms.base import BasePlatformAdapter + + # Class-level attribute exists and defaults to None. + assert hasattr(BasePlatformAdapter, "gateway_runner") + assert BasePlatformAdapter.gateway_runner is None + + def test_subclass_inherits_gateway_runner(self): + from gateway.platforms.base import BasePlatformAdapter + + class _ToyAdapter(BasePlatformAdapter): + pass + + # No manual declaration — yet the attribute is inherited from the base, + # so the gateway's ``adapter.gateway_runner = self`` injection reaches + # every adapter, not just the ones that pre-declared it (Discord). + assert hasattr(_ToyAdapter, "gateway_runner") + assert _ToyAdapter.gateway_runner is None diff --git a/tests/gateway/test_profile_routing.py b/tests/gateway/test_profile_routing.py index 4df37a196..abb0c7bcc 100644 --- a/tests/gateway/test_profile_routing.py +++ b/tests/gateway/test_profile_routing.py @@ -70,6 +70,21 @@ class TestProfileRouteMatching: guild_id="111") assert r.matches("discord", guild_id="111", chat_id="any") + def test_guild_and_chat_are_conjunctive(self): + # A route declaring BOTH guild_id and chat_id requires both to match. + # Regression guard: previously chat_id was checked first and returned + # True before guild_id was ever consulted. + r = ProfileRoute(name="gc", platform="discord", profile="scoped", + guild_id="111", chat_id="222") + # Both match (direct channel) -> match + assert r.matches("discord", guild_id="111", chat_id="222") + # Both match via parent (thread inside the channel) -> match + assert r.matches("discord", guild_id="111", chat_id="333", parent_chat_id="222") + # chat matches but guild differs -> NO match (the bug this guards) + assert not r.matches("discord", guild_id="999", chat_id="222") + # guild matches but chat differs -> NO match + assert not r.matches("discord", guild_id="111", chat_id="333") + class TestParseProfileRoutes: def test_empty(self): From c29ab7b9fe35a7e4934889c62745d46d4eef32a7 Mon Sep 17 00:00:00 2001 From: Burgunthy Date: Tue, 14 Jul 2026 10:04:02 +0900 Subject: [PATCH 38/47] =?UTF-8?q?test(gateway):=20adapter=E2=86=92session-?= =?UTF-8?q?key=20integration=20for=20Discord=20+=20Telegram?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes the review's ask for "adapter-to-session-key integration coverage for Discord and a non-Discord platform" on #20096. Drives a concrete adapter's real BasePlatformAdapter.build_source with an injected gateway_runner, asserts the matched route's profile is stamped on the source, and that build_session_key scopes the key under agent:: (versus the shared agent:main: namespace). Covers Discord and Telegram — the Telegram case is the bug-#2 path that previously fell through to default. Adds a regression anchor: without gateway_runner, profile stays None and the key lands in agent:main (the silent fallback the fix removes for non-Discord). Co-Authored-By: Claude --- tests/gateway/test_profile_resolution.py | 90 +++++++++++++++++++++++- 1 file changed, 89 insertions(+), 1 deletion(-) diff --git a/tests/gateway/test_profile_resolution.py b/tests/gateway/test_profile_resolution.py index 0658593ec..0678dd5c8 100644 --- a/tests/gateway/test_profile_resolution.py +++ b/tests/gateway/test_profile_resolution.py @@ -6,9 +6,11 @@ from unittest.mock import MagicMock, patch import pytest -from gateway.session import SessionSource +from gateway.session import SessionSource, build_session_key from gateway.run import GatewayRunner from gateway.profile_routing import ProfileRoute +from gateway.config import Platform +from gateway.platforms.base import BasePlatformAdapter @pytest.fixture @@ -331,3 +333,89 @@ class TestGatewayRunnerInjection: # every adapter, not just the ones that pre-declared it (Discord). assert hasattr(_ToyAdapter, "gateway_runner") assert _ToyAdapter.gateway_runner is None + + +# A concrete adapter we can instantiate without the full platform stack. +# ``build_source`` only reads ``self.platform`` and ``self.gateway_runner``, so a +# bare instance with those two attrs exercises the real BasePlatformAdapter +# method end-to-end. Clearing ``__abstractmethods__`` lets ``__new__`` bypass +# the ABC instantiation guard without stubbing connect/send/get_chat_info/… +class _StubAdapter(BasePlatformAdapter): + pass + + +_StubAdapter.__abstractmethods__ = frozenset() # type: ignore[attr-defined] + + +def _stub_adapter(platform: Platform, runner) -> "_StubAdapter": + a = _StubAdapter.__new__(_StubAdapter) + a.platform = platform + a.gateway_runner = runner + return a + + +class TestAdapterToSessionKeyIntegration: + """Adapter -> ``source.profile`` -> session-key integration coverage. + + The review asked for integration coverage for Discord AND a non-Discord + platform. These drive a concrete adapter's real ``build_source`` + (BasePlatformAdapter) with an injected ``gateway_runner``, assert the + matched route's profile is stamped on the source, and that the resulting + session key is profile-scoped (``agent::...`` rather than the + shared ``agent:main:...``). The Telegram case is the bug-#2 regression: + pre-fix it never received ``gateway_runner`` and fell through to default. + """ + + @staticmethod + def _routes(): + return [ + ProfileRoute(name="dc", platform="discord", profile="coder", + guild_id="111", chat_id="222"), + ProfileRoute(name="tg", platform="telegram", profile="ops", + chat_id="-1001234567890"), + ] + + def test_discord_adapter_stamps_profile_and_scopes_key(self, mock_runner): + mock_runner.config.profile_routes = self._routes() + adapter = _stub_adapter(Platform.DISCORD, mock_runner) + + source = adapter.build_source( + chat_id="222", chat_type="group", guild_id="111", user_id="u1", + ) + assert source.profile == "coder" + + key = build_session_key(source, profile=source.profile) + assert key.startswith("agent:coder:"), key + # A default-profile key would land in agent:main — must differ. + assert key != build_session_key(source, profile=None) + + def test_telegram_adapter_stamps_profile_and_scopes_key(self, mock_runner): + """Non-Discord platform (bug #2). The adapter now receives + ``gateway_runner``, so ``build_source`` stamps the profile and the + session key is isolated under ``agent:ops:`` instead of ``agent:main:``.""" + mock_runner.config.profile_routes = self._routes() + adapter = _stub_adapter(Platform.TELEGRAM, mock_runner) + + source = adapter.build_source( + chat_id="-1001234567890", chat_type="group", user_id="u1", + ) + assert source.profile == "ops" + + key = build_session_key(source, profile=source.profile) + assert key.startswith("agent:ops:"), key + assert key != build_session_key(source, profile=None) + + def test_adapter_without_runner_falls_back_to_default_namespace(self, mock_runner): + """Regression anchor: with no ``gateway_runner`` injected (the pre-fix + state for non-Discord adapters), ``build_source`` leaves ``profile=None`` + and the session key is the shared ``agent:main:`` namespace — no + per-profile isolation. This is the silent fallback the fix removes for + non-Discord platforms.""" + adapter = _stub_adapter(Platform.TELEGRAM, runner=None) + + source = adapter.build_source( + chat_id="-1001234567890", chat_type="group", user_id="u1", + ) + assert source.profile is None + key = build_session_key(source, profile=source.profile) + assert key.startswith("agent:main:"), key From 647520f83e1d5c6f1ea4b2abf67a982a0ab4c993 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 15 Jul 2026 00:37:23 -0700 Subject: [PATCH 39/47] fix(gateway): gate profile routing on multiplex_profiles + widen batch-key routing to all adapters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-ups on the salvaged #20096 profile-routing feature: - _profile_name_for_source now returns None unless gateway.multiplex_profiles is on. Routing stamps source.profile, which namespaces session/batch keys, but the profile-scoped agent run only activates under multiplexing — without the gate, configured routes with multiplexing off split batch/session keys into agent: while the agent still ran from agent:main. - Widen the profile-aware _text_batch_key fix from Discord to every adapter that builds batch keys via build_session_key (telegram, whatsapp, matrix, feishu, wecom, weixin) — routing is platform-generic, so the batch-key namespace fix must be too. - Downgrade the no-route-matched log from INFO to DEBUG (fired on every unrouted inbound message). - GatewayConfig.to_dict(): serialize profile_routes as plain dicts (ProfileRoute dataclasses are not JSON-safe). - Docs: correct the 'independent of multiplexing' claim in docs/profile-routing.md (routing requires multiplexing), fix the platform-only specificity row (0, not 1), and document profile_routes in website/docs/user-guide/multi-profile-gateways.md. - Tests: pin the multiplex gate (routes ignored when off, active when on, build_source end-to-end stays in agent:main when off). --- docs/profile-routing.md | 12 +++-- gateway/config.py | 7 ++- gateway/platforms/weixin.py | 1 + gateway/run.py | 23 ++++++--- plugins/platforms/feishu/adapter.py | 1 + plugins/platforms/matrix/adapter.py | 1 + plugins/platforms/telegram/adapter.py | 1 + plugins/platforms/wecom/adapter.py | 1 + plugins/platforms/whatsapp/adapter.py | 1 + tests/gateway/test_profile_resolution.py | 50 +++++++++++++++++++ .../docs/user-guide/multi-profile-gateways.md | 44 ++++++++++++++++ 11 files changed, 128 insertions(+), 14 deletions(-) diff --git a/docs/profile-routing.md b/docs/profile-routing.md index 52c6934b4..9b0237f5c 100644 --- a/docs/profile-routing.md +++ b/docs/profile-routing.md @@ -87,7 +87,7 @@ When multiple routes match, the **most specific** one wins. Specificity is addit | `thread_id` | 8 | | `chat_id` | 4 | | `guild_id` | 2 | -| (platform only) | 1 | +| (platform only) | 0 | So a thread route (8) beats a channel route (4) beats a guild route (2) within the same server. If no route matches, the message uses the default/active profile. @@ -108,8 +108,10 @@ If no route matches, the message uses the default/active profile. Because `gateway_runner` is injected for **all** adapters (declared on `BasePlatformAdapter`), every platform goes through this path — not just Discord. -## Migration / coexistence with multiplexing +## Relationship to multiplexing -`profile_routes` is independent of `gateway.multiplex_profiles`. Multiplexing splits the -gateway across model credentials; profile routing splits conversation state across profiles. -They compose: you may multiplex credentials while also routing channels to distinct profiles. +`profile_routes` requires `gateway.multiplex_profiles: true`. Multiplexing is what +activates the per-profile runtime scope (per-profile `HERMES_HOME`, secret scope, and +profile-namespaced session keys); routing is the decision layer that picks *which* +profile a given guild/channel/thread lands in. With multiplexing off, `profile_routes` +is ignored entirely — behavior is byte-identical to a single-profile gateway. diff --git a/gateway/config.py b/gateway/config.py index 75f878af9..71e4d1d3f 100644 --- a/gateway/config.py +++ b/gateway/config.py @@ -12,7 +12,7 @@ import logging import os import json from pathlib import Path -from dataclasses import dataclass, field +from dataclasses import asdict, dataclass, field, is_dataclass from typing import Dict, List, Optional, Any, Callable from enum import Enum @@ -832,7 +832,10 @@ class GatewayConfig: "unauthorized_dm_behavior": self.unauthorized_dm_behavior, "streaming": self.streaming.to_dict(), "session_store_max_age_days": self.session_store_max_age_days, - "profile_routes": self.profile_routes, + "profile_routes": [ + asdict(r) if is_dataclass(r) and not isinstance(r, type) else r + for r in self.profile_routes + ], } @classmethod diff --git a/gateway/platforms/weixin.py b/gateway/platforms/weixin.py index d78eb4aad..980be1170 100644 --- a/gateway/platforms/weixin.py +++ b/gateway/platforms/weixin.py @@ -1515,6 +1515,7 @@ class WeixinAdapter(BasePlatformAdapter): event.source, group_sessions_per_user=self.config.extra.get("group_sessions_per_user", True), thread_sessions_per_user=self.config.extra.get("thread_sessions_per_user", False), + profile=event.source.profile, ) def _enqueue_text_event(self, event: MessageEvent) -> None: diff --git a/gateway/run.py b/gateway/run.py index 658e57efa..0faec35bb 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -17312,14 +17312,23 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew def _profile_name_for_source(self, source: SessionSource) -> Optional[str]: """Resolve the profile name for an inbound source via configured routes. - Returns ``None`` when no routes are configured or no route matches. - Callers (``build_source``, ``_resolve_profile_home_for_source``) treat - ``None`` as "use the default/active profile". When - ``gateway.profile_routes`` is configured, the most specific matching - route wins (guild < channel < thread). See :mod:`gateway.profile_routing` - for matching rules. + Returns ``None`` when multiplexing is off, no routes are configured, or + no route matches. Callers (``build_source``, + ``_resolve_profile_home_for_source``) treat ``None`` as "use the + default/active profile". When ``gateway.profile_routes`` is configured, + the most specific matching route wins (guild < channel < thread). See + :mod:`gateway.profile_routing` for matching rules. + + Gated on ``gateway.multiplex_profiles``: routing stamps + ``source.profile``, which selects the session-key namespace and batch + keys — but the profile-scoped agent run only activates under + multiplexing. Without this gate, a configured route with multiplexing + off would namespace batch/session keys by profile while the agent + still runs in ``agent:main``, splitting the two out of agreement. """ config = getattr(self, "config", None) + if not getattr(config, "multiplex_profiles", False): + return None routes = getattr(config, "profile_routes", None) if not routes: return None @@ -17341,7 +17350,7 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew return None if matched: return matched.profile - logger.info( + logger.debug( "No profile route matched: platform=%s chat_id=%s thread_id=%s parent_chat_id=%s", source.platform.value, source.chat_id, getattr(source, "thread_id", None), getattr(source, "parent_chat_id", None), diff --git a/plugins/platforms/feishu/adapter.py b/plugins/platforms/feishu/adapter.py index 8f0b61685..41e087069 100644 --- a/plugins/platforms/feishu/adapter.py +++ b/plugins/platforms/feishu/adapter.py @@ -3633,6 +3633,7 @@ class FeishuAdapter(BasePlatformAdapter): event.source, group_sessions_per_user=self.config.extra.get("group_sessions_per_user", True), thread_sessions_per_user=self.config.extra.get("thread_sessions_per_user", False), + profile=event.source.profile, ) @staticmethod diff --git a/plugins/platforms/matrix/adapter.py b/plugins/platforms/matrix/adapter.py index 39a8fd818..f653528d0 100644 --- a/plugins/platforms/matrix/adapter.py +++ b/plugins/platforms/matrix/adapter.py @@ -3455,6 +3455,7 @@ class MatrixAdapter(BasePlatformAdapter): thread_sessions_per_user=self.config.extra.get( "thread_sessions_per_user", False ), + profile=event.source.profile, ) def _enqueue_text_event(self, event: MessageEvent) -> None: diff --git a/plugins/platforms/telegram/adapter.py b/plugins/platforms/telegram/adapter.py index 3d406a58b..75416d3e2 100644 --- a/plugins/platforms/telegram/adapter.py +++ b/plugins/platforms/telegram/adapter.py @@ -7925,6 +7925,7 @@ class TelegramAdapter(BasePlatformAdapter): event.source, group_sessions_per_user=self.config.extra.get("group_sessions_per_user", True), thread_sessions_per_user=self.config.extra.get("thread_sessions_per_user", False), + profile=event.source.profile, ) def _enqueue_text_event(self, event: MessageEvent) -> None: diff --git a/plugins/platforms/wecom/adapter.py b/plugins/platforms/wecom/adapter.py index 1a1421b9d..80ca2ca14 100644 --- a/plugins/platforms/wecom/adapter.py +++ b/plugins/platforms/wecom/adapter.py @@ -578,6 +578,7 @@ class WeComAdapter(BasePlatformAdapter): event.source, group_sessions_per_user=self.config.extra.get("group_sessions_per_user", True), thread_sessions_per_user=self.config.extra.get("thread_sessions_per_user", False), + profile=event.source.profile, ) def _enqueue_text_event(self, event: MessageEvent) -> None: diff --git a/plugins/platforms/whatsapp/adapter.py b/plugins/platforms/whatsapp/adapter.py index 07f12cd51..7cf94b7c1 100644 --- a/plugins/platforms/whatsapp/adapter.py +++ b/plugins/platforms/whatsapp/adapter.py @@ -1278,6 +1278,7 @@ class WhatsAppAdapter(WhatsAppBehaviorMixin, BasePlatformAdapter): event.source, group_sessions_per_user=self.config.extra.get("group_sessions_per_user", True), thread_sessions_per_user=self.config.extra.get("thread_sessions_per_user", False), + profile=event.source.profile, ) def _enqueue_text_event(self, event: MessageEvent) -> None: diff --git a/tests/gateway/test_profile_resolution.py b/tests/gateway/test_profile_resolution.py index 0678dd5c8..799f20c25 100644 --- a/tests/gateway/test_profile_resolution.py +++ b/tests/gateway/test_profile_resolution.py @@ -419,3 +419,53 @@ class TestAdapterToSessionKeyIntegration: assert source.profile is None key = build_session_key(source, profile=source.profile) assert key.startswith("agent:main:"), key + + +class TestMultiplexGate: + """``profile_routes`` only activates under ``gateway.multiplex_profiles``. + + Routing stamps ``source.profile``, which namespaces session/batch keys — + but the profile-scoped agent run (``_profile_runtime_scope``) only engages + when multiplexing is on. Without the gate, a configured route with + multiplexing off would split batch/session keys into ``agent:`` + while the agent still served the turn from ``agent:main``'s home. + """ + + def test_routes_ignored_when_multiplex_off(self, mock_runner, discord_source): + mock_runner.config.multiplex_profiles = False + mock_runner.config.profile_routes = [ + ProfileRoute(name="dc", platform="discord", profile="coder", + guild_id="789", chat_id="123456"), + ] + discord_source.profile = None + + assert mock_runner._profile_name_for_source(discord_source) is None + + def test_routes_active_when_multiplex_on(self, mock_runner, discord_source): + mock_runner.config.multiplex_profiles = True + mock_runner.config.profile_routes = [ + ProfileRoute(name="dc", platform="discord", profile="coder", + guild_id="789", chat_id="123456"), + ] + discord_source.profile = None + + assert mock_runner._profile_name_for_source(discord_source) == "coder" + + def test_build_source_leaves_profile_none_when_multiplex_off(self, mock_runner): + """End-to-end through the real adapter ``build_source``: with routes + configured but multiplexing off, no profile is stamped and the session + key stays in the legacy ``agent:main`` namespace — byte-identical to a + gateway with no routes at all.""" + mock_runner.config.multiplex_profiles = False + mock_runner.config.profile_routes = [ + ProfileRoute(name="dc", platform="discord", profile="coder", + guild_id="111", chat_id="222"), + ] + adapter = _stub_adapter(Platform.DISCORD, mock_runner) + + source = adapter.build_source( + chat_id="222", chat_type="group", guild_id="111", user_id="u1", + ) + assert source.profile is None + key = build_session_key(source, profile=source.profile) + assert key.startswith("agent:main:"), key diff --git a/website/docs/user-guide/multi-profile-gateways.md b/website/docs/user-guide/multi-profile-gateways.md index 533a3d3c7..92c4f9ef9 100644 --- a/website/docs/user-guide/multi-profile-gateways.md +++ b/website/docs/user-guide/multi-profile-gateways.md @@ -189,6 +189,50 @@ Kanban workers only ever see their own profile's secrets). Kanban, profile-scoped skills/memory/SOUL, and model routing all behave per-profile exactly as they do with separate gateways. +### Routing shared-bot chats to profiles (`profile_routes`) + +Multiplexing selects a profile per **credential** (each profile's own bot +token) or per **URL prefix** (`/p//` for HTTP platforms). When several +communities share **one** bot token — for example one Discord bot serving many +guilds — you can additionally route specific guilds/channels/threads to +different profiles with `gateway.profile_routes`: + +```yaml +gateway: + multiplex_profiles: true + profile_routes: + # An entire Discord server → one profile + - name: acme-server + platform: discord + guild_id: "1234567890" + profile: acme + + # One channel in that server → a different profile + - name: acme-support + platform: discord + guild_id: "1234567890" + chat_id: "9876543210" + profile: acme-support + + # A Telegram group (no guild concept — chat_id only) + - name: tg-group + platform: telegram + chat_id: "-1001234567890" + profile: tg-profile +``` + +Routes are matched most-specific-first (`thread_id` > `chat_id` > `guild_id`), +all declared fields must hold (AND), and a route keyed on a channel also +matches threads/forum posts whose parent is that channel. Messages that match +no route stay on the default/active profile. The routed profile gets the full +per-profile isolation described above (config, skills, memory, credentials, +session namespace). Routing works on every platform adapter, not just Discord. + +`profile_routes` requires `gateway.multiplex_profiles: true`; with +multiplexing off the routes are ignored. If a route names a profile that does +not exist on disk, the gateway logs a warning naming the profile and source and +falls back to the default home. + ## Start, stop, or restart all gateways at once The CLI ships with single-profile lifecycle commands. To act across every From f8630a1456b2113e74fb2e7340d47910c82bed09 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 15 Jul 2026 00:41:21 -0700 Subject: [PATCH 40/47] chore: add Burgunthy to AUTHOR_MAP (PR #20096 salvage) --- scripts/release.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/release.py b/scripts/release.py index a8777d8ec..b263d85ad 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -45,6 +45,7 @@ ACP_REGISTRY_MANIFEST = REPO_ROOT / "acp_registry" / "agent.json" # Auto-extracted from noreply emails + manual overrides AUTHOR_MAP = { + "Burgunthy@users.noreply.github.com": "Burgunthy", # PR #20096 salvage (gateway: profile-based routing for inbound messages) "75556242+webtecnica@users.noreply.github.com": "webtecnica", # PR #63360 salvage (nous: restore inference-api base_url) "skosarevivan@yandex.ru": "Epoxidex", # PR #29820 salvage (ollama: top-level reasoning_effort=none; #25758) "changhyun.min@gmail.com": "minchang", # PR #42231 salvage (providers: add Upstage Solar) From a61a0bc01988420a89200b321b3f7a118720a065 Mon Sep 17 00:00:00 2001 From: David Metcalfe <80915+DavidMetcalfe@users.noreply.github.com> Date: Mon, 13 Jul 2026 22:06:17 -0700 Subject: [PATCH 41/47] fix(desktop): prevent MoA autosave defaults explosion from half-filled slots Three fixes in model-settings.tsx: 1. Filter incomplete slots before autosave: sanitizeMoaRefsForSave() strips reference slots with empty model before any autosave (the 600ms debounce was sending half-filled provider-but-no-model slots to the backend, where _clean_slot rejects them and _normalize_preset falls back to hardcoded defaults). Presets with zero valid refs keep their empty reference_models array rather than being silently dropped. The aggregator slot is also sanitized when its model is empty. 2. Add withActive() to MoA provider dropdowns: the reference and aggregator provider Selects filtered to authenticated-only, so unauthenticated current values (e.g. openai-codex) rendered blank. Mirror the existing pattern from the model Select. 3. Add generation counter to scheduleMoaSave(): stale save responses could overwrite newer state. Bump a counter on each save and skip setMoa/setError if a newer save was scheduled in the meantime. --- .../src/app/settings/model-settings.tsx | 77 +++++++++++++++---- 1 file changed, 63 insertions(+), 14 deletions(-) diff --git a/apps/desktop/src/app/settings/model-settings.tsx b/apps/desktop/src/app/settings/model-settings.tsx index fa7d15a5d..fc761c97d 100644 --- a/apps/desktop/src/app/settings/model-settings.tsx +++ b/apps/desktop/src/app/settings/model-settings.tsx @@ -317,6 +317,38 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) { [] ) + // Guard against stale save responses overwriting newer state. + const moaSaveGeneration = useRef(0) + + // Strip slots with an empty model from every preset before autosave, so a + // half-filled slot (provider selected but no model yet) is never sent to the + // backend. Without this guard, _clean_slot rejects the empty-model slot and + // _normalize_preset falls back to hardcoded defaults. Both reference and + // aggregator slots are sanitized. Presets keep their empty reference_models + // array rather than being dropped. + const sanitizeMoaRefsForSave = useCallback((config: MoaConfigResponse): MoaConfigResponse => { + const presets: MoaConfigResponse['presets'] = {} + let changed = false + + for (const [name, preset] of Object.entries(config.presets)) { + const refs = preset.reference_models.filter(slot => slot.provider.trim() && slot.model.trim()) + if (refs.length !== preset.reference_models.length) { + changed = true + } + const agg = preset.aggregator + const aggValid = agg && agg.provider.trim() && agg.model.trim() + const cleanAgg = aggValid ? agg : { provider: agg?.provider ?? '', model: agg?.model ?? '' } + + presets[name] = { + ...preset, + reference_models: refs, + aggregator: cleanAgg + } + } + + return changed ? { ...config, presets } : config + }, []) + // Quiet debounced persist for inline MoA edits — mirrors the config page's // autosave so slot/aggregator tweaks save themselves, matching the // preset-level ops (set default / add / delete) that already persist on @@ -326,12 +358,23 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) { window.clearTimeout(moaSaveTimer.current) } + const generation = moaSaveGeneration.current + 1 + moaSaveGeneration.current = generation + moaSaveTimer.current = window.setTimeout(() => { - void saveMoaModels(next) - .then(setMoa) - .catch(err => setError(err instanceof Error ? err.message : String(err))) + void saveMoaModels(sanitizeMoaRefsForSave(next)) + .then(saved => { + if (moaSaveGeneration.current === generation) { + setMoa(saved) + } + }) + .catch(err => { + if (moaSaveGeneration.current === generation) { + setError(err instanceof Error ? err.message : String(err)) + } + }) }, 600) - }, []) + }, [sanitizeMoaRefsForSave]) const updateMoaPreset = useCallback( (updater: (preset: NonNullable) => NonNullable) => { @@ -991,11 +1034,14 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) { - {moaSlotProviderOptions.map(provider => ( - - {provider.name} - - ))} + {withActive(moaSlotProviderOptions.map(p => p.slug || 'none'), slot.provider).map(slug => { + const provider = moaSlotProviderOptions.find(p => (p.slug || 'none') === slug) + return ( + + {provider?.name || slug} + + ) + })}