Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
51 changes: 51 additions & 0 deletions lore-revision/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -5937,6 +5970,24 @@ async fn diff_filesystem_directory_walk(
continue;
};

// Discard reverted uncommitted directories (staged then removed from disk before commit) to match the filesystem.
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).
Expand Down
226 changes: 226 additions & 0 deletions lore-revision/tests/reverted_directory.rs
Original file line number Diff line number Diff line change
@@ -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<dyn lore_storage::ImmutableStore>,
mutable_store: Arc<dyn lore_storage::MutableStore>,
) -> Arc<RepositoryContext> {
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<RepositoryContext>,
state_staged: Arc<state::State>,
state_current: Arc<state::State>,
) -> Vec<lore_revision::change::NodeChange> {
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::<Vec<_>>()
);
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::<Vec<_>>()
);

// 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::<Vec<_>>()
);

// 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::<Vec<_>>()
);
}))
.await
.expect("Test task panicked");
}
}
87 changes: 87 additions & 0 deletions scripts/test/test_dirty.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import json
import logging
import os
import shutil

import pytest
from lore_parsers import parse_jsonl, parse_status_json, parse_status_summary_json
Expand Down Expand Up @@ -2806,3 +2807,89 @@ def check(label: str, **kwargs) -> None:
repo.dirty(added, offline=True)
check("second dirty, plain status")
check("second dirty, --check-dirty", check_dirty=True)


@pytest.mark.smoke
def test_scan_discards_reverted_uncommitted_directory(new_lore_repo):
"""Verify that `status --scan` discards reverted uncommitted directories.

A directory is staged, then removed from disk before any commit -- a
"zombie" entry that is in staged state but neither committed nor on disk.
Only `--scan` walks the filesystem and can discard it (see
lore-revision/src/repository/status.rs: the filesystem diff, and with it
the discard logic, only runs `if show_scan`).

The scan that performs the discard still reports its *own* output from
the staged snapshot captured before the discard was persisted (a
pre-existing report/persist ordering quirk, not part of what's under
test here) -- so the discard is only observable on a subsequent status
call. Verify that after one scan runs:
- A follow-up status call shows the reverted directory and everything
staged under it gone entirely (no add, delete, or other action
references it) -- checked with no `type` filter, since the directory
node itself (not just the files under it) is what the discard logic
operates on
- No duplicate entries are reported
- An unrelated concurrent change is still correctly reported
- A second scan does not resurrect the discarded directory
"""
repo: Lore = new_lore_repo()

with repo.open_file("base.txt", "w+") as f:
f.write("base file\n")
repo.stage(scan=True, offline=True)
repo.commit(offline=True)

os.makedirs(os.path.join(repo.path, "reverted_dir", "subdir"), exist_ok=True)
with repo.open_file("reverted_dir/file1.txt", "w+") as f:
f.write("file1\n")
with repo.open_file("reverted_dir/subdir/file2.txt", "w+") as f:
f.write("file2\n")
repo.stage(scan=True, offline=True)

with repo.open_file("other_change.txt", "w+") as f:
f.write("other\n")
repo.dirty("other_change.txt", offline=True)

# Reverts the staged add: the directory is now staged but neither
# committed nor present on disk.
shutil.rmtree(os.path.join(repo.path, "reverted_dir"))

def reverted_dir_entries(entries: list[dict]) -> list[str]:
paths = [to_posix(e.get("path", "")) for e in entries]
return [p for p in paths if p == "reverted_dir" or p.startswith("reverted_dir/")]

# The discard queued by this scan is only observable on a later call
# (see docstring), so its own output isn't asserted on.
get_status_files(repo, scan=True)

entries = get_status_files(repo, scan=True)
all_paths = [to_posix(e.get("path", "")) for e in entries]

# No `type` filter here: the directory node itself, not just the files
# under it, is what the discard logic operates on.
assert not reverted_dir_entries(entries), (
f"Reverted directory should be fully discarded from status, "
f"got: {reverted_dir_entries(entries)}"
)
assert len(all_paths) == len(set(all_paths)), (
f"Duplicate entries reported: {sorted(all_paths)}"
)

other_entries = [e for e in entries if to_posix(e.get("path", "")) == "other_change.txt"]
assert len(other_entries) == 1, (
f"other_change.txt should be reported once, got: {other_entries}"
)
other_entry = other_entries[0]
assert other_entry.get("action") == "add", (
f"other_change.txt should be marked as add: {other_entry}"
)
assert other_entry.get("flagDirty") is True, (
f"other_change.txt should be flagDirty: {other_entry}"
)

entries_again = get_status_files(repo, scan=True)
assert not reverted_dir_entries(entries_again), (
f"Later scan resurrected the reverted directory: "
f"{reverted_dir_entries(entries_again)}"
)