feat: categorized settings, new options, and consistent download settings across HTTP, FTP and torrent - #263
Merged
Merged
Conversation
AppearanceConfig gains a ThemeMode (system, light, dark) and ServerConfig an autoStart flag plus loopback helpers, so the apps can offer a theme switch, start the server on launch and a this-device-only toggle. Configs written before these fields existed decode to the old behaviour.
The settings page was one long scroll of cards, each with its own Save button. It is now split into categories — General, Downloads, Network, Remote access, AI discovery and About — shown beside the selected page on wide windows and as a list that opens each page on phones (with system back support). Rows are grouped on cards with a compact control each: switches, drop-downs, a segmented theme picker and accent swatches. Changes apply as they are made; text fields save when typing pauses, on Enter, when focus leaves or when the page closes, and only while valid. New options: - Theme: system, light or dark - Retry attempts for failed downloads, and a custom speed limit in KB/s or MB/s with decimals - Network: pick the interfaces HTTP downloads are spread across - Remote access: start automatically, allow other devices or only this one, generate an access token, and restart when saved settings differ from the running server - About: version, build and project links Download and network settings now belong to the active instance (InstanceSettingsController): a remote instance's own values are loaded and changed on the server, instead of this device's values being shown and pushed to it; only the embedded instance's are saved to config.toml. Rapid changes are serialised so the instance ends on the last value. The server now starts with the saved config and keeps it in ServerState.Running, so the status shows the real port (the instance picker assumed 8642) and Settings knows when a restart is needed. A start that throws, e.g. port in use, becomes ServerState.Failed instead of crashing the app.
With unlimited (or more than TorrentConfig.maxActiveTorrents) simultaneous
downloads, the extra torrent tasks failed with a non-retryable SourceError
("Too many active torrents") when no seeding session could be evicted.
Torrent tasks now wait for an engine slot through TorrentActiveSlots:
higher priority first, then arrival order, without polling. Waiting is
cancellable, so pause and cancel return at once, and every exit path
(completion, failure, pause, cancel) hands the slot to the next waiter.
The oldest seeding session still yields its slot first, and a finished
download does not start seeding while another download is waiting.
While waiting, a task reports as downloading at 0 bytes/s with its
restored progress, which also keeps it pausable.
Magnet metadata fetches beyond the runtime's 16 pending fetches now wait
for a permit instead of failing with "Too many pending metadata requests".
Torrent downloads wrapped every failure, including KetchError subtypes such as Disk or Network, in a non-retryable SourceError, so DownloadConfig.retryCount never applied to torrents. Failures are now reported in Ketch's terms: KetchError passes through unchanged, cancellation is never wrapped, internal timeouts (such as magnet metadata resolution) become retryable Network errors, and a busy peer listen port or a raw socket failure while fetching remote metainfo is Network too. Storage failures become Disk, and verification, protocol and admission failures stay SourceError; neither is retried. Peer, tracker and DHT errors were already retried inside the swarm and still do not fail the task.
…tions For torrents, a task's connections setting is its peer cap. v1 and v2 each had their own inline mapping; both now use torrentPeerLimit, which clamps an explicit value to the session's accepted range (1..512 for v1, 1..500 for v2) instead of risking a rejected value, and uses TorrentConfig.connectionsPerTorrent when the task sets none. Ketch's maxConnectionsPerDownload (HTTP segments) is never used as a peer cap.
Describe simultaneous downloads and the engine's active-torrent limit, global and per-task speed limits, per-task connections as a clamped peer cap, priority, and which failures Ketch retries.
Ketch.updateConfig only applied the speed limit and queue limits; the coordinator, executions and HTTP source kept the constructor config. - DownloadCoordinator takes a config provider backed by Ketch's current config and snapshots it for each start or resume. - DownloadContext carries that snapshot (config) and exposes effectiveConnections(): live override > request.connections > maxConnectionsPerDownload. - DownloadSource gains resolve(url, properties, config); Ketch calls it so ResolvedSource.maxSegments reflects the current default. - HttpDownloadSource reads connections and progress interval from the context instead of constructor ints. - status() reports the current default directory. - Default dispatcher pools no longer scale with the constructor's maxConnectionsPerDownload; network I/O suspends, so pool size does not cap connections raised later.
…tasks DownloadTask.setSpeedLimit and setConnections were dropped unless the task had a running execution, so the value snapped back for queued, scheduled, paused and failed tasks. They are now persisted in the task record (like setPriority) before being applied to a running execution, and a new execution initializes its task limiter from the persisted request. setConnections validates its argument before persisting.
- DownloadQueue.updateLimits replaces both limits under the queue mutex and promotes queued tasks that now fit; Ketch.updateConfig uses it. Lowering a limit never interrupts running downloads. - extractHost strips user info (ftp://user:pass@host keyed on the host), lowercases, handles bracketed IPv6 and ports, and returns null for URIs without a network host (magnet:, torrent:, file:, local paths), which are no longer subject to the per-host limit.
A live setConnections(>1) resegmented the transfer even when the server does not support byte ranges, and the next Range request failed the task with Unsupported. HttpDownloadSource now clamps the effective connection count to 1 for such servers on fresh starts, retries and resumes, and SegmentedDownloadHelper.downloadAll takes supportsRanges so sources can ignore live connection changes that would need unsupported offsets. FakeHttpEngine now rejects non-zero range offsets when the server does not accept ranges, like KtorHttpEngine's response validation.
DownloadTask.reschedule only changed in-memory state, so a restart restored the previous schedule or paused state. DownloadScheduler now saves the new schedule (and in-memory conditions) in the task request and marks the record SCHEDULED. Scheduled tasks are enqueued with preferResume, so a task rescheduled after it started resumes its saved segments after a restart instead of starting over; tasks without segments still start fresh. Conditions remain unpersisted by design.
FtpDownloadSource ignored the global config: it used its own constructor defaults (4 connections, 200 ms progress) and the apps construct it without arguments. It also restarted a failed fresh download from byte zero on every retry. - Remove the maxConnections/progressIntervalMs constructor knobs. The connection count follows DownloadContext.effectiveConnections() (live override > request.connections > maxConnectionsPerDownload), progress uses the context's progressIntervalMs and the FTP client reads with DownloadConfig.bufferSize. - A retry reuses the segment progress recorded in the context, like HTTP. - Servers without REST always use one connection: live connection changes are ignored and a resume restarts from zero instead of failing on REST. - resolve(url, properties, config) reports maxConnectionsPerDownload as maxSegments. Adds FakeFtpServer/FakeFtpClient (via the internal clientFactory hook) and an in-memory FileAccessor for source-level tests.
DownloadTask.pause() ignored tasks in the Queued state, so a download waiting for a slot could not be held back. Pausing a queued task now removes it from the queue, persists PAUSED and publishes Paused with the saved progress; resume() queues it again. A task paused before it saved any segments is also persisted as PAUSED so it does not restart on its own after a restart.
- KetchApi.updateConfig lists which fields apply immediately and which apply to downloads that start or resume later. - DownloadRequest.speedLimit applies in addition to the global limit (the lower rate wins) instead of overriding it. - DownloadConfig documents per-source connection behavior, the per-host key (host-less URIs are not counted), queue-limit changes and that bufferSize applies to FTP reads. - DownloadTask documents persistence of speed limit, connections and reschedule for inactive tasks, and pausing queued tasks. - AGENTS.md and docs replace the removed setGlobalSpeedLimit API with updateConfig.
Only magnet links skip the per-server limit (.torrent files count against the site serving them), and lowering a queue limit lets running downloads finish rather than stopping them.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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.
Reviewed commit: 20b6f1b8f6
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| networkLock.withLock { | ||
| attempt(onError = { | ||
| networkError = it | ||
| networks = current |
There was a problem hiding this comment.
When multiple interface changes are queued, current may already contain an optimistic, unconfirmed selection. For example, starting from no interfaces, selecting A and then A+B before the first request finishes captures A as the second rollback value; if both API calls fail, this line leaves A checked even though the server applied neither request. Track the last server-confirmed configuration or coalesce pending selections instead of rolling back to the per-call optimistic snapshot.
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Fixed in 5a7df78: the controller now keeps the last server-confirmed interfaces and falls back to them on failure, and selections queued behind an in-flight request are coalesced into the newest. A regression test covers the A then A+B case with both requests failing.
Contributor
Test Results 867 files + 22 867 suites +22 6m 11s ⏱️ +28s Results for commit 4054999. ± Comparison against base commit 9f9a1f7. This pull request removes 39 and adds 200 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Brings in the settings overlay dialog from #257 and keeps its presentation: wide windows open Settings as a panel over the current destination, narrow ones as a page, and ⌘, / Ctrl+, plus the macOS app menu open it. The panel and page now show the categorized settings from this branch. With changes applied as they are made there are no drafts to lose, so the unsaved-section tracking and discard confirmation (SettingsEdits) and the per-section cards are dropped. Back on the phone category list now closes Settings.
The iOS test build failed with 'Name contains illegal characters' on a backtick test name containing a comma.
A failed network interface update restored the snapshot taken before that call, which could be another unconfirmed selection: selecting A, then A+B, with both rejected left A ticked. The controller now keeps what the instance last reported and falls back to it, and selections made while one is in flight are coalesced into the newest.
Brings in drag-to-add torrents (#261), extra torrent trackers and DHT state (#256) and the Dock icon fix (#262). - AppShell: keeps the new FileDropTarget wrapper and uses the categorized SettingsPage inside it. - DownloadSource: keeps both new members, the config-aware resolve overload and canHandleContent/resolveContent. - AGENTS.md: keeps the TorrentSettings entry alongside the settings notes.
Brings in opening .torrent files from the system file manager (#260). App passes the new IncomingDownloads through alongside the theme mode; AppShell keeps its import next to the settings controller and categories.
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
1 participant
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
The settings page was one long scroll of cards, each with its own Save button. It's now organised into categories, applies changes as they're made, and gains the options that were missing. An audit of the engine showed that most global download settings didn't reach FTP or torrent downloads, and weren't applied after a change at runtime. This PR fixes that too, so each setting behaves the way its Settings page describes.
App: categorized settings
AppearanceConfig.theme).ServerConfig.autoStart).config.toml.ServerState.Running. The instance picker used to report port 8642 regardless of the setting.ServerState.Failedinstead of crashing the app.Engine: settings apply to every source
updateConfig: the speed limit and queue limits still apply immediately. Directory, connections, retries and intervals now apply to downloads that start or resume afterwards; before, they only took effect after a restart.status()reports the current directory.FtpDownloadSource()no longer takes arguments.bufferSize.maxActiveTorrents(default 5) wait for a slot instead of failing: highest priority first, then arrival order. Pause and cancel work while waiting.DownloadRequest.speedLimitnow states that the lower of the task and global limits wins.Reviewer notes
DownloadSource.resolve(url, properties, config)overload, with a default implementation.DownloadContext.config, set from the current global config when a download starts or resumes.DownloadContext.effectiveConnections().FtpDownloadSource()takes no arguments.Ketch's default dispatchers now use fixed-size pools.DownloadContext.maxConnectionsas its peer cap. It still starts atrequest.connections, so the global HTTP connection count never caps torrent peers.bufferSize, since that would change theHttpEngineinterface. Its KDoc now says it applies to FTP.org.jetbrains.androidx.navigationevent:navigationevent-compose. Compose'sBackHandleris deprecated in 1.12.config.tomlfiles without the new fields decode to the previous behaviour; tests cover this.Testing
:library:api:jvmTest(94),:library:core:jvmTest(263),:library:ftp:jvmTest(86),:library:ktor:jvmTest(27),:library:torrent:jvmTest(656),:library:remote:jvmTest(6),:library:server:test(46),:config:jvmTest(21),:app:shared:jvmTest(131): all passing.app/shared.FakeFtpClient.:library:core:wasmWasiTesthas 2 failures that also occur onmain. CI doesn't run that task.