Skip to content

fix(desktop): keep recovered actions bound to the selected session - #64673

Closed
33hodl wants to merge 2312 commits into
NousResearch:mainfrom
33hodl:fix/gateway-session-resilience
Closed

fix(desktop): keep recovered actions bound to the selected session#64673
33hodl wants to merge 2312 commits into
NousResearch:mainfrom
33hodl:fix/gateway-session-resilience

Conversation

@33hodl

@33hodl 33hodl commented Jul 15, 2026

Copy link
Copy Markdown

1|## What does this PR do?
2|
3|Keeps Hermes Desktop's recovered prompt, redirect, and interrupt actions bound to the conversation the user actually selected.
4|
5|After a gateway restart, a stale runtime session can return session not found. Desktop resumes the durable session and receives a replacement runtime ID. Current main can update only the hot ref while leaving $activeSessionId stale, and asynchronous recovery can continue after the user switches conversations — including switch-away-and-back cases where the final IDs look unchanged.
6|
7|This narrowed PR fixes only that session-correctness boundary:
8|
9|- synchronize recovered runtime IDs into both activeSessionIdRef.current and $activeSessionId;
10|- make profile-switch fresh-session teardown synchronous before gateway activation can move to another profile;
11|- advance a monotonic selection generation synchronously at the stored-selection write boundary, so batched A → B → A is still observable;
12|- abort prompt-submit, redirect, and interrupt recovery when route, stored selection, runtime selection, or generation changes;
13|- prevent late recovered-submit failures from adding errors to a newly selected session;
14|- preserve a newly selected session's busy/notification state when a recovered interrupt resolves or rejects late;
15|- keep background queued sends pinned to their queued session;
16|- migrate or remove optimistic state only inside the operation's target runtime;
17|- add focused regression coverage for submit, redirect, interrupt, busy retry, optimistic cleanup, profile switch, and switch-away-and-back races.
18|
19|## Why this PR is narrower now
20|
21|The earlier version also changed OAuth-ticket classification and remote liveness. That work overlaps merged PR #68250 and is removed from this branch.
22|
23|Backend-restart connection retry is also intentionally excluded and is tracked separately in #69059 so maintainers can evaluate retry policy independently from this correctness fix.
24|
25|## Related work and attribution
26|
27|- Preserves the atom/ref synchronization contribution from #62471 and its original author via cherry-pick.
28|- Incorporates the profile-switch timing fix from #66889 and preserves its original author via cherry-pick.
29|- Complements #68250; it does not duplicate that merged OAuth/liveness implementation.
30|
31|## Correctness invariants
32|
33|1. The runtime-session ref and reactive atom agree after recovery.
34|2. Recovery never overwrites, reports errors into, or clears state in a conversation selected after the operation began.
35|3. Switching away and back is still observable as user intent.
36|4. A profile switch clears foreground session identity before the active gateway can change.
37|5. Background queued sends remain pinned to their queued destination.
38|6. An intentional recovery runtime-ID rotation is distinguished from a later user selection.
39|
40|## Validation
41|
42|Run from the repository root on current main (683059feb54f511717c831e0989e0f3c54450785):
43|
44|- npm test --workspace apps/desktop -- --project ui src/app/session/hooks/use-prompt-actions/index.test.tsx src/app/hooks/use-fresh-session-requests.test.tsx src/app/session/hooks/use-background-queue-drain.test.tsx src/store/session.test.ts
45| - 121 passed
46|- npm run typecheck --workspace apps/desktop
47| - passed (UI, Electron, and E2E TypeScript projects)
48|- npm run lint --workspace apps/desktop
49| - 0 errors; 21 unrelated existing warnings
50|- git diff --check origin/main...HEAD
51| - passed
52|- changed-line secret-pattern scan
53| - no private keys, GitHub tokens, AWS keys, or generic secret assignments
54|- package/dependency review
55| - no manifest or lockfile changes
56|
57|Independent review was performed after repair against the live-main-rebased head. Publication was held until every blocking finding had a RED regression test and a verified fix.
58|
59|## Scope
60|
61|Desktop renderer/session state and regression tests only. No authentication, liveness, backend-retry, generated artifact, dependency, configuration, or credential changes are included.
62|

