diff --git a/go.mod b/go.mod index 6ff6803..31dbc86 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/modelcontextprotocol/go-sdk v1.6.1 github.com/teamwork/desksdkgo v1.0.1 github.com/teamwork/spacessdkgo v0.0.0-20260518181558-a6af69d00abb - github.com/teamwork/twapi-go-sdk v1.20.2 + github.com/teamwork/twapi-go-sdk v1.20.3 ) require ( diff --git a/go.sum b/go.sum index 89455d6..75f14ae 100644 --- a/go.sum +++ b/go.sum @@ -184,8 +184,8 @@ github.com/teamwork/desksdkgo v1.0.1 h1:Mi5D1pnGfL5JwD7s7g7OyHml7Y9jsak5MNJaotJt github.com/teamwork/desksdkgo v1.0.1/go.mod h1:Mgvw83q8iqHr7Sm9xV1iI/T89o3ObaPU3ChMJheRzwA= github.com/teamwork/spacessdkgo v0.0.0-20260518181558-a6af69d00abb h1:bQluDjySZeC5etnWgjk4WFRy0PvzGDw8XEBd4JJYWCQ= github.com/teamwork/spacessdkgo v0.0.0-20260518181558-a6af69d00abb/go.mod h1:jfE0RLsZuk/3Glzs5bJ95pNb92emV7uXZYgoGSLQ76I= -github.com/teamwork/twapi-go-sdk v1.20.2 h1:wO9c28OXtwDXjeAZB8u44OkotKsAaaBVgO8B+9hYfew= -github.com/teamwork/twapi-go-sdk v1.20.2/go.mod h1:uj6BNtyyKtggfeY3whzn8bLWuhP1twhghjWD6z3h3F4= +github.com/teamwork/twapi-go-sdk v1.20.3 h1:W4V2adgnrdCqdAupUgm1ElurmFpaND8fOUI/SuH2gBU= +github.com/teamwork/twapi-go-sdk v1.20.3/go.mod h1:uj6BNtyyKtggfeY3whzn8bLWuhP1twhghjWD6z3h3F4= github.com/tinylib/msgp v1.6.3 h1:bCSxiTz386UTgyT1i0MSCvdbWjVW+8sG3PjkGsZQt4s= github.com/tinylib/msgp v1.6.3/go.mod h1:RSp0LW9oSxFut3KzESt5Voq4GVWyS+PSulT77roAqEA= github.com/tklauser/go-sysconf v0.3.16 h1:frioLaCQSsF5Cy1jgRBrzr6t502KIIwQ0MArYICU0nA= diff --git a/internal/testutil/mcp.go b/internal/testutil/mcp.go index d94d009..423bb33 100644 --- a/internal/testutil/mcp.go +++ b/internal/testutil/mcp.go @@ -183,6 +183,42 @@ func ProjectsMCPServerRoutedMock( return mcpServer } +// ProjectsMCPServerRoutedMockWithRequestBody is like ProjectsMCPServerRoutedMock +// but also captures the body of the most recent HTTP request that carried one, +// so tests can assert on the serialized payload of the final write while still +// serving distinct responses per endpoint (e.g. a field-type GET at 200 +// followed by a value POST at 201). +func ProjectsMCPServerRoutedMockWithRequestBody( + t *testing.T, + routes []ProjectsMockRoute, + fallbackStatus int, + fallbackBody []byte, +) (*mcp.Server, *[]byte) { + t.Helper() + + var lastBody []byte + engine := twapi.NewEngine(ProjectsSessionMock{}, twapi.WithMiddleware(func(twapi.HTTPClient) twapi.HTTPClient { + return twapi.HTTPClientFunc(func(req *http.Request) (*http.Response, error) { + if req.Body != nil { + body, err := io.ReadAll(req.Body) + if err != nil { + return nil, err + } + lastBody = body + } + path := req.URL.Path + for _, route := range routes { + if strings.Contains(path, route.Match) { + return newProjectsMockHTTPResponse(route.Status, route.Body), nil + } + } + return newProjectsMockHTTPResponse(fallbackStatus, fallbackBody), nil + }) + })) + + return projectsMCPServer(t, engine), &lastBody +} + // ProjectsMCPServerSequencedMock creates a mock MCP server for twprojects // testing whose engine returns the given response bodies in order, one per HTTP // request the engine makes. Once the sequence is exhausted the final body is diff --git a/internal/twprojects/custom_field_values.go b/internal/twprojects/custom_field_values.go index 0fb3368..b32681b 100644 --- a/internal/twprojects/custom_field_values.go +++ b/internal/twprojects/custom_field_values.go @@ -5,7 +5,9 @@ import ( "encoding/json" "fmt" "io" + "math" "net/http" + "strconv" "github.com/google/jsonschema-go/jsonschema" "github.com/modelcontextprotocol/go-sdk/mcp" @@ -79,11 +81,9 @@ func CustomFieldValueCreate(engine *twapi.Engine) toolsets.ToolWrapper { Description: "The ID of the custom field the value belongs to.", }, "value": { - Description: "The value to assign to the custom field. " + - "The concrete type depends on the custom field definition: " + - "strings for text fields, numbers for number fields, booleans for checkboxes, " + - "option IDs for dropdown fields (array of integers for multiselect), " + - "ISO-8601 date strings for date fields.", + Description: "The value to assign, typed per the field: string (text), " + + "number (number), boolean (checkbox), choice value string " + + "(dropdown/status; array for multiselect), ISO-8601 string (date).", AnyOf: []*jsonschema.Schema{ {Type: "string"}, {Type: "number"}, @@ -91,7 +91,10 @@ func CustomFieldValueCreate(engine *twapi.Engine) toolsets.ToolWrapper { { Type: "array", Items: &jsonschema.Schema{ - Type: "string", + AnyOf: []*jsonschema.Schema{ + {Type: "string"}, + {Type: "number"}, + }, }, }, {Type: "null"}, @@ -145,6 +148,7 @@ func CustomFieldValueCreate(engine *twapi.Engine) toolsets.ToolWrapper { if !ok { return helpers.NewToolResultTextError("invalid parameters: 'value' is required"), nil } + value = coerceCustomFieldValue(ctx, engine, customFieldID, value) var customFieldValueCreateRequest projects.CustomFieldValueCreateRequest switch entity { @@ -203,11 +207,9 @@ func CustomFieldValueUpdate(engine *twapi.Engine) toolsets.ToolWrapper { Description: "The ID of the custom field the value belongs to.", }, "value": { - Description: "The value to assign to the custom field. " + - "The concrete type depends on the custom field definition: " + - "strings for text fields, numbers for number fields, booleans for checkboxes, " + - "option IDs for dropdown fields (array of integers for multiselect), " + - "ISO-8601 date strings for date fields.", + Description: "The value to assign, typed per the field: string (text), " + + "number (number), boolean (checkbox), choice value string " + + "(dropdown/status; array for multiselect), ISO-8601 string (date).", AnyOf: []*jsonschema.Schema{ {Type: "string"}, {Type: "number"}, @@ -215,7 +217,10 @@ func CustomFieldValueUpdate(engine *twapi.Engine) toolsets.ToolWrapper { { Type: "array", Items: &jsonschema.Schema{ - Type: "string", + AnyOf: []*jsonschema.Schema{ + {Type: "string"}, + {Type: "number"}, + }, }, }, {Type: "null"}, @@ -267,6 +272,7 @@ func CustomFieldValueUpdate(engine *twapi.Engine) toolsets.ToolWrapper { } value := arguments["value"] + value = coerceCustomFieldValue(ctx, engine, customFieldID, value) var customFieldValueUpdateRequest projects.CustomFieldValueUpdateRequest switch entity { @@ -574,3 +580,93 @@ func CustomFieldValueList(engine *twapi.Engine) toolsets.ToolWrapper { }, } } + +// coerceCustomFieldValue normalizes the raw value into the wire shape the +// Teamwork API expects. Dropdown and multiselect custom fields store their +// choices as strings, but MCP clients frequently send a choice as a JSON +// number (the tool schema also permits numbers, for number-type fields). A +// numeric value posted to a dropdown field is rejected by the API with +// "cannot unmarshal number ... into string", so when the raw value carries a +// number we resolve the field type and stringify it for dropdown/multiselect +// fields. Every other case is passed through untouched; if the field type +// cannot be resolved we forward the value unchanged and let the API validate. +func coerceCustomFieldValue(ctx context.Context, engine *twapi.Engine, customFieldID int64, raw any) any { + if customFieldID == 0 || !containsNumber(raw) { + return raw + } + resp, err := projects.CustomFieldGet(ctx, engine, projects.NewCustomFieldGetRequest(customFieldID)) + if err != nil { + return raw + } + // Dropdown, status and multiselect fields store their choices as strings, + // so a numeric choice must be stringified. Dropdown and status are + // single-valued; multiselect is an array. + switch resp.CustomField.Type { + case projects.CustomFieldTypeDropdown, projects.CustomFieldTypeStatus: + if s, ok := stringifyScalar(raw); ok { + return s + } + case projects.CustomFieldTypeMultiselect: + if arr, ok := raw.([]any); ok { + out := make([]any, len(arr)) + for i, v := range arr { + if s, ok := stringifyScalar(v); ok { + out[i] = s + } else { + out[i] = v + } + } + return out + } + if s, ok := stringifyScalar(raw); ok { + return s + } + } + return raw +} + +// containsNumber reports whether raw is a JSON number, or an array holding at +// least one number. It is the cheap pre-check that lets coerceCustomFieldValue +// skip the field lookup for the common case of already-string values. +func containsNumber(raw any) bool { + switch v := raw.(type) { + case float64, float32, int, int32, int64, json.Number: + return true + case []any: + for _, e := range v { + if containsNumber(e) { + return true + } + } + } + return false +} + +// stringifyScalar renders a scalar JSON value as the string the API expects for +// dropdown choices. Integer-valued numbers are rendered without a trailing +// decimal (1 -> "1", not "1.000000"). It reports false for values that have no +// meaningful scalar string form (e.g. arrays or objects). +func stringifyScalar(raw any) (string, bool) { + switch v := raw.(type) { + case string: + return v, true + case float64: + if !math.IsInf(v, 0) && !math.IsNaN(v) && v == math.Trunc(v) { + return strconv.FormatInt(int64(v), 10), true + } + return strconv.FormatFloat(v, 'f', -1, 64), true + case float32: + return stringifyScalar(float64(v)) + case int: + return strconv.FormatInt(int64(v), 10), true + case int32: + return strconv.FormatInt(int64(v), 10), true + case int64: + return strconv.FormatInt(v, 10), true + case bool: + return strconv.FormatBool(v), true + case json.Number: + return v.String(), true + } + return "", false +} diff --git a/internal/twprojects/custom_field_values_test.go b/internal/twprojects/custom_field_values_test.go index 1d7381c..c380268 100644 --- a/internal/twprojects/custom_field_values_test.go +++ b/internal/twprojects/custom_field_values_test.go @@ -2,6 +2,7 @@ package twprojects_test import ( "net/http" + "strings" "testing" "github.com/teamwork/mcp/internal/testutil" @@ -38,6 +39,95 @@ func TestCustomFieldValueCreateMultiselect(t *testing.T) { }) } +func TestCustomFieldValueCreateDropdownCoercesNumberToString(t *testing.T) { + // A numeric value bound for a dropdown field must be stringified: the API + // stores dropdown choices as strings and rejects a raw number. The create + // handler resolves the field type via a GET (200) before the value POST + // (201), so route by URL to give each leg the right status. + mcpServer, body := testutil.ProjectsMCPServerRoutedMockWithRequestBody(t, []testutil.ProjectsMockRoute{ + {Match: "/customfields/555", Status: http.StatusOK, Body: []byte(`{"customfield":{"id":555,"type":"dropdown"}}`)}, + }, http.StatusCreated, []byte(`{"customfieldTask":{"id":123}}`)) + testutil.ExecuteToolRequest(t, mcpServer, twprojects.MethodCustomFieldValueCreate.String(), map[string]any{ + "entity": "task", + "entity_id": float64(777), + "custom_field_id": float64(555), + "value": float64(1), + }) + if got := string(*body); !strings.Contains(got, `"value":"1"`) { + t.Errorf("expected posted value to be stringified as \"1\", got body: %s", got) + } +} + +func TestCustomFieldValueCreateNumberFieldKeepsNumber(t *testing.T) { + // A numeric value bound for a number field must stay a number, not be + // stringified — only dropdown/multiselect choices are coerced. + mcpServer, body := testutil.ProjectsMCPServerRoutedMockWithRequestBody(t, []testutil.ProjectsMockRoute{ + {Match: "/customfields/555", Status: http.StatusOK, Body: []byte(`{"customfield":{"id":555,"type":"number-integer"}}`)}, + }, http.StatusCreated, []byte(`{"customfieldTask":{"id":123}}`)) + testutil.ExecuteToolRequest(t, mcpServer, twprojects.MethodCustomFieldValueCreate.String(), map[string]any{ + "entity": "task", + "entity_id": float64(777), + "custom_field_id": float64(555), + "value": float64(42), + }) + if got := string(*body); !strings.Contains(got, `"value":42`) { + t.Errorf("expected posted value to remain the number 42, got body: %s", got) + } +} + +func TestCustomFieldValueCreateStatusCoercesNumberToString(t *testing.T) { + // Status fields are choice-based strings too, so a numeric value must be + // stringified just like a dropdown. + mcpServer, body := testutil.ProjectsMCPServerRoutedMockWithRequestBody(t, []testutil.ProjectsMockRoute{ + {Match: "/customfields/555", Status: http.StatusOK, Body: []byte(`{"customfield":{"id":555,"type":"status"}}`)}, + }, http.StatusCreated, []byte(`{"customfieldTask":{"id":123}}`)) + testutil.ExecuteToolRequest(t, mcpServer, twprojects.MethodCustomFieldValueCreate.String(), map[string]any{ + "entity": "task", + "entity_id": float64(777), + "custom_field_id": float64(555), + "value": float64(5), + }) + if got := string(*body); !strings.Contains(got, `"value":"5"`) { + t.Errorf("expected posted value to be stringified as \"5\", got body: %s", got) + } +} + +func TestCustomFieldValueUpdateDropdownCoercesNumberToString(t *testing.T) { + // Same coercion applies on update. The field-type GET (custom_field_id=555) + // and the value PATCH (value_id=123) both live under /customfields/, so the + // route matches the field id specifically and the PATCH falls through. + mcpServer, body := testutil.ProjectsMCPServerRoutedMockWithRequestBody(t, []testutil.ProjectsMockRoute{ + {Match: "/customfields/555", Status: http.StatusOK, Body: []byte(`{"customfield":{"id":555,"type":"dropdown"}}`)}, + }, http.StatusOK, []byte(`{"customfieldTask":{"id":123}}`)) + testutil.ExecuteToolRequest(t, mcpServer, twprojects.MethodCustomFieldValueUpdate.String(), map[string]any{ + "entity": "task", + "entity_id": float64(777), + "value_id": float64(123), + "custom_field_id": float64(555), + "value": float64(3), + }) + if got := string(*body); !strings.Contains(got, `"value":"3"`) { + t.Errorf("expected patched value to be stringified as \"3\", got body: %s", got) + } +} + +func TestCustomFieldValueCreateMultiselectCoercesNumbersToStrings(t *testing.T) { + // Multiselect values arrive as an array; each numeric element must be + // stringified to match the stored choice values. + mcpServer, body := testutil.ProjectsMCPServerRoutedMockWithRequestBody(t, []testutil.ProjectsMockRoute{ + {Match: "/customfields/555", Status: http.StatusOK, Body: []byte(`{"customfield":{"id":555,"type":"multiselect"}}`)}, + }, http.StatusCreated, []byte(`{"customfieldTask":{"id":123}}`)) + testutil.ExecuteToolRequest(t, mcpServer, twprojects.MethodCustomFieldValueCreate.String(), map[string]any{ + "entity": "task", + "entity_id": float64(777), + "custom_field_id": float64(555), + "value": []any{float64(10), float64(20)}, + }) + if got := string(*body); !strings.Contains(got, `"value":["10","20"]`) { + t.Errorf("expected posted value to be stringified array, got body: %s", got) + } +} + func TestCustomFieldValueUpdate(t *testing.T) { mcpServer := mcpServerMock(t, http.StatusOK, []byte(`{"customfieldTask":{"id":123}}`)) testutil.ExecuteToolRequest(t, mcpServer, twprojects.MethodCustomFieldValueUpdate.String(), map[string]any{