Skip to content

Conversation

@nquinquenel
Copy link
Member

No description provided.

@hashicorp-vault-sonar-prod
Copy link

hashicorp-vault-sonar-prod bot commented Feb 5, 2026

MCP-276

@nquinquenel
Copy link
Member Author

@copilot Can you mark as accepted the issues about "Refactor your code to get this URI from a customizable parameter." and provide a fix for the other issues

Copy link

Copilot AI commented Feb 6, 2026

@nquinquenel I've opened a new pull request, #195, to work on those changes. Once the pull request is ready, I'll request review from you.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 6, 2026

SonarQube reviewer guide

Review in SonarQube

Summary: Add test coverage analysis toolset with two new tools for searching files by coverage and viewing detailed line-by-line coverage information.

Review Focus:

  • The coverage filtering logic in SearchFilesByCoverageTool needs careful review—verify the min/max coverage threshold application is correct
  • The HTML tag stripping in GetFileCoverageDetailsTool.stripHtmlTags() is a regex operation that should handle edge cases properly
  • API response parsing in the new ComponentTreeResponse and SourceLinesResponse classes—ensure null-safety and metric value parsing handle SonarQube's response variations
  • Tool registration in SonarQubeMcpServer.loadBackendIndependentTools() and the new COVERAGE tool category enum

Start review at: src/main/java/org/sonarsource/sonarqube/mcp/tools/measures/SearchFilesByCoverageTool.java. This is the main tool implementation that orchestrates multiple API calls and applies filtering logic—it's critical for correct functionality and has the most complex business logic of the changeset.

💬 Please send your feedback

Quality Gate Passed Quality Gate passed

Issues
0 New issues
2 Accepted issues
0 Dependency risks

Measures
0 Security Hotspots
87.2% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@nquinquenel nquinquenel marked this pull request as ready for review February 9, 2026 16:04
Copy link
Contributor

@eray-felek-sonarsource eray-felek-sonarsource left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We seem to be doing a lot of complex computation here and relying on the Internal API. Did we contact any team regarding this feature, I think it would be best if they had an actual API that is not Internal and is simple for us to use for this use case. Also not sure about the benefit of searching a range for coverage, I think we can just return ordered from least to most covered files.

}

// Set default page values
var actualPageIndex = (pageIndex != null && pageIndex > 0) ? pageIndex : 1;
Copy link
Contributor

@eray-felek-sonarsource eray-felek-sonarsource Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since page index is optional there is a good chance the LLM will not provide it most of the time and we might actually have a coverage in other pages that we don't see that fits our range

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new “Coverage” toolset to the SonarQube MCP Server, exposing project/file coverage search and line-by-line file coverage details via new tools and supporting server API endpoints.

Changes:

  • Introduces ToolCategory.COVERAGE and documents it in the README.
  • Adds two new MCP tools: search_files_by_coverage and get_file_coverage_details, including schema/response models.
  • Extends server API clients with /api/measures/component_tree and /api/sources/lines response models and request methods, plus tests.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
src/test/java/org/sonarsource/sonarqube/mcp/tools/sources/GetFileCoverageDetailsToolTests.java Adds tests validating coverage-details tool schema and behavior.
src/test/java/org/sonarsource/sonarqube/mcp/tools/measures/SearchFilesByCoverageToolTests.java Adds tests validating coverage-search tool schema and behavior.
src/test/java/org/sonarsource/sonarqube/mcp/tools/ToolCategoryTest.java Updates category assertions to include the new coverage toolset.
src/main/java/org/sonarsource/sonarqube/mcp/tools/sources/GetFileCoverageDetailsToolResponse.java Defines the structured response for file coverage details.
src/main/java/org/sonarsource/sonarqube/mcp/tools/sources/GetFileCoverageDetailsTool.java Implements the line-by-line coverage details tool over /api/sources/lines.
src/main/java/org/sonarsource/sonarqube/mcp/tools/measures/SearchFilesByCoverageToolResponse.java Defines the structured response for searching files by coverage.
src/main/java/org/sonarsource/sonarqube/mcp/tools/measures/SearchFilesByCoverageTool.java Implements coverage-based file search using /api/measures/component_tree.
src/main/java/org/sonarsource/sonarqube/mcp/tools/ToolCategory.java Adds COVERAGE("coverage") to the tool category enum.
src/main/java/org/sonarsource/sonarqube/mcp/serverapi/sources/response/SourceLinesResponse.java Adds model for /api/sources/lines response payload.
src/main/java/org/sonarsource/sonarqube/mcp/serverapi/sources/SourcesApi.java Adds /api/sources/lines request method.
src/main/java/org/sonarsource/sonarqube/mcp/serverapi/measures/response/ComponentTreeResponse.java Adds model for measures component tree response.
src/main/java/org/sonarsource/sonarqube/mcp/serverapi/measures/MeasuresApi.java Adds /api/measures/component_tree request method.
src/main/java/org/sonarsource/sonarqube/mcp/serverapi/measures/ComponentTreeParams.java Adds request parameter record for component tree query.
src/main/java/org/sonarsource/sonarqube/mcp/SonarQubeMcpServer.java Registers the new coverage tools with the server.
README.md Documents the new “Coverage” toolset and its tools/parameters.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

