golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(#1374): surface diagnostic when configured agent skills all fail to resolve (#1376)

Browse Source 
* fix(#1374): surface diagnostic when configured agent skills all fail to resolve

buildAgentSkillsBlock returned '' (only ad-hoc per-path stderr warnings) when an agent configured via agent_skills had paths that all failed to resolve — missing SKILL.md, unsafe path, invalid global name, OR a malformed (non-string/non-array) value. query agent-skills --json reported skills_count>0 with an empty block and no machine-readable signal, so a fully-dropped configuration was indistinguishable from a resolved one.

Thread an optional diagnostics collector through buildAgentSkillsBlock: route every skip warning through a warn() helper (stderr + collector), flag truthy-but-malformed config values, emit an aggregate warning when configured paths resolve to zero skills, and surface the collected reasons in a new warnings[] field on the query agent-skills --json IR. Empty arrays and falsy values stay silent (skills_count is honestly 0). skills_count semantics unchanged. Docs updated for the new IR field.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore(#1374): backfill changeset PR number (#1376)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-06-17 10:16:51 -04:00
committed by GitHub
parent 120f85164b
commit 66085d0080
4 changed files with 177 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
.changeset/wise-zebras-run.md Normal file
View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 1376
---
**Misconfigured agent skills no longer fail silently** — when an agent's configured `agent_skills` paths all fail to resolve (e.g. a missing `SKILL.md`), `gsd-tools query agent-skills` now emits an aggregate warning to stderr and adds a `warnings[]` field to its `--json` output, instead of returning an empty block with no signal. (#1376)

4
docs/CLI-TOOLS.md
View File

@@ -425,11 +425,11 @@ Emit the skill block for a given agent type.
# Emit raw XML skill block (default — safe for shell expansion)
node gsd-tools.cjs agent-skills <agent-type>
# Emit typed JSON surface (#455) — { agent_type, block, skills_count }
# Emit typed JSON surface (#455) — { agent_type, block, skills_count, warnings }
node gsd-tools.cjs agent-skills <agent-type> --json
```
The `--json` flag returns a typed IR object suitable for structured consumption and test assertions, while the default (no flag) preserves the raw XML output that workflow shell expansions rely on.
The `--json` flag returns a typed IR object suitable for structured consumption and test assertions, while the default (no flag) preserves the raw XML output that workflow shell expansions rely on. The IR also includes a `warnings` array naming any configured skill paths that were skipped (for example a missing `SKILL.md`); it is empty when every configured skill resolved.
---

48
src/init.cts
View File

@@ -1876,7 +1876,13 @@ function buildAgentSkillsBlock(
config: Record<string, unknown>,
agentType: string,
projectRoot: string,
diagnostics?: { warnings: string[] },
): string {
const warn = (message: string): void => {
process.stderr.write(message);
if (diagnostics) diagnostics.warnings.push(message.replace(/\n+$/, ''));
};
const runtime = (config && (config['runtime'] as string)) || 'claude';
const globalSkillsBase = getGlobalSkillsBase(runtime);
@@ -1886,7 +1892,13 @@ function buildAgentSkillsBlock(
if (!skillPaths) return '';
if (typeof skillPaths === 'string') skillPaths = [skillPaths];
if (!Array.isArray(skillPaths) || skillPaths.length === 0) return '';
if (!Array.isArray(skillPaths)) {
warn(
`[agent-skills] WARNING: Agent "${agentType}" has a malformed agent_skills value (expected string or array, got ${typeof skillPaths}) — ignoring\n`,
);
return '';
}
if (skillPaths.length === 0) return '';
// Hoist trusted roots computation before the loop: loadTrustedGlobalRoots does
// realpathSync I/O and should run at most once per call, not once per failing skill.
@@ -1898,12 +1910,15 @@ function buildAgentSkillsBlock(
// Skill-tool directive ({ kind: 'directive', name }) for plugin-provided namespaced skills.
const validEntries: Array<{ kind: 'include'; ref: string; display: string } | { kind: 'directive'; name: string }> = [];
for (const skillPath of skillPaths) {
if (typeof skillPath !== 'string') continue;
if (typeof skillPath !== 'string') {
warn(`[agent-skills] WARNING: Ignoring non-string skill entry (${typeof skillPath}) — skipping\n`);
continue;
}
if (skillPath.startsWith('global:')) {
const skillName = skillPath.slice(7);
if (!skillName) {
process.stderr.write(
warn(
`[agent-skills] WARNING: "global:" prefix with empty skill name — skipping\n`,
);
continue;
@@ -1911,7 +1926,7 @@ function buildAgentSkillsBlock(
// Accept: one or more [A-Za-z0-9_-]+ segments joined by single colons.
// Rejects: empty segments (::), leading/trailing colon, dots, slashes, backslashes.
if (!/^[A-Za-z0-9_-]+(:[A-Za-z0-9_-]+)*$/.test(skillName)) {
process.stderr.write(
warn(
`[agent-skills] WARNING: Invalid global skill name "${skillName}" — skipping\n`,
);
continue;
@@ -1923,7 +1938,7 @@ function buildAgentSkillsBlock(
// Emit a natural-language Skill-tool directive (not a @-include).
validEntries.push({ kind: 'directive', name: skillName });
} else {
process.stderr.write(
warn(
`[agent-skills] WARNING: Plugin-namespaced skill "global:${skillName}" requires a Skill-tool-capable runtime (claude) — skipping on runtime "${runtime}"\n`,
);
}
@@ -1931,7 +1946,7 @@ function buildAgentSkillsBlock(
}
// Non-namespaced bare name: attempt filesystem resolution as before.
if (globalSkillsBase === null) {
process.stderr.write(
warn(
`[agent-skills] WARNING: Runtime "${runtime}" does not use a skills directory — "global:${skillName}" is not supported on this runtime\n`,
);
continue;
@@ -1940,7 +1955,7 @@ function buildAgentSkillsBlock(
const globalSkillMd = path.join(globalSkillDir, 'SKILL.md');
const displayPath = getGlobalSkillDisplayPath(runtime, skillName);
if (!fs.existsSync(globalSkillMd)) {
process.stderr.write(
warn(
`[agent-skills] WARNING: Global skill not found at "${displayPath}/SKILL.md" — skipping\n`,
);
continue;
@@ -1952,11 +1967,13 @@ function buildAgentSkillsBlock(
return Boolean(rootCheck['safe']);
});
if (!acceptedViaTrustedRoot) {
process.stderr.write(
warn(
`[agent-skills] WARNING: Global skill "${skillName}" failed path check (symlink escape?) — skipping\n`,
);
continue;
}
// Intentionally a direct stderr write, NOT warn(): this is an acceptance
// trace, not a skip, so it must not land in the diagnostics warnings[].
process.stderr.write(`[agent-skills] NOTE: Global skill "${skillName}" accepted via trusted_global_roots (resolves outside the default skills dir)\n`);
}
validEntries.push({ kind: 'include', ref: `${globalSkillDir}/SKILL.md`, display: displayPath });
@@ -1965,7 +1982,7 @@ function buildAgentSkillsBlock(
const pathCheck = validatePath(skillPath, projectRoot) as unknown as Record<string, unknown>;
if (!pathCheck['safe']) {
process.stderr.write(
warn(
`[agent-skills] WARNING: Skipping unsafe path "${skillPath}": ${pathCheck['error'] as string}\n`,
);
continue;
@@ -1973,7 +1990,7 @@ function buildAgentSkillsBlock(
const skillMdPath = path.join(projectRoot, skillPath, 'SKILL.md');
if (!fs.existsSync(skillMdPath)) {
process.stderr.write(
warn(
`[agent-skills] WARNING: Skill not found at "${skillPath}/SKILL.md" — skipping\n`,
);
continue;
@@ -1982,7 +1999,12 @@ function buildAgentSkillsBlock(
validEntries.push({ kind: 'include', ref: `${skillPath}/SKILL.md`, display: skillPath });
}
if (validEntries.length === 0) return '';
if (validEntries.length === 0) {
warn(
`[agent-skills] WARNING: Agent "${agentType}" has ${skillPaths.length} configured skill path(s) but none resolved to a valid skill — all were skipped (see warnings above)\n`,
);
return '';
}
const lines = validEntries.map((entry) => {
if (entry.kind === 'directive') {
@@ -2005,10 +2027,12 @@ function cmdAgentSkills(
}
const config = loadConfig(cwd);
const diagnostics = { warnings: [] as string[] };
const block = buildAgentSkillsBlock(
config,
agentType,
cwd,
diagnostics,
);
if (jsonMode) {
@@ -2019,7 +2043,7 @@ function cmdAgentSkills(
: skillPaths
? [skillPaths]
: [];
output({ agent_type: agentType, block: block || '', skills_count: normalizedPaths.length }, raw);
output({ agent_type: agentType, block: block || '', skills_count: normalizedPaths.length, warnings: diagnostics.warnings }, raw);
return;
}

134
tests/agent-skills.test.cjs
View File

@@ -234,6 +234,140 @@ describe('agent-skills command', () => {
});
});
// ─── empty-resolution diagnostics (silent-drop visibility) ────────────────────
//
// When an agent is CONFIGURED with skill paths but every path fails to resolve
// (missing SKILL.md, unsafe path, invalid global name), buildAgentSkillsBlock
// previously returned '' with only per-path stderr warnings and no aggregate
// signal — so `query agent-skills --json` reported skills_count > 0 with an
// empty block and no indication the configured skills were dropped.
//
// Fix: emit an aggregate stderr WARNING when configured paths all resolve to
// zero skills, and surface every skip reason in a `warnings[]` field on the
// --json IR.
describe('agent-skills empty-resolution diagnostics', () => {
let tmpDir;
beforeEach(() => {
tmpDir = createTempProject();
});
afterEach(() => {
cleanup(tmpDir);
});
test('configured agent whose only skill is missing → warnings[] names the path and the aggregate drop', () => {
writeConfig(tmpDir, {
agent_skills: { 'gsd-phase-researcher': ['references/other-skill'] },
});
const r = runAgentSkillsJson(['agent-skills', 'gsd-phase-researcher'], tmpDir);
assert.ok(r.success, `Command failed: ${r.error}`);
assert.strictEqual(r.ir.block, '', 'block must be empty when the only configured skill is missing');
assert.strictEqual(r.ir.skills_count, 1, 'skills_count still reflects the configured path count');
assert.ok(Array.isArray(r.ir.warnings), 'IR must include a warnings array');
assert.ok(r.ir.warnings.length >= 1, `warnings must be non-empty, got: ${JSON.stringify(r.ir.warnings)}`);
assert.ok(
r.ir.warnings.some((w) => w.includes('references/other-skill')),
`warnings must name the skipped path, got: ${JSON.stringify(r.ir.warnings)}`,
);
assert.ok(
r.ir.warnings.some((w) => /none resolved to a valid skill/.test(w)),
`warnings must include the aggregate empty-resolution diagnostic, got: ${JSON.stringify(r.ir.warnings)}`,
);
});
test('configured agent with all skills missing → aggregate WARNING on stderr naming the agent', () => {
writeConfig(tmpDir, {
agent_skills: { 'gsd-planner': ['references/a', 'references/b'] },
});
const r = runGsdToolsWithStderr(['agent-skills', '--json', 'gsd-planner'], tmpDir, {
HOME: tmpDir,
USERPROFILE: tmpDir,
});
assert.ok(r.success, `Command failed (exit ${r.exitCode}): ${r.stderr}`);
assert.ok(
r.stderr.includes('[agent-skills] WARNING') &&
r.stderr.includes('gsd-planner') &&
r.stderr.includes('none resolved to a valid skill'),
`stderr must carry the aggregate empty-resolution warning naming the agent, got: ${r.stderr}`,
);
const ir = JSON.parse(r.stdout);
assert.strictEqual(ir.block, '');
assert.ok(ir.warnings.length >= 2, `warnings must list both skipped paths, got: ${JSON.stringify(ir.warnings)}`);
});
test('partial resolution: one valid + one missing → block present, NO aggregate warning, skipped path still listed', () => {
const skillDir = path.join(tmpDir, 'skills', 'present');
fs.mkdirSync(skillDir, { recursive: true });
fs.writeFileSync(path.join(skillDir, 'SKILL.md'), '# present\n');
writeConfig(tmpDir, {
agent_skills: { 'gsd-executor': ['skills/present', 'skills/absent'] },
});
const r = runAgentSkillsJson(['agent-skills', 'gsd-executor'], tmpDir);
assert.ok(r.success, `Command failed: ${r.error}`);
assert.ok(r.ir.block.includes('skills/present/SKILL.md'), 'block must include the resolvable skill');
assert.strictEqual(r.ir.skills_count, 2, 'skills_count reflects both configured paths');
assert.ok(Array.isArray(r.ir.warnings), 'IR must include a warnings array');
assert.ok(
r.ir.warnings.some((w) => w.includes('skills/absent')),
`warnings must list the one skipped path, got: ${JSON.stringify(r.ir.warnings)}`,
);
assert.ok(
!r.ir.warnings.some((w) => /none resolved to a valid skill/.test(w)),
`aggregate empty-resolution warning must NOT fire when at least one skill resolved, got: ${JSON.stringify(r.ir.warnings)}`,
);
});
test('all skills resolve → warnings[] is empty', () => {
const skillDir = path.join(tmpDir, 'skills', 'only');
fs.mkdirSync(skillDir, { recursive: true });
fs.writeFileSync(path.join(skillDir, 'SKILL.md'), '# only\n');
writeConfig(tmpDir, {
agent_skills: { 'gsd-executor': ['skills/only'] },
});
const r = runAgentSkillsJson(['agent-skills', 'gsd-executor'], tmpDir);
assert.ok(r.success, `Command failed: ${r.error}`);
assert.ok(r.ir.block.includes('skills/only/SKILL.md'), 'block must include the resolved skill');
assert.ok(Array.isArray(r.ir.warnings), 'IR must include a warnings array');
assert.strictEqual(r.ir.warnings.length, 0, `warnings must be empty when all skills resolve, got: ${JSON.stringify(r.ir.warnings)}`);
});
test('unconfigured agent → warnings[] empty (no skills configured is not a drop)', () => {
writeConfig(tmpDir, {
agent_skills: { 'gsd-executor': ['skills/whatever'] },
});
const r = runAgentSkillsJson(['agent-skills', 'gsd-planner'], tmpDir);
assert.ok(r.success, `Command failed: ${r.error}`);
assert.strictEqual(r.ir.block, '');
assert.ok(Array.isArray(r.ir.warnings), 'IR must include a warnings array');
assert.strictEqual(r.ir.warnings.length, 0, 'an agent with no configured skills is not a drop — warnings must be empty');
});
test('malformed (non-array, non-string) configured value → flagged in warnings[], not a silent drop', () => {
// A hand-edited config.json could carry a scalar instead of an array.
// cmdAgentSkills still counts it as a configured path, so it must be surfaced.
writeConfig(tmpDir, {
agent_skills: { 'gsd-executor': 42 },
});
const r = runAgentSkillsJson(['agent-skills', 'gsd-executor'], tmpDir);
assert.ok(r.success, `Command failed: ${r.error}`);
assert.strictEqual(r.ir.block, '', 'block must be empty for a malformed value');
assert.ok(Array.isArray(r.ir.warnings), 'IR must include a warnings array');
assert.ok(
r.ir.warnings.some((w) => /malformed agent_skills value/.test(w)),
`malformed scalar config must be flagged in warnings[], got: ${JSON.stringify(r.ir.warnings)}`,
);
});
});
// ─── config-ensure-section includes agent_skills ────────────────────────────
describe('config-ensure-section with agent_skills', () => {