Feature/chewy standards tool #510
Conversation
There was a problem hiding this comment.
Pull Request Overview
Adds a new MCP tool for fetching development standards from Chewy’s GitHub repository, including registration, implementation, tests, and documentation updates.
- Introduces
GetChewyStandardstool and registers it inInitToolsets - Implements handler logic to fetch either an explicit path or the full repository tree
- Adds comprehensive tests and updates the README
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/github/tools.go | Registers get_chewy_standards in the read-only toolset |
| pkg/github/repositories.go | Implements GetChewyStandards handler and tool definition |
| pkg/github/repositories_test.go | Adds tests for default, custom-path, and error cases |
| README.md | Documents the new get_chewy_standards tool |
Comments suppressed due to low confidence (3)
pkg/github/repositories_test.go:1968
- Add a test case that supplies a custom
refvalue and assert that the handler uses that ref when fetching contents.
func Test_GetChewyStandards(t *testing.T) {
README.md:512
- Align the bullet levels and indentation for the
get_chewy_standardssection so it matches the existing format, and ensure each sub-item is nested correctly.
- **get_chewy_standards** - Get development standards and guidelines from Chewy's repository
README.md:604
- Remove the stray trailing `- `` at the end of the file; it appears to be an accidental leftover.
- `
|
|
||
| if searchWholeRepo { | ||
| // Get the entire repository tree | ||
| tree, resp, err := client.Git.GetTree(ctx, owner, repo, "HEAD", true) |
There was a problem hiding this comment.
Use the user-provided ref parameter when fetching the repository tree instead of hardcoding "HEAD" so that branch/tag/commit references are respected.
| tree, resp, err := client.Git.GetTree(ctx, owner, repo, "HEAD", true) | |
| tree, resp, err := client.Git.GetTree(ctx, owner, repo, ref, true) |
Copilot uses AI. Check for mistakes.
| defer func() { _ = resp.Body.Close() }() | ||
| } | ||
|
|
||
| if resp.StatusCode == 200 && tree != nil { |
There was a problem hiding this comment.
Add a check for non-200 responses from GetTree (e.g. resp.StatusCode != http.StatusOK) and return an error, rather than continuing silently when the tree isn’t fetched.
| if resp.StatusCode == 200 && tree != nil { | |
| if resp.StatusCode != http.StatusOK { | |
| return mcp.NewToolResultError(fmt.Sprintf("Failed to get repository tree: received status code %d", resp.StatusCode)), nil | |
| } | |
| if tree != nil { |
Copilot uses AI. Check for mistakes.
| continue | ||
| } | ||
| if resp != nil { | ||
| defer func() { _ = resp.Body.Close() }() |
There was a problem hiding this comment.
Avoid deferring resp.Body.Close() inside a loop; close the response body immediately after reading to prevent resource buildup.
Copilot uses AI. Check for mistakes.
Closes: