Skip to content

fix(cli): resolve config set provider/base_url shortcuts to canonical dotted paths - #41956

Open
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/config-set-provider-dead-key
Open

fix(cli): resolve config set provider/base_url shortcuts to canonical dotted paths#41956
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/config-set-provider-dead-key

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

Resolves shortcut keys provider and base_url in hermes config set to their canonical dotted paths (model.provider, model.base_url) so the value lands in the key the runtime actually reads instead of a dead top-level key.

Related Issue

Fixes #41943

Type of Change

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

Changes Made

  • hermes_cli/config.py: Added _KEY_SHORTCUTS mapping that resolves providermodel.provider and base_urlmodel.base_url before calling _set_nested. Updated the _config_to_env_sync lookup and the success message to use the resolved key.
  • tests/cli/test_config_set_shortcut.py: 5 regression tests covering shortcut resolution, dotted-key passthrough, non-shortcut keys, and preservation of sibling model keys.

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. Run pytest tests/cli/test_config_set_shortcut.py -v — all 5 tests pass
  4. Verify hermes config set model.provider anthropic still works (explicit dotted path unaffected)

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

Documentation & Housekeeping

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

Code Intelligence

  • Analyzed: hermes_cli/config.py::set_config_value() (callers: CLI config set command, setup wizard)
  • Blast radius: LOW — only affects hermes config set for the two shortcut keys; all dotted-path usage unchanged
  • Related patterns: _config_to_env_sync already maps dotted config keys to env vars; this adds a parallel shortcut resolution layer

… dotted paths

`hermes config set provider X` wrote to a dead top-level `provider:`
key instead of the runtime key `model.provider`, creating a false sense
of configuration success. The same issue affected `base_url`.

Add a _KEY_SHORTCUTS mapping that resolves these shortcut keys to their
canonical dotted paths before calling _set_nested, so the value lands in
the key the runtime actually reads.

Fixes NousResearch#41943
@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

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for isolating the bare-key configuration path. The issue remains reproducible on current main: set_config_value() still writes the caller-supplied bare key at hermes_cli/config.py:8167, while root-key normalization only promotes it when the canonical model.* key is empty (hermes_cli/config.py:6552-6554).

Problems

  • The shortcut is resolved after the managed-scope guard. Current main rejects managed keys at hermes_cli/config.py:8112, but the PR resolves provider to model.provider at hermes_cli/config.py:6040. Thus a managed model.provider can be reached through the bare shortcut and written despite the guard.
  • tests/cli/test_config_set_shortcut.py covers only unmanaged writes; add managed-scope coverage for both aliases.

Suggested changes

  • Resolve the shortcut before the managed-key check, then use the resolved dotted key for the guard and all later operations.
  • Add tests proving provider and base_url shortcuts cannot persist values when their canonical keys are managed.

This is an automated hermes-sweeper review.

Comment thread hermes_cli/config.py
"provider": "model.provider",
"base_url": "model.base_url",
}
resolved_key = _KEY_SHORTCUTS.get(key, key)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolve this alias before the earlier managed_scope.is_key_managed(key) guard. As written, a managed model.provider or model.base_url can be modified through the bare shortcut because the guard only sees provider or base_url.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
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 sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data 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