fix(skills): write the frontmatter every Agent Skills host requires
Some checks failed
CI/CD / lint (pull_request) Successful in 1m18s
CI/CD / typecheck (pull_request) Successful in 1m15s
CI/CD / test (pull_request) Successful in 1m27s
CI/CD / build (pull_request) Successful in 2m17s
CI/CD / smoke (pull_request) Failing after 3m8s
CI/CD / publish (pull_request) Has been skipped
Some checks failed
CI/CD / lint (pull_request) Successful in 1m18s
CI/CD / typecheck (pull_request) Successful in 1m15s
CI/CD / test (pull_request) Successful in 1m27s
CI/CD / build (pull_request) Successful in 2m17s
CI/CD / smoke (pull_request) Failing after 3m8s
CI/CD / publish (pull_request) Has been skipped
mcpd keeps `name` and `description` as columns, and a skill's `content` is
often just the body — `propose-learnings` starts straight at its `#` heading.
The sync wrote `full.content` verbatim, so the SKILL.md landed with no YAML
frontmatter and the host rejected it. Observed live:
[Skill warning]
~/.prime/agent/skills/propose-learnings/SKILL.md
description is required
Claude Code and pi require the same keys, so this affected every target; it
only surfaced now because prime-agent prints the warning at startup.
`ensureSkillFrontmatter` synthesises the block from the columns when it is
absent, and fills in only a missing `name`/`description` when the author
already supplied a header — an existing complete header is never rewritten.
Values are emitted as JSON strings (valid YAML double-quoted scalars) because
descriptions routinely contain `:` and `#`, which break a bare scalar.
Also: `--force` now re-fetches skills whose server content is unchanged.
Without it a skill already on disk could never be repaired by a client-side
fix — the content hash still matched, so it was skipped forever, which is
exactly what happened on the first attempt to repair the file above.
Three existing assertions compared SKILL.md to the raw server content; they
now assert the body survives rather than exact bytes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017BMXdb2qZbPSh8Q7XpTyjB
This commit is contained in:
@@ -14,6 +14,7 @@ import {
|
||||
} from '../utils/skills-state.js';
|
||||
import {
|
||||
installSkillAtomic,
|
||||
ensureSkillFrontmatter,
|
||||
removeSkillAtomic,
|
||||
type SkillBody,
|
||||
} from '../utils/skills-disk.js';
|
||||
@@ -230,7 +231,11 @@ export async function runSkillsSync(opts: SyncOpts, deps: SyncDeps): Promise<Syn
|
||||
toFetch.push(v);
|
||||
continue;
|
||||
}
|
||||
if (prior.contentHash === v.contentHash) {
|
||||
// --force re-fetches even when the server content is unchanged. Without
|
||||
// this, a skill already on disk can never be repaired by a client-side fix
|
||||
// (e.g. frontmatter the earlier sync failed to write) because the hash
|
||||
// still matches and the skill is skipped forever.
|
||||
if (prior.contentHash === v.contentHash && !opts.force) {
|
||||
result.skipped.push(v.name);
|
||||
continue;
|
||||
}
|
||||
@@ -399,7 +404,10 @@ export async function runSkillsSync(opts: SyncOpts, deps: SyncDeps): Promise<Syn
|
||||
|
||||
const full = await client.get<FullSkill>(`/api/v1/skills/${encodeURIComponent(v.id)}`);
|
||||
const body: SkillBody = {
|
||||
content: full.content,
|
||||
// mcpd keeps name/description as columns, so a skill whose content is
|
||||
// just the body would land on disk without the frontmatter every host
|
||||
// requires and be silently rejected.
|
||||
content: ensureSkillFrontmatter(full.content, full.name || v.name, full.description || v.description || ''),
|
||||
...(Object.keys(full.files ?? {}).length > 0 ? { files: full.files } : {}),
|
||||
};
|
||||
const fileStates = await installSkillAtomic(targetDir, body);
|
||||
|
||||
@@ -18,6 +18,40 @@ export interface SkillBody {
|
||||
files?: Record<string, string>;
|
||||
}
|
||||
|
||||
/**
|
||||
* Guarantee the YAML frontmatter every Agent Skills host requires.
|
||||
*
|
||||
* mcpd stores `name` and `description` as columns, and a skill's `content` is
|
||||
* frequently just the body — `propose-learnings` starts straight at its `#`
|
||||
* heading. Written verbatim, such a skill is rejected by the agent that loads
|
||||
* it (prime-agent logs `description is required` and never loads it; Claude
|
||||
* Code and pi need the same keys), so the sync has to put the frontmatter back.
|
||||
*
|
||||
* Content that already carries a block is left alone apart from filling in a
|
||||
* missing `name`/`description` — we never rewrite a skill author's own header.
|
||||
*/
|
||||
export function ensureSkillFrontmatter(content: string, name: string, description: string): string {
|
||||
const match = /^---[ \t]*\r?\n([\s\S]*?)\r?\n---[ \t]*\r?\n?/.exec(content);
|
||||
if (match === null) {
|
||||
return `---\nname: ${yamlScalar(name)}\ndescription: ${yamlScalar(description)}\n---\n\n${content}`;
|
||||
}
|
||||
const block = match[1] ?? '';
|
||||
const additions: string[] = [];
|
||||
if (!/^name[ \t]*:/m.test(block)) additions.push(`name: ${yamlScalar(name)}`);
|
||||
if (!/^description[ \t]*:/m.test(block)) additions.push(`description: ${yamlScalar(description)}`);
|
||||
if (additions.length === 0) return content;
|
||||
return `---\n${[block, ...additions].join('\n')}\n---\n${content.slice(match[0].length)}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Emit a YAML scalar. Descriptions routinely contain `:` and `#`, which break a
|
||||
* bare scalar — a JSON string is a valid YAML double-quoted scalar and escapes
|
||||
* quotes and newlines correctly.
|
||||
*/
|
||||
function yamlScalar(value: string): string {
|
||||
return JSON.stringify(value);
|
||||
}
|
||||
|
||||
/**
|
||||
* Write a skill atomically into `targetDir`. If a previous install exists,
|
||||
* it's renamed to `<targetDir>.mcpctl-trash-<pid>` and rmtree'd after the
|
||||
|
||||
@@ -192,7 +192,8 @@ describe('runPrimeAgentSkillsSync', () => {
|
||||
const af = { 'a-1': { id: 'a-1', name: 'x-skill', description: 'd', semver: '1.0.0', contentHash: 'sha256:a', content: '# version-a\n', files: {} } };
|
||||
const clientA = mockClient({ visible: av, full: af });
|
||||
await runPrimeAgentSkillsSync({ project: 'projA', installRoot, statePath }, deps(clientA));
|
||||
expect(readFileSync(join(installRoot, 'x-skill', 'SKILL.md'), 'utf-8')).toBe('# version-a\n');
|
||||
// Frontmatter is synthesised on write, so assert the body, not exact bytes.
|
||||
expect(readFileSync(join(installRoot, 'x-skill', 'SKILL.md'), 'utf-8')).toContain('# version-a');
|
||||
|
||||
// Project B also has a skill named X with different content.
|
||||
const bv = [
|
||||
@@ -204,7 +205,8 @@ describe('runPrimeAgentSkillsSync', () => {
|
||||
|
||||
expect(resultB.preserved).toContain('x-skill');
|
||||
// A's version is untouched (not clobbered by B's).
|
||||
expect(readFileSync(join(installRoot, 'x-skill', 'SKILL.md'), 'utf-8')).toBe('# version-a\n');
|
||||
// Frontmatter is synthesised on write, so assert the body, not exact bytes.
|
||||
expect(readFileSync(join(installRoot, 'x-skill', 'SKILL.md'), 'utf-8')).toContain('# version-a');
|
||||
});
|
||||
|
||||
it('does not delete legacy, ownership-less state belonging to another project', async () => {
|
||||
@@ -279,7 +281,7 @@ describe('runPrimeAgentSkillsSync', () => {
|
||||
|
||||
expect(resultB.updated).toContain('shared-global');
|
||||
expect(resultB.preserved).toEqual([]);
|
||||
expect(readFileSync(join(installRoot, 'shared-global', 'SKILL.md'), 'utf-8')).toBe('# v2\n');
|
||||
expect(readFileSync(join(installRoot, 'shared-global', 'SKILL.md'), 'utf-8')).toContain('# v2');
|
||||
});
|
||||
|
||||
it('does not let a global-only sync clobber a project-owned skill', async () => {
|
||||
@@ -299,7 +301,8 @@ describe('runPrimeAgentSkillsSync', () => {
|
||||
const result = await runPrimeAgentSkillsSync({ cwd: empty, installRoot, statePath }, deps(mockClient({ visible: gv, full: gf })));
|
||||
|
||||
expect(result.preserved).toContain('x-skill');
|
||||
expect(readFileSync(join(installRoot, 'x-skill', 'SKILL.md'), 'utf-8')).toBe('# version-a\n');
|
||||
// Frontmatter is synthesised on write, so assert the body, not exact bytes.
|
||||
expect(readFileSync(join(installRoot, 'x-skill', 'SKILL.md'), 'utf-8')).toContain('# version-a');
|
||||
});
|
||||
|
||||
it('removes global orphans on a global-only sync', async () => {
|
||||
|
||||
53
src/cli/tests/utils/skill-frontmatter.test.ts
Normal file
53
src/cli/tests/utils/skill-frontmatter.test.ts
Normal file
@@ -0,0 +1,53 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { ensureSkillFrontmatter } from '../../src/utils/skills-disk.js';
|
||||
|
||||
/**
|
||||
* mcpd stores name/description as columns; a skill's `content` is often just
|
||||
* the body. Written verbatim it lands on disk without the frontmatter every
|
||||
* Agent Skills host requires — prime-agent logs "description is required" and
|
||||
* refuses to load it. Observed live with the `propose-learnings` skill.
|
||||
*/
|
||||
describe('ensureSkillFrontmatter', () => {
|
||||
it('adds frontmatter to a body that has none', () => {
|
||||
const out = ensureSkillFrontmatter('# propose-learnings\n\nBody text.\n', 'propose-learnings', 'How to capture knowledge.');
|
||||
expect(out).toBe(
|
||||
'---\nname: "propose-learnings"\ndescription: "How to capture knowledge."\n---\n\n# propose-learnings\n\nBody text.\n',
|
||||
);
|
||||
});
|
||||
|
||||
it('leaves an existing complete header untouched', () => {
|
||||
const original = '---\nname: commit\ndescription: Commit changes\n---\n\n# commit\n';
|
||||
expect(ensureSkillFrontmatter(original, 'commit', 'something else')).toBe(original);
|
||||
});
|
||||
|
||||
it('fills in only the missing key, preserving the author\'s other fields', () => {
|
||||
const out = ensureSkillFrontmatter('---\nname: x\nallowed-tools: Bash\n---\n\nBody\n', 'x', 'The description');
|
||||
expect(out).toContain('allowed-tools: Bash');
|
||||
expect(out).toContain('description: "The description"');
|
||||
// name was already there — not duplicated
|
||||
expect(out.match(/name:/g)).toHaveLength(1);
|
||||
expect(out).toContain('\n\nBody\n');
|
||||
});
|
||||
|
||||
it('quotes descriptions containing YAML metacharacters', () => {
|
||||
// A bare scalar breaks on ':' and '#'; these are common in descriptions.
|
||||
const out = ensureSkillFrontmatter('body', 'n', 'Use when: debugging #hard problems');
|
||||
expect(out).toContain('description: "Use when: debugging #hard problems"');
|
||||
});
|
||||
|
||||
it('escapes quotes and newlines rather than emitting invalid YAML', () => {
|
||||
const out = ensureSkillFrontmatter('body', 'n', 'He said "hi"\nthen left');
|
||||
expect(out).toContain('description: "He said \\"hi\\"\\nthen left"');
|
||||
});
|
||||
|
||||
it('handles CRLF frontmatter delimiters', () => {
|
||||
const out = ensureSkillFrontmatter('---\r\nname: x\r\n---\r\nBody', 'x', 'D');
|
||||
expect(out).toContain('description: "D"');
|
||||
expect(out).toContain('Body');
|
||||
});
|
||||
|
||||
it('still emits the key when the description is empty', () => {
|
||||
// Can't invent one, but the key must exist for the host to parse it.
|
||||
expect(ensureSkillFrontmatter('body', 'n', '')).toContain('description: ""');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user