From 7befc87170b5693dfa00ce1104120e497fe86c27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hu=C3=A2n=20L=C3=AA-V=C6=B0=C6=A1ng?= <65440815+lehuan5062@users.noreply.github.com> Date: Thu, 9 Jul 2026 10:23:54 +0700 Subject: [PATCH 1/4] lore: Discard reverted uncommitted directory adds on scan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A working-tree scan (`status --scan` / `stage --scan`) already discards a reverted uncommitted *file* add so `state_staged` matches the filesystem: a file that was staged and then removed before any commit has no committed base to delete from, so reporting a `Delete` would leave an unremovable "zombie" entry. A reverted uncommitted *directory* add was not handled the same way. Extend the same treatment to directories. When a directory node exists in `state_from` but neither in `state_current` (never committed) nor on disk, queue it for discard instead of emitting a meaningless `Delete`. Discarding a directory node must also reclaim its subtree: `apply_pending_discards` now recursively discards every child below a directory node before unlinking the node itself, so no stale descendant slots are left behind. Includes a Rust test covering the index-then-remove cycle for a directory add, asserting the node is discarded (and does not resurface on a later scan) rather than reported as a delete. Signed-off-by: Huân Lê-Vương <65440815+lehuan5062@users.noreply.github.com> --- lore-revision/src/state.rs | 58 ++++++ lore-revision/tests/reverted_directory.rs | 226 ++++++++++++++++++++++ 2 files changed, 284 insertions(+) create mode 100644 lore-revision/tests/reverted_directory.rs diff --git a/lore-revision/src/state.rs b/lore-revision/src/state.rs index af01711e..ed16d429 100644 --- a/lore-revision/src/state.rs +++ b/lore-revision/src/state.rs @@ -4684,6 +4684,30 @@ async fn apply_pending_discards( } let initial_ancestor = discard_node.parent; + + // For a directory, discard the whole subtree below it first so its node + // slots are reclaimed; the node itself is unlinked from its parent and + // discarded by node_discard_patch below. Each child's sibling pointer is + // captured before discarding it, since discard_node repurposes that + // pointer for the block's free list. + if discard_node.is_directory() { + let mut child_ref = discard_node.child(); + while let Some(child_id) = child_ref { + let child_node = state.node(repository.clone(), child_id).await?; + let next_sibling = child_node.sibling(); + node_discard_recurse( + state.clone(), + repository.clone(), + child_id, + true, /* recurse */ + true, /* discard */ + |_, _| {}, + ) + .await?; + child_ref = next_sibling; + } + } + node_discard_patch( state.clone(), repository.clone(), @@ -5606,6 +5630,15 @@ async fn emit_filesystem_subtree_deletes( Ok(false) } +/// Match each filesystem item from `file_receiver` against `node_list` (the +/// `from` state's children) and `current_node_list` (the `current` state's +/// children), emitting changes into `changes`, marking matched entries in +/// `node_list_found`, spawning subtree-recursion tasks into `tasks`, and +/// queueing stale directory nodes into `pending_discards`. Items with no +/// match in `node_list` are buffered and processed as new adds once the +/// receiver is drained. Must only be called from [`diff_filesystem_directory`], +/// which sorts `node_list` and `current_node_list` by name beforehand — the +/// binary searches here assume that ordering. #[allow(clippy::too_many_arguments)] async fn diff_filesystem_directory_walk( ctx: &DiffFilesystemContext, @@ -5937,6 +5970,31 @@ async fn diff_filesystem_directory_walk( continue; }; + // A directory node that exists in state_from but neither in state_current + // (never committed) nor on disk is a reverted, uncommitted add: the + // directory was staged and then removed from disk before any commit, + // together with whatever of its contents had been staged under it. + // Reporting it as a `Delete` is meaningless because there is no committed + // base to delete from, and no mutation verb can clear it (the "zombie" + // entry). Discard the whole subtree so state_staged matches the filesystem + // instead, the same way a reverted single-file add is discarded below. + if ctx.scan_dirty && from_node.node.is_directory() { + let in_current = current_node_list + .children + .as_slice() + .binary_search_by(|child| child.name.cmp(&from_named_node.name)) + .is_ok(); + if !in_current { + lore_trace!( + "Queueing reverted uncommitted directory node {} (no entry at {}, not in current)", + from_named_node.node, + from_node.path + ); + pending_discards.push(from_named_node.node); + continue; + } + } + // Emit deletes only for the materialized portion of the subtree, // suppressing directories the filter merely descended through but never // wrote to disk (see emit_filesystem_subtree_deletes). diff --git a/lore-revision/tests/reverted_directory.rs b/lore-revision/tests/reverted_directory.rs new file mode 100644 index 00000000..a2d65051 --- /dev/null +++ b/lore-revision/tests/reverted_directory.rs @@ -0,0 +1,226 @@ +// SPDX-FileCopyrightText: 2026 Epic Games, Inc. +// SPDX-License-Identifier: MIT + +//! Working-tree scan handling of a reverted uncommitted directory add. +//! +//! When a directory (and its contents) is indexed as an uncommitted add and +//! then removed from disk before any commit, the next scan must discard the +//! stale node rather than report a delete. The parent has no committed base the +//! directory could be a deletion of, so a delete entry would be an unremovable +//! "zombie" — the same treatment already given to a reverted single-file add. + +#[cfg(test)] +mod tests { + #![allow(clippy::disallowed_methods)] // Test fixture writes; not subject to repository write-token discipline. + + use std::fs::File; + use std::io::Write; + use std::path::Path; + use std::sync::Arc; + + use lore_base::error::NoRemote; + use lore_base::runtime::LORE_CONTEXT; + use lore_base::runtime::runtime; + use lore_base::types::Context; + use lore_revision::branch; + use lore_revision::change::FileAction; + use lore_revision::filter::FilterMode; + use lore_revision::lore::RepositoryId; + use lore_revision::repository; + use lore_revision::repository::RepositoryContext; + use lore_revision::repository::RepositoryFormat; + use lore_revision::repository::load_filter; + use lore_revision::state; + use lore_transport::ProtocolError; + + include!("helper.rs"); + + /// Create (or truncate) a read/write file at `path` and write `contents` to + /// it, returning the open handle. Panics if the file cannot be created or + /// written, since a failed fixture setup invalidates the test. + fn create_file(path: &Path, contents: &[u8]) -> File { + let mut file = File::options() + .create(true) + .truncate(true) + .read(true) + .write(true) + .open(path) + .unwrap_or_else(|_| panic!("Failed to create test file at {}", path.display())); + file.write_all(contents) + .unwrap_or_else(|_| panic!("Failed to write test file at {}", path.display())); + file + } + + /// Build a fresh on-disk repository at `path` with no commits (revision 0) + /// and return a write-capable [`RepositoryContext`] for it. + async fn create_repository( + path: &Path, + repository_id: RepositoryId, + immutable_store: Arc, + mutable_store: Arc, + ) -> Arc { + std::fs::create_dir_all(path).expect("Create repository directory failed"); + let default_branch = Context::from(uuid::Uuid::now_v7()); + let write_token = repository::RepositoryWriteToken::acquire(path).await; + let created_repo = repository::create_local( + path, + &write_token, + repository_id, + default_branch, + branch::DEFAULT_DEFAULT_NAME.to_string(), + repository::RepositoryConfig::default(), + false, + ) + .await + .expect("Failed to create repository"); + + let repository = Arc::new( + RepositoryContext::new( + Some(path.to_path_buf()), + immutable_store, + mutable_store, + repository_id, + created_repo.instance_id, + Err(ProtocolError::from(NoRemote)), + load_filter(path).expect("Failed to load filter"), + RepositoryFormat::Lore, + ) + .with_write_token(write_token.share()), + ); + lore_revision::instance::store_current_anchor_branch(&repository, default_branch) + .await + .expect("Failed to store anchor branch"); + repository + } + + /// Reconcile the working tree against the staged state, mutating `state_staged` + /// in place exactly as `lore status --scan` does, and return the detected + /// changes. + async fn scan( + repository: Arc, + state_staged: Arc, + state_current: Arc, + ) -> Vec { + let (changes, _stats) = state::diff_filesystem_ex( + repository.clone(), + state_staged, + repository, + state_current, + None, /* full tree */ + FilterMode::Full, + true, /* scan_dirty */ + Arc::new(Vec::new()), + ) + .await + .expect("Failed to diff filesystem"); + changes + } + + /// A directory indexed as an uncommitted add (along with its contents) and + /// then removed from disk must be discarded on the next scan rather than + /// reported as a delete: with no committed base there is nothing to delete, + /// and a delete entry would be an unremovable "zombie". + #[tokio::test] + async fn removed_uncommitted_directory_is_discarded_not_deleted() { + let (immutable_store, mutable_store, execution) = + test_store_create().await.expect("Failed to create stores"); + let repository_id = RepositoryId::from(uuid::Uuid::now_v7()); + + runtime() + .spawn(LORE_CONTEXT.scope(execution.clone(), async move { + let tempdir = generate_tempdir(); + let path = tempdir.to_path_buf(); + let repository = create_repository( + path.as_path(), + repository_id, + immutable_store.clone(), + mutable_store.clone(), + ) + .await; + + // A directory with content that gets indexed as an uncommitted + // add (the directory node plus its child file). + std::fs::create_dir(path.join("ghost").as_path()) + .expect("Create ghost directory failed"); + let _ = create_file(path.join("ghost").join("inner.txt").as_path(), &[7, 7, 7]); + + let (current_revision, _branch) = + lore_revision::instance::load_current_anchor(&repository) + .await + .expect("Failed to load current anchor"); + let state_current = state::State::deserialize(repository.clone(), current_revision) + .await + .expect("Failed to deserialize current state"); + let state_staged = state::State::deserialize(repository.clone(), current_revision) + .await + .expect("Failed to deserialize staged state"); + + // First scan indexes the directory as an add. + let changes = scan( + repository.clone(), + state_staged.clone(), + state_current.clone(), + ) + .await; + assert!( + changes + .iter() + .any(|c| c.path.as_str() == "ghost" && c.action == FileAction::Add), + "expected the new directory to be indexed as an add, found: {:?}", + changes + .iter() + .map(|c| (c.path.as_str().to_string(), c.action)) + .collect::>() + ); + assert!( + changes.iter().any(|c| c.path.as_str() == "ghost/inner.txt"), + "expected the directory's contents to be indexed too, found: {:?}", + changes + .iter() + .map(|c| (c.path.as_str().to_string(), c.action)) + .collect::>() + ); + + // Remove it from disk and rescan against the same staged state. + std::fs::remove_dir_all(path.join("ghost")) + .expect("Failed to remove ghost directory"); + let changes = scan( + repository.clone(), + state_staged.clone(), + state_current.clone(), + ) + .await; + assert!( + changes + .iter() + .all(|c| !c.path.as_str().starts_with("ghost")), + "removed uncommitted directory must be discarded, not reported, found: {:?}", + changes + .iter() + .map(|c| (c.path.as_str().to_string(), c.action)) + .collect::>() + ); + + // A further scan stays clean — the node was discarded, not merely + // hidden, so it cannot resurface. + let changes = scan( + repository.clone(), + state_staged.clone(), + state_current.clone(), + ) + .await; + assert!( + changes + .iter() + .all(|c| !c.path.as_str().starts_with("ghost")), + "discarded directory must not resurface on a later scan, found: {:?}", + changes + .iter() + .map(|c| (c.path.as_str().to_string(), c.action)) + .collect::>() + ); + })) + .await + .expect("Test task panicked"); + } +} From 2e65f1b93ba63aff6a220a14a4b6c73934db76c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hu=C3=A2n=20L=C3=AA-V=C6=B0=C6=A1ng?= <65440815+lehuan5062@users.noreply.github.com> Date: Thu, 9 Jul 2026 10:27:19 +0700 Subject: [PATCH 2/4] lore: Handle nested repositories as working-tree scan boundaries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A child directory that carries its own `.lore/` control directory is a nested repository — its contents belong to it, not the parent. The parent's working-tree scan (from `status --scan` or `stage --scan`) must treat it as an implicit boundary: do not descend into or index it, just as git treats a nested `.git` directory as a submodule boundary. When scanning the filesystem, skip any child directory that is itself a Lore working copy. A node previously indexed for such a directory (from before the boundary check existed) falls through to the delete pass, where it is cleared as a reverted uncommitted directory add on the next scan. Add a new optional-paths argument struct (FileOptionalPathsTargetsArgs) to support `lore stage --scan` with no path, which reconciles and stages the entire working tree from the repository root. Without `--scan`, a path remains required. Includes: a Rust test covering the nested-repository boundary (with state-based setup), plus a CLI smoke test for both nested-repository handling and `stage --scan` with no path. Signed-off-by: Huân Lê-Vương <65440815+lehuan5062@users.noreply.github.com> --- docs/reference/lore-cli-commands.md | 8 +- lore-client/src/cli/commands/file.rs | 23 ++- lore-revision/src/state.rs | 47 +++++- lore-revision/tests/nested_repository.rs | 184 +++++++++++++++++++++++ scripts/test/test_nested_repository.py | 83 ++++++++++ 5 files changed, 334 insertions(+), 11 deletions(-) create mode 100644 lore-revision/tests/nested_repository.rs create mode 100644 scripts/test/test_nested_repository.py diff --git a/docs/reference/lore-cli-commands.md b/docs/reference/lore-cli-commands.md index d0e74551..9ffe9b91 100644 --- a/docs/reference/lore-cli-commands.md +++ b/docs/reference/lore-cli-commands.md @@ -1696,7 +1696,7 @@ Specific file paths are checked against the filesystem and staged if content dif `--scan` walks the filesystem under the given paths, marks every detected modification/add/delete dirty, and stages them in one step. -**Usage:** `lore file stage [OPTIONS] > +**Usage:** `lore file stage [OPTIONS] [paths|--targets ] stage [OPTIONS] ` ###### **Subcommands:** @@ -1725,6 +1725,8 @@ Specific file paths are checked against the filesystem and staged if content dif Detected changes are marked dirty and staged in a single pass. Use this when changes were made externally (without going through `lore dirty`), or to recover after losing track of dirty state. Equivalent in effect to running `lore status --scan` followed by `lore stage`, but performed in one traversal. Without `--scan`, directory staging stages only files already marked dirty under that directory — mark them first with `lore dirty `, or run `lore status --scan` to reconcile dirty flags across a tree. Single-file stage paths are always checked against the filesystem regardless of this flag. + + With `--scan` and no path, `lore` reconciles and stages the entire working tree from the repository root, matching the bulk reconciliation `lore dirty` recommends. * `--targets ` — Path to a targets file containing all the paths to all files @@ -2261,7 +2263,7 @@ Specific file path: checked against the filesystem and staged if its on-disk con `--scan`: forces a filesystem walk under the given paths, marks modified, added, and deleted files dirty, and stages them in one step. Use this when changes were made externally without going through `lore dirty`, or to recover after losing track of dirty state. -**Usage:** `lore stage [OPTIONS] > +**Usage:** `lore stage [OPTIONS] [paths|--targets ] stage [OPTIONS] ` ###### **Subcommands:** @@ -2290,6 +2292,8 @@ Specific file path: checked against the filesystem and staged if its on-disk con Detected changes are marked dirty and staged in a single pass. Use this when changes were made externally (without going through `lore dirty`), or to recover after losing track of dirty state. Equivalent in effect to running `lore status --scan` followed by `lore stage`, but performed in one traversal. Without `--scan`, directory staging stages only files already marked dirty under that directory — mark them first with `lore dirty `, or run `lore status --scan` to reconcile dirty flags across a tree. Single-file stage paths are always checked against the filesystem regardless of this flag. + + With `--scan` and no path, `lore` reconciles and stages the entire working tree from the repository root, matching the bulk reconciliation `lore dirty` recommends. * `--targets ` — Path to a targets file containing all the paths to all files diff --git a/lore-client/src/cli/commands/file.rs b/lore-client/src/cli/commands/file.rs index 9b234184..1b0dadb9 100644 --- a/lore-client/src/cli/commands/file.rs +++ b/lore-client/src/cli/commands/file.rs @@ -317,11 +317,15 @@ pub struct FileStageArgs { /// `lore dirty `, or run `lore status --scan` to reconcile /// dirty flags across a tree. Single-file stage paths are always /// checked against the filesystem regardless of this flag. + /// + /// With `--scan` and no path, `lore` reconciles and stages the entire + /// working tree from the repository root, matching the bulk reconciliation + /// `lore dirty` recommends. #[clap(long, action)] scan: bool, #[clap(flatten)] - paths: FilePathsTargetsArgs, + paths: FileOptionalPathsTargetsArgs, #[clap(flatten)] stage: FileStageCommandArgs, @@ -1146,7 +1150,22 @@ pub fn handle_file_stage(globals: LoreGlobalArgs, args: &FileStageArgs) -> u8 { // Standard stage if args.stage.subcommand.is_none() { - let paths = convert_paths_and_targets(&args.paths.paths, &args.paths.targets); + let mut paths = convert_paths_and_targets(&args.paths.paths, &args.paths.targets); + + // `lore stage --scan` with no path reconciles and stages the whole + // working tree, defaulting to the repository root. Without `--scan` a + // path is still required, since directory staging without a scan only + // picks up already-dirty entries and an empty path set has nothing to do. + if paths.is_empty() { + if args.scan { + paths = LoreArray::from_vec(vec![LoreString::from(".")]); + } else { + println!( + "error: a path is required; pass one or more paths, or use --scan to stage the whole tree" + ); + return 1; + } + } let stage_args = LoreFileStageArgs { paths, diff --git a/lore-revision/src/state.rs b/lore-revision/src/state.rs index ed16d429..cc6c396b 100644 --- a/lore-revision/src/state.rs +++ b/lore-revision/src/state.rs @@ -5630,6 +5630,23 @@ async fn emit_filesystem_subtree_deletes( Ok(false) } +/// Returns whether the on-disk directory at `relative_path` (resolved under +/// `repository_root`) is itself a Lore working copy — it contains its own +/// `.lore/` (or legacy `.urc/`) control directory. +/// +/// Such a nested repository is an implicit boundary for the parent's +/// working-tree scan: its contents belong to the nested repository, not the +/// parent, so the parent neither descends into nor indexes it. This mirrors the +/// way git treats a nested `.git` directory as a submodule boundary rather than +/// pulling its files into the outer repository. +fn is_nested_repository_root( + repository_root: &std::path::Path, + relative_path: &RelativePath, +) -> bool { + let absolute_path = relative_path.to_absolute_path(repository_root); + absolute_path.join(DOT_LORE).is_dir() || absolute_path.join(DOT_URC).is_dir() +} + /// Match each filesystem item from `file_receiver` against `node_list` (the /// `from` state's children) and `current_node_list` (the `current` state's /// children), emitting changes into `changes`, marking matched entries in @@ -5663,6 +5680,21 @@ async fn diff_filesystem_directory_walk( .push_into_buf(item.name.as_str()) .freeze(); + // A child directory that is itself a Lore working copy (it contains its + // own `.lore`/`.urc` control directory) is an implicit boundary: do not + // descend into or index it, mirroring how git ignores nested `.git` + // directories. Skipping it keeps the nested repository's contents out of + // the parent's tree. A node previously indexed for it (from before this + // boundary check existed) is left unmatched and so falls through to the + // delete pass below, where an empty never-committed entry is discarded — + // clearing a pre-existing stale "zombie" entry on the next scan. + if item.metadata.is_dir() + && is_nested_repository_root(ctx.from.repository.require_path()?, &item_path) + { + lore_trace!("Skipping nested repository root {item_path}"); + continue; + } + if ctx.from.repository.filter.emit_excludes( &item_path, item.metadata.is_dir(), @@ -5971,13 +6003,14 @@ async fn diff_filesystem_directory_walk( }; // A directory node that exists in state_from but neither in state_current - // (never committed) nor on disk is a reverted, uncommitted add: the - // directory was staged and then removed from disk before any commit, - // together with whatever of its contents had been staged under it. - // Reporting it as a `Delete` is meaningless because there is no committed - // base to delete from, and no mutation verb can clear it (the "zombie" - // entry). Discard the whole subtree so state_staged matches the filesystem - // instead, the same way a reverted single-file add is discarded below. + // (never committed) nor on disk is a reverted, uncommitted add — for + // example a nested repository root that was indexed before the boundary + // check above existed and has since been removed, together with whatever + // of its contents had been pulled into the parent tree. Reporting it as a + // `Delete` is meaningless because there is no committed base to delete + // from, and no mutation verb can clear it (the "zombie" entry). Discard + // the whole subtree so state_staged matches the filesystem instead, the + // same way a reverted single-file add is discarded below. if ctx.scan_dirty && from_node.node.is_directory() { let in_current = current_node_list .children diff --git a/lore-revision/tests/nested_repository.rs b/lore-revision/tests/nested_repository.rs new file mode 100644 index 00000000..5aaeedff --- /dev/null +++ b/lore-revision/tests/nested_repository.rs @@ -0,0 +1,184 @@ +// SPDX-FileCopyrightText: 2026 Epic Games, Inc. +// SPDX-License-Identifier: MIT + +//! Working-tree scan handling of a nested repository — a child directory that +//! is itself a Lore working copy (it carries its own `.lore/`). +//! +//! A nested repository must be treated as an implicit boundary: its contents +//! belong to the nested repository, not the parent, so the parent scan neither +//! descends into nor indexes it. A child directory that was indexed before the +//! boundary existed and is then removed must not leave an unremovable delete +//! entry behind (the "zombie" entry) — the parent has no committed base it +//! could be a deletion of, so the stale node is discarded on the next scan. + +#[cfg(test)] +mod tests { + #![allow(clippy::disallowed_methods)] // Test fixture writes; not subject to repository write-token discipline. + + use std::fs::File; + use std::io::Write; + use std::path::Path; + use std::sync::Arc; + + use lore_base::error::NoRemote; + use lore_base::runtime::LORE_CONTEXT; + use lore_base::runtime::runtime; + use lore_base::types::Context; + use lore_revision::branch; + use lore_revision::filter::FilterMode; + use lore_revision::lore::RepositoryId; + use lore_revision::repository; + use lore_revision::repository::DOT_LORE; + use lore_revision::repository::RepositoryContext; + use lore_revision::repository::RepositoryFormat; + use lore_revision::repository::load_filter; + use lore_revision::state; + use lore_transport::ProtocolError; + + include!("helper.rs"); + + /// Create (or truncate) a read/write file at `path` and write `contents` to + /// it, returning the open handle. Panics if the file cannot be created or + /// written, since a failed fixture setup invalidates the test. + fn create_file(path: &Path, contents: &[u8]) -> File { + let mut file = File::options() + .create(true) + .truncate(true) + .read(true) + .write(true) + .open(path) + .unwrap_or_else(|_| panic!("Failed to create test file at {}", path.display())); + file.write_all(contents) + .unwrap_or_else(|_| panic!("Failed to write test file at {}", path.display())); + file + } + + /// Build a fresh on-disk repository at `path` with no commits (revision 0) + /// and return a write-capable [`RepositoryContext`] for it. + async fn create_repository( + path: &Path, + repository_id: RepositoryId, + immutable_store: Arc, + mutable_store: Arc, + ) -> Arc { + std::fs::create_dir_all(path).expect("Create repository directory failed"); + let default_branch = Context::from(uuid::Uuid::now_v7()); + let write_token = repository::RepositoryWriteToken::acquire(path).await; + let created_repo = repository::create_local( + path, + &write_token, + repository_id, + default_branch, + branch::DEFAULT_DEFAULT_NAME.to_string(), + repository::RepositoryConfig::default(), + false, + ) + .await + .expect("Failed to create repository"); + + let repository = Arc::new( + RepositoryContext::new( + Some(path.to_path_buf()), + immutable_store, + mutable_store, + repository_id, + created_repo.instance_id, + Err(ProtocolError::from(NoRemote)), + load_filter(path).expect("Failed to load filter"), + RepositoryFormat::Lore, + ) + .with_write_token(write_token.share()), + ); + lore_revision::instance::store_current_anchor_branch(&repository, default_branch) + .await + .expect("Failed to store anchor branch"); + repository + } + + /// Reconcile the working tree against the staged state, mutating `state_staged` + /// in place exactly as `lore status --scan` does, and return the detected + /// changes. + async fn scan( + repository: Arc, + state_staged: Arc, + state_current: Arc, + ) -> Vec { + let (changes, _stats) = state::diff_filesystem_ex( + repository.clone(), + state_staged, + repository, + state_current, + None, /* full tree */ + FilterMode::Full, + true, /* scan_dirty */ + Arc::new(Vec::new()), + ) + .await + .expect("Failed to diff filesystem"); + changes + } + + /// A child directory carrying its own `.lore/` is a nested repository: the + /// parent scan must not index it or pull its contents into the parent tree. + #[tokio::test] + async fn nested_repository_is_not_indexed() { + let (immutable_store, mutable_store, execution) = + test_store_create().await.expect("Failed to create stores"); + let repository_id = RepositoryId::from(uuid::Uuid::now_v7()); + + runtime() + .spawn(LORE_CONTEXT.scope(execution.clone(), async move { + let tempdir = generate_tempdir(); + let path = tempdir.to_path_buf(); + let repository = create_repository( + path.as_path(), + repository_id, + immutable_store.clone(), + mutable_store.clone(), + ) + .await; + + // A tracked file in the parent so the scan has real work to do. + let _ = create_file(path.join("parent_file.txt").as_path(), &[0, 1, 2, 3]); + + // A nested repository: a child directory with its own `.lore/` + // control directory and content that belongs to it, not the parent. + std::fs::create_dir(path.join("nested").as_path()) + .expect("Create nested directory failed"); + std::fs::create_dir(path.join("nested").join(DOT_LORE).as_path()) + .expect("Create nested/.lore directory failed"); + let _ = create_file(path.join("nested").join("inner.txt").as_path(), &[9, 9, 9]); + + let (current_revision, _branch) = + lore_revision::instance::load_current_anchor(&repository) + .await + .expect("Failed to load current anchor"); + let state_current = state::State::deserialize(repository.clone(), current_revision) + .await + .expect("Failed to deserialize current state"); + let state_staged = state::State::deserialize(repository.clone(), current_revision) + .await + .expect("Failed to deserialize staged state"); + + let changes = scan(repository.clone(), state_staged, state_current).await; + + // The parent file is indexed; nothing under the nested repository is. + assert!( + changes.iter().any(|c| c.path.as_str() == "parent_file.txt"), + "expected the parent's own file to be indexed" + ); + assert!( + changes + .iter() + .all(|c| !c.path.as_str().starts_with("nested")), + "nested repository contents must not be indexed, found: {:?}", + changes + .iter() + .map(|c| c.path.as_str().to_string()) + .collect::>() + ); + })) + .await + .expect("Test task panicked"); + } +} diff --git a/scripts/test/test_nested_repository.py b/scripts/test/test_nested_repository.py new file mode 100644 index 00000000..916abda7 --- /dev/null +++ b/scripts/test/test_nested_repository.py @@ -0,0 +1,83 @@ +# SPDX-FileCopyrightText: 2026 Epic Games, Inc. +# SPDX-License-Identifier: MIT +"""Smoke coverage for nested-repository handling during a working-tree scan. + +A child directory that is itself a Lore working copy (it carries its own +`.lore/`) is an implicit boundary: the parent's `status --scan` must not index +it or pull its contents into the parent tree, and removing it must not leave an +unremovable "zombie" delete entry behind. Also covers `stage --scan` with no +path defaulting to the repository root. +""" + +import logging +import os + +import pytest + +from lore import Lore +from lore_parsers import parse_status_json + +logger = logging.getLogger(__name__) + + +def _scan_paths(repo: Lore) -> list[str]: + """Return the set of status-file paths reported by `status --scan --json`.""" + entries = parse_status_json(repo.status(scan=True, json=True, offline=True)) + return [e.get("path", "").replace("\\", "/") for e in entries] + + +@pytest.mark.smoke +def test_nested_repository_not_indexed_and_no_zombie(new_lore_repo): + repo: Lore = new_lore_repo() + + # A file that legitimately belongs to the parent. + with repo.open_file("parent_file.txt", "w+b") as handle: + handle.write(os.urandom(32)) + + # A nested repository: its own working copy with its own `.lore/`. + nested_abs = os.path.join(repo.path, "nested") + os.makedirs(nested_abs, exist_ok=True) + repo.run(["repository", "create", "nested"], path=nested_abs, offline=True) + with repo.open_file(os.path.join("nested", "inner.txt"), "w+b") as handle: + handle.write(os.urandom(32)) + + # The parent indexes its own file but nothing under the nested repository. + paths = _scan_paths(repo) + assert any(p == "parent_file.txt" for p in paths), ( + f"expected the parent's own file to be indexed, got {paths}" + ) + assert not any(p == "nested" or p.startswith("nested/") for p in paths), ( + f"nested repository contents must not be indexed, got {paths}" + ) + + # Removing the nested repository leaves no "zombie" delete entry, because the + # parent never indexed it and has no committed base it could be deleted from. + repo.rmtree("nested") + paths = _scan_paths(repo) + assert not any(p == "nested" or p.startswith("nested/") for p in paths), ( + f"removed nested repository must not leave a status entry, got {paths}" + ) + + +@pytest.mark.smoke +def test_stage_scan_without_path_defaults_to_root(new_lore_repo): + repo: Lore = new_lore_repo() + with repo.open_file("tracked.txt", "w+b") as handle: + handle.write(os.urandom(32)) + + # `lore stage --scan` with no path reconciles and stages the whole tree. + repo.run(["stage", "--scan"], offline=True) + + staged = [ + e.get("path", "").replace("\\", "/") + for e in parse_status_json(repo.status(json=True, offline=True)) + ] + assert any(p == "tracked.txt" for p in staged), ( + f"`stage --scan` with no path should stage the whole tree, got {staged}" + ) + + # Without `--scan`, a bare `lore stage` still requires a path. + output = repo.run(["stage"], offline=True, check=False) + assert "a path is required" in output, ( + f"bare `stage` with no path should report a required-path error, got: {output}" + ) From 76409e6300b2dd7126386a362f78189af0252590 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hu=C3=A2n=20L=C3=AA-V=C6=B0=C6=A1ng?= <65440815+lehuan5062@users.noreply.github.com> Date: Fri, 10 Jul 2026 21:51:39 +0700 Subject: [PATCH 3/4] lore-revision: Optimize nested-repository boundary check off hot path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the is_nested_repository_root() check from the directory-walk loop (where it runs for every item) into the unmatched-child arm of binary_search_by, since a nested repository will never be tracked in the current repository's merkle tree and therefore always falls there. Normal directory descent (matched tracked nodes) now pays zero extra filesystem metadata queries (no is_dir probes for .lore or .urc). Preserve zombie-entry cleanup by restricting the check in the matched `was_directory && is_directory` branch to the staged-only case (scan_dirty && !current_node_id.is_valid_node_id()), where the cost is already paid by other mutations. This prevents re-indexing of nested repositories that were indexed before this boundary feature existed. BEHAVIOR CHANGE: A nested repository directory that was *committed* into the parent before this boundary check existed is no longer auto-unmatched and will stay tracked until explicitly removed, matching git's behavior with nested .git directories. Only staged (never-committed) nested roots are auto-discarded on the next scan. Add test case for the zombie-migration path: a staged directory that becomes a nested repository root must be discarded and its contents not re-indexed. Signed-off-by: Huân Lê-Vương <65440815+lehuan5062@users.noreply.github.com> --- lore-revision/src/state.rs | 43 +++++++++----- lore-revision/tests/nested_repository.rs | 76 ++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 15 deletions(-) diff --git a/lore-revision/src/state.rs b/lore-revision/src/state.rs index cc6c396b..ce3b33c9 100644 --- a/lore-revision/src/state.rs +++ b/lore-revision/src/state.rs @@ -5680,21 +5680,6 @@ async fn diff_filesystem_directory_walk( .push_into_buf(item.name.as_str()) .freeze(); - // A child directory that is itself a Lore working copy (it contains its - // own `.lore`/`.urc` control directory) is an implicit boundary: do not - // descend into or index it, mirroring how git ignores nested `.git` - // directories. Skipping it keeps the nested repository's contents out of - // the parent's tree. A node previously indexed for it (from before this - // boundary check existed) is left unmatched and so falls through to the - // delete pass below, where an empty never-committed entry is discarded — - // clearing a pre-existing stale "zombie" entry on the next scan. - if item.metadata.is_dir() - && is_nested_repository_root(ctx.from.repository.require_path()?, &item_path) - { - lore_trace!("Skipping nested repository root {item_path}"); - continue; - } - if ctx.from.repository.filter.emit_excludes( &item_path, item.metadata.is_dir(), @@ -5710,6 +5695,19 @@ async fn diff_filesystem_directory_walk( { index } else { + // A new child directory that is itself a Lore working copy (it + // contains its own `.lore`/`.urc` control directory) is an implicit + // boundary: do not descend into or index it, mirroring how git + // ignores nested `.git` directories. Checked only for unmatched + // items — a directory tracked in the parent's merkle tree is by + // definition not a boundary — so the normal directory descent pays + // no extra filesystem metadata queries. + if item.metadata.is_dir() + && is_nested_repository_root(ctx.from.repository.require_path()?, &item_path) + { + lore_trace!("Skipping nested repository root {item_path}"); + continue; + } new_file_list.push(item); continue; }; @@ -5861,6 +5859,21 @@ async fn diff_filesystem_directory_walk( .await }); } else if was_directory && is_directory { + // A staged never-committed directory that turns out to be a nested + // repository root is a stale "zombie" entry (indexed before the + // boundary check above existed, or a repository created inside a + // just-staged directory). Un-match it so it falls through to the + // delete pass below, which discards the never-committed subtree. + // Restricting the probe to the staged-only case keeps the normal + // committed-directory descent free of extra metadata queries. + if ctx.scan_dirty + && !current_node_id.is_valid_node_id() + && is_nested_repository_root(ctx.from.repository.require_path()?, &item_path) + { + lore_trace!("Discarding zombie entry for nested repository root {item_path}"); + node_list_found[current_index] = false; + continue; + } if ctx.scan_dirty && !current_node_id.is_valid_node_id() { // Re-emit a staged dirty-add directory (in staged, absent from // the current revision) as a single node so repeated scans stay diff --git a/lore-revision/tests/nested_repository.rs b/lore-revision/tests/nested_repository.rs index 5aaeedff..27e2738a 100644 --- a/lore-revision/tests/nested_repository.rs +++ b/lore-revision/tests/nested_repository.rs @@ -181,4 +181,80 @@ mod tests { .await .expect("Test task panicked"); } + + /// A directory already staged as a normal dirty-add that then becomes a + /// nested repository root (a `.lore/` appears inside it) is a stale + /// "zombie" entry: the next scan must discard the staged subtree instead + /// of continuing to index the nested repository's contents. + #[tokio::test] + async fn staged_directory_becoming_nested_repository_is_discarded() { + let (immutable_store, mutable_store, execution) = + test_store_create().await.expect("Failed to create stores"); + let repository_id = RepositoryId::from(uuid::Uuid::now_v7()); + + runtime() + .spawn(LORE_CONTEXT.scope(execution.clone(), async move { + let tempdir = generate_tempdir(); + let path = tempdir.to_path_buf(); + let repository = create_repository( + path.as_path(), + repository_id, + immutable_store.clone(), + mutable_store.clone(), + ) + .await; + + // A plain child directory with content: the first scan stages + // it as an ordinary dirty-add subtree. + std::fs::create_dir(path.join("nested").as_path()) + .expect("Create nested directory failed"); + let _ = create_file(path.join("nested").join("inner.txt").as_path(), &[9, 9, 9]); + + let (current_revision, _branch) = + lore_revision::instance::load_current_anchor(&repository) + .await + .expect("Failed to load current anchor"); + let state_current = state::State::deserialize(repository.clone(), current_revision) + .await + .expect("Failed to deserialize current state"); + let state_staged = state::State::deserialize(repository.clone(), current_revision) + .await + .expect("Failed to deserialize staged state"); + + let changes = scan( + repository.clone(), + state_staged.clone(), + state_current.clone(), + ) + .await; + assert!( + changes + .iter() + .any(|c| c.path.as_str().starts_with("nested")), + "expected the plain directory to be indexed by the first scan" + ); + + // The directory becomes a nested repository root — as when + // `lore repository create` runs inside a staged directory. + std::fs::create_dir(path.join("nested").join(DOT_LORE).as_path()) + .expect("Create nested/.lore directory failed"); + + // The rescan discards the stale staged entry instead of + // continuing to index the nested repository's contents. + let changes = scan(repository.clone(), state_staged, state_current).await; + assert!( + changes + .iter() + .all(|c| !c.path.as_str().starts_with("nested")), + "zombie entry for a staged directory turned nested repository must be \ + discarded, found: {:?}", + changes + .iter() + .map(|c| c.path.as_str().to_string()) + .collect::>() + ); + })) + .await + .expect("Test task panicked"); + } } From 3cba2740088cd8f9793743d5c6e1474150c6f227 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hu=C3=A2n=20L=C3=AA-V=C6=B0=C6=A1ng?= <65440815+lehuan5062@users.noreply.github.com> Date: Fri, 10 Jul 2026 21:57:23 +0700 Subject: [PATCH 4/4] lore-revision: Add test for committed nested-repository boundary change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit's zombie-discard fix only fires for never-committed staged directories, leaving a gap: no test exercised the documented behavior change where a directory committed before becoming a nested repository root stays tracked. Add committed_directory_becoming_nested_repository_stays_tracked, which stages+commits a directory, turns it into a nested repository root, then modifies its tracked file to confirm the parent still descends into and indexes it rather than treating it as an implicit boundary. Signed-off-by: Huân Lê-Vương <65440815+lehuan5062@users.noreply.github.com> --- lore-revision/tests/nested_repository.rs | 115 ++++++++++++++++++++++- 1 file changed, 110 insertions(+), 5 deletions(-) diff --git a/lore-revision/tests/nested_repository.rs b/lore-revision/tests/nested_repository.rs index 27e2738a..55056ddc 100644 --- a/lore-revision/tests/nested_repository.rs +++ b/lore-revision/tests/nested_repository.rs @@ -25,13 +25,21 @@ mod tests { use lore_base::runtime::runtime; use lore_base::types::Context; use lore_revision::branch; + use lore_revision::commit; + use lore_revision::commit::CommitOptions; + use lore_revision::file; use lore_revision::filter::FilterMode; + use lore_revision::interface::LoreArray; + use lore_revision::interface::LoreString; use lore_revision::lore::RepositoryId; + use lore_revision::node::NodeFlags; use lore_revision::repository; use lore_revision::repository::DOT_LORE; use lore_revision::repository::RepositoryContext; use lore_revision::repository::RepositoryFormat; use lore_revision::repository::load_filter; + use lore_revision::stage; + use lore_revision::stage::StageOptions; use lore_revision::state; use lore_transport::ProtocolError; @@ -54,13 +62,14 @@ mod tests { } /// Build a fresh on-disk repository at `path` with no commits (revision 0) - /// and return a write-capable [`RepositoryContext`] for it. + /// and return a write-capable [`RepositoryContext`] for it, along with the + /// [`repository::RepositoryWriteToken`] needed to stage/commit into it. async fn create_repository( path: &Path, repository_id: RepositoryId, immutable_store: Arc, mutable_store: Arc, - ) -> Arc { + ) -> (Arc, repository::RepositoryWriteToken) { std::fs::create_dir_all(path).expect("Create repository directory failed"); let default_branch = Context::from(uuid::Uuid::now_v7()); let write_token = repository::RepositoryWriteToken::acquire(path).await; @@ -92,7 +101,7 @@ mod tests { lore_revision::instance::store_current_anchor_branch(&repository, default_branch) .await .expect("Failed to store anchor branch"); - repository + (repository, write_token) } /// Reconcile the working tree against the staged state, mutating `state_staged` @@ -130,7 +139,7 @@ mod tests { .spawn(LORE_CONTEXT.scope(execution.clone(), async move { let tempdir = generate_tempdir(); let path = tempdir.to_path_buf(); - let repository = create_repository( + let (repository, _write_token) = create_repository( path.as_path(), repository_id, immutable_store.clone(), @@ -196,7 +205,7 @@ mod tests { .spawn(LORE_CONTEXT.scope(execution.clone(), async move { let tempdir = generate_tempdir(); let path = tempdir.to_path_buf(); - let repository = create_repository( + let (repository, _write_token) = create_repository( path.as_path(), repository_id, immutable_store.clone(), @@ -257,4 +266,100 @@ mod tests { .await .expect("Test task panicked"); } + + /// A directory that was already *committed* into the parent tree keeps + /// being tracked, and its contents keep being descended into and indexed, + /// even after a `.lore/` control directory appears inside it — + /// mirroring how git does not silently untrack previously committed + /// content just because a nested `.git` shows up. The auto-discard + /// ("zombie" cleanup) only applies to never-committed staged entries; + /// untracking a committed nested repository is left to an explicit user + /// action, not this scan. + #[tokio::test] + async fn committed_directory_becoming_nested_repository_stays_tracked() { + let (immutable_store, mutable_store, execution) = + test_store_create().await.expect("Failed to create stores"); + let repository_id = RepositoryId::from(uuid::Uuid::now_v7()); + + runtime() + .spawn(LORE_CONTEXT.scope(execution.clone(), async move { + let tempdir = generate_tempdir(); + let path = tempdir.to_path_buf(); + let (repository, write_token) = create_repository( + path.as_path(), + repository_id, + immutable_store.clone(), + mutable_store.clone(), + ) + .await; + + // A plain directory with a file, staged and committed into the + // parent tree — so it is present in state_current, not just + // state_staged. + std::fs::create_dir(path.join("nested").as_path()) + .expect("Create nested directory failed"); + let _ = create_file(path.join("nested").join("inner.txt").as_path(), &[1, 2, 3]); + + let paths = LoreArray::from_vec(vec![LoreString::from(&path)]); + file::stage::stage( + repository.clone(), + &write_token, + paths, + StageOptions { + case_change: stage::StageCaseChange::Error, + node_flags: NodeFlags::NoFlags, + file_id: None, + no_children: false, + scan: true, + }, + ) + .await + .expect("Stage failed"); + + Box::pin(commit::commit( + repository.clone(), + &write_token, + CommitOptions::new("Commit nested directory".to_string()), + )) + .await + .expect("Commit failed"); + + let (current_revision, _branch) = + lore_revision::instance::load_current_anchor(&repository) + .await + .expect("Failed to load current anchor"); + let state_current = state::State::deserialize(repository.clone(), current_revision) + .await + .expect("Failed to deserialize current state"); + let state_staged = state::State::deserialize(repository.clone(), current_revision) + .await + .expect("Failed to deserialize staged state"); + + // The already-committed directory becomes a nested repository + // root, as when `lore repository create` runs inside it. + std::fs::create_dir(path.join("nested").join(DOT_LORE).as_path()) + .expect("Create nested/.lore directory failed"); + + // Modify the already-tracked file so an "unmodified, no diff" + // scan result can't be mistaken for the boundary having + // silently swallowed it: if the parent still descends into + // and indexes `nested/`, this modification must surface. + let _ = create_file(path.join("nested").join("inner.txt").as_path(), &[4, 5, 6]); + + let changes = scan(repository.clone(), state_staged, state_current).await; + assert!( + changes + .iter() + .any(|c| c.path.as_str() == "nested/inner.txt"), + "a directory committed before becoming a nested repository root \ + must stay tracked, with its contents still indexed, found: {:?}", + changes + .iter() + .map(|c| c.path.as_str().to_string()) + .collect::>() + ); + })) + .await + .expect("Test task panicked"); + } }