Enforce MCP tool name uniqueness per capset

internal/admin/admin.go

@@ -1338,7 +1338,11 @@ func (s *Server) handleCapsetPath(w http.ResponseWriter, r *http.Request, capset
 					}
 				}
 				if err := s.Store.AddCapsetMethod(r.Context(), domain.CapsetMethod{CapsetInstanceID: ci.ID, MethodFullName: method.FullName, MCPToolName: toolName, Enabled: true}); err != nil {
-					writeError(w, http.StatusBadRequest, err.Error())
+					statusCode := http.StatusBadRequest
+					if errors.Is(err, store.ErrMCPToolNameConflict) {
+						statusCode = http.StatusConflict
+					}
+					writeError(w, statusCode, err.Error())
 					return
 				}
 			}
@@ -1440,7 +1444,11 @@ func (s *Server) handleCapsetPath(w http.ResponseWriter, r *http.Request, capset
 			}
 		}
 		if err := s.Store.AddCapsetMethod(r.Context(), domain.CapsetMethod{CapsetInstanceID: ciID, MethodFullName: selected.FullName, MCPToolName: toolName, Enabled: true}); err != nil {
-			writeError(w, http.StatusBadRequest, err.Error())
+			statusCode := http.StatusBadRequest
+			if errors.Is(err, store.ErrMCPToolNameConflict) {
+				statusCode = http.StatusConflict
+			}
+			writeError(w, statusCode, err.Error())
 			return
 		}
 		s.logger().Info("capset_method_selected", "capset_id", capsetID, "instance_id", req.InstanceID, "method", selected.FullName, "mcp_tool", toolName)

internal/store/mcp_tool_name_test.go

@@ -0,0 +1,44 @@
+package store
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"octobus/internal/domain"
+)
+
+func TestAddCapsetMethodEnforcesToolNameUniquenessWithinCapset(t *testing.T) {
+	st, err := Open(":memory:")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer st.Close()
+	ctx := context.Background()
+
+	if err := st.UpsertService(ctx, domain.Service{ID: "echo", Name: "Echo"}); err != nil {
+		t.Fatal(err)
+	}
+	if err := st.UpsertInstance(ctx, domain.Instance{ID: "echo-instance", ServiceID: "echo", Name: "Echo", Enabled: true}); err != nil {
+		t.Fatal(err)
+	}
+	for _, capsetID := range []string{"dev", "qa"} {
+		if err := st.CreateCapset(ctx, domain.Capset{ID: capsetID, Name: capsetID, Enabled: true}); err != nil {
+			t.Fatal(err)
+		}
+		if err := st.AddCapsetInstance(ctx, domain.CapsetInstance{ID: capsetID + ":echo-instance", CapsetID: capsetID, ServiceID: "echo", InstanceID: "echo-instance", Enabled: true}); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	first := domain.CapsetMethod{CapsetInstanceID: "dev:echo-instance", MethodFullName: "echo.Echo/Call", MCPToolName: "shared_tool", Enabled: true}
+	if err := st.AddCapsetMethod(ctx, first); err != nil {
+		t.Fatal(err)
+	}
+	if err := st.AddCapsetMethod(ctx, domain.CapsetMethod{CapsetInstanceID: "dev:echo-instance", MethodFullName: "echo.Echo/Other", MCPToolName: "shared_tool", Enabled: true}); !errors.Is(err, ErrMCPToolNameConflict) {
+		t.Fatalf("duplicate tool error = %v", err)
+	}
+	if err := st.AddCapsetMethod(ctx, domain.CapsetMethod{CapsetInstanceID: "qa:echo-instance", MethodFullName: "echo.Echo/Call", MCPToolName: "shared_tool", Enabled: true}); err != nil {
+		t.Fatalf("same tool in another capset error = %v", err)
+	}
+}

internal/store/store.go

@@ -22,6 +22,10 @@ type Store struct {
 	db *sql.DB
 }
 
+var ErrMCPToolNameConflict = errors.New("MCP tool name conflict")
+
+const mcpToolKeySeparator = "\x1f"
+
 type ServiceInUseError struct {
 	ServiceID  string
 	InstanceID string
@@ -109,6 +113,15 @@ func (s *Store) Migrate(ctx context.Context) error {
 	if err := addColumnIfMissing(ctx, s.db, "services", "service_root", "TEXT NOT NULL DEFAULT '.'"); err != nil {
 		return err
 	}
+	if err := addColumnIfMissing(ctx, s.db, "capset_methods", "mcp_tool_key", "TEXT NOT NULL DEFAULT ''"); err != nil {
+		return err
+	}
+	if _, err := s.db.ExecContext(ctx, `UPDATE capset_methods SET mcp_tool_key = capset_instance_id || char(31) || mcp_tool_name WHERE mcp_tool_name <> '' AND mcp_tool_key = ''`); err != nil {
+		return err
+	}
+	if _, err := s.db.ExecContext(ctx, `CREATE UNIQUE INDEX IF NOT EXISTS uq_capset_methods_mcp_tool_key ON capset_methods(mcp_tool_key) WHERE mcp_tool_key <> ''`); err != nil {
+		return err
+	}
 	_, err := s.db.ExecContext(ctx, `UPDATE services SET runtime_mode = 'long-running' WHERE runtime_mode = ''`)
 	return err
 }
@@ -782,8 +795,25 @@ func (s *Store) AddCapsetMethod(ctx context.Context, method domain.CapsetMethod)
 	now := time.Now().UTC()
 	method.CreatedAt = now
 	method.UpdatedAt = now
-	_, err := s.db.ExecContext(ctx, `INSERT INTO capset_methods (id, capset_instance_id, method_full_name, rest_alias, mcp_tool_name, enabled, created_at, updated_at) VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, method.ID, method.CapsetInstanceID, method.MethodFullName, method.RestAlias, method.MCPToolName, boolInt(method.Enabled), formatTime(method.CreatedAt), formatTime(method.UpdatedAt))
-	return err
+
+	tx, err := s.db.BeginTx(ctx, nil)
+	if err != nil {
+		return err
+	}
+	defer tx.Rollback()
+
+	toolKey := ""
+	if method.MCPToolName != "" {
+		toolKey = method.CapsetInstanceID + mcpToolKeySeparator + method.MCPToolName
+	}
+	_, err = tx.ExecContext(ctx, `INSERT INTO capset_methods (id, capset_instance_id, method_full_name, rest_alias, mcp_tool_name, mcp_tool_key, enabled, created_at, updated_at) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)`, method.ID, method.CapsetInstanceID, method.MethodFullName, method.RestAlias, method.MCPToolName, toolKey, boolInt(method.Enabled), formatTime(method.CreatedAt), formatTime(method.UpdatedAt))
+	if err != nil {
+		if strings.Contains(strings.ToUpper(err.Error()), "UNIQUE") && strings.Contains(err.Error(), "mcp_tool_key") {
+			return ErrMCPToolNameConflict
+		}
+		return err
+	}
+	return tx.Commit()
 }
 
 func (s *Store) GetCapsetMethod(ctx context.Context, capsetInstanceID, methodFullName string) (domain.CapsetMethod, error) {
@@ -1257,6 +1287,7 @@ CREATE TABLE IF NOT EXISTS capset_methods (
   method_full_name TEXT NOT NULL,
   rest_alias TEXT NOT NULL DEFAULT '',
   mcp_tool_name TEXT NOT NULL DEFAULT '',
+  mcp_tool_key TEXT NOT NULL DEFAULT '',
   enabled INTEGER NOT NULL DEFAULT 1,
   created_at TEXT NOT NULL,
   updated_at TEXT NOT NULL,