Skip to content

fix(desktop): preserve turn-elapsed timer across session switches - #61518

Closed
yingliang-zhang wants to merge 2 commits into
NousResearch:mainfrom
yingliang-zhang:fix/desktop-turn-timer-resets-on-switch
Closed

fix(desktop): preserve turn-elapsed timer across session switches#61518
yingliang-zhang wants to merge 2 commits into
NousResearch:mainfrom
yingliang-zhang:fix/desktop-turn-timer-resets-on-switch

Conversation

@yingliang-zhang

Copy link
Copy Markdown
Contributor

Problem

The desktop status bar's turn-elapsed timer (running-timer) resets to 0:00 every time the user switches away from a busy session and switches back.

Root Cause

  1. Gateway never reports turn_started_at: Neither session.resume responses nor session.info events include the turn start timestamp, so the desktop has no way to restore the clock after a session switch.

  2. Cold-resume path loses the timer: use-session-actions/index.ts creates a new ClientSessionState with turnStartedAt: null (the default). When the subsequent session.info event arrives with running: true, gateway-event.ts falls back to state.turnStartedAt ?? Date.now()Date.now() → timer resets to zero.

The warm-cache path correctly preserves turnStartedAt via syncSessionStateToView, but if session.usage RPC fails (e.g. pooled backend idle-reaped), the cache is purged and the cold path takes over — resetting the timer.

Fix

Gateway side (tui_gateway/server.py):

  • _session_info(): extract turn_started_at from inflight_turn.started_at and include it in the info dict
  • _live_session_payload(): include turn_started_at in the resume response payload

Desktop side (3 files):

  • types/hermes.ts: add turn_started_at?: number | null to SessionResumeResponse
  • lib/chat-messages.ts: add turn_started_at?: number | null to GatewayEventPayload
  • use-session-actions/index.ts: cold-resume path uses resumed.turn_started_at (converted to ms) to restore the timer instead of defaulting to null
  • gateway-event.ts: session.info busy-transition handler prefers gateway-reported turn_started_at over Date.now() fallback

Verification

  • npx tsc --noEmit — PASS (0 errors)
  • py_compile tui_gateway/server.py — PASS
  • vitest run src/app/session/hooks/ — 117/121 pass; 4 failures are pre-existing on origin/main (confirmed by stashing changes and re-running)
  • No personal info / internal paths in diff
  • git log --oneline origin/main..HEAD shows only this commit

Changes

File Change
tui_gateway/server.py +16 lines: report turn_started_at in _session_info() and _live_session_payload()
apps/desktop/src/types/hermes.ts +2: turn_started_at field on SessionResumeResponse
apps/desktop/src/lib/chat-messages.ts +1: turn_started_at field on GatewayEventPayload
apps/desktop/src/app/session/hooks/use-session-actions/index.ts +11: use resumed.turn_started_at in cold-resume path
apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts +8: prefer gateway turn_started_at in session.info handler

Total: +36/-2 across 5 files. No new config knobs — the field is purely additive.

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: Comment

Overview

  • 5 files changed, +36/-2 lines
  • Preserves turn-elapsed timer across desktop session switches
  • Fixes a UX issue where the timer would reset

Looks Good

  • Well-scoped fix targeting the session switch path
  • Proper state preservation in the desktop state manager
  • No security concerns

Reviewed by Hermes Agent

@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) comp/tui Terminal UI (ui-tui/ + tui_gateway/) P3 Low — cosmetic, nice to have labels Jul 9, 2026
@yingliang-zhang
yingliang-zhang force-pushed the fix/desktop-turn-timer-resets-on-switch branch from 242e02f to 735725f Compare July 11, 2026 05:21
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tracing the cold-resume path; the implementation targets a real current-main gap.

Problems

  • The new gateway-to-desktop timestamp contract has no regression test. Current protocol coverage for session.resume begins at tests/tui_gateway/test_protocol.py:296, and the existing cache coverage starts from an already-populated local turnStartedAt at apps/desktop/src/app/session/hooks/use-session-state-cache.test.tsx:83; neither covers a cold running-session resume.

Suggested changes

  • Add a tui_gateway protocol test for an inflight_turn.started_at value through both session.resume and _session_info().
  • Add desktop coverage that verifies turn_started_at converts to milliseconds on cold resume and is preferred over the Date.now() fallback in apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 11, 2026
@yingliang-zhang
yingliang-zhang force-pushed the fix/desktop-turn-timer-resets-on-switch branch 2 times, most recently from 829290e to 627f6d5 Compare July 18, 2026 22:33
@teknium1 teknium1 added the area/sessions Session lifecycle, resume, persistence, history label Jul 19, 2026
@yingliang-zhang

Copy link
Copy Markdown
Contributor Author

Addressed in commit 97bb62b4a7cfd604cf43d03e7ecf737f9c945219: added a shared, JSON-round-tripped active-turn session.resume fixture/contract across gateway and Desktop. The Desktop now restores the elapsed timer only for running responses with a valid numeric turn_started_at; invalid or absent values clear stale state instead of synthesizing Date.now(). Focused gates: Python 84 passed; Desktop 28 passed; Node 22 typecheck passed.

@yingliang-zhang
yingliang-zhang force-pushed the fix/desktop-turn-timer-resets-on-switch branch 5 times, most recently from 9e03a5b to d33bbfd Compare July 26, 2026 13:17
@yingliang-zhang
yingliang-zhang force-pushed the fix/desktop-turn-timer-resets-on-switch branch 2 times, most recently from 7f6f129 to f9dd16a Compare July 31, 2026 06:35
@yingliang-zhang
yingliang-zhang requested a review from a team July 31, 2026 06:35
@yingliang-zhang
yingliang-zhang force-pushed the fix/desktop-turn-timer-resets-on-switch branch 2 times, most recently from bf56942 to 69b295c Compare August 7, 2026 04:21
@yingliang-zhang
yingliang-zhang force-pushed the fix/desktop-turn-timer-resets-on-switch branch 3 times, most recently from 92d51fe to bf56942 Compare August 12, 2026 05:51
Rebased onto latest origin/main. Resolved conflicts in:
- use-session-actions.test.tsx: kept both HEAD's image-attachment test
  and PR's turn-clock restoration test (orthogonal features)
- use-session-actions/index.ts, gateway-event.ts, server.py,
  test_tui_gateway_server.py, test_protocol.py: adapted to HEAD's
  refactored structure while preserving PR's turn-origin tracking
@yingliang-zhang
yingliang-zhang force-pushed the fix/desktop-turn-timer-resets-on-switch branch from bf56942 to c1041d5 Compare August 12, 2026 06:08
@teknium1

Copy link
Copy Markdown
Contributor

Salvaged and merged in #87237 — both your commits landed on main with authorship preserved (c1041d5 + fb27c1a → rebase-merged), including the gateway turn_started_at contract, the shared round-trip fixture you added after the sweeper review, and the cold-resume restore. One conflict was resolved in favor of main's newer live-turn transcript reconcile design (your !running gate predated it); on the heartbeat path your gateway timestamp now composes with the newly-added submit-time clock seed (#86923) as state.turnStartedAt ?? gatewayTurnStartedAt ?? Date.now(). Thanks for the careful root-cause work on both sides of the wire!

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

Labels

area/sessions Session lifecycle, resume, persistence, history comp/desktop Electron desktop app (apps/desktop/*) comp/tui Terminal UI (ui-tui/ + tui_gateway/) P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants