Skip to content

Fix stop-after time preservation during workflow recompilation#3950

Merged
pelikhan merged 8 commits into
mainfrom
copilot/investigate-compile-stop-time
Nov 14, 2025
Merged

Fix stop-after time preservation during workflow recompilation#3950
pelikhan merged 8 commits into
mainfrom
copilot/investigate-compile-stop-time

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Nov 14, 2025

Fix: Stop Time Preservation in Workflow Recompilation ✅

Problem Statement

When running make recompile, the ci-doctor workflow's stop time in the lock file was being updated unnecessarily, causing unwanted changes in version control.

Root Cause

The ExtractStopTimeFromLockFile function was looking for an old pattern STOP_TIME="YYYY-MM-DD HH:MM:SS" (used in job-based stop time checks), but the actual lock files now use multiple patterns including a comment format and an environment variable format. When extraction failed, the system regenerated the timestamp with time.Now().UTC() on every compilation.

Solution Implemented

1. Fixed Stop Time Extraction

File: pkg/workflow/stop_after.go

  • ✅ Added support for GH_AW_STOP_TIME: pattern (most stable - in YAML env section)
  • ✅ Updated ExtractStopTimeFromLockFile() to use prefix-based extraction
  • ✅ Fixed bug where splitting by ':' broke on timestamps with colons (HH:MM:SS)
  • ✅ Maintained support for comment pattern # Effective stop-time:
  • ✅ Maintained backward compatibility with old STOP_TIME= pattern
  • ✅ Updated processStopAfterConfiguration() to respect new refreshStopTime flag

Pattern Priority:

  1. GH_AW_STOP_TIME: - Most stable (part of YAML structure, used by runtime check)
  2. # Effective stop-time: - Comment format at top of file
  3. STOP_TIME= - Legacy format in job steps

2. Added CLI Flag for Controlled Refresh

Files: cmd/gh-aw/main.go, pkg/cli/compile_command.go, pkg/workflow/compiler.go

  • ✅ Added --refresh-stop-time flag to force regeneration when needed
  • ✅ Default behavior preserves existing stop times
  • ✅ Flag allows intentional refresh when desired

3. Comprehensive Testing

File: pkg/workflow/stop_after_test.go

  • ✅ Added tests for GH_AW_STOP_TIME: format
  • ✅ Added tests for comment format extraction
  • ✅ Added tests for backward compatibility with old format
  • ✅ Added tests for flag behavior (preserve vs refresh)
  • ✅ Added tests for multiple patterns in same file
  • ✅ All tests pass

4. Merged Main Branch

  • ✅ Resolved merge conflicts
  • ✅ Incorporated latest changes from main

Verification Results

Stop Time Extraction with Multiple Patterns

$ grep "Effective stop-time" .github/workflows/ci-doctor.lock.yml
# Effective stop-time: 2025-11-16 13:10:44

$ grep "GH_AW_STOP_TIME:" .github/workflows/ci-doctor.lock.yml
          GH_AW_STOP_TIME: 2025-11-16 13:10:44

$ ./gh-aw compile ci-doctor --verbose
ℹ Preserving existing stop time from lock file: 2025-11-16 13:10:44

$ grep "Effective stop-time" .github/workflows/ci-doctor.lock.yml
# Effective stop-time: 2025-11-16 13:10:44  # <-- UNCHANGED ✓

With Refresh Flag

$ ./gh-aw compile ci-doctor --refresh-stop-time --verbose
ℹ Refreshed relative stop-after to: 2025-11-16 13:21:22

$ grep "Effective stop-time" .github/workflows/ci-doctor.lock.yml
# Effective stop-time: 2025-11-16 13:21:22  # <-- Updated as intended

Test Results

  • ✅ All unit tests pass (make test-unit)
  • ✅ ExtractStopTimeFromLockFile tests pass (including new GH_AW_STOP_TIME test)
  • ✅ RefreshStopTimeBehavior tests pass
  • ✅ Linter passes (make lint)
  • ✅ Formatter passes (make fmt)
  • ✅ No changes to workflow lock files after recompile

Usage

Default Behavior (Recommended)

# Preserves existing stop times during recompilation
gh aw compile
make recompile

Force Refresh (When Needed)

# Forces regeneration of stop-after times
gh aw compile --refresh-stop-time
gh aw compile ci-doctor --refresh-stop-time

