From 7f556e14a7da8129fc9da75e7ee2c87400c24b75 Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Mon, 14 Sep 2026 18:26:21 +0000 Subject: [PATCH 1/2] fix: pin --untracked-files so status.showUntrackedFiles cannot hide work [patch] GitStatusBuilder only emitted --untracked-files when a caller called WithUntrackedFiles. Without it git resolves the mode from the host's status.showUntrackedFiles, so a machine that sets it to "no" reported the same working copy as IsClean with untracked files sitting in it. The flag is now emitted unconditionally, defaulting to git's own documented "normal", the way GitCommandBuilder already pins --no-pager, core.quotepath and color.ui. WithUntrackedFiles replaces that default rather than adding a second value. Fixes #112 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WVrdn7dcoGasLJEgrnb67u --- CLAUDE.md | 11 +++++ .../Builders/GitStatusBuilderTests.cs | 22 ++++++++++ .../Integration/GitRoundTripTests.cs | 41 +++++++++++++++++++ GitIntegration/Builders/GitStatusBuilder.cs | 29 ++++++++++--- 4 files changed, 97 insertions(+), 6 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index a56a1f2..4096dcd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -328,6 +328,17 @@ Non-obvious, load-bearing design points: positional, because a rename is spelled differently in each section. A binary file's counts are `-`, which is why they are nullable — a binary change is not a zero-line change. +14. **`Status()` emits `--untracked-files` whether or not the caller chose a mode.** Git resolves the + flag from `status.showUntrackedFiles` when a command omits it, and a host — a minimal CI image, a + developer's `~/.gitconfig` — can set that to `no` to keep `git status` fast in a large tree. The + same working copy would then report `IsClean == true` with untracked files sitting in it, which is + exactly the answer a caller deciding whether a directory is safe to discard must not get. So + `GitStatusBuilder.DefaultUntrackedFiles` (git's own documented default, `normal`) is pinned into + the argument vector alongside the `--no-pager`, `core.quotepath=false` and `color.ui=false` that + `GitCommandBuilder.BuildArguments` already pins — same reasoning, and this was the one verb still + leaving a host-configurable default unspecified. `WithUntrackedFiles(...)` replaces the default + rather than joining it, so the vector never carries two `--untracked-files` values. + **Hosting layer.** `GitProvider` is an abstract base with two implementations: `GitHubProvider` over Octokit, and `AzureDevOpsProvider` over a raw `HttpClient` — Azure DevOps has no client library this library uses (see the dependency note below). Both go through the same shape: every request-issuing diff --git a/GitIntegration.Test/Builders/GitStatusBuilderTests.cs b/GitIntegration.Test/Builders/GitStatusBuilderTests.cs index 231d96c..4dd356c 100644 --- a/GitIntegration.Test/Builders/GitStatusBuilderTests.cs +++ b/GitIntegration.Test/Builders/GitStatusBuilderTests.cs @@ -28,6 +28,11 @@ public void BuildsTheDefaultStatusVector() "--porcelain=v2", "--branch", "-z", + + // Present without the caller asking for it: left off, the mode would come from the host's + // status.showUntrackedFiles, and a host that sets it to "no" would report a working copy + // with untracked files in it as clean. + "--untracked-files=normal", ]; CollectionAssert.AreEqual(expectedArguments, arguments.ToArray()); } @@ -50,6 +55,23 @@ public void MapsTheUntrackedFilesModeToItsOption() CollectionAssert.Contains(all.BuildArguments().ToArray(), "--untracked-files=all"); } + [TestMethod] + public void ChoosingAModeReplacesThePinnedDefaultRatherThanJoiningIt() + { + // The default is emitted unconditionally, so the risk it introduces is a vector carrying both + // --untracked-files=normal and the caller's choice. Git would honour the last one, which is the + // caller's, but only by accident of ordering — and a No caller who silently got a "normal" pass + // over a large tree first would pay for a mode they declined. + RecordingGitProcessRunner runner = new(); + GitStatusBuilder builder = new(runner, TestPaths.Root); + _ = builder.WithUntrackedFiles(GitUntrackedFilesMode.No); + + string[] arguments = [.. builder.BuildArguments()]; + + Assert.AreEqual(1, arguments.Count(argument => argument.StartsWith("--untracked-files=", StringComparison.Ordinal))); + CollectionAssert.DoesNotContain(arguments, "--untracked-files=normal"); + } + [TestMethod] public void AddsTheIgnoredOptionOnlyWhenAsked() { diff --git a/GitIntegration.Test/Integration/GitRoundTripTests.cs b/GitIntegration.Test/Integration/GitRoundTripTests.cs index 9180e99..31848de 100644 --- a/GitIntegration.Test/Integration/GitRoundTripTests.cs +++ b/GitIntegration.Test/Integration/GitRoundTripTests.cs @@ -153,6 +153,47 @@ public async Task StatusReflectsStagedAndUntrackedWorkAsync() Assert.IsFalse(dirty.IsClean); } + [TestMethod] + public async Task StatusReportsUntrackedWorkEvenWhereTheHostHidesItAsync() + { + // status.showUntrackedFiles is a host setting — a CI image or a developer's ~/.gitconfig can set + // it to "no" to keep git status fast in a large tree — and git applies it to any status command + // carrying no --untracked-files of its own. Written into the throwaway repository's own config + // rather than the runner's global one: it is the same variable resolved from the nearest scope, + // so the override is exercised without the test depending on, or disturbing, the host. + CancellationToken cancellationToken = TestContext.CancellationTokenSource.Token; + await IntegrationGitFixture.RequireGitAsync(cancellationToken).ConfigureAwait(false); + + using TemporaryRepository temporary = new(); + GitRepository repository = await InitialiseAsync(temporary, cancellationToken).ConfigureAwait(false); + + temporary.WriteFile("a.txt", "one\n"); + _ = await repository.Add().All().ExecuteAsync(cancellationToken).ConfigureAwait(false); + _ = await repository.Commit("c1".As()).ExecuteAsync(cancellationToken).ConfigureAwait(false); + + _ = await new GitTextBuilder( + repository.ProcessRunner!, repository.LocalPath, "config", "status.showUntrackedFiles", "no") + .ExecuteAsync(cancellationToken).ConfigureAwait(false); + + temporary.WriteFile("untracked.txt", "two\n"); + + GitStatus status = await repository.Status().ExecuteAsync(cancellationToken).ConfigureAwait(false); + + // IsClean is asserted beside the entry because it is the property a caller reads before + // discarding a working copy — reporting true here would mean losing that file. + Assert.IsFalse(status.IsClean); + Assert.IsTrue( + status.Entries.Any(entry => entry.Path.WeakString.EndsWith("untracked.txt", StringComparison.Ordinal)), + "Status() dropped the untracked file because the repository's status.showUntrackedFiles said to."); + + // A caller's own choice still wins, including in the direction the config happens to agree with: + // the pinned default replaces the host's setting rather than overriding the caller's. + GitStatus suppressed = await repository.Status() + .WithUntrackedFiles(GitUntrackedFilesMode.No) + .ExecuteAsync(cancellationToken).ConfigureAwait(false); + Assert.IsTrue(suppressed.IsClean); + } + [TestMethod] public async Task BranchCreateCheckoutAndDeleteRoundTripAsync() { diff --git a/GitIntegration/Builders/GitStatusBuilder.cs b/GitIntegration/Builders/GitStatusBuilder.cs index 19c6b19..8de3b6a 100644 --- a/GitIntegration/Builders/GitStatusBuilder.cs +++ b/GitIntegration/Builders/GitStatusBuilder.cs @@ -16,6 +16,11 @@ public interface IGitStatusBuilder : IGitCommandBuilder /// /// Sets how much untracked detail git should report. /// + /// + /// Without this call the command reports untracked files the way + /// describes, whatever the host's + /// status.showUntrackedFiles is set to. + /// /// The reporting mode. /// The same builder, to allow chaining. /// is not a recognised value. @@ -29,14 +34,23 @@ public interface IGitStatusBuilder : IGitCommandBuilder } /// -/// Builds git status --porcelain=v2 --branch -z. +/// Builds git status --porcelain=v2 --branch -z --untracked-files=normal. /// /// Runs the assembled command. /// The repository to scope the command to. internal sealed class GitStatusBuilder(IGitProcessRunner runner, AbsoluteDirectoryPath repositoryPath) : GitCommandBuilder(runner, repositoryPath), IGitStatusBuilder { - private GitUntrackedFilesMode? _untrackedFiles; + /// + /// The untracked reporting mode emitted when the caller never calls . + /// + /// + /// Git's own documented default for status.showUntrackedFiles, so pinning it changes nothing + /// on a host that has not set the variable and everything on a host that has. + /// + internal const GitUntrackedFilesMode DefaultUntrackedFiles = GitUntrackedFilesMode.Normal; + + private GitUntrackedFilesMode _untrackedFiles = DefaultUntrackedFiles; private bool _includeIgnored; /// @@ -76,10 +90,13 @@ protected override void AppendVerbArguments(ICollection arguments) arguments.Add("--branch"); arguments.Add("-z"); - if (_untrackedFiles is GitUntrackedFilesMode mode) - { - arguments.Add("--untracked-files=" + ToOptionValue(mode)); - } + // Emitted whether or not the caller chose a mode. Omitting the flag hands the decision to + // status.showUntrackedFiles, which a CI image or a developer's ~/.gitconfig can set to "no" to + // speed up status in a large tree — and then the same working copy reports IsClean == true with + // an untracked file sitting in it. A caller asking "is there work here I would destroy?" + // deserves the same answer on every host, so the default is pinned in the argument vector + // beside --no-pager, core.quotepath and color.ui rather than left to the host. + arguments.Add("--untracked-files=" + ToOptionValue(_untrackedFiles)); if (_includeIgnored) { From dccc7a8adab709254e4a2ab081c25f691b88a997 Mon Sep 17 00:00:00 2001 From: Matthew Edmondson Date: Mon, 14 Sep 2026 18:38:52 +0000 Subject: [PATCH 2/2] test: use the MSTest assertions the analyzers point at [patch] SonarCloud's analysis of the new tests raised three MSTest suggestions, all in code this branch added: - MSTEST0037 -> Assert.ContainsSingle for the "exactly one --untracked-files argument" check, which also folds the following DoesNotContain into an equality check on the single argument found - MSTEST0068 -> resolved by that fold; the CollectionAssert call is gone - MSTEST0037 -> Assert.Contains for the untracked-entry check, keeping the failure message MSTEST0054 (TestContext.CancellationToken over TestContext.CancellationTokenSource.Token) is left alone: the older form is used at roughly 250 call sites across 32 files here, and changing one new line would make it the only outlier. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WVrdn7dcoGasLJEgrnb67u --- GitIntegration.Test/Builders/GitStatusBuilderTests.cs | 8 +++++--- GitIntegration.Test/Integration/GitRoundTripTests.cs | 5 +++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/GitIntegration.Test/Builders/GitStatusBuilderTests.cs b/GitIntegration.Test/Builders/GitStatusBuilderTests.cs index 4dd356c..828aa3e 100644 --- a/GitIntegration.Test/Builders/GitStatusBuilderTests.cs +++ b/GitIntegration.Test/Builders/GitStatusBuilderTests.cs @@ -66,10 +66,12 @@ public void ChoosingAModeReplacesThePinnedDefaultRatherThanJoiningIt() GitStatusBuilder builder = new(runner, TestPaths.Root); _ = builder.WithUntrackedFiles(GitUntrackedFilesMode.No); - string[] arguments = [.. builder.BuildArguments()]; + IReadOnlyList arguments = builder.BuildArguments(); - Assert.AreEqual(1, arguments.Count(argument => argument.StartsWith("--untracked-files=", StringComparison.Ordinal))); - CollectionAssert.DoesNotContain(arguments, "--untracked-files=normal"); + string untracked = Assert.ContainsSingle( + argument => argument.StartsWith("--untracked-files=", StringComparison.Ordinal), + arguments); + Assert.AreEqual("--untracked-files=no", untracked); } [TestMethod] diff --git a/GitIntegration.Test/Integration/GitRoundTripTests.cs b/GitIntegration.Test/Integration/GitRoundTripTests.cs index 31848de..7df366a 100644 --- a/GitIntegration.Test/Integration/GitRoundTripTests.cs +++ b/GitIntegration.Test/Integration/GitRoundTripTests.cs @@ -182,8 +182,9 @@ public async Task StatusReportsUntrackedWorkEvenWhereTheHostHidesItAsync() // IsClean is asserted beside the entry because it is the property a caller reads before // discarding a working copy — reporting true here would mean losing that file. Assert.IsFalse(status.IsClean); - Assert.IsTrue( - status.Entries.Any(entry => entry.Path.WeakString.EndsWith("untracked.txt", StringComparison.Ordinal)), + Assert.Contains( + entry => entry.Path.WeakString.EndsWith("untracked.txt", StringComparison.Ordinal), + status.Entries, "Status() dropped the untracked file because the repository's status.showUntrackedFiles said to."); // A caller's own choice still wins, including in the direction the config happens to agree with: