diff --git a/integration/vt_test.go b/integration/vt_test.go index ce50522..561ee17 100644 --- a/integration/vt_test.go +++ b/integration/vt_test.go @@ -18,7 +18,9 @@ var mcphubRoutingFailureCodes = map[string]struct{}{ "session_not_open": {}, "tool_not_found": {}, "tool_denied": {}, - "provider_unavailable": {}, + "provider_unreachable": {}, + "provider_error": {}, + "internal_error": {}, } var vtTools = []struct { diff --git a/java/src/main/java/dev/sorted/mcphub/CapabilityRegistry.java b/java/src/main/java/dev/sorted/mcphub/CapabilityRegistry.java index bc28d1b..ba3a40b 100644 --- a/java/src/main/java/dev/sorted/mcphub/CapabilityRegistry.java +++ b/java/src/main/java/dev/sorted/mcphub/CapabilityRegistry.java @@ -291,6 +291,13 @@ public Optional findByDisplayName(String displayName) { .findFirst(); } + /** Finds a capability by its capability_id. */ + public Optional findByCapabilityId(String capabilityId) { + return entries.stream() + .filter(e -> e.capabilityId.equals(capabilityId)) + .findFirst(); + } + /** Returns all entries where enabled=true. */ public List getEnabled() { return entries.stream() diff --git a/java/src/main/java/dev/sorted/mcphub/ControlHandler.java b/java/src/main/java/dev/sorted/mcphub/ControlHandler.java index 4e2195a..31f2843 100644 --- a/java/src/main/java/dev/sorted/mcphub/ControlHandler.java +++ b/java/src/main/java/dev/sorted/mcphub/ControlHandler.java @@ -152,15 +152,16 @@ public JsonNode handle(String method, JsonNode params) sessionManager.resetActivity(); } return switch (method) { - case "mcphub.control.status" -> handleStatus(); - case "mcphub.control.arm" -> handleArm(); - case "mcphub.control.open" -> handleOpen(); - case "mcphub.control.close" -> handleClose(); - case "mcphub.control.bridge_detach"-> handleBridgeDetach(); - case "mcphub.control.lock" -> handleLock(params); - case "mcphub.control.unlock" -> handleUnlock(); - case "mcphub.control.health" -> handleHealth(); - case "mcphub.control.capabilities" -> handleCapabilities(); + case "mcphub.control.status" -> handleStatus(); + case "mcphub.control.arm" -> handleArm(); + case "mcphub.control.open" -> handleOpen(); + case "mcphub.control.close" -> handleClose(); + case "mcphub.control.bridge_detach" -> handleBridgeDetach(); + case "mcphub.control.lock" -> handleLock(params); + case "mcphub.control.unlock" -> handleUnlock(); + case "mcphub.control.health" -> handleHealth(); + case "mcphub.control.capabilities" -> handleCapabilities(); + case "mcphub.control.add_session_rule" -> handleAddSessionRule(params); default -> throw new JsonRpcServer.JsonRpcException( JsonRpcServer.ERR_NOT_FOUND, "Unknown method: " + method); }; @@ -396,6 +397,43 @@ private JsonNode handleBridgeDetach() { // Helpers // ------------------------------------------------------------------------- + /** mcphub.control.add_session_rule — REQ-7.5.1 */ + private JsonNode handleAddSessionRule(JsonNode params) throws JsonRpcServer.JsonRpcException { + StateMachine.State current = stateMachine.getState(); + if (current != StateMachine.State.ARMED && current != StateMachine.State.OPEN) { + throw new JsonRpcServer.JsonRpcException(-32001, + "add_session_rule requires Armed or Open state"); + } + if (params == null || !params.has("tool_pattern") || !params.has("action")) { + throw new JsonRpcServer.JsonRpcException(-32602, + "Missing required params: tool_pattern and action"); + } + String toolPattern = params.get("tool_pattern").asText(); + String action = params.get("action").asText().toLowerCase(); + if (!"allow".equals(action) && !"deny".equals(action) && !"hide".equals(action)) { + throw new JsonRpcServer.JsonRpcException(-32602, + "Invalid action: " + action + ". Must be allow, deny, or hide."); + } + + PolicyRule rule = new PolicyRule(); + rule.ruleId = params.has("rule_id") ? params.get("rule_id").asText() + : ("session-" + toolPattern + "-" + action); + rule.toolPattern = toolPattern; + rule.action = action; + rule.priority = params.has("priority") ? params.get("priority").asInt(100) : 100; + rule.scope = "session"; + + policy.addSessionRule(rule); + log.info("Added session rule: id={}, pattern={}, action={}", rule.ruleId, toolPattern, action); + + ObjectNode r = mapper.createObjectNode(); + r.put("status", "ok"); + r.put("rule_id", rule.ruleId); + r.put("scope", "session"); + r.put("state", current.name()); + return r; + } + /** Simulate drain and transition to CLOSED. (Session 1: immediate drain) */ private void doCoolingDownAndClose(String sessionId, String trigger) { try { diff --git a/java/src/main/java/dev/sorted/mcphub/McpHandler.java b/java/src/main/java/dev/sorted/mcphub/McpHandler.java index 79ad4cc..f3da885 100644 --- a/java/src/main/java/dev/sorted/mcphub/McpHandler.java +++ b/java/src/main/java/dev/sorted/mcphub/McpHandler.java @@ -8,8 +8,10 @@ import org.slf4j.LoggerFactory; import java.time.Instant; +import java.util.ArrayList; import java.util.HashSet; import java.util.List; +import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; @@ -168,12 +170,34 @@ private ObjectNode toMcpToolEntry(CapabilityEntry entry) { tool.put("description", desc); // inputSchema from registry entry + JsonNode schemaNode; if (entry.schema != null) { - tool.set("inputSchema", mapper.valueToTree(entry.schema)); + schemaNode = mapper.valueToTree(entry.schema); } else { ObjectNode emptySchema = mapper.createObjectNode(); emptySchema.put("type", "object"); - tool.set("inputSchema", emptySchema); + schemaNode = emptySchema; + } + tool.set("inputSchema", schemaNode); + + // REQ-5.5.4: inject _intent into inputSchema so AI clients discover it + if (schemaNode instanceof ObjectNode schemaObj) { + JsonNode propsNode = schemaObj.get("properties"); + ObjectNode props; + if (propsNode instanceof ObjectNode) { + props = (ObjectNode) propsNode; + } else { + props = mapper.createObjectNode(); + schemaObj.set("properties", props); + } + if (!props.has("_intent")) { + ObjectNode intentProp = mapper.createObjectNode(); + intentProp.put("type", "string"); + intentProp.put("description", + "Optional: brief reason WHY you chose this tool. " + + "Persisted in route logs for auditing. Does not affect routing."); + props.set("_intent", intentProp); + } } return tool; } @@ -199,6 +223,13 @@ private ObjectNode buildDisambiguationTool() { candidateTools.set("items", items); candidateTools.put("description", "Optional: restrict to these tool names"); props.set("candidate_tools", candidateTools); + // REQ-5.5.4: inject _intent so AI clients discover it + ObjectNode intentProp = mapper.createObjectNode(); + intentProp.put("type", "string"); + intentProp.put("description", + "Optional: brief reason WHY you chose this tool. " + + "Persisted in route logs for auditing. Does not affect routing."); + props.set("_intent", intentProp); schema.set("properties", props); ArrayNode required = mapper.createArrayNode(); required.add("task_description"); @@ -219,6 +250,10 @@ private JsonNode handleToolsCall(JsonNode params) { if (intentAnnotation != null && intentAnnotation.length() > 500) { intentAnnotation = intentAnnotation.substring(0, 500); } + // REQ-5.5.5, REQ-5.10.3: scrub secret patterns from intent annotation + if (intentAnnotation != null && SecretScanner.containsSecret(intentAnnotation)) { + intentAnnotation = "[scrubbed: secret pattern detected]"; + } int requestSizeBytes = params != null ? params.toString().length() : 0; @@ -281,10 +316,10 @@ private JsonNode handleToolsCall(JsonNode params) { if (!"confirmed".equals(entry.runtimeState)) { long latency = System.currentTimeMillis() - startMs; logRoute(null, toolName, entry.providerId, providerType, - "error", null, latency, requestSizeBytes, null, intentAnnotation, "provider_unavailable"); + "error", null, latency, requestSizeBytes, null, intentAnnotation, "provider_unreachable"); List available = policy.filterForAI(registry.getConfirmed()) .stream().map(e -> e.displayName).collect(Collectors.toList()); - return failureResponse("provider_unavailable", + return failureResponse("provider_unreachable", "Tool '" + toolName + "' is registered but its provider adapter is not running.", "wait_session", available, null); } @@ -296,9 +331,12 @@ private JsonNode handleToolsCall(JsonNode params) { logRoute(null, toolName, entry.providerId, providerType, "denied", policyResult.matchedRuleId(), latency, requestSizeBytes, null, intentAnnotation, "tool_denied"); + List contractFallbacks = findContractFallbacks(entry); + String nextAction = (contractFallbacks != null && !contractFallbacks.isEmpty()) + ? "use_alternative" : "disambiguate"; return failureResponse("tool_denied", "Tool '" + toolName + "' is denied by policy rule: " + policyResult.matchedRuleId(), - "abort", null, policyResult.matchedRuleId()); + nextAction, null, policyResult.matchedRuleId(), entry); } if (policyResult.decision() == PolicyEngine.Decision.HIDE) { long latency = System.currentTimeMillis() - startMs; @@ -339,9 +377,9 @@ private JsonNode handleToolsCall(JsonNode params) { long latency = providerStartMs - startMs; logRoute(null, toolName, entry.providerId, providerType, "error", policyResult.matchedRuleId(), latency, - requestSizeBytes, null, intentAnnotation, "provider_call_failed"); - return failureResponse("provider_call_failed", - "Provider '" + groupId + "' call failed: " + e.getMessage(), + requestSizeBytes, null, intentAnnotation, "provider_error"); + return failureResponse("provider_error", + "Provider '" + groupId + "' returned an error. Retry or check provider health.", "retry", null, null); } } else { @@ -349,10 +387,10 @@ private JsonNode handleToolsCall(JsonNode params) { long latency = System.currentTimeMillis() - startMs; logRoute(null, toolName, entry.providerId, providerType, "error", policyResult.matchedRuleId(), latency, - requestSizeBytes, null, intentAnnotation, "provider_unavailable"); + requestSizeBytes, null, intentAnnotation, "provider_unreachable"); List available = policy.filterForAI(registry.getConfirmed()) .stream().map(e2 -> e2.displayName).collect(Collectors.toList()); - return failureResponse("provider_unavailable", + return failureResponse("provider_unreachable", "Provider group '" + groupId + "' is not running.", "wait_session", available, null); } @@ -410,26 +448,56 @@ private JsonNode handleDisambiguate(String taskDescription, JsonNode params) { result.put("unresolvable_reason", "Registry has no enabled, policy-allowed tools to recommend."); } else { - // Multiple candidates — cannot determine deterministically (REQ-5.3.3 forbids heuristic) - result.putNull("recommended_tool"); // REQ-5.3.6: MUST be null when not deterministic - result.put("confidence", "none"); - result.put("reason", - candidates.size() + " tools are available. Hub cannot select without heuristic " + - "guessing, which is prohibited by REQ-5.3.3. " + - "Narrow via 'candidate_tools' to a single tool for a deterministic answer."); - result.put("unresolvable_reason", - "Multiple tools match. Use candidate_tools to specify exactly one tool."); + // Multiple candidates — try contract-based resolution (REQ-5.3.3, REQ-5.3.8) + CapabilityEntry contractWinner = resolveByContracts(taskDescription, candidates); + if (contractWinner != null) { + result.put("recommended_tool", contractWinner.displayName); + result.put("confidence", "deterministic"); + result.put("reason", + "Contract-based disambiguation: '" + contractWinner.displayName + + "' unambiguously covers all other candidates via disambiguates_from entries."); + result.putNull("unresolvable_reason"); + } else { + result.putNull("recommended_tool"); // REQ-5.3.6: MUST be null when not deterministic + result.put("confidence", "none"); + result.put("reason", + candidates.size() + " tools are available. Hub cannot select without heuristic " + + "guessing, which is prohibited by REQ-5.3.3. " + + "Narrow via 'candidate_tools' to a single tool for a deterministic answer."); + result.put("unresolvable_reason", + "Multiple tools match. Use candidate_tools to specify exactly one tool."); + } } // REQ-5.3.4: alternatives — list all candidates when no deterministic recommendation - // When confidence=deterministic (1 candidate), alternatives is empty. + // When confidence=deterministic (1 candidate or contract winner), alternatives is empty. ArrayNode alts = mapper.createArrayNode(); - if (candidates.size() != 1) { + boolean deterministic = candidates.size() == 1 + || (candidates.size() > 1 && resolveByContracts(taskDescription, candidates) != null); + if (!deterministic) { for (CapabilityEntry e : candidates) { if (e.contract == null || e.contract.purpose == null) continue; ObjectNode alt = mapper.createObjectNode(); alt.put("tool", e.displayName); alt.put("reason", e.contract.purpose); + if (e.contract.sideEffectClass != null) { + alt.put("side_effect_class", e.contract.sideEffectClass); + } + if (e.contract.whenToCall != null) { + ArrayNode wtc = mapper.createArrayNode(); + e.contract.whenToCall.forEach(wtc::add); + alt.set("when_to_call", wtc); + } + if (e.contract.disambiguatesFrom != null) { + ArrayNode dfArr = mapper.createArrayNode(); + for (CapabilityContract.DisambiguatesFrom df : e.contract.disambiguatesFrom) { + ObjectNode d = mapper.createObjectNode(); + d.put("capability_id", df.capabilityId); + d.put("distinction", df.distinction); + dfArr.add(d); + } + alt.set("disambiguates_from", dfArr); + } alts.add(alt); } } @@ -438,6 +506,39 @@ private JsonNode handleDisambiguate(String taskDescription, JsonNode params) { return result; } + /** + * Deterministic contract-based resolution among multiple candidates. + * REQ-5.3.3: no heuristic. REQ-5.3.8: deterministic when contracts unambiguously select one. + * + * @return the single candidate whose disambiguates_from covers ALL other candidates, + * or null if ambiguous. + */ + private CapabilityEntry resolveByContracts(String taskDescription, List candidates) { + if (candidates == null || candidates.size() < 2) return null; + CapabilityEntry winner = null; + int winnerCount = 0; + for (CapabilityEntry candidate : candidates) { + if (candidate.contract == null || candidate.contract.disambiguatesFrom == null) continue; + Set coveredIds = candidate.contract.disambiguatesFrom.stream() + .map(df -> df.capabilityId) + .filter(Objects::nonNull) + .collect(Collectors.toSet()); + boolean coversAllOthers = true; + for (CapabilityEntry other : candidates) { + if (other == candidate) continue; + if (!coveredIds.contains(other.capabilityId)) { + coversAllOthers = false; + break; + } + } + if (coversAllOthers) { + winner = candidate; + winnerCount++; + } + } + return winnerCount == 1 ? winner : null; + } + // ------------------------------------------------------------------------- // Helpers // ------------------------------------------------------------------------- @@ -501,30 +602,61 @@ private JsonNode handleProviderHealthUpdate(JsonNode params) { /** Build structured failure response. REQ-5.6.1 (P1-03) — MCP CallToolResult compliant */ private ObjectNode failureResponse(String errorCode, String reason, - String nextAction, List fallbackTools, String policyDetail) { + String nextAction, List availableTools, String policyDetail) { + return failureResponse(errorCode, reason, nextAction, availableTools, policyDetail, null); + } + + private ObjectNode failureResponse(String errorCode, String reason, + String nextAction, List availableTools, String policyDetail, CapabilityEntry entry) { ObjectNode r = mapper.createObjectNode(); - // MCP-compliant content array (REQ-5.6.1 structured failure as text) ArrayNode content = mapper.createArrayNode(); ObjectNode textItem = mapper.createObjectNode(); textItem.put("type", "text"); - StringBuilder msg = new StringBuilder(); - msg.append("[MCPHUB error] ").append(errorCode).append(": ").append(reason); - if (nextAction != null) { - msg.append(" | next_action: ").append(nextAction); - } - if (policyDetail != null) { - msg.append(" | policy: ").append(policyDetail); - } + + ObjectNode structured = mapper.createObjectNode(); + structured.put("error_code", errorCode); + structured.put("reason", reason); + if (nextAction != null) structured.put("next_action", nextAction); + if (policyDetail != null) structured.put("policy_detail", policyDetail); + + List fallbackTools = entry != null ? findContractFallbacks(entry) : null; if (fallbackTools != null && !fallbackTools.isEmpty()) { - msg.append(" | available_tools: ").append(String.join(", ", fallbackTools)); + ArrayNode ft = mapper.createArrayNode(); + fallbackTools.forEach(ft::add); + structured.set("fallback_tools", ft); } - textItem.put("text", msg.toString()); + + if (availableTools != null && !availableTools.isEmpty()) { + ArrayNode at = mapper.createArrayNode(); + availableTools.forEach(at::add); + structured.set("available_tools", at); + } + + textItem.put("text", structured.toString()); content.add(textItem); r.set("content", content); r.put("isError", true); return r; } + /** Look up disambiguates_from entries and return registered tool names. REQ-5.6.5 */ + private List findContractFallbacks(CapabilityEntry entry) { + if (entry == null || entry.contract == null || entry.contract.disambiguatesFrom == null) { + return null; + } + List result = new ArrayList<>(); + for (CapabilityContract.DisambiguatesFrom df : entry.contract.disambiguatesFrom) { + if (df.capabilityId != null) { + registry.findByCapabilityId(df.capabilityId).ifPresent(e -> { + if (policy.evaluate(e.displayName).decision() == PolicyEngine.Decision.ALLOW) { + result.add(e.displayName); + } + }); + } + } + return result.isEmpty() ? null : result; + } + /** Fire-and-forget route log (IS-05, REQ-8.3.2: async, must not block) */ private void logRoute(String sessionId, String toolName, String providerId, String providerType, String decision, String policyRuleId, diff --git a/java/src/main/java/dev/sorted/mcphub/PolicyEngine.java b/java/src/main/java/dev/sorted/mcphub/PolicyEngine.java index e42b498..c336b2b 100644 --- a/java/src/main/java/dev/sorted/mcphub/PolicyEngine.java +++ b/java/src/main/java/dev/sorted/mcphub/PolicyEngine.java @@ -71,7 +71,9 @@ public PolicyResult evaluate(String toolName) { boosted.ruleId = r.ruleId; boosted.toolPattern = r.toolPattern; boosted.action = r.action; - boosted.priority = r.priority + 10000; + // REQ-7.5.2: session rules have higher default priority than global, + // unless operator explicitly sets negative priority (opt-out of boost) + boosted.priority = r.priority >= 0 ? r.priority + 10000 : r.priority; boosted.scope = "session"; allRules.add(boosted); } diff --git a/java/src/test/java/dev/sorted/mcphub/ControlHandlerTest.java b/java/src/test/java/dev/sorted/mcphub/ControlHandlerTest.java index fc89da1..0c5bdb1 100644 --- a/java/src/test/java/dev/sorted/mcphub/ControlHandlerTest.java +++ b/java/src/test/java/dev/sorted/mcphub/ControlHandlerTest.java @@ -2,6 +2,7 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; import org.junit.jupiter.api.*; import org.junit.jupiter.api.io.TempDir; @@ -187,6 +188,167 @@ void coolingDown_clearsHealthTracker() throws Exception { "Tracker must be cleared on COOLING_DOWN transition"); } + // ------------------------------------------------------------------------- + // AC-1: add_session_rule + // ------------------------------------------------------------------------- + + @Test + void addSessionRule_whenArmed_succeeds() throws Exception { + ControlHandler h = createCapabilityAwareHandler(); + h.handle("mcphub.control.arm", null); + + ObjectNode params = mapper.createObjectNode(); + params.put("tool_pattern", "webfetch"); + params.put("action", "deny"); + params.put("rule_id", "test-deny-webfetch"); + params.put("priority", 500); + + JsonNode r = h.handle("mcphub.control.add_session_rule", params); + assertEquals("ok", r.path("status").asText()); + assertEquals("test-deny-webfetch", r.path("rule_id").asText()); + assertEquals("session", r.path("scope").asText()); + assertEquals("ARMED", r.path("state").asText()); + } + + @Test + void addSessionRule_whenOpen_succeeds() throws Exception { + ControlHandler h = createCapabilityAwareHandler(); + h.handle("mcphub.control.arm", null); + h.handle("mcphub.control.open", null); + + ObjectNode params = mapper.createObjectNode(); + params.put("tool_pattern", "webfetch"); + params.put("action", "hide"); + + JsonNode r = h.handle("mcphub.control.add_session_rule", params); + assertEquals("ok", r.path("status").asText()); + assertEquals("session", r.path("scope").asText()); + assertEquals("OPEN", r.path("state").asText()); + } + + @Test + void addSessionRule_whenClosed_rejects() throws Exception { + ControlHandler h = createCapabilityAwareHandler(); + ObjectNode params = mapper.createObjectNode(); + params.put("tool_pattern", "webfetch"); + params.put("action", "deny"); + + JsonRpcServer.JsonRpcException ex = assertThrows( + JsonRpcServer.JsonRpcException.class, + () -> h.handle("mcphub.control.add_session_rule", params)); + assertTrue(ex.getMessage().contains("Armed or Open")); + } + + @Test + void addSessionRule_invalidAction_rejects() throws Exception { + ControlHandler h = createCapabilityAwareHandler(); + h.handle("mcphub.control.arm", null); + + ObjectNode params = mapper.createObjectNode(); + params.put("tool_pattern", "webfetch"); + params.put("action", "block"); + + JsonRpcServer.JsonRpcException ex = assertThrows( + JsonRpcServer.JsonRpcException.class, + () -> h.handle("mcphub.control.add_session_rule", params)); + assertTrue(ex.getMessage().contains("allow, deny, or hide")); + } + + @Test + void addSessionRule_missingToolPattern_rejects() throws Exception { + ControlHandler h = createCapabilityAwareHandler(); + h.handle("mcphub.control.arm", null); + + ObjectNode params = mapper.createObjectNode(); + params.put("action", "deny"); + + JsonRpcServer.JsonRpcException ex = assertThrows( + JsonRpcServer.JsonRpcException.class, + () -> h.handle("mcphub.control.add_session_rule", params)); + assertTrue(ex.getMessage().contains("Missing required params")); + } + + @Test + void addSessionRule_missingAction_rejects() throws Exception { + ControlHandler h = createCapabilityAwareHandler(); + h.handle("mcphub.control.arm", null); + + ObjectNode params = mapper.createObjectNode(); + params.put("tool_pattern", "webfetch"); + + JsonRpcServer.JsonRpcException ex = assertThrows( + JsonRpcServer.JsonRpcException.class, + () -> h.handle("mcphub.control.add_session_rule", params)); + assertTrue(ex.getMessage().contains("Missing required params")); + } + + @Test + void sessionRule_purgedOnClose_thenToolAllowedAgain() throws Exception { + CapabilityRegistry registry = new CapabilityRegistry(); + try (InputStream is = ControlHandlerTest.class.getResourceAsStream("/capabilities.yaml")) { + assertNotNull(is, "capabilities.yaml must be in classpath"); + registry.load(is); + } + PolicyEngine policy = new PolicyEngine(); + policy.loadGlobalRules(registry.getPolicyRules()); + BodyBudgetService bodyBudget = new BodyBudgetService(db); + bodyBudget.setMcphubHostedToolCount(registry.getLoadedCount()); + ControlHandler h = new ControlHandler(sm, session, db, registry, policy, bodyBudget); + + // Arm session + h.handle("mcphub.control.arm", null); + + // Add session rule to deny webfetch + ObjectNode ruleParams = mapper.createObjectNode(); + ruleParams.put("tool_pattern", "webfetch"); + ruleParams.put("action", "deny"); + ruleParams.put("rule_id", "session-deny-webfetch"); + JsonNode addResult = h.handle("mcphub.control.add_session_rule", ruleParams); + assertEquals("ok", addResult.path("status").asText()); + + // Verify tool is denied before close + assertEquals(PolicyEngine.Decision.DENY, policy.evaluate("webfetch").decision()); + + // Open, then close (triggers COOLING_DOWN → clearSessionRules) + h.handle("mcphub.control.open", null); + h.handle("mcphub.control.close", null); + + // Arm and open a new session + h.handle("mcphub.control.arm", null); + h.handle("mcphub.control.open", null); + + // Verify previously-denied tool is now allowed (session rule was purged) + assertEquals(PolicyEngine.Decision.ALLOW, policy.evaluate("webfetch").decision()); + } + + @Test + void sessionRule_withExplicitLowPriority_doesNotOverrideGlobalRule() throws Exception { + // Global rule: deny webfetch at priority 1000 + PolicyEngine policy = new PolicyEngine(); + PolicyRule globalDeny = new PolicyRule(); + globalDeny.ruleId = "global-deny-webfetch"; + globalDeny.toolPattern = "webfetch"; + globalDeny.action = "deny"; + globalDeny.priority = 1000; + globalDeny.scope = "global"; + policy.loadGlobalRules(java.util.List.of(globalDeny)); + + // Session rule: allow webfetch at priority -1 (explicitly low, opt-out of boost) + PolicyRule sessionAllow = new PolicyRule(); + sessionAllow.ruleId = "session-allow-webfetch"; + sessionAllow.toolPattern = "webfetch"; + sessionAllow.action = "allow"; + sessionAllow.priority = -1; + sessionAllow.scope = "session"; + policy.addSessionRule(sessionAllow); + + // Global deny should win because session rule has explicit low priority (-1) and is NOT boosted + PolicyEngine.PolicyResult result = policy.evaluate("webfetch"); + assertEquals(PolicyEngine.Decision.DENY, result.decision(), + "Global deny at priority 1000 must override session allow at priority -1"); + assertEquals("global-deny-webfetch", result.matchedRuleId()); + } + private ControlHandler createCapabilityAwareHandler() throws Exception { CapabilityRegistry registry = new CapabilityRegistry(); try (InputStream is = ControlHandlerTest.class.getResourceAsStream("/capabilities.yaml")) { diff --git a/java/src/test/java/dev/sorted/mcphub/McpHandlerTest.java b/java/src/test/java/dev/sorted/mcphub/McpHandlerTest.java index 4b776de..3018197 100644 --- a/java/src/test/java/dev/sorted/mcphub/McpHandlerTest.java +++ b/java/src/test/java/dev/sorted/mcphub/McpHandlerTest.java @@ -138,9 +138,9 @@ void toolsCall_sessionNotOpen_returnsSessionNotOpen() throws Exception { JsonNode r = handler.handle("tools/call", params); assertTrue(r.path("isError").asBoolean(), "isError must be true"); - String text = r.path("content").get(0).path("text").asText(); - assertTrue(text.contains("session_not_open"), "Error text must contain error code"); - assertTrue(text.contains("wait_session"), "Error text must contain next_action"); + JsonNode err = parseErrorJson(r); + assertEquals("session_not_open", err.path("error_code").asText()); + assertEquals("wait_session", err.path("next_action").asText()); } @Test @@ -158,10 +158,12 @@ void toolsCall_unknownTool_returnsToolNotFound() throws Exception { JsonNode r = handler.handle("tools/call", params); assertTrue(r.path("isError").asBoolean(), "isError must be true"); - String text = r.path("content").get(0).path("text").asText(); - assertTrue(text.contains("tool_not_found"), "Error text must contain error code"); - // REQ-5.4.2: available_tools included in error text - assertTrue(text.contains("available_tools"), "Error text must list available tools"); + JsonNode err = parseErrorJson(r); + assertEquals("tool_not_found", err.path("error_code").asText()); + // REQ-5.4.2: available_tools included + assertTrue(err.has("available_tools"), "Error must contain available_tools array"); + assertTrue(err.path("available_tools").isArray()); + assertTrue(err.path("available_tools").size() > 0); } @Test @@ -184,15 +186,17 @@ void toolsCall_deniedTool_returnsToolDenied() throws Exception { JsonNode r = handler.handle("tools/call", params); assertTrue(r.path("isError").asBoolean(), "isError must be true"); - String text = r.path("content").get(0).path("text").asText(); - assertTrue(text.contains("tool_denied"), "Error text must contain error code"); + JsonNode err = parseErrorJson(r); + assertEquals("tool_denied", err.path("error_code").asText()); + assertEquals("deny-webfetch", err.path("policy_detail").asText()); // REQ-5.6.4: MUST NOT suggest retry for policy denial - assertFalse(text.contains("next_action: retry"), "MUST NOT suggest retry for policy denial"); + assertNotEquals("retry", err.path("next_action").asText(), + "MUST NOT suggest retry for policy denial"); } @Test - void toolsCall_registeredTool_withoutProviderManager_returnsUnavailable() throws Exception { - // AMD-MCPHUB-001: without a ProviderManager wired, tools/call returns provider_unavailable + void toolsCall_registeredTool_withoutProviderManager_returnsProviderUnreachable() throws Exception { + // AMD-MCPHUB-001: without a ProviderManager wired, tools/call returns provider_unreachable sm.transition(StateMachine.Trigger.ARM, "s1"); sm.transition(StateMachine.Trigger.OPEN, "s1"); confirmGroup("web", "webfetch", "websearch"); @@ -204,10 +208,10 @@ void toolsCall_registeredTool_withoutProviderManager_returnsUnavailable() throws params.set("arguments", args); JsonNode r = handler.handle("tools/call", params); - // Without providerManager, dispatch returns provider_unavailable failure response + // Without providerManager, dispatch returns provider_unreachable failure response assertTrue(r.path("isError").asBoolean(), "isError must be true"); - String text = r.path("content").get(0).path("text").asText(); - assertTrue(text.contains("provider_unavailable"), "Error text must contain error code"); + JsonNode err = parseErrorJson(r); + assertEquals("provider_unreachable", err.path("error_code").asText()); } @Test @@ -273,7 +277,7 @@ void adapterRegistration_confirmsEntries() throws Exception { } @Test - void toolsCall_pendingTool_returnsProviderUnavailable() throws Exception { + void toolsCall_pendingTool_returnsProviderUnreachable() throws Exception { sm.transition(StateMachine.Trigger.ARM, "s1"); sm.transition(StateMachine.Trigger.OPEN, "s1"); @@ -283,8 +287,8 @@ void toolsCall_pendingTool_returnsProviderUnavailable() throws Exception { JsonNode r = handler.handle("tools/call", params); assertTrue(r.path("isError").asBoolean(), "isError must be true"); - String text = r.path("content").get(0).path("text").asText(); - assertTrue(text.contains("provider_unavailable"), "Error text must contain error code"); + JsonNode err = parseErrorJson(r); + assertEquals("provider_unreachable", err.path("error_code").asText()); } // ------------------------------------------------------------------------- @@ -352,6 +356,161 @@ private void confirmGroup(String groupId, String... tools) throws Exception { handler.handle("mcphub.internal.adapter_registration", params); } + // ------------------------------------------------------------------------- + // AC-2: structured failure response with fallback_tools from contracts + // ------------------------------------------------------------------------- + + @Test + void toolsCall_deniedTool_fallbackToolsFromContract() throws Exception { + PolicyRule deny = new PolicyRule(); + deny.ruleId = "deny-webfetch"; + deny.toolPattern = "webfetch"; + deny.action = "deny"; + deny.priority = 999; + deny.scope = "global"; + policy.loadGlobalRules(java.util.List.of(deny)); + + sm.transition(StateMachine.Trigger.ARM, "s1"); + sm.transition(StateMachine.Trigger.OPEN, "s1"); + confirmGroup("web", "webfetch", "websearch"); + + ObjectNode params = mapper.createObjectNode(); + params.put("name", "webfetch"); + params.set("arguments", mapper.createObjectNode()); + + JsonNode r = handler.handle("tools/call", params); + JsonNode err = parseErrorJson(r); + assertEquals("tool_denied", err.path("error_code").asText()); + // webfetch contract disambiguates_from includes websearch + assertTrue(err.has("fallback_tools"), "fallback_tools must be populated from contract"); + assertTrue(err.path("fallback_tools").isArray()); + boolean hasWebsearch = false; + for (JsonNode ft : err.path("fallback_tools")) { + if ("websearch".equals(ft.asText())) hasWebsearch = true; + } + assertTrue(hasWebsearch, "fallback_tools should contain websearch from contract"); + // REQ-5.8.1: use_alternative when fallbacks exist + assertEquals("use_alternative", err.path("next_action").asText()); + } + + @Test + void toolsCall_deniedTool_noFallbacks_suggestsDisambiguate() throws Exception { + // Deny a tool that has no disambiguates_from entries in contract + PolicyRule deny = new PolicyRule(); + deny.ruleId = "deny-list"; + deny.toolPattern = "list"; + deny.action = "deny"; + deny.priority = 999; + deny.scope = "global"; + policy.loadGlobalRules(java.util.List.of(deny)); + + sm.transition(StateMachine.Trigger.ARM, "s1"); + sm.transition(StateMachine.Trigger.OPEN, "s1"); + confirmGroup("project", "list"); + + ObjectNode params = mapper.createObjectNode(); + params.put("name", "list"); + params.set("arguments", mapper.createObjectNode()); + + JsonNode r = handler.handle("tools/call", params); + JsonNode err = parseErrorJson(r); + assertEquals("tool_denied", err.path("error_code").asText()); + assertFalse(err.has("fallback_tools"), "No contract fallbacks for list"); + assertEquals("disambiguate", err.path("next_action").asText()); + } + + // ------------------------------------------------------------------------- + // AC-3: disambiguation with multi-candidate contract resolution + // ------------------------------------------------------------------------- + + @Test + void disambiguate_contractResolution_deterministic() throws Exception { + sm.transition(StateMachine.Trigger.ARM, "s1"); + sm.transition(StateMachine.Trigger.OPEN, "s1"); + confirmGroup("web", "webfetch", "websearch"); + confirmGroup("project", "codesearch"); + + ObjectNode params = mapper.createObjectNode(); + params.put("name", "mcphub_disambiguate"); + ObjectNode args = mapper.createObjectNode(); + args.put("task_description", "find information online"); + ArrayNode candidates = mapper.createArrayNode(); + candidates.add("websearch"); + candidates.add("webfetch"); + candidates.add("codesearch"); + args.set("candidate_tools", candidates); + params.set("arguments", args); + + JsonNode r = handler.handle("tools/call", params); + assertTrue(r.has("content"), "Disambiguation must return 'content' field"); + String text = r.path("content").get(0).path("text").asText(); + JsonNode result = mapper.readTree(text); + + // websearch disambiguates_from covers webfetch AND codesearch + assertEquals("websearch", result.path("recommended_tool").asText()); + assertEquals("deterministic", result.path("confidence").asText()); + // Alternatives should be empty when deterministic + assertEquals(0, result.path("alternatives").size(), + "Alternatives must be empty when confidence is deterministic"); + } + + @Test + void disambiguate_ambiguous_returnsNone() throws Exception { + sm.transition(StateMachine.Trigger.ARM, "s1"); + sm.transition(StateMachine.Trigger.OPEN, "s1"); + confirmGroup("web", "webfetch", "websearch"); + + ObjectNode params = mapper.createObjectNode(); + params.put("name", "mcphub_disambiguate"); + ObjectNode args = mapper.createObjectNode(); + args.put("task_description", "fetch or search"); + ArrayNode candidates = mapper.createArrayNode(); + candidates.add("webfetch"); + candidates.add("websearch"); + args.set("candidate_tools", candidates); + params.set("arguments", args); + + JsonNode r = handler.handle("tools/call", params); + String text = r.path("content").get(0).path("text").asText(); + JsonNode result = mapper.readTree(text); + + // Both cover each other → ambiguous + assertTrue(result.path("recommended_tool").isNull()); + assertEquals("none", result.path("confidence").asText()); + // Alternatives should be enriched with contract fields + JsonNode alts = result.path("alternatives"); + assertTrue(alts.size() > 0, "Alternatives must be populated when ambiguous"); + boolean hasSideEffectClass = false; + for (JsonNode alt : alts) { + if (alt.has("side_effect_class")) hasSideEffectClass = true; + } + assertTrue(hasSideEffectClass, "Alternatives must include side_effect_class"); + } + + // ------------------------------------------------------------------------- + // AC-4: intent annotation in tool schemas + // ------------------------------------------------------------------------- + + @Test + void toolsList_includesIntentProperty() throws Exception { + sm.transition(StateMachine.Trigger.ARM, "s1"); + sm.transition(StateMachine.Trigger.OPEN, "s1"); + confirmGroup("web", "webfetch", "websearch"); + + JsonNode r = handler.handle("tools/list", null); + JsonNode tools = r.path("tools"); + assertTrue(tools.size() > 0, "tools/list must return tools"); + + for (JsonNode t : tools) { + JsonNode props = t.path("inputSchema").path("properties"); + assertTrue(props.has("_intent"), + "Tool '" + t.path("name").asText() + "' must have _intent in inputSchema"); + JsonNode intent = props.path("_intent"); + assertEquals("string", intent.path("type").asText()); + assertTrue(intent.path("description").asText().contains("route logs")); + } + } + // ------------------------------------------------------------------------- // REQ-3.7.3: tools/call MUST reset the session idle timer // ------------------------------------------------------------------------- @@ -404,4 +563,69 @@ void toolsCall_withoutSessionManager_doesNotCrash() throws Exception { JsonNode r = handler.handle("tools/call", params); assertNotNull(r); } + + // ------------------------------------------------------------------------- + // AC-4 follow-up: _intent persistence to route_log (F-4) + // ------------------------------------------------------------------------- + + @Test + void toolsCall_intentAnnotation_withSecret_scrubbedInRouteLog() throws Exception { + sm.transition(StateMachine.Trigger.ARM, "s1"); + sm.transition(StateMachine.Trigger.OPEN, "s1"); + confirmGroup("web", "webfetch", "websearch"); + + ObjectNode params = mapper.createObjectNode(); + params.put("name", "webfetch"); + ObjectNode args = mapper.createObjectNode(); + args.put("url", "https://example.com"); + params.set("arguments", args); + params.put("_intent", "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.dozjgNryP4J3jVmNHl0w5N_XgL0n3I9PlFUP0THsR8U"); + + handler.handle("tools/call", params); + + // Wait for async route log write + Thread.sleep(100); + + var conn = db.getConnection(); + try (var st = conn.createStatement(); + var rs = st.executeQuery( + "SELECT intent_annotation FROM route_log WHERE tool_name='webfetch'")) { + assertTrue(rs.next(), "route_log must contain an entry for webfetch"); + assertEquals("[scrubbed: secret pattern detected]", rs.getString("intent_annotation"), + "JWT token in _intent must be scrubbed before DB write"); + } + } + + @Test + void toolsCall_intentAnnotation_persistedToRouteLog() throws Exception { + sm.transition(StateMachine.Trigger.ARM, "s1"); + sm.transition(StateMachine.Trigger.OPEN, "s1"); + confirmGroup("web", "webfetch", "websearch"); + + ObjectNode params = mapper.createObjectNode(); + params.put("name", "webfetch"); + ObjectNode args = mapper.createObjectNode(); + args.put("url", "https://example.com"); + params.set("arguments", args); + params.put("_intent", "testing intent persistence"); + + handler.handle("tools/call", params); + + // Wait for async route log write + Thread.sleep(100); + + var conn = db.getConnection(); + try (var st = conn.createStatement(); + var rs = st.executeQuery( + "SELECT intent_annotation FROM route_log WHERE tool_name='webfetch'")) { + assertTrue(rs.next(), "route_log must contain an entry for webfetch"); + assertEquals("testing intent persistence", rs.getString("intent_annotation")); + } + } + + /** Parse the JSON-encoded structured error inside the MCP text content. */ + private JsonNode parseErrorJson(JsonNode response) throws Exception { + String text = response.path("content").get(0).path("text").asText(); + return mapper.readTree(text); + } } diff --git a/java/src/test/java/dev/sorted/mcphub/PolicyEngineTest.java b/java/src/test/java/dev/sorted/mcphub/PolicyEngineTest.java index 67d414b..a7f1e50 100644 --- a/java/src/test/java/dev/sorted/mcphub/PolicyEngineTest.java +++ b/java/src/test/java/dev/sorted/mcphub/PolicyEngineTest.java @@ -180,6 +180,26 @@ void clearSessionRules_restoresGlobalBehavior() { assertEquals(PolicyEngine.Decision.ALLOW, engine.evaluate("webfetch").decision()); } + @Test + void sessionRule_withExplicitHighPriority_beatsGlobalRule() { + // REQ-7.5.2: session rule priority 200 → boosted to 10200, beating global 1000 + engine.loadGlobalRules(List.of(rule("global-deny", "webfetch", "deny", 1000))); + PolicyRule sessionRule = rule("session-allow", "webfetch", "allow", 200); + sessionRule.scope = "session"; + engine.addSessionRule(sessionRule); + assertEquals(PolicyEngine.Decision.ALLOW, engine.evaluate("webfetch").decision()); + } + + @Test + void sessionRule_withNegativePriority_doesNotOverrideGlobalRule() { + // REQ-7.5.2: negative priority = explicit opt-out of boost + engine.loadGlobalRules(List.of(rule("global-deny", "webfetch", "deny", 100))); + PolicyRule sessionRule = rule("session-allow", "webfetch", "allow", -1); + sessionRule.scope = "session"; + engine.addSessionRule(sessionRule); + assertEquals(PolicyEngine.Decision.DENY, engine.evaluate("webfetch").decision()); + } + // --- Helpers --- private PolicyRule rule(String id, String pattern, String action, int priority) {