refactor: improve checkpoint system reliability and error handling (#3041)

- Add work_tree_path field to CheckpointManager for explicit work tree tracking
- Refactor run_git function to use Option<&Path> for cleaner work tree handling
- Improve error messages with more user-friendly descriptions
- Add debug logging for git operations to aid troubleshooting
- Fix checkpoint state management in conversation restoration
- Update experiment UI formatting for better readability
- Add missing fields to test mocks for rmcp::model::Prompt
- Clean up imports and formatting in reply.rs

Author: kkashilk

crates/chat-cli/src/cli/chat/checkpoint.rs

@@ -25,6 +25,7 @@ use serde::{
     Deserialize,
     Serialize,
 };
+use tracing::debug;
 
 use crate::cli::ConversationState;
 use crate::cli::chat::conversation::HistoryEntry;
@@ -36,6 +37,9 @@ pub struct CheckpointManager {
     /// Path to the shadow (bare) git repository
     pub shadow_repo_path: PathBuf,
 
+    /// Path to current working directory
+    pub work_tree_path: PathBuf,
+
     /// All checkpoints in chronological order
     pub checkpoints: Vec<Checkpoint>,
 
@@ -84,10 +88,10 @@ impl CheckpointManager {
         current_history: &VecDeque<HistoryEntry>,
     ) -> Result<Self> {
         if !is_git_installed() {
-            bail!("Git is not installed. Checkpoints require git to function.");
+            bail!("Checkpoints are not available. Git is required but not installed.");
         }
         if !is_in_git_repo() {
-            bail!("Not in a git repository. Use '/checkpoint init' to manually enable checkpoints.");
+            bail!("Checkpoints are not available in this directory. Use '/checkpoint init' to enable checkpoints.");
         }
 
         let manager = Self::manual_init(os, shadow_path, current_history).await?;
@@ -103,14 +107,17 @@ impl CheckpointManager {
         let path = path.as_ref();
         os.fs.create_dir_all(path).await?;
 
+        let work_tree_path =
+            std::env::current_dir().map_err(|e| eyre!("Failed to get current working directory: {}", e))?;
+
         // Initialize bare repository
-        run_git(path, false, &["init", "--bare", &path.to_string_lossy()])?;
+        run_git(path, None, &["init", "--bare", &path.to_string_lossy()])?;
 
         // Configure git
         configure_git(&path.to_string_lossy())?;
 
         // Create initial checkpoint
-        stage_commit_tag(&path.to_string_lossy(), "Initial state", "0")?;
+        stage_commit_tag(&path.to_string_lossy(), &work_tree_path, "Initial state", "0")?;
 
         let initial_checkpoint = Checkpoint {
             tag: "0".to_string(),
@@ -126,6 +133,7 @@ impl CheckpointManager {
 
         Ok(Self {
             shadow_repo_path: path.to_path_buf(),
+            work_tree_path,
             checkpoints: vec![initial_checkpoint],
             tag_index,
             current_turn: 0,
@@ -146,7 +154,12 @@ impl CheckpointManager {
         tool_name: Option<String>,
     ) -> Result<()> {
         // Stage, commit and tag
-        stage_commit_tag(&self.shadow_repo_path.to_string_lossy(), description, tag)?;
+        stage_commit_tag(
+            &self.shadow_repo_path.to_string_lossy(),
+            &self.work_tree_path,
+            description,
+            tag,
+        )?;
 
         // Record checkpoint metadata
         let checkpoint = Checkpoint {
@@ -175,9 +188,14 @@ impl CheckpointManager {
 
         if hard {
             // Hard: reset the whole work-tree to the tag
-            let output = run_git(&self.shadow_repo_path, true, &["reset", "--hard", tag])?;
+            let output = run_git(&self.shadow_repo_path, Some(&self.work_tree_path), &[
+                "reset", "--hard", tag,
+            ])?;
             if !output.status.success() {
-                bail!("Failed to restore: {}", String::from_utf8_lossy(&output.stderr));
+                bail!(
+                    "Failed to restore checkpoint: {}",
+                    String::from_utf8_lossy(&output.stderr)
+                );
             }
         } else {
             // Soft: only restore tracked files. If the tag is an empty tree, this is a no-op.
@@ -187,9 +205,14 @@ impl CheckpointManager {
                 return Ok(());
             }
             // Use checkout against work-tree
-            let output = run_git(&self.shadow_repo_path, true, &["checkout", tag, "--", "."])?;
+            let output = run_git(&self.shadow_repo_path, Some(&self.work_tree_path), &[
+                "checkout", tag, "--", ".",
+            ])?;
             if !output.status.success() {
-                bail!("Failed to restore: {}", String::from_utf8_lossy(&output.stderr));
+                bail!(
+                    "Failed to restore checkpoint: {}",
+                    String::from_utf8_lossy(&output.stderr)
+                );
             }
         }
 
@@ -205,7 +228,7 @@ impl CheckpointManager {
         let out = run_git(
             &self.shadow_repo_path,
             // work_tree
-            false,
+            None,
             &["ls-tree", "-r", "--name-only", tag],
         )?;
         Ok(!out.stdout.is_empty())
@@ -223,7 +246,7 @@ impl CheckpointManager {
 
     /// Compute file statistics between two checkpoints
     pub fn compute_stats_between(&self, from: &str, to: &str) -> Result<FileStats> {
-        let output = run_git(&self.shadow_repo_path, false, &["diff", "--name-status", from, to])?;
+        let output = run_git(&self.shadow_repo_path, None, &["diff", "--name-status", from, to])?;
 
         let mut stats = FileStats::default();
         for line in String::from_utf8_lossy(&output.stdout).lines() {
@@ -246,7 +269,7 @@ impl CheckpointManager {
         let mut result = String::new();
 
         // Get file changes
-        let output = run_git(&self.shadow_repo_path, false, &["diff", "--name-status", from, to])?;
+        let output = run_git(&self.shadow_repo_path, None, &["diff", "--name-status", from, to])?;
 
         for line in String::from_utf8_lossy(&output.stdout).lines() {
             if let Some((status, file)) = line.split_once('\t') {
@@ -261,7 +284,7 @@ impl CheckpointManager {
         }
 
         // Add statistics
-        let stat_output = run_git(&self.shadow_repo_path, false, &[
+        let stat_output = run_git(&self.shadow_repo_path, None, &[
             "diff",
             from,
             to,
@@ -279,7 +302,10 @@ impl CheckpointManager {
 
     /// Check for uncommitted changes
     pub fn has_changes(&self) -> Result<bool> {
-        let output = run_git(&self.shadow_repo_path, true, &["status", "--porcelain"])?;
+        let output = run_git(&self.shadow_repo_path, Some(&self.work_tree_path), &[
+            "status",
+            "--porcelain",
+        ])?;
         Ok(!output.stdout.is_empty())
     }
 
@@ -351,18 +377,18 @@ fn is_in_git_repo() -> bool {
 }
 
 fn configure_git(shadow_path: &str) -> Result<()> {
-    run_git(Path::new(shadow_path), false, &["config", "user.name", "Q"])?;
-    run_git(Path::new(shadow_path), false, &["config", "user.email", "qcli@local"])?;
-    run_git(Path::new(shadow_path), false, &["config", "core.preloadindex", "true"])?;
+    run_git(Path::new(shadow_path), None, &["config", "user.name", "Q"])?;
+    run_git(Path::new(shadow_path), None, &["config", "user.email", "qcli@local"])?;
+    run_git(Path::new(shadow_path), None, &["config", "core.preloadindex", "true"])?;
     Ok(())
 }
 
-fn stage_commit_tag(shadow_path: &str, message: &str, tag: &str) -> Result<()> {
+fn stage_commit_tag(shadow_path: &str, work_tree: &Path, message: &str, tag: &str) -> Result<()> {
     // Stage all changes
-    run_git(Path::new(shadow_path), true, &["add", "-A"])?;
+    run_git(Path::new(shadow_path), Some(work_tree), &["add", "-A"])?;
 
     // Commit
-    let output = run_git(Path::new(shadow_path), true, &[
+    let output = run_git(Path::new(shadow_path), Some(work_tree), &[
         "commit",
         "--allow-empty",
         "--no-verify",
@@ -371,33 +397,53 @@ fn stage_commit_tag(shadow_path: &str, message: &str, tag: &str) -> Result<()> {
     ])?;
 
     if !output.status.success() {
-        bail!("Git commit failed: {}", String::from_utf8_lossy(&output.stderr));
+        bail!(
+            "Checkpoint initialization failed: {}",
+            String::from_utf8_lossy(&output.stderr)
+        );
     }
 
     // Tag
-    let output = run_git(Path::new(shadow_path), false, &["tag", tag])?;
+    let output = run_git(Path::new(shadow_path), None, &["tag", tag])?;
     if !output.status.success() {
-        bail!("Git tag failed: {}", String::from_utf8_lossy(&output.stderr));
+        bail!(
+            "Checkpoint initialization failed: {}",
+            String::from_utf8_lossy(&output.stderr)
+        );
     }
 
     Ok(())
 }
 
-fn run_git(dir: &Path, with_work_tree: bool, args: &[&str]) -> Result<Output> {
+fn run_git(dir: &Path, work_tree: Option<&Path>, args: &[&str]) -> Result<Output> {
     let mut cmd = Command::new("git");
     cmd.arg(format!("--git-dir={}", dir.display()));
 
-    if with_work_tree {
-        cmd.arg("--work-tree=.");
+    if let Some(work_tree_path) = work_tree {
+        cmd.arg(format!("--work-tree={}", work_tree_path.display()));
     }
 
     cmd.args(args);
 
+    debug!("Executing git command: {:?}", cmd);
     let output = cmd.output()?;
-    if !output.status.success() && !output.stderr.is_empty() {
-        bail!(String::from_utf8_lossy(&output.stderr).to_string());
+
+    if !output.status.success() {
+        debug!("Git command failed with exit code: {:?}", output.status.code());
+        debug!("Git stderr: {}", String::from_utf8_lossy(&output.stderr));
+        debug!("Git stdout: {}", String::from_utf8_lossy(&output.stdout));
+
+        if !output.stderr.is_empty() {
+            bail!(
+                "Checkpoint operation failed: {}",
+                String::from_utf8_lossy(&output.stderr)
+            );
+        } else {
+            bail!("Checkpoint operation failed unexpectedly");
+        }
     }
 
+    debug!("Git command succeeded");
     Ok(output)
 }
 

crates/chat-cli/src/cli/chat/cli/checkpoint.rs

@@ -107,7 +107,7 @@ impl CheckpointSubcommand {
                 session.stderr,
                 style::SetForegroundColor(Color::Yellow),
                 style::Print(
-                    "⚠️ Checkpoint is disabled while in tangent mode. Disable tangent mode with: q settings -d chat.enableTangentMode.\n\n"
+                    "⚠️ Checkpoint is disabled while in tangent mode. Please exit tangent mode if you want to use checkpoint.\n\n"
                 ),
                 style::SetForegroundColor(Color::Reset),
             )?;

crates/chat-cli/src/cli/chat/cli/experiment.rs

@@ -51,11 +51,7 @@ static AVAILABLE_EXPERIMENTS: &[Experiment] = &[
     },
     Experiment {
         name: "Checkpoint",
-        description: concat!(
-            "Enables workspace checkpoints to snapshot, list, expand, diff, and restore files (/checkpoint)\n",
-            "                             ",
-            "Cannot be used in tangent mode (to avoid mixing up conversation history)"
-        ),
+        description: "Enables workspace checkpoints to snapshot, list, expand, diff, and restore files (/checkpoint)\nNote: Cannot be used in tangent mode (to avoid mixing up conversation history)",
         setting_key: Setting::EnabledCheckpoint,
     },
     Experiment {
@@ -84,17 +80,21 @@ async fn select_experiment(os: &mut Os, session: &mut ChatSession) -> Result<Opt
         let is_enabled = os.database.settings.get_bool(experiment.setting_key).unwrap_or(false);
 
         current_states.push(is_enabled);
-        // Create clean single-line format: "Knowledge    [ON]   - Description"
+
         let status_indicator = if is_enabled {
-            style::Stylize::green("[ON] ")
+            format!("{}", style::Stylize::green("[ON] "))
         } else {
-            style::Stylize::grey("[OFF]")
+            format!("{}", style::Stylize::grey("[OFF]"))
         };
+
+        // Handle multi-line descriptions with proper indentation
+        let description = experiment.description.replace('\n', &format!("\n{}", " ".repeat(34)));
+
         let label = format!(
-            "{:<18} {} - {}",
+            "{:<25} {:<6} {}",
             experiment.name,
             status_indicator,
-            style::Stylize::dark_grey(experiment.description)
+            style::Stylize::dark_grey(description)
         );
         experiment_labels.push(label);
     }

crates/chat-cli/src/cli/chat/cli/mod.rs

@@ -108,7 +108,11 @@ pub enum SlashCommand {
     Persist(PersistSubcommand),
     // #[command(flatten)]
     // Root(RootSubcommand),
-    #[command(subcommand)]
+    #[command(
+        about = "(Beta) Manage workspace checkpoints (init, list, restore, expand, diff, clean)\nExperimental features may be changed or removed at any time",
+        hide = true,
+        subcommand
+    )]
     Checkpoint(CheckpointSubcommand),
     /// View, manage, and resume to-do lists
     #[command(subcommand)]

crates/chat-cli/src/cli/chat/cli/prompts.rs

@@ -2462,19 +2462,19 @@ mod tests {
         let prompt1 = rmcp::model::Prompt {
             name: "test_prompt".to_string(),
             description: Some("Test description".to_string()),
-            title: None,
+            title: Some("Test Prompt".to_string()),
             icons: None,
             arguments: Some(vec![
                 PromptArgument {
                     name: "arg1".to_string(),
                     description: Some("First argument".to_string()),
-                    title: None,
+                    title: Some("Argument 1".to_string()),
                     required: Some(true),
                 },
                 PromptArgument {
                     name: "arg2".to_string(),
+                    title: Some("Argument 2".to_string()),
                     description: None,
-                    title: None,
                     required: Some(false),
                 },
             ]),

crates/chat-cli/src/cli/chat/conversation.rs

@@ -253,6 +253,10 @@ impl ConversationState {
         self.transcript = checkpoint.main_transcript;
         self.latest_summary = checkpoint.main_latest_summary;
         self.valid_history_range = (0, self.history.len());
+        if let Some(manager) = self.checkpoint_manager.as_mut() {
+            manager.message_locked = false;
+            manager.pending_user_message = None;
+        }
     }
 
     /// Enter tangent mode - creates checkpoint of current state

crates/chat-cli/src/cli/chat/mod.rs

@@ -3030,11 +3030,11 @@ impl ChatSession {
 
                         // Reset for next turn
                         manager.tools_in_turn = 0;
-                        manager.message_locked = false; // Unlock for next turn
                     } else {
                         // Clear pending message even if no tools were used
                         manager.pending_user_message = None;
                     }
+                    manager.message_locked = false; // Unlock for next turn
 
                     // Put manager back
                     self.conversation.checkpoint_manager = Some(manager);

crates/chat-cli/src/cli/chat/tool_manager.rs

@@ -2112,8 +2112,8 @@ mod tests {
         // Create mock prompt bundles
         let prompt = rmcp::model::Prompt {
             name: "test_prompt".to_string(),
+            title: Some("Test Prompt".to_string()),
             description: Some("Test description".to_string()),
-            title: None,
             icons: None,
             arguments: None,
         };