Skip to content

bug(pull-requests): pull_request_review_write requires owner, repo, and pullNumber in inputSchema, breaking resolve_thread and unresolve_thread #3501

Open

Description

@divyanshu07-code

In pull_request_review_write, the tool's JSON inputSchema unconditionally marks owner, repo, and pullNumber as required for all methods:

// pkg/github/pullrequests.go:2046
Required: []string{"method", "owner", "repo", "pullNumber"},

However, the tool description documents that resolve_thread and unresolve_thread only need threadId:

- resolve_thread: Resolve a review thread. Requires only "threadId" parameter with the thread's node ID (e.g., PRRT_kwDOxxx). The owner, repo, and pullNumber parameters are not used for this method. Resolving an already-resolved thread is a no-op.
- unresolve_thread: Unresolve a previously resolved review thread. Requires only "threadId" parameter. The owner, repo, and pullNumber parameters are not used for this method. Unresolving an already-unresolved thread is a no-op.

The handler (ResolveReviewThread / ResolveReviewThreadWithReason) runs a GraphQL mutation that only takes threadId:

mutation($input: ResolveReviewThreadInput!) {
  resolveReviewThread(input: $input) { ... }
}

owner, repo, and pullNumber are neither passed nor needed.

Impact

  • Valid calls are rejected by schema-validating MCP clients. Clients that enforce JSON Schema validation (VS Code MCP extension, Claude Desktop, Cursor, Goose, OpenAI/LangChain agents) reject calls that follow the documented contract:
  {
    "method": "resolve_thread",
    "threadId": "PRRT_kwDO123456"
  }

with: Missing required parameters: owner, repo, pullNumber.

  • Invalid calls pass validation. threadId is not in required, so a call with dummy owner/repo/pullNumber but no threadId passes client-side validation and only fails at runtime.

  • Inconsistent with other tools:

    • The granular equivalents (GranularResolveReviewThread and GranularUnresolveReviewThread in pkg/github/pullrequests_granular.go) correctly declare only Required: []string{"threadID"}.
    • Other consolidated dispatch tools (e.g., issue_write) don't globally require method-specific fields like issue_number (Required: []string{"method", "owner", "repo"}).

Affected version

v2.0.1 / latest main

Steps to reproduce the behavior

  1. Connect an MCP client with schema validation enabled (Claude Desktop, VS Code MCP, etc.) to github-mcp-server.
  2. Inspect the advertised schema for pull_request_review_write. inputSchema.required is ["method", "owner", "repo", "pullNumber"].
  3. Prompt an agent to resolve a review thread using pull_request_review_write with method: "resolve_thread" and threadId: "PRRT_...".
  4. Observe the client-side validation failure because owner, repo, and pullNumber were not provided.

Expected behavior

Calling pull_request_review_write with {"method": "resolve_thread", "threadId": "PRRT_..."} or {"method": "unresolve_thread", "threadId": "PRRT_..."} should pass schema validation without requiring dummy owner, repo, and pullNumber arguments.

Actual behavior

The tool cannot be invoked for resolve_thread or unresolve_thread without supplying unused dummy parameters (owner, repo, pullNumber) to bypass client-side schema validation.

Affected files

  • pkg/github/pullrequests.go (lines 2046–2075)
  • pkg/github/consolidated_pull_request_types.go (lines 582–622)
  • pkg/github/__toolsnaps__/pull_request_review_write.snap

Proposed fix

  1. In pkg/github/pullrequests.go, relax the root Required fields of pull_request_review_write to []string{"method"} (or use a relaxed input validation schema option via NewToolWithSchemaOptions).
  2. In normalizePullRequestReviewWriteArguments or in the handler, validate per method:
    • For create, submit_pending, and delete_pending: require owner, repo, and pullNumber.
    • For resolve_thread and unresolve_thread: require threadId.
  3. Update the tool snapshot pkg/github/__toolsnaps__/pull_request_review_write.snap.

I'm happy to share a PR for this if the approach above looks good to the maintainers.

Activity

  1. added a commit that references this issue on Oct 14, 2026
    b81ca20
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

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions