fix: Exclude local index from changed files when an explicit head is provided (#2617)

* fix: Exclude local index from changed files when an explicit head is provided.

`moon query changed-files` (and affected detection) always merged
`git status` into the results, even when both `--base` and `--head`
were passed as explicit revisions. Local uncommitted changes and
untracked files showed up as false positives when walking history
commit-by-commit (issue #2511, remaining half after the v2.4 rework
fixed the dropped `--head`).

The local index is now only consulted when no explicit head is
provided, i.e. when the comparison targets the working tree. Head-only
queries previously ignored the head in local mode (returning local
status only); they now diff merge-base(defaultBranch, head)..head.
Additionally, `MOON_BASE`/`MOON_HEAD` values that are set but empty are
treated as not provided, as CI templates commonly pass these through
unconditionally.

Closes #2511

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix: Do not let empty base/head environment variables mask explicit options.

Review feedback: the empty-value filter ran after the env/option
fallback, so MOON_BASE="" or MOON_HEAD="" would discard an explicitly
passed --base/--head. Filter the environment variables before falling
back, and filter the options after. Also reworded the docs bullet and
fixed changelog punctuation.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Author: milesj

CHANGELOG.md

@@ -35,6 +35,11 @@
 - Fixed tasks that emit read-only outputs (e.g. `0444` files) failing on every cache hit with a
   "Permission denied" error when the `casOutputsCache` experiment is enabled. Caches that already
   contain read-only objects are healed automatically.
+- Fixed an issue where `moon query changed-files` would include uncommitted changes from the local
+  index (`git status`) even when an explicit `--head` revision was provided, causing false positives
+  when comparing 2 revisions. This also applies to affected detection with an explicit head, e.g.
+  the `MOON_HEAD` environment variable or `--affected base:head`. Additionally, `MOON_BASE` and
+  `MOON_HEAD` environment variables that are set but empty are now ignored.
 - Fixed an issue where synced VCS hooks were always written to `.moon/hooks`, even when the workspace
   configuration lived in `.config/moon`. Hooks are now placed alongside the config, in
   `.config/moon/hooks`.

crates/app/src/queries/changed_files.rs

@@ -87,9 +87,20 @@ async fn query_changed_files_without_stdin(
     let bag = GlobalEnvBag::instance();
     let default_branch = vcs.get_default_branch().await?;
     let current_branch = vcs.get_local_branch().await?;
-    let base_value = bag.get("MOON_BASE").or(options.base.clone());
+    // Treat empty values as not provided, as CI templates typically
+    // pass these environment variables through unconditionally. An empty
+    // environment variable must not mask an explicit option either
+    let base_value = bag
+        .get("MOON_BASE")
+        .filter(|value| !value.is_empty())
+        .or(options.base.clone())
+        .filter(|value| !value.is_empty());
     let base = base_value.as_deref().unwrap_or(&default_branch);
-    let head_value = bag.get("MOON_HEAD").or(options.head.clone());
+    let head_value = bag
+        .get("MOON_HEAD")
+        .filter(|value| !value.is_empty())
+        .or(options.head.clone())
+        .filter(|value| !value.is_empty());
     let head = head_value.as_deref().unwrap_or("HEAD");
 
     // Determine whether we should check against the previous
@@ -110,7 +121,7 @@ async fn query_changed_files_without_stdin(
         );
     }
 
-    let only_local = options.local && base_value.is_none();
+    let only_local = options.local && base_value.is_none() && head_value.is_none();
     let mut changed_files_map = ChangedFiles::default();
     let mut changed_files = FxHashSet::default();
 
@@ -138,10 +149,14 @@ async fn query_changed_files_without_stdin(
         }
     }
 
