fix(requests): mark requests as completed when media is already available

[gauthier-th]

Description

Fixes requests getting stuck and not appearing under the "available" filter when media is already available.

sendToRadarr and sendToSonarr marked requests as APPROVED when media was already available when this should be COMPLETED. This caused the retry path to leave requests stuck since the "available" filter requires both COMPLETED and AVAILABLE.

In addition, MediaSubscriber.updateRelatedMediaRequest only queried APPROVED requests when transitioning to COMPLETED, so FAILED requests were never picked up when media became available via library scan. This is now updated to include FAILED requests in the completion query.

afterUpdate and afterInsert also had a single try/catch, so a failure in sendTo*arr would skip updateParentStatus entirely. These are now separated so updateParentStatus runs regardless.

• Fixes After a failed attempt, if the media is available on the Jellyfin server and show available on Jellyseerr, the filter available is not working #2409

How Has This Been Tested?

Not tested. Check #2409.

Checklist:

• I have read and followed the contribution guidelines.
• Disclosed any use of AI (see our policy)
• I have updated the documentation accordingly.
• All new and existing tests passed.
• Successful build pnpm build
• Translation keys pnpm i18n:extract
• Database migration (if required)

Summary by CodeRabbit

• Refactor

  • Improved error handling and logging during media request processing by separating send-to-external-service operations from parent-status updates to ensure clearer failure visibility.

• Behavioral Change

  • Some requests that previously became APPROVED now become COMPLETED.
  • Completion now propagates to related items (including seasons) and considers a broader set of related statuses when determining completion.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d4a5a6f and 3bd3bb8.

📒 Files selected for processing (1)

• server/subscriber/MediaRequestSubscriber.ts

---

📝 Walkthrough

Walkthrough

Split sending-to-Radarr/Sonarr from parent-status updates in subscriber hooks, added separate try/catch logging for each path, changed request completion to unconditionally set COMPLETED (and propagate to TV seasons), and widened related-request selection to include FAILED alongside APPROVED.

Changes

Cohort / File(s)	Summary
Media request subscriber server/subscriber/MediaRequestSubscriber.ts	Separated send-to-*arr and parent-status update into distinct try/catch blocks with dedicated error logs; changed logic to set request status to COMPLETED unconditionally when media exists and propagate COMPLETED to associated TV seasons; adjusted logging text from "APPROVED" to "COMPLETED".
Related-media selection server/subscriber/MediaSubscriber.ts	Imported In from TypeORM and expanded related MediaRequest filtering to In([MediaRequestStatus.APPROVED, MediaRequestStatus.FAILED]), broadening which requests are considered for completion updates.

Sequence Diagram(s)

/* AfterInsert / AfterUpdate high-level flow */

sequenceDiagram
    participant Subscriber as Subscriber
    participant Arr as Radarr/Sonarr
    participant DB as Database
    participant Notifier as Notifier

    Subscriber->>Arr: sendToRadarr/sendToSonarr
    alt send succeeded
        Subscriber->>DB: updateParentStatus (set COMPLETED / propagate to seasons)
        DB->>Notifier: notifyAvailableMovie/notifyAvailableSeries (if applicable)
    else send failed
        Arr-->>Subscriber: error (logged)
        Subscriber->>DB: set conflicting items to COMPLETED where applicable
        DB->>Notifier: (notifications may still run if applicable)
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through hooks with nimble feet,
Split catches neat so logs can meet,
When *arr stumbles or parents sway,
Each path now speaks its own clear way,
Carrots, code, and completion sweet. 🥕

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main fix: marking requests as completed when media is already available, which directly addresses the root cause of the issue.
Linked Issues check	✅ Passed	The PR fully implements the coding requirements from issue #2409: marking failed/stuck requests as COMPLETED when media is available, including FAILED requests in status transition queries, and separating error handling to ensure parent status updates regardless of sendTo*arr failures.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the request completion flow and status query logic as required by issue #2409; no unrelated modifications are present.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

This PR fixes a bug where media requests would remain in FAILED status even after the associated media becomes AVAILABLE on Jellyfin. The fix separates error handling for sending requests to *arr services from updating parent status, ensuring that the parent status update executes even when *arr services fail.

Changes:

• Split try-catch blocks in afterUpdate and afterInsert methods to isolate *arr service errors from parent status updates
• Improved error logging with specific messages for each operation (sending to *arr vs. updating parent status)

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[gauthier-th]

Preview tag: preview-test-retry

[fallenbagel]

@gauthier-th is this ready for review?

