Skip to content

fix(desktop): bound inflight journal persistence (#63047) - #82832

Closed
egilewski wants to merge 2 commits into
NousResearch:mainfrom
egilewski:codex/fix-63047-minimal-journal
Closed

fix(desktop): bound inflight journal persistence (#63047)#82832
egilewski wants to merge 2 commits into
NousResearch:mainfrom
egilewski:codex/fix-63047-minimal-journal

Conversation

@egilewski

@egilewski egilewski commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Replace synchronous aggregate v1 localStorage journal rewrites with bounded per-session snapshots.
  • Migrate legacy state once, isolate quota/storage failures, retain recoverable matching and tool completion state, and avoid oversized writes blocking renderer streaming.
  • Add focused journal regressions and a real Electron/CDP macOS-arm64 diagnostic harness with native hide/restore, renderer heartbeat, Settings, composer, and transcript checks.

Scope

This is the narrow journal hot-path fix for #63047. It deliberately excludes the broader diagnostics stack and does not include .circleci/config.yml; the harness can be run locally on macOS arm64 using the command documented in the source package scripts.

Validation

  • Baseline focused regression: 1 failure, 16 passing.
  • Focused journal tests: 33 passing.
  • Full desktop suite: 4,646 passing, 2 skipped.
  • Desktop typecheck, ESLint, Prettier, Node syntax, and diff checks passed.
  • macOS arm64 A/B harness: baseline and candidate each completed 3/3 measured runs without a hard reproduction; candidate remained responsive through native hide/restore and interactive-surface checks.
  • The harness uses real Electron/CDP and Hermes UI/backend flow; only model inference is a local deterministic HTTP stream.

Local macOS arm64 run

npm ci
npm run --prefix apps/desktop repro:short-session-hang:test
npm run --prefix apps/desktop repro:short-session-hang -- \
  --baseline 2446c8bb6755ff5e6feff4d26e425661edd4019b \
  --candidate 12b7cd542bafc18903de39754d36e73f4700244e \
  --repetitions 3 \
  --output "$PWD/short-session-hang-artifacts"

Fixes #63047

Related #80872

@egilewski
egilewski requested a review from a team August 10, 2026 00:40
@alt-glitch alt-glitch added type/perf Performance improvement or optimization comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 10, 2026
@alt-glitch

alt-glitch commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: #80872 also bounds the in-flight turn journal, but carries a much broader diagnostics and desktop stack. This PR is a focused hot-path alternative for #63047; maintainers should choose the preferred scope.

@egilewski egilewski mentioned this pull request Aug 10, 2026
Closed
1 task
@egilewski
fix(desktop): bound inflight journal persistence (NousResearch#63047)
0664439 
The desktop journal synchronously read, parsed, cloned, and rewrote one
aggregate localStorage value while streamed turns were repainting. Large
tool results and multi-session state could therefore block the renderer and
leave the app unresponsive, while the existing macOS diagnostic path lacked
a real native hide/restore regression check.

Store bounded recovery projections under per-session keys, migrate legacy v1
data once, isolate quota and storage failures, and preserve the newest
recoverable tail without allowing oversized writes to replace valid state.
Add a real Electron/CDP macOS-arm64 A/B harness with native visibility control,
renderer heartbeat and Settings/composer/transcript checks, plus focused
regressions. Keep bulk tool payloads, diagnostics, and the existing recovery
merge behavior out of the hot path.

Fixes NousResearch#63047
@egilewski
egilewski force-pushed the codex/fix-63047-minimal-journal branch from 12b7cd5 to 0664439 Compare August 10, 2026 13:49
This was referenced Aug 10, 2026
Open
Closed
@Nicolas-Formenton Nicolas-Formenton mentioned this pull request Aug 11, 2026
Closed
18 tasks
@egilewski
test(desktop): spy on the active localStorage implementation (NousRes…
a0ab644 
…earch#82832)

The inflight journal regression test always spied on Storage.prototype,
but Node 26's jsdom setup can provide a plain in-memory localStorage fallback.
That left the test unable to observe the setItem call in CI even though the
per-session journal write was correct.

Select the native window.Storage prototype when available and otherwise spy
on the active localStorage object, preserving the assertion across both
storage implementations.

Refs NousResearch#82832
@marcatmadu

marcatmadu commented Aug 13, 2026

Copy link
Copy Markdown

Scope decision noted — for the record: this PR's branch was already tested on Windows 11 (dumps #6 and #7, freeze reproduced on head 066443953; see #63047 / #83420). The new head a0ab6446f only adds a test spy (+10/-2 in inflight-turn-journal.test.ts), so that result stands for the final version. The Windows stall itself is tracked separately in #83420 with fix candidate #83540.

royalaid pushed a commit to royalaid/hermes-agent that referenced this pull request Aug 14, 2026
@egilewski @royalaid
fix(desktop): keep journal projection traversal bounded
a5d1061 
Semantically replay the bounded inflight journal foundation from 0664439 and its NousResearch#82832 follow-up onto the newer journal architecture. Avoid deep-cloning live tool payloads before the bounded preview projection can stop traversal.
royalaid pushed a commit to royalaid/hermes-agent that referenced this pull request Aug 14, 2026
@egilewski @royalaid
fix(desktop): keep journal projection traversal bounded
20e46b4 
Semantically replay the bounded inflight journal foundation from 0664439 and its NousResearch#82832 follow-up onto the newer journal architecture. Avoid deep-cloning live tool payloads before the bounded preview projection can stop traversal.
royalaid pushed a commit to royalaid/hermes-agent that referenced this pull request Aug 14, 2026
@egilewski @royalaid
fix(desktop): keep journal projection traversal bounded
d7f3817 
Semantically replay the bounded inflight journal foundation from 0664439 and its NousResearch#82832 follow-up onto the newer journal architecture. Avoid deep-cloning live tool payloads before the bounded preview projection can stop traversal.
@marcatmadu marcatmadu mentioned this pull request Aug 14, 2026
Closed
3 tasks
@teknium1 teknium1 mentioned this pull request Aug 15, 2026
Merged
teknium1 pushed a commit that referenced this pull request Aug 15, 2026
@egilewski @teknium1
test(desktop): spy on the active localStorage implementation (#82832)
6066991 
The inflight journal regression test always spied on Storage.prototype,
but Node 26's jsdom setup can provide a plain in-memory localStorage fallback.
That left the test unable to observe the setItem call in CI even though the
per-session journal write was correct.

Select the native window.Storage prototype when available and otherwise spy
on the active localStorage object, preserving the assertion across both
storage implementations.

Refs #82832
teknium1 pushed a commit that referenced this pull request Aug 15, 2026
@egilewski @teknium1
test(desktop): spy on the active localStorage implementation (#82832)
4d4bf0e 
The inflight journal regression test always spied on Storage.prototype,
but Node 26's jsdom setup can provide a plain in-memory localStorage fallback.
That left the test unable to observe the setItem call in CI even though the
per-session journal write was correct.

Select the native window.Storage prototype when available and otherwise spy
on the active localStorage object, preserving the assertion across both
storage implementations.

Refs #82832
@teknium1

teknium1 commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Thank you for this fix! It was salvaged into #86590 (cherry-picked onto current main with your authorship preserved in the commit history) and is now merged. Closing this PR since the work has landed.

@teknium1 teknium1 closed this Aug 15, 2026
atirna pushed a commit to atirna/hermes-agent that referenced this pull request Aug 17, 2026
@egilewski @teknium1
test(desktop): spy on the active localStorage implementation (NousRes…
67b45a1 
…earch#82832)

The inflight journal regression test always spied on Storage.prototype,
but Node 26's jsdom setup can provide a plain in-memory localStorage fallback.
That left the test unable to observe the setItem call in CI even though the
per-session journal write was correct.

Select the native window.Storage prototype when available and otherwise spy
on the active localStorage object, preserving the assertion across both
storage implementations.

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

Reviewers

No reviews

Assignees

No one assigned

Labels

comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/perf Performance improvement or optimization

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: Desktop app becomes completely unresponsive (including Settings) after ~5 messages on macOS 27 beta

4 participants

@egilewski @alt-glitch @marcatmadu @teknium1