feat: add BitTorrent engine with magnet link and .torrent file support#326
Open
mvanhorn wants to merge 3 commits intoSurgeDM:mainfrom
Open
feat: add BitTorrent engine with magnet link and .torrent file support#326mvanhorn wants to merge 3 commits intoSurgeDM:mainfrom
mvanhorn wants to merge 3 commits intoSurgeDM:mainfrom
Conversation
Add a torrent engine package wrapping anacrolix/torrent (6K stars, used by Gopeed and bitmagnet). Supports magnet URIs, .torrent files, DHT, PEX, and protocol encryption. Downloads report progress with peer count and speed metrics. The engine is modular per community request on SurgeDM#170. When Torrent is disabled in settings (the default), the engine is not initialized. Torrent settings include seed ratio, max peers, and an enable toggle. Wiring into the download lifecycle, CLI, and TUI display will follow in separate PRs. This PR establishes the core engine and settings. Closes SurgeDM#170 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Binary Size Analysis
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+71
to
+87
| // AddMagnet adds a magnet link and returns a handle to monitor progress. | ||
| func (e *Engine) AddMagnet(magnetURI string) (*torrent.Torrent, error) { | ||
| t, err := e.client.AddMagnet(magnetURI) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("torrent: failed to add magnet: %w", err) | ||
| } | ||
| return t, nil | ||
| } | ||
|
|
||
| // AddTorrentFile adds a .torrent file and returns a handle. | ||
| func (e *Engine) AddTorrentFile(path string) (*torrent.Torrent, error) { | ||
| t, err := e.client.AddTorrentFromFile(path) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("torrent: failed to add torrent file: %w", err) | ||
| } | ||
| return t, nil | ||
| } |
Contributor
There was a problem hiding this comment.
anacrolix/torrent type across the package boundary
AddMagnet, AddTorrentFile, and Download all expose or accept *torrent.Torrent directly. This means every future caller (download lifecycle, CLI, TUI) must import github.com/anacrolix/torrent, tightly coupling Surge internals to a third-party type. Swapping libraries later would require changes in every call site.
Consider wrapping the handle:
// Handle represents an in-progress torrent download.
type Handle struct {
t *torrent.Torrent
}
func (e *Engine) AddMagnet(magnetURI string) (*Handle, error) { ... }
func (e *Engine) Download(ctx context.Context, h *Handle, progressCh chan<- Progress) error { ... }This keeps the anacrolix/torrent import confined to this package.
Rule Used: What: Enforce separation of concerns by keeping bu... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/engine/torrent/engine.go
Line: 71-87
Comment:
**Public API leaks the `anacrolix/torrent` type across the package boundary**
`AddMagnet`, `AddTorrentFile`, and `Download` all expose or accept `*torrent.Torrent` directly. This means every future caller (download lifecycle, CLI, TUI) must import `github.com/anacrolix/torrent`, tightly coupling Surge internals to a third-party type. Swapping libraries later would require changes in every call site.
Consider wrapping the handle:
```go
// Handle represents an in-progress torrent download.
type Handle struct {
t *torrent.Torrent
}
func (e *Engine) AddMagnet(magnetURI string) (*Handle, error) { ... }
func (e *Engine) Download(ctx context.Context, h *Handle, progressCh chan<- Progress) error { ... }
```
This keeps the `anacrolix/torrent` import confined to this package.
**Rule Used:** What: Enforce separation of concerns by keeping bu... ([source](https://app.greptile.com/review/custom-context?memory=c1138776-52c4-4b49-81a0-1cd68fe194dc))
How can I resolve this? If you propose a fix, please make it concise.The anacrolix/torrent SQLite storage may hold file locks briefly after Close() on Windows, causing t.TempDir() cleanup to fail. Use os.MkdirTemp with defer RemoveAll instead, which tolerates locked files. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
- Fix IsTorrentFile: use exact == comparison instead of HasSuffix - Fix blocking channel send: use non-blocking select/default - Fix done check: require total > 0 to prevent false completion - Wire MaxPeers to EstablishedConnsPerTorrent - Replace descriptive comments with rationale comments - Add positive and edge case tests for IsTorrentFile
Contributor
Author
|
@sfzaw Thanks for sharing those concerns and for the thoughtful writeup. Glad the modular approach addresses them. Addressed greptile findings in a9f317d:
Left the API wrapping (P2 - Verified: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add a BitTorrent engine wrapping anacrolix/torrent that supports magnet URIs and .torrent files. The engine is modular per #170's community request.
Why this matters
Issue #170: "one reason I can't switch yet is because aria2 has torrent support while surge does not." @SuperCoolPencil confirmed this is planned (12 heart reactions on the reply). @sfzaw requested modular design: "please make this feature modular if possible."
anacrolix/torrent (6K stars) is the standard Go BitTorrent library, used by Gopeed (22.8K stars) and bitmagnet (3.9K stars). It supports DHT, PEX, uTP, WebTorrent, WebSeeds, BitTorrent v2, and protocol encryption.
Changes
internal/engine/torrent/package:Enginewrappingtorrent.Clientwith Surge-compatible progress reportingAddMagnet(uri)andAddTorrentFile(path)for adding torrentsDownload(ctx, torrent, progressCh)for progress tracking with peer count and speedIsMagnetURI()andIsTorrentFile()for source detectionTorrentSettingsto config (enabled, seed_ratio, max_peers) with TUI tabEnabled: false(default), the engine is not initialized (modular per request)This is Phase 1: the core engine and settings. Wiring into the download lifecycle, CLI commands, and TUI display will follow in separate PRs.
Closes #170
Testing
This contribution was developed with AI assistance (Codex + Claude Code).
Greptile Summary
This PR adds a BitTorrent engine to Surge by wrapping the
anacrolix/torrentlibrary, supporting magnet URIs and.torrentfiles. It introducesinternal/engine/torrent/(engine, config, tests) and extendsinternal/config/settings.gowithTorrentSettings. The engine is modular (opt-in viaEnabled: falsedefault), and all previously flagged issues from earlier review rounds have been resolved — context cancellation is now non-blocking,IsTorrentFileuses==instead ofHasSuffix,MaxPeersis correctly wired viaEstablishedConnsPerTorrent, and test coverage includes positive and edge cases.Key remaining concerns:
SeedRatiois not enforced at runtime — any non-zero value only toggles seeding on indefinitely; the ratio target is never monitored or enforced, making the setting misleading to users who configure it.SeedTimeexists in engineConfigbut has no counterpart inTorrentSettings— when Phase 2 bridges settings to the engine, this field will silently default to zero.anacrolix/torrentbrings in ~50+ transitive dependencies including a full pion/WebRTC stack, SQLite bindings, and gorilla/websocket, which meaningfully increases binary size and attack surface for a tool with an explicit lightweight goal.Confidence Score: 4/5
Safe to merge for Phase 1 (no production wiring yet), but SeedRatio silently misbehaves when non-zero — misleading to users who configure it.
All previously flagged P0/P1 issues from prior rounds are resolved (non-blocking channel send, IsTorrentFile logic, MaxPeers wiring, done guard, test coverage). One new P1 remains: SeedRatio is exposed with a ratio-limit description but only acts as a binary toggle, meaning users who set a target ratio will get unlimited seeding. This should be addressed before GA, though it does not break Phase 1 functionality since the engine is not yet wired into downloads.
internal/engine/torrent/engine.go (SeedRatio enforcement), internal/engine/torrent/config.go (SeedTime/ListenPort exposure gap), go.mod (dependency footprint)
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[User adds magnet/torrent] --> B{IsMagnetURI?} B -- yes --> C[Engine.AddMagnet] B -- no --> D{IsTorrentFile?} D -- yes --> E[Engine.AddTorrentFile] D -- no --> F[Error: unsupported source] C --> G[anacrolix client.AddMagnet] E --> H[anacrolix client.AddTorrentFromFile] G --> I[torrent.Torrent handle] H --> I I --> J[Engine.Download] J --> K{Wait for GotInfo} K -- timeout 2m --> L[Error: metadata timeout] K -- ctx cancelled --> M[Return ctx.Err] K -- success --> N[t.DownloadAll] N --> O[500ms ticker loop] O --> P{ctx.Done?} P -- yes --> M P -- no --> Q[Compute progress + speed] Q --> R[Non-blocking send to progressCh] R --> S{done = total>0 AND completed>=total} S -- yes --> T[Return nil] S -- no --> O subgraph Settings U[TorrentSettings.Enabled] V[TorrentSettings.SeedRatio] W[TorrentSettings.MaxPeers] X[engine.Config.SeedTime - no UI mapping] Y[engine.Config.ListenPort - no UI mapping] endPrompt To Fix All With AI
Reviews (4): Last reviewed commit: "fix(torrent): address greptile review fi..." | Re-trigger Greptile
Context used: