Skip to content

Commit 88b6d08

Browse files
authored
fix: remove auto-activation of first profile on hub import (#104)
When importing a hub, the extension was automatically activating the first profile in the hub's profile list. This unexpected behavior prevented users from having control over which profile to activate. This change removes the auto-activation logic and allows users to explicitly choose which profile to activate through the UI. - Remove auto-activation code from extension.ts hub import flow - Add comprehensive tests to prevent regression - Profiles now remain inactive until explicitly activated by user Fixes #103
1 parent 6cbb76f commit 88b6d08

File tree

2 files changed

+275
-15
lines changed

2 files changed

+275
-15
lines changed

src/extension.ts

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1398,21 +1398,9 @@ export class PromptRegistryExtension {
13981398
this.logger.warn('Failed to sync sources after hub import', syncError as Error);
13991399
}
14001400

1401-
// Try to activate a default profile if hub has profiles
1402-
try {
1403-
const hubProfiles = await hubManager.listActiveHubProfiles();
1404-
if (hubProfiles.length > 0) {
1405-
// Activate the first profile as default
1406-
const defaultProfile = hubProfiles[0];
1407-
this.logger.info(`Auto-activating default profile: ${defaultProfile.name}`);
1408-
await this.registryManager!.activateProfile(defaultProfile.id);
1409-
this.logger.info(`Default profile ${defaultProfile.id} activated successfully`);
1410-
} else {
1411-
this.logger.info('No profiles found in hub for auto-activation');
1412-
}
1413-
} catch (profileError) {
1414-
this.logger.warn('Failed to auto-activate default profile', profileError as Error);
1415-
}
1401+
// Note: We intentionally do NOT auto-activate any profile here.
1402+
// Users should explicitly choose which profile to activate.
1403+
this.logger.info('Hub imported successfully. User can manually activate a profile if desired.');
14161404

14171405
await vscode.commands.executeCommand('promptRegistry.refresh');
14181406
vscode.window.showInformationMessage(`Successfully activated ${selected.hubConfig.name}`);
Lines changed: 272 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,272 @@
1+
/**
2+
* Hub Import - No Auto-Activation Tests
3+
*
4+
* Verifies that importing a hub does NOT automatically activate any profile.
5+
* Users should explicitly choose which profile to activate.
6+
*
7+
* This test prevents regression of the bug where the first profile was
8+
* automatically activated upon hub import.
9+
*/
10+
11+
import * as assert from 'assert';
12+
import * as path from 'path';
13+
import * as fs from 'fs';
14+
import { HubManager } from '../../src/services/HubManager';
15+
import { HubStorage } from '../../src/storage/HubStorage';
16+
import { HubConfig, HubReference } from '../../src/types/hub';
17+
import { ValidationResult } from '../../src/services/SchemaValidator';
18+
19+
// Mock SchemaValidator
20+
class MockSchemaValidator {
21+
async validate(_data: any, _schemaPath: string): Promise<ValidationResult> {
22+
return { valid: true, errors: [], warnings: [] };
23+
}
24+
}
25+
26+
// Mock RegistryManager to track profile activation calls
27+
class MockRegistryManager {
28+
public activateProfileCalls: string[] = [];
29+
public sources: any[] = [];
30+
31+
async activateProfile(profileId: string): Promise<void> {
32+
this.activateProfileCalls.push(profileId);
33+
}
34+
35+
async listSources(): Promise<any[]> {
36+
return this.sources;
37+
}
38+
39+
async addSource(source: any): Promise<void> {
40+
this.sources.push(source);
41+
}
42+
43+
async updateSource(_id: string, _updates: any): Promise<void> {
44+
// no-op for tests
45+
}
46+
}
47+
48+
suite('Hub Import - No Auto-Activation', () => {
49+
let hubManager: HubManager;
50+
let storage: HubStorage;
51+
let mockValidator: MockSchemaValidator;
52+
let mockRegistryManager: MockRegistryManager;
53+
let tempDir: string;
54+
55+
const createHubWithProfiles = (): HubConfig => ({
56+
version: '1.0.0',
57+
metadata: {
58+
name: 'Test Hub with Profiles',
59+
description: 'Hub for testing no auto-activation',
60+
maintainer: 'Test',
61+
updatedAt: new Date().toISOString()
62+
},
63+
sources: [
64+
{
65+
id: 'source-1',
66+
name: 'Test Source',
67+
type: 'github',
68+
url: 'github:test/repo',
69+
enabled: true,
70+
priority: 1,
71+
metadata: { description: 'Test source' }
72+
}
73+
],
74+
profiles: [
75+
{
76+
id: 'profile-1',
77+
name: 'First Profile',
78+
description: 'This should NOT be auto-activated',
79+
bundles: [
80+
{ id: 'bundle-1', version: '1.0.0', source: 'source-1', required: true }
81+
],
82+
icon: '📦',
83+
active: false,
84+
createdAt: new Date().toISOString(),
85+
updatedAt: new Date().toISOString()
86+
},
87+
{
88+
id: 'profile-2',
89+
name: 'Second Profile',
90+
description: 'Another profile',
91+
bundles: [],
92+
icon: '📦',
93+
active: false,
94+
createdAt: new Date().toISOString(),
95+
updatedAt: new Date().toISOString()
96+
}
97+
]
98+
});
99+
100+
setup(() => {
101+
tempDir = path.join(__dirname, '..', '..', 'test-temp-no-auto-activation');
102+
if (fs.existsSync(tempDir)) {
103+
fs.rmSync(tempDir, { recursive: true });
104+
}
105+
fs.mkdirSync(tempDir, { recursive: true });
106+
107+
storage = new HubStorage(tempDir);
108+
mockValidator = new MockSchemaValidator();
109+
mockRegistryManager = new MockRegistryManager();
110+
111+
hubManager = new HubManager(
112+
storage,
113+
mockValidator as any,
114+
process.cwd(),
115+
undefined, // bundleInstaller
116+
mockRegistryManager as any // registryManager
117+
);
118+
});
119+
120+
teardown(() => {
121+
if (fs.existsSync(tempDir)) {
122+
fs.rmSync(tempDir, { recursive: true });
123+
}
124+
});
125+
126+
suite('Hub Import Behavior', () => {
127+
test('should NOT activate any profile when importing a hub with profiles', async () => {
128+
// Arrange: Create a hub config file with multiple profiles
129+
const hubConfig = createHubWithProfiles();
130+
const hubConfigPath = path.join(tempDir, 'hub-config.yml');
131+
const yaml = require('js-yaml');
132+
fs.writeFileSync(hubConfigPath, yaml.dump(hubConfig));
133+
134+
const localRef: HubReference = {
135+
type: 'local',
136+
location: hubConfigPath
137+
};
138+
139+
// Act: Import the hub
140+
const hubId = await hubManager.importHub(localRef, 'test-hub');
141+
142+
// Assert: No profile should be activated
143+
// 1. Check that RegistryManager.activateProfile was never called
144+
assert.strictEqual(
145+
mockRegistryManager.activateProfileCalls.length,
146+
0,
147+
'RegistryManager.activateProfile should NOT be called during hub import'
148+
);
149+
150+
// 2. Verify no profile has active=true in storage
151+
const profiles = await hubManager.listProfilesFromHub(hubId);
152+
const activeProfiles = profiles.filter(p => p.active);
153+
assert.strictEqual(
154+
activeProfiles.length,
155+
0,
156+
'No profile should be marked as active after hub import'
157+
);
158+
159+
// 3. Verify no activation state exists
160+
const activationState = await hubManager.getActiveProfile(hubId);
161+
assert.strictEqual(
162+
activationState,
163+
null,
164+
'No activation state should exist after hub import'
165+
);
166+
});
167+
168+
test('should NOT activate any profile when setting a hub as active', async () => {
169+
// Arrange: Create and import a hub
170+
const hubConfig = createHubWithProfiles();
171+
const hubConfigPath = path.join(tempDir, 'hub-config-active.yml');
172+
const yaml = require('js-yaml');
173+
fs.writeFileSync(hubConfigPath, yaml.dump(hubConfig));
174+
175+
const localRef: HubReference = {
176+
type: 'local',
177+
location: hubConfigPath
178+
};
179+
180+
const hubId = await hubManager.importHub(localRef, 'test-active-hub');
181+
182+
// Act: Set the hub as active
183+
await hubManager.setActiveHub(hubId);
184+
185+
// Assert: Still no profile should be activated
186+
assert.strictEqual(
187+
mockRegistryManager.activateProfileCalls.length,
188+
0,
189+
'RegistryManager.activateProfile should NOT be called when setting active hub'
190+
);
191+
192+
const profiles = await hubManager.listProfilesFromHub(hubId);
193+
const activeProfiles = profiles.filter(p => p.active);
194+
assert.strictEqual(
195+
activeProfiles.length,
196+
0,
197+
'No profile should be marked as active after setting hub as active'
198+
);
199+
});
200+
201+
test('profiles should remain inactive until explicitly activated by user', async () => {
202+
// Arrange: Create and import a hub
203+
const hubConfig = createHubWithProfiles();
204+
const hubConfigPath = path.join(tempDir, 'hub-config-explicit.yml');
205+
const yaml = require('js-yaml');
206+
fs.writeFileSync(hubConfigPath, yaml.dump(hubConfig));
207+
208+
const localRef: HubReference = {
209+
type: 'local',
210+
location: hubConfigPath
211+
};
212+
213+
const hubId = await hubManager.importHub(localRef, 'test-explicit-hub');
214+
await hubManager.setActiveHub(hubId);
215+
216+
// Verify profiles are inactive
217+
let profiles = await hubManager.listProfilesFromHub(hubId);
218+
assert.ok(profiles.every(p => !p.active), 'All profiles should be inactive initially');
219+
220+
// Act: Explicitly activate a profile (simulating user action)
221+
await hubManager.activateProfile(hubId, 'profile-1', { installBundles: false });
222+
223+
// Assert: Only the explicitly activated profile should be active
224+
profiles = await hubManager.listProfilesFromHub(hubId);
225+
const activeProfile = profiles.find(p => p.active);
226+
assert.ok(activeProfile, 'One profile should now be active');
227+
assert.strictEqual(activeProfile?.id, 'profile-1', 'The explicitly activated profile should be active');
228+
229+
const inactiveProfiles = profiles.filter(p => !p.active);
230+
assert.strictEqual(inactiveProfiles.length, 1, 'Other profiles should remain inactive');
231+
});
232+
});
233+
234+
suite('Hub Sync Behavior', () => {
235+
test('should NOT activate any profile when syncing a hub', async () => {
236+
// Arrange: Create and import a hub
237+
const hubConfig = createHubWithProfiles();
238+
const hubConfigPath = path.join(tempDir, 'hub-config-sync.yml');
239+
const yaml = require('js-yaml');
240+
fs.writeFileSync(hubConfigPath, yaml.dump(hubConfig));
241+
242+
const localRef: HubReference = {
243+
type: 'local',
244+
location: hubConfigPath
245+
};
246+
247+
const hubId = await hubManager.importHub(localRef, 'test-sync-hub');
248+
249+
// Modify the hub config (simulate remote update)
250+
hubConfig.metadata.description = 'Updated description';
251+
fs.writeFileSync(hubConfigPath, yaml.dump(hubConfig));
252+
253+
// Act: Sync the hub
254+
await hubManager.syncHub(hubId);
255+
256+
// Assert: No profile should be activated
257+
assert.strictEqual(
258+
mockRegistryManager.activateProfileCalls.length,
259+
0,
260+
'RegistryManager.activateProfile should NOT be called during hub sync'
261+
);
262+
263+
const profiles = await hubManager.listProfilesFromHub(hubId);
264+
const activeProfiles = profiles.filter(p => p.active);
265+
assert.strictEqual(
266+
activeProfiles.length,
267+
0,
268+
'No profile should be marked as active after hub sync'
269+
);
270+
});
271+
});
272+
});

0 commit comments

Comments
 (0)