Skip to content

Commit aed8aee

Browse files
Harden inline routing transitions
Invalidate changed metadata immediately, restore the effective manager after override removal, and align persisted validation with cache invariants. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6b12d843-8011-4bfc-9ba9-f75761eadee2
1 parent 96f38ef commit aed8aee

7 files changed

Lines changed: 420 additions & 60 deletions

File tree

src/common/inlineScript/routingRegistry.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ export class InlineScriptRoutingRegistry implements Disposable {
5555
metadata,
5656
metadataIdentity,
5757
metadataRevision,
58+
validatedAssociation:
59+
state.metadataIdentity === metadataIdentity ? state.validatedAssociation : false,
5860
};
5961
},
6062
true,

src/features/envManagers.ts

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,12 +376,22 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
376376
const project = scope ? this.pm.get(scope) : undefined;
377377
const key = this.getActiveSelectionKey(scope, manager, project);
378378
const operation = this.beginSelectionOperation(key);
379+
const clearingInlineRoutingOverride =
380+
scope instanceof Uri &&
381+
environment === undefined &&
382+
this.getInlineRoutingOverrideManager(scope)?.id === manager.id;
379383
const publishInlineSelection =
380384
!(scope instanceof Uri) || this.shouldPublishInlineSelectionImmediately(scope, manager);
381385
const inlineClearOperation =
382386
scope instanceof Uri && manager.id !== INLINE_SCRIPT_MANAGER_ID
383387
? this.beginSelectionOperation(this.getInlineScriptSelectionKey(scope))
384388
: undefined;
389+
const inlineOverrideHandoffOperation =
390+
clearingInlineRoutingOverride &&
391+
scope instanceof Uri &&
392+
this.inlineScriptRouting?.shouldRoute(scope)
393+
? this.beginSelectionOperation(this.getInlineScriptSelectionKey(scope))
394+
: undefined;
385395
await manager.set(scope, environment);
386396

387397
// Only persist to settings when explicitly requested
@@ -412,6 +422,18 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
412422
if (scope instanceof Uri) {
413423
this.updateInlineRoutingOverride(scope, manager, environment);
414424
this.clearInlineActiveSelection(scope, manager, inlineClearOperation);
425+
if (
426+
clearingInlineRoutingOverride &&
427+
(await this.publishEffectiveEnvironmentAfterOverrideClear(
428+
scope,
429+
manager,
430+
key,
431+
operation,
432+
inlineOverrideHandoffOperation,
433+
))
434+
) {
435+
return;
436+
}
415437
}
416438
if (!publishInlineSelection) {
417439
return;
@@ -782,6 +804,57 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
782804
this._inlineRoutingOverrides.delete(this.getInlineScriptSelectionKey(scope));
783805
}
784806

807+
private async publishEffectiveEnvironmentAfterOverrideClear(
808+
scope: Uri,
809+
previousManager: InternalEnvironmentManager,
810+
previousKey: string,
811+
previousOperation: number,
812+
reservedInlineOperation: number | undefined,
813+
): Promise<boolean> {
814+
const effectiveManager = this.getEnvironmentManager(scope);
815+
if (!effectiveManager || effectiveManager.id === previousManager.id) {
816+
return false;
817+
}
818+
if (!this.commitSelectionOperation(previousKey, previousOperation)) {
819+
return true;
820+
}
821+
822+
const oldEnvironment = this._activeSelection.get(previousKey);
823+
const project = this.pm.get(scope);
824+
const effectiveKey = this.getActiveSelectionKey(scope, effectiveManager, project);
825+
const effectiveOperation =
826+
effectiveManager.id === INLINE_SCRIPT_MANAGER_ID && reservedInlineOperation !== undefined
827+
? reservedInlineOperation
828+
: this.beginSelectionOperation(effectiveKey);
829+
if (!this.isLatestSelectionOperation(effectiveKey, effectiveOperation)) {
830+
return true;
831+
}
832+
const newEnvironment = await effectiveManager.get(scope);
833+
if (
834+
this.getEnvironmentManager(scope) !== effectiveManager ||
835+
!this.isLatestSelectionOperation(effectiveKey, effectiveOperation) ||
836+
!this.commitSelectionOperation(effectiveKey, effectiveOperation)
837+
) {
838+
return true;
839+
}
840+
841+
this._activeSelection.set(effectiveKey, newEnvironment);
842+
if (!this.isSameEnvironment(oldEnvironment, newEnvironment)) {
843+
await this.fireActiveEnvironmentEvents([
844+
{
845+
uri: this.getActiveSelectionUri(scope, effectiveManager, project),
846+
old: oldEnvironment,
847+
new: newEnvironment,
848+
},
849+
]);
850+
}
851+
return true;
852+
}
853+
854+
private isLatestSelectionOperation(key: string, operation: number): boolean {
855+
return (this._selectionOperationCounters.get(key) ?? 0) === operation;
856+
}
857+
785858
private async handleInlineScriptRouteabilityChange(
786859
event: InlineScriptRouteabilityChangeEvent,
787860
): Promise<void> {

src/managers/builtin/inlineScript/envManager.ts

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1155,11 +1155,12 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable {
11551155
if (metadataMatch === 'mismatched') {
11561156
return undefined;
11571157
}
1158-
const metadataIdentityProven = await this.currentCacheEntryProvesSourceMetadataIdentity(
1159-
resolved,
1160-
metadataIdentity,
1161-
metadata,
1162-
);
1158+
const sidecar = await this.readCurrentCacheEntrySidecar(resolved);
1159+
if (sidecar && !this.cacheEntryMatchesRuntimeAndMetadata(sidecar, resolved, metadata)) {
1160+
return undefined;
1161+
}
1162+
const metadataIdentityProven =
1163+
!!sidecar && this.cacheEntryProvesSourceMetadataIdentity(sidecar, resolved, metadataIdentity, metadata);
11631164
if (!this.isCurrentAssociationRevision(scriptPath, revision)) {
11641165
return this.fsPathToEnv.get(scriptPath);
11651166
}
@@ -1335,11 +1336,12 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable {
13351336
if (metadataMatch === 'mismatched') {
13361337
return undefined;
13371338
}
1338-
const metadataIdentityProven = await this.currentCacheEntryProvesSourceMetadataIdentity(
1339-
resolved,
1340-
metadataIdentity,
1341-
metadata,
1342-
);
1339+
const sidecar = await this.readCurrentCacheEntrySidecar(resolved);
1340+
if (sidecar && !this.cacheEntryMatchesRuntimeAndMetadata(sidecar, resolved, metadata)) {
1341+
return undefined;
1342+
}
1343+
const metadataIdentityProven =
1344+
!!sidecar && this.cacheEntryProvesSourceMetadataIdentity(sidecar, resolved, metadataIdentity, metadata);
13431345
if (!this.isCurrentAssociationRevision(scriptPath, revision)) {
13441346
return this.fsPathToEnv.get(scriptPath);
13451347
}
@@ -1624,12 +1626,27 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable {
16241626
metadataIdentity: string,
16251627
metadata: InlineScriptMetadata,
16261628
): boolean {
1629+
if (!this.cacheEntryMatchesRuntimeAndMetadata(sidecar, environment, metadata)) {
1630+
return false;
1631+
}
16271632
return (
16281633
this.sidecarProvesSourceMetadataIdentity(sidecar, metadataIdentity) ||
16291634
this.isMetadataOnlyCacheEntryForMetadata(sidecar, environment, metadata)
16301635
);
16311636
}
16321637

1638+
private cacheEntryMatchesRuntimeAndMetadata(
1639+
sidecar: InlineScriptEnvMeta,
1640+
environment: PythonEnvironment,
1641+
metadata: InlineScriptMetadata,
1642+
): boolean {
1643+
if (!this.areEqualPythonReleases(environment.version, sidecar.baseInterpreterVersion)) {
1644+
return false;
1645+
}
1646+
const requiresPython = metadata.requiresPython?.trim();
1647+
return !requiresPython || this.matchesInstallConstraint(requiresPython, environment.version);
1648+
}
1649+
16331650
private async resolveVerifiedSourceMetadataIdentity(
16341651
script: ScriptReference,
16351652
environment: PythonEnvironment,
@@ -1976,7 +1993,9 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable {
19761993
if (!sourceMetadataIdentity) {
19771994
return {
19781995
environmentPath,
1979-
metadataBinding: { kind: 'legacy' },
1996+
metadataBinding: currentMetadataIdentity
1997+
? { kind: 'pending', sourceIdentity: currentMetadataIdentity }
1998+
: { kind: 'legacy' },
19801999
};
19812000
}
19822001
return {

src/test/common/inlineScript/routingRegistry.unit.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,41 @@ const METADATA = {
1515
};
1616

1717
suite('InlineScriptRoutingRegistry', () => {
18+
test('invalidates a validated association synchronously when metadata identity changes', () => {
19+
const registry = new InlineScriptRoutingRegistry();
20+
const uri = Uri.file('/workspace/script.py');
21+
const routeabilityEvents: boolean[] = [];
22+
registry.onDidChangeRouteability((event) => routeabilityEvents.push(event.routeable));
23+
registry.setMetadata(uri, METADATA);
24+
registry.setValidatedAssociation(uri, true);
25+
26+
registry.setMetadata(uri, {
27+
...METADATA,
28+
dependencies: ['httpx'],
29+
});
30+
31+
assert.strictEqual(registry.hasValidatedAssociation(uri), false);
32+
assert.strictEqual(registry.shouldRoute(uri), false);
33+
assert.deepStrictEqual(routeabilityEvents, [true, false]);
34+
registry.dispose();
35+
});
36+
37+
test('preserves validation when saved metadata has the same routing identity', () => {
38+
const registry = new InlineScriptRoutingRegistry();
39+
const uri = Uri.file('/workspace/script.py');
40+
registry.setMetadata(uri, METADATA);
41+
registry.setValidatedAssociation(uri, true);
42+
43+
registry.setMetadata(uri, {
44+
...METADATA,
45+
dependencies: ['Requests'],
46+
});
47+
48+
assert.strictEqual(registry.hasValidatedAssociation(uri), true);
49+
assert.strictEqual(registry.shouldRoute(uri), true);
50+
registry.dispose();
51+
});
52+
1853
test('keeps metadata revisions monotonic after an empty state is removed', () => {
1954
const registry = new InlineScriptRoutingRegistry();
2055
const uri = Uri.file('/workspace/script.py');

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

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,107 @@ suite('PythonEnvironmentManagers getLastKnownEnvironment', () => {
535535
assert.strictEqual(envManagers.getLastKnownEnvironment(script), selectedEnvironment);
536536
});
537537

538+
test('publishes through the inline manager after an explicit override is cleared', async () => {
539+
const script = Uri.file('/workspace/project/script.py');
540+
projectsByUri.set(script.toString(), { name: 'project', uri: Uri.file('/workspace/project') });
541+
const selectedSet = sinon.stub().resolves();
542+
let selectedEnvironment: PythonEnvironment;
543+
const selectedId = registerManager(async () => selectedEnvironment, selectedSet, 'venv');
544+
let inlineEnvironment: PythonEnvironment;
545+
const inlineGet = sinon.stub().callsFake(async () => inlineEnvironment);
546+
const inlineId = registerManager(inlineGet, async () => undefined, 'inline-script');
547+
selectedEnvironment = { ...makeEnv('selected'), envId: { id: 'selected', managerId: selectedId } };
548+
inlineEnvironment = { ...makeEnv('inline'), envId: { id: 'inline', managerId: inlineId } };
549+
defaultManagerId = selectedId;
550+
markInlineScript(script);
551+
await envManagers.setEnvironment(script, inlineEnvironment, false);
552+
await envManagers.setEnvironment(script, selectedEnvironment, false);
553+
await new Promise((resolve) => setImmediate(resolve));
554+
selectedSet.resetHistory();
555+
inlineGet.resetHistory();
556+
const events: DidChangeEnvironmentEventArgs[] = [];
557+
envManagers.onDidChangeActiveEnvironment((event) => events.push(event));
558+
559+
await envManagers.setEnvironment(script, undefined, false);
560+
561+
sinon.assert.calledOnceWithExactly(selectedSet, script, undefined);
562+
sinon.assert.calledOnceWithExactly(inlineGet, script);
563+
assert.strictEqual(envManagers.getEnvironmentManager(script)?.id, inlineId);
564+
assert.strictEqual(envManagers.getLastKnownEnvironment(script), inlineEnvironment);
565+
assert.deepStrictEqual(events, [
566+
{
567+
uri: script,
568+
old: selectedEnvironment,
569+
new: inlineEnvironment,
570+
},
571+
]);
572+
});
573+
574+
test('does not let override clearing overwrite a newer inline selection', async () => {
575+
const script = Uri.file('/workspace/project/script.py');
576+
projectsByUri.set(script.toString(), { name: 'project', uri: Uri.file('/workspace/project') });
577+
let releaseOverrideClear: (() => void) | undefined;
578+
let signalOverrideClear: (() => void) | undefined;
579+
const overrideClearStarted = new Promise<void>((resolve) => {
580+
signalOverrideClear = resolve;
581+
});
582+
const overrideClearGate = new Promise<void>((resolve) => {
583+
releaseOverrideClear = resolve;
584+
});
585+
const selectedSet = sinon.stub();
586+
selectedSet.onFirstCall().resolves();
587+
selectedSet.onSecondCall().callsFake(async () => {
588+
signalOverrideClear!();
589+
await overrideClearGate;
590+
});
591+
let selectedEnvironment: PythonEnvironment;
592+
const selectedId = registerManager(async () => selectedEnvironment, selectedSet, 'venv');
593+
594+
let releaseNewInline: (() => void) | undefined;
595+
let signalNewInline: (() => void) | undefined;
596+
const newInlineStarted = new Promise<void>((resolve) => {
597+
signalNewInline = resolve;
598+
});
599+
const newInlineGate = new Promise<void>((resolve) => {
600+
releaseNewInline = resolve;
601+
});
602+
const inlineSet = sinon.stub();
603+
inlineSet.onFirstCall().resolves();
604+
inlineSet.onSecondCall().callsFake(async () => {
605+
signalNewInline!();
606+
await newInlineGate;
607+
});
608+
let inlineEnvironment: PythonEnvironment;
609+
const inlineId = registerManager(async () => inlineEnvironment, inlineSet, 'inline-script');
610+
const oldInline = {
611+
...makeEnv('old-inline'),
612+
envId: { id: 'old-inline', managerId: inlineId },
613+
};
614+
const newInline = {
615+
...makeEnv('new-inline'),
616+
envId: { id: 'new-inline', managerId: inlineId },
617+
};
618+
inlineEnvironment = oldInline;
619+
selectedEnvironment = { ...makeEnv('selected'), envId: { id: 'selected', managerId: selectedId } };
620+
defaultManagerId = selectedId;
621+
markInlineScript(script);
622+
await envManagers.setEnvironment(script, oldInline, false);
623+
await envManagers.setEnvironment(script, selectedEnvironment, false);
624+
625+
const clearOverride = envManagers.setEnvironment(script, undefined, false);
626+
await overrideClearStarted;
627+
inlineEnvironment = newInline;
628+
const newerSelection = envManagers.setEnvironment(script, newInline, false);
629+
await newInlineStarted;
630+
releaseOverrideClear!();
631+
await clearOverride;
632+
releaseNewInline!();
633+
await newerSelection;
634+
635+
assert.strictEqual(envManagers.getEnvironmentManager(script)?.id, inlineId);
636+
assert.strictEqual(envManagers.getLastKnownEnvironment(script), newInline);
637+
});
638+
538639
test('clears inline routing after a no-op inline refresh during settings persistence', async () => {
539640
const script = Uri.file('/workspace/project/script.py');
540641
projectsByUri.set(script.toString(), { name: 'project', uri: Uri.file('/workspace/project') });

0 commit comments

Comments
 (0)