fix(hooks): session-owned profile-keyed shell hooks for dashboard/TUI

[Ne0teric]

Problem

Shell hooks (pre_llm_call / post_llm_call) only worked from the CLI on Windows. Two independent issues:

1. Hooks never registered outside the CLI startup path. register_from_config() was only called on CLI startup. The dashboard backend (cmd_dashboard) and the TUI entrypoint (tui_gateway/entry.py) never called it — so the desktop app (which launches hermes dashboard as its backend) and the TUI registered zero hooks and silently injected/saved nothing. No error — the hooks just never fired. Confirmed via agent.log: a desktop/dashboard restart produced no shell hook registered line, only CLI sessions did.

2. Hook I/O crashed on non-ASCII on Windows. _spawn() ran the hook with subprocess.run(..., text=True) and no explicit encoding, so it used the platform locale codec (cp1252 on Windows). Hook output containing non-ASCII (em dash, arrows, etc.) raised 'charmap' codec can't encode character ... and failed the hook every turn.

Fix

• Call register_from_config() at dashboard (cmd_dashboard) and TUI (tui_gateway/entry.py) startup, matching the CLI path. register_from_config already resolves hooks_auto_accept internally, so the no-TTY dashboard/gateway auto-accepts correctly.
• Pass encoding="utf-8" to the _spawn() subprocess.run so hook stdin/stdout is UTF-8 on every platform.

24 insertions, 0 deletions across 3 files.

Testing

tests/agent/test_shell_hooks.py on Windows: 46 passed, 7 failed — and the same 7 fail on clean main without this patch (verified by running the suite against unpatched main). They're pre-existing Windows-only test-harness limitations (bash-shebang scripts can't execute under shell=False; os.access(path, X_OK) has no meaning on Windows), unrelated to this change — it adds zero new failures. The 46 passing tests cover registration, allowlisting, config parsing, payload serialization, and idempotence.

Behavior confirmed on a Windows desktop install: before the fix the hermes dashboard backend registered zero hooks; after, it registers both and the pre_llm_call hook injects context each turn.

[alt-glitch]

This was generated by AI during triage.

Related: this fixes the same TUI/Desktop shell-hook-registration gap as the open cluster anchored by earliest-open #13854 (also #34112, #24237, #28953, #41464; issue #41457). This PR is not a duplicate — it adds net-new coverage (dashboard backend registration + a Windows UTF-8 hook-I/O fix) beyond the TUI-only competitors. Flagging the cluster so a maintainer can pick a canonical PR.

[tonydwb]

Code Review Summary

Verdict: Approved

Clean fix for shell hooks not firing on dashboard and TUI/desktop startup. The hooks (pre_llm_call / post_llm_call) were only registered in the CLI and gateway paths.

Changes:

• Dashboard startup registers hooks via register_from_config(load_config(), accept_hooks=False)
• TUI/desktop startup registers hooks via the same pattern with hooks_auto_accept config support
• encoding="utf-8" added to subprocess calls in shell_hooks.py for consistent behavior
• Fail-soft: hook registration failures are logged at debug level and don't block startup

The accept_hooks=False for dashboard is correct — dashboard processes user commands, not interactive agents. The TUI's accept_hooks=_auto respects the user's config preference.

No concerns. Ready to merge.

---

Reviewed by Hermes Agent

[Ne0teric]

Friendly ping on this one — it was approved by @tonydwb on Jun 28 but hasn't landed yet. Just re-verified it still merges cleanly against current main (no conflicts with the recent gateway/auxiliary refactors). Happy to rebase or adjust if anything's needed from my side.

For context: without this fix, config-defined shell hooks silently never fire for dashboard/TUI/desktop sessions (only CLI gets them), and hook scripts that emit non-ASCII output crash on Windows' cp1252 default.

[teknium1]

Thanks for addressing two real shell-hook gaps. Current main still lacks TUI registration (tui_gateway/entry.py:293-349) and _spawn() still uses text=True without an encoding (agent/shell_hooks.py:462-470).

Problems