[gauthier-th]

@gauthier-th is this ready for review?

Technically it is, except that it didn't have the desired effect (see #2409 (comment)).

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

server/subscriber/MediaRequestSubscriber.ts (1)

351-355: Update stale log text to match the new status transition.

The message says “marking request as APPROVED”, but the code now sets COMPLETED. Updating logs will avoid confusion during incident/debug review.

Also applies to: 527-531

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/subscriber/MediaRequestSubscriber.ts` around lines 351 - 355, Update
the stale log message text in MediaRequestSubscriber where logger.warn currently
says "Media already exists, marking request as APPROVED" to reflect the new
status transition to COMPLETED; locate the logger.warn calls (referenced around
the handling of entity and entity.media in MediaRequestSubscriber) and change
the message to something like "Media already exists, marking request as
COMPLETED" (do the same for the second occurrence later in the file that mirrors
this logic).

🤖 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/subscriber/MediaRequestSubscriber.ts`:
- Around line 533-535: When the subscriber sets entity.status =
MediaRequestStatus.COMPLETED and saves it in MediaRequestSubscriber, also update
any child season requests so they don't remain stale; after saving the parent
use the same requestRepository (getRepository(MediaRequest)) to either load
children (where parentId === entity.id and type === 'SEASON') and set their
status to MediaRequestStatus.COMPLETED and save them, or run a single update
query via createQueryBuilder().update().set({ status:
MediaRequestStatus.COMPLETED }).where({ parentId: entity.id, type: /* season
type constant */ }); this mirrors updateParentStatus behavior for APPROVED but
ensures COMPLETED parents propagate to season rows.

---

Nitpick comments:
In `@server/subscriber/MediaRequestSubscriber.ts`:
- Around line 351-355: Update the stale log message text in
MediaRequestSubscriber where logger.warn currently says "Media already exists,
marking request as APPROVED" to reflect the new status transition to COMPLETED;
locate the logger.warn calls (referenced around the handling of entity and
entity.media in MediaRequestSubscriber) and change the message to something like
"Media already exists, marking request as COMPLETED" (do the same for the second
occurrence later in the file that mirrors this logic).

---

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between de1bf4c and 8b5ade9.

📒 Files selected for processing (2)

• server/subscriber/MediaRequestSubscriber.ts
• server/subscriber/MediaSubscriber.ts

[coderabbitai]

♻️ Duplicate comments (1)

server/subscriber/MediaRequestSubscriber.ts (1)

533-535: ⚠️ Potential issue | 🟠 Major

Propagate COMPLETED to child season requests when completing TV parent.

Setting only the parent request to COMPLETED can leave child SeasonRequest rows stale (e.g., PENDING/APPROVED), causing inconsistent state.

Proposed fix

           const requestRepository = getRepository(MediaRequest);
+          const seasonRequestRepository = getRepository(SeasonRequest);
           entity.status = MediaRequestStatus.COMPLETED;
+          entity.seasons.forEach((season) => {
+            season.status = MediaRequestStatus.COMPLETED;
+          });
+          await seasonRequestRepository.save(entity.seasons);
           await requestRepository.save(entity);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/subscriber/MediaRequestSubscriber.ts` around lines 533 - 535, When the
parent MediaRequest is marked as MediaRequestStatus.COMPLETED in
MediaRequestSubscriber, also update any child SeasonRequest rows to COMPLETED to
avoid stale states; locate the spot where entity.status =
MediaRequestStatus.COMPLETED is set and, after saving the parent via
getRepository(MediaRequest).save(entity), load the SeasonRequest repository
(getRepository(SeasonRequest)), query for child season requests linked to this
parent (by parent id/reference on SeasonRequest), set their status =
MediaRequestStatus.COMPLETED, and save them (or bulk update) so child rows
reflect the parent's COMPLETED state.

🧹 Nitpick comments (2)

server/subscriber/MediaRequestSubscriber.ts (2)

955-974: Separate notification errors from parent-status errors in afterUpdate.

The current catch logs only parent-status failure, but this block also sends notifications. A notification exception will be misclassified.

Proposed structure

     try {
       await this.updateParentStatus(event.entity as MediaRequest);
+    } catch (e) {
+      logger.error(
+        'Error while updating parent status in afterUpdate subscriber',
+        {
+          label: 'Media Request',
+          requestId: (event.entity as MediaRequest).id,
+          errorMessage: e instanceof Error ? e.message : String(e),
+        }
+      );
+    }

-      if (event.entity.status === MediaRequestStatus.COMPLETED) {
+    try {
+      if (event.entity.status === MediaRequestStatus.COMPLETED) {
         if (event.entity.media.mediaType === MediaType.MOVIE) {
           await this.notifyAvailableMovie(event.entity as MediaRequest, event);
         }
         if (event.entity.media.mediaType === MediaType.TV) {
           await this.notifyAvailableSeries(event.entity as MediaRequest, event);
         }
       }
     } catch (e) {
-      logger.error(
-        'Error while updating parent status in afterUpdate subscriber',
-        {
-          label: 'Media Request',
-          requestId: (event.entity as MediaRequest).id,
-          errorMessage: e instanceof Error ? e.message : String(e),
-        }
-      );
+      logger.error('Error while sending availability notification in afterUpdate subscriber', {
+        label: 'Media Request',
+        requestId: (event.entity as MediaRequest).id,
+        errorMessage: e instanceof Error ? e.message : String(e),
+      });
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/subscriber/MediaRequestSubscriber.ts` around lines 955 - 974, The
catch currently conflates failures from updateParentStatus and the notification
calls; split the logic so updateParentStatus(event.entity) is awaited in its own
try/catch and the notification calls (notifyAvailableMovie and
notifyAvailableSeries) are wrapped in a separate try/catch inside the
afterUpdate subscriber, logging distinct messages and metadata for parent-status
errors vs notification errors (use the same request id from event.entity as
MediaRequest.id and include e.message or String(e) accordingly) so notification
exceptions are not misclassified as parent-status failures.