@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) area/auth Authentication, OAuth, credential pools sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state P3 Low — cosmetic, nice to have labels Jul 15, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks — the reported behavior is present on current main.

Problems

  • The new 401/403-only classification conflicts with the current Desktop engineering contract: apps/desktop/AGENTS.md:127-130 says any OAuth ticket-mint failure means reauthentication. Update that guidance so future changes preserve the intended transient-failure behavior.

Suggested changes

  • Amend apps/desktop/AGENTS.md:127-130 to state that a fresh ticket is required on every dial, but only confirmed 401/403 failures require sign-in; transient/network/server failures remain retryable.

Salvageability is high. The PR base predates current main, and current-main changes touch several modified production files, so a maintainer cherry-pick will need focused conflict resolution rather than a blind apply. This is an automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 16, 2026
@33hodl
33hodl force-pushed the fix/gateway-session-resilience branch from 6ce51c7 to b2d0a35 Compare July 17, 2026 10:08
@33hodl

33hodl commented Jul 17, 2026

Copy link
Copy Markdown
Author

Addressed the hermes-sweeper review request in commit 60b75077f. Updated apps/desktop/AGENTS.md to clarify that every OAuth dial mints a fresh WebSocket ticket, while only confirmed 401/403 (or explicitly tagged auth) failures require sign-in; transient network, timeout, and server failures remain retryable.

The branch is rebased onto current main, conflicts are resolved, and focused Electron validation passed: 439 passed, 1 skipped.

@teknium1 teknium1 added the area/sessions Session lifecycle, resume, persistence, history label Jul 19, 2026
@33hodl
33hodl force-pushed the fix/gateway-session-resilience branch from 60b7507 to 71bf366 Compare July 20, 2026 12:23
@33hodl

33hodl commented Jul 21, 2026

Copy link
Copy Markdown
Author

Rebased the remote Desktop gateway/session resilience series onto current origin/main (fb0ed8396) and added a fifth fix from independent review:

  • abort recovered prompt.submit when the user switches sessions during a bounded session busy retry delay;
  • re-check the pinned session/route before every retry attempt;
  • add a regression test covering the switch-during-retry window.

Validation on the refreshed head:

  • focused UI: 73 passed;
  • focused Electron: 64 passed;
  • full Electron: 484 passed, 1 skipped;
  • TypeScript: passed;
  • ESLint: 0 errors;
  • production build: passed;
  • macOS ARM64 Electron Builder package: passed;
  • production dependency audit: 0 vulnerabilities;
  • added-line credential scan and git diff --check: passed.

The branch remains a small upstream-plus-five-commit delta. The macOS build stamp is clean and points to the PR head.

@33hodl
33hodl force-pushed the fix/gateway-session-resilience branch 2 times, most recently from a8f5c3e to 2256650 Compare July 22, 2026 02:35
@33hodl 33hodl changed the title fix(desktop): harden remote gateway session resilience fix(desktop): keep recovered actions bound to the selected session Jul 22, 2026
@33hodl

33hodl commented Jul 22, 2026

Copy link
Copy Markdown
Author

@austinpickett This PR has been narrowed to the Desktop session-correctness boundary and rebased onto current main. The OAuth/liveness overlap from #68250 and restart-retry policy were removed. Focused UI tests pass 58/58, Desktop typecheck passes, and an independent review approved the final recovery/session-switch behavior. Could you review when bandwidth permits? The stale area/auth label is no longer applicable.

@33hodl
33hodl force-pushed the fix/gateway-session-resilience branch 5 times, most recently from 809f4cd to bcb972b Compare July 23, 2026 15:38
@33hodl

