From cb3a82621f263eed0a80cd744d33c4346da7c4f4 Mon Sep 17 00:00:00 2001 From: Jamie Sunderland Date: Mon, 23 Mar 2026 17:58:31 -0700 Subject: [PATCH] [jrs] make glob paths use static root for bind mounting --- coast-core/src/coastfile/mod.rs | 59 ++++++++------- coast-core/src/coastfile/tests_parsing.rs | 77 +++++++++++++++----- coast-daemon/src/handlers/assign/services.rs | 28 +++++++ docs/coastfiles/WORKTREE_DIR.md | 6 +- 4 files changed, 120 insertions(+), 50 deletions(-) diff --git a/coast-core/src/coastfile/mod.rs b/coast-core/src/coastfile/mod.rs index 3e15120..8a8bd2e 100644 --- a/coast-core/src/coastfile/mod.rs +++ b/coast-core/src/coastfile/mod.rs @@ -768,18 +768,39 @@ impl Coastfile { dir.contains('*') || dir.contains('?') || dir.contains('[') } - /// Resolve all external worktree dirs, expanding glob patterns. + /// Extract the path prefix before the first component containing a glob + /// metacharacter. This is the deepest directory that is guaranteed to exist + /// regardless of which subdirectories match the pattern. + /// + /// ```text + /// /home/user/.shep/repos/*/wt → /home/user/.shep/repos + /// /foo/ba?/baz → /foo + /// /a/b/[abc]/c → /a/b + /// ``` + pub fn glob_root(resolved: &str) -> PathBuf { + let path = Path::new(resolved); + let mut root = PathBuf::new(); + for component in path.components() { + let s = component.as_os_str().to_string_lossy(); + if s.contains('*') || s.contains('?') || s.contains('[') { + break; + } + root.push(component); + } + root + } + + /// Resolve all external worktree dirs. /// /// Non-glob entries keep their original `worktree_dirs` index as the mount - /// index (backward compatible). For glob entries the first match reuses the - /// original index; additional matches are allocated sequentially starting - /// from `worktree_dirs.len()`. + /// index (backward compatible). Glob entries resolve to the **glob root** + /// (the path prefix before the first wildcard component) so the bind mount + /// covers all current *and future* matches without container recreation. pub fn resolve_external_worktree_dirs_expanded( worktree_dirs: &[String], project_root: &Path, ) -> Vec { let mut results = Vec::new(); - let mut overflow_index = worktree_dirs.len(); for (idx, dir) in worktree_dirs.iter().enumerate() { if !Self::is_external_worktree_dir(dir) { @@ -789,28 +810,12 @@ impl Coastfile { let resolved_str = resolved.to_string_lossy().to_string(); if Self::is_glob_pattern(&resolved_str) { - let mut matches: Vec = glob::glob(&resolved_str) - .into_iter() - .flatten() - .filter_map(std::result::Result::ok) - .filter(|p| p.is_dir()) - .collect(); - matches.sort(); - - for (i, matched_path) in matches.into_iter().enumerate() { - let mount_index = if i == 0 { - idx - } else { - let mi = overflow_index; - overflow_index += 1; - mi - }; - results.push(ResolvedExternalDir { - mount_index, - raw_pattern: dir.clone(), - resolved_path: matched_path, - }); - } + let root = Self::glob_root(&resolved_str); + results.push(ResolvedExternalDir { + mount_index: idx, + raw_pattern: dir.clone(), + resolved_path: root, + }); } else { results.push(ResolvedExternalDir { mount_index: idx, diff --git a/coast-core/src/coastfile/tests_parsing.rs b/coast-core/src/coastfile/tests_parsing.rs index 64f242d..b6ed110 100644 --- a/coast-core/src/coastfile/tests_parsing.rs +++ b/coast-core/src/coastfile/tests_parsing.rs @@ -1708,6 +1708,28 @@ fn test_is_glob_pattern() { assert!(!Coastfile::is_glob_pattern("/absolute/path")); } +#[test] +fn test_glob_root_computation() { + use std::path::Path; + + assert_eq!( + Coastfile::glob_root("/home/user/.shep/repos/*/wt"), + Path::new("/home/user/.shep/repos") + ); + assert_eq!(Coastfile::glob_root("/foo/ba?/baz"), Path::new("/foo")); + assert_eq!(Coastfile::glob_root("/a/b/[abc]/c"), Path::new("/a/b")); + assert_eq!( + Coastfile::glob_root("/no/globs/here"), + Path::new("/no/globs/here"), + "path without globs returns the full path" + ); + assert_eq!( + Coastfile::glob_root("/*/everything/is/glob"), + Path::new("/"), + "glob in first component returns root" + ); +} + #[test] fn test_resolve_external_worktree_dirs_expanded_no_globs() { let dir = tempfile::tempdir().unwrap(); @@ -1719,38 +1741,44 @@ fn test_resolve_external_worktree_dirs_expanded_no_globs() { } #[test] -fn test_resolve_external_worktree_dirs_expanded_glob_with_matches() { +fn test_resolve_external_worktree_dirs_expanded_glob_returns_root() { let dir = tempfile::tempdir().unwrap(); let ext = dir.path().join("ext"); std::fs::create_dir_all(ext.join("aaa").join("wt")).unwrap(); std::fs::create_dir_all(ext.join("bbb").join("wt")).unwrap(); - std::fs::create_dir_all(ext.join("ccc")).unwrap(); // no "wt" subdir + std::fs::create_dir_all(ext.join("ccc")).unwrap(); let pattern = format!("{}/*/wt", ext.display()); let dirs = vec![".worktrees".to_string(), pattern.clone()]; let result = Coastfile::resolve_external_worktree_dirs_expanded(&dirs, dir.path()); - assert_eq!(result.len(), 2, "should match aaa/wt and bbb/wt"); assert_eq!( - result[0].mount_index, 1, - "first match reuses original index" + result.len(), + 1, + "glob should produce a single entry for the root" ); + assert_eq!(result[0].mount_index, 1, "reuses original index"); assert_eq!( - result[1].mount_index, 2, - "second match overflows to dirs.len()" + result[0].resolved_path, ext, + "resolved_path should be the glob root" ); - assert!(result[0].resolved_path.ends_with("aaa/wt")); - assert!(result[1].resolved_path.ends_with("bbb/wt")); assert_eq!(result[0].raw_pattern, pattern); } #[test] -fn test_resolve_external_worktree_dirs_expanded_glob_no_matches() { +fn test_resolve_external_worktree_dirs_expanded_glob_no_matches_still_returns_root() { let dir = tempfile::tempdir().unwrap(); - let pattern = format!("{}/nonexistent/*/wt", dir.path().display()); + let base = dir.path().join("nonexistent"); + let pattern = format!("{}/*/wt", base.display()); let dirs = vec![".worktrees".to_string(), pattern]; let result = Coastfile::resolve_external_worktree_dirs_expanded(&dirs, dir.path()); - assert!(result.is_empty(), "no matches should produce empty result"); + assert_eq!( + result.len(), + 1, + "glob root should be returned even when nothing matches yet" + ); + assert_eq!(result[0].resolved_path, base); + assert_eq!(result[0].mount_index, 1); } #[test] @@ -1770,26 +1798,35 @@ fn test_resolve_external_worktree_dirs_expanded_preserves_non_glob_index() { assert_eq!(result.len(), 3); assert_eq!(result[0].mount_index, 1, "codex keeps index 1"); - assert_eq!(result[1].mount_index, 2, "glob first match keeps index 2"); + assert_eq!(result[1].mount_index, 2, "glob gets index 2"); + assert_eq!( + result[1].resolved_path, ext, + "glob entry resolves to the glob root" + ); assert_eq!(result[2].mount_index, 3, "literal keeps index 3"); } #[test] -fn test_resolve_external_worktree_dirs_expanded_sorted_deterministic() { +fn test_resolve_external_worktree_dirs_expanded_glob_root_covers_future_dirs() { let dir = tempfile::tempdir().unwrap(); let ext = dir.path().join("repos"); - std::fs::create_dir_all(ext.join("zzz").join("wt")).unwrap(); std::fs::create_dir_all(ext.join("aaa").join("wt")).unwrap(); - std::fs::create_dir_all(ext.join("mmm").join("wt")).unwrap(); + std::fs::create_dir_all(ext.join("bbb").join("wt")).unwrap(); let pattern = format!("{}/*/wt", ext.display()); let dirs = vec![pattern]; let result = Coastfile::resolve_external_worktree_dirs_expanded(&dirs, dir.path()); - assert_eq!(result.len(), 3); - assert!(result[0].resolved_path.ends_with("aaa/wt"), "sorted first"); - assert!(result[1].resolved_path.ends_with("mmm/wt"), "sorted second"); - assert!(result[2].resolved_path.ends_with("zzz/wt"), "sorted third"); + assert_eq!(result.len(), 1); + assert_eq!(result[0].resolved_path, ext); + + // A new directory created after resolution is still under the glob root. + let future = ext.join("ccc").join("wt"); + std::fs::create_dir_all(&future).unwrap(); + assert!( + future.starts_with(&result[0].resolved_path), + "future directory should be covered by the glob root mount" + ); } #[test] diff --git a/coast-daemon/src/handlers/assign/services.rs b/coast-daemon/src/handlers/assign/services.rs index df13eb8..f503ab1 100644 --- a/coast-daemon/src/handlers/assign/services.rs +++ b/coast-daemon/src/handlers/assign/services.rs @@ -2143,4 +2143,32 @@ mod tests { assert_eq!(entries.len(), 1); assert_eq!(entries[0].branch_line, "detached"); } + + #[test] + fn test_match_porcelain_external_worktree_under_glob_root() { + let ext_dir = tempfile::tempdir().unwrap(); + let ext_path = ext_dir.path().to_path_buf(); + + // Simulate a glob root at ext_path (e.g. ~/.shep/repos) with a + // worktree nested several levels deep (e.g. hash/wt/my-branch). + let wt = ext_path.join("a21f").join("wt").join("my-branch"); + std::fs::create_dir_all(&wt).unwrap(); + + let porcelain = format!("worktree {}\nbranch refs/heads/my-branch\n\n", wt.display(),); + + let external_dirs = vec![coast_core::coastfile::ResolvedExternalDir { + mount_index: 2, + raw_pattern: "~/.shep/repos/*/wt".to_string(), + resolved_path: ext_path, + }]; + + let loc = match_porcelain_to_external(&porcelain, "my-branch", &external_dirs); + assert!(loc.is_some(), "should match worktree under glob root"); + let loc = loc.unwrap(); + assert_eq!( + loc.container_mount_src, + format!("/host-external-wt/2/a21f/wt/my-branch"), + "mount source should include the full relative path from the glob root" + ); + } } diff --git a/docs/coastfiles/WORKTREE_DIR.md b/docs/coastfiles/WORKTREE_DIR.md index 0fefd73..46b9e41 100644 --- a/docs/coastfiles/WORKTREE_DIR.md +++ b/docs/coastfiles/WORKTREE_DIR.md @@ -47,7 +47,7 @@ worktree_dir = ["/shared/worktrees", ".worktrees"] ### Glob patterns (external) -External paths can contain glob metacharacters (`*`, `?`, `[...]`). Coast expands them at runtime against the host filesystem, creating a bind mount for each matching directory. +External paths can contain glob metacharacters (`*`, `?`, `[...]`). ```toml worktree_dir = [".worktrees", "~/.shep/repos/*/wt"] @@ -62,9 +62,9 @@ Supported glob syntax: - `[abc]` — matches any character in the set - `[!abc]` — matches any character not in the set -Glob expansion happens everywhere worktree dirs are resolved: container creation, assign, start, lookup, and the git watcher. Matches are sorted for deterministic ordering. If a glob matches no directories, it is silently skipped. +Coast mounts the **glob root** — the directory prefix before the first wildcard component — rather than each individual match. For `~/.shep/repos/*/wt`, the glob root is `~/.shep/repos/`. This means new directories that appear after container creation (e.g., a new hash directory created by Shep) are automatically accessible inside the container without recreation. Dynamic assigns to worktrees under new glob matches work immediately. -Like other external paths, the container must be recreated (`coast run`) after adding a glob pattern for the bind mount to take effect. +Adding a *new* glob pattern to the Coastfile still requires `coast run` to create the bind mount. But once the pattern exists, new directories matching it are picked up automatically. ## How external directories work