daemon: build velox-nmhost, systemd socket activation + units, velox(1) man page

Closes build order items 7 (the systemd half) and 9 (D11 in deferrals.md).

velox-nmhost (nmhost/src/main.cpp, 185 lines): a poll()-driven byte pump between
Firefox's native-messaging framing on stdio (4-byte native-byte-order length prefix)
and veloxd's own NDJSON framing on the Unix socket. Reframes each direction, no JSON
parsing, no retry/backoff (the extension relaunches a fresh host on its own
reconnect), exits the moment either side closes. Deliberately dependency-free — no
veloxd_* library, no nlohmann_json — since it runs unconfined outside Firefox's
sandbox regardless of packaging format.

Two real bugs found and fixed while getting the integration test to actually pass
rather than hang, both exactly the class of bug a "trivial pump" invites:
1. Never set the pumped fds non-blocking, so the "drain what's available" read loop
   blocked on its own second read() instead of returning to poll().
2. stdin and stdout are two different descriptors (0 and 1), not one — an early draft
   polled POLLOUT on fd 0, which is opened read-only, so EOF and writability were
   never both observable through the same pollfd entry.

packaging/nativehost/com.velox.host.json + its own README.md supersede
AGENT-DAEMON.md's stale "four locations" line: spike S1 / ADR 0003 found only three
manifest locations are real (~/.mozilla/native-messaging-hosts/ for BOTH deb/tarball
and snap Firefox, /usr/lib/mozilla/... for deb/tarball only, the flatpak sandbox path)
— the fourth, ~/snap/firefox/common/.mozilla/..., is not read by snap Firefox at all.
The README spells out the per-user-manifest / postinst enumeration implication for
PKG/QA (postinst runs once as root; the two ~/-relative locations are per-user) and
flags that docs/07-packaging.md's own install-layout line only shows the one
root-owned path.

Socket activation: rpc/systemd_activation.cpp is a from-scratch sd_listen_fds() (env
vars only — LISTEN_PID/LISTEN_FDS, fd 3 — no libsystemd link) that UdsServer::start()
checks first, skipping its own create/bind/chmod/listen when systemd already bound
the socket. packaging/systemd/velox.socket + velox.service are the unit pair,
verified both by systemd-analyze verify and by an actual fork/dup2/execve simulation
of the activation handshake — a real session.hello round-tripped over the handed-off
fd with no bind() ever called inside the daemon for that run. velox.service
deliberately skips ProtectSystem=/ProtectHome=/ReadWritePaths=: saveTo.allowedRoots is
user-configurable to anywhere on the filesystem, and a sandbox here would turn a
legitimately-configured save location into an opaque EROFS/EACCES instead of the
daemon's own clear -32011.

cli/man/velox.1 documents the CLI as it actually exists today (add/ls/pause/resume/rm,
--json) — the queue/settings subcommands AGENT-DAEMON.md's build order originally
sketched aren't implemented in cli/src/main.cpp yet, so the page doesn't claim they
are. Checked warning-free with groff -mandoc -ww -z.

Full ctest: 57/57 (excluding the pre-existing, unrelated conformance failure noted in
earlier commits).

Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GRDjHGgpYmMoPE2UFbe7pP
This commit is contained in:
2026-09-12 22:46:52 +04:00
co-authored by Claude Sonnet 5
parent 4f6c0cc9d2
commit eb72aa522c
21 changed files with 932 additions and 2 deletions
+1
View File
@@ -75,6 +75,7 @@ target_link_libraries(veloxd_sched PUBLIC velox::proto velox::core veloxd_store
add_library(veloxd_rpc STATIC
src/rpc/runtime_dir.cpp
src/rpc/single_instance.cpp
src/rpc/systemd_activation.cpp
src/rpc/event_loop.cpp
src/rpc/event_hub.cpp
src/rpc/uds_server.cpp
+2 -1
View File
@@ -7,7 +7,7 @@ close. Kept here (not buried in commit messages) so the next pass can see them a
|---|---|---|---|---|
| ~~D7~~ | **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 |
| ~~D8~~ | **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 | Pairing prompt is `EnvAutoApprover` (needs `VELOX_PAIR_AUTO=1`) | `rpc/pairing.hpp`, `main.cpp` | A GUI dialog / `org.freedesktop.Notifications` approver is integration work | the `org.freedesktop.Notifications` half of build step 7 — the systemd half closed as D11 below |
| — | **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<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 |
@@ -26,4 +26,5 @@ close. Kept here (not buried in commit messages) so the next pass can see them a
| ~~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 |
| ~~D11~~ | **Closed — build order items 7 (the systemd half) and 9: `velox-nmhost`, socket activation, the systemd user units, and `velox(1)`.** `nmhost/src/main.cpp` (185 lines): a `poll()`-driven byte pump between Firefox's native-messaging framing on stdio (4-byte native-byte-order length prefix) and `veloxd`'s own NDJSON framing on the Unix socket — reframes each direction, no JSON parsing, no retry/backoff, exits the moment either side closes. Deliberately dependency-free (no `veloxd_*` library, no `nlohmann_json`) since it runs unconfined outside Firefox's sandbox whatever the packaging format. Two real bugs found and fixed while getting the integration test to actually pass rather than hang: (1) never set the pumped fds non-blocking, so the "drain what's available" read loop blocked on its own second `read()` instead of returning to `poll()`; (2) stdin and stdout are two different descriptors (0 and 1), not one — an early draft polled `POLLOUT` on fd 0, which is opened read-only, so EOF/writability were never both observable through the same `pollfd` entry. Both are exactly the class of bug a "trivial pump" invites and unit tests over the real binary (not just its helper functions) exist specifically to catch. `packaging/nativehost/com.velox.host.json` + its own `README.md` supersede `AGENT-DAEMON.md`'s stale "four locations" (spike S1 / ADR 0003 found only three are real — the fourth, `~/snap/firefox/common/.mozilla/...`, is not read by snap Firefox at all) and spell out the per-user-manifest / postinst implication for PKG/QA. `EnginePort`-style: `rpc/systemd_activation.cpp` is a from-scratch `sd_listen_fds()` (env vars only, no `libsystemd` link — `LISTEN_PID`/`LISTEN_FDS`, fd 3) that `UdsServer::start()` checks first, skipping its own create/bind/chmod/listen when systemd already bound the socket; `packaging/systemd/velox.socket` + `velox.service` are the unit pair, verified both by `systemd-analyze verify` and by an actual fork/dup2/execve simulation of the activation handshake (a real `session.hello` round-tripped over the handed-off fd with no `bind()` ever called inside the daemon for that run). `velox.service` deliberately skips `ProtectSystem=`/`ProtectHome=`/`ReadWritePaths=``saveTo.allowedRoots` is user-configurable to anywhere on the filesystem, and a sandbox here would turn a legitimately-configured save location into an opaque `EROFS`/`EACCES` instead of the daemon's own clear `-32011`. `cli/man/velox.1` documents the CLI as it actually exists today (`add`/`ls`/`pause`/`resume`/`rm`, `--json`, the three-tier `queue`/`settings` subcommands `AGENT-DAEMON.md` build step 8 originally sketched are not implemented in `cli/src/main.cpp` yet, so the page doesn't claim they are) — checked warning-free with `groff -mandoc -ww -z`. | `nmhost/{CMakeLists.txt,src/main.cpp,tests/}`, `daemon/src/rpc/{systemd_activation.{hpp,cpp},uds_server.cpp}`, `packaging/{nativehost,systemd}/`, `cli/man/velox.1` | — | done |
| — | ~~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. |
+37
View File
@@ -0,0 +1,37 @@
#include "rpc/systemd_activation.hpp"
#include <unistd.h>
#include <cstdlib>
#include <string>
namespace velox::daemon::rpc {
namespace {
constexpr int kListenFdsStart = 3; // SD_LISTEN_FDS_START
} // namespace
int systemd_activated_fd() {
const char* pid_env = std::getenv("LISTEN_PID");
const char* fds_env = std::getenv("LISTEN_FDS");
int fd = -1;
if (pid_env != nullptr && fds_env != nullptr) {
try {
if (std::stol(pid_env) == static_cast<long>(::getpid()) && std::stol(fds_env) == 1) {
fd = kListenFdsStart;
}
} catch (...) {
// Malformed env from something other than systemd; treat as not activated.
}
}
// Contract: consumed once, then cleared, so a value meant for veloxd is never
// mistaken for one meant for a process it might itself exec later.
::unsetenv("LISTEN_PID");
::unsetenv("LISTEN_FDS");
::unsetenv("LISTEN_FDNAMES");
return fd;
}
} // namespace velox::daemon::rpc
+20
View File
@@ -0,0 +1,20 @@
#pragma once
// Minimal sd_listen_fds(3) reimplementation — one function, no libsystemd dependency, for
// the one fd velox.socket ever hands us. See velox.socket / velox.service in
// packaging/nativehost's systemd unit pair: the socket unit binds
// $XDG_RUNTIME_DIR/velox/velox.sock itself (before veloxd ever runs, so the very first
// connection attempt after boot is queued by the kernel rather than refused) and execs
// veloxd with that listening fd already open at fd 3, LISTEN_FDS=1, LISTEN_PID=<our pid>.
namespace velox::daemon::rpc {
// The systemd-activated listening socket fd, or -1 if this process was not socket-
// activated (LISTEN_PID doesn't match our pid, or LISTEN_FDS is unset/not exactly 1 — more
// than one would mean a unit file mismatch, since veloxd only ever asks for one socket).
// Clears LISTEN_PID/LISTEN_FDS from the environment on the way out either way, per
// sd_listen_fds's own contract, so a value meant for us is never mistaken for one meant for
// a process veloxd might itself exec later.
int systemd_activated_fd();
} // namespace velox::daemon::rpc
+17
View File
@@ -12,7 +12,10 @@
#include <nlohmann/json.hpp>
#include <fcntl.h>
#include "rpc/event_loop.hpp"
#include "rpc/systemd_activation.hpp"
#include "version.hpp"
namespace velox::daemon::rpc {
@@ -74,6 +77,20 @@ UdsServer::~UdsServer() {
}
std::error_code UdsServer::start() {
// velox.socket (systemd user unit, socket activation): the unit binds this path itself
// before veloxd ever runs and hands the already-listening fd over at fd 3 — the first
// connection after boot is queued by the kernel rather than refused, and there is no
// window where a client sees ECONNREFUSED while the daemon is still starting. Skips
// create/bind/chmod/listen entirely; the socket file's lifecycle (including removal on
// stop) belongs to the unit, not to us, so bound_ stays false.
if (const int activated = systemd_activated_fd(); activated >= 0) {
::fcntl(activated, F_SETFL, O_NONBLOCK);
::fcntl(activated, F_SETFD, FD_CLOEXEC);
listen_fd_ = activated;
loop_.add_fd(listen_fd_, kRead, [this](int, unsigned) { on_listener_readable(); });
return {};
}
if (path_.size() + 1 > sizeof(sockaddr_un::sun_path)) return errc(ENAMETOOLONG);
const int fd = ::socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK | SOCK_CLOEXEC, 0);
+1
View File
@@ -24,6 +24,7 @@ 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(systemd_activation LIBS veloxd_rpc)
veloxd_test(dispatcher_settings LIBS veloxd_rpc veloxd_store)
veloxd_test(capture_offer LIBS veloxd_rpc veloxd_store)
veloxd_test(dispatcher_misc LIBS veloxd_rpc veloxd_store)
+53
View File
@@ -0,0 +1,53 @@
// systemd_activated_fd(): the LISTEN_PID/LISTEN_FDS contract, without a real systemd.
#include <unistd.h>
#include <cstdlib>
#include <string>
#include "check.hpp"
#include "rpc/systemd_activation.hpp"
using namespace velox::daemon::rpc;
namespace {
void set_env(const char* k, const std::string& v) { ::setenv(k, v.c_str(), 1); }
} // namespace
void run() {
// Not activated: neither var set.
::unsetenv("LISTEN_PID");
::unsetenv("LISTEN_FDS");
CHECK_EQ(systemd_activated_fd(), -1);
// LISTEN_PID for a different process: not us, so not activated.
set_env("LISTEN_PID", std::to_string(::getpid() + 1));
set_env("LISTEN_FDS", "1");
CHECK_EQ(systemd_activated_fd(), -1);
// Consumed regardless of the outcome — a stale value from some other process's
// exec chain must not leak into what veloxd checks next time.
CHECK(::getenv("LISTEN_PID") == nullptr);
CHECK(::getenv("LISTEN_FDS") == nullptr);
// Our own pid, LISTEN_FDS=1: activated, fd 3 (SD_LISTEN_FDS_START).
set_env("LISTEN_PID", std::to_string(::getpid()));
set_env("LISTEN_FDS", "1");
CHECK_EQ(systemd_activated_fd(), 3);
CHECK(::getenv("LISTEN_PID") == nullptr);
// Our own pid but LISTEN_FDS=2: a unit file mismatch (veloxd only ever asks for one
// socket) — refuse rather than guess which of two fds is the right one.
set_env("LISTEN_PID", std::to_string(::getpid()));
set_env("LISTEN_FDS", "2");
CHECK_EQ(systemd_activated_fd(), -1);
// Garbage LISTEN_FDS: not activated, not a crash.
set_env("LISTEN_PID", std::to_string(::getpid()));
set_env("LISTEN_FDS", "not-a-number");
CHECK_EQ(systemd_activated_fd(), -1);
::unsetenv("LISTEN_PID");
::unsetenv("LISTEN_FDS");
}
TEST_MAIN()