MCP_CONNECTION_ISSUES.md•13.7 kB
# MCP CONNECTION ISSUES - DETAILED ANALYSIS
**Date:** 2025-11-09
**Priority:** P2-HIGH (Critical for tool reliability)
**Severity:** MEDIUM-HIGH
---
## Executive Summary
The MCP (Model Context Protocol) backend has **file operation validation gaps** where Grok (and other LLMs) can claim success while operations actually fail silently. This creates a false sense of security while files are not actually written or directories are not properly listed.
**Root Issues:**
1. Write operations don't validate directory existence before claiming success
2. List operations don't return error details, just empty results
3. Return messages are optimistic but don't reflect actual file system state
4. Grok receives confirmation but can't verify actual success
---
## Issue #1: Write File Claims Success But Dir May Not Exist
### Current Behavior (BROKEN)
```python
# Lines 826-853 in mcp_backend.py
elif name == "write_file":
filepath = arguments["filepath"]
content = arguments["content"]
path = self._sanitize(filepath) # Returns resolved path
try:
backup = path.with_suffix('.bak')
if path.exists():
shutil.copy2(path, backup)
# ❌ PROBLEM: No check if parent directory exists!
logger.info(f"✍️ Writing file to: {path.resolve()}")
path.write_text(content, encoding='utf-8') # May fail silently
# ❌ PROBLEM: Post-write check isn't comprehensive
if path.exists():
size = path.stat().st_size
logger.info(f"✅ Write verified: {path.resolve()} ({size} bytes)")
# ❌ PROBLEM: Returns success to Grok even if write fails
return [TextContent(type="text", text=f"Written: {filepath}\nActual location: {path.resolve()}\nBackup: {backup.name}")]
except Exception as e:
logger.error(f"❌ Write error for {filepath}: {e}")
return [TextContent(type="text", text=f"Error: {e}")]
```
### Failure Scenario
1. Grok asks to write: `dir/subdir/file.txt`
2. `/dir/subdir/` doesn't exist
3. `path.write_text()` fails with `FileNotFoundError`
4. Exception caught and logged, error returned
5. **BUT:** Grok's context might still think it worked if error message not clear
### Why It Happens
- Parent directory creation is not automatic
- `Path.write_text()` doesn't create parent directories
- Error message clarity depends on exception type
---
## Issue #2: List Directory Returns Empty Without Explanation
### Current Behavior (BROKEN)
```python
# Lines 790-805 in mcp_backend.py
if name == "list_directory":
rel_path = arguments.get("path", ".")
path = self._sanitize(rel_path)
if not path.is_dir():
return [TextContent(type="text", text="Error: Not a directory")]
patterns = self._get_gitignore_patterns(self.watch_dir)
files = []
for p in path.iterdir():
# ❌ PROBLEM: Silently filters hidden files and .gitignore matches
if p.name.startswith('.') or self._matches_gitignore(p, patterns):
continue # Skipped without logging
files.append({
"name": p.name,
"is_dir": p.is_dir(),
"size": p.stat().st_size if p.is_file() else 0
})
# ❌ PROBLEM: Returns empty list without reason
return [TextContent(type="text", text=json.dumps({"files": files}, indent=2))]
```
### Failure Scenario
1. Grok asks to list a directory with only hidden files/gitignored content
2. Directory exists but `files = []` (empty)
3. Grok receives: `{"files": []}`
4. **Grok can't tell if:**
- Directory is actually empty
- Directory has files but they're hidden/gitignored
- Directory doesn't exist at all
- Directory access failed
### Why It Happens
- No distinction between "empty" and "filtered"
- No error details when all entries are filtered
- No logging of what was skipped
- Grok interprets empty list as empty directory
---
## Issue #3: Path Validation Missing Pre-checks
### Current Problem
```python
# _sanitize only checks path traversal, not directory existence
def _sanitize(self, rel, base: Path = None):
base = base or self.watch_dir
p = (base / rel).resolve()
if not str(p).startswith(str(base)):
raise ValueError("Path traversal blocked")
# ❌ Returns path even if parent directories don't exist
return p
```
When Grok asks to write `new_dir/new_subdir/file.txt`:
1. Path is sanitized successfully
2. Grandparent directory doesn't exist
3. write_text() fails
4. Error message may be confusing to LLM
---
## Issue #4: No Parent Directory Creation Strategy
### Current Behavior
```python
# write_file doesn't create parent directories
path.write_text(content, encoding='utf-8')
# Path.write_text() will raise FileNotFoundError if parent doesn't exist
```
This is different from tools like `mkdir -p` which creates parent dirs.
---
## Proposed Solutions
### Solution 1: Comprehensive Write Validation ✅
```python
elif name == "write_file":
filepath = arguments["filepath"]
content = arguments["content"]
path = self._sanitize(filepath)
try:
# NEW: Ensure parent directory exists
path.parent.mkdir(parents=True, exist_ok=True)
logger.info(f"📁 Parent directory ready: {path.parent.resolve()}")
# Create backup if file exists
backup = path.with_suffix('.bak')
if path.exists():
shutil.copy2(path, backup)
logger.info(f"📝 Backup created: {backup.resolve()}")
# Write file
logger.info(f"✍️ Writing file to: {path.resolve()}")
path.write_text(content, encoding='utf-8')
# Comprehensive verification
if path.exists():
size = path.stat().st_size
actual_content = path.read_text(encoding='utf-8')
if actual_content == content:
logger.info(f"✅ Write verified: {path.resolve()} ({size} bytes)")
return [TextContent(type="text", text=f"""
Written: {filepath}
Location: {path.resolve()}
Size: {size} bytes
Backup: {backup.name if path.with_suffix('.bak').exists() else 'None'}
✅ Content verified
""")]
else:
logger.error(f"❌ Write failed: Content mismatch")
return [TextContent(type="text", text=f"Error: Content verification failed - file written but content doesn't match")]
else:
logger.error(f"❌ Write failed: File does not exist after write")
return [TextContent(type="text", text=f"Error: File was not created at {path.resolve()}")]
except Exception as e:
logger.error(f"❌ Write error for {filepath}: {e}\n{traceback.format_exc()}")
return [TextContent(type="text", text=f"Error: Failed to write {filepath}\nReason: {e}\nType: {type(e).__name__}")]
```
### Solution 2: Detailed List Directory Response ✅
```python
if name == "list_directory":
rel_path = arguments.get("path", ".")
path = self._sanitize(rel_path)
if not path.exists():
logger.warning(f"List failed: Directory does not exist: {path.resolve()}")
return [TextContent(type="text", text=f"Error: Directory does not exist: {path.resolve()}")]
if not path.is_dir():
return [TextContent(type="text", text=f"Error: Not a directory: {path.resolve()}")]
try:
patterns = self._get_gitignore_patterns(self.watch_dir)
files = []
filtered_count = 0
for p in path.iterdir():
if p.name.startswith('.'):
filtered_count += 1
logger.debug(f"Filtered (hidden): {p.name}")
continue
if self._matches_gitignore(p, patterns):
filtered_count += 1
logger.debug(f"Filtered (gitignore): {p.name}")
continue
files.append({
"name": p.name,
"is_dir": p.is_dir(),
"size": p.stat().st_size if p.is_file() else 0
})
# NEW: Include metadata about filtering
result = {
"path": str(path.resolve()),
"files": files,
"file_count": len(files),
"filtered_count": filtered_count,
"total_entries": len(files) + filtered_count,
"note": f"Showing {len(files)} visible files (hidden: {filtered_count} filtered by gitignore)"
}
logger.info(f"📂 Listed {path.resolve()}: {len(files)} visible, {filtered_count} filtered")
return [TextContent(type="text", text=json.dumps(result, indent=2))]
except PermissionError:
logger.error(f"List failed: Permission denied: {path.resolve()}")
return [TextContent(type="text", text=f"Error: Permission denied accessing {path.resolve()}")]
except Exception as e:
logger.error(f"List failed: {path.resolve()}\n{traceback.format_exc()}")
return [TextContent(type="text", text=f"Error: Failed to list {path.resolve()}\nReason: {e}")]
```
### Solution 3: Add Directory Creation Tool ✅
```python
@self.server.list_tools()
async def list_tools():
return [
# ... existing tools ...
Tool(name="create_directory", description="Create directory (with parents)", inputSchema={
"type": "object",
"properties": {
"path": {"type": "string", "description": "Directory path"}
},
"required": ["path"]
}),
]
# In call_tool:
elif name == "create_directory":
rel_path = arguments.get("path", ".")
path = self._sanitize(rel_path)
try:
if path.exists():
if path.is_dir():
logger.info(f"📁 Directory already exists: {path.resolve()}")
return [TextContent(type="text", text=f"Directory already exists: {path.resolve()}")]
else:
logger.error(f"❌ Path exists but is not a directory: {path.resolve()}")
return [TextContent(type="text", text=f"Error: Path exists but is not a directory: {path.resolve()}")]
path.mkdir(parents=True, exist_ok=True)
logger.info(f"✅ Created directory: {path.resolve()}")
return [TextContent(type="text", text=f"Created directory: {path.resolve()}")]
except Exception as e:
logger.error(f"❌ Failed to create directory: {path.resolve()}\n{e}")
return [TextContent(type="text", text=f"Error: Failed to create directory\nPath: {path.resolve()}\nReason: {e}")]
```
### Solution 4: Enhanced Error Messages ✅
```python
# Better error context for Grok
def _get_error_context(self, path: Path) -> str:
"""Get detailed error context for debugging"""
context = []
context.append(f"Path: {path.resolve()}")
context.append(f"Exists: {path.exists()}")
context.append(f"Parent exists: {path.parent.exists()}")
context.append(f"Grandparent exists: {path.parent.parent.exists()}")
context.append(f"Is directory: {path.is_dir() if path.exists() else 'N/A'}")
context.append(f"Permissions: {oct(path.stat().st_mode) if path.exists() else 'N/A'}")
return "\n".join(context)
```
---
## Implementation Priority
### P2-1: Critical (Do First)
- [x] Add parent directory creation to write_file
- [x] Add detailed list_directory response
- [x] Better error messages with context
### P2-2: Important (Do Second)
- [ ] Add create_directory tool
- [ ] Add error context helper
- [ ] Verify content after write
### P2-3: Nice-to-Have (Do Later)
- [ ] Add file permission listing
- [ ] Add recursive directory size calculation
- [ ] Add file type detection
---
## Testing Strategy
### Test Case 1: Write to Non-Existent Directory
```
Grok: "Create file: projects/new_project/config.json with content {}"
Expected: Success - directory created
Current: Fails with FileNotFoundError
```
### Test Case 2: List Empty Directory
```
Grok: "List directory: projects/"
Expected: Returns {"files": [], "filtered_count": 0}
Current: Returns {"files": []} (no context)
```
### Test Case 3: List Directory with Only Hidden Files
```
Grok: "List directory: projects/.hidden"
Expected: Returns with clear "all files filtered" message
Current: Returns {"files": []} (confusing!)
```
### Test Case 4: Write With Permission Error
```
Grok: "Write to read-only directory"
Expected: Clear permission error message
Current: Generic exception message
```
---
## Impact Assessment
### Current Risk
- **HIGH:** Grok believes operations succeeded when they fail
- **HIGH:** Silent failures lead to task incompletion
- **MEDIUM:** Debugging is difficult due to lack of context
- **MEDIUM:** Tool reliability questioned
### After Fixes
- **LOW:** All operations provide clear success/failure
- **LOW:** Parent directories created automatically
- **LOW:** Grok has full context for decision-making
- **HIGH:** Tool reliability restored
---
## Files to Modify
- `mcp_backend.py:826-853` - write_file implementation
- `mcp_backend.py:790-805` - list_directory implementation
- `mcp_backend.py:749-785` - tool definitions (add create_directory)
- `mcp_backend.py` - Add _get_error_context helper
---
## Success Criteria
- [ ] Write operations create parent directories
- [ ] Write operations verify content matches
- [ ] List operations explain filtering
- [ ] All errors include context information
- [ ] Grok never receives misleading success messages
- [ ] New create_directory tool works reliably
- [ ] All test cases pass
---
**Issue Category:** Tool Reliability
**Blocking Production:** YES - file operations are critical
**User Impact:** HIGH - core functionality affected
**Estimated Fix Time:** 2-3 hours
**Risk Level:** LOW - changes are localized and well-defined