A mount point's ancestors were unreachable whenever / was mounted

CHANGELOG.md

@@ -17,6 +17,10 @@ breaking entries are marked **BREAKING**.
   second surface cannot drift from the first by omission.
 
 ### Fixed
+- **A mount point's ancestors are navigable again** — with a backend at `/` and
+  another mounted deeper, `stat`, `ls`, `cd`, and the file tests answered "not
+  found" for the directories above the deeper mount. They synthesize as
+  directories, as they already did when no backend covered `/`.
 - **`help <tool>` and `kaish-tools <name>` only rendered one level of
   subcommands** (`kaish-tools` rendered none), hiding a nested verb
   (`worktree list`) and its flags. Both now recurse to any depth, one flat

crates/kaish-kernel/src/vfs/router.rs

@@ -142,6 +142,34 @@ impl VfsRouter {
     /// (`/v` over mounts `/v/jobs`, `/v/blobs` → `blobs`, `jobs`). `dir` is
     /// expected to be a non-root ancestor with no mount of its own; root is
     /// handled by `list_root`, which also folds in a `/` mount's real contents.
+    /// Recover a mount point's ancestor from a `NotFound`.
+    ///
+    /// A mount at `/a/b/c` implies `/a` and `/a/b` are directories, the way a
+    /// real mount implies its mount point's parents. The router synthesizes
+    /// them because no backend owns them.
+    ///
+    /// Mounting `/` makes `mount_of` match every path, so the backend covering
+    /// `/` is asked for `/a`, answers `NotFound`, and that reaches the caller
+    /// before the ancestor check below the `Err` arm can run. That check is
+    /// therefore unreachable whenever a root mount exists, which is the
+    /// ordinary embedder shape. This runs on the answer instead of on the
+    /// routing.
+    ///
+    /// Only `NotFound` is recovered. Any other error is the backend's answer
+    /// about a path it owns and must reach the caller unchanged.
+    fn or_synthesized_ancestor<T>(
+        &self,
+        path: &Path,
+        error: io::Error,
+        synthesize: impl FnOnce() -> T,
+    ) -> io::Result<T> {
+        if error.kind() == io::ErrorKind::NotFound && self.has_mount_under(path) {
+            Ok(synthesize())
+        } else {
+            Err(error)
+        }
+    }
+
     fn list_mount_children(&self, dir: &Path) -> Vec<DirEntry> {
         let dir = Self::normalize_mount_path(dir.to_path_buf());
         let prefix = format!("{}/", dir.to_string_lossy());
@@ -327,17 +355,15 @@ impl Filesystem for VfsRouter {
             return self.list_root().await;
         }
 
-        match self.find_mount(path) {
+        let answer = match self.find_mount(path) {
             Ok((fs, relative)) => fs.list(&relative).await,
-            // Not covered by a mount, but an ancestor of one (e.g. `/v` above
-            // `/v/jobs`): synthesize its child mount directories rather than 404.
-            Err(e) => {
-                if self.has_mount_under(path) {
-                    Ok(self.list_mount_children(path))
-                } else {
-                    Err(e)
-                }
-            }
+            Err(e) => Err(e),
+        };
+        match answer {
+            Ok(entries) => Ok(entries),
+            // An ancestor of a mount lists the mounts beneath it rather
+            // than 404ing.
+            Err(e) => self.or_synthesized_ancestor(path, e, || self.list_mount_children(path)),
         }
     }
 
@@ -359,16 +385,18 @@ impl Filesystem for VfsRouter {
             return Ok(DirEntry::directory(name));
         }
 
-        match self.find_mount(path) {
+        let answer = match self.find_mount(path) {
             Ok((fs, relative)) => fs.stat(&relative).await,
-            // Intermediate ancestor of a mount (e.g. `/v` above `/v/jobs`)
-            // exists as a synthesized directory.
+            Err(e) => Err(e),
+        };
+        match answer {
+            Ok(entry) => Ok(entry),
+            // An ancestor of a mount (`/v` above `/v/jobs`) is a synthesized
+            // directory, whether the routing missed or the backend did.
             Err(e) => {
-                if self.has_mount_under(path) {
-                    Ok(DirEntry::directory(Self::path_basename(path)))
-                } else {
-                    Err(e)
-                }
+                self.or_synthesized_ancestor(path, e, || {
+                    DirEntry::directory(Self::path_basename(path))
+                })
             }
         }
     }
@@ -425,16 +453,18 @@ impl Filesystem for VfsRouter {
             return Ok(DirEntry::directory(name));
         }
 
-        match self.find_mount(path) {
+        let answer = match self.find_mount(path) {
             Ok((fs, relative)) => fs.lstat(&relative).await,
-            // Intermediate ancestor of a mount (e.g. `/v` above `/v/jobs`)
-            // exists as a synthesized directory.
+            Err(e) => Err(e),
+        };
+        match answer {
+            Ok(entry) => Ok(entry),
+            // A synthesized ancestor is a directory, never a symlink, so
+            // lstat and stat agree about it.
             Err(e) => {
-                if self.has_mount_under(path) {
-                    Ok(DirEntry::directory(Self::path_basename(path)))
-                } else {
-                    Err(e)
-                }
+                self.or_synthesized_ancestor(path, e, || {
+                    DirEntry::directory(Self::path_basename(path))
+                })
             }
         }
     }
@@ -481,18 +511,19 @@ impl Filesystem for VfsRouter {
     /// A synthesized directory is readable and searchable, and never
     /// writable — the router creates nothing in one.
     async fn path_access(&self, path: &Path) -> io::Result<PathAccess> {
-        match self.find_mount(path) {
+        let path_str = path.to_string_lossy();
+        if path_str.is_empty() || path_str == "/" {
+            return Ok(PathAccess::resolve(Some(SYNTHESIZED_DIRECTORY_MODE), true));
+        }
+        let answer = match self.find_mount(path) {
             Ok((fs, relative)) => fs.path_access(&relative).await,
-            Err(e) => {
-                let path_str = path.to_string_lossy();
-                let is_synthesized =
-                    path_str.is_empty() || path_str == "/" || self.has_mount_under(path);
-                if is_synthesized {
-                    Ok(PathAccess::resolve(Some(SYNTHESIZED_DIRECTORY_MODE), true))
-                } else {
-                    Err(e)
-                }
-            }
+            Err(e) => Err(e),
+        };
+        match answer {
+            Ok(access) => Ok(access),
+            Err(e) => self.or_synthesized_ancestor(path, e, || {
+                PathAccess::resolve(Some(SYNTHESIZED_DIRECTORY_MODE), true)
+            }),
         }
     }
 

crates/kaish-kernel/tests/router_mount_ancestor_tests.rs

@@ -0,0 +1,145 @@
+//! A mount point's ancestors must be navigable.
+//!
+//! `VfsRouter::mount_of` picks the longest matching mount, and `/` matches
+//! everything. When a backend is mounted several components below `/` — the
+//! common embedder shape (`kaibo`, `kaijutsu`) — the components ABOVE that
+//! mount point are owned by whichever filesystem covers `/`, which has no
+//! entry for them. The router answers for a path it does not own, and the
+//! answer is "No such file or directory".
+//!
+//! In a real filesystem a mount point's ancestors necessarily exist: you
+//! cannot mount at `/a/b/c` unless `/a/b` is a directory. The router
+//! synthesizes the mount point itself; it must synthesize the path to it too.
+
+// Test-fixture code: unwrap/expect on known-good setup is the idiom here.
+#![allow(clippy::unwrap_used, clippy::expect_used)]
+#![cfg(all(feature = "localfs", unix))]
+
+use std::path::{Path, PathBuf};
+use std::sync::Arc;
+
+use kaish_kernel::vfs::{LocalFs, MemoryFs, VfsRouter};
+use kaish_kernel::{Kernel, KernelBackend, KernelConfig, LocalBackend};
+
+fn tempdir() -> tempfile::TempDir {
+    tempfile::Builder::new()
+        .prefix("router-ancestor-")
+        .tempdir_in(env!("CARGO_TARGET_TMPDIR"))
+        .expect("tempdir under CARGO_TARGET_TMPDIR")
+}
+
+/// The mount root, several components below `/`, mirroring its own host path.
+fn fixture_root(base: &tempfile::TempDir) -> PathBuf {
+    let root = base.path().join("project").join("fixture");
+    std::fs::create_dir_all(&root).expect("mkdir project/fixture");
+    std::fs::write(root.join("top.txt"), "top\n").expect("write top.txt");
+    root
+}
+
+fn rooted_kernel(root: &Path) -> Kernel {
+    let mut vfs = VfsRouter::new();
+    vfs.mount(root.to_path_buf(), LocalFs::read_only(root.to_path_buf()));
+    vfs.mount("/", MemoryFs::new());
+    let backend: Arc<dyn KernelBackend> = Arc::new(LocalBackend::new(Arc::new(vfs)));
+    let config = KernelConfig::isolated().with_cwd(root.to_path_buf());
+    Kernel::with_backend(backend, config, |_| {}, |_| {}).expect("with_backend kernel")
+}
+
+async fn run(kernel: &Kernel, line: &str) -> (String, String, i64) {
+    let result = kernel.execute(line).await.expect("execute");
+    (
+        result.text_out().trim_end().to_string(),
+        result.err.trim_end().to_string(),
+        result.code,
+    )
+}
+
+/// Every strict ancestor of the mount point, from `/` down to its parent.
+fn ancestors_of(root: &Path) -> Vec<String> {
+    let mut out = Vec::new();
+    let mut current = root.parent();
+    while let Some(path) = current {
+        out.push(path.to_string_lossy().into_owned());
+        current = path.parent();
+    }
+    out.reverse();
+    out
+}
+
+#[tokio::test]
+async fn stat_answers_for_every_ancestor_of_a_mount_point() {
+    let base = tempdir();
+    let root = fixture_root(&base);
+    let kernel = rooted_kernel(&root);
+
+    for ancestor in ancestors_of(&root) {
+        let (_out, err, code) = run(&kernel, &format!("stat {ancestor}")).await;
+        assert_eq!(
+            code, 0,
+            "stat must answer for the mount ancestor {ancestor}: {err}"
+        );
+    }
+}
+
+#[tokio::test]
+async fn ls_lists_an_ancestor_of_a_mount_point() {
+    let base = tempdir();
+    let root = fixture_root(&base);
+    let kernel = rooted_kernel(&root);
+
+    let parent = root.parent().expect("mount root has a parent");
+    let (out, err, code) = run(&kernel, &format!("ls {}", parent.display())).await;
+    assert_eq!(code, 0, "ls must list a mount ancestor: {err}");
+    let leaf = root.file_name().expect("mount root has a name").to_string_lossy();
+    assert!(
+        out.contains(leaf.as_ref()),
+        "listing a mount's parent must show the mount itself, got: {out:?}"
+    );
+}
+
+#[tokio::test]
+async fn cd_into_an_ancestor_of_a_mount_point_succeeds() {
+    let base = tempdir();
+    let root = fixture_root(&base);
+    let kernel = rooted_kernel(&root);
+
+    let parent = root.parent().expect("mount root has a parent");
+    let (_out, err, code) = run(&kernel, &format!("cd {}", parent.display())).await;
+    assert_eq!(code, 0, "cd into a mount ancestor must succeed: {err}");
+}
+
+#[tokio::test]
+async fn a_file_test_sees_an_ancestor_as_a_directory() {
+    let base = tempdir();
+    let root = fixture_root(&base);
+    let kernel = rooted_kernel(&root);
+
+    let parent = root.parent().expect("mount root has a parent");
+    let (out, err, code) = run(
+        &kernel,
+        &format!("if [[ -d {} ]]; then echo IS_DIR; fi", parent.display()),
+    )
+    .await;
+    assert_eq!(code, 0, "the file test must not error: {err}");
+    assert_eq!(out, "IS_DIR", "a mount ancestor must test as a directory");
+}
+
+/// Control: a path that is NOT an ancestor of any mount is still absent.
+/// Synthesizing ancestors must not make the whole namespace exist.
+#[tokio::test]
+async fn an_unrelated_missing_path_is_still_missing() {
+    let base = tempdir();
+    let root = fixture_root(&base);
+    let kernel = rooted_kernel(&root);
+
+    let (_out, _err, code) = run(&kernel, "stat /definitely/not/here").await;
+    assert_ne!(code, 0, "an unrelated missing path must still be missing");
+
+    let (out, _err, code) = run(
+        &kernel,
+        "if [[ -d /definitely/not/here ]]; then echo IS_DIR; else echo ABSENT; fi",
+    )
+    .await;
+    assert_eq!(code, 0);
+    assert_eq!(out, "ABSENT", "a non-ancestor must not be synthesized");
+}