Skip to content

Change schema of fields #1166

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
JoannaaKL wants to merge 1 commit into from
Closed

Change schema of fields #1166

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -708,7 +708,7 @@ The following sets of tools are available (all are on by default):
- `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)
- `fields`: A list of field updates to apply. (object[], 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)
Expand Down
18 changes: 18 additions & 0 deletions pkg/github/__toolsnaps__/update_project_item.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,24 @@
"properties": {
"fields": {
"description": "A list of field updates to apply.",
"items": {
"additionalProperties": false,
"properties": {
"id": {
"description": "ID of the project field",
"type": "int"
},
"value": {
"description": "value of the project field",
"type": "any"
}
},
"required": [
"id",
"value"
],
"type": "object"
},
"type": "array"
},
"item_id": {
Expand Down
106 changes: 65 additions & 41 deletions pkg/github/projects.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -624,12 +624,44 @@ 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.")),
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."),
mcp.Items(
map[string]interface{}{
"type": "object",
"additionalProperties": false,
"required": []string{"id", "value"},
"properties": map[string]interface{}{
"id": map[string]interface{}{
"type": "int",
"description": "ID of the project field",
},
"value": map[string]interface{}{
"type": "any",
// intentionally left without a specific JSON schema type to allow any value
"description": "value of the project field",
},
},
}),
),
), func(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) {
owner, err := RequiredParam[string](req, "owner")
if err != nil {
Expand All @@ -651,16 +683,18 @@ func UpdateProjectItem(getClient GetClientFn, t translations.TranslationHelperFu
if err != nil {
return mcp.NewToolResultError(err.Error()), nil
}
fieldsParam, ok := req.GetArguments()["fields"]
if !ok {

// Extract and validate fields parameter (required & non-empty)
args := req.GetArguments()
rawFields, present := args["fields"]
if !present {
return mcp.NewToolResultError("missing required parameter: fields"), nil
}

rawFields, ok := fieldsParam.([]any)
fields, ok := rawFields.([]interface{})
if !ok {
return mcp.NewToolResultError("parameter fields must be an array of objects"), nil
return mcp.NewToolResultError("fields parameter must be an array of objects with id and value"), nil
}
if len(rawFields) == 0 {
if len(fields) == 0 {
return mcp.NewToolResultError("fields must contain at least one field update"), nil
}

Expand All @@ -671,43 +705,32 @@ func UpdateProjectItem(getClient GetClientFn, t translations.TranslationHelperFu
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)
updateFields := make([]*newProjectV2Field, 0, len(fields))
for _, rawField := range fields {
m, ok := rawField.(map[string]any)
if !ok {
return mcp.NewToolResultError(fmt.Sprintf("fields[%d] must be an object", idx)), nil
return mcp.NewToolResultError("each element of fields must be an object"), nil
}

rawID, ok := fieldMap["id"]
idVal, ok := m["id"]
if !ok {
return mcp.NewToolResultError(fmt.Sprintf("fields[%d] is missing 'id'", idx)), nil
return mcp.NewToolResultError("each field update must include an 'id'"), nil
}

var fieldID int64
switch v := rawID.(type) {
var idInt64 int64
switch v := idVal.(type) {
case float64:
fieldID = int64(v)
idInt64 = int64(v)
case int:
idInt64 = 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
idInt64 = v
default:
return mcp.NewToolResultError(fmt.Sprintf("fields[%d].id must be a numeric value", idx)), nil
return mcp.NewToolResultError("field 'id' must be a number"), nil
}

value, ok := fieldMap["value"]
value, ok := m["value"]
if !ok {
return mcp.NewToolResultError(fmt.Sprintf("fields[%d] is missing 'value'", idx)), nil
return mcp.NewToolResultError("each field update must include a 'value'"), nil
}

updateFields = append(updateFields, &newProjectV2Field{
ID: github.Ptr(fieldID),
Value: value,
})
updateFields = append(updateFields, &newProjectV2Field{ID: github.Ptr(idInt64), Value: value})
}

updateProjectItemOptions := &updateProjectItemOptions{Fields: updateFields}
Expand Down Expand Up @@ -749,8 +772,9 @@ type updateProjectItemOptions struct {
}

type newProjectV2Field struct {
ID *int64 `json:"id,omitempty"`
Value any `json:"value,omitempty"`
ID *int64 `json:"id,omitempty"`
// Value should be sent as explicit null if user supplies null, so do NOT use omitempty
Value any `json:"value"`
}

type newProjectItem struct {
Expand Down
106 changes: 106 additions & 0 deletions pkg/github/projects_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1383,6 +1383,82 @@ func Test_UpdateProjectItem(t *testing.T) {
expectedID: 801,
expectedCreatorLogin: "octocat",
},
{
name: "success organization multi-field 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, 2)
if len(payload.Fields) == 2 {
assert.Equal(t, 1111, payload.Fields[0].ID)
assert.Equal(t, "Backlog", payload.Fields[0].Value)
assert.Equal(t, 2222, payload.Fields[1].ID)
assert.Equal(t, 5.0, payload.Fields[1].Value) // JSON numbers -> float64
}
w.WriteHeader(http.StatusOK)
_, _ = w.Write(mock.MustMarshal(orgUpdated))
}),
),
),
requestArgs: map[string]any{
"owner": "octo-org",
"owner_type": "org",
"project_number": float64(9999),
"item_id": float64(8888),
"fields": []any{
map[string]any{"id": float64(1111), "value": "Backlog"},
map[string]any{"id": float64(2222), "value": float64(5)},
},
},
expectedID: 801,
expectedCreatorLogin: "octocat",
},
{
name: "success organization update null value",
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 raw map[string]any
assert.NoError(t, json.Unmarshal(body, &raw))
fieldsRaw, ok := raw["fields"].([]any)
assert.True(t, ok)
assert.Len(t, fieldsRaw, 1)
first, ok := fieldsRaw[0].(map[string]any)
assert.True(t, ok)
// Ensure key exists and is explicitly null (presence matters)
val, present := first["value"]
assert.True(t, present)
assert.Nil(t, val)
w.WriteHeader(http.StatusOK)
_, _ = w.Write(mock.MustMarshal(orgUpdated))
}),
),
),
requestArgs: map[string]any{
"owner": "octo-org",
"owner_type": "org",
"project_number": float64(1010),
"item_id": float64(2020),
"fields": []any{
map[string]any{"id": float64(999), "value": nil},
},
},
expectedID: 801,
expectedCreatorLogin: "octocat",
},
{
name: "success user update",
mockedClient: mock.NewMockedHTTPClient(
Expand Down Expand Up @@ -1502,6 +1578,36 @@ func Test_UpdateProjectItem(t *testing.T) {
expectError: true,
expectedErrMsg: "fields must contain at least one field update",
},
{
name: "field missing id",
mockedClient: mock.NewMockedHTTPClient(),
requestArgs: map[string]any{
"owner": "octo-org",
"owner_type": "org",
"project_number": float64(1),
"item_id": float64(1),
"fields": []any{
map[string]any{"value": "X"},
},
},
expectError: true,
expectedErrMsg: "each field update must include an 'id'",
},
{
name: "field missing value",
mockedClient: mock.NewMockedHTTPClient(),
requestArgs: map[string]any{
"owner": "octo-org",
"owner_type": "org",
"project_number": float64(1),
"item_id": float64(1),
"fields": []any{
map[string]any{"id": float64(55)},
},
},
expectError: true,
expectedErrMsg: "each field update must include a 'value'",
},
}

for _, tc := range tests {
Expand Down
Loading