Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 29 additions & 7 deletions plans/store-split.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,21 @@ Optimize for store reduction, not store proliferation. Avoid splitting `modal-he

For this repo, assume most RPCs hit a local service and are cheap. Default toward reloading in the owning component instead of keeping a convenience cache unless the data truly needs to survive navigation or serve unrelated entry points.

Bias toward less frontend bookkeeping. If the Go service already owns a piece of data and the UI can cheaply query it on mount or refresh, prefer doing that over mirroring it in Zustand. A store that exists only to cache service-layer data and avoid a few additional local RPCs is not justified here.

Cross-cutting rule: when an engine action is only a screen-local refresh or prompt nudge, prefer the typed engine listener layer over a one-field store or an `init/shared.tsx` forwarding shim.

Cross-cutting rule: do not replace a removed Zustand store with module-local mutable state. No feature-local cache singletons, in-flight dedupe registries, or other hidden module-scope coordination objects that recreate store behavior in disguise. If multiple mounted descendants in one route need the same loaded value, prefer a route-owned screen component or feature-local provider.

Recommended implementation order:

- [x] `settings-email`
- [x] `settings-phone`
- [x] `people`
- [x] `recover-password`
- [ ] `settings-password`
- [x] `settings-password`
- [ ] `tracker`
- [ ] `team-building`
- [x] `team-building`
- [ ] `modal-header` only for param/local-state extraction, not store splitting

## Key Changes
Expand Down Expand Up @@ -135,7 +139,7 @@ Public/API impact:
- Reduce `useModalHeaderState.setState(...)` writes opportunistically.
- Do not introduce new tiny stores as replacements.

### 6. Re-evaluate `settings-password` as a merge candidate only if there is a real home
### 6. Delete `settings-password` instead of keeping a convenience cache

Current shape:

Expand All @@ -144,13 +148,25 @@ Current shape:

Planned change:

- Do not localize this to one screen.
- Merge it only if there is an obviously correct account/session store with the same lifecycle.
- Otherwise leave it as an intentionally retained tiny notification-backed cache.
- Delete `shared/stores/settings-password.tsx`; do not keep a dedicated store just to cache `randomPW`.
- Move the initial load into the owning settings UI via a shared settings-local hook or equivalent feature code.
- Use the typed engine listener layer for mounted updates only, and rely on load-on-mount or explicit refresh instead of keeping a warmed global cache.
- Do not retain frontend bookkeeping only to avoid repeated `userLoadPassphraseState` calls to the local service.
- Do not use a module-local cache or singleton dedupe layer for `randomPW`.
- For the password modal specifically, prefer a single route owner for the loaded value:
- let the password screen own the `randomPW` load and set its own header title/options
- if several mounted descendants in the same route need the value, share it through a feature-local provider rather than module scope

Result:

- Deleted `shared/stores/settings-password.tsx`.
- Replaced it with settings-local mounted logic that queries the service and listens for `NotifyUsers.passwordChanged` via the typed engine listener layer.

Public/API impact:

- None unless a clear merge target emerges.
- Remove `usePWState`.
- Remove `init/shared.tsx` wiring that only forwards `NotifyUsers.passwordChanged` into that store.
- Update settings consumers to own their own load/unknown state.

### 7. Split `tracker` by behavior only where it reduces real store coupling

Expand Down Expand Up @@ -190,6 +206,11 @@ Planned change:
- `sendNotification`
- shared session search results/recommendations if multiple subtrees still need them

Result:

- Removed duplicated provider state for `selectedService`, `searchQuery`, and `searchLimit`.
- Kept the provider for shared builder session state and shared search results.

Public/API impact:

- Preserve `TBProvider`/`useTBContext`.
Expand Down Expand Up @@ -231,6 +252,7 @@ Critical scenarios:
## Assumptions And Defaults

- Prefer route params or local feature state over creating new one-field stores.
- Never use module-local mutable caches/singletons as a substitute for a removed store.
- Prefer the typed engine listener layer over store plumbing when an engine event only nudges mounted feature UI and can be safely missed while unmounted.
- Keep notification-backed shared caches unless they are clearly just convenience state for one screen.
- `modal-header` is not a restructuring target beyond extracting route-owned or screen-owned values.
Expand Down
8 changes: 0 additions & 8 deletions shared/constants/init/shared.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import type * as UseChatStateType from '@/stores/chat'
import type * as UseFSStateType from '@/stores/fs'
import type * as UseNotificationsStateType from '@/stores/notifications'
import type * as UsePinentryStateType from '@/stores/pinentry'
import type * as UseSettingsPasswordStateType from '@/stores/settings-password'
import type * as UseTeamsStateType from '@/stores/teams'
import type * as UseTracker2StateType from '@/stores/tracker'
import type * as UnlockFoldersType from '@/stores/unlock-folders'
Expand Down Expand Up @@ -497,13 +496,6 @@ export const _onEngineIncoming = (action: EngineGen.Actions) => {
usePinentryState.getState().dispatch.onEngineIncomingImpl(action)
}
break
case 'keybase.1.NotifyUsers.passwordChanged':
{
const randomPW = action.payload.params.state === T.RPCGen.PassphraseState.random
const {usePWState} = require('@/stores/settings-password') as typeof UseSettingsPasswordStateType
usePWState.getState().dispatch.notifyUsersPasswordChanged(randomPW)
}
break
case 'keybase.1.NotifyPhoneNumber.phoneNumbersChanged': {
const {list} = action.payload.params
useSettingsPhoneState.getState().dispatch.notifyPhoneNumberPhoneNumbersChanged(list ?? undefined)
Expand Down
16 changes: 6 additions & 10 deletions shared/settings/account/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ import {loadSettings} from '../load-settings'
import {useNavigation, type NavigationProp} from '@react-navigation/native'
import {useIsFocused} from '@react-navigation/core'
import {useConfigState} from '@/stores/config'
import {usePWState} from '@/stores/settings-password'
import {makePhoneError, useSettingsPhoneState} from '@/stores/settings-phone'
import {useSettingsEmailState} from '@/stores/settings-email'
import {type settingsAccountTab, settingsPasswordTab} from '@/constants/settings'
import type {SettingsAccountRouteParams} from '../routes'
import {useRandomPWState} from '../use-random-pw'

export const SettingsSection = ({children}: {children: React.ReactNode}) => (
<Kb.Box2 direction="vertical" gap="tiny" fullWidth={true} style={styles.section}>
Expand Down Expand Up @@ -83,12 +83,12 @@ const EmailPhone = ({onEmailVerificationSuccess}: {onEmailVerificationSuccess: (
)
}

const Password = () => {
const Password = ({randomPW}: {randomPW?: boolean}) => {
const navigateAppend = C.Router2.navigateAppend
const onSetPassword = () => {
navigateAppend({name: settingsPasswordTab, params: {}})
}
const hasPassword = usePWState(s => !s.randomPW)
const hasPassword = !randomPW
let passwordLabel: string
if (hasPassword) {
passwordLabel = Kb.Styles.isMobile ? 'Change' : 'Change password'
Expand Down Expand Up @@ -189,11 +189,7 @@ const AccountSettings = ({route}: Props) => {
const deletePhoneNumber = C.useRPC(T.RPCGen.phoneNumbersDeletePhoneNumberRpcPromise)
const [addedEmail, setAddedEmail] = React.useState(addedEmailFromRoute ?? '')
const [addedPhone, setAddedPhone] = React.useState(addedPhoneFromRoute)
const {loadHasRandomPw} = usePWState(
C.useShallow(s => ({
loadHasRandomPw: s.dispatch.loadHasRandomPw,
}))
)
const {randomPW, reload: reloadRandomPW} = useRandomPWState()
const {navigateAppend, switchTab} = C.Router2
const _onClearSupersededPhoneNumber = (phone: string) => {
deletePhoneNumber(
Expand Down Expand Up @@ -239,7 +235,7 @@ const AccountSettings = ({route}: Props) => {
const onClearAddedPhone = () => setAddedPhone(false)
const onReload = () => {
loadSettings()
loadHasRandomPw()
reloadRandomPW()
}
const onStartPhoneConversation = () => {
switchTab(C.Tabs.chatTab)
Expand Down Expand Up @@ -294,7 +290,7 @@ const AccountSettings = ({route}: Props) => {
<Kb.Box2 direction="vertical" fullWidth={true} fullHeight={true}>
<EmailPhone onEmailVerificationSuccess={setAddedEmail} />
<Kb.Divider />
<Password />
<Password randomPW={randomPW} />
<Kb.Divider />
<WebAuthTokenLogin />
<Kb.Divider />
Expand Down
21 changes: 9 additions & 12 deletions shared/settings/advanced.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ import * as T from '@/constants/types'
import * as React from 'react'
import {ProxySettings} from './proxy'
import {processorProfileInProgressKey, traceInProgressKey} from '@/constants/settings'
import {usePWState} from '@/stores/settings-password'
import {useFSState} from '@/stores/fs'
import {useConfigState} from '@/stores/config'
import {useShellState} from '@/stores/shell'
import {ignorePromise, timeoutPromise} from '@/constants/utils'
import {pprofDir} from '@/constants/platform'
import {clearLocalLogs} from '@/util/misc'
import {useWaitingState} from '@/stores/waiting'
import {useRandomPWState} from './use-random-pw'

let initialUseNativeFrame: boolean | undefined

Expand Down Expand Up @@ -103,16 +103,17 @@ const UseNativeFrame = () => {

const LockdownCheckbox = (p: {
hasRandomPW: boolean
loaded: boolean
lockdownModeEnabled?: boolean
setLockdownMode: (enabled: boolean) => void
settingLockdownMode: boolean
}) => {
const {hasRandomPW, lockdownModeEnabled, setLockdownMode, settingLockdownMode} = p
const {hasRandomPW, loaded, lockdownModeEnabled, setLockdownMode, settingLockdownMode} = p
const onChangeLockdownMode = setLockdownMode
const readMoreUrlProps = Kb.useClickURL('https://keybase.io/docs/lockdown/index')
const label = 'Enable account lockdown mode' + (hasRandomPW ? ' (you need to set a password first)' : '')
const checked = hasRandomPW || !!lockdownModeEnabled
const disabled = hasRandomPW || settingLockdownMode
const disabled = !loaded || hasRandomPW || settingLockdownMode
return (
<Kb.Checkbox
checked={checked}
Expand Down Expand Up @@ -144,12 +145,8 @@ let disableSpellCheckInitialValue: boolean | undefined

const Advanced = () => {
const settingLockdownMode = C.Waiting.useAnyWaiting(C.waitingKeySettingsSetLockdownMode)
const {hasRandomPW, loadHasRandomPw} = usePWState(
C.useShallow(s => ({
hasRandomPW: !!s.randomPW,
loadHasRandomPw: s.dispatch.loadHasRandomPw,
}))
)
const {loaded: randomPWLoaded, randomPW} = useRandomPWState()
const hasRandomPW = !!randomPW
const {onSetOpenAtLogin, openAtLogin} = useShellState(
C.useShallow(s => ({
onSetOpenAtLogin: s.dispatch.setOpenAtLogin,
Expand Down Expand Up @@ -233,9 +230,8 @@ const Advanced = () => {
}

React.useEffect(() => {
loadHasRandomPw()
loadLockdownMode()
}, [loadHasRandomPw, loadLockdownMode])
}, [loadLockdownMode])

return (
<Kb.KeyboardAvoidingView2>
Expand All @@ -245,6 +241,7 @@ const Advanced = () => {
{settingLockdownMode && <Kb.ProgressIndicator />}
<LockdownCheckbox
hasRandomPW={hasRandomPW}
loaded={randomPWLoaded}
lockdownModeEnabled={lockdownModeEnabled}
setLockdownMode={setLockdownMode}
settingLockdownMode={settingLockdownMode}
Expand All @@ -254,7 +251,7 @@ const Advanced = () => {
{setLockdownModeError}
</Kb.Text>
)}
{!hasRandomPW && (
{randomPWLoaded && !hasRandomPW && (
<Kb.Checkbox
checked={!!rememberPassword}
disabled={rememberPassword === undefined}
Expand Down
39 changes: 28 additions & 11 deletions shared/settings/delete-confirm/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import * as React from 'react'
import * as Kb from '@/common-adapters'
import {useSafeNavigation} from '@/util/safe-navigation'
import {useDeleteAccount} from '../use-delete-account'
import {usePWState} from '@/stores/settings-password'
import {useCurrentUserState} from '@/stores/current-user'
import {useRandomPWState} from '../use-random-pw'

type CheckboxesProps = {
checkData: boolean
Expand Down Expand Up @@ -36,7 +36,8 @@ const Checkboxes = (props: CheckboxesProps) => (
)

const DeleteConfirm = () => {
const hasPassword = usePWState(s => !s.randomPW)
const {randomPW, reload} = useRandomPWState()
const needsMobilePassphraseCheck = Kb.Styles.isMobile && randomPW !== true
const deleteAccountForever = useDeleteAccount()
const username = useCurrentUserState(s => s.username)
const [checkData, setCheckData] = React.useState(false)
Expand All @@ -50,7 +51,7 @@ const DeleteConfirm = () => {
// dont do this in a preflight test
return
}
if (Kb.Styles.isMobile && hasPassword) {
if (needsMobilePassphraseCheck) {
navigateAppend({name: 'checkPassphraseBeforeDeleteAccount', params: {}})
} else {
Comment on lines 39 to 56
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

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

On mobile, needsMobilePassphraseCheck treats randomPW === undefined the same as "has a password" and routes to checkPassphraseBeforeDeleteAccount. If the account actually has a random passphrase (no password) but the RPC hasn’t returned yet (or failed), this flow blocks deletion behind a password prompt the user can’t satisfy. Consider gating the passphrase-check route only when randomPW === false (known to have a password), and disabling/deactivating the confirm action while randomPW is still undefined (or forcing a reload and waiting) so you don’t guess wrong.

Copilot uses AI. Check for mistakes.
deleteAccountForever()
Expand All @@ -61,14 +62,26 @@ const DeleteConfirm = () => {
<Kb.ConfirmModal
confirmText="Yes, permanently delete it"
content={
<Checkboxes
checkData={checkData}
checkTeams={checkTeams}
checkUsername={checkUsername}
onCheckData={setCheckData}
onCheckTeams={setCheckTeams}
onCheckUsername={setCheckUsername}
/>
<Kb.Box2 direction="vertical" fullWidth={true}>
{randomPW === undefined ? (
<Kb.Box2 direction="vertical" gap="xtiny" style={styles.randomPWStatus} fullWidth={true}>
<Kb.Text type="BodySmall">
Still checking whether this account has a password. You can continue, or retry the check.
</Kb.Text>
<Kb.Text type="BodySmallPrimaryLink" onClick={reload}>
Retry password check
</Kb.Text>
</Kb.Box2>
) : null}
<Checkboxes
checkData={checkData}
checkTeams={checkTeams}
checkUsername={checkUsername}
onCheckData={setCheckData}
onCheckTeams={setCheckTeams}
onCheckUsername={setCheckUsername}
/>
</Kb.Box2>
}
description="This cannot be undone. By deleting this account, you agree that:"
header={
Expand All @@ -91,6 +104,10 @@ const styles = Kb.Styles.styleSheetCreate(() => ({
padding: Kb.Styles.globalMargins.mediumLarge,
},
}),
randomPWStatus: {
padding: Kb.Styles.globalMargins.small,
paddingBottom: 0,
},
}))

export default DeleteConfirm
15 changes: 2 additions & 13 deletions shared/settings/logout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,15 @@ import * as Kb from '@/common-adapters'
import {UpdatePassword, useSubmitNewPassword} from './password'
import {useRequestLogout} from './use-request-logout'
import {usePasswordCheck} from './use-password-check'
import {usePWState} from '@/stores/settings-password'
import {useRandomPWState} from './use-random-pw'

const LogoutContainer = () => {
const {checkPassword, checkPasswordIsCorrect, reset} = usePasswordCheck()
const {hasRandomPW, loadHasRandomPw} = usePWState(
C.useShallow(s => ({
hasRandomPW: s.randomPW,
loadHasRandomPw: s.dispatch.loadHasRandomPw,
}))
)
const {randomPW: hasRandomPW} = useRandomPWState()
const {error, onSave, waitingForResponse} = useSubmitNewPassword(true)
const [hasPGPKeyOnServer, setHasPGPKeyOnServer] = React.useState<boolean | undefined>(undefined)
const loadPgpSettings = C.useRPC(T.RPCGen.accountHasServerKeysRpcPromise)
const requestLogout = useRequestLogout()

const onBootstrap = loadHasRandomPw
const onCheckPassword = checkPassword

const _onLogout = () => {
Expand All @@ -35,10 +28,6 @@ const LogoutContainer = () => {
const [password, setPassword] = React.useState('')
const [showTyping, setShowTyping] = React.useState(false)

React.useEffect(() => {
onBootstrap()
}, [onBootstrap])

React.useEffect(
() => () => {
reset()
Expand Down
11 changes: 9 additions & 2 deletions shared/settings/password.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ import * as React from 'react'
import * as Kb from '@/common-adapters'
import * as C from '@/constants'
import * as T from '@/constants/types'
import {useNavigation} from '@react-navigation/native'
import {useRequestLogout} from './use-request-logout'
import {usePWState} from '@/stores/settings-password'
import {useRandomPWState} from './use-random-pw'

type Props = {
error: string
Expand Down Expand Up @@ -209,9 +210,15 @@ export const useSubmitNewPassword = (thenLogout: boolean) => {
}

const Container = () => {
const randomPW = usePWState(s => s.randomPW)
const {randomPW} = useRandomPWState()
const navigation = useNavigation()
const saveLabel = randomPW ? 'Create password' : 'Save'
const {error, onSave, waitingForResponse} = useSubmitNewPassword(false)
const title = randomPW === undefined ? 'Password' : randomPW ? 'Set a password' : 'Change password'

React.useEffect(() => {
navigation.setOptions({title})
}, [navigation, title])

const [hasPGPKeyOnServer, setHasPGPKeyOnServer] = React.useState<boolean | undefined>(undefined)
const loadPgpSettings = C.useRPC(T.RPCGen.accountHasServerKeysRpcPromise)
Expand Down
Loading