Skip to content

Fix PEP 723 cache drift and support deleting individual script environments - #1780

Merged
Stella Huang (StellaHuang95) merged 4 commits into
microsoft:mainfrom
StellaHuang95:fix/pep723-inline-script-ux
Sep 11, 2026
Merged

Fix PEP 723 cache drift and support deleting individual script environments#1780
Stella Huang (StellaHuang95) merged 4 commits into
microsoft:mainfrom
StellaHuang95:fix/pep723-inline-script-ux

Conversation

@StellaHuang95

@StellaHuang95 Stella Huang (StellaHuang95) commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Part of #1602. Design: #1601. Follow-up to #1772.

Summary

Fix PEP 723 inline-script cache and routing issues, make cached environments read-only through the package-management UI, and support deleting one cached environment without clearing the entire script cache.

The feature remains behind the internal, undeclared python-envs.inlineScripts.enabled flag, which defaults to false and is latched at activation. This does not enable inline scripts by default or add automatic installation on Run/Debug.

Commits and user-visible changes

Commit Problem or missing behavior Change
64960273 The first package enumeration could be mistaken for an installation, so expanding Packages or refreshing inventory invalidated a working script environment. Case-only Windows renames could also lose the association. Replace package-event-based drift detection with a recorded inventory hash; hide package mutation actions and reject the palette install path for inline environments; preserve Windows case-only rename associations, including dirty aliases and pending reads.
669ffe18 A lookup could detect drift and return no environment while routing still advertised the cache as valid and the setup CodeLens remained hidden. Other scripts sharing the entry could retain stale state. Confirm drift against the current entry, invalidate affected routing/cached validation, and notify environment consumers without requiring a save. Protect newer repairs and selections from stale observations.
1d3ea92a Delete Environment was offered in the Projects view, but the inline manager did not implement removal and returned "Remove Environment not supported". Delete the selected cache entry without a confirmation dialog, clear its known workspace associations, and update the environment collection. Preserve script files, project entries/settings, the base interpreter, and unrelated cache entries.

Implementation and behavior notes

Record package inventory instead of interpreting package events

  • When inventory is readable, setup records an installedPackagesHash in the entry's .meta.json, under the existing cache-entry lock. The hash is computed from sorted, case-normalized *.dist-info names, which include distribution names and versions.
  • Validation and reuse compare the recorded inventory with disk. Merely expanding or refreshing the package list no longer has authority to invalidate the environment.
  • A missing baseline or unreadable inventory is unknown, not evidence that packages were added. Existing manuallyModified sidecars remain honored.
  • The package list stays visible, but inline environments do not offer Install, Uninstall, or Change Version actions in the trees. The palette install command explains that dependencies should be edited in the script's metadata and setup rerun.
  • No new package-provider event metadata, per-package operation queues, or provider opt-in interfaces are introduced. The pip, Conda, Poetry, shared package-change helper, and package-watcher implementations are unchanged.

Keep routing and the setup action consistent

  • A confirmed mismatch clears the affected scripts' cached environments and validation state, advances revisions, restores their setup CodeLenses, and publishes environment changes.
  • Invalidation rechecks the current sidecar/inventory under a non-waiting cache-entry lock. An observation made before a repair must not invalidate the repaired entry.
  • Busy entries and unavailable confirmation reads do not publish stale invalidations. A newer selection wins when an older lookup finishes later.
  • Detection is on demand, after the existing five-second validation cache expires; this is not a new background polling service.

Delete one cached environment

  • Delete means physical deletion whether the entry has one associated script or several. It is not silently changed into "detach this script" for shared entries.
  • The action has no confirmation dialog. Scripts sharing the deleted entry need setup again.
  • Reuse the existing cache maintenance, ownership/path checks, locks, and association-cleanup machinery, with a single-entry target. Do not invoke the bulk project-settings cleanup.
  • Route an inline Projects-view item through the manager owning the clicked environment, rather than the file's potentially changed fallback manager. Ordinary environment removal keeps its existing routing.
  • Reject missing arguments, foreign/out-of-cache targets, redirected entries, and active creation conflicts. Preserve unrelated and unrecognized association records.
  • Invalidate a valid sidecar before filesystem removal so an interrupted deletion does not leave surviving cache files trusted as healthy. Report filesystem/persistence failures and update local state for what was actually removed.

Testing

Local validation before the rebase/signing update (1afe4bc9):

  • npm run compile-tests: passed.
  • npm run compile (webpack extension bundle): passed.
  • npm run lint: passed.
  • Full unit suite: 2,074 passing, 6 pending.

Coverage includes get-only drift detection and CodeLens recovery, shared versus unrelated entries, stale observations after newer repairs/selections, real cache-entry locks, unavailable inventory, Windows case-only renames, palette restrictions, targeted deletion without prompting, protected source/settings/base-Python files, already-missing entries, unsafe paths, partial failures, and concurrent create/delete/rehydration.

Suggested manual verification

These are reviewer checks, not a claim that a full live VS Code end-to-end pass has been completed:

  1. Enable python-envs.inlineScripts.enabled and reload. Use the Python Environments integration and companion builds that support per-file interpreters.
  2. Set up two scripts with the same metadata so they share a cached environment, plus a third script using a different cache entry.
  3. Expand/refresh Packages: setup remains valid and inline package mutation actions are absent. The palette install action for an inline project should show guidance rather than install.
  4. In a disposable test environment, change installed packages externally. After the validation cache expires, request the script's environment without saving. Verify the affected setup actions return and explicit setup repairs the entry.
  5. On Windows, rename job.py to Job.py; verify the existing environment remains associated and unsaved metadata is not replaced with stale disk metadata.
  6. Delete a shared inline environment from the Projects view. Verify there is no confirmation, only that cache entry is deleted, affected scripts need setup again, and the unrelated environment, scripts, project settings, and base Python remain.
  7. Exercise a busy/failed deletion and verify it reports failure rather than success. Recheck normal package management/removal for an ordinary venv.

Scope and known follow-ups

  • Inventory comparison is not a full package-integrity audit: it does not inspect package contents or legacy .egg-info-only installations.
  • Shared cache entries can be referenced by other workspaces. Association cleanup/notifications here are local to the current workspace; other windows notice deletion on revalidation. Cache locks do not track arbitrary running Python processes, so running jobs should be stopped before deletion.
  • The current unknown-inventory behavior is retained. The case where an entirely missing site-packages directory is accepted during reuse remains a follow-up; deleting the cache entry and running setup provides a recovery path.
  • A rejected inline selection can still briefly show the temporary "selected" badge; the actual selection is not changed. That feedback fix is separate from the routing invalidation addressed here.
  • Companion-version guidance is covered separately by Warn when companion extensions are too old for inline-script envs #1777. No installer-backend, retention-policy, or Run/Debug auto-setup changes are included.

@rchiodo

Rich Chiodo (rchiodo) commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

🔒 Automated review in progress — Rich Chiodo (@rchiodo) is auto-reviewing this PR.

Inline-script environments are built from a script's `# /// script` block and
cached per (dependencies, base interpreter), so one entry is shared by every
script that resolves to it. The extension must therefore notice when such an
environment stops matching what setup installed.

It previously answered that by interpreting package-change events, which
required classifying every event's origin: provenance metadata on a shared
notification contract, per-environment revisions and operation queues, a
pre-debounce snapshot captured by the file watcher, and a provider opt-in flag
so third-party package managers kept their existing contract. Several defects
came from that machinery misjudging the extension's own work - most visibly,
merely listing an environment's packages compared them against an empty
baseline and forced a working environment through setup again.

This records the installed distributions in the entry's own `.meta.json`,
inside the cache-entry lock that already guards the build, and compares that
record against the `*.dist-info` directories when validating. Because the
record is written under the same lock as the installation it describes, the
extension cannot observe its own work as an external edit, so the races the
provenance machinery refereed cannot occur.

An entry with no recorded inventory, or one whose inventory cannot be read, is
unknown rather than modified. `compareInstalledPackages` is the only place that
distinction is made; treating "no record" as "everything was added" is the
mistake this must never repeat.

Package management is no longer offered for inline-script environments. Editing
one by hand would silently change every script sharing it, so the install,
uninstall, and version-change actions are hidden; their package list stays
visible. Read-only packages use a distinct context value, so the existing menu
clauses simply do not match them.
Package management is refused at the command resolution chokepoint as well as
hidden in the tree. The palette entry can still resolve an inline-script
environment from the active script, so `getPackageCommandOptions` rejects one
with actionable guidance rather than letting the install proceed.

Also fixed, independent of the above:

- A modified environment stays unroutable until setup runs again. The check is
  enforced on association lookup, selection, and metadata-provenance
  validation, not only on creation and reuse, so saving or reopening a script
  cannot make a drifted environment look usable. Explicitly selecting one is
  rejected with actionable guidance rather than an unhandled error.

