diff --git a/DECISION_LOG.md b/DECISION_LOG.md index 440aa59..2c2fae4 100644 --- a/DECISION_LOG.md +++ b/DECISION_LOG.md @@ -193,3 +193,18 @@ - `apps/chronogrid_daemon/config.example.toml` — fully documented reference configuration. - `docs/DEPLOYMENT/01-headless-daemon-setup.md` — comprehensive setup guide covering build, Linux systemd service, systemd unit hardening (NoNewPrivileges, ProtectSystem, etc.), cluster swarm-key distribution, CLI reference, key management, firewall rules, upgrade procedure, and troubleshooting table. - Full ADR at `docs/ARCHITECTURE/ADR/0012-daemon-scaffold.md`. + +## [2026-08-23] - ADL-008: Workspace-Wide Lint Cleanup & Test Determinism Fixes + +- **Status**: Accepted +- **Context**: The VS Code Problems tab reported 51 diagnostics across the workspace. Investigation showed `flutter analyze` was already clean, so all reported problems originated from `cargo clippy --workspace --all-targets --all-features`. The dominant cluster (44 errors) was in `crates/chronogrid_core/src/sync/mod.rs`, whose `#[cfg(test)]` module had been written against a `TaskCrdt`/`NoteCrdt` API that no longer matched the implementation — meaning the `chronogrid_core` test target had never compiled since Task 2.6. +- **Decision**: + - **`chronogrid_core::sync` tests realigned to the real CRDT API**: `TaskCrdt::new(id, namespace_id, title, hlc) -> Result` and `NoteCrdt::new(id, namespace_id, title, content_markdown, hlc)`; `set_*(value, hlc)` argument order (previously reversed); `from_bytes()` instead of the non-existent `load()`; getters return `Result>` rather than `Option<&str>`. Added `make_task`/`make_note` helpers and switched delta helpers to `&mut` (Automerge `save()` requires `&mut self`). + - **`CrdtDoc`** gained an `impl Default` (clippy `new_without_default`); the `put_get_f64_round_trip` test constant changed from `3.14` to `12.345_678_9` (clippy `approx_constant` flagged the former as an approximate `PI`). + - **`chronogrid_daemon`**: `Config` now derives `Default`. The previously dead `logging`/`level` fields were resolved *functionally* rather than by suppression — configuration is now loaded **before** tracing initialisation so `[logging].level` genuinely drives the subscriber. Precedence is `RUST_LOG` > `--log-level` > `[logging].level` > `"info"`. A `LoadedConfig` struct carries load provenance so the "config not found" warning can still be emitted after the subscriber exists. + - **Minor lints**: `div_ceil` in `chronogrid_crypto::keychain`; needless borrow in `chronogrid_net::swarm_key`; collapsible `if let` folded into the `match` arm in `chronogrid_net::swarm_manager`; `assert!(true)` in `chronogrid_ai` replaced with a real `VERSION` constant and assertion. + - **Bug 1 — unreachable test contract in `SyncEngine`** (exposed once the crate compiled): `SyncEngine` owns a `SwarmHandle` holding a `broadcast::Sender` clone, and `run()` moves `self` into the spawned task. A live sender therefore always exists *inside* the loop, so `RecvError::Closed` can never fire when an external `event_tx` is dropped. The former `run_event_loop_stops_on_channel_close` test asserted an impossible condition and hung. Replaced with `JoinHandle::abort()`-based teardown, an explanatory comment block, and a new `run_event_loop_survives_sender_drop` regression guard. + - **Bug 2 — parallel-test race on the FFI `ENGINE` singleton**: `chronogrid_ffi` tests mutate a process-wide `static Mutex>` while `cargo test` runs them across threads, producing intermittent `engine not initialized` / `engine is already initialized` failures. This was failing the gating `scripts/verify_agent_work.ps1` pipeline. Added a poison-tolerant `engine_test_guard()` in `api/state.rs` (`#[cfg(test)]` only) and routed all 21 engine-touching tests in `engine.rs`, `database.rs`, `task_crdt.rs`, and `note_crdt.rs` through it. This restores determinism without forcing `--test-threads=1` on the whole suite. + - **IDE/toolchain alignment**: added `.vscode/settings.json` setting `rust-analyzer.cargo.features = "all"` plus a clippy-based check command. `chronogrid_core::sync` is gated behind the optional `p2p` feature; rust-analyzer defaults to the crate's *default* feature set, so every `tracing::`, `tokio::`, `broadcast::`, and `chronogrid_net::` path in `sync/mod.rs` appeared unresolved in the Problems tab even when `cargo clippy --all-features` was clean. Note that `.vscode/` is currently listed in `.gitignore`, so this setting is machine-local; making `p2p` a default feature remains an option if the phantom diagnostics recur for other contributors. +- **Verification**: `cargo clippy --workspace --all-targets --all-features` → 0 warnings / 0 errors; `cargo test --workspace --all-features` → 221 passed / 0 failed; `flutter analyze` → clean; `scripts/verify_agent_work.ps1` → ALL VERIFICATION CHECKS PASSED. FFI determinism confirmed over 5 consecutive parallel runs (25/25 each). +- **No schema changes** — `docs/API_CONTRACTS/schema.sql` untouched; no ADR required as no architectural boundary changed. diff --git a/apps/chronogrid_daemon/src/main.rs b/apps/chronogrid_daemon/src/main.rs index c548fa6..85b88a6 100644 --- a/apps/chronogrid_daemon/src/main.rs +++ b/apps/chronogrid_daemon/src/main.rs @@ -50,14 +50,19 @@ struct Cli { db: Option, /// Logging verbosity level (error/warn/info/debug/trace or RUST_LOG directive). - #[arg(long, value_name = "LEVEL", default_value = "info")] + /// + /// Overrides `[logging].level` from the TOML config file when supplied. + #[arg(long, value_name = "LEVEL", default_value = DEFAULT_LOG_LEVEL)] log_level: String, } +/// Fallback logging verbosity used by both the CLI and the TOML config. +const DEFAULT_LOG_LEVEL: &str = "info"; + // ── TOML configuration structures ──────────────────────────────────────────── /// Root configuration loaded from the TOML config file. -#[derive(Debug, Deserialize)] +#[derive(Debug, Default, Deserialize)] struct Config { #[serde(default)] database: DatabaseConfig, @@ -69,17 +74,6 @@ struct Config { logging: LoggingConfig, } -impl Default for Config { - fn default() -> Self { - Self { - database: DatabaseConfig::default(), - network: NetworkConfig::default(), - crypto: CryptoConfig::default(), - logging: LoggingConfig::default(), - } - } -} - /// SQLCipher database configuration. #[derive(Debug, Deserialize)] struct DatabaseConfig { @@ -183,15 +177,45 @@ impl Default for LoggingConfig { } } -fn default_log_level() -> String { "info".to_string() } +fn default_log_level() -> String { DEFAULT_LOG_LEVEL.to_string() } // ── Config loading ──────────────────────────────────────────────────────────── +/// Result of a configuration load, retaining provenance information. +/// +/// The daemon must resolve the logging verbosity *before* the tracing +/// subscriber is installed, which means the config file has to be read before +/// any log macro can emit output. This struct carries the information needed +/// to log the outcome once the subscriber is ready. +#[derive(Debug)] +struct LoadedConfig { + /// The parsed (or default) configuration. + config: Config, + /// Path that was probed for the TOML file. + path: PathBuf, + /// `true` when the file existed and was parsed, `false` when defaults were used. + from_file: bool, +} + +/// Resolve the effective logging directive. +/// +/// Precedence (highest first): +/// 1. `RUST_LOG` environment variable (handled by the caller). +/// 2. `--log-level` CLI flag, when explicitly set to a non-default value. +/// 3. `[logging].level` from the TOML config file. +fn resolve_log_level(cli_level: &str, config: &Config) -> String { + if cli_level != DEFAULT_LOG_LEVEL { + cli_level.to_string() + } else { + config.logging.level.clone() + } +} + /// Load and parse the TOML configuration file. /// /// If the file does not exist, built-in defaults are returned so the daemon /// can start without a config file (useful for initial boot / CI). -fn load_config(path: Option<&Path>) -> Result { +fn load_config(path: Option<&Path>) -> Result { let resolved: PathBuf = match path { Some(p) => p.to_path_buf(), None => { @@ -206,11 +230,11 @@ fn load_config(path: Option<&Path>) -> Result { }; if !resolved.exists() { - warn!( - path = %resolved.display(), - "Config file not found — using built-in defaults" - ); - return Ok(Config::default()); + return Ok(LoadedConfig { + config: Config::default(), + path: resolved, + from_file: false, + }); } let raw = std::fs::read_to_string(&resolved) @@ -219,8 +243,11 @@ fn load_config(path: Option<&Path>) -> Result { let config: Config = toml::from_str(&raw) .with_context(|| format!("Failed to parse config file: {}", resolved.display()))?; - info!(path = %resolved.display(), "Loaded configuration"); - Ok(config) + Ok(LoadedConfig { + config, + path: resolved, + from_file: true, + }) } // ── Master key resolution ───────────────────────────────────────────────────── @@ -293,9 +320,23 @@ fn generate_and_store_key(passphrase: &str, service: &str, account: &str) -> Res async fn main() -> Result<()> { let cli = Cli::parse(); - // ── 1. Initialise structured logging ────────────────────────────────────── - let filter = EnvFilter::try_from_default_env() - .unwrap_or_else(|_| EnvFilter::new(&cli.log_level)); + // ── 1. Load TOML configuration ──────────────────────────────────────────── + // + // The config is read before tracing is initialised so that the + // `[logging].level` directive can shape the subscriber. Any diagnostics + // about the load are emitted immediately after the subscriber is ready. + let LoadedConfig { + mut config, + path: config_path, + from_file, + } = load_config(cli.config.as_deref())?; + + // ── 2. Initialise structured logging ────────────────────────────────────── + // + // Precedence: RUST_LOG > --log-level > [logging].level > "info". + let log_level = resolve_log_level(&cli.log_level, &config); + let filter = + EnvFilter::try_from_default_env().unwrap_or_else(|_| EnvFilter::new(&log_level)); fmt() .with_env_filter(filter) @@ -304,10 +345,16 @@ async fn main() -> Result<()> { .compact() .init(); - info!(version = env!("CARGO_PKG_VERSION"), "ChronoGrid daemon starting"); + info!(version = env!("CARGO_PKG_VERSION"), %log_level, "ChronoGrid daemon starting"); - // ── 2. Load TOML configuration ──────────────────────────────────────────── - let mut config = load_config(cli.config.as_deref())?; + if from_file { + info!(path = %config_path.display(), "Loaded configuration"); + } else { + warn!( + path = %config_path.display(), + "Config file not found — using built-in defaults" + ); + } // CLI --db overrides the config-file database path. if let Some(db_path) = cli.db { diff --git a/crates/chronogrid_ai/src/lib.rs b/crates/chronogrid_ai/src/lib.rs index 8073ed3..c0dbfc4 100644 --- a/crates/chronogrid_ai/src/lib.rs +++ b/crates/chronogrid_ai/src/lib.rs @@ -3,10 +3,15 @@ //! Local AI inference and task prioritization engine for ChronoGrid P2P. //! Runs entirely on-device – no cloud API calls. +/// Crate version, surfaced for diagnostics and FFI capability reporting. +pub const VERSION: &str = env!("CARGO_PKG_VERSION"); + #[cfg(test)] mod tests { + use super::*; + #[test] - fn it_works() { - assert!(true); + fn version_is_populated() { + assert!(!VERSION.is_empty(), "crate version must not be empty"); } } diff --git a/crates/chronogrid_core/src/crdt/doc.rs b/crates/chronogrid_core/src/crdt/doc.rs index ad092e4..9c8194c 100644 --- a/crates/chronogrid_core/src/crdt/doc.rs +++ b/crates/chronogrid_core/src/crdt/doc.rs @@ -27,6 +27,12 @@ impl std::fmt::Debug for CrdtDoc { } } +impl Default for CrdtDoc { + fn default() -> Self { + Self::new() + } +} + impl CrdtDoc { // ── Construction ───────────────────────────────────────────────────── @@ -223,11 +229,15 @@ mod tests { #[test] fn put_get_f64_round_trip() { + // A value that is not exactly representable in binary floating point, + // so the assertion genuinely exercises the round-trip fidelity. + const SCORE: f64 = 12.345_678_9; + let mut doc = CrdtDoc::new(); - doc.put_f64("score", 3.14).unwrap(); + doc.put_f64("score", SCORE).unwrap(); let val = doc.get_f64("score").unwrap().unwrap(); - assert!((val - 3.14).abs() < f64::EPSILON); + assert!((val - SCORE).abs() < f64::EPSILON); } #[test] diff --git a/crates/chronogrid_core/src/sync/mod.rs b/crates/chronogrid_core/src/sync/mod.rs index 712ddcd..53b7e47 100644 --- a/crates/chronogrid_core/src/sync/mod.rs +++ b/crates/chronogrid_core/src/sync/mod.rs @@ -57,7 +57,6 @@ use chronogrid_net::{ use tokio::sync::broadcast; use crate::{ - crdt::{Hlc, NoteCrdt, TaskCrdt}, db::Database, error::{CoreError, Result}, }; @@ -403,7 +402,7 @@ fn unix_timestamp_secs() -> u64 { #[cfg(test)] mod tests { use super::*; - use crate::crdt::{Hlc, TaskCrdt}; + use crate::crdt::{Hlc, NoteCrdt, TaskCrdt}; use crate::db::Database; use chronogrid_net::swarm_manager::SwarmHandle; use chronogrid_net::PeerId; @@ -439,14 +438,29 @@ mod tests { (engine, event_tx) } + /// Namespace UUID used by every CRDT document created in these tests. + const TEST_NAMESPACE: &str = "ns-test-0000"; + + /// Build a `TaskCrdt` with the given entity ID and title. + fn make_task(entity_id: &str, title: &str, hlc: &mut Hlc) -> TaskCrdt { + TaskCrdt::new(entity_id, TEST_NAMESPACE, title, hlc).expect("create TaskCrdt") + } + + /// Build a `NoteCrdt` with the given entity ID and title. + fn make_note(entity_id: &str, title: &str, hlc: &mut Hlc) -> NoteCrdt { + NoteCrdt::new(entity_id, TEST_NAMESPACE, title, "", hlc).expect("create NoteCrdt") + } + // ── merge_automerge_changeset ───────────────────────────────────────────── #[test] fn merge_into_empty_local_doc() { // Create a remote document with a task. let mut hlc = Hlc::new("node-remote"); - let mut remote_task = TaskCrdt::new(&mut hlc); - remote_task.set_title(&mut hlc, "Remote Task Title"); + let mut remote_task = make_task("task-remote-001", "Initial", &mut hlc); + remote_task + .set_title("Remote Task Title", &mut hlc) + .expect("set_title"); let remote_bytes = remote_task.save(); // Merge into a non-existent local doc (None). @@ -454,21 +468,26 @@ mod tests { .expect("merge into empty must succeed"); // The merged doc must be loadable and contain the remote data. - let loaded = TaskCrdt::load(&merged).expect("load merged doc"); - assert_eq!(loaded.title(), Some("Remote Task Title")); + let loaded = TaskCrdt::from_bytes(&merged).expect("load merged doc"); + assert_eq!( + loaded.title().expect("title"), + Some("Remote Task Title".to_string()) + ); } #[test] fn merge_two_concurrent_edits_converges() { // Simulate two peers editing different fields of the same task. let mut hlc_a = Hlc::new("node-a"); - let mut task_a = TaskCrdt::new(&mut hlc_a); - task_a.set_title(&mut hlc_a, "Title from A"); + let mut task_a = make_task("task-conc-001", "Initial", &mut hlc_a); + task_a.set_title("Title from A", &mut hlc_a).expect("set_title A"); let mut hlc_b = Hlc::new("node-b"); - let mut task_b = TaskCrdt::new(&mut hlc_b); - task_b.set_title(&mut hlc_b, "Title from B"); - task_b.set_status(&mut hlc_b, "in_progress"); + let mut task_b = make_task("task-conc-001", "Initial", &mut hlc_b); + task_b.set_title("Title from B", &mut hlc_b).expect("set_title B"); + task_b + .set_status("in_progress", &mut hlc_b) + .expect("set_status B"); let bytes_a = task_a.save(); let bytes_b = task_b.save(); @@ -482,19 +501,25 @@ mod tests { merge_automerge_changeset(Some(&bytes_b), &bytes_a).expect("merge B←A must succeed"); // Both merged documents must be loadable. - let doc_a_b = TaskCrdt::load(&merged_a_b).expect("load A←B"); - let doc_b_a = TaskCrdt::load(&merged_b_a).expect("load B←A"); + let doc_a_b = TaskCrdt::from_bytes(&merged_a_b).expect("load A←B"); + let doc_b_a = TaskCrdt::from_bytes(&merged_b_a).expect("load B←A"); // Status must be "in_progress" in both (only B set it). - assert_eq!(doc_a_b.status(), Some("in_progress")); - assert_eq!(doc_b_a.status(), Some("in_progress")); + assert_eq!( + doc_a_b.status().expect("status A←B"), + Some("in_progress".to_string()) + ); + assert_eq!( + doc_b_a.status().expect("status B←A"), + Some("in_progress".to_string()) + ); } #[test] fn merge_idempotent_same_changeset() { let mut hlc = Hlc::new("node-x"); - let mut task = TaskCrdt::new(&mut hlc); - task.set_title(&mut hlc, "Idempotent Task"); + let mut task = make_task("task-idem-001", "Initial", &mut hlc); + task.set_title("Idempotent Task", &mut hlc).expect("set_title"); let bytes = task.save(); // Merge the same bytes twice. @@ -502,11 +527,11 @@ mod tests { let merged2 = merge_automerge_changeset(Some(&merged1), &bytes).expect("second merge"); - let doc1 = TaskCrdt::load(&merged1).expect("load merged1"); - let doc2 = TaskCrdt::load(&merged2).expect("load merged2"); + let doc1 = TaskCrdt::from_bytes(&merged1).expect("load merged1"); + let doc2 = TaskCrdt::from_bytes(&merged2).expect("load merged2"); // Title must be the same after idempotent merge. - assert_eq!(doc1.title(), doc2.title()); + assert_eq!(doc1.title().expect("title 1"), doc2.title().expect("title 2")); } #[test] @@ -522,12 +547,18 @@ mod tests { // ── SyncEngine::merge_remote_deltas ────────────────────────────────────── /// Build a minimal `DeltaPacket` from a `TaskCrdt` for testing. - fn make_task_delta(entity_id: &str, hlc: &str, task: &TaskCrdt, seq: u64) -> DeltaPacket { + /// + /// Takes `&mut TaskCrdt` because Automerge serialization + /// ([`TaskCrdt::save`]) compacts the document in place. + fn make_task_delta(entity_id: &str, hlc: &str, task: &mut TaskCrdt, seq: u64) -> DeltaPacket { DeltaPacket::new("task", entity_id, hlc, task.save(), seq) } /// Build a minimal `DeltaPacket` from a `NoteCrdt` for testing. - fn make_note_delta(entity_id: &str, hlc: &str, note: &NoteCrdt, seq: u64) -> DeltaPacket { + /// + /// Takes `&mut NoteCrdt` because Automerge serialization + /// ([`NoteCrdt::save`]) compacts the document in place. + fn make_note_delta(entity_id: &str, hlc: &str, note: &mut NoteCrdt, seq: u64) -> DeltaPacket { DeltaPacket::new("note", entity_id, hlc, note.save(), seq) } @@ -546,10 +577,10 @@ mod tests { // Create a remote task delta. let mut hlc = Hlc::new("remote-node"); - let mut task = TaskCrdt::new(&mut hlc); - task.set_title(&mut hlc, "Synced Task"); + let mut task = make_task("task-uuid-001", "Initial", &mut hlc); + task.set_title("Synced Task", &mut hlc).expect("set_title"); let hlc_str = hlc.tick().to_string(); - let delta = make_task_delta("task-uuid-001", &hlc_str, &task, 0); + let delta = make_task_delta("task-uuid-001", &hlc_str, &mut task, 0); let merged = engine.merge_remote_deltas(vec![delta]).expect("merge"); assert_eq!(merged, vec!["task-uuid-001"]); @@ -562,8 +593,11 @@ mod tests { .expect("load") .expect("must exist"); - let loaded_task = TaskCrdt::load(&stored).expect("load task"); - assert_eq!(loaded_task.title(), Some("Synced Task")); + let loaded_task = TaskCrdt::from_bytes(&stored).expect("load task"); + assert_eq!( + loaded_task.title().expect("title"), + Some("Synced Task".to_string()) + ); } #[test] @@ -573,10 +607,10 @@ mod tests { let mut engine = SyncEngine::new(Arc::clone(&db), handle); let mut hlc = Hlc::new("remote-node"); - let mut note = NoteCrdt::new(&mut hlc); - note.set_title(&mut hlc, "Synced Note"); + let mut note = make_note("note-uuid-001", "Initial", &mut hlc); + note.set_title("Synced Note", &mut hlc).expect("set_title"); let hlc_str = hlc.tick().to_string(); - let delta = make_note_delta("note-uuid-001", &hlc_str, ¬e, 0); + let delta = make_note_delta("note-uuid-001", &hlc_str, &mut note, 0); let merged = engine.merge_remote_deltas(vec![delta]).expect("merge"); assert_eq!(merged, vec!["note-uuid-001"]); @@ -588,8 +622,11 @@ mod tests { .expect("load") .expect("must exist"); - let loaded_note = NoteCrdt::load(&stored).expect("load note"); - assert_eq!(loaded_note.title(), Some("Synced Note")); + let loaded_note = NoteCrdt::from_bytes(&stored).expect("load note"); + assert_eq!( + loaded_note.title().expect("title"), + Some("Synced Note".to_string()) + ); } #[test] @@ -599,10 +636,10 @@ mod tests { let mut engine = SyncEngine::new(db, handle); let mut hlc = Hlc::new("remote-node"); - let mut task = TaskCrdt::new(&mut hlc); - task.set_title(&mut hlc, "Clock Test Task"); + let mut task = make_task("task-clock-001", "Initial", &mut hlc); + task.set_title("Clock Test Task", &mut hlc).expect("set_title"); let hlc_str = "1724000000000-0001-ab12".to_string(); - let delta = make_task_delta("task-clock-001", &hlc_str, &task, 0); + let delta = make_task_delta("task-clock-001", &hlc_str, &mut task, 0); engine.merge_remote_deltas(vec![delta]).expect("merge"); @@ -668,17 +705,17 @@ mod tests { let mut hlc = Hlc::new("remote-node"); - let mut task1 = TaskCrdt::new(&mut hlc); - task1.set_title(&mut hlc, "Task One"); + let mut task1 = make_task("task-multi-001", "Initial", &mut hlc); + task1.set_title("Task One", &mut hlc).expect("set_title 1"); let hlc1 = hlc.tick().to_string(); - let mut task2 = TaskCrdt::new(&mut hlc); - task2.set_title(&mut hlc, "Task Two"); + let mut task2 = make_task("task-multi-002", "Initial", &mut hlc); + task2.set_title("Task Two", &mut hlc).expect("set_title 2"); let hlc2 = hlc.tick().to_string(); let deltas = vec![ - make_task_delta("task-multi-001", &hlc1, &task1, 0), - make_task_delta("task-multi-002", &hlc2, &task2, 1), + make_task_delta("task-multi-001", &hlc1, &mut task1, 0), + make_task_delta("task-multi-002", &hlc2, &mut task2, 1), ]; let merged = engine.merge_remote_deltas(deltas).expect("merge"); @@ -702,8 +739,8 @@ mod tests { // Pre-populate a CRDT doc in the DB. let mut hlc = Hlc::new("local-node"); - let mut task = TaskCrdt::new(&mut hlc); - task.set_title(&mut hlc, "Outbound Task"); + let mut task = make_task("task-out-001", "Initial", &mut hlc); + task.set_title("Outbound Task", &mut hlc).expect("set_title"); let doc_bytes = task.save(); let hlc_str = hlc.tick().to_string(); @@ -768,6 +805,17 @@ mod tests { } // ── SyncEngine::run event loop ──────────────────────────────────────────── + // + // NOTE ON EVENT-LOOP LIFETIME + // + // `SyncEngine` owns a `SwarmHandle`, and `SwarmHandle` holds a clone of the + // `broadcast::Sender`. Because `SyncEngine::run` moves `self` + // into the spawned task, a live sender clone is kept *inside* the loop for + // as long as the loop runs. Consequently the broadcast channel can never + // reach zero senders while the loop is alive, and + // `RecvError::Closed` is unreachable from a test that merely drops its own + // `event_tx`. The loop is therefore terminated with `JoinHandle::abort()`, + // which is also how a supervisor would stop it in practice. #[tokio::test] async fn run_event_loop_processes_deltas_received() { @@ -777,8 +825,8 @@ mod tests { // Prepare a real task delta. let mut hlc = Hlc::new("remote-node"); - let mut task = TaskCrdt::new(&mut hlc); - task.set_title(&mut hlc, "Event Loop Task"); + let mut task = make_task("task-loop-001", "Initial", &mut hlc); + task.set_title("Event Loop Task", &mut hlc).expect("set_title"); let hlc_str = hlc.tick().to_string(); let delta = DeltaPacket::new("task", "task-loop-001", &hlc_str, task.save(), 0); @@ -796,8 +844,9 @@ mod tests { // Give the event loop time to process. tokio::time::sleep(std::time::Duration::from_millis(50)).await; - // Close the channel to stop the event loop. + // Stop the event loop (see the lifetime note above). drop(event_tx); + join.abort(); let _ = join.await; // Verify the delta was merged into the DB. @@ -808,25 +857,55 @@ mod tests { .expect("load") .expect("must exist after event loop merge"); - let loaded = TaskCrdt::load(&stored).expect("load task"); - assert_eq!(loaded.title(), Some("Event Loop Task")); + let loaded = TaskCrdt::from_bytes(&stored).expect("load task"); + assert_eq!( + loaded.title().expect("title"), + Some("Event Loop Task".to_string()) + ); } #[tokio::test] - async fn run_event_loop_stops_on_channel_close() { + async fn run_event_loop_stops_on_abort() { let (handle, event_tx) = make_test_handle(); let db = make_db(); let engine = SyncEngine::new(db, handle); let join = engine.run(); - // Close the channel immediately. + // Drop the test-side sender; the engine's own handle keeps the channel + // open, so the loop stays alive. drop(event_tx); - // The event loop must terminate cleanly. - tokio::time::timeout(std::time::Duration::from_secs(1), join) + // Aborting must tear the loop down promptly. + join.abort(); + + let outcome = tokio::time::timeout(std::time::Duration::from_secs(1), join) .await - .expect("event loop must stop within 1 second") - .expect("join handle must not panic"); + .expect("event loop must stop within 1 second"); + + match outcome { + Ok(()) => {} + Err(e) => assert!(e.is_cancelled(), "loop must stop via cancellation, got: {e}"), + } + } + + #[tokio::test] + async fn run_event_loop_survives_sender_drop() { + // Regression guard for the lifetime note above: the loop must NOT exit + // just because an external `event_tx` clone was dropped, since the + // engine's own `SwarmHandle` still holds a sender. + let (handle, event_tx) = make_test_handle(); + let db = make_db(); + let engine = SyncEngine::new(db, handle); + + let join = engine.run(); + drop(event_tx); + + // The loop must still be running after a short grace period. + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + assert!(!join.is_finished(), "event loop must outlive an external sender drop"); + + join.abort(); + let _ = join.await; } } diff --git a/crates/chronogrid_crypto/src/keychain.rs b/crates/chronogrid_crypto/src/keychain.rs index 6826ced..827e1fc 100644 --- a/crates/chronogrid_crypto/src/keychain.rs +++ b/crates/chronogrid_crypto/src/keychain.rs @@ -117,7 +117,7 @@ mod platform { b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; pub fn encode(input: &[u8]) -> String { - let mut out = String::with_capacity((input.len() + 2) / 3 * 4); + let mut out = String::with_capacity(input.len().div_ceil(3) * 4); for chunk in input.chunks(3) { let b0 = chunk[0] as u32; let b1 = if chunk.len() > 1 { chunk[1] as u32 } else { 0 }; diff --git a/crates/chronogrid_ffi/src/api/database.rs b/crates/chronogrid_ffi/src/api/database.rs index f4c4201..d7cd3ae 100644 --- a/crates/chronogrid_ffi/src/api/database.rs +++ b/crates/chronogrid_ffi/src/api/database.rs @@ -57,6 +57,7 @@ pub fn db_delete_crdt_doc(entity_id: String) -> anyhow::Result { #[cfg(test)] mod tests { use super::super::engine::{close_engine, init_engine_in_memory}; + use super::super::state::engine_test_guard; use super::*; const TEST_KEY: [u8; 32] = [0xAA; 32]; @@ -73,6 +74,7 @@ mod tests { #[test] fn save_load_round_trip() { + let _guard = engine_test_guard(); setup(); let blob = b"fake-automerge-bytes".to_vec(); @@ -86,6 +88,7 @@ mod tests { #[test] fn load_missing_returns_none() { + let _guard = engine_test_guard(); setup(); let loaded = db_load_crdt_doc("nonexistent".into()).unwrap(); assert_eq!(loaded, None); @@ -94,6 +97,7 @@ mod tests { #[test] fn delete_existing_returns_true() { + let _guard = engine_test_guard(); setup(); db_save_crdt_doc("t-002".into(), "task".into(), b"data".to_vec(), 1000).unwrap(); assert!(db_delete_crdt_doc("t-002".into()).unwrap()); @@ -103,6 +107,7 @@ mod tests { #[test] fn delete_missing_returns_false() { + let _guard = engine_test_guard(); setup(); assert!(!db_delete_crdt_doc("nonexistent".into()).unwrap()); teardown(); @@ -110,6 +115,7 @@ mod tests { #[test] fn operations_without_engine_fail() { + let _guard = engine_test_guard(); let _ = close_engine(); assert!(db_load_crdt_doc("x".into()).is_err()); } diff --git a/crates/chronogrid_ffi/src/api/engine.rs b/crates/chronogrid_ffi/src/api/engine.rs index e06226d..5957d83 100644 --- a/crates/chronogrid_ffi/src/api/engine.rs +++ b/crates/chronogrid_ffi/src/api/engine.rs @@ -88,6 +88,7 @@ pub fn is_engine_initialized() -> bool { #[cfg(test)] mod tests { + use super::super::state::engine_test_guard; use super::*; /// 32-byte test key. @@ -102,6 +103,7 @@ mod tests { #[test] fn init_and_close_lifecycle() { + let _guard = engine_test_guard(); reset_engine(); assert!(!is_engine_initialized()); @@ -115,6 +117,7 @@ mod tests { #[test] fn double_init_is_rejected() { + let _guard = engine_test_guard(); reset_engine(); init_engine_in_memory(TEST_KEY.to_vec(), NODE.into()).unwrap(); @@ -126,6 +129,7 @@ mod tests { #[test] fn close_without_init_is_noop() { + let _guard = engine_test_guard(); reset_engine(); // Should not panic or error. close_engine().unwrap(); @@ -133,6 +137,7 @@ mod tests { #[test] fn bad_key_length_is_rejected() { + let _guard = engine_test_guard(); reset_engine(); let bad_key = vec![0u8; 16]; let result = init_engine_in_memory(bad_key, NODE.into()); diff --git a/crates/chronogrid_ffi/src/api/note_crdt.rs b/crates/chronogrid_ffi/src/api/note_crdt.rs index 4d46ff3..80a5e17 100644 --- a/crates/chronogrid_ffi/src/api/note_crdt.rs +++ b/crates/chronogrid_ffi/src/api/note_crdt.rs @@ -145,6 +145,7 @@ pub fn note_crdt_merge(doc_a: Vec, doc_b: Vec) -> anyhow::Result #[cfg(test)] mod tests { use super::super::engine::{close_engine, init_engine_in_memory}; + use super::super::state::engine_test_guard; use super::*; const TEST_KEY: [u8; 32] = [0xAA; 32]; @@ -161,6 +162,7 @@ mod tests { #[test] fn create_and_read_round_trip() { + let _guard = engine_test_guard(); setup(); let bytes = note_crdt_create( @@ -184,6 +186,7 @@ mod tests { #[test] fn set_all_fields_and_read() { + let _guard = engine_test_guard(); setup(); let mut bytes = note_crdt_create( @@ -211,6 +214,7 @@ mod tests { #[test] fn clear_optional_fields() { + let _guard = engine_test_guard(); setup(); let mut bytes = note_crdt_create( @@ -232,6 +236,7 @@ mod tests { #[test] fn merge_concurrent_edits() { + let _guard = engine_test_guard(); setup(); let bytes_a = note_crdt_create( @@ -262,6 +267,7 @@ mod tests { #[test] fn read_is_stateless() { + let _guard = engine_test_guard(); let _ = close_engine(); init_engine_in_memory(TEST_KEY.to_vec(), NODE.into()).unwrap(); @@ -280,6 +286,7 @@ mod tests { #[test] fn mutation_without_engine_fails() { + let _guard = engine_test_guard(); let _ = close_engine(); let result = note_crdt_create("x".into(), "y".into(), "z".into(), "w".into()); assert!(result.is_err()); diff --git a/crates/chronogrid_ffi/src/api/state.rs b/crates/chronogrid_ffi/src/api/state.rs index c5817a3..a9b3d2e 100644 --- a/crates/chronogrid_ffi/src/api/state.rs +++ b/crates/chronogrid_ffi/src/api/state.rs @@ -56,3 +56,34 @@ pub(crate) fn with_engine_mut( .ok_or_else(|| anyhow::anyhow!("engine not initialized — call init_engine first"))?; f(engine) } + +// ── Test-only serialization guard ──────────────────────────────────────────── + +/// Serializes tests that mutate the global [`ENGINE`] singleton. +/// +/// # Why this is required +/// +/// [`ENGINE`] is process-wide state, but `cargo test` runs the tests of a +/// single binary **in parallel across threads by default**. Every test that +/// calls `init_engine*` / `close_engine` therefore races with every other such +/// test: one test tears the engine down while another is mid-assertion, +/// producing spurious `engine not initialized` and `engine is already +/// initialized` failures. +/// +/// Acquiring [`engine_test_guard`] at the top of each affected test restores a +/// total order over those tests without forcing the whole suite to run with +/// `--test-threads=1`. +#[cfg(test)] +pub(crate) static ENGINE_TEST_LOCK: Mutex<()> = Mutex::new(()); + +/// Acquire the exclusive engine-test lock. +/// +/// Returns the guard, which must be held for the duration of the test. The +/// lock is deliberately poison-tolerant: a panicking test would otherwise +/// poison the mutex and cascade failures into every subsequent test. +#[cfg(test)] +pub(crate) fn engine_test_guard() -> std::sync::MutexGuard<'static, ()> { + ENGINE_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) +} diff --git a/crates/chronogrid_ffi/src/api/task_crdt.rs b/crates/chronogrid_ffi/src/api/task_crdt.rs index 8c687f1..599c08a 100644 --- a/crates/chronogrid_ffi/src/api/task_crdt.rs +++ b/crates/chronogrid_ffi/src/api/task_crdt.rs @@ -294,6 +294,7 @@ pub fn task_crdt_merge(doc_a: Vec, doc_b: Vec) -> anyhow::Result #[cfg(test)] mod tests { use super::super::engine::{close_engine, init_engine_in_memory}; + use super::super::state::engine_test_guard; use super::*; const TEST_KEY: [u8; 32] = [0xAA; 32]; @@ -310,6 +311,7 @@ mod tests { #[test] fn create_and_read_round_trip() { + let _guard = engine_test_guard(); setup(); let bytes = task_crdt_create("t-001".into(), "ns-1".into(), "My Task".into()) @@ -329,6 +331,7 @@ mod tests { #[test] fn set_all_fields_and_read() { + let _guard = engine_test_guard(); setup(); let mut bytes = @@ -365,6 +368,7 @@ mod tests { #[test] fn clear_optional_fields() { + let _guard = engine_test_guard(); setup(); let mut bytes = @@ -397,6 +401,7 @@ mod tests { #[test] fn merge_concurrent_edits() { + let _guard = engine_test_guard(); setup(); let bytes_a = @@ -422,6 +427,7 @@ mod tests { #[test] fn read_is_stateless() { + let _guard = engine_test_guard(); // Ensure read works without engine. let _ = close_engine(); @@ -438,6 +444,7 @@ mod tests { #[test] fn mutation_without_engine_fails() { + let _guard = engine_test_guard(); let _ = close_engine(); let result = task_crdt_create("x".into(), "y".into(), "z".into()); assert!(result.is_err()); diff --git a/crates/chronogrid_net/src/swarm_key.rs b/crates/chronogrid_net/src/swarm_key.rs index 7cfd583..2c8492b 100644 --- a/crates/chronogrid_net/src/swarm_key.rs +++ b/crates/chronogrid_net/src/swarm_key.rs @@ -97,7 +97,7 @@ impl SwarmKey { /// The fingerprint is the first 16 hex characters of the SHA-256 hash of /// the key bytes (8 bytes → 16 hex chars), matching the schema definition. pub fn fingerprint(&self) -> String { - let hash = Sha256::digest(&self.bytes); + let hash = Sha256::digest(self.bytes); hex::encode(&hash[..8]) // first 8 bytes → 16 hex chars } diff --git a/crates/chronogrid_net/src/swarm_manager.rs b/crates/chronogrid_net/src/swarm_manager.rs index a05c1d2..29877a3 100644 --- a/crates/chronogrid_net/src/swarm_manager.rs +++ b/crates/chronogrid_net/src/swarm_manager.rs @@ -456,10 +456,12 @@ impl SwarmManager { self.handle_behaviour_event(behaviour_event).await; } - SwarmEvent::OutgoingConnectionError { peer_id, error, .. } => { - if let Some(pid) = peer_id { - warn!(%pid, %error, "SwarmManager: outgoing connection error"); - } + SwarmEvent::OutgoingConnectionError { + peer_id: Some(pid), + error, + .. + } => { + warn!(%pid, %error, "SwarmManager: outgoing connection error"); } SwarmEvent::IncomingConnectionError { error, .. } => {