The SDK query handler `agent-skills` previously scanned every skill directory on the filesystem and returned a flat JSON list, ignoring `config.agent_skills[agentType]` entirely. Workflows that interpolate $(gsd-sdk query agent-skills <type>) into Task() prompts got a JSON dump of all skills instead of the documented <agent_skills> block. Port `buildAgentSkillsBlock` semantics from get-shit-done/bin/lib/init.cjs into the SDK handler: - Read config.agent_skills[agentType] via loadConfig() - Support single-string and array forms - Validate each project-relative path stays inside the project root (symlink-aware, mirrors security.cjs#validatePath) - Support `global:<name>` prefix for ~/.claude/skills/<name>/ - Skip entries whose SKILL.md is missing, with a stderr warning - Return the exact string block workflows embed: <agent_skills>\nRead these user-configured skills:\n- @.../SKILL.md\n</agent_skills> - Empty string when no agent type, no config, or nothing valid — matches gsd-tools.cjs cmdAgentSkills output.
This commit is contained in:
Tom Boucher
@@ -1,80 +1,123 @@
|
||||
/**
|
||||
* Tests for agent skills query handler.
|
||||
*
|
||||
* Verifies the handler reads `config.agent_skills[agentType]` from
|
||||
* `.planning/config.json` and returns the `<agent_skills>` XML block
|
||||
* workflows interpolate into Task() prompts (regression for #2555).
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||
import { mkdtemp, mkdir, rm, writeFile } from 'node:fs/promises';
|
||||
import { join } from 'node:path';
|
||||
import { tmpdir } from 'node:os';
|
||||
|
||||
import { agentSkills } from './skills.js';
|
||||
|
||||
function writeSkill(rootDir: string, name: string, description = 'Skill under test') {
|
||||
async function writeSkill(rootDir: string, name: string) {
|
||||
const skillDir = join(rootDir, name);
|
||||
return mkdir(skillDir, { recursive: true }).then(() => writeFile(join(skillDir, 'SKILL.md'), [
|
||||
'---',
|
||||
`name: ${name}`,
|
||||
`description: ${description}`,
|
||||
'---',
|
||||
'',
|
||||
`# ${name}`,
|
||||
].join('\n')));
|
||||
await mkdir(skillDir, { recursive: true });
|
||||
await writeFile(
|
||||
join(skillDir, 'SKILL.md'),
|
||||
`---\nname: ${name}\ndescription: test skill\n---\n\n# ${name}\n`,
|
||||
);
|
||||
}
|
||||
|
||||
async function writeConfig(projectDir: string, config: unknown) {
|
||||
await mkdir(join(projectDir, '.planning'), { recursive: true });
|
||||
await writeFile(join(projectDir, '.planning', 'config.json'), JSON.stringify(config, null, 2));
|
||||
}
|
||||
|
||||
describe('agentSkills', () => {
|
||||
let tmpDir: string;
|
||||
let homeDir: string;
|
||||
|
||||
it('returns empty string when no agent type (matches gsd-tools)', async () => {
|
||||
const r = await agentSkills([], tmpdir());
|
||||
expect(r.data).toBe('');
|
||||
});
|
||||
|
||||
beforeEach(async () => {
|
||||
tmpDir = await mkdtemp(join(tmpdir(), 'gsd-skills-'));
|
||||
homeDir = await mkdtemp(join(tmpdir(), 'gsd-skills-home-'));
|
||||
await writeSkill(join(tmpDir, '.cursor', 'skills'), 'my-skill');
|
||||
await writeSkill(join(tmpDir, '.codex', 'skills'), 'project-codex');
|
||||
await mkdir(join(tmpDir, '.claude', 'skills', 'orphaned-dir'), { recursive: true });
|
||||
await writeSkill(join(homeDir, '.claude', 'skills'), 'global-claude');
|
||||
await writeSkill(join(homeDir, '.codex', 'skills'), 'global-codex');
|
||||
await writeSkill(join(homeDir, '.claude', 'get-shit-done', 'skills'), 'legacy-import');
|
||||
vi.stubEnv('HOME', homeDir);
|
||||
// Windows `os.homedir()` reads USERPROFILE, not HOME
|
||||
vi.stubEnv('USERPROFILE', homeDir);
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
vi.unstubAllEnvs();
|
||||
await rm(tmpDir, { recursive: true, force: true });
|
||||
await rm(homeDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('returns deduped skill names from project and managed global skill dirs', async () => {
|
||||
const r = await agentSkills(['gsd-executor'], tmpDir);
|
||||
const data = r.data as Record<string, unknown>;
|
||||
const skills = data.skills as string[];
|
||||
|
||||
expect(skills).toEqual(expect.arrayContaining([
|
||||
'my-skill',
|
||||
'project-codex',
|
||||
'global-claude',
|
||||
'global-codex',
|
||||
]));
|
||||
expect(skills).not.toContain('orphaned-dir');
|
||||
expect(skills).not.toContain('legacy-import');
|
||||
expect(data.skill_count).toBe(skills.length);
|
||||
it('returns empty string when no agent type is provided', async () => {
|
||||
const r = await agentSkills([], tmpDir);
|
||||
expect(r.data).toBe('');
|
||||
});
|
||||
|
||||
it('counts deduped skill names when the same skill exists in multiple roots', async () => {
|
||||
await writeSkill(join(tmpDir, '.claude', 'skills'), 'shared-skill');
|
||||
await writeSkill(join(tmpDir, '.agents', 'skills'), 'shared-skill');
|
||||
it('returns empty string when project has no config', async () => {
|
||||
const r = await agentSkills(['gsd-planner'], tmpDir);
|
||||
expect(r.data).toBe('');
|
||||
});
|
||||
|
||||
const r = await agentSkills(['gsd-executor'], tmpDir);
|
||||
const data = r.data as Record<string, unknown>;
|
||||
const skills = data.skills as string[];
|
||||
it('returns empty string when agent type not in config.agent_skills', async () => {
|
||||
await writeConfig(tmpDir, { agent_skills: { 'gsd-executor': ['.claude/skills/foo'] } });
|
||||
const r = await agentSkills(['gsd-planner'], tmpDir);
|
||||
expect(r.data).toBe('');
|
||||
});
|
||||
|
||||
expect(skills.filter((skill) => skill === 'shared-skill')).toHaveLength(1);
|
||||
expect(data.skill_count).toBe(skills.length);
|
||||
it('returns <agent_skills> block for each configured skill path', async () => {
|
||||
await writeSkill(join(tmpDir, '.claude', 'skills'), 'skill-a');
|
||||
await writeSkill(join(tmpDir, '.claude', 'skills'), 'skill-b');
|
||||
await writeConfig(tmpDir, {
|
||||
agent_skills: {
|
||||
'gsd-planner': ['.claude/skills/skill-a', '.claude/skills/skill-b'],
|
||||
},
|
||||
});
|
||||
|
||||
const r = await agentSkills(['gsd-planner'], tmpDir);
|
||||
expect(r.data).toBe(
|
||||
'<agent_skills>\n' +
|
||||
'Read these user-configured skills:\n' +
|
||||
'- @.claude/skills/skill-a/SKILL.md\n' +
|
||||
'- @.claude/skills/skill-b/SKILL.md\n' +
|
||||
'</agent_skills>',
|
||||
);
|
||||
});
|
||||
|
||||
it('accepts a single string skill path (normalizes to array)', async () => {
|
||||
await writeSkill(join(tmpDir, '.claude', 'skills'), 'only-one');
|
||||
await writeConfig(tmpDir, {
|
||||
agent_skills: { 'gsd-planner': '.claude/skills/only-one' },
|
||||
});
|
||||
|
||||
const r = await agentSkills(['gsd-planner'], tmpDir);
|
||||
expect(r.data).toBe(
|
||||
'<agent_skills>\n' +
|
||||
'Read these user-configured skills:\n' +
|
||||
'- @.claude/skills/only-one/SKILL.md\n' +
|
||||
'</agent_skills>',
|
||||
);
|
||||
});
|
||||
|
||||
it('skips skills whose SKILL.md is missing', async () => {
|
||||
await writeSkill(join(tmpDir, '.claude', 'skills'), 'exists');
|
||||
await mkdir(join(tmpDir, '.claude', 'skills', 'missing-md'), { recursive: true });
|
||||
await writeConfig(tmpDir, {
|
||||
agent_skills: {
|
||||
'gsd-planner': ['.claude/skills/exists', '.claude/skills/missing-md'],
|
||||
},
|
||||
});
|
||||
|
||||
const r = await agentSkills(['gsd-planner'], tmpDir);
|
||||
expect(r.data).toBe(
|
||||
'<agent_skills>\n' +
|
||||
'Read these user-configured skills:\n' +
|
||||
'- @.claude/skills/exists/SKILL.md\n' +
|
||||
'</agent_skills>',
|
||||
);
|
||||
});
|
||||
|
||||
it('rejects path traversal escaping the project root', async () => {
|
||||
await writeConfig(tmpDir, {
|
||||
agent_skills: { 'gsd-planner': ['../evil-skill'] },
|
||||
});
|
||||
|
||||
const r = await agentSkills(['gsd-planner'], tmpDir);
|
||||
expect(r.data).toBe('');
|
||||
});
|
||||
|
||||
it('returns empty string when agent_skills value is an empty array', async () => {
|
||||
await writeConfig(tmpDir, { agent_skills: { 'gsd-planner': [] } });
|
||||
const r = await agentSkills(['gsd-planner'], tmpDir);
|
||||
expect(r.data).toBe('');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,26 +1,53 @@
|
||||
/**
|
||||
* Agent skills query handler — scan installed skill directories.
|
||||
* Agent skills query handler — read configured skills from `.planning/config.json`
|
||||
* and emit the `<agent_skills>` XML block workflows interpolate into Task() prompts.
|
||||
*
|
||||
* Reads from project `.claude/skills/`, `.agents/skills/`, `.cursor/skills/`,
|
||||
* `.github/skills/`, `.codex/skills/`, plus managed global `~/.claude/skills/`
|
||||
* and `~/.codex/skills/` roots.
|
||||
* Ports `buildAgentSkillsBlock` semantics from
|
||||
* `get-shit-done/bin/lib/init.cjs` so the SDK path honors
|
||||
* `config.agent_skills[agentType]` the same way the legacy
|
||||
* `gsd-tools.cjs agent-skills <type>` path does. Fixes #2555.
|
||||
*
|
||||
* @example
|
||||
* ```typescript
|
||||
* import { agentSkills } from './skills.js';
|
||||
*
|
||||
* await agentSkills(['gsd-executor'], '/project');
|
||||
* // { data: { agent_type: 'gsd-executor', skills: ['plan', 'verify'], skill_count: 2 } }
|
||||
* // With config.agent_skills = { "gsd-planner": [".claude/skills/demo-skill"] }
|
||||
* await agentSkills(['gsd-planner'], '/project');
|
||||
* // { data: '<agent_skills>\nRead these user-configured skills:\n- @.claude/skills/demo-skill/SKILL.md\n</agent_skills>' }
|
||||
*
|
||||
* // No agent type → empty string (matches gsd-tools cmdAgentSkills).
|
||||
* await agentSkills([], '/project');
|
||||
* // { data: '' } — matches gsd-tools when no agent type is passed
|
||||
* // { data: '' }
|
||||
* ```
|
||||
*/
|
||||
|
||||
import { existsSync, readdirSync } from 'node:fs';
|
||||
import { join } from 'node:path';
|
||||
import { existsSync, realpathSync } from 'node:fs';
|
||||
import { join, resolve, sep } from 'node:path';
|
||||
import { homedir } from 'node:os';
|
||||
|
||||
import type { QueryHandler } from './utils.js';
|
||||
import { loadConfig } from '../config.js';
|
||||
|
||||
const GLOBAL_SKILL_NAME_RE = /^[a-zA-Z0-9_-]+$/;
|
||||
|
||||
/**
|
||||
* Resolve `target` and ensure it stays inside `baseDir` after symlink resolution.
|
||||
* Mirrors the symlink-escape guard in `bin/lib/security.cjs#validatePath`.
|
||||
*/
|
||||
function resolveWithinBase(target: string, baseDir: string): string | null {
|
||||
try {
|
||||
const resolvedBase = existsSync(baseDir) ? realpathSync(baseDir) : resolve(baseDir);
|
||||
const absTarget = resolve(baseDir, target);
|
||||
const resolvedTarget = existsSync(absTarget) ? realpathSync(absTarget) : absTarget;
|
||||
const baseWithSep = resolvedBase.endsWith(sep) ? resolvedBase : resolvedBase + sep;
|
||||
if (resolvedTarget !== resolvedBase && !resolvedTarget.startsWith(baseWithSep)) {
|
||||
return null;
|
||||
}
|
||||
return resolvedTarget;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
export const agentSkills: QueryHandler = async (args, projectDir) => {
|
||||
const agentType = (args[0] || '').trim();
|
||||
@@ -28,35 +55,75 @@ export const agentSkills: QueryHandler = async (args, projectDir) => {
|
||||
if (!agentType) {
|
||||
return { data: '' };
|
||||
}
|
||||
const skillDirs = [
|
||||
join(projectDir, '.claude', 'skills'),
|
||||
join(projectDir, '.agents', 'skills'),
|
||||
join(projectDir, '.cursor', 'skills'),
|
||||
join(projectDir, '.github', 'skills'),
|
||||
join(projectDir, '.codex', 'skills'),
|
||||
join(homedir(), '.claude', 'skills'),
|
||||
join(homedir(), '.codex', 'skills'),
|
||||
];
|
||||
|
||||
const skills: string[] = [];
|
||||
for (const dir of skillDirs) {
|
||||
if (!existsSync(dir)) continue;
|
||||
try {
|
||||
const entries = readdirSync(dir, { withFileTypes: true });
|
||||
for (const entry of entries) {
|
||||
if (!entry.isDirectory()) continue;
|
||||
if (!existsSync(join(dir, entry.name, 'SKILL.md'))) continue;
|
||||
skills.push(entry.name);
|
||||
}
|
||||
} catch { /* skip */ }
|
||||
let config;
|
||||
try {
|
||||
config = await loadConfig(projectDir);
|
||||
} catch {
|
||||
return { data: '' };
|
||||
}
|
||||
|
||||
const dedupedSkills = [...new Set(skills)];
|
||||
const raw = config.agent_skills?.[agentType];
|
||||
if (!raw) return { data: '' };
|
||||
|
||||
let skillPaths: unknown[];
|
||||
if (typeof raw === 'string') {
|
||||
skillPaths = [raw];
|
||||
} else if (Array.isArray(raw)) {
|
||||
skillPaths = raw;
|
||||
} else {
|
||||
return { data: '' };
|
||||
}
|
||||
if (skillPaths.length === 0) return { data: '' };
|
||||
|
||||
const globalSkillsBase = join(homedir(), '.claude', 'skills');
|
||||
const validEntries: Array<{ ref: string }> = [];
|
||||
|
||||
for (const entry of skillPaths) {
|
||||
if (typeof entry !== 'string') continue;
|
||||
|
||||
// `global:<name>` — skill installed under ~/.claude/skills/<name>/ (#1992).
|
||||
if (entry.startsWith('global:')) {
|
||||
const skillName = entry.slice(7);
|
||||
if (!skillName) {
|
||||
process.stderr.write('[agent-skills] WARNING: "global:" prefix with empty skill name — skipping\n');
|
||||
continue;
|
||||
}
|
||||
if (!GLOBAL_SKILL_NAME_RE.test(skillName)) {
|
||||
process.stderr.write(`[agent-skills] WARNING: Invalid global skill name "${skillName}" — skipping\n`);
|
||||
continue;
|
||||
}
|
||||
const skillDir = join(globalSkillsBase, skillName);
|
||||
const skillMd = join(skillDir, 'SKILL.md');
|
||||
if (!existsSync(skillMd)) {
|
||||
process.stderr.write(`[agent-skills] WARNING: Global skill not found at "~/.claude/skills/${skillName}/SKILL.md" — skipping\n`);
|
||||
continue;
|
||||
}
|
||||
if (resolveWithinBase(skillMd, globalSkillsBase) === null) {
|
||||
process.stderr.write(`[agent-skills] WARNING: Global skill "${skillName}" failed path check (symlink escape?) — skipping\n`);
|
||||
continue;
|
||||
}
|
||||
validEntries.push({ ref: `~/.claude/skills/${skillName}/SKILL.md` });
|
||||
continue;
|
||||
}
|
||||
|
||||
// Project-relative path — must resolve within projectDir.
|
||||
if (resolveWithinBase(entry, projectDir) === null) {
|
||||
process.stderr.write(`[agent-skills] WARNING: Skipping unsafe path "${entry}"\n`);
|
||||
continue;
|
||||
}
|
||||
const skillMd = join(projectDir, entry, 'SKILL.md');
|
||||
if (!existsSync(skillMd)) {
|
||||
process.stderr.write(`[agent-skills] WARNING: Skill not found at "${entry}/SKILL.md" — skipping\n`);
|
||||
continue;
|
||||
}
|
||||
validEntries.push({ ref: `${entry}/SKILL.md` });
|
||||
}
|
||||
|
||||
if (validEntries.length === 0) return { data: '' };
|
||||
|
||||
const lines = validEntries.map((e) => `- @${e.ref}`).join('\n');
|
||||
return {
|
||||
data: {
|
||||
agent_type: agentType,
|
||||
skills: dedupedSkills,
|
||||
skill_count: dedupedSkills.length,
|
||||
},
|
||||
data: `<agent_skills>\nRead these user-configured skills:\n${lines}\n</agent_skills>`,
|
||||
};
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user