From bdccdd67a1c3f16aa4d15f700a9615bbc4d141f7 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Thu, 26 Mar 2026 16:29:38 -0700 Subject: [PATCH] fix: OpenClaw migration overwrites defaults and setup wizard skips imported sections (#3282) Two bugs caused the OpenClaw migration during first-time setup to be ineffective, forcing users to reconfigure everything manually: 1. The setup wizard created config.yaml with all defaults BEFORE running the migration, then the migrator ran with overwrite=False. Every config setting was reported as a 'conflict' against the defaults and skipped. Fix: use overwrite=True during setup-time migration (safe because only defaults exist at that point). The hermes claw migrate CLI command still defaults to overwrite=False for post-setup use. 2. After migration, the full setup wizard ran all 5 sections unconditionally, forcing the user through model/terminal/agent/messaging/tools configuration even when those settings were just imported. Fix: add _get_section_config_summary() and _skip_configured_section() helpers. After migration, each section checks if it's already configured (API keys present, non-default values, platform tokens) and offers 'Reconfigure? [y/N]' with default No. Unconfigured sections still run normally. Reported by Dev Bredda on social media. --- hermes_cli/setup.py | 117 ++++++++++- .../test_setup_openclaw_migration.py | 181 +++++++++++++++++- 2 files changed, 290 insertions(+), 8 deletions(-) diff --git a/hermes_cli/setup.py b/hermes_cli/setup.py index 54ecbf165..fadccb58e 100644 --- a/hermes_cli/setup.py +++ b/hermes_cli/setup.py @@ -2968,6 +2968,95 @@ def setup_tools(config: dict, first_install: bool = False): tools_command(first_install=first_install, config=config) +# ============================================================================= +# Post-Migration Section Skip Logic +# ============================================================================= + + +def _get_section_config_summary(config: dict, section_key: str) -> Optional[str]: + """Return a short summary if a setup section is already configured, else None. + + Used after OpenClaw migration to detect which sections can be skipped. + ``get_env_value`` is the module-level import from hermes_cli.config + so that test patches on ``setup_mod.get_env_value`` take effect. + """ + if section_key == "model": + has_key = bool( + get_env_value("OPENROUTER_API_KEY") + or get_env_value("OPENAI_API_KEY") + or get_env_value("ANTHROPIC_API_KEY") + ) + if not has_key: + # Check for OAuth providers + try: + from hermes_cli.auth import get_active_provider + if get_active_provider(): + has_key = True + except Exception: + pass + if not has_key: + return None + model = config.get("model") + if isinstance(model, str) and model.strip(): + return model.strip() + if isinstance(model, dict): + return str(model.get("default") or model.get("model") or "configured") + return "configured" + + elif section_key == "terminal": + backend = config.get("terminal", {}).get("backend", "local") + return f"backend: {backend}" + + elif section_key == "agent": + max_turns = config.get("agent", {}).get("max_turns", 90) + return f"max turns: {max_turns}" + + elif section_key == "gateway": + platforms = [] + if get_env_value("TELEGRAM_BOT_TOKEN"): + platforms.append("Telegram") + if get_env_value("DISCORD_BOT_TOKEN"): + platforms.append("Discord") + if get_env_value("SLACK_BOT_TOKEN"): + platforms.append("Slack") + if get_env_value("WHATSAPP_PHONE_NUMBER_ID"): + platforms.append("WhatsApp") + if get_env_value("SIGNAL_ACCOUNT"): + platforms.append("Signal") + if platforms: + return ", ".join(platforms) + return None # No platforms configured — section must run + + elif section_key == "tools": + tools = [] + if get_env_value("ELEVENLABS_API_KEY"): + tools.append("TTS/ElevenLabs") + if get_env_value("BROWSERBASE_API_KEY"): + tools.append("Browser") + if get_env_value("FIRECRAWL_API_KEY"): + tools.append("Firecrawl") + if tools: + return ", ".join(tools) + return None + + return None + + +def _skip_configured_section( + config: dict, section_key: str, label: str +) -> bool: + """Show an already-configured section summary and offer to skip. + + Returns True if the user chose to skip, False if the section should run. + """ + summary = _get_section_config_summary(config, section_key) + if not summary: + return False + print() + print_success(f" {label}: {summary}") + return not prompt_yes_no(f" Reconfigure {label.lower()}?", default=False) + + # ============================================================================= # OpenClaw Migration # ============================================================================= @@ -3039,7 +3128,7 @@ def _offer_openclaw_migration(hermes_home: Path) -> bool: target_root=hermes_home.resolve(), execute=True, workspace_target=None, - overwrite=False, + overwrite=True, migrate_secrets=True, output_dir=None, selected_options=selected, @@ -3195,6 +3284,8 @@ def run_setup_wizard(args): ) ) + migration_ran = False + if is_existing: # ── Returning User Menu ── print() @@ -3264,7 +3355,8 @@ def run_setup_wizard(args): return # Offer OpenClaw migration before configuration begins - if _offer_openclaw_migration(hermes_home): + migration_ran = _offer_openclaw_migration(hermes_home) + if migration_ran: # Reload config in case migration wrote to it config = load_config() @@ -3277,20 +3369,31 @@ def run_setup_wizard(args): print() print_info("You can edit these files directly or use 'hermes config edit'") + if migration_ran: + print() + print_info("Settings were imported from OpenClaw.") + print_info("Each section below will show what was imported — press Enter to keep,") + print_info("or choose to reconfigure if needed.") + # Section 1: Model & Provider - setup_model_provider(config) + if not (migration_ran and _skip_configured_section(config, "model", "Model & Provider")): + setup_model_provider(config) # Section 2: Terminal Backend - setup_terminal_backend(config) + if not (migration_ran and _skip_configured_section(config, "terminal", "Terminal Backend")): + setup_terminal_backend(config) # Section 3: Agent Settings - setup_agent_settings(config) + if not (migration_ran and _skip_configured_section(config, "agent", "Agent Settings")): + setup_agent_settings(config) # Section 4: Messaging Platforms - setup_gateway(config) + if not (migration_ran and _skip_configured_section(config, "gateway", "Messaging Platforms")): + setup_gateway(config) # Section 5: Tools - setup_tools(config, first_install=not is_existing) + if not (migration_ran and _skip_configured_section(config, "tools", "Tools")): + setup_tools(config, first_install=not is_existing) # Save and show summary save_config(config) diff --git a/tests/hermes_cli/test_setup_openclaw_migration.py b/tests/hermes_cli/test_setup_openclaw_migration.py index be5d61bab..0991b6d1b 100644 --- a/tests/hermes_cli/test_setup_openclaw_migration.py +++ b/tests/hermes_cli/test_setup_openclaw_migration.py @@ -94,7 +94,7 @@ class TestOfferOpenclawMigration: fake_mod.Migrator.assert_called_once() call_kwargs = fake_mod.Migrator.call_args[1] assert call_kwargs["execute"] is True - assert call_kwargs["overwrite"] is False + assert call_kwargs["overwrite"] is True assert call_kwargs["migrate_secrets"] is True assert call_kwargs["preset_name"] == "full" fake_migrator.migrate.assert_called_once() @@ -285,3 +285,182 @@ class TestSetupWizardOpenclawIntegration: setup_mod.run_setup_wizard(args) mock_migration.assert_not_called() + + +# --------------------------------------------------------------------------- +# _get_section_config_summary / _skip_configured_section — unit tests +# --------------------------------------------------------------------------- + + +class TestGetSectionConfigSummary: + """Test the _get_section_config_summary helper.""" + + def test_model_returns_none_without_api_key(self): + with patch.object(setup_mod, "get_env_value", return_value=""): + result = setup_mod._get_section_config_summary({}, "model") + assert result is None + + def test_model_returns_summary_with_api_key(self): + def env_side(key): + return "sk-xxx" if key == "OPENROUTER_API_KEY" else "" + + with patch.object(setup_mod, "get_env_value", side_effect=env_side): + result = setup_mod._get_section_config_summary( + {"model": "openai/gpt-4"}, "model" + ) + assert result == "openai/gpt-4" + + def test_model_returns_dict_default_key(self): + def env_side(key): + return "sk-xxx" if key == "OPENAI_API_KEY" else "" + + with patch.object(setup_mod, "get_env_value", side_effect=env_side): + result = setup_mod._get_section_config_summary( + {"model": {"default": "claude-opus-4", "provider": "anthropic"}}, + "model", + ) + assert result == "claude-opus-4" + + def test_terminal_always_returns(self): + with patch.object(setup_mod, "get_env_value", return_value=""): + result = setup_mod._get_section_config_summary( + {"terminal": {"backend": "docker"}}, "terminal" + ) + assert result == "backend: docker" + + def test_agent_always_returns(self): + with patch.object(setup_mod, "get_env_value", return_value=""): + result = setup_mod._get_section_config_summary( + {"agent": {"max_turns": 120}}, "agent" + ) + assert result == "max turns: 120" + + def test_gateway_returns_none_without_tokens(self): + with patch.object(setup_mod, "get_env_value", return_value=""): + result = setup_mod._get_section_config_summary({}, "gateway") + assert result is None + + def test_gateway_lists_platforms(self): + def env_side(key): + if key == "TELEGRAM_BOT_TOKEN": + return "tok123" + if key == "DISCORD_BOT_TOKEN": + return "disc456" + return "" + + with patch.object(setup_mod, "get_env_value", side_effect=env_side): + result = setup_mod._get_section_config_summary({}, "gateway") + assert "Telegram" in result + assert "Discord" in result + + def test_tools_returns_none_without_keys(self): + with patch.object(setup_mod, "get_env_value", return_value=""): + result = setup_mod._get_section_config_summary({}, "tools") + assert result is None + + def test_tools_lists_configured(self): + def env_side(key): + return "key" if key == "BROWSERBASE_API_KEY" else "" + + with patch.object(setup_mod, "get_env_value", side_effect=env_side): + result = setup_mod._get_section_config_summary({}, "tools") + assert "Browser" in result + + +class TestSkipConfiguredSection: + """Test the _skip_configured_section helper.""" + + def test_returns_false_when_not_configured(self): + with patch.object(setup_mod, "get_env_value", return_value=""): + result = setup_mod._skip_configured_section({}, "model", "Model") + assert result is False + + def test_returns_true_when_user_skips(self): + def env_side(key): + return "sk-xxx" if key == "OPENROUTER_API_KEY" else "" + + with ( + patch.object(setup_mod, "get_env_value", side_effect=env_side), + patch.object(setup_mod, "prompt_yes_no", return_value=False), + ): + result = setup_mod._skip_configured_section( + {"model": "openai/gpt-4"}, "model", "Model" + ) + assert result is True + + def test_returns_false_when_user_wants_reconfig(self): + def env_side(key): + return "sk-xxx" if key == "OPENROUTER_API_KEY" else "" + + with ( + patch.object(setup_mod, "get_env_value", side_effect=env_side), + patch.object(setup_mod, "prompt_yes_no", return_value=True), + ): + result = setup_mod._skip_configured_section( + {"model": "openai/gpt-4"}, "model", "Model" + ) + assert result is False + + +class TestSetupWizardSkipsConfiguredSections: + """After migration, already-configured sections should offer skip.""" + + def test_sections_skipped_when_migration_imported_settings(self, tmp_path): + """When migration ran and API key exists, model section should be skippable. + + Simulates the real flow: get_env_value returns "" during the is_existing + check (before migration), then returns a key after migration imported it. + """ + args = _first_time_args() + + # Track whether migration has "run" — after it does, API key is available + migration_done = {"value": False} + + def env_side(key): + if migration_done["value"] and key == "OPENROUTER_API_KEY": + return "sk-xxx" + return "" + + def fake_migration(hermes_home): + migration_done["value"] = True + return True + + reloaded_config = {"model": "openai/gpt-4"} + + with ( + patch.object(setup_mod, "ensure_hermes_home"), + patch.object( + setup_mod, "load_config", + side_effect=[{}, reloaded_config], + ), + patch.object(setup_mod, "get_hermes_home", return_value=tmp_path), + patch.object(setup_mod, "get_env_value", side_effect=env_side), + patch.object(setup_mod, "is_interactive_stdin", return_value=True), + patch("hermes_cli.auth.get_active_provider", return_value=None), + patch("builtins.input", return_value=""), + # Migration succeeds and flips the env_side flag + patch.object( + setup_mod, "_offer_openclaw_migration", + side_effect=fake_migration, + ), + # User says No to all reconfig prompts + patch.object(setup_mod, "prompt_yes_no", return_value=False), + patch.object(setup_mod, "setup_model_provider") as mock_model, + patch.object(setup_mod, "setup_terminal_backend") as mock_terminal, + patch.object(setup_mod, "setup_agent_settings") as mock_agent, + patch.object(setup_mod, "setup_gateway") as mock_gateway, + patch.object(setup_mod, "setup_tools") as mock_tools, + patch.object(setup_mod, "save_config"), + patch.object(setup_mod, "_print_setup_summary"), + ): + setup_mod.run_setup_wizard(args) + + # Model has API key → skip offered, user said No → section NOT called + mock_model.assert_not_called() + # Terminal/agent always have a summary → skip offered, user said No + mock_terminal.assert_not_called() + mock_agent.assert_not_called() + # Gateway has no tokens (env_side returns "" for gateway keys) → section runs + mock_gateway.assert_called_once() + # Tools have no keys → section runs + mock_tools.assert_called_once()