Open
Conversation
Contributor
|
@macneale4 DOLT
|
…ationFailed Export runCommitVerification as RunCommitVerification so callers outside the package (cherry-pick, merge, rebase) can invoke verification explicitly before NewPendingCommit. Add ErrCommitVerificationFailed typed error using goerrors.NewKind so callers can use errors.Is() to detect verification failures vs other errors. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When commit verification fails during CherryPick(), instead of returning a bare error and losing all staged changes, explicitly run verification before NewPendingCommit(). On failure, call StartCherryPick() to set merge-in-progress state and save the working set, leaving staged changes intact so the user can fix the failing tests and --continue. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When ContinueCherryPick() calls NewPendingCommit() and verification fails, return ErrCommitVerificationFailed directly rather than wrapping it as "failed to create pending commit". The merge state is not cleared at this point, so the workspace remains dirty and the user can fix and --continue again. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… bug Add a 5th verification_failures column to dolt_cherry_pick result schema so verification failures can be surfaced without returning a SQL error (which would roll back working set changes and lose dirty state). Fix the StartCherryPick preMergeWorking bug: previously StartCherryPick was called after SetWorkingRoot, so --abort would restore to the cherry-pick result instead of the original state. Now we capture preApplyWs before any working set changes and use it to build the new working set with the correct pre-merge root. Update all existing cherry-pick test expectations to the 5-column schema. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When a cherry-pick step during rebase fails commit verification, commit
the current SQL transaction before returning an error. This preserves
the dirty working set (staged tables + active cherry-pick merge state)
so the user can fix failing tests and call dolt_rebase('--continue').
Defines ErrRebaseVerificationFailed for structured error messages.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Run commit verification explicitly in executeNoFFMerge() and performMerge() before creating the pending commit. On failure, return the error message through the result message field instead of as a SQL error, so the SQL transaction commits normally and the dirty merge state (staged tables + active merge state) is preserved. This lets users fix failing tests and call CALL dolt_commit() to complete the merge, mirroring the conflict-resolution workflow. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add engine tests covering the dirty-state workflow for all three operations when commit verification fails: Cherry-pick: tests pass, tests fail (dirty state preserved + --abort restores clean state), fix data then --continue succeeds. Rebase: tests pass, tests fail (--abort works, --skip-verification succeeds after abort). Merge: tests pass, tests fail (dirty state preserved), dolt_commit without fixing re-fails, fix data then dolt_commit succeeds. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- merge CLI (merge.go): add getMergeMessage() helper and check for "commit verification failed:" prefix in message column. Print the message and return exit code 1 when verification fails, after first committing the SQL transaction to preserve staged state. - rebase CLI (rebase.go): extend isRebaseConflictError() to also match ErrCommitVerificationFailed (typed) and "commit verification failed:" prefix (string match for over-the-wire). This ensures syncCliBranch- ToSqlSessionBranch() is called, switching the CLI to dolt_rebase_<b> so staged changes are visible after verification failure. - dolt_rebase.go: return verification failure error directly (not wrapped in ErrRebaseVerificationFailed) so engine tests can match the exact "commit verification failed: ..." string without including the non-deterministic commit hash. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Each test file gets two new bats tests:
- verification failure halts with dirty state preserved
- fix data then continue succeeds
Tests use SET @@PERSIST.dolt_commit_verification_groups = '*' (not SET
GLOBAL) to persist the setting across CLI invocations in non-server mode.
cherry-pick.bats: user fixes dolt_tests and calls dolt cherry-pick --continue
merge.bats: user fixes dolt_tests and calls dolt commit after merge fails
rebase.bats: user fixes dolt_tests and calls dolt rebase --continue;
after failure the CLI is on dolt_rebase_b1 (not b1), where
staged changes are visible in dolt status
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Remove verification failure tests from cherry-pick.bats, merge.bats, rebase.bats - Consolidate into commit_verification.bats with 6 new tests covering dirty-state-preserved and fix+continue workflows for cherry-pick, merge, rebase - Fix pre-existing tests that assumed clean-state after verification failure: add --abort before --skip-verification retries for cherry-pick and merge tests - Remove unskip and fix pre-existing rebase test (was skipped, now works correctly) - Remove invalid test_users_count assertion from cherry-pick CLI test (cherry-pick CLI uses ErrCherryPickVerificationFailed which does not include test names) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…trings Add CommitVerificationFailedPrefix = "commit verification failed:" to actions/commit.go and use it in ErrCommitVerificationFailed's format string. Replace the hardcoded string literals in merge.go and rebase.go with this constant, and add the actions import to merge.go. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Instead of running verification manually before NewPendingCommit and skipping it inside, let NewPendingCommit run verification exactly once via GetCommitStaged. When it returns ErrCommitVerificationFailed, catch it, set the merge state (same logic as before), and return nil Go error so CommitTransaction persists the dirty working set. This matches the pattern already used in ContinueCherryPick. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
macneale4
force-pushed
the
macneale4-claude/verify-workflow
branch
from
e34bb7b to
af85eb1
Compare
Previously, cherry-pick verification failure was reduced to verification_failures=1 in the stored proc result row and the CLI emitted a generic ErrCherryPickVerificationFailed message with no test details. Now: - doDoltCherryPick: when VerificationFailureErr is set, commit the transaction to persist dirty working set + merge state, then return the specific error (containing test name and failure details). - ContinueCherryPick: return ErrCommitVerificationFailed directly instead of (0,0,0,1,nil), since the merge state is already on disk from the previous call. - cherry-pick.go CLI: detect ErrCommitVerificationFailed in both the initial and --continue paths, print the specific error, then return ErrCherryPickVerificationFailed for the --continue/--abort guidance. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
@macneale4 DOLT
|
…or not row
The three test assertions that expected sql.Row{{"",0,0,0,1}} now use
ExpectedErrStr with the specific "commit verification failed: ..." message.
The dolt_status check between the initial failure and --continue remains,
confirming dirty state is preserved via the explicit CommitTransaction.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ions Replace generic "Commit verification failed" checks with specific assertions matching the actual ErrCommitVerificationFailed output: lowercase prefix, test name (test_count), failure details (expected_single_value equal to 1, got 2), and both --continue and --abort guidance. Applied to both the initial cherry-pick failure and the --continue-still-fails case in tests 11 and 12. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…error not row The "cherry-pick with test verification enabled - tests pass" test had a fourth verification failure assertion (commit_2_hash) that also returned verification_failures=1. Update it to ExpectedErrStr matching the specific ErrCommitVerificationFailed message. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ocedure result Verification failures are now surfaced as structured errors (ErrCommitVerificationFailed) rather than as a result column. Update ContinueCherryPick to return 4 values instead of 5, clean up the CLI's row-reading code, and update all engine tests to expect 4-column results. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
@macneale4 DOLT
|
…vate The pre-emptive verification call before NewPendingCommit in executeNoFFMerge duplicated the verification that already runs inside GetCommitStaged. Remove it and let NewPendingCommit surface ErrCommitVerificationFailed, handling it the same way the caller already did. Since RunCommitVerification and GetCommitRunTestGroups are now only called within the actions package, make them unexported. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
@macneale4 DOLT
|
Contributor
|
@macneale4 DOLT
|
Contributor
|
@macneale4 DOLT
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.