getMergeBase lacks explanation on how to fix it when it fails #212

Activity

1. JuanGalilea

   added

   CLI

   on Oct 20, 2026
2. JuanGalilea

   added the

   Task

   issue type on Oct 20, 2026
3. ilker-ekin commented on Oct 20, 2026

   

   ilker-ekin

   on Oct 20, 2026

   More actions

   Hi @JuanGalilea, thanks for splitting this out. I'd like to pick it up. I can base it on git-diff / refactor/cosmetic-change, or wait until that lands, whichever you prefer.

   I reproduced the failure modes with a shallow checkout. git merge-base <target> HEAD fails in three distinct ways, and two of them have the same exit code:

   Case	Exit code	stderr	What the user should do
   Target ref not fetched (e.g. --single-branch / default PR checkout)	128	Not a valid object name origin/master	fetch the target branch, or use fetch-depth: 0
   Shallow history, fork point not fetched	1	(empty)	fetch-depth: 0 in actions/checkout (or git fetch --deepen=<n> / --unshallow)
   Genuinely unrelated histories / wrong --target-branch	1	(empty)	check the target branch

   Cases 2 and 3 look the same from git's side, so today the user only gets Git error: with an empty message. My proposal:

   1. Catch GitCommandError in getMergeBase and branch on the cause:
      • exit 128 → "target ref <ref> is not in the local repository" + fetch hint
      • exit 1 → run git rev-parse --is-shallow-repository. If true, show the shallow-checkout message with the fetch-depth: 0 hint. Otherwise show the "no common ancestor, check --target-branch" message.
   2. Keep the wording consistent with the existing messages in resolveReleaseBaseCommit (same fetch-depth: 0 phrasing).
   3. Add unit tests for all three branches by mocking runGitCommand, similar to the existing merge-base tests in git.test.ts.
   4. Add a short bullet to the README near the existing fetch-depth: 0 note, so the error message and the docs say the same thing.

   I'd deliberately not auto-deepen or unshallow on failure. It hides a misconfigured checkout, adds a network round-trip with unbounded depth, and behaves differently across CIs. A clear, actionable error seems better. Happy to change that if you see it differently.

   Does this match what you had in mind?

   👍React with 👍1Reacted by JuanGalilea

   ❤️React with ❤️1Reacted by JuanGalilea
4. JuanGalilea commented on Oct 20, 2026

   

   JuanGalilea

   on Oct 20, 2026

   ContributorAuthor

   More actions

   you can wait if you want, but i ended up closing the git-diff ones in favor of #204 btw.

   👍React with 👍1Reacted by İlker Ekin Erdoğdu
5. metalwarrior665 commented on Oct 26, 2026

   

   metalwarrior665

   on Oct 26, 2026

   Member

   More actions

   #204 is merged so feel free to address this

   👍React with 👍1Reacted by İlker Ekin Erdoğdu

   ❤️React with ❤️1Reacted by İlker Ekin Erdoğdu