Skip to content

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

Open
5018fhxk wants to merge 1 commit into
NousResearch:mainfrom
5018fhxk:pr/desktop-hooks
Open

5018fhxk wants to merge 1 commit into
NousResearch:mainfrom
5018fhxk:pr/desktop-hooks

Conversation

@5018fhxk

Copy link
Copy Markdown

Problem

serve / dashboard are not in _AGENT_COMMANDS = {None, "chat", "acp", "rl"}, so the CLI-side register_from_config in _prepare_agent_startup (main.py) never runs for the desktop backend. The gateway path registers hooks explicitly (gateway/run.py), but cmd_dashboard has no such call.

Result: the desktop app silently runs WITHOUT any pre_tool_call hooks — even when hooks are configured and allowlisted in cli-config.yaml. That kills every hook-based guardrail for desktop sessions: budget gates, command danger filters, memory routing, skill-creation gates. Hook failures are invisible (no error, no log) — the hooks just never fire.

Fix

Mirror the gateway (gateway/run.py): register declarative shell hooks in cmd_dashboard before start_server, with accept_hooks=False (non-TTY backend → only allowlisted hooks register; consent prompts never fire). Registration failures are logged at debug level and never block startup — same policy as the gateway.

Tests

  • test_cmd_dashboard_registers_shell_hookscmd_dashboard calls register_from_config exactly once with accept_hooks=False
  • test_cmd_dashboard_hook_registration_never_blocks_startup — a failing registration still reaches start_server

Both pass; test_serve_command.py + test_dashboard_lifecycle_flags.py (10 tests) still pass.

serve/dashboard are not in _AGENT_COMMANDS, so the CLI-side
register_from_config in _prepare_agent_startup never runs for the
desktop backend. Result: the desktop app runs WITHOUT any pre_tool_call
hooks (budget gate, danger, memory routing, skill gate) even when
allowlisted in config.yaml — a silent security hole for hook-based
guardrails.

Mirror the gateway (gateway/run.py): register declarative shell hooks in
cmd_dashboard before start_server with accept_hooks=False (non-TTY start
registers only allowlisted hooks; consent prompts never fire). Failures
are logged at debug level and never block startup.

Tests: cmd_dashboard registers hooks exactly once; a failing
register_from_config still lets the server start.
@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) P2 Medium — degraded but workaround exists duplicate This issue or pull request already exists labels Aug 13, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Duplicate of #69832 — both implement the same cmd_dashboard shell-hook registration call with accept_hooks=False for the serve/dashboard startup gap.

@Enough1122

Copy link
Copy Markdown
Contributor

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

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

Fills a real gap (desktop backend running without pre_tool_call hooks); the tests pin the contract well. Observations:

  1. Allowlisted hooks must be TTY-safe in a headless backend. With accept_hooks=False only allowlisted hooks register — but the desktop backend has no console for interactive confirmations. Worth verifying each allowlisted pre_tool_call hook (budget gate, danger, memory routing, skill gate) degrades fail-safe without a TTY (e.g., no prompt-then-hang on the danger gate) rather than assuming the interactive CLI path's behavior.
  2. Failure is logged at DEBUG. The registration is wrapped so failures never block startup, but the failure is reported at logger.debug — the exact silent-absence-of-hooks regression this PR fixes would again be invisible at default log levels. Since a misregistration is a silent behavioral regression, logger.warning would make a recurrence diagnosable.
  3. Double-registration / placement. The call sits unconditionally in cmd_dashboard before start_server, which also covers the interactive hermes dashboard browser surface. If register_from_config is not idempotent and any future path registers hooks again in the same process, hooks could be registered twice — worth confirming idempotency (or that no other code path in this process calls it).

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) duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants