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

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>

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',
+    });
+  });
+});

app/src/pages/Skills.tsx

@@ -9,6 +9,7 @@ import {
   KNOWN_COMPOSIO_TOOLKITS,
 } from '../components/composio/toolkitMeta';
 import ConnectionBadge, { isMessagingId } from '../components/ConnectionBadge';
+import { ToastContainer } from '../components/intelligence/Toast';
 import AutocompleteSetupModal from '../components/skills/AutocompleteSetupModal';
 import CreateSkillModal from '../components/skills/CreateSkillModal';
 import InstallSkillDialog from '../components/skills/InstallSkillDialog';
@@ -24,6 +25,7 @@ import {
   SkillCategoryIcon,
 } from '../components/skills/skillIcons';
 import SkillSearchBar from '../components/skills/SkillSearchBar';
+import UninstallSkillConfirmDialog from '../components/skills/UninstallSkillConfirmDialog';
 import VoiceSetupModal from '../components/skills/VoiceSetupModal';
 import { useAutocompleteSkillStatus } from '../features/autocomplete/useAutocompleteSkillStatus';
 import { useScreenIntelligenceSkillStatus } from '../features/screen-intelligence/useScreenIntelligenceSkillStatus';
@@ -35,6 +37,7 @@ import { type ComposioConnection, deriveComposioState } from '../lib/composio/ty
 import { skillsApi, type SkillSummary } from '../services/api/skillsApi';
 import { useAppSelector } from '../store/hooks';
 import type { ChannelConnectionStatus, ChannelDefinition, ChannelType } from '../types/channels';
+import type { ToastNotification } from '../types/intelligence';
 import { IS_DEV } from '../utils/config';
 import { subconsciousEscalationsDismiss } from '../utils/tauriCommands';
 
@@ -194,6 +197,15 @@ export default function Skills() {
   const [selectedSkill, setSelectedSkill] = useState<SkillSummary | null>(null);
   const [createModalOpen, setCreateModalOpen] = useState(false);
   const [installDialogOpen, setInstallDialogOpen] = useState(false);
+  const [uninstallCandidate, setUninstallCandidate] = useState<SkillSummary | null>(null);
+  const [toasts, setToasts] = useState<ToastNotification[]>([]);
+  const addToast = useCallback((toast: Omit<ToastNotification, 'id'>) => {
+    const newToast: ToastNotification = { ...toast, id: `toast-${Date.now()}-${Math.random()}` };
+    setToasts(prev => [...prev, newToast]);
+  }, []);
+  const removeToast = useCallback((id: string) => {
+    setToasts(prev => prev.filter(toast => toast.id !== id));
+  }, []);
   const pendingEscalationId =
     location.state &&
     typeof location.state === 'object' &&
@@ -631,6 +643,7 @@ export default function Skills() {
                             : skill.scope === 'project'
                               ? 'text-amber-600'
                               : 'text-stone-600';
+                        const canUninstall = skill.scope === 'user' && !skill.legacy;
                         return (
                           <UnifiedSkillCard
                             key={item.id}
@@ -647,6 +660,36 @@ export default function Skills() {
                               });
                               setSelectedSkill(skill);
                             }}
+                            secondaryActions={
+                              canUninstall
+                                ? [
+                                    {
+                                      label: 'Uninstall',
+                                      testId: `uninstall-skill-${skill.id}`,
+                                      icon: (
+                                        <svg
+                                          className="h-3.5 w-3.5"
+                                          fill="none"
+                                          stroke="currentColor"
+                                          strokeWidth="2"
+                                          viewBox="0 0 24 24">
+                                          <path
+                                            strokeLinecap="round"
+                                            strokeLinejoin="round"
+                                            d="M3 6h18M8 6V4a2 2 0 012-2h4a2 2 0 012 2v2m3 0v14a2 2 0 01-2 2H7a2 2 0 01-2-2V6h14z"
+                                          />
+                                        </svg>
+                                      ),
+                                      onClick: () => {
+                                        console.debug('[skills][discovered] open uninstall', {
+                                          skillId: skill.id,
+                                        });
+                                        setUninstallCandidate(skill);
+                                      },
+                                    },
+                                  ]
+                                : undefined
+                            }
                           />
                         );
                       }
@@ -784,6 +827,34 @@ export default function Skills() {
           }}
         />
       )}
