fix: stay in editing pane for skill when editing name

Author: kalvinnchau

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

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<void>;
+  onSaved: (savedSkill?: SkillInfo) => void | Promise<void>;
   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({
       <SkillEditor
         isOpen={dialogOpen}
         onClose={onDialogClose}
-        onCreated={onCreated}
+        onSaved={onSaved}
         editingSkill={editingSkill}
       />
 

ui/goose2/src/features/skills/ui/SkillsView.tsx

@@ -55,7 +55,7 @@ export function SkillsView({ onStartChatWithSkill }: SkillsViewProps) {
   const [expandedSectionIds, setExpandedSectionIds] = useState<string[]>([]);
   const loadRequestIdRef = useRef(0);
 
-  const loadSkills = useCallback(async () => {
+  const loadSkills = useCallback(async (): Promise<SkillViewInfo[]> => {
     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,14 +193,31 @@ 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,
     dropHandlers,
     handleFileChange,
     openFilePicker,
     handleExport,
-  } = useSkillImportExport(loadSkills);
+  } = useSkillImportExport(refreshSkills);
 
   const handleSelectSkill = (skill: SkillViewInfo) => {
     setActiveSkillId(skill.id);
@@ -207,7 +227,7 @@ export function SkillsView({ onStartChatWithSkill }: SkillsViewProps) {
     <SkillsDialogs
       dialogOpen={dialogOpen}
       onDialogClose={handleDialogClose}
-      onCreated={loadSkills}
+      onSaved={handleSkillSaved}
       editingSkill={editingSkill}
       deletingSkill={deletingSkill}
       onDeletingSkillChange={setDeletingSkill}

ui/goose2/src/features/skills/ui/__tests__/SkillEditor.test.tsx

@@ -20,7 +20,7 @@ const { createSkill, updateSkill } = await import("../../api/skills");
 const defaultProps = {
   isOpen: true,
   onClose: vi.fn(),
-  onCreated: vi.fn(),
+  onSaved: vi.fn(),
 };
 
 describe("SkillEditor", () => {
@@ -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(<SkillEditor {...defaultProps} onCreated={onCreated} />);
+      const onSaved = vi.fn();
+      render(<SkillEditor {...defaultProps} onSaved={onSaved} />);
 
       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 () => {

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<typeof vi.fn>;
   deleteSkill: ReturnType<typeof vi.fn>;
+  updateSkill: ReturnType<typeof vi.fn>;
 };
 
 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(<SkillsView />);
+    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();