fix(cron): end-to-end fix for cron session rendering on Hermes Desktop - #43233
fix(cron): end-to-end fix for cron session rendering on Hermes Desktop#43233ValentinSergief wants to merge 1 commit into
Conversation
ac4ee27 to
233fba9
Compare
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Looks Good:
- End-to-end fix for cron session rendering on Hermes Desktop. The issue was that cron job output was not being properly streamed to the Desktop chat interface, causing blank or missing session content.
- Fixes include proper handling in the desktop controller, session message streaming, and cron scheduler.
- 8 files, +140/-17. Changes span desktop React components, cron/scheduler.py, hermes_state.py, and tui_gateway.
- Includes test coverage.
Reviewed by Hermes Agent
|
Verified compatible with #44630 (composer status stack + subagent windows). Tested in local build with both sets of changes — no conflicts, all features coexist cleanly. |
233fba9 to
7138c2f
Compare
7138c2f to
5216f7e
Compare
5216f7e to
159ed67
Compare
159ed67 to
60c9e6b
Compare
2c35d05 to
ff4d42a
Compare
ff4d42a to
eaeab2c
Compare
9292765 to
db17aa9
Compare
db17aa9 to
d7c9696
Compare
d7c9696 to
095c002
Compare
095c002 to
fc45fe4
Compare
fc45fe4 to
b8254fb
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the Desktop resume pipeline. Two current-head behaviors support the core report: tui_gateway/server.py:4939-4950 drops empty assistant tool-call frames, and apps/desktop/src/app/session/hooks/use-session-actions/index.ts:550-559 prefers any non-empty REST snapshot over the resume payload.
Problems
cron/scheduler.py:3208in this PR changes shutdown towait=True, but the inactivity branch callsagent.interrupt()only after thisfinallyblock. A hung future will therefore block before the watchdog can interrupt it._cron_emitis initialized toNoneatcron/scheduler.py:2771and is never assigned in this diff; the new completion branches cannot emit._cron_emit_mapis also not defined by the patch.- The PR changes
list_cron_job_runssemantics withended_at IS NOT NULLbut adds no test, and the stated SQLitesynchronous=FULLchange is not present in the diff. - No regression tests are included. Current
tests/test_tui_gateway_server.py:915-941explicitly expects the tool-call-only assistant frame to be omitted.
Suggested changes
- Preserve a non-blocking, bounded inactivity cleanup path before waiting on the worker.
- Rework or remove the incomplete cron event-emitter wiring, then add lifecycle coverage.
- Add serializer and Desktop resume-order regression tests; cover any intended active-versus-completed cron-run listing behavior.
Automated hermes-sweeper review.
|
Comment anchored to an earlier commit. The current head (bc66d193) uses |
|
One more review point for the record: the |
Cron sessions on Hermes Desktop were unviewable after a run: tool calls and the LLM response stayed invisible until a gateway restart. Two bugs: 1. cron/scheduler.py — pool.shutdown(wait=True) runs before the inactivity watchdog branch calls agent.interrupt(). On a hung future, shutdown blocks forever and the interrupt never runs. Both shutdown sites now use wait=False + cancel_futures=True, so the watchdog always stays reachable. 2. tui_gateway/server.py — _history_to_messages dropped every assistant frame with empty content (i.e. every tool-invocation frame), and the synthesized tool frames lacked tool_call_id, so the desktop resume payload lost the link between a tool result and the call that produced it. Assistant frames carrying tool_calls are now preserved, and the synthesized tool frames carry the original tool_call_id. The raw tool output is deliberately NOT forwarded — the link is kept, the leak is not (tool output can contain secrets; the desktop transcript stays redacted). Tests: test_watchdog_can_fire_after_shutdown_does_not_block, test_history_to_messages_preserves_tool_calls_for_resume_display, test_history_to_messages_preserves_tool_calls_on_assistant_frame. PR: NousResearch#43233
Problem
Cron sessions on Hermes Desktop are unviewable after the run: clicking a completed cron job shows only the user prompt, with tool calls and the LLM response invisible until a full gateway restart. Two bugs in the resume pipeline cause it.
Root cause
cron/scheduler.py— shutdown could block the inactivity watchdog —pool.shutdown(wait=True)runs before the_inactivity_timeoutbranch callsagent.interrupt(). If the future is the hung operation the watchdog detected, shutdown blocks indefinitely and the interrupt never runs.tui_gateway/server.py— tool-call-only assistant frames dropped —_history_to_messagesskips every assistant message with emptycontent, which is every tool-invocation frame. The resume payload loses the tool chain — and worse, the synthesizedtoolframes don't carrytool_call_id, so the desktop can't pair a tool result with the call that produced it.Fix
wait=False(+cancel_futures=True) on both_cron_pool.shutdownsites — the inactivity branch is always reachable and is responsible for killing stuck agents. Update (2026-08-19): this half is now also present upstream, landed via a separate contributor's PR; it is kept here for the record. The tool-call half below is the still-novel part._history_to_messagespreserves assistant frames that carrytool_callseven with emptycontent, and the synthesizedtoolframes now carry the originaltool_call_id. The raw result payload is deliberately NOT forwarded — tool output can contain secrets, and the desktop transcript stays redacted. The link is kept, the leak is not.Tests
test_watchdog_can_fire_after_shutdown_does_not_block— regression for the shutdown orderingtest_history_to_messages_preserves_tool_calls_for_resume_display+test_history_to_messages_preserves_tool_calls_on_assistant_frame— tool-call frames and the id link survivetest_cron_inactivity_timeout.py; serializer tests intest_tui_gateway_server.py