daemon: D3 (partial) — category.upsert/remove, queue.upsert, download.remove/addBatch/provideAuth

Closes a bounded, high-value slice of the remaining D3 stubs. settings.*/rules.*/ limiter.*/schedule.*/grabber.*/media.*/capture.*/queue.reorder/download.refreshUrl/ download.update stay deferred — reasons noted individually in deferrals.md (settings.* specifically: a real, large field<->key<->JSON-type mapping table across ~43 fields / ~50 SettingKeys, not started rather than rushed). category.upsert/remove: store/categories.hpp gains get/upsert/remove. upsert generates an id when absent and always ignores the payload's `builtin` (preserved from the existing row on replace, false on create); saveDir goes through the same fs::resolve_target canonicalize-and-root-check as download.add. remove refuses a builtin at both layers (dispatcher's -32602 pre-check; the store's own "DELETE ... AND builtin = 0" as defense in depth) and reassigns member tasks to reassignTo (default "general") inside one transaction before deleting the row. queue.upsert: store/queues.hpp gains get/upsert (set_state already existed from D4b). Same create-generates-id pattern; a create always starts 'stopped', a replace keeps the queue's current run state (upsert edits config, not run state — that's queue.start/stop). Also fixed in passing: on_complete has been a real column since migration 0001 but Queues::list/get never projected it onto Queue.onComplete. download.remove: cancels with discard_partial=true (always drops the .veloxpart pair — unlike download.cancel, which keeps them, the row is gone either way), deletes the finished file only when deleteFile is true and the task was complete (best-effort), 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 via 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 against real veloxd + tools/testserver: category create/replace/ remove-with-reassignment, queue create/replace-keeps-run-state, a batch add sharing defaults.saveDir, and download.remove with deleteFile actually deleting the file and the task then 404ing download.get with -32010. Full ctest: 39/39. Co-Authored-By: Claude Sonnet 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01GRDjHGgpYmMoPE2UFbe7pP
2026-09-11 17:43:43 +04:00
parent 4f6fb1029d
commit c89158ea09
11 changed files with 493 additions and 31 deletions
@@ -1,5 +1,6 @@
 #include "rpc/dispatcher.hpp"

+#include <filesystem>
 #include <random>
 #include <string>

@@ -141,9 +142,13 @@ VeloxDispatcher::on_download_list(const proto::DownloadListParams& params) {
 }

 // --- download.add : canonicalise + root-check the destination, then persist -----------
+//
+// add_one() is the whole of download.add's body; on_download_add and on_download_addBatch
+// (each item merged against `defaults` first) both call it, so there is exactly one place
+// that turns a DownloadSpec into a stored, admitted task.

-proto::HandlerResult<proto::DownloadAddResult>
-VeloxDispatcher::on_download_add(const proto::DownloadSpec& spec) {
+proto::HandlerResult<proto::DownloadAddResult> VeloxDispatcher::add_one(
+    const proto::DownloadSpec& spec) {
    store::Settings settings(db_);

    std::string save_dir = spec.saveDir && !spec.saveDir->empty()
@@ -226,6 +231,11 @@ VeloxDispatcher::on_download_add(const proto::DownloadSpec& spec) {
    return r;
 }

+proto::HandlerResult<proto::DownloadAddResult>
+VeloxDispatcher::on_download_add(const proto::DownloadSpec& spec) {
+    return add_one(spec);
+}
+
 // --- everything else : not implemented until the store and scheduler land -------------

 proto::HandlerResult<proto::CaptureRules>
@@ -248,16 +258,94 @@ VeloxDispatcher::on_category_list(const proto::CategoryListParams&) {
    return r;
 }
 proto::HandlerResult<proto::CategoryRemoveResult>
-VeloxDispatcher::on_category_remove(const proto::CategoryRemoveParams&) {
-    return not_implemented<proto::CategoryRemoveResult>("category.remove");
+VeloxDispatcher::on_category_remove(const proto::CategoryRemoveParams& params) {
+    store::Categories categories(db_);
+    // A builtin category refuses with -32602 (category.remove's own words) — checked here
+    // rather than inferred from RemoveResult.removed=false, which also covers "no such
+    // category" and would otherwise collapse two different error stories into one.
+    auto existing = categories.get(params.categoryId);
+    if (!existing)
+        return std::unexpected(proto::HandlerError{proto::ErrorCode::InternalError,
+                                                   "category.remove: " + existing.error().message});
+    if (!existing->has_value())
+        return std::unexpected(proto::HandlerError{
+            proto::ErrorCode::InvalidParams, "no such category",
+            nlohmann::json{{"categoryId", params.categoryId}}});
+    if ((*existing)->builtin)
+        return std::unexpected(proto::HandlerError{
+            proto::ErrorCode::InvalidParams, "builtin categories cannot be removed",
+            nlohmann::json{{"categoryId", params.categoryId}}});
+
+    auto removed = categories.remove(params.categoryId, params.reassignTo);
+    if (!removed)
+        return std::unexpected(proto::HandlerError{proto::ErrorCode::InternalError,
+                                                   "category.remove: " + removed.error().message});
+    proto::CategoryRemoveResult r;
+    r.removed = removed->removed;
+    r.reassignedTaskIds = removed->reassigned_task_ids;
+    if (on_mutation_) on_mutation_();  // download.list's categoryId column just moved
+    return r;
 }
 proto::HandlerResult<proto::CategoryUpsertResult>
-VeloxDispatcher::on_category_upsert(const proto::CategoryUpsertParams&) {
-    return not_implemented<proto::CategoryUpsertResult>("category.upsert");
+VeloxDispatcher::on_category_upsert(const proto::CategoryUpsertParams& params) {
+    store::Settings settings(db_);
+    std::string save_dir = expand_tilde(params.category.saveDir);
+
+    std::vector<std::string> roots;
+    for (const auto& r : settings.get_string_array("saveTo.allowedRoots")) {
+        if (auto c = fs::canonicalize_root(r)) roots.push_back(*c);
+    }
+    // No file is ever written for this marker leaf — resolve_target only validates/creates
+    // the directory chain (fs/safepath.hpp); it never creates the leaf itself.
+    auto target = fs::resolve_target(save_dir, ".category-marker", roots);
+    if (!target) {
+        return std::unexpected(proto::HandlerError{
+            proto::ErrorCode::InvalidPath, target.error().message,
+            nlohmann::json{{"path", params.category.saveDir}}});
+    }
+
+    store::Categories categories(db_);
+    proto::Category to_store = params.category;
+    to_store.saveDir = target->dir;
+    auto stored = categories.upsert(std::move(to_store));
+    if (!stored)
+        return std::unexpected(proto::HandlerError{proto::ErrorCode::InternalError,
+                                                   "category.upsert: " + stored.error().message});
+    proto::CategoryUpsertResult r;
+    r.category = std::move(*stored);
+    return r;
 }
 proto::HandlerResult<proto::DownloadAddBatchResult>
-VeloxDispatcher::on_download_addBatch(const proto::DownloadAddBatchParams&) {
-    return not_implemented<proto::DownloadAddBatchResult>("download.addBatch");
+VeloxDispatcher::on_download_addBatch(const proto::DownloadAddBatchParams& params) {
+    proto::DownloadAddBatchResult r;
+    for (std::size_t i = 0; i < params.items.size(); ++i) {
+        proto::DownloadSpec spec = params.items[i];  // "its url is ignored" only for defaults
+        if (params.defaults) {
+            const auto& d = *params.defaults;
+            if (!spec.headers) spec.headers = d.headers;
+            if (!spec.cookies) spec.cookies = d.cookies;
+            if (!spec.referrer) spec.referrer = d.referrer;
+            if (!spec.userAgent) spec.userAgent = d.userAgent;
+            if (!spec.filename) spec.filename = d.filename;
+            if (!spec.saveDir) spec.saveDir = d.saveDir;
+            if (!spec.categoryId) spec.categoryId = d.categoryId;
+            if (!spec.queueId) spec.queueId = d.queueId;
+            if (!spec.segments) spec.segments = d.segments;
+            if (!spec.bufferBytes) spec.bufferBytes = d.bufferBytes;
+            if (!spec.startMode) spec.startMode = d.startMode;
+            if (!spec.description) spec.description = d.description;
+            if (!spec.checksum) spec.checksum = d.checksum;
+        }
+
+        auto added = add_one(spec);
+        if (added) {
+            r.taskIds.push_back(added->taskId);
+        } else {
+            r.failed.push_back({static_cast<std::int64_t>(i), added.error().code,
+                               added.error().message});
+        }
+    }
+    return r;
 }
 proto::HandlerResult<proto::BulkTaskResult>
 VeloxDispatcher::on_download_cancel(const proto::DownloadCancelParams& params) {
@@ -313,16 +401,56 @@ VeloxDispatcher::on_download_probe(const proto::DownloadProbeParams&) {
    return not_implemented<proto::DownloadProbeResult>("download.probe");
 }
 proto::HandlerResult<proto::DownloadProvideAuthResult>
-VeloxDispatcher::on_download_provideAuth(const proto::DownloadProvideAuthParams&) {
-    return not_implemented<proto::DownloadProvideAuthResult>("download.provideAuth");
+VeloxDispatcher::on_download_provideAuth(const proto::DownloadProvideAuthParams& params) {
+    if (!actions_) return not_implemented<proto::DownloadProvideAuthResult>("download.provideAuth");
+    proto::DownloadProvideAuthResult r;
+    r.ok = actions_->provide_auth(params.taskId, params.username, params.password,
+                                  params.save.value_or(false));
+    return r;
 }
 proto::HandlerResult<proto::DownloadRefreshUrlResult>
 VeloxDispatcher::on_download_refreshUrl(const proto::DownloadRefreshUrlParams&) {
    return not_implemented<proto::DownloadRefreshUrlResult>("download.refreshUrl");
 }
 proto::HandlerResult<proto::DownloadRemoveResult>
-VeloxDispatcher::on_download_remove(const proto::DownloadRemoveParams&) {
-    return not_implemented<proto::DownloadRemoveResult>("download.remove");
+VeloxDispatcher::on_download_remove(const proto::DownloadRemoveParams& params) {
+    store::Tasks tasks(db_);
+    proto::DownloadRemoveResult r;
+    for (const auto& id : params.taskIds) {
+        auto got = tasks.get(id);
+        if (!got || !got->has_value()) {
+            r.failed.push_back({id, proto::ErrorCode::TaskNotFound, "no such task"});
+            continue;
+        }
+        const store::TaskRow row = **got;  // copy: row is gone once tasks.remove() runs
+
+        // Always discard the .veloxpart/.veloxpart.meta pair (the schema's own words) —
+        // download.remove deletes the row outright, so there is no reason to leave a
+        // partial behind the way download.cancel does. A no-op if the task never had a
+        // live engine handle (already terminal, or never started).
+        if (actions_) (void)actions_->user_cancel(id, /*discard_partial=*/true);
+
+        // The finished file only when deleteFile is true — never touched for a partial or
+        // failed transfer (there is nothing there but what discard_partial above already
+        // took care of). Best-effort: a missing file is not a reason to fail the remove.
+        bool deleted_file = false;
+        if (params.deleteFile && row.state == "complete") {
+            std::error_code ec;
+            deleted_file =
+                std::filesystem::remove(std::filesystem::path(row.save_dir) / row.filename, ec);
+        }
+
+        (void)tasks.remove(id);  // segments cascade via the FK (ON DELETE CASCADE)
+        r.removed.push_back(id);
+
+        hub_.publish(proto::Event::TaskRemoved,
+                    proto::make_notification(
+                        proto::Event::TaskRemoved,
+                        nlohmann::json{{"taskId", id}, {"deletedFile", deleted_file}}),
+                    id);
+    }
+    if (on_mutation_) on_mutation_();
+    return r;
 }
 proto::HandlerResult<proto::BulkTaskResult>
 VeloxDispatcher::on_download_resume(const proto::DownloadResumeParams& params) {
@@ -436,8 +564,15 @@ VeloxDispatcher::on_queue_stop(const proto::QueueStopParams& params) {
    return r;
 }
 proto::HandlerResult<proto::QueueUpsertResult>
-VeloxDispatcher::on_queue_upsert(const proto::QueueUpsertParams&) {
-    return not_implemented<proto::QueueUpsertResult>("queue.upsert");
+VeloxDispatcher::on_queue_upsert(const proto::QueueUpsertParams& params) {
+    store::Queues queues(db_);
+    auto stored = queues.upsert(params.queue);
+    if (!stored)
+        return std::unexpected(proto::HandlerError{proto::ErrorCode::InternalError,
+                                                   "queue.upsert: " + stored.error().message});
+    proto::QueueUpsertResult r;
+    r.queue = std::move(*stored);
+    return r;
 }
 proto::HandlerResult<proto::RulesListResult>
 VeloxDispatcher::on_rules_list(const proto::RulesListParams&) {
@@ -116,6 +116,12 @@ public:
    on_settings_set(const velox::proto::SettingsSetParams&) override;

 private:
+    // The whole of download.add's body; on_download_add and on_download_addBatch (each
+    // item merged against DownloadAddBatchParams.defaults first) both call this — exactly
+    // one place turns a DownloadSpec into a stored, admitted task.
+    velox::proto::HandlerResult<velox::proto::DownloadAddResult> add_one(
+        const velox::proto::DownloadSpec& spec);
+
    velox::daemon::store::Db& db_;
    EventHub& hub_;
    TaskActionPort* actions_;
@@ -43,6 +43,14 @@ public:
    // Returns the wire ids actually paused.
    virtual std::vector<std::string> pause_queue(const std::string& queue_id) = 0;

+    // download.provideAuth: answers a task auto-paused on a 401/407. false if the task
+    // isn't currently holding a live engine handle (nothing waiting on credentials).
+    // `remember` is accepted but not yet acted on — persisting to the Secret Service isn't
+    // wired anywhere in this build yet (CLAUDE.md §4: never SQLite, never logs); this
+    // always does the "this retry only" half. Noted in deferrals.md.
+    virtual bool provide_auth(const std::string& wire_id, const std::string& username,
+                              const std::string& password, bool remember) = 0;
+
    // download.probe (D2), the File Info dialog's own network round trip — no task row
    // involved. Genuinely async (the engine's probe pool; up to the schema's 30s
    // x-deadlineMs) and so cannot fit VeloxDispatcher's synchronous on_download_probe: