From 6b56a239704caf59a14e82a9d8124509c2d77cbe Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 4 Mar 2026 13:24:05 -0700 Subject: [PATCH 1/2] Remove unnecessary StateReasonDuplicate feature detection The DUPLICATE enum variant for IssueClosedStateReason was added in GHES 3.16, which is older than the earliest supported GHES version. The feature detection check is therefore unnecessary. Addresses: https://github.com/cli/cli/pull/12811#issuecomment-3997044372 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../featuredetection/feature_detection.go | 29 ++------ .../feature_detection_test.go | 45 ++--------- pkg/cmd/issue/close/close.go | 6 -- pkg/cmd/issue/close/close_test.go | 74 ------------------- 4 files changed, 14 insertions(+), 140 deletions(-) diff --git a/internal/featuredetection/feature_detection.go b/internal/featuredetection/feature_detection.go index e40c134bc..7a200e20c 100644 --- a/internal/featuredetection/feature_detection.go +++ b/internal/featuredetection/feature_detection.go @@ -23,15 +23,13 @@ type Detector interface { } type IssueFeatures struct { - StateReason bool - StateReasonDuplicate bool - ActorIsAssignable bool + StateReason bool + ActorIsAssignable bool } var allIssueFeatures = IssueFeatures{ - StateReason: true, - StateReasonDuplicate: true, - ActorIsAssignable: true, + StateReason: true, + ActorIsAssignable: true, } type PullRequestFeatures struct { @@ -140,9 +138,8 @@ func (d *detector) IssueFeatures() (IssueFeatures, error) { } features := IssueFeatures{ - StateReason: false, - StateReasonDuplicate: false, - ActorIsAssignable: false, // replaceActorsForAssignable GraphQL mutation unavailable on GHES + StateReason: false, + ActorIsAssignable: false, // replaceActorsForAssignable GraphQL mutation unavailable on GHES } var featureDetection struct { @@ -151,11 +148,6 @@ func (d *detector) IssueFeatures() (IssueFeatures, error) { Name string } `graphql:"fields(includeDeprecated: true)"` } `graphql:"Issue: __type(name: \"Issue\")"` - IssueClosedStateReason struct { - EnumValues []struct { - Name string - } `graphql:"enumValues(includeDeprecated: true)"` - } `graphql:"IssueClosedStateReason: __type(name: \"IssueClosedStateReason\")"` } gql := api.NewClientFromHTTP(d.httpClient) @@ -170,15 +162,6 @@ func (d *detector) IssueFeatures() (IssueFeatures, error) { } } - if features.StateReason { - for _, enumValue := range featureDetection.IssueClosedStateReason.EnumValues { - if enumValue.Name == "DUPLICATE" { - features.StateReasonDuplicate = true - break - } - } - } - return features, nil } diff --git a/internal/featuredetection/feature_detection_test.go b/internal/featuredetection/feature_detection_test.go index 7417cc7d8..032f5cda0 100644 --- a/internal/featuredetection/feature_detection_test.go +++ b/internal/featuredetection/feature_detection_test.go @@ -23,9 +23,8 @@ func TestIssueFeatures(t *testing.T) { name: "github.com", hostname: "github.com", wantFeatures: IssueFeatures{ - StateReason: true, - StateReasonDuplicate: true, - ActorIsAssignable: true, + StateReason: true, + ActorIsAssignable: true, }, wantErr: false, }, @@ -33,9 +32,8 @@ func TestIssueFeatures(t *testing.T) { name: "ghec data residency (ghe.com)", hostname: "stampname.ghe.com", wantFeatures: IssueFeatures{ - StateReason: true, - StateReasonDuplicate: true, - ActorIsAssignable: true, + StateReason: true, + ActorIsAssignable: true, }, wantErr: false, }, @@ -46,50 +44,23 @@ func TestIssueFeatures(t *testing.T) { `query Issue_fields\b`: `{"data": {}}`, }, wantFeatures: IssueFeatures{ - StateReason: false, - StateReasonDuplicate: false, - ActorIsAssignable: false, + StateReason: false, + ActorIsAssignable: false, }, wantErr: false, }, { - name: "GHE has state reason field without duplicate enum", + name: "GHE has state reason field", hostname: "git.my.org", queryResponse: map[string]string{ `query Issue_fields\b`: heredoc.Doc(` { "data": { "Issue": { "fields": [ {"name": "stateReason"} - ] }, "IssueClosedStateReason": { "enumValues": [ - {"name": "COMPLETED"}, - {"name": "NOT_PLANNED"} ] } } } `), }, wantFeatures: IssueFeatures{ - StateReason: true, - StateReasonDuplicate: false, - ActorIsAssignable: false, - }, - wantErr: false, - }, - { - name: "GHE has duplicate state reason enum value", - hostname: "git.my.org", - queryResponse: map[string]string{ - `query Issue_fields\b`: heredoc.Doc(` - { "data": { "Issue": { "fields": [ - {"name": "stateReason"} - ] }, "IssueClosedStateReason": { "enumValues": [ - {"name": "COMPLETED"}, - {"name": "NOT_PLANNED"}, - {"name": "DUPLICATE"} - ] } } } - `), - }, - wantFeatures: IssueFeatures{ - StateReason: true, - StateReasonDuplicate: true, - ActorIsAssignable: false, + StateReason: true, }, wantErr: false, }, diff --git a/pkg/cmd/issue/close/close.go b/pkg/cmd/issue/close/close.go index 4c1c7d382..ab7e30688 100644 --- a/pkg/cmd/issue/close/close.go +++ b/pkg/cmd/issue/close/close.go @@ -196,12 +196,6 @@ func apiClose(httpClient *http.Client, repo ghrepo.Interface, issue *api.Issue, return fmt.Errorf("closing as duplicate is not supported on %s", repo.RepoHost()) } reason = "" - } else if reason == "duplicate" && !features.StateReasonDuplicate { - if duplicateIssueID != "" { - return fmt.Errorf("closing as duplicate is not supported on %s", repo.RepoHost()) - } - // If DUPLICATE is not supported silently close issue without setting StateReason. - reason = "" } } diff --git a/pkg/cmd/issue/close/close_test.go b/pkg/cmd/issue/close/close_test.go index ddab71210..300fe5fc5 100644 --- a/pkg/cmd/issue/close/close_test.go +++ b/pkg/cmd/issue/close/close_test.go @@ -16,15 +16,6 @@ import ( "github.com/stretchr/testify/require" ) -type issueFeaturesDetectorMock struct { - fd.EnabledDetectorMock - issueFeatures fd.IssueFeatures -} - -func (md *issueFeaturesDetectorMock) IssueFeatures() (fd.IssueFeatures, error) { - return md.issueFeatures, nil -} - func TestNewCmdClose(t *testing.T) { // Test shared parsing of issue number / URL. argparsetest.TestArgParsing(t, NewCmdClose) @@ -282,71 +273,6 @@ func TestCloseRun(t *testing.T) { }, wantStderr: "✓ Closed issue OWNER/REPO#13 (The title of the issue)\n", }, - { - name: "close issue with duplicate reason when duplicate is not supported", - opts: &CloseOptions{ - IssueNumber: 13, - Reason: "duplicate", - Detector: &issueFeaturesDetectorMock{ - issueFeatures: fd.IssueFeatures{ - StateReason: true, - StateReasonDuplicate: false, - }, - }, - }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.GraphQL(`query IssueByNumber\b`), - httpmock.StringResponse(` - { "data": { "repository": { - "hasIssuesEnabled": true, - "issue": { "id": "THE-ID", "number": 13, "title": "The title of the issue"} - } } }`), - ) - reg.Register( - httpmock.GraphQL(`mutation IssueClose\b`), - httpmock.GraphQLMutation(`{"id": "THE-ID"}`, - func(inputs map[string]interface{}) { - assert.Equal(t, 1, len(inputs)) - assert.Equal(t, "THE-ID", inputs["issueId"]) - }), - ) - }, - wantStderr: "✓ Closed issue OWNER/REPO#13 (The title of the issue)\n", - }, - { - name: "close issue as duplicate when duplicate is not supported", - opts: &CloseOptions{ - IssueNumber: 13, - DuplicateOf: "99", - Detector: &issueFeaturesDetectorMock{ - issueFeatures: fd.IssueFeatures{ - StateReason: true, - StateReasonDuplicate: false, - }, - }, - }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.GraphQL(`query IssueByNumber\b`), - httpmock.StringResponse(` - { "data": { "repository": { - "hasIssuesEnabled": true, - "issue": { "id": "THE-ID", "number": 13, "title": "The title of the issue"} - } } }`), - ) - reg.Register( - httpmock.GraphQL(`query IssueByNumber\b`), - httpmock.StringResponse(` - { "data": { "repository": { - "hasIssuesEnabled": true, - "issue": { "id": "DUPLICATE-ID", "number": 99} - } } }`), - ) - }, - wantErr: true, - errMsg: "closing as duplicate is not supported on github.com", - }, { name: "duplicate of cannot point to same issue", opts: &CloseOptions{ From 4f2304d4e5d69e9981b55020212594a642f3d71f Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 4 Mar 2026 13:31:30 -0700 Subject: [PATCH 2/2] Remove StateReason feature detection for issue close The stateReason field was added in GHES ~3.4, which is far older than the earliest supported GHES version (3.14). The feature detection and conditional inclusion of stateReason is therefore unnecessary. This removes: - StateReason field from IssueFeatures struct - GHES introspection query in IssueFeatures() (only ActorIsAssignable remains, which is always false on GHES) - Conditional stateReason field inclusion in issue list - Feature detection guard in issue close - Feature detection guard in FindIssueOrPR Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../featuredetection/feature_detection.go | 29 ++--------------- .../feature_detection_test.go | 23 +------------- pkg/cmd/issue/close/close.go | 27 ++-------------- pkg/cmd/issue/close/close_test.go | 31 ------------------- pkg/cmd/issue/list/list.go | 10 +----- pkg/cmd/issue/shared/lookup.go | 14 --------- 6 files changed, 6 insertions(+), 128 deletions(-) diff --git a/internal/featuredetection/feature_detection.go b/internal/featuredetection/feature_detection.go index 7a200e20c..9af4c5aec 100644 --- a/internal/featuredetection/feature_detection.go +++ b/internal/featuredetection/feature_detection.go @@ -23,12 +23,10 @@ type Detector interface { } type IssueFeatures struct { - StateReason bool ActorIsAssignable bool } var allIssueFeatures = IssueFeatures{ - StateReason: true, ActorIsAssignable: true, } @@ -137,32 +135,9 @@ func (d *detector) IssueFeatures() (IssueFeatures, error) { return allIssueFeatures, nil } - features := IssueFeatures{ - StateReason: false, + return IssueFeatures{ ActorIsAssignable: false, // replaceActorsForAssignable GraphQL mutation unavailable on GHES - } - - var featureDetection struct { - Issue struct { - Fields []struct { - Name string - } `graphql:"fields(includeDeprecated: true)"` - } `graphql:"Issue: __type(name: \"Issue\")"` - } - - gql := api.NewClientFromHTTP(d.httpClient) - err := gql.Query(d.host, "Issue_fields", &featureDetection, nil) - if err != nil { - return features, err - } - - for _, field := range featureDetection.Issue.Fields { - if field.Name == "stateReason" { - features.StateReason = true - } - } - - return features, nil + }, nil } func (d *detector) PullRequestFeatures() (PullRequestFeatures, error) { diff --git a/internal/featuredetection/feature_detection_test.go b/internal/featuredetection/feature_detection_test.go index 032f5cda0..82132ab83 100644 --- a/internal/featuredetection/feature_detection_test.go +++ b/internal/featuredetection/feature_detection_test.go @@ -23,7 +23,6 @@ func TestIssueFeatures(t *testing.T) { name: "github.com", hostname: "github.com", wantFeatures: IssueFeatures{ - StateReason: true, ActorIsAssignable: true, }, wantErr: false, @@ -32,38 +31,18 @@ func TestIssueFeatures(t *testing.T) { name: "ghec data residency (ghe.com)", hostname: "stampname.ghe.com", wantFeatures: IssueFeatures{ - StateReason: true, ActorIsAssignable: true, }, wantErr: false, }, { - name: "GHE empty response", + name: "GHE", hostname: "git.my.org", - queryResponse: map[string]string{ - `query Issue_fields\b`: `{"data": {}}`, - }, wantFeatures: IssueFeatures{ - StateReason: false, ActorIsAssignable: false, }, wantErr: false, }, - { - name: "GHE has state reason field", - hostname: "git.my.org", - queryResponse: map[string]string{ - `query Issue_fields\b`: heredoc.Doc(` - { "data": { "Issue": { "fields": [ - {"name": "stateReason"} - ] } } } - `), - }, - wantFeatures: IssueFeatures{ - StateReason: true, - }, - wantErr: false, - }, } for _, tt := range tests { diff --git a/pkg/cmd/issue/close/close.go b/pkg/cmd/issue/close/close.go index ab7e30688..d9b02b6a2 100644 --- a/pkg/cmd/issue/close/close.go +++ b/pkg/cmd/issue/close/close.go @@ -3,11 +3,9 @@ package close import ( "fmt" "net/http" - "time" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" - fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmd/issue/shared" prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared" @@ -26,8 +24,6 @@ type CloseOptions struct { Comment string Reason string DuplicateOf string - - Detector fd.Detector } func NewCmdClose(f *cmdutil.Factory, runF func(*CloseOptions) error) *cobra.Command { @@ -165,7 +161,7 @@ func closeRun(opts *CloseOptions) error { } } - err = apiClose(httpClient, baseRepo, issue, opts.Detector, closeReason, duplicateIssueID) + err = apiClose(httpClient, baseRepo, issue, closeReason, duplicateIssueID) if err != nil { return err } @@ -175,30 +171,11 @@ func closeRun(opts *CloseOptions) error { return nil } -func apiClose(httpClient *http.Client, repo ghrepo.Interface, issue *api.Issue, detector fd.Detector, reason string, duplicateIssueID string) error { +func apiClose(httpClient *http.Client, repo ghrepo.Interface, issue *api.Issue, reason string, duplicateIssueID string) error { if issue.IsPullRequest() { return api.PullRequestClose(httpClient, repo, issue.ID) } - if reason != "" || duplicateIssueID != "" { - if detector == nil { - cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24) - detector = fd.NewDetector(cachedClient, repo.RepoHost()) - } - features, err := detector.IssueFeatures() - if err != nil { - return err - } - // TODO stateReasonCleanup - if !features.StateReason { - // If StateReason is not supported silently close issue without setting StateReason. - if duplicateIssueID != "" { - return fmt.Errorf("closing as duplicate is not supported on %s", repo.RepoHost()) - } - reason = "" - } - } - switch reason { case "": // If no reason is specified do not set it. diff --git a/pkg/cmd/issue/close/close_test.go b/pkg/cmd/issue/close/close_test.go index 300fe5fc5..e7dfa1627 100644 --- a/pkg/cmd/issue/close/close_test.go +++ b/pkg/cmd/issue/close/close_test.go @@ -5,7 +5,6 @@ import ( "net/http" "testing" - fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmd/issue/argparsetest" "github.com/cli/cli/v2/pkg/cmdutil" @@ -185,7 +184,6 @@ func TestCloseRun(t *testing.T) { opts: &CloseOptions{ IssueNumber: 13, Reason: "not planned", - Detector: &fd.EnabledDetectorMock{}, }, httpStubs: func(reg *httpmock.Registry) { reg.Register( @@ -213,7 +211,6 @@ func TestCloseRun(t *testing.T) { opts: &CloseOptions{ IssueNumber: 13, Reason: "duplicate", - Detector: &fd.EnabledDetectorMock{}, }, httpStubs: func(reg *httpmock.Registry) { reg.Register( @@ -241,7 +238,6 @@ func TestCloseRun(t *testing.T) { opts: &CloseOptions{ IssueNumber: 13, DuplicateOf: "99", - Detector: &fd.EnabledDetectorMock{}, }, httpStubs: func(reg *httpmock.Registry) { reg.Register( @@ -338,33 +334,6 @@ func TestCloseRun(t *testing.T) { wantErr: true, errMsg: "invalid value for `--duplicate-of`: invalid issue format: \"not-an-issue\"", }, - { - name: "close issue with reason when reason is not supported", - opts: &CloseOptions{ - IssueNumber: 13, - Reason: "not planned", - Detector: &fd.DisabledDetectorMock{}, - }, - httpStubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.GraphQL(`query IssueByNumber\b`), - httpmock.StringResponse(` - { "data": { "repository": { - "hasIssuesEnabled": true, - "issue": { "id": "THE-ID", "number": 13, "title": "The title of the issue"} - } } }`), - ) - reg.Register( - httpmock.GraphQL(`mutation IssueClose\b`), - httpmock.GraphQLMutation(`{"id": "THE-ID"}`, - func(inputs map[string]interface{}) { - assert.Equal(t, 1, len(inputs)) - assert.Equal(t, "THE-ID", inputs["issueId"]) - }), - ) - }, - wantStderr: "✓ Closed issue OWNER/REPO#13 (The title of the issue)\n", - }, { name: "issue already closed", opts: &CloseOptions{ diff --git a/pkg/cmd/issue/list/list.go b/pkg/cmd/issue/list/list.go index f5a0a4429..d58357ac4 100644 --- a/pkg/cmd/issue/list/list.go +++ b/pkg/cmd/issue/list/list.go @@ -147,15 +147,7 @@ func listRun(opts *ListOptions) error { cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24) opts.Detector = fd.NewDetector(cachedClient, baseRepo.RepoHost()) } - features, err := opts.Detector.IssueFeatures() - if err != nil { - return err - } - fields := defaultFields - // TODO stateReasonCleanup - if features.StateReason { - fields = append(defaultFields, "stateReason") - } + fields := append(defaultFields, "stateReason") filterOptions := prShared.FilterOptions{ Entity: "issue", diff --git a/pkg/cmd/issue/shared/lookup.go b/pkg/cmd/issue/shared/lookup.go index 2975ad484..63efd61f7 100644 --- a/pkg/cmd/issue/shared/lookup.go +++ b/pkg/cmd/issue/shared/lookup.go @@ -8,10 +8,8 @@ import ( "regexp" "strconv" "strings" - "time" "github.com/cli/cli/v2/api" - fd "github.com/cli/cli/v2/internal/featuredetection" "github.com/cli/cli/v2/internal/ghrepo" o "github.com/cli/cli/v2/pkg/option" "github.com/cli/cli/v2/pkg/set" @@ -138,18 +136,6 @@ func FindIssuesOrPRs(httpClient *http.Client, repo ghrepo.Interface, issueNumber func FindIssueOrPR(httpClient *http.Client, repo ghrepo.Interface, number int, fields []string) (*api.Issue, error) { fieldSet := set.NewStringSet() fieldSet.AddValues(fields) - if fieldSet.Contains("stateReason") { - cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24) - detector := fd.NewDetector(cachedClient, repo.RepoHost()) - features, err := detector.IssueFeatures() - if err != nil { - return nil, err - } - // TODO stateReasonCleanup - if !features.StateReason { - fieldSet.Remove("stateReason") - } - } var getProjectItems bool if fieldSet.Contains("projectItems") {