cli: try squash merge first, fall back to merge commit if not allowed#25609
cli: try squash merge first, fall back to merge commit if not allowed#25609
Conversation
|
@copilot try squash merge first |
…t allowed Agent-Logs-Url: https://github.com/github/gh-aw/sessions/09aca23e-7fc7-47c4-9bb4-76a7fa5a2ad0 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…allowed Agent-Logs-Url: https://github.com/github/gh-aw/sessions/dfe9e489-1d7b-43ef-8c5b-1e6696b05d06 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates the interactive add flow’s PR auto-merge behavior to handle repositories with merge-method restrictions by trying squash merges first and retrying with a merge commit when squash merges are disallowed.
Changes:
- Add detection for “squash merges are not allowed” errors from
gh pr merge --squash. - Attempt
gh pr merge --squashfirst, and fall back to--mergeon the detected restriction. - Print an informational message when switching strategies so the retry is visible to the user.
Show a summary per file
| File | Description |
|---|---|
| pkg/cli/add_interactive_git.go | Implements squash-first merge strategy with fallback to merge-commit when squash merges aren’t permitted. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
pkg/cli/add_interactive_git.go:305
- If the fallback
--mergeattempt fails, the returned error only includes the merge-commit output, losing the original squash failure context (and why the retry happened). Consider including both the initial squash error/output and the merge-commit error/output in the final error, so diagnosis doesn’t depend on having seen the transient stderr info message.
if strings.Contains(combinedText, squashMergeNotAllowedErr) {
fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Squash merges are not allowed on this repository, retrying with merge commit"))
mergeOutput, mergeErr := workflow.RunGHCombined("Merging pull request...", "pr", "merge", prArg, "--repo", c.RepoOverride, "--merge")
if mergeErr != nil {
return fmt.Errorf("merge failed: %w (output: %s)", mergeErr, string(mergeOutput))
}
- Files reviewed: 1/1 changed files
- Comments generated: 1
| func (c *AddInteractiveConfig) mergePullRequest(prNumber int) error { | ||
| output, err := workflow.RunGHCombined("Merging pull request...", "pr", "merge", strconv.Itoa(prNumber), "--repo", c.RepoOverride, "--merge") | ||
| if err != nil { | ||
| return fmt.Errorf("merge failed: %w (output: %s)", err, string(output)) | ||
| prArg := strconv.Itoa(prNumber) | ||
| squashOutput, squashErr := workflow.RunGHCombined("Merging pull request (squash)...", "pr", "merge", prArg, "--repo", c.RepoOverride, "--squash") |
There was a problem hiding this comment.
The returned error message always says "merge failed" even when the attempted strategy was a squash merge. This makes troubleshooting harder (e.g., users can’t tell whether the failure came from --squash vs the retry path). Consider making the error text strategy-specific (e.g., "squash merge failed") so the surfaced error matches what was attempted.
This issue also appears on line 300 of the same file.
Repositories that restrict merge commits fail the
gh pr merge --mergecall withGraphQL: Merge commits are not allowed on this repository.— makinggh aw addunable to auto-merge the setup PR.Changes
mergePullRequestmerge strategy — attempts squash merge first (--squash); if the repository does not permit squash merges, detects the GitHub GraphQL error (case-insensitive, checked against both combined output and error string) and retries with a merge commit (--merge)squashMergeNotAllowedErrconstant — names the expected error substring for clarity and easy updates if the API message ever changesconsole.FormatInfoMessagebefore the retry so the strategy change is visible