Skip to content

fix(doctor): recognize configured MoA orchestration - #64203

Closed
mudrii wants to merge 1 commit into
NousResearch:mainfrom
mudrii:fix/doctor-moa-config
Closed

fix(doctor): recognize configured MoA orchestration#64203
mudrii wants to merge 1 commit into
NousResearch:mainfrom
mudrii:fix/doctor-moa-config

Conversation

@mudrii

@mudrii mudrii commented Jul 14, 2026

Copy link
Copy Markdown

Summary

  • recognize the configured moa orchestration provider in hermes doctor
  • avoid treating moa as a credential-bearing catalog provider
  • add moa to the existing provider-ID acceptance coverage

Why

moa is an orchestration layer, not a standalone model API provider. A valid MoA configuration should not produce an unknown-provider or missing-credentials warning.

Verification

  • scripts/run_tests.sh tests/hermes_cli/test_doctor.py
  • 67 passed, 0 failed
  • ruff check passed
  • git diff --check passed

Copilot AI review requested due to automatic review settings July 14, 2026 06:00

Copilot AI 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.

Pull request overview

This PR updates hermes doctor to treat moa as a valid, configured orchestration provider (rather than an unknown provider / missing-credentials case), aligning doctor’s provider validation with how MoA is represented in Hermes.

Changes:

  • Add moa to hermes doctor’s accepted provider IDs so it won’t be flagged as unrecognized.
  • Exclude moa from the “non-auto provider must be a recognized catalog/auth provider” validation path.
  • Extend existing doctor test coverage to include moa in the provider-ID acceptance parametrization.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
hermes_cli/doctor.py Recognizes moa as a known/accepted configured provider and avoids unknown-provider warnings for it.
tests/hermes_cli/test_doctor.py Adds a regression case ensuring moa isn’t rejected as an unknown provider.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@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 P3 Low — cosmetic, nice to have duplicate This issue or pull request already exists labels Jul 14, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #58771 (the earliest still-open PR making the same hermes doctor change — adding moa to the recognized-providers set). Same code site and semantic fix; only the test style differs. #59016 was already closed as a duplicate of #58771. Flagging so a maintainer can pick the canonical one.

@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 doctor regression fix. The underlying issue is real: current main omits moa from known_providers at hermes_cli/doctor.py:754, while the runtime defines it as a virtual provider at hermes_cli/providers.py:47-51.

Problems

  • The same focused change is already present in the earlier open canonical PR #58771 (33cf91192144072e7663fd339c15c7254d488861).
  • The additional validation bypass is not needed for credential handling. The credential check only fails for an API-key PROVIDER_REGISTRY entry (hermes_cli/doctor.py:885-906); MoA is not one.

Suggested changes

  • Prefer #58771 for salvage. If selecting this patch instead, keep the recognized-provider addition and regression test, and drop the redundant bypass.

Automated hermes-sweeper review.

Comment thread hermes_cli/doctor.py
@@ -815,7 +817,7 @@ def run_doctor(args):
if catalog_provider is not None:

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.

This MoA-specific bypass is redundant once moa is added to known_providers: the credential path only fails registry entries with auth_type == "api_key" (hermes_cli/doctor.py:885-906), while MoA is a virtual provider. Please omit this hunk if this patch is salvaged.

@teknium1 teknium1 added 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 teknium1 added the area/install-update Installer, updater, packaging, wheels, doctor label Jul 19, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Closing as a duplicate of #58771 (@liuhao1024, submitted Jul 5 vs Jul 14) — both add moa to doctor's known_providers, and we verified empirically that the outcomes are identical (with moa in the set, resolve_provider_full("moa") passes the downstream intersection check, making the second hunk redundant belt-and-suspenders). #58771 will be merged in the MoA config-plumbing salvage. Thanks — the diagnosis was right, just beaten to it by nine days.

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/install-update Installer, updater, packaging, wheels, doctor comp/cli CLI entry point, hermes_cli/, setup wizard duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have 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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants