fix: config get/unset still reach a legacy gateway.platforms-only value (review follow-up)
The gateway.platforms.<p>.<field> -> platforms.<p>.<field> canonicalization was applied to `config get` and `config unset` too, so a legacy config whose only value lived under the nested block reported "Config key not set" (get) and left the nested key in place (unset) while merge_platform_sections kept honouring it. get now falls back to the nested key when the canonical one is absent, unset removes both spellings, and set drops the shadowed nested duplicate so the file keeps one source of truth.
This commit is contained in:
@@ -3408,6 +3408,17 @@ def _redirect_platform_display_key(key: str) -> tuple[str, Optional[str]]:
|
||||
return canonical, f" (note: per-platform display setting — saved as {canonical})"
|
||||
|
||||
|
||||
def _legacy_gateway_platforms_key(requested_key: str) -> Optional[str]:
|
||||
"""The ``gateway.platforms.<name>.<field>`` spelling the user typed, when that is what they typed.
|
||||
``merge_platform_sections`` still honours a value that lives only there, so ``get`` must fall
|
||||
back to it and ``unset``/``set`` must clear it, or the CLI reports "not set" / writes a value
|
||||
while the gateway keeps reading the nested one."""
|
||||
segs = _split_key_path(requested_key)
|
||||
if len(segs) >= 3 and segs[0] == "gateway" and segs[1] == "platforms":
|
||||
return ".".join(segs)
|
||||
return None
|
||||
|
||||
|
||||
def _exit_if_key_managed(key: str, action: str) -> None:
|
||||
"""A key pinned by the managed layer cannot be set/unset (the next load would reinstate it):
|
||||
hard-reject and name the source. Distinct from ``is_managed()``; env-shaped keys route to the
|
||||
@@ -3546,6 +3557,7 @@ def set_config_value(key: str, value: str, force: bool = False):
|
||||
|
||||
# Canonicalize per-platform display keys BEFORE validation/coercion so both see the path the
|
||||
# runtime reads.
|
||||
legacy_key = _legacy_gateway_platforms_key(key)
|
||||
key, _redirect_note = _redirect_platform_display_key(key)
|
||||
if _redirect_note:
|
||||
print(_redirect_note)
|
||||
@@ -3572,6 +3584,8 @@ def set_config_value(key: str, value: str, force: bool = False):
|
||||
_set_nested(user_config, key, value)
|
||||
except ValueError as e:
|
||||
_exit_invalid(f"✗ {e}")
|
||||
if legacy_key and _unset_nested(user_config, legacy_key):
|
||||
print(f" (removed the shadowed {legacy_key} duplicate)")
|
||||
# A provider switch re-points ``model:`` at a new route; ``base_url``/``api_mode`` are route
|
||||
# state of the OLD provider, and the runtime honours them for whatever provider the block now
|
||||
# names — the new provider's key would be posted to the old endpoint (#113719, #40862). Sync
|
||||
@@ -3640,8 +3654,12 @@ def get_config_value(key: str, *, as_json: bool = False, raw: bool = False):
|
||||
else:
|
||||
# Mirror set_config_value: read the canonical display.platforms path.
|
||||
# See #71047.
|
||||
legacy_key = _legacy_gateway_platforms_key(key)
|
||||
key, _ = _redirect_platform_display_key(key)
|
||||
value = _get_nested(load_config(), key)
|
||||
config = load_config()
|
||||
value = _get_nested(config, key)
|
||||
if value is _MISSING and legacy_key:
|
||||
value = _get_nested(config, legacy_key)
|
||||
|
||||
if value is _MISSING:
|
||||
_exit_invalid(f"Config key not set: {key}")
|
||||
@@ -3703,11 +3721,14 @@ def unset_config_value(key: str):
|
||||
config_path = get_config_path()
|
||||
user_config = require_readable_config_before_write(config_path)
|
||||
|
||||
legacy_key = _legacy_gateway_platforms_key(key)
|
||||
key, _redirect_note = _redirect_platform_display_key(key)
|
||||
if _redirect_note:
|
||||
# Mirror set_config_value's display.platforms canonicalization (#71047).
|
||||
print(_redirect_note.replace("saved as", "resolved as"))
|
||||
removed = _unset_nested(user_config, key)
|
||||
if legacy_key:
|
||||
removed = _unset_nested(user_config, legacy_key) or removed
|
||||
|
||||
env_var = terminal_config_env_var_for_key(key)
|
||||
if env_var and key != "terminal.cwd":
|
||||
|
||||
@@ -110,6 +110,26 @@ class TestGatewayPlatformsPrefixRedirect:
|
||||
key, _ = _redirect_platform_display_key("gateway.platforms.telegram.streaming")
|
||||
assert key == "display.platforms.telegram.streaming"
|
||||
|
||||
def test_get_and_unset_still_reach_a_legacy_nested_only_value(self, _isolated_hermes_home, capsys):
|
||||
"""A config whose value lives ONLY under ``gateway.platforms`` is still honoured by the gateway
|
||||
(``merge_platform_sections``), so ``get`` must read it and ``unset`` must remove it instead of
|
||||
reporting "not set" while the gateway keeps the platform enabled."""
|
||||
from hermes_cli.config import get_config_value, unset_config_value
|
||||
|
||||
legacy = "gateway:\n platforms:\n telegram:\n enabled: true\n"
|
||||
(_isolated_hermes_home / "config.yaml").write_text(legacy, encoding="utf-8")
|
||||
get_config_value("gateway.platforms.telegram.enabled")
|
||||
assert capsys.readouterr().out.strip().lower() == "true"
|
||||
|
||||
unset_config_value("gateway.platforms.telegram.enabled")
|
||||
assert "gateway" not in (yaml.safe_load(_read_config(_isolated_hermes_home)) or {})
|
||||
|
||||
# set on top of a nested-only value leaves one source of truth, not a shadowed duplicate
|
||||
(_isolated_hermes_home / "config.yaml").write_text(legacy, encoding="utf-8")
|
||||
set_config_value("gateway.platforms.telegram.enabled", "false")
|
||||
loaded = yaml.safe_load(_read_config(_isolated_hermes_home))
|
||||
assert loaded == {"platforms": {"telegram": {"enabled": False}}}
|
||||
|
||||
|
||||
class TestConfigYamlRouting:
|
||||
"""Regular config keys should go to config.yaml, NOT .env."""
|
||||
|
||||
Reference in New Issue
Block a user