Skip to content

fix(tui_gateway): register shell hooks in _make_agent so they dispatch interactively - #67084

Open
Kailigithub wants to merge 1 commit into
NousResearch:mainfrom
Kailigithub:fix/issue-67053-tui-gateway-shell-hook-registration
Open

fix(tui_gateway): register shell hooks in _make_agent so they dispatch interactively#67084
Kailigithub wants to merge 1 commit into
NousResearch:mainfrom
Kailigithub:fix/issue-67053-tui-gateway-shell-hook-registration

Conversation

@Kailigithub

Copy link
Copy Markdown
Contributor

Problem

Shell hooks configured under hooks: in ~/.hermes/config.yaml silently never fired during interactive hermes chat sessions. hermes hooks doctor / list / test reported healthy, but the live run dispatched only the built-in plugin callbacks — every configured pre_llm_call / on_session_start / etc. hook was a no-op.

Root cause

register_from_config() is called from hermes_cli/main.py and cli.py (the transient launcher process), but not from anywhere in the tui_gateway daemon that actually runs the agent conversation. PluginManager._hooks is an in-memory per-process dict, so the launcher's registrations are invisible to the gateway. The daemon builds agents through tui_gateway/server.py::_make_agent without ever wiring hooks up.

hermes hooks test is misleading here — it invokes the configured script directly via shell_hooks.run_once(), bypassing the plugin dispatcher entirely. The script runs fine; real events never reach it.

Fix

Add a small helper _register_shell_hooks_from_config() in tui_gateway.server that loads config.yaml and calls register_from_config(), and invoke it inside _make_agent under the existing try/except pattern used by neighboring init steps (e.g. wait_for_mcp_discovery).

register_from_config is idempotent (deduped on (event, matcher, command)), so launcher + gateway calls register the hook exactly once. HERMES_SAFE_MODE=1 is honored inside register_from_config and turns this into a no-op.

This mirrors the issue reporter's verified fix (see #67053 root cause analysis) — placement in _make_agent was chosen over tui_gateway/entry.py:main so the gateway's two agent-construction paths (inline and compute-host) both pick it up.

Reproduction

  1. Add a shell hook to ~/.hermes/config.yaml, e.g.:
    hooks:
      on_session_start:
        - command: "/abs/path/to/hook.sh"
    (hook.sh: printf '{}\n')
  2. Allowlist: hermes hooks allow /abs/path/to/hook.sh
  3. hermes --accept-hooks chat → send one message
  4. Before fix: hook callback absent from PluginManager._hooks in the daemon process; script never executes.
  5. After fix: register_from_config runs in _make_agent, callback is wired up, hook fires.

Verification

Three targeted regression tests in tests/tui_gateway/test_issue_67053_shell_hook_registration.py:

  • test_register_shell_hooks_helper_exists_and_registers — calls the patched helper directly, asserts the configured hook shows up on PluginManager._hooks. Fails on main (helper doesn't exist); passes with the fix.
  • test_register_shell_hooks_helper_is_idempotent — two calls register exactly one callback, confirming launcher + gateway calls dedupe safely.
  • test_safe_mode_skips_registrationHERMES_SAFE_MODE=1 short-circuits the helper, matching the safety guarantee register_from_config already provides.

Iron Rule 1 verified via the stash three-step dance: stash the source fix → test_register_shell_hooks_helper_exists_and_registers fails on main; pop the fix → all three tests pass.

$ python3 -m pytest tests/tui_gateway/test_issue_67053_shell_hook_registration.py -v
tests/tui_gateway/test_issue_67053_shell_hook_registration.py::test_register_shell_hooks_helper_exists_and_registers PASSED
tests/tui_gateway/test_issue_67053_shell_hook_registration.py::test_register_shell_hooks_helper_is_idempotent PASSED
tests/tui_gateway/test_issue_67053_shell_hook_registration.py::test_safe_mode_skips_registration PASSED
3 passed

Out of scope

  • Re-running register_from_config on config.yaml reload events — the current call-on-_make_agent model is enough for the common case (one _make_agent per session). Config-reload mid-session is a separate concern.
  • Adding telemetry for "hook registered vs dispatched" — out of scope for a bug fix.

Closes #67053

…h interactively

Shell hooks configured under the hooks: block in config.yaml were
invisible to the tui_gateway daemon process: register_from_config only
ran in the transient hermes chat launcher, but PluginManager._hooks
is per-process, so the long-lived daemon that actually runs the
agent conversation never saw the configured callbacks and
configured hooks silently never fired.

Add a small helper in tui_gateway.server that loads config.yaml and
calls register_from_config, and invoke it inside _make_agent under
the existing try/except pattern used by neighboring init steps.
register_from_config is idempotent so launcher + gateway calls
dedupe to a single registration. HERMES_SAFE_MODE=1 is honored
inside register_from_config and turns this into a no-op.

Fixes NousResearch#67053
@alt-glitch alt-glitch added type/bug Something isn't working comp/tui Terminal UI (ui-tui/ + tui_gateway/) P3 Low — cosmetic, nice to have duplicate This issue or pull request already exists labels Jul 18, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #48770: the live diff registers configured shell hooks in tui_gateway.server::_make_agent, matching the earlier open interactive-TUI repair. Related report: #67053.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for covering the TUI-process registration gap. The premise is confirmed on current main: tui_gateway/server.py:4808-4829 builds an agent without shell-hook registration, while agent/conversation_loop.py:362-374 dispatches on_session_start through the process-local plugin manager.

Problems

  • tests/tui_gateway/test_issue_67053_shell_hook_registration.py:79 invokes _register_shell_hooks_from_config() directly. It does not prove that _make_agent() calls it at PR tui_gateway/server.py:4882; removing that production call would leave the added tests green.

Suggested changes

  • Add a factory-level regression using the existing _make_agent() test setup at tests/test_tui_gateway_server.py:8796, asserting that an agent build invokes shell-hook registration with the loaded config. Retain the helper idempotence and safe-mode coverage.

The patch otherwise follows the established register_from_config contract in agent/shell_hooks.py:203-284, and applies cleanly to current main. Automated hermes-sweeper review.

"_register_shell_hooks_from_config"
)

server._register_shell_hooks_from_config()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This verifies the helper but not its production wiring. Please add a test that calls _make_agent() through its existing factory seams and asserts it invokes this registration path; otherwise removal of the _make_agent call remains undetected.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data 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 labels Jul 19, 2026
@Kailigithub

Copy link
Copy Markdown
Contributor Author

Re-confirming the relationship to #48770 for maintainer clarity (autopilot housekeeping note, not a close candidate):

Both PRs add the same root-cause fix (call register_from_config() in tui_gateway.server._make_agent). The shapes differ:

The triage bot's Duplicate of #48770 note is technically correct on file/topic but undersells the test coverage this PR adds. Keeping this PR open; if a maintainer prefers to land #48770 first and cherry-pick the test suite from this branch, that's a happy path too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tui Terminal UI (ui-tui/ + tui_gateway/) duplicate This issue or pull request already exists 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 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.

Shell hooks register in the launcher but never dispatch in interactive hermes chat — tui_gateway daemon doesn't call register_from_config

3 participants