Skip to content

Commit

Permalink
refactor(server.py): remove legacy format support for file arguments …
Browse files Browse the repository at this point in the history 
…to simplify code and enforce new format

fix(server.py): ensure 'files' argument is mandatory for tool execution to prevent runtime errors
refactor(test_server.py): update tests to use new 'files' argument format for consistency and clarity
refactor(text_editor.py): streamline file result initialization for better readability
test(tests): enhance tests to validate new response structure and error handling for missing arguments
  • Loading branch information
@tumf
tumf committed Dec 14, 2024
1 parent 79af627 commit 216723b
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 105 deletions.
47 changes: 3 additions & 44 deletions src/mcp_text_editor/server.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,54 +76,13 @@ def get_tool_description(self) -> Tool:
async def run_tool(self, arguments: Dict[str, Any]) -> Sequence[TextContent]:
"""Execute the tool with given arguments."""
try:
# Check for required argument 'files'
if "files" not in arguments:
if "file_path" not in arguments:
raise RuntimeError("Missing required argument: 'files'")

# Legacy format support
file_path = arguments["file_path"]
line_start = arguments.get("line_start", 1)
line_end = arguments.get("line_end")
# Convert to new format with mandatory fields
arguments = {
"files": [
{
"file_path": file_path,
"ranges": [
{
"start": line_start,
"end": line_end if line_end else None,
}
],
}
]
}
# Legacy format request
legacy_format = True
else:
legacy_format = False
raise RuntimeError("Missing required argument: 'files'")

# Handle request
result = await self.editor.read_multiple_ranges(arguments["files"])

# Convert to appropriate format based on request type
if legacy_format:
# Legacy format expects a flat structure
file_path = arguments["files"][0]["file_path"]
file_result = result[file_path]
range_result = file_result["ranges"][0]
response = {
"file_path": file_path,
"contents": range_result["content"],
"line_start": range_result["start_line"],
"line_end": range_result["end_line"],
"file_hash": file_result["file_hash"],
"file_lines": range_result["total_lines"],
"file_size": range_result["content_size"],
}
else:
# New format returns multi-file, multi-range structure
response = result
response = result

return [TextContent(type="text", text=json.dumps(response, indent=2))]

Expand Down
5 changes: 1 addition & 4 deletions src/mcp_text_editor/text_editor.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,7 @@ async def read_multiple_ranges(
for file_range in ranges:
file_path = file_range["file_path"]
self._validate_file_path(file_path)
result[file_path] = {
"ranges": [],
"file_hash": ""
}
result[file_path] = {"ranges": [], "file_hash": ""}

try:
# Detect the file encoding before reading
Expand Down
76 changes: 41 additions & 35 deletions tests/test_server.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,23 +44,25 @@ async def test_list_tools():
@pytest.mark.asyncio
async def test_get_contents_handler(test_file):
"""Test GetTextFileContents handler."""
args = {"file_path": test_file, "line_start": 1, "line_end": 3}
args = {"files": [{"file_path": test_file, "ranges": [{"start": 1, "end": 3}]}]}
result = await get_contents_handler.run_tool(args)
assert len(result) == 1
assert isinstance(result[0], TextContent)
content = json.loads(result[0].text)
assert "contents" in content
assert "line_start" in content
assert "line_end" in content
assert "file_hash" in content
assert "file_lines" in content
assert "file_size" in content
assert test_file in content
range_result = content[test_file]["ranges"][0]
assert "content" in range_result
assert "start_line" in range_result
assert "end_line" in range_result
assert "file_hash" in content[test_file]
assert "total_lines" in range_result
assert "content_size" in range_result


@pytest.mark.asyncio
async def test_get_contents_handler_invalid_file(test_file):
"""Test GetTextFileContents handler with invalid file."""
args = {"file_path": "nonexistent.txt"}
args = {"files": [{"file_path": "nonexistent.txt", "ranges": [{"start": 1}]}]}
with pytest.raises(RuntimeError) as exc_info:
await get_contents_handler.run_tool(args)
assert "File not found" in str(exc_info.value)
Expand All @@ -70,10 +72,10 @@ async def test_get_contents_handler_invalid_file(test_file):
async def test_edit_contents_handler(test_file):
"""Test EditTextFileContents handler."""
# First, get the current content and hash
get_args = {"file_path": test_file}
get_args = {"files": [{"file_path": test_file, "ranges": [{"start": 1}]}]}
get_result = await get_contents_handler.run_tool(get_args)
content_info = json.loads(get_result[0].text)
initial_hash = content_info["file_hash"]
initial_hash = content_info[test_file]["file_hash"]

# Get range hash for the target lines
get_range_args = {
Expand All @@ -82,7 +84,7 @@ async def test_edit_contents_handler(test_file):
range_result = json.loads(
(await get_contents_handler.run_tool(get_range_args))[0].text
)
range_hash = range_result[test_file][0]["range_hash"]
range_hash = range_result[test_file]["ranges"][0]["range_hash"]

# Create edit operation
edit_args = {
Expand Down Expand Up @@ -113,17 +115,19 @@ async def test_edit_contents_handler(test_file):
@pytest.mark.asyncio
async def test_call_tool_get_contents(test_file):
"""Test call_tool with GetTextFileContents."""
args = {"file_path": test_file, "line_start": 1, "line_end": 3}
args = {"files": [{"file_path": test_file, "ranges": [{"start": 1, "end": 3}]}]}
result = await call_tool("get_text_file_contents", args)
assert len(result) == 1
assert isinstance(result[0], TextContent)
content = json.loads(result[0].text)
assert "contents" in content
assert "line_start" in content
assert "line_end" in content
assert "file_hash" in content
assert "file_lines" in content
assert "file_size" in content
assert test_file in content
range_result = content[test_file]["ranges"][0]
assert "content" in range_result
assert "start_line" in range_result
assert "end_line" in range_result
assert "file_hash" in content[test_file]
assert "total_lines" in range_result
assert "content_size" in range_result


@pytest.mark.asyncio
Expand All @@ -144,7 +148,10 @@ async def test_call_tool_error_handling():

# Test with invalid file path
with pytest.raises(RuntimeError) as exc_info:
await call_tool("get_text_file_contents", {"file_path": "nonexistent.txt"})
await call_tool(
"get_text_file_contents",
{"files": [{"file_path": "nonexistent.txt", "ranges": [{"start": 1}]}]},
)
assert "File not found" in str(exc_info.value)


Expand All @@ -162,9 +169,10 @@ async def test_edit_contents_handler_multiple_files(tmp_path):
file_operations = []
for file_path in test_files:
# Get file hash
get_result = await get_contents_handler.run_tool({"file_path": file_path})
get_args = {"files": [{"file_path": file_path, "ranges": [{"start": 1}]}]}
get_result = await get_contents_handler.run_tool(get_args)
content_info = json.loads(get_result[0].text)
file_hash = content_info["file_hash"]
file_hash = content_info[file_path]["file_hash"]

# Get range hash
get_range_args = {
Expand All @@ -178,7 +186,7 @@ async def test_edit_contents_handler_multiple_files(tmp_path):
range_result = json.loads(
(await get_contents_handler.run_tool(get_range_args))[0].text
)
range_hash = range_result[file_path][0]["range_hash"]
range_hash = range_result[file_path]["ranges"][0]["range_hash"]

# Create operation for this file
file_operations.append(
Expand All @@ -189,7 +197,7 @@ async def test_edit_contents_handler_multiple_files(tmp_path):
{
"line_start": 2,
"line_end": 2,
"contents": f"Modified Line 2 in file {file_path}\n",
"contents": "Modified Line 2\n",
"range_hash": range_hash,
}
],
Expand Down Expand Up @@ -218,9 +226,10 @@ async def test_edit_contents_handler_partial_failure(tmp_path):
valid_path = str(valid_file)

# Get hash for valid file
get_result = await get_contents_handler.run_tool({"file_path": valid_path})
get_args = {"files": [{"file_path": valid_path, "ranges": [{"start": 1}]}]}
get_result = await get_contents_handler.run_tool(get_args)
content_info = json.loads(get_result[0].text)
valid_hash = content_info["file_hash"]
valid_hash = content_info[valid_path]["file_hash"]

# Get range hash for the target lines
get_range_args = {
Expand All @@ -229,7 +238,7 @@ async def test_edit_contents_handler_partial_failure(tmp_path):
range_result = json.loads(
(await get_contents_handler.run_tool(get_range_args))[0].text
)
valid_range_hash = range_result[valid_path][0]["range_hash"]
valid_range_hash = range_result[valid_path]["ranges"][0]["range_hash"]

# Create edit operations for both valid and invalid files
edit_args = {
Expand Down Expand Up @@ -270,14 +279,11 @@ async def test_edit_contents_handler_partial_failure(tmp_path):


@pytest.mark.asyncio
async def test_edit_contents_handler_empty_args():
"""Test EditTextFileContents handler with empty files."""
edit_args = {"files": []}
result = await edit_contents_handler.run_tool(edit_args)
assert len(result) == 1
edit_results = json.loads(result[0].text)
assert isinstance(edit_results, dict)
assert len(edit_results) == 0
async def test_get_contents_handler_missing_args():
"""Test GetTextFileContents handler with missing arguments."""
with pytest.raises(RuntimeError) as exc_info:
await get_contents_handler.run_tool({})
assert "Missing required argument: 'files'" in str(exc_info.value)


@pytest.mark.asyncio
Expand Down Expand Up @@ -306,7 +312,7 @@ async def test_get_contents_handler_legacy_missing_args():
"""Test GetTextFileContents handler with legacy single file request missing arguments."""
with pytest.raises(RuntimeError) as exc_info:
await get_contents_handler.run_tool({})
assert "Missing required argument: files" in str(exc_info.value)
assert "Missing required argument: 'files'" in str(exc_info.value)


@pytest.mark.asyncio
Expand Down
Loading

0 comments on commit 216723b

Please sign in to comment.