refactor(pull-requests): simplify compatibility validation

Remove normalization and test assertions that only pinned which invalid argument wins. Keep per-field validation, accepted coercions, and response-shape behavior.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Author: SamMorrowDrums

pkg/github/consolidated_pull_request_types.go

@@ -495,23 +495,9 @@ func normalizePullRequestArguments(kind string) func(json.RawMessage) (json.RawM
 		stringFields := spec.strings
 		boolFields := spec.bools
 		stringArrayFields := spec.stringArrays
-		if kind == "create" {
-			stringFields = []string{"title", "head", "base"}
-			boolFields = []string{"draft", "maintainer_can_modify"}
-			stringArrayFields = []string{"reviewers"}
-		}
-		if kind == "update" {
-			boolFields = []string{"maintainer_can_modify"}
-		}
 		if kind == "reply" {
 			stringFields = []string{"body", "reaction"}
 		}
-		if kind == "update" {
-			// The update handler checks draft before title/body/state/base.
-			if _, err := OptionalParam[bool](args, "draft"); err != nil {
-				return nil, err
-			}
-		}
 		if kind == "create" || kind == "update" {
 			// This flag is inspected from the raw call only to control form
 			// deferral; legacy handlers never type-validated it.
@@ -524,38 +510,6 @@ func normalizePullRequestArguments(kind string) func(json.RawMessage) (json.RawM
 				return nil, err
 			}
 		}
-		if kind == "create" {
-			missingRequired := ""
-			for _, field := range []string{"title", "head", "base"} {
-				if args[field] == "" && missingRequired == "" {
-					missingRequired = field
-				}
-			}
-			if missingRequired != "" {
-				// The handler may defer to the form before checking these later
-				// fields. Drop malformed values so strict typed decoding cannot
-				// change the earlier missing-field result.
-				if _, err := OptionalParam[string](args, "body"); err != nil {
-					delete(args, "body")
-				}
-				for _, field := range []string{"draft", "maintainer_can_modify", "_ui_submitted"} {
-					if _, err := OptionalParam[bool](args, field); err != nil {
-						delete(args, field)
-					}
-				}
-				if _, exists := args["reviewers"]; exists {
-					if _, err := OptionalStringArrayParam(args, "reviewers"); err != nil {
-						delete(args, "reviewers")
-					}
-				}
-				boolFields = nil
-				stringArrayFields = nil
-			} else {
-				if _, err := OptionalParam[string](args, "body"); err != nil {
-					return nil, err
-				}
-			}
-		}
 		for _, field := range boolFields {
 			if _, err := OptionalParam[bool](args, field); err != nil {
 				return nil, err
@@ -584,27 +538,12 @@ func normalizePullRequestArguments(kind string) func(json.RawMessage) (json.RawM
 		if spec.methods != nil && !slices.Contains(spec.methods, args["method"].(string)) {
 			return nil, fmt.Errorf("unknown method: %s", args["method"])
 		}
-		if kind == "read" {
-			if args["method"] == "get_review_comments" {
-				if _, err := OptionalParam[string](args, "after"); err != nil {
-					return nil, err
-				}
-			} else {
-				delete(args, "after")
-			}
-		}
 		if kind == "reply" {
 			_, hasBody := args["body"]
 			_, hasReaction := args["reaction"]
 			if !hasBody && !hasReaction {
 				return nil, fmt.Errorf("at least one of body or reaction is required")
 			}
-			if hasBody && args["body"] == "" {
-				return nil, fmt.Errorf("body cannot be empty when provided")
-			}
-			if hasReaction && args["reaction"] == "" {
-				return nil, fmt.Errorf("reaction cannot be empty when provided")
-			}
 			// pullNumber is only read, and validated, when a reply body is sent.
 			if value, exists := args["pullNumber"]; exists {
 				pullNumber, err := toInt(value)
@@ -617,6 +556,12 @@ func normalizePullRequestArguments(kind string) func(json.RawMessage) (json.RawM
 					delete(args, "pullNumber")
 				}
 			}
+			if hasBody && args["body"] == "" {
+				return nil, fmt.Errorf("body cannot be empty when provided")
+			}
+			if hasReaction && args["reaction"] == "" {
+				return nil, fmt.Errorf("reaction cannot be empty when provided")
+			}
 		}
 		for _, field := range spec.ignoredStrings {
 			if _, isString := args[field].(string); !isString {

pkg/github/typed_consolidated_pull_request_outputs_test.go

@@ -278,7 +278,6 @@ func TestTypedConsolidatedPullRequestOutputs(t *testing.T) {
 	cases := []typedPullRequestCase{
 		{"pull_request_read", map[string]any{"method": "get", "pullNumber": "1"}, minimalPR},
 		{"pull_request_read", map[string]any{"method": "get", "pullNumber": 3}, `null`},
-		{"pull_request_read", map[string]any{"method": "get", "pullNumber": 1, "after": 1}, minimalPR},
 		{"pull_request_read", map[string]any{"method": "get_diff", "pullNumber": 1.0}, typedPRDiff},
 		{"pull_request_read", map[string]any{"method": "get_status", "pullNumber": 1}, `{"state":"success","sha":"abc","total_count":1,"statuses":[{"state":"success","context":"ci","target_url":"https://ci"}]}`},
 		{"pull_request_read", map[string]any{"method": "get_files", "pullNumber": 1, "page": "1", "perPage": 5}, `[{"filename":"file","status":"modified","additions":1,"changes":1,"patch":"+added"}]`},
@@ -409,9 +408,8 @@ func TestTypedConsolidatedPullRequestErrors(t *testing.T) {
 		{"pull_request_read", map[string]any{"method": "unknown", "pullNumber": 1}, "unknown method: unknown"},
 		{"pull_request_read", map[string]any{"method": "get", "pullNumber": 1.5}, "parameter pullNumber is not a valid number: non-integer numeric value: 1.5"},
 		{"create_pull_request", map[string]any{"title": "Subject", "head": "feature", "base": "main", "draft": "yes"}, "parameter draft is not of type bool, is string"},
-		{"create_pull_request", map[string]any{"head": "feature", "base": "main", "draft": "yes"}, "missing required parameter: title"},
 		{"update_pull_request", map[string]any{"pullNumber": 0, "title": "Subject"}, "missing required parameter: pullNumber"},
-		{"update_pull_request", map[string]any{"pullNumber": 1, "draft": "yes", "title": 1}, "parameter draft is not of type bool, is string"},
+		{"update_pull_request", map[string]any{"pullNumber": 1, "draft": "yes"}, "parameter draft is not of type bool, is string"},
 		{"pull_request_review_write", map[string]any{"method": "nope", "pullNumber": 1}, "unknown method: nope"},
 		{"add_comment_to_pending_review", map[string]any{"pullNumber": 1, "body": "nit", "subjectType": "LINE"}, "missing required parameter: path"},
 		{"add_reply_to_pull_request_comment", map[string]any{"commentId": 0, "body": "reply"}, "missing required parameter: commentId"},
@@ -427,7 +425,6 @@ func TestTypedConsolidatedPullRequestErrors(t *testing.T) {
 		{"add_reply_to_pull_request_comment", map[string]any{"commentId": 42, "body": "reply"}, "missing required parameter: pullNumber"},
 		{"add_reply_to_pull_request_comment", map[string]any{"commentId": "1e30", "body": "reply"}, "parameter commentId is not a valid number: numeric value 1e+30 is too large to fit in int64"},
 		{"add_reply_to_pull_request_comment", map[string]any{"commentId": 42, "pullNumber": "x", "body": "reply"}, "parameter pullNumber is not a valid number: invalid numeric value: x"},
-		{"add_reply_to_pull_request_comment", map[string]any{"commentId": 42, "pullNumber": "x", "body": ""}, "body cannot be empty when provided"},
 		{"add_reply_to_pull_request_comment", map[string]any{"commentId": 42, "pullNumber": "x", "body": "reply", "reaction": 1}, "parameter reaction is not of type string, is float64"},
 	}
 	for _, protocol := range typedPullRequestProtocols {