Skip to content

fix(config): map provider shortcut to model.provider for hermes config set - #41970

Closed
xiaoxinova wants to merge 1 commit into
NousResearch:mainfrom
xiaoxinova:fix/provider-key-alias
Closed

fix(config): map provider shortcut to model.provider for hermes config set#41970
xiaoxinova wants to merge 1 commit into
NousResearch:mainfrom
xiaoxinova:fix/provider-key-alias

Conversation

@xiaoxinova

@xiaoxinova xiaoxinova commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

hermes config set provider X writes to a top-level provider: key in config.yaml, but the runtime reads the effective provider from model.provider — a nested key. The two keys silently diverge, so a user who runs hermes config set provider openai and later checks hermes config show sees the value, but the agent never actually uses it.

Add a CONFIG_KEY_ALIASES registry inside set_config_value() that transparently maps the user-friendly shortcut provider to the canonical dotted path model.provider, and cleans up any legacy bare key left by previous writes.

Related Issue

Fixes #41943

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • hermes_cli/config.py: Added _CONFIG_KEY_ALIASES mapping that resolves providermodel.provider before calling _set_nested. Also cleans up any lingering bare provider: key from previous writes.

How to Test

  1. Run hermes config set provider opencode-go and verify it writes to model.provider in config.yaml (not a top-level provider: key)
  2. Run hermes config show and confirm the provider reflects the new value
  3. Verify hermes config set model.provider anthropic still works (explicit dotted path unaffected)

Checklist

Code

  • I have read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, etc.)
  • I searched for existing PRs to make sure this is not a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I have run pytest tests/ -q and all tests pass
  • I have added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I have tested on my platform: Docker/Linux

Documentation & Housekeeping

  • I have updated relevant documentation (README, docs/, docstrings) — or N/A
  • I have updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I have updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I have considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I have updated tool descriptions/schemas if I changed tool behavior — or N/A

@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard area/config Config system, migrations, profiles P2 Medium — degraded but workaround exists labels Jun 8, 2026
@xiaoxinova
xiaoxinova force-pushed the fix/provider-key-alias branch from 8c090ca to d5d1c74 Compare June 8, 2026 09:52
@liuhao1024

Copy link
Copy Markdown
Contributor

⚠️ PR title does not match the diff — bundled unrelated regressions

The title says fix(config): map provider shortcut to model.provider, but the diff contains four unrelated changes beyond the config alias:

1. agent/chat_completion_helpers.py — removes request cancellation flags (−953 lines)

This reverts the fix for cascading-interrupt hang (#6600). The _request_cancelled flag was specifically added so the worker thread recognizes a force-close as intentional (not a network bug) and exits without burning full retry cycles. Removing it reintroduces the 7-minute hang on interrupt.

2. agent/memory_manager.py — removes background sync executor

The ThreadPoolExecutor + _drain_sync_executor pattern exists because a misconfigured provider's sync_turn was observed blocking ~298s inline. Removing it means a slow/wedged memory provider blocks the turn-completion path again, keeping the agent marked "running" in all interfaces.

3. scripts/install.ps1 — removes initial commit validation

Reverts the fix for #40998 (interrupted clone with no initial commit causing git stash to abort). The three-check validation (rev-parse + status + HEAD) was belt-and-braces against partial checkouts.

4. scripts/install.sh — same as above

Recommendation: The hermes_cli/config.py shortcut alias (13 lines) is a clean, correct fix. The other three changes should be in separate PRs if they're intentional — but based on the code comments referencing specific issue numbers, these look like accidental deletions rather than deliberate reverts. Please confirm the intent.

@xiaoxinova
xiaoxinova force-pushed the fix/provider-key-alias branch 2 times, most recently from d5d1c74 to 00f36c4 Compare June 8, 2026 09:57
@xiaoxinova

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review! The unrelated regressions you pointed out were actually an artifact of a messy force-push on my fork branch. I've since squashed the history and the current diff should only contain the config.py +14 alias change. The current PR diff is clean now — just the one file. Appreciate you catching that.

@xiaoxinova

Copy link
Copy Markdown
Contributor Author

Superseded by #41956 — same approach, more complete (includes base_url alias + tests). Closing, but learned a ton from this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

config: "hermes config set provider" writes to dead key, diverges from model.provider

3 participants