Describe the bug
pull_request_read exposes two pagination mechanisms in one schema:
page / perPage — used by get_files, get_commits, get_reviews, get_comments, get_check_runs
after (cursor) — used only by get_review_comments
When a caller passes a parameter the selected method doesn't use, the
server silently drops it and returns results as if it were never sent.
There is no error, warning, or hint in the response.
The schema description for after does say "used only by the
get_review_comments method", and the code comment confirms other methods
"ignore after" — but for an LLM caller a description is guidance, not
enforcement. The model sees a pagination cursor in the schema, receives a
page of files, and passes after to fetch the next page. It gets the first
page again and has no signal that anything was ignored.
The symmetric case also applies: passing page to get_review_comments
is silently ignored.
Steps to reproduce
- Call
pull_request_read with method: "get_files" on a PR with more
than one page of changed files, perPage: 10.
- Take any non-empty string as
after and call again with the same
method, perPage: 10, after: "<value>".
- Observe: the response is identical to step 1. No error.
Or:
- Call with
method: "get_review_comments", page: 2.
- Observe:
page is ignored, first page is returned.
Expected behavior
A pagination parameter that the selected method cannot honour should
produce a validation error naming the method and the parameter, e.g.
method "get_files" uses page/perPage pagination; "after" is not supported. This is consistent with how the tool already rejects an
unknown method.
Why this matters
Silent parameter drops are worst-case for agentic callers: the tool reports
success with a plausible payload, so the model has no reason to retry.
This is the same failure class as #2347 (silent truncation), and a
follow-up to #2489, which added after to the schema but did not add the
guard.
Suggested fix
Before dispatching on method, check whether the caller supplied a
pagination parameter the method doesn't use, and return
utils.NewToolResultError with a clear message. A small helper in
pullrequests.go covers all nine methods.
Happy to open a PR.
Describe the bug
pull_request_readexposes two pagination mechanisms in one schema:page/perPage— used byget_files,get_commits,get_reviews,get_comments,get_check_runsafter(cursor) — used only byget_review_commentsWhen a caller passes a parameter the selected method doesn't use, the
server silently drops it and returns results as if it were never sent.
There is no error, warning, or hint in the response.
The schema description for
afterdoes say "used only by theget_review_comments method", and the code comment confirms other methods
"ignore
after" — but for an LLM caller a description is guidance, notenforcement. The model sees a pagination cursor in the schema, receives a
page of files, and passes
afterto fetch the next page. It gets the firstpage again and has no signal that anything was ignored.
The symmetric case also applies: passing
pagetoget_review_commentsis silently ignored.
Steps to reproduce
pull_request_readwithmethod: "get_files"on a PR with morethan one page of changed files,
perPage: 10.afterand call again with the samemethod,perPage: 10,after: "<value>".Or:
method: "get_review_comments",page: 2.pageis ignored, first page is returned.Expected behavior
A pagination parameter that the selected method cannot honour should
produce a validation error naming the method and the parameter, e.g.
method "get_files" uses page/perPage pagination; "after" is not supported. This is consistent with how the tool already rejects anunknown
method.Why this matters
Silent parameter drops are worst-case for agentic callers: the tool reports
success with a plausible payload, so the model has no reason to retry.
This is the same failure class as #2347 (silent truncation), and a
follow-up to #2489, which added
afterto the schema but did not add theguard.
Suggested fix
Before dispatching on
method, check whether the caller supplied apagination parameter the method doesn't use, and return
utils.NewToolResultErrorwith a clear message. A small helper inpullrequests.gocovers all nine methods.Happy to open a PR.