fix(config): warn when a platform_toolsets entry is an empty list
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) <noreply@anthropic.com>
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -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 == []
|
||||
|
||||
Reference in New Issue
Block a user