Skip to content

Remove comments from create_pull_request_review #343

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
toby opened this issue Apr 24, 2025 · 3 comments
Open

Remove comments from create_pull_request_review #343

toby opened this issue Apr 24, 2025 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@toby
Copy link
Member

toby commented Apr 24, 2025

Due to OpenAI, Windsurf and Gemini all handling JSON schemas in a conflicting manner, we need to remove commenting from create_pull_request_review. The LLM should still be able to leave comments on PRs using add_pull_request_review_comment. More info on modelcontextprotocol/modelcontextprotocol/issues/410.

@toby toby added the bug Something isn't working label Apr 24, 2025
@williammartin williammartin self-assigned this Apr 25, 2025
@williammartin
Copy link
Collaborator

To do this we're going to need to adjust add_pull_request_review_comment to use the GraphQL API, because the REST API doesn't allow you to add new comments to an existing review: microsoft/vscode-pull-request-github#908 (comment)

@williammartin
Copy link
Collaborator

williammartin commented Apr 25, 2025

I have a branch now that implements most of this in an MVP manner: 1e9fbbc

Here's the results of the e2e test running:

➜  git:(343-remove-comments-from-create_pull_request_review) ✗ GITHUB_MCP_SERVER_E2E_TOKEN=$(gh auth token) go test -v -count=1 -run  "TestPullRequestReview" --tags e2e ./e2e

=== RUN   TestPullRequestReview
=== PAUSE TestPullRequestReview
=== CONT  TestPullRequestReview
    e2e_test.go:45: Building Docker image for e2e tests...
    e2e_test.go:118: Starting Stdio MCP client...
    e2e_test.go:221: Getting current user...
    e2e_test.go:250: Creating repository williammartin/github-mcp-server-e2e-TestPullRequestReview-1745611991474...
    e2e_test.go:274: Creating branch in williammartin/github-mcp-server-e2e-TestPullRequestReview-1745611991474...
    e2e_test.go:291: Creating commit with new file in williammartin/github-mcp-server-e2e-TestPullRequestReview-1745611991474...
    e2e_test.go:319: Creating pull request in williammartin/github-mcp-server-e2e-TestPullRequestReview-1745611991474...
    e2e_test.go:334: Creating pending review for pull request in williammartin/github-mcp-server-e2e-TestPullRequestReview-1745611991474...
    e2e_test.go:359: Adding review comment to pull request in williammartin/github-mcp-server-e2e-TestPullRequestReview-1745611991474...
    e2e_test.go:373: Submitting review for pull request in williammartin/github-mcp-server-e2e-TestPullRequestReview-1745611991474...
    e2e_test.go:387: Getting reviews for pull request in williammartin/github-mcp-server-e2e-TestPullRequestReview-1745611991474...
    e2e_test.go:259: Deleting repository williammartin/github-mcp-server-e2e-TestPullRequestReview-1745611991474...
--- PASS: TestPullRequestReview (7.81s)
PASS
ok      github.com/github/github-mcp-server/e2e 8.103s
Image showing PR with review in web browser

All that said, if we move to GQL, we might still be able to expose adding threads and submission in one flow since the types might be different. Haven't explored it yet.

@MrOrz
Copy link

MrOrz commented Apr 27, 2025

For those who encountering argument schema type issue when connecting Github MCP with OpenAI / Gemini Github: pin docker image's version to v0.1.1 helps.

Tried out v0.2.0 and v0.2.1, both have errors:

  • Github MCP v0.2.1 with OpenAI models: OpenAIException - Invalid schema for function 'create_pull_request_review': 'STRING' is not valid under any of the given schemas.
  • Github MCP v0.2.1 with Gemini models: {'error': {'code': 400, 'message': 'Unable to submit request because create_pull_request_reviewfunctionDeclarationparameters.comments.sideschema specified other fields alongside any_of. When using any_of, it must be the only field set. Learn more: https://cloud.google.com/vertex-ai/generative-ai/docs/multimodal/function-calling', 'status': 'INVALID_ARGUMENT'}}
  • Github MCP v0.2.0 with ADK (any model): Pydantic error Input should be 'TYPE_UNSPECIFIED', 'STRING', 'NUMBER', 'INTEGER', 'BOOLEAN', 'ARRAY' or 'OBJECT' [type=enum, input_value=['number', 'null'], input_type=list]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

3 participants