-
Notifications
You must be signed in to change notification settings - Fork 53
MCP-248 Create Advanced Analysis Tool #175
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feature/ef/MCP-245-integrate-with-a3s
Are you sure you want to change the base?
MCP-248 Create Advanced Analysis Tool #175
Conversation
301878e to
8a6f907
Compare
src/main/java/org/sonarsource/sonarqube/mcp/configuration/McpServerLaunchConfiguration.java
Outdated
Show resolved
Hide resolved
| * Loads analysis tools based on configuration. | ||
| * When advanced analysis is enabled on SonarCloud, registers the advanced remote analysis tool. | ||
| * Otherwise, registers local analysis tools (IDE bridge tools or analyze_code_snippet). | ||
| */ | ||
| private void loadBackendDependentTools() { | ||
| private void loadAnalysisTools() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd keep the previous name loadBackendDependentTools, it's important because otherwise we would simply add these tools where others are declared.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My only concern is that this tool does not depend on backend and it would be very hard for someone to understand why we have this tool under a function named this way. I think a better option would having the check higher level and calling either loadBackendDependentTools or loadAdvancedAnalysis(making it a separate function) WDYT?
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| class AdvancedAnalysisEnablementTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need this test class
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was for coverage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to keep it I'd move it somewhere else, it's currently at the root of the test folder
8a6f907 to
2e99400
Compare
2e99400 to
2108f70
Compare
SonarQube reviewer guideImportant We are currently testing different models for AI Summary. Model A:Summary: Add advanced analysis mode for SonarCloud with feature flag to replace local analysis tools Review Focus: The conditional tool loading logic in Start review at: Model B:Summary: Add support for advanced code analysis mode on SonarCloud, controlled by the Review Focus:
Start review at:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces an advanced analysis tool for SonarCloud environments, allowing users to opt into a new analysis mode that replaces local analysis tools with a remote advanced analysis capability.
Changes:
- Added
RunAdvancedCodeAnalysisTool- a new tool that provides advanced code analysis functionality (currently returns "Hello, World" as a placeholder implementation) - Added configuration support via
SONARQUBE_ADVANCED_ANALYSIS_ENABLEDenvironment variable/system property - Implemented conditional tool loading logic that enables advanced analysis tools on SonarCloud while falling back to standard analysis on SonarQube Server
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/main/java/org/sonarsource/sonarqube/mcp/tools/analysis/RunAdvancedCodeAnalysisTool.java |
New tool implementation with placeholder "Hello, World" response |
src/main/java/org/sonarsource/sonarqube/mcp/configuration/McpServerLaunchConfiguration.java |
Added configuration field and getter for advanced analysis enablement flag |
src/main/java/org/sonarsource/sonarqube/mcp/SonarQubeMcpServer.java |
Modified tool loading logic to conditionally load advanced or standard analysis tools based on configuration |
src/test/java/org/sonarsource/sonarqube/mcp/tools/analysis/RunAdvancedCodeAnalysisToolTests.java |
Added unit tests for the new tool's basic properties and placeholder behavior |
src/test/java/org/sonarsource/sonarqube/mcp/configuration/McpServerLaunchConfigurationTest.java |
Added tests for parsing and handling the new configuration flag |
src/test/java/org/sonarsource/sonarqube/mcp/AdvancedAnalysisEnablementTest.java |
Added integration tests verifying tool registration behavior and fallback logic |
README.md |
Updated documentation to describe the new advanced analysis feature and configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| loadBackendDependentTools(); | ||
| } | ||
|
|
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The loadProxiedServerTools() call has been removed from the initialization flow, which means proxied MCP servers and their tools will never be loaded. This breaks functionality that depends on proxied tools. The call to loadProxiedServerTools() should be restored after the conditional tool loading logic.
| loadProxiedServerTools(); |
| private void loadAdvancedAnalysisTools() { | ||
| supportedTools.add(new RunAdvancedCodeAnalysisTool()); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure we need a method for a single line addition to a list
| | Toolset | Key | Description | | ||
| |--------------------|---------------------|----------------------------------------------------------------------| | ||
| | **Analysis** | `analysis` | Code analysis tools (analyze code snippets and files) | | ||
| | **Analysis** | `analysis` | Code analysis tools (local analysis and advanced analysis) | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should mention that it's remote
| super(SchemaToolBuilder.forOutput(RunAdvancedCodeAnalysisToolResponse.class) | ||
| .setName(TOOL_NAME) | ||
| .setTitle("Advanced Code Analysis") | ||
| .setDescription("Run advanced code analysis remotely using SonarCloud advanced analysis.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| .setDescription("Run advanced code analysis remotely using SonarCloud advanced analysis.") | |
| .setDescription("Run advanced code analysis remotely using SonarQube Cloud advanced analysis.") |



No description provided.