Skip to content

fix(pull_requests): validate required params in add_comment_to_pending_review#2770

Merged
SamMorrowDrums merged 1 commit into
github:mainfrom
syf2211:fix/2718-pending-review-required-params
Jun 26, 2026
Merged

fix(pull_requests): validate required params in add_comment_to_pending_review#2770
SamMorrowDrums merged 1 commit into
github:mainfrom
syf2211:fix/2718-pending-review-required-params

Conversation

@syf2211

@syf2211 syf2211 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Validate required arguments in add_comment_to_pending_review before calling the GraphQL API, returning clear missing required parameter errors consistent with other pull request tools.

Motivation

When a required argument such as owner was omitted, the handler used mapstructure.WeakDecode and proceeded with zero values. The downstream GraphQL call then failed with a confusing repository-resolution error instead of indicating which argument was missing.

Fixes #2718

Changes

  • Replace mapstructure.WeakDecode in AddCommentToPendingReview with RequiredParam / RequiredInt / OptionalIntParam / OptionalParam
  • Align optional pointer handling with the granular pull request review comment tool
  • Add regression tests for missing owner and path

Tests

go test ./pkg/github/... -run TestAddPullRequestReviewCommentToPendingReview -count=1
./script/lint

All tests pass; lint reports 0 issues.

Notes

The same WeakDecode-without-validation pattern also exists in copilot.go and discussions.go; those are out of scope for this focused fix as noted in the issue.

…g_review

Replace mapstructure.WeakDecode with RequiredParam/RequiredInt checks so
missing arguments like owner return a clear validation error instead of
running the GraphQL query with zero values.

Fixes github#2718
@syf2211 syf2211 requested a review from a team as a code owner June 25, 2026 12:05

@SamMorrowDrums SamMorrowDrums left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, nice work!

@SamMorrowDrums SamMorrowDrums merged commit 5aa8ed3 into github:main Jun 26, 2026
13 checks passed
syf2211 added a commit to syf2211/github-mcp-server that referenced this pull request Jun 27, 2026
…g_review (github#2770)

Replace mapstructure.WeakDecode with RequiredParam/RequiredInt checks so
missing arguments like owner return a clear validation error instead of
running the GraphQL query with zero values.

Fixes github#2718

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants