fix(web): strip phantom session panels on env-layout restore (#944)

apps/web/components/task/dockview-desktop-layout.tsx

@@ -10,7 +10,7 @@ import {
 } from "dockview-react";
 import { themeKandev } from "@/lib/layout/dockview-theme";
 import { useDockviewStore, performLayoutSwitch } from "@/lib/state/dockview-store";
-import { tryRestoreLayout } from "./dockview-layout-restore";
+import { restoreEnvLayout } from "./dockview-layout-restore";
 import {
   setupContainerResizeSync,
   setupGroupTracking,
@@ -513,8 +513,8 @@ export const DockviewDesktopLayout = memo(function DockviewDesktopLayout({
       setApi(api);
 
       const currentEnvId = envIdRef.current;
-      // If a layout intent was passed via URL, skip saved layout restoration
-      const restored = !initialLayout && tryRestoreLayout(api, currentEnvId, VALID_COMPONENTS);
+      const restored =
+        !initialLayout && restoreEnvLayout(api, currentEnvId, appStore, VALID_COMPONENTS);
       if (!restored) {
         buildDefaultLayout(api, initialLayout ?? (compact ? "compact" : undefined));
       }

apps/web/components/task/dockview-layout-restore.test.ts

@@ -1,7 +1,14 @@
-import { describe, it, expect } from "vitest";
-import { sanitizeLayout } from "./dockview-layout-restore";
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import {
+  collectPhantomSessionIdsForEnv,
+  sanitizeLayout,
+  tryRestoreLayout,
+} from "./dockview-layout-restore";
+import * as localStorage from "@/lib/local-storage";
 
 const VALID_COMPONENTS = new Set<string>(["chat", "files", "shell", "git", "terminal"]);
+const PHANTOM_PANEL_ID = "session:phantom";
+const ALIVE_PANEL_ID = "session:alive-1";
 
 /**
  * Build a minimal valid SerializedDockview-shaped object — matches what
@@ -186,4 +193,185 @@ describe("sanitizeLayout - session panel handling", () => {
     expect(Object.keys(result.panels)).not.toContain(SESSION_PANEL_ID);
     expect(Object.keys(result.panels)).toContain("chat");
   });
+
+  describe("excludeSessionIds (per-env restore filter)", () => {
+    function buildLayoutWithTwoSessions(sidA: string, sidB: string) {
+      const layout = buildLayout();
+      layout.grid.root.data[1].data.views = [`session:${sidA}`, `session:${sidB}`];
+      layout.grid.root.data[1].data.activeView = `session:${sidA}`;
+      layout.panels = {
+        files: { id: "files", contentComponent: "files" },
+        chat: { id: "chat", contentComponent: "chat" },
+        git: { id: "git", contentComponent: "git" },
+        shell: { id: "shell", contentComponent: "shell" },
+        [`session:${sidA}`]: { id: `session:${sidA}`, contentComponent: "chat" },
+        [`session:${sidB}`]: { id: `session:${sidB}`, contentComponent: "chat" },
+      };
+      return layout;
+    }
+
+    it("strips session panels whose ids are in excludeSessionIds (phantom panels from deleted tasks)", () => {
+      // Regression for "phantom panels from local/session storage": a stale
+      // session id from a previously-deleted task can end up serialized in an
+      // env-layout (e.g. via a debounced save firing during a task switch).
+      // The env restore must drop session panels we know belong to another env.
+      const aliveId = "alive-1";
+      const phantomId = "phantom-deleted";
+      const layout = buildLayoutWithTwoSessions(aliveId, phantomId);
+      const result = sanitizeLayout(layout, VALID_COMPONENTS, {
+        excludeSessionIds: new Set([phantomId]),
+      });
+      expect(result).not.toBeNull();
+      const alivePanel = `session:${aliveId}`;
+      expect(Object.keys(result.panels)).toContain(alivePanel);
+      expect(Object.keys(result.panels)).not.toContain(`session:${phantomId}`);
+      // The leaf's views list also drops the phantom.
+      const centerLeaf = result.grid.root.data[1];
+      expect(centerLeaf.data.views).toEqual([alivePanel]);
+      expect(centerLeaf.data.activeView).toBe(alivePanel);
+    });
+
+    it("keeps unmapped session ids (preserves still-loading sessions across restore)", () => {
+      // A session id present in the saved layout that the store has not yet
+      // mapped (WS hasn't arrived) MUST be kept — otherwise the user loses
+      // their valid chat tab on load. Reconcile cleans it up later if stale.
+      const layout = buildLayoutWithTwoSessions("a", "b");
+      const result = sanitizeLayout(layout, VALID_COMPONENTS, {
+        excludeSessionIds: new Set<string>(),
+      });
+      expect(result).not.toBeNull();
+      expect(Object.keys(result.panels)).toContain("session:a");
+      expect(Object.keys(result.panels)).toContain("session:b");
+    });
+
+    it("ignores excludeSessionIds when undefined (preserves default per-env behavior)", () => {
+      // Without the option, behavior is unchanged: session panels are kept.
+      const layout = buildLayoutWithTwoSessions("a", "b");
+      const result = sanitizeLayout(layout, VALID_COMPONENTS);
+      expect(result).not.toBeNull();
+      expect(Object.keys(result.panels)).toContain("session:a");
+      expect(Object.keys(result.panels)).toContain("session:b");
+    });
+
+    it("rejects mixing stripSessionPanels with excludeSessionIds at the type level", () => {
+      // The two options are intent-mutually-exclusive; the type union enforces
+      // it so a caller can't silently get strip-wins behavior.
+      const layout = buildLayoutWithTwoSessions("a", "b");
+      const result = sanitizeLayout(layout, VALID_COMPONENTS, {
+        stripSessionPanels: true,
+        // @ts-expect-error - the discriminated union forbids passing both
+        excludeSessionIds: new Set(["a"]),
+      });
+      // Compile-time guard is the contract; this just asserts the call still
+      // returns a usable value so the test isn't a no-op at runtime.
+      expect(result).not.toBeNull();
+    });
+  });
+});
+
+describe("tryRestoreLayout - phantom session panel filtering", () => {
+  beforeEach(() => {
+    vi.restoreAllMocks();
+  });
+
+  function makeFakeApi() {
+    return {
+      fromJSON: vi.fn(),
+      layout: vi.fn(),
+      width: 1600,
+      height: 600,
+      groups: [],
+      panels: [],
+      activeGroup: { id: "g-center" },
+      getPanel: vi.fn(() => null),
+      onDidActiveGroupChange: vi.fn(() => ({ dispose: vi.fn() })),
+      onDidLayoutChange: vi.fn(() => ({ dispose: vi.fn() })),
+    } as unknown as Parameters<typeof tryRestoreLayout>[0];
+  }
+
+  it("restores the (stripped) layout when every session in the saved env-layout is a phantom", () => {
+    // Saved layout had a single session panel — a phantom from a previously-
+    // deleted task. After stripping, no session panels remain. We still
+    // restore the (truncated) layout; useAutoSessionTab will add the active
+    // session's chat panel afterwards.
+    const layout = buildLayout();
+    layout.grid.root.data[1].data.views = [PHANTOM_PANEL_ID];
+    layout.grid.root.data[1].data.activeView = PHANTOM_PANEL_ID;
+    Object.assign(layout.panels, {
+      [PHANTOM_PANEL_ID]: { id: PHANTOM_PANEL_ID, contentComponent: "chat" },
+    });
+    vi.spyOn(localStorage, "getEnvLayout").mockReturnValue(layout);
+    vi.spyOn(localStorage, "getEnvMaximizeState").mockReturnValue(null);
+
+    const api = makeFakeApi();
+    const restored = tryRestoreLayout(api, "env-new", VALID_COMPONENTS, new Set(["phantom"]));
+    expect(restored).toBe(true);
+    expect(api.fromJSON).toHaveBeenCalledOnce();
+    // The phantom was stripped from the layout passed to fromJSON.
+    const restoredLayout = (api.fromJSON as ReturnType<typeof vi.fn>).mock.calls[0][0];
+    expect(Object.keys(restoredLayout.panels)).not.toContain(PHANTOM_PANEL_ID);
+  });
+
+  it("restores normally when at least one session panel survives the filter", () => {
+    const layout = buildLayout();
+    layout.grid.root.data[1].data.views = [ALIVE_PANEL_ID, PHANTOM_PANEL_ID];
+    layout.grid.root.data[1].data.activeView = ALIVE_PANEL_ID;
+    Object.assign(layout.panels, {
+      [ALIVE_PANEL_ID]: { id: ALIVE_PANEL_ID, contentComponent: "chat" },
+      [PHANTOM_PANEL_ID]: { id: PHANTOM_PANEL_ID, contentComponent: "chat" },
+    });
+    vi.spyOn(localStorage, "getEnvLayout").mockReturnValue(layout);
+    vi.spyOn(localStorage, "getEnvMaximizeState").mockReturnValue(null);
+
+    const api = makeFakeApi();
+    const restored = tryRestoreLayout(api, "env-X", VALID_COMPONENTS, new Set(["phantom"]));
+    expect(restored).toBe(true);
+    expect(api.fromJSON).toHaveBeenCalledOnce();
+  });
+
+  it("restores normally when the saved layout has no session panels at all (legit non-session env layout)", () => {
+    // An env may persist a layout with only files/terminal/etc. — no session
+    // panels to begin with. The fallback guard must NOT discard this layout.
+    const layout = buildLayout(); // center has "chat" only, no session:* ids
+    vi.spyOn(localStorage, "getEnvLayout").mockReturnValue(layout);
+    vi.spyOn(localStorage, "getEnvMaximizeState").mockReturnValue(null);
+
+    const api = makeFakeApi();
+    const restored = tryRestoreLayout(api, "env-X", VALID_COMPONENTS, new Set());
+    expect(restored).toBe(true);
+    expect(api.fromJSON).toHaveBeenCalledOnce();
+  });
+});
+
+/**
+ * Regression: an env-layout could be restored with a session panel whose id
+ * referred to a previously-deleted task's session. The fix strips session
+ * panels we KNOW belong to a different env; sessions not yet mapped in the
+ * store are preserved (they may still be loading via WS).
+ */
+describe("collectPhantomSessionIdsForEnv", () => {
+  it("returns session ids whose mapping is a different env", () => {
+    const state = {
+      environmentIdBySessionId: {
+        "sess-1": "env-A",
+        "sess-2": "env-A",
+        "sess-3": "env-B",
+      },
+    };
+    expect(collectPhantomSessionIdsForEnv(state, "env-A")).toEqual(new Set(["sess-3"]));
+    expect(collectPhantomSessionIdsForEnv(state, "env-B")).toEqual(new Set(["sess-1", "sess-2"]));
+  });
+
+  it("returns every mapped session as phantom when the env has no own sessions yet", () => {
+    const state = { environmentIdBySessionId: { "sess-1": "env-A" } };
+    expect(collectPhantomSessionIdsForEnv(state, "env-new")).toEqual(new Set(["sess-1"]));
+  });
+
+  it("does NOT classify a session as a phantom when its mapping is absent (still loading via WS)", () => {
+    // A session id present in a saved layout but not in environmentIdBySessionId
+    // could be a not-yet-arrived session for this very env. Keep it; reconcile
+    // will clean it up later if it really is stale.
+    const state = { environmentIdBySessionId: {} };
+    expect(collectPhantomSessionIdsForEnv(state, "env-A")).toEqual(new Set());
+  });
 });

apps/web/components/task/dockview-layout-restore.ts

@@ -1,9 +1,11 @@
 import type { DockviewReadyEvent, SerializedDockview } from "dockview-react";
+import type { StoreApi } from "zustand";
 import { useDockviewStore } from "@/lib/state/dockview-store";
 import { applyLayoutFixups } from "@/lib/state/dockview-layout-builders";
 import { isLayoutShapeHealthy } from "@/lib/state/dockview-layout-health";
 import { measureDockviewContainer } from "@/lib/state/dockview-measure";
 import type { LayoutState } from "@/lib/state/layout-manager";
+import type { AppState } from "@/lib/state/store";
 import { getEnvLayout, getEnvMaximizeState, removeEnvMaximizeState } from "@/lib/local-storage";
 
 const LAYOUT_STORAGE_KEY = "dockview-layout-v1";
@@ -12,7 +14,9 @@ const LAYOUT_STORAGE_KEY = "dockview-layout-v1";
 export function sanitizeLayout(
   layout: any,
   validComponents: Set<string>,
-  options: { stripSessionPanels?: boolean } = {},
+  options:
+    | { stripSessionPanels: true; excludeSessionIds?: never }
+    | { stripSessionPanels?: false | undefined; excludeSessionIds?: Set<string> } = {},
 ): any {
   if (!isLayoutShapeHealthy(layout)) return null;
 
@@ -29,6 +33,18 @@ export function sanitizeLayout(
     if (id.startsWith("session:")) {
       if (options.stripSessionPanels) {
         invalidIds.add(id);
+      } else if (options.excludeSessionIds) {
+        // Per-env restore: drop session panels that we know belong to a
+        // different env (a phantom from a previously-deleted task). Sessions
+        // we have no mapping for are kept — they may be a still-loading WS
+        // arrival, and useAutoSessionTab's reconcile will clean them up if
+        // they turn out to be stale.
+        const sid = id.slice("session:".length);
+        if (options.excludeSessionIds.has(sid)) {
+          invalidIds.add(id);
+        } else {
+          validPanels[id] = panel;
+        }
       } else {
         validPanels[id] = panel;
       }
@@ -111,21 +127,37 @@ function tryRestoreMaximizeOnly(api: DockviewReadyEvent["api"], envId: string):
   }
 }
 
+/**
+ * Restore the per-env saved layout, after sanitizing phantom session panels.
+ * Returns true on a successful fromJSON, false when no usable saved layout
+ * exists (caller falls through to maximize-only / global / default build).
+ */
+function tryRestoreEnvLayout(
+  api: DockviewReadyEvent["api"],
+  envId: string,
+  validComponents: Set<string>,
+  phantomSessionIds: Set<string> | undefined,
+): boolean {
+  const envLayout = getEnvLayout(envId);
+  if (!envLayout) return false;
+  const sanitized = sanitizeLayout(envLayout, validComponents, {
+    excludeSessionIds: phantomSessionIds,
+  });
+  if (!sanitized) return false;
+  api.fromJSON(sanitized as SerializedDockview);
+  applyFixupsWithMaximize(api, envId);
+  return true;
+}
+
 export function tryRestoreLayout(
   api: DockviewReadyEvent["api"],
   currentEnvId: string | null,
   validComponents: Set<string>,
+  phantomSessionIds?: Set<string>,
 ): boolean {
   if (currentEnvId) {
     try {
-      const envLayout = getEnvLayout(currentEnvId);
-      if (envLayout) {
-        const sanitized = sanitizeLayout(envLayout, validComponents);
-        if (!sanitized) return false;
-        api.fromJSON(sanitized as SerializedDockview);
-        applyFixupsWithMaximize(api, currentEnvId);
-        return true;
-      }
+      if (tryRestoreEnvLayout(api, currentEnvId, validComponents, phantomSessionIds)) return true;
     } catch {
       // fall through to maximize-only / global fallback
     }
@@ -151,3 +183,39 @@ export function tryRestoreLayout(
 
   return false;
 }
+
+/**
+ * Collect session ids that DEFINITIVELY belong to a different env than `envId`.
+ * These are phantoms (typically from a previously-deleted task) that must be
+ * stripped on env-layout restore.
+ *
+ * Sessions absent from `environmentIdBySessionId` are NOT classified as
+ * phantoms — they may be a still-loading WS arrival that legitimately belongs
+ * to this env. `useAutoSessionTab`'s reconcile cleans up anything that turns
+ * out to be stale once the store catches up.
+ */
+export function collectPhantomSessionIdsForEnv(
+  state: { environmentIdBySessionId: Record<string, string> },
+  envId: string,
+): Set<string> {
+  const result = new Set<string>();
+  for (const [sessionId, mappedEnv] of Object.entries(state.environmentIdBySessionId)) {
+    if (mappedEnv && mappedEnv !== envId) result.add(sessionId);
+  }
+  return result;
+}
+
+/**
+ * Restore the env's saved layout, stripping session panels that we KNOW
+ * belong to a different env — guards against phantom panels from
+ * previously-deleted tasks resurfacing on restore.
+ */
+export function restoreEnvLayout(
+  api: DockviewReadyEvent["api"],
+  envId: string | null,
+  appStore: StoreApi<AppState>,
+  validComponents: Set<string>,
+): boolean {
+  const phantoms = envId ? collectPhantomSessionIdsForEnv(appStore.getState(), envId) : undefined;
+  return tryRestoreLayout(api, envId, validComponents, phantoms);
+}

apps/web/lib/state/dockview-env-switch.test.ts

@@ -204,6 +204,47 @@ describe("performEnvSwitch", () => {
   });
 });
 
+describe("performEnvSwitch slow-path stale session strip", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it("closes stale session chat panels after the slow-path fromJSON", () => {
+    // Regression: a saved env layout could carry a `session:*` panel from a
+    // previously-deleted task (phantom). On the slow-path restore, that
+    // panel would land in the live api as a stray tab. removeStaleSessionPanels
+    // must close any session:* panel whose id != the incoming active session.
+    vi.mocked(getEnvLayout)
+      .mockReturnValueOnce(makeHealthyLayoutWith({}))
+      .mockReturnValueOnce(makeHealthyLayoutWith({}));
+    // Force the slow path: layouts don't structurally match.
+    vi.mocked(savedLayoutMatchesLive).mockReturnValueOnce(false);
+
+    const closeStale = vi.fn();
+    const closeFileEditor = vi.fn();
+    const closeKeep = vi.fn();
+    const api = {
+      ...makeMockApi(),
+      // api.fromJSON is a no-op mock; populate `panels` with what would exist
+      // post-restore so removeStaleSessionPanels' filter has something to act on.
+      panels: [
+        { id: "session:old-session", api: { component: "chat", close: closeStale } },
+        { id: NEW_SESSION_PANEL_ID, api: { component: "chat", close: closeKeep } },
+        // file editors are NOT session panels — they must NOT be closed.
+        { id: "preview:file-editor", api: { component: "file-editor", close: closeFileEditor } },
+      ],
+    } as unknown as EnvSwitchParams["api"];
+    const params = makeParams({ api });
+
+    performEnvSwitch(params);
+
+    expect(closeStale).toHaveBeenCalledOnce();
+    expect(closeKeep).not.toHaveBeenCalled();
+    expect(closeFileEditor).not.toHaveBeenCalled();
+    expect(params.api.fromJSON).toHaveBeenCalledOnce();
+  });
+});
+
 describe("performEnvSwitch fast-path active view restoration", () => {
   beforeEach(() => {
     vi.clearAllMocks();

apps/web/lib/state/dockview-env-switch.ts

@@ -147,6 +147,30 @@ function removeEphemeralPanels(api: DockviewApi, keepSessionId: string | null):
   }
 }
 
+/**
+ * Close stale session chat panels — any `session:*` panel whose id isn't
+ * `session:${keepSessionId}`. Used after `fromJSON` in the slow path to
+ * strip phantom sessions carried in from a saved layout, WITHOUT touching
+ * file-editors/diffs/browser/etc. that legitimately belong to this env.
+ */
+function removeStaleSessionPanels(api: DockviewApi, keepSessionId: string | null): void {
+  const keepId = keepSessionId ? `session:${keepSessionId}` : null;
+  // keepId=null (sessionless task) → strips all session panels, unlike the
+  // fast path's shouldRemoveDuringSwitch which keeps them. In practice
+  // sessionless tasks should have no session panels; useAutoSessionTab
+  // re-adds the panel when a session arrives.
+  const toRemove = api.panels.filter(
+    (p) => p.api.component === "chat" && p.id.startsWith("session:") && p.id !== keepId,
+  );
+  for (const p of toRemove) {
+    try {
+      p.api.close();
+    } catch {
+      /* panel may already be gone */
+    }
+  }
+}
+
 /**
  * Given the panels of a group and the id of the panel being replaced, return
  * the target tab index for the replacement among the siblings that will
@@ -257,7 +281,7 @@ function addIncomingSessionPanel(
  * env-scoped portals before calling this function.
  */
 export function performEnvSwitch(params: EnvSwitchParams): LayoutGroupIds {
-  const { api, newEnvId, safeWidth, safeHeight, buildDefault } = params;
+  const { api, newEnvId, activeSessionId, safeWidth, safeHeight, buildDefault } = params;
 
   const fastResult = tryFastEnvSwitch(params);
   if (fastResult) return fastResult;
@@ -266,6 +290,12 @@ export function performEnvSwitch(params: EnvSwitchParams): LayoutGroupIds {
   if (saved) {
     try {
       api.fromJSON(saved as SerializedDockview);
+      // Saved layout may carry a stale session panel from a previously-deleted
+      // task (phantom). Strip session panels that don't belong to the incoming
+      // active session — file editors/diffs/etc. were legitimately part of
+      // this env's saved state and must NOT be touched.
+      // useAutoSessionTab will add the current session's panel if missing.
+      removeStaleSessionPanels(api, activeSessionId);
       api.layout(safeWidth, safeHeight);
       return applyLayoutFixups(api);
     } catch (err) {