Skip to content

Sort GH_AW_RATE_LIMIT_EVENTS alphabetically #15018

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

Merged
pelikhan merged 5 commits into from
Feb 11, 2026
Merged

Sort GH_AW_RATE_LIMIT_EVENTS alphabetically #15018

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b36ae88
Initial plan
Copilot Feb 11, 2026
8f09609
Initial plan: Sort GH_AW_RATE_LIMIT_EVENTS alphabetically
Copilot Feb 11, 2026
f26ac65
Sort GH_AW_RATE_LIMIT_EVENTS alphabetically
Copilot Feb 11, 2026
73b2275
Add sorting for GH_AW_RATE_LIMIT_EVENTS in generated workflows
Copilot Feb 11, 2026
54108d2
Merge branch 'main' into copilot/sort-gh-aw-rate-limit-events
pelikhan Feb 11, 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
2 changes: 1 addition & 1 deletion .github/workflows/ai-moderator.lock.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 13 additions & 6 deletions pkg/workflow/role_checks.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package workflow

import (
"fmt"
"sort"
"strings"

"github.com/github/gh-aw/pkg/constants"
Expand Down Expand Up @@ -62,7 +63,11 @@ func (c *Compiler) generateRateLimitCheck(data *WorkflowData, steps []string) []

// Set events to check (if specified)
if len(data.RateLimit.Events) > 0 {
steps = append(steps, fmt.Sprintf(" GH_AW_RATE_LIMIT_EVENTS: %q\n", strings.Join(data.RateLimit.Events, ",")))
// Sort events alphabetically for consistent output
events := make([]string, len(data.RateLimit.Events))
copy(events, data.RateLimit.Events)
sort.Strings(events)
steps = append(steps, fmt.Sprintf(" GH_AW_RATE_LIMIT_EVENTS: %q\n", strings.Join(events, ",")))
}

steps = append(steps, " with:\n")
Expand Down Expand Up @@ -211,15 +216,15 @@ func (c *Compiler) inferEventsFromTriggers(frontmatter map[string]any) []string

var events []string
programmaticTriggers := map[string]string{
"workflow_dispatch": "workflow_dispatch",
"repository_dispatch": "repository_dispatch",
"issues": "issues",
"discussion": "discussion",
"discussion_comment": "discussion_comment",
"issue_comment": "issue_comment",
"issues": "issues",
"pull_request": "pull_request",
"pull_request_review": "pull_request_review",
"pull_request_review_comment": "pull_request_review_comment",
"discussion": "discussion",
"discussion_comment": "discussion_comment",
"repository_dispatch": "repository_dispatch",
"workflow_dispatch": "workflow_dispatch",
}
Comment on lines 218 to 228
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

Alphabetizing the programmaticTriggers map in the source code does not make the events slice deterministically ordered. Go map iteration order is non-deterministic - when iterating over the input 'on' map (lines 227-230 in the full function), events will be appended in random order. The test now expects alphabetical order, but the implementation doesn't sort. You need to add sort.Strings(events) before returning the events slice (after line 244 in the full function) and import the "sort" package.

Copilot uses AI. Check for mistakes.

switch on := onValue.(type) {
Expand All @@ -243,6 +248,8 @@ func (c *Compiler) inferEventsFromTriggers(frontmatter map[string]any) []string
}
}

// Sort events alphabetically for consistent output
sort.Strings(events)
return events
}

Expand Down
20 changes: 8 additions & 12 deletions pkg/workflow/role_checks_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ func TestInferEventsFromTriggers(t *testing.T) {
"workflow_dispatch": nil,
},
},
expected: []string{"issues", "issue_comment", "workflow_dispatch"},
expected: []string{"issue_comment", "issues", "workflow_dispatch"},
},
{
name: "infer only programmatic triggers",
Expand Down Expand Up @@ -206,28 +206,24 @@ func TestInferEventsFromTriggers(t *testing.T) {
},
},
expected: []string{
"workflow_dispatch",
"repository_dispatch",
"issues",
"discussion",
"discussion_comment",
"issue_comment",
"issues",
"pull_request",
"pull_request_review",
"pull_request_review_comment",
"discussion",
"discussion_comment",
"repository_dispatch",
"workflow_dispatch",
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := c.inferEventsFromTriggers(tt.frontmatter)
// Use ElementsMatch since map iteration order is non-deterministic
if len(tt.expected) > 0 && len(result) > 0 {
assert.ElementsMatch(t, tt.expected, result, "Inferred events should match expected")
} else {
assert.Equal(t, tt.expected, result, "Inferred events should match expected")
}
// Events should be sorted alphabetically
assert.Equal(t, tt.expected, result, "Inferred events should match expected (in sorted order)")
})
}
}
Loading