Commit f8d5108
authored
feat: describe every tool parameter and annotate tools for MCP clients (#35)
* feat: describe every tool parameter and annotate tools for MCP clients
An MCP client hands the model the tool's JSON input schema, not its
docstring. Until now every property carried only pydantic's auto title
(push_refs: string | null), so the pre-push line format, the "omit to
check upstream" behaviour and the "non-empty when provided" rule were
discovered by trial and error. Every parameter now has a description
through Annotated[..., Field(description=...)], which the SDK preserves
in the schema; the shared ones (config, repo_path, config_path, branch,
author_*) are defined once as module-level aliases.
Every tool also gets a display title and ToolAnnotations with
readOnlyHint=True, destructiveHint=False and idempotentHint=True, so
hosts that gate calls on those hints can auto-approve them.
openWorldHint is True only for validate_push_safety and
validate_repository_state, because the force-push rule may run
`git fetch <remote> <ref>` to resolve a SHA it does not know locally.
MCPServer now receives version=__version__, so serverInfo.version is no
longer an empty string.
The tool descriptions are rewritten around one shared paragraph that
defines the result: {status, warnings, checks[]}, that only 'fail' is a
rejection and 'skip' means nothing was validated, what 'warn' and 'skip'
mean per check, and that rule_id and docs_url exist. Each tool keeps its
own "when to use" sentence. validate_push_safety no longer claims that
push.allow_force_push can re-enable force pushes: _validate_push forces
that flag off after merging config, so the tool always rejects them.
The server instructions now describe the loop an agent should follow:
validate first, read status, apply a non-empty fix verbatim or follow
suggest, validate again, and consult describe_validation_rules before
guessing at a format.
Tests list the tools through the in-process server API and assert the
titles, annotations, open-world split, per-property descriptions, the
version and the instructions. README documents rule_id/docs_url, the
schema metadata, and that config_path replaces the repository's own
config file rather than merging with it.
* fix: reject unresolvable push SHAs and stop calling fetching tools read-only
Two review findings on the previous commit.
The force-push rule answers with `git merge-base --is-ancestor`. When a
SHA in push_refs is not a commit the repository knows, git exits 128,
commit-check tries to fetch the remote ref, and if the SHA is still
unknown the rule falls through to PASS. validate_push_safety therefore
reported `status: pass` for made-up SHAs, and an agent would read that
as clearance to push. After the rule has run (so a SHA its fetch brought
in counts as resolved), _validate_push now checks every SHA in explicit
push_refs with `git cat-file -e <sha>^{commit}` inside repo_path,
skipping the 40-zero new-branch placeholder, and raises a ToolError
naming the SHA when one is not a commit. An engine fail still wins: a
rejection is already the right answer. The upstream-fallback path
(push_refs omitted, and validate_repository_state's include_push) reads
HEAD and the upstream ref, which always resolve, and is unchanged.
validate_push_safety and validate_repository_state may run `git fetch`,
which writes FETCH_HEAD and remote-tracking refs, so advertising them
with readOnlyHint=True was not honest. _tool now takes `fetches` and
sets readOnlyHint=False, openWorldHint=True for those two; the other six
keep readOnlyHint=True, openWorldHint=False. destructiveHint stays False
and idempotentHint True everywhere: repeating the same fetch has no
further effect. The tool descriptions, push_refs description, server
instructions and README say the same thing instead of "read-only".
Tests: fake SHAs are a tool error, a real fast-forward pair passes, a
real history rewrite fails, a zero remote SHA passes, an engine fail is
not turned into an error by a later unresolvable line, repository_state
include_push is unaffected, and the annotation split is asserted.1 parent d20bc2c commit f8d5108
3 files changed
Lines changed: 557 additions & 128 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
19 | 19 | | |
20 | 20 | | |
21 | 21 | | |
22 | | - | |
| 22 | + | |
23 | 23 | | |
24 | 24 | | |
25 | 25 | | |
| |||
33 | 33 | | |
34 | 34 | | |
35 | 35 | | |
| 36 | + | |
36 | 37 | | |
37 | 38 | | |
38 | 39 | | |
39 | 40 | | |
40 | 41 | | |
41 | | - | |
| 42 | + | |
| 43 | + | |
42 | 44 | | |
43 | 45 | | |
44 | 46 | | |
45 | 47 | | |
46 | 48 | | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
47 | 52 | | |
48 | 53 | | |
49 | 54 | | |
| |||
59 | 64 | | |
60 | 65 | | |
61 | 66 | | |
62 | | - | |
63 | | - | |
64 | | - | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
65 | 75 | | |
66 | 76 | | |
67 | 77 | | |
| |||
266 | 276 | | |
267 | 277 | | |
268 | 278 | | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
269 | 295 | | |
270 | 296 | | |
271 | | - | |
272 | | - | |
| 297 | + | |
| 298 | + | |
273 | 299 | | |
274 | 300 | | |
275 | 301 | | |
| |||
355 | 381 | | |
356 | 382 | | |
357 | 383 | | |
358 | | - | |
359 | | - | |
360 | | - | |
| 384 | + | |
| 385 | + | |
361 | 386 | | |
362 | 387 | | |
363 | 388 | | |
| |||
0 commit comments