From eff4c3c041742426f417f7c2247b96bbf6d60b69 Mon Sep 17 00:00:00 2001 From: Tommaso Moro <37270480+tommaso-moro@users.noreply.github.com> Date: Mon, 10 Aug 2026 15:13:30 +0100 Subject: [PATCH 1/7] Minimize Actions workflow list responses (#3047) Return compact response types for workflow run and workflow job lists while retaining diagnostic, step, and runner metadata. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0eecbca7-7271-4a04-8d28-d952c27ed9c1 --- pkg/github/actions.go | 4 +- pkg/github/actions_minimal_test.go | 267 +++++++++++++++++++++++++++++ pkg/github/actions_test.go | 44 ++++- pkg/github/minimal_types.go | 220 ++++++++++++++++++++++++ 4 files changed, 529 insertions(+), 6 deletions(-) create mode 100644 pkg/github/actions_minimal_test.go diff --git a/pkg/github/actions.go b/pkg/github/actions.go index c16efa0f18..1629b818f7 100644 --- a/pkg/github/actions.go +++ b/pkg/github/actions.go @@ -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..e5c0662cc4 100644 --- a/pkg/github/actions_test.go +++ b/pkg/github/actions_test.go @@ -154,10 +154,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 +204,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) diff --git a/pkg/github/minimal_types.go b/pkg/github/minimal_types.go index e2bf8b684b..c05c38e7bd 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. @@ -1832,6 +1914,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"` From ff15f6825deca167aea593dbb23781705b0d21ab Mon Sep 17 00:00:00 2001 From: Tommaso Moro <37270480+tommaso-moro@users.noreply.github.com> Date: Wed, 12 Aug 2026 12:33:51 +0100 Subject: [PATCH 2/7] Use minimal types for tool responses (#3055) Return compact response shapes for pull request statuses, review comment replies, and individual workflow runs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d6786153-698a-4563-97ad-a8221c40e306 --- pkg/github/actions.go | 2 +- pkg/github/actions_test.go | 23 +++-- pkg/github/minimal_types.go | 54 ++++++++++++ pkg/github/pullrequests.go | 20 +++-- pkg/github/pullrequests_test.go | 145 ++++++++++++++++++++++++++------ 5 files changed, 198 insertions(+), 46 deletions(-) diff --git a/pkg/github/actions.go b/pkg/github/actions.go index 1629b818f7..85dd99e1aa 100644 --- a/pkg/github/actions.go +++ b/pkg/github/actions.go @@ -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) } diff --git a/pkg/github/actions_test.go b/pkg/github/actions_test.go index e5c0662cc4..a25a35f704 100644 --- a/pkg/github/actions_test.go +++ b/pkg/github/actions_test.go @@ -307,14 +307,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) }), @@ -338,11 +333,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") }) } diff --git a/pkg/github/minimal_types.go b/pkg/github/minimal_types.go index c05c38e7bd..15993f35a0 100644 --- a/pkg/github/minimal_types.go +++ b/pkg/github/minimal_types.go @@ -699,6 +699,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"` @@ -1057,6 +1075,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 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) } }) } From d6cab9757ac577eecf1821ff167895a76b342beb Mon Sep 17 00:00:00 2001 From: Bryan Zwicker Date: Wed, 12 Aug 2026 08:41:33 -0400 Subject: [PATCH 3/7] Add basic project view management (#2961) * Add basic project view management Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c6f8ede6-efee-4191-900d-59a1bb0af000 * Harden project view mutations Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c6f8ede6-efee-4191-900d-59a1bb0af000 * Resolve project view fields by name Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c6f8ede6-efee-4191-900d-59a1bb0af000 * Clear project view filters with explicit null Align the filter parameter with the nullable-parameter convention: omit to preserve, pass null to clear. Empty strings are now rejected rather than treated as a clear sentinel. The GraphQL and REST wire format is unchanged, since the API still clears a filter with an empty string. Also replace the "" string comparison in deleteProjectView with a direct nil check on the returned ID. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Use caller-specific project field hints Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c6f8ede6-efee-4191-900d-59a1bb0af000 --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c6f8ede6-efee-4191-900d-59a1bb0af000 --- README.md | 9 +- pkg/github/__toolsnaps__/projects_get.snap | 9 +- pkg/github/__toolsnaps__/projects_list.snap | 7 +- pkg/github/__toolsnaps__/projects_write.snap | 47 +- pkg/github/minimal_types.go | 9 + pkg/github/projects.go | 553 +++++++++- pkg/github/projects_resolver.go | 10 +- pkg/github/projects_resolver_test.go | 54 +- pkg/github/projects_test.go | 12 + pkg/github/projects_v2_test.go | 1042 ++++++++++++++++++ pkg/github/toolset_instructions.go | 2 + 11 files changed, 1728 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index f3114fb900..aa5a0f56f0 100644 --- a/README.md +++ b/README.md @@ -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`: Field names for table or board creation; mutually exclusive with visible_fields. (string[], optional) + - `visible_fields`: Field database IDs for table or board creation; mutually exclusive with visible_field_names. (string[], optional) diff --git a/pkg/github/__toolsnaps__/projects_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..ba19e40315 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": "Field names for table or board creation; mutually exclusive with visible_fields.", + "items": { + "type": "string" + }, + "type": "array" + }, + "visible_fields": { + "description": "Field database IDs for table or board creation; mutually exclusive with visible_field_names.", + "items": { + "type": "string" + }, + "type": "array" } }, "required": [ diff --git a/pkg/github/minimal_types.go b/pkg/github/minimal_types.go index 15993f35a0..de6799a289 100644 --- a/pkg/github/minimal_types.go +++ b/pkg/github/minimal_types.go @@ -433,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,omitempty"` +} + type MinimalProjectItem struct { ID int64 `json:"id"` NodeID string `json:"node_id,omitempty"` diff --git a/pkg/github/projects.go b/pkg/github/projects.go index 4ceb432e69..3f7c5f075e 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,89 @@ type statusUpdateNodeQuery struct { } `graphql:"node(id: $id)"` } +type projectViewNode struct { + ID githubv4.ID + Number githubv4.Int + Name githubv4.String + Layout githubv4.ProjectV2ViewLayout + Filter *githubv4.String +} + +type projectViewNodeWithProject struct { + projectViewNode + Project projectVisibility +} + +type projectViewConnection struct { + Nodes []projectViewNode + PageInfo PageInfoFragment +} + +type projectViewsProject struct { + ID githubv4.ID + Public githubv4.Boolean + Views projectViewConnection `graphql:"views(first: $first, after: $after, last: $last, before: $before)"` +} + +type projectViewsUserQuery struct { + User struct { + ProjectV2 projectViewsProject `graphql:"projectV2(number: $projectNumber)"` + } `graphql:"user(login: $owner)"` +} + +type projectViewsOrgQuery struct { + Organization struct { + ProjectV2 projectViewsProject `graphql:"projectV2(number: $projectNumber)"` + } `graphql:"organization(login: $owner)"` +} + +type projectViewNodeQuery struct { + Node struct { + ProjectView projectViewNodeWithProject `graphql:"... on ProjectV2View"` + } `graphql:"node(id: $id)"` +} + +type projectViewParentQuery struct { + Node struct { + ProjectView struct { + ID githubv4.ID + Project struct { + ID githubv4.ID + } + } `graphql:"... on ProjectV2View"` + } `graphql:"node(id: $id)"` +} + +// CreateProjectV2ViewRequest is the REST request for creating a project view. +type CreateProjectV2ViewRequest struct { + Name string `json:"name"` + Layout string `json:"layout"` + Filter *string `json:"filter,omitempty"` + VisibleFields []int64 `json:"visible_fields,omitempty"` +} + +type projectV2ViewRESTResponse struct { + NodeID string `json:"node_id"` + Number int `json:"number"` + Name string `json:"name"` + Layout string `json:"layout"` + Filter *string `json:"filter,omitempty"` + VisibleFields []int64 `json:"visible_fields,omitempty"` +} + +// UpdateProjectV2ViewInput is the GraphQL input for updating a project view. +type UpdateProjectV2ViewInput struct { + ViewID githubv4.ID `json:"viewId"` + Name *githubv4.String `json:"name,omitempty"` + Layout *githubv4.ProjectV2ViewLayout `json:"layout,omitempty"` + Filter *githubv4.String `json:"filter,omitempty"` +} + +// DeleteProjectV2ViewInput is the GraphQL input for deleting a project view. +type DeleteProjectV2ViewInput struct { + ViewID githubv4.ID `json:"viewId"` +} + // CreateProjectV2StatusUpdateInput is the input for the createProjectV2StatusUpdate mutation. // Defined locally because the shurcooL/githubv4 library does not include this type. type CreateProjectV2StatusUpdateInput struct { @@ -161,7 +255,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 +272,7 @@ Use this tool to list projects for a user or organization, or list project field projectsMethodListProjectFields, projectsMethodListProjectItems, projectsMethodListProjectStatusUpdates, + projectsMethodListProjectViews, }, }, "owner_type": { @@ -191,7 +286,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 +349,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 +393,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 +419,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 +436,7 @@ Use this tool to get details about individual projects, project fields, and proj projectsMethodGetProjectField, projectsMethodGetProjectItem, projectsMethodGetProjectStatusUpdate, + projectsMethodGetProjectView, }, }, "owner_type": { @@ -374,6 +478,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 +493,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 +507,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 +588,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 +697,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 +715,9 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { projectsMethodUpdateProjectItems, projectsMethodDeleteProjectItem, projectsMethodCreateProjectStatusUpdate, + projectsMethodCreateProjectView, + projectsMethodUpdateProjectView, + projectsMethodDeleteProjectView, projectsMethodCreateProject, projectsMethodCreateIterationField, }, @@ -615,6 +739,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: "Field database IDs for table or board creation; mutually exclusive with visible_field_names.", + Items: &jsonschema.Schema{ + Type: "string", + }, + }, + "visible_field_names": { + Type: "array", + Description: "Field names for table or board creation; mutually exclusive with visible_fields.", + Items: &jsonschema.Schema{ + Type: "string", + }, + }, "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 +873,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 +900,26 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { } } + if method == projectsMethodCreateProjectView { + visibleFieldNames, namesErr := OptionalStringArrayParam(args, "visible_field_names") + if namesErr != nil { + return utils.NewToolResultError(namesErr.Error()), nil, nil + } + var gqlClient *githubv4.Client + if len(visibleFieldNames) > 0 { + gqlClient, err = deps.GetGQLClient(ctx) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + } + return createProjectView(ctx, client, gqlClient, args, owner, ownerType, projectNumber, visibleFieldNames) + } + + gqlClient, err := deps.GetGQLClient(ctx) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + switch method { case projectsMethodAddProjectItem: itemType, err := RequiredParam[string](args, "item_type") @@ -833,6 +1010,10 @@ 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 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 +1228,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 +1859,352 @@ func getProjectStatusUpdate(ctx context.Context, gqlClient *githubv4.Client, sta return utils.NewToolResultText(string(r)), isPrivate, nil, nil } +func convertToMinimalProjectView(node projectViewNode) MinimalProjectView { + return MinimalProjectView{ + ID: fmt.Sprintf("%v", node.ID), + Number: int(node.Number), + Name: string(node.Name), + Layout: projectViewLayoutName(node.Layout), + Filter: derefString(node.Filter), + } +} + +func projectViewLayoutName(layout githubv4.ProjectV2ViewLayout) string { + switch layout { + case githubv4.ProjectV2ViewLayoutTableLayout: + return "table" + case githubv4.ProjectV2ViewLayoutBoardLayout: + return "board" + case githubv4.ProjectV2ViewLayoutRoadmapLayout: + return "roadmap" + default: + return strings.ToLower(strings.TrimSuffix(string(layout), "_LAYOUT")) + } +} + +func parseProjectViewLayout(layout string) (githubv4.ProjectV2ViewLayout, error) { + switch strings.ToLower(strings.TrimSpace(layout)) { + case "table": + return githubv4.ProjectV2ViewLayoutTableLayout, nil + case "board": + return githubv4.ProjectV2ViewLayoutBoardLayout, nil + case "roadmap": + return githubv4.ProjectV2ViewLayoutRoadmapLayout, nil + default: + return "", fmt.Errorf("invalid layout %q: must be \"table\", \"board\", or \"roadmap\"", layout) + } +} + +func listProjectViews(ctx context.Context, gqlClient *githubv4.Client, args map[string]any, owner, ownerType string) (*mcp.CallToolResult, bool, any, error) { + if ownerType != "user" && ownerType != "org" { + return utils.NewToolResultError(fmt.Sprintf("invalid owner_type %q: must be \"user\" or \"org\"", ownerType)), false, nil, nil + } + + projectNumber, err := RequiredInt(args, "project_number") + if err != nil { + return utils.NewToolResultError(err.Error()), false, nil, nil + } + perPage, err := OptionalIntParamWithDefault(args, "per_page", MaxProjectsPerPage) + if err != nil { + return utils.NewToolResultError(err.Error()), false, nil, nil + } + if perPage < 1 || perPage > MaxProjectsPerPage { + perPage = MaxProjectsPerPage + } + after, err := OptionalParam[string](args, "after") + if err != nil { + return utils.NewToolResultError(err.Error()), false, nil, nil + } + before, err := OptionalParam[string](args, "before") + if err != nil { + return utils.NewToolResultError(err.Error()), false, nil, nil + } + if after != "" && before != "" { + return utils.NewToolResultError("provide either 'after' or 'before', not both"), false, nil, nil + } + + vars := map[string]any{ + "owner": githubv4.String(owner), + "projectNumber": githubv4.Int(int32(projectNumber)), //nolint:gosec // Project numbers are small integers + "first": (*githubv4.Int)(nil), + "after": (*githubv4.String)(nil), + "last": (*githubv4.Int)(nil), + "before": (*githubv4.String)(nil), + } + if before != "" { + last := githubv4.Int(int32(perPage)) //nolint:gosec // perPage is bounded by MaxProjectsPerPage + cursor := githubv4.String(before) + vars["last"] = &last + vars["before"] = &cursor + } else { + first := githubv4.Int(int32(perPage)) //nolint:gosec // perPage is bounded by MaxProjectsPerPage + vars["first"] = &first + if after != "" { + cursor := githubv4.String(after) + vars["after"] = &cursor + } + } + + var project projectViewsProject + if ownerType == "org" { + var query projectViewsOrgQuery + if err := gqlClient.Query(ctx, &query, vars); err != nil { + return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewListFailedError, err)), false, nil, nil + } + project = query.Organization.ProjectV2 + } else { + var query projectViewsUserQuery + if err := gqlClient.Query(ctx, &query, vars); err != nil { + return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewListFailedError, err)), false, nil, nil + } + project = query.User.ProjectV2 + } + if project.ID == nil || project.ID == "" { + return utils.NewToolResultError(fmt.Sprintf("%s: project was not found", ProjectViewListFailedError)), false, nil, nil + } + + views := make([]MinimalProjectView, 0, len(project.Views.Nodes)) + for _, node := range project.Views.Nodes { + views = append(views, convertToMinimalProjectView(node)) + } + response := map[string]any{ + "views": views, + "pageInfo": map[string]any{ + "hasNextPage": project.Views.PageInfo.HasNextPage, + "hasPreviousPage": project.Views.PageInfo.HasPreviousPage, + "nextCursor": string(project.Views.PageInfo.EndCursor), + "prevCursor": string(project.Views.PageInfo.StartCursor), + }, + } + result, err := json.Marshal(response) + if err != nil { + return nil, false, nil, fmt.Errorf("failed to marshal response: %w", err) + } + return utils.NewToolResultText(string(result)), !bool(project.Public), nil, nil +} + +func getProjectView(ctx context.Context, gqlClient *githubv4.Client, viewID string) (*mcp.CallToolResult, bool, any, error) { + var query projectViewNodeQuery + vars := map[string]any{"id": githubv4.ID(viewID)} + if err := gqlClient.Query(ctx, &query, vars); err != nil { + return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewGetFailedError, err)), false, nil, nil + } + if query.Node.ProjectView.ID == nil || query.Node.ProjectView.ID == "" { + return utils.NewToolResultError(fmt.Sprintf("%s: node is not a ProjectV2View or was not found", ProjectViewGetFailedError)), false, nil, nil + } + + view := convertToMinimalProjectView(query.Node.ProjectView.projectViewNode) + result, err := json.Marshal(view) + if err != nil { + return nil, false, nil, fmt.Errorf("failed to marshal response: %w", err) + } + return utils.NewToolResultText(string(result)), !bool(query.Node.ProjectView.Project.Public), nil, nil +} + +func createProjectView(ctx context.Context, client *github.Client, gqlClient *githubv4.Client, args map[string]any, owner, ownerType string, projectNumber int, visibleFieldNames []string) (*mcp.CallToolResult, any, error) { + name, err := RequiredParam[string](args, "name") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + 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 + } + visibleFields, err := OptionalBigIntArrayParam(args, "visible_fields") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if len(visibleFields) > 0 && len(visibleFieldNames) > 0 { + return utils.NewToolResultError("provide either 'visible_fields' or 'visible_field_names', not both"), nil, nil + } + if len(visibleFieldNames) > 0 { + resolvedIDs, resolveErr := resolveFieldNamesToIDs(ctx, gqlClient, owner, ownerType, projectNumber, visibleFieldNames, "visible_fields") + if resolveErr != nil { + var structured *ghErrors.StructuredResolutionError + if errors.As(resolveErr, &structured) { + return ghErrors.NewStructuredResolutionErrorResponse(structured), nil, nil + } + return utils.NewToolResultError(resolveErr.Error()), nil, nil + } + visibleFields = resolvedIDs + } + if layout == githubv4.ProjectV2ViewLayoutRoadmapLayout && len(visibleFields) > 0 { + return utils.NewToolResultError("visible fields are not supported for roadmap views"), nil, nil + } + + requestBody := CreateProjectV2ViewRequest{ + Name: name, + Layout: projectViewLayoutName(layout), + VisibleFields: visibleFields, + } + if hasFilter { + // The API clears a filter with an empty string, so a null filter is sent as "". + value := "" + if filter != nil { + value = *filter + } + requestBody.Filter = &value + } + + var endpoint string + switch ownerType { + case "org": + endpoint = fmt.Sprintf("orgs/%s/projectsV2/%d/views", owner, projectNumber) + case "user": + endpoint = fmt.Sprintf("users/%s/projectsV2/%d/views", owner, projectNumber) + default: + return utils.NewToolResultError(fmt.Sprintf("invalid owner_type %q: must be \"user\" or \"org\"", ownerType)), nil, nil + } + + req, err := client.NewRequest(ctx, http.MethodPost, endpoint, requestBody) + if err != nil { + return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewCreateFailedError, err)), nil, nil + } + var response projectV2ViewRESTResponse + resp, err := client.Do(req, &response) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, ProjectViewCreateFailedError, resp, err), nil, nil + } + if response.NodeID == "" { + return utils.NewToolResultError(fmt.Sprintf("%s: response did not include a project view node ID", ProjectViewCreateFailedError)), nil, nil + } + + filterValue := "" + if response.Filter != nil { + filterValue = *response.Filter + } + view := MinimalProjectView{ + ID: response.NodeID, + Number: response.Number, + Name: response.Name, + Layout: projectViewLayoutName(githubv4.ProjectV2ViewLayout(response.Layout)), + Filter: filterValue, + VisibleFields: response.VisibleFields, + } + return MarshalledTextResult(view), nil, nil +} + +func verifyProjectViewParent(ctx context.Context, gqlClient *githubv4.Client, viewID, owner, ownerType string, projectNumber int) error { + expectedProjectID, err := resolveProjectNodeID(ctx, gqlClient, owner, ownerType, projectNumber) + if err != nil { + return fmt.Errorf("failed to resolve requested project: %w", err) + } + if expectedProjectID == nil || expectedProjectID == "" { + return fmt.Errorf("requested project was not found") + } + + var query projectViewParentQuery + if err := gqlClient.Query(ctx, &query, map[string]any{"id": githubv4.ID(viewID)}); err != nil { + return fmt.Errorf("failed to resolve project view: %w", err) + } + if query.Node.ProjectView.ID == nil || query.Node.ProjectView.ID == "" { + return fmt.Errorf("node is not a ProjectV2View or was not found") + } + if query.Node.ProjectView.Project.ID != expectedProjectID { + return fmt.Errorf("project view does not belong to the requested project") + } + return nil +} + +func updateProjectView(ctx context.Context, gqlClient *githubv4.Client, args map[string]any, owner, ownerType string, projectNumber int) (*mcp.CallToolResult, any, error) { + viewID, err := RequiredParam[string](args, "view_id") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + 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 + } + if !hasName && !hasLayout && !hasFilter { + return utils.NewToolResultError("update_project_view requires at least one of name, layout, or filter"), nil, nil + } + if hasName && strings.TrimSpace(name) == "" { + return utils.NewToolResultError("name must not be empty"), nil, nil + } + + input := UpdateProjectV2ViewInput{ViewID: githubv4.ID(viewID)} + if hasName { + value := githubv4.String(name) + input.Name = &value + } + if hasLayout { + layout, err := parseProjectViewLayout(layoutName) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + input.Layout = &layout + } + if hasFilter { + // 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 + } + if err := verifyProjectViewParent(ctx, gqlClient, viewID, owner, ownerType, projectNumber); err != nil { + return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewUpdateFailedError, err)), nil, nil + } + + var mutation struct { + UpdateProjectV2View struct { + ProjectV2View projectViewNode `graphql:"projectV2View"` + } `graphql:"updateProjectV2View(input: $input)"` + } + if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil { + return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewUpdateFailedError, err)), nil, nil + } + if mutation.UpdateProjectV2View.ProjectV2View.ID == nil || mutation.UpdateProjectV2View.ProjectV2View.ID == "" { + return utils.NewToolResultError(fmt.Sprintf("%s: response did not include a project view", ProjectViewUpdateFailedError)), nil, nil + } + return MarshalledTextResult(convertToMinimalProjectView(mutation.UpdateProjectV2View.ProjectV2View)), nil, nil +} + +func deleteProjectView(ctx context.Context, gqlClient *githubv4.Client, args map[string]any, owner, ownerType string, projectNumber int) (*mcp.CallToolResult, any, error) { + viewID, err := RequiredParam[string](args, "view_id") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if err := verifyProjectViewParent(ctx, gqlClient, viewID, owner, ownerType, projectNumber); err != nil { + return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewDeleteFailedError, err)), nil, nil + } + input := DeleteProjectV2ViewInput{ViewID: githubv4.ID(viewID)} + var mutation struct { + DeleteProjectV2View struct { + ProjectV2View struct { + ID githubv4.ID + } `graphql:"projectV2View"` + } `graphql:"deleteProjectV2View(input: $input)"` + } + if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil { + return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewDeleteFailedError, err)), nil, nil + } + if id := mutation.DeleteProjectV2View.ProjectV2View.ID; id == nil || id == "" { + return utils.NewToolResultError(fmt.Sprintf("%s: response did not include the deleted view", ProjectViewDeleteFailedError)), nil, nil + } + deletedID := fmt.Sprintf("%v", mutation.DeleteProjectV2View.ProjectV2View.ID) + return MarshalledTextResult(map[string]string{"deleted_view_id": deletedID}), nil, nil +} + // validateAndConvertToInt64 ensures the value is a number and converts it to int64. func validateAndConvertToInt64(value any) (int64, error) { switch v := value.(type) { diff --git a/pkg/github/projects_resolver.go b/pkg/github/projects_resolver.go index 1c9ba9fdcb..537cdec394 100644 --- a/pkg/github/projects_resolver.go +++ b/pkg/github/projects_resolver.go @@ -530,7 +530,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,6 +540,10 @@ func resolveFieldNamesToIDs(ctx context.Context, gqlClient *githubv4.Client, own return nil, err } + return resolveFieldNamesToIDsFromFields(all, names, owner, projectNumber, idParameter) +} + +func resolveFieldNamesToIDsFromFields(all []ResolvedField, names []string, owner string, projectNumber int, idParameter string) ([]int64, error) { // 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)) @@ -566,7 +570,7 @@ func resolveFieldNamesToIDs(ctx context.Context, gqlClient *githubv4.Client, own 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) + return nil, fmt.Errorf("resolved field %q has non-numeric ID %q; pass it via '%s' instead", name, matches[0].ID, idParameter) } out = append(out, id) default: @@ -577,7 +581,7 @@ func resolveFieldNamesToIDs(ctx context.Context, gqlClient *githubv4.Client, own return nil, ghErrors.NewStructuredResolutionError( "field_ambiguous", name, - "multiple fields share this name; pass numeric IDs via 'fields' to disambiguate", + fmt.Sprintf("multiple fields share this name; pass numeric IDs via '%s' to disambiguate", idParameter), candidates, ) } diff --git a/pkg/github/projects_resolver_test.go b/pkg/github/projects_resolver_test.go index 8b11690abe..1c526286ce 100644 --- a/pkg/github/projects_resolver_test.go +++ b/pkg/github/projects_resolver_test.go @@ -236,7 +236,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)) @@ -734,7 +734,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 +781,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..aa9c587100 100644 --- a/pkg/github/projects_v2_test.go +++ b/pkg/github/projects_v2_test.go @@ -165,6 +165,83 @@ func resolveProjectNodeIDOrgMatcher(owner string, projectNumber int, nodeID stri ) } +func resolveProjectNodeIDUserMatcher(owner string, projectNumber int, nodeID string) githubv4mock.Matcher { + return githubv4mock.NewQueryMatcher( + struct { + User struct { + ProjectV2 struct { + ID githubv4.ID + } `graphql:"projectV2(number: $projectNumber)"` + } `graphql:"user(login: $owner)"` + }{}, + map[string]any{ + "owner": githubv4.String(owner), + "projectNumber": githubv4.Int(int32(projectNumber)), //nolint:gosec // test constant + }, + githubv4mock.DataResponse(map[string]any{ + "user": map[string]any{ + "projectV2": map[string]any{ + "id": nodeID, + }, + }, + }), + ) +} + +func projectViewParentMatcher(viewID, projectID string) githubv4mock.Matcher { + return githubv4mock.NewQueryMatcher( + projectViewParentQuery{}, + map[string]any{"id": githubv4.ID(viewID)}, + githubv4mock.DataResponse(map[string]any{ + "node": map[string]any{ + "id": viewID, + "project": map[string]any{"id": projectID}, + }, + }), + ) +} + +func projectViewParentErrorMatcher(viewID, message string) githubv4mock.Matcher { + return githubv4mock.NewQueryMatcher( + projectViewParentQuery{}, + map[string]any{"id": githubv4.ID(viewID)}, + githubv4mock.ErrorResponse(message), + ) +} + +func projectFieldNamesMatcher(owner, ownerType string, projectNumber int, nodes []map[string]any) githubv4mock.Matcher { + var response map[string]any + if ownerType == "org" { + response = fieldsResponse(nodes) + return githubv4mock.NewQueryMatcher( + projectFieldsQueryOrg{}, + fieldsQueryVars(owner, projectNumber), + githubv4mock.DataResponse(response), + ) + } + + response = map[string]any{ + "user": map[string]any{ + "projectV2": map[string]any{ + "fields": map[string]any{ + "nodes": nodes, + "pageInfo": map[string]any{ + "hasNextPage": false, + "hasPreviousPage": false, + "startCursor": "", + "endCursor": "", + }, + }, + }, + }, + } + return githubv4mock.NewQueryMatcher( + projectFieldsQueryUser{}, + fieldsQueryVars(owner, projectNumber), + githubv4mock.DataResponse(response), + ) +} + func createFieldMatcher() githubv4mock.Matcher { return githubv4mock.NewMutationMatcher( struct { @@ -455,3 +532,968 @@ func Test_ProjectsWrite_CreateIterationField(t *testing.T) { assert.Equal(t, "PVTIF_field1", response["id"]) }) } + +func Test_ProjectsList_ListProjectViews(t *testing.T) { + toolDef := ProjectsList(translations.NullTranslationHelper) + + t.Run("lists organization views with forward pagination and IFC", func(t *testing.T) { + first := githubv4.Int(2) + after := githubv4.String("after-cursor") + matcher := githubv4mock.NewQueryMatcher( + projectViewsOrgQuery{}, + map[string]any{ + "owner": githubv4.String("octo-org"), + "projectNumber": githubv4.Int(7), + "first": &first, + "after": &after, + "last": (*githubv4.Int)(nil), + "before": (*githubv4.String)(nil), + }, + githubv4mock.DataResponse(map[string]any{ + "organization": map[string]any{ + "projectV2": map[string]any{ + "id": "PVT_project7", + "public": false, + "views": map[string]any{ + "nodes": []map[string]any{ + { + "id": "PVTV_view1", + "number": 1, + "name": "Ready work", + "layout": "TABLE_LAYOUT", + "filter": "status:Ready", + }, + }, + "pageInfo": map[string]any{ + "hasNextPage": true, + "hasPreviousPage": false, + "startCursor": "start-cursor", + "endCursor": "end-cursor", + }, + }, + }, + }, + }), + ) + matcher.Variables["first"] = first + matcher.Variables["after"] = after + gqlClient := githubv4mock.NewMockedHTTPClient( + matcher, + ) + deps := BaseDeps{ + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), + GQLClient: githubv4.NewClient(gqlClient), + featureChecker: featureCheckerFor(FeatureFlagIFCLabels), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "list_project_views", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(7), + "per_page": float64(2), + "after": "after-cursor", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError, getTextResult(t, result).Text) + + var response struct { + Views []MinimalProjectView `json:"views"` + PageInfo map[string]any `json:"pageInfo"` + } + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &response)) + require.Len(t, response.Views, 1) + assert.Equal(t, MinimalProjectView{ + ID: "PVTV_view1", + Number: 1, + Name: "Ready work", + Layout: "table", + Filter: "status:Ready", + }, response.Views[0]) + assert.Equal(t, "end-cursor", response.PageInfo["nextCursor"]) + require.NotNil(t, result.Meta) + ifcMap := unmarshalIFC(t, result.Meta["ifc"]) + assert.Equal(t, "untrusted", ifcMap["integrity"]) + assert.Equal(t, "private", ifcMap["confidentiality"]) + }) + + t.Run("lists user views with backward pagination", func(t *testing.T) { + last := githubv4.Int(3) + before := githubv4.String("before-cursor") + matcher := githubv4mock.NewQueryMatcher( + projectViewsUserQuery{}, + map[string]any{ + "owner": githubv4.String("octocat"), + "projectNumber": githubv4.Int(8), + "first": (*githubv4.Int)(nil), + "after": (*githubv4.String)(nil), + "last": &last, + "before": &before, + }, + githubv4mock.DataResponse(map[string]any{ + "user": map[string]any{ + "projectV2": map[string]any{ + "id": "PVT_project8", + "public": true, + "views": map[string]any{ + "nodes": []map[string]any{}, + "pageInfo": map[string]any{ + "hasNextPage": false, + "hasPreviousPage": true, + "startCursor": "previous-cursor", + "endCursor": "", + }, + }, + }, + }, + }), + ) + matcher.Variables["last"] = last + matcher.Variables["before"] = before + gqlClient := githubv4mock.NewMockedHTTPClient( + matcher, + ) + deps := BaseDeps{ + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), + GQLClient: githubv4.NewClient(gqlClient), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "list_project_views", + "owner": "octocat", + "owner_type": "user", + "project_number": float64(8), + "per_page": float64(3), + "before": "before-cursor", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError, getTextResult(t, result).Text) + + var response map[string]any + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &response)) + pageInfo := response["pageInfo"].(map[string]any) + assert.Equal(t, "previous-cursor", pageInfo["prevCursor"]) + }) + + t.Run("rejects conflicting cursors", func(t *testing.T) { + deps := BaseDeps{ + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), + GQLClient: githubv4.NewClient(githubv4mock.NewMockedHTTPClient()), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "list_project_views", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(7), + "after": "a", + "before": "b", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "provide either 'after' or 'before'") + }) +} + +func Test_ProjectsGet_GetProjectView(t *testing.T) { + toolDef := ProjectsGet(translations.NullTranslationHelper) + + t.Run("gets a private project view by node ID", func(t *testing.T) { + gqlClient := githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + projectViewNodeQuery{}, + map[string]any{"id": githubv4.ID("PVTV_view1")}, + githubv4mock.DataResponse(map[string]any{ + "node": map[string]any{ + "id": "PVTV_view1", + "number": 1, + "name": "Ready work", + "layout": "BOARD_LAYOUT", + "filter": "status:Ready", + "project": map[string]any{"public": false}, + }, + }), + ), + ) + deps := BaseDeps{ + GQLClient: githubv4.NewClient(gqlClient), + featureChecker: featureCheckerFor(FeatureFlagIFCLabels), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "get_project_view", + "view_id": "PVTV_view1", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + + var view MinimalProjectView + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &view)) + assert.Equal(t, "PVTV_view1", view.ID) + assert.Equal(t, "board", view.Layout) + require.NotNil(t, result.Meta) + ifcMap := unmarshalIFC(t, result.Meta["ifc"]) + assert.Equal(t, "private", ifcMap["confidentiality"]) + }) + + t.Run("rejects a missing or wrong node type", func(t *testing.T) { + gqlClient := githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + projectViewNodeQuery{}, + map[string]any{"id": githubv4.ID("I_issue1")}, + githubv4mock.DataResponse(map[string]any{"node": map[string]any{}}), + ), + ) + deps := BaseDeps{GQLClient: githubv4.NewClient(gqlClient)} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "get_project_view", + "view_id": "I_issue1", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "node is not a ProjectV2View or was not found") + }) +} + +func Test_ProjectsWrite_CreateProjectView(t *testing.T) { + toolDef := ProjectsWrite(translations.NullTranslationHelper) + + t.Run("creates organization view with filter and visible fields", func(t *testing.T) { + restClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "POST /orgs/{org}/projectsV2/{project}/views": func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, "/orgs/octo-org/projectsV2/7/views", r.URL.Path) + var body map[string]any + require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) + assert.Equal(t, "Ready work", body["name"]) + assert.Equal(t, "table", body["layout"]) + assert.Equal(t, "status:Ready", body["filter"]) + assert.Equal(t, []any{float64(101), float64(202)}, body["visible_fields"]) + mockResponse(t, http.StatusCreated, map[string]any{ + "node_id": "PVTV_view1", + "number": 1, + "name": "Ready work", + "layout": "table", + "filter": "status:Ready", + "visible_fields": []int64{101, 202}, + })(w, r) + }, + }) + deps := BaseDeps{Client: mustNewGHClient(t, restClient)} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "create_project_view", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(7), + "name": "Ready work", + "layout": "table", + "filter": "status:Ready", + "visible_fields": []any{"101", "202"}, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + + var view MinimalProjectView + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &view)) + assert.Equal(t, "PVTV_view1", view.ID) + assert.Equal(t, []int64{101, 202}, view.VisibleFields) + }) + + t.Run("resolves organization visible field names in caller order", func(t *testing.T) { + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient( + projectFieldNamesMatcher("octo-org", "org", 7, []map[string]any{ + statusFieldNode("PVTSSF_status", 101, "Status", nil), + statusFieldNode("PVTSSF_priority", 202, "Priority", nil), + }), + )) + restClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "POST /orgs/{org}/projectsV2/{project}/views": func(w http.ResponseWriter, r *http.Request) { + var body map[string]any + require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) + assert.Equal(t, []any{float64(202), float64(101)}, body["visible_fields"]) + mockResponse(t, http.StatusCreated, map[string]any{ + "node_id": "PVTV_named_org", + "number": 2, + "name": "Named fields", + "layout": "table", + "visible_fields": []int64{202, 101}, + })(w, r) + }, + }) + deps := BaseDeps{ + Client: mustNewGHClient(t, restClient), + GQLClient: gqlClient, + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "create_project_view", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(7), + "name": "Named fields", + "layout": "table", + "visible_field_names": []any{"Priority", "status"}, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + }) + + t.Run("resolves user visible field names", func(t *testing.T) { + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient( + projectFieldNamesMatcher("octocat", "user", 8, []map[string]any{ + statusFieldNode("PVTSSF_status", 303, "Status", nil), + }), + )) + restClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "POST /users/{user_id}/projectsV2/{project}/views": func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, "/users/octocat/projectsV2/8/views", r.URL.Path) + var body map[string]any + require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) + assert.Equal(t, []any{float64(303)}, body["visible_fields"]) + mockResponse(t, http.StatusCreated, map[string]any{ + "node_id": "PVTV_named_user", + "number": 3, + "name": "User fields", + "layout": "board", + "visible_fields": []int64{303}, + })(w, r) + }, + }) + deps := BaseDeps{ + Client: mustNewGHClient(t, restClient), + GQLClient: gqlClient, + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "create_project_view", + "owner": "octocat", + "owner_type": "user", + "project_number": float64(8), + "name": "User fields", + "layout": "board", + "visible_field_names": []any{"Status"}, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + }) + + t.Run("creates a user view by login", func(t *testing.T) { + restClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + "POST /users/{user_id}/projectsV2/{project}/views": func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, "/users/octocat/projectsV2/8/views", r.URL.Path) + mockResponse(t, http.StatusCreated, map[string]any{ + "node_id": "PVTV_view2", + "number": 2, + "name": "Board", + "layout": "board", + })(w, r) + }, + }) + deps := BaseDeps{Client: mustNewGHClient(t, restClient)} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "create_project_view", + "owner": "octocat", + "owner_type": "user", + "project_number": float64(8), + "name": "Board", + "layout": "board", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, `"id":"PVTV_view2"`) + }) + + t.Run("auto-detects an organization owner", func(t *testing.T) { + restClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + GetUsersByUsername: mockResponse(t, http.StatusOK, map[string]any{ + "id": 99, + "type": "Organization", + }), + "POST /orgs/{org}/projectsV2/{project}/views": mockResponse(t, http.StatusCreated, map[string]any{ + "node_id": "PVTV_view3", + "number": 3, + "name": "Table", + "layout": "table", + }), + }) + deps := BaseDeps{Client: mustNewGHClient(t, restClient)} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "create_project_view", + "owner": "octo-org", + "project_number": float64(9), + "name": "Table", + "layout": "table", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + }) + + t.Run("rejects visible fields and names together", func(t *testing.T) { + deps := BaseDeps{ + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), + GQLClient: githubv4.NewClient(githubv4mock.NewMockedHTTPClient()), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "create_project_view", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(7), + "name": "Table", + "layout": "table", + "visible_fields": []any{"101"}, + "visible_field_names": []any{"Status"}, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "provide either 'visible_fields' or 'visible_field_names', not both") + }) + + for _, tc := range []struct { + name string + nodes []map[string]any + requestedName string + expectedError string + expectedHint string + }{ + { + name: "returns structured not-found errors", + nodes: []map[string]any{ + statusFieldNode("PVTSSF_status", 101, "Status", nil), + }, + requestedName: "Priority", + expectedError: "field_not_found", + }, + { + name: "returns structured ambiguous errors", + nodes: []map[string]any{ + statusFieldNode("PVTSSF_status1", 101, "Status", nil), + statusFieldNode("PVTSSF_status2", 202, "Status", nil), + }, + requestedName: "Status", + expectedError: "field_ambiguous", + expectedHint: "visible_fields", + }, + } { + t.Run(tc.name, func(t *testing.T) { + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient( + projectFieldNamesMatcher("octo-org", "org", 7, tc.nodes), + )) + deps := BaseDeps{ + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), + GQLClient: gqlClient, + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "create_project_view", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(7), + "name": "Table", + "layout": "table", + "visible_field_names": []any{tc.requestedName}, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + var response map[string]any + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &response)) + assert.Equal(t, tc.expectedError, response["error"]) + assert.Equal(t, tc.requestedName, response["name"]) + if tc.expectedHint != "" { + assert.Contains(t, response["hint"], tc.expectedHint) + assert.NotContains(t, response["hint"], "'fields'") + } + }) + } + + t.Run("rejects visible fields for roadmap layout", func(t *testing.T) { + deps := BaseDeps{Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}))} + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "create_project_view", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(7), + "name": "Roadmap", + "layout": "roadmap", + "visible_fields": []any{"101"}, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "visible fields are not supported for roadmap views") + }) + + t.Run("resolves visible field names before rejecting roadmap layout", func(t *testing.T) { + gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient( + projectFieldNamesMatcher("octo-org", "org", 7, []map[string]any{ + statusFieldNode("PVTSSF_status", 101, "Status", nil), + }), + )) + deps := BaseDeps{ + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), + GQLClient: gqlClient, + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "create_project_view", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(7), + "name": "Roadmap", + "layout": "roadmap", + "visible_field_names": []any{"Status"}, + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "visible fields are not supported for roadmap views") + }) +} + +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": map[string]any{ + "id": "PVTV_view1", + "number": 1, + "name": "Renamed", + "layout": "TABLE_LAYOUT", + "filter": "status:Ready", + }, + }, + }), + ), + ) + deps := BaseDeps{ + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), + GQLClient: githubv4.NewClient(gqlClient), + } + handler := toolDef.Handler(deps) + request := createMCPRequest(map[string]any{ + "method": "update_project_view", + "owner": "octo-org", + "owner_type": "org", + "project_number": float64(7), + "view_id": "PVTV_view1", + "name": "Renamed", + }) + + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + require.False(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, `"name":"Renamed"`) + }) + + t.Run("sends 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, or filter") + }) +} + +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/toolset_instructions.go b/pkg/github/toolset_instructions.go index ba6659612a..3b3a54eadd 100644 --- a/pkg/github/toolset_instructions.go +++ b/pkg/github/toolset_instructions.go @@ -41,6 +41,8 @@ Workflow: 1) list_project_fields (get field IDs), 2) list_project_items (with pa Project lifecycle: Use create_project to create a new ProjectsV2 for a user or organization (requires owner_type and title). Returns the new project's id, number, title, and url; pass the returned number as project_number to subsequent project tools. +Views: Use list_project_views and get_project_view to inspect views. Use create_project_view, update_project_view, and delete_project_view for basic name, layout, and filter management; visible_fields is create-only and unavailable for roadmap views. + Iteration fields: Use create_iteration_field to add a new ITERATION field (e.g. "Sprint") to an existing project. Required: field_name, iteration_duration (days), start_date (YYYY-MM-DD). Only pass the iterations array when iterations need varying durations, breaks between them, or specific titles; otherwise omit it and GitHub creates three default iterations of iteration_duration days starting on start_date. Status updates: Use list_project_status_updates to read recent project status updates (newest first). Use get_project_status_update with a node ID to get a single update. Use create_project_status_update to create a new status update for a project. From 2198e8599bbbcb98a0d6cd7cabe9a48629acdf29 Mon Sep 17 00:00:00 2001 From: Bryan Zwicker Date: Wed, 12 Aug 2026 09:40:51 -0400 Subject: [PATCH 4/7] Add visible fields to project views (#2988) * Add visible fields to project views Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1421a5d5-fdce-4c0e-9528-56d555ec30d4 * Fail fast and surface orphaned views on project view writes Reject roadmap layouts before enumerating project fields in both the create and update paths, and verify view ownership before resolving visible fields on update, so rejected requests no longer pay for a paginated field listing. Skip the follow-up filter mutation when the filter is explicitly null, since a new view has no filter to clear, and include the created view ID when cleanup after a failed filter mutation also fails so the caller can recover the orphaned view. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1421a5d5-fdce-4c0e-9528-56d555ec30d4 --- README.md | 4 +- pkg/github/__toolsnaps__/projects_write.snap | 4 +- pkg/github/minimal_types.go | 2 +- pkg/github/projects.go | 373 +++++---- pkg/github/projects_resolver.go | 152 ++-- pkg/github/projects_resolver_test.go | 17 + pkg/github/projects_v2_test.go | 769 +++++++++++++------ 7 files changed, 880 insertions(+), 441 deletions(-) diff --git a/README.md b/README.md index aa5a0f56f0..6585ab30f6 100644 --- a/README.md +++ b/README.md @@ -1141,8 +1141,8 @@ The following sets of tools are available: - `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`: Field names for table or board creation; mutually exclusive with visible_fields. (string[], optional) - - `visible_fields`: Field database IDs for table or board creation; mutually exclusive with visible_field_names. (string[], optional) + - `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_write.snap b/pkg/github/__toolsnaps__/projects_write.snap index ba19e40315..0ea38c2b0a 100644 --- a/pkg/github/__toolsnaps__/projects_write.snap +++ b/pkg/github/__toolsnaps__/projects_write.snap @@ -257,14 +257,14 @@ "type": "string" }, "visible_field_names": { - "description": "Field names for table or board creation; mutually exclusive with visible_fields.", + "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": "Field database IDs for table or board creation; mutually exclusive with visible_field_names.", + "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" }, diff --git a/pkg/github/minimal_types.go b/pkg/github/minimal_types.go index de6799a289..2424823c2c 100644 --- a/pkg/github/minimal_types.go +++ b/pkg/github/minimal_types.go @@ -439,7 +439,7 @@ type MinimalProjectView struct { Name string `json:"name"` Layout string `json:"layout"` Filter string `json:"filter"` - VisibleFields []int64 `json:"visible_fields,omitempty"` + VisibleFields []int64 `json:"visible_fields"` } type MinimalProjectItem struct { diff --git a/pkg/github/projects.go b/pkg/github/projects.go index 3f7c5f075e..dece52cd13 100644 --- a/pkg/github/projects.go +++ b/pkg/github/projects.go @@ -121,11 +121,35 @@ type statusUpdateNodeQuery struct { } type projectViewNode struct { - ID githubv4.ID - Number githubv4.Int - Name githubv4.String - Layout githubv4.ProjectV2ViewLayout - Filter *githubv4.String + 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 { @@ -166,6 +190,7 @@ type projectViewParentQuery struct { Node struct { ProjectView struct { ID githubv4.ID + Layout githubv4.ProjectV2ViewLayout Project struct { ID githubv4.ID } @@ -173,29 +198,38 @@ type projectViewParentQuery struct { } `graphql:"node(id: $id)"` } -// CreateProjectV2ViewRequest is the REST request for creating a project view. -type CreateProjectV2ViewRequest struct { - Name string `json:"name"` - Layout string `json:"layout"` - Filter *string `json:"filter,omitempty"` - VisibleFields []int64 `json:"visible_fields,omitempty"` +// ProjectV2ViewConfigurationInput is the GraphQL view configuration input. +type ProjectV2ViewConfigurationInput struct { + VisibleFieldIDs []githubv4.ID `json:"visibleFieldIds"` } -type projectV2ViewRESTResponse struct { - NodeID string `json:"node_id"` - Number int `json:"number"` - Name string `json:"name"` - Layout string `json:"layout"` - Filter *string `json:"filter,omitempty"` - VisibleFields []int64 `json:"visible_fields,omitempty"` +// 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"` + 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. @@ -761,14 +795,14 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { }, "visible_fields": { Type: "array", - Description: "Field database IDs for table or board creation; mutually exclusive with visible_field_names.", + 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: "Field names for table or board creation; mutually exclusive with visible_fields.", + 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", }, @@ -900,21 +934,6 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { } } - if method == projectsMethodCreateProjectView { - visibleFieldNames, namesErr := OptionalStringArrayParam(args, "visible_field_names") - if namesErr != nil { - return utils.NewToolResultError(namesErr.Error()), nil, nil - } - var gqlClient *githubv4.Client - if len(visibleFieldNames) > 0 { - gqlClient, err = deps.GetGQLClient(ctx) - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } - } - return createProjectView(ctx, client, gqlClient, args, owner, ownerType, projectNumber, visibleFieldNames) - } - gqlClient, err := deps.GetGQLClient(ctx) if err != nil { return utils.NewToolResultError(err.Error()), nil, nil @@ -1010,6 +1029,8 @@ 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: @@ -1860,12 +1881,26 @@ func getProjectStatusUpdate(ctx context.Context, gqlClient *githubv4.Client, sta } 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), + ID: fmt.Sprintf("%v", node.ID), + Number: int(node.Number), + Name: string(node.Name), + Layout: projectViewLayoutName(node.Layout), + Filter: derefString(node.Filter), + VisibleFields: visibleFields, } } @@ -2001,7 +2036,81 @@ func getProjectView(ctx context.Context, gqlClient *githubv4.Client, viewID stri return utils.NewToolResultText(string(result)), !bool(query.Node.ProjectView.Project.Public), nil, nil } -func createProjectView(ctx context.Context, client *github.Client, gqlClient *githubv4.Client, args map[string]any, owner, ownerType string, projectNumber int, visibleFieldNames []string) (*mcp.CallToolResult, any, error) { +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 @@ -2021,100 +2130,80 @@ func createProjectView(ctx context.Context, client *github.Client, gqlClient *gi if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - visibleFields, err := OptionalBigIntArrayParam(args, "visible_fields") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } - if len(visibleFields) > 0 && len(visibleFieldNames) > 0 { - return utils.NewToolResultError("provide either 'visible_fields' or 'visible_field_names', not both"), nil, nil + if layout == githubv4.ProjectV2ViewLayoutRoadmapLayout && projectViewRequestsVisibleFields(args) { + return utils.NewToolResultError("visible fields are not supported for roadmap views"), nil, nil } - if len(visibleFieldNames) > 0 { - resolvedIDs, resolveErr := resolveFieldNamesToIDs(ctx, gqlClient, owner, ownerType, projectNumber, visibleFieldNames, "visible_fields") - if resolveErr != nil { - var structured *ghErrors.StructuredResolutionError - if errors.As(resolveErr, &structured) { - return ghErrors.NewStructuredResolutionErrorResponse(structured), nil, nil - } - return utils.NewToolResultError(resolveErr.Error()), nil, nil + 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 } - visibleFields = resolvedIDs - } - if layout == githubv4.ProjectV2ViewLayoutRoadmapLayout && len(visibleFields) > 0 { - return utils.NewToolResultError("visible fields are not supported for roadmap views"), nil, nil + return utils.NewToolResultError(err.Error()), nil, nil } - requestBody := CreateProjectV2ViewRequest{ - Name: name, - Layout: projectViewLayoutName(layout), - VisibleFields: visibleFields, + 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 hasFilter { - // The API clears a filter with an empty string, so a null filter is sent as "". - value := "" - if filter != nil { - value = *filter - } - requestBody.Filter = &value + if projectID == nil || projectID == "" { + return utils.NewToolResultError(fmt.Sprintf("%s: project was not found", ProjectViewCreateFailedError)), nil, nil } - var endpoint string - switch ownerType { - case "org": - endpoint = fmt.Sprintf("orgs/%s/projectsV2/%d/views", owner, projectNumber) - case "user": - endpoint = fmt.Sprintf("users/%s/projectsV2/%d/views", owner, projectNumber) - default: - return utils.NewToolResultError(fmt.Sprintf("invalid owner_type %q: must be \"user\" or \"org\"", ownerType)), nil, nil + input := CreateProjectV2ViewInput{ + ProjectID: projectID, + Name: githubv4.String(name), + Layout: layout, + Configuration: configuration, } - - req, err := client.NewRequest(ctx, http.MethodPost, endpoint, requestBody) - if err != nil { + var mutation createProjectV2ViewMutation + if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil { return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewCreateFailedError, err)), nil, nil } - var response projectV2ViewRESTResponse - resp, err := client.Do(req, &response) - if err != nil { - return ghErrors.NewGitHubAPIErrorResponse(ctx, ProjectViewCreateFailedError, resp, err), nil, nil - } - if response.NodeID == "" { - return utils.NewToolResultError(fmt.Sprintf("%s: response did not include a project view node ID", ProjectViewCreateFailedError)), nil, nil + 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 } - filterValue := "" - if response.Filter != nil { - filterValue = *response.Filter - } - view := MinimalProjectView{ - ID: response.NodeID, - Number: response.Number, - Name: response.Name, - Layout: projectViewLayoutName(githubv4.ProjectV2ViewLayout(response.Layout)), - Filter: filterValue, - VisibleFields: response.VisibleFields, + 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(view), nil, nil + return MarshalledTextResult(convertToMinimalProjectView(view)), nil, nil } -func verifyProjectViewParent(ctx context.Context, gqlClient *githubv4.Client, viewID, owner, ownerType string, projectNumber int) error { +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) + return "", fmt.Errorf("failed to resolve requested project: %w", err) } if expectedProjectID == nil || expectedProjectID == "" { - return fmt.Errorf("requested project was not found") + 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) + 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") + 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 "", fmt.Errorf("project view does not belong to the requested project") } - return nil + 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) { @@ -2134,8 +2223,10 @@ func updateProjectView(ctx context.Context, gqlClient *githubv4.Client, args map if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - if !hasName && !hasLayout && !hasFilter { - return utils.NewToolResultError("update_project_view requires at least one of name, layout, or filter"), nil, nil + _, 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 @@ -2161,15 +2252,29 @@ func updateProjectView(ctx context.Context, gqlClient *githubv4.Client, args map } input.Filter = &value } - if err := verifyProjectViewParent(ctx, gqlClient, viewID, owner, ownerType, projectNumber); err != nil { + 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 + } - var mutation struct { - UpdateProjectV2View struct { - ProjectV2View projectViewNode `graphql:"projectV2View"` - } `graphql:"updateProjectV2View(input: $input)"` + 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 } @@ -2179,15 +2284,8 @@ func updateProjectView(ctx context.Context, gqlClient *githubv4.Client, args map return MarshalledTextResult(convertToMinimalProjectView(mutation.UpdateProjectV2View.ProjectV2View)), nil, nil } -func deleteProjectView(ctx context.Context, gqlClient *githubv4.Client, args map[string]any, owner, ownerType string, projectNumber int) (*mcp.CallToolResult, any, error) { - viewID, err := RequiredParam[string](args, "view_id") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } - if err := verifyProjectViewParent(ctx, gqlClient, viewID, owner, ownerType, projectNumber); err != nil { - return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewDeleteFailedError, err)), nil, nil - } - input := DeleteProjectV2ViewInput{ViewID: githubv4.ID(viewID)} +func deleteProjectViewByID(ctx context.Context, gqlClient *githubv4.Client, viewID githubv4.ID) error { + input := DeleteProjectV2ViewInput{ViewID: viewID} var mutation struct { DeleteProjectV2View struct { ProjectV2View struct { @@ -2196,13 +2294,26 @@ func deleteProjectView(ctx context.Context, gqlClient *githubv4.Client, args map } `graphql:"deleteProjectV2View(input: $input)"` } if err := gqlClient.Mutate(ctx, &mutation, input, nil); err != nil { - return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewDeleteFailedError, err)), nil, nil + return err } if id := mutation.DeleteProjectV2View.ProjectV2View.ID; id == nil || id == "" { - return utils.NewToolResultError(fmt.Sprintf("%s: response did not include the deleted view", ProjectViewDeleteFailedError)), nil, nil + 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 } - deletedID := fmt.Sprintf("%v", mutation.DeleteProjectV2View.ProjectV2View.ID) - return MarshalledTextResult(map[string]string{"deleted_view_id": deletedID}), 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. diff --git a/pkg/github/projects_resolver.go b/pkg/github/projects_resolver.go index 537cdec394..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. @@ -544,47 +598,17 @@ func resolveFieldNamesToIDs(ctx context.Context, gqlClient *githubv4.Client, own } func resolveFieldNamesToIDsFromFields(all []ResolvedField, names []string, owner string, projectNumber int, idParameter string) ([]int64, error) { - // 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) + 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 '%s' instead", name, matches[0].ID, idParameter) - } - 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, - fmt.Sprintf("multiple fields share this name; pass numeric IDs via '%s' to disambiguate", idParameter), - 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 1c526286ce..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{ @@ -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 { diff --git a/pkg/github/projects_v2_test.go b/pkg/github/projects_v2_test.go index aa9c587100..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" @@ -195,6 +197,7 @@ func projectViewParentMatcher(viewID, projectID string) githubv4mock.Matcher { githubv4mock.DataResponse(map[string]any{ "node": map[string]any{ "id": viewID, + "layout": "TABLE_LAYOUT", "project": map[string]any{"id": projectID}, }, }), @@ -209,6 +212,24 @@ func projectViewParentErrorMatcher(viewID, message string) githubv4mock.Matcher ) } +// 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" { @@ -242,6 +263,23 @@ func projectFieldNamesMatcher(owner, ownerType string, projectNumber int, nodes ) } +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 { @@ -562,6 +600,11 @@ func Test_ProjectsList_ListProjectViews(t *testing.T) { "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{ @@ -606,11 +649,12 @@ func Test_ProjectsList_ListProjectViews(t *testing.T) { 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", + 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) @@ -717,6 +761,11 @@ func Test_ProjectsGet_GetProjectView(t *testing.T) { "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}}, + }, + }, }, }), ), @@ -739,6 +788,7 @@ func Test_ProjectsGet_GetProjectView(t *testing.T) { 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"]) @@ -768,307 +818,426 @@ func Test_ProjectsGet_GetProjectView(t *testing.T) { func Test_ProjectsWrite_CreateProjectView(t *testing.T) { toolDef := ProjectsWrite(translations.NullTranslationHelper) + emptyRESTClient := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})) - t.Run("creates organization view with filter and visible fields", func(t *testing.T) { - restClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - "POST /orgs/{org}/projectsV2/{project}/views": func(w http.ResponseWriter, r *http.Request) { - require.Equal(t, "/orgs/octo-org/projectsV2/7/views", r.URL.Path) - var body map[string]any - require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) - assert.Equal(t, "Ready work", body["name"]) - assert.Equal(t, "table", body["layout"]) - assert.Equal(t, "status:Ready", body["filter"]) - assert.Equal(t, []any{float64(101), float64(202)}, body["visible_fields"]) - mockResponse(t, http.StatusCreated, map[string]any{ - "node_id": "PVTV_view1", - "number": 1, - "name": "Ready work", - "layout": "table", - "filter": "status:Ready", - "visible_fields": []int64{101, 202}, - })(w, r) - }, - }) - deps := BaseDeps{Client: mustNewGHClient(t, restClient)} + 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_fields": []any{"101", "202"}, + "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) - + 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_view1", view.ID) - assert.Equal(t, []int64{101, 202}, view.VisibleFields) + assert.Equal(t, []int64{202, 101}, view.VisibleFields) + assert.Equal(t, "status:Ready", view.Filter) }) - t.Run("resolves organization visible field names in caller order", func(t *testing.T) { - gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient( - projectFieldNamesMatcher("octo-org", "org", 7, []map[string]any{ - statusFieldNode("PVTSSF_status", 101, "Status", nil), - statusFieldNode("PVTSSF_priority", 202, "Priority", nil), - }), - )) - restClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - "POST /orgs/{org}/projectsV2/{project}/views": func(w http.ResponseWriter, r *http.Request) { - var body map[string]any - require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) - assert.Equal(t, []any{float64(202), float64(101)}, body["visible_fields"]) - mockResponse(t, http.StatusCreated, map[string]any{ - "node_id": "PVTV_named_org", - "number": 2, - "name": "Named fields", - "layout": "table", - "visible_fields": []int64{202, 101}, - })(w, r) - }, - }) - deps := BaseDeps{ - Client: mustNewGHClient(t, restClient), - GQLClient: gqlClient, - } + 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": "octo-org", - "owner_type": "org", - "project_number": float64(7), - "name": "Named fields", - "layout": "table", - "visible_field_names": []any{"Priority", "status"}, + "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) + 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("resolves user visible field names", func(t *testing.T) { - gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient( - projectFieldNamesMatcher("octocat", "user", 8, []map[string]any{ - statusFieldNode("PVTSSF_status", 303, "Status", nil), - }), - )) - restClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - "POST /users/{user_id}/projectsV2/{project}/views": func(w http.ResponseWriter, r *http.Request) { - require.Equal(t, "/users/octocat/projectsV2/8/views", r.URL.Path) - var body map[string]any - require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) - assert.Equal(t, []any{float64(303)}, body["visible_fields"]) - mockResponse(t, http.StatusCreated, map[string]any{ - "node_id": "PVTV_named_user", - "number": 3, - "name": "User fields", - "layout": "board", - "visible_fields": []int64{303}, - })(w, r) - }, - }) - deps := BaseDeps{ - Client: mustNewGHClient(t, restClient), - GQLClient: gqlClient, - } + 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": "octocat", - "owner_type": "user", - "project_number": float64(8), - "name": "User fields", - "layout": "board", - "visible_field_names": []any{"Status"}, + "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.False(t, result.IsError) + 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("creates a user view by login", func(t *testing.T) { - restClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - "POST /users/{user_id}/projectsV2/{project}/views": func(w http.ResponseWriter, r *http.Request) { - require.Equal(t, "/users/octocat/projectsV2/8/views", r.URL.Path) - mockResponse(t, http.StatusCreated, map[string]any{ - "node_id": "PVTV_view2", - "number": 2, - "name": "Board", - "layout": "board", - })(w, r) - }, - }) - deps := BaseDeps{Client: mustNewGHClient(t, restClient)} + 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": "octocat", - "owner_type": "user", - "project_number": float64(8), - "name": "Board", - "layout": "board", + "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.False(t, result.IsError) - assert.Contains(t, getTextResult(t, result).Text, `"id":"PVTV_view2"`) + 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("auto-detects an organization owner", func(t *testing.T) { - restClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - GetUsersByUsername: mockResponse(t, http.StatusOK, map[string]any{ - "id": 99, - "type": "Organization", - }), - "POST /orgs/{org}/projectsV2/{project}/views": mockResponse(t, http.StatusCreated, map[string]any{ - "node_id": "PVTV_view3", - "number": 3, - "name": "Table", - "layout": "table", - }), - }) - deps := BaseDeps{Client: mustNewGHClient(t, restClient)} + 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", - "project_number": float64(9), - "name": "Table", + "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) + 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("rejects visible fields and names together", func(t *testing.T) { - deps := BaseDeps{ - Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), - GQLClient: githubv4.NewClient(githubv4mock.NewMockedHTTPClient()), - } + 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": "Table", - "layout": "table", - "visible_fields": []any{"101"}, - "visible_field_names": []any{"Status"}, + "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.True(t, result.IsError) - assert.Contains(t, getTextResult(t, result).Text, "provide either 'visible_fields' or 'visible_field_names', not both") + require.False(t, result.IsError, getTextResult(t, result).Text) + assert.Contains(t, getTextResult(t, result).Text, `"visible_fields":[101]`) }) - for _, tc := range []struct { - name string - nodes []map[string]any - requestedName string - expectedError string - expectedHint string - }{ - { - name: "returns structured not-found errors", - nodes: []map[string]any{ - statusFieldNode("PVTSSF_status", 101, "Status", nil), - }, - requestedName: "Priority", - expectedError: "field_not_found", - }, - { - name: "returns structured ambiguous errors", - nodes: []map[string]any{ - statusFieldNode("PVTSSF_status1", 101, "Status", nil), - statusFieldNode("PVTSSF_status2", 202, "Status", nil), - }, - requestedName: "Status", - expectedError: "field_ambiguous", - expectedHint: "visible_fields", - }, - } { - t.Run(tc.name, func(t *testing.T) { - gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient( - projectFieldNamesMatcher("octo-org", "org", 7, tc.nodes), - )) - deps := BaseDeps{ - Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), - GQLClient: gqlClient, - } - handler := toolDef.Handler(deps) - request := createMCPRequest(map[string]any{ - "method": "create_project_view", - "owner": "octo-org", - "owner_type": "org", - "project_number": float64(7), - "name": "Table", - "layout": "table", - "visible_field_names": []any{tc.requestedName}, - }) - - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.True(t, result.IsError) - var response map[string]any - require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &response)) - assert.Equal(t, tc.expectedError, response["error"]) - assert.Equal(t, tc.requestedName, response["name"]) - if tc.expectedHint != "" { - assert.Contains(t, response["hint"], tc.expectedHint) - assert.NotContains(t, response["hint"], "'fields'") - } + 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"}), }) - } - - t.Run("rejects visible fields for roadmap layout", func(t *testing.T) { - deps := BaseDeps{Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}))} + 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", - "owner_type": "org", - "project_number": float64(7), - "name": "Roadmap", - "layout": "roadmap", - "visible_fields": []any{"101"}, + "project_number": float64(9), + "name": "Table", + "layout": "table", }) 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") + require.False(t, result.IsError, getTextResult(t, result).Text) }) - t.Run("resolves visible field names before rejecting roadmap layout", func(t *testing.T) { - gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient( - projectFieldNamesMatcher("octo-org", "org", 7, []map[string]any{ - statusFieldNode("PVTSSF_status", 101, "Status", nil), - }), - )) - deps := BaseDeps{ - Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), - GQLClient: gqlClient, + 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": "Roadmap", + "name": "Timeline", "layout": "roadmap", "visible_field_names": []any{"Status"}, }) @@ -1077,6 +1246,7 @@ func Test_ProjectsWrite_CreateProjectView(t *testing.T) { 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") }) } @@ -1101,13 +1271,7 @@ func Test_ProjectsWrite_UpdateProjectView(t *testing.T) { nil, githubv4mock.DataResponse(map[string]any{ "updateProjectV2View": map[string]any{ - "projectV2View": map[string]any{ - "id": "PVTV_view1", - "number": 1, - "name": "Renamed", - "layout": "TABLE_LAYOUT", - "filter": "status:Ready", - }, + "projectV2View": projectViewResponse("PVTV_view1", 1, "Renamed", "TABLE_LAYOUT", "status:Ready", 101, 202), }, }), ), @@ -1130,6 +1294,129 @@ func Test_ProjectsWrite_UpdateProjectView(t *testing.T) { 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) { @@ -1380,7 +1667,7 @@ func Test_ProjectsWrite_UpdateProjectView(t *testing.T) { result, err := handler(ContextWithDeps(context.Background(), deps), &request) require.NoError(t, err) require.True(t, result.IsError) - assert.Contains(t, getTextResult(t, result).Text, "requires at least one of name, layout, or filter") + assert.Contains(t, getTextResult(t, result).Text, "requires at least one of name, layout, filter, visible_fields, or visible_field_names") }) } From accc2e0970795b2a5789a3c4c373fe7f5112cec2 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Fri, 14 Aug 2026 09:04:13 +0200 Subject: [PATCH 5/7] fix(actions): avoid malformed response on log download failure Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9cdeefb9-91cd-4f65-8eb2-089c9e00a2b8 --- pkg/github/actions.go | 8 ++++---- pkg/github/actions_test.go | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/pkg/github/actions.go b/pkg/github/actions.go index 85dd99e1aa..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" diff --git a/pkg/github/actions_test.go b/pkg/github/actions_test.go index a25a35f704..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" @@ -624,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) From 0c825b4233cb321c1b6b6271f49495bd2230a5a0 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Fri, 14 Aug 2026 13:10:55 +0200 Subject: [PATCH 6/7] fix(security): enforce HTTPS for gh-host/GITHUB_HOST to prevent cleartext credentials GHES hosts accepted an http:// scheme, which was interpolated into every REST/GraphQL/upload/raw/authorization URL. Authenticated requests would then carry the bearer token/PAT over cleartext http, exposing it to network interception and replay. Add a central HTTPS check in parseAPIHost so no deployment can build authenticated URLs over http, mirroring the existing GHEC behaviour. Permit http only for loopback hosts (localhost, 127.0.0.1, ::1) so local development against a dev server still works. Closes github/copilot-mcp-core#1815 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- README.md | 2 +- pkg/http/oauth/oauth_test.go | 9 +++++---- pkg/utils/api.go | 37 ++++++++++++++++++++++++++++++++++++ pkg/utils/api_test.go | 28 ++++++++++++++++++++++++++- 4 files changed, 70 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 6585ab30f6..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 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..090c1850a1 100644 --- a/pkg/utils/api.go +++ b/pkg/utils/api.go @@ -235,6 +235,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 +252,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..7aa762a9b1 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,38 @@ 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 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) From 0ea1f775a7c73eff1bd2e25904d01136756bbfe2 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Fri, 14 Aug 2026 13:22:26 +0200 Subject: [PATCH 7/7] fix: preserve authority for loopback GHES hosts Address review: the loopback exception accepted http://localhost:3000 and http://[::1], but newGHESHost built URLs from u.Hostname(), which drops the port (silently retargeting the dev server to port 80) and strips IPv6 brackets (producing an unusable URL such as http://::1/api/v3/). Derive the base-host REST/GraphQL/upload/raw/authorization URLs from u.Host so the port and IPv6 brackets are preserved. Subdomain-isolation URLs keep using the bare hostname, since a label cannot be prepended to a host:port or an IP literal. Add tests for the ::1 case and for port preservation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/utils/api.go | 18 +++++++++++++----- pkg/utils/api_test.go | 15 +++++++++++++++ 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/pkg/utils/api.go b/pkg/utils/api.go index 090c1850a1..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) } diff --git a/pkg/utils/api_test.go b/pkg/utils/api_test.go index 7aa762a9b1..baa1eb30ce 100644 --- a/pkg/utils/api_test.go +++ b/pkg/utils/api_test.go @@ -76,6 +76,21 @@ func TestParseAPIHost(t *testing.T) { 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",