feat(plex|jellyfin): add regex-based ignored path patterns for media filtering#2606
feat(plex|jellyfin): add regex-based ignored path patterns for media filtering#2606Rubeanie wants to merge 11 commits intoseerr-team:developfrom
Conversation
📝 WalkthroughWalkthroughAdds ignore-path pattern support end-to-end: settings, UI, regex caching, path extraction for Plex/Jellyfin, scanner pre-filtering, availability gating, and a BaseScanner helper to clear stale media-server links; plus a small Plex Media typing addition. Changes
Sequence Diagram(s)sequenceDiagram
participant User as "User"
participant UI as "Settings UI"
participant API as "Settings API"
participant Store as "Settings Store"
participant Scanner as "Plex/Jellyfin Scanner"
participant Filter as "Path Filter Utils"
participant Processor as "Media Processor"
User->>UI: Edit ignoredPathPatterns
UI->>API: POST /main (patterns)
API->>Store: Save settings.ignoredPathPatterns
Store-->>API: Confirm saved
API-->>UI: Return updated settings
Scanner->>Store: Read ignoredPathPatterns
Store-->>Scanner: Return patterns
Scanner->>Filter: getPlexFilePaths / getJellyfinFilePaths
Filter-->>Scanner: filePaths
Scanner->>Filter: isPathIgnored(filePaths)
Filter-->>Scanner: true / false
alt matches ignore
Scanner->>Processor: clearMediaServerLink / Skip item (log)
else not ignored
Scanner->>Processor: Process item (count/store)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/components/Settings/SettingsMain/index.tsx (2)
556-589: AddinputIdfor accessibility and consider z-index for dropdown stacking.Two minor issues:
The
<label htmlFor="ignoredPathPatterns">won't associate with the input becauseCreatableSelectrequires aninputIdprop (notid) to link with the label. This impacts accessibility—clicking the label won't focus the input.Other select fields in this form use
relative z-XXclasses to manage dropdown stacking order. Without this, the dropdown may render behind other form elements.♻️ Proposed fix
<div className="form-input-area"> - <div className="form-input-field relative"> + <div className="form-input-field relative z-10"> <CreatableSelect<TagOption, true> + inputId="ignoredPathPatterns" components={{ DropdownIndicator: null }} isClearable isMulti🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Settings/SettingsMain/index.tsx` around lines 556 - 589, The label isn't wired to the CreatableSelect and the dropdown may stack behind other elements; add inputId="ignoredPathPatterns" to the CreatableSelect so the existing <label htmlFor="ignoredPathPatterns"> associates correctly, and ensure the select gets the same stacking context as other selects by adding the appropriate positioning/z-index class (e.g., add a relative z-<n> class to the CreatableSelect's container or its className prop) so the dropdown appears above other form elements; locate the CreatableSelect component in SettingsMain and update its props accordingly.
566-586: Consider adding client-side regex validation for immediate feedback.Users could enter invalid regex patterns without knowing until the backend logs a warning during a scan. Adding validation in the
onChangehandler or via Yup schema would provide immediate feedback.This is optional since the server handles invalid patterns gracefully per the PR description.
💡 Example validation approach
onChange={(newValue) => { const validPatterns = newValue.filter((v) => { try { new RegExp(v.value); return true; } catch { addToast(`Invalid regex pattern: ${v.value}`, { appearance: 'warning', autoDismiss: true, }); return false; } }); setFieldValue( 'ignoredPathPatterns', validPatterns.map((v) => v.value) ); }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Settings/SettingsMain/index.tsx` around lines 566 - 586, Add client-side regex validation for ignoredPathPatterns by updating the CreatableSelect onChange handler used for the ignoredPathPatterns field: validate each newValue item by attempting new RegExp(v.value) and only pass valid patterns into setFieldValue('ignoredPathPatterns', ...); for invalid patterns show immediate feedback (e.g., call addToast or set a Formik field error) so users know which entries were rejected; ensure the handler still supports isMulti and preserves existing behavior for valid entries in the CreatableSelect component.
🤖 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 885-890: The series-level ignore check is insufficient because
getPlexFilePaths/isPathIgnored are only applied to top-level
plexMedia.Media/MediaSources (e.g., the plexMedia variable), missing
episode-level file paths; update the availability sync logic so season/show
existence resolution iterates episodes (and their Media/MediaSources), applies
getPlexFilePaths + isPathIgnored per episode, and treat a season/show as
existing only if at least one episode remains not ignored (adjust the season
existence resolver and any checks that use plexMedia/MediaSources such as the
block around plexMedia and the logic referenced near the other MediaSources
check at 1009–1015). Ensure you filter out ignored episode file paths before
deciding existence and propagate that filtered result back to the
show/season-level checks.
In `@server/routes/settings/index.ts`:
- Around line 82-84: The code is directly persisting
req.body.ignoredPathPatterns into settings.main.ignoredPathPatterns which can
include empty or whitespace-only strings (an empty regex matches everything);
before assignment, normalize by mapping each item to a trimmed string, filter
out empty/whitespace-only entries, optionally deduplicate, and validate each
remaining pattern by attempting to construct a RegExp (skip invalid patterns).
Update the block handling ignoredPathPatterns to assign only the cleaned,
validated array to settings.main.ignoredPathPatterns.
In `@server/utils/mediaFilter.ts`:
- Around line 16-29: isPathIgnored currently recompiles each admin-supplied
pattern on every call and repeatedly warns on the same bad patterns; change it
to compile-and-cache regexes once and dedupe invalid-pattern warnings by:
maintain a module-level Map<string, RegExp> (keyed by the raw pattern string or
a stable key derived from the patterns array) and a Set<string> for invalid
patterns, update the cache when the input patterns array changes, replace
on-the-fly new RegExp(...) calls in isPathIgnored with lookups into the Map, and
when compilation fails add the pattern to the invalid Set and call logger.warn
(from MediaFilter) only the first time that pattern is seen; keep using
normalized.some(fp => regex.test(fp)) for matching.
---
Nitpick comments:
In `@src/components/Settings/SettingsMain/index.tsx`:
- Around line 556-589: The label isn't wired to the CreatableSelect and the
dropdown may stack behind other elements; add inputId="ignoredPathPatterns" to
the CreatableSelect so the existing <label htmlFor="ignoredPathPatterns">
associates correctly, and ensure the select gets the same stacking context as
other selects by adding the appropriate positioning/z-index class (e.g., add a
relative z-<n> class to the CreatableSelect's container or its className prop)
so the dropdown appears above other form elements; locate the CreatableSelect
component in SettingsMain and update its props accordingly.
- Around line 566-586: Add client-side regex validation for ignoredPathPatterns
by updating the CreatableSelect onChange handler used for the
ignoredPathPatterns field: validate each newValue item by attempting new
RegExp(v.value) and only pass valid patterns into
setFieldValue('ignoredPathPatterns', ...); for invalid patterns show immediate
feedback (e.g., call addToast or set a Formik field error) so users know which
entries were rejected; ensure the handler still supports isMulti and preserves
existing behavior for valid entries in the CreatableSelect component.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
server/api/plexapi.tsserver/lib/availabilitySync.tsserver/lib/scanners/jellyfin/index.tsserver/lib/scanners/plex/index.tsserver/lib/settings/index.tsserver/routes/settings/index.tsserver/utils/mediaFilter.tssrc/components/Settings/SettingsMain/index.tsxsrc/i18n/locale/en.json
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
server/lib/availabilitySync.ts (3)
938-940: Minor edge case: emptyfilteredSeasonstriggers false negative.When
filteredSeasonsis empty (no seasons marked AVAILABLE/PARTIALLY_AVAILABLE for the current resolution mode),seasonsMapremains empty, causingexistsInPlexto be reset tofalseeven though the show metadata was found in Plex.This could occur if media is loaded due to its 4K status but checked for non-4K availability (or vice versa). The impact is likely low given the sync's purpose, but consider guarding against this:
🔧 Suggested refinement
- if (existsInPlex && seasonsMap.size === 0 && !preventSeasonSearch) { + const hasFilteredSeasons = media.seasons.some( + (s) => + s[is4k ? 'status4k' : 'status'] === MediaStatus.AVAILABLE || + s[is4k ? 'status4k' : 'status'] === MediaStatus.PARTIALLY_AVAILABLE + ); + if (existsInPlex && seasonsMap.size === 0 && !preventSeasonSearch && hasFilteredSeasons) { existsInPlex = false; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/lib/availabilitySync.ts` around lines 938 - 940, The code flips existsInPlex to false when seasonsMap.size === 0 which wrongly treats an empty filteredSeasons as a negative; change the condition so existsInPlex is only reset when seasonsMap is empty AND there were seasons actually considered for the current resolution mode (e.g., filteredSeasons.length > 0) and !preventSeasonSearch. In other words, in the block using existsInPlex, seasonsMap, preventSeasonSearch, and filteredSeasons, add a guard that skips resetting existsInPlex if filteredSeasons is empty (or undefined) so finding show-level metadata in Plex isn’t negated by having no season matches for the chosen resolution.
994-996: Consider logging the error for observability.Silently returning
trueon error is a safe fallback, but logging the failure would help diagnose issues when seasons are unexpectedly retained.🔧 Suggested enhancement
} catch { + logger.debug( + `Failed to fetch episodes for Plex season ${season.seasonNumber} [TMDB ID ${media.tmdbId}]. Defaulting to exists=true.`, + { label: 'AvailabilitySync' } + ); return true; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/lib/availabilitySync.ts` around lines 994 - 996, The catch block in server/lib/availabilitySync.ts that currently just returns true should log the caught error for observability before returning; update that catch to accept the error (e) and emit a clear log entry (using your project's logger instance such as logger.error or console.error) with context like "availabilitySync: failed to determine whether to retain seasons" and the error details, then continue to return true as the safe fallback.
1149-1151: Consider logging the error for observability.Same suggestion as for Plex at lines 994-996 — logging failures helps diagnose unexpected behavior.
🔧 Suggested enhancement
} catch { + logger.debug( + `Failed to fetch episodes for Jellyfin season ${season.seasonNumber} [TMDB ID ${media.tmdbId}]. Defaulting to exists=true.`, + { label: 'AvailabilitySync' } + ); return true; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/lib/availabilitySync.ts` around lines 1149 - 1151, The catch block that currently reads "} catch { return true; }" should capture the thrown error and log it for observability before returning; modify that block to catch the error (e.g., catch (err)) and call the module's logger (for example processLogger.error or the existing logger used elsewhere in availabilitySync.ts) with a clear message and the error details, then return true as before.
🤖 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 1142-1148: The extraction of file paths inside the episodes.some
callback can produce undefined entries because MediaSources?.map((ms) =>
ms.Path) may yield undefined Path values; update the logic that builds filePaths
(the inline ternary using 'MediaSources' in episode and the map) to filter out
falsy/undefined paths (e.g., map then .filter(Boolean) or reduce) before
checking filePaths.length and calling isPathIgnored, ensuring only valid string
paths are passed to isPathIgnored and the length check.
---
Nitpick comments:
In `@server/lib/availabilitySync.ts`:
- Around line 938-940: The code flips existsInPlex to false when seasonsMap.size
=== 0 which wrongly treats an empty filteredSeasons as a negative; change the
condition so existsInPlex is only reset when seasonsMap is empty AND there were
seasons actually considered for the current resolution mode (e.g.,
filteredSeasons.length > 0) and !preventSeasonSearch. In other words, in the
block using existsInPlex, seasonsMap, preventSeasonSearch, and filteredSeasons,
add a guard that skips resetting existsInPlex if filteredSeasons is empty (or
undefined) so finding show-level metadata in Plex isn’t negated by having no
season matches for the chosen resolution.
- Around line 994-996: The catch block in server/lib/availabilitySync.ts that
currently just returns true should log the caught error for observability before
returning; update that catch to accept the error (e) and emit a clear log entry
(using your project's logger instance such as logger.error or console.error)
with context like "availabilitySync: failed to determine whether to retain
seasons" and the error details, then continue to return true as the safe
fallback.
- Around line 1149-1151: The catch block that currently reads "} catch { return
true; }" should capture the thrown error and log it for observability before
returning; modify that block to catch the error (e.g., catch (err)) and call the
module's logger (for example processLogger.error or the existing logger used
elsewhere in availabilitySync.ts) with a clear message and the error details,
then return true as before.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
server/lib/availabilitySync.ts (1)
889-894:⚠️ Potential issue | 🟠 MajorRestrict top-level ignore checks to movies to avoid false TV show deletions.
Lines 889-894 (Plex) and 1052-1057 (Jellyfin) apply root metadata path-ignore checks to both movies and TV shows. For TV shows, if a series root path is ignored but episodes still exist in non-ignored locations,
existsInPlex/existsInJellyfinremains false before season-level checks run, preventing proper season-level availability evaluation and incorrectly marking the show as deleted.Proposed fix
- if (plexMedia) { + if (plexMedia && media.mediaType === 'movie') { const filePaths = getPlexFilePaths(plexMedia.Media ?? []); if (isPathIgnored(filePaths)) { plexMedia = undefined; } } @@ - if (jellyfinMedia) { + if (jellyfinMedia && media.mediaType === 'movie') { const jfFilePaths = getJellyfinFilePaths(jellyfinMedia.MediaSources); if (isPathIgnored(jfFilePaths)) { jellyfinMedia = undefined; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/lib/availabilitySync.ts` around lines 889 - 894, Restrict the top-level root-path ignore check so it runs only for movies: in availabilitySync.ts update the Plex block that currently inspects plexMedia using getPlexFilePaths and isPathIgnored (and the analogous Jellyfin block that handles jellyfinMedia) to skip clearing plexMedia/jellyfinMedia when the metadata represents a TV show; instead allow season/episode-level checks to determine availability. Concretely, guard the existing isPathIgnored logic with a check that the item is a movie (use the metadata/type on plexMedia/jellyfinMedia or presence/absence of season/episode fields) so existsInPlex/existsInJellyfin remains correct for series and seasons are evaluated later.
🧹 Nitpick comments (1)
server/lib/scanners/jellyfin/index.ts (1)
129-135: Use debug level for ignored-item skip logs to reduce sync noise.This message can get very chatty in large libraries with broad patterns. Consider downgrading it from default logging level.
Suggested tweak
if (isPathIgnored(jfFilePaths)) { this.log( - `Skipping movie ${metadata.Name} (file path matched ignore pattern)` + `Skipping movie ${metadata.Name} (file path matched ignore pattern)`, + 'debug' ); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/lib/scanners/jellyfin/index.ts` around lines 129 - 135, The skip message for ignored Jellyfin file paths is logged at the default level via this.log; change it to a debug-level log to reduce noise by replacing the this.log(...) call in the block that checks getJellyfinFilePaths(metadata.MediaSources) and isPathIgnored(jfFilePaths) with a debug-level method (e.g. this.debug(...) or this.logger.debug(...), matching the scanner's logger API) so the message is emitted only when debug logging is enabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@server/lib/availabilitySync.ts`:
- Around line 889-894: Restrict the top-level root-path ignore check so it runs
only for movies: in availabilitySync.ts update the Plex block that currently
inspects plexMedia using getPlexFilePaths and isPathIgnored (and the analogous
Jellyfin block that handles jellyfinMedia) to skip clearing
plexMedia/jellyfinMedia when the metadata represents a TV show; instead allow
season/episode-level checks to determine availability. Concretely, guard the
existing isPathIgnored logic with a check that the item is a movie (use the
metadata/type on plexMedia/jellyfinMedia or presence/absence of season/episode
fields) so existsInPlex/existsInJellyfin remains correct for series and seasons
are evaluated later.
---
Nitpick comments:
In `@server/lib/scanners/jellyfin/index.ts`:
- Around line 129-135: The skip message for ignored Jellyfin file paths is
logged at the default level via this.log; change it to a debug-level log to
reduce noise by replacing the this.log(...) call in the block that checks
getJellyfinFilePaths(metadata.MediaSources) and isPathIgnored(jfFilePaths) with
a debug-level method (e.g. this.debug(...) or this.logger.debug(...), matching
the scanner's logger API) so the message is emitted only when debug logging is
enabled.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
server/lib/availabilitySync.tsserver/lib/scanners/jellyfin/index.tsserver/utils/mediaFilter.ts
|
@coderabbitai resume Sorry, not actively developing just using the coderabbit feedback 😅 |
✅ Actions performedReviews resumed. |
|
✅ Actions performedReviews resumed. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/scanners/plex/index.ts`:
- Around line 255-261: The early-return skip in processPlexMovieByTmdbId using
isPathIgnored on filePaths causes HAMA movies to be false-negatively skipped if
only the first episode is ignored; change the logic to check all episodes
returned to processHamaMovie (or filter episodes before probing): compute
getPlexFilePaths for each episode in the episodes array and only skip the movie
if every episode’s file path matches isPathIgnored, otherwise proceed (or in
processHamaMovie, iterate/filter episodes so probe attempts are made on the
first non-ignored episode); update references in processPlexMovieByTmdbId and
processHamaMovie accordingly.
- Around line 361-365: The season total currently uses TMDB's
season.episode_count which ignores locally filtered episodes, causing ignored
episodes to appear "missing"; update the totalEpisodes calculation so that when
hasIgnoredPatterns is true it derives the count from the filtered episodes array
(the variable episodes produced by filtering allEpisodes using
isPathIgnored(getPlexFilePaths(...))) and otherwise falls back to
season.episode_count, ensuring totalEpisodes reflects the actual non-ignored
episodes used in availability math.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
server/lib/scanners/jellyfin/index.ts (1)
318-336: Good defensive handling, but clarify behavior for episodes without MediaSources.The check
'MediaSources' in episodeis a good defensive pattern. WhenhasIgnoredPatternsis true,needMediaInfoensures media info is requested (line 320-325). If an episode still lacksMediaSources(API edge case), the fallback to an empty array meansisPathIgnored([])is called—the episode won't be filtered, which is the safe default.Consider adding a debug log when an episode is expected to have
MediaSourcesbut doesn't, to help diagnose potential issues:💡 Optional: Add debug logging for missing MediaSources
const episodes = hasIgnoredPatterns ? allEpisodes.filter((episode) => { - const paths = - 'MediaSources' in episode - ? getJellyfinFilePaths(episode.MediaSources) - : []; + if (!('MediaSources' in episode)) { + this.log( + `Episode ${episode.Name ?? episode.Id} missing MediaSources, cannot check ignore patterns`, + 'debug' + ); + return true; // Don't filter if we can't check + } + const paths = getJellyfinFilePaths(episode.MediaSources); return !isPathIgnored(paths); }) : allEpisodes;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/lib/scanners/jellyfin/index.ts` around lines 318 - 336, When filtering episodes with hasIgnoredPatterns, add a debug log when an episode is expected to have MediaSources but doesn't: inside the episodes filter (where 'MediaSources' in episode is checked) emit a debug message (using the scanner's logger, e.g., this.logger.debug) including identifying info (episode.Id, episode.Name, season Id/Id variable) so missing MediaSources can be diagnosed; keep the current safe fallback to [] and still call isPathIgnored([]) but log the anomaly when needMediaInfo is true/hasIgnoredPatterns is true to aid debugging.
🤖 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/scanners/jellyfin/index.ts`:
- Around line 401-403: The totalEpisodes calculation in the Jellyfin scanner
(where totalEpisodes is computed using season.episode_count, hasIgnoredPatterns,
allEpisodes and episodes) can go negative if Jellyfin has more episodes than
TMDB; fix it by applying the same defensive guard as Plex—wrap the computed
value with Math.max(0, computedValue) so totalEpisodes never drops below 0
(i.e., replace the ternary result with a clamped value using Math.max and keep
the existing hasIgnoredPatterns logic intact).
In `@server/lib/scanners/plex/index.ts`:
- Around line 361-365: The subtraction for totalEpisodes can become negative
when Plex reports more episodes than TMDB; locate the calculation that uses
season.episode_count - (allEpisodes.length - episodes.length) (and the similar
calculation around the other occurrence) and clamp the result to a non-negative
integer (e.g., wrap with Math.max(0, ...)) and ensure the result is an integer
type before using it so totalEpisodes never becomes negative.
---
Nitpick comments:
In `@server/lib/scanners/jellyfin/index.ts`:
- Around line 318-336: When filtering episodes with hasIgnoredPatterns, add a
debug log when an episode is expected to have MediaSources but doesn't: inside
the episodes filter (where 'MediaSources' in episode is checked) emit a debug
message (using the scanner's logger, e.g., this.logger.debug) including
identifying info (episode.Id, episode.Name, season Id/Id variable) so missing
MediaSources can be diagnosed; keep the current safe fallback to [] and still
call isPathIgnored([]) but log the anomaly when needMediaInfo is
true/hasIgnoredPatterns is true to aid debugging.
|
Apologies for the activity, I believe all the found issues are now resolved and I don't see any significant remaining improvements |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/scanners/baseScanner.ts`:
- Around line 610-636: The current read/modify/save on Media
(mediaRepository.findOne + mediaRepository.save) can clobber concurrent updates
from processMovie/processShow; instead perform partial, conditional updates so
only the fields that need clearing are changed and only if they still match the
stale value. Replace the full-entity save with targeted repository/queryBuilder
updates: for each field that needs clearing (ratingKey, ratingKey4k,
jellyfinMediaId, jellyfinMediaId4k) call a partial update (e.g.,
mediaRepository.createQueryBuilder().update().set({ ratingKey: null }).where("id
= :id AND ratingKey = :rk", { id: media.id, rk: options.ratingKey })). Do the
same for each field pair (use options.ratingKey/options.jellyfinMediaId) so you
only clear when the DB still holds the stale value; keep the initial findOne
check to short-circuit when media is missing and log if any update queries
affected rows.
In `@server/lib/scanners/jellyfin/index.ts`:
- Around line 342-347: episodeCount can become negative when
episode.IndexNumberEnd < episode.IndexNumber; update the calculation that sets
episodeCount (where it currently uses episode.IndexNumberEnd -
episode.IndexNumber + 1) to clamp the result to at least 1 (e.g., compute
rawCount = episode.IndexNumberEnd - episode.IndexNumber + 1 and then set
episodeCount = Math.max(1, rawCount)) so ranged episode counts never go below 1;
ensure this change is applied wherever episodeCount is assigned from
IndexNumber/IndexNumberEnd.
In `@server/lib/scanners/plex/index.ts`:
- Around line 405-413: The current guard requires tvdbId before calling
this.processShow, which incorrectly skips shows that can be resolved by TMDB;
remove the tvdbId requirement so that when hasAnyEpisodes is true you always
call this.processShow with mediaIds.tmdbId and pass tvdbId (which may be
undefined) and the same processableSeasons and metadata object; keep the
existing else branch that calls this.clearMediaServerLink({ ratingKey }) when
!hasAnyEpisodes. Ensure you reference the same parameter names (processShow,
tvdbId, tmdbId, hasAnyEpisodes, clearMediaServerLink) so BaseScanner.processShow
can accept an optional tvdbId.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
server/lib/scanners/baseScanner.tsserver/lib/scanners/jellyfin/index.tsserver/lib/scanners/plex/index.ts
There was a problem hiding this comment.
♻️ Duplicate comments (1)
server/lib/scanners/baseScanner.ts (1)
610-631:⚠️ Potential issue | 🟠 MajorUse stale-value predicates in updates to avoid concurrent key clobbering.
Line 610 reads, then Line 631 updates by
idonly. A concurrentprocessMovie/processShowwrite can land between those calls and still get nulled here. Clear each field withWHERE id = :id AND <field> = :staleValueso only unchanged stale links are removed.Suggested fix
- const patch: Partial<Media> = {}; - if (options.ratingKey) { - if (media.ratingKey === options.ratingKey) { - patch.ratingKey = null; - } - if (media.ratingKey4k === options.ratingKey) { - patch.ratingKey4k = null; - } - } - if (options.jellyfinMediaId) { - if (media.jellyfinMediaId === options.jellyfinMediaId) { - patch.jellyfinMediaId = null; - } - if (media.jellyfinMediaId4k === options.jellyfinMediaId) { - patch.jellyfinMediaId4k = null; - } - } - if (Object.keys(patch).length > 0) { - await mediaRepository.update({ id: media.id }, patch); - this.log('Cleared stale media server link for ignored item', 'info'); - } + let cleared = false; + const markCleared = (affected?: number) => { + if ((affected ?? 0) > 0) cleared = true; + }; + + if (options.ratingKey) { + const rk = options.ratingKey; + markCleared( + ( + await mediaRepository + .createQueryBuilder() + .update(Media) + .set({ ratingKey: null }) + .where('id = :id AND ratingKey = :rk', { id: media.id, rk }) + .execute() + ).affected + ); + markCleared( + ( + await mediaRepository + .createQueryBuilder() + .update(Media) + .set({ ratingKey4k: null }) + .where('id = :id AND ratingKey4k = :rk', { id: media.id, rk }) + .execute() + ).affected + ); + } + + if (options.jellyfinMediaId) { + const jf = options.jellyfinMediaId; + markCleared( + ( + await mediaRepository + .createQueryBuilder() + .update(Media) + .set({ jellyfinMediaId: null }) + .where('id = :id AND jellyfinMediaId = :jf', { id: media.id, jf }) + .execute() + ).affected + ); + markCleared( + ( + await mediaRepository + .createQueryBuilder() + .update(Media) + .set({ jellyfinMediaId4k: null }) + .where('id = :id AND jellyfinMediaId4k = :jf', { + id: media.id, + jf, + }) + .execute() + ).affected + ); + } + + if (cleared) { + this.log('Cleared stale media server link for ignored item', 'info'); + }#!/bin/bash # Verify clearMediaServerLink still uses read-then-id-only-update (TOCTOU risk). python - <<'PY' from pathlib import Path path = Path("server/lib/scanners/baseScanner.ts") text = path.read_text() start = text.find("protected async clearMediaServerLink") end = text.find("protected startRun", start) method = text[start:end] print("findOne present:", "findOne({ where: whereConditions })" in method) print("id-only update present:", "update({ id: media.id }, patch)" in method) print( "conditional stale-value WHERE present:", any(s in method for s in [ "id = :id AND ratingKey = :rk", "id = :id AND ratingKey4k = :rk", "id = :id AND jellyfinMediaId = :jf", "id = :id AND jellyfinMediaId4k = :jf", ]) ) PYExpected: first two
Trueand lastFalseconfirms the race window still exists.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/lib/scanners/baseScanner.ts` around lines 610 - 631, The clearMediaServerLink method currently reads the Media with mediaRepository.findOne and later calls mediaRepository.update({ id: media.id }, patch), which creates a TOCTOU race where concurrent writes can clobber keys; change the update logic so each cleared field is updated only if it still matches the stale value read—e.g., instead of a single id-only update, perform conditional updates using WHERE id = :id AND ratingKey = :staleRatingKey (and similarly for ratingKey4k, jellyfinMediaId, jellyfinMediaId4k), or use the query builder to issue UPDATE ... WHERE id = :id AND <field> = :staleValue for each field present in patch so only unchanged stale links are nulled and concurrent updates are preserved (update the clearMediaServerLink function and replace mediaRepository.update({ id: media.id }, patch) with per-field conditional updates).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@server/lib/scanners/baseScanner.ts`:
- Around line 610-631: The clearMediaServerLink method currently reads the Media
with mediaRepository.findOne and later calls mediaRepository.update({ id:
media.id }, patch), which creates a TOCTOU race where concurrent writes can
clobber keys; change the update logic so each cleared field is updated only if
it still matches the stale value read—e.g., instead of a single id-only update,
perform conditional updates using WHERE id = :id AND ratingKey = :staleRatingKey
(and similarly for ratingKey4k, jellyfinMediaId, jellyfinMediaId4k), or use the
query builder to issue UPDATE ... WHERE id = :id AND <field> = :staleValue for
each field present in patch so only unchanged stale links are nulled and
concurrent updates are preserved (update the clearMediaServerLink function and
replace mediaRepository.update({ id: media.id }, patch) with per-field
conditional updates).
304b160 to
d256e75
Compare
|
Rebase |
|
✅ Actions performedReview triggered.
|
|
The PR description images show the Ignored Path Patterns UI — the first image displays an empty input with the placeholder "Type a regex pattern and press Enter...", and the second shows a
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
server/lib/scanners/baseScanner.ts (1)
610-632:⚠️ Potential issue | 🟠 MajorGuard nulling updates with stale-value predicates to avoid clobbering concurrent writes.
Line 631 updates by
idonly. A concurrentprocessMovie/processShowrelink can be overwritten tonullafter the read at Line 610.🛠️ Suggested fix
- const patch: Partial<Media> = {}; - if (options.ratingKey) { - if (media.ratingKey === options.ratingKey) { - patch.ratingKey = null; - } - if (media.ratingKey4k === options.ratingKey) { - patch.ratingKey4k = null; - } - } - if (options.jellyfinMediaId) { - if (media.jellyfinMediaId === options.jellyfinMediaId) { - patch.jellyfinMediaId = null; - } - if (media.jellyfinMediaId4k === options.jellyfinMediaId) { - patch.jellyfinMediaId4k = null; - } - } - if (Object.keys(patch).length > 0) { - await mediaRepository.update({ id: media.id }, patch); + let updated = false; + if (options.ratingKey && media.ratingKey === options.ratingKey) { + const result = await mediaRepository.update( + { id: media.id, ratingKey: options.ratingKey }, + { ratingKey: null } + ); + updated ||= (result.affected ?? 0) > 0; + } + if (options.ratingKey && media.ratingKey4k === options.ratingKey) { + const result = await mediaRepository.update( + { id: media.id, ratingKey4k: options.ratingKey }, + { ratingKey4k: null } + ); + updated ||= (result.affected ?? 0) > 0; + } + if ( + options.jellyfinMediaId && + media.jellyfinMediaId === options.jellyfinMediaId + ) { + const result = await mediaRepository.update( + { id: media.id, jellyfinMediaId: options.jellyfinMediaId }, + { jellyfinMediaId: null } + ); + updated ||= (result.affected ?? 0) > 0; + } + if ( + options.jellyfinMediaId && + media.jellyfinMediaId4k === options.jellyfinMediaId + ) { + const result = await mediaRepository.update( + { id: media.id, jellyfinMediaId4k: options.jellyfinMediaId }, + { jellyfinMediaId4k: null } + ); + updated ||= (result.affected ?? 0) > 0; + } + if (updated) { this.log('Cleared stale media server link for ignored item', 'info'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/lib/scanners/baseScanner.ts` around lines 610 - 632, The current clear-by-id update can clobber concurrent relinks (e.g., from processMovie/processShow); instead build conditional update predicates that include the current stale values so you only null fields if the DB still matches what you read. For the update call used with mediaRepository.update({ id: media.id }, patch) add matching predicates for each cleared field (e.g., require ratingKey === media.ratingKey, ratingKey4k === media.ratingKey4k, jellyfinMediaId === media.jellyfinMediaId, jellyfinMediaId4k === media.jellyfinMediaId4k) and only include those predicates for fields you intend to null; keep the patch logic that sets fields to null and only perform the update when Object.keys(patch).length > 0 so the DB change occurs atomically and won’t overwrite concurrent relinks.
🧹 Nitpick comments (1)
src/components/Settings/SettingsMain/index.tsx (1)
582-599: Normalize before validation to keep UI behavior consistent with submit/backend.Line 586 validates untrimmed input, while submit trims at Line 213. Trimming in
onChangeavoids inconsistent acceptance/error behavior.♻️ Suggested refactor
onChange={(newValue) => { const valid: string[] = []; for (const v of newValue) { + const pattern = v.value.trim(); + if (!pattern) continue; try { - new RegExp(v.value); - valid.push(v.value); + new RegExp(pattern); + valid.push(pattern); } catch { addToast( intl.formatMessage( messages.ignoredPathPatternsInvalidRegex, - { pattern: v.value } + { pattern } ), { autoDismiss: true, appearance: 'error' } ); } } - setFieldValue('ignoredPathPatterns', valid); + setFieldValue( + 'ignoredPathPatterns', + [...new Set(valid)] + ); }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Settings/SettingsMain/index.tsx` around lines 582 - 599, In the onChange handler for ignoredPathPatterns in SettingsMain (the function that builds and validates newValue before calling setFieldValue), trim each v.value before attempting RegExp validation and before adding to the valid array; on validation failure use the trimmed pattern in the addToast error message. Ensure the final setFieldValue('ignoredPathPatterns', valid) receives trimmed strings so behavior matches the submit logic that trims at 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/routes/settings/index.ts`:
- Around line 79-99: The code merges req.body into settings.main before
sanitizing ignoredPathPatterns, which allows non-array or malformed
ignoredPathPatterns to persist; fix by handling req.body.ignoredPathPatterns
before calling merge(): read req.body.ignoredPathPatterns, if it's an array
sanitize/trim/validate into cleaned and set req.body.ignoredPathPatterns =
cleaned (or delete req.body.ignoredPathPatterns if not an array or cleaned is
empty), then call merge(settings.main, req.body) and finally (or as currently)
assign settings.main.ignoredPathPatterns = cleaned only when you produced a
sanitized array; reference settings.main, req.body.ignoredPathPatterns, and
merge() to locate the changes.
---
Duplicate comments:
In `@server/lib/scanners/baseScanner.ts`:
- Around line 610-632: The current clear-by-id update can clobber concurrent
relinks (e.g., from processMovie/processShow); instead build conditional update
predicates that include the current stale values so you only null fields if the
DB still matches what you read. For the update call used with
mediaRepository.update({ id: media.id }, patch) add matching predicates for each
cleared field (e.g., require ratingKey === media.ratingKey, ratingKey4k ===
media.ratingKey4k, jellyfinMediaId === media.jellyfinMediaId, jellyfinMediaId4k
=== media.jellyfinMediaId4k) and only include those predicates for fields you
intend to null; keep the patch logic that sets fields to null and only perform
the update when Object.keys(patch).length > 0 so the DB change occurs atomically
and won’t overwrite concurrent relinks.
---
Nitpick comments:
In `@src/components/Settings/SettingsMain/index.tsx`:
- Around line 582-599: In the onChange handler for ignoredPathPatterns in
SettingsMain (the function that builds and validates newValue before calling
setFieldValue), trim each v.value before attempting RegExp validation and before
adding to the valid array; on validation failure use the trimmed pattern in the
addToast error message. Ensure the final setFieldValue('ignoredPathPatterns',
valid) receives trimmed strings so behavior matches the submit logic that trims
at save.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bfce8c62-8c64-4e8e-ad1d-34b65a37ebe7
📒 Files selected for processing (10)
server/api/plexapi.tsserver/lib/availabilitySync.tsserver/lib/scanners/baseScanner.tsserver/lib/scanners/jellyfin/index.tsserver/lib/scanners/plex/index.tsserver/lib/settings/index.tsserver/routes/settings/index.tsserver/utils/mediaFilter.tssrc/components/Settings/SettingsMain/index.tsxsrc/i18n/locale/en.json
🚧 Files skipped from review as they are similar to previous changes (1)
- src/i18n/locale/en.json
There was a problem hiding this comment.
🧹 Nitpick comments (2)
server/lib/availabilitySync.ts (1)
986-987: Snapshot ignore patterns once per sync run.Reading settings inside each season evaluation can make one sync pass apply mixed policies if settings are edited mid-run. Prefer a run-scoped snapshot.
♻️ Proposed refactor
class AvailabilitySync { public running = false; + private ignoredPathPatterns: string[] = []; private plexClient: PlexAPI; @@ async run() { const settings = getSettings(); + this.ignoredPathPatterns = settings.main.ignoredPathPatterns ?? []; @@ - const hasIgnorePatterns = - (getSettings().main.ignoredPathPatterns ?? []).length > 0; + const hasIgnorePatterns = this.ignoredPathPatterns.length > 0; @@ - const hasIgnorePatterns = - (getSettings().main.ignoredPathPatterns ?? []).length > 0; + const hasIgnorePatterns = this.ignoredPathPatterns.length > 0;Also applies to: 1150-1151
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/lib/availabilitySync.ts` around lines 986 - 987, The code calls getSettings() repeatedly during a sync run (e.g., when computing hasIgnorePatterns) which can lead to mixed behavior if settings change mid-run; capture a run-scoped snapshot instead: at the start of the availability sync (the main sync routine that calls evaluateSeason/season evaluation code), read getSettings().main.ignoredPathPatterns once into a const (e.g., ignoredPathPatternsSnapshot or hasIgnorePatternsSnapshot) and replace all in-run calls to getSettings().main.ignoredPathPatterns / hasIgnorePatterns with that snapshot (also update the occurrences around the other mentioned region, lines referencing getSettings() at 1150-1151) so the entire sync run uses a stable policy.server/lib/scanners/plex/index.ts (1)
405-413: Consider allowing shows without TVDB ID to be processed.Line 405 requires both
tvdbIdandhasAnyEpisodesto process a show. However,BaseScanner.processShowacceptstvdbIdas optional. Shows that only resolve via TMDB (without TVDB ID) will be skipped even when they have episodes.The condition
else if (!hasAnyEpisodes)at Line 411 only clears the link when there are no episodes, leaving shows with episodes but no TVDB ID in limbo.♻️ Suggested change
- if (tvdbId && hasAnyEpisodes) { + if (hasAnyEpisodes) { await this.processShow(mediaIds.tmdbId, tvdbId, processableSeasons, { mediaAddedAt: new Date(metadata.addedAt * 1000), ratingKey: ratingKey, title: metadata.title, }); - } else if (!hasAnyEpisodes) { + } else { await this.clearMediaServerLink({ ratingKey }); }🤖 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 405 - 413, The current check requires tvdbId to call processShow, which skips shows that have episodes but no TVDB mapping; change the logic so that if hasAnyEpisodes is true you always call this.processShow(mediaIds.tmdbId, tvdbId, processableSeasons, {...}) even if tvdbId is undefined, and only call this.clearMediaServerLink({ ratingKey }) when hasAnyEpisodes is false. Update the conditional around tvdbId/hasAnyEpisodes near where processShow and clearMediaServerLink are called so processShow is invoked for shows with episodes (using tvdbId optionally) and clearing the link only occurs when there are no episodes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@server/lib/availabilitySync.ts`:
- Around line 986-987: The code calls getSettings() repeatedly during a sync run
(e.g., when computing hasIgnorePatterns) which can lead to mixed behavior if
settings change mid-run; capture a run-scoped snapshot instead: at the start of
the availability sync (the main sync routine that calls evaluateSeason/season
evaluation code), read getSettings().main.ignoredPathPatterns once into a const
(e.g., ignoredPathPatternsSnapshot or hasIgnorePatternsSnapshot) and replace all
in-run calls to getSettings().main.ignoredPathPatterns / hasIgnorePatterns with
that snapshot (also update the occurrences around the other mentioned region,
lines referencing getSettings() at 1150-1151) so the entire sync run uses a
stable policy.
In `@server/lib/scanners/plex/index.ts`:
- Around line 405-413: The current check requires tvdbId to call processShow,
which skips shows that have episodes but no TVDB mapping; change the logic so
that if hasAnyEpisodes is true you always call this.processShow(mediaIds.tmdbId,
tvdbId, processableSeasons, {...}) even if tvdbId is undefined, and only call
this.clearMediaServerLink({ ratingKey }) when hasAnyEpisodes is false. Update
the conditional around tvdbId/hasAnyEpisodes near where processShow and
clearMediaServerLink are called so processShow is invoked for shows with
episodes (using tvdbId optionally) and clearing the link only occurs when there
are no episodes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cea75729-707b-4825-b607-c4fc6773dc19
📒 Files selected for processing (10)
server/api/plexapi.tsserver/lib/availabilitySync.tsserver/lib/scanners/baseScanner.tsserver/lib/scanners/jellyfin/index.tsserver/lib/scanners/plex/index.tsserver/lib/settings/index.tsserver/routes/settings/index.tsserver/utils/mediaFilter.tssrc/components/Settings/SettingsMain/index.tsxsrc/i18n/locale/en.json
🚧 Files skipped from review as they are similar to previous changes (1)
- src/i18n/locale/en.json
✅ Actions performedReviews resumed. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/routes/settings/index.ts`:
- Around line 79-80: Destructuring req.body can throw if it's null or not an
object; before extracting ignoredPathPatterns and rest, ensure req.body is a
plain object (e.g., if typeof req.body !== "object" || req.body === null then
treat it as {}), then destructure from that safe object and merge into
settings.main using merge(settings.main, rest); update the code paths around
settings.main, ignoredPathPatterns, and any validation logic to operate on the
guarded object so invalid payloads yield a 4xx validation response instead of a
500.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d386ba7f-b435-49d6-82fe-53514c6c14cd
📒 Files selected for processing (1)
server/routes/settings/index.ts
…and episode-level filtering
…ilability sync ignore checks
…etJellyfinFilePaths
…n availability sync
…id race condition
…alformed values persisting
186d3fa to
5aca7eb
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 998-1005: The current logic in seasonExistsInPlex() and
seasonExistsInJellyfin() treats an empty episodes array as "available" (it
returns true when episodes is empty or undefined), which is wrong — an empty
successful fetch means the season is missing; change the check so that only
undefined/null (fetch failure) triggers the fail-open behavior in the catch,
while an empty array returns false. Concretely, replace the guard that does `if
(!episodes?.length) return true;` with logic that returns false when episodes is
an empty array (e.g., if episodes is defined and episodes.length === 0 return
false), and keep the catch block returning true for actual fetch errors; apply
this same change in both seasonExistsInPlex and seasonExistsInJellyfin.
In `@server/lib/scanners/jellyfin/index.ts`:
- Around line 129-136: The current ignore check uses
getJellyfinFilePaths(metadata.MediaSources) and isPathIgnored on a single path
which can be the placeholder episode for AniDB mixed-library movies; instead,
change the logic to only skip when all candidate episode paths are ignored or to
pick the first non-ignored episode path: gather all candidate file paths for the
movie (use extractMovieIds / the episodes returned for series-backed AniDB
movies or iterate all paths from getJellyfinFilePaths), run isPathIgnored
against each, and only call this.log(...) and await this.clearMediaServerLink({
jellyfinMediaId: metadata.Id }) and return if every candidate path is ignored;
otherwise proceed using the first non-ignored episode path. Ensure you update
references where jfFilePaths is used so it now represents the chosen non-ignored
path or the full list for the “all ignored” check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c52f1224-b686-4ad8-b767-85f26ceefd6d
📒 Files selected for processing (10)
server/api/plexapi.tsserver/lib/availabilitySync.tsserver/lib/scanners/baseScanner.tsserver/lib/scanners/jellyfin/index.tsserver/lib/scanners/plex/index.tsserver/lib/settings/index.tsserver/routes/settings/index.tsserver/utils/mediaFilter.tssrc/components/Settings/SettingsMain/index.tsxsrc/i18n/locale/en.json
🚧 Files skipped from review as they are similar to previous changes (4)
- server/lib/settings/index.ts
- src/i18n/locale/en.json
- src/components/Settings/SettingsMain/index.tsx
- server/lib/scanners/baseScanner.ts
186d3fa to
5aca7eb
Compare
Description
Adds configurable content filtering in General settings, allowing users to exclude media from library scans and availability checks using regex patterns on file paths. Supports both Plex and Jellyfin.
File path regex filtering:
UI:
Goal:
I hoped this approach would simplify the feature and make it general enough to support additional use cases.
Related
How Has This Been Tested?
Built and published to DockerHub
rubeanie/seerr:ignore-media-regex. Self-tested with Plex, open to community testing on Jellyfin (no local Jellyfin instance available for testing).Screenshots / Logs (if applicable)
Default:

Example:

Checklist:
pnpm buildpnpm i18n:extractSummary by CodeRabbit
New Features
Bug Fixes