fix(desktop): keep quiet sessions visibly running - #65870
Conversation
Code Review SummaryVerdict: Comment Desktop sidebar running watchdog fix. Keeps quiet sessions visibly running. +78/-82. -- |
tonydwb
left a comment
There was a problem hiding this comment.
LGTM! Fix keeps quiet sessions visibly running. Clean fix for the watchdog timer handling.
Reviewed by Hermes Agent
c73c192 to
b0e2998
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for identifying the quiet-turn and reconnect behavior; the current-main watchdog still clears an otherwise live session after eight minutes of silence.
Problems
- This branch predates #66454 (
cf52edbb5), which moved working/attention state into computed projections of$sessionStatesinapps/desktop/src/store/session-states.ts:193-208. The PR imports mutablesession.tsAPIs that current main no longer provides. - On current main, the behavior to replace spans
apps/desktop/src/store/session-states.ts:58-70andapps/desktop/src/app/session/hooks/use-session-state-cache.ts:290-307: the timeout invokes a callback that publishesbusy: falseand clears awaiting/input state.
Suggested changes
- Rework the quiet presentation and
session.active_listreconciliation through the current cache pluspublishSessionState, preservingbusyuntil an authoritative terminal event. - Port the regressions to
apps/desktop/src/store/session-watchdog.test.ts, where the current watchdog contract is tested at lines 169-195.
This is an automated hermes-sweeper review.
| @@ -1,7 +1,16 @@ | |||
| import { useEffect } from 'react' | |||
|
|
|||
| import { refreshActiveProfile } from '@/store/profile' | |||
| import { $activeSessionId, $currentCwd, setCurrentCwd } from '@/store/session' | |||
| import { | |||
There was a problem hiding this comment.
Current main no longer exposes these mutable session-status APIs: #66454 moved working/attention to computed projections in apps/desktop/src/store/session-states.ts:193-208. Please adapt recovery through the current cache/state publisher rather than restoring independent atoms.
4680097 to
cd327cb
Compare
|
Refreshed onto current main after the #66454 session-state refactor. The old stale implementation has been replaced with the already-adapted three-commit stack using |
cd327cb to
6b02541
Compare
|
CI correctly caught one cumulative-branch dependency on the still-unmerged #67118 helper |
6b02541 to
5a97d94
Compare
|
Follow-up: the reconnect commit itself also carried the historical helper diff, so the first autosquash corrected an intermediate commit but not the final snapshot. That later commit is now corrected too. Verified final head |
|
Final CI confirmation for head |
5a97d94 to
aee688d
Compare
austinpickett
left a comment
There was a problem hiding this comment.
Review — approve
Verified against head aee688d9203b32bd731cb3661270f20247b4910c in a fresh worktree on top of a full local checkout.
Prior review reconciliation
The hermes-sweeper review (2026-07-18, keep_open salvageability=medium) flagged that the branch predated #66454 and imported mutable session-status APIs that no longer exist. Both concerns are resolved at the current head:
- The old
setWatchdogClearFnwiring inuse-session-state-cache.tsand the mutable-clear path insession-states.tsare removed; the implementation is re-homed on the post-#66454 architecture ($sessionStates+publishSessionState, with$workingSessionIds/$attentionSessionIdsstaying computed projections). - The regressions were ported to
src/store/session-watchdog.test.tsas requested, and the new stalled contract is covered there (silent turn stays in$workingSessionIds, activity re-arms, terminal transition clears both, gateway wipe clears both).
Correctness spot-checks
- Design is right per the app's authority model: the 8-minute watchdog becomes a presentation hint (
$stalledSessionIds) and never mutates backend-derivedbusy. Silence ≠ completion — long tool calls legitimately go quiet. Only an authoritative terminal transition (or the authoritativesession.active_listsnapshot reporting non-working) clears the running state, which also means a genuinely finished-while-disconnected session gets cleared by the 1.5 s poll rather than by a heuristic timer. - Gateway contract verified against current
main'stui_gateway/server.py:session.active_listis non-mutating (snapshots_sessions, filters_finalized), and_session_live_itemreally does emitid,session_key,last_active(epoch seconds — the* 1000conversion inrehydrateLiveSessionStatusesis correct), andstatus ∈ {idle, starting, waiting, working}from_session_live_status, which matches the renderer's union exactly.waiting→busy && needsInputmirrors how live stream events treat a blocking prompt. - Poll hygiene:
visiblePoll+inFlightguard + publish-only-on-diff means the 1.5 s cadence doesn't re-arm the watchdog or churn atoms on no-ops; thecatch-and-ignore keeps older gateways withoutsession.active_listworking unchanged.activeGatewayProfilein the effect deps correctly re-seeds after a profile swap. - Stalled bookkeeping is keyed by stored id while timers stay keyed by runtime id, and every clear path is covered: busy publish, terminal transition (both
nextandpreviousstored ids — handles compression rotation),dropSessionState,clearAllSessionStates, and the gateway-switch wipe. sessionDotState/sessionShowsRunningArcextraction is a clean DI-testable move consistent with the repo's "never read source in tests" rule.
Local verification (worktree at PR head, Windows)
npx vitest run --project uion the five touched suites: 36/36 passnpm run typecheck(bothtsc -p .andtsconfig.electron.json): cleannpx eslintacross all 11 changed files: clean- Upstream CI on this head: all required checks green (
All required checks pass✅),mergeable: MERGEABLE, merge stateCLEAN— no conflicts, no salvage needed.
One non-blocking note (fine as follow-up)
rehydrateLiveSessionStatuses restores/reconciles rows present in the snapshot but doesn't clear a renderer-side working row whose session was finished and reaped (torn down) while Desktop was disconnected — such a session is absent from session.active_list, and with the watchdog no longer force-clearing, that row keeps its (stalled) arc until the chat is opened or the app restarts. Narrow edge (requires the reap to happen inside the disconnect window); if it shows up in practice, the rehydrator could additionally clear busy sessions absent from a successful snapshot.
Good to merge.
* fix(desktop): keep quiet sessions visibly running * fix(desktop): restore running status after reconnect * fix(desktop): keep live session status unmistakable
What does this PR do?
Fixes three related Hermes Desktop sidebar-state gaps that can make a live session look finished:
The watchdog now treats silence as presentation state only, never as completion. A quiet but authoritative running turn keeps the unmistakable running arc while using a softer dot. Desktop also reads the gateway's non-mutating
session.active_listsnapshot immediately on connection and every 1.5 seconds while visible (matching the TUI cadence), restoring running or waiting rows without resuming, focusing, or otherwise changing a chat.Related Issue
No issue filed; all three paths were reproduced in the packaged Windows desktop app.
Type of Change
Changes Made
Rebased onto current main and re-homed the implementation on the post-refactor(desktop): derive working/attention session sets from $sessionStates #66454 session-states.ts / use-session-state-cache.ts architecture.
Added a separate stalled-session presentation state without clearing authoritative working state.
Removed the watchdog callback that incorrectly cleared busy/awaiting state after silence.
Kept the row's running arc visible for quiet/stalled turns; only a blocking prompt replaces it with the needs-input treatment.
Rehydrates live working and waiting sessions from
session.active_listafter reconnect.Rechecks the lightweight in-memory live-session snapshot every 1.5 seconds while Desktop is visible, matching Hermes TUI.
Preserves quiet styling for rehydrated turns whose last activity exceeds the watchdog window.
Added regressions for silent turns, reconnect recovery, waiting-for-input recovery, malformed/idle snapshots, and the quiet-running row appearance.
How to Test
From
apps/desktop, run:npx vitest run --project ui src/app/chat/sidebar/session-row-state.test.ts src/app/contrib/hooks/use-background-sync.test.ts src/store/session-watchdog.test.ts src/store/gateway-switch.test.ts src/app/session/hooks/use-session-state-cache.test.tsxStart a desktop turn and leave it without stream events for the watchdog window.
Confirm the dot softens but the running arc and composer running state remain visible.
Disconnect/reconnect Desktop while another chat is still running.
Confirm its sidebar row restores automatically without opening that chat.
Let a long conversation auto-compress and confirm its rotated sidebar row recovers within about 1.5 seconds.
Confirm a real terminal event clears the running indicator.
Checklist
Code
Documentation & Housekeeping
Screenshots / Logs
Validated against the packaged Windows desktop application. After reinstall/restart, the live gateway still reported the ongoing Temporary Folder Cleanup session as
working, and its sidebar liveness now rehydrates without requiring a click.