feat: support multiple media server instances#2689
Closed
KakatkarAkshay wants to merge 77 commits intoseerr-team:developfrom
Closed
feat: support multiple media server instances#2689KakatkarAkshay wants to merge 77 commits intoseerr-team:developfrom
KakatkarAkshay wants to merge 77 commits intoseerr-team:developfrom
Conversation
Author
|
Built this primarily for my own personal usage, but I wanted to open the PR here in case this feature is already in the pipeline or useful as a reference point. If it looks promising, I am very happy to make changes, refine the implementation, or adjust it to better fit the project direction so it could potentially be improved and merged later. If not, that is completely fine too. I am also happy to just leave this here as a reference for the team, regardless of whether it gets merged. |
Member
|
Can you update your PR to use our template please ? https://github.com/seerr-team/seerr/blob/develop/.github/PULL_REQUEST_TEMPLATE.md |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces multi-server support for Plex and Jellyfin/Emby by adding server-scoped configuration endpoints, database columns for per-server identifiers, scanner per-server iteration logic, and frontend multi-server selection UI, enabling Seerr to simultaneously manage multiple instances of each media server type. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Frontend as Client UI
participant Backend as API Server
participant DB as Database
participant MediaSvr as Media Server
User->>Frontend: Select Jellyfin Server from dropdown
User->>Frontend: Enter admin credentials for new server
Frontend->>Backend: POST /api/v1/settings/jellyfin<br/>{serverId?, mediaServerType,<br/>username, password, ...}
Backend->>MediaSvr: JellyfinAPI.getSystemInfo() /<br/>test connectivity
alt Success
MediaSvr-->>Backend: Server info + auto-generated API key
Backend->>Backend: Generate UUID for serverId
Backend->>DB: Upsert JellyfinServerSettings<br/>(id, mediaServerType, apiKey, ...)
DB-->>Backend: ✓
Backend->>DB: Save settings
DB-->>Backend: ✓
Backend-->>Frontend: {id, name, mediaServerType, ...}
Frontend->>Frontend: Update selectedServerId<br/>Refresh server list
Frontend->>Frontend: Revalidate settings cache
else Auth Failure
MediaSvr-->>Backend: 401 Invalid Credentials
Backend-->>Frontend: {error: InvalidCredentials}
Frontend->>User: Show error toast
end
sequenceDiagram
actor User
participant Frontend as Client UI
participant Backend as API Server
participant DB as Database
participant Jellyfin as Jellyfin Server
participant Scanner as Jellyfin Scanner
User->>Frontend: Trigger library sync
Frontend->>Backend: POST /api/v1/settings/jellyfin/sync<br/>?serverId=xyz
Backend->>Scanner: jellyfinFullScanner.run(serverId: 'xyz')
Scanner->>DB: Filter settings.jellyfinServers<br/>for serverId='xyz'
loop Per enabled library in filtered server
Scanner->>Jellyfin: JellyfinAPI.getRecentlyAdded(libId)
Jellyfin-->>Scanner: Recent items
Scanner->>Scanner: Process items<br/>(inject jellyfinServerId='xyz')
Scanner->>DB: Update/create Media entries<br/>with jellyfinServerId='xyz'
DB-->>Scanner: ✓
end
Scanner-->>Backend: Sync complete
Backend-->>Frontend: {status: 'completed', ...}
Frontend->>User: Show success notification
sequenceDiagram
actor Admin
participant Frontend as Settings UI
participant Backend as API Server
participant DB as Database
participant JF as Jellyfin Server
Admin->>Frontend: Click "Import Users"
Frontend->>Frontend: Show list of media servers
Alt Multiple Jellyfin Servers
Admin->>Frontend: Select specific server
End
Frontend->>Backend: GET /api/v1/settings/jellyfin/users<br/>?serverId=server-id-xyz
Backend->>DB: Resolve JellyfinServerSettings<br/>for serverId='server-id-xyz'
Backend->>JF: JellyfinAPI.getUsers()<br/>(using server's apiKey & mediaServerType)
JF-->>Backend: [user1, user2, ...]
Backend->>DB: Check existing imports<br/>via findJellyfinUser()<br/>(scoped to serverId)
DB-->>Backend: [user2 already imported]
Backend-->>Frontend: [user1, user3, ...] (filtered)
Admin->>Frontend: Select users + confirm
Frontend->>Backend: POST /api/v1/user/import-from-jellyfin<br/>{jellyfinUserIds, serverId}
Backend->>DB: Find/create Users<br/>+ set jellyfinServerId
DB-->>Backend: ✓
Backend-->>Frontend: {imported: [...]}
Frontend->>Admin: Show success + user list
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/UserList/JellyfinImportModal.tsx (1)
278-284:⚠️ Potential issue | 🟡 MinorInconsistent server type source in empty state message.
The "no users to import" message uses
settings.currentSettings.mediaServerType(global setting) instead of themediaServerTypeprop passed to the component. This could show the wrong server type name when importing from a server of a different type than the primary.🐛 Proposed fix
<Alert title={intl.formatMessage(messages.noJellyfinuserstoimport, { - mediaServerName: - settings.currentSettings.mediaServerType === MediaServerType.EMBY - ? 'Emby' - : 'Jellyfin', + mediaServerName, })} type="info" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/UserList/JellyfinImportModal.tsx` around lines 278 - 284, The empty-state Alert in JellyfinImportModal uses settings.currentSettings.mediaServerType (global) instead of the component prop mediaServerType, which can show the wrong server name; update the Alert call that builds intl.formatMessage(messages.noJellyfinuserstoimport, ...) to use the mediaServerType prop (and map it to 'Emby' or 'Jellyfin' the same way the current ternary does) so the displayed mediaServerName reflects the server being imported; ensure you update the reference inside the JSX where messages.noJellyfinuserstoimport is used (within the Alert component) and leave other uses of settings.currentSettings untouched.server/lib/settings/index.ts (1)
727-773:⚠️ Potential issue | 🟠 MajorUse the active Jellyfin/Emby type when filling legacy public fields.
fullPublicSettingsalways callsgetPrimaryJellyfinLikeServer()without a type filter. If both Jellyfin and Emby are configured,jellyfinExternalHost,jellyfinForgotPasswordUrl, andjellyfinServerNamecan come from the wrong server even whenmediaServerTypepoints at the other one. That can send legacy auth flows to the wrong host.🎯 Narrow the selection to the active jellyfin-like type
get fullPublicSettings(): FullPublicSettings { const mediaServers = this.getMediaServers(); - const jellyfinServer = this.getPrimaryJellyfinLikeServer(); + const primaryMediaServerType = this.getPrimaryMediaServerType(); + const jellyfinServer = + primaryMediaServerType === MediaServerType.JELLYFIN || + primaryMediaServerType === MediaServerType.EMBY + ? this.getPrimaryJellyfinLikeServer(primaryMediaServerType) + : this.getPrimaryJellyfinLikeServer(); return { ...this.data.public, @@ - mediaServerType: this.getPrimaryMediaServerType(), + mediaServerType: primaryMediaServerType, @@Also applies to: 927-933
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/lib/settings/index.ts` around lines 727 - 773, fullPublicSettings uses getPrimaryJellyfinLikeServer() without considering the active mediaServerType, which can return data from the wrong Jellyfin/Emby instance; update calls to getPrimaryJellyfinLikeServer() inside fullPublicSettings (and the similar block around the later 927-933 section) to pass the currently active type returned by getPrimaryMediaServerType() (e.g., getPrimaryJellyfinLikeServer(this.getPrimaryMediaServerType())) so jellyfinExternalHost, jellyfinForgotPasswordUrl, and jellyfinServerName are selected from the active Jellyfin-like server matching mediaServerType.server/lib/availabilitySync.ts (1)
858-900:⚠️ Potential issue | 🟠 Major4K Plex season availability is still too broad.
The new episode walk only proves that some season has a 4K file.
seasonExistsInPlex()still returnstruefor any season container inplexSeasonsCache, so once one season has 4K,finalSeasons4kcan keep non-4K-only seasons marked available. Filter/cache only the seasons that actually have a 4K episode, or do the per-season episode check insideseasonExistsInPlex()whenis4kis true.Also applies to: 957-981
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/lib/availabilitySync.ts` around lines 858 - 900, The current logic populates this.plexSeasonsCache with all season containers so once any season has a 4K episode other seasons are still considered available; update the code so the cache and lookups only include seasons that actually contain a 4K episode when is4k is true (or alternatively make seasonExistsInPlex() perform the per-season episode-level check when is4k is true). Concretely: when building this.plexSeasonsCache (and in the same pattern at the other block mentioned), iterate each season.ratingKey and call plexClient.getChildrenMetadata to detect Media items with width >= 2000, then store only those seasons in plexSeasonsCache (or make seasonExistsInPlex(…) query episodes and check Media widths when is4k is true) so finalSeasons4k only contains seasons that truly have 4K episodes.
🧹 Nitpick comments (7)
server/routes/discover.ts (1)
709-719: LGTM!Good use of type guards for the
allbranch to properly narrow types at runtime. The fallback tomapTvResultis reasonable given TMDB's current API contract.Optional consideration: The else clause assumes any unmatched type is TV. If TMDB ever adds new result types, they'd silently map as TV. You could add a
isTvguard for explicitness and log/skip unknown types, but this is purely defensive and not required given current API behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/discover.ts` around lines 709 - 719, The mapper currently falls back to mapTvResult for any non-movie/person/collection, which could mis-handle future TMDB types; update the mapper in the mapper: (result: TrendingResult, media?: Media): Results => { ... } block to explicitly check isTv(result) before calling mapTvResult, and add an else branch that logs a warning (or returns a safe fallback/skip) for unknown types (use existing logger or console.warn) so new/unexpected result kinds are not silently treated as TV.server/routes/auth.ts (2)
739-745: Consider graceful handling when server is not found during logout.If a user was associated with a Jellyfin server that has since been removed from settings, the
throw new Erroron line 744 will skip the device deletion but still allow session destruction to proceed (caught by outer try-catch). This is acceptable, but you could make it more explicit with areturnorcontinuepattern instead of throwing, to avoid the confusing double error log (lines 766-771 duplicate the error logging).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/auth.ts` around lines 739 - 745, Currently the code throws an Error when jellyfinServer is not found, which triggers duplicate error logs and skips explicit device-deletion handling; replace the throw with an early return (or continue if inside a loop) so that when settings.jellyfinServers.find(server => server.id === user.jellyfinServerId) yields no jellyfinServer you simply skip the device deletion logic and allow session destruction to proceed normally; update the block referencing jellyfinServer, settings.jellyfinServers, and user.jellyfinServerId to use return/continue and optionally log a single, clear info/debug message instead of throwing.
251-259: Redundant fallback logic injellyfinMediaServerTypederivation.The
jellyfinMediaServerTypevariable has a circular fallback: it checks ifresolvedServerTypeis JELLYFIN or EMBY, and if not, falls back toconfiguredServer?.mediaServerType— which is already used to computeresolvedServerTypeon line 251-252. This means the ternary on lines 254-258 will always resolve toresolvedServerTypewhen it's a valid Jellyfin/Emby type, making the fallback toconfiguredServer?.mediaServerTypeeffectively unreachable in that branch.Consider simplifying:
♻️ Proposed simplification
- const jellyfinMediaServerType = - resolvedServerType === MediaServerType.JELLYFIN || - resolvedServerType === MediaServerType.EMBY - ? resolvedServerType - : configuredServer?.mediaServerType; + const jellyfinMediaServerType = + resolvedServerType === MediaServerType.JELLYFIN || + resolvedServerType === MediaServerType.EMBY + ? resolvedServerType + : undefined;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/auth.ts` around lines 251 - 259, The current ternary for jellyfinMediaServerType redundantly falls back to configuredServer?.mediaServerType even though resolvedServerType was already derived from that, creating unreachable logic; change jellyfinMediaServerType to only accept resolvedServerType when it equals MediaServerType.JELLYFIN or MediaServerType.EMBY (otherwise undefined or null), and keep loginJellyfinMediaServerType as jellyfinMediaServerType ?? MediaServerType.JELLYFIN so the intent is clear; update the variables resolvedServerType, jellyfinMediaServerType, and loginJellyfinMediaServerType in server/routes/auth.ts accordingly.src/hooks/useDeepLinks.ts (1)
29-31: Consider memoizing or moving the helper insideuseEffect.
isIOSDeviceis defined as a component-level function but used insideuseEffect. While it's stable because it has no dependencies on props/state, for clarity and to avoid potential issues with future modifications, consider either:
- Moving it inside the
useEffect, or- Defining it outside the component entirely since it's a pure utility function
♻️ Option: Define outside component
+const isIOSDevice = () => + /iPad|iPhone|iPod/.test(navigator.userAgent) || + (navigator.userAgent.includes('Mac') && navigator.maxTouchPoints > 1); + const useDeepLinks = ({ mediaUrl, ... }: useDeepLinksProps) => { - const isIOSDevice = () => - /iPad|iPhone|iPod/.test(navigator.userAgent) || - (navigator.userAgent.includes('Mac') && navigator.maxTouchPoints > 1);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useDeepLinks.ts` around lines 29 - 31, isIOSDevice is a pure helper defined at component scope but only used inside useEffect; to avoid future stale-dep or readability issues, either move the isIOSDevice function into the useEffect block where it's used or hoist it entirely outside the component as a top-level utility function (keeping the same implementation using navigator.userAgent and navigator.maxTouchPoints); update any references to isIOSDevice accordingly and ensure ESLint warnings about hooks/deps are resolved.src/components/Settings/SettingsPlex.tsx (1)
262-291: Redundant early return check.The
serverIdparameter defaults toselectedServerId, so the check at line 263 and the spread at line 271 both handle the'new'case.♻️ Simplify the params construction
const syncLibraries = async (serverId = selectedServerId) => { if (serverId === 'new') { return; } setIsSyncing(true); - const params: { sync: boolean; enable?: string; serverId?: string } = { + const params: { sync: boolean; enable?: string; serverId: string } = { sync: true, - ...(serverId !== 'new' ? { serverId } : {}), + serverId, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Settings/SettingsPlex.tsx` around lines 262 - 291, The early return in syncLibraries is redundant—remove the initial if (serverId === 'new') return; and rely on the existing params construction that only includes serverId when serverId !== 'new'; keep setIsSyncing(true)/finally cleanup, the params object (sync: true and ...(serverId !== 'new' ? { serverId } : {})), and the activeLibraries handling, so behavior is unchanged but the redundant check is removed from syncLibraries (refer to syncLibraries, params, selectedServerId, and activeLibraries).src/components/Settings/SettingsJellyfin.tsx (1)
800-806: Unconventional error path key for compound validation.The error path
'apiKey | username | password'with pipe characters works but is fragile since it relies on exact string matching. Consider using a more conventional field name like'credentials'for the cross-field validation error.💡 Alternative approach using a dedicated error field
// In the Yup test function: return this.createError({ - path: 'apiKey | username | password', + path: 'credentials', message: intl.formatMessage(messages.validationApiKeyOrCredentials), }); // In the JSX error display: - {'apiKey | username | password' in errors && - typeof errors['apiKey | username | password'] === - 'string' && ( + {errors.credentials && typeof errors.credentials === 'string' && ( <div className="error"> - {errors['apiKey | username | password'] as string} + {errors.credentials} </div> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Settings/SettingsJellyfin.tsx` around lines 800 - 806, The component SettingsJellyfin is using a fragile compound error key 'apiKey | username | password'; change the cross-field validation to set and read a conventional key like 'credentials' instead. Update the validation logic that currently writes errors['apiKey | username | password'] to use errors['credentials'] and update the render block in the SettingsJellyfin component to check typeof errors['credentials'] === 'string' before rendering the <div className="error"> with that message; ensure any other code that sets or clears the compound key is updated to the new 'credentials' key so all validations remain consistent.server/routes/settings/index.ts (1)
569-592: DuplicateserverIndexcomputation and same potential out-of-bounds issue.
serverIndexis computed at lines 569-571 inside thereq.query.syncblock, then computed again at lines 580-582 outside that block. This is redundant and the second lookup could also hit-1if the server was removed.♻️ Proposed refactor to compute serverIndex once with defensive check
+ const serverIndex = settings.jellyfinServers.findIndex( + (jellyfinServer) => jellyfinServer.id === server.id + ); + + if (serverIndex === -1) { + return res.status(404).json({ message: 'Jellyfin server not found.' }); + } + if (req.query.sync) { // ... existing sync logic ... - const serverIndex = settings.jellyfinServers.findIndex( - (jellyfinServer) => jellyfinServer.id === server.id - ); - settings.jellyfinServers[serverIndex].libraries = newLibraries; } const enabledLibraries = req.query.enable ? (req.query.enable as string).split(',') : []; - const serverIndex = settings.jellyfinServers.findIndex( - (jellyfinServer) => jellyfinServer.id === server.id - ); - settings.jellyfinServers[serverIndex].libraries = settings.jellyfinServers[🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/settings/index.ts` around lines 569 - 592, Compute serverIndex once before the req.query.sync block (instead of twice) by finding the index in settings.jellyfinServers (the current code uses serverIndex = settings.jellyfinServers.findIndex(...)); add a defensive check for serverIndex === -1 and return an appropriate error response (e.g., 404) to avoid out-of-bounds access; then reuse that serverIndex for updating libraries (assigning newLibraries and mapping enabledLibraries) and call settings.save() as before (references: serverIndex, req.query.sync, newLibraries, enabledLibraries, settings.jellyfinServers, settings.save()).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/lib/availabilitySync.ts`:
- Around line 112-123: The code currently only logs when the Plex admin (queried
via userRepository.findOne) has no plexToken, causing later calls to
getPlexClient() to be undefined and Plex-backed items to be treated as missing;
change the branch in the hasPlexServers check so that if admin?.plexToken is
falsy you abort the sync early (e.g., throw or return from the containing
method) instead of just logging, ensuring plexAdminToken is not left undefined
and preventing mediaUpdater() from clearing Plex-backed media; update any
callers of this method as needed to handle the early exit.
In `@server/lib/scanners/jellyfin/index.ts`:
- Around line 483-537: Wrap the for-loop over jellyfinServers so each server
iteration is enclosed in its own try/catch: inside the loop that sets
this.currentServer and this.jfClient, add a try { ... } catch (err) {
this.log(`Error scanning server ${server.name || server.id}: ${err.message ||
err}`, 'error', { serverId: server.id }); continue; } around the existing logic
that calls getRecentlyAdded, getLibraryContents, and await
this.loop(this.processItem.bind(this), ...). Ensure you still set
this.currentLibrary and reset this.processedAnidbSeason per library inside the
try block so failures for one server don’t abort processing of subsequent
servers.
In `@server/lib/settings/index.ts`:
- Around line 904-921: getPrimaryMediaServerType currently derives the primary
type from the first element of getMediaServers() (which concatenates Plex before
Jellyfin) causing unintended flips when a new Plex server is added; instead,
make getPrimaryMediaServerType return a persisted/explicit choice when available
(check this.data.main.mediaServerType and return it if it's set and not
MediaServerType.NOT_CONFIGURED), otherwise compute a sensible default (e.g., if
only one unique mediaServerType exists return that, else return NOT_CONFIGURED).
Also ensure any code that writes main.mediaServerType (the assignment that
persists the primary) only updates it when the user explicitly changes the
selection, not implicitly based on server array ordering.
In `@server/lib/settings/migrations/0007_migrate_arr_tags.ts`:
- Around line 15-24: The buildArrUrl function can produce malformed URLs when
settings.baseUrl lacks a leading slash or contains a trailing slash; update
buildArrUrl to normalize baseUrl before composing the string (e.g., treat
undefined/empty as '', ensure it starts with a single leading '/' and strip any
trailing '/'), then use the normalized value when building the endpoint;
reference the buildArrUrl function and the settings.baseUrl property when making
this change so all generated ARR endpoints are valid.
In `@server/lib/settings/migrations/0009_multi_media_servers.ts`:
- Around line 45-48: The normalization logic in 0009_multi_media_servers.ts
incorrectly maps any non-EMBY server to the fallback, which can convert an
explicit Jellyfin setting; update the mediaServerType assignment so it preserves
server.mediaServerType when it equals MediaServerType.JELLYFIN (and any other
known explicit types) instead of forcing fallbackMediaServerType—i.e., check for
server.mediaServerType === MediaServerType.EMBY OR MediaServerType.JELLYFIN (or
generally if server.mediaServerType is a valid MediaServerType) and return it,
otherwise use fallbackMediaServerType.
In `@server/lib/settings/migrator.ts`:
- Around line 25-27: The migration file list created in migrator.ts (the
migrations constant built from fs.readdir(migrationsDir)) must be made
deterministic: after reading and filtering files, sort them by their leading
numeric prefix so migrations like 0001_... through 0009_... execute in numeric
filename order. Update the migrations assignment to sort the filtered array
(e.g., extract the /^\d+/ prefix or use filename.localeCompare with numeric:
true) before returning/using it so the migration execution order is stable and
predictable.
In `@server/routes/avatarproxy.ts`:
- Around line 35-55: The cache currently uses only server.id (cacheKey) while
the ImageProxy created by new ImageProxy('avatar', ...) embeds server.apiKey and
the derived deviceId in its headers; update the cache key used with
avatarImageProxies to include the server.apiKey and the deviceId (derived from
admin?.jellyfinDeviceId || 'BOT_seerr') so that a new ImageProxy is created when
credentials or deviceId change (i.e., build cacheKey from server.id +
server.apiKey + deviceId or similar), and ensure all references to cacheKey in
this block use the new composite key so stale authorization headers are not
reused.
- Around line 150-156: The code currently falls back to a user's linked/primary
Jellyfin server when an explicit requestedServerId is provided but not found;
change the logic so if requestedServerId is present you call
getJellyfinServer(requestedServerId) and if that returns undefined you fail fast
(e.g., send a 400/404 error response or throw) instead of falling back to
user?.jellyfinServerId or primary. Update the block that assigns server
(currently using requestedServerId, getJellyfinServer, and user) to first check
requestedServerId and handle the not-found case immediately; only when
requestedServerId is absent should you resolve server from the user lookup or
default.
In `@server/routes/settings/index.ts`:
- Around line 346-358: The code assumes settings.plexServers[serverIndex] exists
but serverIndex can be -1; add a defensive check after computing serverIndex to
ensure serverIndex !== -1 (or that settings.plexServers[serverIndex] is truthy)
and return an appropriate error response (e.g., 404 or 409) before mutating
libraries or calling settings.save; update the block that uses serverIndex,
settings.plexServers, server.id, enabledLibraries and settings.save to bail out
early when the server is not found to avoid array index out-of-bounds errors.
In `@src/components/Settings/SettingsLayout.tsx`:
- Around line 103-124: getAvailableMediaServerName currently returns the
Jellyfin-specific label as soon as any Jellyfin server exists, which drops the
shared label for mixed Jellyfin+Emby installs; fix by computing hasJellyfin and
hasEmby from settings.currentSettings.mediaServers (checking
MediaServerType.JELLYFIN and MediaServerType.EMBY), then: if both are true
return intl.formatMessage(messages.menuJellyfinEmbySettings); else if only
hasJellyfin return intl.formatMessage(messages.menuJellyfinSettings, {
mediaServerName: 'Jellyfin' }); else if only hasEmby return
intl.formatMessage(messages.menuJellyfinSettings, { mediaServerName: 'Emby' });
otherwise return intl.formatMessage(messages.menuJellyfinEmbySettings). Ensure
this logic is implemented inside getAvailableMediaServerName.
In `@src/components/UserList/index.tsx`:
- Around line 202-231: The import flow ignores Plex server identity: update the
frontend and backend to support selecting a specific Plex server. On the
frontend, change importSources to enumerate each Plex media server from
settings.currentSettings.mediaServers (filter by MediaServerType.PLEX) and
create a source object per server (use getMediaServerDisplayName for the label),
add a plexServerId prop to the PlexImportModal component and pass the selected
server's id from the UserList component where the modal is opened (the
invocation near the current modal launch point). On the backend, update the
/api/v1/settings/plex/users and /api/v1/user/import-from-plex handlers to accept
a serverId (query or body) and scope Plex calls to that server instead of always
using the primary/admin token.
---
Outside diff comments:
In `@server/lib/availabilitySync.ts`:
- Around line 858-900: The current logic populates this.plexSeasonsCache with
all season containers so once any season has a 4K episode other seasons are
still considered available; update the code so the cache and lookups only
include seasons that actually contain a 4K episode when is4k is true (or
alternatively make seasonExistsInPlex() perform the per-season episode-level
check when is4k is true). Concretely: when building this.plexSeasonsCache (and
in the same pattern at the other block mentioned), iterate each season.ratingKey
and call plexClient.getChildrenMetadata to detect Media items with width >=
2000, then store only those seasons in plexSeasonsCache (or make
seasonExistsInPlex(…) query episodes and check Media widths when is4k is true)
so finalSeasons4k only contains seasons that truly have 4K episodes.
In `@server/lib/settings/index.ts`:
- Around line 727-773: fullPublicSettings uses getPrimaryJellyfinLikeServer()
without considering the active mediaServerType, which can return data from the
wrong Jellyfin/Emby instance; update calls to getPrimaryJellyfinLikeServer()
inside fullPublicSettings (and the similar block around the later 927-933
section) to pass the currently active type returned by
getPrimaryMediaServerType() (e.g.,
getPrimaryJellyfinLikeServer(this.getPrimaryMediaServerType())) so
jellyfinExternalHost, jellyfinForgotPasswordUrl, and jellyfinServerName are
selected from the active Jellyfin-like server matching mediaServerType.
In `@src/components/UserList/JellyfinImportModal.tsx`:
- Around line 278-284: The empty-state Alert in JellyfinImportModal uses
settings.currentSettings.mediaServerType (global) instead of the component prop
mediaServerType, which can show the wrong server name; update the Alert call
that builds intl.formatMessage(messages.noJellyfinuserstoimport, ...) to use the
mediaServerType prop (and map it to 'Emby' or 'Jellyfin' the same way the
current ternary does) so the displayed mediaServerName reflects the server being
imported; ensure you update the reference inside the JSX where
messages.noJellyfinuserstoimport is used (within the Alert component) and leave
other uses of settings.currentSettings untouched.
---
Nitpick comments:
In `@server/routes/auth.ts`:
- Around line 739-745: Currently the code throws an Error when jellyfinServer is
not found, which triggers duplicate error logs and skips explicit
device-deletion handling; replace the throw with an early return (or continue if
inside a loop) so that when settings.jellyfinServers.find(server => server.id
=== user.jellyfinServerId) yields no jellyfinServer you simply skip the device
deletion logic and allow session destruction to proceed normally; update the
block referencing jellyfinServer, settings.jellyfinServers, and
user.jellyfinServerId to use return/continue and optionally log a single, clear
info/debug message instead of throwing.
- Around line 251-259: The current ternary for jellyfinMediaServerType
redundantly falls back to configuredServer?.mediaServerType even though
resolvedServerType was already derived from that, creating unreachable logic;
change jellyfinMediaServerType to only accept resolvedServerType when it equals
MediaServerType.JELLYFIN or MediaServerType.EMBY (otherwise undefined or null),
and keep loginJellyfinMediaServerType as jellyfinMediaServerType ??
MediaServerType.JELLYFIN so the intent is clear; update the variables
resolvedServerType, jellyfinMediaServerType, and loginJellyfinMediaServerType in
server/routes/auth.ts accordingly.
In `@server/routes/discover.ts`:
- Around line 709-719: The mapper currently falls back to mapTvResult for any
non-movie/person/collection, which could mis-handle future TMDB types; update
the mapper in the mapper: (result: TrendingResult, media?: Media): Results => {
... } block to explicitly check isTv(result) before calling mapTvResult, and add
an else branch that logs a warning (or returns a safe fallback/skip) for unknown
types (use existing logger or console.warn) so new/unexpected result kinds are
not silently treated as TV.
In `@server/routes/settings/index.ts`:
- Around line 569-592: Compute serverIndex once before the req.query.sync block
(instead of twice) by finding the index in settings.jellyfinServers (the current
code uses serverIndex = settings.jellyfinServers.findIndex(...)); add a
defensive check for serverIndex === -1 and return an appropriate error response
(e.g., 404) to avoid out-of-bounds access; then reuse that serverIndex for
updating libraries (assigning newLibraries and mapping enabledLibraries) and
call settings.save() as before (references: serverIndex, req.query.sync,
newLibraries, enabledLibraries, settings.jellyfinServers, settings.save()).
In `@src/components/Settings/SettingsJellyfin.tsx`:
- Around line 800-806: The component SettingsJellyfin is using a fragile
compound error key 'apiKey | username | password'; change the cross-field
validation to set and read a conventional key like 'credentials' instead. Update
the validation logic that currently writes errors['apiKey | username |
password'] to use errors['credentials'] and update the render block in the
SettingsJellyfin component to check typeof errors['credentials'] === 'string'
before rendering the <div className="error"> with that message; ensure any other
code that sets or clears the compound key is updated to the new 'credentials'
key so all validations remain consistent.
In `@src/components/Settings/SettingsPlex.tsx`:
- Around line 262-291: The early return in syncLibraries is redundant—remove the
initial if (serverId === 'new') return; and rely on the existing params
construction that only includes serverId when serverId !== 'new'; keep
setIsSyncing(true)/finally cleanup, the params object (sync: true and
...(serverId !== 'new' ? { serverId } : {})), and the activeLibraries handling,
so behavior is unchanged but the redundant check is removed from syncLibraries
(refer to syncLibraries, params, selectedServerId, and activeLibraries).
In `@src/hooks/useDeepLinks.ts`:
- Around line 29-31: isIOSDevice is a pure helper defined at component scope but
only used inside useEffect; to avoid future stale-dep or readability issues,
either move the isIOSDevice function into the useEffect block where it's used or
hoist it entirely outside the component as a top-level utility function (keeping
the same implementation using navigator.userAgent and navigator.maxTouchPoints);
update any references to isIOSDevice accordingly and ensure ESLint warnings
about hooks/deps are resolved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c2ff1de7-ec2a-4f43-a43c-d67b2f2baa63
📒 Files selected for processing (52)
seerr-api.ymlserver/api/jellyfin.tsserver/api/plexapi.tsserver/api/rating/imdbRadarrProxy.tsserver/entity/Media.tsserver/entity/User.tsserver/interfaces/api/settingsInterfaces.tsserver/job/schedule.tsserver/lib/availabilitySync.tsserver/lib/scanners/baseScanner.tsserver/lib/scanners/jellyfin/index.tsserver/lib/scanners/plex/index.tsserver/lib/settings/index.tsserver/lib/settings/migrations/0001_migrate_hostname.tsserver/lib/settings/migrations/0002_migrate_apitokens.tsserver/lib/settings/migrations/0003_emby_media_server_type.tsserver/lib/settings/migrations/0004_migrate_region_setting.tsserver/lib/settings/migrations/0005_migrate_network_settings.tsserver/lib/settings/migrations/0006_remove_lunasea.tsserver/lib/settings/migrations/0007_migrate_arr_tags.tsserver/lib/settings/migrations/0008_migrate_blacklist_to_blocklist.tsserver/lib/settings/migrations/0009_multi_media_servers.tsserver/lib/settings/migrations/types.tsserver/lib/settings/migrator.tsserver/migration/postgres/1772100000000-AddMediaServerSourceIds.tsserver/migration/sqlite/1772100000000-AddMediaServerSourceIds.tsserver/routes/auth.tsserver/routes/avatarproxy.tsserver/routes/discover.tsserver/routes/settings/index.tsserver/routes/user/index.tsserver/routes/user/usersettings.tsserver/utils/mediaServers.tssrc/components/Login/JellyfinLogin.tsxsrc/components/Login/index.tsxsrc/components/MovieDetails/index.tsxsrc/components/RegionSelector/index.tsxsrc/components/Settings/SettingsJellyfin.tsxsrc/components/Settings/SettingsLayout.tsxsrc/components/Settings/SettingsPlex.tsxsrc/components/Settings/SettingsUsers/index.tsxsrc/components/Setup/index.tsxsrc/components/TvDetails/index.tsxsrc/components/UserList/JellyfinImportModal.tsxsrc/components/UserList/index.tsxsrc/components/UserProfile/UserSettings/UserLinkedAccountsSettings/LinkJellyfinModal.tsxsrc/components/UserProfile/UserSettings/UserLinkedAccountsSettings/index.tsxsrc/context/SettingsContext.tsxsrc/hooks/useDeepLinks.tssrc/pages/_app.tsxsrc/utils/countryFlags.tssrc/utils/mediaServers.ts
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/using-seerr/users/adding-users.mdx (1)
42-51:⚠️ Potential issue | 🟡 MinorMissing multi-server notice for Plex tab.
The Jellyfin and Emby tabs both include a notice about separate import actions when multiple servers are configured, but the Plex tab is missing this notice. Since the PR adds support for multiple Plex instances as well, this tab should have the same informational text for consistency.
📝 Suggested addition after line 43
Clicking the **Import Plex Users** button on the **User List** page will fetch the list of users with access to the Plex server from [plex.tv](https://www.plex.tv/), and add them to Seerr automatically. + If multiple Plex servers are configured, Seerr will show a separate import action for each configured server. + Importing Plex users is not required, however. Any user with access to the Plex server can log in to Seerr even if they have not been imported, and will be assigned the configured [default permissions](/using-seerr/settings/users#default-permissions) upon their first login.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/using-seerr/users/adding-users.mdx` around lines 42 - 51, Add the same multi-server informational notice to the Plex TabItem (TabItem value="plex" label="Plex") that exists in the Jellyfin and Emby tabs: after the first paragraph inside the Plex TabItem, insert a short note explaining that if multiple Plex servers are configured each server must be imported separately and indicate where to manage/import them (mirror the wording and placement used in the Jellyfin/Emby TabItem blocks to keep consistency).
🧹 Nitpick comments (1)
docs/using-seerr/backups.md (1)
14-18: Consider clarifying the synchronization behavior between legacy and array configurations.The note accurately describes the dual storage structure but could be more explicit about what happens when both exist. Based on the synchronization logic in
server/lib/settings/index.ts, the system merges the first array entry with the legacy object fields, with array entries taking precedence for server identity.For users manually restoring configurations, it might be helpful to explicitly state that:
- The arrays are the primary storage for multi-server setups
- If both exist, the system uses array entries but synchronizes certain fields with legacy objects
- When restoring, users should primarily focus on the array entries
This would reduce ambiguity for users who encounter both structures in their
settings.jsonduring backup/restore operations.📝 Suggested clarity improvement
:::note -For multi-server setups, the active media server entries are stored in the `plexServers` and `jellyfinServers` arrays. +For multi-server setups, the active media server entries are stored in the `plexServers` and `jellyfinServers` arrays. These arrays are the primary storage for server configurations. -The legacy `plex` and `jellyfin` objects are still kept for backward compatibility, but if you manually edit or restore a multi-server configuration, make sure the array entries contain the server details you want Seerr to use. +The legacy `plex` and `jellyfin` objects are still kept for backward compatibility (Seerr synchronizes the first array entry with these legacy fields). When manually editing or restoring a multi-server configuration, focus on the array entries—they contain the authoritative server details Seerr will use. :::🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/using-seerr/backups.md` around lines 14 - 18, Update the backups.md note to explicitly describe the sync behavior implemented in server/lib/settings/index.ts: state that plexServers/jellyfinServers arrays are the canonical storage for multi-server setups, that when both the legacy plex/jellyfin objects and arrays exist the system merges the first array entry into the legacy object but array entries take precedence for server identity, and therefore when restoring backups users should primarily edit the array entries (the legacy objects may be synchronized from the first array entry but should not be relied on as the source of truth).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/i18n/locale/en.json`:
- Line 1125: The current localization key components.Settings.addServer contains
a Plex-specific string ("Add New Plex Server") but is also used by
SettingsJellyfin.tsx/SettingsEmby flows; change the i18n entry to a generic
label (e.g., "Add New Server") or create a new Plex-specific key and update
usages accordingly: update components.Settings.addServer in
src/i18n/locale/en.json to a neutral value and ensure SettingsJellyfin.tsx (and
any Emby components) reference that generic key, while Plex-specific UI uses a
new key like components.Settings.addPlexServer if needed.
---
Outside diff comments:
In `@docs/using-seerr/users/adding-users.mdx`:
- Around line 42-51: Add the same multi-server informational notice to the Plex
TabItem (TabItem value="plex" label="Plex") that exists in the Jellyfin and Emby
tabs: after the first paragraph inside the Plex TabItem, insert a short note
explaining that if multiple Plex servers are configured each server must be
imported separately and indicate where to manage/import them (mirror the wording
and placement used in the Jellyfin/Emby TabItem blocks to keep consistency).
---
Nitpick comments:
In `@docs/using-seerr/backups.md`:
- Around line 14-18: Update the backups.md note to explicitly describe the sync
behavior implemented in server/lib/settings/index.ts: state that
plexServers/jellyfinServers arrays are the canonical storage for multi-server
setups, that when both the legacy plex/jellyfin objects and arrays exist the
system merges the first array entry into the legacy object but array entries
take precedence for server identity, and therefore when restoring backups users
should primarily edit the array entries (the legacy objects may be synchronized
from the first array entry but should not be relied on as the source of truth).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 709d9445-36ff-46cd-9ff3-994b31a0bc7f
📒 Files selected for processing (4)
docs/using-seerr/backups.mddocs/using-seerr/settings/mediaserver.mdxdocs/using-seerr/users/adding-users.mdxsrc/i18n/locale/en.json
7 tasks
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
seerr-api.yml (1)
4312-4324:⚠️ Potential issue | 🟠 MajorAlign
/user/import-from-jellyfinOpenAPI contract with handler expectations.Line 4312 currently allows an empty body, but the handler dereferences
body.serverId(Line 686 inserver/routes/user/index.ts) and iteratesbody.jellyfinUserIds(Line 723). A request valid per spec can still hit a 500 path.🛠️ Proposed OpenAPI fix
/user/import-from-jellyfin: post: @@ requestBody: - required: false + required: true content: application/json: schema: type: object properties: jellyfinUserIds: type: array items: type: string serverId: type: string nullable: true + required: + - jellyfinUserIds🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@seerr-api.yml` around lines 4312 - 4324, The OpenAPI schema for POST /user/import-from-jellyfin allows an empty body but the handler dereferences body.serverId and iterates body.jellyfinUserIds; update the request body schema so the body is required and must include jellyfinUserIds and serverId: in the content/application/json schema add required: ["jellyfinUserIds","serverId"], mark jellyfinUserIds as an array of strings (add minItems: 1 if you want to prevent empty arrays) and keep serverId as nullable if needed but present, so the runtime handler (which accesses serverId and loops jellyfinUserIds) cannot receive an undefined body or missing properties.src/components/Settings/SettingsPlex.tsx (1)
800-805:⚠️ Potential issue | 🟠 MajorDon't gate the global Plex scan on the currently selected server.
This button still starts the global
/api/v1/settings/plex/syncscanner, but its disabled state now depends only onactiveLibrariesfor the selected server. In a multi-server setup, selecting an empty/new server prevents starting a scan for other configured Plex servers.Suggested direction
- disabled={isSyncing || !activeLibraries.length} + disabled={isSyncing || !hasAnyEnabledLibraries}
hasAnyEnabledLibrariesshould aggregate enabled libraries across all configured Plex servers instead of reading only the current dropdown selection.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Settings/SettingsPlex.tsx` around lines 800 - 805, The button's disabled state currently uses activeLibraries (selected server) which prevents starting the global Plex scan when another server has enabled libraries; change the disabled prop on the Button (the one rendering when !dataSync?.running and calling startScan()) to use the aggregated hasAnyEnabledLibraries flag instead of activeLibraries (i.e., disabled={isSyncing || !hasAnyEnabledLibraries}), and ensure hasAnyEnabledLibraries is computed by iterating all Plex server configs/libraries (not just the current selection) so it reflects any enabled libraries across all configured Plex servers.server/lib/settings/index.ts (1)
729-775:⚠️ Potential issue | 🟠 MajorKeep the legacy Jellyfin/Emby public fields aligned with the selected primary type.
mediaServerTypenow correctly honors the explicit primary selection, butjellyfinServeris still taken from the first Jellyfin-like server regardless of type. In a mixed Jellyfin+Emby setup,mediaServerType: EMBYcan be paired with Jellyfin host/name/forgot-password metadata here. Pick the compatibility server bygetPrimaryMediaServerType()when it resolves toJELLYFINorEMBY.Suggested fix
get fullPublicSettings(): FullPublicSettings { const mediaServers = this.getMediaServers(); - const jellyfinServer = this.getPrimaryJellyfinLikeServer(); + const primaryMediaServerType = this.getPrimaryMediaServerType(); + const jellyfinServer = + primaryMediaServerType === MediaServerType.JELLYFIN || + primaryMediaServerType === MediaServerType.EMBY + ? this.getPrimaryJellyfinLikeServer(primaryMediaServerType) + : this.getPrimaryJellyfinLikeServer(); return { @@ - mediaServerType: this.getPrimaryMediaServerType(), + mediaServerType: primaryMediaServerType,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/lib/settings/index.ts` around lines 729 - 775, The code picks a Jellyfin-like server via getPrimaryJellyfinLikeServer() regardless of the selected primary media server type, causing Jellyfin fields to be shown even when getPrimaryMediaServerType() returns EMBY; update the selection so the compatibility server is chosen by matching mediaServerType to getPrimaryMediaServerType() (e.g., filter the result of getMediaServers() for server.mediaServerType === getPrimaryMediaServerType() and that server being Jellyfin-like) and only fall back to getPrimaryJellyfinLikeServer() if no matching primary-type server exists, then use that chosen server for jellyfinExternalHost, jellyfinForgotPasswordUrl and jellyfinServerName.
♻️ Duplicate comments (2)
server/routes/avatarproxy.ts (1)
35-55:⚠️ Potential issue | 🟠 MajorRefresh the cached proxy when credentials change.
The cache key is still only
server.id, but the cachedImageProxyembedsserver.apiKeyanddeviceIdin its authorization header. Rotating either value will keep reusing stale credentials until the process restarts.Suggested fix
- const cacheKey = server.id; - const cachedProxy = avatarImageProxies.get(cacheKey); - if (cachedProxy) { - return cachedProxy; - } - const userRepository = getRepository(User); const admin = await userRepository.findOne({ where: { id: 1 }, select: ['id', 'jellyfinUserId', 'jellyfinDeviceId'], order: { id: 'ASC' }, }); const deviceId = admin?.jellyfinDeviceId || 'BOT_seerr'; + const cacheKey = `${server.id}:${server.apiKey}:${deviceId}`; + const cachedProxy = avatarImageProxies.get(cacheKey); + if (cachedProxy) { + return cachedProxy; + } const imageProxy = new ImageProxy('avatar', '', { headers: { 'X-Emby-Authorization': `MediaBrowser Client="Seerr", Device="Seerr", DeviceId="${deviceId}", Version="${getAppVersion()}", Token="${server.apiKey}"`, }, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/avatarproxy.ts` around lines 35 - 55, The current cache uses cacheKey = server.id but the ImageProxy stored in avatarImageProxies embeds server.apiKey and a deviceId (from admin.jellyfinDeviceId), so rotating credentials keeps returning a proxy with stale headers; update the cache key to include the credential-identifying values (e.g., combine server.id, server.apiKey and the resolved deviceId) before checking avatarImageProxies, or detect mismatched headers and recreate the ImageProxy; ensure the symbols involved are cacheKey, avatarImageProxies, ImageProxy, server.apiKey, deviceId and the admin lookup (userRepository/findOne) so the lookup and set use the expanded key and the new ImageProxy is created when credentials change.server/routes/settings/index.ts (1)
573-596:⚠️ Potential issue | 🟡 MinorReintroduce the missing
serverIndexguard in the Jellyfin library path.The earlier
serverlookup does not guaranteesettings.jellyfinServers[serverIndex]still exists after the awaited Jellyfin calls. If the server is deleted concurrently, the assignments at Lines 577 and 588 dereference-1and turn the request into a 500. This is the same race the Plex branch already guards against.Suggested fix
const serverIndex = settings.jellyfinServers.findIndex( (jellyfinServer) => jellyfinServer.id === server.id ); + + if (serverIndex === -1 || !settings.jellyfinServers[serverIndex]) { + return res.status(404).json({ message: 'Jellyfin server not found.' }); + } settings.jellyfinServers[serverIndex].libraries = newLibraries; @@ const serverIndex = settings.jellyfinServers.findIndex( (jellyfinServer) => jellyfinServer.id === server.id ); + + if (serverIndex === -1 || !settings.jellyfinServers[serverIndex]) { + return res.status(404).json({ message: 'Jellyfin server not found.' }); + } settings.jellyfinServers[serverIndex].libraries = settings.jellyfinServers[🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/settings/index.ts` around lines 573 - 596, The code dereferences settings.jellyfinServers[serverIndex] after awaits, risking serverIndex === -1 if the server was deleted concurrently; add the same guard used in the Plex branch: re-compute/find serverIndex (or check that serverIndex !== -1) immediately before modifying settings.jellyfinServers[serverIndex].libraries and before mapping enabled flags, and if not found return a 404/appropriate error response instead of proceeding to assignment; ensure the checks surround both the assignment of newLibraries and the enabled-libraries mapping so you never index with -1.
🧹 Nitpick comments (4)
server/lib/settings/migrations/0007_migrate_arr_tags.ts (2)
107-109: Same silent skip issue for Sonarr instances.Apply the same warning pattern here for consistency with both the Radarr handling suggestion and the error handling at lines 144-151.
Proposed fix
if (!sonarrSettings.apiKey || !sonarrSettings.hostname) { + writeMigrationWarning( + `Skipping Sonarr instance "${sonarrSettings.name ?? 'unnamed'}": missing apiKey or hostname.` + ); continue; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/lib/settings/migrations/0007_migrate_arr_tags.ts` around lines 107 - 109, The Sonarr loop silently skips instances when apiKey or hostname are missing; update the check in 0007_migrate_arr_tags.ts to log a warning instead of silently continuing: when encountering sonarrSettings with missing sonarrSettings.apiKey or sonarrSettings.hostname, call the same warning used for Radarr/Arr handling (e.g., processLogger.warn or logger.warn) including identifying info (instance id/name from sonarrSettings) and which field is missing, then continue; ensure the log matches the pattern used in the Radarr branch and the error handling around lines 144-151 for consistency.
55-57: Silent skip whenapiKeyorhostnameis missing may obscure misconfiguration.When these required fields are missing, the migration silently continues without notifying the user. This is inconsistent with the error handling at lines 92-99, which emits a warning. Users with incomplete configurations won't understand why their Radarr tags weren't migrated.
Consider emitting a warning when skipping due to missing configuration:
Proposed fix
if (!radarrSettings.apiKey || !radarrSettings.hostname) { + writeMigrationWarning( + `Skipping Radarr instance "${radarrSettings.name ?? 'unnamed'}": missing apiKey or hostname.` + ); continue; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/lib/settings/migrations/0007_migrate_arr_tags.ts` around lines 55 - 57, The code currently silently continues when radarrSettings.apiKey or radarrSettings.hostname is missing; change that to emit a warning instead of silently skipping by reusing the same warning/logger used later in this migration (the warning emitted in the API error handling block). Specifically, inside the check for radarrSettings.apiKey || radarrSettings.hostname, call the migration warning/logger with a message that includes identifying info from radarrSettings (e.g., name or id) and the reason ("missing apiKey or hostname") before continuing, so skipped Radarr configs are visible to users.server/routes/settings/index.ts (1)
603-617: Evict the avatar proxy cache when a Jellyfin/Emby server is deleted.
server/routes/avatarproxy.ts:14-57cachesImageProxyinstances inavatarImageProxiesbyserver.id. Removing the settings entry here leaves that proxy, including its auth header, resident for the rest of the process. Add a small invalidation hook on this delete path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/settings/index.ts` around lines 603 - 617, When deleting a Jellyfin/Emby server in the settingsRoutes.delete handler, also evict any cached avatar ImageProxy for that server from the avatarImageProxies cache to avoid leaving its auth headers alive; after computing removedServer (from settings.jellyfinServers.splice) use removedServer.id to look up avatarImageProxies[removedServer.id], if a proxy exists call its cleanup method if available (e.g., destroy/close/stop) and then delete the key from avatarImageProxies (avatarImageProxies[removedServer.id] and delete avatarImageProxies[removedServer.id]); this ensures the cache in server/routes/avatarproxy.ts is invalidated when a server is removed.src/i18n/locale/en.json (1)
1264-1264: Consider including server instance identity in Plex sync failure toast.In multi-server setups, this message is clearer if it names the failing server.
💡 Suggested copy refinement
- "components.Settings.toastPlexSyncFailure": "Something went wrong while syncing Plex libraries.", + "components.Settings.toastPlexSyncFailure": "Something went wrong while syncing libraries for {serverName}.",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/i18n/locale/en.json` at line 1264, Update the localization entry "components.Settings.toastPlexSyncFailure" to include a server instance placeholder (e.g. "Something went wrong while syncing Plex libraries for {serverName}.") and then update the call sites that show this toast (the code that invokes the i18n key) to pass the failing server instance name in the interpolation object (e.g. t("components.Settings.toastPlexSyncFailure", { serverName })). Ensure the placeholder syntax you use matches the project's i18n interpolation format and update any tests/usage accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/lib/availabilitySync.ts`:
- Around line 839-846: The code currently picks a single serverId (via
plexServerId/plexServerId4k) and probes only that instance; change the logic to
iterate over all candidate server IDs for the variant (e.g., plexServerIds /
plexServerId4k arrays or fallback to the single plexServerId fields) and for
each serverId call getPlexClient(serverId), compute a per-server cacheKey
(`${serverId}-${ratingKey}`), and probe until any client reports the item (set
existsInPlex = true) while preserving preventSeasonSearch semantics per-server;
update any caching or short-circuit logic to use the per-server cacheKey and
only mark the media deleted if no server reports it exists. Apply the same
iterative/server-list fix to the duplicate block referenced by mediaUpdater so
both Plex and Jellyfin checks consider all registered servers rather than a
single stored serverId.
In `@server/lib/settings/migrations/0001_migrate_hostname.ts`:
- Line 7: The expression in the declaration of const useSsl uses
protocolMatch?.[1].toLowerCase() which can call toLowerCase on undefined when
protocolMatch is null; update the guard so you only call toLowerCase when the
capture exists (e.g., use protocolMatch?.[1]?.toLowerCase() === 'https' or check
protocolMatch && protocolMatch[1] before calling toLowerCase) in the const
useSsl assignment to avoid the TypeError.
In `@server/routes/auth.ts`:
- Around line 64-68: The route currently validates Plex access only against the
legacy/default machine by calling checkUserAccess(account.id) without a machine
id; update the logic to iterate over settings.plexServers and call
checkUserAccess for each server using its machine identifier (e.g.,
server.machineId or server.machineIdentifier) until one returns success, and use
that result to authorize the user; apply the same change to the other occurrence
handling lines 122-126 so every configured Plex server is checked instead of
only the default.
- Around line 248-279: The code currently allows falling back to manual hostname
login when body.serverId is provided but no matching configuredServer is found;
update the auth route to explicitly reject unknown server IDs: after computing
configuredServer (from body.serverId and settings.jellyfinServers.find(...)), if
body.serverId is present and configuredServer is undefined, immediately return
an error (e.g., 400/500) indicating unknown serverId rather than proceeding to
the manual hostname flow; ensure this check sits before the existing hostname
presence/absence checks (the branches that reference configuredServer and
body.hostname) so you never authenticate/update users against the wrong host.
In `@server/routes/settings/index.ts`:
- Around line 167-175: The code seeds a new Plex/Jellyfin/Emby server object
from the global primary settings (spreading settings.plex / settings.jellyfin)
when existingServer is undefined, which copies per-server state like apiKey;
change the construction of plexServer (and the analogous jellyfin/emby blocks)
to only spread existingServer when it exists and otherwise start from a blank
default object (e.g., { id: req.body.id ?? randomUUID(), mediaServerType:
MediaServerType.PLEX } plus req.body and enforced mediaServerType) — remove any
fallback spread of settings.plex/settings.jellyfin so new entries are not
initialized from the primary server config and retain per-server defaults
instead.
- Around line 148-154: The GET /plex handler currently uses
getPlexServerFromRequest(...) and falls back to getSettings().plex even when a
serverId was provided but no server exists; change the logic in the
settingsRoutes.get('/plex') handler so that if req.query.serverId is present and
getPlexServerFromRequest(...) returns null/undefined you respond with
res.status(404).json({ error: 'Plex server not found' }) instead of falling back
to getSettings().plex; keep the existing fallback to getSettings().plex only
when serverId is omitted. Apply the same fix to the corresponding jellyfin
handler (the handler that calls
getJellyfinServerFromRequest/getSettings().jellyfin).
In `@server/routes/user/index.ts`:
- Around line 729-747: The code constructs a new User using possibly undefined
jellyfin data; guard against stale/invalid IDs by checking jellyfinUser (from
jellyfinUsersById.get(jellyfinUserId)) before building the payload — if
jellyfinUser is undefined, skip creating the user (or log/warn and continue)
instead of passing undefined into the new User constructor; update the block
around jellyfinUser, findJellyfinUser, and the new User(...) creation to
early-return/continue when jellyfinUser is falsy and ensure any dependent fields
(jellyfinUsername, jellyfinUserId, jellyfinDeviceId, email, avatar) are only set
when jellyfinUser exists.
In `@src/components/Settings/SettingsPlex.tsx`:
- Around line 251-260: The auto-select effect should only run when this is the
very first Plex server being discovered to avoid snapping the UI when the user
intentionally chooses "new" later; update the useEffect condition (and the other
similar blocks at the other locations) to check plexServers.length === 1 in
addition to hasInitializedServerSelection === false and selectedServerId ===
'new', then call setSelectedServerId(plexServers[0].id) and
setHasInitializedServerSelection(true); this ensures auto-selection happens only
once when the first server appears and won't interfere when adding a second
server.
---
Outside diff comments:
In `@seerr-api.yml`:
- Around line 4312-4324: The OpenAPI schema for POST /user/import-from-jellyfin
allows an empty body but the handler dereferences body.serverId and iterates
body.jellyfinUserIds; update the request body schema so the body is required and
must include jellyfinUserIds and serverId: in the content/application/json
schema add required: ["jellyfinUserIds","serverId"], mark jellyfinUserIds as an
array of strings (add minItems: 1 if you want to prevent empty arrays) and keep
serverId as nullable if needed but present, so the runtime handler (which
accesses serverId and loops jellyfinUserIds) cannot receive an undefined body or
missing properties.
In `@server/lib/settings/index.ts`:
- Around line 729-775: The code picks a Jellyfin-like server via
getPrimaryJellyfinLikeServer() regardless of the selected primary media server
type, causing Jellyfin fields to be shown even when getPrimaryMediaServerType()
returns EMBY; update the selection so the compatibility server is chosen by
matching mediaServerType to getPrimaryMediaServerType() (e.g., filter the result
of getMediaServers() for server.mediaServerType === getPrimaryMediaServerType()
and that server being Jellyfin-like) and only fall back to
getPrimaryJellyfinLikeServer() if no matching primary-type server exists, then
use that chosen server for jellyfinExternalHost, jellyfinForgotPasswordUrl and
jellyfinServerName.
In `@src/components/Settings/SettingsPlex.tsx`:
- Around line 800-805: The button's disabled state currently uses
activeLibraries (selected server) which prevents starting the global Plex scan
when another server has enabled libraries; change the disabled prop on the
Button (the one rendering when !dataSync?.running and calling startScan()) to
use the aggregated hasAnyEnabledLibraries flag instead of activeLibraries (i.e.,
disabled={isSyncing || !hasAnyEnabledLibraries}), and ensure
hasAnyEnabledLibraries is computed by iterating all Plex server
configs/libraries (not just the current selection) so it reflects any enabled
libraries across all configured Plex servers.
---
Duplicate comments:
In `@server/routes/avatarproxy.ts`:
- Around line 35-55: The current cache uses cacheKey = server.id but the
ImageProxy stored in avatarImageProxies embeds server.apiKey and a deviceId
(from admin.jellyfinDeviceId), so rotating credentials keeps returning a proxy
with stale headers; update the cache key to include the credential-identifying
values (e.g., combine server.id, server.apiKey and the resolved deviceId) before
checking avatarImageProxies, or detect mismatched headers and recreate the
ImageProxy; ensure the symbols involved are cacheKey, avatarImageProxies,
ImageProxy, server.apiKey, deviceId and the admin lookup
(userRepository/findOne) so the lookup and set use the expanded key and the new
ImageProxy is created when credentials change.
In `@server/routes/settings/index.ts`:
- Around line 573-596: The code dereferences
settings.jellyfinServers[serverIndex] after awaits, risking serverIndex === -1
if the server was deleted concurrently; add the same guard used in the Plex
branch: re-compute/find serverIndex (or check that serverIndex !== -1)
immediately before modifying settings.jellyfinServers[serverIndex].libraries and
before mapping enabled flags, and if not found return a 404/appropriate error
response instead of proceeding to assignment; ensure the checks surround both
the assignment of newLibraries and the enabled-libraries mapping so you never
index with -1.
---
Nitpick comments:
In `@server/lib/settings/migrations/0007_migrate_arr_tags.ts`:
- Around line 107-109: The Sonarr loop silently skips instances when apiKey or
hostname are missing; update the check in 0007_migrate_arr_tags.ts to log a
warning instead of silently continuing: when encountering sonarrSettings with
missing sonarrSettings.apiKey or sonarrSettings.hostname, call the same warning
used for Radarr/Arr handling (e.g., processLogger.warn or logger.warn) including
identifying info (instance id/name from sonarrSettings) and which field is
missing, then continue; ensure the log matches the pattern used in the Radarr
branch and the error handling around lines 144-151 for consistency.
- Around line 55-57: The code currently silently continues when
radarrSettings.apiKey or radarrSettings.hostname is missing; change that to emit
a warning instead of silently skipping by reusing the same warning/logger used
later in this migration (the warning emitted in the API error handling block).
Specifically, inside the check for radarrSettings.apiKey ||
radarrSettings.hostname, call the migration warning/logger with a message that
includes identifying info from radarrSettings (e.g., name or id) and the reason
("missing apiKey or hostname") before continuing, so skipped Radarr configs are
visible to users.
In `@server/routes/settings/index.ts`:
- Around line 603-617: When deleting a Jellyfin/Emby server in the
settingsRoutes.delete handler, also evict any cached avatar ImageProxy for that
server from the avatarImageProxies cache to avoid leaving its auth headers
alive; after computing removedServer (from settings.jellyfinServers.splice) use
removedServer.id to look up avatarImageProxies[removedServer.id], if a proxy
exists call its cleanup method if available (e.g., destroy/close/stop) and then
delete the key from avatarImageProxies (avatarImageProxies[removedServer.id] and
delete avatarImageProxies[removedServer.id]); this ensures the cache in
server/routes/avatarproxy.ts is invalidated when a server is removed.
In `@src/i18n/locale/en.json`:
- Line 1264: Update the localization entry
"components.Settings.toastPlexSyncFailure" to include a server instance
placeholder (e.g. "Something went wrong while syncing Plex libraries for
{serverName}.") and then update the call sites that show this toast (the code
that invokes the i18n key) to pass the failing server instance name in the
interpolation object (e.g. t("components.Settings.toastPlexSyncFailure", {
serverName })). Ensure the placeholder syntax you use matches the project's i18n
interpolation format and update any tests/usage accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 960e2b48-f679-4b23-a810-801db80daa24
📒 Files selected for processing (20)
seerr-api.ymlserver/api/plextv.tsserver/lib/availabilitySync.tsserver/lib/scanners/jellyfin/index.tsserver/lib/settings/index.tsserver/lib/settings/migrations/0001_migrate_hostname.tsserver/lib/settings/migrations/0002_migrate_apitokens.tsserver/lib/settings/migrations/0005_migrate_network_settings.tsserver/lib/settings/migrations/0007_migrate_arr_tags.tsserver/lib/settings/migrations/0009_multi_media_servers.tsserver/routes/auth.tsserver/routes/avatarproxy.tsserver/routes/settings/index.tsserver/routes/user/index.tsserver/utils/findJellyfinUser.tssrc/components/Settings/SettingsLayout.tsxsrc/components/Settings/SettingsPlex.tsxsrc/components/UserList/PlexImportModal.tsxsrc/components/UserList/index.tsxsrc/i18n/locale/en.json
🚧 Files skipped from review as they are similar to previous changes (1)
- server/lib/settings/migrations/0009_multi_media_servers.ts
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
src/components/Settings/SettingsPlex.tsx (1)
167-169:⚠️ Potential issue | 🟠 MajorManual scan behavior is still global while shown in a server-scoped section.
The controls are rendered under a selected Plex server context, but they still call the unscoped
/api/v1/settings/plex/syncendpoint. In multi-server setups this remains misleading unless explicitly labeled global.Also applies to: 908-914, 966-979
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Settings/SettingsPlex.tsx` around lines 167 - 169, The manual-scan controls in SettingsPlex.tsx are fetching and mutating the global endpoint '/api/v1/settings/plex/sync' via the useSWR call (dataSync / revalidateSync) even though the UI is rendered per-selected Plex server; change the request target to the server-scoped endpoint (include the selected server id or key from props/state in the URL or query) for all occurrences (the useSWR at the shown location and the duplicate blocks around lines 908-914 and 966-979), and update corresponding mutate/revalidate calls to use the same server-scoped path so the controls reflect and update that specific server rather than the global setting (or alternatively, explicitly label the UI as global if you intend to keep the global endpoint).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@seerr-api.yml`:
- Around line 576-578: The OpenAPI schema exposes JellyfinSettings with property
serverID which mismatches the backend model's serverId; update the schema to use
serverId (replace serverID with serverId, preserving type: string and readOnly:
true) and sweep any references to JellyfinSettings/serverID to use serverId so
generated clients match the backend; if you need compatibility, add serverID as
a deprecated/nullable alias that maps to serverId until clients are migrated.
- Around line 368-373: PlexSettings schema currently lists machineId as required
but the backend model marks it optional; update the OpenAPI spec by removing
"machineId" from the required array for the PlexSettings component (ensure the
machineId property remains defined with the same type/nullability as the backend
model), then regenerate any client typings to reflect the optional field (verify
code paths using PlexSettings handle missing machineId).
In `@server/lib/availabilitySync.ts`:
- Around line 1011-1023: The helpers seasonExistsInPlex and
seasonExistsInJellyfin currently treat an empty cached seasons array as "not
found" and return false; change their logic to fail-open: when a candidate
server's cache entry exists but is an empty array (plexSeasonsCache[...] or
jellyfinSeasonsCache[...] is []), treat that as a successful fetch and return
true to avoid marking seasons deleted. Locate the loops in seasonExistsInPlex
and seasonExistsInJellyfin, detect cache entries that are defined with length
=== 0 and return true in that case, otherwise keep the existing check that
returns true when a matching season.index/seasonNumber is present, and only
return false after all candidates are exhausted.
In `@server/routes/settings/index.ts`:
- Around line 448-474: The GET handler is mutating library enabled flags and
saving settings even when req.query.enable is omitted; change the flow in the
route so that if req.query.enable is not present you simply return the current
libraries (from settings.plexServers.find(...).libraries) without calling
settings.updatePlexServer or settings.save, and only perform updatePlexServer +
save when enable is provided (parse enable into enabledLibraries and then update
via settings.updatePlexServer as currently written). Apply the same fix to the
parallel handler that covers lines around the other occurrence (the other plex
libraries GET handler noted at 731-758).
In `@src/components/Settings/SettingsPlex.tsx`:
- Around line 256-260: The computed flag hasAnyEnabledLibraries is derived from
plexServers (from the /api/v1/settings/plex/servers response) but your library
toggle/sync mutation handlers only call revalidate() for the selected server,
leaving plexServers stale and the Start Scan button state incorrect; update the
mutation success callbacks that currently call revalidate() (the handlers used
for toggling/syncing libraries) to also revalidate the overall plex servers
cache (e.g., call the same revalidate used for plexServers or a new
revalidatePlexServers) so plexServers is refreshed after library mutations—apply
this change to the places around the hasAnyEnabledLibraries usage and the
mutation callbacks referenced (the toggle/sync library mutation handlers at the
noted locations).
---
Duplicate comments:
In `@src/components/Settings/SettingsPlex.tsx`:
- Around line 167-169: The manual-scan controls in SettingsPlex.tsx are fetching
and mutating the global endpoint '/api/v1/settings/plex/sync' via the useSWR
call (dataSync / revalidateSync) even though the UI is rendered per-selected
Plex server; change the request target to the server-scoped endpoint (include
the selected server id or key from props/state in the URL or query) for all
occurrences (the useSWR at the shown location and the duplicate blocks around
lines 908-914 and 966-979), and update corresponding mutate/revalidate calls to
use the same server-scoped path so the controls reflect and update that specific
server rather than the global setting (or alternatively, explicitly label the UI
as global if you intend to keep the global endpoint).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0b28032a-21a6-4662-bc88-a5f1e274689d
📒 Files selected for processing (6)
seerr-api.ymlserver/index.tsserver/lib/availabilitySync.tsserver/routes/settings/index.tsserver/routes/user/usersettings.tssrc/components/Settings/SettingsPlex.tsx
Comment on lines
448
to
+474
| const enabledLibraries = req.query.enable | ||
| ? (req.query.enable as string).split(',') | ||
| : []; | ||
| settings.plex.libraries = settings.plex.libraries.map((library) => ({ | ||
| ...library, | ||
| enabled: enabledLibraries.includes(library.id), | ||
|
|
||
| if (!settings.plexServers.some((plexServer) => plexServer.id === server.id)) { | ||
| return res.status(404).json({ message: 'Plex server not found.' }); | ||
| } | ||
|
|
||
| const updated = settings.updatePlexServer(server.id, (plexServer) => ({ | ||
| ...plexServer, | ||
| libraries: plexServer.libraries.map((library) => ({ | ||
| ...library, | ||
| enabled: enabledLibraries.includes(library.id), | ||
| })), | ||
| })); | ||
|
|
||
| if (!updated) { | ||
| return res.status(404).json({ message: 'Plex server not found.' }); | ||
| } | ||
|
|
||
| await settings.save(); | ||
| return res.status(200).json(settings.plex.libraries); | ||
| return res | ||
| .status(200) | ||
| .json( | ||
| settings.plexServers.find((plexServer) => plexServer.id === server.id) | ||
| ?.libraries ?? [] | ||
| ); |
There was a problem hiding this comment.
Avoid mutating library enablement on read-only GET requests.
When enable is omitted, both handlers currently write enabled = false for every library and save settings. This makes a plain fetch destructive.
🐛 Proposed fix
- const enabledLibraries = req.query.enable
- ? (req.query.enable as string).split(',')
- : [];
+ const enabledLibraries =
+ typeof req.query.enable === 'string'
+ ? req.query.enable.split(',')
+ : undefined;
- const updated = settings.updatePlexServer(server.id, (plexServer) => ({
- ...plexServer,
- libraries: plexServer.libraries.map((library) => ({
- ...library,
- enabled: enabledLibraries.includes(library.id),
- })),
- }));
-
- if (!updated) {
- return res.status(404).json({ message: 'Plex server not found.' });
- }
-
- await settings.save();
+ if (enabledLibraries) {
+ const updated = settings.updatePlexServer(server.id, (plexServer) => ({
+ ...plexServer,
+ libraries: plexServer.libraries.map((library) => ({
+ ...library,
+ enabled: enabledLibraries.includes(library.id),
+ })),
+ }));
+
+ if (!updated) {
+ return res.status(404).json({ message: 'Plex server not found.' });
+ }
+
+ await settings.save();
+ }- const enabledLibraries = req.query.enable
- ? (req.query.enable as string).split(',')
- : [];
+ const enabledLibraries =
+ typeof req.query.enable === 'string'
+ ? req.query.enable.split(',')
+ : undefined;
- const updated = settings.updateJellyfinServer(
- server.id,
- (jellyfinServer) => ({
- ...jellyfinServer,
- libraries: jellyfinServer.libraries.map((library) => ({
- ...library,
- enabled: enabledLibraries.includes(library.id),
- })),
- })
- );
-
- if (!updated) {
- return res.status(404).json({ error: 'Jellyfin server not found.' });
- }
-
- await settings.save();
+ if (enabledLibraries) {
+ const updated = settings.updateJellyfinServer(
+ server.id,
+ (jellyfinServer) => ({
+ ...jellyfinServer,
+ libraries: jellyfinServer.libraries.map((library) => ({
+ ...library,
+ enabled: enabledLibraries.includes(library.id),
+ })),
+ })
+ );
+
+ if (!updated) {
+ return res.status(404).json({ error: 'Jellyfin server not found.' });
+ }
+
+ await settings.save();
+ }Also applies to: 731-758
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/routes/settings/index.ts` around lines 448 - 474, The GET handler is
mutating library enabled flags and saving settings even when req.query.enable is
omitted; change the flow in the route so that if req.query.enable is not present
you simply return the current libraries (from
settings.plexServers.find(...).libraries) without calling
settings.updatePlexServer or settings.save, and only perform updatePlexServer +
save when enable is provided (parse enable into enabledLibraries and then update
via settings.updatePlexServer as currently written). Apply the same fix to the
parallel handler that covers lines around the other occurrence (the other plex
libraries GET handler noted at 731-758).
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
seerr-api.yml (1)
2377-2384: StandardizeserverIdparameter schema across endpoints.
serverIdis defined slightly differently across routes. Consider extracting a shared parameter/component (type: string,format: uuid, consistent nullable/description) to keep generated clients and docs stable.Also applies to: 2431-2436, 2468-2473, 2567-2573, 2664-2669, 2788-2792, 4443-4445, 4478-4479, 5239-5242
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@seerr-api.yml` around lines 2377 - 2384, Create a single reusable parameter definition for serverId and replace the inline variants with a $ref to it: add a components.parameters.serverId entry with type: string, format: uuid, nullable: true and a standardized description, then update the endpoints that currently define serverId inline (e.g., the query parameter blocks shown between lines like 2377–2384 and the other ranges listed) to reference components.parameters.serverId instead of repeating the schema; ensure names remain "serverId" and that any generated clients use the shared definition.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/lib/availabilitySync.ts`:
- Around line 58-65: The getCandidatePlexServerIds method currently expands a
provided serverId into a set containing that id plus all configuredPlex server
IDs, which allows other servers to satisfy checks; change
getCandidatePlexServerIds so that when serverId (the recorded/stored server) is
provided it only returns that serverId (not merged with
getSettings().plexServers), and when no serverId is provided it returns the
configuredServerIds as before; update references to getCandidatePlexServerIds
accordingly to ensure existence checks only target the recorded server when
present.
- Around line 889-918: The 4K season check currently treats a successful episode
fetch that returns an empty array as "no 4K" and can clear plexMedia; update the
loop in availabilitySync.ts (inside the is4k branch that iterates plexSeasons
and calls plexClient.getChildrenMetadata) so that if episodes is an empty array
(episodes?.length === 0) you treat that as a fail-open success (i.e., consider
the season as present/unknown rather than definitively missing) — for example,
set has4kEpisode = true or push that season into seasonsWith4kEpisodes when
episodes is empty; keep the catch block to set fetchFailed only on true
network/errors and preserve the existing fallback that pushes all plexSeasons
into seasonsWith4kEpisodes when fetchFailed is true.
In `@src/components/Settings/SettingsPlex.tsx`:
- Around line 421-431: The request omits params.enable when disabling the last
enabled library so the backend sees only serverId and doesn't update flags;
update the logic around activeLibraries/params.enable in SettingsPlex (where
activeLibraries, libraryId, params, selectedServerId are used and the
axios.get('/api/v1/settings/plex/library', { params }) call is made) to always
set params.enable to the joined list (i.e., activeLibraries.filter(id => id !==
libraryId).join(',')) even if that produces an empty string so the backend
receives an explicit "disable all" instruction.
---
Nitpick comments:
In `@seerr-api.yml`:
- Around line 2377-2384: Create a single reusable parameter definition for
serverId and replace the inline variants with a $ref to it: add a
components.parameters.serverId entry with type: string, format: uuid, nullable:
true and a standardized description, then update the endpoints that currently
define serverId inline (e.g., the query parameter blocks shown between lines
like 2377–2384 and the other ranges listed) to reference
components.parameters.serverId instead of repeating the schema; ensure names
remain "serverId" and that any generated clients use the shared definition.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 296fc97a-8450-4bd8-952d-15c43b31f715
📒 Files selected for processing (4)
seerr-api.ymlserver/lib/availabilitySync.tsserver/routes/settings/index.tssrc/components/Settings/SettingsPlex.tsx
|
This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged. |
github-actions
Bot
removed
the
merge conflict
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
server/job/schedule.ts (1)
35-109:⚠️ Potential issue | 🟠 MajorDon't couple Plex account jobs to Plex library servers.
Lines 76-108 now sit under
hasPlexServers, so a setup with Plex auth still enabled but no Plex library servers will stop refreshing Plex tokens and syncing Plex watchlists altogether. Only the library scan jobs needsettings.plexServers.length > 0; the account jobs should follow the Plex auth/token capability instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/job/schedule.ts` around lines 35 - 109, The Plex account jobs ("plex-refresh-token" and "plex-watchlist-sync") are incorrectly nested under the hasPlexServers block so they don't run when a user has Plex auth but no library servers; move the scheduledJobs.push(...) blocks for id 'plex-refresh-token' and 'plex-watchlist-sync' out of the hasPlexServers conditional and instead guard them with a Plex-auth/token capability check (e.g., compute hasPlexAuth = Boolean(settings.plexAuthEnabled || settings.plexToken) or whatever existing flag represents Plex auth), leaving the scan jobs (ids 'plex-recently-added-scan' and 'plex-full-scan') still behind hasPlexServers; ensure the moved jobs still call refreshToken.run() and watchlistSync.syncWatchlist() and keep their error logging intact.server/routes/user/index.ts (1)
644-697:⚠️ Potential issue | 🟠 MajorKeep Plex imports scoped to the selected server before updating existing users.
Lines 683-695 only verify server access in the create path, and
checkUserAccess()falls back tosettings.plex.machineIdwhenplexServer.machineIdis missing. In a multi-server import that can still link/update an existing user against the wrong Plex server. Compute the access predicate once fromrawUser.Server(or reject servers without amachineId) and gate both branches on it; that also avoids the extra Plex.tv round-trip per candidate.Possible fix
const plexUsersResponse = await mainPlexTv.getUsers(); const createdUsers: User[] = []; + if (!plexServer.machineId) { + return next({ + status: 400, + message: 'Selected Plex server is missing its machine identifier.', + }); + } + for (const rawUser of plexUsersResponse.MediaContainer.User) { const account = rawUser.$; + const hasAccessToSelectedServer = rawUser.Server?.some( + (server) => server.$.machineIdentifier === plexServer.machineId + ); + + if (!hasAccessToSelectedServer) { + continue; + } if (account.email) { const user = await userRepository .createQueryBuilder('user') .where('user.plexId = :id', { id: account.id }) @@ user.plexId = parseInt(account.id); user.plexUsername = account.username; await userRepository.save(user); } else if (!body?.plexIds || body.plexIds.includes(account.id)) { - if ( - await mainPlexTv.checkUserAccess( - parseInt(account.id), - plexServer.machineId - ) - ) { - const newUser = new User({ - plexUsername: account.username, - email: account.email, - permissions: settings.main.defaultPermissions, - plexId: parseInt(account.id), - plexToken: '', - avatar: account.thumb, - userType: UserType.PLEX, - }); - await userRepository.save(newUser); - createdUsers.push(newUser); - } + const newUser = new User({ + plexUsername: account.username, + email: account.email, + permissions: settings.main.defaultPermissions, + plexId: parseInt(account.id), + plexToken: '', + avatar: account.thumb, + userType: UserType.PLEX, + }); + await userRepository.save(newUser); + createdUsers.push(newUser); } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/user/index.ts` around lines 644 - 697, The code updates existing users without verifying that the Plex account entry actually belongs to the selected plexServer because checkUserAccess() can fall back to a default machineId; to fix, compute a single access predicate per rawUser (using rawUser.Server and ensuring Server.machineIdentifier/machineId is present, or reject that rawUser) and reuse that predicate for both the "user exists" update path and the "create new user" path, rather than calling mainPlexTv.checkUserAccess() only in the create branch; update the loop over plexUsersResponse.MediaContainer.User to derive serverMachineId once, skip entries without machineId, call checkUserAccess(parseInt(account.id), serverMachineId) once and gate both the existing-user update (userRepository.save) and the new-user creation on that result so we don’t link/update users to the wrong Plex server or perform extra Plex.tv round-trips.server/routes/settings/index.ts (1)
486-502:⚠️ Potential issue | 🟠 MajorThread
serverIdthrough the Plex scan endpoints.These handlers still ignore the requested server and call the global scanner methods. With multiple Plex instances, status/start/cancel from one settings page can affect the shared scan instead of the targeted server.
Suggested fix
-settingsRoutes.get('/plex/sync', (_req, res) => { - return res.status(200).json(plexFullScanner.status()); +settingsRoutes.get('/plex/sync', (req, res) => { + const requestedServerId = req.query.serverId?.toString(); + const server = getPlexServerFromRequest({ + serverId: requestedServerId, + }); + + if (requestedServerId && !server) { + return res.status(404).json({ error: 'Plex server not found' }); + } + + return res.status(200).json(plexFullScanner.status(requestedServerId)); }); settingsRoutes.post('/plex/sync', (req, res) => { + const requestedServerId = req.query.serverId?.toString(); + if (req.user?.id !== 1) { return res.status(403).json({ message: 'Only the owner can run Plex scans from settings.', }); } + + const server = getPlexServerFromRequest({ + serverId: requestedServerId, + }); + + if (requestedServerId && !server) { + return res.status(404).json({ error: 'Plex server not found' }); + } if (req.body.cancel) { - plexFullScanner.cancel(); + plexFullScanner.cancel(requestedServerId); } else if (req.body.start) { - plexFullScanner.run(); + plexFullScanner.run(requestedServerId); } - return res.status(200).json(plexFullScanner.status()); + return res.status(200).json(plexFullScanner.status(requestedServerId)); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/settings/index.ts` around lines 486 - 502, The plex sync endpoints currently call the global plexFullScanner methods (plexFullScanner.status/run/cancel) and ignore which Plex server the request targets; update the GET and POST handlers for '/plex/sync' to read a server identifier from the request (e.g., req.query.serverId or req.body.serverId), resolve the corresponding scanner/instance for that server (rather than using the global plexFullScanner), validate the server exists and the caller is allowed (req.user?.id check stays), then invoke the per-server scanner's status(), run(), or cancel() methods and return that scanner.status() in the response; reference the existing handler functions for '/plex/sync' and the methods plexFullScanner.status, plexFullScanner.run, and plexFullScanner.cancel when locating the code to change.
♻️ Duplicate comments (3)
src/components/Settings/SettingsJellyfin.tsx (1)
349-358:⚠️ Potential issue | 🟠 MajorAlways send
enable, even when the new library set is empty.Same edge case as the Plex toggle path: when
libraryIdis the only enabled library, this branch sends onlyserverId, so the backend never receives an explicit “disable all” instruction. The last library stays impossible to turn off.Suggested fix
- const params: { enable?: string; serverId?: string } = { - serverId: selectedServerId, - }; - - if (activeLibraries.length > 1) { - params.enable = activeLibraries - .filter((id) => id !== libraryId) - .join(','); - } + const params: { enable: string; serverId: string } = { + enable: activeLibraries + .filter((id) => id !== libraryId) + .join(','), + serverId: selectedServerId, + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Settings/SettingsJellyfin.tsx` around lines 349 - 358, In SettingsJellyfin (the activeLibraries toggle branch) the params object omits params.enable when the resulting activeLibraries list is empty, so the backend never receives an explicit "disable all"; always set params.enable (use an empty string when no libraries remain) instead of conditionally adding it—update the logic around activeLibraries, params, libraryId and selectedServerId to always include params.enable = activeLibraries.filter(id => id !== libraryId).join(',') (which may be an empty string) before sending the request.server/lib/scanners/plex/index.ts (1)
88-172:⚠️ Potential issue | 🟠 MajorOne failing Plex server still aborts the rest of the multi-server scan.
The loop now walks multiple servers, but errors from
hasHamaAgent(),getRecentlyAdded(),paginateLibrary(), orsettings.save()still bubble to the outercatch, which stops every remaining server. The Jellyfin scanner in this PR already isolates each server iteration; Plex needs the same guard or one unhealthy instance defeats the feature.Suggested fix
for (const server of plexServers) { - this.currentServer = server; - this.plexClient = new PlexAPI({ - plexToken: admin.plexToken, - plexSettings: server, - plexServerId: server.id, - }); - - const serverLibraries = server.libraries.filter( - (library) => library.enabled - ); - - const hasHama = await this.hasHamaAgent(serverLibraries); - if (hasHama) { - await animeList.sync(); - } - - if (this.isRecentOnly) { - // existing per-library processing... - } else { - // existing per-library processing... - } + try { + this.currentServer = server; + this.plexClient = new PlexAPI({ + plexToken: admin.plexToken, + plexSettings: server, + plexServerId: server.id, + }); + + const serverLibraries = server.libraries.filter( + (library) => library.enabled + ); + + const hasHama = await this.hasHamaAgent(serverLibraries); + if (hasHama) { + await animeList.sync(); + } + + if (this.isRecentOnly) { + // existing per-library processing... + } else { + // existing per-library processing... + } + } catch (err) { + this.log( + `Error scanning server ${server.name || server.id}: ${ + err instanceof Error ? err.message : String(err) + }`, + 'error', + { serverId: server.id } + ); + continue; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/lib/scanners/plex/index.ts` around lines 88 - 172, Wrap each plex server iteration in its own try/catch and handle/log errors so one failing server doesn't abort the outer scan: enclose the body of the for (const server of plexServers) loop in try { ... } catch (err) { this.log(`Plex server scan failed`, 'error', { serverId: server.id, error: err }); continue; } and additionally guard per-library async calls (hasHamaAgent, this.plexClient.getRecentlyAdded, this.paginateLibrary, settings.save) with small try/catches where appropriate so a failure in one library won’t stop other libraries on the same server; use the unique symbols hasHamaAgent, getRecentlyAdded, paginateLibrary, settings.save, this.loop/processItem to locate places to add the guards and logging.server/routes/settings/index.ts (1)
284-288:⚠️ Potential issue | 🟡 MinorDon't fall back to
settings.plexafter a scoped save.
plexServerIdis set before the save, so a miss here means the requested server was not persisted or was concurrently removed. Returning the primary Plex config hides that failure and can make the client render the wrong server.Suggested fix
- return res - .status(200) - .json( - getPlexServerFromRequest({ serverId: plexServerId }) ?? settings.plex - ); + const requestedServer = plexServerId + ? getPlexServerFromRequest({ serverId: plexServerId }) + : undefined; + + if (plexServerId && !requestedServer) { + return res.status(404).json({ error: 'Plex server not found' }); + } + + return res.status(200).json(requestedServer ?? settings.plex);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/settings/index.ts` around lines 284 - 288, The current response returns getPlexServerFromRequest({ serverId: plexServerId }) ?? settings.plex, which can hide a failed or concurrent deletion because plexServerId was set before the scoped save; remove the fallback to settings.plex and instead return an explicit not-found or error when getPlexServerFromRequest(...) is null (e.g., respond 404 or a clear error body). Update the response logic in the handler that uses plexServerId / getPlexServerFromRequest to only return the found server and not fall back to settings.plex.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@seerr-api.yml`:
- Around line 245-256: Update the OpenAPI spec so GET /settings/public returns
the runtime fields the client expects: add mediaServerLogin, plexLogin,
jellyfinLogin, embyLogin booleans and the mediaServerTypes and mediaServers
properties to the PublicSettings schema; ensure the new properties' types match
existing definitions (e.g., mediaServerTypes as an array of strings or the
existing enum type and mediaServers as the same object/array shape used
elsewhere) and reference the same models if they already exist so the published
contract aligns with the bootstrap payload the client uses.
In `@server/entity/Media.ts`:
- Around line 253-304: The helpers buildPlexUrls and buildJellyfinUrls currently
fall back to the primary server even when a specific serverId was provided but
not found; change them so that if a serverId was supplied and the find(...)
returns no match, the function returns null immediately, and only when serverId
is not supplied should you fall back to
settings.getPrimaryPlexServer()/settings.getPrimaryJellyfinLikeServer(); keep
the existing machineId/serverId existence checks (server?.machineId /
server?.serverId) after this change so unresolved explicit IDs yield null and
legacy rows with no stored serverId still use the primary.
In `@server/lib/settings/index.ts`:
- Around line 998-1023: The call to synchronizeMediaServerSettings() can
generate stable IDs for plexServers/jellyfinServers but its changes aren't
persisted because the local `change` flag only tracks API/client/VAPID updates;
update finalizeLoadedSettings (or synchronizeMediaServerSettings) so that
synchronizeMediaServerSettings reports whether it modified settings (e.g.,
return a boolean or set a property) and then set `change = true` when it
indicates modifications so that await this.save() runs and the generated server
IDs are persisted; refer to finalizeLoadedSettings,
synchronizeMediaServerSettings, the `change` local variable, and this.save()
when making this change.
In `@server/routes/settings/index.ts`:
- Around line 574-595: The block that logs into Jellyfin and persists only
jellyfinServer.apiKey must also store the authenticated admin user id for that
specific server to avoid falling back to the global jellyfinUserId; after the
admin check and before/after calling createApiToken('Seerr'), set the
server-scoped user id from account.User.Id on the jellyfinServer object (create
the jellyfinServer.jellyfinUserId property if it doesn't exist) so later
server-scoped syncs use this per-server id instead of the global one.
In `@src/components/Login/JellyfinLogin.tsx`:
- Around line 144-165: The server selector is missing a real label for
accessibility; replace the standalone div containing
intl.formatMessage(messages.server) with a proper label element (or add a label
tied to the select) using htmlFor="serverId" so the <select id="serverId"
name="serverId" ...> is associated with it; update the component around
jellyfinServers, selectedServer and setSelectedServerId to render that label
(preserving the "label-tip" styling/class) or add aria-labelledby pointing to
the label ID if you prefer.
- Around line 57-64: The current logic around
selectedServerId/selectedServer/resolvedServerType can drop the provider type
when no server instance is selected; ensure the Jellyfin auth request still
includes the provider type by preserving serverType as a fallback whenever
selectedServer is undefined. Update the selection/fallback so resolvedServerType
is computed from selectedServer?.mediaServerType OR the component prop
serverType (or MediaServerType.JELLYFIN) and make sure the auth request payload
(where selectedServerId and server info are used) always sends body.serverType =
resolvedServerType when selectedServer is falsy; check functions/components
referencing selectedServerId, selectedServer, and resolvedServerType and adjust
them to use resolvedServerType as the canonical provider value in the API call.
In `@src/components/Settings/SettingsPlex.tsx`:
- Around line 462-465: The Tautulli Formik instance is mounting with undefined
initialValues because dataTautulli is now loaded after render; update the
Formik(s) that use dataTautulli (and the analogous Formik at the other
occurrence) to include enableReinitialize={true} so the form re-hydrates when
SWR data arrives. Locate the Formik components in SettingsPlex (the instances
that reference initialValues derived from dataTautulli) and add the
enableReinitialize prop to them to ensure saved config isn't overwritten by
uninitialized state.
---
Outside diff comments:
In `@server/job/schedule.ts`:
- Around line 35-109: The Plex account jobs ("plex-refresh-token" and
"plex-watchlist-sync") are incorrectly nested under the hasPlexServers block so
they don't run when a user has Plex auth but no library servers; move the
scheduledJobs.push(...) blocks for id 'plex-refresh-token' and
'plex-watchlist-sync' out of the hasPlexServers conditional and instead guard
them with a Plex-auth/token capability check (e.g., compute hasPlexAuth =
Boolean(settings.plexAuthEnabled || settings.plexToken) or whatever existing
flag represents Plex auth), leaving the scan jobs (ids
'plex-recently-added-scan' and 'plex-full-scan') still behind hasPlexServers;
ensure the moved jobs still call refreshToken.run() and
watchlistSync.syncWatchlist() and keep their error logging intact.
In `@server/routes/settings/index.ts`:
- Around line 486-502: The plex sync endpoints currently call the global
plexFullScanner methods (plexFullScanner.status/run/cancel) and ignore which
Plex server the request targets; update the GET and POST handlers for
'/plex/sync' to read a server identifier from the request (e.g.,
req.query.serverId or req.body.serverId), resolve the corresponding
scanner/instance for that server (rather than using the global plexFullScanner),
validate the server exists and the caller is allowed (req.user?.id check stays),
then invoke the per-server scanner's status(), run(), or cancel() methods and
return that scanner.status() in the response; reference the existing handler
functions for '/plex/sync' and the methods plexFullScanner.status,
plexFullScanner.run, and plexFullScanner.cancel when locating the code to
change.
In `@server/routes/user/index.ts`:
- Around line 644-697: The code updates existing users without verifying that
the Plex account entry actually belongs to the selected plexServer because
checkUserAccess() can fall back to a default machineId; to fix, compute a single
access predicate per rawUser (using rawUser.Server and ensuring
Server.machineIdentifier/machineId is present, or reject that rawUser) and reuse
that predicate for both the "user exists" update path and the "create new user"
path, rather than calling mainPlexTv.checkUserAccess() only in the create
branch; update the loop over plexUsersResponse.MediaContainer.User to derive
serverMachineId once, skip entries without machineId, call
checkUserAccess(parseInt(account.id), serverMachineId) once and gate both the
existing-user update (userRepository.save) and the new-user creation on that
result so we don’t link/update users to the wrong Plex server or perform extra
Plex.tv round-trips.
---
Duplicate comments:
In `@server/lib/scanners/plex/index.ts`:
- Around line 88-172: Wrap each plex server iteration in its own try/catch and
handle/log errors so one failing server doesn't abort the outer scan: enclose
the body of the for (const server of plexServers) loop in try { ... } catch
(err) { this.log(`Plex server scan failed`, 'error', { serverId: server.id,
error: err }); continue; } and additionally guard per-library async calls
(hasHamaAgent, this.plexClient.getRecentlyAdded, this.paginateLibrary,
settings.save) with small try/catches where appropriate so a failure in one
library won’t stop other libraries on the same server; use the unique symbols
hasHamaAgent, getRecentlyAdded, paginateLibrary, settings.save,
this.loop/processItem to locate places to add the guards and logging.
In `@server/routes/settings/index.ts`:
- Around line 284-288: The current response returns getPlexServerFromRequest({
serverId: plexServerId }) ?? settings.plex, which can hide a failed or
concurrent deletion because plexServerId was set before the scoped save; remove
the fallback to settings.plex and instead return an explicit not-found or error
when getPlexServerFromRequest(...) is null (e.g., respond 404 or a clear error
body). Update the response logic in the handler that uses plexServerId /
getPlexServerFromRequest to only return the found server and not fall back to
settings.plex.
In `@src/components/Settings/SettingsJellyfin.tsx`:
- Around line 349-358: In SettingsJellyfin (the activeLibraries toggle branch)
the params object omits params.enable when the resulting activeLibraries list is
empty, so the backend never receives an explicit "disable all"; always set
params.enable (use an empty string when no libraries remain) instead of
conditionally adding it—update the logic around activeLibraries, params,
libraryId and selectedServerId to always include params.enable =
activeLibraries.filter(id => id !== libraryId).join(',') (which may be an empty
string) before sending the request.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 59e179c2-7d22-4be6-b336-de8d75f76a56
📒 Files selected for processing (27)
seerr-api.ymlserver/api/jellyfin.tsserver/api/plextv.tsserver/api/rating/imdbRadarrProxy.tsserver/entity/Media.tsserver/job/schedule.tsserver/lib/availabilitySync.tsserver/lib/scanners/jellyfin/index.tsserver/lib/scanners/plex/index.tsserver/lib/settings/index.tsserver/routes/avatarproxy.tsserver/routes/discover.tsserver/routes/settings/index.tsserver/routes/user/index.tssrc/components/Login/JellyfinLogin.tsxsrc/components/MovieDetails/index.tsxsrc/components/Settings/SettingsJellyfin.tsxsrc/components/Settings/SettingsPlex.tsxsrc/components/Settings/SettingsUsers/index.tsxsrc/components/Setup/index.tsxsrc/components/TitleCard/index.tsxsrc/components/TvDetails/index.tsxsrc/components/UserList/JellyfinImportModal.tsxsrc/components/UserList/PlexImportModal.tsxsrc/components/UserList/index.tsxsrc/i18n/locale/en.jsonsrc/pages/_app.tsx
🚧 Files skipped from review as they are similar to previous changes (8)
- src/components/UserList/PlexImportModal.tsx
- server/api/plextv.ts
- src/components/Settings/SettingsUsers/index.tsx
- server/routes/avatarproxy.ts
- server/api/jellyfin.ts
- server/lib/availabilitySync.ts
- src/components/UserList/JellyfinImportModal.tsx
- src/i18n/locale/en.json
Comment on lines
+462
to
+465
| if ( | ||
| (!plexAuthStatus && !plexAuthStatusError) || | ||
| (canManagePlexSettings && selectedServerId !== 'new' && !data && !error) | ||
| ) { |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Formik 2.x, does a form re-read async initialValuesafter mount whenenableReinitialize is not set?
💡 Result:
No, in Formik 2.x, a form does not re-read async initialValues after mount when enableReinitialize is not set (default is false). The initialValues are only used to set the form state on initial mount. Subsequent changes to initialValues, such as from an async resolution (e.g., state update after API call), do not update the form's values unless enableReinitialize is explicitly set to true, which triggers a reset to the new initialValues using deep equality comparison.
Citations:
- 1: https://formik.org/docs/api/formik
- 2: Add enableReinitialize option and set it to false by default jaredpalmer/formik#168
- 3: https://stackoverflow.com/questions/66372097/updating-one-of-the-formik-initial-values-with-state-resets-all-other-values
- 4: Formik does not re-render when initialValues change jaredpalmer/formik#811
- 5: Recommended way to set initialValues from async api call jaredpalmer/formik#1033
🏁 Script executed:
# First, let's look at the file structure and the specific lines mentioned
wc -l src/components/Settings/SettingsPlex.tsxRepository: seerr-team/seerr
Length of output: 105
🏁 Script executed:
# Read the lines around 462-465 and 994-1003 to understand the context
sed -n '450,475p' src/components/Settings/SettingsPlex.tsxRepository: seerr-team/seerr
Length of output: 649
🏁 Script executed:
# Also check lines 994-1003
sed -n '985,1010p' src/components/Settings/SettingsPlex.tsxRepository: seerr-team/seerr
Length of output: 1257
🏁 Script executed:
# Search for dataTautulli definition to understand its source (SWR or state)
rg -n "dataTautulli\s*=" src/components/Settings/SettingsPlex.tsx | head -20Repository: seerr-team/seerr
Length of output: 42
🏁 Script executed:
# Search for SWR hooks to understand the data loading pattern
rg -n "useSWR|useQuery" src/components/Settings/SettingsPlex.tsx | head -20Repository: seerr-team/seerr
Length of output: 418
🏁 Script executed:
# Look at the entire Formik initialization section to understand context
sed -n '980,1030p' src/components/Settings/SettingsPlex.tsxRepository: seerr-team/seerr
Length of output: 2130
🏁 Script executed:
# Let me look at the broader structure - where is the Tautulli form positioned relative to the render condition?
sed -n '460,520p' src/components/Settings/SettingsPlex.tsxRepository: seerr-team/seerr
Length of output: 1715
🏁 Script executed:
# And check the component structure more broadly
sed -n '480,1010p' src/components/Settings/SettingsPlex.tsx | head -100Repository: seerr-team/seerr
Length of output: 3251
🏁 Script executed:
# Let me check if there's a conditional around the Tautulli form itself
sed -n '975,1000p' src/components/Settings/SettingsPlex.tsxRepository: seerr-team/seerr
Length of output: 1104
🏁 Script executed:
# Search for all conditionals that might affect Tautulli rendering
rg -n "dataTautulli|Tautulli" src/components/Settings/SettingsPlex.tsx | head -30Repository: seerr-team/seerr
Length of output: 1392
Add enableReinitialize to the Tautulli form.
dataTautulli is no longer checked in the render-blocking condition, so this Formik mounts with undefined initialValues. Without enableReinitialize, the form never hydrates when SWR resolves, and saving from that state overwrites the stored config.
Suggested fix
<Formik
+ enableReinitialize
initialValues={{
tautulliHostname: dataTautulli?.hostname,
tautulliPort: dataTautulli?.port ?? 8181,
tautulliUseSsl: dataTautulli?.useSsl,Also applies to: 994-1003
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/Settings/SettingsPlex.tsx` around lines 462 - 465, The
Tautulli Formik instance is mounting with undefined initialValues because
dataTautulli is now loaded after render; update the Formik(s) that use
dataTautulli (and the analogous Formik at the other occurrence) to include
enableReinitialize={true} so the form re-hydrates when SWR data arrives. Locate
the Formik components in SettingsPlex (the instances that reference
initialValues derived from dataTautulli) and add the enableReinitialize prop to
them to ensure saved config isn't overwritten by uninitialized state.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
server/routes/settings/index.ts (1)
486-502:⚠️ Potential issue | 🟠 MajorScope Plex manual scans by
serverIdtoo.This route still ignores the requested server, so starting or canceling a scan from one Plex server page operates on the global scanner, and the returned status can belong to a different instance.
Suggested fix
-settingsRoutes.get('/plex/sync', (_req, res) => { - return res.status(200).json(plexFullScanner.status()); +settingsRoutes.get('/plex/sync', (req, res, next) => { + const requestedServerId = req.query.serverId?.toString(); + const server = getPlexServerFromRequest({ serverId: requestedServerId }); + + if (requestedServerId && !server) { + return next({ status: 404, message: 'Plex server not found.' }); + } + + return res.status(200).json(plexFullScanner.status(requestedServerId)); }); -settingsRoutes.post('/plex/sync', (req, res) => { +settingsRoutes.post('/plex/sync', (req, res, next) => { + const requestedServerId = req.query.serverId?.toString(); + const server = getPlexServerFromRequest({ serverId: requestedServerId }); + + if (requestedServerId && !server) { + return next({ status: 404, message: 'Plex server not found.' }); + } + if (req.user?.id !== 1) { return res.status(403).json({ message: 'Only the owner can run Plex scans from settings.', }); } if (req.body.cancel) { - plexFullScanner.cancel(); + plexFullScanner.cancel(requestedServerId); } else if (req.body.start) { - plexFullScanner.run(); + plexFullScanner.run(requestedServerId); } - return res.status(200).json(plexFullScanner.status()); + return res.status(200).json(plexFullScanner.status(requestedServerId)); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/settings/index.ts` around lines 486 - 502, The Plex sync endpoints currently operate on a global plexFullScanner; modify both handlers (settingsRoutes.get('/plex/sync') and settingsRoutes.post('/plex/sync')) to scope operations by the requested serverId: read serverId from the request (e.g., req.query.serverId for GET, req.body.serverId for POST), locate the scanner instance for that server (use or add a method on plexFullScanner such as getScanner(serverId) or forServer(serverId)), then call .status(), .run(), or .cancel() on that per-server scanner instead of the global one and return that instance's status; preserve the existing owner check (req.user?.id !== 1) and handle missing/invalid serverId by returning a 400 with a clear message.src/components/Settings/SettingsJellyfin.tsx (1)
349-362:⚠️ Potential issue | 🟠 MajorToggling off the last enabled library is now a no-op.
When
activeLibraries.length === 1, this branch omitsenableentirely. The backend now treats a missingenableas a read-only request, so the call returns current libraries and leaves the last one enabled.Suggested fix
if (activeLibraries.includes(libraryId)) { - const params: { enable?: string; serverId?: string } = { + const params: { enable: string; serverId?: string } = { serverId: selectedServerId, + enable: activeLibraries + .filter((id) => id !== libraryId) + .join(','), }; - - if (activeLibraries.length > 1) { - params.enable = activeLibraries - .filter((id) => id !== libraryId) - .join(','); - } await axios.get('/api/v1/settings/jellyfin/library', { params, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Settings/SettingsJellyfin.tsx` around lines 349 - 362, The branch that handles disabling a library omits the enable param when activeLibraries.length === 1, causing the backend to treat the request as read-only; update the block around params (the variables activeLibraries, libraryId, params and the axios.get('/api/v1/settings/jellyfin/library') call) so that when toggling off the last library you still send an explicit enable parameter (e.g., an empty string or empty list) instead of omitting it, ensuring the backend receives a write intent to clear enabled libraries while still including serverId.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/lib/availabilitySync.ts`:
- Around line 891-918: The current loop over plexSeasons treats any per-season
getChildrenMetadata() error by setting fetchFailed and skipping that season,
which can cause seasonExistsInPlex() to treat the missing cache entry as
deleted; change behavior to "fail open" per season by treating a fetch error for
a given season the same as an empty episode list (i.e., assume the season has 4K
episodes rather than omitting it) so transient errors don't clear 4K
availability — modify the logic around getChildrenMetadata, has4kEpisode,
seasonsWith4kEpisodes and fetchFailed (and adjust how plexSeasonsCache is
populated) so that on per-season fetch error you add the season to
seasonsWith4kEpisodes (or otherwise preserve it in plexSeasonsCache) instead of
silently skipping it; ensure seasonExistsInPlex()’s expectation that an empty
episodes array means “exists” is preserved.
- Around line 1066-1091: The code currently only sets existsInJellyfin after
getSeasons() succeeds; change the flow so that as soon as
jellyfinClient.getItemData(ratingKey) returns successfully you set
existsInJellyfin = true (before calling jellyfinClient.getSeasons), and keep
that true even if getSeasons() throws; if getSeasons() fails, set
preventSeasonSearch = true and log the error but do not clear existsInJellyfin
or allow seasonsMap to be treated as a definitive "not present" result; adjust
logic around jellyfinSeasonsCache and error handling in the try/catch that
references getItemData, getSeasons, existsInJellyfin, and preventSeasonSearch
accordingly.
In `@server/routes/settings/index.ts`:
- Around line 236-253: When req.body.id is provided but
getPlexServerFromRequest(...) returns undefined, do not fall back to a create
path; instead surface a 404/NotFound error and abort the save. Update the save
handlers that build PlexServerSettings (and the analogous Jellyfin branch) to
check the presence of existingServer after calling getPlexServerFromRequest and,
if missing, return or throw a 404 before calling
upsertPlexServer/upsertJellyfinServer; do not allow upsert to silently insert an
unknown id. Ensure the check references getPlexServerFromRequest,
PlexServerSettings construction, and the upsertPlexServer/upsertJellyfinServer
calls so the code path is correctly guarded.
In `@src/components/Settings/SettingsJellyfin.tsx`:
- Around line 242-258: The effect that auto-selects the first saved Jellyfin
server is blocked by the guard `selectedServerId !== 'new'`, causing the UI to
stick on the "Add New Server" form; remove that guard from the useEffect
condition so the effect can set the first server on initial load (keep the other
guards: `!hasInitializedServerSelection`, `isSetupSettings`,
`jellyfinServers?.length && jellyfinServers?.[0]`), then let the existing
onChange logic (which already calls `setHasInitializedServerSelection(true)`)
continue to prevent overwrites after a user explicitly chooses "Add New Server";
update references: useEffect, hasInitializedServerSelection, isSetupSettings,
selectedServerId, jellyfinServers, setSelectedServerId, setNewServerType,
setHasInitializedServerSelection, onChange.
---
Outside diff comments:
In `@server/routes/settings/index.ts`:
- Around line 486-502: The Plex sync endpoints currently operate on a global
plexFullScanner; modify both handlers (settingsRoutes.get('/plex/sync') and
settingsRoutes.post('/plex/sync')) to scope operations by the requested
serverId: read serverId from the request (e.g., req.query.serverId for GET,
req.body.serverId for POST), locate the scanner instance for that server (use or
add a method on plexFullScanner such as getScanner(serverId) or
forServer(serverId)), then call .status(), .run(), or .cancel() on that
per-server scanner instead of the global one and return that instance's status;
preserve the existing owner check (req.user?.id !== 1) and handle
missing/invalid serverId by returning a 400 with a clear message.
In `@src/components/Settings/SettingsJellyfin.tsx`:
- Around line 349-362: The branch that handles disabling a library omits the
enable param when activeLibraries.length === 1, causing the backend to treat the
request as read-only; update the block around params (the variables
activeLibraries, libraryId, params and the
axios.get('/api/v1/settings/jellyfin/library') call) so that when toggling off
the last library you still send an explicit enable parameter (e.g., an empty
string or empty list) instead of omitting it, ensuring the backend receives a
write intent to clear enabled libraries while still including serverId.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c33aa76e-898a-4ce4-9e80-cd74ec1ce3fb
📒 Files selected for processing (6)
server/lib/availabilitySync.tsserver/lib/scanners/jellyfin/index.tsserver/routes/settings/index.tssrc/components/Settings/SettingsJellyfin.tsxsrc/components/Settings/SettingsPlex.tsxsrc/i18n/locale/en.json
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
server/routes/user/index.ts (1)
683-695:⚠️ Potential issue | 🟠 MajorKeep the server filter on the existing-user path too.
getUsers()returns the full Plex shared list, but this branch links any email match before applyingplexIdsorcheckUserAccess(...). In a multi-server Plex account, importing from server A can silently attach a local user who only belongs to server B.Proposed fix
if (account.email) { const user = await userRepository .createQueryBuilder('user') .where('user.plexId = :id', { id: account.id }) .orWhere('user.email = :email', { email: account.email.toLowerCase(), }) .getOne(); + const isSelected = + !body?.plexIds || body.plexIds.includes(account.id); + const hasServerAccess = + isSelected && + (await mainPlexTv.checkUserAccess( + parseInt(account.id), + plexServer.machineId + )); - if (user) { + if (user && hasServerAccess) { // Update the user's avatar with their Plex thumbnail, in case it changed user.avatar = account.thumb; user.email = account.email; user.plexId = parseInt(account.id); user.plexUsername = account.username; await userRepository.save(user); - } else if (!body?.plexIds || body.plexIds.includes(account.id)) { - if ( - await mainPlexTv.checkUserAccess( - parseInt(account.id), - plexServer.machineId - ) - ) { + } else if (hasServerAccess) { const newUser = new User({ plexUsername: account.username, email: account.email, permissions: settings.main.defaultPermissions, plexId: parseInt(account.id), plexToken: '', avatar: account.thumb, userType: UserType.PLEX, }); await userRepository.save(newUser); createdUsers.push(newUser); - } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/user/index.ts` around lines 683 - 695, The current existing-user branch updates and links a user by email without applying the same server filtering used in the import branch; ensure the same guards are applied before modifying an existing user. Concretely, in the block handling "if (user) { ... }" add the same condition used in the else-if branch (check that !body?.plexIds || body.plexIds.includes(account.id)) and that await mainPlexTv.checkUserAccess(parseInt(account.id), plexServer.machineId) returns true before assigning user.avatar/email/plexId/plexUsername and calling userRepository.save(user).server/api/jellyfin.ts (1)
130-161:⚠️ Potential issue | 🟡 MinorRemove the 3-arg JellyfinAPI constructor call in the migration script.
The migration in
server/lib/settings/migrations/0002_migrate_apitokens.ts:24instantiates JellyfinAPI with only 3 arguments, causing it to fall back tosettings.main.mediaServerTypeinstead of deriving the server type from the actual Jellyfin host. WhilecreateApiToken()does not depend onmediaServerType, this creates a maintenance risk: if future changes add calls to methods likegetRecentlyAdded()(which does condition its endpoint onmediaServerType), the wrong route will be used in a mixed-provider install.Pass the fourth argument: either derive it from the Jellyfin settings object, or refactor the migration to accept the server type as a parameter.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/api/jellyfin.ts` around lines 130 - 161, The migration instantiates JellyfinAPI with only three arguments which causes the constructor (JellyfinAPI) to default mediaServerType to settings.main.mediaServerType; update the migration (the createApiToken call in the 0002_migrate_apitokens migration) to pass the fourth parameter so the correct server type is used—derive it from the Jellyfin settings object (e.g., jellyfinSettings.mediaServerType or similar property) and pass that as the mediaServerType argument to the JellyfinAPI constructor to avoid using the global default and ensure methods like getRecentlyAdded resolve routes correctly.server/lib/availabilitySync.ts (1)
535-554:⚠️ Potential issue | 🟠 MajorMixed-provider deletions need to clear both source families.
run()only reachesmediaUpdater()after the title has failed both Plex and Jellyfin checks, but this branch still nulls either the Plex fields or the Jellyfin fields based on a singlemediaServerType. On rows that carry both families, the untouched IDs survive on a deleted record and keep stale source attribution/play links around.🧹 Safer cleanup
- if (mediaServerType === MediaServerType.PLEX) { - media[is4k ? 'ratingKey4k' : 'ratingKey'] = isMediaProcessing - ? media[is4k ? 'ratingKey4k' : 'ratingKey'] - : null; - media[is4k ? 'plexServerId4k' : 'plexServerId'] = isMediaProcessing - ? media[is4k ? 'plexServerId4k' : 'plexServerId'] - : null; - } else if ( - mediaServerType === MediaServerType.JELLYFIN || - mediaServerType === MediaServerType.EMBY - ) { - media[is4k ? 'jellyfinMediaId4k' : 'jellyfinMediaId'] = - isMediaProcessing - ? media[is4k ? 'jellyfinMediaId4k' : 'jellyfinMediaId'] - : null; - media[is4k ? 'jellyfinServerId4k' : 'jellyfinServerId'] = - isMediaProcessing - ? media[is4k ? 'jellyfinServerId4k' : 'jellyfinServerId'] - : null; - } + if (!isMediaProcessing) { + media[is4k ? 'ratingKey4k' : 'ratingKey'] = null; + media[is4k ? 'plexServerId4k' : 'plexServerId'] = null; + media[is4k ? 'jellyfinMediaId4k' : 'jellyfinMediaId'] = null; + media[is4k ? 'jellyfinServerId4k' : 'jellyfinServerId'] = null; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/lib/availabilitySync.ts` around lines 535 - 554, The current branch in mediaUpdater uses mediaServerType to decide which provider fields to null, leaving IDs from the other provider intact; change it so when isMediaProcessing is false you clear both Plex and Jellyfin/Emby fields regardless of mediaServerType. Specifically, in availabilitySync.ts inside mediaUpdater (symbols: mediaServerType, MediaServerType.PLEX, MediaServerType.JELLYFIN, MediaServerType.EMBY, is4k, isMediaProcessing), ensure you set media[is4k ? 'ratingKey4k' : 'ratingKey'] and media[is4k ? 'plexServerId4k' : 'plexServerId'] to null when not isMediaProcessing, and also set media[is4k ? 'jellyfinMediaId4k' : 'jellyfinMediaId'] and media[is4k ? 'jellyfinServerId4k' : 'jellyfinServerId'] to null when not isMediaProcessing (instead of choosing one branch by mediaServerType).seerr-api.yml (1)
553-587:⚠️ Potential issue | 🟠 MajorAlign
JellyfinSettingsOpenAPI schema with backend model for multi-server endpoints.The OpenAPI
JellyfinSettingsschema defineshostname,adminUser, andadminPass, but the backendJellyfinSettingsinterface (server/lib/settings/index.ts:51–62) usesip,port,useSsl, andurlBaseinstead. SinceJellyfinServerInstancereuses this base schema and theGET /jellyfin/serversendpoint returnsJellyfinServerSettingsinstances directly, clients generated from this spec will expect fields that don't exist in the actual response, breaking the new multi-server flows.Update the
JellyfinSettingsschema to match the backend model fields.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@seerr-api.yml` around lines 553 - 587, The OpenAPI JellyfinSettings schema is out of sync with the backend model used by JellyfinServerInstance and GET /jellyfin/servers; replace the current schema properties (hostname, adminUser, adminPass) with the backend fields ip, port, useSsl, and urlBase to match server/lib/settings/index.ts's JellyfinSettings interface, ensure required/readonly flags mirror the backend (e.g., id, name, libraries, serverId remain), and update any references to JellyfinSettings (such as JellyfinServerInstance) so generated clients expect the same fields as the API responses.
♻️ Duplicate comments (1)
src/components/Settings/SettingsJellyfin.tsx (1)
242-258:⚠️ Potential issue | 🟠 MajorThe initial Jellyfin server auto-select never runs in normal settings.
selectedServerIdstarts as'new', so this predicate is false on the first render outside setup. That leaves upgraded installs on the blank “Add New Server” form instead of loading the saved Jellyfin/Emby server, which makes the existing config look missing and makes accidental duplicates much more likely.Suggested fix
useEffect(() => { if ( !hasInitializedServerSelection && - (isSetupSettings || selectedServerId !== 'new') && - jellyfinServers?.length && + selectedServerId === 'new' && + jellyfinServers?.length === 1 && jellyfinServers?.[0] ) { setSelectedServerId(jellyfinServers[0].id); setNewServerType(jellyfinServers[0].mediaServerType); setHasInitializedServerSelection(true); } }, [ hasInitializedServerSelection, - isSetupSettings, jellyfinServers, selectedServerId, ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Settings/SettingsJellyfin.tsx` around lines 242 - 258, The auto-select useEffect doesn't run because the predicate uses selectedServerId !== 'new' (so on first render when selectedServerId === 'new' it skips); update the conditional in the effect that reads hasInitializedServerSelection/isSetupSettings/selectedServerId/jellyfinServers to allow auto-selection when selectedServerId is the default 'new' (e.g., change the (isSetupSettings || selectedServerId !== 'new') check to (isSetupSettings || selectedServerId === 'new')), then keep the body that calls setSelectedServerId(jellyfinServers[0].id), setNewServerType(jellyfinServers[0].mediaServerType) and setHasInitializedServerSelection(true) so upgraded installs load the saved Jellyfin/Emby server.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@seerr-api.yml`:
- Around line 2609-2630: Add the missing non-204 response docs to the POST
"Authenticate Plex for admin settings" operation in seerr-api.yml: add '400',
'403', and '500' responses matching the actual handler behavior in
server/routes/settings/index.ts, each with a short description and a response
body that uses the same error schema/component used elsewhere (e.g.,
ErrorResponse or the common error schema) so generated clients know how to parse
error payloads; ensure the descriptions map to the handler's returned cases (bad
request, forbidden, internal server error) and mirror wording/structure used by
other documented endpoints.
In `@server/api/jellyfin.ts`:
- Around line 226-235: getActiveUserId currently treats any falsy value the same
(so empty string triggers getUser) and getUser builds the URL using this.userId
?? 'Me' which does not default when userId is an empty string; update
getActiveUserId to only return when this.userId is a non-empty string (e.g. if
(typeof this.userId === 'string' && this.userId.length > 0) return this.userId;)
and update getUser to build its endpoint using a truthy check (e.g. use
this.userId || 'Me' or (this.userId && this.userId.length) ? this.userId : 'Me')
so empty strings resolve to 'Me' and you avoid the malformed /Users/ endpoint.
In `@server/entity/Media.ts`:
- Around line 191-220: The entity currently only stores one Plex/Jellyfin slot
per quality (plexServerId, plexServerId4k, jellyfinMediaId, jellyfinMediaId4k,
jellyfinServerId, jellyfinServerId4k), which collapses multiple sources from the
same provider; replace these singular provider-ID columns with a canonical
multi-source representation by using the existing mediaUrls/mediaUrls4k arrays
(or a new Column of type json storing MediaLink[]), persist all provider
links/ids there, remove or deprecate the single-ID fields (plexServerId*,
jellyfin*), and update any code that reads/writes plexServerId, plexServerId4k,
jellyfinMediaId, jellyfinServerId, mediaUrl/mediaUrl4k to read/write the
mediaUrls/mediaUrls4k array entries (or a map keyed by provider+server) so
multiple Plex/Jellyfin sources per quality can be stored and returned.
- Around line 253-304: The current buildPlexUrls and buildJellyfinUrls fall back
to the primary server whenever find(...) returns undefined, which remaps an
explicit stored serverId to the primary and can produce incorrect deep links;
change the lookup logic so that if a serverId was provided (serverId !==
null/undefined) and no matching server is found, the function returns null,
otherwise (when serverId was not provided) use settings.getPrimaryPlexServer() /
settings.getPrimaryJellyfinLikeServer(); keep the subsequent checks for
server.machineId / server.serverId and the existing URL construction (including
tautulliUrl handling) unchanged.
In `@server/lib/scanners/plex/index.ts`:
- Around line 84-86: this.libraries and currentLibrary are flattened to plain
Library objects causing duplicate library.id collisions across multiple Plex
servers; preserve the originating server id when flattening and when selecting
currentLibrary by adding serverId to each flattened library (or create a
composite key like `${server.id}:${library.id}`) so all internal tracking and
the UI can match on serverId+id instead of id alone; update the places that
build/consume libraries (the this.libraries assignment, the code that sets
currentLibrary, and the scan status structures referenced around the other
mentioned blocks) to include serverId/compositeId and ensure the UI uses that
composite key for matching and remaining-count calculations.
In `@server/routes/settings/index.ts`:
- Around line 414-483: The route settingsRoutes.get('/plex/library') currently
checks ownership only for sync but not for enable; modify the early
authorization check to require the owner (req.user?.id === 1) when either
req.query.sync or req.query.enable is present (e.g., replace the current
sync-only check with a combined condition), returning 403 with the existing
message if the user is not the owner; update logic around
getPlexServerFromRequest and before calling settings.updatePlexServer so that
non-owners cannot persist changes, and keep subsequent calls to
settings.updatePlexServer and settings.save unchanged.
- Around line 267-270: Before calling settings.upsertPlexServer, detect whether
a server with the same external identifier already exists (for Plex use
result.MediaContainer.machineIdentifier / plexServer.machineId; for Jellyfin use
result.Id) and reject the request instead of upserting a new record under a
different local UUID; implement this by querying existing servers (e.g., via the
settings API: find/get list of Plex/Jellyfin servers or add a new helper like
findPlexServerByMachineId/findJellyfinServerById) and if a match is found return
an error or validation failure rather than calling settings.upsertPlexServer
(also apply the same duplicate-check logic to the Jellyfin upsert block around
the lines referenced 610-614).
In `@src/components/Login/JellyfinLogin.tsx`:
- Around line 144-165: The server picker (<select id="serverId" name="serverId"
value={selectedServer?.id} onChange={setSelectedServerId} inside the conditional
that checks jellyfinServers.length) lacks an accessible label; add a semantic
label element or an aria-label/aria-labelledby referencing "serverId" and use
the same intl.formatMessage(messages.server) text so screen readers announce the
field—update the JSX to include a <label> tied to serverId (or add
aria-label={intl.formatMessage(messages.server)}) alongside the existing select
to make it accessible.
---
Outside diff comments:
In `@seerr-api.yml`:
- Around line 553-587: The OpenAPI JellyfinSettings schema is out of sync with
the backend model used by JellyfinServerInstance and GET /jellyfin/servers;
replace the current schema properties (hostname, adminUser, adminPass) with the
backend fields ip, port, useSsl, and urlBase to match
server/lib/settings/index.ts's JellyfinSettings interface, ensure
required/readonly flags mirror the backend (e.g., id, name, libraries, serverId
remain), and update any references to JellyfinSettings (such as
JellyfinServerInstance) so generated clients expect the same fields as the API
responses.
In `@server/api/jellyfin.ts`:
- Around line 130-161: The migration instantiates JellyfinAPI with only three
arguments which causes the constructor (JellyfinAPI) to default mediaServerType
to settings.main.mediaServerType; update the migration (the createApiToken call
in the 0002_migrate_apitokens migration) to pass the fourth parameter so the
correct server type is used—derive it from the Jellyfin settings object (e.g.,
jellyfinSettings.mediaServerType or similar property) and pass that as the
mediaServerType argument to the JellyfinAPI constructor to avoid using the
global default and ensure methods like getRecentlyAdded resolve routes
correctly.
In `@server/lib/availabilitySync.ts`:
- Around line 535-554: The current branch in mediaUpdater uses mediaServerType
to decide which provider fields to null, leaving IDs from the other provider
intact; change it so when isMediaProcessing is false you clear both Plex and
Jellyfin/Emby fields regardless of mediaServerType. Specifically, in
availabilitySync.ts inside mediaUpdater (symbols: mediaServerType,
MediaServerType.PLEX, MediaServerType.JELLYFIN, MediaServerType.EMBY, is4k,
isMediaProcessing), ensure you set media[is4k ? 'ratingKey4k' : 'ratingKey'] and
media[is4k ? 'plexServerId4k' : 'plexServerId'] to null when not
isMediaProcessing, and also set media[is4k ? 'jellyfinMediaId4k' :
'jellyfinMediaId'] and media[is4k ? 'jellyfinServerId4k' : 'jellyfinServerId']
to null when not isMediaProcessing (instead of choosing one branch by
mediaServerType).
In `@server/routes/user/index.ts`:
- Around line 683-695: The current existing-user branch updates and links a user
by email without applying the same server filtering used in the import branch;
ensure the same guards are applied before modifying an existing user.
Concretely, in the block handling "if (user) { ... }" add the same condition
used in the else-if branch (check that !body?.plexIds ||
body.plexIds.includes(account.id)) and that await
mainPlexTv.checkUserAccess(parseInt(account.id), plexServer.machineId) returns
true before assigning user.avatar/email/plexId/plexUsername and calling
userRepository.save(user).
---
Duplicate comments:
In `@src/components/Settings/SettingsJellyfin.tsx`:
- Around line 242-258: The auto-select useEffect doesn't run because the
predicate uses selectedServerId !== 'new' (so on first render when
selectedServerId === 'new' it skips); update the conditional in the effect that
reads
hasInitializedServerSelection/isSetupSettings/selectedServerId/jellyfinServers
to allow auto-selection when selectedServerId is the default 'new' (e.g., change
the (isSetupSettings || selectedServerId !== 'new') check to (isSetupSettings ||
selectedServerId === 'new')), then keep the body that calls
setSelectedServerId(jellyfinServers[0].id),
setNewServerType(jellyfinServers[0].mediaServerType) and
setHasInitializedServerSelection(true) so upgraded installs load the saved
Jellyfin/Emby server.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0e58a853-cdba-439b-9843-15011c6bfedb
📒 Files selected for processing (26)
seerr-api.ymlserver/api/jellyfin.tsserver/api/plextv.tsserver/entity/Media.tsserver/job/schedule.tsserver/lib/availabilitySync.tsserver/lib/scanners/jellyfin/index.tsserver/lib/scanners/plex/index.tsserver/lib/settings/index.tsserver/routes/avatarproxy.tsserver/routes/discover.tsserver/routes/settings/index.tsserver/routes/user/index.tssrc/components/Login/JellyfinLogin.tsxsrc/components/MovieDetails/index.tsxsrc/components/Settings/SettingsJellyfin.tsxsrc/components/Settings/SettingsPlex.tsxsrc/components/Settings/SettingsUsers/index.tsxsrc/components/Setup/index.tsxsrc/components/TitleCard/index.tsxsrc/components/TvDetails/index.tsxsrc/components/UserList/JellyfinImportModal.tsxsrc/components/UserList/PlexImportModal.tsxsrc/components/UserList/index.tsxsrc/i18n/locale/en.jsonsrc/pages/_app.tsx
🚧 Files skipped from review as they are similar to previous changes (8)
- server/api/plextv.ts
- src/pages/_app.tsx
- src/components/UserList/index.tsx
- src/components/Setup/index.tsx
- src/components/UserList/JellyfinImportModal.tsx
- src/components/TvDetails/index.tsx
- src/i18n/locale/en.json
- server/lib/settings/index.ts
Comment on lines
414
to
+483
| settingsRoutes.get('/plex/library', async (req, res) => { | ||
| if (req.query.sync && req.user?.id !== 1) { | ||
| return res.status(403).json({ | ||
| message: 'Only the owner can sync Plex libraries from settings.', | ||
| }); | ||
| } | ||
|
|
||
| const settings = getSettings(); | ||
| const server = getPlexServerFromRequest({ | ||
| serverId: req.query.serverId?.toString(), | ||
| }); | ||
|
|
||
| if (!server) { | ||
| if (req.query.serverId) { | ||
| return res.status(404).json({ error: 'Plex server not found' }); | ||
| } | ||
|
|
||
| return res.status(200).json([]); | ||
| } | ||
|
|
||
| if (req.query.sync) { | ||
| const userRepository = getRepository(User); | ||
| const admin = await userRepository.findOneOrFail({ | ||
| select: { id: true, plexToken: true }, | ||
| where: { id: 1 }, | ||
| }); | ||
| const plexapi = new PlexAPI({ plexToken: admin.plexToken }); | ||
| const plexapi = new PlexAPI({ | ||
| plexSettings: server, | ||
| plexToken: admin.plexToken, | ||
| }); | ||
|
|
||
| await plexapi.syncLibraries(); | ||
| await plexapi.syncLibraries(server.id); | ||
| } | ||
|
|
||
| const enabledLibraries = req.query.enable | ||
| ? (req.query.enable as string).split(',') | ||
| : []; | ||
| settings.plex.libraries = settings.plex.libraries.map((library) => ({ | ||
| ...library, | ||
| enabled: enabledLibraries.includes(library.id), | ||
|
|
||
| if (!settings.plexServers.some((plexServer) => plexServer.id === server.id)) { | ||
| return res.status(404).json({ message: 'Plex server not found.' }); | ||
| } | ||
|
|
||
| if (req.query.enable === undefined) { | ||
| return res | ||
| .status(200) | ||
| .json( | ||
| settings.plexServers.find((plexServer) => plexServer.id === server.id) | ||
| ?.libraries ?? [] | ||
| ); | ||
| } | ||
|
|
||
| const updated = settings.updatePlexServer(server.id, (plexServer) => ({ | ||
| ...plexServer, | ||
| libraries: plexServer.libraries.map((library) => ({ | ||
| ...library, | ||
| enabled: enabledLibraries.includes(library.id), | ||
| })), | ||
| })); | ||
|
|
||
| if (!updated) { | ||
| return res.status(404).json({ message: 'Plex server not found.' }); | ||
| } | ||
|
|
||
| await settings.save(); | ||
| return res.status(200).json(settings.plex.libraries); | ||
| return res | ||
| .status(200) | ||
| .json( | ||
| settings.plexServers.find((plexServer) => plexServer.id === server.id) | ||
| ?.libraries ?? [] | ||
| ); |
There was a problem hiding this comment.
/plex/library still lets non-owners change Plex configuration.
This route only rejects non-owners for sync=true. A non-owner admin can still send enable= and persist library-selection changes directly, which bypasses the owner-only guards on the rest of the Plex settings surface.
Suggested fix
settingsRoutes.get('/plex/library', async (req, res) => {
- if (req.query.sync && req.user?.id !== 1) {
+ if ((req.query.sync || req.query.enable !== undefined) && req.user?.id !== 1) {
return res.status(403).json({
- message: 'Only the owner can sync Plex libraries from settings.',
+ message: 'Only the owner can modify Plex libraries from settings.',
});
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/routes/settings/index.ts` around lines 414 - 483, The route
settingsRoutes.get('/plex/library') currently checks ownership only for sync but
not for enable; modify the early authorization check to require the owner
(req.user?.id === 1) when either req.query.sync or req.query.enable is present
(e.g., replace the current sync-only check with a combined condition), returning
403 with the existing message if the user is not the owner; update logic around
getPlexServerFromRequest and before calling settings.updatePlexServer so that
non-owners cannot persist changes, and keep subsequent calls to
settings.updatePlexServer and settings.save unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Refs #100
This PR adds support for configuring and running multiple media server instances simultaneously in Seerr, including any combination of:
The goal of this PR is to extend the existing architecture so multiple media servers can coexist without breaking current single-server behavior or legacy configurations.
Changes
settings.jsonlayouts while treatingplexServers/jellyfinServersas the canonical multi-server configurationAI Assistance Disclosure
This PR was generated primarily with Codex as a coding assistant. I reviewed the code in depth myself, tested the behavior locally, and understand the implementation being proposed here. Some parts of the authentication / authorization handling were also inspired by the approach explored in PR #2690 around simultaneous multi-auth support and related login/settings flows. I am happy to make follow-up changes if that would be helpful.
How Has This Been Tested?
Automated checks run locally:
fnm exec --using 22 pnpm typecheck:serverfnm exec --using 22 pnpm typecheck:clientfnm exec --using 22 pnpm lintfnm exec --using 22 pnpm buildCI=true fnm exec --using 22 pnpm testManual testing performed locally:
Screenshots (if applicable)
To be added later.
Checklist:
pnpm buildpnpm i18n:extractSummary by CodeRabbit
New Features
Improvements