Skip to content

Commit 14b20a4

Browse files
CopilotdsymeCopilot
authored
Fix add-wizard corrupting frontmatter when engine is a block mapping (#18486)
* Initial plan * Fix UpdateFieldInFrontmatter to remove child lines when replacing a block-mapped field Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com> * Update pkg/cli/frontmatter_formatting_test.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update pkg/cli/frontmatter_editor.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix fmt --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com> Co-authored-by: Don Syme <dsyme@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Don Syme <dsyme@github.com>
1 parent 4d12abd commit 14b20a4

2 files changed

Lines changed: 127 additions & 10 deletions

File tree

‎pkg/cli/frontmatter_editor.go‎

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,26 @@ func UpdateFieldInFrontmatter(content, fieldName, fieldValue string) (string, er
3131
frontmatterEditorLog.Printf("Using raw frontmatter lines for field update (%d lines)", len(result.FrontmatterLines))
3232
// Look for existing field in the raw lines
3333
fieldUpdated := false
34-
frontmatterLines := append([]string(nil), result.FrontmatterLines...)
34+
skipChildren := false
35+
fieldIndentLevel := 0
36+
newFrontmatterLines := make([]string, 0, len(result.FrontmatterLines))
3537

36-
// Try to find and update the field in place
37-
for i, line := range frontmatterLines {
38+
for _, line := range result.FrontmatterLines {
3839
trimmedLine := strings.TrimSpace(line)
40+
41+
// If we just updated the field, skip its child lines (block mapping values)
42+
if skipChildren {
43+
currentIndent := len(line) - len(strings.TrimLeft(line, " \t"))
44+
if currentIndent > fieldIndentLevel {
45+
// This line is a child of the replaced field — drop it
46+
continue
47+
}
48+
// No longer in the child block
49+
skipChildren = false
50+
}
51+
3952
// Check if this line contains our field
40-
if strings.HasPrefix(trimmedLine, fieldName+":") {
53+
if !fieldUpdated && strings.HasPrefix(trimmedLine, fieldName+":") {
4154
// Preserve the original indentation and comments
4255
leadingSpace := line[:len(line)-len(strings.TrimLeft(line, " \t"))]
4356

@@ -49,28 +62,35 @@ func UpdateFieldInFrontmatter(content, fieldName, fieldValue string) (string, er
4962
}
5063

5164
// Update the field value while preserving formatting
65+
var updatedLine string
5266
if comment != "" {
53-
frontmatterLines[i] = fmt.Sprintf("%s%s: %s %s", leadingSpace, fieldName, fieldValue, comment)
67+
updatedLine = fmt.Sprintf("%s%s: %s %s", leadingSpace, fieldName, fieldValue, comment)
5468
} else {
55-
frontmatterLines[i] = fmt.Sprintf("%s%s: %s", leadingSpace, fieldName, fieldValue)
69+
updatedLine = fmt.Sprintf("%s%s: %s", leadingSpace, fieldName, fieldValue)
5670
}
71+
newFrontmatterLines = append(newFrontmatterLines, updatedLine)
5772
fieldUpdated = true
58-
frontmatterEditorLog.Printf("Updated existing field %s in place (line %d)", fieldName, i+1)
59-
break
73+
// Track the indent level so we can skip any child lines that follow
74+
fieldIndentLevel = len(leadingSpace)
75+
skipChildren = true
76+
frontmatterEditorLog.Printf("Updated existing field %s", fieldName)
77+
continue
6078
}
79+
80+
newFrontmatterLines = append(newFrontmatterLines, line)
6181
}
6282

6383
// If field wasn't found in the raw lines, add it at the end
6484
if !fieldUpdated {
6585
newField := fmt.Sprintf("%s: %s", fieldName, fieldValue)
66-
frontmatterLines = append(frontmatterLines, newField)
86+
newFrontmatterLines = append(newFrontmatterLines, newField)
6787
frontmatterEditorLog.Printf("Added new field %s at end of frontmatter", fieldName)
6888
}
6989

7090
// Reconstruct the file with preserved formatting
7191
var lines []string
7292
lines = append(lines, "---")
73-
lines = append(lines, frontmatterLines...)
93+
lines = append(lines, newFrontmatterLines...)
7494
lines = append(lines, "---")
7595
if result.Markdown != "" {
7696
// Add empty line before markdown content to match original format

‎pkg/cli/frontmatter_formatting_test.go‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,103 @@ This is test content.`
139139
})
140140
}
141141

142+
// TestUpdateFieldInFrontmatterBlockMapping tests that UpdateFieldInFrontmatter correctly replaces
143+
// a block-mapped field (multi-line YAML object) with a scalar value, removing child lines.
144+
// This mirrors the add-wizard bug where a block-mapped engine:
145+
//
146+
// id: claude
147+
//
148+
// was updated to engine: copilot but the child " id: claude" line remained, producing invalid YAML.
149+
func TestUpdateFieldInFrontmatterBlockMapping(t *testing.T) {
150+
t.Run("replace block-mapped engine with scalar value removes child lines", func(t *testing.T) {
151+
content := `---
152+
engine:
153+
id: claude
154+
permissions:
155+
contents: read
156+
---
157+
158+
# Test Workflow`
159+
160+
result, err := UpdateFieldInFrontmatter(content, "engine", "copilot")
161+
if err != nil {
162+
t.Fatalf("Unexpected error: %v", err)
163+
}
164+
165+
// Should have the new scalar engine value
166+
if !strings.Contains(result, "engine: copilot") {
167+
t.Error("engine field was not updated to copilot")
168+
}
169+
170+
// The child line " id: claude" must be removed
171+
if strings.Contains(result, "id: claude") {
172+
t.Error("child line 'id: claude' was not removed when replacing block-mapped engine")
173+
}
174+
175+
// Other fields should be preserved
176+
if !strings.Contains(result, "permissions:") {
177+
t.Error("permissions field was removed unexpectedly")
178+
}
179+
if !strings.Contains(result, "contents: read") {
180+
t.Error("permissions contents field was removed unexpectedly")
181+
}
182+
})
183+
184+
t.Run("replace block-mapped engine with deeper nesting removes all child lines", func(t *testing.T) {
185+
content := `---
186+
engine:
187+
id: claude
188+
model: claude-3-5-sonnet
189+
source: owner/repo/workflow.md@main
190+
---
191+
192+
# Test`
193+
194+
result, err := UpdateFieldInFrontmatter(content, "engine", "copilot")
195+
if err != nil {
196+
t.Fatalf("Unexpected error: %v", err)
197+
}
198+
199+
if !strings.Contains(result, "engine: copilot") {
200+
t.Error("engine field was not updated to copilot")
201+
}
202+
if strings.Contains(result, "id: claude") {
203+
t.Error("child line 'id: claude' was not removed")
204+
}
205+
if strings.Contains(result, "model: claude-3-5-sonnet") {
206+
t.Error("child line 'model: claude-3-5-sonnet' was not removed")
207+
}
208+
if !strings.Contains(result, "source: owner/repo/workflow.md@main") {
209+
t.Error("source field was removed unexpectedly")
210+
}
211+
})
212+
213+
t.Run("replace scalar engine still works correctly", func(t *testing.T) {
214+
content := `---
215+
engine: claude
216+
permissions:
217+
contents: read
218+
---
219+
220+
# Test`
221+
222+
result, err := UpdateFieldInFrontmatter(content, "engine", "copilot")
223+
if err != nil {
224+
t.Fatalf("Unexpected error: %v", err)
225+
}
226+
227+
if !strings.Contains(result, "engine: copilot") {
228+
t.Error("engine field was not updated to copilot")
229+
}
230+
if strings.Contains(result, "engine: claude") {
231+
t.Error("old engine value was not replaced")
232+
}
233+
if !strings.Contains(result, "permissions:") {
234+
t.Error("permissions field was removed unexpectedly")
235+
}
236+
})
237+
}
238+
142239
// TestRemoveFieldFromOnTriggerEdgeCases tests edge cases for field removal
143240
func TestRemoveFieldFromOnTriggerEdgeCases(t *testing.T) {
144241
t.Run("remove field that doesn't exist", func(t *testing.T) {

0 commit comments

Comments
 (0)