Skip to content

fix(web): register configured shell hooks for Web UI sessions - #83997

Open
andyst-dev wants to merge 1 commit into
NousResearch:mainfrom
andyst-dev:fix/web-ui-register-config-hooks
Open

andyst-dev wants to merge 1 commit into
NousResearch:mainfrom
andyst-dev:fix/web-ui-register-config-hooks

Conversation

@andyst-dev

Copy link
Copy Markdown
Contributor

Fixes #83980

Problem

Ordinary hermes dashboard Web UI sessions never registered the lifecycle hooks declared in cli-config.yaml. agent.shell_hooks.register_from_config() had call sites in cli.py and gateway/run.py but none on the Web UI path. So the persistent Relay observability (and any config-defined shell hook) silently stopped emitting for Web UI chats — while the same config looked healthy on CLI, TUI, messaging, and webhook surfaces.

Root cause

The FastAPI app's lifespan never invoked register_from_config. Web UI sessions built the agent but skipped the config-driven hook and outbound-webhook registration that every other surface performs at startup.

Fix

Register declarative shell hooks and outbound webhooks from the loaded config during the FastAPI lifespan (hermes_cli/web_server.py), mirroring the gateway and CLI call sites. The web server has no TTY, so consent is resolved from the same opt-in channels (--accept-hooks, HERMES_ACCEPT_HOOKS, hooks_auto_accept). Failures are logged but never block startup — identical to the gateway path.

Tests

Added tests/hermes_cli/test_web_server_registers_config_hooks.py: starts the app through TestClient (which runs the lifespan) with register_from_config/outbound-webhook registration patched at the source module, and asserts the web server calls them with the loaded config and consent resolved internally (accept_hooks=False).

Verified: new test passes, existing web-server tests (test_web_server_git, test_web_server_config_offloop) still pass, ruff clean.

@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard comp/dashboard Web dashboard / control panel UI (dashboard/, landing) area/config Config system, migrations, profiles P2 Medium — degraded but workaround exists needs-decision Awaiting maintainer decision before any implementation sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 11, 2026
@andyst-dev
andyst-dev force-pushed the fix/web-ui-register-config-hooks branch from 9f8dd85 to 479c7b9 Compare August 14, 2026 06:17
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(web): register configured shell hooks for Web UI sessions

  1. hermes_cli/web_server.py (lifespan): the registration failure path logs at DEBUG (_log.debug(..., exc_info=True)). This PR exists precisely because a configured observability hook silently disappeared for Web UI sessions; swallowing registration failures at debug level recreates the same silent-failure class in production. Keep startup non-blocking, but log at WARNING so a misconfigured hook is visible in agent.log.

  2. Double-registration risk: the lifespan registers hooks from config, but other surfaces in the same process (gateway runner, per-agent construction in CLI/TUI paths) also call register_from_config. If registration is not idempotent, a Web session could fire hooks twice (or the same hook object registered multiple times). Worth verifying register_from_config dedups by hook identity, and if it does, a one-line comment here stating that would close the question.

  3. Test gap: test_lifespan_registers_shell_hooks_from_config asserts the registration calls happen with the right args, but not that a registered hook actually fires during a Web session agent turn (register → invoke chain). The current assertion covers the call site; a light end-to-end check would guard against a future refactor that registers but never invokes.

@andyst-dev

Copy link
Copy Markdown
Contributor Author

Addressed the review points on this PR.

Point 1 (log registration failure at WARNING, not DEBUG): done. A configured hook silently vanishing is exactly the failure this PR exists to fix (Web UI sessions previously skipped configured hooks entirely), so the lifespan failure path now logs at WARNING (with exc_info=True) instead of DEBUG — a misconfigured hook is visible in agent.log while startup still never blocks.

Point 2 (double-registration): verified — agent/shell_hooks.register_from_config dedups by hook identity under a lock: each (event, command, matcher) key is checked against _registered before registering (and again before appending), so repeated registration from multiple surfaces (gateway, CLI/TUI, and this Web lifespan in the same process) cannot fire a hook twice. The docstring already notes "already registered" entries are logged but not returned.

Point 3 (register → invoke chain): the existing test_lifespan_registers_shell_hooks_from_config asserts the lifespan calls register_from_config and outbound_webhooks.register_from_config with the loaded config and accept_hooks=False; a full register→invoke E2E over an agent turn is heavier scope than this call-site regression, but the dedup check above plus the call-site assertion cover the wiring this PR adds.

Validation: pytest tests/hermes_cli/test_web_server_registers_config_hooks.py → 1 passed.

@andyst-dev
andyst-dev force-pushed the fix/web-ui-register-config-hooks branch from dfc436c to 869899c Compare September 1, 2026 10:20
@andyst-dev
andyst-dev force-pushed the fix/web-ui-register-config-hooks branch from 869899c to 3662c2f Compare September 8, 2026 11:10
@andyst-dev

Copy link
Copy Markdown
Contributor Author

Rebased on current main (web_server refactored; shell-hooks registration re-injected after thread start, accept_hooks=False, failures log WARNING not blocking boot). test_web_server.py → 185 passed (2 pre-existing env failures — web dist/theme build — also fail on clean main). Mergeable.

@andyst-dev
andyst-dev force-pushed the fix/web-ui-register-config-hooks branch from 3662c2f to 6b87a15 Compare September 24, 2026 08:19

This branch has not been deployed

No deployments
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 comp/dashboard Web dashboard / control panel UI (dashboard/, landing) needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists 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]: Web UI sessions do not register config shell hooks, breaking persistent Relay telemetry

3 participants