Activity

1. youlemei2022-web commented on Oct 27, 2026

   

   youlemei2022-web

   on Oct 27, 2026

   More actions

   Confirming the behaviour from the v2 tree: max_count reaches git log unvalidated, and neither 0 nor -1 means what a caller would expect. git log -n 0 returns the full history rather than nothing, and a negative value is accepted and also returns everything - so a caller asking for zero commits silently gets the whole repository.

   Three changes, in order of value:

   1. Constrain the schema. Set minimum: 1 on max_count so well-behaved clients get a validation error before the call ever reaches git, instead of a surprising success with the wrong result set.
   2. Guard at the boundary anyway, for clients that bypass schema validation:

   if not isinstance(max_count, int) or isinstance(max_count, bool) or max_count < 1:
       raise ValueError("max_count must be a positive integer")

   Keep 0 an error rather than "empty list". The silent full-history response is a correctness bug, not a formatting choice - a model that asked for 0 commits and received 200 will happily summarise the wrong scope.

   3. Pass the value in --opt=value form (--max-count=<n> as a single argv entry) rather than as two arguments, so a value that begins with - can never be reinterpreted as another option by git. This also protects future numeric options.

   Tests that would have caught it: -1, 0, 1, and a repository with fewer commits than requested - the last one should still return all commits rather than error, which distinguishes the validation rule from an off-by-one.
2. cliffhall

   added

   v2

   bugSomething isn't workingSomething isn't working

   server-gitReference implementation for the Git MCP server - src/gitReference implementation for the Git MCP server - src/git

   on Oct 28, 2026
3. cliffhall commented on Oct 28, 2026

   

   cliffhall

   on Oct 28, 2026

   Member

   More actions

   Triage: Priority Low (total 5)

   • Severity 2: a max_count of 0 or below is passed to git unvalidated and does not mean what the parameter says; the workaround is to pass a positive count. Confirmed on v2/main: the tool's schema has no minimum.
   • Urgency 2: wanted eventually, nothing blocked on it
   • Bonuses: +1 bug label

   Board: Servers V2 (#43), Status Incoming, awaiting maintainer review (no milestone yet).
4. cliffhall

   added this to the v2.4.0 milestone on Oct 29, 2026
5. scks1 commented on Oct 29, 2026

   

   scks1

   on Oct 29, 2026

   More actions

   Prepared comment for issue #5214 — NOT POSTED

   Channel note: modelcontextprotocol/servers accepts issues, not outside PRs
   (CONTRIBUTING.md TL;DR). The sanctioned way to contribute a locally built fix
   is a comment on the issue sharing the prompt that produced it. This text is
   ready for Simon/the parent to post; the subagent that prepared it made no
   external writes.

   ---

   Disclosure: this contribution was written by Árbol, an AI assistant working under the direction of the account holder, who set the acceptance criteria and verified the results before submission.

   Adding a data point and a verified prototype for #5214, against current
   v2/main (72ad22c), following the fix shape the issue prescribes:

   Prompt used (produced the local change; no diff attached, per this
   repository's contribution model):

   In src/git of modelcontextprotocol/servers (checkout of v2/main), fix
   issue #5214: the git_log tool's max_count has no lower bound, so -1
   silently returns the entire history and 0 is accepted. (1) In the log
   tool registration in src/git/src/mcp_server_git/server.py, annotate
   max_count as Annotated[int, Field(ge=1, description="Maximum number of commits to show. Must be at least 1.")] = 10, following the file's existing
   Annotated[..., Field(description=...)] convention for the timestamp
   parameters, so the advertised schema carries "minimum": 1 and the SDK's
   argument validation rejects non-positive values as a tool error.
   (2) In the internal git_log() function, add a defense-in-depth guard in
   the style of its existing flag-injection guards:
   if max_count < 1: raise ValueError(f"Invalid max_count: {max_count} - must be at least 1"),
   with a comment explaining that git reads a negative -n as "no limit".
   (3) Update the git_log entry in tests/snapshots/list_tools.json for the
   new schema, and the max_count line in src/git/README.md to state the
   bound. (4) Tests: in tests/test_protocol.py, after
   test_git_log_max_count, add a protocol test that creates 3 commits and
   asserts max_count of -1 and 0 each return isError with a
   logArguments validation message naming max_count and the
   greater-than-or-equal-to-1 bound; in tests/test_server.py, next to
   test_git_log_rejects_timestamp_flag_injection, add a unit test asserting
   git_log(repo, max_count=0) and max_count=-1 raise ValueError.
   Do not change behavior for positive values.

   Verified locally (Linux, Python 3.10 venv from the server's uv.lock;
   full log in the working record's SUMMARY.md): the src/git suite passes
   301 tests at v2/main (72ad22c) and 303 with this change — the two added
   tests fail against the unmodified source and pass with it. The list_tools
   wire diff is exactly the two new keys on max_count (description,
   minimum: 1). ruff check and ruff format --check are clean, pyright
   reports 0 errors, uv build succeeds, and server.py coverage stays at
   100% lines / 100% branches. A valid max_count (including requesting more
   commits than the repository has) is unaffected.
6. cliffhall

   added a parent issue on Nov 5, 2026

   • Tracker: Git v2.4.0 #5442
7. cliffhall

   self-assigned this

   on Nov 5, 2026
8. cliffhall

   linked a pull request that will close this issuefix(git): require git_log max_count to be at least 1 #5455on Nov 5, 2026
9. cliffhall commented on Nov 5, 2026

   

   cliffhall

   on Nov 5, 2026

   Member

   More actions

   Shipped in #5455 (merged to v2/main).
10. cliffhall

    closed this as completedon Nov 5, 2026