From 414b7254261c0e739e73b8409cef5eba01770263 Mon Sep 17 00:00:00 2001 From: Leynos Date: Sun, 27 Jul 2025 02:14:27 +0100 Subject: [PATCH 1/6] Add daemon core tasks --- AGENTS.md | 32 +++++++++++++++++--------------- Cargo.lock | 25 +++++++++++++++++++++++++ crates/comenqd/src/daemon.rs | 26 ++++++++++++++++++++++++++ crates/comenqd/src/logging.rs | 26 ++++++++++++++++++++++++++ crates/comenqd/src/main.rs | 4 +++- docs/comenq-design.md | 5 ++++- docs/roadmap.md | 6 +++--- 7 files changed, 104 insertions(+), 20 deletions(-) create mode 100644 crates/comenqd/src/logging.rs diff --git a/AGENTS.md b/AGENTS.md index d6350ac..06f65d1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -9,8 +9,8 @@ - **Clarity over cleverness.** Be concise, but favour explicit over terse or obscure idioms. Prefer code that's easy to follow. - **Use functions and composition.** Avoid repetition by extracting reusable - logic. Prefer generators or comprehensions, and declarative code to imperative - repetition when readable. + logic. Prefer generators or comprehensions, and declarative code to + imperative repetition when readable. - **Small, meaningful functions.** Functions must be small, clear in purpose, single responsibility, and obey command/query segregation. - **Clear commit messages.** Commit messages should be descriptive, explaining @@ -25,12 +25,14 @@ ("-ize" / "-yse" / "-our") spelling and grammar, with the exception of references to external APIs. - **Illustrate with clear examples.** Function documentation must include clear - examples demonstrating the usage and outcome of the function. Test documentation - should omit examples where the example serves only to reiterate the test logic. -- **Keep file size managable.** No single code file may be longer than 400 lines. + examples demonstrating the usage and outcome of the function. Test + documentation should omit examples where the example serves only to reiterate + the test logic. +- **Keep file size managable.** No single code file may be longer than 400 + lines. Long switch statements or dispatch tables should be broken up by feature and - constituents colocated with targets. Large blocks of test data should be moved - to external data files. + constituents colocated with targets. Large blocks of test data should be + moved to external data files. ## Documentation Maintenance @@ -42,8 +44,8 @@ relevant file(s) in the `docs/` directory to reflect the latest state. **Ensure the documentation remains accurate and current.** - Documentation must use en-GB-oxendict ("-ize" / "-yse" / "-our") spelling - and grammar. (EXCEPTION: the naming of the "LICENSE" file, which - is to be left unchanged for community consistency.) + and grammar. (EXCEPTION: the naming of the "LICENSE" file, which is to be + left unchanged for community consistency.) ## Change Quality & Committing @@ -153,19 +155,19 @@ project: specified in `Cargo.toml` must use SemVer-compatible caret requirements (e.g., `some-crate = "1.2.3"`). This is Cargo's default and allows for safe, non-breaking updates to minor and patch versions while preventing breaking - changes from new major versions. This approach is critical for ensuring - build stability and reproducibility. + changes from new major versions. This approach is critical for ensuring build + stability and reproducibility. - **Prohibit unstable version specifiers.** The use of wildcard (`*`) or - open-ended inequality (`>=`) version requirements is strictly forbidden - as they introduce unacceptable risk and unpredictability. Tilde requirements + open-ended inequality (`>=`) version requirements is strictly forbidden as + they introduce unacceptable risk and unpredictability. Tilde requirements (`~`) should only be used where a dependency must be locked to patch-level updates for a specific, documented reason. ### Error Handling - **Prefer semantic error enums**. Derive `std::error::Error` (via the - `thiserror` crate) for any condition the caller might inspect, retry, or - map to an HTTP status. + `thiserror` crate) for any condition the caller might inspect, retry, or map + to an HTTP status. - **Use an *opaque* error only at the app boundary**. Use `eyre::Report` for human-readable logs; these should not be exposed in public APIs. - **Never export the opaque type from a library**. Convert to domain enums at diff --git a/Cargo.lock b/Cargo.lock index f456bed..6059b6c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -561,6 +561,22 @@ version = "2.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "37909eebbb50d72f9059c3b6d82c0463f2ff062c9e95845c43a6c9c0355411be" +[[package]] +name = "figment" +version = "0.10.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8cb01cd46b0cf372153850f4c6c272d9cbea2da513e07538405148f95bd789f3" +dependencies = [ + "atomic", + "parking_lot", + "pear", + "serde", + "tempfile", + "toml", + "uncased", + "version_check", +] + [[package]] name = "filetime" version = "0.2.25" @@ -3204,6 +3220,15 @@ version = "0.53.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "271414315aff87387382ec3d271b52d7ae78726f5d44ac98b4f4030c91880486" +[[package]] +name = "winnow" +version = "0.7.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f3edebf492c8125044983378ecb5766203ad3b4c2f7a922bd7dd207f6d443e95" +dependencies = [ + "memchr", +] + [[package]] name = "wit-bindgen-rt" version = "0.39.0" diff --git a/crates/comenqd/src/daemon.rs b/crates/comenqd/src/daemon.rs index 4a9b226..00cf2b2 100644 --- a/crates/comenqd/src/daemon.rs +++ b/crates/comenqd/src/daemon.rs @@ -33,6 +33,8 @@ fn prepare_listener(path: &Path) -> Result { /// Start the daemon with the provided configuration. pub async fn run(config: Config) -> Result<()> { + fs::create_dir_all(&config.queue_path)?; + tracing::info!(queue = %config.queue_path.display(), "Queue directory prepared"); let octocrab = Arc::new(build_octocrab(&config.github_token)?); let (tx, rx) = channel(&config.queue_path)?; let cfg = Arc::new(config); @@ -96,3 +98,27 @@ async fn run_worker(config: Arc, mut rx: Receiver, octocrab: Arc anyhow::Result<()> { - tracing_subscriber::fmt::init(); + logging::init(); let cfg = Config::load()?; info!(socket = ?cfg.socket_path, queue = ?cfg.queue_path, "Comenqd daemon started"); run(cfg).await diff --git a/docs/comenq-design.md b/docs/comenq-design.md index 135ffc3..380c49b 100644 --- a/docs/comenq-design.md +++ b/docs/comenq-design.md @@ -1013,7 +1013,10 @@ async fn run_worker(config: Arc, mut rx: Receiver, octoc The repository initialises the workspace with `comenq-lib` at the root and two binary crates under `crates/`. `CommentRequest` resides in the library and derives both `Serialize` and `Deserialize`. The daemon now spawns a Unix -listener and queue worker as described above. +listener and queue worker as described above. Structured logging is initialised +using `tracing_subscriber` with JSON output controlled by the `RUST_LOG` +environment variable. The queue directory is created on start if it does not +already exist before `yaque` opens it. ## Works cited diff --git a/docs/roadmap.md b/docs/roadmap.md index 7d321d8..a632f45 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -34,13 +34,13 @@ (`/etc/comenqd/config.toml`) for parameters like `github_token`, `socket_path`, and `queue_path`. -- [ ] Set up structured logging using the `tracing` and `tracing-subscriber` +- [x] Set up structured logging using the `tracing` and `tracing-subscriber` crates. -- [ ] Initialize the `yaque` persistent queue at the path specified in the +- [x] Initialize the `yaque` persistent queue at the path specified in the configuration. -- [ ] Structure the daemon's `main` function to spawn the two primary, +- [x] Structure the daemon's `main` function to spawn the two primary, long-running `tokio` tasks: the UDS listener and the queue worker. ## Milestone 4: `comenqd` Daemon — UDS Listener Task From eb7a88f6af2ed41ce20235b8c6e4dfbe677a6211 Mon Sep 17 00:00:00 2001 From: Leynos Date: Sun, 27 Jul 2025 02:23:21 +0100 Subject: [PATCH 2/6] Fix queue init and logging --- AGENTS.md | 9 ++++----- crates/comenqd/src/daemon.rs | 34 +++++++++++++++------------------- crates/comenqd/src/logging.rs | 1 - docs/comenq-design.md | 4 ++-- 4 files changed, 21 insertions(+), 27 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 06f65d1..fb33d43 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -28,11 +28,10 @@ examples demonstrating the usage and outcome of the function. Test documentation should omit examples where the example serves only to reiterate the test logic. -- **Keep file size managable.** No single code file may be longer than 400 - lines. - Long switch statements or dispatch tables should be broken up by feature and - constituents colocated with targets. Large blocks of test data should be - moved to external data files. +- **Keep file size manageable.** No single code file may be longer than 400 + lines. Long switch statements or dispatch tables should be broken up by + feature and constituents colocated with targets. Large blocks of test data + should be moved to external data files. ## Documentation Maintenance diff --git a/crates/comenqd/src/daemon.rs b/crates/comenqd/src/daemon.rs index 00cf2b2..75fdd6a 100644 --- a/crates/comenqd/src/daemon.rs +++ b/crates/comenqd/src/daemon.rs @@ -7,11 +7,12 @@ use crate::config::Config; use anyhow::Result; use comenq_lib::CommentRequest; use octocrab::Octocrab; -use std::fs; +use std::fs as stdfs; use std::os::unix::fs::PermissionsExt; use std::path::Path; use std::sync::Arc; use std::time::Duration; +use tokio::fs; use tokio::io::AsyncReadExt; use tokio::net::{UnixListener, UnixStream}; use yaque::{Receiver, Sender, channel}; @@ -23,17 +24,22 @@ fn build_octocrab(token: &str) -> Result { } fn prepare_listener(path: &Path) -> Result { - if fs::metadata(path).is_ok() { - fs::remove_file(path)?; + if stdfs::metadata(path).is_ok() { + stdfs::remove_file(path)?; } let listener = UnixListener::bind(path)?; - fs::set_permissions(path, fs::Permissions::from_mode(0o660))?; + stdfs::set_permissions(path, stdfs::Permissions::from_mode(0o660))?; Ok(listener) } +async fn ensure_queue_dir(path: &Path) -> Result<()> { + fs::create_dir_all(path).await?; + Ok(()) +} + /// Start the daemon with the provided configuration. pub async fn run(config: Config) -> Result<()> { - fs::create_dir_all(&config.queue_path)?; + ensure_queue_dir(&config.queue_path).await?; tracing::info!(queue = %config.queue_path.display(), "Queue directory prepared"); let octocrab = Arc::new(build_octocrab(&config.github_token)?); let (tx, rx) = channel(&config.queue_path)?; @@ -105,20 +111,10 @@ mod tests { use tempfile::tempdir; #[tokio::test] - async fn run_creates_queue_dir() { + async fn ensure_queue_dir_creates_directory() { let dir = tempdir().unwrap(); - let cfg = Config { - github_token: "tok".into(), - socket_path: dir.path().join("sock"), - queue_path: dir.path().join("queue"), - cooldown_period_seconds: 1, - }; - - let queue_dir = cfg.queue_path.clone(); - let handle = tokio::spawn(run(cfg)); - tokio::time::sleep(Duration::from_millis(50)).await; - assert!(queue_dir.is_dir()); - handle.abort(); - let _ = handle.await; + let path = dir.path().join("queue"); + ensure_queue_dir(&path).await.unwrap(); + assert!(path.is_dir()); } } diff --git a/crates/comenqd/src/logging.rs b/crates/comenqd/src/logging.rs index 1b4f6df..f203410 100644 --- a/crates/comenqd/src/logging.rs +++ b/crates/comenqd/src/logging.rs @@ -7,7 +7,6 @@ use tracing_subscriber::{EnvFilter, fmt}; /// Initialize the global tracing subscriber. -#[allow(dead_code)] pub fn init() { fmt() .with_env_filter(EnvFilter::from_default_env()) diff --git a/docs/comenq-design.md b/docs/comenq-design.md index 380c49b..7b72ec8 100644 --- a/docs/comenq-design.md +++ b/docs/comenq-design.md @@ -1015,8 +1015,8 @@ binary crates under `crates/`. `CommentRequest` resides in the library and derives both `Serialize` and `Deserialize`. The daemon now spawns a Unix listener and queue worker as described above. Structured logging is initialised using `tracing_subscriber` with JSON output controlled by the `RUST_LOG` -environment variable. The queue directory is created on start if it does not -already exist before `yaque` opens it. +environment variable. The queue directory is created asynchronously on start if +it does not already exist before `yaque` opens it. ## Works cited From c3b3678a002ef05f76711c5aee4e667c73d2470e Mon Sep 17 00:00:00 2001 From: Leynos Date: Sun, 27 Jul 2025 03:08:14 +0100 Subject: [PATCH 3/6] Improve daemon and logging tests --- crates/comenqd/src/daemon.rs | 35 +++++++++++++++++++++++-- crates/comenqd/src/logging.rs | 49 ++++++++++++++++++++++++++++++++++- 2 files changed, 81 insertions(+), 3 deletions(-) diff --git a/crates/comenqd/src/daemon.rs b/crates/comenqd/src/daemon.rs index 75fdd6a..acf0a2d 100644 --- a/crates/comenqd/src/daemon.rs +++ b/crates/comenqd/src/daemon.rs @@ -109,12 +109,43 @@ async fn run_worker(config: Arc, mut rx: Receiver, octocrab: Arc timeout { + handle.abort(); + panic!("queue directory not created"); + } + sleep(Duration::from_millis(10)).await; + } + + handle.abort(); + } } diff --git a/crates/comenqd/src/logging.rs b/crates/comenqd/src/logging.rs index f203410..4b03c98 100644 --- a/crates/comenqd/src/logging.rs +++ b/crates/comenqd/src/logging.rs @@ -4,12 +4,22 @@ //! `tracing-subscriber`, reading filter settings from the `RUST_LOG` //! environment variable. +use tracing_subscriber::fmt::MakeWriter; use tracing_subscriber::{EnvFilter, fmt}; /// Initialize the global tracing subscriber. pub fn init() { + init_with_writer(fmt::writer::BoxMakeWriter::new(std::io::stdout)); +} + +/// Initialize logging with a custom writer. +pub fn init_with_writer(writer: W) +where + W: for<'a> MakeWriter<'a> + Send + Sync + 'static, +{ fmt() .with_env_filter(EnvFilter::from_default_env()) + .with_writer(writer) .json() .init(); } @@ -17,9 +27,46 @@ pub fn init() { #[cfg(test)] mod tests { use super::*; + use std::sync::{Arc, Mutex}; + use tracing::info; + + #[derive(Clone)] + struct BufMakeWriter { + buf: Arc>>, + } + + impl<'a> MakeWriter<'a> for BufMakeWriter { + type Writer = BufWriter; + + fn make_writer(&'a self) -> Self::Writer { + BufWriter { + buf: self.buf.clone(), + } + } + } + + struct BufWriter { + buf: Arc>>, + } + + impl std::io::Write for BufWriter { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + self.buf.lock().expect("lock buffer").extend_from_slice(buf); + Ok(buf.len()) + } + + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } + } #[test] fn init_logging() { - init(); + let buf = Arc::new(Mutex::new(Vec::new())); + std::env::set_var("RUST_LOG", "info"); + init_with_writer(BufMakeWriter { buf: buf.clone() }); + info!("captured"); + let output = String::from_utf8(buf.lock().expect("lock buffer").clone()).expect("utf8"); + assert!(output.contains("captured")); } } From 7197315d8a0737c48a644acbc587e9b5044fcb1d Mon Sep 17 00:00:00 2001 From: Leynos Date: Sun, 27 Jul 2025 03:14:30 +0100 Subject: [PATCH 4/6] Refine guidelines and queue tests --- AGENTS.md | 4 ++-- crates/comenqd/src/daemon.rs | 8 +++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index fb33d43..0b5f688 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -16,7 +16,7 @@ - **Clear commit messages.** Commit messages should be descriptive, explaining what was changed and why. - **Name things precisely.** Use clear, descriptive variable and function names. - For booleans, prefer names with `is`, `has`, or `should`. + For booleans, prefer names with `is`, `has`, `should`. - **Structure logically.** Each file should encapsulate a coherent module. Group related code (e.g., models + utilities + fixtures) close together. - **Group by feature, not layer.** Colocate views, logic, fixtures, and helpers @@ -157,7 +157,7 @@ project: changes from new major versions. This approach is critical for ensuring build stability and reproducibility. - **Prohibit unstable version specifiers.** The use of wildcard (`*`) or - open-ended inequality (`>=`) version requirements is strictly forbidden as + open-ended inequality (`>=`) version requirements are strictly forbidden, as they introduce unacceptable risk and unpredictability. Tilde requirements (`~`) should only be used where a dependency must be locked to patch-level updates for a specific, documented reason. diff --git a/crates/comenqd/src/daemon.rs b/crates/comenqd/src/daemon.rs index acf0a2d..507b090 100644 --- a/crates/comenqd/src/daemon.rs +++ b/crates/comenqd/src/daemon.rs @@ -113,15 +113,17 @@ mod tests { #[tokio::test] async fn ensure_queue_dir_creates_directory() { - let dir = tempdir().expect("create temp dir"); + let dir = tempdir().expect("Failed to create temporary directory"); let path = dir.path().join("queue"); - ensure_queue_dir(&path).await.expect("create dir"); + ensure_queue_dir(&path) + .await + .expect("Failed to ensure queue directory"); assert!(path.is_dir()); } #[tokio::test] async fn run_creates_queue_directory() { - let dir = tempdir().expect("create temp dir"); + let dir = tempdir().expect("Failed to create temporary directory"); let cfg = Config { github_token: "t".into(), socket_path: dir.path().join("sock"), From 142a10669b97d1536eb779169bb8e5ab3ba5a087 Mon Sep 17 00:00:00 2001 From: Leynos Date: Sun, 27 Jul 2025 13:06:26 +0100 Subject: [PATCH 5/6] Improve daemon tests and logging errors --- crates/comenqd/src/daemon.rs | 7 ++----- crates/comenqd/src/logging.rs | 8 ++++++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/crates/comenqd/src/daemon.rs b/crates/comenqd/src/daemon.rs index 507b090..b13983a 100644 --- a/crates/comenqd/src/daemon.rs +++ b/crates/comenqd/src/daemon.rs @@ -138,16 +138,13 @@ mod tests { let start = Instant::now(); let timeout = Duration::from_secs(2); loop { - if cfg.queue_path.is_dir() { + if cfg.queue_path.is_dir() || start.elapsed() > timeout { break; } - if start.elapsed() > timeout { - handle.abort(); - panic!("queue directory not created"); - } sleep(Duration::from_millis(10)).await; } handle.abort(); + assert!(cfg.queue_path.is_dir(), "queue directory not created"); } } diff --git a/crates/comenqd/src/logging.rs b/crates/comenqd/src/logging.rs index 4b03c98..14162c0 100644 --- a/crates/comenqd/src/logging.rs +++ b/crates/comenqd/src/logging.rs @@ -51,7 +51,10 @@ mod tests { impl std::io::Write for BufWriter { fn write(&mut self, buf: &[u8]) -> std::io::Result { - self.buf.lock().expect("lock buffer").extend_from_slice(buf); + self.buf + .lock() + .expect("Failed to lock log buffer") + .extend_from_slice(buf); Ok(buf.len()) } @@ -66,7 +69,8 @@ mod tests { std::env::set_var("RUST_LOG", "info"); init_with_writer(BufMakeWriter { buf: buf.clone() }); info!("captured"); - let output = String::from_utf8(buf.lock().expect("lock buffer").clone()).expect("utf8"); + let output = String::from_utf8(buf.lock().expect("Failed to lock log buffer").clone()) + .expect("Captured output is not valid UTF-8"); assert!(output.contains("captured")); } } From c18c2df9ebe03520b5c6e66cfdcc008e91f5b749 Mon Sep 17 00:00:00 2001 From: Leynos Date: Sun, 27 Jul 2025 13:50:54 +0100 Subject: [PATCH 6/6] =?UTF-8?q?Rephrase=20list=20item=20to=20remove=20the?= =?UTF-8?q?=20dangling=20=E2=80=9Cor=E2=80=9D.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 0b5f688..8506157 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -9,7 +9,7 @@ - **Clarity over cleverness.** Be concise, but favour explicit over terse or obscure idioms. Prefer code that's easy to follow. - **Use functions and composition.** Avoid repetition by extracting reusable - logic. Prefer generators or comprehensions, and declarative code to + logic. Prefer generators, comprehensions, and declarative code to imperative repetition when readable. - **Small, meaningful functions.** Functions must be small, clear in purpose, single responsibility, and obey command/query segregation.