From f4c00ff4ba60714292a104c7b2effb799cdccc42 Mon Sep 17 00:00:00 2001
From: JJ Lee <jeonghun.jj.lee@gmail.com>
Date: Tue, 8 Sep 2026 15:36:08 -0400
Subject: [PATCH] fix(sidebar): workspace source always wins environment dedup
 regardless of folder order (#895)

Root cause: getRoots() iterated workspace folders in order. If a research
project appeared before the environment folder, the project's resolution
added the environment to envBySlug with source 'resolved' first. When the
actual environment workspace folder was reached later, the dedup guard
(!envBySlug.has(slug)) prevented the workspace entry from overwriting it.

Fix: workspace-folder environments always overwrite the envBySlug entry
(removed the has() guard). This ensures source is 'workspace' regardless
of iteration order, so the context menu correctly shows 'Remove from
Workspace'.

- 1 new test reproducing the exact bug scenario
- 1 updated test for duplicate-slug edge case
---
 .../extension/src/sidebar_tree_service.ts     | 27 +++++++-------
 .../extension/test/sidebar_env_tree.test.ts   | 35 +++++++++++++++++--
 2 files changed, 46 insertions(+), 16 deletions(-)

diff --git a/packages/extension/src/sidebar_tree_service.ts b/packages/extension/src/sidebar_tree_service.ts
index 3bd639bf..7cb15818 100644
--- a/packages/extension/src/sidebar_tree_service.ts
+++ b/packages/extension/src/sidebar_tree_service.ts
@@ -73,26 +73,25 @@ export class SidebarTreeService {
 
       if (projectType === "environment") {
         // Collect environment workspace folders (#895)
+        // Always overwrite: workspace source wins dedup over resolved (#895 bugfix)
         if (this.deps.readEnvironmentToml) {
           try {
             const envManifest = this.deps.readEnvironmentToml(dir);
             if (envManifest) {
               const slug = envManifest.slug;
-              if (!envBySlug.has(slug)) {
-                envBySlug.set(slug, {
-                  path: dir,
+              envBySlug.set(slug, {
+                path: dir,
+                name: envManifest.name,
+                projectType: "environment",
+                source: "workspace",
+                boundProjectCount: 0,
+                environment: {
                   name: envManifest.name,
-                  projectType: "environment",
-                  source: "workspace",
-                  boundProjectCount: 0,
-                  environment: {
-                    name: envManifest.name,
-                    slug,
-                    path: dir,
-                    colorIndex: envColorIndex(slug),
-                  },
-                });
-              }
+                  slug,
+                  path: dir,
+                  colorIndex: envColorIndex(slug),
+                },
+              });
             }
           } catch {
             // Manifest read failure → skip this environment
diff --git a/packages/extension/test/sidebar_env_tree.test.ts b/packages/extension/test/sidebar_env_tree.test.ts
index 7951702f..8cfc7912 100644
--- a/packages/extension/test/sidebar_env_tree.test.ts
+++ b/packages/extension/test/sidebar_env_tree.test.ts
@@ -207,7 +207,7 @@ describe("SidebarTreeService environment discovery (#895)", () => {
     expect(envRoots[0].environment?.slug).toBe("spin-qubit");
   });
 
-  it("two workspace folders with same slug — first in order wins", () => {
+  it("two workspace folders with same slug — last in order wins (both workspace-sourced)", () => {
     const service = makeDiscoveryService({
       folders: [
         { path: "/env/first", name: "first" },
@@ -223,7 +223,8 @@ describe("SidebarTreeService environment discovery (#895)", () => {
     const roots = service.getRoots();
     const envRoots = roots.filter((r) => r.projectType === "environment");
     expect(envRoots.length).toBe(1);
-    expect(envRoots[0].path).toBe("/env/first");
+    expect(envRoots[0].path).toBe("/env/second");
+    expect(envRoots[0].source).toBe("workspace");
   });
 
   it("boundProjectCount is 0 when no projects bind to the environment", () => {
@@ -270,4 +271,34 @@ describe("SidebarTreeService environment discovery (#895)", () => {
     expect(roots[1].projectType).toBe("research");
     expect(roots[2].projectType).toBe("dev");
   });
+
+  it("workspace source wins dedup even when project appears before environment in folder order", () => {
+    // This is the root cause of the "Remove from Workspace" bug:
+    // if a research project is iterated before the environment workspace folder,
+    // the project resolution adds the environment with source "resolved" first,
+    // and the workspace folder must overwrite it.
+    const service = makeDiscoveryService({
+      folders: [
+        // Project comes FIRST — its resolution will try to add the env as "resolved"
+        { path: "/proj/a", name: "a" },
+        // Environment workspace folder comes SECOND — must still win the dedup
+        { path: "/env/shared", name: "shared" },
+      ],
+      projectTypes: {
+        "/proj/a": "research",
+        "/env/shared": "environment",
+      },
+      toml: { "/proj/a": { name: "Project A" } },
+      envToml: { "/env/shared": { name: "Shared Env", slug: "shared" } },
+      envResolution: {
+        "/proj/a": { path: "/env/shared", slug: "shared", name: "Shared Env" },
+      },
+    });
+
+    const roots = service.getRoots();
+    const envRoots = roots.filter((r) => r.projectType === "environment");
+    expect(envRoots).toHaveLength(1);
+    expect(envRoots[0].source).toBe("workspace");
+    expect(envRoots[0].path).toBe("/env/shared");
+  });
 });