fix(downloader): apply the global remap to sent save paths and lowercase Deluge labels (#2665)

changelog.d/2665-save-path-remap-deluge-label.md

@@ -0,0 +1,4 @@
+### Fixed
+- **The global path remap now applies to the save path Bindery sends** (#2665). With only `BINDERY_DOWNLOAD_PATH_REMAP` set, rTorrent and qBittorrent (when the client has no category) were told to save into Bindery's own folder, such as `/downloads`, which may not exist on the client's side. The save path now goes through the same remaps Bindery reads paths back with: the client's own path remap first, then the global one. The qBittorrent category health check now honours the global remap too.
+- **Upgrade note for `BINDERY_DOWNLOAD_PATH_REMAP` users with rTorrent, or qBittorrent without a category** (#2665). The folder Bindery sends those clients changes to the global remap's client side. Run **Diagnose** on each such client after upgrading. If the client mounts Bindery's download folder at the same path, because the global remap is meant for a different client, give it an identity path remap such as `/downloads:/downloads` so it keeps being sent Bindery's own folder.
+- **Deluge categories with capital letters now label the torrent** (#2665). Deluge stores labels in lowercase and rejects any other spelling, so a category typed as `Books` left grabs unlabelled and any move path on the label was skipped. Bindery now sends the label lowercased, logs a warning when Deluge still refuses a label (for example when the Label plugin is off or the label does not exist), and Diagnose checks your Deluge categories against the labels Deluge has.

docs/API.md

@@ -601,9 +601,10 @@ only, never a path, and it runs only when called.
 
 * `code` is stable: `config`, `connect`, `category`, `client_path`, `remap`, `local_path`, `hardlinks`, `indexer_reach`. `message` and `fix` are English sentences.
 * `status` is `pass`, `warn`, `fail`, `skipped` or `unknown`. Every check after a `fail` is `skipped` without running, except `indexer_reach`, which is always `unknown` because Bindery cannot test the client's own route to indexers.
-* The folder checked is where a grab actually lands, worked out the way the grab itself is sent: the save path Bindery sends (rTorrent always, qBittorrent without a category, Transmission with an absolute category), the category save path (qBittorrent, with an empty one meaning the default save path plus the category name), the category folder (SABnzbd), the category DestDir or DestDir plus the category name when `AppendCategoryDir` is on (NZBGet), a Deluge label's move completed path, or the client default. `source` names which. The category is used exactly as a grab sends it: a category with spaces around it, or a Deluge category with capital letters (the Label plugin only accepts lowercase labels, so such grabs are not labelled), gives `client_path: warn`.
+* The folder checked is where a grab actually lands, worked out the way the grab itself is sent: the save path Bindery sends (rTorrent always, qBittorrent without a category, Transmission with an absolute category), the category save path (qBittorrent, with an empty one meaning the default save path plus the category name), the category folder (SABnzbd), the category DestDir or DestDir plus the category name when `AppendCategoryDir` is on (NZBGet), a Deluge label's move completed path, or the client default. `source` names which. The category is used exactly as a grab sends it: a category with spaces around it gives `client_path: warn`. A Deluge category is sent lowercased, because the Label plugin stores labels in lowercase and only accepts that form when labelling a torrent, so `Books` uses the `books` label's move completed path.
+* For Deluge, `category` compares the configured categories, lowercased, with the labels the Label plugin lists, and says when it lowercases one. When the plugin is off it cannot list labels and `category` is `unknown`.
 * Ebook and audiobook grabs are checked separately, because each resolves its own category and download folder. When both land in the same folder there is one `paths` row and the `client_path`, `remap` and `local_path` checks have no `mediaType`; otherwise each carries `mediaType` `ebook` or `audiobook`.
-* `remapRule` is `client` (this client's path remap changed the path), `global` (`BINDERY_DOWNLOAD_PATH_REMAP` did) or `none`. A Windows drive path fails for a missing remap only when Bindery itself is not running on Windows. When no remap applies and the client's folder is not under any folder Bindery uses, `remap` is `warn` and `local_path` carries the failure and the fix. When Bindery sends the save path itself (rTorrent, qBittorrent without a category) and only the global remap is set, `remap` is `warn`: sending does not apply the global remap, so the client is given Bindery's own folder path and the round trip proves nothing.
+* `remapRule` is `client` (this client's path remap changed the path), `global` (`BINDERY_DOWNLOAD_PATH_REMAP` did) or `none`. A Windows drive path fails for a missing remap only when Bindery itself is not running on Windows. When no remap applies and the client's folder is not under any folder Bindery uses, `remap` is `warn` and `local_path` carries the failure and the fix. When Bindery sends the save path itself (rTorrent, qBittorrent without a category) it runs its download folder through the same remaps in reverse, the client's own first and `BINDERY_DOWNLOAD_PATH_REMAP` as the fallback, so `clientPath` is the folder the client is actually given. A client remap rule that covers the folder wins even when it maps the folder to itself (`/downloads:/downloads`), which is how a client opts out of a global remap meant for others. When the global remap produced the sent folder but the client's own default save folder is an absolute path on Bindery's own folder (that folder, inside it or above it) rather than on the sent one, `client_path` is `warn` and the fix names that identity remap. A relative default such as rTorrent's stock `./` is not checked.
 * `hardlinks` has one row per download folder and library root pair, `{mediaType, downloadPath, root, result, linkable, reason}`. Every pair gets a real link probe, because two bind mounts of one filesystem share a device ID yet refuse links across them. `result` is `yes`, `no`, `unknown` (Bindery could not write a test file in the download folder) or `missing` (the library folder does not exist and was not probed).
 * `primaryFix` is the fix of the first failure, or of the first warning when nothing failed.
 

docs/DEPLOYMENT.md

@@ -377,6 +377,10 @@ When Bindery and your download client run in **separate containers**, they typic
 
 Set a download-client path remap in **Settings → Download clients** or set the global `BINDERY_DOWNLOAD_PATH_REMAP` fallback to a comma-separated list of `from:to` pairs. Bindery applies a longest-prefix match to every path the download client reports, replacing the matched prefix before it tries to access the file. A per-client remap takes precedence when it matches the reported path; the global env var still applies as a fallback.
 
+The same remaps also run in reverse, with the same precedence, when Bindery tells a client where to save a grab. rTorrent always gets a download directory, and qBittorrent gets a save path when the client has no category set. That directory is Bindery's download folder (`BINDERY_DOWNLOAD_DIR`, or `BINDERY_AUDIOBOOK_DOWNLOAD_DIR` for audiobooks) translated into the client's path: by the client's own remap when it covers that folder, otherwise by `BINDERY_DOWNLOAD_PATH_REMAP`. With `BINDERY_DOWNLOAD_PATH_REMAP=/data:/downloads` and no client remap, rTorrent is told to save into `/data`. Bindery 1.40.0 and earlier applied only the client's own remap here, so with just the global remap set the client was sent Bindery's `/downloads`.
+
+On the way out the global remap matches every client, because its right side is Bindery's own folder. If the global remap is written for one client (say SABnzbd sees Bindery's `/downloads` as `/data`) and an rTorrent or category-less qBittorrent mounts Bindery's folder at the same path, give that client an identity path remap, `/downloads:/downloads`. A client remap that covers the folder always wins, even when it maps it to itself, so that client keeps being sent `/downloads`. **Diagnose** on the client warns when the global remap produced the folder it sends while the client's own default save folder sits on Bindery's path, which is what that setup looks like. If the client really keeps that storage at the remapped path, ignore the warning.
+
 Per-client remaps are stored on each download client, so separate qBittorrent / SABnzbd / NZBGet instances can map different mount points. Existing download clients keep an empty remap after upgrade, which preserves the previous global-only behavior until you add a client-specific value.
 
 For a per-client remap, open **Settings → Download clients**, edit the client, and set **Download client path remap**. To see what a remap actually resolves to, press **Diagnose** on the client: it reports where that client says ebook and audiobook grabs land, which remap applies, and whether Bindery can read the result. The left side is the path the client reports; the right side is the path Bindery can read. For qBittorrent this normally means mapping the qBittorrent category save path or torrent content path to Bindery's download mount. Example: if qBittorrent reports `/downloads/books/My.Book` and Bindery sees that same folder as `/media/books/My.Book`, set `/downloads:/media/books`.

internal/api/download_client_diagnose.go

@@ -10,12 +10,14 @@ import (
 	"os"
 	"path/filepath"
 	"runtime"
+	"slices"
 	"sort"
 	"strings"
 	"time"
 
 	"github.com/vavallee/bindery/internal/config"
 	"github.com/vavallee/bindery/internal/downloader"
+	"github.com/vavallee/bindery/internal/downloader/deluge"
 	"github.com/vavallee/bindery/internal/httpsec"
 	"github.com/vavallee/bindery/internal/models"
 	"github.com/vavallee/bindery/internal/pathmap"
@@ -104,10 +106,9 @@ type diagnoseResponse struct {
 // diagTarget is one folder grabs land in: ebook, audiobook, or both when
 // they resolve to the same client folder.
 type diagTarget struct {
-	row           diagPathRow
-	sentByBindery bool   // the client path is the save path Bindery sends
-	probePath     string // symlink-resolved local path, set once readable
-	stopped       bool   // an earlier per folder check had nothing to pass on
+	row       diagPathRow
+	probePath string // symlink-resolved local path, set once readable
+	stopped   bool   // an earlier per folder check had nothing to pass on
 }
 
 // diagState is what the checks share. Each check reads what earlier checks
@@ -429,7 +430,20 @@ func checkDiagCategory(ctx context.Context, st *diagState) []diagCheckResult {
 			Fix:     fmt.Sprintf("Create the category %s in %s, or change this client's category in Bindery to one that exists.", quoteJoin(report.Missing), st.clientName),
 		})
 	}
-	return one(diagCheckResult{Status: diagPass, Message: fmt.Sprintf("%s has the category %s.", st.clientName, quoteJoin(report.Wanted))})
+	res := diagCheckResult{Status: diagPass, Message: fmt.Sprintf("%s has the category %s.", st.clientName, quoteJoin(report.Wanted))}
+	if st.client.Type == "deluge" {
+		// Grabs lowercase the label (#2665), so a capital is not a problem,
+		// but say so, because the label in Deluge will not look like the
+		// category typed here.
+		var noted []string
+		for _, c := range []string{st.client.Category, st.client.CategoryAudiobook} {
+			if id := deluge.LabelID(c); id != c && !slices.Contains(noted, c) {
+				noted = append(noted, c)
+				res.Message += fmt.Sprintf(" Deluge keeps labels in lowercase, so Bindery sends the category %q as %q.", c, id)
+			}
+		}
+	}
+	return one(res)
 }
 
 // checkDiagClientPath works out where ebook and audiobook grabs land, using
@@ -440,7 +454,7 @@ func checkDiagClientPath(ctx context.Context, st *diagState) []diagCheckResult {
 		return downloader.ResolveCategory(st.client, mediaType) + "\x00" +
 			downloader.TargetDownloadDir(mediaType, st.downloadDir, st.audiobookDownloadDir)
 	}
-	ebook, ebookErr := downloader.GrabSavePath(ctx, st.client, models.MediaTypeEbook, st.downloadDir, st.audiobookDownloadDir)
+	ebook, ebookErr := downloader.GrabSavePath(ctx, st.client, models.MediaTypeEbook, st.downloadDir, st.audiobookDownloadDir, st.globalRemap)
 	type resolved struct {
 		mediaType string
 		info      downloader.ClientPathInfo
@@ -450,7 +464,7 @@ func checkDiagClientPath(ctx context.Context, st *diagState) []diagCheckResult {
 	if grabKey(models.MediaTypeEbook) == grabKey(models.MediaTypeAudiobook) {
 		found = []resolved{{"", ebook, ebookErr}}
 	} else {
-		audio, audioErr := downloader.GrabSavePath(ctx, st.client, models.MediaTypeAudiobook, st.downloadDir, st.audiobookDownloadDir)
+		audio, audioErr := downloader.GrabSavePath(ctx, st.client, models.MediaTypeAudiobook, st.downloadDir, st.audiobookDownloadDir, st.globalRemap)
 		if ebookErr == nil && audioErr == nil && ebook.Path == audio.Path && ebook.Source == audio.Source && ebook.Note == audio.Note {
 			found = []resolved{{"", ebook, nil}}
 		} else {
@@ -460,7 +474,7 @@ func checkDiagClientPath(ctx context.Context, st *diagState) []diagCheckResult {
 
 	out := make([]diagCheckResult, 0, len(found))
 	for _, f := range found {
-		t := &diagTarget{row: diagPathRow{MediaType: f.mediaType, ClientPath: f.info.Path, Source: f.info.Source}, sentByBindery: f.info.SentByBindery}
+		t := &diagTarget{row: diagPathRow{MediaType: f.mediaType, ClientPath: f.info.Path, Source: f.info.Source}}
 		st.targets = append(st.targets, t)
 		res := describeClientPath(st, f.mediaType, f.info, f.err)
 		res.MediaType = f.mediaType
@@ -570,16 +584,6 @@ func remapTarget(st *diagState, t *diagTarget, global *pathmap.Remapper) diagChe
 			Fix:     "Make both sides of the path remap absolute paths.",
 		}
 	}
-	// torrentSavePath inverts only the client's own remap when sending, so
-	// with only the global remap set the client is sent Bindery's own folder,
-	// and remapping it back proves nothing about the client's side.
-	if t.sentByBindery && strings.TrimSpace(st.client.PathRemap) == "" && !global.Empty() {
-		return diagCheckResult{
-			Status:  diagWarn,
-			Message: fmt.Sprintf("Bindery sends its own folder %q to %s, because BINDERY_DOWNLOAD_PATH_REMAP is not applied when sending. This check can only confirm Bindery reads its own folder, not that %s writes there.", raw, st.clientName, st.clientName),
-			Fix:     fmt.Sprintf("Set a path remap on this client, so the folder Bindery sends is translated into the path %s uses for the same storage.", st.clientName),
-		}
-	}
 	switch rule {
 	case downloader.RemapRuleClient:
 		return diagCheckResult{Status: diagPass, Message: fmt.Sprintf("This client's path remap turns %q into %q.", raw, local)}