Enhancement: Analyzer MCP Server#259
Conversation
Signed-off-by: JonahSussman <sussmanjonah@gmail.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 54 minutes and 46 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as MCP Client
participant Server as analyzer-mcp
participant Engine as kai-analyzer
participant Cache as Results Cache
Client->>Server: MCP tool request (e.g., analyze_run)
Server->>Cache: lookup cached result
alt cached hit
Cache-->>Server: cached result
Server-->>Client: return cached result
else cache miss
Server->>Engine: forward analysis request
Engine-->>Server: analysis results
Server->>Cache: store results
Server-->>Client: return results
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@enhancements/analyzer-mcp/README.md`:
- Line 75: Replace the redundant phrase "CLI interface" in the README (the line
that currently reads "Replace the existing CLI interface (MCP is
complementary)") with either "CLI" or "command-line interface" (e.g., "Replace
the existing CLI (MCP is complementary)" or "Replace the existing command-line
interface (MCP is complementary)") so the wording is not repetitive.
- Line 132: The fenced code block in the README.md containing the project tree
starting with "cmd/analyzer/mcp/" (showing entries like main.go and server.go)
is missing a language identifier; update the opening triple-backticks to include
a language (e.g., ```text) so the block renders and syntax-highlights correctly
in Markdown.
🧹 Nitpick comments (1)
enhancements/analyzer-mcp/README.md (1)
171-173: Consider more specific path traversal mitigation details.While the mitigation mentions validating paths and rejecting suspicious patterns, consider documenting more specific validation strategies for the implementation phase, such as:
- Canonicalizing paths and verifying they remain within allowed directories
- Rejecting paths containing
..components- Using allowlist-based validation for accessible directories
- Employing filepath.Clean() or equivalent in the actual implementation
This would provide clearer security guidance for implementers.
|
|
||
| ## Proposal | ||
|
|
||
| Implement an MCP server as a new command (`cmd/analyzer/mcp/`) that wraps existing analyzer-lsp functionality and exposes it through the Model Context Protocol. The server will support two transports: stdio for local usage and HTTP for remote access. |
There was a problem hiding this comment.
Would it make sense to create this in a new repo, as another integrator of analyzer-lsp?
Is there a need for this to be in the tree?
| **Key Design Decisions:** | ||
|
|
||
| 1. **Reuse Existing Code**: All tools delegate to existing analyzer-lsp libraries (`parser`, `engine`, `provider`, etc.) rather than reimplementing logic | ||
| 2. **Stateless Operation**: Each tool invocation is independent; no session state is maintained |
There was a problem hiding this comment.
How is this done when running providers?
There was a problem hiding this comment.
@shawn-hurley Good question about stateless operation with providers. I don't think true statelessness is viable here.
The Performance Problem:
- Provider
Init()+Prepare()phase takes 10s-5min depending on codebase size (symbol cache population) - If each MCP tool invocation is truly independent, every query would re-run this = 5min wait for AI assistants = terrible UX
- The
Prepare()phase is specifically designed to pre-populate caches for subsequent queries
What I Think We Need:
Stateful session + Stateless tools
│
├─ Session: Long-running analyzer-lsp process (stays alive between queries)
│ - Init() once on first query
│ - Prepare() once, cache persists
│ - File watching + incremental updates via NotifyFileChanges()
│
└─ Tools: Each MCP call is independent (get_violations, get_dependencies, etc.)
- Query against cached state
- No side effects between tool calls
- Idempotent operations
Proposal:
- Clarify in the enhancement doc: "MCP server maintains a stateful session per workspace, but individual tool invocations are stateless operations against that session"
- Add session lifecycle management (start on first query, configurable timeout, explicit cleanup)
- Design for eventual multi-workspace support (one session per workspace root)
Questions:
- Does this stateful-session model align with your concerns?
- Should we add explicit session management tools (
start_session,get_session_status,end_session)? - How do we handle session cleanup? (Timeout after N minutes idle? Explicit user command?)
Related to your infrastructure question - this definitely requires process management infrastructure.
There was a problem hiding this comment.
I think it starts to look very much like kai_analyzer_rpc. The interaction between VSCode and analyzer-lsp is pretty "stateless," aside from the "hey, this updated, you should re-look" message.
That is an important bit, though. We need to know when the files were updated so we can re-gen the cache for those files.
So I don't know if the session is the right layer to understand the state at, if that makes sense?
We have some cache invalidation in kai_analyzer_rpc that invalidates the cache when you request a specific file. I don't know if this would be desirable from a performance standpoint for this use case.
I think that this idea is valuable, and obviously necessary for where I think we should be in terms of agentic workflows and such. I just think there are many more design questions here that we should at least capture.
I guess we could do an MVP of an option and see where we get, but we do have some prior art here, and I think we should probably take the learnings from that.
There was a problem hiding this comment.
@shawn-hurley I agree with Shawn here. There are many design details to talk out here. But here's something I have been thinking about "sessions". The only distinction between a MCP server and an analysis skill that I see is being able to be stateful. I 100% agree that this MCP server is kai_analyzer_rpc more than it is analyzer_rpc and as a result, I see it being implemented there. However, It wouldn't be ideal to compare IDE <-> kai_analyzer_rpc because there's a strict 1:1 relationship between kai_analyzer_rpc instance and an IDE instance. That is not true for an MCP server. Imagine an MCP server having potentially more than one workspaces initiated. The core of it still looks like kai_analyzer_rpc, while the session layer differentiates the workspaces.
|
|
||
| **Development:** | ||
| - No new infrastructure required | ||
| - Existing analyzer-lsp repository and CI pipeline sufficient |
There was a problem hiding this comment.
I don't believe that this is the case
There was a problem hiding this comment.
Thinking through the performance/UX requirements, here's what we'd actually need:
Phase 1 (MVP) - Minimal Infrastructure:
- ✅ Long-running process management (not batch execution like current CLI)
- ✅ GRPC client libraries in MCP server (TypeScript/Python)
- ✅ MCP protocol implementation (stdio/HTTP transports)
- ✅ Basic session state management
Phase 2 (Production) - Significant Infrastructure:
- ✅ Caching layer for violation results (in-memory or Redis for multi-process)
- ✅ File watching system (chokidar, watchdog, or similar)
- ✅ Cache invalidation logic when files change
- ✅ Background re-analysis orchestration
- ✅ Indexes for fast querying (by file, rule, category, severity)
Phase 3 (Advanced) - Optional:
⚠️ Possible embedding/vector store for semantic search⚠️ Multi-workspace coordination if shared server
Why This Infrastructure Matters:
Without caching (Phase 2), every MCP query would be:
AI Assistant: "Show me violations in Main.java"
MCP Server: "Running full analysis... (5 minutes later)... here are results"
AI Assistant: "Now show me violations in Controller.java"
MCP Server: "Running full analysis again... (5 minutes later)... here are results"
This is unusable. We need:
AI Assistant: "Show me violations in Main.java"
MCP Server: "Checking cache... found 12 violations (200ms)"
AI Assistant: "Now show me violations in Controller.java"
MCP Server: "Checking cache... found 8 violations (150ms)"
Recommendation: Update the proposal to:
- Phase 1: "Minimal infrastructure - long-running process, basic GRPC wrapper"
- Phase 2: "Production infrastructure - caching, file watching, incremental updates"
- Acknowledge this is more than a thin wrapper, but infrastructure is scoped and manageable
Questions:
- Does this align with your infrastructure concerns?
- Should Phase 1 include basic caching, or can we defer to Phase 2?
- What's the expected deployment model? (One process per workspace? Shared service? Embedded in IDE?)
There was a problem hiding this comment.
@tsanders-rh Most of the points here such as caching, process management, grpc are already implemented in kai_analyzer_rpc. The only thing we need is a file watcher (which we currently rely on vscode for) and session management for multiple workspace management. This makes my point in the previous comment even more relevant I think. This MCP server belongs more in kai_analyzer_rpc rather than analyzer-lsp
Strategic Opportunity: MCP as Platform PlayI want to raise a strategic framing that's missing from this proposal. Currently it's positioned as "add MCP support to analyzer," but there's a much bigger opportunity here. The Platform VisionCurrent Framing: "Analyzer gets another output format (MCP in addition to GRPC)" Alternative Framing: "Konveyor becomes the migration context platform that powers AI development tools" What This Means: Think of it like:
Why This MattersNetwork Effects:
Sustainable Advantage:
What This Changes About the ProposalSuccess Metrics Should Include:
Positioning Should Emphasize:
Questions for the Team
I think there's real strategic value here beyond just "another integration." Happy to discuss further. |
Technical Gap: Source/Target Migration ContextOne technical issue I noticed - the proposal doesn't address how source/target migration context works with MCP. The Problem: Currently, source/target is configured via analyzer-lsp --label-selector "konveyor.io/source=eap6" \
--label-selector "konveyor.io/target=eap8"From the codebase:
Why This Matters for MCP: AI assistants will want to know:
Example Scenario: Potential Solutions: Option 1: Store in MCP server config
Option 2: Infer from violations
Option 3: Add GRPC API upstream (longer-term)
Option 4: Document limitation
|
I would like to consider a world where the user never queries this information directly as a primary use case. I think we should focus on enabling agents to reliably use this information to generate better code. I personally would like to have some way to see that in an automated test early. I feel like that should be a big lesson learned from the initial kai work: we didn't have a good way to know the impact of any given change to the query. I could easily see a similar issue arising here.
Of note on this section, these labels are purely from the rulesets, and are not stored anywhere else (well, I guess in the hub database, but I don't think that is relevant here). I think any solution to this problem requires a way to manage rulesets for the MCP server. I think that we should probably break that down into two categories:
It is worth noting that you DO NOT need a source/target or label-selector to run rules from rulesets. If you do not specify one of these options in the analysis-option, we run all rules that have accessible providers for the capabilities in the rules. I would argue that we should start any solution to this problem with a user experience centered on the ruleset, and allow the label-selector to filter a large set of rules.
I agree with this framing and think it is an excellent goal. As I alluded to in another comment, this is exactly the way we need to go IMO. Should we consider alternatives to an MCP server, though? I think that we should consider: I am sure there are others that folks have heard of or seen that I am not aware of, might be nice to at least write down why an MCP server vs other options in the enhancement. I personally had a hard time seeing how we could make an MCP truly useful in an agentic system when I tried to build one awhile ago. The protocol itself doesn't seem to have changed drastically, so I think it may still be the case. You just don't have enough information for the system to understand what it should do with the data. It also seems that there is a lot of talk, rightly or wrongly, about MCP servers having a real scale problem. I don't know how real that is, but it's something we should at least consider. Sorry for the wall of text. I do wonder if this is a good topic for the next community meeting? |
|
This pull request has been automatically marked as stale because it has not had any activity for 60 days. |
| **Future Version Compatibility:** | ||
| - Tool input schemas will be versioned in descriptions | ||
| - Breaking changes will result in new tool names (e.g., `analyze_run_v2`) | ||
| - Deprecated tools will remain available for 2+ releases |
There was a problem hiding this comment.
do we have a Konveyor wide policy regarding the number of releases we support? if so, we should follow that. If not, then should discuss about it
JonahSussman
changed the title
Signed-off-by: JonahSussman <sussmanjonah@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
enhancements/analyzer-mcp/README.md (1)
77-77:⚠️ Potential issue | 🟡 MinorWording nit already reported: “CLI interface” is redundant.
Line 77 repeats the acronym expansion (“CLI interface”). This is the same issue previously flagged.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@enhancements/analyzer-mcp/README.md` at line 77, Edit the README line that currently reads "kantra CLI interface (MCP is complementary)" to remove the redundant word "interface" — e.g., change it to "kantra CLI (MCP is complementary)" or "kantra command-line interface (MCP is complementary)"; locate the phrase "kantra CLI interface (MCP is complementary)" in the README and update it accordingly.
🧹 Nitpick comments (1)
enhancements/analyzer-mcp/README.md (1)
235-247: Add protocol-alternative rationale (beyond MCP transport choices).The alternatives section compares implementation placements/transports, but not protocol choices. Given the discussion context, add at least one non-MCP protocol alternative and a short decision matrix (interop, ecosystem adoption, complexity, scaling implications).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@enhancements/analyzer-mcp/README.md` around lines 235 - 247, Update the Alternatives section (near "Alternative 1: MCP server inside analyzer-lsp" and "Alternative 2: Stdio-Only (No HTTP)") to add at least one non‑MCP protocol alternative (for example gRPC or WebSocket) describing pros/cons, and append a concise decision matrix comparing MCP vs the new protocol vs stdio across interop, ecosystem adoption, implementation complexity, and scaling implications; keep entries short (one line each) so reviewers can quickly see the rationale.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@enhancements/analyzer-mcp/README.md`:
- Line 120: Update the two fenced code blocks that begin with "```" (the block
that starts with "MCP Client (Claude Code, VS Code, CI/CD)" and the block that
starts with "analyzer-mcp/") to include a language identifier (for example
change "```" to "```text") so markdown linting passes; edit the README.md fenced
openings at those two locations accordingly.
- Around line 52-53: The README's description of get_migration_context is
underspecified and can lead to nondeterministic agent behavior; update the docs
to explicitly define how an active migration context or ruleset is set and
surfaced at runtime (e.g., a runtime API/parameter, priority/override rules, and
how rules are selected), add a clear function signature or pseudocode for
get_migration_context that shows required inputs (current project, explicit
ruleset id, fallback behavior), and describe how notify_file_changes interacts
with an explicitly set context for incremental analysis; also add a short
deterministic example and test guidance (reproducible inputs → expected context)
and apply the same clarity to the corresponding section referenced around lines
169–177.
---
Duplicate comments:
In `@enhancements/analyzer-mcp/README.md`:
- Line 77: Edit the README line that currently reads "kantra CLI interface (MCP
is complementary)" to remove the redundant word "interface" — e.g., change it to
"kantra CLI (MCP is complementary)" or "kantra command-line interface (MCP is
complementary)"; locate the phrase "kantra CLI interface (MCP is complementary)"
in the README and update it accordingly.
---
Nitpick comments:
In `@enhancements/analyzer-mcp/README.md`:
- Around line 235-247: Update the Alternatives section (near "Alternative 1: MCP
server inside analyzer-lsp" and "Alternative 2: Stdio-Only (No HTTP)") to add at
least one non‑MCP protocol alternative (for example gRPC or WebSocket)
describing pros/cons, and append a concise decision matrix comparing MCP vs the
new protocol vs stdio across interop, ecosystem adoption, implementation
complexity, and scaling implications; keep entries short (one line each) so
reviewers can quickly see the rationale.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0a8efe28-25c9-486f-ac18-ec07a4171ef9
📒 Files selected for processing (1)
enhancements/analyzer-mcp/README.md
| - **get_migration_context**: Infer migration sources and targets from rules | ||
| - **notify_file_changes**: Notify providers of file changes for incremental analysis |
There was a problem hiding this comment.
Migration context is underspecified for reliable agent behavior.
The design currently says get_migration_context infers source/target from rules, but it does not define how active migration context/rulesets are explicitly surfaced or set at runtime. That can make tool results nondeterministic for agents and hard to test.
Suggested doc update
- **get_migration_context**: Infer migration sources and targets from rules
+ **get_migration_context**: Return effective migration context for the current workspace/session, including:
+ - active source/target technology pair(s)
+ - active rulesets and label selectors
+ - whether values are explicit config vs inferred defaults
+ **set_migration_context** (optional for MVP): Set/override source/target + ruleset selection for current workspace/sessionAlso applies to: 169-177
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@enhancements/analyzer-mcp/README.md` around lines 52 - 53, The README's
description of get_migration_context is underspecified and can lead to
nondeterministic agent behavior; update the docs to explicitly define how an
active migration context or ruleset is set and surfaced at runtime (e.g., a
runtime API/parameter, priority/override rules, and how rules are selected), add
a clear function signature or pseudocode for get_migration_context that shows
required inputs (current project, explicit ruleset id, fallback behavior), and
describe how notify_file_changes interacts with an explicitly set context for
incremental analysis; also add a short deterministic example and test guidance
(reproducible inputs → expected context) and apply the same clarity to the
corresponding section referenced around lines 169–177.
| ### Implementation Details/Notes/Constraints [optional] | ||
|
|
||
| **Architecture:** | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks.
Both opening fences (Line 120 and Line 141) should include a language tag (for example, text) to satisfy markdown linting.
Proposed fix
-```
+```text
MCP Client (Claude Code, VS Code, CI/CD)
|
| MCP Protocol (stdio or HTTP)
|
analyzer-mcp binary
(MCP tool handlers)
|
| In-process Go import
|
kai-analyzer-rpc
service.Analyzer
(caching, providers, progress)
|
| Uses internally
|
analyzer-lsp
(engine, providers, parser, output types)@@
- +text
analyzer-mcp/
├── go.mod
├── go.sum
├── Makefile
├── Dockerfile
└── cmd/
└── analyzer-mcp/
├── main.go # CLI entry: cobra flags, transport selection
├── server.go # NewMCPServer(), Run(), bearerAuthMiddleware()
├── service.go # AnalyzerService interface + types
├── analyzer_service.go # kaiAnalyzerService: wraps kai-analyzer
├── tools.go # 9 MCP tool handler registrations
├── helpers.go # Shared utilities
├── main_test.go # CLI flag tests
├── tools_test.go # Unit tests with mockAnalyzerService
├── helpers_test.go # Helper function tests
├── service_test.go # kaiAnalyzerService unit tests
└── integration_test.go # Full workflow + HTTP + auth tests
Also applies to: 141-141
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 120-120: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@enhancements/analyzer-mcp/README.md` at line 120, Update the two fenced code
blocks that begin with "```" (the block that starts with "MCP Client (Claude
Code, VS Code, CI/CD)" and the block that starts with "analyzer-mcp/") to
include a language identifier (for example change "```" to "```text") so
markdown linting passes; edit the README.md fenced openings at those two
locations accordingly.
Signed-off-by: JonahSussman <sussmanjonah@gmail.com>
djzager
left a comment
djzager
left a comment
There was a problem hiding this comment.
I'm excited about this future, the only thing that really caught my eye was the analyze vs get_analysis_results.
| - `analyze`: Execute full analysis with configurable rules, label selectors, and scoping | ||
| - `get_analysis_results`: Retrieve cached results from the last analysis run |
There was a problem hiding this comment.
#267 (comment) is relevant here. I'm not sure if @pranavgaikwad was saying there should only be one endpoint or if the skills should only use one.
There was a problem hiding this comment.
Yes, right now, there is no use case for letting an agent run analysis, send file change events, etc. Its best to keep these things deterministic and outside of MCP server. If I understand correctly, right now, our primary use cases for the MCP server are - IDE and a standalone runner in the container. I think...in both these environments we only ever need the get_analysis_results function. Am I missing something?
| - `list_providers`: List available analysis providers and their capabilities | ||
| - `get_dependencies`: Extract dependency trees from analyzed projects | ||
| - `get_migration_context`: Infer migration sources and targets from rules | ||
| - `notify_file_changes`: Notify providers of file changes for incremental analysis |
There was a problem hiding this comment.
this looks too brittle to be exposed to an LLM. I think this should be handled elsewhere.
| - `analyze`: Execute full analysis with configurable rules, label selectors, and scoping | ||
| - `get_analysis_results`: Retrieve cached results from the last analysis run |
There was a problem hiding this comment.
Yes, right now, there is no use case for letting an agent run analysis, send file change events, etc. Its best to keep these things deterministic and outside of MCP server. If I understand correctly, right now, our primary use cases for the MCP server are - IDE and a standalone runner in the container. I think...in both these environments we only ever need the get_analysis_results function. Am I missing something?
|
This pull request has been automatically marked as stale because it has not had any activity for 60 days. |
7 participants
Summary
Enhancement proposal for a standalone MCP server that wraps kai-analyzer-rpc to expose Konveyor's analysis capabilities to AI agents and automation tools.
Implementation PRs
Summary by CodeRabbit