From 15ca2eecb8348121a92518e54574eeca5abd4d86 Mon Sep 17 00:00:00 2001 From: Lizeth Vera <47796851+veralizeth@users.noreply.github.com> Date: Sat, 18 Jul 2026 00:36:22 +0000 Subject: [PATCH] Adding an initial draft of batch/bulk field-value writes --- README.md | 3 +- pkg/github/__toolsnaps__/projects_write.snap | 33 +- pkg/github/projects.go | 422 ++++++++++++++++++- pkg/github/projects_resolver_test.go | 203 +++++++++ pkg/github/projects_test.go | 236 +++++++++++ 5 files changed, 893 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 10d987a262..512e9084af 100644 --- a/README.md +++ b/README.md @@ -1101,6 +1101,7 @@ The following sets of tools are available: - `item_owner`: The owner (user or organization) of the repository containing the issue or pull request. Required for 'add_project_item' method. Also accepted by 'update_project_item' when resolving the item by issue number. (string, optional) - `item_repo`: The name of the repository containing the issue or pull request. Required for 'add_project_item' method. Also accepted by 'update_project_item' when resolving the item by issue number. (string, optional) - `item_type`: The item's type, either issue or pull_request. Required for 'add_project_item' method. (string, optional) + - `items`: The set of items to update. Required for 'update_project_items'. Each entry references either an existing item by 'item_id', or by (item_owner + item_repo + issue_number). Each entry may optionally include its own 'updated_field'; if omitted, the top-level 'updated_field' is applied. Limit: 100 items per call. (object[], optional) - `iteration_duration`: Duration in days for iterations of the field (e.g. 7 for weekly, 14 for bi-weekly). Required for 'create_iteration_field' method. (number, optional) - `iterations`: Custom iterations for 'create_iteration_field' method. Only set this when you need iterations with varying durations, breaks between them, or specific titles. Otherwise omit it: GitHub auto-creates three iterations of 'iteration_duration' days starting on 'start_date', which is the right choice for most cases. (object[], optional) - `method`: The method to execute (string, required) @@ -1112,7 +1113,7 @@ The following sets of tools are available: - `status`: The status of the project. Used for 'create_project_status_update' method. (string, optional) - `target_date`: The target date of the status update in YYYY-MM-DD format. Used for 'create_project_status_update' method. (string, optional) - `title`: The project title. Required for 'create_project' method. (string, optional) - - `updated_field`: Object describing the field to update and its new value. Required for 'update_project_item'. Two shapes are accepted: (1) by ID — {"id": 123456, "value": "..."}; (2) by name — {"name": "Status", "value": "In Progress"}. For single-select fields, option-name resolution requires the by-name shape; on the by-ID shape, pass the option ID. Set value to null to clear the field. (object, optional) + - `updated_field`: Object describing the field to update and its new value. Required for 'update_project_item'. For 'update_project_item' provide either a single-item shape at the top level, or override per-item on 'update_project_items'. Two shapes are accepted: (1) by ID — {"id": 123456, "value": "..."}; (2) by name — {"name": "Status", "value": "In Progress"}. For single-select fields, option-name resolution requires the by-name shape; on the by-ID shape, pass the option ID. Set value to null to clear the field. For 'update_project_items', a top-level 'updated_field' is applied to every item that does not supply its own 'updated_field'. (object, optional) diff --git a/pkg/github/__toolsnaps__/projects_write.snap b/pkg/github/__toolsnaps__/projects_write.snap index 762ee08c93..c742b68ed9 100644 --- a/pkg/github/__toolsnaps__/projects_write.snap +++ b/pkg/github/__toolsnaps__/projects_write.snap @@ -40,6 +40,36 @@ ], "type": "string" }, + "items": { + "description": "The set of items to update. Required for 'update_project_items'. Each entry references either an existing item by 'item_id', or by (item_owner + item_repo + issue_number). Each entry may optionally include its own 'updated_field'; if omitted, the top-level 'updated_field' is applied. Limit: 100 items per call.", + "items": { + "additionalProperties": false, + "properties": { + "issue_number": { + "description": "Issue number (or pull request number) used to resolve the project item together with item_owner and item_repo.", + "type": "number" + }, + "item_id": { + "description": "The project item ID. Provide either item_id, or (item_owner + item_repo + issue_number).", + "type": "number" + }, + "item_owner": { + "description": "Owner of the repository containing the issue or pull request. Combine with item_repo and issue_number to resolve the item.", + "type": "string" + }, + "item_repo": { + "description": "Repository containing the issue or pull request. Combine with item_owner and issue_number to resolve the item.", + "type": "string" + }, + "updated_field": { + "description": "Per-item field update. Overrides the top-level 'updated_field'. Same shape as the top-level 'updated_field'.", + "type": "object" + } + }, + "type": "object" + }, + "type": "array" + }, "iteration_duration": { "description": "Duration in days for iterations of the field (e.g. 7 for weekly, 14 for bi-weekly). Required for 'create_iteration_field' method.", "type": "number" @@ -76,6 +106,7 @@ "enum": [ "add_project_item", "update_project_item", + "update_project_items", "delete_project_item", "create_project_status_update", "create_project", @@ -127,7 +158,7 @@ "type": "string" }, "updated_field": { - "description": "Object describing the field to update and its new value. Required for 'update_project_item'. Two shapes are accepted: (1) by ID — {\"id\": 123456, \"value\": \"...\"}; (2) by name — {\"name\": \"Status\", \"value\": \"In Progress\"}. For single-select fields, option-name resolution requires the by-name shape; on the by-ID shape, pass the option ID. Set value to null to clear the field.", + "description": "Object describing the field to update and its new value. Required for 'update_project_item'. For 'update_project_item' provide either a single-item shape at the top level, or override per-item on 'update_project_items'. Two shapes are accepted: (1) by ID — {\"id\": 123456, \"value\": \"...\"}; (2) by name — {\"name\": \"Status\", \"value\": \"In Progress\"}. For single-select fields, option-name resolution requires the by-name shape; on the by-ID shape, pass the option ID. Set value to null to clear the field. For 'update_project_items', a top-level 'updated_field' is applied to every item that does not supply its own 'updated_field'.", "type": "object" } }, diff --git a/pkg/github/projects.go b/pkg/github/projects.go index 308c2b87e8..2df6921d99 100644 --- a/pkg/github/projects.go +++ b/pkg/github/projects.go @@ -8,6 +8,7 @@ import ( "io" "net/http" "strconv" + "strings" "time" ghErrors "github.com/github/github-mcp-server/pkg/errors" @@ -32,6 +33,10 @@ const ( ProjectStatusUpdateCreateFailedError = "failed to create project status update" ProjectResolveIDFailedError = "failed to resolve project ID" MaxProjectsPerPage = 50 + // MaxProjectItemsPerBatch caps the number of items processed in a single + // update_project_items call. This keeps a batch bounded, prevents runaway + // resolution costs, and matches the practical scale of typical triage sweeps. + MaxProjectItemsPerBatch = 100 ) // Method constants for consolidated project tools @@ -44,6 +49,7 @@ const ( projectsMethodGetProjectItem = "get_project_item" projectsMethodAddProjectItem = "add_project_item" projectsMethodUpdateProjectItem = "update_project_item" + projectsMethodUpdateProjectItems = "update_project_items" projectsMethodDeleteProjectItem = "delete_project_item" projectsMethodListProjectStatusUpdates = "list_project_status_updates" projectsMethodGetProjectStatusUpdate = "get_project_status_update" @@ -511,6 +517,7 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { Enum: []any{ projectsMethodAddProjectItem, projectsMethodUpdateProjectItem, + projectsMethodUpdateProjectItems, projectsMethodDeleteProjectItem, projectsMethodCreateProjectStatusUpdate, projectsMethodCreateProject, @@ -561,7 +568,37 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { }, "updated_field": { Type: "object", - Description: "Object describing the field to update and its new value. Required for 'update_project_item'. Two shapes are accepted: (1) by ID — {\"id\": 123456, \"value\": \"...\"}; (2) by name — {\"name\": \"Status\", \"value\": \"In Progress\"}. For single-select fields, option-name resolution requires the by-name shape; on the by-ID shape, pass the option ID. Set value to null to clear the field.", + Description: "Object describing the field to update and its new value. Required for 'update_project_item'. For 'update_project_item' provide either a single-item shape at the top level, or override per-item on 'update_project_items'. Two shapes are accepted: (1) by ID — {\"id\": 123456, \"value\": \"...\"}; (2) by name — {\"name\": \"Status\", \"value\": \"In Progress\"}. For single-select fields, option-name resolution requires the by-name shape; on the by-ID shape, pass the option ID. Set value to null to clear the field. For 'update_project_items', a top-level 'updated_field' is applied to every item that does not supply its own 'updated_field'.", + }, + "items": { + Type: "array", + Description: "The set of items to update. Required for 'update_project_items'. Each entry references either an existing item by 'item_id', or by (item_owner + item_repo + issue_number). Each entry may optionally include its own 'updated_field'; if omitted, the top-level 'updated_field' is applied. Limit: " + strconv.Itoa(MaxProjectItemsPerBatch) + " items per call.", + Items: &jsonschema.Schema{ + Type: "object", + AdditionalProperties: &jsonschema.Schema{Not: &jsonschema.Schema{}}, + Properties: map[string]*jsonschema.Schema{ + "item_id": { + Type: "number", + Description: "The project item ID. Provide either item_id, or (item_owner + item_repo + issue_number).", + }, + "item_owner": { + Type: "string", + Description: "Owner of the repository containing the issue or pull request. Combine with item_repo and issue_number to resolve the item.", + }, + "item_repo": { + Type: "string", + Description: "Repository containing the issue or pull request. Combine with item_owner and issue_number to resolve the item.", + }, + "issue_number": { + Type: "number", + Description: "Issue number (or pull request number) used to resolve the project item together with item_owner and item_repo.", + }, + "updated_field": { + Type: "object", + Description: "Per-item field update. Overrides the top-level 'updated_field'. Same shape as the top-level 'updated_field'.", + }, + }, + }, }, "body": { Type: "string", @@ -722,6 +759,8 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { return utils.NewToolResultError("updated_field must be an object"), nil, nil } return updateProjectItem(ctx, client, gqlClient, owner, ownerType, projectNumber, itemID, fieldValue) + case projectsMethodUpdateProjectItems: + return updateProjectItemsBatch(ctx, client, gqlClient, owner, ownerType, projectNumber, args) case projectsMethodDeleteProjectItem: itemID, err := RequiredBigInt(args, "item_id") if err != nil { @@ -1193,6 +1232,298 @@ func updateProjectItem(ctx context.Context, client *github.Client, gqlClient *gi return utils.NewToolResultText(string(r)), nil, nil } +// batchItemResult is the per-item outcome returned by update_project_items. +// On success, Item is populated. On failure, Error carries a stable machine-readable +// code (mirroring the StructuredResolutionError shape where relevant) plus a human +// message so agents can self-correct without failing the whole batch. +type batchItemResult struct { + Index int `json:"index"` + OK bool `json:"ok"` + Item *MinimalProjectItem `json:"item,omitempty"` + Error *batchItemError `json:"error,omitempty"` + // Ref echoes the identifying input for the item so agents can correlate results + // with the request even when resolution fails (before item_id is known). + Ref map[string]any `json:"ref,omitempty"` +} + +type batchItemError struct { + Code string `json:"code"` + Message string `json:"message"` + Candidates []any `json:"candidates,omitempty"` + Hint string `json:"hint,omitempty"` +} + +// updateProjectItemsBatch handles the update_project_items method. It applies one or +// more field updates in a single tool call, with resolution amortised across the +// batch and per-item error mapping. See docs/tool-renaming.md for the shape. +// +// Current transport: N sequential REST calls (mirroring update_project_item) with a +// shared, single-round-trip field-name cache and a single owner-type detection. The +// underlying `updateProjectV2ItemFieldValue` GraphQL mutation supports request +// aliasing (multiple mutations per HTTP round trip); once the pinned +// shurcooL/githubv4 client supports dynamic aliased mutations we can swap the loop +// out for a chunked aliased request without changing the tool contract. +func updateProjectItemsBatch(ctx context.Context, client *github.Client, gqlClient *githubv4.Client, owner, ownerType string, projectNumber int, args map[string]any) (*mcp.CallToolResult, any, error) { + rawItems, exists := args["items"] + if !exists { + return utils.NewToolResultError("missing required parameter: items"), nil, nil + } + itemsRaw, ok := rawItems.([]any) + if !ok { + return utils.NewToolResultError("items must be an array"), nil, nil + } + if len(itemsRaw) == 0 { + return utils.NewToolResultError("items must contain at least one entry"), nil, nil + } + if len(itemsRaw) > MaxProjectItemsPerBatch { + return utils.NewToolResultError(fmt.Sprintf("items exceeds maximum of %d entries per call (got %d)", MaxProjectItemsPerBatch, len(itemsRaw))), nil, nil + } + + // Optional top-level updated_field, applied to every item that does not + // supply its own updated_field. + var sharedField map[string]any + if rawShared, hasShared := args["updated_field"]; hasShared { + if m, ok := rawShared.(map[string]any); ok && m != nil { + sharedField = m + } else { + return utils.NewToolResultError("updated_field must be an object"), nil, nil + } + } + + cache := newProjectFieldsCache() + results := make([]batchItemResult, len(itemsRaw)) + + for i, raw := range itemsRaw { + entry, ok := raw.(map[string]any) + if !ok || entry == nil { + results[i] = batchItemResult{ + Index: i, + OK: false, + Error: &batchItemError{ + Code: "invalid_item", + Message: fmt.Sprintf("items[%d] must be an object", i), + }, + } + continue + } + + ref := itemRefEcho(entry) + + // Resolve the effective updated_field: per-item override wins over shared. + effectiveField, fieldErr := effectiveUpdatedField(entry, sharedField) + if fieldErr != nil { + results[i] = batchItemResult{ + Index: i, + OK: false, + Ref: ref, + Error: &batchItemError{ + Code: "invalid_updated_field", + Message: fieldErr.Error(), + }, + } + continue + } + + // Resolve item ID: either provided directly or resolved from issue ref. + itemID, resolveErr := resolveBatchItemID(ctx, gqlClient, owner, ownerType, projectNumber, entry) + if resolveErr != nil { + results[i] = batchItemResult{ + Index: i, + OK: false, + Ref: ref, + Error: batchErrorFromResolution(resolveErr), + } + continue + } + + payload, buildErr := buildUpdateProjectItemWithCache(ctx, gqlClient, cache, owner, ownerType, projectNumber, effectiveField) + if buildErr != nil { + results[i] = batchItemResult{ + Index: i, + OK: false, + Ref: ref, + Error: batchErrorFromResolution(buildErr), + } + continue + } + + var ( + resp *github.Response + updatedItem *github.ProjectV2Item + err error + ) + if ownerType == "org" { + updatedItem, resp, err = client.Projects.UpdateOrganizationProjectItem(ctx, owner, projectNumber, itemID, payload) + } else { + updatedItem, resp, err = client.Projects.UpdateUserProjectItem(ctx, owner, projectNumber, itemID, payload) + } + if err != nil { + results[i] = batchItemResult{ + Index: i, + OK: false, + Ref: ref, + Error: &batchItemError{ + Code: "update_failed", + Message: fmt.Sprintf("%s: %v", ProjectUpdateFailedError, err), + }, + } + if resp != nil { + _ = resp.Body.Close() + } + continue + } + if resp.StatusCode != http.StatusOK { + body, readErr := io.ReadAll(resp.Body) + _ = resp.Body.Close() + if readErr != nil { + return nil, nil, fmt.Errorf("failed to read response body: %w", readErr) + } + results[i] = batchItemResult{ + Index: i, + OK: false, + Ref: ref, + Error: &batchItemError{ + Code: "update_failed", + Message: fmt.Sprintf("%s: status %d: %s", ProjectUpdateFailedError, resp.StatusCode, strings.TrimSpace(string(body))), + }, + } + continue + } + _ = resp.Body.Close() + + item := convertToMinimalProjectItem(updatedItem) + results[i] = batchItemResult{ + Index: i, + OK: true, + Ref: ref, + Item: &item, + } + } + + success := 0 + for _, r := range results { + if r.OK { + success++ + } + } + + response := map[string]any{ + "total": len(results), + "succeeded": success, + "failed": len(results) - success, + "results": results, + } + r, err := json.Marshal(response) + if err != nil { + return nil, nil, fmt.Errorf("failed to marshal response: %w", err) + } + return utils.NewToolResultText(string(r)), nil, nil +} + +// effectiveUpdatedField picks the per-item updated_field when set, otherwise falls +// back to the shared top-level value. Returns an error if neither is present or the +// per-item override is malformed. +func effectiveUpdatedField(entry, shared map[string]any) (map[string]any, error) { + if raw, ok := entry["updated_field"]; ok { + m, isMap := raw.(map[string]any) + if !isMap || m == nil { + return nil, fmt.Errorf("updated_field must be an object") + } + return m, nil + } + if shared != nil { + return shared, nil + } + return nil, fmt.Errorf("updated_field is required either at the top level or on each item") +} + +// itemRefEcho returns a copy of the identifying fields of an item entry, used to +// help agents correlate per-item results with their inputs. +func itemRefEcho(entry map[string]any) map[string]any { + ref := map[string]any{} + for _, key := range []string{"item_id", "item_owner", "item_repo", "issue_number"} { + if v, ok := entry[key]; ok { + ref[key] = v + } + } + if len(ref) == 0 { + return nil + } + return ref +} + +// resolveBatchItemID resolves a single batch entry to a project item ID. Accepts +// either an explicit item_id or an (item_owner, item_repo, issue_number) triplet +// (same semantics as update_project_item's single-item path). +func resolveBatchItemID(ctx context.Context, gqlClient *githubv4.Client, owner, ownerType string, projectNumber int, entry map[string]any) (int64, error) { + if _, has := entry["item_id"]; has { + id, err := validateAndConvertToInt64(entry["item_id"]) + if err != nil { + return 0, fmt.Errorf("item_id: %w", err) + } + return id, nil + } + + issueOwner, ownerErr := stringFromEntry(entry, "item_owner") + issueRepo, repoErr := stringFromEntry(entry, "item_repo") + issueNumber, numErr := intFromEntry(entry, "issue_number") + if ownerErr != nil || repoErr != nil || numErr != nil { + return 0, fmt.Errorf("each item requires either item_id, or item_owner + item_repo + issue_number") + } + if gqlClient == nil { + return 0, fmt.Errorf("internal error: gqlClient is required to resolve items by issue number") + } + return resolveProjectItemIDByIssueNumber(ctx, gqlClient, owner, ownerType, projectNumber, issueOwner, issueRepo, issueNumber) +} + +func stringFromEntry(entry map[string]any, key string) (string, error) { + v, ok := entry[key] + if !ok { + return "", fmt.Errorf("missing %s", key) + } + s, ok := v.(string) + if !ok || s == "" { + return "", fmt.Errorf("%s must be a non-empty string", key) + } + return s, nil +} + +func intFromEntry(entry map[string]any, key string) (int, error) { + v, ok := entry[key] + if !ok { + return 0, fmt.Errorf("missing %s", key) + } + switch n := v.(type) { + case float64: + return int(n), nil + case int: + return n, nil + case int64: + return int(n), nil + default: + return 0, fmt.Errorf("%s must be a number", key) + } +} + +// batchErrorFromResolution converts an error from resolution/build helpers into a +// batch-item error. Structured resolution errors preserve their code, hint, and +// candidates so agents can self-correct per-item without failing the batch. +func batchErrorFromResolution(err error) *batchItemError { + var structured *ghErrors.StructuredResolutionError + if errors.As(err, &structured) { + return &batchItemError{ + Code: structured.Kind, + Message: fmt.Sprintf("%s: %s", structured.Kind, structured.Name), + Hint: structured.Hint, + Candidates: structured.Candidates, + } + } + return &batchItemError{ + Code: "invalid_argument", + Message: err.Error(), + } +} + func deleteProjectItem(ctx context.Context, client *github.Client, owner, ownerType string, projectNumber int, itemID int64) (*mcp.CallToolResult, any, error) { var resp *github.Response var err error @@ -1530,8 +1861,91 @@ func validateAndConvertToInt64(value any) (int64, error) { } } +// projectFieldsCache lazily loads every field on a project once, so callers that +// need to resolve multiple field names against the same project pay a single +// GraphQL round trip instead of one per name. +type projectFieldsCache struct { + loaded bool + fields []ResolvedField + byName map[string][]ResolvedField +} + +func newProjectFieldsCache() *projectFieldsCache { + return &projectFieldsCache{} +} + +func (c *projectFieldsCache) ensureLoaded(ctx context.Context, gqlClient *githubv4.Client, owner, ownerType string, projectNumber int) error { + if c.loaded { + return nil + } + all, err := listAllProjectFields(ctx, gqlClient, owner, ownerType, projectNumber) + if err != nil { + return err + } + c.fields = all + c.byName = make(map[string][]ResolvedField, len(all)) + for _, f := range all { + key := strings.ToLower(f.Name) + c.byName[key] = append(c.byName[key], f) + } + c.loaded = true + return nil +} + +// resolveFieldByName is a cached variant of resolveProjectFieldByName: it uses the +// cache's in-memory index instead of querying GraphQL per lookup. +func (c *projectFieldsCache) resolveFieldByName(ctx context.Context, gqlClient *githubv4.Client, owner, ownerType string, projectNumber int, fieldName string) (*ResolvedField, error) { + if fieldName == "" { + return nil, fmt.Errorf("field name must not be empty") + } + if err := c.ensureLoaded(ctx, gqlClient, owner, ownerType, projectNumber); err != nil { + return nil, err + } + matches := c.byName[strings.ToLower(fieldName)] + switch len(matches) { + case 0: + candidates := make([]any, 0, len(c.fields)) + for _, f := range c.fields { + candidates = append(candidates, map[string]any{ + "name": f.Name, + "data_type": f.DataType, + }) + } + return nil, ghErrors.NewStructuredResolutionError( + "field_not_found", + fieldName, + fmt.Sprintf("no project field named %q on project %s#%d; see candidates for available names", fieldName, owner, projectNumber), + candidates, + ) + case 1: + f := matches[0] + return &f, nil + default: + candidates := make([]any, 0, len(matches)) + for _, f := range matches { + candidates = append(candidates, map[string]any{ + "id": f.ID, + "data_type": f.DataType, + }) + } + return nil, ghErrors.NewStructuredResolutionError( + "field_ambiguous", + fieldName, + "multiple fields share this name; pass updated_field.id to disambiguate", + candidates, + ) + } +} + // buildUpdateProjectItem builds UpdateProjectItemOptions, resolving field names and SINGLE_SELECT option names server-side. func buildUpdateProjectItem(ctx context.Context, gqlClient *githubv4.Client, owner, ownerType string, projectNumber int, input map[string]any) (*github.UpdateProjectItemOptions, error) { + return buildUpdateProjectItemWithCache(ctx, gqlClient, nil, owner, ownerType, projectNumber, input) +} + +// buildUpdateProjectItemWithCache is the cache-aware variant of buildUpdateProjectItem. +// When cache is non-nil, name-based field lookups reuse the cache's in-memory index; when +// nil, each name lookup performs its own GraphQL round trip (single-item behaviour). +func buildUpdateProjectItemWithCache(ctx context.Context, gqlClient *githubv4.Client, cache *projectFieldsCache, owner, ownerType string, projectNumber int, input map[string]any) (*github.UpdateProjectItemOptions, error) { if input == nil { return nil, fmt.Errorf("updated_field must be an object") } @@ -1571,7 +1985,11 @@ func buildUpdateProjectItem(ctx context.Context, gqlClient *githubv4.Client, own return nil, fmt.Errorf("internal error: gqlClient is required to resolve updated_field.name") } var err error - resolved, err = resolveProjectFieldByName(ctx, gqlClient, owner, ownerType, projectNumber, fieldName, "") + if cache != nil { + resolved, err = cache.resolveFieldByName(ctx, gqlClient, owner, ownerType, projectNumber, fieldName) + } else { + resolved, err = resolveProjectFieldByName(ctx, gqlClient, owner, ownerType, projectNumber, fieldName, "") + } if err != nil { return nil, err } diff --git a/pkg/github/projects_resolver_test.go b/pkg/github/projects_resolver_test.go index e701441c1e..b9663a2210 100644 --- a/pkg/github/projects_resolver_test.go +++ b/pkg/github/projects_resolver_test.go @@ -537,3 +537,206 @@ func Test_ProjectsWrite_UpdateProjectItem_NameNotFound_StructuredError(t *testin assert.Equal(t, "field_not_found", msg["error"]) assert.Equal(t, "Doesnt Exist", msg["name"]) } + +// Test_ProjectsWrite_UpdateProjectItems_Batch_ByNameAndIssueRef exercises the +// batch method's happy-path resolvers: one item is addressed by numeric item_id, +// another by (item_owner, item_repo, issue_number). Field-name resolution runs +// once for the entire batch. +func Test_ProjectsWrite_UpdateProjectItems_Batch_ByNameAndIssueRef(t *testing.T) { + toolDef := ProjectsWrite(translations.NullTranslationHelper) + + updatedItem := verbosePullRequestProjectItemFixture() + mockedREST := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchOrgsProjectsV2ItemsByProjectByItemID: mockResponse(t, http.StatusOK, updatedItem), + }) + restClient := mustNewGHClient(t, mockedREST) + + mockedGQL := githubv4mock.NewMockedHTTPClient( + // Project node id resolution (used to filter the issue's project items). + githubv4mock.NewQueryMatcher( + struct { + Organization struct { + ProjectV2 struct { + ID githubv4.ID + } `graphql:"projectV2(number: $projectNumber)"` + } `graphql:"organization(login: $owner)"` + }{}, + map[string]any{ + "owner": githubv4.String("octo-org"), + "projectNumber": githubv4.Int(1), + }, + githubv4mock.DataResponse(map[string]any{ + "organization": map[string]any{ + "projectV2": map[string]any{"id": "PVT_project1"}, + }, + }), + ), + // Issue -> projectItems traversal for the issue-ref item. + githubv4mock.NewQueryMatcher( + resolveItemByIssueQuery{}, + map[string]any{ + "issueOwner": githubv4.String("github"), + "issueRepo": githubv4.String("planning-tracking"), + "issueNumber": githubv4.Int(123), + }, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "issue": map[string]any{ + "projectItems": map[string]any{ + "nodes": []any{ + map[string]any{ + "fullDatabaseId": "2002", + "project": map[string]any{"id": "PVT_project1"}, + }, + }, + "pageInfo": map[string]any{ + "hasNextPage": false, "hasPreviousPage": false, + "startCursor": "", "endCursor": "", + }, + }, + }, + }, + }), + ), + // One-shot field-name resolution shared across the batch. + githubv4mock.NewQueryMatcher( + projectFieldsTestQuery{}, + fieldsQueryVars("octo-org", 1), + githubv4mock.DataResponse(fieldsResponse([]map[string]any{ + statusFieldNode("PVTSSF_lADOBBcDeFg101", 101, "Status", []map[string]any{ + {"id": "OPT_in_progress", "name": "In Progress"}, + }), + })), + ), + ) + gqlClient := githubv4.NewClient(mockedGQL) + + deps := BaseDeps{Client: restClient, GQLClient: gqlClient} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "update_project_items", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(1), + "updated_field": map[string]any{ + "name": "Status", + "value": "In Progress", + }, + "items": []any{ + map[string]any{"item_id": float64(1001)}, + map[string]any{ + "item_owner": "github", + "item_repo": "planning-tracking", + "issue_number": float64(123), + }, + }, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + + require.NoError(t, err) + require.False(t, result.IsError, getTextResult(t, result).Text) + + var response map[string]any + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &response)) + assert.Equal(t, float64(2), response["succeeded"]) + assert.Equal(t, float64(0), response["failed"]) +} + +// Test_ProjectsWrite_UpdateProjectItems_Batch_PartialFailure_ItemNotInProject +// verifies that when one item's issue is not on the project, its per-item error +// is a structured item_not_in_project code and does not abort the batch. +func Test_ProjectsWrite_UpdateProjectItems_Batch_PartialFailure_ItemNotInProject(t *testing.T) { + toolDef := ProjectsWrite(translations.NullTranslationHelper) + + updatedItem := verbosePullRequestProjectItemFixture() + mockedREST := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchOrgsProjectsV2ItemsByProjectByItemID: mockResponse(t, http.StatusOK, updatedItem), + }) + restClient := mustNewGHClient(t, mockedREST) + + mockedGQL := githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + struct { + Organization struct { + ProjectV2 struct { + ID githubv4.ID + } `graphql:"projectV2(number: $projectNumber)"` + } `graphql:"organization(login: $owner)"` + }{}, + map[string]any{ + "owner": githubv4.String("octo-org"), + "projectNumber": githubv4.Int(1), + }, + githubv4mock.DataResponse(map[string]any{ + "organization": map[string]any{ + "projectV2": map[string]any{"id": "PVT_project1"}, + }, + }), + ), + // Issue exists but its projectItems do NOT include our project. + githubv4mock.NewQueryMatcher( + resolveItemByIssueQuery{}, + map[string]any{ + "issueOwner": githubv4.String("github"), + "issueRepo": githubv4.String("planning-tracking"), + "issueNumber": githubv4.Int(999), + }, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "issue": map[string]any{ + "projectItems": map[string]any{ + "nodes": []any{}, + "pageInfo": map[string]any{ + "hasNextPage": false, "hasPreviousPage": false, + "startCursor": "", "endCursor": "", + }, + }, + }, + }, + }), + ), + ) + gqlClient := githubv4.NewClient(mockedGQL) + + deps := BaseDeps{Client: restClient, GQLClient: gqlClient} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "update_project_items", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(1), + "updated_field": map[string]any{ + "id": float64(101), + "value": "In Progress", + }, + "items": []any{ + map[string]any{"item_id": float64(1001)}, + map[string]any{ + "item_owner": "github", + "item_repo": "planning-tracking", + "issue_number": float64(999), + }, + }, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + + require.NoError(t, err) + require.False(t, result.IsError, getTextResult(t, result).Text) + + var response map[string]any + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &response)) + assert.Equal(t, float64(1), response["succeeded"]) + assert.Equal(t, float64(1), response["failed"]) + + results := response["results"].([]any) + require.Len(t, results, 2) + failed := results[1].(map[string]any) + assert.Equal(t, false, failed["ok"]) + errObj := failed["error"].(map[string]any) + assert.Equal(t, "item_not_in_project", errObj["code"]) + // The ref should echo the identifying issue-ref inputs so agents can correlate. + ref := failed["ref"].(map[string]any) + assert.Equal(t, "github", ref["item_owner"]) + assert.Equal(t, "planning-tracking", ref["item_repo"]) + assert.Equal(t, float64(999), ref["issue_number"]) +} diff --git a/pkg/github/projects_test.go b/pkg/github/projects_test.go index 553c2421a4..09e99fa61e 100644 --- a/pkg/github/projects_test.go +++ b/pkg/github/projects_test.go @@ -879,6 +879,7 @@ func Test_ProjectsWrite(t *testing.T) { assert.Contains(t, inputSchema.Properties, "issue_number") assert.Contains(t, inputSchema.Properties, "pull_request_number") assert.Contains(t, inputSchema.Properties, "updated_field") + assert.Contains(t, inputSchema.Properties, "items") assert.ElementsMatch(t, inputSchema.Required, []string{"method", "owner"}) // Verify DestructiveHint is set @@ -1224,6 +1225,241 @@ func Test_ProjectsWrite_UpdateProjectItem(t *testing.T) { }) } +func Test_ProjectsWrite_UpdateProjectItems_Batch(t *testing.T) { + toolDef := ProjectsWrite(translations.NullTranslationHelper) + + t.Run("success with shared updated_field applied to two items", func(t *testing.T) { + updatedItem := verbosePullRequestProjectItemFixture() + mockedREST := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchOrgsProjectsV2ItemsByProjectByItemID: mockResponse(t, http.StatusOK, updatedItem), + }) + restClient := mustNewGHClient(t, mockedREST) + // No GraphQL calls expected: both entries use item_id + updated_field.id, so + // no name resolution or issue-number lookup is needed. + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient()) + + deps := BaseDeps{Client: restClient, GQLClient: gqlClient} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "update_project_items", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(1), + "updated_field": map[string]any{ + "id": float64(101), + "value": "In Progress", + }, + "items": []any{ + map[string]any{"item_id": float64(1001)}, + map[string]any{"item_id": float64(1002)}, + }, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + + require.NoError(t, err) + require.False(t, result.IsError, getTextResult(t, result).Text) + + var response map[string]any + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &response)) + assert.Equal(t, float64(2), response["total"]) + assert.Equal(t, float64(2), response["succeeded"]) + assert.Equal(t, float64(0), response["failed"]) + results, ok := response["results"].([]any) + require.True(t, ok) + require.Len(t, results, 2) + for _, r := range results { + entry := r.(map[string]any) + assert.Equal(t, true, entry["ok"]) + assert.Nil(t, entry["error"]) + assert.NotNil(t, entry["item"]) + } + }) + + t.Run("resolves field name once for the whole batch", func(t *testing.T) { + updatedItem := verbosePullRequestProjectItemFixture() + + // Count fields(...) calls to confirm we only resolve once for the batch. + var fieldCallCount int32 + fieldsHandler := &countingHTTPHandler{count: &fieldCallCount} + fieldsHandler.matchers = []githubv4mock.Matcher{ + githubv4mock.NewQueryMatcher( + projectFieldsTestQuery{}, + fieldsQueryVars("octo-org", 1), + githubv4mock.DataResponse(fieldsResponse([]map[string]any{ + statusFieldNode("PVTSSF_lADOBBcDeFg101", 101, "Status", []map[string]any{ + {"id": "OPT_in_progress", "name": "In Progress"}, + }), + })), + ), + } + gqlClient := githubv4.NewClient(&http.Client{Transport: fieldsHandler}) + + mockedREST := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchOrgsProjectsV2ItemsByProjectByItemID: mockResponse(t, http.StatusOK, updatedItem), + }) + restClient := mustNewGHClient(t, mockedREST) + + deps := BaseDeps{Client: restClient, GQLClient: gqlClient} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "update_project_items", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(1), + "updated_field": map[string]any{ + "name": "Status", + "value": "In Progress", + }, + "items": []any{ + map[string]any{"item_id": float64(1001)}, + map[string]any{"item_id": float64(1002)}, + map[string]any{"item_id": float64(1003)}, + }, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + + require.NoError(t, err) + require.False(t, result.IsError, getTextResult(t, result).Text) + // Exactly one field-resolution round trip regardless of batch size. + assert.Equal(t, int32(1), fieldCallCount) + }) + + t.Run("partial failure: one item resolves, one has an invalid updated_field", func(t *testing.T) { + updatedItem := verbosePullRequestProjectItemFixture() + mockedREST := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchOrgsProjectsV2ItemsByProjectByItemID: mockResponse(t, http.StatusOK, updatedItem), + }) + restClient := mustNewGHClient(t, mockedREST) + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient()) + + deps := BaseDeps{Client: restClient, GQLClient: gqlClient} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "update_project_items", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(1), + "items": []any{ + map[string]any{ + "item_id": float64(1001), + "updated_field": map[string]any{ + "id": float64(101), + "value": "In Progress", + }, + }, + // Missing both id and name on updated_field: should surface as a per-item error. + map[string]any{ + "item_id": float64(1002), + "updated_field": map[string]any{ + "value": "In Progress", + }, + }, + }, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + + require.NoError(t, err) + require.False(t, result.IsError, getTextResult(t, result).Text) + + var response map[string]any + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &response)) + assert.Equal(t, float64(2), response["total"]) + assert.Equal(t, float64(1), response["succeeded"]) + assert.Equal(t, float64(1), response["failed"]) + + results := response["results"].([]any) + require.Len(t, results, 2) + first := results[0].(map[string]any) + second := results[1].(map[string]any) + assert.Equal(t, true, first["ok"]) + assert.Equal(t, false, second["ok"]) + errObj := second["error"].(map[string]any) + assert.Equal(t, "invalid_argument", errObj["code"]) + assert.Contains(t, errObj["message"], "either id or name") + }) + + t.Run("missing items array", func(t *testing.T) { + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient()) + restClient := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})) + deps := BaseDeps{Client: restClient, GQLClient: gqlClient} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "update_project_items", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(1), + "updated_field": map[string]any{"id": float64(101), "value": "Todo"}, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "missing required parameter: items") + }) + + t.Run("empty items array", func(t *testing.T) { + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient()) + restClient := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})) + deps := BaseDeps{Client: restClient, GQLClient: gqlClient} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "update_project_items", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(1), + "updated_field": map[string]any{"id": float64(101), "value": "Todo"}, + "items": []any{}, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "at least one entry") + }) + + t.Run("item with no updated_field and no shared updated_field is a per-item error", func(t *testing.T) { + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient()) + restClient := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})) + deps := BaseDeps{Client: restClient, GQLClient: gqlClient} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "update_project_items", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(1), + "items": []any{ + map[string]any{"item_id": float64(1001)}, + }, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError, getTextResult(t, result).Text) + + var response map[string]any + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &response)) + assert.Equal(t, float64(0), response["succeeded"]) + assert.Equal(t, float64(1), response["failed"]) + results := response["results"].([]any) + errObj := results[0].(map[string]any)["error"].(map[string]any) + assert.Equal(t, "invalid_updated_field", errObj["code"]) + }) +} + +// countingHTTPHandler wraps a set of githubv4mock matchers with a request counter. +// It behaves like githubv4mock.NewMockedHTTPClient's transport but exposes a hit +// count so tests can assert on the number of round trips. +type countingHTTPHandler struct { + matchers []githubv4mock.Matcher + count *int32 +} + +func (c *countingHTTPHandler) RoundTrip(req *http.Request) (*http.Response, error) { + client := githubv4mock.NewMockedHTTPClient(c.matchers...) + resp, err := client.Transport.RoundTrip(req) + if resp != nil && resp.StatusCode == http.StatusOK { + *c.count++ + } + return resp, err +} + func Test_ProjectsWrite_DeleteProjectItem(t *testing.T) { toolDef := ProjectsWrite(translations.NullTranslationHelper)