• The dashboard insertion needs current-main placement and scoping. cmd_dashboard() supports a multi-profile backend, while tui_gateway/server.py:995-1017 and :1341-1353 bind sessions to other profile homes. Registering launch-profile hooks for every dashboard process is not profile-safe. It must also run after _maybe_setup_dashboard_auth_interactively() (hermes_cli/main.py:12202), because that flow can force plugin rediscovery; hermes_cli/plugins.py:1292 clears hooks on force reload.
• The TUI hunk's bool(_hooks_cfg.get("hooks_auto_accept", False)) bypasses the resolver in agent/shell_hooks.py:834-854: a quoted "false" is truthy. Pass accept_hooks=False and let register_from_config() parse config/env consistently.
• The PR adds no tests for its three new paths.

Suggested changes

• Salvage dashboard registration after auth setup, immediately before start_server(), gated by HERMES_DESKTOP == "1"; register standalone TUI hooks before gateway.ready.
• Add focused startup-order and UTF-8 subprocess-kwargs tests.

Automated hermes-sweeper review.

[Ne0teric]

Addressed the hermes-sweeper review (keep_open / salvageability=medium) on current main:

Changes

1. Dashboard registration — runs only when HERMES_DESKTOP=1 (Desktop per-profile backends), after _maybe_setup_dashboard_auth_interactively() and immediately before start_server(). Avoids stamping launch-profile hooks onto multi-profile machine dashboards, and survives auth-path force rediscovery clearing hooks.
2. TUI entry — registers before gateway.ready with accept_hooks=False so register_from_config() owns hooks_auto_accept / HERMES_ACCEPT_HOOKS parsing (no bool(cfg.get(...)) truthiness trap for quoted "false").
3. UTF-8 — still includes encoding="utf-8" on _spawn() subprocess I/O (Windows cp1252).
4. Tests
   • TestSpawnEncoding — subprocess kwargs include encoding="utf-8"
   • TestRegisterAcceptHooksResolution — "false" string is not auto-accept; bool True still is
   • tests/hermes_cli/test_shell_hooks_startup_registration.py — Desktop dashboard order is auth → register → start; non-Desktop skips register; TUI registers before ready with accept_hooks=False

Local: 6 passed for the new tests.

[Ne0teric]

Rebased onto current main (f3cda0ceb).

Notes from the rebase:

• UTF-8 hook I/O is already on main (encoding='utf-8', errors='replace' in _spawn). Dropped that production delta; kept a regression test that spawn still passes encoding=\"utf-8\".
• Remaining delta is the actual gap: desktop (HERMES_DESKTOP=1) dashboard registration after interactive auth / before start_server, and standalone TUI entry registration before gateway.ready with accept_hooks=False.
• Startup-order + accept-resolution tests green (6/6). Pre-existing Windows .sh subprocess tests still fail locally (bash scripts not found) — unrelated.

Happy to adjust further if anything's needed.

[Ne0teric]

CI fix: session-boundary mocks now accept profile_home= kwarg (3 tests).

[d31tcjg]

I independently reproduced the missing shell-hook registration path on macOS Hermes Desktop and live-validated that making configured hooks available to the dashboard backend restores lifecycle callbacks. I am adding the evidence here rather than opening another PR because this PR uses the stronger session/profile-owned design needed by the multi-profile dashboard architecture.

Pinned reproduction environment:

• tested upstream main: de0abc0617794a8c4ae661d2f67f4b23d4ab51fe
• current main observed during this draft update: d6a5cb9725df4b3d14a61aa7a2f717acb30c901b
• current PR head: 9f6ca20c7d34c889fe479bf8692116ba39e7c964
• GitHub currently reports the PR open, rebaseable, and mergeable/clean

Validation on the pinned reproduction SHA:

• the dashboard startup path did not make configured shell hooks available to Desktop sessions
• a narrow startup registration reproduction restored prompt, tool, approval, and completion callbacks in a real Desktop session
• tests/test_resource_limits.py: 22/22 passed
• changed files compiled; focused Ruff and git diff --check passed
• removing the legacy hook configuration and fully restarting Desktop produced zero residual callbacks

The live result confirms the user-visible defect and the need for dashboard/serve registration. It does not, by itself, validate the current PR head against today’s main; upstream advanced after my pinned reproduction, so this should be treated as runtime evidence rather than a current merge-readiness verdict.

The profile/session ownership in this PR remains important: one dashboard process can serve multiple profiles, so registration plus reset/end/finalize dispatch should stay scoped to the owning profile. I have kept my narrow startup-global reproduction unpublished and will not open a competing PR.