diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 07ff6a87f..c1084af0c 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -2293,6 +2293,20 @@ func AddCommentToPendingReview(t translations.TranslationHelperFunc) inventory.S return utils.NewToolResultError(err.Error()), nil, nil } + // WeakDecode leaves an omitted field as its zero value, so without an + // explicit check a missing required argument (e.g. owner) would be + // sent to the GraphQL API and surface as a confusing downstream error + // ("Could not resolve to a Repository ...") instead of a clear + // missing-parameter message. Validate the required arguments up front. + for _, p := range []string{"owner", "repo", "path", "body", "subjectType"} { + if _, err := RequiredParam[string](args, p); err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + } + if _, err := RequiredInt(args, "pullNumber"); err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + client, err := deps.GetGQLClient(ctx) if err != nil { return utils.NewToolResultErrorFromErr("failed to get GitHub GQL client", err), nil, nil diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index 0f372519e..9092c1b8c 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -3575,6 +3575,19 @@ func TestAddPullRequestReviewCommentToPendingReview(t *testing.T) { expectToolError: true, expectedToolErrMsg: "Failed to add comment to pending review", }, + { + name: "missing required owner is reported instead of running the query", + mockedClient: githubv4mock.NewMockedHTTPClient(), + requestArgs: map[string]any{ + "repo": "repo", + "pullNumber": float64(42), + "path": "file.go", + "body": "This is a test comment", + "subjectType": "LINE", + }, + expectToolError: true, + expectedToolErrMsg: "missing required parameter: owner", + }, } for _, tc := range tests {