From 85b948aff7310c4b8ce92ed98957bc690d354b75 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Sat, 4 Apr 2026 17:29:38 +0000
Subject: [PATCH 1/3] Initial plan


From 661518c032c241b6af160d6d6f339befac5af10e Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Sat, 4 Apr 2026 17:57:34 +0000
Subject: [PATCH 2/3] fix: skip engine:copilot in add/add-wizard since copilot
 is the default

- Skip adding 'engine: copilot' frontmatter field since copilot is the
  default engine; adding it causes unnecessary noise and conflicts during
  later workflow updates
- Move engine addition before source addition so engine field precedes
  source in the frontmatter
- Add trailing blank line after new engine declarations to visually
  separate engine from the source field and prevent adjacent-line merge
  conflicts during updates
- Add tests for new addEngineToWorkflow behavior

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/ec85fdea-9fa4-4fdf-8385-6e567a8b0b26

Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com>
---
 pkg/cli/add_command.go              |  33 ++++----
 pkg/cli/add_source_test.go          | 122 ++++++++++++++++++++++++++++
 pkg/cli/add_workflow_compilation.go |   9 +-
 pkg/cli/frontmatter_editor.go       |   7 +-
 4 files changed, 152 insertions(+), 19 deletions(-)

diff --git a/pkg/cli/add_command.go b/pkg/cli/add_command.go
index b214657714d..42e4dac980f 100644
--- a/pkg/cli/add_command.go
+++ b/pkg/cli/add_command.go
@@ -393,6 +393,24 @@ func addWorkflowWithTracking(resolved *ResolvedWorkflow, tracker *FileTracker, o
 
 	content := string(sourceContent)
 
+	// Handle engine override - add/update the engine field in frontmatter before source so
+	// the engine declaration appears above the source field in the final file.
+	// Copilot is the default engine, so we skip adding it to avoid unnecessary noise and
+	// prevent conflicts during later workflow updates.
+	if opts.EngineOverride != "" && opts.EngineOverride != string(constants.CopilotEngine) {
+		updatedContent, err := addEngineToWorkflow(content, opts.EngineOverride)
+		if err != nil {
+			if opts.Verbose {
+				fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to set engine field: %v", err)))
+			}
+		} else {
+			content = updatedContent
+			if opts.Verbose {
+				fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Set engine field to: "+opts.EngineOverride))
+			}
+		}
+	}
+
 	// Add source field to frontmatter
 	commitSHA := ""
 	if sourceInfo != nil {
@@ -459,21 +477,6 @@ func addWorkflowWithTracking(resolved *ResolvedWorkflow, tracker *FileTracker, o
 		}
 	}
 
-	// Handle engine override - add/update the engine field in frontmatter
-	if opts.EngineOverride != "" {
-		updatedContent, err := addEngineToWorkflow(content, opts.EngineOverride)
-		if err != nil {
-			if opts.Verbose {
-				fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to set engine field: %v", err)))
-			}
-		} else {
-			content = updatedContent
-			if opts.Verbose {
-				fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Set engine field to: "+opts.EngineOverride))
-			}
-		}
-	}
-
 	// Append text if provided
 	if opts.AppendText != "" {
 		if !strings.HasSuffix(content, "\n") {
diff --git a/pkg/cli/add_source_test.go b/pkg/cli/add_source_test.go
index d090e2fd3d2..5f349bb03a0 100644
--- a/pkg/cli/add_source_test.go
+++ b/pkg/cli/add_source_test.go
@@ -179,3 +179,125 @@ This workflow has proper formatting with comments and blank lines.`,
 		})
 	}
 }
+
+// TestAddEngineToWorkflow tests the addEngineToWorkflow function
+func TestAddEngineToWorkflow(t *testing.T) {
+	tests := []struct {
+		name            string
+		content         string
+		engine          string
+		expectError     bool
+		checkEngine     bool
+		expectBlankLine bool // expect blank line after engine declaration
+	}{
+		{
+			name: "add_new_engine_field_has_trailing_blank_line",
+			content: `---
+on: push
+permissions:
+  contents: read
+---
+
+# Test Workflow`,
+			engine:          "claude",
+			expectError:     false,
+			checkEngine:     true,
+			expectBlankLine: true,
+		},
+		{
+			name: "update_existing_engine_field_no_extra_blank_line",
+			content: `---
+on: push
+engine: codex
+---
+
+# Test Workflow`,
+			engine:          "claude",
+			expectError:     false,
+			checkEngine:     true,
+			expectBlankLine: false, // updating existing field doesn't add a blank line
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result, err := addEngineToWorkflow(tt.content, tt.engine)
+
+			if tt.expectError && err == nil {
+				t.Errorf("addEngineToWorkflow() expected error, got nil")
+				return
+			}
+
+			if !tt.expectError && err != nil {
+				t.Errorf("addEngineToWorkflow() error = %v", err)
+				return
+			}
+
+			if !tt.expectError && tt.checkEngine {
+				// Verify that the engine field is present with the correct value
+				if !strings.Contains(result, "engine: "+tt.engine) {
+					t.Errorf("addEngineToWorkflow() result does not contain 'engine: %s'. Result:\n%s", tt.engine, result)
+				}
+
+				// Verify that frontmatter delimiters are present
+				if !strings.Contains(result, "---") {
+					t.Errorf("addEngineToWorkflow() result does not contain frontmatter delimiters")
+				}
+
+				// Check for blank line after engine declaration by verifying the frontmatter
+				// contains the engine line followed immediately by a blank line.
+				if tt.expectBlankLine {
+					// Extract the frontmatter section (between --- markers)
+					parts := strings.SplitN(result, "---", 3)
+					if len(parts) < 3 {
+						t.Errorf("addEngineToWorkflow() result has unexpected structure. Result:\n%s", result)
+						return
+					}
+					frontmatter := parts[1]
+					if !strings.Contains(frontmatter, "engine: "+tt.engine+"\n\n") {
+						t.Errorf("addEngineToWorkflow() frontmatter does not have blank line after engine declaration. Frontmatter:\n%s", frontmatter)
+					}
+				}
+			}
+		})
+	}
+
+	// Verify that engine declaration blank line separates it from a subsequently added source field.
+	t.Run("engine_blank_line_separates_from_source", func(t *testing.T) {
+		original := `---
+on: push
+permissions:
+  contents: read
+---
+
+# Test Workflow`
+
+		// Add engine first
+		withEngine, err := addEngineToWorkflow(original, "claude")
+		if err != nil {
+			t.Fatalf("addEngineToWorkflow() error = %v", err)
+		}
+
+		// Then add source
+		withSource, err := addSourceToWorkflow(withEngine, "owner/repo/workflows/test.md@abc123")
+		if err != nil {
+			t.Fatalf("addSourceToWorkflow() error = %v", err)
+		}
+
+		// Extract frontmatter
+		parts := strings.SplitN(withSource, "---", 3)
+		if len(parts) < 3 {
+			t.Fatalf("unexpected file structure. Result:\n%s", withSource)
+		}
+		frontmatter := parts[1]
+
+		// engine and source must NOT be adjacent (no blank line between would mean conflict risk)
+		if strings.Contains(frontmatter, "engine: claude\nsource:") {
+			t.Errorf("engine and source fields are adjacent with no blank line separator. Frontmatter:\n%s", frontmatter)
+		}
+		// engine line must be followed by a blank line
+		if !strings.Contains(frontmatter, "engine: claude\n\n") {
+			t.Errorf("engine field is not followed by a blank line. Frontmatter:\n%s", frontmatter)
+		}
+	})
+}
diff --git a/pkg/cli/add_workflow_compilation.go b/pkg/cli/add_workflow_compilation.go
index a14c623cb62..13573c733bb 100644
--- a/pkg/cli/add_workflow_compilation.go
+++ b/pkg/cli/add_workflow_compilation.go
@@ -173,12 +173,15 @@ func compileDispatchWorkflowDependencies(workflowFile string, verbose, quiet boo
 // This function preserves the existing frontmatter formatting while adding the source field.
 func addSourceToWorkflow(content, source string) (string, error) {
 	// Use shared frontmatter logic that preserves formatting
-	return addFieldToFrontmatter(content, "source", source)
+	return addFieldToFrontmatter(content, "source", source, false)
 }
 
 // addEngineToWorkflow adds or updates the engine field in the workflow's frontmatter.
 // This function preserves the existing frontmatter formatting while setting the engine field.
+// A trailing blank line is added after the engine declaration to visually separate it from
+// the source field that follows, preventing adjacent-line merge conflicts during updates.
 func addEngineToWorkflow(content, engine string) (string, error) {
-	// Use shared frontmatter logic that preserves formatting
-	return addFieldToFrontmatter(content, "engine", engine)
+	// Use shared frontmatter logic that preserves formatting; trailing blank line separates
+	// the engine declaration from the source field added immediately after.
+	return addFieldToFrontmatter(content, "engine", engine, true)
 }
diff --git a/pkg/cli/frontmatter_editor.go b/pkg/cli/frontmatter_editor.go
index f1fc51cca71..ce9bd131506 100644
--- a/pkg/cli/frontmatter_editor.go
+++ b/pkg/cli/frontmatter_editor.go
@@ -141,7 +141,9 @@ func updateFieldInFrontmatterFallback(result *parser.FrontmatterResult, fieldNam
 
 // addFieldToFrontmatter adds a new field to the frontmatter while preserving formatting.
 // This is used when we know the field doesn't exist yet.
-func addFieldToFrontmatter(content, fieldName, fieldValue string) (string, error) {
+// When trailingBlankLine is true, a blank line is appended after the new field to provide
+// visual separation from fields added subsequently (e.g., separating engine from source).
+func addFieldToFrontmatter(content, fieldName, fieldValue string, trailingBlankLine bool) (string, error) {
 	// Parse frontmatter using parser package
 	result, err := parser.ExtractFrontmatterFromContent(content)
 	if err != nil {
@@ -164,6 +166,9 @@ func addFieldToFrontmatter(content, fieldName, fieldValue string) (string, error
 		// Add field at the end of the frontmatter, preserving original formatting
 		newField := fmt.Sprintf("%s: %s", fieldName, fieldValue)
 		frontmatterLines = append(frontmatterLines, newField)
+		if trailingBlankLine {
+			frontmatterLines = append(frontmatterLines, "")
+		}
 
 		// Reconstruct the file with preserved formatting
 		var lines []string

From 6d0bcacefe24b90a28da151f6cb0a8c0ec7ae825 Mon Sep 17 00:00:00 2001
From: Don Syme <dsyme@users.noreply.github.com>
Date: Sat, 4 Apr 2026 19:45:28 +0100
Subject: [PATCH 3/3] Update pkg/cli/add_source_test.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
---
 pkg/cli/add_source_test.go | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/pkg/cli/add_source_test.go b/pkg/cli/add_source_test.go
index 5f349bb03a0..82af4916754 100644
--- a/pkg/cli/add_source_test.go
+++ b/pkg/cli/add_source_test.go
@@ -246,17 +246,21 @@ engine: codex
 
 				// Check for blank line after engine declaration by verifying the frontmatter
 				// contains the engine line followed immediately by a blank line.
+				// Extract the frontmatter section (between --- markers)
+				parts := strings.SplitN(result, "---", 3)
+				if len(parts) < 3 {
+					t.Errorf("addEngineToWorkflow() result has unexpected structure. Result:\n%s", result)
+					return
+				}
+				frontmatter := parts[1]
 				if tt.expectBlankLine {
-					// Extract the frontmatter section (between --- markers)
-					parts := strings.SplitN(result, "---", 3)
-					if len(parts) < 3 {
-						t.Errorf("addEngineToWorkflow() result has unexpected structure. Result:\n%s", result)
-						return
-					}
-					frontmatter := parts[1]
 					if !strings.Contains(frontmatter, "engine: "+tt.engine+"\n\n") {
 						t.Errorf("addEngineToWorkflow() frontmatter does not have blank line after engine declaration. Frontmatter:\n%s", frontmatter)
 					}
+				} else {
+					if strings.Contains(frontmatter, "engine: "+tt.engine+"\n\n") {
+						t.Errorf("addEngineToWorkflow() frontmatter unexpectedly has blank line after engine declaration. Frontmatter:\n%s", frontmatter)
+					}
 				}
 			}
 		})
