From 9ed9af2f7d5c8db93da721b3c9efd00ed0e02cc0 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sat, 30 May 2026 20:42:37 -0700 Subject: [PATCH] fix(update): name new config options in migration prompt; skip prompt for pure version bumps (#35658) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 'hermes update' config-migration prompt printed only counts ('1 new config option available') then asked 'configure them now?' without ever saying what the options were. Users said no because they couldn't tell what they were agreeing to. For pure config-format version bumps (no new env/config keys) it still asked the question, where saying yes just bumped the version and looked like a no-op. - List each new env var / config key by name + description before prompting (cap at 8, then '… and N more'). The data was already available; we just threw it away and printed a count. - Pure version bump (no new options): apply the format migration non-interactively and print what happened, instead of asking a misleading yes/no. Reported by ScottFive and Tt2021. --- hermes_cli/main.py | 49 +++++++++++++++++- tests/hermes_cli/test_cmd_update.py | 77 +++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 2 deletions(-) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 27105c570..1cb4bd3d6 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -9557,16 +9557,61 @@ def _cmd_update_impl(args, gateway_mode: bool): missing_config = get_missing_config_fields() current_ver, latest_ver = check_config_version() - needs_migration = missing_env or missing_config or current_ver < latest_ver + has_new_options = bool(missing_env or missing_config) + version_bump_only = ( + not has_new_options and current_ver < latest_ver + ) + needs_migration = has_new_options or current_ver < latest_ver - if needs_migration: + if version_bump_only: + # Nothing for the user to fill in — only the config format version + # changed (new defaults already merge in transparently). Asking + # "configure new options now?" here is misleading: saying yes just + # bumps the version and looks like a no-op (issue: ScottFive / + # Tt2021). Apply it silently and say what actually happened. print() + print( + f" ℹ Updating config format (v{current_ver} → v{latest_ver})…" + ) + try: + migrate_config(interactive=False, quiet=True) + print(" ✓ Config format updated (no new settings to configure)") + except Exception as _mig_err: + print(f" ⚠️ Config format update failed: {_mig_err}") + print(" Run 'hermes config migrate' to retry.") + elif needs_migration: + print() + # Show WHAT changed, not just a count, so the user can make an + # informed yes/no decision (previously the prompt named nothing). + def _print_items(items, label, key, fallback_key=None): + if not items: + return + print(f" {label}:") + shown = items[:8] + for it in shown: + if isinstance(it, dict): + name = it.get(key) or (fallback_key and it.get(fallback_key)) or "?" + desc = (it.get("description") or "").strip() + else: + # Defensive: some callers/mocks pass bare name strings. + name = str(it) + desc = "" + if desc: + print(f" • {name} — {desc}") + else: + print(f" • {name}") + extra = len(items) - len(shown) + if extra > 0: + print(f" … and {extra} more") + if missing_env: print( f" ⚠️ {len(missing_env)} new required setting(s) need configuration" ) + _print_items(missing_env, "New settings", "name") if missing_config: print(f" ℹ️ {len(missing_config)} new config option(s) available") + _print_items(missing_config, "New options", "key") print() if assume_yes: diff --git a/tests/hermes_cli/test_cmd_update.py b/tests/hermes_cli/test_cmd_update.py index ed9033ffc..0aeb8e408 100644 --- a/tests/hermes_cli/test_cmd_update.py +++ b/tests/hermes_cli/test_cmd_update.py @@ -281,6 +281,83 @@ class TestCmdUpdateBranchFallback: assert "API keys require manual entry" in captured.out +class TestCmdUpdateMigrationPrompt: + """The config-migration prompt names what changed and skips the prompt + entirely when only the config format version moved. + + Regression guard for the contentless-prompt report (ScottFive / Tt2021): + previously the prompt printed only counts ("1 new config option") and + asked "configure them now?" even for pure version bumps, where saying + yes looked like a no-op. + """ + + def test_version_bump_only_applies_silently_without_prompt( + self, mock_args, capsys + ): + """Only the version moved → apply non-interactively, never prompt.""" + with patch("shutil.which", return_value=None), patch( + "subprocess.run" + ) as mock_run, patch("builtins.input") as mock_input, patch( + "hermes_cli.config.get_missing_env_vars", return_value=[] + ), patch( + "hermes_cli.config.get_missing_config_fields", return_value=[] + ), patch( + "hermes_cli.config.check_config_version", return_value=(5, 24) + ), patch( + "hermes_cli.config.migrate_config", + return_value={"env_added": [], "config_added": [], "warnings": []}, + ) as mock_migrate: + mock_run.side_effect = _make_run_side_effect( + branch="main", verify_ok=True, commit_count="1" + ) + + cmd_update(mock_args) + + mock_input.assert_not_called() + mock_migrate.assert_called_once_with(interactive=False, quiet=True) + out = capsys.readouterr().out + assert "Updating config format (v5 → v24)" in out + assert "no new settings to configure" in out + # The misleading question must NOT appear for a pure version bump. + assert "configure them now" not in out.lower() + + def test_new_options_are_listed_by_name_before_prompt( + self, mock_args, capsys + ): + """New env/config keys are printed by name so the user can decide.""" + env_items = [ + {"name": "FOO_API_KEY", "description": "Foo service API key"}, + ] + cfg_items = [ + {"key": "display.new_widget", "description": "New config option: display.new_widget"}, + ] + with patch("shutil.which", return_value=None), patch( + "subprocess.run" + ) as mock_run, patch("builtins.input", return_value="n"), patch( + "hermes_cli.config.get_missing_env_vars", return_value=env_items + ), patch( + "hermes_cli.config.get_missing_config_fields", return_value=cfg_items + ), patch( + "hermes_cli.config.check_config_version", return_value=(1, 24) + ), patch( + "hermes_cli.config.migrate_config", + return_value={"env_added": [], "config_added": [], "warnings": []}, + ), patch("hermes_cli.main.sys") as mock_sys: + mock_sys.stdin.isatty.return_value = True + mock_sys.stdout.isatty.return_value = True + mock_run.side_effect = _make_run_side_effect( + branch="main", verify_ok=True, commit_count="1" + ) + + cmd_update(mock_args) + + out = capsys.readouterr().out + # Names, not just counts. + assert "FOO_API_KEY" in out + assert "Foo service API key" in out + assert "display.new_widget" in out + + class TestCmdUpdateProfileSkillSync: """cmd_update syncs bundled skills to all profiles, including the active one.