33hodl commented Jul 23, 2026

Copy link
Copy Markdown
Author

Repaired branch is now published at bcb972b5581604230f6421aa1e82844a7e48f359.

Final gates:

  • independent review: APPROVE on the exact published head
  • focused + neighboring regression tests: 121/121 passed
  • Desktop typecheck: passed (UI, Electron, E2E)
  • lint: 0 errors (21 existing warnings)
  • git diff --check: passed
  • changed-line secret scan: no matches
  • dependency review: no manifest or lockfile changes
  • original contributor commits/authorship preserved

The branch was rebased once onto NousResearch main at 683059feb54f511717c831e0989e0f3c54450785. Main advanced by 19 commits during final review; none touch this PR’s files and git merge-tree --write-tree origin/main HEAD is clean, so I did not churn the reviewed commits with another unnecessary force-rebase.

I could not create a formal reviewer request because GitHub returned 404 for this fork PR. @austinpickett, please review when available.

…tles

Name sessions instantly from the opening message, and make the name stick
…et fold-in

plugins.manage list rows now carry the canonical registry key and a
portable flag (Agent Plugins v1 plugin.json packages), and toggles
address the key — bare names collide across category dirs
(image_gen/fal vs video_gen/fal), so name-addressed toggles flipped
both. Portable packages' in-memory MCP servers also fold into
enabled_mcp_server_names(); without that their tools registered with
the MCP runtime but never reached the model's schema.
OutThisLife and others added 26 commits August 10, 2026 03:24
…list

A thumb parked on a list you aren't touching is chrome, not information,
and the sidebar stacks several scrollers so it draws several of them at
once. Fade them in on hover instead, sharing the existing scrollbar
colors and the webkit/Firefox split rather than styling a second kind of
bar. Only the thumb's color changes, so the reserved gutter still keeps
rows from shifting sideways.
… ipynb extraction

read_file's .ipynb extraction previously dropped cell outputs entirely,
so a notebook's training logs, tracebacks, and printed results were
invisible to the model. Ported LobeHub's token-efficient conversion:

- stream text and error tracebacks are kept (ANSI-stripped, \r
  progress-bar rewrites collapsed to the final frame)
- execute_result/display_data prefer text/plain over the HTML twin
- base64 images become sized placeholders ([image/png output — 3 KB,
  omitted]); widget state and script-bearing HTML are omitted
- legacy nbformat v3 pyout/pyerr flat-field shapes handled
- per-cell output block capped at 20k chars
…ousResearch#68392)

_live_session_payload() falls back to _fallback_session_info() while a
session's agent is still None (lazy/deferred build). That fallback omitted
desktop_contract, so session.activate returned lazy metadata with no contract
field. Desktop feeds the value straight into reportBackendContract(), where a
missing field reads as contract 0 — a current backend is then falsely flagged
"Backend out of date" on every activate of a live lazy session.

The sibling session.create shape (_lazy_resume_info) was fixed the same way in
NousResearch#36112; this closes the remaining session.activate gap by advertising
DESKTOP_BACKEND_CONTRACT in the fallback payload.

Adds test_session_activate_lazy_info_reports_desktop_contract pinning the
session.activate path against a lazy (agent=None) session.
The PR and profile chips rendered in the row body, left of the kebab's own
column: they never sat flush right and never handed their space to the kebab
on hover, so a row showing only a PR left a hole where the age would have been.
Both now join the tokens/cost/age figures in the actions slot, and the kebab
covers the end of it — losing whichever item reads last, not the whole slot.
The all-profiles scope defaulted to grouping by profile, so "Reset to defaults"
handed back a grouping the user never picked. Both scopes now ship by date, and
a reset clears the scope you are not looking at too — otherwise flipping the
rail restored the customization the reset was supposed to undo.