+
+      {uninstallCandidate && (
+        <UninstallSkillConfirmDialog
+          skill={uninstallCandidate}
+          onClose={() => setUninstallCandidate(null)}
+          onUninstalled={result => {
+            console.debug('[skills][uninstall] complete', {
+              name: result.name,
+              removedPath: result.removedPath,
+            });
+            addToast({
+              type: 'success',
+              title: 'Skill uninstalled',
+              message: `"${result.name}" was removed successfully.`,
+            });
+            // If the detail drawer was showing the skill we just removed,
+            // close it — the resource tree is now stale and any `read_resource`
+            // RPC would fail with a clean "not installed" error.
+            setSelectedSkill(prev => (prev && prev.id === result.name ? null : prev));
+            // Drop it from local state so the card disappears without a
+            // round-trip; refresh to pick up any side effects (e.g. a
+            // previously-shadowed project-scope skill now surfaces).
+            setDiscoveredSkills(prev => prev.filter(s => s.id !== result.name));
+            void refreshDiscoveredSkills();
+          }}
+        />
+      )}
+      <ToastContainer notifications={toasts} onRemove={removeToast} />
     </div>
   );
 }

app/src/services/api/skillsApi.ts

@@ -129,6 +129,25 @@ interface RawInstallSkillFromUrlResult {
   new_skills: string[];
 }
 
+/**
+ * Result of `openhuman.skills_uninstall`.
+ *
+ * Mirrors the Rust-side `UninstallSkillOutcome`. `removedPath` is the
+ * canonicalised on-disk path that was deleted — surface it in success toasts
+ * so the user can confirm exactly what was removed.
+ */
+export interface UninstallSkillResult {
+  name: string;
+  removedPath: string;
+  scope: SkillScope;
+}
+
+interface RawUninstallSkillResult {
+  name: string;
+  removed_path: string;
+  scope: SkillScope;
+}
+
 interface Envelope<T> {
   data?: T;
 }
@@ -250,4 +269,28 @@ export const skillsApi = {
     );
     return normalized;
   },
+
+  /**
+   * Remove an installed user-scope SKILL.md skill via `openhuman.skills_uninstall`.
+   *
+   * Only user-scope installs (`~/.openhuman/skills/<name>/`) are supported.
+   * Project-scope and legacy skills are read-only — trying to uninstall one
+   * returns a backend error surfaced as a rejected promise. The Rust side
+   * canonicalises paths and refuses names with separators / traversal
+   * sequences / anything outside the skills root.
+   */
+  uninstallSkill: async (name: string): Promise<UninstallSkillResult> => {
+    log('uninstallSkill: request name=%s', name);
+    const response = await callCoreRpc<Envelope<RawUninstallSkillResult> | RawUninstallSkillResult>(
+      { method: 'openhuman.skills_uninstall', params: { name } }
+    );
+    const raw = unwrapEnvelope(response);
+    const normalized: UninstallSkillResult = {
+      name: raw.name,
+      removedPath: raw.removed_path,
+      scope: raw.scope,
+    };
+    log('uninstallSkill: response name=%s removedPath=%s', normalized.name, normalized.removedPath);
+    return normalized;
+  },
 };

src/openhuman/skills/inject.rs

