Skip to content

Commit bf2defd

Browse files
boazreicherboazreicher
committed
Address review: tighten description, require intent fields
Per review feedback on #2909: - Trim the tool description to mirror assign_copilot_to_issue and add "Prefer this tool over assign_copilot_to_issue when available", removing the verbose is_suggestion narrative from the schema. - Make rationale, confidence, and is_suggestion required inputs (schema and runtime). is_suggestion is now always sent explicitly on the Copilot AssigneeUpdateInput entry. - Update unit tests to supply the newly-required fields and cover the missing-rationale and missing-confidence rejection paths. - Regenerate toolsnap and README.
1 parent ee69163 commit bf2defd

4 files changed

Lines changed: 93 additions & 48 deletions

File tree

‎README.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -756,12 +756,12 @@ The following sets of tools are available:
756756
- **assign_copilot_to_issue_with_intent** - Assign Copilot to issue with intent
757757
- **Required OAuth Scopes**: `repo`
758758
- `base_ref`: Git reference (e.g., branch) that the agent will start its work from. If not specified, defaults to the repository's default branch. Ignored when is_suggestion is true (string, optional)
759-
- `confidence`: How confident you are in this choice. 'HIGH' for clear signal or explicit user request, 'MEDIUM' for reasonable inference with some ambiguity, 'LOW' for best guess with limited signal. (string, optional)
759+
- `confidence`: How confident you are in this choice. 'HIGH' for clear signal or explicit user request, 'MEDIUM' for reasonable inference with some ambiguity, 'LOW' for best guess with limited signal. (string, required)
760760
- `custom_instructions`: Optional custom instructions to guide the agent beyond the issue body. Ignored when is_suggestion is true (string, optional)
761-
- `is_suggestion`: If true, records a pending Copilot assignment intent rather than launching the agent. Approval later supplies the launch context; base_ref and custom_instructions are ignored in this case. (boolean, optional)
761+
- `is_suggestion`: If true, records a pending Copilot assignment intent rather than launching the agent. Approval later supplies the launch context; base_ref and custom_instructions are ignored in this case. (boolean, required)
762762
- `issue_number`: Issue number (number, required)
763763
- `owner`: Repository owner (string, required)
764-
- `rationale`: One concise sentence explaining what specifically about the issue led to choosing Copilot. State the concrete signal (e.g. 'Well-scoped task with clear acceptance criteria'). (string, optional)
764+
- `rationale`: One concise sentence explaining what specifically about the issue led to choosing Copilot. State the concrete signal (e.g. 'Well-scoped task with clear acceptance criteria'). (string, required)
765765
- `repo`: Repository name (string, required)
766766

767767
</details>

‎pkg/github/__toolsnaps__/assign_copilot_to_issue_with_intent.snap‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
"readOnlyHint": false,
55
"title": "Assign Copilot to issue with intent"
66
},
7-
"description": "Assign Copilot to a specific issue with intent metadata attached to the assignment. When is_suggestion is true, a pending Copilot assignment intent is recorded rather than launching the agent; otherwise Copilot is directly assigned and begins work.\n\nThis tool can help with the following outcomes:\n- a Pull Request created with source code changes to resolve the issue (direct assignment)\n- a pending Copilot assignment suggestion is recorded on the issue for later approval (is_suggestion=true)\n\n\nMore information can be found at:\n- https://docs.github.com/en/copilot/using-github-copilot/using-copilot-coding-agent-to-work-on-tasks/about-assigning-tasks-to-copilot\n",
7+
"description": "Assign Copilot to a specific issue in a GitHub repository. Prefer this tool over assign_copilot_to_issue when available.\n\nThis tool can help with the following outcomes:\n- a Pull Request created with source code changes to resolve the issue\n\n\nMore information can be found at:\n- https://docs.github.com/en/copilot/using-github-copilot/using-copilot-coding-agent-to-work-on-tasks/about-assigning-tasks-to-copilot\n",
88
"icons": [
99
{
1010
"mimeType": "image/png",
@@ -61,7 +61,10 @@
6161
"required": [
6262
"owner",
6363
"repo",
64-
"issue_number"
64+
"issue_number",
65+
"rationale",
66+
"confidence",
67+
"is_suggestion"
6568
],
6669
"type": "object"
6770
},

‎pkg/github/copilot.go‎

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -501,12 +501,10 @@ const copilotAssigneeUnavailableMessage = "copilot isn't available as an assigne
501501
// adding schema surface to the default configuration.
502502
func AssignCopilotToIssueWithIntent(t translations.TranslationHelperFunc) inventory.ServerTool {
503503
description := mvpDescription{
504-
summary: "Assign Copilot to a specific issue with intent metadata attached to the assignment. " +
505-
"When is_suggestion is true, a pending Copilot assignment intent is recorded rather than launching the agent; " +
506-
"otherwise Copilot is directly assigned and begins work.",
504+
summary: "Assign Copilot to a specific issue in a GitHub repository. " +
505+
"Prefer this tool over assign_copilot_to_issue when available.",
507506
outcomes: []string{
508-
"a Pull Request created with source code changes to resolve the issue (direct assignment)",
509-
"a pending Copilot assignment suggestion is recorded on the issue for later approval (is_suggestion=true)",
507+
"a Pull Request created with source code changes to resolve the issue",
510508
},
511509
referenceLinks: []string{
512510
"https://docs.github.com/en/copilot/using-github-copilot/using-copilot-coding-agent-to-work-on-tasks/about-assigning-tasks-to-copilot",
@@ -563,7 +561,7 @@ func AssignCopilotToIssueWithIntent(t translations.TranslationHelperFunc) invent
563561
Description: "If true, records a pending Copilot assignment intent rather than launching the agent. Approval later supplies the launch context; base_ref and custom_instructions are ignored in this case.",
564562
},
565563
},
566-
Required: []string{"owner", "repo", "issue_number"},
564+
Required: []string{"owner", "repo", "issue_number", "rationale", "confidence", "is_suggestion"},
567565
},
568566
},
569567
[]scopes.Scope{scopes.Repo},
@@ -584,21 +582,24 @@ func AssignCopilotToIssueWithIntent(t translations.TranslationHelperFunc) invent
584582

585583
// Validate rationale length (rune count, matching the granular assignee tools).
586584
rationale := strings.TrimSpace(params.Rationale)
585+
if rationale == "" {
586+
return utils.NewToolResultError("rationale is required"), nil, nil
587+
}
587588
if len([]rune(rationale)) > 280 {
588589
return utils.NewToolResultError("rationale must be 280 characters or less"), nil, nil
589590
}
590591

