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.