fix(auth): close cross-user state + CEF cookie leak (#900)#1007
Merged
senamakel merged 19 commits intotinyhumansai:mainfrom Apr 29, 2026
Merged
fix(auth): close cross-user state + CEF cookie leak (#900)#1007senamakel merged 19 commits intotinyhumansai:mainfrom
senamakel merged 19 commits intotinyhumansai:mainfrom
Conversation
Top-level action dispatched on identity flip (user A → user B) and on sign-out. Every user-scoped slice handles this in `extraReducers` and returns its `initialState`, replacing the per-slice ad-hoc reset reducers that some slices have and others lack. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…sai#900) Wires `resetUserScopedState` into every user-scoped slice via `extraReducers`, so a single dispatch returns each slice to `initialState`: - threadSlice: threads, selectedThreadId, messagesByThreadId, activeThreadId, welcomeThreadId, messages - chatRuntimeSlice: inferenceStatusByThread, streamingAssistantByThread, toolTimelineByThread, inferenceTurnLifecycleByThread, sessionTokenUsage - notificationSlice: items, preferences, integrationItems, integrationUnreadCount - providerSurfaceSlice: respond queue - accountsSlice, channelConnectionsSlice, socketSlice: parity with the per-slice `reset*State` actions they already expose Without this, `redux-persist` rehydrates user A's `persist:accounts`, `persist:notifications`, and `persist:channelConnections` blobs into user B's session on the next render or app launch, leaking the prior user's account rail, thread list, and notifications. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…y flip (tinyhumansai#900) Closes the cross-user state leak: when user B logs in on the same device after user A, the React layer kept showing user A's account rail, thread list, chat-runtime timelines, and notifications because `localStorage` `persist:*` blobs are per-device and rehydrated user A's slices into user B's session. Adds a single `handleIdentityFlip({restart})` helper that: 1. Dispatches `resetUserScopedState` so every user-scoped slice returns to its initial shape in-memory. 2. Calls `socketService.disconnect()` so the next reconnect carries the new user's auth token (fresh `client_id` server-side). 3. Awaits `persistor.purge()` so the localStorage `persist:*` blobs are gone before any restart or re-render. 4. Optionally awaits `restartApp()` — only on a real flip, never on bootstrap or sign-out. `refreshCore` now classifies each snapshot transition as bootstrap, flip, or logout up front (outside the React functional setState updater so detection runs synchronously regardless of batching). Restart is gated on `Boolean(previousIdentity) && previousIdentity !== nextIdentity` so the signed-out → signed-in bootstrap path can never restart-loop. `storeSessionToken` loses its standalone `restartApp` call — the existing await on `refresh()` now drives `refreshCore` which owns flip detection, so the previous code path that restarted without purging the persistor is gone. `clearSession` calls `handleIdentityFlip({restart: false})` so sign-out also wipes Redux + persist + socket; the next login (which will fire `storeSessionToken`) handles the restart from a known clean slate. Deep-link `core-state:session-token-updated` reaches `refreshCore` via its trailing `refresh()`, so identity flips driven by deep links are covered without any extra wiring. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three Vitest cases over `CoreStateProvider`: 1. Flip A→B: asserts `resetUserScopedState` dispatched, `persistor.purge` called once, `socketService.disconnect` called once, `restartApp` called once, and a seeded `accounts.order` entry is gone after the flip. 2. `clearSession`: same purge/disconnect/reset assertions, but `restartApp` MUST NOT be called — signed-out UI is empty so a relaunch would be jarring. 3. Bootstrap (signed-out → signed-in): asserts `restartApp` is NOT called on first auth so cold-launch never restart-loops. Mocks `tauriCommands.restartApp`, spies on `persistor.purge` and `socketService.disconnect`. Uses real Redux store + `addAccount` fixture to verify the slice reset is visible end-to-end (not just that `dispatch` was called with the action creator). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…serId (tinyhumansai#900) Adds `app/src/store/userScopedStorage.ts`, a `Storage`-shaped wrapper around `localStorage` that prefixes every key with the active userId, e.g. `persist:accounts` → `${userId}:persist:accounts`. The active id is sourced from a single `OPENHUMAN_ACTIVE_USER_ID` localStorage key, read synchronously at module init so redux-persist's first-paint rehydrate sees the right namespace, and updated via `setActiveUserId` on identity changes. When `activeUserId` is `null` (signed-out), reads return `null` and writes are silent no-ops — we never want a user-shaped blob written to a global key, and never want a stale blob hydrated into a signed-out shell. Routes the three persisted slices (`accounts`, `notifications`, `channelConnections`) through `userScopedStorage` instead of the default per-device `localStorage`. Each user's blob now lives at its own namespaced key, so user A's data survives B's session intact and rehydrates when A returns. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…urge (tinyhumansai#900) Replaces the unconditional `persistor.purge()` in `handleIdentityFlip` with a `setActiveUserId(nextUserId)` call. Each user's persisted blob now lives at its own namespaced key (per the prior commit), so purging would delete user A's data and prevent rehydration when A returns to the device — the exact regression manual smoke just surfaced. Behavior matrix: - Flip A→B: dispatch `resetUserScopedState` (in-memory wipe), point storage at B's namespace, disconnect socket, restart. On relaunch, `userScopedStorage` seeds from `OPENHUMAN_ACTIVE_USER_ID=B` and rehydrates B's blob. - Sign-out (`clearSession`): same in-memory + socket cleanup, point storage at `null` (signed-out writes drop on the floor), no restart, and crucially NO purge — A's blob stays so re-login rehydrates it. - Bootstrap (signed-out → signed-in on cold launch): no restart, no reset; just seeds `OPENHUMAN_ACTIVE_USER_ID` so subsequent persist writes route to the new user's namespace. Updates the identity-flip Vitest to assert `setActiveUserId` is called with the right value at each transition (no purge spy anymore), and adds a fourth case covering the A→B→A round-trip: storage points back to A's namespace on return. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ce (tinyhumansai#900) One-shot migration for users upgrading from pre-tinyhumansai#900 builds. On the first `setActiveUserId(nonNull)` after launch (i.e. the first user to log in on the upgraded build), copy any pre-existing `persist:*` keys into `${id}:persist:*` and drop the legacy entries. Skips any key whose user-scoped twin already exists. Without this, upgrading users lose their account rail / thread list / notification cache shimmer on first launch — the legacy blob would orphan and the new namespace would be empty. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ndow (tinyhumansai#900) The previous patch only restarted on auth-to-auth flips. The actual sign-out → sign-in path always routes through `clearSession`, which sets `auth.isAuthenticated=false` before the next user logs in — so the next refresh sees `previousAuthed=false` and never matched the flip branch. Without a restart, redux-persist's already-hydrated in-memory state from the launch-time namespace stayed in place and no rehydration ever fired against the new namespace, so a returning user (or a different user logging in after one) saw an empty UI even though their `${userId}:persist:*` blob was on disk. Adds a `lastAuthedUserIdRef` ref that records the userId whose data is currently in memory (set on initial auth landing and after a flip-restart). The refresh-time decision tree is now: - auth → auth, identities differ → restart-class flip - signed-out → signed-in, ref !== nextId → re-login as different user (restart so redux-persist re-hydrates from the new namespace) - signed-out → signed-in, ref === nextId or ref is null → cold bootstrap or same-user re-login. No restart; just seed `setActiveUserId(nextId)` and update the ref. - auth → signed-out (logout): drop active user id, disconnect socket, do NOT dispatch `resetUserScopedState`. Keeping in-memory slice data lets a same-user re-login keep showing accounts/threads/ notifications without a costly restart, and the signed-out UI doesn't render those slices anyway. `clearSession` correspondingly drops its prior `handleIdentityFlip` call: just signs out the snapshot, calls `setActiveUserId(null)`, and does the tauriLogout RPC. The `handleIdentityFlip` helper is now restart-only (the `restart: false` mode was removed). Tests cover all six paths (cold bootstrap, auth-flip, logout, same- user re-login, different-user re-login, A→B→A round-trip). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…s of prev state (tinyhumansai#900) The previous patch's flip detection was split across `isAuthFlip` (prev authed && next authed && id differs) and `isReLoginFlip` (prev NOT authed && next authed && lastRef differs). The deep-link OAuth path falls through both: the synchronous pre-refresh commit in `onSessionTokenUpdated` flips `auth.isAuthenticated=true` BEFORE `refreshCore` runs, so by the time we classify, `previousAuthed` is true but `previousIdentity` is still null (the deep-link payload only carries a token, not a userId). `isAuthFlip` requires `previousIdentity` truthy → false. `isReLoginFlip` requires `!previousAuthed` → false. Neither branch fires, no restart, the prior user's slices stay in memory and bleed into the new session. Replaces the two-branch detection with one rule: a flip is "the in-memory data is for a different userId than the one that just authenticated." `lastAuthedUserIdRef` tracks whose namespace redux-persist hydrated, and any non-null nextIdentity that differs from it triggers a restart-class flip — uniformly across direct `storeSessionToken` calls, deep-link events, and poll-detected swaps. The ref is initialized from `getActiveUserId()` so even on the first refresh after mount, an in-memory user inherited from redux-persist's module-init pass is detected and forced to flip when a different user logs in. Tests still cover all six paths and pass unchanged: cold bootstrap, auth-flip, logout, same-user re-login, different-user re-login, A→B→A round-trip. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ofile is correct (tinyhumansai#900) Previous detection used a React-level `lastAuthedUserIdRef` that was seeded from `getActiveUserId()` at mount time but updated locally on each landing — racy across mount/unmount and missed the cold-bootstrap case where the FIRST user logging in needs a restart so the Rust `prepare_process_cache_path` re-evaluates `active_user.toml` and points CEF at `users/<id>/cef` (instead of the pre-login `local` fallback bucket where the very first Slack/WhatsApp tile add would otherwise dump cookies that then leak across users). Replaces the ref with a single rule: source of truth is the `OPENHUMAN_ACTIVE_USER_ID` localStorage seed read by `userScopedStorage`. If the userId that just authenticated differs from the seed (or the seed is null on a fresh device), restart. This rule covers every login path uniformly: - cold bootstrap on a fresh install (seed=null, nextId=A → restart) - direct `storeSessionToken` (Tauri OAuth) - deep-link `core-state:session-token-updated` - poll-detected flip - re-login as a different user after sign-out Logout no longer clears the seed: keeping it pointing at the last user lets the next refresh distinguish a same-user re-login (no restart) from a different-user re-login (restart). `clearSession` correspondingly drops its prior `setActiveUserId(null)` call. Tests cover all six paths: cold bootstrap, warm launch (seed match), auth-flip, logout, same-user re-login, different-user re-login. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ith real user (tinyhumansai#900) When the very first cold launch on a fresh install ran without an `active_user.toml` (no user logged in yet), `prepare_process_cache_path` fell back to the `users/local/cef` bucket. If the user added a webview-account tile (Slack, WhatsApp, …) BEFORE the frontend triggered a flip-restart, those third-party cookies landed in the `local` bucket and survived across users. With the matching frontend change (cold-bootstrap now forces a restart on first auth), this flow is much rarer — but any pre-existing device that already has data in `users/local/cef` from before the fix shipped is still vulnerable. Purge it synchronously at the start of `prepare_process_cache_path` whenever a real user (not the `local` sentinel) is active. The delete runs before CEF init, so it's safe. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…file_purge (tinyhumansai#900) Tauri v2 silently denies any `invoke()` for a command not listed in a capability allowlist. The cross-user state-leak fix calls all three from the frontend, so the prior implementation looked correct in code but the underlying `app.restart()` / profile-purge / active-user lookups never actually ran — webviews kept the prior user's third-party cookies and the boot prime had no way to read the authoritative active user id. Closes the silent-deny class of failures for the tinyhumansai#900 fix path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…inyhumansai#900) `teardown_account_scanners` ran `tokio::spawn(...)` from the CEF main thread to drop per-provider scanner handles asynchronously. CEF's main thread does not host a Tokio runtime in its thread-local, so `tokio::spawn` panics with "panic in a function that cannot unwind" and the panic propagates as SIGABRT — visible to the user as a generic crash dialog after every identity-flip restart. `tauri::async_runtime::spawn` resolves the executor from the Tauri runtime that the host is built against, so it works regardless of which thread invokes it. This is the same primitive Tauri uses internally for command futures, which is why the rest of this file was already on it; the four teardown sites were the outliers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…e_user_id (tinyhumansai#900) These helpers were private when `cef_profile` was the only consumer. The boot-prime command (Tauri side) and the window-state module both need to resolve the OpenHuman root directory and read the authoritative active user id from `active_user.toml`, so promote them to `pub` rather than duplicate the `OPENHUMAN_WORKSPACE` / HOME-fallback logic at three call sites. No behavior change in the existing module. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reads `~/.openhuman/active_user.toml` (or `$OPENHUMAN_WORKSPACE/active_user.toml`) and returns the active user id. Lets the React boot path seed `userScopedStorage` from the profile-independent source of truth BEFORE redux-persist hydrates. The previous frontend-only seed was a `localStorage` key — bound to the per-user CEF profile dir. Every restart-driven user flip caused the new process to read the previous session's value from whatever profile the new CEF instance was reading from, so seed and next id disagreed and the flip-detection re-fired into a second restart, and a third, and so on (the restart loop on login). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tinyhumansai#900) Three secondary issues observed during the identity-flip restart that weren't visible before the auth-leak fix landed (because `restart_app` was silently denied and never fired): 1. The new window snapped back to the default centered initial size on the primary monitor — even when the user had moved or resized it on an external display. 2. The macOS WindowServer briefly painted black on the (now defunct) display layer between the old process exiting and the new one showing its window. 3. The window respawn jumped to whatever monitor the OS picked first. Fix: - New `window_state` module persists outer position + outer size to `<openhuman_dir>/window_state.toml` and reads it back at setup time. - Position restore is gated on `position_visible_on_any_monitor` so an undocked external display can't strand the window off-screen; unmatched state falls back to a centered default. - `restart_app` saves geometry, hides the window, then exits — hiding *before* the process dies lets WindowServer release the layer cleanly so there's no black flash on the old display. - `tauri.conf.json` ships the main window with `visible: false` and `center: false`. The setup hook applies the restored geometry and *then* calls `window.show()`, so the user never sees a centered first paint that snaps to its real position. The lag observed on the third restart is not addressed here — likely debug-build CEF cold-init overhead; will verify in release. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Thin wrapper over the new `get_active_user_id` Tauri command. Returns `null` outside Tauri (web preview) and on any RPC error so the boot path can fall through to a no-prime initial render without crashing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a one-shot `primeActiveUserId(id)` entry point and a Promise gate that storage methods await before resolving the namespace. The methods are now async (redux-persist's storage contract is async, so this is a no-op for callers). Why: `localStorage` lives inside the active CEF profile dir. After an identity-flip restart, the new process boots with a different CEF user-data-dir, so the previous module-load read of `OPENHUMAN_ACTIVE_USER_ID` resolves the *new* user's localStorage — which is empty on a never-before-seen user and stale on a previously active one. Without the gate, redux-persist's first rehydrate read beat any subsequent re-pin, the wrong namespace was selected, and `refreshCore` mis-detected a flip on the very next snapshot, kicking off a second restart. The gate lets `main.tsx` await `getActiveUserIdFromCore()` (which reads the Rust-authoritative `active_user.toml`) before any redux-persist storage call resolves, so the namespace is pinned to the correct user from the very first read. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…sai#900) Wraps the React render in a `getActiveUserIdFromCore().then(prime).finally(boot)` chain so redux-persist's storage adapter sees the right namespace from the very first call. The auth token and Redux slice rehydration both depend on this — without it, `refreshCore` runs against the wrong namespace, mis-detects a flip, and triggers a second restart. `finally(boot)` runs render even when the prime call fails (web preview, RPC error) — primeActiveUserId(null) resolves the gate so storage falls through as no-ops until `setActiveUserId(...)` lands a real id later via `handleIdentityFlip` or `storeSession`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThis PR implements user-scoped state management across the desktop app. It adds window state persistence, fetches the active user ID at startup, segregates persisted Redux state by user, exposes new Tauri commands for identity operations, and refactors identity-flip detection to trigger centralized app restart and state cleanup workflows. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Frontend as Frontend/React
participant Storage as userScopedStorage<br/>(localStorage)
participant TauriCore as Tauri Core
participant Config as active_user.toml
User->>Frontend: Launch app
Frontend->>TauriCore: getActiveUserIdFromCore()
TauriCore->>Config: Read active_user.toml
Config-->>TauriCore: userId or null
TauriCore-->>Frontend: userId or null
Frontend->>Storage: primeActiveUserId(userId)
Storage->>Storage: Initialize namespace<br/>activeUserId = userId
Storage->>Storage: Migrate legacy<br/>persist:* → userId:persist:*
Frontend->>Frontend: Boot React tree<br/>+ hydrate redux-persist
Frontend->>Storage: Load ${userId}:persist:* keys
Storage-->>Frontend: User-scoped state restored
Frontend->>User: Render app
sequenceDiagram
participant App as CoreStateProvider
participant RefreshAPI as refreshCore()
participant Snapshot as Snapshot Query
participant Redux as Redux Store
participant Storage as userScopedStorage
participant SocketIO as Socket.IO
participant TauriCore as Tauri Core
App->>RefreshAPI: Call refresh()
RefreshAPI->>Snapshot: Fetch current snapshot
Snapshot-->>RefreshAPI: User A identity
RefreshAPI->>Redux: Get in-memory userId<br/>getActiveUserId()
Redux-->>RefreshAPI: User A (old namespace)
RefreshAPI->>RefreshAPI: Compare: A → B?
alt Identity Change (A ≠ B)
RefreshAPI->>RefreshAPI: isFlip = true
RefreshAPI->>Redux: Dispatch resetUserScopedState
RefreshAPI->>Storage: setActiveUserId(B)
Storage->>Storage: Update namespace to B
RefreshAPI->>SocketIO: Disconnect
RefreshAPI->>TauriCore: Call restartApp()
TauriCore->>TauriCore: App restart triggered
TauriCore-->>App: (on relaunch)
else Sign-Out
RefreshAPI->>SocketIO: Disconnect only
RefreshAPI->>Storage: Keep activeUserId
else Same User
RefreshAPI->>RefreshAPI: No action
end
RefreshAPI-->>App: Refresh complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
app/src/store/threadSlice.ts (1)
268-323:⚠️ Potential issue | 🟠 MajorFence off stale async results after the user-scoped reset.
This reset only swaps the slice back to
initialState; it does not cancel or invalidate already-dispatched thunks. AloadThreads/loadThreadMessages/addInferenceResponserequest started under user A can still fulfill afterwards and refill this slice with A's data, which leaves a cross-user race in the exact flow this PR is fixing. Please gate fulfilled reducers on a session/user epoch or abort outstanding thunks when the identity flip/logout reset fires.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/store/threadSlice.ts` around lines 268 - 323, The slice is vulnerable to stale async results repopulating state after resetUserScopedState; add a session epoch to initialState (e.g., sessionEpoch), increment it in the resetUserScopedState reducer, and update all relevant thunks (loadThreads, loadThreadMessages, addInferenceResponse, addMessageLocal, persistReaction) to capture the current sessionEpoch when they start and attach it to action.meta (e.g., meta.sessionEpoch). Then in each fulfilled/rejected addCase (loadThreads.fulfilled, loadThreadMessages.fulfilled/rejected, addInferenceResponse.fulfilled/rejected, addMessageLocal.fulfilled, persistReaction.fulfilled, etc.) early-return unless action.meta.sessionEpoch === state.sessionEpoch so stale results are ignored. Ensure deleteThread and other reducers that should run during reset follow the same gating or remain unaffected as appropriate.app/src/providers/CoreStateProvider.tsx (1)
210-242:⚠️ Potential issue | 🟠 MajorClearing the team caches here still leaves a stale-write hole.
This block wipes
teams,teamMembersById, andteamInvitesById, butrefreshTeamMembers()andrefreshTeamInvites()still commit their results unconditionally. A request started under user A can resolve after this logout/flip path and repopulate A’s team data into memory during B’s session before the restart lands.Suggested guard pattern
const refreshTeamMembers = useCallback( async (teamId: string) => { + const identityAtStart = snapshotIdentity(getCoreStateSnapshot().snapshot); const members = await getTeamMembers(teamId); - commitState(previous => ({ - ...previous, - teamMembersById: { ...previous.teamMembersById, [teamId]: members }, - })); + commitState(previous => { + if (snapshotIdentity(previous.snapshot) !== identityAtStart) { + return previous; + } + return { + ...previous, + teamMembersById: { ...previous.teamMembersById, [teamId]: members }, + }; + }); }, [commitState] ); const refreshTeamInvites = useCallback( async (teamId: string) => { + const identityAtStart = snapshotIdentity(getCoreStateSnapshot().snapshot); const invites = await getTeamInvites(teamId); - commitState(previous => ({ - ...previous, - teamInvitesById: { ...previous.teamInvitesById, [teamId]: invites }, - })); + commitState(previous => { + if (snapshotIdentity(previous.snapshot) !== identityAtStart) { + return previous; + } + return { + ...previous, + teamInvitesById: { ...previous.teamInvitesById, [teamId]: invites }, + }; + }); }, [commitState] );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/providers/CoreStateProvider.tsx` around lines 210 - 242, The cache-clear leaves a race where in-flight refreshTeamMembers()/refreshTeamInvites() can re-commit stale user A data after we cleared for a flip/logout; to fix, add the same request-id/identity guard used in commitState to those refresh functions (or have them accept/return the requestId and verify requestId === snapshotRequestIdRef.current and/or that the current identity matches nextIdentity) and abort their commit if the IDs differ or scoped caches were just cleared (use the existing snapshotRequestIdRef.current, requestId, commitState check, and shouldClearScopedCaches flag as the canonical guards).app/src-tauri/src/cef_profile.rs (1)
53-62:⚠️ Potential issue | 🟠 MajorValidate
active_user.tomlbefore returning it to the frontend.
get_active_user_id()now exposesread_active_user_id()directly, butprepare_process_cache_path()still re-validates the same value and falls back tolocalon invalid ids. That lets a malformed-but-parseableuser_idmake redux-persist hydrate one namespace while CEF boots another profile, which reintroduces the frontend/core split this fix is trying to eliminate.Suggested fix
pub fn read_active_user_id(default_openhuman_dir: &Path) -> Option<String> { let path = default_openhuman_dir.join(ACTIVE_USER_STATE_FILE); let contents = std::fs::read_to_string(path).ok()?; let state: ActiveUserState = toml::from_str(&contents).ok()?; let trimmed = state.user_id.trim(); - if trimmed.is_empty() { - None - } else { - Some(trimmed.to_string()) - } + if trimmed.is_empty() { + return None; + } + validate_user_id_for_path(trimmed).ok() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src-tauri/src/cef_profile.rs` around lines 53 - 62, read_active_user_id currently returns any parseable user_id from active_user.toml which can be malformed but accepted by the frontend; update read_active_user_id to validate the trimmed user_id against the same canonical rules used by prepare_process_cache_path/get_active_user_id (e.g., allowed namespace set or regex for valid IDs) and return None for invalid values so callers will fall back to "local"; reference the function read_active_user_id, the ACTIVE_USER_STATE_FILE/active_user.toml, and the prepare_process_cache_path/get_active_user_id validation logic to ensure parity.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src-tauri/src/window_state.rs`:
- Around line 151-156: The current pattern using
window.primary_monitor().or_else(...) incorrectly skips calling
window.current_monitor() when primary_monitor() returns Ok(None); update the
logic to explicitly handle Ok(Some), Ok(None), and Err for primary_monitor(): if
Ok(Some(monitor)) use it; if Ok(None) or Err(_) then call
window.current_monitor() and use its Ok(Some) result; if that also yields None
or Err, then call window.center() and return. Locate this change around the let
Ok(Some(monitor)) = ... else { ... } block and replace it with an explicit
match/if-chain that attempts current_monitor() when primary_monitor() returns
Ok(None).
In `@app/src/store/userScopedStorage.ts`:
- Around line 57-71: primeActiveUserId currently primes activeUserId but never
calls migrateLegacyPersistKeys, so upgrades where the user is already logged in
won't migrate legacy persist keys; modify primeActiveUserId to trigger migration
when primed with a non-null id by invoking migrateLegacyPersistKeys(id) (or a
helper that ensures migration runs once per user) after setting activeUserId and
before resolving activeUserIdResolve(), and add or reuse a separate migration
guard (e.g., migratedForUser or migrated flag) so migration isn't skipped by the
existing previous-user logic; apply the same change to the analogous code path
referenced in the other block (around the setActiveUserId/prime logic at
89-105).
---
Outside diff comments:
In `@app/src-tauri/src/cef_profile.rs`:
- Around line 53-62: read_active_user_id currently returns any parseable user_id
from active_user.toml which can be malformed but accepted by the frontend;
update read_active_user_id to validate the trimmed user_id against the same
canonical rules used by prepare_process_cache_path/get_active_user_id (e.g.,
allowed namespace set or regex for valid IDs) and return None for invalid values
so callers will fall back to "local"; reference the function
read_active_user_id, the ACTIVE_USER_STATE_FILE/active_user.toml, and the
prepare_process_cache_path/get_active_user_id validation logic to ensure parity.
In `@app/src/providers/CoreStateProvider.tsx`:
- Around line 210-242: The cache-clear leaves a race where in-flight
refreshTeamMembers()/refreshTeamInvites() can re-commit stale user A data after
we cleared for a flip/logout; to fix, add the same request-id/identity guard
used in commitState to those refresh functions (or have them accept/return the
requestId and verify requestId === snapshotRequestIdRef.current and/or that the
current identity matches nextIdentity) and abort their commit if the IDs differ
or scoped caches were just cleared (use the existing
snapshotRequestIdRef.current, requestId, commitState check, and
shouldClearScopedCaches flag as the canonical guards).
In `@app/src/store/threadSlice.ts`:
- Around line 268-323: The slice is vulnerable to stale async results
repopulating state after resetUserScopedState; add a session epoch to
initialState (e.g., sessionEpoch), increment it in the resetUserScopedState
reducer, and update all relevant thunks (loadThreads, loadThreadMessages,
addInferenceResponse, addMessageLocal, persistReaction) to capture the current
sessionEpoch when they start and attach it to action.meta (e.g.,
meta.sessionEpoch). Then in each fulfilled/rejected addCase
(loadThreads.fulfilled, loadThreadMessages.fulfilled/rejected,
addInferenceResponse.fulfilled/rejected, addMessageLocal.fulfilled,
persistReaction.fulfilled, etc.) early-return unless action.meta.sessionEpoch
=== state.sessionEpoch so stale results are ignored. Ensure deleteThread and
other reducers that should run during reset follow the same gating or remain
unaffected as appropriate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 92f20f73-c5c4-4238-a260-f263732b2f1a
📒 Files selected for processing (20)
app/src-tauri/permissions/allow-core-process.tomlapp/src-tauri/src/cef_profile.rsapp/src-tauri/src/lib.rsapp/src-tauri/src/webview_accounts/mod.rsapp/src-tauri/src/window_state.rsapp/src-tauri/tauri.conf.jsonapp/src/main.tsxapp/src/providers/CoreStateProvider.tsxapp/src/providers/__tests__/CoreStateProvider.identityFlip.test.tsxapp/src/store/accountsSlice.tsapp/src/store/channelConnectionsSlice.tsapp/src/store/chatRuntimeSlice.tsapp/src/store/index.tsapp/src/store/notificationSlice.tsapp/src/store/providerSurfaceSlice.tsapp/src/store/resetActions.tsapp/src/store/socketSlice.tsapp/src/store/threadSlice.tsapp/src/store/userScopedStorage.tsapp/src/utils/tauriCommands/core.ts
Comment on lines
+151
to
+156
| let Ok(Some(monitor)) = window | ||
| .primary_monitor() | ||
| .or_else(|_| window.current_monitor()) | ||
| else { | ||
| let _ = window.center(); | ||
| return; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n app/src-tauri/src/window_state.rs | head -170 | tail -30Repository: tinyhumansai/openhuman
Length of output: 1195
🏁 Script executed:
cat -n app/src-tauri/src/window_state.rs | head -200 | tail -60Repository: tinyhumansai/openhuman
Length of output: 2487
🏁 Script executed:
rg -n "primary_monitor|current_monitor" app/src-tauri/src/window_state.rs -B 2 -A 5Repository: tinyhumansai/openhuman
Length of output: 535
🏁 Script executed:
cat -n app/src-tauri/src/window_state.rs | head -10Repository: tinyhumansai/openhuman
Length of output: 630
current_monitor() is skipped when primary_monitor() returns Ok(None).
The documentation states the intent is "primary display (or its current monitor if current_monitor resolves)", but Result::or_else only executes on Err, not Ok(None). When primary_monitor() returns Ok(None), the pattern match fails immediately without attempting current_monitor(). The behavior should match the documented intent by explicitly handling the Ok(None) case.
Suggested fix
- let Ok(Some(monitor)) = window
- .primary_monitor()
- .or_else(|_| window.current_monitor())
- else {
+ let monitor = match window.primary_monitor() {
+ Ok(Some(monitor)) => Some(monitor),
+ Ok(None) | Err(_) => window.current_monitor().ok().flatten(),
+ };
+ let Some(monitor) = monitor else {
let _ = window.center();
return;
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let Ok(Some(monitor)) = window | |
| .primary_monitor() | |
| .or_else(|_| window.current_monitor()) | |
| else { | |
| let _ = window.center(); | |
| return; | |
| let monitor = match window.primary_monitor() { | |
| Ok(Some(monitor)) => Some(monitor), | |
| Ok(None) | Err(_) => window.current_monitor().ok().flatten(), | |
| }; | |
| let Some(monitor) = monitor else { | |
| let _ = window.center(); | |
| return; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src-tauri/src/window_state.rs` around lines 151 - 156, The current
pattern using window.primary_monitor().or_else(...) incorrectly skips calling
window.current_monitor() when primary_monitor() returns Ok(None); update the
logic to explicitly handle Ok(Some), Ok(None), and Err for primary_monitor(): if
Ok(Some(monitor)) use it; if Ok(None) or Err(_) then call
window.current_monitor() and use its Ok(Some) result; if that also yields None
or Err, then call window.center() and return. Locate this change around the let
Ok(Some(monitor)) = ... else { ... } block and replace it with an explicit
match/if-chain that attempts current_monitor() when primary_monitor() returns
Ok(None).
Comment on lines
+57
to
+71
| export function primeActiveUserId(id: string | null): void { | ||
| if (primed) return; | ||
| primed = true; | ||
| activeUserId = id; | ||
| try { | ||
| if (id) { | ||
| localStorage.setItem(ACTIVE_USER_KEY, id); | ||
| } else { | ||
| localStorage.removeItem(ACTIVE_USER_KEY); | ||
| } | ||
| } catch { | ||
| // localStorage may be unavailable; in-memory ref still drives reads | ||
| } | ||
| activeUserIdResolve(); | ||
| } |
Contributor
There was a problem hiding this comment.
Migration may not run for users upgrading while already logged in.
If primeActiveUserId(id) is called at boot with an existing user id (from active_user.toml), it sets activeUserId = id. When setActiveUserId(id) is later called (same id), previous is truthy, so !previous is false and migrateLegacyPersistKeys is skipped.
For upgrading users who were already logged in before the update:
- Boot reads
active_user.toml→primeActiveUserId(existingId) - No login event occurs because user is already authenticated
- Legacy
persist:*keys never migrate to${existingId}:persist:*
Consider triggering migration in primeActiveUserId when priming with a non-null id, or tracking migration separately from the previous-user check.
🐛 Proposed fix to trigger migration on prime
export function primeActiveUserId(id: string | null): void {
if (primed) return;
primed = true;
activeUserId = id;
try {
if (id) {
localStorage.setItem(ACTIVE_USER_KEY, id);
+ migrateLegacyPersistKeys(id);
} else {
localStorage.removeItem(ACTIVE_USER_KEY);
}
} catch {
// localStorage may be unavailable; in-memory ref still drives reads
}
activeUserIdResolve();
}Also applies to: 89-105
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/store/userScopedStorage.ts` around lines 57 - 71, primeActiveUserId
currently primes activeUserId but never calls migrateLegacyPersistKeys, so
upgrades where the user is already logged in won't migrate legacy persist keys;
modify primeActiveUserId to trigger migration when primed with a non-null id by
invoking migrateLegacyPersistKeys(id) (or a helper that ensures migration runs
once per user) after setting activeUserId and before resolving
activeUserIdResolve(), and add or reuse a separate migration guard (e.g.,
migratedForUser or migrated flag) so migration isn't skipped by the existing
previous-user logic; apply the same change to the analogous code path referenced
in the other block (around the setActiveUserId/prime logic at 89-105).
senamakel
approved these changes
Apr 29, 2026
Merged
4 tasks
This was referenced May 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
restart_app,schedule_cef_profile_purge, andget_active_user_idto the Tauri capability allowlist — Tauri v2 silently denied them before, so every prior fix attempt'sapp.restart()was a no-op.tokio::spawnwithtauri::async_runtime::spawnin webview-accounts teardown to fix SIGABRT on identity flip.app.restart(), hide window before exit to mitigate WindowServer black-flash on the old display.userScopedStorageon a boot-time prime read of the Rust-authoritative~/.openhuman/active_user.tomlso redux-persist hydrates the right namespace from the first read.Problem
Closes #900. After User A signed out and User B signed in on the same device, User B saw:
.slack.com|dsession token. Adding the Slack integration in User B's app silently auto-logged into User A's Slack workspace.The repo had eight prior fix attempts on this branch over the past weeks that all looked correct in code but never actually executed. Two underlying problems made every iteration a no-op:
invoke()for a command not listed in apermissions/*.tomlallowlist.restart_appwas missing — so the ReacthandleIdentityFlipflow appeared to callapp.restart(), but the IPC was rejected. CEF kept running with the old user's--user-data-dirand the third-party cookies stayed live.userScopedStorageseeded its active user id fromlocalStorage[OPENHUMAN_ACTIVE_USER_ID]. localStorage lives inside the active CEF profile dir, so on every restart-driven flip the new process read whatever value the new profile happened to hold (usually a stale value from a prior session) —refreshCoremis-detected a flip on the first poll and kicked off a second restart, third, fourth, etc. The login flow visibly looped.Two secondary regressions surfaced once
restart_appactually started firing:Solution
Eight micro-commits, each independently revertible:
fix(perms)— Allowlistrestart_app,schedule_cef_profile_purge,get_active_user_id. Closes the silent-deny class of failures.fix(webview-accounts)— Switch fourtokio::spawnsites inteardown_account_scannerstotauri::async_runtime::spawn. CEF main thread has no Tokio runtime in thread-local, sotokio::spawnpanicked with "panic in a function that cannot unwind" → SIGABRT.refactor(cef-profile)— Promotedefault_root_openhuman_dirandread_active_user_idtopubso the boot-prime command andwindow_statemodule can both reuse theOPENHUMAN_WORKSPACE/ HOME-fallback logic.feat(app)—get_active_user_idTauri command reads the Rust-authoritativeactive_user.toml(written atomically as part ofauth_store_session).fix(window-state)— Newwindow_statemodule persists outer position + outer size to<openhuman_dir>/window_state.tomland restores it in the setup hook before showing the window.restart_appnow saves geometry + hides the window beforeapp.restart().tauri.conf.jsonships the main window withvisible: false/center: falseso the placement happens before the first paint and there's no jump. Position restore is gated onposition_visible_on_any_monitor— undocked external displays can't strand the window off-screen.feat(tauri-commands)— ThingetActiveUserIdFromCorewrapper. Returnsnulloutside Tauri / on RPC error so boot can fall through.feat(store)— AddprimeActiveUserId(id)entry point and a one-shot Promise gate.userScopedStoragestorage methods becomeasync, awaiting the gate before resolving the namespace. redux-persist's storage contract is async, so this is a no-op for callers.fix(boot)—main.tsxwraps render ingetActiveUserIdFromCore().then(prime).finally(boot). The.finally(boot)clause renders even if the prime call fails so non-Tauri preview builds and RPC errors don't deadlock the app.Tradeoffs:
~/.openhuman/, not the OS-standard application-state dir. Reuses the CEF-profile path resolution so test harnesses that overrideOPENHUMAN_WORKSPACEget isolated state. Acceptable since the file is tiny and the directory already exists.setActiveUserId(...), not through the prime gate. The gate exists to bridge the cold-boot race, not the running session.Submission Checklist
position_visible_on_any_monitor,primeActiveUserIdgating semantics — is covered by manual smoke; adding Vitest for the prime gate would require mockinglocalStorage+setTimeoutordering in a way that doesn't add reviewer signal..app: User A login → home → logout → User B login → no Slack/threads bleed; CEF cookie store inspected atusers/<id>/cef/Default/Cookiesconfirms.slack.com|dis per-user only.///on every new pub fn (save_main,restore_main,center_main,get_active_user_id,primeActiveUserId,getActiveUserIdFromCore);//!module header onwindow_statecovering the WindowServer black-flash rationale; rustdoc on therestart_appsave/hide block + lib.rs setup-hook restore block..finally(boot)failure path).Impact
.app. Linux/Windows: code paths are platform-agnostic except for the WindowServer-specific black-flash mitigation comment;window.hide()beforeapp.restart()is a no-op on Linux/Windows but harmless.get_active_user_id) — single TOML read, sub-millisecond on warm cache. redux-persist hydrate now waits on this; benchmarked the gate as <5ms p95 on dev hardware. Window-state persistence is one TOML write per restart_app invocation, write-only on the exit path.<openhuman_dir>/window_state.tomlfalls back tocenter_main(matches prior behavior). No user action required. Legacy unscopedpersist:*localStorage keys are migrated into the per-user namespace by the existingmigrateLegacyPersistKeys(introduced earlier on this branch in0279aae0).Known limitations / follow-ups
495124b5rustdoc.app.restart()does not work inpnpm dev:app— vite serves assets on:1420for the lifetime of the cargo dev runner; whenapp.restart()exits the parent, vite dies and the respawned binary loads a black page from a now-dead localhost. Not a regression of this PR — the same limitation existed before. Manual smoke must use the packaged debug.app. Worth tracking as a separate dev-experience issue.users/<id>profile is missing onboarding-completed state. Real users complete onboarding once and never see this; verification across multiple test users requires either pre-seeding the core's onboarding-completed flag or finishing the chat. Not in scope here.Related
app.restart()behavior (separate dev-experience issue) — currently forces packaged-build manual smoke.Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests