Skip to content

pull_request_read silently ignores pagination parameters that don't apply to the selected method #3316

Description

@larik15

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

  1. Call pull_request_read with method: "get_files" on a PR with more
    than one page of changed files, perPage: 10.
  2. Take any non-empty string as after and call again with the same
    method, perPage: 10, after: "<value>".
  3. Observe: the response is identical to step 1. No error.

Or:

  1. Call with method: "get_review_comments", page: 2.
  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.

Activity

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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions