fix: preserve conda environments on transient discovery failure (and fix fast-path/concurrency races)

[StellaHuang95]

Context

CondaEnvManager keeps an in-memory collection of discovered conda environments and emits onDidChangeEnvironments add/remove events as that collection changes. Discovery is delegated to refreshCondaEnvs in condaUtils.ts, which drives the native finder (PET). Persisted global/workspace selections are restored separately by loadEnvMap(), which resolves the persisted paths and appends them to the collection (it never removes).

Previously, refreshCondaEnvs returned PythonEnvironment[] and used [] to represent two different outcomes:

• a successful discovery that found zero conda environments, and
• a failure — the native finder rejecting/throwing.

Because both looked identical ([]), the manager could not tell them apart.

Transient-failure reproduction

1. Conda is installed and the user has one or more conda environments; discovery succeeds and collection is populated (envs visible in the UI, exposed to Python/Jupyter API consumers).
2. A transient failure occurs on a subsequent refresh — e.g. nativeFinder.refresh() rejects (PET spawn/timeout/IO error).
3. refreshCondaEnvs caught the error internally and returned [].
4. CondaEnvManager.refresh() did discard = collection; collection = await refreshCondaEnvs(...), then fired remove for every env in discard and add for none.
5. Result: a known-good collection was wiped and spurious remove events were emitted after a purely transient error. The next successful refresh re-added them, causing visible flicker and churn for downstream API consumers — and any persisted selection appeared to vanish.

Root cause

The [] sentinel conflated "authoritative empty" with "discovery failed", so every collection-assignment site in CondaEnvManager treated a transient failure as "conda now has zero environments".

Failure-vs-empty contract

refreshCondaEnvs now returns Promise<PythonEnvironment[] | undefined>:

• undefined -> discovery failed. The native finder threw/rejected. Callers must preserve any previously known-good collection and must not emit removals.
• An array (including []) -> authoritative success. An empty array means conda genuinely has no environments (or conda is not installed); stale environments should be removed normally.

The native finder already normalizes malformed worker output to [] before conda sees it, so the defensive non-array guard in refreshCondaEnvs keeps its preexisting behavior and returns [] (a successful-empty result), not a failure. This PR does not change shared finder behavior and does not reclassify malformed production output — the authoritative distinction is purely rejection/exception vs successful array (including []).

Fix

• condaUtils.ts refreshCondaEnvs: return type -> PythonEnvironment[] | undefined; only the native-finder rejection/exception catch returns undefined. The non-array guard and the legitimate "conda not installed / not found" paths keep returning [].

• condaEnvManager.ts guards all three discovery sites (initialize, explicit refresh, and background startBackgroundInit inside get). On refreshCondaEnvs() === undefined:

  • the known-good this.collection is preserved (never replaced with []);
  • no remove events are emitted;
  • loadEnvMap() is still run so persisted global/workspace selections are restored/updated even though discovery failed.

  A new private helper preserveCollectionOnFailedDiscovery() runs loadEnvMap() and emits add events only for the environments loadEnvMap() reports it appended — never removals.

• Existing _initialized retry semantics are untouched (a separate PR owns initialization retries). A failure now returns undefined without throwing, so tryFastPathGet's reject-triggered retry path in fastPath.ts is unchanged.

• No new user notifications — only existing trace/log channels (log.warn / traceVerbose).

Why legitimate deletion remains intact

A genuine "no more conda environments" result is a successful [], which is !== undefined, so refresh() falls through to the normal discard/collection swap and fires remove events for every stale environment. Uninstalling conda or deleting all conda envs still clears the collection and emits removals exactly as before — only transient failures (rejections) are now preserved.

Persisted-selection restoration on failure

Two reviews flagged a blocking regression: early-returning on failure without calling loadEnvMap() would leave a persisted global/workspace selection unrestored. The fix keeps loadEnvMap() on the failure path so a persisted selection that resolves independently is still restored, mapped, and made available via get(). Only genuinely newly-appended environments are emitted as add events; the failed-discovery path never emits removals.

Concurrency races (follow-up review)

A final review reproduced two races between the fast path, the failed-discovery recovery, and concurrent create/resolve. Both are fixed surgically:

