From 35d74d79c959fbae8234f03301a934653b587583 Mon Sep 17 00:00:00 2001 From: Disha Prakash Date: Sat, 5 Jul 2025 18:43:46 +0000 Subject: [PATCH 1/3] chore: Add support for optional parameters --- core/e2e_test.go | 138 ++++++++++++++++++++++++++++++++++++++++++ core/protocol.go | 6 +- core/protocol_test.go | 79 ++++++++++++++++++++++++ core/tool.go | 14 ++++- 4 files changed, 235 insertions(+), 2 deletions(-) diff --git a/core/e2e_test.go b/core/e2e_test.go index 9f615a2..f0d0f57 100644 --- a/core/e2e_test.go +++ b/core/e2e_test.go @@ -335,3 +335,141 @@ func TestE2E_Auth(t *testing.T) { assert.Contains(t, err.Error(), "no field named row_data in claims") }) } + +// TestE2E_OptionalParams maps to the TestOptionalParams class +func TestE2E_OptionalParams(t *testing.T) { + // Helper to create a new client + newClient := func(t *testing.T) *core.ToolboxClient { + client, err := core.NewToolboxClient("http://localhost:5000") + require.NoError(t, err, "Failed to create ToolboxClient") + return client + } + + // Helper to load the search-rows tool + searchRowsTool := func(t *testing.T, client *core.ToolboxClient) *core.ToolboxTool { + tool, err := client.LoadTool("search-rows", context.Background()) + require.NoError(t, err, "Failed to load tool 'search-rows'") + return tool + } + + t.Run("test_tool_schema_is_correct", func(t *testing.T) { + client := newClient(t) + tool := searchRowsTool(t, client) + params := tool.Parameters() + + // Convert slice to map for easy lookup + paramMap := make(map[string]core.ParameterSchema) + for _, p := range params { + paramMap[p.Name] = p + } + + // Check required parameter 'email' + emailParam, ok := paramMap["email"] + require.True(t, ok, "email parameter should exist") + assert.True(t, emailParam.Required, "'email' should be required") + assert.Equal(t, "string", emailParam.Type) + + // Check optional parameter 'data' + dataParam, ok := paramMap["data"] + require.True(t, ok, "data parameter should exist") + assert.False(t, dataParam.Required, "'data' should be optional") + assert.Equal(t, "string", dataParam.Type) + + // Check optional parameter 'id' + idParam, ok := paramMap["id"] + require.True(t, ok, "id parameter should exist") + assert.False(t, idParam.Required, "'id' should be optional") + assert.Equal(t, "integer", idParam.Type) + }) + + t.Run("test_run_tool_omitting_optionals", func(t *testing.T) { + client := newClient(t) + tool := searchRowsTool(t, client) + + // Test case 1: Optional params are completely omitted + response1, err1 := tool.Invoke(context.Background(), map[string]any{ + "email": "twishabansal@google.com", + }) + require.NoError(t, err1) + respStr1, ok1 := response1.(string) + require.True(t, ok1) + assert.Contains(t, respStr1, `"email":"twishabansal@google.com"`) + assert.Contains(t, respStr1, "row2") + assert.NotContains(t, respStr1, "row3") + + // Test case 2: Optional params are explicitly nil + // This should produce the same result as omitting them + response2, err2 := tool.Invoke(context.Background(), map[string]any{ + "email": "twishabansal@google.com", + "data": nil, + "id": nil, + }) + require.NoError(t, err2) + respStr2, ok2 := response2.(string) + require.True(t, ok2) + assert.Equal(t, respStr1, respStr2) + }) + + t.Run("test_run_tool_with_all_params_provided", func(t *testing.T) { + client := newClient(t) + tool := searchRowsTool(t, client) + response, err := tool.Invoke(context.Background(), map[string]any{ + "email": "twishabansal@google.com", + "data": "row3", + "id": 3, + }) + require.NoError(t, err) + respStr, ok := response.(string) + require.True(t, ok) + assert.Contains(t, respStr, `"email":"twishabansal@google.com"`) + assert.Contains(t, respStr, `"id":3`) + assert.Contains(t, respStr, "row3") + assert.NotContains(t, respStr, "row2") + }) + + t.Run("test_run_tool_missing_required_param", func(t *testing.T) { + client := newClient(t) + tool := searchRowsTool(t, client) + _, err := tool.Invoke(context.Background(), map[string]any{ + "data": "row5", + "id": 5, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "missing required parameter 'email'") + }) + + t.Run("test_run_tool_required_param_is_nil", func(t *testing.T) { + client := newClient(t) + tool := searchRowsTool(t, client) + _, err := tool.Invoke(context.Background(), map[string]any{ + "email": nil, + "id": 5, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "parameter 'email' is required but received a nil value") + }) + + // Corresponds to tests that check server-side logic by providing data that doesn't match + t.Run("test_run_tool_with_non_matching_data", func(t *testing.T) { + client := newClient(t) + tool := searchRowsTool(t, client) + + // Test with a different email + response, err := tool.Invoke(context.Background(), map[string]any{ + "email": "anubhavdhawan@google.com", + "id": 3, + "data": "row3", + }) + require.NoError(t, err) + assert.Equal(t, "null", response, "Response should be null for non-matching email") + + // Test with different data + response, err = tool.Invoke(context.Background(), map[string]any{ + "email": "twishabansal@google.com", + "id": 3, + "data": "row4", // This data doesn't match the id + }) + require.NoError(t, err) + assert.Equal(t, "null", response, "Response should be null for non-matching data") + }) +} diff --git a/core/protocol.go b/core/protocol.go index e0d5f67..e43a898 100644 --- a/core/protocol.go +++ b/core/protocol.go @@ -23,6 +23,7 @@ import ( type ParameterSchema struct { Name string `json:"name"` Type string `json:"type"` + Required bool `json:"required,omitempty"` // Add this field Description string `json:"description"` AuthSources []string `json:"authSources,omitempty"` Items *ParameterSchema `json:"items,omitempty"` @@ -31,7 +32,10 @@ type ParameterSchema struct { // validateType is a helper for manual type checking. func (p *ParameterSchema) validateType(value any) error { if value == nil { - return fmt.Errorf("parameter '%s' received a nil value", p.Name) + if p.Required { + return fmt.Errorf("parameter '%s' is required but received a nil value", p.Name) + } + return nil } switch p.Type { diff --git a/core/protocol_test.go b/core/protocol_test.go index acdcbac..981cc70 100644 --- a/core/protocol_test.go +++ b/core/protocol_test.go @@ -252,3 +252,82 @@ func TestParameterSchemaUndefinedType(t *testing.T) { } } + +func TestOptionalStringParameter(t *testing.T) { + schema := ParameterSchema{ + Name: "nickname", + Type: "string", + Description: "An optional nickname", + Required: false, // Explicitly optional + } + + t.Run("allows nil value for optional parameter", func(t *testing.T) { + err := schema.validateType(nil) + if err != nil { + t.Errorf("validateType() with nil should not return an error for an optional parameter, but got: %v", err) + } + }) + + t.Run("allows valid string value", func(t *testing.T) { + err := schema.validateType("my-name") + if err != nil { + t.Errorf("validateType() should not return an error for a valid string, but got: %v", err) + } + }) +} + +func TestRequiredParameter(t *testing.T) { + schema := ParameterSchema{ + Name: "id", + Type: "integer", + Description: "A required ID", + Required: true, // Explicitly required + } + + t.Run("rejects nil value for required parameter", func(t *testing.T) { + err := schema.validateType(nil) + if err == nil { + t.Errorf("validateType() with nil should return an error for a required parameter, but it didn't") + } + }) + + t.Run("allows valid integer value", func(t *testing.T) { + err := schema.validateType(12345) + if err != nil { + t.Errorf("validateType() should not return an error for a valid integer, but got: %v", err) + } + }) +} + +func TestOptionalArrayParameter(t *testing.T) { + schema := ParameterSchema{ + Name: "optional_scores", + Type: "array", + Description: "An optional list of scores", + Required: false, + Items: &ParameterSchema{ + Type: "integer", + }, + } + + t.Run("allows nil value for optional array", func(t *testing.T) { + err := schema.validateType(nil) + if err != nil { + t.Errorf("validateType() with nil should not return an error for an optional array, but got: %v", err) + } + }) + + t.Run("allows valid integer slice", func(t *testing.T) { + err := schema.validateType([]int{95, 100}) + if err != nil { + t.Errorf("validateType() should not return an error for a valid slice, but got: %v", err) + } + }) + + t.Run("rejects slice with wrong item type", func(t *testing.T) { + err := schema.validateType([]string{"not", "an", "int"}) + if err == nil { + t.Errorf("validateType() should have returned an error for a slice with incorrect item types, but it didn't") + } + }) +} diff --git a/core/tool.go b/core/tool.go index 38929ca..3bd0ee6 100644 --- a/core/tool.go +++ b/core/tool.go @@ -343,10 +343,22 @@ func (tt *ToolboxTool) validateAndBuildPayload(input map[string]any) (map[string } } + for _, param := range tt.parameters { + if param.Required { + // A required parameter must be present in either the user input or as a bound parameter. + _, isProvided := input[param.Name] + _, isBound := tt.boundParams[param.Name] + + if !isProvided && !isBound { + return nil, fmt.Errorf("missing required parameter '%s'", param.Name) + } + } + } + // Initialize the final payload with the validated user input. finalPayload := make(map[string]any, len(input)+len(tt.boundParams)) for k, v := range input { - if _, ok := paramSchema[k]; ok { + if _, ok := paramSchema[k]; ok && v != nil { finalPayload[k] = v } } From 5b21f0c58c00bbb420de6ed02530f43fc6792fc8 Mon Sep 17 00:00:00 2001 From: Disha Prakash Date: Sat, 5 Jul 2025 18:56:10 +0000 Subject: [PATCH 2/3] change error message --- core/e2e_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/e2e_test.go b/core/e2e_test.go index f0d0f57..b9858bb 100644 --- a/core/e2e_test.go +++ b/core/e2e_test.go @@ -167,7 +167,7 @@ func TestE2E_Basic(t *testing.T) { // The Go SDK performs validation inside Invoke, so we check the error there. _, err := tool.Invoke(context.Background(), map[string]any{}) require.Error(t, err) - assert.Contains(t, err.Error(), "parameter \"num_rows\" is required") + assert.Contains(t, err.Error(), "missing required parameter 'num_rows'") }) t.Run("test_run_tool_wrong_param_type", func(t *testing.T) { From 21eec499ce2491caa3d88cb544e10a02f61f26fc Mon Sep 17 00:00:00 2001 From: Disha Prakash Date: Sat, 5 Jul 2025 23:18:23 +0000 Subject: [PATCH 3/3] minor fix --- core/protocol.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/protocol.go b/core/protocol.go index e43a898..7881d89 100644 --- a/core/protocol.go +++ b/core/protocol.go @@ -23,7 +23,7 @@ import ( type ParameterSchema struct { Name string `json:"name"` Type string `json:"type"` - Required bool `json:"required,omitempty"` // Add this field + Required bool `json:"required,omitempty"` Description string `json:"description"` AuthSources []string `json:"authSources,omitempty"` Items *ParameterSchema `json:"items,omitempty"`