typo

CONTRIBUTING.md

@@ -59,6 +59,13 @@ You can manually trigger the bot by commenting on your Pull Request:
 *   `/gemini summary`: Posts a summary of the changes in the pull request.
 *   `/gemini help`: Overview of the available commands
 
+## Guidelines for Pull Requests
+
+1. Please keep your PR small for more thorough review and easier updates. In case of regression, it also allows us to roll back a single feature instead of multiple ones.
+1. For non-trivial changes, consider opening an issue and discussing it with the code owners first.
+1. Provide a good PR description as a record of what change is being made and why it was made. Link to a GitHub issue if it exists.
+1. Make sure your code is thoroughly tested with unit tests and integration tests. Remember to clean up the test instances properly in your code to avoid memory leaks.
+
 ## Adding a New Database Source or Tool
 
 Please create an
@@ -110,6 +117,8 @@ implementation](https://github.com/googleapis/genai-toolbox/blob/main/internal/s
 We recommend looking at an [example tool
 implementation](https://github.com/googleapis/genai-toolbox/tree/main/internal/tools/postgres/postgressql).
 
+Remember to keep your PRs small. For example, if you are contributing a new Source, only include one or two core Tools within the same PR, the rest of the Tools can come in subsequent PRs. 
+
 * **Create a new directory** under `internal/tools` for your tool type (e.g., `internal/tools/newdb/newdbtool`).
 * **Define a configuration struct** for your tool in a file named `newdbtool.go`.
 Create a `Config` struct and a `Tool` struct to store necessary parameters for
@@ -163,6 +172,8 @@ tools.
             parameters][temp-param-doc]. Only run this test if template
             parameters apply to your tool.
 
+* **Add additional tests** for the tools that are not covered by the predefined tests. Every tool must be tested!
+
 * **Add the new database to the integration test workflow** in
   [integration.cloudbuild.yaml](.ci/integration.cloudbuild.yaml).
 
@@ -179,6 +190,7 @@ tools.
 [temp-param-doc]:
     https://googleapis.github.io/genai-toolbox/resources/tools/#template-parameters
 
+
 ### Adding Documentation
 
 * **Update the documentation** to include information about your new data source

internal/server/config.go

@@ -16,7 +16,6 @@ package server
 import (
 	"context"
 	"fmt"
-	"regexp"
 	"strings"
 
 	yaml "github.com/goccy/go-yaml"
@@ -140,10 +139,6 @@ func (c *SourceConfigs) UnmarshalYAML(ctx context.Context, unmarshal func(interf
 	}
 
 	for name, u := range raw {
-		err := NameValidation(name)
-		if err != nil {
-			return err
-		}
 		// Unmarshal to a general type that ensure it capture all fields
 		var v map[string]any
 		if err := u.Unmarshal(&v); err != nil {
@@ -188,10 +183,6 @@ func (c *AuthServiceConfigs) UnmarshalYAML(ctx context.Context, unmarshal func(i
 	}
 
 	for name, u := range raw {
-		err := NameValidation(name)
-		if err != nil {
-			return err
-		}
 		var v map[string]any
 		if err := u.Unmarshal(&v); err != nil {
 			return fmt.Errorf("unable to unmarshal %q: %w", name, err)
@@ -235,10 +226,6 @@ func (c *EmbeddingModelConfigs) UnmarshalYAML(ctx context.Context, unmarshal fun
 	}
 
 	for name, u := range raw {
-		err := NameValidation(name)
-		if err != nil {
-			return err
-		}
 		// Unmarshal to a general type that ensure it capture all fields
 		var v map[string]any
 		if err := u.Unmarshal(&v); err != nil {
@@ -283,10 +270,6 @@ func (c *ToolConfigs) UnmarshalYAML(ctx context.Context, unmarshal func(interfac
 	}
 
 	for name, u := range raw {
-		err := NameValidation(name)
-		if err != nil {
-			return err
-		}
 		var v map[string]any
 		if err := u.Unmarshal(&v); err != nil {
 			return fmt.Errorf("unable to unmarshal %q: %w", name, err)
@@ -340,10 +323,6 @@ func (c *ToolsetConfigs) UnmarshalYAML(ctx context.Context, unmarshal func(inter
 	}
 
 	for name, toolList := range raw {
-		err := NameValidation(name)
-		if err != nil {
-			return err
-		}
 		(*c)[name] = tools.ToolsetConfig{Name: name, ToolNames: toolList}
 	}
 	return nil
@@ -363,10 +342,6 @@ func (c *PromptConfigs) UnmarshalYAML(ctx context.Context, unmarshal func(interf
 	}
 
 	for name, u := range raw {
-		err := NameValidation(name)
-		if err != nil {
-			return err
-		}
 		var v map[string]any
 		if err := u.Unmarshal(&v); err != nil {
 			return fmt.Errorf("unable to unmarshal prompt %q: %w", name, err)
@@ -414,31 +389,7 @@ func (c *PromptsetConfigs) UnmarshalYAML(ctx context.Context, unmarshal func(int
 	}
 
 	for name, promptList := range raw {
-		err := NameValidation(name)
-		if err != nil {
-			return err
-		}
 		(*c)[name] = prompts.PromptsetConfig{Name: name, PromptNames: promptList}
 	}
 	return nil
 }
-
-// Tools naming validation is added in the MCP v2025-11-25, but we'll be
-// implementing it across Toolbox
-// Tool names SHOULD be between 1 and 128 characters in length (inclusive).
-// Tool names SHOULD be considered case-sensitive.
-// The following SHOULD be the only allowed characters: uppercase and lowercase ASCII letters (A-Z, a-z), digits (0-9), underscore (_), hyphen (-), and dot (.)
-// Tool names SHOULD NOT contain spaces, commas, or other special characters.
-// Tool names SHOULD be unique within a server.
-func NameValidation(name string) error {
-	strLen := len(name)
-	if strLen < 1 || strLen > 128 {
-		return fmt.Errorf("resource name SHOULD be between 1 and 128 characters in length (inclusive)")
-	}
-	validChars := regexp.MustCompile("^[a-zA-Z0-9_.-]+$")
-	isValid := validChars.MatchString(name)
-	if !isValid {
-		return fmt.Errorf("invalid character for resource name; only uppercase and lowercase ASCII letters (A-Z, a-z), digits (0-9), underscore (_), hyphen (-), and dot (.) is allowed")
-	}
-	return nil
-}

internal/server/server_test.go

@@ -200,62 +200,3 @@ func TestUpdateServer(t *testing.T) {
 		t.Errorf("error updating server, promptset (-want +got):\n%s", diff)
 	}
 }
-
-func TestNameValidation(t *testing.T) {
-	testCases := []struct {
-		desc         string
-		resourceName string
-		errStr       string
-	}{
-		{
-			desc:         "names with 0 length",
-			resourceName: "",
-			errStr:       "resource name SHOULD be between 1 and 128 characters in length (inclusive)",
-		},
-		{
-			desc:         "names with allowed length",
-			resourceName: "foo",
-		},
-		{
-			desc:         "names with 128 length",
-			resourceName: strings.Repeat("a", 128),
-		},
-		{
-			desc:         "names with more than 128 length",
-			resourceName: strings.Repeat("a", 129),
-			errStr:       "resource name SHOULD be between 1 and 128 characters in length (inclusive)",
-		},
-		{
-			desc:         "names with space",
-			resourceName: "foo bar",
-			errStr:       "invalid character for resource name; only uppercase and lowercase ASCII letters (A-Z, a-z), digits (0-9), underscore (_), hyphen (-), and dot (.) is allowed",
-		},
-		{
-			desc:         "names with commas",
-			resourceName: "foo,bar",
-			errStr:       "invalid character for resource name; only uppercase and lowercase ASCII letters (A-Z, a-z), digits (0-9), underscore (_), hyphen (-), and dot (.) is allowed",
-		},
-		{
-			desc:         "names with other special character",
-			resourceName: "foo!",
-			errStr:       "invalid character for resource name; only uppercase and lowercase ASCII letters (A-Z, a-z), digits (0-9), underscore (_), hyphen (-), and dot (.) is allowed",
-		},
-		{
-			desc:         "names with allowed special character",
-			resourceName: "foo_.-bar6",
-		},
-	}
-	for _, tc := range testCases {
-		t.Run(tc.desc, func(t *testing.T) {
-			err := server.NameValidation(tc.resourceName)
-			if err != nil {
-				if tc.errStr != err.Error() {
-					t.Fatalf("unexpected error: %s", err)
-				}
-			}
-			if err == nil && tc.errStr != "" {
-				t.Fatalf("expect error: %s", tc.errStr)
-			}
-		})
-	}
-}