From b83cbd9b7c56c0e1d6a80da7542219c1e13e95e0 Mon Sep 17 00:00:00 2001 From: Tanzim Hossain Romel Date: Mon, 18 May 2026 00:08:02 +0600 Subject: [PATCH] Expose MCP AST diff summaries --- documentation/mcp.md | 28 ++- .../mcp/DiffBrowserLauncher.java | 8 +- .../mcp/McpAnalysisResult.java | 20 +- .../mcp/McpAstDiffResult.java | 197 ++++++++++++++++ .../mcp/McpDiffBrowserResult.java | 23 +- .../mcp/RefactoringMinerMcpService.java | 154 +++++++++++-- .../mcp/RefactoringMinerMcpTools.java | 114 ++++++++-- .../mcp/WebDiffBrowserLauncher.java | 5 +- .../mcp/RefactoringMinerMcpServiceTest.java | 214 +++++++++++++++++- .../mcp/RefactoringMinerMcpToolsTest.java | 26 ++- 10 files changed, 737 insertions(+), 52 deletions(-) create mode 100644 src/main/java/org/refactoringminer/mcp/McpAstDiffResult.java diff --git a/documentation/mcp.md b/documentation/mcp.md index 4974ec4466..03c272d9c4 100644 --- a/documentation/mcp.md +++ b/documentation/mcp.md @@ -166,18 +166,22 @@ Outside Docker, WebDiff binds to `127.0.0.1` by default. These settings can be o ## Analysis tools -Analysis tools report the refactorings RefactoringMiner detects. They return `status`, `summary`, `refactoringCount`, `astDiffCount`, `moveAstDiffCount`, `filesBefore`, `filesAfter`, a limited `refactorings` list, and `warnings`. +Analysis tools report the refactorings RefactoringMiner detects. They return `status`, `summary`, `refactoringCount`, `astDiffCount`, `moveAstDiffCount`, `filesBefore`, `filesAfter`, a limited `refactorings` list, bounded `astDiffs` summaries, and `warnings`. | Tool | Required inputs | Optional inputs | |------|-----------------|-----------------| -| `refactoringminer_analyze_file_contents` | `beforeFiles`, `afterFiles` | `maxFiles`, `maxBytesPerFile`, `maxRefactorings` | -| `refactoringminer_analyze_worktree` | None | `repositoryPath`, `baseRef`, `includeUntracked`, `maxFiles`, `maxBytesPerFile`, `maxRefactorings` | -| `refactoringminer_analyze_commit` | `commitId` | `repositoryPath`, `parentIndex`, `maxRefactorings` | -| `refactoringminer_analyze_pull_request` | `pullRequestId` | `cloneUrl`, `timeoutSeconds`, `maxRefactorings` | -| `refactoringminer_analyze_directories` | `beforePath`, `afterPath` | `maxRefactorings` | +| `refactoringminer_analyze_file_contents` | `beforeFiles`, `afterFiles` | `maxFiles`, `maxBytesPerFile`, `maxRefactorings`, `maxAstDiffs`, `maxActionsPerAstDiff` | +| `refactoringminer_analyze_worktree` | None | `repositoryPath`, `baseRef`, `includeUntracked`, `maxFiles`, `maxBytesPerFile`, `maxRefactorings`, `maxAstDiffs`, `maxActionsPerAstDiff` | +| `refactoringminer_analyze_commit` | `commitId` | `repositoryPath`, `parentIndex`, `maxRefactorings`, `maxAstDiffs`, `maxActionsPerAstDiff` | +| `refactoringminer_analyze_pull_request` | `pullRequestId` | `cloneUrl`, `timeoutSeconds`, `maxRefactorings`, `maxAstDiffs`, `maxActionsPerAstDiff` | +| `refactoringminer_analyze_directories` | `beforePath`, `afterPath` | `maxRefactorings`, `maxAstDiffs`, `maxActionsPerAstDiff` | Local paths must be absolute when provided. Worktree and commit tools default `repositoryPath` to the MCP server working directory, which is usually the directory where the agent was started. File-content maps use repository-relative paths as keys and file contents as values. Explicit file-content tools default to `maxFiles=100` and `maxBytesPerFile=200000` to keep calls small. Analysis tools default to `maxRefactorings=20`; set a higher value when the agent needs more returned detail, or `0` when counts and warnings are enough. +Analysis tools also return compact AST diff summaries by default. `maxAstDiffs` controls how many file-pair summaries are returned, and `maxActionsPerAstDiff` controls how many edit actions are sampled per summary. Set `maxAstDiffs=0` when an agent only needs refactoring counts and warnings. + +Each `astDiffs` entry includes `kind` (`standard` or `moved`), before/after file paths, mapping and action counts, per-action counts, and sampled action ranges. Sampled actions include the source-side `filePath`, a real `targetFilePath` when the action points to another file or the destination side, and `moveGroupId` for grouped multi-move actions. Missing node positions use `-1` for offsets and lines. This is intended for agent triage of moved, renamed, extracted, or inlined code without relying only on ordinary Git diff delete/add blocks. + Commit tools first try the GitHub API when the working tree has a GitHub `origin` remote. If the commit is not available from GitHub, they fall back to the local repository. This keeps pushed commits small for MCP clients while still supporting local-only commits. Pull-request tools also default `cloneUrl` from the MCP server working directory's GitHub `origin` remote. In a Docker config that mounts the repository at `/workspace` and starts the MCP process with `-w /workspace`, agents can usually pass only `pullRequestId`. Keep using an explicit `cloneUrl` when the MCP server is launched outside the target repository, or when the PR belongs to a different repository than the current working directory. @@ -231,14 +235,14 @@ Use `maxCandidates` to limit returned matches and candidates. If the result is t ## AST diff browser tools -Diff browser tools generate a RefactoringMiner AST diff, start the existing local WebDiff server, and return a localhost URL that the user can open in a browser. They return `status`, `summary`, `message`, `url`, `port`, `inputSummary`, `refactoringCount`, `astDiffCount`, `moveAstDiffCount`, `filesBefore`, `filesAfter`, a limited `affectedFiles` list, and `warnings`. +Diff browser tools generate a RefactoringMiner AST diff, start the existing local WebDiff server, and return a localhost URL that the user can open in a browser. They return `status`, `summary`, `message`, `url`, `port`, `inputSummary`, `refactoringCount`, `astDiffCount`, `moveAstDiffCount`, `filesBefore`, `filesAfter`, a limited `affectedFiles` list, bounded `astDiffs` summaries, and `warnings`. | Tool | Required inputs | Optional inputs | |------|-----------------|-----------------| -| `refactoringminer_diff_file_contents` | `beforeFiles`, `afterFiles` | `maxFiles`, `maxBytesPerFile`, `port` | -| `refactoringminer_diff_worktree` | None | `repositoryPath`, `baseRef`, `includeUntracked`, `maxFiles`, `maxBytesPerFile`, `port` | -| `refactoringminer_diff_commit` | `commitId` | `repositoryPath`, `parentIndex`, `port` | -| `refactoringminer_diff_pull_request` | `pullRequestId` | `cloneUrl`, `timeoutSeconds`, `port` | +| `refactoringminer_diff_file_contents` | `beforeFiles`, `afterFiles` | `maxFiles`, `maxBytesPerFile`, `port`, `maxAstDiffs`, `maxActionsPerAstDiff` | +| `refactoringminer_diff_worktree` | None | `repositoryPath`, `baseRef`, `includeUntracked`, `maxFiles`, `maxBytesPerFile`, `port`, `maxAstDiffs`, `maxActionsPerAstDiff` | +| `refactoringminer_diff_commit` | `commitId` | `repositoryPath`, `parentIndex`, `port`, `maxAstDiffs`, `maxActionsPerAstDiff` | +| `refactoringminer_diff_pull_request` | `pullRequestId` | `cloneUrl`, `timeoutSeconds`, `port`, `maxAstDiffs`, `maxActionsPerAstDiff` | The default port is `6789`. The returned `message` uses the same wording as the WebDiff CLI startup line: @@ -248,6 +252,8 @@ Starting server: http://127.0.0.1:6789 MCP tools do not auto-open the desktop browser. They bind WebDiff to `127.0.0.1` by default and return the URL in the tool result so the user or client can decide when to open it. +Diff browser tools return the same compact `astDiffs` summaries as analysis tools, with the same `maxAstDiffs` and `maxActionsPerAstDiff` controls. Set `maxAstDiffs=0` when the client only needs the WebDiff URL and counts. + The returned URL is only available while the MCP server process is still running. If an MCP client starts RefactoringMiner for a single command and immediately exits, the local WebDiff server can disappear before the user opens the URL. Use an interactive client session for browser tools, or use the direct WebDiff CLI when the goal is only manual browser inspection. Repeated browser-tool calls replace the active local WebDiff view in the MCP server process. The WebDiff Quit button stops the active local WebDiff view but does not exit the MCP JVM. If the requested port is invalid or already occupied by another process, the tool returns an `error` result with a summary and warnings. It does not corrupt stdio output or discard the previous view on another port. diff --git a/src/main/java/org/refactoringminer/mcp/DiffBrowserLauncher.java b/src/main/java/org/refactoringminer/mcp/DiffBrowserLauncher.java index ce4598ace3..496fd27ab8 100644 --- a/src/main/java/org/refactoringminer/mcp/DiffBrowserLauncher.java +++ b/src/main/java/org/refactoringminer/mcp/DiffBrowserLauncher.java @@ -6,5 +6,11 @@ @FunctionalInterface interface DiffBrowserLauncher { - McpDiffBrowserResult launch(ProjectASTDiff diff, int port, String inputSummary, List warnings) throws Exception; + McpDiffBrowserResult launch(ProjectASTDiff diff, int port, String inputSummary, List warnings, + int maxAstDiffs, int maxActionsPerAstDiff) throws Exception; + + default McpDiffBrowserResult launch(ProjectASTDiff diff, int port, String inputSummary, List warnings) + throws Exception { + return launch(diff, port, inputSummary, warnings, 20, 20); + } } diff --git a/src/main/java/org/refactoringminer/mcp/McpAnalysisResult.java b/src/main/java/org/refactoringminer/mcp/McpAnalysisResult.java index fb5b1b8b30..20fd1044fd 100644 --- a/src/main/java/org/refactoringminer/mcp/McpAnalysisResult.java +++ b/src/main/java/org/refactoringminer/mcp/McpAnalysisResult.java @@ -9,13 +9,23 @@ public record McpAnalysisResult(String status, String summary, int refactoringCount, int astDiffCount, int moveAstDiffCount, int filesBefore, int filesAfter, List refactorings, - List warnings) { + List astDiffs, List warnings) { public static McpAnalysisResult ok(ProjectASTDiff diff, int maxRefactorings) { - return ok(diff, maxRefactorings, Collections.emptyList()); + return ok(diff, maxRefactorings, 0, 0, Collections.emptyList()); + } + + public static McpAnalysisResult ok(ProjectASTDiff diff, int maxRefactorings, int maxAstDiffs, + int maxActionsPerAstDiff) { + return ok(diff, maxRefactorings, maxAstDiffs, maxActionsPerAstDiff, Collections.emptyList()); } public static McpAnalysisResult ok(ProjectASTDiff diff, int maxRefactorings, List additionalWarnings) { + return ok(diff, maxRefactorings, 0, 0, additionalWarnings); + } + + public static McpAnalysisResult ok(ProjectASTDiff diff, int maxRefactorings, int maxAstDiffs, + int maxActionsPerAstDiff, List additionalWarnings) { List sourceRefactorings = diff.getRefactorings() == null ? Collections.emptyList() : diff.getRefactorings(); int boundedSize = Math.min(sourceRefactorings.size(), maxRefactorings); @@ -30,6 +40,7 @@ public static McpAnalysisResult ok(ProjectASTDiff diff, int maxRefactorings, Lis warnings.add(String.format("Refactorings truncated to %d of %d.", boundedSize, sourceRefactorings.size())); } + List astDiffs = McpAstDiffResult.from(diff, maxAstDiffs, maxActionsPerAstDiff, warnings); int astDiffCount = diff.getDiffSet() == null ? 0 : diff.getDiffSet().size(); int moveAstDiffCount = diff.getMoveDiffSet() == null ? 0 : diff.getMoveDiffSet().size(); int filesBefore = diff.getFileContentsBefore() == null ? 0 : diff.getFileContentsBefore().size(); @@ -38,10 +49,11 @@ public static McpAnalysisResult ok(ProjectASTDiff diff, int maxRefactorings, Lis sourceRefactorings.size(), astDiffCount, moveAstDiffCount, filesBefore, filesAfter); return new McpAnalysisResult("ok", summary, sourceRefactorings.size(), astDiffCount, moveAstDiffCount, - filesBefore, filesAfter, boundedRefactorings, warnings); + filesBefore, filesAfter, boundedRefactorings, astDiffs, warnings); } public static McpAnalysisResult error(String summary, List warnings) { - return new McpAnalysisResult("error", summary, 0, 0, 0, 0, 0, Collections.emptyList(), warnings); + return new McpAnalysisResult("error", summary, 0, 0, 0, 0, 0, Collections.emptyList(), + Collections.emptyList(), warnings); } } diff --git a/src/main/java/org/refactoringminer/mcp/McpAstDiffResult.java b/src/main/java/org/refactoringminer/mcp/McpAstDiffResult.java new file mode 100644 index 0000000000..be4f11802b --- /dev/null +++ b/src/main/java/org/refactoringminer/mcp/McpAstDiffResult.java @@ -0,0 +1,197 @@ +package org.refactoringminer.mcp; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import com.github.gumtreediff.actions.model.Action; +import com.github.gumtreediff.actions.model.Delete; +import com.github.gumtreediff.actions.model.Insert; +import com.github.gumtreediff.actions.model.TreeAddition; +import com.github.gumtreediff.actions.model.TreeDelete; +import com.github.gumtreediff.actions.model.TreeInsert; +import com.github.gumtreediff.actions.model.Update; +import com.github.gumtreediff.tree.Tree; +import org.refactoringminer.astDiff.actions.model.MoveIn; +import org.refactoringminer.astDiff.actions.model.MoveOut; +import org.refactoringminer.astDiff.actions.model.MultiMove; +import org.refactoringminer.astDiff.models.ASTDiff; +import org.refactoringminer.astDiff.models.ProjectASTDiff; + +public record McpAstDiffResult(String kind, String beforeFilePath, String afterFilePath, int mappingCount, + int actionCount, Map actionCounts, List sampleActions) { + private static final String STANDARD = "standard"; + private static final String MOVED = "moved"; + + public McpAstDiffResult { + actionCounts = actionCounts == null ? Collections.emptyMap() + : Collections.unmodifiableMap(new LinkedHashMap<>(actionCounts)); + sampleActions = sampleActions == null ? Collections.emptyList() : List.copyOf(sampleActions); + } + + static List from(ProjectASTDiff diff, int maxAstDiffs, int maxActionsPerAstDiff, + List warnings) { + if (maxAstDiffs <= 0) { + return Collections.emptyList(); + } + List entries = entries(diff); + int boundedSize = Math.min(entries.size(), maxAstDiffs); + List results = new ArrayList<>(); + for (int i = 0; i < boundedSize; i++) { + DiffEntry entry = entries.get(i); + results.add(from(entry.diff(), entry.kind(), diff.getFileContentsBefore(), diff.getFileContentsAfter(), + maxActionsPerAstDiff, warnings)); + } + if (boundedSize < entries.size()) { + warnings.add(String.format("AST diffs truncated to %d of %d.", boundedSize, entries.size())); + } + return List.copyOf(results); + } + + private static McpAstDiffResult from(ASTDiff astDiff, String kind, Map beforeFiles, + Map afterFiles, int maxActionsPerAstDiff, List warnings) { + Map actionCounts = new LinkedHashMap<>(); + List sampleActions = new ArrayList<>(); + int actionCount = 0; + for (Action action : astDiff.editScript) { + actionCount++; + actionCounts.merge(action.getName(), 1, Integer::sum); + if (sampleActions.size() < maxActionsPerAstDiff) { + sampleActions.add(ActionSummary.from(action, astDiff, beforeFiles, afterFiles)); + } + } + if (maxActionsPerAstDiff > 0 && sampleActions.size() < actionCount) { + warnings.add(String.format("Actions for %s -> %s truncated to %d of %d.", astDiff.getSrcPath(), + astDiff.getDstPath(), sampleActions.size(), actionCount)); + } + int mappingCount = astDiff.getAllMappings() == null ? 0 : astDiff.getAllMappings().size(); + return new McpAstDiffResult(kind, astDiff.getSrcPath(), astDiff.getDstPath(), mappingCount, actionCount, + actionCounts, sampleActions); + } + + private static List entries(ProjectASTDiff diff) { + List entries = new ArrayList<>(); + Set moved = diff.getMoveDiffSet() == null ? Collections.emptySet() : diff.getMoveDiffSet(); + Set seen = new LinkedHashSet<>(); + if (diff.getDiffSet() != null) { + for (ASTDiff astDiff : diff.getDiffSet()) { + entries.add(new DiffEntry(astDiff, moved.contains(astDiff) ? MOVED : STANDARD)); + seen.add(astDiff); + } + } + for (ASTDiff astDiff : moved) { + if (seen.add(astDiff)) { + entries.add(new DiffEntry(astDiff, MOVED)); + } + } + return entries; + } + + private record DiffEntry(ASTDiff diff, String kind) { + } + + public record ActionSummary(String name, String side, String filePath, String targetFilePath, Integer moveGroupId, + String nodeType, String nodeLabel, int startOffset, int endOffset, int startLine, int endLine, + String newValue, Integer parentPosition, String parentType) { + private static ActionSummary from(Action action, ASTDiff astDiff, Map beforeFiles, + Map afterFiles) { + Tree node = action.getNode(); + String side = side(action); + String filePath = filePath(action, astDiff, side); + String content = "after".equals(side) ? get(afterFiles, filePath) : get(beforeFiles, filePath); + LineRange lineRange = LineRange.from(content, node); + String targetFilePath = targetFilePath(action, astDiff); + Integer moveGroupId = action instanceof MultiMove multiMove ? multiMove.getGroupId() : null; + String newValue = action instanceof Update update ? update.getValue() : null; + Integer parentPosition = action instanceof TreeAddition addition ? addition.getPosition() : null; + String parentType = action instanceof TreeAddition addition && addition.getParent() != null + ? addition.getParent().getType().toString() : null; + return new ActionSummary(action.getName(), side, filePath, targetFilePath, moveGroupId, type(node), + label(node), position(node), endPosition(node), lineRange.startLine(), lineRange.endLine(), + newValue, parentPosition, parentType); + } + + private static String side(Action action) { + if (action instanceof Insert || action instanceof TreeInsert || action instanceof MoveIn) { + return "after"; + } + if (action instanceof Delete || action instanceof TreeDelete || action instanceof MoveOut) { + return "before"; + } + return "before"; + } + + private static String filePath(Action action, ASTDiff astDiff, String side) { + if (action instanceof MoveIn) { + return astDiff.getDstPath(); + } + if (action instanceof MoveOut) { + return astDiff.getSrcPath(); + } + return "after".equals(side) ? astDiff.getDstPath() : astDiff.getSrcPath(); + } + + private static String targetFilePath(Action action, ASTDiff astDiff) { + if (action instanceof MoveIn moveIn) { + return moveIn.getSrcFile(); + } + if (action instanceof MoveOut moveOut) { + return moveOut.getDstFile(); + } + if (action instanceof MultiMove) { + return astDiff.getDstPath(); + } + return null; + } + + private static String get(Map files, String filePath) { + return files == null ? null : files.get(filePath); + } + + private static String type(Tree node) { + return node == null || node.getType() == null ? null : node.getType().toString(); + } + + private static String label(Tree node) { + if (node == null || !node.hasLabel()) { + return null; + } + String label = node.getLabel(); + return label == null || label.isBlank() ? null : label; + } + + private static int position(Tree node) { + return node == null ? -1 : node.getPos(); + } + + private static int endPosition(Tree node) { + return node == null ? -1 : node.getEndPos(); + } + } + + private record LineRange(int startLine, int endLine) { + private static LineRange from(String content, Tree node) { + if (content == null || node == null || node.getPos() < 0 || node.getEndPos() < 0) { + return new LineRange(-1, -1); + } + int startOffset = node.getPos(); + int endOffset = Math.max(startOffset, node.getEndPos() - 1); + return new LineRange(lineNumber(content, startOffset), lineNumber(content, endOffset)); + } + + private static int lineNumber(String content, int offset) { + int boundedOffset = Math.max(0, Math.min(offset, content.length())); + int line = 1; + for (int i = 0; i < boundedOffset; i++) { + if (content.charAt(i) == '\n') { + line++; + } + } + return line; + } + } +} diff --git a/src/main/java/org/refactoringminer/mcp/McpDiffBrowserResult.java b/src/main/java/org/refactoringminer/mcp/McpDiffBrowserResult.java index 293c9a0bc7..b2e88aaebe 100644 --- a/src/main/java/org/refactoringminer/mcp/McpDiffBrowserResult.java +++ b/src/main/java/org/refactoringminer/mcp/McpDiffBrowserResult.java @@ -14,14 +14,17 @@ @JsonInclude(JsonInclude.Include.NON_NULL) public record McpDiffBrowserResult(String status, String summary, String message, String url, Integer port, String inputSummary, int refactoringCount, int astDiffCount, int moveAstDiffCount, int filesBefore, - int filesAfter, List affectedFiles, List warnings) { + int filesAfter, List affectedFiles, List astDiffs, List warnings) { private static final int MAX_AFFECTED_FILES = 50; + private static final int DEFAULT_MAX_AST_DIFFS = 20; + private static final int DEFAULT_MAX_ACTIONS_PER_AST_DIFF = 20; public static final String OK = "ok"; public static final String ERROR = "error"; public McpDiffBrowserResult { affectedFiles = affectedFiles == null ? Collections.emptyList() : List.copyOf(affectedFiles); + astDiffs = astDiffs == null ? Collections.emptyList() : List.copyOf(astDiffs); warnings = warnings == null ? Collections.emptyList() : List.copyOf(warnings); } @@ -30,8 +33,20 @@ public static McpDiffBrowserResult ok(ProjectASTDiff diff, int port, String inpu return ok(diff, port, WebDiff.LOCAL_HOST, inputSummary, additionalWarnings); } + public static McpDiffBrowserResult ok(ProjectASTDiff diff, int port, String inputSummary, + List additionalWarnings, int maxAstDiffs, int maxActionsPerAstDiff) { + return ok(diff, port, WebDiff.LOCAL_HOST, inputSummary, additionalWarnings, maxAstDiffs, + maxActionsPerAstDiff); + } + public static McpDiffBrowserResult ok(ProjectASTDiff diff, int port, String publicHost, String inputSummary, List additionalWarnings) { + return ok(diff, port, publicHost, inputSummary, additionalWarnings, DEFAULT_MAX_AST_DIFFS, + DEFAULT_MAX_ACTIONS_PER_AST_DIFF); + } + + public static McpDiffBrowserResult ok(ProjectASTDiff diff, int port, String publicHost, String inputSummary, + List additionalWarnings, int maxAstDiffs, int maxActionsPerAstDiff) { List refactorings = diff.getRefactorings() == null ? Collections.emptyList() : diff.getRefactorings(); int astDiffCount = diff.getDiffSet() == null ? 0 : diff.getDiffSet().size(); int moveAstDiffCount = diff.getMoveDiffSet() == null ? 0 : diff.getMoveDiffSet().size(); @@ -43,17 +58,19 @@ public static McpDiffBrowserResult ok(ProjectASTDiff diff, int port, String publ warnings.addAll(additionalWarnings); } List affectedFiles = affectedFiles(diff, warnings); + List astDiffs = McpAstDiffResult.from(diff, maxAstDiffs, maxActionsPerAstDiff, warnings); String url = WebDiff.localUrl(publicHost, port); String summary = String.format("Started local AST diff browser with %d refactorings, %d AST diffs, %d moved AST diffs across %d before files and %d after files.", refactorings.size(), astDiffCount, moveAstDiffCount, filesBefore, filesAfter); return new McpDiffBrowserResult(OK, summary, WebDiff.startupMessage(publicHost, port), url, port, inputSummary, - refactorings.size(), astDiffCount, moveAstDiffCount, filesBefore, filesAfter, affectedFiles, warnings); + refactorings.size(), astDiffCount, moveAstDiffCount, filesBefore, filesAfter, affectedFiles, astDiffs, + warnings); } public static McpDiffBrowserResult error(String summary, Integer port, String inputSummary, List warnings) { return new McpDiffBrowserResult(ERROR, summary, null, null, port, inputSummary, 0, 0, 0, 0, 0, - Collections.emptyList(), warnings); + Collections.emptyList(), Collections.emptyList(), warnings); } private static List affectedFiles(ProjectASTDiff diff, List warnings) { diff --git a/src/main/java/org/refactoringminer/mcp/RefactoringMinerMcpService.java b/src/main/java/org/refactoringminer/mcp/RefactoringMinerMcpService.java index 022066d21a..5665d90b7d 100644 --- a/src/main/java/org/refactoringminer/mcp/RefactoringMinerMcpService.java +++ b/src/main/java/org/refactoringminer/mcp/RefactoringMinerMcpService.java @@ -18,6 +18,8 @@ public class RefactoringMinerMcpService { private static final int DEFAULT_MAX_FILES = 100; private static final int DEFAULT_MAX_BYTES_PER_FILE = 200_000; + private static final int DEFAULT_MAX_AST_DIFFS = 10; + private static final int DEFAULT_MAX_ACTIONS_PER_AST_DIFF = 10; private final FileContentDiffer differ; private final CommitDiffer commitDiffer; @@ -77,15 +79,24 @@ public RefactoringMinerMcpService() { public McpAnalysisResult analyzeFileContents(Map beforeFiles, Map afterFiles, int maxRefactorings) { return analyzeFileContents(beforeFiles, afterFiles, maxRefactorings, DEFAULT_MAX_FILES, - DEFAULT_MAX_BYTES_PER_FILE); + DEFAULT_MAX_BYTES_PER_FILE, 0, 0); } public McpAnalysisResult analyzeFileContents(Map beforeFiles, Map afterFiles, int maxRefactorings, int maxFiles, int maxBytesPerFile) { + return analyzeFileContents(beforeFiles, afterFiles, maxRefactorings, maxFiles, maxBytesPerFile, 0, 0); + } + + public McpAnalysisResult analyzeFileContents(Map beforeFiles, Map afterFiles, + int maxRefactorings, int maxFiles, int maxBytesPerFile, int maxAstDiffs, int maxActionsPerAstDiff) { if (maxRefactorings < 0) { return McpAnalysisResult.error("maxRefactorings must be greater than or equal to 0.", List.of("maxRefactorings=" + maxRefactorings)); } + McpAnalysisResult limitError = validateAstDiffLimits(maxAstDiffs, maxActionsPerAstDiff); + if (limitError != null) { + return limitError; + } if (beforeFiles == null || afterFiles == null) { return McpAnalysisResult.error("beforeFiles and afterFiles are required.", List.of("beforeFiles and afterFiles must be JSON objects mapping paths to file contents.")); @@ -102,7 +113,7 @@ public McpAnalysisResult analyzeFileContents(Map beforeFiles, Ma try { ProjectASTDiff diff = differ.diffAtFileContents(beforeFiles, afterFiles); - return toResult(diff, maxRefactorings, "File-content"); + return toResult(diff, maxRefactorings, maxAstDiffs, maxActionsPerAstDiff, "File-content"); } catch (Exception e) { return McpAnalysisResult.error("File-content analysis failed: " + e.getMessage(), List.of(e.getClass().getName())); @@ -253,10 +264,20 @@ public McpValidationResult validateDirectories(Path beforePath, Path afterPath, public McpAnalysisResult analyzeWorktree(Path repositoryPath, String baseRef, boolean includeUntracked, int maxFiles, int maxBytesPerFile, int maxRefactorings) { + return analyzeWorktree(repositoryPath, baseRef, includeUntracked, maxFiles, maxBytesPerFile, maxRefactorings, + 0, 0); + } + + public McpAnalysisResult analyzeWorktree(Path repositoryPath, String baseRef, boolean includeUntracked, int maxFiles, + int maxBytesPerFile, int maxRefactorings, int maxAstDiffs, int maxActionsPerAstDiff) { if (maxRefactorings < 0) { return McpAnalysisResult.error("maxRefactorings must be greater than or equal to 0.", List.of("maxRefactorings=" + maxRefactorings)); } + McpAnalysisResult limitError = validateAstDiffLimits(maxAstDiffs, maxActionsPerAstDiff); + if (limitError != null) { + return limitError; + } try { Path resolvedRepositoryPath = resolveRepositoryPath(repositoryPath); WorktreeChangeCollector.WorktreeChanges changes = new WorktreeChangeCollector() @@ -266,7 +287,7 @@ public McpAnalysisResult analyzeWorktree(Path repositoryPath, String baseRef, bo return McpAnalysisResult.error("Worktree analysis did not produce a diff.", List.of("RefactoringMiner returned null.")); } - return McpAnalysisResult.ok(diff, maxRefactorings, changes.warnings()); + return McpAnalysisResult.ok(diff, maxRefactorings, maxAstDiffs, maxActionsPerAstDiff, changes.warnings()); } catch (Exception e) { return McpAnalysisResult.error("Worktree analysis failed: " + e.getMessage(), List.of(e.getClass().getName())); } @@ -274,10 +295,19 @@ public McpAnalysisResult analyzeWorktree(Path repositoryPath, String baseRef, bo public McpAnalysisResult analyzeCommit(Path repositoryPath, String commitId, Integer parentIndex, int maxRefactorings) { + return analyzeCommit(repositoryPath, commitId, parentIndex, maxRefactorings, 0, 0); + } + + public McpAnalysisResult analyzeCommit(Path repositoryPath, String commitId, Integer parentIndex, + int maxRefactorings, int maxAstDiffs, int maxActionsPerAstDiff) { if (maxRefactorings < 0) { return McpAnalysisResult.error("maxRefactorings must be greater than or equal to 0.", List.of("maxRefactorings=" + maxRefactorings)); } + McpAnalysisResult limitError = validateAstDiffLimits(maxAstDiffs, maxActionsPerAstDiff); + if (limitError != null) { + return limitError; + } if (commitId == null || commitId.isBlank()) { return McpAnalysisResult.error("commitId must be a non-empty string.", List.of("commitId is required.")); } @@ -289,7 +319,7 @@ public McpAnalysisResult analyzeCommit(Path repositoryPath, String commitId, Int try { ProjectASTDiff diff = diffCommit(repositoryPath, commitId, resolvedParentIndex, 300); - return toResult(diff, maxRefactorings, "Commit"); + return toResult(diff, maxRefactorings, maxAstDiffs, maxActionsPerAstDiff, "Commit"); } catch (Exception e) { return McpAnalysisResult.error("Commit analysis failed: " + e.getMessage(), List.of(e.getClass().getName())); } @@ -297,10 +327,19 @@ public McpAnalysisResult analyzeCommit(Path repositoryPath, String commitId, Int public McpAnalysisResult analyzePullRequest(String cloneUrl, int pullRequestId, int timeoutSeconds, int maxRefactorings) { + return analyzePullRequest(cloneUrl, pullRequestId, timeoutSeconds, maxRefactorings, 0, 0); + } + + public McpAnalysisResult analyzePullRequest(String cloneUrl, int pullRequestId, int timeoutSeconds, + int maxRefactorings, int maxAstDiffs, int maxActionsPerAstDiff) { if (maxRefactorings < 0) { return McpAnalysisResult.error("maxRefactorings must be greater than or equal to 0.", List.of("maxRefactorings=" + maxRefactorings)); } + McpAnalysisResult limitError = validateAstDiffLimits(maxAstDiffs, maxActionsPerAstDiff); + if (limitError != null) { + return limitError; + } if (pullRequestId <= 0) { return McpAnalysisResult.error("pullRequestId must be greater than 0.", List.of("pullRequestId=" + pullRequestId)); @@ -319,7 +358,7 @@ public McpAnalysisResult analyzePullRequest(String cloneUrl, int pullRequestId, try { ProjectASTDiff diff = pullRequestDiffer.diffAtPullRequest(resolvedCloneUrl, pullRequestId, timeoutSeconds); - return toResult(diff, maxRefactorings, "Pull-request"); + return toResult(diff, maxRefactorings, maxAstDiffs, maxActionsPerAstDiff, "Pull-request"); } catch (Exception e) { return McpAnalysisResult.error("Pull-request analysis failed: " + e.getMessage(), List.of(e.getClass().getName())); @@ -327,16 +366,25 @@ public McpAnalysisResult analyzePullRequest(String cloneUrl, int pullRequestId, } public McpAnalysisResult analyzeDirectories(Path beforePath, Path afterPath, int maxRefactorings) { + return analyzeDirectories(beforePath, afterPath, maxRefactorings, 0, 0); + } + + public McpAnalysisResult analyzeDirectories(Path beforePath, Path afterPath, int maxRefactorings, + int maxAstDiffs, int maxActionsPerAstDiff) { if (maxRefactorings < 0) { return McpAnalysisResult.error("maxRefactorings must be greater than or equal to 0.", List.of("maxRefactorings=" + maxRefactorings)); } + McpAnalysisResult limitError = validateAstDiffLimits(maxAstDiffs, maxActionsPerAstDiff); + if (limitError != null) { + return limitError; + } try { requireExistingAbsolutePath(beforePath, "beforePath"); requireExistingAbsolutePath(afterPath, "afterPath"); ProjectASTDiff diff = directoryDiffer.diffAtDirectories(beforePath, afterPath); - return toResult(diff, maxRefactorings, "Directory"); + return toResult(diff, maxRefactorings, maxAstDiffs, maxActionsPerAstDiff, "Directory"); } catch (Exception e) { return McpAnalysisResult.error("Directory analysis failed: " + e.getMessage(), List.of(e.getClass().getName())); @@ -345,16 +393,28 @@ public McpAnalysisResult analyzeDirectories(Path beforePath, Path afterPath, int public McpDiffBrowserResult diffFileContents(Map beforeFiles, Map afterFiles, int port) { - return diffFileContents(beforeFiles, afterFiles, port, DEFAULT_MAX_FILES, DEFAULT_MAX_BYTES_PER_FILE); + return diffFileContents(beforeFiles, afterFiles, port, DEFAULT_MAX_FILES, DEFAULT_MAX_BYTES_PER_FILE, + DEFAULT_MAX_AST_DIFFS, DEFAULT_MAX_ACTIONS_PER_AST_DIFF); } public McpDiffBrowserResult diffFileContents(Map beforeFiles, Map afterFiles, int port, int maxFiles, int maxBytesPerFile) { + return diffFileContents(beforeFiles, afterFiles, port, maxFiles, maxBytesPerFile, DEFAULT_MAX_AST_DIFFS, + DEFAULT_MAX_ACTIONS_PER_AST_DIFF); + } + + public McpDiffBrowserResult diffFileContents(Map beforeFiles, Map afterFiles, + int port, int maxFiles, int maxBytesPerFile, int maxAstDiffs, int maxActionsPerAstDiff) { if (beforeFiles == null || afterFiles == null) { return McpDiffBrowserResult.error("beforeFiles and afterFiles are required.", port, "Explicit file contents", List.of("beforeFiles and afterFiles must be JSON objects mapping paths to file contents.")); } + McpDiffBrowserResult invalidAstLimits = validateDiffAstLimits(maxAstDiffs, maxActionsPerAstDiff, port, + "Explicit file contents"); + if (invalidAstLimits != null) { + return invalidAstLimits; + } if (beforeFiles.isEmpty() && afterFiles.isEmpty()) { return McpDiffBrowserResult.error("At least one before or after file is required.", port, "Explicit file contents", List.of("beforeFiles and afterFiles cannot both be empty.")); @@ -370,7 +430,7 @@ public McpDiffBrowserResult diffFileContents(Map beforeFiles, Ma beforeFiles.size(), afterFiles.size()); try { ProjectASTDiff diff = differ.diffAtFileContents(beforeFiles, afterFiles); - return launchDiffBrowser(diff, port, inputSummary, List.of()); + return launchDiffBrowser(diff, port, inputSummary, List.of(), maxAstDiffs, maxActionsPerAstDiff); } catch (Exception e) { return McpDiffBrowserResult.error("File-content diff browser failed: " + e.getMessage(), port, inputSummary, List.of(e.getClass().getName())); @@ -379,14 +439,26 @@ public McpDiffBrowserResult diffFileContents(Map beforeFiles, Ma public McpDiffBrowserResult diffWorktree(Path repositoryPath, String baseRef, boolean includeUntracked, int maxFiles, int maxBytesPerFile, int port) { + return diffWorktree(repositoryPath, baseRef, includeUntracked, maxFiles, maxBytesPerFile, port, + DEFAULT_MAX_AST_DIFFS, DEFAULT_MAX_ACTIONS_PER_AST_DIFF); + } + + public McpDiffBrowserResult diffWorktree(Path repositoryPath, String baseRef, boolean includeUntracked, + int maxFiles, int maxBytesPerFile, int port, int maxAstDiffs, int maxActionsPerAstDiff) { String resolvedBaseRef = baseRef == null || baseRef.isBlank() ? "HEAD" : baseRef; Path resolvedRepositoryPath = resolveRepositoryPath(repositoryPath); String inputSummary = "Worktree changes in " + resolvedRepositoryPath + " against " + resolvedBaseRef + "."; + McpDiffBrowserResult invalidAstLimits = validateDiffAstLimits(maxAstDiffs, maxActionsPerAstDiff, port, + inputSummary); + if (invalidAstLimits != null) { + return invalidAstLimits; + } try { WorktreeChangeCollector.WorktreeChanges changes = new WorktreeChangeCollector() .collect(resolvedRepositoryPath, resolvedBaseRef, includeUntracked, maxFiles, maxBytesPerFile); ProjectASTDiff diff = differ.diffAtFileContents(changes.beforeFiles(), changes.afterFiles()); - return launchDiffBrowser(diff, port, inputSummary, changes.warnings()); + return launchDiffBrowser(diff, port, inputSummary, changes.warnings(), maxAstDiffs, + maxActionsPerAstDiff); } catch (Exception e) { return McpDiffBrowserResult.error("Worktree diff browser failed: " + e.getMessage(), port, inputSummary, List.of(e.getClass().getName())); @@ -394,6 +466,12 @@ public McpDiffBrowserResult diffWorktree(Path repositoryPath, String baseRef, bo } public McpDiffBrowserResult diffCommit(Path repositoryPath, String commitId, Integer parentIndex, int port) { + return diffCommit(repositoryPath, commitId, parentIndex, port, DEFAULT_MAX_AST_DIFFS, + DEFAULT_MAX_ACTIONS_PER_AST_DIFF); + } + + public McpDiffBrowserResult diffCommit(Path repositoryPath, String commitId, Integer parentIndex, int port, + int maxAstDiffs, int maxActionsPerAstDiff) { if (commitId == null || commitId.isBlank()) { return McpDiffBrowserResult.error("commitId must be a non-empty string.", port, "Commit diff", List.of("commitId is required.")); @@ -406,12 +484,17 @@ public McpDiffBrowserResult diffCommit(Path repositoryPath, String commitId, Int String inputSummary = String.format("Commit %s in %s against parent index %d.", commitId, repositoryPath, resolvedParentIndex); + McpDiffBrowserResult invalidAstLimits = validateDiffAstLimits(maxAstDiffs, maxActionsPerAstDiff, port, + inputSummary); + if (invalidAstLimits != null) { + return invalidAstLimits; + } try { Path resolvedRepositoryPath = resolveRepositoryPath(repositoryPath); inputSummary = String.format("Commit %s in %s against parent index %d.", commitId, resolvedRepositoryPath, resolvedParentIndex); ProjectASTDiff diff = diffCommit(resolvedRepositoryPath, commitId, resolvedParentIndex, 300); - return launchDiffBrowser(diff, port, inputSummary, List.of()); + return launchDiffBrowser(diff, port, inputSummary, List.of(), maxAstDiffs, maxActionsPerAstDiff); } catch (Exception e) { return McpDiffBrowserResult.error("Commit diff browser failed: " + e.getMessage(), port, inputSummary, List.of(e.getClass().getName())); @@ -419,6 +502,12 @@ public McpDiffBrowserResult diffCommit(Path repositoryPath, String commitId, Int } public McpDiffBrowserResult diffPullRequest(String cloneUrl, int pullRequestId, int timeoutSeconds, int port) { + return diffPullRequest(cloneUrl, pullRequestId, timeoutSeconds, port, DEFAULT_MAX_AST_DIFFS, + DEFAULT_MAX_ACTIONS_PER_AST_DIFF); + } + + public McpDiffBrowserResult diffPullRequest(String cloneUrl, int pullRequestId, int timeoutSeconds, int port, + int maxAstDiffs, int maxActionsPerAstDiff) { if (pullRequestId <= 0) { return McpDiffBrowserResult.error("pullRequestId must be greater than 0.", port, "Pull-request diff", List.of("pullRequestId=" + pullRequestId)); @@ -429,6 +518,11 @@ public McpDiffBrowserResult diffPullRequest(String cloneUrl, int pullRequestId, } String inputSummary = "Pull request #" + pullRequestId + "."; + McpDiffBrowserResult invalidAstLimits = validateDiffAstLimits(maxAstDiffs, maxActionsPerAstDiff, port, + inputSummary); + if (invalidAstLimits != null) { + return invalidAstLimits; + } String resolvedCloneUrl; try { resolvedCloneUrl = resolvePullRequestCloneUrl(cloneUrl); @@ -440,7 +534,7 @@ public McpDiffBrowserResult diffPullRequest(String cloneUrl, int pullRequestId, inputSummary = String.format("Pull request #%d in %s.", pullRequestId, resolvedCloneUrl); try { ProjectASTDiff diff = pullRequestDiffer.diffAtPullRequest(resolvedCloneUrl, pullRequestId, timeoutSeconds); - return launchDiffBrowser(diff, port, inputSummary, List.of()); + return launchDiffBrowser(diff, port, inputSummary, List.of(), maxAstDiffs, maxActionsPerAstDiff); } catch (Exception e) { return McpDiffBrowserResult.error("Pull-request diff browser failed: " + e.getMessage(), port, inputSummary, List.of(e.getClass().getName())); @@ -560,21 +654,53 @@ static Path resolveCommitRepositoryPath(Path repositoryPath) throws Exception { return realPath; } - private static McpAnalysisResult toResult(ProjectASTDiff diff, int maxRefactorings, String label) { + private static McpAnalysisResult toResult(ProjectASTDiff diff, int maxRefactorings, int maxAstDiffs, + int maxActionsPerAstDiff, String label) { if (diff == null) { return McpAnalysisResult.error(label + " analysis did not produce a diff.", List.of("RefactoringMiner returned null.")); } - return McpAnalysisResult.ok(diff, maxRefactorings); + return McpAnalysisResult.ok(diff, maxRefactorings, maxAstDiffs, maxActionsPerAstDiff); + } + + private static McpAnalysisResult validateAstDiffLimits(int maxAstDiffs, int maxActionsPerAstDiff) { + if (maxAstDiffs < 0) { + return McpAnalysisResult.error("maxAstDiffs must be greater than or equal to 0.", + List.of("maxAstDiffs=" + maxAstDiffs)); + } + if (maxActionsPerAstDiff < 0) { + return McpAnalysisResult.error("maxActionsPerAstDiff must be greater than or equal to 0.", + List.of("maxActionsPerAstDiff=" + maxActionsPerAstDiff)); + } + return null; } private McpDiffBrowserResult launchDiffBrowser(ProjectASTDiff diff, int port, String inputSummary, List warnings) throws Exception { + return launchDiffBrowser(diff, port, inputSummary, warnings, DEFAULT_MAX_AST_DIFFS, + DEFAULT_MAX_ACTIONS_PER_AST_DIFF); + } + + private McpDiffBrowserResult launchDiffBrowser(ProjectASTDiff diff, int port, String inputSummary, + List warnings, int maxAstDiffs, int maxActionsPerAstDiff) throws Exception { if (diff == null) { return McpDiffBrowserResult.error("Analysis did not produce a diff.", port, inputSummary, List.of("RefactoringMiner returned null.")); } - return diffBrowserLauncher.launch(diff, port, inputSummary, warnings); + return diffBrowserLauncher.launch(diff, port, inputSummary, warnings, maxAstDiffs, maxActionsPerAstDiff); + } + + private static McpDiffBrowserResult validateDiffAstLimits(int maxAstDiffs, int maxActionsPerAstDiff, int port, + String inputSummary) { + if (maxAstDiffs < 0) { + return McpDiffBrowserResult.error("maxAstDiffs must be greater than or equal to 0.", port, + inputSummary, List.of("maxAstDiffs=" + maxAstDiffs)); + } + if (maxActionsPerAstDiff < 0) { + return McpDiffBrowserResult.error("maxActionsPerAstDiff must be greater than or equal to 0.", port, + inputSummary, List.of("maxActionsPerAstDiff=" + maxActionsPerAstDiff)); + } + return null; } private static void validateFileContentBounds(Map beforeFiles, Map afterFiles, diff --git a/src/main/java/org/refactoringminer/mcp/RefactoringMinerMcpTools.java b/src/main/java/org/refactoringminer/mcp/RefactoringMinerMcpTools.java index fab56e7e7a..4a638ea9be 100644 --- a/src/main/java/org/refactoringminer/mcp/RefactoringMinerMcpTools.java +++ b/src/main/java/org/refactoringminer/mcp/RefactoringMinerMcpTools.java @@ -34,6 +34,8 @@ public final class RefactoringMinerMcpTools { static final String DIFF_PULL_REQUEST = "refactoringminer_diff_pull_request"; private static final int DEFAULT_MAX_REFACTORINGS = 20; private static final int DEFAULT_MAX_CANDIDATES = 20; + private static final int DEFAULT_MAX_AST_DIFFS = 10; + private static final int DEFAULT_MAX_ACTIONS_PER_AST_DIFF = 10; private static final int DEFAULT_MAX_FILES = 100; private static final int DEFAULT_MAX_BYTES_PER_FILE = 200_000; private static final int DEFAULT_TIMEOUT_SECONDS = 300; @@ -273,11 +275,14 @@ static McpAnalysisResult analyze(RefactoringMinerMcpService service, Map afterFiles = stringMap(arguments.get("afterFiles"), "afterFiles"); int maxRefactorings = integerValue(arguments.get("maxRefactorings"), DEFAULT_MAX_REFACTORINGS, "maxRefactorings"); + int maxAstDiffs = integerValue(arguments.get("maxAstDiffs"), DEFAULT_MAX_AST_DIFFS, "maxAstDiffs"); + int maxActionsPerAstDiff = integerValue(arguments.get("maxActionsPerAstDiff"), + DEFAULT_MAX_ACTIONS_PER_AST_DIFF, "maxActionsPerAstDiff"); int maxFiles = integerValue(arguments.get("maxFiles"), DEFAULT_MAX_FILES, "maxFiles"); int maxBytesPerFile = integerValue(arguments.get("maxBytesPerFile"), DEFAULT_MAX_BYTES_PER_FILE, "maxBytesPerFile"); return service.analyzeFileContents(beforeFiles, afterFiles, maxRefactorings, maxFiles, - maxBytesPerFile); + maxBytesPerFile, maxAstDiffs, maxActionsPerAstDiff); } catch (IllegalArgumentException e) { return McpAnalysisResult.error(e.getMessage(), List.of("Invalid tool arguments.")); } @@ -301,8 +306,11 @@ static McpAnalysisResult analyzeWorktree(RefactoringMinerMcpService service, Map "maxBytesPerFile"); int maxRefactorings = integerValue(arguments.get("maxRefactorings"), DEFAULT_MAX_REFACTORINGS, "maxRefactorings"); + int maxAstDiffs = integerValue(arguments.get("maxAstDiffs"), DEFAULT_MAX_AST_DIFFS, "maxAstDiffs"); + int maxActionsPerAstDiff = integerValue(arguments.get("maxActionsPerAstDiff"), + DEFAULT_MAX_ACTIONS_PER_AST_DIFF, "maxActionsPerAstDiff"); return service.analyzeWorktree(repositoryPath, baseRef, includeUntracked, maxFiles, - maxBytesPerFile, maxRefactorings); + maxBytesPerFile, maxRefactorings, maxAstDiffs, maxActionsPerAstDiff); } catch (IllegalArgumentException e) { return McpAnalysisResult.error(e.getMessage(), List.of("Invalid tool arguments.")); } @@ -323,7 +331,11 @@ static McpAnalysisResult analyzeCommit(RefactoringMinerMcpService service, Map Integer.MAX_VALUE) { + throw new IllegalArgumentException(name + " must be an integer."); + } + return longValue.intValue(); + } + if (number instanceof java.math.BigInteger bigInteger) { + try { + return bigInteger.intValueExact(); + } catch (ArithmeticException e) { + throw new IllegalArgumentException(name + " must be an integer.", e); + } + } + if (number instanceof java.math.BigDecimal bigDecimal) { + try { + return bigDecimal.toBigIntegerExact().intValueExact(); + } catch (ArithmeticException e) { + throw new IllegalArgumentException(name + " must be an integer.", e); + } + } + double doubleValue = number.doubleValue(); + if (!Double.isFinite(doubleValue) || doubleValue % 1 != 0 || doubleValue < Integer.MIN_VALUE + || doubleValue > Integer.MAX_VALUE) { + throw new IllegalArgumentException(name + " must be an integer."); + } + return (int) doubleValue; } if (value instanceof String stringValue) { - return Integer.parseInt(stringValue); + try { + return Integer.parseInt(stringValue); + } catch (NumberFormatException e) { + throw new IllegalArgumentException(name + " must be an integer.", e); + } } throw new IllegalArgumentException(name + " must be an integer."); } @@ -749,6 +816,7 @@ private static JsonSchema inputSchema() { properties.put("beforeFiles", fileMapSchema); properties.put("afterFiles", fileMapSchema); properties.put("maxRefactorings", maxRefactoringsSchema); + putAstDiffSummaryProperties(properties); putFileContentBoundsProperties(properties); return new JsonSchema("object", properties, List.of("beforeFiles", "afterFiles"), false, null, null); } @@ -761,6 +829,7 @@ private static JsonSchema worktreeInputSchema() { properties.put("maxFiles", Map.of("type", "integer", "minimum", 1, "default", DEFAULT_MAX_FILES)); properties.put("maxBytesPerFile", Map.of("type", "integer", "minimum", 1, "default", DEFAULT_MAX_BYTES_PER_FILE)); properties.put("maxRefactorings", Map.of("type", "integer", "minimum", 0, "default", DEFAULT_MAX_REFACTORINGS)); + putAstDiffSummaryProperties(properties); return new JsonSchema("object", properties, List.of(), false, null, null); } @@ -770,6 +839,7 @@ private static JsonSchema commitInputSchema() { properties.put("commitId", Map.of("type", "string")); properties.put("parentIndex", Map.of("type", "integer", "minimum", 0)); properties.put("maxRefactorings", Map.of("type", "integer", "minimum", 0, "default", DEFAULT_MAX_REFACTORINGS)); + putAstDiffSummaryProperties(properties); return new JsonSchema("object", properties, List.of("commitId"), false, null, null); } @@ -779,6 +849,7 @@ private static JsonSchema pullRequestInputSchema() { properties.put("pullRequestId", Map.of("type", "integer", "minimum", 1)); properties.put("timeoutSeconds", Map.of("type", "integer", "minimum", 1, "default", DEFAULT_TIMEOUT_SECONDS)); properties.put("maxRefactorings", Map.of("type", "integer", "minimum", 0, "default", DEFAULT_MAX_REFACTORINGS)); + putAstDiffSummaryProperties(properties); return new JsonSchema("object", properties, List.of("pullRequestId"), false, null, null); } @@ -787,6 +858,7 @@ private static JsonSchema directoriesInputSchema() { properties.put("beforePath", Map.of("type", "string")); properties.put("afterPath", Map.of("type", "string")); properties.put("maxRefactorings", Map.of("type", "integer", "minimum", 0, "default", DEFAULT_MAX_REFACTORINGS)); + putAstDiffSummaryProperties(properties); return new JsonSchema("object", properties, List.of("beforePath", "afterPath"), false, null, null); } @@ -878,6 +950,7 @@ private static void putDiffBrowserProperties(Map properties) { properties.put("port", Map.of("type", "integer", "minimum", 1, "maximum", 65535, "default", DEFAULT_WEB_DIFF_PORT, "description", "Local WebDiff port. Defaults to 6789.")); + putAstDiffSummaryProperties(properties); } private static void putRepositoryPathProperty(Map properties) { @@ -903,6 +976,19 @@ private static void putValidationProperties(Map properties) { properties.put("maxCandidates", Map.of("type", "integer", "minimum", 0, "default", DEFAULT_MAX_CANDIDATES)); } + private static void putAstDiffSummaryProperties(Map properties) { + properties.put("maxAstDiffs", Map.of( + "type", "integer", + "minimum", 0, + "default", DEFAULT_MAX_AST_DIFFS, + "description", "Maximum number of AST diff summaries to include. Use 0 to omit astDiffs.")); + properties.put("maxActionsPerAstDiff", Map.of( + "type", "integer", + "minimum", 0, + "default", DEFAULT_MAX_ACTIONS_PER_AST_DIFF, + "description", "Maximum number of sampled edit actions per AST diff summary.")); + } + private static Map fileMapSchema() { return Map.of( "type", "object", @@ -960,12 +1046,13 @@ private static Map outputSchema() { properties.put("filesBefore", Map.of("type", "integer")); properties.put("filesAfter", Map.of("type", "integer")); properties.put("refactorings", Map.of("type", "array")); + properties.put("astDiffs", Map.of("type", "array")); properties.put("warnings", Map.of("type", "array", "items", Map.of("type", "string"))); return Map.of( "type", "object", "properties", properties, "required", List.of("status", "summary", "refactoringCount", "astDiffCount", "moveAstDiffCount", - "filesBefore", "filesAfter", "refactorings", "warnings")); + "filesBefore", "filesAfter", "refactorings", "astDiffs", "warnings")); } private static Map diffBrowserOutputSchema() { @@ -982,11 +1069,12 @@ private static Map diffBrowserOutputSchema() { properties.put("filesBefore", Map.of("type", "integer")); properties.put("filesAfter", Map.of("type", "integer")); properties.put("affectedFiles", Map.of("type", "array", "items", Map.of("type", "string"))); + properties.put("astDiffs", Map.of("type", "array")); properties.put("warnings", Map.of("type", "array", "items", Map.of("type", "string"))); return Map.of( "type", "object", "properties", properties, "required", List.of("status", "summary", "inputSummary", "refactoringCount", "astDiffCount", - "moveAstDiffCount", "filesBefore", "filesAfter", "affectedFiles", "warnings")); + "moveAstDiffCount", "filesBefore", "filesAfter", "affectedFiles", "astDiffs", "warnings")); } } diff --git a/src/main/java/org/refactoringminer/mcp/WebDiffBrowserLauncher.java b/src/main/java/org/refactoringminer/mcp/WebDiffBrowserLauncher.java index 46a26a9c69..538c2cd885 100644 --- a/src/main/java/org/refactoringminer/mcp/WebDiffBrowserLauncher.java +++ b/src/main/java/org/refactoringminer/mcp/WebDiffBrowserLauncher.java @@ -38,7 +38,7 @@ private WebDiffBrowserLauncher(String bindHost, String publicHost) { @Override public synchronized McpDiffBrowserResult launch(ProjectASTDiff diff, int port, String inputSummary, - List warnings) throws Exception { + List warnings, int maxAstDiffs, int maxActionsPerAstDiff) throws Exception { if (diff == null) { throw new IllegalArgumentException("ProjectASTDiff is required."); } @@ -64,7 +64,8 @@ public synchronized McpDiffBrowserResult launch(ProjectASTDiff diff, int port, S } activeView = view; activePort = port; - return McpDiffBrowserResult.ok(diff, port, publicHost, inputSummary, warnings); + return McpDiffBrowserResult.ok(diff, port, publicHost, inputSummary, warnings, maxAstDiffs, + maxActionsPerAstDiff); } private void stopActiveView() { diff --git a/src/test/java/org/refactoringminer/mcp/RefactoringMinerMcpServiceTest.java b/src/test/java/org/refactoringminer/mcp/RefactoringMinerMcpServiceTest.java index 3e41904587..b548ee63d4 100644 --- a/src/test/java/org/refactoringminer/mcp/RefactoringMinerMcpServiceTest.java +++ b/src/test/java/org/refactoringminer/mcp/RefactoringMinerMcpServiceTest.java @@ -10,11 +10,20 @@ import java.util.Map; import java.util.Set; +import com.github.gumtreediff.actions.model.Action; +import com.github.gumtreediff.actions.model.Update; +import com.github.gumtreediff.tree.Tree; +import com.github.gumtreediff.tree.TreeContext; +import com.github.gumtreediff.tree.TypeSet; import org.apache.commons.lang3.tuple.ImmutablePair; import org.junit.jupiter.api.Test; import org.refactoringminer.api.Refactoring; import org.refactoringminer.api.RefactoringType; +import org.refactoringminer.astDiff.actions.model.MultiMove; +import org.refactoringminer.astDiff.models.ASTDiff; +import org.refactoringminer.astDiff.models.ExtendedMultiMappingStore; import org.refactoringminer.astDiff.models.ProjectASTDiff; +import org.refactoringminer.astDiff.utils.Constants; import gr.uom.java.xmi.LocationInfo.CodeElementType; import gr.uom.java.xmi.diff.CodeRange; @@ -76,6 +85,110 @@ void analyzeFileContentsUsesExistingDifferAndPreservesCounts() { assertTrue(result.summary().contains("0 refactorings")); } + @Test + void analyzeFileContentsCanReturnBoundedAstDiffSummaries() { + RefactoringMinerMcpService service = new RefactoringMinerMcpService((before, after) -> { + ProjectASTDiff diff = new ProjectASTDiff(before, after); + diff.addASTDiff(astDiffWithUpdate()); + diff.setRefactorings(List.of()); + return diff; + }); + Map before = Map.of("src/main/java/A.java", "class A {\n void f() {}\n}"); + Map after = Map.of("src/main/java/A.java", "class A {\n void g() {}\n}"); + + McpAnalysisResult result = service.analyzeFileContents(before, after, 20, 100, 200_000, 1, 1); + + assertEquals("ok", result.status()); + assertEquals(1, result.astDiffs().size()); + McpAstDiffResult astDiff = result.astDiffs().get(0); + assertEquals("standard", astDiff.kind()); + assertEquals("src/main/java/A.java", astDiff.beforeFilePath()); + assertEquals("src/main/java/A.java", astDiff.afterFilePath()); + assertEquals(1, astDiff.mappingCount()); + assertEquals(1, astDiff.actionCount()); + assertEquals(1, astDiff.sampleActions().size()); + assertEquals("update-node", astDiff.sampleActions().get(0).name()); + assertEquals(2, astDiff.sampleActions().get(0).startLine()); + } + + @Test + void analyzeFileContentsOmitsActionSamplesWithoutTruncationWarningWhenLimitIsZero() { + RefactoringMinerMcpService service = new RefactoringMinerMcpService((before, after) -> { + ProjectASTDiff diff = new ProjectASTDiff(before, after); + diff.addASTDiff(astDiffWithUpdate()); + diff.setRefactorings(List.of()); + return diff; + }); + Map before = Map.of("src/main/java/A.java", "class A {\n void f() {}\n}"); + Map after = Map.of("src/main/java/A.java", "class A {\n void g() {}\n}"); + + McpAnalysisResult result = service.analyzeFileContents(before, after, 20, 100, 200_000, 1, 0); + + assertEquals("ok", result.status()); + assertEquals(1, result.astDiffs().size()); + assertEquals(1, result.astDiffs().get(0).actionCount()); + assertTrue(result.astDiffs().get(0).sampleActions().isEmpty()); + assertTrue(result.warnings().stream().noneMatch(warning -> warning.contains("truncated to 0"))); + } + + @Test + void analyzeFileContentsReportsMissingPositionsWithConsistentSentinels() { + RefactoringMinerMcpService service = new RefactoringMinerMcpService((before, after) -> { + ProjectASTDiff diff = new ProjectASTDiff(before, after); + diff.addASTDiff(astDiffWithNullNodeAction()); + diff.setRefactorings(List.of()); + return diff; + }); + Map before = Map.of("src/main/java/A.java", "class A {}"); + Map after = Map.of("src/main/java/A.java", "class A {}"); + + McpAnalysisResult result = service.analyzeFileContents(before, after, 20, 100, 200_000, 1, 1); + + McpAstDiffResult.ActionSummary action = result.astDiffs().get(0).sampleActions().get(0); + assertEquals(-1, action.startOffset()); + assertEquals(-1, action.endOffset()); + assertEquals(-1, action.startLine()); + assertEquals(-1, action.endLine()); + } + + @Test + void analyzeFileContentsTreatsEndOffsetAsExclusiveForEndLine() { + String content = "class A {\n void f() {}\n}\n"; + RefactoringMinerMcpService service = new RefactoringMinerMcpService((before, after) -> { + ProjectASTDiff diff = new ProjectASTDiff(before, after); + diff.addASTDiff(astDiffWithWholeFileUpdate(content)); + diff.setRefactorings(List.of()); + return diff; + }); + Map before = Map.of("src/main/java/A.java", content); + Map after = Map.of("src/main/java/A.java", content); + + McpAnalysisResult result = service.analyzeFileContents(before, after, 20, 100, 200_000, 1, 1); + + McpAstDiffResult.ActionSummary action = result.astDiffs().get(0).sampleActions().get(0); + assertEquals(1, action.startLine()); + assertEquals(3, action.endLine()); + } + + @Test + void analyzeFileContentsReportsMultiMoveTargetPathAndGroupSeparately() { + RefactoringMinerMcpService service = new RefactoringMinerMcpService((before, after) -> { + ProjectASTDiff diff = new ProjectASTDiff(before, after); + diff.addASTDiff(astDiffWithMultiMove()); + diff.setRefactorings(List.of()); + return diff; + }); + Map before = Map.of("src/main/java/A.java", "class A {\n void f() {}\n}"); + Map after = Map.of("src/main/java/A.java", "class A {\n void f() {}\n}"); + + McpAnalysisResult result = service.analyzeFileContents(before, after, 20, 100, 200_000, 1, 1); + + McpAstDiffResult.ActionSummary action = result.astDiffs().get(0).sampleActions().get(0); + assertEquals("multi-move-tree", action.name()); + assertEquals("src/main/java/A.java", action.targetFilePath()); + assertEquals(42, action.moveGroupId()); + } + @Test void diffFileContentsUsesLauncherAndReturnsLocalBrowserResult() { RefactoringMinerMcpService service = new RefactoringMinerMcpService((before, after) -> { @@ -85,7 +198,9 @@ void diffFileContentsUsesLauncherAndReturnsLocalBrowserResult() { }, (repositoryPath, commitId, parentIndex) -> new ProjectASTDiff(Map.of(), Map.of()), (cloneUrl, pullRequestId, timeoutSeconds) -> new ProjectASTDiff(Map.of(), Map.of()), (beforePath, afterPath) -> new ProjectASTDiff(Map.of(), Map.of()), - (diff, port, inputSummary, warnings) -> McpDiffBrowserResult.ok(diff, port, inputSummary, warnings)); + (diff, port, inputSummary, warnings, maxAstDiffs, maxActionsPerAstDiff) -> + McpDiffBrowserResult.ok(diff, port, inputSummary, warnings, maxAstDiffs, + maxActionsPerAstDiff)); Map before = Map.of("src/main/java/A.java", "class A { void f() {} }"); Map after = Map.of("src/main/java/A.java", "class A { void g() {} }"); @@ -107,7 +222,7 @@ void diffFileContentsReturnsErrorShapeForLauncherFailures() { (repositoryPath, commitId, parentIndex) -> new ProjectASTDiff(Map.of(), Map.of()), (cloneUrl, pullRequestId, timeoutSeconds) -> new ProjectASTDiff(Map.of(), Map.of()), (beforePath, afterPath) -> new ProjectASTDiff(Map.of(), Map.of()), - (diff, port, inputSummary, warnings) -> { + (diff, port, inputSummary, warnings, maxAstDiffs, maxActionsPerAstDiff) -> { throw new IllegalArgumentException("port must be between 1 and 65535."); }); @@ -121,6 +236,101 @@ void diffFileContentsReturnsErrorShapeForLauncherFailures() { assertFalse(result.warnings().isEmpty()); } + private static ASTDiff astDiffWithUpdate() { + TreeContext beforeContext = new TreeContext(); + Tree beforeRoot = beforeContext.createTree(TypeSet.type("CompilationUnit"), ""); + Tree beforeMethod = beforeContext.createTree(TypeSet.type("MethodDeclaration"), "f"); + beforeMethod.setPos(12); + beforeMethod.setLength(8); + beforeRoot.addChild(beforeMethod); + beforeContext.setRoot(beforeRoot); + + TreeContext afterContext = new TreeContext(); + Tree afterRoot = afterContext.createTree(TypeSet.type("CompilationUnit"), ""); + Tree afterMethod = afterContext.createTree(TypeSet.type("MethodDeclaration"), "g"); + afterMethod.setPos(12); + afterMethod.setLength(8); + afterRoot.addChild(afterMethod); + afterContext.setRoot(afterRoot); + + ExtendedMultiMappingStore mappings = new ExtendedMultiMappingStore(beforeRoot, afterRoot, + new Constants("src/main/java/A.java"), new Constants("src/main/java/A.java")); + mappings.addMapping(beforeMethod, afterMethod); + ASTDiff astDiff = new ASTDiff("src/main/java/A.java", "src/main/java/A.java", beforeContext, afterContext, + mappings); + astDiff.editScript.add(new Update(beforeMethod, "g")); + return astDiff; + } + + private static ASTDiff astDiffWithNullNodeAction() { + TreeContext beforeContext = new TreeContext(); + Tree beforeRoot = beforeContext.createTree(TypeSet.type("CompilationUnit"), ""); + beforeContext.setRoot(beforeRoot); + + TreeContext afterContext = new TreeContext(); + Tree afterRoot = afterContext.createTree(TypeSet.type("CompilationUnit"), ""); + afterContext.setRoot(afterRoot); + + ExtendedMultiMappingStore mappings = new ExtendedMultiMappingStore(beforeRoot, afterRoot, + new Constants("src/main/java/A.java"), new Constants("src/main/java/A.java")); + ASTDiff astDiff = new ASTDiff("src/main/java/A.java", "src/main/java/A.java", beforeContext, afterContext, + mappings); + astDiff.editScript.add(new Action(null) { + @Override + public String getName() { + return "unknown-action"; + } + }); + return astDiff; + } + + private static ASTDiff astDiffWithWholeFileUpdate(String content) { + TreeContext beforeContext = new TreeContext(); + Tree beforeRoot = beforeContext.createTree(TypeSet.type("CompilationUnit"), ""); + beforeRoot.setPos(0); + beforeRoot.setLength(content.length()); + beforeContext.setRoot(beforeRoot); + + TreeContext afterContext = new TreeContext(); + Tree afterRoot = afterContext.createTree(TypeSet.type("CompilationUnit"), ""); + afterRoot.setPos(0); + afterRoot.setLength(content.length()); + afterContext.setRoot(afterRoot); + + ExtendedMultiMappingStore mappings = new ExtendedMultiMappingStore(beforeRoot, afterRoot, + new Constants("src/main/java/A.java"), new Constants("src/main/java/A.java")); + mappings.addMapping(beforeRoot, afterRoot); + ASTDiff astDiff = new ASTDiff("src/main/java/A.java", "src/main/java/A.java", beforeContext, afterContext, + mappings); + astDiff.editScript.add(new Update(beforeRoot, "")); + return astDiff; + } + + private static ASTDiff astDiffWithMultiMove() { + TreeContext beforeContext = new TreeContext(); + Tree beforeRoot = beforeContext.createTree(TypeSet.type("CompilationUnit"), ""); + Tree beforeMethod = beforeContext.createTree(TypeSet.type("MethodDeclaration"), "f"); + beforeMethod.setPos(12); + beforeMethod.setLength(8); + beforeRoot.addChild(beforeMethod); + beforeContext.setRoot(beforeRoot); + + TreeContext afterContext = new TreeContext(); + Tree afterRoot = afterContext.createTree(TypeSet.type("CompilationUnit"), ""); + Tree afterMethod = afterContext.createTree(TypeSet.type("MethodDeclaration"), "f"); + afterMethod.setPos(12); + afterMethod.setLength(8); + afterRoot.addChild(afterMethod); + afterContext.setRoot(afterRoot); + + ExtendedMultiMappingStore mappings = new ExtendedMultiMappingStore(beforeRoot, afterRoot, + new Constants("src/main/java/A.java"), new Constants("src/main/java/A.java")); + ASTDiff astDiff = new ASTDiff("src/main/java/A.java", "src/main/java/A.java", beforeContext, afterContext, + mappings); + astDiff.editScript.add(new MultiMove(beforeMethod, afterRoot, 0, 42, false)); + return astDiff; + } + private static class FakeRefactoring implements Refactoring { private final String description; diff --git a/src/test/java/org/refactoringminer/mcp/RefactoringMinerMcpToolsTest.java b/src/test/java/org/refactoringminer/mcp/RefactoringMinerMcpToolsTest.java index 38cda66f1b..ed13bee185 100644 --- a/src/test/java/org/refactoringminer/mcp/RefactoringMinerMcpToolsTest.java +++ b/src/test/java/org/refactoringminer/mcp/RefactoringMinerMcpToolsTest.java @@ -4,6 +4,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.math.BigDecimal; import java.nio.file.Files; import java.nio.file.Path; import java.util.List; @@ -41,6 +42,8 @@ void fileContentsToolMetadataIsReadOnlyAndNamedForAgents() { assertFalse(tool.tool().annotations().destructiveHint()); assertTrue(tool.tool().inputSchema().required().contains("beforeFiles")); assertTrue(tool.tool().inputSchema().required().contains("afterFiles")); + assertTrue(tool.tool().inputSchema().properties().containsKey("maxAstDiffs")); + assertTrue(tool.tool().inputSchema().properties().containsKey("maxActionsPerAstDiff")); } @Test @@ -185,6 +188,7 @@ void fileContentsToolReturnsJsonTextAndStructuredContent() throws Exception { assertEquals("ok", json.get("status").asText()); assertEquals(1, json.get("filesBefore").asInt()); assertEquals(1, json.get("filesAfter").asInt()); + assertTrue(json.has("astDiffs")); } @Test @@ -204,6 +208,23 @@ void fileContentsToolReturnsErrorShapeForInvalidArguments() throws Exception { assertTrue(json.get("summary").asText().contains("beforeFiles and afterFiles are required")); } + @Test + void numericToolArgumentsRejectFractionalValues() throws Exception { + SyncToolSpecification tool = RefactoringMinerMcpTools.analyzeFileContentsTool(fakeService()); + CallToolRequest request = new CallToolRequest(RefactoringMinerMcpTools.ANALYZE_FILE_CONTENTS, Map.of( + "beforeFiles", Map.of("src/main/java/A.java", "class A {}"), + "afterFiles", Map.of("src/main/java/A.java", "class A { int x; }"), + "maxRefactorings", new BigDecimal("1.5"))); + + CallToolResult result = tool.callHandler().apply(null, request); + + assertTrue(result.isError()); + TextContent content = (TextContent) result.content().get(0); + JsonNode json = OBJECT_MAPPER.readTree(content.text()); + assertEquals("error", json.get("status").asText()); + assertTrue(json.get("summary").asText().contains("maxRefactorings must be an integer")); + } + @Test void explicitFileContentToolsRejectOversizedInput() throws Exception { Map beforeFiles = Map.of("src/main/java/A.java", "class A { void f() {} }"); @@ -587,11 +608,12 @@ private static RefactoringMinerMcpService fakeDiffBrowserService() { (beforePath, afterPath) -> diffWithRefactoring( Map.of("src/main/java/A.java", "class A { void f() {} }"), Map.of("src/main/java/A.java", "class A { void g() {} }")), - (diff, port, inputSummary, warnings) -> { + (diff, port, inputSummary, warnings, maxAstDiffs, maxActionsPerAstDiff) -> { if (port < 1 || port > 65535) { throw new IllegalArgumentException("port must be between 1 and 65535."); } - return McpDiffBrowserResult.ok(diff, port, inputSummary, warnings); + return McpDiffBrowserResult.ok(diff, port, inputSummary, warnings, maxAstDiffs, + maxActionsPerAstDiff); }); }