Support user-hosted daemon execution under systemd --user - #148
Conversation
Add socket path resolution helpers to `comenq-lib` so both binaries agree on where a user-hosted daemon lives: - `user_socket_path` derives `$XDG_RUNTIME_DIR/comenq/comenq.sock` when a user runtime directory is available. - `default_socket_path` prefers the user runtime path and falls back to the system-wide `/run/comenq/comenq.sock`. - `discover_socket_path` returns the first existing socket among the user and system paths so clients find whichever daemon is running. The daemon's default `socket_path` now uses the context-aware resolution, and the listener creates the socket's parent directory when absent so a user-hosted daemon works without systemd's `RuntimeDirectory=` support. The client's `--socket` flag becomes optional, resolves through discovery at connect time, and gains a `COMENQ_SOCKET` environment override. Resolution happens in `Args::socket_path` rather than clap's `default_value_os_t` because clap caches computed defaults in a process-wide static and would ignore environment changes between parses. Exclude `comenq_lib` from the `no_std_fs_operations` Whitaker lint: its new discovery tests create placeholder sockets in ambient tempdirs, the same sanctioned test-support category as the cucumber crate. Move the daemon config tests to `config/tests.rs` to respect the 400-line module cap.
Add an optional `github_token_file` key to the daemon configuration.
When set, the file is read at startup and its trimmed contents
override `github_token`; an empty or unreadable file fails loudly.
A leading `${VAR}` placeholder in the path is expanded from the
environment, so a systemd unit can pass
`LoadCredential=token:%h/pandalump-token` and the configuration can
reference `github_token_file = "${CREDENTIALS_DIRECTORY}/token"`
without hard-coding the credentials directory.
`github_token` is no longer required by deserialization; instead a
validation step demands that one of the two sources yields a
non-empty token, preserving the previous failure mode for configs
with no token at all.
Also add `--github-token-file` and `--cooldown-period-seconds` to
the daemon's command-line overrides, so the posting interval can be
set without editing the configuration file. Both keys remain
settable through `COMENQD_*` environment variables as before.
Introduce a `cooldown_flutter_seconds` configuration key (default 0, disabled). After each processed request the worker now waits the full configured cooldown plus a fresh uniformly random duration between zero and the flutter ceiling. Flutter only ever lengthens the wait, so the guaranteed minimum spacing between posts is preserved while the cadence stops being perfectly regular. Use the `rand` crate for sampling and `saturating_add` so extreme configurations cannot overflow. Move the worker unit tests to `worker/tests.rs` to stay within the 400-line module cap.
Ship `packaging/linux/comenqd-user.service` for hosting the daemon
under `systemd --user`:
- `%h`-based paths for the binary and configuration file
- `RuntimeDirectory=comenq` for the socket under `$XDG_RUNTIME_DIR`
- `StateDirectory=comenq` plus `COMENQD_QUEUE_PATH` for a queue in
`~/.local/state/comenq/queue`
- `LoadCredential=token:%h/pandalump-token` feeding the PAT to the
daemon via `github_token_file =
"${CREDENTIALS_DIRECTORY}/token"`
- `WantedBy=default.target`
Add the matching example configuration
`packaging/config/comenqd-user.toml`, document the new
`github_token_file` and `cooldown_flutter_seconds` keys in the system
example config, and update the man pages, README, and design document
for socket discovery, token files, flutter, and user-mode
deployment. Cover the user unit with a packaging scenario.
The daemon opened a full `yaque::channel()` in three places: at startup for the writer's sender, in the worker spawn for its receiver, and again on writer respawn. Each `channel()` call acquires both of yaque's per-side lock files, so the worker could never start while the writer held the sender — it failed with "sender side already in use" and sat in a permanent restart loop, and a writer respawn would likewise have contended with the worker's receiver. Open exactly the required side instead: `Sender::open` for the writer (startup and respawn) and `Receiver::open` for the worker. Add a regression test that spawns the worker while a sender is held and asserts it starts and shuts down cleanly.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 41 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (28)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 10389af8c0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| [user_socket_path(), Some(PathBuf::from(DEFAULT_SOCKET_PATH))] | ||
| .into_iter() | ||
| .flatten() | ||
| .find(|candidate| candidate.exists()) |
There was a problem hiding this comment.
Fall back when the preferred socket is stale
When a user daemon exits outside the packaged systemd unit, run_listener drops the listener without unlinking its socket, so the path can continue to exist even though nothing is listening. If a healthy system daemon is also available, this existence check always selects the stale user socket; UnixStream::connect then fails without trying /run/comenq/comenq.sock, contradicting the advertised fallback. Probe candidates by connecting in order, or otherwise remove/skip stale sockets before selecting the path.
Useful? React with 👍 / 👎.
A daemon that exits outside the packaged unit leaves its socket file behind, and the existence-based discovery always selected that stale user socket, so `UnixStream::connect` failed without ever trying the healthy system daemon — contradicting the advertised fallback. Replace `discover_socket_path` with `socket_candidates`, which returns the user runtime path (when available) followed by the system path, and have the client connect to each candidate in order, using the first socket that accepts. A stale file fails to connect (typically `ECONNREFUSED`) and is skipped with a warning; the last connection error is surfaced when every candidate fails. An explicit `--socket` or `COMENQ_SOCKET` still yields exactly that path with no fallback. Add regression tests binding and dropping a listener so the socket file persists: the client must fall past it to a live socket, and report the connection error when all candidates fail.
Summary
This branch makes Comenq practical to host as a
systemd --userservice. The daemon and client now agree on a per-user socket under$XDG_RUNTIME_DIR, the GitHub Personal Access Token can be read from a file (enabling systemdLoadCredentialintegration), the cooldown gains a command-line override plus an optional random flutter, and a packaged user unit with a matching example configuration documents the whole arrangement. Deploying the result also surfaced a startup bug in the supervisor — the queue worker could never start because both queue sides were opened twice — which this branch fixes.Review walkthrough
$XDG_RUNTIME_DIR/comenq/comenq.sockwhen a user runtime directory exists, and the client connects to the first existing socket among the user and system paths.--socketbecomes an optional override with aCOMENQ_SOCKETenvironment variable, resolved inArgs::socket_pathat connect time. Clap'sdefault_value_os_tcaches computed defaults in a process-wide static, so resolving lazily is deliberate.github_token_file(read at startup, trimmed contents overridegithub_token,${VAR}prefix expanded from the environment for${CREDENTIALS_DIRECTORY}/token), thecooldown_flutter_secondskey, and the--github-token-fileand--cooldown-period-secondsoverrides, with validation that some source yields a non-empty token.saturating_add— flutter only ever lengthens the wait.yaque::channel()for the writer, the worker, and writer respawns, so the worker always failed with "sender side already in use" and restarted forever. Each site now opens exactly one side (Sender::open/Receiver::open); a regression test spawns the worker while a sender is held.RuntimeDirectory=.%hpaths,RuntimeDirectory=comenq,StateDirectory=comenq,LoadCredential=token:%h/pandalump-token,WantedBy=default.target), packaging/config/comenqd-user.toml, the updated man pages under packaging/man, and the configuration table plus §4.2.1 in docs/comenq-design.md.Validation
make lint(clippy with warnings denied plus the Whitaker Dylint suite): clean. Test modules moved toconfig/tests.rsandworker/tests.rsto respect the 400-line module cap;comenq_libjoins the sanctioned test-support exclusions in dylint.toml.make test: 136 unit and behavioural tests passed; all cucumber scenarios passed.make markdownlintandmake nixie: clean.systemd --userwith the packaged unit, credential-fed token, socket at/run/user/1000/comenq/comenq.sock, and queue in~/.local/state/comenq/queue; the client discovers the socket without flags.Notes
mainbut invisible to the existing test suite, which exercised the tasks individually rather than the assembled topology.COMENQD_COOLDOWN_FLUTTER_SECONDS.