fix(dashboard): skill-menu highlight survives identity-only skills revalidation

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>
This commit is contained in:
gsxdsm
2026-06-03 13:16:51 -07:00
parent e08a5633fa
commit 87d044f18d
2 changed files with 52 additions and 1 deletions

View File

@@ -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);

View File

@@ -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" }),