From 5d3571597a80bea6c75320ec7877400b91db1999 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Tue, 1 Sep 2026 11:44:57 -0400 Subject: [PATCH] docs: address Copilot review round 6 on #2203 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The truncation guard rejected valid frontmatter. It assumed the only way a raw scalar can exceed its parsed value is a comment, but YAML also strips presentation syntax, so `description: © Some text` and `description: !!str Some text` were both reported as truncated — naming an unquoted `#` the value does not contain. The length check is now gated on an actual comment marker: `#` at the start of the scalar or preceded by whitespace, which is where YAML begins a comment. Anchors and tags fall out before the length is consulted. Regression cases cover both, and assert the genuine `#` case still fails so the fix cannot pass by disarming the guard. Also corrects the checklist, which still said an unquoted `#` lets every check pass. That predated the verifier; verify:skills rejects it now, and the sentence was telling authors the gate is blind to something it catches. No skill description or eval case changed, so skills:eval is unchanged from round 4's 63/63. local:gate passes, 34/34. 391/391 script tests. Signed-off-by: cliffhall --- docs/skill-authoring.md | 5 ++++- scripts/lib/skill-manifest.mjs | 8 ++++++++ scripts/lib/skill-manifest.test.mjs | 29 +++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/docs/skill-authoring.md b/docs/skill-authoring.md index 4e641e3a..c45515f0 100644 --- a/docs/skill-authoring.md +++ b/docs/skill-authoring.md @@ -234,7 +234,10 @@ break. description containing `:` or `#` is **quoted**. An unquoted colon loads the body with an _empty_ description, so `/name` still works while the skill can never auto-fire again; an unquoted `#` is worse-behaved, truncating the - description from that point on so it stays non-empty and every check passes. + description from that point on so it stays *non-empty* — which is why it went + unnoticed until `board-ops` lost both its board numbers. `verify:skills` now + rejects that truncation, so the gate catches it; quote the value rather than + relying on the check to tell you. 2. `disable-model-invocation` is declared explicitly; default it to `false`. 3. Description is action-first, with `Use when …; when …; when …`. 4. **If the skill is model-invoked**, `evals/evals.json` carries **at least diff --git a/scripts/lib/skill-manifest.mjs b/scripts/lib/skill-manifest.mjs index 691669b3..683dd89e 100644 --- a/scripts/lib/skill-manifest.mjs +++ b/scripts/lib/skill-manifest.mjs @@ -192,6 +192,14 @@ export function parseSkill(dirName, text) { const collapse = (s) => s.replace(/\s+/g, " ").trim(); const rawText = collapse(parts.join(" ")); const keptText = collapse(parsed); + // A raw scalar can legitimately be longer than the parsed value without any + // text being lost: YAML strips presentation syntax such as an anchor + // (`&name`) or a tag (`!!str`) from the value. Comparing lengths alone + // reported those as truncation, with a message blaming a `#` that is not + // there. Only look at the length when the scalar actually contains a comment + // marker — a `#` at the start or preceded by whitespace, which is precisely + // where YAML begins a comment (Copilot). + if (!/(^|\s)#/.test(rawText)) continue; if (rawText.length > keptText.length) { errors.push( "`" + diff --git a/scripts/lib/skill-manifest.test.mjs b/scripts/lib/skill-manifest.test.mjs index 826d35b4..c1e5f0a9 100644 --- a/scripts/lib/skill-manifest.test.mjs +++ b/scripts/lib/skill-manifest.test.mjs @@ -504,3 +504,32 @@ test("the truncation guard spans a multiline plain scalar", () => { assert.deepEqual(ok.errors, []); assert.equal(ok.description, "Covers boards and the option-deletion hazard."); }); + +test("the truncation guard ignores anchors and tags, which are not `#`", () => { + // YAML strips presentation syntax from the parsed value, so `&anchor` and + // `!!tag` make the raw scalar legitimately longer than what it parses to. + // A bare length comparison called those truncation and blamed a `#` that was + // nowhere in the text. + const mk = (value) => + [ + "---", + "name: alpha", + `description: ${value}`, + "disable-model-invocation: false", + "---", + "body", + ].join("\n"); + + const anchored = parseSkill("alpha", mk("© Some valid text.")); + assert.deepEqual(anchored.errors, []); + assert.equal(anchored.description, "Some valid text."); + + const tagged = parseSkill("alpha", mk("!!str Some tagged text.")); + assert.deepEqual(tagged.errors, []); + + // The real case must still fail, so the gate is not simply disarmed. + assert.match( + parseSkill("alpha", mk("Covers board #28 (v2).")).errors.join(" "), + /`description` is truncated/, + ); +});