feat: add --onto and fork-point support to git_rebase (#159)

[MementoRC]

Closes #159

Summary

• Add onto, fork_point, branch optional params to GitRebase model
• Build rebase command dynamically: --onto new-base old-base [branch]
• Validate onto/fork_point must be provided together
• Skip branch existence check when --onto is used (user supplies exact refs)
• Add allow_empty flag to git_commit for empty commits (CI triggers, merge markers)
• Include cherry-picked git_rm docstring improvement from feat/git-rm-safe
• 20 unit tests covering all parameter combinations and error paths

Test plan

• GitRebase model tests: 4 tests verify new fields accept values and default to None
• git_rebase operation tests: 8 tests cover simple rebase, --onto, --onto+branch, branch-only, validation errors, conflict/error handling
• git_commit allow_empty tests: 8 tests cover model default, flag present/absent, position, combined with amend
• Integration test with real git repo (manual)

🤖 Generated with Claude Code

[github-actions]

Claude finished @MementoRC's task —— View job

---

Pull Request Review - feat: add --onto and fork-point support to git_rebase

I've completed a thorough review of this PR. Here's my comprehensive analysis:

Overall Assessment ✅

This is a well-implemented feature that adds valuable --onto and fork-point support to git rebase operations. The code demonstrates good software engineering practices with proper validation, comprehensive testing, and clear documentation.

Code Quality & Architecture

Strengths:

• Clean model extension (models.py:132-134): New optional fields are well-defined with proper descriptions
• Logical command building (operations.py:1485-1493): The argument construction correctly handles different rebase scenarios
• Proper validation (operations.py:1461-1465): Validates that onto and fork_point must be provided together
• Good error handling (operations.py:1502-1508): Distinguishes between conflict and other error types
• Backward compatibility: All existing rebase functionality remains unchanged

Minor Issues:

• The validation logic is sound but could benefit from more descriptive error messages
• Consider adding parameter validation for git ref format (though current validation is adequate)

Security Analysis ✅

No security concerns identified:

• All parameters are properly passed to GitPython without shell injection risks
• The --onto functionality doesn't introduce new attack vectors
• Branch existence checks are appropriately skipped for --onto mode (as intended)
• Input validation follows established patterns in the codebase

Test Coverage Analysis ✅

Excellent test coverage (test_rebase.py):

• Model tests (4 tests): Verify new fields accept values and default to None
• Operation tests (8+ tests): Cover all parameter combinations:
  • Simple rebase (existing behavior)
  • --onto with fork_point
  • --onto with branch parameter
  • Branch-only rebase
  • Validation errors (missing fork_point/onto)
  • Conflict handling
  • Error handling

Test quality is high:

• Good use of mocking for git operations
• Clear test names and documentation
• Proper assertion patterns
• Edge cases covered

Performance Considerations ✅

No performance concerns:

• Branch existence validation is appropriately skipped for --onto mode
• Command building is efficient with minimal string operations
• No unnecessary git calls or redundant operations

Specific Code Review

models.py:132-134

onto: str | None = Field(default=None, description="Rebase --onto target (new base)")
fork_point: str | None = Field(default=None, description="Fork point / old base for --onto")
branch: str | None = Field(default=None, description="Branch to rebase (default: current HEAD)")

✅ Good: Clear field definitions with helpful descriptions

operations.py:1461-1465 (Validation Logic)

if onto and not fork_point:
    return "❌ --onto requires fork_point (the old base to rebase from)"
if fork_point and not onto:
    return "❌ fork_point requires --onto (the new base to rebase onto)"

✅ Good: Proper validation ensures parameters are used correctly

operations.py:1485-1493 (Command Building)

if onto:
    args = ["--onto", onto, fork_point]
    if branch:
        args.append(branch)
elif branch:
    args = [target_branch, branch]
else:
    args = [target_branch]

✅ Good: Clean logic that correctly constructs git command arguments

Recommendations

1. Consider adding ref format validation (optional enhancement):

   # Could validate that refs don't contain dangerous characters
   if any(char in onto for char in [';', '&', '|']) if onto else False:
       return "❌ Invalid characters in onto parameter"

2. Documentation enhancement (minor): The registry description could be more specific about the --onto syntax

Cherry-picked git_rm Changes ✅

The inclusion of git_rm improvements (test_git_rm.py:29-33) is appropriate and adds value to this PR without scope creep.

Final Assessment

This PR represents high-quality work that:

• ✅ Implements the requested feature correctly
• ✅ Maintains backward compatibility
• ✅ Includes comprehensive tests
• ✅ Follows established code patterns
• ✅ Has no security vulnerabilities
• ✅ Is ready for production use

Recommendation: APPROVE 🚀

The implementation is solid, well-tested, and adds valuable functionality to the MCP Git server.

---

[github-actions]

Claude finished @MementoRC's task —— View job

---

Pull Request Review - feat: add --onto and fork-point support to git_rebase

I've completed a comprehensive review of this PR. Here's my analysis:

Review Checklist:

• Read and understand the changed files
• Analyze code quality and architecture
• Check for potential bugs or issues
• Evaluate performance considerations
• Assess security concerns
• Review test coverage adequacy
• Provide constructive feedback and recommendations

Overall Assessment ✅

This is a well-implemented feature that adds valuable --onto and fork-point support to git rebase operations. The code demonstrates good software engineering practices with proper validation, comprehensive testing, and clear documentation.

Code Quality & Architecture

Strengths:

• Clean model extension (models.py:136-138): New optional fields are well-defined with proper descriptions
• Logical command building (operations.py:1489-1496): The argument construction correctly handles different rebase scenarios
• Proper validation (operations.py:1465-1468): Validates that onto and fork_point must be provided together
• Good error handling (operations.py:1506-1511): Distinguishes between conflict and other error types
• Backward compatibility: All existing rebase functionality remains unchanged

Minor Issues:

• The validation logic is sound but could benefit from more descriptive error messages
• Consider adding parameter validation for git ref format (though current validation is adequate)

Security Analysis ✅

No security concerns identified:

• All parameters are properly passed to GitPython without shell injection risks
• The --onto functionality doesn't introduce new attack vectors
• Branch existence checks are appropriately skipped for --onto mode (as intended)
• Input validation follows established patterns in the codebase

Test Coverage Analysis ✅

Excellent test coverage (test_rebase.py):

• Model tests (4 tests): Verify new fields accept values and default to None
• Operation tests (8+ tests): Cover all parameter combinations:
  • Simple rebase (existing behavior)
  • --onto with fork_point
  • --onto with branch parameter
  • Branch-only rebase
  • Validation errors (missing fork_point/onto)
  • Conflict handling
  • Error handling

Test quality is high:

• Good use of mocking for git operations
• Clear test names and documentation
• Proper assertion patterns
• Edge cases covered

Performance Considerations ✅

No performance concerns:

• Branch existence validation is appropriately skipped for --onto mode
• Command building is efficient with minimal string operations
• No unnecessary git calls or redundant operations

Specific Code Review

models.py:136-138

onto: str | None = Field(default=None, description="Rebase --onto target (new base)")
fork_point: str | None = Field(default=None, description="Fork point / old base for --onto")
branch: str | None = Field(default=None, description="Branch to rebase (default: current HEAD)")

✅ Good: Clear field definitions with helpful descriptions

operations.py:1465-1468 (Validation Logic)

if onto and not fork_point:
    return "❌ --onto requires fork_point (the old base to rebase from)"
if fork_point and not onto:
    return "❌ fork_point requires --onto (the new base to rebase onto)"

✅ Good: Proper validation ensures parameters are used correctly

operations.py:1489-1496 (Command Building)

if onto:
    args = ["--onto", onto, fork_point]
    if branch:
        args.append(branch)
elif branch:
    args = [target_branch, branch]
else:
    args = [target_branch]

✅ Good: Clean logic that correctly constructs git command arguments

Additional Features Review

GitCommit allow_empty Flag ✅

The addition of the allow_empty flag to GitCommit is well-implemented:

• Model properly defaults to False (models.py:48-51)
• Command construction correctly adds --allow-empty when needed (operations.py:469)
• Comprehensive test coverage verifies flag positioning and behavior
• Use case is clear for CI triggers and empty merge commits

Cherry-picked git_rm Improvements ✅

The inclusion of git_rm improvements (test_git_rm.py:29-33) adds value without scope creep:

• Enhanced docstrings with examples
• Additional safety validation tests
• Improved error handling patterns

Registry Description Update ✅

The updated description in registry_git.py:195 properly documents the new --onto functionality:

description="Rebase branch onto another. Supports --onto <new_base> <fork_point> [<branch>] for moving branches between bases"

Recommendations

1. Consider adding ref format validation (optional enhancement):

   # Could validate that refs don't contain dangerous characters
   if any(char in onto for char in [';', '&', '|']) if onto else False:
       return "❌ Invalid characters in onto parameter"

2. Documentation enhancement (minor): The examples in docstrings are helpful and accurate

Final Assessment

This PR represents high-quality work that:

• ✅ Implements the requested feature correctly
• ✅ Maintains backward compatibility
• ✅ Includes comprehensive tests
• ✅ Follows established code patterns
• ✅ Has no security vulnerabilities
• ✅ Is ready for production use

Recommendation: APPROVE 🚀

The implementation is solid, well-tested, and adds valuable functionality to the MCP Git server.

[github-actions]

Claude finished @MementoRC's task —— View job

---

Pull Request Review - feat: add --onto and fork-point support to git_rebase

I've completed a comprehensive review of this PR. Here's my analysis:

Review Checklist:

• Read and understand the changed files
• Analyze code quality and architecture
• Check for potential bugs or issues
• Evaluate performance considerations
• Assess security concerns
• Review test coverage adequacy
• Provide constructive feedback and recommendations

Overall Assessment ✅

This is a well-implemented feature that adds valuable --onto and fork-point support to git rebase operations. The code demonstrates good software engineering practices with proper validation, comprehensive testing, and clear documentation.

Code Quality & Architecture

Strengths:

• Clean model extension (models.py:136-138): New optional fields are well-defined with proper descriptions
• Logical command building (operations.py:1489-1496): The argument construction correctly handles different rebase scenarios
• Proper validation (operations.py:1465-1468): Validates that onto and fork_point must be provided together
• Good error handling (operations.py:1506-1511): Distinguishes between conflict and other error types
• Backward compatibility: All existing rebase functionality remains unchanged

Minor Issues:

• The validation logic is sound but could benefit from more descriptive error messages
• Consider adding parameter validation for git ref format (though current validation is adequate)

Security Analysis ✅

No security concerns identified:

• All parameters are properly passed to GitPython without shell injection risks
• The --onto functionality doesn't introduce new attack vectors
• Branch existence checks are appropriately skipped for --onto mode (as intended)
• Input validation follows established patterns in the codebase

Test Coverage Analysis ✅

Excellent test coverage (test_rebase.py):

• Model tests (4 tests): Verify new fields accept values and default to None
• Operation tests (8+ tests): Cover all parameter combinations:
  • Simple rebase (existing behavior)
  • --onto with fork_point
  • --onto with branch parameter
  • Branch-only rebase
  • Validation errors (missing fork_point/onto)
  • Conflict handling
  • Error handling

Test quality is high:

• Good use of mocking for git operations
• Clear test names and documentation
• Proper assertion patterns
• Edge cases covered

Performance Considerations ✅

No performance concerns:

• Branch existence validation is appropriately skipped for --onto mode
• Command building is efficient with minimal string operations
• No unnecessary git calls or redundant operations

Specific Code Review

models.py:136-138

onto: str | None = Field(default=None, description="Rebase --onto target (new base)")
fork_point: str | None = Field(default=None, description="Fork point / old base for --onto")
branch: str | None = Field(default=None, description="Branch to rebase (default: current HEAD)")

✅ Good: Clear field definitions with helpful descriptions

operations.py:1465-1468 (Validation Logic)

if onto and not fork_point:
    return "❌ --onto requires fork_point (the old base to rebase from)"
if fork_point and not onto:
    return "❌ fork_point requires --onto (the new base to rebase onto)"

✅ Good: Proper validation ensures parameters are used correctly

operations.py:1489-1496 (Command Building)

if onto:
    args = ["--onto", onto, fork_point]
    if branch:
        args.append(branch)
elif branch:
    args = [target_branch, branch]
else:
    args = [target_branch]

✅ Good: Clean logic that correctly constructs git command arguments

Additional Features Review

GitCommit allow_empty Flag ✅

The addition of the allow_empty flag to GitCommit is well-implemented:

• Model properly defaults to False (models.py:48-51)
• Command construction correctly adds --allow-empty when needed (operations.py:469)
• Comprehensive test coverage verifies flag positioning and behavior
• Use case is clear for CI triggers and empty merge commits

Cherry-picked git_rm Improvements ✅

The inclusion of git_rm improvements (test_git_rm.py:29-33) adds value without scope creep:

• Enhanced docstrings with examples
• Additional safety validation tests
• Improved error handling patterns

Registry Description Update ✅

The updated description in registry_git.py:195 properly documents the new --onto functionality:

description="Rebase branch onto another. Supports --onto <new_base> <fork_point> [<branch>] for moving branches between bases"

Recommendations

1. Consider adding ref format validation (optional enhancement):

   # Could validate that refs don't contain dangerous characters
   if any(char in onto for char in [';', '&', '|']) if onto else False:
       return "❌ Invalid characters in onto parameter"

2. Documentation enhancement (minor): The examples in docstrings are helpful and accurate

Final Assessment

This PR represents high-quality work that:

• ✅ Implements the requested feature correctly
• ✅ Maintains backward compatibility
• ✅ Includes comprehensive tests
• ✅ Follows established code patterns
• ✅ Has no security vulnerabilities
• ✅ Is ready for production use

Recommendation: APPROVE 🚀

The implementation is solid, well-tested, and adds valuable functionality to the MCP Git server.