[0.16] fix(submit): panic when there are merged downstack nodes (#790)

Cherry picks #789 from main.

When the 'spice.submit.navigationCommentSync' is set to 'downstack',
we traverse the downstack nodes to update their navigation comments.
This logic did not account for the possibility of some downstack nodes
being already merged, which means they do not have corresponding
infos entries.

Resolves #788

Author: abhinav

.changes/v0.16.1.md

@@ -0,0 +1,3 @@
+## <a name="v0.16.1">v0.16.1</a> - 2025-08-08
+### Fixed
+- Fix panic when the 'spice.submit.navigationCommentSync' configuration option is set to 'downstack', and one of the downstacks has already been merged.

CHANGELOG.md

@@ -5,6 +5,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html),
 and is generated by [Changie](https://github.com/miniscruff/changie).
 
+## <a name="v0.16.1">v0.16.1</a> - 2025-08-08
+### Fixed
+- Fix panic when the 'spice.submit.navigationCommentSync' configuration option is set to 'downstack', and one of the downstacks has already been merged.
+
 ## <a name="v0.16.0">v0.16.0</a> - 2025-08-02
 
 This release contains a number of improvements.

internal/handler/submit/nav_comment.go

@@ -224,7 +224,13 @@ func updateNavigationComments(
 
 			baseIdx := nodes[nodeIdx].Base
 			for baseIdx != -1 {
-				updateBranches[baseIdx] = struct{}{}
+				// Only add tracked branches
+				// that have corresponding infos entries.
+				// Merged downstack nodes don't have info
+				// and can't be commented on.
+				if baseIdx < len(infos) {
+					updateBranches[baseIdx] = struct{}{}
+				}
 				baseIdx = nodes[baseIdx].Base
 			}
 		}

internal/handler/submit/nav_comment_test.go

@@ -22,9 +22,10 @@ import (
 
 func TestUpdateNavigationComments(t *testing.T) {
 	type trackedBranch struct {
-		Name     string
-		Base     string // empty = trunk
-		ChangeID int    // 0 = unsubmitted
+		Name            string
+		Base            string // empty = trunk
+		ChangeID        int    // 0 = unsubmitted
+		MergedDownstack []int  // list of merged downstack change IDs
 	}
 
 	tests := []struct {
@@ -273,6 +274,42 @@ func TestUpdateNavigationComments(t *testing.T) {
 				),
 			},
 		},
+		{
+			// Regression test for https://github.com/abhinav/git-spice/issues/788
+			name: "MergedDownstack",
+			trackedBranches: []trackedBranch{
+				{Name: "feat1", ChangeID: 123, MergedDownstack: []int{100, 101}},
+				{Name: "feat2", Base: "feat1", ChangeID: 124},
+				{Name: "feat3", Base: "feat2", ChangeID: 125},
+			},
+			sync:   NavCommentSyncDownstack,
+			submit: []string{"feat3"},
+			// This should not panic when accessing infos[idx]
+			// where idx corresponds to merged downstack nodes
+			wantComments: map[int]string{
+				123: joinLines(
+					"- #100",
+					"    - #101",
+					"        - #123 ◀",
+					"            - #124",
+					"                - #125",
+				),
+				124: joinLines(
+					"- #100",
+					"    - #101",
+					"        - #123",
+					"            - #124 ◀",
+					"                - #125",
+				),
+				125: joinLines(
+					"- #100",
+					"    - #101",
+					"        - #123",
+					"            - #124",
+					"                - #125 ◀",
+				),
+			},
+		},
 	}
 
 	for _, tt := range tests {
@@ -301,6 +338,14 @@ func TestUpdateNavigationComments(t *testing.T) {
 							}
 						}
 
+						// Add merged downstack entries if any
+						for _, mergedID := range b.MergedDownstack {
+							mergedMeta := &shamhub.ChangeMetadata{Number: mergedID}
+							mergedJSON, err := json.Marshal(mergedMeta)
+							require.NoError(t, err)
+							item.MergedDownstack = append(item.MergedDownstack, mergedJSON)
+						}
+
 						items[i] = item
 					}
 
@@ -318,6 +363,16 @@ func TestUpdateNavigationComments(t *testing.T) {
 					return json.Marshal(md)
 				}).
 				AnyTimes()
+			mockForge.EXPECT().
+				UnmarshalChangeID(gomock.Any()).
+				DoAndReturn(func(data json.RawMessage) (forge.ChangeID, error) {
+					var md shamhub.ChangeMetadata
+					if err := json.Unmarshal(data, &md); err != nil {
+						return nil, err
+					}
+					return shamhub.ChangeID(md.Number), nil
+				}).
+				AnyTimes()
 
 			mockRemoteRepo := forgetest.NewMockRepository(ctrl)
 			mockRemoteRepo.EXPECT().Forge().Return(mockForge).AnyTimes()

testdata/script/branch_submit_navigation_comment_sync_downstack.txt

@@ -40,6 +40,21 @@ gs bs --fill
 shamhub dump comments
 cmp stdout $WORK/golden/all-comments.txt
 
+# Merge a change and update.
+shamhub merge alice/example 1
+gs repo sync
+gs stack restack
+
+# Re-submit each branch
+gs bco feature2
+gs branch submit
+gs bco feature3
+gs branch submit
+
+# Comments are the same.
+shamhub dump comments
+cmp stdout $WORK/golden/all-comments.txt
+
 -- repo/feature1.txt --
 feature 1