Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions app/src/pane_group/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1333,9 +1333,8 @@ impl PaneGroup {
Self::create_ambient_agent_terminal(resources, view_size, ctx)
}
PaneMode::Terminal | PaneMode::Agent => PaneGroup::create_session(
// Use cwd from the template iff such path exists, otherwise None
// TODO(CORE-3187): On Windows, support WSL directory restoration.
Some(cwd).filter(|p| p.exists()),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we removing this filter?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Previously, Some(cwd).filter(|p| p.exists()) turned a missing directory into None, which made create_session silently fall back to home and hide the problem. Since this PR's goal is to surface missing tab-config directories (via the validation + toast), we keep Some(cwd) so the missing path is preserved and detected.

Some(cwd),
HashMap::new(),
uuid.as_bytes(),
IsSharedSessionCreator::No,
Expand Down
42 changes: 42 additions & 0 deletions app/src/tab_configs/tab_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,48 @@ impl TabConfig {
}
}

pub(crate) fn validate_load_time_tab_config_directories(config: &TabConfig) -> Result<(), String> {
let mut invalid_directories = Vec::new();

for pane in &config.panes {
let Some(directory) = pane.directory.as_deref() else {
continue;
};

if directory.is_empty() || contains_template_placeholder(directory) {
continue;
}

let expanded = shellexpand::tilde(directory).into_owned();
let path = PathBuf::from(&expanded);
if path.is_dir() {
continue;
}

let reason = if path.exists() {
"path exists but is not a directory"
} else {
"directory does not exist"
};
invalid_directories.push(format!(
"pane '{}' references invalid directory '{}': {}",
pane.id, expanded, reason
));
}

if invalid_directories.is_empty() {
Ok(())
} else {
Err(invalid_directories.join("; "))
}
}

fn contains_template_placeholder(value: &str) -> bool {
value
.split_once("{{")
.is_some_and(|(_, rest)| rest.contains("}}"))
}

/// Renders a [`TabConfig`] with the given param values into a [`PaneTemplateType`]
/// and an optional rendered title.
///
Expand Down
148 changes: 148 additions & 0 deletions app/src/user_config/mod_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,3 +174,151 @@ fn test_load_tab_configs_skips_non_toml_files() {
assert_eq!(configs.len(), 1);
assert_eq!(configs[0].name, "Real");
}

#[cfg(feature = "local_fs")]
fn write_tab_config_toml_with_directory(
dir: &Path,
file_name: &str,
config_name: &str,
directory: &str,
) {
let path = dir.join(file_name);
let toml = format!(
r#"name = "{config_name}"

[[panes]]
id = "main"
type = "terminal"
directory = "{directory}"
"#
);
std::fs::write(path, toml).unwrap();
}

#[cfg(feature = "local_fs")]
#[test]
fn test_load_tab_configs_rejects_nonexistent_literal_directory() {
let dir = tempfile::tempdir().unwrap();
let nonexistent_dir = dir.path().join("does-not-exist");
write_tab_config_toml_with_directory(
dir.path(),
"invalid.toml",
"Invalid",
&nonexistent_dir.display().to_string(),
);

let (configs, errors) = load_tab_configs(dir.path());

assert!(configs.is_empty());
assert_eq!(errors.len(), 1);
assert_eq!(errors[0].file_name, "invalid.toml");
assert!(errors[0].file_path.ends_with("invalid.toml"));
assert!(errors[0].error_message.contains("pane 'main'"));
assert!(errors[0].error_message.contains("does-not-exist"));
assert!(errors[0].error_message.contains("directory does not exist"));
}

#[cfg(feature = "local_fs")]
#[test]
fn test_load_tab_configs_rejects_literal_directory_that_is_file() {
let dir = tempfile::tempdir().unwrap();
let file_path = dir.path().join("not-a-directory");
std::fs::write(&file_path, "not a directory").unwrap();
write_tab_config_toml_with_directory(
dir.path(),
"invalid.toml",
"Invalid",
&file_path.display().to_string(),
);

let (configs, errors) = load_tab_configs(dir.path());

assert!(configs.is_empty());
assert_eq!(errors.len(), 1);
assert!(errors[0].error_message.contains("pane 'main'"));
assert!(errors[0].error_message.contains("not-a-directory"));
assert!(
errors[0]
.error_message
.contains("path exists but is not a directory")
);
}

#[cfg(feature = "local_fs")]
#[test]
fn test_load_tab_configs_accepts_existing_literal_directory() {
let dir = tempfile::tempdir().unwrap();
let existing_dir = tempfile::tempdir().unwrap();
write_tab_config_toml_with_directory(
dir.path(),
"valid.toml",
"Valid",
&existing_dir.path().display().to_string(),
);

let (configs, errors) = load_tab_configs(dir.path());

assert!(errors.is_empty());
assert_eq!(configs.len(), 1);
assert_eq!(configs[0].name, "Valid");
}

#[cfg(feature = "local_fs")]
#[test]
fn test_load_tab_configs_expands_tilde_in_literal_directory() {
if !dirs::home_dir().is_some_and(|home_dir| home_dir.is_dir()) {
return;
}
let dir = tempfile::tempdir().unwrap();
write_tab_config_toml_with_directory(dir.path(), "valid.toml", "Valid", "~");

let (configs, errors) = load_tab_configs(dir.path());

assert!(errors.is_empty());
assert_eq!(configs.len(), 1);
assert_eq!(configs[0].name, "Valid");
}

#[cfg(feature = "local_fs")]
#[test]
fn test_load_tab_configs_allows_empty_directory() {
let dir = tempfile::tempdir().unwrap();
write_tab_config_toml_with_directory(dir.path(), "valid.toml", "Valid", "");

let (configs, errors) = load_tab_configs(dir.path());

assert!(errors.is_empty());
assert_eq!(configs.len(), 1);
assert_eq!(configs[0].name, "Valid");
}

