diff --git a/README.md b/README.md index dff62321b8..4494114802 100644 --- a/README.md +++ b/README.md @@ -860,7 +860,7 @@ The following sets of tools are available: - `body`: Issue body content (string, optional) - `duplicate_of`: Issue number that this issue is a duplicate of. Only used when state_reason is 'duplicate'. (number, optional) - `issue_number`: Issue number to update (number, optional) - - `labels`: Labels to apply to this issue (string[], optional) + - `labels`: Labels to apply to this issue ([], optional) - `method`: Write operation to perform on a single issue. Options are: - 'create' - creates a new issue. diff --git a/docs/feature-flags.md b/docs/feature-flags.md index 63fb28dc44..e0f7a96d39 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -57,7 +57,7 @@ runtime behavior (such as output formatting) won't appear here. - `body`: Issue body content (string, optional) - `duplicate_of`: Issue number that this issue is a duplicate of. Only used when state_reason is 'duplicate'. (number, optional) - `issue_number`: Issue number to update (number, optional) - - `labels`: Labels to apply to this issue (string[], optional) + - `labels`: Labels to apply to this issue ([], optional) - `method`: Write operation to perform on a single issue. Options are: - 'create' - creates a new issue. @@ -80,7 +80,7 @@ runtime behavior (such as output formatting) won't appear here. - `duplicate_of`: Issue number that this issue is a duplicate of. Only used when state_reason is 'duplicate'. (number, optional) - `issue_fields`: Issue field values to set or clear. Each item requires 'field_name' and exactly one of 'value', 'field_option_name', or 'delete: true'. (object[], optional) - `issue_number`: Issue number to update (number, optional) - - `labels`: Labels to apply to this issue (string[], optional) + - `labels`: Labels to apply to this issue ([], optional) - `method`: Write operation to perform on a single issue. Options are: - 'create' - creates a new issue. diff --git a/docs/insiders-features.md b/docs/insiders-features.md index 881030f020..c270289135 100644 --- a/docs/insiders-features.md +++ b/docs/insiders-features.md @@ -51,7 +51,7 @@ The list below is generated from the Go source. It covers tool **inventory and s - `body`: Issue body content (string, optional) - `duplicate_of`: Issue number that this issue is a duplicate of. Only used when state_reason is 'duplicate'. (number, optional) - `issue_number`: Issue number to update (number, optional) - - `labels`: Labels to apply to this issue (string[], optional) + - `labels`: Labels to apply to this issue ([], optional) - `method`: Write operation to perform on a single issue. Options are: - 'create' - creates a new issue. @@ -74,7 +74,7 @@ The list below is generated from the Go source. It covers tool **inventory and s - `duplicate_of`: Issue number that this issue is a duplicate of. Only used when state_reason is 'duplicate'. (number, optional) - `issue_fields`: Issue field values to set or clear. Each item requires 'field_name' and exactly one of 'value', 'field_option_name', or 'delete: true'. (object[], optional) - `issue_number`: Issue number to update (number, optional) - - `labels`: Labels to apply to this issue (string[], optional) + - `labels`: Labels to apply to this issue ([], optional) - `method`: Write operation to perform on a single issue. Options are: - 'create' - creates a new issue. diff --git a/pkg/github/__toolsnaps__/issue_write.snap b/pkg/github/__toolsnaps__/issue_write.snap index a125864f04..12a9753e48 100644 --- a/pkg/github/__toolsnaps__/issue_write.snap +++ b/pkg/github/__toolsnaps__/issue_write.snap @@ -36,7 +36,42 @@ "labels": { "description": "Labels to apply to this issue", "items": { - "type": "string" + "oneOf": [ + { + "description": "Label name", + "type": "string" + }, + { + "properties": { + "confidence": { + "description": "How confident you are in this choice. Use 'high' for clear signal or explicit user request, 'medium' for reasonable inference with some ambiguity, 'low' for best guess with limited signal.", + "enum": [ + "low", + "medium", + "high" + ], + "type": "string" + }, + "is_suggestion": { + "description": "If true, this value is sent to the API as a suggestion rather than an applied value. Whether it is applied or recorded as a proposal is determined by the API. Only honored when updating an existing issue.", + "type": "boolean" + }, + "name": { + "description": "Label name", + "type": "string" + }, + "rationale": { + "description": "A concise explanation of what specifically about the issue led you to this choice. State the concrete signal (e.g. 'Reports a crash when saving' → bug).", + "maxLength": 280, + "type": "string" + } + }, + "required": [ + "name" + ], + "type": "object" + } + ] }, "type": "array" }, diff --git a/pkg/github/__toolsnaps__/issue_write_ff_remote_mcp_issue_fields.snap b/pkg/github/__toolsnaps__/issue_write_ff_remote_mcp_issue_fields.snap index 6fb00d2490..891436e96e 100644 --- a/pkg/github/__toolsnaps__/issue_write_ff_remote_mcp_issue_fields.snap +++ b/pkg/github/__toolsnaps__/issue_write_ff_remote_mcp_issue_fields.snap @@ -72,7 +72,42 @@ "labels": { "description": "Labels to apply to this issue", "items": { - "type": "string" + "oneOf": [ + { + "description": "Label name", + "type": "string" + }, + { + "properties": { + "confidence": { + "description": "How confident you are in this choice. Use 'high' for clear signal or explicit user request, 'medium' for reasonable inference with some ambiguity, 'low' for best guess with limited signal.", + "enum": [ + "low", + "medium", + "high" + ], + "type": "string" + }, + "is_suggestion": { + "description": "If true, this value is sent to the API as a suggestion rather than an applied value. Whether it is applied or recorded as a proposal is determined by the API. Only honored when updating an existing issue.", + "type": "boolean" + }, + "name": { + "description": "Label name", + "type": "string" + }, + "rationale": { + "description": "A concise explanation of what specifically about the issue led you to this choice. State the concrete signal (e.g. 'Reports a crash when saving' → bug).", + "maxLength": 280, + "type": "string" + } + }, + "required": [ + "name" + ], + "type": "object" + } + ] }, "type": "array" }, diff --git a/pkg/github/granular_tools_test.go b/pkg/github/granular_tools_test.go index eb688a0b9f..ecb54d4217 100644 --- a/pkg/github/granular_tools_test.go +++ b/pkg/github/granular_tools_test.go @@ -429,7 +429,7 @@ func TestGranularUpdateIssueLabelsInvalidRationale(t *testing.T) { map[string]any{"name": "bug", "rationale": strings.Repeat("a", 281)}, }, }, - expectedErrText: "label rationale must be 280 characters or less", + expectedErrText: "rationale must be 280 characters or less", }, { name: "label object missing name", @@ -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", }, } diff --git a/pkg/github/issues.go b/pkg/github/issues.go index ef9bbc4305..3c9b06a3e0 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -1858,7 +1858,19 @@ Options are: Type: "array", Description: "Labels to apply to this issue", Items: &jsonschema.Schema{ - Type: "string", + OneOf: []*jsonschema.Schema{ + {Type: "string", Description: "Label name"}, + { + Type: "object", + Properties: withIntentProperties(map[string]*jsonschema.Schema{ + "name": { + Type: "string", + Description: "Label name", + }, + }), + Required: []string{"name"}, + }, + }, }, }, "milestone": { @@ -1975,13 +1987,11 @@ Options are: assigneesValue, assigneesProvided := args["assignees"] assigneesProvided = assigneesProvided && assigneesValue != nil - // Get labels - labels, err := OptionalStringArrayParam(args, "labels") + // Get labels (plain names or per-label intent objects) + labels, labelsPayload, labelsHaveIntent, labelsProvided, err := parseParamWithIntent(args, "labels", "name") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - labelsValue, labelsProvided := args["labels"] - labelsProvided = labelsProvided && labelsValue != nil // Get optional milestone milestone, err := OptionalIntParam(args, "milestone") @@ -2053,10 +2063,14 @@ Options are: if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, assignees, labels, milestoneNum, issueType, issueFieldValues, fieldIDsToDelete, state, stateReason, duplicateOf, UpdateIssueOptions{ + updateOpts := UpdateIssueOptions{ AssigneesProvided: assigneesProvided, LabelsProvided: labelsProvided, - }) + } + if labelsHaveIntent { + updateOpts.LabelsWithIntent = labelsPayload + } + result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, assignees, labels, milestoneNum, issueType, issueFieldValues, fieldIDsToDelete, state, stateReason, duplicateOf, updateOpts) return result, nil, err default: return utils.NewToolResultError("invalid method, must be either 'create' or 'update'"), nil, nil @@ -2132,7 +2146,19 @@ Options are: Type: "array", Description: "Labels to apply to this issue", Items: &jsonschema.Schema{ - Type: "string", + OneOf: []*jsonschema.Schema{ + {Type: "string", Description: "Label name"}, + { + Type: "object", + Properties: withIntentProperties(map[string]*jsonschema.Schema{ + "name": { + Type: "string", + Description: "Label name", + }, + }), + Required: []string{"name"}, + }, + }, }, }, "milestone": { @@ -2214,13 +2240,11 @@ Options are: assigneesValue, assigneesProvided := args["assignees"] assigneesProvided = assigneesProvided && assigneesValue != nil - // Get labels - labels, err := OptionalStringArrayParam(args, "labels") + // Get labels (plain names or per-label intent objects) + labels, labelsPayload, labelsHaveIntent, labelsProvided, err := parseParamWithIntent(args, "labels", "name") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - labelsValue, labelsProvided := args["labels"] - labelsProvided = labelsProvided && labelsValue != nil // Get optional milestone milestone, err := OptionalIntParam(args, "milestone") @@ -2277,10 +2301,14 @@ Options are: if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, assignees, labels, milestoneNum, issueType, nil, nil, state, stateReason, duplicateOf, UpdateIssueOptions{ + updateOpts := UpdateIssueOptions{ AssigneesProvided: assigneesProvided, LabelsProvided: labelsProvided, - }) + } + if labelsHaveIntent { + updateOpts.LabelsWithIntent = labelsPayload + } + result, err := UpdateIssue(ctx, client, gqlClient, owner, repo, issueNumber, title, body, assignees, labels, milestoneNum, issueType, nil, nil, state, stateReason, duplicateOf, updateOpts) return result, nil, err default: return utils.NewToolResultError("invalid method, must be either 'create' or 'update'"), nil, nil @@ -2350,6 +2378,28 @@ type UpdateIssueOptions struct { AssigneesProvided bool // 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 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 +} + +// 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 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) + if err != nil { + return nil, err + } + payload := map[string]any{} + if err := json.Unmarshal(data, &payload); err != nil { + return nil, err + } + payload["labels"] = labels + return payload, nil } func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4.Client, owner string, repo string, issueNumber int, title string, body string, assignees []string, labels []string, milestoneNum int, issueType string, issueFieldValues []*github.IssueRequestFieldValue, fieldIDsToDelete []int64, state string, stateReason string, duplicateOf int, opts ...UpdateIssueOptions) (*mcp.CallToolResult, error) { @@ -2360,6 +2410,9 @@ func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4 for _, opt := range opts { updateOptions.AssigneesProvided = updateOptions.AssigneesProvided || opt.AssigneesProvided updateOptions.LabelsProvided = updateOptions.LabelsProvided || opt.LabelsProvided + if len(opt.LabelsWithIntent) > 0 { + updateOptions.LabelsWithIntent = opt.LabelsWithIntent + } } // Create the issue request with only provided fields @@ -2374,7 +2427,9 @@ func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4 issueRequest.Body = github.Ptr(body) } - if updateOptions.LabelsProvided { + // When labels carry per-label intent, they are sent via a custom request + // below instead of through issueRequest.Labels. + if updateOptions.LabelsProvided && len(updateOptions.LabelsWithIntent) == 0 { issueRequest.Labels = &labels } @@ -2415,7 +2470,27 @@ func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4 issueRequest.IssueFieldValues = merged } - updatedIssue, resp, err := client.Issues.Edit(ctx, owner, repo, issueNumber, issueRequest) + var updatedIssue *github.Issue + var resp *github.Response + var err error + if len(updateOptions.LabelsWithIntent) > 0 { + // Send labels in object form so per-label rationale and suggestion intent + // are preserved. Marshal the standard request (labels omitted), then inject + // the object-form labels into the payload. + payload, mErr := issueRequestWithLabels(issueRequest, updateOptions.LabelsWithIntent) + if mErr != nil { + return utils.NewToolResultErrorFromErr("failed to build issue update request", mErr), nil + } + apiURL := fmt.Sprintf("repos/%s/%s/issues/%d", owner, repo, issueNumber) + httpReq, rErr := client.NewRequest(ctx, "PATCH", apiURL, payload) + if rErr != nil { + return utils.NewToolResultErrorFromErr("failed to create request", rErr), nil + } + updatedIssue = &github.Issue{} + resp, err = client.Do(httpReq, updatedIssue) + } else { + updatedIssue, resp, err = client.Issues.Edit(ctx, owner, repo, issueNumber, issueRequest) + } if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to update issue", diff --git a/pkg/github/issues_granular.go b/pkg/github/issues_granular.go index 22d26cc47f..7ff97a9b48 100644 --- a/pkg/github/issues_granular.go +++ b/pkg/github/issues_granular.go @@ -258,22 +258,187 @@ func GranularUpdateIssueAssignees(t translations.TranslationHelperFunc) inventor ) } -// labelWithIntent represents the object form of a label entry, allowing a -// rationale, confidence level, and/or suggest flag to be sent alongside the label name. -type labelWithIntent struct { - Name string `json:"name"` +// maxIntentRationaleLength bounds the free-text rationale an agent may attach to +// a value it writes to an issue. +const maxIntentRationaleLength = 280 + +// valueIntent holds the optional reasoning metadata an agent can attach to a +// value it writes to an issue: a free-text rationale, a confidence level, and a +// flag marking the value as a suggestion rather than an applied value. It is +// embedded into the object form of a written value (today labels; issue types +// and field values in future) so the same metadata travels with each value on +// the wire. +type valueIntent struct { Rationale string `json:"rationale,omitempty"` Confidence string `json:"confidence,omitempty"` Suggest bool `json:"suggest,omitempty"` } +// HasIntent reports whether any intent metadata was supplied. +func (v valueIntent) HasIntent() bool { + return v.Rationale != "" || v.Confidence != "" || v.Suggest +} + +// parseValueIntent extracts the shared intent metadata (rationale, confidence, +// is_suggestion) from the object form of a written value. It trims and +// length-checks the rationale, validates confidence against the allowed levels, +// and reports whether any intent field was present. +func parseValueIntent(obj map[string]any) (valueIntent, bool, error) { + rationale, err := OptionalParam[string](obj, "rationale") + if err != nil { + return valueIntent{}, false, err + } + rationale = strings.TrimSpace(rationale) + if len([]rune(rationale)) > maxIntentRationaleLength { + return valueIntent{}, false, fmt.Errorf("rationale must be %d characters or less", maxIntentRationaleLength) + } + + confidence, err := OptionalParam[string](obj, "confidence") + if err != nil { + return valueIntent{}, false, err + } + switch confidence { + case "", "low", "medium", "high": + default: + return valueIntent{}, false, fmt.Errorf("confidence must be one of: low, medium, high") + } + + suggest, err := OptionalParam[bool](obj, "is_suggestion") + if err != nil { + return valueIntent{}, false, err + } + + intent := valueIntent{Rationale: rationale, Confidence: confidence, Suggest: suggest} + return intent, intent.HasIntent(), nil +} + +// intentSchemaProperties returns the shared schema properties (rationale, +// confidence, is_suggestion) describing the reasoning metadata an agent can +// attach to a written value. The same properties are reused across value types +// (today labels; issue types and field values in future). +func intentSchemaProperties() map[string]*jsonschema.Schema { + return map[string]*jsonschema.Schema{ + "rationale": { + Type: "string", + Description: "A concise explanation of what specifically about the issue led you to this choice. " + + "State the concrete signal (e.g. 'Reports a crash when saving' → bug).", + MaxLength: jsonschema.Ptr(maxIntentRationaleLength), + }, + "confidence": { + Type: "string", + Description: "How confident you are in this choice. Use 'high' for clear signal or explicit user request, 'medium' for reasonable inference with some ambiguity, 'low' for best guess with limited signal.", + Enum: []any{"low", "medium", "high"}, + }, + "is_suggestion": { + Type: "boolean", + Description: "If true, this value is sent to the API as a suggestion rather than an applied value. " + + "Whether it is applied or recorded as a proposal is determined by the API. Only honored when updating an existing issue.", + }, + } +} + +// withIntentProperties merges the shared intent schema properties into props and +// returns props, so a value's object schema (e.g. a label's {name}) can offer +// the same rationale/confidence/is_suggestion fields. +func withIntentProperties(props map[string]*jsonschema.Schema) map[string]*jsonschema.Schema { + maps.Copy(props, intentSchemaProperties()) + return props +} + +// 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( @@ -357,62 +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 - } - rationale, err := OptionalParam[string](v, "rationale") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } - rationale = strings.TrimSpace(rationale) - if len([]rune(rationale)) > 280 { - return utils.NewToolResultError("label rationale must be 280 characters or less"), nil, nil - } - confidence, err := OptionalParam[string](v, "confidence") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } - if confidence != "" && confidence != "low" && confidence != "medium" && confidence != "high" { - return utils.NewToolResultError("confidence must be one of: low, medium, high"), nil, nil - } - isSuggestion, err := OptionalParam[bool](v, "is_suggestion") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } - if rationale == "" && !isSuggestion && confidence == "" { - payload = append(payload, name) - } else { - useObjectForm = true - payload = append(payload, labelWithIntent{Name: name, Rationale: rationale, Confidence: confidence, Suggest: isSuggestion}) - } - 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) @@ -425,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} } diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 7e47cdb527..e352e8b087 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -3474,6 +3474,158 @@ func Test_LegacyUpdateIssueClearsLabelsAndAssignees(t *testing.T) { assert.Equal(t, updatedIssue.GetHTMLURL(), updateResp.URL) } +func Test_IssueWrite_UpdateLabelsWithIntent(t *testing.T) { + serverTool := IssueWrite(translations.NullTranslationHelper) + updatedIssue := &github.Issue{ + Number: github.Ptr(1), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/1"), + } + + tests := []struct { + name string + labels any + expectedReq map[string]any + }{ + { + name: "plain label names sent as strings", + labels: []any{"bug", "triage"}, + expectedReq: map[string]any{ + "labels": []any{"bug", "triage"}, + }, + }, + { + name: "suggested label without rationale", + labels: []any{ + map[string]any{"name": "bug", "is_suggestion": true}, + }, + expectedReq: map[string]any{ + "labels": []any{ + map[string]any{"name": "bug", "suggest": true}, + }, + }, + }, + { + name: "applied label with rationale", + labels: []any{ + map[string]any{"name": "bug", "rationale": "Reports a crash when saving"}, + }, + expectedReq: map[string]any{ + "labels": []any{ + map[string]any{"name": "bug", "rationale": "Reports a crash when saving"}, + }, + }, + }, + { + name: "suggested label with rationale and confidence", + labels: []any{ + map[string]any{"name": "bug", "rationale": "Reports a crash when saving", "confidence": "high", "is_suggestion": true}, + }, + expectedReq: map[string]any{ + "labels": []any{ + map[string]any{"name": "bug", "rationale": "Reports a crash when saving", "confidence": "high", "suggest": true}, + }, + }, + }, + { + name: "mix of plain, applied-with-rationale, and suggested labels", + labels: []any{ + "triage", + map[string]any{"name": "bug", "rationale": "Reports a crash when saving"}, + map[string]any{"name": "needs-design", "is_suggestion": true}, + }, + expectedReq: map[string]any{ + "labels": []any{ + "triage", + map[string]any{"name": "bug", "rationale": "Reports a crash when saving"}, + map[string]any{"name": "needs-design", "suggest": true}, + }, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ + PatchReposIssuesByOwnerByRepoByIssueNumber: expectRequestBody(t, tc.expectedReq). + andThen(mockResponse(t, http.StatusOK, updatedIssue)), + })) + deps := BaseDeps{ + Client: client, + GQLClient: githubv4.NewClient(githubv4mock.NewMockedHTTPClient()), + } + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "method": "update", + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "labels": tc.labels, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + if result.IsError { + t.Fatalf("unexpected error result: %s", getErrorResult(t, result).Text) + } + }) + } +} + +func Test_IssueWrite_UpdateLabelsWithIntentErrors(t *testing.T) { + serverTool := IssueWrite(translations.NullTranslationHelper) + + tests := []struct { + name string + labels any + expectedErrText string + }{ + { + name: "rationale too long", + labels: []any{ + map[string]any{"name": "bug", "rationale": strings.Repeat("a", 281)}, + }, + expectedErrText: "rationale must be 280 characters or less", + }, + { + name: "invalid confidence value", + labels: []any{ + map[string]any{"name": "bug", "confidence": "very_high"}, + }, + expectedErrText: "confidence must be one of: low, medium, high", + }, + { + name: "label object missing name", + labels: []any{ + map[string]any{"rationale": "no name provided"}, + }, + expectedErrText: "each labels object must have a 'name' string", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + deps := BaseDeps{ + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(nil)), + GQLClient: githubv4.NewClient(githubv4mock.NewMockedHTTPClient()), + } + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "method": "update", + "owner": "owner", + "repo": "repo", + "issue_number": float64(1), + "labels": tc.labels, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + require.NoError(t, err) + + errorContent := getErrorResult(t, result) + assert.Contains(t, errorContent.Text, tc.expectedErrText) + }) + } +} + func Test_ParseISOTimestamp(t *testing.T) { tests := []struct { name string