Skip to content

Commit 7b30ee5

Browse files
dsymeCopilot
andauthored
🚦 Improve Workflow Polling and Signal Handling (#4002)
* fixes to waiting and repo reuse * update disable logic and messages * Update pkg/cli/signal_aware_poll.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 88365d7 commit 7b30ee5

7 files changed

Lines changed: 559 additions & 82 deletions

File tree

‎pkg/cli/enable.go‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,3 +250,105 @@ func toggleWorkflowsByNames(workflowNames []string, enable bool) error {
250250

251251
return nil
252252
}
253+
254+
// DisableAllWorkflowsExcept disables all workflows except the specified ones
255+
// Typically used to disable all workflows except the one being trialled
256+
func DisableAllWorkflowsExcept(repoSlug string, exceptWorkflows []string, verbose bool) error {
257+
workflowsDir := ".github/workflows"
258+
259+
// Check if workflows directory exists
260+
if _, err := os.Stat(workflowsDir); os.IsNotExist(err) {
261+
if verbose {
262+
fmt.Fprintln(os.Stderr, console.FormatInfoMessage("No .github/workflows directory found, nothing to disable"))
263+
}
264+
return nil
265+
}
266+
267+
// Get all .yml and .yaml files
268+
ymlFiles, _ := filepath.Glob(filepath.Join(workflowsDir, "*.yml"))
269+
yamlFiles, _ := filepath.Glob(filepath.Join(workflowsDir, "*.yaml"))
270+
allYAMLFiles := append(ymlFiles, yamlFiles...)
271+
272+
if len(allYAMLFiles) == 0 {
273+
if verbose {
274+
fmt.Fprintln(os.Stderr, console.FormatInfoMessage("No YAML workflow files found"))
275+
}
276+
return nil
277+
}
278+
279+
// Create a set of workflows to keep enabled
280+
keepEnabled := make(map[string]bool)
281+
for _, workflowName := range exceptWorkflows {
282+
// Add both .md and .lock.yml variants
283+
keepEnabled[workflowName+".md"] = true
284+
keepEnabled[workflowName+".lock.yml"] = true
285+
keepEnabled[workflowName] = true // In case the full filename is provided
286+
}
287+
288+
// Filter to find workflows to disable
289+
var workflowsToDisable []string
290+
291+
for _, yamlFile := range allYAMLFiles {
292+
base := filepath.Base(yamlFile)
293+
294+
// Skip if it's in the keep-enabled set
295+
if keepEnabled[base] {
296+
if verbose {
297+
fmt.Fprintf(os.Stderr, "Keeping enabled: %s\n", base)
298+
}
299+
continue
300+
}
301+
302+
// Check if the base name without extension matches
303+
nameWithoutExt := strings.TrimSuffix(base, filepath.Ext(base))
304+
if keepEnabled[nameWithoutExt] {
305+
if verbose {
306+
fmt.Fprintf(os.Stderr, "Keeping enabled: %s\n", base)
307+
}
308+
continue
309+
}
310+
311+
workflowsToDisable = append(workflowsToDisable, base)
312+
}
313+
314+
if len(workflowsToDisable) == 0 {
315+
if verbose {
316+
fmt.Fprintln(os.Stderr, console.FormatInfoMessage("No workflows to disable"))
317+
}
318+
return nil
319+
}
320+
321+
// Show what will be disabled
322+
fmt.Fprintf(os.Stderr, "Disabling %d workflow(s) in cloned repository:\n", len(workflowsToDisable))
323+
for _, workflow := range workflowsToDisable {
324+
fmt.Fprintf(os.Stderr, " %s\n", workflow)
325+
}
326+
327+
// Disable each workflow
328+
var failures []string
329+
for _, workflow := range workflowsToDisable {
330+
args := []string{"workflow", "disable", workflow}
331+
if repoSlug != "" {
332+
args = append(args, "--repo", repoSlug)
333+
}
334+
335+
cmd := exec.Command("gh", args...)
336+
if output, err := cmd.CombinedOutput(); err != nil {
337+
if verbose {
338+
fmt.Fprintf(os.Stderr, "Warning: Failed to disable workflow %s: %v\n%s\n", workflow, err, string(output))
339+
}
340+
failures = append(failures, workflow)
341+
} else {
342+
if verbose {
343+
fmt.Fprintf(os.Stderr, "Disabled workflow: %s\n", workflow)
344+
}
345+
}
346+
}
347+
348+
if len(failures) > 0 {
349+
return fmt.Errorf("failed to disable %d workflow(s): %s", len(failures), strings.Join(failures, ", "))
350+
}
351+
352+
fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("Disabled %d workflow(s)", len(workflowsToDisable))))
353+
return nil
354+
}

‎pkg/cli/pr_automerge.go‎

Lines changed: 30 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -120,52 +120,41 @@ func AutoMergePullRequestsLegacy(repoSlug string, verbose bool) error {
120120
func WaitForWorkflowCompletion(repoSlug, runID string, timeoutMinutes int, verbose bool) error {
121121
prAutomergeLog.Printf("Waiting for workflow completion: repo=%s, runID=%s, timeout=%d minutes", repoSlug, runID, timeoutMinutes)
122122

123-
if verbose {
124-
fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Waiting for workflow completion (timeout: %d minutes)", timeoutMinutes)))
125-
}
126-
127-
// Use the repository slug directly
128-
fullRepoName := repoSlug
129-
130123
timeout := time.Duration(timeoutMinutes) * time.Minute
131-
start := time.Now()
132124

133-
for {
134-
// Check if timeout exceeded
135-
if time.Since(start) > timeout {
136-
return fmt.Errorf("workflow execution timed out after %d minutes", timeoutMinutes)
137-
}
138-
139-
// Check workflow status
140-
cmd := exec.Command("gh", "run", "view", runID, "--repo", fullRepoName, "--json", "status,conclusion")
141-
output, err := cmd.Output()
125+
return PollWithSignalHandling(PollOptions{
126+
PollInterval: 10 * time.Second,
127+
Timeout: timeout,
128+
PollFunc: func() (PollResult, error) {
129+
// Check workflow status
130+
cmd := exec.Command("gh", "run", "view", runID, "--repo", repoSlug, "--json", "status,conclusion")
131+
output, err := cmd.Output()
142132

143-
if err != nil {
144-
return fmt.Errorf("failed to check workflow status: %w", err)
145-
}
146-
147-
status := string(output)
133+
if err != nil {
134+
return PollFailure, fmt.Errorf("failed to check workflow status: %w", err)
135+
}
148136

149-
// Check if completed
150-
if strings.Contains(status, `"status":"completed"`) {
151-
if strings.Contains(status, `"conclusion":"success"`) {
152-
if verbose {
153-
fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Workflow completed successfully"))
137+
status := string(output)
138+
139+
// Check if completed
140+
if strings.Contains(status, `"status":"completed"`) {
141+
if strings.Contains(status, `"conclusion":"success"`) {
142+
return PollSuccess, nil
143+
} else if strings.Contains(status, `"conclusion":"failure"`) {
144+
return PollFailure, fmt.Errorf("workflow failed")
145+
} else if strings.Contains(status, `"conclusion":"cancelled"`) {
146+
return PollFailure, fmt.Errorf("workflow was cancelled")
147+
} else {
148+
return PollFailure, fmt.Errorf("workflow completed with unknown conclusion")
154149
}
155-
return nil
156-
} else if strings.Contains(status, `"conclusion":"failure"`) {
157-
return fmt.Errorf("workflow failed")
158-
} else if strings.Contains(status, `"conclusion":"cancelled"`) {
159-
return fmt.Errorf("workflow was cancelled")
160-
} else {
161-
return fmt.Errorf("workflow completed with unknown conclusion")
162150
}
163-
}
164151

165-
// Still running, wait before checking again
166-
if verbose {
167-
fmt.Fprintln(os.Stderr, console.FormatProgressMessage("Workflow still running..."))
168-
}
169-
time.Sleep(10 * time.Second)
170-
}
152+
// Still running, continue polling
153+
return PollContinue, nil
154+
},
155+
StartMessage: fmt.Sprintf("Waiting for workflow completion (timeout: %d minutes)", timeoutMinutes),
156+
ProgressMessage: "Workflow still running...",
157+
SuccessMessage: "Workflow completed successfully",
158+
Verbose: verbose,
159+
})
171160
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
package cli
2+
3+
import (
4+
"testing"
5+
)
6+
7+
// TestWaitForWorkflowCompletionUsesSignalHandling verifies that WaitForWorkflowCompletion
8+
// uses the signal-aware polling helper, which provides Ctrl-C support
9+
func TestWaitForWorkflowCompletionUsesSignalHandling(t *testing.T) {
10+
// This test verifies that the function uses PollWithSignalHandling
11+
// by checking that it times out correctly (a key feature of the helper)
12+
13+
// We can't easily test the actual workflow checking without a real workflow,
14+
// but we can verify that the timeout mechanism works, which confirms
15+
// it's using the polling helper
16+
17+
err := WaitForWorkflowCompletion("nonexistent/repo", "12345", 0, false)
18+
19+
// Should timeout or fail to check workflow status
20+
if err == nil {
21+
t.Error("Expected error for nonexistent workflow, got nil")
22+
}
23+
}
Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
package cli
2+
3+
import (
4+
"fmt"
5+
"os"
6+
"os/signal"
7+
"syscall"
8+
"time"
9+
10+
"github.com/githubnext/gh-aw/pkg/console"
11+
"github.com/githubnext/gh-aw/pkg/logger"
12+
)
13+
14+
var pollLog = logger.New("cli:signal_aware_poll")
15+
16+
// PollResult represents the result of a polling operation
17+
type PollResult int
18+
19+
const (
20+
// PollContinue indicates polling should continue
21+
PollContinue PollResult = iota
22+
// PollSuccess indicates polling completed successfully
23+
PollSuccess
24+
// PollFailure indicates polling failed
25+
PollFailure
26+
)
27+
28+
// PollOptions contains configuration for signal-aware polling
29+
type PollOptions struct {
30+
// Interval between poll attempts
31+
PollInterval time.Duration
32+
// Timeout for the entire polling operation
33+
Timeout time.Duration
34+
// Function to call on each poll iteration
35+
// Should return PollContinue to keep polling, PollSuccess to succeed, or PollFailure to fail
36+
PollFunc func() (PollResult, error)
37+
// Message to display when polling starts (optional)
38+
StartMessage string
39+
// Message to display on each poll iteration (optional)
40+
ProgressMessage string
41+
// Message to display on successful completion (optional)
42+
SuccessMessage string
43+
// Whether to show verbose progress messages
44+
Verbose bool
45+
}
46+
47+
// PollWithSignalHandling polls with a function until it succeeds, fails, times out, or receives an interrupt signal
48+
// This provides a reusable pattern for any operation that needs to poll with graceful Ctrl-C handling
49+
func PollWithSignalHandling(options PollOptions) error {
50+
pollLog.Printf("Starting polling: interval=%v, timeout=%v", options.PollInterval, options.Timeout)
51+
52+
if options.Verbose && options.StartMessage != "" {
53+
fmt.Fprintln(os.Stderr, console.FormatInfoMessage(options.StartMessage))
54+
}
55+
56+
// Set up signal handling for graceful shutdown
57+
sigChan := make(chan os.Signal, 1)
58+
signal.Notify(sigChan, os.Interrupt, syscall.SIGTERM)
59+
defer signal.Stop(sigChan)
60+
61+
// Set up timeout
62+
start := time.Now()
63+
ticker := time.NewTicker(options.PollInterval)
64+
defer ticker.Stop()
65+
66+
// Perform initial check immediately
67+
result, err := options.PollFunc()
68+
if result == PollSuccess {
69+
if options.Verbose && options.SuccessMessage != "" {
70+
fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(options.SuccessMessage))
71+
}
72+
return nil
73+
} else if result == PollFailure {
74+
return err
75+
}
76+
77+
// Continue polling
78+
for {
79+
select {
80+
case <-sigChan:
81+
pollLog.Print("Received interrupt signal")
82+
fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Received interrupt signal, stopping wait..."))
83+
return fmt.Errorf("interrupted by user")
84+
85+
case <-ticker.C:
86+
// Check if timeout exceeded
87+
if options.Timeout > 0 && time.Since(start) > options.Timeout {
88+
pollLog.Printf("Timeout exceeded: %v", options.Timeout)
89+
return fmt.Errorf("operation timed out after %v", options.Timeout)
90+
}
91+
92+
// Poll for status
93+
result, err := options.PollFunc()
94+
95+
if result == PollSuccess {
96+
if options.Verbose && options.SuccessMessage != "" {
97+
fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(options.SuccessMessage))
98+
}
99+
return nil
100+
} else if result == PollFailure {
101+
return err
102+
}
103+
104+
// Still waiting, show progress if enabled
105+
if options.Verbose && options.ProgressMessage != "" {
106+
fmt.Fprintln(os.Stderr, console.FormatProgressMessage(options.ProgressMessage))
107+
}
108+
}
109+
}
110+
}

0 commit comments

Comments
 (0)