#[cfg(feature = "local_fs")]
#[test]
fn test_load_tab_configs_allows_missing_directory() {
let dir = tempfile::tempdir().unwrap();
write_tab_config_toml(dir.path(), "valid.toml", "Valid");

let (configs, errors) = load_tab_configs(dir.path());

assert!(errors.is_empty());
assert_eq!(configs.len(), 1);
assert_eq!(configs[0].name, "Valid");
}

#[cfg(feature = "local_fs")]
#[test]
fn test_load_tab_configs_allows_parameterized_directory() {
let dir = tempfile::tempdir().unwrap();
write_tab_config_toml_with_directory(
dir.path(),
"parameterized.toml",
"Parameterized",
"{{repo}}/does-not-need-to-exist-yet",
);

let (configs, errors) = load_tab_configs(dir.path());

assert!(errors.is_empty());
assert_eq!(configs.len(), 1);
assert_eq!(configs[0].name, "Parameterized");
}
6 changes: 6 additions & 0 deletions app/src/user_config/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use serde::de::DeserializeOwned;
use walkdir::{DirEntry, WalkDir};

use crate::launch_configs::launch_config::LaunchConfig;
use crate::tab_configs::tab_config::validate_load_time_tab_config_directories;
use crate::tab_configs::{TabConfig, TabConfigError};
use crate::themes::theme::{ThemeKind, WarpTheme, WarpThemeConfig};
use crate::workflows::workflow::Workflow;
Expand Down Expand Up @@ -178,6 +179,11 @@ pub(super) fn parse_tab_config_dir_entry(
config.source_path = Some(item.path().into());
config
})
.and_then(|config| {
validate_load_time_tab_config_directories(&config)
.map_err(anyhow::Error::msg)
.map(|()| config)
})
.map_err(|e| TabConfigError {
file_name,
file_path: item.path().into(),
Expand Down
5 changes: 5 additions & 0 deletions app/src/view_components/dismissible_toast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,11 @@ impl<A: Action + Clone> DismissibleToastStack<A> {
pub fn has_toasts(&self) -> bool {
!self.toasts.is_empty()
}

#[cfg(test)]
pub fn toast_count(&self) -> usize {
self.toasts.len()
}
}

impl<A: Action + Clone> View for DismissibleToastStack<A> {
Expand Down
54 changes: 53 additions & 1 deletion app/src/workspace/view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ use crate::editor::{
use crate::env_vars::manager::{EnvVarCollectionManager, EnvVarCollectionSource};
use crate::env_vars::CloudEnvVarCollection;
use crate::experiments::{BlockOnboarding, Experiment};
use crate::launch_configs::launch_config::WindowTemplate;
use crate::launch_configs::launch_config::{PaneTemplateType, WindowTemplate};
use crate::launch_configs::save_modal::{LaunchConfigModalEvent, LaunchConfigSaveModal};
use crate::menu::{Event as MenuEvent, Menu, MenuItem, MenuItemFields, MenuSelectionSource};
use crate::modal::{Modal, ModalEvent, ModalViewState};
Expand Down Expand Up @@ -6639,6 +6639,12 @@ impl Workspace {
let tab_color = tab_config.color;
let (rendered_title, pane_template) =
crate::tab_configs::render_tab_config(&tab_config, &param_values, worktree_branch_name);

if let Some(invalid_cwd) = find_invalid_tab_config_cwd(&pane_template) {
self.show_invalid_tab_config_cwd_toast(&tab_config, invalid_cwd, ctx);
return;
}

self.add_tab_with_pane_layout(
PanesLayout::Template(pane_template),
Arc::new(HashMap::new()),
Expand All @@ -6653,6 +6659,37 @@ impl Workspace {
}
}

fn show_invalid_tab_config_cwd_toast(
&mut self,
tab_config: &crate::tab_configs::TabConfig,
invalid_cwd: &Path,
ctx: &mut ViewContext<Self>,
) {
let message = format!(
"Tab config '{}' references a directory that does not exist or is not a directory: {}",
tab_config.name,
invalid_cwd.display()
);

let object_id = tab_config.source_path.as_ref().map_or_else(
|| format!("tab_config_error:{}", tab_config.name),
|path| format!("tab_config_error:{}", path.display()),
);
let mut toast = DismissibleToast::error(message).with_object_id(object_id.clone());
if let Some(path) = &tab_config.source_path {
toast = toast.with_link(ToastLink::new("Open file".to_string()).with_onclick_action(
WorkspaceAction::OpenTabConfigErrorFile {
path: path.clone(),
toast_object_id: object_id,
},
));
}

self.toast_stack.update(ctx, |toast_stack, ctx| {
toast_stack.add_persistent_toast(toast, ctx);
});
}

/// Opens a tab config, showing the param-fill modal when the config has parameters,
/// or opening the tab directly when there are no parameters.
pub(crate) fn open_tab_config(
Expand Down Expand Up @@ -22568,6 +22605,21 @@ impl Workspace {
}
}

fn find_invalid_tab_config_cwd(pane_template: &PaneTemplateType) -> Option<&Path> {
match pane_template {
PaneTemplateType::PaneTemplate { cwd, .. } => {
if cwd.as_os_str().is_empty() || cwd.is_dir() {
None
} else {
Some(cwd.as_path())
}
}
PaneTemplateType::PaneBranchTemplate { panes, .. } => {
panes.iter().find_map(find_invalid_tab_config_cwd)
}
}
}

impl Entity for Workspace {
type Event = ();
}
Expand Down
Loading