feat: add git tools and fix issue bug (#151-#155)

[MementoRC]

Summary

• Fix github_create_issue JSON parsing for long body text with markdown (github_create_issue fails with long body text (JSON parsing error) #151)
• Add git_restore tool for file restoration and unstaging (Add git_restore tool #152)
• Add git_branch_update tool for force-update refs and branch deletion (Add git_branch_update tool (force-update and delete refs) #153)
• Add git_worktree_list and git_worktree_remove tools (Add git_worktree_list and git_worktree_remove tools #154)
• Add git_merge_tree tool for dry-run conflict detection (Add git_merge_tree tool for dry-run conflict detection #155)

Architecture

New tools in git/operations_extended.py to avoid bloating the 2284-line operations.py. Follows established 3-layer pattern: Pydantic model, operation function, lean registry entry.

Test plan

• 80+ new unit tests across 6 test files
• All existing tests pass (full suite green)
• Lint and format checks pass
• Security validation (dangerous char injection) for all new tools

[github-actions]

Claude finished @MementoRC's task —— View job

---

PR Review Complete ✅

This is an excellent PR that demonstrates careful planning, robust implementation, and comprehensive testing. Here's my detailed review:

• Read repository guidelines (no CLAUDE.md found)
• Review architectural decisions and new operations_extended.py
• Examine each new git tool implementation
• Check JSON parsing fix for issue github_create_issue fails with long body text (JSON parsing error) #151
• Review test coverage and quality
• Assess security considerations
• Check for potential bugs and performance issues
• Provide comprehensive feedback

✅ Code Quality & Architecture

Excellent separation of concerns:

• Smart decision to create operations_extended.py to avoid bloating the 2284-line operations.py
• Consistent 3-layer pattern: Pydantic model → operation function → lean registry entry
• Clean imports and module organization

Code consistency:

• All new tools follow the same error handling patterns as existing code
• Consistent emoji usage (✅ success, ❌ error, ⚠️ warnings)
• Proper docstrings with clear parameter descriptions

🔒 Security - Excellent

Robust input validation:

• Consistent DANGEROUS_CHARS = re.compile(r"[;&|$()]")` pattern across all tools
• Dedicated _validate_ref() function prevents command injection in git refs
• Path validation for worktree operations at operations_extended.py:153
• All dangerous characters properly tested in unit tests

Security best practices:

• No hardcoded paths or credentials
• Proper stderr/stdout handling with bytes/string coercion
• Input sanitization before git command execution

🐛 Bug Analysis - Clean Implementation

Robust error handling:

• All tools handle GitCommandError with both bytes and string stderr
• Graceful degradation with descriptive error messages
• Proper exception catching hierarchy (specific → general)

Edge cases covered:

• Empty file lists in git_restore (operations_extended.py:38-39)
• Conflicting parameters in git_branch_update (operations_extended.py:79-82)
• Empty worktree output handled gracefully (operations_extended.py:110-111)
• Detached HEAD state in worktree listing (operations_extended.py:126-127)

JSON parsing fix for #151:

• Excellent solution in meta_tools.py:342-361 with fallback sanitization
• Handles literal newlines, tabs, and control characters in markdown content
• Comprehensive escape coverage including C0 controls (meta_tools.py:51-52)

⚡ Performance Considerations

Efficient implementations:

• Proper use of git porcelain format for worktree listing
• Short SHA display (8 chars) for better readability (test_git_worktree.py:95-102)
• Smart file list truncation for large operations (operations_extended.py:57-58)
• No unnecessary git operations or redundant calls

🧪 Test Coverage - Exceptional Quality

80+ comprehensive unit tests across 6 files:

test_git_restore.py (176 lines):

• ✅ Success scenarios with different flag combinations
• ✅ Input validation with parametrized dangerous character testing
• ✅ Error handling for GitCommandError and general exceptions
• ✅ Edge cases like empty file lists and file count display logic

test_git_branch_update.py (165 lines):

• ✅ Force-update and delete operations
• ✅ Conflicting parameter validation
• ✅ Comprehensive dangerous character testing for both branch names and targets
• ✅ Error handling with proper stderr coercion

test_git_worktree.py (242 lines):

• ✅ Porcelain output parsing with multiple complex scenarios
• ✅ Detached HEAD and branch reference handling
• ✅ Force removal flag testing
• ✅ Path validation and error handling

test_git_merge_tree.py (146 lines):

• ✅ Clean merge scenarios with tree hash output
• ✅ Conflict detection with proper warning messages
• ✅ Multiple conflict file parsing
• ✅ Input validation and error handling

test_meta_tools_json_parsing.py (240 lines):

• ✅ Direct testing of _sanitize_json_string helper
• ✅ Comprehensive control character escape testing
• ✅ Real-world markdown scenarios with code fences
• ✅ Fallback coercion path testing

Test quality highlights:

• Parametrized testing for security validation
• Mock usage follows best practices
• Clear test naming and organization
• Edge case coverage (empty outputs, malformed input)
• Error message validation

🚀 Best Practices & Maintainability

Follows established patterns:

• Registry integration in registry_git.py with proper imports
• Pydantic model definitions in models.py with clear field descriptions
• Wrapper functions for repo path → Repo object conversion

Documentation quality:

• Clear function docstrings with parameter explanations
• Git version requirements noted (merge-tree requires Git 2.38+)
• Comprehensive test documentation explaining scenarios

Code organization:

• Logical grouping of related operations
• Clean separation between core operations and extended tools
• Proper module exports with __all__

💡 Minor Suggestions

1. Line 121 in operations_extended.py: The 8-character SHA truncation could be configurable, though current implementation is sensible for display purposes.

2. Error message consistency: All tools properly include the problematic input in validation errors, which aids debugging.

🎯 Overall Assessment

This PR exemplifies excellent software engineering practices:

• Architecture: Smart modularization prevents code bloat
• Security: Comprehensive input validation prevents command injection
• Testing: 80+ tests with excellent coverage of edge cases and error scenarios
• Documentation: Clear, comprehensive documentation throughout
• Performance: Efficient implementations with proper git command usage
• Maintainability: Follows existing patterns and conventions

The JSON parsing fix for issue #151 is particularly well-executed, handling the complex edge case of markdown content with control characters through a robust sanitization approach.

Recommendation: ✅ APPROVE - This PR is ready for merge. It adds valuable functionality while maintaining the high code quality standards of the project.

---