feat(servarr): add removal and tag/season cleanup methods (1/5 removal requests)#3175
Open
MrDWilson wants to merge 1 commit into
Open
feat(servarr): add removal and tag/season cleanup methods (1/5 removal requests)#3175MrDWilson wants to merge 1 commit into
MrDWilson wants to merge 1 commit into
Conversation
Adds idempotent removal helpers to the Radarr and Sonarr API clients: - Radarr: idempotent removeMovie, removeTagFromMovie - Sonarr: idempotent removeSeries, removeTagFromSeries, removeSeasonFiles (unmonitor + delete episode files for selected seasons) These are standalone API additions consumed by the upcoming media removal request feature. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthrough
ChangesServarr Removal Helpers
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
7 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
Adds new Servarr (Radarr/Sonarr) client helpers to support upcoming “media removal request” functionality, including idempotent removals and tag/season cleanup operations.
Changes:
- Make Radarr
removeMovieand SonarrremoveSeriesidempotent when the title is no longer present in the library. - Add Radarr
removeTagFromMovie. - Add Sonarr
removeTagFromSeriesandremoveSeasonFiles(unmonitor + delete episode files for selected seasons).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| server/api/servarr/sonarr.ts | Adds series tag removal and per-season file cleanup; updates series removal to be idempotent. |
| server/api/servarr/radarr.ts | Adds movie tag removal; updates movie removal to be idempotent. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+444
to
+452
| const series = await this.getSeriesByTvdbId(tvdbId); | ||
| if (!series.id) { | ||
| throw new Error('Series not found in Sonarr'); | ||
| } | ||
| const updatedTags = series.tags.filter((t) => t !== tagId); | ||
| await this.axios.put(`/series/${series.id}`, { | ||
| ...series, | ||
| tags: updatedTags, | ||
| }); |
Comment on lines
+500
to
+506
| // Unmonitor the seasons | ||
| series.seasons = series.seasons.map((s) => ({ | ||
| ...s, | ||
| monitored: seasonNumbers.includes(s.seasonNumber) ? false : s.monitored, | ||
| })); | ||
| await this.axios.put(`/series/${series.id}`, series); | ||
|
|
Comment on lines
+303
to
+309
| const movie = await this.getMovieByTmdbId(tmdbId); | ||
| const updatedTags = movie.tags.filter((t) => t !== tagId); | ||
| await this.axios.put(`/movie`, { | ||
| ...movie, | ||
| tags: updatedTags, | ||
| }); | ||
| logger.info(`[Radarr] Removed tag ${tagId} from movie ${movie.title}`); |
![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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
server/api/servarr/sonarr.ts (1)
413-437:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSame idempotency issue: guard won't trigger when series is absent.
getSeriesByTvdbIdthrows'Series not found'when!response.data[0](line 176-178), so the!idguard at line 416 never executes for already-removed series. The exception propagates to the outer catch block.Apply the same fix pattern as suggested for
removeMovie:🔧 Proposed fix
public removeSeries = async (serieId: number): Promise<void> => { try { - const { id, title } = await this.getSeriesByTvdbId(serieId); - if (!id) { - // Series is not in the Sonarr library (e.g. already removed). Treat the - // desired end-state as reached so retries remain idempotent. - logger.info( - '[Sonarr] Series not present in library; nothing to remove', - { tvdbId: serieId } - ); - return; + let series; + try { + series = await this.getSeriesByTvdbId(serieId); + } catch { + logger.info( + '[Sonarr] Series not present in library; nothing to remove', + { tvdbId: serieId } + ); + return; } + if (!series.id) { + logger.info( + '[Sonarr] Series not present in library; nothing to remove', + { tvdbId: serieId } + ); + return; + } + const { id, title } = series; await this.axios.delete(`/series/${id}`, {🤖 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/api/servarr/sonarr.ts` around lines 413 - 437, The removeSeries method attempts to guard against removing non-existent series with an `!id` check, but getSeriesByTvdbId throws an exception when the series is not found instead of returning a falsy value, so the guard never executes. To fix the idempotency issue, wrap the getSeriesByTvdbId call in its own try-catch block that catches the "Series not found" exception and returns early (treating it as a successful no-op), rather than letting the exception propagate to the outer catch block. Apply the same pattern that was used to fix removeMovie.server/api/servarr/radarr.ts (1)
270-296:⚠️ Potential issue | 🟠 Major | ⚡ Quick winIdempotency guard won't trigger when movie is not in library.
The
getMovieByTmdbIdmethod (lines 95-116) throws'Movie not found'when!response.data[0], which means the catch block at line 291-295 will handle that case and rethrow the error. The!idguard at line 273 only executes ifgetMovieByTmdbIdsucceeds but the returned movie lacks anid.For true idempotency (returning successfully when the movie was already removed), you need to catch the "Movie not found" scenario specifically:
🔧 Proposed fix for idempotent handling
public removeMovie = async (movieId: number): Promise<void> => { try { - const { id, title } = await this.getMovieByTmdbId(movieId); - if (!id) { - // Movie is not in the Radarr library (e.g. already removed). Treat the - // desired end-state as reached so retries remain idempotent. - logger.info( - '[Radarr] Movie not present in library; nothing to remove', - { - tmdbId: movieId, - } - ); - return; - } + let movie; + try { + movie = await this.getMovieByTmdbId(movieId); + } catch { + // Movie is not in the Radarr library (e.g. already removed). Treat the + // desired end-state as reached so retries remain idempotent. + logger.info( + '[Radarr] Movie not present in library; nothing to remove', + { tmdbId: movieId } + ); + return; + } + if (!movie.id) { + logger.info( + '[Radarr] Movie not present in library; nothing to remove', + { tmdbId: movieId } + ); + return; + } + const { id, title } = movie; await this.axios.delete(`/movie/${id}`, { params: { deleteFiles: true, addImportExclusion: false, }, }); logger.info(`[Radarr] Removed movie ${title}`); } catch (e) { throw new Error(`[Radarr] Failed to remove movie: ${e.message}`, { cause: e, }); } };🤖 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/api/servarr/radarr.ts` around lines 270 - 296, The idempotency guard at the !id check in the removeMovie method won't trigger when getMovieByTmdbId throws a "Movie not found" error, because that error gets caught by the outer catch block instead. To fix this, wrap the getMovieByTmdbId call in a separate try-catch block that specifically catches the "Movie not found" error scenario and returns early with an info log (similar to the existing !id guard behavior), rather than letting that error propagate to the outer catch block. This ensures removeMovie remains idempotent whether the movie is already removed or was never present in the Radarr library.
🧹 Nitpick comments (1)
server/api/servarr/sonarr.ts (1)
495-498: ⚖️ Poor tradeoffConsider resilience for partial file deletion failures.
If one
DELETE /episodefile/{fileId}call fails (e.g., network hiccup), the loop exits early. Episodes are already unmonitored, but remaining files and the series update (line 505) are skipped, leaving the system partially inconsistent.A more resilient approach would use
Promise.allSettledor continue-on-error with aggregated reporting. That said, a retry of the entire operation should be safe given the idempotent design.🤖 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/api/servarr/sonarr.ts` around lines 495 - 498, The episode file deletion loop using a for loop with await will exit early if any single delete request fails, leaving remaining files undeleteted and skipping the series update. Replace the for loop that iterates over episodeFileIds with Promise.allSettled to send all delete requests concurrently, then handle the results afterward to allow individual failures without stopping the entire operation. This ensures the system remains more consistent by completing all deletion attempts even if some individual requests fail, and you can aggregate any errors for logging or further handling.
🤖 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/api/servarr/sonarr.ts`:
- Around line 462-476: The removeSeasonFiles method has an idempotency issue
where the guard check for series.id never executes because getSeriesByTvdbId
throws an error when the series is not found, rather than returning a series
object without an id. Wrap the call to getSeriesByTvdbId in a nested try-catch
block within the removeSeasonFiles method to catch the error when the series is
absent, log an appropriate message indicating the series is not present in the
library, and return early to treat this as an idempotent success case.
---
Outside diff comments:
In `@server/api/servarr/radarr.ts`:
- Around line 270-296: The idempotency guard at the !id check in the removeMovie
method won't trigger when getMovieByTmdbId throws a "Movie not found" error,
because that error gets caught by the outer catch block instead. To fix this,
wrap the getMovieByTmdbId call in a separate try-catch block that specifically
catches the "Movie not found" error scenario and returns early with an info log
(similar to the existing !id guard behavior), rather than letting that error
propagate to the outer catch block. This ensures removeMovie remains idempotent
whether the movie is already removed or was never present in the Radarr library.
In `@server/api/servarr/sonarr.ts`:
- Around line 413-437: The removeSeries method attempts to guard against
removing non-existent series with an `!id` check, but getSeriesByTvdbId throws
an exception when the series is not found instead of returning a falsy value, so
the guard never executes. To fix the idempotency issue, wrap the
getSeriesByTvdbId call in its own try-catch block that catches the "Series not
found" exception and returns early (treating it as a successful no-op), rather
than letting the exception propagate to the outer catch block. Apply the same
pattern that was used to fix removeMovie.
---
Nitpick comments:
In `@server/api/servarr/sonarr.ts`:
- Around line 495-498: The episode file deletion loop using a for loop with
await will exit early if any single delete request fails, leaving remaining
files undeleteted and skipping the series update. Replace the for loop that
iterates over episodeFileIds with Promise.allSettled to send all delete requests
concurrently, then handle the results afterward to allow individual failures
without stopping the entire operation. This ensures the system remains more
consistent by completing all deletion attempts even if some individual requests
fail, and you can aggregate any errors for logging or further handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c51f5e73-08b6-4069-9e3d-2444d6dab168
📒 Files selected for processing (2)
server/api/servarr/radarr.tsserver/api/servarr/sonarr.ts
Comment on lines
+462
to
+476
| public removeSeasonFiles = async ( | ||
| tvdbId: number, | ||
| seasonNumbers: number[] | ||
| ): Promise<void> => { | ||
| try { | ||
| const series = await this.getSeriesByTvdbId(tvdbId); | ||
| if (!series.id) { | ||
| // Series is not in the Sonarr library (e.g. already removed). Treat the | ||
| // desired end-state as reached so retries remain idempotent. | ||
| logger.info( | ||
| '[Sonarr] Series not present in library; nothing to remove', | ||
| { tvdbId } | ||
| ); | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
Same idempotency issue in removeSeasonFiles.
The !series.id guard at line 468 won't execute when the series is absent because getSeriesByTvdbId throws first. Apply the nested try-catch pattern here as well.
🔧 Proposed fix
public removeSeasonFiles = async (
tvdbId: number,
seasonNumbers: number[]
): Promise<void> => {
try {
- const series = await this.getSeriesByTvdbId(tvdbId);
- if (!series.id) {
- // Series is not in the Sonarr library (e.g. already removed). Treat the
- // desired end-state as reached so retries remain idempotent.
+ let series;
+ try {
+ series = await this.getSeriesByTvdbId(tvdbId);
+ } catch {
logger.info(
'[Sonarr] Series not present in library; nothing to remove',
{ tvdbId }
);
return;
}
+ if (!series.id) {
+ logger.info(
+ '[Sonarr] Series not present in library; nothing to remove',
+ { tvdbId }
+ );
+ return;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public removeSeasonFiles = async ( | |
| tvdbId: number, | |
| seasonNumbers: number[] | |
| ): Promise<void> => { | |
| try { | |
| const series = await this.getSeriesByTvdbId(tvdbId); | |
| if (!series.id) { | |
| // Series is not in the Sonarr library (e.g. already removed). Treat the | |
| // desired end-state as reached so retries remain idempotent. | |
| logger.info( | |
| '[Sonarr] Series not present in library; nothing to remove', | |
| { tvdbId } | |
| ); | |
| return; | |
| } | |
| public removeSeasonFiles = async ( | |
| tvdbId: number, | |
| seasonNumbers: number[] | |
| ): Promise<void> => { | |
| try { | |
| let series; | |
| try { | |
| series = await this.getSeriesByTvdbId(tvdbId); | |
| } catch { | |
| logger.info( | |
| '[Sonarr] Series not present in library; nothing to remove', | |
| { tvdbId } | |
| ); | |
| return; | |
| } | |
| if (!series.id) { | |
| logger.info( | |
| '[Sonarr] Series not present in library; nothing to remove', | |
| { tvdbId } | |
| ); | |
| return; | |
| } |
🤖 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/api/servarr/sonarr.ts` around lines 462 - 476, The removeSeasonFiles
method has an idempotency issue where the guard check for series.id never
executes because getSeriesByTvdbId throws an error when the series is not found,
rather than returning a series object without an id. Wrap the call to
getSeriesByTvdbId in a nested try-catch block within the removeSeasonFiles
method to catch the error when the series is absent, log an appropriate message
indicating the series is not present in the library, and return early to treat
this as an idempotent success case.
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
This is the first of a stacked series that splits the media removal request feature (originally #2828) into independently reviewable PRs, following the same approach as the Lidarr split (#3109).
This PR adds the Servarr connectivity layer only: new removal/cleanup helpers on the Radarr and Sonarr API clients. It is functionally stand-alone — it can be merged on its own with no behavioural change to the app, and is consumed by the later PRs in the series.
removeMovieis now idempotent (no-op when the movie is no longer in the library, so retries are safe); addsremoveTagFromMovie.removeSeriesis now idempotent; addsremoveTagFromSeriesandremoveSeasonFiles(unmonitors the selected seasons/episodes and deletes their episode files).Stacked series (each opened against
developas the previous merges)Tracking: #2828
How Has This Been Tested?
pnpm typecheck(server + client)pnpm lintChecklist:
pnpm buildpnpm i18n:extract— n/a (no UI strings)AI Assistance Notice
The mechanical splitting of the original branch into this stacked series, and drafting of this description, were done with assistance from Claude (Opus 4.8). All code is unchanged from the reviewed-by-author original feature branch.
Summary by CodeRabbit
New Features
Bug Fixes