make the parser fully agnostic of type

Author: iulia-b

pkg/github/granular_tools_test.go

@@ -441,7 +441,7 @@ func TestGranularUpdateIssueLabelsInvalidRationale(t *testing.T) {
 					map[string]any{"rationale": "no name provided"},
 				},
 			},
-			expectedErrText: "each label object must have a 'name' string",
+			expectedErrText: "each labels object must have a 'name' string",
 		},
 	}
 

pkg/github/issues.go

@@ -1988,7 +1988,7 @@ Options are:
 			assigneesProvided = assigneesProvided && assigneesValue != nil
 
 			// Get labels (plain names or per-label intent objects)
-			labels, labelsPayload, labelsHaveIntent, labelsProvided, err := parseIssueWriteLabels(args)
+			labels, labelsPayload, labelsHaveIntent, labelsProvided, err := parseParamWithIntent(args, "labels", "name")
 			if err != nil {
 				return utils.NewToolResultError(err.Error()), nil, nil
 			}
@@ -2241,7 +2241,7 @@ Options are:
 			assigneesProvided = assigneesProvided && assigneesValue != nil
 
 			// Get labels (plain names or per-label intent objects)
-			labels, labelsPayload, labelsHaveIntent, labelsProvided, err := parseIssueWriteLabels(args)
+			labels, labelsPayload, labelsHaveIntent, labelsProvided, err := parseParamWithIntent(args, "labels", "name")
 			if err != nil {
 				return utils.NewToolResultError(err.Error()), nil, nil
 			}
@@ -2379,71 +2379,15 @@ type UpdateIssueOptions struct {
 	// LabelsProvided sends the labels field even when the slice is empty.
 	LabelsProvided bool
 	// LabelsWithIntent, when non-empty, sends labels in object form (a mix of
-	// plain label names and labelWithIntent objects) via a custom request so
+	// plain label names and valueWithIntent objects) via a custom request so
 	// per-label rationale and suggestion intent are preserved. When set, it
 	// takes precedence over the labels slice.
 	LabelsWithIntent []any
 }
 
-// parseIssueWriteLabels parses the issue_write "labels" parameter, which accepts
-// either plain label names (strings) or objects carrying intent metadata
-// (name, rationale, confidence, is_suggestion). It returns the plain label names
-// (intent stripped), the object-form payload to send when any label carries
-// intent, whether any label carried intent, whether the labels parameter was
-// provided at all, and an error.
-func parseIssueWriteLabels(args map[string]any) (names []string, payload []any, hasIntent bool, provided bool, err error) {
-	raw, ok := args["labels"]
-	if !ok || raw == nil {
-		return []string{}, nil, false, false, nil
-	}
-
-	var labelsSlice []any
-	switch v := raw.(type) {
-	case []any:
-		labelsSlice = v
-	case []string:
-		labelsSlice = make([]any, len(v))
-		for i, s := range v {
-			labelsSlice[i] = s
-		}
-	default:
-		return nil, nil, false, true, fmt.Errorf("labels must be an array")
-	}
-
-	names = make([]string, 0, len(labelsSlice))
-	payload = make([]any, 0, len(labelsSlice))
-	for _, item := range labelsSlice {
-		switch v := item.(type) {
-		case string:
-			names = append(names, v)
-			payload = append(payload, v)
-		case map[string]any:
-			name, nameErr := RequiredParam[string](v, "name")
-			if nameErr != nil {
-				return nil, nil, false, true, fmt.Errorf("each label object must have a 'name' string")
-			}
-			names = append(names, name)
-
-			intent, itemHasIntent, intentErr := parseValueIntent(v)
-			if intentErr != nil {
-				return nil, nil, false, true, intentErr
-			}
-			if itemHasIntent {
-				hasIntent = true
-				payload = append(payload, labelWithIntent{Name: name, valueIntent: intent})
-			} else {
-				payload = append(payload, name)
-			}
-		default:
-			return nil, nil, false, true, fmt.Errorf("each label must be a string or an object with 'name' and optional 'rationale', 'confidence', and/or 'is_suggestion'")
-		}
-	}
-	return names, payload, hasIntent, true, nil
-}
-
 // issueRequestWithLabels marshals an IssueRequest into a generic map and sets
 // the labels field to the provided object-form payload (a mix of plain label