-    // Always include local changes
-    debug!("Against local index");
+    // Only include local changes when the head is the working tree;
+    // an explicit head requests a comparison between 2 revisions,
+    // of which the local index is not a part of
+    if head_value.is_none() {
+        debug!("Against local index");
 
-    changed_files_map.merge(vcs.get_changed_files().await?);
+        changed_files_map.merge(vcs.get_changed_files().await?);
+    }
 
     if options.status.is_empty() {
         debug!(

crates/cli/tests/query_changed_files_test.rs

@@ -48,6 +48,138 @@ mod query_changed_files {
         assert_eq!(json.options.status, vec![ChangedStatus::Deleted]);
     }
 
+    #[test]
+    fn excludes_local_index_with_explicit_head() {
+        let sandbox = create_query_sandbox();
+
+        sandbox.create_file("basic/committed.txt", "contents");
+
+        sandbox.run_git(|cmd| {
+            cmd.args(["add", "--all", "."]);
+        });
+
+        sandbox.run_git(|cmd| {
+            cmd.args(["commit", "-m", "Commit"]);
+        });
+
+        // Should not appear in the revision to revision diff
+        sandbox.create_file("basic/dirty.txt", "contents");
+
+        let assert = sandbox.run_bin(|cmd| {
+            cmd.arg("query")
+                .arg("changed-files")
+                .args(["--base", "HEAD~1", "--head", "HEAD"]);
+        });
+
+        let json: QueryChangedFilesResult = serde_json::from_str(assert.stdout().trim()).unwrap();
+
+        assert_eq!(
+            json.files.into_iter().collect::<Vec<_>>(),
+            ["basic/committed.txt"]
+        );
+    }
+
+    #[test]
+    fn includes_local_index_without_explicit_head() {
+        let sandbox = create_query_sandbox();
+
+        sandbox.create_file("basic/committed.txt", "contents");
+
+        sandbox.run_git(|cmd| {
+            cmd.args(["add", "--all", "."]);
+        });
+
+        sandbox.run_git(|cmd| {
+            cmd.args(["commit", "-m", "Commit"]);
+        });
+
+        sandbox.create_file("basic/dirty.txt", "contents");
+
+        let assert = sandbox.run_bin(|cmd| {
+            cmd.arg("query")
+                .arg("changed-files")
+                .args(["--base", "HEAD~1"]);
+        });
+
+        let json: QueryChangedFilesResult = serde_json::from_str(assert.stdout().trim()).unwrap();
+
+        let mut files = json
+            .files
+            .into_iter()
+            .filter(|file| !file.as_str().starts_with(".moon"))
+            .collect::<Vec<_>>();
+        files.sort();
+
+        assert_eq!(files, ["basic/committed.txt", "basic/dirty.txt"]);
+    }
+
+    #[test]
+    fn treats_empty_head_as_working_tree() {
+        let sandbox = create_query_sandbox();
+
+        sandbox.create_file("basic/committed.txt", "contents");
+
+        sandbox.run_git(|cmd| {
+            cmd.args(["add", "--all", "."]);
+        });
+
+        sandbox.run_git(|cmd| {
+            cmd.args(["commit", "-m", "Commit"]);
+        });
+
+        sandbox.create_file("basic/dirty.txt", "contents");
+
+        let assert = sandbox.run_bin(|cmd| {
+            cmd.arg("query")
+                .arg("changed-files")
+                .args(["--base", "HEAD~1"])
+                .env("MOON_HEAD", "");
+        });
+
+        let json: QueryChangedFilesResult = serde_json::from_str(assert.stdout().trim()).unwrap();
+
+        let mut files = json
+            .files
+            .into_iter()
+            .filter(|file| !file.as_str().starts_with(".moon"))
+            .collect::<Vec<_>>();
+        files.sort();
+
+        assert_eq!(files, ["basic/committed.txt", "basic/dirty.txt"]);
+    }
+
+    #[test]
+    fn empty_env_vars_dont_mask_explicit_args() {
+        let sandbox = create_query_sandbox();
+
+        sandbox.create_file("basic/committed.txt", "contents");
+
+        sandbox.run_git(|cmd| {
+            cmd.args(["add", "--all", "."]);
+        });
+
+        sandbox.run_git(|cmd| {
+            cmd.args(["commit", "-m", "Commit"]);
+        });
+
+        sandbox.create_file("basic/dirty.txt", "contents");
+
+        let assert = sandbox.run_bin(|cmd| {
+            cmd.arg("query")
+                .arg("changed-files")
+                .args(["--base", "HEAD~1", "--head", "HEAD"])
+                .env("MOON_BASE", "")
+                .env("MOON_HEAD", "");
+        });
+
+        let json: QueryChangedFilesResult = serde_json::from_str(assert.stdout().trim()).unwrap();
+
+        assert_eq!(
+            json.files.into_iter().collect::<Vec<_>>(),
+            ["basic/committed.txt"]
+        );
+    }
+
     #[test]
     fn can_supply_multi_status() {
         let sandbox = create_query_sandbox();

website/docs/commands/query/changed-files.mdx

@@ -15,6 +15,9 @@ Changed files are determined using the following logic:
   uses.
 - If `--local` is provided, changed files are based on your local index only (`git status`).
 - Otherwise, then compare the defined base (`--base`) against head (`--head`).
+- When no explicit `--head` is provided, the comparison targets your working tree, so changed files
+  in your local index (`git status`) are also included. An explicit `--head` is a pure revision to
+  revision comparison, and does not include the local index.
 
 ```shell
 # Return all files