Skip to content

Commit 981ff0c

Browse files
author
LittleCoinCoin
committed
fix: config backup restore system
Root cause: MCPHostConfigBackupManager.restore_backup() was a placeholder implementation that always returned True without performing actual file restoration operations. Critical fixes: 1. **Implement actual restore functionality**: Added proper file restoration using MCPHostRegistry.get_host_config_path() to get target config paths 2. **Atomic restore operations**: Use AtomicFileOperations.atomic_copy() for safe file restoration with rollback capability 3. **Host validation**: Validate hostname using MCPHostType enum before attempting restore operations 4. **Directory creation**: Ensure target directory exists before restoration 5. **ASCII compatibility**: Replace Unicode symbols with ASCII-compatible text in CLI output to prevent encoding errors Resolves Issue 1: Backup Restore System Malfunction - ✅ Backup restore actually restores configuration files to previous state - ✅ CLI reports accurate success/failure status based on actual operations - ✅ Atomic operations ensure data integrity during restoration - ✅ Backward compatibility maintained for existing backup file formats Tested: Manual verification shows restore correctly restores mcp_101 server configuration from backup, changing file size from 288 to 338 bytes with proper content restoration. Follows Phase 4 development workflow with systematic debugging approach.
1 parent 55efeaa commit 981ff0c

File tree

3 files changed

+28
-11
lines changed

3 files changed

+28
-11
lines changed

hatch/cli_hatch.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -308,10 +308,10 @@ def handle_mcp_backup_restore(host: str, backup_file: Optional[str] = None, dry_
308308
success = backup_manager.restore_backup(host, backup_file)
309309

310310
if success:
311-
print(f" Successfully restored backup '{backup_file}' for host '{host}'")
311+
print(f"[SUCCESS] Successfully restored backup '{backup_file}' for host '{host}'")
312312
return 0
313313
else:
314-
print(f" Failed to restore backup '{backup_file}' for host '{host}'")
314+
print(f"[ERROR] Failed to restore backup '{backup_file}' for host '{host}'")
315315
return 1
316316

317317
except Exception as e:

hatch/mcp_host_config/backup.py

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -266,11 +266,11 @@ def create_backup(self, config_path: Path, hostname: str) -> BackupResult:
266266

267267
def restore_backup(self, hostname: str, backup_file: Optional[str] = None) -> bool:
268268
"""Restore configuration from backup.
269-
269+
270270
Args:
271271
hostname (str): Host identifier
272272
backup_file (str, optional): Specific backup file name. Defaults to latest.
273-
273+
274274
Returns:
275275
bool: True if restoration successful, False otherwise
276276
"""
@@ -280,14 +280,31 @@ def restore_backup(self, hostname: str, backup_file: Optional[str] = None) -> bo
280280
backup_path = self.backup_root / hostname / backup_file
281281
else:
282282
backup_path = self._get_latest_backup(hostname)
283-
283+
284284
if not backup_path or not backup_path.exists():
285285
return False
286-
287-
# For now, we don't have host-specific config paths (future implementation)
288-
# This is a placeholder that would be implemented in host configuration phase
289-
return True
290-
286+
287+
# Get target configuration path using host registry
288+
from .host_management import MCPHostRegistry
289+
from .models import MCPHostType
290+
291+
try:
292+
host_type = MCPHostType(hostname)
293+
target_path = MCPHostRegistry.get_host_config_path(host_type)
294+
295+
if not target_path:
296+
return False
297+
298+
# Ensure target directory exists
299+
target_path.parent.mkdir(parents=True, exist_ok=True)
300+
301+
# Perform atomic restore operation
302+
return self.atomic_ops.atomic_copy(backup_path, target_path)
303+
304+
except ValueError:
305+
# Invalid hostname
306+
return False
307+
291308
except Exception:
292309
return False
293310

tests/test_mcp_cli_backup_management.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ def test_backup_restore_successful(self):
127127

128128
# Verify success message
129129
print_calls = [call[0][0] for call in mock_print.call_args_list]
130-
self.assertTrue(any(" Successfully restored backup" in call for call in print_calls))
130+
self.assertTrue(any("[SUCCESS] Successfully restored backup" in call for call in print_calls))
131131

132132

133133
class TestMCPBackupListCommand(unittest.TestCase):

0 commit comments

Comments
 (0)