Skip to content

Add() with no ForPath/All/UpdateTrackedOnly reports success having staged nothing, and All().UpdateTrackedOnly() reaches git as a fatal usage error #172

Description

@matt-edmondson

What's wrong

repository.Add().ExecuteAsync() with nothing configured runs git … add with no pathspec. Git prints Nothing specified, nothing added. and exits 0, so the builder returns a GitCompleted as if staging had worked.

GitAddBuilder.AppendVerbArguments (GitIntegration/Builders/GitAddBuilder.cs:69-97) emits add, then --all/--update if set, then operands if any. Nothing checks that there is anything to stage.

The opposite mistake isn't checked either. Add().All().UpdateTrackedOnly() emits add --all --update, which git rejects with fatal: options '-A' and '-u' cannot be used together (exit 128). The caller gets a generic GitCommandException with no hint that two fluent calls conflict.

Verified with git 2.43:

$ git add; echo $?
Nothing specified, nothing added.
hint: Maybe you wanted to say 'git add .'?
0
$ git add --all --update; echo $?
fatal: options '-A' and '-u' cannot be used together
128

Why it matters

A caller who forgets .All() or .ForPath(...) gets a success result. The following Commit(...) then either throws GitNothingToCommitException or commits only what was staged earlier, far from the real mistake.

Other builders already refuse impossible configurations before spawning git:

Suggested fix / acceptance criteria

  • An Add() builder with no paths and neither All() nor UpdateTrackedOnly() throws InvalidOperationException before any process starts, and TryExecuteAsync behaves the same, matching the Fetch guard.
  • All() combined with UpdateTrackedOnly() is either refused the same way, or documented as last-call-wins like IGitBranchListBuilder.LocalOnly/RemoteOnly. Git's fatal error should not be the only signal.
  • If the check goes in BuildArguments() as Fetch's does, update GitRepositoryMutatingVerbTests.EveryMutatingVerbIsScopedToTheRepositoryPath (GitIntegration.Test/GitRepositoryMutatingVerbTests.cs:28). It calls repository.Add().BuildArguments() bare, so it needs e.g. .All().
  • Builder tests cover both refusals, and the existing vectors for ForPath, All and UpdateTrackedOnly used alone are kept.

Activity

  1. matt-edmondson commented on Oct 5, 2026

    @matt-edmondson
    ContributorAuthor

    Triage

    • Category: Bug
    • Priority: Medium. A bare Add() reports success and stages nothing, so the mistake only shows up later, at Commit(). The All() + UpdateTrackedOnly() conflict fails loudly but with an unhelpful error.
    • Area / suggested owner: Builder validation in GitAddBuilder. It should follow the existing guard pattern in GitFetchBuilder and in GitCheckoutBuilder (GitCheckoutBuilder does not reject CreatingBranch() + Detach(), which git refuses at runtime #91).
    • Duplicates: None found.
    • In progress: No open PR covers this.
    • Next step: Add an InvalidOperationException guard for both cases. Update GitRepositoryMutatingVerbTests.EveryMutatingVerbIsScopedToTheRepositoryPath, which calls Add() bare.

    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions