feat(cli): migrate Deluge, rTorrent and Transmission to qBittorrent with stats#616
feat(cli): migrate Deluge, rTorrent and Transmission to qBittorrent with stats#616
Conversation
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as CLI Command
participant Migr as Migrater
participant Imp as Importer
participant Backup as Backup Manager
participant FS as File System
User->>CLI: migrate <client> --source-dir /src --qbit-dir /dst [--dry-run] [--skip-backup]
CLI->>Migr: New(opts) / Migrate(ctx)
Migr->>Backup: perform backup (unless SkipBackup)
Backup->>FS: archive sourceDir & qbitDir
Migr->>Imp: select importer (deluge|rtorrent|transmission)
Imp->>FS: read .torrent and client resume/state files
Imp->>Imp: decode state, build qbittorrent.Fastresume
alt Dry-Run
Imp->>CLI: log intended writes (no file changes)
else Not Dry-Run
Imp->>FS: write .fastresume files
Imp->>FS: copy .torrent files to qbitDir
end
Imp->>Migr: report summary
Migr->>CLI: display results
CLI->>User: output completion
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| newFastResume.ConvertFilePriority(len(metaInfo.Files)) | ||
|
|
||
| // fill pieces to set as completed | ||
| newFastResume.FillPieces() |
There was a problem hiding this comment.
The rTorrent importer calls FillPieces() with NumPieces still at its zero value (no assignment anywhere before lines 186-189), so the generated fastresume contains an empty pieces bitfield. Migrated torrents will be treated as unchecked in qBittorrent, forcing a full recheck or leaving them inactive instead of resuming seeding despite existing data.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
It's assigned on line 129 on the newFastResume struct.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (3)
go.mod (1)
23-32: Confirm new archive/bencode dependencies are necessary and rungo mod tidy.The added deps (e.g.
github.com/mholt/archives,github.com/zeebo/bencodeand the related compression libs) look aligned with the new migration/backup functionality, but they significantly expand the module graph.Two suggestions:
- Ensure all of these are actually referenced (high‑level: run
go mod tidyand check for removals).- Consider whether you really need both
archivesand separate zip/xz/lz4 stacks, or if some can remain purely transitive to keep the surface smaller.Also applies to: 49-109
internal/qbittorrent/fastresume.go (2)
88-114: Handle buffered writer flush errors and avoid logging from deep inside library code.Two small issues in
Encode:
- You ignore the error from
bufferedWriter.Flush(), which can hide I/O failures even thoughEncodereports success.- Using the std
logpackage here is inconsistent with the rest of the codebase (which uses zerolog) and mixes concerns; callers are usually better placed to decide how to log.Consider:
- bufferedWriter := bufio.NewWriter(file) - enc := bencode.NewEncoder(bufferedWriter) - if err := enc.Encode(fr); err != nil { - log.Printf("encode error: %v", err) - return err - } - - bufferedWriter.Flush() - return nil + bufferedWriter := bufio.NewWriter(file) + enc := bencode.NewEncoder(bufferedWriter) + if err := enc.Encode(fr); err != nil { + return fmt.Errorf("encode fastresume: %w", err) + } + + if err := bufferedWriter.Flush(); err != nil { + return fmt.Errorf("flush fastresume: %w", err) + } + return nil(and let the caller log as needed).
116-140: ClarifyConvertFilePrioritybehavior for single‑file torrents.
ConvertFilePrioritybuildsFilePrioritybased solely onnumFiles. When called withlen(metaInfo.Files), single‑file torrents usually haveFiles == nil, which meansnumFiles == 0andFilePriorityremains empty.Please verify how qBittorrent behaves when
file_priorityis missing for a single‑file torrent. If it expects one entry, you may want:-func (fr *Fastresume) ConvertFilePriority(numFiles int) { +func (fr *Fastresume) ConvertFilePriority(numFiles int) { + if numFiles == 0 { + // Single-file torrent: still have one logical file. + numFiles = 1 + } newPrioList := make([]int, numFiles) for i := 0; i < numFiles; i++ { newPrioList[i] = 1 } fr.FilePriority = newPrioList }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (6)
cmd/qui/main.go(3 hunks)go.mod(5 hunks)internal/clientmigrate/deluge.go(1 hunks)internal/clientmigrate/migrate.go(1 hunks)internal/clientmigrate/rtorrent.go(1 hunks)internal/qbittorrent/fastresume.go(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Audionut
Repo: autobrr/qui PR: 553
File: internal/services/crossseed/service.go:1045-1082
Timestamp: 2025-11-06T12:11:04.963Z
Learning: The autobrr/qui project uses a custom go-qbittorrent client library (github.com/autobrr/go-qbittorrent) that supports both "paused" and "stopped" parameters when adding torrents via the options map. Both parameters should be set together when controlling torrent start state, as seen in internal/services/crossseed/service.go and throughout the codebase.
📚 Learning: 2025-11-06T12:11:04.963Z
Learnt from: Audionut
Repo: autobrr/qui PR: 553
File: internal/services/crossseed/service.go:1045-1082
Timestamp: 2025-11-06T12:11:04.963Z
Learning: The autobrr/qui project uses a custom go-qbittorrent client library (github.com/autobrr/go-qbittorrent) that supports both "paused" and "stopped" parameters when adding torrents via the options map. Both parameters should be set together when controlling torrent start state, as seen in internal/services/crossseed/service.go and throughout the codebase.
Applied to files:
internal/clientmigrate/rtorrent.go
🧬 Code graph analysis (4)
internal/clientmigrate/deluge.go (2)
internal/clientmigrate/migrate.go (4)
Options(16-22)ClientMigrater(24-26)MkDirIfNotExists(154-168)CopyFile(175-212)internal/qbittorrent/fastresume.go (1)
Fastresume(14-86)
internal/clientmigrate/rtorrent.go (2)
internal/clientmigrate/migrate.go (3)
Options(16-22)ClientMigrater(24-26)CopyFile(175-212)internal/qbittorrent/fastresume.go (1)
Fastresume(14-86)
internal/clientmigrate/migrate.go (2)
internal/clientmigrate/rtorrent.go (1)
NewRTorrentImporter(23-25)internal/clientmigrate/deluge.go (1)
NewDelugeImporter(19-21)
cmd/qui/main.go (1)
internal/clientmigrate/migrate.go (2)
New(32-34)Options(16-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests
🔇 Additional comments (7)
internal/clientmigrate/rtorrent.go (7)
1-30: LGTM!The package structure, imports, and type definitions are well organized. The constructor properly returns the
ClientMigraterinterface.
35-56: LGTM!Directory validation and file discovery logic is robust with appropriate error handling and early returns.
220-235: LGTM!The tracker conversion logic correctly filters out DHT pseudo-trackers and disabled trackers, producing the qBittorrent-compatible format.
238-244: LGTM!Filename extraction and normalization logic is correct.
246-272: LGTM!The decode helper functions follow a standard pattern for reading and unmarshaling bencode files.
275-286: LGTM with a note on error handling.The function silently returns
0for invalid input, which is appropriate for a migration utility where some data corruption is tolerable and shouldn't halt processing.
288-302: LGTM!The type definitions correctly map to RTorrent's bencode format with appropriate struct tags.
| // Fix savepath for torrents with subfolder | ||
| // directory contains the whole torrent path, which gives error in qBit. | ||
| // remove file.sourceDirInfo.name from full path in id.rtorrent directory | ||
| newPath := strings.ReplaceAll(rtFile.Directory, metaInfo.Name, "") | ||
|
|
||
| newFastResume.Path = newPath | ||
| newFastResume.SavePath = newPath | ||
| newFastResume.QbtSavePath = newPath |
There was a problem hiding this comment.
Path manipulation is fragile and may break in edge cases.
Using strings.ReplaceAll to remove the torrent name from the directory path can fail if:
- The torrent name appears multiple times in the path
- The name is a substring of a parent directory
Consider using strings.TrimSuffix or filepath.Dir for more robust path handling.
Apply this diff:
- // Fix savepath for torrents with subfolder
- // directory contains the whole torrent path, which gives error in qBit.
- // remove file.sourceDirInfo.name from full path in id.rtorrent directory
- newPath := strings.ReplaceAll(rtFile.Directory, metaInfo.Name, "")
+ // Fix savepath for torrents with subfolder
+ // directory contains the whole torrent path, which gives error in qBit.
+ // remove the torrent name from the end of the path
+ newPath := strings.TrimSuffix(rtFile.Directory, string(filepath.Separator)+metaInfo.Name)
+ if newPath == rtFile.Directory {
+ // If no suffix was removed, try without separator (edge case)
+ newPath = strings.TrimSuffix(rtFile.Directory, metaInfo.Name)
+ }📝 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.
| // Fix savepath for torrents with subfolder | |
| // directory contains the whole torrent path, which gives error in qBit. | |
| // remove file.sourceDirInfo.name from full path in id.rtorrent directory | |
| newPath := strings.ReplaceAll(rtFile.Directory, metaInfo.Name, "") | |
| newFastResume.Path = newPath | |
| newFastResume.SavePath = newPath | |
| newFastResume.QbtSavePath = newPath | |
| // Fix savepath for torrents with subfolder | |
| // directory contains the whole torrent path, which gives error in qBit. | |
| // remove the torrent name from the end of the path | |
| newPath := strings.TrimSuffix(rtFile.Directory, string(filepath.Separator)+metaInfo.Name) | |
| if newPath == rtFile.Directory { | |
| // If no suffix was removed, try without separator (edge case) | |
| newPath = strings.TrimSuffix(rtFile.Directory, metaInfo.Name) | |
| } | |
| newFastResume.Path = newPath | |
| newFastResume.SavePath = newPath | |
| newFastResume.QbtSavePath = newPath |
🤖 Prompt for AI Agents
internal/clientmigrate/rtorrent.go around lines 167 to 174: current code uses
strings.ReplaceAll(rtFile.Directory, metaInfo.Name, "") to strip the torrent
name from the directory which is fragile; instead compute the intended parent
directory robustly (e.g., if rtFile.Directory ends with the torrent name, use
strings.TrimSuffix to remove that trailing segment, or use filepath.Dir on a
constructed path to get the parent) and assign that result to
newFastResume.Path, newFastResume.SavePath and newFastResume.QbtSavePath; ensure
you preserve leading/trailing separators correctly (use filepath.Clean and
filepath.Separator aware functions) and only remove a single trailing occurrence
so names embedded in parent folders are not altered.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
internal/clientmigrate/migrate.go (1)
57-96: Backup directory must be created before writing archives.The
qbt_backupdirectory is never created before callingos.Create()on lines 80 and 112, causing backups to fail withENOENT.Apply this diff:
// Backup data before running if !skipBackup { log.Info().Msg("prepare to backup torrent data before import..") timeStamp := time.Now().Format("20060102150405") + + backupDir := "qbt_backup" + if err := MkDirIfNotExists(backupDir); err != nil { + return errors.Wrap(err, "could not create backup directory") + } - sourceBackupArchive := filepath.Join("qbt_backup", source+"_backup_"+timeStamp+".tar.gz") - qbitBackupArchive := filepath.Join("qbt_backup", "qBittorrent_backup_"+timeStamp+".tar.gz") + sourceBackupArchive := filepath.Join(backupDir, source+"_backup_"+timeStamp+".tar.gz") + qbitBackupArchive := filepath.Join(backupDir, "qBittorrent_backup_"+timeStamp+".tar.gz")Consider also anchoring
backupDirto a more predictable location (e.g., nearqbitDiror the app's data directory) rather than the current working directory.internal/clientmigrate/rtorrent.go (5)
76-79: Dry-run log message is misleading.The message "successfully imported" incorrectly implies that an import occurred, when dry-run mode performs no actual writes.
Apply this diff:
if i.opts.DryRun { - log.Info().Msgf("dry-run: (%d/%d) successfully imported: %s", positionNum, totalJobs, torrentID) + log.Info().Msgf("dry-run: (%d/%d) would import: %s", positionNum, totalJobs, torrentID) continue }
217-219: Active time calculation is fundamentally incorrect.
getActiveTimetreats RTorrent'sseedingtimeas a start timestamp and calculates duration from then until now usingtime.Since(). This incorrectly assumes continuous activity, overestimating if the torrent was paused or stopped. qBittorrent'sactive_timeandseeding_timefields expect cumulative seconds of actual activity, not a calculated duration from a start time.If
seedingtimeis already a cumulative duration in seconds, use it directly:-func getActiveTime(t string) int64 { - return int64(time.Since(time.Unix(strToIntClean(t), 0)).Seconds()) -} +func getActiveTime(t string) int64 { + // RTorrent stores cumulative seconds directly + return strToIntClean(t) +}If
seedingtimeis truly a timestamp and you cannot reconstruct cumulative time, default to0rather than using an incorrect calculation.
121-121: FinishedTime must use seconds, not minutes.qBittorrent (via libtorrent) expects
finished_timeas cumulative seconds, not minutes. Using.Minutes()creates a value off by a factor of 60.Apply this diff:
- FinishedTime: int64(time.Since(time.Unix(rtFile.TimestampFinished, 0)).Minutes()), + FinishedTime: int64(time.Since(time.Unix(rtFile.TimestampFinished, 0)).Seconds()),
169-176: Path manipulation using ReplaceAll is fragile.Using
strings.ReplaceAllto remove the torrent name from the directory path can fail if:
- The torrent name appears multiple times in the path
- The name is a substring of a parent directory
Apply this diff:
// Fix savepath for torrents with subfolder // directory contains the whole torrent path, which gives error in qBit. - // remove file.sourceDirInfo.name from full path in id.rtorrent directory - newPath := strings.ReplaceAll(rtFile.Directory, metaInfo.Name, "") + // remove the torrent name from the end of the path + newPath := strings.TrimSuffix(rtFile.Directory, string(filepath.Separator)+metaInfo.Name) + if newPath == rtFile.Directory { + // If no suffix was removed, try without separator (edge case) + newPath = strings.TrimSuffix(rtFile.Directory, metaInfo.Name) + }
197-206: File write errors halt entire migration unnecessarily.If encoding the fastresume file or copying the torrent file fails for a single torrent, the entire migration stops, preventing other valid torrents from being processed.
Apply this diff to log and continue instead:
// copy torrent file fastResumeOutFile := filepath.Join(i.opts.QbitDir, torrentID+".fastresume") if err = newFastResume.Encode(fastResumeOutFile); err != nil { log.Error().Err(err).Msgf("Could not create qBittorrent fastresume file %s error: %q", fastResumeOutFile, err) - return err + continue } if err = CopyFile(match, torrentOutFile); err != nil { log.Error().Err(err).Msgf("Could copy qBittorrent torrent file %s error %q", torrentOutFile, err) - return err + continue }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
cmd/qui/main.go(3 hunks)internal/clientmigrate/deluge.go(1 hunks)internal/clientmigrate/migrate.go(1 hunks)internal/clientmigrate/rtorrent.go(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Audionut
Repo: autobrr/qui PR: 553
File: internal/services/crossseed/service.go:1045-1082
Timestamp: 2025-11-06T12:11:04.963Z
Learning: The autobrr/qui project uses a custom go-qbittorrent client library (github.com/autobrr/go-qbittorrent) that supports both "paused" and "stopped" parameters when adding torrents via the options map. Both parameters should be set together when controlling torrent start state, as seen in internal/services/crossseed/service.go and throughout the codebase.
📚 Learning: 2025-11-06T12:11:04.963Z
Learnt from: Audionut
Repo: autobrr/qui PR: 553
File: internal/services/crossseed/service.go:1045-1082
Timestamp: 2025-11-06T12:11:04.963Z
Learning: The autobrr/qui project uses a custom go-qbittorrent client library (github.com/autobrr/go-qbittorrent) that supports both "paused" and "stopped" parameters when adding torrents via the options map. Both parameters should be set together when controlling torrent start state, as seen in internal/services/crossseed/service.go and throughout the codebase.
Applied to files:
internal/clientmigrate/rtorrent.go
🧬 Code graph analysis (4)
cmd/qui/main.go (1)
internal/clientmigrate/migrate.go (2)
New(32-34)Options(16-22)
internal/clientmigrate/migrate.go (2)
internal/clientmigrate/rtorrent.go (1)
NewRTorrentImporter(23-25)internal/clientmigrate/deluge.go (1)
NewDelugeImporter(19-21)
internal/clientmigrate/deluge.go (2)
internal/clientmigrate/migrate.go (3)
Options(16-22)MkDirIfNotExists(154-168)CopyFile(175-212)internal/qbittorrent/fastresume.go (1)
Fastresume(14-86)
internal/clientmigrate/rtorrent.go (2)
internal/clientmigrate/migrate.go (3)
Options(16-22)ClientMigrater(24-26)CopyFile(175-212)internal/qbittorrent/fastresume.go (1)
Fastresume(14-86)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
internal/clientmigrate/migrate.go (1)
56-130: Backup directory "qbt_backup" must be created before use.The backup logic hardcodes
"qbt_backup"at lines 62-63 but never creates this directory. Whenos.Create()is called at lines 79 and 111, it will fail withENOENTifqbt_backupdoesn't already exist, causing migrations to fail by default.🔎 Proposed fix
// Backup data before running if !skipBackup { log.Info().Msg("prepare to backup torrent data before import..") timeStamp := time.Now().Format("20060102150405") + + backupDir := "qbt_backup" + if err := MkDirIfNotExists(backupDir); err != nil { + return errors.Wrap(err, "could not create backup directory") + } - sourceBackupArchive := filepath.Join("qbt_backup", source+"_backup_"+timeStamp+".tar.gz") - qbitBackupArchive := filepath.Join("qbt_backup", "qBittorrent_backup_"+timeStamp+".tar.gz") + sourceBackupArchive := filepath.Join(backupDir, source+"_backup_"+timeStamp+".tar.gz") + qbitBackupArchive := filepath.Join(backupDir, "qBittorrent_backup_"+timeStamp+".tar.gz")Additionally, consider anchoring
backupDirto a predictable location (e.g., nearqbitDiror the app's data directory) rather than the current working directory, especially when run as a service.
🧹 Nitpick comments (3)
internal/clientmigrate/transmission.go (3)
65-65: Use Info or Debug level for expected skip behavior.Logging at Error level when a torrent already exists is misleading—this is expected behavior during incremental migrations or re-runs, not an error condition.
🔎 Suggested change
- if _, err = os.Stat(torrentOutFile); err == nil { - log.Error().Err(err).Msgf("(%d/%d) %s Torrent already exists, skipping", positionNum, totalJobs, torrentOutFile) + if _, err = os.Stat(torrentOutFile); err == nil { + log.Info().Msgf("(%d/%d) %s Torrent already exists, skipping", positionNum, totalJobs, torrentOutFile) continue }Note: Also remove
.Err(err)sinceerris nil in this branch.
94-94: Remove or demote debug log statement.This verbose log statement (logging the entire resume file structure) should either be removed or changed to Debug level to avoid cluttering production logs.
🔎 Suggested change
- log.Info().Msgf("resume file, %v", resumeFile) + log.Debug().Msgf("resume file, %v", resumeFile)Or remove it entirely if it was only needed during development.
161-165: Path manipulation withstrings.ReplaceAllis fragile and may fail on edge cases.Using
strings.ReplaceAll(resumeFile.Destination, metaInfo.Name, "")to extract the base path can produce incorrect results ifmetaInfo.Nameappears multiple times in the path or not at all. A safer approach is to usefilepath.Dir()orstrings.TrimSuffix()after verifying the torrent name is the last component.🔎 Safer alternative
- // Fix savepath for torrents with subfolder - // directory contains the whole torrent path, which gives error in qBit. - // remove file.sourceDirInfo.name from full path directory - newPath := strings.ReplaceAll(resumeFile.Destination, metaInfo.Name, "") + // Fix savepath for torrents with subfolder: strip the torrent name if it's the last component + newPath := resumeFile.Destination + if strings.HasSuffix(newPath, metaInfo.Name) { + newPath = strings.TrimSuffix(newPath, metaInfo.Name) + newPath = filepath.Clean(newPath) + } newFastResume.Path = newPathThis avoids unintended replacements and ensures the path is well-formed.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
cmd/qui/main.gogo.modinternal/clientmigrate/migrate.gointernal/clientmigrate/transmission.go
🚧 Files skipped from review as they are similar to previous changes (1)
- go.mod
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-12-28T18:44:10.496Z
Learnt from: s0up4200
Repo: autobrr/qui PR: 876
File: internal/logstream/hub_test.go:188-192
Timestamp: 2025-12-28T18:44:10.496Z
Learning: In Go 1.25 (Aug 2025), use wg.Go(func()) to spawn a goroutine and automate the Add/Done lifecycle. Replace manual patterns like wg.Add(1); go func(){ defer wg.Done(); ... }() with wg.Go(func(){ ... }). Ensure the codebase builds with Go 1.25+ and apply this in relevant Go files (e.g., internal/logstream/hub_test.go). If targeting older Go versions, maintain the existing pattern.
Applied to files:
cmd/qui/main.gointernal/clientmigrate/migrate.gointernal/clientmigrate/transmission.go
📚 Learning: 2025-11-25T22:46:03.762Z
Learnt from: s0up4200
Repo: autobrr/qui PR: 632
File: internal/backups/service.go:1401-1404
Timestamp: 2025-11-25T22:46:03.762Z
Learning: In qui's backup service (internal/backups/service.go), background torrent downloads initiated during manifest import intentionally use a fire-and-forget pattern with the shared service context (s.ctx). Per-run cancellation is not needed, as orphaned downloads completing after run deletion are considered harmless and acceptable. This design prioritizes simplicity over per-run lifecycle management for background downloads.
Applied to files:
internal/clientmigrate/migrate.gointernal/clientmigrate/transmission.go
📚 Learning: 2025-11-06T12:11:04.963Z
Learnt from: Audionut
Repo: autobrr/qui PR: 553
File: internal/services/crossseed/service.go:1045-1082
Timestamp: 2025-11-06T12:11:04.963Z
Learning: The autobrr/qui project uses a custom go-qbittorrent client library (github.com/autobrr/go-qbittorrent) that supports both "paused" and "stopped" parameters when adding torrents via the options map. Both parameters should be set together when controlling torrent start state, as seen in internal/services/crossseed/service.go and throughout the codebase.
Applied to files:
internal/clientmigrate/transmission.go
🧬 Code graph analysis (2)
cmd/qui/main.go (1)
internal/clientmigrate/migrate.go (2)
New(32-45)Options(15-21)
internal/clientmigrate/migrate.go (3)
internal/clientmigrate/deluge.go (1)
NewDelugeImporter(19-21)internal/clientmigrate/rtorrent.go (1)
NewRTorrentImporter(23-25)internal/clientmigrate/transmission.go (1)
NewTransmissionImporter(21-23)
🔇 Additional comments (4)
internal/clientmigrate/transmission.go (1)
177-177: Verify thatConvertFilePrioritycannot fail or handle its potential errors.Line 177 calls
newFastResume.ConvertFilePriority(len(metaInfo.Files))without checking for errors. If this method can fail (e.g., due to invalid input), the migration could produce corrupted fastresume data.Please confirm whether
ConvertFilePrioritycan return an error or panic. If so, add appropriate error handling:if err := newFastResume.ConvertFilePriority(len(metaInfo.Files)); err != nil { log.Error().Err(err).Msgf("Could not convert file priority for %s", torrentID) continue }If it's infallible, consider documenting that in the method or adding a comment here.
cmd/qui/main.go (1)
400-454: LGTM! The migrate command is well-structured.The command properly:
- Validates the source argument against allowed values
- Marks required flags (
source-dir,qbit-dir)- Passes
SkipBackupto Options (fixing the previous review issue)- Wraps errors with context
The implementation is clean and follows the project's CLI patterns.
internal/clientmigrate/migrate.go (2)
152-167: LGTM!MkDirIfNotExistsis well-implemented.The utility correctly checks for directory existence, creates it with appropriate permissions if missing, and handles errors properly.
169-211: LGTM!CopyFilefollows best practices for safe file copying.The implementation properly:
- Handles deferred cleanup with error capture
- Syncs data to disk before closing
- Preserves source file mode
This is a robust file copy utility.
✅ Deploy Preview for getqui canceled.
|
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @internal/clientmigrate/rtorrent.go:
- Line 121: The FinishedTime computation uses Minutes() but should use Seconds()
for consistency; change the expression in the struct field FinishedTime from
int64(time.Since(time.Unix(rtFile.TimestampFinished, 0)).Minutes()) to use
.Seconds() instead (i.e., int64(time.Since(time.Unix(rtFile.TimestampFinished,
0)).Seconds())), ensuring the FinishedTime field and its source
rtFile.TimestampFinished match the seconds-based units used elsewhere like
getActiveTime()/transmission.go.
- Around line 216-219: The getActiveTime function uses
time.Since(time.Unix(...)) which is correct but obscures intent; change it to
explicitly compute elapsed seconds by parsing the input via strToIntClean(t)
into startTime, return 0 if startTime==0, otherwise return time.Now().Unix() -
startTime so the code clearly converts the rtorrent UNIX seeding timestamp into
elapsed seconds; update the getActiveTime function to use startTime and
time.Now().Unix() and keep using strToIntClean as the parser.
In @internal/clientmigrate/transmission.go:
- Around line 185-193: The qBittorrent importer currently aborts the whole
migration on fastresume or torrent copy errors by returning after
newFastResume.Encode(...) and CopyFile(...); change this to match the rtorrent
importer by logging the error (retain log.Error().Err(err).Msgf(...) with
contextual file names fastResumeOutFile and torrentOutFile) and then continue
processing the next torrent instead of returning; ensure the loop around these
calls (the function handling each match/torrent) does not exit on these errors
and optionally track a non-fatal error counter or aggregated error for final
reporting rather than immediate return.
🧹 Nitpick comments (8)
internal/clientmigrate/migrate.go (3)
47-82: Context parameter is unused.The
ctxparameter is not used within the method, nor is it passed tom.imp.Migrate(). Consider either:
- Updating the
ClientMigraterinterface to accept context:Migrate(context.Context) error- Using the context to check for cancellation before/during the migration
- Removing the unused parameter if cancellation support isn't needed
♻️ Option 1: Update interface to accept context
Update the interface definition:
type ClientMigrater interface { - Migrate() error + Migrate(context.Context) error }Then pass context to the importer:
- if err := m.imp.Migrate(); err != nil { + if err := m.imp.Migrate(ctx); err != nil { return errors.Wrapf(err, "could not import from %s", source) }And update all implementers (RTorrentImport, DelugeImport, TransmissionImport) to accept and use the context.
119-147: Consider cleaning up partial archives on error.If
format.Archivefails, the partially written archive file remains on disk. Consider removing the incomplete archive to avoid confusion.♻️ Proposed enhancement
func (m Migrater) archiveDir(ctx context.Context, dir, archiveName string) error { // map files on disk to their paths in the archive using default settings (second arg) files, err := archives.FilesFromDisk(ctx, nil, map[string]string{ dir: "", }) if err != nil { return err } // create the output file we'll write to out, err := os.Create(archiveName) if err != nil { return err } defer out.Close() format := archives.CompressedArchive{ Compression: archives.Gz{}, Archival: archives.Tar{}, } // create the archive err = format.Archive(ctx, out, files) if err != nil { + os.Remove(archiveName) // Clean up partial archive return errors.Wrapf(err, "could not create backup archive: %s", out.Name()) } return nil }
149-164: Consider more restrictive directory permissions.The function uses
os.ModePerm(0777), which creates world-writable directories. Backup directories typically don't need to be world-accessible.🔒 Proposed change
func MkDirIfNotExists(dir string) error { if _, err := os.Stat(dir); err != nil { if os.IsNotExist(err) { - if err := os.MkdirAll(dir, os.ModePerm); err != nil { + if err := os.MkdirAll(dir, 0750); err != nil { return err } return nil } return err } return nil }internal/clientmigrate/rtorrent.go (3)
70-74: Use Info level for expected skip condition.Skipping an already-imported torrent is expected behavior, not an error condition.
📝 Proposed fix
// If file already exists, skip if _, err = os.Stat(torrentOutFile); err == nil { - log.Error().Err(err).Msgf("(%d/%d) %s Torrent already exists, skipping", positionNum, totalJobs, torrentOutFile) + log.Info().Msgf("(%d/%d) %s Torrent already exists, skipping", positionNum, totalJobs, torrentOutFile) continue }
169-176: Path manipulation could be more robust.Using
strings.ReplaceAllmight replace multiple occurrences of the torrent name if it appears in parent directories. Consider using path manipulation functions for more predictable behavior.🛠️ Proposed improvement
// Fix savepath for torrents with subfolder // directory contains the whole torrent path, which gives error in qBit. // remove file.sourceDirInfo.name from full path in id.rtorrent directory - newPath := strings.ReplaceAll(rtFile.Directory, metaInfo.Name, "") + // Use TrimSuffix to remove only the trailing torrent name + newPath := strings.TrimSuffix(rtFile.Directory, string(filepath.Separator) + metaInfo.Name) + if newPath == rtFile.Directory { + // If nothing was trimmed, the directory might not end with the torrent name + // Fall back to the directory as-is + newPath = rtFile.Directory + }Alternatively, verify that the directory ends with the torrent name before trimming to avoid unexpected behavior.
277-288: Consider logging parse errors.The function silently returns 0 on parse errors, which could hide data corruption. Consider logging a warning when parsing fails to aid troubleshooting.
📝 Proposed enhancement
func strToIntClean(line string) int64 { if line == "" { return 0 } s := strings.TrimSuffix(line, "\n") i, err := strconv.ParseInt(s, 10, 64) if err != nil { + log.Warn().Str("value", s).Msg("failed to parse integer from rtorrent data") return 0 } return i }internal/clientmigrate/transmission.go (2)
25-26: Document Transmission directory structure assumption.The code assumes Transmission stores torrents in a
/torrentssubdirectory under the source directory. This might not hold for all Transmission configurations. Consider documenting this requirement in the command help or validating the directory structure.
156-163: Path manipulation could be more robust.Same concern as in rtorrent.go: using
strings.ReplaceAllto remove the torrent name from the path could cause issues if the name appears multiple times. Consider usingstrings.TrimSuffixor path manipulation functions.🛠️ Proposed improvement
// Fix savepath for torrents with subfolder // directory contains the whole torrent path, which gives error in qBit. // remove file.sourceDirInfo.name from full path directory - newPath := strings.ReplaceAll(resumeFile.Destination, metaInfo.Name, "") + newPath := strings.TrimSuffix(resumeFile.Destination, string(filepath.Separator) + metaInfo.Name) + if newPath == resumeFile.Destination { + newPath = resumeFile.Destination + }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (6)
cmd/qui/main.gogo.modinternal/clientmigrate/deluge.gointernal/clientmigrate/migrate.gointernal/clientmigrate/rtorrent.gointernal/clientmigrate/transmission.go
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/clientmigrate/deluge.go
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-12-28T18:44:10.496Z
Learnt from: s0up4200
Repo: autobrr/qui PR: 876
File: internal/logstream/hub_test.go:188-192
Timestamp: 2025-12-28T18:44:10.496Z
Learning: In Go 1.25 (Aug 2025), use wg.Go(func()) to spawn a goroutine and automate the Add/Done lifecycle. Replace manual patterns like wg.Add(1); go func(){ defer wg.Done(); ... }() with wg.Go(func(){ ... }). Ensure the codebase builds with Go 1.25+ and apply this in relevant Go files (e.g., internal/logstream/hub_test.go). If targeting older Go versions, maintain the existing pattern.
Applied to files:
cmd/qui/main.gointernal/clientmigrate/migrate.gointernal/clientmigrate/transmission.gointernal/clientmigrate/rtorrent.go
📚 Learning: 2025-11-25T22:46:03.762Z
Learnt from: s0up4200
Repo: autobrr/qui PR: 632
File: internal/backups/service.go:1401-1404
Timestamp: 2025-11-25T22:46:03.762Z
Learning: In qui's backup service (internal/backups/service.go), background torrent downloads initiated during manifest import intentionally use a fire-and-forget pattern with the shared service context (s.ctx). Per-run cancellation is not needed, as orphaned downloads completing after run deletion are considered harmless and acceptable. This design prioritizes simplicity over per-run lifecycle management for background downloads.
Applied to files:
internal/clientmigrate/migrate.gointernal/clientmigrate/transmission.gointernal/clientmigrate/rtorrent.go
📚 Learning: 2025-11-21T21:11:50.633Z
Learnt from: s0up4200
Repo: autobrr/qui PR: 625
File: internal/qbittorrent/sync_manager.go:1112-1123
Timestamp: 2025-11-21T21:11:50.633Z
Learning: In internal/qbittorrent/sync_manager.go, the GetCachedFilesBatch interface documentation (around line 39-40) should specify "uppercase hex" instead of "lowercase hex" to match the actual normalization practice used throughout the codebase (e.g., normalizeHash in internal/services/crossseed/service.go uses strings.ToUpper, and hash filtering uses uppercase).
Applied to files:
internal/clientmigrate/transmission.go
📚 Learning: 2025-11-28T20:32:30.126Z
Learnt from: s0up4200
Repo: autobrr/qui PR: 641
File: internal/services/crossseed/service.go:209-212
Timestamp: 2025-11-28T20:32:30.126Z
Learning: Repo: autobrr/qui PR: 641
File: internal/services/crossseed/service.go
Learning: The cross-seed recheck-resume worker intentionally runs for the process lifetime and keys pending entries by hash only. This is acceptable under the current constraint that background seeded-search runs operate on a single instance at a time; graceful shutdown and instanceID|hash keying are deferred by design.
Applied to files:
internal/clientmigrate/transmission.gointernal/clientmigrate/rtorrent.go
📚 Learning: 2025-11-06T12:11:04.963Z
Learnt from: Audionut
Repo: autobrr/qui PR: 553
File: internal/services/crossseed/service.go:1045-1082
Timestamp: 2025-11-06T12:11:04.963Z
Learning: The autobrr/qui project uses a custom go-qbittorrent client library (github.com/autobrr/go-qbittorrent) that supports both "paused" and "stopped" parameters when adding torrents via the options map. Both parameters should be set together when controlling torrent start state, as seen in internal/services/crossseed/service.go and throughout the codebase.
Applied to files:
internal/clientmigrate/transmission.gointernal/clientmigrate/rtorrent.go
📚 Learning: 2025-12-11T08:40:01.329Z
Learnt from: s0up4200
Repo: autobrr/qui PR: 746
File: internal/services/reannounce/service.go:480-481
Timestamp: 2025-12-11T08:40:01.329Z
Learning: In autobrr/qui's internal/services/reannounce/service.go, the hasHealthyTracker, getProblematicTrackers, and getHealthyTrackers functions intentionally match qbrr's lenient tracker health logic (skip unregistered trackers and check if any other tracker is healthy) rather than go-qbittorrent's strict isTrackerStatusOK logic (which treats unregistered as an immediate failure). For multi-tracker torrents, if one tracker is working, reannouncing won't help. The duplication of the health check logic across these three functions is acceptable as it's a simple one-liner, and extracting it would add unnecessary complexity.
Applied to files:
internal/clientmigrate/rtorrent.go
📚 Learning: 2025-11-28T22:21:20.730Z
Learnt from: s0up4200
Repo: autobrr/qui PR: 641
File: internal/services/crossseed/service.go:2415-2457
Timestamp: 2025-11-28T22:21:20.730Z
Learning: Repo: autobrr/qui PR: 641
File: internal/services/crossseed/service.go
Learning: The determineSavePath function intentionally includes a contentLayout string parameter for future/content-layout branching and API consistency. Its presence is by design even if unused in the current body; do not flag as an issue in reviews.
Applied to files:
internal/clientmigrate/rtorrent.go
🧬 Code graph analysis (4)
cmd/qui/main.go (1)
internal/clientmigrate/migrate.go (2)
New(32-45)Options(15-21)
internal/clientmigrate/migrate.go (3)
internal/clientmigrate/deluge.go (1)
NewDelugeImporter(19-21)internal/clientmigrate/rtorrent.go (1)
NewRTorrentImporter(23-25)internal/clientmigrate/transmission.go (1)
NewTransmissionImporter(21-23)
internal/clientmigrate/transmission.go (2)
internal/clientmigrate/migrate.go (3)
Options(15-21)ClientMigrater(23-25)CopyFile(171-208)internal/qbittorrent/fastresume.go (1)
Fastresume(14-86)
internal/clientmigrate/rtorrent.go (2)
internal/clientmigrate/migrate.go (3)
Options(15-21)ClientMigrater(23-25)CopyFile(171-208)internal/qbittorrent/fastresume.go (1)
Fastresume(14-86)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests
🔇 Additional comments (13)
cmd/qui/main.go (3)
402-419: LGTM! Well-structured command definition.The command properly validates the source client argument against a whitelist and provides helpful examples for each supported client.
421-434: LGTM! Flags are well-defined.The required flags are properly marked, and the help text clearly describes each option.
436-453: LGTM! Clean error handling and integration.The implementation correctly constructs options from flags, instantiates the migrator, and wraps errors with context. The command context is passed to Migrate for potential cancellation support.
internal/clientmigrate/migrate.go (2)
84-117: LGTM! Backup logic is well-structured.The backup properly creates timestamped archives of both source and destination directories before migration. The context is correctly passed to archiveDir for cancellation support during archive creation.
166-208: LGTM! Robust file copy implementation.The function properly handles errors, syncs data to disk, and preserves file permissions. The deferred close with error capture ensures cleanup happens even on failure.
internal/clientmigrate/rtorrent.go (4)
32-56: LGTM! Thorough source directory validation.The validation properly checks for directory existence, type, and available torrent files, with clear error messages for each failure case.
81-106: LGTM! Proper error handling with graceful degradation.Errors are logged and the loop continues, ensuring that one corrupted torrent doesn't abort the entire migration.
185-214: LGTM! Proper finalization and output.The fastresume encoding and file copying are correctly handled with appropriate error logging and progress tracking.
222-237: LGTM! Tracker conversion handles DHT exclusion properly.The function correctly filters out DHT and disabled trackers, converting to qBittorrent's tier-based tracker format.
internal/clientmigrate/transmission.go (3)
63-72: LGTM! Correct log level for expected skip.Unlike the rtorrent importer, this correctly uses
log.Info()when skipping existing torrents.
203-215: LGTM! Standard bencode decode pattern.The function properly reads and decodes the Transmission resume file with appropriate error propagation.
217-266: LGTM! Comprehensive Transmission resume file structure.The types properly map Transmission's resume file format with correct bencode tags.
go.mod (1)
26-26: Packages are current and secure.Both packages are at their latest versions and free from known vulnerabilities:
github.com/mholt/archives v0.1.5is the latest version (released Oct 2025) with no CVEsgithub.com/zeebo/bencode v1.0.0is the current version with no CVEs; the library is stable and maintenance-focused (last update Aug 2018), which is appropriate for a mature encoding implementation
| FileFormat: "libtorrent resume file", | ||
| FileVersion: 1, | ||
| FilePriority: []int{}, | ||
| FinishedTime: int64(time.Since(time.Unix(resumeFile.DoneDate, 0)).Seconds()), |
There was a problem hiding this comment.
Inconsistent FinishedTime calculation across importers.
This importer uses .Seconds() on line 108, while rtorrent.go uses .Minutes() on line 121. Both should use the same unit for consistency. Verify which is correct according to qBittorrent's expected format.
Based on learnings, this appears to be a discrepancy that should be reconciled for consistency.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @internal/clientmigrate/rtorrent.go:
- Around line 203-206: The error log message in the CopyFile error branch uses a
typo "Could copy" instead of "Could not copy"; update the Msgf call inside the
error handling after CopyFile(match, torrentOutFile) so the message reads "Could
not copy qBittorrent torrent file %s error %q" (retain the use of
log.Error().Err(err).Msgf, and keep the same arguments match/torrentOutFile and
err).
- Around line 70-74: The log call inside the os.Stat success branch is
incorrect: when os.Stat(torrentOutFile) returns nil, err is nil, so remove the
erroneous .Err(err) and demote the severity from log.Error to log.Warn to
reflect an expected skip; update the logging statement that references
torrentOutFile, positionNum, and totalJobs (the block around the os.Stat check
in internal/clientmigrate/rtorrent.go) to use log.Warn().Msgf(...) without
attaching Err(err).
🧹 Nitpick comments (4)
internal/clientmigrate/migrate.go (3)
23-24: Consider adding context parameter to the interface.The
Migrate(ctx context.Context)method on line 49 receives a context but cannot pass it tom.imp.Migrate()on line 75 because theClientMigraterinterface doesn't accept context. This prevents proper cancellation and timeout support during potentially long-running migration operations.♻️ Proposed fix to add context support
type ClientMigrater interface { - Migrate() error + Migrate(ctx context.Context) error }Note: This will require updating all implementations (DelugeImporter, RTorrentImporter, TransmissionImporter) to accept the context parameter.
42-43: Consider returning an error instead of calling log.Fatal().Using
log.Fatal()terminates the entire program, which may not be desirable in library code. While acceptable for CLI usage, returning an error would provide better composability and allow callers to handle unsupported sources gracefully.♻️ Alternative approach
-func New(opts Options) Migrater { +func New(opts Options) (Migrater, error) { m := Migrater{opts: opts} switch m.opts.Source { case "deluge": m.imp = NewDelugeImporter(m.opts) case "rtorrent": m.imp = NewRTorrentImporter(m.opts) case "transmission": m.imp = NewTransmissionImporter(m.opts) default: - log.Fatal().Str("source", m.opts.Source).Msg("unsupported source client") + return m, errors.Errorf("unsupported source client: %s", m.opts.Source) } - return m + return m, nil }
152-166: Consider using more restrictive permissions for backup directory.Line 155 uses
os.ModePerm(0777), which grants read, write, and execute permissions to all users. For security-sensitive backup directories containing torrent data, consider using more restrictive permissions like0755or0700.♻️ Proposed fix
func MkDirIfNotExists(dir string) error { if _, err := os.Stat(dir); err != nil { if os.IsNotExist(err) { - if err := os.MkdirAll(dir, os.ModePerm); err != nil { + if err := os.MkdirAll(dir, 0755); err != nil { return err } return nil }internal/clientmigrate/rtorrent.go (1)
169-176: Path manipulation using string replacement is fragile.Line 172 uses
strings.ReplaceAllto remove the torrent name from the directory path. This approach is fragile and could fail if the torrent name appears multiple times in the path or contains special characters.♻️ More robust alternative using filepath operations
// Fix savepath for torrents with subfolder // directory contains the whole torrent path, which gives error in qBit. // remove file.sourceDirInfo.name from full path in id.rtorrent directory - newPath := strings.ReplaceAll(rtFile.Directory, metaInfo.Name, "") + // If the directory ends with the torrent name, remove it + newPath := rtFile.Directory + if strings.HasSuffix(rtFile.Directory, string(filepath.Separator)+metaInfo.Name) { + newPath = filepath.Dir(rtFile.Directory) + } newFastResume.Path = newPath newFastResume.SavePath = newPath newFastResume.QbtSavePath = newPathThis approach is safer as it only removes the name if it's the final path component.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
internal/clientmigrate/migrate.gointernal/clientmigrate/rtorrent.gointernal/clientmigrate/transmission.go
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/clientmigrate/transmission.go
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-11-25T22:46:03.762Z
Learnt from: s0up4200
Repo: autobrr/qui PR: 632
File: internal/backups/service.go:1401-1404
Timestamp: 2025-11-25T22:46:03.762Z
Learning: In qui's backup service (internal/backups/service.go), background torrent downloads initiated during manifest import intentionally use a fire-and-forget pattern with the shared service context (s.ctx). Per-run cancellation is not needed, as orphaned downloads completing after run deletion are considered harmless and acceptable. This design prioritizes simplicity over per-run lifecycle management for background downloads.
Applied to files:
internal/clientmigrate/migrate.gointernal/clientmigrate/rtorrent.go
📚 Learning: 2025-12-28T18:44:10.496Z
Learnt from: s0up4200
Repo: autobrr/qui PR: 876
File: internal/logstream/hub_test.go:188-192
Timestamp: 2025-12-28T18:44:10.496Z
Learning: In Go 1.25 (Aug 2025), use wg.Go(func()) to spawn a goroutine and automate the Add/Done lifecycle. Replace manual patterns like wg.Add(1); go func(){ defer wg.Done(); ... }() with wg.Go(func(){ ... }). Ensure the codebase builds with Go 1.25+ and apply this in relevant Go files (e.g., internal/logstream/hub_test.go). If targeting older Go versions, maintain the existing pattern.
Applied to files:
internal/clientmigrate/migrate.gointernal/clientmigrate/rtorrent.go
📚 Learning: 2025-11-06T12:11:04.963Z
Learnt from: Audionut
Repo: autobrr/qui PR: 553
File: internal/services/crossseed/service.go:1045-1082
Timestamp: 2025-11-06T12:11:04.963Z
Learning: The autobrr/qui project uses a custom go-qbittorrent client library (github.com/autobrr/go-qbittorrent) that supports both "paused" and "stopped" parameters when adding torrents via the options map. Both parameters should be set together when controlling torrent start state, as seen in internal/services/crossseed/service.go and throughout the codebase.
Applied to files:
internal/clientmigrate/rtorrent.go
📚 Learning: 2025-11-28T20:32:30.126Z
Learnt from: s0up4200
Repo: autobrr/qui PR: 641
File: internal/services/crossseed/service.go:209-212
Timestamp: 2025-11-28T20:32:30.126Z
Learning: Repo: autobrr/qui PR: 641
File: internal/services/crossseed/service.go
Learning: The cross-seed recheck-resume worker intentionally runs for the process lifetime and keys pending entries by hash only. This is acceptable under the current constraint that background seeded-search runs operate on a single instance at a time; graceful shutdown and instanceID|hash keying are deferred by design.
Applied to files:
internal/clientmigrate/rtorrent.go
📚 Learning: 2025-12-11T08:40:01.329Z
Learnt from: s0up4200
Repo: autobrr/qui PR: 746
File: internal/services/reannounce/service.go:480-481
Timestamp: 2025-12-11T08:40:01.329Z
Learning: In autobrr/qui's internal/services/reannounce/service.go, the hasHealthyTracker, getProblematicTrackers, and getHealthyTrackers functions intentionally match qbrr's lenient tracker health logic (skip unregistered trackers and check if any other tracker is healthy) rather than go-qbittorrent's strict isTrackerStatusOK logic (which treats unregistered as an immediate failure). For multi-tracker torrents, if one tracker is working, reannouncing won't help. The duplication of the health check logic across these three functions is acceptable as it's a simple one-liner, and extracting it would add unnecessary complexity.
Applied to files:
internal/clientmigrate/rtorrent.go
📚 Learning: 2025-11-28T22:21:20.730Z
Learnt from: s0up4200
Repo: autobrr/qui PR: 641
File: internal/services/crossseed/service.go:2415-2457
Timestamp: 2025-11-28T22:21:20.730Z
Learning: Repo: autobrr/qui PR: 641
File: internal/services/crossseed/service.go
Learning: The determineSavePath function intentionally includes a contentLayout string parameter for future/content-layout branching and API consistency. Its presence is by design even if unused in the current body; do not flag as an issue in reviews.
Applied to files:
internal/clientmigrate/rtorrent.go
🧬 Code graph analysis (2)
internal/clientmigrate/migrate.go (3)
internal/clientmigrate/deluge.go (1)
NewDelugeImporter(19-21)internal/clientmigrate/rtorrent.go (1)
NewRTorrentImporter(23-25)internal/clientmigrate/transmission.go (1)
NewTransmissionImporter(21-23)
internal/clientmigrate/rtorrent.go (2)
internal/clientmigrate/migrate.go (3)
Options(15-21)ClientMigrater(23-25)CopyFile(173-210)internal/qbittorrent/fastresume.go (1)
Fastresume(14-86)
🔇 Additional comments (4)
internal/clientmigrate/migrate.go (1)
173-210: LGTM! Robust file copy implementation.The
CopyFileimplementation follows best practices:
- Properly closes file handles with defer
- Syncs data to disk before closing
- Preserves source file permissions
- Handles errors correctly with named return values
internal/clientmigrate/rtorrent.go (3)
217-223: LGTM! Correct time calculation.The
getActiveTimefunction correctly converts rTorrent's seeding start timestamp to elapsed seconds, with proper handling of the zero case.
226-241: LGTM! Proper tracker conversion.The
convertTrackersfunction correctly:
- Skips DHT pseudo-trackers
- Filters for enabled trackers only
- Converts to qBittorrent's nested array format
281-292: LGTM! Defensive string-to-int conversion.The
strToIntCleanfunction handles edge cases appropriately by returning 0 for empty strings and parse errors, which is suitable for timestamp fields that may be unset.
zze0s
changed the title
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (3)
internal/clientmigrate/deluge.go (1)
170-181: Replaceinterface{}withanyper static analysis and coding guidelines.The linter flags
interface{}as replaceable byany(Go 1.18+). Also, the coding guideline says to avoidmap[string]interface{}and use proper structs. Here the map is used because keys are dynamic torrent IDs — that's reasonable, but at minimum useanyfor the modernize lint fix.♻️ Proposed fix
-func decodeFastresumeFile(path string) (map[string]interface{}, error) { +func decodeFastresumeFile(path string) (map[string]any, error) { dat, err := os.ReadFile(path) if err != nil { return nil, err } - var fastresumeFile map[string]interface{} + var fastresumeFile map[string]any if err := bencode.DecodeBytes(dat, &fastresumeFile); err != nil { return nil, err }As per coding guidelines:
**/*.go: Avoidmap[string]interface{}— use proper structs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/clientmigrate/deluge.go` around lines 170 - 181, The function decodeFastresumeFile uses map[string]interface{} which the linter wants modernized; change all occurrences in the function signature, local variable declaration, and return types from map[string]interface{} to map[string]any (e.g., update the return type of decodeFastresumeFile, the fastresumeFile variable declaration, and any downstream uses expecting that type) so the code compiles and satisfies the "use any" lint rule while preserving the existing dynamic-key behavior.go.mod (2)
56-56: Consider the dependency footprint and supply chain implications.The addition of
github.com/mholt/archivesintroduces 13 new indirect dependencies, primarily for compression and archive format support. While this is expected for a multi-format archive library, it significantly expands the dependency tree and supply chain surface area.Consider whether all archive format support is necessary for the migration feature, or if a more focused library with fewer transitive dependencies could suffice. If the broad format support is required, ensure you have:
- Dependency scanning in CI/CD pipelines
- Regular dependency updates via Dependabot or Renovate
- SBOM generation for supply chain visibility
Also applies to: 61-63, 66-66, 76-76, 80-80, 84-85, 91-91, 94-94, 101-101, 111-111
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` at line 56, The new dependency github.com/mholt/archives (and the indirect github.com/STARRY-S/zip entry) increases transitive dependencies; decide whether full multi-format support is required and if not replace it with a narrower library or vendor only needed codecs, otherwise add CI/ops controls: add dependency scanning to pipelines, enable Dependabot/Renovate for go.mod, and generate an SBOM during builds. Locate the go.mod entries referencing github.com/mholt/archives and github.com/STARRY-S/zip and either swap to a smaller alternative or add CI config (dependency scanner) and Dependabot/renovate config plus an SBOM step (e.g., syft) to your build workflow.
66-66: Note the use of pseudo-versioned indirect dependencies with inactive upstream libraries.Two indirect dependencies rely on pseudo-versioned commits rather than tagged releases:
github.com/dsnet/compress– last commit Sep 4, 2023 (dormant ~2.5 years)go4.org– commits only in Feb 2023 and Jan 2026; no commits throughout 2024–2025While
github.com/mholt/archives(the direct parent dependency controlling these transitive versions) is actively maintained with recent activity through Oct 2025, the dormancy of these underlying libraries may limit future updates or security patches if vulnerabilities are discovered. Monitor these transitive dependencies for any security issues.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` at line 66, The go.mod contains pseudo-versioned indirect modules (github.com/dsnet/compress and go4.org) which are potentially dormant; update go.mod to pin supported/tagged releases or explicitly require safer versions and/or add replace directives to known-good forks if official tags are unavailable, and try bumping the direct parent module (github.com/mholt/archives) to a newer release so its transitive constraints refresh; locate references to github.com/dsnet/compress, go4.org, and github.com/mholt/archives in go.mod and replace or require explicit versions (or add replace rules) and run go mod tidy to verify the graph and ensure builds/tests pass.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/qui/main.go`:
- Around line 436-437: The calls to command.MarkFlagRequired("source-dir") and
command.MarkFlagRequired("qbit-dir") ignore returned errors causing a linter
failure; either handle the error or explicitly acknowledge it. Fix by capturing
each returned error (e := command.MarkFlagRequired(...)) and then use a
must-pattern (panic/log.Fatalf on non-nil) or assign to the blank identifier (_
= command.MarkFlagRequired(...)) to show the return value is intentionally
ignored; reference the MarkFlagRequired calls on the command variable to locate
and update the two lines.
In `@go.mod`:
- Line 26: The dependency list includes the unmaintained module
github.com/zeebo/bencode v1.0.0; review BitTorrent fastresume handling in your
code (places that import or call github.com/zeebo/bencode) and either (a)
replace it with a maintained bencode library (update go.mod to the new module
and adjust code to the new API), or (b) add a short justification comment and
documentation that confirms zeebo/bencode is acceptable for our use case; ensure
all imports, tests, and any call sites referencing github.com/zeebo/bencode are
updated to the chosen alternative or annotated with the decision.
In `@internal/clientmigrate/deluge.go`:
- Around line 152-159: The log lines in the qBittorrent migration block attach
the error twice by calling log.Error().Err(err) and also interpolating err with
%q in Msgf; update the two calls that reference fastResume.Encode
(fastResumeOutFile) and CopyFile (torrentOutFile) to remove the duplicated error
text — either keep .Err(err) and change Msgf to a plain descriptive string (e.g.
"Could not create qBittorrent fastresume file %s") or drop .Err(err) and include
the error only in the formatted message; adjust the Msgf calls and/or the
.Err(err) usage consistently so each log entry contains the error exactly once.
- Around line 88-92: The log call in the type-assertion block (where value is
checked and assigned to strValue) incorrectly uses the "%s" verb for the
interface{} value; change the formatting in the log.Error().Msgf call to use
"%v" (or "%#v" if you want Go-syntax) so non-string types render safely and
predictably when logging the variable named value in the deluge.go
type-assertion block.
In `@internal/clientmigrate/rtorrent.go`:
- Line 160: Update the inline comment formatting in
internal/clientmigrate/rtorrent.go: change the malformed comment "//if
file.Info.Files != nil {" to use a space after the slashes ("// if
file.Info.Files != nil {") so it satisfies the linter; locate the comment near
the handling of file.Info.Files in the function that processes rtorrent file
info and update only the comment text.
- Around line 197-206: The log calls for fastresume and torrent copy duplicate
the error output by using .Err(err) and also formatting err into the message;
update the two logging lines that follow newFastResume.Encode(fastResumeOutFile)
and CopyFile(match, torrentOutFile) to remove the error interpolation from Msgf
(use a static descriptive message that references fastResumeOutFile or
torrentOutFile) and rely on log.Error().Err(err).Msg(...) to include the error;
ensure you keep the existing context (fastResumeOutFile, torrentOutFile, match)
in the message but do not include %q or the err variable inside the format
string.
- Line 5: Replace use of path.Ext with filepath.Ext to ensure OS-native path
handling: remove the "path" import from the imports block and update the call
site that currently uses path.Ext (the rTorrent migration code where file
extension is computed) to call filepath.Ext instead; keep the existing filepath
import and update any references to the old package name.
- Around line 32-47: The RTorrentImport.Migrate method validates the source
directory but never ensures the qBittorrent output directory exists, causing
writes of .fastresume files to fail; after the sourceDirInfo.IsDir() check in
RTorrentImport.Migrate, call MkDirIfNotExists(i.opts.QbitDir) (same helper used
by the Deluge importer) and handle/return any error from that call so the
QbitDir is created before writing files.
In `@internal/clientmigrate/transmission.go`:
- Line 26: Replace the OS-unsafe string concatenation that builds torrentsDir
from i.opts.SourceDir (torrentsDir := i.opts.SourceDir + "/torrents") with a
call to filepath.Join to construct the path portably; import "path/filepath" if
not already imported and update the assignment to use
filepath.Join(i.opts.SourceDir, "torrents") in the transmission.go code where
torrentsDir is set.
- Line 147: Update the inline comment to follow linter style by adding a space
after the comment slashes: change the comment "//if file.Info.Files != nil {" to
"// if file.Info.Files != nil {" wherever it appears (search for the commented
check referencing file.Info.Files in transmission.go) so the linter no longer
flags `//if` formatting.
- Around line 184-193: The log lines in the block that calls
newFastResume.Encode and CopyFile double-report the error by using .Err(err) and
also formatting the error into the Msgf string; update the two error logs to use
one approach consistently: either keep .Err(err) and remove the %q/%v error
interpolation from the Msgf message, or remove .Err(err) and include the error
string in Msgf. Locate the calls to newFastResume.Encode(...) and CopyFile(...)
and modify the corresponding log.Error().Err(err).Msgf(...) calls so the error
is reported only once (prefer keeping .Err(err) and simplifying the Msgf text to
omit the error formatting).
- Around line 25-49: The TransmissionImport.Migrate function fails to ensure the
qBittorrent output directory exists; add a call to
MkDirIfNotExists(i.opts.QbitDir) (the same helper used by the Deluge importer)
after validating torrentsDir and before processing/writing torrents so writes
won't fail when QbitDir is missing; handle and return any error from
MkDirIfNotExists wrapped/descriptive (use errors.Wrapf) to match existing error
handling.
---
Duplicate comments:
In `@internal/clientmigrate/rtorrent.go`:
- Around line 76-79: The dry-run branch currently logs "successfully imported"
which incorrectly implies an actual import; update the log inside the
i.opts.DryRun branch (the call to log.Info().Msgf that uses positionNum,
totalJobs, torrentID) to clearly indicate this is a simulation (e.g. "dry-run:
would import" or "dry-run: simulated import") so it does not claim a real import
occurred; ensure you only change the log message text and keep the same
variables and formatting.
- Around line 169-176: The current logic uses
strings.ReplaceAll(rtFile.Directory, metaInfo.Name, "") which is fragile;
instead detect and remove the trailing torrent folder only if it exactly matches
the final path segment: use filepath.Clean(rtFile.Directory) and check if
filepath.Base(cleanDir) == metaInfo.Name, then set newPath =
strings.TrimSuffix(cleanDir, string(os.PathSeparator)+metaInfo.Name) (or use
filepath.Dir(cleanDir) to get parent) so newFastResume.Path,
newFastResume.SavePath and newFastResume.QbtSavePath get the parent directory
only when the basename equals metaInfo.Name; otherwise leave rtFile.Directory
unchanged.
In `@internal/clientmigrate/transmission.go`:
- Around line 156-163: The code uses strings.ReplaceAll to remove metaInfo.Name
from resumeFile.Destination which can wrongly remove occurrences elsewhere in
the path; replace this with a path-aware operation: use path/filepath to Clean
resumeFile.Destination, check if it ends with the torrent folder component (e.g.
use filepath.Base(resumeFile.Destination) == metaInfo.Name or
strings.HasSuffix(resumeFile.Destination,
string(os.PathSeparator)+metaInfo.Name)), and if so remove only that trailing
segment (e.g. newPath = filepath.Dir(resumeFile.Destination)); otherwise leave
the directory untouched; then assign newPath to newFastResume.Path,
newFastResume.SavePath and newFastResume.QbtSavePath so you only strip the final
folder safely.
---
Nitpick comments:
In `@go.mod`:
- Line 56: The new dependency github.com/mholt/archives (and the indirect
github.com/STARRY-S/zip entry) increases transitive dependencies; decide whether
full multi-format support is required and if not replace it with a narrower
library or vendor only needed codecs, otherwise add CI/ops controls: add
dependency scanning to pipelines, enable Dependabot/Renovate for go.mod, and
generate an SBOM during builds. Locate the go.mod entries referencing
github.com/mholt/archives and github.com/STARRY-S/zip and either swap to a
smaller alternative or add CI config (dependency scanner) and
Dependabot/renovate config plus an SBOM step (e.g., syft) to your build
workflow.
- Line 66: The go.mod contains pseudo-versioned indirect modules
(github.com/dsnet/compress and go4.org) which are potentially dormant; update
go.mod to pin supported/tagged releases or explicitly require safer versions
and/or add replace directives to known-good forks if official tags are
unavailable, and try bumping the direct parent module
(github.com/mholt/archives) to a newer release so its transitive constraints
refresh; locate references to github.com/dsnet/compress, go4.org, and
github.com/mholt/archives in go.mod and replace or require explicit versions (or
add replace rules) and run go mod tidy to verify the graph and ensure
builds/tests pass.
In `@internal/clientmigrate/deluge.go`:
- Around line 170-181: The function decodeFastresumeFile uses
map[string]interface{} which the linter wants modernized; change all occurrences
in the function signature, local variable declaration, and return types from
map[string]interface{} to map[string]any (e.g., update the return type of
decodeFastresumeFile, the fastresumeFile variable declaration, and any
downstream uses expecting that type) so the code compiles and satisfies the "use
any" lint rule while preserving the existing dynamic-key behavior.
| command.MarkFlagRequired("source-dir") | ||
| command.MarkFlagRequired("qbit-dir") |
There was a problem hiding this comment.
Unchecked return values from MarkFlagRequired (linter failure).
The static analysis check fails because the error returns from MarkFlagRequired are discarded. While these only fail on programmer error (mismatched flag name), the CI pipeline will reject this.
🐛 Proposed fix — use must-pattern or assign to underscore explicitly
- command.MarkFlagRequired("source-dir")
- command.MarkFlagRequired("qbit-dir")
+ _ = command.MarkFlagRequired("source-dir")
+ _ = command.MarkFlagRequired("qbit-dir")🧰 Tools
🪛 GitHub Check: backend
[failure] 437-437:
Error return value of command.MarkFlagRequired is not checked (errcheck)
[failure] 436-436:
Error return value of command.MarkFlagRequired is not checked (errcheck)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/qui/main.go` around lines 436 - 437, The calls to
command.MarkFlagRequired("source-dir") and command.MarkFlagRequired("qbit-dir")
ignore returned errors causing a linter failure; either handle the error or
explicitly acknowledge it. Fix by capturing each returned error (e :=
command.MarkFlagRequired(...)) and then use a must-pattern (panic/log.Fatalf on
non-nil) or assign to the blank identifier (_ = command.MarkFlagRequired(...))
to show the return value is intentionally ignored; reference the
MarkFlagRequired calls on the command variable to locate and update the two
lines.
| github.com/klauspost/compress v1.18.3 | ||
| github.com/lithammer/fuzzysearch v1.1.8 | ||
| github.com/mat/besticon/v3 v3.21.0 | ||
| github.com/mholt/archives v0.1.5 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's confirm the exact lines in go.mod
head -40 go.mod | tail -20Repository: autobrr/qui
Length of output: 728
🌐 Web query:
github.com/mholt/archives latest release version security
💡 Result:
-
Latest release (github.com/mholt/archives): v0.1.5 (released Oct 13; GitHub “Latest” tag). [1]
- pkg.go.dev also lists v0.1.5 as the latest tagged version (published Oct 9, 2025). [2]
-
Security status (repo-level):
- No published GitHub Security Advisories for
mholt/archives. [3] - No
SECURITY.mdpolicy is set in the repo. [4]
- No published GitHub Security Advisories for
Sources: [1] [2] [3] [4]
🌐 Web query:
github.com/zeebo/bencode latest release version
💡 Result:
The latest tagged version of github.com/zeebo/bencode is v1.0.0 (tag date: Aug 19, 2018). [1]
While github.com/mholt/archives v0.1.5 is the current latest version and has no known security advisories, github.com/zeebo/bencode v1.0.0 is over 7 years old with no recent updates. For BitTorrent fastresume file handling, consider evaluating whether this library is still adequate or if a more actively maintained alternative exists. If this library is confirmed to be stable and suitable for your use case, document the decision; otherwise, investigate modern bencode implementations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@go.mod` at line 26, The dependency list includes the unmaintained module
github.com/zeebo/bencode v1.0.0; review BitTorrent fastresume handling in your
code (places that import or call github.com/zeebo/bencode) and either (a)
replace it with a maintained bencode library (update go.mod to the new module
and adjust code to the new API), or (b) add a short justification comment and
documentation that confirms zeebo/bencode is acceptable for our use case; ensure
all imports, tests, and any call sites referencing github.com/zeebo/bencode are
updated to the chosen alternative or annotated with the decision.
| strValue, ok := value.(string) | ||
| if !ok { | ||
| log.Error().Msgf("Could not convert value %s to string", value) | ||
| continue | ||
| } |
There was a problem hiding this comment.
Use %v instead of %s when formatting an any value.
Line 90 uses %s to format value which is interface{}/any. The %s verb may not produce useful output for non-string types; %v is safer.
🐛 Proposed fix
- log.Error().Msgf("Could not convert value %s to string", value)
+ log.Error().Msgf("Could not convert value %v to string", value)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/clientmigrate/deluge.go` around lines 88 - 92, The log call in the
type-assertion block (where value is checked and assigned to strValue)
incorrectly uses the "%s" verb for the interface{} value; change the formatting
in the log.Error().Msgf call to use "%v" (or "%#v" if you want Go-syntax) so
non-string types render safely and predictably when logging the variable named
value in the deluge.go type-assertion block.
| if err = fastResume.Encode(fastResumeOutFile); err != nil { | ||
| log.Error().Err(err).Msgf("Could not create qBittorrent fastresume file %s error: %q", fastResumeOutFile, err) | ||
| continue | ||
| } | ||
|
|
||
| if err = CopyFile(fastResume.TorrentFilePath, torrentOutFile); err != nil { | ||
| log.Error().Err(err).Msgf("Could not copy qBittorrent torrent file %s error %q", torrentOutFile, err) | ||
| continue |
There was a problem hiding this comment.
Error is logged twice — via .Err(err) and %q format verb.
Both lines 153 and 158 attach the error with .Err(err) and interpolate it again with %q in the message format string. This produces duplicate error text in the log output.
🐛 Proposed fix
if err = fastResume.Encode(fastResumeOutFile); err != nil {
- log.Error().Err(err).Msgf("Could not create qBittorrent fastresume file %s error: %q", fastResumeOutFile, err)
+ log.Error().Err(err).Msgf("Could not create qBittorrent fastresume file %s", fastResumeOutFile)
continue
}
if err = CopyFile(fastResume.TorrentFilePath, torrentOutFile); err != nil {
- log.Error().Err(err).Msgf("Could not copy qBittorrent torrent file %s error %q", torrentOutFile, err)
+ log.Error().Err(err).Msgf("Could not copy qBittorrent torrent file %s", torrentOutFile)
continue
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/clientmigrate/deluge.go` around lines 152 - 159, The log lines in
the qBittorrent migration block attach the error twice by calling
log.Error().Err(err) and also interpolating err with %q in Msgf; update the two
calls that reference fastResume.Encode (fastResumeOutFile) and CopyFile
(torrentOutFile) to remove the duplicated error text — either keep .Err(err) and
change Msgf to a plain descriptive string (e.g. "Could not create qBittorrent
fastresume file %s") or drop .Err(err) and include the error only in the
formatted message; adjust the Msgf calls and/or the .Err(err) usage consistently
so each log entry contains the error exactly once.
|
|
||
| import ( | ||
| "os" | ||
| "path" |
There was a problem hiding this comment.
Use filepath.Ext instead of path.Ext for OS-correct path handling.
The path package is for slash-separated (URL/POSIX) paths. On Windows, path.Ext may not correctly handle backslash-separated paths. Since filepath is already imported, use filepath.Ext on line 246 and drop the path import.
🐛 Proposed fix
import (
"os"
- "path"
"path/filepath"
"strconv"And on line 246:
- trimmed := strings.TrimSuffix(fileName, path.Ext(fileName))
+ trimmed := strings.TrimSuffix(fileName, filepath.Ext(fileName))📝 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.
| "path" | |
| import ( | |
| "os" | |
| "path/filepath" | |
| "strconv" | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/clientmigrate/rtorrent.go` at line 5, Replace use of path.Ext with
filepath.Ext to ensure OS-native path handling: remove the "path" import from
the imports block and update the call site that currently uses path.Ext (the
rTorrent migration code where file extension is computed) to call filepath.Ext
instead; keep the existing filepath import and update any references to the old
package name.
| fastResumeOutFile := filepath.Join(i.opts.QbitDir, torrentID+".fastresume") | ||
| if err = newFastResume.Encode(fastResumeOutFile); err != nil { | ||
| log.Error().Err(err).Msgf("Could not create qBittorrent fastresume file %s error: %q", fastResumeOutFile, err) | ||
| continue | ||
| } | ||
|
|
||
| if err = CopyFile(match, torrentOutFile); err != nil { | ||
| log.Error().Err(err).Msgf("Could not copy qBittorrent torrent file %s error %q", torrentOutFile, err) | ||
| continue | ||
| } |
There was a problem hiding this comment.
Error is logged twice — via .Err(err) and %q format verb.
Same pattern as in deluge.go. Both lines 199 and 204 attach the error with .Err(err) and interpolate it again in the message string.
🐛 Proposed fix
if err = newFastResume.Encode(fastResumeOutFile); err != nil {
- log.Error().Err(err).Msgf("Could not create qBittorrent fastresume file %s error: %q", fastResumeOutFile, err)
+ log.Error().Err(err).Msgf("Could not create qBittorrent fastresume file %s", fastResumeOutFile)
continue
}
if err = CopyFile(match, torrentOutFile); err != nil {
- log.Error().Err(err).Msgf("Could not copy qBittorrent torrent file %s error %q", torrentOutFile, err)
+ log.Error().Err(err).Msgf("Could not copy qBittorrent torrent file %s", torrentOutFile)
continue
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/clientmigrate/rtorrent.go` around lines 197 - 206, The log calls for
fastresume and torrent copy duplicate the error output by using .Err(err) and
also formatting err into the message; update the two logging lines that follow
newFastResume.Encode(fastResumeOutFile) and CopyFile(match, torrentOutFile) to
remove the error interpolation from Msgf (use a static descriptive message that
references fastResumeOutFile or torrentOutFile) and rely on
log.Error().Err(err).Msg(...) to include the error; ensure you keep the existing
context (fastResumeOutFile, torrentOutFile, match) in the message but do not
include %q or the err variable inside the format string.
| func (i *TransmissionImport) Migrate() error { | ||
| torrentsDir := i.opts.SourceDir + "/torrents" | ||
|
|
||
| sourceDirInfo, err := os.Stat(torrentsDir) | ||
| if err != nil { | ||
| if os.IsNotExist(err) { | ||
| return errors.Errorf("source directory does not exist: %s", torrentsDir) | ||
| } | ||
|
|
||
| return errors.Wrapf(err, "source directory error: %s", torrentsDir) | ||
| } | ||
|
|
||
| if !sourceDirInfo.IsDir() { | ||
| return errors.Errorf("source is a file, not a directory: %s", torrentsDir) | ||
| } | ||
|
|
||
| matches, err := filepath.Glob(filepath.Join(torrentsDir, "*.torrent")) | ||
| if err != nil { | ||
| return errors.Wrapf(err, "glob error: %s", torrentsDir) | ||
| } | ||
|
|
||
| if len(matches) == 0 { | ||
| log.Info().Msgf("Found 0 files to process in: %s", torrentsDir) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Missing MkDirIfNotExists for the qBittorrent output directory.
Same issue as in the RTorrent importer — the Deluge importer creates QbitDir before writing, but the Transmission importer does not. Writes will fail if the directory doesn't exist.
🐛 Proposed fix — add after directory validation
if !sourceDirInfo.IsDir() {
return errors.Errorf("source is a file, not a directory: %s", torrentsDir)
}
+
+ if !i.opts.DryRun {
+ if err := MkDirIfNotExists(i.opts.QbitDir); err != nil {
+ return errors.Wrapf(err, "qbit directory error: %s", i.opts.QbitDir)
+ }
+ }📝 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.
| func (i *TransmissionImport) Migrate() error { | |
| torrentsDir := i.opts.SourceDir + "/torrents" | |
| sourceDirInfo, err := os.Stat(torrentsDir) | |
| if err != nil { | |
| if os.IsNotExist(err) { | |
| return errors.Errorf("source directory does not exist: %s", torrentsDir) | |
| } | |
| return errors.Wrapf(err, "source directory error: %s", torrentsDir) | |
| } | |
| if !sourceDirInfo.IsDir() { | |
| return errors.Errorf("source is a file, not a directory: %s", torrentsDir) | |
| } | |
| matches, err := filepath.Glob(filepath.Join(torrentsDir, "*.torrent")) | |
| if err != nil { | |
| return errors.Wrapf(err, "glob error: %s", torrentsDir) | |
| } | |
| if len(matches) == 0 { | |
| log.Info().Msgf("Found 0 files to process in: %s", torrentsDir) | |
| return nil | |
| } | |
| func (i *TransmissionImport) Migrate() error { | |
| torrentsDir := i.opts.SourceDir + "/torrents" | |
| sourceDirInfo, err := os.Stat(torrentsDir) | |
| if err != nil { | |
| if os.IsNotExist(err) { | |
| return errors.Errorf("source directory does not exist: %s", torrentsDir) | |
| } | |
| return errors.Wrapf(err, "source directory error: %s", torrentsDir) | |
| } | |
| if !sourceDirInfo.IsDir() { | |
| return errors.Errorf("source is a file, not a directory: %s", torrentsDir) | |
| } | |
| if !i.opts.DryRun { | |
| if err := MkDirIfNotExists(i.opts.QbitDir); err != nil { | |
| return errors.Wrapf(err, "qbit directory error: %s", i.opts.QbitDir) | |
| } | |
| } | |
| matches, err := filepath.Glob(filepath.Join(torrentsDir, "*.torrent")) | |
| if err != nil { | |
| return errors.Wrapf(err, "glob error: %s", torrentsDir) | |
| } | |
| if len(matches) == 0 { | |
| log.Info().Msgf("Found 0 files to process in: %s", torrentsDir) | |
| return nil | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/clientmigrate/transmission.go` around lines 25 - 49, The
TransmissionImport.Migrate function fails to ensure the qBittorrent output
directory exists; add a call to MkDirIfNotExists(i.opts.QbitDir) (the same
helper used by the Deluge importer) after validating torrentsDir and before
processing/writing torrents so writes won't fail when QbitDir is missing; handle
and return any error from MkDirIfNotExists wrapped/descriptive (use
errors.Wrapf) to match existing error handling.
| } | ||
|
|
||
| func (i *TransmissionImport) Migrate() error { | ||
| torrentsDir := i.opts.SourceDir + "/torrents" |
There was a problem hiding this comment.
Use filepath.Join instead of string concatenation for path construction.
i.opts.SourceDir + "/torrents" uses a hard-coded forward slash, which isn't OS-portable. Use filepath.Join for consistent behavior.
🐛 Proposed fix
- torrentsDir := i.opts.SourceDir + "/torrents"
+ torrentsDir := filepath.Join(i.opts.SourceDir, "torrents")📝 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.
| torrentsDir := i.opts.SourceDir + "/torrents" | |
| torrentsDir := filepath.Join(i.opts.SourceDir, "torrents") |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/clientmigrate/transmission.go` at line 26, Replace the OS-unsafe
string concatenation that builds torrentsDir from i.opts.SourceDir (torrentsDir
:= i.opts.SourceDir + "/torrents") with a call to filepath.Join to construct the
path portably; import "path/filepath" if not already imported and update the
assignment to use filepath.Join(i.opts.SourceDir, "torrents") in the
transmission.go code where torrentsDir is set.
| //Path: resumeFile.Destination, | ||
| } | ||
|
|
||
| //if file.Info.Files != nil { |
There was a problem hiding this comment.
Fix comment formatting per linter.
Static analysis flags: //if should be // if.
- //if file.Info.Files != nil {
+ // if file.Info.Files != nil {📝 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.
| //if file.Info.Files != nil { | |
| // if file.Info.Files != nil { |
🧰 Tools
🪛 GitHub Check: backend
[failure] 147-147:
commentFormatting: put a space between // and comment text (gocritic)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/clientmigrate/transmission.go` at line 147, Update the inline
comment to follow linter style by adding a space after the comment slashes:
change the comment "//if file.Info.Files != nil {" to "// if file.Info.Files !=
nil {" wherever it appears (search for the commented check referencing
file.Info.Files in transmission.go) so the linter no longer flags `//if`
formatting.
| fastResumeOutFile := filepath.Join(i.opts.QbitDir, torrentID+".fastresume") | ||
| if err = newFastResume.Encode(fastResumeOutFile); err != nil { | ||
| log.Error().Err(err).Msgf("Could not create qBittorrent fastresume file %s error: %q", fastResumeOutFile, err) | ||
| continue | ||
| } | ||
|
|
||
| if err = CopyFile(match, torrentOutFile); err != nil { | ||
| log.Error().Err(err).Msgf("Could not copy qBittorrent torrent file %s error %q", torrentOutFile, err) | ||
| continue | ||
| } |
There was a problem hiding this comment.
Error is logged twice — via .Err(err) and %q format verb.
Same double-logging pattern as in the other importers.
🐛 Proposed fix
if err = newFastResume.Encode(fastResumeOutFile); err != nil {
- log.Error().Err(err).Msgf("Could not create qBittorrent fastresume file %s error: %q", fastResumeOutFile, err)
+ log.Error().Err(err).Msgf("Could not create qBittorrent fastresume file %s", fastResumeOutFile)
continue
}
if err = CopyFile(match, torrentOutFile); err != nil {
- log.Error().Err(err).Msgf("Could not copy qBittorrent torrent file %s error %q", torrentOutFile, err)
+ log.Error().Err(err).Msgf("Could not copy qBittorrent torrent file %s", torrentOutFile)
continue
}📝 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.
| fastResumeOutFile := filepath.Join(i.opts.QbitDir, torrentID+".fastresume") | |
| if err = newFastResume.Encode(fastResumeOutFile); err != nil { | |
| log.Error().Err(err).Msgf("Could not create qBittorrent fastresume file %s error: %q", fastResumeOutFile, err) | |
| continue | |
| } | |
| if err = CopyFile(match, torrentOutFile); err != nil { | |
| log.Error().Err(err).Msgf("Could not copy qBittorrent torrent file %s error %q", torrentOutFile, err) | |
| continue | |
| } | |
| fastResumeOutFile := filepath.Join(i.opts.QbitDir, torrentID+".fastresume") | |
| if err = newFastResume.Encode(fastResumeOutFile); err != nil { | |
| log.Error().Err(err).Msgf("Could not create qBittorrent fastresume file %s", fastResumeOutFile) | |
| continue | |
| } | |
| if err = CopyFile(match, torrentOutFile); err != nil { | |
| log.Error().Err(err).Msgf("Could not copy qBittorrent torrent file %s", torrentOutFile) | |
| continue | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/clientmigrate/transmission.go` around lines 184 - 193, The log lines
in the block that calls newFastResume.Encode and CopyFile double-report the
error by using .Err(err) and also formatting the error into the Msgf string;
update the two error logs to use one approach consistently: either keep
.Err(err) and remove the %q/%v error interpolation from the Msgf message, or
remove .Err(err) and include the error string in Msgf. Locate the calls to
newFastResume.Encode(...) and CopyFile(...) and modify the corresponding
log.Error().Err(err).Msgf(...) calls so the error is reported only once (prefer
keeping .Err(err) and simplifying the Msgf text to omit the error formatting).
1 participant
Implement client migrator from Deluge, rTorrent and Transmission to qBittorrent with stats.
Summary by CodeRabbit
New Features
Chores