fix: do not gate the skills modal on runtime readiness (#1122)

This commit is contained in:
Hiep Le
2026-06-04 14:40:19 +00:00
committed by GitHub
parent 0c892a51b5
commit 02eb964ff1
3 changed files with 20 additions and 62 deletions
@@ -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(<SkillsModal {...defaultProps} />);
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(<SkillsModal {...defaultProps} />);
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(<SkillsModal {...defaultProps} />);
// Assert: a clear failure message is shown instead of an endless spinner
expect(await screen.findByText("COMMON$FETCH_ERROR")).toBeInTheDocument();
});
});
});
@@ -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<SkillScope, I18nKey> = {
export function SkillsModal({ onClose }: SkillsModalProps) {
const { t } = useTranslation("openhands");
const { curAgentState } = useAgentState();
const projectDir = getAgentServerWorkingDir();
const [expandedAgents, setExpandedAgents] = useState<Record<string, boolean>>(
{},
@@ -58,10 +54,6 @@ export function SkillsModal({ onClose }: SkillsModalProps) {
}));
};
const isAgentReady = ![AgentState.LOADING, AgentState.INIT].includes(
curAgentState,
);
return (
<ModalBackdrop onClose={onClose}>
<ModalBody
@@ -77,9 +69,7 @@ export function SkillsModal({ onClose }: SkillsModalProps) {
/>
<div className="w-full h-[60vh] overflow-auto rounded-md border border-[var(--oh-border)] bg-surface-raised custom-scrollbar-always">
{!isAgentReady ? (
<SkillsRuntimeWaitingState />
) : isLoading ? (
{isLoading ? (
<SkillsLoadingState />
) : isError || !skills || skills.length === 0 ? (
<SkillsEmptyState isError={isError} />
@@ -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 (
<div
data-testid="skills-runtime-waiting"
className="flex h-full w-full flex-col items-center justify-center gap-3 py-8 text-center"
>
<LoadingSpinner size="small" />
<Typography.Text className="text-sm text-[var(--oh-muted)]">
{t(I18nKey.DIFF_VIEWER$WAITING_FOR_RUNTIME)}
</Typography.Text>
</div>
);
}