Users/Statuses/Notifications Pinia migration and refactor #3555

Merged
hj merged 210 commits from users-statuses-pinia into develop 2026-09-01 16:32:32 +00:00
Member

What changed underneath

  • statuses notifications and users vuex modules migrated to pinia.
  • statuses store split into:
    • statuses, responsible for keeping index of all known statuses, updaing them and interacting with them
    • timelines, responsible for handling timelines, their pagination etc.
  • Added new store streaming responsible for (mastoapi) websocket. Taken mostly from the api module.
    • Instead of directly calling other stores and updating their states, other modules subscribe to streaming store and listen for socket events.
    • Subscribes to stream events on WS based on internal subscriptions:
      • Other stores can subscribe to specific stream events (i.e. user stream), or just global subscription that listens to everything (useful for i.e. interface store listening for connects/disconnects to display notification)
      • Store will subscribe to relevant streams on connection and also in-flight, i.e. if user switches timelines it will unsubscribe from old timeline and subscribe to new
  • Timelines store relieves statuses and api stores from having to keep tabs on timeline state
    • Stuff like min/max ids are still kept within each timeline object
    • Timeline fetcher and socket subscription are now directly associated with a timeline
    • Timeline itself handles whether it should be polling or waiting for push events
    • Fetcher is reused for fetching older posts in the timeline, no need to keep track of arguments and credentials externally
    • Fetcher now has separate states for fetching older posts and newer posts
    • Bottomed-out state is now part of the store, not component. Fetcher won't fetch older posts if bottomed out
    • Some timelines (friends and also DMs) are made persistent, i.e. they will keep their state even if user navigates away from the page
      • Currently timeline argument (i.e. userId) is also assigned to timeline, so for now it is impossible to keep two separate user timelines.
    • Timelines can be activated and deactivated:
      • Handled in Timeline component on created/unmounted hooks as well as in watcher for props change.
      • Deactivated timeline drops its entire state to a blank one.
      • Persistent timelines are never deactivated
      • This also makes embedded (i.e. user profile) timelines easier to manage.
      • Timeline tracks state of whether it's streaming or fetching separately (technically both can happen at same time but current logic doesn't allow for it)
      • There is no explicit "active" state, but timeline is assumed to be active if its fetcher isn't null
  • All timeline pages now use Timeline component instead of custom wrappers.
  • Pinned statuses are now their own timeline embedded above non-pinned on user profile
  • Virtual scrolling is completely handled by Conversation (updating its height) and Timeline (determining what should be visible), virtual scrolling data isn't saved in store anymore.
  • Everything that uses Status or User now should pull data from store by id instead of reading embedded version which might be outdated

Known issues:

  • There are still a few vuex stores remain but they're out of scope of this MR and will be migrated to pinia in a later MR
  • Follow requests might be somewhat broken (until we migrate them to pinia)
### What changed underneath - `statuses` `notifications` and `users` vuex modules migrated to pinia. - `statuses` store split into: - `statuses`, responsible for keeping index of all known statuses, updaing them and interacting with them - `timelines`, responsible for handling timelines, their pagination etc. - Added new store `streaming` responsible for (mastoapi) websocket. Taken mostly from the `api` module. - Instead of directly calling other stores and updating their states, other modules subscribe to `streaming` store and listen for socket events. - Subscribes to stream events on WS based on internal subscriptions: - Other stores can subscribe to specific stream events (i.e. `user` stream), or just global subscription that listens to everything (useful for i.e. `interface` store listening for connects/disconnects to display notification) - Store will subscribe to relevant streams on connection and also in-flight, i.e. if user switches timelines it will unsubscribe from old timeline and subscribe to new - Timelines store relieves `statuses` and `api` stores from having to keep tabs on timeline state - Stuff like min/max ids are still kept within each timeline object - Timeline fetcher and socket subscription are now directly associated with a timeline - Timeline itself handles whether it should be polling or waiting for push events - Fetcher is reused for fetching older posts in the timeline, no need to keep track of arguments and credentials externally - Fetcher now has separate states for fetching older posts and newer posts - Bottomed-out state is now part of the store, not component. Fetcher won't fetch older posts if bottomed out - Some timelines (friends and also DMs) are made persistent, i.e. they will keep their state even if user navigates away from the page - Currently timeline argument (i.e. userId) is also assigned to timeline, so for now it is impossible to keep two separate user timelines. - Timelines can be activated and deactivated: - Handled in `Timeline` component on created/unmounted hooks as well as in watcher for props change. - Deactivated timeline drops its entire state to a blank one. - Persistent timelines are never deactivated - This also makes embedded (i.e. user profile) timelines easier to manage. - Timeline tracks state of whether it's streaming or fetching separately (technically both can happen at same time but current logic doesn't allow for it) - There is no explicit "active" state, but timeline is assumed to be active if its `fetcher` isn't `null` - All timeline pages now use `Timeline` component instead of custom wrappers. - Pinned statuses are now their own timeline embedded above non-pinned on user profile - Virtual scrolling is completely handled by Conversation (updating its height) and Timeline (determining what should be visible), virtual scrolling data isn't saved in store anymore. - Everything that uses Status or User now should pull data from store by id instead of reading embedded version which might be outdated ### Known issues: - There are still a few vuex stores remain but they're out of scope of this MR and will be migrated to pinia in a later MR - Follow requests might be somewhat broken (until we migrate them to pinia)
hj added 41 commits 2026-08-14 17:10:07 +00:00
fix extra indexes not working
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline failed
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline failed
ci/woodpecker/pr/test-e2e unknown status
5ef72358aa
hj changed title from users-statuses-pinia to Users/Statuses/Notifications Pinia migration and refactor 2026-08-14 17:10:23 +00:00
changelogs
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline failed
ci/woodpecker/pr/test-e2e unknown status
3af372f327
hj added 4 commits 2026-08-18 00:32:24 +00:00
replacements
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was canceled
ci/woodpecker/pr/test-e2e unknown status
95a087c313
hj added 8 commits 2026-08-18 23:03:37 +00:00
timeline fetcher improvements
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/test Pipeline failed
ci/woodpecker/pr/test-e2e unknown status
d7f19d75f1
fixes
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/test Pipeline failed
ci/woodpecker/pr/test-e2e unknown status
1f1c0c1300
hj added 2 commits 2026-08-19 20:47:02 +00:00
fix tests
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline failed
25d0d16dbb
remove related dispatches
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline failed
ae5984d2f1
fix thread view
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline failed
4bba716409
fix build
Some checks failed
ci/woodpecker/pr/changelog Pipeline is pending
ci/woodpecker/pr/lint Pipeline is pending
ci/woodpecker/pr/test-e2e Pipeline is pending
ci/woodpecker/pr/test Pipeline is pending
ci/woodpecker/pr/build Pipeline was canceled
3741d8df72
Merge remote-tracking branch 'origin/develop' into users-statuses-pinia
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline failed
36672031f9
fix posting
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline failed
16b93b2bf6
hj added 2 commits 2026-08-20 21:13:59 +00:00
restore clearTimeline
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline failed
a9c0edc92e
hj added 4 commits 2026-08-21 13:15:19 +00:00
fix reply replying to repeat instead of repated
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was canceled
e6ef8e8017
fix user suggestor
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was canceled
afdf22675f
fix error on non-status notifications
Some checks failed
ci/woodpecker/pr/test-e2e Pipeline is pending
ci/woodpecker/pr/test Pipeline is pending
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was canceled
d5fefa23fd
fix another error
Some checks failed
ci/woodpecker/pr/changelog Pipeline is pending
ci/woodpecker/pr/lint Pipeline is pending
ci/woodpecker/pr/test-e2e Pipeline is pending
ci/woodpecker/pr/test Pipeline is pending
ci/woodpecker/pr/build Pipeline was canceled
5467bfc84b
lint
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline failed
89dc6d2075
fix settings
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline failed
025c197f8c
fix
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline failed
11595d6882
Member

Can'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

TypeError: Cannot read properties of undefined (reading 'closest')
    at Proxy.mounted (http://localhost:8080/src/components/popover/popover.js?vue&type=script&src=true&lang.js:331:39)
    at http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:3593:87
    at callWithErrorHandling (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:1844:17)
    at callWithAsyncErrorHandling (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:1851:15)
    at hook.__weh.hook.__weh (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:3582:16)
    at flushPostFlushCbs (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:1976:25)
    at flushJobs (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:2004:3)

(logginIn) on /about and /login

TypeError: Cannot read properties of undefined (reading 'loggingIn')
    at Proxy.loggingIn (http://localhost:8080/src/components/login_form/login_form.js?vue&type=script&src=true&lang.js:22:41)
    at Proxy.mappedState (http://localhost:8080/node_modules/.vite/deps/vuex.js?v=38251505:843:43)
    at refreshComputed (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:357:26)
    at get value (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:1305:3)
    at Object.get [as loggingIn] (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:4035:17)
    at Object.get (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:3773:15)
    at Proxy._sfc_render (http://localhost:8080/src/components/login_form/login_form.vue:50:34)
    at renderComponentRoot (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:5732:35)
    at ReactiveEffect.componentUpdateFn [as fn] (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:5032:41)
    at ReactiveEffect.run (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:234:16)

develop doesn't have this issue

Can'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 ``` TypeError: Cannot read properties of undefined (reading 'closest') at Proxy.mounted (http://localhost:8080/src/components/popover/popover.js?vue&type=script&src=true&lang.js:331:39) at http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:3593:87 at callWithErrorHandling (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:1844:17) at callWithAsyncErrorHandling (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:1851:15) at hook.__weh.hook.__weh (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:3582:16) at flushPostFlushCbs (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:1976:25) at flushJobs (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:2004:3) ``` (logginIn) on /about and /login ``` TypeError: Cannot read properties of undefined (reading 'loggingIn') at Proxy.loggingIn (http://localhost:8080/src/components/login_form/login_form.js?vue&type=script&src=true&lang.js:22:41) at Proxy.mappedState (http://localhost:8080/node_modules/.vite/deps/vuex.js?v=38251505:843:43) at refreshComputed (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:357:26) at get value (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:1305:3) at Object.get [as loggingIn] (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:4035:17) at Object.get (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:3773:15) at Proxy._sfc_render (http://localhost:8080/src/components/login_form/login_form.vue:50:34) at renderComponentRoot (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:5732:35) at ReactiveEffect.componentUpdateFn [as fn] (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:5032:41) at ReactiveEffect.run (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:234:16) ``` develop doesn't have this issue
hj added 5 commits 2026-08-24 13:09:00 +00:00
cleanup & lint
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline failed
d521a06dad
render placeholder avatars while fetching mentions
Some checks failed
ci/woodpecker/pr/test-e2e Pipeline is pending
ci/woodpecker/pr/test Pipeline is pending
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was canceled
0b40236eef
hj force-pushed users-statuses-pinia from 0b40236eef
Some checks failed
ci/woodpecker/pr/test-e2e Pipeline is pending
ci/woodpecker/pr/test Pipeline is pending
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was canceled
to 034b2c3bf9
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline failed
ci/woodpecker/pr/test-e2e unknown status
2026-08-24 13:35:22 +00:00
Compare
hj added 6 commits 2026-08-24 14:35:28 +00:00
more precise height detection
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/test Pipeline failed
ci/woodpecker/pr/test-e2e unknown status
fdea276b80
changelog
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/test Pipeline failed
ci/woodpecker/pr/test-e2e unknown status
6946f7d201
hj added 2 commits 2026-08-24 14:57:24 +00:00
fix admin view
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/test Pipeline failed
ci/woodpecker/pr/test-e2e unknown status
74319a5d4c
hj added 2 commits 2026-08-24 15:10:24 +00:00
fix tests
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline failed
a392f8e4a9
login/logout/register woes
All checks were successful
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was successful
556a37e836
fix page conversation
All checks were successful
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was successful
4a3c5fade9
Author
Member

@phnt wrote in #3555 (comment):

Can'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

TypeError: Cannot read properties of undefined (reading 'closest')
    at Proxy.mounted (http://localhost:8080/src/components/popover/popover.js?vue&type=script&src=true&lang.js:331:39)
    at http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:3593:87
    at callWithErrorHandling (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:1844:17)
    at callWithAsyncErrorHandling (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:1851:15)
    at hook.__weh.hook.__weh (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:3582:16)
    at flushPostFlushCbs (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:1976:25)
    at flushJobs (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:2004:3)

(logginIn) on /about and /login

TypeError: Cannot read properties of undefined (reading 'loggingIn')
    at Proxy.loggingIn (http://localhost:8080/src/components/login_form/login_form.js?vue&type=script&src=true&lang.js:22:41)
    at Proxy.mappedState (http://localhost:8080/node_modules/.vite/deps/vuex.js?v=38251505:843:43)
    at refreshComputed (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:357:26)
    at get value (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:1305:3)
    at Object.get [as loggingIn] (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:4035:17)
    at Object.get (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:3773:15)
    at Proxy._sfc_render (http://localhost:8080/src/components/login_form/login_form.vue:50:34)
    at renderComponentRoot (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:5732:35)
    at ReactiveEffect.componentUpdateFn [as fn] (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:5032:41)
    at ReactiveEffect.run (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:234:16)

develop doesn't have this issue

should be fixed now

@phnt wrote in https://git.pleroma.social/pleroma/pleroma-fe/pulls/3555#issuecomment-117956: > Can'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 > > ```text > TypeError: Cannot read properties of undefined (reading 'closest') > at Proxy.mounted (http://localhost:8080/src/components/popover/popover.js?vue&type=script&src=true&lang.js:331:39) > at http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:3593:87 > at callWithErrorHandling (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:1844:17) > at callWithAsyncErrorHandling (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:1851:15) > at hook.__weh.hook.__weh (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:3582:16) > at flushPostFlushCbs (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:1976:25) > at flushJobs (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:2004:3) > ``` > > (logginIn) on /about and /login > > ```text > TypeError: Cannot read properties of undefined (reading 'loggingIn') > at Proxy.loggingIn (http://localhost:8080/src/components/login_form/login_form.js?vue&type=script&src=true&lang.js:22:41) > at Proxy.mappedState (http://localhost:8080/node_modules/.vite/deps/vuex.js?v=38251505:843:43) > at refreshComputed (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:357:26) > at get value (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:1305:3) > at Object.get [as loggingIn] (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:4035:17) > at Object.get (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:3773:15) > at Proxy._sfc_render (http://localhost:8080/src/components/login_form/login_form.vue:50:34) > at renderComponentRoot (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:5732:35) > at ReactiveEffect.componentUpdateFn [as fn] (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:5032:41) > at ReactiveEffect.run (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:234:16) > ``` > > develop doesn't have this issue should be fixed now
fixes
Some checks failed
ci/woodpecker/pr/test-e2e Pipeline is pending
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was canceled
e128e21315
Merge remote-tracking branch 'origin/develop' into users-statuses-pinia
All checks were successful
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was successful
f76070640a
fix
All checks were successful
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was successful
opencode/review OpenCode execution completed
8c568d3653
Author
Member

/oc review

/oc review
opencode-bot left a comment

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:20

Suggested replacement (manual):

showDesktopNotification(opts)

[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, so reactedWith reads 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:64

Suggested replacement (manual):

if (hasNoAccounts) {
  return await useStatusesStore().fetchEmojiReactions(this.status.id)
}

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

<!-- opencode-pull-review:v1:4e589eeef7a9df654e62093138413376 --> <!-- opencode-pull-review-result:v1:72666f25ad59ea56a91dfd18014bba6f --> ## 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:20` **Suggested replacement (manual):** showDesktopNotification(opts) ### [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`, so `reactedWith` reads 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:64` **Suggested replacement (manual):** if (hasNoAccounts) { return await useStatusesStore().fetchEmojiReactions(this.status.id) } ## 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 &&
Owner

[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, .value returns undefined). timeline.js:242 !this.timeline.fetcher.loadingOlder.value is 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):

if (
  !this.timeline.fetcher.loadingOlder &&
  window.innerHeight + window.pageYOffset >= height - 750
) {
**[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, `.value` returns undefined). timeline.js:242 `!this.timeline.fetcher.loadingOlder.value` is 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):** if ( !this.timeline.fetcher.loadingOlder && window.innerHeight + window.pageYOffset >= height - 750 ) { <!-- opencode-pull-review-finding:v1:9e39b5b0390cdb7a3ff4a0865e3d4220 -->
Author
Member

fixed

fixed
hj marked this conversation as resolved
@ -0,0 +223,4 @@
case 'follow_request':
break
default:
this.markSingleNotificationAsSeen({ id })
Owner

[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 seen flag 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):

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 `seen` flag 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):** this.markSingleNotificationAsSeen(id) <!-- opencode-pull-review-finding:v1:da481e79a78fcd0fae5296503e442ede -->
Author
Member

fixed

fixed
hj marked this conversation as resolved
@ -0,0 +31,4 @@
statuses,
})
const output = {}
Owner

[P2] Hashtag search results dropped; concat(undefined) can crash the results view

stores/search.js:22-40 builds the return value with only statuses and accounts; the old Vuex action returned the full API payload including hashtags. 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 hit undefined.history (lastHistoryRecord), breaking the whole results view.

Suggested replacement (manual):

const { accounts, statuses, hashtags } = data
// ...
output.hashtags = hashtags ?? []
return output
**[P2] Hashtag search results dropped; concat(undefined) can crash the results view** stores/search.js:22-40 builds the return value with only `statuses` and `accounts`; the old Vuex action returned the full API payload including `hashtags`. 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 hit `undefined.history` (lastHistoryRecord), breaking the whole results view. **Suggested replacement (manual):** const { accounts, statuses, hashtags } = data // ... output.hashtags = hashtags ?? [] return output <!-- opencode-pull-review-finding:v1:08ae100c7a11b30ece4a528db65a1d09 -->
Author
Member

fixed

fixed
hj marked this conversation as resolved
@ -0,0 +64,4 @@
this.socket = socket
},
resetStatuses() {
this.socket.et.removeEventListener('update', this.socket.handleUpdate)
Owner

[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.et on null and throws, aborting logout cleanup (token not cleared, cookie not removed).

Suggested replacement (manual):

Re-attach the socket on login (call useStatusesStore().attachSocket() in loginUser after resetting state), and guard resetStatuses with `if (this.socket) { ... }` before removing listeners.
**[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.et` on null and throws, aborting logout cleanup (token not cleared, cookie not removed). **Suggested replacement (manual):** Re-attach the socket on login (call useStatusesStore().attachSocket() in loginUser after resetting state), and guard resetStatuses with `if (this.socket) { ... }` before removing listeners. <!-- opencode-pull-review-finding:v1:3cf344520dde363313acc6a8b81c6f75 -->
Author
Member

fixed

fixed
hj marked this conversation as resolved
@ -0,0 +152,4 @@
// implicit: if oldTimestamp is undefined this will still be false
if (oldTimestamp > timestamp) return [existing, false] // not overwriting old data with new
const newStatus = {
Owner

[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: true is 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):

const newStatus = {
  ...old,
  ...Object.fromEntries(Object.entries(neu).filter(([, v]) => v !== undefined)),
  user,
}
**[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: true` is 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):** const newStatus = { ...old, ...Object.fromEntries(Object.entries(neu).filter(([, v]) => v !== undefined)), user, } <!-- opencode-pull-review-finding:v1:c18a4fb852599eafd4aa119cab0bd513 -->
Author
Member

should be fixed now

should be fixed now
hj marked this conversation as resolved
@ -0,0 +102,4 @@
}
if (this.state === WSConnectionStatus.JOINED) {
this.socket.unsubscribe(...this.getSubArgs(stream))
Owner

[P1] Logout aborts when streaming is connected: removeSubscriber dereferences undefined stream

streaming.js:104-106: for global subscribers (the notifications socket { et }), stream is 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):

if (stream && 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 }`), `stream` is 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):** if (stream && this.state === WSConnectionStatus.JOINED) { this.socket.unsubscribe(...this.getSubArgs(stream)) } <!-- opencode-pull-review-finding:v1:fe00f3aba2aa5eab7b047e0188b76e9a -->
Author
Member

fixed

fixed
hj marked this conversation as resolved
@ -0,0 +105,4 @@
const openHandler = () => this.onStreamConnect(timelineName, argument)
const closeHandler = () =>
this.onStreamDisconnect(timelineName, argument)
const messageHandler =
Owner

[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) reads event.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):

const messageHandler = ({ data: status }) =>
  this.addStatusesToTimeline(timelineName, argument, { statuses: [status.id] })

and remove the same 'update' name used in addEventListener; drop onStreamMessage or adapt it to the StreamMessageEvent shape.
**[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) reads `event.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):** const messageHandler = ({ data: status }) => this.addStatusesToTimeline(timelineName, argument, { statuses: [status.id] }) and remove the same 'update' name used in addEventListener; drop onStreamMessage or adapt it to the StreamMessageEvent shape. <!-- opencode-pull-review-finding:v1:734c42a36bb92dffcd8cd58913fc921b -->
Author
Member

fixed

fixed
hj marked this conversation as resolved
@ -0,0 +610,4 @@
credentials: useOAuthStore().token,
})
user.blockIds = new Set()
Owner

[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):

In loginUser, set user.credentials = accessToken before storing the user (as the old Vuex action did), or replace every currentUser.credentials consumer with useOAuthStore().token; also fix who_to_follow_panel.js to import/call useUsersStore() directly and make interface.js unregisterPushNotifications use useOAuthStore().token.
**[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):** In loginUser, set user.credentials = accessToken before storing the user (as the old Vuex action did), or replace every currentUser.credentials consumer with useOAuthStore().token; also fix who_to_follow_panel.js to import/call useUsersStore() directly and make interface.js unregisterPushNotifications use useOAuthStore().token. <!-- opencode-pull-review-finding:v1:52ae385c2d722273c0c0c2cfe15ef941 -->
Author
Member

replaced remaining currentUser.credentials with useOAuthStore().token as it's intended

replaced remaining `currentUser.credentials` with `useOAuthStore().token` as it's intended
hj marked this conversation as resolved
remove remaining currentUser.credentials
Some checks failed
ci/woodpecker/pr/build Pipeline failed
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/test unknown status
ci/woodpecker/pr/test-e2e unknown status
4d5fcff514
hj added 11 commits 2026-08-25 17:15:54 +00:00
hj added 2 commits 2026-08-25 17:32:30 +00:00
attach chats socket
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/test Pipeline failed
ci/woodpecker/pr/test-e2e unknown status
283b91826f
fix chats
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/test Pipeline failed
ci/woodpecker/pr/test-e2e unknown status
f030328cd1
hj added 2 commits 2026-08-25 18:42:58 +00:00
fix marking chat as read
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/test Pipeline failed
ci/woodpecker/pr/test-e2e unknown status
61f25e4f0f
lint
All checks were successful
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was successful
fd536f4921
Author
Member

[P2] Chat streaming and chat desktop notifications broken

fixed

[P2] Emoji reaction refetch condition inverted: fetches when data present, skips when missing

should be fixed

>[P2] Chat streaming and chat desktop notifications broken fixed >[P2] Emoji reaction refetch condition inverted: fetches when data present, skips when missing should be fixed
fix unsub again
All checks were successful
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was successful
opencode/review OpenCode execution completed
f1e9c56569
Author
Member

/oc review

/oc review

/oc rerun

/oc rerun

/oc rerun

/oc rerun
opencode-bot left a comment

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 status globals 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 status global — backend-muted statuses are never muted or hidden

The refactor dropped the old const { status } = this destructuring but kept (status.muted && !status.thread_muted). In the browser status resolves 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:322

Suggested replacement (manual):

return (
  (this.status.muted && !this.status.thread_muted) ||
  (reblog?.muted && !reblog.thread_muted) ||
  relationship.muting ||
  relationshipReblog?.muting
)

[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:28

Suggested replacement (manual):

const statuses = preTickedStatuses.concat(
  [...useStatusesStore().allStatuses.values()].filter(
    (status) =>
      status.user.id === userId && !preTickedIds.includes(status.id),
  ),
)

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

<!-- opencode-pull-review:v1:4e589eeef7a9df654e62093138413376 --> <!-- opencode-pull-review-result:v1:b123a054b885d577f44a8b19cc1e3370 --> ## 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 `status` globals 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 `status` global — backend-muted statuses are never muted or hidden The refactor dropped the old `const { status } = this` destructuring but kept `(status.muted && !status.thread_muted)`. In the browser `status` resolves 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:322` **Suggested replacement (manual):** return ( (this.status.muted && !this.status.thread_muted) || (reblog?.muted && !reblog.thread_muted) || relationship.muting || relationshipReblog?.muting ) ### [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:28` **Suggested replacement (manual):** const statuses = preTickedStatuses.concat( [...useStatusesStore().allStatuses.values()].filter( (status) => status.user.id === userId && !preTickedIds.includes(status.id), ), ) ## 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
Owner

[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):

fetchStatusHistory({ id, status, credentials }) with status passed in from the caller (e.g. useStatusesStore().allStatuses.get(id)), keeping 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):** fetchStatusHistory({ id, status, credentials }) with status passed in from the caller (e.g. useStatusesStore().allStatuses.get(id)), keeping item.originalStatus = status <!-- opencode-pull-review-finding:v1:7bf892762cf1c04df16f6cc12be7eb39 -->
Author
Member

adding id field seem to be just enough to make <Status> render properly for our needs.

adding `id` field seem to be just enough to make `<Status>` render properly for our needs.
hj marked this conversation as resolved
@ -120,1 +126,4 @@
)
if (this.testMode) return
this.deactivate()
Owner

[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):

Add `this.detachSocket()` before/next to `this.deactivate()` in unmounted() (guarding for testMode)
**[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):** Add `this.detachSocket()` before/next to `this.deactivate()` in unmounted() (guarding for testMode) <!-- opencode-pull-review-finding:v1:3bcdbf2e24afdd91becfabd3ce5cd492 -->
Author
Member

added

added
hj marked this conversation as resolved
@ -78,3 +78,3 @@
}))
if (this.suggestionsEnabled) {
getWhoToFollow(this)
getWhoToFollow()
Owner

[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.forEach on 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):

getWhoToFollow(this)  // in both mounted() and the user watcher
**[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.forEach` on 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):** getWhoToFollow(this) // in both mounted() and the user watcher <!-- opencode-pull-review-finding:v1:631f206e2d7f17c4e75899657b550125 -->
Author
Member

refactored this component to use methods and this instead of loose functions

refactored this component to use methods and `this` instead of loose functions
hj marked this conversation as resolved
@ -11,1 +7,3 @@
return
const validActions = {
sync_config: new Set(['setPreference']),
interface: new Set(['setNotificationPermission', 'setLoginStatus']),
Owner

[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):

Either re-add a setLoginStatus(true/false) action to the interface store called from onLogin/onLogout (keeping the plugin logic), or call useInterfaceStore().unregisterPushNotifications() directly in users.logout() and registerPushNotifications() after successful login
**[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):** Either re-add a setLoginStatus(true/false) action to the interface store called from onLogin/onLogout (keeping the plugin logic), or call useInterfaceStore().unregisterPushNotifications() directly in users.logout() and registerPushNotifications() after successful login <!-- opencode-pull-review-finding:v1:febd9c2f996c660e7f52a36be3308886 -->
Author
Member

added onLogin and onLogout to list of valid actions

added `onLogin` and `onLogout` to list of valid actions
hj marked this conversation as resolved
@ -66,2 +78,3 @@
})
chats.forEach((updatedChat) => {
result.data.forEach((updatedChat) => {
Owner

[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) where chat is 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):

addNewChats: after updating a known chat, keep the old `const isNewMessage = chat.lastMessage?.id !== updatedChat.lastMessage?.id` check and call `if (isNewMessage && chat.unread) maybeShowChatNotification(chat)`; updateChat: call `maybeShowChatNotification(chat ?? 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) where `chat` is 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):** addNewChats: after updating a known chat, keep the old `const isNewMessage = chat.lastMessage?.id !== updatedChat.lastMessage?.id` check and call `if (isNewMessage && chat.unread) maybeShowChatNotification(chat)`; updateChat: call `maybeShowChatNotification(chat ?? updatedChat)` <!-- opencode-pull-review-finding:v1:143b3e6f47d465182ad99c6a6843f9da -->
Author
Member

refactored store, now chat insert/update is unified and always calls maybeShowChatNotification, which now also checks for unreads.

refactored store, now chat insert/update is unified and always calls `maybeShowChatNotification`, which now also checks for unreads.
hj marked this conversation as resolved
@ -0,0 +77,4 @@
return { statuses, pagination }
})
.catch((error) => {
if (error.statusCode === 403 && timeline === 'favorites') {
Owner

[P2] Favorites 403 fallback is dead code: object compared to string and 'favourites'/'favorites' name mismatch

timeline_fetcher catches error.statusCode === 403 && timeline === 'favorites', but timeline is now the timeline object, so the branch never runs and pleromaPublicFavouritesAvailable is never set false; additionally timelines.activate() guards timelineName === '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):

if (error.statusCode === 403 && timeline.name === 'favorites') {
  useInstanceCapabilitiesStore().pleromaPublicFavouritesAvailable = false
  return
}
— and in timelines.activate() change 'favourites' to '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'`, but `timeline` is now the timeline object, so the branch never runs and pleromaPublicFavouritesAvailable is never set false; additionally timelines.activate() guards `timelineName === '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):** if (error.statusCode === 403 && timeline.name === 'favorites') { useInstanceCapabilitiesStore().pleromaPublicFavouritesAvailable = false return } — and in timelines.activate() change 'favourites' to 'favorites' <!-- opencode-pull-review-finding:v1:c00c3aa806218f466e43bab3677f71cd -->
Author
Member

cleaned up favorites/favourites confusion, also fixed favorites tab being permanently disabled

cleaned up favorites/favourites confusion, also fixed favorites tab being permanently disabled
hj marked this conversation as resolved
@ -0,0 +530,4 @@
const removed = new Set()
this.allStatuses.forEach((status) => {
if (status.user.id === userId) {
this.allStatuses.delete(status.id)
Owner

[P2] Blocking a user leaves dangling conversation indexes — thread views throw TypeError

wipeUserStatuses deletes statuses from allStatuses but never removes their ids from the conversations Map. Conversation's conversation computed does [...conversation.keys()].map(k => allStatuses.get(k)).toSorted(sortById); for a blocked user's status the map yields undefined and sortById reads .type on 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):

In wipeUserStatuses, also clean the conversation index: `this.conversations.forEach((set) => removed.forEach((id) => set.delete(id)))` (and delete empty conversation sets)
**[P2] Blocking a user leaves dangling conversation indexes — thread views throw TypeError** wipeUserStatuses deletes statuses from allStatuses but never removes their ids from the `conversations` Map. Conversation's `conversation` computed does `[...conversation.keys()].map(k => allStatuses.get(k)).toSorted(sortById)`; for a blocked user's status the map yields undefined and sortById reads `.type` on 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):** In wipeUserStatuses, also clean the conversation index: `this.conversations.forEach((set) => removed.forEach((id) => set.delete(id)))` (and delete empty conversation sets) <!-- opencode-pull-review-finding:v1:10ac320717370e26e3812e8d8715fff5 -->
Author
Member

wipe now also wipes conversations, in addition added missing wipeStatuses from notifications store

wipe now also wipes conversations, in addition added missing `wipeStatuses` from notifications store
hj marked this conversation as resolved
hj added 9 commits 2026-08-26 11:34:04 +00:00
detach socket
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline failed
ci/woodpecker/pr/test-e2e unknown status
b485ccd82e
push notifications fix
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline failed
ci/woodpecker/pr/test-e2e unknown status
ad52027760
hj added 2 commits 2026-08-26 12:02:35 +00:00
dang, ai was right
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/test Pipeline failed
ci/woodpecker/pr/test-e2e unknown status
97e3f24bdf
Author
Member

[P2] status.js references bare status global — backend-muted statuses are never muted or hidden

Refactored that piece of code to use this.repeatedStatus and this.repeater instead

[P2] Report-user modal crashes: openUserReportingModal reads the removed vuex statuses module

Added stausesPerUser index to statuses store for easier search (no need to iterate over entire allStatuses) and refactored report modal/store to utilize that.

>[P2] status.js references bare status global — backend-muted statuses are never muted or hidden Refactored that piece of code to use `this.repeatedStatus` and `this.repeater` instead >[P2] Report-user modal crashes: openUserReportingModal reads the removed vuex statuses module Added `stausesPerUser` index to `statuses` store for easier search (no need to iterate over entire `allStatuses`) and refactored report modal/store to utilize that.
hj added 2 commits 2026-08-26 12:15:47 +00:00
fix tests
All checks were successful
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was successful
5ff886ba94
comment
All checks were successful
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was successful
opencode/review OpenCode execution completed
ae127a8d51
Author
Member

/oc review

/oc review
opencode-bot left a comment

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:43

Suggested replacement (manual):

if (store.$id === 'interface') {
  if (actionName === 'setNotificationPermission') {
    permissionGranted = args[0] === 'granted'
  } else if (actionName === 'setLoginStatus' || actionName === 'onLogin') {
    user = true
  } else if (actionName === 'onLogout') {
    user = false
  } else {
    return
  }
}

and in users.logout(), move oauth.clearToken() (or capture the token) so onLogout-driven unregistration still has a valid token.

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

<!-- opencode-pull-review:v1:4e589eeef7a9df654e62093138413376 --> <!-- opencode-pull-review-result:v1:56a3bd66b4b7d716d839db5b5296b435 --> ## 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 `return`s 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:43` **Suggested replacement (manual):** if (store.$id === 'interface') { if (actionName === 'setNotificationPermission') { permissionGranted = args[0] === 'granted' } else if (actionName === 'setLoginStatus' || actionName === 'onLogin') { user = true } else if (actionName === 'onLogout') { user = false } else { return } } and in users.logout(), move oauth.clearToken() (or capture the token) so onLogout-driven unregistration still has a valid token. ## 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 user
useStatusesStore().wipeUserStatuses(status.user.id)
Owner

[P2] Admin account deletion throws: deleteUsers uses undefined status instead of userId

The migrated forEach callback calls useStatusesStore().wipeUserStatuses(status.user.id), but the loop variable is userId; status is 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):

resultUserIds.data.forEach((userId) => {
  useStatusesStore().wipeUserStatuses(userId)
  // Users are technically never deleted, just deactivated
  // so there's no real need to delete them from store.
})
**[P2] Admin account deletion throws: deleteUsers uses undefined `status` instead of userId** The migrated forEach callback calls `useStatusesStore().wipeUserStatuses(status.user.id)`, but the loop variable is `userId`; `status` is 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):** resultUserIds.data.forEach((userId) => { useStatusesStore().wipeUserStatuses(userId) // Users are technically never deleted, just deactivated // so there's no real need to delete them from store. }) <!-- opencode-pull-review-finding:v1:1bcd8ea20caff91922c4ec0fc192ead5 -->
Author
Member

fixed

fixed
hj marked this conversation as resolved
@ -40,2 +42,2 @@
stopFetchingChats() {
this.setChatListFetcher(null)
startFetching() {
this.fetcher = () => promiseInterval(() => this.fetchChats(), 5000)
Owner

[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's this.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):

startFetching() {
  if (this.fetcher) return
  this.fetcher = promiseInterval(() => this.fetchChats(), 5000)
},
stopFetching() {
  this.fetcher?.stop()
  this.fetcher = null
},
resetChats() {
  this.data = new Map()
  this.stopFetching()
},
**[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's `this.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):** startFetching() { if (this.fetcher) return this.fetcher = promiseInterval(() => this.fetchChats(), 5000) }, stopFetching() { this.fetcher?.stop() this.fetcher = null }, resetChats() { this.data = new Map() this.stopFetching() }, <!-- opencode-pull-review-finding:v1:7b1c871f9d3f03aee322855eabd16867 -->
Author
Member

fixed

fixed
hj marked this conversation as resolved
@ -99,3 +88,1 @@
this.chatList.data.unshift(updatedChat)
}
this.chatList.idStore[updatedChat.id] = updatedChat
maybeShowChatNotification(chat ?? updatedChat)
Owner

[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?.id before 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):

updateChat(updatedChat) {
  const chat = this.data.get(updatedChat.id)
  if (chat) {
    const isNewMessage = chat.lastMessage?.id !== updatedChat.lastMessage?.id
    chat.lastMessage = updatedChat.lastMessage
    chat.unread = updatedChat.unread
    chat.updated_at = updatedChat.updated_at
    if (!isNewMessage) return
  } else {
    this.data.set(updatedChat.id, updatedChat)
  }
  maybeShowChatNotification(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?.id` before 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):** updateChat(updatedChat) { const chat = this.data.get(updatedChat.id) if (chat) { const isNewMessage = chat.lastMessage?.id !== updatedChat.lastMessage?.id chat.lastMessage = updatedChat.lastMessage chat.unread = updatedChat.unread chat.updated_at = updatedChat.updated_at if (!isNewMessage) return } else { this.data.set(updatedChat.id, updatedChat) } maybeShowChatNotification(chat ?? updatedChat) }, <!-- opencode-pull-review-finding:v1:c940631b824eddf2828dcd7aed8c05af -->
Author
Member

fixed

fixed
hj marked this conversation as resolved
@ -0,0 +46,4 @@
}
args.withMuted = !hideMutedPosts
if (loggedIn && REPLY_VISIBILITY_TIMELINES.has(timeline)) {
Owner

[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 checks REPLY_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):

if (loggedIn && REPLY_VISIBILITY_TIMELINES.has(timeline.name)) {
  args.replyVisibility = replyVisibility
}
**[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 checks `REPLY_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):** if (loggedIn && REPLY_VISIBILITY_TIMELINES.has(timeline.name)) { args.replyVisibility = replyVisibility } <!-- opencode-pull-review-finding:v1:61f691d42fbe00a91b68041897bc61ea -->
Author
Member

fixed

fixed
hj marked this conversation as resolved
@ -31,1 +21,3 @@
)
const preTickedIds = new Set(statusIds)
// There shouldn't be a case where this is undefined
const userAllStatusesIds = useStatusesStore().statusesPerUser.get(userId)
Owner

[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.forEach on undefined), reachable from blockUser and admin_settings deleteUsers once those paths execute. Fix both with ?? new Set().

Suggested replacement (manual):

const userAllStatusesIds = useStatusesStore().statusesPerUser.get(userId) ?? new Set()

and in statuses.js wipeUserStatuses:
const removed = this.statusesPerUser.get(userId) ?? new Set()
**[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.forEach` on undefined), reachable from blockUser and admin_settings deleteUsers once those paths execute. Fix both with `?? new Set()`. **Suggested replacement (manual):** const userAllStatusesIds = useStatusesStore().statusesPerUser.get(userId) ?? new Set() and in statuses.js wipeUserStatuses: const removed = this.statusesPerUser.get(userId) ?? new Set() <!-- opencode-pull-review-finding:v1:503ab9de8fe17962e6357bf2b7a97e61 -->
Author
Member

fixed

fixed
hj marked this conversation as resolved
@ -0,0 +185,4 @@
timeline.paused = true
console.debug('[Timelines] Pausing timeline', name)
if (timeline.fetcher && timeline.fetching) {
timeline.fetcher.stopFetching()
Owner

[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-guard interval.value in timeline_fetcher.stopFetching, and don't resume/restart fetchers after a completed logout.

Suggested replacement (manual):

pause(name) {
  const timeline = this[name]
  timeline.paused = true
  if (timeline.fetcher && timeline.fetching) {
    timeline.fetcher.stopFetching()
    timeline.fetching = false
  }
},

and in timeline_fetcher.js:
const stopFetching = () => {
  interval.value?.stop()
  interval.value = null
}
**[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-guard `interval.value` in timeline_fetcher.stopFetching, and don't resume/restart fetchers after a completed logout. **Suggested replacement (manual):** pause(name) { const timeline = this[name] timeline.paused = true if (timeline.fetcher && timeline.fetching) { timeline.fetcher.stopFetching() timeline.fetching = false } }, and in timeline_fetcher.js: const stopFetching = () => { interval.value?.stop() interval.value = null } <!-- opencode-pull-review-finding:v1:7941332e986758e09a32e2e8a2a41c42 -->
Author
Member

fetching should remain as true, 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.

`fetching` should remain as `true`, 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.
hj marked this conversation as resolved
Member

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.

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

Also the "checking for new TL posts" spinner can collide with the TL name on mobile when the "Show new/reload" button is present.

Also the "checking for new TL posts" spinner can collide with the TL name on mobile when the "Show new/reload" button is present.
Author
Member

I noticed slowdown too. I think mutating maps is slow in pinia maybe.

I noticed slowdown too. I think mutating maps is slow in pinia maybe.
Member

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):

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: - https://fluffytail.org/notice/B9lcOv0iSgksfcnS1Q (FE develop + this MR, newt's avatar is never requested) - https://pl.borked.technology/notice/B9lcP29HFRa0hLTGBE (FE bundled in current BE stable, newt's avatar is requested and rendered) - lain's post with the issue in question: https://lain.com/objects/0c81d175-e1e5-4ed2-8405-1c61cf8e955b emoji reacts (also has the favs/repeats issue): - https://fluffytail.org/notice/B9jprYUo9yKkKPwBGq - https://pl.borked.technology/notice/B9jprbFpEJwslxZ8Ay - bara's post: https://clubcyberia.co/objects/9b8a4a95-0aa7-45d5-b396-9a0fccbacd14
hj added 6 commits 2026-08-26 15:07:27 +00:00
fix reply visibility timelines
All checks were successful
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was successful
b112bf11c2
Author
Member

[P2] Web Push subscription still never unregistered on logout (plugin early-returns for onLogin/onLogout)

fixed

[P2] Web Push subscription still never unregistered on logout (plugin early-returns for onLogin/onLogout)

fixed

>[P2] Web Push subscription still never unregistered on logout (plugin early-returns for onLogin/onLogout) fixed >[P2] Web Push subscription still never unregistered on logout (plugin early-returns for onLogin/onLogout) fixed
fix reports
Some checks failed
ci/woodpecker/pr/changelog Pipeline is pending
ci/woodpecker/pr/lint Pipeline is pending
ci/woodpecker/pr/test-e2e Pipeline is pending
ci/woodpecker/pr/test Pipeline is pending
ci/woodpecker/pr/build Pipeline was canceled
56dbf10949
hj force-pushed users-statuses-pinia from 56dbf10949
Some checks failed
ci/woodpecker/pr/changelog Pipeline is pending
ci/woodpecker/pr/lint Pipeline is pending
ci/woodpecker/pr/test-e2e Pipeline is pending
ci/woodpecker/pr/test Pipeline is pending
ci/woodpecker/pr/build Pipeline was canceled
to a44b60dbc8
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was successful
2026-08-26 15:13:03 +00:00
Compare

review from sol:

Yes — there are three separate regressions.

1. “Show new” slowdown

This is probably not Pinia Map mutation, as hj suspected. The migration removed the old 50-status display cap:

  • develop exposed statuses.slice(0, 50) when “Show new” was clicked.
  • PR head exposes every accumulated status by copying the complete statusIds set in src/stores/timelines.js:349-354.
  • Statuses continue accumulating while the tab is idle/unfocused.
  • Clicking “Show new” therefore creates/reconciles one Conversation component 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; develop would expose 50. That directly explains why the delay grows with idle time.

The fix should restore a bounded window while updating order, statusIds, visibleStatusIds, and minId consistently. Simply slicing visibleStatusIds could 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 use timeline.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-name rather than .timeline-title.

For the reported combination, the smallest fix is probably:

v-if="timeline.fetcher.loadingNewer && !showLoadButton"

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:

  • AvatarList and UserListPopover now pass only user.id.
  • UserAvatar resolves that ID exclusively through the canonical users store.
  • Favorites, repeats, and emoji-reaction fetches retain complete user objects on the status but never register those users in the 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 call useUsersStore().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

  • Temporary Chromium Vitest probes: 2 passed, confirming the unbounded 500-item Show-new transition and the missing canonical interaction user.
  • Public example API payloads inspected and confirmed.
  • No PR code was changed or upstream comment posted.
  • Findings were documented and archived in the local investigation issue, commits 2111506230 and 1d0115d433.
review from sol: Yes — there are three separate regressions. ### 1. “Show new” slowdown This is probably **not Pinia `Map` mutation**, as hj suspected. The migration removed the old 50-status display cap: - `develop` exposed `statuses.slice(0, 50)` when “Show new” was clicked. - PR head exposes **every accumulated status** by copying the complete `statusIds` set in `src/stores/timelines.js:349-354`. - Statuses continue accumulating while the tab is idle/unfocused. - Clicking “Show new” therefore creates/reconciles one `Conversation` component 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; `develop` would expose 50. That directly explains why the delay grows with idle time. The fix should restore a bounded window while updating `order`, `statusIds`, `visibleStatusIds`, and `minId` consistently. Simply slicing `visibleStatusIds` could 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 use `timeline.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-name` rather than `.timeline-title`. For the reported combination, the smallest fix is probably: ```vue v-if="timeline.fetcher.loadingNewer && !showLoadButton" ``` 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: - `AvatarList` and `UserListPopover` now pass only `user.id`. - `UserAvatar` resolves that ID exclusively through the canonical users store. - Favorites, repeats, and emoji-reaction fetches retain complete user objects on the status but **never register those users in the 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 call `useUsersStore().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 - Temporary Chromium Vitest probes: **2 passed**, confirming the unbounded 500-item Show-new transition and the missing canonical interaction user. - Public example API payloads inspected and confirmed. - No PR code was changed or upstream comment posted. - Findings were documented and archived in the local investigation issue, commits `2111506230` and `1d0115d433`.
even more login/logout woes!
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was successful
4e1d6f704d
documentation
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was successful
7c46ba462f
fix chats fetcher again
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was successful
713eb828c8
Author
Member

Oh boy, our persist plugin is misbehaving, it's being called way too many times

Oh boy, our persist plugin is misbehaving, it's being called way too many times
remove persist plugin from users
Some checks failed
ci/woodpecker/pr/changelog Pipeline is pending
ci/woodpecker/pr/lint Pipeline is pending
ci/woodpecker/pr/test-e2e Pipeline is pending
ci/woodpecker/pr/test Pipeline is pending
ci/woodpecker/pr/build Pipeline was canceled
9676f95cf4
lint
All checks were successful
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was successful
18bdc84200
Author
Member

pinia was persisting (cloning!) entire users store because it was configured to persist lastLoginName which is no longer used, I removed it and at least notifications load much faster now.

pinia was persisting (cloning!) entire users store because it was configured to persist `lastLoginName` which is no longer used, I removed it and at least notifications load much faster now.
Author
Member

This is probably not Pinia Map mutation, as hj suspected. The migration removed the old 50-status display cap:

oh, true. i removed it because it messed with pagination, i guess it needs to be implemented properly

>This is probably not Pinia Map mutation, as hj suspected. The migration removed the old 50-status display cap: oh, true. i removed it because it messed with pagination, i guess it needs to be implemented properly
limit timeline to 50 statuses when updating visible statuses
All checks were successful
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was successful
8f6b83fc3d
Member

@hj wrote in #3555 (comment):

This is probably not Pinia Map mutation, as hj suspected. The migration removed the old 50-status display cap:

oh, true. i removed it because it messed with pagination, i guess it needs to be implemented properly

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.

@hj wrote in https://git.pleroma.social/pleroma/pleroma-fe/pulls/3555#issuecomment-118078: > > This is probably not Pinia Map mutation, as hj suspected. The migration removed the old 50-status display cap: > > oh, true. i removed it because it messed with pagination, i guess it needs to be implemented properly 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.
better display of empty/initially-loading timeline
All checks were successful
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was successful
d8e530581d
hj added 3 commits 2026-08-27 17:28:21 +00:00
reprööt deduplication
Some checks failed
ci/woodpecker/pr/test-e2e Pipeline is pending
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/test Pipeline was canceled
4733eb7872
don't show "up to date" indicator together with loadingNewer indicator on mobile
Some checks failed
ci/woodpecker/pr/changelog Pipeline is pending
ci/woodpecker/pr/lint Pipeline is pending
ci/woodpecker/pr/test-e2e Pipeline is pending
ci/woodpecker/pr/test Pipeline is pending
ci/woodpecker/pr/build Pipeline was canceled
422c12c026
Merge remote-tracking branch 'origin/develop' into users-statuses-pinia
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was successful
572842fb79
hj added 2 commits 2026-08-27 17:58:20 +00:00
fix virtual scrolling
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was canceled
a8166dff96
Author
Member
  1. Mobile spinner/title collision

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.

>2. Mobile spinner/title collision 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.
lint
All checks were successful
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was successful
2297cf8ab0
fixes for reprööts/favorites display
Some checks failed
ci/woodpecker/pr/changelog Pipeline is pending
ci/woodpecker/pr/lint Pipeline is pending
ci/woodpecker/pr/test-e2e Pipeline is pending
ci/woodpecker/pr/test Pipeline is pending
ci/woodpecker/pr/build Pipeline was canceled
5854b222bd
lint
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline failed
ci/woodpecker/pr/test-e2e unknown status
d64821e93b
hj added 2 commits 2026-08-28 13:15:56 +00:00
process users in reactions + tests
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was canceled
87f8f3fcaa
Author
Member
  1. Missing interaction avatars

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/favoritedBy property of statuses.

Mobile spinner/title collision

When "show new" button is shown loading newer spinner doesn't show up at all.

>3. Missing interaction avatars 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`/`favoritedBy` property of statuses. >Mobile spinner/title collision When "show new" button is shown loading newer spinner doesn't show up at all.
lint
All checks were successful
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was successful
opencode/review OpenCode execution completed
b124d2b8ea
Author
Member

/oc review

/oc review
opencode-bot left a comment

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

<!-- opencode-pull-review:v1:4e589eeef7a9df654e62093138413376 --> <!-- opencode-pull-review-result:v1:2caf9993f40295b0cb74df4e35f59445 --> ## 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`_
Lines 183-185
@ -196,3 +185,4 @@
}
return sortAndFilterConversation(conversation, this.status)
return [...conversation.keys()]
.map((k) => useStatusesStore().allStatuses.get(k))
.toSorted(sortById)
Owner

[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 new conversation computed 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.

      return [...conversation.keys()]
        .map((k) => useStatusesStore().allStatuses.get(k))
        .filter((status) =>
          this.status.retweeted_status
            ? status.type === 'retweet' ||
              status.id !== this.status.retweeted_status.id
            : status.type !== 'retweet',
        )
        .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 new `conversation` computed 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. ```suggestion return [...conversation.keys()] .map((k) => useStatusesStore().allStatuses.get(k)) .filter((status) => this.status.retweeted_status ? status.type === 'retweet' || status.id !== this.status.retweeted_status.id : status.type !== 'retweet', ) .toSorted(sortById) ``` <!-- opencode-pull-review-finding:v1:c965bededdfdf259e89f78d404f04e5d -->
Author
Member

This is factually incorrect on modern backend

  • Repeats do not have pleroma.conversation_id so they don't get added to conversations
  • Opening conversation by repeat's ID and by repeated's ID yields correct conversation display with no reblogs

But I'm guessing this might have been different behavior on older backend so we can bring back filter

This is factually incorrect on modern backend - Repeats do not have `pleroma.conversation_id` so they don't get added to conversations - Opening conversation by repeat's ID and by repeated's ID yields correct conversation display with no reblogs But I'm guessing this might have been different behavior on older backend so we can bring back filter
hj marked this conversation as resolved
@ -16,2 +17,3 @@
name: 'UserListPopover',
props: ['users'],
props: {
userIds: Set,
Owner

[P2] Emoji-reaction "who reacted" popover broken: still passes removed users prop

UserListPopover's prop was renamed from users (array of user objects) to userIds (Set of ids) in this PR, and status.vue was updated, but emoji_reactions.vue line 55 still binds :users="accountsForEmoji[reaction.name]". userIds stays undefined, so when the popover content renders on hover the template's userIds.size throws 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.

**[P2] Emoji-reaction "who reacted" popover broken: still passes removed `users` prop** UserListPopover's prop was renamed from `users` (array of user objects) to `userIds` (Set of ids) in this PR, and status.vue was updated, but emoji_reactions.vue line 55 still binds `:users="accountsForEmoji[reaction.name]"`. `userIds` stays undefined, so when the popover content renders on hover the template's `userIds.size` throws 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. <!-- opencode-pull-review-finding:v1:da95ff69f38678778af2bcd808fdf62a -->
Author
Member

There's a bit of a confusion in emoji reacts - status data has account_ids but reaction data has accounts, cleaned it up and now status data always has account_ids which we use in popover.

There's a bit of a confusion in emoji reacts - status data has `account_ids` but reaction data has `accounts`, cleaned it up and now status data always has `account_ids` which we use in popover.
hj marked this conversation as resolved
@ -93,1 +77,4 @@
updateChat(updatedChat) {
const chat = this.data.get(updatedChat.id)
if (chat) {
const isNewMessage = chat.lastMessage !== updatedChat.lastMessage
Owner

[P2] Chat desktop notifications re-fire every 5s poll while a chat stays unread

Re-raised: the earlier fix compares chat.lastMessage !== updatedChat.lastMessage by 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.

        const isNewMessage = chat.lastMessage?.id !== updatedChat.lastMessage?.id
**[P2] Chat desktop notifications re-fire every 5s poll while a chat stays unread** Re-raised: the earlier fix compares `chat.lastMessage !== updatedChat.lastMessage` by 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. ```suggestion const isNewMessage = chat.lastMessage?.id !== updatedChat.lastMessage?.id ``` <!-- opencode-pull-review-finding:v1:8413cbb39793007efaac75d1f94ad9fe -->
Author
Member

fixed manually

fixed manually
hj marked this conversation as resolved
Lines 65-67
@ -0,0 +86,6 @@
},
resume() {
this.paused = false
if (this.fetcher && this.fetching) {
this.startFetching('Notifications resumed')
}
Owner

[P2] Failed logout permanently stops notification polling and stacks duplicate chats pollers

notifications.pause() calls stopFetching(), which clears fetching, but resume() only restarts when this.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.

      if (this.fetcher) {
        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 when `this.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. ```suggestion if (this.fetcher) { this.startFetching('Notifications resumed') } ``` <!-- opencode-pull-review-finding:v1:6bb823fad0f9603cabe716df7afea177 -->
Author
Member

pause/resume incorrectly calls this.stopFetching/this.startFetching while it should be calling fetcher directly, bypassing resetting this.fetching property.

pause/resume incorrectly calls `this.stopFetching`/`this.startFetching` while it should be calling fetcher directly, bypassing resetting `this.fetching` property.
hj marked this conversation as resolved
Lines 65-68
@ -0,0 +96,7 @@
removeSubscriber(subscriber) {
const { stream } = subscriber
this.subscribers.delete(subscriber)
if (stream) {
this.subscriptions.get(stream.name).delete(stream.argument)
}
Owner

[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 as subscribers, but removeSubscriber only deletes from subscribers and only cleans subscriptions for 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.

      this.subscribers.delete(subscriber)
      if (stream) {
        this.subscriptions.get(stream.name).delete(stream.argument)
      } else {
        this.globalSubscriptions.delete(subscriber)
      }
**[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 as `subscribers`, but removeSubscriber only deletes from `subscribers` and only cleans `subscriptions` for 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. ```suggestion this.subscribers.delete(subscriber) if (stream) { this.subscriptions.get(stream.name).delete(stream.argument) } else { this.globalSubscriptions.delete(subscriber) } ``` <!-- opencode-pull-review-finding:v1:e390fb74726091111a3f447774f52571 -->
Author
Member

fixed

fixed
hj marked this conversation as resolved
Lines 213-216
@ -0,0 +209,7 @@
updateUserAdminData(id, data) {
const user = this.users.get(id)
user.adminData = data
user.deactivated = !data.is_active
user.tags = new Set(data.tags)
},
Owner

[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 = data with 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.

    updateUserAdminData(id, data) {
      const user = this.users.get(id)
      if (!user) return

      user.adminData = data
      user.deactivated = !data.is_active
      user.tags = new Set(data.tags)
    },
@ -217,4 +217,8 @@
user.adminData = data
user.deactivated = !data.is_active
user.tags = new Set(data.tags)
},
updateUserAdminData(id, data) {
const user = this.users.get(id)
if (!user) return
user.adminData = data
user.deactivated = !data.is_active
user.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 = data` with 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. ```suggestion updateUserAdminData(id, data) { const user = this.users.get(id) if (!user) return user.adminData = data user.deactivated = !data.is_active user.tags = new Set(data.tags) }, ``` <!-- opencode-pull-review-finding:v1:3ba75792cefb04f70673201c490eaf4f -->
Author
Member

applied suggestion manually (lines are messed up)

applied suggestion manually (lines are messed up)
hj marked this conversation as resolved
hj added 4 commits 2026-08-28 17:17:35 +00:00
subscriptions leak
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/test Pipeline failed
ci/woodpecker/pr/test-e2e unknown status
7b27f337c6
UserLink
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/test Pipeline failed
ci/woodpecker/pr/test-e2e unknown status
224b9faa02
lost line
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/test Pipeline failed
ci/woodpecker/pr/test-e2e unknown status
e0d6d5ac85
hj added 13 commits 2026-08-28 20:05:34 +00:00
wtf
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/test Pipeline failed
ci/woodpecker/pr/test-e2e unknown status
8f9635f5b3
lint
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline failed
ci/woodpecker/pr/test-e2e unknown status
42900f6336
fix
Some checks failed
ci/woodpecker/pr/changelog Pipeline is pending
ci/woodpecker/pr/lint Pipeline is pending
ci/woodpecker/pr/test-e2e Pipeline is pending
ci/woodpecker/pr/test Pipeline is pending
ci/woodpecker/pr/build Pipeline was canceled
75d38d3f2b
fix
Some checks are pending
ci/woodpecker/pr/test-e2e Pipeline is pending
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
4114349b0d
hj left a comment

.

.
forgejo is poop
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was canceled
7feb071d0b
lint
All checks were successful
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was successful
opencode/review OpenCode execution completed
5dc3a87a97
Author
Member

/oc review

/oc review
Member

Creating a chat with a new user from search results in the following error (after clicking on the user):

Error: Missing required param "chatUserId"
    at Object.stringify (http://localhost:8080/node_modules/.vite/deps/vue-router.js?v=38251505:1653:18)
    at Object.resolve (http://localhost:8080/node_modules/.vite/deps/vue-router.js?v=38251505:1848:19)
    at resolve (http://localhost:8080/node_modules/.vite/deps/vue-router.js?v=38251505:2347:32)
    at pushWithRedirect (http://localhost:8080/node_modules/.vite/deps/vue-router.js?v=38251505:2403:44)
    at Object.push (http://localhost:8080/node_modules/.vite/deps/vue-router.js?v=38251505:2377:10)
    at Proxy.goToChat (http://localhost:8080/src/components/chat_new/chat_new.js?vue&type=script&src=true&lang.js:54:20)
    at http://localhost:8080/src/components/chat_new/chat_new.vue:55:60
    at http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:8324:10
    at callWithErrorHandling (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:1844:17)
    at callWithAsyncErrorHandling (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:1851:15)

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:
image

Clicking on the wrench for Lists in the sidebar from an open chat results in the following error:

TypeError: this.globalSubscriptions.remove is not a function
    at Proxy.removeSubscriber (http://localhost:8080/src/stores/streaming.js:103:34)
    at Proxy.wrappedAction (http://localhost:8080/node_modules/.vite/deps/pinia.js?v=38251505:4789:14)
    at store.<computed> (http://localhost:8080/node_modules/.vite/deps/pinia.js?v=38251505:4476:40)
    at Proxy.detachSocket (http://localhost:8080/src/components/chat_view/chat_view.js?vue&type=script&src=true&lang.js:295:27)
    at Proxy.unmounted (http://localhost:8080/src/components/chat_view/chat_view.js?vue&type=script&src=true&lang.js:130:10)
    at http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:3593:87
    at callWithErrorHandling (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:1844:17)
    at callWithAsyncErrorHandling (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:1851:15)
    at hook.__weh.hook.__weh (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:3582:16)
    at flushPostFlushCbs (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:1976:25)

Lists don't have their name in the TL header (the name is shown when list is first created though):
image

Creating a chat with a new user from search results in the following error (after clicking on the user): ``` Error: Missing required param "chatUserId" at Object.stringify (http://localhost:8080/node_modules/.vite/deps/vue-router.js?v=38251505:1653:18) at Object.resolve (http://localhost:8080/node_modules/.vite/deps/vue-router.js?v=38251505:1848:19) at resolve (http://localhost:8080/node_modules/.vite/deps/vue-router.js?v=38251505:2347:32) at pushWithRedirect (http://localhost:8080/node_modules/.vite/deps/vue-router.js?v=38251505:2403:44) at Object.push (http://localhost:8080/node_modules/.vite/deps/vue-router.js?v=38251505:2377:10) at Proxy.goToChat (http://localhost:8080/src/components/chat_new/chat_new.js?vue&type=script&src=true&lang.js:54:20) at http://localhost:8080/src/components/chat_new/chat_new.vue:55:60 at http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:8324:10 at callWithErrorHandling (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:1844:17) at callWithAsyncErrorHandling (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:1851:15) ``` 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: ![image](/attachments/c3ac6f30-371a-48cf-9e06-e3adc4da1395) Clicking on the wrench for Lists in the sidebar from an open chat results in the following error: ``` TypeError: this.globalSubscriptions.remove is not a function at Proxy.removeSubscriber (http://localhost:8080/src/stores/streaming.js:103:34) at Proxy.wrappedAction (http://localhost:8080/node_modules/.vite/deps/pinia.js?v=38251505:4789:14) at store.<computed> (http://localhost:8080/node_modules/.vite/deps/pinia.js?v=38251505:4476:40) at Proxy.detachSocket (http://localhost:8080/src/components/chat_view/chat_view.js?vue&type=script&src=true&lang.js:295:27) at Proxy.unmounted (http://localhost:8080/src/components/chat_view/chat_view.js?vue&type=script&src=true&lang.js:130:10) at http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:3593:87 at callWithErrorHandling (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:1844:17) at callWithAsyncErrorHandling (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:1851:15) at hook.__weh.hook.__weh (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:3582:16) at flushPostFlushCbs (http://localhost:8080/node_modules/.vite/deps/vue.runtime.esm-bundler-BaPWM87I.js?v=38251505:1976:25) ``` Lists don't have their name in the TL header (the name is shown when list is first created though): ![image](/attachments/c6c043cc-5ac0-4f13-a0b2-bb7f89bdc4a7)
Author
Member

this.globalSubscriptions.remove

fixed already

>this.globalSubscriptions.remove fixed already
opencode-bot left a comment

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

<!-- opencode-pull-review:v1:4e589eeef7a9df654e62093138413376 --> <!-- opencode-pull-review-result:v1:d7b6fbb3d4acd944d07554051355b315 --> ## 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`_
Lines 43-44
@ -43,0 +40,5 @@
return useUsersStore().findUser(this.userId)
},
showActorTypeIndicator() {
return useMergedConfigStore().mergedConfig.hideBotIndication
},
Owner

[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'").

    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'"). ```suggestion showActorTypeIndicator() { return !useMergedConfigStore().mergedConfig.hideBotIndication }, ``` <!-- opencode-pull-review-finding:v1:ed807f495a1a3baa5bfc4dc87537fa09 -->
Author
Member

fixed

fixed
hj marked this conversation as resolved
Lines 403-407
@ -403,4 +403,5 @@
userAdminData,
}),
),
users.map((user) => {
useUsersStore().updateUserAdminData(user.id, user)
return useUsersStore().findUser(user.id)
}),
),
Owner

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

        items: await Promise.all(
          users.map(async (user) => {
            const fullUser = await useUsersStore().fetchUserIfMissing({
              id: user.id,
            })
            if (fullUser) useUsersStore().updateUserAdminData(user.id, user)
            return fullUser
          }),
        ),
**[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. ```suggestion items: await Promise.all( users.map(async (user) => { const fullUser = await useUsersStore().fetchUserIfMissing({ id: user.id, }) if (fullUser) useUsersStore().updateUserAdminData(user.id, user) return fullUser }), ), ``` <!-- opencode-pull-review-finding:v1:3418bfb9b600072d7280cc520993df3d -->
Author
Member

fixed

fixed
hj marked this conversation as resolved
Member

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

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).
review
All checks were successful
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was successful
8d49ef11ff
Author
Member

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

>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_id` is 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.
Author
Member

/oc review

/oc review
hj added 4 commits 2026-08-31 14:41:16 +00:00
fix chat view pending messages
Some checks failed
ci/woodpecker/pr/changelog Pipeline is pending
ci/woodpecker/pr/lint Pipeline is pending
ci/woodpecker/pr/test-e2e Pipeline is pending
ci/woodpecker/pr/test Pipeline is pending
ci/woodpecker/pr/build Pipeline was canceled
5b55901611
lint
All checks were successful
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was successful
456f25e94e
First-time contributor

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 (base develop @ 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 of 8d49ef11 must not be published as a current review artifact, and nothing else was mutated.

Status:

  • Authenticated as opencode-fix-bot at git.pleroma.social (verified twice, before and after)
  • Target verified: pleroma/pleroma-fe !3555, open, not merged, title/body/base unchanged and matching the claim
  • Head SHA mismatch: claimed 8d49ef11ff72… vs live 456f25e94e5c… → review publication blocked as stale
  • ℹ️ No prior artifact for marker opencode-agent:job=72:generation=1 existed (checked comments and reviews); nothing was updated or duplicated
  • ℹ️ No code changes, branches, or forks were used; nothing was pushed, nothing merged

A re-run of this job against the new head can pick up the completed review findings.

opencode-agent:job=72:generation=1

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` (base `develop` @ `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 of `8d49ef11` must not be published as a current review artifact, and nothing else was mutated. Status: - ✅ Authenticated as `opencode-fix-bot` at `git.pleroma.social` (verified twice, before and after) - ✅ Target verified: `pleroma/pleroma-fe` !3555, open, not merged, title/body/base unchanged and matching the claim - ❌ Head SHA mismatch: claimed `8d49ef11ff72…` vs live `456f25e94e5c…` → review publication blocked as stale - ℹ️ No prior artifact for marker `opencode-agent:job=72:generation=1` existed (checked comments and reviews); nothing was updated or duplicated - ℹ️ No code changes, branches, or forks were used; nothing was pushed, nothing merged A re-run of this job against the new head can pick up the completed review findings. opencode-agent:job=72:generation=1
Author
Member

/oc review

/oc review
Member

checked and max_id is 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.

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.

>checked and max_id is 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. 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.
Author
Member

oh. yeah, that might be an issue, when fetching posts returned pagination sorta overrides data-derived one.

oh. yeah, that might be an issue, when fetching posts returned pagination sorta overrides data-derived one.
Author
Member

found a bit of an issue - profileConfig is most likely broken now but I'll fix it in next MR

found a bit of an issue - profileConfig is most likely broken now but I'll fix it in next MR
First-time contributor

opencode-agent:job=74:generation=1

OpenCode review of current head (456f25e94e5c76548118965972179a11798af1fc, base develop @ 7e8f63c71420812f53b48edae68954b02d64eb17)

Verified at this head:

  • Unit suite is effectively green: all 36 spec files pass. One flake in 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.
  • Both P2s from the previous review round are confirmed fixed: admin Users tab user hydration (fetchUserIfMissing in admin_settings.js) and the inverted hideBotIndication condition in UserAvatar.
  • Recent seam fixes verified correct: account_actions chat route param, pendingMessagesIndex keyed by idempotency_key (previously read a nonexistent message.idempotencyKeyIndex property — genuine fix), timelineName methods→computed conversion, and lists.setLists rebuilding allListsObject.

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 by updateTimelineExtremes, but its guard older = minNew < timeline.minId only ever moves minId backwards. When many statuses accumulated over WS while idle, last(timeline.order) is newer than the pre-idle cursor, so timeline.minId stays at the hours-old position. fetchOlder then requests maxId = <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)), but minId stays id(80) instead of advancing to id(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):

timeline.minId = last(timeline.order) ?? ''

(or allow forward movement in updateTimelineExtremes via an explicit force), plus a regression test like the above.

[P2] Chat-from-search still broken: chat_new.js pushes the removed recipient_id param

The Missing required param "chatUserId" error phnt hit was fixed for the profile "Message" button (a9ce0ee changed account_actions.js to chatUserId), but chat_new.js:54 (goToChat) still pushes { name: 'chat', params: { recipient_id: user.id } } — no username, no chatUserId — 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.

this.$router.push({
  name: 'chat',
  params: { username: this.currentUser.screen_name, chatUserId: user.id },
})

[P3] Debug leftover: console.log in the timelineName computed

src/components/timeline_menu/timeline_menu.js:71 logs on every lists-timeline render (landed in ce93f51 and 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.

opencode-agent:job=74:generation=1 ## OpenCode review of current head (`456f25e94e5c76548118965972179a11798af1fc`, base `develop` @ `7e8f63c71420812f53b48edae68954b02d64eb17`) **Verified at this head:** - Unit suite is effectively green: all 36 spec files pass. One flake in `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. - Both P2s from the previous review round are confirmed fixed: admin Users tab user hydration (`fetchUserIfMissing` in `admin_settings.js`) and the inverted `hideBotIndication` condition in `UserAvatar`. - Recent seam fixes verified correct: `account_actions` chat route param, `pendingMessagesIndex` keyed by `idempotency_key` (previously read a nonexistent `message.idempotencyKeyIndex` property — genuine fix), `timelineName` methods→computed conversion, and `lists.setLists` rebuilding `allListsObject`. **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 by `updateTimelineExtremes`, but its guard `older = minNew < timeline.minId` only ever moves `minId` *backwards*. When many statuses accumulated over WS while idle, `last(timeline.order)` is *newer* than the pre-idle cursor, so `timeline.minId` stays at the hours-old position. `fetchOlder` then requests `maxId = <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)`), but `minId` stays `id(80)` instead of advancing to `id(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`): ```js timeline.minId = last(timeline.order) ?? '' ``` (or allow forward movement in `updateTimelineExtremes` via an explicit `force`), plus a regression test like the above. ### [P2] Chat-from-search still broken: `chat_new.js` pushes the removed `recipient_id` param The `Missing required param "chatUserId"` error phnt hit was fixed for the profile "Message" button (a9ce0ee changed `account_actions.js` to `chatUserId`), but `chat_new.js:54` (`goToChat`) still pushes `{ name: 'chat', params: { recipient_id: user.id } }` — no `username`, no `chatUserId` — 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. ```js this.$router.push({ name: 'chat', params: { username: this.currentUser.screen_name, chatUserId: user.id }, }) ``` ### [P3] Debug leftover: `console.log` in the `timelineName` computed `src/components/timeline_menu/timeline_menu.js:71` logs on every lists-timeline render (landed in ce93f51 and 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.
Author
Member

but its guard older = minNew < timeline.minId only ever moves minId backwards

OH

>but its guard older = minNew < timeline.minId only ever moves minId backwards OH
hj added 2 commits 2026-08-31 17:06:55 +00:00
fix creating a new chat
Some checks failed
ci/woodpecker/pr/lint Pipeline is pending
ci/woodpecker/pr/test-e2e Pipeline is pending
ci/woodpecker/pr/test Pipeline is pending
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was canceled
32c27b85b0
cleanup
All checks were successful
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was successful
8e63f189fc
Author
Member

hope it's gonna be the last one

hope it's gonna be the last one
Author
Member

/oc review

/oc review
First-time contributor

opencode-agent:job=76:generation=1

OpenCode review of head 8e63f189fcaae7462ec8fbd9a1db51bbab9414d4 (base develop @ 7e8f63c71420812f53b48edae68954b02d64eb17)

Reviewed the full base→head diff (216 files, +7887/−5680) with focus on the new statuses/timelines/streaming/notifications/users stores, fetchers, and the three newest commits (f2f15a0c02 extremes-overwrite option, 32c27b85b0 chat creation fix, 8e63f189fc cleanup). Every finding below was verified against the code at this exact head.

Verification summary

  • Unit suite green: all 36 spec files pass (stores: statuses 49, users 56, timelines 19, notifications 17, sync_config 35+1skip, oauth 14, lists 6, user_highlight 14; plus all component/service/boot specs; rich_content 15+1skip). Run per-file because this runner's batched browser mode is flaky — no test failures, only infra noise.
  • Lint: biome clean (688 files); eslint 0 errors (6 style warnings); stylelint clean.

P1 — regressions

  1. Multiply-repeated posts vanish from timelines: checkSeenBefore uses lodash first() on a Setsrc/stores/timelines.js:345-350. knownRepeats values are Sets, but _.first(new Set(['a','b']))undefined, so first(knownRepeats) !== statusId is always true once size > 1; the "show the oldest reprööt" branch never fires. Since populateRepeats (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 to ignoredIds (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.
  2. "Reload" on a bottomed-out timeline empties it and spins foreversrc/stores/fetchers/timeline_fetcher.js:27-31,55,107-113 + src/components/timeline/timeline.js:175-178 + src/components/timeline/timeline.vue:97-107. fetchAndUpdate sets loadingOlder = true before the if (older && bottomedOut.value) return guard, so the flag is never cleared by the .finally. bottomedOut lives in the fetcher closure and clearTimeline (which the Reload path calls, also clearing ordercount === 0) can't reset it. Result: empty timeline + infinite spinner; if the WS reconnected meanwhile (polling stopped), it never self-heals. Fix: check bottomedOut before setting flags, and reset it in the reload path.

P2 — clear bugs

  1. WS-delivered statuses never advance timeline.maxIdsrc/stores/timelines.js:281-290 + 313-323 + 417-433. updateTimelineExtremes runs before the order insert and onStreamMessage passes pagination = {}, so maxNew = first(timeline.order) is the pre-insert (previous) newest: maxId lags 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 stale sinceId; with >20 missed statuses the hole is permanent (later polls only move since_id forward), and the duplicate-heavy poll also trips reloadNeeded (≥20 rule) spuriously. This is the remaining piece of the reported "max_id not incremented with websockets enabled" issue — f2f15a0c02's force fix repairs maxId only 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.
  2. Bookmarks pagination NaNentity_normalizer.service.js:389-390 + timelines.js:419-426. Bookmarks are the only non-flakeId timeline; when the Link header lacks next/prev (final page), Number.parseInt(undefined)NaN, and pagination.maxId ?? last(...) doesn't filter NaN (?? ≠ old falsy check). minId never advances → "load older" refetches the final page forever, bottomedOut never set, "no more statuses" never shown, and ≥20 responses re-trigger "Reload" spuriously.
  3. Streaming lifecycle holessrc/stores/streaming.js. (a) stopSocket() (line 131) does this.socket.close() unguarded → TypeError if toggling useStreamingApi off before any socket exists (settings path general_tab.js:78; anonymous sessions or setting-off-at-login). (b) onClose (line 231) schedules the reconnect setTimeout with no stored handle and nothing ever cancels it — logging out during a retry window fires initSocket with the now-cleared token (anonymous socket, bogus "connection established" toast, possible retry loop); initSocket also replaces this.socket without closing a live previous one (the old enableMastoSockets state guard was dropped), so a failed revokeToken logout can leave two sockets feeding the same handlers. Fix: generation-token/identity check in handlers, cancel timer in stopSocket, this.socket?.close() + close-existing-first in initSocket.
  4. Emoji-reaction/favs popover crashes on first rendersrc/components/user_list_popover/user_list_popover.js:26-31 + .vue:13-16. usersCapped maps findUser(id) over account_ids without filtering; reacting users are only added to the store by the async @show fetch, so the first (pre-fetch) render contains undefined entries → :key="user.id" render TypeError. Needs .filter(Boolean) or per-id loading state.
  5. Standalone status page throws on loadsrc/components/conversation/conversation.js:623-633 (new method). mounted()updateVirtualHeight()nextTickthis.status.id, but on a direct status-link load status (allStatuses.get(statusId)) is still undefined → TypeError. Guard for missing status.
  6. showReasonMutedThread typo mainSatussrc/components/status/status.js:152-157. this.mainSatus.reblog throws when evaluated; currently unreferenced (landmine), and .reblog isn't a normalized field anyway (retweeted_status). Fix or drop.

P3 — minor

  • timelines.js deactivate removes 'message' but the handler was registered as 'update' (lines 150/183) — latent leak.
  • Notifications fetcher drops the older flag (addNewNotifications(response), notifications_fetcher.js:39) so the push branch is dead; POSITIVE_INFINITY sentinels were kept but initial extremes are now '' → empty-string sinceId/maxId params sent where develop sent none (notifications_fetcher.js:81,89).
  • users.js:521,540,557 mute/unmute/block optimistic predictions read this.relationships[id] on a Map (always undefined) — accidentally benign, broken as written. Also: failed user fetches permanently cache the rejected promise (no retry until reload), and usersByName isn't refreshed on rename.
  • populateRepeats runs before the argument-mismatch guard (timelines.js:269 vs 275-279) — a late fetch for user A can pollute user B's fresh repeat maps (rare dedup misfire).
  • New green "Realtime connection closed" toast fires on every logout / streaming toggle-off (intended closes now dispatch 'close' and interface shows a success notice for codes 1000/1001) — probably unintended.
  • Dead code: timelines.requireReloadAll(); statusesToDisplay returns nonexistent this.visibleStatusIds when virtual scrolling is off (timeline.js:105); favorites timeline silently drops its userId argument (ARGUMENT_MAP has no favorites entry — pre-existing).
  • Follow-request approve/deny still dispatches the nonexistent vuex removeFollowRequest action (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 (showNewStatuses slice) — the reported "Show new" slowdown is addressed.
  • 32c27b85b0: chat-creation fix is complete and correct — route param chatUserId matches routes/props, all three navigation sites updated, search results resolve through the users store. The reported Missing required param "chatUserId" error is fixed.
  • No dangling vuex references to the removed statuses/users/notifications/api modules anywhere in src/ — the module removal is clean; remaining $store usage maps to surviving modules.
  • Store-by-id architecture is coherent: users spliced into statuses, notifications/quotes re-pointed at stored copies, timestamped merge discipline everywhere; favs/repeats/reactions stored separately from statuses (the reported missing-interaction-avatars bug is genuinely fixed); mute handling via store lookups (repeater relationships included) verified; optimistic interactions have rollback; block flow wipes statuses/timelines/notifications end-to-end.
  • Streaming: no listener leaks on timeline switches, resubscribe-after-reconnect works, no double-handling of the shared user stream; notifications store advances extremes per-id (the model timelines should copy).
  • Timelines store is not persisted; persistent friends/DM timelines correctly skip component-driven deactivate and are wired to login/logout.

Notes

  • The declared known issues (remaining vuex stores, follow requests, profileConfig deferred to next MR) were treated as out of scope as stated.
  • Suggested fix priority before merge: #1 and #2 (user-visible data loss / bricked timeline), then #3/#5 (hole-healing and WS lifecycle), then the popover/status-page guards.
opencode-agent:job=76:generation=1 ## OpenCode review of head `8e63f189fcaae7462ec8fbd9a1db51bbab9414d4` (base `develop` @ `7e8f63c71420812f53b48edae68954b02d64eb17`) Reviewed the full base→head diff (216 files, +7887/−5680) with focus on the new `statuses`/`timelines`/`streaming`/`notifications`/`users` stores, fetchers, and the three newest commits (`f2f15a0c02` extremes-overwrite option, `32c27b85b0` chat creation fix, `8e63f189fc` cleanup). Every finding below was verified against the code at this exact head. ### Verification summary - **Unit suite green**: all 36 spec files pass (stores: statuses 49, users 56, timelines 19, notifications 17, sync_config 35+1skip, oauth 14, lists 6, user_highlight 14; plus all component/service/boot specs; rich_content 15+1skip). Run per-file because this runner's batched browser mode is flaky — no test failures, only infra noise. - **Lint**: biome clean (688 files); eslint 0 errors (6 style warnings); stylelint clean. ### P1 — regressions 1. **Multiply-repeated posts vanish from timelines: `checkSeenBefore` uses lodash `first()` on a `Set`** — `src/stores/timelines.js:345-350`. `knownRepeats` values are `Set`s, but `_.first(new Set(['a','b']))` → `undefined`, so `first(knownRepeats) !== statusId` is **always true** once `size > 1`; the "show the oldest reprööt" branch never fires. Since `populateRepeats` (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 to `ignoredIds` (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`. 2. **"Reload" on a bottomed-out timeline empties it and spins forever** — `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`. `fetchAndUpdate` sets `loadingOlder = true` *before* the `if (older && bottomedOut.value) return` guard, so the flag is never cleared by the `.finally`. `bottomedOut` lives in the fetcher closure and `clearTimeline` (which the Reload path calls, also clearing `order` → `count === 0`) can't reset it. Result: empty timeline + infinite spinner; if the WS reconnected meanwhile (polling stopped), it never self-heals. Fix: check `bottomedOut` before setting flags, and reset it in the reload path. ### P2 — clear bugs 3. **WS-delivered statuses never advance `timeline.maxId`** — `src/stores/timelines.js:281-290` + `313-323` + `417-433`. `updateTimelineExtremes` runs *before* the order insert and `onStreamMessage` passes `pagination = {}`, so `maxNew = first(timeline.order)` is the pre-insert (previous) newest: `maxId` lags 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 stale `sinceId`; with >20 missed statuses the hole is permanent (later polls only move `since_id` forward), and the duplicate-heavy poll also trips `reloadNeeded` (≥20 rule) spuriously. This is the remaining piece of the reported "max_id not incremented with websockets enabled" issue — `f2f15a0c02`'s `force` fix repairs `maxId` only 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. 4. **Bookmarks pagination NaN** — `entity_normalizer.service.js:389-390` + `timelines.js:419-426`. Bookmarks are the only non-flakeId timeline; when the Link header lacks `next`/`prev` (final page), `Number.parseInt(undefined)` → `NaN`, and `pagination.maxId ?? last(...)` doesn't filter `NaN` (`??` ≠ old falsy check). `minId` never advances → "load older" refetches the final page forever, `bottomedOut` never set, "no more statuses" never shown, and ≥20 responses re-trigger "Reload" spuriously. 5. **Streaming lifecycle holes** — `src/stores/streaming.js`. (a) `stopSocket()` (line 131) does `this.socket.close()` unguarded → TypeError if toggling `useStreamingApi` off before any socket exists (settings path `general_tab.js:78`; anonymous sessions or setting-off-at-login). (b) `onClose` (line 231) schedules the reconnect `setTimeout` with no stored handle and nothing ever cancels it — logging out during a retry window fires `initSocket` with the now-cleared token (anonymous socket, bogus "connection established" toast, possible retry loop); `initSocket` also replaces `this.socket` without closing a live previous one (the old `enableMastoSockets` state guard was dropped), so a failed `revokeToken` logout can leave two sockets feeding the same handlers. Fix: generation-token/identity check in handlers, cancel timer in `stopSocket`, `this.socket?.close()` + close-existing-first in `initSocket`. 6. **Emoji-reaction/favs popover crashes on first render** — `src/components/user_list_popover/user_list_popover.js:26-31` + `.vue:13-16`. `usersCapped` maps `findUser(id)` over `account_ids` without filtering; reacting users are only added to the store by the async `@show` fetch, so the first (pre-fetch) render contains `undefined` entries → `:key="user.id"` render TypeError. Needs `.filter(Boolean)` or per-id loading state. 7. **Standalone status page throws on load** — `src/components/conversation/conversation.js:623-633` (new method). `mounted()` → `updateVirtualHeight()` → `nextTick` → `this.status.id`, but on a direct status-link load `status` (`allStatuses.get(statusId)`) is still `undefined` → TypeError. Guard for missing status. 8. **`showReasonMutedThread` typo `mainSatus`** — `src/components/status/status.js:152-157`. `this.mainSatus.reblog` throws when evaluated; currently unreferenced (landmine), and `.reblog` isn't a normalized field anyway (`retweeted_status`). Fix or drop. ### P3 — minor - `timelines.js` deactivate removes `'message'` but the handler was registered as `'update'` (lines 150/183) — latent leak. - Notifications fetcher drops the `older` flag (`addNewNotifications(response)`, `notifications_fetcher.js:39`) so the `push` branch is dead; `POSITIVE_INFINITY` sentinels were kept but initial extremes are now `''` → empty-string `sinceId`/`maxId` params sent where develop sent none (`notifications_fetcher.js:81,89`). - `users.js:521,540,557` mute/unmute/block optimistic predictions read `this.relationships[id]` on a `Map` (always `undefined`) — accidentally benign, broken as written. Also: failed user fetches permanently cache the rejected promise (no retry until reload), and `usersByName` isn't refreshed on rename. - `populateRepeats` runs before the argument-mismatch guard (`timelines.js:269` vs `275-279`) — a late fetch for user A can pollute user B's fresh repeat maps (rare dedup misfire). - New green "Realtime connection closed" toast fires on every logout / streaming toggle-off (intended closes now dispatch `'close'` and `interface` shows a success notice for codes 1000/1001) — probably unintended. - Dead code: `timelines.requireReloadAll()`; `statusesToDisplay` returns nonexistent `this.visibleStatusIds` when virtual scrolling is off (`timeline.js:105`); favorites timeline silently drops its `userId` argument (`ARGUMENT_MAP` has no `favorites` entry — pre-existing). - Follow-request approve/deny still dispatches the nonexistent vuex `removeFollowRequest` action (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 (`showNewStatuses` slice) — the reported "Show new" slowdown is addressed. - `32c27b85b0`: chat-creation fix is complete and correct — route param `chatUserId` matches routes/props, all three navigation sites updated, search results resolve through the users store. The reported `Missing required param "chatUserId"` error is fixed. - No dangling vuex references to the removed `statuses`/`users`/`notifications`/`api` modules anywhere in `src/` — the module removal is clean; remaining `$store` usage maps to surviving modules. - Store-by-id architecture is coherent: users spliced into statuses, notifications/quotes re-pointed at stored copies, timestamped merge discipline everywhere; favs/repeats/reactions stored separately from statuses (the reported missing-interaction-avatars bug is genuinely fixed); mute handling via store lookups (repeater relationships included) verified; optimistic interactions have rollback; block flow wipes statuses/timelines/notifications end-to-end. - Streaming: no listener leaks on timeline switches, resubscribe-after-reconnect works, no double-handling of the shared `user` stream; notifications store advances extremes per-id (the model timelines should copy). - Timelines store is not persisted; persistent friends/DM timelines correctly skip component-driven deactivate and are wired to login/logout. ### Notes - The declared known issues (remaining vuex stores, follow requests, profileConfig deferred to next MR) were treated as out of scope as stated. - Suggested fix priority before merge: #1 and #2 (user-visible data loss / bricked timeline), then #3/#5 (hole-healing and WS lifecycle), then the popover/status-page guards. <!-- opencode-agent:job=76:generation=1 -->
Author
Member

I think I'll fix P1 and P2 bugs and merge, leaving P3 to be fixed in follow-up MR(s)

I think I'll fix P1 and P2 bugs and merge, leaving P3 to be fixed in follow-up MR(s)
Author
Member

Multiply-repeated posts vanish from timelines

fixed

"Reload" on a bottomed-out timeline empties it and spins forever

fixed

Bookmarks pagination NaN

Literally doesn't happen but added a guard against it anyway.

>Multiply-repeated posts vanish from timelines fixed >"Reload" on a bottomed-out timeline empties it and spins forever fixed >Bookmarks pagination NaN Literally doesn't happen but added a guard against it anyway.
hj added 6 commits 2026-09-01 08:52:06 +00:00
guard
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/test Pipeline failed
ci/woodpecker/pr/test-e2e unknown status
5ea41244c1
Author
Member

Streaming lifecycle holes

Plugged some holes

  • settings modal shouldn't toggle socket for unauthenticated users
  • retry socket init checks if we're actually retrying before retrying, closing socket clears this.retrying
  • check if socket already exists in initializing, at least system would actually error out instead of double-overlaying two sockets

Emoji-reaction/favs popover crashes on first render

fixed

Standalone status page throws on load

fixed

showReasonMutedThread typo mainSatus

fixed, also clarified separation between status, mainStatus, repeatedStatus and newly added repeatStatus (which gives null on non-repeats)

>Streaming lifecycle holes Plugged some holes - settings modal shouldn't toggle socket for unauthenticated users - retry socket init checks if we're actually retrying before retrying, closing socket clears `this.retrying` - check if socket already exists in initializing, at least system would actually error out instead of double-overlaying two sockets >Emoji-reaction/favs popover crashes on first render fixed >Standalone status page throws on load fixed >showReasonMutedThread typo mainSatus fixed, also clarified separation between `status`, `mainStatus`, `repeatedStatus` and newly added `repeatStatus` (which gives `null` on non-repeats)
hj added 3 commits 2026-09-01 10:20:36 +00:00
better clarification + thread_muting handling (fixed typo)
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/test Pipeline failed
ci/woodpecker/pr/test-e2e unknown status
3589a5302e
hj added 3 commits 2026-09-01 10:50:34 +00:00
logged-out state fixes
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/test Pipeline failed
ci/woodpecker/pr/test-e2e unknown status
39f7d522e3
Author
Member

timelines.js deactivate removes 'message'

fixed

Notifications fetcher drops the older flag

fixed

users.js:521,540,557 mute/unmute/block optimistic predictions read

fixed

Also: failed user fetches permanently cache the rejected promise (no retry until reload), and usersByName isn't refreshed on rename.

failed promises are no longer cached

populateRepeats runs before the argument-mismatch guard

fixed

New green "Realtime connection closed" toast fires on every logout

Ehhhhhh yeah we can remove it.

Dead code: timelines.requireReloadAll();

It was calling statuses.requireReloadAll() instead of timelines.requireReloadAll(), now it has a proper call. Also it was broken so I fixed that

statusesToDisplay returns nonexistent this.visibleStatusIds when virtual scrolling is off (timeline.js:105);

fixed

favorites timeline silently drops its userId argument (ARGUMENT_MAP has no favorites entry — pre-existing).

fixed

>timelines.js deactivate removes 'message' fixed >Notifications fetcher drops the older flag fixed >users.js:521,540,557 mute/unmute/block optimistic predictions read fixed >Also: failed user fetches permanently cache the rejected promise (no retry until reload), and usersByName isn't refreshed on rename. failed promises are no longer cached >populateRepeats runs before the argument-mismatch guard fixed >New green "Realtime connection closed" toast fires on every logout Ehhhhhh yeah we can remove it. >Dead code: timelines.requireReloadAll(); It was calling `statuses.requireReloadAll()` instead of `timelines.requireReloadAll()`, now it has a proper call. Also it was broken so I fixed that >statusesToDisplay returns nonexistent this.visibleStatusIds when virtual scrolling is off (timeline.js:105); fixed >favorites timeline silently drops its userId argument (ARGUMENT_MAP has no favorites entry — pre-existing). fixed
hj added 2 commits 2026-09-01 10:56:17 +00:00
lint
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline failed
ci/woodpecker/pr/test-e2e unknown status
9a263692ea
fix repeat deduplication logic
All checks were successful
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was successful
53498072e5
Author
Member

/oc review

/oc review
First-time contributor

opencode-agent:job=77:generation=1

OpenCode review of head 53498072e59c6d09e94b419cda925bb651cee845 (base develop @ 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

  • Unit suite green: all 36 spec files pass, each run in its own isolated browser session (batched browser mode on this runner drops iframe connections — every batch-level failure retried in isolation passed; zero assertion failures anywhere). Per-file: stores — timelines 19, users 56, statuses 49, notifications 17, sync_config 35+1skip, oauth 14, lists 6, user_highlight 14; components — rich_content 15+1skip, gallery 14, post_status_form 14, chat_message_list 7, emoji_input 9, draft 7, chat_view 4; theme_data suite 98 total; all remaining service/boot/lib specs green.
  • Lint: biome clean (688 files); eslint 0 errors (6 pre-existing style warnings); stylelint clean.
  • All previously reported findings are fixed at this head (details at the end).

P1 — new regression: websocket never reconnects after an unexpected drop, and logout during a retry window still spawns a socket (src/stores/streaming.js, commit 64cc748a9c)

onClose retry path:

setTimeout(() => {
  if (this.retrying) return // retry aborted (i.e. due to logout)

  this.initSocket()
}, retryTimeout(this.retryMultiplier))
// …a few lines below, same synchronous flow:
this.retrying = true

The guard is inverted. retrying is set to true synchronously right after the timer is scheduled, and nothing can clear it before the timer fires (onOpen can't fire — there is no socket to open; only stopSocket() clears it). Both directions are wrong:

  1. Every normal retry aborts itself. After any unexpected close (network blip, server restart, code 1006…), the callback sees retrying === true and returns; initSocket is never called again. Realtime stays dead until page reload or a manual settings toggle (timelines silently fall back to 10s polling). Before 64cc748a9c this path reconnected.
  2. Logout during the retry window does exactly what the comment claims to prevent. stopSocket() sets retrying = false, so the pending timer now passes the guard and calls initSocket() with the cleared token → post-logout socket creation.

Fix: invert the check — if (!this.retrying) return. (Then onOpen's retrying = false re-arms later retries and stopSocket's retrying = false aborts pending retries as intended.)

P2 — the "failed user fetch caches rejected promise" fix is misplaced and still doesn't work (src/stores/users.js, commit 5788d3e8ec)

The new try { … } catch (e) { map.delete(identifier); throw e } wraps only the post-await result handling, but const result = await promise sits outside the try. A rejected fetch therefore throws before the catch ever runs, map.delete(identifier) never executes, and the rejected promise stays cached in fetchesIds/fetchesNames forever — the exact P3 issue this commit was meant to fix. Every later fetchUserByIdOrName for 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: move const result = await promise inside the try (or clear the cache entry in a .catch before rethrowing).

P3 — minor

  • stopSocket() (streaming.js) still does an unguarded this.socket.close() → TypeError if ever called with no socket. The new general_tab token guard covers the anonymous settings path and logout is state !== 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. Prefer console.warn + early return (or closing the existing socket first) over throwing into the login chain.

Verified fixed at this head (previously reported at 8e63f189)

  • Reprööt vanish (P1): knownRepeats.values().next().value replaces lodash first() on a Set (590ca6fb6f); head commit 53498072e5 additionally 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).
  • Bottomed-out reload bricking the timeline (P1): resetBottomedOut added to the fetcher and called from clearTimeline/showNewStatuses; loadingOlder is reset before the early return (725b2f5387). Checked fetchOlder call sites — results aren't chained, so the bare return is safe.
  • WS statuses never advancing maxId (P2): updateTimelineExtremes now runs after the order insert (b75bd5ae73); traced WS / newer-poll / older-page paths — extremes now move correctly in all three.
  • Bookmarks NaN pagination (P2): parseLinkHeaderPagination omits maxId/minId when the Link header lacks them and '' sentinels replace POSITIVE_INFINITY (43ddbfd652). Worst case is one extra empty older fetch before bottomedOut is set — terminates correctly.
  • Popover crash on not-yet-fetched reacting users (P2): user_list_popover filters findUser results and caps after (b3754b26c4).
  • Standalone status page throw (P2): updateVirtualHeight guards !this.status (abe6af133f).
  • mainSatus typo/landmine (P2): reworked into mainStatus/repeatStatus computeds with correct thread_muted semantics (3589a5302e).
  • P3 batch: deactivate now removes the registered 'update' handler; notifications fetcher passes older through and uses '' sentinels (stream and poll paths both verified against addNewNotifications(result, older)); mute/unmute/block predictions use relationships.get(); populateRepeats moved after the argument-mismatch guard; dead socket_closed toast/string removed; statusesToDisplay non-virtual path returns real ids; requireReloadAll iterates TIMELINES and is wired into quick-filter/clutter/filtering tabs; favorites timeline gets its userId via ARGUMENT_MAP; logged-out currentUser?.id guards in follow_card/user_card/appearance_tab/general_tab.

Notes

  • The declared known issues (residual vuex stores, follow requests, profileConfig) remain out of scope as declared.
  • Suggested priority: fix the P1 retry inversion before merge — it silently kills realtime for every user after the first connection hiccup; the P2 one-liner is worth folding in alongside.
opencode-agent:job=77:generation=1 ## OpenCode review of head `53498072e59c6d09e94b419cda925bb651cee845` (base `develop` @ `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 - **Unit suite green**: all 36 spec files pass, each run in its own isolated browser session (batched browser mode on this runner drops iframe connections — every batch-level failure retried in isolation passed; zero assertion failures anywhere). Per-file: stores — timelines 19, users 56, statuses 49, notifications 17, sync_config 35+1skip, oauth 14, lists 6, user_highlight 14; components — rich_content 15+1skip, gallery 14, post_status_form 14, chat_message_list 7, emoji_input 9, draft 7, chat_view 4; theme_data suite 98 total; all remaining service/boot/lib specs green. - **Lint**: biome clean (688 files); eslint 0 errors (6 pre-existing style warnings); stylelint clean. - **All previously reported findings are fixed at this head** (details at the end). ### P1 — new regression: websocket never reconnects after an unexpected drop, and logout during a retry window still spawns a socket (`src/stores/streaming.js`, commit `64cc748a9c`) `onClose` retry path: ```js setTimeout(() => { if (this.retrying) return // retry aborted (i.e. due to logout) this.initSocket() }, retryTimeout(this.retryMultiplier)) // …a few lines below, same synchronous flow: this.retrying = true ``` The guard is inverted. `retrying` is set to `true` synchronously right after the timer is scheduled, and nothing can clear it before the timer fires (`onOpen` can't fire — there is no socket to open; only `stopSocket()` clears it). Both directions are wrong: 1. **Every normal retry aborts itself.** After any unexpected close (network blip, server restart, code 1006…), the callback sees `retrying === true` and returns; `initSocket` is never called again. Realtime stays dead until page reload or a manual settings toggle (timelines silently fall back to 10s polling). Before `64cc748a9c` this path reconnected. 2. **Logout during the retry window does exactly what the comment claims to prevent.** `stopSocket()` sets `retrying = false`, so the pending timer now *passes* the guard and calls `initSocket()` with the cleared token → post-logout socket creation. Fix: invert the check — `if (!this.retrying) return`. (Then `onOpen`'s `retrying = false` re-arms later retries and `stopSocket`'s `retrying = false` aborts pending retries as intended.) ### P2 — the "failed user fetch caches rejected promise" fix is misplaced and still doesn't work (`src/stores/users.js`, commit `5788d3e8ec`) The new `try { … } catch (e) { map.delete(identifier); throw e }` wraps only the *post-`await`* result handling, but `const result = await promise` sits **outside** the `try`. A rejected fetch therefore throws before the catch ever runs, `map.delete(identifier)` never executes, and the rejected promise stays cached in `fetchesIds`/`fetchesNames` forever — the exact P3 issue this commit was meant to fix. Every later `fetchUserByIdOrName` for 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: move `const result = await promise` inside the `try` (or clear the cache entry in a `.catch` before rethrowing). ### P3 — minor - `stopSocket()` (`streaming.js`) still does an unguarded `this.socket.close()` → TypeError if ever called with no socket. The new `general_tab` token guard covers the anonymous settings path and logout is `state !== 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. Prefer `console.warn` + early return (or closing the existing socket first) over throwing into the login chain. ### Verified fixed at this head (previously reported at `8e63f189`) - **Reprööt vanish (P1)**: `knownRepeats.values().next().value` replaces lodash `first()` on a Set (`590ca6fb6f`); head commit `53498072e5` additionally 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). - **Bottomed-out reload bricking the timeline (P1)**: `resetBottomedOut` added to the fetcher and called from `clearTimeline`/`showNewStatuses`; `loadingOlder` is reset before the early return (`725b2f5387`). Checked `fetchOlder` call sites — results aren't chained, so the bare return is safe. - **WS statuses never advancing `maxId` (P2)**: `updateTimelineExtremes` now runs after the order insert (`b75bd5ae73`); traced WS / newer-poll / older-page paths — extremes now move correctly in all three. - **Bookmarks NaN pagination (P2)**: `parseLinkHeaderPagination` omits `maxId`/`minId` when the Link header lacks them and `''` sentinels replace `POSITIVE_INFINITY` (`43ddbfd652`). Worst case is one extra empty older fetch before `bottomedOut` is set — terminates correctly. - **Popover crash on not-yet-fetched reacting users (P2)**: `user_list_popover` filters `findUser` results and caps after (`b3754b26c4`). - **Standalone status page throw (P2)**: `updateVirtualHeight` guards `!this.status` (`abe6af133f`). - **`mainSatus` typo/landmine (P2)**: reworked into `mainStatus`/`repeatStatus` computeds with correct `thread_muted` semantics (`3589a5302e`). - **P3 batch**: deactivate now removes the registered `'update'` handler; notifications fetcher passes `older` through and uses `''` sentinels (stream and poll paths both verified against `addNewNotifications(result, older)`); mute/unmute/block predictions use `relationships.get()`; `populateRepeats` moved after the argument-mismatch guard; dead `socket_closed` toast/string removed; `statusesToDisplay` non-virtual path returns real ids; `requireReloadAll` iterates `TIMELINES` and is wired into quick-filter/clutter/filtering tabs; favorites timeline gets its `userId` via `ARGUMENT_MAP`; logged-out `currentUser?.id` guards in follow_card/user_card/appearance_tab/general_tab. ### Notes - The declared known issues (residual vuex stores, follow requests, profileConfig) remain out of scope as declared. - Suggested priority: fix the P1 retry inversion before merge — it silently kills realtime for every user after the first connection hiccup; the P2 one-liner is worth folding in alongside. <!-- opencode-agent:job=77:generation=1 -->
hj added 2 commits 2026-09-01 15:21:39 +00:00
invert check
All checks were successful
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was successful
7b037ef60f
P3
Some checks failed
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was successful
c8593a9668
Author
Member

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.

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.
lint
All checks were successful
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was successful
7b58a96ece
Author
Member

/oc review

/oc review
First-time contributor

opencode-agent:job=78:generation=1

OpenCode review of head 7b58a96ecee24e97fe6e5daf7bba6484449e403a (base develop @ 7e8f63c71420812f53b48edae68954b02d64eb17)

This round covers the four commits added since the last review (53498072e5): b864dfb056 (whoops), 7b037ef60f (invert check), c8593a9668 (P3), 7b58a96ece (lint) — only src/stores/users.js and src/stores/streaming.js changed. Everything below was verified against the code at this exact head.

Verification summary

  • Unit suite green: all 36 spec files pass, each run in its own isolated browser session. Per-file: stores — users 56, statuses 49, timelines 19, notifications 17, sync_config 35+1skip, oauth 14, lists 6, user_highlight 14; components/services/boot/lib — rich_content 15+1skip, gallery 14, post_status_form 14, iss_deserializer 29, persisted_state 14, helpers 13, html_line_converter 19, matcher 10, emoji_input 9, sanity_checks 9, theme_data 8, theme_data3 6, entity_normalizer 8, chat_message_list 7, draft 7, gesture_service 7, routes 6, date_utils 6, html_tree_converter 5, chat_view 4, completion 4, utility 3, style_setter 3, user_profile_link_generator 3, file_type 2, notification_utils 2, file_size_format 1, theme2_to_theme3 1. Zero assertion failures anywhere.
  • Lint: biome clean (688 files); eslint 0 errors (6 pre-existing style warnings); stylelint clean.

All three previously reported findings are fixed at this head

  1. P1 (WS never reconnects / post-logout socket) — fixed (7b037ef60f). The guard is now if (!this.retrying) return. Traced both directions: after an unexpected close the pending timer now passes the guard and calls initSocket() (realtime recovers; onOpen resets retrying/multiplier so later retries re-arm), and stopSocket() during the retry window sets retrying = false so the timer aborts — no socket after logout. Intended closures (1000/1001) still reset everything without scheduling a retry.
  2. P2 (failed user fetch caches rejected promise) — fixed (b864dfb056). const result = await promise moved inside the try; a rejection now reaches the catch, deletes the fetchesIds/fetchesNames entry, 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.
  3. P3 (crash hardening) — fixed (c8593a9668, formatting in 7b58a96ece). initSocket no 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. stopSocket now uses this.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 clears this.socket — only stopSocket() 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: set this.socket = null in the retry branch of onClose (or after the force-close in initSocket), keeping the force-close for genuinely live strays. Non-blocking.

Notes

  • error.tatusCode === 401 typo in the login catch (src/stores/users.js:710) produces the wrong error message for bad passwords — pre-existing on develop (came in via 1ca0ffb1f0), not introduced by this MR, drive-by fix material only.
  • Declared known issues (residual vuex stores, follow requests, profileConfig) remain out of scope as declared in the MR description.

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: 7b58a96ecee

opencode-agent:job=78:generation=1 ## OpenCode review of head `7b58a96ecee24e97fe6e5daf7bba6484449e403a` (base `develop` @ `7e8f63c71420812f53b48edae68954b02d64eb17`) This round covers the four commits added since the last review (`53498072e5`): `b864dfb056` (whoops), `7b037ef60f` (invert check), `c8593a9668` (P3), `7b58a96ece` (lint) — only `src/stores/users.js` and `src/stores/streaming.js` changed. Everything below was verified against the code at this exact head. ### Verification summary - **Unit suite green**: all 36 spec files pass, each run in its own isolated browser session. Per-file: stores — users 56, statuses 49, timelines 19, notifications 17, sync_config 35+1skip, oauth 14, lists 6, user_highlight 14; components/services/boot/lib — rich_content 15+1skip, gallery 14, post_status_form 14, iss_deserializer 29, persisted_state 14, helpers 13, html_line_converter 19, matcher 10, emoji_input 9, sanity_checks 9, theme_data 8, theme_data3 6, entity_normalizer 8, chat_message_list 7, draft 7, gesture_service 7, routes 6, date_utils 6, html_tree_converter 5, chat_view 4, completion 4, utility 3, style_setter 3, user_profile_link_generator 3, file_type 2, notification_utils 2, file_size_format 1, theme2_to_theme3 1. Zero assertion failures anywhere. - **Lint**: biome clean (688 files); eslint 0 errors (6 pre-existing style warnings); stylelint clean. ### All three previously reported findings are fixed at this head 1. **P1 (WS never reconnects / post-logout socket) — fixed** (`7b037ef60f`). The guard is now `if (!this.retrying) return`. Traced both directions: after an unexpected close the pending timer now passes the guard and calls `initSocket()` (realtime recovers; `onOpen` resets `retrying`/multiplier so later retries re-arm), and `stopSocket()` during the retry window sets `retrying = false` so the timer aborts — no socket after logout. Intended closures (1000/1001) still reset everything without scheduling a retry. 2. **P2 (failed user fetch caches rejected promise) — fixed** (`b864dfb056`). `const result = await promise` moved inside the `try`; a rejection now reaches the catch, deletes the `fetchesIds`/`fetchesNames` entry, 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. 3. **P3 (crash hardening) — fixed** (`c8593a9668`, formatting in `7b58a96ece`). `initSocket` no 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. `stopSocket` now uses `this.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 clears `this.socket` — only `stopSocket()` 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: set `this.socket = null` in the retry branch of `onClose` (or after the force-close in `initSocket`), keeping the force-close for genuinely live strays. Non-blocking. ### Notes - `error.tatusCode === 401` typo in the login catch (`src/stores/users.js:710`) produces the wrong error message for bad passwords — pre-existing on develop (came in via `1ca0ffb1f0`), not introduced by this MR, drive-by fix material only. - Declared known issues (residual vuex stores, follow requests, profileConfig) remain out of scope as declared in the MR description. ## 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: `7b58a96ecee`_ <!-- opencode-agent:job=78:generation=1 -->
drive-by
All checks were successful
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/changelog Pipeline was successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful
ci/woodpecker/pr/test-e2e Pipeline was successful
90348c7da4
Author
Member

I'll look deeper into streaming/sockets some time later, this is already better than what we have

I'll look deeper into streaming/sockets some time later, this is already better than what we have
hj merged commit d603aac31b into develop 2026-09-01 16:32:32 +00:00
hj deleted branch users-statuses-pinia 2026-09-01 16:32:32 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
5 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
pleroma/pleroma-fe!3555
No description provided.