Merge branch 'remove-file-hash-from-error' into develop

Author: tumf

src/mcp_text_editor/__init__.py

@@ -29,7 +29,7 @@ async def get_text_file_contents(
 async def insert_text_file_contents(request: Dict[str, Any]) -> Dict[str, Any]:
     """Insert text content before or after a specific line in a file."""
     return await _text_editor.insert_text_file_contents(
-        file_path=request["path"],
+        file_path=request["file_path"],
         file_hash=request["file_hash"],
         after=request.get("after"),
         before=request.get("before"),

src/mcp_text_editor/handlers/append_text_file_contents.py

@@ -52,14 +52,14 @@ def get_tool_description(self) -> Tool:
     async def run_tool(self, arguments: Dict[str, Any]) -> Sequence[TextContent]:
         """Execute the tool with given arguments."""
         try:
-            if "path" not in arguments:
-                raise RuntimeError("Missing required argument: path")
+            if "file_path" not in arguments:
+                raise RuntimeError("Missing required argument: file_path")
             if "contents" not in arguments:
                 raise RuntimeError("Missing required argument: contents")
             if "file_hash" not in arguments:
                 raise RuntimeError("Missing required argument: file_hash")
 
-            file_path = arguments["path"]
+            file_path = arguments["file_path"]
             if not os.path.isabs(file_path):
                 raise RuntimeError(f"File path must be absolute: {file_path}")
 

src/mcp_text_editor/handlers/create_text_file.py

@@ -29,7 +29,7 @@ def get_tool_description(self) -> Tool:
             inputSchema={
                 "type": "object",
                 "properties": {
-                    "path": {
+                    "file_path": {
                         "type": "string",
                         "description": "Path to the text file. File path must be absolute.",
                     },
@@ -50,12 +50,12 @@ def get_tool_description(self) -> Tool:
     async def run_tool(self, arguments: Dict[str, Any]) -> Sequence[TextContent]:
         """Execute the tool with given arguments."""
         try:
-            if "path" not in arguments:
-                raise RuntimeError("Missing required argument: path")
+            if "file_path" not in arguments:
+                raise RuntimeError("Missing required argument: file_path")
             if "contents" not in arguments:
                 raise RuntimeError("Missing required argument: contents")
 
-            file_path = arguments["path"]
+            file_path = arguments["file_path"]
             if not os.path.isabs(file_path):
                 raise RuntimeError(f"File path must be absolute: {file_path}")
 

src/mcp_text_editor/handlers/delete_text_file_contents.py

@@ -17,7 +17,7 @@ class DeleteTextFileContentsHandler(BaseHandler):
     """Handler for deleting content from a text file."""
 
     name = "delete_text_file_contents"
-    description = "Delete specified content ranges from a text file. The file must exist. File paths must be absolute."
+    description = "Delete specified content ranges from a text file. The file must exist. File paths must be absolute. You need to provide the file_hash comes from get_text_file_contents."
 
     def get_tool_description(self) -> Tool:
         """Get the tool description."""

src/mcp_text_editor/handlers/insert_text_file_contents.py

@@ -17,7 +17,7 @@ class InsertTextFileContentsHandler(BaseHandler):
     """Handler for inserting content before or after a specific line in a text file."""
 
     name = "insert_text_file_contents"
-    description = "Insert content before or after a specific line in a text file. Uses hash-based validation for concurrency control."
+    description = "Insert content before or after a specific line in a text file. Uses hash-based validation for concurrency control. You need to provide the file_hash comes from get_text_file_contents."
 
     def get_tool_description(self) -> Tool:
         """Get the tool description."""
@@ -27,7 +27,7 @@ def get_tool_description(self) -> Tool:
             inputSchema={
                 "type": "object",
                 "properties": {
-                    "path": {
+                    "file_path": {
                         "type": "string",
                         "description": "Path to the text file. File path must be absolute.",
                     },
@@ -53,21 +53,21 @@ def get_tool_description(self) -> Tool:
                         "default": "utf-8",
                     },
                 },
-                "required": ["path", "file_hash", "contents"],
+                "required": ["file_path", "file_hash", "contents"],
             },
         )
 
     async def run_tool(self, arguments: Dict[str, Any]) -> Sequence[TextContent]:
         """Execute the tool with given arguments."""
         try:
-            if "path" not in arguments:
-                raise RuntimeError("Missing required argument: path")
+            if "file_path" not in arguments:
+                raise RuntimeError("Missing required argument: file_path")
             if "file_hash" not in arguments:
                 raise RuntimeError("Missing required argument: file_hash")
             if "contents" not in arguments:
                 raise RuntimeError("Missing required argument: contents")
 
-            file_path = arguments["path"]
+            file_path = arguments["file_path"]
             if not os.path.isabs(file_path):
                 raise RuntimeError(f"File path must be absolute: {file_path}")
 

src/mcp_text_editor/text_editor.py

@@ -219,27 +219,18 @@ async def edit_file_contents(
         Args:
             file_path (str): Path to the file to edit
             expected_hash (str): Expected hash of the file before editing
-            patches (List[EditPatch]): List of patches to apply
-                - start (int): Starting line number (1-based, optional, default: 1)
-                - end (Optional[int]): Ending line number (inclusive)
-                - contents (str): New content to insert
-        Edit file contents with hash-based conflict detection and multiple patches (supporting new file creation).
-
-        Args:
-            file_path (str): Path to the file to edit (parent directories are created automatically)
-            expected_hash (str): Expected hash of the file before editing (empty string for new files)
             patches (List[Dict[str, Any]]): List of patches to apply, each containing:
                 - start (int): Starting line number (1-based)
                 - end (Optional[int]): Ending line number (inclusive)
-                - contents (str): New content to insert
+                - contents (str): New content to insert (if empty string, consider using delete_text_file_contents instead)
                 - range_hash (str): Expected hash of the content being replaced
 
         Returns:
             Dict[str, Any]: Results of the operation containing:
                 - result: "ok" or "error"
                 - hash: New file hash if successful, None if error
                 - reason: Error message if result is "error"
-                    "content": None,
+                    "file_hash": None,
                 }
 
             # Read current file content and verify hash
@@ -251,7 +242,6 @@ async def edit_file_contents(
                     return {
                         "result": "error",
                         "reason": "File not found and non-empty hash provided",
-                        "content": None,
                     }
                 # Create parent directories if they don't exist
                 parent_dir = os.path.dirname(file_path)
@@ -262,7 +252,6 @@ async def edit_file_contents(
                         return {
                             "result": "error",
                             "reason": f"Failed to create directory: {str(e)}",
-                            "content": None,
                         }
                 # Initialize empty state for new file
                 current_content = ""
@@ -271,9 +260,14 @@ async def edit_file_contents(
                 encoding = "utf-8"
             else:
                 # Read current file content and verify hash
-                current_content, _, _, current_hash, total_lines, _ = (
-                    await self.read_file_contents(file_path, encoding=encoding)
-                )
+                (
+                    current_content,
+                    _,
+                    _,
+                    current_hash,
+                    total_lines,
+                    _,
+                ) = await self.read_file_contents(file_path, encoding=encoding)
 
                 # Treat empty file as new file
                 if not current_content:
@@ -284,14 +278,11 @@ async def edit_file_contents(
                     return {
                         "result": "error",
                         "reason": "Unexpected error - Cannot treat existing file as new",
-                        "file_hash": None,
-                        "content": None,
                     }
                 elif current_hash != expected_hash:
                     return {
                         "result": "error",
                         "reason": "FileHash mismatch - Please use get_text_file_contents tool to get current content and hashes, then retry with the updated hashes.",
-                        "content": None,
                     }
                 else:
                     lines = current_content.splitlines(keepends=True)
@@ -324,8 +315,6 @@ async def edit_file_contents(
                         return {
                             "result": "error",
                             "reason": "Overlapping patches detected",
-                            "hash": None,
-                            "content": None,
                         }
 
             # Apply patches
@@ -355,12 +344,7 @@ async def edit_file_contents(
                     and current_content
                     and expected_hash == ""
                 ):
-                    return {
-                        "result": "error",
-                        "reason": "Unexpected error",
-                        "file_hash": None,
-                        "content": None,
-                    }
+                    return {"result": "error", "reason": "Unexpected error"}
 
                 # Calculate line ranges for zero-based indexing
                 start_zero = start - 1
@@ -400,7 +384,6 @@ async def edit_file_contents(
                             return {
                                 "result": "error",
                                 "reason": "Content range hash mismatch - Please use get_text_file_contents tool with the same start and end to get current content and hashes, then retry with the updated hashes.",
-                                "content": current_content,
                             }
 
                 # Prepare new content
@@ -409,6 +392,30 @@ async def edit_file_contents(
                 else:
                     contents = patch["contents"]
 
+                # Check if this is a deletion (empty content)
+                if not contents.strip():
+                    return {
+                        "result": "ok",
+                        "file_hash": current_hash,  # Return current hash since no changes made
+                        "hint": "For content deletion, please consider using delete_text_file_contents instead of patch with empty content",
+                        "suggestion": "delete",
+                    }
+
+                # Set suggestions for alternative tools
+                suggestion = None
+                hint = None
+                if not os.path.exists(file_path) or not current_content:
+                    suggestion = "append"
+                    hint = "For new or empty files, please consider using append_text_file_contents instead"
+                elif is_insertion:
+                    if start_zero >= len(lines):
+                        suggestion = "append"
+                        hint = "For adding content at the end of file, please consider using append_text_file_contents instead"
+                    else:
+                        suggestion = "insert"
+                        hint = "For inserting content within file, please consider using insert_text_file_contents instead"
+
+                # Prepare the content
                 new_content = contents if contents.endswith("\n") else contents + "\n"
                 new_lines = new_content.splitlines(keepends=True)
 
@@ -432,21 +439,14 @@ async def edit_file_contents(
                 "result": "ok",
                 "file_hash": new_hash,
                 "reason": None,
+                "suggestion": suggestion,
+                "hint": hint,
             }
 
         except FileNotFoundError:
-            return {
-                "result": "error",
-                "reason": f"File not found: {file_path}",
-                "file_hash": None,
-                "content": None,
-            }
+            return {"result": "error", "reason": f"File not found: {file_path}"}
         except (IOError, UnicodeError, PermissionError) as e:
-            return {
-                "result": "error",
-                "reason": f"Error editing file: {str(e)}",
-                "content": None,
-            }
+            return {"result": "error", "reason": f"Error editing file: {str(e)}"}
         except Exception as e:
             import traceback
 
@@ -455,7 +455,7 @@ async def edit_file_contents(
             return {
                 "result": "error",
                 "reason": "Unexpected error occurred",
-                "content": None,
+                "file_hash": None,
             }
 
     async def insert_text_file_contents(
@@ -493,11 +493,16 @@ async def insert_text_file_contents(
             }
 
         try:
-            current_content, _, _, current_hash, total_lines, _ = (
-                await self.read_file_contents(
-                    file_path,
-                    encoding=encoding,
-                )
+            (
+                current_content,
+                _,
+                _,
+                current_hash,
+                total_lines,
+                _,
+            ) = await self.read_file_contents(
+                file_path,
+                encoding=encoding,
             )
 
             if current_hash != file_hash:
@@ -585,11 +590,16 @@ async def delete_text_file_contents(
         self._validate_file_path(request.file_path)
 
         try:
-            current_content, _, _, current_hash, total_lines, _ = (
-                await self.read_file_contents(
-                    request.file_path,
-                    encoding=request.encoding or "utf-8",
-                )
+            (
+                current_content,
+                _,
+                _,
+                current_hash,
+                total_lines,
+                _,
+            ) = await self.read_file_contents(
+                request.file_path,
+                encoding=request.encoding or "utf-8",
             )
 
             # Check for conflicts

tests/test_append_text_file.py

@@ -42,7 +42,7 @@ async def test_append_text_file_success(test_dir: str, cleanup_files: None) -> N
 
     # Append content using handler
     arguments: Dict[str, Any] = {
-        "path": test_file,
+        "file_path": test_file,
         "contents": append_content,
         "file_hash": file_hash,
     }
@@ -66,7 +66,7 @@ async def test_append_text_file_not_exists(test_dir: str, cleanup_files: None) -
 
     # Try to append to non-existent file
     arguments: Dict[str, Any] = {
-        "path": test_file,
+        "file_path": test_file,
         "contents": "Some content\n",
         "file_hash": "dummy_hash",
     }
@@ -91,7 +91,7 @@ async def test_append_text_file_hash_mismatch(
 
     # Try to append with incorrect hash
     arguments: Dict[str, Any] = {
-        "path": test_file,
+        "file_path": test_file,
         "contents": "New content\n",
         "file_hash": "incorrect_hash",
     }
@@ -108,7 +108,7 @@ async def test_append_text_file_relative_path(
 ) -> None:
     """Test attempting to append using a relative path."""
     arguments: Dict[str, Any] = {
-        "path": "relative_path.txt",
+        "file_path": "relative_path.txt",
         "contents": "Some content\n",
         "file_hash": "dummy_hash",
     }
@@ -125,19 +125,19 @@ async def test_append_text_file_missing_args() -> None:
     # Test missing path
     with pytest.raises(RuntimeError) as exc_info:
         await append_handler.run_tool({"contents": "content\n", "file_hash": "hash"})
-    assert "Missing required argument: path" in str(exc_info.value)
+    assert "Missing required argument: file_path" in str(exc_info.value)
 
     # Test missing contents
     with pytest.raises(RuntimeError) as exc_info:
         await append_handler.run_tool(
-            {"path": "/absolute/path.txt", "file_hash": "hash"}
+            {"file_path": "/absolute/path.txt", "file_hash": "hash"}
         )
     assert "Missing required argument: contents" in str(exc_info.value)
 
     # Test missing file_hash
     with pytest.raises(RuntimeError) as exc_info:
         await append_handler.run_tool(
-            {"path": "/absolute/path.txt", "contents": "content\n"}
+            {"file_path": "/absolute/path.txt", "contents": "content\n"}
         )
     assert "Missing required argument: file_hash" in str(exc_info.value)
 
@@ -163,7 +163,7 @@ async def test_append_text_file_custom_encoding(
 
     # Append content using handler with specified encoding
     arguments: Dict[str, Any] = {
-        "path": test_file,
+        "file_path": test_file,
         "contents": append_content,
         "file_hash": file_hash,
         "encoding": "utf-8",