diff --git a/crates/op-config-store/src/lib.rs b/crates/op-config-store/src/lib.rs index 8135e5f40..c7e251829 100644 --- a/crates/op-config-store/src/lib.rs +++ b/crates/op-config-store/src/lib.rs @@ -7,6 +7,11 @@ use serde::{de::DeserializeOwned, Serialize}; use std::ffi::OsString; use std::io::{Error, ErrorKind, Write}; use std::path::{Component, Path, PathBuf}; +use std::sync::OnceLock; + +/// Process-wide replacement for the `~/.openpencil` root. See +/// [`redirect_user_root_for_tests`]. +static USER_ROOT_OVERRIDE: OnceLock = OnceLock::new(); /// Name of the per-user config directory under the home directory /// (`~/.openpencil`). Exposed for callers that must resolve the @@ -105,7 +110,34 @@ pub fn write_json_path(path: &Path, value: &T) -> std::io::Result< Ok(()) } +/// Redirect [`ConfigStore::user`] — and everything built on it +/// (`read_json` / `write_json` / [`openpencil_dir`]) — at `root` for the +/// remainder of THIS PROCESS. +/// +/// Exists so a test binary can point the whole crate graph at a scratch +/// directory instead of the developer's real `~/.openpencil`. Without it +/// a test that exercises a persistence path rewrites live user config: +/// `cargo test -p op-host-desktop` was measured clobbering +/// `agents.json`, the file that decides which agent providers +/// auto-reconnect at launch, with whichever value the last parallel test +/// happened to write. +/// +/// Deliberately NOT an environment variable. The harness runs cases in +/// parallel threads of a single process, where `set_var` races every +/// concurrent reader — and is `unsafe` on current Rust besides. +/// +/// First call wins and later calls are no-ops, so every test can install +/// the same root unconditionally with no ordering rules between them. +/// Returns the root actually in force. +#[doc(hidden)] +pub fn redirect_user_root_for_tests(root: impl Into) -> &'static Path { + USER_ROOT_OVERRIDE.get_or_init(|| root.into()).as_path() +} + fn default_openpencil_dir() -> std::io::Result { + if let Some(root) = USER_ROOT_OVERRIDE.get() { + return Ok(root.clone()); + } home_dir().map(|home| home.join(OPENPENCIL_DIR_NAME)) } @@ -244,6 +276,29 @@ mod tests { assert!(store.write_json("/tmp/bad.json", &json!({})).is_err()); } + #[test] + fn user_root_redirect_covers_the_process_level_helpers_and_wins_once() { + let root = temp_root("redirect"); + assert_eq!(redirect_user_root_for_tests(&root), root); + assert_eq!(ConfigStore::user().unwrap().root(), root); + assert_eq!(openpencil_dir().unwrap(), root); + + // The free helpers are what callers actually reach for, and they + // must land in the redirected root, not the real home directory. + write_json("probe.json", &json!({ "ok": true })).unwrap(); + assert!(root.join("probe.json").is_file()); + let back: Option = read_json("probe.json").unwrap(); + assert_eq!(back, Some(json!({ "ok": true }))); + assert_ne!(root, home_dir().unwrap().join(".openpencil")); + + // Idempotent: a second install does not move the root, so tests + // may all call it in any order. + assert_eq!( + redirect_user_root_for_tests(temp_root("redirect-again")), + root + ); + } + #[test] fn well_known_files_preserve_existing_names() { assert_eq!(well_known::GIT_AUTH, "git-auth.json"); diff --git a/crates/op-host-desktop/src/agent_connect_store.rs b/crates/op-host-desktop/src/agent_connect_store.rs index adf0de8e4..68507d608 100644 --- a/crates/op-host-desktop/src/agent_connect_store.rs +++ b/crates/op-host-desktop/src/agent_connect_store.rs @@ -23,6 +23,7 @@ struct PersistedAgentConnections { /// Persist the currently-connected provider ids. Best-effort: a failed /// write must never break the probe flow. pub(crate) fn save(connected: &[bool; 6]) { + crate::test_config_root::guard_user_config(); let value = PersistedAgentConnections { connected: AgentProvider::ALL .iter() @@ -39,6 +40,7 @@ pub(crate) fn save(connected: &[bool; 6]) { /// Providers remembered as connected from the previous session, in /// `AgentProvider::ALL` order. pub(crate) fn load() -> Vec { + crate::test_config_root::guard_user_config(); let Ok(Some(value)) = op_config_store::read_json::(FILE) else { return Vec::new(); }; diff --git a/crates/op-host-desktop/src/main.rs b/crates/op-host-desktop/src/main.rs index 381a843a5..feaea51ec 100644 --- a/crates/op-host-desktop/src/main.rs +++ b/crates/op-host-desktop/src/main.rs @@ -80,6 +80,7 @@ mod single_instance; mod sub_agent_session; mod sub_agent_spawn_error; mod tcc_selftest; +mod test_config_root; mod theme_preset_host; mod ui_prefs; mod update_check; diff --git a/crates/op-host-desktop/src/test_config_root.rs b/crates/op-host-desktop/src/test_config_root.rs new file mode 100644 index 000000000..d230a5512 --- /dev/null +++ b/crates/op-host-desktop/src/test_config_root.rs @@ -0,0 +1,37 @@ +//! Keeps test runs off the user's real `~/.openpencil`. +//! +//! `agent_connect_store` and `ui_prefs` persist through `op_config_store`'s +//! process-level helpers, which resolve the real user directory. Exercising +//! either from a test therefore rewrites live config — measured: a +//! `cargo test -p op-host-desktop` run rewrote `~/.openpencil/agents.json`, +//! the file that decides which agent providers auto-reconnect at launch, +//! and since the harness runs cases in parallel whichever one finished last +//! decided its contents. +//! +//! The guard sits at the persistence entry points rather than in a test +//! helper on purpose: a redirect installed by the tests themselves only +//! protects whatever runs after it, and nothing orders the harness's +//! threads. Guarding where the user root is resolved makes the isolation +//! independent of which test happens to run first. + +/// Point the config store at a scratch directory the first time a desktop +/// persistence path runs inside a TEST binary. +/// +/// Idempotent and shared process-wide (`redirect_user_root_for_tests` is +/// first-caller-wins), so every entry point may call it unconditionally. +/// One shared directory is fine because no test asserts on a stored file's +/// contents — they assert on in-memory state, and the only goal here is to +/// keep the writes off the user's real config. +#[cfg(test)] +pub(crate) fn guard_user_config() { + let root = std::env::temp_dir().join(format!( + "op-host-desktop-test-config-{}", + std::process::id() + )); + op_config_store::redirect_user_root_for_tests(root); +} + +/// Real builds must resolve the real `~/.openpencil` — compiles away. +#[cfg(not(test))] +#[inline] +pub(crate) fn guard_user_config() {} diff --git a/crates/op-host-desktop/src/ui_prefs.rs b/crates/op-host-desktop/src/ui_prefs.rs index 687d39c03..918b6ee46 100644 --- a/crates/op-host-desktop/src/ui_prefs.rs +++ b/crates/op-host-desktop/src/ui_prefs.rs @@ -12,6 +12,7 @@ struct UiPrefs { } pub(crate) fn save_pencil_cursor(style: PencilCursorStyle) { + crate::test_config_root::guard_user_config(); let value = UiPrefs { pencil_cursor: Some(style.id().to_string()), }; @@ -21,6 +22,7 @@ pub(crate) fn save_pencil_cursor(style: PencilCursorStyle) { } pub(crate) fn load_pencil_cursor() -> Option { + crate::test_config_root::guard_user_config(); let value: UiPrefs = op_config_store::read_json(FILE).ok().flatten()?; PencilCursorStyle::from_id(value.pencil_cursor.as_deref()?) }