Hovering a row's PR chip also holds the kebab back now: the chip is a link, and
the button that covers the end of the trailing slot was taking the click.
…l-profiles

Sidebar: show every profile at once
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
… icons

Left cluster gets a macOS-only translate to sit on the traffic-light row.
All titlebar tools use 24×24 hit areas with 13.9px Codicons (inline size
beats unlayered codicon.css). Clusters share one flex shell with no gap —
buttons abut and the hit target is the spacing.
…ontrols-y

fix(desktop): titlebar clusters — macOS Y nudge, 24px targets, 13.9px icons
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…le (NousResearch#83091)

* fix(desktop): stop HUD window growing on drag; add corner resize handle

The HUD window is created frame:false + transparent:true + resizable:true.
On Windows, a transparent frameless window silently grows ~1px per
setPosition call (worse at >100% DPI scaling) — every drag of the composer
bar accumulated size drift, and the HUD could end up enormous (reported at
1385x1052 against a 620x320 default). Reading the size back mid-drag
compounds the drift because getSize() returns the already-drifted value.

Fix, mirroring the pet overlay's pattern:
- create the HUD window non-resizable (no system edge resize hot-zone)
- moveBy uses setBounds with a size snapshotted on the first move of each
  drag, so the OS can never accumulate drift (verified: 500 moveBy calls
  with zero size change on Electron 40 / Win11 / 175% DPI)
- add a bottom-right corner resize handle (resize-handle.ts) driving a new
  hermes:hud:set-bounds IPC that flips resizable on for the call, restoring
  the ability to resize a window that is otherwise non-resizable

* fix(desktop): pin HUD drag size in renderer, not main-process globals

The superseding pass drops hudDragWidth/hudDragHeight from main: composer
drag snapshots outerWidth/outerHeight when the hold arms (pet overlay
pattern) and passes them on every moveBy. Adds one test for that contract.

Supersedes NousResearch#82455.

Co-authored-by: Ringo6107 <199014580+Ringo6107@users.noreply.github.com>

* fix(desktop): keep the HUD solid through a corner resize; drop dead handle state

The resize handle's `resizing` flag only fed a CSS rule that restated the
cursor it already had, so nothing pinned the window mid-gesture: click-through
hands the mouse away the moment the growing edge outruns the cursor. Raise the
composer drag's existing `data-hud-grabbing` instead — one flag for "a gesture
owns the window" — and cover it in click-through's tests.

Also drops the hook's always-true `enabled` param and routes teardown through a
`reset` callback, matching composer-drag.ts and clearing the atom-mirrored-ref
lint rule.

---------

Co-authored-by: Ringo6107 <199014580+Ringo6107@users.noreply.github.com>
…ne-resize

fix(desktop): in-app browser kept squishing the chat and jamming its resize sash
Keep the current desktop lint spacing around the rebased recovery and drift-guard statements.
@33hodl
33hodl force-pushed the fix/gateway-session-resilience branch from bcb972b to 941403d Compare August 10, 2026 10:30
@33hodl

33hodl commented Aug 10, 2026

Copy link
Copy Markdown
Author

Rebased onto current main and adapted to the newer shared resolver — new head 941403d80f.

  • The recovery/drift guards are now implemented in withSessionNotFoundResume (shared by every caller), not just the submit path; the manual per-commit recovery code was removed in favor of current main's helper.
  • sessionId rebinds on recovery so cleanup can't strand busy state or attach errors to a stale runtime.
  • Validation: 169 focused tests pass (use-prompt-actions, use-fresh-session-requests, session), typecheck clean, ESLint has no new issues.
  • The luxles co-author commit is preserved.

Ready for another pass.

@33hodl 33hodl closed this Aug 12, 2026
@33hodl
33hodl force-pushed the fix/gateway-session-resilience branch from 941403d to 4e370a4 Compare August 12, 2026 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools area/sessions Session lifecycle, resume, persistence, history comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.