Skip to content

fix(update): warn when configured gateway extras stay missing - #10733

Open
LeonSGP43 wants to merge 1 commit into
NousResearch:mainfrom
LeonSGP43:fix/update-configured-extras-warning
Open

fix(update): warn when configured gateway extras stay missing#10733
LeonSGP43 wants to merge 1 commit into
NousResearch:mainfrom
LeonSGP43:fix/update-configured-extras-warning

Conversation

@LeonSGP43

Copy link
Copy Markdown
Contributor

Summary

  • keep the optional-extra fallback behavior, but add a high-signal warning when a configured gateway platform still lacks its dependency after fallback installs
  • detect configured gateway platforms from the current Hermes config/env and check whether their runtime packages are still unavailable
  • print an exact reinstall command so users can recover before restarting the gateway

Testing

  • python3 -m pytest -o addopts='' tests/hermes_cli/test_update_autostash.py

Fixes #10651

@drousselbot

Copy link
Copy Markdown

QA triage + validation summary from Hermes Agent.

This PR arrived in the QA lane unlabeled, so I treated it as ownerless intake triage.

What I validated:

  • reviewed the diff for hermes_cli/main.py and tests/hermes_cli/test_update_autostash.py
  • confirmed the new warning only triggers when a configured gateway platform still has a missing runtime dependency after fallback installs
  • confirmed the warning prints an explicit reinstall command for the affected extra
  • ran python3 -m pytest -o addopts='' tests/hermes_cli/test_update_autostash.py

Result:

  • 24 tests passed locally
  • no blocking defects found in the warning-path change
  • classified as QA-passed and handed to the next steward lane

Non-blocking note:

  • coverage is strongest for the Feishu path; shared messaging-extra variants (Telegram/Discord/Slack) are not directly exercised by the new test

Operational note:

  • GitHub currently reports no PR checks on this branch, so this QA result is based on independent local validation rather than CI evidence.

@drousselbot

Copy link
Copy Markdown

Follow-up: I attempted to record this QA outcome through the deterministic workflow dispatcher, but the transition could not be persisted because this repository does not currently have the dispatcher workflow labels (for example qa-passed).

Observed failure:

  • dispatcher transition qa-pass returned: 'qa-passed' not found

Impact:

  • the QA verdict is documented here, but the repo's label-based lane state machine cannot currently record or route the next owner
  • I also checked the repository label list and only the generic/public labels are present; the workflow vocabulary is missing

Recommended fix:

  • add the dispatcher label set (ready-for-dev, in-dev, needs-qa, qa-passed, qa-failed, po-approved, blocked, waiting-on-human, plus any docs/security labels the repo intends to use)
  • then re-apply the appropriate dispatcher transition so the PR can move cleanly to the next lane

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery labels Apr 25, 2026
@blightbow

Copy link
Copy Markdown

Just diagnosed the exact same class of failure on a Homebrew hermes-agent install (v2026.5.7), but for the MCP SDK rather than Feishu:

Symptom: parkour (stdio) — failed in the startup banner. Zero MCP-related log entries at INFO level. The MCP server binary works fine when tested manually over stdio — JSON-RPC initialize handshake succeeds, tools are discovered.

Root cause: Homebrew's virtualenv_install_with_resources only installs core deps. The [mcp] extra (mcp>=1.2.0,<2) is absent from the libexec venv. tools/mcp_tool.py has a graceful import guard that sets _MCP_AVAILABLE = False and logs at DEBUG level only — invisible with the default INFO log setting. The startup UI still reports "failed" because get_mcp_status() reads config independently and reports any server not in _servers as disconnected.

Fix: python -m pip install "mcp>=1.2.0,<2" into the libexec venv. Restart. Works immediately.

Relevance to this PR: This PR warns on upgrade when configured extras are missing — great. But the initial install path (Homebrew, Docker, any packaging that only pulls core deps) has the same silent failure with no warning at all. The import guard pattern in tools/mcp_tool.py:230-231 (debug-only log when mcp_servers is configured but the SDK is absent) is the same PoLA violation described in #10651. A startup-time check — "mcp_servers configured but mcp SDK not importable" → WARNING with reinstall command — would catch both paths.

Happy to open a focused issue for the MCP variant if that's preferred over expanding scope here.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused recovery warning. The fallback target has changed since this branch was opened.

Problems

  • On current main, failed_extras is populated only from the aggregate [all] references (hermes_cli/main.py:7690-7697, 6932-6958). [all] now deliberately excludes messaging, slack, matrix, dingtalk, and feishu (pyproject.toml:273-305). The proposed gateway mapping therefore cannot match a production fallback failure.
  • The new test mocks feishu into that list, so it verifies an unreachable current-main branch.
  • The MCP failure described in the discussion is not covered: [all] retains mcp (pyproject.toml:298), while the mapping has no MCP case.

Suggested changes

  • Rebase the approach on the lazy platform dependency path: adapters call ensure() / ensure_and_bind() (for example plugins/platforms/feishu/adapter.py:1361-1420) and update refreshes active lazy features at hermes_cli/main.py:7615-7653.
  • Add a real-config regression test for a configured platform’s failed lazy install and its actionable remediation.

Automated hermes-sweeper review.

@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 area/install-update Installer, updater, packaging, wheels, doctor labels Jul 12, 2026
@alt-glitch alt-glitch added P3 Low — cosmetic, nice to have needs-decision Awaiting maintainer decision before any implementation and removed comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists labels Jul 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/install-update Installer, updater, packaging, wheels, doctor comp/cli CLI entry point, hermes_cli/, setup wizard needs-decision Awaiting maintainer decision before any implementation 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.

Bug: hermes upgrade silently fails optional extras (e.g. feishu/lark-oapi), gateway crashes on restart

5 participants