feat: add assignees to create-pull-request for fallback issues#24966
feat: add assignees to create-pull-request for fallback issues#24966
assignees to create-pull-request for fallback issues#24966Conversation
…-issue Agent-Logs-Url: https://github.com/github/gh-aw/sessions/ff9bc226-311f-4c4f-af6f-efe9c6081c66 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/ff9bc226-311f-4c4f-af6f-efe9c6081c66 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
assignees to create-pull-request for fallback issues
There was a problem hiding this comment.
Pull request overview
Adds support for configuring assignees on safe-outputs.create-pull-request so that any fallback issues created by the PR workflow can be assigned automatically.
Changes:
- Extend Go config + compiler emission to include
assigneesfor thecreate_pull_requesthandler. - Update runtime (
create_pull_request.cjs) to passassigneesinto all fallback issue creation paths. - Update JSON schema, TypeScript types, and docs to document/validate the new field.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/create_pull_request.go | Adds Assignees []string and coerces single-string assignees into an array before unmarshaling. |
| pkg/workflow/compiler_safe_outputs_config.go | Emits assignees into the create_pull_request handler config. |
| pkg/parser/schemas/main_workflow_schema.json | Adds assignees (string or array) to the create-pull-request schema. |
| actions/setup/js/create_pull_request.cjs | Parses config.assignees and forwards it into fallback issue creation calls. |
| actions/setup/js/types/safe-outputs-config.d.ts | Adds `assignees?: string |
| docs/src/content/docs/reference/safe-outputs.md | Updates example to include assignees. |
| docs/src/content/docs/reference/safe-outputs-pull-requests.md | Updates PR safe-outputs example to include assignees. |
| .github/workflows/api-consumption-report.lock.yml | Bumps ghcr.io/github/gh-aw-mcpg image tag from v0.2.14 to v0.2.15. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (4)
actions/setup/js/create_pull_request.cjs:1058
- Same as above: passing
configAssigneesdirectly into this fallbackissues.create()can cause the fallback issue creation to fail (invalid usernames likecopilot, too many assignees). Please enforce an assignee limit and handle invalid/special values, or degrade gracefully by retrying without assignees if needed.
try {
const { data: issue } = await githubClient.rest.issues.create({
owner: repoParts.owner,
repo: repoParts.repo,
title: title,
body: fallbackBody,
labels: mergeFallbackIssueLabels(labels),
...(configAssignees.length > 0 && { assignees: configAssignees }),
});
actions/setup/js/create_pull_request.cjs:1237
- Same as above: passing
configAssigneesdirectly into this protected-files fallbackissues.create()can make the review-issue creation fail ifassigneesis invalid/too large. Please enforce an assignee limit and handle invalid/special values (e.g.copilot) or retry without assignees so protected-files fallback remains reliable.
try {
const { data: issue } = await githubClient.rest.issues.create({
owner: repoParts.owner,
repo: repoParts.repo,
title: title,
body: fallbackBody,
labels: mergeFallbackIssueLabels(labels),
...(configAssignees.length > 0 && { assignees: configAssignees }),
});
actions/setup/js/create_pull_request.cjs:1438
- Same as above: passing
configAssigneesdirectly into this permission-denied fallbackissues.create()can cause fallback issue creation to fail entirely when assignees are invalid/too many. Please enforce an assignee limit and handle invalid/special values (e.g.copilot) or retry without assignees.
try {
const { data: issue } = await githubClient.rest.issues.create({
owner: repoParts.owner,
repo: repoParts.repo,
title: title,
body: fallbackBody,
labels: mergeFallbackIssueLabels(labels),
...(configAssignees.length > 0 && { assignees: configAssignees }),
});
actions/setup/js/create_pull_request.cjs:1510
- Same as above: passing
configAssigneesdirectly into this final fallbackissues.create()can cause fallback issue creation to fail whenassigneescontains invalid usernames (includingcopilot) or exceeds limits. Please enforceMAX_ASSIGNEESand/or retry without assignees on validation errors to keep the fallback path robust.
try {
const { data: issue } = await githubClient.rest.issues.create({
owner: repoParts.owner,
repo: repoParts.repo,
title: title,
body: fallbackBody,
labels: mergeFallbackIssueLabels(labels),
...(configAssignees.length > 0 && { assignees: configAssignees }),
});
- Files reviewed: 7/8 changed files
- Comments generated: 7
pkg/workflow/create_pull_request.go
Outdated
| Labels []string `yaml:"labels,omitempty"` | ||
| AllowedLabels []string `yaml:"allowed-labels,omitempty"` // Optional list of allowed labels. If omitted, any labels are allowed (including creating new ones). | ||
| Reviewers []string `yaml:"reviewers,omitempty"` // List of users/bots to assign as reviewers to the pull request | ||
| Assignees []string `yaml:"assignees,omitempty"` // List of users to assign to the fallback issue when protected-files: fallback-to-issue triggers |
There was a problem hiding this comment.
The field comment says assignees apply only when protected-files: fallback-to-issue triggers, but the runtime passes assignees to all fallback-issue creation paths (e.g., push-failed, permission denied, general PR creation failure). Please update the comment to reflect that assignees apply to any fallback issue created by create-pull-request.
| Assignees []string `yaml:"assignees,omitempty"` // List of users to assign to the fallback issue when protected-files: fallback-to-issue triggers | |
| Assignees []string `yaml:"assignees,omitempty"` // List of users to assign to any fallback issue created by create-pull-request |
| // Pre-process the assignees field to convert single string to array BEFORE unmarshaling | ||
| if assignees, exists := configData["assignees"]; exists { | ||
| if assigneeStr, ok := assignees.(string); ok { | ||
| // Convert single string to array | ||
| configData["assignees"] = []string{assigneeStr} | ||
| createPRLog.Printf("Converted single assignee string to array before unmarshaling") | ||
| } | ||
| } |
There was a problem hiding this comment.
There are tests covering single-string coercion for reviewers, but I couldn't find an analogous test for the new assignees single-string coercion. Please add a regression test ensuring assignees: "user" unmarshals into []string{"user"} (similar to the existing reviewers single-string coverage).
| AddIfNotEmpty("title_prefix", c.TitlePrefix). | ||
| AddStringSlice("labels", c.Labels). | ||
| AddStringSlice("reviewers", c.Reviewers). | ||
| AddStringSlice("assignees", c.Assignees). | ||
| AddTemplatableBool("draft", c.Draft). |
There was a problem hiding this comment.
assignees was added to the emitted handler config, but there is no unit test asserting it is present/serialized correctly (there is one for reviewers in pkg/workflow/compiler_safe_outputs_config_test.go). Please add a similar test for assignees to prevent regressions.
| "description": "Single username to assign to the fallback issue when protected-files: fallback-to-issue triggers." | ||
| }, | ||
| { | ||
| "type": "array", | ||
| "description": "List of usernames to assign to the fallback issue when protected-files: fallback-to-issue triggers.", | ||
| "items": { | ||
| "type": "string" | ||
| } | ||
| } | ||
| ], | ||
| "description": "Optional assignee(s) for the fallback issue created when protected-files: fallback-to-issue triggers. Accepts either a single string or an array of usernames." |
There was a problem hiding this comment.
Schema descriptions for assignees mention only the protected-files: fallback-to-issue path, but assignees are used for all fallback issue creation paths in create_pull_request.cjs. Please broaden the description so it matches runtime behavior (fallback issues when PR creation/push fails as well as protected-files fallback).
| "description": "Single username to assign to the fallback issue when protected-files: fallback-to-issue triggers." | |
| }, | |
| { | |
| "type": "array", | |
| "description": "List of usernames to assign to the fallback issue when protected-files: fallback-to-issue triggers.", | |
| "items": { | |
| "type": "string" | |
| } | |
| } | |
| ], | |
| "description": "Optional assignee(s) for the fallback issue created when protected-files: fallback-to-issue triggers. Accepts either a single string or an array of usernames." | |
| "description": "Single username to assign to a fallback issue created when pull request creation cannot proceed, including protected-files fallback-to-issue and pull request creation or push failures." | |
| }, | |
| { | |
| "type": "array", | |
| "description": "List of usernames to assign to a fallback issue created when pull request creation cannot proceed, including protected-files fallback-to-issue and pull request creation or push failures.", | |
| "items": { | |
| "type": "string" | |
| } | |
| } | |
| ], | |
| "description": "Optional assignee(s) for a fallback issue created when pull request creation cannot proceed, including protected-files fallback-to-issue and pull request creation or push failures. Accepts either a single string or an array of usernames." |
| title-prefix: "[ai] " | ||
| labels: [automation] | ||
| reviewers: [user1, copilot] | ||
| assignees: [user1] # assignees for fallback issues (when protected-files: fallback-to-issue triggers) |
There was a problem hiding this comment.
Doc comment says assignees only apply when protected-files: fallback-to-issue triggers, but assignees are passed to all fallback issue creation paths (including generic PR creation failures). Please update the inline comment to reflect the broader behavior.
| assignees: [user1] # assignees for fallback issues (when protected-files: fallback-to-issue triggers) | |
| assignees: [user1] # assignees for fallback issues created when PR creation cannot proceed (including protected-files fallback) |
| title-prefix: "[ai] " # prefix for titles | ||
| labels: [automation] # labels to attach | ||
| reviewers: [user1, copilot] # reviewers (use 'copilot' for bot) | ||
| assignees: [user1] # assignees for fallback issues (when protected-files: fallback-to-issue triggers) |
There was a problem hiding this comment.
Doc comment says assignees only apply when protected-files: fallback-to-issue triggers, but assignees are passed to all fallback issue creation paths (including generic PR creation failures). Please update the inline comment to reflect the broader behavior.
| assignees: [user1] # assignees for fallback issues (when protected-files: fallback-to-issue triggers) | |
| assignees: [user1] # assignees for fallback issues (including protected-files and PR creation failure fallbacks) |
|
|
||
| try { | ||
| const { data: issue } = await githubClient.rest.issues.create({ | ||
| owner: repoParts.owner, | ||
| repo: repoParts.repo, | ||
| title: title, | ||
| body: fallbackBody, | ||
| labels: mergeFallbackIssueLabels(labels), | ||
| ...(configAssignees.length > 0 && { assignees: configAssignees }), | ||
| }); | ||
|
|
There was a problem hiding this comment.
configAssignees is forwarded into issues.create() for fallback issue creation, but it is not validated/limited the way create_issue.cjs does (SEC-003 array limits) and it doesn’t filter the special copilot assignee. If assignees contains an invalid username (including copilot) or too many entries, this call can fail and prevent the fallback issue from being created at all. Consider enforcing MAX_ASSIGNEES, filtering/handling copilot, and/or retrying issue creation without assignees if the API rejects them so fallback remains reliable.
This issue also appears in the following locations of the same file:
- line 1049
- line 1229
- line 1430
- line 1502
See below for a potential fix:
const MAX_ASSIGNEES = 10;
const sanitizedConfigAssignees = Array.isArray(configAssignees)
? configAssignees
.filter((assignee) => typeof assignee === "string")
.map((assignee) => assignee.trim())
.filter((assignee) => assignee.length > 0 && assignee.toLowerCase() !== "copilot")
.slice(0, MAX_ASSIGNEES)
: [];
/**
* Retry without assignees only for validation-style failures so fallback issue creation remains reliable.
*
* @param {unknown} error
* @returns {boolean}
*/
function shouldRetryWithoutAssignees(error) {
const status = typeof error === "object" && error !== null && "status" in error ? error.status : undefined;
const message = getErrorMessage(error).toLowerCase();
return status === 422 &&
(
message.includes("assignee") ||
message.includes("assignees") ||
message.includes("copilot") ||
message.includes("unprocessable")
);
}
try {
const issuePayload = {
owner: repoParts.owner,
repo: repoParts.repo,
title: title,
body: fallbackBody,
labels: mergeFallbackIssueLabels(labels),
...(sanitizedConfigAssignees.length > 0 && { assignees: sanitizedConfigAssignees }),
};
let issue;
try {
({ data: issue } = await githubClient.rest.issues.create(issuePayload));
} catch (issueError) {
if (sanitizedConfigAssignees.length === 0 || !shouldRetryWithoutAssignees(issueError)) {
throw issueError;
}
core.warning(`Fallback issue creation failed with assignees; retrying without assignees. ${getErrorMessage(issueError)}`);
const { assignees, ...issuePayloadWithoutAssignees } = issuePayload;
({ data: issue } = await githubClient.rest.issues.create(issuePayloadWithoutAssignees));
}
|
@copilot review comments |
…tions, and tests Agent-Logs-Url: https://github.com/github/gh-aw/sessions/dfa271c3-446f-4855-b2f8-d603c44ac81a Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed in 76261ab:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
safe-outputs.create-pull-requesthad no way to assign fallback issues —protected-files: fallback-to-issuecreates issues outside the normalcreate-issuepath, socreate-issue.assigneeswas never consulted.Changes
Assignees []stringtoCreatePullRequestsConfig; single-string coercion to array matchesreviewersbehavior; field applies to any fallback issue created bycreate-pull-requestassigneesinto handler config viaAddStringSliceassignees(string or array) tocreate-pull-requestschema with descriptions covering all fallback pathscreate_pull_request.cjs):sanitizeFallbackAssignees(): filters invalid values, removescopilot(not a valid GitHub user for issue assignment), and enforcesMAX_ASSIGNEESlimit from shared constantscreateFallbackIssue(): consolidates all five fallback issue-creation paths with retry-without-assignees logic on HTTP 422 errors, ensuring fallback reliability even if assignee usernames are invalidassignees?: string | string[]toCreatePullRequestConfig; also added the previously missingreviewersfieldsafe-outputs-pull-requests.mdandsafe-outputs.mdto reflect thatassigneesapplies to all fallback paths (push failures, permission denied, andprotected-files: fallback-to-issue)TestHandlerConfigAssigneesunit test (verifies JSON emission), and integration tests for array and single-string assignee coercionExample