diff --git a/README.md b/README.md index bd0963abf..fc8fae03e 100644 --- a/README.md +++ b/README.md @@ -707,13 +707,6 @@ The following sets of tools are available (all are on by default): - `per_page`: Number of results per page (max 100, default: 30) (number, optional) - `query`: Filter projects by a search query (matches title and description) (string, optional) -- **update_project_item** - Update project item - - `fields`: A list of field updates to apply. (array, required) - - `item_id`: The numeric ID of the project item to update (not the issue or pull request ID). (number, required) - - `owner`: If owner_type == user it is the handle for the GitHub user account. If owner_type == org it is the name of the organization. The name is not case sensitive. (string, required) - - `owner_type`: Owner type (string, required) - - `project_number`: The project's number. (number, required) -
diff --git a/go.mod b/go.mod index 73a043f8c..61b4b971a 100644 --- a/go.mod +++ b/go.mod @@ -31,7 +31,7 @@ require ( github.com/fsnotify/fsnotify v1.8.0 // indirect github.com/go-viper/mapstructure/v2 v2.4.0 github.com/google/go-github/v71 v71.0.0 // indirect - github.com/google/go-querystring v1.1.0 // indirect + github.com/google/go-querystring v1.1.0 github.com/google/uuid v1.6.0 // indirect github.com/gorilla/mux v1.8.0 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect diff --git a/pkg/github/projects.go b/pkg/github/projects.go index 09bcbd5ed..95b859c03 100644 --- a/pkg/github/projects.go +++ b/pkg/github/projects.go @@ -621,138 +621,6 @@ func DeleteProjectItem(getClient GetClientFn, t translations.TranslationHelperFu } } -func UpdateProjectItem(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { - return mcp.NewTool("update_project_item", - mcp.WithDescription(t("TOOL_UPDATE_PROJECT_ITEM_DESCRIPTION", "Update a specific Project item for a user or org")), - mcp.WithToolAnnotation(mcp.ToolAnnotation{Title: t("TOOL_UPDATE_PROJECT_ITEM_USER_TITLE", "Update project item"), ReadOnlyHint: ToBoolPtr(false)}), - mcp.WithString("owner_type", mcp.Required(), mcp.Description("Owner type"), mcp.Enum("user", "org")), - mcp.WithString("owner", mcp.Required(), mcp.Description("If owner_type == user it is the handle for the GitHub user account. If owner_type == org it is the name of the organization. The name is not case sensitive.")), - mcp.WithNumber("project_number", mcp.Required(), mcp.Description("The project's number.")), - mcp.WithNumber("item_id", mcp.Required(), mcp.Description("The numeric ID of the project item to update (not the issue or pull request ID).")), - mcp.WithArray("fields", mcp.Required(), mcp.Description("A list of field updates to apply.")), - ), func(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) { - owner, err := RequiredParam[string](req, "owner") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - ownerType, err := RequiredParam[string](req, "owner_type") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - projectNumber, err := RequiredInt(req, "project_number") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - itemID, err := RequiredInt(req, "item_id") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - client, err := getClient(ctx) - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - fieldsParam, ok := req.GetArguments()["fields"] - if !ok { - return mcp.NewToolResultError("missing required parameter: fields"), nil - } - - rawFields, ok := fieldsParam.([]any) - if !ok { - return mcp.NewToolResultError("parameter fields must be an array of objects"), nil - } - if len(rawFields) == 0 { - return mcp.NewToolResultError("fields must contain at least one field update"), nil - } - - var projectsURL string - if ownerType == "org" { - projectsURL = fmt.Sprintf("orgs/%s/projectsV2/%d/items/%d", owner, projectNumber, itemID) - } else { - projectsURL = fmt.Sprintf("users/%s/projectsV2/%d/items/%d", owner, projectNumber, itemID) - } - - updateFields := make([]*newProjectV2Field, 0, len(rawFields)) - for idx, rawField := range rawFields { - fieldMap, ok := rawField.(map[string]any) - if !ok { - return mcp.NewToolResultError(fmt.Sprintf("fields[%d] must be an object", idx)), nil - } - - rawID, ok := fieldMap["id"] - if !ok { - return mcp.NewToolResultError(fmt.Sprintf("fields[%d] is missing 'id'", idx)), nil - } - - var fieldID int64 - switch v := rawID.(type) { - case float64: - fieldID = int64(v) - case int64: - fieldID = v - case json.Number: - n, convErr := v.Int64() - if convErr != nil { - return mcp.NewToolResultError(fmt.Sprintf("fields[%d].id must be a numeric value", idx)), nil - } - fieldID = n - default: - return mcp.NewToolResultError(fmt.Sprintf("fields[%d].id must be a numeric value", idx)), nil - } - - value, ok := fieldMap["value"] - if !ok { - return mcp.NewToolResultError(fmt.Sprintf("fields[%d] is missing 'value'", idx)), nil - } - - updateFields = append(updateFields, &newProjectV2Field{ - ID: github.Ptr(fieldID), - Value: value, - }) - } - - updateProjectItemOptions := &updateProjectItemOptions{Fields: updateFields} - - httpRequest, err := client.NewRequest("PATCH", projectsURL, updateProjectItemOptions) - if err != nil { - return nil, fmt.Errorf("failed to create request: %w", err) - } - - updatedItem := projectV2Item{} - resp, err := client.Do(ctx, httpRequest, &updatedItem) - if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, - "failed to update a project item", - resp, - err, - ), nil - } - defer func() { _ = resp.Body.Close() }() - - if resp.StatusCode != http.StatusOK { - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read response body: %w", err) - } - return mcp.NewToolResultError(fmt.Sprintf("failed to update a project item: %s", string(body))), nil - } - r, err := json.Marshal(convertToMinimalProjectItem(&updatedItem)) - if err != nil { - return nil, fmt.Errorf("failed to marshal response: %w", err) - } - - return mcp.NewToolResultText(string(r)), nil - } -} - -type updateProjectItemOptions struct { - Fields []*newProjectV2Field `json:"fields,omitempty"` -} - -type newProjectV2Field struct { - ID *int64 `json:"id,omitempty"` - Value any `json:"value,omitempty"` -} - type newProjectItem struct { ID int64 `json:"id,omitempty"` // Issue or Pull Request ID to add to the project. Type string `json:"type,omitempty"` diff --git a/pkg/github/projects_test.go b/pkg/github/projects_test.go index 628bad8fb..19f23510b 100644 --- a/pkg/github/projects_test.go +++ b/pkg/github/projects_test.go @@ -1311,238 +1311,3 @@ func Test_DeleteProjectItem(t *testing.T) { }) } } - -func Test_UpdateProjectItem(t *testing.T) { - mockClient := gh.NewClient(nil) - tool, _ := UpdateProjectItem(stubGetClientFn(mockClient), translations.NullTranslationHelper) - require.NoError(t, toolsnaps.Test(tool.Name, tool)) - - assert.Equal(t, "update_project_item", tool.Name) - assert.NotEmpty(t, tool.Description) - assert.Contains(t, tool.InputSchema.Properties, "owner_type") - assert.Contains(t, tool.InputSchema.Properties, "owner") - assert.Contains(t, tool.InputSchema.Properties, "project_number") - assert.Contains(t, tool.InputSchema.Properties, "item_id") - assert.Contains(t, tool.InputSchema.Properties, "fields") - assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner_type", "owner", "project_number", "item_id", "fields"}) - - orgUpdated := map[string]any{ - "id": 801, - "content_type": "Issue", - "creator": map[string]any{"login": "octocat"}, - } - userUpdated := map[string]any{ - "id": 901, - "content_type": "PullRequest", - "creator": map[string]any{"login": "hubot"}, - } - - tests := []struct { - name string - mockedClient *http.Client - requestArgs map[string]any - expectError bool - expectedErrMsg string - expectedID int - expectedCreatorLogin string - }{ - { - name: "success organization update", - mockedClient: mock.NewMockedHTTPClient( - mock.WithRequestMatchHandler( - mock.EndpointPattern{Pattern: "/orgs/{org}/projectsV2/{project}/items/{item_id}", Method: http.MethodPatch}, - http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - body, err := io.ReadAll(r.Body) - assert.NoError(t, err) - var payload struct { - Fields []struct { - ID int `json:"id"` - Value interface{} `json:"value"` - } `json:"fields"` - } - assert.NoError(t, json.Unmarshal(body, &payload)) - assert.Len(t, payload.Fields, 1) - if len(payload.Fields) == 1 { - assert.Equal(t, 123, payload.Fields[0].ID) - assert.Equal(t, "In Progress", payload.Fields[0].Value) - } - w.WriteHeader(http.StatusOK) - _, _ = w.Write(mock.MustMarshal(orgUpdated)) - }), - ), - ), - requestArgs: map[string]any{ - "owner": "octo-org", - "owner_type": "org", - "project_number": float64(111), - "item_id": float64(2222), - "fields": []any{ - map[string]any{"id": float64(123), "value": "In Progress"}, - }, - }, - expectedID: 801, - expectedCreatorLogin: "octocat", - }, - { - name: "success user update", - mockedClient: mock.NewMockedHTTPClient( - mock.WithRequestMatchHandler( - mock.EndpointPattern{Pattern: "/users/{user}/projectsV2/{project}/items/{item_id}", Method: http.MethodPatch}, - http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - body, err := io.ReadAll(r.Body) - assert.NoError(t, err) - var payload map[string]any - assert.NoError(t, json.Unmarshal(body, &payload)) - fields, ok := payload["fields"].([]any) - assert.True(t, ok) - assert.Len(t, fields, 1) - w.WriteHeader(http.StatusOK) - _, _ = w.Write(mock.MustMarshal(userUpdated)) - }), - ), - ), - requestArgs: map[string]any{ - "owner": "octocat", - "owner_type": "user", - "project_number": float64(222), - "item_id": float64(3333), - "fields": []any{ - map[string]any{"id": float64(456), "value": 42}, - }, - }, - expectedID: 901, - expectedCreatorLogin: "hubot", - }, - { - name: "api error", - mockedClient: mock.NewMockedHTTPClient( - mock.WithRequestMatchHandler( - mock.EndpointPattern{Pattern: "/orgs/{org}/projectsV2/{project}/items/{item_id}", Method: http.MethodPatch}, - mockResponse(t, http.StatusInternalServerError, map[string]string{"message": "boom"}), - ), - ), - requestArgs: map[string]any{ - "owner": "octo-org", - "owner_type": "org", - "project_number": float64(333), - "item_id": float64(4444), - "fields": []any{ - map[string]any{"id": float64(789), "value": "Done"}, - }, - }, - expectError: true, - expectedErrMsg: "failed to update a project item", - }, - { - name: "missing owner", - mockedClient: mock.NewMockedHTTPClient(), - requestArgs: map[string]any{ - "owner_type": "org", - "project_number": float64(1), - "item_id": float64(1), - "fields": []any{map[string]any{"id": float64(1), "value": "X"}}, - }, - expectError: true, - }, - { - name: "missing owner_type", - mockedClient: mock.NewMockedHTTPClient(), - requestArgs: map[string]any{ - "owner": "octo-org", - "project_number": float64(1), - "item_id": float64(1), - "fields": []any{map[string]any{"id": float64(1), "value": "X"}}, - }, - expectError: true, - }, - { - name: "missing project_number", - mockedClient: mock.NewMockedHTTPClient(), - requestArgs: map[string]any{ - "owner": "octo-org", - "owner_type": "org", - "item_id": float64(1), - "fields": []any{map[string]any{"id": float64(1), "value": "X"}}, - }, - expectError: true, - }, - { - name: "missing item_id", - mockedClient: mock.NewMockedHTTPClient(), - requestArgs: map[string]any{ - "owner": "octo-org", - "owner_type": "org", - "project_number": float64(1), - "fields": []any{map[string]any{"id": float64(1), "value": "X"}}, - }, - expectError: true, - }, - { - name: "missing fields", - mockedClient: mock.NewMockedHTTPClient(), - requestArgs: map[string]any{ - "owner": "octo-org", - "owner_type": "org", - "project_number": float64(1), - "item_id": float64(1), - }, - expectError: true, - expectedErrMsg: "missing required parameter: fields", - }, - { - name: "empty fields", - mockedClient: mock.NewMockedHTTPClient(), - requestArgs: map[string]any{ - "owner": "octo-org", - "owner_type": "org", - "project_number": float64(1), - "item_id": float64(1), - "fields": []any{}, - }, - expectError: true, - expectedErrMsg: "fields must contain at least one field update", - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - client := gh.NewClient(tc.mockedClient) - _, handler := UpdateProjectItem(stubGetClientFn(client), translations.NullTranslationHelper) - request := createMCPRequest(tc.requestArgs) - result, err := handler(context.Background(), request) - - require.NoError(t, err) - if tc.expectError { - require.True(t, result.IsError) - text := getTextResult(t, result).Text - if tc.expectedErrMsg != "" { - assert.Contains(t, text, tc.expectedErrMsg) - } - switch tc.name { - case "missing owner": - assert.Contains(t, text, "missing required parameter: owner") - case "missing owner_type": - assert.Contains(t, text, "missing required parameter: owner_type") - case "missing project_number": - assert.Contains(t, text, "missing required parameter: project_number") - case "missing item_id": - assert.Contains(t, text, "missing required parameter: item_id") - } - return - } - - require.False(t, result.IsError) - textContent := getTextResult(t, result) - var item map[string]any - require.NoError(t, json.Unmarshal([]byte(textContent.Text), &item)) - if tc.expectedID != 0 { - assert.Equal(t, float64(tc.expectedID), item["id"]) - } - if tc.expectedCreatorLogin != "" { - creator, ok := item["creator"].(map[string]any) - require.True(t, ok) - assert.Equal(t, tc.expectedCreatorLogin, creator["login"]) - } - }) - } -} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index dec0a9e37..147d2347d 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -202,7 +202,6 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG AddWriteTools( toolsets.NewServerTool(AddProjectItem(getClient, t)), toolsets.NewServerTool(DeleteProjectItem(getClient, t)), - toolsets.NewServerTool(UpdateProjectItem(getClient, t)), ) // Add toolsets to the group