daemon: D4b — download.pause/resume/start/cancel and queue.start/stop drive the scheduler
download.pause/resume/start/cancel and queue.start/stop were stubs; now they call into the scheduler and take effect immediately, not on the next 1s tick — pausing, resuming or cancelling a live transfer can't wait, 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 what dispatcher.hpp depends on instead of sched/scheduler.hpp directly. Needed because veloxd_sched already links veloxd_rpc (for EventHub); dispatcher.hpp pulling in sched/scheduler.hpp directly would make it a real circular library dependency, breaking anything that links veloxd_rpc alone (cli's tests, as it turned out — hit and fixed during this change). Scheduler::user_pause/user_resume/user_start/user_cancel + pause_queue follow tick()'s existing to_pause pattern: call the engine (async, no synchronous effect) and transition the store eagerly so download.get/list are correct the instant the RPC call returns. Fixed a real bug surfaced while building this: transition() always overwrote pause_reason to NULL when the engine's own delayed pause-ack callback (on_state to paused, no error) arrived after whoever actually initiated the pause had already written the real reason — now it preserves the stored reason when the callback supplies none, instead of clobbering it. Covered by a regression check in sched_scheduler_test. store/queues gets get() and set_state() (was list()-only) for queue.start/stop. Verified against real veloxd + tools/testserver, not just unit tests: pausing a live single-segment throttled transfer freezes downloadedBytes, resume continues it from that point, cancel stops it; a bad taskId comes back in BulkTaskResult.failed with -32010, not a crash; queue.stop(pauseRunning:true) pauses the queue's running task immediately and queue.start resumes admission. Known gap: download.start's contract "a task in 'queued' jumps its queue" (priority bump) is not implemented — admission is still plain FIFO by created_at. Noted in deferrals.md. Co-Authored-By: Claude Sonnet 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01GRDjHGgpYmMoPE2UFbe7pP
This commit is contained in:
@@ -7,9 +7,9 @@ close. Kept here (not buried in commit messages) so the next pass can see them a
|
||||
|---|---|---|---|---|
|
||||
| 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) |
|
||||
| D2 | `download.probe` → `-32603` | `rpc/dispatcher.cpp` | `download.add` is wired (`fs/safepath` + store, real `-32011`); `download.probe` needs the engine's probe path for `-32013` | probe with the engine link (CORE stage 3 is landed; wire `Engine::probe`) |
|
||||
| D3 | Stub handlers for the rest: `download.pause/resume/start/cancel/remove/addBatch/refreshUrl/provideAuth`, `rules.*`, `settings.*`, `limiter.*`, `schedule.*`, `queue.upsert/reorder/start/stop`, `category.upsert/remove`, `grabber.*`, `media.*` | `rpc/dispatcher.cpp` | No store/scheduler wiring behind them yet. `category.list` and `queue.list` are done (`store/categories`, `store/queues`) | Per method, as each wires to the store/scheduler |
|
||||
| D3 | Stub handlers for the rest: `download.remove/addBatch/refreshUrl/provideAuth/update`, `rules.*`, `settings.*`, `limiter.*`, `schedule.*`, `queue.upsert/reorder`, `category.upsert/remove`, `grabber.*`, `media.*`, `capture.*` | `rpc/dispatcher.cpp` | No store/scheduler wiring behind them yet. `category.list`, `queue.list`, `queue.start`/`stop` are done | Per method, as each wires to the store/scheduler |
|
||||
| ~~D4a~~ | **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) |
|
||||
| D4b | timer + nudges: a 1 s `timerfd` re-runs `Scheduler::tick()` and `download.add` nudges via `on_mutation`. `download.pause`/`resume`/`start`/`cancel` and the queue.* handlers still don't touch the scheduler | `rpc/dispatcher.cpp` | those handlers are still stubs (D3) | as each handler is implemented behind the store, it calls `on_mutation` / drives the scheduler |
|
||||
| ~~D4b~~ | **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 |
|
||||
| ~~D5~~ | **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 |
|
||||
| ~~D6~~ | **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 |
|
||||
| — | **Observed, not fixed (CORE, not this lane):** `vdm::task::Progress.speed_bps` reads back as `0` for the whole lifetime of a live, real (non-fake) throttled download in the E2E check above, despite `downloadedBytes` visibly advancing between polls — `core/src/task/download_task.cpp`'s per-worker EWMA (`w->speed_bps`, ~line 505-513) 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. Filed here rather than worked around — CLAUDE.md §2/§3: not core/'s owner, don't patch around a wrong upstream number locally. Confirm with CORE before the GUI's live speed readout ships. |
|
||||
| — | ~~Observed, not fixed (CORE, not this lane)~~ — **routed to CORE by the user.** `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. |
|
||||
|
||||
Reference in New Issue
Block a user