- Renaming a script with a case-only change on Windows keeps its environment.
  The association key is unchanged, so the rename is no longer treated as a
  reason to clear it. Dirty documents and pending metadata reads are resolved
  through canonical script keys.

No shared package-manager code is touched: `packageChanges`, `packageWatcher`,
`internal.api`, the environment-manager event relay, the conda and poetry
package managers, the pip package manager, and both views are unchanged, so
other environment types and third-party providers keep their exact behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a4277008-9dd3-4beb-b9ac-4197eb085404
Environment lookup could reject a drifted entry while leaving its routing
state and the other scripts sharing it marked valid. Confirmed drift now
clears those cached associations and validation results, restores the setup
CodeLens without a save, and notifies environment consumers.

Recheck the current inventory and sidecar under the existing cache-entry
lock before invalidating. Do not wait for another installer or invalidate
from an unreadable confirmation. Preserve newer selections when an older
lookup finishes, and keep persisted associations available for repair.

Add regression coverage for get-only detection, shared and unrelated entries,
cold rehydration, newer repairs and selections, lock contention, unavailable
inventory, and the real cache-entry lock. No shared package-manager changes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a4277008-9dd3-4beb-b9ac-4197eb085404
Support Delete Environment for an individual inline-script cache entry without
a confirmation prompt. Reuse the existing cache maintenance and entry-lock
checks, but limit disk deletion and association cleanup to the selected entry.

Clear affected routing and notify consumers, remove the deleted environment
from the discovered collection, and preserve scripts, project settings, base
Python installations, unrelated environments, and opaque association records.
Resolve inline Projects-view items through their owning manager rather than
the file's potentially changed fallback selection.

Reject missing, foreign, redirected, and busy targets. Mark an entry unusable
before deletion so a partial failure cannot leave surviving files trusted as a
healthy environment. Report filesystem and persistence failures, and preserve
operation ordering with concurrent setup and rehydration.

Add coverage for shared/unshared deletion, no confirmation, isolation, missing
entries, unsafe targets, locks, partial failures, and command routing.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a4277008-9dd3-4beb-b9ac-4197eb085404
Comment thread src/test/features/views/treeViewItems.unit.test.ts Outdated
@rchiodo

Copy link
Copy Markdown
Contributor

Result: 🔴 could-not-verify

Verification details

Verification: Isolated verification observed failures that were not classified as caused by this PR: Dependency and test discovery. The relevant tests could not be fully run in the isolated environment; this review is not fully verified.

Summary: Verification could not start because the container has no Node.js executable; discovery failed with `/bin/sh: node: not found`. Consequently, TypeScript compilation and the targeted Mocha suites were not run. The PR adds extensive unit coverage for drift detection, deletion, routing, and package-management restrictions, but none could be executed here. Overall confidence is limited by the verification environment.

Test runs: 1 failed, 4 not run

  • ⚠️ Not run | TypeScript test compilation | npm run compile-tests
  • ⚠️ Not run | Inline manager drift and deletion tests | node ./node_modules/mocha/bin/mocha.js --require source-map-support/register --require out/test/unittests.js --ui tdd --timeout 180000 out/test/managers/builtin/inlineScript/envManager.unit.test.js --grep "package drift|single cached environment removal|temporarily unreadable|case-only rename"
  • ⚠️ Not run | Inline routing and command UI tests | node ./node_modules/mocha/bin/mocha.js --require source-map-support/register --require out/test/unittests.js --ui tdd --timeout 180000 out/test/features/inlineScript/lazyDetector.unit.test.js out/test/features/envCommands.inlineSelectionErrors.unit.test.js out/test/features/envCommands.unit.test.js out/test/features/views/treeViewItems.unit.test.js
  • ⚠️ Not run | Package-event forwarding tests | node ./node_modules/mocha/bin/mocha.js --require source-map-support/register --require out/test/unittests.js --ui tdd --timeout 180000 out/test/features/envManagers.packageEvents.unit.test.js
  • Failed | unrelated to this PR | Dependency and test discovery | printf '%s\n' '== toolchain ==' && node --version && npm --version && printf '%s\n' '== dependencies ==' && test -d node_modules && test -x node_modules/.bin/tsc && test -x node_modules/.bin/mocha && echo 'node_modules and test runners present' && printf '%s\n' '== changed files ==' && base=$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main 2>/dev/null) && echo "base=$base" && git diff --name-status "$base"...HEAD && printf '%s\n' '== relevant scripts ==' && node -e "const p=require('./package.json'); for (const k of ['compile-tests','compile','lint','unittest']) console.log(k+'='+p.scripts[k])"
