Skip to content

Add opportunistic inline-script TTL eviction (PEP 723 PR 14/16) - #1745

Open
Stella Huang (StellaHuang95) wants to merge 2 commits into
microsoft:mainfrom
StellaHuang95:copilot/pep723-pr14-ttl-eviction
Open

Add opportunistic inline-script TTL eviction (PEP 723 PR 14/16)#1745
Stella Huang (StellaHuang95) wants to merge 2 commits into
microsoft:mainfrom
StellaHuang95:copilot/pep723-pr14-ttl-eviction

Conversation

@StellaHuang95

Copy link
Copy Markdown
Contributor

Part of #1602 (PEP 723 inline script env support). Design doc: #1601.

Roadmap context

This is PR 14 of 16 in the PEP 723 inline-script roadmap. PR 13 added explicit, user-confirmed cache cleanup; this PR adds the separate best-effort TTL path.

Phase 5: Lifecycle and polish PR Status
PR 7: persistence (get / set + Memento) merged (#1697)
PR 13: Clear Script Environment Cache merged (#1724)
PR 14: opportunistic 14-day TTL eviction this PR
PR 15: lifecycle telemetry merged (#1723)
PR 16: status-bar decision resolved; no code PR

Why this PR

Inline-script environments are dependency/interpreter-keyed and intentionally rebuilt instead of synchronized in place. Without lifecycle cleanup, old cache keys accumulate whenever dependencies or the selected Python change.

The design calls for a pipx-style 14-day TTL in addition to the explicit clear command. Because TTL cleanup is silent, it must be more conservative than user-confirmed cleanup and must never prevent the requested environment from being created.

What this PR does

  • Attempts one TTL sweep per InlineScriptEnvManager session.
  • Runs the sweep before the first environment creation/reuse reaches the cache.
  • Reads only valid .meta.json sidecars and selects entries whose lastUsedAt is strictly older than 14 days.
  • Reuses PR 13's physical-root, normal-directory, direct-child, and entry-lock safety checks.
  • Re-reads lastUsedAt under the entry lock before deletion.
  • Preserves held, retained, malformed, redirected, unavailable, or failed-to-delete entries.
  • Treats all TTL failures as best-effort warnings and continues the triggering creation.
  • Invalidates persisted and warm script associations only for entries confirmed deleted or definitively removed by another host.
  • Removes evicted environments from the discovered collection and emits the existing environment-change events.
  • Prevents a concurrent discovery refresh from publishing a stale snapshot after eviction.

Eviction semantics

State Behavior
lastUsedAt older than 14 days Lock, revalidate, and delete
Age exactly 14 days Keep
Recent or future timestamp Keep
Missing, invalid, unsupported, or unreadable sidecar Keep
Timestamp becomes fresh before lock acquisition Keep
Entry is actively locked Keep
Entry has a retained cancellation lock Keep; only explicit cleanup may reclaim it
Entry deletion fails Keep and continue creation
Another host already deleted the entry Confirm with lstat, then invalidate local associations
Association persistence fails after deletion Keep in-memory state consistent, log, and continue creation
Refresh started before deletion Reject its stale collection snapshot
Refresh starts during maintenance Wait, then scan the post-eviction cache

Concurrency and safety

  • The once-per-session latch is set synchronously, so concurrent creates share one sweep.
  • The sweep uses the existing cache-maintenance then selection-queue ordering.
  • The active-create counter is incremented before the sweep, preserving PR 13's clear-vs-create behavior.
  • Each deletion is protected by the existing cross-host cache-entry lock.
  • Physical root and entry containment are revalidated after lock acquisition.
  • A cache mutation revision prevents stale discovery publication without waiting on, or deadlocking with, a refresh already blocked by maintenance.
  • Only ENOENT counts as confirmed cross-host deletion; permission and transient access errors preserve associations.

Performance

  • The cache is scanned once per extension-host session, not on every lookup.
  • Entries are inspected and deleted sequentially to avoid I/O spikes.
  • Activation remains unchanged and is not blocked by TTL work.
  • Environments with missing or uncertain metadata are not repeatedly modified.

User impact

The feature remains behind python-envs.inlineScripts.enabled. Users without inline-script environments see no behavior change. Existing environments are retained unless a valid sidecar proves they have not been successfully reused for more than 14 days.

Tests

  • npm run compile-tests
  • npm run compile
  • npm run lint
  • Complete unit suite: 1,928 passing, 6 pending

Focused coverage includes:

  • strict 14-day cutoff;
  • once-per-session and failed-sweep behavior;
  • under-lock freshness revalidation;
  • held and retained lock preservation;
  • deletion and persistence failure isolation;
  • cross-host deletion confirmation;
  • association and discovered-collection invalidation;
  • refresh-versus-eviction publication races;
  • unchanged explicit clear-cache behavior.

Scope and follow-up

This PR does not add telemetry, activation-time cleanup, a cache-root lock, project-setting cleanup, UX, or retry loops. Explicit clear-cache behavior remains unchanged.

Run a best-effort 14-day cache sweep once per manager session before the first inline-script environment creation. Reuse the existing safe deletion and association cleanup paths while preserving uncertain or active entries.

Part of microsoft#1602.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@StellaHuang95 Stella Huang (StellaHuang95) added the feature-request Request for new features or functionality label Aug 28, 2026
@rchiodo

Rich Chiodo (rchiodo) commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

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

@rchiodo

Copy link
Copy Markdown
Contributor

Verification: The relevant tests could not be fully run in the isolated environment; this review is not fully verified.

Result: could-not-verify

Summary: Verification could not start because no trusted sandbox image is configured for this repository, and local execution was not authorized. No compile, lint, or targeted unit test command ran. The PR adds 10 focused TTL-eviction unit tests, but their results could not be confirmed. Confidence is therefore limited.

Test runs: 1 not run

  • ⚠️ Not run | Dependency and test discovery preflight | git status --short && git branch --show-current && git --no-pager diff --name-status upstream/main...HEAD && if exist node_modules (echo NODE_MODULES_PRESENT) else (echo NODE_MODULES_MISSING) && echo AUTOMATION_SANDBOX_PROFILE=%AUTOMATION_SANDBOX_PROFILE%
⚠️ Dependency and test discovery preflight diagnostic output
Container verification could not start and local execution was not authorized for this PR HEAD: No trusted sandbox image is configured for microsoft/vscode-python-environments.

@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 the review-auto:approved Automated review: no blocking findings (approval posted). label Aug 28, 2026
TTL eviction ranked entries only by the sidecar lastUsedAt, which is refreshed on create/reuse but never when an environment is resolved for run, debug, or Pylance. A stable script's environment could therefore look stale and be evicted (clearing its association) while still in active use. Skip eviction of any cache entry a script association still references, so the sweep reclaims only orphaned entries (superseded by a dependency change, or left by a deleted/deselected script).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Stella Huang (StellaHuang95) added a commit to StellaHuang95/vscode-python-environments that referenced this pull request Aug 29, 2026
Update sections 6, 7, 9, and 10 to reflect what was actually built and
the in-flight PRs (vscode-python-environments microsoft#1744/microsoft#1745, pyrx #9265,
vscode-python #26129):

- Q6: dedicated InlineScriptAssociationStore + metadata-identity
  binding (pending/matched) instead of reusing VenvManager persistence;
  clarify pythonProjects[] is user-visible registration, not routing.
- Q7: TTL runs once per session (runTtlEvictionOnce) and protects
  entries referenced by a live association (PR microsoft#1745).
- Q9: Pylance re-routes via the existing didChangeConfiguration signal
  (_revalidateOpenRegularFiles / revalidateWorkspaceForFile), so PR 18's
  dedicated notification is optional; fix persistence/event-source bullets.
- Q10: correct the '~10 LOC' estimate to the real exactResource +
  __pythonIsProgramInterpreter + middleware change; add useEnvExtension
  gating caveat for Run and Debug.

Add a status banner noting the doc was revised post-implementation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature-request Request for new features or functionality review-auto:approved Automated review: no blocking findings (approval posted).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants