Bryan Zwicker
2198e8599b
* 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
1064 lines
34 KiB
Go
1064 lines
34 KiB
Go
package github
|
|
|
|
import (
|
|
"context"
|
|
"encoding/json"
|
|
"net/http"
|
|
"testing"
|
|
|
|
"github.com/github/github-mcp-server/internal/githubv4mock"
|
|
"github.com/github/github-mcp-server/pkg/http/headers"
|
|
transportpkg "github.com/github/github-mcp-server/pkg/http/transport"
|
|
"github.com/github/github-mcp-server/pkg/translations"
|
|
"github.com/shurcooL/githubv4"
|
|
"github.com/stretchr/testify/assert"
|
|
"github.com/stretchr/testify/require"
|
|
)
|
|
|
|
// projectFieldsQueryMatcher is the GraphQL shape we use for fields(first:100) resolution.
|
|
// Keep this in sync with projectFieldsConnection in projects_resolver.go.
|
|
type projectFieldsTestQuery struct {
|
|
Organization struct {
|
|
ProjectV2 struct {
|
|
Fields 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"`
|
|
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"`
|
|
}
|
|
PageInfo PageInfoFragment
|
|
} `graphql:"fields(first: $first, after: $after)"`
|
|
} `graphql:"projectV2(number: $projectNumber)"`
|
|
} `graphql:"organization(login: $owner)"`
|
|
}
|
|
|
|
func fieldsQueryVars(owner string, projectNumber int) map[string]any {
|
|
return map[string]any{
|
|
"owner": githubv4.String(owner),
|
|
"projectNumber": githubv4.Int(int32(projectNumber)), //nolint:gosec
|
|
"first": githubv4.Int(resolverFieldsPageSize),
|
|
"after": (*githubv4.String)(nil),
|
|
}
|
|
}
|
|
|
|
// statusFieldNode is a single-select field response node for use in mock data.
|
|
// `nodeID` is the global node ID (e.g. "PVTSSF_lADO...") and `databaseID` is
|
|
// the numeric database ID the REST API expects.
|
|
func statusFieldNode(nodeID string, databaseID int, name string, options []map[string]any) map[string]any {
|
|
return map[string]any{
|
|
"id": nodeID,
|
|
"databaseId": databaseID,
|
|
"name": name,
|
|
"dataType": "SINGLE_SELECT",
|
|
"options": options,
|
|
}
|
|
}
|
|
|
|
// iterationFieldNode is an iteration field response node for use in mock data.
|
|
func iterationFieldNode(nodeID string, databaseID int, name string) map[string]any {
|
|
return map[string]any{
|
|
"id": nodeID,
|
|
"databaseId": databaseID,
|
|
"name": name,
|
|
"dataType": "ITERATION",
|
|
}
|
|
}
|
|
|
|
// genericFieldNode is a plain field response node (neither single-select nor
|
|
// iteration, e.g. TEXT or NUMBER) for use in mock data.
|
|
func genericFieldNode(nodeID string, databaseID int, name, dataType string) map[string]any {
|
|
return map[string]any{
|
|
"id": nodeID,
|
|
"databaseId": databaseID,
|
|
"name": name,
|
|
"dataType": dataType,
|
|
}
|
|
}
|
|
|
|
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{
|
|
"projectV2": map[string]any{
|
|
"fields": map[string]any{
|
|
"nodes": nodes,
|
|
"pageInfo": map[string]any{
|
|
"hasNextPage": false,
|
|
"hasPreviousPage": false,
|
|
"startCursor": "",
|
|
"endCursor": "",
|
|
},
|
|
},
|
|
},
|
|
},
|
|
}
|
|
}
|
|
|
|
func Test_ResolveProjectFieldByName_Success(t *testing.T) {
|
|
mocked := githubv4mock.NewMockedHTTPClient(
|
|
githubv4mock.NewQueryMatcher(
|
|
projectFieldsTestQuery{},
|
|
fieldsQueryVars("octo-org", 7),
|
|
githubv4mock.DataResponse(fieldsResponse([]map[string]any{
|
|
statusFieldNode("PVTSSF_lADOBBcDeFg123", 12345, "Status", []map[string]any{
|
|
{"id": "OPT_a", "name": "Todo"},
|
|
{"id": "OPT_b", "name": "In Progress"},
|
|
{"id": "OPT_c", "name": "Done"},
|
|
}),
|
|
})),
|
|
),
|
|
)
|
|
gql := githubv4.NewClient(mocked)
|
|
|
|
field, err := resolveProjectFieldByName(context.Background(), gql, "octo-org", "org", 7, "Status", "SINGLE_SELECT")
|
|
require.NoError(t, err)
|
|
require.NotNil(t, field)
|
|
assert.Equal(t, "12345", field.ID)
|
|
assert.Equal(t, "PVTSSF_lADOBBcDeFg123", field.NodeID)
|
|
assert.Equal(t, "SINGLE_SELECT", field.DataType)
|
|
assert.Len(t, field.Options, 3)
|
|
|
|
optionID, err := resolveSingleSelectOptionByName(field, "In Progress")
|
|
require.NoError(t, err)
|
|
assert.Equal(t, "OPT_b", optionID)
|
|
}
|
|
|
|
func Test_ResolveIssueFieldForUpdate(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
resolved ResolvedField
|
|
databaseID int
|
|
typeName string
|
|
issueField map[string]any
|
|
wantID string
|
|
wantOption ResolvedFieldOption
|
|
}{
|
|
{name: "text", resolved: ResolvedField{ID: "101", Name: "Customer", DataType: "TEXT"}, databaseID: 101, typeName: "ProjectV2Field", issueField: map[string]any{"id": "IF_TEXT"}, wantID: "IF_TEXT"},
|
|
{
|
|
name: "single select", resolved: ResolvedField{ID: "102", Name: "Impact", DataType: "SINGLE_SELECT"},
|
|
databaseID: 102, typeName: "ProjectV2SingleSelectField",
|
|
issueField: map[string]any{"id": "IF_SELECT", "options": []any{map[string]any{"id": "OPT_HIGH", "name": "High"}}},
|
|
wantID: "IF_SELECT",
|
|
wantOption: ResolvedFieldOption{ID: "OPT_HIGH", Name: "High"},
|
|
},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
mocked := githubv4mock.NewMockedHTTPClient(
|
|
githubv4mock.NewQueryMatcher(projectIssueFieldMetadataQueryOrg{}, fieldsQueryVars("octo-org", 7),
|
|
githubv4mock.DataResponse(issueFieldMetadataResponse(tt.typeName, tt.databaseID, true, tt.issueField))),
|
|
)
|
|
capture := &headerCaptureTransport{inner: mocked.Transport}
|
|
gql := githubv4.NewClient(&http.Client{Transport: &transportpkg.GraphQLFeaturesTransport{Transport: capture}})
|
|
|
|
field, err := resolveIssueFieldForUpdate(context.Background(), gql, "octo-org", "org", 7, &tt.resolved)
|
|
require.NoError(t, err)
|
|
assert.True(t, field.IsIssueField)
|
|
assert.Equal(t, tt.wantID, field.IssueFieldID)
|
|
if tt.wantOption.ID != "" {
|
|
assert.Equal(t, []ResolvedFieldOption{tt.wantOption}, field.Options)
|
|
}
|
|
assert.Equal(t, "issue_fields", capture.captured.Get(headers.GraphQLFeaturesHeader))
|
|
})
|
|
}
|
|
}
|
|
|
|
func Test_ResolveIssueFieldForUpdate_ErrorHandling(t *testing.T) {
|
|
for _, tt := range []struct {
|
|
name, message string
|
|
fallback bool
|
|
}{
|
|
{name: "missing schema falls back", message: "Field 'isIssueField' doesn't exist on type 'ProjectV2Field'", fallback: true},
|
|
{name: "missing text fragment type falls back", message: "No such type IssueFieldText, so it cannot be a fragment condition", fallback: true},
|
|
{name: "missing number fragment type falls back", message: "No such type IssueFieldNumber, so it cannot be a fragment condition", fallback: true},
|
|
{name: "missing date fragment type falls back", message: "No such type IssueFieldDate, so it cannot be a fragment condition", fallback: true},
|
|
{name: "missing single select fragment type falls back", message: "No such type IssueFieldSingleSelect, so it cannot be a fragment condition", fallback: true},
|
|
{name: "unknown fragment type propagates", message: "No such type IssueFieldMultiSelect, so it cannot be a fragment condition"},
|
|
{name: "unrelated error propagates", message: "Resource not accessible by integration"},
|
|
} {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
mocked := githubv4mock.NewMockedHTTPClient(githubv4mock.NewQueryMatcher(
|
|
projectIssueFieldMetadataQueryOrg{}, fieldsQueryVars("octo-org", 7), githubv4mock.ErrorResponse(tt.message),
|
|
))
|
|
resolved := &ResolvedField{ID: "101", Name: "Status", DataType: "SINGLE_SELECT"}
|
|
field, err := resolveIssueFieldForUpdate(context.Background(), githubv4.NewClient(mocked), "octo-org", "org", 7, resolved)
|
|
if tt.fallback {
|
|
require.NoError(t, err)
|
|
assert.Equal(t, resolved, field)
|
|
} else {
|
|
require.ErrorContains(t, err, tt.message)
|
|
}
|
|
})
|
|
}
|
|
|
|
t.Run("supported type missing metadata still fails", func(t *testing.T) {
|
|
mocked := githubv4mock.NewMockedHTTPClient(githubv4mock.NewQueryMatcher(
|
|
projectIssueFieldMetadataQueryOrg{},
|
|
fieldsQueryVars("octo-org", 7),
|
|
githubv4mock.DataResponse(fieldsResponse(nil)),
|
|
))
|
|
resolved := &ResolvedField{ID: "101", Name: "Customer", DataType: "TEXT"}
|
|
|
|
_, err := resolveIssueFieldForUpdate(context.Background(), githubv4.NewClient(mocked), "octo-org", "org", 7, resolved)
|
|
require.ErrorContains(t, err, "missing_field_metadata")
|
|
})
|
|
}
|
|
|
|
func Test_ResolveFieldNamesToIDs_QueryRemainsIssueFieldUngated(t *testing.T) {
|
|
mocked := githubv4mock.NewMockedHTTPClient(
|
|
githubv4mock.NewQueryMatcher(
|
|
projectFieldsTestQuery{},
|
|
fieldsQueryVars("octo-org", 1),
|
|
githubv4mock.DataResponse(fieldsResponse([]map[string]any{
|
|
genericFieldNode("PVTF_text", 101, "Customer", "TEXT"),
|
|
})),
|
|
),
|
|
)
|
|
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"}, "fields")
|
|
require.NoError(t, err)
|
|
assert.Equal(t, []int64{101}, ids)
|
|
assert.Empty(t, capture.captured.Get(headers.GraphQLFeaturesHeader))
|
|
}
|
|
|
|
func issueFieldMetadataResponse(typeName string, databaseID any, isIssueField bool, issueField map[string]any) map[string]any {
|
|
node := map[string]any{
|
|
"__typename": typeName,
|
|
"databaseId": databaseID,
|
|
"isIssueField": isIssueField,
|
|
}
|
|
if issueField != nil {
|
|
node["issueField"] = issueField
|
|
}
|
|
return fieldsResponse([]map[string]any{node})
|
|
}
|
|
|
|
func Test_ResolveProjectFieldByName_NodeIDsForAllVariants(t *testing.T) {
|
|
mocked := githubv4mock.NewMockedHTTPClient(
|
|
githubv4mock.NewQueryMatcher(
|
|
projectFieldsTestQuery{},
|
|
fieldsQueryVars("octo-org", 7),
|
|
githubv4mock.DataResponse(fieldsResponse([]map[string]any{
|
|
statusFieldNode("PVTSSF_single1", 111, "Status", []map[string]any{
|
|
{"id": "OPT_a", "name": "Todo"},
|
|
}),
|
|
iterationFieldNode("PVTIF_iteration1", 222, "Sprint"),
|
|
multiSelectFieldNode("PVTMSSF_multi1", 444, "Teams"),
|
|
genericFieldNode("PVTF_text1", 333, "Notes", "TEXT"),
|
|
})),
|
|
),
|
|
)
|
|
gql := githubv4.NewClient(mocked)
|
|
|
|
variants := []struct {
|
|
fieldName string
|
|
expectedType string
|
|
wantNodeID string
|
|
}{
|
|
{"Status", "SINGLE_SELECT", "PVTSSF_single1"},
|
|
{"Sprint", "ITERATION", "PVTIF_iteration1"},
|
|
{"Teams", "MULTI_SELECT", "PVTMSSF_multi1"},
|
|
{"Notes", "TEXT", "PVTF_text1"},
|
|
}
|
|
for _, v := range variants {
|
|
t.Run(v.fieldName, func(t *testing.T) {
|
|
field, err := resolveProjectFieldByName(context.Background(), gql, "octo-org", "org", 7, v.fieldName, v.expectedType)
|
|
require.NoError(t, err)
|
|
require.NotNil(t, field)
|
|
assert.Equal(t, v.wantNodeID, field.NodeID)
|
|
assert.Equal(t, v.expectedType, field.DataType)
|
|
})
|
|
}
|
|
}
|
|
|
|
func Test_ResolveProjectFieldByName_NotFound_ReturnsStructuredError(t *testing.T) {
|
|
mocked := githubv4mock.NewMockedHTTPClient(
|
|
githubv4mock.NewQueryMatcher(
|
|
projectFieldsTestQuery{},
|
|
fieldsQueryVars("octo-org", 7),
|
|
githubv4mock.DataResponse(fieldsResponse([]map[string]any{
|
|
statusFieldNode("PVTSSF_lADOBBcDeFg123", 12345, "Status", nil),
|
|
})),
|
|
),
|
|
)
|
|
gql := githubv4.NewClient(mocked)
|
|
|
|
_, err := resolveProjectFieldByName(context.Background(), gql, "octo-org", "org", 7, "Priority", "")
|
|
require.Error(t, err)
|
|
|
|
var msg map[string]any
|
|
require.NoError(t, json.Unmarshal([]byte(err.Error()), &msg))
|
|
assert.Equal(t, "field_not_found", msg["error"])
|
|
assert.Equal(t, "Priority", msg["name"])
|
|
assert.NotEmpty(t, msg["candidates"])
|
|
}
|
|
|
|
func Test_ResolveProjectFieldByName_Ambiguous_ReturnsStructuredError(t *testing.T) {
|
|
mocked := githubv4mock.NewMockedHTTPClient(
|
|
githubv4mock.NewQueryMatcher(
|
|
projectFieldsTestQuery{},
|
|
fieldsQueryVars("octo-org", 7),
|
|
githubv4mock.DataResponse(fieldsResponse([]map[string]any{
|
|
statusFieldNode("PVTSSF_lADOBBcDeFg123", 12345, "Status", nil),
|
|
statusFieldNode("PVTSSF_lADOBBcDeFg678", 67890, "Status", nil),
|
|
})),
|
|
),
|
|
)
|
|
gql := githubv4.NewClient(mocked)
|
|
|
|
_, err := resolveProjectFieldByName(context.Background(), gql, "octo-org", "org", 7, "Status", "")
|
|
require.Error(t, err)
|
|
|
|
var msg map[string]any
|
|
require.NoError(t, json.Unmarshal([]byte(err.Error()), &msg))
|
|
assert.Equal(t, "field_ambiguous", msg["error"])
|
|
candidates, _ := msg["candidates"].([]any)
|
|
assert.Len(t, candidates, 2)
|
|
}
|
|
|
|
func Test_ResolveSingleSelectOptionByName_NotFound(t *testing.T) {
|
|
field := &ResolvedField{
|
|
ID: "12345",
|
|
Name: "Status",
|
|
DataType: "SINGLE_SELECT",
|
|
Options: []ResolvedFieldOption{
|
|
{ID: "OPT_a", Name: "Todo"},
|
|
{ID: "OPT_b", Name: "Done"},
|
|
},
|
|
}
|
|
|
|
_, err := resolveSingleSelectOptionByName(field, "Blocked")
|
|
require.Error(t, err)
|
|
|
|
var msg map[string]any
|
|
require.NoError(t, json.Unmarshal([]byte(err.Error()), &msg))
|
|
assert.Equal(t, "option_not_found", msg["error"])
|
|
assert.Equal(t, "Blocked", msg["name"])
|
|
}
|
|
|
|
func Test_ResolveSingleSelectOptionByName_WrongFieldType(t *testing.T) {
|
|
field := &ResolvedField{
|
|
ID: "12345",
|
|
Name: "Description",
|
|
DataType: "TEXT",
|
|
}
|
|
|
|
_, err := resolveSingleSelectOptionByName(field, "anything")
|
|
require.Error(t, err)
|
|
|
|
var msg map[string]any
|
|
require.NoError(t, json.Unmarshal([]byte(err.Error()), &msg))
|
|
assert.Equal(t, "wrong_field_type", msg["error"])
|
|
}
|
|
|
|
// resolveItemByIssueQuery matches the GraphQL shape used by
|
|
// resolveProjectItemIDByIssueNumber for the issue.projectItems traversal.
|
|
type resolveItemByIssueQuery struct {
|
|
Repository struct {
|
|
Issue struct {
|
|
ProjectItems struct {
|
|
Nodes []struct {
|
|
ID githubv4.ID
|
|
FullDatabaseID githubv4.String `graphql:"fullDatabaseId"`
|
|
Project struct {
|
|
ID githubv4.ID
|
|
}
|
|
}
|
|
PageInfo PageInfoFragment
|
|
} `graphql:"projectItems(first: 50, includeArchived: true)"`
|
|
} `graphql:"issue(number: $issueNumber)"`
|
|
} `graphql:"repository(owner: $issueOwner, name: $issueRepo)"`
|
|
}
|
|
|
|
type resolveItemByIssuePageQuery struct {
|
|
Repository struct {
|
|
Issue struct {
|
|
ProjectItems struct {
|
|
Nodes []struct {
|
|
ID githubv4.ID
|
|
FullDatabaseID githubv4.String `graphql:"fullDatabaseId"`
|
|
Project struct {
|
|
ID githubv4.ID
|
|
}
|
|
}
|
|
PageInfo PageInfoFragment
|
|
} `graphql:"projectItems(first: 50, after: $after, includeArchived: true)"`
|
|
} `graphql:"issue(number: $issueNumber)"`
|
|
} `graphql:"repository(owner: $issueOwner, name: $issueRepo)"`
|
|
}
|
|
|
|
type requestCountingTransport struct {
|
|
inner http.RoundTripper
|
|
count int
|
|
}
|
|
|
|
func (t *requestCountingTransport) RoundTrip(req *http.Request) (*http.Response, error) {
|
|
t.count++
|
|
return t.inner.RoundTrip(req)
|
|
}
|
|
|
|
func Test_ResolveProjectItemByIssueNumber_Success(t *testing.T) {
|
|
mocked := githubv4mock.NewMockedHTTPClient(
|
|
// project node id lookup (org)
|
|
githubv4mock.NewQueryMatcher(
|
|
struct {
|
|
Organization struct {
|
|
ProjectV2 struct {
|
|
ID githubv4.ID
|
|
} `graphql:"projectV2(number: $projectNumber)"`
|
|
} `graphql:"organization(login: $owner)"`
|
|
}{},
|
|
map[string]any{
|
|
"owner": githubv4.String("octo-org"),
|
|
"projectNumber": githubv4.Int(1),
|
|
},
|
|
githubv4mock.DataResponse(map[string]any{
|
|
"organization": map[string]any{
|
|
"projectV2": map[string]any{
|
|
"id": "PVT_project1",
|
|
},
|
|
},
|
|
}),
|
|
),
|
|
// issue.projectItems lookup
|
|
githubv4mock.NewQueryMatcher(
|
|
resolveItemByIssueQuery{},
|
|
map[string]any{
|
|
"issueOwner": githubv4.String("octo-issue-owner"),
|
|
"issueRepo": githubv4.String("repo"),
|
|
"issueNumber": githubv4.Int(123),
|
|
},
|
|
githubv4mock.DataResponse(map[string]any{
|
|
"repository": map[string]any{
|
|
"issue": map[string]any{
|
|
"projectItems": map[string]any{
|
|
"nodes": []any{
|
|
map[string]any{
|
|
"fullDatabaseId": "9999",
|
|
"project": map[string]any{"id": "PVT_other"},
|
|
},
|
|
map[string]any{
|
|
"id": "PVTI_target",
|
|
"fullDatabaseId": "4242",
|
|
"project": map[string]any{"id": "PVT_project1"},
|
|
},
|
|
},
|
|
"pageInfo": map[string]any{
|
|
"hasNextPage": false,
|
|
"hasPreviousPage": false,
|
|
"startCursor": "",
|
|
"endCursor": "",
|
|
},
|
|
},
|
|
},
|
|
},
|
|
}),
|
|
),
|
|
)
|
|
gql := githubv4.NewClient(mocked)
|
|
|
|
nodeID, itemID, err := resolveProjectItemByIssueNumber(context.Background(), gql, "octo-org", "org", 1, "octo-issue-owner", "repo", 123)
|
|
require.NoError(t, err)
|
|
assert.Equal(t, "PVTI_target", nodeID)
|
|
assert.Equal(t, int64(4242), itemID)
|
|
}
|
|
|
|
func Test_ResolveProjectItemByIssueNumber_TargetOnSecondPage(t *testing.T) {
|
|
mocked := githubv4mock.NewMockedHTTPClient(
|
|
githubv4mock.NewQueryMatcher(
|
|
struct {
|
|
Organization struct {
|
|
ProjectV2 struct {
|
|
ID githubv4.ID
|
|
} `graphql:"projectV2(number: $projectNumber)"`
|
|
} `graphql:"organization(login: $owner)"`
|
|
}{},
|
|
map[string]any{
|
|
"owner": githubv4.String("octo-org"),
|
|
"projectNumber": githubv4.Int(1),
|
|
},
|
|
githubv4mock.DataResponse(map[string]any{
|
|
"organization": map[string]any{
|
|
"projectV2": map[string]any{"id": "PVT_project1"},
|
|
},
|
|
}),
|
|
),
|
|
githubv4mock.NewQueryMatcher(
|
|
resolveItemByIssueQuery{},
|
|
map[string]any{
|
|
"issueOwner": githubv4.String("octo-issue-owner"),
|
|
"issueRepo": githubv4.String("repo"),
|
|
"issueNumber": githubv4.Int(123),
|
|
},
|
|
githubv4mock.DataResponse(map[string]any{
|
|
"repository": map[string]any{
|
|
"issue": map[string]any{
|
|
"projectItems": map[string]any{
|
|
"nodes": []any{
|
|
map[string]any{
|
|
"fullDatabaseId": "9999",
|
|
"project": map[string]any{"id": "PVT_other"},
|
|
},
|
|
},
|
|
"pageInfo": map[string]any{
|
|
"hasNextPage": true,
|
|
"hasPreviousPage": false,
|
|
"startCursor": "first",
|
|
"endCursor": "page-one",
|
|
},
|
|
},
|
|
},
|
|
},
|
|
}),
|
|
),
|
|
githubv4mock.NewQueryMatcher(
|
|
resolveItemByIssuePageQuery{},
|
|
map[string]any{
|
|
"issueOwner": githubv4.String("octo-issue-owner"),
|
|
"issueRepo": githubv4.String("repo"),
|
|
"issueNumber": githubv4.Int(123),
|
|
"after": githubv4.String("page-one"),
|
|
},
|
|
githubv4mock.DataResponse(map[string]any{
|
|
"repository": map[string]any{
|
|
"issue": map[string]any{
|
|
"projectItems": map[string]any{
|
|
"nodes": []any{
|
|
map[string]any{
|
|
"id": "PVTI_target",
|
|
"fullDatabaseId": "4242",
|
|
"project": map[string]any{"id": "PVT_project1"},
|
|
},
|
|
},
|
|
"pageInfo": map[string]any{
|
|
"hasNextPage": false,
|
|
"hasPreviousPage": true,
|
|
"startCursor": "page-two",
|
|
"endCursor": "page-two",
|
|
},
|
|
},
|
|
},
|
|
},
|
|
}),
|
|
),
|
|
)
|
|
gql := githubv4.NewClient(mocked)
|
|
|
|
nodeID, itemID, err := resolveProjectItemByIssueNumber(context.Background(), gql, "octo-org", "org", 1, "octo-issue-owner", "repo", 123)
|
|
require.NoError(t, err)
|
|
assert.Equal(t, "PVTI_target", nodeID)
|
|
assert.Equal(t, int64(4242), itemID)
|
|
}
|
|
|
|
func Test_ResolveProjectItemIDByIssueNumber_NotInProject(t *testing.T) {
|
|
mocked := githubv4mock.NewMockedHTTPClient(
|
|
githubv4mock.NewQueryMatcher(
|
|
struct {
|
|
Organization struct {
|
|
ProjectV2 struct {
|
|
ID githubv4.ID
|
|
} `graphql:"projectV2(number: $projectNumber)"`
|
|
} `graphql:"organization(login: $owner)"`
|
|
}{},
|
|
map[string]any{
|
|
"owner": githubv4.String("octo-org"),
|
|
"projectNumber": githubv4.Int(1),
|
|
},
|
|
githubv4mock.DataResponse(map[string]any{
|
|
"organization": map[string]any{
|
|
"projectV2": map[string]any{
|
|
"id": "PVT_project1",
|
|
},
|
|
},
|
|
}),
|
|
),
|
|
githubv4mock.NewQueryMatcher(
|
|
resolveItemByIssueQuery{},
|
|
map[string]any{
|
|
"issueOwner": githubv4.String("octo-issue-owner"),
|
|
"issueRepo": githubv4.String("repo"),
|
|
"issueNumber": githubv4.Int(123),
|
|
},
|
|
githubv4mock.DataResponse(map[string]any{
|
|
"repository": map[string]any{
|
|
"issue": map[string]any{
|
|
"projectItems": map[string]any{
|
|
"nodes": []any{
|
|
map[string]any{
|
|
"fullDatabaseId": "9999",
|
|
"project": map[string]any{"id": "PVT_other"},
|
|
},
|
|
},
|
|
"pageInfo": map[string]any{
|
|
"hasNextPage": false,
|
|
"hasPreviousPage": false,
|
|
"startCursor": "",
|
|
"endCursor": "",
|
|
},
|
|
},
|
|
},
|
|
},
|
|
}),
|
|
),
|
|
)
|
|
gql := githubv4.NewClient(mocked)
|
|
|
|
_, err := resolveProjectItemIDByIssueNumber(context.Background(), gql, "octo-org", "org", 1, "octo-issue-owner", "repo", 123)
|
|
require.Error(t, err)
|
|
|
|
var msg map[string]any
|
|
require.NoError(t, json.Unmarshal([]byte(err.Error()), &msg))
|
|
assert.Equal(t, "item_not_in_project", msg["error"])
|
|
}
|
|
|
|
func Test_ResolveProjectItemIDByIssueNumber_NotInProjectAfterMultiplePages(t *testing.T) {
|
|
mocked := githubv4mock.NewMockedHTTPClient(
|
|
githubv4mock.NewQueryMatcher(
|
|
struct {
|
|
Organization struct {
|
|
ProjectV2 struct {
|
|
ID githubv4.ID
|
|
} `graphql:"projectV2(number: $projectNumber)"`
|
|
} `graphql:"organization(login: $owner)"`
|
|
}{},
|
|
map[string]any{
|
|
"owner": githubv4.String("octo-org"),
|
|
"projectNumber": githubv4.Int(1),
|
|
},
|
|
githubv4mock.DataResponse(map[string]any{
|
|
"organization": map[string]any{
|
|
"projectV2": map[string]any{"id": "PVT_project1"},
|
|
},
|
|
}),
|
|
),
|
|
githubv4mock.NewQueryMatcher(
|
|
resolveItemByIssueQuery{},
|
|
map[string]any{
|
|
"issueOwner": githubv4.String("octo-issue-owner"),
|
|
"issueRepo": githubv4.String("repo"),
|
|
"issueNumber": githubv4.Int(123),
|
|
},
|
|
githubv4mock.DataResponse(map[string]any{
|
|
"repository": map[string]any{
|
|
"issue": map[string]any{
|
|
"projectItems": map[string]any{
|
|
"nodes": []any{
|
|
map[string]any{
|
|
"fullDatabaseId": "9999",
|
|
"project": map[string]any{"id": "PVT_other"},
|
|
},
|
|
},
|
|
"pageInfo": map[string]any{
|
|
"hasNextPage": true,
|
|
"hasPreviousPage": false,
|
|
"startCursor": "first",
|
|
"endCursor": "page-one",
|
|
},
|
|
},
|
|
},
|
|
},
|
|
}),
|
|
),
|
|
githubv4mock.NewQueryMatcher(
|
|
resolveItemByIssuePageQuery{},
|
|
map[string]any{
|
|
"issueOwner": githubv4.String("octo-issue-owner"),
|
|
"issueRepo": githubv4.String("repo"),
|
|
"issueNumber": githubv4.Int(123),
|
|
"after": githubv4.String("page-one"),
|
|
},
|
|
githubv4mock.DataResponse(map[string]any{
|
|
"repository": map[string]any{
|
|
"issue": map[string]any{
|
|
"projectItems": map[string]any{
|
|
"nodes": []any{
|
|
map[string]any{
|
|
"fullDatabaseId": "8888",
|
|
"project": map[string]any{"id": "PVT_another"},
|
|
},
|
|
},
|
|
"pageInfo": map[string]any{
|
|
"hasNextPage": false,
|
|
"hasPreviousPage": true,
|
|
"startCursor": "page-two",
|
|
"endCursor": "page-two",
|
|
},
|
|
},
|
|
},
|
|
},
|
|
}),
|
|
),
|
|
)
|
|
countingTransport := &requestCountingTransport{inner: mocked.Transport}
|
|
mocked.Transport = countingTransport
|
|
gql := githubv4.NewClient(mocked)
|
|
|
|
_, err := resolveProjectItemIDByIssueNumber(context.Background(), gql, "octo-org", "org", 1, "octo-issue-owner", "repo", 123)
|
|
require.Error(t, err)
|
|
assert.Equal(t, 3, countingTransport.count)
|
|
|
|
var msg map[string]any
|
|
require.NoError(t, json.Unmarshal([]byte(err.Error()), &msg))
|
|
assert.Equal(t, "item_not_in_project", msg["error"])
|
|
}
|
|
|
|
func Test_ResolveFieldNamesToIDs_Success(t *testing.T) {
|
|
mocked := githubv4mock.NewMockedHTTPClient(
|
|
githubv4mock.NewQueryMatcher(
|
|
projectFieldsTestQuery{},
|
|
fieldsQueryVars("octo-org", 1),
|
|
githubv4mock.DataResponse(fieldsResponse([]map[string]any{
|
|
statusFieldNode("PVTSSF_lADOBBcDeFg100", 100, "Status", nil),
|
|
statusFieldNode("PVTSSF_lADOBBcDeFg200", 200, "Priority", nil),
|
|
})),
|
|
),
|
|
)
|
|
gql := githubv4.NewClient(mocked)
|
|
|
|
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)
|
|
}
|
|
|
|
// Field and single-select option name matching is case-insensitive so agents passing lowercase
|
|
// names like "status" or "in progress" resolve to "Status" and "In Progress" respectively.
|
|
func Test_ResolveProjectFieldByName_CaseInsensitive(t *testing.T) {
|
|
mocked := githubv4mock.NewMockedHTTPClient(
|
|
githubv4mock.NewQueryMatcher(
|
|
projectFieldsTestQuery{},
|
|
fieldsQueryVars("octo-org", 7),
|
|
githubv4mock.DataResponse(fieldsResponse([]map[string]any{
|
|
statusFieldNode("PVTSSF_lADOBBcDeFg123", 12345, "Status", []map[string]any{
|
|
{"id": "OPT_a", "name": "Todo"},
|
|
{"id": "OPT_b", "name": "In Progress"},
|
|
}),
|
|
})),
|
|
),
|
|
)
|
|
gql := githubv4.NewClient(mocked)
|
|
|
|
field, err := resolveProjectFieldByName(context.Background(), gql, "octo-org", "org", 7, "status", "")
|
|
require.NoError(t, err)
|
|
require.NotNil(t, field)
|
|
assert.Equal(t, "12345", field.ID)
|
|
|
|
optionID, err := resolveSingleSelectOptionByName(field, "in progress")
|
|
require.NoError(t, err)
|
|
assert.Equal(t, "OPT_b", optionID)
|
|
}
|
|
|
|
// Test_ResolveFieldNamesToIDs_CaseInsensitive verifies bulk name resolution
|
|
// also matches case-insensitively.
|
|
func Test_ResolveFieldNamesToIDs_CaseInsensitive(t *testing.T) {
|
|
mocked := githubv4mock.NewMockedHTTPClient(
|
|
githubv4mock.NewQueryMatcher(
|
|
projectFieldsTestQuery{},
|
|
fieldsQueryVars("octo-org", 1),
|
|
githubv4mock.DataResponse(fieldsResponse([]map[string]any{
|
|
statusFieldNode("PVTSSF_lADOBBcDeFg100", 100, "Status", nil),
|
|
statusFieldNode("PVTSSF_lADOBBcDeFg200", 200, "Priority", nil),
|
|
})),
|
|
),
|
|
)
|
|
gql := githubv4.NewClient(mocked)
|
|
|
|
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) {
|
|
toolDef := ProjectsWrite(translations.NullTranslationHelper)
|
|
|
|
updatedItem := verbosePullRequestProjectItemFixture()
|
|
|
|
mockedREST := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
|
|
PatchOrgsProjectsV2ItemsByProjectByItemID: mockResponse(t, http.StatusOK, updatedItem),
|
|
})
|
|
restClient := mustNewGHClient(t, mockedREST)
|
|
|
|
mockedGQL := githubv4mock.NewMockedHTTPClient(
|
|
// 1. project node id (used by resolveProjectItemIDByIssueNumber)
|
|
githubv4mock.NewQueryMatcher(
|
|
struct {
|
|
Organization struct {
|
|
ProjectV2 struct {
|
|
ID githubv4.ID
|
|
} `graphql:"projectV2(number: $projectNumber)"`
|
|
} `graphql:"organization(login: $owner)"`
|
|
}{},
|
|
map[string]any{
|
|
"owner": githubv4.String("octo-org"),
|
|
"projectNumber": githubv4.Int(1),
|
|
},
|
|
githubv4mock.DataResponse(map[string]any{
|
|
"organization": map[string]any{
|
|
"projectV2": map[string]any{"id": "PVT_project1"},
|
|
},
|
|
}),
|
|
),
|
|
// 2. issue -> projectItems lookup
|
|
githubv4mock.NewQueryMatcher(
|
|
resolveItemByIssueQuery{},
|
|
map[string]any{
|
|
"issueOwner": githubv4.String("github"),
|
|
"issueRepo": githubv4.String("planning-tracking"),
|
|
"issueNumber": githubv4.Int(123),
|
|
},
|
|
githubv4mock.DataResponse(map[string]any{
|
|
"repository": map[string]any{
|
|
"issue": map[string]any{
|
|
"projectItems": map[string]any{
|
|
"nodes": []any{
|
|
map[string]any{
|
|
"fullDatabaseId": "1001",
|
|
"project": map[string]any{"id": "PVT_project1"},
|
|
},
|
|
},
|
|
"pageInfo": map[string]any{
|
|
"hasNextPage": false, "hasPreviousPage": false,
|
|
"startCursor": "", "endCursor": "",
|
|
},
|
|
},
|
|
},
|
|
},
|
|
}),
|
|
),
|
|
// 3. fields(first:100) for name resolution
|
|
githubv4mock.NewQueryMatcher(
|
|
projectFieldsTestQuery{},
|
|
fieldsQueryVars("octo-org", 1),
|
|
githubv4mock.DataResponse(fieldsResponse([]map[string]any{
|
|
statusFieldNode("PVTSSF_lADOBBcDeFg101", 101, "Status", []map[string]any{
|
|
{"id": "OPT_in_progress", "name": "In Progress"},
|
|
}),
|
|
})),
|
|
),
|
|
// 4. supplemental update metadata confirms this is a standard Project field
|
|
githubv4mock.NewQueryMatcher(
|
|
projectIssueFieldMetadataQueryOrg{},
|
|
fieldsQueryVars("octo-org", 1),
|
|
githubv4mock.DataResponse(map[string]any{
|
|
"organization": map[string]any{
|
|
"projectV2": map[string]any{
|
|
"fields": map[string]any{
|
|
"nodes": []any{
|
|
map[string]any{
|
|
"__typename": "ProjectV2SingleSelectField",
|
|
"databaseId": 101,
|
|
"isIssueField": false,
|
|
},
|
|
},
|
|
"pageInfo": map[string]any{
|
|
"hasNextPage": false, "hasPreviousPage": false,
|
|
"startCursor": "", "endCursor": "",
|
|
},
|
|
},
|
|
},
|
|
},
|
|
}),
|
|
),
|
|
)
|
|
gqlClient := githubv4.NewClient(mockedGQL)
|
|
|
|
deps := BaseDeps{Client: restClient, GQLClient: gqlClient}
|
|
handler := toolDef.Handler(deps)
|
|
request := createMCPRequest(map[string]any{
|
|
"method": "update_project_item",
|
|
"owner": "octo-org",
|
|
"owner_type": "org",
|
|
"project_number": float64(1),
|
|
"item_owner": "github",
|
|
"item_repo": "planning-tracking",
|
|
"issue_number": float64(123),
|
|
"updated_field": map[string]any{
|
|
"name": "Status",
|
|
"value": "In Progress",
|
|
},
|
|
})
|
|
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
|
|
|
|
require.NoError(t, err)
|
|
require.False(t, result.IsError, getTextResult(t, result).Text)
|
|
}
|
|
|
|
func Test_ProjectsWrite_UpdateProjectItem_ByNameIteration(t *testing.T) {
|
|
updatedItem := verbosePullRequestProjectItemFixture()
|
|
restCalled := false
|
|
mockedREST := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
|
|
PatchOrgsProjectsV2ItemsByProjectByItemID: func(w http.ResponseWriter, r *http.Request) {
|
|
restCalled = true
|
|
var update struct {
|
|
Fields []struct {
|
|
ID int64 `json:"id"`
|
|
Value any `json:"value"`
|
|
} `json:"fields"`
|
|
}
|
|
require.NoError(t, json.NewDecoder(r.Body).Decode(&update))
|
|
require.Len(t, update.Fields, 1)
|
|
assert.Equal(t, int64(222), update.Fields[0].ID)
|
|
assert.Equal(t, "ITERATION_1", update.Fields[0].Value)
|
|
|
|
w.Header().Set("Content-Type", "application/json")
|
|
require.NoError(t, json.NewEncoder(w).Encode(updatedItem))
|
|
},
|
|
})
|
|
mockedGQL := githubv4mock.NewMockedHTTPClient(
|
|
githubv4mock.NewQueryMatcher(
|
|
projectFieldsTestQuery{},
|
|
fieldsQueryVars("octo-org", 1),
|
|
githubv4mock.DataResponse(fieldsResponse([]map[string]any{
|
|
iterationFieldNode("PVTIF_iteration1", 222, "Sprint"),
|
|
})),
|
|
),
|
|
)
|
|
deps := BaseDeps{
|
|
Client: mustNewGHClient(t, mockedREST),
|
|
GQLClient: githubv4.NewClient(mockedGQL),
|
|
}
|
|
toolDef := ProjectsWrite(translations.NullTranslationHelper)
|
|
handler := toolDef.Handler(deps)
|
|
request := createMCPRequest(map[string]any{
|
|
"method": "update_project_item",
|
|
"owner": "octo-org",
|
|
"owner_type": "org",
|
|
"project_number": float64(1),
|
|
"item_id": float64(1001),
|
|
"updated_field": map[string]any{
|
|
"name": "Sprint",
|
|
"value": "ITERATION_1",
|
|
},
|
|
})
|
|
|
|
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
|
|
require.NoError(t, err)
|
|
require.False(t, result.IsError, getTextResult(t, result).Text)
|
|
assert.True(t, restCalled)
|
|
}
|
|
|
|
func Test_ProjectsWrite_UpdateProjectItem_NameNotFound_StructuredError(t *testing.T) {
|
|
toolDef := ProjectsWrite(translations.NullTranslationHelper)
|
|
|
|
mockedGQL := githubv4mock.NewMockedHTTPClient(
|
|
githubv4mock.NewQueryMatcher(
|
|
projectFieldsTestQuery{},
|
|
fieldsQueryVars("octo-org", 1),
|
|
githubv4mock.DataResponse(fieldsResponse([]map[string]any{
|
|
statusFieldNode("PVTSSF_lADOBBcDeFg101", 101, "Status", nil),
|
|
})),
|
|
),
|
|
)
|
|
gqlClient := githubv4.NewClient(mockedGQL)
|
|
restClient := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{}))
|
|
|
|
deps := BaseDeps{Client: restClient, GQLClient: gqlClient}
|
|
handler := toolDef.Handler(deps)
|
|
request := createMCPRequest(map[string]any{
|
|
"method": "update_project_item",
|
|
"owner": "octo-org",
|
|
"owner_type": "org",
|
|
"project_number": float64(1),
|
|
"item_id": float64(1001),
|
|
"updated_field": map[string]any{
|
|
"name": "Doesnt Exist",
|
|
"value": "whatever",
|
|
},
|
|
})
|
|
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
|
|
|
|
require.NoError(t, err)
|
|
require.True(t, result.IsError)
|
|
|
|
textContent := getTextResult(t, result)
|
|
var msg map[string]any
|
|
require.NoError(t, json.Unmarshal([]byte(textContent.Text), &msg))
|
|
assert.Equal(t, "field_not_found", msg["error"])
|
|
assert.Equal(t, "Doesnt Exist", msg["name"])
|
|
}
|