⚠️ TypeScript test compilation diagnostic output
Not run: Node.js and npm were unavailable in the container.
⚠️ Inline manager drift and deletion tests diagnostic output
Not run: Node.js was unavailable.
⚠️ Inline routing and command UI tests diagnostic output
Not run: Node.js was unavailable.
⚠️ Package-event forwarding tests diagnostic output
Not run: Node.js was unavailable.
Dependency and test discovery diagnostic output
== toolchain ==
/bin/sh: 1: node: not found

@rchiodo Rich Chiodo (rchiodo) added the review-auto:changes-requested Automated review: posted blocking findings to address. label Sep 11, 2026
Replace the negative package-capability substring assertion with the complete
expected contextValue. The test now also detects missing environment, remove,
or activation markers instead of accepting an empty or undefined context.

Addresses PR microsoft#1780 review comment 3993748325. No production-code changes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a4277008-9dd3-4beb-b9ac-4197eb085404
@rchiodo

Copy link
Copy Markdown
Contributor

Result: 🔴 could-not-verify

Verification details

Verification: Isolated verification observed failures that were not classified as caused by this PR: Dependency and test-runner preflight. The relevant tests could not be fully run in the isolated environment; this review is not fully verified.

Summary: Verification could not proceed because the container lacks Node.js (`node: not found`). Consequently, compilation and the four targeted Mocha groups were not run. The PR adds extensive unit coverage for drift detection, routing, package restrictions, renames, and targeted deletion, but none could be executed here.

Test runs: 1 failed, 4 not run

  • ⚠️ Not run | InlineScriptEnvManager unit tests | node ./node_modules/mocha/bin/mocha.js --require source-map-support/register --require out/test/unittests.js --ui tdd --timeout 180000 out/test/managers/builtin/inlineScript/envManager.unit.test.js
  • ⚠️ Not run | InlineScriptLazyDetector unit tests | node ./node_modules/mocha/bin/mocha.js --require source-map-support/register --require out/test/unittests.js --ui tdd --timeout 180000 out/test/features/inlineScript/lazyDetector.unit.test.js
  • ⚠️ Not run | Environment command unit tests | node ./node_modules/mocha/bin/mocha.js --require source-map-support/register --require out/test/unittests.js --ui tdd --timeout 180000 out/test/features/envCommands.unit.test.js out/test/features/envCommands.inlineSelectionErrors.unit.test.js
  • ⚠️ Not run | Package-event and tree-view unit tests | node ./node_modules/mocha/bin/mocha.js --require source-map-support/register --require out/test/unittests.js --ui tdd --timeout 180000 out/test/features/envManagers.packageEvents.unit.test.js out/test/features/views/treeViewItems.unit.test.js
  • Failed | unrelated to this PR | Dependency and test-runner preflight | printf '%s\n' '== profile ==' && node --version && npm --version && printf '%s\n' '== dependencies ==' && if [ -d node_modules ]; then echo node_modules=present; else echo node_modules=missing; fi && printf '%s\n' '== test scripts ==' && node -e "const p=require('./package.json'); for (const k of ['compile-tests','unittest','compile','lint']) console.log(k+'='+(p.scripts[k]||''))" && printf '%s\n' '== mocha config ==' && cat build/.mocha.unittests.json && printf '%s\n' '== changed test files ==' && git diff --name-only HEAD^ -- 'src/test/**/*.test.ts'
⚠️ InlineScriptEnvManager unit tests diagnostic output
Not run: the preflight reported `/bin/sh: 1: node: not found`.
⚠️ InlineScriptLazyDetector unit tests diagnostic output
Not run: Node.js was unavailable.
⚠️ Environment command unit tests diagnostic output
Not run: Node.js was unavailable.
⚠️ Package-event and tree-view unit tests diagnostic output
Not run: Node.js was unavailable.
Dependency and test-runner preflight diagnostic output
== profile ==
/bin/sh: 1: node: not found

@rchiodo Rich Chiodo (rchiodo) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved via Review Center.

@rchiodo Rich Chiodo (rchiodo) added review-auto:approved Automated review: no blocking findings (approval posted). and removed review-auto:changes-requested Automated review: posted blocking findings to address. labels Sep 11, 2026
@StellaHuang95
Stella Huang (StellaHuang95) merged commit 6ad1427 into microsoft:main Sep 11, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Issue identified by VS Code Team member as probable bug review-auto:approved Automated review: no blocking findings (approval posted). skip package*.json

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants