Merge pull request #2586 from openchamber/feat/skill-renaming-content-preservation-c1d5

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

packages/web/server/lib/opencode/DOCUMENTATION.md

@@ -358,6 +358,8 @@ 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; restricted to managed skill roots under `.opencode/skills|skill`, `.claude/skills`, and `.agents/skills`)
+  - Skill list responses include authoritative `renamable` derived from the same managed-root policy used by rename
   - Skills catalog listing/source pagination, scan, and install routes
   - Supporting skill file read/write/delete routes
   - Directory resolution prefers an explicit request directory, then soft-falls

packages/web/server/lib/opencode/feature-routes-runtime.js

@@ -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, isManagedSkillPath } 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';
@@ -257,6 +257,8 @@ export const createFeatureRoutesRuntime = (dependencies) => {
       createSkill,
       updateSkill,
       deleteSkill,
+      renameSkill,
+      isManagedSkillPath,
       readSkillSupportingFile,
       writeSkillSupportingFile,
       deleteSkillSupportingFile,

packages/web/server/lib/opencode/skill-routes.js

@@ -21,6 +21,8 @@ export const registerSkillRoutes = (app, dependencies) => {
     createSkill,
     updateSkill,
     deleteSkill,
+    renameSkill,
+    isManagedSkillPath,
     readSkillSupportingFile,
     writeSkillSupportingFile,
     deleteSkillSupportingFile,
@@ -236,9 +238,15 @@ export const registerSkillRoutes = (app, dependencies) => {
 
       const enrichedSkills = skills.map((skill) => {
         const sources = getSkillSources(skill.name, directory, skill);
+        const skillPath = typeof skill.path === 'string' ? skill.path : null;
         return {
           ...skill,
-          sources
+          sources,
+          renamable: Boolean(
+            skillPath
+            && skillPath !== '<built-in>'
+            && isManagedSkillPath(skillPath, directory)
+          ),
         };
       });
 
@@ -635,6 +643,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);
 

packages/web/server/lib/opencode/skill-routes.test.js

@@ -9,7 +9,9 @@ import {
   deleteSkill,
   discoverSkills,
   getSkillSources,
+  isManagedSkillPath,
   mergeDiscoveredSkills,
+  renameSkill,
   updateSkill,
 } from './skills.js';
 import {
@@ -58,6 +60,8 @@ const startSkillsApp = ({ projectRoot }) => {
     createSkill,
     updateSkill,
     deleteSkill,
+    renameSkill,
+    isManagedSkillPath,
     readSkillSupportingFile,
     writeSkillSupportingFile,
     deleteSkillSupportingFile,
@@ -154,4 +158,62 @@ describe('skill-routes directory soft fallback', () => {
     const payload = await listResponse.json();
     expect(payload.skills.map((skill) => skill.name)).toContain('manual-repo-skill');
   });
+
+  it('marks managed-root skills renamable and cache skills not renamable', async () => {
+    projectRoot = createTempProject();
+    const managedDir = path.join(projectRoot, '.opencode', 'skills', 'managed-list-skill');
+    fs.mkdirSync(managedDir, { recursive: true });
+    fs.writeFileSync(
+      path.join(managedDir, 'SKILL.md'),
+      [
+        '---',
+        'name: managed-list-skill',
+        'description: Managed list skill',
+        '---',
+        '',
+        'Managed body',
+        '',
+      ].join('\n'),
+      'utf8',
+    );
+
+    const cacheStamp = `oc-skill-routes-${Date.now()}`;
+    const cacheDir = path.join(os.homedir(), '.cache', 'opencode', 'skills', cacheStamp, 'cache-list-skill');
+    fs.mkdirSync(cacheDir, { recursive: true });
+    fs.writeFileSync(
+      path.join(cacheDir, 'SKILL.md'),
+      [
+        '---',
+        'name: cache-list-skill',
+        'description: Cache list skill',
+        '---',
+        '',
+        'Cache body',
+        '',
+      ].join('\n'),
+      'utf8',
+    );
+
+    try {
+      appHandle = startSkillsApp({ projectRoot });
+      const listResponse = await fetch(
+        `${appHandle.baseUrl}/api/config/skills?directory=${encodeURIComponent(projectRoot)}`,
+      );
+      expect(listResponse.status).toBe(200);
+      const payload = await listResponse.json();
+
+      const managed = payload.skills.find((entry) => entry.name === 'managed-list-skill');
+      const cached = payload.skills.find((entry) => entry.name === 'cache-list-skill');
+
+      expect(managed).toBeTruthy();
+      expect(managed.renamable).toBe(true);
+      expect(cached).toBeTruthy();
+      expect(cached.renamable).toBe(false);
+    } finally {
+      fs.rmSync(path.join(os.homedir(), '.cache', 'opencode', 'skills', cacheStamp), {
+        recursive: true,
+        force: true,
+      });
+    }
+  });
 });

packages/web/server/lib/opencode/skills.js

@@ -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,130 @@ function deleteSkill(skillName, workingDirectory) {
   }
 }
 
+function isPathInside(candidatePath, parentPath) {
+  if (!candidatePath || !parentPath) return false;
+  const resolvedCandidate = path.resolve(candidatePath);
+  const resolvedParent = path.resolve(parentPath);
+  return resolvedCandidate === resolvedParent
+    || resolvedCandidate.startsWith(`${resolvedParent}${path.sep}`);
+}
+
+function getManagedSkillRoots(workingDirectory) {
+  const roots = [];
+  const pushRoot = (dir) => {
+    if (!dir) return;
+    const resolved = path.resolve(dir);
+    if (!roots.includes(resolved)) {
+      roots.push(resolved);
+    }
+  };
+
+  pushRoot(SKILL_DIR);
+  pushRoot(path.join(OPENCODE_CONFIG_DIR, 'skill'));
+  pushRoot(path.join(os.homedir(), '.opencode', 'skills'));
+  pushRoot(path.join(os.homedir(), '.opencode', 'skill'));
+  pushRoot(path.join(os.homedir(), '.claude', 'skills'));
+  pushRoot(path.join(os.homedir(), '.agents', 'skills'));
+
+  const customConfigDir = process.env.OPENCODE_CONFIG_DIR
+    ? path.resolve(process.env.OPENCODE_CONFIG_DIR)
+    : null;
+  if (customConfigDir) {
+    pushRoot(path.join(customConfigDir, 'skills'));
+    pushRoot(path.join(customConfigDir, 'skill'));
+  }
+
+  if (workingDirectory) {
+    const worktreeRoot = findWorktreeRoot(workingDirectory) || path.resolve(workingDirectory);
+    for (const ancestor of getAncestors(workingDirectory, worktreeRoot)) {
+      pushRoot(path.join(ancestor, '.opencode', 'skills'));
+      pushRoot(path.join(ancestor, '.opencode', 'skill'));
+      pushRoot(path.join(ancestor, '.claude', 'skills'));
+      pushRoot(path.join(ancestor, '.agents', 'skills'));
+    }
+  }
+
+  return roots;
+}
+
+function isManagedSkillPath(skillMdPath, workingDirectory) {
+  if (!skillMdPath || skillMdPath === BUILT_IN_SKILL_LOCATION) {
+    return false;
+  }
+  const skillDir = path.dirname(path.resolve(skillMdPath));
+  return getManagedSkillRoots(workingDirectory).some((root) => isPathInside(skillDir, root));
+}
+
+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`);
+  }
+  if (!isManagedSkillPath(existing.path, workingDirectory)) {
+    throw new Error(`Skill "${oldName}" is outside managed skill directories and cannot be renamed`);
+  }
+
+  const mdDataBeforeMove = parseMdFile(existing.path);
+  const frontmatterName = typeof mdDataBeforeMove.frontmatter?.name === 'string'
+    ? mdDataBeforeMove.frontmatter.name
+    : oldName;
+  if (frontmatterName !== oldName) {
+    throw new Error(`Skill "${oldName}" does not match ${existing.path}`);
+  }
+
+  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 +733,6 @@ export {
   createSkill,
   updateSkill,
   deleteSkill,
+  renameSkill,
+  isManagedSkillPath,
 };

packages/web/server/lib/opencode/skills.test.js

@@ -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 { discoverSkills, getSkillSources, mergeDiscoveredSkills } from './skills.js';
+import { discoverSkills, getSkillSources, mergeDiscoveredSkills, renameSkill } from './skills.js';
 
 describe('skills', () => {
   it('merges locally discovered skills missing from OpenCode live discovery', () => {
@@ -147,4 +148,198 @@ 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 });
+    }
+  });
+
+  it('rolls back the directory rename when frontmatter write fails', async () => {
+    const tempRoot = await fsPromises.mkdtemp(path.join(os.tmpdir(), 'oc-skills-rename-rollback-'));
+    const projectRoot = path.join(tempRoot, 'project');
+    const skillDir = path.join(projectRoot, '.opencode', 'skills', 'rollback-skill');
+    const skillPath = path.join(skillDir, 'SKILL.md');
+    const body = '# Rollback body\n\nMust remain in the original directory.';
+
+    try {
+      await fsPromises.mkdir(skillDir, { recursive: true });
+      await fsPromises.writeFile(
+        skillPath,
+        [
+          '---',
+          'name: rollback-skill',
+          'description: Rollback skill',
+          '---',
+          '',
+          body,
+          '',
+        ].join('\n'),
+        'utf8',
+      );
+      await fsPromises.chmod(skillPath, 0o444);
+
+      expect(() => renameSkill('rollback-skill', 'rollback-skill-renamed', projectRoot)).toThrow();
+
+      expect(fs.existsSync(skillDir)).toBe(true);
+      expect(fs.existsSync(path.join(projectRoot, '.opencode', 'skills', 'rollback-skill-renamed'))).toBe(false);
+      expect(await fsPromises.readFile(skillPath, 'utf8')).toContain(body);
+    } finally {
+      try {
+        await fsPromises.chmod(skillPath, 0o644);
+      } catch {
+        // Best-effort cleanup when the file was rolled back under a different mode.
+      }
+      await fsPromises.rm(tempRoot, { recursive: true, force: true });
+    }
+  });
+
+  it('rejects invalid names, missing skills, conflicts, unmanaged paths, and frontmatter mismatches', async () => {
+    const tempRoot = await fsPromises.mkdtemp(path.join(os.tmpdir(), 'oc-skills-rename-reject-'));
+    const projectRoot = path.join(tempRoot, 'project');
+    const managedDir = path.join(projectRoot, '.opencode', 'skills', 'managed-skill');
+    const conflictDir = path.join(projectRoot, '.opencode', 'skills', 'taken-name');
+    const mismatchDir = path.join(projectRoot, '.opencode', 'skills', 'folder-name');
+    const cacheStamp = `oc-rename-${Date.now()}`;
+    const cacheDir = path.join(os.homedir(), '.cache', 'opencode', 'skills', cacheStamp, 'cache-skill');
+
+    try {
+      await fsPromises.mkdir(managedDir, { recursive: true });
+      await fsPromises.writeFile(
+        path.join(managedDir, 'SKILL.md'),
+        [
+          '---',
+          'name: managed-skill',
+          'description: Managed',
+          '---',
+          '',
+          'Managed body',
+          '',
+        ].join('\n'),
+        'utf8',
+      );
+
+      await fsPromises.mkdir(conflictDir, { recursive: true });
+      await fsPromises.writeFile(
+        path.join(conflictDir, 'SKILL.md'),
+        [
+          '---',
+          'name: taken-name',
+          'description: Taken',
+          '---',
+          '',
+          'Taken body',
+          '',
+        ].join('\n'),
+        'utf8',
+      );
+
+      await fsPromises.mkdir(mismatchDir, { recursive: true });
+      await fsPromises.writeFile(
+        path.join(mismatchDir, 'SKILL.md'),
+        [
+          '---',
+          'name: frontmatter-name',
+          'description: Mismatch',
+          '---',
+          '',
+          'Mismatch body',
+          '',
+        ].join('\n'),
+        'utf8',
+      );
+
+      await fsPromises.mkdir(cacheDir, { recursive: true });
+      await fsPromises.writeFile(
+        path.join(cacheDir, 'SKILL.md'),
+        [
+          '---',
+          'name: cache-skill',
+          'description: Cache skill',
+          '---',
+          '',
+          'Cache body',
+          '',
+        ].join('\n'),
+        'utf8',
+      );
+
+      expect(() => renameSkill('managed-skill', 'Invalid_Name', projectRoot)).toThrow(/Invalid skill name/);
+      expect(() => renameSkill('missing-skill', 'new-skill', projectRoot)).toThrow(/not found/);
+      expect(() => renameSkill('managed-skill', 'taken-name', projectRoot)).toThrow(/already exists/);
+      expect(() => renameSkill('folder-name', 'renamed-mismatch', projectRoot)).toThrow(/does not match/);
+      expect(() => renameSkill('cache-skill', 'cache-skill-renamed', projectRoot)).toThrow(/managed skill directories/);
+
+      expect(fs.existsSync(managedDir)).toBe(true);
+      expect(fs.existsSync(cacheDir)).toBe(true);
+      expect(fs.existsSync(path.join(projectRoot, '.opencode', 'skills', 'renamed-mismatch'))).toBe(false);
+    } finally {
+      await fsPromises.rm(tempRoot, { recursive: true, force: true });
+      await fsPromises.rm(path.join(os.homedir(), '.cache', 'opencode', 'skills', cacheStamp), {
+        recursive: true,
+        force: true,
+      });
+    }
+  });
 });