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 c867e201d..8085ea740 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 @@ -34,6 +34,15 @@ "items": { "additionalProperties": false, "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" + }, "delete": { "description": "Set to true to clear this field's current value on the issue. Cannot be combined with 'value' or 'field_option_name'.", "enum": [ @@ -49,6 +58,15 @@ "description": "Option name for single-select fields. Validated against the field's options before the API call. Cannot be combined with 'value' or 'delete'.", "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" + }, + "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" + }, "value": { "description": "Value to set. Use for text, number, and date fields (date as YYYY-MM-DD). For single-select fields, prefer 'field_option_name' so the option is validated before the API call. Cannot be combined with 'field_option_name' or 'delete'.", "type": [ diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 903bba1ec..6c969fe54 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -45,6 +45,9 @@ type issueWriteFieldInput struct { Value any FieldOptionName string Delete bool + // Intent carries optional rationale/confidence/suggestion metadata for a + // value-setting field. It is ignored for delete entries and on create. + Intent valueIntent } const ( @@ -290,19 +293,30 @@ func optionalIssueWriteFields(args map[string]any) ([]issueWriteFieldInput, erro return nil, fmt.Errorf("issue field %q must specify either value or field_option_name", fieldName) } + intent, _, err := parseValueIntent(itemMap) + if err != nil { + return nil, err + } + issueFields = append(issueFields, issueWriteFieldInput{ FieldName: fieldName, Value: value, FieldOptionName: fieldOptionName, + Intent: intent, }) } return issueFields, nil } -func resolveIssueRequestFieldValues(ctx context.Context, gqlClient *githubv4.Client, owner, repo string, issueFields []issueWriteFieldInput) ([]*github.IssueRequestFieldValue, []int64, error) { +// resolveIssueRequestFieldValues resolves user-friendly field inputs into REST +// IssueRequestFieldValue entries (field IDs and option values resolved). It also +// returns the IDs of fields marked for deletion and a map of field ID to intent +// metadata for fields that carried rationale/confidence/suggestion intent, so the +// caller can send those values in object form. +func resolveIssueRequestFieldValues(ctx context.Context, gqlClient *githubv4.Client, owner, repo string, issueFields []issueWriteFieldInput) ([]*github.IssueRequestFieldValue, []int64, map[int64]valueIntent, error) { if len(issueFields) == 0 { - return nil, nil, nil + return nil, nil, nil, nil } ctxWithFeatures := ghcontext.WithGraphQLFeatures(ctx, "issue_fields", "repo_issue_fields") @@ -312,7 +326,7 @@ func resolveIssueRequestFieldValues(ctx context.Context, gqlClient *githubv4.Cli "repo": githubv4.String(repo), } if err := gqlClient.Query(ctxWithFeatures, &query, vars); err != nil { - return nil, nil, fmt.Errorf("failed to query issue fields metadata: %w", err) + return nil, nil, nil, fmt.Errorf("failed to query issue fields metadata: %w", err) } // Build name → node map, dispatching on concrete type to extract name. @@ -336,10 +350,11 @@ func resolveIssueRequestFieldValues(ctx context.Context, gqlClient *githubv4.Cli resolved := make([]*github.IssueRequestFieldValue, 0, len(issueFields)) var fieldIDsToDelete []int64 + var fieldIntents map[int64]valueIntent for _, fieldInput := range issueFields { node, ok := fieldByName[strings.ToLower(strings.TrimSpace(fieldInput.FieldName))] if !ok { - return nil, nil, fmt.Errorf("issue field %q was not found in %s/%s", fieldInput.FieldName, owner, repo) + return nil, nil, nil, fmt.Errorf("issue field %q was not found in %s/%s", fieldInput.FieldName, owner, repo) } var fullDatabaseIDStr, dataType string @@ -360,7 +375,7 @@ func resolveIssueRequestFieldValues(ctx context.Context, gqlClient *githubv4.Cli fieldID := parseFullDatabaseID(fullDatabaseIDStr) if fieldID == 0 { - return nil, nil, fmt.Errorf("issue field %q is missing fullDatabaseId", fieldInput.FieldName) + return nil, nil, nil, fmt.Errorf("issue field %q is missing fullDatabaseId", fieldInput.FieldName) } if fieldInput.Delete { @@ -371,7 +386,7 @@ func resolveIssueRequestFieldValues(ctx context.Context, gqlClient *githubv4.Cli resolvedValue := fieldInput.Value if fieldInput.FieldOptionName != "" { if !strings.EqualFold(dataType, "single_select") { - return nil, nil, fmt.Errorf("issue field %q is %q, so field_option_name cannot be used", fieldInput.FieldName, dataType) + return nil, nil, nil, fmt.Errorf("issue field %q is %q, so field_option_name cannot be used", fieldInput.FieldName, dataType) } optionFound := false @@ -385,17 +400,24 @@ func resolveIssueRequestFieldValues(ctx context.Context, gqlClient *githubv4.Cli } if !optionFound { - return nil, nil, fmt.Errorf("issue field option %q was not found for field %q", fieldInput.FieldOptionName, fieldInput.FieldName) + return nil, nil, nil, fmt.Errorf("issue field option %q was not found for field %q", fieldInput.FieldOptionName, fieldInput.FieldName) } } + if fieldInput.Intent.HasIntent() { + if fieldIntents == nil { + fieldIntents = make(map[int64]valueIntent) + } + fieldIntents[fieldID] = fieldInput.Intent + } + resolved = append(resolved, &github.IssueRequestFieldValue{ FieldID: fieldID, Value: resolvedValue, }) } - return resolved, fieldIDsToDelete, nil + return resolved, fieldIDsToDelete, fieldIntents, nil } // fetchExistingIssueFieldValues retrieves the current field values for an issue @@ -1922,7 +1944,7 @@ Options are: Items: &jsonschema.Schema{ Type: "object", AdditionalProperties: &jsonschema.Schema{Not: &jsonschema.Schema{}}, - Properties: map[string]*jsonschema.Schema{ + Properties: withIntentProperties(map[string]*jsonschema.Schema{ "field_name": { Type: "string", Description: "Issue field name (case-insensitive). Must match a field " + @@ -1947,7 +1969,7 @@ Options are: Description: "Set to true to clear this field's current value on the " + "issue. Cannot be combined with 'value' or 'field_option_name'.", }, - }, + }), Required: []string{"field_name"}, }, }, @@ -2068,8 +2090,9 @@ Options are: var issueFieldValues []*github.IssueRequestFieldValue var fieldIDsToDelete []int64 + var fieldValuesWithIntent map[int64]valueIntent if len(issueFields) > 0 { - issueFieldValues, fieldIDsToDelete, err = resolveIssueRequestFieldValues(ctx, gqlClient, owner, repo, issueFields) + issueFieldValues, fieldIDsToDelete, fieldValuesWithIntent, err = resolveIssueRequestFieldValues(ctx, gqlClient, owner, repo, issueFields) if err != nil { return utils.NewToolResultError(fmt.Sprintf("failed to resolve issue_fields: %v", err)), nil, nil } @@ -2094,6 +2117,9 @@ Options are: if issueTypeHasIntent { updateOpts.TypeWithIntent = issueTypePayload } + if len(fieldValuesWithIntent) > 0 { + updateOpts.FieldValuesWithIntent = fieldValuesWithIntent + } 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: @@ -2412,6 +2438,10 @@ type UpdateIssueOptions struct { // intent are preserved. When set, it takes precedence over the issueType // string. TypeWithIntent any + // FieldValuesWithIntent, when non-empty, maps a field ID to the intent + // metadata supplied for it, so those field values are sent in object form + // (field_id, value, plus rationale/confidence/suggest) via a custom request. + FieldValuesWithIntent map[int64]valueIntent } // issueRequestWithIntentOverrides marshals an IssueRequest into a generic map and @@ -2431,6 +2461,33 @@ func issueRequestWithIntentOverrides(issueRequest *github.IssueRequest, override return payload, nil } +// issueFieldValueWithIntent is the object form of an issue field value: its +// field ID and value alongside optional intent metadata. It marshals to a single +// object merging field_id and value with the embedded +// rationale/confidence/suggest fields, mirroring how labels and types carry +// intent on the wire. +type issueFieldValueWithIntent struct { + FieldID int64 + Value any + valueIntent +} + +// MarshalJSON renders the value as a single object with field_id and value +// alongside the embedded intent metadata. +func (v issueFieldValueWithIntent) 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["field_id"] = v.FieldID + obj["value"] = v.Value + return json.Marshal(obj) +} + 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) { updateOptions := UpdateIssueOptions{ AssigneesProvided: len(assignees) > 0, @@ -2445,6 +2502,9 @@ func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4 if opt.TypeWithIntent != nil { updateOptions.TypeWithIntent = opt.TypeWithIntent } + if len(opt.FieldValuesWithIntent) > 0 { + updateOptions.FieldValuesWithIntent = opt.FieldValuesWithIntent + } } // Create the issue request with only provided fields @@ -2505,10 +2565,11 @@ func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4 var updatedIssue *github.Issue var resp *github.Response var err error - if len(updateOptions.LabelsWithIntent) > 0 || updateOptions.TypeWithIntent != nil { - // Send values that carry intent (labels and/or type) in object form so - // their rationale and suggestion intent are preserved. Marshal the standard - // request (those fields omitted), then inject the object-form values. + if len(updateOptions.LabelsWithIntent) > 0 || updateOptions.TypeWithIntent != nil || len(updateOptions.FieldValuesWithIntent) > 0 { + // Send values that carry intent (labels, type, and/or field values) in + // object form so their rationale and suggestion intent are preserved. + // Marshal the standard request (those fields omitted), then inject the + // object-form values. overrides := map[string]any{} if len(updateOptions.LabelsWithIntent) > 0 { overrides["labels"] = updateOptions.LabelsWithIntent @@ -2516,6 +2577,17 @@ func UpdateIssue(ctx context.Context, client *github.Client, gqlClient *githubv4 if updateOptions.TypeWithIntent != nil { overrides["type"] = updateOptions.TypeWithIntent } + if len(updateOptions.FieldValuesWithIntent) > 0 && len(issueRequest.IssueFieldValues) > 0 { + fieldValues := make([]any, 0, len(issueRequest.IssueFieldValues)) + for _, v := range issueRequest.IssueFieldValues { + if intent, ok := updateOptions.FieldValuesWithIntent[v.FieldID]; ok { + fieldValues = append(fieldValues, issueFieldValueWithIntent{FieldID: v.FieldID, Value: v.Value, valueIntent: intent}) + } else { + fieldValues = append(fieldValues, map[string]any{"field_id": v.FieldID, "value": v.Value}) + } + } + overrides["issue_field_values"] = fieldValues + } payload, mErr := issueRequestWithIntentOverrides(issueRequest, overrides) if mErr != nil { return utils.NewToolResultErrorFromErr("failed to build issue update request", mErr), nil diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index e58881639..a422bc494 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -3752,6 +3752,186 @@ func Test_IssueWrite_UpdateTypeWithIntentErrors(t *testing.T) { } } +func Test_IssueWrite_UpdateFieldValuesWithIntent(t *testing.T) { + serverTool := IssueWrite(translations.NullTranslationHelper) + updatedIssue := &github.Issue{ + Number: github.Ptr(123), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/123"), + } + + const fetchExistingFieldValuesQuery = "query($number:Int!$owner:String!$repo:String!){repository(owner: $owner, name: $repo){issue(number: $number){issueFieldValues(first: 25){nodes{__typename,... on IssueFieldDateValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value},... on IssueFieldNumberValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},valueNumber: value},... on IssueFieldSingleSelectValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value},... on IssueFieldTextValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value}}}}}}" + + // newGQLMock returns a mocked GraphQL client that reports no existing field + // values (so incoming values are used as-is) and two fields: Priority + // (single_select, id 101, option P1) and Customer (text, id 102). + newGQLMock := func() *http.Client { + return githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + fetchExistingFieldValuesQuery, + map[string]any{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + "number": githubv4.Int(123), + }, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "issue": map[string]any{ + "issueFieldValues": map[string]any{ + "nodes": []any{}, + }, + }, + }, + }), + ), + githubv4mock.NewQueryMatcher( + issueFieldWriteMetadataQuery{}, + map[string]any{ + "owner": githubv4.String("owner"), + "repo": githubv4.String("repo"), + }, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "issueFields": map[string]any{ + "nodes": []any{ + map[string]any{ + "__typename": "IssueFieldSingleSelect", + "fullDatabaseId": "101", + "name": "Priority", + "dataType": "single_select", + "options": []any{map[string]any{"fullDatabaseId": "9001", "name": "P1"}}, + }, + map[string]any{ + "__typename": "IssueFieldText", + "fullDatabaseId": "102", + "name": "Customer", + "dataType": "text", + }, + }, + }, + }, + }), + ), + ) + } + + tests := []struct { + name string + issueFields []any + expectedReq map[string]any + }{ + { + name: "applied field value with rationale", + issueFields: []any{ + map[string]any{"field_name": "Customer", "value": "Acme", "rationale": "Customer named in the report"}, + }, + expectedReq: map[string]any{ + "issue_field_values": []any{ + map[string]any{"field_id": float64(102), "value": "Acme", "rationale": "Customer named in the report"}, + }, + }, + }, + { + name: "suggested field value with confidence", + issueFields: []any{ + map[string]any{"field_name": "Priority", "field_option_name": "P1", "confidence": "high", "is_suggestion": true}, + }, + expectedReq: map[string]any{ + "issue_field_values": []any{ + map[string]any{"field_id": float64(101), "value": "P1", "confidence": "high", "suggest": true}, + }, + }, + }, + { + name: "mix of field value with intent and plain field value", + issueFields: []any{ + map[string]any{"field_name": "Priority", "field_option_name": "P1", "rationale": "Reports a crash when saving"}, + map[string]any{"field_name": "Customer", "value": "Acme"}, + }, + expectedReq: map[string]any{ + "issue_field_values": []any{ + map[string]any{"field_id": float64(101), "value": "P1", "rationale": "Reports a crash when saving"}, + map[string]any{"field_id": float64(102), "value": "Acme"}, + }, + }, + }, + } + + 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(newGQLMock()), + } + handler := serverTool.Handler(deps) + + request := createMCPRequest(map[string]any{ + "method": "update", + "owner": "owner", + "repo": "repo", + "issue_number": float64(123), + "issue_fields": tc.issueFields, + }) + 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_UpdateFieldValuesWithIntentErrors(t *testing.T) { + serverTool := IssueWrite(translations.NullTranslationHelper) + + tests := []struct { + name string + issueFields []any + expectedErrText string + }{ + { + name: "rationale too long", + issueFields: []any{ + map[string]any{"field_name": "Customer", "value": "Acme", "rationale": strings.Repeat("a", 281)}, + }, + expectedErrText: "rationale must be 280 characters or less", + }, + { + name: "invalid confidence value", + issueFields: []any{ + map[string]any{"field_name": "Customer", "value": "Acme", "confidence": "very_high"}, + }, + expectedErrText: "confidence must be one of: low, medium, high", + }, + } + + 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(123), + "issue_fields": tc.issueFields, + }) + 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