Skip to content

fix(gateway): preserve multiplex profile in model picker - #63447

Closed
cresslank wants to merge 1 commit into
NousResearch:mainfrom
cresslank:fix/multiplex-model-picker-scope
Closed

fix(gateway): preserve multiplex profile in model picker#63447
cresslank wants to merge 1 commit into
NousResearch:mainfrom
cresslank:fix/multiplex-model-picker-scope

Conversation

@cresslank

Copy link
Copy Markdown
Contributor

What does this PR do?

Preserves one exact multiplex profile identity across the gateway /model picker: adapter selection, deferred callback credential resolution, and global config read/write.

On current main, the picker reads self.adapters directly, so a named-profile event can present through the default profile's adapter. The stored on_model_selected callback also runs after the routed profile scope has exited, and --global can seed a named-profile write from the default profile's config.

This resolves the adapter with _adapter_for_source(source), captures the routed profile home once, and re-enters that scope when the deferred callback runs. The same captured home selects the config file used for persistence.

Related Issue

Follow-up to #58587. Complements #61488 and #48187; it does not duplicate their session-default or context-length policy changes.

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

  • gateway/slash_commands.py: route the picker through the profile-aware adapter registry.
  • gateway/slash_commands.py: capture the routed profile home for deferred callback execution and profile-local config persistence.
  • tests/gateway/test_model_picker_persist.py: exercise the production handler, profile secret scope, routed adapter registry, and profile-local config write.

How to Test

  1. Use a temporary HERMES_HOME with plugins.enabled: [] so a user plugin cannot mask core behavior.

  2. On unpatched current main, run the two new multiplex picker tests: both fail because the default adapter captures the picker.

  3. On this branch, run:

    python -m pytest -q \
      tests/gateway/test_model_picker_persist.py \
      tests/gateway/test_multiplex_adapter_registry.py \
      tests/gateway/test_model_command_custom_providers.py \
      tests/gateway/test_model_command_flat_string_config.py

    Result: 23 passed.

  4. Additional validation:

    ruff check gateway/slash_commands.py tests/gateway/test_model_picker_persist.py
    git diff --check origin/main...HEAD
    python scripts/check-windows-footguns.py --all

    All clean.

Not in Scope

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: Fedora Linux 44

Documentation & Housekeeping

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

Screenshots / Logs

Unpatched current main + new regressions: 2 failed
Patched focused suite:                     23 passed
Ruff:                                      clean
Windows footguns:                          clean

@cresslank
cresslank force-pushed the fix/multiplex-model-picker-scope branch from 97714d4 to 60dcc1d Compare July 15, 2026 12:41
@cresslank

cresslank commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (9df5f879b) and force-pushed the PR branch as a single commit: 60dcc1d68. The rebase applied cleanly.

Validation after rebase:

  • focused gateway model-picker suite with an isolated HERMES_HOME and plugins.enabled: []: 23 passed
  • Ruff on both changed Python files: clean
  • Windows footgun scan: clean (762 files)
  • git diff --check: clean

All GitHub checks now pass (23 successful, 7 expected skips, 1 neutral OSV check), and the PR reports MERGEABLE / CLEAN. This should be current for review.

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery area/config Config system, migrations, profiles P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 15, 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 the focused multiplex picker fix. The current-main premise is real: gateway/slash_commands.py:1506 selects from the default adapter map, while secondary adapters live in the profile registry.

Problems

  • Blocking: the initial picker setup remains outside the routed profile scope. Production dispatch calls _handle_model_command() directly at gateway/run.py:9930; _load_gateway_config() uses the default home unless an override scope is active (gateway/run.py:2335-2355). The new scope only wraps the deferred callback, so a named profile can still build its picker from default-profile config/providers.

Suggested changes

  • Scope the initial config/provider-list work with the captured profile home as well as the callback.
  • Update tests/gateway/test_model_picker_persist.py to invoke production dispatch without manually entering _profile_runtime_scope; the current test does so at PR line 310 and masks this gap.

Automated hermes-sweeper review.

Comment thread gateway/slash_commands.py
from hermes_cli.providers import get_label

raw_args = event.get_command_args().strip()
source = event.source

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.

Capturing the routed home is not sufficient for the initial picker setup: production dispatch reaches this handler without _profile_runtime_scope (gateway/run.py:9930), so the subsequent _load_gateway_config() still reads the default home. Please scope the initial config/provider-list work with this captured home too; the deferred callback scope only fixes the later selection phase.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages 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 16, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #65700 — your commit was cherry-picked onto current main with your authorship preserved in git log (rebase merge). All three gaps you found were confirmed live: the picker's default-adapter-map straggler, the unscoped selection callback, and --global writing the wrong profile's config. The tests were excellent — real _profile_adapters resolution with byte-identical default-config assertions. Thanks!

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 area/profiles Multi-profile isolation, HERMES_HOME scoping comp/gateway Gateway runner, session dispatch, delivery 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-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants