fix(thread-picker): keep one active picker per runtime

src/execution-runtime.ts

@@ -54,7 +54,7 @@ export class CodexExecutionRuntime implements AppServerClientAccess {
   private readonly shortLivedClients = new Set<AppServerClient>();
   private readonly structuredTurnClients = new Set<EphemeralStructuredTurnClient>();
   private readonly structuredTurnOperations = new Set<AbortController>();
-  private readonly threadPickers = new Set<ThreadPickerController>();
+  private activeThreadPicker: ThreadPickerController | null = null;
   private readonly chatViews = new Set<ChatRuntimeView>();
   private readonly threadsViews = new Set<ThreadsRuntimeView>();
   private disposed = false;
@@ -206,13 +206,19 @@ export class CodexExecutionRuntime implements AppServerClientAccess {
   }
 
   openThreadPicker(): void {
-    const picker = openThreadPicker({
-      app: this.options.app,
-      threadCatalog: this.threadCatalog,
-      openThreadInCurrentView: (threadId) => this.options.openThreadInCurrentView(threadId),
-      openThreadInAvailableView: (threadId) => this.options.openThreadInAvailableView(threadId),
-    });
-    this.threadPickers.add(picker);
+    this.activeThreadPicker?.close();
+    const picker = openThreadPicker(
+      {
+        app: this.options.app,
+        threadCatalog: this.threadCatalog,
+        openThreadInCurrentView: (threadId) => this.options.openThreadInCurrentView(threadId),
+        openThreadInAvailableView: (threadId) => this.options.openThreadInAvailableView(threadId),
+      },
+      () => {
+        if (this.activeThreadPicker === picker) this.activeThreadPicker = null;
+      },
+    );
+    this.activeThreadPicker = picker;
   }
 
   dispose(): ExecutionRuntimeViews {
@@ -237,11 +243,10 @@ export class CodexExecutionRuntime implements AppServerClientAccess {
     }
     this.chatViews.clear();
     this.threadsViews.clear();
-    for (const picker of this.threadPickers)
-      this.tryCleanup(() => {
-        picker.close();
-      });
-    this.threadPickers.clear();
+    this.tryCleanup(() => {
+      this.activeThreadPicker?.close();
+    });
+    this.activeThreadPicker = null;
     for (const operation of this.structuredTurnOperations)
       this.tryCleanup(() => {
         operation.abort();

src/features/thread-picker/modal.obsidian.ts

@@ -24,9 +24,15 @@ type ThreadOpenMode = "current" | "available";
 
 const THREAD_PICKER_MODIFIER_ENTER_LISTENER_OPTIONS = { capture: true } as const;
 
-export function openThreadPicker(host: ThreadPickerHost): ThreadPickerController {
+export function openThreadPicker(host: ThreadPickerHost, onClosed: () => void): ThreadPickerController {
   const state: { closed: boolean; modal: ThreadPickerModal | null } = { closed: false, modal: null };
-  void (async () => {
+  const finish = (): void => {
+    if (state.closed) return;
+    state.closed = true;
+    state.modal = null;
+    onClosed();
+  };
+  const loadAndOpen = async (): Promise<void> => {
     try {
       const recentSnapshot = host.threadCatalog.recentActiveSnapshot();
       const loadedThreads = recentSnapshot ?? (await host.threadCatalog.loadActive());
@@ -34,19 +40,22 @@ export function openThreadPicker(host: ThreadPickerHost): ThreadPickerController
       const recentThreads = host.threadCatalog.recentActiveSnapshot() ?? loadedThreads;
       if (recentThreads.length === 0 && !host.threadCatalog.hasMoreActive()) {
         new Notice("No Codex threads found.");
+        finish();
         return;
       }
-      state.modal = new ThreadPickerModal(host, recentThreads);
+      state.modal = new ThreadPickerModal(host, recentThreads, finish);
       state.modal.open();
     } catch (error) {
       if (!state.closed) new Notice(error instanceof Error ? error.message : String(error));
+      finish();
     }
-  })();
+  };
+  queueMicrotask(() => void loadAndOpen());
   return {
     close: () => {
-      state.closed = true;
+      if (state.closed) return;
       state.modal?.close();
-      state.modal = null;
+      finish();
     },
   };
 }
@@ -71,6 +80,7 @@ class ThreadPickerModal extends SuggestModal<ThreadSuggestion> {
   constructor(
     private readonly host: ThreadPickerHost,
     recentThreads: readonly Thread[],
+    private readonly onClosed: () => void,
   ) {
     super(host.app);
     this.recentThreads = Object.freeze([...recentThreads]);
@@ -90,6 +100,7 @@ class ThreadPickerModal extends SuggestModal<ThreadSuggestion> {
   override onClose(): void {
     this.inputEl.removeEventListener("keydown", this.handleInputKeydown, THREAD_PICKER_MODIFIER_ENTER_LISTENER_OPTIONS);
     super.onClose();
+    this.onClosed();
   }
 
   override async getSuggestions(query: string): Promise<ThreadSuggestion[]> {

tests/execution-runtime.test.ts

@@ -0,0 +1,79 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+import { CodexExecutionRuntime } from "../src/execution-runtime";
+import type { ThreadPickerController } from "../src/features/thread-picker/modal.obsidian";
+import { DEFAULT_SETTINGS } from "../src/settings/model";
+
+const { openThreadPickerMock } = vi.hoisted(() => ({
+  openThreadPickerMock: vi.fn(),
+}));
+
+vi.mock("../src/features/thread-picker/modal.obsidian", () => ({
+  openThreadPicker: openThreadPickerMock,
+}));
+
+describe("CodexExecutionRuntime thread picker ownership", () => {
+  beforeEach(() => {
+    openThreadPickerMock.mockReset();
+  });
+
+  it("replaces the active picker and disposes only the current picker", () => {
+    const pickers = pickerFactory();
+    const runtime = executionRuntime();
+
+    runtime.openThreadPicker();
+    runtime.openThreadPicker();
+
+    expect(pickers.controllers[0]?.close).toHaveBeenCalledOnce();
+    expect(pickers.controllers[1]?.close).not.toHaveBeenCalled();
+
+    runtime.dispose();
+
+    expect(pickers.controllers[0]?.close).toHaveBeenCalledOnce();
+    expect(pickers.controllers[1]?.close).toHaveBeenCalledOnce();
+  });
+
+  it("releases a naturally closed picker before runtime disposal", () => {
+    const pickers = pickerFactory();
+    const runtime = executionRuntime();
+
+    runtime.openThreadPicker();
+    pickers.finish[0]?.();
+    runtime.dispose();
+
+    expect(pickers.controllers[0]?.close).not.toHaveBeenCalled();
+  });
+});
+
+function pickerFactory(): {
+  controllers: Array<ThreadPickerController & { close: ReturnType<typeof vi.fn> }>;
+  finish: Array<() => void>;
+} {
+  const controllers: Array<ThreadPickerController & { close: ReturnType<typeof vi.fn> }> = [];
+  const finish: Array<() => void> = [];
+  openThreadPickerMock.mockImplementation((_host: unknown, onClosed: () => void) => {
+    const controller = {
+      close: vi.fn(() => {
+        onClosed();
+      }),
+    };
+    controllers.push(controller);
+    finish.push(onClosed);
+    return controller;
+  });
+  return { controllers, finish };
+}
+
+function executionRuntime(): CodexExecutionRuntime {
+  return new CodexExecutionRuntime({
+    app: {} as never,
+    context: { codexPath: "codex", vaultPath: "/vault" },
+    settings: () => ({ ...DEFAULT_SETTINGS }),
+    workspace: {} as never,
+    onThreadCatalogEvent: vi.fn(),
+    openNewPanel: vi.fn(),
+    openThreadInCurrentView: vi.fn(),
+    openThreadInAvailableView: vi.fn(),
+    openPanelActivities: () => [],
+  });
+}

tests/features/thread-picker/modal.test.ts

@@ -1,7 +1,7 @@
 // @vitest-environment jsdom
 
 import { SuggestModal } from "obsidian";
-import { describe, expect, it } from "vitest";
+import { describe, expect, it, vi } from "vitest";
 
 import type { Thread } from "../../../src/domain/threads/model";
 import { openThreadPicker, type ThreadPickerHost } from "../../../src/features/thread-picker/modal.obsidian";
@@ -120,6 +120,61 @@ describe("threadOpenModeFromEvent", () => {
   });
 });
 
+describe("thread picker lifecycle", () => {
+  it("reports natural close only once", async () => {
+    const onClosed = vi.fn();
+    const { controller, modal } = await openedThreadPickerSession([thread({ id: "thread" })], onClosed);
+
+    modal.onClose();
+    controller.close();
+
+    expect(onClosed).toHaveBeenCalledOnce();
+  });
+
+  it("finishes when the initial inventory is empty", async () => {
+    const onClosed = vi.fn();
+
+    openThreadPicker(threadPickerHost([]), onClosed);
+    await flushMicrotasks();
+
+    expect(onClosed).toHaveBeenCalledOnce();
+  });
+
+  it("finishes when the initial inventory fails to load", async () => {
+    const host = threadPickerHost([], [], [], false);
+    host.threadCatalog.loadActive = async () => {
+      throw new Error("inventory unavailable");
+    };
+    const onClosed = vi.fn();
+
+    openThreadPicker(host, onClosed);
+    await flushMicrotasks();
+
+    expect(onClosed).toHaveBeenCalledOnce();
+  });
+
+  it("does not open after being replaced during the initial inventory load", async () => {
+    const pending = deferred<readonly Thread[]>();
+    const host = threadPickerHost([], [], [], false);
+    host.threadCatalog.loadActive = () => pending.promise;
+    const open = vi.spyOn(SuggestModal.prototype, "open");
+    const onClosed = vi.fn();
+
+    try {
+      const controller = openThreadPicker(host, onClosed);
+      await Promise.resolve();
+      controller.close();
+      pending.resolve([thread({ id: "stale" })]);
+      await flushMicrotasks();
+
+      expect(open).not.toHaveBeenCalled();
+      expect(onClosed).toHaveBeenCalledOnce();
+    } finally {
+      open.mockRestore();
+    }
+  });
+});
+
 interface ThreadSuggestion {
   thread: Thread;
   title: string;
@@ -129,23 +184,33 @@ interface CapturedThreadPickerModal {
   emptyStateText: string;
   resultContainerEl: HTMLElement;
   getSuggestions(query: string): Promise<ThreadSuggestion[]>;
+  onClose(): void;
   onChooseSuggestion(item: ThreadSuggestion, evt: MouseEvent | KeyboardEvent): void;
 }
 
 async function openedThreadPicker(input: readonly Thread[] | TestThreadPickerHost): Promise<CapturedThreadPickerModal> {
+  return (await openedThreadPickerSession(input)).modal;
+}
+
+async function openedThreadPickerSession(
+  input: readonly Thread[] | TestThreadPickerHost,
+  onClosed: () => void = () => undefined,
+): Promise<{ controller: ReturnType<typeof openThreadPicker>; modal: CapturedThreadPickerModal }> {
   const captured: CapturedThreadPickerModal[] = [];
   const originalOpen = Object.getOwnPropertyDescriptor(SuggestModal.prototype, "open");
   SuggestModal.prototype.open = function captureOpen(this: SuggestModal<ThreadSuggestion>): void {
     captured.push(this as CapturedThreadPickerModal);
   };
+  let controller: ReturnType<typeof openThreadPicker>;
   try {
-    await openThreadPicker(isThreadPickerHost(input) ? input : threadPickerHost(input));
+    controller = openThreadPicker(isThreadPickerHost(input) ? input : threadPickerHost(input), onClosed);
+    await flushMicrotasks();
   } finally {
     if (originalOpen) Object.defineProperty(SuggestModal.prototype, "open", originalOpen);
   }
   const modal = captured[0];
   if (!modal) throw new Error("Expected thread picker modal to open");
-  return modal;
+  return { controller, modal };
 }
 
 async function firstSuggestion(modal: CapturedThreadPickerModal): Promise<ThreadSuggestion> {
@@ -225,3 +290,8 @@ function deferred<T>() {
   });
   return { promise, resolve };
 }
+
+async function flushMicrotasks(): Promise<void> {
+  await Promise.resolve();
+  await Promise.resolve();
+}