diff --git a/editor/CMakeLists.txt b/editor/CMakeLists.txt index 3a03cc4..08bd7be 100644 --- a/editor/CMakeLists.txt +++ b/editor/CMakeLists.txt @@ -2659,4 +2659,13 @@ target_link_libraries(step419_test PRIVATE tree_sitter_javascript tree_sitter_typescript tree_sitter_java tree_sitter_rust tree_sitter_go) +add_executable(step420_test tests/step420_test.cpp) +target_include_directories(step420_test PRIVATE src) +target_link_libraries(step420_test PRIVATE + nlohmann_json::nlohmann_json + unofficial::tree-sitter::tree-sitter + tree_sitter_python tree_sitter_cpp tree_sitter_elisp + tree_sitter_javascript tree_sitter_typescript + tree_sitter_java tree_sitter_rust tree_sitter_go) + # Step 12: Dear ImGui shell scaffolding created (main.cpp exists but not built due to dependencies) diff --git a/editor/src/AgentPermissionPolicy.h b/editor/src/AgentPermissionPolicy.h index bc921f8..facaf9f 100644 --- a/editor/src/AgentPermissionPolicy.h +++ b/editor/src/AgentPermissionPolicy.h @@ -71,6 +71,8 @@ struct AgentPermissionPolicy { method == "getWorkItem" || method == "getRoutingExplanation" || method == "getReviewPolicy" || + method == "getReviewQueue" || + method == "getReviewContext" || method == "getBlockers" || method == "getProgress" || method == "getEventStream" || @@ -108,6 +110,8 @@ struct AgentPermissionPolicy { method == "routeAllReady" || method == "executeTask" || method == "setReviewPolicy" || + method == "approveReviewItem" || + method == "rejectReviewItem" || method == "orchestrateStep" || method == "orchestrateAdvance" || method == "orchestrateRunDeterministic" || diff --git a/editor/src/HeadlessAgentRPCHandler.h b/editor/src/HeadlessAgentRPCHandler.h index 289cc8d..3292c28 100644 --- a/editor/src/HeadlessAgentRPCHandler.h +++ b/editor/src/HeadlessAgentRPCHandler.h @@ -2602,6 +2602,145 @@ inline json handleHeadlessAgentRequest(HeadlessEditorState& state, }); } + // --- getReviewQueue --- + if (method == "getReviewQueue") { + if (!AgentPermissionPolicy::canInvoke(role, method)) + return headlessRpcError(id, -32031, "Role not permitted"); + if (!state.workflow) + return headlessRpcError(id, -32000, "No active workflow"); + + auto reviewItems = state.workflow->queue.getByStatus(WI_REVIEW); + json arr = json::array(); + for (const auto& wi : reviewItems) { + arr.push_back({ + {"itemId", wi.id}, + {"nodeName", wi.nodeName}, + {"nodeType", wi.nodeType}, + {"bufferId", wi.bufferId}, + {"workerType", wi.workerType}, + {"priority", wi.priority}, + {"confidence", wi.result.confidence}, + {"reviewRequired", wi.reviewRequired}, + {"status", wi.status} + }); + } + + return headlessRpcResult(id, { + {"items", arr}, + {"count", (int)arr.size()} + }); + } + + // --- getReviewContext --- + if (method == "getReviewContext") { + if (!AgentPermissionPolicy::canInvoke(role, method)) + return headlessRpcError(id, -32031, "Role not permitted"); + if (!state.workflow) + return headlessRpcError(id, -32000, "No active workflow"); + + auto params = request.contains("params") ? request["params"] : json::object(); + std::string itemId = params.value("itemId", ""); + if (itemId.empty()) + return headlessRpcError(id, -32602, "Missing itemId"); + + auto item = state.workflow->queue.getItem(itemId); + if (!item) + return headlessRpcError(id, -32602, "Work item not found"); + + std::string summary; + summary += "Review Item: " + item->id + "\n"; + summary += "Node: " + item->nodeName + " (" + item->nodeType + ")\n"; + summary += "Worker: " + item->workerType + ", Priority: " + item->priority + "\n"; + summary += "Status: " + item->status + ", ReviewRequired: "; + summary += item->reviewRequired ? "true\n" : "false\n"; + summary += "Confidence: " + std::to_string(item->result.confidence) + "\n"; + summary += "Reasoning: " + item->result.reasoning + "\n"; + summary += "Generated Code:\n" + item->result.generatedCode + "\n"; + if (!item->rejectionFeedback.empty()) { + summary += "Previous Feedback: " + item->rejectionFeedback + "\n"; + } + + return headlessRpcResult(id, { + {"item", workItemToJson(*item)}, + {"generatedCode", item->result.generatedCode}, + {"confidence", item->result.confidence}, + {"reasoning", item->result.reasoning}, + {"dependencies", item->dependencies}, + {"humanSummary", summary} + }); + } + + // --- approveReviewItem --- + if (method == "approveReviewItem") { + if (!AgentPermissionPolicy::canInvoke(role, method)) + return headlessRpcError(id, -32031, "Role not permitted"); + if (!state.workflow) + return headlessRpcError(id, -32000, "No active workflow"); + + auto params = request.contains("params") ? request["params"] : json::object(); + std::string itemId = params.value("itemId", ""); + std::string feedback = params.value("feedback", ""); + if (itemId.empty()) + return headlessRpcError(id, -32602, "Missing itemId"); + + auto item = state.workflow->queue.getItem(itemId); + if (!item) + return headlessRpcError(id, -32602, "Work item not found"); + if (item->status != WI_REVIEW) + return headlessRpcError(id, -32000, + "Cannot approve item in status: " + item->status); + + WorkItem updated = *item; + bool ok = transitionWorkItem(updated, WI_COMPLETE); + if (!ok) + return headlessRpcError(id, -32000, "Approve transition failed"); + if (!feedback.empty()) updated.result.reasoning += "\nReviewer Note: " + feedback; + + state.workflow->queue.updateItem(itemId, updated); + state.workflow->recordChange(itemId, WI_REVIEW, WI_COMPLETE, + "human:" + sessionId, feedback); + + return headlessRpcResult(id, { + {"success", true}, + {"item", workItemToJson(updated)} + }); + } + + // --- rejectReviewItem --- + if (method == "rejectReviewItem") { + if (!AgentPermissionPolicy::canInvoke(role, method)) + return headlessRpcError(id, -32031, "Role not permitted"); + if (!state.workflow) + return headlessRpcError(id, -32000, "No active workflow"); + + auto params = request.contains("params") ? request["params"] : json::object(); + std::string itemId = params.value("itemId", ""); + std::string feedback = params.value("feedback", ""); + if (itemId.empty()) + return headlessRpcError(id, -32602, "Missing itemId"); + if (feedback.empty()) + return headlessRpcError(id, -32602, "Missing feedback"); + + auto item = state.workflow->queue.getItem(itemId); + if (!item) + return headlessRpcError(id, -32602, "Work item not found"); + if (item->status != WI_REVIEW) + return headlessRpcError(id, -32000, + "Cannot reject item in status: " + item->status); + + bool ok = state.workflow->queue.reject(itemId, feedback); + if (!ok) + return headlessRpcError(id, -32000, "Reject failed"); + state.workflow->recordChange(itemId, WI_REVIEW, WI_READY, + "human:" + sessionId, feedback); + + auto updated = state.workflow->queue.getItem(itemId); + return headlessRpcResult(id, { + {"success", true}, + {"item", updated ? workItemToJson(*updated) : json::object()} + }); + } + // --- setReviewPolicy --- if (method == "setReviewPolicy") { if (!AgentPermissionPolicy::canInvoke(role, method)) diff --git a/editor/src/MCPServer.h b/editor/src/MCPServer.h index 7368bb8..710e626 100644 --- a/editor/src/MCPServer.h +++ b/editor/src/MCPServer.h @@ -1630,6 +1630,62 @@ private: } void registerReviewTools() { + // whetstone_get_review_queue + tools_.push_back({"whetstone_get_review_queue", + "Get items currently awaiting human review with concise queue metadata.", + {{"type", "object"}, {"properties", json::object()}} + }); + toolHandlers_["whetstone_get_review_queue"] = + [this](const json& args) { + return callWhetstone("getReviewQueue", args); + }; + + // whetstone_get_review_context + tools_.push_back({"whetstone_get_review_context", + "Get full review context for one item: skeleton metadata, generated code, " + "confidence, reasoning, and a human-readable summary.", + {{"type", "object"}, {"properties", { + {"itemId", {{"type", "string"}, + {"description", "Review item ID"}}} + }}, {"required", json::array({"itemId"})}} + }); + toolHandlers_["whetstone_get_review_context"] = + [this](const json& args) { + return callWhetstone("getReviewContext", args); + }; + + // whetstone_approve_item + tools_.push_back({"whetstone_approve_item", + "Approve a review item and mark it complete. Optional feedback is stored " + "as reviewer note.", + {{"type", "object"}, {"properties", { + {"itemId", {{"type", "string"}, + {"description", "Review item ID"}}}, + {"feedback", {{"type", "string"}, + {"description", "Optional reviewer note"}}} + }}, {"required", json::array({"itemId"})}} + }); + toolHandlers_["whetstone_approve_item"] = + [this](const json& args) { + return callWhetstone("approveReviewItem", args); + }; + + // whetstone_reject_item + tools_.push_back({"whetstone_reject_item", + "Reject a review item back to ready state. Feedback is required to explain " + "what must change before re-review.", + {{"type", "object"}, {"properties", { + {"itemId", {{"type", "string"}, + {"description", "Review item ID"}}}, + {"feedback", {{"type", "string"}, + {"description", "Required rejection feedback"}}} + }}, {"required", json::array({"itemId", "feedback"})}} + }); + toolHandlers_["whetstone_reject_item"] = + [this](const json& args) { + return callWhetstone("rejectReviewItem", args); + }; + // whetstone_set_review_policy tools_.push_back({"whetstone_set_review_policy", "Configure auto-approve rules for the review gate. Rules specify " diff --git a/editor/tests/step420_test.cpp b/editor/tests/step420_test.cpp new file mode 100644 index 0000000..d75220a --- /dev/null +++ b/editor/tests/step420_test.cpp @@ -0,0 +1,204 @@ +// Step 420: Human Review Interface via MCP Tests (12 tests) + +#include "MCPServer.h" +#include "HeadlessEditorState.h" + +#include +#include + +static int passed = 0, failed = 0; +#define TEST(name) { std::cout << " " << #name << "... "; } +#define PASS() { std::cout << "PASS\n"; ++passed; } +#define FAIL(msg) { std::cout << "FAIL: " << msg << "\n"; ++failed; } +#define CHECK(cond, msg) if (!(cond)) { FAIL(msg); return; } else {} + +static HeadlessEditorState makeStateWithReviewItem(const std::string& status = WI_REVIEW) { + HeadlessEditorState state; + state.setAgentRole("sess", AgentRole::Refactor); + state.workflow = WorkflowState("review-proj"); + + WorkItem wi; + wi.id = "w1"; + wi.nodeId = "node1"; + wi.nodeName = "processOrder"; + wi.nodeType = "Function"; + wi.bufferId = "orders.py"; + wi.workerType = "llm"; + wi.priority = "high"; + wi.reviewRequired = true; + wi.status = status; + wi.result.generatedCode = "def process_order(x):\n return x\n"; + wi.result.confidence = 0.77f; + wi.result.reasoning = "Generated from blocker context."; + state.workflow->queue.enqueue(wi); + return state; +} + +static json call(HeadlessEditorState& state, const std::string& method, const json& params = json::object()) { + return state.processAgentRequest({ + {"jsonrpc", "2.0"}, + {"id", 1}, + {"method", method}, + {"params", params} + }, "sess"); +} + +void test_mcp_review_tools_registered() { + TEST(mcp_review_tools_registered); + MCPServer server; + bool q = false, c = false, a = false, r = false; + for (const auto& t : server.getTools()) { + if (t.name == "whetstone_get_review_queue") q = true; + if (t.name == "whetstone_get_review_context") c = true; + if (t.name == "whetstone_approve_item") a = true; + if (t.name == "whetstone_reject_item") r = true; + } + CHECK(q && c && a && r, "expected all review MCP tools"); + PASS(); +} + +void test_mcp_tool_mapping_get_review_queue() { + TEST(mcp_tool_mapping_get_review_queue); + MCPServer server; + std::string calledMethod; + server.setRpcCallback([&](const json& req) { + calledMethod = req.value("method", ""); + return json{{"jsonrpc", "2.0"}, {"id", 1}, {"result", {{"items", json::array()}, {"count", 0}}}}; + }); + auto resp = server.handleRequest({ + {"jsonrpc", "2.0"}, {"id", 1}, {"method", "tools/call"}, + {"params", {{"name", "whetstone_get_review_queue"}, {"arguments", json::object()}}} + }); + CHECK(resp.contains("result"), "missing result"); + CHECK(calledMethod == "getReviewQueue", "wrong backend method mapping"); + PASS(); +} + +void test_get_review_queue_returns_items() { + TEST(get_review_queue_returns_items); + auto state = makeStateWithReviewItem(); + auto resp = call(state, "getReviewQueue"); + CHECK(resp.contains("result"), "expected result"); + auto result = resp["result"]; + CHECK(result.value("count", 0) == 1, "expected one review item"); + CHECK(result["items"][0].value("itemId", "") == "w1", "wrong item id"); + PASS(); +} + +void test_get_review_queue_empty_when_no_review_items() { + TEST(get_review_queue_empty_when_no_review_items); + auto state = makeStateWithReviewItem(WI_READY); + auto resp = call(state, "getReviewQueue"); + CHECK(resp.contains("result"), "expected result"); + CHECK(resp["result"].value("count", -1) == 0, "expected empty review queue"); + PASS(); +} + +void test_get_review_context_contains_human_summary() { + TEST(get_review_context_contains_human_summary); + auto state = makeStateWithReviewItem(); + auto resp = call(state, "getReviewContext", {{"itemId", "w1"}}); + CHECK(resp.contains("result"), "expected result"); + std::string summary = resp["result"].value("humanSummary", ""); + CHECK(summary.find("Review Item: w1") != std::string::npos, "missing review header"); + CHECK(summary.find("Generated Code:") != std::string::npos, "missing generated code section"); + PASS(); +} + +void test_get_review_context_missing_item_errors() { + TEST(get_review_context_missing_item_errors); + auto state = makeStateWithReviewItem(); + auto resp = call(state, "getReviewContext", {{"itemId", "missing"}}); + CHECK(resp.contains("error"), "expected error"); + PASS(); +} + +void test_approve_item_marks_complete() { + TEST(approve_item_marks_complete); + auto state = makeStateWithReviewItem(); + auto resp = call(state, "approveReviewItem", {{"itemId", "w1"}, {"feedback", "looks good"}}); + CHECK(resp.contains("result"), "expected result"); + auto item = state.workflow->queue.getItem("w1"); + CHECK(item.has_value(), "missing item after approval"); + CHECK(item->status == WI_COMPLETE, "expected complete status"); + PASS(); +} + +void test_approve_item_wrong_status_errors() { + TEST(approve_item_wrong_status_errors); + auto state = makeStateWithReviewItem(WI_READY); + auto resp = call(state, "approveReviewItem", {{"itemId", "w1"}}); + CHECK(resp.contains("error"), "expected error for wrong status"); + PASS(); +} + +void test_reject_item_requires_feedback() { + TEST(reject_item_requires_feedback); + auto state = makeStateWithReviewItem(); + auto resp = call(state, "rejectReviewItem", {{"itemId", "w1"}}); + CHECK(resp.contains("error"), "expected missing feedback error"); + PASS(); +} + +void test_reject_item_moves_back_to_ready() { + TEST(reject_item_moves_back_to_ready); + auto state = makeStateWithReviewItem(); + auto resp = call(state, "rejectReviewItem", {{"itemId", "w1"}, {"feedback", "rename variable"}}); + CHECK(resp.contains("result"), "expected result"); + auto item = state.workflow->queue.getItem("w1"); + CHECK(item.has_value(), "missing item"); + CHECK(item->status == WI_READY, "expected item re-queued to ready"); + PASS(); +} + +void test_reject_item_wrong_status_errors() { + TEST(reject_item_wrong_status_errors); + auto state = makeStateWithReviewItem(WI_READY); + auto resp = call(state, "rejectReviewItem", {{"itemId", "w1"}, {"feedback", "nope"}}); + CHECK(resp.contains("error"), "expected error for wrong status"); + PASS(); +} + +void test_mcp_end_to_end_review_tools() { + TEST(mcp_end_to_end_review_tools); + auto state = makeStateWithReviewItem(); + MCPServer server; + server.setRpcCallback([&](const json& req) { return state.processAgentRequest(req, "sess"); }); + + auto queueResp = server.handleRequest({ + {"jsonrpc", "2.0"}, {"id", 1}, {"method", "tools/call"}, + {"params", {{"name", "whetstone_get_review_queue"}, {"arguments", json::object()}}} + }); + CHECK(queueResp.contains("result"), "missing queue result"); + + auto approveResp = server.handleRequest({ + {"jsonrpc", "2.0"}, {"id", 2}, {"method", "tools/call"}, + {"params", {{"name", "whetstone_approve_item"}, + {"arguments", {{"itemId", "w1"}, {"feedback", "approved"}}}}} + }); + CHECK(approveResp.contains("result"), "missing approve result"); + auto item = state.workflow->queue.getItem("w1"); + CHECK(item.has_value() && item->status == WI_COMPLETE, "approve should complete item"); + PASS(); +} + +int main() { + std::cout << "Step 420: Human Review Interface via MCP Tests\n"; + + test_mcp_review_tools_registered(); // 1 + test_mcp_tool_mapping_get_review_queue(); // 2 + test_get_review_queue_returns_items(); // 3 + test_get_review_queue_empty_when_no_review_items();// 4 + test_get_review_context_contains_human_summary(); // 5 + test_get_review_context_missing_item_errors(); // 6 + test_approve_item_marks_complete(); // 7 + test_approve_item_wrong_status_errors(); // 8 + test_reject_item_requires_feedback(); // 9 + test_reject_item_moves_back_to_ready(); // 10 + test_reject_item_wrong_status_errors(); // 11 + test_mcp_end_to_end_review_tools(); // 12 + + std::cout << "\nResults: " << passed << "/" << (passed + failed) + << " passed\n"; + return failed == 0 ? 0 : 1; +} diff --git a/progress.md b/progress.md index 40f6181..2e79d1c 100644 --- a/progress.md +++ b/progress.md @@ -4373,6 +4373,57 @@ blocker items. - `editor/src/MCPServer.h` (`1679` > `600`) - `editor/src/HeadlessAgentRPCHandler.h` (`2629` > `600`) +### Step 420: Human Review Interface via MCP +**Status:** PASS (12/12 tests) + +Added human-review MCP capabilities so review-required items can be listed, +inspected with readable context, and explicitly approved/rejected through the +headless JSON-RPC path. + +**Files created:** +- `editor/tests/step420_test.cpp` — 12 tests covering: + 1. MCP review tools are registered + 2. tool-to-RPC mapping for `whetstone_get_review_queue` + 3. review queue returns review items + 4. review queue empty behavior + 5. review context includes human-readable summary + 6. review context missing-item error + 7. approve path marks item complete + 8. approve wrong-status error + 9. reject requires feedback + 10. reject moves item back to ready + 11. reject wrong-status error + 12. MCP end-to-end queue+approve flow + +**Files modified:** +- `editor/src/AgentPermissionPolicy.h` — added review RPC permissions: + - read: `getReviewQueue`, `getReviewContext` + - mutate: `approveReviewItem`, `rejectReviewItem` +- `editor/src/MCPServer.h` — added review MCP tools and handlers: + - `whetstone_get_review_queue` -> `getReviewQueue` + - `whetstone_get_review_context` -> `getReviewContext` + - `whetstone_approve_item` -> `approveReviewItem` + - `whetstone_reject_item` -> `rejectReviewItem` +- `editor/src/HeadlessAgentRPCHandler.h` — implemented review RPC methods: + - `getReviewQueue` + - `getReviewContext` (includes `humanSummary`) + - `approveReviewItem` + - `rejectReviewItem` +- `editor/CMakeLists.txt` — `step420_test` target + +**Verification run:** +- `step420_test` — PASS (12/12) new step coverage +- `step419_test` — PASS (12/12) regression coverage +- `step418_test` — PASS (12/12) regression coverage + +**Architecture gate check:** +- `editor/src/AgentPermissionPolicy.h` within header-size limit (`128` <= `600`) +- `editor/tests/step420_test.cpp` within test-file size guidance (`204` lines) +- Legacy oversized headers persist: + - `editor/src/ast/Serialization.h` (`1427` > `600`) + - `editor/src/MCPServer.h` (`1735` > `600`) + - `editor/src/HeadlessAgentRPCHandler.h` (`2768` > `600`) + # Roadmap Planning — Sprints 12-25+ ## Status: Planning Complete (Sprints 12-19 detailed, 20-25 in roadmap.md)