Address follow-up persistence review

Publish same-path version rebuilds, emit completed multi-manager unset groups consistently, and preserve cold associations across transient rehydration failures.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 1a9f6ba1-9bd3-4664-bc25-a0d34d7a2e91

Author: StellaHuang95

src/features/envManagers.ts

@@ -522,7 +522,6 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
                 });
             }
         } else {
-            const events: DidChangeEnvironmentEventArgs[] = [];
             if (Array.isArray(scope) && scope.every((s) => s instanceof Uri)) {
                 const groupedScopes = new Map<InternalEnvironmentManager, Uri[]>();
                 scope.forEach((uri) => {
@@ -532,6 +531,7 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
                     }
                 });
                 for (const [manager, uris] of groupedScopes) {
+                    const events: DidChangeEnvironmentEventArgs[] = [];
                     const selections = uris.map((uri) => this.beginPendingSelection(uri, manager));
                     await manager.set(uris);
                     await Promise.all(
@@ -551,8 +551,10 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
                             }
                         }),
                     );
+                    await this.fireActiveEnvironmentEvents(events);
                 }
             } else if (typeof scope === 'string' && scope === 'global') {
+                const events: DidChangeEnvironmentEventArgs[] = [];
                 const manager = this.getEnvironmentManager(undefined);
                 if (manager) {
                     const operation = this.beginSelectionOperation('global');
@@ -566,18 +568,7 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
                         }
                     }
                 }
-            }
-            if (events.length > 0) {
-                await new Promise<void>((resolve, reject) => {
-                    setImmediate(() => {
-                        try {
-                            events.forEach((e) => this._onDidChangeActiveEnvironment.fire(e));
-                            resolve();
-                        } catch (err) {
-                            reject(err);
-                        }
-                    });
-                });
+                await this.fireActiveEnvironmentEvents(events);
             }
         }
     }
@@ -768,10 +759,27 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
             return false;
         }
         return first.envId.managerId === INLINE_SCRIPT_MANAGER_ID
-            ? normalizePath(first.environmentPath.fsPath) === normalizePath(second.environmentPath.fsPath)
+            ? normalizePath(first.environmentPath.fsPath) === normalizePath(second.environmentPath.fsPath) &&
+                  first.version === second.version
             : first.envId.id === second.envId.id;
     }
 
+    private async fireActiveEnvironmentEvents(events: readonly DidChangeEnvironmentEventArgs[]): Promise<void> {
+        if (events.length === 0) {
+            return;
+        }
+        await new Promise<void>((resolve, reject) => {
+            setImmediate(() => {
+                try {
+                    events.forEach((event) => this._onDidChangeActiveEnvironment.fire(event));
+                    resolve();
+                } catch (error) {
+                    reject(error);
+                }
+            });
+        });
+    }
+
     getProjectEnvManagers(uris: Uri[]): InternalEnvironmentManager[] {
         const projectEnvManagers: InternalEnvironmentManager[] = [];
         uris.forEach((uri) => {

src/managers/builtin/inlineScript/envManager.ts

@@ -570,13 +570,21 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable {
             return undefined;
         }
 
-        const resolved = await resolveVenvPythonEnvironmentPath(
-            environmentPath,
-            this.nativeFinder,
-            this.api,
-            this,
-            this.baseManager,
-        );
+        let resolved: PythonEnvironment | undefined;
+        try {
+            resolved = await resolveVenvPythonEnvironmentPath(
+                environmentPath,
+                this.nativeFinder,
+                this.api,
+                this,
+                this.baseManager,
+            );
+        } catch (error) {
+            this.log.warn(
+                `Unable to resolve persisted inline-script environment ${environmentPath}: ${getErrorMessage(error)}`,
+            );
+            return undefined;
+        }
         if (!resolved) {
             // PET/API resolution can fail transiently. Keep the association for a later retry.
             return undefined;
@@ -585,7 +593,15 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable {
         if (!this.isCurrentAssociationRevision(scriptPath, revision)) {
             return this.fsPathToEnv.get(scriptPath);
         }
-        const ownership = await this.inspectAssociationOwnership(resolved);
+        let ownership: CacheEnvironmentInspection;
+        try {
+            ownership = await this.inspectAssociationOwnership(resolved);
+        } catch (error) {
+            this.log.warn(
+                `Unable to inspect persisted inline-script environment ${environmentPath}: ${getErrorMessage(error)}`,
+            );
+            return undefined;
+        }
         if (ownership === 'stale') {
             await this.removeStalePersistedAssociation(scriptPath, environmentPath, revision, scriptUri);
             return undefined;

src/test/features/envManagers.lastKnown.unit.test.ts

@@ -301,6 +301,68 @@ suite('PythonEnvironmentManagers getLastKnownEnvironment', () => {
         assert.deepStrictEqual(events.map((event) => event.new), [first, second]);
     });
 
+    test('publishes same-path inline rebuilds but ignores generated-ID-only changes', async () => {
+        const scope = Uri.file('/workspace/script.py');
+        const managerId = registerManager(async () => undefined, async () => undefined, 'inline-script');
+        const environmentPath = Uri.file('/env/inline/python');
+        const first = {
+            ...makeEnv('first'),
+            envId: { id: 'first', managerId },
+            environmentPath,
+            version: '3.12.0',
+        };
+        const regenerated = {
+            ...first,
+            envId: { id: 'regenerated', managerId },
+        };
+        const rebuilt = {
+            ...regenerated,
+            envId: { id: 'rebuilt', managerId },
+            version: '3.13.0',
+        };
+        const events: DidChangeEnvironmentEventArgs[] = [];
+        envManagers.onDidChangeActiveEnvironment((event) => events.push(event));
+
+        await envManagers.setEnvironment(scope, first, false);
+        await envManagers.setEnvironment(scope, regenerated, false);
+        await envManagers.setEnvironment(scope, rebuilt, false);
+
+        assert.strictEqual(envManagers.getLastKnownEnvironment(scope), rebuilt);
+        assert.deepStrictEqual(events.map((event) => event.new), [first, rebuilt]);
+    });
+
+    test('publishes completed manager groups before a later group rejects', async () => {
+        const firstScope = Uri.file('/workspace/first.py');
+        const secondScope = Uri.file('/workspace/second.py');
+        const firstProject = { name: 'first.py', uri: firstScope };
+        const secondProject = { name: 'second.py', uri: secondScope };
+        projectsByUri.set(firstScope.toString(), firstProject);
+        projectsByUri.set(secondScope.toString(), secondProject);
+        const firstSet = sinon.stub().resolves();
+        const firstId = registerManager(async () => undefined, firstSet, 'first-manager');
+        const secondSet = sinon.stub();
+        secondSet.onFirstCall().resolves();
+        secondSet.onSecondCall().rejects(new Error('second group rejected'));
+        const secondId = registerManager(async () => undefined, secondSet, 'second-manager');
+        const firstEnvironment = { ...makeEnv('first'), envId: { id: 'first', managerId: firstId } };
+        const secondEnvironment = { ...makeEnv('second'), envId: { id: 'second', managerId: secondId } };
+        await envManagers.setEnvironment(firstScope, firstEnvironment, false);
+        await envManagers.setEnvironment(secondScope, secondEnvironment, false);
+        exactManagerSettings.set(firstScope.toString(), firstId);
+        exactManagerSettings.set(secondScope.toString(), secondId);
+        const events: DidChangeEnvironmentEventArgs[] = [];
+        envManagers.onDidChangeActiveEnvironment((event) => events.push(event));
+
+        await assert.rejects(
+            envManagers.setEnvironments([firstScope, secondScope], undefined, false),
+            /second group rejected/,
+        );
+
+        assert.strictEqual(envManagers.getLastKnownEnvironment(firstScope), undefined);
+        assert.strictEqual(envManagers.getLastKnownEnvironment(secondScope), secondEnvironment);
+        assert.deepStrictEqual(events, [{ uri: firstScope, old: firstEnvironment, new: undefined }]);
+    });
+
     test('tracks inline-script selections independently for scripts in the same project', async () => {
         const firstUri = Uri.file('/workspace/first.py');
         const secondUri = Uri.file('/workspace/second.py');

src/test/managers/builtin/inlineScript/envManager.unit.test.ts

@@ -1573,6 +1573,40 @@ suite('InlineScriptEnvManager', () => {
             restarted.dispose();
         });
 
+        test('preserves and retries a cold association when resolution rejects', async () => {
+            const uri = scriptUri();
+            const environment = await createOwnedEnvironment();
+            persistedAssociations = { [normalizePath(uri.fsPath)]: environment.environmentPath.fsPath };
+            resolveVenvStub.onFirstCall().rejects(new Error('resolver unavailable'));
+            resolveVenvStub.onSecondCall().resolves(environment);
+
+            assert.strictEqual(await manager.get(uri), undefined);
+            assert.deepStrictEqual(persistedAssociations, {
+                [normalizePath(uri.fsPath)]: environment.environmentPath.fsPath,
+            });
+            assert.strictEqual(await manager.get(uri), environment);
+        });
+
+        test('preserves and retries a cold association when ownership inspection rejects', async () => {
+            const uri = scriptUri();
+            const environment = await createOwnedEnvironment();
+            persistedAssociations = { [normalizePath(uri.fsPath)]: environment.environmentPath.fsPath };
+            resolveVenvStub.resolves(environment);
+            const inspectionManager = manager as unknown as {
+                inspectAssociationOwnership(
+                    candidate: PythonEnvironment,
+                ): Promise<'expected' | 'stale' | 'uncertain'>;
+            };
+            const ownershipStub = sinon.stub(inspectionManager, 'inspectAssociationOwnership').callThrough();
+            ownershipStub.onFirstCall().rejects(new Error('filesystem unavailable'));
+
+            assert.strictEqual(await manager.get(uri), undefined);
+            assert.deepStrictEqual(persistedAssociations, {
+                [normalizePath(uri.fsPath)]: environment.environmentPath.fsPath,
+            });
+            assert.strictEqual(await manager.get(uri), environment);
+        });
+
         test('notifies when a slow persisted association finishes rehydrating', async () => {
             const uri = scriptUri();
             const environment = await createOwnedEnvironment();