feat(scanner): add regex-based ignored path patterns for media scanner filtering - #2606
Open
Rubeanie wants to merge 6 commits into
Open
feat(scanner): add regex-based ignored path patterns for media scanner filtering#2606Rubeanie wants to merge 6 commits into
Rubeanie wants to merge 6 commits into
Conversation
7 tasks
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughAdds configurable ignored-path regex support across settings, media filtering, Plex/Jellyfin scanning, availability checks, API routes, frontend configuration, documentation, and tests. Plex media typings now expose optional ChangesIgnored-paths filtering feature
Estimated code review effort: 4 (Complex) | ~60 minutes Merge Risk: ⚪ Minimal · up to The PR adds configurable case-insensitive path-pattern filtering that can exclude matching media from scans and availability checks. No actionable merge-blocking risk remains; it is merge-ready after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant Admin
participant SettingsAPI
participant MediaScanner
participant MediaFilter
participant AvailabilitySync
Admin->>SettingsAPI: submit ignored path patterns
SettingsAPI->>SettingsAPI: sanitize and save patterns
MediaScanner->>MediaFilter: provide Plex or Jellyfin media paths
MediaFilter-->>MediaScanner: return unignored media
AvailabilitySync->>MediaFilter: filter availability media
MediaFilter-->>AvailabilitySync: return unignored media
AvailabilitySync->>AvailabilitySync: determine availability and 4K status
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
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.
Contributor
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
Contributor
✅ Actions performedReviews resumed. |
Contributor
|
✅ Actions performedReviews resumed. |
Contributor
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.
Contributor
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.
Author
|
Apologies for the activity, I believe all the found issues are now resolved and I don't see any significant remaining improvements |
Contributor
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
Contributor
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).
Rubeanie
force-pushed
the
ignore-media-regex
branch
from
March 5, 2026 03:38
304b160 to
d256e75
Compare
Contributor
|
✅ Actions performedReview triggered.
|
Contributor
|
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.
|
Contributor
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
Contributor
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
Contributor
✅ Actions performedReviews resumed. |
Contributor
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
Rubeanie
force-pushed
the
ignore-media-regex
branch
from
March 7, 2026 06:04
186d3fa to
5aca7eb
Compare
Contributor
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
Rubeanie
force-pushed
the
ignore-media-regex
branch
3 times, most recently
from
March 11, 2026 02:19
5aca7eb to
ce045f8
Compare
Contributor
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 (2)
server/lib/scanners/plex/index.ts (1)
229-242:⚠️ Potential issue | 🟠 MajorIgnore Plex paths at
Mediagranularity, not whole-item granularity.These branches collapse the entire
Mediaarray into one ignore verdict. For mixed-version items, that breaks availability: an ignored placeholder source can hide a valid version outright, or still leak into the later standard/4K checks. FilterMediaentries first, then skip/clear only when no non-ignored source remains.Also applies to: 256-267, 363-379, 596-606, 622-625
🤖 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 229 - 242, The current logic calls getPlexFilePaths and isPathIgnored on the entire plexitem.Media array which collapses multiple Media sources into a single ignore decision; instead, filter plexitem.Media to remove ignored entries first (use getPlexFilePaths per media entry and isPathIgnored per path) and then proceed: if the filtered Media array is empty then log and call this.clearMediaServerLink({ ratingKey: plexitem.ratingKey }) and return, otherwise compute mediaIds from the filtered Media (update getMediaIds usage) and compute has4k by checking videoResolution on the filtered Media array; apply the same per-Media filtering pattern to the other affected blocks (lines referenced around 256-267, 363-379, 596-606, 622-625) so all decisions use the non-ignored Media entries only.server/lib/scanners/jellyfin/index.ts (1)
129-152:⚠️ Potential issue | 🟠 MajorIgnore Jellyfin paths at
MediaSourcesgranularity, not whole-item granularity.This has the same mixed-version problem as Plex: the code makes one ignore decision from all source paths, then later derives standard/4K availability from those same unfiltered sources. A placeholder source under an ignored path can either suppress a usable file or still count toward availability. Filter
MediaSourcesfirst and only skip when none remain.Also applies to: 329-369
🤖 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 - 152, The current logic uses getJellyfinFilePaths/isPathIgnored on the whole item and then computes has4k/hasOtherResolution from unfiltered metadata.MediaSources, which can let ignored-source placeholders affect availability; instead first filter metadata.MediaSources to a new variable (e.g., filteredMediaSources) by running getJellyfinFilePaths/isPathIgnored per MediaSource path and keeping only non-ignored sources, use filteredMediaSources when computing has4k and hasOtherResolution, and only call clearMediaServerLink({ jellyfinMediaId: metadata.Id }) and return when filteredMediaSources is empty; update references to MediaSources in the has4k/hasOtherResolution checks to use the filtered variable (and ensure null/undefined checks mirror existing optional chaining).
♻️ Duplicate comments (1)
server/lib/scanners/baseScanner.ts (1)
610-632:⚠️ Potential issue | 🟠 MajorMake the stale-link clear conditional on the current DB value.
update({ id: media.id }, patch)can still wipe a freshly-writtenratingKey/jellyfinMediaIdif another scanner updates the row after Line 610. Keep the partial update, but gate each field with the stale value you matched so only that exact link is cleared.Suggested fix
- 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; + + if (patch.ratingKey === null) { + const result = await mediaRepository + .createQueryBuilder() + .update(Media) + .set({ ratingKey: null }) + .where('id = :id AND ratingKey = :ratingKey', { + id: media.id, + ratingKey: options.ratingKey, + }) + .execute(); + cleared = cleared || (result.affected ?? 0) > 0; + } + + if (patch.ratingKey4k === null) { + const result = await mediaRepository + .createQueryBuilder() + .update(Media) + .set({ ratingKey4k: null }) + .where('id = :id AND ratingKey4k = :ratingKey', { + id: media.id, + ratingKey: options.ratingKey, + }) + .execute(); + cleared = cleared || (result.affected ?? 0) > 0; + } + + if (patch.jellyfinMediaId === null) { + const result = await mediaRepository + .createQueryBuilder() + .update(Media) + .set({ jellyfinMediaId: null }) + .where('id = :id AND jellyfinMediaId = :jellyfinMediaId', { + id: media.id, + jellyfinMediaId: options.jellyfinMediaId, + }) + .execute(); + cleared = cleared || (result.affected ?? 0) > 0; + } + + if (patch.jellyfinMediaId4k === null) { + const result = await mediaRepository + .createQueryBuilder() + .update(Media) + .set({ jellyfinMediaId4k: null }) + .where('id = :id AND jellyfinMediaId4k = :jellyfinMediaId', { + id: media.id, + jellyfinMediaId: options.jellyfinMediaId, + }) + .execute(); + cleared = cleared || (result.affected ?? 0) > 0; + } + + if (cleared) { + 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 unconditional mediaRepository.update({ id: media.id }, patch) can overwrite newly written links; instead perform guarded updates that include the exact stale DB value in the WHERE so only the matching stale field is cleared. For each field you planned to clear (ratingKey, ratingKey4k, jellyfinMediaId, jellyfinMediaId4k) call mediaRepository.update with a WHERE that includes id plus the matched current value (e.g. id + ratingKey === options.ratingKey) and set that single field to null, or execute separate conditional updates per field using the repository/query-builder so you never clear a field unless it still equals the stale value you detected in media.ratingKey/media.ratingKey4k/media.jellyfinMediaId/media.jellyfinMediaId4k.
🤖 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 986-1005: The season-level availability check currently treats any
non-ignored episode as keeping the season alive, which ignores resolution;
update the logic in availabilitySync.ts where episodes are retrieved via
this.plexClient?.getChildrenMetadata and examined (the episodes.some(...) block)
to only consider episodes whose Media entries meet the 4K resolution guard used
by the show-level pass (i.e., replicate the same Media.width check used there),
then apply getPlexFilePaths(episode.Media ?? []) and isPathIgnored only for
those qualifying high-resolution Media items so seasons are preserved only when
4K files exist.
---
Outside diff comments:
In `@server/lib/scanners/jellyfin/index.ts`:
- Around line 129-152: The current logic uses getJellyfinFilePaths/isPathIgnored
on the whole item and then computes has4k/hasOtherResolution from unfiltered
metadata.MediaSources, which can let ignored-source placeholders affect
availability; instead first filter metadata.MediaSources to a new variable
(e.g., filteredMediaSources) by running getJellyfinFilePaths/isPathIgnored per
MediaSource path and keeping only non-ignored sources, use filteredMediaSources
when computing has4k and hasOtherResolution, and only call
clearMediaServerLink({ jellyfinMediaId: metadata.Id }) and return when
filteredMediaSources is empty; update references to MediaSources in the
has4k/hasOtherResolution checks to use the filtered variable (and ensure
null/undefined checks mirror existing optional chaining).
In `@server/lib/scanners/plex/index.ts`:
- Around line 229-242: The current logic calls getPlexFilePaths and
isPathIgnored on the entire plexitem.Media array which collapses multiple Media
sources into a single ignore decision; instead, filter plexitem.Media to remove
ignored entries first (use getPlexFilePaths per media entry and isPathIgnored
per path) and then proceed: if the filtered Media array is empty then log and
call this.clearMediaServerLink({ ratingKey: plexitem.ratingKey }) and return,
otherwise compute mediaIds from the filtered Media (update getMediaIds usage)
and compute has4k by checking videoResolution on the filtered Media array; apply
the same per-Media filtering pattern to the other affected blocks (lines
referenced around 256-267, 363-379, 596-606, 622-625) so all decisions use the
non-ignored Media entries only.
---
Duplicate comments:
In `@server/lib/scanners/baseScanner.ts`:
- Around line 610-632: The current unconditional mediaRepository.update({ id:
media.id }, patch) can overwrite newly written links; instead perform guarded
updates that include the exact stale DB value in the WHERE so only the matching
stale field is cleared. For each field you planned to clear (ratingKey,
ratingKey4k, jellyfinMediaId, jellyfinMediaId4k) call mediaRepository.update
with a WHERE that includes id plus the matched current value (e.g. id +
ratingKey === options.ratingKey) and set that single field to null, or execute
separate conditional updates per field using the repository/query-builder so you
never clear a field unless it still equals the stale value you detected in
media.ratingKey/media.ratingKey4k/media.jellyfinMediaId/media.jellyfinMediaId4k.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7251d8e3-7ca0-4a80-aca6-4ad3c0a9ae3b
📒 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 (2)
- server/lib/settings/index.ts
- src/components/Settings/SettingsMain/index.tsx
Contributor
|
Tip For best results, initiate chat on the files or code changes.
|
|
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
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/utils/ignoredPathPatterns.ts (1)
8-11: 🔒 Security & Privacy | 🔵 Trivial | 🏗️ Heavy liftDo not treat
safeRegexas a complete ReDoS boundary.
safe-regex@2.1.1acceptsRegExpinputs and uses star-height-based heuristics, which can still miss vulnerability classes and polynomial-time ReDoS. The returned nativeregexp.test()is synchronous across media paths, so add stronger containment such as an execution timeout/linear-time engine or strict deterministic length/budget bounds in addition to the current heuristic check.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/utils/ignoredPathPatterns.ts` around lines 8 - 11, Strengthen the pattern validation in the try block around safeRegex so accepted expressions cannot cause unbounded synchronous RegExp.test execution. Retain safeRegex as a preliminary check, then add a concrete containment mechanism such as a timeout/linear-time engine or strict deterministic input-length and execution-budget bounds before returning the native regex; preserve returning null for rejected patterns.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@server/utils/ignoredPathPatterns.ts`:
- Around line 8-11: Strengthen the pattern validation in the try block around
safeRegex so accepted expressions cannot cause unbounded synchronous RegExp.test
execution. Retain safeRegex as a preliminary check, then add a concrete
containment mechanism such as a timeout/linear-time engine or strict
deterministic input-length and execution-budget bounds before returning the
native regex; preserve returning null for rejected patterns.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: aa8d5eb2-90de-4109-9430-913a32c92d30
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
docs/using-seerr/settings/general.mdpackage.jsonserver/api/plexapi.tsserver/lib/availabilitySync.test.tsserver/lib/availabilitySync.tsserver/lib/scanners/jellyfin/index.tsserver/lib/scanners/plex/index.tsserver/lib/settings/index.tsserver/routes/settings/index.tsserver/utils/ignoredPathPatterns.test.tsserver/utils/ignoredPathPatterns.tsserver/utils/mediaFilter.test.tsserver/utils/mediaFilter.tssrc/components/Settings/SettingsMain/index.tsxsrc/i18n/locale/en.json
🚧 Files skipped from review as they are similar to previous changes (13)
- server/routes/settings/index.ts
- server/lib/settings/index.ts
- src/i18n/locale/en.json
- package.json
- server/utils/ignoredPathPatterns.test.ts
- docs/using-seerr/settings/general.md
- server/utils/mediaFilter.test.ts
- server/lib/scanners/jellyfin/index.ts
- src/components/Settings/SettingsMain/index.tsx
- server/utils/mediaFilter.ts
- server/lib/availabilitySync.test.ts
- server/lib/scanners/plex/index.ts
- server/lib/availabilitySync.ts
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new advanced “Ignored Path Patterns” setting that lets admins exclude placeholder/trailer media (and other undesired files) from Plex/Jellyfin scans and availability sync by matching configurable, case-insensitive regexes against normalized file paths. This addresses the “placeholder files make content look available” problem described in #2235 in a media-server-agnostic way.
Changes:
- Added a new General setting (UI + i18n) for managing multiple regex-based ignored path patterns, including validation feedback.
- Implemented shared server-side filtering utilities and applied them across Plex/Jellyfin scanners and availability sync (including 4K and season/episode handling).
- Added tests and documentation for the new filtering behavior, including safety validation for regex patterns.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/i18n/locale/en.json | Adds translation strings for the new “Ignored Path Patterns” setting and validation toast. |
| src/components/Settings/SettingsMain/index.tsx | Adds the multi-tag regex input UI and client-side validation/toasts for ignored patterns. |
| server/utils/mediaFilter.ts | New shared filtering helpers to determine whether paths/media sources should be ignored. |
| server/utils/mediaFilter.test.ts | Tests for path normalization, caching behavior, and mixed ignored/unignored handling. |
| server/utils/ignoredPathPatterns.ts | New regex compilation + sanitization utilities using safe-regex. |
| server/utils/ignoredPathPatterns.test.ts | Tests for validation/sanitization (invalid, unsafe, dedupe, trimming). |
| server/routes/settings/index.ts | Sanitizes/replaces ignoredPathPatterns on save; hides the setting from non-admin responses. |
| server/lib/settings/index.ts | Adds ignoredPathPatterns to main settings interface and defaults. |
| server/lib/scanners/plex/index.ts | Applies ignored-path filtering to Plex scanning and 4K/season logic. |
| server/lib/scanners/jellyfin/index.ts | Applies ignored-path filtering to Jellyfin scanning and 4K/season logic. |
| server/lib/availabilitySync.ts | Applies ignored-path filtering and adds safer season/episode existence validation to avoid stale/false availability. |
| server/lib/availabilitySync.test.ts | Adds coverage for ignored-path handling (movies/seasons, Plex/Jellyfin, 4K) and season-metadata ambiguity cases. |
| server/api/plexapi.ts | Updates Plex media typing to include Part.file for path extraction. |
| package.json | Adds safe-regex and @types/safe-regex dependencies. |
| pnpm-lock.yaml | Locks new dependencies (safe-regex, transitive deps, and typings). |
| docs/using-seerr/settings/general.md | Documents the new setting behavior, normalization rules, and examples. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (2)
server/utils/ignoredPathPatterns.ts:45
- sanitizeIgnoredPathPatterns() can store arbitrarily long rejected values (including over-length strings), which then get logged by the settings route. A malicious client could submit a huge value and cause log/memory amplification. Consider truncating rejected values (and over-length patterns) before adding them to the rejected set.
(compileIgnoredPathPattern(trimmed) ? cleaned : rejected).add(trimmed);
server/utils/mediaFilter.ts:31
- This warning logs the full invalid pattern string. If the settings file is manually edited (or validation is bypassed) with an extremely long pattern, this can spam logs. Consider truncating the pattern (and/or logging it as structured metadata) to keep log volume bounded.
logger.warn(
`Invalid or unsafe regex pattern in ignored path patterns: ${pattern}`,
{ label: 'MediaFilter' }
);
Rubeanie
force-pushed
the
ignore-media-regex
branch
from
August 2, 2026 07:06
3489dc7 to
49b25ff
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (2)
server/utils/mediaFilter.ts:30
- The warning logs include the raw user-provided regex pattern. Since patterns can contain newlines/control characters, this can result in log injection / multi-line log entries. Consider escaping the pattern before logging (e.g., JSON.stringify) so log output stays single-line and unambiguous.
logger.warn(
`Invalid or unsafe regex pattern in ignored path patterns: ${pattern}`,
{ label: 'MediaFilter' }
);
server/routes/settings/index.ts:93
- This warning logs rejected regex patterns directly. Because patterns are user input and may contain newlines/control characters, logging them unescaped can lead to log injection or confusing multi-line entries. Escape each pattern (e.g., JSON.stringify) before joining.
logger.warn(
`Ignored invalid or unsafe regex pattern(s) in ignoredPathPatterns: ${rejected.join(', ')}`,
{ label: 'Settings' }
);
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/utils/ignoredPathPatterns.ts`:
- Around line 3-5: Update the comment above MAX_PATTERN_LENGTH to describe 256
characters only as a pattern-size limit, not a ReDoS or complexity boundary.
Ensure ignored-path matching does not rely on safe-regex and the length cap to
guarantee bounded regex.test(file) performance; use a non-backtracking engine or
stronger validation with worst-case matching coverage.
🪄 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 Plus
Run ID: 6802a8d3-921e-475a-9ae2-e96792c27524
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
docs/using-seerr/settings/general.mdpackage.jsonserver/api/plexapi.tsserver/lib/availabilitySync.test.tsserver/lib/availabilitySync.tsserver/lib/scanners/jellyfin/index.tsserver/lib/scanners/plex/index.tsserver/lib/settings/index.tsserver/routes/settings/index.tsserver/utils/ignoredPathPatterns.test.tsserver/utils/ignoredPathPatterns.tsserver/utils/mediaFilter.test.tsserver/utils/mediaFilter.tssrc/components/Settings/SettingsMain/index.tsxsrc/i18n/locale/en.json
🚧 Files skipped from review as they are similar to previous changes (13)
- server/api/plexapi.ts
- server/routes/settings/index.ts
- docs/using-seerr/settings/general.md
- server/utils/mediaFilter.test.ts
- package.json
- server/utils/ignoredPathPatterns.test.ts
- src/components/Settings/SettingsMain/index.tsx
- server/lib/scanners/jellyfin/index.ts
- server/lib/scanners/plex/index.ts
- src/i18n/locale/en.json
- server/lib/settings/index.ts
- server/lib/availabilitySync.test.ts
- server/lib/availabilitySync.ts
Contributor
✅ Action performedReviews resumed. |
Author
|
@0xSysR3ll Code scanning results / CodeQL failures: All four are the same as previous, "this is intentional, admin supplied regex patterns are the core of this feature". I've done everything I can to keep it secure, but let me know if there's any required changes I haven't resolved the conversations myself, but I'm more than happy to |
Contributor
Yes these are false positive, I'll dismiss them. |
safe-regex is a heuristic, so bound patterns to 256 chars to limit the complexity of anything its star-height check misses. Also make the regex cache generics explicit.
Rubeanie
force-pushed
the
ignore-media-regex
branch
2 times, most recently
from
August 21, 2026 16:03
c33d3f7 to
6f01acf
Compare
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
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
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:
placeholders/,.*trailers/.*)UI:
Goal:
This approach simplifies the feature, doesn't impact availability discovery time and makes it general enough to support additional use cases past just UMTK/Agregarr placeholders.
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).Image currently in use by 15+ confirmed active users, including myself.
Screenshots / Logs (if applicable)
Default:
Example:
Checklist:
pnpm buildpnpm i18n:extractSummary by CodeRabbit
Summary by CodeRabbit