Skip to content

Commit a4ca628

Browse files
authored
one time mitigation of unintentional setting saved to user settings (#1527)
On activation (right after `setPersistentState`), runs once per profile. If `python-envs.defaultEnvManager` has a globalValue of 1ms-python.python:system1, it clears it via `config.update(..., undefined, ConfigurationTarget.Global)`, logs an info trace pointing at issue #1468, and sets a persistent-state flag `(globalSettingsMigration.systemEnvManagerRemoved)` so it never runs again. NOTE: Here we can't distinguish "user intent" from "bug-set value". If anyone deliberately set `defaultEnvManager: "system"` globally, this silently undoes it. This is a tradeoff Im willing to take given the number of users potentially in each bucket
1 parent dbc1d18 commit a4ca628

4 files changed

Lines changed: 348 additions & 4 deletions

File tree

src/common/telemetry/constants.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,18 @@ export enum EventNames {
197197
* - errorType: string (classified error category, on failure only)
198198
*/
199199
PET_RESOLVE = 'PET.RESOLVE',
200+
/**
201+
* Telemetry event for the one-time migration that removes a stale
202+
* `python-envs.defaultEnvManager: system` value from User (global) settings.
203+
* Fires only on the activation when the migration actually runs (not on subsequent runs).
204+
* Properties:
205+
* - outcome: 'removed' (was set to system, all user-scope slots cleared)
206+
* | 'partial' (cleared current context's slot but another user-scope slot still has it; will retry)
207+
* | 'not_set' (no user-scope slot of system found, nothing to do)
208+
* | 'failed' (attempted removal threw)
209+
* - errorType: string (only when outcome === 'failed')
210+
*/
211+
MIGRATION_SYSTEM_ENV_MANAGER = 'MIGRATION.SYSTEM_ENV_MANAGER',
200212
}
201213

202214
// Map all events to their properties
@@ -634,4 +646,15 @@ export interface IEventNamePropertyMapping {
634646
result: 'success' | 'timeout' | 'error';
635647
errorType?: string;
636648
};
649+
650+
/* __GDPR__
651+
"migration.system_env_manager": {
652+
"outcome": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" },
653+
"errorType": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }
654+
}
655+
*/
656+
[EventNames.MIGRATION_SYSTEM_ENV_MANAGER]: {
657+
outcome: 'removed' | 'partial' | 'not_set' | 'failed';
658+
errorType?: string;
659+
};
637660
}

src/extension.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ import {
7171
import { PythonProjectManagerImpl } from './features/projectManager';
7272
import { getPythonApi, setPythonApi } from './features/pythonApi';
7373
import { registerCompletionProvider } from './features/settings/settingCompletions';
74+
import { migrateGlobalDefaultEnvManagerSetting } from './features/settings/settingHelpers';
7475
import { setActivateMenuButtonContext } from './features/terminal/activateMenuButton';
7576
import { normalizeShellPath } from './features/terminal/shells/common/shellUtils';
7677
import {
@@ -162,6 +163,15 @@ export async function activate(context: ExtensionContext): Promise<PythonEnviron
162163
// Setup the persistent state for the extension.
163164
setPersistentState(context);
164165

166+
// One-time migration: remove `system` defaultEnvManager from User settings if a previous
167+
// version wrote it there (bug #1468). Awaited so the migration deterministically affects
168+
// initial environment selection on the very first activation after upgrade.
169+
try {
170+
await migrateGlobalDefaultEnvManagerSetting();
171+
} catch (err) {
172+
traceError(`[migration] migrateGlobalDefaultEnvManagerSetting threw: ${err}`);
173+
}
174+
165175
const statusBar = new PythonStatusBarImpl();
166176
context.subscriptions.push(statusBar);
167177

src/features/settings/settingHelpers.ts

Lines changed: 94 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
import * as path from 'path';
22
import { ConfigurationScope, ConfigurationTarget, Uri, WorkspaceConfiguration, WorkspaceFolder } from 'vscode';
33
import { PythonProject } from '../../api';
4-
import { DEFAULT_ENV_MANAGER_ID, DEFAULT_PACKAGE_MANAGER_ID } from '../../common/constants';
4+
import { DEFAULT_ENV_MANAGER_ID, DEFAULT_PACKAGE_MANAGER_ID, SYSTEM_MANAGER_ID } from '../../common/constants';
55
import { traceError, traceInfo, traceVerbose, traceWarn } from '../../common/logging';
6+
import { getGlobalPersistentState } from '../../common/persistentState';
7+
import { EventNames } from '../../common/telemetry/constants';
8+
import { sendTelemetryEvent } from '../../common/telemetry/sender';
69
import * as workspaceApis from '../../common/workspace.apis';
710
import { PythonProjectManager, PythonProjectSettings } from '../../internal.api';
811

@@ -576,3 +579,93 @@ export function getSettingUserScope<T>(section: string, key: string): T | undefi
576579
}
577580
return undefined;
578581
}
582+
583+
const MIGRATION_KEY = 'globalSettingsMigration.systemEnvManagerRemoved';
584+
585+
/**
586+
* Returns true if any user-scope slot of the inspection result equals `value`.
587+
* For window-scoped settings VS Code may populate `globalRemoteValue` and/or
588+
* `globalLocalValue` in addition to `globalValue` depending on context.
589+
*/
590+
function userScopeHasValue(inspect: { globalValue?: string } | undefined, value: string): boolean {
591+
if (!inspect) {
592+
return false;
593+
}
594+
const record = inspect as Record<string, unknown>;
595+
if (record.globalRemoteValue === value) {
596+
return true;
597+
}
598+
if (record.globalLocalValue === value) {
599+
return true;
600+
}
601+
if (inspect.globalValue === value) {
602+
return true;
603+
}
604+
return false;
605+
}
606+
607+
/**
608+
* One-time migration: removes `defaultEnvManager` from User (global) settings if it was
609+
* set to `system` by the extension. This was an unintentional side effect of a bug where
610+
* the extension wrote to User scope when no workspace was open. Having `system` at the
611+
* User level causes all workspaces to ignore local .venv environments.
612+
*
613+
* Because `python-envs.defaultEnvManager` is a window-scoped setting, the stale value can
614+
* land in any of `globalValue`, `globalLocalValue`, or `globalRemoteValue` depending on
615+
* which context (local vs remote) hit the bug. We check all three, attempt removal via
616+
* `ConfigurationTarget.Global` (which clears the slot for the current context), then
617+
* re-inspect. If any user-scope slot still holds the stale value we do NOT mark the
618+
* migration complete, so a future activation in the other context can finish the job.
619+
*
620+
* See: https://github.com/microsoft/vscode-python-environments/issues/1468
621+
*/
622+
export async function migrateGlobalDefaultEnvManagerSetting(): Promise<void> {
623+
const state = await getGlobalPersistentState();
624+
const alreadyMigrated = await state.get<boolean>(MIGRATION_KEY);
625+
if (alreadyMigrated) {
626+
return;
627+
}
628+
629+
const config = workspaceApis.getConfiguration('python-envs', undefined);
630+
const inspect = config.inspect<string>('defaultEnvManager');
631+
632+
if (!userScopeHasValue(inspect, SYSTEM_MANAGER_ID)) {
633+
sendTelemetryEvent(EventNames.MIGRATION_SYSTEM_ENV_MANAGER, undefined, { outcome: 'not_set' });
634+
await state.set(MIGRATION_KEY, true);
635+
return;
636+
}
637+
638+
try {
639+
await config.update('defaultEnvManager', undefined, ConfigurationTarget.Global);
640+
} catch (err) {
641+
// Don't mark migration done; we'll retry on a future activation.
642+
traceWarn(
643+
`[migration] Failed to remove 'python-envs.defaultEnvManager: ${SYSTEM_MANAGER_ID}' from User settings: ${err}`,
644+
);
645+
sendTelemetryEvent(EventNames.MIGRATION_SYSTEM_ENV_MANAGER, undefined, {
646+
outcome: 'failed',
647+
errorType: err instanceof Error ? err.name : typeof err,
648+
});
649+
return;
650+
}
651+
652+
// Re-inspect: `update(Global)` only clears the current context's slot. If another
653+
// context's slot (local vs remote) still holds the stale value, leave the flag unset
654+
// so the next activation in that context can clear it.
655+
const after = config.inspect<string>('defaultEnvManager');
656+
if (userScopeHasValue(after, SYSTEM_MANAGER_ID)) {
657+
traceInfo(
658+
`[migration] Partially removed 'python-envs.defaultEnvManager: ${SYSTEM_MANAGER_ID}' from User settings; ` +
659+
`another context still has it set. Will retry on next activation.`,
660+
);
661+
sendTelemetryEvent(EventNames.MIGRATION_SYSTEM_ENV_MANAGER, undefined, { outcome: 'partial' });
662+
return;
663+
}
664+
665+
traceInfo(
666+
`[migration] Removed 'python-envs.defaultEnvManager: ${SYSTEM_MANAGER_ID}' from User settings ` +
667+
`(was set unintentionally by a previous version). See https://github.com/microsoft/vscode-python-environments/issues/1468`,
668+
);
669+
sendTelemetryEvent(EventNames.MIGRATION_SYSTEM_ENV_MANAGER, undefined, { outcome: 'removed' });
670+
await state.set(MIGRATION_KEY, true);
671+
}

0 commit comments

Comments
 (0)