Skip to content

Commit 43855b4

Browse files
niechenclaude
andcommitted
Fix v1 to v2 migration to only occur after user confirmation
Previously, ProfileConfigManager would automatically migrate v1 profiles.json files on initialization, which happened before users confirmed they wanted to migrate. This caused profiles to be migrated prematurely. Changes: - Removed automatic migration from ProfileConfigManager.__init__ - Removed unused _migrate_legacy_profiles methods from ProfileConfigManager - Added create_profile() method to ProfileConfigManager for V1ToV2Migrator - Updated V1ToV2Migrator to also remove profiles.json after migration - Updated test to reflect that ProfileConfigManager no longer auto-migrates - Fixed ASCII art logo spacing in cli.py and rich_click_config.py Now profile migration only happens after user explicitly chooses to migrate via the V1ToV2Migrator prompt. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent 345f08a commit 43855b4

File tree

5 files changed

+42
-67
lines changed

5 files changed

+42
-67
lines changed

src/mcpm/cli.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,12 +49,12 @@ def print_logo():
4949

5050
# Clean ASCII art design - simplified with light shades
5151
logo_text = """
52-
███░ ███░ ██████░ ██████░ ███░ ███░
53-
████░ ████░ ██░░░░░░ ██░░░██░████░ ████░
54-
██░████░██░ ██░ ██████░░██░████░██░
55-
██░░██░░██░ ██░ ██░░░░░ ██░░██░░██░
56-
██░ ░░░ ██░ ░██████░ ██░ ██░ ░░░ ██░
57-
░░░ ░░░ ░░░░░░░ ░░░ ░░░ ░░░
52+
███░ ███░ ██████░ ██████░ ███░ ███░
53+
████░ ████░ ██░░░░░░ ██░░░██░ ████░ ████░
54+
██░████░██░ ██░ ██████░░ ██░████░██░
55+
██░░██░░██░ ██░ ██░░░░░ ██░░██░░██░
56+
██░ ░░░ ██░ ░██████░ ██░ ██░ ░░░ ██░
57+
░░░ ░░░ ░░░░░░░ ░░░ ░░░ ░░░
5858
5959
"""
6060

src/mcpm/migration/v1_migrator.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -430,14 +430,22 @@ def _cleanup_main_config(self):
430430
except Exception as e:
431431
console.print(f" ⚠️ Failed to remove config.json: {e}")
432432

433+
# Remove v1 profiles.json file after migration
434+
if self.detector.profiles_file.exists():
435+
try:
436+
self.detector.profiles_file.unlink()
437+
console.print(" 🧹 Removed v1 profiles.json file")
438+
except Exception as e:
439+
console.print(f" ⚠️ Failed to remove profiles.json: {e}")
440+
433441
def _show_migration_summary(self, migrated_profiles: Dict[str, int]):
434442
"""Show migration completion summary"""
435443
summary_panel = Panel(
436444
"[bold green]✅ Migration Completed Successfully![/]\n\n"
437445
f"📊 [bold]Migration Results[/]\n"
438446
f" • {len(migrated_profiles)} profiles migrated\n"
439447
f" • {sum(migrated_profiles.values())} servers migrated\n"
440-
f" • v1 config.json removed\n"
448+
f" • v1 config.json and profiles.json removed\n"
441449
f" • v1 configs backed up in [cyan]~/.config/mcpm/backups/[/]\n\n"
442450
f"🎯 [bold]What's Different Now[/]\n"
443451
f" • All servers are in global configuration\n"

src/mcpm/profile/profile_config.py

Lines changed: 14 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
1-
import json
21
import logging
32
import os
43
from typing import Dict, List, Optional
54

6-
from pydantic import TypeAdapter
7-
85
from mcpm.core.schema import ProfileMetadata, ServerConfig
96
from mcpm.global_config import GlobalConfigManager
107

@@ -30,47 +27,8 @@ def __init__(
3027
self.profile_path = os.path.expanduser(profile_path)
3128
self.global_config = global_config_manager or GlobalConfigManager()
3229

33-
# For backward compatibility, attempt migration if old profiles.json exists
34-
self._migrate_legacy_profiles_if_needed()
35-
36-
def _migrate_legacy_profiles_if_needed(self) -> None:
37-
"""Migrate old profiles.json to virtual profile system if it exists."""
38-
if os.path.exists(self.profile_path):
39-
logger.info(f"Migrating legacy profiles from {self.profile_path}")
40-
try:
41-
self._migrate_legacy_profiles()
42-
# Backup the old file
43-
backup_path = self.profile_path + ".backup"
44-
os.rename(self.profile_path, backup_path)
45-
logger.info(f"Legacy profiles migrated successfully. Backup saved to {backup_path}")
46-
except Exception as e:
47-
logger.error(f"Failed to migrate legacy profiles: {e}")
48-
49-
def _migrate_legacy_profiles(self) -> None:
50-
"""Migrate profiles from old profiles.json format to virtual profiles."""
51-
try:
52-
with open(self.profile_path, "r", encoding="utf-8") as f:
53-
legacy_profiles = json.load(f) or {}
54-
except (json.JSONDecodeError, FileNotFoundError):
55-
return
56-
57-
for profile_name, configs in legacy_profiles.items():
58-
# Create profile metadata
59-
self.global_config.create_profile_metadata(profile_name)
60-
61-
# Add servers to global config and tag them with profile
62-
for config_data in configs:
63-
try:
64-
server_config = TypeAdapter(ServerConfig).validate_python(config_data)
65-
66-
# Add server to global config if not exists
67-
if not self.global_config.server_exists(server_config.name):
68-
self.global_config.add_server(server_config)
69-
70-
# Tag server with profile
71-
self.global_config.add_profile_tag_to_server(server_config.name, profile_name)
72-
except Exception as e:
73-
logger.error(f"Failed to migrate server {config_data.get('name', 'unknown')}: {e}")
30+
# Note: Legacy profile migration is now handled by V1ToV2Migrator
31+
# to ensure it only happens after user confirms migration
7432

7533
def _load_profiles(self) -> Dict[str, List[ServerConfig]]:
7634
"""Legacy method - now returns virtual profiles from global config."""
@@ -225,6 +183,18 @@ def get_complete_profile(self, profile_name: str) -> Optional[Dict]:
225183
"""Get complete profile info including metadata and servers."""
226184
return self.global_config.get_complete_profile(profile_name)
227185

186+
def create_profile(self, profile_name: str, description: str = "") -> bool:
187+
"""Create a new profile with optional description."""
188+
# Check if profile already exists
189+
if self.global_config.get_profile_metadata(profile_name) or self.global_config.virtual_profile_exists(
190+
profile_name
191+
):
192+
return False
193+
194+
# Create profile metadata with description
195+
metadata = ProfileMetadata(name=profile_name, description=description)
196+
return self.global_config.update_profile_metadata(metadata)
197+
228198
def add_server_to_profile(self, profile_name: str, server_name: str) -> bool:
229199
"""Add an existing global server to a profile by tagging it."""
230200
# Ensure profile exists

src/mcpm/utils/rich_click_config.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,12 @@
2020

2121
# ASCII art logo - simplified with light shades
2222
ASCII_ART = """
23-
███░ ███░ ██████░ ██████░ ███░ ███░
24-
████░ ████░ ██░░░░░░ ██░░░██░████░ ████░
25-
██░████░██░ ██░ ██████░░██░████░██░
26-
██░░██░░██░ ██░ ██░░░░░ ██░░██░░██░
27-
██░ ░░░ ██░ ░██████░ ██░ ██░ ░░░ ██░
28-
░░░ ░░░ ░░░░░░░ ░░░ ░░░ ░░░
23+
███░ ███░ ██████░ ██████░ ███░ ███░
24+
████░ ████░ ██░░░░░░ ██░░░██░ ████░ ████░
25+
██░████░██░ ██░ ██████░░ ██░████░██░
26+
██░░██░░██░ ██░ ██░░░░░ ██░░██░░██░
27+
██░ ░░░ ██░ ░██████░ ██░ ██░ ░░░ ██░
28+
░░░ ░░░ ░░░░░░░ ░░░ ░░░ ░░░
2929
3030
"""
3131

tests/test_profile.py

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -71,22 +71,19 @@ def test_profile_manager_init_custom_path(profile_manager_clean, temp_dirs):
7171

7272

7373
def test_legacy_migration(profile_manager_with_legacy, temp_dirs):
74-
"""Test that legacy profiles.json is migrated to virtual profiles"""
74+
"""Test that legacy profiles.json is NOT automatically migrated"""
7575
temp_dir, servers_path, metadata_path, legacy_path = temp_dirs
7676
manager = profile_manager_with_legacy
7777

78-
# Check that legacy file was moved to backup
79-
assert os.path.exists(legacy_path + ".backup")
78+
# Check that legacy file still exists (no auto-migration)
79+
assert os.path.exists(legacy_path)
80+
assert not os.path.exists(legacy_path + ".backup")
8081

81-
# Check that profiles were migrated
82+
# Check that profiles were NOT migrated automatically
8283
profiles = manager.list_profiles()
83-
assert "test_profile" in profiles
84-
assert "empty_profile" in profiles
84+
assert len(profiles) == 0 # No profiles should exist yet
8585

86-
# Check that server was migrated to global config
87-
test_profile_servers = manager.get_profile("test_profile")
88-
assert len(test_profile_servers) == 1
89-
assert test_profile_servers[0].name == "test-server"
86+
# Legacy migration is now handled by V1ToV2Migrator, not ProfileConfigManager
9087

9188

9289
def test_new_profile(profile_manager_clean):

0 commit comments

Comments
 (0)