Skip to content

Commit 71e5d0d

Browse files
authored
Fix stop-after time preservation during workflow recompilation (#3950)
1 parent 23a6333 commit 71e5d0d

6 files changed

Lines changed: 165 additions & 35 deletions

File tree

cmd/gh-aw/main.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@ Examples:
193193
logicalRepo, _ := cmd.Flags().GetString("logical-repo")
194194
dependabot, _ := cmd.Flags().GetBool("dependabot")
195195
forceOverwrite, _ := cmd.Flags().GetBool("force")
196+
refreshStopTime, _ := cmd.Flags().GetBool("refresh-stop-time")
196197
zizmor, _ := cmd.Flags().GetBool("zizmor")
197198
poutine, _ := cmd.Flags().GetBool("poutine")
198199
actionlint, _ := cmd.Flags().GetBool("actionlint")
@@ -227,6 +228,7 @@ Examples:
227228
Strict: strict,
228229
Dependabot: dependabot,
229230
ForceOverwrite: forceOverwrite,
231+
RefreshStopTime: refreshStopTime,
230232
Zizmor: zizmor,
231233
Poutine: poutine,
232234
Actionlint: actionlint,
@@ -419,6 +421,7 @@ Use "` + constants.CLIExtensionPrefix + ` help all" to show help for all command
419421
compileCmd.Flags().String("logical-repo", "", "Repository to simulate workflow execution against (for trial mode)")
420422
compileCmd.Flags().Bool("dependabot", false, "Generate dependency manifests (package.json, requirements.txt, go.mod) and Dependabot config when dependencies are detected")
421423
compileCmd.Flags().Bool("force", false, "Force overwrite of existing files (e.g., dependabot.yml)")
424+
compileCmd.Flags().Bool("refresh-stop-time", false, "Force regeneration of stop-after times instead of preserving existing values from lock files")
422425
compileCmd.Flags().Bool("zizmor", false, "Run zizmor security scanner on generated .lock.yml files")
423426
compileCmd.Flags().Bool("poutine", false, "Run poutine security scanner on generated .lock.yml files")
424427
compileCmd.Flags().Bool("actionlint", false, "Run actionlint linter on generated .lock.yml files")

docs/src/content/docs/status.mdx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ Status of all agentic workflows. [Browse source files](https://github.com/github
1616
| [Blog Auditor](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/blog-auditor.md) | claude | [![Blog Auditor](https://github.com/githubnext/gh-aw/actions/workflows/blog-auditor.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/blog-auditor.lock.yml) | `0 12 * * 3` | - |
1717
| [Brave Web Search Agent](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/brave.md) | copilot | [![Brave Web Search Agent](https://github.com/githubnext/gh-aw/actions/workflows/brave.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/brave.lock.yml) | - | `/brave` |
1818
| [Changeset Generator](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/changeset.md) | copilot | [![Changeset Generator](https://github.com/githubnext/gh-aw/actions/workflows/changeset.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/changeset.lock.yml) | `0 */2 * * *` | - |
19+
| [CI Failure Doctor](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/ci-doctor.md) | copilot | [![CI Failure Doctor](https://github.com/githubnext/gh-aw/actions/workflows/ci-doctor.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/ci-doctor.lock.yml) | - | - |
1920
| [CLI Consistency Checker](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/cli-consistency-checker.md) | copilot | [![CLI Consistency Checker](https://github.com/githubnext/gh-aw/actions/workflows/cli-consistency-checker.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/cli-consistency-checker.lock.yml) | `0 13 * * 1-5` | - |
2021
| [CLI Version Checker](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/cli-version-checker.md) | copilot | [![CLI Version Checker](https://github.com/githubnext/gh-aw/actions/workflows/cli-version-checker.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/cli-version-checker.lock.yml) | `0 15 * * *` | - |
2122
| [Commit Changes Analyzer](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/commit-changes-analyzer.md) | claude | [![Commit Changes Analyzer](https://github.com/githubnext/gh-aw/actions/workflows/commit-changes-analyzer.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/commit-changes-analyzer.lock.yml) | - | - |
@@ -28,6 +29,7 @@ Status of all agentic workflows. [Browse source files](https://github.com/github
2829
| [Daily Documentation Updater](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/daily-doc-updater.md) | claude | [![Daily Documentation Updater](https://github.com/githubnext/gh-aw/actions/workflows/daily-doc-updater.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/daily-doc-updater.lock.yml) | `0 6 * * *` | - |
2930
| [Daily Firewall Logs Collector and Reporter](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/daily-firewall-report.md) | copilot | [![Daily Firewall Logs Collector and Reporter](https://github.com/githubnext/gh-aw/actions/workflows/daily-firewall-report.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/daily-firewall-report.lock.yml) | `0 10 * * *` | - |
3031
| [Daily News](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/daily-news.md) | copilot | [![Daily News](https://github.com/githubnext/gh-aw/actions/workflows/daily-news.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/daily-news.lock.yml) | `0 9 * * 1-5` | - |
32+
| [Daily Team Status](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/daily-team-status.md) | copilot | [![Daily Team Status](https://github.com/githubnext/gh-aw/actions/workflows/daily-team-status.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/daily-team-status.lock.yml) | `0 9 * * 1-5` | - |
3133
| [Dependabot Go Module Dependency Checker](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/dependabot-go-checker.md) | copilot | [![Dependabot Go Module Dependency Checker](https://github.com/githubnext/gh-aw/actions/workflows/dependabot-go-checker.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/dependabot-go-checker.lock.yml) | `0 9 * * 1,3,5` | - |
3234
| [Dev](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/dev.md) | copilot | [![Dev](https://github.com/githubnext/gh-aw/actions/workflows/dev.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/dev.lock.yml) | - | - |
3335
| [Dev Firewall](https://github.com/githubnext/gh-aw/blob/main/.github/workflows/dev.firewall.md) | copilot | [![Dev Firewall](https://github.com/githubnext/gh-aw/actions/workflows/dev.firewall.lock.yml/badge.svg)](https://github.com/githubnext/gh-aw/actions/workflows/dev.firewall.lock.yml) | - | - |

pkg/cli/compile_command.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@ type CompileConfig struct {
165165
Poutine bool // Run poutine security scanner on generated .lock.yml files
166166
Actionlint bool // Run actionlint linter on generated .lock.yml files
167167
JSONOutput bool // Output validation results as JSON
168+
RefreshStopTime bool // Force regeneration of stop-after times instead of preserving existing ones
168169
}
169170

170171
// CompilationStats tracks the results of workflow compilation
@@ -285,6 +286,12 @@ func CompileWorkflows(config CompileConfig) ([]*workflow.WorkflowData, error) {
285286
}
286287
}
287288

289+
// Set refresh stop time flag
290+
compiler.SetRefreshStopTime(config.RefreshStopTime)
291+
if config.RefreshStopTime {
292+
compileLog.Print("Stop time refresh enabled: will regenerate stop-after times")
293+
}
294+
288295
if watch {
289296
// Watch mode: watch for file changes and recompile automatically
290297
// For watch mode, we only support a single file for now

pkg/workflow/compiler.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ type Compiler struct {
5656
strictMode bool // If true, enforce strict validation requirements
5757
trialMode bool // If true, suppress safe outputs for trial mode execution
5858
trialLogicalRepoSlug string // If set in trial mode, the logical repository to checkout
59+
refreshStopTime bool // If true, regenerate stop-after times instead of preserving existing ones
5960
jobManager *JobManager // Manages jobs and dependencies
6061
engineRegistry *EngineRegistry // Registry of available agentic engines
6162
fileTracker FileTracker // Optional file tracker for tracking created files
@@ -110,6 +111,11 @@ func (c *Compiler) SetStrictMode(strict bool) {
110111
c.strictMode = strict
111112
}
112113

114+
// Configures whether to force regeneration of stop-after times
115+
func (c *Compiler) SetRefreshStopTime(refresh bool) {
116+
c.refreshStopTime = refresh
117+
}
118+
113119
// IncrementWarningCount increments the warning counter
114120
func (c *Compiler) IncrementWarningCount() {
115121
c.warningCount++

pkg/workflow/stop_after.go

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,22 @@ func (c *Compiler) processStopAfterConfiguration(frontmatter map[string]any, wor
5050
lockFile := strings.TrimSuffix(markdownPath, ".md") + ".lock.yml"
5151
existingStopTime := ExtractStopTimeFromLockFile(lockFile)
5252

53-
if existingStopTime != "" {
54-
// Preserve existing stop time during recompilation
53+
// If refresh flag is set, always regenerate the stop time
54+
if c.refreshStopTime {
55+
resolvedStopTime, err := resolveStopTime(workflowData.StopTime, time.Now().UTC())
56+
if err != nil {
57+
return fmt.Errorf("invalid stop-after format: %w", err)
58+
}
59+
originalStopTime := stopAfter
60+
workflowData.StopTime = resolvedStopTime
61+
62+
if c.verbose && isRelativeStopTime(originalStopTime) {
63+
fmt.Println(console.FormatInfoMessage(fmt.Sprintf("Refreshed relative stop-after to: %s", resolvedStopTime)))
64+
} else if c.verbose && originalStopTime != resolvedStopTime {
65+
fmt.Println(console.FormatInfoMessage(fmt.Sprintf("Refreshed absolute stop-after from '%s' to: %s", originalStopTime, resolvedStopTime)))
66+
}
67+
} else if existingStopTime != "" {
68+
// Preserve existing stop time during recompilation (default behavior)
5569
workflowData.StopTime = existingStopTime
5670
if c.verbose {
5771
fmt.Println(console.FormatInfoMessage(fmt.Sprintf("Preserving existing stop time from lock file: %s", existingStopTime)))
@@ -112,16 +126,14 @@ func ExtractStopTimeFromLockFile(lockFilePath string) string {
112126
return ""
113127
}
114128

115-
// Look for the STOP_TIME line in the safety checks section
116-
// Pattern: STOP_TIME="YYYY-MM-DD HH:MM:SS"
117129
lines := strings.Split(string(content), "\n")
118130
for _, line := range lines {
119-
if strings.Contains(line, "STOP_TIME=") {
120-
// Extract the value between quotes
121-
start := strings.Index(line, `"`) + 1
122-
end := strings.LastIndex(line, `"`)
123-
if start > 0 && end > start {
124-
return line[start:end]
131+
// Look for GH_AW_STOP_TIME: YYYY-MM-DD HH:MM:SS
132+
// This is in the env section of the stop time check job
133+
if strings.Contains(line, "GH_AW_STOP_TIME:") {
134+
prefix := "GH_AW_STOP_TIME:"
135+
if idx := strings.Index(line, prefix); idx != -1 {
136+
return strings.TrimSpace(line[idx+len(prefix):])
125137
}
126138
}
127139
}

pkg/workflow/stop_after_test.go

Lines changed: 125 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package workflow
22

33
import (
4+
"fmt"
45
"os"
56
"path/filepath"
67
"strings"
@@ -16,18 +17,18 @@ func TestExtractStopTimeFromLockFile(t *testing.T) {
1617
expectedTime string
1718
}{
1819
{
19-
name: "valid stop-time in lock file",
20+
name: "valid stop-time in GH_AW_STOP_TIME format",
2021
lockContent: `name: Test Workflow
2122
on:
2223
workflow_dispatch:
2324
jobs:
24-
safety_checks:
25+
stop_time_check:
2526
runs-on: ubuntu-latest
2627
steps:
27-
- name: Safety checks
28-
run: |
29-
STOP_TIME="2025-12-31 23:59:59"
30-
echo "Checking stop-time limit: $STOP_TIME"`,
28+
- uses: actions/github-script@v8
29+
env:
30+
GH_AW_STOP_TIME: 2025-12-31 23:59:59
31+
GH_AW_WORKFLOW_NAME: "Test Workflow"`,
3132
expectedTime: "2025-12-31 23:59:59",
3233
},
3334
{
@@ -44,32 +45,20 @@ jobs:
4445
expectedTime: "",
4546
},
4647
{
47-
name: "malformed stop-time line",
48+
name: "GH_AW_STOP_TIME with extra whitespace",
4849
lockContent: `name: Test Workflow
4950
on:
5051
workflow_dispatch:
5152
jobs:
52-
safety_checks:
53+
stop_time_check:
5354
runs-on: ubuntu-latest
5455
steps:
55-
- name: Safety checks
56-
run: |
57-
STOP_TIME=malformed-no-quotes`,
58-
expectedTime: "",
56+
- uses: actions/github-script@v8
57+
env:
58+
GH_AW_STOP_TIME: 2025-06-01 12:00:00
59+
GH_AW_WORKFLOW_NAME: "Test Workflow"`,
60+
expectedTime: "2025-06-01 12:00:00",
5961
},
60-
{
61-
name: "multiple stop-time lines (should get first)",
62-
lockContent: `name: Test Workflow
63-
on:
64-
workflow_dispatch:
65-
jobs:
66-
safety_checks:
67-
runs-on: ubuntu-latest
68-
steps:
69-
- name: Safety checks
70-
run: |
71-
STOP_TIME="2025-06-01 12:00:00"
72-
echo "Checking stop-time limit: $STOP_TIME"
7362
STOP_TIME="2025-07-01 12:00:00"`,
7463
expectedTime: "2025-06-01 12:00:00",
7564
},
@@ -158,3 +147,114 @@ func TestResolveStopTimeRejectsMinutes(t *testing.T) {
158147
})
159148
}
160149
}
150+
151+
// TestRefreshStopTimeBehavior tests that the refreshStopTime flag controls stop time preservation
152+
func TestRefreshStopTimeBehavior(t *testing.T) {
153+
// Create a temporary directory for test files
154+
tmpDir, err := os.MkdirTemp("", "refresh-stop-time-test")
155+
if err != nil {
156+
t.Fatalf("Failed to create temp dir: %v", err)
157+
}
158+
defer os.RemoveAll(tmpDir)
159+
160+
// Create a markdown workflow file with stop-after
161+
mdFile := filepath.Join(tmpDir, "test.md")
162+
lockFile := filepath.Join(tmpDir, "test.lock.yml")
163+
164+
// Create a lock file with existing stop time
165+
existingStopTime := "2025-12-31 23:59:59"
166+
lockContent := fmt.Sprintf(`name: Test Workflow
167+
on:
168+
workflow_dispatch:
169+
jobs:
170+
stop_time_check:
171+
runs-on: ubuntu-latest
172+
steps:
173+
- uses: actions/github-script@v8
174+
env:
175+
GH_AW_STOP_TIME: %s
176+
GH_AW_WORKFLOW_NAME: "Test Workflow"
177+
`, existingStopTime)
178+
err = os.WriteFile(lockFile, []byte(lockContent), 0644)
179+
if err != nil {
180+
t.Fatalf("Failed to create lock file: %v", err)
181+
}
182+
183+
// Test 1: Default behavior should preserve existing stop time
184+
t.Run("default behavior preserves stop time", func(t *testing.T) {
185+
compiler := NewCompiler(false, "", "test")
186+
compiler.SetRefreshStopTime(false)
187+
188+
frontmatter := map[string]any{
189+
"on": map[string]any{
190+
"workflow_dispatch": nil,
191+
"stop-after": "+48h",
192+
},
193+
}
194+
195+
workflowData := &WorkflowData{}
196+
err = compiler.processStopAfterConfiguration(frontmatter, workflowData, mdFile)
197+
if err != nil {
198+
t.Fatalf("processStopAfterConfiguration failed: %v", err)
199+
}
200+
201+
if workflowData.StopTime != existingStopTime {
202+
t.Errorf("Expected stop time to be preserved as %q, got %q", existingStopTime, workflowData.StopTime)
203+
}
204+
})
205+
206+
// Test 2: With refresh flag, should generate new stop time
207+
t.Run("refresh flag generates new stop time", func(t *testing.T) {
208+
compiler := NewCompiler(false, "", "test")
209+
compiler.SetRefreshStopTime(true)
210+
211+
frontmatter := map[string]any{
212+
"on": map[string]any{
213+
"workflow_dispatch": nil,
214+
"stop-after": "+48h",
215+
},
216+
}
217+
218+
workflowData := &WorkflowData{}
219+
err = compiler.processStopAfterConfiguration(frontmatter, workflowData, mdFile)
220+
if err != nil {
221+
t.Fatalf("processStopAfterConfiguration failed: %v", err)
222+
}
223+
224+
if workflowData.StopTime == existingStopTime {
225+
t.Errorf("Expected stop time to be refreshed, but got the same value: %q", workflowData.StopTime)
226+
}
227+
228+
// Verify the new stop time is a valid timestamp
229+
if workflowData.StopTime == "" {
230+
t.Error("Expected stop time to be set, got empty string")
231+
}
232+
})
233+
234+
// Test 3: First compilation without existing lock file should generate new stop time
235+
t.Run("first compilation generates new stop time", func(t *testing.T) {
236+
// Remove the lock file for this test
237+
os.Remove(lockFile)
238+
239+
compiler := NewCompiler(false, "", "test")
240+
compiler.SetRefreshStopTime(false)
241+
242+
frontmatter := map[string]any{
243+
"on": map[string]any{
244+
"workflow_dispatch": nil,
245+
"stop-after": "+48h",
246+
},
247+
}
248+
249+
workflowData := &WorkflowData{}
250+
err = compiler.processStopAfterConfiguration(frontmatter, workflowData, mdFile)
251+
if err != nil {
252+
t.Fatalf("processStopAfterConfiguration failed: %v", err)
253+
}
254+
255+
// Verify a new stop time was generated
256+
if workflowData.StopTime == "" {
257+
t.Error("Expected stop time to be set, got empty string")
258+
}
259+
})
260+
}

0 commit comments

Comments
 (0)