From 94a4504d6a3a0de27497f5e3722233014dd713db Mon Sep 17 00:00:00 2001 From: Myoungdo Park Date: Thu, 19 Feb 2026 19:13:37 +0900 Subject: [PATCH 1/2] Improve execute_commands response schema and diagnostics --- README.md | 7 +- design.md | 4 +- ...cute-commands-response-improvement-plan.md | 158 +++++++++ .../mcp/mod/command/ChatMessageCapture.java | 10 +- .../mcp/mod/command/CommandExecutor.java | 310 ++++++++++++------ .../mod/command/CommandOutcomeAnalyzer.java | 80 +++++ .../mcp/mod/command/CommandResult.java | 74 +++-- .../cuspymd/mcp/mod/server/MCPProtocol.java | 6 +- .../mod/command/ChatMessageCaptureTest.java | 61 ++++ .../CommandExecutorResponseSchemaTest.java | 89 +++++ .../command/CommandOutcomeAnalyzerTest.java | 109 ++++++ .../mcp/mod/server/MCPProtocolTest.java | 2 + 12 files changed, 773 insertions(+), 137 deletions(-) create mode 100644 docs/execute-commands-response-improvement-plan.md create mode 100644 src/client/java/cuspymd/mcp/mod/command/CommandOutcomeAnalyzer.java create mode 100644 src/test/java/cuspymd/mcp/mod/command/ChatMessageCaptureTest.java create mode 100644 src/test/java/cuspymd/mcp/mod/command/CommandExecutorResponseSchemaTest.java create mode 100644 src/test/java/cuspymd/mcp/mod/command/CommandOutcomeAnalyzerTest.java diff --git a/README.md b/README.md index 33b4c2f..6e814cc 100644 --- a/README.md +++ b/README.md @@ -146,6 +146,11 @@ Execute one or more Minecraft commands sequentially with safety validation. - `commands` (array): List of Minecraft commands (without leading slash) - `validate_safety` (boolean): Enable safety validation (default: true) +**Response schema (text payload JSON):** +- Top-level: `totalCommands`, `acceptedCount`, `appliedCount`, `failedCount`, `results`, `chatMessages` +- Per command: `index`, `command`, `status`, `accepted`, `applied`, `summary`, `chatMessages` +- `status` values: `applied`, `rejected_by_game`, `execution_error`, `timed_out`, `rejected_by_safety`, `unknown` + **Example Request:** ```json { @@ -237,4 +242,4 @@ This project is licensed under the CC0-1.0 License. For issues and questions: - Check the [Issues](https://github.com/your-repo/issues) page - Review the configuration documentation -- Enable debug logging for detailed troubleshooting \ No newline at end of file +- Enable debug logging for detailed troubleshooting diff --git a/design.md b/design.md index c6d4cc6..80bf097 100644 --- a/design.md +++ b/design.md @@ -141,7 +141,7 @@ minecraft-mcp-client/ "content": [ { "type": "text", - "text": "Executed 3 commands successfully:\n1. fill ~ ~ ~ ~10 ~5 ~8 oak_planks (360 blocks affected)\n2. setblock ~5 ~6 ~4 oak_door (1 block affected)\n3. summon villager ~5 ~1 ~4 (1 entity spawned)" + "text": "{\"totalCommands\":2,\"acceptedCount\":2,\"appliedCount\":1,\"failedCount\":1,\"results\":[{\"index\":0,\"command\":\"fill 0 64 0 1 64 1 stone\",\"status\":\"applied\",\"accepted\":true,\"applied\":true,\"summary\":\"Successfully filled 4 block(s)\",\"chatMessages\":[\"Successfully filled 4 block(s)\"]},{\"index\":1,\"command\":\"enchant @s minecraft:unbreaking 1\",\"status\":\"rejected_by_game\",\"accepted\":true,\"applied\":false,\"summary\":\"Carrot cannot support that enchantment\",\"chatMessages\":[\"Carrot cannot support that enchantment\"]}],\"chatMessages\":[\"Successfully filled 4 block(s)\",\"Carrot cannot support that enchantment\"]}" } ] } @@ -294,4 +294,4 @@ User-Agent: MCP-Client/1.0 "require_op_for_admin_commands": true } } -``` \ No newline at end of file +``` diff --git a/docs/execute-commands-response-improvement-plan.md b/docs/execute-commands-response-improvement-plan.md new file mode 100644 index 0000000..d504a5e --- /dev/null +++ b/docs/execute-commands-response-improvement-plan.md @@ -0,0 +1,158 @@ +# execute_commands 응답 개선 상세 수정 계획 + +## 1. 배경과 목표 +현재 `execute_commands` 응답은 다음 문제가 있다. +1. `results[].success`가 실제 게임 적용 성공을 의미하지 못한다. +2. `chatMessages`가 전역 배열이라 명령별 매핑이 불가능하다. +3. `message`가 대부분 `Command executed`로 고정되어 진단성과 자동화 활용성이 낮다. + +목표는 응답 스키마를 확장해 실제 적용 결과와 진단 정보를 명령 단위로 제공하는 것이다. + +## 2. 범위 +대상 범위는 `execute_commands` 도구 응답에 한정한다. +1. 입력 스키마: `validate_safety` 등 기존 입력은 유지. +2. 출력 스키마: 명령 단위 적용 결과와 진단 정보 중심으로 확장. +3. 안전 검증 실패 응답 형식은 기존 구조를 유지하되 메타 정보 일관성만 개선. + +## 3. 설계 원칙 +1. 의미 분리: 전송 성공과 게임 적용 성공을 별도 필드로 분리한다. +2. 기계 친화성: 숫자/상태를 구조화 필드로 제공한다. +3. 명령 단위 진단성: 각 명령의 요약과 메시지를 독립적으로 제공한다. + +## 4. 목표 응답 스키마 (제안) + +```json +{ + "totalCommands": 2, + "acceptedCount": 2, + "appliedCount": 1, + "failedCount": 1, + "results": [ + { + "index": 0, + "command": "fill -40 200 -40 -39 200 -39 minecraft:glass", + "status": "applied", + "accepted": true, + "applied": true, + "executionTimeMs": 51, + "summary": "Filled 4 blocks", + "chatMessages": [ + "Successfully filled 4 block(s)" + ] + }, + { + "index": 1, + "command": "enchant @s minecraft:unbreaking 1", + "status": "rejected_by_game", + "accepted": true, + "applied": false, + "executionTimeMs": 50, + "summary": "Carrot cannot support that enchantment", + "chatMessages": [ + "Carrot cannot support that enchantment" + ] + } + ], + "chatMessages": [ + "Successfully filled 4 block(s)", + "Carrot cannot support that enchantment" + ], + "hint": "Use get_blocks_in_area to verify the built structure and fix any issues." +} +``` + +상태값(`status`) 권장 집합은 `applied`, `rejected_by_game`, `execution_error`, `timed_out`, `rejected_by_safety`로 정의한다. + +## 5. 구현 상세 계획 + +### 5.1 CommandResult 모델 확장 +파일: `src/client/java/cuspymd/mcp/mod/command/CommandResult.java` + +1. `transportAccepted` 필드 추가. +2. `applied` 필드 추가. +3. `status` 필드 추가. +4. `summary` 필드 추가. +5. `chatMessages` 필드 추가. +6. 기존 `success`/`message` 의존 코드를 제거하고 `status`/`applied`/`summary`를 단일 기준으로 사용. + +## 5.2 executeOneCommand 결과 수집 개선 +파일: `src/client/java/cuspymd/mcp/mod/command/CommandExecutor.java` + +1. `sendChatCommand` 호출 성공 여부를 `transportAccepted`에 반영. +2. 명령 실행 직후 글로벌 수집 대신 명령 단위 수집 윈도우를 적용. +3. 명령별 메시지 파싱기를 도입해 `applied`와 `summary`를 결정. +4. 파싱 실패 시 안전한 기본값을 적용한다. +5. 기본 규칙은 다음과 같다. + +| 조건 | applied | status | +|---|---|---| +| 실패 표현 포함 (`cannot`, `failed`, `no entity was found`, `unknown`) | false | rejected_by_game | +| 성공 표현 포함 (`successfully`, `teleported`, `summoned`, `given`, `set the weather`) | true | applied | +| 메시지 없음 + 전송 성공 | null 또는 true(정책 선택) | applied 또는 unknown | +| 예외 발생 | false | execution_error | + +정책 결정안: 초기 구현은 메시지 없음이면 `applied`를 `null`로 두고 상태를 `unknown`으로 둔다. + +## 5.3 응답 조립 로직 개선 +파일: `src/client/java/cuspymd/mcp/mod/command/CommandExecutor.java` + +1. `successCount`를 `acceptedCount`와 `appliedCount`로 분리. +2. `results[]`에 `status`, `accepted`, `applied`, `summary`, `chatMessages`를 추가. +3. `results[]`의 핵심 판단 기준을 `status`, `applied`, `summary`로 통일한다. +4. 전역 `chatMessages`는 디버깅/요약 용도로만 제공한다. +5. 응답 구조는 핵심 필드(`status`, `accepted`, `applied`, `summary`)를 기준으로 단순하게 유지한다. + +## 5.4 MCP 도구 설명 동기화 +파일: `src/main/java/cuspymd/mcp/mod/server/MCPProtocol.java` + +1. `execute_commands` description에 응답 구조 확장 항목을 반영. +2. `commands` description의 `per-command results` 문구를 상태 분리 기준으로 구체화. + +## 5.5 문서 갱신 +파일: `README.md`, 필요 시 `design.md` + +1. `execute_commands` 응답 예시를 확장 스키마 기준으로 교체. +2. `status`, `applied`, `summary` 중심의 파싱 방법을 문서화. +3. 전역 `chatMessages`와 명령별 `chatMessages`의 용도 차이를 문서화. + +## 6. 테스트 계획 + +### 6.1 단위 테스트 추가 +파일(신규): `src/test/java/cuspymd/mcp/mod/command/CommandExecutorResponseSchemaTest.java` + +1. 명령 성공 메시지 입력 시 `applied=true` 판정 검증. +2. 명령 실패 메시지 입력 시 `applied=false` 판정 검증. +3. `results[].chatMessages` 귀속 검증. +4. 집계 필드 `acceptedCount`, `appliedCount`, `failedCount` 계산 검증. + +구현 편의를 위해 메시지 판정/요약 로직은 별도 클래스로 분리 권장. +파일(신규): `src/client/java/cuspymd/mcp/mod/command/CommandOutcomeAnalyzer.java` + +### 6.2 기존 테스트 보강 +파일: `src/test/java/cuspymd/mcp/mod/server/MCPProtocolTest.java` + +1. `execute_commands` description에 신규 응답 안내 문구 포함 여부 검증. +2. 허용 명령 필터 테스트는 그대로 유지. + +## 7. 릴리즈 전략 +1. 1차 릴리즈: 신규 스키마(`status`, `accepted`, `applied`, `summary`, `chatMessages`)를 기본 응답으로 적용. +2. 2차 릴리즈: README와 릴리즈 노트에 신규 필드 기반 파싱 예시를 제공. +3. 3차 릴리즈: 메시지 판정 규칙 튜닝과 상태값 집합 안정화. + +## 8. 리스크와 대응 +1. 메시지 파싱 오판 가능성. +대응: `applied=null`, `status=unknown` 경로를 제공해 오탐보다 보수적으로 처리. + +2. 서버/클라이언트 메시지 로케일 차이. +대응: 문자열 규칙을 최소화하고, 규칙 기반 + 명령 종류별 보조 규칙으로 개선. + +3. 성능 저하. +대응: 명령별 수집 대기 시간을 제한하고 전체 타임아웃을 기존 값 이내로 유지. + +## 9. 작업 순서 제안 +1. `CommandOutcomeAnalyzer` 추가. +2. `CommandResult` 확장. +3. `CommandExecutor` 결과 수집/응답 조립 교체. +4. `MCPProtocol` 설명 갱신. +5. `README.md`/`design.md` 갱신. +6. 테스트 추가 및 회귀 확인. diff --git a/src/client/java/cuspymd/mcp/mod/command/ChatMessageCapture.java b/src/client/java/cuspymd/mcp/mod/command/ChatMessageCapture.java index a744f92..74bffc5 100644 --- a/src/client/java/cuspymd/mcp/mod/command/ChatMessageCapture.java +++ b/src/client/java/cuspymd/mcp/mod/command/ChatMessageCapture.java @@ -4,6 +4,8 @@ import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.TimeUnit; import java.util.function.Predicate; +import java.util.ArrayList; +import java.util.List; public class ChatMessageCapture { private static final ChatMessageCapture INSTANCE = new ChatMessageCapture(); @@ -43,4 +45,10 @@ public String waitForMessage(long timeoutMs, Predicate filter) throws In } return null; } -} \ No newline at end of file + + public List drainAvailableMessages() { + List drained = new ArrayList<>(); + messageQueue.drainTo(drained); + return drained; + } +} diff --git a/src/client/java/cuspymd/mcp/mod/command/CommandExecutor.java b/src/client/java/cuspymd/mcp/mod/command/CommandExecutor.java index 2f23d2c..c9a5c3e 100644 --- a/src/client/java/cuspymd/mcp/mod/command/CommandExecutor.java +++ b/src/client/java/cuspymd/mcp/mod/command/CommandExecutor.java @@ -1,6 +1,7 @@ package cuspymd.mcp.mod.command; import com.google.gson.JsonArray; +import com.google.gson.JsonNull; import com.google.gson.JsonObject; import cuspymd.mcp.mod.config.MCPConfig; import cuspymd.mcp.mod.server.MCPProtocol; @@ -12,10 +13,13 @@ import java.util.ArrayList; import java.util.List; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeoutException; import java.util.concurrent.TimeUnit; public class CommandExecutor { private static final Logger LOGGER = LoggerFactory.getLogger(CommandExecutor.class); + private static final long COMMAND_MESSAGE_WAIT_MS = 700L; + private static final long COMMAND_MESSAGE_IDLE_MS = 120L; private final MCPConfig config; private final SafetyValidator safetyValidator; @@ -41,16 +45,8 @@ public JsonObject executeCommands(JsonObject arguments) { String command = commands.get(i); SafetyValidator.ValidationResult validation = safetyValidator.validate(command); if (!validation.isValid()) { - JsonObject meta = new JsonObject(); - meta.addProperty("failed_command_index", i); - meta.addProperty("failed_command", command); - meta.addProperty("total_commands", commands.size()); - meta.addProperty("executed_commands", 0); - - return MCPProtocol.createErrorResponse( - "Command rejected by safety validator at command " + (i + 1) + ": " + validation.getErrorMessage(), - meta - ); + JsonObject responseJson = buildSafetyRejectedResponse(commands, i, validation.getErrorMessage()); + return MCPProtocol.createSuccessResponse(responseJson.toString()); } } } @@ -70,93 +66,23 @@ private JsonObject executeCommandsSequentially(List commands) { } List results = new ArrayList<>(); - List capturedMessages = new ArrayList<>(); + List allCapturedMessages = new ArrayList<>(); - // Start capturing chat messages for all commands ChatMessageCapture capture = ChatMessageCapture.getInstance(); capture.startCapturing(); try { - for (int i = 0; i < commands.size(); i++) { - String command = commands.get(i); - try { - CommandResult result = executeOneCommand(command).get(config.getServer().getRequestTimeoutMs(), TimeUnit.MILLISECONDS); - results.add(result); - - if (!result.isSuccess()) { - capture.stopCapturing(); - JsonObject meta = new JsonObject(); - meta.addProperty("failed_command_index", i); - meta.addProperty("failed_command", command); - meta.addProperty("total_commands", commands.size()); - meta.addProperty("executed_commands", i); - - return MCPProtocol.createErrorResponse( - "Command execution failed at command " + (i + 1) + ": " + result.getMessage(), - meta - ); - } - - } catch (Exception e) { - capture.stopCapturing(); - JsonObject meta = new JsonObject(); - meta.addProperty("failed_command_index", i); - meta.addProperty("failed_command", command); - meta.addProperty("total_commands", commands.size()); - meta.addProperty("executed_commands", i); - - return MCPProtocol.createErrorResponse( - "Command execution failed at command " + (i + 1) + ": " + e.getMessage(), - meta - ); - } - } - - // Wait for chat messages after all commands are executed - long totalWaitTime = Math.min(commands.size() * 200, 2000); // Max 2 seconds - long startTime = System.currentTimeMillis(); - - while (System.currentTimeMillis() - startTime < totalWaitTime) { - try { - String message = capture.waitForMessage(100); - if (message != null) { - capturedMessages.add(message); - } - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - break; - } - } - - capture.stopCapturing(); + capture.drainAvailableMessages(); + for (String command : commands) { + CommandResult executionResult = executeCommandWithTimeout(command); + List commandMessages = collectMessagesForCommand(capture); + allCapturedMessages.addAll(commandMessages); - // Build structured response with per-command results - JsonObject responseJson = new JsonObject(); - responseJson.addProperty("totalCommands", commands.size()); - responseJson.addProperty("successCount", results.size()); - - JsonArray commandResults = new JsonArray(); - for (int i = 0; i < results.size(); i++) { - JsonObject cmdResult = new JsonObject(); - cmdResult.addProperty("index", i); - cmdResult.addProperty("command", commands.get(i)); - cmdResult.addProperty("success", results.get(i).isSuccess()); - cmdResult.addProperty("message", results.get(i).getMessage()); - cmdResult.addProperty("executionTimeMs", results.get(i).getExecutionTimeMs()); - commandResults.add(cmdResult); + CommandResult analyzedResult = applyOutcomeAnalysis(executionResult, commandMessages); + results.add(analyzedResult); } - responseJson.add("results", commandResults); - - if (!capturedMessages.isEmpty()) { - JsonArray messages = new JsonArray(); - for (String message : capturedMessages) { - messages.add(message); - } - responseJson.add("chatMessages", messages); - } - - responseJson.addProperty("hint", "Use get_blocks_in_area to verify the built structure and fix any issues."); + JsonObject responseJson = buildExecuteCommandsResponse(commands.size(), results, allCapturedMessages); return MCPProtocol.createSuccessResponse(responseJson.toString()); } finally { @@ -173,8 +99,10 @@ private CompletableFuture executeOneCommand(String command) { if (player == null) { return CommandResult.builder() - .success(false) - .message("Player is not available") + .accepted(false) + .applied(false) + .status("execution_error") + .summary("Player is not available") .originalCommand(command) .executionTimeMs(System.currentTimeMillis() - startTime) .build(); @@ -186,34 +114,214 @@ private CompletableFuture executeOneCommand(String command) { if (config.getClient().isLogCommands()) { LOGGER.info("Executing command: {}", fullCommand); } - + + boolean hasNetworkHandler = client.getNetworkHandler() != null; client.execute(() -> { if (client.getNetworkHandler() != null) { client.getNetworkHandler().sendChatCommand(command); } }); + + if (!hasNetworkHandler) { + return CommandResult.builder() + .accepted(false) + .applied(false) + .status("execution_error") + .summary("Network handler is not available") + .originalCommand(command) + .executionTimeMs(System.currentTimeMillis() - startTime) + .build(); + } // Small delay to allow command to execute Thread.sleep(50); - - String resultMessage = "Command executed"; - boolean success = true; - + + return CommandResult.builder() + .accepted(true) + .applied(null) + .status("unknown") + .summary("Command sent") + .originalCommand(command) + .executionTimeMs(System.currentTimeMillis() - startTime) + .build(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); return CommandResult.builder() - .success(success) - .message(resultMessage) + .accepted(false) + .applied(false) + .status("execution_error") + .summary("Command execution interrupted") .originalCommand(command) .executionTimeMs(System.currentTimeMillis() - startTime) .build(); } catch (Exception e) { return CommandResult.builder() - .success(false) - .message("Failed to execute command: " + e.getMessage()) + .accepted(false) + .applied(false) + .status("execution_error") + .summary("Failed to execute command: " + e.getMessage()) .originalCommand(command) .executionTimeMs(System.currentTimeMillis() - startTime) .build(); } }); } -} \ No newline at end of file + + private CommandResult executeCommandWithTimeout(String command) { + try { + return executeOneCommand(command).get(config.getServer().getRequestTimeoutMs(), TimeUnit.MILLISECONDS); + } catch (TimeoutException e) { + return CommandResult.builder() + .accepted(false) + .applied(false) + .status("timed_out") + .summary("Command timed out") + .originalCommand(command) + .executionTimeMs(config.getServer().getRequestTimeoutMs()) + .build(); + } catch (Exception e) { + return CommandResult.builder() + .accepted(false) + .applied(false) + .status("execution_error") + .summary("Command failed before completion: " + e.getMessage()) + .originalCommand(command) + .executionTimeMs(0L) + .build(); + } + } + + private List collectMessagesForCommand(ChatMessageCapture capture) { + List messages = new ArrayList<>(); + long start = System.currentTimeMillis(); + long lastMessageAt = start; + + while (System.currentTimeMillis() - start < COMMAND_MESSAGE_WAIT_MS) { + try { + String message = capture.waitForMessage(75); + if (message != null) { + messages.add(message); + lastMessageAt = System.currentTimeMillis(); + continue; + } + + if (!messages.isEmpty() && System.currentTimeMillis() - lastMessageAt >= COMMAND_MESSAGE_IDLE_MS) { + break; + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + break; + } + } + + messages.addAll(capture.drainAvailableMessages()); + return messages; + } + + private CommandResult applyOutcomeAnalysis(CommandResult result, List chatMessages) { + if (!result.isAccepted()) { + return CommandResult.builder() + .accepted(false) + .applied(result.getApplied()) + .status(result.getStatus()) + .summary(result.getSummary()) + .chatMessages(chatMessages) + .originalCommand(result.getOriginalCommand()) + .executionTimeMs(result.getExecutionTimeMs()) + .build(); + } + + CommandOutcomeAnalyzer.Outcome outcome = + CommandOutcomeAnalyzer.analyze(true, chatMessages, result.getSummary()); + + return CommandResult.builder() + .accepted(outcome.accepted()) + .applied(outcome.applied()) + .status(outcome.status()) + .summary(outcome.summary()) + .chatMessages(chatMessages) + .originalCommand(result.getOriginalCommand()) + .executionTimeMs(result.getExecutionTimeMs()) + .build(); + } + + static JsonObject buildExecuteCommandsResponse(int totalCommands, List results, List capturedMessages) { + JsonObject responseJson = new JsonObject(); + int acceptedCount = 0; + int appliedCount = 0; + int failedCount = 0; + + JsonArray commandResults = new JsonArray(); + for (int i = 0; i < results.size(); i++) { + CommandResult result = results.get(i); + if (result.isAccepted()) { + acceptedCount++; + } + if (Boolean.TRUE.equals(result.getApplied())) { + appliedCount++; + } + if (Boolean.FALSE.equals(result.getApplied())) { + failedCount++; + } + + JsonObject cmdResult = new JsonObject(); + cmdResult.addProperty("index", i); + cmdResult.addProperty("command", result.getOriginalCommand()); + cmdResult.addProperty("status", result.getStatus()); + cmdResult.addProperty("accepted", result.isAccepted()); + if (result.getApplied() == null) { + cmdResult.add("applied", JsonNull.INSTANCE); + } else { + cmdResult.addProperty("applied", result.getApplied()); + } + cmdResult.addProperty("executionTimeMs", result.getExecutionTimeMs()); + cmdResult.addProperty("summary", result.getSummary()); + + JsonArray perCommandMessages = new JsonArray(); + for (String chatMessage : result.getChatMessages()) { + perCommandMessages.add(chatMessage); + } + cmdResult.add("chatMessages", perCommandMessages); + commandResults.add(cmdResult); + } + + responseJson.addProperty("totalCommands", totalCommands); + responseJson.addProperty("acceptedCount", acceptedCount); + responseJson.addProperty("appliedCount", appliedCount); + responseJson.addProperty("failedCount", failedCount); + responseJson.add("results", commandResults); + + JsonArray messages = new JsonArray(); + for (String message : capturedMessages) { + messages.add(message); + } + responseJson.add("chatMessages", messages); + + responseJson.addProperty("hint", "Use get_blocks_in_area to verify the built structure and fix any issues."); + return responseJson; + } + + static JsonObject buildSafetyRejectedResponse(List commands, int failedCommandIndex, String reason) { + List results = new ArrayList<>(); + String failedSummary = "Command rejected by safety validator: " + reason; + String skippedSummary = "Skipped because safety validation failed at command " + (failedCommandIndex + 1); + + for (int i = 0; i < commands.size(); i++) { + String summary = i == failedCommandIndex ? failedSummary : skippedSummary; + results.add(CommandResult.builder() + .accepted(false) + .applied(false) + .status("rejected_by_safety") + .summary(summary) + .chatMessages(List.of()) + .originalCommand(commands.get(i)) + .executionTimeMs(0L) + .build()); + } + + JsonObject responseJson = buildExecuteCommandsResponse(commands.size(), results, List.of()); + responseJson.addProperty("hint", "Adjust commands to satisfy safety validation, then retry."); + return responseJson; + } +} diff --git a/src/client/java/cuspymd/mcp/mod/command/CommandOutcomeAnalyzer.java b/src/client/java/cuspymd/mcp/mod/command/CommandOutcomeAnalyzer.java new file mode 100644 index 0000000..e4bce9d --- /dev/null +++ b/src/client/java/cuspymd/mcp/mod/command/CommandOutcomeAnalyzer.java @@ -0,0 +1,80 @@ +package cuspymd.mcp.mod.command; + +import java.util.List; +import java.util.Locale; +import java.util.Optional; + +public final class CommandOutcomeAnalyzer { + private static final List FAILURE_MARKERS = List.of( + "cannot", + "failed", + "unknown", + "no entity was found", + "is not holding any item", + "invalid", + "error" + ); + + private static final List SUCCESS_MARKERS = List.of( + "successfully", + "teleported", + "summoned", + "given", + "gave", + "set the weather", + "set the time", + "filled", + "set block", + "changed the block" + ); + + private CommandOutcomeAnalyzer() { + } + + public static Outcome analyze(boolean accepted, List chatMessages, String fallbackSummary) { + if (!accepted) { + String summary = fallbackSummary != null && !fallbackSummary.isBlank() + ? fallbackSummary + : "Command was not accepted for execution"; + return new Outcome(false, false, "execution_error", summary); + } + + Optional firstFailure = firstMatching(chatMessages, FAILURE_MARKERS); + if (firstFailure.isPresent()) { + return new Outcome(true, false, "rejected_by_game", firstFailure.get()); + } + + Optional firstSuccess = firstMatching(chatMessages, SUCCESS_MARKERS); + if (firstSuccess.isPresent()) { + return new Outcome(true, true, "applied", firstSuccess.get()); + } + + if (chatMessages != null && !chatMessages.isEmpty()) { + return new Outcome(true, null, "unknown", chatMessages.get(0)); + } + + return new Outcome(true, null, "unknown", "No command feedback captured"); + } + + private static Optional firstMatching(List messages, List markers) { + if (messages == null || messages.isEmpty()) { + return Optional.empty(); + } + + for (String message : messages) { + if (message == null) { + continue; + } + String normalized = message.toLowerCase(Locale.ROOT); + for (String marker : markers) { + if (normalized.contains(marker)) { + return Optional.of(message); + } + } + } + return Optional.empty(); + } + + public record Outcome(boolean accepted, Boolean applied, String status, String summary) { + } +} diff --git a/src/client/java/cuspymd/mcp/mod/command/CommandResult.java b/src/client/java/cuspymd/mcp/mod/command/CommandResult.java index 8e5b636..2d032fb 100644 --- a/src/client/java/cuspymd/mcp/mod/command/CommandResult.java +++ b/src/client/java/cuspymd/mcp/mod/command/CommandResult.java @@ -1,68 +1,80 @@ package cuspymd.mcp.mod.command; +import java.util.ArrayList; +import java.util.List; + public class CommandResult { - private final boolean success; - private final String message; + private final boolean accepted; + private final Boolean applied; + private final String status; + private final String summary; + private final List chatMessages; private final String originalCommand; private final long executionTimeMs; - private final int blocksAffected; - private final int entitiesAffected; private CommandResult(Builder builder) { - this.success = builder.success; - this.message = builder.message; + this.accepted = builder.accepted; + this.applied = builder.applied; + this.status = builder.status; + this.summary = builder.summary; + this.chatMessages = List.copyOf(builder.chatMessages); this.originalCommand = builder.originalCommand; this.executionTimeMs = builder.executionTimeMs; - this.blocksAffected = builder.blocksAffected; - this.entitiesAffected = builder.entitiesAffected; } - public boolean isSuccess() { return success; } - public String getMessage() { return message; } + public boolean isAccepted() { return accepted; } + public Boolean getApplied() { return applied; } + public String getStatus() { return status; } + public String getSummary() { return summary; } + public List getChatMessages() { return chatMessages; } public String getOriginalCommand() { return originalCommand; } public long getExecutionTimeMs() { return executionTimeMs; } - public int getBlocksAffected() { return blocksAffected; } - public int getEntitiesAffected() { return entitiesAffected; } public static Builder builder() { return new Builder(); } public static class Builder { - private boolean success; - private String message; + private boolean accepted; + private Boolean applied; + private String status = "unknown"; + private String summary = ""; + private List chatMessages = new ArrayList<>(); private String originalCommand; private long executionTimeMs; - private int blocksAffected = 0; - private int entitiesAffected = 0; - public Builder success(boolean success) { - this.success = success; + public Builder accepted(boolean accepted) { + this.accepted = accepted; return this; } - public Builder message(String message) { - this.message = message; + public Builder applied(Boolean applied) { + this.applied = applied; return this; } - - public Builder originalCommand(String originalCommand) { - this.originalCommand = originalCommand; + + public Builder status(String status) { + this.status = status; return this; } - - public Builder executionTimeMs(long executionTimeMs) { - this.executionTimeMs = executionTimeMs; + + public Builder summary(String summary) { + this.summary = summary; + return this; + } + + public Builder chatMessages(List chatMessages) { + this.chatMessages = chatMessages == null ? new ArrayList<>() : new ArrayList<>(chatMessages); return this; } - public Builder blocksAffected(int blocksAffected) { - this.blocksAffected = blocksAffected; + public Builder originalCommand(String originalCommand) { + this.originalCommand = originalCommand; return this; } - public Builder entitiesAffected(int entitiesAffected) { - this.entitiesAffected = entitiesAffected; + public Builder executionTimeMs(long executionTimeMs) { + this.executionTimeMs = executionTimeMs; return this; } @@ -70,4 +82,4 @@ public CommandResult build() { return new CommandResult(this); } } -} \ No newline at end of file +} diff --git a/src/main/java/cuspymd/mcp/mod/server/MCPProtocol.java b/src/main/java/cuspymd/mcp/mod/server/MCPProtocol.java index bca83c9..b7ed987 100644 --- a/src/main/java/cuspymd/mcp/mod/server/MCPProtocol.java +++ b/src/main/java/cuspymd/mcp/mod/server/MCPProtocol.java @@ -22,6 +22,10 @@ public static JsonArray getToolsListResponse(MCPConfig config) { executeCommandsTool.addProperty("description", "Execute one or more Minecraft commands sequentially. " + "Allowed commands: " + allowedCommandsText + ".\n\n" + + "Response schema highlights:\n" + + "- top-level: totalCommands, acceptedCount, appliedCount, failedCount\n" + + "- per command: status, accepted, applied, summary, chatMessages\n" + + "- status values: applied, rejected_by_game, execution_error, timed_out, rejected_by_safety, unknown\n\n" + "BLOCK STATE SYNTAX (critical for quality builds):\n" + "- Doors: setblock X Y Z oak_door[facing=north,half=lower,hinge=left,open=false] then setblock X Y+1 Z oak_door[facing=north,half=upper,hinge=left,open=false]\n" + "- Stairs: setblock X Y Z oak_stairs[facing=east,half=bottom,shape=straight]\n" + @@ -60,7 +64,7 @@ public static JsonArray getToolsListResponse(MCPConfig config) { JsonObject commandsProperty = new JsonObject(); commandsProperty.addProperty("type", "array"); - commandsProperty.addProperty("description", "Array of Minecraft commands to execute without leading slash. Each command is executed sequentially. The response includes per-command results showing success/failure and any chat feedback from the server."); + commandsProperty.addProperty("description", "Array of Minecraft commands to execute without leading slash. Each command is executed sequentially. Per-command results include status, accepted/applied booleans, summary, and command-scoped chat messages."); commandsProperty.addProperty("minItems", 1); JsonObject commandsItems = new JsonObject(); diff --git a/src/test/java/cuspymd/mcp/mod/command/ChatMessageCaptureTest.java b/src/test/java/cuspymd/mcp/mod/command/ChatMessageCaptureTest.java new file mode 100644 index 0000000..235e029 --- /dev/null +++ b/src/test/java/cuspymd/mcp/mod/command/ChatMessageCaptureTest.java @@ -0,0 +1,61 @@ +package cuspymd.mcp.mod.command; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +public class ChatMessageCaptureTest { + private final ChatMessageCapture capture = ChatMessageCapture.getInstance(); + + @BeforeEach + public void resetCaptureState() { + capture.stopCapturing(); + capture.startCapturing(); + capture.drainAvailableMessages(); + capture.stopCapturing(); + } + + @Test + public void captureMessageIsIgnoredWhenNotCapturing() throws Exception { + capture.captureMessage("ignored"); + + assertNull(capture.waitForMessage(30)); + } + + @Test + public void startCapturingEnablesQueueAndClearPreviousMessages() { + capture.startCapturing(); + capture.captureMessage("first"); + assertEquals(1, capture.drainAvailableMessages().size()); + + capture.startCapturing(); // should clear queue + assertEquals(0, capture.drainAvailableMessages().size()); + } + + @Test + public void waitForMessageWithFilterReturnsMatchingMessage() throws Exception { + capture.startCapturing(); + capture.captureMessage("alpha"); + capture.captureMessage("beta"); + + String result = capture.waitForMessage(150, m -> m.contains("bet")); + + assertEquals("beta", result); + } + + @Test + public void drainAvailableMessagesReturnsAllCapturedMessages() { + capture.startCapturing(); + capture.captureMessage("one"); + capture.captureMessage("two"); + + List drained = capture.drainAvailableMessages(); + + assertEquals(List.of("one", "two"), drained); + assertEquals(0, capture.drainAvailableMessages().size()); + } +} diff --git a/src/test/java/cuspymd/mcp/mod/command/CommandExecutorResponseSchemaTest.java b/src/test/java/cuspymd/mcp/mod/command/CommandExecutorResponseSchemaTest.java new file mode 100644 index 0000000..08b1f66 --- /dev/null +++ b/src/test/java/cuspymd/mcp/mod/command/CommandExecutorResponseSchemaTest.java @@ -0,0 +1,89 @@ +package cuspymd.mcp.mod.command; + +import com.google.gson.JsonObject; +import org.junit.jupiter.api.Test; + +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class CommandExecutorResponseSchemaTest { + + @Test + public void responseIncludesCountsAndPerCommandMessages() { + CommandResult applied = CommandResult.builder() + .accepted(true) + .applied(true) + .status("applied") + .summary("Filled 4 blocks") + .chatMessages(List.of("Successfully filled 4 block(s)")) + .originalCommand("fill 0 0 0 1 0 1 stone") + .executionTimeMs(50) + .build(); + + CommandResult rejected = CommandResult.builder() + .accepted(true) + .applied(false) + .status("rejected_by_game") + .summary("Cannot enchant") + .chatMessages(List.of("Carrot cannot support that enchantment")) + .originalCommand("enchant @s minecraft:unbreaking 1") + .executionTimeMs(52) + .build(); + + JsonObject payload = CommandExecutor.buildExecuteCommandsResponse( + 2, + List.of(applied, rejected), + List.of("Successfully filled 4 block(s)", "Carrot cannot support that enchantment") + ); + + assertEquals(2, payload.get("totalCommands").getAsInt()); + assertEquals(2, payload.get("acceptedCount").getAsInt()); + assertEquals(1, payload.get("appliedCount").getAsInt()); + assertEquals(1, payload.get("failedCount").getAsInt()); + + JsonObject first = payload.getAsJsonArray("results").get(0).getAsJsonObject(); + assertEquals("applied", first.get("status").getAsString()); + assertTrue(first.get("accepted").getAsBoolean()); + assertTrue(first.get("applied").getAsBoolean()); + + JsonObject second = payload.getAsJsonArray("results").get(1).getAsJsonObject(); + assertEquals("rejected_by_game", second.get("status").getAsString()); + assertTrue(second.get("accepted").getAsBoolean()); + assertFalse(second.get("applied").getAsBoolean()); + assertEquals(1, second.getAsJsonArray("chatMessages").size()); + } + + @Test + public void safetyRejectedResponseUsesPerCommandSchema() { + JsonObject payload = CommandExecutor.buildSafetyRejectedResponse( + List.of("say ok", "kill @s", "give @s dirt 1"), + 1, + "Command 'kill' is not allowed" + ); + + assertEquals(3, payload.get("totalCommands").getAsInt()); + assertEquals(0, payload.get("acceptedCount").getAsInt()); + assertEquals(0, payload.get("appliedCount").getAsInt()); + assertEquals(3, payload.get("failedCount").getAsInt()); + + JsonObject first = payload.getAsJsonArray("results").get(0).getAsJsonObject(); + assertEquals("rejected_by_safety", first.get("status").getAsString()); + assertFalse(first.get("accepted").getAsBoolean()); + assertFalse(first.get("applied").getAsBoolean()); + + JsonObject second = payload.getAsJsonArray("results").get(1).getAsJsonObject(); + assertEquals("rejected_by_safety", second.get("status").getAsString()); + assertTrue(second.get("summary").getAsString().contains("Command rejected by safety validator")); + assertTrue(second.get("summary").getAsString().contains("kill")); + + JsonObject third = payload.getAsJsonArray("results").get(2).getAsJsonObject(); + assertEquals("rejected_by_safety", third.get("status").getAsString()); + assertTrue(third.get("summary").getAsString().contains("Skipped because safety validation failed at command 2")); + + assertNotNull(payload.get("hint")); + } +} diff --git a/src/test/java/cuspymd/mcp/mod/command/CommandOutcomeAnalyzerTest.java b/src/test/java/cuspymd/mcp/mod/command/CommandOutcomeAnalyzerTest.java new file mode 100644 index 0000000..23fdaf7 --- /dev/null +++ b/src/test/java/cuspymd/mcp/mod/command/CommandOutcomeAnalyzerTest.java @@ -0,0 +1,109 @@ +package cuspymd.mcp.mod.command; + +import org.junit.jupiter.api.Test; + +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class CommandOutcomeAnalyzerTest { + + @Test + public void successMessageIsClassifiedAsApplied() { + CommandOutcomeAnalyzer.Outcome outcome = CommandOutcomeAnalyzer.analyze( + true, + List.of("Successfully filled 4 block(s)"), + "fallback" + ); + + assertTrue(outcome.accepted()); + assertTrue(outcome.applied()); + assertEquals("applied", outcome.status()); + assertEquals("Successfully filled 4 block(s)", outcome.summary()); + } + + @Test + public void failureMessageIsClassifiedAsRejectedByGame() { + CommandOutcomeAnalyzer.Outcome outcome = CommandOutcomeAnalyzer.analyze( + true, + List.of("Carrot cannot support that enchantment"), + "fallback" + ); + + assertTrue(outcome.accepted()); + assertFalse(outcome.applied()); + assertEquals("rejected_by_game", outcome.status()); + } + + @Test + public void noMessageIsClassifiedAsUnknown() { + CommandOutcomeAnalyzer.Outcome outcome = CommandOutcomeAnalyzer.analyze( + true, + List.of(), + "fallback" + ); + + assertTrue(outcome.accepted()); + assertNull(outcome.applied()); + assertEquals("unknown", outcome.status()); + } + + @Test + public void holdingItemFailureIsClassifiedAsRejectedByGame() { + CommandOutcomeAnalyzer.Outcome outcome = CommandOutcomeAnalyzer.analyze( + true, + List.of("Player691 is not holding any item"), + "fallback" + ); + + assertTrue(outcome.accepted()); + assertFalse(outcome.applied()); + assertEquals("rejected_by_game", outcome.status()); + assertEquals("Player691 is not holding any item", outcome.summary()); + } + + @Test + public void gaveMessageIsClassifiedAsApplied() { + CommandOutcomeAnalyzer.Outcome outcome = CommandOutcomeAnalyzer.analyze( + true, + List.of("Gave 1 [Dirt] to Player691"), + "fallback" + ); + + assertTrue(outcome.accepted()); + assertTrue(outcome.applied()); + assertEquals("applied", outcome.status()); + assertEquals("Gave 1 [Dirt] to Player691", outcome.summary()); + } + + @Test + public void changedBlockMessageIsClassifiedAsApplied() { + CommandOutcomeAnalyzer.Outcome outcome = CommandOutcomeAnalyzer.analyze( + true, + List.of("Changed the block at -24, 72, -29"), + "fallback" + ); + + assertTrue(outcome.accepted()); + assertTrue(outcome.applied()); + assertEquals("applied", outcome.status()); + assertEquals("Changed the block at -24, 72, -29", outcome.summary()); + } + + @Test + public void setTimeMessageIsClassifiedAsApplied() { + CommandOutcomeAnalyzer.Outcome outcome = CommandOutcomeAnalyzer.analyze( + true, + List.of("Set the time to 1000"), + "fallback" + ); + + assertTrue(outcome.accepted()); + assertTrue(outcome.applied()); + assertEquals("applied", outcome.status()); + assertEquals("Set the time to 1000", outcome.summary()); + } +} diff --git a/src/test/java/cuspymd/mcp/mod/server/MCPProtocolTest.java b/src/test/java/cuspymd/mcp/mod/server/MCPProtocolTest.java index 83af965..ca96f49 100644 --- a/src/test/java/cuspymd/mcp/mod/server/MCPProtocolTest.java +++ b/src/test/java/cuspymd/mcp/mod/server/MCPProtocolTest.java @@ -30,5 +30,7 @@ public void executeCommandsDescriptionUsesFilteredAllowList() { assertTrue(allowedLine.contains("Allowed commands: tp, fill.")); assertFalse(allowedLine.contains(" op")); assertFalse(allowedLine.contains("reload")); + assertTrue(description.contains("acceptedCount")); + assertTrue(description.contains("status values: applied, rejected_by_game, execution_error, timed_out, rejected_by_safety, unknown")); } } From a50cacf80abd31b3b984638e75a38b5bf83577b4 Mon Sep 17 00:00:00 2001 From: Myoungdo Park Date: Fri, 20 Feb 2026 01:12:42 +0900 Subject: [PATCH 2/2] Fix MCPProtocol compile errors after merge --- .../cuspymd/mcp/mod/server/MCPProtocol.java | 41 ++++++++++++++++++- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/src/main/java/cuspymd/mcp/mod/server/MCPProtocol.java b/src/main/java/cuspymd/mcp/mod/server/MCPProtocol.java index b7ed987..a956fc3 100644 --- a/src/main/java/cuspymd/mcp/mod/server/MCPProtocol.java +++ b/src/main/java/cuspymd/mcp/mod/server/MCPProtocol.java @@ -3,17 +3,22 @@ import com.google.gson.JsonArray; import com.google.gson.JsonObject; import cuspymd.mcp.mod.config.MCPConfig; -import cuspymd.mcp.mod.safety.CommandSafetyPolicy; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; public class MCPProtocol { + private static final Set DESCRIBABLE_COMMANDS = Set.of( + "fill", "clone", "setblock", "summon", "tp", "give", "gamemode", + "effect", "enchant", "weather", "time", "say", "tell", "title" + ); public static JsonArray getToolsListResponse(MCPConfig config) { JsonArray tools = new JsonArray(); List configuredAllowedCommands = (config != null && config.getServer() != null && config.getServer().getAllowedCommands() != null) ? config.getServer().getAllowedCommands() : List.of("fill", "clone", "setblock", "summon", "tp", "give", "gamemode", "effect", "enchant", "weather", "time", "say", "tell", "title"); - List allowedCommands = CommandSafetyPolicy.filterAllowedCommands(configuredAllowedCommands); + List allowedCommands = filterAllowedCommandsForDescription(configuredAllowedCommands); String allowedCommandsText = allowedCommands.isEmpty() ? "(none configured)" : String.join(", ", allowedCommands); // Execute commands tool @@ -216,4 +221,36 @@ public static JsonObject createErrorResponse(String message, JsonObject meta) { return response; } + + public static JsonObject createImageResponse(String base64Data, String mimeType) { + JsonObject response = new JsonObject(); + response.addProperty("isError", false); + + JsonArray content = new JsonArray(); + JsonObject imageContent = new JsonObject(); + imageContent.addProperty("type", "image"); + imageContent.addProperty("data", base64Data); + imageContent.addProperty("mimeType", mimeType); + content.add(imageContent); + + response.add("content", content); + return response; + } + + private static List filterAllowedCommandsForDescription(List configuredAllowedCommands) { + LinkedHashSet filtered = new LinkedHashSet<>(); + for (String command : configuredAllowedCommands) { + if (command == null) { + continue; + } + String normalized = command.trim().toLowerCase(); + if (normalized.startsWith("/")) { + normalized = normalized.substring(1); + } + if (!normalized.isEmpty() && DESCRIBABLE_COMMANDS.contains(normalized)) { + filtered.add(normalized); + } + } + return List.copyOf(filtered); + } }