Skip to content

handle merge queue rewrites in sync prune#732

Open
tulioz wants to merge 1 commit into
masterfrom
prune-handle-merge-queue-rewrites
Open

handle merge queue rewrites in sync prune#732
tulioz wants to merge 1 commit into
masterfrom
prune-handle-merge-queue-rewrites

Conversation

@tulioz

@tulioz tulioz commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

merge queues rewrite pr commits so refs/pull/N/head diverges from the local tip by sha, which made the old prune check refuse to delete those branches. adds patch-id and merge-tree fallbacks

@tulioz tulioz requested a review from a team as a code owner April 24, 2026 22:01
@aviator-app

aviator-app Bot commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Current Aviator status

Aviator will automatically update this comment as the status of the PR changes.
Comment /aviator refresh to force Aviator to re-examine your PR (or learn about other /aviator commands).

This pull request is currently open (not queued).

How to merge

To merge this PR, comment /aviator merge or add the mergequeue label.


See the real-time status of this PR on the Aviator webapp.
Use the Aviator Chrome Extension to see the status of your PR within GitHub.

@aviator-app

aviator-app Bot commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

🔃 FlexReview Status

Common Owner: aviator-co/engineering (expert-load-balance assignment)
Owner and Assignment:

  • 🔒 aviator-co/engineering (expert-load-balance assignment)
    Owned Files
    • 🔒 e2e_tests/testdata/script/sync_delete_merged_queue_rewrite.txtar
    • 🔒 e2e_tests/testdata/script/sync_delete_merged_squash_rewrite.txtar
    • 🔒 e2e_tests/testdata/script/sync_keep_merged_unpushed_work.txtar
    • 🔒 internal/git/gitui/prune.go

Review SLO: 7 business hours if PR size is <= 200 LOC for the first response.

@aviator-app aviator-app Bot requested a review from simsinght April 24, 2026 22:01

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request enhances the branch pruning logic to handle cases where merge queues rebase or squash commits, causing local branch SHAs to diverge from the remote PR head. It introduces two fallback checks: branchFullyPatchMerged using git cherry for rebase merges and branchSquashMergedByTree using git merge-tree for squash merges. Comprehensive end-to-end tests were added to verify these scenarios and ensure branches with unpushed work are not accidentally deleted. Feedback suggests using the more explicit ^1 notation when referencing the parent of a merge commit for better clarity.

return false
}
ctx := context.Background()
mergeParent := mergeCommitSHA + "^"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using ^ to refer to the parent of the merge commit is generally correct for squash merges and standard merge commits (where it defaults to the first parent). However, it might be more explicit to use ^1 or ~1 to clearly indicate that we are comparing against the state of the trunk branch before the merge occurred.

Suggested change
mergeParent := mergeCommitSHA + "^"
mergeParent := mergeCommitSHA + "^1"

@tulioz tulioz force-pushed the prune-handle-merge-queue-rewrites branch from 248ce57 to 501e8c0 Compare May 18, 2026 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant