Skip to content

Warn instead of failing when issue fields can't be set #13899

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
imkp1 wants to merge 6 commits into
base: trunk
Choose a base branch
from
+225 −18
Open

Warn instead of failing when issue fields can't be set #13899

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c82266c
Warn instead of failing when issue fields can't be set
imkp1 Jul 16, 2026
8fbdb9b
Skip issue type picker without triage access
imkp1 Jul 17, 2026
745dc68
Resolve deferred issue fields before creating the issue
imkp1 Jul 21, 2026
96a469b
Merge branch 'trunk' into fix/13804-gh-issue-create-errors-after-succ…
imkp1 Jul 21, 2026
0834fed
Test unresolvable ref later in the blocked-by loop
imkp1 Jul 21, 2026
0542803
Merge branch 'trunk' into fix/13804-gh-issue-create-errors-after-succ…
imkp1 Jul 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 26 additions & 14 deletions pkg/cmd/issue/create/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,8 +304,10 @@ func createRun(opts *CreateOptions) (err error) {
}
}

// Interactive issue type selection
if opts.IssueType == "" {
// Interactive issue type selection. Setting a type requires triage
// access, so don't offer the picker to a viewer who cannot apply it:
// the mutation would only fail once the issue already exists.
if opts.IssueType == "" && repo.ViewerCanTriage() {
issueTypes, typesErr := api.RepoIssueTypes(apiClient, baseRepo)
if typesErr == nil && len(issueTypes) > 0 {
typeNames := make([]string, len(issueTypes))
Expand Down Expand Up @@ -393,6 +395,15 @@ func createRun(opts *CreateOptions) (err error) {
}
return opts.Browser.Browse(openURL)
} else if action == prShared.SubmitAction {
// Resolve the deferred fields before creating anything. These lookups
// can fail on user error (an unknown --type, a bad issue reference) and
// such a failure must not leave a created issue behind.
var updateOpts api.DeferredUpdateIssueOptions
updateOpts, err = resolveDeferredUpdateIssueOptions(apiClient, baseRepo, opts)
if err != nil {
return
}

params := map[string]interface{}{
"title": tb.Title,
"body": tb.Body,
Expand All @@ -412,13 +423,14 @@ func createRun(opts *CreateOptions) (err error) {
return
}

var updateOpts api.DeferredUpdateIssueOptions
updateOpts, err = deferredUpdateIssueOptions(apiClient, baseRepo, newIssue, opts)
if err != nil {
return
}
if err = api.DeferredUpdateIssue(apiClient, updateOpts); err != nil {
return
updateOpts.IssueID = newIssue.ID
// The issue exists by now, so failing to apply the deferred fields is
// not a failure of the whole operation. Returning the error here would
// make PreserveInput write a recovery file and print "operation
// failed", implying the issue was never created.
if updateErr := api.DeferredUpdateIssue(apiClient, updateOpts); updateErr != nil {
Comment on lines +427 to +431

@imkp1 imkp1 Jul 21, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed and fixed in 745dc68. Resolution now happens before the create mutation.

The ordering was as described: resolution ran at create.go:418, after IssueCreate at :412, and assigned to the named return, so an unknown --type or an unresolvable ref returned fatally with the issue already created — no URL, and PreserveInput writing a recovery file in interactive mode.

Split as suggested: resolveDeferredUpdateIssueOptions() takes only the client, repo and flags, runs before IssueCreate, and IssueID is attached to the resolved options afterwards. Only DeferredUpdateIssue itself stays downgraded to a warning.

Two tests cover it: the existing "type not found" case now excludes the IssueCreate mutation instead of stubbing it, and a new "unresolvable parent" case does the same for a bad ref.

fmt.Fprintf(opts.IO.ErrOut, "%s Issue created, but not all fields could be set: %s\n",
opts.IO.ColorScheme().WarningIcon(), updateErr)
}

fmt.Fprintln(opts.IO.Out, newIssue.URL)
Expand All @@ -434,12 +446,12 @@ func generatePreviewURL(apiClient *api.Client, baseRepo ghrepo.Interface, tb prS
return prShared.WithPrAndIssueQueryParams(apiClient, baseRepo, openURL, tb, projectsV1Support)
}

// deferredUpdateIssueOptions resolves the user-supplied --type / --parent /
// --blocked-by / --blocking flags into the IDs that DeferredUpdateIssue
// expects.
func deferredUpdateIssueOptions(client *api.Client, baseRepo ghrepo.Interface, issue *api.Issue, opts *CreateOptions) (api.DeferredUpdateIssueOptions, error) {
// resolveDeferredUpdateIssueOptions resolves the user-supplied --type /
// --parent / --blocked-by / --blocking flags into the IDs that
// DeferredUpdateIssue expects. It performs reads only and needs no issue ID, so
// callers run it before creating the issue and set IssueID afterwards.
func resolveDeferredUpdateIssueOptions(client *api.Client, baseRepo ghrepo.Interface, opts *CreateOptions) (api.DeferredUpdateIssueOptions, error) {
updateOpts := api.DeferredUpdateIssueOptions{
IssueID: issue.ID,
Hostname: baseRepo.RepoHost(),
}

Expand Down
203 changes: 199 additions & 4 deletions pkg/cmd/issue/create/create_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -787,6 +787,106 @@ func Test_createRun(t *testing.T) {
wantsStdout: "https://github.com/OWNER/REPO/issues/123\n",
wantsStderr: "\nCreating issue in OWNER/REPO\n\n",
},
{
// Setting a type requires triage access. Without it the picker is
// skipped entirely, so the types are never even fetched.
name: "interactive does not prompt for type without triage access",
opts: CreateOptions{
Interactive: true,
Detector: &fd.EnabledDetectorMock{},
Title: "feature request",
Body: "would be nice to have",
},
promptStubs: func(pm *prompter.PrompterMock) {
pm.SelectFunc = func(message, defaultValue string, options []string) (int, error) {
switch message {
case "What's next?":
return prompter.IndexFor(options, "Submit")
default:
return 0, fmt.Errorf("unexpected select prompt: %s", message)
}
}
},
httpStubs: func(t *testing.T, r *httpmock.Registry) {
r.Register(
httpmock.GraphQL(`query IssueRepositoryInfo\b`),
httpmock.StringResponse(`
{ "data": { "repository": {
"id": "REPOID",
"hasIssuesEnabled": true,
"viewerPermission": "READ"
} } }`))
// A failed types fetch also silently skips the picker, so
// asserting on the prompt alone would pass even ungated.
// Exclude proves the gate short-circuits before the fetch.
r.Exclude(t, httpmock.GraphQL(`query RepositoryIssueTypes\b`))
r.Register(
httpmock.GraphQL(`mutation IssueCreate\b`),
httpmock.StringResponse(`
{ "data": { "createIssue": { "issue": {
"id": "ISSUE_ID_123",
"URL": "https://github.com/OWNER/REPO/issues/123"
} } } }`))
},
wantsStdout: "https://github.com/OWNER/REPO/issues/123\n",
wantsStderr: "\nCreating issue in OWNER/REPO\n\n",
},
{
// Triage is the lowest role that can set a type, so the picker is
// still offered to it.
name: "interactive prompts for type with triage access",
opts: CreateOptions{
Interactive: true,
Detector: &fd.EnabledDetectorMock{},
Title: "feature request",
Body: "would be nice to have",
},
promptStubs: func(pm *prompter.PrompterMock) {
pm.SelectFunc = func(message, defaultValue string, options []string) (int, error) {
switch message {
case "Issue type":
return prompter.IndexFor(options, "Feature")
case "What's next?":
return prompter.IndexFor(options, "Submit")
default:
return 0, fmt.Errorf("unexpected select prompt: %s", message)
}
}
},
httpStubs: func(t *testing.T, r *httpmock.Registry) {
r.Register(
httpmock.GraphQL(`query IssueRepositoryInfo\b`),
httpmock.StringResponse(`
{ "data": { "repository": {
"id": "REPOID",
"hasIssuesEnabled": true,
"viewerPermission": "TRIAGE"
} } }`))
r.Register(
httpmock.GraphQL(`query RepositoryIssueTypes\b`),
httpmock.StringResponse(`
{ "data": { "repository": { "issueTypes": { "nodes": [
{ "id": "IT_1", "name": "Bug", "description": "", "color": "d73a4a" },
{ "id": "IT_2", "name": "Feature", "description": "", "color": "0075ca" }
] } } } }`))
r.Register(
httpmock.GraphQL(`mutation IssueCreate\b`),
httpmock.StringResponse(`
{ "data": { "createIssue": { "issue": {
"id": "ISSUE_ID_123",
"URL": "https://github.com/OWNER/REPO/issues/123"
} } } }`))
r.Register(
httpmock.GraphQL(`mutation UpdateIssueIssueType\b`),
httpmock.GraphQLMutation(`
{ "data": { "updateIssueIssueType": { "issue": { "id": "ISSUE_ID_123" } } } }`,
func(inputs map[string]interface{}) {
assert.Equal(t, "IT_2", inputs["issueTypeId"])
}))
},
wantsStdout: "https://github.com/OWNER/REPO/issues/123\n",
wantsStderr: "\nCreating issue in OWNER/REPO\n\n",
},
{
name: "create with type not found",
opts: CreateOptions{
Expand All @@ -795,6 +895,91 @@ func Test_createRun(t *testing.T) {
Body: "bug body",
IssueType: "Bugz",
},
httpStubs: func(t *testing.T, r *httpmock.Registry) {
r.Register(
httpmock.GraphQL(`query IssueRepositoryInfo\b`),
httpmock.StringResponse(`
{ "data": { "repository": {
"id": "REPOID",
"hasIssuesEnabled": true
} } }`))
r.Register(
httpmock.GraphQL(`query RepositoryIssueTypes\b`),
httpmock.StringResponse(`
{ "data": { "repository": { "issueTypes": { "nodes": [
{ "id": "IT_1", "name": "Bug", "description": "", "color": "d73a4a" },
{ "id": "IT_2", "name": "Feature", "description": "", "color": "0075ca" },
{ "id": "IT_3", "name": "Task", "description": "", "color": "e4e669" }
] } } } }`))
// An unknown type is user error, so it must fail before the
// issue exists rather than leave one behind.
r.Exclude(t, httpmock.GraphQL(`mutation IssueCreate\b`))
},
wantsErr: `type "Bugz" not found; available types: Bug, Feature, Task`,
},
{
// A reference that cannot be resolved is user error too, and is
// resolved in the same pre-create step.
name: "create with unresolvable parent",
opts: CreateOptions{
Detector: &fd.EnabledDetectorMock{},
Title: "child issue",
Body: "child body",
Parent: "999",
},
httpStubs: func(t *testing.T, r *httpmock.Registry) {
r.Register(
httpmock.GraphQL(`query IssueRepositoryInfo\b`),
httpmock.StringResponse(`
{ "data": { "repository": {
"id": "REPOID",
"hasIssuesEnabled": true
} } }`))
r.Register(
httpmock.GraphQL(`query IssueNodeID\b`),
httpmock.StringResponse(`
{ "errors": [ { "type": "NOT_FOUND", "message": "Could not resolve to an Issue with the number of 999." } ] }`))
r.Exclude(t, httpmock.GraphQL(`mutation IssueCreate\b`))
},
wantsErr: `resolving --parent reference "999": GraphQL: Could not resolve to an Issue with the number of 999.`,
},
{
// Refs are resolved in a loop, so a later ref failing after an
// earlier one succeeded must still abort before the issue exists.
name: "create with unresolvable blocked-by among several",
opts: CreateOptions{
Detector: &fd.EnabledDetectorMock{},
Title: "blocked issue",
Body: "blocked body",
BlockedBy: []string{"200", "999"},
},
httpStubs: func(t *testing.T, r *httpmock.Registry) {
r.Register(
httpmock.GraphQL(`query IssueRepositoryInfo\b`),
httpmock.StringResponse(`
{ "data": { "repository": {
"id": "REPOID",
"hasIssuesEnabled": true
} } }`))
r.Register(
issueNodeIDByNumberMatcher(200),
httpmock.StringResponse(`{ "data": { "repository": { "issue": { "id": "BLOCKER_ID_200" } } } }`))
r.Register(
issueNodeIDByNumberMatcher(999),
httpmock.StringResponse(`
{ "errors": [ { "type": "NOT_FOUND", "message": "Could not resolve to an Issue with the number of 999." } ] }`))
r.Exclude(t, httpmock.GraphQL(`mutation IssueCreate\b`))
},
wantsErr: `resolving --blocked-by reference "999": GraphQL: Could not resolve to an Issue with the number of 999.`,
},
{
name: "create with type that cannot be set",
opts: CreateOptions{
Detector: &fd.EnabledDetectorMock{},
Title: "bug title",
Body: "bug body",
IssueType: "Bug",
},
httpStubs: func(_ *testing.T, r *httpmock.Registry) {
r.Register(
httpmock.GraphQL(`query IssueRepositoryInfo\b`),
Expand All @@ -814,12 +999,22 @@ func Test_createRun(t *testing.T) {
httpmock.GraphQL(`query RepositoryIssueTypes\b`),
httpmock.StringResponse(`
{ "data": { "repository": { "issueTypes": { "nodes": [
{ "id": "IT_1", "name": "Bug", "description": "", "color": "d73a4a" },
{ "id": "IT_2", "name": "Feature", "description": "", "color": "0075ca" },
{ "id": "IT_3", "name": "Task", "description": "", "color": "e4e669" }
{ "id": "IT_1", "name": "Bug", "description": "", "color": "d73a4a" }
] } } } }`))
// The issue exists by now; the type cannot be applied because
// the user lacks permission on the repository.
r.Register(
httpmock.GraphQL(`mutation UpdateIssueIssueType\b`),
httpmock.StringResponse(`
{ "errors": [
{
"type": "FORBIDDEN",
"message": "monalisa does not have the correct permissions to execute `+"`UpdateIssueIssueType`"+`"
}
] }`))
},
wantsErr: `type "Bugz" not found; available types: Bug, Feature, Task`,
wantsStdout: "https://github.com/OWNER/REPO/issues/123\n",
wantsStderr: "\nCreating issue in OWNER/REPO\n\n! Issue created, but not all fields could be set: GraphQL: monalisa does not have the correct permissions to execute `UpdateIssueIssueType`\n",
},
{
name: "create with parent",
Expand Down