public static final String FROM_PROPERTY = "from";
public static final String TO_PROPERTY = "to";

private static final Pattern HTML_TAG_PATTERN = Pattern.compile("<[^>]*>");
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current HTML stripping regex will remove any angle-bracket content, which can corrupt legitimate Java code like generics (List<String>) or comparisons containing <...>. Consider stripping only the known syntax-highlighting tags (e.g., matching only <span ...> and </span>) rather than any <...> sequence.

Suggested change
private static final Pattern HTML_TAG_PATTERN = Pattern.compile("<[^>]*>");
private static final Pattern HTML_TAG_PATTERN = Pattern.compile("</?span\\b[^>]*>");

Copilot uses AI. Check for mistakes.
Comment on lines +162 to +164
static String stripHtmlTags(String html) {
return HTML_TAG_PATTERN.matcher(html).replaceAll("");
}
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current HTML stripping regex will remove any angle-bracket content, which can corrupt legitimate Java code like generics (List<String>) or comparisons containing <...>. Consider stripping only the known syntax-highlighting tags (e.g., matching only <span ...> and </span>) rather than any <...> sequence.

Copilot uses AI. Check for mistakes.
Comment on lines +51 to +52
.addNumberProperty(FROM_PROPERTY, "First line to analyze (1-based, default: 1)")
.addNumberProperty(TO_PROPERTY, "Last line to analyze (inclusive). If not specified, all lines are returned")
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The schema declares from/to as “Number”, but the implementation reads them as integers. This can create client/server mismatches (e.g., a client sending 10.0 or any non-integer JSON number). Prefer using an integer-typed schema property (if available in SchemaToolBuilder) or parse as a numeric type and validate/coerce to int before calling the API.

Suggested change
.addNumberProperty(FROM_PROPERTY, "First line to analyze (1-based, default: 1)")
.addNumberProperty(TO_PROPERTY, "Last line to analyze (inclusive). If not specified, all lines are returned")
.addIntegerProperty(FROM_PROPERTY, "First line to analyze (1-based, default: 1)")
.addIntegerProperty(TO_PROPERTY, "Last line to analyze (inclusive). If not specified, all lines are returned")

Copilot uses AI. Check for mistakes.
Comment on lines +64 to +65
var from = arguments.getOptionalInteger(FROM_PROPERTY);
var to = arguments.getOptionalInteger(TO_PROPERTY);
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The schema declares from/to as “Number”, but the implementation reads them as integers. This can create client/server mismatches (e.g., a client sending 10.0 or any non-integer JSON number). Prefer using an integer-typed schema property (if available in SchemaToolBuilder) or parse as a numeric type and validate/coerce to int before calling the API.

Copilot uses AI. Check for mistakes.
Comment on lines +59 to +60
.addNumberProperty(PAGE_INDEX_PROPERTY, "Page index (1-based, default: 1)")
.addNumberProperty(PAGE_SIZE_PROPERTY, "Page size (default: 100, max: 500)")
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These parameters are exposed as “Number” in the tool schema but are parsed as integers in the implementation. To avoid client interoperability issues, align the schema and parsing: declare these as integer properties (preferred) or parse numeric values as Double/Number then validate and convert to int.

Suggested change
.addNumberProperty(PAGE_INDEX_PROPERTY, "Page index (1-based, default: 1)")
.addNumberProperty(PAGE_SIZE_PROPERTY, "Page size (default: 100, max: 500)")
.addIntegerProperty(PAGE_INDEX_PROPERTY, "Page index (1-based, default: 1)")
.addIntegerProperty(PAGE_SIZE_PROPERTY, "Page size (default: 100, max: 500)")

