Add GitHub Discussions Support to MCP Server

[azizalzahrani]

User description

This pull request implements GitHub Discussions support in the GitHub MCP server, addressing issue github#213. The new functionality allows LLMs to interact with discussions in GitHub repositories, enhancing community engagement.

Changes Made:

• New Tools Added:
  • ListDiscussions: Lists discussions in a repository with filtering options.
  • GetDiscussion: Retrieves details of a specific discussion.
  • GetDiscussionCategories: Fetches discussion categories in a repository.
  • GetDiscussionComments: Gets comments for a specific discussion.
  • AddDiscussionComment: Adds a comment to a discussion.
  • CreateDiscussion: Creates a new discussion in a repository.

Testing:

• Comprehensive tests have been added to ensure the functionality of the new tools, covering various scenarios including success and error cases.

This enhancement will significantly benefit community-driven repositories by providing a reliable way for LLMs to interact with discussions.

---

This pull request was co-created with Cosine Genie

Original Task: github-mcp-server/9302sccg5r2c
Author: Aziz Alzahrani

---

PR Type

Enhancement, Tests

---

Description

• Added GitHub Discussions tools for repository interaction.

  • Tools include listing, retrieving, creating discussions, and managing comments.

• Integrated tools into the server with conditional read-only checks.

• Comprehensive test coverage for all tools and scenarios.

---

Changes walkthrough 📝

	Relevant files
Enhancement	discussions.go Implement GitHub Discussions tools                                              pkg/github/discussions.go Added tools for GitHub Discussions management. Implemented functions for listing, retrieving, and creating discussions. Added support for managing discussion comments and categories. +455/-0  server.go Integrate GitHub Discussions tools into server                      pkg/github/server.go Integrated GitHub Discussions tools into the server. Added conditional checks for read-only mode. +11/-0
Tests	discussions_test.go Add tests for GitHub Discussions tools                                      pkg/github/discussions_test.go Added tests for all GitHub Discussions tools. Covered success and error scenarios for each tool. Verified tool definitions and input schema. +768/-0

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Error Handling The error handling in ListDiscussions and other functions is inconsistent. Some errors are returned directly while others are wrapped in a tool result. This could lead to confusing behavior for API consumers. if err != nil { return nil, fmt.Errorf("failed to list discussions: %w", err) } defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusOK { body, err := io.ReadAll(resp.Body) if err != nil { return nil, fmt.Errorf("failed to read response body: %w", err) } return mcp.NewToolResultError(fmt.Sprintf("failed to list discussions: %s", string(body))), nil } Resource Cleanup The defer statements for response body closing should check if resp is nil before attempting to close the body to avoid potential nil pointer dereference. defer func() { _ = resp.Body.Close() }()

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Prevent nil pointer dereference The response body is being closed without checking if resp is nil. If the API call fails before a response is created, this will cause a nil pointer dereference panic. Always check if resp is nil before attempting to close its body. pkg/github/discussions.go [100] -defer func() { _ = resp.Body.Close() }() +if resp != nil { + defer resp.Body.Close() +} Apply this suggestion Suggestion importance[1-10]: 9 __ Why: The current code could cause a nil pointer dereference panic if the API call fails before a response is created. This is a critical bug that could crash the application in error scenarios.	High
	Fix optional parameter error handling The error handling for optional parameters is inconsistent with the function's purpose. Since OptionalParam is meant to handle optional parameters, returning an error when the parameter is missing doesn't align with its intended use. The error should only be checked if it indicates a real problem, not just a missing parameter. pkg/github/discussions.go [55-58] direction, err := OptionalParam[string](request, "direction") -if err != nil { +if err != nil && err != ErrParamNotFound { return mcp.NewToolResultError(err.Error()), nil } Apply this suggestion Suggestion importance[1-10]: 8 __ Why: The current implementation treats errors from OptionalParam as fatal, which contradicts its purpose. This could cause the tool to fail unnecessarily when optional parameters are missing, affecting functionality.	Medium
	Fix HTTP status code check The status code check in GetDiscussion, CreateDiscussion, and AddDiscussionComment functions should check for http.StatusCreated (201) instead of http.StatusOK (200) since these are creation operations. GitHub API returns 201 for successful resource creation. pkg/github/discussions.go [361-367] -if resp.StatusCode != http.StatusOK { +if resp.StatusCode != http.StatusCreated { body, err := io.ReadAll(resp.Body) if err != nil { return nil, fmt.Errorf("failed to read response body: %w", err) } - return mcp.NewToolResultError(fmt.Sprintf("failed to list discussions: %s", string(body))), nil + return mcp.NewToolResultError(fmt.Sprintf("failed to create discussion: %s", string(body))), nil } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 7 __ Why: The code is checking for http.StatusOK (200) instead of http.StatusCreated (201) for creation operations. GitHub API returns 201 for successful resource creation, so this mismatch could cause false error reporting.	Medium
More