fix: preserve conda environments on transient discovery failure

[StellaHuang95]

Context

Conda environment discovery runs the native finder (PET). When that refresh throws/rejects — a
transient failure — refreshCondaEnvs previously returned [], the same value it returns for a
genuine "no conda environments" result. CondaEnvManager could not tell the two apart, so a
transient failure replaced the known-good collection with [] and emitted spurious remove
events, making previously-discovered environments disappear from the UI until the next successful
refresh.

Root cause

refreshCondaEnvs collapsed distinct outcomes into []:

• the native finder threw (discovery failed),
• the native finder returned a non-array / malformed value, and
• the native finder succeeded with zero environments.

Every manager discovery path (initialize, refresh, background get) then assigned
this.collection = result and emitted add/remove events, so a failure wiped the collection.

Fix

• refreshCondaEnvs now returns PythonEnvironment[] | undefined:
  • undefined = discovery failed (native finder threw/rejected, or returned a non-array value).
  • an array (including []) = authoritative success; [] means genuinely no environments.
• The three manager discovery paths guard on undefined:
  • Failure: keep the existing collection and emit no removals, but still call loadEnvMap()
    so persisted global/workspace selections are restored (loadEnvMapPreservingCollection).
    loadEnvMap() returns the exact environments it appended, and only those are announced as
    adds — so an overlapping successful refresh that replaces the collection cannot be
    double-announced.
  • Success (including []): replace the collection and emit the normal add/remove events.

Legitimate deletion is unchanged: a successful [] is authoritative and removes stale environments
exactly as before. Only a undefined failure is treated as "preserve".

Tests

• condaUtils.refreshCondaEnvs.unit.test.ts: the utility returns undefined on a rejected refresh
  and on a non-array/malformed result, and [] on a successful empty discovery.
• condaEnvManager.resultPreservation.unit.test.ts:
  • a failed refresh/initialize preserves the known-good collection and emits no changes when
    nothing persisted resolves;
  • a successful [] empties the collection and emits removals; a successful non-empty result
    replaces the collection and emits removals + adds;
  • a failed discovery still restores a persisted global or workspace selection, emitting only its
    addition and never a removal, and retains it across get calls;
  • with loadEnvMap() paused, a concurrent refresh that replaces the collection announces only
    the environments this resolution appended (no duplicate add).

[StellaHuang95]

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

[StellaHuang95]

GitHub cannot anchor PR review comments to unchanged lines in the diff. Falling back to a general PR comment for src/managers/conda/condaUtils.ts:L865.

Issue · Please address or respond

📍 src/managers/conda/condaUtils.ts:862
Return undefined when PET produces a non-array value. This path was debugger-verified to return [], which makes malformed discovery data authoritative and removes every known environment—the same failure mode this PR is intended to prevent.

[verified]

[StellaHuang95]

Re: non-array PET output at src/managers/conda/condaUtils.ts (refreshCondaEnvs) — addressed in d7bbf65.

The non-array/malformed guard now returns the undefined failure sentinel instead of [], so malformed discovery data is no longer treated as an authoritative empty result and can no longer remove known environments — the same failure mode this PR prevents for thrown/rejected discovery.

Added a regression test — returns undefined when the native finder produces a non-array result (malformed discovery) — covering null, undefined, a plain object, and a string. The contract is now consistent: undefined = discovery failure (throw/reject or non-array); an array (including []) = authoritative success.

[StellaHuang95]

GitHub cannot anchor PR review comments to unchanged lines in the diff. Falling back to a general PR comment for src/managers/conda/condaEnvManager.ts:L336.

Issue · Please address or respond

A verified interleaving lets a failed refresh announce a persisted environment and then lets this successful path announce it again by reading shared this.collection after await loadEnvMap(). Serialize collection mutation/notification or have the successful path emit only its authoritative refresh results plus environments appended by its own loadEnvMap invocation.

[verified]

[StellaHuang95]

Re: double-announce at src/managers/conda/condaEnvManager.ts L336 (refresh() success path) — verified and fixed in the pushed commit.

The interleaving is real: the success path sets this.collection = refreshed, then await this.loadEnvMap(), then fired this.collection.map(...) — the entire current collection. If a concurrent failed refresh runs during that await, its loadEnvMapPreservingCollection appends a persisted environment into the shared this.collection and announces it once; the resuming success path then read the mutated collection and announced that same environment a second time.

Fix (the second option you suggested — no serialization): the success path now announces only its own authoritative results plus what its own loadEnvMap appended, captured before the await:

this.collection = refreshed;
const refreshedAdds = refreshed.map((env) => ({ kind: EnvironmentChangeKind.add, environment: env }));
const appended = await this.loadEnvMap();
const args = [
    ...discard.map((env) => ({ kind: EnvironmentChangeKind.remove, environment: env })),
    ...refreshedAdds,
    ...appended.map((env) => ({ kind: EnvironmentChangeKind.add, environment: env })),
];
this._onDidChangeEnvironments.fire(args);

refreshedAdds is snapshotted before loadEnvMap mutates the array (they alias after the assignment), so a concurrent append no longer leaks into the success emission; that env is announced exactly once by whichever path actually appended it. In the non-concurrent case this is identical to the previous whole-collection emit. loadEnvMap's appends stay disjoint from refreshed via the existing exact-path check, so no path is announced twice. The same guard is applied to the sibling background-init success path (previously L361), which had the identical whole-collection emit.

Regression added — successful refresh announces only its own results/appends, not an env a concurrent failed refresh appended: a successful refresh is gated inside its loadEnvMap; a concurrent failed refresh appends and announces a persisted env; on resume the success path emits only its own base result. It fails without the fix (persisted announced twice) and passes with it.

This was a general PR comment (GitHub could not anchor it to an unchanged line), so it has no review thread to resolve; posting this as the disposition.

[StellaHuang95]

GitHub cannot anchor PR review comments to unchanged lines in the diff. Falling back to a general PR comment for src/managers/conda/condaEnvManager.ts:L343.

Issue · Please address or respond

📍 src/managers/conda/condaEnvManager.ts:322
The prior verified mixed-outcome race remains in the reverse ordering: a failed refresh can emit add(persisted) before a delayed successful refresh captures it in discard and emits remove(persisted), add(persisted). Add that deterministic schedule and prevent the churn through serialization, generation ownership, or centralized event reconciliation.

[verified]

[StellaHuang95]

Re: reverse mixed-outcome race at src/managers/conda/condaEnvManager.ts (refresh(), issuecomment 5384887632) — verified and fixed at head 2bf9603 via centralized event reconciliation.

refresh() no longer discards the whole prior collection and re-emits every env. On the successful path it now reconciles adds/removes by normalized path: remove fires only for discarded paths absent from the resolved set, and add fires only for resolved paths absent from the discarded set. A path present both before and after the refresh is therefore neither removed nor re-added, so a delayed success that captures a path a prior failed refresh already announced emits nothing for that path — no remove(persisted), add(persisted) churn and no transient disappearance. Path is the reconciliation key because conda envId is regenerated per resolution; the refresh's own authoritative results are also snapshotted before loadEnvMap() awaits, so a concurrent failed refresh's aliased append cannot leak into the successful path's emissions.

Deterministic regression added — a delayed successful refresh does not remove/re-add a path a prior failed refresh already announced: it runs exactly the reverse schedule (a failed refresh emits add(persisted) first, then a gated successful refresh completes and resolves a same-path env with a different id), and asserts the full event sequence stays exactly ['add:persisted'], one collection entry for that path, and the exact final collection. It fails against the pre-fix logic and passes with the fix. initialize() and background-get emit adds only (no discard/remove) and are unchanged. Focused suite 15/15; lint and compile-tests clean.