manual review/cleanup

go/libraries/doltcore/cherry_pick/cherry_pick.go

@@ -144,16 +144,13 @@ func CherryPick(ctx *sql.Context, commit string, options CherryPickOptions) (str
 		// Returning nil error instead lets CommitTransaction persist the staged changes and merge
 		// state, so the user can fix the failing tests and --continue.
 		if actions.ErrCommitVerificationFailed.Is(err) {
-			// Build the new working set: start from the pre-apply WS (so preMergeWorking
-			// = original working root for correct abort behavior), then apply the current
-			// working and staged roots.
 			newWs := preApplyWs.StartCherryPick(originalCommit, commit).
 				WithWorkingRoot(roots.Working).
 				WithStagedRoot(roots.Staged)
 			if wsErr := doltSession.SetWorkingSet(ctx, dbName, newWs); wsErr != nil {
 				return "", nil, wsErr
 			}
-			mergeResult.VerificationFailureErr = err
+			mergeResult.CommitVerificationErr = err
 			return "", mergeResult, nil
 		}
 		return "", nil, err
@@ -336,9 +333,6 @@ func ContinueCherryPick(ctx *sql.Context, dbName string) (string, int, int, int,
 
 	pendingCommit, err := doltSession.NewPendingCommit(ctx, dbName, roots, commitProps)
 	if err != nil {
-		// If verification failed, return the specific error so the caller can surface the failing
-		// test details. The merge state on disk remains active (it was committed in the previous
-		// cherry-pick call), so the user can fix the failing tests and --continue again.
 		if actions.ErrCommitVerificationFailed.Is(err) {
 			return "", 0, 0, 0, err
 		}

go/libraries/doltcore/env/actions/commit.go

@@ -29,13 +29,8 @@ import (
 	"github.com/dolthub/dolt/go/store/datas"
 )
 
-// CommitVerificationFailedPrefix is the prefix of the error message returned when commit
-// verification tests fail. It is used both to generate the error and to detect it in CLI output.
 const CommitVerificationFailedPrefix = "commit verification failed:"
 
-// ErrCommitVerificationFailed is returned when commit verification tests fail during a commit,
-// cherry-pick, merge, or rebase operation. The workspace is left dirty so the user can fix the
-// failing tests and retry the operation using --continue (cherry-pick/rebase) or CALL dolt_commit() (merge).
 var ErrCommitVerificationFailed = goerrors.NewKind(CommitVerificationFailedPrefix + " %s")
 
 type CommitStagedProps struct {

go/libraries/doltcore/merge/merge.go

@@ -87,13 +87,10 @@ func MergeCommits(ctx *sql.Context, tableResolver doltdb.TableResolver, commit,
 }
 
 type Result struct {
-	Root            doltdb.RootValue
-	SchemaConflicts []SchemaConflict
-	Stats           map[doltdb.TableName]*MergeStats
-	// VerificationFailureErr is set when commit verification tests fail during a cherry-pick or merge.
-	// When set, the operation is halted and the workspace is left dirty so the user can fix the
-	// failing tests and retry using --continue (cherry-pick) or CALL dolt_commit() (merge).
-	VerificationFailureErr error
+	Root                  doltdb.RootValue
+	SchemaConflicts       []SchemaConflict
+	Stats                 map[doltdb.TableName]*MergeStats
+	CommitVerificationErr error
 }
 
 func (r Result) HasSchemaConflicts() bool {

go/libraries/doltcore/sqle/dprocedures/dolt_cherry_pick.go

@@ -121,15 +121,14 @@ func doDoltCherryPick(ctx *sql.Context, args []string) (string, int, int, int, e
 	}
 
 	if mergeResult != nil {
-		if mergeResult.VerificationFailureErr != nil {
-			// Commit the transaction to persist the dirty working set and merge state to disk,
-			// then return the specific verification error. The caller (CLI or SQL client) receives
-			// the error message including the failing test name and details.
+		if mergeResult.CommitVerificationErr != nil {
+			// Commit the transaction to persist the dirty working set to the staged working set.
+			// This allows the user to address the verification failure and then `--continue` the cherry-pick.
 			doltSession := dsess.DSessFromSess(ctx.Session)
 			if txErr := doltSession.CommitTransaction(ctx, doltSession.GetTransaction()); txErr != nil {
 				return "", 0, 0, 0, txErr
 			}
-			return "", 0, 0, 0, mergeResult.VerificationFailureErr
+			return "", 0, 0, 0, mergeResult.CommitVerificationErr
 		}
 		return "",
 			mergeResult.CountOfTablesWithDataConflicts(),

go/libraries/doltcore/sqle/dprocedures/dolt_merge.go

@@ -246,8 +246,6 @@ func performMerge(
 				ctx.Warn(DoltMergeWarningCode, "%s", err.Error())
 				return ws, "", hasConflictsOrViolations, threeWayMerge, "", err
 			} else if actions.ErrCommitVerificationFailed.Is(err) {
-				// Verification failed: working set already has dirty merge state in the session.
-				// Return without a SQL error so the transaction commits and preserves dirty state.
 				return ws, "", noConflictsOrViolations, threeWayMerge, err.Error(), nil
 			} else if err != nil {
 				return ws, "", noConflictsOrViolations, threeWayMerge, "", err
@@ -319,9 +317,6 @@ func performMerge(
 		commit, _, err = doDoltCommit(ctx, args)
 		if err != nil {
 			if actions.ErrCommitVerificationFailed.Is(err) {
-				// Verification failed: the working set already has the dirty merge state set in the session
-				// (SetWorkingSet was called before doDoltCommit). Return without a SQL error so the
-				// transaction commits and preserves the dirty state for the user to fix and retry.
 				return ws, "", noConflictsOrViolations, threeWayMerge, err.Error(), nil
 			}
 			return ws, commit, noConflictsOrViolations, threeWayMerge, "", err

go/libraries/doltcore/sqle/dprocedures/dolt_rebase.go

@@ -144,7 +144,6 @@ var ErrRebaseDataConflict = goerrors.NewKind(RebaseDataConflictPrefix + " %s (%s
 	"Resolve the conflicts and remove them from the dolt_conflicts_<table> tables, " +
 	"then continue the rebase by calling dolt_rebase('--continue')")
 
-// RebaseVerificationFailedPrefix is a prefix used by ErrRebaseVerificationFailed.
 const RebaseVerificationFailedPrefix = "commit verification failed while rebasing commit"
 
 // ErrRebaseVerificationFailed is used when commit verification tests fail while rebasing a commit.
@@ -993,10 +992,7 @@ func handleRebaseCherryPick(
 		return newRebaseError(ErrRebaseDataConflict.New(planStep.CommitHash, planStep.CommitMsg))
 	}
 
-	// If commit verification failed, commit the SQL transaction to preserve the dirty state (staged changes
-	// and merge state set by cherry-pick), then return an error to halt the rebase. The user can fix the
-	// failing tests, run dolt_add(), then call dolt_rebase('--continue') to retry.
-	if mergeResult != nil && mergeResult.VerificationFailureErr != nil {
+	if mergeResult != nil && mergeResult.CommitVerificationErr != nil {
 		if doltSession.GetTransaction() == nil {
 			_, txErr := doltSession.StartTransaction(ctx, sql.ReadWrite)
 			if txErr != nil {
@@ -1006,11 +1002,7 @@ func handleRebaseCherryPick(
 		if txErr := doltSession.CommitTransaction(ctx, doltSession.GetTransaction()); txErr != nil {
 			return newRebaseError(txErr)
 		}
-		// Return the verification failure error directly so the user sees the standard
-		// "commit verification failed: ..." message format, and so the engine tests can
-		// match against it exactly. The CLI detects this via isRebaseConflictError which
-		// checks for the "commit verification failed:" prefix.
-		return newRebaseError(mergeResult.VerificationFailureErr)
+		return newRebaseError(mergeResult.CommitVerificationErr)
 	}
 
 	// If this is an edit action and no conflicts occurred, pause the rebase to allow user modifications

go/libraries/doltcore/sqle/enginetest/dolt_queries_commit_verification.go

@@ -188,7 +188,7 @@ var DoltCommitVerificationScripts = []queries.ScriptTest{
 		},
 	},
 	{
-		Name: "cherry-pick with test verification enabled - tests pass",
+		Name: "cherry-pick with test verification enabled - test pass, then test fails",
 		SetUpScript: []string{
 			"SET GLOBAL dolt_commit_verification_groups = '*'",
 			"CREATE TABLE users (id INT PRIMARY KEY, name VARCHAR(100) NOT NULL, email VARCHAR(100))",
@@ -209,19 +209,29 @@ var DoltCommitVerificationScripts = []queries.ScriptTest{
 		},
 		Assertions: []queries.ScriptTestAssertion{
 			{
+				// Verify that tests _can_ pass by cherry-picking the first commit, which adds Bob and updates the test to expect 2 users.
 				Query:    "CALL dolt_cherry_pick(@commit_1_hash)",
 				Expected: []sql.Row{{commitHash, int64(0), int64(0), int64(0)}},
 			},
 			{
-				// Verification fails; returns specific error, dirty state preserved
 				Query:          "CALL dolt_cherry_pick(@commit_2_hash)",
 				ExpectedErrStr: "commit verification failed: test_user_count_update (Assertion failed: expected_single_value equal to 2, got 3)",
 			},
 			{
-				// Abort restores clean state
+				// users table is staged (merged content with charlie added is preserved)
+				Query:    "SELECT staged FROM dolt_status WHERE table_name = 'users'",
+				Expected: []sql.Row{{uint64(1)}},
+			},
+			{
+				// --abort restores clean state, with Bob in the table (from first cherry-pick).
 				Query:            "CALL dolt_cherry_pick('--abort')",
 				SkipResultsCheck: true,
 			},
+			{
+				Query:    "SELECT COUNT(*) FROM users",
+				Expected: []sql.Row{{int64(2)}},
+			},
+			{},
 			{ // Test harness bleeds GLOBAL variable changes across tests, so reset after each test.
 				Query:            "SET GLOBAL dolt_commit_verification_groups = ''",
 				SkipResultsCheck: true,