From 81f2106631f0c9f4b572b06857bab96997f712ff Mon Sep 17 00:00:00 2001 From: kikootwo Date: Tue, 20 Oct 2026 14:46:58 -0400 Subject: [PATCH 1/2] fix: wait for SABnzbd post-processing before completing a download SABnzbd keeps a finished download in its queue at 100% while post-processing (verify, repair, unpack, move) still runs. RMAB mapped that queue state to completed, but the final storage path only exists once the item reaches history, so the monitor failed the request with "Download path not available from download client". A queue item at 100% now maps to extracting, so monitoring continues until history reports the item completed with its storage path. Re-lands #306 by @connor2k. Co-authored-by: connor2k Co-Authored-By: Claude Opus 5.5 --- documentation/phase3/sabnzbd.md | 4 +- src/lib/integrations/sabnzbd.service.ts | 5 +- tests/integrations/sabnzbd.service.test.ts | 62 +++++++++++++++++++++- 3 files changed, 67 insertions(+), 4 deletions(-) diff --git a/documentation/phase3/sabnzbd.md b/documentation/phase3/sabnzbd.md index 9b17f304..2d8ff887 100644 --- a/documentation/phase3/sabnzbd.md +++ b/documentation/phase3/sabnzbd.md @@ -181,7 +181,8 @@ interface SABnzbdConfig { - `Paused` → Manually paused - `Extracting/Unpacking` → Post-processing extraction - `Repairing/Verifying` → Post-processing par2 repair -- `Completed` → Successfully downloaded and extracted +- Queue item at 100% (any other status) → `extracting` (unified `processing`); not completed until it reaches history +- `Completed` (history) → Successfully downloaded and extracted; `storage` gives the download path - `Failed` → Download or post-processing failed ## Remote Path Mapping @@ -241,6 +242,7 @@ organizePath = PathMapper.transform(sabPath, config) - Applying `reverseTransform()` when setting category path (outgoing) - Applying `transform()` when processing completed downloads (incoming) - Using same `PathMapper` utility as qBittorrent for consistency +**8. Requests failed at 100% during post-processing** - A queue item at 100% mapped to `completed` before SABnzbd had a storage path, so the monitor failed the request with "Download path not available". Fixed by mapping it to `extracting` in `getDownloadProgress()`; completion comes from history only. ## Automatic Cleanup diff --git a/src/lib/integrations/sabnzbd.service.ts b/src/lib/integrations/sabnzbd.service.ts index 40df08b2..912661c5 100644 --- a/src/lib/integrations/sabnzbd.service.ts +++ b/src/lib/integrations/sabnzbd.service.ts @@ -927,7 +927,10 @@ export class SABnzbdService implements IDownloadClient { } else if (statusLower.includes('repairing') || statusLower.includes('verifying')) { state = 'repairing'; } else if (percent >= 1.0) { - state = 'completed'; + // 100% in the queue only means the download finished. Post-processing may + // still be running, and the final storage path exists only once SABnzbd + // moves the item to history (which reports completion). + state = 'extracting'; } return { diff --git a/tests/integrations/sabnzbd.service.test.ts b/tests/integrations/sabnzbd.service.test.ts index ba385db7..08bf3834 100644 --- a/tests/integrations/sabnzbd.service.test.ts +++ b/tests/integrations/sabnzbd.service.test.ts @@ -380,7 +380,7 @@ describe('SABnzbdService', () => { expect(extracting.state).toBe('extracting'); }); - it('maps completed status when percentage is 100', () => { + it('maps a 100 percent queue item to extracting, not completed', () => { const service = new SABnzbdService('http://sab', 'key'); const progress = service.getDownloadProgress({ nzbId: 'nzb-7', @@ -394,10 +394,68 @@ describe('SABnzbdService', () => { priority: 'Normal', }); - expect(progress.state).toBe('completed'); + expect(progress.state).toBe('extracting'); expect(progress.percent).toBe(1); }); + it('reports completed only once the item reaches history with its storage path', async () => { + // SAB keeps a finished download in the queue at 100% while post-processing runs; + // the final path only exists once the item moves to history. + let inHistory = false; + clientMock.get.mockImplementation(async (_url: string, config: { params: { mode: string } }) => { + if (config.params.mode === 'queue') { + return { + data: { + queue: { + slots: inHistory ? [] : [{ + nzo_id: 'nzb-pp', + filename: 'Post Processing Book', + mb: '100', + mbleft: '0', + percentage: '100', + status: 'Downloading', + timeleft: '0:00:00', + cat: 'readmeabook', + }], + }, + }, + }; + } + if (config.params.mode === 'history') { + return { + data: { + history: { + slots: inHistory ? [{ + nzo_id: 'nzb-pp', + name: 'Post Processing Book', + category: 'readmeabook', + status: 'Completed', + bytes: '104857600', + storage: '/downloads/readmeabook/Post Processing Book', + completed: '1700000000', + }] : [], + }, + }, + }; + } + throw new Error(`Unexpected SABnzbd mode: ${config.params.mode}`); + }); + + const service = new SABnzbdService('http://sab', 'key'); + + // Still in the queue: keep monitoring, there is no path to organize yet. + const postProcessing = await service.getDownload('nzb-pp'); + expect(postProcessing).toMatchObject({ status: 'processing', progress: 1 }); + expect(postProcessing?.downloadPath).toBeUndefined(); + + inHistory = true; + const done = await service.getDownload('nzb-pp'); + expect(done).toMatchObject({ + status: 'completed', + downloadPath: '/downloads/readmeabook/Post Processing Book', + }); + }); + it('creates the default category when missing', async () => { clientMock.get .mockResolvedValueOnce({ From bd5f2710b87fa30aa4186d472b0ca8b52800ea74 Mon Sep 17 00:00:00 2001 From: kikootwo Date: Tue, 20 Oct 2026 15:54:37 -0400 Subject: [PATCH 2/2] fix: keep Retry from re-adding a SABnzbd download that is still processing A 100% SABnzbd queue item now reports processing instead of completed, and admin Retry did not count processing as alive, so it re-added the NZB and SABnzbd got a duplicate. Retry now resumes monitoring for any processing download. Also corrects the post-processing wording (100% in the queue is the tail of file assembly; post-processing is reported in history) and adds the history post-processing step to the lifecycle test: Extracting and Moving keep polling, only Completed completes. Co-authored-by: connor2k Co-Authored-By: Claude Opus 5.5 --- documentation/phase3/sabnzbd.md | 7 +-- .../requests/[id]/retry-download/route.ts | 1 + src/lib/integrations/sabnzbd.service.ts | 7 +-- tests/api/retry-download-guard.routes.test.ts | 20 +++++++- tests/integrations/sabnzbd.service.test.ts | 46 +++++++++++-------- 5 files changed, 55 insertions(+), 26 deletions(-) diff --git a/documentation/phase3/sabnzbd.md b/documentation/phase3/sabnzbd.md index 2d8ff887..8f5eec85 100644 --- a/documentation/phase3/sabnzbd.md +++ b/documentation/phase3/sabnzbd.md @@ -167,7 +167,7 @@ interface SABnzbdConfig { **Queue vs History:** - **Queue:** Active downloads (downloading, queued, paused) -- **History:** Completed or failed downloads +- **History:** Post-processing (Verifying, Repairing, Extracting, Moving, Running), then Completed or Failed **Monitoring Flow:** 1. Check queue for NZB ID @@ -181,7 +181,8 @@ interface SABnzbdConfig { - `Paused` → Manually paused - `Extracting/Unpacking` → Post-processing extraction - `Repairing/Verifying` → Post-processing par2 repair -- Queue item at 100% (any other status) → `extracting` (unified `processing`); not completed until it reaches history +- Queue item at 100% (any other status) → `extracting` (unified `processing`): the tail of file assembly (and direct unpack), not completion +- History post-processing statuses → not completed; monitoring keeps polling - `Completed` (history) → Successfully downloaded and extracted; `storage` gives the download path - `Failed` → Download or post-processing failed @@ -242,7 +243,7 @@ organizePath = PathMapper.transform(sabPath, config) - Applying `reverseTransform()` when setting category path (outgoing) - Applying `transform()` when processing completed downloads (incoming) - Using same `PathMapper` utility as qBittorrent for consistency -**8. Requests failed at 100% during post-processing** - A queue item at 100% mapped to `completed` before SABnzbd had a storage path, so the monitor failed the request with "Download path not available". Fixed by mapping it to `extracting` in `getDownloadProgress()`; completion comes from history only. +**8. Requests failed at 100% before post-processing finished** - A queue item at 100% (the tail of file assembly, before SABnzbd moves the job to history for post-processing) mapped to `completed` with no storage path, so the monitor failed the request with "Download path not available". Fixed by mapping it to `extracting` in `getDownloadProgress()`; only a `Completed` history slot completes. Admin Retry treats `processing` as alive (`ALIVE_STATUSES` in `retry-download/route.ts`), so it resumes monitoring instead of re-adding the NZB. ## Automatic Cleanup diff --git a/src/app/api/admin/requests/[id]/retry-download/route.ts b/src/app/api/admin/requests/[id]/retry-download/route.ts index 78ddcb1d..8ee66d8b 100644 --- a/src/app/api/admin/requests/[id]/retry-download/route.ts +++ b/src/app/api/admin/requests/[id]/retry-download/route.ts @@ -26,6 +26,7 @@ const ALIVE_STATUSES = new Set([ 'queued', 'paused', 'checking', + 'processing', // e.g. a 100% SABnzbd item still finishing; re-adding would duplicate it 'seeding', 'completed', ]); diff --git a/src/lib/integrations/sabnzbd.service.ts b/src/lib/integrations/sabnzbd.service.ts index 912661c5..7d043f85 100644 --- a/src/lib/integrations/sabnzbd.service.ts +++ b/src/lib/integrations/sabnzbd.service.ts @@ -927,9 +927,10 @@ export class SABnzbdService implements IDownloadClient { } else if (statusLower.includes('repairing') || statusLower.includes('verifying')) { state = 'repairing'; } else if (percent >= 1.0) { - // 100% in the queue only means the download finished. Post-processing may - // still be running, and the final storage path exists only once SABnzbd - // moves the item to history (which reports completion). + // 100% in the queue is the tail of file assembly (and direct unpack). SABnzbd + // then moves the job to history for post-processing (Verifying, Repairing, + // Extracting, Moving, Running); only a Completed history slot has the final + // storage path, so this is not completion yet. state = 'extracting'; } diff --git a/tests/api/retry-download-guard.routes.test.ts b/tests/api/retry-download-guard.routes.test.ts index de142245..3c173920 100644 --- a/tests/api/retry-download-guard.routes.test.ts +++ b/tests/api/retry-download-guard.routes.test.ts @@ -3,7 +3,8 @@ * Documentation: documentation/phase3/download-clients.md * * The stored link is only checked when the download is re-added; resuming - * monitoring of a live torrent never uses it. + * monitoring of a live torrent never uses it. A download the client still + * reports as processing counts as live. */ import { beforeEach, describe, expect, it, vi } from 'vitest'; @@ -51,6 +52,23 @@ describe('POST /api/admin/requests/[id]/retry-download', () => { expect(jobQueueMock.addDownloadJob).not.toHaveBeenCalled(); }); + it('resumes monitoring a SABnzbd download that is still processing instead of re-adding it', async () => { + // A 100% SABnzbd queue item reports processing; re-adding it would give SABnzbd a duplicate NZB. + prismaMock.downloadHistory.findFirst.mockResolvedValue({ + id: 'dh-2', magnetLink: 'http://prowlarr-new:9696/1/download?link=abc', downloadClient: 'sabnzbd', + downloadClientId: 'SABnzbd_nzo_1', torrentName: 'Release', indexerName: 'Indexer', indexerId: 1, + torrentSizeBytes: null, seeders: null, leechers: null, torrentHash: null, torrentUrl: null, + }); + clientMock.getDownload.mockResolvedValue({ status: 'processing', progress: 1 }); + const { POST } = await import('@/app/api/admin/requests/[id]/retry-download/route'); + const response = await POST({} as any, { params: Promise.resolve({ id: 'req-1' }) }); + + expect(response.status).toBe(200); + expect(await response.json()).toMatchObject({ path: 'resumed_monitoring' }); + expect(jobQueueMock.addMonitorJob).toHaveBeenCalledWith('req-1', 'dh-2', 'SABnzbd_nzo_1', 'sabnzbd', 0); + expect(jobQueueMock.addDownloadJob).not.toHaveBeenCalled(); + }); + it('rejects re-adding a stored link that is not a Prowlarr download link or magnet', async () => { clientMock.getDownload.mockResolvedValue(null); const { POST } = await import('@/app/api/admin/requests/[id]/retry-download/route'); diff --git a/tests/integrations/sabnzbd.service.test.ts b/tests/integrations/sabnzbd.service.test.ts index 08bf3834..f95b63fc 100644 --- a/tests/integrations/sabnzbd.service.test.ts +++ b/tests/integrations/sabnzbd.service.test.ts @@ -398,16 +398,18 @@ describe('SABnzbdService', () => { expect(progress.percent).toBe(1); }); - it('reports completed only once the item reaches history with its storage path', async () => { - // SAB keeps a finished download in the queue at 100% while post-processing runs; - // the final path only exists once the item moves to history. - let inHistory = false; + it('reports completed only once history says Completed, with its storage path', async () => { + // 100% in the queue is the tail of file assembly. SAB then moves the job to history, + // where post-processing (Verifying, Repairing, Extracting, Moving, Running) is reported; + // only a Completed history slot is done and has the final path. + const finalPath = '/downloads/readmeabook/Post Processing Book'; + let historyStatus: string | null = null; // null: still in the queue clientMock.get.mockImplementation(async (_url: string, config: { params: { mode: string } }) => { if (config.params.mode === 'queue') { return { data: { queue: { - slots: inHistory ? [] : [{ + slots: historyStatus !== null ? [] : [{ nzo_id: 'nzb-pp', filename: 'Post Processing Book', mb: '100', @@ -425,15 +427,16 @@ describe('SABnzbdService', () => { return { data: { history: { - slots: inHistory ? [{ + slots: historyStatus === null ? [] : [{ nzo_id: 'nzb-pp', name: 'Post Processing Book', category: 'readmeabook', - status: 'Completed', + status: historyStatus, bytes: '104857600', - storage: '/downloads/readmeabook/Post Processing Book', - completed: '1700000000', - }] : [], + // Even a known path must not complete the job before SAB says Completed. + storage: historyStatus === 'Extracting' ? '' : finalPath, + completed: historyStatus === 'Completed' ? '1700000000' : '0', + }], }, }, }; @@ -443,17 +446,22 @@ describe('SABnzbdService', () => { const service = new SABnzbdService('http://sab', 'key'); - // Still in the queue: keep monitoring, there is no path to organize yet. - const postProcessing = await service.getDownload('nzb-pp'); - expect(postProcessing).toMatchObject({ status: 'processing', progress: 1 }); - expect(postProcessing?.downloadPath).toBeUndefined(); + // Still in the queue at 100%: keep monitoring, there is no path to organize yet. + const assembling = await service.getDownload('nzb-pp'); + expect(assembling).toMatchObject({ status: 'processing', progress: 1 }); + expect(assembling?.downloadPath).toBeUndefined(); - inHistory = true; + // Post-processing in history: neither completed nor failed, so the monitor keeps polling. + for (const status of ['Extracting', 'Moving']) { + historyStatus = status; + const postProcessing = await service.getDownload('nzb-pp'); + expect(postProcessing, status).not.toBeNull(); + expect(['completed', 'failed'], status).not.toContain(postProcessing?.status); + } + + historyStatus = 'Completed'; const done = await service.getDownload('nzb-pp'); - expect(done).toMatchObject({ - status: 'completed', - downloadPath: '/downloads/readmeabook/Post Processing Book', - }); + expect(done).toMatchObject({ status: 'completed', downloadPath: finalPath }); }); it('creates the default category when missing', async () => {