From e0edf7a0842a1c5f60f5360a28511dcbfe874608 Mon Sep 17 00:00:00 2001 From: sami Date: Sat, 12 Sep 2026 13:42:45 +0400 Subject: [PATCH] daemon: file capture.offer/getRules in deferrals (D7/D8); close settings.get/set (D9) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rebased lane/daemon onto main first — fast-forward, no conflicts (lane/daemon was already fully merged; main has since taken lane/gui, lane/pkg-qa, lane/proto). 1. capture.offer and capture.getRules were stubs, buried inside D3's generic stub list with no flag for how load-bearing capture.offer is: it's a CLAUDE.md §4 non-negotiable (750ms deadline, fails open) and AGENT-DAEMON.md build step 6, and against the real daemon it 500s to -32603 every time — the extension captures nothing, not "falls through to Firefox on a slow path." Split into their own D7/D8 rows so they stop being invisible. 2. settings.get/settings.set (D9, closed). Four pointer-to-member tables in dispatcher.cpp (bool / ranged int / plain string / string array), five enum-typed keys handled individually since parse_XXX already validates those — covers all 43 SettingKeys without ~40 repetitive hand blocks. store::Settings::kDefaults grew from 14 entries to 43 (the schema itself carries no "default" keyword anywhere, so these are hand-chosen — conservative for the ones ADR 0012 doesn't speak to; capture.monitoredExtensions defaults to the union of every builtin category's extensions rather than an arbitrary list of its own). settings.set validates every field before writing any of them: numeric min/max (hand-checked — the generated parser only checks JSON type, not schema constraints) and saveTo.* paths via fs::resolve_target/canonicalize_root (-32011) — allowedRoots entries checked as roots in their own right, defaultDir/tempDir checked as paths resolving inside the (possibly just-updated, same call) root list. Reports exactly the keys whose effective value actually changed, publishes event.settings.changed with that list, and calls the scheduler's reload_config() when a connection.* key took effect — live, not on next restart. Verified against real veloxd: all 43 keys round-trip with defaults, a keys subset filters, an out-of-range value rejects the whole call, a bad saveTo.defaultDir is -32011, allowedRoots+defaultDir set together cross-validate against the new roots, event.settings.changed fires over a live subscription. New dispatcher_settings_test covers the same ground without a socket. Full ctest: 53/53 (excluding a pre-existing, unrelated conformance failure — see below). Also found, not fixed (not this lane): tests/conformance/run.sh's live-veloxd leg fails "pairing failed: no token issued" reproducibly, isolated, on main before any of this session's changes — it launches the isolated veloxd without VELOX_PAIR_AUTO=1, so EnvAutoApprover denies every session.pair and the WS leg's session.hello never gets a token. tests/conformance/ is PROTO/QA-owned; flagging rather than editing across the lane boundary. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01GRDjHGgpYmMoPE2UFbe7pP --- daemon/docs/deferrals.md | 6 +- daemon/src/rpc/dispatcher.cpp | 291 +++++++++++++++++++++- daemon/src/rpc/task_action_port.hpp | 8 + daemon/src/sched/scheduler.hpp | 6 + daemon/src/store/settings.cpp | 69 ++++- daemon/src/store/settings.hpp | 1 + daemon/tests/CMakeLists.txt | 1 + daemon/tests/dispatcher_settings_test.cpp | 142 +++++++++++ 8 files changed, 509 insertions(+), 15 deletions(-) create mode 100644 daemon/tests/dispatcher_settings_test.cpp diff --git a/daemon/docs/deferrals.md b/daemon/docs/deferrals.md index c509c95..014fd51 100644 --- a/daemon/docs/deferrals.md +++ b/daemon/docs/deferrals.md @@ -5,11 +5,13 @@ close. Kept here (not buried in commit messages) so the next pass can see them a | # | What | Where | Why deferred | Closes when | |---|---|---|---|---| +| **D7** | **`capture.offer` → `-32603`.** This is a CLAUDE.md §4 non-negotiable ("Capture fails open... the extension gives up" — but a fail-open answer still has to arrive; a hard error is not a substitute for `ignore`) and AGENT-DAEMON.md build step 6 ("`capture.offer` must answer within 750 ms, always"). Against the real daemon right now, every offer 500s, so the extension captures nothing at all — not "falls through to Firefox on a slow path," genuinely nothing, because the extension is waiting on a promise that never resolves with a decision it can act on. This was buried inside D3's stub list ("...`capture.*`") with no flag for how load-bearing it is; called out on its own row now. | `rpc/dispatcher.cpp` | rules table has no matcher yet, no dedup-against-active-tasks, no category-folder resolution | build step 6 | +| **D8** | `capture.getRules` → `-32603`. The extension mirrors this into its own capture policy on connect (docs/05 §4) so the two can never disagree about what should be intercepted; until this returns something real the extension runs with an empty/stale policy. Also buried in D3's list before this pass. | `rpc/dispatcher.cpp` | same rules-engine gap as D7 | alongside D7 | | D1 | Pairing prompt is `EnvAutoApprover` (needs `VELOX_PAIR_AUTO=1`) | `rpc/pairing.hpp`, `main.cpp` | A GUI dialog / `org.freedesktop.Notifications` approver is integration work | Build step 7 (systemd + notifications) | | — | **D1, checked this pass, not attempted:** `libdbus-1-dev` (or `libsystemd-dev` for `sd-bus`) has no headers installed in this build environment — only the runtime `.so`s (`dpkg -l`/`apt-cache policy` confirm `libdbus-1-3` present, `libdbus-1-dev` not, "Candidate" available but not installed). A real notification-backed approver needs one of those linked into `veloxd`, which is a new build dependency for `daemon/CMakeLists.txt` (`find_package`/`pkg_check_modules`) and — since packaging manifests need to know about it too — arguably a decision to surface rather than something to reach for silently mid-session. `PairingApprover::approve()` is also still synchronous by shape (its own doc comment already says so: "the real notification-backed approver will run async and is not this shape") — swapping it for the async pattern this session built for `download.probe` (`rpc::TaskActionPort` + the server-layer deferred-reply special-case) is the right shape once there's a real implementation to justify the churn; reshaping the interface with nothing behind it yet would just be churn. Left `EnvAutoApprover` in place rather than build a fragile hand-rolled D-Bus wire client to avoid the missing headers — a broken pairing approver is worse than an honest stub. | `rpc/pairing.hpp` | missing dev headers + an undiscussed new dependency | once `libdbus-1-dev`/`libsystemd-dev` is available and the dependency is approved | | ~~D2~~ | **Closed** — `download.probe` is real on both transports. It's genuinely async (the engine's probe pool, up to the schema's 30s `x-deadlineMs`) and so cannot fit `VeloxDispatcher::on_download_probe`'s synchronous `HandlerResult` return — `uds_server.cpp`/`ws_server.cpp` special-case `"download.probe"` before the generic `dispatch()`, exactly the way they already special-case `session.hello`/`session.subscribe`, and queue the reply whenever the callback fires. `rpc::TaskActionPort::probe_now` (kept in proto/std terms, no `vdm::net::*`, so `veloxd_rpc` never needs `core/include`'s vdm headers) is what both transports call; `sched::Scheduler::probe_now` is the implementation — builds a `vdm::net::ProbeRequest`, runs it on the engine's probe pool, maps a failure to `-32013 ProbeFailed` (with `data.httpStatus` when there was one), and fills `suggestedCategoryId`/`suggestedSaveDir` with a plain extension match against the categories table (not the real rules engine — that's still D3). Verified live: a real probe answers in ~5ms; a bad host maps to `-32013`; a connection issuing a 10s `slow-loris` probe does not block a second connection's `download.list` (answered in ~1ms) — confirms the async design actually keeps the loop free, not just compiles. | `rpc/task_action_port.hpp`, `rpc/{uds_server,ws_server}.{hpp,cpp}`, `sched/scheduler.{cpp,hpp}` | — | done | -| D3 | Stub handlers for the rest: `download.refreshUrl/update`, `rules.*`, `settings.*`, `limiter.*`, `schedule.*`, `queue.reorder`, `grabber.*`, `media.*`, `capture.*` | `rpc/dispatcher.cpp` | No store/scheduler wiring behind them yet, or (`settings.*`) sound but large — see the note below. `category.list/upsert/remove`, `queue.list/upsert/start/stop`, `download.remove/addBatch/provideAuth` are done | Per method, as each wires to the store/scheduler | -| — | **`settings.get`/`settings.set` specifically, not started:** `proto::Settings` is a flat struct of ~43 `std::optional` fields, one per `SettingKey` (~50 keys) in `Settings.schema.json`; `store::Settings` already has `get_raw`/`set_raw`/`overrides` keyed by the same dotted strings the JSON uses (`"connection.maxSegmentsPerDownload"`, …). The handlers are a mechanical field <-> key <-> JSON-type mapping table in both directions (get: row-or-default -> struct field; set: struct field -> validate against the key's schema type -> `set_raw`, collecting `changed`) — real work, just long and repetitive rather than hard. Left alone this pass rather than rushed; every other read of settings in this codebase already goes through `store::Settings`'s typed helpers directly (`reload_config`, `on_download_add`'s segment default, `capture.offer`'s allowed roots when that lands), so nothing downstream is blocked on the RPC surface existing. | `rpc/dispatcher.cpp`, `store/settings.{hpp,cpp}` | the mapping table is genuinely large, not genuinely hard | its own pass | +| D3 | Stub handlers for the rest: `download.refreshUrl/update`, `rules.*`, `limiter.*`, `schedule.*`, `queue.reorder`, `grabber.*`, `media.*` | `rpc/dispatcher.cpp` | No store/scheduler wiring behind them yet | Per method, as each wires to the store/scheduler | +| ~~D9~~ | **Closed — `settings.get`/`settings.set`.** The field <-> `SettingKey` <-> JSON-type mapping is four pointer-to-member tables in `dispatcher.cpp` (one per C++ field type: bool, ranged int, plain string, string array) plus five enum-typed keys handled individually (`parse_XXX` already validates those); every one of the 43 `SettingKey`s now has a real default (`store::Settings::kDefaults` grew from 14 entries to 43 — `capture.monitoredExtensions`'s default is the union of every builtin category's extensions, so the two never drift apart). `settings.get` honors `keys: null` = everything. `settings.set` validates every field *before* writing any of them (numeric min/max — the schema itself carries none of this, so it's hand-checked against each key's documented range; `-32602` names the offending key, its value, and its bounds) and validates `saveTo.*` paths against `fs::resolve_target`/`canonicalize_root` (`-32011`) — `saveTo.allowedRoots` entries are checked as roots in their own right, `saveTo.defaultDir`/`.tempDir` are checked as paths resolving *inside* the (possibly, in the same call, just-updated) root list. Reports exactly the keys whose *effective* value actually changed (a `set` to the value already in effect reports `changed: []`, not the key), publishes `event.settings.changed` with that same list, and calls `TaskActionPort::apply_settings_reload()` (-> `Scheduler::reload_config()`) when any `connection.*` key took effect, live rather than waiting for a restart. Verified against real `veloxd`: all 43 keys round-trip with sane defaults, a `keys` subset filters correctly, an out-of-range value is rejected with nothing else in the same call landing, `saveTo.defaultDir` outside every allowed root is `-32011`, setting `allowedRoots` and `defaultDir` together cross-validates against the *new* roots, and `event.settings.changed` fires over a live subscription. New `dispatcher_settings_test` covers the same ground without a socket. | `rpc/dispatcher.cpp`, `store/settings.{hpp,cpp}`, `rpc/task_action_port.hpp`, `sched/scheduler.hpp` | — | done | | ~~D3a~~ | **Closed** — `category.upsert`/`category.remove`: `store/categories.hpp` gains `get`/`upsert`/`remove`. `upsert` generates an id when absent (create) and always ignores the payload's `builtin` (preserved from the existing row on replace, false on create — a client can never mint or revoke it); the `saveDir` goes through the same `fs::resolve_target` canonicalize-and-root-check as `download.add` (`-32011` on failure). `remove` refuses a builtin at both layers (dispatcher pre-checks for the `-32602` error text; the store's own `DELETE ... AND builtin = 0` is defense in depth) and reassigns member tasks to `reassignTo` (default `"general"`) inside one transaction before deleting the row. Note: the `categories` table (0001) has no columns for `Category.mimeTypes`/`.sortOrder` — accepted on `upsert` but not persisted. | `store/categories.{hpp,cpp}`, `rpc/dispatcher.cpp` | — | done, `mimeTypes`/`sortOrder` gap noted | | ~~D3b~~ | **Closed** — `queue.upsert`: `store/queues.hpp` gains `get`/`upsert` (`set_state` already existed from D4b). Same create-generates-id pattern as categories; `taskIds` in the payload is ignored (schema's own note) and a create always starts `'stopped'` while a replace keeps the queue's current run state — `queue.upsert` edits config, not run state (that's `queue.start`/`stop`). Also fixed: `on_complete` was a real column since 0001 but `Queues::list`/`get` never projected it onto `Queue.onComplete` — now they do. | `store/queues.{hpp,cpp}`, `rpc/dispatcher.cpp` | — | done | | ~~D3c~~ | **Closed** — `download.remove`: cancels with `discard_partial=true` through `TaskActionPort` (always drops any `.veloxpart`/`.veloxpart.meta` — the row is gone either way, unlike `download.cancel`, which keeps them), deletes the finished file only when `deleteFile` is true and the task was `complete` (best-effort — a missing file doesn't fail the call), deletes the row (segments cascade via the FK), and publishes `event.task.removed` (closing the last open note under D5). `download.addBatch`: `on_download_add`'s body is now a shared `add_one()`, called once per item after merging each item's unset fields against `params.defaults`. `download.provideAuth`: forwards to `EnginePort::provide_auth` through a new `TaskActionPort::provide_auth`; `remember`/persisting to the Secret Service is accepted but not acted on — nothing in this build talks to libsecret yet (verified: no such integration exists anywhere in the tree). Verified against real `veloxd` + `tools/testserver`: category create/replace/remove-with-reassignment, queue create/replace-keeps-state, a batch add with shared `defaults.saveDir`, and remove-with-deleteFile actually deleting the file and the task then 404ing `download.get` with `-32010`. | `rpc/dispatcher.{hpp,cpp}`, `rpc/task_action_port.hpp`, `sched/scheduler.{cpp,hpp}` | `download.provideAuth`'s `remember` (needs the Secret Service, unbuilt) | done, `remember` persistence gap noted | diff --git a/daemon/src/rpc/dispatcher.cpp b/daemon/src/rpc/dispatcher.cpp index c88d8be..7490e33 100644 --- a/daemon/src/rpc/dispatcher.cpp +++ b/daemon/src/rpc/dispatcher.cpp @@ -1,6 +1,9 @@ #include "rpc/dispatcher.hpp" +#include +#include #include +#include #include #include @@ -102,6 +105,115 @@ std::string filename_from_url(std::string_view url) { return out; } +// --- settings.get / settings.set : the field <-> SettingKey <-> JSON-type mapping ------ +// +// One table per C++ field type in proto::Settings, keyed by pointer-to-member so get/set +// each need one small loop per type instead of ~43 repetitive hand-written blocks. Every +// SettingKey appears in exactly one table (or, for the five enum-typed keys, is handled +// individually just below — parse_XXX already does their validation, so there is nothing +// generic left for those five to share). store::Settings::get_raw/get_bool/get_int/ +// get_string/get_string_array already do the row-or-default fallback; these tables just +// say which key goes with which field and, for the numeric ones, its valid range (schema +// minimum/maximum — NOT enforced by the generated parser, which only checks JSON type). + +struct BoolSetting { + proto::SettingKey key; + std::optional proto::Settings::* field; +}; +constexpr BoolSetting kBoolSettings[] = { + {proto::SettingKey::GeneralLaunchOnLogin, &proto::Settings::general_launchOnLogin}, + {proto::SettingKey::GeneralMinimizeToTray, &proto::Settings::general_minimizeToTray}, + {proto::SettingKey::GeneralShowDropTarget, &proto::Settings::general_showDropTarget}, + {proto::SettingKey::GeneralConfirmOnExit, &proto::Settings::general_confirmOnExit}, + {proto::SettingKey::GeneralCheckForUpdates, &proto::Settings::general_checkForUpdates}, + {proto::SettingKey::CaptureEnabled, &proto::Settings::capture_enabled}, + {proto::SettingKey::SaveToCreateSubfolderPerSite, &proto::Settings::saveTo_createSubfolderPerSite}, + {proto::SettingKey::DownloadsSpeedLimitEnabled, &proto::Settings::downloads_speedLimitEnabled}, + {proto::SettingKey::DownloadsVerifyChecksums, &proto::Settings::downloads_verifyChecksums}, + {proto::SettingKey::SoundsEnabled, &proto::Settings::sounds_enabled}, +}; + +struct IntSetting { + proto::SettingKey key; + std::optional proto::Settings::* field; + std::int64_t min; + std::int64_t max; +}; +constexpr std::int64_t kNoMax = std::numeric_limits::max(); +constexpr IntSetting kIntSettings[] = { + {proto::SettingKey::CaptureMinSizeBytes, &proto::Settings::capture_minSizeBytes, 0, kNoMax}, + {proto::SettingKey::ConnectionMaxSegmentsPerDownload, + &proto::Settings::connection_maxSegmentsPerDownload, 1, 32}, + {proto::SettingKey::ConnectionBufferBytes, &proto::Settings::connection_bufferBytes, 65536, + 16777216}, + {proto::SettingKey::ConnectionMaxTotalBufferBytes, + &proto::Settings::connection_maxTotalBufferBytes, 16777216, 2147483648}, + {proto::SettingKey::ConnectionMaxActiveSegments, &proto::Settings::connection_maxActiveSegments, + 1, 256}, + {proto::SettingKey::ConnectionMaxConcurrentDownloads, + &proto::Settings::connection_maxConcurrentDownloads, 1, 64}, + {proto::SettingKey::ConnectionTimeoutSec, &proto::Settings::connection_timeoutSec, 1, 3600}, + {proto::SettingKey::ConnectionMaxRetries, &proto::Settings::connection_maxRetries, 0, 100}, + {proto::SettingKey::ConnectionRetryBackoffSec, &proto::Settings::connection_retryBackoffSec, 0, + 3600}, + {proto::SettingKey::DownloadsSpeedLimitBps, &proto::Settings::downloads_speedLimitBps, 0, kNoMax}, + {proto::SettingKey::ProxyPort, &proto::Settings::proxy_port, 1, 65535}, +}; + +struct StrSetting { + proto::SettingKey key; + std::optional proto::Settings::* field; +}; +// saveTo.defaultDir / saveTo.tempDir are here too (read/write is identical to any other +// string); on_settings_set gives them one extra pass validating the path itself before +// either is written. +constexpr StrSetting kStrSettings[] = { + {proto::SettingKey::GeneralLanguage, &proto::Settings::general_language}, + {proto::SettingKey::SaveToDefaultDir, &proto::Settings::saveTo_defaultDir}, + {proto::SettingKey::SaveToTempDir, &proto::Settings::saveTo_tempDir}, + {proto::SettingKey::DownloadsVirusScanCommand, &proto::Settings::downloads_virusScanCommand}, + {proto::SettingKey::DownloadsPostDownloadCommand, &proto::Settings::downloads_postDownloadCommand}, + {proto::SettingKey::ProxyHost, &proto::Settings::proxy_host}, + {proto::SettingKey::ProxyUsername, &proto::Settings::proxy_username}, + {proto::SettingKey::ProxyPacUrl, &proto::Settings::proxy_pacUrl}, + {proto::SettingKey::SoundsOnComplete, &proto::Settings::sounds_onComplete}, + {proto::SettingKey::SoundsOnQueueComplete, &proto::Settings::sounds_onQueueComplete}, + {proto::SettingKey::SoundsOnError, &proto::Settings::sounds_onError}, +}; + +struct StrArrSetting { + proto::SettingKey key; + std::optional> proto::Settings::* field; +}; +// saveTo.allowedRoots is here too; same deal as the two directory strings above, except +// every entry is checked (it IS the root list, not a path resolved against it). +constexpr StrArrSetting kStrArrSettings[] = { + {proto::SettingKey::CaptureMonitoredExtensions, &proto::Settings::capture_monitoredExtensions}, + {proto::SettingKey::CaptureMonitoredMimeTypes, &proto::Settings::capture_monitoredMimeTypes}, + {proto::SettingKey::CaptureExcludedHosts, &proto::Settings::capture_excludedHosts}, + {proto::SettingKey::CaptureAutoStartTypes, &proto::Settings::capture_autoStartTypes}, + {proto::SettingKey::ProxyBypassHosts, &proto::Settings::proxy_bypassHosts}, + {proto::SettingKey::SaveToAllowedRoots, &proto::Settings::saveTo_allowedRoots}, +}; + +// Whether `key` is in `wanted` (settings.get's own params.keys, nullopt meaning "all" per +// the schema's "keys null means everything"). +bool settings_key_wanted(const std::optional>& wanted, + proto::SettingKey key) { + if (!wanted) return true; + return std::find(wanted->begin(), wanted->end(), key) != wanted->end(); +} + +// Whether writing `key` would actually change the effective value (row-or-default) — +// settings.set / event.settings.changed report exactly the keys that "took effect", not +// every key the caller merely mentioned. +bool settings_value_changed(store::Settings& settings, proto::SettingKey key, + const std::string& new_json) { + auto old = settings.get_raw(proto::to_string(key)); + const std::string old_json = (old && *old) ? **old : std::string(); + return old_json != new_json; +} + } // namespace // --- session.* : handled in the server layer, unreachable here in the running daemon --- @@ -591,12 +703,183 @@ VeloxDispatcher::on_schedule_set(const proto::ScheduleSetParams&) { return not_implemented("schedule.set"); } proto::HandlerResult -VeloxDispatcher::on_settings_get(const proto::SettingsGetParams&) { - return not_implemented("settings.get"); +VeloxDispatcher::on_settings_get(const proto::SettingsGetParams& params) { + store::Settings settings(db_); + proto::Settings out; + + for (const auto& s : kBoolSettings) + if (settings_key_wanted(params.keys, s.key)) out.*(s.field) = settings.get_bool(proto::to_string(s.key)); + for (const auto& s : kIntSettings) + if (settings_key_wanted(params.keys, s.key)) out.*(s.field) = settings.get_int(proto::to_string(s.key)); + for (const auto& s : kStrSettings) + if (settings_key_wanted(params.keys, s.key)) out.*(s.field) = settings.get_string(proto::to_string(s.key)); + for (const auto& s : kStrArrSettings) + if (settings_key_wanted(params.keys, s.key)) + out.*(s.field) = settings.get_string_array(proto::to_string(s.key)); + + // The five enum-typed keys: get_string() already returns the raw-or-default JSON + // string content (quotes stripped); parse_XXX validates it against the same set the + // schema's enum names. A stored value that somehow doesn't parse (should not happen — + // set_raw always goes through the same parser) leaves the field unset rather than + // guessing, which is a legal Settings shape (every property is optional). + if (settings_key_wanted(params.keys, proto::SettingKey::CaptureBypassModifier)) + if (auto v = proto::parse_BypassModifier(settings.get_string("capture.bypassModifier"))) + out.capture_bypassModifier = *v; + if (settings_key_wanted(params.keys, proto::SettingKey::SaveToFileExistsPolicy)) + if (auto v = proto::parse_SettingsSaveToFileExistsPolicy( + settings.get_string("saveTo.fileExistsPolicy"))) + out.saveTo_fileExistsPolicy = *v; + if (settings_key_wanted(params.keys, proto::SettingKey::ConnectionPreset)) + if (auto v = proto::parse_SettingsConnectionPreset(settings.get_string("connection.preset"))) + out.connection_preset = *v; + if (settings_key_wanted(params.keys, proto::SettingKey::DownloadsDuplicatePolicy)) + if (auto v = proto::parse_SettingsDownloadsDuplicatePolicy( + settings.get_string("downloads.duplicatePolicy"))) + out.downloads_duplicatePolicy = *v; + if (settings_key_wanted(params.keys, proto::SettingKey::ProxyMode)) + if (auto v = proto::parse_SettingsProxyMode(settings.get_string("proxy.mode"))) + out.proxy_mode = *v; + + proto::SettingsGetResult r; + r.values = std::move(out); + return r; } + proto::HandlerResult -VeloxDispatcher::on_settings_set(const proto::SettingsSetParams&) { - return not_implemented("settings.set"); +VeloxDispatcher::on_settings_set(const proto::SettingsSetParams& params) { + const proto::Settings& v = params.values; + store::Settings settings(db_); + + // --- pass 1: validate everything before writing anything (no partial update on a + // rejected call) ------------------------------------------------------------------ + for (const auto& s : kIntSettings) { + if (!(v.*(s.field))) continue; + const auto val = *(v.*(s.field)); + if (val < s.min || val > s.max) { + return std::unexpected(proto::HandlerError{ + proto::ErrorCode::InvalidParams, + "value out of range for " + std::string(proto::to_string(s.key)), + nlohmann::json{{"key", std::string(proto::to_string(s.key))}, {"value", val}, + {"min", s.min}, {"max", s.max}}}); + } + } + + // Directory keys: saveTo.allowedRoots entries are checked as roots in their own + // right; saveTo.defaultDir / saveTo.tempDir are checked as paths that must resolve + // *inside* the (possibly, in this same call, just-updated) root list — the same rule + // download.add's own saveDir is held to ("every write target is canonicalized and + // must resolve inside one of these", Settings.schema.json's own words for + // saveTo.allowedRoots). + const std::vector effective_roots = + v.saveTo_allowedRoots ? *v.saveTo_allowedRoots : settings.get_string_array("saveTo.allowedRoots"); + std::vector canon_roots; + for (const auto& r : effective_roots) { + auto c = fs::canonicalize_root(r); + if (!c) { + return std::unexpected(proto::HandlerError{ + proto::ErrorCode::InvalidPath, "not a writable directory", + nlohmann::json{{"key", "saveTo.allowedRoots"}, {"path", r}}}); + } + canon_roots.push_back(*c); + } + + auto check_dir = [&](const std::string& dir, + const char* key_name) -> std::optional { + if (dir.empty()) return std::nullopt; // "" means "unset / use the default" + auto target = fs::resolve_target(expand_tilde(dir), ".settings-marker", canon_roots); + if (!target) { + return proto::HandlerError{proto::ErrorCode::InvalidPath, target.error().message, + nlohmann::json{{"key", key_name}, {"path", dir}}}; + } + return std::nullopt; + }; + if (v.saveTo_defaultDir) { + if (auto e = check_dir(*v.saveTo_defaultDir, "saveTo.defaultDir")) return std::unexpected(*e); + } + if (v.saveTo_tempDir) { + if (auto e = check_dir(*v.saveTo_tempDir, "saveTo.tempDir")) return std::unexpected(*e); + } + + // --- pass 2: write, tracking which keys actually took effect ---------------------- + std::vector changed; + bool touched_connection = false; + + auto apply = [&](proto::SettingKey key, const std::string& json_text) { + if (settings_value_changed(settings, key, json_text)) changed.push_back(key); + (void)settings.set_raw(proto::to_string(key), json_text); + }; + + for (const auto& s : kBoolSettings) + if (v.*(s.field)) apply(s.key, *(v.*(s.field)) ? "true" : "false"); + for (const auto& s : kIntSettings) { + if (!(v.*(s.field))) continue; + apply(s.key, std::to_string(*(v.*(s.field)))); + touched_connection = touched_connection || proto::to_string(s.key).starts_with("connection."); + } + for (const auto& s : kStrSettings) + if (v.*(s.field)) apply(s.key, nlohmann::json(*(v.*(s.field))).dump()); + for (const auto& s : kStrArrSettings) + if (v.*(s.field)) apply(s.key, nlohmann::json(*(v.*(s.field))).dump()); + + if (v.capture_bypassModifier) + apply(proto::SettingKey::CaptureBypassModifier, + nlohmann::json(std::string(proto::to_string(*v.capture_bypassModifier))).dump()); + if (v.saveTo_fileExistsPolicy) + apply(proto::SettingKey::SaveToFileExistsPolicy, + nlohmann::json(std::string(proto::to_string(*v.saveTo_fileExistsPolicy))).dump()); + if (v.connection_preset) { + apply(proto::SettingKey::ConnectionPreset, + nlohmann::json(std::string(proto::to_string(*v.connection_preset))).dump()); + touched_connection = true; + } + if (v.downloads_duplicatePolicy) + apply(proto::SettingKey::DownloadsDuplicatePolicy, + nlohmann::json(std::string(proto::to_string(*v.downloads_duplicatePolicy))).dump()); + if (v.proxy_mode) + apply(proto::SettingKey::ProxyMode, + nlohmann::json(std::string(proto::to_string(*v.proxy_mode))).dump()); + + // connection.* governs admission (Governor/segment budget) — push it live rather than + // wait for the next restart, same trigger Scheduler::reload_config's own doc comment + // names. + if (touched_connection && actions_) actions_->apply_settings_reload(); + + if (!changed.empty()) { + hub_.publish(proto::Event::SettingsChanged, + proto::make_notification(proto::Event::SettingsChanged, + nlohmann::json{{"keys", changed}})); + } + + proto::SettingsSetResult r; + // Echo back every key the caller actually named (not the whole bag) — its stored value + // after this write, whether or not it was among the ones that changed. + for (const auto& s : kBoolSettings) + if (v.*(s.field)) r.values.*(s.field) = settings.get_bool(proto::to_string(s.key)); + for (const auto& s : kIntSettings) + if (v.*(s.field)) r.values.*(s.field) = settings.get_int(proto::to_string(s.key)); + for (const auto& s : kStrSettings) + if (v.*(s.field)) r.values.*(s.field) = settings.get_string(proto::to_string(s.key)); + for (const auto& s : kStrArrSettings) + if (v.*(s.field)) r.values.*(s.field) = settings.get_string_array(proto::to_string(s.key)); + if (v.capture_bypassModifier) + r.values.capture_bypassModifier = proto::parse_BypassModifier( + settings.get_string("capture.bypassModifier")).value_or(*v.capture_bypassModifier); + if (v.saveTo_fileExistsPolicy) + r.values.saveTo_fileExistsPolicy = + proto::parse_SettingsSaveToFileExistsPolicy(settings.get_string("saveTo.fileExistsPolicy")) + .value_or(*v.saveTo_fileExistsPolicy); + if (v.connection_preset) + r.values.connection_preset = + proto::parse_SettingsConnectionPreset(settings.get_string("connection.preset")) + .value_or(*v.connection_preset); + if (v.downloads_duplicatePolicy) + r.values.downloads_duplicatePolicy = proto::parse_SettingsDownloadsDuplicatePolicy( + settings.get_string("downloads.duplicatePolicy")).value_or(*v.downloads_duplicatePolicy); + if (v.proxy_mode) + r.values.proxy_mode = + proto::parse_SettingsProxyMode(settings.get_string("proxy.mode")).value_or(*v.proxy_mode); + r.changed = std::move(changed); + return r; } } // namespace velox::daemon::rpc diff --git a/daemon/src/rpc/task_action_port.hpp b/daemon/src/rpc/task_action_port.hpp index a036e96..f0aa6c8 100644 --- a/daemon/src/rpc/task_action_port.hpp +++ b/daemon/src/rpc/task_action_port.hpp @@ -66,6 +66,14 @@ public: const velox::proto::DownloadProbeParams& params, std::function)> done) = 0; + + // settings.set of a connection.* key: re-read connection.maxConcurrentDownloads / + // maxActiveSegments and the per-host cap table, push the new caps to the engine and + // the governor (Scheduler::reload_config's own doc comment names this exact trigger). + // Named apply_settings_reload rather than reload_config to avoid colliding with + // sched::Scheduler's own already-public reload_config() (returns DbResult, + // consumed by main.cpp and a unit test — kept as-is rather than reshaped to fit here). + virtual void apply_settings_reload() = 0; }; } // namespace velox::daemon::rpc diff --git a/daemon/src/sched/scheduler.hpp b/daemon/src/sched/scheduler.hpp index c020ac2..fc68e53 100644 --- a/daemon/src/sched/scheduler.hpp +++ b/daemon/src/sched/scheduler.hpp @@ -135,6 +135,12 @@ public: bool provide_auth(const std::string& wire_id, const std::string& username, const std::string& password, bool remember) override; + // rpc::TaskActionPort::apply_settings_reload — a void-returning wrapper around the + // already-public reload_config() above (which returns DbResult, consumed by + // main.cpp and by sched_scheduler_test; kept as-is rather than changed to match the + // port, which has no caller that wants the DbError). + void apply_settings_reload() override { (void)reload_config(); } + // rpc::TaskActionPort. Builds a vdm::net::ProbeRequest from `params`, runs it on the // engine's probe pool (outside the segment budget, ADR 0011 §5), and converts the // result back to proto terms — including the suggestedCategoryId/-SaveDir guess (a diff --git a/daemon/src/store/settings.cpp b/daemon/src/store/settings.cpp index 3b2da51..3841746 100644 --- a/daemon/src/store/settings.cpp +++ b/daemon/src/store/settings.cpp @@ -9,24 +9,62 @@ namespace velox::daemon::store { namespace { -// Built-in defaults, mirroring Settings.schema.json / ADR 0012. Only the keys the daemon -// currently reads or is likely to need before the full settings.get handler lands; the -// rest resolve through the schema's own defaults at that layer. -constexpr std::array, 14> kDefaults{{ +// Built-in defaults, one per SettingKey (Settings.schema.json / SettingKey.schema.json). +// The schema itself carries no "default" keyword anywhere — these are what settings.get +// falls back to for a key with no stored row, chosen per ADR 0012 where it speaks (the +// connection.* buffer/segment keys) and otherwise the conservative, least-surprising +// value for that key's own description. capture.monitoredExtensions defaults to the union +// of every builtin category's extensions (0001_initial.sql's seed) rather than an +// arbitrary list of its own, so the two stay in sync without a second place to edit. +constexpr std::array, 43> kDefaults{{ + {"general.launchOnLogin", "false"}, + {"general.minimizeToTray", "false"}, + {"general.showDropTarget", "true"}, + {"general.confirmOnExit", "true"}, + {"general.language", "\"system\""}, + {"general.checkForUpdates", "true"}, + {"capture.enabled", "true"}, + {"capture.monitoredExtensions", + "[\"exe\",\"msi\",\"deb\",\"rpm\",\"dmg\",\"appimage\",\"iso\",\"zip\",\"tar\",\"gz\"," + "\"xz\",\"7z\",\"mp4\",\"mkv\",\"webm\",\"avi\",\"mov\",\"flv\",\"m4v\",\"ts\",\"mp3\"," + "\"flac\",\"aac\",\"ogg\",\"opus\",\"wav\",\"m4a\",\"pdf\",\"doc\",\"docx\",\"xls\"," + "\"xlsx\",\"ppt\",\"pptx\",\"odt\",\"epub\",\"jpg\",\"jpeg\",\"png\",\"gif\",\"webp\"," + "\"svg\",\"bmp\",\"tiff\"]"}, + {"capture.monitoredMimeTypes", "[]"}, + {"capture.minSizeBytes", "0"}, + {"capture.excludedHosts", "[]"}, + {"capture.bypassModifier", "\"shift\""}, + {"capture.autoStartTypes", "[]"}, + {"saveTo.defaultDir", "\"~/Downloads\""}, + {"saveTo.tempDir", "\"\""}, + {"saveTo.allowedRoots", "[\"~/Downloads\"]"}, + {"saveTo.fileExistsPolicy", "\"ask\""}, + {"saveTo.createSubfolderPerSite", "false"}, + {"connection.preset", "\"auto\""}, {"connection.maxSegmentsPerDownload", "8"}, {"connection.bufferBytes", "1048576"}, - {"connection.maxConcurrentDownloads", "5"}, - {"connection.maxActiveSegments", "32"}, {"connection.maxTotalBufferBytes", "134217728"}, + {"connection.maxActiveSegments", "32"}, + {"connection.maxConcurrentDownloads", "5"}, {"connection.timeoutSec", "30"}, {"connection.maxRetries", "10"}, {"connection.retryBackoffSec", "5"}, - {"saveTo.defaultDir", "\"~/Downloads\""}, - {"saveTo.allowedRoots", "[\"~/Downloads\"]"}, - {"saveTo.createSubfolderPerSite", "false"}, {"downloads.speedLimitBps", "0"}, {"downloads.speedLimitEnabled", "false"}, + {"downloads.virusScanCommand", "\"\""}, + {"downloads.postDownloadCommand", "\"\""}, {"downloads.duplicatePolicy", "\"ask\""}, + {"downloads.verifyChecksums", "true"}, + {"proxy.mode", "\"system\""}, + {"proxy.host", "\"\""}, + {"proxy.port", "1"}, + {"proxy.username", "\"\""}, + {"proxy.bypassHosts", "[]"}, + {"proxy.pacUrl", "\"\""}, + {"sounds.enabled", "true"}, + {"sounds.onComplete", "\"\""}, + {"sounds.onQueueComplete", "\"\""}, + {"sounds.onError", "\"\""}, }}; } // namespace @@ -86,6 +124,19 @@ std::int64_t Settings::get_int(std::string_view key) { return 0; } +bool Settings::get_bool(std::string_view key) { + auto raw = get_raw(key); + if (raw && *raw) { + auto j = nlohmann::json::parse(**raw, nullptr, false); + if (j.is_boolean()) return j.get(); + } + if (auto d = default_for(key)) { + auto j = nlohmann::json::parse(*d, nullptr, false); + if (j.is_boolean()) return j.get(); + } + return false; +} + std::string Settings::get_string(std::string_view key) { auto raw = get_raw(key); if (raw && *raw) { diff --git a/daemon/src/store/settings.hpp b/daemon/src/store/settings.hpp index 43b9ed2..0500e2a 100644 --- a/daemon/src/store/settings.hpp +++ b/daemon/src/store/settings.hpp @@ -35,6 +35,7 @@ public: // Typed convenience over get_raw + the defaults. A malformed stored value falls back // to the default rather than throwing. std::int64_t get_int(std::string_view key); + bool get_bool(std::string_view key); std::string get_string(std::string_view key); std::vector get_string_array(std::string_view key); diff --git a/daemon/tests/CMakeLists.txt b/daemon/tests/CMakeLists.txt index 91803c6..b687143 100644 --- a/daemon/tests/CMakeLists.txt +++ b/daemon/tests/CMakeLists.txt @@ -24,3 +24,4 @@ veloxd_test(sched_scheduler LIBS veloxd_sched veloxd_rpc) veloxd_test(event_hub LIBS veloxd_rpc) veloxd_test(store_categories_queues LIBS veloxd_store) veloxd_test(single_instance LIBS veloxd_rpc) +veloxd_test(dispatcher_settings LIBS veloxd_rpc veloxd_store) diff --git a/daemon/tests/dispatcher_settings_test.cpp b/daemon/tests/dispatcher_settings_test.cpp new file mode 100644 index 0000000..4e16321 --- /dev/null +++ b/daemon/tests/dispatcher_settings_test.cpp @@ -0,0 +1,142 @@ +// settings.get / settings.set: the field <-> SettingKey <-> JSON-type mapping table in +// dispatcher.cpp, called directly (no socket) against an in-memory store. + +#include + +#include "check.hpp" +#include "rpc/dispatcher.hpp" +#include "rpc/event_hub.hpp" +#include "store/migrations.hpp" +#include "store/sqlite.hpp" +#include "velox_proto.hpp" + +using namespace velox::daemon; +namespace proto = velox::proto; + +void run() { + auto db = store::Db::open(":memory:"); + CHECK(db.has_value()); + if (!db) return; + CHECK(store::migrate_to_head(*db).has_value()); + + rpc::EventHub hub; + rpc::VeloxDispatcher dispatcher(*db, hub); + + // --- get with keys=nullopt: every one of the 43 SettingKeys comes back ----------- + { + proto::SettingsGetParams p; + auto r = dispatcher.on_settings_get(p); + CHECK(r.has_value()); + if (r) { + int present = 0; + present += r->values.general_launchOnLogin.has_value(); + present += r->values.capture_enabled.has_value(); + present += r->values.saveTo_allowedRoots.has_value(); + present += r->values.connection_maxConcurrentDownloads.has_value(); + present += r->values.proxy_mode.has_value(); + present += r->values.sounds_onError.has_value(); + CHECK_EQ(present, 6); + // A default, sight-checked: connection.maxConcurrentDownloads is 5 (settings.cpp). + CHECK_EQ(r->values.connection_maxConcurrentDownloads.value_or(-1), std::int64_t{5}); + CHECK(r->values.saveTo_allowedRoots.has_value()); + if (r->values.saveTo_allowedRoots) + CHECK_EQ(r->values.saveTo_allowedRoots->size(), std::size_t{1}); + // Enum-typed default round-trips through parse_XXX correctly. + CHECK(r->values.proxy_mode == proto::SettingsProxyMode::System); + } + } + + // --- get with an explicit key list: only those fields are populated -------------- + { + proto::SettingsGetParams p; + p.keys = {proto::SettingKey::GeneralLaunchOnLogin, + proto::SettingKey::ConnectionMaxConcurrentDownloads}; + auto r = dispatcher.on_settings_get(p); + CHECK(r.has_value()); + if (r) { + CHECK(r->values.general_launchOnLogin.has_value()); + CHECK(r->values.connection_maxConcurrentDownloads.has_value()); + CHECK(!r->values.capture_enabled.has_value()); + CHECK(!r->values.proxy_mode.has_value()); + } + } + + // --- set: a valid change is persisted, reported in `changed`, and readable back --- + { + proto::SettingsSetParams p; + p.values.connection_maxConcurrentDownloads = 12; + p.values.general_launchOnLogin = true; + auto r = dispatcher.on_settings_set(p); + CHECK(r.has_value()); + if (r) { + CHECK_EQ(r->changed.size(), std::size_t{2}); + CHECK_EQ(r->values.connection_maxConcurrentDownloads.value_or(-1), std::int64_t{12}); + CHECK_EQ(r->values.general_launchOnLogin.value_or(false), true); + } + + proto::SettingsGetParams g; + g.keys = {proto::SettingKey::ConnectionMaxConcurrentDownloads}; + auto g_r = dispatcher.on_settings_get(g); + CHECK(g_r.has_value()); + if (g_r) CHECK_EQ(g_r->values.connection_maxConcurrentDownloads.value_or(-1), std::int64_t{12}); + } + + // --- set: setting the same value again reports it unchanged ---------------------- + { + proto::SettingsSetParams p; + p.values.connection_maxConcurrentDownloads = 12; + auto r = dispatcher.on_settings_set(p); + CHECK(r.has_value()); + if (r) CHECK(r->changed.empty()); + } + + // --- set: out of range is rejected, and nothing else in the same call lands ------ + { + proto::SettingsSetParams p; + p.values.connection_maxConcurrentDownloads = 999; // max 64 + p.values.general_minimizeToTray = true; // would otherwise succeed + auto r = dispatcher.on_settings_set(p); + CHECK(!r.has_value()); + if (!r) CHECK(r.error().code == proto::ErrorCode::InvalidParams); + + proto::SettingsGetParams g; + g.keys = {proto::SettingKey::GeneralMinimizeToTray}; + auto g_r = dispatcher.on_settings_get(g); + CHECK(g_r.has_value()); + // Rejected as a whole: minimizeToTray was never written even though its own value + // was valid on its own. + if (g_r) CHECK_EQ(g_r->values.general_minimizeToTray.value_or(true), false); + } + + // --- set: an enum field round-trips ------------------------------------------------ + { + proto::SettingsSetParams p; + p.values.proxy_mode = proto::SettingsProxyMode::Socks5; + auto r = dispatcher.on_settings_set(p); + CHECK(r.has_value()); + if (r) { + CHECK_EQ(r->changed.size(), std::size_t{1}); + CHECK(r->values.proxy_mode == proto::SettingsProxyMode::Socks5); + } + } + + // --- set: saveTo.defaultDir outside every allowed root is rejected with -32011 ---- + { + proto::SettingsSetParams p; + p.values.saveTo_defaultDir = "/definitely/not/an/allowed/root"; + auto r = dispatcher.on_settings_set(p); + CHECK(!r.has_value()); + if (!r) CHECK(r.error().code == proto::ErrorCode::InvalidPath); + } + + // --- set: saveTo.allowedRoots with an unresolvable entry is rejected with -32011 -- + { + proto::SettingsSetParams p; + p.values.saveTo_allowedRoots = {"/this/path/should/never/exist/anywhere"}; + auto r = dispatcher.on_settings_set(p); + CHECK(!r.has_value()); + if (!r) CHECK(r.error().code == proto::ErrorCode::InvalidPath); + } +} + +TEST_MAIN()