Smashing an "index out of range [-1]" crash in lazygit (80k⭐)

#devchallenge #bugsmash #go #opensource

Summer Bug Smash: Clear the Lineup 🐛🛹

This is a submission for DEV's Summer Bug Smash: Clear the Lineup powered by Sentry.

Project Overview

lazygit is a terminal UI for git used by tens of thousands of developers daily. One of its power features is the custom patch builder — you cherry-pick individual hunks or files out of commits and move them around. I picked up issue #5802: a hard crash (panic: runtime error: index out of range [-1]) triggered through that feature.

Bug Fix or Performance Improvement

Repro:

Build a custom patch from a commit on branch B.
Check out branch A, which doesn't contain that commit.
Choose "Move patch out into index."
💥 panic: runtime error: index out of range [-1]

Root cause. The patch remembers the hash of the commit it was built from. When the action fires, lazygit looks that hash up in the currently-displayed commits to get an index:

func (self *CustomPatchOptionsMenuAction) getPatchCommitIndex() int {
    for index, commit := range self.c.Model().Commits {
        if commit.Hash() == self.c.Git().Patch.PatchBuilder.To {
            return index
        }
    }
    return -1 // <- not found (e.g. after switching branches)
}

After the checkout, that commit isn't in the list, so the function returns the sentinel -1. That -1 is passed straight into a slice index:

// commits[-1] panics
if commitIndex < len(commits) && commits[commitIndex].IsMerge() { ... }

The existing guard only checks the upper bound. -1 < len(commits) is true, so execution proceeds to commits[-1] and Go panics. And it isn't just the one action — the same -1 flows into all five custom-patch menu handlers (delete-from-commit, move-to-selected-commit, move-into-index, and the two pull-into-new-commit actions).

Code

PR: jesseduffield/lazygit#5865

Rather than sprinkle lower-bound guards on every slice access downstream, I fixed it at the source of the bad value: getPatchCommitIndex() now returns an error when the commit isn't found, and every caller surfaces a friendly message instead of crashing.

func (self *CustomPatchOptionsMenuAction) getPatchCommitIndex() (int, error) {
    for index, commit := range self.c.Model().Commits {
        if commit.Hash() == self.c.Git().Patch.PatchBuilder.To {
            return index, nil
        }
    }
    return -1, errors.New(self.c.Tr.PatchCommitNotInCommitsErr)
}

commitIndex, err := self.getPatchCommitIndex()
if err != nil {
    return err
}

Now, instead of a crash, you get:

Cannot find the commit this custom patch was created from in the current commits list. This can happen after switching branches; recreate the patch to continue.

My Improvements

Fixed the root cause, not the symptom. One unhandled sentinel fed five different crash paths. Guarding each downstream slice access would've been five band-aids; making the lookup return an error fixes the whole class in one place.
Added a regression test that actually reproduces it. lazygit has a great integration-test harness, so I wrote MoveToIndexWhenCommitNotInCurrentBranch: it builds a patch, checks out a branch without the source commit, and moves the patch into the index.
- On master: fails with panic: runtime error: index out of range [-1] — the exact reported crash.
- With the fix: passes (asserts the friendly error popup).
Flagged a sibling. PatchBuildingController has the same return -1 pattern; I noted it in the PR and offered to cover it too, keeping this PR scoped to the reported crash.

Best Use of Google AI

I used Google's Gemini 3.6 Flash (via AI Studio) to independently pressure-test my diagnosis before writing the fix. I gave it the buggy function, the downstream slice access, and the repro, and asked for the root cause and minimal fix. It was sharp:

Root cause: an unhandled sentinel value (-1) representing a missing commit… getPatchCommitIndex() fails to match the hash and returns -1 … commits[-1] triggers Go's runtime panic.

Why the guard fails: commitIndex < len(commits) only checks the upper bound. -1 < len(commits) evaluates to true, so short-circuiting doesn't prevent commits[commitIndex].IsMerge() from accessing commits[-1].

Fix — Option B (idiomatic): Since -1 means the commit was not found, the operation should fail early or display an error rather than rebasing against a non-existent index.

That matched my conclusion exactly — and notably Gemini preferred Option B (guard at the caller and return an error) over Option A (just add a >= 0 bound check), which is the more idiomatic, user-friendly fix and the one I shipped. Where AI got me ~90% of the way (correct root cause + the right fix shape), the remaining human work was the part it couldn't see from a snippet: that the same sentinel hit five handlers, matching lazygit's i18n/error conventions, and proving it with an integration test that fails before and passes after.