From bfea13ef1d8a67e7bb417d1377513a2d6c093209 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 3 Aug 2026 08:54:29 +0000 Subject: [PATCH] fix(skills): harden rename to managed roots and cover failures Restrict in-place skill rename to managed skill directories, require frontmatter name to match before moving, roll back/reject with tests, hide rename in the UI for unmanaged paths, and drop unused toast keys. Co-authored-by: Serhii Dziupin --- .../sections/skills/SkillsSidebar.tsx | 19 ++- .../sections/skills/skillLocations.ts | 19 +++ .../ui/src/lib/i18n/messages/de.settings.ts | 2 - .../ui/src/lib/i18n/messages/en.settings.ts | 2 - .../ui/src/lib/i18n/messages/es.settings.ts | 2 - .../ui/src/lib/i18n/messages/fr.settings.ts | 2 - .../ui/src/lib/i18n/messages/ja.settings.ts | 2 - .../ui/src/lib/i18n/messages/ko.settings.ts | 2 - .../ui/src/lib/i18n/messages/pl.settings.ts | 2 - .../src/lib/i18n/messages/pt-BR.settings.ts | 2 - .../ui/src/lib/i18n/messages/uk.settings.ts | 2 - .../src/lib/i18n/messages/zh-CN.settings.ts | 2 - .../src/lib/i18n/messages/zh-TW.settings.ts | 2 - packages/vscode/src/opencodeConfig.ts | 62 ++++++++ .../web/server/lib/opencode/DOCUMENTATION.md | 2 +- packages/web/server/lib/opencode/skills.js | 65 ++++++++ .../web/server/lib/opencode/skills.test.js | 143 ++++++++++++++++++ 17 files changed, 303 insertions(+), 29 deletions(-) diff --git a/packages/ui/src/components/sections/skills/SkillsSidebar.tsx b/packages/ui/src/components/sections/skills/SkillsSidebar.tsx index 5060624e..98646486 100644 --- a/packages/ui/src/components/sections/skills/SkillsSidebar.tsx +++ b/packages/ui/src/components/sections/skills/SkillsSidebar.tsx @@ -27,6 +27,7 @@ import { SidebarGroup } from '@/components/sections/shared/SidebarGroup'; import { Icon } from "@/components/icon/Icon"; import { useI18n } from '@/lib/i18n'; import { SETTINGS_PANEL_TITLE_CLASS } from '@/components/sections/shared/SettingsSection'; +import { isManagedSkillFilesystemPath } from '@/components/sections/skills/skillLocations'; interface SkillsSidebarProps { onItemSelect?: () => void; @@ -35,6 +36,9 @@ interface SkillsSidebarProps { const BUILT_IN_SKILL_LOCATION = ''; const isBuiltInSkill = (skill: DiscoveredSkill | null | undefined): boolean => skill?.path === BUILT_IN_SKILL_LOCATION; +const isRenamableSkill = (skill: DiscoveredSkill | null | undefined): boolean => ( + !!skill && !isBuiltInSkill(skill) && isManagedSkillFilesystemPath(skill.path) +); export const SkillsSidebar: React.FC = ({ onItemSelect }) => { const { t } = useI18n(); @@ -140,14 +144,14 @@ export const SkillsSidebar: React.FC = ({ onItemSelect }) => }; const handleOpenRenameDialog = (skill: DiscoveredSkill) => { - if (isBuiltInSkill(skill)) return; + if (!isRenamableSkill(skill)) return; setRenameNewName(skill.name); setRenameDialogSkill(skill); }; const handleRenameSkill = async () => { if (!renameDialogSkill) return; - if (isBuiltInSkill(renameDialogSkill)) { + if (!isRenamableSkill(renameDialogSkill)) { setRenameDialogSkill(null); return; } @@ -443,13 +447,16 @@ const SkillListItem: React.FC = ({ : t('settings.skills.sidebar.badge.opencode'); const badgeClassName = 'typography-micro text-muted-foreground bg-[var(--surface-muted)] px-1 rounded flex-shrink-0 leading-none pb-px border border-[var(--interactive-border)]/50'; const isBuiltIn = isBuiltInSkill(skill); + const canRename = isRenamableSkill(skill); const [isContextMenuOpen, setIsContextMenuOpen] = React.useState(false); const renderMenuItems = (Item: React.ElementType) => ( <> - { e.stopPropagation(); onRename(); }}> - - {t('settings.common.actions.rename')} - + {canRename ? ( + { e.stopPropagation(); onRename(); }}> + + {t('settings.common.actions.rename')} + + ) : null} { e.stopPropagation(); onDuplicate(); }}> {t('settings.common.actions.duplicate')} diff --git a/packages/ui/src/components/sections/skills/skillLocations.ts b/packages/ui/src/components/sections/skills/skillLocations.ts index a0009a20..cc07fd5e 100644 --- a/packages/ui/src/components/sections/skills/skillLocations.ts +++ b/packages/ui/src/components/sections/skills/skillLocations.ts @@ -57,3 +57,22 @@ export function locationPartsFrom(value: SkillLocationValue): { scope: SkillScop } return { scope: match.scope, source: match.source }; } + +/** True when a discovered skill path is under a managed skill root that rename/delete may mutate. */ +export function isManagedSkillFilesystemPath(skillPath: string | null | undefined): boolean { + if (!skillPath || skillPath === '') return false; + const normalized = skillPath.replace(/\\/g, '/'); + if ( + normalized.includes('/.cache/opencode/skills/') + || normalized.includes('/Caches/opencode/skills/') + || normalized.includes('/Library/Caches/opencode/skills/') + ) { + return false; + } + return ( + /\/\.opencode\/skills?\//.test(normalized) + || /\/\.claude\/skills\//.test(normalized) + || /\/\.agents\/skills\//.test(normalized) + || /\/\.config\/opencode\/skills?\//.test(normalized) + ); +} diff --git a/packages/ui/src/lib/i18n/messages/de.settings.ts b/packages/ui/src/lib/i18n/messages/de.settings.ts index 4f9ea716..d35dbb81 100644 --- a/packages/ui/src/lib/i18n/messages/de.settings.ts +++ b/packages/ui/src/lib/i18n/messages/de.settings.ts @@ -666,8 +666,6 @@ export const settingsDict = { 'settings.skills.sidebar.toast.skillDeleted': 'Skill "{name}" erfolgreich gelöscht', 'settings.skills.sidebar.toast.deleteSkillFailed': 'Skill konnte nicht gelöscht werden', 'settings.skills.sidebar.toast.duplicateLoadFailed': 'Skill-Details für Duplizierung konnten nicht geladen werden', - 'settings.skills.sidebar.toast.renameLoadFailed': 'Skill-Details konnten nicht geladen werden', - 'settings.skills.sidebar.toast.removeOldAfterRenameFailed': 'Alter Skill konnte nach Umbenennung nicht entfernt werden', 'settings.skills.sidebar.toast.renameFailed': 'Skill konnte nicht umbenannt werden', 'settings.skills.sidebar.toast.skillRenamed': 'Skill umbenannt in "{name}"', 'settings.skills.sidebar.deleteDialog.title': 'Skill löschen', diff --git a/packages/ui/src/lib/i18n/messages/en.settings.ts b/packages/ui/src/lib/i18n/messages/en.settings.ts index 210f7f3b..fa36d723 100644 --- a/packages/ui/src/lib/i18n/messages/en.settings.ts +++ b/packages/ui/src/lib/i18n/messages/en.settings.ts @@ -718,8 +718,6 @@ export const settingsDict = { 'settings.skills.sidebar.toast.skillDeleted': 'Skill "{name}" deleted successfully', 'settings.skills.sidebar.toast.deleteSkillFailed': 'Failed to delete skill', 'settings.skills.sidebar.toast.duplicateLoadFailed': 'Failed to load skill details for duplication', - 'settings.skills.sidebar.toast.renameLoadFailed': 'Failed to load skill details', - 'settings.skills.sidebar.toast.removeOldAfterRenameFailed': 'Failed to remove old skill after rename', 'settings.skills.sidebar.toast.renameFailed': 'Failed to rename skill', 'settings.skills.sidebar.toast.skillRenamed': 'Skill renamed to "{name}"', 'settings.skills.sidebar.deleteDialog.title': 'Delete Skill', diff --git a/packages/ui/src/lib/i18n/messages/es.settings.ts b/packages/ui/src/lib/i18n/messages/es.settings.ts index 59b253e5..8ed3af1a 100644 --- a/packages/ui/src/lib/i18n/messages/es.settings.ts +++ b/packages/ui/src/lib/i18n/messages/es.settings.ts @@ -685,8 +685,6 @@ export const settingsDict = { "settings.skills.sidebar.toast.skillDeleted": "Habilidad \"{name}\" eliminada con éxito", "settings.skills.sidebar.toast.deleteSkillFailed": "No se pudo eliminar la habilidad", "settings.skills.sidebar.toast.duplicateLoadFailed": "No se pudo cargar la información de la habilidad para duplicarla", - "settings.skills.sidebar.toast.renameLoadFailed": "No se pudo cargar la información de la habilidad", - "settings.skills.sidebar.toast.removeOldAfterRenameFailed": "No se pudo eliminar la habilidad antigua después del cambio de nombre", "settings.skills.sidebar.toast.renameFailed": "No se pudo cambiar el nombre de la habilidad", "settings.skills.sidebar.toast.skillRenamed": "Habilidad renombrada a \"{name}\"", "settings.skills.sidebar.deleteDialog.title": "Eliminar habilidad", diff --git a/packages/ui/src/lib/i18n/messages/fr.settings.ts b/packages/ui/src/lib/i18n/messages/fr.settings.ts index aa8ef8e0..afb4cf1b 100644 --- a/packages/ui/src/lib/i18n/messages/fr.settings.ts +++ b/packages/ui/src/lib/i18n/messages/fr.settings.ts @@ -606,8 +606,6 @@ export const settingsDict = { 'settings.skills.sidebar.toast.skillDeleted': 'Skill "{name}" supprimé avec succès', 'settings.skills.sidebar.toast.deleteSkillFailed': 'Échec de la suppression du skill', 'settings.skills.sidebar.toast.duplicateLoadFailed': 'Échec du chargement des détails du skill pour la duplication', - 'settings.skills.sidebar.toast.renameLoadFailed': 'Échec du chargement des détails du skill', - 'settings.skills.sidebar.toast.removeOldAfterRenameFailed': 'Échec de la suppression de l\'ancien skill après le renommage', 'settings.skills.sidebar.toast.renameFailed': 'Échec du renommage du skill', 'settings.skills.sidebar.toast.skillRenamed': 'Skill renommé en "{name}"', 'settings.skills.sidebar.deleteDialog.title': 'Supprimer le skill', diff --git a/packages/ui/src/lib/i18n/messages/ja.settings.ts b/packages/ui/src/lib/i18n/messages/ja.settings.ts index 8e2e97de..b820e176 100644 --- a/packages/ui/src/lib/i18n/messages/ja.settings.ts +++ b/packages/ui/src/lib/i18n/messages/ja.settings.ts @@ -718,8 +718,6 @@ export const settingsDict = { 'settings.skills.sidebar.toast.skillDeleted': 'Skill "{name}" を削除しました', 'settings.skills.sidebar.toast.deleteSkillFailed': 'Skill の削除に失敗しました', 'settings.skills.sidebar.toast.duplicateLoadFailed': '複製用の Skill 詳細の読み込みに失敗しました', - 'settings.skills.sidebar.toast.renameLoadFailed': 'Skill 詳細の読み込みに失敗しました', - 'settings.skills.sidebar.toast.removeOldAfterRenameFailed': '名前変更後に古い Skill の削除に失敗しました', 'settings.skills.sidebar.toast.renameFailed': 'Skill の名前変更に失敗しました', 'settings.skills.sidebar.toast.skillRenamed': 'Skill の名前を「{name}」に変更しました', 'settings.skills.sidebar.deleteDialog.title': 'Skill を削除', diff --git a/packages/ui/src/lib/i18n/messages/ko.settings.ts b/packages/ui/src/lib/i18n/messages/ko.settings.ts index 6bb43de5..80bde6b0 100644 --- a/packages/ui/src/lib/i18n/messages/ko.settings.ts +++ b/packages/ui/src/lib/i18n/messages/ko.settings.ts @@ -685,8 +685,6 @@ export const settingsDict = { 'settings.skills.sidebar.toast.skillDeleted': '스킬 "{name}"을 삭제했습니다', 'settings.skills.sidebar.toast.deleteSkillFailed': '스킬을 삭제하지 못했습니다', 'settings.skills.sidebar.toast.duplicateLoadFailed': '복제를 위한 스킬 세부 정보를 로드하지 못했습니다', - 'settings.skills.sidebar.toast.renameLoadFailed': '스킬 세부 정보를 로드하지 못했습니다', - 'settings.skills.sidebar.toast.removeOldAfterRenameFailed': '이름 변경 후 이전 스킬을 제거하지 못했습니다', 'settings.skills.sidebar.toast.renameFailed': '스킬 이름을 변경하지 못했습니다', 'settings.skills.sidebar.toast.skillRenamed': '스킬 이름이 "{name}"(으)로 변경되었습니다', 'settings.skills.sidebar.deleteDialog.title': '스킬 삭제', diff --git a/packages/ui/src/lib/i18n/messages/pl.settings.ts b/packages/ui/src/lib/i18n/messages/pl.settings.ts index 27077db4..eb524ec0 100644 --- a/packages/ui/src/lib/i18n/messages/pl.settings.ts +++ b/packages/ui/src/lib/i18n/messages/pl.settings.ts @@ -1863,9 +1863,7 @@ export const settingsDict = { 'settings.skills.sidebar.title': 'Umiejętności', 'settings.skills.sidebar.toast.deleteSkillFailed': 'Nie udało się usunąć umiejętności', 'settings.skills.sidebar.toast.duplicateLoadFailed': 'Nie udało się załadować szczegółów umiejętności do duplikacji', - 'settings.skills.sidebar.toast.removeOldAfterRenameFailed': 'Nie udało się usunąć starej umiejętności po zmianie nazwy', 'settings.skills.sidebar.toast.renameFailed': 'Nie udało się zmienić nazwy umiejętności', - 'settings.skills.sidebar.toast.renameLoadFailed': 'Nie udało się załadować szczegółów umiejętności', 'settings.skills.sidebar.toast.skillDeleted': 'Umiejętność „{name}” została usunięta', 'settings.skills.sidebar.toast.skillRenamed': 'Zmieniono nazwę umiejętności na „{name}”', 'settings.skills.sidebar.total': 'Suma: {count}', diff --git a/packages/ui/src/lib/i18n/messages/pt-BR.settings.ts b/packages/ui/src/lib/i18n/messages/pt-BR.settings.ts index 55273e8a..d83e4afb 100644 --- a/packages/ui/src/lib/i18n/messages/pt-BR.settings.ts +++ b/packages/ui/src/lib/i18n/messages/pt-BR.settings.ts @@ -685,8 +685,6 @@ export const settingsDict = { "settings.skills.sidebar.toast.skillDeleted": "Habilidade \"{name}\" excluída com sucesso", "settings.skills.sidebar.toast.deleteSkillFailed": "Não foi possível excluir a habilidade", "settings.skills.sidebar.toast.duplicateLoadFailed": "Não foi possível carregar as informações da habilidade para duplicá-la", - "settings.skills.sidebar.toast.renameLoadFailed": "Não foi possível carregar as informações da habilidade", - "settings.skills.sidebar.toast.removeOldAfterRenameFailed": "Não foi possível excluir a habilidade antiga depois da renomeação", "settings.skills.sidebar.toast.renameFailed": "Não foi possível renomear da habilidade", "settings.skills.sidebar.toast.skillRenamed": "Habilidade renomeada para \"{name}\"", "settings.skills.sidebar.deleteDialog.title": "Excluir habilidade", diff --git a/packages/ui/src/lib/i18n/messages/uk.settings.ts b/packages/ui/src/lib/i18n/messages/uk.settings.ts index a32a8c01..0a64e6e3 100644 --- a/packages/ui/src/lib/i18n/messages/uk.settings.ts +++ b/packages/ui/src/lib/i18n/messages/uk.settings.ts @@ -685,8 +685,6 @@ export const settingsDict = { "settings.skills.sidebar.toast.skillDeleted": "Навичку \"{name}\" успішно видалено", "settings.skills.sidebar.toast.deleteSkillFailed": "Не вдалося видалити навичку", "settings.skills.sidebar.toast.duplicateLoadFailed": "Не вдалося завантажити деталі навичок для дублювання", - "settings.skills.sidebar.toast.renameLoadFailed": "Не вдалося завантажити деталі навичок", - "settings.skills.sidebar.toast.removeOldAfterRenameFailed": "Не вдалося видалити стару навичку після перейменування", "settings.skills.sidebar.toast.renameFailed": "Не вдалося перейменувати навичку", "settings.skills.sidebar.toast.skillRenamed": "Навичку перейменовано на \"{name}\"", "settings.skills.sidebar.deleteDialog.title": "Видалити навичку", diff --git a/packages/ui/src/lib/i18n/messages/zh-CN.settings.ts b/packages/ui/src/lib/i18n/messages/zh-CN.settings.ts index dc2c0b36..f906328d 100644 --- a/packages/ui/src/lib/i18n/messages/zh-CN.settings.ts +++ b/packages/ui/src/lib/i18n/messages/zh-CN.settings.ts @@ -685,8 +685,6 @@ export const settingsDict = { 'settings.skills.sidebar.toast.skillDeleted': '技能“{name}”已删除', 'settings.skills.sidebar.toast.deleteSkillFailed': '删除技能失败', 'settings.skills.sidebar.toast.duplicateLoadFailed': '加载技能详情以复制失败', - 'settings.skills.sidebar.toast.renameLoadFailed': '加载技能详情失败', - 'settings.skills.sidebar.toast.removeOldAfterRenameFailed': '重命名后移除旧技能失败', 'settings.skills.sidebar.toast.renameFailed': '重命名技能失败', 'settings.skills.sidebar.toast.skillRenamed': '技能已重命名为“{name}”', 'settings.skills.sidebar.deleteDialog.title': '删除技能', diff --git a/packages/ui/src/lib/i18n/messages/zh-TW.settings.ts b/packages/ui/src/lib/i18n/messages/zh-TW.settings.ts index 8d35e718..29563dee 100644 --- a/packages/ui/src/lib/i18n/messages/zh-TW.settings.ts +++ b/packages/ui/src/lib/i18n/messages/zh-TW.settings.ts @@ -682,8 +682,6 @@ 'settings.skills.sidebar.toast.skillDeleted': 'skill「{name}」已刪除', 'settings.skills.sidebar.toast.deleteSkillFailed': '刪除 skill 失敗', 'settings.skills.sidebar.toast.duplicateLoadFailed': '複製 skill 的詳細資訊載入失敗', - 'settings.skills.sidebar.toast.renameLoadFailed': '載入 skill 詳情失敗', - 'settings.skills.sidebar.toast.removeOldAfterRenameFailed': '重新命名後移除舊 skill 失敗', 'settings.skills.sidebar.toast.renameFailed': '重新命名 skill 失敗', 'settings.skills.sidebar.toast.skillRenamed': 'skill 已重新命名為「{name}」', 'settings.skills.sidebar.deleteDialog.title': '刪除 Skill', diff --git a/packages/vscode/src/opencodeConfig.ts b/packages/vscode/src/opencodeConfig.ts index c46da9eb..1ecce46e 100644 --- a/packages/vscode/src/opencodeConfig.ts +++ b/packages/vscode/src/opencodeConfig.ts @@ -2834,6 +2834,57 @@ export const deleteSkill = (skillName: string, workingDirectory?: string): void } }; +const isPathInside = (candidatePath: string, parentPath: string): boolean => { + const resolvedCandidate = path.resolve(candidatePath); + const resolvedParent = path.resolve(parentPath); + return resolvedCandidate === resolvedParent + || resolvedCandidate.startsWith(`${resolvedParent}${path.sep}`); +}; + +const getManagedSkillRoots = (workingDirectory?: string): string[] => { + const roots: string[] = []; + const pushRoot = (dir?: string | null) => { + 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; + pushRoot(customConfigDir ? path.join(customConfigDir, 'skills') : null); + pushRoot(customConfigDir ? path.join(customConfigDir, 'skill') : null); + + 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; +}; + +const isManagedSkillPath = (skillMdPath: string, workingDirectory?: string): boolean => { + if (!skillMdPath || skillMdPath === BUILT_IN_SKILL_LOCATION) { + return false; + } + const skillDir = path.dirname(path.resolve(skillMdPath)); + return getManagedSkillRoots(workingDirectory).some((root) => isPathInside(skillDir, root)); +}; + export const renameSkill = (oldName: string, newName: string, workingDirectory?: string): void => { ensureSkillDirs(); validateSkillName(newName); @@ -2852,6 +2903,17 @@ export const renameSkill = (oldName: string, newName: string, workingDirectory?: 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) { diff --git a/packages/web/server/lib/opencode/DOCUMENTATION.md b/packages/web/server/lib/opencode/DOCUMENTATION.md index 02088195..1f21f955 100644 --- a/packages/web/server/lib/opencode/DOCUMENTATION.md +++ b/packages/web/server/lib/opencode/DOCUMENTATION.md @@ -351,7 +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) + - 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`) - Skills catalog listing/source pagination, scan, and install routes - Supporting skill file read/write/delete routes diff --git a/packages/web/server/lib/opencode/skills.js b/packages/web/server/lib/opencode/skills.js index 5f1a53d2..a0594ad3 100644 --- a/packages/web/server/lib/opencode/skills.js +++ b/packages/web/server/lib/opencode/skills.js @@ -602,6 +602,60 @@ 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); @@ -620,6 +674,17 @@ function renameSkill(oldName, newName, workingDirectory) { 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) { diff --git a/packages/web/server/lib/opencode/skills.test.js b/packages/web/server/lib/opencode/skills.test.js index bda9ea97..6fec177e 100644 --- a/packages/web/server/lib/opencode/skills.test.js +++ b/packages/web/server/lib/opencode/skills.test.js @@ -178,4 +178,147 @@ describe('skills', () => { 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 unmanagedDir = path.join(projectRoot, 'custom-skills', 'unmanaged-skill'); + 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(unmanagedDir, { recursive: true }); + await fsPromises.writeFile( + path.join(unmanagedDir, 'SKILL.md'), + [ + '---', + 'name: unmanaged-skill', + 'description: Unmanaged', + '---', + '', + 'Unmanaged 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, + }); + } + }); });