Impact

  • ✅ Solves the immediate problem: make recompile no longer updates stop times
  • ✅ Adds control: New flag allows intentional refresh when needed
  • ✅ Maintains compatibility: All three patterns supported
  • ✅ More stable: Now prioritizes GH_AW_STOP_TIME pattern (YAML structure)
  • ✅ Well tested: Comprehensive test coverage including all patterns
  • ✅ Bug fixed: Extraction now handles colons in timestamps correctly

Files Changed

  • cmd/gh-aw/main.go - Added CLI flag
  • pkg/cli/compile_command.go - Added config field
  • pkg/workflow/compiler.go - Added compiler field and setter
  • pkg/workflow/stop_after.go - Added GH_AW_STOP_TIME support, fixed extraction with prefix-based approach, added refresh logic
  • pkg/workflow/stop_after_test.go - Added comprehensive tests for all patterns
Original prompt

Investigate why "make recompile" updates the ci-doctor stop time in the lock file. Add new flag to "compile" to ensure stop time are not modified.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits November 14, 2025 13:00
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- Fix ExtractStopTimeFromLockFile to recognize comment pattern
- Add --refresh-stop-time flag to force regeneration when needed
- Update tests to cover new comment format and flag behavior
- Default behavior now preserves existing stop times

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@pelikhan pelikhan marked this pull request as ready for review November 14, 2025 13:15
Copilot AI review requested due to automatic review settings November 14, 2025 13:15
@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot merge main

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Investigate ci-doctor lock file update issue with make recompile Fix stop-after time preservation during workflow recompilation Nov 14, 2025
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes an issue where running make recompile was unnecessarily updating the stop times in lock files, causing unwanted version control changes. The root cause was that ExtractStopTimeFromLockFile was looking for an old pattern (STOP_TIME="...") instead of the current comment pattern (# Effective stop-time: ...), causing it to fail and generate new timestamps.

The implementation:

  • Updates the extraction function to recognize the current comment-based stop-time format
  • Maintains backward compatibility with the old format
  • Adds a --refresh-stop-time flag to explicitly force regeneration when needed
  • Includes comprehensive test coverage for all scenarios

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
pkg/workflow/stop_after_test.go Adds comprehensive test coverage for comment format extraction, backward compatibility, and refresh flag behavior
pkg/workflow/stop_after.go Fixes stop-time extraction to recognize comment pattern, adds refresh logic with new flag
pkg/workflow/compiler.go Adds refreshStopTime field and setter method to support the new flag
pkg/cli/compile_command.go Adds RefreshStopTime configuration field and wires it to compiler
cmd/gh-aw/main.go Adds --refresh-stop-time CLI flag definition and passes it through to compile config
pkg/cli/.github/aw/actions-lock.json Creates new empty actions lock file
.github/workflows/daily-team-status.lock.yml Updates stop time (unintended - demonstrates the issue being fixed)
.github/workflows/ci-doctor.lock.yml Updates stop time (unintended - demonstrates the issue being fixed)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/workflow/stop_after.go Outdated
Comment thread .github/workflows/ci-doctor.lock.yml Outdated
Comment thread .github/workflows/daily-team-status.lock.yml Outdated
pelikhan and others added 2 commits November 14, 2025 05:23
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot look for pattern below which is more stable:

GH_AW_STOP_TIME: yyy-mm-dd hh:mm:ss

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Nov 14, 2025

@copilot merge main

Merged main branch in commit 79c1459. Resolved conflicts by accepting main's versions of the lock files.

The GH_AW_STOP_TIME env variable pattern is more stable than comments
as it's part of the YAML structure and used by the runtime check.

Now checks for:
1. GH_AW_STOP_TIME: (most stable, in env section)
2. # Effective stop-time: (comment format)
3. STOP_TIME= (legacy format)

All patterns are supported for maximum compatibility.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Nov 14, 2025

@copilot look for pattern below which is more stable:

GH_AW_STOP_TIME: yyy-mm-dd hh:mm:ss

Added support for the GH_AW_STOP_TIME: pattern in commit ca304c5. The extraction function now checks for this pattern first as it's more stable (part of the YAML structure). All three patterns are now supported:

  1. GH_AW_STOP_TIME: (most stable, in env section)
  2. # Effective stop-time: (comment format)
  3. STOP_TIME= (legacy format)

Tests confirm the extraction works correctly with all patterns.

@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot only check for GH_AW_STOP_TIME remove other legacy formats

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants