Add support for org-level discussions in list_discussion_categories tool #819
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the list_discussion_categories tool to support organization-level discussions by making the repo parameter optional and automatically defaulting to .github when querying at the organization level.
- Makes the
repoparameter optional instead of required - Adds logic to default to
.githubrepo whenrepois not provided for organization-level queries - Updates tool description and documentation to clarify the new behavior
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/github/discussions.go | Makes repo parameter optional and adds logic to default to .github for org-level queries |
| pkg/github/discussions_test.go | Adds comprehensive test coverage for both repository and organization-level scenarios |
| README.md | Updates documentation to reflect the optional repo parameter |
Comments suppressed due to low confidence (2)
pkg/github/discussions_test.go:648
- The test verifies that only 'owner' is required but doesn't verify that 'repo' is not in the required array. Consider adding an explicit assertion that 'repo' is not required to make the test more complete.
mockClient := githubv4.NewClient(nil)
toolDef, _ := ListDiscussionCategories(stubGetGQLClientFn(mockClient), translations.NullTranslationHelper)
assert.Equal(t, "list_discussion_categories", toolDef.Name)
assert.NotEmpty(t, toolDef.Description)
assert.Contains(t, toolDef.Description, "or organisation")
assert.Contains(t, toolDef.InputSchema.Properties, "owner")
assert.Contains(t, toolDef.InputSchema.Properties, "repo")
assert.ElementsMatch(t, toolDef.InputSchema.Required, []string{"owner"})
pkg/github/discussions_test.go:733
- Consider adding a test case where
repois explicitly set to an empty string to verify the behavior when the parameter is present but empty, as this might behave differently from when the parameter is completely omitted.
name: "list org-level discussion categories (no repo provided)",
reqParams: map[string]interface{}{
"owner": "owner",
// repo is not provided, it will default to ".github"
},
mattdholloway
approved these changes
Aug 5, 2025
Contributor
mattdholloway
left a comment
There was a problem hiding this comment.
Test improvements look great 🚀
nickytonline
pushed a commit
to nickytonline/github-mcp-http
that referenced
this pull request
Oct 4, 2025
…ool (github#819) * hide implementation detail for org-level queries * update tests * autogen * made tests consistent with other tests
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Closes: #818
This PR improves the existing
list_discussion_categoriestool, by adding logic to handle organisation-level discussions. (It implements similar logic to what I implemented in PR #775 )Changes introduced
Before, the "repo" argument was required, so when the user would ask to query org-level discussions the model would either (i) hallucinate random values for
repoor (ii) it would have to know the implementation detail about the Github API which is that org-level discussions are stored in a hidden.githubrepo.With these changes:
repoargument is made optionalrepoif discussions are to be queried at the org-levelrepovalue is deterministically set to.githubwhen neededBefore (the model hallucinates repo values)
After (the model correctly omits the repo argument, and the tool uses the deterministic flow to query org-level discussions)
