Skip to content

Commit 343c1cc

Browse files
CopilotCopilot
authored
Validate local modifications when workflow source is up-to-date (#3932)
1 parent a2d5d22 commit 343c1cc

3 files changed

Lines changed: 221 additions & 0 deletions

File tree

‎pkg/cli/frontmatter_editor.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,8 @@ func UpdateFieldInFrontmatter(content, fieldName, fieldValue string) (string, er
7373
lines = append(lines, frontmatterLines...)
7474
lines = append(lines, "---")
7575
if result.Markdown != "" {
76+
// Add empty line before markdown content to match original format
77+
lines = append(lines, "")
7678
lines = append(lines, result.Markdown)
7779
}
7880

@@ -150,6 +152,8 @@ func addFieldToFrontmatter(content, fieldName, fieldValue string) (string, error
150152
lines = append(lines, frontmatterLines...)
151153
lines = append(lines, "---")
152154
if result.Markdown != "" {
155+
// Add empty line before markdown content to match original format
156+
lines = append(lines, "")
153157
lines = append(lines, result.Markdown)
154158
}
155159

‎pkg/cli/update_command.go‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -571,6 +571,30 @@ func updateWorkflow(wf *workflowWithSource, allowMajor, force, verbose bool, eng
571571

572572
// Check if update is needed
573573
if !force && currentRef == latestRef {
574+
// Download the source content to check if local file has been modified
575+
sourceContent, err := downloadWorkflowContent(sourceSpec.Repo, sourceSpec.Path, currentRef, verbose)
576+
if err != nil {
577+
// If we can't download for comparison, just show the up-to-date message
578+
if verbose {
579+
fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to download source for comparison: %v", err)))
580+
}
581+
fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Workflow %s is already up to date (%s)", wf.Name, currentRef)))
582+
return nil
583+
}
584+
585+
// Read current workflow content
586+
currentContent, err := os.ReadFile(wf.Path)
587+
if err != nil {
588+
return fmt.Errorf("failed to read current workflow: %w", err)
589+
}
590+
591+
// Check if local file differs from source
592+
if hasLocalModifications(string(sourceContent), string(currentContent), wf.SourceSpec, verbose) {
593+
fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Workflow %s is already up to date (%s)", wf.Name, currentRef)))
594+
fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("⚠️ Local copy of %s has been modified from source", wf.Name)))
595+
return nil
596+
}
597+
574598
fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Workflow %s is already up to date (%s)", wf.Name, currentRef)))
575599
return nil
576600
}
@@ -670,6 +694,64 @@ func normalizeWhitespace(content string) string {
670694
return normalized
671695
}
672696

697+
// hasLocalModifications checks if the local workflow file has been modified from its source
698+
// It resolves the source field and imports on the remote content, then compares with local
699+
func hasLocalModifications(sourceContent, localContent, sourceSpec string, verbose bool) bool {
700+
// Normalize both contents
701+
sourceNormalized := normalizeWhitespace(sourceContent)
702+
localNormalized := normalizeWhitespace(localContent)
703+
704+
// Parse the source spec to get repo and ref information
705+
parsedSourceSpec, err := parseSourceSpec(sourceSpec)
706+
if err != nil {
707+
if verbose {
708+
fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf("Failed to parse source spec: %v", err)))
709+
}
710+
// Fall back to simple comparison
711+
return sourceNormalized != localNormalized
712+
}
713+
714+
// Add the source field to the remote content
715+
sourceWithSource, err := UpdateFieldInFrontmatter(sourceNormalized, "source", sourceSpec)
716+
if err != nil {
717+
if verbose {
718+
fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf("Failed to add source field to remote content: %v", err)))
719+
}
720+
// Fall back to simple comparison
721+
return sourceNormalized != localNormalized
722+
}
723+
724+
// Resolve imports on the remote content
725+
workflow := &WorkflowSpec{
726+
RepoSpec: RepoSpec{
727+
RepoSlug: parsedSourceSpec.Repo,
728+
Version: parsedSourceSpec.Ref,
729+
},
730+
WorkflowPath: parsedSourceSpec.Path,
731+
}
732+
733+
sourceResolved, err := processIncludesInContent(sourceWithSource, workflow, parsedSourceSpec.Ref, verbose)
734+
if err != nil {
735+
if verbose {
736+
fmt.Fprintln(os.Stderr, console.FormatVerboseMessage(fmt.Sprintf("Failed to process imports on remote content: %v", err)))
737+
}
738+
// Use the version with source field but without resolved imports
739+
sourceResolved = sourceWithSource
740+
}
741+
742+
// Normalize again after processing
743+
sourceResolvedNormalized := normalizeWhitespace(sourceResolved)
744+
745+
// Compare the normalized contents
746+
hasModifications := sourceResolvedNormalized != localNormalized
747+
748+
if verbose && hasModifications {
749+
fmt.Fprintln(os.Stderr, console.FormatVerboseMessage("Local modifications detected"))
750+
}
751+
752+
return hasModifications
753+
}
754+
673755
// MergeWorkflowContent performs a 3-way merge of workflow content using git merge-file
674756
// It returns the merged content, whether conflicts exist, and any error
675757
func MergeWorkflowContent(base, current, new, oldSourceSpec, newRef string, verbose bool) (string, bool, error) {

‎pkg/cli/update_command_test.go‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -511,3 +511,138 @@ func TestShowUpdateSummary(t *testing.T) {
511511
})
512512
}
513513
}
514+
515+
// TestHasLocalModifications tests the local modifications detection
516+
func TestHasLocalModifications(t *testing.T) {
517+
tests := []struct {
518+
name string
519+
sourceContent string
520+
localContent string
521+
sourceSpec string
522+
expectModified bool
523+
description string
524+
}{
525+
{
526+
name: "no modifications - identical content",
527+
sourceContent: `---
528+
on: push
529+
engine: claude
530+
---
531+
532+
# Test Workflow
533+
534+
Test content.`,
535+
localContent: `---
536+
on: push
537+
engine: claude
538+
source: test/repo/workflow.md@v1.0.0
539+
---
540+
541+
# Test Workflow
542+
543+
Test content.`,
544+
sourceSpec: "test/repo/workflow.md@v1.0.0",
545+
expectModified: false,
546+
description: "Local file with source field should match source without it",
547+
},
548+
{
549+
name: "local modifications in frontmatter",
550+
sourceContent: `---
551+
on: push
552+
engine: claude
553+
---
554+
555+
# Test Workflow
556+
557+
Test content.`,
558+
localContent: `---
559+
on: push
560+
engine: claude
561+
permissions:
562+
contents: read
563+
source: test/repo/workflow.md@v1.0.0
564+
---
565+
566+
# Test Workflow
567+
568+
Test content.`,
569+
sourceSpec: "test/repo/workflow.md@v1.0.0",
570+
expectModified: true,
571+
description: "Local has extra permissions field",
572+
},
573+
{
574+
name: "local modifications in markdown",
575+
sourceContent: `---
576+
on: push
577+
engine: claude
578+
---
579+
580+
# Test Workflow
581+
582+
Test content.`,
583+
localContent: `---
584+
on: push
585+
engine: claude
586+
source: test/repo/workflow.md@v1.0.0
587+
---
588+
589+
# Test Workflow
590+
591+
Test content with local additions.`,
592+
sourceSpec: "test/repo/workflow.md@v1.0.0",
593+
expectModified: true,
594+
description: "Local has modified markdown content",
595+
},
596+
{
597+
name: "whitespace differences should be ignored",
598+
sourceContent: `---
599+
on: push
600+
engine: claude
601+
---
602+
603+
# Test Workflow
604+
605+
Test content.`,
606+
localContent: `---
607+
on: push
608+
engine: claude
609+
source: test/repo/workflow.md@v1.0.0
610+
---
611+
612+
# Test Workflow
613+
614+
Test content.
615+
`,
616+
sourceSpec: "test/repo/workflow.md@v1.0.0",
617+
expectModified: false,
618+
description: "Trailing whitespace should be normalized",
619+
},
620+
{
621+
name: "both empty",
622+
sourceContent: `---
623+
on: push
624+
---
625+
626+
# Empty`,
627+
localContent: `---
628+
on: push
629+
source: test/repo/workflow.md@v1.0.0
630+
---
631+
632+
# Empty`,
633+
sourceSpec: "test/repo/workflow.md@v1.0.0",
634+
expectModified: false,
635+
description: "Both files minimal but identical",
636+
},
637+
}
638+
639+
for _, tt := range tests {
640+
t.Run(tt.name, func(t *testing.T) {
641+
result := hasLocalModifications(tt.sourceContent, tt.localContent, tt.sourceSpec, false)
642+
643+
if result != tt.expectModified {
644+
t.Errorf("%s: expected modified=%v, got %v", tt.description, tt.expectModified, result)
645+
}
646+
})
647+
}
648+
}

0 commit comments

Comments
 (0)