Skip to content

fix(hooks): register hooks in profile-isolated Desktop and TUI backends - #62681

Closed
skeptikoss wants to merge 1 commit into
NousResearch:mainfrom
skeptikoss:fix/desktop-tui-shell-hook-registration
Closed

skeptikoss wants to merge 1 commit into
NousResearch:mainfrom
skeptikoss:fix/desktop-tui-shell-hook-registration

Conversation

@skeptikoss

Copy link
Copy Markdown

Problem

Shell hooks configured in config.yaml are registered in CLI and messaging-gateway startup, but not in the profile-specific Desktop backend or standalone TUI gateway. The turn loop still invokes hooks, but the process-local registry is empty, so hooks such as pre_llm_call and pre_tool_call silently do nothing.

This is especially misleading because hermes hooks doctor can pass while the active Desktop/TUI process has registered no hooks.

Fix

  • Register configured shell hooks in cmd_dashboard() only when HERMES_DESKTOP=1.
  • Register after interactive auth setup and immediately before start_server() because auth setup can force plugin rediscovery and clear registered callbacks.
  • Leave the ordinary multi-profile browser dashboard unchanged; it must not install one profile's hooks process-wide.
  • Register hooks in standalone tui_gateway.entry.main() before gateway.ready.
  • Keep both startup paths fail-open: registration errors are debug-logged and never prevent Hermes from starting.
  • Preserve side-effect-free --status and --stop lifecycle actions.

Profile isolation

The HERMES_DESKTOP=1 guard is intentional. Desktop starts a separate backend process per profile, while the ordinary browser dashboard can serve multiple profiles. Unconditionally registering the launch/default profile's hooks in the browser-dashboard process risks executing them during another profile's turns.

Ordering

Registration occurs after _maybe_setup_dashboard_auth_interactively(). That path may force plugin rediscovery, which clears PluginManager._hooks; registering earlier can leave shell-hook deduplication state populated while the actual callbacks have been removed.

Tests

Added regression coverage for:

  • ordinary dashboard does not register profile hooks;
  • Desktop registers after auth setup and immediately before server start;
  • registration failure still starts the Desktop server;
  • --status and --stop do not register hooks;
  • standalone TUI registers before gateway.ready;
  • TUI config-load and registration failures still emit readiness.

Local verification:

19 focused tests passed
Ruff passed
Python compilation passed
git diff --check passed
Independent review: PASS — no security concerns or logic errors

The same patch was also exercised through real isolated Desktop WebSocket and standalone TUI prompt paths with a controlled pre_llm_call hook; both produced completed hook invocations.

Related work

Related issues: #41457, #43823.

This overlaps #53894, #57020 and #61823, but differs in two material ways:

  1. it avoids process-global hook registration in the ordinary multi-profile browser dashboard;
  2. it registers only after auth/plugin rediscovery, preventing callbacks from being silently cleared during startup.

Register config shell hooks only in profile-specific Desktop backends after auth/plugin rediscovery, and in standalone TUI before readiness. Keep ordinary multi-profile dashboards isolated and make failures non-blocking.
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard comp/tui Terminal UI (ui-tui/ + tui_gateway/) comp/desktop Electron desktop app (apps/desktop/*) sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data P2 Medium — degraded but workaround exists labels Jul 11, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for tracing the separate backend startup paths. The production wiring matches the current topology: Desktop launches a per-profile serve process with HERMES_DESKTOP='1' (apps/desktop/electron/main.ts:6100-6128), while current cmd_dashboard() goes directly from auth setup to start_server() without hook registration (hermes_cli/main.py:12170-12189). tui_gateway.entry.main() likewise currently reaches readiness without registering shell hooks (tui_gateway/entry.py:293-353).

Problems

  • The new tests stub both hermes_cli.config.load_config and agent.shell_hooks.register_from_config, so they prove ordering but not actual profile-config-to-callback propagation. The callback is process-local (agent/shell_hooks.py:245-278), and this is a shell-execution/config boundary for which AGENTS.md requires real-import E2E coverage using a temporary HERMES_HOME.

Suggested changes

  • Add focused real-import coverage with a temporary profile home and allowlisted executable hook. Mock only server/transport boundaries; assert the real plugin manager contains the configured callback after Desktop startup and before TUI gateway.ready.

Automated hermes-sweeper review.

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks @skeptikoss for tracing the three startup paths and showing that hermes hooks doctor could pass while the Desktop backend / TUI gateway process had registered no shell hooks at all — a genuinely misleading failure.

Superseded on main by 3094cde5138 (2026-09-07, "honor configured shell hooks for each chat profile"): tui_gateway/server.py::_build_agent now calls agent.shell_hooks.register_from_config(cfg) with the same profile config it uses to construct that agent, so both the Desktop backend and the standalone TUI gateway (which share this server) register hooks — and they do it per chat profile at agent build time instead of once process-wide at cmd_dashboard/entry.py startup, which is why that shape won for a backend serving several profiles from one process. tests/tui_gateway/test_profile_shell_hooks.py pins it.

Your diagnosis pre-dated the fix and is credited. Reopen if a configured pre_tool_call/pre_llm_call hook still doesn't fire in a Desktop or TUI session on a build ≥ 2026-09-07.

@teknium1 teknium1 closed this Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/profiles Multi-profile isolation, HERMES_HOME scoping comp/cli CLI entry point, hermes_cli/, setup wizard comp/desktop Electron desktop app (apps/desktop/*) comp/tui Terminal UI (ui-tui/ + tui_gateway/) 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.

3 participants