Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
61 changes: 35 additions & 26 deletions pkg/cli/run_push.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ import (
var runPushLog = logger.New("cli:run_push")

// collectWorkflowFiles collects the workflow .md file, its corresponding .lock.yml file,
// and the transitive closure of all imported files
// and the transitive closure of all imported files.
// Note: This function always recompiles the workflow to ensure the lock file is up-to-date,
// regardless of the frontmatter hash status.
func collectWorkflowFiles(workflowPath string, verbose bool) ([]string, error) {
runPushLog.Printf("Collecting files for workflow: %s", workflowPath)

Expand All @@ -38,53 +40,53 @@ func collectWorkflowFiles(workflowPath string, verbose bool) ([]string, error) {
files[absWorkflowPath] = true
runPushLog.Printf("Added workflow file: %s", absWorkflowPath)

// Check if lock file needs recompilation
// Check lock file and log hash status for observability
lockFilePath := stringutil.MarkdownToLockFile(absWorkflowPath)
runPushLog.Printf("Checking lock file: %s", lockFilePath)
needsRecompile := false

// Always recompile, but check and log hash status for observability
if _, err := os.Stat(lockFilePath); err == nil {
runPushLog.Printf("Lock file exists: %s", lockFilePath)
// Lock file exists - check frontmatter hash
runPushLog.Print("Checking frontmatter hash")
// Check frontmatter hash for observability
runPushLog.Print("Checking frontmatter hash for observability")
if hashMismatch, err := checkFrontmatterHashMismatch(absWorkflowPath, lockFilePath); err != nil {
runPushLog.Printf("Error checking frontmatter hash: %v", err)
// Don't fail, just log the error
} else if hashMismatch {
needsRecompile = true
runPushLog.Print("Lock file is outdated: frontmatter hash mismatch")
runPushLog.Print("Lock file frontmatter hash changed (will recompile)")
if verbose {
fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Detected outdated lock file (frontmatter changed), recompiling workflow..."))
fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Frontmatter hash changed, recompiling workflow..."))
}
} else {
runPushLog.Printf("Lock file is up-to-date (frontmatter hash matches)")
runPushLog.Print("Lock file frontmatter hash unchanged (will still recompile)")
if verbose {
fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Recompiling workflow..."))
}
}
} else if os.IsNotExist(err) {
// Lock file doesn't exist - needs compilation
needsRecompile = true
// Lock file doesn't exist
runPushLog.Printf("Lock file not found: %s", lockFilePath)
if verbose {
fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Lock file not found, compiling workflow..."))
}
} else {
runPushLog.Printf("Error checking lock file: %v", err)
}

// Recompile if needed
if needsRecompile {
runPushLog.Printf("Recompilation needed for %s", absWorkflowPath)
if err := recompileWorkflow(absWorkflowPath, verbose); err != nil {
runPushLog.Printf("Failed to recompile workflow: %v", err)
return nil, fmt.Errorf("failed to recompile workflow: %w", err)
}
if verbose {
fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Workflow compiled successfully"))
fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Compiling workflow..."))
}
runPushLog.Printf("Recompilation completed successfully")
} else {
runPushLog.Printf("Recompilation not needed")
}

// Always recompile (hash check is for observability only)
runPushLog.Printf("Always recompiling workflow: %s", absWorkflowPath)
if err := recompileWorkflow(absWorkflowPath, verbose); err != nil {
runPushLog.Printf("Failed to recompile workflow: %v", err)
return nil, fmt.Errorf("failed to recompile workflow: %w", err)
}
if verbose {
fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Workflow compiled successfully"))
}
runPushLog.Printf("Recompilation completed successfully")

// Add the corresponding .lock.yml file
if _, err := os.Stat(lockFilePath); err == nil {
files[lockFilePath] = true
Expand Down Expand Up @@ -149,14 +151,19 @@ func recompileWorkflow(workflowPath string, verbose bool) error {
return nil
}

// checkLockFileStatus checks if a lock file is missing or outdated and returns status info
// checkLockFileStatus checks if a lock file is missing or outdated and returns status info.
// Note: This is used for display/warning purposes only. The actual compilation decision
// is made in collectWorkflowFiles, which always recompiles regardless of hash status.
type LockFileStatus struct {
Missing bool
Outdated bool
LockPath string
}

// checkLockFileStatus checks the status of a workflow's lock file
// checkLockFileStatus checks the status of a workflow's lock file.
// This function is used to determine whether to show warnings to the user about
// outdated lock files. It does NOT control whether recompilation happens -
// collectWorkflowFiles always recompiles regardless of the hash status.
func checkLockFileStatus(workflowPath string) (*LockFileStatus, error) {
runPushLog.Printf("Checking lock file status for: %s", workflowPath)

Expand Down Expand Up @@ -589,6 +596,8 @@ func pushWorkflowFiles(workflowName string, files []string, refOverride string,
// checkFrontmatterHashMismatch checks if the frontmatter hash in the lock file
// matches the recomputed hash from the workflow file.
// Returns true if there's a mismatch (lock file is stale), false if they match.
// Note: This is used for logging/observability only. The compilation decision
// is made by collectWorkflowFiles, which always recompiles regardless of hash status.
func checkFrontmatterHashMismatch(workflowPath, lockFilePath string) (bool, error) {
runPushLog.Printf("Checking frontmatter hash for %s", workflowPath)

Expand Down
62 changes: 62 additions & 0 deletions pkg/cli/run_push_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -434,3 +434,65 @@ func TestPushWorkflowFiles_WithStagedFiles(t *testing.T) {
require.Error(t, err)
assert.Contains(t, err.Error(), "staged files")
}

func TestCollectWorkflowFiles_AlwaysRecompiles(t *testing.T) {
// Test that workflows are always recompiled, even when frontmatter hash matches
tmpDir := t.TempDir()

// Create a workflow file
workflowPath := filepath.Join(tmpDir, "test-workflow.md")
workflowContent := `---
name: Test Workflow
engine: copilot
on: workflow_dispatch
---
# Test Workflow
This is a test workflow.
`
err := os.WriteFile(workflowPath, []byte(workflowContent), 0644)
require.NoError(t, err)

// Compute the correct hash for this workflow
cache := parser.NewImportCache("")
correctHash, err := parser.ComputeFrontmatterHashFromFile(workflowPath, cache)
require.NoError(t, err)
require.NotEmpty(t, correctHash)

// Create a lock file with the CORRECT hash (matching)
lockFilePath := filepath.Join(tmpDir, "test-workflow.lock.yml")
lockContent := fmt.Sprintf(`# frontmatter-hash: %s

name: Test Workflow
"on": workflow_dispatch
jobs:
agent:
runs-on: ubuntu-latest
steps:
- name: Test
run: echo "test"
`, correctHash)
err = os.WriteFile(lockFilePath, []byte(lockContent), 0644)
require.NoError(t, err)

// Record the modification time of the lock file before collection
lockStatBefore, err := os.Stat(lockFilePath)
require.NoError(t, err)
timeBefore := lockStatBefore.ModTime()

// Sleep to ensure modification time would change if file is rewritten
time.Sleep(100 * time.Millisecond)

// Collect workflow files (which should always trigger recompilation)
files, err := collectWorkflowFiles(workflowPath, false)
require.NoError(t, err)
assert.Len(t, files, 2, "Should collect workflow .md and .lock.yml files")

// Verify the lock file was recompiled (modification time should be newer)
lockStatAfter, err := os.Stat(lockFilePath)
require.NoError(t, err)
timeAfter := lockStatAfter.ModTime()

// The lock file should have been recompiled even though the hash matched
assert.True(t, timeAfter.After(timeBefore),
"Lock file should be recompiled even when frontmatter hash matches (always recompile behavior)")
}
Loading