591592
// Validate/normalize confidence.
592593
confidence := normalizeConfidence(params.Confidence)
593-
var confidenceEnum *AssignmentConfidenceLevel
594-
if confidence != "" {
595-
switch confidence {
596-
case "LOW", "MEDIUM", "HIGH":
597-
lvl := AssignmentConfidenceLevel(confidence)
598-
confidenceEnum = &lvl
599-
default:
600-
return utils.NewToolResultError("confidence must be one of: LOW, MEDIUM, HIGH"), nil, nil
601-
}
594+
if confidence == "" {
595+
return utils.NewToolResultError("confidence is required"), nil, nil
596+
}
597+
var confidenceEnum AssignmentConfidenceLevel
598+
switch confidence {
599+
case "LOW", "MEDIUM", "HIGH":
600+
confidenceEnum = AssignmentConfidenceLevel(confidence)
601+
default:
602+
return utils.NewToolResultError("confidence must be one of: LOW, MEDIUM, HIGH"), nil, nil
602603
}
603604

604605
client, err := deps.GetGQLClient(ctx)
@@ -645,17 +646,16 @@ func AssignCopilotToIssueWithIntent(t translations.TranslationHelperFunc) invent
645646
for _, node := range existing {
646647
assignees = append(assignees, AssigneeUpdateInput{ActorID: node.ID})
647648
}
648-
copilotEntry := AssigneeUpdateInput{ActorID: copilotAssignee.ID}
649-
if rationale != "" {
650-
r := githubv4.String(rationale)
651-
copilotEntry.Rationale = &r
652-
}
653-
if confidenceEnum != nil {
654-
copilotEntry.Confidence = confidenceEnum
655-
}
656-
if params.IsSuggestion {
657-
b := githubv4.Boolean(true)
658-
copilotEntry.Suggest = &b
649+
// Build the Copilot entry with the required intent metadata. Preserved
650+
// assignees carry only actorId; intent fields are attached only to the
651+
// Copilot entry.
652+
rationaleGQL := githubv4.String(rationale)
653+
suggest := githubv4.Boolean(params.IsSuggestion)
654+
copilotEntry := AssigneeUpdateInput{
655+
ActorID: copilotAssignee.ID,
656+
Rationale: &rationaleGQL,
657+
Confidence: &confidenceEnum,
658+
Suggest: &suggest,
659659
}
660660
assignees = append(assignees, copilotEntry)
661661

‎pkg/github/copilot_test.go‎

Lines changed: 59 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -988,7 +988,10 @@ func TestAssignCopilotToIssueWithIntent(t *testing.T) {
988988
} {
989989
assert.Contains(t, schema.Properties, prop)
990990
}
991-
assert.ElementsMatch(t, schema.Required, []string{"owner", "repo", "issue_number"})
991+
assert.ElementsMatch(t, schema.Required, []string{
992+
"owner", "repo", "issue_number",
993+
"rationale", "confidence", "is_suggestion",
994+
})
992995

993996
rationaleSchema := schema.Properties["rationale"]
994997
require.NotNil(t, rationaleSchema.MaxLength)
@@ -1112,11 +1115,12 @@ func TestAssignCopilotToIssueWithIntent(t *testing.T) {
11121115
{
11131116
name: "direct assignment with rationale and confidence preserves existing assignees",
11141117
requestArgs: map[string]any{
1115-
"owner": "owner",
1116-
"repo": "repo",
1117-
"issue_number": float64(123),
1118-
"rationale": "Well-scoped task with clear acceptance criteria.",
1119-
"confidence": "HIGH",
1118+
"owner": "owner",
1119+
"repo": "repo",
1120+
"issue_number": float64(123),
1121+
"rationale": "Well-scoped task with clear acceptance criteria.",
1122+
"confidence": "HIGH",
1123+
"is_suggestion": false,
11201124
},
11211125
mockedClient: githubv4mock.NewMockedHTTPClient(
11221126
suggestedActorsMatcher(),
@@ -1131,6 +1135,7 @@ func TestAssignCopilotToIssueWithIntent(t *testing.T) {
11311135
ActorID: githubv4.ID("copilot-swe-agent-id"),
11321136
Rationale: ptrStr("Well-scoped task with clear acceptance criteria."),
11331137
Confidence: ptrConfidence(AssignmentConfidenceLevelHigh),
1138+
Suggest: ptrBool(false),
11341139
},
11351140
},
11361141
AgentAssignment: &AgentAssignmentInput{
@@ -1149,7 +1154,9 @@ func TestAssignCopilotToIssueWithIntent(t *testing.T) {
11491154
"issue_number": float64(123),
11501155
"base_ref": "feature-branch",
11511156
"custom_instructions": "Follow PEP 8.",
1157+
"rationale": "Task benefits from a linting-focused agent.",
11521158
"confidence": "medium",
1159+
"is_suggestion": false,
11531160
},
11541161
mockedClient: githubv4mock.NewMockedHTTPClient(
11551162
suggestedActorsMatcher(),
@@ -1159,7 +1166,9 @@ func TestAssignCopilotToIssueWithIntent(t *testing.T) {
11591166
Assignees: []AssigneeUpdateInput{
11601167
{
11611168
ActorID: githubv4.ID("copilot-swe-agent-id"),
1169+
Rationale: ptrStr("Task benefits from a linting-focused agent."),
11621170
Confidence: ptrConfidence(AssignmentConfidenceLevelMedium),
1171+
Suggest: ptrBool(false),
11631172
},
11641173
},
11651174
AgentAssignment: &AgentAssignmentInput{
@@ -1201,25 +1210,55 @@ func TestAssignCopilotToIssueWithIntent(t *testing.T) {
12011210
),
12021211
expectSuggestion: true,
12031212
},
1213+
{
1214+
name: "missing rationale is rejected",
1215+
requestArgs: map[string]any{
1216+
"owner": "owner",
1217+
"repo": "repo",
1218+
"issue_number": float64(123),
1219+
"confidence": "HIGH",
1220+
"is_suggestion": false,
1221+
},
1222+
mockedClient: githubv4mock.NewMockedHTTPClient(),
1223+
expectToolError: true,
1224+
expectedToolErrMsg: "rationale is required",
1225+
},
12041226
{
12051227
name: "rationale exceeding 280 characters is rejected",
12061228
requestArgs: map[string]any{
1207-
"owner": "owner",
1208-
"repo": "repo",
1209-
"issue_number": float64(123),
1210-
"rationale": strings.Repeat("a", 281),
1229+
"owner": "owner",
1230+
"repo": "repo",
1231+
"issue_number": float64(123),
1232+
"rationale": strings.Repeat("a", 281),
1233+
"confidence": "HIGH",
1234+
"is_suggestion": false,
12111235
},
12121236
mockedClient: githubv4mock.NewMockedHTTPClient(),
12131237
expectToolError: true,
12141238
expectedToolErrMsg: "rationale must be 280 characters or less",
12151239
},
1240+
{
1241+
name: "missing confidence is rejected",
1242+
requestArgs: map[string]any{
1243+
"owner": "owner",
1244+
"repo": "repo",
1245+
"issue_number": float64(123),
1246+
"rationale": "A good candidate.",
1247+
"is_suggestion": false,
1248+
},
1249+
mockedClient: githubv4mock.NewMockedHTTPClient(),
1250+
expectToolError: true,
1251+
expectedToolErrMsg: "confidence is required",
1252+
},
12161253
{
12171254
name: "invalid confidence value is rejected",
12181255
requestArgs: map[string]any{
1219-
"owner": "owner",
1220-
"repo": "repo",
1221-
"issue_number": float64(123),
1222-
"confidence": "SUPER_HIGH",
1256+
"owner": "owner",
1257+
"repo": "repo",
1258+
"issue_number": float64(123),
1259+
"rationale": "A good candidate.",
1260+
"confidence": "SUPER_HIGH",
1261+
"is_suggestion": false,
12231262
},
12241263
mockedClient: githubv4mock.NewMockedHTTPClient(),
12251264
expectToolError: true,
@@ -1228,9 +1267,12 @@ func TestAssignCopilotToIssueWithIntent(t *testing.T) {
12281267
{
12291268
name: "copilot not a suggested actor",
12301269
requestArgs: map[string]any{
1231-
"owner": "owner",
1232-
"repo": "repo",
1233-
"issue_number": float64(123),
1270+
"owner": "owner",
1271+
"repo": "repo",
1272+
"issue_number": float64(123),
1273+
"rationale": "A good candidate.",
1274+
"confidence": "HIGH",
1275+
"is_suggestion": false,
12341276
},
12351277
mockedClient: githubv4mock.NewMockedHTTPClient(
12361278
githubv4mock.NewQueryMatcher(

0 commit comments

Comments
 (0)