Skip to content

fix(serve): register shell hooks on dashboard/serve startup - #102151

Open
LiTerBo wants to merge 1 commit into
NousResearch:mainfrom
LiTerBo:fix/serve-dashboard-shell-hooks
Open

LiTerBo wants to merge 1 commit into
NousResearch:mainfrom
LiTerBo:fix/serve-dashboard-shell-hooks

Conversation

@LiTerBo

@LiTerBo LiTerBo commented Sep 3, 2026

Copy link
Copy Markdown

Problem

The desktop app and remote backends run agent turns through serve's in-process /api/ws gateway, but shell-hook registration lived only in _prepare_agent_startup, gated on _AGENT_COMMANDS = {None, "chat", "acp", "rl"} (+ cron/gateway/mcp subcommands). serve/dashboard are deliberately absent from that set, so lifecycle hooks (e.g. mnemon prime/remind/nudge) never registered on the headless path.

Fix

Extract a shared _register_shell_hooks() helper and call it from both:

  • _prepare_agent_startup (interactive agent entrypoints), and
  • cmd_dashboard (the headless serve backend + dashboard)

so hooks fire regardless of how the agent turn is hosted.

Tests

  • test_serve_registers_shell_hooks: serve registers hooks on startup
  • test_register_shell_hooks_wires_shell_and_outbound: the helper wires both shell and outbound webhooks

Context

Follow-up to #92655, which added the shared registration path but left serve/dashboard gated out.

The desktop app and remote backends run agent turns through serve's in-process /api/ws gateway, but shell-hook registration lived only in _prepare_agent_startup, gated on _AGENT_COMMANDS = {None, chat, acp, rl} (+ cron/gateway/mcp subcommands). serve/dashboard are deliberately absent from that set, so lifecycle hooks (mnemon prime/remind/nudge) never registered on the headless path.

Extract a shared _register_shell_hooks() helper and call it from both _prepare_agent_startup (interactive entrypoints) and cmd_dashboard (serve + dashboard), so hooks fire regardless of how the agent turn is hosted.

Adds tests pinning that serve registers hooks and that the helper wires both shell and outbound webhooks.
@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/dashboard Web dashboard / control panel UI (dashboard/, landing) sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Sep 3, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

Chunk5 review — fix(serve): register shell hooks on dashboard/serve startup

Right fix for a real gap: serve hosts agent turns via in-process /api/ws but was excluded from the _AGENT_COMMANDS gate, so lifecycle hooks silently never fired there.

  • hermes_cli/main.py:12401-12408 — registering with accept_hooks=False on the headless path is the safe choice (no interactive acceptance). Correct.
  • hermes_cli/main.py:12662-12696 — extracting _register_shell_hooks removes the duplication rather than adding a second copy. The refactor of _prepare_agent_startup to call it preserves the _accept_hooks behaviour on interactive paths. Verified no behaviour change there.
  • tests/hermes_cli/test_dashboard_shell_hooks.py:1-102 — regression test names the exact gate (_AGENT_COMMANDS exclusion) so a future refactor that re-breaks this fails loudly.
  • Non-blocking: confirm register_from_config is idempotent, since cmd_dashboard + a later _prepare_agent_startup in the same process would now run it twice (the docstring claims idempotency — trusting it, just flagging the double-registration path exists).

Non-blocking. Merge-ready.

@kvnloo

kvnloo commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

exact-head c1ae41e — void if moved

CHECK / pick-one: serve-path registration via hermes_cli/main.py only. Prefer #111315 (TUI+serve+shared helper+parity test).

Recommend: close as duplicate of the #111315 consolidation, or fold the test_dashboard_shell_hooks coverage into #111315 if anything unique remains.

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

comp/cli CLI entry point, hermes_cli/, setup wizard comp/dashboard Web dashboard / control panel UI (dashboard/, landing) P2 Medium — degraded but workaround exists 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.

4 participants