diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 04e4887e645f..762dfd9a0191 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -8724,9 +8724,9 @@ def set_config_value(key: str, value: str, force: bool = False): Args: key: Dotted config path (e.g. ``terminal.backend``). value: String value (auto-coerced to bool/int/float when matching). - force: When True, skip schema validation — useful for setting keys - that a newer Hermes version added but this one doesn't know about - yet. The CLI exposes this via ``hermes config set --force``. + force: When True, skip the unknown-key warning — useful for scripted + writes of keys the running version doesn't recognize yet. The CLI + exposes this via ``hermes config set --force``. """ if is_managed(): managed_error("set configuration values") @@ -8757,28 +8757,14 @@ def set_config_value(key: str, value: str, force: bool = False): print(f"✓ Set {key} in {get_env_path()}") return - # Schema validation (#34067): refuse to silently write unknown top-level - # keys into config.yaml. The previous behavior accepted arbitrary key - # paths (e.g. ``gateway.discord.gateway_restart_notification`` instead of - # the correct ``discord.gateway_restart_notification``), reported success, - # and left the user debugging behavior that hadn't actually changed. + # Unknown-key notice (#34067): the key is still written (arbitrary keys + # are supported — top-level scalars are bridged into os.environ for + # skills and external apps), but a plausible-but-wrong dotted path like + # ``gateway.discord.gateway_restart_notification`` previously reported + # bare success and left the user debugging behavior that never changed. + # Warn after the write so the user gets immediate feedback plus a + # "did you mean" hint, without blocking legitimate unknown keys. is_known, suggestion = _validate_config_key(key) - if not is_known and not force: - print(color(f"✗ Unknown config key: {key}", Colors.RED, Colors.BOLD)) - if suggestion: - print(color(f" Did you mean: {suggestion}", Colors.YELLOW)) - else: - print(color( - " No close match found in the known config schema.", - Colors.YELLOW, - )) - print() - print( - " If this is a key your version of Hermes doesn't know about yet " - "but a newer version does, bypass this check with:\n" - f" hermes config set --force {key} {value}" - ) - sys.exit(2) # Otherwise it goes to config.yaml # Read the raw user config (not merged with defaults) to avoid @@ -8846,6 +8832,23 @@ def set_config_value(key: str, value: str, force: bool = False): _display_value = value print(f"✓ Set {key} = {_display_value} in {config_path}") + # Post-write unknown-key notice (#34067): value IS saved, but tell the + # user the runtime may never read it and suggest the likely-intended path. + if not is_known and not force: + print(color( + f"⚠ '{key}' is not a recognized config key — it was saved anyway, " + "but Hermes may not read it.", + Colors.YELLOW, + )) + if suggestion: + print(color(f" Did you mean: {suggestion}", Colors.YELLOW)) + print(color( + " (Custom top-level keys are supported and bridged to the " + "environment for skills/external tools. Use --force to skip " + "this notice.)", + Colors.DIM, + )) + def get_config_value(key: str, *, as_json: bool = False): """Print a resolved configuration value.""" @@ -8958,7 +8961,7 @@ def config_command(args): print(" hermes config set terminal.backend docker") print(" hermes config set OPENROUTER_API_KEY sk-or-...") print() - print(" --force: bypass schema validation for unknown keys") + print(" --force: skip the unknown-key notice for unrecognized keys") sys.exit(1) set_config_value(key, value, force=force) diff --git a/hermes_cli/subcommands/config.py b/hermes_cli/subcommands/config.py index 00bc47961100..8fe1297407f4 100644 --- a/hermes_cli/subcommands/config.py +++ b/hermes_cli/subcommands/config.py @@ -43,9 +43,8 @@ def build_config_parser(subparsers, *, cmd_config: Callable) -> None: config_set.add_argument( "--force", action="store_true", - help="Bypass schema validation (write unknown keys without warning). " - "Use this when setting a key that a newer Hermes version supports " - "but the running version doesn't recognize yet.", + help="Skip the unknown-key notice printed after writing a key the " + "running version doesn't recognize (the value is saved either way).", ) # config unset diff --git a/tests/hermes_cli/test_set_config_value.py b/tests/hermes_cli/test_set_config_value.py index 8770483eedfe..ad9641300df5 100644 --- a/tests/hermes_cli/test_set_config_value.py +++ b/tests/hermes_cli/test_set_config_value.py @@ -469,86 +469,85 @@ class TestSecretRedactionInDisplay: # --------------------------------------------------------------------------- class TestSchemaValidation: - """#34067: ``hermes config set`` must refuse to silently write unknown - keys to config.yaml. The headline case in the issue was - ``gateway.discord.gateway_restart_notification`` being silently accepted - even though the correct path is ``discord.gateway_restart_notification`` - (platform configs live at the top level, not nested under ``gateway``). + """#34067: ``hermes config set`` must not report bare success for + unrecognized keys. The key IS written (arbitrary keys are supported — + top-level scalars bridge into os.environ for skills/external apps), but + a post-write notice warns that Hermes may never read it and suggests the + likely-intended path. Headline case: the plausible-but-wrong + ``gateway.discord.gateway_restart_notification`` (correct path: + ``discord.gateway_restart_notification``). """ - def test_unknown_top_level_key_is_refused(self, _isolated_hermes_home, capsys): - """An entirely-unknown top-level key triggers SystemExit(2) with a - red error message.""" - with pytest.raises(SystemExit) as exc_info: - set_config_value("totally_made_up_key", "value") - assert exc_info.value.code == 2 + def test_unknown_top_level_key_written_with_notice(self, _isolated_hermes_home, capsys): + """An unknown top-level key is saved AND a notice is printed.""" + set_config_value("totally_made_up_key", "value") out = capsys.readouterr().out - assert "Unknown config key" in out + assert "not a recognized config key" in out assert "totally_made_up_key" in out - # And nothing was written to config.yaml. - assert "totally_made_up_key" not in _read_config(_isolated_hermes_home) + assert "saved anyway" in out + # The value WAS written. + assert "totally_made_up_key" in _read_config(_isolated_hermes_home) - def test_unknown_subkey_is_refused(self, _isolated_hermes_home, capsys): - """The headline #34067 bug: ``gateway`` is a valid top-level key but - ``gateway.discord`` is not a valid sub-key (gateway only has - strict/media_delivery_allow_dirs/trust_recent_files/...).""" - with pytest.raises(SystemExit) as exc_info: - set_config_value("gateway.discord.gateway_restart_notification", "false") - assert exc_info.value.code == 2 + def test_unknown_subkey_written_with_notice(self, _isolated_hermes_home, capsys): + """The headline #34067 path: written, but warned about — no more + bare success for gateway.discord.gateway_restart_notification.""" + set_config_value("gateway.discord.gateway_restart_notification", "false") out = capsys.readouterr().out - assert "Unknown config key" in out - assert "gateway.discord.gateway_restart_notification" in out - # Nothing was written. - assert "discord" not in _read_config(_isolated_hermes_home).split("gateway:")[-1] if "gateway:" in _read_config(_isolated_hermes_home) else True + assert "✓ Set" in out + assert "not a recognized config key" in out - def test_platforms_container_is_accepted(self, _isolated_hermes_home): + def test_platforms_container_is_accepted(self, _isolated_hermes_home, capsys): """``platforms..`` is a valid current shape: gateway/ config.py resolves a top-level ``platforms`` map in addition to the - top-level platform blocks, so it must NOT be refused.""" + top-level platform blocks, so it must NOT trigger the notice.""" set_config_value("platforms.discord.enabled", "true") content = _read_config(_isolated_hermes_home) assert "enabled: true" in content + assert "not a recognized config key" not in capsys.readouterr().out - def test_gateway_platforms_nested_is_accepted(self, _isolated_hermes_home): + def test_gateway_platforms_nested_is_accepted(self, _isolated_hermes_home, capsys): """Docs configure platforms under ``gateway.platforms.`` — the - canonical layout must validate as known.""" + canonical layout must validate as known (no notice).""" set_config_value("gateway.platforms.my_platform.extra.token", "abc") content = _read_config(_isolated_hermes_home) assert "token: abc" in content + assert "not a recognized config key" not in capsys.readouterr().out - def test_unknown_approvals_subkey_is_refused(self, _isolated_hermes_home, capsys): - """``approvals`` is a defined schema, so a typo'd sub-key must be - rejected rather than silently written.""" - with pytest.raises(SystemExit): - set_config_value("approvals.notarealkey", "true") + def test_unknown_approvals_subkey_warns_but_writes(self, _isolated_hermes_home, capsys): + """``approvals`` is a defined schema, so a typo'd sub-key gets the + notice — but is still written.""" + set_config_value("approvals.notarealkey", "true") + out = capsys.readouterr().out + assert "not a recognized config key" in out + assert "notarealkey" in _read_config(_isolated_hermes_home) - def test_known_approvals_subkey_is_accepted(self, _isolated_hermes_home): - """Real ``approvals.*`` keys still validate.""" + def test_known_approvals_subkey_is_accepted(self, _isolated_hermes_home, capsys): + """Real ``approvals.*`` keys still validate silently.""" set_config_value("approvals.mode", "off") import yaml saved = yaml.safe_load(_read_config(_isolated_hermes_home)) assert saved["approvals"]["mode"] == "off" + assert "not a recognized config key" not in capsys.readouterr().out def test_close_typo_suggests_correct_key(self, _isolated_hermes_home, capsys): """Typo'd top-level keys should get a fuzzy-match suggestion.""" - with pytest.raises(SystemExit): - set_config_value("disco", "false") + set_config_value("disco", "false") out = capsys.readouterr().out assert "Did you mean" in out assert "discord" in out def test_typoed_subkey_suggests_sibling(self, _isolated_hermes_home, capsys): """``agent.max_turn`` should suggest ``agent.max_turns``.""" - with pytest.raises(SystemExit): - set_config_value("agent.max_turn", "100") + set_config_value("agent.max_turn", "100") out = capsys.readouterr().out assert "agent.max_turns" in out - def test_force_bypasses_validation(self, _isolated_hermes_home): - """``--force`` allows writing unknown keys (forward-compat with - config keys that a newer Hermes version adds).""" - # Should not raise. + def test_force_suppresses_notice(self, _isolated_hermes_home, capsys): + """``--force`` writes unknown keys without the notice (scripted + forward-compat writes).""" set_config_value("brand_new_future_key", "value", force=True) + out = capsys.readouterr().out + assert "not a recognized config key" not in out # And the value WAS written. content = _read_config(_isolated_hermes_home) assert "brand_new_future_key" in content