From 31b03b3eb8cf8c2d498616d938e98ab74dcc4247 Mon Sep 17 00:00:00 2001 From: "Edward Z. Yang" Date: Fri, 2 May 2025 20:29:39 -0400 Subject: [PATCH] Update [ghstack-poisoned] --- codemcp/code_command.py | 2 +- codemcp/main.py | 39 ++++++++ codemcp/tools/__init__.py | 2 + codemcp/tools/init_project.py | 22 ++++- codemcp/tools/mv.py | 135 +++++++++++++++++++++++++ e2e/test_mv.py | 181 ++++++++++++++++++++++++++++++++++ 6 files changed, 378 insertions(+), 3 deletions(-) create mode 100644 codemcp/tools/mv.py create mode 100644 e2e/test_mv.py diff --git a/codemcp/code_command.py b/codemcp/code_command.py index 40778e10..88e5b1eb 100644 --- a/codemcp/code_command.py +++ b/codemcp/code_command.py @@ -239,7 +239,7 @@ async def run_formatter_without_commit(file_path: str) -> Tuple[bool, str]: return False, "No format command configured in codemcp.toml" # Use relative path from project_dir for the formatting command - rel_path = os.path.relpath(file_path, project_dir) + os.path.relpath(file_path, project_dir) # Run the formatter with the specific file path command = format_command diff --git a/codemcp/main.py b/codemcp/main.py index 2771e65f..a249842d 100644 --- a/codemcp/main.py +++ b/codemcp/main.py @@ -22,6 +22,7 @@ from .tools.grep import grep_files from .tools.init_project import init_project from .tools.ls import ls_directory +from .tools.mv import mv_file from .tools.read_file import read_file_content from .tools.rm import rm_file from .tools.run_command import run_command @@ -89,6 +90,8 @@ async def codemcp( | None = None, # Whether to reuse the chat ID from the HEAD commit thought: str | None = None, # Added for Think tool mode: str | None = None, # Added for Chmod tool + source_path: str | None = None, # Added for MV tool + target_path: str | None = None, # Added for MV tool commit_hash: str | None = None, # Added for Git commit hash tracking ) -> str: # NOTE: Do NOT add more documentation to this docblock when you add a new @@ -142,6 +145,13 @@ async def codemcp( "Grep": {"pattern", "path", "include", "chat_id", "commit_hash"}, "Glob": {"pattern", "path", "limit", "offset", "chat_id", "commit_hash"}, "RM": {"path", "description", "chat_id", "commit_hash"}, + "MV": { + "source_path", + "target_path", + "description", + "chat_id", + "commit_hash", + }, "Think": {"thought", "chat_id", "commit_hash"}, "Chmod": {"path", "mode", "chat_id", "commit_hash"}, } @@ -198,6 +208,9 @@ def normalize_newlines(s: object) -> object: "thought": thought, # Chmod tool parameter "mode": mode, + # MV tool parameters + "source_path": source_path, + "target_path": target_path, # Git commit hash tracking "commit_hash": commit_hash, }.items() @@ -427,6 +440,32 @@ def normalize_newlines(s: object) -> object: result, new_commit_hash = await append_commit_hash(result, normalized_path) return result + if subtool == "MV": + # Extract parameters specific to MV + source_path = provided_params.get("source_path") + target_path = provided_params.get("target_path") + + if source_path is None: + raise ValueError("source_path is required for MV subtool") + if target_path is None: + raise ValueError("target_path is required for MV subtool") + if description is None: + raise ValueError("description is required for MV subtool") + + # Normalize the paths (expand tilde) before proceeding + normalized_source_path = normalize_file_path(source_path) + normalized_target_path = normalize_file_path(target_path) + + if chat_id is None: + raise ValueError("chat_id is required for MV subtool") + result = await mv_file( + normalized_source_path, normalized_target_path, description, chat_id + ) + result, new_commit_hash = await append_commit_hash( + result, normalized_source_path + ) + return result + if subtool == "Think": if thought is None: raise ValueError("thought is required for Think subtool") diff --git a/codemcp/tools/__init__.py b/codemcp/tools/__init__.py index 202e8e2c..26eb4399 100644 --- a/codemcp/tools/__init__.py +++ b/codemcp/tools/__init__.py @@ -6,6 +6,7 @@ from .git_diff import git_diff from .git_log import git_log from .git_show import git_show +from .mv import mv_file from .rm import rm_file __all__ = [ @@ -14,5 +15,6 @@ "git_diff", "git_log", "git_show", + "mv_file", "rm_file", ] diff --git a/codemcp/tools/init_project.py b/codemcp/tools/init_project.py index 04b00051..84f0b34e 100644 --- a/codemcp/tools/init_project.py +++ b/codemcp/tools/init_project.py @@ -449,6 +449,22 @@ async def init_project( description: Short description of why the file is being removed chat_id: The unique ID to identify the chat session +## MV chat_id source_path target_path description + +Moves a file using git mv and commits the change. +Provide a short description of why the file is being moved. + +Before using this tool: +1. Ensure the source file exists and is tracked by git +2. Ensure the target directory exists within the git repository +3. Provide a meaningful description of why the file is being moved + +Args: + source_path: The path to the file to move (can be relative to the project root or absolute) + target_path: The destination path where the file should be moved to (can be relative to the project root or absolute) + description: Short description of why the file is being moved + chat_id: The unique ID to identify the chat session + ## Chmod chat_id path mode Changes file permissions using chmod. Unlike standard chmod, this tool only supports @@ -467,14 +483,16 @@ async def init_project( ## Summary Args: - subtool: The subtool to execute (ReadFile, WriteFile, EditFile, LS, InitProject, UserPrompt, RunCommand, RM, Think, Chmod) + subtool: The subtool to execute (ReadFile, WriteFile, EditFile, LS, InitProject, UserPrompt, RunCommand, RM, MV, Think, Chmod) path: The path to the file or directory to operate on content: Content for WriteFile subtool (any type will be serialized to string if needed) old_string: String to replace for EditFile subtool new_string: Replacement string for EditFile subtool offset: Line offset for ReadFile subtool limit: Line limit for ReadFile subtool - description: Short description of the change (for WriteFile/EditFile/RM) + description: Short description of the change (for WriteFile/EditFile/RM/MV) + source_path: The path to the source file for MV subtool + target_path: The destination path for MV subtool arguments: A string containing space-separated arguments for RunCommand subtool user_prompt: The user's verbatim text (for UserPrompt subtool) thought: The thought content (for Think subtool) diff --git a/codemcp/tools/mv.py b/codemcp/tools/mv.py new file mode 100644 index 00000000..bcffd750 --- /dev/null +++ b/codemcp/tools/mv.py @@ -0,0 +1,135 @@ +#!/usr/bin/env python3 + +import logging +import os +import pathlib + +from ..common import normalize_file_path +from ..git import commit_changes, get_repository_root +from ..shell import run_command + +__all__ = [ + "mv_file", +] + + +async def mv_file( + source_path: str, + target_path: str, + description: str, + chat_id: str = "", +) -> str: + """Move a file using git mv. + + Args: + source_path: The path to the source file to move (can be absolute or relative to repository root) + target_path: The path to the target location (can be absolute or relative to repository root) + description: Short description of why the file is being moved + chat_id: The unique ID of the current chat session + + Returns: + A string containing the result of the move operation + """ + # Use the directory from the path as our starting point for source + source_path = normalize_file_path(source_path) + source_dir_path = ( + os.path.dirname(source_path) if os.path.dirname(source_path) else "." + ) + + # Normalize target path as well + target_path = normalize_file_path(target_path) + + # Validations for source file + if not os.path.exists(source_path): + raise FileNotFoundError(f"Source file does not exist: {source_path}") + + if not os.path.isfile(source_path): + raise ValueError(f"Source path is not a file: {source_path}") + + # Get git repository root + git_root = await get_repository_root(source_dir_path) + # Ensure paths are absolute and resolve any symlinks + source_path_resolved = os.path.realpath(source_path) + git_root_resolved = os.path.realpath(git_root) + target_path_resolved = ( + os.path.realpath(target_path) + if os.path.exists(os.path.dirname(target_path)) + else target_path + ) + + # Use pathlib to check if the source file is within the git repo + # This handles path traversal correctly on all platforms + try: + # Convert to Path objects + source_path_obj = pathlib.Path(source_path_resolved) + git_root_obj = pathlib.Path(git_root_resolved) + + # Check if file is inside the git repo using Path.relative_to + # This will raise ValueError if source_path is not inside git_root + source_path_obj.relative_to(git_root_obj) + except ValueError: + msg = ( + f"Source path {source_path} is not within the git repository at {git_root}" + ) + logging.error(msg) + raise ValueError(msg) + + # Check if target directory exists and is within the git repo + target_dir = os.path.dirname(target_path) + if target_dir and not os.path.exists(target_dir): + raise FileNotFoundError(f"Target directory does not exist: {target_dir}") + + try: + # Convert to Path objects + target_dir_obj = pathlib.Path( + os.path.realpath(target_dir) if target_dir else git_root_resolved + ) + # Check if target directory is inside the git repo + target_dir_obj.relative_to(git_root_obj) + except ValueError: + msg = f"Target directory {target_dir} is not within the git repository at {git_root}" + logging.error(msg) + raise ValueError(msg) + + # Get the relative paths using pathlib + source_rel_path = os.path.relpath(source_path_resolved, git_root_resolved) + target_rel_path = os.path.relpath( + target_path_resolved + if os.path.exists(os.path.dirname(target_path)) + else os.path.join(git_root_resolved, os.path.basename(target_path)), + git_root_resolved, + ) + + logging.info(f"Using relative paths: {source_rel_path} -> {target_rel_path}") + + # Check if the source file is tracked by git from the git root + await run_command( + ["git", "ls-files", "--error-unmatch", source_rel_path], + cwd=git_root_resolved, + check=True, + capture_output=True, + text=True, + ) + + # If we get here, the file is tracked by git, so we can move it + await run_command( + ["git", "mv", source_rel_path, target_rel_path], + cwd=git_root_resolved, + check=True, + capture_output=True, + text=True, + ) + + # Commit the changes + logging.info(f"Committing move of file: {source_rel_path} -> {target_rel_path}") + success, commit_message = await commit_changes( + git_root_resolved, + f"Move {source_rel_path} -> {target_rel_path}: {description}", + chat_id, + commit_all=False, # No need for commit_all since git mv already stages the change + ) + + if success: + return f"Successfully moved file from {source_rel_path} to {target_rel_path}." + else: + return f"File was moved from {source_rel_path} to {target_rel_path} but failed to commit: {commit_message}" diff --git a/e2e/test_mv.py b/e2e/test_mv.py new file mode 100644 index 00000000..fff02d23 --- /dev/null +++ b/e2e/test_mv.py @@ -0,0 +1,181 @@ +#!/usr/bin/env python3 + +"""End-to-end tests for the mv tool.""" + +import os +import unittest + +from codemcp.testing import MCPEndToEndTestCase + + +class MVTest(MCPEndToEndTestCase): + """Test the MV subtool functionality.""" + + async def test_mv_file(self): + """Test moving a file using the MV subtool.""" + # Create a test file + source_file_path = os.path.join(self.temp_dir.name, "file_to_move.txt") + with open(source_file_path, "w") as f: + f.write("This file will be moved") + + # Create target directory + target_dir = os.path.join(self.temp_dir.name, "target_dir") + os.makedirs(target_dir, exist_ok=True) + target_file_path = os.path.join(target_dir, "moved_file.txt") + + # Add the file using git + await self.git_run(["add", "file_to_move.txt"]) + await self.git_run(["commit", "-m", "Add file that will be moved"]) + + # Initial count of commits + initial_log = await self.git_run( + ["log", "--oneline"], capture_output=True, text=True + ) + initial_commit_count = len(initial_log.strip().split("\n")) + + async with self.create_client_session() as session: + # Get a valid chat_id + chat_id = await self.get_chat_id(session) + + # For debugging, print some path information + print(f"DEBUG - Source file path: {source_file_path}") + print(f"DEBUG - Target file path: {target_file_path}") + # Check if files exist + print( + f"DEBUG - Source file exists before MV: {os.path.exists(source_file_path)}" + ) + print( + f"DEBUG - Target file exists before MV: {os.path.exists(target_file_path)}" + ) + + # Call the MV tool with the chat_id - use absolute paths + result = await self.call_tool_assert_success( + session, + "codemcp", + { + "subtool": "MV", + "source_path": source_file_path, + "target_path": target_file_path, + "description": "Test file movement", + "chat_id": chat_id, + }, + ) + + # Print the result for debugging + print(f"DEBUG - MV result: {result}") + + # Check that the source file no longer exists + print( + f"DEBUG - Source file exists after MV: {os.path.exists(source_file_path)}" + ) + self.assertFalse( + os.path.exists(source_file_path), "Source file should have been moved" + ) + + # Check that the target file now exists + print( + f"DEBUG - Target file exists after MV: {os.path.exists(target_file_path)}" + ) + self.assertTrue( + os.path.exists(target_file_path), "Target file should now exist" + ) + + # Verify the target file has the same content + with open(target_file_path, "r") as f: + content = f.read() + self.assertEqual(content, "This file will be moved") + + # Verify the output message indicates success + self.assertIn("Successfully moved file", result) + + # Verify a commit was created for the movement + final_log = await self.git_run( + ["log", "--oneline"], capture_output=True, text=True + ) + final_commit_count = len(final_log.strip().split("\n")) + self.assertEqual( + final_commit_count, + initial_commit_count + 1, + "Should have one more commit", + ) + + # Verify the commit message contains the description + latest_commit_msg = await self.git_run( + ["log", "-1", "--pretty=%B"], capture_output=True, text=True + ) + self.assertIn( + "Move file_to_move.txt -> target_dir/moved_file.txt", latest_commit_msg + ) + self.assertIn("Test file movement", latest_commit_msg) + + async def test_mv_file_source_does_not_exist(self): + """Test attempting to move a non-existent source file.""" + # Create target directory + target_dir = os.path.join(self.temp_dir.name, "target_dir") + os.makedirs(target_dir, exist_ok=True) + target_file_path = os.path.join(target_dir, "moved_file.txt") + + async with self.create_client_session() as session: + # Get a valid chat_id + chat_id = await self.get_chat_id(session) + + # Attempt to move a file that doesn't exist - should fail + result = await self.call_tool_assert_error( + session, + "codemcp", + { + "subtool": "MV", + "source_path": "non_existent_file.txt", + "target_path": target_file_path, + "description": "Move non-existent file", + "chat_id": chat_id, + }, + ) + + # Verify the operation failed with proper error message + self.assertIn("Source file does not exist", result) + + async def test_mv_outside_repo(self): + """Test attempting to move a file outside the repository.""" + # Create a file inside the repository + source_file_path = os.path.join(self.temp_dir.name, "inside_repo_file.txt") + with open(source_file_path, "w") as f: + f.write("This file is inside the repository") + + # Add the file using git + await self.git_run(["add", "inside_repo_file.txt"]) + await self.git_run(["commit", "-m", "Add file inside repo"]) + + # Create a directory outside the repository + outside_dir = os.path.join(os.path.dirname(self.temp_dir.name), "outside_repo") + os.makedirs(outside_dir, exist_ok=True) + outside_file = os.path.join(outside_dir, "outside_file.txt") + + async with self.create_client_session() as session: + # Get a valid chat_id + chat_id = await self.get_chat_id(session) + + # Attempt to move the file to a location outside the repo - should fail + result = await self.call_tool_assert_error( + session, + "codemcp", + { + "subtool": "MV", + "source_path": source_file_path, + "target_path": outside_file, + "description": "Move file outside repo", + "chat_id": chat_id, + }, + ) + + # Verify the operation failed with proper error message + self.assertIn("not within the git repository", result) + + # Ensure the source file still exists + self.assertTrue( + os.path.exists(source_file_path), "Source file should still exist" + ) + + +if __name__ == "__main__": + unittest.main()