diff --git a/hermes_cli/setup.py b/hermes_cli/setup.py index 1fcd88dff0..b4fa877d8c 100644 --- a/hermes_cli/setup.py +++ b/hermes_cli/setup.py @@ -2358,6 +2358,74 @@ def setup_tools(config: dict, first_install: bool = False): # ============================================================================= +def _model_section_has_credentials(config: dict) -> bool: + """Return True when any known inference provider has usable credentials. + + Sources of truth: + * ``PROVIDER_REGISTRY`` in ``hermes_cli.auth`` — lists every supported + provider along with its ``api_key_env_vars``. + * ``active_provider`` in the auth store — covers OAuth device-code / + external-OAuth providers (Nous, Codex, Qwen, Gemini CLI, ...). + * The legacy OpenRouter aggregator env vars, which route generic + ``OPENAI_API_KEY`` / ``OPENROUTER_API_KEY`` values through OpenRouter. + """ + try: + from hermes_cli.auth import get_active_provider + if get_active_provider(): + return True + except Exception: + pass + + try: + from hermes_cli.auth import PROVIDER_REGISTRY + except Exception: + PROVIDER_REGISTRY = {} # type: ignore[assignment] + + def _has_key(pconfig) -> bool: + for env_var in pconfig.api_key_env_vars: + # CLAUDE_CODE_OAUTH_TOKEN is set by Claude Code itself, not by + # the user — mirrors is_provider_explicitly_configured in auth.py. + if env_var == "CLAUDE_CODE_OAUTH_TOKEN": + continue + if get_env_value(env_var): + return True + return False + + # Prefer the provider declared in config.yaml, avoids false positives + # from stray env vars (GH_TOKEN, etc.) when the user has already picked + # a different provider. + model_cfg = config.get("model") if isinstance(config, dict) else None + if isinstance(model_cfg, dict): + provider_id = (model_cfg.get("provider") or "").strip().lower() + if provider_id in PROVIDER_REGISTRY: + if _has_key(PROVIDER_REGISTRY[provider_id]): + return True + if provider_id == "openrouter": + for env_var in ("OPENROUTER_API_KEY", "OPENAI_API_KEY"): + if get_env_value(env_var): + return True + + # OpenRouter aggregator fallback (no provider declared in config). + for env_var in ("OPENROUTER_API_KEY", "OPENAI_API_KEY"): + if get_env_value(env_var): + return True + + for pid, pconfig in PROVIDER_REGISTRY.items(): + # Skip copilot in auto-detect: GH_TOKEN / GITHUB_TOKEN are + # commonly set for git tooling. Mirrors resolve_provider in auth.py. + if pid == "copilot": + continue + if _has_key(pconfig): + return True + return False + + +def _gateway_platform_short_label(label: str) -> str: + """Strip trailing parenthetical qualifiers from a gateway platform label.""" + base = label.split("(", 1)[0].strip() + return base or label + + 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. @@ -2366,20 +2434,7 @@ def _get_section_config_summary(config: dict, section_key: str) -> Optional[str] 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: + if not _model_section_has_credentials(config): return None model = config.get("model") if isinstance(model, str) and model.strip(): @@ -2397,37 +2452,11 @@ def _get_section_config_summary(config: dict, section_key: str) -> Optional[str] 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("SIGNAL_ACCOUNT"): - platforms.append("Signal") - if get_env_value("EMAIL_ADDRESS"): - platforms.append("Email") - if get_env_value("TWILIO_ACCOUNT_SID"): - platforms.append("SMS") - if get_env_value("MATRIX_ACCESS_TOKEN") or get_env_value("MATRIX_PASSWORD"): - platforms.append("Matrix") - if get_env_value("MATTERMOST_TOKEN"): - platforms.append("Mattermost") - if get_env_value("WHATSAPP_PHONE_NUMBER_ID"): - platforms.append("WhatsApp") - if get_env_value("DINGTALK_CLIENT_ID"): - platforms.append("DingTalk") - if get_env_value("FEISHU_APP_ID"): - platforms.append("Feishu") - if get_env_value("WECOM_BOT_ID"): - platforms.append("WeCom") - if get_env_value("WEIXIN_ACCOUNT_ID"): - platforms.append("Weixin") - if get_env_value("BLUEBUBBLES_SERVER_URL"): - platforms.append("BlueBubbles") - if get_env_value("WEBHOOK_ENABLED"): - platforms.append("Webhooks") + platforms = [ + _gateway_platform_short_label(label) + for label, env_var, _ in _GATEWAY_PLATFORMS + if get_env_value(env_var) + ] if platforms: return ", ".join(platforms) return None # No platforms configured — section must run diff --git a/tests/hermes_cli/test_setup_openclaw_migration.py b/tests/hermes_cli/test_setup_openclaw_migration.py index fe80263905..a458bd3761 100644 --- a/tests/hermes_cli/test_setup_openclaw_migration.py +++ b/tests/hermes_cli/test_setup_openclaw_migration.py @@ -437,6 +437,112 @@ class TestGetSectionConfigSummary: result = setup_mod._get_section_config_summary({}, "tools") assert "Browser" in result + # Regression tests for issue #13025: the model / gateway summaries used + # stale, hardcoded env-var allowlists that drifted from the real setup + + # status flows. Every case below would previously return ``None`` and + # force OpenClaw migration to re-run setup for an already-configured + # section. + + def test_model_recognises_zai_glm_api_key(self): + """GLM_API_KEY (zai provider) should count as configured.""" + def env_side(key): + return "glm-test-key" if key == "GLM_API_KEY" else "" + + with patch.object(setup_mod, "get_env_value", side_effect=env_side): + result = setup_mod._get_section_config_summary( + {"model": {"provider": "zai", "default": "glm-5"}}, "model" + ) + assert result == "glm-5" + + def test_model_recognises_minimax_api_key(self): + """MINIMAX_API_KEY should count as configured.""" + def env_side(key): + return "minimax-key" if key == "MINIMAX_API_KEY" else "" + + with patch.object(setup_mod, "get_env_value", side_effect=env_side): + result = setup_mod._get_section_config_summary( + {"model": {"provider": "minimax", "default": "MiniMax-M1"}}, + "model", + ) + assert result == "MiniMax-M1" + + def test_gateway_recognises_whatsapp_enabled(self): + """WhatsApp uses WHATSAPP_ENABLED (not WHATSAPP_PHONE_NUMBER_ID).""" + def env_side(key): + return "true" if key == "WHATSAPP_ENABLED" else "" + + with patch.object(setup_mod, "get_env_value", side_effect=env_side): + result = setup_mod._get_section_config_summary({}, "gateway") + assert result is not None + assert "WhatsApp" in result + + def test_gateway_recognises_signal_http_url(self): + """Signal uses SIGNAL_HTTP_URL (not SIGNAL_ACCOUNT).""" + def env_side(key): + return "http://signal.local" if key == "SIGNAL_HTTP_URL" else "" + + with patch.object(setup_mod, "get_env_value", side_effect=env_side): + result = setup_mod._get_section_config_summary({}, "gateway") + assert result is not None + assert "Signal" in result + + def test_model_ignores_bare_gh_token(self): + """GH_TOKEN is commonly set for `gh` / git and must NOT count as a + configured inference provider on its own — mirrors the copilot + exclusion in resolve_provider().""" + def env_side(key): + return "gho_xxx" if key == "GH_TOKEN" else "" + + with patch.object(setup_mod, "get_env_value", side_effect=env_side): + result = setup_mod._get_section_config_summary({}, "model") + assert result is None + + def test_model_ignores_bare_github_token(self): + """GITHUB_TOKEN is commonly set in CI and must not trigger skip.""" + def env_side(key): + return "ghp_xxx" if key == "GITHUB_TOKEN" else "" + + with patch.object(setup_mod, "get_env_value", side_effect=env_side): + result = setup_mod._get_section_config_summary({}, "model") + assert result is None + + def test_model_ignores_claude_code_oauth_token(self): + """CLAUDE_CODE_OAUTH_TOKEN is set by Claude Code itself and must not + trigger skip — mirrors the _IMPLICIT_ENV_VARS guard in + is_provider_explicitly_configured().""" + def env_side(key): + return "sk-ant-oat01-xxx" if key == "CLAUDE_CODE_OAUTH_TOKEN" else "" + + with patch.object(setup_mod, "get_env_value", side_effect=env_side): + result = setup_mod._get_section_config_summary({}, "model") + assert result is None + + def test_model_copilot_recognised_when_explicitly_chosen(self): + """If the user picked copilot in config, GH_TOKEN *does* count — + only the auto-detect path excludes it.""" + def env_side(key): + return "gho_xxx" if key == "GH_TOKEN" else "" + + cfg = {"model": {"provider": "copilot", "default": "gpt-5"}} + with patch.object(setup_mod, "get_env_value", side_effect=env_side): + result = setup_mod._get_section_config_summary(cfg, "model") + assert result == "gpt-5" + + def test_gateway_matches_platform_registry(self): + """Every platform in _GATEWAY_PLATFORMS should be recognised by its + own env-var sentinel — i.e. the summary must not drift from the + registry used by the setup checklist.""" + for label, env_var, _fn in setup_mod._GATEWAY_PLATFORMS: + def env_side(key, _target=env_var): + return "x" if key == _target else "" + with patch.object(setup_mod, "get_env_value", side_effect=env_side): + result = setup_mod._get_section_config_summary({}, "gateway") + expected = setup_mod._gateway_platform_short_label(label) + assert result is not None, f"{label} ({env_var}) not recognised" + assert expected in result, ( + f"{label} ({env_var}) recognised but label missing from summary: {result!r}" + ) + class TestSkipConfiguredSection: """Test the _skip_configured_section helper."""