From dd1d59b6053bee00f3689c85965f0c772c2b53aa Mon Sep 17 00:00:00 2001 From: Aaron Ogle Date: Thu, 7 May 2026 16:09:47 -0500 Subject: [PATCH 1/5] fix(status): detect modifications when FILE_INDEX entry is missing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A tracked file that lands in the working copy via `atomic insert`, `atomic clone`, or `atomic view switch` has no FILE_INDEX entry — those code paths materialize content but only `record()` and `materialize_view()` populate the index. When that file was later modified, `status()` looked up FILE_INDEX, found nothing, and fell through with an "Assume clean" comment. The modification became invisible to `atomic status`, `atomic diff`, and `atomic record -a`. Concretely this caused agent turns to silently drop edits to already-tracked files: the agent record path uses `repo.record(all=true)`, which calls `status(default())` internally, which dropped the entry, which made the file invisible to filter_files. Turns that mixed new files with edits to tracked files recorded only the new files. Replace the silent fallthrough with a conservative Modified entry. When `hash_contents=true`, hash the file (consistent with the existing fast-path branch when mtime+size differ). When false, emit the entry without hashing — same shape as the existing "mtime changed but no hash" branch. The recording workflow already handles false positives: `record_modified_file` produces an empty hunk for content that matches pristine, and the file is filtered out by `recorded.is_empty()`. Subsequent records re-populate FILE_INDEX, returning the file to the fast path. Test added in status_tests.rs records a file (populating FILE_INDEX), drops the entry to simulate the post-insert state, modifies the file, and asserts status() reports it as Modified. Fails against the previous code with entries=[]. --- atomic-repository/src/repository/status.rs | 45 ++++++++++++- .../src/repository/tests/status_tests.rs | 65 +++++++++++++++++++ 2 files changed, 108 insertions(+), 2 deletions(-) diff --git a/atomic-repository/src/repository/status.rs b/atomic-repository/src/repository/status.rs index c27b3a2..2e4c7e1 100644 --- a/atomic-repository/src/repository/status.rs +++ b/atomic-repository/src/repository/status.rs @@ -307,8 +307,49 @@ impl Repository { } // No FILE_INDEX entry — file is tracked with graph content - // but was never indexed. Assume clean (can't compare without - // reconstructing graph content, which is expensive). + // but was never indexed. This happens after `atomic insert`, + // `atomic clone`, or `atomic view switch` materializes a file + // into the working copy without going through `record()` (only + // record/materialize_view populate FILE_INDEX today). + // + // We CANNOT silently treat this as Clean: that would let real + // edits to such files become invisible to status/diff/record, + // and `record(all=true)` would silently drop them. + // + // Conservative correctness: mark Modified so the caller can + // run a full diff against pristine. If the file is actually + // unchanged, the recording workflow produces an empty hunk + // and skips it (record_modified_file returns is_empty()). + // Subsequent records re-populate FILE_INDEX, returning the + // file to the fast path. + if options.hash_contents { + hash_count += 1; + let mut entry = FileStatusEntry::new(path.clone(), FileStatus::Modified); + if let Some(inode) = inode { + entry.set_inode(inode); + } + match hash_file_contents(&abs_path) { + Ok(current_hash) => { + entry.set_current_hash(current_hash); + } + Err(_) => { + entry.set_details("Unable to read file contents".to_string()); + } + } + entry.set_details("FILE_INDEX entry missing".to_string()); + status.add_entry(entry); + } else { + // Fast mode: skip the hash but still surface the entry so + // it isn't silently dropped. Callers using fast mode (e.g. + // the agent record path) re-query with hash_contents=true + // before recording. + let mut entry = FileStatusEntry::new(path.clone(), FileStatus::Modified); + if let Some(inode) = inode { + entry.set_inode(inode); + } + entry.set_details("FILE_INDEX entry missing".to_string()); + status.add_entry(entry); + } } let classify_ms = classify_start.elapsed().as_millis(); diff --git a/atomic-repository/src/repository/tests/status_tests.rs b/atomic-repository/src/repository/tests/status_tests.rs index ec1a649..363ac17 100644 --- a/atomic-repository/src/repository/tests/status_tests.rs +++ b/atomic-repository/src/repository/tests/status_tests.rs @@ -373,3 +373,68 @@ fn test_repo_status_ignores_node_modules_no_trailing_newline() { "Should include app.js" ); } + +#[test] +fn test_status_detects_modification_when_file_index_missing() { + // Repro for the silent-data-loss bug: when a tracked file has no + // FILE_INDEX entry (e.g. after `atomic insert` / `atomic clone` / + // `atomic view switch` materialized it without going through `record`), + // `status()` falls through with the "Assume clean" comment and the + // file becomes invisible to `status`, `diff`, and `record -a`. Any + // edits the user (or an agent) makes to that file are silently + // dropped on the next record. + // + // This test pins the correct behavior: status() must detect the + // modification regardless of whether the index has been populated. + use crate::record::RecordOptions; + use crate::status::FileStatus; + + let (temp_dir, repo) = create_temp_repo(); + + // Step 1: create + record a tracked file. Recording with + // apply_after_record (the default) populates FILE_INDEX for this file. + let file_path = temp_dir.path().join("tracked.txt"); + std::fs::write(&file_path, b"original content").unwrap(); + repo.add("tracked.txt", TrackingOptions::default()).unwrap(); + let header = ChangeHeader::new("Add tracked.txt"); + repo.record(header, RecordOptions::new().with_all(true)) + .expect("initial record failed"); + + // Step 2: drop the FILE_INDEX entry to simulate the post-insert / + // post-clone / post-switch state. In production, those code paths + // materialize files into the working copy without writing FILE_INDEX + // entries — only `record()` and `materialize_view()` populate it. + repo.del_file_index("tracked.txt") + .expect("del_file_index failed"); + + // Step 3: modify the file on disk. mtime, size, and content all change. + std::fs::write(&file_path, b"modified content (different size)").unwrap(); + + // Step 4: status() must report it as Modified. + let status = repo + .status(StatusOptions::default()) + .expect("status failed"); + + let modified_paths: Vec = status + .entries() + .iter() + .filter(|e| e.status() == FileStatus::Modified) + .map(|e| e.path().to_string_lossy().to_string()) + .collect(); + + assert!( + modified_paths.iter().any(|p| p == "tracked.txt"), + "status() must detect modifications to tracked files even when \ + FILE_INDEX has no entry for them. \ + entries={:?}", + status + .entries() + .iter() + .map(|e| (e.path().to_string_lossy().to_string(), e.status())) + .collect::>() + ); + assert!( + !status.is_clean(), + "status() must not be clean when a tracked file is modified" + ); +} From b2064c226e772829b839b0148fe5365fc180aea2 Mon Sep 17 00:00:00 2001 From: Aaron Ogle Date: Thu, 7 May 2026 16:20:56 -0500 Subject: [PATCH 2/5] fix(switch): clean status after view switch with sibling changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two related bugs caused `atomic status` to lie about the working copy immediately after `atomic view switch`. The disk was correct (materialize had restored the destination view's content), but status reported phantom `Deleted` entries for files that only exist on sibling views and false `Modified` entries for files that materialize had just rewritten. Test added in integration_tests.rs records alpha.txt + bravo.txt on dev, splits feature off dev, on feature edits alpha.txt and adds delta.txt and records, then switches back to dev. Asserts status().is_clean() after the switch. Failed on the parent commit with [("delta.txt", Deleted), ("alpha.txt", Modified)]. 1) status.rs — drop the unsound "universal filter" fast path For `is_shared() && parent.is_none()` views, status was skipping the view-change-id filter on the assumption that ALL changes in GRAPH belonged to the view. That assumption breaks the moment `atomic split` creates a sibling view: the dev view is still shared+root, but it no longer contains every change — feature's record introduces TREE entries (e.g. delta.txt's inode_position pointing to feature's change) that must NOT surface in dev's status. Always compute `current_view_change_ids` via `collect_visible_change_ids_with_deps` and apply the filter. Cost is one O(C) B-tree scan per status call where C is changes on the view — bounded and fast even on large repos. Preserve the legacy "show everything" fallback when no current view exists by using `Option>`. 2) materialize.rs — actually populate FILE_INDEX after materialize `populate_file_index` iterated `result.file_results.keys()` to update FILE_INDEX with current on-disk hashes. But `materialize_view` calls `merge_file_result(_, store_result=false)` at its only production call site, so `file_results` is always empty in practice and this function silently no-op'd. FILE_INDEX retained whatever hashes the previous view had recorded, so post-switch `status()` would hash alpha.txt on disk, find it differs from the cached (sibling-view) hash, and report Modified — even though materialize had just written the destination view's correct content. Iterate `list_tracked_files()` instead. Materialize has just synced the working copy to the destination view's recorded state, so hashing the on-disk content for each tracked file produces the correct authoritative baseline for FILE_INDEX. Cost is bounded by tracked-file count, same order as materialize itself. Together: the view filter excludes sibling-only TREE entries (no more phantom Deleted), and the FILE_INDEX refresh keeps cached hashes consistent with materialized disk content (no more false Modified). --- .../src/repository/materialize.rs | 70 ++++++++---- atomic-repository/src/repository/status.rs | 59 +++++------ .../src/repository/tests/integration_tests.rs | 100 ++++++++++++++++++ 3 files changed, 176 insertions(+), 53 deletions(-) diff --git a/atomic-repository/src/repository/materialize.rs b/atomic-repository/src/repository/materialize.rs index aa40dcf..df43012 100644 --- a/atomic-repository/src/repository/materialize.rs +++ b/atomic-repository/src/repository/materialize.rs @@ -146,32 +146,58 @@ impl Repository { /// # Returns /// /// Statistics about the materialize operation. - /// Populate the file index for all files in a materialize result. + /// Populate the file index for all tracked files after a materialize. /// - /// Stats each written file from disk and stores its mtime + size + - /// content hash in the pristine database. Errors are silently ignored + /// Stats each tracked file from disk and stores its mtime + size + + /// content hash in the pristine database. Errors are silently ignored /// (best-effort). - fn populate_file_index(&self, result: &MaterializeResult) { + /// + /// Why we ignore `result.file_results` and walk the tracked set: the + /// `MaterializeResult.file_results` map is only populated when + /// `merge_file_result(_, store_result=true)` is called, and the + /// `materialize_view` call site in `atomic-core` passes `false`. As a + /// result `result.file_results.keys()` is empty in production, and the + /// previous implementation of this function silently no-op'd — + /// FILE_INDEX was never refreshed by materialize, leaving stale + /// per-view hashes after `view switch` and producing false `Modified` + /// reports from `status`. + /// + /// Walking `list_tracked_files()` is correct because materialize has + /// just brought the working copy into sync with the destination + /// view's recorded state — every tracked file's on-disk content is + /// the authoritative baseline FILE_INDEX should cache. + fn populate_file_index(&self, _result: &MaterializeResult) { use std::time::SystemTime; - let mut entries: Vec<(String, i64, u32, u64, Hash)> = Vec::new(); - - for path in result.file_results.keys() { - let abs_path = self.root.join(path); - if let Ok(metadata) = std::fs::metadata(&abs_path) { - let mtime = metadata.modified().unwrap_or(SystemTime::UNIX_EPOCH); - let duration = mtime - .duration_since(SystemTime::UNIX_EPOCH) - .unwrap_or_default(); - let secs = duration.as_secs() as i64; - let nanos = duration.subsec_nanos(); - let size = metadata.len(); - let content_hash = std::fs::read(&abs_path) - .map(|bytes| Hash::of(&bytes)) - .unwrap_or(Hash::ZERO); - let normalized = path.replace('\\', "/"); - entries.push((normalized, secs, nanos, size, content_hash)); - } + let tracked = match self.list_tracked_files() { + Ok(t) => t, + Err(_) => return, + }; + + let mut entries: Vec<(String, i64, u32, u64, Hash)> = Vec::with_capacity(tracked.len()); + + for file in &tracked { + let abs_path = self.root.join(&file.path); + let metadata = match std::fs::metadata(&abs_path) { + Ok(m) if m.is_file() => m, + _ => continue, + }; + + let mtime = metadata.modified().unwrap_or(SystemTime::UNIX_EPOCH); + let duration = mtime + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap_or_default(); + let secs = duration.as_secs() as i64; + let nanos = duration.subsec_nanos(); + let size = metadata.len(); + + let content_hash = match std::fs::read(&abs_path) { + Ok(bytes) => Hash::of(&bytes), + Err(_) => continue, + }; + + let normalized = file.path.to_string_lossy().replace('\\', "/"); + entries.push((normalized, secs, nanos, size, content_hash)); } if !entries.is_empty() { diff --git a/atomic-repository/src/repository/status.rs b/atomic-repository/src/repository/status.rs index 2e4c7e1..de924fc 100644 --- a/atomic-repository/src/repository/status.rs +++ b/atomic-repository/src/repository/status.rs @@ -40,28 +40,33 @@ impl Repository { // ── View-aware filtering ─────────────────────────────────────── // - // Fast path: for a Shared view with no parent (the common case - // after `atomic init` or `atomic git import`), ALL changes in - // GRAPH are visible. Skip the expensive O(N) scan entirely. + // Build the effective view filter entirely from pristine indexes. + // Do not load `.change` files here: status must stay proportional + // to working-copy/index state, not object-store history size. // - // Slow path: for Draft views or views with parents, we need the - // actual filter set to hide changes from other views. - let (current_view_change_ids, filter_is_universal) = if let Some(ref view) = txn + // We always compute the filter, even for shared root views. The + // previous "universal" fast-path (skip filter for is_shared() && + // parent.is_none()) was unsound after `atomic split`: the dev view + // is still shared with no parent, but it no longer contains every + // change in the repo — sibling draft/shared views may have unique + // changes. Skipping the filter caused TREE entries created by + // those sibling changes to surface as phantom `Deleted` files in + // dev's status (their inode_position pointed to a change dev + // doesn't have). + // + // The filter computation is O(C) where C is changes on the view — + // a single B-tree scan, fast even on large repos. + // + // None means "no current view" (a misconfigured repo); preserve + // the legacy "show everything" behavior in that case rather than + // producing an empty status. + let current_view_change_ids: Option> = if let Some(ref view) = txn .get_view(&self.current_view) .map_err(|e| RepositoryError::Database(e.to_string()))? { - if view.kind.is_shared() && view.parent.is_none() { - (HashSet::new(), true) - } else { - // Build the effective view filter entirely from pristine - // indexes. Do not load `.change` files here: status must stay - // proportional to working-copy/index state, not object-store - // history size. - let ids = collect_visible_change_ids_with_deps(&txn, view)?; - (ids, false) - } + Some(collect_visible_change_ids_with_deps(&txn, view)?) } else { - (HashSet::new(), true) + None }; let phase1_ms = overall_start.elapsed().as_millis(); @@ -86,21 +91,13 @@ impl Repository { let (path, inode) = result.map_err(|e| RepositoryError::Database(e.to_string()))?; // View filter: skip files whose creating change is not on - // the current view. - // Check inode_position for every file: - // - For non-universal views: also apply the view filter - // - For universal views: just determine has_graph - // - // This is correct and required — files in TREE without a - // graph position must be classified as Added, not Clean. + // the current view. Files in TREE without a graph position + // must be classified as Added, not Clean (caller-side logic). let has_graph = if let Ok(Some(position)) = txn.inode_position(inode) { - // View filter: skip files whose creating change is not - // on the current view. - if !filter_is_universal - && !position.change.is_root() - && !current_view_change_ids.contains(&position.change) - { - continue; + if let Some(ref ids) = current_view_change_ids { + if !position.change.is_root() && !ids.contains(&position.change) { + continue; + } } true } else { diff --git a/atomic-repository/src/repository/tests/integration_tests.rs b/atomic-repository/src/repository/tests/integration_tests.rs index dc779a0..814d350 100644 --- a/atomic-repository/src/repository/tests/integration_tests.rs +++ b/atomic-repository/src/repository/tests/integration_tests.rs @@ -393,3 +393,103 @@ fn test_switch_view_shows_view_content() { "Content should be feature version after switching to feature" ); } + +/// Repro for the post-switch phantom-Deleted / false-Modified bug. +/// +/// Scenario: +/// 1. Record alpha.txt + bravo.txt on dev. +/// 2. Split feature off dev, switch to feature. +/// 3. On feature: edit alpha.txt and add delta.txt; record. +/// 4. Switch back to dev. +/// +/// At step 4 the working copy is correct (alpha.txt has dev's original +/// content, delta.txt is gone), and dev's recorded state is also correct. +/// Yet `status()` reports `alpha.txt: Modified` and `delta.txt: Deleted`. +/// +/// Cause: `switch_view` updates the disk via `materialize` and removes +/// stale files from the working copy, but it does not reconcile TREE +/// or FILE_INDEX with the destination view's recorded state. +/// - delta.txt remains in TREE (it was added by feature's record on a +/// globally-shared TREE), and dev's status filter is "universal" for +/// a no-parent shared view, so iter_tree surfaces delta.txt → since +/// it's not on disk, it's reported Deleted. +/// - alpha.txt's FILE_INDEX entry still holds feature's hash; status +/// hashes the disk content, sees it differs from the cached hash, +/// and reports Modified. +/// +/// The correct post-switch invariant is: if no working-copy edits have +/// happened since the switch, `status().is_clean()` must hold and there +/// must be no phantom Deleted or false Modified entries. +#[test] +fn test_status_clean_after_view_switch_with_sibling_changes() { + let (temp_dir, mut repo) = create_temp_repo(); + + // Step 1: record alpha.txt + bravo.txt on dev. + std::fs::write(temp_dir.path().join("alpha.txt"), b"alpha-original\n").unwrap(); + std::fs::write(temp_dir.path().join("bravo.txt"), b"bravo-original\n").unwrap(); + repo.add("alpha.txt", TrackingOptions::default()).unwrap(); + repo.add("bravo.txt", TrackingOptions::default()).unwrap(); + repo.record( + ChangeHeader::new("Add alpha + bravo on dev"), + RecordOptions::new().with_all(true), + ) + .unwrap(); + + // Step 2: split feature off dev and switch to it. + repo.create_view_from("feature", "dev").unwrap(); + repo.switch_view("feature").unwrap(); + + // Step 3: on feature, modify alpha.txt and add delta.txt; record. + std::fs::write( + temp_dir.path().join("alpha.txt"), + b"alpha-modified-on-feature\n", + ) + .unwrap(); + std::fs::write(temp_dir.path().join("delta.txt"), b"delta-feature\n").unwrap(); + repo.add("delta.txt", TrackingOptions::default()).unwrap(); + repo.record( + ChangeHeader::new("Edit alpha + add delta on feature"), + RecordOptions::new().with_all(true), + ) + .unwrap(); + + // Step 4: switch back to dev. + repo.switch_view("dev").unwrap(); + + // Sanity-check the disk before checking status. The switch should + // have restored alpha.txt to dev's content and removed delta.txt. + let alpha_on_disk = std::fs::read(temp_dir.path().join("alpha.txt")).unwrap(); + assert_eq!( + alpha_on_disk, b"alpha-original\n", + "switch_view must restore alpha.txt to dev's content" + ); + assert!( + !temp_dir.path().join("delta.txt").exists(), + "switch_view must remove delta.txt (not in dev) from the working copy" + ); + + // The actual assertion: status must reflect the disk truth. + let status = repo + .status(StatusOptions::default()) + .expect("status failed"); + + // Collect any non-clean entries for diagnostics. + let dirty: Vec<(String, crate::status::FileStatus)> = status + .entries() + .iter() + .filter(|e| e.status().is_dirty()) + .map(|e| (e.path().to_string_lossy().to_string(), e.status())) + .collect(); + + assert!( + dirty.is_empty(), + "status() after view switch must be clean — disk and view state agree, \ + but status reported phantom dirty entries: {:?}", + dirty + ); + assert!( + status.is_clean(), + "status().is_clean() must hold immediately after a view switch with no \ + working-copy edits" + ); +} From 82b4e57e2b4e0afa013ec909728ee85e93f83f04 Mon Sep 17 00:00:00 2001 From: Lee Faus Date: Thu, 7 May 2026 17:47:14 -0400 Subject: [PATCH 3/5] combine changes for status update --- atomic-repository/src/repository/status.rs | 38 ++++++++ atomic-repository/src/repository/views.rs | 3 +- tests/harness/03_cross_stack.sh | 106 +++++++++++++++++++++ 3 files changed, 145 insertions(+), 2 deletions(-) diff --git a/atomic-repository/src/repository/status.rs b/atomic-repository/src/repository/status.rs index de924fc..022ad86 100644 --- a/atomic-repository/src/repository/status.rs +++ b/atomic-repository/src/repository/status.rs @@ -40,6 +40,7 @@ impl Repository { // ── View-aware filtering ─────────────────────────────────────── // +<<<<<<< Updated upstream // Build the effective view filter entirely from pristine indexes. // Do not load `.change` files here: status must stay proportional // to working-copy/index state, not object-store history size. @@ -67,6 +68,29 @@ impl Repository { Some(collect_visible_change_ids_with_deps(&txn, view)?) } else { None +======= + // Always build the explicit change filter. The TREE table is + // global — it contains entries from ALL views, including child + // views. Without filtering, files recorded on child views leak + // into the parent's status as false "Deleted" entries. + // + // Previously there was a fast path (`filter_is_universal = true`) + // for root shared views that skipped this scan. That optimisation + // is invalid once child views can record files into the shared + // TREE table — every TREE entry was treated as belonging to the + // current view, causing the ghost-deletion bug. + let current_view_change_ids = if let Some(ref view) = txn + .get_view(&self.current_view) + .map_err(|e| RepositoryError::Database(e.to_string()))? + { + // Build the effective view filter entirely from pristine + // indexes. Do not load `.change` files here: status must stay + // proportional to working-copy/index state, not object-store + // history size. + collect_visible_change_ids_with_deps(&txn, view)? + } else { + HashSet::new() +>>>>>>> Stashed changes }; let phase1_ms = overall_start.elapsed().as_millis(); @@ -91,6 +115,7 @@ impl Repository { let (path, inode) = result.map_err(|e| RepositoryError::Database(e.to_string()))?; // View filter: skip files whose creating change is not on +<<<<<<< Updated upstream // the current view. Files in TREE without a graph position // must be classified as Added, not Clean (caller-side logic). let has_graph = if let Ok(Some(position)) = txn.inode_position(inode) { @@ -98,6 +123,19 @@ impl Repository { if !position.change.is_root() && !ids.contains(&position.change) { continue; } +======= + // the current view. + // + // Check inode_position for every file. Files in TREE + // without a graph position must be classified as Added, not + // Clean. Files whose creating change is not in the current + // view's filter are skipped entirely — they belong to other + // views and must not appear in this view's status. + let has_graph = if let Ok(Some(position)) = txn.inode_position(inode) { + if !position.change.is_root() && !current_view_change_ids.contains(&position.change) + { + continue; +>>>>>>> Stashed changes } true } else { diff --git a/atomic-repository/src/repository/views.rs b/atomic-repository/src/repository/views.rs index 2ae6687..223bdaf 100644 --- a/atomic-repository/src/repository/views.rs +++ b/atomic-repository/src/repository/views.rs @@ -157,8 +157,7 @@ impl Repository { /// Change a view's scope between Draft and Shared. /// /// This is useful after `git import` which creates Draft views by - /// default. Changing to Shared enables the fast `filter_is_universal` - /// path in `status`, avoiding the O(N) change-loading slow path. + /// default. pub fn set_view_scope(&self, name: &str, scope: ViewScope) -> Result<(), RepositoryError> { let mut txn = self .pristine diff --git a/tests/harness/03_cross_stack.sh b/tests/harness/03_cross_stack.sh index 8c5737f..d3bd18e 100755 --- a/tests/harness/03_cross_stack.sh +++ b/tests/harness/03_cross_stack.sh @@ -1038,6 +1038,112 @@ assert_file_exists "journey.txt on feature (final check)" "journey.txt" switch_view "staging" >/dev/null 2>&1 || true assert_file_exists "journey.txt on staging (final check)" "journey.txt" +# ═══════════════════════════════════════════════════════════════════════════ +begin_section "Cross-View: Child view file NOT marked Deleted on parent (init -k rust)" +# ═══════════════════════════════════════════════════════════════════════════ +# +# Regression test for the bug: +# +# 1. atomic init -k rust (creates dev with .vault/ and .atomicignore) +# 2. Create child view "hola" (parented on dev) +# 3. On hola, add a new file, record it → status clean ✓ +# 4. Switch to dev +# 5. BUG: the file from hola shows as Deleted on dev +# 6. EXPECTED: the file should NOT appear in dev's status at all +# +# Root cause: status() has a fast-path for root shared views +# (filter_is_universal=true) that skips the change filter, so every +# TREE entry — including files from child views — is treated as +# belonging to the current view. When the file doesn't exist on disk +# (correctly removed by switch_view), status classifies it as Deleted. + +make_temp_repo "cross-child-not-deleted" +init_repo -k rust + +# dev should have .vault and .atomicignore tracked after init +assert_file_exists ".atomicignore exists on dev" ".atomicignore" +assert_dir_exists ".vault exists on dev" ".vault" + +# Verify dev is clean (init records .vault + .atomicignore) +assert_clean "dev is clean after init -k rust" + +# Create child view "hola" (parented on dev) +new_view "hola" >/dev/null 2>&1 || true +switch_view "hola" >/dev/null 2>&1 || true +assert_current_view "on hola" "hola" + +# Add a new file on hola +create_file "hola_only.txt" "this file belongs to hola" +assert_success "add hola_only.txt on hola" atomic add hola_only.txt +record_change "Add hola_only.txt on hola" >/dev/null 2>&1 || true + +# Status should be clean on hola +assert_clean "hola is clean after record" +assert_file_exists "hola_only.txt exists on hola" "hola_only.txt" + +# Switch back to dev +switch_view "dev" >/dev/null 2>&1 || true +assert_current_view "back on dev" "dev" + +# The file should NOT exist on disk (correctly handled by switch_view) +assert_file_not_exists \ + "hola_only.txt NOT on disk on dev" \ + "hola_only.txt" + +# CRITICAL: the file must NOT appear in status at all — especially not as Deleted +assert_status_no_entry \ + "hola_only.txt NOT in dev status (must not be Deleted)" \ + "hola_only.txt" + +# Double-check: dev's own files should be fine +assert_file_exists ".atomicignore still on dev" ".atomicignore" +assert_dir_exists ".vault still on dev" ".vault" +assert_clean "dev is still clean (no false Deleted entries)" + +# Switch back to hola — file should reappear +switch_view "hola" >/dev/null 2>&1 || true +assert_file_exists "hola_only.txt back on hola" "hola_only.txt" +assert_file_content "hola_only.txt has correct content" "hola_only.txt" "this file belongs to hola" +assert_clean "hola still clean after round-trip" + +# ═══════════════════════════════════════════════════════════════════════════ +begin_section "Cross-View: Multiple child views, parent sees no ghost deletions" +# ═══════════════════════════════════════════════════════════════════════════ +# +# Extend the above: create TWO child views from dev, each with unique files. +# Switching to dev should show zero Deleted entries from either child. + +make_temp_repo "cross-multi-child-no-ghost" +init_repo -k rust + +# Child view alpha +new_view "alpha" >/dev/null 2>&1 || true +switch_view "alpha" >/dev/null 2>&1 || true +create_file "alpha_file.txt" "alpha content" +assert_success "add alpha_file.txt" atomic add alpha_file.txt +record_change "Add alpha_file.txt" >/dev/null 2>&1 || true +assert_clean "alpha is clean" + +# Child view beta +switch_view "dev" >/dev/null 2>&1 || true +new_view "beta" >/dev/null 2>&1 || true +switch_view "beta" >/dev/null 2>&1 || true +create_file "beta_file.txt" "beta content" +assert_success "add beta_file.txt" atomic add beta_file.txt +record_change "Add beta_file.txt" >/dev/null 2>&1 || true +assert_clean "beta is clean" + +# Switch to dev — neither child's file should appear +switch_view "dev" >/dev/null 2>&1 || true +assert_current_view "on dev" "dev" + +assert_file_not_exists "alpha_file.txt NOT on dev" "alpha_file.txt" +assert_file_not_exists "beta_file.txt NOT on dev" "beta_file.txt" + +assert_status_no_entry "alpha_file.txt NOT in dev status" "alpha_file.txt" +assert_status_no_entry "beta_file.txt NOT in dev status" "beta_file.txt" +assert_clean "dev has no ghost deletions from children" + # ═══════════════════════════════════════════════════════════════════════════ print_summary From 2cddea4e6198e90d289b96e882a22ef630f9cb8b Mon Sep 17 00:00:00 2001 From: Lee Faus Date: Thu, 7 May 2026 17:49:40 -0400 Subject: [PATCH 4/5] fix conflicts --- atomic-repository/src/repository/status.rs | 67 +++++----------------- 1 file changed, 15 insertions(+), 52 deletions(-) diff --git a/atomic-repository/src/repository/status.rs b/atomic-repository/src/repository/status.rs index 022ad86..636d983 100644 --- a/atomic-repository/src/repository/status.rs +++ b/atomic-repository/src/repository/status.rs @@ -40,20 +40,17 @@ impl Repository { // ── View-aware filtering ─────────────────────────────────────── // -<<<<<<< Updated upstream - // Build the effective view filter entirely from pristine indexes. - // Do not load `.change` files here: status must stay proportional - // to working-copy/index state, not object-store history size. + // Always build the explicit change filter. The TREE table is + // global — it contains entries from ALL views, including child + // views. Without filtering, files recorded on child views leak + // into the parent's status as false "Deleted" entries. // - // We always compute the filter, even for shared root views. The - // previous "universal" fast-path (skip filter for is_shared() && - // parent.is_none()) was unsound after `atomic split`: the dev view - // is still shared with no parent, but it no longer contains every - // change in the repo — sibling draft/shared views may have unique - // changes. Skipping the filter caused TREE entries created by - // those sibling changes to surface as phantom `Deleted` files in - // dev's status (their inode_position pointed to a change dev - // doesn't have). + // The previous "universal" fast-path (skip filter for + // is_shared() && parent.is_none()) was unsound: the dev view is + // shared with no parent, but child/sibling views may have unique + // changes. Skipping the filter caused TREE entries created by + // those changes to surface as phantom `Deleted` files in dev's + // status. // // The filter computation is O(C) where C is changes on the view — // a single B-tree scan, fast even on large repos. @@ -68,29 +65,6 @@ impl Repository { Some(collect_visible_change_ids_with_deps(&txn, view)?) } else { None -======= - // Always build the explicit change filter. The TREE table is - // global — it contains entries from ALL views, including child - // views. Without filtering, files recorded on child views leak - // into the parent's status as false "Deleted" entries. - // - // Previously there was a fast path (`filter_is_universal = true`) - // for root shared views that skipped this scan. That optimisation - // is invalid once child views can record files into the shared - // TREE table — every TREE entry was treated as belonging to the - // current view, causing the ghost-deletion bug. - let current_view_change_ids = if let Some(ref view) = txn - .get_view(&self.current_view) - .map_err(|e| RepositoryError::Database(e.to_string()))? - { - // Build the effective view filter entirely from pristine - // indexes. Do not load `.change` files here: status must stay - // proportional to working-copy/index state, not object-store - // history size. - collect_visible_change_ids_with_deps(&txn, view)? - } else { - HashSet::new() ->>>>>>> Stashed changes }; let phase1_ms = overall_start.elapsed().as_millis(); @@ -115,27 +89,16 @@ impl Repository { let (path, inode) = result.map_err(|e| RepositoryError::Database(e.to_string()))?; // View filter: skip files whose creating change is not on -<<<<<<< Updated upstream - // the current view. Files in TREE without a graph position - // must be classified as Added, not Clean (caller-side logic). + // the current view. Files in TREE without a graph position + // must be classified as Added, not Clean. Files whose + // creating change is not in the current view's filter are + // skipped entirely — they belong to other views and must not + // appear in this view's status. let has_graph = if let Ok(Some(position)) = txn.inode_position(inode) { if let Some(ref ids) = current_view_change_ids { if !position.change.is_root() && !ids.contains(&position.change) { continue; } -======= - // the current view. - // - // Check inode_position for every file. Files in TREE - // without a graph position must be classified as Added, not - // Clean. Files whose creating change is not in the current - // view's filter are skipped entirely — they belong to other - // views and must not appear in this view's status. - let has_graph = if let Ok(Some(position)) = txn.inode_position(inode) { - if !position.change.is_root() && !current_view_change_ids.contains(&position.change) - { - continue; ->>>>>>> Stashed changes } true } else { From c2085f62c0faac20218c3f7de74ff8f1155f76c2 Mon Sep 17 00:00:00 2001 From: Lee Faus Date: Thu, 7 May 2026 18:13:08 -0400 Subject: [PATCH 5/5] Add stale FILE_INDEX detection and user hints Track when status encounters files with missing FILE_INDEX entries (conservatively reported as Modified) and offer `--reindex` suggestion. Two new methods on `RepositoryStatus`: - `needs_reindex()` - whether any entries were marked stale - `stale_index_count()` - count of affected files The status command now prints a hint when stale entries are detected, guiding users toward the fix. Also adds regression tests for false-Modified bug after child views record changes, ensuring parent view files remain clean after switching back. --- atomic-cli/src/commands/status.rs | 16 +++ atomic-repository/src/repository/status.rs | 1 + atomic-repository/src/status.rs | 27 +++++ tests/harness/03_cross_stack.sh | 131 +++++++++++++++++++++ 4 files changed, 175 insertions(+) diff --git a/atomic-cli/src/commands/status.rs b/atomic-cli/src/commands/status.rs index e172ee0..46c8c6d 100644 --- a/atomic-cli/src/commands/status.rs +++ b/atomic-cli/src/commands/status.rs @@ -345,6 +345,22 @@ impl Status { print_blank(); } + // Hint: stale FILE_INDEX detected + if status.needs_reindex() { + println!( + " {}", + hint(&format!( + "hint: {} file(s) reported as modified due to a stale index.", + status.stale_index_count() + )) + ); + println!( + " {}", + hint("Run \"atomic status --reindex\" to rebuild the file index.") + ); + print_blank(); + } + // Print summary hint if has_changes { print_hint("Use \"atomic record\" to record your changes"); diff --git a/atomic-repository/src/repository/status.rs b/atomic-repository/src/repository/status.rs index 636d983..a3a8f9d 100644 --- a/atomic-repository/src/repository/status.rs +++ b/atomic-repository/src/repository/status.rs @@ -320,6 +320,7 @@ impl Repository { // and skips it (record_modified_file returns is_empty()). // Subsequent records re-populate FILE_INDEX, returning the // file to the fast path. + status.add_stale_index_hit(); if options.hash_contents { hash_count += 1; let mut entry = FileStatusEntry::new(path.clone(), FileStatus::Modified); diff --git a/atomic-repository/src/status.rs b/atomic-repository/src/status.rs index a96cce3..2e3a7b3 100644 --- a/atomic-repository/src/status.rs +++ b/atomic-repository/src/status.rs @@ -399,6 +399,14 @@ pub struct RepositoryStatus { /// Index of entries by path for quick lookup. path_index: HashMap, + + /// Number of tracked files that had no FILE_INDEX entry. + /// + /// These files are conservatively reported as Modified because we + /// cannot confirm they match the recorded graph content without + /// hashing. A non-zero value means `atomic status --reindex` + /// would likely resolve the false positives. + stale_index_count: usize, } impl RepositoryStatus { @@ -414,6 +422,7 @@ impl RepositoryStatus { state, entries: Vec::new(), path_index: HashMap::new(), + stale_index_count: 0, } } @@ -430,6 +439,7 @@ impl RepositoryStatus { state, entries: Vec::with_capacity(capacity), path_index: HashMap::with_capacity(capacity), + stale_index_count: 0, } } @@ -448,6 +458,23 @@ impl RepositoryStatus { self.state.as_ref() } + /// Number of tracked files that were reported as Modified only because + /// their FILE_INDEX entry was missing. When non-zero, running + /// `atomic status --re-index` will likely resolve the false positives. + pub fn stale_index_count(&self) -> usize { + self.stale_index_count + } + + /// Whether the FILE_INDEX appears stale (some entries are missing). + pub fn needs_reindex(&self) -> bool { + self.stale_index_count > 0 + } + + /// Increment the stale-index counter. + pub fn add_stale_index_hit(&mut self) { + self.stale_index_count += 1; + } + /// Add a file status entry. pub fn add_entry(&mut self, entry: FileStatusEntry) { let index = self.entries.len(); diff --git a/tests/harness/03_cross_stack.sh b/tests/harness/03_cross_stack.sh index d3bd18e..97e038d 100755 --- a/tests/harness/03_cross_stack.sh +++ b/tests/harness/03_cross_stack.sh @@ -1144,6 +1144,137 @@ assert_status_no_entry "alpha_file.txt NOT in dev status" "alpha_file.txt" assert_status_no_entry "beta_file.txt NOT in dev status" "beta_file.txt" assert_clean "dev has no ghost deletions from children" +# ═══════════════════════════════════════════════════════════════════════════ +begin_section "Cross-View: Parent's own files NOT falsely Modified after child records" +# ═══════════════════════════════════════════════════════════════════════════ +# +# Regression test for the false-Modified bug: +# +# After fixing the ghost-deletion bug (filter_is_universal removal), +# the view change filter must include ALL changes that belong to the +# current view. If the filter is incomplete (e.g. dependency closure +# misses some changes), files whose inode was created by a missing +# change are skipped during the TREE scan and then fall through to +# the "no FILE_INDEX entry" path, where they are conservatively +# classified as Modified. +# +# This test verifies that dev's own recorded files remain Clean after +# a child view records new content. The child's record must not +# disturb the parent's change filter completeness. +# +# Scenario: +# 1. atomic init -k rust → dev has .atomicignore, .vault/*, etc. +# 2. Record additional files on dev → status clean +# 3. Create child view, record a file there +# 4. Switch back to dev +# 5. EXPECTED: dev is fully clean (no Deleted, no Modified) + +make_temp_repo "cross-no-false-modified" +init_repo -k rust + +# dev has .atomicignore and .vault tracked after init +assert_clean "dev is clean after init" + +# Record a couple more files on dev to increase surface area +create_file "src/main.rs" 'fn main() { println!("hello"); }' +create_file "Cargo.toml" '[package]\nname = "test"' +assert_success "add src/main.rs" atomic add src/main.rs +assert_success "add Cargo.toml" atomic add Cargo.toml +record_change "Add rust skeleton on dev" >/dev/null 2>&1 || true +assert_clean "dev clean after recording rust skeleton" + +# Snapshot dev status: capture the short output to verify later +dev_status_before="$(get_status_short)" + +# Create child view, record a file there +new_view "child-feat" >/dev/null 2>&1 || true +switch_view "child-feat" >/dev/null 2>&1 || true +assert_current_view "on child-feat" "child-feat" + +create_file "feature.rs" "fn feature() {}" +assert_success "add feature.rs on child" atomic add feature.rs +record_change "Add feature.rs on child-feat" >/dev/null 2>&1 || true +assert_clean "child-feat is clean after record" + +# Switch back to dev +switch_view "dev" >/dev/null 2>&1 || true +assert_current_view "back on dev" "dev" + +# CRITICAL: dev's own files must NOT show as Modified +assert_status_not_contains \ + "no 'modified:' in dev status (no false Modified regression)" \ + "modified:" + +assert_status_not_contains \ + "no 'deleted:' in dev status (no ghost deletions)" \ + "deleted:" + +# Assert specific files are NOT in short status output +assert_status_no_entry ".atomicignore not dirty" ".atomicignore" +assert_status_no_entry "src/main.rs not dirty" "src/main.rs" +assert_status_no_entry "Cargo.toml not dirty" "Cargo.toml" +assert_status_no_entry "feature.rs not in dev status" "feature.rs" + +# The gold standard: dev is fully clean +assert_clean "dev is fully clean (no Deleted, no Modified)" + +# Verify files are on disk and correct +assert_file_exists "src/main.rs on dev" "src/main.rs" +assert_file_exists "Cargo.toml on dev" "Cargo.toml" +assert_file_not_exists "feature.rs NOT on dev" "feature.rs" + +# Round-trip: child-feat still works +switch_view "child-feat" >/dev/null 2>&1 || true +assert_file_exists "feature.rs back on child-feat" "feature.rs" +assert_file_exists "src/main.rs inherited on child-feat" "src/main.rs" +assert_clean "child-feat still clean after round-trip" + +# ═══════════════════════════════════════════════════════════════════════════ +begin_section "Cross-View: Insert to parent does not cause false Modified" +# ═══════════════════════════════════════════════════════════════════════════ +# +# Simulate the pull scenario: insert changes from a child view into +# dev (which is what pull does under the hood — apply + materialize). +# After the insert + materialize, dev should be clean. + +make_temp_repo "cross-insert-no-false-modified" +init_repo -k rust + +# Record a base file on dev +create_file "base.rs" "fn base() {}" +assert_success "add base.rs" atomic add base.rs +record_change "Add base.rs on dev" >/dev/null 2>&1 || true +assert_clean "dev clean with base.rs" + +# Create child view, record new files +new_view "contributor" >/dev/null 2>&1 || true +switch_view "contributor" >/dev/null 2>&1 || true + +create_file "new_feature.rs" "fn new_feature() {}" +assert_success "add new_feature.rs" atomic add new_feature.rs +record_change "Add new_feature.rs on contributor" >/dev/null 2>&1 || true +assert_clean "contributor is clean" + +# Insert from contributor → dev (simulates what pull does) +insert_from_view "contributor" "dev" >/dev/null 2>&1 || true + +# Switch to dev to trigger materialize +switch_view "dev" >/dev/null 2>&1 || true +assert_current_view "on dev after insert" "dev" + +# Both files should exist on dev now +assert_file_exists "base.rs on dev" "base.rs" +assert_file_exists "new_feature.rs on dev after insert" "new_feature.rs" + +# CRITICAL: dev must be fully clean after insert + switch +assert_status_not_contains \ + "no 'modified:' after insert" \ + "modified:" +assert_status_not_contains \ + "no 'deleted:' after insert" \ + "deleted:" +assert_clean "dev is clean after insert from child" + # ═══════════════════════════════════════════════════════════════════════════ print_summary