1. Fast-path result was not registered. get()'s fast path could resolve a persisted environment and return it without registering it in collection/fsPathToEnv. If background discovery then failed and loadEnvMap()'s own re-resolution also failed, initialization completed with the environment lost, and the next get() returned undefined. Fix: get() now calls a new private registerFastPathEnv(scope, env) immediately after a successful fast path, registering the environment into the collection + workspace map by normalized-path identity (reusing any existing entry). Registration is silent (no event) by design: the discovery success path replaces this.collection wholesale and re-emits the entire collection, so emitting on registration would double-emit on success. Background failure recovery now finds and reuses the registered environment via findEnvironmentByPath without requiring a second successful resolve.
2. Recovery could double-emit a concurrent append. preserveCollectionOnFailedDiscovery() previously inferred additions by snapshotting the whole collection around the awaited loadEnvMap(), so a concurrent create()/resolve() that appended (and already emitted) its own environment during the await was emitted a second time by recovery. Fix: loadEnvMap() now returns exactly the environments it itself appended, and re-checks findEnvironmentByPath immediately before each push (after the awaited resolveCondaPath, at both the global and workspace resolve sites) so a concurrent insert cannot duplicate the collection. Recovery emits adds for only that returned list.

Tests

Focused unit tests (condaUtils.refreshCondaEnvs.unit.test.ts, condaEnvManager.resultPreservation.unit.test.ts):

• utility contract: native-finder rejection -> undefined; successful empty discovery -> [] (defined array).
• prior environments survive a failed refresh (undefined) with no change events when nothing persisted resolves;
• a successful [] empties the collection and emits the expected remove events;
• a successful non-empty result replaces the collection and emits removals + adds;
• a failed initialize with a persisted global selection restores it, emits only its addition (no removals), and it is retained across first and subsequent get(undefined) calls;
• a failed explicit refresh preserves the old collection and restores a persisted global selection, emitting only its addition;
• a fast/background get whose background discovery fails restores a persisted workspace selection, returns it on the first get, retains it after initialization settles, and never emits a duplicate/removal;
• race Pep723/stage 1 #1 (deterministic): the fast path resolves + registers the env, then background discovery fails and a second re-resolution would fail — a gated refreshCondaEnvs proves recovery reuses the registered env (resolveCondaPath called exactly once) and repeated get() returns the same environment with no duplicate collection entry or event;
• race Add inline-script environment creation tests (PEP 723 PR 5/16 - tests) #2 (deterministic): a paused loadEnvMap (gated resolveCondaPath) runs concurrently with a resolve() that appends + emits the same env — asserting exactly one collection entry and exactly one raw add event (recovery emits nothing extra).

Validation: npm run lint clean, npm run compile-tests clean, targeted conda + fast-path suites pass. Full unit suite: 1701 passing; the only failure is a pre-existing, unrelated Windows EPERM file-rename flake in inlineScriptCacheLayout (touches no conda code; CI runs on Linux).

Limitations

• Scope is deliberately narrow: this PR handles thrown/rejected native-finder failures only. It does not broaden into shared NativePythonFinder behavior; malformed worker output is still normalized to [] upstream (preexisting) and treated as a successful-empty result.
• Telemetry still cannot distinguish a native-finder failure during initialize from a genuine empty result (result='success', envCount=0). This was already true before this change; the new undefined signal makes a follow-up possible.
• Scope is conda-only and surgical; other managers and PET are unchanged. No generic snapshot abstraction was introduced.

Fixes the transient-failure environment wipe described above.

Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com

[StellaHuang95]

🔒 Automated review in progress — @StellaHuang95 is auto-reviewing this PR.

[StellaHuang95]

Warning · Non-blocking recommendation

📍 src/test/managers/conda/condaEnvManager.resultPreservation.unit.test.ts:255
The Set deduplicates addition names, so this test passes even if the background path emits duplicate add events. Assert the raw addition count and ordered names instead.

[verified]

[StellaHuang95]

Warning · Non-blocking recommendation

📍 src/managers/conda/condaEnvManager.ts:131
The same failure-result interpretation is repeated at all three discovery entry points, leaving the preservation invariant vulnerable to drift. Centralize applying a discovery result in a private helper, or otherwise keep the branches mechanically linked.

[verified]

[StellaHuang95]

Issue · Please address or respond

📍 src/managers/conda/condaEnvManager.ts:394
The Skeptic reproduced a race where successful background discovery finishes before the fast path, after which this silently inserts an environment into the already-announced authoritative collection. Reconcile this completion order by emitting an add when necessary or routing the late result through a common post-discovery application path, and add a gated regression test.

[verified]

[StellaHuang95]

Warning · Non-blocking recommendation

📍 src/managers/conda/condaEnvManager.ts:131
The prior maintainability concern is only partially addressed: recovery is shared, but three callers still independently interpret undefined. Centralize discovery-result interpretation so future call sites cannot accidentally collapse failure into an authoritative empty result while retaining their distinct success-event policies.

[verified]