Files
msd-core/sdk/src/query/skills.test.ts
Tom Boucher 397c34142a Deepen SDK package seam and converge runtime skills policy (#3238)
* Deepen SDK package seam and converge runtime skills policy

* fix(sdk): unified install-root resolution for workflows and agents (CR finding 1)

Use the already-resolved gsdInstallDir constant instead of calling
resolveLegacyInstallDir() again when computing agentsDir, ensuring
workflowsDir and agentsDir share the same install root.

* fix(sdk): tilde shortening requires path-boundary match (CR finding 2)

Both renderGlobalSkillsBaseDisplayPath and renderGlobalSkillDisplayPath
used startsWith(home) which could incorrectly shorten unrelated paths
sharing the same prefix. Now checks for home === base or
base.startsWith(home + sep) to ensure a real directory boundary.

* fix(sdk): validate loadConfig export before invocation (CR finding 3)

After requiring core.cjs, check typeof mod.loadConfig === 'function'
before calling it. Throws a classified GSDError with the module path
if the export is missing, rather than a generic TypeError.

* fix(test): guard root lookup before .path dereference (CR finding 4)

Added assert.ok() guards for claudeRoot and codexRoot after the .find()
calls so that a missing root produces an explicit assertion failure
rather than a TypeError on .path dereference.

* fix(ci): fail-safe on transient API errors in approval dismissal (CR finding 6)

resolveRole() returns 'unknown' for non-404 errors (rate limits, 5xx,
network blips). shouldDismissReviewer() now treats 'unknown' as
unresolvable and skips dismissal, preventing legitimate approvals from
being dismissed due to a transient API failure. Only 'none' (true 404)
is treated as a confirmed non-collaborator.

* changeset: pr=3238 SDK package seam and runtime skills convergence

* fix(sdk): harden resolveGlobalSkillDir against path traversal (CR finding 1)

Use resolve+relative to validate that skillName cannot escape the global
skills base directory. Values like "../../foo" or absolute paths now
return null instead of joining directly. All imports (resolve, relative,
isAbsolute) were already present in helpers.ts.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(sdk): split skill-dir-resolution and skill-not-found warnings (CR finding 2)

After resolveGlobalSkillDir's hardening can return null for traversal
attempts, the old single-branch warning "Global skill not found at ..."
was misleading. Split into two distinct cases:
- skillDir === null → "Could not resolve global skill directory for ..."
- skillMd missing → "Global skill not found at ..."

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* test: lock skill path-traversal rejection in resolveGlobalSkillDir

Regression test verifying that traversal segments (../../foo, ../escape),
empty string, and absolute paths are all rejected (return null), while
a legitimate skill name resolves correctly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* test(sdk): align display-path contract + traversal coverage for resolveGlobalSkillMarkdownPath (CR nitpicks)

- renderGlobalSkillsBaseDisplayPath now returns a non-null string for
  unsupported runtimes (e.g. cline → "(cline does not use a skills directory)")
  matching the existing renderGlobalSkillDisplayPath contract; callers
  of both helpers no longer need null-checks for unsupported runtimes.
- Remove now-redundant ! non-null assertion on renderGlobalSkillsBaseDisplayPath
  calls in skill-manifest.ts (return type is string, not string | null).
- Extend the path-traversal test block to assert resolveGlobalSkillMarkdownPath
  also propagates null for ../../foo, ../escape, empty, and /abs/path inputs,
  locking the null-propagation contract against future refactors.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-08 09:06:43 -04:00

235 lines
8.2 KiB
TypeScript

/**
* 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 } from 'vitest';
import { mkdtemp, mkdir, rm, writeFile } from 'node:fs/promises';
import { execSync } from 'node:child_process';
import { join, resolve } from 'node:path';
import { tmpdir, homedir } from 'node:os';
import { fileURLToPath } from 'node:url';
import { agentSkills } from './skills.js';
const CLI = resolve(fileURLToPath(import.meta.url), '../../../dist/cli.js');
async function writeSkill(rootDir: string, name: string) {
const skillDir = join(rootDir, name);
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;
beforeEach(async () => {
tmpDir = await mkdtemp(join(tmpdir(), 'gsd-skills-'));
});
afterEach(async () => {
await rm(tmpDir, { recursive: true, force: true });
});
it('returns empty string when no agent type is provided', async () => {
const r = await agentSkills([], tmpDir);
expect(r.data).toBe('');
});
it('returns empty string when project has no config', async () => {
const r = await agentSkills(['gsd-planner'], tmpDir);
expect(r.data).toBe('');
});
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('');
});
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('');
});
it('signals format:"text" for non-empty blocks (used by CLI dispatcher)', async () => {
await writeSkill(join(tmpDir, '.claude', 'skills'), 'a-skill');
await writeConfig(tmpDir, {
agent_skills: { 'gsd-planner': '.claude/skills/a-skill' },
});
const r = await agentSkills(['gsd-planner'], tmpDir);
expect(r.format).toBe('text');
});
it('resolves global: skills through runtime-aware global skills dir policy', async () => {
const codexHome = join(tmpDir, 'codex-home');
await writeSkill(join(codexHome, 'skills'), 'global-skill');
await writeConfig(tmpDir, {
runtime: 'codex',
agent_skills: { 'gsd-planner': ['global:global-skill'] },
});
const prevCodexHome = process.env.CODEX_HOME;
process.env.CODEX_HOME = codexHome;
try {
const r = await agentSkills(['gsd-planner'], tmpDir);
expect(r.data).toBe(
'<agent_skills>\n' +
'Read these user-configured skills:\n' +
`- @${join(codexHome, 'skills', 'global-skill', 'SKILL.md')}\n` +
'</agent_skills>',
);
expect(r.format).toBe('text');
} finally {
if (prevCodexHome === undefined) delete process.env.CODEX_HOME;
else process.env.CODEX_HOME = prevCodexHome;
}
});
it('returns empty string for global: skills on runtimes without a skills dir', async () => {
await writeConfig(tmpDir, {
runtime: 'cline',
agent_skills: { 'gsd-planner': ['global:nope'] },
});
const r = await agentSkills(['gsd-planner'], tmpDir);
expect(r.data).toBe('');
expect(r.format).toBeUndefined();
});
it('does not signal format:"text" for empty result', async () => {
const r = await agentSkills(['gsd-planner'], tmpDir);
expect(r.format).toBeUndefined();
});
});
// ─── CLI stdout integration ─────────────────────────────────────────────────
// Regression guard for the JSON-wrapping bug (#2914): the CLI must emit the
// raw <agent_skills> block to stdout, not a JSON-quoted string. Spawns the
// CLI as a child process so the full dispatch path (including cli.ts format
// handling) is exercised.
describe('agentSkills CLI stdout', () => {
let tmpDir: string;
beforeEach(async () => {
tmpDir = await mkdtemp(join(tmpdir(), 'gsd-skills-cli-'));
});
afterEach(async () => {
await rm(tmpDir, { recursive: true, force: true });
});
it('writes raw <agent_skills> block to stdout — not JSON-wrapped', async () => {
const skillDir = join(tmpDir, '.claude', 'skills', 'cli-skill');
await mkdir(skillDir, { recursive: true });
await writeFile(join(skillDir, 'SKILL.md'), '# cli-skill\n');
await mkdir(join(tmpDir, '.planning'), { recursive: true });
await writeFile(
join(tmpDir, '.planning', 'config.json'),
JSON.stringify({ agent_skills: { 'gsd-planner': '.claude/skills/cli-skill' } }),
);
const stdout = execSync(
`node "${CLI}" query --project-dir "${tmpDir}" agent-skills gsd-planner`,
{ encoding: 'utf-8' },
);
expect(stdout).toBe(
'<agent_skills>\nRead these user-configured skills:\n- @.claude/skills/cli-skill/SKILL.md\n</agent_skills>\n',
);
});
it('emits empty output (no JSON null) when agent type is unmapped', async () => {
await mkdir(join(tmpDir, '.planning'), { recursive: true });
await writeFile(
join(tmpDir, '.planning', 'config.json'),
JSON.stringify({ agent_skills: { 'gsd-executor': ['.claude/skills/foo'] } }),
);
const stdout = execSync(
`node "${CLI}" query --project-dir "${tmpDir}" agent-skills gsd-planner`,
{ encoding: 'utf-8' },
);
// Unmapped agent → empty string → CLI falls through to JSON (""), not raw
// text. This is acceptable: workflows that embed an empty var are no-ops.
// The important invariant is that a MAPPED agent never gets JSON-wrapped.
expect(stdout.trim()).toBe('""');
});
});