diff --git a/README.md b/README.md index 432541f434..a875a17d52 100644 --- a/README.md +++ b/README.md @@ -704,6 +704,15 @@ For negotiated supported protocol versions `2026-07-28` or newer, the consolidat - `owner`: The owner of the repository. (string, required) - `repo`: The name of the repository. (string, required) +- **list_code_quality_findings** - List code quality findings + - **OAuth Challenge Scopes**: `repo` + - `after`: Cursor for pagination. Use the cursor from the previous response. (string, optional) + - `direction`: Sort by creation date: `desc` (newest first, default) or `asc` (oldest first). (string, optional) + - `owner`: The owner of the repository. (string, required) + - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) + - `repo`: The name of the repository. (string, required) + - `state`: Filter code quality findings by state. Defaults to open (string, optional) +
diff --git a/pkg/github/__toolsnaps__/list_code_quality_findings.snap b/pkg/github/__toolsnaps__/list_code_quality_findings.snap new file mode 100644 index 0000000000..3e62c218bd --- /dev/null +++ b/pkg/github/__toolsnaps__/list_code_quality_findings.snap @@ -0,0 +1,222 @@ +{ + "annotations": { + "idempotentHint": false, + "readOnlyHint": true, + "title": "List code quality findings" + }, + "description": "List code quality findings in a GitHub repository.", + "inputSchema": { + "properties": { + "after": { + "description": "Cursor for pagination. Use the cursor from the previous response.", + "type": "string" + }, + "direction": { + "description": "Sort by creation date: `desc` (newest first, default) or `asc` (oldest first).", + "enum": [ + "asc", + "desc" + ], + "type": "string" + }, + "owner": { + "description": "The owner of the repository.", + "type": "string" + }, + "perPage": { + "description": "Results per page for pagination (min 1, max 100)", + "maximum": 100, + "minimum": 1, + "type": "number" + }, + "repo": { + "description": "The name of the repository.", + "type": "string" + }, + "state": { + "default": "open", + "description": "Filter code quality findings by state. Defaults to open", + "enum": [ + "open", + "dismissed" + ], + "type": "string" + } + }, + "required": [ + "owner", + "repo" + ], + "type": "object" + }, + "name": "list_code_quality_findings", + "outputSchema": { + "additionalProperties": false, + "properties": { + "findings": { + "items": { + "additionalProperties": false, + "properties": { + "created_at": { + "description": "Finding creation time (RFC 3339).", + "type": [ + "null", + "string" + ] + }, + "location": { + "additionalProperties": false, + "properties": { + "end_column": { + "type": [ + "null", + "integer" + ] + }, + "end_line": { + "type": [ + "null", + "integer" + ] + }, + "path": { + "type": [ + "null", + "string" + ] + }, + "start_column": { + "type": [ + "null", + "integer" + ] + }, + "start_line": { + "type": [ + "null", + "integer" + ] + } + }, + "type": [ + "null", + "object" + ] + }, + "message": { + "additionalProperties": false, + "properties": { + "markdown": { + "type": "string" + }, + "text": { + "type": "string" + } + }, + "required": [ + "text", + "markdown" + ], + "type": [ + "null", + "object" + ] + }, + "number": { + "type": [ + "null", + "integer" + ] + }, + "rule": { + "additionalProperties": false, + "properties": { + "category": { + "type": [ + "null", + "string" + ] + }, + "description": { + "type": [ + "null", + "string" + ] + }, + "help": { + "type": [ + "null", + "string" + ] + }, + "id": { + "type": [ + "null", + "string" + ] + }, + "severity": { + "type": [ + "null", + "string" + ] + }, + "title": { + "type": [ + "null", + "string" + ] + } + }, + "type": [ + "null", + "object" + ] + }, + "state": { + "type": [ + "null", + "string" + ] + } + }, + "type": [ + "null", + "object" + ] + }, + "type": [ + "null", + "array" + ] + }, + "pageInfo": { + "additionalProperties": false, + "properties": { + "hasNextPage": { + "type": "boolean" + }, + "hasPreviousPage": { + "type": "boolean" + }, + "nextCursor": { + "type": "string" + }, + "prevCursor": { + "type": "string" + } + }, + "required": [ + "hasNextPage", + "hasPreviousPage" + ], + "type": "object" + } + }, + "required": [ + "findings", + "pageInfo" + ], + "type": "object" + } +} \ No newline at end of file diff --git a/pkg/github/code_quality.go b/pkg/github/code_quality.go index e5ef82c394..a891623e5c 100644 --- a/pkg/github/code_quality.go +++ b/pkg/github/code_quality.go @@ -6,6 +6,8 @@ import ( "fmt" "io" "net/http" + "net/url" + "strconv" "github.com/google/jsonschema-go/jsonschema" "github.com/modelcontextprotocol/go-sdk/mcp" @@ -99,6 +101,133 @@ func GetCodeQualityFinding(t translations.TranslationHelperFunc) inventory.Serve ) } +func ListCodeQualityFindings(t translations.TranslationHelperFunc) inventory.ServerTool { + schema := &jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "The owner of the repository.", + }, + "repo": { + Type: "string", + Description: "The name of the repository.", + }, + "state": { + Type: "string", + Description: "Filter code quality findings by state. Defaults to open", + Enum: []any{"open", "dismissed"}, + Default: json.RawMessage(`"open"`), + }, + "direction": { + Type: "string", + Description: "Sort by creation date: `desc` (newest first, default) or `asc` (oldest first).", + Enum: []any{"asc", "desc"}, + }, + }, + Required: []string{"owner", "repo"}, + } + WithCursorPagination(schema) + + return NewToolWithSchemaOptions[ListCodeQualityFindingsInput, CodeQualityFindingsOutput]( + ToolsetMetadataCodeQuality, + mcp.Tool{ + Name: "list_code_quality_findings", + Description: t("TOOL_LIST_CODE_QUALITY_FINDINGS_DESCRIPTION", "List code quality findings in a GitHub repository."), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_LIST_CODE_QUALITY_FINDINGS_USER_TITLE", "List code quality findings"), + ReadOnlyHint: true, + }, + InputSchema: schema, + }, + scopes.PublicRead(scopes.Repo), + inventory.TypedSchemaOptions{ + ValidationInputSchema: inventory.CloneSchemaWithoutDefaults(schema), + }, + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args ListCodeQualityFindingsInput) (*mcp.CallToolResult, CodeQualityFindingsOutput, error) { + if args.Owner == "" { + return utils.NewToolResultError("missing required parameter: owner"), CodeQualityFindingsOutput{}, nil + } + if args.Repo == "" { + return utils.NewToolResultError("missing required parameter: repo"), CodeQualityFindingsOutput{}, nil + } + + pagination := securityCursorPagination(args.PerPage, args.After) + client, err := deps.GetClient(ctx) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), CodeQualityFindingsOutput{}, nil + } + + query := url.Values{} + if args.State != "" { + query.Set("state", args.State) + } + if args.Direction != "" { + query.Set("direction", args.Direction) + } + query.Set("per_page", strconv.Itoa(pagination.PerPage)) + if pagination.After != "" { + query.Set("after", pagination.After) + } + apiURL := fmt.Sprintf("repos/%s/%s/code-quality/findings?%s", url.PathEscape(args.Owner), url.PathEscape(args.Repo), query.Encode()) + req, err := client.NewRequest(ctx, http.MethodGet, apiURL, nil) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to create request", err), CodeQualityFindingsOutput{}, nil + } + + var rawFindings json.RawMessage + resp, err := client.Do(req, &rawFindings) + if resp != nil { + defer func() { _ = resp.Body.Close() }() + } + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, + fmt.Sprintf("failed to list findings for repository '%s/%s'", args.Owner, args.Repo), + resp, + err, + ), CodeQualityFindingsOutput{}, nil + } + + if resp.StatusCode != http.StatusOK { + body, err := io.ReadAll(resp.Body) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to read response body", err), CodeQualityFindingsOutput{}, nil + } + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list findings", resp, body), CodeQualityFindingsOutput{}, nil + } + + if len(rawFindings) == 0 { + rawFindings = json.RawMessage("null") + } + var findings []*CodeQualityFindingOutput + if err := json.Unmarshal(rawFindings, &findings); err != nil { + return utils.NewToolResultErrorFromErr("failed to decode findings", err), CodeQualityFindingsOutput{}, nil + } + + apiPageInfo := buildPageInfo(resp) + output := CodeQualityFindingsOutput{ + Findings: findings, + PageInfo: SecurityPageInfo(apiPageInfo), + } + + response := struct { + Findings json.RawMessage `json:"findings"` + PageInfo pageInfo `json:"pageInfo"` + }{ + Findings: rawFindings, + PageInfo: apiPageInfo, + } + r, err := json.Marshal(response) + if err != nil { + return utils.NewToolResultErrorFromErr("failed to marshal findings", err), CodeQualityFindingsOutput{}, nil + } + + return utils.NewToolResultText(string(r)), output, nil + }, + normalizeTypedReadArguments(nil, false), + ) +} + func marshalLegacyCodeQualityFinding(rawFinding json.RawMessage) ([]byte, error) { // Match main's map-based formatting, including key ordering and number encoding. var finding map[string]any diff --git a/pkg/github/code_quality_test.go b/pkg/github/code_quality_test.go index 0542cd2f53..23c694fd44 100644 --- a/pkg/github/code_quality_test.go +++ b/pkg/github/code_quality_test.go @@ -152,3 +152,237 @@ func Test_GetCodeQualityFinding(t *testing.T) { }) } } + +func Test_ListCodeQualityFindings(t *testing.T) { + // Verify tool definition once + toolDef := ListCodeQualityFindings(translations.NullTranslationHelper) + tool := toolDef.Tool + testSecurityToolSnapshot(t, tool) + + assert.Equal(t, "list_code_quality_findings", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.True(t, tool.Annotations.ReadOnlyHint, "list_code_quality_findings tool should be read-only") + + schema, ok := tool.InputSchema.(*jsonschema.Schema) + require.True(t, ok, "InputSchema should be *jsonschema.Schema") + assert.Contains(t, schema.Properties, "owner") + assert.Contains(t, schema.Properties, "repo") + assert.Contains(t, schema.Properties, "state") + assert.Contains(t, schema.Properties, "direction") + assert.Contains(t, schema.Properties, "perPage") + assert.Contains(t, schema.Properties, "after") + assert.NotContains(t, schema.Properties, "page", "the API only supports cursor pagination") + assert.ElementsMatch(t, schema.Required, []string{"owner", "repo"}) + assert.Equal(t, []any{"open", "dismissed"}, schema.Properties["state"].Enum) + assert.Equal(t, []any{"asc", "desc"}, schema.Properties["direction"].Enum) + + openFinding := json.RawMessage(`{ + "number": 116, + "state": "open", + "url": "https://api.github.com/repos/owner/repo/code-quality/findings/116", + "rule": {"id": "js/conditional-comment", "title": "Conditional comments", "severity": "warning", "category": "reliability"}, + "location": {"path": "src/app.js", "start_line": 21, "end_line": 21}, + "message": {"text": "Do not use conditional comments.", "markdown": "Do not use conditional comments."}, + "created_at": "2026-09-08T18:16:36Z" + }`) + dismissedFinding := json.RawMessage(`{ + "number": 7, + "state": "dismissed", + "url": "https://api.github.com/repos/owner/repo/code-quality/findings/7", + "rule": {"id": "js/unused-local-variable", "title": "Unused variable", "severity": "note", "category": "maintainability"}, + "location": {"path": "src/util.js", "start_line": 3}, + "message": {"text": "Unused variable x.", "markdown": "Unused variable x."}, + "created_at": "2026-09-01T10:00:00Z" + }`) + + writeFindings := func(t *testing.T, link string, findings ...json.RawMessage) http.HandlerFunc { + return func(w http.ResponseWriter, _ *http.Request) { + if link != "" { + w.Header().Set("Link", link) + } + w.WriteHeader(http.StatusOK) + b, err := json.Marshal(findings) + require.NoError(t, err) + _, _ = w.Write(b) + } + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]any + expectError bool + expectedNumbers []int + expectedStates []string + expectedNextCursor string + expectedErrMsg string + }{ + { + name: "successful listing with defaults", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposCodeQualityFindingsByOwnerByRepo: expectQueryParams(t, map[string]string{ + "per_page": "30", + }).andThen(writeFindings(t, "", openFinding)), + }), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + }, + expectedNumbers: []int{116}, + expectedStates: []string{"open"}, + }, + { + name: "successful dismissed listing in ascending order", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposCodeQualityFindingsByOwnerByRepo: expectQueryParams(t, map[string]string{ + "state": "dismissed", + "direction": "asc", + "per_page": "30", + }).andThen(writeFindings(t, "", dismissedFinding)), + }), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "state": "dismissed", + "direction": "asc", + }, + expectedNumbers: []int{7}, + expectedStates: []string{"dismissed"}, + }, + { + name: "successful listing passes cursor and page size", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposCodeQualityFindingsByOwnerByRepo: expectQueryParams(t, map[string]string{ + "after": "Y3Vyc29yOnYyOpK5", + "per_page": "100", + }).andThen(writeFindings(t, "", openFinding)), + }), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "after": "Y3Vyc29yOnYyOpK5", + "perPage": float64(100), + }, + expectedNumbers: []int{116}, + expectedStates: []string{"open"}, + }, + { + name: "successful listing surfaces next cursor and ignores empty last cursor", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposCodeQualityFindingsByOwnerByRepo: expectQueryParams(t, map[string]string{ + "per_page": "2", + }).andThen(writeFindings(t, + `; rel="next", `+ + `; rel="last"`, + openFinding, dismissedFinding, + )), + }), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + "perPage": float64(2), + }, + expectedNumbers: []int{116, 7}, + expectedStates: []string{"open", "dismissed"}, + expectedNextCursor: "nextcursor123", + }, + { + name: "listing forbidden", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposCodeQualityFindingsByOwnerByRepo: func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"message": "Code quality is not enabled for this repository."}`)) + }, + }), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + }, + expectError: true, + expectedErrMsg: "failed to list findings for repository 'owner/repo'", + }, + { + name: "listing not found", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposCodeQualityFindingsByOwnerByRepo: func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"message": "Not Found"}`)) + }, + }), + requestArgs: map[string]any{ + "owner": "owner", + "repo": "repo", + }, + expectError: true, + expectedErrMsg: "Not Found", + }, + { + name: "missing owner", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}), + requestArgs: map[string]any{ + "repo": "repo", + }, + expectError: true, + expectedErrMsg: "missing required parameter: owner", + }, + { + name: "missing repo", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}), + requestArgs: map[string]any{ + "owner": "owner", + }, + expectError: true, + expectedErrMsg: "missing required parameter: repo", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := mustNewGHClient(t, tc.mockedClient) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + + request := createMCPRequest(tc.requestArgs) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + + if tc.expectError { + require.NoError(t, err) + require.True(t, result.IsError) + errorContent := getErrorResult(t, result) + assert.Contains(t, errorContent.Text, tc.expectedErrMsg) + return + } + + require.NoError(t, err) + require.False(t, result.IsError) + + textContent := getTextResult(t, result) + + var returnedResult struct { + Findings []struct { + Number int `json:"number"` + State string `json:"state"` + URL string `json:"url"` + } `json:"findings"` + PageInfo struct { + HasNextPage bool `json:"hasNextPage"` + HasPreviousPage bool `json:"hasPreviousPage"` + NextCursor string `json:"nextCursor"` + PrevCursor string `json:"prevCursor"` + } `json:"pageInfo"` + } + require.NoError(t, json.Unmarshal([]byte(textContent.Text), &returnedResult)) + require.Len(t, returnedResult.Findings, len(tc.expectedNumbers)) + for i, finding := range returnedResult.Findings { + assert.Equal(t, tc.expectedNumbers[i], finding.Number) + assert.Equal(t, tc.expectedStates[i], finding.State) + assert.NotEmpty(t, finding.URL, "legacy text should return findings as the API returns them") + } + assert.Equal(t, tc.expectedNextCursor != "", returnedResult.PageInfo.HasNextPage) + assert.Equal(t, tc.expectedNextCursor, returnedResult.PageInfo.NextCursor) + assert.False(t, returnedResult.PageInfo.HasPreviousPage, "an empty before cursor must not be surfaced") + assert.Empty(t, returnedResult.PageInfo.PrevCursor) + }) + } +} diff --git a/pkg/github/helper_test.go b/pkg/github/helper_test.go index 80a2e523f2..d6c9291680 100644 --- a/pkg/github/helper_test.go +++ b/pkg/github/helper_test.go @@ -114,6 +114,7 @@ const ( GetReposReleasesTagsByOwnerByRepoByTag = "GET /repos/{owner}/{repo}/releases/tags/{tag}" // Code quality endpoints + GetReposCodeQualityFindingsByOwnerByRepo = "GET /repos/{owner}/{repo}/code-quality/findings" GetReposCodeQualityFindingsByOwnerByRepoByFindingNumber = "GET /repos/{owner}/{repo}/code-quality/findings/{finding_number}" // Code scanning endpoints diff --git a/pkg/github/security_outputs.go b/pkg/github/security_outputs.go index 88fb34f12b..ffb19e1d22 100644 --- a/pkg/github/security_outputs.go +++ b/pkg/github/security_outputs.go @@ -18,6 +18,15 @@ type GetCodeQualityFindingInput struct { FindingNumber int `json:"findingNumber"` } +type ListCodeQualityFindingsInput struct { + Owner string `json:"owner"` + Repo string `json:"repo"` + State string `json:"state,omitempty"` + Direction string `json:"direction,omitempty"` + PerPage *int `json:"perPage,omitempty"` + After string `json:"after,omitempty"` +} + type GetSecurityAlertInput struct { Owner string `json:"owner"` Repo string `json:"repo"` @@ -254,6 +263,11 @@ type SecurityPageInfo struct { PrevCursor string `json:"prevCursor,omitempty"` } +type CodeQualityFindingsOutput struct { + Findings []*CodeQualityFindingOutput `json:"findings"` + PageInfo SecurityPageInfo `json:"pageInfo"` +} + type DependabotAlertsOutput struct { Alerts []*DependabotAlertOutput `json:"alerts"` PageInfo SecurityPageInfo `json:"pageInfo"` diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 53503f68fe..5342051c0a 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -299,6 +299,7 @@ func AllTools(t translations.TranslationHelperFunc, opts ...ToolOption) []invent // Code quality tools GetCodeQualityFinding(t), + ListCodeQualityFindings(t), // Code security tools GetCodeScanningAlert(t), diff --git a/pkg/github/typed_security_compatibility_test.go b/pkg/github/typed_security_compatibility_test.go index 994aacb69e..83c5ee333c 100644 --- a/pkg/github/typed_security_compatibility_test.go +++ b/pkg/github/typed_security_compatibility_test.go @@ -42,6 +42,18 @@ func TestSecurityRegisteredFilters(t *testing.T) { arguments: map[string]any{"owner": "owner", "repo": "repo", "perPage": 2}, wantQuery: url.Values{"per_page": {"2"}}, }, + { + name: "code quality omitted state and direction", + tool: ListCodeQualityFindings(translations.NullTranslationHelper), route: GetReposCodeQualityFindingsByOwnerByRepo, + arguments: map[string]any{"owner": "owner", "repo": "repo"}, + wantQuery: url.Values{"per_page": {"30"}}, + }, + { + name: "code quality explicit state and direction", + tool: ListCodeQualityFindings(translations.NullTranslationHelper), route: GetReposCodeQualityFindingsByOwnerByRepo, + arguments: map[string]any{"owner": "owner", "repo": "repo", "state": "dismissed", "direction": "asc"}, + wantQuery: url.Values{"per_page": {"30"}, "state": {"dismissed"}, "direction": {"asc"}}, + }, { name: "code scanning omitted state", tool: ListCodeScanningAlerts(translations.NullTranslationHelper), route: GetReposCodeScanningAlertsByOwnerByRepo, @@ -99,6 +111,7 @@ func TestSecurityEmptyAndErrorOutputs(t *testing.T) { list bool }{ {GetCodeQualityFinding(translations.NullTranslationHelper), GetReposCodeQualityFindingsByOwnerByRepoByFindingNumber, map[string]any{"owner": "owner", "repo": "repo", "findingNumber": 42}, false}, + {ListCodeQualityFindings(translations.NullTranslationHelper), GetReposCodeQualityFindingsByOwnerByRepo, map[string]any{"owner": "owner", "repo": "repo"}, true}, {GetCodeScanningAlert(translations.NullTranslationHelper), GetReposCodeScanningAlertsByOwnerByRepoByAlertNumber, map[string]any{"owner": "owner", "repo": "repo", "alertNumber": 42}, false}, {ListCodeScanningAlerts(translations.NullTranslationHelper), GetReposCodeScanningAlertsByOwnerByRepo, map[string]any{"owner": "owner", "repo": "repo"}, true}, {GetSecretScanningAlert(translations.NullTranslationHelper), GetReposSecretScanningAlertsByOwnerByRepoByAlertNumber, map[string]any{"owner": "owner", "repo": "repo", "alertNumber": 42}, false}, @@ -148,9 +161,12 @@ func TestSecurityEmptyAndErrorOutputs(t *testing.T) { return } if protocol == "2025-11-25" { - if tt.tool.Tool.Name == "list_dependabot_alerts" { + switch tt.tool.Tool.Name { + case "list_dependabot_alerts": assert.JSONEq(t, `{"alerts":`+body+`,"pageInfo":{"hasNextPage":false,"hasPreviousPage":false}}`, getTextResult(t, result).Text) - } else { + case "list_code_quality_findings": + assert.JSONEq(t, `{"findings":`+body+`,"pageInfo":{"hasNextPage":false,"hasPreviousPage":false}}`, getTextResult(t, result).Text) + default: assert.Equal(t, body, getTextResult(t, result).Text) } assert.Nil(t, result.StructuredContent) @@ -175,7 +191,7 @@ func TestSecurityEmptyAndErrorOutputs(t *testing.T) { } if tt.list { assert.JSONEq(t, mustMarshalJSON(t, result.StructuredContent), getTextResult(t, result).Text, "modern lists expose the same DTO through both channels") - if tt.tool.Tool.Name == "list_dependabot_alerts" { + if tt.tool.Tool.Name == "list_dependabot_alerts" || tt.tool.Tool.Name == "list_code_quality_findings" { assert.JSONEq(t, getTextResult(t, result).Text, mustMarshalJSON(t, result.StructuredContent)) } else { assert.Equal(t, body, mustMarshalJSON(t, result.StructuredContent)) @@ -196,6 +212,7 @@ func TestLegacyCodeQualityFindingFormatting(t *testing.T) { func TestSecurityInputSchemasPreserveEnums(t *testing.T) { for _, tool := range []inventory.ServerTool{ + ListCodeQualityFindings(translations.NullTranslationHelper), ListCodeScanningAlerts(translations.NullTranslationHelper), ListDependabotAlerts(translations.NullTranslationHelper), ListGlobalSecurityAdvisories(translations.NullTranslationHelper), diff --git a/pkg/github/typed_security_outputs_test.go b/pkg/github/typed_security_outputs_test.go index e793fcbbd2..db1f1a1ecd 100644 --- a/pkg/github/typed_security_outputs_test.go +++ b/pkg/github/typed_security_outputs_test.go @@ -24,6 +24,8 @@ func securityOutputSchema(t *testing.T, name string) *jsonschema.Schema { switch name { case "get_code_quality_finding": schema, err = jsonschema.For[*CodeQualityFindingOutput](nil) + case "list_code_quality_findings": + schema, err = jsonschema.For[CodeQualityFindingsOutput](nil) case "get_code_scanning_alert": schema, err = jsonschema.For[*CodeScanningAlertOutput](nil) case "list_code_scanning_alerts": @@ -135,8 +137,13 @@ func TestTypedSecurityToolOutputs(t *testing.T) { } globalAdvisory.CWEs = []*github.AdvisoryCWEs{{CWEID: new("CWE-79"), Name: new("Cross-site scripting")}} codeQualityJSON := json.RawMessage(`{"unknown_legacy_field":"preserved in text","url":"https://api.github.com/repos/owner/repo/code-quality/findings/42","state":"open","rule":{"id":"test-rule","description":"Test rule"},"number":42}`) + codeQualityListJSON := json.RawMessage(`[` + string(codeQualityJSON) + `]`) legacyOutputs := map[string]any{ - "get_code_quality_finding": codeQualityFinding, + "get_code_quality_finding": codeQualityFinding, + "list_code_quality_findings": struct { + Findings json.RawMessage `json:"findings"` + PageInfo pageInfo `json:"pageInfo"` + }{Findings: codeQualityListJSON}, "get_code_scanning_alert": codeScanningAlert, "list_code_scanning_alerts": []*github.Alert{codeScanningAlert}, "get_secret_scanning_alert": secretScanningAlert, @@ -155,22 +162,24 @@ func TestTypedSecurityToolOutputs(t *testing.T) { deps := BaseDeps{ Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ GetReposCodeQualityFindingsByOwnerByRepoByFindingNumber: mockResponse(t, http.StatusOK, codeQualityJSON), - GetReposByOwnerByRepo: mockResponse(t, http.StatusOK, &github.Repository{Private: new(true)}), - GetReposCodeScanningAlertsByOwnerByRepoByAlertNumber: mockResponse(t, http.StatusOK, codeScanningAlert), - GetReposCodeScanningAlertsByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.Alert{codeScanningAlert}), - GetReposSecretScanningAlertsByOwnerByRepoByAlertNumber: mockResponse(t, http.StatusOK, secretScanningAlert), - GetReposSecretScanningAlertsByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.SecretScanningAlert{secretScanningAlert}), - GetReposDependabotAlertsByOwnerByRepoByAlertNumber: mockResponse(t, http.StatusOK, dependabotAlert), - GetReposDependabotAlertsByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.DependabotAlert{dependabotAlert}), - GetAdvisories: mockResponse(t, http.StatusOK, []*github.GlobalSecurityAdvisory{globalAdvisory}), - GetAdvisoriesByGhsaID: mockResponse(t, http.StatusOK, globalAdvisory), - GetReposSecurityAdvisoriesByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.SecurityAdvisory{advisory}), - GetOrgsSecurityAdvisoriesByOrg: mockResponse(t, http.StatusOK, []*github.SecurityAdvisory{advisory}), + GetReposCodeQualityFindingsByOwnerByRepo: mockResponse(t, http.StatusOK, codeQualityListJSON), + GetReposByOwnerByRepo: mockResponse(t, http.StatusOK, &github.Repository{Private: new(true)}), + GetReposCodeScanningAlertsByOwnerByRepoByAlertNumber: mockResponse(t, http.StatusOK, codeScanningAlert), + GetReposCodeScanningAlertsByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.Alert{codeScanningAlert}), + GetReposSecretScanningAlertsByOwnerByRepoByAlertNumber: mockResponse(t, http.StatusOK, secretScanningAlert), + GetReposSecretScanningAlertsByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.SecretScanningAlert{secretScanningAlert}), + GetReposDependabotAlertsByOwnerByRepoByAlertNumber: mockResponse(t, http.StatusOK, dependabotAlert), + GetReposDependabotAlertsByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.DependabotAlert{dependabotAlert}), + GetAdvisories: mockResponse(t, http.StatusOK, []*github.GlobalSecurityAdvisory{globalAdvisory}), + GetAdvisoriesByGhsaID: mockResponse(t, http.StatusOK, globalAdvisory), + GetReposSecurityAdvisoriesByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.SecurityAdvisory{advisory}), + GetOrgsSecurityAdvisoriesByOrg: mockResponse(t, http.StatusOK, []*github.SecurityAdvisory{advisory}), })), featureChecker: featureCheckerFor(FeatureFlagIFCLabels), } tools := []inventory.ServerTool{ GetCodeQualityFinding(translations.NullTranslationHelper), + ListCodeQualityFindings(translations.NullTranslationHelper), GetCodeScanningAlert(translations.NullTranslationHelper), ListCodeScanningAlerts(translations.NullTranslationHelper), GetSecretScanningAlert(translations.NullTranslationHelper), @@ -184,6 +193,7 @@ func TestTypedSecurityToolOutputs(t *testing.T) { } calls := []mcp.CallToolParams{ {Name: "get_code_quality_finding", Arguments: map[string]any{"owner": "owner", "repo": "repo", "findingNumber": "42"}}, + {Name: "list_code_quality_findings", Arguments: map[string]any{"owner": "owner", "repo": "repo", "perPage": "30"}}, {Name: "get_code_scanning_alert", Arguments: map[string]any{"owner": "owner", "repo": "repo", "alertNumber": "42"}}, {Name: "list_code_scanning_alerts", Arguments: map[string]any{"owner": "owner", "repo": "repo", "page": "1", "perPage": "30"}}, {Name: "get_secret_scanning_alert", Arguments: map[string]any{"owner": "owner", "repo": "repo", "alertNumber": "43"}}, @@ -251,7 +261,7 @@ func TestTypedSecurityToolOutputs(t *testing.T) { require.NotNil(t, result.StructuredContent, "%s must return structured content", call.Name) assert.Equal(t, legacyIFC[call.Name], mustMarshalJSON(t, result.Meta["ifc"]), "IFC labels must protect text and structured content identically") - if call.Name != "get_code_quality_finding" { + if call.Name != "get_code_quality_finding" && call.Name != "list_code_quality_findings" { assert.NotNil(t, result.Meta["ifc"], "%s must retain its IFC label", call.Name) } schemaJSON, err := json.Marshal(toolsByName[call.Name].OutputSchema) @@ -293,7 +303,7 @@ func TestTypedSecurityToolOutputs(t *testing.T) { assert.Contains(t, string(structuredJSON), "unsafeQuery") } - if call.Name == "get_code_quality_finding" { + if call.Name == "get_code_quality_finding" || call.Name == "list_code_quality_findings" { assert.NotContains(t, mustMarshalJSON(t, result.StructuredContent), "unknown_legacy_field") } @@ -382,6 +392,7 @@ func TestSecurityOutputDTOs(t *testing.T) { } for _, typ := range []reflect.Type{ reflect.TypeFor[CodeQualityFindingOutput](), + reflect.TypeFor[CodeQualityFindingsOutput](), reflect.TypeFor[CodeScanningAlertOutput](), reflect.TypeFor[SecretScanningAlertOutput](), reflect.TypeFor[DependabotAlertsOutput](),