chore: address Copilot review round 11
- skill-manifest: accept CRLF on the opening fence. A Windows checkout with `core.autocrlf` has `---\r\n`, the fence is still the first line, and the authoritative validator accepts it — so `npm run validate` was failing a valid manifest, and failing it *before* that validator ever ran. The closing fence now strips a preceding CR too, so the YAML handed to the parser is clean. Regression cases for CRLF, mixed endings, and CRLF-with-no-body, plus cases proving the first-line rule and the whole-line terminator rule still hold with CRLF. Verified by mutation. - docs/quality-gate: `verify:skills` requires eval cases only from model-invoked skills; the seven name-only ones intentionally have none, and the summary read as an instruction to add them everywhere. - docs/quality-gate: the `test:scripts` inventory said four new suites; `claude-cli.test.mjs` makes five. Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
23c59d7a17
commit
97122b508b
2 changed files with 58 additions and 5 deletions
|
|
@ -46,17 +46,23 @@ const FRONTMATTER_FENCE = "---";
|
|||
export function splitFrontmatter(text) {
|
||||
// A BOM or a leading blank line means the fence is not the first line, and
|
||||
// Claude Code then treats the WHOLE file — `---` markers included — as body.
|
||||
if (!text.startsWith(FRONTMATTER_FENCE + "\n")) {
|
||||
//
|
||||
// CRLF is accepted on both fences. A Windows checkout with `core.autocrlf`
|
||||
// has `---\r\n` and the fence is still the first line, so rejecting it would
|
||||
// fail `npm run validate` on a valid manifest — before the authoritative
|
||||
// validator, which accepts platform line endings, ever ran (Copilot).
|
||||
const open = /^---[ \t]*\r?\n/.exec(text);
|
||||
if (open === null) {
|
||||
return {
|
||||
error:
|
||||
"frontmatter must open with `---` on the very first line (no BOM, no leading blank line)",
|
||||
};
|
||||
}
|
||||
const rest = text.slice(FRONTMATTER_FENCE.length + 1);
|
||||
const rest = text.slice(open[0].length);
|
||||
// The closing fence must occupy the WHOLE line. Matching a mere prefix would
|
||||
// accept `---oops` as a terminator, silently truncating the frontmatter to
|
||||
// whatever preceded it — the very failure this function exists to catch.
|
||||
const close = /\n---[ \t]*(\r?\n|$)/.exec(rest);
|
||||
const close = /\r?\n---[ \t]*(\r?\n|$)/.exec(rest);
|
||||
if (close === null)
|
||||
return { error: "frontmatter is never closed by a `---` line" };
|
||||
return {
|
||||
|
|
|
|||
|
|
@ -254,3 +254,50 @@ test("listingCost counts when_to_use too", () => {
|
|||
7,
|
||||
);
|
||||
});
|
||||
|
||||
test("a CRLF manifest is valid — the fence is still the first line", () => {
|
||||
// A Windows checkout with `core.autocrlf` produces these. Rejecting one would
|
||||
// fail `npm run validate` on a manifest the authoritative validator accepts,
|
||||
// and would fail it *before* that validator ever ran.
|
||||
const crlf =
|
||||
"---\r\nname: x\r\ndescription: d\r\ndisable-model-invocation: true\r\n---\r\n\r\nBody\r\n";
|
||||
assert.equal(
|
||||
splitFrontmatter(crlf).frontmatter,
|
||||
"name: x\r\ndescription: d\r\ndisable-model-invocation: true",
|
||||
);
|
||||
assert.deepEqual(parseSkill("x", crlf).errors, []);
|
||||
|
||||
// Mixed endings are still fine, and so is a CRLF file with no trailing body.
|
||||
assert.deepEqual(
|
||||
parseSkill(
|
||||
"x",
|
||||
"---\r\nname: x\ndescription: d\r\ndisable-model-invocation: true\n---\r\n",
|
||||
).errors,
|
||||
[],
|
||||
);
|
||||
assert.deepEqual(
|
||||
parseSkill(
|
||||
"x",
|
||||
"---\r\nname: x\r\ndescription: d\r\ndisable-model-invocation: true\r\n---",
|
||||
).errors,
|
||||
[],
|
||||
);
|
||||
});
|
||||
|
||||
test("CRLF does not weaken the first-line rule", () => {
|
||||
// The point of the opening check is that anything before the fence turns the
|
||||
// whole file into body — accepting CRLF must not start accepting those.
|
||||
for (const text of [
|
||||
"\r\n---\r\nname: x\r\n---\r\n",
|
||||
"---\r\nname: x\r\n---\r\n",
|
||||
"# Title\r\n---\r\nname: x\r\n---\r\n",
|
||||
"--- \r\nname: x\r\n---\r\n".replace("--- ", "---x"),
|
||||
]) {
|
||||
assert.match(splitFrontmatter(text).error ?? "", /first line|never closed/);
|
||||
}
|
||||
// And `---oops` is still not a terminator with CRLF endings either.
|
||||
assert.match(
|
||||
splitFrontmatter("---\r\nname: x\r\n---oops\r\nbody\r\n").error ?? "",
|
||||
/never closed/,
|
||||
);
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue