github / github/github-mcp-server

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

Ouverte
#2,525 5 commentaires 2 réactions 0 personnes assignées Voir sur GitHub
request ai review
Langage dominant
Go
Étoiles
33k
Forks
5k
Merge moyen
2 j 1 h
PR mergées (30 j)
52

Description

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.

Guide de contribution

Ouvrir le guide de contribution

Évaluation

Cette issue n'a pas encore été évaluée.

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.