From ccd32a9f0b44e34ff379876dbe46d2c04e7f4c47 Mon Sep 17 00:00:00 2001 From: phihu Date: Tue, 18 Aug 2026 17:54:26 +0900 Subject: [PATCH] fix(config): warn when a platform_toolsets entry is an empty list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit validate_platform_toolsets() accumulated a single valid_count across every platform, so the "zero valid toolsets" safety net was suppressed as soon as any one platform carried a valid toolset. A platform wiped to [] — the active one, typically cli — therefore produced no warning at all. resolve_enabled_toolsets() honours that empty list verbatim ([] is a list, so the platform-default fallback is skipped), leaving the agent with zero tool schemas. The model then has nothing to call and emits the tool call as assistant text with finish_reason=stop: no error, no warning, no log entry. That is the silent-failure mode this module was written to prevent (#38798). Note the asymmetry this leaves intact: a malformed *string* value is not a list, so it falls back to the platform default and fails open (#78103); an empty list fails closed. The fail-closed resolution is deliberate (the explicit_empty_ selection contract in tools_config.py, and #82010 wants it persistable), so this only adds the missing warning and does not change resolution semantics. Fixes #89050 Co-Authored-By: Claude Opus 5 (1M context) --- hermes_cli/toolset_validation.py | 18 ++++++++ tests/hermes_cli/test_toolset_validation.py | 50 +++++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/hermes_cli/toolset_validation.py b/hermes_cli/toolset_validation.py index 4b72ec6be0..990cfe072d 100644 --- a/hermes_cli/toolset_validation.py +++ b/hermes_cli/toolset_validation.py @@ -29,6 +29,9 @@ def validate_platform_toolsets( the warning includes that as a suggestion. 2. The mapping is non-empty but resolves to *zero* valid toolsets, so the agent would start with no tools at all. + 3. A platform is configured with an *empty* toolset list. Checked + per-platform because the global zero-valid-toolsets net in (2) is + suppressed as soon as any other platform carries a valid toolset. ``is_valid_toolset`` is injected (normally :func:`toolsets.validate_toolset`) so this function performs no imports or I/O and is testable in isolation. @@ -48,6 +51,21 @@ def validate_platform_toolsets( valid_count = 0 for platform, raw in platform_toolsets.items(): + # An explicitly-empty list is honoured verbatim by + # ``resolve_enabled_toolsets()``: ``[]`` *is* a list, so the + # platform-default fallback is skipped and the platform starts with zero + # tools. That is the intended contract for a deliberate opt-out, but it + # reads identically to an accidental wipe, and the global ``valid_count`` + # below cannot catch it — any *other* populated platform pushes the count + # above zero and suppresses the safety net. Report it per-platform so the + # zero-tools end state is never silent. + if isinstance(raw, list) and not raw: + warnings.append( + f"platform '{platform}' is configured with an empty toolset " + f"list — the agent will have no tools on this platform. " + f"Run `hermes tools` to reconfigure." + ) + continue names = raw if isinstance(raw, list) else [raw] for name in names: if not isinstance(name, str) or not name: diff --git a/tests/hermes_cli/test_toolset_validation.py b/tests/hermes_cli/test_toolset_validation.py index 13e7f98717..4a1c470074 100644 --- a/tests/hermes_cli/test_toolset_validation.py +++ b/tests/hermes_cli/test_toolset_validation.py @@ -48,3 +48,53 @@ def test_mixed_valid_and_invalid_flags_only_the_invalid(): + + +def test_empty_list_on_a_platform_warns_even_when_others_are_valid(): + # The #89050 shape: the active platform is wiped to [] while every other + # platform stays populated. The global zero-valid-toolsets net does not fire + # (telegram/discord are valid), so without a per-platform check this config + # produces no warning at all and the agent silently starts with no tools. + cfg = { + "cli": [], + "telegram": ["hermes-telegram"], + "discord": ["hermes-discord"], + } + warnings = validate_platform_toolsets(cfg, _is_valid) + + empty = [w for w in warnings if "empty toolset list" in w] + assert len(empty) == 1 + assert "platform 'cli'" in empty[0] + assert "no tools" in empty[0] + # Populated platforms must not be implicated. + assert "telegram" not in empty[0] + # The global net is genuinely suppressed here — the per-platform warning is + # the only thing standing between the user and a silent zero-tool agent. + assert not any("zero valid toolsets" in w for w in warnings) + + +def test_empty_list_warns_for_each_affected_platform(): + cfg = {"cli": [], "discord": [], "telegram": ["hermes-telegram"]} + warnings = validate_platform_toolsets(cfg, _is_valid) + + empty = [w for w in warnings if "empty toolset list" in w] + assert len(empty) == 2 + assert {"cli", "discord"} == { + p for p in ("cli", "discord") if any(f"platform '{p}'" in w for w in empty) + } + + +def test_empty_list_does_not_mask_unknown_names_on_other_platforms(): + cfg = {"cli": [], "discord": ["bogus"]} + warnings = validate_platform_toolsets(cfg, _is_valid) + + assert any("empty toolset list" in w and "platform 'cli'" in w for w in warnings) + assert any("unknown toolset 'bogus'" in w for w in warnings) + # Nothing valid anywhere, so the global net still fires too. + assert any("zero valid toolsets" in w for w in warnings) + + +def test_populated_platforms_produce_no_empty_list_warning(): + cfg = {"cli": ["hermes-cli"], "telegram": ["hermes-telegram"]} + warnings = validate_platform_toolsets(cfg, _is_valid) + assert warnings == []