From c021c5568c22fc511dabb81bdb9efc42f4d00f95 Mon Sep 17 00:00:00 2001 From: mlogclub Date: Sun, 26 Jul 2026 12:39:34 +0800 Subject: [PATCH] refactor: update MCP tool handling to treat administrator-selected tools as direct tools without confirmation --- internal/ai/tooling/registry.go | 9 +++++---- internal/ai/tooling/registry_test.go | 15 ++++++++------- .../services/ai_agent_workflow_service_test.go | 8 ++++---- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/internal/ai/tooling/registry.go b/internal/ai/tooling/registry.go index cd68514..504c1f6 100644 --- a/internal/ai/tooling/registry.go +++ b/internal/ai/tooling/registry.go @@ -74,15 +74,16 @@ func (r *Registry) Resolve(toolCode string) (Definition, error) { if serverCode == "" || toolName == "" { return Definition{}, fmt.Errorf("unsupported tool code: %s", toolCode) } - // MCP metadata cannot reliably describe side effects. Treat it as sensitive - // until an administrator provides a more specific policy in a later phase. + // MCP tools are explicitly selected by an administrator before an Agent can + // call them. Treat that persisted allow-list as the authorization boundary; + // only tools with an explicit built-in policy require extra confirmation. return Definition{ Code: toolCode, Name: toolName, InputSchema: map[string]any{"type": "object", "additionalProperties": true}, SourceType: enums.ToolSourceTypeMCP, - RiskLevel: RiskLevelSensitive, - RequireConfirmation: true, + RiskLevel: RiskLevelRead, + RequireConfirmation: false, MaxCallsPerRun: 3, TimeoutMS: 30000, IdempotencyMode: "caller", diff --git a/internal/ai/tooling/registry_test.go b/internal/ai/tooling/registry_test.go index db2476e..5bd4b19 100644 --- a/internal/ai/tooling/registry_test.go +++ b/internal/ai/tooling/registry_test.go @@ -61,15 +61,15 @@ func TestRegistryIncludesAllTicketDraftToolInputs(t *testing.T) { } } -func TestRegistryTreatsMCPToolsAsSensitive(t *testing.T) { +func TestRegistryTreatsAdministratorSelectedMCPToolsAsDirectTools(t *testing.T) { definition, err := DefaultRegistry.Resolve("knowledge/search") if err != nil { t.Fatalf("Resolve returned error: %v", err) } - if definition.RiskLevel != RiskLevelSensitive || !definition.RequireConfirmation { + if definition.RiskLevel != RiskLevelRead || definition.RequireConfirmation { t.Fatalf("unexpected MCP definition: %#v", definition) } - if err := DefaultRegistry.Authorize(definition, Policy{AllowedToolCodes: []string{"knowledge/search"}, Confirmed: true}); err != nil { + if err := DefaultRegistry.Authorize(definition, Policy{AllowedToolCodes: []string{"knowledge/search"}, AllowedRiskLevels: []string{RiskLevelRead}}); err != nil { t.Fatalf("Authorize returned error: %v", err) } } @@ -91,13 +91,14 @@ func TestNormalizeCustomerReplyRejectsSecretAndNormalizesText(t *testing.T) { } } -func TestMCPExecutorRejectsUnconfirmedToolBeforeRuntimeCall(t *testing.T) { +func TestMCPExecutorAllowsSelectedToolThroughAuthorization(t *testing.T) { executor := NewMCPExecutor(DefaultRegistry, nil) _, _, err := executor.Execute(t.Context(), "knowledge/search", nil, Policy{ - AllowedToolCodes: []string{"knowledge/search"}, + AllowedToolCodes: []string{"knowledge/search"}, + AllowedRiskLevels: []string{RiskLevelRead}, }) - if err == nil || !strings.Contains(err.Error(), "confirmation") { - t.Fatalf("expected confirmation rejection, got %v", err) + if err == nil || !strings.Contains(err.Error(), "runtime is not configured") { + t.Fatalf("expected authorization to pass before the missing runtime error, got %v", err) } } diff --git a/internal/services/ai_agent_workflow_service_test.go b/internal/services/ai_agent_workflow_service_test.go index cc4ff8a..c8cb996 100644 --- a/internal/services/ai_agent_workflow_service_test.go +++ b/internal/services/ai_agent_workflow_service_test.go @@ -336,11 +336,11 @@ func TestAIAgentServiceRejectsPublishWithUnavailableModelConfig(t *testing.T) { } } -func TestAIAgentServiceRejectsPublishWithSensitiveDirectTool(t *testing.T) { +func TestAIAgentServiceAllowsPublishWithAdministratorSelectedMCPTool(t *testing.T) { setupAIAgentWorkflowTestDB(t) operator := aiAgentWorkflowTestOperator() agent, err := AIAgentService.CreateAIAgent(request.CreateAIAgentRequest{ - Name: "sensitive tool agent", AIConfigID: createAIAgentWorkflowTestConfig(t), RuntimeMode: enums.AIAgentRuntimeModeAutonomous, + Name: "mcp tool agent", AIConfigID: createAIAgentWorkflowTestConfig(t), RuntimeMode: enums.AIAgentRuntimeModeAutonomous, ServiceMode: enums.IMConversationServiceModeAIOnly, HandoffMode: enums.AIAgentHandoffModeWaitPool, FallbackMode: enums.AIAgentFallbackModeNoAnswer, }, operator) if err != nil { @@ -349,8 +349,8 @@ func TestAIAgentServiceRejectsPublishWithSensitiveDirectTool(t *testing.T) { if err := sqls.DB().Model(&models.AIAgent{}).Where("id = ?", agent.ID).Update("allowed_mcp_tools", `[{"toolCode":"mcp/demo/write_order"}]`).Error; err != nil { t.Fatalf("set direct tool: %v", err) } - if _, err := AIAgentService.PublishAIAgent(agent.ID, operator); err == nil || !strings.Contains(err.Error(), "confirmed playbook") { - t.Fatalf("expected sensitive direct tool publish rejection, got %v", err) + if _, err := AIAgentService.PublishAIAgent(agent.ID, operator); err != nil { + t.Fatalf("expected administrator-selected MCP tool to be publishable, got %v", err) } }