From b16ee5a5ef2262a6d1defe81c19b1da530bfa4e8 Mon Sep 17 00:00:00 2001 From: perfectra1n Date: Mon, 13 Jul 2026 12:58:23 -0700 Subject: [PATCH 1/7] refactor: compile the module tree once instead of twice main.rs re-declared all 15 modules as private `mod`s while also importing `claude_code_sync::VerbosityLevel` from its own library. The tree therefore compiled twice -- once into the lib, once into the bin -- producing two incompatible type universes that happened to share names. Make the binary a consumer of the library: add `pub mod handlers` to lib.rs and let main.rs import from `claude_code_sync::*`. Imports are explicit rather than glob: the crate root and `handlers` both export `config`, so two globs would be an ambiguity error at every `config::` call site. This lets both `#[allow(unused_imports)]` in undo/mod.rs go. They were not cargo-culted -- in the lib those `pub use`s are real public API, but in the bin `mod undo` was private, so they were unreachable and warned. The attribute silenced the duplicate copy. Removing the duplicate removes the need for it. The `#[allow(dead_code)]` attributes are unrelated (they serve the non-test lib build) and stay. `cargo test` now reports 336 tests where it previously reported 490. No test was lost: 154 of those entries were the bin harness re-running the lib's own tests. The set of unique test paths is unchanged at 336, verified by diffing `cargo test -- --list` before and after. The `src/main.rs` unittest target now correctly reports 0 tests. --- src/lib.rs | 6 ++++++ src/main.rs | 32 +++++++++++--------------------- src/undo/mod.rs | 10 ++-------- 3 files changed, 19 insertions(+), 29 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index e5289474..ebc9ceb9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -133,3 +133,9 @@ pub mod sync; /// Snapshots enable undoing pull operations (by restoring files) and push operations /// (by resetting Git commits). Includes validation and security checks for safe restoration. pub mod undo; + +/// Command handlers backing the `claude-code-sync` CLI subcommands. +/// +/// Each handler owns one subcommand end-to-end: prompting, calling into the +/// modules above, and reporting. The binary is a thin `clap` shell over these. +pub mod handlers; diff --git a/src/main.rs b/src/main.rs index 87c9e050..4d95ecd4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,29 +1,19 @@ -mod artifacts; -mod config; -mod conflict; -mod filter; -mod handlers; -mod history; -mod interactive_conflict; -mod logger; -mod merge; -mod onboarding; -mod parser; -mod report; -mod scm; -mod sync; -mod undo; - use anyhow::Result; use clap::{Parser, Subcommand}; use colored::Colorize; use std::path::PathBuf; -// Import all handler functions -use handlers::*; - -// Import VerbosityLevel from lib -use claude_code_sync::VerbosityLevel; +// The binary is a thin CLI over the `claude_code_sync` library. Import the +// modules explicitly rather than glob-importing both the crate root and +// `handlers` — both export a `config`, and two globs supplying the same name +// is an ambiguity error at every `config::` call site. +use claude_code_sync::handlers::{ + handle_cleanup_snapshots, handle_config_export, handle_config_interactive, + handle_config_wizard, handle_history_clear, handle_history_last, handle_history_list, + handle_history_review, handle_repo_selector, handle_undo_pull, handle_undo_push, + is_initialized, run_init_from_config, run_onboarding_flow, try_init_from_config, +}; +use claude_code_sync::{config, filter, logger, report, scm, sync, VerbosityLevel}; #[derive(Parser)] #[command(name = "claude-code-sync")] diff --git a/src/undo/mod.rs b/src/undo/mod.rs index 8cdf5f94..bfb07b72 100644 --- a/src/undo/mod.rs +++ b/src/undo/mod.rs @@ -11,17 +11,11 @@ mod restore; mod snapshot; // Re-export public types and functions to maintain API compatibility -pub use cleanup::{cleanup_old_snapshots, SnapshotCleanupConfig}; +pub use cleanup::{cleanup_old_snapshots, cleanup_old_snapshots_with_dir, SnapshotCleanupConfig}; pub use operations::{undo_pull, undo_push}; -pub use preview::{preview_undo_pull, preview_undo_push, VerbosityLevel}; +pub use preview::{preview_undo_pull, preview_undo_push, UndoPreview, VerbosityLevel}; pub use snapshot::Snapshot; -// These are part of the public API but currently only used in tests -#[allow(unused_imports)] -pub use cleanup::cleanup_old_snapshots_with_dir; -#[allow(unused_imports)] -pub use preview::UndoPreview; - #[cfg(test)] mod tests { use super::*; From cf5678861007a9588fd1a879aa3147af910b1677 Mon Sep 17 00:00:00 2001 From: perfectra1n Date: Mon, 13 Jul 2026 13:05:25 -0700 Subject: [PATCH 2/7] refactor(undo): move the 32 orphaned tests next to the code they test undo/mod.rs was 1397 lines, of which 1373 were a single `#[cfg(test)] mod tests`. The production code had already been split into cleanup/operations/ preview/restore/snapshot; the tests never followed, so every one of them sat in the parent testing its siblings through the public re-exports. Redistribute all 32 to the module each actually exercises, matching the convention preview.rs already followed. mod.rs is now 21 lines of module declarations and re-exports. Split snapshot.rs (542 lines) into snapshot.rs and differential.rs along a seam that was already there: full snapshots (struct, base64 serde, create, save/load) versus differential chains (create_differential*, find_latest_ snapshot, reconstruct_full_state*). The differential half builds on the full half and never the reverse. This mirrors restore.rs, which already keeps an `impl Snapshot` block in a sibling file. Without it, snapshot.rs plus its tests would have been ~790 lines. Add undo/test_support.rs with the fixtures the tests genuinely share -- most importantly a HistoryBuilder, which collapses the ~30 lines of snapshot + OperationHistory + OperationRecord setup that all ten operations tests previously spelled out longhand. Drop five of the six `#[allow(dead_code)]`. Their comment claimed "the bin compiles this module separately", which stopped being true in the previous commit: `create`, the three `create_differential*`, and `find_latest_snapshot` are all reachable public API in the lib and were only ever dead in the binary's private copy of the tree. The one that survives, `reconstruct_full_state`, is genuinely pub(crate) with no production caller. No test was lost: `cargo test -- --list` diffs to 32 renames, each with a matching leaf name, and the unique test-path count is unchanged at 336. --- src/undo/cleanup.rs | 132 ++++ src/undo/differential.rs | 472 +++++++++++++ src/undo/mod.rs | 1378 +------------------------------------- src/undo/operations.rs | 313 +++++++++ src/undo/restore.rs | 127 +++- src/undo/snapshot.rs | 406 +++++------ src/undo/test_support.rs | 137 ++++ 7 files changed, 1341 insertions(+), 1624 deletions(-) create mode 100644 src/undo/differential.rs create mode 100644 src/undo/test_support.rs diff --git a/src/undo/cleanup.rs b/src/undo/cleanup.rs index ce04c5e8..3bc3cee3 100644 --- a/src/undo/cleanup.rs +++ b/src/undo/cleanup.rs @@ -141,3 +141,135 @@ pub fn cleanup_old_snapshots( ) -> Result { cleanup_old_snapshots_with_dir(config, dry_run, None) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::undo::test_support::metadata_only_snapshot; + use chrono::Duration; + use tempfile::tempdir; + + /// A snapshots dir holding `count` metadata-only snapshots of one type, + /// aged 0, 1, 2, ... days. + fn snapshots_aged_by_day(dir: &Path, count: i64, operation_type: OperationType) { + fs::create_dir_all(dir).unwrap(); + for i in 0..count { + let id = format!("{operation_type:?}_{i}").to_lowercase(); + metadata_only_snapshot(&id, operation_type, Duration::days(i)) + .save_to_disk(Some(dir)) + .unwrap(); + } + } + + #[test] + fn test_cleanup_snapshots_respects_count_limit() { + let temp_dir = tempdir().unwrap(); + let snapshots_dir = temp_dir.path().join("snapshots"); + snapshots_aged_by_day(&snapshots_dir, 10, OperationType::Pull); + + let config = SnapshotCleanupConfig { + max_count_per_type: 5, + max_age_days: 0, // Only count matters, not age + }; + + let deleted = + cleanup_old_snapshots_with_dir(Some(config), false, Some(&snapshots_dir)).unwrap(); + assert_eq!(deleted, 5, "Should delete 5 old snapshots"); + + let remaining = fs::read_dir(&snapshots_dir).unwrap().count(); + assert_eq!(remaining, 5, "Should have 5 snapshots remaining"); + } + + #[test] + fn test_cleanup_snapshots_respects_age_limit() { + let temp_dir = tempdir().unwrap(); + let snapshots_dir = temp_dir.path().join("snapshots"); + fs::create_dir_all(&snapshots_dir).unwrap(); + + // Ages are spread well away from the 50-day threshold so that clock + // drift during the test can't flip a snapshot across the boundary: + // days 5, 15, 25, 35, 45 are kept; 55, 65, 75, 85, 95 are deleted. + for i in 0..10 { + metadata_only_snapshot( + &format!("snapshot_{i}"), + OperationType::Pull, + Duration::days(5 + i * 10), + ) + .save_to_disk(Some(&snapshots_dir)) + .unwrap(); + } + + let config = SnapshotCleanupConfig { + max_count_per_type: 0, // Count doesn't matter, only age + max_age_days: 50, + }; + + let deleted = + cleanup_old_snapshots_with_dir(Some(config), false, Some(&snapshots_dir)).unwrap(); + assert_eq!(deleted, 5, "Should delete 5 old snapshots"); + + let remaining = fs::read_dir(&snapshots_dir).unwrap().count(); + assert_eq!(remaining, 5, "Should have 5 snapshots remaining"); + } + + #[test] + fn test_cleanup_snapshots_separates_operation_types() { + let temp_dir = tempdir().unwrap(); + let snapshots_dir = temp_dir.path().join("snapshots"); + fs::create_dir_all(&snapshots_dir).unwrap(); + + for i in 0..10 { + metadata_only_snapshot(&format!("pull_{i}"), OperationType::Pull, Duration::days(i)) + .save_to_disk(Some(&snapshots_dir)) + .unwrap(); + + let mut push = metadata_only_snapshot( + &format!("push_{i}"), + OperationType::Push, + Duration::days(i), + ); + push.git_commit_hash = Some(format!("hash_{i}")); + push.branch = Some("main".to_string()); + push.save_to_disk(Some(&snapshots_dir)).unwrap(); + } + + let config = SnapshotCleanupConfig { + max_count_per_type: 3, + max_age_days: 0, // Don't keep by age + }; + + let deleted = + cleanup_old_snapshots_with_dir(Some(config), false, Some(&snapshots_dir)).unwrap(); + + // 7 pull + 7 push: the limit applies per type, not across all snapshots. + assert_eq!(deleted, 14, "Should delete 14 old snapshots"); + + let remaining = fs::read_dir(&snapshots_dir).unwrap().count(); + assert_eq!( + remaining, 6, + "Should have 6 snapshots remaining (3 per type)" + ); + } + + #[test] + fn test_cleanup_snapshots_dry_run() { + let temp_dir = tempdir().unwrap(); + let snapshots_dir = temp_dir.path().join("snapshots"); + snapshots_aged_by_day(&snapshots_dir, 10, OperationType::Pull); + + let config = SnapshotCleanupConfig { + max_count_per_type: 3, + max_age_days: 0, // Only count matters for this test + }; + + let deleted = + cleanup_old_snapshots_with_dir(Some(config), true, Some(&snapshots_dir)).unwrap(); + assert_eq!(deleted, 7, "Should report 7 snapshots would be deleted"); + + let remaining = fs::read_dir(&snapshots_dir).unwrap().count(); + assert_eq!( + remaining, 10, + "All snapshots should still exist after dry run" + ); + } +} diff --git a/src/undo/differential.rs b/src/undo/differential.rs new file mode 100644 index 00000000..6c20f696 --- /dev/null +++ b/src/undo/differential.rs @@ -0,0 +1,472 @@ +//! Differential snapshots: storing only what changed since the previous snapshot. +//! +//! A differential snapshot records the delta against a base snapshot rather than +//! every file, which keeps repeated snapshots of large conversation histories +//! from ballooning on disk. `base_snapshot_id` links each snapshot to its parent, +//! forming a chain that `reconstruct_full_state` walks to recover the full state. +//! +//! Building on `snapshot.rs` in a sibling `impl Snapshot` block mirrors how +//! `restore.rs` is organised. + +use anyhow::{anyhow, Context, Result}; +use std::collections::HashMap; +use std::fs; +use std::path::{Path, PathBuf}; +use uuid::Uuid; + +use super::snapshot::Snapshot; +use crate::history::OperationType; + +impl Snapshot { + /// Create a differential snapshot that only stores changes since the last snapshot + /// + /// This significantly reduces disk usage by only storing files that have changed. + /// + /// # Arguments + /// * `operation_type` - Type of operation this snapshot is for + /// * `file_paths` - Iterator of file paths to include in snapshot + /// * `commit_hash` - Optional git commit hash to store in the snapshot + /// * `snapshots_dir` - Optional custom snapshots directory (for testing) + /// + /// # Returns + /// A new differential Snapshot, or a full snapshot if no base exists + pub fn create_differential_with_dir( + operation_type: OperationType, + file_paths: I, + commit_hash: Option<&str>, + snapshots_dir: Option<&Path>, + ) -> Result + where + P: AsRef, + I: IntoIterator, + { + // Try to find the most recent snapshot of the same operation type + let base_snapshot = Self::find_latest_snapshot(operation_type, snapshots_dir)?; + + let snapshot_id = Uuid::new_v4().to_string(); + let timestamp = chrono::Utc::now(); + + // Collect current file paths and their content + let mut current_files: HashMap> = HashMap::new(); + for path in file_paths { + let path = path.as_ref(); + match fs::read(path) { + Ok(content) => { + current_files.insert(path.to_string_lossy().to_string(), content); + } + Err(e) if e.kind() == std::io::ErrorKind::NotFound => { + continue; + } + Err(e) => { + return Err(e).with_context(|| { + format!("Failed to read file for snapshot: {}", path.display()) + }); + } + } + } + + // If no base snapshot exists, create a full snapshot + let (files, base_snapshot_id, deleted_files) = if let Some(base) = base_snapshot { + let mut changed_files = HashMap::new(); + let mut deleted = Vec::new(); + + // Reconstruct the full state from the base snapshot chain + // This is crucial: if the base is differential, we need the complete state, + // not just the changed files in that differential snapshot + let base_full_state = base.reconstruct_full_state_with_dir(snapshots_dir)?; + + // Find files that changed or are new + for (path, content) in ¤t_files { + if let Some(base_content) = base_full_state.get(path) { + // File exists in base - only include if content changed + if base_content != content { + changed_files.insert(path.clone(), content.clone()); + } + } else { + // New file - always include + changed_files.insert(path.clone(), content.clone()); + } + } + + // Find files that were deleted + for path in base_full_state.keys() { + if !current_files.contains_key(path) { + deleted.push(path.clone()); + } + } + + (changed_files, Some(base.snapshot_id), deleted) + } else { + // No base snapshot - include all files (full snapshot) + (current_files, None, Vec::new()) + }; + + Ok(Snapshot { + snapshot_id, + timestamp, + operation_type, + git_commit_hash: commit_hash.map(|s| s.to_string()), + files, + branch: None, + base_snapshot_id, + deleted_files, + }) + } + + /// Create a differential snapshot using the default snapshots directory + /// + /// This is a convenience wrapper around `create_differential_with_dir` that + /// uses the default snapshots directory. + /// + /// # Arguments + /// * `operation_type` - Type of operation this snapshot is for + /// * `file_paths` - Iterator of file paths to include in snapshot + /// * `commit_hash` - Optional git commit hash to store in the snapshot + /// + /// # Returns + /// A new differential Snapshot, or a full snapshot if no base exists + pub fn create_differential( + operation_type: OperationType, + file_paths: I, + commit_hash: Option<&str>, + ) -> Result + where + P: AsRef, + I: IntoIterator, + { + Self::create_differential_with_dir(operation_type, file_paths, commit_hash, None) + } + + /// Create a differential snapshot with a commit hash (convenience alias) + /// + /// This is the same as `create_differential` but with a clearer name + /// when used with push operations that need to store a commit hash. + pub fn create_differential_with_commit( + operation_type: OperationType, + file_paths: I, + commit_hash: Option<&str>, + ) -> Result + where + P: AsRef, + I: IntoIterator, + { + Self::create_differential(operation_type, file_paths, commit_hash) + } + + /// Find the most recent snapshot of a given operation type + /// + /// # Arguments + /// * `operation_type` - Type of operation to find snapshots for + /// * `custom_dir` - Optional custom snapshots directory (for testing) + /// + /// # Returns + /// The most recent snapshot, or None if no snapshots exist + pub(crate) fn find_latest_snapshot( + operation_type: OperationType, + custom_dir: Option<&Path>, + ) -> Result> { + let snapshots_dir = if let Some(dir) = custom_dir { + dir.to_path_buf() + } else { + Self::snapshots_dir()? + }; + + if !snapshots_dir.exists() { + return Ok(None); + } + + let mut snapshots: Vec<(PathBuf, chrono::DateTime)> = Vec::new(); + + // Scan snapshots directory + for entry in fs::read_dir(&snapshots_dir)? { + let entry = entry?; + let path = entry.path(); + + if path.extension().is_none_or(|ext| ext != "json") { + continue; + } + + // Quick parse to get timestamp and operation type without loading full snapshot + if let Ok(content) = fs::read_to_string(&path) { + if let Ok(snapshot) = serde_json::from_str::(&content) { + if snapshot.operation_type == operation_type { + snapshots.push((path, snapshot.timestamp)); + } + } + } + } + + // Sort by timestamp descending and get the most recent + snapshots.sort_by_key(|s| std::cmp::Reverse(s.1)); + + if let Some((path, _)) = snapshots.first() { + let snapshot = Self::load_from_disk(path)?; + Ok(Some(snapshot)) + } else { + Ok(None) + } + } + + /// Reconstruct the full file state by walking the snapshot chain + /// + /// For differential snapshots, this loads all base snapshots recursively + /// and merges them to produce the complete file state. + /// + /// # Arguments + /// * `snapshots_dir` - Optional custom snapshots directory (for testing) + /// + /// # Returns + /// A HashMap containing the full state of all files + pub fn reconstruct_full_state_with_dir( + &self, + snapshots_dir: Option<&Path>, + ) -> Result>> { + let mut state = HashMap::new(); + + // If this is a differential snapshot, load the base chain + if let Some(base_id) = &self.base_snapshot_id { + let snapshots_dir = if let Some(dir) = snapshots_dir { + dir.to_path_buf() + } else { + Self::snapshots_dir()? + }; + let base_path = snapshots_dir.join(format!("{}.json", base_id)); + + if !base_path.exists() { + return Err(anyhow!( + "Base snapshot not found: {}. \ + The snapshot chain is broken. Cannot restore differential snapshot.", + base_id + )); + } + + // Recursively load the base snapshot's state + let base_snapshot = Self::load_from_disk(&base_path)?; + state = base_snapshot.reconstruct_full_state_with_dir(Some(&snapshots_dir))?; + } + + // Apply this snapshot's changes on top of the base state + for (path, content) in &self.files { + state.insert(path.clone(), content.clone()); + } + + // Remove deleted files + for deleted_path in &self.deleted_files { + state.remove(deleted_path); + } + + Ok(state) + } + + /// Reconstruct the full file state using the default snapshots directory + /// + /// This is a convenience wrapper around `reconstruct_full_state_with_dir`. + #[allow(dead_code)] // no production caller yet; exercised by the tests below + pub(crate) fn reconstruct_full_state(&self) -> Result>> { + self.reconstruct_full_state_with_dir(None) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::tempdir; + + /// Snapshot `files` under `snapshots_dir`, saving it, and hand it back. + fn differential(snapshots_dir: &Path, files: &[&PathBuf]) -> Snapshot { + let snapshot = Snapshot::create_differential_with_dir( + OperationType::Pull, + files.iter().copied(), + None, + Some(snapshots_dir), + ) + .unwrap(); + snapshot.save_to_disk(Some(snapshots_dir)).unwrap(); + snapshot + } + + #[test] + fn test_differential_snapshot_first_snapshot_is_full() { + let temp_dir = tempdir().unwrap(); + let snapshots_dir = temp_dir.path().join("snapshots"); + let file1 = temp_dir.path().join("file1.txt"); + let file2 = temp_dir.path().join("file2.txt"); + + fs::write(&file1, b"content1").unwrap(); + fs::write(&file2, b"content2").unwrap(); + + // With nothing to diff against, the first snapshot must be a full one. + let snapshot = differential(&snapshots_dir, &[&file1, &file2]); + + assert!( + snapshot.base_snapshot_id.is_none(), + "First snapshot should not have a base" + ); + assert_eq!( + snapshot.files.len(), + 2, + "First snapshot should contain all files" + ); + assert!( + snapshot.deleted_files.is_empty(), + "First snapshot should have no deleted files" + ); + } + + #[test] + fn test_differential_snapshot_only_stores_changes() { + let temp_dir = tempdir().unwrap(); + let snapshots_dir = temp_dir.path().join("snapshots"); + let file1 = temp_dir.path().join("file1.txt"); + let file2 = temp_dir.path().join("file2.txt"); + let file3 = temp_dir.path().join("file3.txt"); + + fs::write(&file1, b"content1").unwrap(); + fs::write(&file2, b"content2").unwrap(); + let snapshot1 = differential(&snapshots_dir, &[&file1, &file2]); + + // file1 changes, file3 appears, file2 is untouched. + fs::write(&file1, b"modified_content1").unwrap(); + fs::write(&file3, b"content3").unwrap(); + let snapshot2 = differential(&snapshots_dir, &[&file1, &file2, &file3]); + + assert_eq!( + snapshot2.base_snapshot_id.as_ref().unwrap(), + &snapshot1.snapshot_id, + "Base should be the first snapshot" + ); + + assert_eq!( + snapshot2.files.len(), + 2, + "Should only contain changed and new files" + ); + assert!(snapshot2 + .files + .contains_key(&file1.to_string_lossy().to_string())); + assert!(snapshot2 + .files + .contains_key(&file3.to_string_lossy().to_string())); + assert!( + !snapshot2 + .files + .contains_key(&file2.to_string_lossy().to_string()), + "unchanged file must not be stored again" + ); + } + + #[test] + fn test_differential_snapshot_tracks_deletions() { + let temp_dir = tempdir().unwrap(); + let snapshots_dir = temp_dir.path().join("snapshots"); + let file1 = temp_dir.path().join("file1.txt"); + let file2 = temp_dir.path().join("file2.txt"); + + fs::write(&file1, b"content1").unwrap(); + fs::write(&file2, b"content2").unwrap(); + differential(&snapshots_dir, &[&file1, &file2]); + + fs::remove_file(&file2).unwrap(); + let snapshot2 = differential(&snapshots_dir, &[&file1]); + + // A file's absence has to be recorded explicitly — "not in `files`" + // means "unchanged", not "deleted". + assert_eq!(snapshot2.deleted_files.len(), 1); + assert!(snapshot2 + .deleted_files + .contains(&file2.to_string_lossy().to_string())); + } + + #[test] + fn test_differential_snapshot_reconstruction() { + let temp_dir = tempdir().unwrap(); + let snapshots_dir = temp_dir.path().join("snapshots"); + let file1 = temp_dir.path().join("file1.txt"); + let file2 = temp_dir.path().join("file2.txt"); + let file3 = temp_dir.path().join("file3.txt"); + + fs::write(&file1, b"v1").unwrap(); + fs::write(&file2, b"v1").unwrap(); + differential(&snapshots_dir, &[&file1, &file2]); + + fs::write(&file1, b"v2").unwrap(); + fs::write(&file3, b"v2").unwrap(); + let snapshot2 = differential(&snapshots_dir, &[&file1, &file2, &file3]); + + let full_state = snapshot2 + .reconstruct_full_state_with_dir(Some(&snapshots_dir)) + .unwrap(); + + assert_eq!(full_state.len(), 3); + let at = |p: &PathBuf| full_state.get(&p.to_string_lossy().to_string()).unwrap(); + assert_eq!(at(&file1), b"v2", "changed file takes the new content"); + assert_eq!(at(&file2), b"v1", "unchanged file comes from the base"); + assert_eq!(at(&file3), b"v2", "new file comes from this snapshot"); + } + + #[test] + fn test_differential_snapshot_broken_chain() { + let temp_dir = tempdir().unwrap(); + let snapshots_dir = temp_dir.path().join("snapshots"); + let file1 = temp_dir.path().join("file1.txt"); + + fs::write(&file1, b"content1").unwrap(); + + let mut snapshot = Snapshot::create_differential_with_dir( + OperationType::Pull, + vec![&file1], + None, + Some(&snapshots_dir), + ) + .unwrap(); + + // Point at a base that was never written: a chain with a hole in it + // must fail loudly rather than silently reconstruct a partial state. + snapshot.base_snapshot_id = Some("non-existent-base-id".to_string()); + snapshot.save_to_disk(Some(&snapshots_dir)).unwrap(); + + let result = snapshot.reconstruct_full_state(); + assert!(result.is_err(), "Should fail when base snapshot is missing"); + assert!(result + .unwrap_err() + .to_string() + .contains("Base snapshot not found")); + } + + #[test] + fn test_differential_snapshot_long_chain() { + let temp_dir = tempdir().unwrap(); + let snapshots_dir = temp_dir.path().join("snapshots"); + let file1 = temp_dir.path().join("file1.txt"); + + let mut snapshots = Vec::new(); + for i in 1..=5 { + fs::write(&file1, format!("version_{i}").as_bytes()).unwrap(); + snapshots.push(differential(&snapshots_dir, &[&file1])); + } + + assert!( + snapshots[0].base_snapshot_id.is_none(), + "First should have no base" + ); + for i in 1..5 { + assert_eq!( + snapshots[i].base_snapshot_id.as_ref().unwrap(), + &snapshots[i - 1].snapshot_id, + "Snapshot {i} should reference snapshot {}", + i - 1 + ); + } + + // Walking five links back has to land on the newest content. + let full_state = snapshots[4] + .reconstruct_full_state_with_dir(Some(&snapshots_dir)) + .unwrap(); + assert_eq!( + full_state + .get(&file1.to_string_lossy().to_string()) + .unwrap(), + b"version_5" + ); + } +} diff --git a/src/undo/mod.rs b/src/undo/mod.rs index bfb07b72..e4205577 100644 --- a/src/undo/mod.rs +++ b/src/undo/mod.rs @@ -5,1387 +5,17 @@ //! (by resetting Git commits). Includes validation and security checks for safe restoration. mod cleanup; +mod differential; mod operations; mod preview; mod restore; mod snapshot; +#[cfg(test)] +mod test_support; + // Re-export public types and functions to maintain API compatibility pub use cleanup::{cleanup_old_snapshots, cleanup_old_snapshots_with_dir, SnapshotCleanupConfig}; pub use operations::{undo_pull, undo_push}; pub use preview::{preview_undo_pull, preview_undo_push, UndoPreview, VerbosityLevel}; pub use snapshot::Snapshot; - -#[cfg(test)] -mod tests { - use super::*; - use crate::history::{ - ConversationSummary, OperationHistory, OperationRecord, OperationType, SyncOperation, - }; - use crate::scm::{self, Scm}; - use std::collections::HashMap; - use std::fs; - use std::path::{Path, PathBuf}; - use tempfile::{tempdir, TempDir}; - use uuid::Uuid; - - /// Helper to create a test file with content - fn create_test_file(dir: &Path, name: &str, content: &str) -> PathBuf { - let path = dir.join(name); - fs::write(&path, content).unwrap(); - path - } - - /// Helper to setup test SCM repository - fn setup_test_repo() -> (TempDir, Box) { - let temp_dir = tempdir().unwrap(); - let repo = scm::init(temp_dir.path()).unwrap(); - - // Create and commit a test file - let test_file = temp_dir.path().join("test.txt"); - fs::write(&test_file, "initial content").unwrap(); - repo.stage_all().unwrap(); - repo.commit("Initial commit").unwrap(); - - (temp_dir, repo) - } - - #[test] - fn test_snapshot_create_and_save() { - let temp_dir = tempdir().unwrap(); - let file1 = create_test_file(temp_dir.path(), "file1.txt", "content 1"); - let file2 = create_test_file(temp_dir.path(), "file2.txt", "content 2"); - - let snapshot = Snapshot::create(OperationType::Pull, vec![&file1, &file2], None).unwrap(); - - assert_eq!(snapshot.operation_type, OperationType::Pull); - assert_eq!(snapshot.files.len(), 2); - assert!(snapshot.git_commit_hash.is_none()); - - // Test save - let snapshots_dir = temp_dir.path().join("snapshots"); - let snapshot_path = snapshot.save_to_disk(Some(&snapshots_dir)).unwrap(); - assert!(snapshot_path.exists()); - } - - #[test] - fn test_snapshot_with_commit_hash() { - let (temp_dir, repo) = setup_test_repo(); - let file1 = temp_dir.path().join("test.txt"); - let commit_hash = repo.current_commit_hash().unwrap(); - - let snapshot = - Snapshot::create(OperationType::Push, vec![&file1], Some(&commit_hash)).unwrap(); - - assert_eq!(snapshot.operation_type, OperationType::Push); - assert!(snapshot.git_commit_hash.is_some()); - - let stored_hash = snapshot.git_commit_hash.unwrap(); - assert_eq!(stored_hash.len(), 40); // Git SHA-1 hash length - assert_eq!(stored_hash, commit_hash); - } - - #[test] - fn test_snapshot_restore() { - let temp_dir = tempdir().unwrap(); - let file1 = create_test_file(temp_dir.path(), "file1.txt", "original content"); - - // Create snapshot - let snapshot = Snapshot::create(OperationType::Pull, vec![&file1], None).unwrap(); - - // Modify the file - fs::write(&file1, "modified content").unwrap(); - assert_eq!(fs::read_to_string(&file1).unwrap(), "modified content"); - - // Restore snapshot with temp dir as allowed base - snapshot.restore_with_base(Some(temp_dir.path())).unwrap(); - - // Verify original content is restored - assert_eq!(fs::read_to_string(&file1).unwrap(), "original content"); - } - - #[test] - fn test_snapshot_save_and_load() { - let temp_dir = tempdir().unwrap(); - let file1 = create_test_file(temp_dir.path(), "file1.txt", "test content"); - - let original_snapshot = Snapshot::create(OperationType::Pull, vec![&file1], None).unwrap(); - - let snapshots_dir = temp_dir.path().join("snapshots"); - let snapshot_path = original_snapshot - .save_to_disk(Some(&snapshots_dir)) - .unwrap(); - - // Load the snapshot - let loaded_snapshot = Snapshot::load_from_disk(&snapshot_path).unwrap(); - - assert_eq!(loaded_snapshot.snapshot_id, original_snapshot.snapshot_id); - assert_eq!( - loaded_snapshot.operation_type, - original_snapshot.operation_type - ); - assert_eq!(loaded_snapshot.files.len(), original_snapshot.files.len()); - } - - #[test] - fn test_snapshot_handles_binary_files() { - let temp_dir = tempdir().unwrap(); - let binary_file = temp_dir.path().join("binary.dat"); - - // Create a binary file with non-UTF8 bytes - let binary_content: Vec = vec![0xFF, 0xFE, 0x00, 0x01, 0x02, 0x03]; - fs::write(&binary_file, &binary_content).unwrap(); - - let snapshot = Snapshot::create(OperationType::Pull, vec![&binary_file], None).unwrap(); - - // Verify binary content is preserved - let stored_content = snapshot - .files - .get(&binary_file.to_string_lossy().to_string()) - .unwrap(); - assert_eq!(stored_content, &binary_content); - - // Test save/load preserves binary data - let snapshots_dir = temp_dir.path().join("snapshots"); - let snapshot_path = snapshot.save_to_disk(Some(&snapshots_dir)).unwrap(); - - let loaded_snapshot = Snapshot::load_from_disk(&snapshot_path).unwrap(); - let loaded_content = loaded_snapshot - .files - .get(&binary_file.to_string_lossy().to_string()) - .unwrap(); - assert_eq!(loaded_content, &binary_content); - } - - #[test] - fn test_undo_pull_no_history() { - let temp_dir = tempdir().unwrap(); - let history_path = temp_dir.path().join("history.json"); - - let result = undo_pull(Some(history_path), Some(temp_dir.path())); - assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("No pull operation found")); - } - - #[test] - fn test_undo_pull_success() { - let temp_dir = tempdir().unwrap(); - let history_path = temp_dir.path().join("history.json"); - let snapshots_dir = temp_dir.path().join("snapshots"); - - // Create a test file - let file1 = create_test_file(temp_dir.path(), "conversation.jsonl", "original"); - - // Create a snapshot - let snapshot = Snapshot::create(OperationType::Pull, vec![&file1], None).unwrap(); - let snapshot_path = snapshot.save_to_disk(Some(&snapshots_dir)).unwrap(); - - // Create operation history with a pull operation - let mut history = OperationHistory::from_path(Some(history_path.clone())).unwrap(); - - let conv_summary = ConversationSummary::new( - "test-session".to_string(), - "test/path".to_string(), - None, - 5, - SyncOperation::Modified, - ) - .unwrap(); - - let mut record = OperationRecord::new( - OperationType::Pull, - Some("main".to_string()), - vec![conv_summary], - ); - record.snapshot_path = Some(snapshot_path.clone()); - - history.add_operation(record).unwrap(); - history.save_to(Some(history_path.clone())).unwrap(); - - // Modify the file (simulating changes from pull) - fs::write(&file1, "modified by pull").unwrap(); - - // Undo the pull - let result = undo_pull(Some(history_path.clone()), Some(temp_dir.path())).unwrap(); - assert!(result.contains("Successfully undone")); - - // Verify file is restored - assert_eq!(fs::read_to_string(&file1).unwrap(), "original"); - - // Verify snapshot is cleaned up - assert!(!snapshot_path.exists()); - } - - #[test] - fn test_undo_pull_missing_snapshot() { - let temp_dir = tempdir().unwrap(); - let history_path = temp_dir.path().join("history.json"); - - // Create operation history with a pull but no snapshot file - let mut history = OperationHistory::from_path(Some(history_path.clone())).unwrap(); - - let conv_summary = ConversationSummary::new( - "test-session".to_string(), - "test/path".to_string(), - None, - 5, - SyncOperation::Modified, - ) - .unwrap(); - - let mut record = OperationRecord::new( - OperationType::Pull, - Some("main".to_string()), - vec![conv_summary], - ); - - // Set a snapshot path that doesn't exist - record.snapshot_path = Some(PathBuf::from("/nonexistent/snapshot.json")); - - history.add_operation(record).unwrap(); - history.save_to(Some(history_path.clone())).unwrap(); - - // Try to undo - let result = undo_pull(Some(history_path), Some(temp_dir.path())); - assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("Snapshot file not found")); - } - - #[test] - fn test_undo_push_success() { - let (temp_dir, repo) = setup_test_repo(); - let history_path = temp_dir.path().join("history.json"); - let snapshots_dir = temp_dir.path().join("snapshots"); - - // Get the initial commit hash - let initial_hash = repo.current_commit_hash().unwrap(); - - // Create and commit a new file (simulating a push) - let new_file = temp_dir.path().join("new.txt"); - fs::write(&new_file, "new content").unwrap(); - repo.stage_all().unwrap(); - repo.commit("Second commit").unwrap(); - - // Create a snapshot with the initial commit hash - let mut snapshot = - Snapshot::create(OperationType::Push, vec![&new_file], Some(&initial_hash)).unwrap(); - - // Set the commit hash to the initial commit (for undo) - snapshot.git_commit_hash = Some(initial_hash.clone()); - - let snapshot_path = snapshot.save_to_disk(Some(&snapshots_dir)).unwrap(); - - // Create operation history with a push operation - let mut history = OperationHistory::from_path(Some(history_path.clone())).unwrap(); - - let conv_summary = ConversationSummary::new( - "test-session".to_string(), - "test/path".to_string(), - None, - 5, - SyncOperation::Added, - ) - .unwrap(); - - let mut record = OperationRecord::new( - OperationType::Push, - Some("master".to_string()), - vec![conv_summary], - ); - record.snapshot_path = Some(snapshot_path.clone()); - - history.add_operation(record).unwrap(); - history.save_to(Some(history_path.clone())).unwrap(); - - // Undo the push - let result = undo_push(temp_dir.path(), Some(history_path)).unwrap(); - assert!(result.contains("Successfully undone")); - assert!(result.contains(&initial_hash[..8])); - - // Verify we're back at the initial commit - let repo_check = scm::open(temp_dir.path()).unwrap(); - let current_hash = repo_check.current_commit_hash().unwrap(); - assert_eq!(current_hash, initial_hash); - } - - #[test] - fn test_undo_push_no_history() { - let temp_dir = tempdir().unwrap(); - let history_path = temp_dir.path().join("history.json"); - - let result = undo_push(temp_dir.path(), Some(history_path)); - assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("No push operation found")); - } - - #[test] - fn test_undo_push_missing_commit_hash() { - let temp_dir = tempdir().unwrap(); - let history_path = temp_dir.path().join("history.json"); - let snapshots_dir = temp_dir.path().join("snapshots"); - - // Create a snapshot without a commit hash - let snapshot = Snapshot { - snapshot_id: Uuid::new_v4().to_string(), - timestamp: chrono::Utc::now(), - operation_type: OperationType::Push, - git_commit_hash: None, // Missing commit hash - files: HashMap::new(), - branch: Some("main".to_string()), - base_snapshot_id: None, - deleted_files: Vec::new(), - }; - - let snapshot_path = snapshot.save_to_disk(Some(&snapshots_dir)).unwrap(); - - // Create operation history - let mut history = OperationHistory::from_path(Some(history_path.clone())).unwrap(); - - let conv_summary = ConversationSummary::new( - "test-session".to_string(), - "test/path".to_string(), - None, - 5, - SyncOperation::Added, - ) - .unwrap(); - - let mut record = OperationRecord::new( - OperationType::Push, - Some("main".to_string()), - vec![conv_summary], - ); - record.snapshot_path = Some(snapshot_path); - - history.add_operation(record).unwrap(); - history.save_to(Some(history_path.clone())).unwrap(); - - // Initialize a repo for testing - let repo = scm::init(temp_dir.path()).unwrap(); - let test_file = temp_dir.path().join("test.txt"); - fs::write(&test_file, "test").unwrap(); - repo.stage_all().unwrap(); - repo.commit("Initial commit").unwrap(); - - // Try to undo - let result = undo_push(temp_dir.path(), Some(history_path)); - assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("No commit hash found")); - } - - #[test] - fn test_snapshot_serialization_with_special_characters() { - let temp_dir = tempdir().unwrap(); - let file_with_unicode = temp_dir.path().join("日本語.txt"); - fs::write(&file_with_unicode, "Hello 世界").unwrap(); - - let snapshot = - Snapshot::create(OperationType::Pull, vec![&file_with_unicode], None).unwrap(); - - // Save and reload - let snapshots_dir = temp_dir.path().join("snapshots"); - let snapshot_path = snapshot.save_to_disk(Some(&snapshots_dir)).unwrap(); - - let loaded = Snapshot::load_from_disk(&snapshot_path).unwrap(); - - // Verify content is preserved - let content = loaded.files.values().next().unwrap(); - assert_eq!(String::from_utf8_lossy(content), "Hello 世界"); - } - - #[test] - fn test_base64_encoding_for_binary_data() { - let temp_dir = tempdir().unwrap(); - - // Create a file with various binary values - let binary_file = temp_dir.path().join("binary.dat"); - let binary_data: Vec = (0..=255).collect(); - fs::write(&binary_file, &binary_data).unwrap(); - - let snapshot = Snapshot::create(OperationType::Pull, vec![&binary_file], None).unwrap(); - - // Serialize to JSON - let json = serde_json::to_string(&snapshot).unwrap(); - - // Verify it's valid JSON (shouldn't panic) - let _parsed: serde_json::Value = serde_json::from_str(&json).unwrap(); - - // Deserialize back - let deserialized: Snapshot = serde_json::from_str(&json).unwrap(); - - // Verify binary data is identical - let original_data = snapshot - .files - .get(&binary_file.to_string_lossy().to_string()) - .unwrap(); - let restored_data = deserialized - .files - .get(&binary_file.to_string_lossy().to_string()) - .unwrap(); - - assert_eq!(original_data, restored_data); - assert_eq!(restored_data, &binary_data); - } - - #[test] - fn test_snapshot_restores_file_hierarchy() { - let temp_dir = tempdir().unwrap(); - - // Create nested directory structure - let nested_dir = temp_dir.path().join("dir1").join("dir2"); - fs::create_dir_all(&nested_dir).unwrap(); - let nested_file = nested_dir.join("deep.txt"); - fs::write(&nested_file, "deep content").unwrap(); - - // Create snapshot - let snapshot = Snapshot::create(OperationType::Pull, vec![&nested_file], None).unwrap(); - - // Delete the entire directory tree - fs::remove_dir_all(temp_dir.path().join("dir1")).unwrap(); - assert!(!nested_file.exists()); - - // Restore should recreate the directory structure - snapshot.restore_with_base(Some(temp_dir.path())).unwrap(); - - assert!(nested_file.exists()); - assert_eq!(fs::read_to_string(&nested_file).unwrap(), "deep content"); - } - - #[test] - fn test_empty_snapshot() { - let snapshot = Snapshot::create::(OperationType::Pull, vec![], None).unwrap(); - - assert_eq!(snapshot.files.len(), 0); - assert!(snapshot.git_commit_hash.is_none()); - - // Should be able to save and restore empty snapshot - let temp_dir = tempdir().unwrap(); - let snapshot_path = snapshot.save_to_disk(Some(temp_dir.path())).unwrap(); - - let loaded = Snapshot::load_from_disk(&snapshot_path).unwrap(); - assert_eq!(loaded.files.len(), 0); - - // Restore should not fail - loaded.restore().unwrap(); - } - - #[test] - fn test_snapshot_path_traversal_protection() { - let _temp_dir = tempdir().unwrap(); - - // Create a malicious snapshot that tries to write outside home directory - let mut malicious_snapshot = Snapshot { - snapshot_id: Uuid::new_v4().to_string(), - timestamp: chrono::Utc::now(), - operation_type: OperationType::Pull, - git_commit_hash: None, - files: HashMap::new(), - branch: None, - base_snapshot_id: None, - deleted_files: Vec::new(), - }; - - // Try to add a path that escapes the home directory using .. - // This should be caught by canonicalization - let home = dirs::home_dir().unwrap(); - let evil_path = home.join("..").join("..").join("etc").join("passwd"); - - malicious_snapshot.files.insert( - evil_path.to_string_lossy().to_string(), - b"malicious content".to_vec(), - ); - - // Attempting to restore should fail due to path traversal protection - let result = malicious_snapshot.restore(); - - // The restore should either fail during path validation - // or the path should not be outside home dir after canonicalization - if let Err(err) = result { - let err_msg = err.to_string(); - // Should contain security error message - assert!( - err_msg.contains("Security") || err_msg.contains("outside home"), - "Error message should indicate security issue: {err_msg}" - ); - } else { - // If it didn't error, verify the file wasn't written outside home - assert!( - !PathBuf::from("/etc/passwd").exists() - || !fs::read_to_string("/etc/passwd") - .unwrap_or_default() - .contains("malicious") - ); - } - } - - #[test] - fn test_snapshot_create_handles_missing_files() { - let temp_dir = tempdir().unwrap(); - - // Create one file that exists - let existing_file = create_test_file(temp_dir.path(), "exists.txt", "content"); - - // And one path that doesn't exist - let missing_file = temp_dir.path().join("does_not_exist.txt"); - - // Create snapshot with both paths - let snapshot = Snapshot::create( - OperationType::Pull, - vec![&existing_file, &missing_file], - None, - ) - .unwrap(); - - // Should only contain the existing file - assert_eq!(snapshot.files.len(), 1); - assert!(snapshot - .files - .contains_key(&existing_file.to_string_lossy().to_string())); - assert!(!snapshot - .files - .contains_key(&missing_file.to_string_lossy().to_string())); - } - - #[test] - fn test_undo_pull_preserves_other_operations() { - let temp_dir = tempdir().unwrap(); - let history_path = temp_dir.path().join("history.json"); - let snapshots_dir = temp_dir.path().join("snapshots"); - - // Create a test file - let file1 = create_test_file(temp_dir.path(), "conversation.jsonl", "original"); - - // Create TWO pull snapshots - let snapshot1 = Snapshot::create(OperationType::Pull, vec![&file1], None).unwrap(); - let snapshot_path1 = snapshot1.save_to_disk(Some(&snapshots_dir)).unwrap(); - - let snapshot2 = Snapshot::create(OperationType::Pull, vec![&file1], None).unwrap(); - let snapshot_path2 = snapshot2.save_to_disk(Some(&snapshots_dir)).unwrap(); - - // Create operation history with BOTH pull operations and a push - let mut history = OperationHistory::from_path(Some(history_path.clone())).unwrap(); - - let conv_summary = ConversationSummary::new( - "test-session".to_string(), - "test/path".to_string(), - None, - 5, - SyncOperation::Modified, - ) - .unwrap(); - - // Add first pull - let mut record1 = OperationRecord::new( - OperationType::Pull, - Some("main".to_string()), - vec![conv_summary.clone()], - ); - record1.snapshot_path = Some(snapshot_path1.clone()); - history.add_operation(record1).unwrap(); - - // Add a push operation - let mut push_record = OperationRecord::new( - OperationType::Push, - Some("main".to_string()), - vec![conv_summary.clone()], - ); - push_record.snapshot_path = None; - history.add_operation(push_record).unwrap(); - - // Add second pull (most recent) - let mut record2 = OperationRecord::new( - OperationType::Pull, - Some("main".to_string()), - vec![conv_summary.clone()], - ); - record2.snapshot_path = Some(snapshot_path2.clone()); - history.add_operation(record2).unwrap(); - - history.save_to(Some(history_path.clone())).unwrap(); - - // Verify we have 3 operations - let loaded = OperationHistory::from_path(Some(history_path.clone())).unwrap(); - assert_eq!(loaded.len(), 3); - - // Undo the most recent pull - let result = undo_pull(Some(history_path.clone()), Some(temp_dir.path())).unwrap(); - assert!(result.contains("Successfully undone")); - - // Verify we now have 2 operations (the first pull and the push remain) - let loaded = OperationHistory::from_path(Some(history_path.clone())).unwrap(); - assert_eq!(loaded.len(), 2); - - // Verify the push is still there - let operations = loaded.list_operations(); - assert_eq!(operations[0].operation_type, OperationType::Push); - assert_eq!(operations[1].operation_type, OperationType::Pull); - } - - #[test] - fn test_undo_push_preserves_other_operations() { - let (temp_dir, repo) = setup_test_repo(); - let history_path = temp_dir.path().join("history.json"); - let snapshots_dir = temp_dir.path().join("snapshots"); - - // Get the initial commit hash - let initial_hash = repo.current_commit_hash().unwrap(); - - // Create and commit a new file - let new_file = temp_dir.path().join("new.txt"); - fs::write(&new_file, "new content").unwrap(); - repo.stage_all().unwrap(); - repo.commit("Second commit").unwrap(); - - // Create a snapshot with the initial commit hash - let mut snapshot = - Snapshot::create(OperationType::Push, vec![&new_file], Some(&initial_hash)).unwrap(); - snapshot.git_commit_hash = Some(initial_hash.clone()); - let snapshot_path = snapshot.save_to_disk(Some(&snapshots_dir)).unwrap(); - - // Create operation history with a pull AND a push - let mut history = OperationHistory::from_path(Some(history_path.clone())).unwrap(); - - let conv_summary = ConversationSummary::new( - "test-session".to_string(), - "test/path".to_string(), - None, - 5, - SyncOperation::Added, - ) - .unwrap(); - - // Add a pull operation first - let mut pull_record = OperationRecord::new( - OperationType::Pull, - Some("master".to_string()), - vec![conv_summary.clone()], - ); - pull_record.snapshot_path = None; - history.add_operation(pull_record).unwrap(); - - // Add the push operation - let mut push_record = OperationRecord::new( - OperationType::Push, - Some("master".to_string()), - vec![conv_summary], - ); - push_record.snapshot_path = Some(snapshot_path.clone()); - history.add_operation(push_record).unwrap(); - - history.save_to(Some(history_path.clone())).unwrap(); - - // Verify we have 2 operations - let loaded = OperationHistory::from_path(Some(history_path.clone())).unwrap(); - assert_eq!(loaded.len(), 2); - - // Undo the push - let result = undo_push(temp_dir.path(), Some(history_path.clone())).unwrap(); - assert!(result.contains("Successfully undone")); - - // Verify we now have 1 operation (the pull remains) - let loaded = OperationHistory::from_path(Some(history_path.clone())).unwrap(); - assert_eq!(loaded.len(), 1); - assert_eq!( - loaded.list_operations()[0].operation_type, - OperationType::Pull - ); - } - - #[test] - fn test_undo_pull_transaction_safety() { - // This test verifies that history is updated FIRST, then files are restored. - // If file restoration fails, the history should already be updated. - let temp_dir = tempdir().unwrap(); - let history_path = temp_dir.path().join("history.json"); - let snapshots_dir = temp_dir.path().join("snapshots"); - - // Create a test file - let file1 = create_test_file(temp_dir.path(), "conversation.jsonl", "original"); - - // Create a snapshot - let snapshot = Snapshot::create(OperationType::Pull, vec![&file1], None).unwrap(); - let snapshot_path = snapshot.save_to_disk(Some(&snapshots_dir)).unwrap(); - - // Create operation history with a pull operation - let mut history = OperationHistory::from_path(Some(history_path.clone())).unwrap(); - - let conv_summary = ConversationSummary::new( - "test-session".to_string(), - "test/path".to_string(), - None, - 5, - SyncOperation::Modified, - ) - .unwrap(); - - let mut record = OperationRecord::new( - OperationType::Pull, - Some("main".to_string()), - vec![conv_summary], - ); - record.snapshot_path = Some(snapshot_path.clone()); - - history.add_operation(record).unwrap(); - history.save_to(Some(history_path.clone())).unwrap(); - - // Verify we have 1 operation before undo - let loaded = OperationHistory::from_path(Some(history_path.clone())).unwrap(); - assert_eq!(loaded.len(), 1); - - // Modify the file (simulating changes from pull) - fs::write(&file1, "modified by pull").unwrap(); - - // Make the file read-only to cause restoration to potentially fail - // (though on most systems this won't prevent writing, we can at least test the order) - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - let mut perms = fs::metadata(&file1).unwrap().permissions(); - perms.set_mode(0o444); // read-only - fs::set_permissions(&file1, perms).unwrap(); - } - - // Attempt undo - this might fail on file restoration - let result = undo_pull(Some(history_path.clone()), Some(temp_dir.path())); - - // Whether it succeeds or fails, the history should be updated - // (because we update history FIRST) - let loaded_after = OperationHistory::from_path(Some(history_path.clone())).unwrap(); - - // The key assertion: history should be updated (0 operations) - // This proves we updated history before attempting file restoration - assert_eq!( - loaded_after.len(), - 0, - "History should be updated even if file restoration fails" - ); - - // Verify the snapshot file is removed if successful, or remains if failed - if result.is_ok() { - assert!( - !snapshot_path.exists(), - "Snapshot should be cleaned up on success" - ); - } - - // Clean up permissions for temp dir deletion - #[cfg(unix)] - { - if file1.exists() { - use std::os::unix::fs::PermissionsExt; - let mut perms = fs::metadata(&file1).unwrap().permissions(); - perms.set_mode(0o644); - let _ = fs::set_permissions(&file1, perms); - } - } - } - - #[test] - fn test_undo_push_transaction_safety() { - // This test verifies that history is updated FIRST, then reset is performed. - let (temp_dir, repo) = setup_test_repo(); - let history_path = temp_dir.path().join("history.json"); - let snapshots_dir = temp_dir.path().join("snapshots"); - - // Get the initial commit hash - let initial_hash = repo.current_commit_hash().unwrap(); - - // Create and commit a new file (simulating a push) - let new_file = temp_dir.path().join("new.txt"); - fs::write(&new_file, "new content").unwrap(); - repo.stage_all().unwrap(); - repo.commit("Second commit").unwrap(); - - // Create a snapshot with the initial commit hash - let mut snapshot = - Snapshot::create(OperationType::Push, vec![&new_file], Some(&initial_hash)).unwrap(); - snapshot.git_commit_hash = Some(initial_hash.clone()); - let snapshot_path = snapshot.save_to_disk(Some(&snapshots_dir)).unwrap(); - - // Create operation history with a push operation - let mut history = OperationHistory::from_path(Some(history_path.clone())).unwrap(); - - let conv_summary = ConversationSummary::new( - "test-session".to_string(), - "test/path".to_string(), - None, - 5, - SyncOperation::Added, - ) - .unwrap(); - - let mut record = OperationRecord::new( - OperationType::Push, - Some("master".to_string()), - vec![conv_summary], - ); - record.snapshot_path = Some(snapshot_path.clone()); - - history.add_operation(record).unwrap(); - history.save_to(Some(history_path.clone())).unwrap(); - - // Verify we have 1 operation before undo - let loaded = OperationHistory::from_path(Some(history_path.clone())).unwrap(); - assert_eq!(loaded.len(), 1); - - // Perform undo - let result = undo_push(temp_dir.path(), Some(history_path.clone())); - - // Whether it succeeds or fails, the history should be updated FIRST - let loaded_after = OperationHistory::from_path(Some(history_path.clone())).unwrap(); - - // The key assertion: history should be updated (0 operations) - // This proves we updated history before attempting git reset - assert_eq!( - loaded_after.len(), - 0, - "History should be updated even if git reset fails" - ); - - // If successful, verify we're back at the initial commit - if result.is_ok() { - let repo_check = scm::open(temp_dir.path()).unwrap(); - let current_hash = repo_check.current_commit_hash().unwrap(); - assert_eq!(current_hash, initial_hash); - assert!( - !snapshot_path.exists(), - "Snapshot should be cleaned up on success" - ); - } - } - - // ============================================================================ - // Differential Snapshot Tests - // ============================================================================ - - #[test] - fn test_differential_snapshot_first_snapshot_is_full() { - let temp_dir = tempdir().unwrap(); - let snapshots_dir = temp_dir.path().join("snapshots"); - let file1 = temp_dir.path().join("file1.txt"); - let file2 = temp_dir.path().join("file2.txt"); - - fs::write(&file1, b"content1").unwrap(); - fs::write(&file2, b"content2").unwrap(); - - // First differential snapshot should be a full snapshot (no base) - let snapshot = Snapshot::create_differential_with_dir( - OperationType::Pull, - vec![&file1, &file2], - None, - Some(&snapshots_dir), - ) - .unwrap(); - - snapshot.save_to_disk(Some(&snapshots_dir)).unwrap(); - - // Verify it's a full snapshot - assert!( - snapshot.base_snapshot_id.is_none(), - "First snapshot should not have a base" - ); - assert_eq!( - snapshot.files.len(), - 2, - "First snapshot should contain all files" - ); - assert!( - snapshot.deleted_files.is_empty(), - "First snapshot should have no deleted files" - ); - } - - #[test] - fn test_differential_snapshot_only_stores_changes() { - let temp_dir = tempdir().unwrap(); - let snapshots_dir = temp_dir.path().join("snapshots"); - let file1 = temp_dir.path().join("file1.txt"); - let file2 = temp_dir.path().join("file2.txt"); - let file3 = temp_dir.path().join("file3.txt"); - - // Create initial state - fs::write(&file1, b"content1").unwrap(); - fs::write(&file2, b"content2").unwrap(); - - // First snapshot (full) - let snapshot1 = Snapshot::create_differential_with_dir( - OperationType::Pull, - vec![&file1, &file2], - None, - Some(&snapshots_dir), - ) - .unwrap(); - snapshot1.save_to_disk(Some(&snapshots_dir)).unwrap(); - - // Modify one file and add a new one - fs::write(&file1, b"modified_content1").unwrap(); - fs::write(&file3, b"content3").unwrap(); - - // Second snapshot (differential) - let snapshot2 = Snapshot::create_differential_with_dir( - OperationType::Pull, - vec![&file1, &file2, &file3], - None, - Some(&snapshots_dir), - ) - .unwrap(); - snapshot2.save_to_disk(Some(&snapshots_dir)).unwrap(); - - // Verify it's a differential snapshot - assert!( - snapshot2.base_snapshot_id.is_some(), - "Second snapshot should have a base" - ); - assert_eq!( - snapshot2.base_snapshot_id.as_ref().unwrap(), - &snapshot1.snapshot_id, - "Base should be the first snapshot" - ); - - // Should only contain changed file (file1) and new file (file3), not file2 - assert_eq!( - snapshot2.files.len(), - 2, - "Should only contain changed and new files" - ); - assert!(snapshot2 - .files - .contains_key(&file1.to_string_lossy().to_string())); - assert!(snapshot2 - .files - .contains_key(&file3.to_string_lossy().to_string())); - assert!(!snapshot2 - .files - .contains_key(&file2.to_string_lossy().to_string())); - } - - #[test] - fn test_differential_snapshot_tracks_deletions() { - let temp_dir = tempdir().unwrap(); - let snapshots_dir = temp_dir.path().join("snapshots"); - let file1 = temp_dir.path().join("file1.txt"); - let file2 = temp_dir.path().join("file2.txt"); - - // Create initial state - fs::write(&file1, b"content1").unwrap(); - fs::write(&file2, b"content2").unwrap(); - - // First snapshot - let snapshot1 = Snapshot::create_differential_with_dir( - OperationType::Pull, - vec![&file1, &file2], - None, - Some(&snapshots_dir), - ) - .unwrap(); - snapshot1.save_to_disk(Some(&snapshots_dir)).unwrap(); - - // Delete file2 - fs::remove_file(&file2).unwrap(); - - // Second snapshot - let snapshot2 = Snapshot::create_differential_with_dir( - OperationType::Pull, - vec![&file1], - None, - Some(&snapshots_dir), - ) - .unwrap(); - snapshot2.save_to_disk(Some(&snapshots_dir)).unwrap(); - - // Verify deletion tracking - assert_eq!( - snapshot2.deleted_files.len(), - 1, - "Should track one deleted file" - ); - assert!( - snapshot2 - .deleted_files - .contains(&file2.to_string_lossy().to_string()), - "Should track file2 as deleted" - ); - } - - #[test] - fn test_differential_snapshot_reconstruction() { - let temp_dir = tempdir().unwrap(); - let snapshots_dir = temp_dir.path().join("snapshots"); - let file1 = temp_dir.path().join("file1.txt"); - let file2 = temp_dir.path().join("file2.txt"); - let file3 = temp_dir.path().join("file3.txt"); - - // Create chain of snapshots - fs::write(&file1, b"v1").unwrap(); - fs::write(&file2, b"v1").unwrap(); - - let snapshot1 = Snapshot::create_differential_with_dir( - OperationType::Pull, - vec![&file1, &file2], - None, - Some(&snapshots_dir), - ) - .unwrap(); - snapshot1.save_to_disk(Some(&snapshots_dir)).unwrap(); - - // Modify file1, add file3 - fs::write(&file1, b"v2").unwrap(); - fs::write(&file3, b"v2").unwrap(); - - let snapshot2 = Snapshot::create_differential_with_dir( - OperationType::Pull, - vec![&file1, &file2, &file3], - None, - Some(&snapshots_dir), - ) - .unwrap(); - snapshot2.save_to_disk(Some(&snapshots_dir)).unwrap(); - - // Reconstruct full state from differential snapshot - let full_state = snapshot2 - .reconstruct_full_state_with_dir(Some(&snapshots_dir)) - .unwrap(); - - // Should contain all three files with correct content - assert_eq!( - full_state.len(), - 3, - "Should have 3 files after reconstruction" - ); - assert_eq!( - full_state - .get(&file1.to_string_lossy().to_string()) - .unwrap(), - b"v2" - ); - assert_eq!( - full_state - .get(&file2.to_string_lossy().to_string()) - .unwrap(), - b"v1" - ); // Unchanged from base - assert_eq!( - full_state - .get(&file3.to_string_lossy().to_string()) - .unwrap(), - b"v2" - ); // New file - } - - #[test] - fn test_differential_snapshot_restore_with_deletions() { - let temp_dir = tempdir().unwrap(); - let snapshots_dir = temp_dir.path().join("snapshots"); - let file1 = temp_dir.path().join("file1.txt"); - let file2 = temp_dir.path().join("file2.txt"); - - // Create initial snapshot - fs::write(&file1, b"content1").unwrap(); - fs::write(&file2, b"content2").unwrap(); - - let snapshot1 = Snapshot::create_differential_with_dir( - OperationType::Pull, - vec![&file1, &file2], - None, - Some(&snapshots_dir), - ) - .unwrap(); - snapshot1.save_to_disk(Some(&snapshots_dir)).unwrap(); - - // Delete file2 - fs::remove_file(&file2).unwrap(); - - let snapshot2 = Snapshot::create_differential_with_dir( - OperationType::Pull, - vec![&file1], - None, - Some(&snapshots_dir), - ) - .unwrap(); - snapshot2.save_to_disk(Some(&snapshots_dir)).unwrap(); - - // Restore files (create file2 again to test deletion) - fs::write(&file2, b"should_be_deleted").unwrap(); - - // Restore snapshot2 which should delete file2 - snapshot2 - .restore_with_base_and_snapshots(Some(temp_dir.path()), Some(&snapshots_dir)) - .unwrap(); - - // Verify file1 exists and file2 was deleted - assert!(file1.exists(), "file1 should exist after restore"); - assert!(!file2.exists(), "file2 should be deleted after restore"); - } - - #[test] - fn test_differential_snapshot_broken_chain() { - let temp_dir = tempdir().unwrap(); - let snapshots_dir = temp_dir.path().join("snapshots"); - let file1 = temp_dir.path().join("file1.txt"); - - fs::write(&file1, b"content1").unwrap(); - - // Create a differential snapshot with a fake base ID - let mut snapshot = Snapshot::create_differential_with_dir( - OperationType::Pull, - vec![&file1], - None, - Some(&snapshots_dir), - ) - .unwrap(); - - // Manually set a non-existent base - snapshot.base_snapshot_id = Some("non-existent-base-id".to_string()); - snapshot.save_to_disk(Some(&snapshots_dir)).unwrap(); - - // Trying to reconstruct should fail gracefully - let result = snapshot.reconstruct_full_state(); - assert!(result.is_err(), "Should fail when base snapshot is missing"); - assert!( - result - .unwrap_err() - .to_string() - .contains("Base snapshot not found"), - "Error should mention missing base snapshot" - ); - } - - #[test] - fn test_differential_snapshot_long_chain() { - let temp_dir = tempdir().unwrap(); - let snapshots_dir = temp_dir.path().join("snapshots"); - let file1 = temp_dir.path().join("file1.txt"); - - // Create a chain of 5 snapshots - let mut snapshots = Vec::new(); - - for i in 1..=5 { - fs::write(&file1, format!("version_{}", i).as_bytes()).unwrap(); - - let snapshot = Snapshot::create_differential_with_dir( - OperationType::Pull, - vec![&file1], - None, - Some(&snapshots_dir), - ) - .unwrap(); - snapshot.save_to_disk(Some(&snapshots_dir)).unwrap(); - snapshots.push(snapshot); - } - - // Verify chain structure - assert!( - snapshots[0].base_snapshot_id.is_none(), - "First should have no base" - ); - for i in 1..5 { - assert_eq!( - snapshots[i].base_snapshot_id.as_ref().unwrap(), - &snapshots[i - 1].snapshot_id, - "Snapshot {} should reference snapshot {}", - i, - i - 1 - ); - } - - // Reconstruct from the last snapshot - let full_state = snapshots[4] - .reconstruct_full_state_with_dir(Some(&snapshots_dir)) - .unwrap(); - assert_eq!( - full_state - .get(&file1.to_string_lossy().to_string()) - .unwrap(), - b"version_5", - "Should reconstruct to the latest version" - ); - } - - // ============================================================================ - // Snapshot Cleanup Tests - // ============================================================================ - - #[test] - fn test_cleanup_snapshots_respects_count_limit() { - let temp_dir = tempdir().unwrap(); - let snapshots_dir = temp_dir.path().join("snapshots"); - fs::create_dir_all(&snapshots_dir).unwrap(); - - // Create 10 pull snapshots with different timestamps - for i in 0..10 { - let snapshot = Snapshot { - snapshot_id: format!("snapshot_{}", i), - timestamp: chrono::Utc::now() - chrono::Duration::days(i as i64), - operation_type: OperationType::Pull, - git_commit_hash: None, - files: HashMap::new(), - branch: None, - base_snapshot_id: None, - deleted_files: Vec::new(), - }; - snapshot.save_to_disk(Some(&snapshots_dir)).unwrap(); - } - - // Cleanup keeping last 5 - let config = SnapshotCleanupConfig { - max_count_per_type: 5, - max_age_days: 0, // Only count matters, not age - }; - - let deleted = - cleanup_old_snapshots_with_dir(Some(config), false, Some(&snapshots_dir)).unwrap(); - assert_eq!(deleted, 5, "Should delete 5 old snapshots"); - - // Count remaining snapshots - let remaining = fs::read_dir(&snapshots_dir).unwrap().count(); - assert_eq!(remaining, 5, "Should have 5 snapshots remaining"); - } - - #[test] - fn test_cleanup_snapshots_respects_age_limit() { - let temp_dir = tempdir().unwrap(); - let snapshots_dir = temp_dir.path().join("snapshots"); - fs::create_dir_all(&snapshots_dir).unwrap(); - - // Capture "now" once to ensure consistent timestamp calculations - let now = chrono::Utc::now(); - - // Create snapshots with different ages - // Use larger day offsets well away from the boundary to avoid timing issues - for i in 0..10 { - let snapshot = Snapshot { - snapshot_id: format!("snapshot_{}", i), - timestamp: now - chrono::Duration::days((5 + i * 10) as i64), - operation_type: OperationType::Pull, - git_commit_hash: None, - files: HashMap::new(), - branch: None, - base_snapshot_id: None, - deleted_files: Vec::new(), - }; - snapshot.save_to_disk(Some(&snapshots_dir)).unwrap(); - } - - // Cleanup keeping last 50 days - let config = SnapshotCleanupConfig { - max_count_per_type: 0, // Count doesn't matter, only age - max_age_days: 50, - }; - - let deleted = - cleanup_old_snapshots_with_dir(Some(config), false, Some(&snapshots_dir)).unwrap(); - - // Snapshots are at days: 5, 15, 25, 35, 45, 55, 65, 75, 85, 95 - // Age threshold is now - 50 days - // Keep: days 5, 15, 25, 35, 45 (5 snapshots) - all clearly within 50 days - // Delete: days 55, 65, 75, 85, 95 (5 snapshots) - all clearly older than 50 days - assert_eq!(deleted, 5, "Should delete 5 old snapshots"); - - let remaining = fs::read_dir(&snapshots_dir).unwrap().count(); - assert_eq!(remaining, 5, "Should have 5 snapshots remaining"); - } - - #[test] - fn test_cleanup_snapshots_separates_operation_types() { - let temp_dir = tempdir().unwrap(); - let snapshots_dir = temp_dir.path().join("snapshots"); - fs::create_dir_all(&snapshots_dir).unwrap(); - - // Create 10 pull and 10 push snapshots with different timestamps - for i in 0..10 { - let pull_snapshot = Snapshot { - snapshot_id: format!("pull_{}", i), - timestamp: chrono::Utc::now() - chrono::Duration::days(i as i64), - operation_type: OperationType::Pull, - git_commit_hash: None, - files: HashMap::new(), - branch: None, - base_snapshot_id: None, - deleted_files: Vec::new(), - }; - pull_snapshot.save_to_disk(Some(&snapshots_dir)).unwrap(); - - let push_snapshot = Snapshot { - snapshot_id: format!("push_{}", i), - timestamp: chrono::Utc::now() - chrono::Duration::days(i as i64), - operation_type: OperationType::Push, - git_commit_hash: Some(format!("hash_{}", i)), - files: HashMap::new(), - branch: Some("main".to_string()), - base_snapshot_id: None, - deleted_files: Vec::new(), - }; - push_snapshot.save_to_disk(Some(&snapshots_dir)).unwrap(); - } - - // Cleanup keeping last 3 per type - let config = SnapshotCleanupConfig { - max_count_per_type: 3, - max_age_days: 0, // Don't keep by age - }; - - let deleted = - cleanup_old_snapshots_with_dir(Some(config), false, Some(&snapshots_dir)).unwrap(); - - // Should delete 7 pull + 7 push = 14 total - assert_eq!(deleted, 14, "Should delete 14 old snapshots"); - - // Should keep 3 pull + 3 push = 6 total - let remaining = fs::read_dir(&snapshots_dir).unwrap().count(); - assert_eq!( - remaining, 6, - "Should have 6 snapshots remaining (3 per type)" - ); - } - - #[test] - fn test_cleanup_snapshots_dry_run() { - let temp_dir = tempdir().unwrap(); - let snapshots_dir = temp_dir.path().join("snapshots"); - fs::create_dir_all(&snapshots_dir).unwrap(); - - // Create 10 snapshots with different timestamps - for i in 0..10 { - let snapshot = Snapshot { - snapshot_id: format!("snapshot_{}", i), - timestamp: chrono::Utc::now() - chrono::Duration::days(i as i64), - operation_type: OperationType::Pull, - git_commit_hash: None, - files: HashMap::new(), - branch: None, - base_snapshot_id: None, - deleted_files: Vec::new(), - }; - snapshot.save_to_disk(Some(&snapshots_dir)).unwrap(); - } - - let config = SnapshotCleanupConfig { - max_count_per_type: 3, - max_age_days: 0, // Only count matters for this test - }; - - // Dry run should report but not delete - let deleted = - cleanup_old_snapshots_with_dir(Some(config), true, Some(&snapshots_dir)).unwrap(); - assert_eq!(deleted, 7, "Should report 7 snapshots would be deleted"); - - // All snapshots should still exist - let remaining = fs::read_dir(&snapshots_dir).unwrap().count(); - assert_eq!( - remaining, 10, - "All snapshots should still exist after dry run" - ); - } -} diff --git a/src/undo/operations.rs b/src/undo/operations.rs index d3e75343..43c8df8b 100644 --- a/src/undo/operations.rs +++ b/src/undo/operations.rs @@ -210,3 +210,316 @@ pub fn undo_push(repo_path: &Path, history_path: Option) -> Result Result<()> { self.restore_with_base(None) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::history::OperationType; + use crate::undo::test_support::{create_test_file, metadata_only_snapshot}; + use chrono::Duration; + use tempfile::tempdir; + use uuid::Uuid; + + #[test] + fn test_snapshot_restore() { + let temp_dir = tempdir().unwrap(); + let file1 = create_test_file(temp_dir.path(), "file1.txt", "original content"); + + let snapshot = Snapshot::create(OperationType::Pull, vec![&file1], None).unwrap(); + + fs::write(&file1, "modified content").unwrap(); + assert_eq!(fs::read_to_string(&file1).unwrap(), "modified content"); + + snapshot.restore_with_base(Some(temp_dir.path())).unwrap(); + + assert_eq!(fs::read_to_string(&file1).unwrap(), "original content"); + } + + #[test] + fn test_snapshot_restores_file_hierarchy() { + let temp_dir = tempdir().unwrap(); + + let nested_dir = temp_dir.path().join("dir1").join("dir2"); + fs::create_dir_all(&nested_dir).unwrap(); + let nested_file = nested_dir.join("deep.txt"); + fs::write(&nested_file, "deep content").unwrap(); + + let snapshot = Snapshot::create(OperationType::Pull, vec![&nested_file], None).unwrap(); + + // Blow away the whole tree, not just the file: restore has to recreate + // the intermediate directories, not merely rewrite the leaf. + fs::remove_dir_all(temp_dir.path().join("dir1")).unwrap(); + assert!(!nested_file.exists()); + + snapshot.restore_with_base(Some(temp_dir.path())).unwrap(); + + assert!(nested_file.exists()); + assert_eq!(fs::read_to_string(&nested_file).unwrap(), "deep content"); + } + + #[test] + fn test_snapshot_path_traversal_protection() { + // Note: this deliberately exercises the *real* home directory, because + // that is the boundary `restore()` defends when no base dir is given. + let mut malicious_snapshot = metadata_only_snapshot( + &Uuid::new_v4().to_string(), + OperationType::Pull, + Duration::zero(), + ); + + // A path that escapes home via `..`; canonicalization should catch it. + let home = dirs::home_dir().unwrap(); + let evil_path = home.join("..").join("..").join("etc").join("passwd"); + + malicious_snapshot.files.insert( + evil_path.to_string_lossy().to_string(), + b"malicious content".to_vec(), + ); + + let result = malicious_snapshot.restore(); + + if let Err(err) = result { + let err_msg = err.to_string(); + assert!( + err_msg.contains("Security") || err_msg.contains("outside home"), + "Error message should indicate security issue: {err_msg}" + ); + } else { + // If it didn't error, at least verify nothing was written outside home. + assert!( + !PathBuf::from("/etc/passwd").exists() + || !fs::read_to_string("/etc/passwd") + .unwrap_or_default() + .contains("malicious") + ); + } + } + + #[test] + fn test_differential_snapshot_restore_with_deletions() { + let temp_dir = tempdir().unwrap(); + let snapshots_dir = temp_dir.path().join("snapshots"); + let file1 = temp_dir.path().join("file1.txt"); + let file2 = temp_dir.path().join("file2.txt"); + + fs::write(&file1, b"content1").unwrap(); + fs::write(&file2, b"content2").unwrap(); + + let snapshot1 = Snapshot::create_differential_with_dir( + OperationType::Pull, + vec![&file1, &file2], + None, + Some(&snapshots_dir), + ) + .unwrap(); + snapshot1.save_to_disk(Some(&snapshots_dir)).unwrap(); + + fs::remove_file(&file2).unwrap(); + + let snapshot2 = Snapshot::create_differential_with_dir( + OperationType::Pull, + vec![&file1], + None, + Some(&snapshots_dir), + ) + .unwrap(); + snapshot2.save_to_disk(Some(&snapshots_dir)).unwrap(); + + // Recreate file2 so the restore has something to delete. + fs::write(&file2, b"should_be_deleted").unwrap(); + + snapshot2 + .restore_with_base_and_snapshots(Some(temp_dir.path()), Some(&snapshots_dir)) + .unwrap(); + + assert!(file1.exists(), "file1 should exist after restore"); + assert!(!file2.exists(), "file2 should be deleted after restore"); + } +} diff --git a/src/undo/snapshot.rs b/src/undo/snapshot.rs index e172d914..d910aa2e 100644 --- a/src/undo/snapshot.rs +++ b/src/undo/snapshot.rs @@ -1,4 +1,4 @@ -use anyhow::{anyhow, Context, Result}; +use anyhow::{Context, Result}; use colored::Colorize; use serde::{Deserialize, Serialize}; use std::collections::HashMap; @@ -113,7 +113,6 @@ impl Snapshot { /// /// # Returns /// A new Snapshot instance with all file contents captured - #[allow(dead_code)] // Used in integration tests pub fn create( operation_type: OperationType, file_paths: I, @@ -163,199 +162,6 @@ impl Snapshot { }) } - /// Create a differential snapshot that only stores changes since the last snapshot - /// - /// This significantly reduces disk usage by only storing files that have changed. - /// - /// # Arguments - /// * `operation_type` - Type of operation this snapshot is for - /// * `file_paths` - Iterator of file paths to include in snapshot - /// * `commit_hash` - Optional git commit hash to store in the snapshot - /// * `snapshots_dir` - Optional custom snapshots directory (for testing) - /// - /// # Returns - /// A new differential Snapshot, or a full snapshot if no base exists - #[allow(dead_code)] // used via the library target; the bin compiles this module separately - pub fn create_differential_with_dir( - operation_type: OperationType, - file_paths: I, - commit_hash: Option<&str>, - snapshots_dir: Option<&Path>, - ) -> Result - where - P: AsRef, - I: IntoIterator, - { - // Try to find the most recent snapshot of the same operation type - let base_snapshot = Self::find_latest_snapshot(operation_type, snapshots_dir)?; - - let snapshot_id = Uuid::new_v4().to_string(); - let timestamp = chrono::Utc::now(); - - // Collect current file paths and their content - let mut current_files: HashMap> = HashMap::new(); - for path in file_paths { - let path = path.as_ref(); - match fs::read(path) { - Ok(content) => { - current_files.insert(path.to_string_lossy().to_string(), content); - } - Err(e) if e.kind() == std::io::ErrorKind::NotFound => { - continue; - } - Err(e) => { - return Err(e).with_context(|| { - format!("Failed to read file for snapshot: {}", path.display()) - }); - } - } - } - - // If no base snapshot exists, create a full snapshot - let (files, base_snapshot_id, deleted_files) = if let Some(base) = base_snapshot { - let mut changed_files = HashMap::new(); - let mut deleted = Vec::new(); - - // Reconstruct the full state from the base snapshot chain - // This is crucial: if the base is differential, we need the complete state, - // not just the changed files in that differential snapshot - let base_full_state = base.reconstruct_full_state_with_dir(snapshots_dir)?; - - // Find files that changed or are new - for (path, content) in ¤t_files { - if let Some(base_content) = base_full_state.get(path) { - // File exists in base - only include if content changed - if base_content != content { - changed_files.insert(path.clone(), content.clone()); - } - } else { - // New file - always include - changed_files.insert(path.clone(), content.clone()); - } - } - - // Find files that were deleted - for path in base_full_state.keys() { - if !current_files.contains_key(path) { - deleted.push(path.clone()); - } - } - - (changed_files, Some(base.snapshot_id), deleted) - } else { - // No base snapshot - include all files (full snapshot) - (current_files, None, Vec::new()) - }; - - Ok(Snapshot { - snapshot_id, - timestamp, - operation_type, - git_commit_hash: commit_hash.map(|s| s.to_string()), - files, - branch: None, - base_snapshot_id, - deleted_files, - }) - } - - /// Create a differential snapshot using the default snapshots directory - /// - /// This is a convenience wrapper around `create_differential_with_dir` that - /// uses the default snapshots directory. - /// - /// # Arguments - /// * `operation_type` - Type of operation this snapshot is for - /// * `file_paths` - Iterator of file paths to include in snapshot - /// * `commit_hash` - Optional git commit hash to store in the snapshot - /// - /// # Returns - /// A new differential Snapshot, or a full snapshot if no base exists - #[allow(dead_code)] // used via the library target; the bin compiles this module separately - pub fn create_differential( - operation_type: OperationType, - file_paths: I, - commit_hash: Option<&str>, - ) -> Result - where - P: AsRef, - I: IntoIterator, - { - Self::create_differential_with_dir(operation_type, file_paths, commit_hash, None) - } - - /// Create a differential snapshot with a commit hash (convenience alias) - /// - /// This is the same as `create_differential` but with a clearer name - /// when used with push operations that need to store a commit hash. - #[allow(dead_code)] // used via the library target; the bin compiles this module separately - pub fn create_differential_with_commit( - operation_type: OperationType, - file_paths: I, - commit_hash: Option<&str>, - ) -> Result - where - P: AsRef, - I: IntoIterator, - { - Self::create_differential(operation_type, file_paths, commit_hash) - } - - /// Find the most recent snapshot of a given operation type - /// - /// # Arguments - /// * `operation_type` - Type of operation to find snapshots for - /// * `custom_dir` - Optional custom snapshots directory (for testing) - /// - /// # Returns - /// The most recent snapshot, or None if no snapshots exist - #[allow(dead_code)] // used via the library target; the bin compiles this module separately - pub(crate) fn find_latest_snapshot( - operation_type: OperationType, - custom_dir: Option<&Path>, - ) -> Result> { - let snapshots_dir = if let Some(dir) = custom_dir { - dir.to_path_buf() - } else { - Self::snapshots_dir()? - }; - - if !snapshots_dir.exists() { - return Ok(None); - } - - let mut snapshots: Vec<(PathBuf, chrono::DateTime)> = Vec::new(); - - // Scan snapshots directory - for entry in fs::read_dir(&snapshots_dir)? { - let entry = entry?; - let path = entry.path(); - - if path.extension().is_none_or(|ext| ext != "json") { - continue; - } - - // Quick parse to get timestamp and operation type without loading full snapshot - if let Ok(content) = fs::read_to_string(&path) { - if let Ok(snapshot) = serde_json::from_str::(&content) { - if snapshot.operation_type == operation_type { - snapshots.push((path, snapshot.timestamp)); - } - } - } - } - - // Sort by timestamp descending and get the most recent - snapshots.sort_by_key(|s| std::cmp::Reverse(s.1)); - - if let Some((path, _)) = snapshots.first() { - let snapshot = Self::load_from_disk(path)?; - Ok(Some(snapshot)) - } else { - Ok(None) - } - } - /// Save this snapshot to disk /// /// Snapshots are saved to `~/.claude-code-sync/snapshots/{snapshot_id}.json` @@ -476,67 +282,169 @@ impl Snapshot { Ok(snapshot) } - /// Reconstruct the full file state by walking the snapshot chain - /// - /// For differential snapshots, this loads all base snapshots recursively - /// and merges them to produce the complete file state. - /// - /// # Arguments - /// * `snapshots_dir` - Optional custom snapshots directory (for testing) - /// - /// # Returns - /// A HashMap containing the full state of all files - pub fn reconstruct_full_state_with_dir( - &self, - snapshots_dir: Option<&Path>, - ) -> Result>> { - let mut state = HashMap::new(); - - // If this is a differential snapshot, load the base chain - if let Some(base_id) = &self.base_snapshot_id { - let snapshots_dir = if let Some(dir) = snapshots_dir { - dir.to_path_buf() - } else { - Self::snapshots_dir()? - }; - let base_path = snapshots_dir.join(format!("{}.json", base_id)); - - if !base_path.exists() { - return Err(anyhow!( - "Base snapshot not found: {}. \ - The snapshot chain is broken. Cannot restore differential snapshot.", - base_id - )); - } + /// Get the default snapshots directory + pub(crate) fn snapshots_dir() -> Result { + crate::config::ConfigManager::snapshots_dir() + } +} - // Recursively load the base snapshot's state - let base_snapshot = Self::load_from_disk(&base_path)?; - state = base_snapshot.reconstruct_full_state_with_dir(Some(&snapshots_dir))?; - } +#[cfg(test)] +mod tests { + use super::*; + use crate::undo::test_support::{create_test_file, setup_test_repo}; + use tempfile::tempdir; - // Apply this snapshot's changes on top of the base state - for (path, content) in &self.files { - state.insert(path.clone(), content.clone()); - } + #[test] + fn test_snapshot_create_and_save() { + let temp_dir = tempdir().unwrap(); + let file1 = create_test_file(temp_dir.path(), "file1.txt", "content 1"); + let file2 = create_test_file(temp_dir.path(), "file2.txt", "content 2"); - // Remove deleted files - for deleted_path in &self.deleted_files { - state.remove(deleted_path); - } + let snapshot = Snapshot::create(OperationType::Pull, vec![&file1, &file2], None).unwrap(); - Ok(state) + assert_eq!(snapshot.operation_type, OperationType::Pull); + assert_eq!(snapshot.files.len(), 2); + assert!(snapshot.git_commit_hash.is_none()); + + let snapshots_dir = temp_dir.path().join("snapshots"); + let snapshot_path = snapshot.save_to_disk(Some(&snapshots_dir)).unwrap(); + assert!(snapshot_path.exists()); } - /// Reconstruct the full file state using the default snapshots directory - /// - /// This is a convenience wrapper around `reconstruct_full_state_with_dir`. - #[allow(dead_code)] // Used in unit tests - pub(crate) fn reconstruct_full_state(&self) -> Result>> { - self.reconstruct_full_state_with_dir(None) + #[test] + fn test_snapshot_with_commit_hash() { + let (temp_dir, repo) = setup_test_repo(); + let file1 = temp_dir.path().join("test.txt"); + let commit_hash = repo.current_commit_hash().unwrap(); + + let snapshot = + Snapshot::create(OperationType::Push, vec![&file1], Some(&commit_hash)).unwrap(); + + assert_eq!(snapshot.operation_type, OperationType::Push); + let stored_hash = snapshot.git_commit_hash.unwrap(); + assert_eq!(stored_hash.len(), 40); // Git SHA-1 hash length + assert_eq!(stored_hash, commit_hash); } - /// Get the default snapshots directory - pub(crate) fn snapshots_dir() -> Result { - crate::config::ConfigManager::snapshots_dir() + #[test] + fn test_snapshot_save_and_load() { + let temp_dir = tempdir().unwrap(); + let file1 = create_test_file(temp_dir.path(), "file1.txt", "test content"); + + let original = Snapshot::create(OperationType::Pull, vec![&file1], None).unwrap(); + + let snapshots_dir = temp_dir.path().join("snapshots"); + let snapshot_path = original.save_to_disk(Some(&snapshots_dir)).unwrap(); + + let loaded = Snapshot::load_from_disk(&snapshot_path).unwrap(); + + assert_eq!(loaded.snapshot_id, original.snapshot_id); + assert_eq!(loaded.operation_type, original.operation_type); + assert_eq!(loaded.files.len(), original.files.len()); + } + + #[test] + fn test_snapshot_handles_binary_files() { + let temp_dir = tempdir().unwrap(); + let binary_file = temp_dir.path().join("binary.dat"); + + let binary_content: Vec = vec![0xFF, 0xFE, 0x00, 0x01, 0x02, 0x03]; + fs::write(&binary_file, &binary_content).unwrap(); + + let snapshot = Snapshot::create(OperationType::Pull, vec![&binary_file], None).unwrap(); + + let key = binary_file.to_string_lossy().to_string(); + assert_eq!(snapshot.files.get(&key).unwrap(), &binary_content); + + // The base64 serde shim has to survive a disk round-trip, not just + // hold the bytes in memory. + let snapshots_dir = temp_dir.path().join("snapshots"); + let snapshot_path = snapshot.save_to_disk(Some(&snapshots_dir)).unwrap(); + + let loaded = Snapshot::load_from_disk(&snapshot_path).unwrap(); + assert_eq!(loaded.files.get(&key).unwrap(), &binary_content); + } + + #[test] + fn test_snapshot_serialization_with_special_characters() { + let temp_dir = tempdir().unwrap(); + let file_with_unicode = temp_dir.path().join("日本語.txt"); + fs::write(&file_with_unicode, "Hello 世界").unwrap(); + + let snapshot = + Snapshot::create(OperationType::Pull, vec![&file_with_unicode], None).unwrap(); + + let snapshots_dir = temp_dir.path().join("snapshots"); + let snapshot_path = snapshot.save_to_disk(Some(&snapshots_dir)).unwrap(); + + let loaded = Snapshot::load_from_disk(&snapshot_path).unwrap(); + + let content = loaded.files.values().next().unwrap(); + assert_eq!(String::from_utf8_lossy(content), "Hello 世界"); + } + + #[test] + fn test_base64_encoding_for_binary_data() { + let temp_dir = tempdir().unwrap(); + + // Every possible byte value, including those that are not valid UTF-8. + let binary_file = temp_dir.path().join("binary.dat"); + let binary_data: Vec = (0..=255).collect(); + fs::write(&binary_file, &binary_data).unwrap(); + + let snapshot = Snapshot::create(OperationType::Pull, vec![&binary_file], None).unwrap(); + + let json = serde_json::to_string(&snapshot).unwrap(); + let _parsed: serde_json::Value = serde_json::from_str(&json).unwrap(); + + let deserialized: Snapshot = serde_json::from_str(&json).unwrap(); + + let key = binary_file.to_string_lossy().to_string(); + assert_eq!( + snapshot.files.get(&key).unwrap(), + deserialized.files.get(&key).unwrap() + ); + assert_eq!(deserialized.files.get(&key).unwrap(), &binary_data); + } + + #[test] + fn test_empty_snapshot() { + let snapshot = Snapshot::create::(OperationType::Pull, vec![], None).unwrap(); + + assert_eq!(snapshot.files.len(), 0); + assert!(snapshot.git_commit_hash.is_none()); + + let temp_dir = tempdir().unwrap(); + let snapshot_path = snapshot.save_to_disk(Some(temp_dir.path())).unwrap(); + + let loaded = Snapshot::load_from_disk(&snapshot_path).unwrap(); + assert_eq!(loaded.files.len(), 0); + + // Restoring nothing must be a no-op, not an error. + loaded.restore().unwrap(); + } + + #[test] + fn test_snapshot_create_handles_missing_files() { + let temp_dir = tempdir().unwrap(); + + let existing_file = create_test_file(temp_dir.path(), "exists.txt", "content"); + let missing_file = temp_dir.path().join("does_not_exist.txt"); + + // A path that has already been deleted is skipped, not an error. + let snapshot = Snapshot::create( + OperationType::Pull, + vec![&existing_file, &missing_file], + None, + ) + .unwrap(); + + assert_eq!(snapshot.files.len(), 1); + assert!(snapshot + .files + .contains_key(&existing_file.to_string_lossy().to_string())); + assert!(!snapshot + .files + .contains_key(&missing_file.to_string_lossy().to_string())); } } diff --git a/src/undo/test_support.rs b/src/undo/test_support.rs new file mode 100644 index 00000000..687d5d0a --- /dev/null +++ b/src/undo/test_support.rs @@ -0,0 +1,137 @@ +//! Shared fixtures for the `undo` unit tests. +//! +//! These live here rather than in each sibling module because the setup they +//! encode is genuinely shared: the ten `operations` tests each need a snapshot +//! on disk plus an `OperationHistory` referencing it, and before this module +//! existed they all spelled that out longhand. + +// Each sibling test module uses a different subset of these. `clippy +// --all-targets` compiles `cfg(test)` code, so a helper unused by one of them +// would otherwise be a `dead_code` warning, and CI runs with `-D warnings`. +#![allow(dead_code)] + +use std::collections::HashMap; +use std::fs; +use std::path::{Path, PathBuf}; + +use chrono::Duration; +use tempfile::{tempdir, TempDir}; + +use super::Snapshot; +use crate::history::{ + ConversationSummary, OperationHistory, OperationRecord, OperationType, SyncOperation, +}; +use crate::scm::{self, Scm}; + +/// Write `content` to `dir/name` and return the path. +pub(super) fn create_test_file(dir: &Path, name: &str, content: &str) -> PathBuf { + let path = dir.join(name); + fs::write(&path, content).unwrap(); + path +} + +/// A temp dir holding an initialized SCM repo with one committed file. +pub(super) fn setup_test_repo() -> (TempDir, Box) { + let temp_dir = tempdir().unwrap(); + let repo = scm::init(temp_dir.path()).unwrap(); + + let test_file = temp_dir.path().join("test.txt"); + fs::write(&test_file, "initial content").unwrap(); + repo.stage_all().unwrap(); + repo.commit("Initial commit").unwrap(); + + (temp_dir, repo) +} + +/// A snapshot carrying metadata but no file contents. +/// +/// Cleanup selects purely on `operation_type` and `timestamp`, so the tests +/// that exercise it never need real file bodies. Callers that care about the +/// commit hash or branch set those fields afterwards. +pub(super) fn metadata_only_snapshot( + id: &str, + operation_type: OperationType, + age: Duration, +) -> Snapshot { + Snapshot { + snapshot_id: id.to_string(), + timestamp: chrono::Utc::now() - age, + operation_type, + git_commit_hash: None, + files: HashMap::new(), + branch: None, + base_snapshot_id: None, + deleted_files: Vec::new(), + } +} + +/// Builds an `OperationHistory` on disk. +/// +/// The undo tests care about which operations are present, in what order, and +/// which snapshot each can be undone from. The conversation summaries attached +/// to each record are filler — nothing asserts on them. +pub(super) struct HistoryBuilder { + path: PathBuf, + history: OperationHistory, +} + +impl HistoryBuilder { + pub(super) fn new(history_path: &Path) -> Self { + Self { + path: history_path.to_path_buf(), + history: OperationHistory::from_path(Some(history_path.to_path_buf())).unwrap(), + } + } + + /// Append one operation. `snapshot` is the snapshot file it can be undone + /// from; `None` models an operation recorded without one. + pub(super) fn push( + mut self, + operation_type: OperationType, + branch: &str, + snapshot: Option<&Path>, + ) -> Self { + let sync_op = match operation_type { + OperationType::Pull => SyncOperation::Modified, + _ => SyncOperation::Added, + }; + let summary = ConversationSummary::new( + "test-session".to_string(), + "test/path".to_string(), + None, + 5, + sync_op, + ) + .unwrap(); + + let mut record = + OperationRecord::new(operation_type, Some(branch.to_string()), vec![summary]); + record.snapshot_path = snapshot.map(Path::to_path_buf); + + self.history.add_operation(record).unwrap(); + self + } + + /// Persist the accumulated operations. + pub(super) fn save(self) { + self.history.save_to(Some(self.path)).unwrap(); + } +} + +/// Number of operations currently recorded at `history_path`. +pub(super) fn operation_count(history_path: &Path) -> usize { + OperationHistory::from_path(Some(history_path.to_path_buf())) + .unwrap() + .len() +} + +/// The operations recorded at `history_path`, **most recent first** — +/// `add_operation` inserts at index 0. +pub(super) fn operation_types(history_path: &Path) -> Vec { + OperationHistory::from_path(Some(history_path.to_path_buf())) + .unwrap() + .list_operations() + .iter() + .map(|op| op.operation_type) + .collect() +} From 803005a436a91d55241e7e8ef5180e33d72d955e Mon Sep 17 00:00:00 2001 From: perfectra1n Date: Mon, 13 Jul 2026 13:09:06 -0700 Subject: [PATCH 3/7] fix(config): reject a non-positive max file size, and split handlers/config.rs BEHAVIOR CHANGE: `config` and `config --wizard` now reject a non-positive or non-finite max file size instead of silently accepting it. handlers/config.rs was 1090 lines holding four unrelated commands, and the two editing modes each carried their own copy of the same value logic: the comma-separated pattern parser appeared four times (include/exclude x interactive/wizard) and the megabyte parser twice. Neither copy validated anything, and neither could be tested -- the parsing was welded directly to the `inquire` prompt, so it only ran with a TTY attached. Split by concern: fields.rs pure value logic, no I/O -- and now unit-tested prompts.rs the terminal glue the two modes share interactive.rs MultiSelect, then edit the chosen settings wizard.rs Confirm-gated walk through every setting repo_select.rs the no-argument repo menu export.rs --export, plus its six tests `prompt_artifact_toggle_selection` lives in prompts.rs, not fields.rs: it runs a MultiSelect, so it is not TTY-free and would have made that module's whole premise false. The two modes keep their distinct prompt flows. They are not accidentally different: interactive treats empty input as "clear this setting" and says so, while the wizard asks "Do you want to ...?" first and has no clear affordance. Only the value logic is shared, not the prompt shape. The file-size fix: both copies did `parse::()` then `(mb * 1024.0 * 1024.0) as u64`. Rust's float-to-int cast saturates, so `-5` became a 0-byte limit -- which makes FilterConfig::should_include reject every file -- and `1e30` became u64::MAX. The `.context("Must be a positive number")` already claimed a check that was never performed. parse_file_size_mb now actually performs it, and eight tests pin the behaviour down. Also give the export tests an RAII guard. They mutate the process cwd and an env var, and previously restored both in a trailing statement that a failed assertion would skip, leaving the harness pointed at a deleted temp dir. Tests: 336 -> 347 (11 new in fields.rs). The 6 export tests are renamed, not lost. --- src/handlers/config.rs | 1090 ---------------------------- src/handlers/config/export.rs | 335 +++++++++ src/handlers/config/fields.rs | 146 ++++ src/handlers/config/interactive.rs | 186 +++++ src/handlers/config/mod.rs | 21 + src/handlers/config/prompts.rs | 111 +++ src/handlers/config/repo_select.rs | 219 ++++++ src/handlers/config/wizard.rs | 219 ++++++ 8 files changed, 1237 insertions(+), 1090 deletions(-) delete mode 100644 src/handlers/config.rs create mode 100644 src/handlers/config/export.rs create mode 100644 src/handlers/config/fields.rs create mode 100644 src/handlers/config/interactive.rs create mode 100644 src/handlers/config/mod.rs create mode 100644 src/handlers/config/prompts.rs create mode 100644 src/handlers/config/repo_select.rs create mode 100644 src/handlers/config/wizard.rs diff --git a/src/handlers/config.rs b/src/handlers/config.rs deleted file mode 100644 index c99c9ab1..00000000 --- a/src/handlers/config.rs +++ /dev/null @@ -1,1090 +0,0 @@ -//! Configuration command handlers -//! -//! Handles interactive configuration management including wizard mode -//! and menu-based configuration editing. - -use anyhow::{Context, Result}; -use colored::Colorize; -use inquire::{Confirm, MultiSelect, Select, Text}; - -use crate::config::ConfigManager; -use crate::filter::FilterConfig; -use crate::onboarding::InitConfig; -use crate::scm; -use crate::sync::{MultiRepoState, RepoConfig, SyncState}; -use std::collections::HashMap; -use std::path::PathBuf; - -/// Handle interactive configuration menu -/// -/// Shows all configuration options and allows user to select which ones to modify -pub fn handle_config_interactive() -> Result<()> { - println!("{}", "Interactive Configuration".cyan().bold()); - println!("{}", "=".repeat(80).cyan()); - println!(); - - // Load current configuration - let current_config = FilterConfig::load().context("Failed to load current configuration")?; - - // Display current configuration - println!("{}", "Current Settings:".bold()); - display_config_summary(¤t_config); - println!(); - - // Define available configuration options - let options = vec![ - "Exclude older than (days)", - "Include patterns", - "Exclude patterns", - "Exclude attachments", - "Max file size", - "Artifact sync categories", - ]; - - // Let user select which settings to modify - let selections = MultiSelect::new( - "Select settings to modify (Space to select, Enter to confirm):", - options, - ) - .with_help_message("Use arrow keys to navigate, Space to select/deselect, Enter when done") - .prompt() - .context("Failed to get user selections")?; - - if selections.is_empty() { - println!( - "{}", - "No settings selected. Configuration unchanged.".yellow() - ); - return Ok(()); - } - - println!(); - println!("{}", "Modifying selected settings:".cyan().bold()); - println!(); - - // Process each selected setting - let mut modified_config = current_config.clone(); - - for selection in selections { - match selection { - "Exclude older than (days)" => { - let current = modified_config - .exclude_older_than_days - .map(|d| d.to_string()) - .unwrap_or_else(|| "Not set".to_string()); - - let input = Text::new("Exclude older than (days):") - .with_help_message(&format!( - "Current: {}. Enter a number or leave empty to unset", - current - )) - .prompt()?; - - if input.trim().is_empty() { - modified_config.exclude_older_than_days = None; - println!(" {} Unset exclude_older_than_days", "✓".green()); - } else { - let days: u32 = input - .trim() - .parse() - .context("Invalid number. Must be a positive integer.")?; - modified_config.exclude_older_than_days = Some(days); - println!( - " {} Set exclude_older_than_days to {} days", - "✓".green(), - days - ); - } - } - - "Include patterns" => { - let current = if modified_config.include_patterns.is_empty() { - "None".to_string() - } else { - modified_config.include_patterns.join(", ") - }; - - let input = Text::new("Include patterns (comma-separated):") - .with_help_message(&format!( - "Current: {}. Glob patterns like '*work*' or '/path/to/project'", - current - )) - .prompt()?; - - if input.trim().is_empty() { - modified_config.include_patterns = Vec::new(); - println!(" {} Cleared include patterns", "✓".green()); - } else { - modified_config.include_patterns = input - .split(',') - .map(|s| s.trim().to_string()) - .filter(|s| !s.is_empty()) - .collect(); - println!( - " {} Set include patterns: {:?}", - "✓".green(), - modified_config.include_patterns - ); - } - } - - "Exclude patterns" => { - let current = if modified_config.exclude_patterns.is_empty() { - "None".to_string() - } else { - modified_config.exclude_patterns.join(", ") - }; - - let input = Text::new("Exclude patterns (comma-separated):") - .with_help_message(&format!( - "Current: {}. Glob patterns like '*test*' or '/tmp/*'", - current - )) - .prompt()?; - - if input.trim().is_empty() { - modified_config.exclude_patterns = Vec::new(); - println!(" {} Cleared exclude patterns", "✓".green()); - } else { - modified_config.exclude_patterns = input - .split(',') - .map(|s| s.trim().to_string()) - .filter(|s| !s.is_empty()) - .collect(); - println!( - " {} Set exclude patterns: {:?}", - "✓".green(), - modified_config.exclude_patterns - ); - } - } - - "Exclude attachments" => { - let current = modified_config.exclude_attachments; - - let exclude = Confirm::new("Exclude attachments (images, PDFs, etc.)?") - .with_default(current) - .with_help_message(&format!( - "Current: {}. If yes, only .jsonl files will be synced", - current - )) - .prompt()?; - - modified_config.exclude_attachments = exclude; - println!(" {} Set exclude_attachments to {}", "✓".green(), exclude); - } - - "Artifact sync categories" => { - modified_config.sync_artifacts = - prompt_artifact_toggle_selection(&modified_config.sync_artifacts)?; - } - - "Max file size" => { - let current_mb = modified_config.max_file_size_bytes as f64 / (1024.0 * 1024.0); - - let input = Text::new("Max file size (MB):") - .with_default(&format!("{:.1}", current_mb)) - .with_help_message("Maximum size for individual files (e.g., 10 for 10MB)") - .prompt()?; - - let size_mb: f64 = input - .trim() - .parse() - .context("Invalid number. Must be a positive number.")?; - - modified_config.max_file_size_bytes = (size_mb * 1024.0 * 1024.0) as u64; - println!(" {} Set max_file_size to {:.1} MB", "✓".green(), size_mb); - } - - _ => {} - } - println!(); - } - - // Show final configuration and confirm - println!("{}", "New Configuration:".cyan().bold()); - display_config_summary(&modified_config); - println!(); - - let confirm = Confirm::new("Save this configuration?") - .with_default(true) - .prompt()?; - - if confirm { - modified_config - .save() - .context("Failed to save configuration")?; - println!("\n{} Configuration saved successfully!", "✓".green().bold()); - } else { - println!("\n{}", "Configuration not saved.".yellow()); - } - - Ok(()) -} - -/// Handle wizard-mode configuration -/// -/// Steps through each configuration option one by one -pub fn handle_config_wizard() -> Result<()> { - println!("{}", "Configuration Wizard".cyan().bold()); - println!("{}", "=".repeat(80).cyan()); - println!(); - println!( - "{}", - "This wizard will walk you through all configuration options.".dimmed() - ); - println!( - "{}", - "Press Enter to keep current value or enter a new value.".dimmed() - ); - println!(); - - // Load current configuration - let current_config = FilterConfig::load().context("Failed to load current configuration")?; - let mut modified_config = current_config.clone(); - - // 1. Exclude older than - println!("{}", "1. Age Filter".bold().cyan()); - let current_age = modified_config - .exclude_older_than_days - .map(|d| d.to_string()) - .unwrap_or_else(|| "Not set".to_string()); - println!(" Current: {}", current_age.yellow()); - - let exclude_old = - Confirm::new("Do you want to exclude projects older than a certain number of days?") - .with_default(modified_config.exclude_older_than_days.is_some()) - .prompt()?; - - if exclude_old { - let default_days = modified_config - .exclude_older_than_days - .unwrap_or(30) - .to_string(); - let input = Text::new("How many days?") - .with_default(&default_days) - .prompt()?; - - let days: u32 = input - .trim() - .parse() - .context("Invalid number. Must be a positive integer.")?; - modified_config.exclude_older_than_days = Some(days); - println!( - " {} Will exclude projects older than {} days\n", - "✓".green(), - days - ); - } else { - modified_config.exclude_older_than_days = None; - println!(" {} Age filter disabled\n", "✓".green()); - } - - // 2. Include patterns - println!("{}", "2. Include Patterns".bold().cyan()); - let current_include = if modified_config.include_patterns.is_empty() { - "None (all projects included)".to_string() - } else { - modified_config.include_patterns.join(", ") - }; - println!(" Current: {}", current_include.yellow()); - - let use_include = Confirm::new("Do you want to limit sync to specific project patterns?") - .with_default(!modified_config.include_patterns.is_empty()) - .with_help_message("Example: *work*, /home/user/important/*") - .prompt()?; - - if use_include { - let default = modified_config.include_patterns.join(", "); - let input = Text::new("Enter include patterns (comma-separated):") - .with_default(&default) - .with_help_message("Glob patterns like '*work*' or '/specific/path'") - .prompt()?; - - modified_config.include_patterns = input - .split(',') - .map(|s| s.trim().to_string()) - .filter(|s| !s.is_empty()) - .collect(); - println!( - " {} Include patterns set: {:?}\n", - "✓".green(), - modified_config.include_patterns - ); - } else { - modified_config.include_patterns = Vec::new(); - println!(" {} All projects will be included\n", "✓".green()); - } - - // 3. Exclude patterns - println!("{}", "3. Exclude Patterns".bold().cyan()); - let current_exclude = if modified_config.exclude_patterns.is_empty() { - "None".to_string() - } else { - modified_config.exclude_patterns.join(", ") - }; - println!(" Current: {}", current_exclude.yellow()); - - let use_exclude = Confirm::new("Do you want to exclude specific project patterns?") - .with_default(!modified_config.exclude_patterns.is_empty()) - .with_help_message("Example: *test*, *tmp*, /temp/*") - .prompt()?; - - if use_exclude { - let default = modified_config.exclude_patterns.join(", "); - let input = Text::new("Enter exclude patterns (comma-separated):") - .with_default(&default) - .with_help_message("Glob patterns like '*test*' or '/tmp/*'") - .prompt()?; - - modified_config.exclude_patterns = input - .split(',') - .map(|s| s.trim().to_string()) - .filter(|s| !s.is_empty()) - .collect(); - println!( - " {} Exclude patterns set: {:?}\n", - "✓".green(), - modified_config.exclude_patterns - ); - } else { - modified_config.exclude_patterns = Vec::new(); - println!(" {} No exclusion patterns\n", "✓".green()); - } - - // 4. Exclude attachments - println!("{}", "4. File Type Filter".bold().cyan()); - println!( - " Current: {}", - if modified_config.exclude_attachments { - "Exclude attachments".yellow() - } else { - "Include all files".yellow() - } - ); - - let exclude_attachments = Confirm::new("Exclude attachments (images, PDFs, etc.)?") - .with_default(modified_config.exclude_attachments) - .with_help_message("If yes, only .jsonl conversation files will be synced") - .prompt()?; - - modified_config.exclude_attachments = exclude_attachments; - println!( - " {} Attachments will be {}\n", - "✓".green(), - if exclude_attachments { - "excluded" - } else { - "included" - } - ); - - // 5. Max file size - println!("{}", "5. File Size Limit".bold().cyan()); - let current_mb = modified_config.max_file_size_bytes as f64 / (1024.0 * 1024.0); - println!(" Current: {:.1} MB", current_mb); - - let change_size = Confirm::new("Do you want to change the maximum file size limit?") - .with_default(false) - .prompt()?; - - if change_size { - let input = Text::new("Max file size (MB):") - .with_default(&format!("{:.1}", current_mb)) - .prompt()?; - - let size_mb: f64 = input - .trim() - .parse() - .context("Invalid number. Must be a positive number.")?; - - modified_config.max_file_size_bytes = (size_mb * 1024.0 * 1024.0) as u64; - println!(" {} Max file size set to {:.1} MB\n", "✓".green(), size_mb); - } else { - println!(" {} Keeping current max file size\n", "✓".green()); - } - - // Artifact sync categories - let change_artifacts = - Confirm::new("Configure artifact sync categories (settings, skills, agents, ...)?") - .with_default(false) - .prompt()?; - if change_artifacts { - modified_config.sync_artifacts = - prompt_artifact_toggle_selection(&modified_config.sync_artifacts)?; - } - - // Summary and confirmation - println!("{}", "=".repeat(80).cyan()); - println!("{}", "Configuration Summary:".bold().cyan()); - println!("{}", "=".repeat(80).cyan()); - display_config_summary(&modified_config); - println!(); - - let confirm = Confirm::new("Save this configuration?") - .with_default(true) - .prompt()?; - - if confirm { - modified_config - .save() - .context("Failed to save configuration")?; - println!("\n{} Configuration saved successfully!", "✓".green().bold()); - } else { - println!("\n{}", "Configuration not saved.".yellow()); - } - - Ok(()) -} - -/// Display a compact configuration summary -fn display_config_summary(config: &FilterConfig) { - println!( - " {} {}", - "Exclude older than:".cyan(), - config - .exclude_older_than_days - .map(|d| format!("{} days", d)) - .unwrap_or_else(|| "Not set".dimmed().to_string()) - ); - - println!( - " {} {}", - "Include patterns:".cyan(), - if config.include_patterns.is_empty() { - "None (all included)".dimmed().to_string() - } else { - config.include_patterns.join(", ") - } - ); - - println!( - " {} {}", - "Exclude patterns:".cyan(), - if config.exclude_patterns.is_empty() { - "None".dimmed().to_string() - } else { - config.exclude_patterns.join(", ") - } - ); - - println!( - " {} {:.1} MB", - "Max file size:".cyan(), - config.max_file_size_bytes as f64 / (1024.0 * 1024.0) - ); - - println!( - " {} {}", - "Exclude attachments:".cyan(), - if config.exclude_attachments { - "Yes (only .jsonl files)".green().to_string() - } else { - "No (all files)".yellow().to_string() - } - ); - - let enabled: Vec<&str> = crate::artifacts::registry::toggleable() - .filter(|d| config.sync_artifacts.is_enabled(d.id)) - .map(|d| d.name) - .collect(); - println!( - " {} {}", - "Artifact sync:".cyan(), - if enabled.is_empty() { - "All disabled".dimmed().to_string() - } else { - enabled.join(", ") - } - ); -} - -/// Try to recover an existing repo if state.json is missing but repo exists -/// -/// This handles the case where a user has a valid repo in the default location -/// but the state.json file is missing (e.g., from an older version or deletion). -fn try_recover_existing_repo() -> Result> { - // Check if default repo directory exists and is a valid git repo - let default_repo = match ConfigManager::default_repo_dir() { - Ok(path) => path, - Err(_) => return Ok(None), - }; - - if !default_repo.exists() || !scm::is_repo(&default_repo) { - return Ok(None); - } - - // Try to detect if it has a remote - let (has_remote, remote_url) = match scm::open(&default_repo) { - Ok(repo) => { - let has_remote = repo.has_remote("origin"); - let remote_url = if has_remote { - repo.get_remote_url("origin").ok() - } else { - None - }; - (has_remote, remote_url) - } - Err(_) => (false, None), - }; - - println!( - "{} Found existing repo at: {}", - "!".yellow(), - default_repo.display() - ); - if let Some(ref url) = remote_url { - println!(" Remote: {}", url.cyan()); - } - println!(" Recovering configuration..."); - println!(); - - // Create the recovered state - let repo_config = RepoConfig { - name: "default".to_string(), - sync_repo_path: default_repo, - has_remote, - is_cloned_repo: false, // We can't know this for sure - remote_url, - description: Some("Recovered from existing repository".to_string()), - }; - - let mut repos = HashMap::new(); - repos.insert("default".to_string(), repo_config); - - let state = MultiRepoState { - version: 2, - active_repo: "default".to_string(), - repos, - }; - - // Save the recovered state - state.save()?; - - Ok(Some(state)) -} - -/// Handle the repository selector menu -/// -/// Shows when `claude-code-sync config` is run with no arguments. -/// Displays all configured repositories and allows switching between them. -pub fn handle_repo_selector() -> Result<()> { - println!("{}", "Repository Configuration".cyan().bold()); - println!("{}", "=".repeat(60).cyan()); - println!(); - - // Try to load state, but handle "not initialized" gracefully - let mut state = match MultiRepoState::load() { - Ok(s) => s, - Err(e) => { - let err_msg = e.to_string(); - if err_msg.contains("not initialized") - || err_msg.contains("Run 'claude-code-sync init'") - { - // Check if there's an existing repo in the default location that we can recover - if let Some(recovered) = try_recover_existing_repo()? { - println!( - "{}", - "Found existing repository - recovered configuration!".green() - ); - println!(); - recovered - } else { - println!("{}", "No repositories configured.".yellow()); - println!(); - println!( - "Run '{}' to set up your first repository.", - "claude-code-sync init".cyan() - ); - return Ok(()); - } - } else { - return Err(e); - } - } - }; - - if state.repos.is_empty() { - println!("{}", "No repositories configured.".yellow()); - println!(); - println!( - "Run '{}' to set up your first repository.", - "claude-code-sync init".cyan() - ); - return Ok(()); - } - - // Build sorted list of repos (active first, then alphabetical) - let mut repo_entries: Vec<_> = state.repos.values().collect(); - repo_entries.sort_by(|a, b| { - // Active repo first - if a.name == state.active_repo { - std::cmp::Ordering::Less - } else if b.name == state.active_repo { - std::cmp::Ordering::Greater - } else { - a.name.cmp(&b.name) - } - }); - - // Build display options - let mut options: Vec = repo_entries - .iter() - .map(|repo| { - let active_marker = if repo.name == state.active_repo { - format!(" {}", "[ACTIVE]".green().bold()) - } else { - String::new() - }; - - let path_str = repo.sync_repo_path.display().to_string(); - let remote_info = repo - .remote_url - .as_ref() - .map(|u| format!(" ({})", u.dimmed())) - .unwrap_or_default(); - - format!( - "{}{} - {}{}", - repo.name, active_marker, path_str, remote_info - ) - }) - .collect(); - - // Add separator and management options - options.push(format!("{}", "─── Actions ───".dimmed())); - options.push("Configure filters (current repo)".to_string()); - options.push("Exit".to_string()); - - let selection = Select::new("Select a repository to make active:", options.clone()) - .with_help_message("Use arrow keys to navigate, Enter to select") - .prompt() - .context("Failed to get user selection")?; - - // Handle selection - if selection.contains("─── Actions ───") || selection == "Exit" { - return Ok(()); - } - - if selection == "Configure filters (current repo)" { - return handle_config_interactive(); - } - - // Extract repo name from selection (first word before space or marker) - let repo_name = selection - .split_whitespace() - .next() - .ok_or_else(|| anyhow::anyhow!("Invalid selection"))?; - - // Check if this repo is already active - if repo_name == state.active_repo { - println!(); - println!( - "{} '{}' is already the active repository.", - "ℹ".blue(), - repo_name.cyan() - ); - return Ok(()); - } - - // Switch to selected repo - if state.repos.contains_key(repo_name) { - state.active_repo = repo_name.to_string(); - state.save()?; - - println!(); - println!( - "{} Switched to repository '{}'", - "✓".green().bold(), - repo_name.cyan() - ); - - // Show repo details - if let Some(repo) = state.repos.get(repo_name) { - println!(" Path: {}", repo.sync_repo_path.display()); - if let Some(ref url) = repo.remote_url { - println!(" Remote: {}", url); - } - if repo.has_remote { - println!(" Has remote: {}", "Yes".green()); - } else { - println!(" Has remote: {}", "No (local only)".yellow()); - } - } - } else { - return Err(anyhow::anyhow!("Repository '{}' not found", repo_name)); - } - - Ok(()) -} - -/// Export current config as a claude-code-sync-init.toml in the current directory. -/// -/// Reads the active sync state and filter config, builds an InitConfig, and writes -/// it to `./claude-code-sync-init.toml`. -pub fn handle_config_export() -> Result<()> { - let filter = FilterConfig::load().context("Failed to load filter configuration")?; - - let repo_path = match SyncState::load() { - Ok(state) => state.sync_repo_path.to_string_lossy().to_string(), - Err(e) => { - eprintln!( - "{} Could not load sync state ({}), using default repo_path", - "!".yellow(), - e - ); - "~/claude-code-sync-repo".to_string() - } - }; - - let remote_url = match MultiRepoState::load() { - Ok(ms) => match ms.repos.get(&ms.active_repo) { - Some(repo) => repo.remote_url.clone(), - None => { - eprintln!( - "{} Active repo '{}' not found in multi-repo state, remote_url will be unset", - "!".yellow(), - ms.active_repo - ); - None - } - }, - Err(e) => { - eprintln!( - "{} Could not load multi-repo state ({}), remote_url will be unset", - "!".yellow(), - e - ); - None - } - }; - - // Warn about filter settings that are not captured in InitConfig - if !filter.include_patterns.is_empty() { - eprintln!( - "{} include_patterns are not included in the export and will need to be reconfigured", - "!".yellow() - ); - } - if !filter.exclude_patterns.is_empty() { - eprintln!( - "{} exclude_patterns are not included in the export and will need to be reconfigured", - "!".yellow() - ); - } - - let init_config = InitConfig { - repo_path, - remote_url: remote_url.clone(), - clone: remote_url.is_some(), - exclude_attachments: filter.exclude_attachments, - exclude_older_than_days: filter.exclude_older_than_days, - enable_lfs: filter.enable_lfs, - scm_backend: filter.scm_backend, - sync_subdirectory: filter.sync_subdirectory, - use_project_name_only: filter.use_project_name_only, - sync_artifacts: filter.sync_artifacts.clone(), - }; - - let content = - toml::to_string_pretty(&init_config).context("Failed to serialize init config")?; - - let output_path = PathBuf::from("claude-code-sync-init.toml"); - std::fs::write(&output_path, content) - .with_context(|| format!("Failed to write {}", output_path.display()))?; - - println!( - "{} Exported to {}", - "✓".green().bold(), - output_path.display() - ); - - Ok(()) -} - -/// MultiSelect over all artifact categories, pre-selecting the currently -/// enabled ones. Returns the resulting toggles. -fn prompt_artifact_toggle_selection( - current: &crate::artifacts::registry::ArtifactToggles, -) -> Result { - use crate::artifacts::registry::{find_by_name, toggleable, ArtifactToggles}; - - let rows: Vec<_> = toggleable().collect(); - let options: Vec = rows - .iter() - .map(|d| format!("{} — {}", d.name, d.description)) - .collect(); - let preselected: Vec = rows - .iter() - .enumerate() - .filter(|(_, d)| current.is_enabled(d.id)) - .map(|(i, _)| i) - .collect(); - - let picked = MultiSelect::new("Artifact categories to sync:", options) - .with_default(&preselected) - .with_help_message( - "Space toggles, Enter confirms. Secrets (credentials, settings.local.json, \ - .env*, keys) are never synced regardless of selection.", - ) - .prompt() - .context("Failed to get artifact category selection")?; - - let mut toggles = ArtifactToggles::default(); - for label in picked { - let name = label.split(" — ").next().unwrap_or(&label); - if let Some(desc) = find_by_name(name) { - toggles.set_enabled(desc.id, true); - } - } - Ok(toggles) -} - -#[cfg(test)] -mod tests { - use super::*; - use serial_test::serial; - use std::collections::HashMap; - use std::path::Path; - use tempfile::TempDir; - - /// Set up an isolated config environment via CLAUDE_CODE_SYNC_CONFIG_DIR (honored - /// on every platform, unlike XDG_CONFIG_HOME which macOS ignores). - /// Returns (config_temp_dir, working_temp_dir) — the working dir is where the - /// exported TOML will be written. - fn setup_export_env() -> (TempDir, TempDir, String) { - let config_dir = TempDir::new().unwrap(); - let work_dir = TempDir::new().unwrap(); - let old_dir = std::env::current_dir() - .unwrap() - .to_string_lossy() - .to_string(); - std::env::set_var("CLAUDE_CODE_SYNC_CONFIG_DIR", config_dir.path()); - std::env::set_current_dir(work_dir.path()).unwrap(); - (config_dir, work_dir, old_dir) - } - - fn teardown(old_dir: &str) { - std::env::remove_var("CLAUDE_CODE_SYNC_CONFIG_DIR"); - std::env::set_current_dir(old_dir).unwrap(); - } - - /// Write a v2 MultiRepoState to the config dir's state.json - fn write_multi_repo_state(config_dir: &Path, state: &MultiRepoState) { - let claude_dir = config_dir.join("claude-code-sync"); - std::fs::create_dir_all(&claude_dir).unwrap(); - let content = serde_json::to_string_pretty(state).unwrap(); - std::fs::write(claude_dir.join("state.json"), content).unwrap(); - } - - /// Write a FilterConfig to the config dir's config.toml - fn write_filter_config(config_dir: &Path, filter: &FilterConfig) { - let claude_dir = config_dir.join("claude-code-sync"); - std::fs::create_dir_all(&claude_dir).unwrap(); - let content = toml::to_string_pretty(filter).unwrap(); - std::fs::write(claude_dir.join("config.toml"), content).unwrap(); - } - - fn make_multi_repo_state(repo_path: &str, remote_url: Option) -> MultiRepoState { - let mut repos = HashMap::new(); - repos.insert( - "default".to_string(), - RepoConfig { - name: "default".to_string(), - sync_repo_path: PathBuf::from(repo_path), - has_remote: remote_url.is_some(), - is_cloned_repo: false, - remote_url, - description: None, - }, - ); - MultiRepoState { - version: 2, - active_repo: "default".to_string(), - repos, - } - } - - /// Read back the exported TOML from the working directory - fn read_exported_config(work_dir: &Path) -> InitConfig { - let path = work_dir.join("claude-code-sync-init.toml"); - assert!(path.exists(), "Exported file should exist"); - let content = std::fs::read_to_string(&path).unwrap(); - toml::from_str(&content).unwrap() - } - - #[test] - #[serial] - fn test_export_with_full_state() { - let (config_dir, work_dir, old_dir) = setup_export_env(); - - let state = make_multi_repo_state( - "/tmp/test-repo", - Some("https://github.com/user/repo.git".to_string()), - ); - write_multi_repo_state(config_dir.path(), &state); - - let filter = FilterConfig { - exclude_attachments: true, - exclude_older_than_days: Some(30), - enable_lfs: true, - scm_backend: "git".to_string(), - sync_subdirectory: "my-projects".to_string(), - use_project_name_only: true, - ..Default::default() - }; - write_filter_config(config_dir.path(), &filter); - - let result = handle_config_export(); - teardown(&old_dir); - - assert!(result.is_ok(), "export should succeed: {:?}", result.err()); - - let exported = read_exported_config(work_dir.path()); - assert_eq!(exported.repo_path, "/tmp/test-repo"); - assert_eq!( - exported.remote_url.as_deref(), - Some("https://github.com/user/repo.git") - ); - assert!( - exported.clone, - "clone should be true when remote_url is set" - ); - assert!(exported.exclude_attachments); - assert_eq!(exported.exclude_older_than_days, Some(30)); - assert!(exported.enable_lfs); - assert_eq!(exported.scm_backend, "git"); - assert_eq!(exported.sync_subdirectory, "my-projects"); - assert!(exported.use_project_name_only); - } - - #[test] - #[serial] - fn test_export_without_remote_url() { - let (config_dir, work_dir, old_dir) = setup_export_env(); - - let state = make_multi_repo_state("/tmp/local-repo", None); - write_multi_repo_state(config_dir.path(), &state); - write_filter_config(config_dir.path(), &FilterConfig::default()); - - let result = handle_config_export(); - teardown(&old_dir); - - assert!(result.is_ok()); - - let exported = read_exported_config(work_dir.path()); - assert_eq!(exported.repo_path, "/tmp/local-repo"); - assert!(exported.remote_url.is_none()); - assert!(!exported.clone, "clone should be false without remote_url"); - } - - #[test] - #[serial] - fn test_export_falls_back_when_no_state() { - // No state.json written — SyncState::load() and MultiRepoState::load() will fail - let (config_dir, work_dir, old_dir) = setup_export_env(); - write_filter_config(config_dir.path(), &FilterConfig::default()); - - let result = handle_config_export(); - teardown(&old_dir); - - assert!(result.is_ok(), "export should still succeed with fallback"); - - let exported = read_exported_config(work_dir.path()); - assert_eq!(exported.repo_path, "~/claude-code-sync-repo"); - assert!(exported.remote_url.is_none()); - assert!(!exported.clone); - } - - #[test] - #[serial] - fn test_export_defaults_from_empty_filter() { - let (config_dir, work_dir, old_dir) = setup_export_env(); - - let state = make_multi_repo_state("/tmp/test-repo", None); - write_multi_repo_state(config_dir.path(), &state); - // No filter config written — FilterConfig::load() returns default - - let result = handle_config_export(); - teardown(&old_dir); - - assert!(result.is_ok()); - - let exported = read_exported_config(work_dir.path()); - assert!(!exported.exclude_attachments); - assert!(exported.exclude_older_than_days.is_none()); - assert!(!exported.enable_lfs); - assert_eq!(exported.scm_backend, "git"); - assert_eq!(exported.sync_subdirectory, "projects"); - assert!(!exported.use_project_name_only); - } - - #[test] - #[serial] - fn test_export_roundtrip_with_init_config() { - // Verify the exported TOML can be parsed back identically - let (config_dir, work_dir, old_dir) = setup_export_env(); - - let state = make_multi_repo_state( - "/home/user/sync-repo", - Some("git@github.com:user/history.git".to_string()), - ); - write_multi_repo_state(config_dir.path(), &state); - - let filter = FilterConfig { - exclude_attachments: true, - exclude_older_than_days: Some(90), - enable_lfs: false, - scm_backend: "mercurial".to_string(), - sync_subdirectory: "conversations".to_string(), - use_project_name_only: true, - ..Default::default() - }; - write_filter_config(config_dir.path(), &filter); - - let result = handle_config_export(); - teardown(&old_dir); - - assert!(result.is_ok()); - - // Re-read and parse — should survive serialization roundtrip - let path = work_dir.path().join("claude-code-sync-init.toml"); - let raw = std::fs::read_to_string(&path).unwrap(); - let parsed: InitConfig = toml::from_str(&raw).unwrap(); - - assert_eq!(parsed.repo_path, "/home/user/sync-repo"); - assert_eq!( - parsed.remote_url.as_deref(), - Some("git@github.com:user/history.git") - ); - assert!(parsed.clone); - assert!(parsed.exclude_attachments); - assert_eq!(parsed.exclude_older_than_days, Some(90)); - assert!(!parsed.enable_lfs); - assert_eq!(parsed.scm_backend, "mercurial"); - assert_eq!(parsed.sync_subdirectory, "conversations"); - assert!(parsed.use_project_name_only); - } - - #[test] - #[serial] - fn test_export_output_file_is_valid_toml() { - let (config_dir, work_dir, old_dir) = setup_export_env(); - - let state = make_multi_repo_state("/tmp/repo", None); - write_multi_repo_state(config_dir.path(), &state); - write_filter_config(config_dir.path(), &FilterConfig::default()); - - handle_config_export().unwrap(); - teardown(&old_dir); - - let path = work_dir.path().join("claude-code-sync-init.toml"); - let raw = std::fs::read_to_string(&path).unwrap(); - - // Should parse as a generic TOML table - let table: toml::Table = toml::from_str(&raw).unwrap(); - assert!(table.contains_key("repo_path")); - assert!(table.contains_key("scm_backend")); - assert!(table.contains_key("sync_subdirectory")); - } -} diff --git a/src/handlers/config/export.rs b/src/handlers/config/export.rs new file mode 100644 index 00000000..f88fb203 --- /dev/null +++ b/src/handlers/config/export.rs @@ -0,0 +1,335 @@ +//! `claude-code-sync config --export`: write the active configuration back out +//! as a `claude-code-sync-init.toml`. + +use anyhow::{Context, Result}; +use colored::Colorize; +use std::path::PathBuf; + +use crate::filter::FilterConfig; +use crate::onboarding::InitConfig; +use crate::sync::{MultiRepoState, SyncState}; + +/// Export current config as a claude-code-sync-init.toml in the current directory. +/// +/// Reads the active sync state and filter config, builds an InitConfig, and writes +/// it to `./claude-code-sync-init.toml`. +pub fn handle_config_export() -> Result<()> { + let filter = FilterConfig::load().context("Failed to load filter configuration")?; + + let repo_path = match SyncState::load() { + Ok(state) => state.sync_repo_path.to_string_lossy().to_string(), + Err(e) => { + eprintln!( + "{} Could not load sync state ({}), using default repo_path", + "!".yellow(), + e + ); + "~/claude-code-sync-repo".to_string() + } + }; + + let remote_url = match MultiRepoState::load() { + Ok(ms) => match ms.repos.get(&ms.active_repo) { + Some(repo) => repo.remote_url.clone(), + None => { + eprintln!( + "{} Active repo '{}' not found in multi-repo state, remote_url will be unset", + "!".yellow(), + ms.active_repo + ); + None + } + }, + Err(e) => { + eprintln!( + "{} Could not load multi-repo state ({}), remote_url will be unset", + "!".yellow(), + e + ); + None + } + }; + + // InitConfig has no home for these, so say so rather than dropping them silently. + if !filter.include_patterns.is_empty() { + eprintln!( + "{} include_patterns are not included in the export and will need to be reconfigured", + "!".yellow() + ); + } + if !filter.exclude_patterns.is_empty() { + eprintln!( + "{} exclude_patterns are not included in the export and will need to be reconfigured", + "!".yellow() + ); + } + + let init_config = InitConfig { + repo_path, + remote_url: remote_url.clone(), + clone: remote_url.is_some(), + exclude_attachments: filter.exclude_attachments, + exclude_older_than_days: filter.exclude_older_than_days, + enable_lfs: filter.enable_lfs, + scm_backend: filter.scm_backend, + sync_subdirectory: filter.sync_subdirectory, + use_project_name_only: filter.use_project_name_only, + sync_artifacts: filter.sync_artifacts.clone(), + }; + + let content = + toml::to_string_pretty(&init_config).context("Failed to serialize init config")?; + + let output_path = PathBuf::from("claude-code-sync-init.toml"); + std::fs::write(&output_path, content) + .with_context(|| format!("Failed to write {}", output_path.display()))?; + + println!( + "{} Exported to {}", + "✓".green().bold(), + output_path.display() + ); + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::sync::RepoConfig; + use serial_test::serial; + use std::collections::HashMap; + use tempfile::TempDir; + + /// Isolates a test from the developer's real config and working directory. + /// + /// `handle_config_export` writes to a *relative* path, so exercising it means + /// mutating two pieces of process-global state: the config-dir override and + /// the current directory. Restoring them in `Drop` rather than at the end of + /// the test body means an assertion failure can't leave the whole harness + /// pointed at a deleted temp dir. + /// + /// `CLAUDE_CODE_SYNC_CONFIG_DIR` is used rather than `XDG_CONFIG_HOME` + /// because macOS ignores the latter. + struct ExportEnv { + config_dir: TempDir, + work_dir: TempDir, + original_dir: PathBuf, + } + + impl ExportEnv { + fn new() -> Self { + let config_dir = TempDir::new().unwrap(); + let work_dir = TempDir::new().unwrap(); + let original_dir = std::env::current_dir().unwrap(); + + std::env::set_var("CLAUDE_CODE_SYNC_CONFIG_DIR", config_dir.path()); + std::env::set_current_dir(work_dir.path()).unwrap(); + + Self { + config_dir, + work_dir, + original_dir, + } + } + + /// Write a v2 MultiRepoState to the config dir's state.json + fn with_multi_repo_state(self, repo_path: &str, remote_url: Option<&str>) -> Self { + let remote_url = remote_url.map(str::to_string); + let mut repos = HashMap::new(); + repos.insert( + "default".to_string(), + RepoConfig { + name: "default".to_string(), + sync_repo_path: PathBuf::from(repo_path), + has_remote: remote_url.is_some(), + is_cloned_repo: false, + remote_url, + description: None, + }, + ); + let state = MultiRepoState { + version: 2, + active_repo: "default".to_string(), + repos, + }; + + let dir = self.claude_dir(); + std::fs::write( + dir.join("state.json"), + serde_json::to_string_pretty(&state).unwrap(), + ) + .unwrap(); + self + } + + /// Write a FilterConfig to the config dir's config.toml + fn with_filter_config(self, filter: &FilterConfig) -> Self { + let dir = self.claude_dir(); + std::fs::write( + dir.join("config.toml"), + toml::to_string_pretty(filter).unwrap(), + ) + .unwrap(); + self + } + + fn claude_dir(&self) -> PathBuf { + let dir = self.config_dir.path().join("claude-code-sync"); + std::fs::create_dir_all(&dir).unwrap(); + dir + } + + /// Read back the exported TOML from the working directory. + fn exported(&self) -> InitConfig { + toml::from_str(&self.exported_raw()).unwrap() + } + + fn exported_raw(&self) -> String { + let path = self.work_dir.path().join("claude-code-sync-init.toml"); + assert!(path.exists(), "Exported file should exist"); + std::fs::read_to_string(&path).unwrap() + } + } + + impl Drop for ExportEnv { + fn drop(&mut self) { + std::env::remove_var("CLAUDE_CODE_SYNC_CONFIG_DIR"); + // Leave the cwd somewhere valid: `work_dir` is about to be deleted. + let _ = std::env::set_current_dir(&self.original_dir); + } + } + + fn filter_with(scm_backend: &str, subdirectory: &str, days: Option) -> FilterConfig { + FilterConfig { + exclude_attachments: true, + exclude_older_than_days: days, + scm_backend: scm_backend.to_string(), + sync_subdirectory: subdirectory.to_string(), + use_project_name_only: true, + ..Default::default() + } + } + + #[test] + #[serial] + fn test_export_with_full_state() { + let env = ExportEnv::new() + .with_multi_repo_state("/tmp/test-repo", Some("https://github.com/user/repo.git")) + .with_filter_config(&FilterConfig { + enable_lfs: true, + ..filter_with("git", "my-projects", Some(30)) + }); + + handle_config_export().expect("export should succeed"); + + let exported = env.exported(); + assert_eq!(exported.repo_path, "/tmp/test-repo"); + assert_eq!( + exported.remote_url.as_deref(), + Some("https://github.com/user/repo.git") + ); + assert!( + exported.clone, + "clone should be true when remote_url is set" + ); + assert!(exported.exclude_attachments); + assert_eq!(exported.exclude_older_than_days, Some(30)); + assert!(exported.enable_lfs); + assert_eq!(exported.scm_backend, "git"); + assert_eq!(exported.sync_subdirectory, "my-projects"); + assert!(exported.use_project_name_only); + } + + #[test] + #[serial] + fn test_export_without_remote_url() { + let env = ExportEnv::new() + .with_multi_repo_state("/tmp/local-repo", None) + .with_filter_config(&FilterConfig::default()); + + handle_config_export().unwrap(); + + let exported = env.exported(); + assert_eq!(exported.repo_path, "/tmp/local-repo"); + assert!(exported.remote_url.is_none()); + assert!(!exported.clone, "clone should be false without remote_url"); + } + + #[test] + #[serial] + fn test_export_falls_back_when_no_state() { + // No state.json: SyncState::load() and MultiRepoState::load() both fail, + // and the export must still produce a usable file. + let env = ExportEnv::new().with_filter_config(&FilterConfig::default()); + + handle_config_export().expect("export should still succeed with fallback"); + + let exported = env.exported(); + assert_eq!(exported.repo_path, "~/claude-code-sync-repo"); + assert!(exported.remote_url.is_none()); + assert!(!exported.clone); + } + + #[test] + #[serial] + fn test_export_defaults_from_empty_filter() { + // No config.toml: FilterConfig::load() returns defaults. + let env = ExportEnv::new().with_multi_repo_state("/tmp/test-repo", None); + + handle_config_export().unwrap(); + + let exported = env.exported(); + assert!(!exported.exclude_attachments); + assert!(exported.exclude_older_than_days.is_none()); + assert!(!exported.enable_lfs); + assert_eq!(exported.scm_backend, "git"); + assert_eq!(exported.sync_subdirectory, "projects"); + assert!(!exported.use_project_name_only); + } + + #[test] + #[serial] + fn test_export_roundtrip_with_init_config() { + let env = ExportEnv::new() + .with_multi_repo_state( + "/home/user/sync-repo", + Some("git@github.com:user/history.git"), + ) + .with_filter_config(&filter_with("mercurial", "conversations", Some(90))); + + handle_config_export().unwrap(); + + // Parse straight from the raw text: the point is that what we wrote + // survives a full serialize/deserialize round trip. + let parsed: InitConfig = toml::from_str(&env.exported_raw()).unwrap(); + + assert_eq!(parsed.repo_path, "/home/user/sync-repo"); + assert_eq!( + parsed.remote_url.as_deref(), + Some("git@github.com:user/history.git") + ); + assert!(parsed.clone); + assert!(parsed.exclude_attachments); + assert_eq!(parsed.exclude_older_than_days, Some(90)); + assert!(!parsed.enable_lfs); + assert_eq!(parsed.scm_backend, "mercurial"); + assert_eq!(parsed.sync_subdirectory, "conversations"); + assert!(parsed.use_project_name_only); + } + + #[test] + #[serial] + fn test_export_output_file_is_valid_toml() { + let env = ExportEnv::new() + .with_multi_repo_state("/tmp/repo", None) + .with_filter_config(&FilterConfig::default()); + + handle_config_export().unwrap(); + + let table: toml::Table = toml::from_str(&env.exported_raw()).unwrap(); + assert!(table.contains_key("repo_path")); + assert!(table.contains_key("scm_backend")); + assert!(table.contains_key("sync_subdirectory")); + } +} diff --git a/src/handlers/config/fields.rs b/src/handlers/config/fields.rs new file mode 100644 index 00000000..6c83d888 --- /dev/null +++ b/src/handlers/config/fields.rs @@ -0,0 +1,146 @@ +//! Pure value logic for the config commands. +//! +//! Nothing in here prompts, prints, or touches the filesystem — which is the +//! point. These conversions used to be inlined into the `inquire` call sites in +//! `interactive.rs` and `wizard.rs`, once per mode, which meant they could only +//! be exercised with a TTY attached and so were never tested at all. + +use anyhow::{bail, Context, Result}; + +const BYTES_PER_MB: f64 = 1024.0 * 1024.0; + +/// Split a comma-separated pattern list into its trimmed, non-empty parts. +/// +/// This is deliberately only the split/trim/filter core. What an *empty* input +/// means is left to the caller, because the two modes report it differently: +/// interactive says "Cleared include patterns", the wizard says "Include +/// patterns set: []". +pub(super) fn parse_patterns(input: &str) -> Vec { + input + .split(',') + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) + .collect() +} + +/// Parse a file-size limit expressed in megabytes. +/// +/// Rejects non-positive and non-finite values. Rust's float-to-int cast +/// saturates rather than wrapping or erroring, so without this check `-5` would +/// quietly become a 0-byte limit — which makes `FilterConfig::should_include` +/// reject every file — and `1e30` would become `u64::MAX`. +pub(super) fn parse_file_size_mb(input: &str) -> Result { + let mb: f64 = input + .trim() + .parse() + .context("Invalid number. Must be a positive number.")?; + + if !mb.is_finite() { + bail!("Max file size must be a finite number of megabytes"); + } + if mb <= 0.0 { + bail!("Max file size must be greater than 0 MB (got {mb})"); + } + + Ok((mb * BYTES_PER_MB) as u64) +} + +/// Render a byte count as megabytes with one decimal place. +pub(super) fn format_size_mb(bytes: u64) -> String { + format!("{:.1}", bytes as f64 / BYTES_PER_MB) +} + +/// The "current value" text for the optional age filter. +pub(super) fn format_age_days(days: Option) -> String { + days.map_or_else(|| "Not set".to_string(), |d| d.to_string()) +} + +/// The "current value" text for a pattern list, with a caller-chosen label for +/// the empty case (the two modes word it differently). +pub(super) fn format_patterns(patterns: &[String], when_empty: &str) -> String { + if patterns.is_empty() { + when_empty.to_string() + } else { + patterns.join(", ") + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_parse_patterns_splits_and_trims() { + assert_eq!( + parse_patterns("*work*, /home/user/x , *test*"), + vec!["*work*", "/home/user/x", "*test*"] + ); + } + + #[test] + fn test_parse_patterns_drops_empty_segments() { + // Trailing commas and stray whitespace are what people actually type. + assert_eq!(parse_patterns("a,,b,"), vec!["a", "b"]); + assert_eq!(parse_patterns(" , , "), Vec::::new()); + } + + #[test] + fn test_parse_patterns_empty_input_yields_empty_list() { + assert_eq!(parse_patterns(""), Vec::::new()); + } + + #[test] + fn test_parse_file_size_accepts_positive_values() { + assert_eq!(parse_file_size_mb("10").unwrap(), 10 * 1024 * 1024); + assert_eq!(parse_file_size_mb("0.5").unwrap(), 512 * 1024); + assert_eq!(parse_file_size_mb(" 2 ").unwrap(), 2 * 1024 * 1024); + } + + #[test] + fn test_parse_file_size_rejects_negative() { + // The bug this function exists to prevent: `as u64` saturates, so a + // negative input used to land as a 0-byte limit, silently filtering out + // every single file instead of erroring. + let err = parse_file_size_mb("-5").unwrap_err().to_string(); + assert!(err.contains("greater than 0"), "unexpected error: {err}"); + } + + #[test] + fn test_parse_file_size_rejects_zero() { + assert!(parse_file_size_mb("0").is_err()); + } + + #[test] + fn test_parse_file_size_rejects_non_finite() { + // `"inf".parse::()` succeeds, and `inf as u64` saturates to u64::MAX. + assert!(parse_file_size_mb("inf").is_err()); + assert!(parse_file_size_mb("NaN").is_err()); + } + + #[test] + fn test_parse_file_size_rejects_garbage() { + assert!(parse_file_size_mb("ten").is_err()); + assert!(parse_file_size_mb("").is_err()); + } + + #[test] + fn test_format_size_mb_round_trips_with_parse() { + let bytes = parse_file_size_mb("12.5").unwrap(); + assert_eq!(format_size_mb(bytes), "12.5"); + } + + #[test] + fn test_format_age_days() { + assert_eq!(format_age_days(Some(30)), "30"); + assert_eq!(format_age_days(None), "Not set"); + } + + #[test] + fn test_format_patterns_uses_caller_label_when_empty() { + assert_eq!(format_patterns(&[], "None"), "None"); + assert_eq!( + format_patterns(&["a".to_string(), "b".to_string()], "None"), + "a, b" + ); + } +} diff --git a/src/handlers/config/interactive.rs b/src/handlers/config/interactive.rs new file mode 100644 index 00000000..aa977fdf --- /dev/null +++ b/src/handlers/config/interactive.rs @@ -0,0 +1,186 @@ +//! Interactive configuration menu: pick the settings to change, then edit only +//! those. +//! +//! Note the empty-input handling: here, submitting an empty value *clears* the +//! setting and says so. The wizard deliberately behaves differently — see +//! `wizard.rs`. + +use anyhow::{Context, Result}; +use colored::Colorize; +use inquire::{Confirm, MultiSelect, Text}; + +use super::fields::{format_patterns, format_size_mb, parse_file_size_mb, parse_patterns}; +use super::prompts::{current_age, display_config_summary, prompt_artifact_toggle_selection}; +use crate::filter::FilterConfig; + +/// Handle interactive configuration menu +/// +/// Shows all configuration options and allows user to select which ones to modify +pub fn handle_config_interactive() -> Result<()> { + println!("{}", "Interactive Configuration".cyan().bold()); + println!("{}", "=".repeat(80).cyan()); + println!(); + + let current_config = FilterConfig::load().context("Failed to load current configuration")?; + + println!("{}", "Current Settings:".bold()); + display_config_summary(¤t_config); + println!(); + + let options = vec![ + "Exclude older than (days)", + "Include patterns", + "Exclude patterns", + "Exclude attachments", + "Max file size", + "Artifact sync categories", + ]; + + let selections = MultiSelect::new( + "Select settings to modify (Space to select, Enter to confirm):", + options, + ) + .with_help_message("Use arrow keys to navigate, Space to select/deselect, Enter when done") + .prompt() + .context("Failed to get user selections")?; + + if selections.is_empty() { + println!( + "{}", + "No settings selected. Configuration unchanged.".yellow() + ); + return Ok(()); + } + + println!(); + println!("{}", "Modifying selected settings:".cyan().bold()); + println!(); + + let mut modified_config = current_config.clone(); + + for selection in selections { + match selection { + "Exclude older than (days)" => { + let input = Text::new("Exclude older than (days):") + .with_help_message(&format!( + "Current: {}. Enter a number or leave empty to unset", + current_age(&modified_config) + )) + .prompt()?; + + if input.trim().is_empty() { + modified_config.exclude_older_than_days = None; + println!(" {} Unset exclude_older_than_days", "✓".green()); + } else { + let days: u32 = input + .trim() + .parse() + .context("Invalid number. Must be a positive integer.")?; + modified_config.exclude_older_than_days = Some(days); + println!( + " {} Set exclude_older_than_days to {} days", + "✓".green(), + days + ); + } + } + + "Include patterns" => { + let input = Text::new("Include patterns (comma-separated):") + .with_help_message(&format!( + "Current: {}. Glob patterns like '*work*' or '/path/to/project'", + format_patterns(&modified_config.include_patterns, "None") + )) + .prompt()?; + + if input.trim().is_empty() { + modified_config.include_patterns = Vec::new(); + println!(" {} Cleared include patterns", "✓".green()); + } else { + modified_config.include_patterns = parse_patterns(&input); + println!( + " {} Set include patterns: {:?}", + "✓".green(), + modified_config.include_patterns + ); + } + } + + "Exclude patterns" => { + let input = Text::new("Exclude patterns (comma-separated):") + .with_help_message(&format!( + "Current: {}. Glob patterns like '*test*' or '/tmp/*'", + format_patterns(&modified_config.exclude_patterns, "None") + )) + .prompt()?; + + if input.trim().is_empty() { + modified_config.exclude_patterns = Vec::new(); + println!(" {} Cleared exclude patterns", "✓".green()); + } else { + modified_config.exclude_patterns = parse_patterns(&input); + println!( + " {} Set exclude patterns: {:?}", + "✓".green(), + modified_config.exclude_patterns + ); + } + } + + "Exclude attachments" => { + let current = modified_config.exclude_attachments; + + let exclude = Confirm::new("Exclude attachments (images, PDFs, etc.)?") + .with_default(current) + .with_help_message(&format!( + "Current: {current}. If yes, only .jsonl files will be synced" + )) + .prompt()?; + + modified_config.exclude_attachments = exclude; + println!(" {} Set exclude_attachments to {}", "✓".green(), exclude); + } + + "Artifact sync categories" => { + modified_config.sync_artifacts = + prompt_artifact_toggle_selection(&modified_config.sync_artifacts)?; + } + + "Max file size" => { + let input = Text::new("Max file size (MB):") + .with_default(&format_size_mb(modified_config.max_file_size_bytes)) + .with_help_message("Maximum size for individual files (e.g., 10 for 10MB)") + .prompt()?; + + modified_config.max_file_size_bytes = parse_file_size_mb(&input)?; + println!( + " {} Set max_file_size to {} MB", + "✓".green(), + format_size_mb(modified_config.max_file_size_bytes) + ); + } + + _ => {} + } + println!(); + } + + println!("{}", "New Configuration:".cyan().bold()); + display_config_summary(&modified_config); + println!(); + + let confirm = Confirm::new("Save this configuration?") + .with_default(true) + .prompt()?; + + if confirm { + modified_config + .save() + .context("Failed to save configuration")?; + println!("\n{} Configuration saved successfully!", "✓".green().bold()); + } else { + println!("\n{}", "Configuration not saved.".yellow()); + } + + Ok(()) +} diff --git a/src/handlers/config/mod.rs b/src/handlers/config/mod.rs new file mode 100644 index 00000000..78251b29 --- /dev/null +++ b/src/handlers/config/mod.rs @@ -0,0 +1,21 @@ +//! Configuration command handlers. +//! +//! Split by concern rather than by command: `fields` holds the pure value logic +//! (parsing, formatting) with no I/O, `prompts` holds the terminal interaction +//! the two editing modes share, and the remaining modules are one command each. +//! +//! `interactive` and `wizard` deliberately keep separate prompt flows — they ask +//! for the same six settings in genuinely different ways — but they no longer +//! keep separate copies of the logic that turns what the user typed into a value. + +mod export; +mod fields; +mod interactive; +mod prompts; +mod repo_select; +mod wizard; + +pub use export::handle_config_export; +pub use interactive::handle_config_interactive; +pub use repo_select::handle_repo_selector; +pub use wizard::handle_config_wizard; diff --git a/src/handlers/config/prompts.rs b/src/handlers/config/prompts.rs new file mode 100644 index 00000000..4c6f8e2b --- /dev/null +++ b/src/handlers/config/prompts.rs @@ -0,0 +1,111 @@ +//! Terminal interaction shared by the interactive and wizard config modes. +//! +//! Separate from `fields.rs` because everything here writes to stdout or opens +//! an `inquire` prompt. Keeping the two apart is what lets `fields.rs` be tested +//! without a TTY. + +use anyhow::{Context, Result}; +use colored::Colorize; +use inquire::MultiSelect; + +use super::fields::{format_age_days, format_patterns, format_size_mb}; +use crate::artifacts::registry::{find_by_name, toggleable, ArtifactToggles}; +use crate::filter::FilterConfig; + +/// Display a compact configuration summary. +pub(super) fn display_config_summary(config: &FilterConfig) { + println!( + " {} {}", + "Exclude older than:".cyan(), + config + .exclude_older_than_days + .map(|d| format!("{d} days")) + .unwrap_or_else(|| "Not set".dimmed().to_string()) + ); + + println!( + " {} {}", + "Include patterns:".cyan(), + format_patterns( + &config.include_patterns, + &"None (all included)".dimmed().to_string() + ) + ); + + println!( + " {} {}", + "Exclude patterns:".cyan(), + format_patterns(&config.exclude_patterns, &"None".dimmed().to_string()) + ); + + println!( + " {} {} MB", + "Max file size:".cyan(), + format_size_mb(config.max_file_size_bytes) + ); + + println!( + " {} {}", + "Exclude attachments:".cyan(), + if config.exclude_attachments { + "Yes (only .jsonl files)".green().to_string() + } else { + "No (all files)".yellow().to_string() + } + ); + + let enabled: Vec<&str> = toggleable() + .filter(|d| config.sync_artifacts.is_enabled(d.id)) + .map(|d| d.name) + .collect(); + println!( + " {} {}", + "Artifact sync:".cyan(), + if enabled.is_empty() { + "All disabled".dimmed().to_string() + } else { + enabled.join(", ") + } + ); +} + +/// The current age filter, rendered for a "Current: ..." line. +pub(super) fn current_age(config: &FilterConfig) -> String { + format_age_days(config.exclude_older_than_days) +} + +/// MultiSelect over all artifact categories, pre-selecting the currently +/// enabled ones. Returns the resulting toggles. +pub(super) fn prompt_artifact_toggle_selection( + current: &ArtifactToggles, +) -> Result { + let rows: Vec<_> = toggleable().collect(); + let options: Vec = rows + .iter() + .map(|d| format!("{} — {}", d.name, d.description)) + .collect(); + let preselected: Vec = rows + .iter() + .enumerate() + .filter(|(_, d)| current.is_enabled(d.id)) + .map(|(i, _)| i) + .collect(); + + let picked = MultiSelect::new("Artifact categories to sync:", options) + .with_default(&preselected) + .with_help_message( + "Space toggles, Enter confirms. Secrets (credentials, settings.local.json, \ + .env*, keys) are never synced regardless of selection.", + ) + .prompt() + .context("Failed to get artifact category selection")?; + + let mut toggles = ArtifactToggles::default(); + for label in picked { + let name = label.split(" — ").next().unwrap_or(&label); + if let Some(desc) = find_by_name(name) { + toggles.set_enabled(desc.id, true); + } + } + Ok(toggles) +} diff --git a/src/handlers/config/repo_select.rs b/src/handlers/config/repo_select.rs new file mode 100644 index 00000000..2405ad79 --- /dev/null +++ b/src/handlers/config/repo_select.rs @@ -0,0 +1,219 @@ +//! The repository selector shown by `claude-code-sync config` with no argument. + +use anyhow::{Context, Result}; +use colored::Colorize; +use inquire::Select; +use std::collections::HashMap; + +use super::interactive::handle_config_interactive; +use crate::config::ConfigManager; +use crate::scm; +use crate::sync::{MultiRepoState, RepoConfig}; + +/// Try to recover an existing repo if state.json is missing but repo exists +/// +/// This handles the case where a user has a valid repo in the default location +/// but the state.json file is missing (e.g., from an older version or deletion). +fn try_recover_existing_repo() -> Result> { + let default_repo = match ConfigManager::default_repo_dir() { + Ok(path) => path, + Err(_) => return Ok(None), + }; + + if !default_repo.exists() || !scm::is_repo(&default_repo) { + return Ok(None); + } + + let (has_remote, remote_url) = match scm::open(&default_repo) { + Ok(repo) => { + let has_remote = repo.has_remote("origin"); + let remote_url = if has_remote { + repo.get_remote_url("origin").ok() + } else { + None + }; + (has_remote, remote_url) + } + Err(_) => (false, None), + }; + + println!( + "{} Found existing repo at: {}", + "!".yellow(), + default_repo.display() + ); + if let Some(ref url) = remote_url { + println!(" Remote: {}", url.cyan()); + } + println!(" Recovering configuration..."); + println!(); + + let repo_config = RepoConfig { + name: "default".to_string(), + sync_repo_path: default_repo, + has_remote, + is_cloned_repo: false, // We can't know this for sure + remote_url, + description: Some("Recovered from existing repository".to_string()), + }; + + let mut repos = HashMap::new(); + repos.insert("default".to_string(), repo_config); + + let state = MultiRepoState { + version: 2, + active_repo: "default".to_string(), + repos, + }; + + state.save()?; + + Ok(Some(state)) +} + +/// Handle the repository selector menu +/// +/// Shows when `claude-code-sync config` is run with no arguments. +/// Displays all configured repositories and allows switching between them. +pub fn handle_repo_selector() -> Result<()> { + println!("{}", "Repository Configuration".cyan().bold()); + println!("{}", "=".repeat(60).cyan()); + println!(); + + // "Not initialized" is recoverable if a repo happens to sit in the default + // location; any other load failure is not ours to interpret. + let mut state = match MultiRepoState::load() { + Ok(s) => s, + Err(e) => { + let err_msg = e.to_string(); + if err_msg.contains("not initialized") + || err_msg.contains("Run 'claude-code-sync init'") + { + if let Some(recovered) = try_recover_existing_repo()? { + println!( + "{}", + "Found existing repository - recovered configuration!".green() + ); + println!(); + recovered + } else { + println!("{}", "No repositories configured.".yellow()); + println!(); + println!( + "Run '{}' to set up your first repository.", + "claude-code-sync init".cyan() + ); + return Ok(()); + } + } else { + return Err(e); + } + } + }; + + if state.repos.is_empty() { + println!("{}", "No repositories configured.".yellow()); + println!(); + println!( + "Run '{}' to set up your first repository.", + "claude-code-sync init".cyan() + ); + return Ok(()); + } + + // Active repo first, then alphabetical. + let mut repo_entries: Vec<_> = state.repos.values().collect(); + repo_entries.sort_by(|a, b| { + if a.name == state.active_repo { + std::cmp::Ordering::Less + } else if b.name == state.active_repo { + std::cmp::Ordering::Greater + } else { + a.name.cmp(&b.name) + } + }); + + let mut options: Vec = repo_entries + .iter() + .map(|repo| { + let active_marker = if repo.name == state.active_repo { + format!(" {}", "[ACTIVE]".green().bold()) + } else { + String::new() + }; + + let path_str = repo.sync_repo_path.display().to_string(); + let remote_info = repo + .remote_url + .as_ref() + .map(|u| format!(" ({})", u.dimmed())) + .unwrap_or_default(); + + format!( + "{}{} - {}{}", + repo.name, active_marker, path_str, remote_info + ) + }) + .collect(); + + options.push(format!("{}", "─── Actions ───".dimmed())); + options.push("Configure filters (current repo)".to_string()); + options.push("Exit".to_string()); + + let selection = Select::new("Select a repository to make active:", options.clone()) + .with_help_message("Use arrow keys to navigate, Enter to select") + .prompt() + .context("Failed to get user selection")?; + + if selection.contains("─── Actions ───") || selection == "Exit" { + return Ok(()); + } + + if selection == "Configure filters (current repo)" { + return handle_config_interactive(); + } + + // The repo name is the first token, before any [ACTIVE] marker. + let repo_name = selection + .split_whitespace() + .next() + .ok_or_else(|| anyhow::anyhow!("Invalid selection"))?; + + if repo_name == state.active_repo { + println!(); + println!( + "{} '{}' is already the active repository.", + "ℹ".blue(), + repo_name.cyan() + ); + return Ok(()); + } + + if state.repos.contains_key(repo_name) { + state.active_repo = repo_name.to_string(); + state.save()?; + + println!(); + println!( + "{} Switched to repository '{}'", + "✓".green().bold(), + repo_name.cyan() + ); + + if let Some(repo) = state.repos.get(repo_name) { + println!(" Path: {}", repo.sync_repo_path.display()); + if let Some(ref url) = repo.remote_url { + println!(" Remote: {url}"); + } + if repo.has_remote { + println!(" Has remote: {}", "Yes".green()); + } else { + println!(" Has remote: {}", "No (local only)".yellow()); + } + } + } else { + return Err(anyhow::anyhow!("Repository '{}' not found", repo_name)); + } + + Ok(()) +} diff --git a/src/handlers/config/wizard.rs b/src/handlers/config/wizard.rs new file mode 100644 index 00000000..f06291fe --- /dev/null +++ b/src/handlers/config/wizard.rs @@ -0,0 +1,219 @@ +//! Wizard-mode configuration: walk every setting in order, gating each one +//! behind a yes/no question. +//! +//! This is intentionally not the same UX as `interactive.rs`. The wizard asks +//! "Do you want to ...?" first and only prompts for a value on yes, and it has +//! no "empty clears the setting" affordance — an empty pattern list here simply +//! parses to `[]` and is reported as such. Both flows share the value logic in +//! `fields.rs`, not the prompt shape. + +use anyhow::{Context, Result}; +use colored::Colorize; +use inquire::{Confirm, Text}; + +use super::fields::{format_patterns, format_size_mb, parse_file_size_mb, parse_patterns}; +use super::prompts::{current_age, display_config_summary, prompt_artifact_toggle_selection}; +use crate::filter::FilterConfig; + +/// Handle wizard-mode configuration +/// +/// Steps through each configuration option one by one +pub fn handle_config_wizard() -> Result<()> { + println!("{}", "Configuration Wizard".cyan().bold()); + println!("{}", "=".repeat(80).cyan()); + println!(); + println!( + "{}", + "This wizard will walk you through all configuration options.".dimmed() + ); + println!( + "{}", + "Press Enter to keep current value or enter a new value.".dimmed() + ); + println!(); + + let current_config = FilterConfig::load().context("Failed to load current configuration")?; + let mut modified_config = current_config.clone(); + + // 1. Exclude older than + println!("{}", "1. Age Filter".bold().cyan()); + println!(" Current: {}", current_age(&modified_config).yellow()); + + let exclude_old = + Confirm::new("Do you want to exclude projects older than a certain number of days?") + .with_default(modified_config.exclude_older_than_days.is_some()) + .prompt()?; + + if exclude_old { + let default_days = modified_config + .exclude_older_than_days + .unwrap_or(30) + .to_string(); + let input = Text::new("How many days?") + .with_default(&default_days) + .prompt()?; + + let days: u32 = input + .trim() + .parse() + .context("Invalid number. Must be a positive integer.")?; + modified_config.exclude_older_than_days = Some(days); + println!( + " {} Will exclude projects older than {} days\n", + "✓".green(), + days + ); + } else { + modified_config.exclude_older_than_days = None; + println!(" {} Age filter disabled\n", "✓".green()); + } + + // 2. Include patterns + println!("{}", "2. Include Patterns".bold().cyan()); + println!( + " Current: {}", + format_patterns( + &modified_config.include_patterns, + "None (all projects included)" + ) + .yellow() + ); + + let use_include = Confirm::new("Do you want to limit sync to specific project patterns?") + .with_default(!modified_config.include_patterns.is_empty()) + .with_help_message("Example: *work*, /home/user/important/*") + .prompt()?; + + if use_include { + let default = modified_config.include_patterns.join(", "); + let input = Text::new("Enter include patterns (comma-separated):") + .with_default(&default) + .with_help_message("Glob patterns like '*work*' or '/specific/path'") + .prompt()?; + + modified_config.include_patterns = parse_patterns(&input); + println!( + " {} Include patterns set: {:?}\n", + "✓".green(), + modified_config.include_patterns + ); + } else { + modified_config.include_patterns = Vec::new(); + println!(" {} All projects will be included\n", "✓".green()); + } + + // 3. Exclude patterns + println!("{}", "3. Exclude Patterns".bold().cyan()); + println!( + " Current: {}", + format_patterns(&modified_config.exclude_patterns, "None").yellow() + ); + + let use_exclude = Confirm::new("Do you want to exclude specific project patterns?") + .with_default(!modified_config.exclude_patterns.is_empty()) + .with_help_message("Example: *test*, *tmp*, /temp/*") + .prompt()?; + + if use_exclude { + let default = modified_config.exclude_patterns.join(", "); + let input = Text::new("Enter exclude patterns (comma-separated):") + .with_default(&default) + .with_help_message("Glob patterns like '*test*' or '/tmp/*'") + .prompt()?; + + modified_config.exclude_patterns = parse_patterns(&input); + println!( + " {} Exclude patterns set: {:?}\n", + "✓".green(), + modified_config.exclude_patterns + ); + } else { + modified_config.exclude_patterns = Vec::new(); + println!(" {} No exclusion patterns\n", "✓".green()); + } + + // 4. Exclude attachments + println!("{}", "4. File Type Filter".bold().cyan()); + println!( + " Current: {}", + if modified_config.exclude_attachments { + "Exclude attachments".yellow() + } else { + "Include all files".yellow() + } + ); + + let exclude_attachments = Confirm::new("Exclude attachments (images, PDFs, etc.)?") + .with_default(modified_config.exclude_attachments) + .with_help_message("If yes, only .jsonl conversation files will be synced") + .prompt()?; + + modified_config.exclude_attachments = exclude_attachments; + println!( + " {} Attachments will be {}\n", + "✓".green(), + if exclude_attachments { + "excluded" + } else { + "included" + } + ); + + // 5. Max file size + println!("{}", "5. File Size Limit".bold().cyan()); + println!( + " Current: {} MB", + format_size_mb(modified_config.max_file_size_bytes) + ); + + let change_size = Confirm::new("Do you want to change the maximum file size limit?") + .with_default(false) + .prompt()?; + + if change_size { + let input = Text::new("Max file size (MB):") + .with_default(&format_size_mb(modified_config.max_file_size_bytes)) + .prompt()?; + + modified_config.max_file_size_bytes = parse_file_size_mb(&input)?; + println!( + " {} Max file size set to {} MB\n", + "✓".green(), + format_size_mb(modified_config.max_file_size_bytes) + ); + } else { + println!(" {} Keeping current max file size\n", "✓".green()); + } + + // Artifact sync categories + let change_artifacts = + Confirm::new("Configure artifact sync categories (settings, skills, agents, ...)?") + .with_default(false) + .prompt()?; + if change_artifacts { + modified_config.sync_artifacts = + prompt_artifact_toggle_selection(&modified_config.sync_artifacts)?; + } + + // Summary and confirmation + println!("{}", "=".repeat(80).cyan()); + println!("{}", "Configuration Summary:".bold().cyan()); + println!("{}", "=".repeat(80).cyan()); + display_config_summary(&modified_config); + println!(); + + let confirm = Confirm::new("Save this configuration?") + .with_default(true) + .prompt()?; + + if confirm { + modified_config + .save() + .context("Failed to save configuration")?; + println!("\n{} Configuration saved successfully!", "✓".green().bold()); + } else { + println!("\n{}", "Configuration not saved.".yellow()); + } + + Ok(()) +} From c6dae11c445cc01c32c8952e126d3f0d2727be38 Mon Sep 17 00:00:00 2001 From: perfectra1n Date: Mon, 13 Jul 2026 13:11:48 -0700 Subject: [PATCH 4/7] test: split test_onboarding.rs by subject and make env cleanup panic-safe tests/test_onboarding.rs was 1088 lines covering five unrelated subjects. Split it by what is actually under test: test_onboarding.rs 12 tests init_from_onboarding, init_sync_repo, cloning, InitConfig validation test_config_state.rs 9 tests ConfigManager paths, FilterConfig, SyncState test_multi_repo.rs 13 tests MultiRepoState v1->v2, active-repo switching Add tests/common/mod.rs with a ConfigEnv guard that unsets CLAUDE_CODE_SYNC_CONFIG_DIR on Drop, replacing 21 hand-written set_var/ remove_var pairs. The cleanup used to be a bare statement at the end of the function body -- and these tests return Result<()> and use `?` throughout, so *any* early return, not only a panic, skipped it and left the variable pointing at a TempDir that was about to be deleted. Every later test in the same binary then resolved its config against a path that no longer existed. Drop runs on the early-return and unwind paths both. Delete setup_test_config_env(). Despite the name it set up no environment -- it was TempDir::new() and nothing else, and all 28 callers went on to set the variable by hand anyway. Four tests ran with no override and no #[serial] at all, and two of them (ensure_config_dir, config_directory_structure) mkdir'd in the developer's real ~/.config -- a comment in the old file admitted as much. They are now guarded like the rest. Their assertions (contains("claude-code-sync"), ends_with(...)) hold identically under the override. 34 tests in, 34 tests out, same names. --- tests/common/mod.rs | 76 +++ tests/test_config_state.rs | 185 +++++++ tests/test_multi_repo.rs | 404 +++++++++++++++ tests/test_onboarding.rs | 1000 ++++-------------------------------- 4 files changed, 762 insertions(+), 903 deletions(-) create mode 100644 tests/common/mod.rs create mode 100644 tests/test_config_state.rs create mode 100644 tests/test_multi_repo.rs diff --git a/tests/common/mod.rs b/tests/common/mod.rs new file mode 100644 index 00000000..86ce7b5f --- /dev/null +++ b/tests/common/mod.rs @@ -0,0 +1,76 @@ +//! Shared fixtures for the integration test binaries. + +// Each `tests/*.rs` file is its own crate and compiles this module separately +// via its own `mod common;`. A helper used by only some of them is therefore +// genuinely unused in the others, and `clippy --all-targets -- -D warnings` +// would reject it. +#![allow(dead_code)] + +use std::path::{Path, PathBuf}; +use tempfile::TempDir; + +/// Points `CLAUDE_CODE_SYNC_CONFIG_DIR` at a fresh temp dir for the lifetime of +/// the guard, and unsets it on drop. +/// +/// This replaces the previous convention of calling `env::set_var` at the top of +/// a test and `env::remove_var` as the last statement of the body. Those tests +/// return `Result<()>` and use `?` throughout, so *any* early return — not only +/// a panic — skipped the cleanup and left the variable pointing at a `TempDir` +/// that was about to be deleted. Every subsequent test in the same binary then +/// resolved its config against a path that no longer existed, which reads like +/// flaky CI rather than the bug it is. `Drop` runs on the early-return and +/// unwind paths both. +/// +/// Note this *removes* the variable rather than restoring a previous value, +/// matching what the hand-written cleanup did. +/// +/// `CLAUDE_CODE_SYNC_CONFIG_DIR` is honoured on every platform, unlike +/// `XDG_CONFIG_HOME`, which macOS ignores. +pub struct ConfigEnv { + temp_dir: TempDir, +} + +impl ConfigEnv { + pub fn new() -> Self { + let temp_dir = TempDir::new().expect("failed to create temp config dir"); + std::env::set_var("CLAUDE_CODE_SYNC_CONFIG_DIR", temp_dir.path()); + Self { temp_dir } + } + + /// The isolated root — the value the environment variable points at. + pub fn path(&self) -> &Path { + self.temp_dir.path() + } + + /// A path inside the isolated root. + pub fn join(&self, name: &str) -> PathBuf { + self.temp_dir.path().join(name) + } + + /// `/claude-code-sync`, created on demand — this is where + /// `ConfigManager` actually reads and writes. + pub fn config_dir(&self) -> PathBuf { + let dir = self.temp_dir.path().join("claude-code-sync"); + std::fs::create_dir_all(&dir).expect("failed to create config dir"); + dir + } + + /// Plant a `state.json` verbatim, for exercising the v1 and v2 on-disk formats. + pub fn write_state_json(&self, contents: &str) -> PathBuf { + let path = self.config_dir().join("state.json"); + std::fs::write(&path, contents).expect("failed to write state.json"); + path + } +} + +impl Default for ConfigEnv { + fn default() -> Self { + Self::new() + } +} + +impl Drop for ConfigEnv { + fn drop(&mut self) { + std::env::remove_var("CLAUDE_CODE_SYNC_CONFIG_DIR"); + } +} diff --git a/tests/test_config_state.rs b/tests/test_config_state.rs new file mode 100644 index 00000000..9142735c --- /dev/null +++ b/tests/test_config_state.rs @@ -0,0 +1,185 @@ +//! Config paths, filter configuration, and SyncState serialization. + +mod common; + +use anyhow::Result; +use claude_code_sync::config::ConfigManager; +use claude_code_sync::filter::FilterConfig; +use claude_code_sync::sync::SyncState; +use common::ConfigEnv; +use serial_test::serial; +use std::path::PathBuf; + +// --------------------------------------------------------------------------- +// ConfigManager paths +// --------------------------------------------------------------------------- + +#[test] +#[serial] +fn test_config_manager_paths() -> Result<()> { + let _env = ConfigEnv::new(); + + let config_dir = ConfigManager::config_dir()?; + assert!(config_dir.to_string_lossy().contains("claude-code-sync")); + + let state_file = ConfigManager::state_file_path()?; + assert!(state_file.ends_with("state.json")); + + let filter_config = ConfigManager::filter_config_path()?; + assert!(filter_config.ends_with("config.toml")); + + let history = ConfigManager::operation_history_path()?; + assert!(history.ends_with("operation-history.json")); + + let snapshots = ConfigManager::snapshots_dir()?; + assert!(snapshots.ends_with("snapshots")); + + let repo = ConfigManager::default_repo_dir()?; + assert!(repo.ends_with("repo")); + + Ok(()) +} + +#[test] +#[serial] +fn test_default_repo_dir_exists() -> Result<()> { + let _env = ConfigEnv::new(); + + let default_dir = ConfigManager::default_repo_dir()?; + assert!(default_dir.ends_with("repo")); + assert!(default_dir.to_string_lossy().contains("claude-code-sync")); + + Ok(()) +} + +#[test] +#[serial] +fn test_ensure_config_dir_creates_directory() -> Result<()> { + // Guarded: this mkdir's, and without the override it would do so in the + // developer's real home directory. + let _env = ConfigEnv::new(); + + let config_dir = ConfigManager::ensure_config_dir()?; + assert!(config_dir.exists()); + assert!(config_dir.is_dir()); + + Ok(()) +} + +#[test] +#[serial] +fn test_config_directory_structure() -> Result<()> { + let _env = ConfigEnv::new(); + + let config_dir = ConfigManager::ensure_config_dir()?; + assert!(config_dir.exists()); + assert!(config_dir.is_dir()); + + let snapshots_dir = ConfigManager::ensure_snapshots_dir()?; + assert!(snapshots_dir.exists()); + assert!(snapshots_dir.is_dir()); + + assert!(snapshots_dir.starts_with(&config_dir)); + + Ok(()) +} + +// --------------------------------------------------------------------------- +// FilterConfig +// --------------------------------------------------------------------------- + +#[test] +#[serial] +fn test_filter_config_save_and_load() -> Result<()> { + let _env = ConfigEnv::new(); + + let config = FilterConfig { + exclude_attachments: true, + exclude_older_than_days: Some(30), + ..Default::default() + }; + config.save()?; + + let loaded = FilterConfig::load()?; + assert!(loaded.exclude_attachments); + assert_eq!(loaded.exclude_older_than_days, Some(30)); + + Ok(()) +} + +#[test] +#[serial] +fn test_multiple_config_operations() -> Result<()> { + let _env = ConfigEnv::new(); + + // Loading from an empty config dir must yield defaults rather than erroring. + let _loaded = FilterConfig::load()?; + + let config = FilterConfig { + exclude_attachments: true, + exclude_older_than_days: Some(99), + ..Default::default() + }; + config.save()?; + + let loaded2 = FilterConfig::load()?; + assert!(loaded2.exclude_attachments); + assert_eq!(loaded2.exclude_older_than_days, Some(99)); + + Ok(()) +} + +#[test] +fn test_filter_config_with_attachments() -> Result<()> { + let config = FilterConfig { + exclude_attachments: true, + ..Default::default() + }; + + assert!(config.should_include(&PathBuf::from("session.jsonl"))); + + assert!(!config.should_include(&PathBuf::from("image.png"))); + assert!(!config.should_include(&PathBuf::from("document.pdf"))); + assert!(!config.should_include(&PathBuf::from("video.mp4"))); + + Ok(()) +} + +// --------------------------------------------------------------------------- +// SyncState (de)serialization +// --------------------------------------------------------------------------- + +#[test] +fn test_sync_state_with_cloned_flag() -> Result<()> { + let state = SyncState { + sync_repo_path: PathBuf::from("/tmp/test-repo"), + has_remote: true, + is_cloned_repo: true, + }; + + let serialized = serde_json::to_string(&state)?; + assert!(serialized.contains("is_cloned_repo")); + assert!(serialized.contains("true")); + + let deserialized: SyncState = serde_json::from_str(&serialized)?; + assert_eq!(deserialized.sync_repo_path, PathBuf::from("/tmp/test-repo")); + assert!(deserialized.has_remote); + assert!(deserialized.is_cloned_repo); + + Ok(()) +} + +#[test] +fn test_sync_state_backwards_compatible() -> Result<()> { + // State files written before `is_cloned_repo` existed must still load. + let old_state_json = r#"{ + "sync_repo_path": "/tmp/test-repo", + "has_remote": true + }"#; + + let state: SyncState = serde_json::from_str(old_state_json)?; + assert!(state.has_remote); + assert!(!state.is_cloned_repo); // Should default to false + + Ok(()) +} diff --git a/tests/test_multi_repo.rs b/tests/test_multi_repo.rs new file mode 100644 index 00000000..bce80222 --- /dev/null +++ b/tests/test_multi_repo.rs @@ -0,0 +1,404 @@ +//! MultiRepoState: the v2 on-disk format, migration from v1, and switching the +//! active repository. + +mod common; + +use anyhow::Result; +use claude_code_sync::scm; +use claude_code_sync::sync::{self, MultiRepoState, RepoConfig, SyncState}; +use common::ConfigEnv; +use serial_test::serial; +use std::collections::HashMap; +use std::path::PathBuf; + +/// A v2 state.json with two repos, `work` active. +const TWO_REPOS_V2: &str = r#"{ + "version": 2, + "active_repo": "work", + "repos": { + "work": { + "name": "work", + "sync_repo_path": "/tmp/work-repo", + "has_remote": true, + "is_cloned_repo": false + }, + "personal": { + "name": "personal", + "sync_repo_path": "/tmp/personal-repo", + "has_remote": false, + "is_cloned_repo": false + } + } +}"#; + +// --------------------------------------------------------------------------- +// Format +// --------------------------------------------------------------------------- + +#[test] +fn test_multi_repo_state_serialization() -> Result<()> { + let repo_config = RepoConfig { + name: "work".to_string(), + sync_repo_path: PathBuf::from("/tmp/work-repo"), + has_remote: true, + is_cloned_repo: false, + remote_url: Some("https://github.com/user/work.git".to_string()), + description: Some("Work projects".to_string()), + }; + + let mut repos = HashMap::new(); + repos.insert("work".to_string(), repo_config); + + let state = MultiRepoState { + version: 2, + active_repo: "work".to_string(), + repos, + }; + + let serialized = serde_json::to_string_pretty(&state)?; + assert!(serialized.contains("\"version\": 2")); + assert!(serialized.contains("\"active_repo\": \"work\"")); + assert!(serialized.contains("\"remote_url\": \"https://github.com/user/work.git\"")); + + let deserialized: MultiRepoState = serde_json::from_str(&serialized)?; + assert_eq!(deserialized.version, 2); + assert_eq!(deserialized.active_repo, "work"); + + let repo = deserialized.repos.get("work").unwrap(); + assert_eq!(repo.name, "work"); + assert!(repo.has_remote); + assert_eq!( + repo.remote_url, + Some("https://github.com/user/work.git".to_string()) + ); + + Ok(()) +} + +// --------------------------------------------------------------------------- +// Migration and the SyncState compatibility layer +// --------------------------------------------------------------------------- + +#[test] +#[serial] +fn test_v1_to_v2_migration() -> Result<()> { + let env = ConfigEnv::new(); + + let state_path = env.write_state_json( + r#"{ + "sync_repo_path": "/tmp/legacy-repo", + "has_remote": true, + "is_cloned_repo": false + }"#, + ); + + // Loading a v1 file must transparently upgrade it. + let multi_state = MultiRepoState::load()?; + + assert_eq!(multi_state.version, 2); + assert_eq!(multi_state.active_repo, "default"); + + let default_repo = multi_state.repos.get("default").unwrap(); + assert_eq!( + default_repo.sync_repo_path, + PathBuf::from("/tmp/legacy-repo") + ); + assert!(default_repo.has_remote); + assert!(!default_repo.is_cloned_repo); + + // ...and persist the upgrade, not just hold it in memory. + let content = std::fs::read_to_string(&state_path)?; + assert!(content.contains("\"version\": 2")); + assert!(content.contains("\"active_repo\": \"default\"")); + + Ok(()) +} + +#[test] +#[serial] +fn test_sync_state_loads_v2_format() -> Result<()> { + let env = ConfigEnv::new(); + + env.write_state_json( + r#"{ + "version": 2, + "active_repo": "myrepo", + "repos": { + "myrepo": { + "name": "myrepo", + "sync_repo_path": "/tmp/my-repo", + "has_remote": true, + "is_cloned_repo": true, + "remote_url": "https://github.com/user/repo.git" + } + } + }"#, + ); + + // SyncState is the v1-shaped view over the active repo. + let sync_state = SyncState::load()?; + + assert_eq!(sync_state.sync_repo_path, PathBuf::from("/tmp/my-repo")); + assert!(sync_state.has_remote); + assert!(sync_state.is_cloned_repo); + + Ok(()) +} + +#[test] +#[serial] +fn test_multi_repo_state_multiple_repos() -> Result<()> { + let env = ConfigEnv::new(); + env.write_state_json(TWO_REPOS_V2); + + let multi_state = MultiRepoState::load()?; + + assert_eq!(multi_state.repos.len(), 2); + assert!(multi_state.repos.contains_key("work")); + assert!(multi_state.repos.contains_key("personal")); + assert_eq!(multi_state.active_repo, "work"); + + let sync_state = SyncState::load()?; + assert_eq!(sync_state.sync_repo_path, PathBuf::from("/tmp/work-repo")); + assert!(sync_state.has_remote); + + Ok(()) +} + +#[test] +#[serial] +fn test_invalid_active_repo_error() -> Result<()> { + let env = ConfigEnv::new(); + + // active_repo names a repo that isn't in the map. + env.write_state_json( + r#"{ + "version": 2, + "active_repo": "nonexistent", + "repos": { + "work": { + "name": "work", + "sync_repo_path": "/tmp/work-repo", + "has_remote": true, + "is_cloned_repo": false + } + } + }"#, + ); + + let result = SyncState::load(); + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("nonexistent")); + + Ok(()) +} + +#[test] +#[serial] +fn test_config_handles_uninitialized_state() -> Result<()> { + // No state file at all — a fresh install. + let _env = ConfigEnv::new(); + + let result = MultiRepoState::load(); + assert!(result.is_err()); + let err_msg = result.unwrap_err().to_string(); + assert!( + err_msg.contains("not initialized") || err_msg.contains("Run 'claude-code-sync init'"), + "Error message should mention not initialized: {err_msg}" + ); + + Ok(()) +} + +// --------------------------------------------------------------------------- +// Switching the active repo +// --------------------------------------------------------------------------- + +#[test] +#[serial] +fn test_switch_active_repo() -> Result<()> { + let env = ConfigEnv::new(); + env.write_state_json(TWO_REPOS_V2); + + let mut multi_state = MultiRepoState::load()?; + assert_eq!(multi_state.active_repo, "work"); + + multi_state.active_repo = "personal".to_string(); + multi_state.save()?; + + let reloaded = MultiRepoState::load()?; + assert_eq!(reloaded.active_repo, "personal"); + + // The switch has to be visible through the compatibility layer too. + let sync_state = SyncState::load()?; + assert_eq!( + sync_state.sync_repo_path, + PathBuf::from("/tmp/personal-repo") + ); + assert!(!sync_state.has_remote); + + Ok(()) +} + +#[test] +#[serial] +fn test_operations_use_active_repo() -> Result<()> { + let env = ConfigEnv::new(); + let repo1_path = env.join("repo1"); + let repo2_path = env.join("repo2"); + + sync::init_sync_repo(&repo1_path, None)?; + + // Add a second repo by hand. + scm::init(&repo2_path)?; + let mut multi_state = MultiRepoState::load()?; + multi_state.repos.insert( + "repo2".to_string(), + RepoConfig { + name: "repo2".to_string(), + sync_repo_path: repo2_path.clone(), + has_remote: false, + is_cloned_repo: false, + remote_url: None, + description: Some("Second repo".to_string()), + }, + ); + multi_state.save()?; + + // Merely existing must not make repo2 active. + assert_eq!(SyncState::load()?.sync_repo_path, repo1_path); + + let mut multi_state = MultiRepoState::load()?; + multi_state.active_repo = "repo2".to_string(); + multi_state.save()?; + + assert_eq!(SyncState::load()?.sync_repo_path, repo2_path); + + Ok(()) +} + +// --------------------------------------------------------------------------- +// init writes v2 +// --------------------------------------------------------------------------- + +#[test] +#[serial] +fn test_init_creates_v2_format_with_git_repo() -> Result<()> { + let env = ConfigEnv::new(); + let repo_path = env.join("test-sync-repo"); + + sync::init_sync_repo(&repo_path, None)?; + + assert!(repo_path.join(".git").exists()); + + let content = std::fs::read_to_string(env.config_dir().join("state.json"))?; + assert!(content.contains("\"version\": 2")); + assert!(content.contains("\"active_repo\": \"default\"")); + + let multi_state = MultiRepoState::load()?; + assert_eq!(multi_state.version, 2); + assert_eq!(multi_state.active_repo, "default"); + + let default_repo = multi_state.repos.get("default").unwrap(); + assert_eq!(default_repo.sync_repo_path, repo_path); + assert!(!default_repo.has_remote); + + Ok(()) +} + +#[test] +#[serial] +fn test_init_with_remote_populates_remote_url() -> Result<()> { + let env = ConfigEnv::new(); + let repo_path = env.join("test-remote-repo"); + + sync::init_sync_repo(&repo_path, Some("https://github.com/user/repo.git"))?; + + let multi_state = MultiRepoState::load()?; + let default_repo = multi_state.repos.get("default").unwrap(); + + assert!(default_repo.has_remote); + assert_eq!( + default_repo.remote_url, + Some("https://github.com/user/repo.git".to_string()) + ); + + Ok(()) +} + +#[test] +#[serial] +fn test_init_from_onboarding_creates_v2() -> Result<()> { + let env = ConfigEnv::new(); + let repo_path = env.join("onboarding-repo"); + + sync::init_from_onboarding(&repo_path, Some("https://github.com/test/repo.git"), false)?; + + let multi_state = MultiRepoState::load()?; + assert_eq!(multi_state.version, 2); + assert_eq!(multi_state.active_repo, "default"); + + let default_repo = multi_state.repos.get("default").unwrap(); + assert_eq!(default_repo.sync_repo_path, repo_path); + assert!(default_repo.has_remote); + assert!(!default_repo.is_cloned_repo); + assert_eq!( + default_repo.remote_url, + Some("https://github.com/test/repo.git".to_string()) + ); + + Ok(()) +} + +#[test] +#[serial] +fn test_cloned_repo_flag_in_v2() -> Result<()> { + let env = ConfigEnv::new(); + let repo_path = env.join("cloned-repo"); + + // Pre-create the repo, as a clone would have. + scm::init(&repo_path)?; + sync::init_from_onboarding(&repo_path, Some("https://github.com/test/repo.git"), true)?; + + let multi_state = MultiRepoState::load()?; + assert!(multi_state.repos.get("default").unwrap().is_cloned_repo); + + // And through the compatibility layer. + assert!(SyncState::load()?.is_cloned_repo); + + Ok(()) +} + +#[test] +#[serial] +fn test_full_workflow_with_git_repos() -> Result<()> { + let env = ConfigEnv::new(); + + // A bare repo standing in for a real remote. + let bare_repo_path = env.join("bare-remote.git"); + std::process::Command::new("git") + .args(["init", "--bare"]) + .arg(&bare_repo_path) + .output()?; + + let repo_path = env.join("local-sync-repo"); + let remote_url = format!("file://{}", bare_repo_path.display()); + + sync::init_sync_repo(&repo_path, Some(&remote_url))?; + + let multi_state = MultiRepoState::load()?; + assert_eq!(multi_state.version, 2); + + let default_repo = multi_state.repos.get("default").unwrap(); + assert!(default_repo.has_remote); + assert_eq!(default_repo.remote_url, Some(remote_url.clone())); + + // The remote must exist in git itself, not just in our state file. + let output = std::process::Command::new("git") + .args(["remote", "-v"]) + .current_dir(&repo_path) + .output()?; + assert!(String::from_utf8_lossy(&output.stdout).contains("origin")); + + Ok(()) +} diff --git a/tests/test_onboarding.rs b/tests/test_onboarding.rs index f15a9490..45f83449 100644 --- a/tests/test_onboarding.rs +++ b/tests/test_onboarding.rs @@ -1,349 +1,153 @@ +//! Repository initialization: the onboarding flow, the `--repo` flag, cloning, +//! and `claude-code-sync-init.toml` validation. + +mod common; + use anyhow::Result; use claude_code_sync::config::ConfigManager; use claude_code_sync::filter::FilterConfig; +use claude_code_sync::onboarding::InitConfig; use claude_code_sync::scm; -use claude_code_sync::sync::SyncState; +use claude_code_sync::sync::{self, SyncState}; +use common::ConfigEnv; use serial_test::serial; +use std::io::Write; use tempfile::TempDir; -/// Test helper to setup a temporary config directory for testing -fn setup_test_config_env() -> Result { - TempDir::new().map_err(Into::into) -} - -#[test] -fn test_config_manager_paths() -> Result<()> { - // Test that all config paths can be retrieved - let config_dir = ConfigManager::config_dir()?; - assert!(config_dir.to_string_lossy().contains("claude-code-sync")); - - let state_file = ConfigManager::state_file_path()?; - assert!(state_file.ends_with("state.json")); - - let filter_config = ConfigManager::filter_config_path()?; - assert!(filter_config.ends_with("config.toml")); - - let history = ConfigManager::operation_history_path()?; - assert!(history.ends_with("operation-history.json")); - - let snapshots = ConfigManager::snapshots_dir()?; - assert!(snapshots.ends_with("snapshots")); - - let repo = ConfigManager::default_repo_dir()?; - assert!(repo.ends_with("repo")); - - Ok(()) -} - -#[test] -fn test_ensure_config_dir_creates_directory() -> Result<()> { - // This test will create the directory if it doesn't exist - // Note: This modifies the actual user's home directory, so we can only verify it succeeds - let config_dir = ConfigManager::ensure_config_dir()?; - assert!(config_dir.exists()); - assert!(config_dir.is_dir()); - Ok(()) -} - -#[test] -fn test_sync_state_with_cloned_flag() -> Result<()> { - let temp_dir = setup_test_config_env()?; - let repo_path = temp_dir.path().join("test-repo"); - - // Test serialization/deserialization with is_cloned_repo field - let state = SyncState { - sync_repo_path: repo_path.clone(), - has_remote: true, - is_cloned_repo: true, - }; - - let serialized = serde_json::to_string(&state)?; - assert!(serialized.contains("is_cloned_repo")); - assert!(serialized.contains("true")); - - let deserialized: SyncState = serde_json::from_str(&serialized)?; - assert_eq!(deserialized.sync_repo_path, repo_path); - assert!(deserialized.has_remote); - assert!(deserialized.is_cloned_repo); - - Ok(()) -} - -#[test] -fn test_sync_state_backwards_compatible() -> Result<()> { - // Test that old state files (without is_cloned_repo) can still be loaded - let old_state_json = r#"{ - "sync_repo_path": "/tmp/test-repo", - "has_remote": true - }"#; - - let state: SyncState = serde_json::from_str(old_state_json)?; - assert!(state.has_remote); - assert!(!state.is_cloned_repo); // Should default to false - - Ok(()) -} - -#[test] -#[serial] -fn test_filter_config_save_and_load() -> Result<()> { - let temp_dir = setup_test_config_env()?; - - // Set XDG_CONFIG_HOME to isolate test config - std::env::set_var("CLAUDE_CODE_SYNC_CONFIG_DIR", temp_dir.path()); - - let config = FilterConfig { - exclude_attachments: true, - exclude_older_than_days: Some(30), - ..Default::default() - }; - - config.save()?; - - let loaded = FilterConfig::load()?; - assert!(loaded.exclude_attachments); - assert_eq!(loaded.exclude_older_than_days, Some(30)); - - // Clean up env var - std::env::remove_var("CLAUDE_CODE_SYNC_CONFIG_DIR"); - - Ok(()) -} - -#[test] -fn test_scm_clone_validates_path() -> Result<()> { - let temp_dir = setup_test_config_env()?; - let clone_path = temp_dir.path().join("cloned-repo"); - - // We can't test actual cloning without a real remote repo - // But we can test that the path validation and setup works - - // Try to clone from an invalid URL (this will fail, but we can test the error handling) - let result = scm::clone("invalid-url", &clone_path); - assert!(result.is_err()); - - // The error should contain helpful information - let err = result.err().unwrap(); - let err_msg = format!("{err}"); - assert!(err_msg.contains("clone failed") || err_msg.contains("Failed")); - - Ok(()) -} +// --------------------------------------------------------------------------- +// init_from_onboarding +// --------------------------------------------------------------------------- #[test] #[serial] fn test_init_from_onboarding() -> Result<()> { - let temp_dir = setup_test_config_env()?; - let repo_path = temp_dir.path().join("onboarding-test-repo"); - - // Set XDG_CONFIG_HOME to isolate test config - std::env::set_var("CLAUDE_CODE_SYNC_CONFIG_DIR", temp_dir.path()); + let env = ConfigEnv::new(); + let repo_path = env.join("onboarding-test-repo"); - // Initialize a repo first scm::init(&repo_path)?; + sync::init_from_onboarding(&repo_path, None, false)?; - // Test init_from_onboarding with a local repository - claude_code_sync::sync::init_from_onboarding(&repo_path, None, false)?; - - // Verify state was saved let state = SyncState::load()?; assert_eq!(state.sync_repo_path, repo_path); assert!(!state.has_remote); assert!(!state.is_cloned_repo); - // Clean up env var - std::env::remove_var("CLAUDE_CODE_SYNC_CONFIG_DIR"); - Ok(()) } #[test] #[serial] fn test_init_from_onboarding_with_remote() -> Result<()> { - let temp_dir = setup_test_config_env()?; - let repo_path = temp_dir.path().join("onboarding-remote-test"); - - // Set XDG_CONFIG_HOME to isolate test config - std::env::set_var("CLAUDE_CODE_SYNC_CONFIG_DIR", temp_dir.path()); + let env = ConfigEnv::new(); + let repo_path = env.join("onboarding-remote-test"); - // Initialize a repo first scm::init(&repo_path)?; + sync::init_from_onboarding(&repo_path, Some("https://github.com/user/repo.git"), true)?; - // Test with remote URL - claude_code_sync::sync::init_from_onboarding( - &repo_path, - Some("https://github.com/user/repo.git"), - true, - )?; - - // Verify state was saved let state = SyncState::load()?; assert_eq!(state.sync_repo_path, repo_path); assert!(state.has_remote); assert!(state.is_cloned_repo); - // Clean up env var - std::env::remove_var("CLAUDE_CODE_SYNC_CONFIG_DIR"); - - Ok(()) -} - -#[test] -fn test_config_directory_structure() -> Result<()> { - // Ensure config directory can be created - let config_dir = ConfigManager::ensure_config_dir()?; - - // Verify it exists - assert!(config_dir.exists()); - assert!(config_dir.is_dir()); - - // Ensure snapshots directory can be created - let snapshots_dir = ConfigManager::ensure_snapshots_dir()?; - assert!(snapshots_dir.exists()); - assert!(snapshots_dir.is_dir()); - - // Verify snapshots is a subdirectory of config - assert!(snapshots_dir.starts_with(&config_dir)); - Ok(()) } #[test] -fn test_filter_config_with_attachments() -> Result<()> { - use std::path::PathBuf; - - // Test exclude_attachments flag - let mut config = FilterConfig { - exclude_attachments: true, - ..Default::default() - }; - - // Should include .jsonl files - assert!(config.should_include(&PathBuf::from("session.jsonl"))); +#[serial] +fn test_init_from_onboarding_sets_is_cloned_flag() -> Result<()> { + let env = ConfigEnv::new(); + let repo_path = env.join("cloned-repo-test"); - // Should exclude non-.jsonl files - assert!(!config.should_include(&PathBuf::from("image.png"))); - assert!(!config.should_include(&PathBuf::from("document.pdf"))); - assert!(!config.should_include(&PathBuf::from("video.mp4"))); + // Simulating the post-clone state. + scm::init(&repo_path)?; + sync::init_from_onboarding(&repo_path, Some("https://github.com/user/repo.git"), true)?; - // With exclude_attachments = false, should include everything - config.exclude_attachments = false; - // Note: This might fail due to file size checks, so we'll skip this part - // in a real test environment + let state = SyncState::load()?; + assert_eq!(state.sync_repo_path, repo_path); + assert!(state.has_remote); + assert!( + state.is_cloned_repo, + "is_cloned_repo should be true for cloned repos" + ); Ok(()) } #[test] #[serial] -fn test_multiple_config_operations() -> Result<()> { - let temp_dir = setup_test_config_env()?; - - // Set XDG_CONFIG_HOME to isolate test config - std::env::set_var("CLAUDE_CODE_SYNC_CONFIG_DIR", temp_dir.path()); - - let _loaded = FilterConfig::load()?; - - // Save a new config with known values - let config = FilterConfig { - exclude_attachments: true, - exclude_older_than_days: Some(99), - ..Default::default() - }; - config.save()?; +fn test_init_from_onboarding_local_repo_not_cloned() -> Result<()> { + let env = ConfigEnv::new(); + let repo_path = env.join("local-repo-test"); - // Verify it was saved - let loaded2 = FilterConfig::load()?; - assert!(loaded2.exclude_attachments); - assert_eq!(loaded2.exclude_older_than_days, Some(99)); + scm::init(&repo_path)?; + sync::init_from_onboarding(&repo_path, None, false)?; - // Clean up env var - std::env::remove_var("CLAUDE_CODE_SYNC_CONFIG_DIR"); + let state = SyncState::load()?; + assert_eq!(state.sync_repo_path, repo_path); + assert!(!state.has_remote); + assert!( + !state.is_cloned_repo, + "is_cloned_repo should be false for local repos" + ); Ok(()) } -// ============================================================================ -// Tests for Bug Fixes: --repo flag creating FilterConfig -// ============================================================================ +// --------------------------------------------------------------------------- +// init_sync_repo (the `--repo` flag) also has to create a FilterConfig +// --------------------------------------------------------------------------- #[test] #[serial] fn test_init_sync_repo_creates_filter_config() -> Result<()> { - let temp_dir = setup_test_config_env()?; - let repo_path = temp_dir.path().join("cli-init-test-repo"); + let env = ConfigEnv::new(); + let repo_path = env.join("cli-init-test-repo"); - // Set XDG_CONFIG_HOME to isolate test config - std::env::set_var("CLAUDE_CODE_SYNC_CONFIG_DIR", temp_dir.path()); - - // Initialize using init_sync_repo (simulates --repo flag) - claude_code_sync::sync::init_sync_repo(&repo_path, None)?; + sync::init_sync_repo(&repo_path, None)?; - // Verify state was saved let state = SyncState::load()?; assert_eq!(state.sync_repo_path, repo_path); assert!(!state.has_remote); assert!(!state.is_cloned_repo); - // BUG FIX: Verify filter config was also saved - let filter_config_path = ConfigManager::filter_config_path()?; assert!( - filter_config_path.exists(), + ConfigManager::filter_config_path()?.exists(), "Filter config should be created by init_sync_repo" ); - // Verify we can load the filter config let filter_config = FilterConfig::load()?; - // Default values should be set assert!(!filter_config.exclude_attachments); - // Clean up env var - std::env::remove_var("CLAUDE_CODE_SYNC_CONFIG_DIR"); - Ok(()) } #[test] #[serial] fn test_init_sync_repo_with_remote_creates_filter_config() -> Result<()> { - let temp_dir = setup_test_config_env()?; - let repo_path = temp_dir.path().join("cli-remote-test-repo"); + let env = ConfigEnv::new(); + let repo_path = env.join("cli-remote-test-repo"); - // Set XDG_CONFIG_HOME to isolate test config - std::env::set_var("CLAUDE_CODE_SYNC_CONFIG_DIR", temp_dir.path()); - - // Initialize with remote URL using init_sync_repo (simulates --repo --remote flags) - claude_code_sync::sync::init_sync_repo(&repo_path, Some("https://github.com/user/repo.git"))?; + sync::init_sync_repo(&repo_path, Some("https://github.com/user/repo.git"))?; - // Verify state was saved with remote let state = SyncState::load()?; assert_eq!(state.sync_repo_path, repo_path); assert!(state.has_remote); assert!(!state.is_cloned_repo); // Not cloned, just added as origin - // Verify filter config was also saved - let filter_config_path = ConfigManager::filter_config_path()?; assert!( - filter_config_path.exists(), + ConfigManager::filter_config_path()?.exists(), "Filter config should be created by init_sync_repo with remote" ); - // Clean up env var - std::env::remove_var("CLAUDE_CODE_SYNC_CONFIG_DIR"); - Ok(()) } #[test] #[serial] fn test_init_sync_repo_does_not_overwrite_existing_filter_config() -> Result<()> { - let temp_dir = setup_test_config_env()?; - let repo_path = temp_dir.path().join("no-overwrite-test-repo"); + let env = ConfigEnv::new(); + let repo_path = env.join("no-overwrite-test-repo"); - // Set XDG_CONFIG_HOME to isolate test config - std::env::set_var("CLAUDE_CODE_SYNC_CONFIG_DIR", temp_dir.path()); - - // Create an existing filter config with custom values let custom_config = FilterConfig { exclude_attachments: true, exclude_older_than_days: Some(42), @@ -351,10 +155,8 @@ fn test_init_sync_repo_does_not_overwrite_existing_filter_config() -> Result<()> }; custom_config.save()?; - // Initialize using init_sync_repo - claude_code_sync::sync::init_sync_repo(&repo_path, None)?; + sync::init_sync_repo(&repo_path, None)?; - // Verify the existing filter config was NOT overwritten let loaded_config = FilterConfig::load()?; assert!( loaded_config.exclude_attachments, @@ -366,22 +168,32 @@ fn test_init_sync_repo_does_not_overwrite_existing_filter_config() -> Result<()> "Custom exclude_older_than_days should be preserved" ); - // Clean up env var - std::env::remove_var("CLAUDE_CODE_SYNC_CONFIG_DIR"); - Ok(()) } -// ============================================================================ -// Tests for --clone flag and remote-only init -// ============================================================================ +// --------------------------------------------------------------------------- +// Cloning +// --------------------------------------------------------------------------- + +#[test] +fn test_scm_clone_validates_path() -> Result<()> { + let temp_dir = TempDir::new()?; + let clone_path = temp_dir.path().join("cloned-repo"); + + let result = scm::clone("invalid-url", &clone_path); + assert!(result.is_err()); + + let err_msg = result.err().unwrap().to_string(); + assert!(err_msg.contains("clone failed") || err_msg.contains("Failed")); + + Ok(()) +} #[test] fn test_clone_with_invalid_url_fails() -> Result<()> { - let temp_dir = setup_test_config_env()?; + let temp_dir = TempDir::new()?; let clone_path = temp_dir.path().join("clone-test-repo"); - // Try to clone from an invalid URL let result = scm::clone("not-a-valid-url", &clone_path); assert!(result.is_err(), "Clone should fail with invalid URL"); @@ -390,7 +202,7 @@ fn test_clone_with_invalid_url_fails() -> Result<()> { #[test] fn test_clone_creates_parent_directories() -> Result<()> { - let temp_dir = setup_test_config_env()?; + let temp_dir = TempDir::new()?; let nested_path = temp_dir .path() .join("deeply") @@ -398,123 +210,41 @@ fn test_clone_creates_parent_directories() -> Result<()> { .join("path") .join("repo"); - // Even though clone will fail (invalid URL), it should create parent directories let result = scm::clone( "https://invalid-url-that-wont-work.example.com/repo.git", &nested_path, ); - // Clone fails but parent directory should be created + // The clone fails, but the parent directories should have been created first. assert!(result.is_err()); - // Parent should exist even though clone failed assert!(nested_path.parent().unwrap().exists()); Ok(()) } -#[test] -#[serial] -fn test_init_from_onboarding_sets_is_cloned_flag() -> Result<()> { - let temp_dir = setup_test_config_env()?; - let repo_path = temp_dir.path().join("cloned-repo-test"); +// --------------------------------------------------------------------------- +// InitConfig validation +// --------------------------------------------------------------------------- - // Set XDG_CONFIG_HOME to isolate test config - std::env::set_var("CLAUDE_CODE_SYNC_CONFIG_DIR", temp_dir.path()); - - // Initialize a repo first (simulating post-clone state) - scm::init(&repo_path)?; - - // Test init_from_onboarding with is_cloned = true - claude_code_sync::sync::init_from_onboarding( - &repo_path, - Some("https://github.com/user/repo.git"), - true, // is_cloned - )?; - - // Verify state was saved with is_cloned_repo = true - let state = SyncState::load()?; - assert_eq!(state.sync_repo_path, repo_path); - assert!(state.has_remote); - assert!( - state.is_cloned_repo, - "is_cloned_repo should be true for cloned repos" - ); - - // Clean up env var - std::env::remove_var("CLAUDE_CODE_SYNC_CONFIG_DIR"); - - Ok(()) -} - -#[test] -#[serial] -fn test_init_from_onboarding_local_repo_not_cloned() -> Result<()> { - let temp_dir = setup_test_config_env()?; - let repo_path = temp_dir.path().join("local-repo-test"); - - // Set XDG_CONFIG_HOME to isolate test config - std::env::set_var("CLAUDE_CODE_SYNC_CONFIG_DIR", temp_dir.path()); - - // Initialize a repo first - scm::init(&repo_path)?; - - // Test init_from_onboarding with is_cloned = false (local repo) - claude_code_sync::sync::init_from_onboarding( - &repo_path, None, false, // not cloned - )?; - - // Verify state was saved with is_cloned_repo = false - let state = SyncState::load()?; - assert_eq!(state.sync_repo_path, repo_path); - assert!(!state.has_remote); - assert!( - !state.is_cloned_repo, - "is_cloned_repo should be false for local repos" - ); - - // Clean up env var - std::env::remove_var("CLAUDE_CODE_SYNC_CONFIG_DIR"); - - Ok(()) -} - -#[test] -fn test_default_repo_dir_exists() -> Result<()> { - // Test that we can get the default repo directory - let default_dir = ConfigManager::default_repo_dir()?; - - // Should end with "repo" - assert!(default_dir.ends_with("repo")); - - // Should contain claude-code-sync in path - assert!(default_dir.to_string_lossy().contains("claude-code-sync")); - - Ok(()) +/// Write an init TOML into `dir` and try to load it. +fn load_init_config(dir: &TempDir, body: &str) -> Result { + let config_path = dir.path().join("init.toml"); + let mut file = std::fs::File::create(&config_path)?; + writeln!(file, "{body}")?; + InitConfig::load(&config_path) } -// ============================================================================ -// Tests for InitConfig validation -// ============================================================================ - #[test] fn test_init_config_clone_requires_remote_url() -> Result<()> { - use claude_code_sync::onboarding::InitConfig; - use std::io::Write; - - let temp_dir = setup_test_config_env()?; + let temp_dir = TempDir::new()?; - // Create config file with clone=true but no remote_url - let config_path = temp_dir.path().join("init.toml"); - let mut file = std::fs::File::create(&config_path)?; - writeln!( - file, + let result = load_init_config( + &temp_dir, r#" repo_path = "/tmp/test" clone = true -"# - )?; - - let result = InitConfig::load(&config_path); +"#, + ); assert!( result.is_err(), @@ -531,24 +261,16 @@ clone = true #[test] fn test_init_config_clone_with_remote_url_valid() -> Result<()> { - use claude_code_sync::onboarding::InitConfig; - use std::io::Write; + let temp_dir = TempDir::new()?; - let temp_dir = setup_test_config_env()?; - - // Create config file with clone=true and remote_url - let config_path = temp_dir.path().join("init.toml"); - let mut file = std::fs::File::create(&config_path)?; - writeln!( - file, + let result = load_init_config( + &temp_dir, r#" repo_path = "/tmp/test" remote_url = "https://github.com/user/repo.git" clone = true -"# - )?; - - let result = InitConfig::load(&config_path); +"#, + ); assert!( result.is_ok(), @@ -558,531 +280,3 @@ clone = true Ok(()) } - -// ============================================================================= -// MultiRepoState Tests - Testing multi-repo configuration and migration -// ============================================================================= - -use claude_code_sync::sync::MultiRepoState; - -/// Test that MultiRepoState can be serialized and deserialized correctly -#[test] -fn test_multi_repo_state_serialization() -> Result<()> { - use claude_code_sync::sync::RepoConfig; - use std::collections::HashMap; - - let repo_config = RepoConfig { - name: "work".to_string(), - sync_repo_path: std::path::PathBuf::from("/tmp/work-repo"), - has_remote: true, - is_cloned_repo: false, - remote_url: Some("https://github.com/user/work.git".to_string()), - description: Some("Work projects".to_string()), - }; - - let mut repos = HashMap::new(); - repos.insert("work".to_string(), repo_config); - - let state = MultiRepoState { - version: 2, - active_repo: "work".to_string(), - repos, - }; - - let serialized = serde_json::to_string_pretty(&state)?; - assert!(serialized.contains("\"version\": 2")); - assert!(serialized.contains("\"active_repo\": \"work\"")); - assert!(serialized.contains("\"remote_url\": \"https://github.com/user/work.git\"")); - - let deserialized: MultiRepoState = serde_json::from_str(&serialized)?; - assert_eq!(deserialized.version, 2); - assert_eq!(deserialized.active_repo, "work"); - assert!(deserialized.repos.contains_key("work")); - - let repo = deserialized.repos.get("work").unwrap(); - assert_eq!(repo.name, "work"); - assert!(repo.has_remote); - assert_eq!( - repo.remote_url, - Some("https://github.com/user/work.git".to_string()) - ); - - Ok(()) -} - -/// Test migration from v1 SyncState format to v2 MultiRepoState format -#[test] -#[serial] -fn test_v1_to_v2_migration() -> Result<()> { - let temp_dir = setup_test_config_env()?; - std::env::set_var("CLAUDE_CODE_SYNC_CONFIG_DIR", temp_dir.path()); - - // Create config directory - let config_dir = temp_dir.path().join("claude-code-sync"); - std::fs::create_dir_all(&config_dir)?; - - // Write a v1 format state.json - let v1_state = r#"{ - "sync_repo_path": "/tmp/legacy-repo", - "has_remote": true, - "is_cloned_repo": false - }"#; - let state_path = config_dir.join("state.json"); - std::fs::write(&state_path, v1_state)?; - - // Load should auto-migrate to v2 - let multi_state = MultiRepoState::load()?; - - assert_eq!(multi_state.version, 2); - assert_eq!(multi_state.active_repo, "default"); - assert!(multi_state.repos.contains_key("default")); - - let default_repo = multi_state.repos.get("default").unwrap(); - assert_eq!( - default_repo.sync_repo_path, - std::path::PathBuf::from("/tmp/legacy-repo") - ); - assert!(default_repo.has_remote); - assert!(!default_repo.is_cloned_repo); - - // Verify the file was rewritten in v2 format - let content = std::fs::read_to_string(&state_path)?; - assert!(content.contains("\"version\": 2")); - assert!(content.contains("\"active_repo\": \"default\"")); - - std::env::remove_var("CLAUDE_CODE_SYNC_CONFIG_DIR"); - Ok(()) -} - -/// Test that SyncState::load() works as a compatibility wrapper for v2 format -#[test] -#[serial] -fn test_sync_state_loads_v2_format() -> Result<()> { - let temp_dir = setup_test_config_env()?; - std::env::set_var("CLAUDE_CODE_SYNC_CONFIG_DIR", temp_dir.path()); - - // Create config directory - let config_dir = temp_dir.path().join("claude-code-sync"); - std::fs::create_dir_all(&config_dir)?; - - // Write v2 format state.json - let v2_state = r#"{ - "version": 2, - "active_repo": "myrepo", - "repos": { - "myrepo": { - "name": "myrepo", - "sync_repo_path": "/tmp/my-repo", - "has_remote": true, - "is_cloned_repo": true, - "remote_url": "https://github.com/user/repo.git" - } - } - }"#; - let state_path = config_dir.join("state.json"); - std::fs::write(&state_path, v2_state)?; - - // SyncState::load() should return the active repo's data - let sync_state = SyncState::load()?; - - assert_eq!( - sync_state.sync_repo_path, - std::path::PathBuf::from("/tmp/my-repo") - ); - assert!(sync_state.has_remote); - assert!(sync_state.is_cloned_repo); - - std::env::remove_var("CLAUDE_CODE_SYNC_CONFIG_DIR"); - Ok(()) -} - -/// Test MultiRepoState with multiple repos -#[test] -#[serial] -fn test_multi_repo_state_multiple_repos() -> Result<()> { - let temp_dir = setup_test_config_env()?; - std::env::set_var("CLAUDE_CODE_SYNC_CONFIG_DIR", temp_dir.path()); - - // Create config directory - let config_dir = temp_dir.path().join("claude-code-sync"); - std::fs::create_dir_all(&config_dir)?; - - // Write v2 format with multiple repos - let v2_state = r#"{ - "version": 2, - "active_repo": "work", - "repos": { - "work": { - "name": "work", - "sync_repo_path": "/tmp/work-repo", - "has_remote": true, - "is_cloned_repo": false - }, - "personal": { - "name": "personal", - "sync_repo_path": "/tmp/personal-repo", - "has_remote": false, - "is_cloned_repo": false - } - } - }"#; - let state_path = config_dir.join("state.json"); - std::fs::write(&state_path, v2_state)?; - - let multi_state = MultiRepoState::load()?; - - assert_eq!(multi_state.repos.len(), 2); - assert!(multi_state.repos.contains_key("work")); - assert!(multi_state.repos.contains_key("personal")); - assert_eq!(multi_state.active_repo, "work"); - - // SyncState should return the active (work) repo - let sync_state = SyncState::load()?; - assert_eq!( - sync_state.sync_repo_path, - std::path::PathBuf::from("/tmp/work-repo") - ); - assert!(sync_state.has_remote); - - std::env::remove_var("CLAUDE_CODE_SYNC_CONFIG_DIR"); - Ok(()) -} - -/// Test switching active repo and persisting -#[test] -#[serial] -fn test_switch_active_repo() -> Result<()> { - let temp_dir = setup_test_config_env()?; - std::env::set_var("CLAUDE_CODE_SYNC_CONFIG_DIR", temp_dir.path()); - - // Create config directory - let config_dir = temp_dir.path().join("claude-code-sync"); - std::fs::create_dir_all(&config_dir)?; - - // Write v2 format with multiple repos - let v2_state = r#"{ - "version": 2, - "active_repo": "work", - "repos": { - "work": { - "name": "work", - "sync_repo_path": "/tmp/work-repo", - "has_remote": true, - "is_cloned_repo": false - }, - "personal": { - "name": "personal", - "sync_repo_path": "/tmp/personal-repo", - "has_remote": false, - "is_cloned_repo": false - } - } - }"#; - let state_path = config_dir.join("state.json"); - std::fs::write(&state_path, v2_state)?; - - // Load, switch, save - let mut multi_state = MultiRepoState::load()?; - assert_eq!(multi_state.active_repo, "work"); - - multi_state.active_repo = "personal".to_string(); - multi_state.save()?; - - // Reload and verify - let reloaded = MultiRepoState::load()?; - assert_eq!(reloaded.active_repo, "personal"); - - // SyncState should now return personal repo - let sync_state = SyncState::load()?; - assert_eq!( - sync_state.sync_repo_path, - std::path::PathBuf::from("/tmp/personal-repo") - ); - assert!(!sync_state.has_remote); - - std::env::remove_var("CLAUDE_CODE_SYNC_CONFIG_DIR"); - Ok(()) -} - -/// Test init creates v2 format with actual git repo -#[test] -#[serial] -fn test_init_creates_v2_format_with_git_repo() -> Result<()> { - use claude_code_sync::sync; - - let temp_dir = setup_test_config_env()?; - std::env::set_var("CLAUDE_CODE_SYNC_CONFIG_DIR", temp_dir.path()); - - let repo_path = temp_dir.path().join("test-sync-repo"); - - // Initialize using the init function - sync::init_sync_repo(&repo_path, None)?; - - // Verify git repo was created - assert!(repo_path.join(".git").exists()); - - // Verify state.json is in v2 format - let config_dir = temp_dir.path().join("claude-code-sync"); - let state_path = config_dir.join("state.json"); - let content = std::fs::read_to_string(&state_path)?; - - assert!(content.contains("\"version\": 2")); - assert!(content.contains("\"active_repo\": \"default\"")); - - // Load and verify - let multi_state = MultiRepoState::load()?; - assert_eq!(multi_state.version, 2); - assert_eq!(multi_state.active_repo, "default"); - - let default_repo = multi_state.repos.get("default").unwrap(); - assert_eq!(default_repo.sync_repo_path, repo_path); - assert!(!default_repo.has_remote); - - std::env::remove_var("CLAUDE_CODE_SYNC_CONFIG_DIR"); - Ok(()) -} - -/// Test init with remote creates v2 format with remote_url populated -#[test] -#[serial] -fn test_init_with_remote_populates_remote_url() -> Result<()> { - use claude_code_sync::sync; - - let temp_dir = setup_test_config_env()?; - std::env::set_var("CLAUDE_CODE_SYNC_CONFIG_DIR", temp_dir.path()); - - let repo_path = temp_dir.path().join("test-remote-repo"); - - // Initialize with a remote URL - sync::init_sync_repo(&repo_path, Some("https://github.com/user/repo.git"))?; - - // Load and verify remote_url is populated - let multi_state = MultiRepoState::load()?; - let default_repo = multi_state.repos.get("default").unwrap(); - - assert!(default_repo.has_remote); - assert_eq!( - default_repo.remote_url, - Some("https://github.com/user/repo.git".to_string()) - ); - - std::env::remove_var("CLAUDE_CODE_SYNC_CONFIG_DIR"); - Ok(()) -} - -/// Test full workflow: init repo, push, verify state persists correctly -#[test] -#[serial] -fn test_full_workflow_with_git_repos() -> Result<()> { - use claude_code_sync::sync; - - let temp_dir = setup_test_config_env()?; - std::env::set_var("CLAUDE_CODE_SYNC_CONFIG_DIR", temp_dir.path()); - - // Create a "remote" bare repo to simulate a git remote - let bare_repo_path = temp_dir.path().join("bare-remote.git"); - std::process::Command::new("git") - .args(["init", "--bare"]) - .arg(&bare_repo_path) - .output()?; - - let repo_path = temp_dir.path().join("local-sync-repo"); - let remote_url = format!("file://{}", bare_repo_path.display()); - - // Initialize with the "remote" - sync::init_sync_repo(&repo_path, Some(&remote_url))?; - - // Verify state - let multi_state = MultiRepoState::load()?; - assert_eq!(multi_state.version, 2); - - let default_repo = multi_state.repos.get("default").unwrap(); - assert!(default_repo.has_remote); - assert_eq!(default_repo.remote_url, Some(remote_url.clone())); - - // Verify git remote is configured - let output = std::process::Command::new("git") - .args(["remote", "-v"]) - .current_dir(&repo_path) - .output()?; - let remote_output = String::from_utf8_lossy(&output.stdout); - assert!(remote_output.contains("origin")); - - std::env::remove_var("CLAUDE_CODE_SYNC_CONFIG_DIR"); - Ok(()) -} - -/// Test that operations use the active repo from MultiRepoState -#[test] -#[serial] -fn test_operations_use_active_repo() -> Result<()> { - use claude_code_sync::sync; - - let temp_dir = setup_test_config_env()?; - std::env::set_var("CLAUDE_CODE_SYNC_CONFIG_DIR", temp_dir.path()); - - // Create two repos - let repo1_path = temp_dir.path().join("repo1"); - let repo2_path = temp_dir.path().join("repo2"); - - // Initialize first repo - sync::init_sync_repo(&repo1_path, None)?; - - // Manually add a second repo to the state - let mut multi_state = MultiRepoState::load()?; - - use claude_code_sync::sync::RepoConfig; - - // Initialize second repo directory with git - scm::init(&repo2_path)?; - - let repo2_config = RepoConfig { - name: "repo2".to_string(), - sync_repo_path: repo2_path.clone(), - has_remote: false, - is_cloned_repo: false, - remote_url: None, - description: Some("Second repo".to_string()), - }; - multi_state.repos.insert("repo2".to_string(), repo2_config); - multi_state.save()?; - - // SyncState should still return repo1 (the active one) - let sync_state = SyncState::load()?; - assert_eq!(sync_state.sync_repo_path, repo1_path); - - // Switch to repo2 - let mut multi_state = MultiRepoState::load()?; - multi_state.active_repo = "repo2".to_string(); - multi_state.save()?; - - // Now SyncState should return repo2 - let sync_state = SyncState::load()?; - assert_eq!(sync_state.sync_repo_path, repo2_path); - - std::env::remove_var("CLAUDE_CODE_SYNC_CONFIG_DIR"); - Ok(()) -} - -/// Test error handling when active repo doesn't exist in repos map -#[test] -#[serial] -fn test_invalid_active_repo_error() -> Result<()> { - let temp_dir = setup_test_config_env()?; - std::env::set_var("CLAUDE_CODE_SYNC_CONFIG_DIR", temp_dir.path()); - - // Create config directory - let config_dir = temp_dir.path().join("claude-code-sync"); - std::fs::create_dir_all(&config_dir)?; - - // Write v2 format with invalid active_repo - let v2_state = r#"{ - "version": 2, - "active_repo": "nonexistent", - "repos": { - "work": { - "name": "work", - "sync_repo_path": "/tmp/work-repo", - "has_remote": true, - "is_cloned_repo": false - } - } - }"#; - let state_path = config_dir.join("state.json"); - std::fs::write(&state_path, v2_state)?; - - // SyncState::load() should return an error - let result = SyncState::load(); - assert!(result.is_err()); - let err = result.unwrap_err().to_string(); - assert!(err.contains("nonexistent")); - - std::env::remove_var("CLAUDE_CODE_SYNC_CONFIG_DIR"); - Ok(()) -} - -/// Test init_from_onboarding creates v2 format -#[test] -#[serial] -fn test_init_from_onboarding_creates_v2() -> Result<()> { - use claude_code_sync::sync; - - let temp_dir = setup_test_config_env()?; - std::env::set_var("CLAUDE_CODE_SYNC_CONFIG_DIR", temp_dir.path()); - - let repo_path = temp_dir.path().join("onboarding-repo"); - - // Use init_from_onboarding (what the onboarding flow uses) - sync::init_from_onboarding(&repo_path, Some("https://github.com/test/repo.git"), false)?; - - // Verify v2 format - let multi_state = MultiRepoState::load()?; - assert_eq!(multi_state.version, 2); - assert_eq!(multi_state.active_repo, "default"); - - let default_repo = multi_state.repos.get("default").unwrap(); - assert_eq!(default_repo.sync_repo_path, repo_path); - assert!(default_repo.has_remote); - assert!(!default_repo.is_cloned_repo); - assert_eq!( - default_repo.remote_url, - Some("https://github.com/test/repo.git".to_string()) - ); - - std::env::remove_var("CLAUDE_CODE_SYNC_CONFIG_DIR"); - Ok(()) -} - -/// Test that handle_repo_selector gracefully handles uninitialized state -/// (This tests the behavior when `config` is run before `init`) -#[test] -#[serial] -fn test_config_handles_uninitialized_state() -> Result<()> { - let temp_dir = setup_test_config_env()?; - std::env::set_var("CLAUDE_CODE_SYNC_CONFIG_DIR", temp_dir.path()); - - // Don't create any state file - simulate fresh install - - // MultiRepoState::load() should return an error about not initialized - let result = MultiRepoState::load(); - assert!(result.is_err()); - let err_msg = result.unwrap_err().to_string(); - assert!( - err_msg.contains("not initialized") || err_msg.contains("Run 'claude-code-sync init'"), - "Error message should mention not initialized: {}", - err_msg - ); - - std::env::remove_var("CLAUDE_CODE_SYNC_CONFIG_DIR"); - Ok(()) -} - -/// Test cloned repo flag is preserved in v2 format -#[test] -#[serial] -fn test_cloned_repo_flag_in_v2() -> Result<()> { - use claude_code_sync::sync; - - let temp_dir = setup_test_config_env()?; - std::env::set_var("CLAUDE_CODE_SYNC_CONFIG_DIR", temp_dir.path()); - - let repo_path = temp_dir.path().join("cloned-repo"); - - // Pre-create the repo to simulate it already existing (as if cloned) - scm::init(&repo_path)?; - - // Use init_from_onboarding with is_cloned=true - sync::init_from_onboarding(&repo_path, Some("https://github.com/test/repo.git"), true)?; - - // Verify is_cloned_repo is true - let multi_state = MultiRepoState::load()?; - let default_repo = multi_state.repos.get("default").unwrap(); - assert!(default_repo.is_cloned_repo); - - // Also verify through SyncState compatibility layer - let sync_state = SyncState::load()?; - assert!(sync_state.is_cloned_repo); - - std::env::remove_var("CLAUDE_CODE_SYNC_CONFIG_DIR"); - Ok(()) -} From 23c871aa78ae81d9bbbbc250a653b726ec276426 Mon Sep 17 00:00:00 2001 From: perfectra1n Date: Mon, 13 Jul 2026 13:36:31 -0700 Subject: [PATCH 5/7] fix(undo): validate restore paths before touching the filesystem `Snapshot::restore` validated a path only *after* creating it. The security check needs a canonical path, `Path::canonicalize` requires the path to exist, so the code created the parent directories and wrote an empty file first and checked second -- and then returned early on rejection, leaving both behind. A snapshot naming a path outside the home directory therefore got directories and an empty file placed there before being refused. The existing traversal test only passed because it aimed at /etc/passwd, which already exists, so the create-first branch never ran. Resolve the path without creating anything instead: reject `..` up front, walk up to the deepest ancestor that does exist, canonicalize *that* (which resolves symlinks in the prefix), and re-attach the remaining components literally. They don't exist, so they can't be symlinks, and they aren't `..` because we just checked. Only then create directories and write. Existence is probed with `symlink_metadata` rather than `exists`. `exists` follows symlinks and so reports false for a dangling one; treating a dangling symlink as merely absent would let us re-attach its name to a canonical in-base prefix, pass the check, and then have `fs::write` follow it out of the sandbox. Three tests: a rejected out-of-base path creates neither the file nor its parent directories; `..` in a non-existent tail is refused; and a dangling symlink inside the base is not followed out of it. --- src/undo/restore.rs | 228 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 204 insertions(+), 24 deletions(-) diff --git a/src/undo/restore.rs b/src/undo/restore.rs index 77beff80..4f25df63 100644 --- a/src/undo/restore.rs +++ b/src/undo/restore.rs @@ -1,9 +1,83 @@ use anyhow::{anyhow, Context, Result}; +use std::ffi::OsStr; use std::fs; -use std::path::{Path, PathBuf}; +use std::path::{Component, Path, PathBuf}; use super::snapshot::Snapshot; +/// Resolve `path` to an absolute, symlink-free location **without creating +/// anything on disk**, so the caller can check it against an allowed base before +/// writing. +/// +/// `Path::canonicalize` requires the path to already exist. The obvious way to +/// satisfy that is to create the file first and validate second — which is +/// exactly what this module used to do, meaning a snapshot naming a path outside +/// the home directory got its parent directories and an empty file created +/// before the traversal check rejected it, and the early return left them +/// behind. +/// +/// Instead: canonicalize the deepest ancestor that *does* exist — which resolves +/// any symlinks in that prefix — then re-attach the remaining components +/// literally. Those components don't exist, so they can't be symlinks, but they +/// can still be `..`, and `canonicalize` is not there to collapse them for us +/// any more. So we reject `..` outright. +/// +/// Existence is probed with `symlink_metadata`, not `exists`: `exists` follows +/// symlinks and so reports `false` for a *dangling* one. Treating a dangling +/// symlink as "not there" would let us re-attach its name lexically to a +/// canonical in-base prefix, pass the check, and then have `fs::write` follow it +/// straight out of the sandbox. +fn resolve_without_creating(path: &Path) -> Result { + // Reject `..` before doing anything else. Snapshot paths are absolute paths + // captured from real files on disk, so a `..` component is never legitimate + // — and now that we no longer call `canonicalize` on the whole path, nothing + // is left to collapse them for us. (`.` needs no handling: `Path::components` + // normalizes it away.) + if path.components().any(|c| c == Component::ParentDir) { + return Err(anyhow!( + "Security: Path traversal detected. Path {} contains '..'", + path.display() + )); + } + + let mut existing = path; + let mut tail: Vec<&OsStr> = Vec::new(); + + while existing.symlink_metadata().is_err() { + match (existing.parent(), existing.file_name()) { + (Some(parent), Some(name)) => { + tail.push(name); + existing = parent; + } + // Walked off the top without finding anything that exists: the path + // is relative, or its root is gone. Either way we can't place it. + _ => { + return Err(anyhow!( + "Cannot resolve path for restore: {}", + path.display() + )) + } + } + } + + // Canonicalizing the existing prefix resolves any symlinks in it. The tail + // components don't exist, so they cannot be symlinks, and we just proved they + // aren't `..` — so re-attaching them literally is safe. + let mut resolved = existing.canonicalize().with_context(|| { + format!( + "Failed to canonicalize {} while restoring {}", + existing.display(), + path.display() + ) + })?; + + for name in tail.iter().rev() { + resolved.push(name); + } + + Ok(resolved) +} + impl Snapshot { /// Restore files from this snapshot /// @@ -57,30 +131,19 @@ impl Snapshot { } } - // Then restore all files from the reconstructed state + // Then restore all files from the reconstructed state. + // + // Every path is resolved and checked BEFORE anything is written. Doing it + // the other way round -- creating the file so that `canonicalize` has + // something to work with, then validating -- means a rejected path has + // already had its parent directories and an empty file created by the + // time we bail. for (path_str, content) in &all_files { let path = PathBuf::from(path_str); - // Canonicalize the path to resolve any symlinks or .. components - // First ensure parent directory exists for canonicalization to work - if let Some(parent) = path.parent() { - fs::create_dir_all(parent) - .with_context(|| format!("Failed to create directory: {}", parent.display()))?; - } - - // Create the file if it doesn't exist for canonicalization - if !path.exists() { - fs::write(&path, b"").with_context(|| { - format!("Failed to create temporary file: {}", path.display()) - })?; - } - - let canonical_path = path - .canonicalize() - .with_context(|| format!("Failed to canonicalize path: {}", path.display()))?; + let target = resolve_without_creating(&path)?; - // Validate the canonical path is within the allowed base directory - if !canonical_path.starts_with(&allowed_base) { + if !target.starts_with(&allowed_base) { return Err(anyhow!( "Security: Path traversal detected. Path {} is outside allowed directory {}", path.display(), @@ -88,9 +151,13 @@ impl Snapshot { )); } - // Now write the actual content - fs::write(&canonical_path, content) - .with_context(|| format!("Failed to restore file: {}", canonical_path.display()))?; + if let Some(parent) = target.parent() { + fs::create_dir_all(parent) + .with_context(|| format!("Failed to create directory: {}", parent.display()))?; + } + + fs::write(&target, content) + .with_context(|| format!("Failed to restore file: {}", target.display()))?; } Ok(()) @@ -235,4 +302,117 @@ mod tests { assert!(file1.exists(), "file1 should exist after restore"); assert!(!file2.exists(), "file2 should be deleted after restore"); } + + #[test] + fn test_restore_rejects_traversal_without_creating_anything() { + // The regression this guards: validation used to happen *after* the file + // was created, because `canonicalize` needs the path to exist. A rejected + // path therefore still got its parent directories and an empty file + // written before the error was returned. + let temp_dir = tempdir().unwrap(); + let allowed = temp_dir.path().join("allowed"); + fs::create_dir_all(&allowed).unwrap(); + + // Outside `allowed`, and does not exist. + let outside = temp_dir + .path() + .join("outside") + .join("deep") + .join("evil.txt"); + + let mut snapshot = metadata_only_snapshot( + &Uuid::new_v4().to_string(), + OperationType::Pull, + Duration::zero(), + ); + snapshot.files.insert( + outside.to_string_lossy().to_string(), + b"malicious content".to_vec(), + ); + + let err = snapshot + .restore_with_base(Some(&allowed)) + .expect_err("restore must reject a path outside the allowed base") + .to_string(); + assert!( + err.contains("Security") || err.contains("outside"), + "unexpected error: {err}" + ); + + assert!(!outside.exists(), "the rejected file must not be created"); + assert!( + !outside.parent().unwrap().exists(), + "the rejected file's parent directories must not be created either" + ); + } + + #[test] + fn test_restore_rejects_dotdot_in_a_path_that_does_not_exist() { + // `..` in a *non-existent* tail is the case canonicalization can no + // longer collapse for us, so it has to be rejected explicitly. + let temp_dir = tempdir().unwrap(); + let allowed = temp_dir.path().join("allowed"); + fs::create_dir_all(&allowed).unwrap(); + + let escape = allowed + .join("nope") + .join("..") + .join("..") + .join("escape.txt"); + + let mut snapshot = metadata_only_snapshot( + &Uuid::new_v4().to_string(), + OperationType::Pull, + Duration::zero(), + ); + snapshot + .files + .insert(escape.to_string_lossy().to_string(), b"escaped".to_vec()); + + let err = snapshot + .restore_with_base(Some(&allowed)) + .expect_err("restore must reject '..' in an unresolvable tail") + .to_string(); + assert!(err.contains("Security"), "unexpected error: {err}"); + + assert!(!temp_dir.path().join("escape.txt").exists()); + } + + #[cfg(unix)] + #[test] + fn test_restore_does_not_follow_a_dangling_symlink_out_of_the_base() { + // A dangling symlink reports `exists() == false` but `symlink_metadata()` + // still sees it. Probing with `exists` would treat it as a plain missing + // file, re-attach its name to a canonical in-base prefix, pass the check, + // and then let `fs::write` follow it out of the sandbox. + use std::os::unix::fs::symlink; + + let temp_dir = tempdir().unwrap(); + let allowed = temp_dir.path().join("allowed"); + fs::create_dir_all(&allowed).unwrap(); + + let target_outside = temp_dir.path().join("victim.txt"); + let link = allowed.join("innocent.txt"); + symlink(&target_outside, &link).unwrap(); + assert!(!link.exists(), "symlink should be dangling"); + assert!(link.symlink_metadata().is_ok(), "but it is still an entry"); + + let mut snapshot = metadata_only_snapshot( + &Uuid::new_v4().to_string(), + OperationType::Pull, + Duration::zero(), + ); + snapshot + .files + .insert(link.to_string_lossy().to_string(), b"pwned".to_vec()); + + // Either it errors, or it writes through the link -- the second is the bug. + let _ = snapshot.restore_with_base(Some(&allowed)); + + assert!( + !target_outside.exists(), + "restore must not write through a dangling symlink to {}", + target_outside.display() + ); + } } From af83e5b902f45701dbac55001fe3fc2a7ff448e8 Mon Sep 17 00:00:00 2001 From: perfectra1n Date: Mon, 13 Jul 2026 13:36:31 -0700 Subject: [PATCH 6/7] fix(config): reject a zero max_file_size_bytes, and repair one on load `FilterConfig::validate()` checked the LFS backend and the reserved sync subdirectory but never the file size limit, so `max_file_size_bytes = 0` was accepted -- and 0 makes `should_include` reject every non-empty file, meaning sync silently does nothing at all. Strict on write, forgiving on read: - `validate()` now rejects 0, so it can never be introduced through the CLI. - `load()` repairs a 0 to the default and warns on stderr, rather than erroring. The asymmetry is deliberate. Configs in the wild already contain a 0, because until the previous commit entering a negative size at the `config` prompt wrote exactly that (the `as u64` cast saturates). Hard-failing on load would be a trap: `handle_config_interactive` opens by calling `FilterConfig::load()`, so an error there would take down the one command that can fix the value and leave hand-editing TOML as the only way out. --- src/filter.rs | 80 +++++++++++++++++++++++++++++++++++++- tests/test_config_state.rs | 29 ++++++++++++++ 2 files changed, 108 insertions(+), 1 deletion(-) diff --git a/src/filter.rs b/src/filter.rs index 0507a008..dc118353 100644 --- a/src/filter.rs +++ b/src/filter.rs @@ -108,9 +108,32 @@ impl FilterConfig { let content = fs::read_to_string(&config_path) .with_context(|| format!("Failed to read config file: {}", config_path.display()))?; - let config: FilterConfig = + let mut config: FilterConfig = toml::from_str(&content).context("Failed to parse config file")?; + // Repair rather than reject. A `max_file_size_bytes` of 0 filters out + // every file, so a user with one in their config sees sync silently do + // nothing -- and until this was fixed, entering a negative size at the + // `config` prompt wrote exactly that (the `as u64` cast saturates), so + // configs in the wild have it. + // + // Hard-failing here would be a trap: `handle_config_interactive` opens + // with `FilterConfig::load()`, so erroring would take down the one command + // that can fix the value and leave hand-editing TOML as the only way out. + // `validate()` is strict, so a bad value can never be *written*; on the + // read path we fall back to the default and say so. + if config.max_file_size_bytes == 0 { + let default = default_max_file_size(); + eprintln!( + "{} max_file_size_bytes is 0 in {}, which would exclude every file. \ + Falling back to {} MB. Run `claude-code-sync config` to set it.", + "!".yellow(), + config_path.display(), + default / (1024 * 1024), + ); + config.max_file_size_bytes = default; + } + Ok(config) } @@ -230,6 +253,12 @@ impl FilterConfig { crate::artifacts::registry::ARTIFACTS_SUBDIR ); } + if self.max_file_size_bytes == 0 { + bail!( + "max_file_size_bytes cannot be 0: every file would be filtered out \ + and nothing would ever sync" + ); + } Ok(()) } } @@ -532,6 +561,55 @@ pub fn show_config() -> Result<()> { mod tests { use super::*; + #[test] + fn test_validate_rejects_zero_max_file_size() { + // 0 bytes means `should_include` rejects everything, so sync silently + // does nothing. It must never be written to disk. + let config = FilterConfig { + max_file_size_bytes: 0, + ..Default::default() + }; + + let err = config + .validate() + .expect_err("a 0-byte size limit must not validate") + .to_string(); + assert!(err.contains("max_file_size_bytes"), "unexpected: {err}"); + } + + #[test] + fn test_validate_accepts_a_positive_max_file_size() { + let config = FilterConfig { + max_file_size_bytes: 1, + ..Default::default() + }; + assert!(config.validate().is_ok()); + } + + #[test] + fn test_zero_max_file_size_excludes_every_real_file() { + // Pins down *why* 0 is rejected, rather than asserting it in the abstract. + // The file has to actually exist: `should_include` only consults the size + // when `fs::metadata` succeeds. + let dir = tempfile::tempdir().unwrap(); + let file = dir.path().join("session.jsonl"); + fs::write(&file, b"x").unwrap(); + + assert!( + FilterConfig::default().should_include(&file), + "sanity: the default config includes this file" + ); + + let broken = FilterConfig { + max_file_size_bytes: 0, + ..Default::default() + }; + assert!( + !broken.should_include(&file), + "a 0-byte limit excludes every non-empty file — this is the damage" + ); + } + #[test] fn test_glob_match() { assert!(glob_match("*test*", "this is a test")); diff --git a/tests/test_config_state.rs b/tests/test_config_state.rs index 9142735c..926bc5d1 100644 --- a/tests/test_config_state.rs +++ b/tests/test_config_state.rs @@ -129,6 +129,35 @@ fn test_multiple_config_operations() -> Result<()> { Ok(()) } +#[test] +#[serial] +fn test_load_repairs_a_zero_max_file_size() -> Result<()> { + let env = ConfigEnv::new(); + + // A config.toml as the pre-fix prompt would have written it: entering a + // negative size saturated the `as u64` cast to 0, which excludes every file. + // Loading must not hand that straight back, or sync silently does nothing. + std::fs::write( + env.config_dir().join("config.toml"), + "max_file_size_bytes = 0\n", + )?; + + let loaded = FilterConfig::load()?; + + assert_ne!( + loaded.max_file_size_bytes, 0, + "load must repair a 0-byte limit rather than return it" + ); + + // And loading must still *succeed* — `config` is the command that fixes this, + // and it opens by calling load(), so a hard error here would lock the user out. + let file = env.join("session.jsonl"); + std::fs::write(&file, b"x")?; + assert!(loaded.should_include(&file)); + + Ok(()) +} + #[test] fn test_filter_config_with_attachments() -> Result<()> { let config = FilterConfig { From df554e79cb8088822f39cb21c42ae80a578dcb2d Mon Sep 17 00:00:00 2001 From: perfectra1n Date: Mon, 13 Jul 2026 13:42:30 -0700 Subject: [PATCH 7/7] docs(lib): fix the broken [git] link, and two other crate-doc errors The Architecture list linked to a `git` module that does not exist -- the module is `scm`, which abstracts over Git *and* Mercurial. `cargo doc` warned about the unresolved link but CI does not deny rustdoc warnings, so it stayed. Fixing it surfaced two more, both of which actively mislead: - The `///` block describing "Platform-agnostic configuration directory management ... XDG on Linux, Application Support on macOS" sat directly above `pub mod artifacts;`, so rustdoc rendered it as the documentation for `artifacts` -- while `config`, which it actually describes, had none at all. Reattached to `config`. `artifacts` documents itself in artifacts/mod.rs. - `scm`'s own doc claimed "a unified interface for Git". The whole point of the abstraction is that it isn't Git-only: src/scm/hg.rs exists and CI runs a Mercurial matrix. Also list `artifacts` and `handlers` in the Architecture section; `handlers` became public in this branch and was missing. Verified with `RUSTDOCFLAGS="-D rustdoc::broken_intra_doc_links" cargo doc`, which is stricter than the `doc` CI task -- the crate is now clean under it. --- src/lib.rs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index ebc9ceb9..e7230d3c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -23,12 +23,14 @@ //! The library is organized into modules that handle different aspects of the sync process: //! //! - Configuration and state management ([`config`], [`filter`]) -//! - Git operations ([`git`]) +//! - Source control operations, Git or Mercurial ([`scm`]) //! - Conversation parsing and analysis ([`parser`]) -//! - Conflict detection and resolution ([`conflict`]) +//! - Conflict detection and resolution ([`conflict`], [`interactive_conflict`], [`merge`]) //! - Operation tracking and undo ([`history`], [`undo`]) //! - User interface and reporting ([`onboarding`], [`report`], [`logger`]) //! - Core synchronization logic ([`sync`]) +//! - Syncing Claude Code state beyond conversations ([`artifacts`]) +//! - The command handlers behind the CLI ([`handlers`]) /// Verbosity level for command output #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -38,12 +40,14 @@ pub enum VerbosityLevel { Verbose, // Detailed output } +// `artifacts` documents itself in artifacts/mod.rs. +pub mod artifacts; + /// Platform-agnostic configuration directory management for claude-code-sync. /// /// Provides utilities for locating and managing configuration files and directories /// following platform conventions (XDG on Linux, Application Support on macOS, /// AppData on Windows). -pub mod artifacts; pub mod config; /// Conflict detection and resolution for conversation synchronization. @@ -69,9 +73,10 @@ pub mod filter; /// Source Control Management abstraction layer. /// -/// Provides a unified interface for Git using CLI commands. -/// Supports repository initialization, cloning, committing, pushing, pulling, -/// and other common SCM operations through the [`scm::Scm`] trait. +/// Provides a unified interface over Git and Mercurial, driving each through its +/// CLI. Supports repository initialization, cloning, committing, pushing, pulling, +/// and other common SCM operations through the [`scm::Scm`] trait; the backend is +/// selected by the `scm_backend` setting in [`filter::FilterConfig`]. pub mod scm; /// Operation history tracking and persistence.