Skip to content

Commit 8b3538f

Browse files
authored
Return a clear error for missing owner/repo/issue_number in the copilot assignment tools (#3221)
assign_copilot_to_issue and assign_copilot_to_issue_with_intent decode owner, repo and issue_number with mapstructure.WeakDecode, which zero-fills a missing value instead of erroring. A missing required arg then reached the GraphQL query and surfaced as a confusing "failed to get suggested actors: Could not resolve to a Repository" error. Reject the zero values after decoding, matching the is_suggestion and rationale/confidence checks in the same handlers, so the caller gets "missing required parameter: <name>". Adds table tests for the missing cases. Co-authored-by: Justin Willhite <5132924+thejdubb02@users.noreply.github.com>
1 parent 9205304 commit 8b3538f

2 files changed

Lines changed: 69 additions & 0 deletions

File tree

pkg/github/copilot.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,19 @@ func AssignCopilotToIssue(t translations.TranslationHelperFunc) inventory.Server
214214
return utils.NewToolResultError(err.Error()), nil, nil
215215
}
216216

217+
// owner, repo and issue_number are required, but WeakDecode zero-fills a
218+
// missing value, so a missing arg reached the query as a confusing
219+
// "Could not resolve to a Repository" error. Reject the zero values.
220+
if params.Owner == "" {
221+
return utils.NewToolResultError("missing required parameter: owner"), nil, nil
222+
}
223+
if params.Repo == "" {
224+
return utils.NewToolResultError("missing required parameter: repo"), nil, nil
225+
}
226+
if params.IssueNumber == 0 {
227+
return utils.NewToolResultError("missing required parameter: issue_number"), nil, nil
228+
}
229+
217230
client, err := deps.GetGQLClient(ctx)
218231
if err != nil {
219232
return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err)
@@ -588,6 +601,19 @@ func AssignCopilotToIssueWithIntent(t translations.TranslationHelperFunc) invent
588601
return utils.NewToolResultError(err.Error()), nil, nil
589602
}
590603

604+
// owner, repo and issue_number are required, but WeakDecode zero-fills a
605+
// missing value, so reject the zero values (as with rationale/confidence
606+
// below) before they reach the query as a confusing repository error.
607+
if params.Owner == "" {
608+
return utils.NewToolResultError("missing required parameter: owner"), nil, nil
609+
}
610+
if params.Repo == "" {
611+
return utils.NewToolResultError("missing required parameter: repo"), nil, nil
612+
}
613+
if params.IssueNumber == 0 {
614+
return utils.NewToolResultError("missing required parameter: issue_number"), nil, nil
615+
}
616+
591617
// Validate rationale length (rune count, matching the granular assignee tools).
592618
rationale := strings.TrimSpace(params.Rationale)
593619
if rationale == "" {

pkg/github/copilot_test.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,36 @@ func TestAssignCopilotToIssue(t *testing.T) {
5959
expectToolError bool
6060
expectedToolErrMsg string
6161
}{
62+
{
63+
name: "missing owner is rejected",
64+
requestArgs: map[string]any{
65+
"repo": "repo",
66+
"issue_number": float64(123),
67+
},
68+
mockedClient: githubv4mock.NewMockedHTTPClient(),
69+
expectToolError: true,
70+
expectedToolErrMsg: "missing required parameter: owner",
71+
},
72+
{
73+
name: "missing repo is rejected",
74+
requestArgs: map[string]any{
75+
"owner": "owner",
76+
"issue_number": float64(123),
77+
},
78+
mockedClient: githubv4mock.NewMockedHTTPClient(),
79+
expectToolError: true,
80+
expectedToolErrMsg: "missing required parameter: repo",
81+
},
82+
{
83+
name: "missing issue_number is rejected",
84+
requestArgs: map[string]any{
85+
"owner": "owner",
86+
"repo": "repo",
87+
},
88+
mockedClient: githubv4mock.NewMockedHTTPClient(),
89+
expectToolError: true,
90+
expectedToolErrMsg: "missing required parameter: issue_number",
91+
},
6292
{
6393
name: "successful assignment when there are no existing assignees",
6494
requestArgs: map[string]any{
@@ -1228,6 +1258,19 @@ func TestAssignCopilotToIssueWithIntent(t *testing.T) {
12281258
expectedToolErrMsg string
12291259
expectSuggestion bool
12301260
}{
1261+
{
1262+
name: "missing owner is rejected",
1263+
requestArgs: map[string]any{
1264+
"repo": "repo",
1265+
"issue_number": float64(123),
1266+
"rationale": "Well-scoped task.",
1267+
"confidence": "HIGH",
1268+
"is_suggestion": false,
1269+
},
1270+
mockedClient: githubv4mock.NewMockedHTTPClient(),
1271+
expectToolError: true,
1272+
expectedToolErrMsg: "missing required parameter: owner",
1273+
},
12311274
{
12321275
name: "direct assignment with rationale and confidence preserves existing assignees",
12331276
requestArgs: map[string]any{

0 commit comments

Comments
 (0)