Skip to content

Commit fdbf834

Browse files
elijahstrJoannaaKL
authored andcommitted
Fix projects_get required params and harden status update tools
Loosen projects_get schema to only require "method", since get_project_status_update only needs status_update_id and never uses owner or project_number. Also use pointer types for optional statusUpdateNode fields, add owner_type validation for list/create status updates, clamp negative per_page values, and fix resolveProjectNodeID to return "" instead of nil on error.
1 parent 910185c commit fdbf834

File tree

3 files changed

+143
-15
lines changed

3 files changed

+143
-15
lines changed

pkg/github/__toolsnaps__/projects_get.snap

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,7 @@
5353
}
5454
},
5555
"required": [
56-
"method",
57-
"owner",
58-
"project_number"
56+
"method"
5957
],
6058
"type": "object"
6159
},

pkg/github/projects.go

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,11 @@ const (
5555

5656
type statusUpdateNode struct {
5757
ID githubv4.ID
58-
Body githubv4.String
59-
Status githubv4.String
58+
Body *githubv4.String
59+
Status *githubv4.String
6060
CreatedAt githubv4.DateTime
61-
StartDate githubv4.String
62-
TargetDate githubv4.String
61+
StartDate *githubv4.String
62+
TargetDate *githubv4.String
6363
Creator struct {
6464
Login githubv4.String
6565
}
@@ -123,15 +123,22 @@ func convertToMinimalStatusUpdate(node statusUpdateNode) MinimalProjectStatusUpd
123123

124124
return MinimalProjectStatusUpdate{
125125
ID: fmt.Sprintf("%v", node.ID),
126-
Body: string(node.Body),
127-
Status: string(node.Status),
126+
Body: derefString(node.Body),
127+
Status: derefString(node.Status),
128128
CreatedAt: node.CreatedAt.Time.Format(time.RFC3339),
129-
StartDate: string(node.StartDate),
130-
TargetDate: string(node.TargetDate),
129+
StartDate: derefString(node.StartDate),
130+
TargetDate: derefString(node.TargetDate),
131131
Creator: creator,
132132
}
133133
}
134134

135+
func derefString(s *githubv4.String) string {
136+
if s == nil {
137+
return ""
138+
}
139+
return string(*s)
140+
}
141+
135142
func ListProjects(t translations.TranslationHelperFunc) inventory.ServerTool {
136143
tool := NewTool(
137144
ToolsetMetadataProjects,
@@ -1482,7 +1489,7 @@ Use this tool to get details about individual projects, project fields, and proj
14821489
Description: "The node ID of the project status update. Required for 'get_project_status_update' method.",
14831490
},
14841491
},
1485-
Required: []string{"method", "owner", "project_number"},
1492+
Required: []string{"method"},
14861493
},
14871494
},
14881495
[]scopes.Scope{scopes.ReadProject},
@@ -2213,14 +2220,14 @@ func resolveProjectNodeID(ctx context.Context, gqlClient *githubv4.Client, owner
22132220
if ownerType == "org" {
22142221
err := gqlClient.Query(ctx, &projectIDQueryOrg, queryVars)
22152222
if err != nil {
2216-
return nil, fmt.Errorf("%s: %w", ProjectResolveIDFailedError, err)
2223+
return "", fmt.Errorf("%s: %w", ProjectResolveIDFailedError, err)
22172224
}
22182225
return projectIDQueryOrg.Organization.ProjectV2.ID, nil
22192226
}
22202227

22212228
err := gqlClient.Query(ctx, &projectIDQueryUser, queryVars)
22222229
if err != nil {
2223-
return nil, fmt.Errorf("%s: %w", ProjectResolveIDFailedError, err)
2230+
return "", fmt.Errorf("%s: %w", ProjectResolveIDFailedError, err)
22242231
}
22252232
return projectIDQueryUser.User.ProjectV2.ID, nil
22262233
}
@@ -2293,6 +2300,9 @@ func validateDateFormat(value, fieldName string) error {
22932300
// createProjectStatusUpdate creates a new status update for a project via GraphQL.
22942301
func createProjectStatusUpdate(ctx context.Context, gqlClient *githubv4.Client, owner, ownerType string, projectNumber int, body, status, startDate, targetDate string) (*mcp.CallToolResult, any, error) {
22952302
// Validate inputs
2303+
if ownerType != "user" && ownerType != "org" {
2304+
return utils.NewToolResultError(fmt.Sprintf("invalid owner_type %q: must be \"user\" or \"org\"", ownerType)), nil, nil
2305+
}
22962306
if status != "" && !validProjectV2StatusUpdateStatuses[status] {
22972307
return utils.NewToolResultError(fmt.Sprintf("invalid status %q: must be one of INACTIVE, ON_TRACK, AT_RISK, OFF_TRACK, COMPLETE", status)), nil, nil
22982308
}
@@ -2360,6 +2370,10 @@ func createProjectStatusUpdate(ctx context.Context, gqlClient *githubv4.Client,
23602370

23612371
// listProjectStatusUpdates lists status updates for a project via GraphQL.
23622372
func listProjectStatusUpdates(ctx context.Context, gqlClient *githubv4.Client, args map[string]any, owner, ownerType string) (*mcp.CallToolResult, any, error) {
2373+
if ownerType != "user" && ownerType != "org" {
2374+
return utils.NewToolResultError(fmt.Sprintf("invalid owner_type %q: must be \"user\" or \"org\"", ownerType)), nil, nil
2375+
}
2376+
23632377
projectNumber, err := RequiredInt(args, "project_number")
23642378
if err != nil {
23652379
return utils.NewToolResultError(err.Error()), nil, nil
@@ -2372,6 +2386,9 @@ func listProjectStatusUpdates(ctx context.Context, gqlClient *githubv4.Client, a
23722386
if perPage > MaxProjectsPerPage {
23732387
perPage = MaxProjectsPerPage
23742388
}
2389+
if perPage < 1 {
2390+
perPage = MaxProjectsPerPage
2391+
}
23752392

23762393
afterCursor, err := OptionalParam[string](args, "after")
23772394
if err != nil {

pkg/github/projects_test.go

Lines changed: 114 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ func Test_ProjectsGet(t *testing.T) {
236236
assert.Contains(t, inputSchema.Properties, "project_number")
237237
assert.Contains(t, inputSchema.Properties, "field_id")
238238
assert.Contains(t, inputSchema.Properties, "item_id")
239-
assert.ElementsMatch(t, inputSchema.Required, []string{"method", "owner", "project_number"})
239+
assert.ElementsMatch(t, inputSchema.Required, []string{"method"})
240240
}
241241

242242
func Test_ProjectsGet_GetProject(t *testing.T) {
@@ -1907,3 +1907,116 @@ func Test_ProjectsWrite_CreateProjectStatusUpdate(t *testing.T) {
19071907
assert.Equal(t, "AT_RISK", response["status"])
19081908
})
19091909
}
1910+
1911+
func Test_ListProjectStatusUpdates_NegativePerPage(t *testing.T) {
1912+
serverTool := ListProjectStatusUpdates(translations.NullTranslationHelper)
1913+
1914+
// With a negative per_page, the handler should clamp to MaxProjectsPerPage (50)
1915+
mockedClient := githubv4mock.NewMockedHTTPClient(
1916+
githubv4mock.NewQueryMatcher(
1917+
statusUpdatesUserQuery{},
1918+
map[string]any{
1919+
"owner": githubv4.String("octocat"),
1920+
"projectNumber": githubv4.Int(1),
1921+
"first": githubv4.Int(50), // clamped to default
1922+
"after": (*githubv4.String)(nil),
1923+
},
1924+
githubv4mock.DataResponse(map[string]any{
1925+
"user": map[string]any{
1926+
"projectV2": map[string]any{
1927+
"statusUpdates": map[string]any{
1928+
"nodes": []map[string]any{
1929+
{
1930+
"id": "SU_neg",
1931+
"body": "Negative per_page test",
1932+
"status": "ON_TRACK",
1933+
"createdAt": "2026-01-15T10:00:00Z",
1934+
"startDate": "2026-01-01",
1935+
"targetDate": "2026-03-01",
1936+
"creator": map[string]any{"login": "octocat"},
1937+
},
1938+
},
1939+
"pageInfo": map[string]any{
1940+
"hasNextPage": false,
1941+
"hasPreviousPage": false,
1942+
"startCursor": "cursor1",
1943+
"endCursor": "cursor1",
1944+
},
1945+
},
1946+
},
1947+
},
1948+
}),
1949+
),
1950+
)
1951+
1952+
client := githubv4.NewClient(mockedClient)
1953+
deps := BaseDeps{
1954+
GQLClient: client,
1955+
}
1956+
handler := serverTool.Handler(deps)
1957+
request := createMCPRequest(map[string]any{
1958+
"owner": "octocat",
1959+
"owner_type": "user",
1960+
"project_number": float64(1),
1961+
"per_page": float64(-5),
1962+
})
1963+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
1964+
1965+
require.NoError(t, err)
1966+
require.False(t, result.IsError)
1967+
1968+
textContent := getTextResult(t, result)
1969+
var response map[string]any
1970+
err = json.Unmarshal([]byte(textContent.Text), &response)
1971+
require.NoError(t, err)
1972+
updates, ok := response["statusUpdates"].([]any)
1973+
require.True(t, ok)
1974+
assert.Len(t, updates, 1)
1975+
}
1976+
1977+
func Test_CreateProjectStatusUpdate_InvalidOwnerType(t *testing.T) {
1978+
serverTool := CreateProjectStatusUpdate(translations.NullTranslationHelper)
1979+
1980+
mockedClient := githubv4mock.NewMockedHTTPClient()
1981+
client := githubv4.NewClient(mockedClient)
1982+
deps := BaseDeps{
1983+
GQLClient: client,
1984+
}
1985+
handler := serverTool.Handler(deps)
1986+
request := createMCPRequest(map[string]any{
1987+
"owner": "octocat",
1988+
"owner_type": "invalid",
1989+
"project_number": float64(2),
1990+
"body": "Test",
1991+
})
1992+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
1993+
1994+
require.NoError(t, err)
1995+
require.True(t, result.IsError)
1996+
1997+
textContent := getTextResult(t, result)
1998+
assert.Contains(t, textContent.Text, `invalid owner_type "invalid"`)
1999+
}
2000+
2001+
func Test_ListProjectStatusUpdates_InvalidOwnerType(t *testing.T) {
2002+
serverTool := ListProjectStatusUpdates(translations.NullTranslationHelper)
2003+
2004+
mockedClient := githubv4mock.NewMockedHTTPClient()
2005+
client := githubv4.NewClient(mockedClient)
2006+
deps := BaseDeps{
2007+
GQLClient: client,
2008+
}
2009+
handler := serverTool.Handler(deps)
2010+
request := createMCPRequest(map[string]any{
2011+
"owner": "octocat",
2012+
"owner_type": "invalid",
2013+
"project_number": float64(1),
2014+
})
2015+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
2016+
2017+
require.NoError(t, err)
2018+
require.True(t, result.IsError)
2019+
2020+
textContent := getTextResult(t, result)
2021+
assert.Contains(t, textContent.Text, `invalid owner_type "invalid"`)
2022+
}

0 commit comments

Comments
 (0)