github / github/github-mcp-server

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

Đang mở
#2,525 5 bình luận 2 reaction 0 người được giao Xem trên GitHub
request ai review
Ngôn ngữ chính
Go
Star
33k
Fork
5k
Merge trung bình
2 ngày 1 giờ
Pull request đã merge (30 ngày)
52

Mô tả

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.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.