[Critical] Worker thread leak in the CLI's @-picker path. useAtCompletion (in packages/cli/src/ui/hooks/useAtCompletion.ts) creates a FileSearch via FileSearchFactory.create() and stores it in fileSearch.current. When cwd/config changes trigger re-initialization or the component unmounts, the old FileSearch is silently overwritten without calling dispose(). The VSCode path was correctly wired up in FileMessageHandler.clearFileSearchCache(), but the CLI hook — by far the most common consumer — was missed.

On workspaces with ≥5 000 files, each RecursiveFileSearch holds a FzfWorkerHandle that owns a worker_threads Worker (~5–10 MB + full file list). In a long-running CLI session, every directory switch or config reload leaks another worker. worker.unref() prevents blocking process.exit(), but the workers accumulate for the lifetime of the process.

Fix in useAtCompletion.ts:

Also dispose the previous instance before overwriting in initialize().

— qwen3.7-max via Qwen Code /review

Fixed in 81a513d. Added dispose() calls in both the initialize() path (dispose old instance before creating new one) and the effect cleanup function. Worker threads are now properly released on re-init and unmount.

[Suggestion] Two robustness gaps in create():

1. Worker leak on init failure: If inst.ready() rejects (worker sends init-error or crashes during construction), create() propagates the error without calling inst.dispose(). The spawned Worker is never terminated. In daemon mode, repeated init failures accumulate idle workers.

2. No timeout on readyPromise: If the worker hangs during new AsyncFzf(...) (e.g., OOM on a huge file list, OS scheduling stall), create() blocks the caller indefinitely with no diagnostic output. The @-picker UI freezes on "loading..." with no error, no stack, no way out.

— qwen3.7-max via Qwen Code /review

Fixed in 81a513d. create() now wraps inst.ready() in a try/catch that calls dispose() on failure. Also added a Promise.race timeout scaled to file count (Math.max(10_000, files.length / 10) ms) so a hung worker doesn't block the @-picker indefinitely.

[Suggestion] fzf.find() returns ALL matching items, which are all serialized via postMessage structured clone. On a large workspace with a broad single-character pattern, this can produce 50k+ results (~2.5 MB of IPC payload). The consumer in fileSearch.ts truncates to maxResults (~72 items) only after the full result set crosses the message channel.

Consider adding a limit field to the FindMessage protocol and slicing results in the worker before serialization:

// In FindMessage:
interface FindMessage {
  type: 'find';
  reqId: number;
  pattern: string;
  limit?: number;
}

// In the .then() handler:
const limit = msg.limit ?? items.length;
const trimmed = items.slice(0, limit).map((entry) => ({ item: entry.item }));

A generous limit (e.g., 200) caps IPC at ~10 KB while giving the main thread enough candidates for downstream filtering.

— qwen3.7-max via Qwen Code /review

Fixed in 81a513d. Added limit?: number to FindMessage protocol. Worker now calls .slice(0, limit) before postMessage. The caller in fileSearch.ts passes Math.max(200, maxResults * 3) — generous enough for downstream ignore-filter to drop entries without starving results, while capping IPC at ~10 KB instead of multi-MB payloads on broad patterns.

[Suggestion] The worker transport code path (138 lines in fzfWorkerHandle.ts — the core feature of this PR) has zero test coverage. All 5 tests here use either the default in-process fallback (tiny inputs below the 5 000 threshold) or explicitly call installInProcessFzfTransport(). No test spawns a real worker_threads Worker.

Untested paths include:

• Worker spawn → init → find → dispose lifecycle
• init-error / find-error message handling
• Worker unexpected exit → failAll() rejection of pending promises
• Dispose-then-find race condition (pending finds should reject)

The in-process tests verify the interface contract, but bugs in IPC message handling, result reconstruction (positions: new Set<number>() reassembly), or worker lifecycle would go undetected.

— qwen3.7-max via Qwen Code /review

Fixed in 81a513d. Added a worker transport (real worker_threads) describe block with 5 tests that use __setWorkerThresholdForTests(1) without installInProcessFzfTransport() to exercise the real spawn/IPC path:

• spawn → init → find → dispose lifecycle
• find() rejects after dispose()
• limit parameter across IPC
• create() rejects and disposes on init failure
• concurrent find() calls

The tests build fzfWorker.js on-the-fly via esbuild in beforeAll and clean up in afterAll. Also discovered and fixed a bug: find() after dispose() would hang forever — now rejects immediately.

[Critical] Effect cleanup disposes the worker on every dependency change, silently breaking @-completion search.

The effect's dependency array is [state.status, state.pattern, config, cwd]. Every status transition (e.g., INITIALIZING→READY after initialize() completes, or READY→SEARCHING on keystroke) triggers this cleanup, which disposes the just-created FzfWorkerHandle and sets fileSearch.current = null.

