Scope inline-script routing hardening to a PEP 723 per-script guard

Replace the generic cross-manager selection-finalization machinery with a
narrow PEP 723-only routing-mutation guard. Only a current (non-superseded)
inline-routing operation may mutate a script's per-script routing override;
the ordinary project/global active-selection and settings lanes are committed
independently and are no longer suppressed by a stale inline operation.

- Restore the origin/main batch settings construction/write order (no
  committedSelections, no pre-settings revision filter, no post-settings
  recheck) and the one-key commitSelectionOperation.
- Add commitInlineRoutingOperation, gated to inlineScriptRouting + file .py
  scopes via getInlineScriptRoutingKey; feature-off and non-script scopes
  return true immediately and preserve the old flow.
- Guard only the single-URI and batch inline routing blocks. Each script
  commits on its own per-file inline key, so two scripts under the same
  containing project no longer contend on a shared project revision (the
  first script now installs its override instead of skipping it).
- The generic cross-manager settings write race is explicitly deferred.

Tests: adapt the stale-non-inline-vs-newer-inline regression to assert the
script still routes to inline without asserting containing-project
suppression; add a same-project batch non-inline regression and a feature-off
distinct-projects batch regression; remove the batch settings-ordering test.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f881304e-3c5f-4144-9f0d-d80c68768d18

Author: StellaHuang95

src/features/envManagers.ts

@@ -18,6 +18,7 @@ import {
 import {
     InlineScriptRouteabilityChangeEvent,
     InlineScriptRoutingRegistry,
+    getInlineScriptRoutingKey,
 } from '../common/inlineScript/routingRegistry';
 import { traceError, traceVerbose } from '../common/logging';
 import { StopWatch } from '../common/stopWatch';
@@ -420,33 +421,29 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
         }
 
         if (scope instanceof Uri) {
-            const inlineOperation =
-                manager.id === INLINE_SCRIPT_MANAGER_ID
-                    ? operation
-                    : (inlineOverrideHandoffOperation ?? inlineClearOperation);
             if (
-                !this.commitSelectionOperations([
-                    { key, operation },
-                    ...(inlineOperation === undefined
-                        ? []
-                        : [{ key: this.getInlineScriptSelectionKey(scope), operation: inlineOperation }]),
-                ])
-            ) {
-                return;
-            }
-            this.updateInlineRoutingOverride(scope, manager, environment);
-            this.clearInlineActiveSelection(scope, manager, inlineOperation);
-            if (
-                clearingInlineRoutingOverride &&
-                (await this.publishEffectiveEnvironmentAfterOverrideClear(
+                this.commitInlineRoutingOperation(
                     scope,
                     manager,
-                    key,
                     operation,
+                    inlineClearOperation,
                     inlineOverrideHandoffOperation,
-                ))
+                )
             ) {
-                return;
+                this.updateInlineRoutingOverride(scope, manager, environment);
+                this.clearInlineActiveSelection(scope, manager, inlineOverrideHandoffOperation ?? inlineClearOperation);
+                if (
+                    clearingInlineRoutingOverride &&
+                    (await this.publishEffectiveEnvironmentAfterOverrideClear(
+                        scope,
+                        manager,
+                        key,
+                        operation,
+                        inlineOverrideHandoffOperation,
+                    ))
+                ) {
+                    return;
+                }
             }
         }
         if (!publishInlineSelection) {
@@ -507,13 +504,7 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
             if (Array.isArray(scope) && scope.every((s) => s instanceof Uri)) {
                 const selections = scope.map((uri) => this.beginPendingSelection(uri, manager));
                 await manager.set(scope, environment);
-                // Commit the winning (non-superseded) selections BEFORE persisting settings so an
-                // out-of-order older batch cannot write a manager setting to settings.json while its
-                // matching routing/selection update is rejected (settings/routing divergence).
-                const committedSelections = selections.filter((selection) =>
-                    this.commitPendingSelection(selection, manager),
-                );
-                committedSelections.forEach((selection) => {
+                selections.forEach((selection) => {
                     const m = this.getEnvironmentManager(selection.scope);
                     // Always add settings when persisting, OR when manager differs
                     if (
@@ -530,18 +521,27 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
                 if (shouldPersistSettings) {
                     await setAllManagerSettings(settings);
                 }
-                committedSelections.forEach((selection) => {
-                    // Re-validate across the (awaited) settings write: a newer selection may have
-                    // superseded this one while settings were being persisted, in which case its
-                    // routing override and active-selection publish must be skipped.
-                    if (!this.commitPendingSelection(selection, manager)) {
-                        return;
+                selections.forEach((selection) => {
+                    // Only a current (non-superseded) PEP 723 routing operation may mutate the
+                    // per-script override; the ordinary project/global selection lane below is
+                    // committed independently so a stale inline op cannot suppress it.
+                    if (
+                        this.commitInlineRoutingOperation(
+                            selection.scope,
+                            manager,
+                            selection.operation,
+                            selection.inlineClearOperation,
+                        )
+                    ) {
+                        this.updateInlineRoutingOverride(selection.scope, manager, environment);
+                        this.clearInlineActiveSelection(selection.scope, manager, selection.inlineClearOperation);
                     }
-                    this.updateInlineRoutingOverride(selection.scope, manager, environment);
-                    this.clearInlineActiveSelection(selection.scope, manager, selection.inlineClearOperation);
                     if (!selection.publishInlineSelection) {
                         return;
                     }
+                    if (!this.commitSelectionOperation(selection.key, selection.operation)) {
+                        return;
+                    }
                     const oldEnv = this._activeSelection.get(selection.key);
                     if (!this.isSameEnvironment(oldEnv, environment)) {
                         this._activeSelection.set(selection.key, environment);
@@ -955,18 +955,26 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
         }
     }
 
-    private commitPendingSelection(
-        selection: PendingEnvironmentSelection,
+    private commitInlineRoutingOperation(
+        scope: Uri,
         manager: InternalEnvironmentManager,
+        selectionOperation: number,
+        inlineClearOperation?: number,
+        inlineOverrideHandoffOperation?: number,
     ): boolean {
-        const inlineOperation =
-            manager.id === INLINE_SCRIPT_MANAGER_ID ? selection.operation : selection.inlineClearOperation;
-        return this.commitSelectionOperations([
-            { key: selection.key, operation: selection.operation },
-            ...(inlineOperation === undefined
-                ? []
-                : [{ key: this.getInlineScriptSelectionKey(selection.scope), operation: inlineOperation }]),
-        ]);
+        // Gate strictly to the manually enabled PEP 723 routing feature and to file .py scopes.
+        // For the feature-off or non-script case, proceed exactly as before.
+        if (!this.inlineScriptRouting || getInlineScriptRoutingKey(scope) === undefined) {
+            return true;
+        }
+        const operation =
+            manager.id === INLINE_SCRIPT_MANAGER_ID
+                ? selectionOperation
+                : (inlineOverrideHandoffOperation ?? inlineClearOperation);
+        return (
+            operation === undefined ||
+            this.commitSelectionOperation(this.getInlineScriptSelectionKey(scope), operation)
+        );
     }
 
     private canPersistManagerSettingForScope(
@@ -988,24 +996,10 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
     }
 
     private commitSelectionOperation(key: string, operation: number): boolean {
-        return this.commitSelectionOperations([{ key, operation }]);
-    }
-
-    private commitSelectionOperations(
-        operations: readonly { readonly key: string; readonly operation: number }[],
-    ): boolean {
-        const latestByKey = new Map<string, number>();
-        for (const { key, operation } of operations) {
-            latestByKey.set(key, Math.max(latestByKey.get(key) ?? operation, operation));
-        }
-        for (const [key, operation] of latestByKey) {
-            if ((this._selectionRevisions.get(key) ?? 0) > operation) {
-                return false;
-            }
-        }
-        for (const [key, operation] of latestByKey) {
-            this._selectionRevisions.set(key, operation);
+        if ((this._selectionRevisions.get(key) ?? 0) > operation) {
+            return false;
         }
+        this._selectionRevisions.set(key, operation);
         return true;
     }
 

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

@@ -334,14 +334,15 @@ suite('PythonEnvironmentManagers getLastKnownEnvironment', () => {
         const olderSelection = envManagers.setEnvironment(script, selectedEnvironment, false);
         await olderSelectionStarted;
         await envManagers.setEnvironment(script, inlineEnvironment, false);
-        const eventsAfterNewerSelection = [...events];
         releaseOlderSelection!();
         await olderSelection;
 
+        // The stale non-inline selection must not hijack the script's PEP 723 routing…
         assert.strictEqual(envManagers.getEnvironmentManager(script)?.id, inlineId);
         assert.strictEqual(envManagers.getLastKnownEnvironment(script), inlineEnvironment);
-        assert.deepStrictEqual(events, eventsAfterNewerSelection);
-        assert.ok(events.every((event) => event.new !== selectedEnvironment));
+        assert.ok(events.some((event) => event.new === inlineEnvironment));
+        // …but its ordinary containing-project selection lane is independent and is not suppressed.
+        assert.ok(events.some((event) => event.new === selectedEnvironment));
     });
 
     test('does not let an older batch inline selection clear a newer non-inline override', async () => {
@@ -388,6 +389,55 @@ suite('PythonEnvironmentManagers getLastKnownEnvironment', () => {
         assert.deepStrictEqual(events, eventsAfterNewerSelection);
     });
 
+    test('routes each same-project script to its own override in a batch non-inline selection', async () => {
+        const project = { name: 'project', uri: Uri.file('/workspace/project') };
+        const firstScript = Uri.file('/workspace/project/first.py');
+        const secondScript = Uri.file('/workspace/project/second.py');
+        projectsByUri.set(firstScript.toString(), project);
+        projectsByUri.set(secondScript.toString(), project);
+        let selectedEnvironment: PythonEnvironment;
+        const selectedId = registerManager(async () => selectedEnvironment, async () => undefined, 'venv');
+        registerManager(async () => undefined, async () => undefined, 'inline-script');
+        selectedEnvironment = { ...makeEnv('selected'), envId: { id: 'selected', managerId: selectedId } };
+        markInlineScript(firstScript);
+        markInlineScript(secondScript);
+
+        await envManagers.setEnvironments([firstScript, secondScript], selectedEnvironment, false);
+
+        // Each script commits on its own per-file inline key, so the shared containing-project
+        // revision cannot make the first script skip installing its routing override.
+        assert.strictEqual(envManagers.getEnvironmentManager(firstScript)?.id, selectedId);
+        assert.strictEqual(envManagers.getEnvironmentManager(secondScript)?.id, selectedId);
+    });
+
+    test('applies a normal non-inline batch across distinct projects without a routing registry', async () => {
+        recreateEnvManagersWithoutRouting();
+        const projectOne = { name: 'one', uri: Uri.file('/workspace/one') };
+        const projectTwo = { name: 'two', uri: Uri.file('/workspace/two') };
+        projectsByUri.set(projectOne.uri.toString(), projectOne);
+        projectsByUri.set(projectTwo.uri.toString(), projectTwo);
+        const managerSet = sinon.stub().resolves();
+        let selectedEnvironment: PythonEnvironment;
+        const managerId = registerManager(async () => selectedEnvironment, managerSet, 'venv');
+        selectedEnvironment = { ...makeEnv('selected'), envId: { id: 'selected', managerId } };
+        const settings = sinon.stub(settingHelpers, 'setAllManagerSettings').resolves();
+        const events: DidChangeEnvironmentEventArgs[] = [];
+        envManagers.onDidChangeActiveEnvironment((event) => events.push(event));
+
+        await envManagers.setEnvironments([projectOne.uri, projectTwo.uri], selectedEnvironment);
+
+        assert.strictEqual(managerSet.callCount, 1);
+        assert.deepStrictEqual(managerSet.firstCall.args[0], [projectOne.uri, projectTwo.uri]);
+        assert.strictEqual(envManagers.getLastKnownEnvironment(projectOne.uri), selectedEnvironment);
+        assert.strictEqual(envManagers.getLastKnownEnvironment(projectTwo.uri), selectedEnvironment);
+        assert.deepStrictEqual(
+            events.map((event) => event.new),
+            [selectedEnvironment, selectedEnvironment],
+        );
+        assert.strictEqual(settings.callCount, 1);
+        assert.strictEqual(settings.firstCall.args[0].length, 2);
+    });
+
     test('publishes inline environments with the same ID at different paths', async () => {
         const scope = Uri.file('/workspace/script.py');
         const managerId = registerManager(async () => undefined, async () => undefined, 'inline-script');
@@ -967,49 +1017,6 @@ suite('PythonEnvironmentManagers getLastKnownEnvironment', () => {
         ]);
     });
 
-    test('does not persist manager settings for a batch superseded before its settings write', async () => {
-        const script = Uri.file('/workspace/script.py');
-        const scriptProject = { name: 'script.py', uri: script };
-        projectsByUri.set(script.toString(), scriptProject);
-        let releaseStaleBatch: (() => void) | undefined;
-        let signalStaleBatch: (() => void) | undefined;
-        const staleBatchStarted = new Promise<void>((resolve) => {
-            signalStaleBatch = resolve;
-        });
-        const staleBatchGate = new Promise<void>((resolve) => {
-            releaseStaleBatch = resolve;
-        });
-        const managerSet = sinon.stub();
-        managerSet.onFirstCall().callsFake(async () => {
-            signalStaleBatch!();
-            await staleBatchGate;
-        });
-        managerSet.onSecondCall().resolves();
-        const managerId = registerManager(async () => undefined, managerSet, 'inline-script');
-        const staleEnv = { ...makeEnv('stale'), envId: { id: 'stale', managerId } };
-        const newerEnv = { ...makeEnv('newer'), envId: { id: 'newer', managerId } };
-        stubPackageManager();
-        const settings = sinon.stub(settingHelpers, 'setAllManagerSettings').resolves();
-
-        const staleBatch = envManagers.setEnvironments([script], staleEnv);
-        await staleBatchStarted;
-        await envManagers.setEnvironment(script, newerEnv);
-        releaseStaleBatch!();
-        await staleBatch;
-
-        // The newer selection persists the manager setting exactly once.
-        assert.deepStrictEqual(settings.firstCall.args[0], [
-            {
-                project: scriptProject,
-                envManager: managerId,
-                packageManager: 'ms-python.python:pip',
-            },
-        ]);
-        // The superseded batch must not persist a stale selection to settings.json; it writes nothing.
-        assert.strictEqual(settings.callCount, 2);
-        assert.deepStrictEqual(settings.secondCall.args[0], []);
-    });
-
     test('retains an earlier successful refresh when a later refresh fails', async () => {
         const refreshed = makeEnv('refreshed');
         let resolveFirst: ((environment: PythonEnvironment) => void) | undefined;