fix: remove empty parent block after last child is removed by codemod

pkg/cli/codemod_mount_as_clis_test.go

@@ -55,6 +55,7 @@ features:
 		require.NoError(t, err, "Should not error")
 		assert.True(t, applied, "Should apply codemod")
 		assert.NotContains(t, result, "mcp-cli:", "Should remove mcp-cli feature flag")
+		assert.NotContains(t, result, "features:", "Should remove empty features block")
 	})
 
 	t.Run("renames mount-as-clis and removes mcp-cli together", func(t *testing.T) {

pkg/cli/codemod_sandbox_agent_false_removal_test.go

@@ -47,7 +47,8 @@ permissions:
 	require.NoError(t, err)
 	assert.True(t, applied)
 	assert.NotContains(t, result, "agent: false")
-	assert.Contains(t, result, "sandbox:")
+	// Empty sandbox block must be removed to avoid "got null, want object" compile error
+	assert.NotContains(t, result, "sandbox:")
 }
 
 func TestSandboxAgentFalseRemoval_PreservesOtherSandboxKeys(t *testing.T) {

pkg/cli/codemod_sandbox_mcp_internal.go

@@ -21,7 +21,13 @@ func getSandboxMCPContainerRemovalCodemod() Codemod {
 				return content, false, nil
 			}
 			newContent, applied, err := applyFrontmatterLineTransform(content, func(lines []string) ([]string, bool) {
-				return removeFieldFromBlock(lines, "container", "mcp")
+				result, modified := removeFieldFromBlock(lines, "container", "mcp")
+				if modified {
+					// If mcp: became empty and was removed, also clean up sandbox: if it
+					// is now empty to avoid leaving a dangling "sandbox: null" key.
+					result = removeParentBlockIfTrulyEmpty(result, "sandbox")
+				}
+				return result, modified
 			})
 			if applied {
 				sandboxMCPInternalCodemodLog.Print("Removed deprecated sandbox.mcp.container")
@@ -48,7 +54,13 @@ func getSandboxMCPVersionRemovalCodemod() Codemod {
 				return content, false, nil
 			}
 			newContent, applied, err := applyFrontmatterLineTransform(content, func(lines []string) ([]string, bool) {
-				return removeFieldFromBlock(lines, "version", "mcp")
+				result, modified := removeFieldFromBlock(lines, "version", "mcp")
+				if modified {
+					// If mcp: became empty and was removed, also clean up sandbox: if it
+					// is now empty to avoid leaving a dangling "sandbox: null" key.
+					result = removeParentBlockIfTrulyEmpty(result, "sandbox")
+				}
+				return result, modified
 			})
 			if applied {
 				sandboxMCPInternalCodemodLog.Print("Removed deprecated sandbox.mcp.version")

pkg/cli/codemod_sandbox_mcp_internal_test.go

@@ -363,3 +363,116 @@ sandbox:
 	assert.Contains(t, result, "container: github/gh-aw-mcpg")
 	assert.Contains(t, result, "port: 8080")
 }
+
+func TestSandboxMCPContainerRemoval_RemovesEmptySandboxGrandparent(t *testing.T) {
+	// When container: is the only field under mcp:, and mcp: is the only field
+	// under sandbox:, the codemod must remove all three levels to avoid a
+	// dangling "sandbox:" key that YAML parses as null.
+	codemod := getSandboxMCPContainerRemovalCodemod()
+
+	content := `---
+on: workflow_dispatch
+sandbox:
+  mcp:
+    container: ghcr.io/example/gateway
+permissions:
+  contents: read
+---
+
+# Test`
+
+	frontmatter := map[string]any{
+		"on": "workflow_dispatch",
+		"sandbox": map[string]any{
+			"mcp": map[string]any{
+				"container": "ghcr.io/example/gateway",
+			},
+		},
+		"permissions": map[string]any{
+			"contents": "read",
+		},
+	}
+
+	result, applied, err := codemod.Apply(content, frontmatter)
+
+	require.NoError(t, err)
+	assert.True(t, applied)
+	assert.NotContains(t, result, "container:", "container field should be removed")
+	assert.NotContains(t, result, "mcp:", "empty mcp block should be removed")
+	assert.NotContains(t, result, "sandbox:", "empty sandbox block should be removed")
+	assert.Contains(t, result, "permissions:", "permissions block should be preserved")
+}
+
+func TestSandboxMCPVersionRemoval_RemovesEmptySandboxGrandparent(t *testing.T) {
+	// When version: is the only field under mcp:, and mcp: is the only field
+	// under sandbox:, the codemod must remove all three levels to avoid a
+	// dangling "sandbox:" key that YAML parses as null.
+	codemod := getSandboxMCPVersionRemovalCodemod()
+
+	content := `---
+on: workflow_dispatch
+sandbox:
+  mcp:
+    version: v0.1.0
+permissions:
+  contents: read
+---
+
+# Test`
+
+	frontmatter := map[string]any{
+		"on": "workflow_dispatch",
+		"sandbox": map[string]any{
+			"mcp": map[string]any{
+				"version": "v0.1.0",
+			},
+		},
+		"permissions": map[string]any{
+			"contents": "read",
+		},
+	}
+
+	result, applied, err := codemod.Apply(content, frontmatter)
+
+	require.NoError(t, err)
+	assert.True(t, applied)
+	assert.NotContains(t, result, "version:", "version field should be removed")
+	assert.NotContains(t, result, "mcp:", "empty mcp block should be removed")
+	assert.NotContains(t, result, "sandbox:", "empty sandbox block should be removed")
+	assert.Contains(t, result, "permissions:", "permissions block should be preserved")
+}
+
+func TestSandboxMCPContainerRemoval_KeepsSandboxWhenOtherChildrenRemain(t *testing.T) {
+	// When sandbox: has other children besides mcp:, it must be preserved even
+	// after mcp: becomes empty and is removed.
+	codemod := getSandboxMCPContainerRemovalCodemod()
+
+	content := `---
+on: workflow_dispatch
+sandbox:
+  mcp:
+    container: ghcr.io/example/gateway
+  agent: awf
+---
+
+# Test`
+
+	frontmatter := map[string]any{
+		"on": "workflow_dispatch",
+		"sandbox": map[string]any{
+			"mcp": map[string]any{
+				"container": "ghcr.io/example/gateway",
+			},
+			"agent": "awf",
+		},
+	}
+
+	result, applied, err := codemod.Apply(content, frontmatter)
+
+	require.NoError(t, err)
+	assert.True(t, applied)
+	assert.NotContains(t, result, "container:", "container field should be removed")
+	assert.NotContains(t, result, "mcp:", "empty mcp block should be removed")
+	assert.Contains(t, result, "sandbox:", "sandbox block should be kept (still has agent:)")
+	assert.Contains(t, result, "agent: awf", "other sandbox children should be preserved")
+}

pkg/cli/yaml_frontmatter_utils.go

@@ -123,8 +123,51 @@ func applyFrontmatterLineTransform(content string, transform func([]string) ([]s
 	return reconstructContent(result, markdown), true, nil
 }
 
-// removeFieldFromBlock removes a field and its nested content from a YAML block
-// Returns the modified lines and whether any changes were made
+// removeParentBlockIfTrulyEmpty removes a bare "parentBlock:" header line only
+// when there are no nested lines at all underneath it — not even comments.
+// This is intentionally more conservative than removeBlockIfEmpty: if a
+// user-authored comment is the only thing left under the block, the header is
+// kept so the comment is not silently deleted.
+func removeParentBlockIfTrulyEmpty(lines []string, parentBlock string) []string {
+	blockKeyLine := parentBlock + ":"
+	var result []string
+
+	for i, line := range lines {
+		trimmed := strings.TrimSpace(line)
+		// Only match a bare block header with no inline value (e.g. "features:")
+		if trimmed != blockKeyLine {
+			result = append(result, line)
+			continue
+		}
+
+		blockIndent := getIndentation(line)
+		hasAnyNested := false
+		for j := i + 1; j < len(lines); j++ {
+			nextTrimmed := strings.TrimSpace(lines[j])
+			if nextTrimmed == "" {
+				continue // skip blank lines
+			}
+			if len(getIndentation(lines[j])) > len(blockIndent) {
+				hasAnyNested = true
+			}
+			break
+		}
+
+		if !hasAnyNested {
+			yamlUtilsLog.Printf("Removed empty parent block '%s'", parentBlock)
+			continue // drop the header
+		}
+		result = append(result, line)
+	}
+
+	return result
+}
+
+// removeFieldFromBlock removes a field and its nested content from a YAML block.
+// If removing the field leaves the parent block truly empty (no children, not
+// even comments), the parent block line is also removed to avoid a dangling
+// "parentBlock:" key (which YAML parses as null).
+// Returns the modified lines and whether any changes were made.
 func removeFieldFromBlock(lines []string, fieldName string, parentBlock string) ([]string, bool) {
 	var result []string
 	var modified bool
@@ -194,5 +237,9 @@ func removeFieldFromBlock(lines []string, fieldName string, parentBlock string)
 		result = append(result, line)
 	}
 
+	if modified {
+		result = removeParentBlockIfTrulyEmpty(result, parentBlock)
+	}
+
 	return result, modified
 }

pkg/cli/yaml_frontmatter_utils_test.go

@@ -428,6 +428,74 @@ func TestRemoveFieldFromBlock(t *testing.T) {
 			},
 			shouldModify: false,
 		},
+		{
+			name: "removes empty parent block when last child is removed",
+			lines: []string{
+				"features:",
+				"  mcp-cli: true",
+				"permissions:",
+				"  contents: read",
+			},
+			fieldName:   "mcp-cli",
+			parentBlock: "features",
+			expectedLines: []string{
+				"permissions:",
+				"  contents: read",
+			},
+			shouldModify: true,
+		},
+		{
+			name: "keeps parent block when other children remain",
+			lines: []string{
+				"features:",
+				"  mcp-cli: true",
+				"  other-flag: true",
+				"permissions:",
+				"  contents: read",
+			},
+			fieldName:   "mcp-cli",
+			parentBlock: "features",
+			expectedLines: []string{
+				"features:",
+				"  other-flag: true",
+				"permissions:",
+				"  contents: read",
+			},
+			shouldModify: true,
+		},
+		{
+			name: "removes empty parent block when it is the last block",
+			lines: []string{
+				"engine: copilot",
+				"features:",
+				"  mcp-cli: true",
+			},
+			fieldName:   "mcp-cli",
+			parentBlock: "features",
+			expectedLines: []string{
+				"engine: copilot",
+			},
+			shouldModify: true,
+		},
+		{
+			name: "keeps parent block header when only a comment remains under it",
+			lines: []string{
+				"features:",
+				"  # user-authored comment",
+				"  mcp-cli: true",
+				"permissions:",
+				"  contents: read",
+			},
+			fieldName:   "mcp-cli",
+			parentBlock: "features",
+			expectedLines: []string{
+				"features:",
+				"  # user-authored comment",
+				"permissions:",
+				"  contents: read",
+			},
+			shouldModify: true,
+		},
 	}
 
 	for _, tt := range tests {