Trace: initialize() creates a searcher, assigns fileSearch.current, dispatches INITIALIZE_SUCCESS → status becomes READY → cleanup fires → dispose() kills the worker → fileSearch.current = null. Then SEARCHING triggers search() which early-returns because !fileSearch.current.

The base version's cleanup only aborted the search controller and cleared the timer — the searcher ref was preserved across re-renders.

Move the dispose() + null assignment to a dedicated lifecycle effect with [cwd, config] deps (matching the existing RESET dispatch), so it only fires on workspace/config changes and unmount — not on every keystroke:

useEffect(() => {
  return () => {
    void fileSearch.current?.dispose?.();
    fileSearch.current = null;
  };
}, [cwd, config]);

— qwen3.7-max via Qwen Code /review

Fixed in f3b9666. Moved dispose() out of the worker effect cleanup (which fires on every [state.status, state.pattern] change) into a separate effect with [cwd, config] deps — now only fires on workspace/config change and unmount.

[Suggestion] setTimeout inside Promise.race is never cleared when inst.ready() resolves first (the happy path). The timer stays alive for Math.max(10_000, files.length / 10) ms (10 s for ≤100k files, up to 100 s at MAX_CRAWL_FILES), holding a closure over the files array and preventing GC.

— qwen3.7-max via Qwen Code /review

Fixed in f3b9666. Added timerId variable and clearTimeout(timerId) in a finally block so the timer is cleaned up on both success and failure paths.

[Suggestion] setupFileWatchers() returns a Disposable that disposes file watchers and the workspace-folders change listener, but never iterates this.fileSearchInstances to call dispose() on the cached FileSearch instances. When the webview panel closes, any RecursiveFileSearch instances that promoted to the worker path (≥5k files) retain live worker threads.

Consider extending the Disposable returned by setupFileWatchers():

return {
  dispose: () => {
    for (const watcher of this.fileWatchers.values()) watcher.dispose();
    this.fileWatchers.clear();
    foldersChangeListener.dispose();
    for (const instance of this.fileSearchInstances.values()) {
      void instance.dispose?.().catch(() => {});
    }
    this.fileSearchInstances.clear();
    this.fileSearchInitializing.clear();
  },
};

— qwen3.7-max via Qwen Code /review

Fixed in f3b9666. Extended the Disposable returned by setupFileWatchers() to iterate fileSearchInstances and call dispose() on each, plus clear the initializing map.

[Suggestion] The comment says new Worker(new URL('./fzfWorker.js', import.meta.url)) but the actual code in fzfWorkerHandle.ts uses resolveBundleDir(import.meta.url) + path.join(dir, 'fzfWorker.js') — a filesystem-based path resolution, not a new URL() constructor pattern.

— qwen3.7-max via Qwen Code /review

Fixed in f3b9666. Updated comment to reference resolveBundleDir(import.meta.url) + path.join(dir, 'fzfWorker.js') instead of the new URL() pattern.

[Suggestion] worker.on('error') and worker.on('exit') handlers that call failAll() have no test coverage. If failAll() has a bug, pending find() calls could hang indefinitely in production when the worker crashes.

Consider adding a test that forces a worker crash (e.g., by passing a script that throws at the top level) and verifying that pending find() calls reject with an appropriate error message.

— qwen3.7-max via Qwen Code /review

Fixed in f3b9666. Added a test that overwrites fzfWorker.js with a script that calls process.exit(1) on find, then verifies handle.find() rejects with /exited unexpectedly/. This exercises the worker.on('exit') → failAll() path.

[Suggestion] RecursiveFileSearch.dispose() is a new public method but fileSearch.test.ts has no tests for it. Callers (useAtCompletion.ts, FileMessageHandler.ts) rely on dispose() for worker thread cleanup — a regression here would silently leak workers.

Consider adding a test:

it('should release fzf handle on dispose', async () => {
  const fileSearch = FileSearchFactory.create({ ... });
  await fileSearch.initialize();
  await expect(fileSearch.dispose()).resolves.toBeUndefined();
  // Idempotent
  await expect(fileSearch.dispose()).resolves.toBeUndefined();
});

— qwen3.7-max via Qwen Code /review

Fixed in f3b9666. Added two tests in fileSearch.test.ts: one verifying RecursiveFileSearch.dispose() resolves and is idempotent, another confirming DirectoryFileSearch has no dispose method (expected — it holds no worker resources).

[Suggestion] The async initialize() is fire-and-forget with no cancellation guard. If the component unmounts or cwd/config changes while await searcher.initialize() is in flight (worker spawn + ready handshake takes several ms), the cleanup effect runs fileSearch.current?.dispose?.() — but fileSearch.current is still null from line 164, so nothing is disposed. When the pending initialize() resolves, fileSearch.current = searcher writes a live worker into the ref with no owner to dispose it.

In React 18 Strict Mode (dev), the double-invocation makes this deterministic: invocation-1's async init completes after cleanup already ran, leaking a worker thread.

Consider adding a cancelled flag or AbortController that the effect cleanup signals:

const initialize = async (signal: AbortSignal) => {
  try {
    await fileSearch.current?.dispose?.();
    fileSearch.current = null;
    const searcher = FileSearchFactory.create({ ... });
    await searcher.initialize();
    if (signal.aborted) { await searcher.dispose?.(); return; }
    fileSearch.current = searcher;
    // ...
  } catch (err) { /* ... */ }
};

— qwen3.7-max via Qwen Code /review

Fixed in 0dae073. Added a cancelled flag inside the effect closure — set to true in cleanup. After await searcher.initialize(), if cancelled is true the searcher is disposed immediately instead of being written to fileSearch.current. This covers both the unmount-during-init and the Strict Mode double-invocation scenarios.

[Suggestion] beforeAll builds fzfWorker.js into the source tree (packages/core/src/utils/filesearch/fzfWorker.js), but .gitignore has no entry for it. If the test process crashes (OOM, Ctrl-C, segfault) before afterAll runs fs.unlinkSync(outfile), the compiled artifact persists and could be accidentally committed.

A stale fzfWorker.js in the source tree also causes getWorkerScriptPath() to return non-null in tsx/dev mode, making the dev path unexpectedly use the worker transport with a potentially outdated build.

Add packages/core/src/utils/filesearch/fzfWorker.js to .gitignore, or write the test artifact to os.tmpdir() instead.

— qwen3.7-max via Qwen Code /review

Fixed in 0dae073. Added packages/core/src/utils/filesearch/fzfWorker.js to .gitignore so a crash-orphaned test artifact won't be accidentally committed or cause dev-mode to use a stale worker build.

MEDIUM — Concurrent initialize() calls can leak worker threads

The await fileSearch.current?.dispose?.() here only disposes the previously assigned reference. When cwd/config changes rapidly, two initialize() invocations can overlap:

1. Init A starts — disposes old searcher, creates searcherA (spawns worker A), awaits searcherA.initialize().
2. cwd changes again — Init B starts — fileSearch.current is still null (Init A hasn't set it yet) — dispose is a no-op.
3. Init A completes — sets fileSearch.current = searcherA.
4. Init B completes — sets fileSearch.current = searcherB — searcherA and its worker thread are leaked.

This race pre-dates the PR, but each leaked worker_threads worker holds ~1–4 MB stack + a V8 isolate, making the leak materially more expensive than the old in-process AsyncFzf.

A module-level let initGeneration = 0; plus a stale-check after await searcher.initialize() would cancel superseded initializations:

await searcher.initialize();
if (myGeneration !== initGeneration) {
  // A newer initialize() already started — dispose ours and bail.
  await searcher.dispose?.();
  return;
}
fileSearch.current = searcher;

Fixed in 0dae073. Added a cancelled flag inside the effect — the cleanup sets it to true, and after await searcher.initialize() the code checks it before assigning to fileSearch.current. If cancelled (because a newer init already started or the component unmounted), the newly-created searcher is disposed immediately. This prevents the overlap scenario you described where Init A's searcher would be orphaned when Init B overwrites the ref.

[Suggestion] failAll() only transitions readyState to 'failed' when the current state is 'pending'. If the worker crashes after a successful init (state 'ready'), readyState remains 'ready' permanently. Subsequent find() calls pass the 'disposed' guard, await readyPromise resolves instantly, and worker.postMessage() throws on a dead worker. Combined with the .catch(() => { return []; }) in fileSearch.ts:178, the @-picker silently returns zero results for every keystroke with no self-healing until cwd/config change.

Consider making failAll unconditionally mark the handle as failed when not disposed:

const failAll = (err: Error) => {
  if (readyState === 'pending') {
    readyReject(err);
  }
  if (readyState !== 'disposed') {
    readyState = 'failed';
  }
  for (const slot of pending.values()) {
    slot.reject(err);
  }
  pending.clear();
};

And guard find() against the 'failed' state:

async find(pattern, limit?) {
  if (readyState === 'disposed' || readyState === 'failed') {
    throw new Error(`fzf worker not available (${readyState})`);
  }
  // ...
}

— qwen3.7-max via Qwen Code /review

Fixed in 769a236. failAll() now unconditionally sets readyState = 'failed' when the state is not 'disposed', so a post-init worker crash correctly marks the handle as dead. find() now guards against both 'disposed' and 'failed' states, rejecting immediately instead of silently returning empty results via the .catch(() => []) in fileSearch.ts. Added a test that verifies: crash worker on first find(), then assert subsequent find() rejects with /not available.*failed/.