From 87d044f18db8df5b8ca9115b7f8ff35d9ca1dd88 Mon Sep 17 00:00:00 2001
From: gsxdsm <gsxdsm@users.noreply.github.com>
Date: Wed, 3 Jun 2026 13:16:51 -0700
Subject: [PATCH] fix(dashboard): skill-menu highlight survives identity-only
 skills revalidation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Root cause of the recurring ChatView skill-menu CI flakes (and a real UX
bug): the highlight-reset effect keyed on filteredSkills array identity,
but useDiscoveredSkillsCache (SWR) re-delivers content-identical lists
with fresh identities — cache reads re-parse JSON and revalidation
notifies a new array. A revalidation landing between a user's (or the
test's) arrow-key press and the next frame wiped the highlight back to 0.

Key the reset on the joined skill-id list instead, so only a semantic
list change resets the keyboard position. Regression test proves the
invariant: deferred revalidation with identical content lands mid-
navigation and the highlight persists (fails on the old identity-keyed
reset in all three vitest projects).

This test family needed three prior stabilization passes (FN-5864,
FN-5745, FN-5725) — this addresses the underlying race rather than the
assertions.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 .../dashboard/app/components/ChatView.tsx     | 11 ++++-
 .../components/__tests__/ChatView.test.tsx    | 42 +++++++++++++++++++
 2 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/packages/dashboard/app/components/ChatView.tsx b/packages/dashboard/app/components/ChatView.tsx
index 8ef3e5efe8..08d319750c 100644
--- a/packages/dashboard/app/components/ChatView.tsx
+++ b/packages/dashboard/app/components/ChatView.tsx
@@ -1239,9 +1239,18 @@ export function ChatView({ projectId, addToast, experimentalFeatures }: ChatView
     return byName;
   }, [mentionAgents]);
 
+  // Key the reset on skill ids, not array identity: useDiscoveredSkillsCache
+  // (SWR) re-delivers content-identical lists with fresh identities (cache
+  // reads re-parse; revalidation notifies a new array). Resetting on identity
+  // alone wipes the user's keyboard highlight mid-navigation when a
+  // revalidation lands — only a *semantic* list change should reset it.
+  const filteredSkillsKey = useMemo(
+    () => filteredSkills.map((skill) => skill.id).join(" "),
+    [filteredSkills],
+  );
   useEffect(() => {
     setHighlightedSkillIndex(0);
-  }, [filteredSkills]);
+  }, [filteredSkillsKey]);
 
   useEffect(() => {
     setMentionHighlightIndex(0);
diff --git a/packages/dashboard/app/components/__tests__/ChatView.test.tsx b/packages/dashboard/app/components/__tests__/ChatView.test.tsx
index e5edcd3f96..9280decbb6 100644
--- a/packages/dashboard/app/components/__tests__/ChatView.test.tsx
+++ b/packages/dashboard/app/components/__tests__/ChatView.test.tsx
@@ -19,6 +19,7 @@ import * as useChatModule from "../../hooks/useChat";
 import type { UseChatReturn, ChatSessionInfo, ChatMessageInfo, ToolCallInfo } from "../../hooks/useChat";
 import * as apiModule from "../../api";
 import { _resetInitialViewportHeight } from "../../hooks/useMobileKeyboard";
+import { SWR_CACHE_KEYS, writeCache } from "../../utils/swrCache";
 import * as useChatRoomsModule from "../../hooks/useChatRooms";
 import type { UseChatRoomsResult } from "../../hooks/useChatRooms";
 
@@ -1844,6 +1845,47 @@ describe("ChatView", () => {
       await waitFor(() => expect(textarea).toHaveValue("/skill:gamma "));
     });
 
+    it("keeps the keyboard highlight when revalidation re-delivers an identical skill list", async () => {
+      // Regression: the SWR skills cache re-delivers content-identical lists
+      // with fresh array identities (cache reads re-parse; revalidation
+      // notifies a new array). The highlight reset must key on skill ids, not
+      // array identity, or a revalidation landing mid-navigation wipes the
+      // user's keyboard position (the source of this test family's CI flakes).
+      const skillsList = [
+        createMockSkill({ id: "skill-alpha", name: "alpha", relativePath: "skills/alpha.md" }),
+        createMockSkill({ id: "skill-beta", name: "beta", relativePath: "skills/beta.md" }),
+        createMockSkill({ id: "skill-gamma", name: "gamma", relativePath: "skills/gamma.md" }),
+      ];
+      // Seed the cache so the menu renders before the (deferred) revalidation fetch.
+      writeCache(`${SWR_CACHE_KEYS.DISCOVERED_SKILLS_PREFIX}proj-123`, skillsList);
+      let resolveFetch!: (skills: DiscoveredSkill[]) => void;
+      mockFetchDiscoveredSkills.mockImplementationOnce(
+        () => new Promise<DiscoveredSkill[]>((resolve) => { resolveFetch = resolve; }),
+      );
+      setupMockChat({ activeSession: activeSessionFixture, messages: [] });
+      render(<ChatView projectId="proj-123" addToast={vi.fn()} />);
+
+      const textarea = screen.getByTestId("chat-input");
+      fireEvent.change(textarea, { target: { value: "/" } });
+      await screen.findByRole("option", { name: /alpha/i });
+
+      fireEvent.keyDown(textarea, { key: "ArrowUp" });
+      await waitFor(() =>
+        expect(screen.getByRole("option", { name: /gamma/i })).toHaveClass(
+          "chat-skill-menu-item--highlighted",
+        ),
+      );
+
+      // Revalidation lands mid-navigation: identical content, new identity.
+      await act(async () => {
+        resolveFetch(JSON.parse(JSON.stringify(skillsList)) as DiscoveredSkill[]);
+      });
+
+      expect(screen.getByRole("option", { name: /gamma/i })).toHaveClass(
+        "chat-skill-menu-item--highlighted",
+      );
+    });
+
     it("supports selecting highlighted skill with Tab", async () => {
       mockFetchDiscoveredSkills.mockResolvedValueOnce([
         createMockSkill({ id: "skill-alpha", name: "alpha", relativePath: "skills/alpha.md" }),