Skip to content

[Hermes][Desktop][hermes-desktop-crash-survivable-turn-progress][2/n] fix(desktop): recover in-flight turn progress after restart - #44339

Closed
OmarB97 wants to merge 1 commit into
NousResearch:mainfrom
OmarB97:repair/crash-survivable-turn-progress-upstream-20260611
Closed

[Hermes][Desktop][hermes-desktop-crash-survivable-turn-progress][2/n] fix(desktop): recover in-flight turn progress after restart#44339
OmarB97 wants to merge 1 commit into
NousResearch:mainfrom
OmarB97:repair/crash-survivable-turn-progress-upstream-20260611

Conversation

@OmarB97

@OmarB97 OmarB97 commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Why

Hermes Desktop can lose visible in-flight turn progress if the app process dies mid-turn. On restart, the persisted session transcript may only contain the initiating user prompt, which makes streamed assistant text and tool progress appear lost even when the turn was actively producing useful output before the crash.

What Changed

  • Added a bounded desktop-only in-flight turn journal in localStorage.
  • Persisted the current visible user/assistant/tool tail while a session is busy, and clear it once the stored transcript catches up or the turn settles.
  • Recovered the journal during session resume, and merged backend inflight resume payloads when present.
  • Extended desktop resume response typing for optional running/in-flight state.

How To Review

Start with apps/desktop/src/lib/inflight-turn-journal.ts, then check the resume integration in apps/desktop/src/app/session/hooks/use-session-actions.ts and the central persistence hook in apps/desktop/src/app/session/hooks/use-session-state-cache.ts.

Evidence

Verification

  • npm run test:ui -- src/lib/inflight-turn-journal.test.ts src/app/session/hooks/use-session-state-cache.test.tsx src/app/session/hooks/use-message-stream.test.tsx exited rc=0: 2 files, 10 tests passed.
  • npx eslint src/lib/inflight-turn-journal.ts src/lib/inflight-turn-journal.test.ts src/app/session/hooks/use-session-state-cache.ts src/app/session/hooks/use-session-actions.ts src/types/hermes.ts exited rc=0.
  • npm run typecheck exited rc=0.
  • git diff --check upstream/main...HEAD exited rc=0.

Risks / Gaps

Accepted no-follow-up rationale: the journal is intentionally local to the desktop renderer and tail-limited, restoring the visible transcript after renderer/app death without mutating model conversation history. Installed-app swap/relaunch verification stays on MeshBoard task hermes-desktop-crash-survivable-turn-progress-20260611 because it requires a safe restart window for active sessions.

Collaborators

  • Omar Baradei: operator on ko-mac, reported the crash-restart UX failure and requested upstream follow-through on 2026-06-11.
  • Codex: ko-mac.codex#765db69c69, local coding lane, iteration 2/n for MeshBoard task hermes-desktop-crash-survivable-turn-progress-20260611, implemented and verified the upstream branch on 2026-06-11.

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/gateway Gateway runner, session dispatch, delivery labels Jun 11, 2026
@OmarB97

OmarB97 commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

Current state after merge-gate sweep:

  • GitHub reports the PR CLEAN / MERGEABLE.
  • Required checks are green for the current head 0aa167b9a63fd557c3370e3ec06c279850c638ed.
  • meshctl pr merge --dry-run accepted the PR: base is 28 commits behind current main, but MeshBoard found no changed-file overlap with newer main.

I attempted the actual merge through meshctl pr merge, but GitHub rejected it because OmarB97 does not have MergePullRequest permission on NousResearch/hermes-agent. This PR is ready for an upstream maintainer to merge.

@OmarB97
OmarB97 force-pushed the repair/crash-survivable-turn-progress-upstream-20260611 branch from 0aa167b to 4557e8c Compare June 12, 2026 14:22
…urn progress after restart

Re-authored onto current upstream/main (c9a150d). Persist each session's
in-flight turn tail to a localStorage journal and, on resume, fold it back onto
the restored transcript so a mid-turn app/renderer crash no longer drops the
streaming assistant.

