fix installed skills discovery and improve editor UX (#1296)
* fix skills discovery from opencode * Fix stale skill description after frontmatter removal * fix: align vscode skill discovery parity
This commit is contained in:
@@ -161,6 +161,7 @@ export const createFeatureRoutesRuntime = (dependencies) => {
|
||||
const {
|
||||
getSkillSources,
|
||||
discoverSkills,
|
||||
mergeDiscoveredSkills,
|
||||
createSkill,
|
||||
updateSkill,
|
||||
deleteSkill,
|
||||
@@ -201,6 +202,7 @@ export const createFeatureRoutesRuntime = (dependencies) => {
|
||||
getOpenCodePort,
|
||||
getSkillSources,
|
||||
discoverSkills,
|
||||
mergeDiscoveredSkills,
|
||||
createSkill,
|
||||
updateSkill,
|
||||
deleteSkill,
|
||||
|
||||
@@ -35,6 +35,7 @@ export {
|
||||
getSkillSources,
|
||||
getSkillScope,
|
||||
discoverSkills,
|
||||
mergeDiscoveredSkills,
|
||||
createSkill,
|
||||
updateSkill,
|
||||
deleteSkill,
|
||||
|
||||
@@ -16,6 +16,8 @@ export const registerSkillRoutes = (app, dependencies) => {
|
||||
getOpenCodeAuthHeaders,
|
||||
getOpenCodePort,
|
||||
getSkillSources,
|
||||
discoverSkills,
|
||||
mergeDiscoveredSkills,
|
||||
createSkill,
|
||||
updateSkill,
|
||||
deleteSkill,
|
||||
@@ -139,17 +141,32 @@ export const registerSkillRoutes = (app, dependencies) => {
|
||||
const name = typeof item?.name === 'string' ? item.name.trim() : '';
|
||||
const location = typeof item?.location === 'string' ? item.location : '';
|
||||
const description = typeof item?.description === 'string' ? item.description : '';
|
||||
if (!name || !location || location === '<built-in>') {
|
||||
const content = typeof item?.content === 'string' ? item.content : '';
|
||||
if (!name || !location) {
|
||||
return null;
|
||||
}
|
||||
if (location === '<built-in>') {
|
||||
return {
|
||||
name,
|
||||
path: location,
|
||||
scope: SKILL_SCOPE.USER,
|
||||
source: 'opencode',
|
||||
description,
|
||||
content,
|
||||
};
|
||||
}
|
||||
const inferred = inferSkillScopeAndSourceFromPath(location, workingDirectory);
|
||||
return {
|
||||
const skill = {
|
||||
name,
|
||||
path: location,
|
||||
scope: inferred.scope,
|
||||
source: inferred.source,
|
||||
description,
|
||||
};
|
||||
if (content) {
|
||||
skill.content = content;
|
||||
}
|
||||
return skill;
|
||||
})
|
||||
.filter(Boolean);
|
||||
} catch (error) {
|
||||
@@ -189,7 +206,9 @@ export const registerSkillRoutes = (app, dependencies) => {
|
||||
if (error) {
|
||||
return res.status(400).json({ error });
|
||||
}
|
||||
const skills = await fetchOpenCodeDiscoveredSkills(directory);
|
||||
const openCodeSkills = await fetchOpenCodeDiscoveredSkills(directory);
|
||||
const localSkills = discoverSkills(directory);
|
||||
const skills = mergeDiscoveredSkills(openCodeSkills, localSkills);
|
||||
|
||||
const enrichedSkills = skills.map((skill) => {
|
||||
const sources = getSkillSources(skill.name, directory, skill);
|
||||
@@ -271,8 +290,11 @@ export const registerSkillRoutes = (app, dependencies) => {
|
||||
return res.status(404).json({ ok: false, error: { kind: 'invalidSource', message: 'Unknown source' } });
|
||||
}
|
||||
|
||||
const discovered = await fetchOpenCodeDiscoveredSkills(directory);
|
||||
const installedByName = new Map(discovered.map((s) => [s.name, s]));
|
||||
const resolvedDiscovered = mergeDiscoveredSkills(
|
||||
await fetchOpenCodeDiscoveredSkills(directory),
|
||||
discoverSkills(directory),
|
||||
);
|
||||
const installedByName = new Map(resolvedDiscovered.map((s) => [s.name, s]));
|
||||
|
||||
if (src.sourceType === 'clawdhub' || isClawdHubSource(src.source)) {
|
||||
const scanned = await scanClawdHubPage({ cursor: cursor || null });
|
||||
|
||||
@@ -21,6 +21,8 @@ import {
|
||||
findWorktreeRoot,
|
||||
} from './shared.js';
|
||||
|
||||
const BUILT_IN_SKILL_LOCATION = '<built-in>';
|
||||
|
||||
function ensureProjectSkillDir(workingDirectory) {
|
||||
const projectSkillDir = path.join(workingDirectory, '.opencode', 'skills');
|
||||
if (!fs.existsSync(projectSkillDir)) {
|
||||
@@ -236,7 +238,39 @@ function discoverSkills(workingDirectory) {
|
||||
return Array.from(skills.values());
|
||||
}
|
||||
|
||||
function mergeDiscoveredSkills(primarySkills = [], fallbackSkills = []) {
|
||||
const merged = [];
|
||||
const seenNames = new Set();
|
||||
|
||||
const appendSkill = (skill) => {
|
||||
const name = typeof skill?.name === 'string' ? skill.name.trim() : '';
|
||||
if (!name || seenNames.has(name)) {
|
||||
return;
|
||||
}
|
||||
seenNames.add(name);
|
||||
merged.push(skill);
|
||||
};
|
||||
|
||||
for (const skill of primarySkills || []) {
|
||||
appendSkill(skill);
|
||||
}
|
||||
for (const skill of fallbackSkills || []) {
|
||||
appendSkill(skill);
|
||||
}
|
||||
|
||||
return merged;
|
||||
}
|
||||
|
||||
function getSkillSources(skillName, workingDirectory, discoveredSkill = null) {
|
||||
const isReadableFile = (filePath) => {
|
||||
if (!filePath) return false;
|
||||
try {
|
||||
return fs.statSync(filePath).isFile();
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
};
|
||||
|
||||
const projectPath = workingDirectory ? getProjectSkillPath(workingDirectory, skillName) : null;
|
||||
const projectExists = projectPath && fs.existsSync(projectPath);
|
||||
const projectDir = projectExists ? path.dirname(projectPath) : null;
|
||||
@@ -259,17 +293,33 @@ function getSkillSources(skillName, workingDirectory, discoveredSkill = null) {
|
||||
const matchedDiscovered = discoveredSkill && discoveredSkill.name === skillName
|
||||
? discoveredSkill
|
||||
: discoverSkills(workingDirectory).find((skill) => skill.name === skillName);
|
||||
const discoveredDescription =
|
||||
matchedDiscovered && typeof matchedDiscovered.description === 'string'
|
||||
? matchedDiscovered.description
|
||||
: '';
|
||||
const discoveredContent =
|
||||
matchedDiscovered && typeof matchedDiscovered.content === 'string'
|
||||
? matchedDiscovered.content
|
||||
: '';
|
||||
const discoveredPath =
|
||||
matchedDiscovered && typeof matchedDiscovered.path === 'string'
|
||||
? matchedDiscovered.path
|
||||
: null;
|
||||
const isBuiltInDiscovered = discoveredPath === BUILT_IN_SKILL_LOCATION;
|
||||
|
||||
let mdPath = null;
|
||||
let mdScope = null;
|
||||
let mdSource = null;
|
||||
let mdDir = null;
|
||||
|
||||
if (matchedDiscovered?.path) {
|
||||
mdPath = matchedDiscovered.path;
|
||||
if (isBuiltInDiscovered) {
|
||||
mdScope = matchedDiscovered.scope || SKILL_SCOPE.USER;
|
||||
mdSource = matchedDiscovered.source || 'opencode';
|
||||
} else if (discoveredPath) {
|
||||
mdPath = discoveredPath;
|
||||
mdScope = matchedDiscovered.scope || null;
|
||||
mdSource = matchedDiscovered.source || null;
|
||||
mdDir = path.dirname(matchedDiscovered.path);
|
||||
mdDir = isReadableFile(discoveredPath) ? path.dirname(discoveredPath) : null;
|
||||
} else if (projectExists) {
|
||||
mdPath = projectPath;
|
||||
mdScope = SKILL_SCOPE.PROJECT;
|
||||
@@ -297,10 +347,12 @@ function getSkillSources(skillName, workingDirectory, discoveredSkill = null) {
|
||||
mdDir = userAgentsDir;
|
||||
}
|
||||
|
||||
const mdExists = !!mdPath && fs.existsSync(mdPath);
|
||||
const mdExists = isBuiltInDiscovered || isReadableFile(mdPath);
|
||||
if (!mdExists) {
|
||||
mdPath = null;
|
||||
mdDir = null;
|
||||
mdScope = null;
|
||||
mdSource = null;
|
||||
}
|
||||
|
||||
const sources = {
|
||||
@@ -310,8 +362,11 @@ function getSkillSources(skillName, workingDirectory, discoveredSkill = null) {
|
||||
dir: mdDir,
|
||||
scope: mdScope,
|
||||
source: mdSource,
|
||||
fields: [],
|
||||
supportingFiles: []
|
||||
fields: isBuiltInDiscovered ? ['description', 'instructions'] : [],
|
||||
supportingFiles: [],
|
||||
name: matchedDiscovered?.name || skillName,
|
||||
description: discoveredDescription,
|
||||
instructions: isBuiltInDiscovered ? discoveredContent : ''
|
||||
},
|
||||
projectMd: {
|
||||
exists: projectExists,
|
||||
@@ -542,6 +597,7 @@ export {
|
||||
getSkillScope,
|
||||
getSkillWritePath,
|
||||
discoverSkills,
|
||||
mergeDiscoveredSkills,
|
||||
createSkill,
|
||||
updateSkill,
|
||||
deleteSkill,
|
||||
|
||||
@@ -0,0 +1,113 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import fsPromises from 'fs/promises';
|
||||
import os from 'os';
|
||||
import path from 'path';
|
||||
import { getSkillSources, mergeDiscoveredSkills } from './skills.js';
|
||||
|
||||
describe('skills', () => {
|
||||
it('merges locally discovered skills missing from OpenCode live discovery', () => {
|
||||
const merged = mergeDiscoveredSkills(
|
||||
[
|
||||
{ name: 'existing-opencode-skill', path: '/home/jkker/.config/opencode/skills/existing-opencode-skill/SKILL.md', source: 'opencode' },
|
||||
{ name: 'existing-agent-skill', path: '/home/jkker/.agents/skills/existing-agent-skill/SKILL.md', source: 'agents' },
|
||||
],
|
||||
[
|
||||
{ name: 'existing-agent-skill', path: '/home/jkker/.agents/skills/existing-agent-skill/SKILL.md', source: 'agents' },
|
||||
{ name: 'new-agent-skill', path: '/home/jkker/.agents/skills/new-agent-skill/SKILL.md', source: 'agents' },
|
||||
],
|
||||
);
|
||||
|
||||
expect(merged.map((skill) => skill.name)).toEqual([
|
||||
'existing-opencode-skill',
|
||||
'existing-agent-skill',
|
||||
'new-agent-skill',
|
||||
]);
|
||||
});
|
||||
|
||||
it('resolves built-in OpenCode skill content without parsing virtual locations as files', () => {
|
||||
const sources = getSkillSources(
|
||||
'customize-opencode',
|
||||
'/tmp/openchamber-skills-test-missing-project',
|
||||
{
|
||||
name: 'customize-opencode',
|
||||
path: '<built-in>',
|
||||
scope: 'user',
|
||||
source: 'opencode',
|
||||
description: 'Customize opencode',
|
||||
content: '# Customizing opencode\n\nUse this skill when updating config.',
|
||||
},
|
||||
);
|
||||
|
||||
expect(sources.md.exists).toBe(true);
|
||||
expect(sources.md.path).toBe(null);
|
||||
expect(sources.md.dir).toBe(null);
|
||||
expect(sources.md.scope).toBe('user');
|
||||
expect(sources.md.source).toBe('opencode');
|
||||
expect(sources.md.description).toBe('Customize opencode');
|
||||
expect(sources.md.instructions).toBe('# Customizing opencode\n\nUse this skill when updating config.');
|
||||
expect(sources.md.fields).toEqual(['description', 'instructions']);
|
||||
});
|
||||
|
||||
it('clears file metadata when a discovered skill path is unreadable', () => {
|
||||
const missingPath = path.join(os.tmpdir(), 'openchamber-skills-test-missing-file', 'SKILL.md');
|
||||
const sources = getSkillSources(
|
||||
'missing-agent-skill',
|
||||
'/tmp/openchamber-skills-test-missing-project',
|
||||
{
|
||||
name: 'missing-agent-skill',
|
||||
path: missingPath,
|
||||
scope: 'user',
|
||||
source: 'agents',
|
||||
description: 'Missing skill',
|
||||
},
|
||||
);
|
||||
|
||||
expect(sources.md.exists).toBe(false);
|
||||
expect(sources.md.path).toBe(null);
|
||||
expect(sources.md.dir).toBe(null);
|
||||
expect(sources.md.scope).toBe(null);
|
||||
expect(sources.md.source).toBe(null);
|
||||
expect(sources.md.description).toBe('Missing skill');
|
||||
expect(sources.md.instructions).toBe('');
|
||||
});
|
||||
|
||||
it('enriches discovered skills when their location is a real markdown file', async () => {
|
||||
const tempRoot = await fsPromises.mkdtemp(path.join(os.tmpdir(), 'oc-skills-'));
|
||||
const skillDir = path.join(tempRoot, 'example-skill');
|
||||
const skillPath = path.join(skillDir, 'SKILL.md');
|
||||
|
||||
try {
|
||||
await fsPromises.mkdir(skillDir, { recursive: true });
|
||||
await fsPromises.writeFile(
|
||||
skillPath,
|
||||
[
|
||||
'---',
|
||||
'name: example-skill',
|
||||
'description: Example from agents',
|
||||
'---',
|
||||
'',
|
||||
'Use this skill for examples.',
|
||||
'',
|
||||
].join('\n'),
|
||||
'utf8',
|
||||
);
|
||||
|
||||
const sources = getSkillSources('example-skill', tempRoot, {
|
||||
name: 'example-skill',
|
||||
path: skillPath,
|
||||
scope: 'user',
|
||||
source: 'agents',
|
||||
description: 'Fallback description',
|
||||
});
|
||||
|
||||
expect(sources.md.exists).toBe(true);
|
||||
expect(sources.md.path).toBe(skillPath);
|
||||
expect(sources.md.scope).toBe('user');
|
||||
expect(sources.md.source).toBe('agents');
|
||||
expect(sources.md.description).toBe('Example from agents');
|
||||
expect(sources.md.instructions).toBe('Use this skill for examples.');
|
||||
} finally {
|
||||
await fsPromises.rm(tempRoot, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user