Skip to content

fix: update openclaw migration vars#13131

Closed
IAvecilla wants to merge 2 commits into
NousResearch:mainfrom
IAvecilla:update-openclaw-migration-vars
Closed

fix: update openclaw migration vars#13131
IAvecilla wants to merge 2 commits into
NousResearch:mainfrom
IAvecilla:update-openclaw-migration-vars

Conversation

@IAvecilla

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a bug in _get_section_config_summary() (hermes_cli/setup.py) where the post-OpenClaw-migration "already configured?" check used a hardcoded env-var allowlist that had drifted out of sync with the rest of the codebase. As a result, users who migrated with a supported provider (zai/GLM, MiniMax, Gemini, xAI, Arcee, Xiaomi, Kimi, DeepSeek, …) or a Signal / WhatsApp gateway were forced to re-run setup even though Hermes already treated those sections as configured.

The approach: stop maintaining a parallel allowlist. The function now derives its env-var set from the same registries the rest of setup uses — PROVIDER_REGISTRY in hermes_cli.auth for providers, and _GATEWAY_PLATFORMS in hermes_cli.setup for gateways. Adding a new provider or platform is now a one-line change in the registry and the skip check picks it up automatically.

Related Issue

Fixes #13025

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

hermes_cli/setup.py

  • Extracted the model-section credential check into _model_section_has_credentials(config). It now:
    1. Honors an active OAuth provider via get_active_provider().
    2. When config["model"]["provider"] is set, checks that provider's own api_key_env_vars from PROVIDER_REGISTRY.
    3. Keeps the OpenRouter aggregator fallback (OPENROUTER_API_KEY / OPENAI_API_KEY).
    4. Scans the remaining registry entries with inline exclusions for copilot (collides with GH_TOKEN / GITHUB_TOKEN) and CLAUDE_CODE_OAUTH_TOKEN (set by Claude Code itself).
  • Replaced the gateway section's 15 hardcoded if get_env_value(...) checks with a comprehension over _GATEWAY_PLATFORMS — the same tuple the setup checklist uses. This fixes the Signal (SIGNAL_ACCOUNTSIGNAL_HTTP_URL) and WhatsApp (WHATSAPP_PHONE_NUMBER_IDWHATSAPP_ENABLED) mismatches.
  • Added _gateway_platform_short_label() to strip parenthetical qualifiers (e.g. "SMS (Twilio)""SMS", "Webhooks (GitHub, GitLab, etc.)""Webhooks").

tests/hermes_cli/test_setup_openclaw_migration.py — 9 new tests:

  • test_model_recognises_zai_glm_api_key
  • test_model_recognises_minimax_api_key
  • test_gateway_recognises_whatsapp_enabled
  • test_gateway_recognises_signal_http_url
  • test_model_ignores_bare_gh_token
  • test_model_ignores_bare_github_token
  • test_model_ignores_claude_code_oauth_token
  • test_model_copilot_recognised_when_explicitly_chosen
  • test_gateway_matches_platform_registry — drift guard: iterates every _GATEWAY_PLATFORMS entry and asserts the summary recognises it

How to Test

1. Run the targeted suite:

python -m pytest tests/hermes_cli/test_setup_openclaw_migration.py -v

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 15 (Darwin 25.3.0), Python 3.12

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — docstrings on the new helpers explain why registry-sourced env vars matter; no user-facing docs change needed
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A (no config keys added or renamed)
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A (internal helper refactor only)
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — N/A (pure Python, reads from the existing get_env_value and in-memory registries)
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

Screenshots / Logs

Before (on main, with the issue's repro):

GLM_API_KEY               => None / provider not recognized
WHATSAPP_ENABLED          => None
SIGNAL_HTTP_URL           => None
OPENROUTER_API_KEY        => anthropic/claude-opus-4.6

After (this PR):

GLM_API_KEY               => glm-5
WHATSAPP_ENABLED          => WhatsApp
SIGNAL_HTTP_URL           => Signal
OPENROUTER_API_KEY        => anthropic/claude-opus-4.6
MINIMAX_API_KEY           => MiniMax-M1
GH_TOKEN                  => None    # guard: not a false positive
GITHUB_TOKEN              => None    # guard: not a false positive
CLAUDE_CODE_OAUTH_TOKEN   => None    # guard: not a false positive

@trevorgordon981

Copy link
Copy Markdown

✅ Review Complete - LGTM

Tested in isolation - all 33 tests passing

Test Results

Component Tests Status
OpenClaw migration env var detection 33 passed

Detailed Analysis

Dynamic env var detection

  • Replacing hardcoded checks with and lookups is the right approach
  • Much more maintainable than manual env var lists
  • Correctly identifies the wrong env vars:
    • → should be
    • → should be
  • Adds missing providers that were previously undetected

Test coverage

  • Validates all migration scenarios: accept/decline/error/missing script
  • Tests config creation when missing
  • Validates first-time setup integration
  • Tests config reload on successful migration

Integration with setup wizard

  • Migration correctly offered during first-time setup
  • Not offered for existing installs (correct behavior)
  • Reloaded config flows into remaining setup sections
  • Sections correctly skipped when migration imported settings

Recommendation

Merge immediately. This fix:

  1. Prevents broken migrations from wrong env var detection
  2. Makes the system extensible for new providers
  3. Has comprehensive test coverage (33 tests)
  4. Integrates cleanly with the setup wizard

No additional changes needed. Ready for merge.


Reviewer: @teknium1
Tested on: macOS (Apple Silicon), Python 3.11.15
Date: April 20, 2026

teknium1 added a commit that referenced this pull request Apr 20, 2026
- Fix duplicate 'timezone' import in e2e conftest
- Fix test_text_before_command_not_detected asserting send() is awaited
  when no agent is present in mock setup (text messages don't produce
  command output)
@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #13187 (#13187). Your commit was cherry-picked onto current main with your authorship preserved in git log. Thanks @IAvecilla!

@teknium1 teknium1 closed this Apr 20, 2026
ulasbilgen pushed a commit to ulasbilgen/hermes-adhd-agent that referenced this pull request May 1, 2026
…ousResearch#9091, NousResearch#13131

- Fix duplicate 'timezone' import in e2e conftest
- Fix test_text_before_command_not_detected asserting send() is awaited
  when no agent is present in mock setup (text messages don't produce
  command output)
aj-nt pushed a commit to aj-nt/hermes-agent that referenced this pull request May 1, 2026
…ousResearch#9091, NousResearch#13131

- Fix duplicate 'timezone' import in e2e conftest
- Fix test_text_before_command_not_detected asserting send() is awaited
  when no agent is present in mock setup (text messages don't produce
  command output)
Luminet2023 pushed a commit to Luminet2023/hermes-agent that referenced this pull request May 1, 2026
…ousResearch#9091, NousResearch#13131

- Fix duplicate 'timezone' import in e2e conftest
- Fix test_text_before_command_not_detected asserting send() is awaited
  when no agent is present in mock setup (text messages don't produce
  command output)
02356abc pushed a commit to 02356abc/hermes-agent that referenced this pull request May 14, 2026
…ousResearch#9091, NousResearch#13131

- Fix duplicate 'timezone' import in e2e conftest
- Fix test_text_before_command_not_detected asserting send() is awaited
  when no agent is present in mock setup (text messages don't produce
  command output)
gweeteve pushed a commit to gweeteve/hermes-agent that referenced this pull request Jun 2, 2026
…ousResearch#9091, NousResearch#13131

- Fix duplicate 'timezone' import in e2e conftest
- Fix test_text_before_command_not_detected asserting send() is awaited
  when no agent is present in mock setup (text messages don't produce
  command output)
Egavasyug pushed a commit to Egavasyug/hermes-agent that referenced this pull request Jun 10, 2026
…ousResearch#9091, NousResearch#13131

- Fix duplicate 'timezone' import in e2e conftest
- Fix test_text_before_command_not_detected asserting send() is awaited
  when no agent is present in mock setup (text messages don't produce
  command output)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: OpenClaw migration section summary uses stale/incomplete provider and gateway checks

3 participants