desktop: validate backend and relay URLs in settings#326
Merged
Conversation
Both URLs were persisted as free-form strings. An invalid host or missing scheme fell through to the worker loop as a confusing runtime error. Validate at two layers: - Rust: reject empty, non-http/https, and hostless URLs in WorkerManager::save_settings before persist (url::Url::parse). - UI: live validation in the settings form and onboarding wizard using new URL(), with inline per-field error messages. Hints now name the common local backends (Ollama 11434, LM Studio 1234, Kiln 8420) so the field is self-documenting. 11 unit tests cover the accept and reject cases, including the three default ports and an http://:port/path hostless case.
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.
What
Validate
backend_urlandrelay_urlin the desktop app at two layers so a bad value can't slip through to the worker loop.WorkerManager::save_settings) — parse withurl::Url::parseand reject empty strings, non-http(s)schemes, and missing hosts before persistingsettings.json.ui/index.html) — live validation in the settings form and onboarding wizard usingnew URL(), with inline per-field error text. Wizard refuses to advance past a bad backend URL at step 0 or a bad relay URL at step 1.11434, LM Studio1234, Kiln8420) so the field is self-documenting.Why
Scoping notes: the backend URL field itself already exists in both the settings tab and the onboarding wizard — it's an
AppSettingsfield (defaulthttp://localhost:11434), surfaces in the UI, is wired toWorkerDaemonConfig.backend_base_url, and hot-reloads a running worker on save. The one real gap from the task checklist was URL validation: a user could typelocalhost:11434(no scheme),foo bar, or leave the field blank and hit Save without any feedback. Bad values then surfaced as opaque reqwest errors in the worker loop.Tests
Added 11 unit tests in
crates/modelrelay-desktop/src/lib.rscovering:ftp://scheme,http://:8080/path(empty host)The validation logic was exercised locally in a scratch crate (all 11 tests green) — Cloud Eric can't run
cargo check/clippyon the desktop crate directly because the sandbox lacks GTK/webkit2gtk system libs, so CI on a Linux runner is the first end-to-end build.cargo fmt --check -p modelrelay-desktoppasses.Release
No release tooling change needed — desktop builds are cut by pushing a
desktop-v*tag, which triggers the existing tauri-action workflow. Not doing that here; a new desktop build can be cut separately once this lands.