From 02eb964ff14dc09d5f23a583a52443faa317a160 Mon Sep 17 00:00:00 2001 From: Hiep Le <69354317+hieptl@users.noreply.github.com> Date: Thu, 4 Jun 2026 21:40:19 +0700 Subject: [PATCH] fix: do not gate the skills modal on runtime readiness (#1122) --- .../modals/skills/skill-modal.test.tsx | 50 +++++++------------ .../conversation-panel/skills-modal.tsx | 12 +---- .../skills-runtime-waiting-state.tsx | 20 -------- 3 files changed, 20 insertions(+), 62 deletions(-) delete mode 100644 src/components/features/conversation-panel/skills-runtime-waiting-state.tsx diff --git a/__tests__/components/modals/skills/skill-modal.test.tsx b/__tests__/components/modals/skills/skill-modal.test.tsx index 452ff14e92..0b1e337f22 100644 --- a/__tests__/components/modals/skills/skill-modal.test.tsx +++ b/__tests__/components/modals/skills/skill-modal.test.tsx @@ -4,12 +4,6 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { renderWithProviders } from "test-utils"; import { SkillsModal } from "#/components/features/conversation-panel/skills-modal"; import SkillsService from "#/api/skills-service"; -import { AgentState } from "#/types/agent-state"; -import { useAgentState } from "#/hooks/use-agent-state"; - -vi.mock("#/hooks/use-agent-state", () => ({ - useAgentState: vi.fn(), -})); describe("SkillsModal", () => { const mockOnClose = vi.fn(); @@ -39,10 +33,6 @@ describe("SkillsModal", () => { vi.clearAllMocks(); vi.spyOn(SkillsService, "getSkills").mockResolvedValue(mockSkills); - - vi.mocked(useAgentState).mockReturnValue({ - curAgentState: AgentState.AWAITING_USER_INPUT, - }); }); afterEach(() => { @@ -94,33 +84,31 @@ describe("SkillsModal", () => { }); }); - describe("Runtime waiting state", () => { - it("shows the warning, refresh button, and spinner while the runtime is starting", async () => { - vi.mocked(useAgentState).mockReturnValue({ - curAgentState: AgentState.LOADING, - }); - - renderWithProviders(); - - expect(await screen.findByTestId("refresh-skills")).toBeInTheDocument(); - expect(screen.getByText("SKILLS_MODAL$WARNING")).toBeInTheDocument(); - expect(screen.getByTestId("skills-runtime-waiting")).toBeInTheDocument(); - expect(screen.getByTestId("loading-spinner")).toBeInTheDocument(); - expect( - screen.getByText("DIFF_VIEWER$WAITING_FOR_RUNTIME"), - ).toBeInTheDocument(); - }); - }); - describe("Skills Display", () => { - it("should display skills correctly", async () => { + it("displays the skills catalog when opened with no active conversation (home page)", async () => { + // Arrange: the catalog fetch succeeds; no conversation exists in this + // render, so no runtime ever starts (the original infinite-spinner bug) vi.spyOn(SkillsService, "getSkills").mockResolvedValue(mockSkills); + // Act renderWithProviders(); - await screen.findByText("Test Skill 1"); - expect(screen.getByText("Test Skill 1")).toBeInTheDocument(); + // Assert: the list renders instead of waiting for a runtime + expect(await screen.findByText("Test Skill 1")).toBeInTheDocument(); expect(screen.getByText("Test Skill 2")).toBeInTheDocument(); }); + + it("surfaces a fetch error message when the skills catalog cannot be loaded", async () => { + // Arrange: the catalog fetch fails (e.g. agent server unreachable) + vi.spyOn(SkillsService, "getSkills").mockRejectedValue( + new Error("network error"), + ); + + // Act + renderWithProviders(); + + // Assert: a clear failure message is shown instead of an endless spinner + expect(await screen.findByText("COMMON$FETCH_ERROR")).toBeInTheDocument(); + }); }); }); diff --git a/src/components/features/conversation-panel/skills-modal.tsx b/src/components/features/conversation-panel/skills-modal.tsx index a93242e594..d548ae6949 100644 --- a/src/components/features/conversation-panel/skills-modal.tsx +++ b/src/components/features/conversation-panel/skills-modal.tsx @@ -5,7 +5,6 @@ import { ModalBody } from "#/components/shared/modals/modal-body"; import { I18nKey } from "#/i18n/declaration"; import { getAgentServerWorkingDir } from "#/api/agent-server-config"; import { useConversationSkills } from "#/hooks/query/use-conversation-skills"; -import { AgentState } from "#/types/agent-state"; import { groupSkillsByScope, SKILL_SCOPE_ORDER, @@ -13,11 +12,9 @@ import { } from "#/utils/skill-scope"; import { SkillsModalHeader } from "./skills-modal-header"; import { SkillsModalSection } from "./skills-modal-section"; -import { SkillsRuntimeWaitingState } from "./skills-runtime-waiting-state"; import { SkillsLoadingState } from "./skills-loading-state"; import { SkillsEmptyState } from "./skills-empty-state"; import { SkillItem } from "./skill-item"; -import { useAgentState } from "#/hooks/use-agent-state"; interface SkillsModalProps { onClose: () => void; @@ -31,7 +28,6 @@ const SECTION_TITLE_KEY: Record = { export function SkillsModal({ onClose }: SkillsModalProps) { const { t } = useTranslation("openhands"); - const { curAgentState } = useAgentState(); const projectDir = getAgentServerWorkingDir(); const [expandedAgents, setExpandedAgents] = useState>( {}, @@ -58,10 +54,6 @@ export function SkillsModal({ onClose }: SkillsModalProps) { })); }; - const isAgentReady = ![AgentState.LOADING, AgentState.INIT].includes( - curAgentState, - ); - return (
- {!isAgentReady ? ( - - ) : isLoading ? ( + {isLoading ? ( ) : isError || !skills || skills.length === 0 ? ( diff --git a/src/components/features/conversation-panel/skills-runtime-waiting-state.tsx b/src/components/features/conversation-panel/skills-runtime-waiting-state.tsx deleted file mode 100644 index 0e9bb6564c..0000000000 --- a/src/components/features/conversation-panel/skills-runtime-waiting-state.tsx +++ /dev/null @@ -1,20 +0,0 @@ -import { useTranslation } from "react-i18next"; -import { LoadingSpinner } from "#/components/shared/loading-spinner"; -import { I18nKey } from "#/i18n/declaration"; -import { Typography } from "#/ui/typography"; - -export function SkillsRuntimeWaitingState() { - const { t } = useTranslation("openhands"); - - return ( -
- - - {t(I18nKey.DIFF_VIEWER$WAITING_FOR_RUNTIME)} - -
- ); -}