---

348-359: Update log text to match the new COMPLETED transition.

Line 351 and Line 527 still log “marking request as APPROVED”, but the code now sets MediaRequestStatus.COMPLETED. This will confuse troubleshooting.

Proposed log-message fix

- logger.warn('Media already exists, marking request as APPROVED', {
+ logger.warn('Media already exists, marking request as COMPLETED', {

Also applies to: 524-535

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@server/subscriber/MediaRequestSubscriber.ts` around lines 348 - 359, Update
the log message that currently says "marking request as APPROVED" to say
"marking request as COMPLETED" in MediaRequestSubscriber where you set
entity.status = MediaRequestStatus.COMPLETED (the logger.warn call that includes
label: 'Media Request', requestId: entity.id, mediaId: entity.media.id); make
the same change at the other occurrence in the same class/handler (the second
logger.warn around the code that saves the request via
getRepository(MediaRequest)) so the logs match the COMPLETED transition.

🤖 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/subscriber/MediaRequestSubscriber.ts`:
- Around line 533-535: When the parent MediaRequest is marked as
MediaRequestStatus.COMPLETED in MediaRequestSubscriber, also update any child
SeasonRequest rows to COMPLETED to avoid stale states; locate the spot where
entity.status = MediaRequestStatus.COMPLETED is set and, after saving the parent
via getRepository(MediaRequest).save(entity), load the SeasonRequest repository
(getRepository(SeasonRequest)), query for child season requests linked to this
parent (by parent id/reference on SeasonRequest), set their status =
MediaRequestStatus.COMPLETED, and save them (or bulk update) so child rows
reflect the parent's COMPLETED state.

---

Nitpick comments:
In `@server/subscriber/MediaRequestSubscriber.ts`:
- Around line 955-974: The catch currently conflates failures from
updateParentStatus and the notification calls; split the logic so
updateParentStatus(event.entity) is awaited in its own try/catch and the
notification calls (notifyAvailableMovie and notifyAvailableSeries) are wrapped
in a separate try/catch inside the afterUpdate subscriber, logging distinct
messages and metadata for parent-status errors vs notification errors (use the
same request id from event.entity as MediaRequest.id and include e.message or
String(e) accordingly) so notification exceptions are not misclassified as
parent-status failures.
- Around line 348-359: Update the log message that currently says "marking
request as APPROVED" to say "marking request as COMPLETED" in
MediaRequestSubscriber where you set entity.status =
MediaRequestStatus.COMPLETED (the logger.warn call that includes label: 'Media
Request', requestId: entity.id, mediaId: entity.media.id); make the same change
at the other occurrence in the same class/handler (the second logger.warn around
the code that saves the request via getRepository(MediaRequest)) so the logs
match the COMPLETED transition.

---

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8b5ade9 and e16abbf.

📒 Files selected for processing (2)

• server/subscriber/MediaRequestSubscriber.ts
• server/subscriber/MediaSubscriber.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• server/subscriber/MediaSubscriber.ts