Users/Statuses/Notifications Pinia migration and refactor #3555
No reviewers
Labels
No labels
BE Request
Backlog
Better behavior
Blocked by deps
Bug
Code Review
Discussion
Doing
Feature Request / Enhancement
In the shining bright future maybe
It's complicated
MS Edge
Mememoon (or other niche browsers)
Missing API
Missing feature
Missing l10n/i18n
NL1
NL2
NL4
Need to verify on develop
RELEASE BLOCKER
Reassign or Close
Refactor
Regression
Safari
To Do
User story
Waiting on godot
accessibility
better documentation
chore
confirmed
easy ticket
incident
l10n update
mastoapi
mobile
needs design
needs-info
needs-review
stupid
No milestone
No project
No assignees
5 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
pleroma/pleroma-fe!3555
Loading…
Reference in a new issue
No description provided.
Delete branch "users-statuses-pinia"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What changed underneath
statusesnotificationsandusersvuex modules migrated to pinia.statusesstore split into:statuses, responsible for keeping index of all known statuses, updaing them and interacting with themtimelines, responsible for handling timelines, their pagination etc.streamingresponsible for (mastoapi) websocket. Taken mostly from theapimodule.streamingstore and listen for socket events.userstream), or just global subscription that listens to everything (useful for i.e.interfacestore listening for connects/disconnects to display notification)statusesandapistores from having to keep tabs on timeline stateTimelinecomponent on created/unmounted hooks as well as in watcher for props change.fetcherisn'tnullTimelinecomponent instead of custom wrappers.Known issues:
users-statuses-piniato Users/Statuses/Notifications Pinia migration and refactorCan't login in an anonymous window or with session cleared. All pages have the login form missing and the below errors are shown:
(closest) on /main/all
(logginIn) on /about and /login
develop doesn't have this issue
0b40236eef034b2c3bf9@phnt wrote in #3555 (comment):
should be fixed now
/oc review
OpenCode Review
This PR migrates the users/statuses/notifications/timelines Vuex modules to Pinia with a new fetcher/streaming architecture (206 files, +7073/-5422). Unit tests at head pass, but several migration seams are concretely broken. Most critical: loginUser never sets currentUser.credentials, so posting, editing and media requests go out unauthenticated and fail (P0). For streaming users, timeline WS events are never applied (broken handler) and logout aborts mid-chain leaving the token uncleared; a second logout also crashes via a nulled statuses socket. Further P2 regressions: notification click never marks seen (object passed as id), chat streaming/desktop notifications broken, hashtag search dropped, emoji-reaction refetch condition inverted, pinned-post dedupe broken by undefined-clobbering merges, and loading guards rendered ineffective by .value access on auto-unwrapped refs.
Rating
1/5
8 finding(s) are attached to changed lines.
[P2] Chat streaming and chat desktop notifications broken
Chats never receive WS updates: chats store attachSocket (chats.js:38-49) is never called anywhere, and even if it were, it listens for 'pleroma:chat_update' with updateChat({ chat }) while the event carries data.chatUpdate, so it would throw on undefined.id; chat_view's socket only listens for 'update', which pleroma:chat_update events never use, so live chat updates don't work (only the 5s poll); chat_view.js:280 defines detachSocket but never calls it, so every visited chat leaks a global subscriber whose onStreamDisconnect restarts a 5s poller after unmount. On top of that, chat_utils.js:20 still calls showDesktopNotification(window.vuex.state, opts) with the removed two-argument signature, so the SW path drops the notification (type undefined) and the fallback shows Notification(undefined) — chat message desktop notifications never display.
Location:
src/services/chat_utils/chat_utils.js:20Suggested replacement (manual):
[P2] Emoji reaction refetch condition inverted: fetches when data present, skips when missing
emoji_reactions.js:62-66: hasNoAccounts finds a reaction lacking
accounts, but fetchEmojiReactions only runs when!hasNoAccounts— the exact inverse of the old behavior (old code fetched when data was missing). Consequently, reacting to a status whose emoji_reactions came without account data (e.g. from notifications or streaming) never hydrates.me/accounts, soreactedWithreads undefined, the toggle can attempt a duplicate reaction that the server rejects, and reaction counts/UI go wrong; conversely it pointlessly refetches when data is already complete.Location:
src/components/emoji_reactions/emoji_reactions.js:64Suggested replacement (manual):
Overall
Not merge-ready. The Pinia refactor is structurally sound and tested at the store level, but the boundary code between stores, services and components contains a P0 auth regression (all status posting/editing/media requests lack the Authorization header because currentUser.credentials is never set) plus streaming and logout failures that would break core flows for real users. Each finding is localized and fixable (mostly single lines), but they must be fixed and manually verified (posting, streaming timelines, logout/login cycles, notifications, chats, hashtag search) before merge.
Last reviewed commit:
8c568d3653b5@ -300,3 +241,2 @@if (this.timeline.loading === false &&this.$el.offsetHeight > 0 &&!this.timeline.fetcher.loadingOlder.value &&[P2] Loading guards read .value on auto-unwrapped refs — older-content fetch spam and dead guards
Fetcher objects are stored in Pinia state, so their refs are unwrapped when accessed through the store (verified empirically: proxy access returns the boolean,
.valuereturns undefined). timeline.js:242!this.timeline.fetcher.loadingOlder.valueis therefore always true, so every throttled scroll event within 750px of the page bottom fires another concurrent fetchOlder even while one is in flight (duplicate requests, wasted server load). Likewise notifications.js:124/127 compute loading/bottomedOut from fetcher.loading.value/fetcher.bottomedOut.value, which are always undefined, so fetchOlderNotifications' in-flight guard never blocks. The template usages in timeline.vue (unwrapped, correct) confirm the intended shape.Suggested replacement (manual):
fixed
@ -0,0 +223,4 @@case 'follow_request':breakdefault:this.markSingleNotificationAsSeen({ id })[P2] Clicking an unread notification never marks it seen (object passed as id)
stores/notifications.js:226: notificationClicked calls this.markSingleNotificationAsSeen({ id }), but the action (line 242) and api/user.js:349 expect the raw id string (all other callers pass it correctly). idStore.get({id…}) misses so the local
seenflag is never set, and the API request serializes id to "[object Object]" and is rejected — clicking like/repeat/reaction/follow notifications leaves them permanently unread (badge count, unseenAtTop ordering and filters stay wrong) until the user presses "Read!".Suggested replacement (manual):
fixed
@ -0,0 +31,4 @@statuses,})const output = {}[P2] Hashtag search results dropped; concat(undefined) can crash the results view
stores/search.js:22-40 builds the return value with only
statusesandaccounts; the old Vuex action returned the full API payload includinghashtags. search.js:98 then does this.hashtags.concat(data.hashtags) → [undefined], so hashtag results never display; when there are no status/user matches, getActiveTab() selects the 'hashtags' tab because its length is 1, and templates iterating hashtags then hitundefined.history(lastHistoryRecord), breaking the whole results view.Suggested replacement (manual):
fixed
@ -0,0 +64,4 @@this.socket = socket},resetStatuses() {this.socket.et.removeEventListener('update', this.socket.handleUpdate)[P2] resetStatuses() nulls the statuses socket: second logout crashes and WS handlers dead after re-login
statuses.js:66-78: resetStatuses copies defaultState() which includes socket: null, but attachSocket is only called once at boot (after_store.js:597) and never on re-login (loginUser doesn't re-attach). After a logout→login cycle, streaming 'update'/'status.update'/'delete' events are no longer processed at all; on the next logout, resetStatuses hits
this.socket.eton null and throws, aborting logout cleanup (token not cleared, cookie not removed).Suggested replacement (manual):
fixed
@ -0,0 +152,4 @@// implicit: if oldTimestamp is undefined this will still be falseif (oldTimestamp > timestamp) return [existing, false] // not overwriting old data with newconst newStatus = {[P2] mergeOrAdd spreads undefined over existing status flags — pinned posts duplicated on profiles
statuses.js:155-159 builds newStatus with
{ ...old, ...neu }. parseStatus always emits favorited/repeated/bookmarked/pinned keys (entity_normalizer.service.js:248-314), usually undefined in plain timeline responses; unlike the old lodash merge (which skips undefined source values), object spread overwrites the stored values. When the plain user-timeline response is newer than the userPinned response for the same status (both fetched at profile activation),pinned: trueis clobbered to undefined, so the Timeline skipPinned filter (timeline.js:59) fails and the post is shown twice on the profile (pinned section + chronological) with the pin badge gone; the same mechanism can silently drop favorited/repeated/bookmarked flags on refetch.Suggested replacement (manual):
should be fixed now
@ -0,0 +102,4 @@}if (this.state === WSConnectionStatus.JOINED) {this.socket.unsubscribe(...this.getSubArgs(stream))[P1] Logout aborts when streaming is connected: removeSubscriber dereferences undefined stream
streaming.js:104-106: for global subscribers (the notifications socket
{ et }),streamis undefined; while state is JOINED, removeSubscriber calls getSubArgs(stream) which reads stream.name and throws TypeError. users.logout calls useNotificationsStore().deactivate() (stores/users.js:732) before stopSocket (line 740), so for streaming users the logout .then() rejects mid-chain: oauth.clearToken, Cookies.remove, statuses reset, timelines teardown and onLogout never run, and the token survives in storage (auto re-login on reload). Any other removal of a global subscriber while connected (e.g. chat_view detachSocket if it were wired) crashes the same way.Suggested replacement (manual):
fixed
@ -0,0 +105,4 @@const openHandler = () => this.onStreamConnect(timelineName, argument)const closeHandler = () =>this.onStreamDisconnect(timelineName, argument)const messageHandler =[P1] Streaming timelines never receive updates: WS message handler is never invoked
timelines.js:108-111 defines messageHandler as a curried function
() => ({ detail: message }) => ...: when the streaming store dispatches an 'update' event, messageHandler(event) merely returns the inner arrow without calling onStreamMessage, so streamed statuses are silently dropped. Even if invoked, onStreamMessage (line 232) readsevent.data.status.id, but StreamMessageEvent.data is the already-unwrapped status array from streaming.js:173-195, so it would throw; deactivate() also removes a 'message' listener (line 147) that was registered as 'update'. Because onStreamConnect stops the polling fetcher when the socket joins, users with useStreamingApi enabled get frozen timelines (no WS events, no polling) until the socket drops.Suggested replacement (manual):
fixed
@ -0,0 +610,4 @@credentials: useOAuthStore().token,})user.blockIds = new Set()[P0] Posting, editing and media upload send no auth token (currentUser.credentials never set)
The old Vuex loginUser set user.credentials = accessToken, but the new loginUser (src/stores/users.js:609-619) never sets it, and verifyCredentials/parseUser don't produce such a field. status_poster.service.js:31/74/104/109 (postStatus, editStatus, uploadMedia, setMediaDescription), instance.js:216 (getKnownDomains), and interface.js:857 still read currentUser.credentials, which is now always undefined — promisedRequest then omits the Authorization header and the server returns 401, so posting statuses, editing, and media upload/description are all broken. Additionally who_to_follow_panel.js:34 calls the nonexistent panel.$useUsersStore() (TypeError, panel fails to load when suggestions are enabled), and interface.js:857 reads this.currentUser which doesn't exist in the interface store, so unregisterPushNotifications throws.
Suggested replacement (manual):
replaced remaining
currentUser.credentialswithuseOAuthStore().tokenas it's intendedfixed
should be fixed
/oc review
/oc rerun
/oc rerun
OpenCode Review
Pinia migration of users/statuses/notifications/timelines plus a new fetcher/streaming layer (205 files). Unit tests pass at head and all previously flagged issues are confirmed fixed (token plumbing via useOAuthStore, streaming handlers, logout subscriber guard, socket reset, undefined-clobbering merges, notification click id, hashtag search, ref guards, emoji-reaction refetch). Remaining defects sit at migration seams: chat desktop notifications silently stop working for default (non-streaming) users and the new WS handler crashes on chats not yet in the list; blocking a user leaves dangling conversation indexes that throw in thread views; chat views leak streaming subscribers and pollers; and several stale references to removed state remain (bare
statusglobals in status.js and public.js, removed vuex statuses module in reports, missing panel argument in who_to_follow_panel, dead push-notification login/logout hook, dead favorites 403 fallback).Rating
3/5
7 finding(s) are attached to changed lines.
[P2] status.js references bare
statusglobal — backend-muted statuses are never muted or hiddenThe refactor dropped the old
const { status } = thisdestructuring but kept(status.muted && !status.thread_muted). In the browserstatusresolves to window.status (a string), so this term is always falsy: statuses the backend marks muted (BE filters) are no longer treated as muted, shown in the muted placeholder, or hidden by hideMutedUsers/hideFilteredStatuses. muted/hideStatus feed directly from userIsMuted in status.vue, so this silently disables server-side mute handling.Location:
src/components/status/status.js:322Suggested replacement (manual):
[P2] Report-user modal crashes: openUserReportingModal reads the removed vuex statuses module
The first vuex reference in this action was migrated to useStatusesStore(), but the filter still reads
window.vuex.state.statuses.allStatuses. This PR deleted the vuex statuses module, so window.vuex.state.statuses is undefined and opening the report modal (account_actions, status 'Report' button) throws TypeError: Cannot read properties of undefined (reading 'allStatuses') — the reporting feature is broken.Location:
src/stores/reports.js:28Suggested replacement (manual):
Overall
Structurally sound migration with good store-level test coverage, and the author has been responsive in fixing earlier findings. However, one P1 remains: chat message desktop notifications are effectively removed for all users with default settings (poll path no longer notifies; the new WS-only path requires useStreamingApi and crashes for new chats). The other findings are localized P2 crashes or silent regressions reachable from common flows (blocking a user, opening edit history, reporting a user, instances with suggestions enabled, logout with web push). All are small, pinpoint fixes, but should be applied and manually exercised (chats, block, edit history, report modal, logout with push enabled) before merge.
Last reviewed commit:
f1e9c565690a@ -186,0 +184,4 @@return {...rest,data: [...data].reverse().map((item) => {item.originalStatus = status[P2] fetchStatusHistory assigns window.status to originalStatus — edit-history entries lose merged fields
The signature was changed to ({ id, credentials }) but the body still does
item.originalStatus = status, which now resolves to the window.status string (''). parseStatus then does Object.assign(output, '') — a no-op — so history entries no longer inherit the original status fields (id and other normalized fields not present in the status-history API response), degrading/breaking the edit-history modal rendering (duplicate undefined keys, missing status flags).Suggested replacement (manual):
adding
idfield seem to be just enough to make<Status>render properly for our needs.@ -120,1 +126,4 @@)if (this.testMode) returnthis.deactivate()[P2] chat_view never calls detachSocket — leaked subscribers restart pollers after unmount
created() attaches a 'chatview' global subscriber to the streaming store, but unmounted() only calls deactivate(); detachSocket (chat_view.js:286) is never invoked anywhere. Every chat visit leaks a subscriber whose handlers keep a dead component alive; on any socket disconnect each leaked subscriber's onStreamDisconnect calls startFetching on the unmounted component, spinning up a new 5-second chat poller per leaked view that runs until the socket reconnects — unbounded network/CPU and memory growth for streaming users who browse chats.
Suggested replacement (manual):
added
@ -78,3 +78,3 @@}))if (this.suggestionsEnabled) {getWhoToFollow(this)getWhoToFollow()[P2] who_to_follow_panel calls getWhoToFollow() without the panel argument — TypeError on instances with suggestions
Old code called getWhoToFollow(this); the migration dropped the argument, so getWhoToFollow immediately does
panel.usersToFollow.forEachon undefined and throws whenever the instance has suggestions enabled (mounted and the user watcher). The panel fails to load and the error surfaces via the global error handler.Suggested replacement (manual):
refactored this component to use methods and
thisinstead of loose functions@ -11,1 +7,3 @@returnconst validActions = {sync_config: new Set(['setPreference']),interface: new Set(['setNotificationPermission', 'setLoginStatus']),[P2] Web Push subscription never unregistered on logout
The PR removed the vuexPushNotificationsPlugin that registered/unregistered push on setCurrentUser/clearCurrentUser, but the pinia plugin still keys off an interface action 'setLoginStatus' that no longer exists (nothing dispatches it, and users.logout/onLogout never call unregisterPushNotifications). After logout the browser keeps its push subscription and the backend keeps sending account notifications to the logged-out device — a privacy leak that the old code prevented.
Suggested replacement (manual):
added
onLoginandonLogoutto list of valid actions@ -66,2 +78,3 @@})chats.forEach((updatedChat) => {result.data.forEach((updatedChat) => {[P1] Chat desktop notifications no longer fire (poll path dropped) and WS handler crashes on new chats
The old chats store called maybeShowChatNotification from the polling path (addNewChats:
if (isNewMessage && chat.unread) maybeShowChatNotification(chat)); the new addNewChats drops this entirely, and chat_update WS events only arrive when useStreamingApi is enabled (default false) — so chat message desktop notifications silently disappear for default users. For streaming users the new WS path is also broken: updateChat calls maybeShowChatNotification(chat) wherechatis undefined whenever the chat isn't already in chatList.data (e.g. first message of a new chat), throwing TypeError inside the listener, skipping the notification and the idStore update.Suggested replacement (manual):
refactored store, now chat insert/update is unified and always calls
maybeShowChatNotification, which now also checks for unreads.@ -0,0 +77,4 @@return { statuses, pagination }}).catch((error) => {if (error.statusCode === 403 && timeline === 'favorites') {[P2] Favorites 403 fallback is dead code: object compared to string and 'favourites'/'favorites' name mismatch
timeline_fetcher catches
error.statusCode === 403 && timeline === 'favorites', buttimelineis now the timeline object, so the branch never runs and pleromaPublicFavouritesAvailable is never set false; additionally timelines.activate() guardstimelineName === 'favourites'while the actual timeline name is 'favorites', so the capability check is dead too. On instances where /api/v1/favourites returns 403, the favorites fetcher keeps polling every 10s and pushes a new error notice each cycle instead of disabling the tab.Suggested replacement (manual):
cleaned up favorites/favourites confusion, also fixed favorites tab being permanently disabled
@ -0,0 +530,4 @@const removed = new Set()this.allStatuses.forEach((status) => {if (status.user.id === userId) {this.allStatuses.delete(status.id)[P2] Blocking a user leaves dangling conversation indexes — thread views throw TypeError
wipeUserStatuses deletes statuses from allStatuses but never removes their ids from the
conversationsMap. Conversation'sconversationcomputed does[...conversation.keys()].map(k => allStatuses.get(k)).toSorted(sortById); for a blocked user's status the map yields undefined and sortById reads.typeon it, throwing and breaking any open/expanded thread (conversation page, inline expanded timelines) that contained one of their posts. The old code only removed statuses from three timeline arrays and never deleted them from the store, so this crash path is new.Suggested replacement (manual):
wipe now also wipes conversations, in addition added missing
wipeStatusesfrom notifications storeRefactored that piece of code to use
this.repeatedStatusandthis.repeaterinsteadAdded
stausesPerUserindex tostatusesstore for easier search (no need to iterate over entireallStatuses) and refactored report modal/store to utilize that./oc review
OpenCode Review
Large Pinia migration of users/statuses/notifications/timelines plus a new fetcher/streaming layer (208 files). Unit suite passes at head and nearly all previously flagged issues are verified fixed in current code (token plumbing, streaming handlers, socket reset, merge semantics, notification click, hashtag search, ref guards, emoji-reaction refetch, muted-status handling, favorites fallback, edit history, report store index, chat socket wiring). Remaining defects cluster around logout and store seams: the new chats store's fetcher lifecycle TypeErrors and aborts every logout (P1); logout also resumes unauthenticated pollers producing 10s error toasts; the push-unregister fix is incomplete; unguarded statusesPerUser lookups crash report/block flows; admin deleteUsers references an undefined variable; reply_visibility is silently dropped; chat notifications re-fire every poll. Note: block/unblock/unmute/removeUserFromFollowers/editUserNote still send no auth token, but that predates this PR.
Rating
3/5
6 finding(s) are attached to changed lines.
[P2] Web Push subscription still never unregistered on logout (plugin early-returns for onLogin/onLogout)
Re-raised: the fix added onLogin/onLogout to validActions, but the interface branch (lines 37-44) still only updates state for setNotificationPermission/setLoginStatus and
returns for everything else — including onLogin and onLogout — so onLogout never reaches the register/unregister decision. Registration on login only happens as a side effect of setNotificationPermission. Additionally logout calls oauth.clearToken() before useInterfaceStore().onLogout(), so even a wired-up unregister would call deleteSubscriptionFromBackEnd(null) and fail. After logout the browser keeps its push subscription and keeps receiving notifications for the logged-out account (privacy leak the old code path intended to prevent).Location:
src/lib/push_notifications_plugin.js:43Suggested replacement (manual):
Overall
Structurally sound migration with good store-level test coverage, and the author has fixed most earlier findings. However, one P1 remains: the chats store stores a factory function as its fetcher, so stopFetching/resetChats throw a TypeError inside users.logout()'s then-chain — on chat-enabled instances every logout aborts before Cookies.remove and interface.onLogout() run, shows the logout-failure toast, leaves the web-push subscription registered and the streaming socket open (verified empirically with a unit test against the real store). A related logout defect leaves unauthenticated timeline pollers running with 10-second error toasts. The rest are localized P2s: report-modal/block-wipe crash on users without loaded statuses, the still-dead push-unregister path on logout, admin deleteUsers crashing on an undefined variable, reply_visibility never being sent, and chat notifications re-firing every 5s poll. All are small pinpoint fixes; fix them and manually exercise logout/login cycles, chats, blocking, reporting, and admin deletion before merge.
Last reviewed commit:
ae127a8d5184@ -433,3 +433,1 @@(status) => userId === status.user.id,)// TODO when migrated to pinia, also remove useruseStatusesStore().wipeUserStatuses(status.user.id)[P2] Admin account deletion throws: deleteUsers uses undefined
statusinstead of userIdThe migrated forEach callback calls
useStatusesStore().wipeUserStatuses(status.user.id), but the loop variable isuserId;statusis not declared in scope, resolves to window.status ('') in the browser, and crashes reading.user.id(ReferenceError/TypeError under test). It throws after the delete API call has already succeeded, so the action rejects, the user's statuses are never wiped from the store, and the admin UI reports an error even though the accounts were deactivated.Suggested replacement (manual):
fixed
@ -40,2 +42,2 @@stopFetchingChats() {this.setChatListFetcher(null)startFetching() {this.fetcher = () => promiseInterval(() => this.fetchChats(), 5000)[P1] Logout always fails: chats store stopFetching calls .stop() on a factory function
startFetching stores the factory itself in this.fetcher (
this.fetcher = () => promiseInterval(...)at line 43) and discards the interval handle that promiseInterval returns, so stopFetching'sthis.fetcher?.stop()(line 47) resolves .stop to undefined and throws TypeError (reproduced in a unit test against the real store: startFetching → stopFetching/resetChats throw). users.logout() calls useChatsStore().resetChats() inside its .then chain, so on any chat-enabled instance every logout throws: the chain aborts before Cookies.remove('__Host-pleroma_key') and useInterfaceStore().onLogout() run, the web-push subscription is never unregistered, the streaming socket is never closed, and the user sees the logout_failure error toast every time. resetChats also calls startFetching unconditionally (line 60), which once the throw is fixed would spawn an unauthenticated chats poller after logout; and because the interval handle is never stored, no chats poller can ever be stopped, so re-login cycles accumulate parallel pollers.Suggested replacement (manual):
fixed
@ -99,3 +88,1 @@this.chatList.data.unshift(updatedChat)}this.chatList.idStore[updatedChat.id] = updatedChatmaybeShowChatNotification(chat ?? updatedChat)[P2] Chat desktop notifications re-fire every 5s poll while a chat stays unread
updateChat now always calls maybeShowChatNotification (line 88) whenever chat.unread > 0, and it is invoked from addNewChats for every chat in the list on each 5-second poll (plus per WS event). The old code guarded with
isNewMessage = chat.lastMessage?.id !== updatedChat.lastMessage?.idbefore notifying. Any chat that stays unread (user away from the app, or browsing the chat list without opening the chat) re-triggers showDesktopNotification every 5 seconds indefinitely — re-showing/re-alerting toasts on platforms that re-surface replaced same-tag notifications and posting repeated showNotification messages to the service worker — instead of notifying once per new message.Suggested replacement (manual):
fixed
@ -0,0 +46,4 @@}args.withMuted = !hideMutedPostsif (loggedIn && REPLY_VISIBILITY_TIMELINES.has(timeline)) {[P2] reply_visibility never sent: REPLY_VISIBILITY_TIMELINES.has() receives the timeline object
timelineFetcher is now called with the timeline store object (timelines.activate passes
timeline), but line 49 checksREPLY_VISIBILITY_TIMELINES.has(timeline)against a Set of name strings, so it is always false and args.replyVisibility is never set for friends/public/publicAndExternal/bubble/dms. The old fetcher compared against the timeline name string. Users who set 'Replies: following only / self only' get all replies in their home/public timelines again — silent regression of a user-facing filtering setting.Suggested replacement (manual):
fixed
@ -31,1 +21,3 @@)const preTickedIds = new Set(statusIds)// There shouldn't be a case where this is undefinedconst userAllStatusesIds = useStatusesStore().statusesPerUser.get(userId)[P2] Report modal and block-wipe crash for users with no loaded statuses (unguarded statusesPerUser lookup)
openUserReportingModal spreads
useStatusesStore().statusesPerUser.get(userId)without a default; statusesPerUser only gains entries when a user's statuses pass through addNewStatuses, so reporting from user cards (search results, follow requests, member lists, who-to-follow) of a user whose posts aren't in the store hits...undefined→ TypeError: not iterable, and the report modal never opens. The comment "There shouldn't be a case where this is undefined" is wrong (also after wipeUserStatuses deletes the entry). Same unguarded pattern in statuses.wipeUserStatuses (line 537:removed.forEachon undefined), reachable from blockUser and admin_settings deleteUsers once those paths execute. Fix both with?? new Set().Suggested replacement (manual):
fixed
@ -0,0 +185,4 @@timeline.paused = trueconsole.debug('[Timelines] Pausing timeline', name)if (timeline.fetcher && timeline.fetching) {timeline.fetcher.stopFetching()[P2] Logout resumes unauthenticated timeline pollers, spamming error toasts every 10s
pause() (timelines.js:187-189) stops the fetcher interval directly but leaves timeline.fetching true. During logout, pauseAll() runs first, so when deactivateAll() → deactivate() calls stopFetchingTimeline → fetcher.stopFetching(), timeline_fetcher's stopFetching dereferences the already-nulled interval (
interval.value.stop()) and throws; the error is swallowed by deactivateAll's try/catch, so those timelines are never reset (fetcher and fetching stay set). logout's .finally then calls resumeAll(), which sees fetcher && fetching truthy and restarts the pollers with the now-revoked token, and logout's .catch additionally restarts announcements/lists/bookmarks fetchers. Result: after every logout, friends/dms and any open timeline keep hitting /api/v1/timelines unauthenticated every 10 seconds, each 401 pushing a global 'timeline.error' toast until the user navigates or reloads. Fix: clear fetching in pause(), null-guardinterval.valuein timeline_fetcher.stopFetching, and don't resume/restart fetchers after a completed logout.Suggested replacement (manual):
fetchingshould remain astrue, it indicates whether it's supposed to be fetching - useful for when resuming timeline, i.e. if we paused timeline that wasn't fetching we shouldn't restart the fetcher, but if it was fetching we should.Been seeing a very noticeable slowdown compared to develop when FE is left idle for prolonged periods of time and then clicking on the "Show new" button for timelines. I would say maybe 2-3x slower than develop.
Also the "checking for new TL posts" spinner can collide with the TL name on mobile when the "Show new/reload" button is present.
I noticed slowdown too. I think mutating maps is slow in pinia maybe.
And a third time (sorry for the spam), some avatars don't load in the favs/repeat section of selected posts. Same with the list of users for emoji reacts.
favs/repeats:
emoji reacts (also has the favs/repeats issue):
fixed
fixed
56dbf10949a44b60dbc8review from sol:
Yes — there are three separate regressions.
1. “Show new” slowdown
This is probably not Pinia
Mapmutation, as hj suspected. The migration removed the old 50-status display cap:developexposedstatuses.slice(0, 50)when “Show new” was clicked.statusIdsset insrc/stores/timelines.js:349-354.Conversationcomponent per accumulated status. Virtual scrolling hides their contents but does not reduce the number of component entries.A targeted test with 500 buffered statuses confirmed that the PR exposes all 500;
developwould expose 50. That directly explains why the delay grows with idle time.The fix should restore a bounded window while updating
order,statusIds,visibleStatusIds, andminIdconsistently. Simply slicingvisibleStatusIdscould create pagination gaps.There is also a secondary typo in
timeline_fetcher.js:REPLY_VISIBILITY_TIMELINES.has(timeline)compares an object against timeline-name strings; it should usetimeline.name. That may increase backlog size for some configurations.2. Mobile spinner/title collision
The PR added another independent grid item for the polling spinner. On mobile, the timeline title, spinner, scroll-top control, Show new/reload button, and quick-view button can overcommit the header width.
The title cannot reliably shrink because the popover trigger is intrinsic-width, and one component-specific ellipsis selector targets nonexistent
.timeline-menu-namerather than.timeline-title.For the reported combination, the smallest fix is probably:
A complete simultaneous-display fix would also constrain the TimelineMenu trigger chain and use a shrinkable title grid track.
3. Missing interaction avatars
This one is definitive:
AvatarListandUserListPopovernow pass onlyuser.id.UserAvatarresolves that ID exclusively through the canonical users store.Thus an interaction-only account renders a placeholder, and its avatar URL is never requested. Accounts previously encountered elsewhere happen to work, explaining why only some avatars disappear.
I verified the example APIs: both newt and the emoji-reacting account are returned with valid avatar URLs. The frontend drops the connection to that data.
The interaction handlers should preserve each response’s
{ data, timestamp }and calluseUsersStore().addNewUsers(...); reaction accounts need to be flattened first. Existing tests accidentally use the same ID for the status author and interaction user, masking the bug.Verification and notes
2111506230and1d0115d433.Oh boy, our persist plugin is misbehaving, it's being called way too many times
pinia was persisting (cloning!) entire users store because it was configured to persist
lastLoginNamewhich is no longer used, I removed it and at least notifications load much faster now.oh, true. i removed it because it messed with pagination, i guess it needs to be implemented properly
@hj wrote in #3555 (comment):
Take your time. After this is done and drafts are fixed, I think we can make a new BE release with new FE bundled. BE is currently waiting on a new dependency release anyway.
partially fixed. spinner replaces the "up to date" indicator (need to fix collision with "show new" button)
Also fixed virtual scrolling being broken at times.
Also re-implemented reprööts deduplication, only oldest known repeats are shown with a small exception of loading original status when fetching older posts. Tests are written for these cases.
fetching favorites, repeats and emoji reactions now processes users. favorites and repeats are now stored separately from status to avoid accidental overwrite of the
rebloggedBy/favoritedByproperty of statuses.When "show new" button is shown loading newer spinner doesn't show up at all.
/oc review
OpenCode Review
Pinia migration of users/statuses/notifications/timelines plus the new fetcher/streaming layer is structurally sound, store-level test coverage is good, and the full unit suite passes at head. Nearly all previously flagged defects are verified fixed in current code (chats fetcher lifecycle, browser-side push unregister on logout, reply_visibility, favorites 403 handling, reports/statusesPerUser guards, 50-status show-new cap, virtual scrolling, interaction-user registration for favs/repeats/reactions, ref-unwrap loading guards). Six medium defects remain at migration seams: repeats now leak into expanded thread views as duplicate top-level posts; chat desktop notifications re-fire every 5s poll (the id-based comparison was replaced by an object-identity check); the admin Users tab crashes for any listed user not already in the users store; the emoji-reaction 'who reacted' popover passes a removed prop and throws on hover; a failed logout permanently stops notification polling and stacks duplicate chats pollers; and removeSubscriber never deletes global subscribers, leaking ets per chat view / login cycle.
Rating
4/5
6 finding(s) are attached to changed lines.
Overall
Merge-ready after fixing the listed P2s. Strengths: consistent store design, real dedup tests for repeats, working logout teardown on the success path, and all earlier P0/P1 findings confirmed resolved at head (
b124d2b8). Remaining work is localized: restore retweet filtering in conversation.js (old sortAndFilterConversation semantics), compare chat lastMessage by id, guard or hydrate users in updateUserAdminData before admin listing, pass user ids to UserListPopover from emoji_reactions, make notifications resume() not depend on the fetching flag cleared by pause(), and delete global subscribers in streaming removeSubscriber. Each is a small pinpoint fix; manually exercise thread expansion, chat unread polling, the admin Users tab, reaction-count hover, and a failed logout (e.g. server down during revocation) before merge.Last reviewed commit:
b124d2b8ea56@ -196,3 +185,4 @@}return sortAndFilterConversation(conversation, this.status)return [...conversation.keys()].map((k) => useStatusesStore().allStatuses.get(k)).toSorted(sortById)[P2] Expanded threads show repeats as duplicate top-level posts (retweet filter dropped)
The migration deleted sortAndFilterConversation, which removed
type === 'retweet'entries from thread display (and hid the original when the root itself is a repeat). The newconversationcomputed maps every id in the conversations Map; repeats share statusnet_conversation_id with the original (verified via the store: both ids land in the same conversation set), and store.addNewStatuses registers retweets in that map. So any repeat of a thread post that has been loaded (e.g. seen in a timeline) now appears inside the expanded thread/conversation page as an extra top-level post (repeats have no in_reply_to_status_id), sorted to the top by id — duplicating content and mangling thread structure in a core view.This is factually incorrect on modern backend
pleroma.conversation_idso they don't get added to conversationsBut I'm guessing this might have been different behavior on older backend so we can bring back filter
@ -16,2 +17,3 @@name: 'UserListPopover',props: ['users'],props: {userIds: Set,[P2] Emoji-reaction "who reacted" popover broken: still passes removed
userspropUserListPopover's prop was renamed from
users(array of user objects) touserIds(Set of ids) in this PR, and status.vue was updated, but emoji_reactions.vue line 55 still binds:users="accountsForEmoji[reaction.name]".userIdsstays undefined, so when the popover content renders on hover the template'suserIds.sizethrows TypeError and the reacting-accounts list never displays — despite this PR's fetchEmojiReactions now correctly registering those accounts in the users store. Fix: pass ids, e.g.:user-ids="new Set((accountsForEmoji[reaction.name] || []).map(({ id }) => id))"in emoji_reactions.vue.There's a bit of a confusion in emoji reacts - status data has
account_idsbut reaction data hasaccounts, cleaned it up and now status data always hasaccount_idswhich we use in popover.@ -93,1 +77,4 @@updateChat(updatedChat) {const chat = this.data.get(updatedChat.id)if (chat) {const isNewMessage = chat.lastMessage !== updatedChat.lastMessage[P2] Chat desktop notifications re-fire every 5s poll while a chat stays unread
Re-raised: the earlier fix compares
chat.lastMessage !== updatedChat.lastMessageby object identity, but the 5s poll re-parses every chat (data.map(parseChat)), so the objects always differ and isNewMessage is always true. addNewChats → updateChat then calls maybeShowChatNotification for every chat with unread > 0 on every poll, re-showing/re-posting a desktop notification indefinitely until the chat is opened — instead of once per new message. The old code compared lastMessage ids. Same for WS redeliveries of an unchanged lastMessage.fixed manually
@ -0,0 +86,6 @@},resume() {this.paused = falseif (this.fetcher && this.fetching) {this.startFetching('Notifications resumed')}[P2] Failed logout permanently stops notification polling and stacks duplicate chats pollers
notifications.pause() calls stopFetching(), which clears
fetching, but resume() only restarts whenthis.fetcher && this.fetching— so after a logout that fails (revokeToken error), the catch path restarts announcements/lists/bookmark folders but notifications polling is never resumed (resume() no-ops) and stays dead until reload/re-login for non-streaming users. The same catch path calls useChatsStore().startFetching(), which unconditionally overwrites the still-running fetcher handle (it was never stopped at logout start), leaking one additional 5s chats poller per failed logout attempt.pause/resume incorrectly calls
this.stopFetching/this.startFetchingwhile it should be calling fetcher directly, bypassing resettingthis.fetchingproperty.@ -0,0 +96,7 @@removeSubscriber(subscriber) {const { stream } = subscriberthis.subscribers.delete(subscriber)if (stream) {this.subscriptions.get(stream.name).delete(stream.argument)}[P2] removeSubscriber never deletes global subscribers — unbounded growth per chat view / login cycle
addSubscriber puts subscribers without a
stream(notifications store, per-chat-view 'chatview' sockets) into globalSubscriptions as well assubscribers, but removeSubscriber only deletes fromsubscribersand only cleanssubscriptionsfor stream subscribers. Every chat view visit (detachSocket on unmount) and every logout (notifications deactivate) therefore permanently leaves a dead EventTarget in globalSubscriptions; onMessage dispatches every WS event to all of them forever. Long sessions accumulate unbounded leaked subscribers and growing per-event dispatch work, in contrast to the PR's own stream-subscriber cleanup.fixed
@ -0,0 +209,7 @@updateUserAdminData(id, data) {const user = this.users.get(id)user.adminData = datauser.deactivated = !data.is_activeuser.tags = new Set(data.tags)},[P2] Admin Users tab crashes: updateUserAdminData dereferences users missing from the store
admin_settings.fetchUsers/getUserData/setUsersActivationStatus/setUsersApprovalStatus/setUsersSuggestionStatus now call useUsersStore().updateUserAdminData(user.id, user) directly, but updateUserAdminData does
const user = this.users.get(id); user.adminData = datawith no guard. Users returned by /api/v1/pleroma/admin/users are usually not in the users store (the old vuex action called fetchUserIfMissing first — verified empirically that it now throws TypeError), so Promise.all rejects, the List shows an error, and the admin Users tab fails to load on any instance where listed users were not previously viewed. Same crash affects the tag/right/activation moderation actions for such users.@ -217,4 +217,8 @@user.adminData = datauser.deactivated = !data.is_activeuser.tags = new Set(data.tags)},updateUserAdminData(id, data) {const user = this.users.get(id)if (!user) returnuser.adminData = datauser.deactivated = !data.is_activeuser.tags = new Set(data.tags)},applied suggestion manually (lines are messed up)
.
/oc review
Creating a chat with a new user from search results in the following error (after clicking on the user):
Chat message is stuck sending (or whatever the spinner means), while also already being sent, thus being duplicated until FE is reloaded or chat is closed:

Clicking on the wrench for Lists in the sidebar from an open chat results in the following error:
Lists don't have their name in the TL header (the name is shown when list is first created though):

fixed already
OpenCode Review
Pinia migration of users/statuses/notifications/timelines with the new fetcher/streaming layer is in good shape at head: the unit suite passes, and all previously flagged defects are verified fixed in current code (conversation repeat filter, chat lastMessage id comparison, notifications pause/resume via direct fetcher calls, streaming global-subscriber cleanup, emoji-reaction popover account_ids, 50-status show-new cap, reply_visibility, interaction-user registration, logout teardown including chats poller and web-push unregister). Two medium defects remain: the admin Users tab still crashes for any listed user not already in the users store (the earlier fix only silenced the store throw but never restored user hydration, so List/AdminUserCard render on undefined items), and UserAvatar's bot/group badge condition is inverted relative to develop (returns hideBotIndication instead of its negation), hiding indicators by default. Both are small localized fixes; the rest of the migration looks merge-ready.
Rating
4/5
2 finding(s) are attached to changed lines.
Overall
Merge-ready after fixing the two listed P2s. Strengths: consistent store design, real regression tests for repeat deduplication and timelines, working logout teardown on both success and failure paths, and every earlier P0/P1/P2 from previous review rounds confirmed resolved at head
5dc3a87a. Remaining work is localized: hydrate (or skip) admin-listed users in fetchUsers so items are never undefined — getUserData and the activation/approval/suggestion admin actions have the same missing-hydration gap for unknown users — and negate hideBotIndication in UserAvatar's showActorTypeIndicator. Manually exercise the admin Users tab with unviewed users and check bot badges on avatars before merge.Last reviewed commit:
5dc3a87a9771@ -43,0 +40,5 @@return useUsersStore().findUser(this.userId)},showActorTypeIndicator() {return useMergedConfigStore().mergedConfig.hideBotIndication},[P2] Bot/group avatar badge condition inverted — indicators hidden by default
UserAvatar now computes showActorTypeIndicator from mergedConfig.hideBotIndication without negation. develop passed :show-actor-type-indicator="!hideBotIndication" from status.js. With hideBotIndication defaulting to false, the robot/people-group badges on avatars are now hidden for everyone by default, and users who enable 'hide bot indication' get the badges shown — the exact inverse of the intended behavior in the user_avatar.vue template (v-if="showActorTypeIndicator && user?.actor_type === 'Service'/'Group'").
fixed
@ -403,4 +403,5 @@userAdminData,}),),users.map((user) => {useUsersStore().updateUserAdminData(user.id, user)return useUsersStore().findUser(user.id)}),),[P2] Admin Users tab crashes: listed users are never hydrated, so fetchUsers returns undefined items
The old vuex action called fetchUserIfMissing before attaching admin data; the migration calls updateUserAdminData(user.id, user) directly, which now (after the guard) only warns and returns for users missing from the store, and then returns useUsersStore().findUser(user.id) — undefined for any user the admin has not already viewed. list.js renders :key="getKey(item)" with getKey = (item) => item.id and users_tab.vue passes :user-id="item.id", so the List render throws TypeError and the admin Users tab fails for exactly the scenario the earlier review flagged. Verified empirically with a probe test against the real store: fetchUsers returns [undefined] and item.id throws. The same missing hydration makes getUserData/setUsersActivationStatus/setUsersApprovalStatus/setUsersSuggestionStatus no-op silently for unknown users, so moderation actions won't update the UI.
fixed
Not sure if it is related to this MR, but max_id might not be getting incremented for Home TL when websockets are enabled. When I leave FE idle to accumulate new posts, the first batch after clicking on "Show new" are new posts, but when scrolling down the next pages are hours behind (probably FE load time).
checked and
max_idis definitively getting updated. it is likely that websocket missed a few messages (i've seen it not notify about some statuses before), creating "holes" in timeline./oc review
opencode-agent:job=72:generation=1
⚠ Blocked — stale target (no review published)
This job's authorized target is PR !3555 at head
users-statuses-pinia@8d49ef11ff72294e9b790f018fac3f415cadae92(basedevelop@7e8f63c71420812f53b48edae68954b02d64eb17).Immediately before publication, a live re-check showed the branch head has moved to
456f25e94e5c76548118965972179a11798af1fc, so the claimed head state changed mid-flight. Per this job's constraints the completed review of8d49ef11must not be published as a current review artifact, and nothing else was mutated.Status:
opencode-fix-botatgit.pleroma.social(verified twice, before and after)pleroma/pleroma-fe!3555, open, not merged, title/body/base unchanged and matching the claim8d49ef11ff72…vs live456f25e94e5c…→ review publication blocked as staleopencode-agent:job=72:generation=1existed (checked comments and reviews); nothing was updated or duplicatedA re-run of this job against the new head can pick up the completed review findings.
opencode-agent:job=72:generation=1
/oc review
The thing is that there were more than 200 new posts. Scrolling down and loading new pages didn't use posts it got from websockets but instead called the Home TL API for them presumably with an old max_id which returned old posts. I'll try to replicate the issue tomorrow and export network logs.
oh. yeah, that might be an issue, when fetching posts returned pagination sorta overrides data-derived one.
found a bit of an issue - profileConfig is most likely broken now but I'll fix it in next MR
opencode-agent:job=74:generation=1
OpenCode review of current head (
456f25e94e5c76548118965972179a11798af1fc, basedevelop@7e8f63c71420812f53b48edae68954b02d64eb17)Verified at this head:
draft.spec.js("should auto-save when close") occurred in a slow headless-browser run and passed on 4 consecutive re-runs — timing-sensitive, not a deterministic failure.fetchUserIfMissinginadmin_settings.js) and the invertedhideBotIndicationcondition inUserAvatar.account_actionschat route param,pendingMessagesIndexkeyed byidempotency_key(previously read a nonexistentmessage.idempotencyKeyIndexproperty — genuine fix),timelineNamemethods→computed conversion, andlists.setListsrebuildingallListsObject.Three pinpoint findings remain:
[P2] "Show new" leaves a stale fetch-older cursor after a WS backlog — next older pages come from hours ago (reproduced)
This is the pagination hole phnt described earlier in this thread. In
showNewStatuses(src/stores/timelines.js:434) the order is truncated to the 50 newest and bounds are recomputed byupdateTimelineExtremes, but its guardolder = minNew < timeline.minIdonly ever movesminIdbackwards. When many statuses accumulated over WS while idle,last(timeline.order)is newer than the pre-idle cursor, sotimeline.minIdstays at the hours-old position.fetchOlderthen requestsmaxId = <hours-old id>(src/stores/fetchers/timeline_fetcher.js:43), so the next pages are hours behind and the entire accumulated backlog (between the 50 shown and the stale cursor) is skipped — matching phnt's report and the suspicion in the latest comment here.Reproduced against the real store with a scratch unit test: set
minId = id(80), accumulate 200 WS statuses (id(501)…id(700)), press "Show new" → order is correctly the 50 newest (id(700)…id(651)), butminIdstaysid(80)instead of advancing toid(651)(the oldest visible status). Note the string comparison only behaves for equal-length snowflake ids — short ids in tests will accidentally pass.Suggested fix (one line, after the truncation in
showNewStatuses):(or allow forward movement in
updateTimelineExtremesvia an explicitforce), plus a regression test like the above.[P2] Chat-from-search still broken:
chat_new.jspushes the removedrecipient_idparamThe
Missing required param "chatUserId"error phnt hit was fixed for the profile "Message" button (a9ce0eechangedaccount_actions.jstochatUserId), butchat_new.js:54(goToChat) still pushes{ name: 'chat', params: { recipient_id: user.id } }— nousername, nochatUserId— against the route/users/:username/chats/:chatUserId(src/boot/routes.js:293). Creating a chat with a new user from search results (the exact flow phnt reported) still throws. The same latent bug exists on develop, but this PR's fix patched only one of the two call sites.[P3] Debug leftover:
console.login thetimelineNamecomputedsrc/components/timeline_menu/timeline_menu.js:71logs on every lists-timeline render (landed ince93f51and survived the following "lint" commit).Overall
Structurally sound migration; the store-level test suite passes and everything from previous rounds is resolved at this head. Remaining work is three small, well-localized fixes — the stale fetch-older cursor is the only behavioral regression beyond chat-from-search. Manually exercise "Show new" after a long idle with WS on, and chat creation from search, before merge.
OH
hope it's gonna be the last one
/oc review
opencode-agent:job=76:generation=1
OpenCode review of head
8e63f189fcaae7462ec8fbd9a1db51bbab9414d4(basedevelop@7e8f63c71420812f53b48edae68954b02d64eb17)Reviewed the full base→head diff (216 files, +7887/−5680) with focus on the new
statuses/timelines/streaming/notifications/usersstores, fetchers, and the three newest commits (f2f15a0c02extremes-overwrite option,32c27b85b0chat creation fix,8e63f189fccleanup). Every finding below was verified against the code at this exact head.Verification summary
P1 — regressions
checkSeenBeforeuses lodashfirst()on aSet—src/stores/timelines.js:345-350.knownRepeatsvalues areSets, but_.first(new Set(['a','b']))→undefined, sofirst(knownRepeats) !== statusIdis always true oncesize > 1; the "show the oldest reprööt" branch never fires. SincepopulateRepeats(line 269) runs for the whole batch before the per-status loop, any initial load/poll delivering 2+ repeats of one status adds every wrapper toignoredIds(line 308) and the post disappears entirely (the original usually isn't in the fetched window). The repeat maps are also never pruned, so this sticks for the session. Fix:[...knownRepeats][0]/knownRepeats.values().next().value.src/stores/fetchers/timeline_fetcher.js:27-31,55,107-113+src/components/timeline/timeline.js:175-178+src/components/timeline/timeline.vue:97-107.fetchAndUpdatesetsloadingOlder = truebefore theif (older && bottomedOut.value) returnguard, so the flag is never cleared by the.finally.bottomedOutlives in the fetcher closure andclearTimeline(which the Reload path calls, also clearingorder→count === 0) can't reset it. Result: empty timeline + infinite spinner; if the WS reconnected meanwhile (polling stopped), it never self-heals. Fix: checkbottomedOutbefore setting flags, and reset it in the reload path.P2 — clear bugs
timeline.maxId—src/stores/timelines.js:281-290+313-323+417-433.updateTimelineExtremesruns before the order insert andonStreamMessagepassespagination = {}, somaxNew = first(timeline.order)is the pre-insert (previous) newest:maxIdlags one batch behind for the whole streaming session (old code derived extremes from the incoming batch). When the WS drops, the catch-up poll uses stalesinceId; with >20 missed statuses the hole is permanent (later polls only movesince_idforward), and the duplicate-heavy poll also tripsreloadNeeded(≥20 rule) spuriously. This is the remaining piece of the reported "max_id not incremented with websockets enabled" issue —f2f15a0c02'sforcefix repairsmaxIdonly at "Show new" time (that part works; post-Show-new pagination is now contiguous). Fix: update extremes from the inserted batch (after the unshift), like notifications already do per-id.entity_normalizer.service.js:389-390+timelines.js:419-426. Bookmarks are the only non-flakeId timeline; when the Link header lacksnext/prev(final page),Number.parseInt(undefined)→NaN, andpagination.maxId ?? last(...)doesn't filterNaN(??≠ old falsy check).minIdnever advances → "load older" refetches the final page forever,bottomedOutnever set, "no more statuses" never shown, and ≥20 responses re-trigger "Reload" spuriously.src/stores/streaming.js. (a)stopSocket()(line 131) doesthis.socket.close()unguarded → TypeError if togglinguseStreamingApioff before any socket exists (settings pathgeneral_tab.js:78; anonymous sessions or setting-off-at-login). (b)onClose(line 231) schedules the reconnectsetTimeoutwith no stored handle and nothing ever cancels it — logging out during a retry window firesinitSocketwith the now-cleared token (anonymous socket, bogus "connection established" toast, possible retry loop);initSocketalso replacesthis.socketwithout closing a live previous one (the oldenableMastoSocketsstate guard was dropped), so a failedrevokeTokenlogout can leave two sockets feeding the same handlers. Fix: generation-token/identity check in handlers, cancel timer instopSocket,this.socket?.close()+ close-existing-first ininitSocket.src/components/user_list_popover/user_list_popover.js:26-31+.vue:13-16.usersCappedmapsfindUser(id)overaccount_idswithout filtering; reacting users are only added to the store by the async@showfetch, so the first (pre-fetch) render containsundefinedentries →:key="user.id"render TypeError. Needs.filter(Boolean)or per-id loading state.src/components/conversation/conversation.js:623-633(new method).mounted()→updateVirtualHeight()→nextTick→this.status.id, but on a direct status-link loadstatus(allStatuses.get(statusId)) is stillundefined→ TypeError. Guard for missing status.showReasonMutedThreadtypomainSatus—src/components/status/status.js:152-157.this.mainSatus.reblogthrows when evaluated; currently unreferenced (landmine), and.reblogisn't a normalized field anyway (retweeted_status). Fix or drop.P3 — minor
timelines.jsdeactivate removes'message'but the handler was registered as'update'(lines 150/183) — latent leak.olderflag (addNewNotifications(response),notifications_fetcher.js:39) so thepushbranch is dead;POSITIVE_INFINITYsentinels were kept but initial extremes are now''→ empty-stringsinceId/maxIdparams sent where develop sent none (notifications_fetcher.js:81,89).users.js:521,540,557mute/unmute/block optimistic predictions readthis.relationships[id]on aMap(alwaysundefined) — accidentally benign, broken as written. Also: failed user fetches permanently cache the rejected promise (no retry until reload), andusersByNameisn't refreshed on rename.populateRepeatsruns before the argument-mismatch guard (timelines.js:269vs275-279) — a late fetch for user A can pollute user B's fresh repeat maps (rare dedup misfire).'close'andinterfaceshows a success notice for codes 1000/1001) — probably unintended.timelines.requireReloadAll();statusesToDisplayreturns nonexistentthis.visibleStatusIdswhen virtual scrolling is off (timeline.js:105); favorites timeline silently drops itsuserIdargument (ARGUMENT_MAPhas nofavoritesentry — pre-existing).removeFollowRequestaction (matches the declared known issue; badge lingers until the next poll).Verified good / fixed since last review
f2f15a0c02: "Show new" extremes overwrite works; post-reload pagination is contiguous, and the 50-status display cap is restored (showNewStatusesslice) — the reported "Show new" slowdown is addressed.32c27b85b0: chat-creation fix is complete and correct — route paramchatUserIdmatches routes/props, all three navigation sites updated, search results resolve through the users store. The reportedMissing required param "chatUserId"error is fixed.statuses/users/notifications/apimodules anywhere insrc/— the module removal is clean; remaining$storeusage maps to surviving modules.userstream; notifications store advances extremes per-id (the model timelines should copy).Notes
I think I'll fix P1 and P2 bugs and merge, leaving P3 to be fixed in follow-up MR(s)
fixed
fixed
Literally doesn't happen but added a guard against it anyway.
Plugged some holes
this.retryingfixed
fixed
fixed, also clarified separation between
status,mainStatus,repeatedStatusand newly addedrepeatStatus(which givesnullon non-repeats)fixed
fixed
fixed
failed promises are no longer cached
fixed
Ehhhhhh yeah we can remove it.
It was calling
statuses.requireReloadAll()instead oftimelines.requireReloadAll(), now it has a proper call. Also it was broken so I fixed thatfixed
fixed
/oc review
opencode-agent:job=77:generation=1
OpenCode review of head
53498072e59c6d09e94b419cda925bb651cee845(basedevelop@7e8f63c71420812f53b48edae68954b02d64eb17)Reviewed the full base→head diff with focus on the 15 commits added since the last review (
8e63f189), which claim to fix the previously reported findings. Everything below was verified against the code at this exact head.Verification summary
P1 — new regression: websocket never reconnects after an unexpected drop, and logout during a retry window still spawns a socket (
src/stores/streaming.js, commit64cc748a9c)onCloseretry path:The guard is inverted.
retryingis set totruesynchronously right after the timer is scheduled, and nothing can clear it before the timer fires (onOpencan't fire — there is no socket to open; onlystopSocket()clears it). Both directions are wrong:retrying === trueand returns;initSocketis never called again. Realtime stays dead until page reload or a manual settings toggle (timelines silently fall back to 10s polling). Before64cc748a9cthis path reconnected.stopSocket()setsretrying = false, so the pending timer now passes the guard and callsinitSocket()with the cleared token → post-logout socket creation.Fix: invert the check —
if (!this.retrying) return. (ThenonOpen'sretrying = falsere-arms later retries andstopSocket'sretrying = falseaborts pending retries as intended.)P2 — the "failed user fetch caches rejected promise" fix is misplaced and still doesn't work (
src/stores/users.js, commit5788d3e8ec)The new
try { … } catch (e) { map.delete(identifier); throw e }wraps only the post-awaitresult handling, butconst result = await promisesits outside thetry. A rejected fetch therefore throws before the catch ever runs,map.delete(identifier)never executes, and the rejected promise stays cached infetchesIds/fetchesNamesforever — the exact P3 issue this commit was meant to fix. Every laterfetchUserByIdOrNamefor that identifier short-circuits into the stale rejection until reload. The wrapped code (truthy check,users.get) can't realistically throw, so the catch is dead code as placed. Fix: moveconst result = await promiseinside thetry(or clear the cache entry in a.catchbefore rethrowing).P3 — minor
stopSocket()(streaming.js) still does an unguardedthis.socket.close()→ TypeError if ever called with no socket. The newgeneral_tabtoken guard covers the anonymous settings path and logout isstate !== CLOSED-guarded, but a logged-in toggle-off with a never-initialized socket still crashes. One-token hardening:this.socket?.close().initSocket()now throws'Socket already exists!'(5ea41244c1). It's called inside the login success chain (users.js), so a re-login after a failed logout (socket left alive) would make the login action report failure after authentication actually succeeded. Preferconsole.warn+ early return (or closing the existing socket first) over throwing into the login chain.Verified fixed at this head (previously reported at
8e63f189)knownRepeats.values().next().valuereplaces lodashfirst()on a Set (590ca6fb6f); head commit53498072e5additionally makes dedup batch-order-independent by splitting insertion and seen-before classification into two phases (original + repeats arriving in one batch now keep exactly the original).resetBottomedOutadded to the fetcher and called fromclearTimeline/showNewStatuses;loadingOlderis reset before the early return (725b2f5387). CheckedfetchOldercall sites — results aren't chained, so the bare return is safe.maxId(P2):updateTimelineExtremesnow runs after the order insert (b75bd5ae73); traced WS / newer-poll / older-page paths — extremes now move correctly in all three.parseLinkHeaderPaginationomitsmaxId/minIdwhen the Link header lacks them and''sentinels replacePOSITIVE_INFINITY(43ddbfd652). Worst case is one extra empty older fetch beforebottomedOutis set — terminates correctly.user_list_popoverfiltersfindUserresults and caps after (b3754b26c4).updateVirtualHeightguards!this.status(abe6af133f).mainSatustypo/landmine (P2): reworked intomainStatus/repeatStatuscomputeds with correctthread_mutedsemantics (3589a5302e).'update'handler; notifications fetcher passesolderthrough and uses''sentinels (stream and poll paths both verified againstaddNewNotifications(result, older)); mute/unmute/block predictions userelationships.get();populateRepeatsmoved after the argument-mismatch guard; deadsocket_closedtoast/string removed;statusesToDisplaynon-virtual path returns real ids;requireReloadAlliteratesTIMELINESand is wired into quick-filter/clutter/filtering tabs; favorites timeline gets itsuserIdviaARGUMENT_MAP; logged-outcurrentUser?.idguards in follow_card/user_card/appearance_tab/general_tab.Notes
Fixed both P1 and P2. Replaced hard-crashes in P3 with console.error. It shoudn't happen but not worth crashing over, even if it masks underlying problem.
/oc review
opencode-agent:job=78:generation=1
OpenCode review of head
7b58a96ecee24e97fe6e5daf7bba6484449e403a(basedevelop@7e8f63c71420812f53b48edae68954b02d64eb17)This round covers the four commits added since the last review (
53498072e5):b864dfb056(whoops),7b037ef60f(invert check),c8593a9668(P3),7b58a96ece(lint) — onlysrc/stores/users.jsandsrc/stores/streaming.jschanged. Everything below was verified against the code at this exact head.Verification summary
All three previously reported findings are fixed at this head
7b037ef60f). The guard is nowif (!this.retrying) return. Traced both directions: after an unexpected close the pending timer now passes the guard and callsinitSocket()(realtime recovers;onOpenresetsretrying/multiplier so later retries re-arm), andstopSocket()during the retry window setsretrying = falseso the timer aborts — no socket after logout. Intended closures (1000/1001) still reset everything without scheduling a retry.b864dfb056).const result = await promisemoved inside thetry; a rejection now reaches the catch, deletes thefetchesIds/fetchesNamesentry, logs and rethrows. Later lookups for that identifier retry instead of short-circuiting into a stale rejection. The 404→null path still resolves (and stays cached) as before, which is the intended no-retry-storm behavior.c8593a9668, formatting in7b58a96ece).initSocketno longer throws"Socket already exists!"into the login success chain — it logs, force-closes the stray socket inside its own try/catch, and proceeds, so a re-login after a failed logout can no longer fail the login action after authentication succeeded.stopSocketnow usesthis.socket?.close()with a log instead of a hard TypeError on a never-initialized socket.[P3] Every automatic reconnect logs a spurious "Socket already exists!" error (new, cosmetic)
The unexpected-close branch of
onClose(src/stores/streaming.js:246-267) never clearsthis.socket— onlystopSocket()nulls it. So when the retry timer fires,initSocket()(src/stores/streaming.js:111-123) finds the dead ProcessedWS still set, enters the "Socket already exists! This shouldn't happen!" branch, and logs a misleading console.error on every reconnect after a network drop (the very scenario the retry loop exists for). Behavior is correct —ProcessedWS.close()on an already-closed socket is a spec no-op and doesn't re-fire the close event, so there's no state clobbering — but the log noise will send anyone debugging realtime down the wrong path. One-liner: setthis.socket = nullin the retry branch ofonClose(or after the force-close ininitSocket), keeping the force-close for genuinely live strays. Non-blocking.Notes
error.tatusCode === 401typo in the login catch (src/stores/users.js:710) produces the wrong error message for bad passwords — pre-existing on develop (came in via1ca0ffb1f0), not introduced by this MR, drive-by fix material only.Overall
Everything flagged in the previous rounds is resolved at this head; the only remaining item is one cosmetic P3 (spurious reconnect log). From this side the MR is merge-ready — manually exercising a reconnect after a network blip and a logout-during-retry remains worthwhile before merge.
Last reviewed commit:
7b58a96eceeI'll look deeper into streaming/sockets some time later, this is already better than what we have