- New apps/desktop/src/lib/inflight-turn-journal.ts (+ unit tests): bounded,
  best-effort journal (persist / read / recover / merge-backend-inflight).
- use-session-state-cache: persist the active tail on every state update; clear
  it on session switch / turn settle.
- types/hermes: SessionResumeResponse.inflight/.running + SessionInFlightTurn.
- use-session-actions resume: recover local journal + backend inflight AFTER
  preferredMessages resolves, so recovery COEXISTS with upstream's prefetch-hit
  lazy-IIFE fast path (no forced always-reconcile). Re-arm
  busy/streamId/turnStartedAt/sawAssistantPayload only when a tail is recovered;
  the empty-transcript fail-latch checks pre-recovery messages so an orphan
  journal entry can't mask a lost transcript.

Recovery wiring covered by new headless tests in use-session-actions.test.tsx.
Full crash-restart flow still warrants manual QA.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@OmarB97
OmarB97 force-pushed the repair/crash-survivable-turn-progress-upstream-20260611 branch from 4557e8c to bb26e09 Compare July 6, 2026 01:05
@OmarB97

OmarB97 commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

Refreshed onto current main as a single-commit re-authoring (the desktop session hooks were refactored since this opened, so the recovery wiring was rebuilt onto use-session-actions/index.ts). Key design point: recovery is hooked after preferredMessages resolves and returns baseMessages by reference on the common no-journal path, so it coexists with upstream's prefetch-hit fast path (no second transcript reconcile, big-session perf preserved); the empty-transcript fail-latch checks the pre-recovery length so an orphan journal entry can't mask a genuinely lost transcript.

Verified headlessly: tsc + vite build clean, journal vitest 5/5, existing resume + state-cache suites unchanged, plus 2 added wiring tests asserting the recovered ClientSessionState (tail grafted + running re-armed when a journal tail exists; no resurrection + journal cleared when the resumed transcript already has the answer) — 15/15.

Verification boundary (manual QA): the true end-to-end packaged-app crash → restart → localStorage-survives → view-repaint can only be exercised in a real Electron run; the wiring and journal logic are unit-verified, but that final layer isn't headlessly testable.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for rebuilding this onto the refactored desktop session hooks. The recovery gap is real on current main: tui_gateway/server.py:6049-6080 returns live inflight state, while apps/desktop/src/app/session/hooks/use-session-actions/index.ts:549-594 currently ignores it.

Problems

  • apps/desktop/src/app/session/hooks/use-session-actions/index.ts:92-93 applies backend recovery before local recovery, despite the comment at :82-85 saying the local journal wins. A backend assistant snapshot causes mergeInFlightMessages() to mark the local tail caught up (apps/desktop/src/lib/inflight-turn-journal.ts:270-280) and clear it. That can drop locally recorded tool progress because the backend snapshot carries only text (tui_gateway/server.py:5156-5169).

Suggested changes

  • Resolve an applicable local journal tail before using backend recovery, or use backend recovery only as the fallback when no local tail applies.
  • Add a combined-source regression test: local tool tail plus backend assistant text for the same prompt.

The branch is currently conflicting with main, so salvage should also retain current session-switch protections. This is an automated hermes-sweeper review.

backendInFlight: BackendInFlightTurn | null | undefined,
keepPending: boolean
): InFlightRecoveryResult {
const backend = mergeBackendInFlightTurn(baseMessages, backendInFlight, { keepPending })

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This ordering makes the backend win, not the local journal described above. mergeBackendInFlightTurn() appends an assistant for the matching user; then local recovery sees that assistant after its user, returns caughtUp, and clears the journal. Since the backend snapshot is text-only while the journal can retain tool parts, resolve the local journal first (or use backend only as its fallback) and add a combined-source test.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
@OutThisLife

Copy link
Copy Markdown
Collaborator

Superseded by #71184, which rebuilds this onto the current split session hooks with the review defects fixed (local-journal-first merge ordering so backend text snapshots can't drop journaled tool progress, plus throttled persistence off the delta-flush hot path). Your journal design carried over largely intact — thanks @OmarB97, credited via Co-authored-by.

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

Labels

comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants