Repository navigation
Keep Python sessions in sync when a workspace .venv is deleted or recreated - #16431
dhruvisompura wants to merge 15 commits into
Conversation
Deleting a venv and recreating it with a different Python version reaches the runtime manager as an update to the same interpreter path, not as a removal and an addition, because the file watcher only reports the deletion of the .venv folder. The manager ignored same-path updates, so the old version stayed registered and sessions started or restarted from it showed the old version while running the new interpreter. Re-resolve the path on a same-path update. If the runtime changed, replace the registered runtime and shut down sessions still backed by the old one.
Deleting a folder is reported by the file watcher as the deletion of that folder, not of each file inside it. Deleting .venv often reports only .venv and .venv/bin, which don't match the watcher's **/python pattern, so the env stayed registered: its session kept running and the picker kept offering it. Watch the workspace for deletes of any path, and remove the envs whose executable was inside the deleted path. The extra watcher sends the same watch request as the existing one, so it adds no file system watcher. Also skip resolving an executable that no longer exists. PET can still resolve a deleted executable, and the lookups that follow a deletion added the env straight back.
|
E2E Tests 🚀 Why these tags?
More on automatic tags from changed files. |
|
/test |
|
🔎 Exploratory testing 542eb9f Not run: this branch predates the drive-positron helper update that exploratory runs need. Rebase it onto main and comment /explore again. |
The same-path branch only acted when a runtime was already registered for the path. Runtimes that came from the workspace recommendation are never added to that registry, so a venv recreated with another Python could leave its old session running with the old version. Discovery and a live resolve can also disagree on the patch version of the same interpreter, which would have shut down a working session. Act when the major or minor version changes, without consulting the registry, and log what the handler decided. The recreateRuntime branch now uses the same shutdown helper. Report folder deletes through the existing workspace event instead of a new one, so pythonWatcher.ts keeps a single Positron block.
|
/test |
|
🔎 Exploratory testing d60beab 1 moderate |
The nativeAPI tests fire the workspace event on a mocked watcher, so they pass even if the deletes-only watcher is removed. Pin the watcher itself: a deleted .venv folder must come through as a Deleted workspace event.
|
/pete |
PETE's assessment 🧪Verdict: 🟢 Adequate -- Every source branch in this three-file fix has a matching extension-host unit test that asserts the exact new behavior, including the #4556 regression. What changed
Tests in this PR
Existing coverageNew tests are the primary coverage and each maps to a changed branch:
Suggested additionsNone. Deployment note (optional)The fix centers on file-watcher delete semantics, a known Windows/web hotspot (file watchers use different backends; web has no direct fs). The unit tests stub the watcher, so they don't exercise real platform behavior. The PR body tags PETE (Positron Extreme Test Experiment) - LLM-based test-coverage advisor, in pilot. Runs only on demand -- comment |
|
This also fixes #16191 BEFORE 2-main.mp4AFTER 1-with-16431.mp4 |
There was a problem hiding this comment.
when running in a workspace that already has a venv: Python: Create Environment... > Delete and Recreate, I am consistently getting this modal. I don't see any errors in the python language pack or kernel supervisor, but my suspicion is there's a race condition somewhere that is conflicting with the venv watcher
In other use cases, including the uv sync, this works super well, I'm excited that this feels less clunky! 🙌
Ah, thanks for finding this issue! I didn't think to test the |
On Linux the file watcher reports a new folder but not the files created inside it, so after .venv was deleted and recreated, nothing saw the new bin/python. The folder delete removed the env, and it stayed out of both interpreter pickers until the next full discovery. A venv that was the selected interpreter or had a console was unaffected, because another lookup re-added it. When a folder delete removes a workspace env, watch the folders between the workspace and its executable. Once one is created, wait briefly for the executable to appear and add the env back. addEnv now checks the list again after its last await, so the folder watcher and the executable watcher reporting the same recreate add the env once instead of twice. The e2e test runs the real file watcher, which is where Linux differs.
|
Okay, I clean-slated everything and can no longer reproduce that Create Environment error 💀 if I see it again, I can open up a follow up! |
| private async _doResolveEnv(envPath: string): Promise<PythonEnvInfo | undefined> { | ||
| // PET can resolve an executable that no longer exists, which would add a | ||
| // just-deleted env straight back. | ||
| if (!(await pathExists(envPath))) { |
There was a problem hiding this comment.
The PET can also resolve from PATH, which means things like python are legitimate interpreters, even though they would not pass here. Easy fix here would be to only run this when path.isAbsolute(envPath), so deleted-venv protection stays and command names still go to PET.
| checking = true; | ||
| try { | ||
| // The executable can lag its folder by a moment while the venv is written. | ||
| for (let attempt = 0; attempt < 10; attempt += 1) { |
There was a problem hiding this comment.
It would be safer to keep the retry loop going until resolving/adding the environment succeeds, not just until the executable file exists.
🤖 suggestion is to dispose the temporary watcher after finder.resolve() returns an environment and addEnv() has had a chance to add it.
|
@isabelizimm testing the Use Existing: 1-use-existing.mp4Delete and Recreate (same Python version): 2-delete-and-recreate.mp4I'm least sure about the changes I made to And here's a quick summary of the three commits for you
26a429a Don't shut down the same Python session twice
afcc654 Start a new session after recreating a Python environment
c944a03 Keep the session running when Create Environment reuses a venv
|
d432a4b to
724927d
Compare
724927d to
2cdb448
Compare
| * removed env, this watches every folder between the workspace and the executable, and | ||
| * looks the executable up once one of them is created. | ||
| */ | ||
| export class RecreatedEnvWatcher implements Disposable { |
There was a problem hiding this comment.
we may want an eventual timeout for this watcher, since not every deleted venv will get recreated
2 participants
Fixes #4556
Fixes #16191
If you delete a workspace
.venvand recreate it with a different Python version while Positron is open, the console and the session picker keep showing the old version. Restarting the console reports "Python 3.12" while it actually runs 3.11.The cause is how the file watcher reports a deleted folder: it sends one delete for the folder, not one for each file inside it. So the Python extension got one of two things:
.venv/binand.venv. Neither matches the**/pythonpattern the extension watches, so the deletion went unnoticed.Either way, the old runtime stayed registered. New sessions and restarts used its name and version, while launching whatever binary was now at that path. Both cases showed up in real runs.
Changes
manager.ts). When the interpreter at a path changes major or minor version, the runtime manager registers the path again and shuts down sessions still on the old runtime. A patch-only change is ignored, because discovery and a live lookup can disagree on the patch version of the same venv. The handler logs what it decided either way.pythonWatcher.ts,nativeAPI.ts).**/pythonwatcher, so VS Code merges the two and starts no extra file system watcher.resolveEnvnow skips a path that doesn't exist. PET still resolves a just-deleted executable. Without this check, the lookups that follow a deletion added the env straight back, as "Python 3.12.14 (Unknown)"..venv/bin/pythonwas never seen and the venv dropped out of both interpreter pickers. When a delete removes a workspace env,nativeAPI.tsnow watches the folders between the workspace and its executable. Once one is created, it waits briefly for the executable and adds the env back.addEnvalso checks the list again after its last await, so two reports of the same recreate add the env once.The folder-delete fix is its own commit (542eb9f), and so is the Linux re-add (ed127af), in case reviewers want either split out.
Both upstream files keep their changes inside Positron markers.
Behavior change: after the swap or the deletion, the console session on the old interpreter shuts down. This already happened when Positron noticed a deleted interpreter. Nothing new starts on its own.
Notes for review
hasChangedinnativeAPI.ts), so the extra lookup is rare.~/.virtualenvs. The new watcher only watches workspace folders.Videos
Recreating
.venvwith Python 3.11:4556-venv-version-change.mp4
Deleting
.venv:4556-venv-deleted.mp4
Release Notes
New Features
Bug Fixes
.venvwith a different Python version no longer leaves the console and session picker showing the old version (Positron reports the wrong Python version if you change the version used in a project #4556).venvnow shuts down its session and removes it from the session picker (Positron reports the wrong Python version if you change the version used in a project #4556)Validation Steps
@:interpreter @:sessions @:new-folder-flow
uv venv -p 3.12 && uv pip install ipykernel), and start its Python session.rm -rf .venv && uv venv -p 3.11 && uv pip install ipykernel.import sys; sys.version. The label and the output should both say 3.11.rm -rf .venv. The session shuts down, and the session picker no longer lists the folder's venv.rm -rf .venv && uv venv .venv. The session picker still lists the folder's venv. The new e2e testpython-venv-recreated.test.tscovers this.