From edfb52f29cf749378e73a292759eb77afd7c3e4d Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Wed, 29 Jul 2026 18:51:41 -0400 Subject: [PATCH] test(stage2a): make three bites bite, and correct one framing claim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Running the bites found three that did not falsify anything. Item 28's rename row cannot pin the walk's containment rule: `reconcile_rename` calls `Path::strip_prefix` to rebuild a descendant's tail, and that is component-aware too, so a string-prefix walk is silently corrected a second time. Deletion has no such second guard — the walk's verdict IS the kill list — so the row moves there, and a string prefix now provably destroys a buffer on `foobar.txt` when `foo/` is deleted. Item 30's composition-order assertion was a tautology: the LSP attach leaves `diagnostic` LAST in the stack, and moving the last element to the end is a no-op, so a remove-and-re-push was indistinguishable from an in-place mutation. The row now pushes one more overlay after it and asserts that precondition explicitly. Item 34 needed both a restructure and a correction. §5's G1 says a stale captured path "materializes a phantom" via `resolve_target_buffer`'s `NotFound` arm. It does not: `pmacs.buffer.find_or_open` calls `file_io::load_file` directly and maps the error, so a missing path RAISES, and the `NotFound` arm belongs to `resolve_target_buffer`, which serves `pmacs.window.display_file` and the startup target rather than this binding. The real defect is smaller and still real — the `pcall` swallows the raise and the user is stranded wherever the last applied op left them — so the plan now edits another file first, which is what makes the restore observable at all. The correction is recorded at the test. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Lv428Fth9LRtffwJSsqH7T --- src/editor_core.rs | 4 +- src/lua_bindings/diag.rs | 3 +- tests/resource_reconciliation_acceptance.rs | 144 +++++++++++++++++--- 3 files changed, 130 insertions(+), 21 deletions(-) diff --git a/src/editor_core.rs b/src/editor_core.rs index 732e19a..f902418 100644 --- a/src/editor_core.rs +++ b/src/editor_core.rs @@ -5317,7 +5317,9 @@ pub fn buffers_bound_under( let mut out = Vec::new(); for id in reg.ids() { let Ok(buf) = reg.get(*id) else { continue }; - let Some(bound) = buf.file_path() else { continue }; + let Some(bound) = buf.file_path() else { + continue; + }; let bound = normalize_buffer_path(bound.to_path_buf()); if bound == target || (include_descendants && bound.starts_with(&target)) { out.push((*id, bound)); diff --git a/src/lua_bindings/diag.rs b/src/lua_bindings/diag.rs index 338dfc3..b9e632a 100644 --- a/src/lua_bindings/diag.rs +++ b/src/lua_bindings/diag.rs @@ -251,7 +251,8 @@ pub fn install_diag( let Some(core) = lua.app_data_ref::() else { return Ok(false); }; - core.borrow_mut().rename_resource_in_views(&old_uri, &new_uri); + core.borrow_mut() + .rename_resource_in_views(&old_uri, &new_uri); Ok(true) })?, )?; diff --git a/tests/resource_reconciliation_acceptance.rs b/tests/resource_reconciliation_acceptance.rs index 90b4e34..716ce76 100644 --- a/tests/resource_reconciliation_acceptance.rs +++ b/tests/resource_reconciliation_acceptance.rs @@ -312,6 +312,55 @@ fn acc28_a_false_string_prefix_is_not_a_path_prefix() { ); } +/// Acceptance 28, delete side — **and this is the row that bites.** +/// +/// The rename row above cannot falsify a string-prefix walk on its own: +/// `reconcile_rename` calls `Path::strip_prefix` to rebuild the +/// descendant's tail, and that is component-aware too, so a false +/// prefix match is silently dropped a second time and the buffer stays +/// put. Deletion has no such second guard — the walk's verdict IS the +/// kill list — so the containment rule has to be pinned here. +/// +/// Bite: a string `starts_with` instead of `Path::starts_with` kills a +/// buffer on `foobar.txt` when `foo/` is deleted. +#[test] +fn acc28_delete_a_false_string_prefix_does_not_widen_the_kill_list() { + let fx = Fixture::new(); + fx.dir("foo"); + let inside = fx.write("foo/a.txt", "in\n"); + let sibling = fx.write("foobar.txt", "out\n"); + let mut state = editor(); + open_as(&state, "KEEP", &fx.write("keep.txt", "k\n")); + open_as(&state, "IN", &inside); + open_as(&state, "OUT", &sibling); + + exec( + &state, + &format!( + "pmacs.buffer.apply_resource_op {{ kind = \"delete\", \ + path = \"{}\", recursive = true }}", + lua_str(&fx.at("foo")) + ), + ); + + assert!(!fx.at("foo").exists(), "the directory is gone"); + assert!( + !buffer_is_valid(&state, "IN"), + "the real descendant is reconciled away" + ); + assert!( + buffer_is_valid(&state, "OUT"), + "`foobar.txt` shares a string prefix with `foo` and is not under \ + it — killing it destroys an unrelated buffer whose file still \ + exists" + ); + assert!( + sibling.exists(), + "and that file is indeed still on disk, which is what makes the \ + kill wrong rather than merely early" + ); +} + // --------------------------------------------------------------------------- // 29 — name provenance, both directions // --------------------------------------------------------------------------- @@ -1320,7 +1369,32 @@ fn acc30_diagnostics_re_root_in_every_window_and_keep_their_stack_position() { exec(&state, "pmacs.window.switch_buffer(_G.B)"); settle_a_while(&mut state); + // Push one more overlay AFTER the diagnostic in each window. + // Without this the composition-order assertion below cannot bite: + // the LSP attach leaves `diagnostic` LAST in the stack, and moving + // the last element to the end is a no-op, so a remove-and-re-push + // would be indistinguishable from an in-place mutation. + // `_attach_highlight` uses `push_overlay` (no dedup), so a second + // call appends. + exec( + &state, + "pmacs.parse._attach_highlight(_G.B, pmacs.parse.buffer_language(_G.B))", + ); + exec(&state, "pmacs.window.focus_next()"); + exec( + &state, + "pmacs.parse._attach_highlight(_G.B, pmacs.parse.buffer_language(_G.B))", + ); + let before_kinds = overlay_kinds_per_window(&state); + assert!( + before_kinds + .iter() + .all(|(_, kinds)| kinds.iter().position(|k| *k == "diagnostic") + < Some(kinds.len() - 1)), + "precondition: the diagnostic overlay must NOT be last, or \ + \"keeps its stack position\" is unfalsifiable; got {before_kinds:?}" + ); let before_paint = error_underlines_per_window(&state); assert_eq!( before_paint.len(), @@ -1586,20 +1660,40 @@ fn wait_for_apply_response(state: &mut EditorState, sink: &Path) -> serde_json:: } /// Acceptance 34. Renaming the **active** file through the full -/// `apply_workspace_edit` path leaves **no phantom empty buffer** at the -/// obsolete path, and the user is returned to the **same buffer** (now -/// under its new path). +/// `apply_workspace_edit` path returns the user to the **same buffer** +/// (now under its new path), and leaves no buffer bound to the obsolete +/// path. +/// +/// The plan deliberately edits *another* file first. Without that the +/// row cannot bite at all: the applier only has to restore the origin if +/// something moved the active buffer away, and a lone rename op does not. /// /// Bite: the applier restoring by path instead of by buffer handle. A /// captured path no longer resolves after its own batch renamed it, so -/// `find_or_open` reaches `resolve_target_buffer`'s `NotFound` arm, -/// which *creates* an empty path-backed buffer and selects it. No -/// reconciliation can reach the string a Lua local already captured. +/// `find_or_open` raises, the `pcall` swallows it, and the user is +/// stranded in whatever buffer the last applied op left active. No +/// reconciliation can reach a string a Lua local already captured. +/// +/// **One framing claim corrected here.** §5's G1 says the stale path +/// "materializes a phantom": that `find_or_open(origin)` reaches +/// `resolve_target_buffer`'s `NotFound` arm, which creates an empty +/// path-backed buffer and selects it. It does not. +/// `pmacs.buffer.find_or_open` (`src/lua_bindings/mod.rs`) calls +/// `crate::file_io::load_file` directly and maps the error, so a missing +/// path **raises**; the `NotFound` arm belongs to +/// `EditorCore::resolve_target_buffer`, which serves +/// `pmacs.window.display_file` and the startup/daemon target, not this +/// binding. The defect is real but smaller than G1 states — a silently +/// swallowed restore, not a fabricated file — and this row asserts the +/// half that is true. The no-buffer-at-the-old-path assertion is kept as +/// a cheap guard against a future fallback that *would* create one, and +/// is not the biting half. #[test] -fn acc34_renaming_the_active_file_through_the_applier_leaves_no_phantom() { +fn acc34_renaming_the_active_file_through_the_applier_returns_the_same_buffer() { let fx = Fixture::new(); fx.write("proj/Cargo.toml", "[package]\nname = \"p\"\n"); let old = fx.write("proj/src/main.rs", "fn main() {}\n"); + let other = fx.write("proj/src/other.rs", "fn other() {}\n"); let new = fx.at("proj/src/renamed.rs"); let mut state = editor(); configure_fake(&state, &fx.root, "rust"); @@ -1607,11 +1701,25 @@ fn acc34_renaming_the_active_file_through_the_applier_leaves_no_phantom() { &state, &fx.root, &serde_json::json!({ - "documentChanges": [{ - "kind": "rename", - "oldUri": file_uri(&old), - "newUri": file_uri(&new), - }], + "documentChanges": [ + { + // Moves the active buffer away, so the restore has + // something to undo. + "textDocument": { "uri": file_uri(&other), "version": 1 }, + "edits": [{ + "range": { + "start": { "line": 0, "character": 0 }, + "end": { "line": 0, "character": 0 }, + }, + "newText": "// touched\n", + }], + }, + { + "kind": "rename", + "oldUri": file_uri(&old), + "newUri": file_uri(&new), + }, + ], }), ); open_as(&state, "B", &old); @@ -1637,7 +1745,9 @@ fn acc34_renaming_the_active_file_through_the_applier_leaves_no_phantom() { state.core.borrow().active_buffer_id(), active_before, "the user must be returned to the SAME buffer, now under its new \ - path — not to a freshly created one" + path — a path-based restore raises on the renamed-away path, the \ + pcall swallows it, and the user is left wherever the last applied \ + op put them" ); assert_eq!( buffer_path(&state, "B").as_deref(), @@ -1645,7 +1755,7 @@ fn acc34_renaming_the_active_file_through_the_applier_leaves_no_phantom() { "and that buffer's path followed the rename" ); - let phantom: bool = eval( + let stale: bool = eval( &state, &format!( "for _, b in ipairs(pmacs.buffer.list()) do @@ -1655,11 +1765,7 @@ fn acc34_renaming_the_active_file_through_the_applier_leaves_no_phantom() { lua_str(&old) ), ); - assert!( - !phantom, - "no buffer may remain bound to the obsolete path — that buffer is \ - the phantom the old path fallback materialized" - ); + assert!(!stale, "no buffer may remain bound to the obsolete path"); } /// Acceptance 35. When the origin buffer is **gone** after the edit, the