Copilot uses AI. Check for mistakes.
Comment on lines +72 to +76
var minCoverage = arguments.getOptionalInteger(MIN_COVERAGE_PROPERTY);
var maxCoverage = arguments.getOptionalInteger(MAX_COVERAGE_PROPERTY);
var pageIndex = arguments.getOptionalInteger(PAGE_INDEX_PROPERTY);
var pageSize = arguments.getOptionalInteger(PAGE_SIZE_PROPERTY);

Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These parameters are exposed as “Number” in the tool schema but are parsed as integers in the implementation. To avoid client interoperability issues, align the schema and parsing: declare these as integer properties (preferred) or parse numeric values as Double/Number then validate and convert to int.

Suggested change
var minCoverage = arguments.getOptionalInteger(MIN_COVERAGE_PROPERTY);
var maxCoverage = arguments.getOptionalInteger(MAX_COVERAGE_PROPERTY);
var pageIndex = arguments.getOptionalInteger(PAGE_INDEX_PROPERTY);
var pageSize = arguments.getOptionalInteger(PAGE_SIZE_PROPERTY);
Number minCoverageNumber = arguments.getOptionalNumber(MIN_COVERAGE_PROPERTY);
Integer minCoverage = null;
if (minCoverageNumber != null) {
if (minCoverageNumber.doubleValue() % 1 != 0) {
return Tool.Result.failure("minCoverage must be an integer value");
}
minCoverage = minCoverageNumber.intValue();
}
Number maxCoverageNumber = arguments.getOptionalNumber(MAX_COVERAGE_PROPERTY);
Integer maxCoverage = null;
if (maxCoverageNumber != null) {
if (maxCoverageNumber.doubleValue() % 1 != 0) {
return Tool.Result.failure("maxCoverage must be an integer value");
}
maxCoverage = maxCoverageNumber.intValue();
}
Number pageIndexNumber = arguments.getOptionalNumber(PAGE_INDEX_PROPERTY);
Integer pageIndex = null;
if (pageIndexNumber != null) {
if (pageIndexNumber.doubleValue() % 1 != 0) {
return Tool.Result.failure("pageIndex must be an integer value");
}
pageIndex = pageIndexNumber.intValue();
}
Number pageSizeNumber = arguments.getOptionalNumber(PAGE_SIZE_PROPERTY);
Integer pageSize = null;
if (pageSizeNumber != null) {
if (pageSizeNumber.doubleValue() % 1 != 0) {
return Tool.Result.failure("pageSize must be an integer value");
}
pageSize = pageSizeNumber.intValue();
}

Copilot uses AI. Check for mistakes.
) {

public record CoverageSummary(
@JsonPropertyDescription("Total number of lines in the file") int totalLines,
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

totalLines is described as “Total number of lines in the file”, but GetFileCoverageDetailsTool computes it as sources.size(), which becomes “number of lines returned” (especially when from/to are used). Update the description to match the actual semantics (e.g., “Total number of lines returned in this response/range”) or change computation to reflect actual file total lines.

Suggested change
@JsonPropertyDescription("Total number of lines in the file") int totalLines,
@JsonPropertyDescription("Total number of lines returned in this response or range") int totalLines,

Copilot uses AI. Check for mistakes.

import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonPropertyDescription;
import jakarta.annotation.Nullable;
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file uses jakarta.annotation.Nullable, while several other newly added/updated classes use javax.annotation.Nullable (e.g., GetFileCoverageDetailsTool, SourceLinesResponse, ComponentTreeResponse, ComponentTreeParams). Mixing jakarta and javax nullability annotations in the same module commonly causes dependency/classpath friction; standardize on one annotation package across the project.

Suggested change
import jakarta.annotation.Nullable;
import javax.annotation.Nullable;

Copilot uses AI. Check for mistakes.
Comment on lines +45 to +50
return conditions != null && coveredConditions != null
&& coveredConditions > 0 && coveredConditions < conditions;
}

public boolean hasNoBranchCoverage() {
return conditions != null && (coveredConditions == null || coveredConditions == 0);
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hasNoBranchCoverage() treats conditions = 0 as “no branch coverage”, which can incorrectly classify lines with no branches as uncovered for branch coverage and add them to partiallyConditionalLines. Consider requiring conditions > 0 (and similarly guarding other branch-coverage helpers) so only lines with actual branches are included.

Suggested change
return conditions != null && coveredConditions != null
&& coveredConditions > 0 && coveredConditions < conditions;
}
public boolean hasNoBranchCoverage() {
return conditions != null && (coveredConditions == null || coveredConditions == 0);
return conditions != null && conditions > 0 && coveredConditions != null
&& coveredConditions > 0 && coveredConditions < conditions;
}
public boolean hasNoBranchCoverage() {
return conditions != null && conditions > 0 && (coveredConditions == null || coveredConditions == 0);

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants