fix(skills): preserve SKILL.md content when renaming

Rename skills by moving the skill directory and updating frontmatter name instead of recreate-with-stub-description, which wiped the body and supporting files. Co-authored-by: Serhii Dziupin <makeittech@users.noreply.github.com>
2026-08-03 07:02:24 +00:00
parent 2ba8ae8bd4
commit f0591515fd
20 changed files with 301 additions and 33 deletions
@@ -351,6 +351,7 @@ an authoritative loopback callback URL even when OpenChamber binds port `0`.
 ## Public exports (skill-routes.js)
 - `registerSkillRoutes(app, dependencies)`: registers skills-related routes:
  - Skills config CRUD and metadata under `/api/config/skills*`
+  - Skill rename via `PATCH /api/config/skills/:name` with `{ renameTo }` (directory rename preserves `SKILL.md` body and supporting files)
  - Skills catalog listing/source pagination, scan, and install routes
  - Supporting skill file read/write/delete routes

@@ -38,7 +38,7 @@ import {
  decodePluginId,
 } from './plugins.js';
 import { SKILL_DIR, SKILL_SCOPE, readSkillSupportingFile, writeSkillSupportingFile, deleteSkillSupportingFile } from './shared.js';
-import { getSkillSources, discoverSkills, mergeDiscoveredSkills, createSkill, updateSkill, deleteSkill } from './skills.js';
+import { getSkillSources, discoverSkills, mergeDiscoveredSkills, createSkill, updateSkill, deleteSkill, renameSkill } from './skills.js';
 import { getCuratedSkillsSources } from '../skills-catalog/curated-sources.js';
 import { getCacheKey, getCachedScan, setCachedScan } from '../skills-catalog/cache.js';
 import { isClawdHubSource, parseSkillRepoSource } from '../skills-catalog/source.js';
@@ -256,6 +256,7 @@ export const createFeatureRoutesRuntime = (dependencies) => {
      createSkill,
      updateSkill,
      deleteSkill,
+      renameSkill,
      readSkillSupportingFile,
      writeSkillSupportingFile,
      deleteSkillSupportingFile,
@@ -21,6 +21,7 @@ export const registerSkillRoutes = (app, dependencies) => {
    createSkill,
    updateSkill,
    deleteSkill,
+    renameSkill,
    readSkillSupportingFile,
    writeSkillSupportingFile,
    deleteSkillSupportingFile,
@@ -611,6 +612,22 @@ export const registerSkillRoutes = (app, dependencies) => {
        return res.status(400).json({ error });
      }

+      if (typeof updates?.renameTo === 'string') {
+        const newName = updates.renameTo.trim();
+        console.log(`[Server] Renaming skill: ${skillName} -> ${newName}`);
+        console.log('[Server] Working directory:', directory);
+        renameSkill(skillName, newName, directory);
+        await refreshOpenCodeAfterConfigChange('skill rename');
+
+        return res.json({
+          success: true,
+          name: newName,
+          requiresReload: true,
+          message: `Skill renamed to ${newName} successfully. Reloading interface…`,
+          reloadDelayMs: clientReloadDelayMs,
+        });
+      }
+
      console.log(`[Server] Updating skill: ${skillName}`);
      console.log('[Server] Working directory:', directory);

@@ -412,12 +412,22 @@ function getSkillSources(skillName, workingDirectory, discoveredSkill = null) {
  return sources;
 }

-function createSkill(skillName, config, workingDirectory, scope) {
-  ensureDirs();
+function isValidSkillName(skillName) {
+  return typeof skillName === 'string'
+    && skillName.length > 0
+    && skillName.length <= 64
+    && /^[a-z0-9][a-z0-9-]*[a-z0-9]$|^[a-z0-9]$/.test(skillName);
+}

-  if (!/^[a-z0-9][a-z0-9-]*[a-z0-9]$|^[a-z0-9]$/.test(skillName) || skillName.length > 64) {
+function assertValidSkillName(skillName) {
+  if (!isValidSkillName(skillName)) {
    throw new Error(`Invalid skill name "${skillName}". Must be 1-64 lowercase alphanumeric characters with hyphens, cannot start or end with hyphen.`);
  }
+}
+
+function createSkill(skillName, config, workingDirectory, scope) {
+  ensureDirs();
+  assertValidSkillName(skillName);

  const existing = getSkillScope(skillName, workingDirectory);
  if (existing.path) {
@@ -505,7 +515,7 @@ function updateSkill(skillName, updates, workingDirectory, targetPath = null) {
  let mdModified = false;

  for (const [field, value] of Object.entries(updates)) {
-    if (field === 'scope' || field === 'source' || field === 'targetPath') {
+    if (field === 'scope' || field === 'source' || field === 'targetPath' || field === 'renameTo') {
      continue;
    }
    
@@ -592,6 +602,65 @@ function deleteSkill(skillName, workingDirectory) {
  }
 }

+function renameSkill(oldName, newName, workingDirectory) {
+  ensureDirs();
+  assertValidSkillName(newName);
+
+  if (oldName === newName) {
+    return;
+  }
+
+  const existing = getSkillScope(oldName, workingDirectory);
+  if (!existing.path) {
+    throw new Error(`Skill "${oldName}" not found`);
+  }
+  if (existing.path === BUILT_IN_SKILL_LOCATION || !fs.existsSync(existing.path)) {
+    throw new Error(`Skill "${oldName}" cannot be renamed`);
+  }
+  if (path.basename(existing.path) !== 'SKILL.md') {
+    throw new Error(`Skill "${oldName}" target must be a SKILL.md file`);
+  }
+
+  const conflict = getSkillScope(newName, workingDirectory);
+  if (conflict.path) {
+    throw new Error(`Skill ${newName} already exists at ${conflict.path}`);
+  }
+
+  const oldDir = path.dirname(existing.path);
+  const newDir = path.join(path.dirname(oldDir), newName);
+  const directoriesDiffer = path.resolve(oldDir) !== path.resolve(newDir);
+
+  if (directoriesDiffer && fs.existsSync(newDir)) {
+    throw new Error(`Skill directory already exists at ${newDir}`);
+  }
+
+  // Rename the skill directory in place so supporting files and SKILL.md body are preserved.
+  if (directoriesDiffer) {
+    fs.renameSync(oldDir, newDir);
+  }
+
+  const newPath = path.join(newDir, 'SKILL.md');
+  try {
+    const mdData = parseMdFile(newPath);
+    mdData.frontmatter = {
+      ...mdData.frontmatter,
+      name: newName,
+    };
+    writeMdFile(newPath, mdData.frontmatter, mdData.body);
+  } catch (error) {
+    if (directoriesDiffer && fs.existsSync(newDir) && !fs.existsSync(oldDir)) {
+      try {
+        fs.renameSync(newDir, oldDir);
+      } catch (rollbackError) {
+        console.error(`Failed to rollback skill rename from ${newDir} to ${oldDir}:`, rollbackError);
+      }
+    }
+    throw error;
+  }
+
+  console.log(`Renamed skill: ${oldName} -> ${newName} (path: ${newPath})`);
+}
+
 export {
  getSkillSources,
  discoverSkills,
@@ -599,4 +668,5 @@ export {
  createSkill,
  updateSkill,
  deleteSkill,
+  renameSkill,
 };
@@ -1,8 +1,9 @@
 import { describe, expect, it } from 'vitest';
+import fs from 'fs';
 import fsPromises from 'fs/promises';
 import os from 'os';
 import path from 'path';
-import { getSkillSources, mergeDiscoveredSkills } from './skills.js';
+import { getSkillSources, mergeDiscoveredSkills, renameSkill } from './skills.js';

 describe('skills', () => {
  it('merges locally discovered skills missing from OpenCode live discovery', () => {
@@ -110,4 +111,71 @@ describe('skills', () => {
      await fsPromises.rm(tempRoot, { recursive: true, force: true });
    }
  });
+
+  it('renames a skill directory while preserving SKILL.md body and supporting files', async () => {
+    const tempRoot = await fsPromises.mkdtemp(path.join(os.tmpdir(), 'oc-skills-rename-'));
+    const projectRoot = path.join(tempRoot, 'project');
+    const skillDir = path.join(projectRoot, '.opencode', 'skills', 'original-skill');
+    const skillPath = path.join(skillDir, 'SKILL.md');
+    const supportPath = path.join(skillDir, 'notes.md');
+    const body = [
+      '# Original Skill',
+      '',
+      'Preserve this non-trivial body across rename.',
+      '',
+      '## Details',
+      '',
+      '- step one',
+      '- step two',
+    ].join('\n');
+
+    try {
+      await fsPromises.mkdir(skillDir, { recursive: true });
+      await fsPromises.writeFile(
+        skillPath,
+        [
+          '---',
+          'name: original-skill',
+          'description: Original skill description',
+          'license: MIT',
+          '---',
+          '',
+          body,
+          '',
+        ].join('\n'),
+        'utf8',
+      );
+      await fsPromises.writeFile(supportPath, 'supporting file contents\n', 'utf8');
+
+      renameSkill('original-skill', 'renamed-skill', projectRoot);
+
+      const renamedDir = path.join(projectRoot, '.opencode', 'skills', 'renamed-skill');
+      const renamedPath = path.join(renamedDir, 'SKILL.md');
+      const renamedSupportPath = path.join(renamedDir, 'notes.md');
+
+      expect(fs.existsSync(skillDir)).toBe(false);
+      expect(fs.existsSync(renamedPath)).toBe(true);
+      expect(fs.existsSync(renamedSupportPath)).toBe(true);
+
+      const sources = getSkillSources('renamed-skill', projectRoot, {
+        name: 'renamed-skill',
+        path: renamedPath,
+        scope: 'project',
+        source: 'opencode',
+        description: 'fallback',
+      });
+
+      expect(sources.md.exists).toBe(true);
+      expect(sources.md.name).toBe('renamed-skill');
+      expect(sources.md.description).toBe('Original skill description');
+      expect(sources.md.instructions).toBe(body);
+      expect(await fsPromises.readFile(renamedSupportPath, 'utf8')).toBe('supporting file contents\n');
+
+      const raw = await fsPromises.readFile(renamedPath, 'utf8');
+      expect(raw).toContain('license: MIT');
+      expect(raw).not.toContain('Renamed skill');
+    } finally {
+      await fsPromises.rm(tempRoot, { recursive: true, force: true });
+    }
+  });
 });