fix(shell-hooks): return bare string from transform_* stdout so response shaping fires (#67890)

[Ahmett101]

Summary

Shell hooks registered for the response-shaping technique (transform_llm_output, transform_tool_result, transform_terminal_output) are documented, allowlisted, and visibly run — but their stdout is silently dropped. _parse_response() forced every event through the {"context": ...} extraction designed for pre_llm_call, and the consumer in turn_finalizer.py reads the hook result via isinstance(_hook_result, str) for replacement — so no shell hook could ever satisfy that check, regardless of what the script printed.

Repro: a transform_llm_output shell hook that prints a JSON string literal leaves the user-visible response unchanged.

Changes

• agent/shell_hooks.py: _parse_response() now returns Optional[Union[str, Dict[str, Any]]]. A transform_* event short-circuits to a bare-string path: a JSON string literal (printf '"replaced text"\n') comes back as a Python str; a dict-shaped payload is unwrapped from a single text or content field as a documented alternative for scripts that want to ship metadata next to the body. Empty body still returns None so the turn keeps the original text.
• agent/shell_hooks.py: _make_callback() return type widened to match. No new public surface — the consumer side already does isinstance(result, str).
• tests/agent/test_shell_hooks.py: 4 new TestParseResponse tests — JSON-string idiom (the repro), dict-with-text, dict-with-content, empty body still no-op. All 21 TestParseResponse tests pass.

How to Test

pytest tests/agent/test_shell_hooks.py::TestParseResponse -q

21/21 passed (4 new: test_transform_llm_output_json_string_returns_bare_str, test_transform_tool_result_dict_with_text_field, test_transform_terminal_output_dict_with_content_field, test_transform_event_no_body_returns_none)

Note: TestCallbackSubprocess (6 tests) fails on Termux because the harness cannot run os.execve on a freshly-written shell-script path. That is a pre-existing environment limitation, unrelated to this PR.

Checklist

• Tests pass — 21/21 (parse); pre-existing subprocess env limit on Termux does not reproduce the fix
• Follows Conventional Commits
• Changes scoped to this fix only
• Cross-platform impact assessed (Linux / macOS / WSL2 / Windows / Termux) — fix is portable; only the verification harness is Termux-limited
• profile-safe paths used
• .env not used for non-credential settings

Risk & Impact: Low. The widened return type is a strict superset (Dict is still returned for pre_tool_call / pre_verify / pre_llm_call), and the consumer's isinstance(result, str) already discriminates; scripts that previously returned None still do, scripts that previously returned a dict still do. Only the previously-silent path (string stdout for transform_*) is now wired.

Type: Bug fix
Closes: #67890

[alt-glitch]

This was generated by AI during triage.

Related to #67890 and #56429 for response-shaping hooks. The hook repair is valid, but this branch also bundles unrelated ACP and cron/dashboard fixes; please split or confirm the combined scope.

[PRATHAMESH75]

Reviewed against #67890. The shell-hooks portion is correct and complete for the reported bug:

• agent/shell_hooks.py::_parse_response() now short-circuits the _TRANSFORM_RESPONSE_EVENTS family before the {"context": ...} extraction and returns a bare str (JSON string literal) or an unwrapped text/content field. The widened Optional[Union[str, Dict[str, Any]]] return type is a clean superset.
• I traced the consumer chain: the shell-hook callback is registered into the plugin manager's _hooks[event] list, so hermes_cli.plugins.invoke_hook("transform_llm_output", ...) collects its raw return unwrapped, and agent/turn_finalizer.py:423 (isinstance(_hook_result, str) and _hook_result) then fires the replacement. So a printf '"replaced text"\n' hook now actually replaces the response — the exact repro in transform_llm_output shell hook is documented but silently non-functional (stdout is dropped) #67890.
• tests/agent/test_shell_hooks.py covers the JSON-string idiom, dict-with-text, dict-with-content, and the empty-body no-op. Good invariant-style coverage.

Two things worth flagging before this lands:

1. Scope — this PR bundles three unrelated changes. The diff also touches cron/jobs.py + hermes_cli/web_server.py (an update-field whitelist that cites Bug: update_job silently persists unknown keys with no validation #67625, a different issue) and acp_adapter/session.py (a list_sessions seen_ids dedup change with no linked issue). That contradicts the PR's own checked box "My PR contains only changes related to this fix" and the repo's one-logical-change-per-PR guideline, and it couples the (clean, ready) transform_llm_output shell hook is documented but silently non-functional (stdout is dropped) #67890 fix to review of two separate concerns. Splitting the cron/Bug: update_job silently persists unknown keys with no validation #67625 work and the ACP change into their own PRs would let the shell-hooks fix merge on its own merits.

2. transform_tool_result / transform_terminal_output have no consumer yet. _parse_response now handles all three _TRANSFORM_RESPONSE_EVENTS, but a grep shows no invoke_hook("transform_tool_result"/"transform_terminal_output", ...) call site — only transform_llm_output is actually consumed (in turn_finalizer.py). The extra two are harmless and forward-looking (the docs' VALID_HOOKS table lists them), just noting they're inert until a caller exists — the doc-vs-behavior gap transform_llm_output shell hook is documented but silently non-functional (stdout is dropped) #67890 raises is only truly closed for transform_llm_output.

Net: the #67890 fix itself is right and I'd be glad to see it merge — ideally on its own, decoupled from the #67625/ACP changes.

[teknium1]

Thanks for identifying a real shell-hook contract mismatch. On current main, agent/shell_hooks.py:596-597 rejects JSON string stdout and agent/shell_hooks.py:617-619 only returns a context dict, while agent/turn_finalizer.py:499-503 only applies a non-empty string replacement. The transform-string approach therefore addresses the reported failure. It also fits the live tool and terminal transform consumers at model_tools.py:1390-1410 and tools/terminal_tool.py:2800-2812.

Problems

• This branch combines three separate fixes: ACP session listing (a7a21b2c6f95), cron/dashboard update-field validation (857fb756acc5), and the shell-hook repair (182942391422). The PR diff spans all three subsystems, so transform_llm_output shell hook is documented but silently non-functional (stdout is dropped) #67890 cannot be reviewed or salvaged independently.

Suggested changes

• Split the shell-hooks commit and its tests into a focused transform_llm_output shell hook is documented but silently non-functional (stdout is dropped) #67890 change; retain the ACP and cron work as separate review units.

This is an automated hermes-sweeper review.

[GottZ]

This was generated by AI during triage.

Summary

Twenty-one PRs address or reference seven related cron/dashboard and shell-hook issues: no-agent run visibility, event-loop starvation, immediate triggering, obsolete root-store routing, update-field validation, shell-transform returns, and pending trigger feedback. Their diffs range from focused fixes to superseded references and bundled cross-subsystem changes.

Related pull requests

• fix(cron): record no_agent runs as sessions so manual triggers show in the Desktop GUI #44087 [closed] best fix — (+191/-3) — n/a: Creates no-agent run sessions before script execution and persists success, failure, and silent outcomes, directly supplying the run-history row, active state, and output missing in [Bug]: Desktop GUI shows no execution status/output after manually triggering no_agent cron jobs #44080; although closed, it remains the canonical precursor duplicated by fix(cron): record no_agent runs as sessions so they show in run history #53692 and extended by fix(cron): record no_agent runs as sessions, keep them out of the sidebar #62247.
• fix(dashboard): offload cron profile scans #45491 [closed] best fix — (+131/-18) — n/a: Moves cron profile and job I/O for the dashboard endpoint family into a threadpool, addressing Dashboard backend blocks its event loop with synchronous work, starving HTTP/WS during desktop boot #45072's confirmed event-loop starvation; although closed, it remains the reference implementation because its commits landed through perf(dashboard): keep session-DB, cron, PID-probe and log-tail I/O off the event loop (salvage #58238/#45491/#53511/#58389/#39140/#53966) #60884.
• Fix cron dashboard trigger-now execution #46956 partial — (+92/-1) — n/a: Calls a profile-retargeted global scheduler tick after trigger_job, which can execute unrelated due jobs; consistent with its keep_open review, salvage requires using _fire_cron_job_for_profile for only the selected job rather than merging this diff as written.
• fix(server): offload cron job listing from dashboard event loop #50978 [closed] partial — (+114/-3) — n/a: Offloads only GET /api/cron/jobs profile enumeration and job reads, covering a narrower portion of Dashboard backend blocks its event loop with synchronous work, starving HTTP/WS during desktop boot #45072 than fix(dashboard): offload cron profile scans #45491; it is closed and remains relevant as the twin of fix(server): offload cron job listing from dashboard event loop #50979.
• fix(server): offload cron job listing from dashboard event loop #50979 [closed] partial — (+114/-3) — n/a: Contains the same listing-only asyncio.to_thread change as fix(server): offload cron job listing from dashboard event loop #50978 and was closed explicitly as a duplicate of the broader fix(dashboard): offload cron profile scans #45491.
• fix(cron): read dashboard jobs from root store #51070 related — (+134/-38) — n/a: Routes dashboard cron operations through the former shared root store, but current main deliberately restored per-profile stores in bb7ff7dc302cbcbe41cf6bc09424ffc9fb2d062f; its keep_open review therefore supports author re-scoping only around a current per-profile defect.
• fix(cron): list_cron_job_runs opens job's own profile state.db #52018 related — (+91/-5) — n/a: Reads run history from a per-job scheduler_profile, but its test manually constructs a cross-profile record unsupported by the current per-profile storage contract; the contributor keep_open review blocks this routing model pending maintainer direction.
• fix(cron): record no_agent runs as sessions so they show in run history #53692 [closed] best fix — (+193/-3) — n/a: Implements the same pre-execution no-agent session lifecycle as fix(cron): record no_agent runs as sessions so manual triggers show in the Desktop GUI #44087, including failure and silent paths; it remains relevant despite being closed because its author explicitly closed it as a duplicate of fix(cron): record no_agent runs as sessions so manual triggers show in the Desktop GUI #44087.
• fix(dashboard): run residual cron profile I/O off the event loop (#50948 follow-through) #61385 [merged] partial — (+96/-2) — n/a: This merged reference moves the residual cron fire-webhook lookup and blueprint creation I/O off the event loop, completing sites left after the main Dashboard backend blocks its event loop with synchronous work, starving HTTP/WS during desktop boot #45072 threadpool migration.
• fix(cron): record no_agent runs as sessions, keep them out of the sidebar #62247 best fix — (+207/-8) — n/a: Recorded best fix for [Bug]: Desktop GUI shows no execution status/output after manually triggering no_agent cron jobs #44080, combining pre-script session creation for every no-agent outcome with exclusion of selected cron sessions from normal recents; its keep_open verdict calls for replacing the placeholder issue reference and adding a desktop exclusion regression.
• fix(cron): record no_agent runs as sessions so they show in run history #62587 partial — (+199/-2) — n/a: Records only successful no-agent runs with visible stdout after completion, preserving the no-SessionDB fast paths but leaving failures, silent runs, and the requested in-flight indicator without session history; its keep_open review also requires an explicit decision on the no-agent DB contract.
• perf(dashboard): lightweight profile listing — multi-profile session list 10s → ~1.5s #62699 [closed] partial — (+100/-11) — n/a: Avoids expensive full profile metadata scans by introducing a lightweight name/home listing, but it was closed after current main addressed the measured performance pathology through d5eee133e and session-query changes.
• fix(cron): persist no_agent runs to the profile session DB #66954 [closed] partial — (+354/-67) — n/a: Persists every no-agent outcome after execution with distinct microsecond IDs and failure detail, but cannot provide the immediate running row created before execution by the recorded [Bug]: Desktop GUI shows no execution status/output after manually triggering no_agent cron jobs #44080 best-fix variants; it remains a closed alternative implementation.
• fix(cron): whitelist cron update_job fields so typos don't silently persist (#67625) #67646 [closed] partial — (+163/-2) — n/a: Adds core and dashboard unknown-field allow-lists for Bug: update_job silently persists unknown keys with no validation #67625 but bundles an unrelated ACP session fix; it is closed and superseded by fix(cron): whitelist cron update_job fields so typos don't silently persist (#67625) #67660's equivalent live diff and review path.
• fix(cron): whitelist cron update_job fields so typos don't silently persist (#67625) #67660 best fix — (+163/-2) — n/a: Recorded best fix for Bug: update_job silently persists unknown keys with no validation #67625 because it rejects unknown fields in both core and dashboard paths, but its blocking contributor review requires splitting ACP, validating metadata as a dict, aligning attach_to_session, and rejecting unknown gateway REST keys before sanitization.
• fix(shell-hooks): return bare string from transform_* stdout so response shaping fires (#67890) #67939 fixes — (+234/-6) — n/a: Its shell-hook portion correctly returns bare strings for transform events and addresses transform_llm_output shell hook is documented but silently non-functional (stdout is dropped) #67890, but the visible keep_open verdict requires splitting out the unrelated ACP and cron-validation changes; the focused hook repair already exists in fix(hooks): return shell transform results (#67890) #67949.
• fix(hooks): return shell transform results (#67890) #67949 best fix — (+95/-4) — n/a: Recorded best fix for transform_llm_output shell hook is documented but silently non-functional (stdout is dropped) #67890, narrowly adding direct-string and event-specific object returns for all three transform consumers with parser, configured-shell integration, and documentation coverage; its keep_open verdict supports retaining this focused review unit.
• fix(cron): make Trigger now execute immediately and safely #70638 best fix — (+2632/-171) — n/a: Recorded best fix for Cron jobs dashboard: "Trigger now" button doesn't execute job, Run History shows "No runs yet" #46918 because it fires only the selected job through a provider-backed, owner-fenced path and adds cross-client guards, but its keep_open review requires preserving current main commit cff9728587da4f3c0beed0786f9bea528e489f13's BaseException terminal guarantee.
• fix(desktop): show pending cron trigger feedback #70833 [closed] best fix — (+390/-26) — n/a: Adds synchronous duplicate-click suppression, immediate busy feedback, and a queued run-history row for [Bug]: Desktop Cron Trigger now accepts duplicate clicks and delays run feedback #70826; although closed, it remains the best-fix precursor superseded by fix(desktop): show pending cron trigger feedback #70840's current-main reconciliation work.
• fix(desktop): show pending cron trigger feedback #70840 best fix — (+657/-22) — n/a: Recorded best fix for [Bug]: Desktop Cron Trigger now accepts duplicate clicks and delays run feedback #70826, retaining queued feedback after pause and integrating serialized pending polling with cron.changed, the event-capable backstop, job-state settlement, and regression coverage; this directly addresses the earlier blocking review concerns.
• fix(cron): reject unknown fields in update_job instead of silently persisting (#67625) #76831 partial — (+71/-0) — n/a: Provides the smallest core-only Bug: update_job silently persists unknown keys with no validation #67625 allow-list, but its keep_open review identifies two contract gaps in the visible diff: existing description compatibility is rejected and mixed unknown gateway REST fields are still silently filtered.

Duplicates

#53692 duplicates #44087; #50978 and #50979 are twins and narrower duplicates of #45491; #67646 is superseded by #67660, while #67939 bundles the same cron-validation change; #67939's shell-hook portion duplicates the focused #67949; #70833 is superseded by #70840. #62247, #62587, and #66954 overlap the no-agent history work but differ in timing, covered outcomes, and sidebar handling.

Suggested consolidation

Author action on #67939: split agent/shell_hooks.py and tests/agent/test_shell_hooks.py from the unrelated ACP and cron/dashboard changes, as required by its high-salvageability keep_open verdict; because #67949 is the recorded focused best fix for #67890, the extracted hook portion can defer to #67949 rather than creating a second review lane. Keep #62247, #67660, #67949, #70638, #70840, and #76831 open only along their explicit review salvage paths, have #46956 defer to #70638's selected-job provider design, and retain the closed duplicate chains #53692 → #44087, #50978/#50979 → #45491, #67646 → #67660, and #70833 → #70840.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I67625(["issue #67625 (open)"])
    I67890(["issue #67890 (open)"])
    subgraph Dup67939 ["PRs duplicating each other"]
        P67939["PR #67939 (open)"]
        P67949["PR #67949 (open)"]
    end
    P67939 -.->|partial| I67625
    P67939 -->|fixes| I67890
    class I67625 open
    class I67890 open
    class P67939 open
    class P67949 open
    class P67949 best
    class P67939 target
    click I67625 "https://github.com/NousResearch/hermes-agent/issues/67625"
    click I67890 "https://github.com/NousResearch/hermes-agent/issues/67890"
    click P67939 "https://github.com/NousResearch/hermes-agent/pull/67939"
    click P67949 "https://github.com/NousResearch/hermes-agent/pull/67949"

Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 21 pull requests and 7 issues in this complex. Each diff was read against this issue; Assessment working set: 390 kB of PR diffs, 66 kB of issue/PR text, 38 kB of discussion (46 comments), 34 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.