Merge branch 'next' into kimi-runtime-support
This commit is contained in:
Colin Johnson
5
.changeset/gallant-lemurs-roar.md
Normal file
5
.changeset/gallant-lemurs-roar.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 929
|
||||
---
|
||||
`cmdSkillManifest` now discovers concrete skills nested under `gsd-ns-*` routers (`<root>/gsd-ns-<router>/skills/<stem>/SKILL.md`), so `gsd-health` and `gsd-settings` report the correct count on nested-layout runtimes (cline, qwen, hermes, augment, trae, antigravity). The scan is scoped to `gsd-ns-*` router dirs only — unrelated user dirs that happen to have a `skills/` subdirectory are not traversed. Dual-routed concretes (same skill installed under two routers) are deduped by name within each root. (#929)
|
||||
68
src/init.cts
68
src/init.cts
@@ -2146,18 +2146,26 @@ function buildSkillManifest(cwd: string, skillsDir: string | null = null): Skill
|
||||
continue;
|
||||
}
|
||||
|
||||
let skillCount = 0;
|
||||
for (const entry of entries) {
|
||||
if (!entry.isDirectory()) continue;
|
||||
|
||||
const skillMdPath = path.join(rootPath, entry.name, 'SKILL.md');
|
||||
const content = platformReadSync(skillMdPath);
|
||||
if (content === null) continue;
|
||||
// Track skill names seen within this root to deduplicate dual-routed concretes
|
||||
// (e.g. spec-phase nested under both gsd-ns-workflow and gsd-ns-manage).
|
||||
const seenNamesInRoot = new Set<string>();
|
||||
|
||||
function pushSkillEntry(
|
||||
// relPath must use forward slashes on all platforms (manifest paths are
|
||||
// posix-style for cross-platform stability; flat entries use template
|
||||
// literals that always produce '/'; nested entries are joined below
|
||||
// with explicit '/' separators rather than path.join).
|
||||
relPath: string,
|
||||
content: string,
|
||||
): boolean {
|
||||
const frontmatter = extractFrontmatter(content);
|
||||
const name = (frontmatter['name'] as string) || entry.name;
|
||||
const description = (frontmatter['description'] as string) || '';
|
||||
const dirPart = relPath.replace(/\/SKILL\.md$/, '');
|
||||
const stem = dirPart.includes('/') ? dirPart.split('/').pop()! : dirPart;
|
||||
const name = (frontmatter['name'] as string) || stem;
|
||||
if (seenNamesInRoot.has(name)) return false; // dedupe dual-routed concretes
|
||||
seenNamesInRoot.add(name);
|
||||
|
||||
const description = (frontmatter['description'] as string) || '';
|
||||
const triggers: string[] = [];
|
||||
const bodyMatch = content.match(/^---[\s\S]*?---\s*\n([\s\S]*)$/);
|
||||
if (bodyMatch) {
|
||||
@@ -2175,14 +2183,50 @@ function buildSkillManifest(cwd: string, skillsDir: string | null = null): Skill
|
||||
name,
|
||||
description,
|
||||
triggers,
|
||||
path: entry.name,
|
||||
file_path: `${entry.name}/SKILL.md`,
|
||||
path: dirPart,
|
||||
file_path: relPath,
|
||||
root: rootInfo.root,
|
||||
scope: rootInfo.scope,
|
||||
installed: rootInfo.scope !== 'import-only',
|
||||
deprecated: !!rootInfo.deprecated,
|
||||
});
|
||||
skillCount++;
|
||||
return true;
|
||||
}
|
||||
|
||||
let skillCount = 0;
|
||||
for (const entry of entries) {
|
||||
if (!entry.isDirectory()) continue;
|
||||
|
||||
const skillMdPath = path.join(rootPath, entry.name, 'SKILL.md');
|
||||
const content = platformReadSync(skillMdPath);
|
||||
if (content !== null) {
|
||||
if (pushSkillEntry(`${entry.name}/SKILL.md`, content)) skillCount++;
|
||||
}
|
||||
|
||||
// Nested layout: <entry>/skills/<stem>/SKILL.md
|
||||
// Used by cline, qwen, hermes, augment, trae, antigravity (#69 nested=true).
|
||||
// Descend exactly one level into <entry>/skills/ — no deeper recursion.
|
||||
// Scope to gsd-ns-* routers only: never vacuum up an unrelated user skill
|
||||
// that happens to have its own `skills/` subdirectory.
|
||||
if (!entry.name.startsWith('gsd-ns-')) continue;
|
||||
const nestedSkillsDir = path.join(rootPath, entry.name, 'skills');
|
||||
let nestedEntries: fs.Dirent[] = [];
|
||||
try {
|
||||
nestedEntries = fs.readdirSync(nestedSkillsDir, { withFileTypes: true });
|
||||
} catch {
|
||||
// No skills/ subdir — flat layout or unreadable; nothing to do.
|
||||
nestedEntries = [];
|
||||
}
|
||||
for (const nested of nestedEntries) {
|
||||
if (!nested.isDirectory()) continue;
|
||||
const nestedSkillMd = path.join(nestedSkillsDir, nested.name, 'SKILL.md');
|
||||
const nestedContent = platformReadSync(nestedSkillMd);
|
||||
if (nestedContent === null) continue;
|
||||
// Use forward-slash separator explicitly so manifest paths are posix-style
|
||||
// on all platforms, matching the flat-layout behaviour above.
|
||||
const relPath = `${entry.name}/skills/${nested.name}/SKILL.md`;
|
||||
if (pushSkillEntry(relPath, nestedContent)) skillCount++;
|
||||
}
|
||||
}
|
||||
|
||||
rootSummary.skill_count = skillCount;
|
||||
|
||||
@@ -148,4 +148,226 @@ describe('skill-manifest', () => {
|
||||
assert.strictEqual(claudeRoot.path, path.join(homeDir, 'claude-custom', 'skills'));
|
||||
assert.strictEqual(codexRoot.path, path.join(homeDir, 'codex-custom', 'skills'));
|
||||
});
|
||||
|
||||
// bug-929: nested layout discovery
|
||||
test('bug-929: discovers concrete skills nested under gsd-ns-* routers', () => {
|
||||
// Mirrors the on-disk shape that stageSkillsForRuntimeAsSkills emits for
|
||||
// cline/qwen/hermes/augment/trae/antigravity when nested=true:
|
||||
// <root>/gsd-ns-workflow/SKILL.md — router (top-level)
|
||||
// <root>/gsd-ns-workflow/skills/plan/SKILL.md — concrete
|
||||
// <root>/gsd-ns-workflow/skills/execute/SKILL.md — concrete
|
||||
// <root>/gsd-ns-workflow/skills/spec-phase/SKILL.md — dual-routed concrete
|
||||
// <root>/gsd-ns-manage/SKILL.md — router (top-level)
|
||||
// <root>/gsd-ns-manage/skills/progress/SKILL.md — concrete
|
||||
// <root>/gsd-ns-manage/skills/spec-phase/SKILL.md — same dual-routed concrete (dedupe by name)
|
||||
// <root>/gsd-standalone/SKILL.md — flat top-level skill (no skills/ subdir)
|
||||
const skillsDir = fs.mkdtempSync(path.join(require('os').tmpdir(), 'gsd-nested-skills-'));
|
||||
|
||||
function writeNestedSkill(dir, name, description) {
|
||||
fs.mkdirSync(dir, { recursive: true });
|
||||
fs.writeFileSync(path.join(dir, 'SKILL.md'), [
|
||||
'---',
|
||||
`name: ${name}`,
|
||||
`description: ${description}`,
|
||||
'---',
|
||||
'',
|
||||
`# ${name}`,
|
||||
].join('\n'));
|
||||
}
|
||||
|
||||
// Router 1: gsd-ns-workflow
|
||||
writeNestedSkill(path.join(skillsDir, 'gsd-ns-workflow'), 'gsd-ns-workflow', 'Workflow router');
|
||||
writeNestedSkill(path.join(skillsDir, 'gsd-ns-workflow', 'skills', 'plan'), 'gsd-plan', 'Plan skill');
|
||||
writeNestedSkill(path.join(skillsDir, 'gsd-ns-workflow', 'skills', 'execute'), 'gsd-execute', 'Execute skill');
|
||||
writeNestedSkill(path.join(skillsDir, 'gsd-ns-workflow', 'skills', 'spec-phase'), 'gsd-spec-phase', 'Spec phase skill');
|
||||
|
||||
// Router 2: gsd-ns-manage
|
||||
writeNestedSkill(path.join(skillsDir, 'gsd-ns-manage'), 'gsd-ns-manage', 'Manage router');
|
||||
writeNestedSkill(path.join(skillsDir, 'gsd-ns-manage', 'skills', 'progress'), 'gsd-progress', 'Progress skill');
|
||||
// Same spec-phase under a second router (dual-routed); must appear exactly once in manifest
|
||||
writeNestedSkill(path.join(skillsDir, 'gsd-ns-manage', 'skills', 'spec-phase'), 'gsd-spec-phase', 'Spec phase skill');
|
||||
|
||||
// Flat top-level skill (not a router, no skills/ subdir)
|
||||
writeNestedSkill(path.join(skillsDir, 'gsd-standalone'), 'gsd-standalone', 'Standalone flat skill');
|
||||
|
||||
const result = runGsdTools(['skill-manifest', '--skills-dir', skillsDir], tmpDir);
|
||||
assert.ok(result.success, `Command should succeed: ${result.error || result.output}`);
|
||||
|
||||
const manifest = JSON.parse(result.output);
|
||||
const skillNames = manifest.skills.map((s) => s.name).sort();
|
||||
|
||||
// 2 routers + 4 unique concretes (gsd-spec-phase deduped) + 1 flat = 7 total
|
||||
assert.deepStrictEqual(skillNames, [
|
||||
'gsd-execute',
|
||||
'gsd-ns-manage',
|
||||
'gsd-ns-workflow',
|
||||
'gsd-plan',
|
||||
'gsd-progress',
|
||||
'gsd-spec-phase',
|
||||
'gsd-standalone',
|
||||
]);
|
||||
assert.strictEqual(manifest.counts.skills, 7, 'dual-routed concrete must be deduped to one entry');
|
||||
|
||||
// Concrete skills should have a forward-slash nested file_path (posix-stable on all platforms)
|
||||
const planSkill = manifest.skills.find((s) => s.name === 'gsd-plan');
|
||||
assert.ok(planSkill, 'gsd-plan should be discovered');
|
||||
assert.ok(
|
||||
planSkill.file_path.includes('skills/plan'),
|
||||
`gsd-plan file_path should reflect nested location with forward slashes, got: ${planSkill.file_path}`
|
||||
);
|
||||
|
||||
// Router should also appear as a skill entry
|
||||
const routerSkill = manifest.skills.find((s) => s.name === 'gsd-ns-workflow');
|
||||
assert.ok(routerSkill, 'gsd-ns-workflow router should be discovered as a top-level skill');
|
||||
|
||||
cleanup(skillsDir);
|
||||
});
|
||||
|
||||
test('bug-929: discovers nested concretes even when router has no top-level SKILL.md', () => {
|
||||
// Edge case: a router dir has a skills/ subdir with concretes but no top-level SKILL.md.
|
||||
// The concrete skills should still be discovered.
|
||||
const skillsDir = fs.mkdtempSync(path.join(require('os').tmpdir(), 'gsd-router-only-skills-'));
|
||||
|
||||
// Router dir with skills/ but no SKILL.md of its own
|
||||
const concreteDir = path.join(skillsDir, 'gsd-ns-noroot', 'skills', 'orphan-skill');
|
||||
fs.mkdirSync(concreteDir, { recursive: true });
|
||||
fs.writeFileSync(path.join(concreteDir, 'SKILL.md'), [
|
||||
'---',
|
||||
'name: gsd-orphan',
|
||||
'description: Orphan skill under router without top-level SKILL.md',
|
||||
'---',
|
||||
'',
|
||||
'# gsd-orphan',
|
||||
].join('\n'));
|
||||
|
||||
const result = runGsdTools(['skill-manifest', '--skills-dir', skillsDir], tmpDir);
|
||||
assert.ok(result.success, `Command should succeed: ${result.error || result.output}`);
|
||||
|
||||
const manifest = JSON.parse(result.output);
|
||||
assert.deepStrictEqual(
|
||||
manifest.skills.map((s) => s.name).sort(),
|
||||
['gsd-orphan'],
|
||||
);
|
||||
assert.strictEqual(manifest.counts.skills, 1);
|
||||
|
||||
cleanup(skillsDir);
|
||||
});
|
||||
|
||||
test('bug-929: flat layout (no nested skills/ subdirs) still works correctly', () => {
|
||||
const skillsDir = fs.mkdtempSync(path.join(require('os').tmpdir(), 'gsd-flat-skills-'));
|
||||
|
||||
function writeFlat(name, description) {
|
||||
const dir = path.join(skillsDir, name);
|
||||
fs.mkdirSync(dir, { recursive: true });
|
||||
fs.writeFileSync(path.join(dir, 'SKILL.md'), [
|
||||
'---',
|
||||
`name: ${name}`,
|
||||
`description: ${description}`,
|
||||
'---',
|
||||
'',
|
||||
`# ${name}`,
|
||||
].join('\n'));
|
||||
}
|
||||
|
||||
writeFlat('gsd-alpha', 'Alpha skill');
|
||||
writeFlat('gsd-beta', 'Beta skill');
|
||||
writeFlat('gsd-gamma', 'Gamma skill');
|
||||
|
||||
const result = runGsdTools(['skill-manifest', '--skills-dir', skillsDir], tmpDir);
|
||||
assert.ok(result.success, `Command should succeed: ${result.error || result.output}`);
|
||||
|
||||
const manifest = JSON.parse(result.output);
|
||||
assert.deepStrictEqual(
|
||||
manifest.skills.map((s) => s.name).sort(),
|
||||
['gsd-alpha', 'gsd-beta', 'gsd-gamma']
|
||||
);
|
||||
assert.strictEqual(manifest.counts.skills, 3, 'flat layout count should be exact, no phantom nesting');
|
||||
|
||||
cleanup(skillsDir);
|
||||
});
|
||||
|
||||
test('bug-929: non-gsd-ns-* dirs with a skills/ subdir are NOT scanned (guard)', () => {
|
||||
// Regression guard for the `if (!entry.name.startsWith('gsd-ns-')) continue;` guard
|
||||
// in buildSkillManifest. A user tool dir like `my-tool/` that happens to have its
|
||||
// own `skills/` subdirectory must NOT have those skills vacuumed up.
|
||||
// Only `gsd-ns-<router>/skills/<stem>/SKILL.md` paths are in scope.
|
||||
const skillsDir = fs.mkdtempSync(path.join(require('os').tmpdir(), 'gsd-guard-test-'));
|
||||
|
||||
// Non-router dir with a flat SKILL.md at its own root — SHOULD be found (flat scan).
|
||||
const topLevelDir = path.join(skillsDir, 'my-tool');
|
||||
fs.mkdirSync(topLevelDir, { recursive: true });
|
||||
fs.writeFileSync(path.join(topLevelDir, 'SKILL.md'), [
|
||||
'---',
|
||||
'name: my-tool',
|
||||
'description: A user-defined top-level skill',
|
||||
'---',
|
||||
'',
|
||||
'# my-tool',
|
||||
].join('\n'));
|
||||
|
||||
// Non-router dir with a nested skills/ subdir — nested skills must NOT be discovered.
|
||||
const nestedDir = path.join(skillsDir, 'my-tool', 'skills', 'helper');
|
||||
fs.mkdirSync(nestedDir, { recursive: true });
|
||||
fs.writeFileSync(path.join(nestedDir, 'SKILL.md'), [
|
||||
'---',
|
||||
'name: my-tool-helper',
|
||||
'description: A nested skill that must not be vacuumed up',
|
||||
'---',
|
||||
'',
|
||||
'# my-tool-helper',
|
||||
].join('\n'));
|
||||
|
||||
// Another non-router dir (prefixed differently, could look router-like but isn't)
|
||||
const otherDir = path.join(skillsDir, 'gsd-settings');
|
||||
fs.mkdirSync(otherDir, { recursive: true });
|
||||
fs.writeFileSync(path.join(otherDir, 'SKILL.md'), [
|
||||
'---',
|
||||
'name: gsd-settings',
|
||||
'description: A flat gsd-* skill that is not a router',
|
||||
'---',
|
||||
'',
|
||||
'# gsd-settings',
|
||||
].join('\n'));
|
||||
// Give gsd-settings its own skills/ subdir — must not be traversed since it's not gsd-ns-*
|
||||
const otherNestedDir = path.join(skillsDir, 'gsd-settings', 'skills', 'subsetting');
|
||||
fs.mkdirSync(otherNestedDir, { recursive: true });
|
||||
fs.writeFileSync(path.join(otherNestedDir, 'SKILL.md'), [
|
||||
'---',
|
||||
'name: gsd-subsetting',
|
||||
'description: A nested skill that must not be vacuumed up',
|
||||
'---',
|
||||
'',
|
||||
'# gsd-subsetting',
|
||||
].join('\n'));
|
||||
|
||||
const result = runGsdTools(['skill-manifest', '--skills-dir', skillsDir], tmpDir);
|
||||
assert.ok(result.success, `Command should succeed: ${result.error || result.output}`);
|
||||
|
||||
const manifest = JSON.parse(result.output);
|
||||
const skillNames = manifest.skills.map((s) => s.name).sort();
|
||||
|
||||
// Only the flat top-level SKILL.md entries should be found; nested non-router skills are ignored
|
||||
assert.deepStrictEqual(
|
||||
skillNames,
|
||||
['gsd-settings', 'my-tool'],
|
||||
'nested skills under non-gsd-ns-* dirs must not be discovered',
|
||||
);
|
||||
assert.strictEqual(
|
||||
manifest.counts.skills,
|
||||
2,
|
||||
'only 2 top-level skills; nested non-router helpers must not inflate the count',
|
||||
);
|
||||
|
||||
// Confirm the forbidden names are absent
|
||||
assert.ok(
|
||||
!skillNames.includes('my-tool-helper'),
|
||||
'my-tool/skills/helper/SKILL.md must not appear (guard: my-tool is not gsd-ns-*)',
|
||||
);
|
||||
assert.ok(
|
||||
!skillNames.includes('gsd-subsetting'),
|
||||
'gsd-settings/skills/subsetting/SKILL.md must not appear (guard: gsd-settings is not gsd-ns-*)',
|
||||
);
|
||||
|
||||
cleanup(skillsDir);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user