fix: stop rejecting skills whose SKILL.md uses thematic breaks (#3297)

* fix: stop rejecting skills whose SKILL.md uses thematic breaks

The quality gate stripped frontmatter with a regex carrying the `m` flag, so
`^---` matched at every line start rather than only at the start of the
document. Frontmatter is optional when publishing, so a SKILL.md that opens
with a heading and uses `---` as an ordinary Markdown thematic break had
everything between its first two rules deleted before the body was measured.

The truncated body then fell under the word and character floors and the
publish was rejected outright with "Skill content is too thin or templated".
The same truncation also fed the structural fingerprint used for template-spam
detection.

The three other frontmatter parsers in the repository are all anchored to the
start of the document; this one is now consistent with them.

* fix: share canonical skill frontmatter parsing

---------

Co-authored-by: Patrick Erichsen <patrick.a.erichsen@gmail.com>
This commit is contained in:
Yiğit ERDOĞAN
2026-07-30 15:00:56 -07:00
committed by GitHub
co-authored by Patrick Erichsen
parent f491d5bb34
commit 3979883360
4 changed files with 204 additions and 10 deletions
+141
View File
@@ -0,0 +1,141 @@
import { describe, expect, it } from "vitest";
import { computeQualitySignals, evaluateQuality } from "./skillQuality";
// Frontmatter is optional when publishing a skill: `publishSkillVersion` takes
// the display name from its arguments and `parseFrontmatter` simply returns an
// empty record when the document has none. A SKILL.md may therefore open with a
// heading and use `---` purely as a Markdown thematic break.
const BODY_LINES = [
"## How it works",
"",
"The skill resolves a location from the incoming request and queries the",
"national weather service for the current forecast window. Responses are",
"normalized into a single unit system before rendering, so a caller never",
"has to reconcile mixed metric and imperial values inside one table.",
"",
"- Resolves the location from the request payload",
"- Calls the upstream forecast endpoint with a bounded timeout",
"- Normalizes temperature, wind speed and precipitation units",
"- Renders the result as a compact table for the agent to read",
"",
"## Configuration",
"",
"Set the upstream endpoint and the request timeout through environment",
"variables. Both carry defaults that work for local development.",
];
const THEMATIC_BREAK_README = [
"# Weather Report Skill",
"",
"---",
"",
...BODY_LINES,
"",
"---",
"",
"Rate limits apply.",
].join("\n");
const LEADING_THEMATIC_BREAK_README = [
"---",
"",
"# Weather Report Skill",
"",
...BODY_LINES,
"",
"---",
"",
"Rate limits apply.",
].join("\n");
const FRONTMATTER_README = [
"---",
"name: weather-report",
"description: Fetches forecasts from the national weather service.",
"---",
"",
"# Weather Report Skill",
"",
...BODY_LINES,
].join("\n");
const NO_FRONTMATTER_README = ["# Weather Report Skill", "", ...BODY_LINES].join("\n");
const SUMMARY = "Fetches forecasts and renders them as a normalized table.";
describe("computeQualitySignals", () => {
it("keeps the body of a frontmatter-less SKILL.md that uses thematic breaks", () => {
const signals = computeQualitySignals({
readmeText: THEMATIC_BREAK_README,
summary: SUMMARY,
});
expect(signals.headingCount).toBe(3);
expect(signals.bulletCount).toBe(4);
expect(signals.bodyWords).toBeGreaterThanOrEqual(80);
});
it("still strips real frontmatter", () => {
const withFrontmatter = computeQualitySignals({
readmeText: FRONTMATTER_README,
summary: SUMMARY,
});
const withoutFrontmatter = computeQualitySignals({
readmeText: NO_FRONTMATTER_README,
summary: SUMMARY,
});
expect(withFrontmatter.bodyWords).toBe(withoutFrontmatter.bodyWords);
expect(withFrontmatter.bodyChars).toBe(withoutFrontmatter.bodyChars);
});
it("strips real frontmatter with CR line endings", () => {
const withFrontmatter = computeQualitySignals({
readmeText: FRONTMATTER_README.replaceAll("\n", "\r"),
summary: SUMMARY,
});
const withoutFrontmatter = computeQualitySignals({
readmeText: NO_FRONTMATTER_README,
summary: SUMMARY,
});
expect(withFrontmatter).toMatchObject({
bodyWords: withoutFrontmatter.bodyWords,
bodyChars: withoutFrontmatter.bodyChars,
headingCount: withoutFrontmatter.headingCount,
bulletCount: withoutFrontmatter.bulletCount,
});
});
});
describe("evaluateQuality", () => {
it("does not reject a documented frontmatter-less skill from a new account", () => {
const signals = computeQualitySignals({
readmeText: THEMATIC_BREAK_README,
summary: SUMMARY,
});
const assessment = evaluateQuality({
signals,
trustTier: "low",
similarRecentCount: 0,
});
expect(assessment.decision).toBe("pass");
});
it("does not reject a frontmatter-less skill that begins with a thematic break", () => {
const signals = computeQualitySignals({
readmeText: LEADING_THEMATIC_BREAK_README,
summary: SUMMARY,
});
const assessment = evaluateQuality({
signals,
trustTier: "low",
similarRecentCount: 0,
});
expect(assessment.decision).toBe("pass");
});
});
+7 -1
View File
@@ -1,3 +1,5 @@
import { parseSkillMarkdown } from "./skills";
const TRUST_TIER_ACCOUNT_AGE_LOW_MS = 30 * 24 * 60 * 60 * 1000;
const TRUST_TIER_ACCOUNT_AGE_MEDIUM_MS = 90 * 24 * 60 * 60 * 1000;
const TRUST_TIER_SKILLS_LOW = 10;
@@ -36,8 +38,12 @@ export type QualityAssessment = {
signals: Omit<QualitySignals, "structuralFingerprint">;
};
// Anchored to the start of the document on purpose. Frontmatter is optional,
// and with the `m` flag `^` also matched at every line start, so a document
// with no frontmatter that used `---` as a Markdown thematic break lost
// everything between its first two rules.
function stripFrontmatter(raw: string) {
return raw.replace(/^---\s*\n[\s\S]*?\n---\s*\n?/m, "");
return parseSkillMarkdown(raw).body;
}
function tokenizeWords(text: string) {
+29
View File
@@ -7,6 +7,7 @@ import {
isMacJunkPath,
parseClawdisMetadata,
parseFrontmatter,
parseSkillMarkdown,
sanitizePath,
} from "./skills";
@@ -22,6 +23,34 @@ describe("skills utils", () => {
expect(parseFrontmatter("---\nname: demo\nBody without end")).toEqual({});
});
it.each(["\n", "\r\n", "\r"])("splits real frontmatter with %j line endings", (lineEnding) => {
const markdown = ["---", "name: demo", "---", "# Body"].join(lineEnding);
expect(parseSkillMarkdown(markdown)).toEqual({
frontmatter: { name: "demo" },
body: "# Body",
});
});
it("keeps a leading thematic break when its contents are not YAML frontmatter", () => {
const markdown = "---\n\n# Body\n\n---\n\nMore documentation.";
expect(parseSkillMarkdown(markdown)).toEqual({
frontmatter: {},
body: markdown,
});
});
it.each(["---\nname: [\n---\n# Body", "---\nname: demo\n# Body without a closing delimiter"])(
"keeps malformed or unterminated frontmatter as body",
(markdown) => {
expect(parseSkillMarkdown(markdown)).toEqual({
frontmatter: {},
body: markdown,
});
},
);
it("strips quotes in frontmatter values", () => {
const frontmatter = parseFrontmatter(`---\nname: "demo"\ndescription: 'Hello'\n---\nBody`);
expect(frontmatter.name).toBe("demo");
+27 -9
View File
@@ -9,9 +9,12 @@ import {
import { parse as parseYaml } from "yaml";
export type ParsedSkillFrontmatter = Record<string, unknown>;
export type ParsedSkillMarkdown = {
frontmatter: ParsedSkillFrontmatter;
body: string;
};
export type { ClawdisSkillMetadata, SkillInstallSpec };
const FRONTMATTER_START = "---";
const DEFAULT_EMBEDDING_MAX_CHARS = 12_000;
// Each OpenAI token maps to at least one UTF-8 byte; keep publish embeddings
// below the 8192-token model limit with conservative headroom.
@@ -19,27 +22,42 @@ const DEFAULT_EMBEDDING_MAX_BYTES = 7_500;
const encoder = new TextEncoder();
export function parseFrontmatter(content: string): ParsedSkillFrontmatter {
export function parseSkillMarkdown(content: string): ParsedSkillMarkdown {
const frontmatter: ParsedSkillFrontmatter = {};
const normalized = content.replace(/\r\n/g, "\n").replace(/\r/g, "\n");
if (!normalized.startsWith(FRONTMATTER_START)) return frontmatter;
const endIndex = normalized.indexOf(`\n${FRONTMATTER_START}`, 3);
if (endIndex === -1) return frontmatter;
const block = normalized.slice(4, endIndex);
const withoutFrontmatter = { frontmatter, body: normalized };
const lines = normalized.split("\n");
if (!isFrontmatterDelimiter(lines[0])) return withoutFrontmatter;
const closingLineIndex = lines.findIndex(
(line, index) => index > 0 && isFrontmatterDelimiter(line),
);
if (closingLineIndex === -1) return withoutFrontmatter;
const block = lines.slice(1, closingLineIndex).join("\n");
try {
const parsed = parseYaml(block) as unknown;
if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return frontmatter;
if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return withoutFrontmatter;
for (const [key, value] of Object.entries(parsed as Record<string, unknown>)) {
if (!/^[\w-]+$/.test(key)) continue;
const jsonValue = toJsonValue(value);
if (jsonValue !== undefined) frontmatter[key] = jsonValue;
}
} catch {
return frontmatter;
return withoutFrontmatter;
}
return frontmatter;
return {
frontmatter,
body: lines.slice(closingLineIndex + 1).join("\n"),
};
}
function isFrontmatterDelimiter(line: string | undefined) {
return line !== undefined && /^---[\t ]*$/.test(line);
}
export function parseFrontmatter(content: string): ParsedSkillFrontmatter {
return parseSkillMarkdown(content).frontmatter;
}
export function getFrontmatterValue(frontmatter: ParsedSkillFrontmatter, key: string) {