@@ -508,6 +508,7 @@ mod tests {
     fn skill(name: &str, description: &str) -> Skill {
         Skill {
             name: name.to_string(),
+            dir_name: name.to_string(),
             description: description.to_string(),
             version: "0.1.0".into(),
             author: None,

src/openhuman/skills/ops.rs

@@ -151,6 +151,12 @@ fn extract_tags(fm: &SkillFrontmatter, warnings: &mut Vec<String>) -> Vec<String
 pub struct Skill {
     /// Display name (from frontmatter, falls back to directory name).
     pub name: String,
+    /// On-disk slug — the directory name under `~/.openhuman/skills/` (user
+    /// scope) or the workspace skills directory (project scope). This is the
+    /// identifier the uninstall RPC resolves against; it may differ from
+    /// [`Skill::name`] when frontmatter declares a mismatched display name.
+    #[serde(default)]
+    pub dir_name: String,
     /// Short description used in the catalog summary.
     pub description: String,
     /// Version string, if declared.
@@ -517,6 +523,7 @@ fn load_from_skill_md(skill_md: &Path, dir: &Path, dir_name: &str, scope: SkillS
 
     Skill {
         name,
+        dir_name: dir_name.to_string(),
         description,
         version,
         author,
@@ -579,6 +586,7 @@ fn load_from_legacy_manifest(
 
     Skill {
         name,
+        dir_name: dir_name.to_string(),
         description,
         version: manifest.version,
         author: manifest.author,
@@ -1741,6 +1749,203 @@ fn is_private_v6(ip: &Ipv6Addr) -> bool {
     false
 }
 
+/// Input for [`uninstall_skill`]. Mirrors the `skills.uninstall` JSON-RPC
+/// payload.
+#[derive(Debug, Clone, Deserialize)]
+pub struct UninstallSkillParams {
+    /// On-disk slug of the installed skill — the directory name under
+    /// `~/.openhuman/skills/<slug>/`. This is `SkillSummary.id` on the wire,
+    /// which may diverge from the frontmatter display name exposed as
+    /// `Skill.name` when a skill's `SKILL.md` declares a name that does not
+    /// match its directory. Callers must send the slug (`SkillSummary.id`),
+    /// not the display name, or the RPC will return "not installed".
+    ///
+    /// Retained as `name` for wire-format back-compat with pre-existing
+    /// clients; the field's semantics are slug-only.
+    pub name: String,
+}
+
+/// Outcome of a successful uninstall.
+#[derive(Debug, Clone, Serialize)]
+pub struct UninstallSkillOutcome {
+    /// The normalised slug that was removed.
+    pub name: String,
+    /// Absolute on-disk path that was deleted (post-canonicalisation). Useful
+    /// for user-visible confirmations and for logs.
+    pub removed_path: String,
+    /// Scope the uninstall applied to. Always `User` today.
+    pub scope: SkillScope,
+}
+
+/// Remove an installed user-scope SKILL.md skill from `~/.openhuman/skills/`.
+///
+/// Only **user-scope** uninstalls are supported:
+///
+/// * Project-scope skills live inside the workspace — the user manages them
+///   via git / editor / filesystem directly. Exposing a destructive delete
+///   over RPC invites foot-guns, since a rogue skill's tool could uninstall
+///   other project skills.
+/// * Legacy `<ws>/skills/` skills are treated as read-only. They predate the
+///   install flow and we don't want to blow away content the user may have
+///   hand-authored.
+///
+/// Resolution is defensive:
+///   1. `skills_root = <home>/.openhuman/skills`, canonicalised.
+///   2. `candidate = skills_root.join(&name)`, canonicalised.
+///   3. Assert `candidate.starts_with(skills_root)` after canonicalisation —
+///      blocks `..` traversal and symlink escapes.
+///   4. Refuse anything that is not a plain directory (no symlinks).
+///   5. Require `SKILL.md` to be present — we only remove things that look
+///      like skills we installed, not arbitrary dirs the user dropped in.
+///   6. `fs::remove_dir_all`.
+///
+/// `home_dir_override` is threaded through for tests. Production callers
+/// pass `None`, which falls back to [`dirs::home_dir`].
+pub fn uninstall_skill(
+    params: UninstallSkillParams,
+    home_dir_override: Option<&Path>,
+) -> Result<UninstallSkillOutcome, String> {
+    let trimmed = params.name.trim().to_string();
+    if trimmed.is_empty() {
+        return Err("skill name is required".to_string());
+    }
+    if trimmed.contains('/') || trimmed.contains('\\') || trimmed.contains("..") {
+        log::warn!(
+            "[skills] uninstall_skill: rejected name with path separators name={:?}",
+            trimmed
+        );
+        return Err(format!(
+            "skill name '{trimmed}' must not contain path separators"
+        ));
+    }
+    if trimmed.len() > MAX_NAME_LEN {
+        return Err(format!(
+            "skill name is {} chars (max {MAX_NAME_LEN})",
+            trimmed.len()
+        ));
+    }
+
+    let home = match home_dir_override
+        .map(|p| p.to_path_buf())
+        .or_else(dirs::home_dir)
+    {
+        Some(h) => h,
+        None => return Err("could not resolve user home directory".to_string()),
+    };
+
+    let skills_root = home.join(".openhuman").join("skills");
+    if !skills_root.exists() {
+        return Err(format!(
+            "no user skills directory at {}",
+            skills_root.display()
+        ));
+    }
+
+    // Refuse a symlinked skills root before canonicalisation would silently
+    // follow it. `canonicalize` dereferences every component, so by the time
+    // `starts_with` runs below a symlinked `~/.openhuman/skills` would have
+    // already resolved elsewhere and the guard is a no-op.
+    let root_meta = std::fs::symlink_metadata(&skills_root)
+        .map_err(|e| format!("stat {} failed: {e}", skills_root.display()))?;
+    if root_meta.file_type().is_symlink() {
+        log::warn!(
+            "[skills] uninstall_skill: refused symlinked skills root path={}",
+            skills_root.display()
+        );
+        return Err(format!(
+            "skills root {} is a symlink — refusing to resolve",
+            skills_root.display()
+        ));
+    }
+
+    let canonical_root = std::fs::canonicalize(&skills_root)
+        .map_err(|e| format!("canonicalize {} failed: {e}", skills_root.display()))?;
+
+    let candidate = skills_root.join(&trimmed);
+    // Reject `~/.openhuman/skills/<trimmed>` aliases that are themselves
+    // symlinks (e.g. `alias -> other-skill`). Same reasoning as the root
+    // check: canonicalise would follow the alias and `starts_with` would
+    // still pass, leading us to delete a different directory than the one
+    // the UI named.
+    match std::fs::symlink_metadata(&candidate) {
+        Ok(m) if m.file_type().is_symlink() => {
+            log::warn!(
+                "[skills] uninstall_skill: refused symlinked alias name={trimmed} path={}",
+                candidate.display()
+            );
+            return Err(format!(
+                "skill '{trimmed}' is a symlinked alias — refusing to resolve"
+            ));
+        }
+        Ok(_) => {}
+        Err(e) if e.kind() == std::io::ErrorKind::NotFound => {
+            return Err(format!("skill '{trimmed}' is not installed"));
+        }
+        Err(e) => {
+            return Err(format!("stat {} failed: {e}", candidate.display()));
+        }
+    }
+
+    // After the raw-path symlink guards above, canonicalise to resolve any
+    // `.` / `..` components introduced by a sneaky input that slipped past
+    // the earlier separator check, and map a still-possible NotFound race
+    // to the friendly message.
+    let canonical_candidate = std::fs::canonicalize(&candidate).map_err(|e| {
+        if e.kind() == std::io::ErrorKind::NotFound {
+            format!("skill '{trimmed}' is not installed")
+        } else {
+            format!("canonicalize {} failed: {e}", candidate.display())
+        }
+    })?;
+
+    if !canonical_candidate.starts_with(&canonical_root) {
+        log::warn!(
+            "[skills] uninstall_skill: path escape rejected candidate={} root={}",
+            canonical_candidate.display(),
+            canonical_root.display()
+        );
+        return Err(format!(
+            "refused to remove {} — path escapes skills root",
+            canonical_candidate.display()
+        ));
+    }
+
+    let meta = std::fs::symlink_metadata(&canonical_candidate)
+        .map_err(|e| format!("stat {} failed: {e}", canonical_candidate.display()))?;
+    if meta.file_type().is_symlink() || !meta.is_dir() {
+        return Err(format!(
+            "{} is not a directory — refusing to remove",
+            canonical_candidate.display()
+        ));
+    }
+
+    // Best-effort guard: reject directories that don't look like SKILL.md
+    // skills before we commit to removing them.  The subsequent
+    // `remove_dir_all` is the authoritative operation; a TOCTOU race here
+    // (file disappears between the check and the remove) is benign — the
+    // remove would fail with a clear error anyway.
+    let skill_md = canonical_candidate.join(SKILL_MD);
+    if !skill_md.exists() {
+        return Err(format!(
+            "{} does not look like a SKILL.md skill (missing {SKILL_MD})",
+            canonical_candidate.display()
+        ));
+    }
+
+    log::info!(
+        "[skills] uninstall_skill: removing name={trimmed} path={}",
+        canonical_candidate.display()
+    );
+    std::fs::remove_dir_all(&canonical_candidate)
+        .map_err(|e| format!("remove {} failed: {e}", canonical_candidate.display()))?;
+
+    Ok(UninstallSkillOutcome {
+        name: trimmed,
+        removed_path: canonical_candidate.display().to_string(),
+        scope: SkillScope::User,
+    })
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -2710,4 +2915,206 @@ mod tests {
             "expected warning, got {warnings:?}"
         );
     }
+
+    /// Happy path: install a SKILL.md under a synthetic user home, verify
+    /// discovery sees it, uninstall, verify discovery no longer sees it and
+    /// the on-disk dir is gone.
+    #[test]
+    fn uninstall_skill_removes_user_scope_dir() {
+        let home = tempfile::tempdir().unwrap();
+        let skill_dir = home
+            .path()
+            .join(".openhuman")
+            .join("skills")
+            .join("weather-helper");
+        write(
+            &skill_dir.join("SKILL.md"),
+            "---\nname: weather-helper\ndescription: forecasts\n---\n\nbody\n",
+        );
+        let before = discover_skills(Some(home.path()), None, false);
+        assert_eq!(before.len(), 1, "setup: skill should be discoverable");
+
+        let outcome = uninstall_skill(
+            UninstallSkillParams {
+                name: "weather-helper".into(),
+            },
+            Some(home.path()),
+        )
+        .unwrap();
+        assert_eq!(outcome.name, "weather-helper");
+        assert_eq!(outcome.scope, SkillScope::User);
+        assert!(!skill_dir.exists(), "uninstall should remove the dir");
+
+        let after = discover_skills(Some(home.path()), None, false);
+        assert!(after.is_empty(), "discovery should no longer see it");
+    }
+
+    /// Names containing path separators or traversal sequences are rejected
+    /// before any filesystem access.
+    #[test]
+    fn uninstall_skill_rejects_path_traversal_names() {
+        let home = tempfile::tempdir().unwrap();
+        std::fs::create_dir_all(home.path().join(".openhuman").join("skills")).unwrap();
+        for bad in ["../etc", "foo/bar", "foo\\bar", "..", "foo/../bar"] {
+            let err = uninstall_skill(UninstallSkillParams { name: bad.into() }, Some(home.path()))
+                .unwrap_err();
+            assert!(
+                err.contains("path separators") || err.contains("is not installed"),
+                "name {bad:?} should be rejected before fs access, got: {err}"
+            );
+        }
+    }
+
+    /// Empty and whitespace-only names return a clear required-field error.
+    #[test]
+    fn uninstall_skill_rejects_empty_name() {
+        let home = tempfile::tempdir().unwrap();
+        std::fs::create_dir_all(home.path().join(".openhuman").join("skills")).unwrap();
+        for bad in ["", "   ", "\t"] {
+            let err = uninstall_skill(UninstallSkillParams { name: bad.into() }, Some(home.path()))
+                .unwrap_err();
+            assert!(err.contains("name is required"), "{bad:?} => {err}");
+        }
+    }
+
+    /// Uninstalling a skill that is not installed surfaces a recognizable
+    /// error rather than a generic I/O failure.
+    #[test]
+    fn uninstall_skill_missing_skill_errors_cleanly() {
+        let home = tempfile::tempdir().unwrap();
+        std::fs::create_dir_all(home.path().join(".openhuman").join("skills")).unwrap();
+        let err = uninstall_skill(
+            UninstallSkillParams {
+                name: "ghost".into(),
+            },
+            Some(home.path()),
+        )
+        .unwrap_err();
+        assert!(err.contains("not installed"), "got: {err}");
+    }
+
+    /// A directory that does not contain a `SKILL.md` is refused — we only
+    /// remove things that look like skills we installed, not arbitrary
+    /// directories the user dropped in.
+    #[test]
+    fn uninstall_skill_refuses_dir_without_skill_md() {
+        let home = tempfile::tempdir().unwrap();
+        let bogus = home.path().join(".openhuman").join("skills").join("bogus");
+        std::fs::create_dir_all(&bogus).unwrap();
+        std::fs::write(bogus.join("random.txt"), "not a skill").unwrap();
+        let err = uninstall_skill(
+            UninstallSkillParams {
+                name: "bogus".into(),
+            },
+            Some(home.path()),
+        )
+        .unwrap_err();
+        assert!(
+            err.contains("does not look like a SKILL.md skill"),
+            "got: {err}"
+        );
+        assert!(bogus.exists(), "non-skill dir should not be deleted");
+    }
+
+    /// A symlink inside the skills root pointing outside the root must be
+    /// rejected by the raw-path symlink preflight before `canonicalize`
+    /// would follow the link. The earlier `starts_with` / `is_dir` guards
+    /// remain as defence-in-depth for anything that slips past the
+    /// preflight on future refactors.
+    #[cfg(unix)]
+    #[test]
+    fn uninstall_skill_rejects_symlink_escape() {
+        let home = tempfile::tempdir().unwrap();
+        let skills_root = home.path().join(".openhuman").join("skills");
+        std::fs::create_dir_all(&skills_root).unwrap();
+        let outside = tempfile::tempdir().unwrap();
+        let target = outside.path().join("real");
+        write(
+            &target.join("SKILL.md"),
+            "---\nname: real\ndescription: out of tree\n---\n",
+        );
+        std::os::unix::fs::symlink(&target, skills_root.join("real")).unwrap();
+        let err = uninstall_skill(
+            UninstallSkillParams {
+                name: "real".into(),
+            },
+            Some(home.path()),
+        )
+        .unwrap_err();
+        assert!(
+            err.contains("symlinked alias")
+                || err.contains("path escapes skills root")
+                || err.contains("is not a directory"),
+            "symlink out of tree must be rejected, got: {err}"
+        );
+        assert!(target.exists(), "symlink target must not be deleted");
+    }
+
+    /// An in-tree symlink alias (`skills/alias -> skills/real`) must be
+    /// rejected even though it does not escape the skills root — otherwise
+    /// the uninstall of `alias` would nuke the real skill directory behind
+    /// it, violating the invariant that the named slug is deleted.
+    #[cfg(unix)]
+    #[test]
+    fn uninstall_skill_rejects_symlinked_alias_in_tree() {
+        let home = tempfile::tempdir().unwrap();
+        let skills_root = home.path().join(".openhuman").join("skills");
+        std::fs::create_dir_all(&skills_root).unwrap();
+        let real_dir = skills_root.join("real");
+        write(
+            &real_dir.join("SKILL.md"),
+            "---\nname: real\ndescription: in tree\n---\n",
+        );
+        std::os::unix::fs::symlink(&real_dir, skills_root.join("alias")).unwrap();
+        let err = uninstall_skill(
+            UninstallSkillParams {
+                name: "alias".into(),
+            },
+            Some(home.path()),
+        )
+        .unwrap_err();
+        assert!(
+            err.contains("symlinked alias"),
+            "in-tree alias must be rejected by preflight, got: {err}"
+        );
+        assert!(
+            real_dir.join("SKILL.md").exists(),
+            "real skill behind the alias must survive"
+        );
+    }
+
+    /// A symlinked skills *root* (`~/.openhuman/skills -> elsewhere`) must
+    /// be refused before canonicalisation, since `canonicalize` would
+    /// resolve it to the target and the `starts_with` guard would then
+    /// compare against the resolved target, not the nominal root.
+    #[cfg(unix)]
+    #[test]
+    fn uninstall_skill_rejects_symlinked_skills_root() {
+        let home = tempfile::tempdir().unwrap();
+        let real_root = tempfile::tempdir().unwrap();
+        let real_skills = real_root.path().join("skills");
+        std::fs::create_dir_all(&real_skills).unwrap();
+        write(
+            &real_skills.join("real").join("SKILL.md"),
+            "---\nname: real\ndescription: in real root\n---\n",
+        );
+        std::fs::create_dir_all(home.path().join(".openhuman")).unwrap();
+        std::os::unix::fs::symlink(&real_skills, home.path().join(".openhuman").join("skills"))
+            .unwrap();
+        let err = uninstall_skill(
+            UninstallSkillParams {
+                name: "real".into(),
+            },
+            Some(home.path()),
+        )
+        .unwrap_err();
+        assert!(
+            err.contains("skills root") && err.contains("symlink"),
+            "symlinked skills root must be refused, got: {err}"
+        );
+        assert!(
+            real_skills.join("real").join("SKILL.md").exists(),
+            "target must survive"
+        );
+    }
 }

src/openhuman/skills/schemas.rs

@@ -27,7 +27,8 @@ use crate::core::{ControllerSchema, FieldSchema, TypeSchema};
 use crate::openhuman::config::Config;
 use crate::openhuman::skills::ops::{
     create_skill, discover_skills, install_skill_from_url, is_workspace_trusted,
-    read_skill_resource, CreateSkillParams, InstallSkillFromUrlParams, Skill, SkillScope,
+    read_skill_resource, uninstall_skill, CreateSkillParams, InstallSkillFromUrlParams, Skill,
+    SkillScope, UninstallSkillParams,
 };
 use crate::rpc::RpcOutcome;
 
@@ -96,8 +97,17 @@ struct SkillSummary {
 
 impl From<Skill> for SkillSummary {
     fn from(s: Skill) -> Self {
+        // `id` is the on-disk slug the uninstall RPC resolves against.
+        // Prefer `dir_name`, but fall back to `name` for back-compat on
+        // deserialised `Skill` values written before `dir_name` existed
+        // (default empty string).
+        let id = if s.dir_name.is_empty() {
+            s.name.clone()
+        } else {
+            s.dir_name.clone()
+        };
         SkillSummary {
-            id: s.name.clone(),
+            id,
             name: s.name,
             description: s.description,
             version: s.version,
@@ -160,12 +170,20 @@ struct SkillsInstallFromUrlResult {
     new_skills: Vec<String>,
 }
 
+#[derive(Debug, Serialize)]
+struct SkillsUninstallResult {
+    name: String,
+    removed_path: String,
+    scope: SkillScope,
+}
+
 pub fn all_skills_controller_schemas() -> Vec<ControllerSchema> {
     vec![
         skills_schemas("skills_list"),
         skills_schemas("skills_read_resource"),
         skills_schemas("skills_create"),
         skills_schemas("skills_install_from_url"),
+        skills_schemas("skills_uninstall"),
     ]
 }
 
@@ -187,6 +205,10 @@ pub fn all_skills_registered_controllers() -> Vec<RegisteredController> {
             schema: skills_schemas("skills_install_from_url"),
             handler: handle_skills_install_from_url,
         },
+        RegisteredController {
+            schema: skills_schemas("skills_uninstall"),
+            handler: handle_skills_uninstall,
+        },
     ]
 }
 
@@ -349,6 +371,37 @@ pub fn skills_schemas(function: &str) -> ControllerSchema {
                 },
             ],
         },
+        "skills_uninstall" => ControllerSchema {
+            namespace: "skills",
+            function: "uninstall",
+            description: "Remove an installed user-scope SKILL.md skill from `~/.openhuman/skills/<name>/`. Only user-scope installs are supported; project-scope and legacy skills are read-only. Rejects path separators and traversal; canonicalises before delete.",
+            inputs: vec![FieldSchema {
+                name: "name",
+                ty: TypeSchema::String,
+                comment: "Exact on-disk slug of the installed skill — matches SkillSummary.id (the directory under ~/.openhuman/skills/), which may differ from the frontmatter display name in Skill.name.",
+                required: true,
+            }],
+            outputs: vec![
+                FieldSchema {
+                    name: "name",
+                    ty: TypeSchema::String,
+                    comment: "Echo of the removed skill slug.",
+                    required: true,
+                },
+                FieldSchema {
+                    name: "removed_path",
+                    ty: TypeSchema::String,
+                    comment: "Canonical on-disk path that was deleted.",
+                    required: true,
+                },
+                FieldSchema {
+                    name: "scope",
+                    ty: TypeSchema::String,
+                    comment: "Scope the uninstall applied to. Always `user` today.",
+                    required: true,
+                },
+            ],
+        },
         _ => ControllerSchema {
             namespace: "skills",
             function: "unknown",
@@ -487,6 +540,34 @@ fn handle_skills_install_from_url(params: Map<String, Value>) -> ControllerFutur
     })
 }
 
+fn handle_skills_uninstall(params: Map<String, Value>) -> ControllerFuture {
+    Box::pin(async move {
+        let payload = deserialize_params::<UninstallSkillParams>(params)?;
+        tracing::debug!(name = %payload.name, "[skills][rpc] uninstall");
+        match uninstall_skill(payload, None) {
+            Ok(outcome) => {
+                tracing::debug!(
+                    name = %outcome.name,
+                    removed_path = %outcome.removed_path,
+                    "[skills][rpc] uninstall: ok"
+                );
+                to_json(RpcOutcome::new(
+                    SkillsUninstallResult {
+                        name: outcome.name,
+                        removed_path: outcome.removed_path,
+                        scope: outcome.scope,
+                    },
+                    Vec::new(),
+                ))
+            }
+            Err(err) => {
+                tracing::debug!(error = %err, "[skills][rpc] uninstall: rejected");
+                Err(err)
+            }
+        }
+    })
+}
+
 /// Resolve the active [`Config`]. Falls back to `Config::default()` with a
 /// best-effort workspace directory if the persisted load times out or errors,
 /// so headless diagnostics still work in partially-initialized environments.

tests/json_rpc_e2e.rs

@@ -2277,3 +2277,115 @@ async fn notification_settings_roundtrip_and_disabled_ingest_skip() {
     mock_join.abort();
     rpc_join.abort();
 }
+
+/// End-to-end coverage for `openhuman.skills_uninstall`.
+///
+/// Validates that the RPC method is registered, wire-decodes
+/// `UninstallSkillParams`, resolves the slug against
+/// `~/.openhuman/skills/<slug>/`, removes the directory on success, and
+/// forwards the core error message verbatim for the two documented
+/// failure modes (missing SKILL.md and path traversal). Previously only
+/// the `uninstall_skill(...)` helper was tested — the wire layer
+/// (controller registration, param decoding, response shape) was not.
+#[tokio::test]
+async fn skills_uninstall_rpc_e2e() {
+    let _env_lock = json_rpc_e2e_env_lock();
+    let tmp = tempdir().expect("tempdir");
+    let home = tmp.path();
+
+    let _home_guard = EnvVarGuard::set_to_path("HOME", home);
+    let _workspace_guard = EnvVarGuard::unset("OPENHUMAN_WORKSPACE");
+
+    let skills_root = home.join(".openhuman").join("skills");
+    std::fs::create_dir_all(&skills_root).expect("mkdir skills root");
+
+    // Seed a skill whose on-disk slug differs from its frontmatter name —
+    // mirrors the bug CodeRabbit flagged for #781: the UI must send the
+    // slug (`SkillSummary.id` / directory name), not the display name.
+    let slug = "weather-helper";
+    let skill_dir = skills_root.join(slug);
+    std::fs::create_dir_all(&skill_dir).expect("mkdir skill dir");
+    std::fs::write(
+        skill_dir.join("SKILL.md"),
+        "---\nname: Weather Helper\ndescription: fetches local weather\n---\n# body\n",
+    )
+    .expect("write SKILL.md");
+
+    let (rpc_addr, rpc_join) = serve_on_ephemeral(build_core_http_router(false)).await;
+    let rpc_base = format!("http://{rpc_addr}");
+
+    // --- success path ------------------------------------------------------
+    let ok = post_json_rpc(
+        &rpc_base,
+        6001,
+        "openhuman.skills_uninstall",
+        json!({ "name": slug }),
+    )
+    .await;
+    let ok_result = assert_no_jsonrpc_error(&ok, "skills_uninstall success");
+    assert_eq!(
+        ok_result.get("name").and_then(Value::as_str),
+        Some(slug),
+        "response echoes the slug we passed"
+    );
+    assert_eq!(
+        ok_result.get("scope").and_then(Value::as_str),
+        Some("user"),
+        "uninstall is user-scope only"
+    );
+    let removed_path = ok_result
+        .get("removed_path")
+        .and_then(Value::as_str)
+        .expect("removed_path in response");
+    assert!(
+        removed_path.ends_with(slug)
+            || removed_path.contains(&format!("skills{}{slug}", std::path::MAIN_SEPARATOR)),
+        "removed_path should reference the slug dir, got: {removed_path}"
+    );
+    assert!(
+        !skill_dir.exists(),
+        "directory must be gone after uninstall"
+    );
+
+    // --- not-installed path: core error forwarded verbatim ----------------
+    let missing = post_json_rpc(
+        &rpc_base,
+        6002,
+        "openhuman.skills_uninstall",
+        json!({ "name": "does-not-exist" }),
+    )
+    .await;
+    let err = missing
+        .get("error")
+        .unwrap_or_else(|| panic!("expected error, got {missing}"));
+    let err_msg = err
+        .get("message")
+        .and_then(Value::as_str)
+        .or_else(|| err.get("data").and_then(Value::as_str))
+        .unwrap_or("");
+    assert!(
+        err_msg.contains("not installed") || err.to_string().contains("not installed"),
+        "expected verbatim 'not installed' error, got: {err}"
+    );
+
+    // --- path-traversal path: core error forwarded verbatim ---------------
+    let traversal = post_json_rpc(
+        &rpc_base,
+        6003,
+        "openhuman.skills_uninstall",
+        json!({ "name": "../etc" }),
+    )
+    .await;
+    let traversal_err = traversal
+        .get("error")
+        .unwrap_or_else(|| panic!("expected error, got {traversal}"));
+    let traversal_msg = traversal_err.to_string();
+    assert!(
+        traversal_msg.contains("path separators")
+            || traversal_msg.contains("path escapes")
+            || traversal_msg.contains("not installed"),
+        "expected traversal rejection error, got: {traversal_err}"
+    );
+
+    rpc_join.abort();
+}