fix(config): greedy literal-key matching + loud phantom-sibling refusal for dotted key names
Builds on webtecnica's escape-aware _split_key_path (#84152, cherry-picked with authorship preserved; earliest fix in the family was RelaxJonh's #80253 greedy-match approach — both behaviors now ship together): - _greedy_literal_match: when navigating an EXISTING mapping, prefer an existing literal key equal to the dot-join of the next N path segments (longest match wins). Dotted model IDs are the norm, so the common unescaped command (config set providers.p.models.grok-4.6.supports_vision true) now hits the real key across set/get/unset instead of creating a phantom sibling. Plain dotted paths with no dotted-key collision split exactly as before. - _phantom_sibling + ValueError in _set_nested: refuse to CREATE a new intermediate mapping that would shadow an existing dotted literal sibling (Soju06's fail-loudly suggestion on #84064); set_config_value surfaces it as a clean CLI error with the escaped spelling to use. - utils.py::atomic_roundtrip_yaml_update (the second split site, #91607 — /model + TUI persistence) now uses the same escape-aware split + greedy literal matching. - CFG-04 empty-segment guard now splits escape-aware so escaped keys are not misclassified. - Tests for every repro shape in the family: #84064 provider model keys, #80006 Matrix room IDs, #91095 dotted models under custom_providers list index (incl. escaped creation-when-absent), #91607 model_overrides via atomic_roundtrip_yaml_update, #99124 dotted leaf keys; plus backward-compat coverage. Also fixed the carrier's one stale assertion (structured-value coercion landed on main after #84152 branched) and removed its dead _MCP_SECRETS_CONFIG fixture flagged in review. - Docs: 'Dots inside key names' section in website/docs/reference/cli-commands.md. Fixes #84064, fixes #80006, fixes #91095, fixes #91607, fixes #99124
This commit is contained in:
+147
-36
@@ -1090,6 +1090,50 @@ def _split_key_path(key: str) -> list[str]:
|
||||
return parts
|
||||
|
||||
|
||||
def _greedy_literal_match(container: dict, parts: list) -> Optional[Tuple[str, int]]:
|
||||
"""Return ``(literal_key, n_consumed)`` for the longest dotted literal match.
|
||||
|
||||
Dots in config key names are the norm, not the exception — model IDs
|
||||
(``grok-4.6``, ``glm-5.3``), Matrix room IDs (``!room:chat.example.cc``),
|
||||
and versioned provider names all embed dots. Users typing
|
||||
``providers.myprov.models.grok-4.6.context_length`` do not know the
|
||||
escape syntax exists, so when navigating an EXISTING mapping we prefer an
|
||||
existing literal key equal to the dot-join of the next N path segments
|
||||
(longest match wins) over blindly splitting. See #84064 / #80006 /
|
||||
#91095 / #91607 / #99124.
|
||||
|
||||
Backward compatible: when no multi-segment literal key exists, the single
|
||||
segment ``parts[0]`` is the only candidate, which is exactly the historic
|
||||
plain-split behavior. Returns ``None`` when nothing matches.
|
||||
"""
|
||||
if not isinstance(container, dict) or not parts:
|
||||
return None
|
||||
for n in range(len(parts), 0, -1):
|
||||
candidate = ".".join(parts[:n])
|
||||
if candidate in container:
|
||||
return candidate, n
|
||||
return None
|
||||
|
||||
|
||||
def _phantom_sibling(container: dict, part: str) -> Optional[str]:
|
||||
"""Return an existing sibling key that ``part`` would shadow, if any.
|
||||
|
||||
Called when a write is about to CREATE a new intermediate mapping named
|
||||
``part``. If the mapping already holds a literal dotted key that starts
|
||||
with ``part + "."`` (e.g. creating ``grok-4`` beside an existing
|
||||
``grok-4.5``), the split almost certainly chopped a dotted leaf name and
|
||||
the write would produce a phantom sibling the runtime never reads.
|
||||
Fail loudly instead of silently corrupting (#84064 discussion).
|
||||
"""
|
||||
if not isinstance(container, dict):
|
||||
return None
|
||||
prefix = part + "."
|
||||
for k in container:
|
||||
if isinstance(k, str) and k.startswith(prefix):
|
||||
return k
|
||||
return None
|
||||
|
||||
|
||||
def _set_nested(config, dotted_key: str, value):
|
||||
"""Set a value at an arbitrarily nested dotted key path.
|
||||
|
||||
@@ -1111,11 +1155,27 @@ def _set_nested(config, dotted_key: str, value):
|
||||
replaced any non-dict value (including lists) with ``{}``, silently
|
||||
destroying list-typed config like ``custom_providers`` whenever a
|
||||
caller used an indexed path.
|
||||
|
||||
Dotted key names (#84064 family): when navigating an existing mapping,
|
||||
an existing literal key equal to the dot-join of the next N segments is
|
||||
preferred over blind splitting (see ``_greedy_literal_match``), so
|
||||
``models.grok-4.6.supports_vision`` lands on the real ``grok-4.6`` entry.
|
||||
And when a write WOULD create a new intermediate mapping that shadows an
|
||||
existing dotted sibling (``grok-4`` beside ``grok-4.5``), it raises
|
||||
``ValueError`` instead of silently writing a phantom the runtime never
|
||||
reads.
|
||||
"""
|
||||
parts = _split_key_path(dotted_key)
|
||||
current = config
|
||||
for part in parts[:-1]:
|
||||
i = 0
|
||||
while i < len(parts):
|
||||
remaining = parts[i:]
|
||||
at_leaf = len(remaining) == 1
|
||||
if isinstance(current, list):
|
||||
part = remaining[0]
|
||||
if at_leaf:
|
||||
current[int(part)] = value
|
||||
return
|
||||
try:
|
||||
idx = int(part)
|
||||
except (TypeError, ValueError):
|
||||
@@ -1124,21 +1184,43 @@ def _set_nested(config, dotted_key: str, value):
|
||||
f"segment {part!r} is not a numeric index"
|
||||
)
|
||||
current = current[idx]
|
||||
i += 1
|
||||
elif isinstance(current, dict):
|
||||
existing = current.get(part)
|
||||
# Preserve dicts and lists; replace missing/scalar with a fresh dict.
|
||||
if part not in current or not isinstance(existing, (dict, list)):
|
||||
current[part] = {}
|
||||
match = _greedy_literal_match(current, remaining)
|
||||
if match is not None:
|
||||
key, consumed = match
|
||||
if i + consumed == len(parts):
|
||||
current[key] = value
|
||||
return
|
||||
existing = current.get(key)
|
||||
# Preserve dicts and lists; replace scalar with a fresh dict.
|
||||
if not isinstance(existing, (dict, list)):
|
||||
current[key] = {}
|
||||
current = current[key]
|
||||
i += consumed
|
||||
continue
|
||||
part = remaining[0]
|
||||
if at_leaf:
|
||||
current[part] = value
|
||||
return
|
||||
# About to CREATE an intermediate mapping. Refuse when that would
|
||||
# write a phantom sibling of an existing dotted literal key.
|
||||
shadowed = _phantom_sibling(current, part)
|
||||
if shadowed is not None:
|
||||
escaped = shadowed.replace(".", "\\.")
|
||||
raise ValueError(
|
||||
f"Refusing to create nested key {part!r} in {dotted_key!r}: "
|
||||
f"the mapping already contains a literal key {shadowed!r} "
|
||||
f"that contains a dot. If you meant that key, escape its "
|
||||
f"dots with a backslash (e.g. {escaped})."
|
||||
)
|
||||
current[part] = {}
|
||||
current = current[part]
|
||||
i += 1
|
||||
else:
|
||||
raise TypeError(
|
||||
f"Cannot navigate into {type(current).__name__} at key {dotted_key!r}"
|
||||
)
|
||||
last = parts[-1]
|
||||
if isinstance(current, list):
|
||||
current[int(last)] = value
|
||||
else:
|
||||
current[last] = value
|
||||
|
||||
|
||||
def clear_model_endpoint_credentials(
|
||||
@@ -1172,59 +1254,84 @@ _MISSING = object()
|
||||
|
||||
|
||||
def _get_nested(config, dotted_key: str):
|
||||
"""Return a dotted-path value from nested dict/list config data."""
|
||||
"""Return a dotted-path value from nested dict/list config data.
|
||||
|
||||
Mirrors ``_set_nested``'s navigation: honors backslash-escaped dots and
|
||||
prefers an existing literal dotted key over blind splitting, so
|
||||
``config get providers.p.models.grok-4.6.context_length`` reads the real
|
||||
``grok-4.6`` entry instead of reporting the key unset (#84064).
|
||||
"""
|
||||
parts = _split_key_path(dotted_key)
|
||||
current = config
|
||||
for part in _split_key_path(dotted_key):
|
||||
i = 0
|
||||
while i < len(parts):
|
||||
remaining = parts[i:]
|
||||
if isinstance(current, list):
|
||||
try:
|
||||
current = current[int(part)]
|
||||
current = current[int(remaining[0])]
|
||||
except (TypeError, ValueError, IndexError):
|
||||
return _MISSING
|
||||
i += 1
|
||||
elif isinstance(current, dict):
|
||||
if part not in current:
|
||||
match = _greedy_literal_match(current, remaining)
|
||||
if match is None:
|
||||
return _MISSING
|
||||
current = current[part]
|
||||
key, consumed = match
|
||||
current = current[key]
|
||||
i += consumed
|
||||
else:
|
||||
return _MISSING
|
||||
return current
|
||||
|
||||
|
||||
def _unset_nested(config, dotted_key: str) -> bool:
|
||||
"""Remove a dotted-path value from nested dict/list config data."""
|
||||
"""Remove a dotted-path value from nested dict/list config data.
|
||||
|
||||
Same escape-aware, greedy-literal navigation as ``_set_nested`` /
|
||||
``_get_nested`` (#84064): unsetting an unescaped dotted key removes the
|
||||
real literal entry rather than a phantom sibling.
|
||||
"""
|
||||
parts = _split_key_path(dotted_key)
|
||||
if not parts:
|
||||
return False
|
||||
|
||||
parents = []
|
||||
current = config
|
||||
for part in parts[:-1]:
|
||||
parents.append((current, part))
|
||||
removed = False
|
||||
i = 0
|
||||
while i < len(parts):
|
||||
remaining = parts[i:]
|
||||
if isinstance(current, list):
|
||||
part = remaining[0]
|
||||
if len(remaining) == 1:
|
||||
try:
|
||||
current.pop(int(part))
|
||||
removed = True
|
||||
break
|
||||
except (TypeError, ValueError, IndexError):
|
||||
return False
|
||||
parents.append((current, part))
|
||||
try:
|
||||
current = current[int(part)]
|
||||
except (TypeError, ValueError, IndexError):
|
||||
return False
|
||||
i += 1
|
||||
elif isinstance(current, dict):
|
||||
if part not in current:
|
||||
match = _greedy_literal_match(current, remaining)
|
||||
if match is None:
|
||||
return False
|
||||
current = current[part]
|
||||
key, consumed = match
|
||||
if i + consumed == len(parts):
|
||||
del current[key]
|
||||
removed = True
|
||||
break
|
||||
parents.append((current, key))
|
||||
current = current[key]
|
||||
i += consumed
|
||||
else:
|
||||
return False
|
||||
|
||||
last = parts[-1]
|
||||
removed = False
|
||||
if isinstance(current, list):
|
||||
try:
|
||||
current.pop(int(last))
|
||||
removed = True
|
||||
except (TypeError, ValueError, IndexError):
|
||||
return False
|
||||
elif isinstance(current, dict):
|
||||
if last not in current:
|
||||
return False
|
||||
del current[last]
|
||||
removed = True
|
||||
else:
|
||||
if not removed:
|
||||
return False
|
||||
|
||||
# Drop empty dict containers left behind by the deletion while preserving
|
||||
@@ -5641,7 +5748,7 @@ def set_config_value(key: str, value: str, force: bool = False):
|
||||
print(f"✗ Invalid config key: {key!r} (empty or surrounding whitespace).",
|
||||
file=sys.stderr)
|
||||
sys.exit(1)
|
||||
if any(seg == "" for seg in key.split(".")):
|
||||
if any(seg == "" for seg in _split_key_path(key)):
|
||||
print(
|
||||
f"✗ Invalid config key: {key!r} — contains an empty path segment "
|
||||
"(leading, trailing, or doubled '.').",
|
||||
@@ -5821,7 +5928,11 @@ def set_config_value(key: str, value: str, force: bool = False):
|
||||
file=sys.stderr,
|
||||
)
|
||||
sys.exit(1)
|
||||
_set_nested(user_config, key, value)
|
||||
try:
|
||||
_set_nested(user_config, key, value)
|
||||
except ValueError as e:
|
||||
print(f"✗ {e}", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
# Normalize the api_base → base_url alias at set-time too (issue #8919),
|
||||
# so a fresh `hermes config set model.api_base ...` lands on the canonical
|
||||
# key the runtime resolver actually reads, instead of being silently
|
||||
|
||||
@@ -0,0 +1,359 @@
|
||||
"""Regression tests for dotted config key names (#84064 bug class).
|
||||
|
||||
Every issue in the family has a distinct repro shape, all rooted in the same
|
||||
unescaped ``key.split(".")``:
|
||||
|
||||
* #84064 — ``providers.<name>.models.<model-with-dot>`` phantom siblings,
|
||||
plus read-path (``config get``) and unset-path breakage.
|
||||
* #80006 — Matrix room IDs with dots under ``matrix.channel_prompts``.
|
||||
* #91095 — creation targets under a ``custom_providers`` list index where the
|
||||
dotted model key already exists (``custom_providers.0.models.qwen3.5:4b``).
|
||||
* #91607 — ``model_overrides.zai.glm-5.3`` via
|
||||
``utils.py::atomic_roundtrip_yaml_update`` (a second split site).
|
||||
* #99124 — dotted leaf keys (``glm-5.3-flash``) across set/get/unset.
|
||||
|
||||
Two complementary behaviors are covered:
|
||||
|
||||
1. Backslash escaping via ``_split_key_path`` (carrier PR #84152).
|
||||
2. Greedy literal-key matching + loud phantom-sibling refusal, because dotted
|
||||
model IDs are the norm and users won't know the escape syntax exists.
|
||||
"""
|
||||
|
||||
import argparse
|
||||
import os
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
import yaml
|
||||
|
||||
from hermes_cli.config import (
|
||||
_MISSING,
|
||||
_get_nested,
|
||||
_greedy_literal_match,
|
||||
_phantom_sibling,
|
||||
_set_nested,
|
||||
_unset_nested,
|
||||
config_command,
|
||||
set_config_value,
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _isolated_hermes_home(tmp_path):
|
||||
"""Point HERMES_HOME at a temp dir so tests never touch real config."""
|
||||
env_file = tmp_path / ".env"
|
||||
env_file.touch()
|
||||
with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}):
|
||||
yield tmp_path
|
||||
|
||||
|
||||
def _write_config(tmp_path, data: dict):
|
||||
(tmp_path / "config.yaml").write_text(yaml.safe_dump(data, sort_keys=False))
|
||||
|
||||
|
||||
def _read_config(tmp_path):
|
||||
return yaml.safe_load((tmp_path / "config.yaml").read_text())
|
||||
|
||||
|
||||
PROVIDER_CONFIG = {
|
||||
"providers": {
|
||||
"myprov": {
|
||||
"name": "My Provider",
|
||||
"base_url": "https://example.invalid/v1",
|
||||
"default_model": "grok-4.6",
|
||||
"models": {
|
||||
"grok-4.6": {"context_length": 500000},
|
||||
"grok-4.5": {"context_length": 500000},
|
||||
},
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Unit level: helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestGreedyLiteralMatch:
|
||||
def test_longest_match_wins(self):
|
||||
d = {"grok-4.6": 1, "grok-4": 2}
|
||||
assert _greedy_literal_match(d, ["grok-4", "6"]) == ("grok-4.6", 2)
|
||||
|
||||
def test_single_segment_fallback(self):
|
||||
assert _greedy_literal_match({"a": 1}, ["a", "b"]) == ("a", 1)
|
||||
|
||||
def test_no_match(self):
|
||||
assert _greedy_literal_match({"a": 1}, ["x", "y"]) is None
|
||||
|
||||
def test_phantom_sibling_detection(self):
|
||||
assert _phantom_sibling({"grok-4.6": {}}, "grok-4") == "grok-4.6"
|
||||
assert _phantom_sibling({"grok-4.6": {}}, "other") is None
|
||||
assert _phantom_sibling({"plain": {}}, "plain") is None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# #84064 — provider model keys with dots: set / get / unset all address the
|
||||
# real literal key WITHOUT any escaping (the common unescaped command).
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestUnescapedDottedModelKeys:
|
||||
def test_set_hits_existing_literal_key_no_phantom(self, _isolated_hermes_home):
|
||||
_write_config(_isolated_hermes_home, PROVIDER_CONFIG)
|
||||
set_config_value("providers.myprov.models.grok-4.6.supports_vision", "true")
|
||||
saved = _read_config(_isolated_hermes_home)
|
||||
models = saved["providers"]["myprov"]["models"]
|
||||
assert models["grok-4.6"]["supports_vision"] is True
|
||||
assert "grok-4" not in models # no phantom sibling
|
||||
|
||||
def test_get_reads_real_dotted_key(self, _isolated_hermes_home, capsys):
|
||||
_write_config(_isolated_hermes_home, PROVIDER_CONFIG)
|
||||
args = argparse.Namespace(
|
||||
config_command="get",
|
||||
key="providers.myprov.models.grok-4.6.context_length",
|
||||
json=False,
|
||||
)
|
||||
config_command(args)
|
||||
assert capsys.readouterr().out.strip() == "500000"
|
||||
|
||||
def test_unset_removes_real_dotted_key_field(self, _isolated_hermes_home):
|
||||
cfg = {
|
||||
"providers": {
|
||||
"myprov": {
|
||||
"models": {
|
||||
"grok-4.6": {
|
||||
"context_length": 500000,
|
||||
"supports_vision": True,
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
_write_config(_isolated_hermes_home, cfg)
|
||||
args = argparse.Namespace(
|
||||
config_command="unset",
|
||||
key="providers.myprov.models.grok-4.6.supports_vision",
|
||||
)
|
||||
config_command(args)
|
||||
saved = _read_config(_isolated_hermes_home)
|
||||
target = saved["providers"]["myprov"]["models"]["grok-4.6"]
|
||||
assert "supports_vision" not in target
|
||||
assert target["context_length"] == 500000
|
||||
|
||||
def test_set_refuses_phantom_when_dotted_sibling_exists(
|
||||
self, _isolated_hermes_home, capsys
|
||||
):
|
||||
"""Soju06's suggestion on #84064: creating a NEW nested mapping that
|
||||
shadows an existing dotted literal key fails loudly instead of
|
||||
silently writing a phantom."""
|
||||
_write_config(_isolated_hermes_home, PROVIDER_CONFIG)
|
||||
with pytest.raises(SystemExit):
|
||||
set_config_value("providers.myprov.models.grok-4.7.context_length", "128000")
|
||||
err = capsys.readouterr().err
|
||||
assert "Refusing to create nested key" in err
|
||||
assert "grok-4" in err
|
||||
# Config untouched.
|
||||
saved = _read_config(_isolated_hermes_home)
|
||||
assert "grok-4" not in saved["providers"]["myprov"]["models"]
|
||||
|
||||
def test_escaped_form_creates_new_dotted_key(self, _isolated_hermes_home):
|
||||
_write_config(_isolated_hermes_home, PROVIDER_CONFIG)
|
||||
set_config_value(
|
||||
"providers.myprov.models.grok-4\\.7.context_length", "128000"
|
||||
)
|
||||
saved = _read_config(_isolated_hermes_home)
|
||||
assert saved["providers"]["myprov"]["models"]["grok-4.7"] == {
|
||||
"context_length": 128000
|
||||
}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# #80006 — Matrix room IDs with dots
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestMatrixRoomIds:
|
||||
def test_set_existing_room_id_key(self, _isolated_hermes_home):
|
||||
_write_config(
|
||||
_isolated_hermes_home,
|
||||
{"matrix": {"channel_prompts": {"!TestRoom:example.org": "old"}}},
|
||||
)
|
||||
set_config_value("matrix.channel_prompts.!TestRoom:example.org", "be brief")
|
||||
saved = _read_config(_isolated_hermes_home)
|
||||
prompts = saved["matrix"]["channel_prompts"]
|
||||
assert prompts["!TestRoom:example.org"] == "be brief"
|
||||
assert "!TestRoom:example" not in prompts
|
||||
|
||||
def test_escaped_creates_new_room_id_key(self, _isolated_hermes_home):
|
||||
_write_config(_isolated_hermes_home, {"matrix": {"channel_prompts": {}}})
|
||||
set_config_value(
|
||||
"matrix.channel_prompts.!TestRoom:example\\.org", "be brief"
|
||||
)
|
||||
saved = _read_config(_isolated_hermes_home)
|
||||
assert saved["matrix"]["channel_prompts"]["!TestRoom:example.org"] == "be brief"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# #91095 — dotted model key under a custom_providers LIST index
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestCustomProvidersListIndex:
|
||||
def test_set_updates_existing_dotted_model_under_list_index(
|
||||
self, _isolated_hermes_home
|
||||
):
|
||||
_write_config(
|
||||
_isolated_hermes_home,
|
||||
{
|
||||
"custom_providers": [
|
||||
{
|
||||
"name": "Local Ollama",
|
||||
"models": {"qwen3.5:4b": {"context_length": 16384}},
|
||||
}
|
||||
]
|
||||
},
|
||||
)
|
||||
set_config_value(
|
||||
"custom_providers.0.models.qwen3.5:4b.context_length", "65536"
|
||||
)
|
||||
saved = _read_config(_isolated_hermes_home)
|
||||
models = saved["custom_providers"][0]["models"]
|
||||
assert models["qwen3.5:4b"]["context_length"] == 65536
|
||||
assert "qwen3" not in models
|
||||
|
||||
def test_escaped_creates_absent_dotted_model_under_list_index(
|
||||
self, _isolated_hermes_home
|
||||
):
|
||||
"""#91095 follow-up: creation of a NOT-yet-existing dotted key must be
|
||||
possible via escaping, even under a list index."""
|
||||
_write_config(
|
||||
_isolated_hermes_home,
|
||||
{"custom_providers": [{"name": "Local Ollama", "models": {}}]},
|
||||
)
|
||||
set_config_value(
|
||||
"custom_providers.0.models.qwen3\\.5:4b.context_length", "65536"
|
||||
)
|
||||
saved = _read_config(_isolated_hermes_home)
|
||||
assert saved["custom_providers"][0]["models"]["qwen3.5:4b"] == {
|
||||
"context_length": 65536
|
||||
}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# #91607 — model_overrides via utils.atomic_roundtrip_yaml_update
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestAtomicRoundtripYamlUpdate:
|
||||
def test_existing_dotted_model_id_not_split(self, tmp_path):
|
||||
from utils import atomic_roundtrip_yaml_update
|
||||
|
||||
path = tmp_path / "config.yaml"
|
||||
path.write_text(
|
||||
"model_overrides:\n"
|
||||
" zai:\n"
|
||||
" glm-5.3:\n"
|
||||
" supports_reasoning: false\n"
|
||||
)
|
||||
atomic_roundtrip_yaml_update(
|
||||
path, "model_overrides.zai.glm-5.3.supports_reasoning", True
|
||||
)
|
||||
saved = yaml.safe_load(path.read_text())
|
||||
overrides = saved["model_overrides"]["zai"]
|
||||
assert overrides["glm-5.3"]["supports_reasoning"] is True
|
||||
assert "glm-5" not in overrides
|
||||
|
||||
def test_escaped_dotted_key_created(self, tmp_path):
|
||||
from utils import atomic_roundtrip_yaml_update
|
||||
|
||||
path = tmp_path / "config.yaml"
|
||||
path.write_text("model_overrides: {}\n")
|
||||
atomic_roundtrip_yaml_update(
|
||||
path, "model_overrides.zai.glm-5\\.3.supports_reasoning", True
|
||||
)
|
||||
saved = yaml.safe_load(path.read_text())
|
||||
assert saved["model_overrides"]["zai"]["glm-5.3"]["supports_reasoning"] is True
|
||||
|
||||
def test_plain_paths_unchanged(self, tmp_path):
|
||||
from utils import atomic_roundtrip_yaml_update
|
||||
|
||||
path = tmp_path / "config.yaml"
|
||||
path.write_text("# keep this comment\ndisplay:\n personality: default\n")
|
||||
atomic_roundtrip_yaml_update(path, "display.personality", "hacker")
|
||||
text = path.read_text()
|
||||
assert "# keep this comment" in text
|
||||
saved = yaml.safe_load(text)
|
||||
assert saved["display"]["personality"] == "hacker"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# #99124 — dotted LEAF keys (glm-5.3-flash) round-trip through set/get/unset
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestDottedLeafKeys:
|
||||
CFG = {
|
||||
"providers": {
|
||||
"Bai": {"models": {"glm-5.3-flash": {"context_length": 128000}}}
|
||||
}
|
||||
}
|
||||
|
||||
def test_set_leaf_parent_is_dotted(self, _isolated_hermes_home):
|
||||
_write_config(_isolated_hermes_home, self.CFG)
|
||||
set_config_value("providers.Bai.models.glm-5.3-flash.context_length", "1000000")
|
||||
saved = _read_config(_isolated_hermes_home)
|
||||
models = saved["providers"]["Bai"]["models"]
|
||||
assert models["glm-5.3-flash"]["context_length"] == 1000000
|
||||
assert "glm-5" not in models
|
||||
|
||||
def test_get_dotted_leaf_mapping(self, _isolated_hermes_home, capsys):
|
||||
_write_config(_isolated_hermes_home, self.CFG)
|
||||
args = argparse.Namespace(
|
||||
config_command="get", key="providers.Bai.models.glm-5.3-flash", json=True
|
||||
)
|
||||
config_command(args)
|
||||
assert "128000" in capsys.readouterr().out
|
||||
|
||||
def test_unset_dotted_leaf(self, _isolated_hermes_home):
|
||||
_write_config(_isolated_hermes_home, self.CFG)
|
||||
args = argparse.Namespace(
|
||||
config_command="unset", key="providers.Bai.models.glm-5.3-flash"
|
||||
)
|
||||
config_command(args)
|
||||
saved = _read_config(_isolated_hermes_home)
|
||||
assert "glm-5.3-flash" not in saved.get("providers", {}).get("Bai", {}).get(
|
||||
"models", {}
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Backward compatibility: plain dotted paths with no dotted-key collision
|
||||
# split exactly as before.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestBackwardCompatibility:
|
||||
def test_plain_nested_set(self, _isolated_hermes_home):
|
||||
set_config_value("terminal.backend", "docker")
|
||||
saved = _read_config(_isolated_hermes_home)
|
||||
assert saved["terminal"]["backend"] == "docker"
|
||||
|
||||
def test_plain_nested_get_and_unset_helpers(self):
|
||||
cfg = {"a": {"b": {"c": 1}}}
|
||||
assert _get_nested(cfg, "a.b.c") == 1
|
||||
assert _unset_nested(cfg, "a.b.c") is True
|
||||
assert _get_nested(cfg, "a.b.c") is _MISSING
|
||||
|
||||
def test_list_index_navigation_unchanged(self):
|
||||
cfg = {"custom_providers": [{"name": "p1"}]}
|
||||
_set_nested(cfg, "custom_providers.0.name", "p2")
|
||||
assert cfg["custom_providers"][0]["name"] == "p2"
|
||||
|
||||
def test_deep_creation_without_siblings_unchanged(self, _isolated_hermes_home):
|
||||
set_config_value("agent.max_iterations", "50")
|
||||
saved = _read_config(_isolated_hermes_home)
|
||||
assert saved["agent"]["max_iterations"] == 50
|
||||
|
||||
def test_greedy_never_beats_exact_nested_structure(self):
|
||||
"""A literal dotted key never shadows the plain-split path when the
|
||||
plain path ALSO fully exists — longest literal match only consumes
|
||||
segments when the literal dotted key is present; nested dicts still
|
||||
resolve segment-by-segment."""
|
||||
cfg = {"a": {"b": {"c": 1}}}
|
||||
assert _get_nested(cfg, "a.b.c") == 1
|
||||
_set_nested(cfg, "a.b.c", 2)
|
||||
assert cfg == {"a": {"b": {"c": 2}}}
|
||||
@@ -491,28 +491,9 @@ class TestSecretRedactionInDisplay:
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# `config get` redacts resolved credentials (issue #84106)
|
||||
# #34067: Schema validation for unknown keys
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
_MCP_SECRETS_CONFIG = """\
|
||||
config_version: 34
|
||||
security:
|
||||
redact_secrets: true
|
||||
mcp_servers:
|
||||
demo_url:
|
||||
url: "https://example.invalid/mcp?apikey=${FMP_API_KEY}"
|
||||
demo_env:
|
||||
command: demo
|
||||
env:
|
||||
COINGECKO_PRO_API_KEY: "${COINGECKO_PRO_API_KEY}"
|
||||
demo_header:
|
||||
url: "https://example.invalid/mcp"
|
||||
headers:
|
||||
Authorization: "Bearer ${TEST_BEARER_TOKEN}"
|
||||
"""
|
||||
|
||||
|
||||
|
||||
class TestSchemaValidation:
|
||||
"""#34067: ``hermes config set`` must not report bare success for
|
||||
unrecognized keys. The key IS written (arbitrary keys are supported —
|
||||
@@ -849,7 +830,9 @@ class TestLiteralDotKeyEscaping:
|
||||
assert "qwen3" not in providers
|
||||
target = providers["qwen3.5-397b-wafer-non-zdr"]
|
||||
assert target["api"] == "https://pass.wafer.ai/v1"
|
||||
assert target["extra_headers"] == '{"Wafer-ZDR": "required"}'
|
||||
# Current main coerces structured-looking values to real mappings
|
||||
# (_looks_structured_value), so the JSON string lands as a dict.
|
||||
assert target["extra_headers"] == {"Wafer-ZDR": "required"}
|
||||
# Sibling provider untouched.
|
||||
assert providers["openrouter"] == {"api_key": "or-keep"}
|
||||
# Escaped key is schema-known (providers.* is an open dict) — no warning.
|
||||
|
||||
@@ -587,14 +587,30 @@ def atomic_roundtrip_yaml_update(
|
||||
config = CommentedMap(config)
|
||||
|
||||
current = config
|
||||
keys = key_path.split(".")
|
||||
for key in keys[:-1]:
|
||||
next_value = current.get(key)
|
||||
# Honor escaped dots and prefer existing literal dotted keys (e.g. model
|
||||
# IDs like ``glm-5.3``) over blind splitting — same navigation as
|
||||
# ``hermes config set``'s ``_set_nested`` (#91607: /model + TUI
|
||||
# persistence route through here and used to write ``glm-5: {'3': ...}``
|
||||
# phantom siblings while the runtime kept reading the literal key).
|
||||
from hermes_cli.config import _greedy_literal_match, _split_key_path
|
||||
|
||||
keys = _split_key_path(key_path)
|
||||
i = 0
|
||||
while True:
|
||||
remaining = keys[i:]
|
||||
seg, consumed = remaining[0], 1
|
||||
match = _greedy_literal_match(dict(current), remaining)
|
||||
if match is not None:
|
||||
seg, consumed = match
|
||||
if i + consumed == len(keys):
|
||||
current[seg] = value
|
||||
break
|
||||
next_value = current.get(seg)
|
||||
if not isinstance(next_value, CommentedMap):
|
||||
next_value = CommentedMap()
|
||||
current[key] = next_value
|
||||
current[seg] = next_value
|
||||
current = next_value
|
||||
current[keys[-1]] = value
|
||||
i += consumed
|
||||
|
||||
original_mode = _preserve_file_mode(path)
|
||||
original_owner = _preserve_file_owner(path)
|
||||
|
||||
@@ -1158,6 +1158,27 @@ Subcommands:
|
||||
| `check` | Check for missing or stale config. |
|
||||
| `migrate` | Add newly introduced options interactively. |
|
||||
|
||||
### Dots inside key names
|
||||
|
||||
`hermes config set/get/unset` use `.` as the nesting separator, but many real
|
||||
key names contain literal dots — model IDs (`grok-4.6`, `glm-5.3-flash`),
|
||||
Matrix room IDs (`!room:example.org`), versioned provider names. Two rules
|
||||
make these addressable:
|
||||
|
||||
- **Existing keys just work.** When navigating an existing mapping, an
|
||||
existing literal key that matches the dotted remainder is preferred over
|
||||
splitting. `hermes config set providers.p.models.grok-4.6.supports_vision true`
|
||||
updates the real `grok-4.6` entry (and `get`/`unset` resolve the same way).
|
||||
- **Creating a new dotted key requires escaping.** Escape literal dots with a
|
||||
backslash: `hermes config set 'providers.p.models.grok-4\.7.context_length' 128000`
|
||||
creates the literal `grok-4.7` key. (Quote the key so your shell keeps the
|
||||
backslash.)
|
||||
|
||||
If an unescaped write would create a nested mapping that shadows an existing
|
||||
dotted sibling (e.g. creating `grok-4` next to an existing `grok-4.6`), the
|
||||
command fails with an error instead of silently writing a phantom entry the
|
||||
runtime would never read.
|
||||
|
||||
## `hermes pairing`
|
||||
|
||||
```bash
|
||||
|
||||
Reference in New Issue
Block a user