diff --git a/README.md b/README.md index f3114fb900..32f8eb82bc 100644 --- a/README.md +++ b/README.md @@ -247,7 +247,7 @@ To keep your GitHub PAT secure and reusable across different MCP hosts: The flag `--gh-host` and the environment variable `GITHUB_HOST` can be used to set the hostname for GitHub Enterprise Server or GitHub Enterprise Cloud with data residency. -- For GitHub Enterprise Server, prefix the hostname with the `https://` URI scheme, as it otherwise defaults to `http://`, which GitHub Enterprise Server does not support. +- For GitHub Enterprise Server, prefix the hostname with the `https://` URI scheme. HTTPS is required and enforced: non-HTTPS hosts are refused so that credentials are never sent over cleartext (the only exception is a loopback host such as `http://localhost` for local development). - For GitHub Enterprise Cloud with data residency, use `https://YOURSUBDOMAIN.ghe.com` as the hostname. ``` json @@ -1099,6 +1099,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` @@ -1111,13 +1112,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 null to clear it. (string | null, 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) @@ -1126,7 +1128,9 @@ The following sets of tools are available: - `items`: The items to update with the top-level 'updated_field'. Required for 'update_project_items'; prefer it over calling 'update_project_item' in a loop. Each entry must match exactly one reference variant: 'node_id', numeric 'item_id', or 'item_owner' + 'item_repo' + 'issue_number'. Limit: 50 items per call. (object[], optional) - `iteration_duration`: Duration in days for iterations of the field (e.g. 7 for weekly, 14 for bi-weekly). Required for 'create_iteration_field' method. (number, optional) - `iterations`: Custom iterations for 'create_iteration_field' method. Only set this when you need iterations with varying durations, breaks between them, or specific titles. Otherwise omit it: GitHub auto-creates three iterations of 'iteration_duration' days starting on 'start_date', which is the right choice for most cases. (object[], optional) + - `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) @@ -1136,6 +1140,9 @@ 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`: The field/value to apply, using {"id": 123, "value": ...} or {"name": "Status", "value": ...}; null clears the field. Required for 'update_project_item' and 'update_project_items', where one top-level field/value applies to every item in a batch. For 'update_project_item' SINGLE_SELECT fields, the name form accepts option names; the ID form expects an option ID. (object, optional) + - `view_id`: Project view node ID for update or delete; must belong to owner/project_number. (string, optional) + - `visible_field_names`: Ordered project field names to show on create or replace on update; omit on update to preserve, or pass [] to reset. Mutually exclusive with visible_fields. Roadmap accepts only []. (string[], optional) + - `visible_fields`: Ordered project field database IDs to show on create or replace on update; omit on update to preserve, or pass [] to reset. Mutually exclusive with visible_field_names. Roadmap accepts only []. (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 d7c5d25eab..0ea38c2b0a 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, bulk-update many items at once, create status updates, and add iteration fields.", + "description": "Create and manage GitHub Projects: create projects, add/update/delete items, bulk-update many items at once, manage views, create status updates, and add iteration fields.", "inputSchema": { "properties": { "body": { @@ -16,6 +16,17 @@ "description": "The name of the iteration field (e.g. 'Sprint'). Required for 'create_iteration_field' method.", "type": "string" }, + "filter": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "description": "Saved view filter; omit on update to preserve it, or pass null to clear it." + }, "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" @@ -129,6 +140,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": [ @@ -137,11 +157,18 @@ "update_project_items", "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" @@ -224,6 +251,24 @@ } ], "type": "object" + }, + "view_id": { + "description": "Project view node ID for update or delete; must belong to owner/project_number.", + "type": "string" + }, + "visible_field_names": { + "description": "Ordered project field names to show on create or replace on update; omit on update to preserve, or pass [] to reset. Mutually exclusive with visible_fields. Roadmap accepts only [].", + "items": { + "type": "string" + }, + "type": "array" + }, + "visible_fields": { + "description": "Ordered project field database IDs to show on create or replace on update; omit on update to preserve, or pass [] to reset. Mutually exclusive with visible_field_names. Roadmap accepts only [].", + "items": { + "type": "string" + }, + "type": "array" } }, "required": [ diff --git a/pkg/github/actions.go b/pkg/github/actions.go index c16efa0f18..0a1db9d387 100644 --- a/pkg/github/actions.go +++ b/pkg/github/actions.go @@ -146,11 +146,11 @@ func getJobLogData(ctx context.Context, client *github.Client, owner, repo strin // Download and return the actual log content content, originalLength, httpResp, err := downloadLogContent(ctx, url.String(), tailLines, contentWindowSize) //nolint:bodyclose // Response body is closed in downloadLogContent, but we need to return httpResp if err != nil { - // To keep the return value consistent wrap the response as a GitHub Response - ghRes := &github.Response{ - Response: httpResp, + var ghResp *github.Response + if httpResp != nil { + ghResp = &github.Response{Response: httpResp} } - return nil, ghRes, fmt.Errorf("failed to download log content for job %d: %w", jobID, err) + return nil, ghResp, fmt.Errorf("failed to download log content for job %d: %w", jobID, err) } result["logs_content"] = content result["message"] = "Job logs content retrieved successfully" @@ -802,7 +802,7 @@ func getWorkflowRun(ctx context.Context, client *github.Client, owner, repo stri return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get workflow run", resp, err), nil, nil } defer func() { _ = resp.Body.Close() }() - r, err := json.Marshal(workflowRun) + r, err := json.Marshal(convertToMinimalWorkflowRun(workflowRun)) if err != nil { return nil, nil, fmt.Errorf("failed to marshal workflow run: %w", err) } @@ -884,7 +884,7 @@ func listWorkflowRuns(ctx context.Context, client *github.Client, args map[strin } defer func() { _ = resp.Body.Close() }() - r, err := json.Marshal(workflowRuns) + r, err := json.Marshal(convertToMinimalWorkflowRuns(workflowRuns)) if err != nil { return nil, nil, fmt.Errorf("failed to marshal workflow runs: %w", err) } @@ -919,7 +919,7 @@ func listWorkflowJobs(ctx context.Context, client *github.Client, args map[strin } response := map[string]any{ - "jobs": workflowJobs, + "jobs": convertToMinimalWorkflowJobs(workflowJobs), } defer func() { _ = resp.Body.Close() }() diff --git a/pkg/github/actions_minimal_test.go b/pkg/github/actions_minimal_test.go new file mode 100644 index 0000000000..4f0f8bd977 --- /dev/null +++ b/pkg/github/actions_minimal_test.go @@ -0,0 +1,267 @@ +package github + +import ( + "encoding/json" + "testing" + "time" + + "github.com/google/go-github/v89/github" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestConvertToMinimalWorkflowRun(t *testing.T) { + workflowRun := actionsTestWorkflowRun() + + minimal := convertToMinimalWorkflowRun(workflowRun) + + assert.Equal(t, workflowRun.GetID(), minimal.ID) + assert.Equal(t, workflowRun.GetWorkflowID(), minimal.WorkflowID) + assert.Equal(t, workflowRun.GetDisplayTitle(), minimal.DisplayTitle) + assert.Equal(t, workflowRun.GetHeadSHA(), minimal.HeadSHA) + assert.Equal(t, []int{42}, minimal.PullRequests) + require.NotNil(t, minimal.HeadCommit) + assert.Equal(t, "Reduce GitHub Actions response payloads", minimal.HeadCommit.Message) + require.Len(t, minimal.ReferencedWorkflows, 1) + assert.Equal(t, ".github/workflows/reusable-tests.yml", minimal.ReferencedWorkflows[0].Path) + assert.Equal(t, "refs/tags/v3", minimal.ReferencedWorkflows[0].Ref) + assert.Equal(t, "9f4f87d9790ab0f5c2c5ad2b74b886cab515a886", minimal.ReferencedWorkflows[0].SHA) + require.NotNil(t, minimal.Actor) + assert.Equal(t, "octocat", minimal.Actor.Login) + require.NotNil(t, minimal.TriggeringActor) + assert.Equal(t, "hubot", minimal.TriggeringActor.Login) + + payload := marshalActionsObject(t, minimal) + assert.NotContains(t, payload, "node_id") + assert.NotContains(t, payload, "repository") + assert.NotContains(t, payload, "head_repository") + assert.NotContains(t, payload, "jobs_url") + assert.NotContains(t, payload, "logs_url") + assert.NotContains(t, payload, "artifacts_url") + assert.Equal(t, map[string]any{ + "message": "Reduce GitHub Actions response payloads", + }, payload["head_commit"]) + assert.Equal(t, []any{ + map[string]any{ + "path": ".github/workflows/reusable-tests.yml", + "sha": "9f4f87d9790ab0f5c2c5ad2b74b886cab515a886", + "ref": "refs/tags/v3", + }, + }, payload["referenced_workflows"]) +} + +func TestConvertToMinimalWorkflowJob(t *testing.T) { + workflowJob := actionsTestWorkflowJob() + + minimal := convertToMinimalWorkflowJob(workflowJob) + + assert.Equal(t, workflowJob.GetID(), minimal.ID) + assert.Equal(t, workflowJob.GetRunID(), minimal.RunID) + assert.Equal(t, workflowJob.GetRunnerID(), minimal.RunnerID) + assert.Equal(t, workflowJob.GetRunnerName(), minimal.RunnerName) + assert.Equal(t, workflowJob.GetRunnerGroupID(), minimal.RunnerGroupID) + assert.Equal(t, workflowJob.GetRunnerGroupName(), minimal.RunnerGroupName) + assert.Equal(t, workflowJob.GetLabels(), minimal.Labels) + require.Len(t, minimal.Steps, 2) + assert.Equal(t, "Run tests", minimal.Steps[1].Name) + assert.Equal(t, "failure", minimal.Steps[1].Conclusion) + + payload := marshalActionsObject(t, minimal) + assert.NotContains(t, payload, "node_id") + assert.NotContains(t, payload, "url") + assert.NotContains(t, payload, "run_url") + assert.NotContains(t, payload, "check_run_url") + assert.Equal(t, float64(1), payload["runner_id"]) + assert.Equal(t, float64(2), payload["runner_group_id"]) + assert.Equal(t, "GitHub Actions", payload["runner_group_name"]) +} + +func TestConvertToMinimalActionsLists(t *testing.T) { + t.Run("workflow runs", func(t *testing.T) { + result := convertToMinimalWorkflowRuns(&github.WorkflowRuns{ + TotalCount: github.Ptr(2), + WorkflowRuns: []*github.WorkflowRun{actionsTestWorkflowRun(), nil}, + }) + assert.Equal(t, 2, result.TotalCount) + assert.Len(t, result.WorkflowRuns, 1) + }) + + t.Run("workflow jobs", func(t *testing.T) { + result := convertToMinimalWorkflowJobs(&github.Jobs{ + TotalCount: github.Ptr(2), + Jobs: []*github.WorkflowJob{actionsTestWorkflowJob(), nil}, + }) + assert.Equal(t, 2, result.TotalCount) + assert.Len(t, result.Jobs, 1) + }) + + t.Run("nil workflow runs", func(t *testing.T) { + result := convertToMinimalWorkflowRuns(nil) + assert.NotNil(t, result.WorkflowRuns) + assert.Empty(t, result.WorkflowRuns) + }) + + t.Run("nil workflow jobs", func(t *testing.T) { + result := convertToMinimalWorkflowJobs(nil) + assert.NotNil(t, result.Jobs) + assert.Empty(t, result.Jobs) + }) +} + +func actionsTestWorkflowRun() *github.WorkflowRun { + repository := &github.Repository{ + ID: github.Ptr(int64(1296269)), + NodeID: github.Ptr("MDEwOlJlcG9zaXRvcnkxMjk2MjY5"), + Name: github.Ptr("octo-repo"), + FullName: github.Ptr("octo-org/octo-repo"), + Description: github.Ptr("A representative repository description included in the full API response."), + HTMLURL: github.Ptr("https://github.com/octo-org/octo-repo"), + URL: github.Ptr("https://api.github.com/repos/octo-org/octo-repo"), + CloneURL: github.Ptr("https://github.com/octo-org/octo-repo.git"), + Language: github.Ptr("Go"), + Topics: []string{"actions", "mcp", "automation"}, + } + + return &github.WorkflowRun{ + ID: github.Ptr(int64(30433642)), + Name: github.Ptr("CI"), + NodeID: github.Ptr("MDEyOldvcmtmbG93IFJ1bjI2OTI4OQ=="), + HeadBranch: github.Ptr("feature/minimal-actions"), + HeadSHA: github.Ptr("acb5820ced9479c074f688cc328bf03f341a511d"), + Path: github.Ptr(".github/workflows/ci.yml"), + RunNumber: github.Ptr(562), + RunAttempt: github.Ptr(2), + Event: github.Ptr("pull_request"), + DisplayTitle: github.Ptr("Reduce GitHub Actions response payloads"), + Status: github.Ptr("completed"), + Conclusion: github.Ptr("failure"), + WorkflowID: github.Ptr(int64(161335)), + CheckSuiteID: github.Ptr(int64(42)), + CheckSuiteNodeID: github.Ptr("MDEwOkNoZWNrU3VpdGU0Mg=="), + URL: github.Ptr("https://api.github.com/repos/octo-org/octo-repo/actions/runs/30433642"), + HTMLURL: github.Ptr("https://github.com/octo-org/octo-repo/actions/runs/30433642"), + JobsURL: github.Ptr("https://api.github.com/repos/octo-org/octo-repo/actions/runs/30433642/jobs"), + LogsURL: github.Ptr("https://api.github.com/repos/octo-org/octo-repo/actions/runs/30433642/logs"), + CheckSuiteURL: github.Ptr("https://api.github.com/repos/octo-org/octo-repo/check-suites/42"), + ArtifactsURL: github.Ptr("https://api.github.com/repos/octo-org/octo-repo/actions/runs/30433642/artifacts"), + CancelURL: github.Ptr("https://api.github.com/repos/octo-org/octo-repo/actions/runs/30433642/cancel"), + RerunURL: github.Ptr("https://api.github.com/repos/octo-org/octo-repo/actions/runs/30433642/rerun"), + PreviousAttemptURL: github.Ptr("https://api.github.com/repos/octo-org/octo-repo/actions/runs/30433642/attempts/1"), + WorkflowURL: github.Ptr("https://api.github.com/repos/octo-org/octo-repo/actions/workflows/161335"), + Repository: repository, + HeadRepository: repository, + Actor: &github.User{ + Login: github.Ptr("octocat"), + ID: github.Ptr(int64(1)), + NodeID: github.Ptr("MDQ6VXNlcjE="), + AvatarURL: github.Ptr("https://github.com/images/error/octocat_happy.gif"), + HTMLURL: github.Ptr("https://github.com/octocat"), + URL: github.Ptr("https://api.github.com/users/octocat"), + Name: github.Ptr("The Octocat"), + Bio: github.Ptr("A long biography that is not needed to identify the workflow run actor."), + }, + TriggeringActor: &github.User{ + Login: github.Ptr("hubot"), + ID: github.Ptr(int64(2)), + HTMLURL: github.Ptr("https://github.com/hubot"), + URL: github.Ptr("https://api.github.com/users/hubot"), + }, + PullRequests: []*github.PullRequest{ + { + ID: github.Ptr(int64(1001)), + Number: github.Ptr(42), + Title: github.Ptr("Reduce GitHub Actions response payloads"), + Body: github.Ptr("A pull request body that is unnecessary in a workflow run response."), + HTMLURL: github.Ptr("https://github.com/octo-org/octo-repo/pull/42"), + Head: &github.PullRequestBranch{ + Ref: github.Ptr("feature/minimal-actions"), + SHA: github.Ptr("acb5820ced9479c074f688cc328bf03f341a511d"), + Repo: repository, + }, + Base: &github.PullRequestBranch{ + Ref: github.Ptr("main"), + SHA: github.Ptr("9a2f3ec"), + Repo: repository, + }, + }, + }, + HeadCommit: &github.HeadCommit{ + Message: github.Ptr("Reduce GitHub Actions response payloads"), + URL: github.Ptr("https://api.github.com/repos/octo-org/octo-repo/commits/acb5820"), + Author: &github.CommitAuthor{ + Name: github.Ptr("The Octocat"), + Email: github.Ptr("octocat@example.com"), + }, + }, + ReferencedWorkflows: []*github.ReferencedWorkflow{ + { + Path: github.Ptr(".github/workflows/reusable-tests.yml"), + SHA: github.Ptr("9f4f87d9790ab0f5c2c5ad2b74b886cab515a886"), + Ref: github.Ptr("refs/tags/v3"), + }, + nil, + }, + CreatedAt: actionsTestTimestamp(), + UpdatedAt: actionsTestTimestamp(), + RunStartedAt: actionsTestTimestamp(), + } +} + +func actionsTestWorkflowJob() *github.WorkflowJob { + return &github.WorkflowJob{ + ID: github.Ptr(int64(399444496)), + RunID: github.Ptr(int64(30433642)), + RunURL: github.Ptr("https://api.github.com/repos/octo-org/octo-repo/actions/runs/30433642"), + NodeID: github.Ptr("MDEyOldvcmtmbG93IEpvYjM5OTQ0NDQ5Ng=="), + HeadBranch: github.Ptr("feature/minimal-actions"), + HeadSHA: github.Ptr("acb5820ced9479c074f688cc328bf03f341a511d"), + URL: github.Ptr("https://api.github.com/repos/octo-org/octo-repo/actions/jobs/399444496"), + HTMLURL: github.Ptr("https://github.com/octo-org/octo-repo/runs/399444496"), + Status: github.Ptr("completed"), + Conclusion: github.Ptr("failure"), + CreatedAt: actionsTestTimestamp(), + StartedAt: actionsTestTimestamp(), + CompletedAt: actionsTestTimestamp(), + Name: github.Ptr("test (ubuntu-latest, Go 1.24)"), + CheckRunURL: github.Ptr("https://api.github.com/repos/octo-org/octo-repo/check-runs/399444496"), + Labels: []string{"ubuntu-latest", "x64"}, + RunnerID: github.Ptr(int64(1)), + RunnerName: github.Ptr("GitHub Actions 1"), + RunnerGroupID: github.Ptr(int64(2)), + RunnerGroupName: github.Ptr("GitHub Actions"), + RunAttempt: github.Ptr(int64(2)), + WorkflowName: github.Ptr("CI"), + Steps: []*github.TaskStep{ + { + Name: github.Ptr("Set up job"), + Status: github.Ptr("completed"), + Conclusion: github.Ptr("success"), + Number: github.Ptr(int64(1)), + StartedAt: actionsTestTimestamp(), + CompletedAt: actionsTestTimestamp(), + }, + { + Name: github.Ptr("Run tests"), + Status: github.Ptr("completed"), + Conclusion: github.Ptr("failure"), + Number: github.Ptr(int64(2)), + StartedAt: actionsTestTimestamp(), + CompletedAt: actionsTestTimestamp(), + }, + }, + } +} + +func actionsTestTimestamp() *github.Timestamp { + return &github.Timestamp{Time: time.Date(2026, time.August, 6, 10, 30, 0, 0, time.UTC)} +} + +func marshalActionsObject(t *testing.T, value any) map[string]any { + t.Helper() + data, err := json.Marshal(value) + require.NoError(t, err) + + var object map[string]any + require.NoError(t, json.Unmarshal(data, &object)) + return object +} diff --git a/pkg/github/actions_test.go b/pkg/github/actions_test.go index 4ed9c87d69..964bc95a6b 100644 --- a/pkg/github/actions_test.go +++ b/pkg/github/actions_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "net/http" + "net/http/httptest" "testing" "github.com/github/github-mcp-server/internal/toolsnaps" @@ -154,10 +155,12 @@ func Test_ActionsList_ListWorkflowRuns(t *testing.T) { require.False(t, result.IsError) textContent := getTextResult(t, result) - var response github.WorkflowRuns + var response MinimalWorkflowRunsResult err = json.Unmarshal([]byte(textContent.Text), &response) require.NoError(t, err) - assert.NotNil(t, response.TotalCount) + assert.Equal(t, 1, response.TotalCount) + require.Len(t, response.WorkflowRuns, 1) + assert.Equal(t, int64(123), response.WorkflowRuns[0].ID) }) t.Run("list all workflow runs without resource_id", func(t *testing.T) { @@ -202,13 +205,47 @@ func Test_ActionsList_ListWorkflowRuns(t *testing.T) { require.False(t, result.IsError) textContent := getTextResult(t, result) - var response github.WorkflowRuns + var response MinimalWorkflowRunsResult err = json.Unmarshal([]byte(textContent.Text), &response) require.NoError(t, err) - assert.Equal(t, 2, *response.TotalCount) + assert.Equal(t, 2, response.TotalCount) + assert.Len(t, response.WorkflowRuns, 2) }) } +func Test_ActionsList_ListWorkflowJobs(t *testing.T) { + toolDef := ActionsList(translations.NullTranslationHelper) + mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposActionsRunsJobsByOwnerByRepoByRunID: mockResponse(t, http.StatusOK, &github.Jobs{ + TotalCount: github.Ptr(1), + Jobs: []*github.WorkflowJob{actionsTestWorkflowJob()}, + }), + }) + + client := mustNewGHClient(t, mockedClient) + deps := BaseDeps{Client: client} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "list_workflow_jobs", + "owner": "owner", + "repo": "repo", + "resource_id": "30433642", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + + var response struct { + Jobs MinimalWorkflowJobsResult `json:"jobs"` + } + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &response)) + assert.Equal(t, 1, response.Jobs.TotalCount) + require.Len(t, response.Jobs.Jobs, 1) + assert.Equal(t, int64(399444496), response.Jobs.Jobs[0].ID) + assert.Len(t, response.Jobs.Jobs[0].Steps, 2) +} + func Test_ActionsGet(t *testing.T) { // Verify tool definition once toolDef := ActionsGet(translations.NullTranslationHelper) @@ -271,14 +308,9 @@ func Test_ActionsGet_GetWorkflowRun(t *testing.T) { toolDef := ActionsGet(translations.NullTranslationHelper) t.Run("successful workflow run get", func(t *testing.T) { + run := actionsTestWorkflowRun() mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ GetReposActionsRunsByOwnerByRepoByRunID: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - run := &github.WorkflowRun{ - ID: github.Ptr(int64(12345)), - Name: github.Ptr("CI"), - Status: github.Ptr("completed"), - Conclusion: github.Ptr("success"), - } w.WriteHeader(http.StatusOK) _ = json.NewEncoder(w).Encode(run) }), @@ -302,11 +334,21 @@ func Test_ActionsGet_GetWorkflowRun(t *testing.T) { require.False(t, result.IsError) textContent := getTextResult(t, result) - var response github.WorkflowRun + var response MinimalWorkflowRun err = json.Unmarshal([]byte(textContent.Text), &response) require.NoError(t, err) - assert.NotNil(t, response.ID) - assert.Equal(t, int64(12345), *response.ID) + + expected := convertToMinimalWorkflowRun(run) + assert.Equal(t, expected, response) + + var payload map[string]any + require.NoError(t, json.Unmarshal([]byte(textContent.Text), &payload)) + assert.Equal(t, marshalActionsObject(t, expected), payload) + assert.NotContains(t, payload, "node_id") + assert.NotContains(t, payload, "repository") + assert.NotContains(t, payload, "head_repository") + assert.NotContains(t, payload, "url") + assert.NotContains(t, payload, "jobs_url") }) } @@ -583,6 +625,25 @@ func Test_ActionsGetJobLogs_SingleJob(t *testing.T) { }) } +func TestGetJobLogData_DownloadTransportErrorReturnsNilResponse(t *testing.T) { + logServer := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) + logURL := logServer.URL + logServer.Close() + + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposActionsJobsLogsByOwnerByRepoByJobID: func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Location", logURL) + w.WriteHeader(http.StatusFound) + }, + })) + + _, resp, err := getJobLogData(t.Context(), client, "owner", "repo", 123, "", true, 100, 5000) + + require.Error(t, err) + assert.Nil(t, resp) + assert.Contains(t, err.Error(), "failed to download log content for job 123") +} + func Test_ActionsGetJobLogs_FailedJobs(t *testing.T) { toolDef := ActionsGetJobLogs(translations.NullTranslationHelper) diff --git a/pkg/github/minimal_types.go b/pkg/github/minimal_types.go index e2bf8b684b..2424823c2c 100644 --- a/pkg/github/minimal_types.go +++ b/pkg/github/minimal_types.go @@ -318,6 +318,88 @@ type MinimalTag struct { SHA string `json:"sha"` } +// MinimalWorkflowRunHeadCommit is the trimmed commit context for a workflow run. +type MinimalWorkflowRunHeadCommit struct { + Message string `json:"message"` +} + +// MinimalReferencedWorkflow identifies a reusable workflow invoked by a workflow run. +type MinimalReferencedWorkflow struct { + Path string `json:"path,omitempty"` + SHA string `json:"sha,omitempty"` + Ref string `json:"ref,omitempty"` +} + +// MinimalWorkflowRun is the trimmed output type for GitHub Actions workflow runs. +type MinimalWorkflowRun struct { + ID int64 `json:"id"` + Name string `json:"name"` + DisplayTitle string `json:"display_title,omitempty"` + WorkflowID int64 `json:"workflow_id"` + RunNumber int `json:"run_number"` + RunAttempt int `json:"run_attempt"` + Event string `json:"event,omitempty"` + Status string `json:"status"` + Conclusion string `json:"conclusion,omitempty"` + HeadBranch string `json:"head_branch,omitempty"` + HeadSHA string `json:"head_sha,omitempty"` + HeadCommit *MinimalWorkflowRunHeadCommit `json:"head_commit,omitempty"` + Path string `json:"path,omitempty"` + HTMLURL string `json:"html_url,omitempty"` + PullRequests []int `json:"pull_requests,omitempty"` + Actor *MinimalUser `json:"actor,omitempty"` + TriggeringActor *MinimalUser `json:"triggering_actor,omitempty"` + ReferencedWorkflows []MinimalReferencedWorkflow `json:"referenced_workflows,omitempty"` + CreatedAt string `json:"created_at,omitempty"` + UpdatedAt string `json:"updated_at,omitempty"` + RunStartedAt string `json:"run_started_at,omitempty"` +} + +// MinimalWorkflowRunsResult is the trimmed output type for workflow run list results. +type MinimalWorkflowRunsResult struct { + TotalCount int `json:"total_count"` + WorkflowRuns []MinimalWorkflowRun `json:"workflow_runs"` +} + +// MinimalWorkflowJobStep is the trimmed output type for workflow job steps. +type MinimalWorkflowJobStep struct { + Name string `json:"name"` + Status string `json:"status"` + Conclusion string `json:"conclusion,omitempty"` + Number int64 `json:"number"` + StartedAt string `json:"started_at,omitempty"` + CompletedAt string `json:"completed_at,omitempty"` +} + +// MinimalWorkflowJob is the trimmed output type for GitHub Actions workflow jobs. +type MinimalWorkflowJob struct { + ID int64 `json:"id"` + RunID int64 `json:"run_id"` + Name string `json:"name"` + WorkflowName string `json:"workflow_name,omitempty"` + Status string `json:"status"` + Conclusion string `json:"conclusion,omitempty"` + HeadBranch string `json:"head_branch,omitempty"` + HeadSHA string `json:"head_sha,omitempty"` + HTMLURL string `json:"html_url,omitempty"` + RunAttempt int64 `json:"run_attempt,omitempty"` + RunnerID int64 `json:"runner_id,omitempty"` + RunnerName string `json:"runner_name,omitempty"` + RunnerGroupID int64 `json:"runner_group_id,omitempty"` + RunnerGroupName string `json:"runner_group_name,omitempty"` + Labels []string `json:"labels,omitempty"` + Steps []MinimalWorkflowJobStep `json:"steps,omitempty"` + CreatedAt string `json:"created_at,omitempty"` + StartedAt string `json:"started_at,omitempty"` + CompletedAt string `json:"completed_at,omitempty"` +} + +// MinimalWorkflowJobsResult is the trimmed output type for workflow job list results. +type MinimalWorkflowJobsResult struct { + TotalCount int `json:"total_count"` + Jobs []MinimalWorkflowJob `json:"jobs"` +} + // MinimalResponse represents a minimal response for all CRUD operations. // Success is implicit in the HTTP response status, and all other information // can be derived from the URL or fetched separately if needed. @@ -351,6 +433,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"` +} + type MinimalProjectItem struct { ID int64 `json:"id"` NodeID string `json:"node_id,omitempty"` @@ -617,6 +708,24 @@ type MinimalPRBranchRepo struct { Description string `json:"description,omitempty"` } +// MinimalRepoStatus is the trimmed output type for an individual commit status. +type MinimalRepoStatus struct { + State string `json:"state"` + Context string `json:"context"` + Description string `json:"description,omitempty"` + TargetURL string `json:"target_url,omitempty"` + CreatedAt string `json:"created_at,omitempty"` + UpdatedAt string `json:"updated_at,omitempty"` +} + +// MinimalCombinedStatus is the trimmed output type for a combined commit status. +type MinimalCombinedStatus struct { + State string `json:"state"` + SHA string `json:"sha"` + TotalCount int `json:"total_count"` + Statuses []MinimalRepoStatus `json:"statuses"` +} + type MinimalProjectStatusUpdate struct { ID string `json:"id"` Body string `json:"body,omitempty"` @@ -975,6 +1084,42 @@ func convertToMinimalPRBranch(branch *github.PullRequestBranch) *MinimalPRBranch return b } +func convertToMinimalCombinedStatus(status *github.CombinedStatus) MinimalCombinedStatus { + minimalStatus := MinimalCombinedStatus{ + Statuses: make([]MinimalRepoStatus, 0), + } + if status == nil { + return minimalStatus + } + + minimalStatus.State = status.GetState() + minimalStatus.SHA = status.GetSHA() + minimalStatus.TotalCount = status.GetTotalCount() + minimalStatus.Statuses = make([]MinimalRepoStatus, 0, len(status.GetStatuses())) + for _, repoStatus := range status.GetStatuses() { + if repoStatus != nil { + minimalStatus.Statuses = append(minimalStatus.Statuses, convertToMinimalRepoStatus(repoStatus)) + } + } + + return minimalStatus +} + +func convertToMinimalRepoStatus(status *github.RepoStatus) MinimalRepoStatus { + if status == nil { + return MinimalRepoStatus{} + } + + return MinimalRepoStatus{ + State: status.GetState(), + Context: status.GetContext(), + Description: status.GetDescription(), + TargetURL: status.GetTargetURL(), + CreatedAt: formatMinimalTimestamp(status.CreatedAt), + UpdatedAt: formatMinimalTimestamp(status.UpdatedAt), + } +} + func convertToMinimalProject(fullProject *github.ProjectV2) *MinimalProject { if fullProject == nil { return nil @@ -1832,6 +1977,144 @@ func convertToMinimalTag(tag *github.RepositoryTag) MinimalTag { return m } +func convertToMinimalWorkflowRun(workflowRun *github.WorkflowRun) MinimalWorkflowRun { + minimalRun := MinimalWorkflowRun{ + ID: workflowRun.GetID(), + Name: workflowRun.GetName(), + DisplayTitle: workflowRun.GetDisplayTitle(), + WorkflowID: workflowRun.GetWorkflowID(), + RunNumber: workflowRun.GetRunNumber(), + RunAttempt: workflowRun.GetRunAttempt(), + Event: workflowRun.GetEvent(), + Status: workflowRun.GetStatus(), + Conclusion: workflowRun.GetConclusion(), + HeadBranch: workflowRun.GetHeadBranch(), + HeadSHA: workflowRun.GetHeadSHA(), + Path: workflowRun.GetPath(), + HTMLURL: workflowRun.GetHTMLURL(), + Actor: convertToMinimalUser(workflowRun.GetActor()), + TriggeringActor: convertToMinimalUser(workflowRun.GetTriggeringActor()), + CreatedAt: formatMinimalTimestamp(workflowRun.CreatedAt), + UpdatedAt: formatMinimalTimestamp(workflowRun.UpdatedAt), + RunStartedAt: formatMinimalTimestamp(workflowRun.RunStartedAt), + } + + for _, pullRequest := range workflowRun.GetPullRequests() { + if pullRequest != nil && pullRequest.GetNumber() != 0 { + minimalRun.PullRequests = append(minimalRun.PullRequests, pullRequest.GetNumber()) + } + } + + if headCommit := workflowRun.GetHeadCommit(); headCommit != nil && headCommit.GetMessage() != "" { + minimalRun.HeadCommit = &MinimalWorkflowRunHeadCommit{ + Message: headCommit.GetMessage(), + } + } + + if len(workflowRun.GetReferencedWorkflows()) > 0 { + minimalRun.ReferencedWorkflows = make([]MinimalReferencedWorkflow, 0, len(workflowRun.ReferencedWorkflows)) + for _, workflow := range workflowRun.GetReferencedWorkflows() { + if workflow != nil { + minimalRun.ReferencedWorkflows = append(minimalRun.ReferencedWorkflows, MinimalReferencedWorkflow{ + Path: workflow.GetPath(), + SHA: workflow.GetSHA(), + Ref: workflow.GetRef(), + }) + } + } + } + + return minimalRun +} + +func convertToMinimalWorkflowRuns(workflowRuns *github.WorkflowRuns) MinimalWorkflowRunsResult { + result := MinimalWorkflowRunsResult{ + WorkflowRuns: make([]MinimalWorkflowRun, 0), + } + if workflowRuns == nil { + return result + } + + result.TotalCount = workflowRuns.GetTotalCount() + result.WorkflowRuns = make([]MinimalWorkflowRun, 0, len(workflowRuns.WorkflowRuns)) + for _, workflowRun := range workflowRuns.WorkflowRuns { + if workflowRun != nil { + result.WorkflowRuns = append(result.WorkflowRuns, convertToMinimalWorkflowRun(workflowRun)) + } + } + return result +} + +func convertToMinimalWorkflowJobStep(step *github.TaskStep) MinimalWorkflowJobStep { + return MinimalWorkflowJobStep{ + Name: step.GetName(), + Status: step.GetStatus(), + Conclusion: step.GetConclusion(), + Number: step.GetNumber(), + StartedAt: formatMinimalTimestamp(step.StartedAt), + CompletedAt: formatMinimalTimestamp(step.CompletedAt), + } +} + +func convertToMinimalWorkflowJob(job *github.WorkflowJob) MinimalWorkflowJob { + minimalJob := MinimalWorkflowJob{ + ID: job.GetID(), + RunID: job.GetRunID(), + Name: job.GetName(), + WorkflowName: job.GetWorkflowName(), + Status: job.GetStatus(), + Conclusion: job.GetConclusion(), + HeadBranch: job.GetHeadBranch(), + HeadSHA: job.GetHeadSHA(), + HTMLURL: job.GetHTMLURL(), + RunAttempt: job.GetRunAttempt(), + RunnerID: job.GetRunnerID(), + RunnerName: job.GetRunnerName(), + RunnerGroupID: job.GetRunnerGroupID(), + RunnerGroupName: job.GetRunnerGroupName(), + Labels: append([]string(nil), job.GetLabels()...), + CreatedAt: formatMinimalTimestamp(job.CreatedAt), + StartedAt: formatMinimalTimestamp(job.StartedAt), + CompletedAt: formatMinimalTimestamp(job.CompletedAt), + } + + if len(job.GetSteps()) > 0 { + minimalJob.Steps = make([]MinimalWorkflowJobStep, 0, len(job.Steps)) + for _, step := range job.GetSteps() { + if step != nil { + minimalJob.Steps = append(minimalJob.Steps, convertToMinimalWorkflowJobStep(step)) + } + } + } + + return minimalJob +} + +func convertToMinimalWorkflowJobs(workflowJobs *github.Jobs) MinimalWorkflowJobsResult { + result := MinimalWorkflowJobsResult{ + Jobs: make([]MinimalWorkflowJob, 0), + } + if workflowJobs == nil { + return result + } + + result.TotalCount = workflowJobs.GetTotalCount() + result.Jobs = make([]MinimalWorkflowJob, 0, len(workflowJobs.Jobs)) + for _, job := range workflowJobs.Jobs { + if job != nil { + result.Jobs = append(result.Jobs, convertToMinimalWorkflowJob(job)) + } + } + return result +} + +func formatMinimalTimestamp(timestamp *github.Timestamp) string { + if timestamp == nil || timestamp.IsZero() { + return "" + } + return timestamp.Format(time.RFC3339) +} + // MinimalCheckRun is the trimmed output type for check run objects. type MinimalCheckRun struct { ID int64 `json:"id"` diff --git a/pkg/github/projects.go b/pkg/github/projects.go index 4ceb432e69..dece52cd13 100644 --- a/pkg/github/projects.go +++ b/pkg/github/projects.go @@ -8,6 +8,7 @@ import ( "io" "net/http" "strconv" + "strings" "time" ghcontext "github.com/github/github-mcp-server/pkg/context" @@ -31,6 +32,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 maxProjectItemsPerBatch = 50 @@ -51,6 +57,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" ) @@ -109,6 +120,123 @@ 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 + Configuration projectViewConfiguration +} + +type projectViewConfiguration struct { + VisibleFields projectViewVisibleFieldsConnection `graphql:"visibleFields(first: 100)"` +} + +type projectViewVisibleFieldsConnection struct { + Nodes []projectViewVisibleFieldNode +} + +type projectViewVisibleFieldNode struct { + ProjectV2Field struct { + DatabaseID githubv4.Int `graphql:"databaseId"` + } `graphql:"... on ProjectV2Field"` + ProjectV2IterationField struct { + DatabaseID githubv4.Int `graphql:"databaseId"` + } `graphql:"... on ProjectV2IterationField"` + ProjectV2MultiSelectField struct { + DatabaseID githubv4.Int `graphql:"databaseId"` + } `graphql:"... on ProjectV2MultiSelectField"` + ProjectV2SingleSelectField struct { + DatabaseID githubv4.Int `graphql:"databaseId"` + } `graphql:"... on ProjectV2SingleSelectField"` +} + +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)"` +} + +type projectViewParentQuery struct { + Node struct { + ProjectView struct { + ID githubv4.ID + Layout githubv4.ProjectV2ViewLayout + Project struct { + ID githubv4.ID + } + } `graphql:"... on ProjectV2View"` + } `graphql:"node(id: $id)"` +} + +// ProjectV2ViewConfigurationInput is the GraphQL view configuration input. +type ProjectV2ViewConfigurationInput struct { + VisibleFieldIDs []githubv4.ID `json:"visibleFieldIds"` +} + +// CreateProjectV2ViewInput is the GraphQL input for creating a project view. +type CreateProjectV2ViewInput struct { + ProjectID githubv4.ID `json:"projectId"` + Name githubv4.String `json:"name"` + Layout githubv4.ProjectV2ViewLayout `json:"layout"` + Configuration *ProjectV2ViewConfigurationInput `json:"configuration,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"` + Configuration *ProjectV2ViewConfigurationInput `json:"configuration,omitempty"` +} + +type createProjectV2ViewMutation struct { + CreateProjectV2View struct { + ProjectV2View projectViewNode `graphql:"projectV2View"` + } `graphql:"createProjectV2View(input: $input)"` +} + +type updateProjectV2ViewMutation struct { + UpdateProjectV2View struct { + ProjectV2View projectViewNode `graphql:"projectV2View"` + } `graphql:"updateProjectV2View(input: $input)"` +} + +// 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 { @@ -161,7 +289,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"), @@ -178,6 +306,7 @@ Use this tool to list projects for a user or organization, or list project field projectsMethodListProjectFields, projectsMethodListProjectItems, projectsMethodListProjectStatusUpdates, + projectsMethodListProjectViews, }, }, "owner_type": { @@ -191,7 +320,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", @@ -254,7 +383,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 { @@ -298,6 +427,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 } @@ -316,7 +453,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"), @@ -333,6 +470,7 @@ Use this tool to get details about individual projects, project fields, and proj projectsMethodGetProjectField, projectsMethodGetProjectItem, projectsMethodGetProjectStatusUpdate, + projectsMethodGetProjectView, }, }, "owner_type": { @@ -374,6 +512,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"}, }, @@ -385,7 +527,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 { @@ -399,6 +541,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 { @@ -467,7 +622,7 @@ Use this tool to get details about individual projects, project fields, and proj if gqlErr != nil { return utils.NewToolResultError(gqlErr.Error()), nil, nil } - resolvedIDs, resolveErr := resolveFieldNamesToIDs(ctx, gqlClient, owner, ownerType, projectNumber, fieldNames) + resolvedIDs, resolveErr := resolveFieldNamesToIDs(ctx, gqlClient, owner, ownerType, projectNumber, fieldNames, "fields") if resolveErr != nil { var structured *ghErrors.StructuredResolutionError if errors.As(resolveErr, &structured) { @@ -576,7 +731,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, bulk-update many items at once, create status updates, and add iteration fields."), + Description: t("TOOL_PROJECTS_WRITE_DESCRIPTION", "Create and manage GitHub Projects: create projects, add/update/delete items, bulk-update many items at once, manage views, create status updates, and add iteration fields."), Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_PROJECTS_WRITE_USER_TITLE", "Manage GitHub Projects"), ReadOnlyHint: false, @@ -594,6 +749,9 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { projectsMethodUpdateProjectItems, projectsMethodDeleteProjectItem, projectsMethodCreateProjectStatusUpdate, + projectsMethodCreateProjectView, + projectsMethodUpdateProjectView, + projectsMethodDeleteProjectView, projectsMethodCreateProject, projectsMethodCreateIterationField, }, @@ -615,6 +773,40 @@ 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; must belong to owner/project_number.", + }, + "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": { + AnyOf: []*jsonschema.Schema{ + {Type: "string"}, + {Type: "null"}, + }, + Description: "Saved view filter; omit on update to preserve it, or pass null to clear it.", + }, + "visible_fields": { + Type: "array", + Description: "Ordered project field database IDs to show on create or replace on update; omit on update to preserve, or pass [] to reset. Mutually exclusive with visible_field_names. Roadmap accepts only [].", + Items: &jsonschema.Schema{ + Type: "string", + }, + }, + "visible_field_names": { + Type: "array", + Description: "Ordered project field names to show on create or replace on update; omit on update to preserve, or pass [] to reset. Mutually exclusive with visible_fields. Roadmap accepts only [].", + 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.", @@ -715,13 +907,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) } @@ -743,6 +934,11 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { } } + 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") @@ -833,6 +1029,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, gqlClient, args, owner, ownerType, projectNumber) + case projectsMethodUpdateProjectView: + return updateProjectView(ctx, gqlClient, args, owner, ownerType, projectNumber) + case projectsMethodDeleteProjectView: + return deleteProjectView(ctx, gqlClient, args, owner, ownerType, projectNumber) default: return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil } @@ -1047,7 +1249,7 @@ func listProjectItems(ctx context.Context, client *github.Client, gqlClient *git return utils.NewToolResultError("provide either 'fields' or 'field_names', not both"), nil, nil } if len(fieldNames) > 0 { - resolvedIDs, resolveErr := resolveFieldNamesToIDs(ctx, gqlClient, owner, ownerType, projectNumber, fieldNames) + resolvedIDs, resolveErr := resolveFieldNamesToIDs(ctx, gqlClient, owner, ownerType, projectNumber, fieldNames, "fields") if resolveErr != nil { var structured *ghErrors.StructuredResolutionError if errors.As(resolveErr, &structured) { @@ -1678,6 +1880,442 @@ func getProjectStatusUpdate(ctx context.Context, gqlClient *githubv4.Client, sta return utils.NewToolResultText(string(r)), isPrivate, nil, nil } +func convertToMinimalProjectView(node projectViewNode) MinimalProjectView { + visibleFields := make([]int64, 0, len(node.Configuration.VisibleFields.Nodes)) + for _, field := range node.Configuration.VisibleFields.Nodes { + switch { + case field.ProjectV2SingleSelectField.DatabaseID != 0: + visibleFields = append(visibleFields, int64(field.ProjectV2SingleSelectField.DatabaseID)) + case field.ProjectV2MultiSelectField.DatabaseID != 0: + visibleFields = append(visibleFields, int64(field.ProjectV2MultiSelectField.DatabaseID)) + case field.ProjectV2IterationField.DatabaseID != 0: + visibleFields = append(visibleFields, int64(field.ProjectV2IterationField.DatabaseID)) + default: + visibleFields = append(visibleFields, int64(field.ProjectV2Field.DatabaseID)) + } + } + return MinimalProjectView{ + ID: fmt.Sprintf("%v", node.ID), + Number: int(node.Number), + Name: string(node.Name), + Layout: projectViewLayoutName(node.Layout), + Filter: derefString(node.Filter), + VisibleFields: visibleFields, + } +} + +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 projectViewVisibleFieldsInput(ctx context.Context, gqlClient *githubv4.Client, args map[string]any, owner, ownerType string, projectNumber int) (*ProjectV2ViewConfigurationInput, error) { + _, hasVisibleFields := args["visible_fields"] + _, hasVisibleFieldNames := args["visible_field_names"] + if !hasVisibleFields && !hasVisibleFieldNames { + return nil, nil + } + + databaseIDs, err := OptionalBigIntArrayParam(args, "visible_fields") + if err != nil { + return nil, err + } + names, err := OptionalStringArrayParam(args, "visible_field_names") + if err != nil { + return nil, err + } + if len(databaseIDs) > 0 && len(names) > 0 { + return nil, errors.New("provide either 'visible_fields' or 'visible_field_names', not both") + } + if len(databaseIDs) == 0 && len(names) == 0 { + return &ProjectV2ViewConfigurationInput{VisibleFieldIDs: []githubv4.ID{}}, nil + } + + all, err := listAllProjectFields(ctx, gqlClient, owner, ownerType, projectNumber) + if err != nil { + return nil, err + } + + var resolved []ResolvedField + if len(names) > 0 { + resolved, err = resolveFieldsByName(all, owner, projectNumber, names, "visible_fields") + if err != nil { + return nil, err + } + } else { + byDatabaseID := make(map[int64]ResolvedField, len(all)) + for _, field := range all { + id, parseErr := parseInt64(field.ID) + if parseErr != nil { + continue + } + byDatabaseID[id] = field + } + resolved = make([]ResolvedField, 0, len(databaseIDs)) + for _, id := range databaseIDs { + field, ok := byDatabaseID[id] + if !ok { + return nil, fmt.Errorf("project field database ID %d was not found on project %s#%d", id, owner, projectNumber) + } + resolved = append(resolved, field) + } + } + + nodeIDs := make([]githubv4.ID, 0, len(resolved)) + seen := make(map[string]struct{}, len(resolved)) + for _, field := range resolved { + if _, ok := seen[field.NodeID]; ok { + return nil, fmt.Errorf("project field %q is included more than once", field.Name) + } + seen[field.NodeID] = struct{}{} + nodeIDs = append(nodeIDs, githubv4.ID(field.NodeID)) + } + return &ProjectV2ViewConfigurationInput{VisibleFieldIDs: nodeIDs}, nil +} + +// projectViewRequestsVisibleFields reports whether the caller asked for a non-empty +// set of visible fields, without resolving them against the project. +func projectViewRequestsVisibleFields(args map[string]any) bool { + if databaseIDs, err := OptionalBigIntArrayParam(args, "visible_fields"); err == nil && len(databaseIDs) > 0 { + return true + } + names, err := OptionalStringArrayParam(args, "visible_field_names") + return err == nil && len(names) > 0 +} + +func createProjectView(ctx context.Context, gqlClient *githubv4.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 := OptionalNullableStringParam(args, "filter") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if layout == githubv4.ProjectV2ViewLayoutRoadmapLayout && projectViewRequestsVisibleFields(args) { + return utils.NewToolResultError("visible fields are not supported for roadmap views"), nil, nil + } + configuration, err := projectViewVisibleFieldsInput(ctx, gqlClient, args, owner, ownerType, projectNumber) + if err != nil { + var structured *ghErrors.StructuredResolutionError + if errors.As(err, &structured) { + return ghErrors.NewStructuredResolutionErrorResponse(structured), nil, nil + } + return utils.NewToolResultError(err.Error()), nil, nil + } + + projectID, err := resolveProjectNodeID(ctx, gqlClient, owner, ownerType, projectNumber) + if err != nil { + return utils.NewToolResultError(fmt.Sprintf("%s: failed to resolve project: %v", ProjectViewCreateFailedError, err)), nil, nil + } + if projectID == nil || projectID == "" { + return utils.NewToolResultError(fmt.Sprintf("%s: project was not found", ProjectViewCreateFailedError)), nil, nil + } + + input := CreateProjectV2ViewInput{ + ProjectID: projectID, + Name: githubv4.String(name), + Layout: layout, + Configuration: configuration, + } + var mutation createProjectV2ViewMutation + if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil { + return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewCreateFailedError, err)), nil, nil + } + view := mutation.CreateProjectV2View.ProjectV2View + if view.ID == nil || view.ID == "" { + return utils.NewToolResultError(fmt.Sprintf("%s: response did not include a project view", ProjectViewCreateFailedError)), nil, nil + } + + if hasFilter && filter != nil { + filterValue := githubv4.String(*filter) + updateInput := UpdateProjectV2ViewInput{ + ViewID: githubv4.ID(fmt.Sprintf("%v", view.ID)), + Filter: &filterValue, + } + var updateMutation updateProjectV2ViewMutation + if err := gqlClient.Mutate(ctx, &updateMutation, updateInput, nil); err != nil { + cleanupErr := deleteProjectViewByID(ctx, gqlClient, updateInput.ViewID) + if cleanupErr != nil { + return utils.NewToolResultError(fmt.Sprintf("%s: failed to set filter: %v; failed to clean up created view %v: %v", ProjectViewCreateFailedError, err, updateInput.ViewID, cleanupErr)), nil, nil + } + return utils.NewToolResultError(fmt.Sprintf("%s: failed to set filter: %v; created view was cleaned up", ProjectViewCreateFailedError, err)), nil, nil + } + view = updateMutation.UpdateProjectV2View.ProjectV2View + } + return MarshalledTextResult(convertToMinimalProjectView(view)), nil, nil +} + +func verifyProjectViewParent(ctx context.Context, gqlClient *githubv4.Client, viewID, owner, ownerType string, projectNumber int) (githubv4.ProjectV2ViewLayout, 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 query.Node.ProjectView.Layout, 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 + } + 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 := OptionalNullableStringParam(args, "filter") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + _, hasVisibleFields := args["visible_fields"] + _, hasVisibleFieldNames := args["visible_field_names"] + if !hasName && !hasLayout && !hasFilter && !hasVisibleFields && !hasVisibleFieldNames { + return utils.NewToolResultError("update_project_view requires at least one of name, layout, filter, visible_fields, or visible_field_names"), 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 { + // The API clears a filter with an empty string, so a null filter is sent as "". + value := githubv4.String("") + if filter != nil { + value = githubv4.String(*filter) + } + input.Filter = &value + } + currentLayout, err := verifyProjectViewParent(ctx, gqlClient, viewID, owner, ownerType, projectNumber) + if err != nil { + return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewUpdateFailedError, err)), nil, nil + } + effectiveLayout := currentLayout + if input.Layout != nil { + effectiveLayout = *input.Layout + } + if effectiveLayout == githubv4.ProjectV2ViewLayoutRoadmapLayout && projectViewRequestsVisibleFields(args) { + return utils.NewToolResultError("visible fields are not supported for roadmap views"), nil, nil + } + + configuration, err := projectViewVisibleFieldsInput(ctx, gqlClient, args, owner, ownerType, projectNumber) + if err != nil { + var structured *ghErrors.StructuredResolutionError + if errors.As(err, &structured) { + return ghErrors.NewStructuredResolutionErrorResponse(structured), nil, nil + } + return utils.NewToolResultError(err.Error()), nil, nil + } + input.Configuration = configuration + + var mutation updateProjectV2ViewMutation + 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 deleteProjectViewByID(ctx context.Context, gqlClient *githubv4.Client, viewID githubv4.ID) error { + input := DeleteProjectV2ViewInput{ViewID: 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 { + return err + } + if id := mutation.DeleteProjectV2View.ProjectV2View.ID; id == nil || id == "" { + return errors.New("response did not include the deleted project view") + } + return nil +} + +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 + } + if err := deleteProjectViewByID(ctx, gqlClient, githubv4.ID(viewID)); err != nil { + return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewDeleteFailedError, err)), nil, nil + } + return MarshalledTextResult(map[string]string{"deleted_view_id": viewID}), 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_resolver.go b/pkg/github/projects_resolver.go index 1c9ba9fdcb..33e82d1de5 100644 --- a/pkg/github/projects_resolver.go +++ b/pkg/github/projects_resolver.go @@ -52,33 +52,40 @@ type projectFieldsQueryUser struct { } `graphql:"user(login: $owner)"` } -// projectFieldsConnection is a paginated list of project fields. We select `id` -// to discriminate the union variant and `databaseId` for the numeric ID REST needs. +type projectFieldNode struct { + ProjectV2Field struct { + ID githubv4.ID + DatabaseID githubv4.Int `graphql:"databaseId"` + Name githubv4.String + DataType githubv4.String + } `graphql:"... on ProjectV2Field"` + ProjectV2IterationField struct { + ID githubv4.ID + DatabaseID githubv4.Int `graphql:"databaseId"` + Name githubv4.String + DataType githubv4.String + } `graphql:"... on ProjectV2IterationField"` + ProjectV2MultiSelectField struct { + ID githubv4.ID + DatabaseID githubv4.Int `graphql:"databaseId"` + Name githubv4.String + DataType githubv4.String + } `graphql:"... on ProjectV2MultiSelectField"` + ProjectV2SingleSelectField struct { + ID githubv4.ID + DatabaseID githubv4.Int `graphql:"databaseId"` + Name githubv4.String + DataType githubv4.String + Options []struct { + ID githubv4.String + Name githubv4.String + } + } `graphql:"... on ProjectV2SingleSelectField"` +} + +// projectFieldsConnection is a paginated list of project fields. type projectFieldsConnection struct { - Nodes []struct { - ProjectV2Field struct { - ID githubv4.ID - DatabaseID githubv4.Int `graphql:"databaseId"` - Name githubv4.String - DataType githubv4.String - } `graphql:"... on ProjectV2Field"` - ProjectV2IterationField struct { - ID githubv4.ID - DatabaseID githubv4.Int `graphql:"databaseId"` - Name githubv4.String - DataType githubv4.String - } `graphql:"... on ProjectV2IterationField"` - ProjectV2SingleSelectField struct { - ID githubv4.ID - DatabaseID githubv4.Int `graphql:"databaseId"` - Name githubv4.String - DataType githubv4.String - Options []struct { - ID githubv4.String - Name githubv4.String - } - } `graphql:"... on ProjectV2SingleSelectField"` - } + Nodes []projectFieldNode PageInfo PageInfoFragment } @@ -134,6 +141,13 @@ func listAllProjectFields(ctx context.Context, gqlClient *githubv4.Client, owner Name: string(n.ProjectV2IterationField.Name), DataType: string(n.ProjectV2IterationField.DataType), }) + case n.ProjectV2MultiSelectField.ID != nil: + all = append(all, ResolvedField{ + ID: fmt.Sprintf("%d", n.ProjectV2MultiSelectField.DatabaseID), + NodeID: fmt.Sprintf("%v", n.ProjectV2MultiSelectField.ID), + Name: string(n.ProjectV2MultiSelectField.Name), + DataType: string(n.ProjectV2MultiSelectField.DataType), + }) case n.ProjectV2Field.ID != nil: all = append(all, ResolvedField{ ID: fmt.Sprintf("%d", n.ProjectV2Field.DatabaseID), @@ -154,6 +168,46 @@ func listAllProjectFields(ctx context.Context, gqlClient *githubv4.Client, owner return all, nil } +func resolveFieldsByName(all []ResolvedField, owner string, projectNumber int, names []string, idParameter string) ([]ResolvedField, error) { + byName := make(map[string][]ResolvedField, len(all)) + for _, field := range all { + key := strings.ToLower(field.Name) + byName[key] = append(byName[key], field) + } + + resolved := make([]ResolvedField, 0, len(names)) + for _, name := range names { + matches := byName[strings.ToLower(name)] + switch len(matches) { + case 0: + candidates := make([]any, 0, len(all)) + for _, field := range all { + candidates = append(candidates, map[string]any{"name": field.Name, "data_type": field.DataType}) + } + return nil, ghErrors.NewStructuredResolutionError( + "field_not_found", + name, + fmt.Sprintf("no project field named %q on project %s#%d", name, owner, projectNumber), + candidates, + ) + case 1: + resolved = append(resolved, matches[0]) + default: + candidates := make([]any, 0, len(matches)) + for _, field := range matches { + candidates = append(candidates, map[string]any{"id": field.ID, "data_type": field.DataType}) + } + return nil, ghErrors.NewStructuredResolutionError( + "field_ambiguous", + name, + fmt.Sprintf("multiple fields share this name; pass numeric IDs via '%s' to disambiguate", idParameter), + candidates, + ) + } + } + return resolved, nil +} + // resolveProjectFieldByName resolves a field by display name. Returns a // structured error on not-found, ambiguous, or wrong-data-type (when // expectedDataType is set) so the agent can self-correct. @@ -530,7 +584,7 @@ func parseInt64(s string) (int64, error) { // resolveFieldNamesToIDs resolves field names to numeric IDs in one GraphQL // hop. Fails fast with a structured error on any unresolved or ambiguous name. -func resolveFieldNamesToIDs(ctx context.Context, gqlClient *githubv4.Client, owner, ownerType string, projectNumber int, names []string) ([]int64, error) { +func resolveFieldNamesToIDs(ctx context.Context, gqlClient *githubv4.Client, owner, ownerType string, projectNumber int, names []string, idParameter string) ([]int64, error) { if len(names) == 0 { return nil, nil } @@ -540,47 +594,21 @@ func resolveFieldNamesToIDs(ctx context.Context, gqlClient *githubv4.Client, own return nil, err } - // Build a name -> []ResolvedField map so we can detect duplicates per name. - // Matching is case-insensitive to align with the GraphQL API's behaviour. - byName := make(map[string][]ResolvedField, len(all)) - for _, f := range all { - key := strings.ToLower(f.Name) - byName[key] = append(byName[key], f) - } + return resolveFieldNamesToIDsFromFields(all, names, owner, projectNumber, idParameter) +} +func resolveFieldNamesToIDsFromFields(all []ResolvedField, names []string, owner string, projectNumber int, idParameter string) ([]int64, error) { + resolved, err := resolveFieldsByName(all, owner, projectNumber, names, idParameter) + if err != nil { + return nil, err + } out := make([]int64, 0, len(names)) - for _, name := range names { - matches := byName[strings.ToLower(name)] - switch len(matches) { - case 0: - candidates := make([]any, 0, len(all)) - for _, f := range all { - candidates = append(candidates, map[string]any{"name": f.Name, "data_type": f.DataType}) - } - return nil, ghErrors.NewStructuredResolutionError( - "field_not_found", - name, - fmt.Sprintf("no project field named %q on project %s#%d", name, owner, projectNumber), - candidates, - ) - case 1: - id, parseErr := parseInt64(matches[0].ID) - if parseErr != nil { - return nil, fmt.Errorf("resolved field %q has non-numeric ID %q; pass it via 'fields' instead", name, matches[0].ID) - } - out = append(out, id) - default: - candidates := make([]any, 0, len(matches)) - for _, f := range matches { - candidates = append(candidates, map[string]any{"id": f.ID, "data_type": f.DataType}) - } - return nil, ghErrors.NewStructuredResolutionError( - "field_ambiguous", - name, - "multiple fields share this name; pass numeric IDs via 'fields' to disambiguate", - candidates, - ) + for i, field := range resolved { + id, parseErr := parseInt64(field.ID) + if parseErr != nil { + return nil, fmt.Errorf("resolved field %q has non-numeric ID %q; pass it via '%s' instead", names[i], field.ID, idParameter) } + out = append(out, id) } return out, nil } diff --git a/pkg/github/projects_resolver_test.go b/pkg/github/projects_resolver_test.go index 8b11690abe..459c6f1192 100644 --- a/pkg/github/projects_resolver_test.go +++ b/pkg/github/projects_resolver_test.go @@ -34,6 +34,12 @@ type projectFieldsTestQuery struct { Name githubv4.String DataType githubv4.String } `graphql:"... on ProjectV2IterationField"` + ProjectV2MultiSelectField struct { + ID githubv4.ID + DatabaseID githubv4.Int `graphql:"databaseId"` + Name githubv4.String + DataType githubv4.String + } `graphql:"... on ProjectV2MultiSelectField"` ProjectV2SingleSelectField struct { ID githubv4.ID DatabaseID githubv4.Int `graphql:"databaseId"` @@ -94,6 +100,15 @@ func genericFieldNode(nodeID string, databaseID int, name, dataType string) map[ } } +func multiSelectFieldNode(nodeID string, databaseID int, name string) map[string]any { + return map[string]any{ + "id": nodeID, + "databaseId": databaseID, + "name": name, + "dataType": "MULTI_SELECT", + } +} + func fieldsResponse(nodes []map[string]any) map[string]any { return map[string]any{ "organization": map[string]any{ @@ -236,7 +251,7 @@ func Test_ResolveFieldNamesToIDs_QueryRemainsIssueFieldUngated(t *testing.T) { capture := &headerCaptureTransport{inner: mocked.Transport} gql := githubv4.NewClient(&http.Client{Transport: &transportpkg.GraphQLFeaturesTransport{Transport: capture}}) - ids, err := resolveFieldNamesToIDs(context.Background(), gql, "octo-org", "org", 1, []string{"Customer"}) + ids, err := resolveFieldNamesToIDs(context.Background(), gql, "octo-org", "org", 1, []string{"Customer"}, "fields") require.NoError(t, err) assert.Equal(t, []int64{101}, ids) assert.Empty(t, capture.captured.Get(headers.GraphQLFeaturesHeader)) @@ -264,6 +279,7 @@ func Test_ResolveProjectFieldByName_NodeIDsForAllVariants(t *testing.T) { {"id": "OPT_a", "name": "Todo"}, }), iterationFieldNode("PVTIF_iteration1", 222, "Sprint"), + multiSelectFieldNode("PVTMSSF_multi1", 444, "Teams"), genericFieldNode("PVTF_text1", 333, "Notes", "TEXT"), })), ), @@ -277,6 +293,7 @@ func Test_ResolveProjectFieldByName_NodeIDsForAllVariants(t *testing.T) { }{ {"Status", "SINGLE_SELECT", "PVTSSF_single1"}, {"Sprint", "ITERATION", "PVTIF_iteration1"}, + {"Teams", "MULTI_SELECT", "PVTMSSF_multi1"}, {"Notes", "TEXT", "PVTF_text1"}, } for _, v := range variants { @@ -734,7 +751,7 @@ func Test_ResolveFieldNamesToIDs_Success(t *testing.T) { ) gql := githubv4.NewClient(mocked) - ids, err := resolveFieldNamesToIDs(context.Background(), gql, "octo-org", "org", 1, []string{"Status", "Priority"}) + ids, err := resolveFieldNamesToIDs(context.Background(), gql, "octo-org", "org", 1, []string{"Status", "Priority"}, "fields") require.NoError(t, err) assert.Equal(t, []int64{100, 200}, ids) } @@ -781,11 +798,59 @@ func Test_ResolveFieldNamesToIDs_CaseInsensitive(t *testing.T) { ) gql := githubv4.NewClient(mocked) - ids, err := resolveFieldNamesToIDs(context.Background(), gql, "octo-org", "org", 1, []string{"status", "PRIORITY"}) + ids, err := resolveFieldNamesToIDs(context.Background(), gql, "octo-org", "org", 1, []string{"status", "PRIORITY"}, "fields") require.NoError(t, err) assert.Equal(t, []int64{100, 200}, ids) } +func Test_ResolveFieldNamesToIDs_IDParameterErrors(t *testing.T) { + tests := []struct { + name string + fields []ResolvedField + idParameter string + want string + }{ + { + name: "normal project item fields", + fields: []ResolvedField{ + {ID: "100", Name: "Status"}, + {ID: "200", Name: "Status"}, + }, + idParameter: "fields", + want: "'fields'", + }, + { + name: "project view visible fields", + fields: []ResolvedField{ + {ID: "100", Name: "Status"}, + {ID: "200", Name: "Status"}, + }, + idParameter: "visible_fields", + want: "'visible_fields'", + }, + { + name: "nonnumeric project item field ID", + fields: []ResolvedField{{ID: "not-numeric", Name: "Status"}}, + idParameter: "fields", + want: "'fields'", + }, + { + name: "nonnumeric project view field ID", + fields: []ResolvedField{{ID: "not-numeric", Name: "Status"}}, + idParameter: "visible_fields", + want: "'visible_fields'", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := resolveFieldNamesToIDsFromFields(tt.fields, []string{"Status"}, "octo-org", 1, tt.idParameter) + require.Error(t, err) + assert.Contains(t, err.Error(), tt.want) + }) + } +} + // Test_ProjectsWrite_UpdateProjectItem_ByName is the acceptance test for the // write side: set Status = "In Progress" using only names plus an issue number. func Test_ProjectsWrite_UpdateProjectItem_ByName(t *testing.T) { diff --git a/pkg/github/projects_test.go b/pkg/github/projects_test.go index 075bbb70ce..e7d67f5264 100644 --- a/pkg/github/projects_test.go +++ b/pkg/github/projects_test.go @@ -36,6 +36,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"}) } @@ -596,6 +597,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"}) @@ -885,6 +888,15 @@ func Test_ProjectsWrite(t *testing.T) { assert.Contains(t, inputSchema.Properties, "pull_request_number") assert.Contains(t, inputSchema.Properties, "updated_field") assert.Contains(t, inputSchema.Properties, "items") + 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, "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) 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..2f397e7fce 100644 --- a/pkg/github/projects_v2_test.go +++ b/pkg/github/projects_v2_test.go @@ -3,7 +3,9 @@ package github import ( "context" "encoding/json" + "maps" "net/http" + "sync/atomic" "testing" "time" @@ -165,6 +167,119 @@ 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, + "layout": "TABLE_LAYOUT", + "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), + ) +} + +// countingGraphQLClient wraps a mocked GraphQL client and reports how many requests it served. +func countingGraphQLClient(matchers ...githubv4mock.Matcher) (*http.Client, func() int) { + client := githubv4mock.NewMockedHTTPClient(matchers...) + counter := &countingRoundTripper{next: client.Transport} + client.Transport = counter + return client, func() int { return int(counter.count.Load()) } +} + +type countingRoundTripper struct { + next http.RoundTripper + count atomic.Int64 +} + +func (c *countingRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + c.count.Add(1) + return c.next.RoundTrip(req) +} + +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 projectViewResponse(id string, number int, name, layout, filter string, visibleFieldIDs ...int) map[string]any { + nodes := make([]map[string]any, 0, len(visibleFieldIDs)) + for _, fieldID := range visibleFieldIDs { + nodes = append(nodes, map[string]any{"databaseId": fieldID}) + } + return map[string]any{ + "id": id, + "number": number, + "name": name, + "layout": layout, + "filter": filter, + "configuration": map[string]any{ + "visibleFields": map[string]any{"nodes": nodes}, + }, + } +} + func createFieldMatcher() githubv4mock.Matcher { return githubv4mock.NewMutationMatcher( struct { @@ -455,3 +570,1217 @@ 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", + "configuration": map[string]any{ + "visibleFields": map[string]any{ + "nodes": []map[string]any{{"databaseId": 101}, {"databaseId": 202}}, + }, + }, + }, + }, + "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", + VisibleFields: []int64{101, 202}, + }, 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}, + "configuration": map[string]any{ + "visibleFields": map[string]any{ + "nodes": []map[string]any{{"databaseId": 101}, {"databaseId": 202}}, + }, + }, + }, + }), + ), + ) + 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) + assert.Equal(t, []int64{101, 202}, view.VisibleFields) + 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) + emptyRESTClient := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})) + + t.Run("creates an ordered view and preserves create filter support", func(t *testing.T) { + filter := githubv4.String("status:Ready") + gqlClient := githubv4mock.NewMockedHTTPClient( + projectFieldNamesMatcher("octo-org", "org", 7, []map[string]any{ + statusFieldNode("PVTSSF_status", 101, "Status", nil), + multiSelectFieldNode("PVTMSSF_teams", 202, "Teams"), + }), + resolveProjectNodeIDOrgMatcher("octo-org", 7, "PVT_project7"), + githubv4mock.NewMutationMatcher( + createProjectV2ViewMutation{}, + CreateProjectV2ViewInput{ + ProjectID: githubv4.ID("PVT_project7"), + Name: githubv4.String("Ready work"), + Layout: githubv4.ProjectV2ViewLayoutTableLayout, + Configuration: &ProjectV2ViewConfigurationInput{ + VisibleFieldIDs: []githubv4.ID{"PVTMSSF_teams", "PVTSSF_status"}, + }, + }, + nil, + githubv4mock.DataResponse(map[string]any{ + "createProjectV2View": map[string]any{ + "projectV2View": projectViewResponse("PVTV_view1", 1, "Ready work", "TABLE_LAYOUT", "", 202, 101), + }, + }), + ), + githubv4mock.NewMutationMatcher( + updateProjectV2ViewMutation{}, + UpdateProjectV2ViewInput{ViewID: githubv4.ID("PVTV_view1"), Filter: &filter}, + nil, + githubv4mock.DataResponse(map[string]any{ + "updateProjectV2View": map[string]any{ + "projectV2View": projectViewResponse("PVTV_view1", 1, "Ready work", "TABLE_LAYOUT", "status:Ready", 202, 101), + }, + }), + ), + ) + deps := BaseDeps{Client: emptyRESTClient, GQLClient: githubv4.NewClient(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": "Ready work", + "layout": "table", + "filter": "status:Ready", + "visible_field_names": []any{"Teams", "Status"}, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError, getTextResult(t, result).Text) + var view MinimalProjectView + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &view)) + assert.Equal(t, []int64{202, 101}, view.VisibleFields) + assert.Equal(t, "status:Ready", view.Filter) + }) + + t.Run("keeps omitted configuration omitted", func(t *testing.T) { + gqlClient := githubv4mock.NewMockedHTTPClient( + resolveProjectNodeIDUserMatcher("octocat", 8, "PVT_project8"), + githubv4mock.NewMutationMatcher( + createProjectV2ViewMutation{}, + CreateProjectV2ViewInput{ + ProjectID: githubv4.ID("PVT_project8"), + Name: githubv4.String("Board"), + Layout: githubv4.ProjectV2ViewLayoutBoardLayout, + }, + nil, + githubv4mock.DataResponse(map[string]any{ + "createProjectV2View": map[string]any{ + "projectV2View": projectViewResponse("PVTV_view2", 2, "Board", "BOARD_LAYOUT", ""), + }, + }), + ), + ) + deps := BaseDeps{Client: emptyRESTClient, GQLClient: githubv4.NewClient(gqlClient)} + 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, getTextResult(t, result).Text) + assert.JSONEq(t, `{"id":"PVTV_view2","number":2,"name":"Board","layout":"board","filter":"","visible_fields":[]}`, getTextResult(t, result).Text) + }) + + t.Run("cleans up when applying a create filter fails", func(t *testing.T) { + filter := githubv4.String("status:Ready") + gqlClient := githubv4mock.NewMockedHTTPClient( + resolveProjectNodeIDOrgMatcher("octo-org", 7, "PVT_project7"), + githubv4mock.NewMutationMatcher( + createProjectV2ViewMutation{}, + CreateProjectV2ViewInput{ + ProjectID: githubv4.ID("PVT_project7"), + Name: githubv4.String("Filtered"), + Layout: githubv4.ProjectV2ViewLayoutTableLayout, + }, + nil, + githubv4mock.DataResponse(map[string]any{ + "createProjectV2View": map[string]any{ + "projectV2View": projectViewResponse("PVTV_cleanup", 4, "Filtered", "TABLE_LAYOUT", ""), + }, + }), + ), + githubv4mock.NewMutationMatcher( + updateProjectV2ViewMutation{}, + UpdateProjectV2ViewInput{ViewID: githubv4.ID("PVTV_cleanup"), Filter: &filter}, + nil, + githubv4mock.ErrorResponse("filter failed"), + ), + githubv4mock.NewMutationMatcher( + struct { + DeleteProjectV2View struct { + ProjectV2View struct { + ID githubv4.ID + } `graphql:"projectV2View"` + } `graphql:"deleteProjectV2View(input: $input)"` + }{}, + DeleteProjectV2ViewInput{ViewID: githubv4.ID("PVTV_cleanup")}, + nil, + githubv4mock.DataResponse(map[string]any{ + "deleteProjectV2View": map[string]any{ + "projectV2View": map[string]any{"id": "PVTV_cleanup"}, + }, + }), + ), + ) + deps := BaseDeps{Client: emptyRESTClient, GQLClient: githubv4.NewClient(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": "Filtered", + "layout": "table", + "filter": "status:Ready", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "filter failed") + assert.Contains(t, getTextResult(t, result).Text, "created view was cleaned up") + }) + + t.Run("returns the orphaned view ID when cleanup fails", func(t *testing.T) { + filter := githubv4.String("status:Ready") + gqlClient := githubv4mock.NewMockedHTTPClient( + resolveProjectNodeIDOrgMatcher("octo-org", 7, "PVT_project7"), + githubv4mock.NewMutationMatcher( + createProjectV2ViewMutation{}, + CreateProjectV2ViewInput{ + ProjectID: githubv4.ID("PVT_project7"), + Name: githubv4.String("Filtered"), + Layout: githubv4.ProjectV2ViewLayoutTableLayout, + }, + nil, + githubv4mock.DataResponse(map[string]any{ + "createProjectV2View": map[string]any{ + "projectV2View": projectViewResponse("PVTV_orphan", 4, "Filtered", "TABLE_LAYOUT", ""), + }, + }), + ), + githubv4mock.NewMutationMatcher( + updateProjectV2ViewMutation{}, + UpdateProjectV2ViewInput{ViewID: githubv4.ID("PVTV_orphan"), Filter: &filter}, + nil, + githubv4mock.ErrorResponse("filter failed"), + ), + githubv4mock.NewMutationMatcher( + struct { + DeleteProjectV2View struct { + ProjectV2View struct { + ID githubv4.ID + } `graphql:"projectV2View"` + } `graphql:"deleteProjectV2View(input: $input)"` + }{}, + DeleteProjectV2ViewInput{ViewID: githubv4.ID("PVTV_orphan")}, + nil, + githubv4mock.ErrorResponse("cleanup failed"), + ), + ) + deps := BaseDeps{Client: emptyRESTClient, GQLClient: githubv4.NewClient(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": "Filtered", + "layout": "table", + "filter": "status:Ready", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + text := getTextResult(t, result).Text + assert.Contains(t, text, "filter failed") + assert.Contains(t, text, "cleanup failed") + assert.Contains(t, text, "PVTV_orphan") + }) + + t.Run("skips the filter mutation when the filter is null", func(t *testing.T) { + // Only the create mutation is registered, so a follow-up filter mutation would 404. + gqlClient := githubv4mock.NewMockedHTTPClient( + resolveProjectNodeIDOrgMatcher("octo-org", 7, "PVT_project7"), + githubv4mock.NewMutationMatcher( + createProjectV2ViewMutation{}, + CreateProjectV2ViewInput{ + ProjectID: githubv4.ID("PVT_project7"), + Name: githubv4.String("Unfiltered"), + Layout: githubv4.ProjectV2ViewLayoutTableLayout, + }, + nil, + githubv4mock.DataResponse(map[string]any{ + "createProjectV2View": map[string]any{ + "projectV2View": projectViewResponse("PVTV_nullfilter", 5, "Unfiltered", "TABLE_LAYOUT", ""), + }, + }), + ), + ) + deps := BaseDeps{Client: emptyRESTClient, GQLClient: githubv4.NewClient(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": "Unfiltered", + "layout": "table", + "filter": nil, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError, getTextResult(t, result).Text) + var view MinimalProjectView + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &view)) + assert.Equal(t, "PVTV_nullfilter", view.ID) + assert.Equal(t, "", view.Filter) + }) + + t.Run("sends explicit empty configuration", func(t *testing.T) { + gqlClient := githubv4mock.NewMockedHTTPClient( + resolveProjectNodeIDOrgMatcher("octo-org", 7, "PVT_project7"), + githubv4mock.NewMutationMatcher( + createProjectV2ViewMutation{}, + CreateProjectV2ViewInput{ + ProjectID: githubv4.ID("PVT_project7"), + Name: githubv4.String("Title only"), + Layout: githubv4.ProjectV2ViewLayoutTableLayout, + Configuration: &ProjectV2ViewConfigurationInput{ + VisibleFieldIDs: []githubv4.ID{}, + }, + }, + nil, + githubv4mock.DataResponse(map[string]any{ + "createProjectV2View": map[string]any{ + "projectV2View": projectViewResponse("PVTV_empty", 3, "Title only", "TABLE_LAYOUT", "", 101), + }, + }), + ), + ) + deps := BaseDeps{Client: emptyRESTClient, GQLClient: githubv4.NewClient(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": "Title only", + "layout": "table", + "visible_fields": []any{}, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError, getTextResult(t, result).Text) + assert.Contains(t, getTextResult(t, result).Text, `"visible_fields":[101]`) + }) + + 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"}), + }) + gqlClient := githubv4mock.NewMockedHTTPClient( + resolveProjectNodeIDOrgMatcher("octo-org", 9, "PVT_project9"), + githubv4mock.NewMutationMatcher( + createProjectV2ViewMutation{}, + CreateProjectV2ViewInput{ + ProjectID: githubv4.ID("PVT_project9"), + Name: githubv4.String("Table"), + Layout: githubv4.ProjectV2ViewLayoutTableLayout, + }, + nil, + githubv4mock.DataResponse(map[string]any{ + "createProjectV2View": map[string]any{ + "projectV2View": projectViewResponse("PVTV_view3", 3, "Table", "TABLE_LAYOUT", ""), + }, + }), + ), + ) + deps := BaseDeps{Client: mustNewGHClient(t, restClient), GQLClient: githubv4.NewClient(gqlClient)} + 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, getTextResult(t, result).Text) + }) + + t.Run("rejects conflicting, unknown, duplicate, and roadmap fields before mutation", func(t *testing.T) { + tests := []struct { + name string + fields []map[string]any + request map[string]any + expectedError string + expectedHint string + }{ + { + name: "conflicting identifiers", + request: map[string]any{ + "visible_fields": []any{"101"}, + "visible_field_names": []any{"Status"}, + }, + expectedError: "provide either 'visible_fields' or 'visible_field_names'", + }, + { + name: "unknown numeric ID", + fields: []map[string]any{statusFieldNode("PVTSSF_status", 101, "Status", nil)}, + request: map[string]any{"visible_fields": []any{"202"}}, + expectedError: "database ID 202 was not found", + }, + { + name: "duplicate name", + fields: []map[string]any{statusFieldNode("PVTSSF_status", 101, "Status", nil)}, + request: map[string]any{"visible_field_names": []any{"Status", "status"}}, + expectedError: "included more than once", + }, + { + name: "unknown name", + fields: []map[string]any{statusFieldNode("PVTSSF_status", 101, "Status", nil)}, + request: map[string]any{"visible_field_names": []any{"Priority"}}, + expectedError: "field_not_found", + }, + { + name: "ambiguous name", + fields: []map[string]any{ + statusFieldNode("PVTSSF_status1", 101, "Status", nil), + statusFieldNode("PVTSSF_status2", 202, "Status", nil), + }, + request: map[string]any{"visible_field_names": []any{"Status"}}, + expectedError: "field_ambiguous", + expectedHint: "visible_fields", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + matchers := []githubv4mock.Matcher{} + if len(tc.fields) > 0 { + matchers = append(matchers, projectFieldNamesMatcher("octo-org", "org", 7, tc.fields)) + } + deps := BaseDeps{ + Client: emptyRESTClient, + GQLClient: githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matchers...)), + } + handler := toolDef.Handler(deps) + requestArgs := map[string]any{ + "method": "create_project_view", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(7), + "name": "Table", + "layout": "table", + } + maps.Copy(requestArgs, tc.request) + request := createMCPRequest(requestArgs) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, tc.expectedError) + if tc.expectedHint != "" { + var response map[string]any + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &response)) + assert.Contains(t, response["hint"], tc.expectedHint) + assert.NotContains(t, response["hint"], "'fields'") + } + }) + } + }) + + t.Run("rejects roadmap layout before resolving visible field names", func(t *testing.T) { + gqlClient, requests := countingGraphQLClient( + projectFieldNamesMatcher("octo-org", "org", 7, []map[string]any{statusFieldNode("PVTSSF_status", 101, "Status", nil)}), + ) + deps := BaseDeps{Client: emptyRESTClient, GQLClient: githubv4.NewClient(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": "Timeline", + "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") + assert.Zero(t, requests(), "expected no field-listing GraphQL request") + }) +} + +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( + resolveProjectNodeIDOrgMatcher("octo-org", 7, "PVT_project7"), + projectViewParentMatcher("PVTV_view1", "PVT_project7"), + githubv4mock.NewMutationMatcher( + struct { + UpdateProjectV2View struct { + ProjectV2View projectViewNode `graphql:"projectV2View"` + } `graphql:"updateProjectV2View(input: $input)"` + }{}, + UpdateProjectV2ViewInput{ + ViewID: githubv4.ID("PVTV_view1"), + Name: &name, + }, + nil, + githubv4mock.DataResponse(map[string]any{ + "updateProjectV2View": map[string]any{ + "projectV2View": projectViewResponse("PVTV_view1", 1, "Renamed", "TABLE_LAYOUT", "status:Ready", 101, 202), + }, + }), + ), + ) + 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"`) + assert.Contains(t, getTextResult(t, result).Text, `"visible_fields":[101,202]`) + }) + + t.Run("replaces and reorders visible fields by database ID", func(t *testing.T) { + gqlClient := githubv4mock.NewMockedHTTPClient( + projectFieldNamesMatcher("octo-org", "org", 7, []map[string]any{ + statusFieldNode("PVTSSF_status", 101, "Status", nil), + multiSelectFieldNode("PVTMSSF_teams", 202, "Teams"), + }), + resolveProjectNodeIDOrgMatcher("octo-org", 7, "PVT_project7"), + projectViewParentMatcher("PVTV_view1", "PVT_project7"), + githubv4mock.NewMutationMatcher( + updateProjectV2ViewMutation{}, + UpdateProjectV2ViewInput{ + ViewID: githubv4.ID("PVTV_view1"), + Configuration: &ProjectV2ViewConfigurationInput{ + VisibleFieldIDs: []githubv4.ID{"PVTMSSF_teams", "PVTSSF_status"}, + }, + }, + nil, + githubv4mock.DataResponse(map[string]any{ + "updateProjectV2View": map[string]any{ + "projectV2View": projectViewResponse("PVTV_view1", 1, "Ready work", "TABLE_LAYOUT", "", 202, 101), + }, + }), + ), + ) + 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", + "visible_fields": []any{"202", "101"}, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError, getTextResult(t, result).Text) + assert.Contains(t, getTextResult(t, result).Text, `"visible_fields":[202,101]`) + }) + + t.Run("sends explicit empty visible fields to reset", func(t *testing.T) { + gqlClient := githubv4mock.NewMockedHTTPClient( + resolveProjectNodeIDOrgMatcher("octo-org", 7, "PVT_project7"), + projectViewParentMatcher("PVTV_view1", "PVT_project7"), + githubv4mock.NewMutationMatcher( + updateProjectV2ViewMutation{}, + UpdateProjectV2ViewInput{ + ViewID: githubv4.ID("PVTV_view1"), + Configuration: &ProjectV2ViewConfigurationInput{ + VisibleFieldIDs: []githubv4.ID{}, + }, + }, + nil, + githubv4mock.DataResponse(map[string]any{ + "updateProjectV2View": map[string]any{ + "projectV2View": projectViewResponse("PVTV_view1", 1, "Ready work", "TABLE_LAYOUT", "", 101), + }, + }), + ), + ) + 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", + "visible_field_names": []any{}, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError, getTextResult(t, result).Text) + assert.Contains(t, getTextResult(t, result).Text, `"visible_fields":[101]`) + }) + + t.Run("rejects nonempty visible fields on an existing roadmap", func(t *testing.T) { + gqlClient := githubv4mock.NewMockedHTTPClient( + projectFieldNamesMatcher("octo-org", "org", 7, []map[string]any{ + statusFieldNode("PVTSSF_status", 101, "Status", nil), + }), + resolveProjectNodeIDOrgMatcher("octo-org", 7, "PVT_project7"), + githubv4mock.NewQueryMatcher( + projectViewParentQuery{}, + map[string]any{"id": githubv4.ID("PVTV_roadmap")}, + githubv4mock.DataResponse(map[string]any{ + "node": map[string]any{ + "id": "PVTV_roadmap", + "layout": "ROADMAP_LAYOUT", + "project": map[string]any{"id": "PVT_project7"}, + }, + }), + ), + ) + 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_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") + }) + + t.Run("sends null 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 `graphql:"projectV2View"` + } `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": nil, + }) + + 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("rejects an empty string filter", 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", + "filter": "", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "must not be empty") + }) + + 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 `graphql:"projectV2View"` + } `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( + resolveProjectNodeIDOrgMatcher("octo-org", 7, "PVT_project7"), + projectViewParentMatcher("PVTV_view1", "PVT_project7"), + githubv4mock.NewMutationMatcher( + struct { + UpdateProjectV2View struct { + ProjectV2View projectViewNode `graphql:"projectV2View"` + } `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") + }) + + 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{})), + 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, filter, visible_fields, or visible_field_names") + }) +} + +func Test_ProjectsWrite_DeleteProjectView(t *testing.T) { + toolDef := ProjectsWrite(translations.NullTranslationHelper) + + 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) + }) + + 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") + }) +} diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 9825ba8845..a86b699f7f 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -307,7 +307,7 @@ func GetPullRequestStatus(ctx context.Context, client *github.Client, owner, rep return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get combined status", resp, body), nil } - r, err := json.Marshal(status) + r, err := json.Marshal(convertToMinimalCombinedStatus(status)) if err != nil { return nil, fmt.Errorf("failed to marshal response: %w", err) } @@ -1281,10 +1281,9 @@ func AddReplyToPullRequestComment(t translations.TranslationHelperFunc) inventor } } - var comment *github.PullRequestComment + var commentResponse *MinimalResponse if hasBody { - var resp *github.Response - comment, resp, err = client.PullRequests.CreateCommentInReplyTo(ctx, owner, repo, pullNumber, body, commentID) + comment, resp, err := client.PullRequests.CreateCommentInReplyTo(ctx, owner, repo, pullNumber, body, commentID) if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to add reply to pull request comment", resp, err), nil, nil } @@ -1297,19 +1296,24 @@ func AddReplyToPullRequestComment(t translations.TranslationHelperFunc) inventor } return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to add reply to pull request comment", resp, bodyBytes), nil, nil } + + commentResponse = &MinimalResponse{ + ID: fmt.Sprintf("%d", comment.GetID()), + URL: comment.GetHTMLURL(), + } } var result any switch { case hasBody && hasReaction: - result = map[string]any{ - "comment": comment, - "reaction": reactionResponse, + result = map[string]MinimalResponse{ + "comment": *commentResponse, + "reaction": *reactionResponse, } case hasReaction: result = reactionResponse default: - result = comment + result = commentResponse } r, err := json.Marshal(result) diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index 5fe1229dba..1edf16e7b7 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -1575,16 +1575,32 @@ func Test_GetPullRequestStatus(t *testing.T) { }, } - // Setup mock status for success case + statusCreatedAt := &github.Timestamp{Time: time.Date(2026, time.August, 11, 9, 30, 0, 0, time.UTC)} + statusUpdatedAt := &github.Timestamp{Time: time.Date(2026, time.August, 11, 9, 35, 0, 0, time.UTC)} mockStatus := &github.CombinedStatus{ + Name: github.Ptr("abcd1234"), State: github.Ptr("success"), - TotalCount: github.Ptr(3), + SHA: github.Ptr("abcd1234"), + TotalCount: github.Ptr(2), + CommitURL: github.Ptr("https://api.github.com/repos/owner/repo/commits/abcd1234"), + RepositoryURL: github.Ptr( + "https://api.github.com/repos/owner/repo", + ), Statuses: []*github.RepoStatus{ { + ID: github.Ptr(int64(101)), + NodeID: github.Ptr("SC_kwDOStatus101"), + URL: github.Ptr("https://api.github.com/repos/owner/repo/statuses/abcd1234"), State: github.Ptr("success"), Context: github.Ptr("continuous-integration/travis-ci"), Description: github.Ptr("Build succeeded"), TargetURL: github.Ptr("https://travis-ci.org/owner/repo/builds/123"), + AvatarURL: github.Ptr("https://avatars.githubusercontent.com/in/123"), + Creator: &github.User{ + Login: github.Ptr("ci-bot"), + }, + CreatedAt: statusCreatedAt, + UpdatedAt: statusUpdatedAt, }, { State: github.Ptr("success"), @@ -1592,25 +1608,25 @@ func Test_GetPullRequestStatus(t *testing.T) { Description: github.Ptr("Coverage increased"), TargetURL: github.Ptr("https://codecov.io/gh/owner/repo/pull/42"), }, - { - State: github.Ptr("success"), - Context: github.Ptr("lint/golangci-lint"), - Description: github.Ptr("No issues found"), - TargetURL: github.Ptr("https://golangci.com/r/owner/repo/pull/42"), - }, }, } + emptyStatus := &github.CombinedStatus{ + State: github.Ptr("pending"), + SHA: github.Ptr("abcd1234"), + TotalCount: github.Ptr(0), + Statuses: []*github.RepoStatus{nil}, + } tests := []struct { name string mockedClient *http.Client requestArgs map[string]any expectError bool - expectedStatus *github.CombinedStatus + expectedStatus *MinimalCombinedStatus expectedErrMsg string }{ { - name: "successful status fetch", + name: "successful status fetch with multiple statuses", mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ GetReposPullsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockPR), GetReposCommitsStatusByOwnerByRepoByRef: mockResponse(t, http.StatusOK, mockStatus), @@ -1621,8 +1637,46 @@ func Test_GetPullRequestStatus(t *testing.T) { "repo": "repo", "pullNumber": float64(42), }, - expectError: false, - expectedStatus: mockStatus, + expectedStatus: &MinimalCombinedStatus{ + State: "success", + SHA: "abcd1234", + TotalCount: 2, + Statuses: []MinimalRepoStatus{ + { + State: "success", + Context: "continuous-integration/travis-ci", + Description: "Build succeeded", + TargetURL: "https://travis-ci.org/owner/repo/builds/123", + CreatedAt: "2026-08-11T09:30:00Z", + UpdatedAt: "2026-08-11T09:35:00Z", + }, + { + State: "success", + Context: "codecov/patch", + Description: "Coverage increased", + TargetURL: "https://codecov.io/gh/owner/repo/pull/42", + }, + }, + }, + }, + { + name: "successful status fetch with no statuses", + mockedClient: MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetReposPullsByOwnerByRepoByPullNumber: mockResponse(t, http.StatusOK, mockPR), + GetReposCommitsStatusByOwnerByRepoByRef: mockResponse(t, http.StatusOK, emptyStatus), + }), + requestArgs: map[string]any{ + "method": "get_status", + "owner": "owner", + "repo": "repo", + "pullNumber": float64(42), + }, + expectedStatus: &MinimalCombinedStatus{ + State: "pending", + SHA: "abcd1234", + TotalCount: 0, + Statuses: []MinimalRepoStatus{}, + }, }, { name: "PR fetch fails", @@ -1691,20 +1745,33 @@ func Test_GetPullRequestStatus(t *testing.T) { require.NoError(t, err) require.False(t, result.IsError) - // Parse the result and get the text content if no error textContent := getTextResult(t, result) - // Unmarshal and verify the result - var returnedStatus github.CombinedStatus + var returnedStatus MinimalCombinedStatus err = json.Unmarshal([]byte(textContent.Text), &returnedStatus) require.NoError(t, err) - assert.Equal(t, *tc.expectedStatus.State, *returnedStatus.State) - assert.Equal(t, *tc.expectedStatus.TotalCount, *returnedStatus.TotalCount) - assert.Len(t, returnedStatus.Statuses, len(tc.expectedStatus.Statuses)) - for i, status := range returnedStatus.Statuses { - assert.Equal(t, *tc.expectedStatus.Statuses[i].State, *status.State) - assert.Equal(t, *tc.expectedStatus.Statuses[i].Context, *status.Context) - assert.Equal(t, *tc.expectedStatus.Statuses[i].Description, *status.Description) + assert.Equal(t, *tc.expectedStatus, returnedStatus) + + expectedJSON, err := json.Marshal(tc.expectedStatus) + require.NoError(t, err) + assert.JSONEq(t, string(expectedJSON), textContent.Text) + + var payload map[string]any + require.NoError(t, json.Unmarshal([]byte(textContent.Text), &payload)) + assert.NotContains(t, payload, "name") + assert.NotContains(t, payload, "commit_url") + assert.NotContains(t, payload, "repository_url") + + statuses, ok := payload["statuses"].([]any) + require.True(t, ok) + for _, status := range statuses { + statusPayload, ok := status.(map[string]any) + require.True(t, ok) + assert.NotContains(t, statusPayload, "id") + assert.NotContains(t, statusPayload, "node_id") + assert.NotContains(t, statusPayload, "url") + assert.NotContains(t, statusPayload, "avatar_url") + assert.NotContains(t, statusPayload, "creator") } }) } @@ -4145,6 +4212,13 @@ func TestAddReplyToPullRequestComment(t *testing.T) { } replyCreatedAfterReactionFailure := &atomic.Bool{} + assertMinimalResponse := func(t *testing.T, response map[string]any, expectedID, expectedURL string) { + t.Helper() + assert.Len(t, response, 2) + assert.Equal(t, expectedID, response["id"]) + assert.Equal(t, expectedURL, response["url"]) + } + tests := []struct { name string mockedClient *http.Client @@ -4354,14 +4428,29 @@ func TestAddReplyToPullRequestComment(t *testing.T) { return } - // Parse the result and verify it's not an error require.False(t, result.IsError) textContent := getTextResult(t, result) - if _, ok := tc.requestArgs["body"]; ok { - assert.Contains(t, textContent.Text, "This is a reply to the comment") - } - if _, ok := tc.requestArgs["reaction"]; ok { - assert.Contains(t, textContent.Text, "789") + + var response map[string]any + require.NoError(t, json.Unmarshal([]byte(textContent.Text), &response)) + + _, hasBody := tc.requestArgs["body"] + _, hasReaction := tc.requestArgs["reaction"] + reactionURL := client.BaseURL() + "repos/owner/repo/pulls/comments/123/reactions/789" + + switch { + case hasBody && hasReaction: + assert.Len(t, response, 2) + commentResponse, ok := response["comment"].(map[string]any) + require.True(t, ok) + assertMinimalResponse(t, commentResponse, "456", "https://github.com/owner/repo/pull/42#discussion_r456") + reactionResponse, ok := response["reaction"].(map[string]any) + require.True(t, ok) + assertMinimalResponse(t, reactionResponse, "789", reactionURL) + case hasBody: + assertMinimalResponse(t, response, "456", "https://github.com/owner/repo/pull/42#discussion_r456") + default: + assertMinimalResponse(t, response, "789", reactionURL) } }) } 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. diff --git a/pkg/http/oauth/oauth_test.go b/pkg/http/oauth/oauth_test.go index f39ef39b87..1c2aa5c7c1 100644 --- a/pkg/http/oauth/oauth_test.go +++ b/pkg/http/oauth/oauth_test.go @@ -691,10 +691,11 @@ func TestAPIHostResolver_AuthorizationServerURL(t *testing.T) { expectedStatusCode: http.StatusOK, }, { - name: "GHES with http scheme returns the correct authorization server URL", - host: "http://ghe.example.com", - expectedURL: "http://ghe.example.com/login/oauth", - expectedStatusCode: http.StatusOK, + name: "GHES with http scheme is rejected to avoid cleartext credentials", + host: "http://ghe.example.com", + expectedURL: "", + expectedError: true, + errorContains: "host must use https", }, { name: "custom authorization server in config takes precedence", diff --git a/pkg/utils/api.go b/pkg/utils/api.go index 95dfbd1d5b..4a8d14e4ef 100644 --- a/pkg/utils/api.go +++ b/pkg/utils/api.go @@ -145,12 +145,20 @@ func newGHESHost(hostname string) (APIHost, error) { return APIHost{}, fmt.Errorf("failed to parse GHES URL: %w", err) } - restURL, err := url.Parse(fmt.Sprintf("%s://%s/api/v3/", u.Scheme, u.Hostname())) + // Preserve the full authority (host, port, and IPv6 brackets) for the + // base-host URLs. u.Hostname() drops the port and strips IPv6 brackets, + // which would silently retarget a loopback dev server to port 80 and produce + // an unusable URL for [::1]. The subdomain-isolation URLs below still derive + // from the bare hostname, since a label cannot be prepended to a host:port or + // an IP literal. + authority := u.Host + + restURL, err := url.Parse(fmt.Sprintf("%s://%s/api/v3/", u.Scheme, authority)) if err != nil { return APIHost{}, fmt.Errorf("failed to parse GHES REST URL: %w", err) } - gqlURL, err := url.Parse(fmt.Sprintf("%s://%s/api/graphql", u.Scheme, u.Hostname())) + gqlURL, err := url.Parse(fmt.Sprintf("%s://%s/api/graphql", u.Scheme, authority)) if err != nil { return APIHost{}, fmt.Errorf("failed to parse GHES GraphQL URL: %w", err) } @@ -165,7 +173,7 @@ func newGHESHost(hostname string) (APIHost, error) { uploadURL, err = url.Parse(fmt.Sprintf("%s://uploads.%s/", u.Scheme, u.Hostname())) } else { // Without subdomain isolation: https://hostname/api/uploads/ - uploadURL, err = url.Parse(fmt.Sprintf("%s://%s/api/uploads/", u.Scheme, u.Hostname())) + uploadURL, err = url.Parse(fmt.Sprintf("%s://%s/api/uploads/", u.Scheme, authority)) } if err != nil { return APIHost{}, fmt.Errorf("failed to parse GHES Upload URL: %w", err) @@ -177,13 +185,13 @@ func newGHESHost(hostname string) (APIHost, error) { rawURL, err = url.Parse(fmt.Sprintf("%s://raw.%s/", u.Scheme, u.Hostname())) } else { // Without subdomain isolation: https://hostname/raw/ - rawURL, err = url.Parse(fmt.Sprintf("%s://%s/raw/", u.Scheme, u.Hostname())) + rawURL, err = url.Parse(fmt.Sprintf("%s://%s/raw/", u.Scheme, authority)) } if err != nil { return APIHost{}, fmt.Errorf("failed to parse GHES Raw URL: %w", err) } - authorizationServerURL, err := url.Parse(fmt.Sprintf("%s://%s/login/oauth", u.Scheme, u.Hostname())) + authorizationServerURL, err := url.Parse(fmt.Sprintf("%s://%s/login/oauth", u.Scheme, authority)) if err != nil { return APIHost{}, fmt.Errorf("failed to parse GHES Authorization Server URL: %w", err) } @@ -235,6 +243,13 @@ func parseAPIHost(s string) (APIHost, error) { return APIHost{}, fmt.Errorf("host must have a scheme (http or https): %s", s) } + // Enforce HTTPS centrally so no deployment (GHES in particular) can build + // authenticated REST/GraphQL/upload/raw URLs over cleartext http, which + // would leak the bearer token/PAT to anyone on the network. + if err := requireSecureScheme(u); err != nil { + return APIHost{}, err + } + switch classifyHost(u) { case HostTypeDotcom: return newDotcomHost() @@ -245,6 +260,36 @@ func parseAPIHost(s string) (APIHost, error) { } } +// requireSecureScheme rejects hosts that would carry credentials over cleartext. +// Every REST/GraphQL/upload/raw/authorization URL is derived from this host and +// used for authenticated requests, so an http scheme would expose the bearer +// token/PAT to network interception and replay. http is permitted only for +// loopback hosts so that local development against a dev server still works. +func requireSecureScheme(u *url.URL) error { + if u.Scheme == "https" { + return nil + } + if u.Scheme == "http" && isLoopbackHost(u.Hostname()) { + return nil + } + return fmt.Errorf( + "host must use https to avoid sending credentials over cleartext: %s (http is only permitted for loopback hosts such as localhost, 127.0.0.1, or ::1)", + u.Scheme+"://"+u.Hostname(), + ) +} + +// isLoopbackHost reports whether hostname is a loopback address. Only exact +// loopback names/addresses qualify, so credentials are never sent in cleartext +// to a remote host. +func isLoopbackHost(hostname string) bool { + switch strings.ToLower(hostname) { + case "localhost", "127.0.0.1", "::1": + return true + default: + return false + } +} + // HostType identifies which GitHub deployment a host refers to. Tools use this // to skip capabilities that only exist on some deployments. type HostType int diff --git a/pkg/utils/api_test.go b/pkg/utils/api_test.go index 40fcb8f26a..baa1eb30ce 100644 --- a/pkg/utils/api_test.go +++ b/pkg/utils/api_test.go @@ -13,6 +13,7 @@ func TestParseAPIHost(t *testing.T) { input string wantRestURL string wantErr bool + errContains string }{ { name: "empty string defaults to dotcom", @@ -59,13 +60,53 @@ func TestParseAPIHost(t *testing.T) { input: "github.com", wantErr: true, }, + { + name: "http GHES rejected to avoid cleartext credentials", + input: "http://ghes.example.com", + wantErr: true, + errContains: "host must use https", + }, + { + name: "http loopback allowed for local development", + input: "http://localhost", + wantRestURL: "http://localhost/api/v3/", + }, + { + name: "http 127.0.0.1 loopback allowed for local development", + input: "http://127.0.0.1", + wantRestURL: "http://127.0.0.1/api/v3/", + }, + { + name: "http loopback preserves port for local development", + input: "http://localhost:3000", + wantRestURL: "http://localhost:3000/api/v3/", + }, + { + name: "http ipv6 loopback preserves brackets", + input: "http://[::1]", + wantRestURL: "http://[::1]/api/v3/", + }, + { + name: "http ipv6 loopback preserves brackets and port", + input: "http://[::1]:8080", + wantRestURL: "http://[::1]:8080/api/v3/", + }, + { + name: "http remote host rejected", + input: "http://notgithub.com", + wantErr: true, + errContains: "host must use https", + }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { host, err := parseAPIHost(tc.input) if tc.wantErr { - assert.Error(t, err) + require.Error(t, err) + if tc.errContains != "" { + assert.Contains(t, err.Error(), tc.errContains) + } return } require.NoError(t, err)