fix(cli): resolve skill uninstall failure when skill name is updated (#22085)

packages/cli/src/utils/skillUtils.test.ts

@@ -8,7 +8,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
 import * as fs from 'node:fs/promises';
 import * as path from 'node:path';
 import * as os from 'node:os';
-import { installSkill, linkSkill } from './skillUtils.js';
+import { installSkill, linkSkill, uninstallSkill } from './skillUtils.js';
 
 describe('skillUtils', () => {
   let tempDir: string;
@@ -17,11 +17,13 @@ describe('skillUtils', () => {
   beforeEach(async () => {
     tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'skill-utils-test-'));
     vi.spyOn(process, 'cwd').mockReturnValue(tempDir);
+    vi.stubEnv('GEMINI_CLI_HOME', tempDir);
   });
 
   afterEach(async () => {
     await fs.rm(tempDir, { recursive: true, force: true });
     vi.restoreAllMocks();
+    vi.unstubAllEnvs();
   });
 
   const itif = (condition: boolean) => (condition ? it : it.skip);
@@ -212,4 +214,74 @@ describe('skillUtils', () => {
     const installedExists = await fs.stat(installedPath).catch(() => null);
     expect(installedExists).toBeNull();
   });
+
+  describe('uninstallSkill', () => {
+    it('should successfully uninstall an existing skill', async () => {
+      const skillsDir = path.join(tempDir, '.gemini/skills');
+      const skillDir = path.join(skillsDir, 'test-skill');
+      await fs.mkdir(skillDir, { recursive: true });
+      await fs.writeFile(
+        path.join(skillDir, 'SKILL.md'),
+        '---\nname: test-skill\ndescription: test\n---\nbody',
+      );
+
+      const result = await uninstallSkill('test-skill', 'user');
+      expect(result?.location).toContain('test-skill');
+
+      const exists = await fs.stat(skillDir).catch(() => null);
+      expect(exists).toBeNull();
+    });
+
+    it('should return null for non-existent skill', async () => {
+      const result = await uninstallSkill('non-existent', 'user');
+      expect(result).toBeNull();
+    });
+
+    itif(process.platform !== 'win32')(
+      'should successfully uninstall a skill even if its name was updated after linking',
+      async () => {
+        // 1. Create source skill
+        const sourceDir = path.join(tempDir, 'source-skill');
+        await fs.mkdir(sourceDir, { recursive: true });
+        const skillMdPath = path.join(sourceDir, 'SKILL.md');
+        await fs.writeFile(
+          skillMdPath,
+          '---\nname: original-name\ndescription: test\n---\nbody',
+        );
+
+        // 2. Link it
+        const skillsDir = path.join(tempDir, '.gemini/skills');
+        await fs.mkdir(skillsDir, { recursive: true });
+        const destPath = path.join(skillsDir, 'original-name');
+        await fs.symlink(sourceDir, destPath, 'dir');
+
+        // 3. Update name in source
+        await fs.writeFile(
+          skillMdPath,
+          '---\nname: updated-name\ndescription: test\n---\nbody',
+        );
+
+        // 4. Uninstall by NEW name (this is the bug fix)
+        const result = await uninstallSkill('updated-name', 'user');
+        expect(result).not.toBeNull();
+        expect(result?.location).toBe(destPath);
+
+        const exists = await fs.lstat(destPath).catch(() => null);
+        expect(exists).toBeNull();
+      },
+    );
+
+    it('should successfully uninstall a skill by directory name if metadata is missing (fallback)', async () => {
+      const skillsDir = path.join(tempDir, '.gemini/skills');
+      const skillDir = path.join(skillsDir, 'test-skill-dir');
+      await fs.mkdir(skillDir, { recursive: true });
+      // No SKILL.md here
+
+      const result = await uninstallSkill('test-skill-dir', 'user');
+      expect(result?.location).toBe(skillDir);
+
+      const exists = await fs.stat(skillDir).catch(() => null);
+      expect(exists).toBeNull();
+    });
+  });
 });

packages/cli/src/utils/skillUtils.ts

@@ -269,14 +269,32 @@ export async function uninstallSkill(
       ? storage.getProjectSkillsDir()
       : Storage.getUserSkillsDir();
 
-  const skillPath = path.join(targetDir, name);
+  // Load all skills in the target directory to find the one with the matching name
+  const discoveredSkills = await loadSkillsFromDir(targetDir);
+  const skillToUninstall = discoveredSkills.find((s) => s.name === name);
 
-  const exists = await fs.stat(skillPath).catch(() => null);
+  if (!skillToUninstall) {
+    // Fallback: Check if a directory with the given name exists.
+    // This maintains backward compatibility for cases where the metadata might be missing or corrupted
+    // but the directory name matches the user's request.
+    const skillPath = path.resolve(targetDir, name);
 
-  if (!exists) {
-    return null;
+    // Security check: ensure the resolved path is within the target directory to prevent path traversal
+    if (!skillPath.startsWith(path.resolve(targetDir))) {
+      return null;
+    }
+
+    const exists = await fs.lstat(skillPath).catch(() => null);
+
+    if (!exists) {
+      return null;
+    }
+
+    await fs.rm(skillPath, { recursive: true, force: true });
+    return { location: skillPath };
   }
 
-  await fs.rm(skillPath, { recursive: true, force: true });
-  return { location: skillPath };
+  const skillDir = path.dirname(skillToUninstall.location);
+  await fs.rm(skillDir, { recursive: true, force: true });
+  return { location: skillDir };
 }