Skip to content

fix(sync): include git stderr in branch-delete fatal error message#719

Merged
aviator-app[bot] merged 2 commits intoaviator-co:masterfrom
mvanhorn:osc/656-branch-delete-stderr
Apr 17, 2026
Merged

fix(sync): include git stderr in branch-delete fatal error message#719
aviator-app[bot] merged 2 commits intoaviator-co:masterfrom
mvanhorn:osc/656-branch-delete-stderr

Conversation

@mvanhorn
Copy link
Copy Markdown
Contributor

Summary

Partially addresses #656. When av sync --prune (or any code path hitting runDelete in internal/git/gitui/prune.go) fails to delete a merged branch for a reason other than worktree ownership, the error message now includes the git stderr instead of the opaque exit status 1.

Why this matters

Issue #656 reports the user-facing error:

error: cannot delete merged branch "more_ci_improvements": git branch: exit status 1

The "cannot delete merged branch" wrapper text traces to line 235 of prune.go, where the fatal-error path formatted the underlying error with %v. Since the error is an *exec.ExitError, %v resolves to exit status 1 — stripping off the stderr git wrote to explain the failure. PR #660 already handled the specific "used by worktree" phrasing by catching that stderr and producing a recovery-oriented message, but any other failure reason still fell into this opaque path.

What changed

internal/git/gitui/prune.go — inside runDelete(), the else branch (non-worktree fatal error) now pulls exiterr.Stderr off the *exec.ExitError (same errutils.As helper the worktree check above already uses) and passes it to the error message:

errMsg := err.Error()
if exiterr, ok := errutils.As[*exec.ExitError](err); ok {
    if stderr := strings.TrimSpace(string(exiterr.Stderr)); stderr != "" {
        errMsg = stderr
    }
}
deletionErr = errors.Errorf("cannot delete merged branch %q: %s", branch.branch.Short(), errMsg)

So the user now sees whatever git actually said rather than the wrapped shell status.

Scope notes

Testing

go vet ./...           # pass
go build ./...         # pass
go test ./internal/git/... ./cmd/av/...   # pass

Part of #656

This contribution was developed with AI assistance (Codex).

When branch deletion during prune fails for a reason other than
worktree ownership, surface the actual git stderr instead of the
opaque 'exit status 1' returned by exec.ExitError.

Part of aviator-co#656
@mvanhorn mvanhorn requested a review from a team as a code owner April 17, 2026 04:45
@aviator-app
Copy link
Copy Markdown
Contributor

aviator-app Bot commented Apr 17, 2026

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 PR was merged using Aviator.


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
Copy link
Copy Markdown
Contributor

aviator-app Bot commented Apr 17, 2026

✅ FlexReview Status

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

  • aviator-co/engineering (expert-load-balance assignment)
    Owned Files
    • 🔒 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 tulioz April 17, 2026 04:46
Copy link
Copy Markdown

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

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 improves error reporting when deleting merged branches in the Git UI. It now captures and displays the standard error output from the Git command if an exit error occurs, providing more specific context than a generic exit status. I have no feedback to provide.

Comment thread internal/git/gitui/prune.go Outdated
errMsg = stderr
}
}
deletionErr = errors.Errorf("cannot delete merged branch %q: %s", branch.branch.Short(), errMsg)
Copy link
Copy Markdown
Contributor

@tulioz tulioz Apr 17, 2026

Choose a reason for hiding this comment

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

we're calling errutils.As[*exec.ExitError](err) twice on the same err here, once above for the worktree check and once here for stderr. can we just extract stderr once at the top of the error block, then branch on strings.Contains(stderr, "used by worktree") for the worktree case and fall through to the fatal path otherwise? also lets us drop the else after continue.

Per tulioz's review on aviator-co#719: avoid calling errutils.As[*exec.ExitError](err)
twice on the same err. Extract stderr at the top of the error block, branch
on strings.Contains for the worktree case, and drop the else after continue.

Signed-off-by: Matt Van Horn <[email protected]>
@mvanhorn
Copy link
Copy Markdown
Contributor Author

@tulioz Good call — pushed 47f68e0 to apply the refactor. stderr is now pulled out once at the top of the error block, the worktree branch uses strings.Contains(stderr, "used by worktree"), and the else after continue is gone.

@aviator-app aviator-app Bot merged commit 5f335ce into aviator-co:master Apr 17, 2026
5 checks passed
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.

2 participants