diff --git a/aw-sync/src/dirs.rs b/aw-sync/src/dirs.rs index d8291c17..03451ef0 100644 --- a/aw-sync/src/dirs.rs +++ b/aw-sync/src/dirs.rs @@ -1,7 +1,7 @@ use dirs::home_dir; +use serde::{Deserialize, Serialize}; use std::error::Error; use std::fs; -#[cfg(any(target_os = "android", test))] use std::path::Path; use std::path::PathBuf; @@ -35,15 +35,98 @@ pub fn resolve_profile( /// Uses the same profile appname as aw-server so a named profile (e.g. /// `research`) does not share prod's sync config. `testing` follows the /// same new-root-plus-legacy-fallback rule as aw-server. -// TODO: add proper config support +#[allow(dead_code)] // used by the aw-sync binary; the lib copy is unused (status.rs uses config_dir_path) #[cfg(not(target_os = "android"))] -#[allow(dead_code)] pub fn get_config_dir() -> Result> { - let dir = sync_config_dir(&aw_server::dirs::appname())?; + let dir = config_dir_path()?; fs::create_dir_all(&dir)?; Ok(dir) } +/// Path to aw-sync's own config dir — construction only, does not create it. +/// For read-only callers (e.g. `status`) that must never mutate the +/// filesystem just to look at it; `get_config_dir` is for the daemon path, +/// which is about to write `config.toml` there anyway. +#[cfg(not(target_os = "android"))] +pub fn config_dir_path() -> Result> { + sync_config_dir(&aw_server::dirs::appname()) +} + +/// `[daemon]` settings — namespaced (rather than top-level) so future +/// aw-sync settings that apply elsewhere (e.g. to the one-shot `sync` +/// command) have their own section instead of colliding with this one +/// (per-module convention: cf. aw-server's `[auth]` in `aw-server/src/config.rs`). +/// +/// `pull` controls whether the **daemon** imports peers on each pass — +/// desktop only; Android stays push-only by design +/// (ActivityWatch/aw-android#291). The one-shot `aw-sync sync` command +/// always pulls+pushes regardless of this file, and an explicit `--mode` +/// on the daemon always wins over it (ActivityWatch/aw-server-rust#714). +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub struct DaemonConfig { + #[serde(default)] + pub pull: bool, +} + +/// aw-sync's own settings, read from `{config_dir}/config.toml`. +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub struct SyncConfig { + #[serde(default)] + pub daemon: DaemonConfig, +} + +const DEFAULT_SYNC_CONFIG_TOML: &str = "\ +# aw-sync config +[daemon] +pull = false # default; set true to import peers from the sync folder every pass +"; + +/// Load `config.toml` from `dir`, writing the commented default if it does +/// not exist yet (so users find the switch on first daemon start). Returns +/// the parsed config and the path it was read from. +// dirs.rs is compiled by both lib.rs and main.rs (dual-include); the lib +// does not call this directly (status.rs uses read_sync_config), but the +// daemon binary does via its own mod dirs copy. +#[cfg_attr(not(test), allow(dead_code))] +#[cfg(not(target_os = "android"))] +pub fn load_or_create_sync_config(dir: &Path) -> Result<(SyncConfig, PathBuf), Box> { + fs::create_dir_all(dir)?; + let path = dir.join("config.toml"); + if !path.is_file() { + fs::write(&path, DEFAULT_SYNC_CONFIG_TOML)?; + } + let content = fs::read_to_string(&path)?; + let config: SyncConfig = toml::from_str(&content)?; + Ok((config, path)) +} + +/// Read-only variant for `status`: returns `(None, path)` when the file is +/// absent rather than writing the default. The daemon's `load_or_create` +/// writes on first start; the doctor should never create files. +#[cfg(not(target_os = "android"))] +pub fn read_sync_config(dir: &Path) -> Result<(Option, PathBuf), Box> { + let path = dir.join("config.toml"); + if !path.is_file() { + return Ok((None, path)); + } + let content = fs::read_to_string(&path)?; + let config: SyncConfig = toml::from_str(&content)?; + Ok((Some(config), path)) +} + +/// Which `SyncMode` a daemon pass should use: an explicit `--mode` always +/// wins; otherwise the config's `pull` flag picks push-only vs both. +pub fn effective_daemon_mode( + cli_mode: Option, + pull: bool, +) -> crate::report::SyncMode { + cli_mode.unwrap_or(if pull { + crate::report::SyncMode::Both + } else { + crate::report::SyncMode::Push + }) +} + /// Path construction only — does not create directories (so tests stay off-disk). #[cfg(not(target_os = "android"))] fn sync_config_dir(appname: &str) -> Result> { @@ -112,7 +195,6 @@ pub(crate) fn files_dir_from_xdg_data_home(xdg_data_home: &Path) -> Option PathBuf { + std::env::temp_dir().join(format!( + "aw-sync-config-tests-{label}-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )) + } + + #[cfg(not(target_os = "android"))] + #[test] + fn load_or_create_sync_config_writes_commented_default_when_missing() { + let dir = temp_sync_config_dir("missing"); + let (config, path) = load_or_create_sync_config(&dir).unwrap(); + assert!(!config.daemon.pull, "default config must be pull = false"); + assert!(path.is_file()); + let content = fs::read_to_string(&path).unwrap(); + assert!( + content.contains("[daemon]") && content.contains("pull = false"), + "commented default should be namespaced under [daemon] and mention pull = false, got: {content}" + ); + let _ = fs::remove_dir_all(&dir); + } + + #[cfg(not(target_os = "android"))] + #[test] + fn load_or_create_sync_config_respects_existing_pull_true() { + let dir = temp_sync_config_dir("pull-true"); + fs::create_dir_all(&dir).unwrap(); + fs::write(dir.join("config.toml"), "[daemon]\npull = true\n").unwrap(); + let (config, _path) = load_or_create_sync_config(&dir).unwrap(); + assert!(config.daemon.pull); + let _ = fs::remove_dir_all(&dir); + } + + #[cfg(not(target_os = "android"))] + #[test] + fn read_sync_config_returns_none_when_missing_and_does_not_create_file() { + let dir = temp_sync_config_dir("read-missing"); + let (config, path) = read_sync_config(&dir).unwrap(); + assert!(config.is_none(), "should return None when file is absent"); + assert!(!path.exists(), "read_sync_config must not create the file"); + let _ = fs::remove_dir_all(&dir); + } + + #[cfg(not(target_os = "android"))] + #[test] + fn read_sync_config_reads_existing_config() { + let dir = temp_sync_config_dir("read-existing"); + fs::create_dir_all(&dir).unwrap(); + fs::write(dir.join("config.toml"), "[daemon]\npull = true\n").unwrap(); + let (config, _path) = read_sync_config(&dir).unwrap(); + assert!(config.unwrap().daemon.pull, "should read pull = true"); + let _ = fs::remove_dir_all(&dir); + } } diff --git a/aw-sync/src/main.rs b/aw-sync/src/main.rs index dae3608c..d69cf192 100644 --- a/aw-sync/src/main.rs +++ b/aw-sync/src/main.rs @@ -88,9 +88,10 @@ enum Commands { buckets: Option>, /// Mode to sync in. Can be "push", "pull", or "both". - /// Defaults to "both". - #[clap(long, default_value = "both")] - mode: sync::SyncMode, + /// If not given, follows aw-sync's own config.toml: push-only unless + /// `pull = true` is set there (ActivityWatch/aw-server-rust#714). + #[clap(long)] + mode: Option, /// Full path to sync db file /// Useful for syncing buckets from a specific db file in the sync directory. @@ -219,7 +220,7 @@ fn main() -> Result<(), Box> { match opts.command.unwrap_or(Commands::Daemon { start_date: None, buckets: None, - mode: sync::SyncMode::Both, + mode: None, sync_db: None, }) { // Start daemon @@ -233,7 +234,36 @@ fn main() -> Result<(), Box> { let effective_buckets = buckets; - daemon(&client, start_date, effective_buckets, sync_db, mode)?; + // An explicit --mode always wins and must not depend on config.toml + // being readable/writable — only touch the config file when no CLI + // mode was given. + let effective_mode = if let Some(explicit_mode) = mode { + info!( + "aw-sync: explicit --mode {} overrides config", + explicit_mode.as_str() + ); + explicit_mode + } else { + let sync_config_dir = dirs::get_config_dir()?; + let (sync_config, sync_config_path) = + dirs::load_or_create_sync_config(&sync_config_dir)?; + let effective_mode = dirs::effective_daemon_mode(None, sync_config.daemon.pull); + info!( + "aw-sync config: {} (pull={}) -> daemon mode: {}", + sync_config_path.display(), + sync_config.daemon.pull, + effective_mode.as_str() + ); + effective_mode + }; + + daemon( + &client, + start_date, + effective_buckets, + sync_db, + effective_mode, + )?; } // Perform sync Commands::Sync { diff --git a/aw-sync/src/status.rs b/aw-sync/src/status.rs index e1d14c6e..25e4c3e2 100644 --- a/aw-sync/src/status.rs +++ b/aw-sync/src/status.rs @@ -90,6 +90,37 @@ pub fn collect_status( "UNREACHABLE" } )); + + // Config-derived only: an explicit `--mode` on a running daemon overrides + // this and status has no way to see that from here. "Last pass" below + // reports the mode actually used on the last completed sync. + let (pull, config_label) = match crate::dirs::config_dir_path() + .and_then(|dir| crate::dirs::read_sync_config(&dir)) + { + Ok((Some(cfg), path)) => { + let effective_mode = crate::dirs::effective_daemon_mode(None, cfg.daemon.pull); + out.push_str(&format!( + "daemon mode: {} (pull={}, config: {}) — config-derived; see 'Last pass' below for the mode actually used, which wins if --mode was passed explicitly\n", + effective_mode.as_str(), + cfg.daemon.pull, + path.display() + )); + (cfg.daemon.pull, path.display().to_string()) + } + Ok((None, path)) => { + out.push_str(&format!( + "daemon mode: push (pull=false, config: {} — not present, default) — config-derived; see 'Last pass' below for the mode actually used, which wins if --mode was passed explicitly\n", + path.display() + )); + (false, path.display().to_string()) + } + Err(e) => { + out.push_str(&format!( + "daemon mode: (could not read aw-sync config: {e})\n" + )); + (false, String::new()) + } + }; out.push('\n'); match crate::report::load_last_report() { @@ -123,7 +154,13 @@ pub fn collect_status( } } - let warnings = collect_warnings(&inspected, local_newest.as_ref(), &imported_origins); + let warnings = collect_warnings( + &inspected, + local_newest.as_ref(), + &imported_origins, + pull, + &config_label, + ); out.push('\n'); if warnings.is_empty() { out.push_str("Warnings: none\n"); @@ -164,9 +201,21 @@ fn collect_warnings( inspected: &[(SyncDirEntry, Option>)], local_newest: Option<&DateTime>, imported_origins: &HashSet, + pull: bool, + config_label: &str, ) -> Vec { let mut warnings = Vec::new(); + // When pull is off, say so once at the top (if there are visible peers) instead + // of emitting a per-peer "not imported locally" warning for every peer. + let has_peers = inspected.iter().any(|(e, _)| e.kind == SyncEntryKind::Peer); + if !pull && has_peers && !config_label.is_empty() { + warnings.push(format!( + "pull is off in {config_label}; peers below are visible but not imported by the \ + daemon (set pull = true, or run `aw-sync sync`)" + )); + } + let has_two = inspected .iter() .any(|(e, _)| e.layout == Some(SyncLayout::TwoLevel) && e.db_path.is_some()); @@ -216,7 +265,7 @@ fn collect_warnings( )); } } - if entry.kind == SyncEntryKind::Peer { + if pull && entry.kind == SyncEntryKind::Peer { if let Some(host) = &info.hostname { if !imported_origins.contains(host) { warnings.push(format!(