diff --git a/ui/goose2/src/features/skills/ui/SkillEditor.tsx b/ui/goose2/src/features/skills/ui/SkillEditor.tsx index a7a95a93..de289d20 100644 --- a/ui/goose2/src/features/skills/ui/SkillEditor.tsx +++ b/ui/goose2/src/features/skills/ui/SkillEditor.tsx @@ -11,21 +11,26 @@ import { DialogHeader, DialogTitle, } from "@/shared/ui/dialog"; -import { createSkill, updateSkill, type EditingSkill } from "../api/skills"; +import { + createSkill, + updateSkill, + type EditingSkill, + type SkillInfo, +} from "../api/skills"; import { formatSkillName, isValidSkillName } from "../lib/skillsHelpers"; import { getRenamedSkillFileLocation } from "../lib/skillsPath"; interface SkillEditorProps { isOpen: boolean; onClose: () => void; - onCreated?: () => void; + onSaved?: (savedSkill?: SkillInfo) => void | Promise; editingSkill?: EditingSkill; } export function SkillEditor({ isOpen, onClose, - onCreated, + onSaved, editingSkill, }: SkillEditorProps) { const { t } = useTranslation(["skills", "common"]); @@ -74,8 +79,9 @@ export function SkillEditor({ setSaving(true); setError(null); try { + let savedSkill: SkillInfo | undefined; if (isEditing) { - await updateSkill( + savedSkill = await updateSkill( editingSkill.path, name, description.trim(), @@ -87,7 +93,7 @@ export function SkillEditor({ setName(""); setDescription(""); setInstructions(""); - onCreated?.(); + await onSaved?.(savedSkill); onClose(); } catch (err) { setError(String(err)); diff --git a/ui/goose2/src/features/skills/ui/SkillsDialogs.tsx b/ui/goose2/src/features/skills/ui/SkillsDialogs.tsx index 2b730057..9401a969 100644 --- a/ui/goose2/src/features/skills/ui/SkillsDialogs.tsx +++ b/ui/goose2/src/features/skills/ui/SkillsDialogs.tsx @@ -16,7 +16,7 @@ import type { EditingSkill, SkillInfo } from "../api/skills"; interface SkillsDialogsProps { dialogOpen: boolean; onDialogClose: () => void; - onCreated: () => void | Promise; + onSaved: (savedSkill?: SkillInfo) => void | Promise; editingSkill?: EditingSkill; deletingSkill: SkillInfo | null; onDeletingSkillChange: (skill: SkillInfo | null) => void; @@ -26,7 +26,7 @@ interface SkillsDialogsProps { export function SkillsDialogs({ dialogOpen, onDialogClose, - onCreated, + onSaved, editingSkill, deletingSkill, onDeletingSkillChange, @@ -39,7 +39,7 @@ export function SkillsDialogs({ diff --git a/ui/goose2/src/features/skills/ui/SkillsView.tsx b/ui/goose2/src/features/skills/ui/SkillsView.tsx index a54aa013..f6bc2c7a 100644 --- a/ui/goose2/src/features/skills/ui/SkillsView.tsx +++ b/ui/goose2/src/features/skills/ui/SkillsView.tsx @@ -55,7 +55,7 @@ export function SkillsView({ onStartChatWithSkill }: SkillsViewProps) { const [expandedSectionIds, setExpandedSectionIds] = useState([]); const loadRequestIdRef = useRef(0); - const loadSkills = useCallback(async () => { + const loadSkills = useCallback(async (): Promise => { const requestId = loadRequestIdRef.current + 1; loadRequestIdRef.current = requestId; setLoading(true); @@ -64,16 +64,19 @@ export function SkillsView({ onStartChatWithSkill }: SkillsViewProps) { const projectDirs = projects.flatMap((project) => project.workingDirs); const result = await listSkills(projectDirs); if (loadRequestIdRef.current !== requestId) { - return; + return []; } - setSkills( - withInferredSkillCategories(hydrateProjectNames(result, projects)), + const nextSkills = withInferredSkillCategories( + hydrateProjectNames(result, projects), ); + setSkills(nextSkills); + return nextSkills; } catch { if (loadRequestIdRef.current === requestId) { setSkills([]); toast.error(t("view.loadError")); } + return []; } finally { if (loadRequestIdRef.current === requestId) { setLoading(false); @@ -190,6 +193,23 @@ export function SkillsView({ onStartChatWithSkill }: SkillsViewProps) { setDialogOpen(true); }; + const handleSkillSaved = useCallback( + async (savedSkill?: SkillInfo) => { + const refreshedSkills = await loadSkills(); + if ( + savedSkill && + refreshedSkills.some((skill) => skill.id === savedSkill.id) + ) { + setActiveSkillId(savedSkill.id); + } + }, + [loadSkills], + ); + + const refreshSkills = useCallback(async () => { + await loadSkills(); + }, [loadSkills]); + const { fileInputRef, isDragOver, @@ -197,7 +217,7 @@ export function SkillsView({ onStartChatWithSkill }: SkillsViewProps) { handleFileChange, openFilePicker, handleExport, - } = useSkillImportExport(loadSkills); + } = useSkillImportExport(refreshSkills); const handleSelectSkill = (skill: SkillViewInfo) => { setActiveSkillId(skill.id); @@ -207,7 +227,7 @@ export function SkillsView({ onStartChatWithSkill }: SkillsViewProps) { { @@ -308,10 +308,10 @@ describe("SkillEditor", () => { ); }); - it("calls onCreated callback after successful save", async () => { + it("calls onSaved callback after successful save", async () => { const user = userEvent.setup(); - const onCreated = vi.fn(); - render(); + const onSaved = vi.fn(); + render(); await user.type(screen.getByPlaceholderText("my-skill-name"), "my-skill"); await user.type( @@ -320,7 +320,7 @@ describe("SkillEditor", () => { ); await user.click(screen.getByRole("button", { name: /create skill/i })); - expect(onCreated).toHaveBeenCalled(); + expect(onSaved).toHaveBeenCalled(); }); it("clears fields after save", async () => { diff --git a/ui/goose2/src/features/skills/ui/__tests__/SkillsView.test.tsx b/ui/goose2/src/features/skills/ui/__tests__/SkillsView.test.tsx index aef50994..92198a82 100644 --- a/ui/goose2/src/features/skills/ui/__tests__/SkillsView.test.tsx +++ b/ui/goose2/src/features/skills/ui/__tests__/SkillsView.test.tsx @@ -62,6 +62,18 @@ const mockSkills: SkillInfo[] = [ vi.mock("../../api/skills", () => ({ listSkills: vi.fn().mockResolvedValue([]), + createSkill: vi.fn().mockResolvedValue(undefined), + updateSkill: vi.fn().mockResolvedValue({ + id: "global:/path/renamed-review", + name: "renamed-review", + description: "Reviews code", + instructions: "Review the code...", + path: "/path/renamed-review", + fileLocation: "/path/renamed-review/SKILL.md", + sourceKind: "global", + sourceLabel: "Personal", + projectLinks: [], + }), deleteSkill: vi.fn().mockResolvedValue(undefined), exportSkill: vi .fn() @@ -75,11 +87,12 @@ vi.mock("@/features/projects/stores/projectStore", () => ({ ) => selector({ projects: mockProjects }), })); -const { listSkills, deleteSkill } = (await import( +const { listSkills, deleteSkill, updateSkill } = (await import( "../../api/skills" )) as unknown as { listSkills: ReturnType; deleteSkill: ReturnType; + updateSkill: ReturnType; }; beforeEach(() => { @@ -288,6 +301,54 @@ describe("SkillsView", () => { expect(screen.queryByText("code-review")).not.toBeInTheDocument(); }); + it("stays on the detail page after renaming a skill", async () => { + const renamedSkill: SkillInfo = { + ...mockSkills[1], + id: "global:/path/renamed-review", + name: "renamed-review", + path: "/path/renamed-review", + fileLocation: "/path/renamed-review/SKILL.md", + }; + listSkills + .mockResolvedValueOnce(mockSkills) + .mockResolvedValueOnce([mockSkills[0], renamedSkill, mockSkills[2]]); + updateSkill.mockResolvedValueOnce(renamedSkill); + const user = userEvent.setup(); + + render(); + await screen.findByText("code-review"); + + await user.click( + screen.getByRole("button", { name: "Open code-review details" }), + ); + await user.click(screen.getByRole("button", { name: "Edit" })); + + const nameInput = screen.getByPlaceholderText("my-skill-name"); + await user.clear(nameInput); + await user.type(nameInput, "renamed-review"); + await user.click(screen.getByRole("button", { name: /save changes/i })); + + await waitFor(() => { + expect(updateSkill).toHaveBeenCalledWith( + "/path/code-review", + "renamed-review", + "Reviews code", + "Review the code...", + ); + }); + await waitFor(() => { + expect( + screen.getByRole("button", { name: "Back to skills" }), + ).toBeInTheDocument(); + expect( + screen.getByRole("heading", { name: "renamed-review" }), + ).toBeInTheDocument(); + }); + expect( + screen.queryByPlaceholderText("my-skill-name"), + ).not.toBeInTheDocument(); + }); + it("filters skills by search text", async () => { listSkills.mockResolvedValue(mockSkills); const user = userEvent.setup();