fix: remove empty parent block after last child is removed by codemod#30216
Merged
fix: remove empty parent block after last child is removed by codemod#30216
Conversation
…Block
When the last child of a parent block (e.g., features.mcp-cli) is
removed by a codemod, the dangling "features:" key is now also pruned
via the existing removeBlockIfEmpty helper. This prevents a null YAML
parse error ("got null, want object") during post-fix recompilation.
Updates:
- yaml_frontmatter_utils.go: call removeBlockIfEmpty after field removal
- yaml_frontmatter_utils_test.go: add 3 test cases for empty block removal
- codemod_mount_as_clis_test.go: assert features: block is also removed
- codemod_sandbox_agent_false_removal_test.go: update to expect empty
sandbox: block removal (same null-YAML protection)
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a01531b9-f673-45dd-9c8a-ad74deff2bc4
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a01531b9-f673-45dd-9c8a-ad74deff2bc4 Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix gh aw fix --write regression for empty features block
fix: remove empty parent block after last child is removed by codemod
May 4, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a codemod regression where removing the last child from a YAML block should also remove the now-empty parent block instead of leaving an invalid key: null frontmatter entry.
Changes:
- Updates
removeFieldFromBlockto prune an empty parent block after deleting a child field. - Adds table-driven utility tests covering empty-parent removal and sibling-preservation cases.
- Tightens codemod regression tests for
features.mcp-cliandsandbox.agent: falseso empty parent blocks are no longer expected to remain.
Show a summary per file
| File | Description |
|---|---|
pkg/cli/yaml_frontmatter_utils.go |
Adds automatic empty-parent cleanup after field removal. |
pkg/cli/yaml_frontmatter_utils_test.go |
Adds direct unit coverage for last-child removal behavior. |
pkg/cli/codemod_sandbox_agent_false_removal_test.go |
Updates sandbox codemod regression to expect empty block pruning. |
pkg/cli/codemod_mount_as_clis_test.go |
Adds regression assertion that empty features: is removed. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/4 changed files
- Comments generated: 2
Comment on lines
+199
to
+200
| if modified { | ||
| result = removeBlockIfEmpty(result, parentBlock) |
Comment on lines
+199
to
+200
| if modified { | ||
| result = removeBlockIfEmpty(result, parentBlock) |
Collaborator
|
@copilot review all comments |
…x cleanup
Issue 1 (nested blocks): sandbox.mcp.container and sandbox.mcp.version
codemods now also cascade removeParentBlockIfTrulyEmpty("sandbox") after
removeFieldFromBlock, so an empty sandbox: grandparent is pruned too
rather than left as a dangling null YAML key.
Issue 2 (comments silently deleted): replace removeBlockIfEmpty with new
removeParentBlockIfTrulyEmpty, which treats any nested line (including
user-authored comments) as 'content'. The parent block header is only
removed when there are genuinely zero nested lines, so comments are
never silently dropped.
New tests added:
- keeps_parent_block_header_when_only_a_comment_remains_under_it
- TestSandboxMCPContainerRemoval_RemovesEmptySandboxGrandparent
- TestSandboxMCPVersionRemoval_RemovesEmptySandboxGrandparent
- TestSandboxMCPContainerRemoval_KeepsSandboxWhenOtherChildrenRemain
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d2df97cd-66ec-4007-ba68-795cd1ad1798
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Addressed both review issues in commit e95ddc1: Comment 1 (nested blocks): The Comment 2 (comments silently deleted): Replaced Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
This was referenced May 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a codemod removes the last child of a parent block (e.g.
features.mcp-cliis the only key underfeatures:), the danglingfeatures:line remains — YAML parses this asnull, causing recompilation to fail witherror: got null, want object.Changes
yaml_frontmatter_utils.go— Adds a newremoveParentBlockIfTrulyEmptyhelper and calls it fromremoveFieldFromBlockafter a field is removed. Unlike the existingremoveBlockIfEmpty, the new helper treats any nested line — including user-authored comments — as "content", so parent blocks with only comments remaining are preserved rather than silently dropped. Applies to all codemods usingremoveFieldFromBlock(features.*,sandbox.*,mcp-scripts.*, etc.)codemod_sandbox_mcp_internal.go— Thesandbox.mcp.containerandsandbox.mcp.versioncodemods now cascaderemoveParentBlockIfTrulyEmpty("sandbox")afterremoveFieldFromBlock. Whenmcp:is the sole child ofsandbox:and becomes empty,sandbox:is also pruned rather than left as a danglingnullkey.yaml_frontmatter_utils_test.go— Four new table-driven cases: removes empty parent when last child deleted, keeps parent when siblings remain, removes empty parent at end-of-frontmatter, and preserves the parent block header when only a user-authored comment remains under it.codemod_sandbox_mcp_internal_test.go— Three new tests: grandparentsandbox:removal for container-only and version-onlymcp:blocks, and a case confirmingsandbox:is kept when it still has other children aftermcp:is removed.codemod_mount_as_clis_test.go— Addsassert.NotContains(result, "features:")to theremoves features.mcp-cli flagcase (the direct regression).codemod_sandbox_agent_false_removal_test.go— Updates assertion fromContains("sandbox:")toNotContains("sandbox:"): an emptysandbox:block has the same null-YAML problem and should also be removed.Before / after: