fix(review-diff): discard fully-staged file reverts to HEAD (#2318)

crates/fresh-editor/plugins/audit_mode.ts

@@ -5110,13 +5110,34 @@ editor.on("prompt_confirmed", async (args) => {
         const f = pendingDiscardFile;
         if (f) {
             const cwd = gitCwd();
+            // Pick the command that fully reverts the file to its HEAD state,
+            // discarding BOTH staged (index) and unstaged (working-tree)
+            // changes. Plain `git checkout -- <path>` only reverts the working
+            // tree from the index, so a fully-staged file is left untouched
+            // while the UI still claims success (#2318).
+            let cmd: string;
+            let cmdArgs: string[];
             if (f.category === 'untracked') {
-                await editor.spawnProcess("rm", ["--", f.path], cwd);
+                // Never committed and not staged: just delete it.
+                cmd = "rm";
+                cmdArgs = ["--", f.path];
+            } else if (f.status === 'A') {
+                // Staged addition with no committed version: its HEAD state is
+                // "does not exist", so a full discard unstages and removes it.
+                cmd = "git";
+                cmdArgs = ["rm", "-f", "--", f.path];
             } else {
-                await editor.spawnProcess("git", ["checkout", "--", f.path], cwd);
+                // Tracked file: restore the index and working tree to HEAD.
+                cmd = "git";
+                cmdArgs = ["checkout", "HEAD", "--", f.path];
             }
+            const result = await editor.spawnProcess(cmd, cmdArgs, cwd);
             await refreshMagitData();
-            editor.setStatus(`Discarded: ${f.path}`);
+            if (result.exit_code === 0) {
+                editor.setStatus(`Discarded: ${f.path}`);
+            } else {
+                editor.setStatus(`Discard failed: ${(result.stderr || "").trim() || f.path}`);
+            }
         }
     } else {
         editor.setStatus("Discard cancelled");

crates/fresh-editor/tests/e2e/plugins/review_diff_ux_bugs.rs

@@ -5,7 +5,7 @@
 //! **fail** (or demonstrate the broken behavior) until the underlying
 //! bug is fixed.  Once fixed, the test becomes the regression guard.
 
-use crate::common::git_test_helper::GitTestRepo;
+use crate::common::git_test_helper::{git_command, GitTestRepo};
 use crate::common::harness::{copy_plugin, copy_plugin_lib, EditorTestHarness};
 use crate::common::tracing::init_tracing_from_env;
 use crossterm::event::{KeyCode, KeyModifiers};
@@ -1996,3 +1996,108 @@ fn test_issue2117_discard_hunk_with_no_trailing_newline() {
          content (removing the unterminated added line). Got: {after:?}"
     );
 }
+
+/// Return `git status --porcelain` for the repo.
+fn git_status_porcelain(repo: &GitTestRepo) -> String {
+    let out = git_command(&repo.path)
+        .args(["status", "--porcelain"])
+        .output()
+        .expect("run git status");
+    String::from_utf8_lossy(&out.stdout).to_string()
+}
+
+/// Issue #2318: file-level Discard (`D`) on a file whose changes are entirely
+/// *staged* (clean working tree) must actually revert the file to HEAD. Before
+/// the fix the plugin ran `git checkout -- <path>`, which only reverts the
+/// working tree from the index — so a fully-staged file was left untouched
+/// while the status bar still reported "Discarded".
+#[test]
+fn test_issue2318_discard_fully_staged_file_reverts_to_head() {
+    init_tracing_from_env();
+    let repo = GitTestRepo::new();
+    repo.setup_typical_project();
+    setup_audit_mode_plugin(&repo);
+    repo.git_add_all();
+    repo.git_commit("Initial commit");
+
+    // Modify main.rs and FULLY stage it, so the working tree is clean and the
+    // only change lives in the index.
+    let main_rs = repo.path.join("src/main.rs");
+    fs::write(
+        &main_rs,
+        "fn main() {\n    println!(\"STAGED_CHANGE\");\n}\n",
+    )
+    .unwrap();
+    repo.git_add(&["src/main.rs"]);
+
+    // Precondition: staged modification with a clean working tree ("M ").
+    let status = git_status_porcelain(&repo);
+    assert!(
+        status.contains("M  src/main.rs"),
+        "precondition: src/main.rs should be fully staged. git status:\n{status}"
+    );
+
+    // Open the editor on an unrelated file (so the discarded file is not the
+    // active buffer) and launch Review Diff.
+    let mut harness = EditorTestHarness::with_config_and_working_dir(
+        120,
+        40,
+        Config::default(),
+        repo.path.clone(),
+    )
+    .unwrap();
+    harness.open_file(&repo.path.join("src/lib.rs")).unwrap();
+    harness.render().unwrap();
+
+    let screen = open_review_diff(&mut harness);
+    assert!(
+        screen.contains("STAGED"),
+        "the staged file should appear in the STAGED section. Screen:\n{screen}"
+    );
+
+    // Move the cursor down into the file's region so file-level discard resolves
+    // the file under the cursor, then press capital `D` (always-file-level
+    // discard).
+    for _ in 0..3 {
+        harness
+            .send_key(KeyCode::Char('j'), KeyModifiers::NONE)
+            .unwrap();
+    }
+    harness.render().unwrap();
+
+    harness
+        .send_key(KeyCode::Char('D'), KeyModifiers::SHIFT)
+        .unwrap();
+    harness.wait_for_prompt().unwrap();
+    // The first suggestion ("Discard changes in file") is selected by default.
+    harness
+        .send_key(KeyCode::Enter, KeyModifiers::NONE)
+        .unwrap();
+    harness.wait_for_prompt_closed().unwrap();
+
+    // The discard runs asynchronously (spawns `git`), and the handler sets its
+    // status message only AFTER the `git` process and the follow-up refresh have
+    // completed. Wait for that signal before inspecting git state, rather than
+    // guessing a fixed number of ticks (which races on slower machines and can
+    // observe a half-applied index/worktree).
+    harness
+        .wait_until(|h| {
+            let s = h.screen_to_string();
+            s.contains("Discarded") || s.contains("Discard failed")
+        })
+        .unwrap();
+
+    // The staged change must be fully discarded: clean git status and the file
+    // reverted to its committed content on disk.
+    let status = git_status_porcelain(&repo);
+    assert!(
+        status.trim().is_empty(),
+        "#2318: the staged change should be fully discarded, but git status is:\n{status}"
+    );
+    let content = fs::read_to_string(&main_rs).unwrap();
+    assert!(
+        !content.contains("STAGED_CHANGE"),
+        "#2318: src/main.rs should be reverted to HEAD, but still contains the \
+         staged change:\n{content}"
+    );
+}