Repository navigation
fix(downloader): apply the global remap to sent save paths and lowercase Deluge labels (#2665) - #3006
Conversation
…ase Deluge labels (#2665) The save path sent to qBittorrent (no category) and rTorrent only ran Bindery's download folder back through the client's own PathRemap, so with just BINDERY_DOWNLOAD_PATH_REMAP set the client was told to save into Bindery's container folder. torrentSavePath now goes through ClientSidePath, the inverse of RemapClientPath with the same precedence the importer, health check and rTorrent removal use: client remap first, the global remap when that leaves the path unchanged. The queue grab and the scheduler pass the global remap in, and the diagnose action's warning about the old behaviour is gone because the case no longer exists. Deluge's Label plugin stores labels lowercased but set_torrent and get_options compare the id exactly, so a category typed as "Books" was rejected as Unknown Label and the error discarded. Bindery now sends deluge.LabelID (lowercase) when labelling and when reading label options, logs a WARN when Deluge still refuses a label, and the diagnose category check lists Deluge's labels and compares them in that form. Deluge downloads are matched by hash, never by label, so nothing else changes. Signed-off-by: vavallee <vavallee@protonmail.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9
…map-deluge-label Signed-off-by: vavallee <vavallee@protonmail.com>
…emap on send (#2665) On the way out the global remap always matches, because its right side is Bindery's own folder, so a BINDERY_DOWNLOAD_PATH_REMAP written for one client (SABnzbd sees /downloads as /data) also rewrote what an rTorrent or category-less qBittorrent mounting /downloads directly was sent. The natural opt-out, an identity client remap "/downloads:/downloads", did not work: ClientSidePath fell back to the global remap whenever the client remap left the path unchanged, which cannot tell "matched and mapped to itself" from "no rule matched". pathmap gains ApplyMatched and ApplyInverseMatched, which also report whether a rule matched; Apply and ApplyInverse delegate to them unchanged. ClientSidePath branches on the match. RemapClientPath (the read side) is left as it was: there the global remap's left side is a client path and does not match a client mounting Bindery's folder directly. Diagnose lost its "global remap not applied when sending" warning with the first commit. It now warns instead when the global remap produced the folder Bindery sends and the client's own default save folder (qBittorrent default save path, rTorrent directory.default) is neither that folder nor inside or above it, and the fix names the identity remap. A folder from the client's own remap is not second guessed. The qBittorrent category health check also resolves the category save path with the global remap as fallback, the importer's precedence. Docs and the changelog fragment describe the identity opt-out and carry an upgrade note for users with the global remap and rTorrent or a category-less qBittorrent. Signed-off-by: vavallee <vavallee@protonmail.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Clean fix. No blocking issues.
Correctness
The round-trip invariant (ClientSidePath → RemapClientPath → original path) is correct for all four cases: client remap matches, client remap misses, identity rule opts out of global, no remap set. pathcheck.go:ClientSidePath nil-receiver on global is safe because ApplyInverseMatched guards on r == nil. Tests cover all these branches explicitly.
The ApplyMatched/ApplyInverseMatched refactor in remap.go is backward-compatible: Apply and ApplyInverse now delegate and the identity-rule case (a rule that matches without changing the path) is correctly distinguished from "no rule matched".
Deluge label path
LabelID = strings.ToLower. setLabel now passes that instead of the raw category; errors log at WARN rather than being swallowed. DownloadLocation correctly drops the early-return-on-capitals block and proceeds with the lowercased id. CheckCategories now calls Labels() (plugin off → Checked=false, existing "turn plugin on" advice) and compares lowercased wanted vs. reported.
Minor observation (not a blocker)
GrabSavePath (clientpath.go ~L135, ~L148) now calls DefaultDirectory (rTorrent) and GetDefaultSavePath (qBittorrent) even when a sent path is already known — one extra round-trip per Diagnose call per such client. This is diagnostic-path-only and the result is used only for noteAgainstDefault, so it's not a correctness issue, but worth noting if Diagnose latency becomes a concern.
Wiring
Both call sites that construct SendOptions — QueueHandler.grab (queue.go:1431) and Scheduler.searchAndGrabFormat (scheduler.go:1248) — now pass GlobalRemap. The health-check path (health.go:1187) uses remapClientPath with the same precedence. Consistent.
Tests
TestTorrentSavePath_RemapPrecedence, TestSendDownload_Qbittorrent/RtorrentGlobalRemapOnly, TestQueueGrab_SavePathUsesGlobalRemap, TestSearchAndGrabFormat_SavePathUsesGlobalRemap, TestAddTorrent_CapitalCategoryIsLabelled, TestAddTorrent_UnknownLabelWarns, TestDiagnose_DelugeLabelCategory, TestDiagnose_GlobalRemapAppliesToSentSavePath — all exercise the new logic and are written to fail before the fix. Coverage looks complete.
Docs/changelog: DEPLOYMENT.md upgrade note and identity-remap escape hatch are clear. API.md diagnose notes updated correctly. Changelog fragment present.
— 🤖 Bindery triage bot (automated). Reply to correct me; a human will see it.
The sent folder warning fired whenever the client's default save folder was unrelated to the folder Bindery sends, which covers plenty of correct setups: a dedicated Bindery folder next to the client default, rTorrent's split incomplete and complete folders, and relative or tilde defaults (stock rTorrent ships with "./"). Its suggested identity remap would have broken those. It now warns only on the signature of a global remap meant for another client: the global remap produced the sent folder, but the client's default is an absolute path on Bindery's own folder rather than on the sent one. Non absolute defaults are not checked. The note says to ignore it if the client really keeps that storage at the remapped path, the one false positive left. Signed-off-by: vavallee <vavallee@protonmail.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9
1 participant
Summary
Two ways a grab silently landed somewhere other than where Bindery expected. The save path sent to qBittorrent (no category) and rTorrent ignored
BINDERY_DOWNLOAD_PATH_REMAP, so with only the global remap set the client was told to save into Bindery's own/downloads. And a Deluge category with capitals (Books) was rejected by the Label plugin as "Unknown Label", with the error discarded, so the torrent stayed unlabelled and a label move path never applied.Closes #2665.
Implementation notes
downloader.ClientSidePathinpathcheck.go, the inverse of the existingRemapClientPath(which the importer, health check, diagnose and rTorrent removal already share). ClientPathRemapfirst, global remap only when no client rule matched. "Matched" comes from newpathmap.ApplyInverseMatched/ApplyMatched(ApplyandApplyInversedelegate, unchanged), so an identity client remap/downloads:/downloadsopts a client out of a global remap meant for others. Windows drive and UNC sides work as before.SendOptions.GlobalRemap;torrentSavePathgoes throughClientSidePath. Manual grabs (QueueHandler) and automatic grabs (Scheduler) pass the global remap they already held for rTorrent removal.GrabSavePathtakes the global remap so the doctor reports the folder actually sent. The oldremap: warn("global remap not applied when sending") is gone. Insteadclient_pathwarns only on the mixed setup signature: the global remap produced the sent folder, but the client's default save folder (qBittorrent default save path, rTorrentdirectory.default) is an absolute path on Bindery's own folder rather than the sent one. The fix names the identity remap; relative and tilde defaults are not checked.deluge.LabelID(lowercase) is used forlabel.set_torrentandlabel.get_options. A failedset_torrentis logged at WARN with the hash, label and Deluge's error, matching how the other non fatal per grab failures (seed ratio) are surfaced; the grab still succeeds.CheckCategoriesnow lists labels vialabel.get_labelsand compares the lowercased categories, so Diagnose fails on a missing label and notes when it lowercased one. With the plugin off it staysunknownwith the existing "turn on the Label plugin" advice. Theclient_pathwarning about capitals is gone because they now work.Other places that send a path to a client, checked for the same omission:
download-diris onlyclient.Categorywhen it is an absolute path the operator typed in the client's own namespace. Bindery's folder is never sent, so no remap belongs there.core.add_torrent_magnet/add_torrent_filewith empty options).Bindery never creates Deluge labels (there is no
label.addcall), so "lowercase when ensuring it exists" does not apply. Deluge downloads are matched by info hash, never by label, so existing torrents are unaffected.Checklist
docs/DEPLOYMENT.mdpath remapping section,docs/API.mddiagnose notes, godoc on new symbols)Test plan
Fail before evidence (tests run against main's implementation):
TestTorrentSavePath_RemapPrecedence: global only, global under the prefix, client remap that misses, Windows drive and UNC global targets all sent/downloads...; client only, both set (client wins) and neither set passed already.TestSendDownload_QbittorrentGlobalRemapOnly:savepath = "/downloads", want/data.TestSendDownload_RtorrentGlobalRemapOnly:d.directory.set="/downloads".TestQueueGrab_SavePathUsesGlobalRemapandTestSearchAndGrabFormat_SavePathUsesGlobalRemap: each fails with the one wiring line removed.TestAddTorrent_CapitalCategoryIsLabelled(fake Deluge RPC that mimics the Label plugin's exact matchset_torrent): label"", wantbooks.TestAddTorrent_UnknownLabelWarns: no WARN logged.TestTorrentSavePath_RemapPrecedenceidentity rows,TestSendDownload_RtorrentMixedSetup): fail with the old "path unchanged" check, sending/data.TestApplyMatchedReportsIdentityRules(pathmap),TestClientPathsOverlap,TestQbittorrentCategoryPath_GlobalRemapOnly, diagnose mixed setup and identity subtests.gofmt -lclean,go vet ./...golangci-lint runv2.11.4: 0 issuesgo test -timeout=15m ./cmd/... ./internal/...go test -race ./internal/downloader/..., plus-raceon theTestDiagnose|TestQueueGrab(api) andTestSearchAndGrab(scheduler) subsets (the unsharded packages time out under race, make test: internal/api race package hits the 30-minute timeout #2293)cd web && npm run build(web untouched)🤖 Generated with Claude Code
https://claude.ai/code/session_016fJcCVbNnKsmj2MAAMwWj9