fix(skills): drive UI rename gating from server renamable flag
Expose authoritative renamable on skill list responses using the same managed-root policy as renameSkill, drop the divergent UI path heuristic, and remove an unused rejection-test fixture. Co-authored-by: Serhii Dziupin <makeittech@users.noreply.github.com>
This commit is contained in:
co-authored by
Serhii Dziupin
parent
bfea13ef1d
commit
20fc675af0
@@ -27,7 +27,6 @@ import { SidebarGroup } from '@/components/sections/shared/SidebarGroup';
|
|||||||
import { Icon } from "@/components/icon/Icon";
|
import { Icon } from "@/components/icon/Icon";
|
||||||
import { useI18n } from '@/lib/i18n';
|
import { useI18n } from '@/lib/i18n';
|
||||||
import { SETTINGS_PANEL_TITLE_CLASS } from '@/components/sections/shared/SettingsSection';
|
import { SETTINGS_PANEL_TITLE_CLASS } from '@/components/sections/shared/SettingsSection';
|
||||||
import { isManagedSkillFilesystemPath } from '@/components/sections/skills/skillLocations';
|
|
||||||
|
|
||||||
interface SkillsSidebarProps {
|
interface SkillsSidebarProps {
|
||||||
onItemSelect?: () => void;
|
onItemSelect?: () => void;
|
||||||
@@ -37,7 +36,7 @@ const BUILT_IN_SKILL_LOCATION = '<built-in>';
|
|||||||
|
|
||||||
const isBuiltInSkill = (skill: DiscoveredSkill | null | undefined): boolean => skill?.path === BUILT_IN_SKILL_LOCATION;
|
const isBuiltInSkill = (skill: DiscoveredSkill | null | undefined): boolean => skill?.path === BUILT_IN_SKILL_LOCATION;
|
||||||
const isRenamableSkill = (skill: DiscoveredSkill | null | undefined): boolean => (
|
const isRenamableSkill = (skill: DiscoveredSkill | null | undefined): boolean => (
|
||||||
!!skill && !isBuiltInSkill(skill) && isManagedSkillFilesystemPath(skill.path)
|
!!skill && !isBuiltInSkill(skill) && skill.renamable === true
|
||||||
);
|
);
|
||||||
|
|
||||||
export const SkillsSidebar: React.FC<SkillsSidebarProps> = ({ onItemSelect }) => {
|
export const SkillsSidebar: React.FC<SkillsSidebarProps> = ({ onItemSelect }) => {
|
||||||
|
|||||||
@@ -57,22 +57,3 @@ export function locationPartsFrom(value: SkillLocationValue): { scope: SkillScop
|
|||||||
}
|
}
|
||||||
return { scope: match.scope, source: match.source };
|
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 === '<built-in>') 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)
|
|
||||||
);
|
|
||||||
}
|
|
||||||
|
|||||||
@@ -70,6 +70,8 @@ export interface DiscoveredSkill {
|
|||||||
description?: string;
|
description?: string;
|
||||||
/** Domain folder parsed from file path, e.g. "automation-ai", "lark-ecosystem" */
|
/** Domain folder parsed from file path, e.g. "automation-ai", "lark-ecosystem" */
|
||||||
group?: string;
|
group?: string;
|
||||||
|
/** Authoritative server flag: skill lives under a managed root and can be renamed in place. */
|
||||||
|
renamable?: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Parse the domain group folder from a skill file path.
|
/** Parse the domain group folder from a skill file path.
|
||||||
@@ -93,6 +95,7 @@ interface RawSkillResponse {
|
|||||||
path: string;
|
path: string;
|
||||||
scope?: SkillScope;
|
scope?: SkillScope;
|
||||||
source?: SkillSource;
|
source?: SkillSource;
|
||||||
|
renamable?: boolean;
|
||||||
sources?: {
|
sources?: {
|
||||||
md?: {
|
md?: {
|
||||||
description?: string;
|
description?: string;
|
||||||
@@ -237,6 +240,7 @@ export const useSkillsStore = create<SkillsStore>()(
|
|||||||
source: s.source ?? 'opencode',
|
source: s.source ?? 'opencode',
|
||||||
description: s.sources?.md?.description || '',
|
description: s.sources?.md?.description || '',
|
||||||
group: parseSkillGroup(s.path),
|
group: parseSkillGroup(s.path),
|
||||||
|
renamable: s.renamable === true,
|
||||||
}));
|
}));
|
||||||
|
|
||||||
set({ skills: configSkills, isLoading: false });
|
set({ skills: configSkills, isLoading: false });
|
||||||
|
|||||||
@@ -26,6 +26,7 @@ import {
|
|||||||
updateSkill,
|
updateSkill,
|
||||||
deleteSkill,
|
deleteSkill,
|
||||||
renameSkill,
|
renameSkill,
|
||||||
|
isManagedSkillPath,
|
||||||
readSkillSupportingFile,
|
readSkillSupportingFile,
|
||||||
writeSkillSupportingFile,
|
writeSkillSupportingFile,
|
||||||
deleteSkillSupportingFile,
|
deleteSkillSupportingFile,
|
||||||
@@ -653,7 +654,21 @@ export async function handleConfigBridgeMessage(
|
|||||||
|
|
||||||
if (!name && normalizedMethod === 'GET') {
|
if (!name && normalizedMethod === 'GET') {
|
||||||
const skills = await resolveDiscoveredSkills(deps, ctx, workingDirectory);
|
const skills = await resolveDiscoveredSkills(deps, ctx, workingDirectory);
|
||||||
return { id, type, success: true, data: { skills } };
|
return {
|
||||||
|
id,
|
||||||
|
type,
|
||||||
|
success: true,
|
||||||
|
data: {
|
||||||
|
skills: skills.map((skill) => ({
|
||||||
|
...skill,
|
||||||
|
renamable: Boolean(
|
||||||
|
skill.path
|
||||||
|
&& skill.path !== '<built-in>'
|
||||||
|
&& isManagedSkillPath(skill.path, workingDirectory)
|
||||||
|
),
|
||||||
|
})),
|
||||||
|
},
|
||||||
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
const skillName = typeof name === 'string' ? name.trim() : '';
|
const skillName = typeof name === 'string' ? name.trim() : '';
|
||||||
|
|||||||
@@ -2885,6 +2885,8 @@ const isManagedSkillPath = (skillMdPath: string, workingDirectory?: string): boo
|
|||||||
return getManagedSkillRoots(workingDirectory).some((root) => isPathInside(skillDir, root));
|
return getManagedSkillRoots(workingDirectory).some((root) => isPathInside(skillDir, root));
|
||||||
};
|
};
|
||||||
|
|
||||||
|
export { isManagedSkillPath };
|
||||||
|
|
||||||
export const renameSkill = (oldName: string, newName: string, workingDirectory?: string): void => {
|
export const renameSkill = (oldName: string, newName: string, workingDirectory?: string): void => {
|
||||||
ensureSkillDirs();
|
ensureSkillDirs();
|
||||||
validateSkillName(newName);
|
validateSkillName(newName);
|
||||||
|
|||||||
@@ -352,6 +352,7 @@ an authoritative loopback callback URL even when OpenChamber binds port `0`.
|
|||||||
- `registerSkillRoutes(app, dependencies)`: registers skills-related routes:
|
- `registerSkillRoutes(app, dependencies)`: registers skills-related routes:
|
||||||
- Skills config CRUD and metadata under `/api/config/skills*`
|
- 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 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
|
- Skills catalog listing/source pagination, scan, and install routes
|
||||||
- Supporting skill file read/write/delete routes
|
- Supporting skill file read/write/delete routes
|
||||||
|
|
||||||
|
|||||||
@@ -38,7 +38,7 @@ import {
|
|||||||
decodePluginId,
|
decodePluginId,
|
||||||
} from './plugins.js';
|
} from './plugins.js';
|
||||||
import { SKILL_DIR, SKILL_SCOPE, readSkillSupportingFile, writeSkillSupportingFile, deleteSkillSupportingFile } from './shared.js';
|
import { SKILL_DIR, SKILL_SCOPE, readSkillSupportingFile, writeSkillSupportingFile, deleteSkillSupportingFile } from './shared.js';
|
||||||
import { getSkillSources, discoverSkills, mergeDiscoveredSkills, createSkill, updateSkill, deleteSkill, renameSkill } from './skills.js';
|
import { getSkillSources, discoverSkills, mergeDiscoveredSkills, createSkill, updateSkill, deleteSkill, renameSkill, isManagedSkillPath } from './skills.js';
|
||||||
import { getCuratedSkillsSources } from '../skills-catalog/curated-sources.js';
|
import { getCuratedSkillsSources } from '../skills-catalog/curated-sources.js';
|
||||||
import { getCacheKey, getCachedScan, setCachedScan } from '../skills-catalog/cache.js';
|
import { getCacheKey, getCachedScan, setCachedScan } from '../skills-catalog/cache.js';
|
||||||
import { isClawdHubSource, parseSkillRepoSource } from '../skills-catalog/source.js';
|
import { isClawdHubSource, parseSkillRepoSource } from '../skills-catalog/source.js';
|
||||||
@@ -257,6 +257,7 @@ export const createFeatureRoutesRuntime = (dependencies) => {
|
|||||||
updateSkill,
|
updateSkill,
|
||||||
deleteSkill,
|
deleteSkill,
|
||||||
renameSkill,
|
renameSkill,
|
||||||
|
isManagedSkillPath,
|
||||||
readSkillSupportingFile,
|
readSkillSupportingFile,
|
||||||
writeSkillSupportingFile,
|
writeSkillSupportingFile,
|
||||||
deleteSkillSupportingFile,
|
deleteSkillSupportingFile,
|
||||||
|
|||||||
@@ -22,6 +22,7 @@ export const registerSkillRoutes = (app, dependencies) => {
|
|||||||
updateSkill,
|
updateSkill,
|
||||||
deleteSkill,
|
deleteSkill,
|
||||||
renameSkill,
|
renameSkill,
|
||||||
|
isManagedSkillPath,
|
||||||
readSkillSupportingFile,
|
readSkillSupportingFile,
|
||||||
writeSkillSupportingFile,
|
writeSkillSupportingFile,
|
||||||
deleteSkillSupportingFile,
|
deleteSkillSupportingFile,
|
||||||
@@ -213,9 +214,15 @@ export const registerSkillRoutes = (app, dependencies) => {
|
|||||||
|
|
||||||
const enrichedSkills = skills.map((skill) => {
|
const enrichedSkills = skills.map((skill) => {
|
||||||
const sources = getSkillSources(skill.name, directory, skill);
|
const sources = getSkillSources(skill.name, directory, skill);
|
||||||
|
const skillPath = typeof skill.path === 'string' ? skill.path : null;
|
||||||
return {
|
return {
|
||||||
...skill,
|
...skill,
|
||||||
sources
|
sources,
|
||||||
|
renamable: Boolean(
|
||||||
|
skillPath
|
||||||
|
&& skillPath !== '<built-in>'
|
||||||
|
&& isManagedSkillPath(skillPath, directory)
|
||||||
|
),
|
||||||
};
|
};
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -734,4 +734,5 @@ export {
|
|||||||
updateSkill,
|
updateSkill,
|
||||||
deleteSkill,
|
deleteSkill,
|
||||||
renameSkill,
|
renameSkill,
|
||||||
|
isManagedSkillPath,
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -224,7 +224,6 @@ describe('skills', () => {
|
|||||||
const managedDir = path.join(projectRoot, '.opencode', 'skills', 'managed-skill');
|
const managedDir = path.join(projectRoot, '.opencode', 'skills', 'managed-skill');
|
||||||
const conflictDir = path.join(projectRoot, '.opencode', 'skills', 'taken-name');
|
const conflictDir = path.join(projectRoot, '.opencode', 'skills', 'taken-name');
|
||||||
const mismatchDir = path.join(projectRoot, '.opencode', 'skills', 'folder-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 cacheStamp = `oc-rename-${Date.now()}`;
|
||||||
const cacheDir = path.join(os.homedir(), '.cache', 'opencode', 'skills', cacheStamp, 'cache-skill');
|
const cacheDir = path.join(os.homedir(), '.cache', 'opencode', 'skills', cacheStamp, 'cache-skill');
|
||||||
|
|
||||||
@@ -274,21 +273,6 @@ describe('skills', () => {
|
|||||||
'utf8',
|
'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.mkdir(cacheDir, { recursive: true });
|
||||||
await fsPromises.writeFile(
|
await fsPromises.writeFile(
|
||||||
path.join(cacheDir, 'SKILL.md'),
|
path.join(cacheDir, 'SKILL.md'),
|
||||||
|
|||||||
Reference in New Issue
Block a user