From 29ccdc487ba44c12e8f70e127b53d3ea204ec47c Mon Sep 17 00:00:00 2001 From: Bryan Zwicker Date: Tue, 28 Jul 2026 10:20:45 -0400 Subject: [PATCH 1/3] Add basic project view management Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c6f8ede6-efee-4191-900d-59a1bb0af000 --- README.md | 8 +- pkg/github/__toolsnaps__/projects_get.snap | 9 +- pkg/github/__toolsnaps__/projects_list.snap | 7 +- pkg/github/__toolsnaps__/projects_write.snap | 33 +- pkg/github/minimal_types.go | 9 + pkg/github/projects.go | 458 +++++++++++++- pkg/github/projects_test.go | 11 + pkg/github/projects_v2_test.go | 614 +++++++++++++++++++ pkg/github/toolset_instructions.go | 2 + 9 files changed, 1138 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 1a06c0697d..83ea726953 100644 --- a/README.md +++ b/README.md @@ -1097,6 +1097,7 @@ The following sets of tools are available: - `owner_type`: Owner type (user or org). If not provided, will be automatically detected. (string, optional) - `project_number`: The project's number. (number, optional) - `status_update_id`: The node ID of the project status update. Required for 'get_project_status_update' method. (string, optional) + - `view_id`: The node ID of the project view. Required for 'get_project_view' method. (string, optional) - **projects_list** - List GitHub Projects resources - **Required OAuth Scopes**: `read:project` @@ -1109,13 +1110,14 @@ The following sets of tools are available: - `owner`: The owner (user or organization login). The name is not case sensitive. (string, required) - `owner_type`: Owner type (user or org). If not provided, will automatically try both. (string, optional) - `per_page`: Results per page (max 50) (number, optional) - - `project_number`: The project's number. Required for 'list_project_fields', 'list_project_items', and 'list_project_status_updates' methods. (number, optional) + - `project_number`: The project's number. Required for 'list_project_fields', 'list_project_items', 'list_project_views', and 'list_project_status_updates' methods. (number, optional) - `query`: Filter/query string. For list_projects: filter by title text and state (e.g. "roadmap is:open"). For list_project_items: advanced filtering using GitHub's project filtering syntax. (string, optional) - **projects_write** - Manage GitHub Projects - **Required OAuth Scopes**: `project` - `body`: The body of the status update (markdown). Used for 'create_project_status_update' method. (string, optional) - `field_name`: The name of the iteration field (e.g. 'Sprint'). Required for 'create_iteration_field' method. (string, optional) + - `filter`: Saved view filter; omit on update to preserve it, or pass an empty string to clear it. (string, optional) - `issue_number`: The issue number. Required for 'add_project_item' when item_type is 'issue'. Also accepted by 'update_project_item' to resolve the item by issue number (combine with item_owner and item_repo). (number, optional) - `item_id`: The project item ID. Required for 'delete_project_item'. For 'update_project_item', provide either item_id, or (item_owner + item_repo + issue_number) to resolve the item by issue. (number, optional) - `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) @@ -1123,7 +1125,9 @@ The following sets of tools are available: - `item_type`: The item's type, either issue or pull_request. Required for 'add_project_item' method. (string, 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) + - `layout`: View layout; required when creating a view. (string, optional) - `method`: The method to execute (string, required) + - `name`: View name; required when creating a view. (string, optional) - `owner`: The project owner (user or organization login). The name is not case sensitive. (string, required) - `owner_type`: Owner type (user or org). Required for 'create_project' method. If not provided for other methods, will be automatically detected. (string, optional) - `project_number`: The project's number. Required for all methods except 'create_project'. (number, optional) @@ -1133,6 +1137,8 @@ The following sets of tools are available: - `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) + - `view_id`: Project view node ID for update or delete. (string, optional) + - `visible_fields`: Field database IDs for table or board creation; unsupported for roadmap. (string[], optional) diff --git a/pkg/github/__toolsnaps__/projects_get.snap b/pkg/github/__toolsnaps__/projects_get.snap index f6a48c9328..1380e84d5d 100644 --- a/pkg/github/__toolsnaps__/projects_get.snap +++ b/pkg/github/__toolsnaps__/projects_get.snap @@ -4,7 +4,7 @@ "readOnlyHint": true, "title": "Get details of GitHub Projects resources" }, - "description": "Get details about specific GitHub Projects resources.\nUse this tool to get details about individual projects, project fields, and project items by their unique IDs.\n", + "description": "Get details about specific GitHub Projects resources.\nUse this tool to get details about individual projects, project fields, project items, and project views by their unique IDs.\n", "inputSchema": { "properties": { "field_id": { @@ -35,7 +35,8 @@ "get_project", "get_project_field", "get_project_item", - "get_project_status_update" + "get_project_status_update", + "get_project_view" ], "type": "string" }, @@ -58,6 +59,10 @@ "status_update_id": { "description": "The node ID of the project status update. Required for 'get_project_status_update' method.", "type": "string" + }, + "view_id": { + "description": "The node ID of the project view. Required for 'get_project_view' method.", + "type": "string" } }, "required": [ diff --git a/pkg/github/__toolsnaps__/projects_list.snap b/pkg/github/__toolsnaps__/projects_list.snap index 547417e983..487119f04a 100644 --- a/pkg/github/__toolsnaps__/projects_list.snap +++ b/pkg/github/__toolsnaps__/projects_list.snap @@ -4,7 +4,7 @@ "readOnlyHint": true, "title": "List GitHub Projects resources" }, - "description": "Tools for listing GitHub Projects resources.\nUse this tool to list projects for a user or organization, or list project fields and items for a specific project.\n", + "description": "Tools for listing GitHub Projects resources.\nUse this tool to list projects for a user or organization, or list project fields, items, views, and status updates for a specific project.\n", "inputSchema": { "properties": { "after": { @@ -35,7 +35,8 @@ "list_projects", "list_project_fields", "list_project_items", - "list_project_status_updates" + "list_project_status_updates", + "list_project_views" ], "type": "string" }, @@ -56,7 +57,7 @@ "type": "number" }, "project_number": { - "description": "The project's number. Required for 'list_project_fields', 'list_project_items', and 'list_project_status_updates' methods.", + "description": "The project's number. Required for 'list_project_fields', 'list_project_items', 'list_project_views', and 'list_project_status_updates' methods.", "type": "number" }, "query": { diff --git a/pkg/github/__toolsnaps__/projects_write.snap b/pkg/github/__toolsnaps__/projects_write.snap index 762ee08c93..f8aa795ce3 100644 --- a/pkg/github/__toolsnaps__/projects_write.snap +++ b/pkg/github/__toolsnaps__/projects_write.snap @@ -5,7 +5,7 @@ "readOnlyHint": false, "title": "Manage GitHub Projects" }, - "description": "Create and manage GitHub Projects: create projects, add/update/delete items, create status updates, and add iteration fields.", + "description": "Create and manage GitHub Projects: create projects, manage items and views, create status updates, and add iteration fields.", "inputSchema": { "properties": { "body": { @@ -16,6 +16,10 @@ "description": "The name of the iteration field (e.g. 'Sprint'). Required for 'create_iteration_field' method.", "type": "string" }, + "filter": { + "description": "Saved view filter; omit on update to preserve it, or pass an empty string to clear it.", + "type": "string" + }, "issue_number": { "description": "The issue number. Required for 'add_project_item' when item_type is 'issue'. Also accepted by 'update_project_item' to resolve the item by issue number (combine with item_owner and item_repo).", "type": "number" @@ -71,6 +75,15 @@ }, "type": "array" }, + "layout": { + "description": "View layout; required when creating a view.", + "enum": [ + "table", + "board", + "roadmap" + ], + "type": "string" + }, "method": { "description": "The method to execute", "enum": [ @@ -78,11 +91,18 @@ "update_project_item", "delete_project_item", "create_project_status_update", + "create_project_view", + "update_project_view", + "delete_project_view", "create_project", "create_iteration_field" ], "type": "string" }, + "name": { + "description": "View name; required when creating a view.", + "type": "string" + }, "owner": { "description": "The project owner (user or organization login). The name is not case sensitive.", "type": "string" @@ -129,6 +149,17 @@ "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.", "type": "object" + }, + "view_id": { + "description": "Project view node ID for update or delete.", + "type": "string" + }, + "visible_fields": { + "description": "Field database IDs for table or board creation; unsupported for roadmap.", + "items": { + "type": "string" + }, + "type": "array" } }, "required": [ diff --git a/pkg/github/minimal_types.go b/pkg/github/minimal_types.go index 75bc8f48f1..cb5a37c077 100644 --- a/pkg/github/minimal_types.go +++ b/pkg/github/minimal_types.go @@ -351,6 +351,15 @@ type MinimalProject struct { OwnerType string `json:"owner_type,omitempty"` } +type MinimalProjectView struct { + ID string `json:"id"` + Number int `json:"number"` + Name string `json:"name"` + Layout string `json:"layout"` + Filter string `json:"filter"` + VisibleFields []int64 `json:"visible_fields,omitempty"` +} + type MinimalProjectItem struct { ID int64 `json:"id"` NodeID string `json:"node_id,omitempty"` diff --git a/pkg/github/projects.go b/pkg/github/projects.go index 308c2b87e8..893e4a8141 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" @@ -30,6 +31,11 @@ const ( ProjectStatusUpdateListFailedError = "failed to list project status updates" ProjectStatusUpdateGetFailedError = "failed to get project status update" ProjectStatusUpdateCreateFailedError = "failed to create project status update" + ProjectViewListFailedError = "failed to list project views" + ProjectViewGetFailedError = "failed to get project view" + ProjectViewCreateFailedError = "failed to create project view" + ProjectViewUpdateFailedError = "failed to update project view" + ProjectViewDeleteFailedError = "failed to delete project view" ProjectResolveIDFailedError = "failed to resolve project ID" MaxProjectsPerPage = 50 ) @@ -48,6 +54,11 @@ const ( projectsMethodListProjectStatusUpdates = "list_project_status_updates" projectsMethodGetProjectStatusUpdate = "get_project_status_update" projectsMethodCreateProjectStatusUpdate = "create_project_status_update" + projectsMethodListProjectViews = "list_project_views" + projectsMethodGetProjectView = "get_project_view" + projectsMethodCreateProjectView = "create_project_view" + projectsMethodUpdateProjectView = "update_project_view" + projectsMethodDeleteProjectView = "delete_project_view" projectsMethodCreateProject = "create_project" projectsMethodCreateIterationField = "create_iteration_field" ) @@ -106,6 +117,78 @@ type statusUpdateNodeQuery struct { } `graphql:"node(id: $id)"` } +type projectViewNode struct { + ID githubv4.ID + Number githubv4.Int + Name githubv4.String + Layout githubv4.ProjectV2ViewLayout + Filter *githubv4.String +} + +type projectViewNodeWithProject struct { + projectViewNode + Project projectVisibility +} + +type projectViewConnection struct { + Nodes []projectViewNode + PageInfo PageInfoFragment +} + +type projectViewsProject struct { + ID githubv4.ID + Public githubv4.Boolean + Views projectViewConnection `graphql:"views(first: $first, after: $after, last: $last, before: $before)"` +} + +type projectViewsUserQuery struct { + User struct { + ProjectV2 projectViewsProject `graphql:"projectV2(number: $projectNumber)"` + } `graphql:"user(login: $owner)"` +} + +type projectViewsOrgQuery struct { + Organization struct { + ProjectV2 projectViewsProject `graphql:"projectV2(number: $projectNumber)"` + } `graphql:"organization(login: $owner)"` +} + +type projectViewNodeQuery struct { + Node struct { + ProjectView projectViewNodeWithProject `graphql:"... on ProjectV2View"` + } `graphql:"node(id: $id)"` +} + +// CreateProjectV2ViewRequest is the REST request for creating a project view. +type CreateProjectV2ViewRequest struct { + Name string `json:"name"` + Layout string `json:"layout"` + Filter *string `json:"filter,omitempty"` + VisibleFields []int64 `json:"visible_fields,omitempty"` +} + +type projectV2ViewRESTResponse struct { + NodeID string `json:"node_id"` + Number int `json:"number"` + Name string `json:"name"` + Layout string `json:"layout"` + Filter *string `json:"filter,omitempty"` + VisibleFields []int64 `json:"visible_fields,omitempty"` +} + +// UpdateProjectV2ViewInput is the GraphQL input for updating a project view. +type UpdateProjectV2ViewInput struct { + ViewID githubv4.ID `json:"viewId"` + Name *githubv4.String `json:"name,omitempty"` + Layout *githubv4.ProjectV2ViewLayout `json:"layout,omitempty"` + Filter *githubv4.String `json:"filter,omitempty"` +} + +// DeleteProjectV2ViewInput is the GraphQL input for deleting a project view. +type DeleteProjectV2ViewInput struct { + ViewID githubv4.ID `json:"viewId"` +} + // CreateProjectV2StatusUpdateInput is the input for the createProjectV2StatusUpdate mutation. // Defined locally because the shurcooL/githubv4 library does not include this type. type CreateProjectV2StatusUpdateInput struct { @@ -158,7 +241,7 @@ func ProjectsList(t translations.TranslationHelperFunc) inventory.ServerTool { Name: "projects_list", Description: t("TOOL_PROJECTS_LIST_DESCRIPTION", `Tools for listing GitHub Projects resources. -Use this tool to list projects for a user or organization, or list project fields and items for a specific project. +Use this tool to list projects for a user or organization, or list project fields, items, views, and status updates for a specific project. `), Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_PROJECTS_LIST_USER_TITLE", "List GitHub Projects resources"), @@ -175,6 +258,7 @@ Use this tool to list projects for a user or organization, or list project field projectsMethodListProjectFields, projectsMethodListProjectItems, projectsMethodListProjectStatusUpdates, + projectsMethodListProjectViews, }, }, "owner_type": { @@ -188,7 +272,7 @@ Use this tool to list projects for a user or organization, or list project field }, "project_number": { Type: "number", - Description: "The project's number. Required for 'list_project_fields', 'list_project_items', and 'list_project_status_updates' methods.", + Description: "The project's number. Required for 'list_project_fields', 'list_project_items', 'list_project_views', and 'list_project_status_updates' methods.", }, "query": { Type: "string", @@ -251,7 +335,7 @@ Use this tool to list projects for a user or organization, or list project field result, visibilities, payload, err := listProjects(ctx, client, args, owner, ownerType) result = attachJoinedIFCLabel(ctx, deps, result, visibilities, ifc.LabelProjectList) return result, payload, err - case projectsMethodListProjectFields, projectsMethodListProjectItems, projectsMethodListProjectStatusUpdates: + case projectsMethodListProjectFields, projectsMethodListProjectItems, projectsMethodListProjectStatusUpdates, projectsMethodListProjectViews: // All other methods require project_number and ownerType detection projectNumber, err := RequiredInt(args, "project_number") if err != nil { @@ -295,6 +379,14 @@ Use this tool to list projects for a user or organization, or list project field result, isPrivate, payload, err := listProjectStatusUpdates(ctx, gqlClient, args, owner, ownerType) result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelProjectContent(isPrivate)) return result, payload, err + case projectsMethodListProjectViews: + gqlClient, err := deps.GetGQLClient(ctx) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + result, isPrivate, payload, err := listProjectViews(ctx, gqlClient, args, owner, ownerType) + result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelProjectContent(isPrivate)) + return result, payload, err default: return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil } @@ -313,7 +405,7 @@ func ProjectsGet(t translations.TranslationHelperFunc) inventory.ServerTool { mcp.Tool{ Name: "projects_get", Description: t("TOOL_PROJECTS_GET_DESCRIPTION", `Get details about specific GitHub Projects resources. -Use this tool to get details about individual projects, project fields, and project items by their unique IDs. +Use this tool to get details about individual projects, project fields, project items, and project views by their unique IDs. `), Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_PROJECTS_GET_USER_TITLE", "Get details of GitHub Projects resources"), @@ -330,6 +422,7 @@ Use this tool to get details about individual projects, project fields, and proj projectsMethodGetProjectField, projectsMethodGetProjectItem, projectsMethodGetProjectStatusUpdate, + projectsMethodGetProjectView, }, }, "owner_type": { @@ -371,6 +464,10 @@ Use this tool to get details about individual projects, project fields, and proj Type: "string", Description: "The node ID of the project status update. Required for 'get_project_status_update' method.", }, + "view_id": { + Type: "string", + Description: "The node ID of the project view. Required for 'get_project_view' method.", + }, }, Required: []string{"method"}, }, @@ -382,7 +479,7 @@ Use this tool to get details about individual projects, project fields, and proj return utils.NewToolResultError(err.Error()), nil, nil } - // Handle get_project_status_update early — it only needs status_update_id + // Handle node-ID-only methods before requiring owner and project_number. if method == projectsMethodGetProjectStatusUpdate { statusUpdateID, err := RequiredParam[string](args, "status_update_id") if err != nil { @@ -396,6 +493,19 @@ Use this tool to get details about individual projects, project fields, and proj result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelProjectContent(isPrivate)) return result, payload, err } + if method == projectsMethodGetProjectView { + viewID, err := RequiredParam[string](args, "view_id") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + gqlClient, err := deps.GetGQLClient(ctx) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + result, isPrivate, payload, err := getProjectView(ctx, gqlClient, viewID) + result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelProjectContent(isPrivate)) + return result, payload, err + } owner, err := RequiredParam[string](args, "owner") if err != nil { @@ -496,7 +606,7 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { ToolsetMetadataProjects, mcp.Tool{ Name: "projects_write", - Description: t("TOOL_PROJECTS_WRITE_DESCRIPTION", "Create and manage GitHub Projects: create projects, add/update/delete items, create status updates, and add iteration fields."), + Description: t("TOOL_PROJECTS_WRITE_DESCRIPTION", "Create and manage GitHub Projects: create projects, manage items and views, create status updates, and add iteration fields."), Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_PROJECTS_WRITE_USER_TITLE", "Manage GitHub Projects"), ReadOnlyHint: false, @@ -513,6 +623,9 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { projectsMethodUpdateProjectItem, projectsMethodDeleteProjectItem, projectsMethodCreateProjectStatusUpdate, + projectsMethodCreateProjectView, + projectsMethodUpdateProjectView, + projectsMethodDeleteProjectView, projectsMethodCreateProject, projectsMethodCreateIterationField, }, @@ -534,6 +647,30 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { Type: "string", Description: "The project title. Required for 'create_project' method.", }, + "view_id": { + Type: "string", + Description: "Project view node ID for update or delete.", + }, + "name": { + Type: "string", + Description: "View name; required when creating a view.", + }, + "layout": { + Type: "string", + Description: "View layout; required when creating a view.", + Enum: []any{"table", "board", "roadmap"}, + }, + "filter": { + Type: "string", + Description: "Saved view filter; omit on update to preserve it, or pass an empty string to clear it.", + }, + "visible_fields": { + Type: "array", + Description: "Field database IDs for table or board creation; unsupported for roadmap.", + Items: &jsonschema.Schema{ + Type: "string", + }, + }, "item_id": { Type: "number", Description: "The project item ID. Required for 'delete_project_item'. For 'update_project_item', provide either item_id, or (item_owner + item_repo + issue_number) to resolve the item by issue.", @@ -748,6 +885,12 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { return createProjectStatusUpdate(ctx, gqlClient, owner, ownerType, projectNumber, body, status, startDate, targetDate) case projectsMethodCreateIterationField: return createIterationField(ctx, gqlClient, owner, ownerType, projectNumber, args) + case projectsMethodCreateProjectView: + return createProjectView(ctx, client, args, owner, ownerType, projectNumber) + case projectsMethodUpdateProjectView: + return updateProjectView(ctx, gqlClient, args) + case projectsMethodDeleteProjectView: + return deleteProjectView(ctx, gqlClient, args) default: return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil } @@ -1511,6 +1654,309 @@ func getProjectStatusUpdate(ctx context.Context, gqlClient *githubv4.Client, sta return utils.NewToolResultText(string(r)), isPrivate, nil, nil } +func convertToMinimalProjectView(node projectViewNode) MinimalProjectView { + return MinimalProjectView{ + ID: fmt.Sprintf("%v", node.ID), + Number: int(node.Number), + Name: string(node.Name), + Layout: projectViewLayoutName(node.Layout), + Filter: derefString(node.Filter), + } +} + +func projectViewLayoutName(layout githubv4.ProjectV2ViewLayout) string { + switch layout { + case githubv4.ProjectV2ViewLayoutTableLayout: + return "table" + case githubv4.ProjectV2ViewLayoutBoardLayout: + return "board" + case githubv4.ProjectV2ViewLayoutRoadmapLayout: + return "roadmap" + default: + return strings.ToLower(strings.TrimSuffix(string(layout), "_LAYOUT")) + } +} + +func parseProjectViewLayout(layout string) (githubv4.ProjectV2ViewLayout, error) { + switch strings.ToLower(strings.TrimSpace(layout)) { + case "table": + return githubv4.ProjectV2ViewLayoutTableLayout, nil + case "board": + return githubv4.ProjectV2ViewLayoutBoardLayout, nil + case "roadmap": + return githubv4.ProjectV2ViewLayoutRoadmapLayout, nil + default: + return "", fmt.Errorf("invalid layout %q: must be \"table\", \"board\", or \"roadmap\"", layout) + } +} + +func listProjectViews(ctx context.Context, gqlClient *githubv4.Client, args map[string]any, owner, ownerType string) (*mcp.CallToolResult, bool, any, error) { + if ownerType != "user" && ownerType != "org" { + return utils.NewToolResultError(fmt.Sprintf("invalid owner_type %q: must be \"user\" or \"org\"", ownerType)), false, nil, nil + } + + projectNumber, err := RequiredInt(args, "project_number") + if err != nil { + return utils.NewToolResultError(err.Error()), false, nil, nil + } + perPage, err := OptionalIntParamWithDefault(args, "per_page", MaxProjectsPerPage) + if err != nil { + return utils.NewToolResultError(err.Error()), false, nil, nil + } + if perPage < 1 || perPage > MaxProjectsPerPage { + perPage = MaxProjectsPerPage + } + after, err := OptionalParam[string](args, "after") + if err != nil { + return utils.NewToolResultError(err.Error()), false, nil, nil + } + before, err := OptionalParam[string](args, "before") + if err != nil { + return utils.NewToolResultError(err.Error()), false, nil, nil + } + if after != "" && before != "" { + return utils.NewToolResultError("provide either 'after' or 'before', not both"), false, nil, nil + } + + vars := map[string]any{ + "owner": githubv4.String(owner), + "projectNumber": githubv4.Int(int32(projectNumber)), //nolint:gosec // Project numbers are small integers + "first": (*githubv4.Int)(nil), + "after": (*githubv4.String)(nil), + "last": (*githubv4.Int)(nil), + "before": (*githubv4.String)(nil), + } + if before != "" { + last := githubv4.Int(int32(perPage)) //nolint:gosec // perPage is bounded by MaxProjectsPerPage + cursor := githubv4.String(before) + vars["last"] = &last + vars["before"] = &cursor + } else { + first := githubv4.Int(int32(perPage)) //nolint:gosec // perPage is bounded by MaxProjectsPerPage + vars["first"] = &first + if after != "" { + cursor := githubv4.String(after) + vars["after"] = &cursor + } + } + + var project projectViewsProject + if ownerType == "org" { + var query projectViewsOrgQuery + if err := gqlClient.Query(ctx, &query, vars); err != nil { + return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewListFailedError, err)), false, nil, nil + } + project = query.Organization.ProjectV2 + } else { + var query projectViewsUserQuery + if err := gqlClient.Query(ctx, &query, vars); err != nil { + return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewListFailedError, err)), false, nil, nil + } + project = query.User.ProjectV2 + } + if project.ID == nil || project.ID == "" { + return utils.NewToolResultError(fmt.Sprintf("%s: project was not found", ProjectViewListFailedError)), false, nil, nil + } + + views := make([]MinimalProjectView, 0, len(project.Views.Nodes)) + for _, node := range project.Views.Nodes { + views = append(views, convertToMinimalProjectView(node)) + } + response := map[string]any{ + "views": views, + "pageInfo": map[string]any{ + "hasNextPage": project.Views.PageInfo.HasNextPage, + "hasPreviousPage": project.Views.PageInfo.HasPreviousPage, + "nextCursor": string(project.Views.PageInfo.EndCursor), + "prevCursor": string(project.Views.PageInfo.StartCursor), + }, + } + result, err := json.Marshal(response) + if err != nil { + return nil, false, nil, fmt.Errorf("failed to marshal response: %w", err) + } + return utils.NewToolResultText(string(result)), !bool(project.Public), nil, nil +} + +func getProjectView(ctx context.Context, gqlClient *githubv4.Client, viewID string) (*mcp.CallToolResult, bool, any, error) { + var query projectViewNodeQuery + vars := map[string]any{"id": githubv4.ID(viewID)} + if err := gqlClient.Query(ctx, &query, vars); err != nil { + return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewGetFailedError, err)), false, nil, nil + } + if query.Node.ProjectView.ID == nil || query.Node.ProjectView.ID == "" { + return utils.NewToolResultError(fmt.Sprintf("%s: node is not a ProjectV2View or was not found", ProjectViewGetFailedError)), false, nil, nil + } + + view := convertToMinimalProjectView(query.Node.ProjectView.projectViewNode) + result, err := json.Marshal(view) + if err != nil { + return nil, false, nil, fmt.Errorf("failed to marshal response: %w", err) + } + return utils.NewToolResultText(string(result)), !bool(query.Node.ProjectView.Project.Public), nil, nil +} + +func createProjectView(ctx context.Context, client *github.Client, args map[string]any, owner, ownerType string, projectNumber int) (*mcp.CallToolResult, any, error) { + name, err := RequiredParam[string](args, "name") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if strings.TrimSpace(name) == "" { + return utils.NewToolResultError("name must not be empty"), nil, nil + } + layoutName, err := RequiredParam[string](args, "layout") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + layout, err := parseProjectViewLayout(layoutName) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + filter, hasFilter, err := OptionalParamOK[string](args, "filter") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + visibleFields, err := OptionalBigIntArrayParam(args, "visible_fields") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if layout == githubv4.ProjectV2ViewLayoutRoadmapLayout && len(visibleFields) > 0 { + return utils.NewToolResultError("visible_fields is not supported for roadmap views"), nil, nil + } + + requestBody := CreateProjectV2ViewRequest{ + Name: name, + Layout: projectViewLayoutName(layout), + VisibleFields: visibleFields, + } + if hasFilter { + requestBody.Filter = &filter + } + + var endpoint string + switch ownerType { + case "org": + endpoint = fmt.Sprintf("orgs/%s/projectsV2/%d/views", owner, projectNumber) + case "user": + user, resp, err := client.Users.Get(ctx, owner) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, ProjectViewCreateFailedError, resp, err), nil, nil + } + userID := user.GetID() + if userID == 0 { + return utils.NewToolResultError(fmt.Sprintf("%s: user response did not include an ID", ProjectViewCreateFailedError)), nil, nil + } + endpoint = fmt.Sprintf("users/%d/projectsV2/%d/views", userID, projectNumber) + default: + return utils.NewToolResultError(fmt.Sprintf("invalid owner_type %q: must be \"user\" or \"org\"", ownerType)), nil, nil + } + + req, err := client.NewRequest(ctx, http.MethodPost, endpoint, requestBody) + if err != nil { + return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewCreateFailedError, err)), nil, nil + } + var response projectV2ViewRESTResponse + resp, err := client.Do(req, &response) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, ProjectViewCreateFailedError, resp, err), nil, nil + } + if response.NodeID == "" { + return utils.NewToolResultError(fmt.Sprintf("%s: response did not include a project view node ID", ProjectViewCreateFailedError)), nil, nil + } + + filterValue := "" + if response.Filter != nil { + filterValue = *response.Filter + } + view := MinimalProjectView{ + ID: response.NodeID, + Number: response.Number, + Name: response.Name, + Layout: projectViewLayoutName(githubv4.ProjectV2ViewLayout(response.Layout)), + Filter: filterValue, + VisibleFields: response.VisibleFields, + } + return MarshalledTextResult(view), nil, nil +} + +func updateProjectView(ctx context.Context, gqlClient *githubv4.Client, args map[string]any) (*mcp.CallToolResult, any, error) { + viewID, err := RequiredParam[string](args, "view_id") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + name, hasName, err := OptionalParamOK[string](args, "name") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + layoutName, hasLayout, err := OptionalParamOK[string](args, "layout") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + filter, hasFilter, err := OptionalParamOK[string](args, "filter") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if !hasName && !hasLayout && !hasFilter { + return utils.NewToolResultError("update_project_view requires at least one of name, layout, or filter"), nil, nil + } + if hasName && strings.TrimSpace(name) == "" { + return utils.NewToolResultError("name must not be empty"), nil, nil + } + + input := UpdateProjectV2ViewInput{ViewID: githubv4.ID(viewID)} + if hasName { + value := githubv4.String(name) + input.Name = &value + } + if hasLayout { + layout, err := parseProjectViewLayout(layoutName) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + input.Layout = &layout + } + if hasFilter { + value := githubv4.String(filter) + input.Filter = &value + } + + var mutation struct { + UpdateProjectV2View struct { + ProjectV2View projectViewNode + } `graphql:"updateProjectV2View(input: $input)"` + } + if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil { + return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewUpdateFailedError, err)), nil, nil + } + if mutation.UpdateProjectV2View.ProjectV2View.ID == nil || mutation.UpdateProjectV2View.ProjectV2View.ID == "" { + return utils.NewToolResultError(fmt.Sprintf("%s: response did not include a project view", ProjectViewUpdateFailedError)), nil, nil + } + return MarshalledTextResult(convertToMinimalProjectView(mutation.UpdateProjectV2View.ProjectV2View)), nil, nil +} + +func deleteProjectView(ctx context.Context, gqlClient *githubv4.Client, args map[string]any) (*mcp.CallToolResult, any, error) { + viewID, err := RequiredParam[string](args, "view_id") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + input := DeleteProjectV2ViewInput{ViewID: githubv4.ID(viewID)} + var mutation struct { + DeleteProjectV2View struct { + ProjectV2View struct { + ID githubv4.ID + } + } `graphql:"deleteProjectV2View(input: $input)"` + } + if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil { + return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewDeleteFailedError, err)), nil, nil + } + deletedID := fmt.Sprintf("%v", mutation.DeleteProjectV2View.ProjectV2View.ID) + if deletedID == "" || deletedID == "" { + return utils.NewToolResultError(fmt.Sprintf("%s: response did not include the deleted view", ProjectViewDeleteFailedError)), nil, nil + } + return MarshalledTextResult(map[string]string{"deleted_view_id": deletedID}), nil, nil +} + // validateAndConvertToInt64 ensures the value is a number and converts it to int64. func validateAndConvertToInt64(value any) (int64, error) { switch v := value.(type) { diff --git a/pkg/github/projects_test.go b/pkg/github/projects_test.go index 553c2421a4..b1b115259d 100644 --- a/pkg/github/projects_test.go +++ b/pkg/github/projects_test.go @@ -31,6 +31,7 @@ func Test_ProjectsList(t *testing.T) { assert.Contains(t, inputSchema.Properties, "project_number") assert.Contains(t, inputSchema.Properties, "query") assert.Contains(t, inputSchema.Properties, "fields") + assert.Contains(t, inputSchema.Properties["method"].Enum, projectsMethodListProjectViews) assert.ElementsMatch(t, inputSchema.Required, []string{"method", "owner"}) } @@ -591,6 +592,8 @@ func Test_ProjectsGet(t *testing.T) { assert.Contains(t, inputSchema.Properties, "owner") assert.Contains(t, inputSchema.Properties, "owner_type") assert.Contains(t, inputSchema.Properties, "project_number") + assert.Contains(t, inputSchema.Properties, "view_id") + assert.Contains(t, inputSchema.Properties["method"].Enum, projectsMethodGetProjectView) assert.Contains(t, inputSchema.Properties, "field_id") assert.Contains(t, inputSchema.Properties, "item_id") assert.ElementsMatch(t, inputSchema.Required, []string{"method"}) @@ -879,6 +882,14 @@ 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, "view_id") + assert.Contains(t, inputSchema.Properties, "name") + assert.Contains(t, inputSchema.Properties, "layout") + assert.Contains(t, inputSchema.Properties, "filter") + assert.Contains(t, inputSchema.Properties, "visible_fields") + assert.Contains(t, inputSchema.Properties["method"].Enum, projectsMethodCreateProjectView) + assert.Contains(t, inputSchema.Properties["method"].Enum, projectsMethodUpdateProjectView) + assert.Contains(t, inputSchema.Properties["method"].Enum, projectsMethodDeleteProjectView) assert.ElementsMatch(t, inputSchema.Required, []string{"method", "owner"}) // Verify DestructiveHint is set diff --git a/pkg/github/projects_v2_test.go b/pkg/github/projects_v2_test.go index 701e194767..ad57242f3e 100644 --- a/pkg/github/projects_v2_test.go +++ b/pkg/github/projects_v2_test.go @@ -455,3 +455,617 @@ func Test_ProjectsWrite_CreateIterationField(t *testing.T) { assert.Equal(t, "PVTIF_field1", response["id"]) }) } + +func Test_ProjectsList_ListProjectViews(t *testing.T) { + toolDef := ProjectsList(translations.NullTranslationHelper) + + t.Run("lists organization views with forward pagination and IFC", func(t *testing.T) { + first := githubv4.Int(2) + after := githubv4.String("after-cursor") + matcher := githubv4mock.NewQueryMatcher( + projectViewsOrgQuery{}, + map[string]any{ + "owner": githubv4.String("octo-org"), + "projectNumber": githubv4.Int(7), + "first": &first, + "after": &after, + "last": (*githubv4.Int)(nil), + "before": (*githubv4.String)(nil), + }, + githubv4mock.DataResponse(map[string]any{ + "organization": map[string]any{ + "projectV2": map[string]any{ + "id": "PVT_project7", + "public": false, + "views": map[string]any{ + "nodes": []map[string]any{ + { + "id": "PVTV_view1", + "number": 1, + "name": "Ready work", + "layout": "TABLE_LAYOUT", + "filter": "status:Ready", + }, + }, + "pageInfo": map[string]any{ + "hasNextPage": true, + "hasPreviousPage": false, + "startCursor": "start-cursor", + "endCursor": "end-cursor", + }, + }, + }, + }, + }), + ) + matcher.Variables["first"] = first + matcher.Variables["after"] = after + gqlClient := githubv4mock.NewMockedHTTPClient( + matcher, + ) + deps := BaseDeps{ + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), + GQLClient: githubv4.NewClient(gqlClient), + featureChecker: featureCheckerFor(FeatureFlagIFCLabels), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "list_project_views", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(7), + "per_page": float64(2), + "after": "after-cursor", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError, getTextResult(t, result).Text) + + var response struct { + Views []MinimalProjectView `json:"views"` + PageInfo map[string]any `json:"pageInfo"` + } + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &response)) + require.Len(t, response.Views, 1) + assert.Equal(t, MinimalProjectView{ + ID: "PVTV_view1", + Number: 1, + Name: "Ready work", + Layout: "table", + Filter: "status:Ready", + }, response.Views[0]) + assert.Equal(t, "end-cursor", response.PageInfo["nextCursor"]) + require.NotNil(t, result.Meta) + ifcMap := unmarshalIFC(t, result.Meta["ifc"]) + assert.Equal(t, "untrusted", ifcMap["integrity"]) + assert.Equal(t, "private", ifcMap["confidentiality"]) + }) + + t.Run("lists user views with backward pagination", func(t *testing.T) { + last := githubv4.Int(3) + before := githubv4.String("before-cursor") + matcher := githubv4mock.NewQueryMatcher( + projectViewsUserQuery{}, + map[string]any{ + "owner": githubv4.String("octocat"), + "projectNumber": githubv4.Int(8), + "first": (*githubv4.Int)(nil), + "after": (*githubv4.String)(nil), + "last": &last, + "before": &before, + }, + githubv4mock.DataResponse(map[string]any{ + "user": map[string]any{ + "projectV2": map[string]any{ + "id": "PVT_project8", + "public": true, + "views": map[string]any{ + "nodes": []map[string]any{}, + "pageInfo": map[string]any{ + "hasNextPage": false, + "hasPreviousPage": true, + "startCursor": "previous-cursor", + "endCursor": "", + }, + }, + }, + }, + }), + ) + matcher.Variables["last"] = last + matcher.Variables["before"] = before + gqlClient := githubv4mock.NewMockedHTTPClient( + matcher, + ) + deps := BaseDeps{ + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), + GQLClient: githubv4.NewClient(gqlClient), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "list_project_views", + "owner": "octocat", + "owner_type": "user", + "project_number": float64(8), + "per_page": float64(3), + "before": "before-cursor", + }) + + 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)) + pageInfo := response["pageInfo"].(map[string]any) + assert.Equal(t, "previous-cursor", pageInfo["prevCursor"]) + }) + + t.Run("rejects conflicting cursors", func(t *testing.T) { + deps := BaseDeps{ + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), + GQLClient: githubv4.NewClient(githubv4mock.NewMockedHTTPClient()), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "list_project_views", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(7), + "after": "a", + "before": "b", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "provide either 'after' or 'before'") + }) +} + +func Test_ProjectsGet_GetProjectView(t *testing.T) { + toolDef := ProjectsGet(translations.NullTranslationHelper) + + t.Run("gets a private project view by node ID", func(t *testing.T) { + gqlClient := githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + projectViewNodeQuery{}, + map[string]any{"id": githubv4.ID("PVTV_view1")}, + githubv4mock.DataResponse(map[string]any{ + "node": map[string]any{ + "id": "PVTV_view1", + "number": 1, + "name": "Ready work", + "layout": "BOARD_LAYOUT", + "filter": "status:Ready", + "project": map[string]any{"public": false}, + }, + }), + ), + ) + deps := BaseDeps{ + GQLClient: githubv4.NewClient(gqlClient), + featureChecker: featureCheckerFor(FeatureFlagIFCLabels), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "get_project_view", + "view_id": "PVTV_view1", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + + var view MinimalProjectView + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &view)) + assert.Equal(t, "PVTV_view1", view.ID) + assert.Equal(t, "board", view.Layout) + require.NotNil(t, result.Meta) + ifcMap := unmarshalIFC(t, result.Meta["ifc"]) + assert.Equal(t, "private", ifcMap["confidentiality"]) + }) + + t.Run("rejects a missing or wrong node type", func(t *testing.T) { + gqlClient := githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + projectViewNodeQuery{}, + map[string]any{"id": githubv4.ID("I_issue1")}, + githubv4mock.DataResponse(map[string]any{"node": map[string]any{}}), + ), + ) + deps := BaseDeps{GQLClient: githubv4.NewClient(gqlClient)} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "get_project_view", + "view_id": "I_issue1", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "node is not a ProjectV2View or was not found") + }) +} + +func Test_ProjectsWrite_CreateProjectView(t *testing.T) { + toolDef := ProjectsWrite(translations.NullTranslationHelper) + + t.Run("creates organization view with filter and visible fields", func(t *testing.T) { + restClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "POST /orgs/{org}/projectsV2/{project}/views": func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, "/orgs/octo-org/projectsV2/7/views", r.URL.Path) + var body map[string]any + require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) + assert.Equal(t, "Ready work", body["name"]) + assert.Equal(t, "table", body["layout"]) + assert.Equal(t, "status:Ready", body["filter"]) + assert.Equal(t, []any{float64(101), float64(202)}, body["visible_fields"]) + mockResponse(t, http.StatusCreated, map[string]any{ + "node_id": "PVTV_view1", + "number": 1, + "name": "Ready work", + "layout": "table", + "filter": "status:Ready", + "visible_fields": []int64{101, 202}, + })(w, r) + }, + }) + deps := BaseDeps{ + Client: mustNewGHClient(t, restClient), + GQLClient: githubv4.NewClient(githubv4mock.NewMockedHTTPClient()), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "create_project_view", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(7), + "name": "Ready work", + "layout": "table", + "filter": "status:Ready", + "visible_fields": []any{"101", "202"}, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + + var view MinimalProjectView + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &view)) + assert.Equal(t, "PVTV_view1", view.ID) + assert.Equal(t, []int64{101, 202}, view.VisibleFields) + }) + + t.Run("resolves a user login to the numeric REST user ID", func(t *testing.T) { + restClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetUsersByUsername: mockResponse(t, http.StatusOK, map[string]any{ + "id": 42, + "type": "User", + }), + "POST /users/{user_id}/projectsV2/{project}/views": func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, "/users/42/projectsV2/8/views", r.URL.Path) + mockResponse(t, http.StatusCreated, map[string]any{ + "node_id": "PVTV_view2", + "number": 2, + "name": "Board", + "layout": "board", + })(w, r) + }, + }) + deps := BaseDeps{ + Client: mustNewGHClient(t, restClient), + GQLClient: githubv4.NewClient(githubv4mock.NewMockedHTTPClient()), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "create_project_view", + "owner": "octocat", + "owner_type": "user", + "project_number": float64(8), + "name": "Board", + "layout": "board", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, `"id":"PVTV_view2"`) + }) + + t.Run("auto-detects an organization owner", func(t *testing.T) { + restClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetUsersByUsername: mockResponse(t, http.StatusOK, map[string]any{ + "id": 99, + "type": "Organization", + }), + "POST /orgs/{org}/projectsV2/{project}/views": mockResponse(t, http.StatusCreated, map[string]any{ + "node_id": "PVTV_view3", + "number": 3, + "name": "Table", + "layout": "table", + }), + }) + deps := BaseDeps{ + Client: mustNewGHClient(t, restClient), + GQLClient: githubv4.NewClient(githubv4mock.NewMockedHTTPClient()), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "create_project_view", + "owner": "octo-org", + "project_number": float64(9), + "name": "Table", + "layout": "table", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + }) + + t.Run("rejects visible fields for roadmap layout", func(t *testing.T) { + deps := BaseDeps{ + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), + GQLClient: githubv4.NewClient(githubv4mock.NewMockedHTTPClient()), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "create_project_view", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(7), + "name": "Roadmap", + "layout": "roadmap", + "visible_fields": []any{"101"}, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "visible_fields is not supported for roadmap views") + }) +} + +func Test_ProjectsWrite_UpdateProjectView(t *testing.T) { + toolDef := ProjectsWrite(translations.NullTranslationHelper) + + t.Run("updates only the supplied name", func(t *testing.T) { + name := githubv4.String("Renamed") + gqlClient := githubv4mock.NewMockedHTTPClient( + githubv4mock.NewMutationMatcher( + struct { + UpdateProjectV2View struct { + ProjectV2View projectViewNode + } `graphql:"updateProjectV2View(input: $input)"` + }{}, + UpdateProjectV2ViewInput{ + ViewID: githubv4.ID("PVTV_view1"), + Name: &name, + }, + nil, + githubv4mock.DataResponse(map[string]any{ + "updateProjectV2View": map[string]any{ + "projectV2View": map[string]any{ + "id": "PVTV_view1", + "number": 1, + "name": "Renamed", + "layout": "TABLE_LAYOUT", + "filter": "status:Ready", + }, + }, + }), + ), + ) + deps := BaseDeps{ + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), + GQLClient: githubv4.NewClient(gqlClient), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "update_project_view", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(7), + "view_id": "PVTV_view1", + "name": "Renamed", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, `"name":"Renamed"`) + }) + + t.Run("sends an explicit empty filter to clear it", func(t *testing.T) { + filter := githubv4.String("") + gqlClient := githubv4mock.NewMockedHTTPClient( + githubv4mock.NewMutationMatcher( + struct { + UpdateProjectV2View struct { + ProjectV2View projectViewNode + } `graphql:"updateProjectV2View(input: $input)"` + }{}, + UpdateProjectV2ViewInput{ + ViewID: githubv4.ID("PVTV_view1"), + Filter: &filter, + }, + nil, + githubv4mock.DataResponse(map[string]any{ + "updateProjectV2View": map[string]any{ + "projectV2View": map[string]any{ + "id": "PVTV_view1", + "number": 1, + "name": "Renamed", + "layout": "TABLE_LAYOUT", + "filter": "", + }, + }, + }), + ), + ) + deps := BaseDeps{ + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), + GQLClient: githubv4.NewClient(gqlClient), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "update_project_view", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(7), + "view_id": "PVTV_view1", + "filter": "", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, `"filter":""`) + }) + + t.Run("normalizes an updated layout to the GraphQL enum", func(t *testing.T) { + layout := githubv4.ProjectV2ViewLayoutBoardLayout + gqlClient := githubv4mock.NewMockedHTTPClient( + githubv4mock.NewMutationMatcher( + struct { + UpdateProjectV2View struct { + ProjectV2View projectViewNode + } `graphql:"updateProjectV2View(input: $input)"` + }{}, + UpdateProjectV2ViewInput{ + ViewID: githubv4.ID("PVTV_view1"), + Layout: &layout, + }, + nil, + githubv4mock.DataResponse(map[string]any{ + "updateProjectV2View": map[string]any{ + "projectV2View": map[string]any{ + "id": "PVTV_view1", + "number": 1, + "name": "Board", + "layout": "BOARD_LAYOUT", + "filter": "", + }, + }, + }), + ), + ) + deps := BaseDeps{ + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), + GQLClient: githubv4.NewClient(gqlClient), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "update_project_view", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(7), + "view_id": "PVTV_view1", + "layout": "board", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, `"layout":"board"`) + }) + + t.Run("surfaces GraphQL API errors", func(t *testing.T) { + name := githubv4.String("Renamed") + gqlClient := githubv4mock.NewMockedHTTPClient( + githubv4mock.NewMutationMatcher( + struct { + UpdateProjectV2View struct { + ProjectV2View projectViewNode + } `graphql:"updateProjectV2View(input: $input)"` + }{}, + UpdateProjectV2ViewInput{ + ViewID: githubv4.ID("PVTV_view1"), + Name: &name, + }, + nil, + githubv4mock.ErrorResponse("update failed"), + ), + ) + deps := BaseDeps{ + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), + GQLClient: githubv4.NewClient(gqlClient), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "update_project_view", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(7), + "view_id": "PVTV_view1", + "name": "Renamed", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, ProjectViewUpdateFailedError) + assert.Contains(t, getTextResult(t, result).Text, "update failed") + }) + + t.Run("rejects an empty update", func(t *testing.T) { + deps := BaseDeps{ + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), + GQLClient: githubv4.NewClient(githubv4mock.NewMockedHTTPClient()), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "update_project_view", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(7), + "view_id": "PVTV_view1", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "requires at least one of name, layout, or filter") + }) +} + +func Test_ProjectsWrite_DeleteProjectView(t *testing.T) { + toolDef := ProjectsWrite(translations.NullTranslationHelper) + gqlClient := githubv4mock.NewMockedHTTPClient( + githubv4mock.NewMutationMatcher( + struct { + DeleteProjectV2View struct { + ProjectV2View struct { + ID githubv4.ID + } + } `graphql:"deleteProjectV2View(input: $input)"` + }{}, + DeleteProjectV2ViewInput{ViewID: githubv4.ID("PVTV_view1")}, + nil, + githubv4mock.DataResponse(map[string]any{ + "deleteProjectV2View": map[string]any{ + "projectV2View": map[string]any{"id": "PVTV_view1"}, + }, + }), + ), + ) + deps := BaseDeps{ + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), + GQLClient: githubv4.NewClient(gqlClient), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "delete_project_view", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(7), + "view_id": "PVTV_view1", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + assert.JSONEq(t, `{"deleted_view_id":"PVTV_view1"}`, getTextResult(t, result).Text) +} diff --git a/pkg/github/toolset_instructions.go b/pkg/github/toolset_instructions.go index ba6659612a..3b3a54eadd 100644 --- a/pkg/github/toolset_instructions.go +++ b/pkg/github/toolset_instructions.go @@ -41,6 +41,8 @@ Workflow: 1) list_project_fields (get field IDs), 2) list_project_items (with pa Project lifecycle: Use create_project to create a new ProjectsV2 for a user or organization (requires owner_type and title). Returns the new project's id, number, title, and url; pass the returned number as project_number to subsequent project tools. +Views: Use list_project_views and get_project_view to inspect views. Use create_project_view, update_project_view, and delete_project_view for basic name, layout, and filter management; visible_fields is create-only and unavailable for roadmap views. + Iteration fields: Use create_iteration_field to add a new ITERATION field (e.g. "Sprint") to an existing project. Required: field_name, iteration_duration (days), start_date (YYYY-MM-DD). Only pass the iterations array when iterations need varying durations, breaks between them, or specific titles; otherwise omit it and GitHub creates three default iterations of iteration_duration days starting on start_date. Status updates: Use list_project_status_updates to read recent project status updates (newest first). Use get_project_status_update with a node ID to get a single update. Use create_project_status_update to create a new status update for a project. From e96511dbf6de479f563122ef3d2ce721da497ca9 Mon Sep 17 00:00:00 2001 From: Bryan Zwicker Date: Tue, 28 Jul 2026 10:23:15 -0400 Subject: [PATCH 2/3] Harden project view mutations Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c6f8ede6-efee-4191-900d-59a1bb0af000 --- README.md | 2 +- pkg/github/__toolsnaps__/projects_write.snap | 2 +- pkg/github/projects.go | 63 +++-- pkg/github/projects_v2_test.go | 281 ++++++++++++++++--- 4 files changed, 287 insertions(+), 61 deletions(-) diff --git a/README.md b/README.md index 83ea726953..b69d6a9ca8 100644 --- a/README.md +++ b/README.md @@ -1137,7 +1137,7 @@ The following sets of tools are available: - `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) - - `view_id`: Project view node ID for update or delete. (string, optional) + - `view_id`: Project view node ID for update or delete; must belong to owner/project_number. (string, optional) - `visible_fields`: Field database IDs for table or board creation; unsupported for roadmap. (string[], optional) diff --git a/pkg/github/__toolsnaps__/projects_write.snap b/pkg/github/__toolsnaps__/projects_write.snap index f8aa795ce3..c82e7de6ad 100644 --- a/pkg/github/__toolsnaps__/projects_write.snap +++ b/pkg/github/__toolsnaps__/projects_write.snap @@ -151,7 +151,7 @@ "type": "object" }, "view_id": { - "description": "Project view node ID for update or delete.", + "description": "Project view node ID for update or delete; must belong to owner/project_number.", "type": "string" }, "visible_fields": { diff --git a/pkg/github/projects.go b/pkg/github/projects.go index 893e4a8141..f4acdfa290 100644 --- a/pkg/github/projects.go +++ b/pkg/github/projects.go @@ -159,6 +159,17 @@ type projectViewNodeQuery struct { } `graphql:"node(id: $id)"` } +type projectViewParentQuery struct { + Node struct { + ProjectView struct { + ID githubv4.ID + Project struct { + ID githubv4.ID + } + } `graphql:"... on ProjectV2View"` + } `graphql:"node(id: $id)"` +} + // CreateProjectV2ViewRequest is the REST request for creating a project view. type CreateProjectV2ViewRequest struct { Name string `json:"name"` @@ -649,7 +660,7 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { }, "view_id": { Type: "string", - Description: "Project view node ID for update or delete.", + Description: "Project view node ID for update or delete; must belong to owner/project_number.", }, "name": { Type: "string", @@ -888,9 +899,9 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { case projectsMethodCreateProjectView: return createProjectView(ctx, client, args, owner, ownerType, projectNumber) case projectsMethodUpdateProjectView: - return updateProjectView(ctx, gqlClient, args) + return updateProjectView(ctx, gqlClient, args, owner, ownerType, projectNumber) case projectsMethodDeleteProjectView: - return deleteProjectView(ctx, gqlClient, args) + return deleteProjectView(ctx, gqlClient, args, owner, ownerType, projectNumber) default: return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil } @@ -1838,15 +1849,7 @@ func createProjectView(ctx context.Context, client *github.Client, args map[stri case "org": endpoint = fmt.Sprintf("orgs/%s/projectsV2/%d/views", owner, projectNumber) case "user": - user, resp, err := client.Users.Get(ctx, owner) - if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, ProjectViewCreateFailedError, resp, err), nil, nil - } - userID := user.GetID() - if userID == 0 { - return utils.NewToolResultError(fmt.Sprintf("%s: user response did not include an ID", ProjectViewCreateFailedError)), nil, nil - } - endpoint = fmt.Sprintf("users/%d/projectsV2/%d/views", userID, projectNumber) + endpoint = fmt.Sprintf("users/%s/projectsV2/%d/views", owner, projectNumber) default: return utils.NewToolResultError(fmt.Sprintf("invalid owner_type %q: must be \"user\" or \"org\"", ownerType)), nil, nil } @@ -1879,7 +1882,29 @@ func createProjectView(ctx context.Context, client *github.Client, args map[stri return MarshalledTextResult(view), nil, nil } -func updateProjectView(ctx context.Context, gqlClient *githubv4.Client, args map[string]any) (*mcp.CallToolResult, any, error) { +func verifyProjectViewParent(ctx context.Context, gqlClient *githubv4.Client, viewID, owner, ownerType string, projectNumber int) error { + expectedProjectID, err := resolveProjectNodeID(ctx, gqlClient, owner, ownerType, projectNumber) + if err != nil { + return fmt.Errorf("failed to resolve requested project: %w", err) + } + if expectedProjectID == nil || expectedProjectID == "" { + return fmt.Errorf("requested project was not found") + } + + var query projectViewParentQuery + if err := gqlClient.Query(ctx, &query, map[string]any{"id": githubv4.ID(viewID)}); err != nil { + return fmt.Errorf("failed to resolve project view: %w", err) + } + if query.Node.ProjectView.ID == nil || query.Node.ProjectView.ID == "" { + return fmt.Errorf("node is not a ProjectV2View or was not found") + } + if query.Node.ProjectView.Project.ID != expectedProjectID { + return fmt.Errorf("project view does not belong to the requested project") + } + return nil +} + +func updateProjectView(ctx context.Context, gqlClient *githubv4.Client, args map[string]any, owner, ownerType string, projectNumber int) (*mcp.CallToolResult, any, error) { viewID, err := RequiredParam[string](args, "view_id") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil @@ -1919,10 +1944,13 @@ func updateProjectView(ctx context.Context, gqlClient *githubv4.Client, args map value := githubv4.String(filter) input.Filter = &value } + if err := verifyProjectViewParent(ctx, gqlClient, viewID, owner, ownerType, projectNumber); err != nil { + return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewUpdateFailedError, err)), nil, nil + } var mutation struct { UpdateProjectV2View struct { - ProjectV2View projectViewNode + ProjectV2View projectViewNode `graphql:"projectV2View"` } `graphql:"updateProjectV2View(input: $input)"` } if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil { @@ -1934,17 +1962,20 @@ func updateProjectView(ctx context.Context, gqlClient *githubv4.Client, args map return MarshalledTextResult(convertToMinimalProjectView(mutation.UpdateProjectV2View.ProjectV2View)), nil, nil } -func deleteProjectView(ctx context.Context, gqlClient *githubv4.Client, args map[string]any) (*mcp.CallToolResult, any, error) { +func deleteProjectView(ctx context.Context, gqlClient *githubv4.Client, args map[string]any, owner, ownerType string, projectNumber int) (*mcp.CallToolResult, any, error) { viewID, err := RequiredParam[string](args, "view_id") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } + if err := verifyProjectViewParent(ctx, gqlClient, viewID, owner, ownerType, projectNumber); err != nil { + return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewDeleteFailedError, err)), nil, nil + } input := DeleteProjectV2ViewInput{ViewID: githubv4.ID(viewID)} var mutation struct { DeleteProjectV2View struct { ProjectV2View struct { ID githubv4.ID - } + } `graphql:"projectV2View"` } `graphql:"deleteProjectV2View(input: $input)"` } if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil { diff --git a/pkg/github/projects_v2_test.go b/pkg/github/projects_v2_test.go index ad57242f3e..0be6d2a9bb 100644 --- a/pkg/github/projects_v2_test.go +++ b/pkg/github/projects_v2_test.go @@ -165,6 +165,50 @@ func resolveProjectNodeIDOrgMatcher(owner string, projectNumber int, nodeID stri ) } +func resolveProjectNodeIDUserMatcher(owner string, projectNumber int, nodeID string) githubv4mock.Matcher { + return githubv4mock.NewQueryMatcher( + struct { + User struct { + ProjectV2 struct { + ID githubv4.ID + } `graphql:"projectV2(number: $projectNumber)"` + } `graphql:"user(login: $owner)"` + }{}, + map[string]any{ + "owner": githubv4.String(owner), + "projectNumber": githubv4.Int(int32(projectNumber)), //nolint:gosec // test constant + }, + githubv4mock.DataResponse(map[string]any{ + "user": map[string]any{ + "projectV2": map[string]any{ + "id": nodeID, + }, + }, + }), + ) +} + +func projectViewParentMatcher(viewID, projectID string) githubv4mock.Matcher { + return githubv4mock.NewQueryMatcher( + projectViewParentQuery{}, + map[string]any{"id": githubv4.ID(viewID)}, + githubv4mock.DataResponse(map[string]any{ + "node": map[string]any{ + "id": viewID, + "project": map[string]any{"id": projectID}, + }, + }), + ) +} + +func projectViewParentErrorMatcher(viewID, message string) githubv4mock.Matcher { + return githubv4mock.NewQueryMatcher( + projectViewParentQuery{}, + map[string]any{"id": githubv4.ID(viewID)}, + githubv4mock.ErrorResponse(message), + ) +} + func createFieldMatcher() githubv4mock.Matcher { return githubv4mock.NewMutationMatcher( struct { @@ -738,14 +782,10 @@ func Test_ProjectsWrite_CreateProjectView(t *testing.T) { assert.Equal(t, []int64{101, 202}, view.VisibleFields) }) - t.Run("resolves a user login to the numeric REST user ID", func(t *testing.T) { + t.Run("creates a user view by login", func(t *testing.T) { restClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - GetUsersByUsername: mockResponse(t, http.StatusOK, map[string]any{ - "id": 42, - "type": "User", - }), "POST /users/{user_id}/projectsV2/{project}/views": func(w http.ResponseWriter, r *http.Request) { - require.Equal(t, "/users/42/projectsV2/8/views", r.URL.Path) + require.Equal(t, "/users/octocat/projectsV2/8/views", r.URL.Path) mockResponse(t, http.StatusCreated, map[string]any{ "node_id": "PVTV_view2", "number": 2, @@ -834,10 +874,12 @@ func Test_ProjectsWrite_UpdateProjectView(t *testing.T) { t.Run("updates only the supplied name", func(t *testing.T) { name := githubv4.String("Renamed") gqlClient := githubv4mock.NewMockedHTTPClient( + resolveProjectNodeIDOrgMatcher("octo-org", 7, "PVT_project7"), + projectViewParentMatcher("PVTV_view1", "PVT_project7"), githubv4mock.NewMutationMatcher( struct { UpdateProjectV2View struct { - ProjectV2View projectViewNode + ProjectV2View projectViewNode `graphql:"projectV2View"` } `graphql:"updateProjectV2View(input: $input)"` }{}, UpdateProjectV2ViewInput{ @@ -881,10 +923,12 @@ func Test_ProjectsWrite_UpdateProjectView(t *testing.T) { t.Run("sends an explicit empty filter to clear it", func(t *testing.T) { filter := githubv4.String("") gqlClient := githubv4mock.NewMockedHTTPClient( + resolveProjectNodeIDOrgMatcher("octo-org", 7, "PVT_project7"), + projectViewParentMatcher("PVTV_view1", "PVT_project7"), githubv4mock.NewMutationMatcher( struct { UpdateProjectV2View struct { - ProjectV2View projectViewNode + ProjectV2View projectViewNode `graphql:"projectV2View"` } `graphql:"updateProjectV2View(input: $input)"` }{}, UpdateProjectV2ViewInput{ @@ -928,10 +972,12 @@ func Test_ProjectsWrite_UpdateProjectView(t *testing.T) { t.Run("normalizes an updated layout to the GraphQL enum", func(t *testing.T) { layout := githubv4.ProjectV2ViewLayoutBoardLayout gqlClient := githubv4mock.NewMockedHTTPClient( + resolveProjectNodeIDOrgMatcher("octo-org", 7, "PVT_project7"), + projectViewParentMatcher("PVTV_view1", "PVT_project7"), githubv4mock.NewMutationMatcher( struct { UpdateProjectV2View struct { - ProjectV2View projectViewNode + ProjectV2View projectViewNode `graphql:"projectV2View"` } `graphql:"updateProjectV2View(input: $input)"` }{}, UpdateProjectV2ViewInput{ @@ -975,10 +1021,12 @@ func Test_ProjectsWrite_UpdateProjectView(t *testing.T) { t.Run("surfaces GraphQL API errors", func(t *testing.T) { name := githubv4.String("Renamed") gqlClient := githubv4mock.NewMockedHTTPClient( + resolveProjectNodeIDOrgMatcher("octo-org", 7, "PVT_project7"), + projectViewParentMatcher("PVTV_view1", "PVT_project7"), githubv4mock.NewMutationMatcher( struct { UpdateProjectV2View struct { - ProjectV2View projectViewNode + ProjectV2View projectViewNode `graphql:"projectV2View"` } `graphql:"updateProjectV2View(input: $input)"` }{}, UpdateProjectV2ViewInput{ @@ -1010,6 +1058,78 @@ func Test_ProjectsWrite_UpdateProjectView(t *testing.T) { assert.Contains(t, getTextResult(t, result).Text, "update failed") }) + for _, tc := range []struct { + name string + owner string + ownerType string + resolveReq githubv4mock.Matcher + }{ + { + name: "rejects organization project mismatch", + owner: "octo-org", + ownerType: "org", + resolveReq: resolveProjectNodeIDOrgMatcher("octo-org", 7, "PVT_org_project"), + }, + { + name: "rejects user project mismatch", + owner: "octocat", + ownerType: "user", + resolveReq: resolveProjectNodeIDUserMatcher("octocat", 7, "PVT_user_project"), + }, + } { + t.Run(tc.name, func(t *testing.T) { + gqlClient := githubv4mock.NewMockedHTTPClient( + tc.resolveReq, + projectViewParentMatcher("PVTV_view1", "PVT_other_project"), + ) + deps := BaseDeps{ + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), + GQLClient: githubv4.NewClient(gqlClient), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "update_project_view", + "owner": tc.owner, + "owner_type": tc.ownerType, + "project_number": float64(7), + "view_id": "PVTV_view1", + "name": "Renamed", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, ProjectViewUpdateFailedError) + assert.Contains(t, getTextResult(t, result).Text, "project view does not belong to the requested project") + }) + } + + t.Run("surfaces parent verification API errors", func(t *testing.T) { + gqlClient := githubv4mock.NewMockedHTTPClient( + resolveProjectNodeIDOrgMatcher("octo-org", 7, "PVT_project7"), + projectViewParentErrorMatcher("PVTV_view1", "lookup failed"), + ) + deps := BaseDeps{ + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), + GQLClient: githubv4.NewClient(gqlClient), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "update_project_view", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(7), + "view_id": "PVTV_view1", + "name": "Renamed", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, ProjectViewUpdateFailedError) + assert.Contains(t, getTextResult(t, result).Text, "failed to resolve project view: lookup failed") + }) + t.Run("rejects an empty update", func(t *testing.T) { deps := BaseDeps{ Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), @@ -1033,39 +1153,114 @@ func Test_ProjectsWrite_UpdateProjectView(t *testing.T) { func Test_ProjectsWrite_DeleteProjectView(t *testing.T) { toolDef := ProjectsWrite(translations.NullTranslationHelper) - gqlClient := githubv4mock.NewMockedHTTPClient( - githubv4mock.NewMutationMatcher( - struct { - DeleteProjectV2View struct { - ProjectV2View struct { - ID githubv4.ID - } - } `graphql:"deleteProjectV2View(input: $input)"` - }{}, - DeleteProjectV2ViewInput{ViewID: githubv4.ID("PVTV_view1")}, - nil, - githubv4mock.DataResponse(map[string]any{ - "deleteProjectV2View": map[string]any{ - "projectV2View": map[string]any{"id": "PVTV_view1"}, - }, - }), - ), - ) - deps := BaseDeps{ - Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), - GQLClient: githubv4.NewClient(gqlClient), - } - handler := toolDef.Handler(deps) - request := createMCPRequest(map[string]any{ - "method": "delete_project_view", - "owner": "octo-org", - "owner_type": "org", - "project_number": float64(7), - "view_id": "PVTV_view1", + + t.Run("deletes a view from the requested project", func(t *testing.T) { + gqlClient := githubv4mock.NewMockedHTTPClient( + resolveProjectNodeIDOrgMatcher("octo-org", 7, "PVT_project7"), + projectViewParentMatcher("PVTV_view1", "PVT_project7"), + githubv4mock.NewMutationMatcher( + struct { + DeleteProjectV2View struct { + ProjectV2View struct { + ID githubv4.ID + } `graphql:"projectV2View"` + } `graphql:"deleteProjectV2View(input: $input)"` + }{}, + DeleteProjectV2ViewInput{ViewID: githubv4.ID("PVTV_view1")}, + nil, + githubv4mock.DataResponse(map[string]any{ + "deleteProjectV2View": map[string]any{ + "projectV2View": map[string]any{"id": "PVTV_view1"}, + }, + }), + ), + ) + deps := BaseDeps{ + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), + GQLClient: githubv4.NewClient(gqlClient), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "delete_project_view", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(7), + "view_id": "PVTV_view1", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + assert.JSONEq(t, `{"deleted_view_id":"PVTV_view1"}`, getTextResult(t, result).Text) }) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) - assert.JSONEq(t, `{"deleted_view_id":"PVTV_view1"}`, getTextResult(t, result).Text) + for _, tc := range []struct { + name string + owner string + ownerType string + resolveReq githubv4mock.Matcher + }{ + { + name: "rejects organization project mismatch", + owner: "octo-org", + ownerType: "org", + resolveReq: resolveProjectNodeIDOrgMatcher("octo-org", 7, "PVT_org_project"), + }, + { + name: "rejects user project mismatch", + owner: "octocat", + ownerType: "user", + resolveReq: resolveProjectNodeIDUserMatcher("octocat", 7, "PVT_user_project"), + }, + } { + t.Run(tc.name, func(t *testing.T) { + gqlClient := githubv4mock.NewMockedHTTPClient( + tc.resolveReq, + projectViewParentMatcher("PVTV_view1", "PVT_other_project"), + ) + deps := BaseDeps{ + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), + GQLClient: githubv4.NewClient(gqlClient), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "delete_project_view", + "owner": tc.owner, + "owner_type": tc.ownerType, + "project_number": float64(7), + "view_id": "PVTV_view1", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, ProjectViewDeleteFailedError) + assert.Contains(t, getTextResult(t, result).Text, "project view does not belong to the requested project") + }) + } + + t.Run("surfaces parent verification API errors", func(t *testing.T) { + gqlClient := githubv4mock.NewMockedHTTPClient( + resolveProjectNodeIDOrgMatcher("octo-org", 7, "PVT_project7"), + projectViewParentErrorMatcher("PVTV_view1", "lookup failed"), + ) + deps := BaseDeps{ + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), + GQLClient: githubv4.NewClient(gqlClient), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "delete_project_view", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(7), + "view_id": "PVTV_view1", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, ProjectViewDeleteFailedError) + assert.Contains(t, getTextResult(t, result).Text, "failed to resolve project view: lookup failed") + }) } From e519780476b836a571e3bbceffcf247035b726f9 Mon Sep 17 00:00:00 2001 From: Bryan Zwicker Date: Tue, 28 Jul 2026 10:25:04 -0400 Subject: [PATCH 3/3] Resolve project view fields by name Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c6f8ede6-efee-4191-900d-59a1bb0af000 --- README.md | 3 +- pkg/github/__toolsnaps__/projects_write.snap | 9 +- pkg/github/projects.go | 58 ++++- pkg/github/projects_test.go | 1 + pkg/github/projects_v2_test.go | 234 +++++++++++++++++-- 5 files changed, 279 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index b69d6a9ca8..a570d06bdb 100644 --- a/README.md +++ b/README.md @@ -1138,7 +1138,8 @@ The following sets of tools are available: - `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) - `view_id`: Project view node ID for update or delete; must belong to owner/project_number. (string, optional) - - `visible_fields`: Field database IDs for table or board creation; unsupported for roadmap. (string[], optional) + - `visible_field_names`: Field names for table or board creation; mutually exclusive with visible_fields. (string[], optional) + - `visible_fields`: Field database IDs for table or board creation; mutually exclusive with visible_field_names. (string[], optional) diff --git a/pkg/github/__toolsnaps__/projects_write.snap b/pkg/github/__toolsnaps__/projects_write.snap index c82e7de6ad..27461406a7 100644 --- a/pkg/github/__toolsnaps__/projects_write.snap +++ b/pkg/github/__toolsnaps__/projects_write.snap @@ -154,8 +154,15 @@ "description": "Project view node ID for update or delete; must belong to owner/project_number.", "type": "string" }, + "visible_field_names": { + "description": "Field names for table or board creation; mutually exclusive with visible_fields.", + "items": { + "type": "string" + }, + "type": "array" + }, "visible_fields": { - "description": "Field database IDs for table or board creation; unsupported for roadmap.", + "description": "Field database IDs for table or board creation; mutually exclusive with visible_field_names.", "items": { "type": "string" }, diff --git a/pkg/github/projects.go b/pkg/github/projects.go index f4acdfa290..930b9e9ee7 100644 --- a/pkg/github/projects.go +++ b/pkg/github/projects.go @@ -677,7 +677,14 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { }, "visible_fields": { Type: "array", - Description: "Field database IDs for table or board creation; unsupported for roadmap.", + Description: "Field database IDs for table or board creation; mutually exclusive with visible_field_names.", + Items: &jsonschema.Schema{ + Type: "string", + }, + }, + "visible_field_names": { + Type: "array", + Description: "Field names for table or board creation; mutually exclusive with visible_fields.", Items: &jsonschema.Schema{ Type: "string", }, @@ -780,13 +787,12 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { return utils.NewToolResultError(err.Error()), nil, nil } - gqlClient, err := deps.GetGQLClient(ctx) - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } - // create_project does not require project_number or a REST client if method == projectsMethodCreateProject { + gqlClient, gqlErr := deps.GetGQLClient(ctx) + if gqlErr != nil { + return utils.NewToolResultError(gqlErr.Error()), nil, nil + } return createProject(ctx, gqlClient, owner, ownerType, args) } @@ -808,6 +814,26 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { } } + if method == projectsMethodCreateProjectView { + visibleFieldNames, namesErr := OptionalStringArrayParam(args, "visible_field_names") + if namesErr != nil { + return utils.NewToolResultError(namesErr.Error()), nil, nil + } + var gqlClient *githubv4.Client + if len(visibleFieldNames) > 0 { + gqlClient, err = deps.GetGQLClient(ctx) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + } + return createProjectView(ctx, client, gqlClient, args, owner, ownerType, projectNumber, visibleFieldNames) + } + + gqlClient, err := deps.GetGQLClient(ctx) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + switch method { case projectsMethodAddProjectItem: itemType, err := RequiredParam[string](args, "item_type") @@ -896,8 +922,6 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { return createProjectStatusUpdate(ctx, gqlClient, owner, ownerType, projectNumber, body, status, startDate, targetDate) case projectsMethodCreateIterationField: return createIterationField(ctx, gqlClient, owner, ownerType, projectNumber, args) - case projectsMethodCreateProjectView: - return createProjectView(ctx, client, args, owner, ownerType, projectNumber) case projectsMethodUpdateProjectView: return updateProjectView(ctx, gqlClient, args, owner, ownerType, projectNumber) case projectsMethodDeleteProjectView: @@ -1807,7 +1831,7 @@ func getProjectView(ctx context.Context, gqlClient *githubv4.Client, viewID stri return utils.NewToolResultText(string(result)), !bool(query.Node.ProjectView.Project.Public), nil, nil } -func createProjectView(ctx context.Context, client *github.Client, args map[string]any, owner, ownerType string, projectNumber int) (*mcp.CallToolResult, any, error) { +func createProjectView(ctx context.Context, client *github.Client, gqlClient *githubv4.Client, args map[string]any, owner, ownerType string, projectNumber int, visibleFieldNames []string) (*mcp.CallToolResult, any, error) { name, err := RequiredParam[string](args, "name") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil @@ -1831,8 +1855,22 @@ func createProjectView(ctx context.Context, client *github.Client, args map[stri if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } + if len(visibleFields) > 0 && len(visibleFieldNames) > 0 { + return utils.NewToolResultError("provide either 'visible_fields' or 'visible_field_names', not both"), nil, nil + } + if len(visibleFieldNames) > 0 { + resolvedIDs, resolveErr := resolveFieldNamesToIDs(ctx, gqlClient, owner, ownerType, projectNumber, visibleFieldNames) + if resolveErr != nil { + var structured *ghErrors.StructuredResolutionError + if errors.As(resolveErr, &structured) { + return ghErrors.NewStructuredResolutionErrorResponse(structured), nil, nil + } + return utils.NewToolResultError(resolveErr.Error()), nil, nil + } + visibleFields = resolvedIDs + } if layout == githubv4.ProjectV2ViewLayoutRoadmapLayout && len(visibleFields) > 0 { - return utils.NewToolResultError("visible_fields is not supported for roadmap views"), nil, nil + return utils.NewToolResultError("visible fields are not supported for roadmap views"), nil, nil } requestBody := CreateProjectV2ViewRequest{ diff --git a/pkg/github/projects_test.go b/pkg/github/projects_test.go index b1b115259d..6e78f6ea1c 100644 --- a/pkg/github/projects_test.go +++ b/pkg/github/projects_test.go @@ -887,6 +887,7 @@ func Test_ProjectsWrite(t *testing.T) { assert.Contains(t, inputSchema.Properties, "layout") assert.Contains(t, inputSchema.Properties, "filter") assert.Contains(t, inputSchema.Properties, "visible_fields") + assert.Contains(t, inputSchema.Properties, "visible_field_names") assert.Contains(t, inputSchema.Properties["method"].Enum, projectsMethodCreateProjectView) assert.Contains(t, inputSchema.Properties["method"].Enum, projectsMethodUpdateProjectView) assert.Contains(t, inputSchema.Properties["method"].Enum, projectsMethodDeleteProjectView) diff --git a/pkg/github/projects_v2_test.go b/pkg/github/projects_v2_test.go index 0be6d2a9bb..21cf074773 100644 --- a/pkg/github/projects_v2_test.go +++ b/pkg/github/projects_v2_test.go @@ -209,6 +209,39 @@ func projectViewParentErrorMatcher(viewID, message string) githubv4mock.Matcher ) } +func projectFieldNamesMatcher(owner, ownerType string, projectNumber int, nodes []map[string]any) githubv4mock.Matcher { + var response map[string]any + if ownerType == "org" { + response = fieldsResponse(nodes) + return githubv4mock.NewQueryMatcher( + projectFieldsQueryOrg{}, + fieldsQueryVars(owner, projectNumber), + githubv4mock.DataResponse(response), + ) + } + + response = map[string]any{ + "user": map[string]any{ + "projectV2": map[string]any{ + "fields": map[string]any{ + "nodes": nodes, + "pageInfo": map[string]any{ + "hasNextPage": false, + "hasPreviousPage": false, + "startCursor": "", + "endCursor": "", + }, + }, + }, + }, + } + return githubv4mock.NewQueryMatcher( + projectFieldsQueryUser{}, + fieldsQueryVars(owner, projectNumber), + githubv4mock.DataResponse(response), + ) +} + func createFieldMatcher() githubv4mock.Matcher { return githubv4mock.NewMutationMatcher( struct { @@ -756,10 +789,7 @@ func Test_ProjectsWrite_CreateProjectView(t *testing.T) { })(w, r) }, }) - deps := BaseDeps{ - Client: mustNewGHClient(t, restClient), - GQLClient: githubv4.NewClient(githubv4mock.NewMockedHTTPClient()), - } + deps := BaseDeps{Client: mustNewGHClient(t, restClient)} handler := toolDef.Handler(deps) request := createMCPRequest(map[string]any{ "method": "create_project_view", @@ -782,6 +812,88 @@ func Test_ProjectsWrite_CreateProjectView(t *testing.T) { assert.Equal(t, []int64{101, 202}, view.VisibleFields) }) + t.Run("resolves organization visible field names in caller order", func(t *testing.T) { + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient( + projectFieldNamesMatcher("octo-org", "org", 7, []map[string]any{ + statusFieldNode("PVTSSF_status", 101, "Status", nil), + statusFieldNode("PVTSSF_priority", 202, "Priority", nil), + }), + )) + restClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "POST /orgs/{org}/projectsV2/{project}/views": func(w http.ResponseWriter, r *http.Request) { + var body map[string]any + require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) + assert.Equal(t, []any{float64(202), float64(101)}, body["visible_fields"]) + mockResponse(t, http.StatusCreated, map[string]any{ + "node_id": "PVTV_named_org", + "number": 2, + "name": "Named fields", + "layout": "table", + "visible_fields": []int64{202, 101}, + })(w, r) + }, + }) + deps := BaseDeps{ + Client: mustNewGHClient(t, restClient), + GQLClient: gqlClient, + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "create_project_view", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(7), + "name": "Named fields", + "layout": "table", + "visible_field_names": []any{"Priority", "status"}, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + }) + + t.Run("resolves user visible field names", func(t *testing.T) { + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient( + projectFieldNamesMatcher("octocat", "user", 8, []map[string]any{ + statusFieldNode("PVTSSF_status", 303, "Status", nil), + }), + )) + restClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "POST /users/{user_id}/projectsV2/{project}/views": func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, "/users/octocat/projectsV2/8/views", r.URL.Path) + var body map[string]any + require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) + assert.Equal(t, []any{float64(303)}, body["visible_fields"]) + mockResponse(t, http.StatusCreated, map[string]any{ + "node_id": "PVTV_named_user", + "number": 3, + "name": "User fields", + "layout": "board", + "visible_fields": []int64{303}, + })(w, r) + }, + }) + deps := BaseDeps{ + Client: mustNewGHClient(t, restClient), + GQLClient: gqlClient, + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "create_project_view", + "owner": "octocat", + "owner_type": "user", + "project_number": float64(8), + "name": "User fields", + "layout": "board", + "visible_field_names": []any{"Status"}, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + }) + t.Run("creates a user view by login", func(t *testing.T) { restClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ "POST /users/{user_id}/projectsV2/{project}/views": func(w http.ResponseWriter, r *http.Request) { @@ -794,10 +906,7 @@ func Test_ProjectsWrite_CreateProjectView(t *testing.T) { })(w, r) }, }) - deps := BaseDeps{ - Client: mustNewGHClient(t, restClient), - GQLClient: githubv4.NewClient(githubv4mock.NewMockedHTTPClient()), - } + deps := BaseDeps{Client: mustNewGHClient(t, restClient)} handler := toolDef.Handler(deps) request := createMCPRequest(map[string]any{ "method": "create_project_view", @@ -827,10 +936,7 @@ func Test_ProjectsWrite_CreateProjectView(t *testing.T) { "layout": "table", }), }) - deps := BaseDeps{ - Client: mustNewGHClient(t, restClient), - GQLClient: githubv4.NewClient(githubv4mock.NewMockedHTTPClient()), - } + deps := BaseDeps{Client: mustNewGHClient(t, restClient)} handler := toolDef.Handler(deps) request := createMCPRequest(map[string]any{ "method": "create_project_view", @@ -845,12 +951,85 @@ func Test_ProjectsWrite_CreateProjectView(t *testing.T) { require.False(t, result.IsError) }) - t.Run("rejects visible fields for roadmap layout", func(t *testing.T) { + t.Run("rejects visible fields and names together", func(t *testing.T) { deps := BaseDeps{ Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), GQLClient: githubv4.NewClient(githubv4mock.NewMockedHTTPClient()), } handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "create_project_view", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(7), + "name": "Table", + "layout": "table", + "visible_fields": []any{"101"}, + "visible_field_names": []any{"Status"}, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "provide either 'visible_fields' or 'visible_field_names', not both") + }) + + for _, tc := range []struct { + name string + nodes []map[string]any + requestedName string + expectedError string + }{ + { + name: "returns structured not-found errors", + nodes: []map[string]any{ + statusFieldNode("PVTSSF_status", 101, "Status", nil), + }, + requestedName: "Priority", + expectedError: "field_not_found", + }, + { + name: "returns structured ambiguous errors", + nodes: []map[string]any{ + statusFieldNode("PVTSSF_status1", 101, "Status", nil), + statusFieldNode("PVTSSF_status2", 202, "Status", nil), + }, + requestedName: "Status", + expectedError: "field_ambiguous", + }, + } { + t.Run(tc.name, func(t *testing.T) { + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient( + projectFieldNamesMatcher("octo-org", "org", 7, tc.nodes), + )) + deps := BaseDeps{ + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), + GQLClient: gqlClient, + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "create_project_view", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(7), + "name": "Table", + "layout": "table", + "visible_field_names": []any{tc.requestedName}, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + var response map[string]any + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &response)) + assert.Equal(t, tc.expectedError, response["error"]) + assert.Equal(t, tc.requestedName, response["name"]) + }) + } + + t.Run("rejects visible fields for roadmap layout", func(t *testing.T) { + deps := BaseDeps{Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}))} + handler := toolDef.Handler(deps) request := createMCPRequest(map[string]any{ "method": "create_project_view", "owner": "octo-org", @@ -864,7 +1043,34 @@ func Test_ProjectsWrite_CreateProjectView(t *testing.T) { result, err := handler(ContextWithDeps(context.Background(), deps), &request) require.NoError(t, err) require.True(t, result.IsError) - assert.Contains(t, getTextResult(t, result).Text, "visible_fields is not supported for roadmap views") + assert.Contains(t, getTextResult(t, result).Text, "visible fields are not supported for roadmap views") + }) + + t.Run("resolves visible field names before rejecting roadmap layout", func(t *testing.T) { + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient( + projectFieldNamesMatcher("octo-org", "org", 7, []map[string]any{ + statusFieldNode("PVTSSF_status", 101, "Status", nil), + }), + )) + deps := BaseDeps{ + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), + GQLClient: gqlClient, + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "create_project_view", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(7), + "name": "Roadmap", + "layout": "roadmap", + "visible_field_names": []any{"Status"}, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "visible fields are not supported for roadmap views") }) }