rules.list/rules.upsert: new store/rules.{hpp,cpp} (list in priority order; apply()
upserts+removes atomically, generating an id when absent, per the schema's own "never
leaves the table in a half-valid state"). Found and fixed along the way: migration
0001's rules table had no column for Rule.name at all — every rules.list/.upsert call
failed outright ("no such column: name"), unit tests included, since :memory: migrates
through the same path. Migration 0004 adds it.
queue.reorder: new store::Queues::reorder — taskIds must be an exact permutation of
the queue's current membership (compared as sorted sets) or nothing is written and
-32602 names the queue; a valid permutation rewrites every member's queue_position in
one transaction.
schedule.get/schedule.set: a thin wrapper over the queues.schedule column (already
read since D3b, never independently settable). nextRunAt is deliberately left unset —
computing it needs the same local-time, DST-aware window logic
sched/schedule_window.hpp's window_open() only has half of; called out rather than
approximated, and the field is optional.
limiter.get/limiter.set: backed by the same downloads.speedLimitEnabled/
downloads.speedLimitBps settings keys D9 already wired — one bag of truth, not two.
The new part is reaching the engine: EnginePort/TaskActionPort gain
set_global_speed_limit(bps) (0 = unlimited, TokenBucket's own convention), wired to
Engine::rate_limiter().set_global_limit(). Pushed live on every limiter.set *and* on
Scheduler::reload_config() so a limit from a previous run isn't silently unlimited
again after a restart. applyToRunning is accepted but has no lever to pull
differently — a single shared global bucket has no "next task only" variant. Also
found, not chased further: the schema's "globalBps:0 with enabled:true means 'stop
everything'" is the opposite of what TokenBucket does with rate_bps==0 (unlimited) —
a real discrepancy, but the schema says the GUI must not offer that combination.
download.update: "moving saveDir or filename moves the file on disk in the same
operation" — resolved and root-checked like download.add's destination, then the
.veloxpart/.veloxpart.meta pair (or the finished file, if complete) is moved via
rename, falling back to copy+remove across filesystems, only when the resolved
location actually differs. categoryId/queueId(appended to the new queue's run
order)/description/segments/bufferBytes/checksum apply through new
store::Tasks::apply_update.
download.refreshUrl: same async server-layer special-case as download.probe (a real
network round trip, same 30s deadline). Re-probes, flags contentChanged only when
size or validator are both known and actually differ, persists the new URL and probe
result, and swaps the URL on a live engine handle via a newly-widened
EnginePort::refresh_url (now takes headers too, matching DownloadHandle's real
signature — the seam had silently dropped them).
Found and documented, not fixed: the generated parser collapses "field absent" and
"field explicitly null" to the same nullopt for every optional<T> patch field
(DownloadUpdateParamsPatch, Settings) — both schemas document "an explicit null
clears the field" but neither handler can act on it because the wire distinction is
already gone by the time either sees the parsed struct. A generator-level gap
(PROTO's), not something to hand-route around locally.
Verified against real veloxd + tools/testserver: rules create/list, limiter.set
takes effect and reads back, schedule.set/get round-trips, queue.reorder against real
membership (and rejects a non-permutation), download.update renames+recategorizes a
task, download.refreshUrl swaps a paused task's URL and reports contentChanged
correctly. Full ctest: 55/55 (excluding the pre-existing, unrelated conformance
failure noted two commits back).
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GRDjHGgpYmMoPE2UFbe7pP
22 KiB
22 KiB
DAEMON — deferred work, tracked
Things that are deliberately incomplete in daemon/ right now, with why and when they
close. Kept here (not buried in commit messages) so the next pass can see them at a glance.
| # | What | Where | Why deferred | Closes when |
|---|---|---|---|---|
Closed — capture.offer is real. Applies capture.enabled/excludedHosts/monitoredExtensions/monitoredMimeTypes/minSizeBytes from settings, then the rules table (store::Rules + CORE's vdm::rules::match_rules/glob_match — DAEMON only converts its own stored proto::Rule JSON into CORE's plain vdm::rules::Rule vocabulary, per that header's own layering note), resolves the category folder (a rule's explicit categoryId/saveDir, else store::Categories::guess_by_extension — the same extension-guess download.probe's suggestedCategoryId already used, now shared instead of duplicated), dedupes against active (non-terminal) tasks by exact URL, and on take calls add_one() — the same path download.add itself uses — so a captured download is a real, admitted, persisted task, not a special case. The 750 ms deadline (CLAUDE.md §4 / AGENT-DAEMON.md build step 6) is checked cooperatively between every step via a new rpc::CaptureDataSource seam (real impl wraps store::*; a test fake can jump its own clock forward to simulate "the store was slow just now" with zero real sleep) — catches the realistic failure mode (several slow steps adding up) though it can't preempt one pathologically stuck single call. Verified against real veloxd + tools/testserver: a monitored-type offer answers in ~5ms and actually creates + downloads the task; an unmonitored type, an excluded host, a rule-vetoed host, and a second offer for a still-active URL all answer ignore with the right reason; a bad category save dir surfaces its real -32011 rather than being swallowed. New capture_offer_test covers all of the above plus the deadline itself (two cases, one per "slow" checkpoint), asserting real wall-clock time barely moves even though the fake clock jumped 2 simulated seconds — proof the check reads the injected clock, not a disguised sleep. |
rpc/capture_data_source.hpp, rpc/dispatcher.{hpp,cpp}, store/rules.{hpp,cpp}, store/categories.{hpp,cpp}, store/tasks.{hpp,cpp} |
— | done | |
Closed alongside D7 — capture.getRules returns the same settings-backed enabled/monitoredExtensions/monitoredMimeTypes/minSizeBytes/excludedHosts/bypassModifier capture.offer itself reads, so the two can never drift. rulesVersion is a constant 1 — there is no persisted revision counter yet (nothing writes rules.* outside this process's own lifetime to need one across a restart), and the extension already re-fetches on event.settings.changed regardless of what this number does; noted in case a real counter becomes worth adding later. |
rpc/dispatcher.cpp |
rulesVersion is a placeholder constant |
— | |
| 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 .sos (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 |
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<T> 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: grabber.*, media.* |
rpc/dispatcher.cpp |
HLS/DASH grabber and media-variant support don't exist anywhere in this build yet — a bigger feature than a store-wiring pass | M4 territory, per AGENT-DAEMON.md |
Closed — rules.list/rules.upsert. New store/rules.{hpp,cpp}: list() in priority order, apply(upsert, remove) in one transaction (an empty ruleId generates one; a reprioritisation and a removal land atomically, per the schema's own "never leaves the table in a half-valid state"). Found and fixed along the way: migration 0001's rules table had no column for Rule.name at all — every rules.list/.upsert call failed outright ("no such column: name") the first time either ran against a real Db, unit tests included, since :memory: migrates through the same path. Migration 0004 adds it. |
store/rules.{hpp,cpp}, rpc/dispatcher.cpp, store/migrations/0004_*.sql |
— | done | |
Closed — queue.reorder. New store::Queues::reorder: taskIds must be an exact permutation of the queue's current membership (compared as sorted sets) or nothing is written and -32602 names the queue; a valid permutation rewrites every member's queue_position in one transaction. |
store/queues.{hpp,cpp}, rpc/dispatcher.cpp |
— | done | |
Closed — schedule.get/schedule.set. Thin wrapper over the queues.schedule column that already existed (Queue.schema.json's own field, read since D3b but never independently settable). nextRunAt is deliberately left unset: computing it correctly needs the same local-time, DST-aware window logic sched/schedule_window.hpp's window_open() only has half of (is-it-open-right-now, not next-transition) — real work, called out rather than approximated. The field is optional; nullopt is a legal answer. |
store/queues.{hpp,cpp}, rpc/dispatcher.cpp |
nextRunAt unset (documented, not silently wrong) |
nextRunAt is its own pass |
|
Closed — limiter.get/limiter.set. Backed by the same downloads.speedLimitEnabled/downloads.speedLimitBps settings keys D9 already wired (one bag of truth, not two) — the new part is actually reaching the engine: EnginePort/TaskActionPort gain set_global_speed_limit(bps) (0 = unlimited, vdm::rate::TokenBucket's own convention), wired to vdm::Engine::rate_limiter().set_global_limit(). Pushed live on every limiter.set and on Scheduler::reload_config() (so a limit from a previous run isn't silently unlimited again after a restart — nothing else re-derives it from settings the way connection.* already does). applyToRunning is accepted but has no lever to pull differently: a single shared global bucket has no "next task only" variant, so this always behaves as if it were true — documented in EnginePort::set_global_speed_limit's own comment. Also found, not chased further: the schema's "globalBps: 0 with enabled: true means 'stop everything'" is the opposite of what TokenBucket does with rate_bps == 0 (unlimited) — a real discrepancy, but the schema itself says the GUI "must not offer" that combination, so nothing sends it in practice. |
sched/engine_port*.hpp, rpc/task_action_port.hpp, sched/scheduler.{cpp,hpp}, rpc/dispatcher.cpp |
the globalBps:0 semantic clash noted above; applyToRunning has no real effect |
flagged for whoever owns the schema/CORE conversation next | |
Closed — download.update. "Moving saveDir or filename moves the file on disk in the same operation" (the schema's own words): resolved and root-checked exactly like download.add's destination, then the .veloxpart/.veloxpart.meta pair (or the finished file, if the task is complete) is moved with std::filesystem::rename, falling back to copy+remove across filesystems (EXDEV) — only if the resolved location actually differs from where the task already is. categoryId/queueId(appended to the end of the new queue's run order)/description/segments(1-32)/bufferBytes(64 KiB-16 MiB)/checksum all apply through a new store::Tasks::apply_update. New store::Tasks::UpdatePatch/apply_update and Tasks::set_url (download.refreshUrl's own need, split out since neither apply_update nor set_probe_result owns the base url column). |
store/tasks.{hpp,cpp}, rpc/dispatcher.cpp |
see the shared note below (null-clearing) | done | |
Closed — download.refreshUrl. Same async reasoning and the same server-layer special-case (uds_server.cpp/ws_server.cpp intercept before generic dispatch(), exactly like download.probe) — a real network round trip, same 30s x-deadlineMs. Re-probes the new URL, flags contentChanged only when size or validator are both known and actually differ (an unknown value on either side is never itself a mismatch — "it says so rather than silently restarting" needs a real disagreement, not an absent comparison), persists the new URL and probe result, and — if the task holds a live engine handle — swaps the URL in place via a newly-widened EnginePort::refresh_url (now takes headers too, matching DownloadHandle::refresh_url's real signature; the seam had silently dropped them). Verified against real veloxd + tools/testserver, live-handle swap covered by a sched_scheduler_test case (asserts the engine got refresh_url(), not a fresh start()). |
sched/engine_port*.hpp, rpc/task_action_port.hpp, sched/scheduler.{cpp,hpp}, rpc/{uds_server,ws_server}.{hpp,cpp} |
— | done | |
| — | Shared note across D3h/download.update and D9/settings.set: the generated parser collapses "field absent" and "field explicitly null" to the same std::optional::nullopt for every optional<T> patch field (DownloadUpdateParamsPatch, Settings) — there is no second bit on the wire path that survives into VeloxDispatcher. Both schemas document "an explicit null clears the field," but neither handler can act on that distinction because the information is already gone by the time either sees the parsed struct. Not something to route around locally (would mean hand-parsing raw JSON past the generated parse<T> for a handful of fields) — this is a generator-level gap, PROTO's to close (e.g. std::optional<std::optional<T>>, or a parallel "which fields were present" bitset). Until then: categoryId/queueId/description/checksum on download.update, and every nullable Settings key, can be set through these RPCs but never explicitly cleared back to null. |
contracts/ (generator), affects rpc/dispatcher.cpp |
the wire distinction the schema documents doesn't survive to the handler | PROTO's generator |
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 SettingKeys 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 | |
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 |
|
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 | |
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 |
|
Closed — sched/engine_port_core.hpp wraps vdm::Engine + segment_budget(); main.cpp constructs Engine + Scheduler, calls reconcile_after_restart / reload_config / tick at startup |
— | — | done (lane/core stage 8 merged) |
|
Closed — download.pause/resume/start/cancel and queue.start/stop all drive the scheduler now, and apply immediately (not deferred to the next tick — pausing/resuming/cancelling a live transfer can't wait up to 1s, and per ADR 0013 §3 the governor never touches a user-owned pause on its own). New rpc::TaskActionPort interface (owned by rpc/, implemented by sched::Scheduler) is the seam dispatcher.hpp depends on instead of sched/scheduler.hpp directly — avoids a real veloxd_rpc <-> veloxd_sched circular library dependency (veloxd_sched already links veloxd_rpc for EventHub). Scheduler::user_pause/resume/start/cancel + pause_queue engine-call-then-eager-transition, matching tick()'s existing to_pause pattern. Fixed a real bug hit while building this: transition() always overwrote pause_reason to NULL when the engine's own delayed pause-ack callback arrived with no explicit reason, clobbering whatever the actual initiator (user or governor) had just written — now it preserves the stored reason when none is supplied. Verified against real veloxd + tools/testserver: pausing a live single-segment throttled transfer freezes downloadedBytes, resume continues it from that point, cancel stops it; queue.stop(pauseRunning:true) pauses the queue's running task immediately. NOTE: download.start's contract "a task in 'queued' jumps its queue" (priority bump) is not implemented — admission is still plain FIFO by created_at. |
sched/scheduler.{cpp,hpp}, rpc/task_action_port.hpp, rpc/dispatcher.{hpp,cpp}, store/queues.{cpp,hpp} |
— | done, except the queue-jump priority bump noted above | |
Mostly closed — rpc/event_hub fans out per-subscription; session.subscribe on both transports registers/updates/tears down a real subscription; Scheduler::transition() publishes event.task.state (with previousState) on every state change, scheduler-driven or engine-reported; dispatcher::on_download_add publishes event.task.added; a 250 ms timer batches Scheduler::progress_snapshot() into one event.task.progress array per AGENT-DAEMON.md item 5 / the schema's x-maxRateHz: 4. Verified live end to end. |
— | event.task.removed has no source yet (download.remove is D3); event.speed.global, event.notify, event.auth.required, event.settings.changed, event.grabber.progress are unpublished — each lands with its owning handler |
as each owning D3 handler lands | |
Closed — engine numbers now reach the store: Scheduler::tick() probes (EnginePort::probe) before every start(), persisting sizeBytes/resumable/validators via Tasks::set_probe_result before a byte moves; Scheduler::persist_progress() (called from progress_snapshot() and once more from on_engine_state right before release()/unmap on every terminal transition) writes downloadedBytes/speedBps/segments/segmentDetail from the engine's Progress, so a task that finishes between two 250 ms ticks (the common case for anything small or fast) still leaves real numbers instead of the pre-persistence defaults. TaskSummary.segments is sourced from segments.size() when the task has any (matching what actually lands in segmentDetail, per the schema's "exactly segments entries"), falling back to the engine's effective_segments (budget slots held, not necessarily physical range count — see core/include/vdm/task/download.hpp's Progress comment) only pre-segmentation. Tasks::set_final_bytes tops up on_finished's byte count as a last-resort backstop. Migration 0002 adds speed_bps to both tasks and segments, and fixes segments.state's CHECK to include 'pending' (0001 omitted it, so a pre-connect snapshot could never be written). Verified against real veloxd + tools/testserver (not just unit tests): download.list/download.get correct immediately after completion and after a daemon restart. |
sched/scheduler.{cpp,hpp}, store/{tasks,segments}.{cpp,hpp}, store/migrations/0002_*.sql |
— | done | |
| — | vdm::task::Progress.speed_bps reads back as 0 for the whole lifetime of a live, real (non-fake) throttled download, despite downloadedBytes visibly advancing between polls — core/src/task/download_task.cpp's per-worker EWMA never seems to produce a nonzero aggregate in this build. DAEMON passes EnginePort::progress()'s speed_bps straight through (Scheduler::persist_progress); nothing in this lane drops it. Still reproduces in the D4b live checks above (0 throughout a paused/resumed/cancelled transfer whose downloadedBytes visibly moved) — not re-filed, since it's already CORE's. |