-// names and labelWithIntent objects). This lets an issue update carry per-label
+// names and valueWithIntent objects). This lets an issue update carry per-label
 // rationale and suggestion intent that github.IssueRequest cannot represent.
 func issueRequestWithLabels(issueRequest *github.IssueRequest, labels []any) (map[string]any, error) {
 	data, err := json.Marshal(issueRequest)

pkg/github/issues_granular.go

@@ -345,20 +345,100 @@ func withIntentProperties(props map[string]*jsonschema.Schema) map[string]*jsons
 	return props
 }
 
-// labelWithIntent represents the object form of a label entry, carrying optional
-// intent metadata (rationale, confidence, suggest) alongside the label name.
-type labelWithIntent struct {
-	Name string `json:"name"`
+// valueWithIntent is the object form of a written value: an identity field whose
+// JSON key varies by value type (e.g. "name" for a label) plus optional intent
+// metadata. It marshals to a single object merging the identity field with the
+// rationale/confidence/suggest fields, so the same structure serves any value
+// type that travels as a named object with intent.
+type valueWithIntent struct {
+	identityKey string
+	identity    string
 	valueIntent
 }
 
+// MarshalJSON renders the value as a single object with the identity field under
+// its configured key alongside the embedded intent metadata.
+func (v valueWithIntent) MarshalJSON() ([]byte, error) {
+	data, err := json.Marshal(v.valueIntent)
+	if err != nil {
+		return nil, err
+	}
+	obj := map[string]any{}
+	if err := json.Unmarshal(data, &obj); err != nil {
+		return nil, err
+	}
+	obj[v.identityKey] = v.identity
+	return json.Marshal(obj)
+}
+
 // labelsUpdateRequest is a custom request body for updating an issue's labels
-// where individual labels may optionally include a rationale. Each element of
-// Labels is either a string (label name) or a labelWithIntent object.
+// where individual labels may optionally include intent metadata. Each element
+// of Labels is either a string (label name) or a valueWithIntent object.
 type labelsUpdateRequest struct {
 	Labels []any `json:"labels"`
 }
 
+// parseParamWithIntent parses an array parameter whose entries are either plain
+// identity strings or objects carrying intent metadata (an identity field named
+// by identityField plus optional rationale, confidence, is_suggestion). param is
+// the argument key (e.g. "labels") and identityField is the identity key within
+// each object (e.g. "name").
+//
+// It returns the plain identities (intent stripped), the wire payload (a mix of
+// plain identity strings and valueWithIntent objects, with intent objects only
+// for entries that carry intent), whether any entry carried intent, whether the
+// parameter was provided at all, and an error. It is reusable across value types
+// that travel as named objects with intent.
+func parseParamWithIntent(args map[string]any, param, identityField string) (identities []string, payload []any, hasIntent bool, provided bool, err error) {
+	raw, ok := args[param]
+	if !ok || raw == nil {
+		return []string{}, nil, false, false, nil
+	}
+
+	var entries []any
+	switch v := raw.(type) {
+	case []any:
+		entries = v
+	case []string:
+		entries = make([]any, len(v))
+		for i, s := range v {
+			entries[i] = s
+		}
+	default:
+		return nil, nil, false, true, fmt.Errorf("%s must be an array", param)
+	}
+
+	identities = make([]string, 0, len(entries))
+	payload = make([]any, 0, len(entries))
+	for _, item := range entries {
+		switch v := item.(type) {
+		case string:
+			identities = append(identities, v)
+			payload = append(payload, v)
+		case map[string]any:
+			identity, identityErr := RequiredParam[string](v, identityField)
+			if identityErr != nil {
+				return nil, nil, false, true, fmt.Errorf("each %s object must have a '%s' string", param, identityField)
+			}
+			identities = append(identities, identity)
+
+			intent, itemHasIntent, intentErr := parseValueIntent(v)
+			if intentErr != nil {
+				return nil, nil, false, true, intentErr
+			}
+			if itemHasIntent {
+				hasIntent = true
+				payload = append(payload, valueWithIntent{identityKey: identityField, identity: identity, valueIntent: intent})
+			} else {
+				payload = append(payload, identity)
+			}
+		default:
+			return nil, nil, false, true, fmt.Errorf("each %s entry must be a string or an object with '%s' and optional 'rationale', 'confidence', and/or 'is_suggestion'", param, identityField)
+		}
+	}
+	return identities, payload, hasIntent, true, nil
+}
+
 // GranularUpdateIssueLabels creates a tool to update an issue's labels.
 func GranularUpdateIssueLabels(t translations.TranslationHelperFunc) inventory.ServerTool {
 	st := NewTool(
@@ -442,47 +522,12 @@ func GranularUpdateIssueLabels(t translations.TranslationHelperFunc) inventory.S
 				return utils.NewToolResultError(err.Error()), nil, nil
 			}
 
-			labelsRaw, ok := args["labels"]
-			if !ok {
-				return utils.NewToolResultError("missing required parameter: labels"), nil, nil
-			}
-			labelsSlice, ok := labelsRaw.([]any)
-			if !ok {
-				// Also accept []string for callers that pre-typed the array.
-				if strs, ok := labelsRaw.([]string); ok {
-					labelsSlice = make([]any, len(strs))
-					for i, s := range strs {
-						labelsSlice[i] = s
-					}
-				} else {
-					return utils.NewToolResultError("parameter labels must be an array"), nil, nil
-				}
+			names, payload, useObjectForm, provided, err := parseParamWithIntent(args, "labels", "name")
+			if err != nil {
+				return utils.NewToolResultError(err.Error()), nil, nil
 			}
-
-			useObjectForm := false
-			payload := make([]any, 0, len(labelsSlice))
-			for _, item := range labelsSlice {
-				switch v := item.(type) {
-				case string:
-					payload = append(payload, v)
-				case map[string]any:
-					name, err := RequiredParam[string](v, "name")
-					if err != nil {
-						return utils.NewToolResultError("each label object must have a 'name' string"), nil, nil
-					}
-					intent, itemHasIntent, err := parseValueIntent(v)
-					if err != nil {
-						return utils.NewToolResultError(err.Error()), nil, nil
-					}
-					if itemHasIntent {
-						useObjectForm = true
-						payload = append(payload, labelWithIntent{Name: name, valueIntent: intent})
-					} else {
-						payload = append(payload, name)
-					}
-				default:
-					return utils.NewToolResultError("each label must be a string or an object with 'name' and optional 'rationale', 'confidence', and/or 'is_suggestion'"), nil, nil
-				}
+			if !provided {
+				return utils.NewToolResultError("missing required parameter: labels"), nil, nil
 			}
 
 			client, err := deps.GetClient(ctx)
@@ -495,10 +540,6 @@ func GranularUpdateIssueLabels(t translations.TranslationHelperFunc) inventory.S
 				body = &labelsUpdateRequest{Labels: payload}
 			} else {
 				// Preserve the standard wire format when no rationale or suggest is supplied.
-				names := make([]string, len(payload))
-				for i, p := range payload {
-					names[i] = p.(string)
-				}
 				body = &github.IssueRequest{Labels: &names}
 			}
 

pkg/github/issues_test.go

@@ -3598,7 +3598,7 @@ func Test_IssueWrite_UpdateLabelsWithIntentErrors(t *testing.T) {
 			labels: []any{
 				map[string]any{"rationale": "no name provided"},
 			},
-			expectedErrText: "each label object must have a 'name' string",
+			expectedErrText: "each labels object must have a 'name' string",
 		},
 	}