Skip to content

fix(desktop): stop Settings Workspace from sticking on skeleton under Strict Mode - #74829

Open
zihaochen68 wants to merge 2 commits into
NousResearch:mainfrom
zihaochen68:fix/desktop-settings-workspace-skeleton
Open

fix(desktop): stop Settings Workspace from sticking on skeleton under Strict Mode#74829
zihaochen68 wants to merge 2 commits into
NousResearch:mainfrom
zihaochen68:fix/desktop-settings-workspace-skeleton

Conversation

@zihaochen68

@zihaochen68 zihaochen68 commented Jul 30, 2026

Copy link
Copy Markdown

What does this PR do?

Fixes Desktop Settings → Workspace (and other ConfigSettings tabs) getting stuck on the skeleton forever under React Strict Mode, and closes the related class of bugs where a mounted settings draft from profile A can be saved through profile B's route after a live gateway/profile switch.

Reported failure chain (ConfigSettings skeleton):

  1. useOnProfileSwitch used a one-shot first flag; Strict Mode's second effect pass looked like a profile switch and cleared the local config draft.
  2. The seed effect only depended on loadedConfig, so React Query structural sharing prevented re-seeding when the draft was empty but the cached reference was unchanged.

Sibling failure class (review feedback):
reset lived inside ConfigSettings, so it only ran when that panel was mounted. Surfaces that stay mounted across a profile swap (notably VoiceProviderFields in Capabilities) kept a one-shot draft + debounced saveHermesConfig with no switch handler — profile A's draft could be PUT into profile B.

Approach:

  1. Compare normalized profile keys in useOnProfileSwitch (not a first-mount flag).
  2. Seed drafts whenever config === null and data is available (state-derived, self-healing).
  3. Centralize a hard reset of hermes-config-record inside invalidateProfileScopedQueries() (resetQueriessetQueryData(undefined) is a bail-out in RQ v5), so every consumer drops profile A's record on switch regardless of which panels are mounted.
  4. Cover remaining config-draft holders: cancel pending autosaves / drop copies on switch, and version-/epoch-guard in-flight cache writes.

Related Issue

Fixes #74824

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • apps/desktop/src/app/hooks/use-on-profile-switch.ts (+ test) — normalized key comparison; Strict Mode safe
  • apps/desktop/src/lib/query-client.ts (+ test) — central hermes-config-record hard reset on profile/gateway switch; HERMES_CONFIG_QUERY_KEY owned here
  • apps/desktop/src/app/hooks/use-config-record.ts — re-exports the shared key (no per-panel resetHermesConfig)
  • apps/desktop/src/app/settings/config-settings.tsx (+ test) — state-derived seeding; drop draft on switch; version-guard post-save cache mirror
  • apps/desktop/src/app/settings/voice-provider-fields.tsx (+ test) — same draft/autosave/switch pattern (Capabilities TTS panel stays mounted)
  • apps/desktop/src/app/settings/sessions-settings.tsx (+ test) — AutoArchiveSetting drops whole-record copy and refetches on switch
  • apps/desktop/src/app/settings/model-settings.tsx — optimistic rollback only within the same profile epoch
  • apps/desktop/src/app/settings/toolset-config-panel.test.tsx — hermes mock stubs for profile-switch imports

How to Test

  1. Automated:
    cd apps/desktop
    npx vitest run \
      src/app/hooks/use-on-profile-switch.test.tsx \
      src/app/settings/config-settings.test.tsx \
      src/app/settings/voice-provider-fields.test.tsx \
      src/app/settings/sessions-settings.test.tsx \
      src/lib/query-client.test.ts
  2. Manual — skeleton: npm run dev → Settings → Workspace; fields render (not a permanent skeleton) under Strict Mode.
  3. Manual — profile switch: with Settings (or Capabilities → TTS provider fields) open, switch gateway profile; draft clears and re-seeds from the new profile; a dirty edit mid-debounce must not autosave into the new profile.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(desktop): …)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix
  • I've added tests for my changes
  • I've tested on my platform: macOS (dev / Strict Mode); desktop suite via npx vitest run in apps/desktop

Documentation & Housekeeping

  • I've updated relevant documentation — N/A
  • I've updated cli-config.yaml.example — N/A
  • I've updated CONTRIBUTING.md or AGENTS.md — N/A
  • I've considered cross-platform impact — N/A (renderer Strict Mode / React Query only)
  • I've updated tool descriptions/schemas — N/A

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

@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 the focused Strict Mode regression coverage. The reported premise is present on current main: the app runs under StrictMode at apps/desktop/src/main.tsx:46, and useOnProfileSwitch's one-shot guard calls its callback on the next effect pass at apps/desktop/src/app/hooks/use-on-profile-switch.ts:10-24.

Problems

  • resetHermesConfig() at apps/desktop/src/app/settings/config-settings.tsx:119 only runs while ConfigSettings is mounted. The Capabilities detail preserves VoiceProviderFields by toolset name (apps/desktop/src/app/skills/index.tsx:331-333,821), while that component retains a one-shot config draft and autosaves it (apps/desktop/src/app/settings/voice-provider-fields.tsx:47-74) without profile-switch handling. A profile-A draft can therefore survive a live swap and be saved against B.

Suggested changes

  • Extend the profile-transition reset/cancellation to VoiceProviderFields, or centralize it at the shared config-record boundary. Add a mounted A→B voice-panel regression proving it reseeds B and never saves A's draft.

Automated hermes-sweeper review.

savedDiscoverySignatureRef.current = undefined
setConfig(null)
saveVersionRef.current = 0
setSaveVersion(0)
void resetHermesConfig()

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 reset only runs when ConfigSettings is mounted. VoiceProviderFields can remain mounted in the Capabilities detail across the same live profile swap (apps/desktop/src/app/skills/index.tsx:331-333,821), but it retains a one-shot local config draft and debounced saveHermesConfig write with no switch handler (apps/desktop/src/app/settings/voice-provider-fields.tsx:47-74). Please cover that sibling so profile A's draft cannot be saved through profile B's active route.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good catch — fixed, and you were right that the deeper issue was where the reset lived. I took the "centralize it at the shared config-record boundary" option: invalidateProfileScopedQueries() (already called from the store/profile.ts subscriber and store/gateway-switch.ts) now hard-resets hermes-config-record via resetQueries while everything else keeps stale-while-revalidate invalidation. The reset no longer depends on any particular panel being mounted, and ConfigSettings just drops its local draft like every other consumer.

While auditing the remaining saveHermesConfig / useHermesConfigRecord consumers for the same class of bug, I found and fixed two more sibling paths beyond VoiceProviderFields — details in the PR comment below.

@teknium1 teknium1 added 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 30, 2026
@zihaochen68
zihaochen68 force-pushed the fix/desktop-settings-workspace-skeleton branch from cdd3488 to fad3c1f Compare July 31, 2026 02:40
@zihaochen68

Copy link
Copy Markdown
Author

Updated per review — rebased onto current main (v0.19.1) and pushed fad3c1f1e, which addresses the sibling-path feedback by centralizing the reset and covering every config-draft holder I could find:

Centralized boundary (lib/query-client.ts)

  • invalidateProfileScopedQueries() now hard-resets hermes-config-record (resetQueries: data → undefined, refetch for active observers) while other profile-scoped keys keep stale-while-revalidate invalidation. It runs at the switch boundary (store/profile.ts subscriber + store/gateway-switch.ts), so no consumer can read profile A's record after a switch regardless of which panels are mounted. HERMES_CONFIG_QUERY_KEY moved here (lib must not import from app); use-config-record.ts re-exports it.

Sibling paths audited (every saveHermesConfig / useHermesConfigRecord consumer):

  • VoiceProviderFields (the one you flagged — stays mounted in the Capabilities detail): one-shot seeded ref replaced with the same state-derived re-seeding as ConfigSettings; useOnProfileSwitch drops the draft and zeroes saveVersion so the pending debounced autosave is cancelled by its effect cleanup; the post-save setHermesConfigCache mirror is version-guarded so a save already in flight at switch time can't stomp profile B's freshly-reset cache.
  • SessionsSettingsAutoArchiveSetting: held a raw whole-record copy fetched once on mount and PUT it back on toggle — profile A's record could be written into B verbatim. Now drops the copy on switch and refetches against the new backend (the effect's alive flag discards a slow in-flight profile-A response).
  • ConfigSettings: cache mirror moved inside the existing saveVersionRef guard (same in-flight stomp), and its component-level reset was removed in favor of the central one.
  • ModelSettings.writeAgentDefault: the optimistic-rollback on save failure now only runs within the same profile epoch, so a failed profile-A save can't write A's record into B's cache after a switch.
  • Verified not affected: mcp-tab.tsx (already epoch/profilePending-guarded — and the central reset shrinks its stale-data window further), and the immediate read-modify-write call sites (store/voice-prefs.ts, i18n/context.tsx, settings reset-to-defaults) which fetch fresh state at interaction time and hold no persistent draft.

Tests

  • New voice-provider-fields.test.tsx: mounted A→B switch with a dirty draft — the pending autosave is cancelled (saveHermesConfig never called) and the panel reseeds B's record; plus a no-switch control proving autosave still fires.
  • New sessions-settings.test.tsx: after A→B, the toggle reseeds B's values and persisting writes a record based on B's copy, not A's.
  • query-client.test.ts: the boundary drops config-record data immediately while other profile-scoped caches stay stale-while-revalidate.
  • Full desktop suite: 4011 passed; tsc, eslint, prettier clean.

@zihaochen68
zihaochen68 force-pushed the fix/desktop-settings-workspace-skeleton branch from fad3c1f to c564086 Compare August 3, 2026 06:05
@zihaochen68

Copy link
Copy Markdown
Author

@teknium1 gentle ping — the sibling-path feedback (centralized config-record reset + VoiceProviderFields / AutoArchiveSetting / ModelSettings) is in c56408653, and the branch is freshly rebased onto current main. Happy to adjust further if anything else stands out on re-review.

@zihaochen68
zihaochen68 force-pushed the fix/desktop-settings-workspace-skeleton branch from c564086 to b41c32f Compare August 10, 2026 04:00
@zihaochen68

Copy link
Copy Markdown
Author

@teknium1 gentle follow-up — still hoping for a re-review when you have a moment.

The sibling-path feedback (centralized hermes-config-record reset + VoiceProviderFields / AutoArchiveSetting / ModelSettings) remains in place, and the branch was just rebased onto current main (b41c32f07). Happy to adjust further if anything else stands out.

Also for triage context: this PR supersedes the narrower hook-only fix in #61826 (same Strict Mode useOnProfileSwitch root cause, plus the draft reseeding / cross-profile save coverage requested in review).

zihaochen68 and others added 2 commits August 17, 2026 15:35
… drafts

Rebased onto current main (per-profile hermesConfigKey preserved).

- Compare normalized profile keys in useOnProfileSwitch (Strict Mode safe)
- State-derived draft reseeding in ConfigSettings / VoiceProviderFields
- Central hermes-config-record hard reset in invalidateProfileScopedQueries
- Cancel pending autosaves / drop AutoArchive copies on profile switch
- Version-/epoch-guard in-flight cache writes (ConfigSettings, ModelSettings)

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@zihaochen68
zihaochen68 force-pushed the fix/desktop-settings-workspace-skeleton branch from b41c32f to 21dfb5b Compare August 17, 2026 07:37
@zihaochen68

Copy link
Copy Markdown
Author

@teknium1 gentle follow-up — still hoping for a re-review when you have a moment.

Just rebased onto current main (454554c6a). While rebasing I preserved the newer per-profile hermesConfigKey(...) API that landed on main, and kept the review fixes in place:

  • Strict Mode–safe useOnProfileSwitch (normalized profile keys)
  • State-derived draft reseeding
  • Centralized hermes-config-record hard reset in invalidateProfileScopedQueries
  • Sibling paths: VoiceProviderFields / AutoArchiveSetting / ModelSettings (+ regression tests)

Also noting #61826 was closed unmerged — this PR still covers that hook fix and the broader draft/cross-profile save class from review. Happy to adjust further if anything else stands out.

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

Labels

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-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.

[Bug]: Desktop Settings Workspace stuck on skeleton under Strict Mode

3 participants