Add ListGlobalSecurityAdvisories and GetGlobalSecurityAdvisory #693
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for listing and retrieving global security advisories from the GitHub Advisory Database by introducing two new tools, wiring them into the toolset group, and providing corresponding tests and snapshots.
- Registers a new
global_security_advisoriestoolset inDefaultToolsetGroup - Implements
ListGlobalSecurityAdvisoriesandGetGlobalSecurityAdvisoryhandlers with option parsing - Adds unit tests and tool definition snapshots for both tools
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/github/tools.go | Registers the new global_security_advisories toolset |
| pkg/github/global_security_advisories.go | Implements handlers for listing and getting global security advisories |
| pkg/github/global_security_advisories_test.go | Adds unit tests covering the new advisory tools |
| pkg/github/toolsnaps/list_global_security_advisories.snap | Snapshot for the list tool definition |
| pkg/github/toolsnaps/get_global_security_advisory.snap | Snapshot for the get tool definition |
Comments suppressed due to low confidence (1)
pkg/github/global_security_advisories_test.go:39
- There are no test cases covering error handling when the ListGlobalSecurityAdvisories API returns a non-200 status or the client errors. Add tests to simulate failures and verify the handler returns an error result.
tests := []struct {
| return nil, fmt.Errorf("failed to get GitHub client: %w", err) | ||
| } | ||
|
|
||
| advisory, resp, err := client.SecurityAdvisories.GetGlobalSecurityAdvisories(ctx, ghsaID) |
There was a problem hiding this comment.
The call to client.SecurityAdvisories.GetGlobalSecurityAdvisories uses an incorrect method name; the correct method is GetGlobalSecurityAdvisory (singular). Update this to call the intended API.
| advisory, resp, err := client.SecurityAdvisories.GetGlobalSecurityAdvisories(ctx, ghsaID) | |
| advisory, resp, err := client.SecurityAdvisories.GetGlobalSecurityAdvisory(ctx, ghsaID) |
Copilot uses AI. Check for mistakes.
| // Note: EPSS parameters may not be supported in current Go SDK version | ||
| // Check if these fields exist before using them | ||
| // For now, we accept the parameters but don't use them in the API call | ||
| if _, err := OptionalParam[string](request, "epss_percentage"); err != nil { |
There was a problem hiding this comment.
The epss_percentage and epss_percentile parameters are parsed but never applied to the API call, which can confuse users. Consider implementing support for these options or removing them from the tool signature until they're supported.
Copilot uses AI. Check for mistakes.
Closes: #684