From 06c70bb0bd418c9f4be865263db6c75dd6a9fb68 Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Fri, 10 Apr 2026 23:04:26 -0700 Subject: [PATCH 01/44] Basic repo creation implemented, with tests. --- Cargo.lock | 1 + Cargo.toml | 50 ++++-- src/server/config.rs | 27 +++ src/server/encryption.rs | 2 +- src/server/gitsync/mod.rs | 342 ++++++++++++++++++++++++++++++++++++++ src/server/mod.rs | 3 + 6 files changed, 414 insertions(+), 11 deletions(-) create mode 100644 src/server/gitsync/mod.rs diff --git a/Cargo.lock b/Cargo.lock index ef6b1b28e..3928567d5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2478,6 +2478,7 @@ dependencies = [ "flate2", "getrandom 0.4.2", "google-cloud-storage", + "hex", "httptest", "idb", "libc", diff --git a/Cargo.toml b/Cargo.toml index 5400909f7..c320881eb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,7 +14,7 @@ edition = "2021" rust-version = "1.91.1" [workspace] -members = [ "xtask" ] +members = ["xtask"] resolver = "2" [lib] @@ -24,15 +24,32 @@ crate-type = ["cdylib", "rlib"] default = ["sync", "bundled", "storage", "tls-webpki-roots"] # Support for all sync solutions -sync = ["server-sync", "server-gcp", "server-aws", "server-local"] +sync = ["server-sync", "server-gcp", "server-aws", "server-local", "server-git"] # Support for sync to a server server-sync = ["encryption", "http"] # Support for sync to GCP -server-gcp = ["cloud", "encryption", "http", "dep:google-cloud-storage", "dep:reqwest-middleware"] +server-gcp = [ + "cloud", + "encryption", + "http", + "dep:google-cloud-storage", + "dep:reqwest-middleware", +] # Support for sync to AWS -server-aws = ["cloud", "encryption", "http", "dep:aws-sdk-s3", "dep:aws-config", "dep:aws-credential-types", "dep:aws-smithy-runtime-api", "dep:aws-smithy-types"] +server-aws = [ + "cloud", + "encryption", + "http", + "dep:aws-sdk-s3", + "dep:aws-config", + "dep:aws-credential-types", + "dep:aws-smithy-runtime-api", + "dep:aws-smithy-types", +] # Suppport for sync to another SQLite database on the same machine server-local = ["dep:rusqlite"] +# Support for sync via Git +server-git = ["git-sync"] # Support for all task storage backends (except indexeddb, which only works on WASM builds) storage = ["storage-sqlite"] @@ -45,6 +62,8 @@ storage-indexeddb = ["dep:idb", "dep:web-sys"] encryption = ["dep:ring"] # (private) Generic support for cloud sync cloud = [] +# (private) Support for Git based sync +git-sync = ["dep:hex"] # static bundling of dependencies bundled = ["rusqlite/bundled"] # use CA roots built into the library for all HTTPS access @@ -64,12 +83,13 @@ async-trait = "0.1.89" byteorder = "1.5" chrono = { version = "^0.4.44", features = ["serde"] } flate2 = "1" +hex = { version = "0.4", optional = true } idb = { version = "0.6.4", optional = true } log = "^0.4.17" # Reqwest is "stuck" at 0.12 because that's what google-cloud-storage depends on. reqwest = { version = "0.12", default-features = false, optional = true } ring = { version = "0.17", optional = true } -rusqlite = { version = "0.39", features = [ "fallible_uint" ], optional = true} +rusqlite = { version = "0.39", features = ["fallible_uint"], optional = true } serde_json = "^1.0" serde = { version = "^1.0.147", features = ["derive"] } strum = "0.28" @@ -82,7 +102,9 @@ url = { version = "2", optional = true } ## wasm-only dependencies. [target.'cfg(target_arch = "wasm32")'.dependencies] uuid = { version = "1", features = ["js"] } -ring = { version = "0", optional = true, features = ["wasm32_unknown_unknown_js"] } +ring = { version = "0", optional = true, features = [ + "wasm32_unknown_unknown_js", +] } getrandom = { version = "0.4", features = ["wasm_js"] } wasm-bindgen = { version = "0.2" } wasm-bindgen-futures = { version = "0.4" } @@ -101,16 +123,24 @@ web-sys = { version = "0.3.83", optional = true, features = [ "EventTarget", "Event", "console", -]} +] } ## non-wasm dependencies [target.'cfg(not(target_arch = "wasm32"))'.dependencies] aws-sdk-s3 = { version = "1.127", default-features = false, optional = true } -aws-config = { version = "1.8", default-features = false, features = ["rt-tokio", "behavior-version-latest"], optional = true } -aws-credential-types = { version = "1", default-features = false, features = ["hardcoded-credentials"], optional = true } +aws-config = { version = "1.8", default-features = false, features = [ + "rt-tokio", + "behavior-version-latest", +], optional = true } +aws-credential-types = { version = "1", default-features = false, features = [ + "hardcoded-credentials", +], optional = true } aws-smithy-runtime-api = { version = "1.11", default-features = false, optional = true } aws-smithy-types = { version = "1.4", default-features = false, optional = true } -google-cloud-storage = { version = "0.24.0", default-features = false, features = ["rustls-tls", "auth"], optional = true } +google-cloud-storage = { version = "0.24.0", default-features = false, features = [ + "rustls-tls", + "auth", +], optional = true } reqwest-middleware = { version = "0.4", optional = true } ## common dev-dependencies diff --git a/src/server/config.rs b/src/server/config.rs index fd2396a0a..3f3289391 100644 --- a/src/server/config.rs +++ b/src/server/config.rs @@ -118,6 +118,26 @@ pub enum ServerConfig { /// be any suitably un-guessable string of bytes. encryption_secret: Vec, }, + /// A git repository + #[cfg(feature = "git-sync")] + Git { + /// The path to the local repo. + local_path: PathBuf, + /// The branch to use. + branch: String, + /// The remote repo. + /// + /// This can either be a named remote such as `origin` or a full git + /// url such as `git@myserver.com:/path/to/repo.git` + /// If `None` will use `origin` if the local repo has one. + /// Otherwise will operate in local only mode. + remote: String, + /// Don't clone/push/pull to `remote` even if it is defined. + local_only: bool, + /// Private encryption secret used to encrypt all data sent to the server. This can + /// be any suitably un-guessable string of bytes. + encryption_secret: Vec, + }, } impl ServerConfig { @@ -162,6 +182,13 @@ impl ServerConfig { ) .await?, ), + ServerConfig::Git { + local_path, + branch, + remote, + local_only, + encryption_secret, + } => todo!(), }) } } diff --git a/src/server/encryption.rs b/src/server/encryption.rs index aa2c52e85..40e8c3f53 100644 --- a/src/server/encryption.rs +++ b/src/server/encryption.rs @@ -27,7 +27,7 @@ impl Cryptor { } /// Generate a suitable random salt. - #[cfg(any(test, feature = "cloud"))] // server-sync uses the clientId as the salt. + #[cfg(any(test, feature = "cloud", feature = "git-sync"))] // server-sync uses the clientId as the salt. pub(super) fn gen_salt() -> Result> { let rng = rand::SystemRandom::new(); let mut salt = [0u8; 16]; diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs new file mode 100644 index 000000000..059995e30 --- /dev/null +++ b/src/server/gitsync/mod.rs @@ -0,0 +1,342 @@ +use crate::errors::Result; +use crate::server::encryption::Cryptor; +use crate::server::{ + AddVersionResult, GetVersionResult, HistorySegment, Server, Snapshot, SnapshotUrgency, + VersionId, +}; +use crate::Error; +use async_trait::async_trait; +use log::info; +use serde::{Deserialize, Serialize}; +use std::fs::{self, File}; +use std::io::Read; +use std::path::{Path, PathBuf}; +use std::process::Command; +use uuid::Uuid; +#[derive(Serialize, Deserialize, Debug)] +struct Version { + version_id: VersionId, + parent_version_id: VersionId, + history_segment: HistorySegment, +} + +/// The meta file holds the UUID of the most recent Version and the salt. +#[derive(Serialize, Deserialize, Debug)] +struct Meta { + #[serde(with = "uuid::serde::simple")] + latest: VersionId, + salt: String, // hex-encoded +} + +pub(crate) struct GitSyncServer { + meta: Meta, + local_path: PathBuf, + branch: String, + remote: String, + local_only: bool, + cryptor: Cryptor, +} + +/// Run a git command in a given directory, returning an error if it exits non-zero. +fn git_cmd(dir: &Path, args: &[&str]) -> Result<()> { + let status = Command::new("git").args(args).current_dir(dir).status()?; + if !status.success() { + return Err(Error::Server(format!( + "git {} failed with status {}", + args.join(" "), + status + ))); + } + Ok(()) +} + +impl GitSyncServer { + pub(crate) fn new( + local_path: PathBuf, + branch: String, + remote: String, + local_only: bool, + encryption_secret: Vec, + ) -> Result { + let meta = Self::init_repo(&local_path, &branch, &remote, local_only)?; + let salt_bytes = hex::decode(&meta.salt) + .map_err(|e| Error::Server(format!("invalid salt in meta file: {e}")))?; + let cryptor = Cryptor::new(&salt_bytes, &encryption_secret.into())?; + let server = GitSyncServer { + meta, + local_path, + branch, + remote, + local_only, + cryptor, + }; + Ok(server) + } + + fn init_repo(local_path: &Path, branch: &str, remote: &str, local_only: bool) -> Result { + // Create the local directory if needed. + fs::create_dir_all(local_path)?; + + // Check if path is already a git repo. + let is_repo = Command::new("git") + .arg("rev-parse") + .current_dir(local_path) + .output()? + .status + .success(); + + if !is_repo { + if local_only { + info!("Creating new repo at {:?}", local_path); + git_cmd(local_path, &["init"])?; + } else { + info!("Cloning repo from {:?} to {:?}", remote, local_path); + let parent = local_path + .parent() + .ok_or_else(|| Error::Server("local_path has no parent".into()))?; + let dir_name = local_path + .file_name() + .ok_or_else(|| Error::Server("local_path has no file name".into()))? + .to_str() + .ok_or_else(|| Error::Server("local_path is not valid UTF-8".into()))?; + git_cmd(parent, &["clone", remote, dir_name])?; + } + // Set identity so commits work in environments without a global git config. + git_cmd(local_path, &["config", "user.email", "taskchampion@local"])?; + git_cmd(local_path, &["config", "user.name", "taskchampion"])?; + } + + // Switch to the requested branch. Try checking out an existing branch first, + // only create a new one if that fails. + info!("Switching branch to {:?}", branch); + let checkout_ok = Command::new("git") + .args(["checkout", branch]) + .current_dir(local_path) + .stderr(std::process::Stdio::null()) + .status()? + .success(); + if !checkout_ok { + // For a brand-new repo (no commits) `git checkout -b` also fails, so use + // `git symbolic-ref` to point HEAD at the desired branch without needing a commit. + let has_commits = Command::new("git") + .args(["rev-parse", "HEAD"]) + .current_dir(local_path) + .stderr(std::process::Stdio::null()) + .status()? + .success(); + if has_commits { + git_cmd(local_path, &["checkout", "-b", branch])?; + } else { + git_cmd( + local_path, + &["symbolic-ref", "HEAD", &format!("refs/heads/{}", branch)], + )?; + } + } + + // Check for meta file, create and commit if missing. + let meta_path = local_path.join("meta"); + let meta = match File::open(&meta_path) { + Ok(mut file) => { + let mut contents = String::new(); + file.read_to_string(&mut contents)?; + serde_json::from_str(&contents)? + } + Err(_) => { + let m = Meta { + latest: Uuid::nil(), + salt: hex::encode(Cryptor::gen_salt()?), + }; + let f = File::create_new(&meta_path)?; + serde_json::to_writer(f, &m)?; + git_cmd(local_path, &["add", "meta"])?; + git_cmd(local_path, &["commit", "-m", "init taskchampion repo"])?; + m + } + }; + + Ok(meta) + } + + /// Stage the given paths and create a commit. + fn stage_and_commit(&self, paths: &[&Path], message: &str) -> Result<()> { + for path in paths { + let path_str = path + .to_str() + .ok_or_else(|| Error::Server("path is not valid UTF-8".into()))?; + git_cmd(&self.local_path, &["add", path_str])?; + } + git_cmd(&self.local_path, &["commit", "-m", message])?; + Ok(()) + } + + /// Read the meta file from disk and update self.meta. + fn read_meta(&mut self) -> Result<()> { + let meta_path = self.local_path.join("meta"); + let mut file = File::open(&meta_path)?; + let mut contents = String::new(); + file.read_to_string(&mut contents)?; + self.meta = serde_json::from_str(&contents)?; + Ok(()) + } + + /// Serialise self.meta to the meta file and return its path. + fn write_meta(&self) -> Result { + let meta_path = self.local_path.join("meta"); + let f = File::create(&meta_path)?; + serde_json::to_writer(f, &self.meta)?; + Ok(meta_path) + } +} + +#[async_trait(?Send)] +impl Server for GitSyncServer { + async fn add_version( + &mut self, + parent_version_id: VersionId, + history_segment: HistorySegment, + ) -> Result<(AddVersionResult, SnapshotUrgency)> { + todo!(); + // check the parent_version_id for linearity + // if parent_version_id != self.meta.latest { + // // Pull and try again if remote exists and not local_only + + // // Else fail + // return Ok(( + // AddVersionResult::ExpectedParentVersion(self.meta.latest), + // SnapshotUrgency::None, + // )); + // } + + // // invent a new ID for this version + // let version_id = Uuid::new_v4(); + // let version_path = self.add_version_by_parent_version_id(Version { + // version_id, + // parent_version_id, + // history_segment, + // })?; + // let meta = self.set_latest_version_id(version_id)?; + // git add and commit version_path and meta + // if remote mode: + // push + // if push fails + // revert local commit + // re-read latest + // return Ok(( + // AddVersionResult::ExpectedParentVersion(self.meta.latest), + // SnapshotUrgency::None, + // )); + // Ok((AddVersionResult::Ok(version_id), SnapshotUrgency::None)) + } + + async fn get_child_version( + &mut self, + parent_version_id: VersionId, + ) -> Result { + todo!(); + // if let Some(version) = self.get_version_by_parent_version_id(parent_version_id)? { + // Ok(GetVersionResult::Version { + // version_id: version.version_id, + // parent_version_id: version.parent_version_id, + // history_segment: version.history_segment, + // }) + // } else { + // Ok(GetVersionResult::NoSuchVersion) + // } + } + + async fn add_snapshot(&mut self, _version_id: VersionId, _snapshot: Snapshot) -> Result<()> { + todo!(); + } + + async fn get_snapshot(&mut self) -> Result> { + todo!(); + } +} + +#[cfg(test)] +mod test { + use super::*; + use tempfile::TempDir; + + fn make_server(dir: &Path) -> Result { + GitSyncServer::new( + dir.to_path_buf(), + "main".into(), + "".into(), + true, + b"test-secret".to_vec(), + ) + } + + #[test] + fn test_init_creates_repo_and_meta() -> Result<()> { + let tmp = TempDir::new()?; + let server = make_server(tmp.path())?; + assert!(tmp.path().join("meta").exists()); + assert_eq!(server.meta.latest, Uuid::nil()); + eprintln!("tmp dir: {}", tmp.path().display()); + std::mem::forget(tmp); + Ok(()) + } + + #[test] + fn test_init_idempotent() -> Result<()> { + let tmp = TempDir::new()?; + let s1 = make_server(tmp.path())?; + // second call should succeed and load the same meta (same salt) + let s2 = make_server(tmp.path())?; + assert_eq!(s1.meta.salt, s2.meta.salt); + Ok(()) + } + + #[test] + fn test_read_write_meta_roundtrip() -> Result<()> { + let tmp = TempDir::new()?; + let mut server = make_server(tmp.path())?; + + let new_id = Uuid::new_v4(); + server.meta.latest = new_id; + server.write_meta()?; + + server.meta.latest = Uuid::nil(); // clobber in-memory value + server.read_meta()?; + assert_eq!(server.meta.latest, new_id); + Ok(()) + } + + #[test] + fn test_stage_and_commit() -> Result<()> { + let tmp = TempDir::new()?; + let server = make_server(tmp.path())?; + + // Write a new file, stage and commit it. + let new_file = tmp.path().join("testfile"); + std::fs::write(&new_file, b"hello, taskchampion")?; + server.stage_and_commit(&[&new_file], "test commit")?; + + // Verify the commit exists, git show HEAD succeeds only if there is a HEAD commit. + git_cmd(tmp.path(), &["show", "HEAD"])?; + eprintln!("tmp dir: {}", tmp.path().display()); + std::mem::forget(tmp); + Ok(()) + } + + // #[tokio::test] + // async fn test_empty() -> Result<()> { ... } + + // #[tokio::test] + // async fn test_add_zero_base() -> Result<()> { ... } + + // #[tokio::test] + // async fn test_add_nonzero_base() -> Result<()> { ... } + + // #[tokio::test] + // async fn test_add_nonzero_base_forbidden() -> Result<()> { ... } + + // #[tokio::test] + // async fn test_snapshot() -> Result<()> { ... } + + // #[tokio::test] + // async fn test_conflict_with_local_remote() -> Result<()> { ... } +} diff --git a/src/server/mod.rs b/src/server/mod.rs index 33bcc41aa..3c5c93eca 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -31,6 +31,9 @@ mod sync; #[cfg(feature = "cloud")] mod cloud; +#[cfg(feature = "git-sync")] +mod gitsync; + pub use config::*; pub use types::*; From a9b32174e266d1be93a9d444b07d198ba7aedf0c Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Sat, 11 Apr 2026 00:25:01 -0700 Subject: [PATCH 02/44] add git push/pull helpers --- src/server/gitsync/mod.rs | 142 +++++++++++++++++++++++++++++++++----- 1 file changed, 124 insertions(+), 18 deletions(-) diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index 059995e30..1fdbcf5ec 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -25,7 +25,7 @@ struct Version { struct Meta { #[serde(with = "uuid::serde::simple")] latest: VersionId, - salt: String, // hex-encoded + salt: String, // hex-encoded } pub(crate) struct GitSyncServer { @@ -187,6 +187,30 @@ impl GitSyncServer { serde_json::to_writer(f, &self.meta)?; Ok(meta_path) } + + /// Fetch and fast-forward to the remote branch. No-op in local-only mode. + fn pull(&self) -> Result<()> { + if self.local_only { + return Ok(()); + } + git_cmd(&self.local_path, &["fetch", &self.remote, &self.branch])?; + git_cmd(&self.local_path, &["reset", "--hard", "FETCH_HEAD"])?; + Ok(()) + } + + /// Push to the remote branch. Returns `true` on success, `false` if the push is rejected + /// Always returns `true` in local-only mode. + fn push(&self) -> Result { + if self.local_only { + return Ok(true); + } + let status = Command::new("git") + .args(["push", &self.remote, &self.branch]) + .current_dir(&self.local_path) + .stderr(std::process::Stdio::null()) + .status()?; + Ok(status.success()) + } } #[async_trait(?Send)] @@ -269,14 +293,36 @@ mod test { ) } + /// Create a bare repo to act as a remote, then clone it into `clone_dir`. + /// Returns a GitSyncServer backed by the clone, with the bare repo as its remote. + fn make_server_with_remote(bare_dir: &Path, clone_dir: &Path) -> Result { + // Initialise the bare remote. + git_cmd( + bare_dir.parent().unwrap(), + &[ + "init", + "--bare", + bare_dir.file_name().unwrap().to_str().unwrap(), + ], + )?; + let bare_url = bare_dir.to_str().unwrap(); + GitSyncServer::new( + clone_dir.to_path_buf(), + "main".into(), + bare_url.into(), + false, + b"test-secret".to_vec(), + ) + } + #[test] fn test_init_creates_repo_and_meta() -> Result<()> { let tmp = TempDir::new()?; let server = make_server(tmp.path())?; assert!(tmp.path().join("meta").exists()); assert_eq!(server.meta.latest, Uuid::nil()); - eprintln!("tmp dir: {}", tmp.path().display()); - std::mem::forget(tmp); + // eprintln!("tmp dir: {}", tmp.path().display()); + // std::mem::forget(tmp); Ok(()) } @@ -314,29 +360,89 @@ mod test { let new_file = tmp.path().join("testfile"); std::fs::write(&new_file, b"hello, taskchampion")?; server.stage_and_commit(&[&new_file], "test commit")?; - // Verify the commit exists, git show HEAD succeeds only if there is a HEAD commit. git_cmd(tmp.path(), &["show", "HEAD"])?; - eprintln!("tmp dir: {}", tmp.path().display()); - std::mem::forget(tmp); + // eprintln!("tmp dir: {}", tmp.path().display()); + // std::mem::forget(tmp); Ok(()) } - // #[tokio::test] - // async fn test_empty() -> Result<()> { ... } + #[test] + fn test_push_and_pull() -> Result<()> { + let tmp = TempDir::new()?; + let bare = tmp.path().join("bare"); + let clone1 = tmp.path().join("clone1"); + let clone2 = tmp.path().join("clone2"); + + // Set up first clone (initialises remote with the meta commit). + let server1 = make_server_with_remote(&bare, &clone1)?; + server1.push()?; + + // Clone a second copy directly via git. + let bare_url = bare.to_str().unwrap(); + git_cmd(tmp.path(), &["clone", bare_url, "clone2"])?; + git_cmd(&clone2, &["config", "user.email", "taskchampion@local"])?; + git_cmd(&clone2, &["config", "user.name", "taskchampion"])?; + + // Write a new file in clone1 and push it. + let new_file = clone1.join("testfile"); + std::fs::write(&new_file, b"hello")?; + server1.stage_and_commit(&[&new_file], "add testfile")?; + assert!(server1.push()?); + + // Build a server for clone2 and pull + // It should see the new file. + let bare_url_str: String = bare_url.into(); + let mut server2 = GitSyncServer::new( + clone2.clone(), + "main".into(), + bare_url_str, + false, + b"test-secret".to_vec(), + )?; + server2.pull()?; + assert!(clone2.join("testfile").exists()); + // eprintln!("tmp dir: {}", tmp.path().display()); + // std::mem::forget(tmp); + Ok(()) + } + + #[test] + fn test_push_rejected_on_conflict() -> Result<()> { + let tmp = TempDir::new()?; + let bare = tmp.path().join("bare"); + let clone1 = tmp.path().join("clone1"); + let clone2 = tmp.path().join("clone2"); - // #[tokio::test] - // async fn test_add_zero_base() -> Result<()> { ... } + let server1 = make_server_with_remote(&bare, &clone1)?; + server1.push()?; - // #[tokio::test] - // async fn test_add_nonzero_base() -> Result<()> { ... } + // Clone a second copy. + let bare_url = bare.to_str().unwrap(); + git_cmd(tmp.path(), &["clone", bare_url, "clone2"])?; + git_cmd(&clone2, &["config", "user.email", "taskchampion@local"])?; + git_cmd(&clone2, &["config", "user.name", "taskchampion"])?; - // #[tokio::test] - // async fn test_add_nonzero_base_forbidden() -> Result<()> { ... } + let mut server2 = GitSyncServer::new( + clone2.clone(), + "main".into(), + bare_url.into(), + false, + b"test-secret".to_vec(), + )?; - // #[tokio::test] - // async fn test_snapshot() -> Result<()> { ... } + // clone1 pushes first. + let f1 = clone1.join("f1"); + std::fs::write(&f1, b"from clone1")?; + server1.stage_and_commit(&[&f1], "clone1 commit")?; + assert!(server1.push()?); - // #[tokio::test] - // async fn test_conflict_with_local_remote() -> Result<()> { ... } + // clone2 also commits (diverged history) and tries to push — should be rejected. + let f2 = clone2.join("f2"); + std::fs::write(&f2, b"from clone2")?; + server2.stage_and_commit(&[&f2], "clone2 commit")?; + assert!(!server2.push()?); + + Ok(()) + } } From 26fbaaff08d9f049b65098d081547c2fe510fb23 Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Sat, 11 Apr 2026 02:21:24 -0700 Subject: [PATCH 03/44] add version and then to bed --- src/server/gitsync/mod.rs | 206 ++++++++++++++++++++++++++------------ 1 file changed, 143 insertions(+), 63 deletions(-) diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index 1fdbcf5ec..320e69acd 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -2,7 +2,7 @@ use crate::errors::Result; use crate::server::encryption::Cryptor; use crate::server::{ AddVersionResult, GetVersionResult, HistorySegment, Server, Snapshot, SnapshotUrgency, - VersionId, + VersionId, NIL_VERSION_ID, }; use crate::Error; use async_trait::async_trait; @@ -211,6 +211,23 @@ impl GitSyncServer { .status()?; Ok(status.success()) } + + /// Encrypt and write a version file. Returns the file path. + fn add_version_by_parent_version_id(&self, version: &Version) -> Result { + use crate::server::encryption::{Sealed, Unsealed}; + let sealed = self.cryptor.seal(Unsealed { + version_id: version.version_id, + payload: version.history_segment.clone(), + })?; + let filename = format!( + "v-{}-{}", + version.parent_version_id.simple(), + version.version_id.simple() + ); + let path = self.local_path.join(&filename); + std::fs::write(&path, Vec::::from(sealed))?; + Ok(path) + } } #[async_trait(?Send)] @@ -220,37 +237,46 @@ impl Server for GitSyncServer { parent_version_id: VersionId, history_segment: HistorySegment, ) -> Result<(AddVersionResult, SnapshotUrgency)> { - todo!(); - // check the parent_version_id for linearity - // if parent_version_id != self.meta.latest { - // // Pull and try again if remote exists and not local_only - - // // Else fail - // return Ok(( - // AddVersionResult::ExpectedParentVersion(self.meta.latest), - // SnapshotUrgency::None, - // )); - // } + // Accept any parent when the repo is empty (latest == NIL). + // Otherwise check if parent is in latest. If it isn't, pull recheck. + if self.meta.latest != Uuid::nil() && parent_version_id != self.meta.latest { + self.pull()?; + self.read_meta()?; + if parent_version_id != self.meta.latest { + return Ok(( + AddVersionResult::ExpectedParentVersion(self.meta.latest), + SnapshotUrgency::None, + )); + } + } - // // invent a new ID for this version - // let version_id = Uuid::new_v4(); - // let version_path = self.add_version_by_parent_version_id(Version { - // version_id, - // parent_version_id, - // history_segment, - // })?; - // let meta = self.set_latest_version_id(version_id)?; - // git add and commit version_path and meta - // if remote mode: - // push - // if push fails - // revert local commit - // re-read latest - // return Ok(( - // AddVersionResult::ExpectedParentVersion(self.meta.latest), - // SnapshotUrgency::None, - // )); - // Ok((AddVersionResult::Ok(version_id), SnapshotUrgency::None)) + // Create the new version and write it to file. + let version_id = Uuid::new_v4(); + let version = Version { + version_id, + parent_version_id, + history_segment, + }; + let version_path = self.add_version_by_parent_version_id(&version)?; + self.meta.latest = version_id; + let meta_path = self.write_meta()?; + + // Commit and push, reverting if push fails. + self.stage_and_commit(&[&version_path, &meta_path], "add version")?; + + if !self.push()? { + // Push was rejected (non-fast-forward). Undo the commit and re-read remote state. + git_cmd(&self.local_path, &["reset", "HEAD~1", "--soft"])?; + std::fs::remove_file(&version_path)?; + self.pull()?; + self.read_meta()?; + return Ok(( + AddVersionResult::ExpectedParentVersion(self.meta.latest), + SnapshotUrgency::None, + )); + } + + Ok((AddVersionResult::Ok(version_id), SnapshotUrgency::None)) } async fn get_child_version( @@ -351,21 +377,6 @@ mod test { Ok(()) } - #[test] - fn test_stage_and_commit() -> Result<()> { - let tmp = TempDir::new()?; - let server = make_server(tmp.path())?; - - // Write a new file, stage and commit it. - let new_file = tmp.path().join("testfile"); - std::fs::write(&new_file, b"hello, taskchampion")?; - server.stage_and_commit(&[&new_file], "test commit")?; - // Verify the commit exists, git show HEAD succeeds only if there is a HEAD commit. - git_cmd(tmp.path(), &["show", "HEAD"])?; - // eprintln!("tmp dir: {}", tmp.path().display()); - // std::mem::forget(tmp); - Ok(()) - } #[test] fn test_push_and_pull() -> Result<()> { @@ -407,42 +418,111 @@ mod test { Ok(()) } - #[test] - fn test_push_rejected_on_conflict() -> Result<()> { + + #[tokio::test] + async fn test_add_zero_base() -> Result<()> { + let tmp = TempDir::new()?; + let mut server = make_server(tmp.path())?; + let history = b"1234".to_vec(); + match server.add_version(NIL_VERSION_ID, history.clone()).await?.0 { + AddVersionResult::ExpectedParentVersion(_) => { + panic!("should have accepted the version") + } + AddVersionResult::Ok(version_id) => { + // Verify the version file exists on disk. + let filename = format!("v-{}-{}", NIL_VERSION_ID.simple(), version_id.simple()); + assert!( + tmp.path().join(&filename).exists(), + "version file missing: {filename}" + ); + // Verify meta was updated. + assert_eq!(server.meta.latest, version_id); + } + } + Ok(()) + } + + #[tokio::test] + async fn test_add_nonzero_base() -> Result<()> { + let tmp = TempDir::new()?; + let mut server = make_server(tmp.path())?; + let history = b"1234".to_vec(); + let parent_version_id = Uuid::new_v4(); + + // OK because latest == NIL (repo is empty). + match server.add_version(parent_version_id, history).await?.0 { + AddVersionResult::ExpectedParentVersion(_) => { + panic!("should have accepted the version") + } + AddVersionResult::Ok(version_id) => { + assert_eq!(server.meta.latest, version_id); + } + } + Ok(()) + } + + #[tokio::test] + async fn test_add_nonzero_base_forbidden() -> Result<()> { + let tmp = TempDir::new()?; + let mut server = make_server(tmp.path())?; + let history = b"1234".to_vec(); + let parent_version_id = Uuid::new_v4(); + + // Add a first version. + if let (AddVersionResult::ExpectedParentVersion(_), _) = server + .add_version(parent_version_id, history.clone()) + .await? + { + panic!("should have accepted the first version"); + } + + // Try to add another with the same (now stale) parent, should be rejected. + match server.add_version(parent_version_id, history).await?.0 { + AddVersionResult::Ok(_) => panic!("should not have accepted the version"), + AddVersionResult::ExpectedParentVersion(expected) => { + assert_eq!(expected, server.meta.latest); + } + } + Ok(()) + } + + #[tokio::test] + async fn test_add_version_conflict_with_remote() -> Result<()> { let tmp = TempDir::new()?; let bare = tmp.path().join("bare"); let clone1 = tmp.path().join("clone1"); let clone2 = tmp.path().join("clone2"); - let server1 = make_server_with_remote(&bare, &clone1)?; + let mut server1 = make_server_with_remote(&bare, &clone1)?; server1.push()?; - // Clone a second copy. let bare_url = bare.to_str().unwrap(); git_cmd(tmp.path(), &["clone", bare_url, "clone2"])?; git_cmd(&clone2, &["config", "user.email", "taskchampion@local"])?; git_cmd(&clone2, &["config", "user.name", "taskchampion"])?; - let mut server2 = GitSyncServer::new( - clone2.clone(), + clone2, "main".into(), bare_url.into(), false, b"test-secret".to_vec(), )?; - // clone1 pushes first. - let f1 = clone1.join("f1"); - std::fs::write(&f1, b"from clone1")?; - server1.stage_and_commit(&[&f1], "clone1 commit")?; - assert!(server1.push()?); - - // clone2 also commits (diverged history) and tries to push — should be rejected. - let f2 = clone2.join("f2"); - std::fs::write(&f2, b"from clone2")?; - server2.stage_and_commit(&[&f2], "clone2 commit")?; - assert!(!server2.push()?); + // server1 adds a version from NIL parent and pushes successfully. + let (result1, _) = server1.add_version(NIL_VERSION_ID, b"v1".to_vec()).await?; + let v1_id = match result1 { + AddVersionResult::Ok(id) => id, + AddVersionResult::ExpectedParentVersion(_) => panic!("server1 add failed"), + }; + // server2 also tries to add from NIL, should fail with ExpectedParentVersion pointing at v1. + let (result2, _) = server2.add_version(NIL_VERSION_ID, b"v2".to_vec()).await?; + match result2 { + AddVersionResult::Ok(_) => panic!("server2 should have been rejected"), + AddVersionResult::ExpectedParentVersion(expected) => { + assert_eq!(expected, v1_id); + } + } Ok(()) } } From e69b03b8c027259ac438809470939f06b9841f6d Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Sat, 11 Apr 2026 12:20:28 -0700 Subject: [PATCH 04/44] Add get_version_by_parent_version_id and test --- Cargo.lock | 1 + Cargo.toml | 1 + src/server/config.rs | 10 ++++- src/server/encryption.rs | 2 + src/server/gitsync/mod.rs | 79 +++++++++++++++++++++++++++++++++++---- 5 files changed, 83 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3928567d5..0146ead8b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2477,6 +2477,7 @@ dependencies = [ "chrono", "flate2", "getrandom 0.4.2", + "glob", "google-cloud-storage", "hex", "httptest", diff --git a/Cargo.toml b/Cargo.toml index c320881eb..04bd4def5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -98,6 +98,7 @@ tokio = { version = "1", features = ["macros", "sync", "rt"] } thiserror = "2.0" uuid = { version = "^1.23.0", features = ["serde", "v4"] } url = { version = "2", optional = true } +glob = "0.3.3" ## wasm-only dependencies. [target.'cfg(target_arch = "wasm32")'.dependencies] diff --git a/src/server/config.rs b/src/server/config.rs index 3f3289391..a54aa2163 100644 --- a/src/server/config.rs +++ b/src/server/config.rs @@ -1,5 +1,4 @@ use super::types::Server; -use crate::errors::Result; #[cfg(feature = "server-aws")] pub use crate::server::cloud::aws::AwsCredentials; #[cfg(feature = "server-aws")] @@ -12,6 +11,7 @@ use crate::server::cloud::CloudServer; use crate::server::local::LocalServer; #[cfg(feature = "server-sync")] use crate::server::sync::SyncServer; +use crate::{errors::Result, server::gitsync::GitSyncServer}; #[cfg(feature = "server-local")] use std::path::PathBuf; #[cfg(feature = "server-sync")] @@ -188,7 +188,13 @@ impl ServerConfig { remote, local_only, encryption_secret, - } => todo!(), + } => Box::new(GitSyncServer::new( + local_path, + branch, + remote, + local_only, + encryption_secret, + )?), }) } } diff --git a/src/server/encryption.rs b/src/server/encryption.rs index 40e8c3f53..951a4fd24 100644 --- a/src/server/encryption.rs +++ b/src/server/encryption.rs @@ -172,6 +172,7 @@ impl<'a> Envelope<'a> { /// A unsealed payload with an attached version_id. The version_id is used to /// validate the context of the payload on unsealing. +#[derive(Debug)] pub(super) struct Unsealed { pub(super) version_id: Uuid, pub(super) payload: Vec, @@ -184,6 +185,7 @@ impl From for Vec { } /// An encrypted payload +#[derive(Debug)] pub(super) struct Sealed { pub(super) version_id: Uuid, pub(super) payload: Vec, diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index 320e69acd..9d0b19ac3 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -1,11 +1,12 @@ use crate::errors::Result; -use crate::server::encryption::Cryptor; +use crate::server::encryption::{Cryptor, Sealed, Unsealed}; use crate::server::{ AddVersionResult, GetVersionResult, HistorySegment, Server, Snapshot, SnapshotUrgency, VersionId, NIL_VERSION_ID, }; use crate::Error; use async_trait::async_trait; +use glob::glob; use log::info; use serde::{Deserialize, Serialize}; use std::fs::{self, File}; @@ -116,7 +117,7 @@ impl GitSyncServer { .status()? .success(); if !checkout_ok { - // For a brand-new repo (no commits) `git checkout -b` also fails, so use + // For a brand-new repo `git checkout -b` also fails, so use // `git symbolic-ref` to point HEAD at the desired branch without needing a commit. let has_commits = Command::new("git") .args(["rev-parse", "HEAD"]) @@ -214,11 +215,11 @@ impl GitSyncServer { /// Encrypt and write a version file. Returns the file path. fn add_version_by_parent_version_id(&self, version: &Version) -> Result { - use crate::server::encryption::{Sealed, Unsealed}; - let sealed = self.cryptor.seal(Unsealed { + let unsealed = Unsealed { version_id: version.version_id, payload: version.history_segment.clone(), - })?; + }; + let sealed = self.cryptor.seal(unsealed)?; let filename = format!( "v-{}-{}", version.parent_version_id.simple(), @@ -228,6 +229,37 @@ impl GitSyncServer { std::fs::write(&path, Vec::::from(sealed))?; Ok(path) } + + /// Read and decrypt a version file. Returns a Version if found, None if not. + fn get_version_by_parent_version_id(&self, version: &VersionId) -> Option { + // glob to find file. + // v-PARENT-CHILD + let pattern = format!("{}/v-{}-*", self.local_path.to_str()?, version.simple()); + for entry in glob(&pattern).ok()? { + let result = (|| -> Option { + let path = entry.ok()?; + let mut buf = Vec::new(); + File::open(&path).ok()?.read_to_end(&mut buf).ok()?; + let filename = path.to_str()?; + let (_, version_id_str) = filename.rsplit_once('-')?; + let version_id = Uuid::parse_str(version_id_str).ok()?; + let sealed = Sealed { + version_id, + payload: buf, + }; + let unsealed = self.cryptor.unseal(sealed).ok()?; + Some(Version { + version_id: unsealed.version_id, + parent_version_id: *version, + history_segment: unsealed.payload, + }) + })(); + if let Some(v) = result { + return Some(v); + } + } + None + } } #[async_trait(?Send)] @@ -377,7 +409,6 @@ mod test { Ok(()) } - #[test] fn test_push_and_pull() -> Result<()> { let tmp = TempDir::new()?; @@ -404,7 +435,7 @@ mod test { // Build a server for clone2 and pull // It should see the new file. let bare_url_str: String = bare_url.into(); - let mut server2 = GitSyncServer::new( + let server2 = GitSyncServer::new( clone2.clone(), "main".into(), bare_url_str, @@ -418,7 +449,6 @@ mod test { Ok(()) } - #[tokio::test] async fn test_add_zero_base() -> Result<()> { let tmp = TempDir::new()?; @@ -525,4 +555,37 @@ mod test { } Ok(()) } + + #[tokio::test] + async fn test_get_version_by_parent_id() -> Result<()> { + let tmp = TempDir::new()?; + let mut server = make_server(tmp.path())?; + let history = b"1234".to_vec(); + let parent_version_id = Uuid::new_v4(); + + // Version doesn't exist yet -> None + assert!(server + .get_version_by_parent_version_id(&parent_version_id) + .is_none()); + + // Add a first version. + let (rst, _) = server + .add_version(parent_version_id, history.clone()) + .await?; + + let AddVersionResult::Ok(version_id) = rst else { + panic!("Couldn't add version"); + }; + + match server.get_version_by_parent_version_id(&parent_version_id) { + Some(version) => { + assert_eq!(version.parent_version_id, parent_version_id); + assert_eq!(version.history_segment, history); + assert_eq!(version.version_id, version_id); + } + None => panic!("Failed to read version"), + } + + Ok(()) + } } From 73989ec05fd8aca2af5d11d8b762910761eae1da Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Sun, 12 Apr 2026 08:49:13 -0700 Subject: [PATCH 05/44] add get_child_version --- src/server/gitsync/mod.rs | 76 +++++++++++++++++++++++++-------------- 1 file changed, 50 insertions(+), 26 deletions(-) diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index 9d0b19ac3..44d540549 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -156,6 +156,9 @@ impl GitSyncServer { } }; + // Remove any untracked files left behind by interrupted writes. + git_cmd(local_path, &["clean", "-fd"])?; + Ok(meta) } @@ -196,6 +199,9 @@ impl GitSyncServer { } git_cmd(&self.local_path, &["fetch", &self.remote, &self.branch])?; git_cmd(&self.local_path, &["reset", "--hard", "FETCH_HEAD"])?; + // Remove any untracked files so orphaned version/snapshot files from + // interrupted writes cannot be served as valid versions. + git_cmd(&self.local_path, &["clean", "-fd"])?; Ok(()) } @@ -231,11 +237,17 @@ impl GitSyncServer { } /// Read and decrypt a version file. Returns a Version if found, None if not. - fn get_version_by_parent_version_id(&self, version: &VersionId) -> Option { + fn get_version_by_parent_version_id(&self, version: &VersionId) -> Result> { // glob to find file. // v-PARENT-CHILD - let pattern = format!("{}/v-{}-*", self.local_path.to_str()?, version.simple()); - for entry in glob(&pattern).ok()? { + let pattern = format!( + "{}/v-{}-*", + self.local_path + .to_str() + .ok_or_else(|| Error::Server("path is not valid UTF-8".into()))?, + version.simple() + ); + for entry in glob(&pattern).map_err(|e| Error::Server(format!("{:?}", e)))? { let result = (|| -> Option { let path = entry.ok()?; let mut buf = Vec::new(); @@ -255,10 +267,10 @@ impl GitSyncServer { }) })(); if let Some(v) = result { - return Some(v); + return Ok(Some(v)); } } - None + Ok(None) } } @@ -315,16 +327,21 @@ impl Server for GitSyncServer { &mut self, parent_version_id: VersionId, ) -> Result { - todo!(); - // if let Some(version) = self.get_version_by_parent_version_id(parent_version_id)? { - // Ok(GetVersionResult::Version { - // version_id: version.version_id, - // parent_version_id: version.parent_version_id, - // history_segment: version.history_segment, - // }) - // } else { - // Ok(GetVersionResult::NoSuchVersion) - // } + let version = match self.get_version_by_parent_version_id(&parent_version_id)? { + Some(v) => Some(v), + None => { + self.pull()?; + self.get_version_by_parent_version_id(&parent_version_id)? + } + }; + match version { + Some(v) => Ok(GetVersionResult::Version { + version_id: v.version_id, + parent_version_id: v.parent_version_id, + history_segment: v.history_segment, + }), + None => Ok(GetVersionResult::NoSuchVersion), + } } async fn add_snapshot(&mut self, _version_id: VersionId, _snapshot: Snapshot) -> Result<()> { @@ -338,6 +355,8 @@ impl Server for GitSyncServer { #[cfg(test)] mod test { + use std::any::Any; + use super::*; use tempfile::TempDir; @@ -557,16 +576,17 @@ mod test { } #[tokio::test] - async fn test_get_version_by_parent_id() -> Result<()> { + async fn get_child_version() -> Result<()> { let tmp = TempDir::new()?; let mut server = make_server(tmp.path())?; let history = b"1234".to_vec(); let parent_version_id = Uuid::new_v4(); - // Version doesn't exist yet -> None - assert!(server - .get_version_by_parent_version_id(&parent_version_id) - .is_none()); + // Version doesn't exist yet + assert_eq!( + server.get_child_version(parent_version_id).await?, + GetVersionResult::NoSuchVersion + ); // Add a first version. let (rst, _) = server @@ -577,13 +597,17 @@ mod test { panic!("Couldn't add version"); }; - match server.get_version_by_parent_version_id(&parent_version_id) { - Some(version) => { - assert_eq!(version.parent_version_id, parent_version_id); - assert_eq!(version.history_segment, history); - assert_eq!(version.version_id, version_id); + match server.get_child_version(parent_version_id).await? { + GetVersionResult::Version { + version_id: v_id, + parent_version_id: p_id, + history_segment: h_seg, + } => { + assert_eq!(v_id, version_id); + assert_eq!(p_id, parent_version_id); + assert_eq!(h_seg, history); } - None => panic!("Failed to read version"), + GetVersionResult::NoSuchVersion => panic!("Failed to read version"), } Ok(()) From 660881cd7bd6ef6463262152bf44d775ce683ac2 Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Sun, 12 Apr 2026 12:05:07 -0700 Subject: [PATCH 06/44] add snapshot system --- Cargo.lock | 162 ++++++++++++++++++++++++++-- Cargo.toml | 6 +- src/server/gitsync/mod.rs | 218 ++++++++++++++++++++++++++++++++------ 3 files changed, 344 insertions(+), 42 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0146ead8b..6e05d1e3c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -642,6 +642,40 @@ dependencies = [ "typenum", ] +[[package]] +name = "darling" +version = "0.23.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "25ae13da2f202d56bd7f91c25fba009e7717a1e4a1cc98a76d844b65ae912e9d" +dependencies = [ + "darling_core", + "darling_macro", +] + +[[package]] +name = "darling_core" +version = "0.23.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9865a50f7c335f53564bb694ef660825eb8610e0a53d3e11bf1b0d3df31e03b0" +dependencies = [ + "ident_case", + "proc-macro2", + "quote", + "strsim", + "syn", +] + +[[package]] +name = "darling_macro" +version = "0.23.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ac3984ec7bd6cfa798e62b4a642426a5be0e68f9401cfc2a01e3fa9ea2fcdb8d" +dependencies = [ + "darling_core", + "quote", + "syn", +] + [[package]] name = "der" version = "0.7.10" @@ -691,6 +725,12 @@ dependencies = [ "syn", ] +[[package]] +name = "dyn-clone" +version = "1.0.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d0881ea181b1df73ff77ffaaf9c7544ecc11e82fba9b5f27b262a3c73a332555" + [[package]] name = "either" version = "1.15.0" @@ -1010,13 +1050,19 @@ dependencies = [ "futures-core", "futures-sink", "http 1.4.0", - "indexmap", + "indexmap 2.13.0", "slab", "tokio", "tokio-util", "tracing", ] +[[package]] +name = "hashbrown" +version = "0.12.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8a9ee70c43aaf417c914396645a0fa852624801b24ebb7ae78fe8272889ac888" + [[package]] name = "hashbrown" version = "0.15.5" @@ -1348,7 +1394,7 @@ version = "0.6.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6554f394e990a1af530a528a7fdcad6e01b29cb1b990f89df3ffd62cf15f7828" dependencies = [ - "indexmap", + "indexmap 2.13.0", "js-sys", "num-traits", "thiserror 2.0.18", @@ -1357,6 +1403,12 @@ dependencies = [ "web-sys", ] +[[package]] +name = "ident_case" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b9e0384b61958566e926dc50660321d12159025e767c18e043daf26b70104c39" + [[package]] name = "idna" version = "1.1.0" @@ -1378,6 +1430,17 @@ dependencies = [ "icu_properties", ] +[[package]] +name = "indexmap" +version = "1.9.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bd070e393353796e801d209ad339e89596eb4c8d430d18ede6a1cced8fafbd99" +dependencies = [ + "autocfg", + "hashbrown 0.12.3", + "serde", +] + [[package]] name = "indexmap" version = "2.13.0" @@ -1927,6 +1990,26 @@ dependencies = [ "bitflags", ] +[[package]] +name = "ref-cast" +version = "1.0.25" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f354300ae66f76f1c85c5f84693f0ce81d747e2c3f21a45fef496d89c960bf7d" +dependencies = [ + "ref-cast-impl", +] + +[[package]] +name = "ref-cast-impl" +version = "1.0.25" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b7186006dcb21920990093f30e3dea63b7d6e977bf1256be20c3563a5db070da" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "regex" version = "1.12.3" @@ -2203,6 +2286,30 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "schemars" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4cd191f9397d57d581cddd31014772520aa448f65ef991055d7f61582c65165f" +dependencies = [ + "dyn-clone", + "ref-cast", + "serde", + "serde_json", +] + +[[package]] +name = "schemars" +version = "1.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a2b42f36aa1cd011945615b92222f6bf73c599a102a300334cd7f8dbeec726cc" +dependencies = [ + "dyn-clone", + "ref-cast", + "serde", + "serde_json", +] + [[package]] name = "scopeguard" version = "1.2.0" @@ -2304,6 +2411,37 @@ dependencies = [ "serde", ] +[[package]] +name = "serde_with" +version = "3.18.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dd5414fad8e6907dbdd5bc441a50ae8d6e26151a03b1de04d89a5576de61d01f" +dependencies = [ + "base64 0.22.1", + "chrono", + "hex", + "indexmap 1.9.3", + "indexmap 2.13.0", + "schemars 0.9.0", + "schemars 1.2.1", + "serde_core", + "serde_json", + "serde_with_macros", + "time", +] + +[[package]] +name = "serde_with_macros" +version = "3.18.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d3db8978e608f1fe7357e211969fd9abdcae80bac1ba7a3369bb7eb6b404eb65" +dependencies = [ + "darling", + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "sha1" version = "0.10.6" @@ -2407,6 +2545,12 @@ version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6ce2be8dc25455e1f91df71bfa12ad37d7af1092ae736f3a6cd0e37bc7810596" +[[package]] +name = "strsim" +version = "0.11.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" + [[package]] name = "strum" version = "0.28.0" @@ -2479,7 +2623,6 @@ dependencies = [ "getrandom 0.4.2", "glob", "google-cloud-storage", - "hex", "httptest", "idb", "libc", @@ -2494,6 +2637,7 @@ dependencies = [ "serde", "serde-wasm-bindgen", "serde_json", + "serde_with", "strum", "strum_macros", "tempfile", @@ -2681,7 +2825,7 @@ version = "0.23.10+spec-1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "84c8b9f757e028cee9fa244aea147aab2a9ec09d5325a9b01e0a49730c2b5269" dependencies = [ - "indexmap", + "indexmap 2.13.0", "toml_datetime", "toml_parser", "winnow", @@ -3031,7 +3175,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bb0e353e6a2fbdc176932bbaab493762eb1255a7900fe0fea1a2f96c296cc909" dependencies = [ "anyhow", - "indexmap", + "indexmap 2.13.0", "wasm-encoder", "wasmparser", ] @@ -3057,7 +3201,7 @@ checksum = "47b807c72e1bac69382b3a6fb3dbe8ea4c0ed87ff5629b8685ae6b9a611028fe" dependencies = [ "bitflags", "hashbrown 0.15.5", - "indexmap", + "indexmap 2.13.0", "semver", ] @@ -3351,7 +3495,7 @@ checksum = "b7c566e0f4b284dd6561c786d9cb0142da491f46a9fbed79ea69cdad5db17f21" dependencies = [ "anyhow", "heck", - "indexmap", + "indexmap 2.13.0", "prettyplease", "syn", "wasm-metadata", @@ -3382,7 +3526,7 @@ checksum = "9d66ea20e9553b30172b5e831994e35fbde2d165325bec84fc43dbf6f4eb9cb2" dependencies = [ "anyhow", "bitflags", - "indexmap", + "indexmap 2.13.0", "log", "serde", "serde_derive", @@ -3401,7 +3545,7 @@ checksum = "ecc8ac4bc1dc3381b7f59c34f00b67e18f910c2c0f50015669dde7def656a736" dependencies = [ "anyhow", "id-arena", - "indexmap", + "indexmap 2.13.0", "log", "semver", "serde", diff --git a/Cargo.toml b/Cargo.toml index 04bd4def5..fbc75b0f8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -63,7 +63,7 @@ encryption = ["dep:ring"] # (private) Generic support for cloud sync cloud = [] # (private) Support for Git based sync -git-sync = ["dep:hex"] +git-sync = ["dep:serde_with", "dep:glob"] # static bundling of dependencies bundled = ["rusqlite/bundled"] # use CA roots built into the library for all HTTPS access @@ -83,7 +83,7 @@ async-trait = "0.1.89" byteorder = "1.5" chrono = { version = "^0.4.44", features = ["serde"] } flate2 = "1" -hex = { version = "0.4", optional = true } +serde_with = { version = "3.18.0", features = ["base64"], optional = true } idb = { version = "0.6.4", optional = true } log = "^0.4.17" # Reqwest is "stuck" at 0.12 because that's what google-cloud-storage depends on. @@ -98,7 +98,7 @@ tokio = { version = "1", features = ["macros", "sync", "rt"] } thiserror = "2.0" uuid = { version = "^1.23.0", features = ["serde", "v4"] } url = { version = "2", optional = true } -glob = "0.3.3" +glob = { version = "0.3.3", optional = true } ## wasm-only dependencies. [target.'cfg(target_arch = "wasm32")'.dependencies] diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index 44d540549..a33e94a56 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -2,31 +2,51 @@ use crate::errors::Result; use crate::server::encryption::{Cryptor, Sealed, Unsealed}; use crate::server::{ AddVersionResult, GetVersionResult, HistorySegment, Server, Snapshot, SnapshotUrgency, - VersionId, NIL_VERSION_ID, + VersionId, }; use crate::Error; use async_trait::async_trait; use glob::glob; use log::info; use serde::{Deserialize, Serialize}; +use serde_with::{base64::Base64, serde_as}; use std::fs::{self, File}; -use std::io::Read; +use std::io::{BufReader, Read}; use std::path::{Path, PathBuf}; use std::process::Command; +use std::time::TryFromFloatSecsError; use uuid::Uuid; + +#[serde_as] #[derive(Serialize, Deserialize, Debug)] struct Version { + #[serde(with = "uuid::serde::simple")] version_id: VersionId, + #[serde(with = "uuid::serde::simple")] parent_version_id: VersionId, + #[serde_as(as = "Base64")] history_segment: HistorySegment, } +#[serde_as] +#[derive(Serialize, Deserialize, Debug)] +struct SnapshotFile { + #[serde(with = "uuid::serde::simple")] + version_id: VersionId, + #[serde_as(as = "Base64")] + payload: Vec, +} + /// The meta file holds the UUID of the most recent Version and the salt. +#[serde_as] #[derive(Serialize, Deserialize, Debug)] struct Meta { #[serde(with = "uuid::serde::simple")] - latest: VersionId, - salt: String, // hex-encoded + latest_version: VersionId, + #[serde(with = "uuid::serde::simple")] + latest_snapshot: VersionId, + #[serde_as(as = "Base64")] + salt: Vec, } pub(crate) struct GitSyncServer { @@ -60,9 +80,7 @@ impl GitSyncServer { encryption_secret: Vec, ) -> Result { let meta = Self::init_repo(&local_path, &branch, &remote, local_only)?; - let salt_bytes = hex::decode(&meta.salt) - .map_err(|e| Error::Server(format!("invalid salt in meta file: {e}")))?; - let cryptor = Cryptor::new(&salt_bytes, &encryption_secret.into())?; + let cryptor = Cryptor::new(&meta.salt, &encryption_secret.into())?; let server = GitSyncServer { meta, local_path, @@ -145,8 +163,9 @@ impl GitSyncServer { } Err(_) => { let m = Meta { - latest: Uuid::nil(), - salt: hex::encode(Cryptor::gen_salt()?), + latest_version: Uuid::nil(), + salt: Cryptor::gen_salt()?, + latest_snapshot: Uuid::nil(), }; let f = File::create_new(&meta_path)?; serde_json::to_writer(f, &m)?; @@ -177,10 +196,9 @@ impl GitSyncServer { /// Read the meta file from disk and update self.meta. fn read_meta(&mut self) -> Result<()> { let meta_path = self.local_path.join("meta"); - let mut file = File::open(&meta_path)?; - let mut contents = String::new(); - file.read_to_string(&mut contents)?; - self.meta = serde_json::from_str(&contents)?; + let file = File::open(&meta_path)?; + let reader = BufReader::new(file); + self.meta = serde_json::from_reader(reader)?; Ok(()) } @@ -283,12 +301,13 @@ impl Server for GitSyncServer { ) -> Result<(AddVersionResult, SnapshotUrgency)> { // Accept any parent when the repo is empty (latest == NIL). // Otherwise check if parent is in latest. If it isn't, pull recheck. - if self.meta.latest != Uuid::nil() && parent_version_id != self.meta.latest { + if self.meta.latest_version != Uuid::nil() && parent_version_id != self.meta.latest_version + { self.pull()?; self.read_meta()?; - if parent_version_id != self.meta.latest { + if parent_version_id != self.meta.latest_version { return Ok(( - AddVersionResult::ExpectedParentVersion(self.meta.latest), + AddVersionResult::ExpectedParentVersion(self.meta.latest_version), SnapshotUrgency::None, )); } @@ -302,7 +321,7 @@ impl Server for GitSyncServer { history_segment, }; let version_path = self.add_version_by_parent_version_id(&version)?; - self.meta.latest = version_id; + self.meta.latest_version = version_id; let meta_path = self.write_meta()?; // Commit and push, reverting if push fails. @@ -315,7 +334,7 @@ impl Server for GitSyncServer { self.pull()?; self.read_meta()?; return Ok(( - AddVersionResult::ExpectedParentVersion(self.meta.latest), + AddVersionResult::ExpectedParentVersion(self.meta.latest_version), SnapshotUrgency::None, )); } @@ -344,18 +363,59 @@ impl Server for GitSyncServer { } } - async fn add_snapshot(&mut self, _version_id: VersionId, _snapshot: Snapshot) -> Result<()> { - todo!(); + async fn add_snapshot(&mut self, version_id: VersionId, snapshot: Snapshot) -> Result<()> { + // Write the snapshot to a file. + let unsealed = Unsealed { + version_id: version_id, + payload: snapshot, + }; + let sealed = self.cryptor.seal(unsealed)?; + let snapshot_file = SnapshotFile { + version_id: version_id, + payload: Vec::::from(sealed), + }; + let snapshot_path = self.local_path.join("snapshot"); + let f = File::create(&snapshot_path)?; + serde_json::to_writer(f, &snapshot_file)?; + + // Update the meta so it contains the latest snapshot. + self.meta.latest_snapshot = version_id; + let meta_path = self.write_meta()?; + + // Commit and push, reverting if push fails. + self.stage_and_commit(&[&snapshot_path, &meta_path], "add snapshot")?; + + if !self.push()? { + // Push was rejected (non-fast-forward). Undo the commit and re-read remote state. + git_cmd(&self.local_path, &["reset", "HEAD~1", "--soft"])?; + std::fs::remove_file(&snapshot_path)?; + self.read_meta()?; + return Err(Error::Server("Couldn't push to remote.".into())); + } + Ok(()) } async fn get_snapshot(&mut self) -> Result> { - todo!(); + self.pull()?; + let snapshot_path = self.local_path.join("snapshot"); + if let Ok(file) = File::open(&snapshot_path) { + let reader = BufReader::new(file); + let s: SnapshotFile = serde_json::from_reader(reader)?; + let sealed = Sealed { + version_id: s.version_id, + payload: s.payload, + }; + let unsealed = self.cryptor.unseal(sealed)?; + return Ok(Some((unsealed.version_id, unsealed.payload))); + } else { + return Ok(None); + } } } #[cfg(test)] mod test { - use std::any::Any; + use crate::server::NIL_VERSION_ID; use super::*; use tempfile::TempDir; @@ -397,7 +457,7 @@ mod test { let tmp = TempDir::new()?; let server = make_server(tmp.path())?; assert!(tmp.path().join("meta").exists()); - assert_eq!(server.meta.latest, Uuid::nil()); + assert_eq!(server.meta.latest_version, Uuid::nil()); // eprintln!("tmp dir: {}", tmp.path().display()); // std::mem::forget(tmp); Ok(()) @@ -419,12 +479,12 @@ mod test { let mut server = make_server(tmp.path())?; let new_id = Uuid::new_v4(); - server.meta.latest = new_id; + server.meta.latest_version = new_id; server.write_meta()?; - server.meta.latest = Uuid::nil(); // clobber in-memory value + server.meta.latest_version = Uuid::nil(); // clobber in-memory value server.read_meta()?; - assert_eq!(server.meta.latest, new_id); + assert_eq!(server.meta.latest_version, new_id); Ok(()) } @@ -485,7 +545,7 @@ mod test { "version file missing: {filename}" ); // Verify meta was updated. - assert_eq!(server.meta.latest, version_id); + assert_eq!(server.meta.latest_version, version_id); } } Ok(()) @@ -504,7 +564,7 @@ mod test { panic!("should have accepted the version") } AddVersionResult::Ok(version_id) => { - assert_eq!(server.meta.latest, version_id); + assert_eq!(server.meta.latest_version, version_id); } } Ok(()) @@ -529,7 +589,7 @@ mod test { match server.add_version(parent_version_id, history).await?.0 { AddVersionResult::Ok(_) => panic!("should not have accepted the version"), AddVersionResult::ExpectedParentVersion(expected) => { - assert_eq!(expected, server.meta.latest); + assert_eq!(expected, server.meta.latest_version); } } Ok(()) @@ -596,7 +656,7 @@ mod test { let AddVersionResult::Ok(version_id) = rst else { panic!("Couldn't add version"); }; - + // Now we should be able to get the version. match server.get_child_version(parent_version_id).await? { GetVersionResult::Version { version_id: v_id, @@ -612,4 +672,102 @@ mod test { Ok(()) } + + #[tokio::test] + async fn test_get_child_version_from_remote() -> Result<()> { + let tmp = TempDir::new()?; + let bare = tmp.path().join("bare"); + let clone1 = tmp.path().join("clone1"); + let clone2 = tmp.path().join("clone2"); + + let mut server1 = make_server_with_remote(&bare, &clone1)?; + let bare_url = bare.to_str().unwrap(); + + // server1 adds a version and pushes it. + let (result, _) = server1 + .add_version(NIL_VERSION_ID, b"history".to_vec()) + .await?; + let AddVersionResult::Ok(version_id) = result else { + panic!("server1 add_version failed"); + }; + + let mut server2 = GitSyncServer::new( + clone2, + "main".into(), + bare_url.into(), + false, + b"test-secret".to_vec(), + )?; + + // get_child_version should pull and find the version. + match server2.get_child_version(NIL_VERSION_ID).await? { + GetVersionResult::Version { + version_id: v_id, + history_segment, + .. + } => { + assert_eq!(v_id, version_id); + assert_eq!(history_segment, b"history".to_vec()); + } + GetVersionResult::NoSuchVersion => panic!("should have found the version after pull"), + } + + Ok(()) + } + + #[tokio::test] + async fn test_snapshot_empty() -> Result<()> { + let tmp = TempDir::new()?; + let mut server = make_server(tmp.path())?; + assert_eq!(server.get_snapshot().await?, None); + Ok(()) + } + + #[tokio::test] + async fn test_snapshot_roundtrip() -> Result<()> { + let tmp = TempDir::new()?; + let mut server = make_server(tmp.path())?; + + let version_id = Uuid::new_v4(); + let data = b"my snapshot data".to_vec(); + server.add_snapshot(version_id, data.clone()).await?; + + let result = server.get_snapshot().await?; + assert!(result.is_some(), "expected a snapshot"); + let (got_id, got_data) = result.unwrap(); + assert_eq!(got_id, version_id); + assert_eq!(got_data, data); + Ok(()) + } + + #[tokio::test] + async fn test_snapshot_from_remote() -> Result<()> { + let tmp = TempDir::new()?; + let bare = tmp.path().join("bare"); + let clone1 = tmp.path().join("clone1"); + let clone2 = tmp.path().join("clone2"); + + let mut server1 = make_server_with_remote(&bare, &clone1)?; + let bare_url = bare.to_str().unwrap(); + + // server1 stores a snapshot and pushes it. + let version_id = Uuid::new_v4(); + let data = b"snapshot payload".to_vec(); + server1.add_snapshot(version_id, data.clone()).await?; + + // server2 should pull and find it. + let mut server2 = GitSyncServer::new( + clone2, + "main".into(), + bare_url.into(), + false, + b"test-secret".to_vec(), + )?; + let result = server2.get_snapshot().await?; + assert!(result.is_some(), "expected snapshot from remote"); + let (got_id, got_data) = result.unwrap(); + assert_eq!(got_id, version_id); + assert_eq!(got_data, data); + Ok(()) + } } From c104e0c184d58793a8680fcf0bf9660fedcc875a Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Sun, 12 Apr 2026 13:21:18 -0700 Subject: [PATCH 07/44] fix git cleanup and misc clippy issues --- Cargo.toml | 2 +- src/server/config.rs | 5 ++++- src/server/gitsync/mod.rs | 28 ++++++++++++++-------------- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index fbc75b0f8..c650a119a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -63,7 +63,7 @@ encryption = ["dep:ring"] # (private) Generic support for cloud sync cloud = [] # (private) Support for Git based sync -git-sync = ["dep:serde_with", "dep:glob"] +git-sync = ["dep:serde_with", "dep:glob", "encryption"] # static bundling of dependencies bundled = ["rusqlite/bundled"] # use CA roots built into the library for all HTTPS access diff --git a/src/server/config.rs b/src/server/config.rs index a54aa2163..ae5d08d7a 100644 --- a/src/server/config.rs +++ b/src/server/config.rs @@ -11,7 +11,9 @@ use crate::server::cloud::CloudServer; use crate::server::local::LocalServer; #[cfg(feature = "server-sync")] use crate::server::sync::SyncServer; -use crate::{errors::Result, server::gitsync::GitSyncServer}; +#[cfg(feature = "git-sync")] +use crate::server::gitsync::GitSyncServer; +use crate::errors::Result; #[cfg(feature = "server-local")] use std::path::PathBuf; #[cfg(feature = "server-sync")] @@ -182,6 +184,7 @@ impl ServerConfig { ) .await?, ), + #[cfg(feature = "git-sync")] ServerConfig::Git { local_path, branch, diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index a33e94a56..9ab47b43f 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -14,7 +14,6 @@ use std::fs::{self, File}; use std::io::{BufReader, Read}; use std::path::{Path, PathBuf}; use std::process::Command; -use std::time::TryFromFloatSecsError; use uuid::Uuid; #[serde_as] @@ -43,8 +42,6 @@ struct SnapshotFile { struct Meta { #[serde(with = "uuid::serde::simple")] latest_version: VersionId, - #[serde(with = "uuid::serde::simple")] - latest_snapshot: VersionId, #[serde_as(as = "Base64")] salt: Vec, } @@ -165,7 +162,6 @@ impl GitSyncServer { let m = Meta { latest_version: Uuid::nil(), salt: Cryptor::gen_salt()?, - latest_snapshot: Uuid::nil(), }; let f = File::create_new(&meta_path)?; serde_json::to_writer(f, &m)?; @@ -176,7 +172,10 @@ impl GitSyncServer { }; // Remove any untracked files left behind by interrupted writes. - git_cmd(local_path, &["clean", "-fd"])?; + git_cmd( + local_path, + &["clean", "-f", "--", "v-*", "snapshot", "meta"], + )?; Ok(meta) } @@ -217,9 +216,11 @@ impl GitSyncServer { } git_cmd(&self.local_path, &["fetch", &self.remote, &self.branch])?; git_cmd(&self.local_path, &["reset", "--hard", "FETCH_HEAD"])?; - // Remove any untracked files so orphaned version/snapshot files from - // interrupted writes cannot be served as valid versions. - git_cmd(&self.local_path, &["clean", "-fd"])?; + // Remove any untracked files left behind by interrupted writes. + git_cmd( + &self.local_path, + &["clean", "-f", "--", "v-*", "snapshot", "meta"], + ); Ok(()) } @@ -299,6 +300,7 @@ impl Server for GitSyncServer { parent_version_id: VersionId, history_segment: HistorySegment, ) -> Result<(AddVersionResult, SnapshotUrgency)> { + // TODO: Fix snapshot urgency. // Accept any parent when the repo is empty (latest == NIL). // Otherwise check if parent is in latest. If it isn't, pull recheck. if self.meta.latest_version != Uuid::nil() && parent_version_id != self.meta.latest_version @@ -364,24 +366,21 @@ impl Server for GitSyncServer { } async fn add_snapshot(&mut self, version_id: VersionId, snapshot: Snapshot) -> Result<()> { + self.pull()?; // Write the snapshot to a file. let unsealed = Unsealed { - version_id: version_id, + version_id, payload: snapshot, }; let sealed = self.cryptor.seal(unsealed)?; let snapshot_file = SnapshotFile { - version_id: version_id, + version_id, payload: Vec::::from(sealed), }; let snapshot_path = self.local_path.join("snapshot"); let f = File::create(&snapshot_path)?; serde_json::to_writer(f, &snapshot_file)?; - // Update the meta so it contains the latest snapshot. - self.meta.latest_snapshot = version_id; - let meta_path = self.write_meta()?; - // Commit and push, reverting if push fails. self.stage_and_commit(&[&snapshot_path, &meta_path], "add snapshot")?; @@ -389,6 +388,7 @@ impl Server for GitSyncServer { // Push was rejected (non-fast-forward). Undo the commit and re-read remote state. git_cmd(&self.local_path, &["reset", "HEAD~1", "--soft"])?; std::fs::remove_file(&snapshot_path)?; + self.pull()?; self.read_meta()?; return Err(Error::Server("Couldn't push to remote.".into())); } From ec2ae744e533c339b0e1357250b25df7282c1d15 Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Sun, 12 Apr 2026 17:50:10 -0700 Subject: [PATCH 08/44] fix/log some git issues from snapshot testing --- src/server/gitsync/mod.rs | 110 ++++++++++++++++++++++++++------------ 1 file changed, 76 insertions(+), 34 deletions(-) diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index 9ab47b43f..ac9f4c937 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -220,7 +220,7 @@ impl GitSyncServer { git_cmd( &self.local_path, &["clean", "-f", "--", "v-*", "snapshot", "meta"], - ); + )?; Ok(()) } @@ -230,12 +230,17 @@ impl GitSyncServer { if self.local_only { return Ok(true); } - let status = Command::new("git") + let output = Command::new("git") .args(["push", &self.remote, &self.branch]) .current_dir(&self.local_path) - .stderr(std::process::Stdio::null()) - .status()?; - Ok(status.success()) + .output()?; + if !output.status.success() { + log::debug!( + "git push failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + } + Ok(output.status.success()) } /// Encrypt and write a version file. Returns the file path. @@ -255,7 +260,7 @@ impl GitSyncServer { Ok(path) } - /// Read and decrypt a version file. Returns a Version if found, None if not. + /// Read and decrypt a version file. Returns a Version if found, None if not, Err on filesystem/encryption errors. fn get_version_by_parent_version_id(&self, version: &VersionId) -> Result> { // glob to find file. // v-PARENT-CHILD @@ -266,31 +271,65 @@ impl GitSyncServer { .ok_or_else(|| Error::Server("path is not valid UTF-8".into()))?, version.simple() ); + for entry in glob(&pattern).map_err(|e| Error::Server(format!("{:?}", e)))? { - let result = (|| -> Option { - let path = entry.ok()?; - let mut buf = Vec::new(); - File::open(&path).ok()?.read_to_end(&mut buf).ok()?; - let filename = path.to_str()?; - let (_, version_id_str) = filename.rsplit_once('-')?; - let version_id = Uuid::parse_str(version_id_str).ok()?; - let sealed = Sealed { - version_id, - payload: buf, - }; - let unsealed = self.cryptor.unseal(sealed).ok()?; - Some(Version { - version_id: unsealed.version_id, - parent_version_id: *version, - history_segment: unsealed.payload, - }) - })(); - if let Some(v) = result { - return Ok(Some(v)); - } + let path = match entry { + Ok(p) => p, + Err(e) => { + log::warn!("glob entry error: {e}"); + continue; + } + }; + let filename = match path.to_str() { + Some(f) => f, + None => { + log::warn!("non-UTF-8 path, skipping"); + continue; + } + }; + let (_, version_id_str) = match filename.rsplit_once('-') { + Some(parts) => parts, + None => { + log::warn!("unexpected filename format: {filename}"); + continue; + } + }; + let version_id = match Uuid::parse_str(version_id_str) { + Ok(id) => id, + Err(e) => { + log::warn!("bad version id in {filename}: {e}"); + continue; + } + }; + // Real errors past this point + let mut buf = Vec::new(); + File::open(&path)?.read_to_end(&mut buf)?; + let sealed = Sealed { + version_id, + payload: buf, + }; + let unsealed = self.cryptor.unseal(sealed)?; + return Ok(Some(Version { + version_id: unsealed.version_id, + parent_version_id: *version, + history_segment: unsealed.payload, + })); } Ok(None) } + + /// Determine snapshot urgency. + /// + /// In general, the more files there are, the slower many of the sync functions will be. + fn snapshot_urgency(&self) -> SnapshotUrgency { + let pattern = format!("{}/v-*", self.local_path.display()); + let count = glob(&pattern).map(|g| g.count()).unwrap_or(0); + match count { + 0..=10 => SnapshotUrgency::None, + 11..=50 => SnapshotUrgency::Low, + _ => SnapshotUrgency::High, + } + } } #[async_trait(?Send)] @@ -341,7 +380,7 @@ impl Server for GitSyncServer { )); } - Ok((AddVersionResult::Ok(version_id), SnapshotUrgency::None)) + Ok((AddVersionResult::Ok(version_id), self.snapshot_urgency())) } async fn get_child_version( @@ -366,7 +405,9 @@ impl Server for GitSyncServer { } async fn add_snapshot(&mut self, version_id: VersionId, snapshot: Snapshot) -> Result<()> { - self.pull()?; + if self.meta.latest_version != Uuid::nil() { + self.pull()?; + } // Write the snapshot to a file. let unsealed = Unsealed { version_id, @@ -382,16 +423,19 @@ impl Server for GitSyncServer { serde_json::to_writer(f, &snapshot_file)?; // Commit and push, reverting if push fails. - self.stage_and_commit(&[&snapshot_path, &meta_path], "add snapshot")?; + self.stage_and_commit(&[&snapshot_path, &snapshot_path], "add snapshot")?; if !self.push()? { - // Push was rejected (non-fast-forward). Undo the commit and re-read remote state. + // Push was rejected. git_cmd(&self.local_path, &["reset", "HEAD~1", "--soft"])?; std::fs::remove_file(&snapshot_path)?; self.pull()?; self.read_meta()?; return Err(Error::Server("Couldn't push to remote.".into())); } + + // TODO: Cleanup old files after a sucessful snapshot. + // Ok(()) } @@ -749,15 +793,13 @@ mod test { let mut server1 = make_server_with_remote(&bare, &clone1)?; let bare_url = bare.to_str().unwrap(); - // server1 stores a snapshot and pushes it. let version_id = Uuid::new_v4(); let data = b"snapshot payload".to_vec(); server1.add_snapshot(version_id, data.clone()).await?; - // server2 should pull and find it. let mut server2 = GitSyncServer::new( - clone2, + clone2.clone(), "main".into(), bare_url.into(), false, From 94fbbd4c78c5c93fd2688d75c886ccf0bbe0cd47 Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Mon, 13 Apr 2026 17:31:23 -0700 Subject: [PATCH 09/44] fix snapshot encryption --- src/server/gitsync/mod.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index ac9f4c937..faac4fc7d 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -441,18 +441,17 @@ impl Server for GitSyncServer { async fn get_snapshot(&mut self) -> Result> { self.pull()?; + let snapshot_path = self.local_path.join("snapshot"); if let Ok(file) = File::open(&snapshot_path) { - let reader = BufReader::new(file); - let s: SnapshotFile = serde_json::from_reader(reader)?; - let sealed = Sealed { + let s: SnapshotFile = serde_json::from_reader(BufReader::new(file))?; + let unsealed = self.cryptor.unseal(Sealed { version_id: s.version_id, payload: s.payload, - }; - let unsealed = self.cryptor.unseal(sealed)?; - return Ok(Some((unsealed.version_id, unsealed.payload))); + })?; + Ok(Some((unsealed.version_id, unsealed.payload))) } else { - return Ok(None); + Ok(None) } } } From 9b2818a9fa3db8f12abe2f5dfee206c46ec7afc8 Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Mon, 13 Apr 2026 20:07:10 -0700 Subject: [PATCH 10/44] add some documentation --- src/server/config.rs | 2 ++ src/server/gitsync/mod.rs | 69 ++++++++++++++++++++++++++++++--------- 2 files changed, 55 insertions(+), 16 deletions(-) diff --git a/src/server/config.rs b/src/server/config.rs index ae5d08d7a..844c8bb5d 100644 --- a/src/server/config.rs +++ b/src/server/config.rs @@ -16,6 +16,8 @@ use crate::server::gitsync::GitSyncServer; use crate::errors::Result; #[cfg(feature = "server-local")] use std::path::PathBuf; +#[cfg(all(feature = "git-sync", not(feature = "server-local")))] +use std::path::PathBuf; #[cfg(feature = "server-sync")] use uuid::Uuid; diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index faac4fc7d..24db34119 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -16,6 +16,7 @@ use std::path::{Path, PathBuf}; use std::process::Command; use uuid::Uuid; +/// A version record stored in a version file on disk. #[serde_as] #[derive(Serialize, Deserialize, Debug)] struct Version { @@ -27,6 +28,9 @@ struct Version { history_segment: HistorySegment, } +/// The snapshot record stored in the `snapshot` file on disk. +/// +/// It is overwritten when a newer snapshot is added #[serde_as] #[derive(Serialize, Deserialize, Debug)] struct SnapshotFile { @@ -36,7 +40,7 @@ struct SnapshotFile { payload: Vec, } -/// The meta file holds the UUID of the most recent Version and the salt. +/// Repository metadata. #[serde_as] #[derive(Serialize, Deserialize, Debug)] struct Meta { @@ -46,6 +50,12 @@ struct Meta { salt: Vec, } +/// A [`Server`] backed by a local git repository. +/// +/// +/// When `local_only` is `false` the server pushes to and pulls from `remote` on the `branch` +/// branch after each write. Conflict resolution is handled as: commit locally, +/// attempt push, and on rejection pull-and-reset before returning. pub(crate) struct GitSyncServer { meta: Meta, local_path: PathBuf, @@ -69,6 +79,12 @@ fn git_cmd(dir: &Path, args: &[&str]) -> Result<()> { } impl GitSyncServer { + /// Create (or re-open) a git-backed sync server. + /// + /// If `local_path` does not yet exist it is created. If it exists but is not a git + /// repository: + /// - In `local_only` mode a new repo is initialised there. + /// - In remote mode the repo is cloned from `remote`. pub(crate) fn new( local_path: PathBuf, branch: String, @@ -89,6 +105,10 @@ impl GitSyncServer { Ok(server) } + /// Initialise or open the git repository and return the current [`Meta`]. + /// + /// Creates the directory, initialises or clones the repo, switches to `branch`, and + /// creates the `meta` file on first run. fn init_repo(local_path: &Path, branch: &str, remote: &str, local_only: bool) -> Result { // Create the local directory if needed. fs::create_dir_all(local_path)?; @@ -101,6 +121,7 @@ impl GitSyncServer { .status .success(); + // Create one if not. if !is_repo { if local_only { info!("Creating new repo at {:?}", local_path); @@ -210,10 +231,28 @@ impl GitSyncServer { } /// Fetch and fast-forward to the remote branch. No-op in local-only mode. + /// If the remote branch does not yet exist (e.g. fresh bare repo), this is also a no-op. fn pull(&self) -> Result<()> { if self.local_only { return Ok(()); } + // Check whether the remote branch exists before fetching. A bare repo with no commits + // has no refs yet, and `git fetch origin ` would fail in that case. + let has_remote_branch = Command::new("git") + .args([ + "ls-remote", + "--exit-code", + "--heads", + &self.remote, + &self.branch, + ]) + .current_dir(&self.local_path) + .stderr(std::process::Stdio::null()) + .status()? + .success(); + if !has_remote_branch { + return Ok(()); + } git_cmd(&self.local_path, &["fetch", &self.remote, &self.branch])?; git_cmd(&self.local_path, &["reset", "--hard", "FETCH_HEAD"])?; // Remove any untracked files left behind by interrupted writes. @@ -243,7 +282,9 @@ impl GitSyncServer { Ok(output.status.success()) } - /// Encrypt and write a version file. Returns the file path. + /// Encrypt and write a version file named `v-{parent_version_id}-{version_id}`. + /// + /// Returns the path of the newly written file. fn add_version_by_parent_version_id(&self, version: &Version) -> Result { let unsealed = Unsealed { version_id: version.version_id, @@ -260,7 +301,7 @@ impl GitSyncServer { Ok(path) } - /// Read and decrypt a version file. Returns a Version if found, None if not, Err on filesystem/encryption errors. + /// Find, read, and decrypt the version file whose parent matches `version`. fn get_version_by_parent_version_id(&self, version: &VersionId) -> Result> { // glob to find file. // v-PARENT-CHILD @@ -318,9 +359,10 @@ impl GitSyncServer { Ok(None) } - /// Determine snapshot urgency. + /// Return the appropriate [`SnapshotUrgency`] based on the number of uncommitted version files. /// - /// In general, the more files there are, the slower many of the sync functions will be. + /// More version files means more work for [`get_child_version`] callers walking the chain, + /// so urgency rises with the file count. fn snapshot_urgency(&self) -> SnapshotUrgency { let pattern = format!("{}/v-*", self.local_path.display()); let count = glob(&pattern).map(|g| g.count()).unwrap_or(0); @@ -339,7 +381,6 @@ impl Server for GitSyncServer { parent_version_id: VersionId, history_segment: HistorySegment, ) -> Result<(AddVersionResult, SnapshotUrgency)> { - // TODO: Fix snapshot urgency. // Accept any parent when the repo is empty (latest == NIL). // Otherwise check if parent is in latest. If it isn't, pull recheck. if self.meta.latest_version != Uuid::nil() && parent_version_id != self.meta.latest_version @@ -405,9 +446,7 @@ impl Server for GitSyncServer { } async fn add_snapshot(&mut self, version_id: VersionId, snapshot: Snapshot) -> Result<()> { - if self.meta.latest_version != Uuid::nil() { - self.pull()?; - } + self.pull()?; // Write the snapshot to a file. let unsealed = Unsealed { version_id, @@ -423,7 +462,7 @@ impl Server for GitSyncServer { serde_json::to_writer(f, &snapshot_file)?; // Commit and push, reverting if push fails. - self.stage_and_commit(&[&snapshot_path, &snapshot_path], "add snapshot")?; + self.stage_and_commit(&[&snapshot_path], "add snapshot")?; if !self.push()? { // Push was rejected. @@ -434,8 +473,10 @@ impl Server for GitSyncServer { return Err(Error::Server("Couldn't push to remote.".into())); } - // TODO: Cleanup old files after a sucessful snapshot. - // + // TODO: After a successful snapshot, delete all superseded version files (v-* files + // whose child version ID predates the snapshot version). The cloud server does this + // in its cleanup() method. Without cleanup, version files accumulate indefinitely + // and snapshot_urgency() will keep returning High even after a snapshot is stored. Ok(()) } @@ -501,8 +542,6 @@ mod test { let server = make_server(tmp.path())?; assert!(tmp.path().join("meta").exists()); assert_eq!(server.meta.latest_version, Uuid::nil()); - // eprintln!("tmp dir: {}", tmp.path().display()); - // std::mem::forget(tmp); Ok(()) } @@ -566,8 +605,6 @@ mod test { )?; server2.pull()?; assert!(clone2.join("testfile").exists()); - // eprintln!("tmp dir: {}", tmp.path().display()); - // std::mem::forget(tmp); Ok(()) } From d40262f1cab3601e88f57641b709bb755a9f9bc1 Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Tue, 14 Apr 2026 09:04:51 -0700 Subject: [PATCH 11/44] Add more snapshot tests --- src/server/gitsync/mod.rs | 109 +++++++++++++++++++++++++++++++++++++- 1 file changed, 107 insertions(+), 2 deletions(-) diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index 24db34119..539e751f0 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -366,9 +366,10 @@ impl GitSyncServer { fn snapshot_urgency(&self) -> SnapshotUrgency { let pattern = format!("{}/v-*", self.local_path.display()); let count = glob(&pattern).map(|g| g.count()).unwrap_or(0); + // For reviewers: Is this reasonable? match count { - 0..=10 => SnapshotUrgency::None, - 11..=50 => SnapshotUrgency::Low, + 0..=50 => SnapshotUrgency::None, + 51..=100 => SnapshotUrgency::Low, _ => SnapshotUrgency::High, } } @@ -848,4 +849,108 @@ mod test { assert_eq!(got_data, data); Ok(()) } + + /// A second `add_snapshot` should overwrite the first: only the latest snapshot is + /// returned by `get_snapshot`. + #[tokio::test] + async fn test_snapshot_overwrite() -> Result<()> { + let tmp = TempDir::new()?; + let mut server = make_server(tmp.path())?; + + let v1 = Uuid::new_v4(); + server.add_snapshot(v1, b"first snapshot".to_vec()).await?; + + let v2 = Uuid::new_v4(); + server.add_snapshot(v2, b"second snapshot".to_vec()).await?; + + let result = server.get_snapshot().await?; + assert!(result.is_some()); + let (got_id, got_data) = result.unwrap(); + assert_eq!(got_id, v2, "expected the second snapshot's version_id"); + assert_eq!(got_data, b"second snapshot".to_vec()); + Ok(()) + } + + /// `add_snapshot` push-failure rollback: install a pre-receive hook on the bare repo that + /// rejects all pushes (while still allowing fetches), forcing a push rejection. + /// After the Err, the working tree must be clean and the snapshot file must be absent. + #[tokio::test] + async fn test_snapshot_push_rejected_rollback() -> Result<()> { + let tmp = TempDir::new()?; + let bare = tmp.path().join("bare"); + let clone1 = tmp.path().join("clone1"); + + let mut server = make_server_with_remote(&bare, &clone1)?; + + // Add a version so the remote branch exists (required for pull's ls-remote check). + server.add_version(NIL_VERSION_ID, b"v1".to_vec()).await?; + + // Install a pre-receive hook that rejects all pushes. + // Fetches still work because hooks only run on the receive side. + let hooks_dir = bare.join("hooks"); + fs::create_dir_all(&hooks_dir)?; + let hook = hooks_dir.join("pre-receive"); + std::fs::write(&hook, b"#!/bin/sh\nexit 1\n")?; + // Make the hook executable. + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&hook, std::fs::Permissions::from_mode(0o755))?; + } + + // add_snapshot should pull successfully then fail to push (hook rejects it). + let v1 = Uuid::new_v4(); + let result = server.add_snapshot(v1, b"snap".to_vec()).await; + assert!( + result.is_err(), + "expected Err when push is rejected by hook" + ); + + // The snapshot file must not remain in the working tree. + assert!( + !server.local_path.join("snapshot").exists(), + "snapshot file should be removed after rollback" + ); + + // The git index must be clean (no staged changes left over). + let status = Command::new("git") + .args(["status", "--porcelain"]) + .current_dir(&server.local_path) + .output()?; + assert!( + status.stdout.is_empty(), + "git index should be clean after rollback, got: {}", + String::from_utf8_lossy(&status.stdout) + ); + + Ok(()) + } + + /// A corrupted version file should return `Err`. + #[tokio::test] + async fn test_get_child_version_corrupted_file() -> Result<()> { + let tmp = TempDir::new()?; + let mut server = make_server(tmp.path())?; + + // Add a real version so we know the parent UUID. + let parent = Uuid::new_v4(); + let (result, _) = server.add_version(parent, b"good data".to_vec()).await?; + let AddVersionResult::Ok(child) = result else { + panic!("add_version failed"); + }; + + // Overwrite the version file with garbage to simulate corruption. + let filename = format!("v-{}-{}", parent.simple(), child.simple()); + std::fs::write(tmp.path().join(&filename), b"this is not valid ciphertext")?; + + // get_child_version should return Err (decryption failure), not NoSuchVersion. + let result = server.get_child_version(parent).await; + assert!( + result.is_err(), + "expected Err on decryption failure, got: {:?}", + result + ); + + Ok(()) + } } From e60303054cd30af646da0a75d58ec77dcf28abeb Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Fri, 17 Apr 2026 09:10:57 -0700 Subject: [PATCH 12/44] add cleanup after snapshot --- src/server/gitsync/mod.rs | 182 ++++++++++++++++++++++++++++++++------ 1 file changed, 154 insertions(+), 28 deletions(-) diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index 539e751f0..990f6d81d 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -1,3 +1,4 @@ +//! TODO: Add overall documentation use crate::errors::Result; use crate::server::encryption::{Cryptor, Sealed, Unsealed}; use crate::server::{ @@ -8,6 +9,7 @@ use crate::Error; use async_trait::async_trait; use glob::glob; use log::info; +use std::collections::{HashMap, HashSet}; use serde::{Deserialize, Serialize}; use serde_with::{base64::Base64, serde_as}; use std::fs::{self, File}; @@ -79,7 +81,7 @@ fn git_cmd(dir: &Path, args: &[&str]) -> Result<()> { } impl GitSyncServer { - /// Create (or re-open) a git-backed sync server. + /// Create or re-open a git-backed sync server. /// /// If `local_path` does not yet exist it is created. If it exists but is not a git /// repository: @@ -361,18 +363,136 @@ impl GitSyncServer { /// Return the appropriate [`SnapshotUrgency`] based on the number of uncommitted version files. /// - /// More version files means more work for [`get_child_version`] callers walking the chain, - /// so urgency rises with the file count. + /// Returns `None` if a snapshot already exists (cleanup will have removed covered files, + /// so a non-zero count reflects only post-snapshot versions). Otherwise urgency rises + /// with the file count. fn snapshot_urgency(&self) -> SnapshotUrgency { + // If a snapshot already exists, cleanup has (or will) remove covered version files. + // Don't request another snapshot until enough new versions accumulate. + if self.local_path.join("snapshot").exists() { + let pattern = format!("{}/v-*", self.local_path.display()); + let count = glob(&pattern).map(|g| g.count()).unwrap_or(0); + return match count { + 0..=50 => SnapshotUrgency::None, + 51..=100 => SnapshotUrgency::Low, + _ => SnapshotUrgency::High, + }; + } let pattern = format!("{}/v-*", self.local_path.display()); let count = glob(&pattern).map(|g| g.count()).unwrap_or(0); - // For reviewers: Is this reasonable? + // TODO: Performance test get_version_by_parent_version_id to help determine + // what a reasonable number is. match count { 0..=50 => SnapshotUrgency::None, 51..=100 => SnapshotUrgency::Low, _ => SnapshotUrgency::High, } } + + /// Cleans up the repository by removing version files covered by the current snapshot. + /// + /// Reads the snapshot to determine which version it covers, then walks backward + /// through the version chain from that version. All version files on that chain + /// (i.e. all history that is now redundant given the snapshot) are deleted, + /// committed, and pushed. If the push is rejected, we reset to the remote state + /// and will retry on the next snapshot. + /// + /// This is a best-effort operation: if called with no snapshot present it is a no-op. + fn cleanup(&self) -> Result<()> { + // Read snapshot metadata. The version_id is stored unencrypted in the JSON wrapper, + // so no decryption is needed here. + let snapshot_path = self.local_path.join("snapshot"); + let snapshot_version: Uuid = if let Ok(file) = File::open(&snapshot_path) { + let s: SnapshotFile = serde_json::from_reader(BufReader::new(file))?; + s.version_id + } else { + return Ok(()); + }; + + // Parse all v-* filenames into a child → (parent, path) map. + let pattern = format!( + "{}/v-*", + self.local_path + .to_str() + .ok_or_else(|| Error::Server("repo path is not valid UTF-8".into()))? + ); + let mut versions: HashMap = HashMap::new(); + for entry in glob(&pattern).map_err(|e| Error::Server(format!("{e:?}")))? { + let path = match entry { + Ok(p) => p, + Err(e) => { + log::warn!("cleanup: glob error: {e}"); + continue; + } + }; + let Some(filename) = path.file_name().and_then(|f| f.to_str()) else { + continue; + }; + // Filename format: v-{parent_simple}-{child_simple} + let Some(stem) = filename.strip_prefix("v-") else { + continue; + }; + let Some((parent_str, child_str)) = stem.split_once('-') else { + continue; + }; + let (Ok(parent_id), Ok(child_id)) = + (Uuid::parse_str(parent_str), Uuid::parse_str(child_str)) + else { + continue; + }; + versions.insert(child_id, (parent_id, path)); + } + + if versions.is_empty() { + return Ok(()); + } + + // Walk the chain backward from snapshot_version to find all versions whose + // history is now captured by the snapshot. + let mut covered: HashSet = HashSet::new(); + let mut current = snapshot_version; + for _ in 0..=versions.len() { + if !covered.insert(current) { + log::warn!("cleanup: version cycle detected, aborting"); + return Ok(()); + } + match versions.get(¤t) { + Some((parent, _)) => current = *parent, + None => break, + } + } + + // Remove each covered version file from the git index and working tree. + let mut any_removed = false; + for (child_id, (_, path)) in &versions { + if covered.contains(child_id) { + let name = path + .file_name() + .and_then(|n| n.to_str()) + .ok_or_else(|| Error::Server("version file path is not valid UTF-8".into()))?; + git_cmd(&self.local_path, &["rm", name])?; + any_removed = true; + } + } + + if !any_removed { + return Ok(()); + } + + git_cmd( + &self.local_path, + &["commit", "-m", "cleanup: remove version files covered by snapshot"], + )?; + + // Best-effort push. A rejected push just means another replica will clean up + // later (or we will on the next snapshot). Reset to remote state so we are in sync. + if !self.push()? { + log::info!("cleanup: push rejected, resetting to remote state"); + self.pull()?; + } + + Ok(()) + } } #[async_trait(?Send)] @@ -474,10 +594,8 @@ impl Server for GitSyncServer { return Err(Error::Server("Couldn't push to remote.".into())); } - // TODO: After a successful snapshot, delete all superseded version files (v-* files - // whose child version ID predates the snapshot version). The cloud server does this - // in its cleanup() method. Without cleanup, version files accumulate indefinitely - // and snapshot_urgency() will keep returning High even after a snapshot is stored. + // Cleanup: delete all version files covered by this snapshot, then commit and push. + self.cleanup()?; Ok(()) } @@ -926,31 +1044,39 @@ mod test { Ok(()) } - /// A corrupted version file should return `Err`. + /// After `add_snapshot`, all version files covered by the snapshot should be deleted + /// (cleanup is called automatically by `add_snapshot`). #[tokio::test] - async fn test_get_child_version_corrupted_file() -> Result<()> { + async fn test_cleanup_removes_version_files() -> Result<()> { let tmp = TempDir::new()?; let mut server = make_server(tmp.path())?; - // Add a real version so we know the parent UUID. - let parent = Uuid::new_v4(); - let (result, _) = server.add_version(parent, b"good data".to_vec()).await?; - let AddVersionResult::Ok(child) = result else { - panic!("add_version failed"); + // Add a chain of three versions. + let (r1, _) = server.add_version(NIL_VERSION_ID, b"v1".to_vec()).await?; + let AddVersionResult::Ok(v1) = r1 else { + panic!("add_version 1 failed"); }; - - // Overwrite the version file with garbage to simulate corruption. - let filename = format!("v-{}-{}", parent.simple(), child.simple()); - std::fs::write(tmp.path().join(&filename), b"this is not valid ciphertext")?; - - // get_child_version should return Err (decryption failure), not NoSuchVersion. - let result = server.get_child_version(parent).await; - assert!( - result.is_err(), - "expected Err on decryption failure, got: {:?}", - result - ); - + let (r2, _) = server.add_version(v1, b"v2".to_vec()).await?; + let AddVersionResult::Ok(v2) = r2 else { + panic!("add_version 2 failed"); + }; + let (r3, _) = server.add_version(v2, b"v3".to_vec()).await?; + let AddVersionResult::Ok(v3) = r3 else { + panic!("add_version 3 failed"); + }; + // Confirm they exist. + assert!(tmp.path().join(format!("v-{}-{}", NIL_VERSION_ID.simple(), v1.simple())).exists()); + assert!(tmp.path().join(format!("v-{}-{}", v1.simple(), v2.simple())).exists()); + assert!(tmp.path().join(format!("v-{}-{}", v2.simple(), v3.simple())).exists()); + + // Snapshot at v3; cleanup should remove all three version files. + server.add_snapshot(v3, b"full state".to_vec()).await?; + + assert!(!tmp.path().join(format!("v-{}-{}", NIL_VERSION_ID.simple(), v1.simple())).exists(), "v1 file should be gone"); + assert!(!tmp.path().join(format!("v-{}-{}", v1.simple(), v2.simple())).exists(), "v2 file should be gone"); + assert!(!tmp.path().join(format!("v-{}-{}", v2.simple(), v3.simple())).exists(), "v3 file should be gone"); + // The snapshot file itself must still exist. + assert!(tmp.path().join("snapshot").exists(), "snapshot file should remain"); Ok(()) } } From 7fc7ecf0c97691eb4aff9d42f050e2d66b1a1eeb Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Fri, 17 Apr 2026 09:16:08 -0700 Subject: [PATCH 13/44] add documentation --- src/server/gitsync/mod.rs | 75 ++++++++++++++++++++++++++++++++------- 1 file changed, 63 insertions(+), 12 deletions(-) diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index 990f6d81d..aed856d79 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -1,4 +1,25 @@ -//! TODO: Add overall documentation +//! Git-backed sync server for TaskChampion. +//! +//! [`GitSyncServer`] implements the [`Server`] trait using a local git repository as its +//! backing store, with optional push/pull to a remote (e.g. a bare repo on a shared +//! filesystem or a remote accessed via SSH). +//! +//! Each sync operation is represented as a commit on a single branch: +//! +//! - **Versions** are stored as files named `v-{parent_uuid}-{child_uuid}`, containing +//! encrypted [`HistorySegment`] bytes. +//! - **Snapshots** are stored as a single file named `snapshot`, containing a JSON wrapper +//! around an encrypted full-state blob. +//! - **Metadata** (`meta`) holds the latest version UUID and the encryption salt as JSON. +//! +//! After each write (`add_version`, `add_snapshot`) the server stages the changed files, +//! creates a commit, and pushes to the remote. If the push is rejected , thecommit is +//! rolled back and the caller receives an [`AddVersionResult::ExpectedParentVersion`] +//! or an [`Error`] so it can retry. +//! +//! After a snapshot is stored, [`GitSyncServer::cleanup`] automatically removes all +//! version files whose history is now captured by the snapshot, keeping the repository +//! compact. use crate::errors::Result; use crate::server::encryption::{Cryptor, Sealed, Unsealed}; use crate::server::{ @@ -9,9 +30,9 @@ use crate::Error; use async_trait::async_trait; use glob::glob; use log::info; -use std::collections::{HashMap, HashSet}; use serde::{Deserialize, Serialize}; use serde_with::{base64::Base64, serde_as}; +use std::collections::{HashMap, HashSet}; use std::fs::{self, File}; use std::io::{BufReader, Read}; use std::path::{Path, PathBuf}; @@ -380,8 +401,7 @@ impl GitSyncServer { } let pattern = format!("{}/v-*", self.local_path.display()); let count = glob(&pattern).map(|g| g.count()).unwrap_or(0); - // TODO: Performance test get_version_by_parent_version_id to help determine - // what a reasonable number is. + // For reviewers: does this seem reasonable? match count { 0..=50 => SnapshotUrgency::None, 51..=100 => SnapshotUrgency::Low, @@ -481,7 +501,11 @@ impl GitSyncServer { git_cmd( &self.local_path, - &["commit", "-m", "cleanup: remove version files covered by snapshot"], + &[ + "commit", + "-m", + "cleanup: remove version files covered by snapshot", + ], )?; // Best-effort push. A rejected push just means another replica will clean up @@ -1065,18 +1089,45 @@ mod test { panic!("add_version 3 failed"); }; // Confirm they exist. - assert!(tmp.path().join(format!("v-{}-{}", NIL_VERSION_ID.simple(), v1.simple())).exists()); - assert!(tmp.path().join(format!("v-{}-{}", v1.simple(), v2.simple())).exists()); - assert!(tmp.path().join(format!("v-{}-{}", v2.simple(), v3.simple())).exists()); + assert!(tmp + .path() + .join(format!("v-{}-{}", NIL_VERSION_ID.simple(), v1.simple())) + .exists()); + assert!(tmp + .path() + .join(format!("v-{}-{}", v1.simple(), v2.simple())) + .exists()); + assert!(tmp + .path() + .join(format!("v-{}-{}", v2.simple(), v3.simple())) + .exists()); // Snapshot at v3; cleanup should remove all three version files. server.add_snapshot(v3, b"full state".to_vec()).await?; - assert!(!tmp.path().join(format!("v-{}-{}", NIL_VERSION_ID.simple(), v1.simple())).exists(), "v1 file should be gone"); - assert!(!tmp.path().join(format!("v-{}-{}", v1.simple(), v2.simple())).exists(), "v2 file should be gone"); - assert!(!tmp.path().join(format!("v-{}-{}", v2.simple(), v3.simple())).exists(), "v3 file should be gone"); + assert!( + !tmp.path() + .join(format!("v-{}-{}", NIL_VERSION_ID.simple(), v1.simple())) + .exists(), + "v1 file should be gone" + ); + assert!( + !tmp.path() + .join(format!("v-{}-{}", v1.simple(), v2.simple())) + .exists(), + "v2 file should be gone" + ); + assert!( + !tmp.path() + .join(format!("v-{}-{}", v2.simple(), v3.simple())) + .exists(), + "v3 file should be gone" + ); // The snapshot file itself must still exist. - assert!(tmp.path().join("snapshot").exists(), "snapshot file should remain"); + assert!( + tmp.path().join("snapshot").exists(), + "snapshot file should remain" + ); Ok(()) } } From fd09e02527de8c58499acb03869066a5008d2f59 Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Fri, 17 Apr 2026 20:43:53 -0700 Subject: [PATCH 14/44] cleanup of snapshot code, add documentation --- src/server/gitsync/mod.rs | 182 ++++++++++++++++++++++---------------- 1 file changed, 107 insertions(+), 75 deletions(-) diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index aed856d79..5362a88bb 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -1,16 +1,13 @@ //! Git-backed sync server for TaskChampion. //! //! [`GitSyncServer`] implements the [`Server`] trait using a local git repository as its -//! backing store, with optional push/pull to a remote (e.g. a bare repo on a shared -//! filesystem or a remote accessed via SSH). +//! backing store, with optional push/pull to a remote. //! -//! Each sync operation is represented as a commit on a single branch: -//! -//! - **Versions** are stored as files named `v-{parent_uuid}-{child_uuid}`, containing +//! - Versions are stored as files named `v-{parent_uuid}-{child_uuid}`, containing //! encrypted [`HistorySegment`] bytes. -//! - **Snapshots** are stored as a single file named `snapshot`, containing a JSON wrapper +//! - Snapshots are stored as a single file named `snapshot`, containing a JSON wrapper //! around an encrypted full-state blob. -//! - **Metadata** (`meta`) holds the latest version UUID and the encryption salt as JSON. +//! - Metadata (`meta`) holds the latest version UUID and the encryption salt as JSON. //! //! After each write (`add_version`, `add_snapshot`) the server stages the changed files, //! creates a commit, and pushes to the remote. If the push is rejected , thecommit is @@ -20,6 +17,27 @@ //! After a snapshot is stored, [`GitSyncServer::cleanup`] automatically removes all //! version files whose history is now captured by the snapshot, keeping the repository //! compact. +//! +//! Notes and Expectations +//! +//! - Since this shells out to git, it assumes that you havea reasonably functional git +//! setup. I.e. 'git init', 'git add', 'git commit', etc shoud just work. +//! - If you are using a remote, 'git push' and 'git pull' shoud work. +//! - Due to the nature of the version and snapshot history, you probably shouldn't do +//! a lot of the things you normally would with a git repo, like merge, squash, etc. +//! Just let TaskChampion manage it. +//! - If you are planning on using it for other things, it is HIGHLY recommended that you +//! create a 'task' branch and let TaskChampion manage that branch. +//! - This does support both defining a remote and having `local_only` mode set at the same +//! time. The idea is that maybe the remote isn't ready yet, or eithe rtemporarily or +//! permanantly down. Either way, you can use this in local mode in the mean time. +//! - Remember, a remote can be on the same machine as local. This is used for testing. +//! +//! Notes for Reviewers +//! +//! - I haven't done any performance testing, but it seems reasonably quick for manual use. +//! - Currently is uses the same salt for all files. This isn't great security practice, +//! but does seem to be what the other servers are doing. use crate::errors::Result; use crate::server::encryption::{Cryptor, Sealed, Unsealed}; use crate::server::{ @@ -324,8 +342,11 @@ impl GitSyncServer { Ok(path) } - /// Find, read, and decrypt the version file whose parent matches `version`. - fn get_version_by_parent_version_id(&self, version: &VersionId) -> Result> { + /// Find, read, and decrypt the version file whose parent matches `parent_version_id`. + fn get_version_by_parent_version_id( + &self, + parent_version_id: &VersionId, + ) -> Result> { // glob to find file. // v-PARENT-CHILD let pattern = format!( @@ -333,7 +354,7 @@ impl GitSyncServer { self.local_path .to_str() .ok_or_else(|| Error::Server("path is not valid UTF-8".into()))?, - version.simple() + parent_version_id.simple() ); for entry in glob(&pattern).map_err(|e| Error::Server(format!("{:?}", e)))? { @@ -375,33 +396,23 @@ impl GitSyncServer { let unsealed = self.cryptor.unseal(sealed)?; return Ok(Some(Version { version_id: unsealed.version_id, - parent_version_id: *version, + parent_version_id: *parent_version_id, history_segment: unsealed.payload, })); } Ok(None) } - /// Return the appropriate [`SnapshotUrgency`] based on the number of uncommitted version files. + /// Return the [`SnapshotUrgency`] based on the number of version files present. /// - /// Returns `None` if a snapshot already exists (cleanup will have removed covered files, - /// so a non-zero count reflects only post-snapshot versions). Otherwise urgency rises - /// with the file count. + /// Since cleanup runs after every successful add_snapshot and removes + /// all version files covered by the snapshot, the count here reflects only post-snapshot + /// versions. fn snapshot_urgency(&self) -> SnapshotUrgency { - // If a snapshot already exists, cleanup has (or will) remove covered version files. - // Don't request another snapshot until enough new versions accumulate. - if self.local_path.join("snapshot").exists() { - let pattern = format!("{}/v-*", self.local_path.display()); - let count = glob(&pattern).map(|g| g.count()).unwrap_or(0); - return match count { - 0..=50 => SnapshotUrgency::None, - 51..=100 => SnapshotUrgency::Low, - _ => SnapshotUrgency::High, - }; - } let pattern = format!("{}/v-*", self.local_path.display()); let count = glob(&pattern).map(|g| g.count()).unwrap_or(0); - // For reviewers: does this seem reasonable? + // TODO: Performance test get_version_by_parent_version_id to help determine + // what reasonable thresholds are. match count { 0..=50 => SnapshotUrgency::None, 51..=100 => SnapshotUrgency::Low, @@ -413,11 +424,10 @@ impl GitSyncServer { /// /// Reads the snapshot to determine which version it covers, then walks backward /// through the version chain from that version. All version files on that chain - /// (i.e. all history that is now redundant given the snapshot) are deleted, - /// committed, and pushed. If the push is rejected, we reset to the remote state + /// are deleted, committed, and pushed. If the push is rejected, we reset to the remote state /// and will retry on the next snapshot. /// - /// This is a best-effort operation: if called with no snapshot present it is a no-op. + /// If called with no snapshot present it is a no-op. fn cleanup(&self) -> Result<()> { // Read snapshot metadata. The version_id is stored unencrypted in the JSON wrapper, // so no decryption is needed here. @@ -429,7 +439,7 @@ impl GitSyncServer { return Ok(()); }; - // Parse all v-* filenames into a child → (parent, path) map. + // Parse all v-* filenames into a child -> (parent, path) map. let pattern = format!( "{}/v-*", self.local_path @@ -483,16 +493,24 @@ impl GitSyncServer { } // Remove each covered version file from the git index and working tree. + // If any `git rm` fails partway through we `git reset HEAD` to unstage, leaving + // the working tree dirty but the index clean for subsequent operations. let mut any_removed = false; - for (child_id, (_, path)) in &versions { - if covered.contains(child_id) { - let name = path - .file_name() - .and_then(|n| n.to_str()) - .ok_or_else(|| Error::Server("version file path is not valid UTF-8".into()))?; - git_cmd(&self.local_path, &["rm", name])?; - any_removed = true; + let rm_result: Result<()> = (|| { + for (child_id, (_, path)) in &versions { + if covered.contains(child_id) { + let name = path.file_name().and_then(|n| n.to_str()).ok_or_else(|| { + Error::Server("version file path is not valid UTF-8".into()) + })?; + git_cmd(&self.local_path, &["rm", name])?; + any_removed = true; + } } + Ok(()) + })(); + if let Err(e) = rm_result { + let _ = git_cmd(&self.local_path, &["reset", "HEAD"]); + return Err(e); } if !any_removed { @@ -508,8 +526,8 @@ impl GitSyncServer { ], )?; - // Best-effort push. A rejected push just means another replica will clean up - // later (or we will on the next snapshot). Reset to remote state so we are in sync. + // A rejected push means another replica will clean up later, or we will + // on the next snapshot. Reset to remote state so we are in sync. if !self.push()? { log::info!("cleanup: push rejected, resetting to remote state"); self.pull()?; @@ -555,7 +573,7 @@ impl Server for GitSyncServer { self.stage_and_commit(&[&version_path, &meta_path], "add version")?; if !self.push()? { - // Push was rejected (non-fast-forward). Undo the commit and re-read remote state. + // Push was rejected, reset and re-read state. git_cmd(&self.local_path, &["reset", "HEAD~1", "--soft"])?; std::fs::remove_file(&version_path)?; self.pull()?; @@ -593,6 +611,10 @@ impl Server for GitSyncServer { async fn add_snapshot(&mut self, version_id: VersionId, snapshot: Snapshot) -> Result<()> { self.pull()?; // Write the snapshot to a file. + // Note: If another replica has pushed a snapshot for a + // later version in the chain between our pull and our push, we will overwrite it. + // This should be harmless: a replica with newer state will overwrite again, + // and push rejection handles concurrent writes. let unsealed = Unsealed { version_id, payload: snapshot, @@ -610,16 +632,18 @@ impl Server for GitSyncServer { self.stage_and_commit(&[&snapshot_path], "add snapshot")?; if !self.push()? { - // Push was rejected. + // Push was rejected. Undo the commit, pull and clean to restore state. git_cmd(&self.local_path, &["reset", "HEAD~1", "--soft"])?; - std::fs::remove_file(&snapshot_path)?; self.pull()?; self.read_meta()?; return Err(Error::Server("Couldn't push to remote.".into())); } - // Cleanup: delete all version files covered by this snapshot, then commit and push. - self.cleanup()?; + // Cleanup is best-effort: the snapshot is already safely stored on the remote. + // A cleanup failure just means version files will linger until the next successful snapshot. + if let Err(e) = self.cleanup() { + log::warn!("snapshot stored but cleanup failed: {e}"); + } Ok(()) } @@ -774,25 +798,6 @@ mod test { Ok(()) } - #[tokio::test] - async fn test_add_nonzero_base() -> Result<()> { - let tmp = TempDir::new()?; - let mut server = make_server(tmp.path())?; - let history = b"1234".to_vec(); - let parent_version_id = Uuid::new_v4(); - - // OK because latest == NIL (repo is empty). - match server.add_version(parent_version_id, history).await?.0 { - AddVersionResult::ExpectedParentVersion(_) => { - panic!("should have accepted the version") - } - AddVersionResult::Ok(version_id) => { - assert_eq!(server.meta.latest_version, version_id); - } - } - Ok(()) - } - #[tokio::test] async fn test_add_nonzero_base_forbidden() -> Result<()> { let tmp = TempDir::new()?; @@ -859,27 +864,26 @@ mod test { } #[tokio::test] - async fn get_child_version() -> Result<()> { + async fn test_get_child_version_local() -> Result<()> { let tmp = TempDir::new()?; let mut server = make_server(tmp.path())?; - let history = b"1234".to_vec(); let parent_version_id = Uuid::new_v4(); - // Version doesn't exist yet + // With no versions written, lookup returns NoSuchVersion. assert_eq!( server.get_child_version(parent_version_id).await?, GetVersionResult::NoSuchVersion ); - // Add a first version. - let (rst, _) = server + // After add_version, lookup returns the Version with matching ids and payload. + let history = b"1234".to_vec(); + let AddVersionResult::Ok(version_id) = server .add_version(parent_version_id, history.clone()) - .await?; - - let AddVersionResult::Ok(version_id) = rst else { - panic!("Couldn't add version"); + .await? + .0 + else { + panic!("add_version failed"); }; - // Now we should be able to get the version. match server.get_child_version(parent_version_id).await? { GetVersionResult::Version { version_id: v_id, @@ -1130,4 +1134,32 @@ mod test { ); Ok(()) } + + /// A corrupted version file should return `Err`. + #[tokio::test] + async fn test_get_child_version_corrupted_file() -> Result<()> { + let tmp = TempDir::new()?; + let mut server = make_server(tmp.path())?; + + // Add a real version so we know the parent UUID. + let parent = Uuid::new_v4(); + let (result, _) = server.add_version(parent, b"good data".to_vec()).await?; + let AddVersionResult::Ok(child) = result else { + panic!("add_version failed"); + }; + + // Overwrite the version file with garbage to simulate corruption. + let filename = format!("v-{}-{}", parent.simple(), child.simple()); + std::fs::write(tmp.path().join(&filename), b"this is not valid ciphertext")?; + + // get_child_version should return Err (decryption failure), not NoSuchVersion. + let result = server.get_child_version(parent).await; + assert!( + result.is_err(), + "expected Err on decryption failure, got: {:?}", + result + ); + + Ok(()) + } } From 022948aa237745cb3d72fca0f68cc5cb86613101 Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Fri, 17 Apr 2026 21:04:45 -0700 Subject: [PATCH 15/44] misc cleanup --- src/server/gitsync/mod.rs | 114 ++++++++++++++++---------------------- 1 file changed, 49 insertions(+), 65 deletions(-) diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index 5362a88bb..a0a606032 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -57,15 +57,11 @@ use std::path::{Path, PathBuf}; use std::process::Command; use uuid::Uuid; -/// A version record stored in a version file on disk. -#[serde_as] -#[derive(Serialize, Deserialize, Debug)] +/// A version record; used as an in-memory carrier between read/write helpers. +#[derive(Debug)] struct Version { - #[serde(with = "uuid::serde::simple")] version_id: VersionId, - #[serde(with = "uuid::serde::simple")] parent_version_id: VersionId, - #[serde_as(as = "Base64")] history_segment: HistorySegment, } @@ -106,6 +102,22 @@ pub(crate) struct GitSyncServer { cryptor: Cryptor, } +/// Load and deserialise a [`Meta`] from the given path. +fn load_meta(path: &Path) -> Result { + let file = File::open(path)?; + Ok(serde_json::from_reader(BufReader::new(file))?) +} + +/// Parse a version filename of the form `v-{parent_simple}-{child_simple}` into +/// `(parent_id, child_id)`. Returns `None` if the filename does not match. +fn parse_version_filename(name: &str) -> Option<(Uuid, Uuid)> { + let stem = name.strip_prefix("v-")?; + let (parent_str, child_str) = stem.split_once('-')?; + let parent_id = Uuid::parse_str(parent_str).ok()?; + let child_id = Uuid::parse_str(child_str).ok()?; + Some((parent_id, child_id)) +} + /// Run a git command in a given directory, returning an error if it exits non-zero. fn git_cmd(dir: &Path, args: &[&str]) -> Result<()> { let status = Command::new("git").args(args).current_dir(dir).status()?; @@ -214,12 +226,8 @@ impl GitSyncServer { // Check for meta file, create and commit if missing. let meta_path = local_path.join("meta"); - let meta = match File::open(&meta_path) { - Ok(mut file) => { - let mut contents = String::new(); - file.read_to_string(&mut contents)?; - serde_json::from_str(&contents)? - } + let meta = match load_meta(&meta_path) { + Ok(m) => m, Err(_) => { let m = Meta { latest_version: Uuid::nil(), @@ -256,10 +264,7 @@ impl GitSyncServer { /// Read the meta file from disk and update self.meta. fn read_meta(&mut self) -> Result<()> { - let meta_path = self.local_path.join("meta"); - let file = File::open(&meta_path)?; - let reader = BufReader::new(file); - self.meta = serde_json::from_reader(reader)?; + self.meta = load_meta(&self.local_path.join("meta"))?; Ok(()) } @@ -347,15 +352,11 @@ impl GitSyncServer { &self, parent_version_id: &VersionId, ) -> Result> { - // glob to find file. - // v-PARENT-CHILD - let pattern = format!( - "{}/v-{}-*", - self.local_path - .to_str() - .ok_or_else(|| Error::Server("path is not valid UTF-8".into()))?, - parent_version_id.simple() - ); + let path_str = self + .local_path + .to_str() + .ok_or_else(|| Error::Server("local_path is not valid UTF-8".into()))?; + let pattern = format!("{}/v-{}-*", path_str, parent_version_id.simple()); for entry in glob(&pattern).map_err(|e| Error::Server(format!("{:?}", e)))? { let path = match entry { @@ -365,35 +366,27 @@ impl GitSyncServer { continue; } }; - let filename = match path.to_str() { + let filename = match path.file_name().and_then(|n| n.to_str()) { Some(f) => f, None => { log::warn!("non-UTF-8 path, skipping"); continue; } }; - let (_, version_id_str) = match filename.rsplit_once('-') { - Some(parts) => parts, + let version_id = match parse_version_filename(filename) { + Some((_, child)) => child, None => { log::warn!("unexpected filename format: {filename}"); continue; } }; - let version_id = match Uuid::parse_str(version_id_str) { - Ok(id) => id, - Err(e) => { - log::warn!("bad version id in {filename}: {e}"); - continue; - } - }; - // Real errors past this point + // Real errors past this point. let mut buf = Vec::new(); File::open(&path)?.read_to_end(&mut buf)?; - let sealed = Sealed { + let unsealed = self.cryptor.unseal(Sealed { version_id, payload: buf, - }; - let unsealed = self.cryptor.unseal(sealed)?; + })?; return Ok(Some(Version { version_id: unsealed.version_id, parent_version_id: *parent_version_id, @@ -440,12 +433,11 @@ impl GitSyncServer { }; // Parse all v-* filenames into a child -> (parent, path) map. - let pattern = format!( - "{}/v-*", - self.local_path - .to_str() - .ok_or_else(|| Error::Server("repo path is not valid UTF-8".into()))? - ); + let path_str = self + .local_path + .to_str() + .ok_or_else(|| Error::Server("local_path is not valid UTF-8".into()))?; + let pattern = format!("{}/v-*", path_str); let mut versions: HashMap = HashMap::new(); for entry in glob(&pattern).map_err(|e| Error::Server(format!("{e:?}")))? { let path = match entry { @@ -458,16 +450,7 @@ impl GitSyncServer { let Some(filename) = path.file_name().and_then(|f| f.to_str()) else { continue; }; - // Filename format: v-{parent_simple}-{child_simple} - let Some(stem) = filename.strip_prefix("v-") else { - continue; - }; - let Some((parent_str, child_str)) = stem.split_once('-') else { - continue; - }; - let (Ok(parent_id), Ok(child_id)) = - (Uuid::parse_str(parent_str), Uuid::parse_str(child_str)) - else { + let Some((parent_id, child_id)) = parse_version_filename(filename) else { continue; }; versions.insert(child_id, (parent_id, path)); @@ -573,9 +556,9 @@ impl Server for GitSyncServer { self.stage_and_commit(&[&version_path, &meta_path], "add version")?; if !self.push()? { - // Push was rejected, reset and re-read state. + // Push was rejected. Undo the commit; pull will reset --hard and git clean + // away the stray version file. git_cmd(&self.local_path, &["reset", "HEAD~1", "--soft"])?; - std::fs::remove_file(&version_path)?; self.pull()?; self.read_meta()?; return Ok(( @@ -591,14 +574,15 @@ impl Server for GitSyncServer { &mut self, parent_version_id: VersionId, ) -> Result { - let version = match self.get_version_by_parent_version_id(&parent_version_id)? { - Some(v) => Some(v), - None => { - self.pull()?; - self.get_version_by_parent_version_id(&parent_version_id)? - } - }; - match version { + if let Some(v) = self.get_version_by_parent_version_id(&parent_version_id)? { + return Ok(GetVersionResult::Version { + version_id: v.version_id, + parent_version_id: v.parent_version_id, + history_segment: v.history_segment, + }); + } + self.pull()?; + match self.get_version_by_parent_version_id(&parent_version_id)? { Some(v) => Ok(GetVersionResult::Version { version_id: v.version_id, parent_version_id: v.parent_version_id, From 8cb46b282614edc99bbc004a84781872d9b90287 Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Fri, 17 Apr 2026 22:39:00 -0700 Subject: [PATCH 16/44] lint --- src/server/config.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/server/config.rs b/src/server/config.rs index 844c8bb5d..b2e8b360a 100644 --- a/src/server/config.rs +++ b/src/server/config.rs @@ -1,4 +1,5 @@ use super::types::Server; +use crate::errors::Result; #[cfg(feature = "server-aws")] pub use crate::server::cloud::aws::AwsCredentials; #[cfg(feature = "server-aws")] @@ -7,13 +8,12 @@ use crate::server::cloud::aws::AwsService; use crate::server::cloud::gcp::GcpService; #[cfg(feature = "cloud")] use crate::server::cloud::CloudServer; +#[cfg(feature = "git-sync")] +use crate::server::gitsync::GitSyncServer; #[cfg(feature = "server-local")] use crate::server::local::LocalServer; #[cfg(feature = "server-sync")] use crate::server::sync::SyncServer; -#[cfg(feature = "git-sync")] -use crate::server::gitsync::GitSyncServer; -use crate::errors::Result; #[cfg(feature = "server-local")] use std::path::PathBuf; #[cfg(all(feature = "git-sync", not(feature = "server-local")))] From 64c3c0a96521730f032940953301133b62b14c8e Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Wed, 22 Apr 2026 18:57:59 -0700 Subject: [PATCH 17/44] fix typos and clarify new repo comment --- src/server/gitsync/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index a0a606032..0e3fb8515 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -10,7 +10,7 @@ //! - Metadata (`meta`) holds the latest version UUID and the encryption salt as JSON. //! //! After each write (`add_version`, `add_snapshot`) the server stages the changed files, -//! creates a commit, and pushes to the remote. If the push is rejected , thecommit is +//! creates a commit, and pushes to the remote. If the push is rejected, the commit is //! rolled back and the caller receives an [`AddVersionResult::ExpectedParentVersion`] //! or an [`Error`] so it can retry. //! @@ -20,7 +20,7 @@ //! //! Notes and Expectations //! -//! - Since this shells out to git, it assumes that you havea reasonably functional git +//! - Since this shells out to git, it assumes that you have a reasonably functional git //! setup. I.e. 'git init', 'git add', 'git commit', etc shoud just work. //! - If you are using a remote, 'git push' and 'git pull' shoud work. //! - Due to the nature of the version and snapshot history, you probably shouldn't do @@ -29,7 +29,7 @@ //! - If you are planning on using it for other things, it is HIGHLY recommended that you //! create a 'task' branch and let TaskChampion manage that branch. //! - This does support both defining a remote and having `local_only` mode set at the same -//! time. The idea is that maybe the remote isn't ready yet, or eithe rtemporarily or +//! time. The idea is that maybe the remote isn't ready yet, or either temporarily or //! permanantly down. Either way, you can use this in local mode in the mean time. //! - Remember, a remote can be on the same machine as local. This is used for testing. //! @@ -206,7 +206,7 @@ impl GitSyncServer { .status()? .success(); if !checkout_ok { - // For a brand-new repo `git checkout -b` also fails, so use + // For a repo with no commits yet, `git checkout -b` also fails, so use // `git symbolic-ref` to point HEAD at the desired branch without needing a commit. let has_commits = Command::new("git") .args(["rev-parse", "HEAD"]) From 7fcb7021ea3bf86cf14912c9994edf6da8027a92 Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Wed, 22 Apr 2026 19:09:21 -0700 Subject: [PATCH 18/44] two feature levels -> one --- Cargo.toml | 4 +--- src/server/config.rs | 8 ++++---- src/server/encryption.rs | 2 +- src/server/mod.rs | 2 +- 4 files changed, 7 insertions(+), 9 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f58626ec3..9ac044e62 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -49,7 +49,7 @@ server-aws = [ # Suppport for sync to another SQLite database on the same machine server-local = ["dep:rusqlite"] # Support for sync via Git -server-git = ["git-sync"] +server-git = ["dep:serde_with", "dep:glob", "encryption"] # Support for all task storage backends (except indexeddb, which only works on WASM builds) storage = ["storage-sqlite"] @@ -62,8 +62,6 @@ storage-indexeddb = ["dep:idb", "dep:web-sys"] encryption = ["dep:ring"] # (private) Generic support for cloud sync cloud = [] -# (private) Support for Git based sync -git-sync = ["dep:serde_with", "dep:glob", "encryption"] # static bundling of dependencies bundled = ["rusqlite/bundled"] # use CA roots built into the library for all HTTPS access diff --git a/src/server/config.rs b/src/server/config.rs index b2e8b360a..e4a48a6ff 100644 --- a/src/server/config.rs +++ b/src/server/config.rs @@ -8,7 +8,7 @@ use crate::server::cloud::aws::AwsService; use crate::server::cloud::gcp::GcpService; #[cfg(feature = "cloud")] use crate::server::cloud::CloudServer; -#[cfg(feature = "git-sync")] +#[cfg(feature = "server-git")] use crate::server::gitsync::GitSyncServer; #[cfg(feature = "server-local")] use crate::server::local::LocalServer; @@ -16,7 +16,7 @@ use crate::server::local::LocalServer; use crate::server::sync::SyncServer; #[cfg(feature = "server-local")] use std::path::PathBuf; -#[cfg(all(feature = "git-sync", not(feature = "server-local")))] +#[cfg(all(feature = "server-git", not(feature = "server-local")))] use std::path::PathBuf; #[cfg(feature = "server-sync")] use uuid::Uuid; @@ -123,7 +123,7 @@ pub enum ServerConfig { encryption_secret: Vec, }, /// A git repository - #[cfg(feature = "git-sync")] + #[cfg(feature = "server-git")] Git { /// The path to the local repo. local_path: PathBuf, @@ -186,7 +186,7 @@ impl ServerConfig { ) .await?, ), - #[cfg(feature = "git-sync")] + #[cfg(feature = "server-git")] ServerConfig::Git { local_path, branch, diff --git a/src/server/encryption.rs b/src/server/encryption.rs index 951a4fd24..cf0966f8f 100644 --- a/src/server/encryption.rs +++ b/src/server/encryption.rs @@ -27,7 +27,7 @@ impl Cryptor { } /// Generate a suitable random salt. - #[cfg(any(test, feature = "cloud", feature = "git-sync"))] // server-sync uses the clientId as the salt. + #[cfg(any(test, feature = "cloud", feature = "server-git"))] // server-sync uses the clientId as the salt. pub(super) fn gen_salt() -> Result> { let rng = rand::SystemRandom::new(); let mut salt = [0u8; 16]; diff --git a/src/server/mod.rs b/src/server/mod.rs index 3c5c93eca..0bc646c8c 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -31,7 +31,7 @@ mod sync; #[cfg(feature = "cloud")] mod cloud; -#[cfg(feature = "git-sync")] +#[cfg(feature = "server-git")] mod gitsync; pub use config::*; From 78c4e55bd5f7a80404039432ef6b0b24a1092e7c Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Wed, 22 Apr 2026 20:24:57 -0700 Subject: [PATCH 19/44] move remote from String to Option --- src/server/config.rs | 7 +++-- src/server/gitsync/mod.rs | 57 +++++++++++++++++++++++---------------- 2 files changed, 37 insertions(+), 27 deletions(-) diff --git a/src/server/config.rs b/src/server/config.rs index e4a48a6ff..2f575b2b3 100644 --- a/src/server/config.rs +++ b/src/server/config.rs @@ -132,10 +132,9 @@ pub enum ServerConfig { /// The remote repo. /// /// This can either be a named remote such as `origin` or a full git - /// url such as `git@myserver.com:/path/to/repo.git` - /// If `None` will use `origin` if the local repo has one. - /// Otherwise will operate in local only mode. - remote: String, + /// URL such as `git@myserver.com:/path/to/repo.git`. + /// If `None`, operates in local-only mode (no push/pull). + remote: Option, /// Don't clone/push/pull to `remote` even if it is defined. local_only: bool, /// Private encryption secret used to encrypt all data sent to the server. This can diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index 0e3fb8515..38327563a 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -89,15 +89,14 @@ struct Meta { /// A [`Server`] backed by a local git repository. /// -/// -/// When `local_only` is `false` the server pushes to and pulls from `remote` on the `branch` -/// branch after each write. Conflict resolution is handled as: commit locally, -/// attempt push, and on rejection pull-and-reset before returning. +/// When `remote` is `Some` and `local_only` is `false`, the server pushes to and pulls from +/// `remote` on the `branch` branch after each write. Conflict resolution is handled as: commit +/// locally, attempt push, and on rejection pull-and-reset before returning. pub(crate) struct GitSyncServer { meta: Meta, local_path: PathBuf, branch: String, - remote: String, + remote: Option, local_only: bool, cryptor: Cryptor, } @@ -141,11 +140,11 @@ impl GitSyncServer { pub(crate) fn new( local_path: PathBuf, branch: String, - remote: String, + remote: Option, local_only: bool, encryption_secret: Vec, ) -> Result { - let meta = Self::init_repo(&local_path, &branch, &remote, local_only)?; + let meta = Self::init_repo(&local_path, &branch, remote.as_deref(), local_only)?; let cryptor = Cryptor::new(&meta.salt, &encryption_secret.into())?; let server = GitSyncServer { meta, @@ -162,7 +161,12 @@ impl GitSyncServer { /// /// Creates the directory, initialises or clones the repo, switches to `branch`, and /// creates the `meta` file on first run. - fn init_repo(local_path: &Path, branch: &str, remote: &str, local_only: bool) -> Result { + fn init_repo( + local_path: &Path, + branch: &str, + remote: Option<&str>, + local_only: bool, + ) -> Result { // Create the local directory if needed. fs::create_dir_all(local_path)?; @@ -176,10 +180,11 @@ impl GitSyncServer { // Create one if not. if !is_repo { - if local_only { + if local_only || remote.is_none() { info!("Creating new repo at {:?}", local_path); git_cmd(local_path, &["init"])?; } else { + let remote = remote.unwrap(); info!("Cloning repo from {:?} to {:?}", remote, local_path); let parent = local_path .parent() @@ -276,9 +281,13 @@ impl GitSyncServer { Ok(meta_path) } - /// Fetch and fast-forward to the remote branch. No-op in local-only mode. - /// If the remote branch does not yet exist (e.g. fresh bare repo), this is also a no-op. + /// Fetch and fast-forward to the remote branch. No-op when there is no remote or in + /// local-only mode. If the remote branch does not yet exist (e.g. fresh bare repo), this is + /// also a no-op. fn pull(&self) -> Result<()> { + let Some(remote) = self.remote.as_deref() else { + return Ok(()); + }; if self.local_only { return Ok(()); } @@ -289,7 +298,7 @@ impl GitSyncServer { "ls-remote", "--exit-code", "--heads", - &self.remote, + remote, &self.branch, ]) .current_dir(&self.local_path) @@ -299,7 +308,7 @@ impl GitSyncServer { if !has_remote_branch { return Ok(()); } - git_cmd(&self.local_path, &["fetch", &self.remote, &self.branch])?; + git_cmd(&self.local_path, &["fetch", remote, &self.branch])?; git_cmd(&self.local_path, &["reset", "--hard", "FETCH_HEAD"])?; // Remove any untracked files left behind by interrupted writes. git_cmd( @@ -309,14 +318,17 @@ impl GitSyncServer { Ok(()) } - /// Push to the remote branch. Returns `true` on success, `false` if the push is rejected - /// Always returns `true` in local-only mode. + /// Push to the remote branch. Returns `true` on success, `false` if the push is rejected. + /// Returns `true` immediately when there is no remote or in local-only mode. fn push(&self) -> Result { + let Some(remote) = self.remote.as_deref() else { + return Ok(true); + }; if self.local_only { return Ok(true); } let output = Command::new("git") - .args(["push", &self.remote, &self.branch]) + .args(["push", remote, &self.branch]) .current_dir(&self.local_path) .output()?; if !output.status.success() { @@ -659,7 +671,7 @@ mod test { GitSyncServer::new( dir.to_path_buf(), "main".into(), - "".into(), + None, true, b"test-secret".to_vec(), ) @@ -681,7 +693,7 @@ mod test { GitSyncServer::new( clone_dir.to_path_buf(), "main".into(), - bare_url.into(), + Some(bare_url.to_string()), false, b"test-secret".to_vec(), ) @@ -746,11 +758,10 @@ mod test { // Build a server for clone2 and pull // It should see the new file. - let bare_url_str: String = bare_url.into(); let server2 = GitSyncServer::new( clone2.clone(), "main".into(), - bare_url_str, + Some(bare_url.to_string()), false, b"test-secret".to_vec(), )?; @@ -824,7 +835,7 @@ mod test { let mut server2 = GitSyncServer::new( clone2, "main".into(), - bare_url.into(), + Some(bare_url.to_string()), false, b"test-secret".to_vec(), )?; @@ -905,7 +916,7 @@ mod test { let mut server2 = GitSyncServer::new( clone2, "main".into(), - bare_url.into(), + Some(bare_url.to_string()), false, b"test-secret".to_vec(), )?; @@ -968,7 +979,7 @@ mod test { let mut server2 = GitSyncServer::new( clone2.clone(), "main".into(), - bare_url.into(), + Some(bare_url.to_string()), false, b"test-secret".to_vec(), )?; From d9cfb364810295692e4f8eb0077f1142d5b37f0f Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Thu, 23 Apr 2026 07:06:00 -0700 Subject: [PATCH 20/44] redirect git output to logs --- src/server/gitsync/mod.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index 38327563a..fcace9aef 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -118,13 +118,22 @@ fn parse_version_filename(name: &str) -> Option<(Uuid, Uuid)> { } /// Run a git command in a given directory, returning an error if it exits non-zero. +/// stdout and stderr are captured and forwarded to the log at debug level. fn git_cmd(dir: &Path, args: &[&str]) -> Result<()> { - let status = Command::new("git").args(args).current_dir(dir).status()?; - if !status.success() { + let output = Command::new("git").args(args).current_dir(dir).output()?; + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + if !stdout.is_empty() { + log::debug!("git {}: stdout: {}", args.join(" "), stdout.trim_end()); + } + if !stderr.is_empty() { + log::debug!("git {}: stderr: {}", args.join(" "), stderr.trim_end()); + } + if !output.status.success() { return Err(Error::Server(format!( "git {} failed with status {}", args.join(" "), - status + output.status ))); } Ok(()) From ddb036bc31ef7990301a6117c2fa6b91ecbfc1bd Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Thu, 23 Apr 2026 15:03:23 -0700 Subject: [PATCH 21/44] Split git_cmd into git_cmd_ok to simplify calls. --- src/server/gitsync/mod.rs | 57 ++++++++++++--------------------------- 1 file changed, 17 insertions(+), 40 deletions(-) diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index fcace9aef..08d335f5e 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -117,9 +117,10 @@ fn parse_version_filename(name: &str) -> Option<(Uuid, Uuid)> { Some((parent_id, child_id)) } -/// Run a git command in a given directory, returning an error if it exits non-zero. +/// Run a git command, returning `Ok(true)` on success and `Ok(false)` on a non-zero exit. /// stdout and stderr are captured and forwarded to the log at debug level. -fn git_cmd(dir: &Path, args: &[&str]) -> Result<()> { +/// Returns `Err` only on I/O failure (e.g. git binary not found). +fn git_cmd_ok(dir: &Path, args: &[&str]) -> Result { let output = Command::new("git").args(args).current_dir(dir).output()?; let stdout = String::from_utf8_lossy(&output.stdout); let stderr = String::from_utf8_lossy(&output.stderr); @@ -129,12 +130,13 @@ fn git_cmd(dir: &Path, args: &[&str]) -> Result<()> { if !stderr.is_empty() { log::debug!("git {}: stderr: {}", args.join(" "), stderr.trim_end()); } - if !output.status.success() { - return Err(Error::Server(format!( - "git {} failed with status {}", - args.join(" "), - output.status - ))); + Ok(output.status.success()) +} + +/// Run a git command, returning an error if it exits non-zero. +fn git_cmd(dir: &Path, args: &[&str]) -> Result<()> { + if !git_cmd_ok(dir, args)? { + return Err(Error::Server(format!("git {} failed", args.join(" ")))); } Ok(()) } @@ -180,12 +182,7 @@ impl GitSyncServer { fs::create_dir_all(local_path)?; // Check if path is already a git repo. - let is_repo = Command::new("git") - .arg("rev-parse") - .current_dir(local_path) - .output()? - .status - .success(); + let is_repo = git_cmd_ok(local_path, &["rev-parse"])?; // Create one if not. if !is_repo { @@ -213,21 +210,10 @@ impl GitSyncServer { // Switch to the requested branch. Try checking out an existing branch first, // only create a new one if that fails. info!("Switching branch to {:?}", branch); - let checkout_ok = Command::new("git") - .args(["checkout", branch]) - .current_dir(local_path) - .stderr(std::process::Stdio::null()) - .status()? - .success(); - if !checkout_ok { + if !git_cmd_ok(local_path, &["checkout", branch])? { // For a repo with no commits yet, `git checkout -b` also fails, so use // `git symbolic-ref` to point HEAD at the desired branch without needing a commit. - let has_commits = Command::new("git") - .args(["rev-parse", "HEAD"]) - .current_dir(local_path) - .stderr(std::process::Stdio::null()) - .status()? - .success(); + let has_commits = git_cmd_ok(local_path, &["rev-parse", "HEAD"])?; if has_commits { git_cmd(local_path, &["checkout", "-b", branch])?; } else { @@ -302,19 +288,10 @@ impl GitSyncServer { } // Check whether the remote branch exists before fetching. A bare repo with no commits // has no refs yet, and `git fetch origin ` would fail in that case. - let has_remote_branch = Command::new("git") - .args([ - "ls-remote", - "--exit-code", - "--heads", - remote, - &self.branch, - ]) - .current_dir(&self.local_path) - .stderr(std::process::Stdio::null()) - .status()? - .success(); - if !has_remote_branch { + if !git_cmd_ok( + &self.local_path, + &["ls-remote", "--exit-code", "--heads", remote, &self.branch], + )? { return Ok(()); } git_cmd(&self.local_path, &["fetch", remote, &self.branch])?; From 4747da05c3d74e338c65f04276c7bc1c077166a1 Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Thu, 23 Apr 2026 15:28:15 -0700 Subject: [PATCH 22/44] add some helper fns --- src/server/gitsync/mod.rs | 62 ++++++++++++++++++++------------------- 1 file changed, 32 insertions(+), 30 deletions(-) diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index 08d335f5e..5d07b196a 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -141,6 +141,22 @@ fn git_cmd(dir: &Path, args: &[&str]) -> Result<()> { Ok(()) } +/// Remove untracked TaskChampion files left behind by interrupted writes. +fn clean_stray_files(dir: &Path) -> Result<()> { + git_cmd(dir, &["clean", "-f", "--", "v-*", "snapshot", "meta"]) +} + +/// Stage each path and create a commit in `dir`. +fn stage_and_commit_files(dir: &Path, paths: &[&Path], message: &str) -> Result<()> { + for path in paths { + let path_str = path + .to_str() + .ok_or_else(|| Error::Server("path is not valid UTF-8".into()))?; + git_cmd(dir, &["add", path_str])?; + } + git_cmd(dir, &["commit", "-m", message]) +} + impl GitSyncServer { /// Create or re-open a git-backed sync server. /// @@ -235,31 +251,20 @@ impl GitSyncServer { }; let f = File::create_new(&meta_path)?; serde_json::to_writer(f, &m)?; - git_cmd(local_path, &["add", "meta"])?; - git_cmd(local_path, &["commit", "-m", "init taskchampion repo"])?; + stage_and_commit_files(local_path, &[&meta_path], "init taskchampion repo")?; m } }; // Remove any untracked files left behind by interrupted writes. - git_cmd( - local_path, - &["clean", "-f", "--", "v-*", "snapshot", "meta"], - )?; + clean_stray_files(local_path)?; Ok(meta) } /// Stage the given paths and create a commit. fn stage_and_commit(&self, paths: &[&Path], message: &str) -> Result<()> { - for path in paths { - let path_str = path - .to_str() - .ok_or_else(|| Error::Server("path is not valid UTF-8".into()))?; - git_cmd(&self.local_path, &["add", path_str])?; - } - git_cmd(&self.local_path, &["commit", "-m", message])?; - Ok(()) + stage_and_commit_files(&self.local_path, paths, message) } /// Read the meta file from disk and update self.meta. @@ -276,10 +281,10 @@ impl GitSyncServer { Ok(meta_path) } - /// Fetch and fast-forward to the remote branch. No-op when there is no remote or in - /// local-only mode. If the remote branch does not yet exist (e.g. fresh bare repo), this is - /// also a no-op. - fn pull(&self) -> Result<()> { + /// Fetch from remote and hard-reset the local branch to match. No-op when there is no remote + /// or in local-only mode. If the remote branch does not yet exist (e.g. fresh bare repo), + /// this is also a no-op. + fn reset_to_remote(&self) -> Result<()> { let Some(remote) = self.remote.as_deref() else { return Ok(()); }; @@ -297,10 +302,7 @@ impl GitSyncServer { git_cmd(&self.local_path, &["fetch", remote, &self.branch])?; git_cmd(&self.local_path, &["reset", "--hard", "FETCH_HEAD"])?; // Remove any untracked files left behind by interrupted writes. - git_cmd( - &self.local_path, - &["clean", "-f", "--", "v-*", "snapshot", "meta"], - )?; + clean_stray_files(&self.local_path)?; Ok(()) } @@ -511,7 +513,7 @@ impl GitSyncServer { // on the next snapshot. Reset to remote state so we are in sync. if !self.push()? { log::info!("cleanup: push rejected, resetting to remote state"); - self.pull()?; + self.reset_to_remote()?; } Ok(()) @@ -529,7 +531,7 @@ impl Server for GitSyncServer { // Otherwise check if parent is in latest. If it isn't, pull recheck. if self.meta.latest_version != Uuid::nil() && parent_version_id != self.meta.latest_version { - self.pull()?; + self.reset_to_remote()?; self.read_meta()?; if parent_version_id != self.meta.latest_version { return Ok(( @@ -557,7 +559,7 @@ impl Server for GitSyncServer { // Push was rejected. Undo the commit; pull will reset --hard and git clean // away the stray version file. git_cmd(&self.local_path, &["reset", "HEAD~1", "--soft"])?; - self.pull()?; + self.reset_to_remote()?; self.read_meta()?; return Ok(( AddVersionResult::ExpectedParentVersion(self.meta.latest_version), @@ -579,7 +581,7 @@ impl Server for GitSyncServer { history_segment: v.history_segment, }); } - self.pull()?; + self.reset_to_remote()?; match self.get_version_by_parent_version_id(&parent_version_id)? { Some(v) => Ok(GetVersionResult::Version { version_id: v.version_id, @@ -591,7 +593,7 @@ impl Server for GitSyncServer { } async fn add_snapshot(&mut self, version_id: VersionId, snapshot: Snapshot) -> Result<()> { - self.pull()?; + self.reset_to_remote()?; // Write the snapshot to a file. // Note: If another replica has pushed a snapshot for a // later version in the chain between our pull and our push, we will overwrite it. @@ -616,7 +618,7 @@ impl Server for GitSyncServer { if !self.push()? { // Push was rejected. Undo the commit, pull and clean to restore state. git_cmd(&self.local_path, &["reset", "HEAD~1", "--soft"])?; - self.pull()?; + self.reset_to_remote()?; self.read_meta()?; return Err(Error::Server("Couldn't push to remote.".into())); } @@ -630,7 +632,7 @@ impl Server for GitSyncServer { } async fn get_snapshot(&mut self) -> Result> { - self.pull()?; + self.reset_to_remote()?; let snapshot_path = self.local_path.join("snapshot"); if let Ok(file) = File::open(&snapshot_path) { @@ -751,7 +753,7 @@ mod test { false, b"test-secret".to_vec(), )?; - server2.pull()?; + server2.reset_to_remote()?; assert!(clone2.join("testfile").exists()); Ok(()) } From e4be3b61d52d80b27ff83e5f94939f704da14fc4 Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Thu, 23 Apr 2026 16:05:27 -0700 Subject: [PATCH 23/44] create a git struct to hold the git helpers and the git path --- src/server/config.rs | 4 + src/server/gitsync/mod.rs | 264 +++++++++++++++++++++++++------------- 2 files changed, 179 insertions(+), 89 deletions(-) diff --git a/src/server/config.rs b/src/server/config.rs index 2f575b2b3..da6e7300f 100644 --- a/src/server/config.rs +++ b/src/server/config.rs @@ -140,6 +140,8 @@ pub enum ServerConfig { /// Private encryption secret used to encrypt all data sent to the server. This can /// be any suitably un-guessable string of bytes. encryption_secret: Vec, + /// Path to the git binary. If `None`, `"git"` on PATH is used. + git_path: Option, }, } @@ -192,12 +194,14 @@ impl ServerConfig { remote, local_only, encryption_secret, + git_path, } => Box::new(GitSyncServer::new( local_path, branch, remote, local_only, encryption_secret, + git_path, )?), }) } diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index 5d07b196a..6249c137c 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -55,6 +55,7 @@ use std::fs::{self, File}; use std::io::{BufReader, Read}; use std::path::{Path, PathBuf}; use std::process::Command; +use std::time::{Duration, SystemTime, UNIX_EPOCH}; use uuid::Uuid; /// A version record; used as an in-memory carrier between read/write helpers. @@ -93,12 +94,15 @@ struct Meta { /// `remote` on the `branch` branch after each write. Conflict resolution is handled as: commit /// locally, attempt push, and on rejection pull-and-reset before returning. pub(crate) struct GitSyncServer { + git: Git, meta: Meta, local_path: PathBuf, branch: String, remote: Option, local_only: bool, cryptor: Cryptor, + /// Minimum age a version file must reach before cleanup() will remove it. + version_retention: Duration, } /// Load and deserialise a [`Meta`] from the given path. @@ -117,44 +121,97 @@ fn parse_version_filename(name: &str) -> Option<(Uuid, Uuid)> { Some((parent_id, child_id)) } -/// Run a git command, returning `Ok(true)` on success and `Ok(false)` on a non-zero exit. -/// stdout and stderr are captured and forwarded to the log at debug level. -/// Returns `Err` only on I/O failure (e.g. git binary not found). -fn git_cmd_ok(dir: &Path, args: &[&str]) -> Result { - let output = Command::new("git").args(args).current_dir(dir).output()?; - let stdout = String::from_utf8_lossy(&output.stdout); - let stderr = String::from_utf8_lossy(&output.stderr); - if !stdout.is_empty() { - log::debug!("git {}: stdout: {}", args.join(" "), stdout.trim_end()); +const VERSION_RETENTION: Duration = Duration::from_secs(180 * 24 * 60 * 60); + +/// Thin wrapper around a git binary path that provides the command helpers used throughout +/// this module. Defaults to `"git"` on PATH when constructed with `Git::new(None)`. +struct Git { + path: PathBuf, +} + +impl Git { + fn new(path: Option) -> Self { + Git { + path: path.unwrap_or_else(|| PathBuf::from("git")), + } } - if !stderr.is_empty() { - log::debug!("git {}: stderr: {}", args.join(" "), stderr.trim_end()); + + /// Run a git command, returning `Ok(true)` on success and `Ok(false)` on a non-zero exit. + /// stdout and stderr are captured and forwarded to the log at debug level. + /// Returns `Err` only on I/O failure (e.g. git binary not found). + fn cmd_ok(&self, dir: &Path, args: &[&str]) -> Result { + let output = Command::new(&self.path) + .args(args) + .current_dir(dir) + .output()?; + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + if !stdout.is_empty() { + log::debug!("git {}: stdout: {}", args.join(" "), stdout.trim_end()); + } + if !stderr.is_empty() { + log::debug!("git {}: stderr: {}", args.join(" "), stderr.trim_end()); + } + Ok(output.status.success()) } - Ok(output.status.success()) -} -/// Run a git command, returning an error if it exits non-zero. -fn git_cmd(dir: &Path, args: &[&str]) -> Result<()> { - if !git_cmd_ok(dir, args)? { - return Err(Error::Server(format!("git {} failed", args.join(" ")))); + /// Run a git command, returning an error if it exits non-zero. + fn cmd(&self, dir: &Path, args: &[&str]) -> Result<()> { + if !self.cmd_ok(dir, args)? { + return Err(Error::Server(format!("git {} failed", args.join(" ")))); + } + Ok(()) } - Ok(()) -} -/// Remove untracked TaskChampion files left behind by interrupted writes. -fn clean_stray_files(dir: &Path) -> Result<()> { - git_cmd(dir, &["clean", "-f", "--", "v-*", "snapshot", "meta"]) -} + /// Run a git command and return its trimmed stdout. Returns an error if the command exits + /// non-zero. Useful for commands like `git log --format=...` that produce structured output. + fn output(&self, dir: &Path, args: &[&str]) -> Result { + let output = Command::new(&self.path) + .args(args) + .current_dir(dir) + .output()?; + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + if !stderr.is_empty() { + log::debug!("git {}: stderr: {}", args.join(" "), stderr.trim_end()); + } + if !output.status.success() { + return Err(Error::Server(format!("git {} failed", args.join(" ")))); + } + Ok(stdout.trim_end().to_string()) + } -/// Stage each path and create a commit in `dir`. -fn stage_and_commit_files(dir: &Path, paths: &[&Path], message: &str) -> Result<()> { - for path in paths { - let path_str = path - .to_str() - .ok_or_else(|| Error::Server("path is not valid UTF-8".into()))?; - git_cmd(dir, &["add", path_str])?; + /// Remove untracked TaskChampion files left behind by interrupted writes. + fn clean_stray_files(&self, dir: &Path) -> Result<()> { + self.cmd(dir, &["clean", "-f", "--", "v-*", "snapshot", "meta"]) + } + + /// Stage each path and create a commit in `dir`. + fn stage_and_commit_files(&self, dir: &Path, paths: &[&Path], message: &str) -> Result<()> { + for path in paths { + let path_str = path + .to_str() + .ok_or_else(|| Error::Server("path is not valid UTF-8".into()))?; + self.cmd(dir, &["add", path_str])?; + } + self.cmd(dir, &["commit", "-m", message]) + } + + /// Return how long ago `filename` was last committed in `dir`, or `None` if git has no + /// record of it. A missing record is treated as "keep". + fn version_file_age(&self, dir: &Path, filename: &str) -> Result> { + let out = self.output(dir, &["log", "-1", "--format=%ct", "--", filename])?; + if out.is_empty() { + return Ok(None); + } + let commit_ts: u64 = out + .parse() + .map_err(|_| Error::Server(format!("unexpected git log output: {out:?}")))?; + let now = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_err(|e| Error::Server(format!("system clock error: {e}")))?; + Ok(Some(now.saturating_sub(Duration::from_secs(commit_ts)))) } - git_cmd(dir, &["commit", "-m", message]) } impl GitSyncServer { @@ -170,16 +227,20 @@ impl GitSyncServer { remote: Option, local_only: bool, encryption_secret: Vec, + git_path: Option, ) -> Result { - let meta = Self::init_repo(&local_path, &branch, remote.as_deref(), local_only)?; + let git = Git::new(git_path); + let meta = Self::init_repo(&git, &local_path, &branch, remote.as_deref(), local_only)?; let cryptor = Cryptor::new(&meta.salt, &encryption_secret.into())?; let server = GitSyncServer { + git, meta, local_path, branch, remote, local_only, cryptor, + version_retention: VERSION_RETENTION, }; Ok(server) } @@ -189,6 +250,7 @@ impl GitSyncServer { /// Creates the directory, initialises or clones the repo, switches to `branch`, and /// creates the `meta` file on first run. fn init_repo( + git: &Git, local_path: &Path, branch: &str, remote: Option<&str>, @@ -198,42 +260,44 @@ impl GitSyncServer { fs::create_dir_all(local_path)?; // Check if path is already a git repo. - let is_repo = git_cmd_ok(local_path, &["rev-parse"])?; + let is_repo = git.cmd_ok(local_path, &["rev-parse"])?; // Create one if not. if !is_repo { - if local_only || remote.is_none() { - info!("Creating new repo at {:?}", local_path); - git_cmd(local_path, &["init"])?; - } else { - let remote = remote.unwrap(); - info!("Cloning repo from {:?} to {:?}", remote, local_path); - let parent = local_path - .parent() - .ok_or_else(|| Error::Server("local_path has no parent".into()))?; - let dir_name = local_path - .file_name() - .ok_or_else(|| Error::Server("local_path has no file name".into()))? - .to_str() - .ok_or_else(|| Error::Server("local_path is not valid UTF-8".into()))?; - git_cmd(parent, &["clone", remote, dir_name])?; + match (local_only, remote) { + (false, Some(remote)) => { + info!("Cloning repo from {:?} to {:?}", remote, local_path); + let parent = local_path + .parent() + .ok_or_else(|| Error::Server("local_path has no parent".into()))?; + let dir_name = local_path + .file_name() + .ok_or_else(|| Error::Server("local_path has no file name".into()))? + .to_str() + .ok_or_else(|| Error::Server("local_path is not valid UTF-8".into()))?; + git.cmd(parent, &["clone", remote, dir_name])?; + } + _ => { + info!("Creating new repo at {:?}", local_path); + git.cmd(local_path, &["init"])?; + } } // Set identity so commits work in environments without a global git config. - git_cmd(local_path, &["config", "user.email", "taskchampion@local"])?; - git_cmd(local_path, &["config", "user.name", "taskchampion"])?; + git.cmd(local_path, &["config", "user.email", "taskchampion@local"])?; + git.cmd(local_path, &["config", "user.name", "taskchampion"])?; } // Switch to the requested branch. Try checking out an existing branch first, // only create a new one if that fails. info!("Switching branch to {:?}", branch); - if !git_cmd_ok(local_path, &["checkout", branch])? { + if !git.cmd_ok(local_path, &["checkout", branch])? { // For a repo with no commits yet, `git checkout -b` also fails, so use // `git symbolic-ref` to point HEAD at the desired branch without needing a commit. - let has_commits = git_cmd_ok(local_path, &["rev-parse", "HEAD"])?; + let has_commits = git.cmd_ok(local_path, &["rev-parse", "HEAD"])?; if has_commits { - git_cmd(local_path, &["checkout", "-b", branch])?; + git.cmd(local_path, &["checkout", "-b", branch])?; } else { - git_cmd( + git.cmd( local_path, &["symbolic-ref", "HEAD", &format!("refs/heads/{}", branch)], )?; @@ -251,20 +315,21 @@ impl GitSyncServer { }; let f = File::create_new(&meta_path)?; serde_json::to_writer(f, &m)?; - stage_and_commit_files(local_path, &[&meta_path], "init taskchampion repo")?; + git.stage_and_commit_files(local_path, &[&meta_path], "init taskchampion repo")?; m } }; // Remove any untracked files left behind by interrupted writes. - clean_stray_files(local_path)?; + git.clean_stray_files(local_path)?; Ok(meta) } /// Stage the given paths and create a commit. fn stage_and_commit(&self, paths: &[&Path], message: &str) -> Result<()> { - stage_and_commit_files(&self.local_path, paths, message) + self.git + .stage_and_commit_files(&self.local_path, paths, message) } /// Read the meta file from disk and update self.meta. @@ -293,16 +358,26 @@ impl GitSyncServer { } // Check whether the remote branch exists before fetching. A bare repo with no commits // has no refs yet, and `git fetch origin ` would fail in that case. - if !git_cmd_ok( + if !self.git.cmd_ok( &self.local_path, &["ls-remote", "--exit-code", "--heads", remote, &self.branch], )? { return Ok(()); } - git_cmd(&self.local_path, &["fetch", remote, &self.branch])?; - git_cmd(&self.local_path, &["reset", "--hard", "FETCH_HEAD"])?; + // Warn if there are uncommitted changes that the hard reset will discard. This should + // only happen if a previous write was interrupted; clean_stray_files handles the fallout. + if !self + .git + .cmd_ok(&self.local_path, &["diff", "--quiet", "HEAD"])? + { + log::warn!("reset_to_remote: discarding uncommitted local changes"); + } + self.git + .cmd(&self.local_path, &["fetch", remote, &self.branch])?; + self.git + .cmd(&self.local_path, &["reset", "--hard", "FETCH_HEAD"])?; // Remove any untracked files left behind by interrupted writes. - clean_stray_files(&self.local_path)?; + self.git.clean_stray_files(&self.local_path)?; Ok(()) } @@ -315,17 +390,8 @@ impl GitSyncServer { if self.local_only { return Ok(true); } - let output = Command::new("git") - .args(["push", remote, &self.branch]) - .current_dir(&self.local_path) - .output()?; - if !output.status.success() { - log::debug!( - "git push failed: {}", - String::from_utf8_lossy(&output.stderr) - ); - } - Ok(output.status.success()) + self.git + .cmd_ok(&self.local_path, &["push", remote, &self.branch]) } /// Encrypt and write a version file named `v-{parent_version_id}-{version_id}`. @@ -475,7 +541,9 @@ impl GitSyncServer { } } - // Remove each covered version file from the git index and working tree. + // Remove each covered version file from the git index and working tree, but only if it + // is older than VERSION_RETENTION. Recent files are kept so that a corrupt snapshot can + // still be recovered by replaying recent history. // If any `git rm` fails partway through we `git reset HEAD` to unstage, leaving // the working tree dirty but the index clean for subsequent operations. let mut any_removed = false; @@ -485,14 +553,18 @@ impl GitSyncServer { let name = path.file_name().and_then(|n| n.to_str()).ok_or_else(|| { Error::Server("version file path is not valid UTF-8".into()) })?; - git_cmd(&self.local_path, &["rm", name])?; + match self.git.version_file_age(&self.local_path, name)? { + Some(age) if age >= self.version_retention => {} + _ => continue, // Too recent or unknown age — keep it. + } + self.git.cmd(&self.local_path, &["rm", name])?; any_removed = true; } } Ok(()) })(); if let Err(e) = rm_result { - let _ = git_cmd(&self.local_path, &["reset", "HEAD"]); + let _ = self.git.cmd(&self.local_path, &["reset", "HEAD"]); return Err(e); } @@ -500,7 +572,7 @@ impl GitSyncServer { return Ok(()); } - git_cmd( + self.git.cmd( &self.local_path, &[ "commit", @@ -558,7 +630,8 @@ impl Server for GitSyncServer { if !self.push()? { // Push was rejected. Undo the commit; pull will reset --hard and git clean // away the stray version file. - git_cmd(&self.local_path, &["reset", "HEAD~1", "--soft"])?; + self.git + .cmd(&self.local_path, &["reset", "HEAD~1", "--soft"])?; self.reset_to_remote()?; self.read_meta()?; return Ok(( @@ -617,7 +690,8 @@ impl Server for GitSyncServer { if !self.push()? { // Push was rejected. Undo the commit, pull and clean to restore state. - git_cmd(&self.local_path, &["reset", "HEAD~1", "--soft"])?; + self.git + .cmd(&self.local_path, &["reset", "HEAD~1", "--soft"])?; self.reset_to_remote()?; self.read_meta()?; return Err(Error::Server("Couldn't push to remote.".into())); @@ -656,20 +730,23 @@ mod test { use tempfile::TempDir; fn make_server(dir: &Path) -> Result { - GitSyncServer::new( + let mut s = GitSyncServer::new( dir.to_path_buf(), "main".into(), None, true, b"test-secret".to_vec(), - ) + None, + )?; + s.version_retention = Duration::ZERO; + Ok(s) } /// Create a bare repo to act as a remote, then clone it into `clone_dir`. /// Returns a GitSyncServer backed by the clone, with the bare repo as its remote. fn make_server_with_remote(bare_dir: &Path, clone_dir: &Path) -> Result { // Initialise the bare remote. - git_cmd( + Git::new(None).cmd( bare_dir.parent().unwrap(), &[ "init", @@ -678,13 +755,16 @@ mod test { ], )?; let bare_url = bare_dir.to_str().unwrap(); - GitSyncServer::new( + let mut s = GitSyncServer::new( clone_dir.to_path_buf(), "main".into(), Some(bare_url.to_string()), false, b"test-secret".to_vec(), - ) + None, + )?; + s.version_retention = Duration::ZERO; + Ok(s) } #[test] @@ -734,9 +814,10 @@ mod test { // Clone a second copy directly via git. let bare_url = bare.to_str().unwrap(); - git_cmd(tmp.path(), &["clone", bare_url, "clone2"])?; - git_cmd(&clone2, &["config", "user.email", "taskchampion@local"])?; - git_cmd(&clone2, &["config", "user.name", "taskchampion"])?; + let git = Git::new(None); + git.cmd(tmp.path(), &["clone", bare_url, "clone2"])?; + git.cmd(&clone2, &["config", "user.email", "taskchampion@local"])?; + git.cmd(&clone2, &["config", "user.name", "taskchampion"])?; // Write a new file in clone1 and push it. let new_file = clone1.join("testfile"); @@ -752,6 +833,7 @@ mod test { Some(bare_url.to_string()), false, b"test-secret".to_vec(), + None, )?; server2.reset_to_remote()?; assert!(clone2.join("testfile").exists()); @@ -817,15 +899,17 @@ mod test { server1.push()?; let bare_url = bare.to_str().unwrap(); - git_cmd(tmp.path(), &["clone", bare_url, "clone2"])?; - git_cmd(&clone2, &["config", "user.email", "taskchampion@local"])?; - git_cmd(&clone2, &["config", "user.name", "taskchampion"])?; + let git = Git::new(None); + git.cmd(tmp.path(), &["clone", bare_url, "clone2"])?; + git.cmd(&clone2, &["config", "user.email", "taskchampion@local"])?; + git.cmd(&clone2, &["config", "user.name", "taskchampion"])?; let mut server2 = GitSyncServer::new( clone2, "main".into(), Some(bare_url.to_string()), false, b"test-secret".to_vec(), + None, )?; // server1 adds a version from NIL parent and pushes successfully. @@ -907,6 +991,7 @@ mod test { Some(bare_url.to_string()), false, b"test-secret".to_vec(), + None, )?; // get_child_version should pull and find the version. @@ -970,6 +1055,7 @@ mod test { Some(bare_url.to_string()), false, b"test-secret".to_vec(), + None, )?; let result = server2.get_snapshot().await?; assert!(result.is_some(), "expected snapshot from remote"); From a110d6d1adf857d52dd3d6dde83d5db01c4659f3 Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Thu, 23 Apr 2026 17:00:53 -0700 Subject: [PATCH 24/44] lots of documentaion cleanup --- src/server/config.rs | 17 ++++- src/server/gitsync/mod.rs | 142 ++++++++++++++++---------------------- 2 files changed, 76 insertions(+), 83 deletions(-) diff --git a/src/server/config.rs b/src/server/config.rs index da6e7300f..4fd81ebef 100644 --- a/src/server/config.rs +++ b/src/server/config.rs @@ -122,7 +122,22 @@ pub enum ServerConfig { /// be any suitably un-guessable string of bytes. encryption_secret: Vec, }, - /// A git repository + /// A git repository used as a sync server. + /// + /// TaskChampion manages the repository. Version history and snapshots are stored as committed + /// files. Do not merge, squash, or otherwise rewrite its history outside of TaskChampion. + /// + /// If the repository will be used for other things, it is HIGHLY recommended that you + /// configure a dedicated TaskChampion branch. + /// + /// This backend shells out to git, so a working `git` installation is required. For + /// remote sync, `git push` and `git pull` must work without prompts. Use SSH keys or a + /// credential helper. + /// + /// Setting `local_only` to `true` while `remote` is `Some` lets the replica operate + /// offline. Re-enable sync later by setting `local_only` back to `false`. However, be aware + /// that if the remote has diverged enough that a fast-forward fails, you will need to fix it + /// manually. #[cfg(feature = "server-git")] Git { /// The path to the local repo. diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index 6249c137c..d962f4a3c 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -14,30 +14,10 @@ //! rolled back and the caller receives an [`AddVersionResult::ExpectedParentVersion`] //! or an [`Error`] so it can retry. //! -//! After a snapshot is stored, [`GitSyncServer::cleanup`] automatically removes all -//! version files whose history is now captured by the snapshot, keeping the repository -//! compact. +//! After a snapshot is stored, [`GitSyncServer::cleanup`] automatically removes version +//! files whose history is captured by the snapshot and that are older than the retention +//! period, keeping the repository compact. //! -//! Notes and Expectations -//! -//! - Since this shells out to git, it assumes that you have a reasonably functional git -//! setup. I.e. 'git init', 'git add', 'git commit', etc shoud just work. -//! - If you are using a remote, 'git push' and 'git pull' shoud work. -//! - Due to the nature of the version and snapshot history, you probably shouldn't do -//! a lot of the things you normally would with a git repo, like merge, squash, etc. -//! Just let TaskChampion manage it. -//! - If you are planning on using it for other things, it is HIGHLY recommended that you -//! create a 'task' branch and let TaskChampion manage that branch. -//! - This does support both defining a remote and having `local_only` mode set at the same -//! time. The idea is that maybe the remote isn't ready yet, or either temporarily or -//! permanantly down. Either way, you can use this in local mode in the mean time. -//! - Remember, a remote can be on the same machine as local. This is used for testing. -//! -//! Notes for Reviewers -//! -//! - I haven't done any performance testing, but it seems reasonably quick for manual use. -//! - Currently is uses the same salt for all files. This isn't great security practice, -//! but does seem to be what the other servers are doing. use crate::errors::Result; use crate::server::encryption::{Cryptor, Sealed, Unsealed}; use crate::server::{ @@ -58,7 +38,7 @@ use std::process::Command; use std::time::{Duration, SystemTime, UNIX_EPOCH}; use uuid::Uuid; -/// A version record; used as an in-memory carrier between read/write helpers. +/// A version record, used as an in-memory carrier between read/write helpers. #[derive(Debug)] struct Version { version_id: VersionId, @@ -84,6 +64,7 @@ struct SnapshotFile { struct Meta { #[serde(with = "uuid::serde::simple")] latest_version: VersionId, + // A single salt is used for all files in this repo, consistent with other server backends. #[serde_as(as = "Base64")] salt: Vec, } @@ -93,6 +74,10 @@ struct Meta { /// When `remote` is `Some` and `local_only` is `false`, the server pushes to and pulls from /// `remote` on the `branch` branch after each write. Conflict resolution is handled as: commit /// locally, attempt push, and on rejection pull-and-reset before returning. +/// +/// Setting `local_only` to `true` while `remote` is `Some` allows the repository to operate +/// offline temporarily (e.g. when the remote is unreachable), and later re-enable sync by +/// setting `local_only` back to `false`. pub(crate) struct GitSyncServer { git: Git, meta: Meta, @@ -181,13 +166,8 @@ impl Git { Ok(stdout.trim_end().to_string()) } - /// Remove untracked TaskChampion files left behind by interrupted writes. - fn clean_stray_files(&self, dir: &Path) -> Result<()> { - self.cmd(dir, &["clean", "-f", "--", "v-*", "snapshot", "meta"]) - } - /// Stage each path and create a commit in `dir`. - fn stage_and_commit_files(&self, dir: &Path, paths: &[&Path], message: &str) -> Result<()> { + fn stage_and_commit(&self, dir: &Path, paths: &[&Path], message: &str) -> Result<()> { for path in paths { let path_str = path .to_str() @@ -196,22 +176,27 @@ impl Git { } self.cmd(dir, &["commit", "-m", message]) } +} - /// Return how long ago `filename` was last committed in `dir`, or `None` if git has no - /// record of it. A missing record is treated as "keep". - fn version_file_age(&self, dir: &Path, filename: &str) -> Result> { - let out = self.output(dir, &["log", "-1", "--format=%ct", "--", filename])?; - if out.is_empty() { - return Ok(None); - } - let commit_ts: u64 = out - .parse() - .map_err(|_| Error::Server(format!("unexpected git log output: {out:?}")))?; - let now = SystemTime::now() - .duration_since(UNIX_EPOCH) - .map_err(|e| Error::Server(format!("system clock error: {e}")))?; - Ok(Some(now.saturating_sub(Duration::from_secs(commit_ts)))) +/// Remove untracked TaskChampion files left behind by interrupted writes. +fn clean_stray_files(git: &Git, dir: &Path) -> Result<()> { + git.cmd(dir, &["clean", "-f", "--", "v-*", "snapshot", "meta"]) +} + +/// Return how long ago `filename` was last committed in `dir`, or `None` if git has no +/// record of it. A missing record is treated as "keep". +fn version_file_age(git: &Git, dir: &Path, filename: &str) -> Result> { + let out = git.output(dir, &["log", "-1", "--format=%ct", "--", filename])?; + if out.is_empty() { + return Ok(None); } + let commit_ts: u64 = out + .parse() + .map_err(|_| Error::Server(format!("unexpected git log output: {out:?}")))?; + let now = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_err(|e| Error::Server(format!("system clock error: {e}")))?; + Ok(Some(now.saturating_sub(Duration::from_secs(commit_ts)))) } impl GitSyncServer { @@ -291,12 +276,10 @@ impl GitSyncServer { // only create a new one if that fails. info!("Switching branch to {:?}", branch); if !git.cmd_ok(local_path, &["checkout", branch])? { - // For a repo with no commits yet, `git checkout -b` also fails, so use - // `git symbolic-ref` to point HEAD at the desired branch without needing a commit. - let has_commits = git.cmd_ok(local_path, &["rev-parse", "HEAD"])?; - if has_commits { - git.cmd(local_path, &["checkout", "-b", branch])?; - } else { + // Branch doesn't exist yet. Create it. + // checkout -b also fails if HEAD already points to this branch name (e.g. a + // fresh git init whose default branch matches), so fall back to symbolic-ref. + if !git.cmd_ok(local_path, &["checkout", "-b", branch])? { git.cmd( local_path, &["symbolic-ref", "HEAD", &format!("refs/heads/{}", branch)], @@ -315,23 +298,17 @@ impl GitSyncServer { }; let f = File::create_new(&meta_path)?; serde_json::to_writer(f, &m)?; - git.stage_and_commit_files(local_path, &[&meta_path], "init taskchampion repo")?; + git.stage_and_commit(local_path, &[&meta_path], "init taskchampion repo")?; m } }; // Remove any untracked files left behind by interrupted writes. - git.clean_stray_files(local_path)?; + clean_stray_files(&git, local_path)?; Ok(meta) } - /// Stage the given paths and create a commit. - fn stage_and_commit(&self, paths: &[&Path], message: &str) -> Result<()> { - self.git - .stage_and_commit_files(&self.local_path, paths, message) - } - /// Read the meta file from disk and update self.meta. fn read_meta(&mut self) -> Result<()> { self.meta = load_meta(&self.local_path.join("meta"))?; @@ -365,7 +342,7 @@ impl GitSyncServer { return Ok(()); } // Warn if there are uncommitted changes that the hard reset will discard. This should - // only happen if a previous write was interrupted; clean_stray_files handles the fallout. + // only happen if a previous write was interrupted. clean_stray_files handles the fallout. if !self .git .cmd_ok(&self.local_path, &["diff", "--quiet", "HEAD"])? @@ -377,7 +354,7 @@ impl GitSyncServer { self.git .cmd(&self.local_path, &["reset", "--hard", "FETCH_HEAD"])?; // Remove any untracked files left behind by interrupted writes. - self.git.clean_stray_files(&self.local_path)?; + clean_stray_files(&self.git, &self.local_path)?; Ok(()) } @@ -464,14 +441,11 @@ impl GitSyncServer { /// Return the [`SnapshotUrgency`] based on the number of version files present. /// - /// Since cleanup runs after every successful add_snapshot and removes - /// all version files covered by the snapshot, the count here reflects only post-snapshot - /// versions. + /// Since cleanup runs after every successful add_snapshot, the count here reflects + /// post-snapshot versions plus any recent files kept by the retention policy. fn snapshot_urgency(&self) -> SnapshotUrgency { let pattern = format!("{}/v-*", self.local_path.display()); let count = glob(&pattern).map(|g| g.count()).unwrap_or(0); - // TODO: Performance test get_version_by_parent_version_id to help determine - // what reasonable thresholds are. match count { 0..=50 => SnapshotUrgency::None, 51..=100 => SnapshotUrgency::Low, @@ -482,9 +456,10 @@ impl GitSyncServer { /// Cleans up the repository by removing version files covered by the current snapshot. /// /// Reads the snapshot to determine which version it covers, then walks backward - /// through the version chain from that version. All version files on that chain - /// are deleted, committed, and pushed. If the push is rejected, we reset to the remote state - /// and will retry on the next snapshot. + /// through the version chain from that version. Version files on that chain older than + /// `version_retention` are deleted, committed, and pushed. Recently-written files are kept + /// to allow recovery if the snapshot is corrupt. If the push is rejected, we reset to the + /// remote state and will retry on the next snapshot. /// /// If called with no snapshot present it is a no-op. fn cleanup(&self) -> Result<()> { @@ -553,9 +528,9 @@ impl GitSyncServer { let name = path.file_name().and_then(|n| n.to_str()).ok_or_else(|| { Error::Server("version file path is not valid UTF-8".into()) })?; - match self.git.version_file_age(&self.local_path, name)? { + match version_file_age(&self.git, &self.local_path, name)? { Some(age) if age >= self.version_retention => {} - _ => continue, // Too recent or unknown age — keep it. + _ => continue, // Too recent or unknown age, keep it. } self.git.cmd(&self.local_path, &["rm", name])?; any_removed = true; @@ -600,7 +575,7 @@ impl Server for GitSyncServer { history_segment: HistorySegment, ) -> Result<(AddVersionResult, SnapshotUrgency)> { // Accept any parent when the repo is empty (latest == NIL). - // Otherwise check if parent is in latest. If it isn't, pull recheck. + // Otherwise check if parent matches latest. If it doesn't, reset_to_remote and recheck. if self.meta.latest_version != Uuid::nil() && parent_version_id != self.meta.latest_version { self.reset_to_remote()?; @@ -625,11 +600,12 @@ impl Server for GitSyncServer { let meta_path = self.write_meta()?; // Commit and push, reverting if push fails. - self.stage_and_commit(&[&version_path, &meta_path], "add version")?; + self.git + .stage_and_commit(&self.local_path, &[&version_path, &meta_path], "add version")?; if !self.push()? { - // Push was rejected. Undo the commit; pull will reset --hard and git clean - // away the stray version file. + // Push was rejected. Undo the commit. reset_to_remote will fetch, reset --hard, + // and clean away the stray version file. self.git .cmd(&self.local_path, &["reset", "HEAD~1", "--soft"])?; self.reset_to_remote()?; @@ -668,10 +644,9 @@ impl Server for GitSyncServer { async fn add_snapshot(&mut self, version_id: VersionId, snapshot: Snapshot) -> Result<()> { self.reset_to_remote()?; // Write the snapshot to a file. - // Note: If another replica has pushed a snapshot for a - // later version in the chain between our pull and our push, we will overwrite it. - // This should be harmless: a replica with newer state will overwrite again, - // and push rejection handles concurrent writes. + // If another replica has pushed a snapshot for a later version in the chain between + // our reset_to_remote and our push, we will overwrite it. This is harmless. A replica + // with newer state will overwrite again, and push rejection handles concurrent writes. let unsealed = Unsealed { version_id, payload: snapshot, @@ -686,10 +661,11 @@ impl Server for GitSyncServer { serde_json::to_writer(f, &snapshot_file)?; // Commit and push, reverting if push fails. - self.stage_and_commit(&[&snapshot_path], "add snapshot")?; + self.git + .stage_and_commit(&self.local_path, &[&snapshot_path], "add snapshot")?; if !self.push()? { - // Push was rejected. Undo the commit, pull and clean to restore state. + // Push was rejected. Undo the commit and reset_to_remote to restore state. self.git .cmd(&self.local_path, &["reset", "HEAD~1", "--soft"])?; self.reset_to_remote()?; @@ -822,7 +798,9 @@ mod test { // Write a new file in clone1 and push it. let new_file = clone1.join("testfile"); std::fs::write(&new_file, b"hello")?; - server1.stage_and_commit(&[&new_file], "add testfile")?; + server1 + .git + .stage_and_commit(&server1.local_path, &[&new_file], "add testfile")?; assert!(server1.push()?); // Build a server for clone2 and pull @@ -1175,7 +1153,7 @@ mod test { .join(format!("v-{}-{}", v2.simple(), v3.simple())) .exists()); - // Snapshot at v3; cleanup should remove all three version files. + // Snapshot at v3, cleanup should remove all three version files. server.add_snapshot(v3, b"full state".to_vec()).await?; assert!( From eee941fae68d607eedbec0b31468129ffa40d0c0 Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Thu, 23 Apr 2026 22:06:14 -0700 Subject: [PATCH 25/44] fix snaphot urgency to only count post-snapshot versions --- src/server/gitsync/mod.rs | 59 ++++++++++++++++++++++++++++++++++----- 1 file changed, 52 insertions(+), 7 deletions(-) diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index d962f4a3c..35657cf59 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -439,13 +439,55 @@ impl GitSyncServer { Ok(None) } - /// Return the [`SnapshotUrgency`] based on the number of version files present. + /// Return the [`SnapshotUrgency`] based on the number of post-snapshot versions. /// - /// Since cleanup runs after every successful add_snapshot, the count here reflects - /// post-snapshot versions plus any recent files kept by the retention policy. + /// Counts only versions added since the current snapshot. fn snapshot_urgency(&self) -> SnapshotUrgency { - let pattern = format!("{}/v-*", self.local_path.display()); - let count = glob(&pattern).map(|g| g.count()).unwrap_or(0); + let path_str = match self.local_path.to_str() { + Some(s) => s, + None => { + log::warn!("snapshot_urgency: local_path is not valid UTF-8"); + return SnapshotUrgency::None; + } + }; + + // Read the snapshot version to know where post-snapshot history begins. + let snapshot_version: Option = File::open(self.local_path.join("snapshot")) + .ok() + .and_then(|f| serde_json::from_reader::<_, SnapshotFile>(BufReader::new(f)).ok()) + .map(|s| s.version_id); + + // Build a child -> parent map from v-* filenames. + let pattern = format!("{}/v-*", path_str); + let mut child_to_parent: HashMap = HashMap::new(); + if let Ok(entries) = glob(&pattern) { + for entry in entries.flatten() { + if let Some(name) = entry.file_name().and_then(|n| n.to_str()) { + if let Some((parent_id, child_id)) = parse_version_filename(name) { + child_to_parent.insert(child_id, parent_id); + } + } + } + } + + // Walk backward from latest_version, stopping at the snapshot version or NIL. + // Only post-snapshot steps are counted. + let stop_at = snapshot_version.unwrap_or(Uuid::nil()); + let mut count = 0usize; + let mut current = self.meta.latest_version; + for _ in 0..=child_to_parent.len() { + if current == stop_at || current == Uuid::nil() { + break; + } + match child_to_parent.get(¤t) { + Some(&parent) => { + count += 1; + current = parent; + } + None => break, + } + } + match count { 0..=50 => SnapshotUrgency::None, 51..=100 => SnapshotUrgency::Low, @@ -600,8 +642,11 @@ impl Server for GitSyncServer { let meta_path = self.write_meta()?; // Commit and push, reverting if push fails. - self.git - .stage_and_commit(&self.local_path, &[&version_path, &meta_path], "add version")?; + self.git.stage_and_commit( + &self.local_path, + &[&version_path, &meta_path], + "add version", + )?; if !self.push()? { // Push was rejected. Undo the commit. reset_to_remote will fetch, reset --hard, From f568ff31aef21b298cc4bd0645d428463903adfb Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Fri, 24 Apr 2026 16:58:07 -0700 Subject: [PATCH 26/44] cleanup cleanup and add cleanup tests --- src/server/gitsync/mod.rs | 135 ++++++++++++++++++++++++++++++-------- 1 file changed, 109 insertions(+), 26 deletions(-) diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index 35657cf59..d22f43475 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -495,6 +495,33 @@ impl GitSyncServer { } } + /// Remove covered version files that are old enough to delete. + /// + /// Stages `git rm` for each entry in `versions` whose child ID is in `covered` and + /// whose commit age exceeds `self.version_retention`. Returns `true` if anything was + /// staged. On error, the caller is responsible for running `git reset HEAD` to unstage. + fn remove_covered_versions( + &self, + versions: &HashMap, + covered: &HashSet, + ) -> Result { + let mut any_removed = false; + for (child_id, (_, path)) in versions { + if covered.contains(child_id) { + let name = path.file_name().and_then(|n| n.to_str()).ok_or_else(|| { + Error::Server("version file path is not valid UTF-8".into()) + })?; + match version_file_age(&self.git, &self.local_path, name)? { + Some(age) if age >= self.version_retention => {} + _ => continue, // Too recent or unknown age, keep it. + } + self.git.cmd(&self.local_path, &["rm", name])?; + any_removed = true; + } + } + Ok(any_removed) + } + /// Cleans up the repository by removing version files covered by the current snapshot. /// /// Reads the snapshot to determine which version it covers, then walks backward @@ -558,32 +585,14 @@ impl GitSyncServer { } } - // Remove each covered version file from the git index and working tree, but only if it - // is older than VERSION_RETENTION. Recent files are kept so that a corrupt snapshot can - // still be recovered by replaying recent history. - // If any `git rm` fails partway through we `git reset HEAD` to unstage, leaving - // the working tree dirty but the index clean for subsequent operations. - let mut any_removed = false; - let rm_result: Result<()> = (|| { - for (child_id, (_, path)) in &versions { - if covered.contains(child_id) { - let name = path.file_name().and_then(|n| n.to_str()).ok_or_else(|| { - Error::Server("version file path is not valid UTF-8".into()) - })?; - match version_file_age(&self.git, &self.local_path, name)? { - Some(age) if age >= self.version_retention => {} - _ => continue, // Too recent or unknown age, keep it. - } - self.git.cmd(&self.local_path, &["rm", name])?; - any_removed = true; - } + // Remove covered files that are old enough. On error, unstage and propagate. + let any_removed = match self.remove_covered_versions(&versions, &covered) { + Ok(removed) => removed, + Err(e) => { + let _ = self.git.cmd(&self.local_path, &["reset", "HEAD"]); + return Err(e); } - Ok(()) - })(); - if let Err(e) = rm_result { - let _ = self.git.cmd(&self.local_path, &["reset", "HEAD"]); - return Err(e); - } + }; if !any_removed { return Ok(()); @@ -676,6 +685,7 @@ impl Server for GitSyncServer { }); } self.reset_to_remote()?; + self.read_meta()?; match self.get_version_by_parent_version_id(&parent_version_id)? { Some(v) => Ok(GetVersionResult::Version { version_id: v.version_id, @@ -714,7 +724,6 @@ impl Server for GitSyncServer { self.git .cmd(&self.local_path, &["reset", "HEAD~1", "--soft"])?; self.reset_to_remote()?; - self.read_meta()?; return Err(Error::Server("Couldn't push to remote.".into())); } @@ -1254,4 +1263,78 @@ mod test { Ok(()) } + + /// Version files younger than `version_retention` must survive cleanup even when + /// they are covered by a snapshot. + #[tokio::test] + async fn test_cleanup_retains_recent_version_files() -> Result<()> { + let tmp = TempDir::new()?; + // Use default version_retention. Freshly committed files are seconds old, + // far below the threshold, so cleanup should leave them in place. + let mut server = GitSyncServer::new( + tmp.path().to_path_buf(), + "main".into(), + None, + true, + b"test-secret".to_vec(), + None, + )?; + + let (r1, _) = server.add_version(NIL_VERSION_ID, b"v1".to_vec()).await?; + let AddVersionResult::Ok(v1) = r1 else { + panic!("add_version 1 failed"); + }; + let (r2, _) = server.add_version(v1, b"v2".to_vec()).await?; + let AddVersionResult::Ok(v2) = r2 else { + panic!("add_version 2 failed"); + }; + + // Snapshot at v2; files are too young to remove. + server.add_snapshot(v2, b"full state".to_vec()).await?; + + assert!( + tmp.path() + .join(format!("v-{}-{}", NIL_VERSION_ID.simple(), v1.simple())) + .exists(), + "v1 file should be retained (too young to remove)" + ); + assert!( + tmp.path() + .join(format!("v-{}-{}", v1.simple(), v2.simple())) + .exists(), + "v2 file should be retained (too young to remove)" + ); + assert!(tmp.path().join("snapshot").exists()); + Ok(()) + } + + /// When `local_only` is `true`, a configured remote is never contacted: no clone on + /// init, no push after writes, no pull on a cache miss. + #[tokio::test] + async fn test_local_only_with_remote_ignores_remote() -> Result<()> { + let tmp = TempDir::new()?; + // Point at a non-existent remote. Any clone/push/pull attempt would fail. + let mut server = GitSyncServer::new( + tmp.path().to_path_buf(), + "main".into(), + Some("https://nonexistent.invalid/repo.git".to_string()), + true, + b"test-secret".to_vec(), + None, + )?; + server.version_retention = Duration::ZERO; + + let (r1, _) = server.add_version(NIL_VERSION_ID, b"v1".to_vec()).await?; + let AddVersionResult::Ok(v1) = r1 else { + panic!("add_version failed"); + }; + let result = server.get_child_version(NIL_VERSION_ID).await?; + assert!( + matches!(result, GetVersionResult::Version { .. }), + "expected Version result, got: {:?}", + result + ); + server.add_snapshot(v1, b"full state".to_vec()).await?; + Ok(()) + } } From b681fe8c76afb79e7da58b817f04ce0f9c170971 Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Fri, 24 Apr 2026 17:07:36 -0700 Subject: [PATCH 27/44] lint --- src/server/gitsync/mod.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index d22f43475..26a3dc6dc 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -508,9 +508,10 @@ impl GitSyncServer { let mut any_removed = false; for (child_id, (_, path)) in versions { if covered.contains(child_id) { - let name = path.file_name().and_then(|n| n.to_str()).ok_or_else(|| { - Error::Server("version file path is not valid UTF-8".into()) - })?; + let name = path + .file_name() + .and_then(|n| n.to_str()) + .ok_or_else(|| Error::Server("version file path is not valid UTF-8".into()))?; match version_file_age(&self.git, &self.local_path, name)? { Some(age) if age >= self.version_retention => {} _ => continue, // Too recent or unknown age, keep it. From 75f0c67c1754bf35f864a1979a40f59728364867 Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Fri, 24 Apr 2026 17:10:20 -0700 Subject: [PATCH 28/44] clippy --- src/server/gitsync/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index 26a3dc6dc..672465327 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -304,7 +304,7 @@ impl GitSyncServer { }; // Remove any untracked files left behind by interrupted writes. - clean_stray_files(&git, local_path)?; + clean_stray_files(git, local_path)?; Ok(meta) } From e1b277b07b6ea4d3d8a62d0247cbdf2716677063 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sat, 11 Apr 2026 23:53:30 +0000 Subject: [PATCH 29/44] Bump the cargo group with 3 updates Bumps the cargo group with 3 updates: [tokio](https://github.com/tokio-rs/tokio), [aws-sdk-s3](https://github.com/awslabs/aws-sdk-rust) and [libc](https://github.com/rust-lang/libc). Updates `tokio` from 1.50.0 to 1.51.0 - [Release notes](https://github.com/tokio-rs/tokio/releases) - [Commits](https://github.com/tokio-rs/tokio/compare/tokio-1.50.0...tokio-1.51.0) Updates `aws-sdk-s3` from 1.127.0 to 1.128.0 - [Release notes](https://github.com/awslabs/aws-sdk-rust/releases) - [Commits](https://github.com/awslabs/aws-sdk-rust/commits) Updates `libc` from 0.2.183 to 0.2.184 - [Release notes](https://github.com/rust-lang/libc/releases) - [Changelog](https://github.com/rust-lang/libc/blob/0.2.184/CHANGELOG.md) - [Commits](https://github.com/rust-lang/libc/compare/0.2.183...0.2.184) --- updated-dependencies: - dependency-name: tokio dependency-version: 1.51.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: cargo - dependency-name: aws-sdk-s3 dependency-version: 1.128.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: cargo - dependency-name: libc dependency-version: 0.2.184 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: cargo ... Signed-off-by: dependabot[bot] --- Cargo.lock | 20 ++++++++++---------- Cargo.toml | 2 +- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6e05d1e3c..9cd2f56b4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -150,9 +150,9 @@ dependencies = [ [[package]] name = "aws-sdk-s3" -version = "1.127.0" +version = "1.128.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "151783f64e0dcddeb4965d08e36c276b4400a46caa88805a2e36d497deaf031a" +checksum = "99304b64672e0d81a3c100a589b93d9ef5e9c0ce12e21c848fd39e50f493c2a1" dependencies = [ "aws-credential-types", "aws-runtime", @@ -1510,9 +1510,9 @@ checksum = "09edd9e8b54e49e587e4f6295a7d29c3ea94d469cb40ab8ca70b288248a81db2" [[package]] name = "libc" -version = "0.2.183" +version = "0.2.184" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b5b646652bf6661599e1da8901b3b9522896f01e736bad5f723fe7a3a27f899d" +checksum = "48f5d2a454e16a5ea0f4ced81bd44e4cfc7bd3a507b61887c99fd3538b28e4af" [[package]] name = "libm" @@ -1627,9 +1627,9 @@ dependencies = [ [[package]] name = "mio" -version = "1.1.1" +version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a69bcab0ad47271a0234d9422b131806bf3968021e5dc9328caf2d4cd58557fc" +checksum = "50b7e5b27aa02a74bac8c3f23f448f8d87ff11f92d3aac1a6ed369ee08cc56c1" dependencies = [ "libc", "wasi", @@ -2762,9 +2762,9 @@ checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20" [[package]] name = "tokio" -version = "1.50.0" +version = "1.51.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "27ad5e34374e03cfffefc301becb44e9dc3c17584f414349ebe29ed26661822d" +checksum = "2bd1c4c0fc4a7ab90fc15ef6daaa3ec3b893f004f915f2392557ed23237820cd" dependencies = [ "bytes", "libc", @@ -2778,9 +2778,9 @@ dependencies = [ [[package]] name = "tokio-macros" -version = "2.6.1" +version = "2.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5c55a2eff8b69ce66c84f85e1da1c233edc36ceb85a2058d11b0d6a3c7e7569c" +checksum = "385a6cb71ab9ab790c5fe8d67f1645e6c450a7ce006a33de03daa956cf70a496" dependencies = [ "proc-macro2", "quote", diff --git a/Cargo.toml b/Cargo.toml index c650a119a..7e7e155d1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -128,7 +128,7 @@ web-sys = { version = "0.3.83", optional = true, features = [ ## non-wasm dependencies [target.'cfg(not(target_arch = "wasm32"))'.dependencies] -aws-sdk-s3 = { version = "1.127", default-features = false, optional = true } +aws-sdk-s3 = { version = "1.128", default-features = false, optional = true } aws-config = { version = "1.8", default-features = false, features = [ "rt-tokio", "behavior-version-latest", From 56eb65c09bbfefc963120b972d223cdfdaf4776d Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 14 Apr 2026 03:38:00 +0000 Subject: [PATCH 30/44] Bump rand from 0.9.2 to 0.9.4 Bumps [rand](https://github.com/rust-random/rand) from 0.9.2 to 0.9.4. - [Release notes](https://github.com/rust-random/rand/releases) - [Changelog](https://github.com/rust-random/rand/blob/0.9.4/CHANGELOG.md) - [Commits](https://github.com/rust-random/rand/compare/rand_core-0.9.2...0.9.4) --- updated-dependencies: - dependency-name: rand dependency-version: 0.9.4 dependency-type: indirect ... Signed-off-by: dependabot[bot] --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9cd2f56b4..dbd4759de 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1945,9 +1945,9 @@ checksum = "f8dcc9c7d52a811697d2151c701e0d08956f92b0e24136cf4cf27b57a6a0d9bf" [[package]] name = "rand" -version = "0.9.2" +version = "0.9.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6db2770f06117d490610c7488547d543617b21bfa07796d7a12f6f1bd53850d1" +checksum = "44c5af06bb1b7d3216d91932aed5265164bf384dc89cd6ba05cf59a35f5f76ea" dependencies = [ "rand_chacha", "rand_core", From 82b18cc6d77ec15902e5e7c4cf74b23137409fcb Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Wed, 22 Apr 2026 18:57:59 -0700 Subject: [PATCH 31/44] fix typos and clarify new repo comment --- src/server/gitsync/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index a0a606032..0e3fb8515 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -10,7 +10,7 @@ //! - Metadata (`meta`) holds the latest version UUID and the encryption salt as JSON. //! //! After each write (`add_version`, `add_snapshot`) the server stages the changed files, -//! creates a commit, and pushes to the remote. If the push is rejected , thecommit is +//! creates a commit, and pushes to the remote. If the push is rejected, the commit is //! rolled back and the caller receives an [`AddVersionResult::ExpectedParentVersion`] //! or an [`Error`] so it can retry. //! @@ -20,7 +20,7 @@ //! //! Notes and Expectations //! -//! - Since this shells out to git, it assumes that you havea reasonably functional git +//! - Since this shells out to git, it assumes that you have a reasonably functional git //! setup. I.e. 'git init', 'git add', 'git commit', etc shoud just work. //! - If you are using a remote, 'git push' and 'git pull' shoud work. //! - Due to the nature of the version and snapshot history, you probably shouldn't do @@ -29,7 +29,7 @@ //! - If you are planning on using it for other things, it is HIGHLY recommended that you //! create a 'task' branch and let TaskChampion manage that branch. //! - This does support both defining a remote and having `local_only` mode set at the same -//! time. The idea is that maybe the remote isn't ready yet, or eithe rtemporarily or +//! time. The idea is that maybe the remote isn't ready yet, or either temporarily or //! permanantly down. Either way, you can use this in local mode in the mean time. //! - Remember, a remote can be on the same machine as local. This is used for testing. //! @@ -206,7 +206,7 @@ impl GitSyncServer { .status()? .success(); if !checkout_ok { - // For a brand-new repo `git checkout -b` also fails, so use + // For a repo with no commits yet, `git checkout -b` also fails, so use // `git symbolic-ref` to point HEAD at the desired branch without needing a commit. let has_commits = Command::new("git") .args(["rev-parse", "HEAD"]) From 6af28a9386f932457b7a89b8cd22a330064080fa Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Wed, 22 Apr 2026 19:09:21 -0700 Subject: [PATCH 32/44] two feature levels -> one --- Cargo.toml | 4 +--- src/server/config.rs | 8 ++++---- src/server/encryption.rs | 2 +- src/server/mod.rs | 2 +- 4 files changed, 7 insertions(+), 9 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 7e7e155d1..7760525af 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -49,7 +49,7 @@ server-aws = [ # Suppport for sync to another SQLite database on the same machine server-local = ["dep:rusqlite"] # Support for sync via Git -server-git = ["git-sync"] +server-git = ["dep:serde_with", "dep:glob", "encryption"] # Support for all task storage backends (except indexeddb, which only works on WASM builds) storage = ["storage-sqlite"] @@ -62,8 +62,6 @@ storage-indexeddb = ["dep:idb", "dep:web-sys"] encryption = ["dep:ring"] # (private) Generic support for cloud sync cloud = [] -# (private) Support for Git based sync -git-sync = ["dep:serde_with", "dep:glob", "encryption"] # static bundling of dependencies bundled = ["rusqlite/bundled"] # use CA roots built into the library for all HTTPS access diff --git a/src/server/config.rs b/src/server/config.rs index b2e8b360a..e4a48a6ff 100644 --- a/src/server/config.rs +++ b/src/server/config.rs @@ -8,7 +8,7 @@ use crate::server::cloud::aws::AwsService; use crate::server::cloud::gcp::GcpService; #[cfg(feature = "cloud")] use crate::server::cloud::CloudServer; -#[cfg(feature = "git-sync")] +#[cfg(feature = "server-git")] use crate::server::gitsync::GitSyncServer; #[cfg(feature = "server-local")] use crate::server::local::LocalServer; @@ -16,7 +16,7 @@ use crate::server::local::LocalServer; use crate::server::sync::SyncServer; #[cfg(feature = "server-local")] use std::path::PathBuf; -#[cfg(all(feature = "git-sync", not(feature = "server-local")))] +#[cfg(all(feature = "server-git", not(feature = "server-local")))] use std::path::PathBuf; #[cfg(feature = "server-sync")] use uuid::Uuid; @@ -123,7 +123,7 @@ pub enum ServerConfig { encryption_secret: Vec, }, /// A git repository - #[cfg(feature = "git-sync")] + #[cfg(feature = "server-git")] Git { /// The path to the local repo. local_path: PathBuf, @@ -186,7 +186,7 @@ impl ServerConfig { ) .await?, ), - #[cfg(feature = "git-sync")] + #[cfg(feature = "server-git")] ServerConfig::Git { local_path, branch, diff --git a/src/server/encryption.rs b/src/server/encryption.rs index 951a4fd24..cf0966f8f 100644 --- a/src/server/encryption.rs +++ b/src/server/encryption.rs @@ -27,7 +27,7 @@ impl Cryptor { } /// Generate a suitable random salt. - #[cfg(any(test, feature = "cloud", feature = "git-sync"))] // server-sync uses the clientId as the salt. + #[cfg(any(test, feature = "cloud", feature = "server-git"))] // server-sync uses the clientId as the salt. pub(super) fn gen_salt() -> Result> { let rng = rand::SystemRandom::new(); let mut salt = [0u8; 16]; diff --git a/src/server/mod.rs b/src/server/mod.rs index 3c5c93eca..0bc646c8c 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -31,7 +31,7 @@ mod sync; #[cfg(feature = "cloud")] mod cloud; -#[cfg(feature = "git-sync")] +#[cfg(feature = "server-git")] mod gitsync; pub use config::*; From c5362fb39b8812e31f7edce0dc1f800007cb3656 Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Wed, 22 Apr 2026 20:24:57 -0700 Subject: [PATCH 33/44] move remote from String to Option --- src/server/config.rs | 7 +++-- src/server/gitsync/mod.rs | 57 +++++++++++++++++++++++---------------- 2 files changed, 37 insertions(+), 27 deletions(-) diff --git a/src/server/config.rs b/src/server/config.rs index e4a48a6ff..2f575b2b3 100644 --- a/src/server/config.rs +++ b/src/server/config.rs @@ -132,10 +132,9 @@ pub enum ServerConfig { /// The remote repo. /// /// This can either be a named remote such as `origin` or a full git - /// url such as `git@myserver.com:/path/to/repo.git` - /// If `None` will use `origin` if the local repo has one. - /// Otherwise will operate in local only mode. - remote: String, + /// URL such as `git@myserver.com:/path/to/repo.git`. + /// If `None`, operates in local-only mode (no push/pull). + remote: Option, /// Don't clone/push/pull to `remote` even if it is defined. local_only: bool, /// Private encryption secret used to encrypt all data sent to the server. This can diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index 0e3fb8515..38327563a 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -89,15 +89,14 @@ struct Meta { /// A [`Server`] backed by a local git repository. /// -/// -/// When `local_only` is `false` the server pushes to and pulls from `remote` on the `branch` -/// branch after each write. Conflict resolution is handled as: commit locally, -/// attempt push, and on rejection pull-and-reset before returning. +/// When `remote` is `Some` and `local_only` is `false`, the server pushes to and pulls from +/// `remote` on the `branch` branch after each write. Conflict resolution is handled as: commit +/// locally, attempt push, and on rejection pull-and-reset before returning. pub(crate) struct GitSyncServer { meta: Meta, local_path: PathBuf, branch: String, - remote: String, + remote: Option, local_only: bool, cryptor: Cryptor, } @@ -141,11 +140,11 @@ impl GitSyncServer { pub(crate) fn new( local_path: PathBuf, branch: String, - remote: String, + remote: Option, local_only: bool, encryption_secret: Vec, ) -> Result { - let meta = Self::init_repo(&local_path, &branch, &remote, local_only)?; + let meta = Self::init_repo(&local_path, &branch, remote.as_deref(), local_only)?; let cryptor = Cryptor::new(&meta.salt, &encryption_secret.into())?; let server = GitSyncServer { meta, @@ -162,7 +161,12 @@ impl GitSyncServer { /// /// Creates the directory, initialises or clones the repo, switches to `branch`, and /// creates the `meta` file on first run. - fn init_repo(local_path: &Path, branch: &str, remote: &str, local_only: bool) -> Result { + fn init_repo( + local_path: &Path, + branch: &str, + remote: Option<&str>, + local_only: bool, + ) -> Result { // Create the local directory if needed. fs::create_dir_all(local_path)?; @@ -176,10 +180,11 @@ impl GitSyncServer { // Create one if not. if !is_repo { - if local_only { + if local_only || remote.is_none() { info!("Creating new repo at {:?}", local_path); git_cmd(local_path, &["init"])?; } else { + let remote = remote.unwrap(); info!("Cloning repo from {:?} to {:?}", remote, local_path); let parent = local_path .parent() @@ -276,9 +281,13 @@ impl GitSyncServer { Ok(meta_path) } - /// Fetch and fast-forward to the remote branch. No-op in local-only mode. - /// If the remote branch does not yet exist (e.g. fresh bare repo), this is also a no-op. + /// Fetch and fast-forward to the remote branch. No-op when there is no remote or in + /// local-only mode. If the remote branch does not yet exist (e.g. fresh bare repo), this is + /// also a no-op. fn pull(&self) -> Result<()> { + let Some(remote) = self.remote.as_deref() else { + return Ok(()); + }; if self.local_only { return Ok(()); } @@ -289,7 +298,7 @@ impl GitSyncServer { "ls-remote", "--exit-code", "--heads", - &self.remote, + remote, &self.branch, ]) .current_dir(&self.local_path) @@ -299,7 +308,7 @@ impl GitSyncServer { if !has_remote_branch { return Ok(()); } - git_cmd(&self.local_path, &["fetch", &self.remote, &self.branch])?; + git_cmd(&self.local_path, &["fetch", remote, &self.branch])?; git_cmd(&self.local_path, &["reset", "--hard", "FETCH_HEAD"])?; // Remove any untracked files left behind by interrupted writes. git_cmd( @@ -309,14 +318,17 @@ impl GitSyncServer { Ok(()) } - /// Push to the remote branch. Returns `true` on success, `false` if the push is rejected - /// Always returns `true` in local-only mode. + /// Push to the remote branch. Returns `true` on success, `false` if the push is rejected. + /// Returns `true` immediately when there is no remote or in local-only mode. fn push(&self) -> Result { + let Some(remote) = self.remote.as_deref() else { + return Ok(true); + }; if self.local_only { return Ok(true); } let output = Command::new("git") - .args(["push", &self.remote, &self.branch]) + .args(["push", remote, &self.branch]) .current_dir(&self.local_path) .output()?; if !output.status.success() { @@ -659,7 +671,7 @@ mod test { GitSyncServer::new( dir.to_path_buf(), "main".into(), - "".into(), + None, true, b"test-secret".to_vec(), ) @@ -681,7 +693,7 @@ mod test { GitSyncServer::new( clone_dir.to_path_buf(), "main".into(), - bare_url.into(), + Some(bare_url.to_string()), false, b"test-secret".to_vec(), ) @@ -746,11 +758,10 @@ mod test { // Build a server for clone2 and pull // It should see the new file. - let bare_url_str: String = bare_url.into(); let server2 = GitSyncServer::new( clone2.clone(), "main".into(), - bare_url_str, + Some(bare_url.to_string()), false, b"test-secret".to_vec(), )?; @@ -824,7 +835,7 @@ mod test { let mut server2 = GitSyncServer::new( clone2, "main".into(), - bare_url.into(), + Some(bare_url.to_string()), false, b"test-secret".to_vec(), )?; @@ -905,7 +916,7 @@ mod test { let mut server2 = GitSyncServer::new( clone2, "main".into(), - bare_url.into(), + Some(bare_url.to_string()), false, b"test-secret".to_vec(), )?; @@ -968,7 +979,7 @@ mod test { let mut server2 = GitSyncServer::new( clone2.clone(), "main".into(), - bare_url.into(), + Some(bare_url.to_string()), false, b"test-secret".to_vec(), )?; From a409c74296370ee97f0093ecd1ab1fd948780122 Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Thu, 23 Apr 2026 07:06:00 -0700 Subject: [PATCH 34/44] redirect git output to logs --- src/server/gitsync/mod.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index 38327563a..fcace9aef 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -118,13 +118,22 @@ fn parse_version_filename(name: &str) -> Option<(Uuid, Uuid)> { } /// Run a git command in a given directory, returning an error if it exits non-zero. +/// stdout and stderr are captured and forwarded to the log at debug level. fn git_cmd(dir: &Path, args: &[&str]) -> Result<()> { - let status = Command::new("git").args(args).current_dir(dir).status()?; - if !status.success() { + let output = Command::new("git").args(args).current_dir(dir).output()?; + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + if !stdout.is_empty() { + log::debug!("git {}: stdout: {}", args.join(" "), stdout.trim_end()); + } + if !stderr.is_empty() { + log::debug!("git {}: stderr: {}", args.join(" "), stderr.trim_end()); + } + if !output.status.success() { return Err(Error::Server(format!( "git {} failed with status {}", args.join(" "), - status + output.status ))); } Ok(()) From 568493910b4c1df5b4f49ff0481c1f9b60a7a9b7 Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Thu, 23 Apr 2026 15:03:23 -0700 Subject: [PATCH 35/44] Split git_cmd into git_cmd_ok to simplify calls. --- src/server/gitsync/mod.rs | 57 ++++++++++++--------------------------- 1 file changed, 17 insertions(+), 40 deletions(-) diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index fcace9aef..08d335f5e 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -117,9 +117,10 @@ fn parse_version_filename(name: &str) -> Option<(Uuid, Uuid)> { Some((parent_id, child_id)) } -/// Run a git command in a given directory, returning an error if it exits non-zero. +/// Run a git command, returning `Ok(true)` on success and `Ok(false)` on a non-zero exit. /// stdout and stderr are captured and forwarded to the log at debug level. -fn git_cmd(dir: &Path, args: &[&str]) -> Result<()> { +/// Returns `Err` only on I/O failure (e.g. git binary not found). +fn git_cmd_ok(dir: &Path, args: &[&str]) -> Result { let output = Command::new("git").args(args).current_dir(dir).output()?; let stdout = String::from_utf8_lossy(&output.stdout); let stderr = String::from_utf8_lossy(&output.stderr); @@ -129,12 +130,13 @@ fn git_cmd(dir: &Path, args: &[&str]) -> Result<()> { if !stderr.is_empty() { log::debug!("git {}: stderr: {}", args.join(" "), stderr.trim_end()); } - if !output.status.success() { - return Err(Error::Server(format!( - "git {} failed with status {}", - args.join(" "), - output.status - ))); + Ok(output.status.success()) +} + +/// Run a git command, returning an error if it exits non-zero. +fn git_cmd(dir: &Path, args: &[&str]) -> Result<()> { + if !git_cmd_ok(dir, args)? { + return Err(Error::Server(format!("git {} failed", args.join(" ")))); } Ok(()) } @@ -180,12 +182,7 @@ impl GitSyncServer { fs::create_dir_all(local_path)?; // Check if path is already a git repo. - let is_repo = Command::new("git") - .arg("rev-parse") - .current_dir(local_path) - .output()? - .status - .success(); + let is_repo = git_cmd_ok(local_path, &["rev-parse"])?; // Create one if not. if !is_repo { @@ -213,21 +210,10 @@ impl GitSyncServer { // Switch to the requested branch. Try checking out an existing branch first, // only create a new one if that fails. info!("Switching branch to {:?}", branch); - let checkout_ok = Command::new("git") - .args(["checkout", branch]) - .current_dir(local_path) - .stderr(std::process::Stdio::null()) - .status()? - .success(); - if !checkout_ok { + if !git_cmd_ok(local_path, &["checkout", branch])? { // For a repo with no commits yet, `git checkout -b` also fails, so use // `git symbolic-ref` to point HEAD at the desired branch without needing a commit. - let has_commits = Command::new("git") - .args(["rev-parse", "HEAD"]) - .current_dir(local_path) - .stderr(std::process::Stdio::null()) - .status()? - .success(); + let has_commits = git_cmd_ok(local_path, &["rev-parse", "HEAD"])?; if has_commits { git_cmd(local_path, &["checkout", "-b", branch])?; } else { @@ -302,19 +288,10 @@ impl GitSyncServer { } // Check whether the remote branch exists before fetching. A bare repo with no commits // has no refs yet, and `git fetch origin ` would fail in that case. - let has_remote_branch = Command::new("git") - .args([ - "ls-remote", - "--exit-code", - "--heads", - remote, - &self.branch, - ]) - .current_dir(&self.local_path) - .stderr(std::process::Stdio::null()) - .status()? - .success(); - if !has_remote_branch { + if !git_cmd_ok( + &self.local_path, + &["ls-remote", "--exit-code", "--heads", remote, &self.branch], + )? { return Ok(()); } git_cmd(&self.local_path, &["fetch", remote, &self.branch])?; From f1af20117f25625924515d61f41d30cccf7edc8e Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Thu, 23 Apr 2026 15:28:15 -0700 Subject: [PATCH 36/44] add some helper fns --- src/server/gitsync/mod.rs | 62 ++++++++++++++++++++------------------- 1 file changed, 32 insertions(+), 30 deletions(-) diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index 08d335f5e..5d07b196a 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -141,6 +141,22 @@ fn git_cmd(dir: &Path, args: &[&str]) -> Result<()> { Ok(()) } +/// Remove untracked TaskChampion files left behind by interrupted writes. +fn clean_stray_files(dir: &Path) -> Result<()> { + git_cmd(dir, &["clean", "-f", "--", "v-*", "snapshot", "meta"]) +} + +/// Stage each path and create a commit in `dir`. +fn stage_and_commit_files(dir: &Path, paths: &[&Path], message: &str) -> Result<()> { + for path in paths { + let path_str = path + .to_str() + .ok_or_else(|| Error::Server("path is not valid UTF-8".into()))?; + git_cmd(dir, &["add", path_str])?; + } + git_cmd(dir, &["commit", "-m", message]) +} + impl GitSyncServer { /// Create or re-open a git-backed sync server. /// @@ -235,31 +251,20 @@ impl GitSyncServer { }; let f = File::create_new(&meta_path)?; serde_json::to_writer(f, &m)?; - git_cmd(local_path, &["add", "meta"])?; - git_cmd(local_path, &["commit", "-m", "init taskchampion repo"])?; + stage_and_commit_files(local_path, &[&meta_path], "init taskchampion repo")?; m } }; // Remove any untracked files left behind by interrupted writes. - git_cmd( - local_path, - &["clean", "-f", "--", "v-*", "snapshot", "meta"], - )?; + clean_stray_files(local_path)?; Ok(meta) } /// Stage the given paths and create a commit. fn stage_and_commit(&self, paths: &[&Path], message: &str) -> Result<()> { - for path in paths { - let path_str = path - .to_str() - .ok_or_else(|| Error::Server("path is not valid UTF-8".into()))?; - git_cmd(&self.local_path, &["add", path_str])?; - } - git_cmd(&self.local_path, &["commit", "-m", message])?; - Ok(()) + stage_and_commit_files(&self.local_path, paths, message) } /// Read the meta file from disk and update self.meta. @@ -276,10 +281,10 @@ impl GitSyncServer { Ok(meta_path) } - /// Fetch and fast-forward to the remote branch. No-op when there is no remote or in - /// local-only mode. If the remote branch does not yet exist (e.g. fresh bare repo), this is - /// also a no-op. - fn pull(&self) -> Result<()> { + /// Fetch from remote and hard-reset the local branch to match. No-op when there is no remote + /// or in local-only mode. If the remote branch does not yet exist (e.g. fresh bare repo), + /// this is also a no-op. + fn reset_to_remote(&self) -> Result<()> { let Some(remote) = self.remote.as_deref() else { return Ok(()); }; @@ -297,10 +302,7 @@ impl GitSyncServer { git_cmd(&self.local_path, &["fetch", remote, &self.branch])?; git_cmd(&self.local_path, &["reset", "--hard", "FETCH_HEAD"])?; // Remove any untracked files left behind by interrupted writes. - git_cmd( - &self.local_path, - &["clean", "-f", "--", "v-*", "snapshot", "meta"], - )?; + clean_stray_files(&self.local_path)?; Ok(()) } @@ -511,7 +513,7 @@ impl GitSyncServer { // on the next snapshot. Reset to remote state so we are in sync. if !self.push()? { log::info!("cleanup: push rejected, resetting to remote state"); - self.pull()?; + self.reset_to_remote()?; } Ok(()) @@ -529,7 +531,7 @@ impl Server for GitSyncServer { // Otherwise check if parent is in latest. If it isn't, pull recheck. if self.meta.latest_version != Uuid::nil() && parent_version_id != self.meta.latest_version { - self.pull()?; + self.reset_to_remote()?; self.read_meta()?; if parent_version_id != self.meta.latest_version { return Ok(( @@ -557,7 +559,7 @@ impl Server for GitSyncServer { // Push was rejected. Undo the commit; pull will reset --hard and git clean // away the stray version file. git_cmd(&self.local_path, &["reset", "HEAD~1", "--soft"])?; - self.pull()?; + self.reset_to_remote()?; self.read_meta()?; return Ok(( AddVersionResult::ExpectedParentVersion(self.meta.latest_version), @@ -579,7 +581,7 @@ impl Server for GitSyncServer { history_segment: v.history_segment, }); } - self.pull()?; + self.reset_to_remote()?; match self.get_version_by_parent_version_id(&parent_version_id)? { Some(v) => Ok(GetVersionResult::Version { version_id: v.version_id, @@ -591,7 +593,7 @@ impl Server for GitSyncServer { } async fn add_snapshot(&mut self, version_id: VersionId, snapshot: Snapshot) -> Result<()> { - self.pull()?; + self.reset_to_remote()?; // Write the snapshot to a file. // Note: If another replica has pushed a snapshot for a // later version in the chain between our pull and our push, we will overwrite it. @@ -616,7 +618,7 @@ impl Server for GitSyncServer { if !self.push()? { // Push was rejected. Undo the commit, pull and clean to restore state. git_cmd(&self.local_path, &["reset", "HEAD~1", "--soft"])?; - self.pull()?; + self.reset_to_remote()?; self.read_meta()?; return Err(Error::Server("Couldn't push to remote.".into())); } @@ -630,7 +632,7 @@ impl Server for GitSyncServer { } async fn get_snapshot(&mut self) -> Result> { - self.pull()?; + self.reset_to_remote()?; let snapshot_path = self.local_path.join("snapshot"); if let Ok(file) = File::open(&snapshot_path) { @@ -751,7 +753,7 @@ mod test { false, b"test-secret".to_vec(), )?; - server2.pull()?; + server2.reset_to_remote()?; assert!(clone2.join("testfile").exists()); Ok(()) } From 4a3cc694cc7e5a8418b4040ce3ed394df6917c35 Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Thu, 23 Apr 2026 16:05:27 -0700 Subject: [PATCH 37/44] create a git struct to hold the git helpers and the git path --- src/server/config.rs | 4 + src/server/gitsync/mod.rs | 264 +++++++++++++++++++++++++------------- 2 files changed, 179 insertions(+), 89 deletions(-) diff --git a/src/server/config.rs b/src/server/config.rs index 2f575b2b3..da6e7300f 100644 --- a/src/server/config.rs +++ b/src/server/config.rs @@ -140,6 +140,8 @@ pub enum ServerConfig { /// Private encryption secret used to encrypt all data sent to the server. This can /// be any suitably un-guessable string of bytes. encryption_secret: Vec, + /// Path to the git binary. If `None`, `"git"` on PATH is used. + git_path: Option, }, } @@ -192,12 +194,14 @@ impl ServerConfig { remote, local_only, encryption_secret, + git_path, } => Box::new(GitSyncServer::new( local_path, branch, remote, local_only, encryption_secret, + git_path, )?), }) } diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index 5d07b196a..6249c137c 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -55,6 +55,7 @@ use std::fs::{self, File}; use std::io::{BufReader, Read}; use std::path::{Path, PathBuf}; use std::process::Command; +use std::time::{Duration, SystemTime, UNIX_EPOCH}; use uuid::Uuid; /// A version record; used as an in-memory carrier between read/write helpers. @@ -93,12 +94,15 @@ struct Meta { /// `remote` on the `branch` branch after each write. Conflict resolution is handled as: commit /// locally, attempt push, and on rejection pull-and-reset before returning. pub(crate) struct GitSyncServer { + git: Git, meta: Meta, local_path: PathBuf, branch: String, remote: Option, local_only: bool, cryptor: Cryptor, + /// Minimum age a version file must reach before cleanup() will remove it. + version_retention: Duration, } /// Load and deserialise a [`Meta`] from the given path. @@ -117,44 +121,97 @@ fn parse_version_filename(name: &str) -> Option<(Uuid, Uuid)> { Some((parent_id, child_id)) } -/// Run a git command, returning `Ok(true)` on success and `Ok(false)` on a non-zero exit. -/// stdout and stderr are captured and forwarded to the log at debug level. -/// Returns `Err` only on I/O failure (e.g. git binary not found). -fn git_cmd_ok(dir: &Path, args: &[&str]) -> Result { - let output = Command::new("git").args(args).current_dir(dir).output()?; - let stdout = String::from_utf8_lossy(&output.stdout); - let stderr = String::from_utf8_lossy(&output.stderr); - if !stdout.is_empty() { - log::debug!("git {}: stdout: {}", args.join(" "), stdout.trim_end()); +const VERSION_RETENTION: Duration = Duration::from_secs(180 * 24 * 60 * 60); + +/// Thin wrapper around a git binary path that provides the command helpers used throughout +/// this module. Defaults to `"git"` on PATH when constructed with `Git::new(None)`. +struct Git { + path: PathBuf, +} + +impl Git { + fn new(path: Option) -> Self { + Git { + path: path.unwrap_or_else(|| PathBuf::from("git")), + } } - if !stderr.is_empty() { - log::debug!("git {}: stderr: {}", args.join(" "), stderr.trim_end()); + + /// Run a git command, returning `Ok(true)` on success and `Ok(false)` on a non-zero exit. + /// stdout and stderr are captured and forwarded to the log at debug level. + /// Returns `Err` only on I/O failure (e.g. git binary not found). + fn cmd_ok(&self, dir: &Path, args: &[&str]) -> Result { + let output = Command::new(&self.path) + .args(args) + .current_dir(dir) + .output()?; + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + if !stdout.is_empty() { + log::debug!("git {}: stdout: {}", args.join(" "), stdout.trim_end()); + } + if !stderr.is_empty() { + log::debug!("git {}: stderr: {}", args.join(" "), stderr.trim_end()); + } + Ok(output.status.success()) } - Ok(output.status.success()) -} -/// Run a git command, returning an error if it exits non-zero. -fn git_cmd(dir: &Path, args: &[&str]) -> Result<()> { - if !git_cmd_ok(dir, args)? { - return Err(Error::Server(format!("git {} failed", args.join(" ")))); + /// Run a git command, returning an error if it exits non-zero. + fn cmd(&self, dir: &Path, args: &[&str]) -> Result<()> { + if !self.cmd_ok(dir, args)? { + return Err(Error::Server(format!("git {} failed", args.join(" ")))); + } + Ok(()) } - Ok(()) -} -/// Remove untracked TaskChampion files left behind by interrupted writes. -fn clean_stray_files(dir: &Path) -> Result<()> { - git_cmd(dir, &["clean", "-f", "--", "v-*", "snapshot", "meta"]) -} + /// Run a git command and return its trimmed stdout. Returns an error if the command exits + /// non-zero. Useful for commands like `git log --format=...` that produce structured output. + fn output(&self, dir: &Path, args: &[&str]) -> Result { + let output = Command::new(&self.path) + .args(args) + .current_dir(dir) + .output()?; + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + if !stderr.is_empty() { + log::debug!("git {}: stderr: {}", args.join(" "), stderr.trim_end()); + } + if !output.status.success() { + return Err(Error::Server(format!("git {} failed", args.join(" ")))); + } + Ok(stdout.trim_end().to_string()) + } -/// Stage each path and create a commit in `dir`. -fn stage_and_commit_files(dir: &Path, paths: &[&Path], message: &str) -> Result<()> { - for path in paths { - let path_str = path - .to_str() - .ok_or_else(|| Error::Server("path is not valid UTF-8".into()))?; - git_cmd(dir, &["add", path_str])?; + /// Remove untracked TaskChampion files left behind by interrupted writes. + fn clean_stray_files(&self, dir: &Path) -> Result<()> { + self.cmd(dir, &["clean", "-f", "--", "v-*", "snapshot", "meta"]) + } + + /// Stage each path and create a commit in `dir`. + fn stage_and_commit_files(&self, dir: &Path, paths: &[&Path], message: &str) -> Result<()> { + for path in paths { + let path_str = path + .to_str() + .ok_or_else(|| Error::Server("path is not valid UTF-8".into()))?; + self.cmd(dir, &["add", path_str])?; + } + self.cmd(dir, &["commit", "-m", message]) + } + + /// Return how long ago `filename` was last committed in `dir`, or `None` if git has no + /// record of it. A missing record is treated as "keep". + fn version_file_age(&self, dir: &Path, filename: &str) -> Result> { + let out = self.output(dir, &["log", "-1", "--format=%ct", "--", filename])?; + if out.is_empty() { + return Ok(None); + } + let commit_ts: u64 = out + .parse() + .map_err(|_| Error::Server(format!("unexpected git log output: {out:?}")))?; + let now = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_err(|e| Error::Server(format!("system clock error: {e}")))?; + Ok(Some(now.saturating_sub(Duration::from_secs(commit_ts)))) } - git_cmd(dir, &["commit", "-m", message]) } impl GitSyncServer { @@ -170,16 +227,20 @@ impl GitSyncServer { remote: Option, local_only: bool, encryption_secret: Vec, + git_path: Option, ) -> Result { - let meta = Self::init_repo(&local_path, &branch, remote.as_deref(), local_only)?; + let git = Git::new(git_path); + let meta = Self::init_repo(&git, &local_path, &branch, remote.as_deref(), local_only)?; let cryptor = Cryptor::new(&meta.salt, &encryption_secret.into())?; let server = GitSyncServer { + git, meta, local_path, branch, remote, local_only, cryptor, + version_retention: VERSION_RETENTION, }; Ok(server) } @@ -189,6 +250,7 @@ impl GitSyncServer { /// Creates the directory, initialises or clones the repo, switches to `branch`, and /// creates the `meta` file on first run. fn init_repo( + git: &Git, local_path: &Path, branch: &str, remote: Option<&str>, @@ -198,42 +260,44 @@ impl GitSyncServer { fs::create_dir_all(local_path)?; // Check if path is already a git repo. - let is_repo = git_cmd_ok(local_path, &["rev-parse"])?; + let is_repo = git.cmd_ok(local_path, &["rev-parse"])?; // Create one if not. if !is_repo { - if local_only || remote.is_none() { - info!("Creating new repo at {:?}", local_path); - git_cmd(local_path, &["init"])?; - } else { - let remote = remote.unwrap(); - info!("Cloning repo from {:?} to {:?}", remote, local_path); - let parent = local_path - .parent() - .ok_or_else(|| Error::Server("local_path has no parent".into()))?; - let dir_name = local_path - .file_name() - .ok_or_else(|| Error::Server("local_path has no file name".into()))? - .to_str() - .ok_or_else(|| Error::Server("local_path is not valid UTF-8".into()))?; - git_cmd(parent, &["clone", remote, dir_name])?; + match (local_only, remote) { + (false, Some(remote)) => { + info!("Cloning repo from {:?} to {:?}", remote, local_path); + let parent = local_path + .parent() + .ok_or_else(|| Error::Server("local_path has no parent".into()))?; + let dir_name = local_path + .file_name() + .ok_or_else(|| Error::Server("local_path has no file name".into()))? + .to_str() + .ok_or_else(|| Error::Server("local_path is not valid UTF-8".into()))?; + git.cmd(parent, &["clone", remote, dir_name])?; + } + _ => { + info!("Creating new repo at {:?}", local_path); + git.cmd(local_path, &["init"])?; + } } // Set identity so commits work in environments without a global git config. - git_cmd(local_path, &["config", "user.email", "taskchampion@local"])?; - git_cmd(local_path, &["config", "user.name", "taskchampion"])?; + git.cmd(local_path, &["config", "user.email", "taskchampion@local"])?; + git.cmd(local_path, &["config", "user.name", "taskchampion"])?; } // Switch to the requested branch. Try checking out an existing branch first, // only create a new one if that fails. info!("Switching branch to {:?}", branch); - if !git_cmd_ok(local_path, &["checkout", branch])? { + if !git.cmd_ok(local_path, &["checkout", branch])? { // For a repo with no commits yet, `git checkout -b` also fails, so use // `git symbolic-ref` to point HEAD at the desired branch without needing a commit. - let has_commits = git_cmd_ok(local_path, &["rev-parse", "HEAD"])?; + let has_commits = git.cmd_ok(local_path, &["rev-parse", "HEAD"])?; if has_commits { - git_cmd(local_path, &["checkout", "-b", branch])?; + git.cmd(local_path, &["checkout", "-b", branch])?; } else { - git_cmd( + git.cmd( local_path, &["symbolic-ref", "HEAD", &format!("refs/heads/{}", branch)], )?; @@ -251,20 +315,21 @@ impl GitSyncServer { }; let f = File::create_new(&meta_path)?; serde_json::to_writer(f, &m)?; - stage_and_commit_files(local_path, &[&meta_path], "init taskchampion repo")?; + git.stage_and_commit_files(local_path, &[&meta_path], "init taskchampion repo")?; m } }; // Remove any untracked files left behind by interrupted writes. - clean_stray_files(local_path)?; + git.clean_stray_files(local_path)?; Ok(meta) } /// Stage the given paths and create a commit. fn stage_and_commit(&self, paths: &[&Path], message: &str) -> Result<()> { - stage_and_commit_files(&self.local_path, paths, message) + self.git + .stage_and_commit_files(&self.local_path, paths, message) } /// Read the meta file from disk and update self.meta. @@ -293,16 +358,26 @@ impl GitSyncServer { } // Check whether the remote branch exists before fetching. A bare repo with no commits // has no refs yet, and `git fetch origin ` would fail in that case. - if !git_cmd_ok( + if !self.git.cmd_ok( &self.local_path, &["ls-remote", "--exit-code", "--heads", remote, &self.branch], )? { return Ok(()); } - git_cmd(&self.local_path, &["fetch", remote, &self.branch])?; - git_cmd(&self.local_path, &["reset", "--hard", "FETCH_HEAD"])?; + // Warn if there are uncommitted changes that the hard reset will discard. This should + // only happen if a previous write was interrupted; clean_stray_files handles the fallout. + if !self + .git + .cmd_ok(&self.local_path, &["diff", "--quiet", "HEAD"])? + { + log::warn!("reset_to_remote: discarding uncommitted local changes"); + } + self.git + .cmd(&self.local_path, &["fetch", remote, &self.branch])?; + self.git + .cmd(&self.local_path, &["reset", "--hard", "FETCH_HEAD"])?; // Remove any untracked files left behind by interrupted writes. - clean_stray_files(&self.local_path)?; + self.git.clean_stray_files(&self.local_path)?; Ok(()) } @@ -315,17 +390,8 @@ impl GitSyncServer { if self.local_only { return Ok(true); } - let output = Command::new("git") - .args(["push", remote, &self.branch]) - .current_dir(&self.local_path) - .output()?; - if !output.status.success() { - log::debug!( - "git push failed: {}", - String::from_utf8_lossy(&output.stderr) - ); - } - Ok(output.status.success()) + self.git + .cmd_ok(&self.local_path, &["push", remote, &self.branch]) } /// Encrypt and write a version file named `v-{parent_version_id}-{version_id}`. @@ -475,7 +541,9 @@ impl GitSyncServer { } } - // Remove each covered version file from the git index and working tree. + // Remove each covered version file from the git index and working tree, but only if it + // is older than VERSION_RETENTION. Recent files are kept so that a corrupt snapshot can + // still be recovered by replaying recent history. // If any `git rm` fails partway through we `git reset HEAD` to unstage, leaving // the working tree dirty but the index clean for subsequent operations. let mut any_removed = false; @@ -485,14 +553,18 @@ impl GitSyncServer { let name = path.file_name().and_then(|n| n.to_str()).ok_or_else(|| { Error::Server("version file path is not valid UTF-8".into()) })?; - git_cmd(&self.local_path, &["rm", name])?; + match self.git.version_file_age(&self.local_path, name)? { + Some(age) if age >= self.version_retention => {} + _ => continue, // Too recent or unknown age — keep it. + } + self.git.cmd(&self.local_path, &["rm", name])?; any_removed = true; } } Ok(()) })(); if let Err(e) = rm_result { - let _ = git_cmd(&self.local_path, &["reset", "HEAD"]); + let _ = self.git.cmd(&self.local_path, &["reset", "HEAD"]); return Err(e); } @@ -500,7 +572,7 @@ impl GitSyncServer { return Ok(()); } - git_cmd( + self.git.cmd( &self.local_path, &[ "commit", @@ -558,7 +630,8 @@ impl Server for GitSyncServer { if !self.push()? { // Push was rejected. Undo the commit; pull will reset --hard and git clean // away the stray version file. - git_cmd(&self.local_path, &["reset", "HEAD~1", "--soft"])?; + self.git + .cmd(&self.local_path, &["reset", "HEAD~1", "--soft"])?; self.reset_to_remote()?; self.read_meta()?; return Ok(( @@ -617,7 +690,8 @@ impl Server for GitSyncServer { if !self.push()? { // Push was rejected. Undo the commit, pull and clean to restore state. - git_cmd(&self.local_path, &["reset", "HEAD~1", "--soft"])?; + self.git + .cmd(&self.local_path, &["reset", "HEAD~1", "--soft"])?; self.reset_to_remote()?; self.read_meta()?; return Err(Error::Server("Couldn't push to remote.".into())); @@ -656,20 +730,23 @@ mod test { use tempfile::TempDir; fn make_server(dir: &Path) -> Result { - GitSyncServer::new( + let mut s = GitSyncServer::new( dir.to_path_buf(), "main".into(), None, true, b"test-secret".to_vec(), - ) + None, + )?; + s.version_retention = Duration::ZERO; + Ok(s) } /// Create a bare repo to act as a remote, then clone it into `clone_dir`. /// Returns a GitSyncServer backed by the clone, with the bare repo as its remote. fn make_server_with_remote(bare_dir: &Path, clone_dir: &Path) -> Result { // Initialise the bare remote. - git_cmd( + Git::new(None).cmd( bare_dir.parent().unwrap(), &[ "init", @@ -678,13 +755,16 @@ mod test { ], )?; let bare_url = bare_dir.to_str().unwrap(); - GitSyncServer::new( + let mut s = GitSyncServer::new( clone_dir.to_path_buf(), "main".into(), Some(bare_url.to_string()), false, b"test-secret".to_vec(), - ) + None, + )?; + s.version_retention = Duration::ZERO; + Ok(s) } #[test] @@ -734,9 +814,10 @@ mod test { // Clone a second copy directly via git. let bare_url = bare.to_str().unwrap(); - git_cmd(tmp.path(), &["clone", bare_url, "clone2"])?; - git_cmd(&clone2, &["config", "user.email", "taskchampion@local"])?; - git_cmd(&clone2, &["config", "user.name", "taskchampion"])?; + let git = Git::new(None); + git.cmd(tmp.path(), &["clone", bare_url, "clone2"])?; + git.cmd(&clone2, &["config", "user.email", "taskchampion@local"])?; + git.cmd(&clone2, &["config", "user.name", "taskchampion"])?; // Write a new file in clone1 and push it. let new_file = clone1.join("testfile"); @@ -752,6 +833,7 @@ mod test { Some(bare_url.to_string()), false, b"test-secret".to_vec(), + None, )?; server2.reset_to_remote()?; assert!(clone2.join("testfile").exists()); @@ -817,15 +899,17 @@ mod test { server1.push()?; let bare_url = bare.to_str().unwrap(); - git_cmd(tmp.path(), &["clone", bare_url, "clone2"])?; - git_cmd(&clone2, &["config", "user.email", "taskchampion@local"])?; - git_cmd(&clone2, &["config", "user.name", "taskchampion"])?; + let git = Git::new(None); + git.cmd(tmp.path(), &["clone", bare_url, "clone2"])?; + git.cmd(&clone2, &["config", "user.email", "taskchampion@local"])?; + git.cmd(&clone2, &["config", "user.name", "taskchampion"])?; let mut server2 = GitSyncServer::new( clone2, "main".into(), Some(bare_url.to_string()), false, b"test-secret".to_vec(), + None, )?; // server1 adds a version from NIL parent and pushes successfully. @@ -907,6 +991,7 @@ mod test { Some(bare_url.to_string()), false, b"test-secret".to_vec(), + None, )?; // get_child_version should pull and find the version. @@ -970,6 +1055,7 @@ mod test { Some(bare_url.to_string()), false, b"test-secret".to_vec(), + None, )?; let result = server2.get_snapshot().await?; assert!(result.is_some(), "expected snapshot from remote"); From f641b679dd759862428faa015e5a57fd43d3bd87 Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Thu, 23 Apr 2026 17:00:53 -0700 Subject: [PATCH 38/44] lots of documentaion cleanup --- src/server/config.rs | 17 ++++- src/server/gitsync/mod.rs | 142 ++++++++++++++++---------------------- 2 files changed, 76 insertions(+), 83 deletions(-) diff --git a/src/server/config.rs b/src/server/config.rs index da6e7300f..4fd81ebef 100644 --- a/src/server/config.rs +++ b/src/server/config.rs @@ -122,7 +122,22 @@ pub enum ServerConfig { /// be any suitably un-guessable string of bytes. encryption_secret: Vec, }, - /// A git repository + /// A git repository used as a sync server. + /// + /// TaskChampion manages the repository. Version history and snapshots are stored as committed + /// files. Do not merge, squash, or otherwise rewrite its history outside of TaskChampion. + /// + /// If the repository will be used for other things, it is HIGHLY recommended that you + /// configure a dedicated TaskChampion branch. + /// + /// This backend shells out to git, so a working `git` installation is required. For + /// remote sync, `git push` and `git pull` must work without prompts. Use SSH keys or a + /// credential helper. + /// + /// Setting `local_only` to `true` while `remote` is `Some` lets the replica operate + /// offline. Re-enable sync later by setting `local_only` back to `false`. However, be aware + /// that if the remote has diverged enough that a fast-forward fails, you will need to fix it + /// manually. #[cfg(feature = "server-git")] Git { /// The path to the local repo. diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index 6249c137c..d962f4a3c 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -14,30 +14,10 @@ //! rolled back and the caller receives an [`AddVersionResult::ExpectedParentVersion`] //! or an [`Error`] so it can retry. //! -//! After a snapshot is stored, [`GitSyncServer::cleanup`] automatically removes all -//! version files whose history is now captured by the snapshot, keeping the repository -//! compact. +//! After a snapshot is stored, [`GitSyncServer::cleanup`] automatically removes version +//! files whose history is captured by the snapshot and that are older than the retention +//! period, keeping the repository compact. //! -//! Notes and Expectations -//! -//! - Since this shells out to git, it assumes that you have a reasonably functional git -//! setup. I.e. 'git init', 'git add', 'git commit', etc shoud just work. -//! - If you are using a remote, 'git push' and 'git pull' shoud work. -//! - Due to the nature of the version and snapshot history, you probably shouldn't do -//! a lot of the things you normally would with a git repo, like merge, squash, etc. -//! Just let TaskChampion manage it. -//! - If you are planning on using it for other things, it is HIGHLY recommended that you -//! create a 'task' branch and let TaskChampion manage that branch. -//! - This does support both defining a remote and having `local_only` mode set at the same -//! time. The idea is that maybe the remote isn't ready yet, or either temporarily or -//! permanantly down. Either way, you can use this in local mode in the mean time. -//! - Remember, a remote can be on the same machine as local. This is used for testing. -//! -//! Notes for Reviewers -//! -//! - I haven't done any performance testing, but it seems reasonably quick for manual use. -//! - Currently is uses the same salt for all files. This isn't great security practice, -//! but does seem to be what the other servers are doing. use crate::errors::Result; use crate::server::encryption::{Cryptor, Sealed, Unsealed}; use crate::server::{ @@ -58,7 +38,7 @@ use std::process::Command; use std::time::{Duration, SystemTime, UNIX_EPOCH}; use uuid::Uuid; -/// A version record; used as an in-memory carrier between read/write helpers. +/// A version record, used as an in-memory carrier between read/write helpers. #[derive(Debug)] struct Version { version_id: VersionId, @@ -84,6 +64,7 @@ struct SnapshotFile { struct Meta { #[serde(with = "uuid::serde::simple")] latest_version: VersionId, + // A single salt is used for all files in this repo, consistent with other server backends. #[serde_as(as = "Base64")] salt: Vec, } @@ -93,6 +74,10 @@ struct Meta { /// When `remote` is `Some` and `local_only` is `false`, the server pushes to and pulls from /// `remote` on the `branch` branch after each write. Conflict resolution is handled as: commit /// locally, attempt push, and on rejection pull-and-reset before returning. +/// +/// Setting `local_only` to `true` while `remote` is `Some` allows the repository to operate +/// offline temporarily (e.g. when the remote is unreachable), and later re-enable sync by +/// setting `local_only` back to `false`. pub(crate) struct GitSyncServer { git: Git, meta: Meta, @@ -181,13 +166,8 @@ impl Git { Ok(stdout.trim_end().to_string()) } - /// Remove untracked TaskChampion files left behind by interrupted writes. - fn clean_stray_files(&self, dir: &Path) -> Result<()> { - self.cmd(dir, &["clean", "-f", "--", "v-*", "snapshot", "meta"]) - } - /// Stage each path and create a commit in `dir`. - fn stage_and_commit_files(&self, dir: &Path, paths: &[&Path], message: &str) -> Result<()> { + fn stage_and_commit(&self, dir: &Path, paths: &[&Path], message: &str) -> Result<()> { for path in paths { let path_str = path .to_str() @@ -196,22 +176,27 @@ impl Git { } self.cmd(dir, &["commit", "-m", message]) } +} - /// Return how long ago `filename` was last committed in `dir`, or `None` if git has no - /// record of it. A missing record is treated as "keep". - fn version_file_age(&self, dir: &Path, filename: &str) -> Result> { - let out = self.output(dir, &["log", "-1", "--format=%ct", "--", filename])?; - if out.is_empty() { - return Ok(None); - } - let commit_ts: u64 = out - .parse() - .map_err(|_| Error::Server(format!("unexpected git log output: {out:?}")))?; - let now = SystemTime::now() - .duration_since(UNIX_EPOCH) - .map_err(|e| Error::Server(format!("system clock error: {e}")))?; - Ok(Some(now.saturating_sub(Duration::from_secs(commit_ts)))) +/// Remove untracked TaskChampion files left behind by interrupted writes. +fn clean_stray_files(git: &Git, dir: &Path) -> Result<()> { + git.cmd(dir, &["clean", "-f", "--", "v-*", "snapshot", "meta"]) +} + +/// Return how long ago `filename` was last committed in `dir`, or `None` if git has no +/// record of it. A missing record is treated as "keep". +fn version_file_age(git: &Git, dir: &Path, filename: &str) -> Result> { + let out = git.output(dir, &["log", "-1", "--format=%ct", "--", filename])?; + if out.is_empty() { + return Ok(None); } + let commit_ts: u64 = out + .parse() + .map_err(|_| Error::Server(format!("unexpected git log output: {out:?}")))?; + let now = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_err(|e| Error::Server(format!("system clock error: {e}")))?; + Ok(Some(now.saturating_sub(Duration::from_secs(commit_ts)))) } impl GitSyncServer { @@ -291,12 +276,10 @@ impl GitSyncServer { // only create a new one if that fails. info!("Switching branch to {:?}", branch); if !git.cmd_ok(local_path, &["checkout", branch])? { - // For a repo with no commits yet, `git checkout -b` also fails, so use - // `git symbolic-ref` to point HEAD at the desired branch without needing a commit. - let has_commits = git.cmd_ok(local_path, &["rev-parse", "HEAD"])?; - if has_commits { - git.cmd(local_path, &["checkout", "-b", branch])?; - } else { + // Branch doesn't exist yet. Create it. + // checkout -b also fails if HEAD already points to this branch name (e.g. a + // fresh git init whose default branch matches), so fall back to symbolic-ref. + if !git.cmd_ok(local_path, &["checkout", "-b", branch])? { git.cmd( local_path, &["symbolic-ref", "HEAD", &format!("refs/heads/{}", branch)], @@ -315,23 +298,17 @@ impl GitSyncServer { }; let f = File::create_new(&meta_path)?; serde_json::to_writer(f, &m)?; - git.stage_and_commit_files(local_path, &[&meta_path], "init taskchampion repo")?; + git.stage_and_commit(local_path, &[&meta_path], "init taskchampion repo")?; m } }; // Remove any untracked files left behind by interrupted writes. - git.clean_stray_files(local_path)?; + clean_stray_files(&git, local_path)?; Ok(meta) } - /// Stage the given paths and create a commit. - fn stage_and_commit(&self, paths: &[&Path], message: &str) -> Result<()> { - self.git - .stage_and_commit_files(&self.local_path, paths, message) - } - /// Read the meta file from disk and update self.meta. fn read_meta(&mut self) -> Result<()> { self.meta = load_meta(&self.local_path.join("meta"))?; @@ -365,7 +342,7 @@ impl GitSyncServer { return Ok(()); } // Warn if there are uncommitted changes that the hard reset will discard. This should - // only happen if a previous write was interrupted; clean_stray_files handles the fallout. + // only happen if a previous write was interrupted. clean_stray_files handles the fallout. if !self .git .cmd_ok(&self.local_path, &["diff", "--quiet", "HEAD"])? @@ -377,7 +354,7 @@ impl GitSyncServer { self.git .cmd(&self.local_path, &["reset", "--hard", "FETCH_HEAD"])?; // Remove any untracked files left behind by interrupted writes. - self.git.clean_stray_files(&self.local_path)?; + clean_stray_files(&self.git, &self.local_path)?; Ok(()) } @@ -464,14 +441,11 @@ impl GitSyncServer { /// Return the [`SnapshotUrgency`] based on the number of version files present. /// - /// Since cleanup runs after every successful add_snapshot and removes - /// all version files covered by the snapshot, the count here reflects only post-snapshot - /// versions. + /// Since cleanup runs after every successful add_snapshot, the count here reflects + /// post-snapshot versions plus any recent files kept by the retention policy. fn snapshot_urgency(&self) -> SnapshotUrgency { let pattern = format!("{}/v-*", self.local_path.display()); let count = glob(&pattern).map(|g| g.count()).unwrap_or(0); - // TODO: Performance test get_version_by_parent_version_id to help determine - // what reasonable thresholds are. match count { 0..=50 => SnapshotUrgency::None, 51..=100 => SnapshotUrgency::Low, @@ -482,9 +456,10 @@ impl GitSyncServer { /// Cleans up the repository by removing version files covered by the current snapshot. /// /// Reads the snapshot to determine which version it covers, then walks backward - /// through the version chain from that version. All version files on that chain - /// are deleted, committed, and pushed. If the push is rejected, we reset to the remote state - /// and will retry on the next snapshot. + /// through the version chain from that version. Version files on that chain older than + /// `version_retention` are deleted, committed, and pushed. Recently-written files are kept + /// to allow recovery if the snapshot is corrupt. If the push is rejected, we reset to the + /// remote state and will retry on the next snapshot. /// /// If called with no snapshot present it is a no-op. fn cleanup(&self) -> Result<()> { @@ -553,9 +528,9 @@ impl GitSyncServer { let name = path.file_name().and_then(|n| n.to_str()).ok_or_else(|| { Error::Server("version file path is not valid UTF-8".into()) })?; - match self.git.version_file_age(&self.local_path, name)? { + match version_file_age(&self.git, &self.local_path, name)? { Some(age) if age >= self.version_retention => {} - _ => continue, // Too recent or unknown age — keep it. + _ => continue, // Too recent or unknown age, keep it. } self.git.cmd(&self.local_path, &["rm", name])?; any_removed = true; @@ -600,7 +575,7 @@ impl Server for GitSyncServer { history_segment: HistorySegment, ) -> Result<(AddVersionResult, SnapshotUrgency)> { // Accept any parent when the repo is empty (latest == NIL). - // Otherwise check if parent is in latest. If it isn't, pull recheck. + // Otherwise check if parent matches latest. If it doesn't, reset_to_remote and recheck. if self.meta.latest_version != Uuid::nil() && parent_version_id != self.meta.latest_version { self.reset_to_remote()?; @@ -625,11 +600,12 @@ impl Server for GitSyncServer { let meta_path = self.write_meta()?; // Commit and push, reverting if push fails. - self.stage_and_commit(&[&version_path, &meta_path], "add version")?; + self.git + .stage_and_commit(&self.local_path, &[&version_path, &meta_path], "add version")?; if !self.push()? { - // Push was rejected. Undo the commit; pull will reset --hard and git clean - // away the stray version file. + // Push was rejected. Undo the commit. reset_to_remote will fetch, reset --hard, + // and clean away the stray version file. self.git .cmd(&self.local_path, &["reset", "HEAD~1", "--soft"])?; self.reset_to_remote()?; @@ -668,10 +644,9 @@ impl Server for GitSyncServer { async fn add_snapshot(&mut self, version_id: VersionId, snapshot: Snapshot) -> Result<()> { self.reset_to_remote()?; // Write the snapshot to a file. - // Note: If another replica has pushed a snapshot for a - // later version in the chain between our pull and our push, we will overwrite it. - // This should be harmless: a replica with newer state will overwrite again, - // and push rejection handles concurrent writes. + // If another replica has pushed a snapshot for a later version in the chain between + // our reset_to_remote and our push, we will overwrite it. This is harmless. A replica + // with newer state will overwrite again, and push rejection handles concurrent writes. let unsealed = Unsealed { version_id, payload: snapshot, @@ -686,10 +661,11 @@ impl Server for GitSyncServer { serde_json::to_writer(f, &snapshot_file)?; // Commit and push, reverting if push fails. - self.stage_and_commit(&[&snapshot_path], "add snapshot")?; + self.git + .stage_and_commit(&self.local_path, &[&snapshot_path], "add snapshot")?; if !self.push()? { - // Push was rejected. Undo the commit, pull and clean to restore state. + // Push was rejected. Undo the commit and reset_to_remote to restore state. self.git .cmd(&self.local_path, &["reset", "HEAD~1", "--soft"])?; self.reset_to_remote()?; @@ -822,7 +798,9 @@ mod test { // Write a new file in clone1 and push it. let new_file = clone1.join("testfile"); std::fs::write(&new_file, b"hello")?; - server1.stage_and_commit(&[&new_file], "add testfile")?; + server1 + .git + .stage_and_commit(&server1.local_path, &[&new_file], "add testfile")?; assert!(server1.push()?); // Build a server for clone2 and pull @@ -1175,7 +1153,7 @@ mod test { .join(format!("v-{}-{}", v2.simple(), v3.simple())) .exists()); - // Snapshot at v3; cleanup should remove all three version files. + // Snapshot at v3, cleanup should remove all three version files. server.add_snapshot(v3, b"full state".to_vec()).await?; assert!( From 242282726d1af28c326d78363d1727375e7502e0 Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Thu, 23 Apr 2026 22:06:14 -0700 Subject: [PATCH 39/44] fix snaphot urgency to only count post-snapshot versions --- src/server/gitsync/mod.rs | 59 ++++++++++++++++++++++++++++++++++----- 1 file changed, 52 insertions(+), 7 deletions(-) diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index d962f4a3c..35657cf59 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -439,13 +439,55 @@ impl GitSyncServer { Ok(None) } - /// Return the [`SnapshotUrgency`] based on the number of version files present. + /// Return the [`SnapshotUrgency`] based on the number of post-snapshot versions. /// - /// Since cleanup runs after every successful add_snapshot, the count here reflects - /// post-snapshot versions plus any recent files kept by the retention policy. + /// Counts only versions added since the current snapshot. fn snapshot_urgency(&self) -> SnapshotUrgency { - let pattern = format!("{}/v-*", self.local_path.display()); - let count = glob(&pattern).map(|g| g.count()).unwrap_or(0); + let path_str = match self.local_path.to_str() { + Some(s) => s, + None => { + log::warn!("snapshot_urgency: local_path is not valid UTF-8"); + return SnapshotUrgency::None; + } + }; + + // Read the snapshot version to know where post-snapshot history begins. + let snapshot_version: Option = File::open(self.local_path.join("snapshot")) + .ok() + .and_then(|f| serde_json::from_reader::<_, SnapshotFile>(BufReader::new(f)).ok()) + .map(|s| s.version_id); + + // Build a child -> parent map from v-* filenames. + let pattern = format!("{}/v-*", path_str); + let mut child_to_parent: HashMap = HashMap::new(); + if let Ok(entries) = glob(&pattern) { + for entry in entries.flatten() { + if let Some(name) = entry.file_name().and_then(|n| n.to_str()) { + if let Some((parent_id, child_id)) = parse_version_filename(name) { + child_to_parent.insert(child_id, parent_id); + } + } + } + } + + // Walk backward from latest_version, stopping at the snapshot version or NIL. + // Only post-snapshot steps are counted. + let stop_at = snapshot_version.unwrap_or(Uuid::nil()); + let mut count = 0usize; + let mut current = self.meta.latest_version; + for _ in 0..=child_to_parent.len() { + if current == stop_at || current == Uuid::nil() { + break; + } + match child_to_parent.get(¤t) { + Some(&parent) => { + count += 1; + current = parent; + } + None => break, + } + } + match count { 0..=50 => SnapshotUrgency::None, 51..=100 => SnapshotUrgency::Low, @@ -600,8 +642,11 @@ impl Server for GitSyncServer { let meta_path = self.write_meta()?; // Commit and push, reverting if push fails. - self.git - .stage_and_commit(&self.local_path, &[&version_path, &meta_path], "add version")?; + self.git.stage_and_commit( + &self.local_path, + &[&version_path, &meta_path], + "add version", + )?; if !self.push()? { // Push was rejected. Undo the commit. reset_to_remote will fetch, reset --hard, From 38d7b494acb9c46e9ff4dc8d85bb8e26a4ea4d9b Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Fri, 24 Apr 2026 16:58:07 -0700 Subject: [PATCH 40/44] cleanup cleanup and add cleanup tests --- src/server/gitsync/mod.rs | 135 ++++++++++++++++++++++++++++++-------- 1 file changed, 109 insertions(+), 26 deletions(-) diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index 35657cf59..d22f43475 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -495,6 +495,33 @@ impl GitSyncServer { } } + /// Remove covered version files that are old enough to delete. + /// + /// Stages `git rm` for each entry in `versions` whose child ID is in `covered` and + /// whose commit age exceeds `self.version_retention`. Returns `true` if anything was + /// staged. On error, the caller is responsible for running `git reset HEAD` to unstage. + fn remove_covered_versions( + &self, + versions: &HashMap, + covered: &HashSet, + ) -> Result { + let mut any_removed = false; + for (child_id, (_, path)) in versions { + if covered.contains(child_id) { + let name = path.file_name().and_then(|n| n.to_str()).ok_or_else(|| { + Error::Server("version file path is not valid UTF-8".into()) + })?; + match version_file_age(&self.git, &self.local_path, name)? { + Some(age) if age >= self.version_retention => {} + _ => continue, // Too recent or unknown age, keep it. + } + self.git.cmd(&self.local_path, &["rm", name])?; + any_removed = true; + } + } + Ok(any_removed) + } + /// Cleans up the repository by removing version files covered by the current snapshot. /// /// Reads the snapshot to determine which version it covers, then walks backward @@ -558,32 +585,14 @@ impl GitSyncServer { } } - // Remove each covered version file from the git index and working tree, but only if it - // is older than VERSION_RETENTION. Recent files are kept so that a corrupt snapshot can - // still be recovered by replaying recent history. - // If any `git rm` fails partway through we `git reset HEAD` to unstage, leaving - // the working tree dirty but the index clean for subsequent operations. - let mut any_removed = false; - let rm_result: Result<()> = (|| { - for (child_id, (_, path)) in &versions { - if covered.contains(child_id) { - let name = path.file_name().and_then(|n| n.to_str()).ok_or_else(|| { - Error::Server("version file path is not valid UTF-8".into()) - })?; - match version_file_age(&self.git, &self.local_path, name)? { - Some(age) if age >= self.version_retention => {} - _ => continue, // Too recent or unknown age, keep it. - } - self.git.cmd(&self.local_path, &["rm", name])?; - any_removed = true; - } + // Remove covered files that are old enough. On error, unstage and propagate. + let any_removed = match self.remove_covered_versions(&versions, &covered) { + Ok(removed) => removed, + Err(e) => { + let _ = self.git.cmd(&self.local_path, &["reset", "HEAD"]); + return Err(e); } - Ok(()) - })(); - if let Err(e) = rm_result { - let _ = self.git.cmd(&self.local_path, &["reset", "HEAD"]); - return Err(e); - } + }; if !any_removed { return Ok(()); @@ -676,6 +685,7 @@ impl Server for GitSyncServer { }); } self.reset_to_remote()?; + self.read_meta()?; match self.get_version_by_parent_version_id(&parent_version_id)? { Some(v) => Ok(GetVersionResult::Version { version_id: v.version_id, @@ -714,7 +724,6 @@ impl Server for GitSyncServer { self.git .cmd(&self.local_path, &["reset", "HEAD~1", "--soft"])?; self.reset_to_remote()?; - self.read_meta()?; return Err(Error::Server("Couldn't push to remote.".into())); } @@ -1254,4 +1263,78 @@ mod test { Ok(()) } + + /// Version files younger than `version_retention` must survive cleanup even when + /// they are covered by a snapshot. + #[tokio::test] + async fn test_cleanup_retains_recent_version_files() -> Result<()> { + let tmp = TempDir::new()?; + // Use default version_retention. Freshly committed files are seconds old, + // far below the threshold, so cleanup should leave them in place. + let mut server = GitSyncServer::new( + tmp.path().to_path_buf(), + "main".into(), + None, + true, + b"test-secret".to_vec(), + None, + )?; + + let (r1, _) = server.add_version(NIL_VERSION_ID, b"v1".to_vec()).await?; + let AddVersionResult::Ok(v1) = r1 else { + panic!("add_version 1 failed"); + }; + let (r2, _) = server.add_version(v1, b"v2".to_vec()).await?; + let AddVersionResult::Ok(v2) = r2 else { + panic!("add_version 2 failed"); + }; + + // Snapshot at v2; files are too young to remove. + server.add_snapshot(v2, b"full state".to_vec()).await?; + + assert!( + tmp.path() + .join(format!("v-{}-{}", NIL_VERSION_ID.simple(), v1.simple())) + .exists(), + "v1 file should be retained (too young to remove)" + ); + assert!( + tmp.path() + .join(format!("v-{}-{}", v1.simple(), v2.simple())) + .exists(), + "v2 file should be retained (too young to remove)" + ); + assert!(tmp.path().join("snapshot").exists()); + Ok(()) + } + + /// When `local_only` is `true`, a configured remote is never contacted: no clone on + /// init, no push after writes, no pull on a cache miss. + #[tokio::test] + async fn test_local_only_with_remote_ignores_remote() -> Result<()> { + let tmp = TempDir::new()?; + // Point at a non-existent remote. Any clone/push/pull attempt would fail. + let mut server = GitSyncServer::new( + tmp.path().to_path_buf(), + "main".into(), + Some("https://nonexistent.invalid/repo.git".to_string()), + true, + b"test-secret".to_vec(), + None, + )?; + server.version_retention = Duration::ZERO; + + let (r1, _) = server.add_version(NIL_VERSION_ID, b"v1".to_vec()).await?; + let AddVersionResult::Ok(v1) = r1 else { + panic!("add_version failed"); + }; + let result = server.get_child_version(NIL_VERSION_ID).await?; + assert!( + matches!(result, GetVersionResult::Version { .. }), + "expected Version result, got: {:?}", + result + ); + server.add_snapshot(v1, b"full state".to_vec()).await?; + Ok(()) + } } From 5232d91719f3c0dde59870f32aa743200bf76e77 Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Fri, 24 Apr 2026 17:07:36 -0700 Subject: [PATCH 41/44] lint --- src/server/gitsync/mod.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index d22f43475..26a3dc6dc 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -508,9 +508,10 @@ impl GitSyncServer { let mut any_removed = false; for (child_id, (_, path)) in versions { if covered.contains(child_id) { - let name = path.file_name().and_then(|n| n.to_str()).ok_or_else(|| { - Error::Server("version file path is not valid UTF-8".into()) - })?; + let name = path + .file_name() + .and_then(|n| n.to_str()) + .ok_or_else(|| Error::Server("version file path is not valid UTF-8".into()))?; match version_file_age(&self.git, &self.local_path, name)? { Some(age) if age >= self.version_retention => {} _ => continue, // Too recent or unknown age, keep it. From 5c83d81d6d34a697dd40a7096b3b582ee2b2f2a1 Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Fri, 24 Apr 2026 17:10:20 -0700 Subject: [PATCH 42/44] clippy --- src/server/gitsync/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index 26a3dc6dc..672465327 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -304,7 +304,7 @@ impl GitSyncServer { }; // Remove any untracked files left behind by interrupted writes. - clean_stray_files(&git, local_path)?; + clean_stray_files(git, local_path)?; Ok(meta) } From 5e857d38f1e8602fb00056ba2c7528fea5daf64d Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Sat, 25 Apr 2026 10:45:19 -0700 Subject: [PATCH 43/44] move free git functions into the git struct --- src/server/gitsync/mod.rs | 43 +++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/src/server/gitsync/mod.rs b/src/server/gitsync/mod.rs index 672465327..9e66fb2f1 100644 --- a/src/server/gitsync/mod.rs +++ b/src/server/gitsync/mod.rs @@ -176,27 +176,26 @@ impl Git { } self.cmd(dir, &["commit", "-m", message]) } -} - -/// Remove untracked TaskChampion files left behind by interrupted writes. -fn clean_stray_files(git: &Git, dir: &Path) -> Result<()> { - git.cmd(dir, &["clean", "-f", "--", "v-*", "snapshot", "meta"]) -} + /// Remove untracked TaskChampion files left behind by interrupted writes. + fn clean_stray_files(&self, dir: &Path) -> Result<()> { + self.cmd(dir, &["clean", "-f", "--", "v-*", "snapshot", "meta"]) + } -/// Return how long ago `filename` was last committed in `dir`, or `None` if git has no -/// record of it. A missing record is treated as "keep". -fn version_file_age(git: &Git, dir: &Path, filename: &str) -> Result> { - let out = git.output(dir, &["log", "-1", "--format=%ct", "--", filename])?; - if out.is_empty() { - return Ok(None); + /// Return how long ago `filename` was last committed in `dir`, or `None` if git has no + /// record of it. A missing record is treated as "keep". + fn version_file_age(&self, dir: &Path, filename: &str) -> Result> { + let out = self.output(dir, &["log", "-1", "--format=%ct", "--", filename])?; + if out.is_empty() { + return Ok(None); + } + let commit_ts: u64 = out + .parse() + .map_err(|_| Error::Server(format!("unexpected git log output: {out:?}")))?; + let now = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_err(|e| Error::Server(format!("system clock error: {e}")))?; + Ok(Some(now.saturating_sub(Duration::from_secs(commit_ts)))) } - let commit_ts: u64 = out - .parse() - .map_err(|_| Error::Server(format!("unexpected git log output: {out:?}")))?; - let now = SystemTime::now() - .duration_since(UNIX_EPOCH) - .map_err(|e| Error::Server(format!("system clock error: {e}")))?; - Ok(Some(now.saturating_sub(Duration::from_secs(commit_ts)))) } impl GitSyncServer { @@ -304,7 +303,7 @@ impl GitSyncServer { }; // Remove any untracked files left behind by interrupted writes. - clean_stray_files(git, local_path)?; + git.clean_stray_files(local_path)?; Ok(meta) } @@ -354,7 +353,7 @@ impl GitSyncServer { self.git .cmd(&self.local_path, &["reset", "--hard", "FETCH_HEAD"])?; // Remove any untracked files left behind by interrupted writes. - clean_stray_files(&self.git, &self.local_path)?; + self.git.clean_stray_files(&self.local_path)?; Ok(()) } @@ -512,7 +511,7 @@ impl GitSyncServer { .file_name() .and_then(|n| n.to_str()) .ok_or_else(|| Error::Server("version file path is not valid UTF-8".into()))?; - match version_file_age(&self.git, &self.local_path, name)? { + match self.git.version_file_age(&self.local_path, name)? { Some(age) if age >= self.version_retention => {} _ => continue, // Too recent or unknown age, keep it. } From ece145b526116132597b36e6ba5240257681eca0 Mon Sep 17 00:00:00 2001 From: Adam Milner Date: Sat, 25 Apr 2026 10:46:09 -0700 Subject: [PATCH 44/44] clippy --- src/task/task.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/task/task.rs b/src/task/task.rs index 599b90240..57be16486 100644 --- a/src/task/task.rs +++ b/src/task/task.rs @@ -305,18 +305,16 @@ impl Task { /// This also updates the task's "end" property appropriately. pub fn set_status(&mut self, status: Status, ops: &mut Operations) -> Result<()> { match status { - Status::Pending | Status::Recurring => { + Status::Pending | Status::Recurring // clear "end" when a task becomes "pending" or "recurring" - if self.data.has(Prop::End.as_ref()) { + if self.data.has(Prop::End.as_ref()) => { self.set_timestamp(Prop::End.as_ref(), None, ops)?; } - } - Status::Completed | Status::Deleted => { + Status::Completed | Status::Deleted // set "end" when a task is deleted or completed - if !self.data.has(Prop::End.as_ref()) { + if !self.data.has(Prop::End.as_ref()) => { self.set_timestamp(Prop::End.as_ref(), Some(Utc::now()), ops)?; } - } _ => {} } self.set_value(