refactor(gate): consolidate numeric ID check and improve workflow matching
Code review improvements: - Make isNumericRunID delegate to isNumericID (DRY) - Extract workflowNameMatches() for clearer, more robust matching - Handle hint→filename matching (e.g., "release" matches "release.yml") - Add TestWorkflowNameMatches with comprehensive test cases 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Executed-By: beads/crew/dave Rig: beads Role: crew
This commit is contained in:
@@ -145,17 +145,10 @@ func runGateDiscover(cmd *cobra.Command, args []string) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// isNumericRunID returns true if the string looks like a GitHub numeric run ID
|
// isNumericRunID returns true if the string looks like a GitHub numeric run ID.
|
||||||
|
// This is a local alias for consistency - the canonical implementation is isNumericID in gate.go.
|
||||||
func isNumericRunID(s string) bool {
|
func isNumericRunID(s string) bool {
|
||||||
if s == "" {
|
return isNumericID(s)
|
||||||
return false
|
|
||||||
}
|
|
||||||
for _, c := range s {
|
|
||||||
if c < '0' || c > '9' {
|
|
||||||
return false
|
|
||||||
}
|
|
||||||
}
|
|
||||||
return true
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// needsDiscovery returns true if a gh:run gate needs run ID discovery.
|
// needsDiscovery returns true if a gh:run gate needs run ID discovery.
|
||||||
@@ -177,6 +170,33 @@ func getWorkflowNameHint(g *types.Issue) string {
|
|||||||
return g.AwaitID
|
return g.AwaitID
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// workflowNameMatches checks if a workflow hint matches a GitHub workflow run.
|
||||||
|
// It handles various naming conventions:
|
||||||
|
// - Exact match (case-insensitive)
|
||||||
|
// - Hint with .yml/.yaml suffix vs display name without
|
||||||
|
// - Hint without suffix vs filename with .yml/.yaml
|
||||||
|
func workflowNameMatches(hint, workflowName, runName string) bool {
|
||||||
|
// Normalize hint by removing .yml/.yaml suffix for comparison
|
||||||
|
hintBase := strings.TrimSuffix(strings.TrimSuffix(hint, ".yml"), ".yaml")
|
||||||
|
|
||||||
|
// Exact matches (case-insensitive)
|
||||||
|
if strings.EqualFold(workflowName, hint) || strings.EqualFold(runName, hint) {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
|
||||||
|
// Match hint base against workflow display name
|
||||||
|
if strings.EqualFold(workflowName, hintBase) {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
|
||||||
|
// Match hint (with suffix added) against run filename
|
||||||
|
if strings.EqualFold(runName, hintBase+".yml") || strings.EqualFold(runName, hintBase+".yaml") {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
// findPendingGates returns open gh:run gates that need run ID discovery.
|
// findPendingGates returns open gh:run gates that need run ID discovery.
|
||||||
// This includes gates with empty AwaitID OR non-numeric AwaitID (workflow name hint).
|
// This includes gates with empty AwaitID OR non-numeric AwaitID (workflow name hint).
|
||||||
func findPendingGates() ([]*types.Issue, error) {
|
func findPendingGates() ([]*types.Issue, error) {
|
||||||
@@ -305,10 +325,7 @@ func matchGateToRun(gate *types.Issue, runs []GHWorkflowRun, maxAge time.Duratio
|
|||||||
// If gate has a workflow name hint, require matching workflow
|
// If gate has a workflow name hint, require matching workflow
|
||||||
// Match against both WorkflowName (display name) and Name (filename)
|
// Match against both WorkflowName (display name) and Name (filename)
|
||||||
if workflowHint != "" {
|
if workflowHint != "" {
|
||||||
workflowMatches := strings.EqualFold(run.WorkflowName, workflowHint) ||
|
workflowMatches := workflowNameMatches(workflowHint, run.WorkflowName, run.Name)
|
||||||
strings.EqualFold(run.Name, workflowHint) ||
|
|
||||||
strings.EqualFold(run.WorkflowName, strings.TrimSuffix(workflowHint, ".yml")) ||
|
|
||||||
strings.EqualFold(run.WorkflowName, strings.TrimSuffix(workflowHint, ".yaml"))
|
|
||||||
if !workflowMatches {
|
if !workflowMatches {
|
||||||
continue // Skip runs that don't match the workflow hint
|
continue // Skip runs that don't match the workflow hint
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -292,6 +292,45 @@ func TestGetWorkflowNameHint(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestWorkflowNameMatches(t *testing.T) {
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
hint string
|
||||||
|
workflowName string
|
||||||
|
runName string
|
||||||
|
want bool
|
||||||
|
}{
|
||||||
|
// Exact matches
|
||||||
|
{"exact workflow name", "Release", "Release", "release.yml", true},
|
||||||
|
{"exact run name", "release.yml", "Release", "release.yml", true},
|
||||||
|
{"case insensitive workflow", "release", "Release", "release.yml", true},
|
||||||
|
{"case insensitive run", "RELEASE.YML", "Release", "release.yml", true},
|
||||||
|
|
||||||
|
// Hint with suffix, match display name without
|
||||||
|
{"hint yml vs display name", "release.yml", "release", "ci.yml", true},
|
||||||
|
{"hint yaml vs display name", "release.yaml", "release", "ci.yaml", true},
|
||||||
|
|
||||||
|
// Hint without suffix, match filename with suffix
|
||||||
|
{"hint base vs filename yml", "release", "CI", "release.yml", true},
|
||||||
|
{"hint base vs filename yaml", "release", "CI", "release.yaml", true},
|
||||||
|
|
||||||
|
// No match
|
||||||
|
{"no match different name", "release", "CI", "ci.yml", false},
|
||||||
|
{"no match partial", "rel", "Release", "release.yml", false},
|
||||||
|
{"empty hint", "", "Release", "release.yml", false},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
got := workflowNameMatches(tt.hint, tt.workflowName, tt.runName)
|
||||||
|
if got != tt.want {
|
||||||
|
t.Errorf("workflowNameMatches(%q, %q, %q) = %v, want %v",
|
||||||
|
tt.hint, tt.workflowName, tt.runName, got, tt.want)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// gateTestContainsIgnoreCase checks if haystack contains needle (case-insensitive)
|
// gateTestContainsIgnoreCase checks if haystack contains needle (case-insensitive)
|
||||||
func gateTestContainsIgnoreCase(haystack, needle string) bool {
|
func gateTestContainsIgnoreCase(haystack, needle string) bool {
|
||||||
return gateTestContains(gateTestLowerCase(haystack), gateTestLowerCase(needle))
|
return gateTestContains(gateTestLowerCase(haystack), gateTestLowerCase(needle))
|
||||||
|
|||||||
Reference in New Issue
Block a user