From 013bef49396b67777cb2bc53e6aff083400ebb9a Mon Sep 17 00:00:00 2001
From: Ryo Kusnadi <ryokusnadi@gmail.com>
Date: Tue, 3 Nov 2026 21:45:56 +0700
Subject: [PATCH] Fix cherry-pick changing the source commit's metadata

Signed-off-by: Ryo Kusnadi <ryokusnadi@gmail.com>
---
 pkg/graveler/graveler.go         |  2 ++
 pkg/graveler/graveler_test.go    | 44 +++++++++++++++++++++++++++++++-
 pkg/graveler/graveler_v2_test.go |  5 +++-
 3 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/pkg/graveler/graveler.go b/pkg/graveler/graveler.go
index 9b920811a41..0127e9d31e2 100644
--- a/pkg/graveler/graveler.go
+++ b/pkg/graveler/graveler.go
@@ -3035,6 +3035,8 @@ func (g *Graveler) CherryPick(ctx context.Context, repository *RepositoryRecord,
 		// the cherry-pick related keys.
 		applyCommitOverrides(&commit, commitOverrides)
 
+		// copy the map: it may be the source commit's (cached and shared) or the caller's
+		commit.Metadata = maps.Clone(commit.Metadata)
 		if commit.Metadata == nil {
 			commit.Metadata = make(map[string]string)
 		}
diff --git a/pkg/graveler/graveler_test.go b/pkg/graveler/graveler_test.go
index 84e237774ee..e499d5edb48 100644
--- a/pkg/graveler/graveler_test.go
+++ b/pkg/graveler/graveler_test.go
@@ -3494,8 +3494,14 @@ func TestGraveler_CherryPickHooks(t *testing.T) {
 			require.Equal(t, cherryPickBranchID, h.BranchID, "Hook branch ID mismatch")
 			require.Equal(t, cherryPickMessage, h.Commit.Message, "Hook commit message mismatch")
 			require.Equal(t, commitCommitter, h.Commit.Committer, "Hook commit committer mismatch")
-			diff := deep.Equal(h.Commit.Metadata, cherryPickMetadata)
+			expectedMetadata := graveler.Metadata{
+				"key1":                  "val1",
+				"cherry-pick-origin":    string(sourceCommitID),
+				"cherry-pick-committer": "",
+			}
+			diff := deep.Equal(h.Commit.Metadata, expectedMetadata)
 			require.Nil(t, diff, "Hook commit metadata diff:", diff)
+			require.Equal(t, graveler.Metadata{"key1": "val1"}, cherryPickMetadata, "Override metadata must not be modified")
 
 			// verify post-cherry-pick hook parameters if it was called
 			if postCommitCalled {
@@ -3504,3 +3510,39 @@ func TestGraveler_CherryPickHooks(t *testing.T) {
 		})
 	}
 }
+
+func TestGraveler_CherryPickDoesNotModifySourceCommitMetadata(t *testing.T) {
+	t.Parallel()
+	const expectedRangeID = graveler.MetaRangeID("expectedRangeID")
+	const expectedCommitID = graveler.CommitID("expectedCommitID")
+	const sourceCommitID = graveler.CommitID("sourceCommitID")
+	const cherryPickBranchID = graveler.BranchID("cherryPickBranchID")
+	// the ref manager returns the same *Commit on every call, like the commit cache in production
+	sourceCommit := &graveler.Commit{
+		MetaRangeID: expectedRangeID,
+		Parents:     graveler.CommitParents{expectedCommitID},
+		Metadata:    graveler.Metadata{"k": "a"},
+	}
+	refManager := &testutil.RefsFake{
+		CommitID: expectedCommitID,
+		Branch:   &graveler.Branch{CommitID: expectedCommitID, StagingToken: "st1"},
+		Commits: map[graveler.CommitID]*graveler.Commit{
+			expectedCommitID: {MetaRangeID: expectedRangeID},
+			sourceCommitID:   sourceCommit,
+		},
+		Refs: map[graveler.Ref]*graveler.ResolvedRef{
+			graveler.Ref(sourceCommitID): {
+				Type:         graveler.ReferenceTypeCommit,
+				BranchRecord: graveler.BranchRecord{Branch: &graveler.Branch{CommitID: sourceCommitID}},
+			},
+		},
+	}
+	committedManager := &testutil.CommittedFake{MetaRangeID: expectedRangeID}
+	stagingManager := &testutil.StagingFake{ValueIterator: testutil.NewValueIteratorFake(nil)}
+	g := newGraveler(t, committedManager, stagingManager, refManager, nil, testutil.NewProtectedBranchesManagerFake())
+
+	_, err := g.CherryPick(t.Context(), repository, cherryPickBranchID, graveler.Ref(sourceCommitID), nil, "committer", nil)
+	require.NoError(t, err)
+
+	require.Equal(t, graveler.Metadata{"k": "a"}, sourceCommit.Metadata, "source commit metadata must not change")
+}
diff --git a/pkg/graveler/graveler_v2_test.go b/pkg/graveler/graveler_v2_test.go
index ccef9b7dcc2..9f8bd8b4915 100644
--- a/pkg/graveler/graveler_v2_test.go
+++ b/pkg/graveler/graveler_v2_test.go
@@ -824,7 +824,10 @@ func TestGravelerCherryPick(t *testing.T) {
 		test.RefManager.EXPECT().AddCommit(ctx, repository, gomock.Any()).DoAndReturn(func(ctx context.Context, repository *graveler.RepositoryRecord, commit graveler.Commit) (graveler.CommitID, error) {
 			require.Equal(t, mr3ID, commit.MetaRangeID)
 			require.Equal(t, "", commit.Message)
-			require.Equal(t, commitOverrides.Metadata, commit.Metadata)
+			require.Equal(t, "overrideValue", commit.Metadata["overrideKey"])
+			require.Contains(t, commit.Metadata, "cherry-pick-origin")
+			require.Len(t, commit.Metadata, 3)
+			require.Equal(t, graveler.Metadata{"overrideKey": "overrideValue"}, commitOverrides.Metadata, "override metadata must not be modified")
 			return commit3ID, nil
 		}).Times(1)
 		parent := 1