diff --git a/pkg/cli/run_push.go b/pkg/cli/run_push.go index 0247d62c77..41a08830f6 100644 --- a/pkg/cli/run_push.go +++ b/pkg/cli/run_push.go @@ -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) @@ -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 @@ -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) @@ -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) diff --git a/pkg/cli/run_push_test.go b/pkg/cli/run_push_test.go index e5db34a9df..37334293af 100644 --- a/pkg/cli/run_push_test.go +++ b/pkg/cli/run_push_test.go @@ -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)") +}