fix: address PR review feedback (#1467)

- guard optional project selection callbacks and keep bookmarked search matches visible
- use structured timeout/rate-limit handling in retry backoff
- add focused regressions for selector and retry behavior

Note: pnpm test passed the merge gate but hit pre-existing non-blocking engine mock failures in changed-package tests.

.changeset/fn-1467-pr-feedback-fixes.md

@@ -0,0 +1,3 @@
+"@runfusion/fusion": patch
+
+Fix project selector review regressions around optional selection handlers and bookmarked search matches, and tighten retry/backoff timeout and rate-limit handling.

packages/dashboard/app/components/Header.tsx

@@ -836,7 +836,7 @@ export function Header({
         )}
 
         {/* Project Selector - Back button when project selected, dropdown when 2+ projects (tablet + desktop) */}
-        {!isMobile && projects.length >= 1 && onViewAllProjects && (
+        {!isMobile && projects.length >= 1 && onViewAllProjects && onSelectProject && (
           <StandaloneProjectSelector
             projects={projects}
             currentProject={currentProject ?? null}

packages/dashboard/app/components/ProjectSelector.tsx

@@ -169,27 +169,31 @@ export function ProjectSelector({
   const displayProjects = useMemo(() => {
     const recentIds = new Set(recentProjects.map((p) => p.id));
     const currentId = currentProject?.id;
+    const hasSearch = Boolean(searchQuery.trim());
 
     // Bookmarked projects (excluding current)
-    const bookmarked = filteredProjects.filter(
-      (p) =>
-        p.id !== currentId &&
-        bookmarkedIds.has(p.id) &&
-        !recentIds.has(p.id)
-    );
+    const bookmarked = hasSearch
+      ? []
+      : filteredProjects.filter(
+          (p) =>
+            p.id !== currentId &&
+            bookmarkedIds.has(p.id) &&
+            !recentIds.has(p.id)
+        );
 
-    // Exclude current, bookmarked, and recent from "others"
+    // Exclude current, bookmarked, and recent from "others" only when those
+    // sections are visible. Search mode surfaces every matching project here.
     const bookmarkedAndRecentIds = new Set([
       ...bookmarked.map((p) => p.id),
-      ...recentIds,
+      ...(hasSearch ? [] : recentIds),
     ]);
     const others = filteredProjects.filter(
       (p) => p.id !== currentId && !bookmarkedAndRecentIds.has(p.id)
     );
 
     return {
-      bookmarked: searchQuery.trim() ? [] : bookmarked,
-      recent: searchQuery.trim() ? [] : recentProjects,
+      bookmarked,
+      recent: hasSearch ? [] : recentProjects,
       others,
     };
   }, [filteredProjects, recentProjects, currentProject, searchQuery, bookmarkedIds]);
@@ -228,13 +232,13 @@ export function ProjectSelector({
 
             if (highlightedIndex < bookmarkedCount) {
               // Select bookmarked project
-              onSelect(displayProjects.bookmarked[highlightedIndex]);
+              onSelect?.(displayProjects.bookmarked[highlightedIndex]);
             } else if (highlightedIndex < bookmarkedCount + recentCount) {
               // Select recent project
-              onSelect(displayProjects.recent[highlightedIndex - bookmarkedCount]);
+              onSelect?.(displayProjects.recent[highlightedIndex - bookmarkedCount]);
             } else if (highlightedIndex < bookmarkedCount + recentCount + othersCount) {
               // Select other project
-              onSelect(displayProjects.others[highlightedIndex - bookmarkedCount - recentCount]);
+              onSelect?.(displayProjects.others[highlightedIndex - bookmarkedCount - recentCount]);
             } else {
               // View All
               onViewAll();
@@ -297,7 +301,7 @@ export function ProjectSelector({
   // Handle project selection
   const handleSelectProject = useCallback(
     (project: ProjectInfo) => {
-      onSelect(project);
+      onSelect?.(project);
       setIsOpen(false);
       setSearchQuery("");
     },

packages/dashboard/app/components/__tests__/ProjectSelector.test.tsx

@@ -193,6 +193,23 @@ describe("ProjectSelector", () => {
     expect(onSelect).toHaveBeenCalledWith(projectTwo);
   });
 
+  it("does not throw when clicking a project without onSelect", () => {
+    render(
+      <ProjectSelector
+        projects={[
+          makeProject({ id: "proj_1", name: "Project One" }),
+          makeProject({ id: "proj_2", name: "Project Two" }),
+        ]}
+        currentProject={makeProject({ id: "proj_1" })}
+        onViewAll={noop}
+      />
+    );
+
+    fireEvent.click(screen.getByTestId("project-selector-trigger"));
+    expect(() => fireEvent.click(screen.getByText("Project Two"))).not.toThrow();
+    expect(screen.queryByTestId("project-selector-dropdown")).toBeNull();
+  });
+
   it("closes dropdown after selection", () => {
     const onSelect = vi.fn();
 
@@ -731,6 +748,53 @@ describe("ProjectSelector", () => {
       expect(screen.getByTestId("project-selector-no-results")).toBeDefined();
     });
 
+    it("shows bookmarked projects that match an active search", () => {
+      mockBookmarkedIds = new Set(["proj_2"]);
+
+      render(
+        <ProjectSelector
+          projects={[
+            makeProject({ id: "proj_1", name: "Alpha" }),
+            makeProject({ id: "proj_2", name: "Starred Project" }),
+          ]}
+          currentProject={makeProject({ id: "proj_1" })}
+          onSelect={noop}
+          onViewAll={noop}
+        />
+      );
+
+      fireEvent.click(screen.getByTestId("project-selector-trigger"));
+      fireEvent.change(screen.getByPlaceholderText("Search projects..."), {
+        target: { value: "Starred" },
+      });
+
+      expect(screen.getByText("Starred")).toBeDefined();
+      expect(screen.getByText("Project")).toBeDefined();
+      expect(screen.queryByTestId("project-selector-no-results")).toBeNull();
+    });
+
+    it("does not throw when pressing Enter on a highlighted project without onSelect", async () => {
+      render(
+        <ProjectSelector
+          projects={[
+            makeProject({ id: "proj_1", name: "Alpha" }),
+            makeProject({ id: "proj_2", name: "Beta" }),
+          ]}
+          currentProject={makeProject({ id: "proj_1" })}
+          onViewAll={noop}
+        />
+      );
+
+      fireEvent.click(screen.getByTestId("project-selector-trigger"));
+      const searchInput = screen.getByPlaceholderText("Search projects...");
+      fireEvent.change(searchInput, { target: { value: "Beta" } });
+      await waitFor(() => {
+        expect(screen.getByTestId("project-selector-item-proj_2").className).toContain("highlighted");
+      });
+      expect(() => fireEvent.keyDown(searchInput, { key: "Enter" })).not.toThrow();
+      expect(screen.queryByTestId("project-selector-dropdown")).toBeNull();
+    });
+
     it("clear button clears search query", () => {
       render(
         <ProjectSelector

packages/engine/src/__tests__/retry-with-backoff.test.ts

@@ -327,6 +327,22 @@ describe("withRetry", () => {
     expect(onRetry).not.toHaveBeenCalled();
   });
 
+  it("does not let a custom retry check override RateLimitError instances", async () => {
+    const fn = vi.fn().mockRejectedValue(new RateLimitError("429"));
+    const onRetry = vi.fn();
+
+    await expect(
+      withRetry(fn, {
+        baseDelayMs: 100,
+        onRetry,
+        isRetryable: () => true,
+      }),
+    ).rejects.toThrow("429");
+
+    expect(fn).toHaveBeenCalledTimes(1);
+    expect(onRetry).not.toHaveBeenCalled();
+  });
+
   it("applies exponential backoff with increasing delays", async () => {
     const fn = vi
       .fn()
@@ -586,6 +602,27 @@ describe("withRetry", () => {
     const retryErr = onRetry.mock.calls[0][2];
     expect(retryErr.code).toBe("TIMEOUT");
   });
+
+  it("passes a per-attempt abort signal that is aborted on timeout", async () => {
+    let attemptSignal: AbortSignal | undefined;
+    const fn = vi.fn((signal?: AbortSignal) => {
+      attemptSignal = signal;
+      return new Promise(() => {});
+    });
+
+    const promise = withRetry(fn, {
+      maxRetries: 0,
+      timeoutMs: 500,
+    });
+    const assertion = expect(promise).rejects.toBeInstanceOf(TimeoutError);
+
+    await vi.advanceTimersByTimeAsync(600);
+
+    await assertion;
+    expect(attemptSignal).toBeInstanceOf(AbortSignal);
+    expect(attemptSignal?.aborted).toBe(true);
+    expect(attemptSignal?.reason).toBeInstanceOf(TimeoutError);
+  });
 });
 
 describe("withRetryResult", () => {

packages/engine/src/engine-errors.ts

@@ -244,7 +244,7 @@ export function classifyThrownError(err: unknown): EngineError {
     return new ServiceUnavailableError(message, undefined, undefined, err instanceof Error ? err : undefined);
   }
 
-  if (/"type":"server_error"|\"code\":\"server_error\"/i.test(message)) {
+  if (/"type":"server_error"|"code":"server_error"/i.test(message)) {
     return new ServiceUnavailableError(message, 500, undefined, err instanceof Error ? err : undefined);
   }
 

packages/engine/src/retry-with-backoff.ts

@@ -40,7 +40,13 @@
  * ```
  */
 
-import { classifyThrownError, isRetryableError, type EngineError } from "./engine-errors.js";
+import {
+  classifyThrownError,
+  isRetryableError,
+  RateLimitError,
+  TimeoutError,
+  type EngineError,
+} from "./engine-errors.js";
 import { isUsageLimitError } from "./usage-limit-detector.js";
 
 // ── Types ───────────────────────────────────────────────────────────────
@@ -168,7 +174,7 @@ export function cancellableSleep(ms: number, signal?: AbortSignal): Promise<void
  * the AbortController.
  */
 function withTimeout<T>(
-  fn: () => Promise<T>,
+  fn: (signal?: AbortSignal) => Promise<T>,
   timeoutMs: number,
   parentSignal?: AbortSignal,
 ): Promise<T> {
@@ -188,11 +194,12 @@ function withTimeout<T>(
     const timer = setTimeout(() => {
       if (settled) return;
       settled = true;
-      ac.abort(new Error(`Operation timed out after ${timeoutMs}ms`));
-      reject(new Error(`Operation timed out after ${timeoutMs}ms`));
+      const timeoutErr = new TimeoutError(`Operation timed out after ${timeoutMs}ms`, timeoutMs);
+      ac.abort(timeoutErr);
+      reject(timeoutErr);
     }, timeoutMs);
 
-    fn()
+    fn(ac.signal)
       .then((result) => {
         if (settled) return;
         settled = true;
@@ -229,7 +236,7 @@ function withTimeout<T>(
  * @returns The return value of `fn()`
  */
 export async function withRetry<T>(
-  fn: () => Promise<T>,
+  fn: (signal?: AbortSignal) => Promise<T>,
   options: RetryOptions = {},
 ): Promise<T> {
   const {
@@ -243,9 +250,7 @@ export async function withRetry<T>(
     isRetryable: customIsRetryable,
   } = options;
 
-  const startTime = Date.now();
   let lastError: EngineError | undefined;
-  let retryCount = 0;
 
   for (let attempt = 0; attempt <= maxRetries; attempt++) {
     // Check abort before each attempt
@@ -257,14 +262,14 @@ export async function withRetry<T>(
       // Wrap with timeout if configured
       const result = timeoutMs
         ? await withTimeout(fn, timeoutMs, signal)
-        : await fn();
+        : await fn(signal);
       return result;
     } catch (err: unknown) {
       // Classify the error into a structured type
       const classified = classifyThrownError(err);
 
       // Rate-limit errors: never retry locally — re-throw immediately
-      if (isUsageLimitError(classified.message)) {
+      if (classified instanceof RateLimitError || isUsageLimitError(classified.message)) {
         throw classified;
       }
 
@@ -297,8 +302,6 @@ export async function withRetry<T>(
 
       // Sleep with cancellation support
       await cancellableSleep(delay, signal);
-
-      retryCount++;
     }
   }
 
@@ -317,17 +320,14 @@ export async function withRetry<T>(
  * @returns A `RetryResult<T>` with value and retry metadata
  */
 export async function withRetryResult<T>(
-  fn: () => Promise<T>,
+  fn: (signal?: AbortSignal) => Promise<T>,
   options: RetryOptions = {},
 ): Promise<RetryResult<T>> {
   const startTime = Date.now();
   let retries = 0;
 
-  const result = await withRetry<T>(async () => {
-    if (retries > 0) {
-      // We're in a retry — count it
-    }
-    return fn();
+  const result = await withRetry<T>(async (signal) => {
+    return fn(signal);
   }, {
     ...options,
     onRetry: (attempt, delayMs, err) => {