`pull_request_review_write` combines create/submit/delete into one tool, making fine-grained permissions by method impossible

[kaovilai]

Today the pull_request_review_write tool consolidates three distinct review operations behind a single tool name with a method enum:

• create
• submit_pending
• delete_pending

The description is:

Create and/or submit, delete review of a pull request.
Available methods:

• create: Create a new review of a pull request. If event parameter is provided, the review is submitted. If event is omitted, a pending review is created.
• submit_pending: Submit an existing pending review of a pull request.
• delete_pending: Delete an existing pending review of a pull request.

This consolidation plays nicely with context/token budgets, but it makes permissions in MCP clients effectively all-or-nothing for these three operations, because most clients (Claude Code, etc.) attach permissions at the tool name level, not on the method argument.

My use case

• I want to always allow create for pending reviews, so the agent can open a pending review and add inline comments.
• But I do not want the agent to be allowed to submit or delete reviews automatically; those should always require explicit human approval (or be disallowed entirely).

With the current API surface, the client can't express this as:

• allow: create_pending_review
• ask/deny: submit_pending_review
• ask/deny: delete_pending_review

because all three are encoded as pull_request_review_write with different method values. There's no way to distinguish them in a tool-level permission model.

Request

Please provide a way to make these operations distinguishable at the tool/permission layer. A few possible approaches:

1. Split into separate tools (preferred for permissioning)

For example:

• create_pending_pull_request_review
• submit_pending_pull_request_review
• delete_pending_pull_request_review

This would let MCP clients and policy engines attach different permissions to creation vs submission/deletion, while still sharing implementation internally.

2. Add a server-side permission hint or annotation per method

If full tool splitting is not desirable, consider some form of metadata/annotation that lets clients understand that:

• method: "create" (without event) is "low-risk, pending-only"
• method: "submit_pending" and method: "delete_pending" are "higher-risk, finalizing/destructive"

so they can enforce stricter prompts or denials for the latter two, even under a single tool name.

3. At minimum, document the permissioning implications

A note in the README or docs that "if your client permission model is per-tool-name, you cannot allow create while denying submit_pending/delete_pending" would help users reason about the tradeoff.

Why this matters

There's a meaningful security and UX distinction between:

• letting an agent open a pending review and propose comments, versus
• letting an agent actually submit or delete reviews without human oversight.

Right now, clients that operate at the MCP tool level either have to:

• allow all three (create + submit_pending + delete_pending), or
• prompt/deny all three,

which removes a useful "middle ground" of: always allow pending review creation, but gate final submission/deletion.

A small API surface adjustment (or richer annotations) would make it much easier for clients to implement that pattern.

Related

• FeatureFlagPullRequestsGranular already exists in the codebase as a feature flag for splitting granular PR tools — this issue is in the same spirit for the review write path.
• The tools.go comment mentions: // Granular pull request tools (feature-flagged, replace consolidated update_pull_request/pull_request_review_write), suggesting this split has already been considered.

[policylayer-dan]

This is a real design tension in MCP today. The consolidated-tool pattern saves context tokens but collapses the permission surface to a single bit, which is too coarse for cases like yours.

Two workarounds at the client side worth knowing:

1. Argument-aware interception. Some MCP proxy/middleware layers let you inspect the full tool-call payload before it reaches the server, so you can apply an approval gate based on the method argument value rather than just the tool name. This gives you "auto-allow create, require human approval for submit_pending" without needing the server to split the tool.

2. Upstream change. Filing this against the server repo (which you've done) is the right call. The server could expose these as three distinct tools and let clients that care about token budget collapse them themselves.

If you're running agents across MCP servers you don't control, the argument-level approval gate pattern is exactly what PolicyLayer handles, including manual approval gates on specific tool-call payloads before execution.

[Ram9199]

This issue looks like a clean example of permission boundaries being hidden by tool shape: one pull_request_review_write tool can represent pending-only creation, final submission, or deletion, but a policy engine sees too little distinction to bind approval to the actual operation.

Limitation: splitting tools or adding annotations is a GitHub MCP design choice. A reusable invariant would only say that the approval/check must bind to the specific method and parameters being authorized, not just to a broad tool name.

For that invariant, would you want it as something the server/client depends on, a library, a spec/conformance test-kit, or implemented directly inside github-mcp-server?

[SamMorrowDrums]

@kaovilai there is actually a feature flag (that users can even enable on the client side of remote server via headers too) for issues and pull requests granular tools.

It is true that consolidated tools requires either using over-privileged tokens or accepting 403s and having to guide the agent. That's trade we had to make because the number of token sensitive users is very high, and so we had to prioritise reducing server token footprint above alternatives.

If the landscape matures with tool search tool etc. then we can do a lot more.

[kaovilai]

Right now I just create pretooluse hook to block unwanted parameters from being used. Works well so far to steer

[tiagovilasboas]

Thanks, SamMorrowDrums — your granular-flag reply captures the practical tradeoff well. As a small docs-first step, I’d suggest documenting the permission tradeoff in the README for consolidated tools. When the granular flag is on, we should also ensure the review-write path is split for create/submit/delete, or that DestructiveHint accurately marks submit/delete. That keeps the permission model explicit without requiring a broad rewrite.