Skip to content

Commit b36634c

Browse files
CAOShurongCAOShurong
committed
fix: register batch label tool with typed output
1 parent 02e3b71 commit b36634c

3 files changed

Lines changed: 70 additions & 66 deletions

File tree

‎pkg/github/granular_issue_types.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,35 @@ type GranularIssueLabelsInput struct {
111111
Labels []GranularIssueLabel `json:"labels"`
112112
}
113113

114+
// GranularBatchUpdateIssueLabelsInput contains one label mutation per issue.
115+
// Add and Remove are both optional, but at least one must contain a label for
116+
// every operation.
117+
type GranularBatchUpdateIssueLabelsInput struct {
118+
Owner string `json:"owner"`
119+
Repo string `json:"repo"`
120+
Operations []GranularBatchUpdateIssueLabelsOperation `json:"operations"`
121+
}
122+
123+
type GranularBatchUpdateIssueLabelsOperation struct {
124+
IssueNumber int `json:"issue_number"`
125+
Add []string `json:"add,omitempty"`
126+
Remove []string `json:"remove,omitempty"`
127+
}
128+
129+
// GranularBatchUpdateIssueLabelsResult reports the outcome for one issue.
130+
type GranularBatchUpdateIssueLabelsResult struct {
131+
IssueNumber int `json:"issue_number"`
132+
Applied bool `json:"applied"`
133+
Error string `json:"error,omitempty"`
134+
Labels []string `json:"labels,omitempty"`
135+
}
136+
137+
// GranularBatchUpdateIssueLabelsOutput is the structured result of a batch
138+
// label update.
139+
type GranularBatchUpdateIssueLabelsOutput struct {
140+
Results []GranularBatchUpdateIssueLabelsResult `json:"results"`
141+
}
142+
114143
type GranularIssueTypeInput struct {
115144
GranularIssueCoordinate
116145
IssueType *string `json:"issue_type"`

‎pkg/github/issues_granular.go‎

Lines changed: 40 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -485,19 +485,11 @@ func GranularUpdateIssueMilestone(t translations.TranslationHelperFunc) inventor
485485
)
486486
}
487487

488-
// batchLabelOperation is one per-issue add/remove entry inside a
489-
// batch_update_issue_labels call.
490-
type batchLabelOperation struct {
491-
IssueNumber int `json:"issue_number"`
492-
Add []string `json:"add,omitempty"`
493-
Remove []string `json:"remove,omitempty"`
494-
}
495-
496488
// GranularBatchUpdateIssueLabels creates a tool to apply label changes to
497489
// several issues in a single call, avoiding N sequential update_issue_labels
498490
// round-trips for bulk triage workflows.
499491
func GranularBatchUpdateIssueLabels(t translations.TranslationHelperFunc) inventory.ServerTool {
500-
st := NewTool(
492+
st := NewTool[GranularBatchUpdateIssueLabelsInput, *GranularBatchUpdateIssueLabelsOutput](
501493
ToolsetMetadataIssues,
502494
mcp.Tool{
503495
Name: "batch_update_issue_labels",
@@ -551,59 +543,33 @@ func GranularBatchUpdateIssueLabels(t translations.TranslationHelperFunc) invent
551543
},
552544
},
553545
scopes.RequireAll(scopes.Repo),
554-
func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
555-
owner, err := RequiredParam[string](args, "owner")
556-
if err != nil {
557-
return utils.NewToolResultError(err.Error()), nil, nil
546+
func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args GranularBatchUpdateIssueLabelsInput) (*mcp.CallToolResult, *GranularBatchUpdateIssueLabelsOutput, error) {
547+
owner, repo := args.Owner, args.Repo
548+
if owner == "" {
549+
return utils.NewToolResultError("missing required parameter: owner"), nil, nil
558550
}
559-
repo, err := RequiredParam[string](args, "repo")
560-
if err != nil {
561-
return utils.NewToolResultError(err.Error()), nil, nil
562-
}
563-
564-
opsRaw, ok := args["operations"]
565-
if !ok {
566-
return utils.NewToolResultError("missing required parameter: operations"), nil, nil
551+
if repo == "" {
552+
return utils.NewToolResultError("missing required parameter: repo"), nil, nil
567553
}
568-
opsSlice, ok := opsRaw.([]any)
569-
if !ok {
570-
return utils.NewToolResultError("parameter operations must be an array"), nil, nil
571-
}
572-
if len(opsSlice) == 0 {
554+
if len(args.Operations) == 0 {
573555
return utils.NewToolResultError("operations must contain at least one entry"), nil, nil
574556
}
575557

576558
// Validate every operation up front so an invalid entry fails the
577559
// whole call before any mutation is attempted.
578-
planned := make([]batchLabelOperation, 0, len(opsSlice))
579-
seenIssues := make(map[int]bool, len(opsSlice))
580-
for i, item := range opsSlice {
581-
opObj, ok := item.(map[string]any)
582-
if !ok {
583-
return utils.NewToolResultError(fmt.Sprintf("operations[%d]: each operation must be an object with issue_number and at least one of add or remove", i)), nil, nil
584-
}
585-
issueNumber, err := RequiredInt(opObj, "issue_number")
586-
if err != nil {
587-
return utils.NewToolResultError(fmt.Sprintf("operations[%d]: %s", i, err.Error())), nil, nil
560+
planned := make([]GranularBatchUpdateIssueLabelsOperation, 0, len(args.Operations))
561+
seenIssues := make(map[int]bool, len(args.Operations))
562+
for i, op := range args.Operations {
563+
if op.IssueNumber == 0 {
564+
return utils.NewToolResultError(fmt.Sprintf("operations[%d]: missing required parameter: issue_number", i)), nil, nil
588565
}
589-
if seenIssues[issueNumber] {
590-
return utils.NewToolResultError(fmt.Sprintf("operations[%d]: duplicate issue_number %d; merge the label changes into a single operation", i, issueNumber)), nil, nil
591-
}
592-
seenIssues[issueNumber] = true
593-
594-
op := batchLabelOperation{IssueNumber: issueNumber}
595-
if addRaw, hasAdd := opObj["add"]; hasAdd && addRaw != nil {
596-
op.Add, err = OptionalStringArrayParam(opObj, "add")
597-
if err != nil {
598-
return utils.NewToolResultError(fmt.Sprintf("operations[%d]: %s", i, err.Error())), nil, nil
599-
}
566+
if op.IssueNumber < 0 {
567+
return utils.NewToolResultError(fmt.Sprintf("operations[%d]: issue_number must be greater than 0", i)), nil, nil
600568
}
601-
if removeRaw, hasRemove := opObj["remove"]; hasRemove && removeRaw != nil {
602-
op.Remove, err = OptionalStringArrayParam(opObj, "remove")
603-
if err != nil {
604-
return utils.NewToolResultError(fmt.Sprintf("operations[%d]: %s", i, err.Error())), nil, nil
605-
}
569+
if seenIssues[op.IssueNumber] {
570+
return utils.NewToolResultError(fmt.Sprintf("operations[%d]: duplicate issue_number %d; merge the label changes into a single operation", i, op.IssueNumber)), nil, nil
606571
}
572+
seenIssues[op.IssueNumber] = true
607573
if len(op.Add) == 0 && len(op.Remove) == 0 {
608574
return utils.NewToolResultError(fmt.Sprintf("operations[%d]: at least one non-empty of add or remove is required", i)), nil, nil
609575
}
@@ -625,16 +591,10 @@ func GranularBatchUpdateIssueLabels(t translations.TranslationHelperFunc) invent
625591
return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil
626592
}
627593

628-
type opOutcome struct {
629-
IssueNumber int `json:"issue_number"`
630-
Applied bool `json:"applied"`
631-
Error string `json:"error,omitempty"`
632-
Labels []string `json:"labels,omitempty"`
633-
}
634-
outcomes := make([]opOutcome, 0, len(planned))
594+
outcomes := make([]GranularBatchUpdateIssueLabelsResult, 0, len(planned))
635595
hadFailure := false
636596
for _, op := range planned {
637-
outcome := opOutcome{IssueNumber: op.IssueNumber, Applied: true}
597+
outcome := GranularBatchUpdateIssueLabelsResult{IssueNumber: op.IssueNumber, Applied: true}
638598

639599
// Add phase: POST .../issues/{n}/labels adds without touching
640600
// existing labels and returns the full resulting label set.
@@ -679,22 +639,37 @@ func GranularBatchUpdateIssueLabels(t translations.TranslationHelperFunc) invent
679639
outcomes = append(outcomes, outcome)
680640
}
681641

682-
response := struct {
683-
Results []opOutcome `json:"results"`
684-
}{Results: outcomes}
685-
r, err := json.Marshal(response)
642+
output := &GranularBatchUpdateIssueLabelsOutput{Results: outcomes}
643+
r, err := json.Marshal(output)
686644
if err != nil {
687645
return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil
688646
}
689647
result := utils.NewToolResultText(string(r))
690648
result.IsError = hadFailure
691-
return result, nil, nil
649+
return result, output, nil
692650
},
651+
granularIssueInputNormalizer(normalizeGranularBatchUpdateIssueLabelsArguments),
693652
)
694653
st.FeatureRule = issuesGranularFeatureRule
695654
return st
696655
}
697656

657+
func normalizeGranularBatchUpdateIssueLabelsArguments(raw json.RawMessage) (json.RawMessage, error) {
658+
var args map[string]any
659+
if err := json.Unmarshal(raw, &args); err != nil {
660+
return nil, err
661+
}
662+
for _, name := range []string{"owner", "repo"} {
663+
if _, err := RequiredParam[string](args, name); err != nil {
664+
return nil, err
665+
}
666+
}
667+
if _, ok := args["operations"]; !ok {
668+
return nil, fmt.Errorf("missing required parameter: operations")
669+
}
670+
return json.Marshal(args)
671+
}
672+
698673
// issueTypeWithIntent represents the object form of the issue type field,
699674
// allowing a rationale, confidence level, and/or suggest flag to be sent alongside the type name.
700675
type issueTypeWithIntent struct {

‎pkg/github/typed_granular_issue_outputs_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import (
2323
func granularIssueSession(t *testing.T, deps BaseDeps, protocol string) (*mcp.ClientSession, map[string]*jsonschema.Resolved) {
2424
t.Helper()
2525
tools := granularToolsForToolset(ToolsetMetadataIssues.ID, FeatureFlagIssuesGranular)
26-
require.Len(t, tools, 18)
26+
require.Len(t, tools, 19)
2727
inv, err := inventory.NewBuilder().SetTools(tools).WithToolsets([]string{"all"}).
2828
WithFeatureChecker(featureCheckerFor(inventory.FeatureFlag(FeatureFlagIssuesGranular))).Build()
2929
require.NoError(t, err)

0 commit comments

Comments
 (0)