From 10df06d523a0da97895d8523845fc6155796c926 Mon Sep 17 00:00:00 2001 From: Ivan Kulikov Date: Wed, 5 Aug 2026 15:35:29 +0300 Subject: [PATCH] PY-91300 [PPTW] open the tool window on the right interpreter, and render it Follow-up to the PY-91300 fix. On a multi-subproject workspace the Python Packages tool window still got its interpreter wrong on first open, for two independent reasons in PyPackagingToolWindowService.initialize(). Wrong interpreter. The SDK was resolved as project.modules.firstNotNullOfOrNull { it.pythonSdk }, which is only correct in a single-module project. Across independent subprojects it picks whichever module comes first, so with an environment configured for one subproject and then another, the tool window opens on the first one's environment while the editor is on the second -- and only starts agreeing with the editor once the next selection change reaches the FileEditorManagerListener, which is the "switch between subprojects to make it refresh" half of the report. Resolve from the module owning the selected file instead, keeping the module scan as the fallback for when no editor is open. No interpreter at all. The service is a project service and outlives the tool window, so it can already be bound by the time a panel is built -- the install dialog and the pyproject.toml "+ Add package" inlay call initForSdk directly. Handing the same SDK back to initForSdk hits its "same SDK" short-circuit and the new panel learns nothing: no path in the header, no module selection, empty package tree. Replay the rendering half explicitly, but only while the binding agrees with the selected file -- replaying a stale binding would show a foreign subproject's environment through the other path. Both decisions are extracted (resolvePackagesToolWindowSdk, shouldReplayBoundSdk) and covered by unit tests. The resolution test builds two modules with their own mock interpreters and asserts symmetrically in both directions: module order follows temp directory names, so a single case could pass by luck, while under a first-module-wins regression exactly one of the two must fail. Verified against a negative control with the old logic restored -- it fails exactly those two assertions. Also makes toolWindowPanel @Volatile: it is written on EDT and read from background coroutines, so a refresh already in flight when the panel is attached could observe null and drop its render. (cherry picked from commit f41d02a23ef49b47aa46136696a140ab90a90bce) IJ-MR-217170 GitOrigin-RevId: f37cd30fad7043a57b85351ae412df4155824be1 --- .../PyPackagingToolWindowService.kt | 88 +++++++++++++-- .../PyPackagesToolWindowReplayDecisionTest.kt | 47 ++++++++ .../PyPackagesToolWindowSdkResolutionTest.kt | 104 ++++++++++++++++++ 3 files changed, 229 insertions(+), 10 deletions(-) create mode 100644 python/testSrc/com/intellij/python/junit5Tests/unit/packaging/PyPackagesToolWindowReplayDecisionTest.kt create mode 100644 python/testSrc/com/intellij/python/junit5Tests/unit/packaging/PyPackagesToolWindowSdkResolutionTest.kt diff --git a/python/src/com/jetbrains/python/packaging/toolwindow/PyPackagingToolWindowService.kt b/python/src/com/jetbrains/python/packaging/toolwindow/PyPackagingToolWindowService.kt index 5c1173020f64..44e99a89d851 100644 --- a/python/src/com/jetbrains/python/packaging/toolwindow/PyPackagingToolWindowService.kt +++ b/python/src/com/jetbrains/python/packaging/toolwindow/PyPackagingToolWindowService.kt @@ -10,6 +10,7 @@ import com.intellij.openapi.application.readAction import com.intellij.openapi.components.Service import com.intellij.openapi.components.service import com.intellij.openapi.components.serviceAsync +import com.intellij.openapi.fileEditor.FileEditorManager import com.intellij.openapi.fileEditor.FileEditorManagerEvent import com.intellij.openapi.fileEditor.FileEditorManagerListener import com.intellij.openapi.module.Module @@ -91,7 +92,10 @@ import org.jetbrains.annotations.Nls @Service(Service.Level.PROJECT) internal class PyPackagingToolWindowService(val project: Project, val serviceScope: CoroutineScope) : Disposable { - private var toolWindowPanel: PyPackagingToolWindowPanel? = null + // Written on EDT when the tool window builds its content, read from every background coroutine + // here. Volatile like `sdkContext` / `installedPackages`, otherwise a refresh already in flight + // when the panel is attached can still observe `null` and silently drop its render. + @Volatile private var toolWindowPanel: PyPackagingToolWindowPanel? = null @Volatile private var installedPackages: List = emptyList() private var searchJob: Job? = null private var currentQuery: String = "" @@ -121,8 +125,22 @@ internal class PyPackagingToolWindowService(val project: Project, val serviceSco fun initialize(toolWindowPanel: PyPackagingToolWindowPanel) { this.toolWindowPanel = toolWindowPanel serviceScope.launch(Dispatchers.IO) { - val sdk = currentSdk ?: readAction { project.modules.firstNotNullOfOrNull { it.pythonSdk } } - initForSdk(sdk) + val sdkToOpenOn = resolvePackagesToolWindowSdk(project) + val boundSdk = sdkContext?.sdk + if (shouldReplayBoundSdk(boundSdk, sdkToOpenOn)) { + checkNotNull(boundSdk) + publishSdkToPanel(boundSdk) + withContext(Dispatchers.EDT) { + toolWindowPanel.contentVisible = true + // `installedPackages` is already in memory from the earlier binding, so replaying the + // active query paints it into the new panel without a second package-manager round-trip. + // Its terminal `resetSearch` / `showSearchResult` also clears the loading state that + // `publishSdkToPanel` just raised. + handleSearch(currentQuery) + } + return@launch + } + initForSdk(sdkToOpenOn) } } @@ -366,6 +384,24 @@ internal class PyPackagingToolWindowService(val project: Project, val serviceSco } } + /** + * Pushes everything the view derives from the SDK alone: header interpreter path, package-list + * header name and loading state, module list selection. Extracted from [initForSdk] because a + * panel can be attached *after* the service is already bound to that SDK, in which case + * [initForSdk] short-circuits and only this part has to be replayed — see [initialize] + * (PY-91300). The presentation is built off EDT: it probes SDK validity. + */ + private suspend fun publishSdkToPanel(sdk: Sdk) { + val interpreterPath = sdk.pyInterpreterPresentation().fullName + withContext(Dispatchers.EDT) { + toolWindowPanel?.let { + it.startLoadingSdk(sdk.name) + it.setInterpreterPath(interpreterPath) + it.syncSdkControllerSelection(sdk) + } + } + } + @ApiStatus.Internal suspend fun initForSdk(sdk: Sdk?) { if (project.isDisposed) return @@ -388,13 +424,7 @@ internal class PyPackagingToolWindowService(val project: Project, val serviceSco return } - withContext(Dispatchers.EDT) { - toolWindowPanel?.let { - it.startLoadingSdk(sdk.name) - it.setInterpreterPath(sdk.pyInterpreterPresentation().fullName) - it.syncSdkControllerSelection(sdk) - } - } + publishSdkToPanel(sdk) sdkContext = SdkContext( sdk = sdk, @@ -850,3 +880,41 @@ internal class PyPackagingToolWindowService(val project: Project, val serviceSco } } } + +/** + * The interpreter the Python Packages tool window should open on: the one belonging to the module + * that owns the file the user is looking at, falling back to any module's interpreter when the + * editor gives no answer. + * + * Scanning `modules.firstNotNullOfOrNull { it.pythonSdk }` outright is only correct in a + * single-module project. Across independent subprojects it picks whichever module happens to come + * first, so the tool window opens on a foreign subproject's environment while the user is editing + * another one, and only starts agreeing with the editor once the next selection change reaches + * `PyPackagingToolWindowService`'s `FileEditorManagerListener` — the "switch between subprojects to + * make it refresh" half of PY-91300. + */ +internal suspend fun resolvePackagesToolWindowSdk(project: Project): Sdk? = readAction { + val selectedFile = FileEditorManager.getInstance(project).selectedFiles.firstOrNull() + val selectedModule = selectedFile?.let { ModuleUtilCore.findModuleForFile(it, project) } + selectedModule?.let { PythonSdkUtil.findPythonSdk(it) } + ?: project.modules.firstNotNullOfOrNull { it.pythonSdk } +} + +/** + * Whether a tool window attaching to an already-bound [PyPackagingToolWindowService] should have the + * binding replayed into its fresh panel instead of re-binding the service. + * + * The service is a project service and outlives the tool window, so it can already be bound by the + * time a panel is built — the install dialog and the pyproject.toml "+ Add package" inlay call + * `initForSdk` directly, and the editor / roots / `PySdkListener` subscriptions keep that binding + * fresh. Handing the same SDK back to `initForSdk` would hit its "same SDK" short-circuit and the + * new panel would learn nothing at all: no path in the header, no module selection, empty package + * tree. Binding and rendering are separate concerns, so the rendering half is replayed explicitly. + * + * Replaying is only right while the binding agrees with [sdkToOpenOn]. A binding left over from + * another subproject has to be replaced instead, or the tool window would open on a foreign + * environment — and re-binding is safe there precisely because the SDKs differ, so `initForSdk` has + * real work to do (PY-91300). + */ +internal fun shouldReplayBoundSdk(boundSdk: Sdk?, sdkToOpenOn: Sdk?): Boolean = + boundSdk != null && (sdkToOpenOn == null || sdkToOpenOn == boundSdk) diff --git a/python/testSrc/com/intellij/python/junit5Tests/unit/packaging/PyPackagesToolWindowReplayDecisionTest.kt b/python/testSrc/com/intellij/python/junit5Tests/unit/packaging/PyPackagesToolWindowReplayDecisionTest.kt new file mode 100644 index 000000000000..87cbf8e12cd4 --- /dev/null +++ b/python/testSrc/com/intellij/python/junit5Tests/unit/packaging/PyPackagesToolWindowReplayDecisionTest.kt @@ -0,0 +1,47 @@ +// Copyright 2000-2026 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +package com.intellij.python.junit5Tests.unit.packaging + +import com.intellij.openapi.projectRoots.Sdk +import com.jetbrains.python.packaging.toolwindow.shouldReplayBoundSdk +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import org.mockito.Mockito.mock + +/** + * Covers [shouldReplayBoundSdk] — whether a tool window attaching to an already-bound packaging + * service gets the binding replayed into its fresh panel, or re-binds the service instead. + * + * Both mistakes this pins down are real: replaying nothing leaves the header empty on first open, + * and replaying unconditionally shows a leftover subproject's environment (PY-91300). + */ +internal class PyPackagesToolWindowReplayDecisionTest { + private val boundSdk: Sdk = mock(Sdk::class.java) + private val otherSdk: Sdk = mock(Sdk::class.java) + + @Test + fun `replays when the binding is the interpreter to open on`() { + assertTrue(shouldReplayBoundSdk(boundSdk = boundSdk, sdkToOpenOn = boundSdk), + "initForSdk would short-circuit on the same SDK, leaving the new panel blank") + } + + @Test + fun `replays when there is nothing better to open on`() { + assertTrue(shouldReplayBoundSdk(boundSdk = boundSdk, sdkToOpenOn = null), + "An existing binding beats showing no interpreter at all") + } + + @Test + fun `rebinds when the binding belongs to another subproject`() { + assertFalse(shouldReplayBoundSdk(boundSdk = boundSdk, sdkToOpenOn = otherSdk), + "Replaying a stale binding would open the tool window on a foreign environment") + } + + @Test + fun `binds from scratch when the service is not bound yet`() { + assertFalse(shouldReplayBoundSdk(boundSdk = null, sdkToOpenOn = otherSdk), + "There is no binding to replay") + assertFalse(shouldReplayBoundSdk(boundSdk = null, sdkToOpenOn = null), + "There is no binding to replay") + } +} diff --git a/python/testSrc/com/intellij/python/junit5Tests/unit/packaging/PyPackagesToolWindowSdkResolutionTest.kt b/python/testSrc/com/intellij/python/junit5Tests/unit/packaging/PyPackagesToolWindowSdkResolutionTest.kt new file mode 100644 index 000000000000..142a905198f0 --- /dev/null +++ b/python/testSrc/com/intellij/python/junit5Tests/unit/packaging/PyPackagesToolWindowSdkResolutionTest.kt @@ -0,0 +1,104 @@ +// Copyright 2000-2026 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +package com.intellij.python.junit5Tests.unit.packaging + +import com.intellij.openapi.application.EDT +import com.intellij.openapi.fileEditor.FileEditorManager +import com.intellij.openapi.project.Project +import com.intellij.openapi.projectRoots.Sdk +import com.intellij.openapi.vfs.VirtualFileManager +import com.intellij.testFramework.common.timeoutRunBlocking +import com.intellij.testFramework.junit5.TestApplication +import com.intellij.testFramework.junit5.fixture.moduleFixture +import com.intellij.testFramework.junit5.fixture.projectFixture +import com.intellij.testFramework.junit5.fixture.tempPathFixture +import com.jetbrains.python.PythonMockSdk +import com.jetbrains.python.PythonTestUtil +import com.jetbrains.python.junit5.framework.pyMockSdkFixture +import com.jetbrains.python.packaging.toolwindow.resolvePackagesToolWindowSdk +import com.jetbrains.python.psi.LanguageLevel +import com.jetbrains.python.sdk.PythonSdkType +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import org.junit.jupiter.api.Assertions.assertNotNull +import org.junit.jupiter.api.Assertions.assertSame +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import java.nio.file.Path +import kotlin.io.path.writeText + +/** + * Covers [resolvePackagesToolWindowSdk], the interpreter the Python Packages tool window opens on. + * + * Regression test for PY-91300: with an environment configured per subproject, the tool window used + * to open on whichever module came first instead of the one owning the file in the editor. + * + * The two "selected file wins" cases are deliberately symmetric. Module order is not something the + * test controls — the fixtures name modules after temp directories — so a single case could pass by + * luck if its module happened to be scanned first. Under a first-module-wins regression exactly one + * of the two must fail, whichever way the order falls. + */ +@TestApplication +internal class PyPackagesToolWindowSdkResolutionTest { + private val projectFixture = projectFixture(openAfterCreation = true) + + private val firstModulePath = tempPathFixture() + private val secondModulePath = tempPathFixture() + private val firstModule = projectFixture.moduleFixture(firstModulePath, addPathToSourceRoot = true) + private val secondModule = projectFixture.moduleFixture(secondModulePath, addPathToSourceRoot = true) + + // Distinct names matter: a module stores its SDK by name, so same-named mocks would make both + // modules resolve to the same interpreter and the test would prove nothing. + private val firstSdk = projectFixture.pyMockSdkFixture(firstModule) { mockPythonSdk("firstModuleSdk") } + private val secondSdk = projectFixture.pyMockSdkFixture(secondModule) { mockPythonSdk("secondModuleSdk") } + + @Test + fun `resolves the interpreter of the module owning the selected file`(): Unit = timeoutRunBlocking { + val expected = bothInterpretersConfigured().second + openFileIn(secondModulePath.get()) + + assertSame(expected, resolvePackagesToolWindowSdk(projectFixture.get()), + "The tool window must open on the interpreter of the subproject being edited") + } + + @Test + fun `resolves the interpreter of the other module when its file is selected`(): Unit = timeoutRunBlocking { + val expected = bothInterpretersConfigured().first + openFileIn(firstModulePath.get()) + + assertSame(expected, resolvePackagesToolWindowSdk(projectFixture.get()), + "The tool window must open on the interpreter of the subproject being edited") + } + + @Test + fun `falls back to a configured interpreter when no file is open`(): Unit = timeoutRunBlocking { + bothInterpretersConfigured() + + assertNotNull(resolvePackagesToolWindowSdk(projectFixture.get()), + "With no editor to go by, any configured interpreter is better than none") + } + + /** + * Initializes both SDK fixtures. Fixtures are lazy, so without this the module that the test does + * not name would have no interpreter at all and a first-module-wins regression would have nothing + * to pick up. + */ + private suspend fun bothInterpretersConfigured(): Pair = Pair(firstSdk.get(), secondSdk.get()) + + private suspend fun openFileIn(moduleDir: Path) { + val project: Project = projectFixture.get() + val path = moduleDir.resolve("main.py").apply { writeText("") } + val file = withContext(Dispatchers.IO) { + VirtualFileManager.getInstance().refreshAndFindFileByNioPath(path) ?: error("$path is not in VFS") + } + withContext(Dispatchers.EDT) { + FileEditorManager.getInstance(project).openFile(file, true) + } + // Guard the precondition: if the editor manager reports no selection the resolution silently + // falls back to scanning modules, and the assertions below would stop meaning anything. + assertTrue(withContext(Dispatchers.EDT) { FileEditorManager.getInstance(project).selectedFiles.isNotEmpty() }, + "Expected $path to be the selected file") + } + + private fun mockPythonSdk(name: String): Sdk = + PythonMockSdk.create(name, PythonTestUtil.getTestDataPath() + "/MockSdk", PythonSdkType.getInstance(), LanguageLevel.getLatest()) +}