diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index bc9a709e52..b4d5d15cdc 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -391,30 +391,22 @@ func (c *Compiler) CompileWorkflowData(workflowData *WorkflowData, markdownPath } else { log.Printf("Writing output to: %s", lockFile) - // Check if we need to force write to update timestamp - shouldForceWrite := false - if existingLockInfo, err := os.Stat(lockFile); err == nil { - if mdInfo, err := os.Stat(markdownPath); err == nil { - // If lock file is newer than source file, check if content changed - if existingLockInfo.ModTime().After(mdInfo.ModTime()) { - // Read existing content to compare - if existingContent, err := os.ReadFile(lockFile); err == nil { - if string(existingContent) == yamlContent { - // Content hasn't changed but timestamp is wrong - force write - shouldForceWrite = true - log.Printf("Lock file timestamp is newer than source, but content unchanged - forcing write to update timestamp") - } - } - } + // Check if content has actually changed + contentUnchanged := false + if existingContent, err := os.ReadFile(lockFile); err == nil { + if string(existingContent) == yamlContent { + // Content is identical - skip write to preserve timestamp + contentUnchanged = true + log.Print("Lock file content unchanged - skipping write to preserve timestamp") } } - if err := os.WriteFile(lockFile, []byte(yamlContent), 0644); err != nil { - return formatCompilerError(lockFile, "error", fmt.Sprintf("failed to write lock file: %v", err)) - } - - if shouldForceWrite { - log.Print("Updated lock file timestamp to match content generation") + // Only write if content has changed + if !contentUnchanged { + if err := os.WriteFile(lockFile, []byte(yamlContent), 0644); err != nil { + return formatCompilerError(lockFile, "error", fmt.Sprintf("failed to write lock file: %v", err)) + } + log.Print("Lock file written successfully") } // Validate file size after writing diff --git a/pkg/workflow/compiler_skip_write_test.go b/pkg/workflow/compiler_skip_write_test.go new file mode 100644 index 0000000000..f1437cda2d --- /dev/null +++ b/pkg/workflow/compiler_skip_write_test.go @@ -0,0 +1,187 @@ +//go:build !integration + +package workflow + +import ( + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// timestampDifferentiationDelay is the delay used in tests to ensure +// filesystem timestamps would be different if a file is written +const timestampDifferentiationDelay = 100 * time.Millisecond + +// TestCompilerSkipsWriteWhenContentUnchanged verifies that the compiler skips writing +// the lock file when the content hasn't changed, preserving the timestamp. +// This prevents unnecessary git diffs when only markdown content (not frontmatter) changes. +func TestCompilerSkipsWriteWhenContentUnchanged(t *testing.T) { + // Create temporary directory for test + tmpDir := t.TempDir() + workflowPath := filepath.Join(tmpDir, "test-workflow.md") + lockPath := filepath.Join(tmpDir, "test-workflow.lock.yml") + + // Create initial workflow with frontmatter + workflowContent := `--- +engine: copilot +on: issues +permissions: + issues: read +--- + +# Test Workflow + +This is the initial markdown content. +` + err := os.WriteFile(workflowPath, []byte(workflowContent), 0644) + require.NoError(t, err, "Failed to create workflow file") + + // Compile initial workflow + compiler := NewCompiler() + compiler.SetQuiet(true) // Suppress output for cleaner test logs + err = compiler.CompileWorkflow(workflowPath) + require.NoError(t, err, "Initial compilation failed") + + // Verify lock file was created + require.FileExists(t, lockPath, "Lock file should exist after initial compilation") + + // Get initial lock file info + initialInfo, err := os.Stat(lockPath) + require.NoError(t, err, "Failed to stat lock file") + initialModTime := initialInfo.ModTime() + + // Wait a bit to ensure timestamp would be different if file is written + time.Sleep(timestampDifferentiationDelay) + + // Change ONLY the markdown content (not frontmatter) + workflowContentV2 := `--- +engine: copilot +on: issues +permissions: + issues: read +--- + +# Test Workflow + +This is DIFFERENT markdown content that should not affect the lock file. +` + err = os.WriteFile(workflowPath, []byte(workflowContentV2), 0644) + require.NoError(t, err, "Failed to update workflow file") + + // Recompile workflow + compiler2 := NewCompiler() + compiler2.SetQuiet(true) + err = compiler2.CompileWorkflow(workflowPath) + require.NoError(t, err, "Recompilation failed") + + // Check lock file timestamp - should be UNCHANGED + afterInfo, err := os.Stat(lockPath) + require.NoError(t, err, "Failed to stat lock file after recompilation") + afterModTime := afterInfo.ModTime() + + assert.Equal(t, initialModTime, afterModTime, + "Lock file timestamp should be preserved when content is unchanged") +} + +// TestCompilerWritesWhenContentChanged verifies that the compiler DOES write +// the lock file when the frontmatter changes, updating the timestamp. +func TestCompilerWritesWhenContentChanged(t *testing.T) { + // Create temporary directory for test + tmpDir := t.TempDir() + workflowPath := filepath.Join(tmpDir, "test-workflow.md") + lockPath := filepath.Join(tmpDir, "test-workflow.lock.yml") + + // Create initial workflow + workflowContent := `--- +engine: copilot +on: issues +--- + +# Test Workflow + +This is the initial markdown content. +` + err := os.WriteFile(workflowPath, []byte(workflowContent), 0644) + require.NoError(t, err, "Failed to create workflow file") + + // Compile initial workflow + compiler := NewCompiler() + compiler.SetQuiet(true) + err = compiler.CompileWorkflow(workflowPath) + require.NoError(t, err, "Initial compilation failed") + + // Verify lock file was created + require.FileExists(t, lockPath, "Lock file should exist after initial compilation") + + // Get initial lock file info + initialInfo, err := os.Stat(lockPath) + require.NoError(t, err, "Failed to stat lock file") + initialModTime := initialInfo.ModTime() + + // Wait to ensure timestamp will be different + time.Sleep(timestampDifferentiationDelay) + + // Change the FRONTMATTER (add permissions) + workflowContentV2 := `--- +engine: copilot +on: issues +permissions: + issues: read +--- + +# Test Workflow + +This is the initial markdown content. +` + err = os.WriteFile(workflowPath, []byte(workflowContentV2), 0644) + require.NoError(t, err, "Failed to update workflow file") + + // Recompile workflow + compiler2 := NewCompiler() + compiler2.SetQuiet(true) + err = compiler2.CompileWorkflow(workflowPath) + require.NoError(t, err, "Recompilation failed") + + // Check lock file timestamp - should be CHANGED + afterInfo, err := os.Stat(lockPath) + require.NoError(t, err, "Failed to stat lock file after recompilation") + afterModTime := afterInfo.ModTime() + + assert.True(t, afterModTime.After(initialModTime), + "Lock file timestamp should be updated when content changes") +} + +// TestCompilerWritesWhenLockFileMissing verifies that the compiler writes +// the lock file when it doesn't exist. +func TestCompilerWritesWhenLockFileMissing(t *testing.T) { + // Create temporary directory for test + tmpDir := t.TempDir() + workflowPath := filepath.Join(tmpDir, "test-workflow.md") + lockPath := filepath.Join(tmpDir, "test-workflow.lock.yml") + + // Create workflow + workflowContent := `--- +engine: copilot +on: issues +--- + +# Test Workflow + +Initial content. +` + err := os.WriteFile(workflowPath, []byte(workflowContent), 0644) + require.NoError(t, err, "Failed to create workflow file") + + // Compile workflow (lock file doesn't exist yet) + compiler := NewCompiler() + compiler.SetQuiet(true) + err = compiler.CompileWorkflow(workflowPath) + require.NoError(t, err, "Initial compilation failed") + + // Verify lock file was created + require.FileExists(t, lockPath, "Lock file should be created when it doesn't exist") +}