feat(skills): uninstall for user-scope SKILL.md skills (#781)

app/src/components/skills/UninstallSkillConfirmDialog.tsx

@@ -0,0 +1,144 @@
+/**
+ * UninstallSkillConfirmDialog
+ * ---------------------------
+ *
+ * Small centered confirm modal for destructive uninstall of a user-scope
+ * SKILL.md skill. Wraps `skillsApi.uninstallSkill` which calls
+ * `openhuman.skills_uninstall` on the Rust side — that RPC only accepts
+ * user-scope installs (`~/.openhuman/skills/<name>/`) and refuses project
+ * and legacy scopes. The card that opens this dialog is responsible for
+ * not surfacing the Uninstall action for non-user-scope entries.
+ *
+ * UI contract:
+ *   - Shows skill name, resolved on-disk path (when known), and a plain
+ *     warning line.
+ *   - "Cancel" dismisses. "Uninstall" fires the RPC.
+ *   - While the RPC is in flight, both buttons disable and the modal is
+ *     non-dismissable (Esc / backdrop ignored) so the caller sees the
+ *     outcome.
+ *   - On success, the parent's `onUninstalled(result)` callback runs and
+ *     the dialog closes. On failure, the raw backend error is surfaced
+ *     inline; the dialog stays open so the user can retry or cancel.
+ *
+ * Design mirrors `InstallSkillDialog` — see
+ * `.claude/rules/15-settings-modal-system.md`.
+ */
+import { useCallback, useEffect, useRef, useState } from 'react';
+import { createPortal } from 'react-dom';
+import debug from 'debug';
+
+import {
+  skillsApi,
+  type SkillSummary,
+  type UninstallSkillResult,
+} from '../../services/api/skillsApi';
+
+const log = debug('skills:uninstall-dialog');
+
+interface Props {
+  skill: SkillSummary;
+  onClose: () => void;
+  /**
+   * Fires when the backend reports the uninstall succeeded. Parent is
+   * responsible for refetching the skills list and closing any detail
+   * panels that were showing this skill.
+   */
+  onUninstalled: (result: UninstallSkillResult) => void;
+}
+
+export default function UninstallSkillConfirmDialog({ skill, onClose, onUninstalled }: Props) {
+  const [submitting, setSubmitting] = useState(false);
+  const [error, setError] = useState<string | null>(null);
+  const cancelBtnRef = useRef<HTMLButtonElement | null>(null);
+  const previousFocusRef = useRef<HTMLElement | null>(null);
+
+  useEffect(() => {
+    previousFocusRef.current = document.activeElement as HTMLElement | null;
+    cancelBtnRef.current?.focus();
+    return () => {
+      previousFocusRef.current?.focus();
+    };
+  }, []);
+
+  useEffect(() => {
+    const handleKey = (e: KeyboardEvent) => {
+      if (e.key === 'Escape' && !submitting) {
+        e.preventDefault();
+        onClose();
+      }
+    };
+    document.addEventListener('keydown', handleKey);
+    return () => document.removeEventListener('keydown', handleKey);
+  }, [onClose, submitting]);
+
+  const handleConfirm = useCallback(async () => {
+    log('confirm: id=%s name=%s', skill.id, skill.name);
+    setSubmitting(true);
+    setError(null);
+    try {
+      // `skill.id` is the on-disk slug (directory under ~/.openhuman/skills/).
+      // `skill.name` is the frontmatter display name and may diverge from the
+      // slug — the backend resolves by slug, so pass `id`.
+      const result = await skillsApi.uninstallSkill(skill.id);
+      log('confirm: done removedPath=%s', result.removedPath);
+      onUninstalled(result);
+      onClose();
+    } catch (e) {
+      const msg = e instanceof Error ? e.message : String(e);
+      log('confirm: error=%s', msg);
+      setError(`Couldn't uninstall skill: ${msg}`);
+      setSubmitting(false);
+    }
+  }, [skill.id, skill.name, onUninstalled, onClose]);
+
+  return createPortal(
+    <div
+      role="dialog"
+      aria-modal="true"
+      aria-labelledby="uninstall-skill-title"
+      className="fixed inset-0 z-50 flex items-center justify-center bg-black/50 backdrop-blur-sm"
+      onMouseDown={e => {
+        if (e.target === e.currentTarget && !submitting) onClose();
+      }}>
+      <div className="w-[420px] max-w-[90vw] rounded-2xl bg-white p-5 shadow-2xl">
+        <h2 id="uninstall-skill-title" className="text-base font-semibold text-stone-900">
+          Uninstall {skill.name}?
+        </h2>
+        <p className="mt-2 text-sm text-stone-600">
+          This permanently deletes the skill directory and all its bundled resources. The agent
+          will stop seeing it at the next turn.
+        </p>
+        {skill.location && (
+          <p className="mt-3 break-all rounded-lg bg-stone-50 px-3 py-2 font-mono text-[11px] text-stone-600">
+            {skill.location.replace(/\/SKILL\.md$/i, '')}
+          </p>
+        )}
+        {error && (
+          <div className="mt-3 rounded-lg border border-coral-200 bg-coral-50 px-3 py-2 text-xs text-coral-700">
+            <div className="font-medium">Could not uninstall</div>
+            <div className="mt-1 break-words font-mono text-[11px] text-coral-700/90">{error}</div>
+          </div>
+        )}
+        <div className="mt-5 flex items-center justify-end gap-2">
+          <button
+            ref={cancelBtnRef}
+            type="button"
+            disabled={submitting}
+            onClick={onClose}
+            className="rounded-lg border border-stone-200 bg-white px-3 py-1.5 text-xs font-medium text-stone-700 hover:bg-stone-50 disabled:cursor-not-allowed disabled:opacity-50">
+            Cancel
+          </button>
+          <button
+            type="button"
+            disabled={submitting}
+            onClick={handleConfirm}
+            data-testid="uninstall-skill-confirm"
+            className="rounded-lg border border-coral-300 bg-coral-50 px-3 py-1.5 text-xs font-medium text-coral-700 hover:bg-coral-100 disabled:cursor-not-allowed disabled:opacity-50">
+            {submitting ? 'Uninstalling…' : 'Uninstall'}
+          </button>
+        </div>
+      </div>
+    </div>,
+    document.body
+  );
+}

app/src/components/skills/__tests__/UninstallSkillConfirmDialog.test.tsx

@@ -0,0 +1,217 @@
+/**
+ * UninstallSkillConfirmDialog — vitest coverage
+ *
+ * Verifies:
+ * - Renders skill name + on-disk path + destructive confirm copy.
+ * - Cancel button fires onClose, does NOT hit the RPC.
+ * - Confirm fires `skillsApi.uninstallSkill(name)` and forwards the result
+ *   to `onUninstalled`, then closes.
+ * - RPC error is surfaced inline and the dialog stays open (no onClose).
+ * - While in-flight, both buttons disable and Esc no-ops (handled by
+ *   disabled flag on the cancel button; dialog-level dismissal blocked).
+ */
+import { fireEvent, render, screen, waitFor } from '@testing-library/react';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+import UninstallSkillConfirmDialog from '../UninstallSkillConfirmDialog';
+import type { SkillSummary } from '../../../services/api/skillsApi';
+
+vi.mock('../../../services/api/skillsApi', () => ({
+  skillsApi: {
+    uninstallSkill: vi.fn(),
+  },
+}));
+
+const fixture: SkillSummary = {
+  id: 'weather-helper',
+  name: 'weather-helper',
+  description: 'Weather forecasts',
+  version: '',
+  author: null,
+  tags: [],
+  tools: [],
+  prompts: [],
+  location: '/Users/me/.openhuman/skills/weather-helper/SKILL.md',
+  resources: [],
+  scope: 'user',
+  legacy: false,
+  warnings: [],
+};
+
+describe('UninstallSkillConfirmDialog', () => {
+  beforeEach(async () => {
+    const { skillsApi } = await import('../../../services/api/skillsApi');
+    vi.mocked(skillsApi.uninstallSkill).mockReset();
+  });
+
+  it('renders skill name, path (stripped of /SKILL.md), and confirm copy', () => {
+    render(
+      <UninstallSkillConfirmDialog
+        skill={fixture}
+        onClose={vi.fn()}
+        onUninstalled={vi.fn()}
+      />
+    );
+    expect(screen.getByText(/Uninstall weather-helper\?/)).toBeInTheDocument();
+    expect(screen.getByText(/permanently deletes/i)).toBeInTheDocument();
+    expect(screen.getByText('/Users/me/.openhuman/skills/weather-helper')).toBeInTheDocument();
+    expect(screen.getByRole('button', { name: /Cancel/ })).toBeInTheDocument();
+    expect(screen.getByRole('button', { name: /^Uninstall$/ })).toBeInTheDocument();
+  });
+
+  it('Confirm uses skill.id (slug), not skill.name (display), when they diverge', async () => {
+    // Regression test for #781: `Skill.name` comes from SKILL.md frontmatter
+    // and can differ from the on-disk directory. The uninstall RPC resolves
+    // by slug — the UI must pass `skill.id` (the slug).
+    const onClose = vi.fn();
+    const onUninstalled = vi.fn();
+    const { skillsApi } = await import('../../../services/api/skillsApi');
+    vi.mocked(skillsApi.uninstallSkill).mockResolvedValueOnce({
+      name: 'weather-helper',
+      removedPath: '/Users/me/.openhuman/skills/weather-helper',
+      scope: 'user',
+    });
+
+    const divergent: SkillSummary = {
+      ...fixture,
+      id: 'weather-helper',
+      name: 'Weather Helper (Pro)',
+    };
+    render(
+      <UninstallSkillConfirmDialog
+        skill={divergent}
+        onClose={onClose}
+        onUninstalled={onUninstalled}
+      />
+    );
+    fireEvent.click(screen.getByTestId('uninstall-skill-confirm'));
+
+    await waitFor(() => {
+      expect(vi.mocked(skillsApi.uninstallSkill)).toHaveBeenCalledWith('weather-helper');
+    });
+    expect(vi.mocked(skillsApi.uninstallSkill)).not.toHaveBeenCalledWith('Weather Helper (Pro)');
+  });
+
+  it('Cancel fires onClose without calling the RPC', async () => {
+    const onClose = vi.fn();
+    const { skillsApi } = await import('../../../services/api/skillsApi');
+    render(
+      <UninstallSkillConfirmDialog
+        skill={fixture}
+        onClose={onClose}
+        onUninstalled={vi.fn()}
+      />
+    );
+    fireEvent.click(screen.getByRole('button', { name: /Cancel/ }));
+    expect(onClose).toHaveBeenCalledTimes(1);
+    expect(vi.mocked(skillsApi.uninstallSkill)).not.toHaveBeenCalled();
+  });
+
+  it('Confirm calls skillsApi.uninstallSkill and forwards result to onUninstalled', async () => {
+    const onClose = vi.fn();
+    const onUninstalled = vi.fn();
+    const { skillsApi } = await import('../../../services/api/skillsApi');
+    vi.mocked(skillsApi.uninstallSkill).mockResolvedValueOnce({
+      name: 'weather-helper',
+      removedPath: '/Users/me/.openhuman/skills/weather-helper',
+      scope: 'user',
+    });
+
+    render(
+      <UninstallSkillConfirmDialog
+        skill={fixture}
+        onClose={onClose}
+        onUninstalled={onUninstalled}
+      />
+    );
+    fireEvent.click(screen.getByTestId('uninstall-skill-confirm'));
+
+    await waitFor(() => {
+      expect(vi.mocked(skillsApi.uninstallSkill)).toHaveBeenCalledWith('weather-helper');
+    });
+    // Assert the caller passed the slug (`id`) — not the frontmatter
+    // display name. Regression guard for the #781 fix that swapped
+    // `skill.name` → `skill.id` in the confirm handler.
+    expect(vi.mocked(skillsApi.uninstallSkill)).toHaveBeenCalledWith(fixture.id);
+    await waitFor(() => {
+      expect(onUninstalled).toHaveBeenCalledWith({
+        name: 'weather-helper',
+        removedPath: '/Users/me/.openhuman/skills/weather-helper',
+        scope: 'user',
+      });
+    });
+    await waitFor(() => {
+      expect(onClose).toHaveBeenCalledTimes(1);
+    });
+  });
+
+  it('surfaces RPC errors inline and keeps the dialog open', async () => {
+    const onClose = vi.fn();
+    const onUninstalled = vi.fn();
+    const { skillsApi } = await import('../../../services/api/skillsApi');
+    vi.mocked(skillsApi.uninstallSkill).mockRejectedValueOnce(
+      new Error("skill 'weather-helper' is not installed")
+    );
+
+    render(
+      <UninstallSkillConfirmDialog
+        skill={fixture}
+        onClose={onClose}
+        onUninstalled={onUninstalled}
+      />
+    );
+    fireEvent.click(screen.getByTestId('uninstall-skill-confirm'));
+
+    await waitFor(() => {
+      expect(screen.getByText(/Could not uninstall/)).toBeInTheDocument();
+    });
+    expect(screen.getByText(/is not installed/)).toBeInTheDocument();
+    expect(onClose).not.toHaveBeenCalled();
+    expect(onUninstalled).not.toHaveBeenCalled();
+    // Confirm button should be re-enabled so the user can retry.
+    const confirm = screen.getByTestId('uninstall-skill-confirm') as HTMLButtonElement;
+    expect(confirm.disabled).toBe(false);
+  });
+
+  it('disables buttons while the RPC is in flight', async () => {
+    const { skillsApi } = await import('../../../services/api/skillsApi');
+    type UninstallResolve = (v: {
+      name: string;
+      removedPath: string;
+      scope: SkillSummary['scope'];
+    }) => void;
+    const deferred: { resolve?: UninstallResolve } = {};
+    vi.mocked(skillsApi.uninstallSkill).mockReturnValueOnce(
+      new Promise<{
+        name: string;
+        removedPath: string;
+        scope: SkillSummary['scope'];
+      }>(resolve => {
+        deferred.resolve = resolve;
+      })
+    );
+
+    render(
+      <UninstallSkillConfirmDialog
+        skill={fixture}
+        onClose={vi.fn()}
+        onUninstalled={vi.fn()}
+      />
+    );
+    fireEvent.click(screen.getByTestId('uninstall-skill-confirm'));
+
+    await waitFor(() => {
+      const cancel = screen.getByRole('button', { name: /Cancel/ }) as HTMLButtonElement;
+      const confirm = screen.getByTestId('uninstall-skill-confirm') as HTMLButtonElement;
+      expect(cancel.disabled).toBe(true);
+      expect(confirm.disabled).toBe(true);
+      expect(confirm.textContent).toMatch(/Uninstalling/);
+    });
+
+    deferred.resolve?.({
+      name: 'weather-helper',
+      removedPath: '/Users/me/.openhuman/skills/weather-helper',
+      scope: 'user',
+    });
+  });
+});