diff --git a/CLAUDE.md b/CLAUDE.md index b68b2bb4ba95..8cdb98e6d4ab 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -2,7 +2,7 @@ - No `Co-Authored-By` in commits. Ever. - Never interact with the Electron app or iOS simulator (screenshots, driving UI, debug ports) without asking first. The user drives and takes screenshots. - Use `--no-ext-diff` with `git diff` (and `git show`/`git log -p`) so external diff tools don't hijack output. -- "Was working before" = base branch, not previous commit. Base branch is almost always `nojima/HOTPOT-next-670-clean-2` (not `master`). Always run `gh pr view --json baseRefName` to confirm before any `git diff` or `git log` comparison. +- "Was working before" = base branch, not previous commit. Base is normally `master`. Always run `gh pr view --json baseRefName` to confirm before any `git diff` or `git log` comparison. - Never use `npm`. Always `yarn`. - Never silently drop features/behavior — ask first, present options. - In tests/stories, use `testuser` / `testuser-mac` as placeholder usernames — never real usernames like `chrisnojima`. @@ -23,4 +23,4 @@ Repo root is `client/`. TS source lives in `shared/`. Always use absolute paths - Plans created by superpowers skills go into `plans/` at the repo root. ## Validation -After TS changes (from `shared/`): `yarn lint` then `yarn tsc`. When debugging visually, skip until fix is confirmed. Never delete the ESLint cache. +After TS changes (from `shared/`): `yarn lint:all` (= `yarn lint` && `yarn lint:bailouts` && `yarn tsc`). Plain `yarn lint` is eslint only and does NOT catch react-compiler bailouts — no compiler rule is wired into `eslint.config.mjs`, so bailouts only surface via `lint:bailouts`. Repo baseline is 0 bailouts; keep it there. When debugging visually, skip until fix is confirmed. Never delete the ESLint cache. diff --git a/shared/chat/conversation/attachment-actions.tsx b/shared/chat/conversation/attachment-actions.tsx index febd30410e86..83457df5b0e7 100644 --- a/shared/chat/conversation/attachment-actions.tsx +++ b/shared/chat/conversation/attachment-actions.tsx @@ -15,6 +15,7 @@ import { useConversationThreadID, useConversationThreadStore, } from './thread-context' +import {registerExternalResetter} from '@/util/zustand' const {darwinCopyToChatTempUploadFile} = KB2.functions @@ -93,6 +94,12 @@ export const showAttachmentPreview = ( const pdfMessageHandoff = new Map() +// module scope outlives sign-out; both hold attachment messages keyed by conversation +registerExternalResetter('chat-attachment-message-handoff', () => { + attachmentPreviewMessageHandoff.clear() + pdfMessageHandoff.clear() +}) + export const takePDFMessage = ( conversationIDKey: T.Chat.ConversationIDKey, messageID: T.Chat.MessageID diff --git a/shared/chat/conversation/attachment-fullscreen/hooks.tsx b/shared/chat/conversation/attachment-fullscreen/hooks.tsx index 81afe8ed4c86..361b1b0b6057 100644 --- a/shared/chat/conversation/attachment-fullscreen/hooks.tsx +++ b/shared/chat/conversation/attachment-fullscreen/hooks.tsx @@ -12,6 +12,7 @@ import { } from '../attachment-actions' import {showConversationInfoPanel} from '../thread-context' import {useConversationMessage} from '../data-hooks' +import {registerExternalResetter} from '@/util/zustand' const blankMessage = Chat.makeMessageAttachment({}) export const useData = ( @@ -116,6 +117,11 @@ export const useData = ( // if we've seen it its likely cached so lets just always just show it and never fallback const seenPaths = new Set() + +// module scope outlives sign-out; holds the previous user's attachment cache paths +registerExternalResetter('chat-attachment-seen-paths', () => { + seenPaths.clear() +}) // preload full and return ''. If too much time passes show preview. Show full when loaded export const usePreviewFallback = ( path: string, diff --git a/shared/chat/conversation/bot/install.tsx b/shared/chat/conversation/bot/install.tsx index 3c8136f05c56..f3def9270c32 100644 --- a/shared/chat/conversation/bot/install.tsx +++ b/shared/chat/conversation/bot/install.tsx @@ -1,6 +1,5 @@ import * as C from '@/constants' import * as ChatCommon from '@/constants/chat/common' -import * as Meta from '@/constants/chat/meta' import * as Teams from '@/constants/teams' import * as Kb from '@/common-adapters' import * as React from 'react' @@ -9,13 +8,15 @@ import ChannelPicker from './channel-picker' import {useChatTeam} from '../team-hooks' import {openURL} from '@/util/misc' import * as T from '@/constants/types' +import * as TestIDs from '@/tests/e2e/shared/test-ids' import {useAllChannelMetas} from '@/teams/common/channel-hooks' +import {useGeneralConvIDKey} from '@/teams/common/general-conv' import {useFeaturedBot} from '@/util/featured-bots' import {useRPCLoad} from '@/util/use-rpc-load' import {RPCError} from '@/util/errors' import logger from '@/logger' import {useBotSettings} from './settings' -import {metasReceived, participantInfoReceived} from '@/chat/inbox/metadata' +import {participantInfoReceived} from '@/chat/inbox/metadata' import {useConversationMeta} from '../data-hooks' const RestrictedItem = '---RESTRICTED---' @@ -66,22 +67,7 @@ export const useRefreshBotMembershipOnSuccess = ( export const useBotConversationIDKey = (inConvIDKey?: T.Chat.ConversationIDKey, teamID?: T.Teams.TeamID) => { const cleanInConvIDKey = T.Chat.isValidConversationIDKey(inConvIDKey ?? '') ? inConvIDKey : undefined - const {data: generalConvIDKey} = useRPCLoad( - T.RPCChat.localFindGeneralConvFromTeamIDRpcPromise, - [{teamID: teamID ?? T.Teams.noTeamID}], - { - enabled: !cleanInConvIDKey && !!teamID, - key: teamID ?? T.Teams.noTeamID, - map: conv => { - const meta = Meta.inboxUIItemToConversationMeta(conv) - if (!meta) { - return undefined - } - metasReceived([meta]) - return meta.conversationIDKey - }, - } - ) + const generalConvIDKey = useGeneralConvIDKey(teamID, !cleanInConvIDKey) return cleanInConvIDKey ?? generalConvIDKey } @@ -622,7 +608,13 @@ const InstallBotPopup = (props: Props) => { ) return ( <> - + {bodyContent} {enabled && (!readOnly || showReviewButton) ? ( diff --git a/shared/chat/conversation/fwd-msg.tsx b/shared/chat/conversation/fwd-msg.tsx index 112984361c51..52c00fa2d258 100644 --- a/shared/chat/conversation/fwd-msg.tsx +++ b/shared/chat/conversation/fwd-msg.tsx @@ -7,6 +7,7 @@ import {useNavigation} from '@react-navigation/native' import {Avatars, TeamAvatar} from '@/chat/avatars' import logger from '@/logger' import {useConversationMessage} from './data-hooks' +import {registerExternalResetter} from '@/util/zustand' type Props = {conversationIDKey?: T.Chat.ConversationIDKey; messageID: T.Chat.MessageID} @@ -16,6 +17,11 @@ const forwardMessageHandoff = new Map() const forwardMessageKey = (conversationIDKey: T.Chat.ConversationIDKey, messageID: T.Chat.MessageID) => `${conversationIDKey}:${T.Chat.messageIDToNumber(messageID)}` +// module scope outlives sign-out; holds message bodies keyed by conversation +registerExternalResetter('chat-forward-message-handoff', () => { + forwardMessageHandoff.clear() +}) + const getForwardMessage = (conversationIDKey: T.Chat.ConversationIDKey, messageID: T.Chat.MessageID) => { const key = forwardMessageKey(conversationIDKey, messageID) return forwardMessageHandoff.get(key) diff --git a/shared/chat/conversation/input-area/suggestors/emoji.tsx b/shared/chat/conversation/input-area/suggestors/emoji.tsx index 20a2a4952b51..864af09001ce 100644 --- a/shared/chat/conversation/input-area/suggestors/emoji.tsx +++ b/shared/chat/conversation/input-area/suggestors/emoji.tsx @@ -53,9 +53,15 @@ const cachedEmojiData = (emoji: T.RPCChat.Emoji) => { } const useDataSource = (conversationIDKey: T.Chat.ConversationIDKey, filter: string) => { - const {emojis: userEmojis, loading: userEmojisLoading} = useUserEmoji({conversationIDKey}) + // a filter the prepass rejects discards the result below, so don't pay for the + // fetch — userEmojis is expensive to resolve service-side + const matchesPrepass = emojiPrepass.test(filter) + const {emojis: userEmojis, loading: userEmojisLoading} = useUserEmoji({ + conversationIDKey, + disabled: !matchesPrepass, + }) - if (!emojiPrepass.test(filter)) { + if (!matchesPrepass) { return { items: empty, loading: false, diff --git a/shared/chat/conversation/input-area/suggestors/users.tsx b/shared/chat/conversation/input-area/suggestors/users.tsx index 5dc07ec5a86b..3b9cec4cb52d 100644 --- a/shared/chat/conversation/input-area/suggestors/users.tsx +++ b/shared/chat/conversation/input-area/suggestors/users.tsx @@ -6,6 +6,7 @@ import {useUsersState} from '@/stores/users' import {useChatTeamMembers} from '../../team-hooks' import {useInboxLayoutState} from '@/chat/inbox/layout-state' import {useConversationMetadata} from '../../data-hooks' +import {registerExternalResetter} from '@/util/zustand' export const transformer = ( input: { @@ -259,6 +260,11 @@ const keyExtractor = (item: ListItem) => { // the memoized suggestion rows can bail. Bounded by users/channels ever // suggested; the size guard is a backstop for pathological accounts. const listItemCache = new Map() + +// module scope outlives sign-out; keyed by username / team#channel +registerExternalResetter('chat-user-suggestor-item-cache', () => { + listItemCache.clear() +}) const canonicalizeItems = (items: Array) => { if (listItemCache.size > 8192) { listItemCache.clear() diff --git a/shared/chat/conversation/messages/attachment/shared.tsx b/shared/chat/conversation/messages/attachment/shared.tsx index 59b62e2c7a51..4a7f692365c8 100644 --- a/shared/chat/conversation/messages/attachment/shared.tsx +++ b/shared/chat/conversation/messages/attachment/shared.tsx @@ -53,12 +53,14 @@ export const ShowToastAfterSaving = ({transferState, toastTargetRef}: Props) => const [allowToast, setAllowToast] = React.useState(true) // since this uses portals we need to hide if we're hidden else we can get stuck showing if our render is frozen - C.Router2.useSafeFocusEffect(() => { - setAllowToast(true) - return () => { - setAllowToast(false) - } - }) + C.Router2.useSafeFocusEffect( + React.useCallback(() => { + setAllowToast(true) + return () => { + setAllowToast(false) + } + }, []) + ) return allowToast && showingToast ? ( diff --git a/shared/chat/conversation/messages/wrapper/send-indicator.tsx b/shared/chat/conversation/messages/wrapper/send-indicator.tsx index ddd714701c94..3a94bd44fc18 100644 --- a/shared/chat/conversation/messages/wrapper/send-indicator.tsx +++ b/shared/chat/conversation/messages/wrapper/send-indicator.tsx @@ -2,6 +2,7 @@ import * as React from 'react' import * as Kb from '@/common-adapters' import {produce} from 'immer' import {useColorScheme} from 'react-native' +import {registerExternalResetter} from '@/util/zustand' type AnimationStatus = | 'encrypting' @@ -46,6 +47,11 @@ const statusToIconDarkExploding = { const shownEncryptingSet = new Set() +// module scope outlives sign-out; holds the previous user's outbox ids +registerExternalResetter('chat-send-indicator-shown', () => { + shownEncryptingSet.clear() +}) + type OwnProps = { failed: boolean id: number diff --git a/shared/chat/conversation/team-hooks.tsx b/shared/chat/conversation/team-hooks.tsx index 872359104947..55545ded9a6d 100644 --- a/shared/chat/conversation/team-hooks.tsx +++ b/shared/chat/conversation/team-hooks.tsx @@ -14,6 +14,7 @@ import { } from '@/teams/use-cached-resource' import {updateChosenChannelsTeamnames, useChosenChannelsTeamnames} from './manage-channels-badge' import {useThreadMeta} from './thread-context' +import {registerExternalResetter} from '@/util/zustand' type ChatTeamState = { allowPromote: boolean @@ -67,6 +68,13 @@ const chatTeamCacheMap: TeamCacheMap = new Map() const chatTeamMembersCacheMap: TeamCacheMap = new Map() const chatTeamReloadStaleMs = 5 * 60_000 +// module scope outlives sign-out, so the next user would be served the previous +// user's team data and member lists +registerExternalResetter('chat-team-hooks-caches', () => { + chatTeamCacheMap.clear() + chatTeamMembersCacheMap.clear() +}) + // A disabled "shadow" instance (one that returns the context value instead of // its own) must NOT share the loader's cache: with enabled=false // useCachedResource resets the cache, which would clobber the loader's data. diff --git a/shared/chat/create-channel/hooks.tsx b/shared/chat/create-channel/hooks.tsx index 679b8bd37602..cbcd86fdf318 100644 --- a/shared/chat/create-channel/hooks.tsx +++ b/shared/chat/create-channel/hooks.tsx @@ -4,6 +4,7 @@ import * as T from '@/constants/types' import {RPCError} from '@/util/errors' import upperFirst from 'lodash/upperFirst' import type {Props} from './index.shared' +import {invalidateTeamChannels} from '@/teams/common/team-channels-invalidation' import {useChatTeam} from '../conversation/team-hooks' export default (p: Props) => { @@ -57,6 +58,7 @@ export default (p: Props) => { C.waitingKeyTeamsCreateChannel(teamID) ) } + invalidateTeamChannels(teamID) onBack() if (navToChatOnSuccess) { previewConversation({channelname, conversationIDKey, reason: 'newChannel', teamname}) diff --git a/shared/chat/inbox-search/index.tsx b/shared/chat/inbox-search/index.tsx index 02ccfccebd9b..c8d7e047b0f6 100644 --- a/shared/chat/inbox-search/index.tsx +++ b/shared/chat/inbox-search/index.tsx @@ -16,6 +16,7 @@ import { type InboxSearchVisibleResultCounts, } from '../inbox/use-inbox-search' import {showTeamByName} from '@/teams/team-page-actions' +import {registerExternalResetter} from '@/util/zustand' type OwnProps = { header?: React.ReactElement | null @@ -57,6 +58,12 @@ const textResultCache = new Map() const openTeamItemCache = new WeakMap() const botItemCache = new WeakMap() +// module scope outlives sign-out; these hold conversation and username hits +registerExternalResetter('chat-inbox-search-item-caches', () => { + nameResultCache.clear() + textResultCache.clear() +}) + const canonNameResult = (next: NameResult) => { if (nameResultCache.size > 4096) { nameResultCache.clear() @@ -258,7 +265,7 @@ export default function InboxSearchContainer(ownProps: OwnProps) { renderHeaderWithMore(section, _botsResults.length, botsCollapsed, botsAll, toggleBotsAll) const renderTextHeader = (section: Section) => { - const ratio = indexPercent / 100.0 + const ratio = (indexPercent ?? 0) / 100.0 return ( - ) : indexPercent > 0 && indexPercent < 100 ? ( + ) : indexPercent !== undefined && indexPercent < 100 ? ( Indexing... {isMobile ? ( diff --git a/shared/chat/inbox/index.tsx b/shared/chat/inbox/index.tsx index 8f0d557b957a..234e61c0338b 100644 --- a/shared/chat/inbox/index.tsx +++ b/shared/chat/inbox/index.tsx @@ -510,9 +510,11 @@ function NativeInboxBody(p: ControlledInboxProps) { const setOpenRow = useOpenedRowState(s => s.dispatch.setOpenRow) - C.Router2.useSafeFocusEffect(() => { - setOpenRow(Chat.noConversationIDKey) - }) + C.Router2.useSafeFocusEffect( + React.useCallback(() => { + setOpenRow(Chat.noConversationIDKey) + }, [setOpenRow]) + ) const promptSmallTeamsNum = React.useCallback(() => { if (isIOS) { diff --git a/shared/chat/inbox/use-inbox-search.tsx b/shared/chat/inbox/use-inbox-search.tsx index babdfa90579e..682bac546f89 100644 --- a/shared/chat/inbox/use-inbox-search.tsx +++ b/shared/chat/inbox/use-inbox-search.tsx @@ -25,7 +25,7 @@ export const makeInboxSearchInfo = (): T.Chat.InboxSearchInfo => ({ botsResults: [], botsResultsSuggested: false, botsStatus: 'initial', - indexPercent: 0, + indexPercent: undefined, nameResults: [], nameResultsUnread: false, nameStatus: 'initial', @@ -311,6 +311,9 @@ export function useInboxSearch(): InboxSearchController { updateIfActive(prev => ({ ...prev, botsStatus: 'inprogress', + // unknown again until this search reports one, rather than carrying + // the previous search's number + indexPercent: undefined, nameStatus: 'inprogress', openTeamsStatus: 'inprogress', selectedIndex: 0, diff --git a/shared/chat/inbox/use-inbox-state.tsx b/shared/chat/inbox/use-inbox-state.tsx index d5a571b60dba..83d55f37e629 100644 --- a/shared/chat/inbox/use-inbox-state.tsx +++ b/shared/chat/inbox/use-inbox-state.tsx @@ -139,11 +139,13 @@ export function useInboxState( C.Router2.setChatRootParams({refreshInbox: undefined}) }, [inboxRefresh, isFocused, loggedIn, refreshInbox, username]) - C.Router2.useSafeFocusEffect(() => { - if (!inboxHasLoaded) { - C.ignorePromise(inboxRefresh('componentNeverLoaded')) - } - }) + C.Router2.useSafeFocusEffect( + React.useCallback(() => { + if (!inboxHasLoaded) { + C.ignorePromise(inboxRefresh('componentNeverLoaded')) + } + }, [inboxHasLoaded, inboxRefresh]) + ) React.useEffect(() => { const ready = loggedIn && !!username diff --git a/shared/chat/user-emoji.tsx b/shared/chat/user-emoji.tsx index d3410c7f47e3..a2a4f3e3c0d4 100644 --- a/shared/chat/user-emoji.tsx +++ b/shared/chat/user-emoji.tsx @@ -1,9 +1,37 @@ +import * as React from 'react' import * as T from '@/constants/types' -import {useRPCLoad} from '@/util/use-rpc-load' +import {useEmojiState} from '@/teams/emojis/use-emoji' +import { + type CachedResourceCache, + createCachedResourceCache, + getCachedResourceCache, + useCachedResource, +} from '@/teams/use-cached-resource' +import {registerExternalResetter} from '@/util/zustand' const emptyEmojiGroups: ReadonlyArray = [] const emptyEmojis: ReadonlyArray = [] +type UserEmojiData = { + emojiGroups: ReadonlyArray + emojis: ReadonlyArray +} + +const emptyUserEmojiData: UserEmojiData = {emojiGroups: emptyEmojiGroups, emojis: emptyEmojis} + +// One cache per request key, shared by every consumer. userEmojis is expensive on +// the service side - it resolves two attachment URLs per custom emoji - so the +// suggestor remounting on each ':' trigger used to refetch the entire set. The +// shared cache also collapses concurrent mounts onto a single in-flight request. +const userEmojiCaches = new Map>() +const userEmojiStaleMs = 60_000 + +// module scope outlives sign-out, so the next user would be served the previous +// user's custom emoji until the entries went stale +registerExternalResetter('chat-user-emoji-caches', () => { + userEmojiCaches.clear() +}) + const flattenUserEmojis = (groups: ReadonlyArray) => { const emojis = new Array() groups.forEach(group => { @@ -25,33 +53,59 @@ export const useUserEmoji = ({ const requestKey = `${conversationIDKey ?? T.Chat.noConversationIDKey}:${ requestOnlyInTeam ? 'team' : 'all' }` - const {data, loading} = useRPCLoad( - T.RPCChat.localUserEmojisRpcPromise, - [ - { - convID: - conversationIDKey && conversationIDKey !== T.Chat.noConversationIDKey - ? T.Chat.keyToConversationID(conversationIDKey) - : null, - opts: { - getAliases: true, - getCreationInfo: false, - onlyInTeam: requestOnlyInTeam, - }, - }, - ], - { - enabled: !disabled, - key: requestKey, - map: results => { - const nextGroups = results.emojis.emojis ?? emptyEmojiGroups - return {emojiGroups: nextGroups, emojis: flattenUserEmojis(nextGroups)} + // adding, aliasing or removing an emoji bumps this; it busts the cache so the + // picker and the suggestor pick the change up instead of serving a stale set + const emojiUpdatedTrigger = useEmojiState(s => s.emojiUpdatedTrigger) + // a disabled instance resets the cache it holds, so keep those off the shared one + const [localCache] = React.useState>(() => + createCachedResourceCache(emptyUserEmojiData, requestKey) + ) + const sharedCache = React.useMemo( + () => getCachedResourceCache(userEmojiCaches, emptyUserEmojiData, requestKey), + [requestKey] + ) + const load = React.useCallback(async () => { + const results = await T.RPCChat.localUserEmojisRpcPromise({ + convID: + conversationIDKey && conversationIDKey !== T.Chat.noConversationIDKey + ? T.Chat.keyToConversationID(conversationIDKey) + : null, + opts: { + getAliases: true, + getCreationInfo: false, + onlyInTeam: requestOnlyInTeam, }, + }) + const emojiGroups = results.emojis.emojis ?? emptyEmojiGroups + return {emojiGroups, emojis: flattenUserEmojis(emojiGroups)} + }, [conversationIDKey, requestOnlyInTeam]) + + const {data, loading, reload} = useCachedResource({ + cache: disabled ? localCache : sharedCache, + cacheKey: requestKey, + enabled: !disabled, + initialData: emptyUserEmojiData, + load, + refreshKey: emojiUpdatedTrigger, + staleMs: userEmojiStaleMs, + }) + + // refreshKey alone only re-checks staleness, and an edit we just made is never + // stale — force past the cache when the emoji set actually changed + const lastEmojiUpdatedTriggerRef = React.useRef(emojiUpdatedTrigger) + React.useEffect(() => { + if (lastEmojiUpdatedTriggerRef.current === emojiUpdatedTrigger) { + return } - ) + lastEmojiUpdatedTriggerRef.current = emojiUpdatedTrigger + if (!disabled) { + void reload() + } + }, [disabled, emojiUpdatedTrigger, reload]) + return { - emojiGroups: disabled ? undefined : (data?.emojiGroups ?? emptyEmojiGroups), - emojis: data?.emojis ?? emptyEmojis, + emojiGroups: disabled ? undefined : data.emojiGroups, + emojis: data.emojis, loading, } } diff --git a/shared/common-adapters/hot-key.tsx b/shared/common-adapters/hot-key.tsx index 0eb12d37105a..67d3c95e1ecc 100644 --- a/shared/common-adapters/hot-key.tsx +++ b/shared/common-adapters/hot-key.tsx @@ -202,20 +202,31 @@ const unregisterKeys = (keysArr: Array, cb: (key: string) => void) => { } export function useHotKey(keys: Array | string, cb: (key: string) => void) { - const keysArr = (() => { - const arr = typeof keys === 'string' ? [keys] : keys - return arr.filter(k => k.length > 0) - })() + // callers pass inline arrays/closures, so derive stable identities for both: + // otherwise every render unregisters and re-registers, which also re-orders the + // LIFO stack and lets a re-rendering background screen steal the key + const keysKey = typeof keys === 'string' ? keys : keys.join(',') + const keysArr = React.useMemo(() => keysKey.split(',').filter(k => k.length > 0), [keysKey]) - C.Router2.useSafeFocusEffect(() => { - if (isMobile || keysArr.length === 0) return - registerKeys(keysArr, cb) - return () => unregisterKeys(keysArr, cb) + const cbRef = React.useRef(cb) + React.useEffect(() => { + cbRef.current = cb }) + const [stableCB] = React.useState(() => (key: string) => cbRef.current(key)) + + // registering again on focus re-pushes us to the top of the stack so the + // focused screen wins the key over screens that merely stayed mounted + C.Router2.useSafeFocusEffect( + React.useCallback(() => { + if (isMobile || keysArr.length === 0) return + registerKeys(keysArr, stableCB) + return () => unregisterKeys(keysArr, stableCB) + }, [keysArr, stableCB]) + ) React.useEffect(() => { if (isMobile || keysArr.length === 0) return - registerKeys(keysArr, cb) - return () => unregisterKeys(keysArr, cb) - }, [keysArr, cb]) + registerKeys(keysArr, stableCB) + return () => unregisterKeys(keysArr, stableCB) + }, [keysArr, stableCB]) } diff --git a/shared/common-adapters/markdown/spoiler.tsx b/shared/common-adapters/markdown/spoiler.tsx index 3446391ef879..c643bdb6db2d 100644 --- a/shared/common-adapters/markdown/spoiler.tsx +++ b/shared/common-adapters/markdown/spoiler.tsx @@ -1,6 +1,7 @@ import * as React from 'react' import * as Styles from '@/styles' import Text from '@/common-adapters/text' +import {registerExternalResetter} from '@/util/zustand' type Props = { children: React.ReactNode @@ -10,6 +11,12 @@ type Props = { const spoilerState = new Map() +// module scope outlives sign-out; keyed by message content, and a spoiler the +// previous user revealed must not come up already revealed for the next one +registerExternalResetter('markdown-spoiler-state', () => { + spoilerState.clear() +}) + const Spoiler = (p: Props) => { const {children, content, context} = p const key = `${context ?? ''}:${content}` diff --git a/shared/common-adapters/profile-card.tsx b/shared/common-adapters/profile-card.tsx index d3c78a9f1c0d..709864bab9df 100644 --- a/shared/common-adapters/profile-card.tsx +++ b/shared/common-adapters/profile-card.tsx @@ -140,7 +140,10 @@ const ProfileCard = ({ onLayoutChange, username, }: Props) => { - const {details: userDetails, loadProfile} = useTrackerProfile(username) + // a hover card is incidental, not "check this identity now" - the cached proof + // results are what it should show, and forcing a check re-fetches every proof + // from its third-party host + const {details: userDetails, loadProfile} = useTrackerProfile(username, {cachedOnMount: true}) const followThem = useFollowerState(s => s.following.has(username)) const followsYou = useFollowerState(s => s.followers.has(username)) const isSelf = useCurrentUserState(s => s.username === username) diff --git a/shared/common-adapters/with-tooltip.tsx b/shared/common-adapters/with-tooltip.tsx index d4722a67af9b..704ec8010d47 100644 --- a/shared/common-adapters/with-tooltip.tsx +++ b/shared/common-adapters/with-tooltip.tsx @@ -62,11 +62,16 @@ function WithTooltip(p: Props) { }, 3000) const {width: screenWidth, height: screenHeight} = useSafeAreaFrame() - C.Router2.useSafeFocusEffect(() => { - return () => { - setNativeVisible(false) - } - }) + // must be stable: an unstable callback re-runs the focus effect (and so its + // cleanup) on every render, which would hide the tooltip as soon as showing it + // re-renders us + C.Router2.useSafeFocusEffect( + React.useCallback(() => { + return () => { + setNativeVisible(false) + } + }, []) + ) if (!isMobile) { const onMouseEnter = () => setDesktopVisible(true) diff --git a/shared/constants/types/chat/index.tsx b/shared/constants/types/chat/index.tsx index 2da28cb4cb6a..60e5d47cc913 100644 --- a/shared/constants/types/chat/index.tsx +++ b/shared/constants/types/chat/index.tsx @@ -50,7 +50,9 @@ export type InboxSearchOpenTeamHit = { } export type InboxSearchInfo = { - indexPercent: number + // undefined until the service reports one; 0 is a real value meaning nothing + // is indexed yet, which is what an index rebuild looks like + indexPercent?: number botsResults: ReadonlyArray botsResultsSuggested: boolean botsStatus: InboxSearchStatus diff --git a/shared/crypto/recipients.tsx b/shared/crypto/recipients.tsx index 1e4aefa93633..7255a968dcf4 100644 --- a/shared/crypto/recipients.tsx +++ b/shared/crypto/recipients.tsx @@ -1,4 +1,5 @@ import * as Kb from '@/common-adapters' +import * as TestIDs from '@/tests/e2e/shared/test-ids' type Props = { inProgress: boolean @@ -22,6 +23,7 @@ const Recipients = ({inProgress, onAddRecipients, onClearRecipients, recipients} {/* Display-only input; block it from taking focus so opening the diff --git a/shared/engine/action-listener.test.ts b/shared/engine/action-listener.test.ts index 3bb0eeb3df42..1242ef301b1f 100644 --- a/shared/engine/action-listener.test.ts +++ b/shared/engine/action-listener.test.ts @@ -64,3 +64,23 @@ test('clearAll removes all registered listeners', () => { expect(homeListener).not.toHaveBeenCalled() }) + +test('a stale unsubscribe from before a reset leaves later subscribers alone', () => { + const before = jest.fn() + const staleUnsubscribe = subscribeToEngineAction('keybase.1.homeUI.homeUIRefresh', before) + + clearAllEngineActionListeners() + + const after = jest.fn() + subscribeToEngineAction('keybase.1.homeUI.homeUIRefresh', after) + + // the component that subscribed before the reset unmounts late + staleUnsubscribe() + + notifyEngineActionListeners({ + payload: {params: {}}, + type: 'keybase.1.homeUI.homeUIRefresh', + } as never) + expect(after).toHaveBeenCalledTimes(1) + expect(before).not.toHaveBeenCalled() +}) diff --git a/shared/engine/action-listener.tsx b/shared/engine/action-listener.tsx index 59fe268bf08e..7c552f915e7c 100644 --- a/shared/engine/action-listener.tsx +++ b/shared/engine/action-listener.tsx @@ -31,7 +31,11 @@ export const subscribeToEngineAction = ( listeners.add(untypedListener) return () => { listeners.delete(untypedListener) - if (!listeners.size) { + // Only drop the entry if the map still holds THIS set. A reset replaces the + // set for a type, so an unsubscribe left over from before the reset would + // otherwise see its own detached, now-empty set and delete the live one, + // silently unsubscribing everybody who registered after the reset. + if (!listeners.size && listenersByType.get(type) === listeners) { listenersByType.delete(type) } } diff --git a/shared/fs/browser/root.tsx b/shared/fs/browser/root.tsx index 4ac9ff4c31ab..3832fdd3786b 100644 --- a/shared/fs/browser/root.tsx +++ b/shared/fs/browser/root.tsx @@ -7,6 +7,7 @@ import SfmiBanner from '../banner/system-file-manager-integration-banner/contain import {WrapRow} from './rows/rows' import * as FS from '@/constants/fs' import {useCurrentUserState} from '@/stores/current-user' +import {registerExternalResetter} from '@/util/zustand' type Props = { destinationPickerSource?: T.FS.MoveOrCopySource | T.FS.IncomingShareSource @@ -57,6 +58,11 @@ const renderSectionHeader = ({section}: {section: {key: string; title: string}}) // recents are re-derived on every FsDataContext write; reuse item identities // so the memoized rows can bail const recentItemCache = new Map() + +// module scope outlives sign-out; keyed by TLF name +registerExternalResetter('fs-browser-recent-item-cache', () => { + recentItemCache.clear() +}) const canonRecentItem = (name: string, tlfType: T.FS.TlfType): SectionListItem => { const key = `${tlfType}-${name}` let item = recentItemCache.get(key) diff --git a/shared/git/index.tsx b/shared/git/index.tsx index a5441c6f5c4e..3e9f62a70ceb 100644 --- a/shared/git/index.tsx +++ b/shared/git/index.tsx @@ -101,13 +101,14 @@ const GitRoot = (ownProps: OwnProps) => { // errors from row actions (toggling chat), load errors come from the hook const [rowError, setRowError] = React.useState() const isNew = useConfigState(s => s.badgeState?.newGitRepoGlobalUniqueIDs) - const {badged} = useLocalBadging(new Set(isNew ?? []), () => { + const clearBadges = React.useCallback(() => { clearGitBadges( [{category: 'new_git_repo'}], () => {}, () => {} ) - }) + }, [clearGitBadges]) + const {badged} = useLocalBadging(new Set(isNew ?? []), clearBadges) const { data, error: loadError, diff --git a/shared/login/join-or-login.tsx b/shared/login/join-or-login.tsx index 1b77007db2ce..9aec72bd3767 100644 --- a/shared/login/join-or-login.tsx +++ b/shared/login/join-or-login.tsx @@ -26,10 +26,12 @@ const Intro = () => { const [showing, setShowing] = React.useState(true) Kb.useInterval(loadIsOnline, showing ? 5000 : undefined) - C.Router2.useSafeFocusEffect(() => { - setShowing(true) - return () => setShowing(false) - }) + C.Router2.useSafeFocusEffect( + React.useCallback(() => { + setShowing(true) + return () => setShowing(false) + }, []) + ) return ( { const onClickUser = (username: string) => navToProfile(username) - const onReload = (isRetry?: boolean) => getData(false, isRetry === true || !followSuggestions.length) + const reload = React.useEffectEvent((isRetry?: boolean) => { + getData(false, isRetry === true || !followSuggestions.length) + }) + // frozen wrapper: effect-event identities change every render, and an unstable + // fn makes useSafeFocusEffect re-run (so re-reload) on every render while focused + const [onReload] = React.useState(() => (isRetry?: boolean) => { + reload(isRetry) + }) C.Router2.useSafeFocusEffect(onReload) useEngineActionListener('keybase.1.homeUI.homeUIRefresh', () => { diff --git a/shared/profile/generic/proofs-list.tsx b/shared/profile/generic/proofs-list.tsx index e74f21e8cdc7..45f593bce6fe 100644 --- a/shared/profile/generic/proofs-list.tsx +++ b/shared/profile/generic/proofs-list.tsx @@ -349,7 +349,7 @@ const runProofFlow = async (p: { const ProofsList = ({platform, reason = 'profile'}: Props) => { const currentUsername = useCurrentUserState(s => s.username) const {proofSuggestions} = useProofSuggestions() - const {loadProfile} = useTrackerProfile(currentUsername) + const {loadProfile} = useTrackerProfile(currentUsername, {loadOnMount: false}) const registerCryptoAddress = C.useRPC(T.RPCGen.cryptocurrencyRegisterAddressRpcPromise) const isDarkMode = useColorScheme() === 'dark' const {clearModals, navigateAppend, navigateUp} = C.Router2 diff --git a/shared/profile/user/hooks.tsx b/shared/profile/user/hooks.tsx index 602dd6eb1d54..61f4dbe6a891 100644 --- a/shared/profile/user/hooks.tsx +++ b/shared/profile/user/hooks.tsx @@ -62,9 +62,7 @@ const useUserData = (username: string) => { loadNonUserProfile, loadProfile, nonUserDetails, - } = useTrackerProfile(username, { - reloadOnFocus: true, - }) + } = useTrackerProfile(username) const requestIDRef = React.useRef(0) const [sharedTeamsState, setSharedTeamsState] = React.useState<{ teams?: ReadonlyArray diff --git a/shared/profile/user/index.tsx b/shared/profile/user/index.tsx index 726286c8e24b..69febb4545d2 100644 --- a/shared/profile/user/index.tsx +++ b/shared/profile/user/index.tsx @@ -15,6 +15,7 @@ import {SiteIcon} from '../generic/site-icon' import useUserData from './hooks' import {LoadedTeamsListProvider} from '@/teams/use-teams-list' import * as TestIDs from '@/tests/e2e/shared/test-ids' +import {registerExternalResetter} from '@/util/zustand' export type BackgroundColorType = 'red' | 'green' | 'blue' @@ -566,6 +567,11 @@ const User = (props: {username: string}) => { const usernameSelectedTab = new Map() +// module scope outlives sign-out; keyed by username +registerExternalResetter('profile-user-selected-tab', () => { + usernameSelectedTab.clear() +}) + const avatarSize = 128 const styles = Kb.Styles.styleSheetCreate(() => ({ diff --git a/shared/profile/user/teams/index.tsx b/shared/profile/user/teams/index.tsx index 544ee5de25ea..6dfa1f08ab24 100644 --- a/shared/profile/user/teams/index.tsx +++ b/shared/profile/user/teams/index.tsx @@ -63,6 +63,9 @@ type TeamShowcaseProps = T.Tracker.TeamShowcase & { const TeamShowcase = (props: TeamShowcaseProps) => { const {name, isOpen} = props const {onClick, popup, popupAnchor} = useTeamInfoPopup({ + // a profile can feature every team its owner is in; annotating them all up + // front is one expensive getAnnotatedTeam per row for a popup nobody opened + loadOnDemand: true, popupInfo: props, teamname: name, }) diff --git a/shared/profile/user/teams/use-team-info-popup.tsx b/shared/profile/user/teams/use-team-info-popup.tsx index f71cdbd12e27..dcd9f159299d 100644 --- a/shared/profile/user/teams/use-team-info-popup.tsx +++ b/shared/profile/user/teams/use-team-info-popup.tsx @@ -27,11 +27,12 @@ const useTeamInfoPopup = ({loadOnDemand = false, popupInfo, teamID: initialTeamI const teamID = initialTeamID ?? teamNameToID.get(teamname) ?? T.Teams.noTeamID const teamLoadEnabled = !loadOnDemand || hasRequestedLoad const {loaded, loading: loadingTeam, teamDetails, teamMeta} = useLoadedTeam(teamID, teamLoadEnabled) - const hasLoadedTeam = teamMeta.teamname.length > 0 + // `loaded`, not a non-empty teamname: useLoadedTeam seeds teamMeta from the cheap + // teams-list, so a team you're in looks named long before its details exist. const inTeam = teamMeta.role !== 'none' - const description = hasLoadedTeam ? teamDetails.description : (popupInfo?.description ?? '') - const isOpen = hasLoadedTeam ? teamDetails.settings.open : (popupInfo?.isOpen ?? false) - const membersCount = hasLoadedTeam ? teamDetails.members.size : (popupInfo?.membersCount ?? 0) + const description = loaded ? teamDetails.description : (popupInfo?.description ?? '') + const isOpen = loaded ? teamDetails.settings.open : (popupInfo?.isOpen ?? false) + const membersCount = loaded ? teamDetails.members.size : (popupInfo?.membersCount ?? 0) const onJoinTeam = React.useCallback( (teamname: string) => { @@ -89,17 +90,23 @@ const useTeamInfoPopup = ({loadOnDemand = false, popupInfo, teamID: initialTeamI }, [loaded, loadingTeam, pendingOpen, showPopup]) const onClick = React.useCallback(() => { - if (!loadOnDemand || teamID === T.Teams.noTeamID || hasLoadedTeam) { + if (!loadOnDemand || teamID === T.Teams.noTeamID || loaded) { + showPopup() + return + } + setHasRequestedLoad(true) + // popupInfo already carries everything the popup shows (it comes from the + // profile's showcase payload), so open now and let the load refine it + if (popupInfo) { showPopup() return } if (loadingTeam || pendingOpen) { return } - setHasRequestedLoad(true) hasSeenPendingLoadRef.current = false setPendingOpen(true) - }, [hasLoadedTeam, loadOnDemand, loadingTeam, pendingOpen, showPopup, teamID]) + }, [loaded, loadOnDemand, loadingTeam, pendingOpen, popupInfo, showPopup, teamID]) return {loadingTeam, onClick, pendingOpen, popup, popupAnchor} } diff --git a/shared/router-v2/screen-layout-modal.desktop.tsx b/shared/router-v2/screen-layout-modal.desktop.tsx index aebb85873915..9af197ebd97b 100644 --- a/shared/router-v2/screen-layout-modal.desktop.tsx +++ b/shared/router-v2/screen-layout-modal.desktop.tsx @@ -86,12 +86,14 @@ export const ModalWrapper = (p: ModalWrapperProps) => { const [topMostModal, setTopMostModal] = React.useState(true) - C.Router2.useSafeFocusEffect(() => { - setTopMostModal(true) - return () => { - setTopMostModal(false) - } - }) + C.Router2.useSafeFocusEffect( + React.useCallback(() => { + setTopMostModal(true) + return () => { + setTopMostModal(false) + } + }, []) + ) React.useEffect(() => { if (!topMostModal || overlayNoClose) return diff --git a/shared/teams/channel/create-channels.tsx b/shared/teams/channel/create-channels.tsx index 2c01b532af9e..c4ef5d5cc912 100644 --- a/shared/teams/channel/create-channels.tsx +++ b/shared/teams/channel/create-channels.tsx @@ -4,6 +4,7 @@ import * as Kb from '@/common-adapters' import * as T from '@/constants/types' import {RPCError} from '@/util/errors' import {CreateChannelsModal} from '../new-team/wizard/create-channels' +import {invalidateTeamChannels} from '../common/team-channels-invalidation' import {useLoadedTeam} from '../team/use-loaded-team' type Props = {teamID: T.Teams.TeamID} @@ -34,6 +35,7 @@ const submitChannels = async ( C.waitingKeyTeamsCreateChannel(teamID) ) } + invalidateTeamChannels(teamID) setSuccess(true) C.Router2.clearModals() } catch (error_) { diff --git a/shared/teams/channel/index.tsx b/shared/teams/channel/index.tsx index e8d8ff55e164..bfc4fff9606b 100644 --- a/shared/teams/channel/index.tsx +++ b/shared/teams/channel/index.tsx @@ -23,6 +23,7 @@ import {LoadedTeamChannelsProvider} from '../common/use-loaded-team-channels' import {useUsersState} from '@/stores/users' import {LoadedTeamProvider, useLoadedTeam} from '../team/use-loaded-team' import {unboxRows, useInboxMetadataState} from '@/chat/inbox/metadata' +import {registerExternalResetter} from '@/util/zustand' export type OwnProps = { teamID: T.Teams.TeamID @@ -75,18 +76,24 @@ const useLoadDataForChannelPage = ( } // keep track during session -const lastSelectedTabs: {[T: string]: TabKey} = {} +const lastSelectedTabs = new Map() const defaultTab: TabKey = 'members' +// module scope outlives sign-out; keyed by conversation, so the next user would +// inherit the previous user's tab choices +registerExternalResetter('teams-channel-index', () => { + lastSelectedTabs.clear() +}) + const useTabsState = ( conversationIDKey: T.Chat.ConversationIDKey, providedTab?: TabKey ): [TabKey, (t: TabKey) => void] => { - const defaultSelectedTab = lastSelectedTabs[conversationIDKey] ?? providedTab ?? defaultTab + const defaultSelectedTab = lastSelectedTabs.get(conversationIDKey) ?? providedTab ?? defaultTab const [selectedTab, setSelectedTab] = React.useState(defaultSelectedTab) React.useEffect(() => { - lastSelectedTabs[conversationIDKey] = selectedTab + lastSelectedTabs.set(conversationIDKey, selectedTab) }, [conversationIDKey, selectedTab]) const prevConvIDRef = React.useRef(conversationIDKey) diff --git a/shared/teams/common/channel-hooks.tsx b/shared/teams/common/channel-hooks.tsx index a28bdd7f1c02..71a6456aab57 100644 --- a/shared/teams/common/channel-hooks.tsx +++ b/shared/teams/common/channel-hooks.tsx @@ -5,12 +5,19 @@ import * as React from 'react' import logger from '@/logger' import {ensureError} from '@/util/errors' import {useLoadedTeam} from '../team/use-loaded-team' -import {createCachedResourceCache, type CachedResourceCache, useCachedResource} from '../use-cached-resource' +import { + createCachedResourceCache, + type CachedResourceCache, + getCachedResourceCache, + useCachedResource, +} from '../use-cached-resource' import { teamChannelsRPCParams, useLoadedTeamChannels, useReloadOnTeamChannelChanges, } from './use-loaded-team-channels' +import {registerExternalResetter} from '@/util/zustand' +import {registerTeamChannelsInvalidator} from './team-channels-invalidation' type ChannelMetasData = { channelMetas: Map @@ -19,6 +26,40 @@ type ChannelMetasData = { const allChannelMetasReloadStaleMs = 5_000 +const emptyChannelMetas = new Map() +const emptyChannelParticipants = new Map() +const emptyChannelMetasData: ChannelMetasData = { + channelMetas: emptyChannelMetas, + channelParticipants: emptyChannelParticipants, +} + +// One cache per team, shared by every consumer: getTLFConversations makes the +// service localize (and remotely refresh participants for) every channel in the +// team, so a per-instance cache turns each extra mount into a full refetch — +// enough of them trips the server's chat rate limit on a big team. +const allChannelMetasCaches = new Map< + T.Teams.TeamID, + CachedResourceCache +>() + +// module scope outlives sign-out, so the next user would be served the previous +// user's channel metas for any team they happen to share +registerExternalResetter('teams-all-channel-metas-caches', () => { + allChannelMetasCaches.clear() +}) + +const allChannelMetasInvalidationListeners = new Set<(teamID: T.Teams.TeamID) => void>() + +// This is the second shared cache of a team's channels, and channel create and +// delete fire no teamChangedByID, so it needs the same explicit drop as +// use-loaded-team-channels. Without it a create/delete leaves every +// useAllChannelMetas consumer - the browse-channels modal, the channels widget, +// the delete confirm itself - serving the pre-change list for the stale window. +registerTeamChannelsInvalidator((teamID: T.Teams.TeamID) => { + allChannelMetasCaches.get(teamID)?.invalidate(teamID) + allChannelMetasInvalidationListeners.forEach(listener => listener(teamID)) +}) + // Filter bots out using team role info, isolate to only when related state changes export const useChannelParticipants = ( teamID: T.Teams.TeamID, @@ -54,21 +95,20 @@ export const useAllChannelMetas = ( teamMeta: {teamname}, } = useLoadedTeam(teamID) const enabled = !dontCallRPC && !!teamname - const emptyChannelMetas = React.useMemo( - () => new Map(), - [] - ) - const emptyChannelParticipants = React.useMemo( - () => new Map(), - [] + const initialData = emptyChannelMetasData + // a dontCallRPC instance is disabled, and useCachedResource resets the cache it + // holds when disabled — give those their own throwaway cache so they can't wipe + // the shared one out from under a real loader + const [localCache] = React.useState>(() => + createCachedResourceCache(initialData, teamID) ) - const initialData = React.useMemo( - () => ({channelMetas: emptyChannelMetas, channelParticipants: emptyChannelParticipants}), - [emptyChannelMetas, emptyChannelParticipants] - ) - const [channelMetasCache] = React.useState>( - () => createCachedResourceCache(initialData, teamID) + const sharedCache = React.useMemo( + () => getCachedResourceCache(allChannelMetasCaches, initialData, teamID), + [initialData, teamID] ) + // gate on `enabled`, not just dontCallRPC: an instance whose teamname has not + // resolved yet is also disabled, and would otherwise reset the shared cache + const channelMetasCache = enabled ? sharedCache : localCache const {data, loading, reload, clear} = useCachedResource({ cache: channelMetasCache, cacheKey: teamID, @@ -111,6 +151,22 @@ export const useAllChannelMetas = ( useReloadOnTeamChannelChanges(teamID, enabled, reload, () => clear(teamID)) + // a mounted list must pick up a create/delete too, not just the next mount + React.useEffect(() => { + if (!enabled) { + return + } + const listener = (invalidatedTeamID: T.Teams.TeamID) => { + if (invalidatedTeamID === teamID) { + void reload() + } + } + allChannelMetasInvalidationListeners.add(listener) + return () => { + allChannelMetasInvalidationListeners.delete(listener) + } + }, [enabled, teamID, reload]) + return { channelMetas: data.channelMetas, channelParticipants: data.channelParticipants, diff --git a/shared/teams/common/general-conv.tsx b/shared/teams/common/general-conv.tsx new file mode 100644 index 000000000000..c7c2ffeb1159 --- /dev/null +++ b/shared/teams/common/general-conv.tsx @@ -0,0 +1,59 @@ +import * as React from 'react' +import * as T from '@/constants/types' +import * as Meta from '@/constants/chat/meta' +import {metasReceived} from '@/chat/inbox/metadata' +import {registerExternalResetter} from '@/util/zustand' +import { + type CachedResourceCache, + createCachedResourceCache, + getCachedResourceCache, + useCachedResource, +} from '../use-cached-resource' + +type GeneralConvData = T.Chat.ConversationIDKey | undefined + +const noGeneralConv: GeneralConvData = undefined + +// A team's #general conversation does not move, but two screens ask for it - the +// team rows and the bot install modal - and each used to hold the answer in its +// own state, so every mount was another findGeneralConvFromTeamID. Share one +// cache per team, and let it live a while since the answer is effectively static. +const generalConvCaches = new Map>() +const generalConvStaleMs = 5 * 60_000 + +registerExternalResetter('teams-general-conv-caches', () => { + generalConvCaches.clear() +}) + +export const useGeneralConvIDKey = (teamID?: T.Teams.TeamID, enabled = true) => { + const validTeamID = teamID && teamID !== T.Teams.noTeamID ? teamID : undefined + const on = enabled && !!validTeamID + const cacheKey = validTeamID ?? T.Teams.noTeamID + // a disabled instance resets whatever cache it holds, so keep it off the shared one + const [localCache] = React.useState>(() => + createCachedResourceCache(noGeneralConv, cacheKey) + ) + const sharedCache = React.useMemo( + () => getCachedResourceCache(generalConvCaches, noGeneralConv, cacheKey), + [cacheKey] + ) + const {data} = useCachedResource({ + cache: on ? sharedCache : localCache, + cacheKey, + enabled: on, + initialData: noGeneralConv, + load: async () => { + const conv = await T.RPCChat.localFindGeneralConvFromTeamIDRpcPromise({ + teamID: validTeamID ?? T.Teams.noTeamID, + }) + const meta = Meta.inboxUIItemToConversationMeta(conv) + if (!meta) { + return noGeneralConv + } + metasReceived([meta]) + return meta.conversationIDKey + }, + staleMs: generalConvStaleMs, + }) + return data +} diff --git a/shared/teams/common/team-channels-invalidation.tsx b/shared/teams/common/team-channels-invalidation.tsx new file mode 100644 index 000000000000..81d40eefb6ec --- /dev/null +++ b/shared/teams/common/team-channels-invalidation.tsx @@ -0,0 +1,23 @@ +import type * as T from '@/constants/types' + +type Invalidator = (teamID: T.Teams.TeamID) => void + +// Creating or deleting a channel fires no teamChangedByID, so the team channel +// cache has to be dropped by hand. This indirection exists so the chat-side +// create-channel screen can trigger it without importing anything from teams: +// teams/common sits in a require cycle (channel-hooks -> @/constants -> +// constants/router -> router-v2/routes -> chat/routes -> teams/common), and a +// chat -> teams import there has broken team rendering on iOS before. Keep this +// module a leaf: type-only imports. +const invalidators = new Set() + +export const registerTeamChannelsInvalidator = (invalidator: Invalidator) => { + invalidators.add(invalidator) + return () => { + invalidators.delete(invalidator) + } +} + +export const invalidateTeamChannels = (teamID: T.Teams.TeamID) => { + invalidators.forEach(invalidator => invalidator(teamID)) +} diff --git a/shared/teams/common/use-loaded-team-channels.tsx b/shared/teams/common/use-loaded-team-channels.tsx index d56861a201e1..679806ce71ca 100644 --- a/shared/teams/common/use-loaded-team-channels.tsx +++ b/shared/teams/common/use-loaded-team-channels.tsx @@ -4,6 +4,8 @@ import * as T from '@/constants/types' import {useEngineActionListener} from '@/engine/action-listener' import logger from '@/logger' import * as React from 'react' +import {registerExternalResetter} from '@/util/zustand' +import {registerTeamChannelsInvalidator} from './team-channels-invalidation' import {useLoadedTeam} from '../team/use-loaded-team' import {type CachedResourceCache, getCachedResourceCache, useCachedResource} from '../use-cached-resource' @@ -25,7 +27,6 @@ type LoadedTeamChannelsCacheMap = Map< > const LoadedTeamChannelsContext = React.createContext(null) -const LoadedTeamChannelsCacheContext = React.createContext(null) const loadedTeamChannelsReloadStaleMs = 5_000 const emptyChannels: ReadonlyMap = new Map() @@ -39,20 +40,37 @@ const emptyLoadedTeamChannelsData: LoadedTeamChannelsData = { channels: emptyChannels, } +// One map for every consumer. The stale window and the single-flight both live on +// the cache object, so callers holding separate maps cannot see each other's +// in-flight request and each issue their own getTLFConversationsLocal - which +// localizes every channel in the team. Measured at 7 calls for one team inside +// 1.5s before this was shared. +const loadedTeamChannelsCache: LoadedTeamChannelsCacheMap = new Map() +const loadedTeamChannelsInvalidationListeners = new Set<(teamID: T.Teams.TeamID | undefined) => void>() + +// module scope outlives sign-out and this is per-user team data +registerExternalResetter('loaded-team-channels-cache', () => { + loadedTeamChannelsCache.forEach(cache => cache.reset(emptyLoadedTeamChannelsData, undefined)) + loadedTeamChannelsCache.clear() +}) + +// While every consumer held a private cache a remount happened to refetch, which +// is what the channel list relied on after a create. Sharing one cache means a +// remount inside the stale window serves the pre-change channels instead, so the +// create/delete screens have to drop this explicitly. +registerTeamChannelsInvalidator((teamID: T.Teams.TeamID) => { + const key = loadableTeamID(teamID) + loadedTeamChannelsCache.get(key)?.invalidate(key) + loadedTeamChannelsInvalidationListeners.forEach(listener => listener(key)) +}) + // forceLocalCache: a disabled "shadow" instance (one that returns the context // value instead of its own) must NOT share the loader's cache map. With enabled=false // useCachedResource resets the cache (loadedAt=0), which would clobber the loader's // loaded data. Give shadows a private throwaway map so their resets are harmless. -const useLoadedTeamChannelsCacheMap = ( - providedCacheMap?: LoadedTeamChannelsCacheMap, - forceLocalCache = false -) => { - const contextCacheMap = React.useContext(LoadedTeamChannelsCacheContext) +const useLoadedTeamChannelsCacheMap = (forceLocalCache: boolean) => { const [localCacheMap] = React.useState(() => new Map()) - if (forceLocalCache) { - return localCacheMap - } - return providedCacheMap ?? contextCacheMap ?? localCacheMap + return forceLocalCache ? localCacheMap : loadedTeamChannelsCache } export const teamChannelsRPCParams = (teamname: string) => ({ @@ -90,7 +108,6 @@ const useLoadedTeamChannelsRaw = ( teamID: T.Teams.TeamID, providedTeamname?: string, enabled = true, - providedCacheMap?: LoadedTeamChannelsCacheMap, forceLocalCache = false ): LoadedTeamChannels => { const validTeamID = loadableTeamID(teamID) @@ -98,7 +115,12 @@ const useLoadedTeamChannelsRaw = ( teamMeta: {teamname: loadedTeamname}, } = useLoadedTeam(teamID, enabled) const teamnameToLoad = providedTeamname || loadedTeamname - const cacheMap = useLoadedTeamChannelsCacheMap(providedCacheMap, forceLocalCache) + // useCachedResource resets whatever cache it holds while disabled, so a + // disabled instance must never hold the shared one — including the ordinary + // consumer whose teamname has not resolved yet, which would otherwise wipe a + // real loader's data mid-flight. Gate the cache on exactly the load condition. + const canLoad = enabled && !!validTeamID && !!teamnameToLoad + const cacheMap = useLoadedTeamChannelsCacheMap(forceLocalCache || !canLoad) const cache = React.useMemo( () => getCachedResourceCache(cacheMap, emptyLoadedTeamChannelsData, validTeamID), [cacheMap, validTeamID] @@ -106,7 +128,7 @@ const useLoadedTeamChannelsRaw = ( const {data, loading, reload, clear} = useCachedResource({ cache, cacheKey: validTeamID, - enabled: enabled && !!validTeamID && !!teamnameToLoad, + enabled: canLoad, initialData: emptyLoadedTeamChannelsData, load: async () => { if (!teamnameToLoad) { @@ -149,21 +171,39 @@ const useLoadedTeamChannelsRaw = ( useReloadOnTeamChannelChanges(validTeamID, enabled, reload, () => clear(validTeamID)) - return {...data, loading, reload} + // a mounted list must pick up an invalidation too, not just the next mount + React.useEffect(() => { + if (!enabled || !validTeamID) { + return + } + const listener = (invalidatedTeamID: T.Teams.TeamID | undefined) => { + if (invalidatedTeamID === validTeamID) { + void reload() + } + } + loadedTeamChannelsInvalidationListeners.add(listener) + return () => { + loadedTeamChannelsInvalidationListeners.delete(listener) + } + }, [enabled, validTeamID, reload]) + + const {channelParticipants, channels} = data + return React.useMemo( + () => ({channelParticipants, channels, loading, reload}), + [channelParticipants, channels, loading, reload] + ) } export const LoadedTeamChannelsProvider = ( props: React.PropsWithChildren<{teamID: T.Teams.TeamID; teamname?: string}> ) => { const {children, teamID, teamname} = props - const [cacheMap] = React.useState(() => new Map()) - const loadedTeamChannels = useLoadedTeamChannelsRaw(teamID, teamname, true, cacheMap) - const value = {...loadedTeamChannels, teamID} - return ( - - {children} - + const loadedTeamChannels = useLoadedTeamChannelsRaw(teamID, teamname, true) + const value = React.useMemo( + () => ({...loadedTeamChannels, teamID}), + [loadedTeamChannels, teamID] ) + return {children} } export const useLoadedTeamChannels = ( @@ -172,7 +212,7 @@ export const useLoadedTeamChannels = ( ): LoadedTeamChannels => { const context = React.useContext(LoadedTeamChannelsContext) const useContextValue = context?.teamID === teamID - const raw = useLoadedTeamChannelsRaw(teamID, teamname, !useContextValue, undefined, useContextValue) + const raw = useLoadedTeamChannelsRaw(teamID, teamname, !useContextValue, useContextValue) return useContextValue ? context : raw } diff --git a/shared/teams/confirm-modals/delete-channel.tsx b/shared/teams/confirm-modals/delete-channel.tsx index 95ce9cf167c5..fea2f71887b7 100644 --- a/shared/teams/confirm-modals/delete-channel.tsx +++ b/shared/teams/confirm-modals/delete-channel.tsx @@ -5,6 +5,7 @@ import * as T from '@/constants/types' import {useNavigation} from '@react-navigation/native' import {pluralize} from '@/util/string' import setRouteParamsIfPresent from './set-route-params-if-present' +import {invalidateTeamChannels} from '@/teams/common/team-channels-invalidation' import {useAllChannelMetas} from '@/teams/common/channel-hooks' type Props = { @@ -77,11 +78,12 @@ const DeleteChannel = (props: Props) => { for (const channelID of channelIDs) { await deleteChannel(channelID) } + invalidateTeamChannels(teamID) setRouteParamsIfPresent(navigation, 'team', {selectedChannels: undefined}) C.Router2.clearModals() } C.ignorePromise(f()) - }, [channelIDs, deleteChannel, navigation]) + }, [channelIDs, deleteChannel, navigation, teamID]) return ( () +const primedParticipantConvs = new Set() const defaultTab: T.Teams.TabKey = 'members' +// module scope outlives sign-out; both are keyed by team/conversation, so the +// next user would inherit the previous user's tab choices and priming state +registerExternalResetter('teams-team-index', () => { + lastSelectedTabs.clear() + primedParticipantConvs.clear() +}) + const getSettingsErrorWaitingKeys = (teamID: T.Teams.TeamID) => [ C.waitingKeyTeamsLoadWelcomeMessage(teamID), @@ -127,29 +136,50 @@ const TeamBody = (props: Props) => { const {loading: loadingTeam, teamDetails, teamMeta, yourOperations} = useLoadedTeam(teamID) - C.Router2.useSafeFocusEffect(() => { - return () => teamSeen(teamID) - }) - C.Router2.useSafeFocusEffect(() => { - return () => { - if (props.justFinishedAddWizard) { - clearJustFinishedAddWizard() + // useSafeFocusEffect re-runs (and so runs its cleanup) whenever the callback + // identity changes, so an inline callback here would fire the gregor dismiss + // RPC on every render of this screen + const justFinishedAddWizard = props.justFinishedAddWizard + C.Router2.useSafeFocusEffect( + React.useCallback(() => { + return () => teamSeen(teamID) + }, [teamID]) + ) + C.Router2.useSafeFocusEffect( + React.useCallback(() => { + return () => { + if (justFinishedAddWizard) { + clearJustFinishedAddWizard() + } } - } - }) + }, [justFinishedAddWizard, clearJustFinishedAddWizard]) + ) const {channels, loading: loadingChannels} = useLoadedTeamChannels(teamID, teamMeta.teamname) // getTLFConversations leaves team channel participants empty; ask the service to // refresh them, which pushes ChatParticipantsInfo into useInboxMetadataState (read // by the channel rows). Without this the member counts render 0. + // + // This only primes them: later membership changes arrive on their own as + // ChatParticipantsInfo. `channels` gets a new identity on every reload of the + // channel list (stale refresh, team change, refocus), and each request costs a + // remote round trip, so a big team re-primed on every reload was enough to trip + // the server's chat rate limit. Ask once per conversation per session. const refreshParticipants = C.useRPC(T.RPCChat.localRefreshParticipantsRpcPromise) React.useEffect(() => { for (const conversationIDKey of channels.keys()) { + if (primedParticipantConvs.has(conversationIDKey)) { + continue + } + primedParticipantConvs.add(conversationIDKey) refreshParticipants( [{convID: T.Chat.keyToConversationID(conversationIDKey)}], () => {}, - () => {} + () => { + // let a later render retry a conversation whose refresh failed + primedParticipantConvs.delete(conversationIDKey) + } ) } }, [channels, refreshParticipants]) diff --git a/shared/teams/team/member/edit-channel.tsx b/shared/teams/team/member/edit-channel.tsx index a2862a134664..7550f45a74db 100644 --- a/shared/teams/team/member/edit-channel.tsx +++ b/shared/teams/team/member/edit-channel.tsx @@ -3,6 +3,7 @@ import * as Kb from '@/common-adapters' import * as React from 'react' import * as T from '@/constants/types' import {useSafeNavigation} from '@/util/safe-navigation' +import {invalidateTeamChannels} from '@/teams/common/team-channels-invalidation' import {useLoadedTeam} from '../use-loaded-team' type Props = { @@ -73,6 +74,12 @@ const EditChannel = (props: Props) => { ] Promise.all(ps) .then(() => { + // renaming a channel or editing its description fires no + // teamChangedByID, so the shared channel caches would keep serving the + // old name and description for their stale window + if (ps.length) { + invalidateTeamChannels(teamID) + } nav.safeNavigateUp() }) .catch(() => {}) diff --git a/shared/teams/team/rows/index.tsx b/shared/teams/team/rows/index.tsx index 9ee62233584f..2ba8fd01f28f 100644 --- a/shared/teams/team/rows/index.tsx +++ b/shared/teams/team/rows/index.tsx @@ -1,8 +1,8 @@ import * as C from '@/constants' -import * as Meta from '@/constants/chat/meta' import * as T from '@/constants/types' import type * as Kb from '@/common-adapters' import * as React from 'react' +import {useGeneralConvIDKey} from '@/teams/common/general-conv' import EmptyRow from './empty-row' import LoadingRow from './loading' import MemberRow from './member-row' @@ -23,7 +23,6 @@ import {getOrderedMemberArray, sortInvites, getOrderedBotsArray} from './helpers import {useEmojiState} from '../../emojis/use-emoji' import {useCurrentUserState} from '@/stores/current-user' import {useTeamsListMap} from '@/teams/use-teams-list' -import {metasReceived} from '@/chat/inbox/metadata' export type Item = | {type: 'members-loading'} @@ -287,49 +286,8 @@ export const useSubteamsSections = ( return sections } -const useGeneralConversationIDKey = (teamID?: T.Teams.TeamID) => { - const [conversationIDKeyResult, setConversationIDKeyResult] = React.useState< - {conversationIDKey: T.Chat.ConversationIDKey; teamID: T.Teams.TeamID} | undefined - >() - const findGeneralConvIDFromTeamID = C.useRPC(T.RPCChat.localFindGeneralConvFromTeamIDRpcPromise) - const requestIDRef = React.useRef(0) - const conversationIDKey = - conversationIDKeyResult && conversationIDKeyResult.teamID === teamID - ? conversationIDKeyResult.conversationIDKey - : undefined - - React.useEffect(() => { - if (conversationIDKey || !teamID) { - return - } - requestIDRef.current += 1 - const requestID = requestIDRef.current - findGeneralConvIDFromTeamID( - [{teamID}], - conv => { - if (requestIDRef.current !== requestID) { - return - } - const meta = Meta.inboxUIItemToConversationMeta(conv) - if (!meta) { - return - } - metasReceived([meta]) - setConversationIDKeyResult({conversationIDKey: meta.conversationIDKey, teamID}) - }, - () => {} - ) - return () => { - if (requestIDRef.current === requestID) { - requestIDRef.current += 1 - } - } - }, [conversationIDKey, findGeneralConvIDFromTeamID, teamID]) - return conversationIDKey -} - export const useEmojiSections = (teamID: T.Teams.TeamID, shouldActuallyLoad: boolean): Array
=> { - const convID = useGeneralConversationIDKey(teamID) + const convID = useGeneralConvIDKey(teamID) const getUserEmoji = C.useRPC(T.RPCChat.localUserEmojisRpcPromise) const [customEmoji, setCustomEmoji] = React.useState([]) const [filter, setFilter] = React.useState('') diff --git a/shared/teams/team/settings-tab/default-channels.tsx b/shared/teams/team/settings-tab/default-channels.tsx index c4d9f71079db..a7361abaf2f1 100644 --- a/shared/teams/team/settings-tab/default-channels.tsx +++ b/shared/teams/team/settings-tab/default-channels.tsx @@ -2,107 +2,68 @@ import * as C from '@/constants' import * as React from 'react' import * as Kb from '@/common-adapters' import * as T from '@/constants/types' -import type {RPCError} from '@/util/errors' -import {produce} from 'immer' +import logger from '@/logger' +import {registerExternalResetter} from '@/util/zustand' import {ChannelsWidget} from '@/teams/common' import {useLoadedTeam} from '../use-loaded-team' +import {type CachedResourceCache, getCachedResourceCache, useCachedResource} from '../../use-cached-resource' type Props = { teamID: T.Teams.TeamID } -export const useDefaultChannels = (teamID: T.Teams.TeamID) => { - type DefaultChannelsState = { - defaultChannels: Array - error?: RPCError - loadedTeamID?: T.Teams.TeamID - waiting: boolean - } +type DefaultChannelsData = ReadonlyArray - const getDefaultChannelsRPC = C.useRPC(T.RPCChat.localGetDefaultTeamChannelsLocalRpcPromise) - const [state, setState] = React.useState({ - defaultChannels: [], - error: undefined, - loadedTeamID: teamID, - waiting: true, - }) - const requestVersionRef = React.useRef(0) - const requestTeamIDRef = React.useRef(teamID) +const defaultChannelsStaleMs = 5_000 +const emptyDefaultChannels: DefaultChannelsData = [] - // no synchronous setState here so the mount effect below can call it directly; - // initial/mismatched-team state already shows waiting - const load = React.useCallback(() => { - const requestVersion = ++requestVersionRef.current - getDefaultChannelsRPC( - [{teamID}], - result => { - if (requestVersion !== requestVersionRef.current) { - return - } - setState({ - defaultChannels: [ - {channelname: 'general', conversationIDKey: 'unused'}, - ...(result.convs || []).map(conv => ({ - channelname: conv.channel, - conversationIDKey: conv.convID, - })), - ], - error: undefined, - loadedTeamID: teamID, - waiting: false, - }) - }, - err => { - if (requestVersion !== requestVersionRef.current) { - return - } - setState( - produce(draft => { - draft.error = err - draft.loadedTeamID = teamID - draft.waiting = false - }) - ) - } - ) - }, [getDefaultChannelsRPC, teamID]) +// One cache per team, shared by every consumer: each load is a remote +// chat.1.remote.getDefaultTeamChannels round trip, so a per-instance cache turns +// every extra mount of the settings tab into another hit on the chat rate limit. +const defaultChannelsCaches = new Map< + T.Teams.TeamID, + CachedResourceCache +>() - const reloadDefaultChannels = React.useCallback(() => { - setState( - produce(draft => { - draft.error = undefined - draft.waiting = true - }) - ) - load() - }, [load]) +// module scope outlives sign-out, so the next user would inherit this user's channels +registerExternalResetter('teams-default-channels-caches', () => { + defaultChannelsCaches.clear() +}) - React.useEffect(() => { - if (requestTeamIDRef.current !== teamID) { - requestTeamIDRef.current = teamID - requestVersionRef.current++ - } - }, [teamID]) - - React.useEffect(() => { - load() - }, [load]) +// resolve rather than reject on failure: consumers render an empty list (and no +// spinner) on error, and a cached failure keeps a broken team from re-requesting +// on every render. Module scope, not inline in the hook: a try/catch wrapping a +// value block is a react-compiler bailout. +const loadDefaultChannels = async (teamID: T.Teams.TeamID): Promise => { + try { + const {convs} = await T.RPCChat.localGetDefaultTeamChannelsLocalRpcPromise({teamID}) + return [ + {channelname: 'general', conversationIDKey: 'unused'}, + ...(convs ?? []).map(conv => ({channelname: conv.channel, conversationIDKey: conv.convID})), + ] + } catch (error) { + logger.warn(`Failed to load default channels for ${teamID}`, error) + return emptyDefaultChannels + } +} - const visibleState = - state.loadedTeamID === teamID - ? state - : { - defaultChannels: [] as Array, - error: undefined, - loadedTeamID: teamID, - waiting: true, - } +export const useDefaultChannels = (teamID: T.Teams.TeamID) => { + const cache = React.useMemo( + () => getCachedResourceCache(defaultChannelsCaches, emptyDefaultChannels, teamID), + [teamID] + ) + const {data, loaded, loading, reload} = useCachedResource({ + cache, + cacheKey: teamID, + initialData: emptyDefaultChannels, + load: async () => await loadDefaultChannels(teamID), + staleMs: defaultChannelsStaleMs, + }) return { - defaultChannels: visibleState.defaultChannels, - defaultChannelsWaiting: visibleState.waiting, - error: visibleState.error, - reloadDefaultChannels, + defaultChannels: data, + defaultChannelsWaiting: loading || !loaded, + reloadDefaultChannels: reload, } } @@ -125,7 +86,7 @@ const DefaultChannels = (props: Props) => { [{convs, teamID}], () => { setWaiting(false) - reloadDefaultChannels() + void reloadDefaultChannels() }, error => { setWaiting(false) @@ -145,7 +106,7 @@ const DefaultChannels = (props: Props) => { [{convs, teamID}], () => { setWaiting(false) - reloadDefaultChannels() + void reloadDefaultChannels() }, error => { setWaiting(false) diff --git a/shared/teams/team/settings-tab/index.tsx b/shared/teams/team/settings-tab/index.tsx index 52ba4e63821d..efae4ed62f5d 100644 --- a/shared/teams/team/settings-tab/index.tsx +++ b/shared/teams/team/settings-tab/index.tsx @@ -8,7 +8,6 @@ import {pluralize} from '@/util/string' import type {RPCError} from '@/util/errors' import RetentionPicker from './retention' import DefaultChannels from './default-channels' -import isEqual from 'lodash/isEqual' import {useLoadedTeam} from '../use-loaded-team' import {useIsBigTeam} from '../../common/use-loaded-team-channels' import {useSettingsTabState} from './use-settings' @@ -138,7 +137,6 @@ const OpenTeam = (props: { Anyone will be able to join immediately. Users will join as { const {error, savePublicity, isBigTeam, teamID, yourOperations, teamname, showOpenTeamWarning} = p const {canShowcase, allowOpenTrigger} = p - const [newPublicityAnyMember, setNewPublicityAnyMember] = React.useState(p.publicityAnyMember) - const [newPublicityTeam, setNewPublicityTeam] = React.useState(p.publicityTeam) - const [newIgnoreAccessRequests, setNewIgnoreAccessRequests] = React.useState(p.ignoreAccessRequests) - const [newPublicityMember, setNewPublicityMember] = React.useState(p.publicityMember) + const serverSettings = { + ignoreAccessRequests: p.ignoreAccessRequests, + openTeam: p.openTeam, + openTeamRole: p.openTeamRole, + publicityAnyMember: p.publicityAnyMember, + publicityMember: p.publicityMember, + publicityTeam: p.publicityTeam, + } + // resync the checkboxes whenever the server-side values change. Doing this in + // render instead of with a key on this component matters: a key would remount the + // whole subtree, and RetentionPicker / DefaultChannels each load over RPC on mount. + const serverKey = `${p.ignoreAccessRequests}:${p.openTeam}:${p.openTeamRole}:${p.publicityAnyMember}:${p.publicityMember}:${p.publicityTeam}` + const [local, setLocal] = React.useState({...serverSettings, serverKey}) const [isRolePickerOpen, setIsRolePickerOpen] = React.useState(false) - const [newOpenTeam, setNewOpenTeam] = React.useState(p.openTeam) - const [newOpenTeamRole, setNewOpenTeamRole] = React.useState(p.openTeamRole) const lastAllowOpenTriggerRef = React.useRef(allowOpenTrigger) - React.useEffect(() => { - if (lastAllowOpenTriggerRef.current !== allowOpenTrigger) { - lastAllowOpenTriggerRef.current = allowOpenTrigger - setNewOpenTeam(o => !o) - } - }, [allowOpenTrigger]) + if (local.serverKey !== serverKey) { + setLocal({...serverSettings, serverKey}) + } - const lastSave = React.useRef({ + const { ignoreAccessRequests: newIgnoreAccessRequests, openTeam: newOpenTeam, openTeamRole: newOpenTeamRole, publicityAnyMember: newPublicityAnyMember, publicityMember: newPublicityMember, publicityTeam: newPublicityTeam, - }) - React.useEffect(() => { - const next = { - ignoreAccessRequests: newIgnoreAccessRequests, - openTeam: newOpenTeam, - openTeamRole: newOpenTeamRole, - publicityAnyMember: newPublicityAnyMember, - publicityMember: newPublicityMember, - publicityTeam: newPublicityTeam, - } - if (!isEqual(next, lastSave.current)) { - lastSave.current = next + serverKey: localServerKey, + } = local + + const applySettings = React.useCallback( + (changes: Partial) => { + const next: T.Teams.PublicitySettings = { + ignoreAccessRequests: newIgnoreAccessRequests, + openTeam: newOpenTeam, + openTeamRole: newOpenTeamRole, + publicityAnyMember: newPublicityAnyMember, + publicityMember: newPublicityMember, + publicityTeam: newPublicityTeam, + ...changes, + } + setLocal({...next, serverKey: localServerKey}) savePublicity(next) + }, + [ + localServerKey, + newIgnoreAccessRequests, + newOpenTeam, + newOpenTeamRole, + newPublicityAnyMember, + newPublicityMember, + newPublicityTeam, + savePublicity, + ] + ) + + React.useEffect(() => { + if (lastAllowOpenTriggerRef.current !== allowOpenTrigger) { + lastAllowOpenTriggerRef.current = allowOpenTrigger + applySettings({openTeam: !newOpenTeam}) } - }, [savePublicity, newIgnoreAccessRequests, newOpenTeam, newOpenTeamRole, newPublicityAnyMember, newPublicityMember, newPublicityTeam]) + }, [allowOpenTrigger, applySettings, newOpenTeam]) return ( @@ -237,7 +259,7 @@ const Settings = (p: Props) => { yourOperationsJoinTeam={yourOperations.joinTeam} canShowcase={canShowcase} newPublicityMember={newPublicityMember} - setNewPublicityMember={setNewPublicityMember} + setNewPublicityMember={publicityMember => applySettings({publicityMember})} /> {(yourOperations.changeOpenTeam || yourOperations.setTeamShowcase || @@ -249,11 +271,14 @@ const Settings = (p: Props) => { {yourOperations.setPublicityAny ? ( applySettings({publicityAnyMember})} /> ) : null} {yourOperations.setTeamShowcase ? ( - + applySettings({publicityTeam})} + /> ) : null} {yourOperations.changeOpenTeam ? ( { onCancelRolePicker={() => setIsRolePickerOpen(false)} onConfirmRolePicker={(role: T.Teams.TeamRoleType) => { setIsRolePickerOpen(false) - setNewOpenTeamRole(role) + applySettings({openTeamRole: role}) }} onOpenRolePicker={() => setIsRolePickerOpen(true)} /> @@ -272,7 +297,7 @@ const Settings = (p: Props) => { {!newOpenTeam && yourOperations.changeTarsDisabled ? ( applySettings({ignoreAccessRequests})} /> ) : null} @@ -431,11 +456,8 @@ const SettingsTabContainer = (ownProps: OwnProps) => { ] ) - const settingsKey = `${ignoreAccessRequests}:${openTeam}:${openTeamRole}:${publicityAnyMember}:${publicityMember}:${publicityTeam}` - return ( boolean, - setTeamRetentionPolicy: (policy?: T.RPCChat.RetentionPolicy | null) => void -) => { - try { - const servicePolicy = await T.RPCChat.localGetTeamRetentionLocalRpcPromise( - {teamID}, - C.waitingKeyTeamsLoadRetentionPolicy(teamID) - ) - if (!shouldApply()) { - return - } - setTeamRetentionPolicy(servicePolicy) - } catch { - if (!shouldApply()) { - return - } - setTeamRetentionPolicy(undefined) - } -} +type TeamRetentionData = T.Retention.RetentionPolicy | undefined -const useLoadedTeamRetentionPolicy = (teamID: T.Teams.TeamID) => { - type TeamRetentionState = { - loadedTeamID?: T.Teams.TeamID - teamPolicy?: T.Retention.RetentionPolicy - } +const teamRetentionStaleMs = 5_000 +const noTeamRetentionPolicy: TeamRetentionData = undefined - const [state, setState] = React.useState({loadedTeamID: teamID, teamPolicy: undefined}) - const requestVersionRef = React.useRef(0) - const requestTeamIDRef = React.useRef(teamID) - - const setTeamRetentionPolicy = React.useCallback((policy?: T.RPCChat.RetentionPolicy | null) => { - const nextPolicy = Teams.serviceRetentionPolicyToRetentionPolicy(policy) - setState({ - loadedTeamID: teamID, - teamPolicy: nextPolicy.type === 'inherit' ? Teams.retentionPolicies.policyRetain : nextPolicy, - }) - }, [teamID]) - - const reload = React.useCallback(async () => { - const requestVersion = ++requestVersionRef.current - await loadTeamRetentionPolicy( - teamID, - () => requestVersion === requestVersionRef.current, - setTeamRetentionPolicy - ) - }, [setTeamRetentionPolicy, teamID]) +// One cache per team, shared by every consumer (settings tab and every chat info +// panel for the team): a burst of remounts would otherwise be a burst of +// GetTeamRetentionLocal calls with identical arguments. +const teamRetentionCaches = new Map>() - React.useEffect(() => { - if (requestTeamIDRef.current !== teamID) { - requestTeamIDRef.current = teamID - requestVersionRef.current++ - } - }, [teamID]) +// module scope outlives sign-out, so the next user would inherit this user's policies +registerExternalResetter('teams-retention-caches', () => { + teamRetentionCaches.clear() +}) - React.useEffect(() => { - let canceled = false - const requestVersion = ++requestVersionRef.current - void loadTeamRetentionPolicy( - teamID, - () => !canceled && requestVersion === requestVersionRef.current, - setTeamRetentionPolicy - ) - return () => { - canceled = true - } - }, [setTeamRetentionPolicy, teamID]) - - C.Router2.useSafeFocusEffect( - React.useCallback(() => { - void reload() - }, [reload]) +const useLoadedTeamRetentionPolicy = (teamID: T.Teams.TeamID) => { + const enabled = !!teamID && teamID !== T.Teams.noTeamID + // an adhoc conversation has no team, and useCachedResource resets the cache it + // holds when disabled — give those instances their own throwaway cache so they + // can't wipe the shared one out from under a real loader + const [localCache] = React.useState>(() => + createCachedResourceCache(noTeamRetentionPolicy, teamID) + ) + const sharedCache = React.useMemo( + () => getCachedResourceCache(teamRetentionCaches, noTeamRetentionPolicy, teamID), + [teamID] ) + const {data: teamPolicy, reload} = useCachedResource({ + cache: enabled ? sharedCache : localCache, + cacheKey: teamID, + enabled, + initialData: noTeamRetentionPolicy, + // resolve rather than reject on failure: the picker falls back to the default + // "retain" policy on error instead of spinning forever + load: async () => { + let servicePolicy: T.RPCChat.RetentionPolicy | undefined | null + try { + servicePolicy = await T.RPCChat.localGetTeamRetentionLocalRpcPromise( + {teamID}, + C.waitingKeyTeamsLoadRetentionPolicy(teamID) + ) + } catch (error) { + logger.warn(`Failed to load team retention policy for ${teamID}`, error) + } + const policy = Teams.serviceRetentionPolicyToRetentionPolicy(servicePolicy) + return policy.type === 'inherit' ? Teams.retentionPolicies.policyRetain : policy + }, + staleMs: teamRetentionStaleMs, + }) + // the notification carries the new policy, but reload() is what actually + // invalidates the shared cache for the other consumers useEngineActionListener('chat.1.NotifyChat.ChatSetTeamRetention', action => { - if (action.payload.params.teamID !== teamID) { - return - } - const first = action.payload.params.convs?.[0] - if (!first?.teamRetention) { + if (action.payload.params.teamID === teamID) { void reload() - return } - setTeamRetentionPolicy(first.teamRetention) }) - const visibleState = - state.loadedTeamID === teamID ? state : {loadedTeamID: teamID, teamPolicy: undefined} - return { - loading: !visibleState.teamPolicy, - teamPolicy: visibleState.teamPolicy, + loading: !teamPolicy, + teamPolicy, } } diff --git a/shared/teams/team/settings-tab/retention/warning.tsx b/shared/teams/team/settings-tab/retention/warning.tsx index 2cd46ed9c451..d21a39c8db69 100644 --- a/shared/teams/team/settings-tab/retention/warning.tsx +++ b/shared/teams/team/settings-tab/retention/warning.tsx @@ -1,5 +1,6 @@ import * as Kb from '@/common-adapters' import * as C from '@/constants' +import * as React from 'react' import type * as T from '@/constants/types' import type {RetentionEntityType} from '.' import ConfirmWarning from '../confirm-warning' @@ -87,12 +88,12 @@ const RetentionWarningContainer = (ownProps: OwnProps) => { const updateConfirm = useConfirm(s => s.dispatch.updateConfirm) C.Router2.useSafeFocusEffect( - () => { + React.useCallback(() => { openModal() return () => { closeModal() } - } + }, [openModal, closeModal]) ) const onConfirm = () => { diff --git a/shared/teams/team/use-loaded-team.test.tsx b/shared/teams/team/use-loaded-team.test.tsx new file mode 100644 index 000000000000..66bdaaaed044 --- /dev/null +++ b/shared/teams/team/use-loaded-team.test.tsx @@ -0,0 +1,101 @@ +/** @jest-environment jsdom */ +/// +import {expect, jest, test} from '@jest/globals' +import {act, render} from '@testing-library/react' +import * as T from '@/constants/types' +import {useCurrentUserState} from '@/stores/current-user' +import {useConfigState} from '@/stores/config' +import {LoadedTeamsListProvider} from '../use-teams-list' +import {LoadedTeamChannelsProvider, useLoadedTeamChannels} from '../common/use-loaded-team-channels' +import {LoadedTeamProvider, useLoadedTeam} from './use-loaded-team' + +const teamID = 'tid1' as T.Teams.TeamID + +const annotated = { + invites: [], + joinRequests: [], + members: [], + name: 'testteam', + settings: {joinAs: T.RPCGen.TeamRole.reader, open: false}, + showcase: {anyMemberShowcase: false, description: '', isShowcased: false}, + tarsDisabled: false, + transitiveSubteamsUnverified: {entries: []}, +} as unknown as T.RPCGen.AnnotatedTeam + +let annotatedCalls = 0 +jest.spyOn(T.RPCGen, 'teamsGetAnnotatedTeamRpcPromise').mockImplementation(async () => { + annotatedCalls++ + await Promise.resolve() + return annotated +}) +jest.spyOn(T.RPCChat, 'localGetTLFConversationsLocalRpcPromise').mockImplementation(async () => { + await Promise.resolve() + return {convs: [], offline: false} as never +}) +let listCalls = 0 +jest.spyOn(T.RPCGen, 'teamsTeamListUnverifiedRpcPromise').mockImplementation(async () => { + listCalls++ + await Promise.resolve() + return { + teams: [ + { + fqName: 'testteam', + isOpenTeam: false, + memberCount: 1, + role: T.RPCGen.TeamRole.owner, + teamID, + username: 'testuser', + }, + ], + } as never +}) +jest.spyOn(T.RPCGen, 'teamsGetTeamRoleMapRpcPromise').mockImplementation(async () => { + await Promise.resolve() + return {teams: {}, version: 1} as never +}) + +let bodyRenders = 0 +const Body = () => { + // counts real renders: compiling this away is exactly what the test measures + 'use no memo' + bodyRenders++ + const {teamMeta} = useLoadedTeam(teamID) + const {channels} = useLoadedTeamChannels(teamID) + return ( +
+ {teamMeta.teamname} + {channels.size} +
+ ) +} + +const WithChannels = () => { + const {teamMeta} = useLoadedTeam(teamID) + return ( + + + + ) +} + +// The team screen mounts one real loader plus a shadow instance per consumer; +// a teams-list load landing must not re-issue getAnnotatedTeam. +test('team screen loads getAnnotatedTeam once', async () => { + useCurrentUserState.setState({username: 'testuser'}) + useConfigState.setState({loggedIn: true}) + render( + + + + + + ) + for (let i = 0; i < 60; i++) { + await act(async () => { + await Promise.resolve() + }) + } + expect(listCalls).toBe(1) + expect(bodyRenders).toBeLessThan(10) + expect(annotatedCalls).toBe(1) +}) diff --git a/shared/teams/team/use-loaded-team.tsx b/shared/teams/team/use-loaded-team.tsx index 84011b0ff685..888a57922cc9 100644 --- a/shared/teams/team/use-loaded-team.tsx +++ b/shared/teams/team/use-loaded-team.tsx @@ -96,12 +96,15 @@ const useLoadedTeamRaw = ( ) // Seed from the teams-list cache so the header (teamname, avatar, member count) // renders immediately instead of waiting for getAnnotatedTeam to round-trip. + // key the memo on this team's meta, not on the map: the map gets a new + // identity on every teams-list reload, and a fresh initialData object churns + // the whole useCachedResource state/effect chain for no reason const teamsListMap = useTeamsListMap() + const listMeta = validTeamID ? teamsListMap.get(validTeamID) : undefined const initialData = React.useMemo(() => { const data = emptyLoadedTeamData(validTeamID) - const listMeta = validTeamID ? teamsListMap.get(validTeamID) : undefined return listMeta ? {...data, teamMeta: listMeta} : data - }, [validTeamID, teamsListMap]) + }, [validTeamID, listMeta]) const {data, loaded, loading, reload, clear} = useCachedResource({ cache, cacheKey: validTeamID, @@ -123,7 +126,13 @@ const useLoadedTeamRaw = ( }, staleMs: loadedTeamReloadStaleMs, }) - const roleAndDetails = roleAndDetailsFromMap(roleMap, validTeamID ?? T.Teams.noTeamID) + // builds a fresh object whenever the team is in the map, so without this the + // memos below (and the context value built from them) never hit and every + // consumer re-renders on every render of the provider + const roleAndDetails = React.useMemo( + () => roleAndDetailsFromMap(roleMap, validTeamID ?? T.Teams.noTeamID), + [roleMap, validTeamID] + ) const teamMeta = React.useMemo( () => ({ ...data.teamMeta, @@ -160,14 +169,18 @@ const useLoadedTeamRaw = ( } }, subscribeToUpdates) - return {...data, loaded, loading, reload, teamMeta, yourOperations} + const teamDetails = data.teamDetails + return React.useMemo( + () => ({loaded, loading, reload, teamDetails, teamMeta, yourOperations}), + [loaded, loading, reload, teamDetails, teamMeta, yourOperations] + ) } export const LoadedTeamProvider = (props: React.PropsWithChildren<{teamID: T.Teams.TeamID}>) => { const {children, teamID} = props const [cacheMap] = React.useState(() => new Map()) const loadedTeam = useLoadedTeamRaw(teamID, true, cacheMap) - const value = {...loadedTeam, teamID} + const value = React.useMemo(() => ({...loadedTeam, teamID}), [loadedTeam, teamID]) return ( {children} diff --git a/shared/teams/use-cached-resource.test.tsx b/shared/teams/use-cached-resource.test.tsx new file mode 100644 index 000000000000..48ded3bda3e0 --- /dev/null +++ b/shared/teams/use-cached-resource.test.tsx @@ -0,0 +1,277 @@ +/** @jest-environment jsdom */ +/// +import {expect, jest, test} from '@jest/globals' +import {act, render} from '@testing-library/react' +import {createCachedResourceCache, useCachedResource} from './use-cached-resource' + +// A setState that lands outside act() - every load settling on its own - is +// scheduled on React's MessageChannel, i.e. a macrotask. A flush built only from +// awaited microtasks never lets the event loop reach it, so the commit lands (or +// not) depending on how the runtime happens to interleave: use a real timer. +const flush = async (turns = 40) => { + for (let i = 0; i < turns; i++) { + await act(async () => { + await new Promise(resolve => setTimeout(resolve, 0)) + }) + } +} + +type Data = {v: number} + +// A caller that rebuilds initialData every render (seeding it from another +// store) must not put useCachedResource into a render loop. +test('unstable initialData does not loop', async () => { + const cache = createCachedResourceCache({v: 0}, 'k') + let calls = 0 + let renders = 0 + const load = jest.fn(async () => { + calls++ + await Promise.resolve() + return {v: calls} + }) + const Comp = () => { + // counts real renders: compiling this away is exactly what the test measures + 'use no memo' + renders++ + const {data} = useCachedResource({cache, cacheKey: 'k', initialData: {v: 0}, load, staleMs: 5000}) + return
{data.v}
+ } + render() + await flush() + expect(calls).toBe(1) + expect(renders).toBeLessThan(10) +}) + +// A load that rejects leaves loadedAt at 0, i.e. permanently stale. Without a +// backoff every re-render re-issued the request the instant the previous one +// settled, which hammered both the service and the server. +test('a failed load backs off instead of retrying on every render', async () => { + const cache = createCachedResourceCache({v: 0}, 'k') + let calls = 0 + let loadIfStale: (() => Promise) | undefined + const load = jest.fn(async () => { + calls++ + await Promise.resolve() + throw new Error('nope') + }) + const Comp = () => { + // drives loadIfStale from the test; the mount effect only ever fires once, + // so without this the assertion below holds even with no backoff at all + 'use no memo' + const resource = useCachedResource({ + cache, + cacheKey: 'k', + initialData: {v: 0}, + load, + onError: () => {}, + staleMs: 5000, + }) + loadIfStale = resource.loadIfStale + return
{resource.data.v}
+ } + render() + await flush() + expect(calls).toBe(1) + + // inside the backoff window: further attempts must not reach `load` + await act(async () => { + await loadIfStale?.() + await loadIfStale?.() + }) + await flush() + expect(calls).toBe(1) + + // past it: the next attempt goes through + const realNow = Date.now + Date.now = () => realNow() + 5_001 + try { + await act(async () => { + await loadIfStale?.() + }) + await flush() + } finally { + Date.now = realNow + } + expect(calls).toBe(2) +}) + +test('reload bypasses the failure backoff', async () => { + const cache = createCachedResourceCache({v: 0}, 'k') + let calls = 0 + let reload: (() => Promise) | undefined + const load = jest.fn(async () => { + calls++ + await Promise.resolve() + throw new Error('nope') + }) + const Comp = () => { + // hoists reload out to the test body; the compiler rejects the assignment + 'use no memo' + const resource = useCachedResource({ + cache, + cacheKey: 'k', + initialData: {v: 0}, + load, + onError: () => {}, + staleMs: 5000, + }) + reload = resource.reload + return
{resource.data.v}
+ } + render() + await flush() + expect(calls).toBe(1) + await act(async () => { + await reload?.() + }) + expect(calls).toBe(2) +}) + +test('a successful load is served from cache while fresh', async () => { + const cache = createCachedResourceCache({v: 0}, 'k') + let calls = 0 + const load = jest.fn(async () => { + calls++ + await Promise.resolve() + return {v: calls} + }) + const Comp = ({staleMs}: {staleMs: number}) => { + const {data, loaded} = useCachedResource({cache, cacheKey: 'k', initialData: {v: 0}, load, staleMs}) + return
{loaded ? data.v : 'x'}
+ } + const first = render() + await flush() + expect(calls).toBe(1) + + // a re-render of the same fiber proves nothing: the effect deps are stable so + // it never re-runs. The window only matters to a NEW consumer of this cache. + first.unmount() + const second = render() + await flush() + expect(calls).toBe(1) + expect(cache.getData()).toEqual({v: 1}) + + // and a consumer that considers it stale does reload it + second.unmount() + render() + await flush() + expect(calls).toBe(2) +}) + +// A reload() fired because something changed must not settle to data that was +// already on the wire before the change: joining it would serve pre-change data +// AND stamp loadedAt on it, pinning the stale value for the whole window. +test('a forced reload supersedes a request that predates it', async () => { + const cache = createCachedResourceCache({v: 0}, 'k') + let calls = 0 + const releases: Array<(v: Data) => void> = [] + const load = jest.fn(async () => { + calls++ + return await new Promise(resolve => { + releases.push(resolve) + }) + }) + let reload: (() => Promise) | undefined + const Comp = () => { + 'use no memo' + const resource = useCachedResource({cache, cacheKey: 'k', initialData: {v: 0}, load, staleMs: 5000}) + reload = resource.reload + return
{resource.loaded ? `v${resource.data.v}` : 'pending'}
+ } + const view = render() + await flush() + expect(calls).toBe(1) + + // the mutation happens here, while request 1 is still outstanding + let forced: Promise | undefined + act(() => { + forced = reload?.() + }) + await flush() + expect(calls).toBe(2) + + // request 1 lands with pre-change data and must not win + act(() => { + releases[0]?.({v: 1}) + }) + await flush() + act(() => { + releases[1]?.({v: 2}) + }) + await forced + await flush() + expect(view.getAllByText('v2')).toHaveLength(1) + expect(cache.getData()).toEqual({v: 2}) +}) + +// The mutation and the reload it triggers routinely fall inside one millisecond, +// so ordering the two by Date.now() compares them equal and the forced load +// joins the very request it exists to supersede. +test('a forced reload supersedes a same-millisecond request', async () => { + const cache = createCachedResourceCache({v: 0}, 'k') + let calls = 0 + const releases: Array<(v: Data) => void> = [] + const load = jest.fn(async () => { + calls++ + return await new Promise(resolve => { + releases.push(resolve) + }) + }) + let reload: (() => Promise) | undefined + const Comp = () => { + 'use no memo' + const resource = useCachedResource({cache, cacheKey: 'k', initialData: {v: 0}, load, staleMs: 5000}) + reload = resource.reload + return
{resource.loaded ? `v${resource.data.v}` : 'pending'}
+ } + const realNow = Date.now + const frozen = realNow() + Date.now = () => frozen + try { + render() + await flush() + expect(calls).toBe(1) + act(() => { + void reload?.() + }) + await flush() + expect(calls).toBe(2) + } finally { + Date.now = realNow + } +}) + +// Two consumers mounting together must share one request, not race two. This is +// the property the module-level caches depend on: without it, sharing a cache +// across screens still issues an RPC per screen. +test('concurrent consumers of one cache share a single load', async () => { + const cache = createCachedResourceCache({v: 0}, 'k') + let calls = 0 + let release: ((v: Data) => void) | undefined + const load = jest.fn(async () => { + calls++ + return await new Promise(resolve => { + release = resolve + }) + }) + const Comp = () => { + const {data, loaded} = useCachedResource({cache, cacheKey: 'k', initialData: {v: 0}, load, staleMs: 5000}) + return
{loaded ? `v${data.v}` : 'pending'}
+ } + const view = render( + <> + + + + ) + await flush() + expect(calls).toBe(1) + + act(() => { + release?.({v: 7}) + }) + await flush() + expect(calls).toBe(1) + // both consumers got the single load's result + expect(view.getAllByText('v7')).toHaveLength(2) +}) diff --git a/shared/teams/use-cached-resource.tsx b/shared/teams/use-cached-resource.tsx index 4d151998b6f2..9cf630d902ff 100644 --- a/shared/teams/use-cached-resource.tsx +++ b/shared/teams/use-cached-resource.tsx @@ -5,16 +5,34 @@ import {produce} from 'immer' export type CachedResourceCache = { clearInFlight: (request: Promise) => void getData: () => T + getFailedAt: () => number getGeneration: () => number getInFlight: () => Promise | undefined + getInFlightSeq: () => number getKey: () => K getLoadedAt: () => number invalidate: (key: K) => void reset: (data: T, key: K) => void setDataLoaded: (data: T, generation: number) => void setInFlight: (request: Promise) => void + setLoadFailed: (generation: number) => void } +// A load that never lands (it rejected, or its result was discarded because the +// cache was invalidated mid-flight) leaves loadedAt at 0, i.e. permanently +// stale. Without this window every re-run of the effect below would re-issue the +// request the instant the previous one settled - an unbounded retry loop at RPC +// speed. An explicit reload()/invalidate() still retries immediately. +const loadFailureBackoffMs = 5_000 + +// Ordering of "was this request already on the wire when I asked?" cannot use +// Date.now(): a mutation and the reload it triggers routinely land in the same +// millisecond, and the two timestamps then compare equal, so the forced load +// joins the very request it must supersede. A counter is exact. +let inFlightSequence = 0 +const nextInFlightSeq = () => ++inFlightSequence +const currentInFlightSeq = () => inFlightSequence + type CachedResourceState = { data: T loaded: boolean @@ -65,10 +83,28 @@ const storedState = ( initialData, }) +// Settling to already-current data must not produce a new state object: every +// loadIfStale() that finds the cache fresh would otherwise force a re-render, +// which re-runs the effect that called it, which re-renders... a loop paced only +// by how fast React can commit. +const settledState = + (cache: CachedResourceCache, cacheKey: K, initialData: T, data: T) => + (prev: StoredCachedResourceState): StoredCachedResourceState => + prev.cache === cache && + Object.is(prev.cacheKey, cacheKey) && + Object.is(prev.initialData, initialData) && + Object.is(prev.data, data) && + prev.loaded && + !prev.loading + ? prev + : storedState(cache, cacheKey, initialData, {data, loaded: true, loading: false}) + export const createCachedResourceCache = (initialData: T, key: K): CachedResourceCache => { let data = initialData + let failedAt = 0 let generation = 0 let inFlight: Promise | undefined + let inFlightSeq = 0 let loadedAt = 0 let storedKey = key @@ -79,11 +115,14 @@ export const createCachedResourceCache = (initialData: T, key: K): CachedR } }, getData: () => data, + getFailedAt: () => failedAt, getGeneration: () => generation, getInFlight: (): Promise | undefined => inFlight, + getInFlightSeq: () => inFlightSeq, getKey: () => storedKey, getLoadedAt: () => loadedAt, invalidate: nextKey => { + failedAt = 0 generation += 1 inFlight = undefined loadedAt = 0 @@ -91,6 +130,7 @@ export const createCachedResourceCache = (initialData: T, key: K): CachedR }, reset: (nextData, nextKey) => { data = nextData + failedAt = 0 generation += 1 inFlight = undefined loadedAt = 0 @@ -99,11 +139,18 @@ export const createCachedResourceCache = (initialData: T, key: K): CachedR setDataLoaded: (nextData, requestGeneration) => { if (generation === requestGeneration) { data = nextData + failedAt = 0 loadedAt = Date.now() } }, setInFlight: request => { inFlight = request + inFlightSeq = nextInFlightSeq() + }, + setLoadFailed: requestGeneration => { + if (generation === requestGeneration) { + failedAt = Date.now() + } }, } } @@ -130,19 +177,31 @@ const runLoad = async ( onError: ((error: unknown) => void) | undefined, requestVersion: number, requestVersionRef: React.RefObject, - setState: React.Dispatch>> + setState: React.Dispatch>>, + force: boolean, + requestedSeq: number ) => { let request: Promise | undefined + // A forced load exists because something changed, so joining a request that + // was already on the wire before we asked would settle to pre-change data - + // and stamp loadedAt on it, holding it stale for the whole window. Only join a + // request that started after this one was asked for, which still collapses the + // N-mounted-consumers-reload-together case to one RPC. + const staleInFlight = force && !!cache.getInFlight() && cache.getInFlightSeq() <= requestedSeq + if (staleInFlight) { + cache.invalidate(cacheKey) + } + // after the invalidate above, which bumps it + const generation = cache.getGeneration() try { - const inFlight = cache.getInFlight() + const inFlight = staleInFlight ? undefined : cache.getInFlight() if (inFlight) { const data = await inFlight if (requestVersion === requestVersionRef.current) { - setState(storedState(cache, cacheKey, initialData, {data, loaded: true, loading: false})) + setState(settledState(cache, cacheKey, initialData, data)) } return } - const generation = cache.getGeneration() request = load().then(data => { cache.setDataLoaded(data, generation) return data @@ -150,9 +209,12 @@ const runLoad = async ( cache.setInFlight(request) const data = await request if (requestVersion === requestVersionRef.current) { - setState(storedState(cache, cacheKey, initialData, {data, loaded: true, loading: false})) + setState(settledState(cache, cacheKey, initialData, data)) } } catch (error) { + // record the failure even for a superseded request: the backoff belongs to + // the shared cache, not to whichever instance happened to own the request + cache.setLoadFailed(generation) if (requestVersion !== requestVersionRef.current) { return } @@ -177,22 +239,6 @@ export const useCachedResource = (props: Props) => { const hasFocusedSinceMountRef = React.useRef(false) const requestVersionRef = React.useRef(0) - const resetCache = React.useCallback( - (nextKey: K) => { - cache.reset(initialData, nextKey) - }, - [cache, initialData] - ) - - const clear = React.useCallback( - (nextKey: K = cacheKey) => { - requestVersionRef.current += 1 - resetCache(nextKey) - setState(storedState(cache, nextKey, initialData, emptyState(initialData))) - }, - [cache, cacheKey, initialData, resetCache] - ) - const latestRef = React.useRef({ cache, cacheKey, @@ -200,7 +246,6 @@ export const useCachedResource = (props: Props) => { initialData, load, onError, - resetCache, staleMs, }) React.useLayoutEffect(() => { @@ -211,13 +256,37 @@ export const useCachedResource = (props: Props) => { initialData, load, onError, - resetCache, staleMs, } - }, [cache, cacheKey, enabled, initialData, load, onError, resetCache, staleMs]) + }, [cache, cacheKey, enabled, initialData, load, onError, staleMs]) + + // deliberately does not depend on initialData: resetCache is in the main + // effect's dep array, and callers routinely rebuild initialData (seeding it + // from another store), which would re-run the effect on every such change. + const resetCache = React.useCallback( + (nextKey: K) => { + cache.reset(latestRef.current.initialData, nextKey) + }, + [cache] + ) + + const clear = React.useCallback( + (nextKey: K = cacheKey) => { + requestVersionRef.current += 1 + resetCache(nextKey) + setState(storedState(cache, nextKey, initialData, emptyState(initialData))) + }, + [cache, cacheKey, initialData, resetCache] + ) const loadResource = React.useCallback(async (force: boolean) => { - const {cache, cacheKey, enabled, initialData, load, onError, resetCache, staleMs} = latestRef.current + // stamped before any await so a forced load can tell an in-flight request + // that predates it from one issued in response to the same change + const requestedSeq = currentInFlightSeq() + const {cache, cacheKey, enabled, initialData, load, onError, staleMs} = latestRef.current + const resetCache = (nextKey: K) => { + cache.reset(initialData, nextKey) + } if (!Object.is(cache.getKey(), cacheKey)) { requestVersionRef.current += 1 resetCache(cacheKey) @@ -229,18 +298,33 @@ export const useCachedResource = (props: Props) => { } const loadedAt = cache.getLoadedAt() if (!force && loadedAt && Date.now() - loadedAt < staleMs) { - setState( - storedState(cache, cacheKey, initialData, {data: cache.getData(), loaded: true, loading: false}) - ) + setState(settledState(cache, cacheKey, initialData, cache.getData())) + return + } + const failedAt = cache.getFailedAt() + if (!force && failedAt && Date.now() - failedAt < loadFailureBackoffMs) { return } const requestVersion = ++requestVersionRef.current setState(prev => prev.cache === cache && Object.is(prev.cacheKey, cacheKey) && Object.is(prev.initialData, initialData) - ? {...prev, loading: true} + ? prev.loading + ? prev + : {...prev, loading: true} : storedState(cache, cacheKey, initialData, {...emptyState(initialData), loading: true}) ) - await runLoad(cache, cacheKey, initialData, load, onError, requestVersion, requestVersionRef, setState) + await runLoad( + cache, + cacheKey, + initialData, + load, + onError, + requestVersion, + requestVersionRef, + setState, + force, + requestedSeq + ) }, []) const reload = React.useCallback(async () => { diff --git a/shared/teams/use-teams-list.tsx b/shared/teams/use-teams-list.tsx index 71ba3a28b531..4155983e2fcf 100644 --- a/shared/teams/use-teams-list.tsx +++ b/shared/teams/use-teams-list.tsx @@ -15,6 +15,7 @@ import { createCachedResourceCache, useCachedResource, } from './use-cached-resource' +import {registerExternalResetter} from '@/util/zustand' type TeamsList = { reload: () => void @@ -44,6 +45,13 @@ const teamsRoleMapCache = createCachedResourceCache { + teamsListCache.reset(emptyTeams, undefined) + teamsRoleMapCache.reset(emptyTeamRoleMap, undefined) +}) + const teamListToArray = (list: ReadonlyArray) => { return [...Teams.teamListToMeta(list).values()] } diff --git a/shared/tests/e2e/electron/flows/chat-modals.test.ts b/shared/tests/e2e/electron/flows/chat-modals.test.ts index 1b7915a975de..ffc6036df91d 100644 --- a/shared/tests/e2e/electron/flows/chat-modals.test.ts +++ b/shared/tests/e2e/electron/flows/chat-modals.test.ts @@ -79,13 +79,16 @@ test('bot install preview opens', async ({page}, testInfo) => { await page.getByTestId(T.CHAT_INFO_PANEL).getByText('Bots', {exact: true}).click() const botRows = page.getByTestId(T.CHAT_BOT_ROW) await expect(botRows.first()).toBeVisible({timeout: 10_000}) - await botRows.first().click() - // installed bot: Edit settings; new bot: Install; restricted bot: Review - const installButton = page.getByText(/^(Install|Edit settings|Review)$/).first() - await expect(installButton).toBeVisible({timeout: 10_000}) + // the row center sits on the "by " username link, which navigates to + // that user's profile instead — click the avatar column on the left + await botRows.first().click({position: {x: 20, y: 28}}) + // the footer button depends on the bot's state (Install / Review / Edit + // settings / Uninstall), so assert on the modal instead of a label + const installModal = page.getByTestId(T.CHAT_BOT_INSTALL).first() + await expect(installModal).toBeVisible({timeout: 10_000}) await snap(page, testInfo) await page.locator('.icon-gen-iconfont-close:visible').first().click() - await expect(installButton).not.toBeVisible({timeout: 5_000}) + await expect(installModal).not.toBeVisible({timeout: 5_000}) // close the info panel again await page.locator('.icon-gen-iconfont-info:visible').first().click() await expect(page.getByTestId(T.CHAT_INFO_PANEL)).not.toBeVisible({timeout: 5_000}) diff --git a/shared/tests/e2e/electron/flows/device-wallet-modals.test.ts b/shared/tests/e2e/electron/flows/device-wallet-modals.test.ts index dc70fa11a5e7..7874851024b6 100644 --- a/shared/tests/e2e/electron/flows/device-wallet-modals.test.ts +++ b/shared/tests/e2e/electron/flows/device-wallet-modals.test.ts @@ -20,7 +20,8 @@ test('add device chooser opens', async ({page}, testInfo) => { test('crypto recipients team builder opens', async ({page}, testInfo) => { await navigateToCrypto(page) await page.getByTestId(T.CRYPTO_NAV_ENCRYPT).click() - await page.getByPlaceholder('Search people').locator('visible=true').first().click() + // the "Search people" input itself is pointerEvents:none — click its wrapper + await page.getByTestId(T.CRYPTO_RECIPIENTS).locator('visible=true').first().click() const search = page.getByPlaceholder('Search Keybase').locator('visible=true') await expect(search.first()).toBeVisible({timeout: 5_000}) await snap(page, testInfo) diff --git a/shared/tests/e2e/electron/flows/teams-inner.test.ts b/shared/tests/e2e/electron/flows/teams-inner.test.ts index 31f3577fb2ce..ba0b56f5806d 100644 --- a/shared/tests/e2e/electron/flows/teams-inner.test.ts +++ b/shared/tests/e2e/electron/flows/teams-inner.test.ts @@ -23,6 +23,9 @@ test('members tab renders', async ({page}) => { test.skip() return } + // the team page remembers the last tab per team for the life of the app, so an + // earlier test can leave it on Channels/Settings — select Members explicitly + await page.getByTestId(T.TEAMS_TAB_MEMBERS_BUTTON).locator('visible=true').first().click() await expect(page.getByTestId(T.TEAMS_MEMBER_LIST).first()).toBeVisible({timeout: 5_000}) }) @@ -32,10 +35,10 @@ test('settings tab renders', async ({page}) => { test.skip() return } - // 'Settings' appears in both nav sidebar and team tabs — use .nth(1) for team tab - await page.getByText('Settings', {exact: true}).nth(1).click() - // Verify we're still in the team view (Members tab still visible) - await expect(page.getByText('Members', {exact: true}).first()).toBeVisible({timeout: 5_000}) + await page.getByTestId(T.TEAMS_TAB_SETTINGS_BUTTON).locator('visible=true').first().click() + // assert the settings BODY, not the 'Members' tab label: that label is part of + // the tab bar and is visible on every tab, so it passes without switching + await expect(page.getByTestId(T.TEAMS_SETTINGS_TAB).first()).toBeVisible({timeout: 5_000}) }) test('bots tab renders', async ({page}) => { @@ -45,7 +48,7 @@ test('bots tab renders', async ({page}) => { return } await page.getByText('Bots', {exact: true}).first().click() - await expect(page.getByText('Members', {exact: true}).first()).toBeVisible({timeout: 5_000}) + await expect(page.getByTestId(T.TEAMS_BOTS_TAB).first()).toBeVisible({timeout: 5_000}) }) test('channels tab renders (if big team or admin)', async ({page}) => { @@ -60,5 +63,5 @@ test('channels tab renders (if big team or admin)', async ({page}) => { return } await channelsTab.click() - await expect(page.getByText('Members', {exact: true}).first()).toBeVisible({timeout: 5_000}) + await expect(page.getByTestId(T.TEAMS_CHANNEL_LIST).first()).toBeVisible({timeout: 5_000}) }) diff --git a/shared/tests/e2e/electron/flows/teams-modals.test.ts b/shared/tests/e2e/electron/flows/teams-modals.test.ts index 313eba84a61e..fd69f12bd6ae 100644 --- a/shared/tests/e2e/electron/flows/teams-modals.test.ts +++ b/shared/tests/e2e/electron/flows/teams-modals.test.ts @@ -130,8 +130,19 @@ test('retention warning opens', async ({page}, testInfo) => { test.skip() return } - await dropdown.click() - await page.getByText('7 days', {exact: true}).locator('visible=true').last().click() + // the settings tab re-renders as the team's retention policy lands, which can + // swallow the popup the first click opens — reopen until the menu sticks + const sevenDays = page.getByText('7 days', {exact: true}).locator('visible=true').last() + let menuOpen = false + for (let i = 0; i < 3 && !menuOpen; i++) { + await dropdown.click() + menuOpen = await becomesVisible(sevenDays, 2_000) + } + if (!menuOpen) { + test.skip() + return + } + await sevenDays.click() const confirm = page.getByText('Yes, set to 7 days') if (!(await becomesVisible(confirm, 3_000))) { // no warning fired (already at/below 7 days) — nothing to capture diff --git a/shared/tests/e2e/electron/playwright.config.ts b/shared/tests/e2e/electron/playwright.config.ts index d360d19b20cf..4413da4ccd21 100644 --- a/shared/tests/e2e/electron/playwright.config.ts +++ b/shared/tests/e2e/electron/playwright.config.ts @@ -2,7 +2,9 @@ import {defineConfig} from '@playwright/test' export default defineConfig({ testDir: './', - timeout: 15_000, + // several flows chain 3-4 five-second waits against a live service, so the + // per-test budget has to clear the sum of their step timeouts + timeout: 30_000, retries: 1, workers: 1, outputDir: '../../results/test-results', diff --git a/shared/tests/e2e/shared/test-ids.ts b/shared/tests/e2e/shared/test-ids.ts index 9b4f455421a5..c758337602cd 100644 --- a/shared/tests/e2e/shared/test-ids.ts +++ b/shared/tests/e2e/shared/test-ids.ts @@ -22,6 +22,9 @@ export const CHAT_EMOJI_PICKER = 'chat-emoji-picker' export const CHAT_ATTACHMENT_IMAGE = 'chat-attachment-image' export const CHAT_ATTACHMENT_FULLSCREEN = 'chat-attachment-fullscreen' export const CHAT_BOT_ROW = 'chat-bot-row' +// the install modal's footer button varies with the bot's state (Install / +// Review / Edit settings / Uninstall), so tests key off the modal itself +export const CHAT_BOT_INSTALL = 'chat-bot-install' export const CHAT_SUGGESTION_LIST = 'chat-suggestion-list' export const CHAT_EMOJI_BUTTON = 'chat-emoji-button' export const CHAT_INFO_PANEL_SETTINGS_TAB = 'chat-info-panel-settings-tab' @@ -95,6 +98,9 @@ export const CRYPTO_DECRYPT_INPUT = 'crypto-decrypt-input' export const CRYPTO_SIGN_INPUT = 'crypto-sign-input' export const CRYPTO_VERIFY_INPUT = 'crypto-verify-input' export const CRYPTO_RUN_BUTTON = 'crypto-run-button' +// The recipients field is a display-only input inside a pointerEvents="none" +// wrapper, so only this outer clickable can receive a click. +export const CRYPTO_RECIPIENTS = 'crypto-recipients' // Common — keep value matching existing testID="backButton" in .maestro subflows export const COMMON_BACK_BUTTON = 'backButton' diff --git a/shared/tracker/identify-session.tsx b/shared/tracker/identify-session.tsx new file mode 100644 index 000000000000..9711cac44c59 --- /dev/null +++ b/shared/tracker/identify-session.tsx @@ -0,0 +1,409 @@ +import * as C from '@/constants' +import * as T from '@/constants/types' +import logger from '@/logger' +import {RPCError} from '@/util/errors' +import {generateGUIID, ignorePromise} from '@/constants/utils' +import {navigateAppend, navigateUp} from '@/constants/router' +import {produce} from 'immer' +import {registerExternalResetter} from '@/util/zustand' +import {subscribeToEngineAction} from '@/engine/action-listener' +import {useCurrentUserState} from '@/stores/current-user' +import {useUsersState} from '@/stores/users' +import { + identifyResultToDetailsState, + makeDetails, + noNonUserDetails, + updateTrackerDetailsBlocked, + updateTrackerDetailsReset, + updateTrackerDetailsResult, + updateTrackerDetailsRow, + updateTrackerDetailsSummary, + updateTrackerDetailsUserCard, +} from './model' + +// One identify per user, shared by every surface showing that user. An +// Identify3 with ignoreCache re-fetches every proof from the third party host, +// so N surfaces each running their own identify is N times the load on those +// hosts (and the service serializes them per uid, so they also run back to +// back). Instead a single session per username owns the identify, accumulates +// every identify3Ui event for it, and fans the result out to its subscribers - +// including ones that join after the identify already started. +type Session = { + details: T.Tracker.Details + generation: number + ignoreCache: boolean + inFlight: boolean + nonUserDetails: T.Tracker.NonUserDetails + startedAt: number + username: string +} + +export type IdentifyLoadOptions = { + // Join an identify that is already running only if it started at or after + // this timestamp, mirroring the singleflight in go/libkb/proof_cache.go. + // 0: any in-flight identify is good enough (mount, focus). + // Date.now(): only one started in reaction to the same event (notifications). + // Infinity: never join, always a fresh identify (explicit user reload). + freshAfter: number + ignoreCache: boolean + // Skip entirely if an identify at least this strong finished less than this + // long ago. Only the mount path sets it: reopening a profile you just looked + // at should not re-run every proof check. An explicit reload leaves it unset. + maxAgeMs?: number +} + +const sessions = new Map() +// Survives the session being dropped when it goes idle: closing a profile and +// reopening it should still count as "we just checked this user". +const lastCompleted = new Map() +// Kept outside of Session so a subscriber stays attached to its username even +// if the session object behind it is replaced. +const subscribersByUsername = new Map void>>() + +const notify = (username: string) => { + const subscribers = subscribersByUsername.get(username) + if (!subscribers?.size) { + return + } + for (const cb of [...subscribers]) { + cb() + } +} + +const setDetails = (s: Session, next: T.Tracker.Details) => { + if (next === s.details) { + return + } + s.details = next + notify(s.username) +} + +const makeSession = (username: string): Session => ({ + details: makeDetails(username), + generation: 0, + ignoreCache: false, + inFlight: false, + nonUserDetails: noNonUserDetails, + startedAt: 0, + username, +}) + +const ensureSession = (username: string) => { + const existing = sessions.get(username) + if (existing) { + return existing + } + const created = makeSession(username) + sessions.set(username, created) + return created +} + +const dropSessionIfIdle = (s: Session) => { + if (s.inFlight || subscribersByUsername.get(s.username)?.size) { + return + } + if (sessions.get(s.username) === s) { + sessions.delete(s.username) + } +} + +const sessionForGuiID = (guiID: string) => { + for (const s of sessions.values()) { + if (s.details.guiID === guiID) { + return s + } + } + return undefined +} + +const loadNonUserDetails = async (s: Session, generation: number) => { + const assertion = s.username + try { + const res = await T.RPCGen.userSearchGetNonUserDetailsRpcPromise({assertion}) + if (s.generation !== generation || !res.isNonUser) { + return + } + const common = { + assertionKey: res.assertionKey, + assertionValue: res.assertionValue, + description: res.description, + siteIcon: res.siteIcon || [], + siteIconDarkmode: res.siteIconDarkmode || [], + siteIconFull: res.siteIconFull || [], + siteIconFullDarkmode: res.siteIconFullDarkmode || [], + siteURL: '', + } + if (res.service) { + s.nonUserDetails = {...noNonUserDetails, ...common, ...res.service} + notify(s.username) + } else { + const {formatPhoneNumberInternational} = await import('@/util/phone-numbers') + const formattedName = + res.assertionKey === 'phone' ? formatPhoneNumberInternational('+' + res.assertionValue) : undefined + const fullName = res.contact?.contactName ?? '' + if (s.generation !== generation) { + return + } + s.nonUserDetails = {...noNonUserDetails, ...common, formattedName, fullName} + notify(s.username) + } + } catch (error) { + if (error instanceof RPCError) { + logger.warn(`Error loading non user profile: ${error.message}`) + } + } finally { + dropSessionIfIdle(s) + } +} + +export const loadNonUserProfile = (username: string) => { + if (!username) { + return + } + const s = ensureSession(username) + ignorePromise(loadNonUserDetails(s, s.generation)) +} + +const runIdentify = async (s: Session, generation: number, guiID: string, ignoreCache: boolean) => { + try { + await T.RPCGen.identify3Identify3RpcListener({ + incomingCallMap: {}, + params: {assertion: s.username, guiID, ignoreCache}, + waitingKey: C.waitingKeyTrackerProfileLoad, + }) + } catch (error) { + if (!(error instanceof RPCError) || s.generation !== generation) { + return + } + if (error.code === T.RPCGen.StatusCode.scresolutionfailed) { + setDetails( + s, + produce(s.details, draft => { + draft.state = 'notAUserYet' + }) + ) + loadNonUserProfile(s.username) + } else if (error.code === T.RPCGen.StatusCode.scnotfound) { + navigateUp() + navigateAppend({ + name: 'keybaseLinkError', + params: { + error: `You followed a profile link for a user (${s.username}) that does not exist.`, + }, + }) + } + logger.error(`Error loading profile: ${error.message}`) + } finally { + if (s.generation === generation) { + s.inFlight = false + lastCompleted.set(s.username, {at: Date.now(), ignoreCache: s.ignoreCache}) + dropSessionIfIdle(s) + } + } +} + +const loadFollowers = async (s: Session, generation: number) => { + try { + const fs = await T.RPCGen.userListTrackersUnverifiedRpcPromise( + {assertion: s.username}, + C.waitingKeyTrackerProfileLoad + ) + if (s.generation !== generation) { + return + } + setDetails( + s, + produce(s.details, draft => { + draft.followers = new Set((fs.users ?? []).map(f => f.username)) + draft.followersCount = fs.users?.length ?? 0 + }) + ) + if (fs.users) { + useUsersState + .getState() + .dispatch.updates(fs.users.map(u => ({info: {fullname: u.fullName}, name: u.username}))) + } + } catch (error) { + if (error instanceof RPCError) { + logger.error(`Error loading follower info: ${error.message}`) + } + } +} + +const loadFollowing = async (s: Session, generation: number) => { + try { + const fs = await T.RPCGen.userListTrackingRpcPromise( + {assertion: s.username, filter: ''}, + C.waitingKeyTrackerProfileLoad + ) + if (s.generation !== generation) { + return + } + setDetails( + s, + produce(s.details, draft => { + draft.following = new Set((fs.users ?? []).map(f => f.username)) + draft.followingCount = fs.users?.length ?? 0 + }) + ) + if (fs.users) { + useUsersState + .getState() + .dispatch.updates(fs.users.map(u => ({info: {fullname: u.fullName}, name: u.username}))) + } + } catch (error) { + if (error instanceof RPCError) { + logger.error(`Error loading following info: ${error.message}`) + } + } +} + +export const loadProfileIdentify = (username: string, options: IdentifyLoadOptions) => { + if (!username) { + return + } + ensureEngineSubscriptions() + const {freshAfter, ignoreCache} = options + const s = ensureSession(username) + // A weaker in-flight identify (one that was allowed to use the proof cache) + // cannot stand in for a caller that asked for a forced remote check. + const strongEnough = s.ignoreCache || !ignoreCache + if (s.inFlight && strongEnough && s.startedAt >= freshAfter) { + return + } + // A recent finished identify counts too, so long as it was at least as strong + // as what is being asked for now. + const {maxAgeMs} = options + const done = lastCompleted.get(username) + if (maxAgeMs !== undefined && done && (done.ignoreCache || !ignoreCache) && Date.now() - done.at < maxAgeMs) { + return + } + + const guiID = generateGUIID() + const generation = ++s.generation + s.ignoreCache = ignoreCache + s.inFlight = true + s.startedAt = Date.now() + setDetails( + s, + produce(s.details, draft => { + draft.guiID = guiID + if (!draft.resetBrokeTrack) { + draft.reason = '' + } + draft.state = 'checking' + }) + ) + ignorePromise(runIdentify(s, generation, guiID, ignoreCache)) + ignorePromise(loadFollowers(s, generation)) + ignorePromise(loadFollowing(s, generation)) +} + +// Registered once for the lifetime of the module. The unsubscribes are dropped +// on purpose; sign out clears the engine listener registry wholesale and the +// flag below lets them be re-registered on the next load. +let engineSubscribed = false +const ensureEngineSubscriptions = () => { + if (engineSubscribed) { + return + } + engineSubscribed = true + + subscribeToEngineAction('keybase.1.identify3Ui.identify3Result', action => { + const {guiID, result} = action.payload.params + const s = sessionForGuiID(guiID) + if (!s) { + return + } + setDetails(s, updateTrackerDetailsResult(s.details, identifyResultToDetailsState(result))) + }) + + subscribeToEngineAction('keybase.1.identify3Ui.identify3UpdateRow', action => { + const {row} = action.payload.params + const s = sessionForGuiID(row.guiID) + if (!s) { + return + } + setDetails(s, updateTrackerDetailsRow(s.details, row)) + }) + + subscribeToEngineAction('keybase.1.identify3Ui.identify3UserReset', action => { + const s = sessionForGuiID(action.payload.params.guiID) + if (!s) { + return + } + setDetails(s, updateTrackerDetailsReset(s.details)) + }) + + subscribeToEngineAction('keybase.1.identify3Ui.identify3UpdateUserCard', action => { + const {guiID, card} = action.payload.params + const s = sessionForGuiID(guiID) + if (!s) { + return + } + setDetails(s, updateTrackerDetailsUserCard(s.details, card)) + useUsersState.getState().dispatch.updates([{info: {fullname: card.fullName}, name: s.username}]) + }) + + subscribeToEngineAction('keybase.1.identify3Ui.identify3Summary', action => { + const {summary} = action.payload.params + const s = sessionForGuiID(summary.guiID) + if (!s) { + return + } + setDetails(s, updateTrackerDetailsSummary(s.details, summary)) + }) + + subscribeToEngineAction('keybase.1.NotifyTracking.notifyUserBlocked', action => { + for (const s of [...sessions.values()]) { + setDetails(s, updateTrackerDetailsBlocked(s.details, action.payload.params.b)) + } + }) + + subscribeToEngineAction('keybase.1.NotifyTracking.trackingChanged', action => { + const s = sessions.get(action.payload.params.username) + if (!s?.details.guiID) { + return + } + lastCompleted.delete(s.username) + loadProfileIdentify(s.username, {freshAfter: Date.now(), ignoreCache: true}) + }) + + subscribeToEngineAction('keybase.1.NotifyUsers.userChanged', action => { + const {uid, username} = useCurrentUserState.getState() + if (!username || uid !== action.payload.params.uid || !sessions.has(username)) { + return + } + lastCompleted.delete(username) + loadProfileIdentify(username, {freshAfter: Date.now(), ignoreCache: false}) + }) +} + +export const subscribeToProfile = (username: string, cb: () => void) => { + ensureSession(username) + let subscribers = subscribersByUsername.get(username) + if (!subscribers) { + subscribers = new Set() + subscribersByUsername.set(username, subscribers) + } + subscribers.add(cb) + return () => { + subscribers.delete(cb) + if (!subscribers.size) { + subscribersByUsername.delete(username) + } + const s = sessions.get(username) + if (s) { + dropSessionIfIdle(s) + } + } +} + +export const getProfileDetails = (username: string) => sessions.get(username)?.details +export const getProfileNonUserDetails = (username: string) => sessions.get(username)?.nonUserDetails + +registerExternalResetter('tracker-identify-sessions', () => { + sessions.clear() + lastCompleted.clear() + engineSubscribed = false +}) diff --git a/shared/tracker/use-profile.tsx b/shared/tracker/use-profile.tsx index b78f06f8f539..05e673efcce0 100644 --- a/shared/tracker/use-profile.tsx +++ b/shared/tracker/use-profile.tsx @@ -1,304 +1,75 @@ -import * as C from '@/constants' import * as React from 'react' -import * as T from '@/constants/types' -import {generateGUIID, ignorePromise} from '@/constants/utils' -import {useCurrentUserState} from '@/stores/current-user' -import {useUsersState} from '@/stores/users' -import {useEngineActionListener} from '@/engine/action-listener' -import {produce} from 'immer' -import logger from '@/logger' -import {RPCError} from '@/util/errors' -import {navigateAppend, navigateUp} from '@/constants/router' +import {makeDetails, noNonUserDetails} from './model' import { - identifyResultToDetailsState, - makeDetails, - noNonUserDetails, - updateTrackerDetailsBlocked, - updateTrackerDetailsReset, - updateTrackerDetailsResult, - updateTrackerDetailsRow, - updateTrackerDetailsSummary, - updateTrackerDetailsUserCard, -} from './model' + getProfileDetails, + getProfileNonUserDetails, + loadNonUserProfile, + loadProfileIdentify, + subscribeToProfile, +} from './identify-session' + +// Reopening a profile you were just looking at re-runs every proof check, and +// each one is an outbound request to a third-party host. Opening it again within +// this window reuses the check that just ran; an explicit reload always forces. +const profileRecheckMs = 30_000 type Options = { - reloadOnFocus?: boolean -} - -const loadNonUserDetails = async ( - username: string, - version: number, - requestVersionRef: React.RefObject, - setNonUserDetails: React.Dispatch< - React.SetStateAction<{details: T.Tracker.NonUserDetails; username: string}> - > -) => { - const assertion = username - try { - const res = await T.RPCGen.userSearchGetNonUserDetailsRpcPromise({assertion}) - if (requestVersionRef.current !== version || !res.isNonUser) { - return - } - const common = { - assertionKey: res.assertionKey, - assertionValue: res.assertionValue, - description: res.description, - siteIcon: res.siteIcon || [], - siteIconDarkmode: res.siteIconDarkmode || [], - siteIconFull: res.siteIconFull || [], - siteIconFullDarkmode: res.siteIconFullDarkmode || [], - siteURL: '', - } - if (res.service) { - setNonUserDetails({details: {...noNonUserDetails, ...common, ...res.service}, username}) - } else { - const {formatPhoneNumberInternational} = await import('@/util/phone-numbers') - const formattedName = - res.assertionKey === 'phone' ? formatPhoneNumberInternational('+' + res.assertionValue) : undefined - const fullName = res.contact?.contactName ?? '' - if (requestVersionRef.current !== version) { - return - } - setNonUserDetails({ - details: {...noNonUserDetails, ...common, formattedName, fullName}, - username, - }) - } - } catch (error) { - if (error instanceof RPCError) { - logger.warn(`Error loading non user profile: ${error.message}`) - } - } + // surfaces that only want loadProfile() to call after an action, and never + // read details, can skip the identify their mount would otherwise trigger + loadOnMount?: boolean + // Opening a profile means "check this identity now", so it forces a remote + // check of every proof. Incidental surfaces - a hover card, a follow button in + // a list - do not: they still need an identify session, but the cached proof + // results answer them, and forcing one re-fetches every proof from its + // third-party host, which those hosts rate limit. + cachedOnMount?: boolean } export const useTrackerProfile = (username: string, options?: Options) => { - const currentUser = useCurrentUserState( - C.useShallow(s => ({ - uid: s.uid, - username: s.username, - })) - ) - const [details, setDetails] = React.useState(() => makeDetails(username)) - const [nonUserDetails, setNonUserDetails] = React.useState<{ - details: T.Tracker.NonUserDetails - username: string - }>(() => ({ - details: {...noNonUserDetails}, - username, - })) - const requestVersionRef = React.useRef(0) - const detailsRef = React.useRef(details) - const hasSeenFocusRef = React.useRef(false) + const emptyDetails = React.useMemo(() => makeDetails(username), [username]) - React.useEffect(() => { - detailsRef.current = details - }, [details]) + const subscribe = React.useCallback((cb: () => void) => subscribeToProfile(username, cb), [username]) + const getDetails = React.useCallback(() => getProfileDetails(username) ?? emptyDetails, [ + emptyDetails, + username, + ]) + const getNonUserDetails = React.useCallback( + () => getProfileNonUserDetails(username) ?? noNonUserDetails, + [username] + ) + const details = React.useSyncExternalStore(subscribe, getDetails) + const nonUserDetails = React.useSyncExternalStore(subscribe, getNonUserDetails) - const loadNonUserProfile = React.useCallback(() => { - if (!username) { - return - } - ignorePromise( - loadNonUserDetails(username, requestVersionRef.current, requestVersionRef, setNonUserDetails) - ) + const loadNonUser = React.useCallback(() => { + loadNonUserProfile(username) }, [username]) + // Every caller of this is a deliberate user action (opening reload, or a + // refresh after follow / profile edit), so it never joins an identify that + // was already running. const loadProfile = React.useCallback( (ignoreCache = true) => { - if (!username) { - return - } - const guiID = generateGUIID() - const version = requestVersionRef.current + 1 - requestVersionRef.current = version - const preserveExistingData = detailsRef.current.username === username - - setDetails(prev => - produce(preserveExistingData ? prev : makeDetails(username), draft => { - draft.guiID = guiID - if (!draft.resetBrokeTrack) { - draft.reason = '' - } - draft.state = 'checking' - }) - ) - - const load = async () => { - try { - await T.RPCGen.identify3Identify3RpcListener({ - incomingCallMap: {}, - params: {assertion: username, guiID, ignoreCache}, - waitingKey: C.waitingKeyTrackerProfileLoad, - }) - } catch (error) { - if (!(error instanceof RPCError) || requestVersionRef.current !== version) { - return - } - if (error.code === T.RPCGen.StatusCode.scresolutionfailed) { - setDetails( - produce(draft => { - draft.state = 'notAUserYet' - }) - ) - loadNonUserProfile() - } else if (error.code === T.RPCGen.StatusCode.scnotfound) { - navigateUp() - navigateAppend({ - name: 'keybaseLinkError', - params: { - error: `You followed a profile link for a user (${username}) that does not exist.`, - }, - }) - } - logger.error(`Error loading profile: ${error.message}`) - } - } - ignorePromise(load()) - - const loadFollowers = async () => { - try { - const fs = await T.RPCGen.userListTrackersUnverifiedRpcPromise( - {assertion: username}, - C.waitingKeyTrackerProfileLoad - ) - if (requestVersionRef.current !== version) { - return - } - setDetails( - produce(draft => { - draft.followers = new Set((fs.users ?? []).map(f => f.username)) - draft.followersCount = fs.users?.length ?? 0 - }) - ) - if (fs.users) { - useUsersState - .getState() - .dispatch.updates(fs.users.map(u => ({info: {fullname: u.fullName}, name: u.username}))) - } - } catch (error) { - if (error instanceof RPCError) { - logger.error(`Error loading follower info: ${error.message}`) - } - } - } - ignorePromise(loadFollowers()) - - const loadFollowing = async () => { - try { - const fs = await T.RPCGen.userListTrackingRpcPromise( - {assertion: username, filter: ''}, - C.waitingKeyTrackerProfileLoad - ) - if (requestVersionRef.current !== version) { - return - } - setDetails( - produce(draft => { - draft.following = new Set((fs.users ?? []).map(f => f.username)) - draft.followingCount = fs.users?.length ?? 0 - }) - ) - if (fs.users) { - useUsersState - .getState() - .dispatch.updates(fs.users.map(u => ({info: {fullname: u.fullName}, name: u.username}))) - } - } catch (error) { - if (error instanceof RPCError) { - logger.error(`Error loading following info: ${error.message}`) - } - } - } - ignorePromise(loadFollowing()) + loadProfileIdentify(username, {freshAfter: Infinity, ignoreCache}) }, - [loadNonUserProfile, username] + [username] ) + const loadOnMount = options?.loadOnMount ?? true + const cachedOnMount = options?.cachedOnMount ?? false React.useEffect(() => { - if (username) { - loadProfile() - } - }, [loadProfile, username]) - - C.Router2.useSafeFocusEffect( - React.useCallback(() => { - if (!options?.reloadOnFocus) { - return - } - // The initial mount path already loads once. Skip the first focus callback - // so entering the screen does not immediately trigger a second hard reload. - if (!hasSeenFocusRef.current) { - hasSeenFocusRef.current = true - return - } - loadProfile(false) - }, [loadProfile, options?.reloadOnFocus]) - ) - - useEngineActionListener('keybase.1.NotifyTracking.trackingChanged', action => { - if (action.payload.params.username === username && detailsRef.current.guiID) { - loadProfile() - } - }) - - useEngineActionListener('keybase.1.identify3Ui.identify3Result', action => { - const {guiID, result} = action.payload.params - if (guiID !== detailsRef.current.guiID) { - return - } - setDetails(prev => updateTrackerDetailsResult(prev, identifyResultToDetailsState(result))) - }) - - useEngineActionListener('keybase.1.NotifyUsers.userChanged', action => { - if (currentUser.username === username && currentUser.uid === action.payload.params.uid) { - loadProfile(false) - } - }) - - useEngineActionListener('keybase.1.NotifyTracking.notifyUserBlocked', action => { - setDetails(prev => updateTrackerDetailsBlocked(prev, action.payload.params.b)) - }) - - useEngineActionListener('keybase.1.identify3Ui.identify3UpdateRow', action => { - const {row} = action.payload.params - if (row.guiID !== detailsRef.current.guiID) { - return - } - setDetails(prev => updateTrackerDetailsRow(prev, row)) - }) - - useEngineActionListener('keybase.1.identify3Ui.identify3UserReset', action => { - if (action.payload.params.guiID !== detailsRef.current.guiID) { - return - } - setDetails(prev => updateTrackerDetailsReset(prev)) - }) - - useEngineActionListener('keybase.1.identify3Ui.identify3UpdateUserCard', action => { - const {guiID, card} = action.payload.params - if (guiID !== detailsRef.current.guiID) { - return - } - setDetails(prev => updateTrackerDetailsUserCard(prev, card)) - useUsersState.getState().dispatch.updates([{info: {fullname: card.fullName}, name: username}]) - }) - - useEngineActionListener('keybase.1.identify3Ui.identify3Summary', action => { - const {summary} = action.payload.params - if (summary.guiID !== detailsRef.current.guiID) { - return + if (loadOnMount) { + loadProfileIdentify(username, { + freshAfter: 0, + ignoreCache: !cachedOnMount, + maxAgeMs: profileRecheckMs, + }) } - setDetails(prev => updateTrackerDetailsSummary(prev, summary)) - }) - - const detailsForUsername = details.username === username ? details : makeDetails(username) - const nonUserDetailsForUsername = - nonUserDetails.username === username ? nonUserDetails.details : {...noNonUserDetails} + }, [cachedOnMount, loadOnMount, username]) return { - details: detailsForUsername, - loadNonUserProfile, + details, + loadNonUserProfile: loadNonUser, loadProfile, - nonUserDetails: nonUserDetailsForUsername, + nonUserDetails, } } diff --git a/shared/util/fs-platform.tsx b/shared/util/fs-platform.tsx index 3d7d90baba58..ec5479e3462f 100644 --- a/shared/util/fs-platform.tsx +++ b/shared/util/fs-platform.tsx @@ -11,6 +11,7 @@ import {launchImageLibraryAsync} from '@/util/expo-image-picker' import {pickDocumentsAsync} from '@/util/expo-document-picker.native' import {saveAttachmentToCameraRoll, showShareActionSheet} from '@/util/platform-specific' import {fsCacheDir, fsDownloadDir, androidAddCompleteDownload} from 'react-native-kb' +import {registerExternalResetter} from '@/util/zustand' // Desktop-only exports (stubs on mobile) export const fuseStatusToDriverStatus = (status?: T.RPCGen.FuseStatus): T.FS.DriverStatus => { @@ -301,6 +302,11 @@ export const afterKbfsDaemonRpcStatusChangedMobile = async () => { const finishedRegularDownloadIDs = new Set() +// module scope outlives sign-out; holds the previous user's download ids +registerExternalResetter('fs-finished-regular-downloads', () => { + finishedRegularDownloadIDs.clear() +}) + export const finishedRegularDownloadMobile = async ( downloadID: string, downloadState: T.FS.DownloadState, diff --git a/shared/util/use-local-badging.tsx b/shared/util/use-local-badging.tsx index 31947c08304a..35febbf7e35f 100644 --- a/shared/util/use-local-badging.tsx +++ b/shared/util/use-local-badging.tsx @@ -26,12 +26,16 @@ export const useLocalBadging = (storeSet: ReadonlySet | undefined, clear }) } - C.Router2.useSafeFocusEffect(() => { - clearStoreBadges() - return () => { - setBadged(noBadges) - } - }) + // clearStoreBadges must be stable in the caller: an unstable identity re-runs + // this focus effect every render, clearing the badges on the server repeatedly + C.Router2.useSafeFocusEffect( + React.useCallback(() => { + clearStoreBadges() + return () => { + setBadged(noBadges) + } + }, [clearStoreBadges]) + ) return {badged} } diff --git a/shared/wallets/index.tsx b/shared/wallets/index.tsx index 01e9dc11d766..7f84beabc539 100644 --- a/shared/wallets/index.tsx +++ b/shared/wallets/index.tsx @@ -118,7 +118,7 @@ const WalletsScreen = () => { const loadAccounts = C.useRPC(T.RPCStellar.localGetWalletAccountsLocalRpcPromise) C.Router2.useSafeFocusEffect( - () => { + React.useCallback(() => { loadAccounts( [undefined, loadAccountsWaitingKey], res => { @@ -138,7 +138,7 @@ const WalletsScreen = () => { } ) return () => {} - } + }, [loadAccounts, checkDisclaimer]) ) const loading = C.Waiting.useAnyWaiting(loadAccountsWaitingKey) diff --git a/skill/keybase-rpc-log-analysis/SKILL.md b/skill/keybase-rpc-log-analysis/SKILL.md new file mode 100644 index 000000000000..6b1864801fbf --- /dev/null +++ b/skill/keybase-rpc-log-analysis/SKILL.md @@ -0,0 +1,145 @@ +--- +name: keybase-rpc-log-analysis +description: Use when hunting redundant, duplicated or looping RPCs in the Keybase client — after a rate-limit error, a service log that rotates every few minutes, a slow or hot app, or to prove a caching fix actually reduced calls. Covers capturing a clean service log against a locally built service and reading it. +--- + +# Keybase RPC log analysis + +The Go service logs every RPC it serves and every call it makes. That log is the +only place the client's real network behaviour is visible — the JS side shows +intent, the log shows what actually went out. A quiet minute is ~45 lines; a +minute with a flood is 120,000. + +## Capture a clean run + +```bash +skill/keybase-rpc-log-analysis/scripts/clean-logs.sh --archive # dry run without a flag +skill/keybase-rpc-log-analysis/scripts/start-service.sh # own terminal, foreground +cd shared && yarn desktop:start:hot:e2e # another terminal +cd shared && yarn test:e2e:desktop # or drive the app by hand +``` + +`start-service.sh` builds from this checkout, stops the running service, and logs +with `-d --log-file` to `/tmp/kb-analysis/service.log`. Its own file matters: the +default log is shared with everything else and a flood rotates the evidence away +in under two minutes. + +`--log-file` **still rotates at 128MB** — a full e2e run produces ~220MB. Nothing +is lost, but the analysis must include the siblings, so always pass them all: + +```bash +python3 skill/keybase-rpc-log-analysis/scripts/rpc-report.py \ + /tmp/kb-analysis/service.log-* /tmp/kb-analysis/service.log +``` + +The service forks kbfs from its own bin directory, and a plain `go install` of +the service does not build it. Without kbfs the Files tab is empty and git +fails with `KBFS client not found`, which fails every `files-*` and `git-*` e2e +test for reasons unrelated to the change under test. `start-service.sh` warns. + +Ask before stopping the service or launching the app — both are the user's. + +## Read it + +| Question | Command | +|---|---| +| What did this run do? | `rpc-report.py [--start 2026-07-24T17:23] [--end ...]` | +| What was actually slow? | `rpc-cost.py [--min-mean-ms 50]` | +| Why is X called so much? | `rpc-why.py --match 'AttachmentHTTPSrv: GetURL'` | +| Did my fix work? | `rpc-diff.py before.log after.log` | + +**Run `rpc-cost.py` early.** Count and cost answer different questions, and the +loudest line in the log is usually not the expensive one. In one capture the top +line by count was an in-memory map lookup — 15,767 calls for 0.16s total, not +worth touching — while the real cost was 469 team loads at 292ms each, 186 +seconds, sitting 30th by count. + +To *prove* a loud flood is free rather than just absent from the table, pass +`--min-mean-ms 0 --top 400`: the default threshold hides cheap chatty ops, which +is exactly the row you need. `AttachmentHTTPSrv: GetURL` at 4,848 calls does not +appear at all by default, and shows as 0.04s total once it does. + +Analysing an already-rotated log is the same — pass +`~/Library/Logs/keybase.service.log--` directly. The filename carries +its own window, so `--start/--end` are only for narrowing further. + +Read `rpc-report.py`'s sections in order: + +- **REMOTE** — service→server. Only these burn server rate limits. Start here. +- **APP** — app→service. One of these usually explains a REMOTE row. +- **BURSTS** — same call and subject, 3+ times in one second. Each row is a + missing cache or a remounting component. +- **FLOODS** — most repeated lines, ids and numbers collapsed. + +A BURSTS row with a bare name and no `(subject)` means the subject could not be +resolved: many RPCs log no arguments. Those rows are same-*name* only — treat +them as a lead, not as proof the same conversation was hit twice. + +`rpc-why.py` answers attribution. **ROOT** (the app RPC that opened the trace) is +usually the answer; **DRIVERS** (nearest preceding call) points at the specific +line. **BATCHES** needs the span that logs the item count, which is normally the +local one, not the remote it wraps — match `HybridConversationSource: GetMessages`, +not `getMessagesRemote`. The script says so when it finds no counts. + +## What a bug looks like + +- **Same args, same second, N times** → no dedupe, or a keyed component remounting + as its props arrive in stages. +- **Fires again the instant the previous lands** → a feedback loop, not user + action. Look for a cache that never records success, or an effect whose deps + change on every render. +- **Batch sizes almost all 1** → the caller loops where it could fetch once. +- **A per-item cost inside a loop** → e.g. a gregor state read per emoji. + +## Mistakes + +- **Guessing the trigger from neighbouring lines.** A burst surrounded by + startup-looking work is not necessarily startup. Correlate against something + independent: the mtimes of `shared/tests/results/test-results/*/test-finished-1.png` + give you a timestamped list of which test ran when, and `app.log` records + renderer reloads. One burst-per-profile-open and one burst-per-bootstrap look + identical in the service log until you check. +- **Attributing to the child call.** The line directly above a flood is often + something the flood itself invoked. Trust ROOT over DRIVERS. +- **Reading a fix as failed because the totals did not move.** If the app's + request volume changed between runs, a working cache can show flat server + counts. Count what the cache actually did — a hit rate, a log line — before + concluding anything. One cache here looked useless in one capture and turned + out to be a 79% reduction once the volume feeding it was fixed. +- **Fixing what you have not measured.** Two of the loudest floods in this + log — 15,767 username lookups and 7,284 chain-storage misses — were worth + 0.16s and 0.24s. Both were correctly left alone. `rpc-cost.py` first. +- **Blaming the biggest number.** Service-side background work — the search + indexer especially — can dwarf the real bug. An empty ROOT means nothing asked + for it. Check whether it recurs on a timer before spending time on it. +- **Reporting a burst as same-subject when it is not.** See the BURSTS caveat + above. If it matters, prove the subject repeats before claiming it. +- **Using plain `grep`.** It is wrapped in this environment and truncates. Use + python, as the scripts do. +- **Comparing unlike runs.** `rpc-diff.py` is only meaningful if both runs did + the same thing. + +## Shapes seen before + +Each of these was a real bug, and each is what its section looks like: + +- An app RPC in the thousands where the screen has tens of items — a per-item + prime re-run on every list reload. +- A local call in the low single digits dragging thousands of lines behind it — a + per-item cost inside a server-side loop. +- FLOODS dominated by one span whose BATCHES are almost all size 1. +- An APP row whose count is a multiple of the number of times a screen was + opened — a component keyed on data that arrives in stages, remounting. +- **A list row eagerly loading what only its hover popup needs.** The single + biggest win here was one `loadOnDemand` flag: a profile rendered a row per + team and each row annotated its team up front, for a popup nobody opened. +- **A hook that subscribes to a broadcast per component instance.** One + notification then does N times the work, and if the work itself emits that + notification it compounds. Subscribe once at module scope and fan out. +- **The same expensive load repeated because each surface holds its own copy.** + Two or three components asking for the same user or team at the same instant + is the common case, not the rare one. + +Where the money actually is in this client: identify/proof checks and team +loads. Both are hundreds of milliseconds each and both fan out to several HTTP +calls. Chat localization is cheap per call but runs over every channel. diff --git a/skill/keybase-rpc-log-analysis/scripts/clean-logs.sh b/skill/keybase-rpc-log-analysis/scripts/clean-logs.sh new file mode 100755 index 000000000000..47a30b7f2450 --- /dev/null +++ b/skill/keybase-rpc-log-analysis/scripts/clean-logs.sh @@ -0,0 +1,45 @@ +#!/bin/bash +# Clear keybase logs so the next run is the only thing in them. +# +# ./clean-logs.sh # list what would move, change nothing +# ./clean-logs.sh --archive # move them to ~/Library/Logs/keybase-archive-/ +# ./clean-logs.sh --delete # remove them +# +# Default is a dry run: an unattended --delete on the wrong machine is not +# recoverable, and the logs are often the only record of a run. +set -euo pipefail + +LOGDIR="$HOME/Library/Logs" +MODE="${1:-}" + +shopt -s nullglob +FILES=("$LOGDIR"/keybase*.log "$LOGDIR"/keybase*.log-* "$LOGDIR"/Keybase.app.log) +shopt -u nullglob + +if [ ${#FILES[@]} -eq 0 ]; then + echo "no keybase logs in $LOGDIR" + exit 0 +fi + +TOTAL=$(du -ch "${FILES[@]}" 2>/dev/null | tail -1 | cut -f1) +echo "${#FILES[@]} files, $TOTAL:" +for f in "${FILES[@]}"; do + printf ' %8s %s\n' "$(du -h "$f" | cut -f1)" "$(basename "$f")" +done + +case "$MODE" in + --archive) + DEST="$LOGDIR/keybase-archive-$(date +%Y%m%dT%H%M%S)" + mkdir -p "$DEST" + mv "${FILES[@]}" "$DEST/" + echo "moved to $DEST" + ;; + --delete) + rm -f "${FILES[@]}" + echo "deleted" + ;; + *) + echo + echo "dry run. pass --archive to move them aside, or --delete to remove them." + ;; +esac diff --git a/skill/keybase-rpc-log-analysis/scripts/kblog.py b/skill/keybase-rpc-log-analysis/scripts/kblog.py new file mode 100755 index 000000000000..5afb98f20423 --- /dev/null +++ b/skill/keybase-rpc-log-analysis/scripts/kblog.py @@ -0,0 +1,87 @@ +"""Shared parsing for keybase service logs. + +Line shape: + 2026-07-24T17:19:03.576384-04:00 ▶ [DEBU keybase utils.go:537] 286366 ++Chat: | Body [tags:k=v,...] +""" + +import re + +TS_LEN = 19 # 2026-07-24T17:19:03 +BODY_RE = re.compile(r"\]\s+\w+\s+(.*)") +TAGS_RE = re.compile(r"\[tags:.*") +TIME_RE = re.compile(r"\[time=[^\]]*\]") +HEXID_RE = re.compile(r"[0-9a-f]{16,}") +# an id explicitly labelled as a conversation +CONVID_RE = re.compile(r"conv(?:ID)?[:= (]+([0-9a-f]{16,})", re.I) +# the logged-in user's id, which appears on a great many lines. It has to be +# excluded when guessing what a call was about, or every call looks like it +# shares a subject. +UID_RE = re.compile(r"uid:\s*([0-9a-f]{16,})") + + +def subject_of(line, uid=None): + """Best guess at what a line is about: a labelled conv id, else the first id + that is not the logged-in user.""" + stripped = TAGS_RE.sub("", line) + m = CONVID_RE.search(stripped) + if m: + return m.group(1) + for candidate in HEXID_RE.findall(stripped): + if candidate != uid: + return candidate + return None +NUM_RE = re.compile(r"\b\d+\b") +TRACE_RE = re.compile(r"chat-trace=(\w+)") + +# app -> service: a local RPC the client asked for +SERVER_RE = re.compile(r"\+ Server: (\w+)(\([^)]*\))?") +# service -> keybase servers: these are what burn the server-side rate limits +REMOTE_RE = re.compile(r"\+ RemoteClient: ([\w.]+)") +# raw HTTP to the API host +HTTP_RE = re.compile(r"(GET|POST) (https?://[^\s?]+)") + + +def body(line): + m = BODY_RE.search(line) + return (m.group(1) if m else line).rstrip() + + +def normalize(line, width=95): + """Collapse ids, numbers and trailing metadata so repeated lines group.""" + s = body(line) + s = TAGS_RE.sub("", s) + s = TIME_RE.sub("", s) + s = HEXID_RE.sub("", s) + s = NUM_RE.sub("", s) + return s.strip()[:width] + + +def stamp(line, precision=19): + """precision: 19=second, 16=minute, 13=hour.""" + return line[:precision] if len(line) >= precision else None + + +def iter_lines(paths, start=None, end=None): + """Yield lines from each path, optionally filtered to a timestamp window. + + start/end are prefixes, e.g. '2026-07-24T17:23'. + """ + for path in paths: + with open(path, errors="ignore") as f: + for line in f: + if start or end: + ts = line[:TS_LEN] + if not ts.startswith("2"): + continue + if start and ts < start: + continue + # end is a prefix, so compare only that many chars: a bare + # '...T19:12' must include all of 19:12, not exclude it + if end and ts[: len(end)] > end: + continue + yield line + + +def trace(line): + m = TRACE_RE.search(line) + return m.group(1) if m else None diff --git a/skill/keybase-rpc-log-analysis/scripts/rpc-cost.py b/skill/keybase-rpc-log-analysis/scripts/rpc-cost.py new file mode 100755 index 000000000000..84a56100009d --- /dev/null +++ b/skill/keybase-rpc-log-analysis/scripts/rpc-cost.py @@ -0,0 +1,101 @@ +#!/usr/bin/env python3 +"""Rank operations by the time they actually took, not by how often they appear. + + python3 rpc-cost.py ... [--top 20] [--min-mean-ms 0] + +Count and cost are different questions, and the loudest thing in the log is +usually not the expensive one. In one capture the top line by count was an +in-memory map lookup running 15,767 times for 0.16s total, while the real cost +was 469 team loads at 292ms each - 186 seconds, and only the 30th most common +line. + +Reads the [time=...] that the Go tracing helpers put on every span exit, so it +only sees operations that are traced. Times overlap across goroutines, so the +totals are service time, not wall clock. +""" + +import argparse +import collections +import re + +import kblog + +# "- Namespace: Op(args) -> ok [time=1.234ms]", optionally behind a "++Chat: " +# prefix. Keep the whole "Namespace: Op": stopping the name at the first colon +# collapsed every "Server: X" and "RemoteClient: X" into one row, which hid the +# per-operation breakdown this script exists to produce. A colon only ends the +# name when it is not followed by a space. +EXIT_RE = re.compile( + r"\]\s+\w+\s+(?:\+\+\w+:\s+)?-\s+((?:[^\s(:]|:(?= )|(?<=:) )+?)\s*(?:\(| ->).*\[time=([^\]]+)\]" +) +# Go durations are compound once they pass a minute ("1m28.867s", "1h2m3s"). +# Matching only a single number+unit silently dropped exactly the slowest spans. +DUR_RE = re.compile(r"(?:([\d.]+)h)?(?:([\d.]+)m(?!s))?(?:([\d.]+)(ms|s|µs|ns))?$") +UNIT_MS = {"ms": 1.0, "s": 1000.0, "µs": 0.001, "ns": 1e-6} + + +def parse_duration_ms(text): + """Return milliseconds for a Go duration string, or None if unparseable.""" + m = DUR_RE.match(text.strip()) + if not m: + return None + hours, minutes, value, unit = m.groups() + total = 0.0 + if hours: + total += float(hours) * 3_600_000 + if minutes: + total += float(minutes) * 60_000 + if value: + total += float(value) * UNIT_MS[unit] + return total if (hours or minutes or value) else None + + +def main(): + ap = argparse.ArgumentParser() + ap.add_argument("logs", nargs="+") + ap.add_argument("--start") + ap.add_argument("--end") + ap.add_argument("--top", type=int, default=20) + ap.add_argument("--min-mean-ms", type=float, default=0.0, help="hide cheap chatty ops") + args = ap.parse_args() + + total = collections.Counter() + calls = collections.Counter() + worst = {} + + for line in kblog.iter_lines(args.logs, args.start, args.end): + m = EXIT_RE.search(line) + if not m: + continue + # "Namespace: Op" is the useful grouping; anything past that is an + # argument ("localizerPipeline: localizeConversation: TLF: foo") and + # would split one operation into a row per argument value. + name = ": ".join(m.group(1).strip().split(": ")[:2])[:45] + ms = parse_duration_ms(m.group(2)) + if ms is None: + continue + total[name] += ms + calls[name] += 1 + if ms > worst.get(name, 0): + worst[name] = ms + + if not calls: + print("no traced spans with [time=] found") + return + + print(f"{sum(calls.values())} traced spans, {sum(total.values()) / 1000:.1f}s of service time") + print("(times overlap across goroutines - this is service time, not wall clock)\n") + print(f"{'total_s':>9} {'calls':>7} {'mean_ms':>9} {'max_ms':>9} operation") + shown = 0 + for name, ms in total.most_common(): + mean = ms / calls[name] + if mean < args.min_mean_ms: + continue + print(f"{ms / 1000:9.2f} {calls[name]:7d} {mean:9.2f} {worst[name]:9.1f} {name}") + shown += 1 + if shown >= args.top: + break + + +if __name__ == "__main__": + main() diff --git a/skill/keybase-rpc-log-analysis/scripts/rpc-diff.py b/skill/keybase-rpc-log-analysis/scripts/rpc-diff.py new file mode 100755 index 000000000000..e2509f5f762f --- /dev/null +++ b/skill/keybase-rpc-log-analysis/scripts/rpc-diff.py @@ -0,0 +1,64 @@ +#!/usr/bin/env python3 +"""Compare RPC counts between two runs, to prove a fix landed. + + python3 rpc-diff.py before.log after.log [--top 30] + +Counts app->service and service->server calls in each and prints the change. +Only meaningful if both runs did the same thing - e.g. the same e2e suite. +""" + +import argparse +import collections + +import kblog + + +def counts(path): + app, remote, http = collections.Counter(), collections.Counter(), collections.Counter() + for line in kblog.iter_lines([path]): + m = kblog.SERVER_RE.search(line) + if m: + app[m.group(1)] += 1 + m = kblog.REMOTE_RE.search(line) + if m: + remote[m.group(1)] += 1 + # HTTP matters as much as the RPC counts: a lot of server work - team + # loads, proof checks, merkle lookups - shows up only here. + m = kblog.HTTP_RE.search(line) + if m: + http[m.group(2)[:70]] += 1 + return app, remote, http + + +def show(title, before, after, top): + keys = set(before) | set(after) + rows = [] + for k in keys: + b, a = before.get(k, 0), after.get(k, 0) + rows.append((a - b, b, a, k)) + rows.sort(key=lambda r: abs(r[0]), reverse=True) + print(f"\n== {title} before {sum(before.values())} -> after {sum(after.values())}") + print(f"{'delta':>8} {'before':>8} {'after':>8} call") + for delta, b, a, k in rows[:top]: + if delta == 0: + continue + mark = " <-- gone" if a == 0 and b else (" <-- new" if b == 0 else "") + print(f"{delta:+8d} {b:8d} {a:8d} {k}{mark}") + + +def main(): + ap = argparse.ArgumentParser() + ap.add_argument("before") + ap.add_argument("after") + ap.add_argument("--top", type=int, default=30) + args = ap.parse_args() + + ba, br, bh = counts(args.before) + aa, ar, ah = counts(args.after) + show("REMOTE service -> keybase servers", br, ar, args.top) + show("APP app -> service", ba, aa, args.top) + show("HTTP", bh, ah, args.top) + + +if __name__ == "__main__": + main() diff --git a/skill/keybase-rpc-log-analysis/scripts/rpc-report.py b/skill/keybase-rpc-log-analysis/scripts/rpc-report.py new file mode 100755 index 000000000000..956f6bb61dcb --- /dev/null +++ b/skill/keybase-rpc-log-analysis/scripts/rpc-report.py @@ -0,0 +1,116 @@ +#!/usr/bin/env python3 +"""Summarize RPC traffic in a keybase service log. + + python3 rpc-report.py ... [--start 2026-07-24T17:23] [--end ...] [--top N] + +Sections, in the order worth reading: + REMOTE service -> keybase servers. These burn the server rate limits. + APP app -> service. One of these usually explains a REMOTE row. + HTTP raw API calls. + BURSTS same RPC, same args, >=3 times inside one second. Missing dedupe. + FLOODS most repeated log lines overall, after collapsing ids/numbers. +""" + +import argparse +import collections +import sys + +import kblog + + +def main(): + ap = argparse.ArgumentParser() + ap.add_argument("logs", nargs="+") + ap.add_argument("--start", help="timestamp prefix, e.g. 2026-07-24T17:23") + ap.add_argument("--end") + ap.add_argument("--top", type=int, default=25) + ap.add_argument("--burst-min", type=int, default=3, help="calls in one second to flag") + args = ap.parse_args() + + app = collections.Counter() + remote = collections.Counter() + http = collections.Counter() + floods = collections.Counter() + per_second = collections.defaultdict(collections.Counter) # sec -> (rpc, arg) -> n + # Many RPCs log no arguments, so "same call twice in a second" would not prove + # it was the same conversation. Recover the subject by correlating on the + # chat-trace: the first id that shows up on the trace is what the call was for. + pending = {} # trace -> (second, rpc name) + uid = None # learned from the log, then excluded when guessing subjects + total = 0 + first = last = None + + for line in kblog.iter_lines(args.logs, args.start, args.end): + total += 1 + ts = kblog.stamp(line) + if ts and ts.startswith("2"): + # min/max, not first/last seen: rotated files are often concatenated + # or globbed out of order, and stream order would report a range of + # zero width + first = ts if first is None or ts < first else first + last = ts if last is None or ts > last else last + floods[kblog.normalize(line)] += 1 + + if uid is None: + u = kblog.UID_RE.search(line) + if u: + uid = u.group(1) + + tr = kblog.trace(line) + m = kblog.SERVER_RE.search(line) + if m: + name, arg = m.group(1), (m.group(2) or "") + app[name] += 1 + if ts and arg: + per_second[ts][(name, arg[:60])] += 1 + elif ts and tr: + pending[tr] = (ts, name) + elif ts: + per_second[ts][(name, "")] += 1 + elif tr and tr in pending: + subject = kblog.subject_of(line, uid) + if subject: + sec, name = pending.pop(tr) + per_second[sec][(name, f"({subject[:16]})")] += 1 + m = kblog.REMOTE_RE.search(line) + if m: + remote[m.group(1)] += 1 + m = kblog.HTTP_RE.search(line) + if m: + http[m.group(2)[:80]] += 1 + + if not total: + sys.exit("no lines matched (check --start/--end)") + + print(f"{total} lines {first} .. {last}") + + def section(title, counter, note=""): + if not counter: + return + print(f"\n== {title} (total {sum(counter.values())}) {note}") + for k, v in counter.most_common(args.top): + print(f"{v:8d} {k}") + + section("REMOTE service -> keybase servers", remote, "<- rate-limited") + section("APP app -> service", app) + section("HTTP", http) + + bursts = collections.Counter() + for sec, calls in per_second.items(): + for (name, arg), n in calls.items(): + if n >= args.burst_min: + bursts[(name, arg)] = max(bursts[(name, arg)], n) + if bursts: + print(f"\n== BURSTS same call+subject >={args.burst_min}x in one second") + print(" subject in () is the call's own argument, or the first id seen on") + print(" its chat-trace when the call logs none. Bare name = neither.") + for (name, arg), n in bursts.most_common(args.top): + print(f"{n:8d}x {name}{arg}") + + print(f"\n== FLOODS most repeated lines") + for k, v in floods.most_common(args.top): + print(f"{v:8d} {k}") + + +if __name__ == "__main__": + main() diff --git a/skill/keybase-rpc-log-analysis/scripts/rpc-why.py b/skill/keybase-rpc-log-analysis/scripts/rpc-why.py new file mode 100755 index 000000000000..0056c017992c --- /dev/null +++ b/skill/keybase-rpc-log-analysis/scripts/rpc-why.py @@ -0,0 +1,137 @@ +#!/usr/bin/env python3 +"""Explain a flood: who drives it, how it is paced, how big its batches are. + + python3 rpc-why.py ... --match 'refreshParticipantsRemote' [--start ...] [--end ...] + +DRIVERS the '+ X' logged immediately before each hit on the same chat-trace. + This is what turns 'GetURL is called 8000 times' into 'userEmojis calls it'. +CADENCE hits per second. A flat rate that never stops is a loop; spikes are mounts. +GAPS ms between the end of one hit and the start of the next. Consistently + ~0 means the caller re-fires the instant the previous lands - a feedback + loop, not a user action. +BATCHES distribution of any 'msgIDs: N' / 'convs: N' count on the line. A pile of + 1s means the caller is not batching. +""" + +import argparse +import collections +import re + +import kblog + +COUNT_RE = re.compile(r"(msgIDs|convs|len|msgs): (\d+)") +# the +/-/| marker sits right after the log's sequence number, optionally behind +# a '++Chat: ' prefix. + enters a span, - leaves it, | is a note inside one. +MARKER_RE = re.compile(r"\]\s+\w+\s+(?:\+\+\w+:\s+)?([+\-|])\s+(.*)") + + +def main(): + ap = argparse.ArgumentParser() + ap.add_argument("logs", nargs="+") + ap.add_argument("--match", required=True, help="substring to explain") + ap.add_argument("--start") + ap.add_argument("--end") + ap.add_argument("--top", type=int, default=10) + args = ap.parse_args() + + drivers = collections.Counter() + roots = collections.Counter() + per_second = collections.Counter() + batches = collections.Counter() + last_enter = {} # trace -> most recent span entered outside our match + root_rpc = {} # trace -> first 'Server: X' seen, i.e. what the app asked for + inside = collections.Counter() # trace -> depth inside our match + hits = 0 + + for line in kblog.iter_lines(args.logs, args.start, args.end): + m = MARKER_RE.search(line) + if not m: + continue + marker, text = m.group(1), kblog.TAGS_RE.sub("", m.group(2)).strip() + # Lines with no chat-trace tag are unrelated to each other. Sharing one + # bucket for them latched every untraced hit onto whichever "+ Server:" + # line happened to come first in the file and reported it as the root - + # a confident, wrong answer, in the section SKILL.md says to trust most. + tr = kblog.trace(line) + untraced = tr is None + mine = args.match in text + + srv = kblog.SERVER_RE.search(line) + if srv and not untraced and tr not in root_rpc: + root_rpc[tr] = srv.group(1) + + if mine and marker == "+": + hits += 1 + ts = kblog.stamp(line) + if ts: + per_second[ts] += 1 + # Every hit gets attributed to its trace's originating RPC. Do NOT gate + # this on nesting depth: the service fans out concurrent goroutines + # under a single chat-trace, so most hits are "inside" an earlier one + # and gating would attribute only a handful of thousands of calls. + if untraced: + roots[""] += 1 + else: + roots[root_rpc.get(tr, "")] += 1 + # DRIVERS stays gated - the nearest preceding span is only meaningful + # for a hit that is not nested inside another of our own + if not untraced and not inside[tr]: + drivers[last_enter.get(tr, "")] += 1 + c = COUNT_RE.search(text) + if c: + batches[int(c.group(2))] += 1 + inside[tr] += 1 + elif mine and marker == "-": + inside[tr] = max(0, inside[tr] - 1) + elif marker == "+" and not inside[tr]: + # a span entered outside our match is a candidate driver; spans entered + # while inside it are its own children + last_enter[tr] = text[:55] + + if not hits: + print(f"no lines matched {args.match!r}") + return + + print(f"{hits} hits on {args.match!r}") + + print("\n== ROOT app RPC that opened the trace this ran on") + for k, v in roots.most_common(args.top): + print(f"{v:8d} {k}") + # Both buckets mean "nothing asked for this": a hit whose trace never saw a + # server RPC, and a hit with no trace at all. The DRIVERS placeholder is not + # one of them - it is only ever counted into drivers, so reading it back out + # of roots always added zero. + unrooted = roots.get("", 0) + roots.get( + "", 0 + ) + if unrooted > hits * 0.5: + print(" most hits have no app RPC above them: this is service-side background") + print(" work, not something the client asked for. Look at DRIVERS instead, and") + print(" grep the trace tags for CHTBKG= to confirm.") + + print("\n== DRIVERS call logged just before, same chat-trace") + for k, v in drivers.most_common(args.top): + print(f"{v:8d} {k}") + + print("\n== CADENCE busiest seconds") + for k, v in per_second.most_common(args.top): + print(f"{v:8d} {k}") + spread = len(per_second) + print(f" spread over {spread} distinct seconds, mean {hits / max(spread, 1):.1f}/s") + + if batches: + print("\n== BATCHES count on the line") + for size, n in batches.most_common(args.top): + print(f"{n:8d}x size {size}") + ones = batches.get(1, 0) + if ones > sum(batches.values()) * 0.5: + print(" >half are size 1 - the caller is not batching") + else: + print("\n== BATCHES none: these lines carry no item count") + print(" Remote calls usually do not log one. To see batch sizes, match the") + print(" local span that wraps this call instead - e.g. match") + print(" 'HybridConversationSource: GetMessages' rather than 'getMessagesRemote'.") + + +if __name__ == "__main__": + main() diff --git a/skill/keybase-rpc-log-analysis/scripts/start-service.sh b/skill/keybase-rpc-log-analysis/scripts/start-service.sh new file mode 100755 index 000000000000..fddd29c1b841 --- /dev/null +++ b/skill/keybase-rpc-log-analysis/scripts/start-service.sh @@ -0,0 +1,84 @@ +#!/bin/bash +# Build the service from this checkout and run it in the foreground with debug +# logging going to its own file. +# +# ./start-service.sh [logfile] # default /tmp/kb-analysis/service.log +# +# Runs against the REAL home directory, so the account, teams and conversations +# the e2e suite needs are all there. That means it replaces whatever service is +# currently running - it stops the existing one first. It does not log you out. +# +# Run this in its own terminal and leave it in the foreground; ^C stops it. Then +# start the app (yarn desktop:start:hot:e2e) so it attaches to this service +# rather than auto-forking the installed one. +set -euo pipefail + +LOGFILE="${1:-/tmp/kb-analysis/service.log}" +mkdir -p "$(dirname "$LOGFILE")" + +# -P resolves symlinks before collapsing "..": this script is normally reached +# through .claude/skills -> skill, and a logical cd would land in .claude/ +REPO="$(cd -P "$(dirname "${BASH_SOURCE[0]}")/../../.." && pwd)" +cd "$REPO/go" + +echo "building from $REPO/go" +go install github.com/keybase/client/go/keybase + +# go install honours GOBIN when it is set and only falls back to GOPATH/bin +# otherwise, so asking for GOPATH unconditionally looks in the wrong place on +# any setup that sets GOBIN. +GOBIN_DIR="$(go env GOBIN)" +BIN="${GOBIN_DIR:-$(go env GOPATH)/bin}/keybase" +[ -x "$BIN" ] || { echo "no binary at $BIN"; exit 1; } +echo "built $("$BIN" version -S 2>/dev/null || echo '?')" + +# The service does not provide the filesystem itself; it forks kbfs from the same +# bin directory. Without it the Files tab is empty and `keybase git` fails with +# "KBFS client not found", which fails every files-* and git-* e2e test for +# reasons that have nothing to do with the code under test. +# +# The name matters: on darwin the service looks for a binary called "kbfs" +# (install_darwin.go), but the package that builds it is kbfsfuse, so a plain +# go install leaves you with kbfsfuse and the service still finds nothing. Hence +# the link in the hint below. +if [ ! -x "$(dirname "$BIN")/kbfs" ]; then + echo + echo "WARNING: no kbfs binary next to $BIN." + echo " Files and git e2e tests will fail with 'KBFS client not found'." + echo " To include them:" + echo " go install github.com/keybase/client/go/kbfs/kbfsfuse" + echo " ln -s $(dirname "$BIN")/kbfsfuse $(dirname "$BIN")/kbfs" + echo " Otherwise ignore those failures - they are not regressions." + echo +fi + +# The installed service and this one cannot share the socket. --include takes a +# comma separated component list; an unknown flag here just prints usage and +# leaves the old service running, which then quietly wins the socket. +# pgrep -x matches the process NAME. Do not use pgrep -f here: the pattern would +# also match any shell whose command line happens to contain it, including the +# one running this check. +if pgrep -x keybase >/dev/null 2>&1; then + echo "stopping the running service" + "$BIN" ctl stop --include service || true + for _ in $(seq 10); do + pgrep -x keybase >/dev/null 2>&1 || break + sleep 1 + done + if pgrep -x keybase >/dev/null 2>&1; then + echo "service still running; kill it before rerunning" + exit 1 + fi +fi + +: > "$LOGFILE" +echo "logging to $LOGFILE" +echo +echo "NOTE: a newly built binary has a new code signature, so macOS will prompt" +echo " for keychain access on first run. Until someone clicks Allow the service" +echo " cannot finish bootstrapping, the app sits on its splash screen, and every" +echo " test fails for a reason that has nothing to do with the code. Watch for the" +echo " prompt before walking away." +echo +echo "leave this running; start the app in another terminal" +exec "$BIN" -d --log-file="$LOGFILE" service