fix(workspace): resolve 51 linter diagnostics and fix two latent test bugs

All reported problems originated from clippy; flutter analyze was already
clean. The dominant cluster (44 errors) was in chronogrid_core::sync, whose
test module targeted a CRDT API that no longer existed - meaning that test
target had not compiled since Task 2.6.

Lint fixes:
- core/sync: realign tests to real TaskCrdt/NoteCrdt API (constructor arity,
  set_*(value, hlc) arg order, from_bytes vs load, Result<Option<String>>
  getters, and &mut receivers for Automerge save())
- core/crdt/doc: add impl Default for CrdtDoc; avoid approx_constant in test
- daemon: derive Default for Config; wire [logging].level into the tracing
  subscriber so the previously dead fields are functionally used
- crypto/keychain: use div_ceil
- net/swarm_key: drop needless borrow
- net/swarm_manager: collapse if-let into match arm
- ai: replace assert!(true) with a real VERSION assertion

Bug fixes exposed once the crate compiled:
- core/sync: SyncEngine::run moves a SwarmHandle (holding a broadcast::Sender
  clone) into the task, so RecvError::Closed is unreachable from an external
  event_tx drop. The old test asserted an impossible contract and hung; it is
  now torn down via JoinHandle::abort() with a new regression guard.
- ffi: serialize the 21 tests mutating the global ENGINE singleton behind a
  poison-tolerant engine_test_guard(), fixing an intermittent parallel-test
  race that was failing verify_agent_work.ps1.

Verification: clippy --workspace --all-targets --all-features = 0 warnings and
0 errors; cargo test --workspace --all-features = 221 passed / 0 failed;
flutter analyze clean; verify_agent_work.ps1 ALL CHECKS PASSED.
This commit is contained in:
Mike Kell 2026-08-23 17:29:46 -04:00
parent c1f7eabda7
commit 1e93ca4a42
13 changed files with 306 additions and 92 deletions

View File

@ -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<Self>` 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<Option<String>>` 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<Option<Engine>>` 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.

View File

@ -50,14 +50,19 @@ struct Cli {
db: Option<PathBuf>,
/// 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<Config> {
fn load_config(path: Option<&Path>) -> Result<LoadedConfig> {
let resolved: PathBuf = match path {
Some(p) => p.to_path_buf(),
None => {
@ -206,11 +230,11 @@ fn load_config(path: Option<&Path>) -> Result<Config> {
};
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<Config> {
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 {

View File

@ -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");
}
}

View File

@ -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]

View File

@ -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, &note, 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<NetEvent>`. 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;
}
}

View File

@ -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 };

View File

@ -57,6 +57,7 @@ pub fn db_delete_crdt_doc(entity_id: String) -> anyhow::Result<bool> {
#[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());
}

View File

@ -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());

View File

@ -145,6 +145,7 @@ pub fn note_crdt_merge(doc_a: Vec<u8>, doc_b: Vec<u8>) -> anyhow::Result<Vec<u8>
#[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());

View File

@ -56,3 +56,34 @@ pub(crate) fn with_engine_mut<T>(
.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())
}

View File

@ -294,6 +294,7 @@ pub fn task_crdt_merge(doc_a: Vec<u8>, doc_b: Vec<u8>) -> anyhow::Result<Vec<u8>
#[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());

View File

@ -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
}

View File

@ -456,11 +456,13 @@ impl SwarmManager {
self.handle_behaviour_event(behaviour_event).await;
}
SwarmEvent::OutgoingConnectionError { peer_id, error, .. } => {
if let Some(pid) = peer_id {
SwarmEvent::OutgoingConnectionError {
peer_id: Some(pid),
error,
..
} => {
warn!(%pid, %error, "SwarmManager: outgoing connection error");
}
}
SwarmEvent::IncomingConnectionError { error, .. } => {
warn!(%error, "SwarmManager: incoming connection error");