Skip to content

Improve Desktop gateway resilience, model discovery latency, and backup safety - #69411

Closed
HAOWANG116 wants to merge 4 commits into
NousResearch:mainfrom
HAOWANG116:codex/hermes-stability-20260722
Closed

Improve Desktop gateway resilience, model discovery latency, and backup safety#69411
HAOWANG116 wants to merge 4 commits into
NousResearch:mainfrom
HAOWANG116:codex/hermes-stability-20260722

Conversation

@HAOWANG116

Copy link
Copy Markdown

Summary

This PR addresses several causes of slow or unavailable Desktop gateway/model-picker behavior.

  • Recover only the known transient tapClientLookup / useClientLookup index race with bounded retries, while preserving the normal scoped error fallback for all other errors.
  • Serialize local profile-backend cold starts and remove hover-triggered prewarming, so one profile switch cannot contend with other local backend launches. Existing and remote connections remain off the queue.
  • Make model inventory discovery bounded and cache-aware:
    • prune explicit-only providers before discovery;
    • limit concurrent provider catalog fetches to four;
    • coalesce identical in-flight requests;
    • cache complete picker payloads for 60 seconds with generation-based invalidation and an LRU cap;
    • add secret-free timings and stall-correlation diagnostics.
  • Make full and quick backups single-flight and atomic, so concurrent backup activity cannot publish partial archives/snapshots or delete a previous valid archive after a failed SQLite snapshot.

Test plan

  • Desktop UI suite — 224 files, 1,860 passed, 1 skipped.
  • Electron suite — 61 files passed, 1 skipped; 695 tests passed, 2 skipped.
  • Python stability and regression suite — 1,247 tests passed.
  • TypeScript typecheck, changed-file ESLint/Prettier, Ruff, diff check, and production package build — passed.
  • The repository-wide ESLint/Prettier commands currently report pre-existing formatting/import-order errors in the newly landed upstream native OAuth files; none are in this PR's changed lines.
  • Manual Desktop smoke test:
    • gateway reached Gateway ready;
    • model picker loaded all three configured providers;
    • cold model inventory request completed in ~0.91 s and warm cache hit in ~0.4 ms;
    • quick snapshot completed successfully while the gateway remained ready.

Risks & rollback

  • The short-lived model cache can show a catalog that is up to 60 seconds old; explicit refresh and configuration/credential generation changes bypass or invalidate it.
  • Local backend cold starts are intentionally serialized. This favors a responsive primary gateway over parallel local child-process startup; remote and already-open connections are unaffected.
  • A second overlapping backup now fails/skips promptly instead of competing for files. This is intentional single-flight behavior.
  • The error boundary only retries the exact transient lookup signature three times; other errors retain the existing fallback and manual retry path.

Rollback requires no data migration: revert these four commits (or redeploy the prior Desktop package/runtime backup). Existing valid backups and snapshots remain intact because new outputs are published atomically.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/desktop Electron desktop app (apps/desktop/*) comp/cli CLI entry point, hermes_cli/, setup wizard comp/tui Terminal UI (ui-tui/ + tui_gateway/) labels Jul 22, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused resilience work. The underlying backup and picker latency problems still exist on current main, but this needs selective salvage rather than a direct cherry-pick.

Problems

  • The backup rewrite predates current recovery-source hardening. Current create_quick_snapshot() records failed_dbs / oversized_skipped and suppresses pruning when a database was not captured (hermes_cli/backup.py:1152-1217); the regression test requires the prior complete snapshot to survive (tests/hermes_cli/test_backup.py:1826-1874). Preserve that invariant while adding staging, atomic publish, and locking.
  • The model-options seam changed after the PR base: current main uses build_model_options_payload() (hermes_cli/inventory.py:283-313) from a thread-pooled endpoint (hermes_cli/web_server.py:6739-6753). Apply caching and bounded discovery there, not against the older direct builder call.

Suggested changes

  • Carry the atomic/single-flight logic into the current backup implementation without dropping incomplete-snapshot metadata or prune suppression.
  • Rework the picker changes on the current inventory endpoint and retain end-to-end behavior coverage for refresh and concurrent requests.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform type/perf Performance improvement or optimization labels Jul 30, 2026
@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Merged via #77913 — thank you @HAOWANG116. Your backup commit was cherry-picked, so you remain the author in git history.

The concurrency hazard you identified was real and under-appreciated: run_backup, create_quick_snapshot, and the automatic full-zip path could all run simultaneously (gateway auto-backup firing mid-/snapshot), interleaving SQLite safe-copies. The shared cross-process lock plus staging-dir-then-rename publication is exactly the right shape — a crashed backup can no longer leave a half-snapshot that restore would trust. Your 105-line stability test file covering serialization and atomic publish made it straightforward to verify (6 passed, 46 across the full backup suite).

One adaptation in the salvage: the track_activity(...) telemetry wrappers were stripped because they import hermes_cli.perf_diagnostics, a module introduced by a different commit in this PR that wasn't part of this vehicle. The locking — the actual fix — is fully intact.

The PR's remaining concerns are worth re-proposing as focused PRs against current main:

  • Desktop startup serialization + prewarm removal — the serialization queue is defensible on its own, but removing the hover-intent prewarm system (useProfilePrewarm, still wired on main) is a product-taste call that needs maintainer sign-off separately from the queue.
  • Model discovery latency + perf_diagnostics — this touches tui_gateway/server.py and ws.py, which just changed in perf(cold-start): mitigate the ~14s GIL stall during backend init (salvage #60807) #77814 (cold-start warm list + off-loop skin resolution). Worth re-measuring against that new baseline before re-proposing; the perf_diagnostics module could then carry the backup telemetry that was stripped here.
  • Transient client lookup races — small and self-contained; would likely land quickly on its own.

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

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard comp/desktop Electron desktop app (apps/desktop/*) comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working type/perf Performance improvement or optimization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants