feat: add global and per download rate limiting to surge

[junaid2005p]

Greptile Summary

This PR adds a complete rate-limiting system to Surge: a custom token-bucket RateLimiter and MultiLimiter, per-download and global/default limit controls wired through the worker pool, HTTP API endpoints, a new surge limit CLI command, and a "Speed Limits" tab in the TUI settings.

• Pool / engine: WorkerPool gains SetGlobalRateLimit, SetDefaultDownloadRateLimit, SetDownloadRateLimit, and ClearDownloadRateLimit; the concurrent downloader reads from d.State.GetRateLimit() at pause time so snapshots are no longer stale.
• HTTP API: three new endpoints with an inherit shortcut routing to ClearRateLimit; parseRateLimitQuery accepts raw byte integers (the CLI converts human-readable units client-side).
• TUI: new Speed Limits settings tab; validateSetting now handles dl:<id> keys, fixing a previously-flagged silent validation failure.

Confidence Score: 3/5

Large new feature with lingering concurrency edge cases from prior review rounds that have only been partially addressed.

Many previously-flagged issues (stale limiter on resume, stale rate in pause snapshot, double-unlock, silent TUI validation for dl: keys) are addressed. However, silent success for non-existent IDs and the TOCTOU window in ClearDownloadRateLimit remain open. SetDefaultDownloadRateLimit also nests both pool mutexes while all other functions keep them sequential.

internal/download/pool.go and internal/core/local_service.go

Important Files Changed

Filename	Overview
internal/download/pool.go	Core rate limiting logic added; fixes several previously-flagged bugs. SetDefaultDownloadRateLimit now calls SetRate on active limiters but nests both pool mutexes unconventionally.
cmd/http_api.go	Three new HTTP endpoints added; inherit path now correctly routes to ClearRateLimit before parseRateLimitQuery.
cmd/limit.go	New CLI command for per-download, global, and default rate limits. Correctly parses human-readable units to bytes before sending to the HTTP API.
internal/core/local_service.go	SetRateLimit, ClearRateLimit, SetGlobalRateLimit, SetDefaultRateLimit added. updateRateLimitSetting uses JSON copy to avoid data race. Several previously-flagged issues appear addressed.
internal/engine/rate_limiter.go	New token-bucket RateLimiter with wake-on-SetRate; SetRate correctly settles the old rate before switching.
internal/tui/update_settings.go	validateSetting now handles the dl: prefix for per-download rate limits, fixing the previously-flagged silent validation failure.
internal/config/settings.go	Speed Limits category added to initializeCategoriesList between Network and Performance, but CategoryOrder appends it last after Extension.
internal/utils/rate_limit.go	ParseRateLimit supports a wide set of units with correct bit-to-byte conversion; FormatRateLimit produces human-readable output.

Comments Outside Diff (3)

1. internal/download/pool.go, line 366-414 (link)

   downloadLimiters entries are never removed

   ensureLimiterForConfig inserts a *RateLimiter into downloadLimiters for every new download ID, but Cancel, the completion branch in worker, and GracefulShutdown never call delete(p.downloadLimiters, id). For a long-running Surge daemon processing many downloads, this map grows without bound.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: internal/download/pool.go
   Line: 366-414
   
   Comment:
   **`downloadLimiters` entries are never removed**
   
   `ensureLimiterForConfig` inserts a `*RateLimiter` into `downloadLimiters` for every new download ID, but `Cancel`, the completion branch in `worker`, and `GracefulShutdown` never call `delete(p.downloadLimiters, id)`. For a long-running Surge daemon processing many downloads, this map grows without bound.
   
   How can I resolve this? If you propose a fix, please make it concise.

2. internal/config/settings.go, line 322 (link)

   The "Speed Limits" category is inserted between the Network and Performance sections in initializeCategoriesList, but CategoryOrder appends it after "Extension" (the last tab). Users navigating the settings TUI will find Network-related rate limit options under the very last tab rather than next to the other Network settings.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: internal/config/settings.go
   Line: 322
   
   Comment:
   The "Speed Limits" category is inserted between the Network and Performance sections in `initializeCategoriesList`, but `CategoryOrder` appends it after "Extension" (the last tab). Users navigating the settings TUI will find Network-related rate limit options under the very last tab rather than next to the other Network settings.
   
   
   
   How can I resolve this? If you propose a fix, please make it concise.

   Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

3. internal/download/pool.go, line 566-574 (link)

   Stale cfg.Limiter makes rate-limit changes invisible after hot-path resume

   ExtractPausedConfig deletes the download's entry from downloadLimiters, but the returned cfg still carries cfg.Limiter — the old *MultiLimiter whose inner per-download *RateLimiter is now orphaned (no longer tracked in the map). When Add calls ensureLimiterForConfig, the guard if cfg.Limiter == nil prevents the new limiter from being installed, so the resumed download executes against the orphaned limiter. Any subsequent surge limit <id> X updates downloadLimiters[id] (the new limiter) but has zero effect on the running download. The fix is to nil out cfg.Limiter before returning, so ensureLimiterForConfig always wires the fresh limiter that it installs into downloadLimiters.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: internal/download/pool.go
   Line: 566-574
   
   Comment:
   **Stale `cfg.Limiter` makes rate-limit changes invisible after hot-path resume**
   
   `ExtractPausedConfig` deletes the download's entry from `downloadLimiters`, but the returned `cfg` still carries `cfg.Limiter` — the old `*MultiLimiter` whose inner per-download `*RateLimiter` is now orphaned (no longer tracked in the map). When `Add` calls `ensureLimiterForConfig`, the guard `if cfg.Limiter == nil` prevents the new limiter from being installed, so the resumed download executes against the orphaned limiter. Any subsequent `surge limit <id> X` updates `downloadLimiters[id]` (the new limiter) but has zero effect on the running download. The fix is to nil out `cfg.Limiter` before returning, so `ensureLimiterForConfig` always wires the fresh limiter that it installs into `downloadLimiters`.
   
   
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
internal/download/pool.go:329-368
**Nested lock acquisition is the only place both mutexes are held simultaneously**

`SetDefaultDownloadRateLimit` holds `p.mu` (via `defer`) and then acquires `p.limiterMu` (also via `defer`), keeping both held until the function returns. Every other path in the pool acquires them sequentially: `Add` releases `limiterMu` before taking `p.mu`; `SetDownloadRateLimit` releases `p.mu` before taking `p.limiterMu`.

There is no current deadlock because no goroutine holds `p.limiterMu` while waiting for `p.mu`. However, this is the only site that nests the two locks, making the invariant non-obvious and fragile for future contributors. Consider splitting the function so `p.mu` is fully released before `p.limiterMu` is acquired.

### Issue 2 of 2
internal/config/settings.go:355-356
**"Speed Limits" tab appears last in the TUI but is grouped with Network settings in `initializeCategoriesList`**

`initializeCategoriesList` inserts "Speed Limits" immediately after Network (before Performance), reflecting its logical relationship. However, `CategoryOrder` appends it last, after "Extension". Users will find rate limit settings in the wrong tab. The order should be: `["General", "Network", "Speed Limits", "Performance", "Categories", "Extension"]`.

_{Reviews (13): Last reviewed commit: "refactor: removed dead code branch" | Re-trigger Greptile}

[github-actions]

Binary Size Analysis

⚠️ Size Increased

Version	Human Readable	Raw Bytes
Main	19.85 MB	20812068
PR	19.96 MB	20926756
Difference	112.00 KB	114688

[SuperCoolPencil]

Let me pull this, tell it out and get back to you by EOW. I'm having a really hectic week rn :|

[junaid2005p]

@greptileai
The remaining finding is a redundant type assertion whose unreachable error branch is dead code.
where is this?

[greptile-apps]

It's in the clearDownloadRateLimit function at the bottom of internal/tui/view_settings.go:

func (m *RootModel) clearDownloadRateLimit(downloadID string) error {
	if m.Service == nil {
		return nil
	}
	clearer, ok := m.Service.(interface{ ClearRateLimit(string) error })  // <-- redundant assertion
	if !ok {
		return fmt.Errorf("service does not support clearing download rate limits")  // <-- unreachable
	}
	return clearer.ClearRateLimit(downloadID)
}

Since m.Service is typed as core.DownloadService and ClearRateLimit(string) error is already a required method on that interface, the type assertion always succeeds — the !ok branch can never be reached. The fix is to call the method directly:

func (m *RootModel) clearDownloadRateLimit(downloadID string) error {
	if m.Service == nil {
		return nil
	}
	return m.Service.ClearRateLimit(downloadID)
}

[SuperCoolPencil]

@junaid2005p gonna make a couple of modifications to the TUI which I feel will look better!

[junaid2005p]

Go ahead @SuperCoolPencil

[SuperCoolPencil]

I also think we do not need a new settings section for this, we could just have a modal

[junaid2005p]

Interesting I will consider and let you know

A modal is generally fine but a settings tab makes it easier to persist the speed limits throughout sessions

[SuperCoolPencil]

@greptileai

can you set limit for a specific download?

[greptile-apps]

Yes — this PR adds exactly that. You can set a per-download rate limit with:

surge limit <id> 2MB/s

You can also reset it back to the default with:

surge limit <id> inherit

The full set of options:

Command	Effect
surge limit <id> 2MB/s	Caps that specific download at 2 MB/s
surge limit <id> unlimited	Removes the per-download cap
surge limit <id> inherit	Falls back to the default rate limit
surge limit --default 2MB/s	Sets the default cap for all downloads
surge limit --global 10MB/s	Sets an overall cap across all downloads

Human-readable units (KB/s, MB/s, Mbps, etc.) are parsed client-side and sent as raw bytes/s to the server.

[SuperCoolPencil]

@junaid2005p

Proposed modal:

Proposed changes to TUI (global speed and limit appears in footer)