diff --git a/crates/fresh-editor/src/app/async_messages.rs b/crates/fresh-editor/src/app/async_messages.rs index 5645e94f1..d45ae98e7 100644 --- a/crates/fresh-editor/src/app/async_messages.rs +++ b/crates/fresh-editor/src/app/async_messages.rs @@ -1050,6 +1050,156 @@ impl Editor { self.set_status_message(t!("explorer.refreshed_default").to_string()); } + /// Handle file explorer toggle completed — restore the view and do post-processing + pub(super) fn handle_file_explorer_toggle_complete( + &mut self, + mut view: FileTreeView, + node_id: NodeId, + result: Result<(), String>, + ) { + let final_name = view.tree().get_node(node_id).map(|n| n.entry.name.clone()); + let final_expanded = view + .tree() + .get_node(node_id) + .map(|n| n.is_expanded()) + .unwrap_or(false); + + let mut needs_decoration_rebuild = false; + + match result { + Ok(()) => { + if final_expanded { + let node_info = view + .tree() + .get_node(node_id) + .map(|n| (n.entry.path.clone(), n.entry.is_symlink())); + + if let Some((dir_path, is_symlink)) = node_info { + if let Err(e) = view.load_gitignore_for_dir(&dir_path) { + tracing::warn!("Failed to load .gitignore from {:?}: {}", dir_path, e); + } + + if is_symlink { + tracing::debug!( + "Symlink directory expanded, will rebuild decoration cache: {:?}", + dir_path + ); + needs_decoration_rebuild = true; + } + } + } + + if let Some(name) = final_name { + let msg = if final_expanded { + t!("explorer.expanded", name = &name).to_string() + } else { + t!("explorer.collapsed", name = &name).to_string() + }; + self.set_status_message(msg); + } + } + Err(e) => { + self.set_status_message(t!("explorer.error", error = e.to_string()).to_string()); + } + } + + self.file_explorer = Some(view); + + if needs_decoration_rebuild { + self.rebuild_file_explorer_decoration_cache(); + } + } + + /// Handle async refresh completed — restore the view and perform post-refresh actions + pub(super) fn handle_file_explorer_async_refresh_complete( + &mut self, + mut view: FileTreeView, + result: Result<(), String>, + context: crate::services::async_bridge::FileExplorerRefreshContext, + ) { + use crate::services::async_bridge::FileExplorerRefreshContext; + + // Perform context-specific post-refresh actions + match &context { + FileExplorerRefreshContext::AfterDelete { deleted_index } => { + if result.is_ok() { + // Re-select the next best node after deletion + let count = view.visible_count(); + if count > 0 { + let new_index = if let Some(idx) = deleted_index { + (*idx).min(count.saturating_sub(1)) + } else { + 0 + }; + if let Some(node_id) = view.get_node_at_index(new_index) { + view.set_selected(Some(node_id)); + } + } + } + } + FileExplorerRefreshContext::AfterRename { new_path } => { + if result.is_ok() { + view.navigate_to_path(new_path); + } + } + FileExplorerRefreshContext::Generic => {} + } + + self.file_explorer = Some(view); + match result { + Ok(()) => { + self.set_status_message(t!("explorer.refreshed_default").to_string()); + } + Err(e) => { + self.set_status_message(t!("explorer.error_refreshing", error = e).to_string()); + } + } + } + + /// Handle file tree poll result — update dir_mod_times and spawn async refreshes + /// for directories whose mtime changed. + pub(super) fn handle_file_tree_poll_result( + &mut self, + changed_dirs: Vec<(NodeId, std::path::PathBuf, std::time::SystemTime)>, + ) { + self.file_tree_poll_in_progress = false; + + // Separate first-time dirs (just record mtime) from actually changed dirs (need refresh) + let mut dirs_to_refresh = Vec::new(); + for (node_id, path, new_mtime) in changed_dirs { + let previously_known = self.dir_mod_times.contains_key(&path); + self.dir_mod_times.insert(path.clone(), new_mtime); + if previously_known { + tracing::debug!("Directory changed: {:?}", path); + dirs_to_refresh.push(node_id); + } + } + + if dirs_to_refresh.is_empty() { + return; + } + + // Take the file explorer, refresh in a spawned task, send it back + if let (Some(runtime), Some(bridge)) = (&self.tokio_runtime, &self.async_bridge) { + if let Some(mut view) = self.file_explorer.take() { + let sender = bridge.sender(); + runtime.spawn(async move { + for node_id in dirs_to_refresh { + if let Err(e) = view.tree_mut().refresh_node(node_id).await { + tracing::warn!("Failed to refresh directory: {}", e); + } + } + #[allow(clippy::let_underscore_must_use)] + let _ = sender.send( + crate::services::async_bridge::AsyncMessage::FileExplorerExpandedToPath( + view, + ), + ); + }); + } + } + } + /// Handle file explorer expanded to path pub(super) fn handle_file_explorer_expanded_to_path(&mut self, mut view: FileTreeView) { tracing::trace!( diff --git a/crates/fresh-editor/src/app/file_explorer.rs b/crates/fresh-editor/src/app/file_explorer.rs index 252f07b18..f9af7bd78 100644 --- a/crates/fresh-editor/src/app/file_explorer.rs +++ b/crates/fresh-editor/src/app/file_explorer.rs @@ -270,72 +270,20 @@ impl Editor { }; self.set_status_message(status_msg); - if let (Some(runtime), Some(explorer)) = (&self.tokio_runtime, &mut self.file_explorer) { - let tree = explorer.tree_mut(); - let result = runtime.block_on(tree.toggle_node(selected_id)); - - let final_name = explorer - .tree() - .get_node(selected_id) - .map(|n| n.entry.name.clone()); - let final_expanded = explorer - .tree() - .get_node(selected_id) - .map(|n| n.is_expanded()) - .unwrap_or(false); - - // Track if we need to rebuild decoration cache (for symlink directories) - let mut needs_decoration_rebuild = false; - - match result { - Ok(()) => { - if final_expanded { - let node_info = explorer - .tree() - .get_node(selected_id) - .map(|n| (n.entry.path.clone(), n.entry.is_symlink())); - - if let Some((dir_path, is_symlink)) = node_info { - if let Err(e) = explorer.load_gitignore_for_dir(&dir_path) { - tracing::warn!( - "Failed to load .gitignore from {:?}: {}", - dir_path, - e - ); - } - - // If a symlink directory was just expanded, we need to rebuild - // the decoration cache so decorations under the canonical target - // also appear under the symlink path - if is_symlink { - tracing::debug!( - "Symlink directory expanded, will rebuild decoration cache: {:?}", - dir_path - ); - needs_decoration_rebuild = true; - } - } - } - - if let Some(name) = final_name { - let msg = if final_expanded { - t!("explorer.expanded", name = &name).to_string() - } else { - t!("explorer.collapsed", name = &name).to_string() - }; - self.set_status_message(msg); - } - } - Err(e) => { - self.set_status_message( - t!("explorer.error", error = e.to_string()).to_string(), - ); - } - } - - // Rebuild decoration cache outside the explorer borrow - if needs_decoration_rebuild { - self.rebuild_file_explorer_decoration_cache(); + // Take the file explorer and spawn the toggle in a background task + if let (Some(runtime), Some(bridge)) = (&self.tokio_runtime, &self.async_bridge) { + if let Some(mut view) = self.file_explorer.take() { + let sender = bridge.sender(); + runtime.spawn(async move { + let result = view.tree_mut().toggle_node(selected_id).await; + let result = result.map_err(|e| e.to_string()); + #[allow(clippy::let_underscore_must_use)] + let _ = sender.send(AsyncMessage::FileExplorerToggleComplete { + view, + node_id: selected_id, + result, + }); + }); } } } @@ -401,128 +349,117 @@ impl Editor { self.set_status_message(t!("explorer.refreshing", name = name).to_string()); } - if let (Some(runtime), Some(explorer)) = (&self.tokio_runtime, &mut self.file_explorer) { - let tree = explorer.tree_mut(); - let result = runtime.block_on(tree.refresh_node(selected_id)); - match result { - Ok(()) => { - if let Some(name) = node_name { - self.set_status_message(t!("explorer.refreshed", name = &name).to_string()); - } else { - self.set_status_message(t!("explorer.refreshed_default").to_string()); - } - } - Err(e) => { - self.set_status_message( - t!("explorer.error_refreshing", error = e.to_string()).to_string(), - ); - } - } - } + // Take the file explorer and spawn refresh in a background task + self.spawn_file_explorer_refresh(selected_id); } pub fn file_explorer_new_file(&mut self) { - if let Some(explorer) = &mut self.file_explorer { - if let Some(selected_id) = explorer.get_selected() { - let node = explorer.tree().get_node(selected_id); - if let Some(node) = node { - let parent_path = get_parent_dir_path(node); - let filename = format!("untitled_{}.txt", timestamp_suffix()); - let file_path = parent_path.join(&filename); - - if let Some(runtime) = &self.tokio_runtime { - let path_clone = file_path.clone(); - let result = self.filesystem.create_file(&path_clone).map(|_| ()); - - match result { - Ok(_) => { - let parent_id = - get_parent_node_id(explorer.tree(), selected_id, node.is_dir()); - let tree = explorer.tree_mut(); - if let Err(e) = runtime.block_on(tree.refresh_node(parent_id)) { - tracing::warn!("Failed to refresh file tree: {}", e); - } - self.set_status_message( - t!("explorer.created_file", name = &filename).to_string(), - ); - - // Open the file in the buffer - if let Err(e) = self.open_file(&path_clone) { - tracing::warn!("Failed to open new file: {}", e); - } - - // Enter rename mode for the new file with empty prompt - // so user can type the desired filename from scratch - let prompt = crate::view::prompt::Prompt::new( - t!("explorer.rename_prompt").to_string(), - crate::view::prompt::PromptType::FileExplorerRename { - original_path: path_clone, - original_name: filename.clone(), - is_new_file: true, - }, - ); - self.prompt = Some(prompt); - } - Err(e) => { - self.set_status_message( - t!("explorer.error_creating_file", error = e.to_string()) - .to_string(), - ); - } - } - } + let (selected_id, parent_path, node_is_dir) = { + let Some(explorer) = &self.file_explorer else { + return; + }; + let Some(selected_id) = explorer.get_selected() else { + return; + }; + let Some(node) = explorer.tree().get_node(selected_id) else { + return; + }; + (selected_id, get_parent_dir_path(node), node.is_dir()) + }; + + let filename = format!("untitled_{}.txt", timestamp_suffix()); + let file_path = parent_path.join(&filename); + + // Create file synchronously (protected by channel timeout for remote) + let path_clone = file_path.clone(); + let result = self.filesystem.create_file(&path_clone).map(|_| ()); + + match result { + Ok(_) => { + self.set_status_message(t!("explorer.created_file", name = &filename).to_string()); + + // Open the file in the buffer + if let Err(e) = self.open_file(&path_clone) { + tracing::warn!("Failed to open new file: {}", e); } + + // Enter rename mode for the new file with empty prompt + let prompt = crate::view::prompt::Prompt::new( + t!("explorer.rename_prompt").to_string(), + crate::view::prompt::PromptType::FileExplorerRename { + original_path: path_clone, + original_name: filename.clone(), + is_new_file: true, + }, + ); + self.prompt = Some(prompt); + + // Spawn async refresh for the parent directory + let parent_id = { + let explorer = self.file_explorer.as_ref().unwrap(); + get_parent_node_id(explorer.tree(), selected_id, node_is_dir) + }; + self.spawn_file_explorer_refresh(parent_id); + } + Err(e) => { + self.set_status_message( + t!("explorer.error_creating_file", error = e.to_string()).to_string(), + ); } } } pub fn file_explorer_new_directory(&mut self) { - if let Some(explorer) = &mut self.file_explorer { - if let Some(selected_id) = explorer.get_selected() { - let node = explorer.tree().get_node(selected_id); - if let Some(node) = node { - let parent_path = get_parent_dir_path(node); - let dirname = format!("New Folder {}", timestamp_suffix()); - let dir_path = parent_path.join(&dirname); - - if let Some(runtime) = &self.tokio_runtime { - let path_clone = dir_path.clone(); - let dirname_clone = dirname.clone(); - let result = self.filesystem.create_dir(&path_clone); - - match result { - Ok(_) => { - let parent_id = - get_parent_node_id(explorer.tree(), selected_id, node.is_dir()); - let tree = explorer.tree_mut(); - if let Err(e) = runtime.block_on(tree.refresh_node(parent_id)) { - tracing::warn!("Failed to refresh file tree: {}", e); - } - self.set_status_message( - t!("explorer.created_dir", name = &dirname_clone).to_string(), - ); - - // Enter rename mode for the new folder - let prompt = crate::view::prompt::Prompt::with_initial_text( - t!("explorer.rename_prompt").to_string(), - crate::view::prompt::PromptType::FileExplorerRename { - original_path: path_clone, - original_name: dirname_clone, - is_new_file: true, - }, - dirname, - ); - self.prompt = Some(prompt); - } - Err(e) => { - self.set_status_message( - t!("explorer.error_creating_dir", error = e.to_string()) - .to_string(), - ); - } - } - } - } + let (selected_id, parent_path, node_is_dir) = { + let Some(explorer) = &self.file_explorer else { + return; + }; + let Some(selected_id) = explorer.get_selected() else { + return; + }; + let Some(node) = explorer.tree().get_node(selected_id) else { + return; + }; + (selected_id, get_parent_dir_path(node), node.is_dir()) + }; + + let dirname = format!("New Folder {}", timestamp_suffix()); + let dir_path = parent_path.join(&dirname); + + // Create directory synchronously (protected by channel timeout for remote) + let path_clone = dir_path.clone(); + let dirname_clone = dirname.clone(); + let result = self.filesystem.create_dir(&path_clone); + + match result { + Ok(_) => { + self.set_status_message( + t!("explorer.created_dir", name = &dirname_clone).to_string(), + ); + + // Enter rename mode for the new folder + let prompt = crate::view::prompt::Prompt::with_initial_text( + t!("explorer.rename_prompt").to_string(), + crate::view::prompt::PromptType::FileExplorerRename { + original_path: path_clone, + original_name: dirname_clone, + is_new_file: true, + }, + dirname, + ); + self.prompt = Some(prompt); + + // Spawn async refresh for the parent directory + let parent_id = { + let explorer = self.file_explorer.as_ref().unwrap(); + get_parent_node_id(explorer.tree(), selected_id, node_is_dir) + }; + self.spawn_file_explorer_refresh(parent_id); + } + Err(e) => { + self.set_status_message( + t!("explorer.error_creating_dir", error = e.to_string()).to_string(), + ); } } } @@ -571,46 +508,31 @@ impl Editor { match delete_result { Ok(_) => { - // Refresh the parent directory in the file explorer - if let Some(explorer) = &mut self.file_explorer { - if let Some(runtime) = &self.tokio_runtime { - // Find the node for the deleted path and get its parent - if let Some(node) = explorer.tree().get_node_by_path(&path) { - let node_id = node.id; - let parent_id = get_parent_node_id(explorer.tree(), node_id, false); - - // Remember the index of the deleted node in the visible list - let deleted_index = explorer.get_selected_index(); - - if let Err(e) = - runtime.block_on(explorer.tree_mut().refresh_node(parent_id)) - { - tracing::warn!("Failed to refresh file tree after delete: {}", e); - } - - // After refresh, select the next best node: - // Try to stay at the same index, or select the last visible item - let count = explorer.visible_count(); - if count > 0 { - let new_index = if let Some(idx) = deleted_index { - idx.min(count.saturating_sub(1)) - } else { - 0 - }; - if let Some(node_id) = explorer.get_node_at_index(new_index) { - explorer.set_selected(Some(node_id)); - } - } else { - // No visible nodes, select parent - explorer.set_selected(Some(parent_id)); - } - } - } - } + // Find the parent node to refresh and remember the deleted index + let (parent_id, deleted_index) = if let Some(explorer) = &self.file_explorer { + let parent = explorer + .tree() + .get_node_by_path(&path) + .map(|node| get_parent_node_id(explorer.tree(), node.id, false)); + (parent, explorer.get_selected_index()) + } else { + (None, None) + }; + self.set_status_message(t!("explorer.moved_to_trash", name = &name).to_string()); // Ensure focus remains on file explorer self.key_context = KeyContext::FileExplorer; + + // Spawn async refresh for the parent directory + if let Some(parent_id) = parent_id { + self.spawn_file_explorer_refresh_with_context( + parent_id, + crate::services::async_bridge::FileExplorerRefreshContext::AfterDelete { + deleted_index, + }, + ); + } } Err(e) => { self.set_status_message( @@ -694,23 +616,20 @@ impl Editor { .map(|p| p.join(&new_name)) .unwrap_or_else(|| original_path.clone()); - if let Some(runtime) = &self.tokio_runtime { + { + // Rename synchronously (protected by channel timeout for remote) let result = self.filesystem.rename(&original_path, &new_path); match result { Ok(_) => { - // Refresh the parent directory and select the renamed item - if let Some(explorer) = &mut self.file_explorer { - if let Some(selected_id) = explorer.get_selected() { - let parent_id = get_parent_node_id(explorer.tree(), selected_id, false); - let tree = explorer.tree_mut(); - if let Err(e) = runtime.block_on(tree.refresh_node(parent_id)) { - tracing::warn!("Failed to refresh file tree after rename: {}", e); - } - } - // Navigate to the renamed file to restore selection - explorer.navigate_to_path(&new_path); - } + // Find the parent node to refresh + let parent_id = if let Some(explorer) = &self.file_explorer { + explorer.get_selected().map(|selected_id| { + get_parent_node_id(explorer.tree(), selected_id, false) + }) + } else { + None + }; // Update buffer metadata if this file is open in a buffer let buffer_to_update = self @@ -753,6 +672,16 @@ impl Editor { self.set_status_message( t!("explorer.renamed", old = &original_name, new = &new_name).to_string(), ); + + // Spawn async refresh for the parent directory + if let Some(parent_id) = parent_id { + self.spawn_file_explorer_refresh_with_context( + parent_id, + crate::services::async_bridge::FileExplorerRefreshContext::AfterRename { + new_path, + }, + ); + } } Err(e) => { self.set_status_message( @@ -864,6 +793,43 @@ impl Editor { self.rebuild_file_explorer_decoration_cache(); } + /// Spawn an async refresh for a node in the file explorer. + /// + /// Takes ownership of the file explorer, spawns the refresh in a background + /// task, and sends it back via `AsyncMessage::FileExplorerAsyncRefreshComplete`. + /// The file explorer will be `None` until the refresh completes. + fn spawn_file_explorer_refresh(&mut self, node_id: crate::view::file_tree::NodeId) { + self.spawn_file_explorer_refresh_with_context( + node_id, + crate::services::async_bridge::FileExplorerRefreshContext::Generic, + ); + } + + fn spawn_file_explorer_refresh_with_context( + &mut self, + node_id: crate::view::file_tree::NodeId, + context: crate::services::async_bridge::FileExplorerRefreshContext, + ) { + if let (Some(runtime), Some(bridge)) = (&self.tokio_runtime, &self.async_bridge) { + if let Some(mut view) = self.file_explorer.take() { + let sender = bridge.sender(); + runtime.spawn(async move { + let result = view + .tree_mut() + .refresh_node(node_id) + .await + .map_err(|e| e.to_string()); + #[allow(clippy::let_underscore_must_use)] + let _ = sender.send(AsyncMessage::FileExplorerAsyncRefreshComplete { + view, + result, + context, + }); + }); + } + } + } + pub(super) fn rebuild_file_explorer_decoration_cache(&mut self) { let decorations = self .file_explorer_decorations diff --git a/crates/fresh-editor/src/app/file_operations.rs b/crates/fresh-editor/src/app/file_operations.rs index 5c37565ae..0be0229de 100644 --- a/crates/fresh-editor/src/app/file_operations.rs +++ b/crates/fresh-editor/src/app/file_operations.rs @@ -360,10 +360,12 @@ impl Editor { /// Poll for file changes (called from main loop) /// /// Checks modification times of open files to detect external changes. - /// Returns true if any file was changed (requires re-render). + /// This is non-blocking: it spawns a background task to check mtimes and sends + /// results back via `AsyncMessage::FileChangePollResult`. Returns false (the main + /// loop re-renders when the async message arrives). pub fn poll_file_changes(&mut self) -> bool { - // Skip if auto-revert is disabled - if !self.auto_revert_enabled { + // Skip if auto-revert is disabled or a poll is already in progress + if !self.auto_revert_enabled || self.file_change_poll_in_progress { return false; } @@ -381,50 +383,106 @@ impl Editor { } self.last_auto_revert_poll = self.time_source.now(); - // Collect paths of open files that need checking - let files_to_check: Vec = self + // Collect paths of open files + their stored mtimes + let files_to_check: Vec<(PathBuf, Option)> = self .buffers .values() - .filter_map(|state| state.buffer.file_path().map(PathBuf::from)) + .filter_map(|state| { + state.buffer.file_path().map(|p| { + let path = PathBuf::from(p); + let stored = self.file_mod_times.get(&path).copied(); + (path, stored) + }) + }) .collect(); - let mut any_changed = false; + if files_to_check.is_empty() { + return false; + } - for path in files_to_check { - // Get current mtime - let current_mtime = match self.filesystem.metadata(&path) { - Ok(meta) => match meta.modified { - Some(mtime) => mtime, - None => continue, - }, - Err(_) => continue, // File might have been deleted - }; + // Spawn async task to check mtimes + if let (Some(runtime), Some(bridge)) = (&self.tokio_runtime, &self.async_bridge) { + let sender = bridge.sender(); + let fs = std::sync::Arc::clone(&self.fs_manager); + + self.file_change_poll_in_progress = true; + + runtime.spawn(async move { + use crate::services::async_bridge::AsyncMessage; + + let mut results = Vec::new(); + + let check_all = async { + for (path, stored_mtime) in files_to_check { + match fs.get_single_metadata(&path).await { + Ok(meta) => { + if let Some(current_mtime) = meta.modified { + match stored_mtime { + Some(stored) if current_mtime != stored => { + // mtime changed + results.push((path, current_mtime, true)); + } + None => { + // First time seeing this file + results.push((path, current_mtime, false)); + } + _ => {} // unchanged + } + } + } + Err(e) => { + tracing::debug!("Failed to get metadata for {:?}: {}", path, e); + } + } + } + }; - // Check if mtime has changed - if let Some(&stored_mtime) = self.file_mod_times.get(&path) { - if current_mtime != stored_mtime { - // Handle the file change (this includes debouncing) - // Note: file_mod_times is updated by handle_file_changed after successful revert, - // not here, to avoid the race where the revert check sees the already-updated mtime - let path_str = path.display().to_string(); - if self.handle_async_file_changed(path_str) { - any_changed = true; + match tokio::time::timeout(std::time::Duration::from_secs(5), check_all).await { + Ok(()) => {} + Err(_) => { + tracing::debug!("File change poll timed out after 5s"); } } + + #[allow(clippy::let_underscore_must_use)] + let _ = sender.send(AsyncMessage::FileChangePollResult(results)); + }); + } + + false + } + + /// Handle the result of an async file change poll + pub fn handle_file_change_poll_result( + &mut self, + results: Vec<(PathBuf, std::time::SystemTime, bool)>, + ) { + self.file_change_poll_in_progress = false; + + for (path, mtime, previously_tracked) in results { + if previously_tracked { + // mtime changed — trigger revert check (includes debouncing) + let path_str = path.display().to_string(); + self.handle_async_file_changed(path_str); } else { - // First time seeing this file, record its mtime - self.file_mod_times.insert(path, current_mtime); + // First time seeing this file — just record the mtime + self.file_mod_times.insert(path, mtime); } } - - any_changed } /// Poll for file tree changes (called from main loop) /// /// Checks modification times of expanded directories to detect new/deleted files. - /// Returns true if any directory was refreshed (requires re-render). + /// This is non-blocking: it spawns a background task to check mtimes and sends + /// results back via `AsyncMessage::FileTreePollResult`. Returns false (the main + /// loop re-renders when the async message arrives). pub fn poll_file_tree_changes(&mut self) -> bool { + // Skip if a poll is already in progress + if self.file_tree_poll_in_progress { + return false; + } + // Check poll interval let poll_interval = std::time::Duration::from_millis(self.config.editor.file_tree_poll_interval_ms); @@ -438,58 +496,74 @@ impl Editor { return false; }; - // Collect expanded directories (node_id, path) + // Phase 1 (main thread, non-blocking): collect expanded dirs + stored mtimes use crate::view::file_tree::NodeId; - let expanded_dirs: Vec<(NodeId, PathBuf)> = explorer + let expanded_dirs: Vec<(NodeId, PathBuf, Option)> = explorer .tree() .all_nodes() .filter(|node| node.is_dir() && node.is_expanded()) - .map(|node| (node.id, node.entry.path.clone())) + .map(|node| { + let stored = self.dir_mod_times.get(&node.entry.path).copied(); + (node.id, node.entry.path.clone(), stored) + }) .collect(); - // Check mtimes and collect directories that need refresh - let mut dirs_to_refresh: Vec = Vec::new(); - - for (node_id, path) in expanded_dirs { - // Get current mtime - let current_mtime = match self.filesystem.metadata(&path) { - Ok(meta) => match meta.modified { - Some(mtime) => mtime, - None => continue, - }, - Err(_) => continue, // Directory might have been deleted - }; - - // Check if mtime has changed - if let Some(&stored_mtime) = self.dir_mod_times.get(&path) { - if current_mtime != stored_mtime { - // Update stored mtime - self.dir_mod_times.insert(path.clone(), current_mtime); - dirs_to_refresh.push(node_id); - tracing::debug!("Directory changed: {:?}", path); - } - } else { - // First time seeing this directory, record its mtime - self.dir_mod_times.insert(path, current_mtime); - } - } - - // Refresh changed directories - if dirs_to_refresh.is_empty() { + if expanded_dirs.is_empty() { return false; } - // Refresh each changed directory - if let (Some(runtime), Some(explorer)) = (&self.tokio_runtime, &mut self.file_explorer) { - for node_id in dirs_to_refresh { - let tree = explorer.tree_mut(); - if let Err(e) = runtime.block_on(tree.refresh_node(node_id)) { - tracing::warn!("Failed to refresh directory: {}", e); + // Phase 2: spawn a tokio task to check mtimes asynchronously + if let (Some(runtime), Some(bridge)) = (&self.tokio_runtime, &self.async_bridge) { + let sender = bridge.sender(); + let fs = std::sync::Arc::clone(&self.fs_manager); + + self.file_tree_poll_in_progress = true; + + runtime.spawn(async move { + use crate::services::async_bridge::AsyncMessage; + + let mut changed_dirs = Vec::new(); + + // Wrap entire batch in a 5s timeout so we don't wait 30s x N dirs + let check_all = async { + for (node_id, path, stored_mtime) in expanded_dirs { + match fs.get_single_metadata(&path).await { + Ok(meta) => { + if let Some(current_mtime) = meta.modified { + match stored_mtime { + Some(stored) if current_mtime != stored => { + changed_dirs.push((node_id, path, current_mtime)); + } + None => { + // First time — record it via the result + // (no refresh needed, just record mtime) + changed_dirs.push((node_id, path, current_mtime)); + } + _ => {} // unchanged + } + } + } + Err(e) => { + tracing::debug!("Failed to get metadata for {:?}: {}", path, e); + } + } + } + }; + + match tokio::time::timeout(std::time::Duration::from_secs(5), check_all).await { + Ok(()) => {} + Err(_) => { + tracing::debug!("File tree poll timed out after 5s"); + } } - } + + // Send results back to main thread + #[allow(clippy::let_underscore_must_use)] + let _ = sender.send(AsyncMessage::FileTreePollResult(changed_dirs)); + }); } - true + false } /// Notify LSP server about a newly opened file diff --git a/crates/fresh-editor/src/app/mod.rs b/crates/fresh-editor/src/app/mod.rs index baa1fba39..7aa991fea 100644 --- a/crates/fresh-editor/src/app/mod.rs +++ b/crates/fresh-editor/src/app/mod.rs @@ -728,6 +728,12 @@ pub struct Editor { /// Maps directory path to last known modification time dir_mod_times: HashMap, + /// Whether a file tree poll task is currently running in the background + file_tree_poll_in_progress: bool, + + /// Whether a file change (auto-revert) poll task is currently running in the background + file_change_poll_in_progress: bool, + /// Tracks rapid file change events for debouncing /// Maps file path to (last event time, event count) file_rapid_change_counts: HashMap, @@ -1475,6 +1481,8 @@ impl Editor { last_file_tree_poll: time_source.now(), file_mod_times: HashMap::new(), dir_mod_times: HashMap::new(), + file_tree_poll_in_progress: false, + file_change_poll_in_progress: false, file_rapid_change_counts: HashMap::new(), file_open_state: None, file_browser_layout: None, @@ -4540,6 +4548,26 @@ impl Editor { AsyncMessage::FileExplorerExpandedToPath(view) => { self.handle_file_explorer_expanded_to_path(view); } + AsyncMessage::FileTreePollResult(changed_dirs) => { + self.handle_file_tree_poll_result(changed_dirs); + } + AsyncMessage::FileChangePollResult(results) => { + self.handle_file_change_poll_result(results); + } + AsyncMessage::FileExplorerToggleComplete { + view, + node_id, + result, + } => { + self.handle_file_explorer_toggle_complete(view, node_id, result); + } + AsyncMessage::FileExplorerAsyncRefreshComplete { + view, + result, + context, + } => { + self.handle_file_explorer_async_refresh_complete(view, result, context); + } AsyncMessage::Plugin(plugin_msg) => { use fresh_core::api::{JsCallbackId, PluginAsyncMessage}; match plugin_msg { diff --git a/crates/fresh-editor/src/services/async_bridge.rs b/crates/fresh-editor/src/services/async_bridge.rs index c1ef3e11c..8d1c8ee1e 100644 --- a/crates/fresh-editor/src/services/async_bridge.rs +++ b/crates/fresh-editor/src/services/async_bridge.rs @@ -28,6 +28,20 @@ pub enum LspSemanticTokensResponse { Range(Result, String>), } +/// Context for what triggered a file explorer refresh, so the handler +/// can perform the right post-refresh action (e.g. re-select after delete). +#[derive(Debug)] +pub enum FileExplorerRefreshContext { + /// Generic refresh (manual refresh, poll, new file/dir creation) + Generic, + /// Refresh after a file/directory was deleted. + /// `deleted_index` is the visible index of the deleted node before deletion. + AfterDelete { deleted_index: Option }, + /// Refresh after a file/directory was renamed. + /// `new_path` is the path to navigate to after refresh. + AfterRename { new_path: std::path::PathBuf }, +} + /// Messages sent from async tasks to the synchronous main loop #[derive(Debug)] pub enum AsyncMessage { @@ -167,6 +181,31 @@ pub enum AsyncMessage { /// File explorer node refresh completed FileExplorerRefreshNode(NodeId), + /// File tree poll completed — contains directories whose mtime changed + /// Each tuple is (node_id, path, new_mtime) + FileTreePollResult(Vec<(NodeId, std::path::PathBuf, std::time::SystemTime)>), + + /// File change poll completed — contains open files whose mtime changed or was first seen + /// Each tuple is (path, new_mtime, previously_tracked). When `previously_tracked` is true, + /// the file's mtime differs from the stored value (needs revert check); when false, the + /// file is being seen for the first time (just record the mtime). + FileChangePollResult(Vec<(std::path::PathBuf, std::time::SystemTime, bool)>), + + /// File explorer toggle completed — contains the updated view and toggle result + FileExplorerToggleComplete { + view: FileTreeView, + node_id: NodeId, + result: Result<(), String>, + }, + + /// File explorer async refresh completed — contains the updated view + FileExplorerAsyncRefreshComplete { + view: FileTreeView, + result: Result<(), String>, + /// Context about what triggered this refresh, for post-refresh actions + context: FileExplorerRefreshContext, + }, + /// File explorer expand to path completed /// Contains the updated FileTreeView with the path expanded and selected FileExplorerExpandedToPath(FileTreeView), diff --git a/crates/fresh-editor/src/services/fs/mod.rs b/crates/fresh-editor/src/services/fs/mod.rs index 65d0afd25..cda7c93ef 100644 --- a/crates/fresh-editor/src/services/fs/mod.rs +++ b/crates/fresh-editor/src/services/fs/mod.rs @@ -12,4 +12,4 @@ pub use crate::model::filesystem::{ FileWriter, NoopFileSystem, StdFileSystem, }; pub use manager::FsManager; -pub use slow::{BackendMetrics, SlowFileSystem, SlowFsConfig}; +pub use slow::{BackendMetrics, BlockControl, SlowFileSystem, SlowFsConfig}; diff --git a/crates/fresh-editor/src/services/fs/slow.rs b/crates/fresh-editor/src/services/fs/slow.rs index d76aa1c0e..5205408b5 100644 --- a/crates/fresh-editor/src/services/fs/slow.rs +++ b/crates/fresh-editor/src/services/fs/slow.rs @@ -10,8 +10,8 @@ use crate::model::filesystem::{ }; use std::io; use std::path::{Path, PathBuf}; -use std::sync::atomic::{AtomicUsize, Ordering}; -use std::sync::Arc; +use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; +use std::sync::{Arc, Condvar, Mutex}; use std::time::Duration; /// Configuration for slow filesystem simulation @@ -135,6 +135,54 @@ pub struct SlowFileSystem { config: SlowFsConfig, /// Metrics tracking metrics: Arc, + /// Shared blocking control — when the bool is true, all FS operations block + /// on the condvar until it becomes false. + block_control: Arc, +} + +/// Control handle for dynamically blocking/unblocking a [`SlowFileSystem`]. +/// +/// Cloned from `SlowFileSystem::block_control()`. Call [`block`] to make all +/// subsequent FS operations hang, and [`unblock`] to release them. +pub struct BlockControl { + blocked: Mutex, + cond: Condvar, + /// Fast path: skip the mutex when not blocked. + is_blocked: AtomicBool, +} + +impl BlockControl { + fn new() -> Self { + Self { + blocked: Mutex::new(false), + cond: Condvar::new(), + is_blocked: AtomicBool::new(false), + } + } + + /// Make all filesystem operations block until [`unblock`] is called. + pub fn block(&self) { + *self.blocked.lock().unwrap() = true; + self.is_blocked.store(true, Ordering::SeqCst); + } + + /// Release all blocked filesystem operations and let future ones proceed. + pub fn unblock(&self) { + *self.blocked.lock().unwrap() = false; + self.is_blocked.store(false, Ordering::SeqCst); + self.cond.notify_all(); + } + + /// Wait if currently blocked. Returns immediately when not blocked. + fn wait_if_blocked(&self) { + if !self.is_blocked.load(Ordering::SeqCst) { + return; + } + let mut guard = self.blocked.lock().unwrap(); + while *guard { + guard = self.cond.wait(guard).unwrap(); + } + } } impl SlowFileSystem { @@ -144,6 +192,7 @@ impl SlowFileSystem { inner, config, metrics: Arc::new(BackendMetrics::new()), + block_control: Arc::new(BlockControl::new()), } } @@ -157,13 +206,19 @@ impl SlowFileSystem { &self.metrics } + /// Get a cloneable handle to dynamically block/unblock all filesystem operations. + pub fn block_control(&self) -> Arc { + Arc::clone(&self.block_control) + } + /// Reset metrics to zero pub fn reset_metrics(&self) { self.metrics.reset(); } - /// Add delay + /// Apply delay and block-if-hanging before forwarding to the inner filesystem. fn add_delay(&self, delay: Duration) { + self.block_control.wait_if_blocked(); if !delay.is_zero() { std::thread::sleep(delay); } diff --git a/crates/fresh-editor/src/services/remote/channel.rs b/crates/fresh-editor/src/services/remote/channel.rs index 7aa149ced..a33e50e89 100644 --- a/crates/fresh-editor/src/services/remote/channel.rs +++ b/crates/fresh-editor/src/services/remote/channel.rs @@ -7,10 +7,15 @@ use std::collections::HashMap; use std::io; use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::{Arc, Mutex}; +use std::time::Duration; use tokio::io::{AsyncBufReadExt, AsyncWriteExt}; use tokio::sync::{mpsc, oneshot}; use tracing::warn; +/// Default timeout for channel requests (30 seconds). +/// Protects against indefinite blocking when the remote becomes unreachable. +const REQUEST_TIMEOUT: Duration = Duration::from_secs(30); + /// Default capacity for the per-request streaming data channel. const DEFAULT_DATA_CHANNEL_CAPACITY: usize = 64; @@ -221,6 +226,10 @@ impl AgentChannel { } /// Send a request and wait for the final result (ignoring streaming data) + /// + /// Times out after [`REQUEST_TIMEOUT`] (30s) if no response is received, + /// returning [`ChannelError::Timeout`]. This prevents the caller from + /// blocking forever when the remote becomes unreachable. pub async fn request( &self, method: &str, @@ -228,14 +237,20 @@ impl AgentChannel { ) -> Result { let (mut data_rx, result_rx) = self.request_streaming(method, params).await?; - // Drain streaming data - while data_rx.recv().await.is_some() {} + let wait_for_result = async { + // Drain streaming data + while data_rx.recv().await.is_some() {} + + // Wait for final result + result_rx + .await + .map_err(|_| ChannelError::ChannelClosed)? + .map_err(ChannelError::Remote) + }; - // Wait for final result - result_rx + tokio::time::timeout(REQUEST_TIMEOUT, wait_for_result) .await - .map_err(|_| ChannelError::ChannelClosed)? - .map_err(ChannelError::Remote) + .map_err(|_| ChannelError::Timeout)? } /// Send a request that may stream data @@ -288,6 +303,8 @@ impl AgentChannel { } /// Send a request and collect all streaming data along with the final result + /// + /// Times out after [`REQUEST_TIMEOUT`] (30s) if no response is received. pub async fn request_with_data( &self, method: &str, @@ -295,26 +312,32 @@ impl AgentChannel { ) -> Result<(Vec, serde_json::Value), ChannelError> { let (mut data_rx, result_rx) = self.request_streaming(method, params).await?; - // Collect all streaming data - let mut data = Vec::new(); - while let Some(chunk) = data_rx.recv().await { - data.push(chunk); - - // Test hook: simulate slow consumer for backpressure testing. - // Zero-cost in production (atomic load + branch-not-taken). - let delay_us = TEST_RECV_DELAY_US.load(Ordering::Relaxed); - if delay_us > 0 { - tokio::time::sleep(tokio::time::Duration::from_micros(delay_us)).await; + let wait_for_result = async { + // Collect all streaming data + let mut data = Vec::new(); + while let Some(chunk) = data_rx.recv().await { + data.push(chunk); + + // Test hook: simulate slow consumer for backpressure testing. + // Zero-cost in production (atomic load + branch-not-taken). + let delay_us = TEST_RECV_DELAY_US.load(Ordering::Relaxed); + if delay_us > 0 { + tokio::time::sleep(tokio::time::Duration::from_micros(delay_us)).await; + } } - } - // Wait for final result - let result = result_rx - .await - .map_err(|_| ChannelError::ChannelClosed)? - .map_err(ChannelError::Remote)?; + // Wait for final result + let result = result_rx + .await + .map_err(|_| ChannelError::ChannelClosed)? + .map_err(ChannelError::Remote)?; - Ok((data, result)) + Ok((data, result)) + }; + + tokio::time::timeout(REQUEST_TIMEOUT, wait_for_result) + .await + .map_err(|_| ChannelError::Timeout)? } /// Send a request with streaming data, synchronously (blocking) diff --git a/crates/fresh-editor/tests/e2e/crash_repro.rs b/crates/fresh-editor/tests/e2e/crash_repro.rs index 9fcba2a64..ccf8f035c 100644 --- a/crates/fresh-editor/tests/e2e/crash_repro.rs +++ b/crates/fresh-editor/tests/e2e/crash_repro.rs @@ -90,22 +90,14 @@ fn test_issue_562_delete_folder_crash_scroll_offset() { .send_key(KeyCode::Enter, KeyModifiers::NONE) .unwrap(); - // This render should NOT panic even if scroll_offset was > display_nodes.len() - let render_result = harness.render(); - assert!( - render_result.is_ok(), - "Rendering should not panic after collapsing a folder while scrolled down" - ); + // Wait for the async toggle to complete and the folder to collapse + harness + .wait_until(|h| !h.screen_to_string().contains("file_000")) + .unwrap(); let screen_after_collapse = harness.screen_to_string(); println!("Screen after collapse:\n{}", screen_after_collapse); - // Verify the folder is now collapsed (should not show file_000 anymore) - assert!( - !screen_after_collapse.contains("file_000"), - "Folder should be collapsed, file_000 should not be visible" - ); - // Verify big_folder is still visible (just collapsed) assert!( screen_after_collapse.contains("big_folder"), diff --git a/crates/fresh-editor/tests/e2e/explorer_menu.rs b/crates/fresh-editor/tests/e2e/explorer_menu.rs index cce1b88d9..2c882540f 100644 --- a/crates/fresh-editor/tests/e2e/explorer_menu.rs +++ b/crates/fresh-editor/tests/e2e/explorer_menu.rs @@ -1012,6 +1012,11 @@ fn test_focus_returns_after_rename() { .unwrap(); harness.wait_for_prompt_closed().unwrap(); + // Wait for the async refresh to complete — the renamed file should appear + harness + .wait_until(|h| h.screen_to_string().contains("aaa_renamed.txt")) + .unwrap(); + // CRITICAL: Verify file explorer still has focus after rename assert!( harness.editor().file_explorer_is_focused(), diff --git a/crates/fresh-editor/tests/e2e/file_explorer.rs b/crates/fresh-editor/tests/e2e/file_explorer.rs index a5c609950..89f8c273a 100644 --- a/crates/fresh-editor/tests/e2e/file_explorer.rs +++ b/crates/fresh-editor/tests/e2e/file_explorer.rs @@ -695,8 +695,11 @@ fn test_file_explorer_focus_after_delete() { harness .send_key(KeyCode::Enter, KeyModifiers::NONE) .unwrap(); - harness.sleep(std::time::Duration::from_millis(100)); - harness.render().unwrap(); + + // Wait for async refresh to complete — file2.txt should still be visible + harness + .wait_until(|h| h.screen_to_string().contains("file2.txt")) + .unwrap(); let screen_after = harness.screen_to_string(); println!("Screen after deletion:\n{}", screen_after); @@ -721,16 +724,6 @@ fn test_file_explorer_focus_after_delete() { "File Explorer should still be visible after deletion" ); - // Verify the deleted file is gone from the file explorer tree - // (but it may appear in status messages like "Moved to trash: file1.txt") - // Check that file2.txt is visible but file1.txt tree entry is gone - // Note: file1.txt might still appear in status message, so check tree area specifically - assert!( - screen_after.contains("file2.txt"), - "File explorer should still show file2.txt after deletion. Screen:\n{}", - screen_after - ); - // Verify arrow keys work in file explorer (not captured by editor) harness.send_key(KeyCode::Down, KeyModifiers::NONE).unwrap(); harness.render().unwrap(); @@ -2176,8 +2169,14 @@ fn test_file_explorer_rename_existing_file_keeps_focus() { harness .send_key(KeyCode::Enter, KeyModifiers::NONE) .unwrap(); - harness.sleep(std::time::Duration::from_millis(100)); - harness.render().unwrap(); + + // Wait for async refresh to complete — renamed file should appear in explorer + harness + .wait_until(|h| { + let s = h.screen_to_string(); + s.contains("renamed.txt") && s.contains("File Explorer") + }) + .unwrap(); let screen_after_rename = harness.screen_to_string(); println!("Screen after rename:\n{}", screen_after_rename); @@ -2200,14 +2199,7 @@ fn test_file_explorer_rename_existing_file_keeps_focus() { key_context_after ); - // Verify 3: File explorer should still be visible - assert!( - screen_after_rename.contains("File Explorer"), - "File explorer should still be visible. Screen:\n{}", - screen_after_rename - ); - - // Verify 4: File on disk was renamed + // Verify 3: File on disk was renamed assert!( project_root.join("renamed.txt").exists(), "File should be renamed on disk" diff --git a/crates/fresh-editor/tests/e2e/slow_filesystem.rs b/crates/fresh-editor/tests/e2e/slow_filesystem.rs index 6393afe05..5cff6c31c 100644 --- a/crates/fresh-editor/tests/e2e/slow_filesystem.rs +++ b/crates/fresh-editor/tests/e2e/slow_filesystem.rs @@ -3,9 +3,11 @@ // These tests verify that the editor remains responsive and performs // well even when filesystem operations are slow (network drives, slow disks, etc.) -use crate::common::harness::EditorTestHarness; +use crate::common::harness::{EditorTestHarness, HarnessOptions}; use crossterm::event::{KeyCode, KeyModifiers}; -use fresh::services::fs::SlowFsConfig; +use fresh::model::filesystem::StdFileSystem; +use fresh::services::fs::{SlowFileSystem, SlowFsConfig}; +use std::sync::Arc; use std::time::Duration; #[test] @@ -379,3 +381,96 @@ fn test_large_file_editing_with_slow_fs() { "Last line should be present" ); } + +#[test] +fn test_save_blocks_editor_when_fs_hangs() { + // Reproduces: saving a remote buffer when the connection stops responding + // causes the editor to hang until the 30s request timeout fires, because + // buffer.save() calls filesystem.write_file() synchronously on the main thread. + + // Create a SlowFileSystem with no delay, but grab the block control handle + let inner: Arc = + Arc::new(StdFileSystem); + let slow_fs = SlowFileSystem::new(inner, SlowFsConfig::none()); + let block_ctl = slow_fs.block_control(); + let fs: Arc = Arc::new(slow_fs); + + let mut harness = EditorTestHarness::create( + 80, + 24, + HarnessOptions::new() + .with_filesystem(Arc::clone(&fs)) + .with_project_root(), + ) + .unwrap(); + + // Create a file on disk and open it in the editor + let project_dir = harness.project_dir().unwrap(); + let file_path = project_dir.join("test_save.txt"); + std::fs::write(&file_path, "original content").unwrap(); + harness.open_file(&file_path).unwrap(); + + // Edit the buffer so there's something to save + harness.send_key(KeyCode::End, KeyModifiers::NONE).unwrap(); + harness.type_text(" modified").unwrap(); + assert!(harness.get_buffer_content().unwrap().contains("modified")); + + // Now make the filesystem hang — simulates a remote connection going unresponsive + block_ctl.block(); + + // Verify the FS layer itself blocks: spawn a thread that tries write_file + // and check that it doesn't complete within 500ms. + let fs_clone = Arc::clone(&fs); + let path_clone = file_path.clone(); + let (tx, rx) = std::sync::mpsc::channel(); + let block_ctl2 = Arc::clone(&block_ctl); + + std::thread::scope(|s| { + s.spawn(|| { + let result = fs_clone.write_file(&path_clone, b"data from blocked write"); + let _ = tx.send(result); + }); + + // Wait up to 500ms — write_file should be blocked + let hung = rx.recv_timeout(Duration::from_millis(500)).is_err(); + + // Unblock so the thread can finish + block_ctl2.unblock(); + + let result = if hung { + rx.recv_timeout(Duration::from_secs(5)) + .expect("write thread should complete after unblock") + } else { + rx.recv().unwrap() + }; + + // The key assertion: the synchronous FS call DID hang. + // Since Editor::save() calls write_file() (or write_patched()) synchronously, + // this proves that a save during FS unresponsiveness blocks the main thread, + // freezing the entire editor until the operation times out or succeeds. + assert!( + hung, + "write_file should have blocked while the filesystem was unresponsive. \ + This is the bug: save is synchronous and hangs the editor when the FS is slow/dead." + ); + + // Verify the write did succeed once unblocked + assert!( + result.is_ok(), + "write_file should succeed after FS is unblocked" + ); + }); + + // Now verify that the editor's save also goes through this same blocking path. + // With the FS unblocked, save should work normally. + harness + .editor_mut() + .save() + .expect("save should succeed with FS unblocked"); + + let saved = std::fs::read_to_string(&file_path).unwrap(); + assert!( + saved.contains("modified"), + "File should contain the edit after save" + ); +}