docs: address Copilot review round 9 on #2203

Restrict the truncation guard to PLAIN scalars. `yaml` attaches a legitimate
trailing comment to quoted and block scalars too, so keying on `.comment` alone
rejected valid frontmatter — `description: "Complete value" # note` was reported
as truncated, telling the author to quote a value that was already quoted. The
regex this replaced tested the same thing by reading the first character; moving
to the node dropped the check rather than encoding it. Verified all three
shapes (double-quoted, single-quoted, block) against the parser, and added the
regression cases.

Also correct the `--max-turns 1` bullet in docs/skill-authoring.md, which
claimed the skill must be the model's first tool CALL. `collectSkillInvocations`
scans every tool_use block in the assistant event without regard to order, so a
turn calling Read and Skill together scores a hit. What actually loses the
sample is needing a tool RESULT first — that decision lands in turn two, which
never runs. The practical advice is unchanged; the mechanism now matches the
collector.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SyBPs7caoMcyvnp9ArMBhV
Signed-off-by: cliffhall <cliff@futurescale.com>

docs/skill-authoring.md

@@ -16,9 +16,15 @@ the `Skill` tool fired with the expected name.
 
 Four properties of that harness drive everything below:
 
-- **`--max-turns 1`.** The skill must be the model's **first** tool call. If it
-  opens a file first — even on its way to a perfect answer — the sample is a
-  miss. This is the single most important fact about writing cases.
+- **`--max-turns 1`.** The skill must fire in the model's **first assistant
+  turn**. Ordering _within_ that turn does not matter — `collectSkillInvocations`
+  scans every `tool_use` block in the event, so a turn that calls `Read` and
+  `Skill` together still scores a hit. What loses the sample is needing a tool
+  *result* first: read a file, look at what came back, then decide — that
+  decision lands in turn two, which never runs. So a prompt that invites the
+  model to go look at something before answering is a miss even when its
+  eventual answer would have been perfect. This is the single most important
+  fact about writing cases.
 - **The session is fresh, but not empty.** `CLAUDE.md` → `AGENTS.md` is loaded
   in full on every turn, so anything the rules file already answers gets
   answered without a skill.

scripts/lib/skill-manifest.mjs

@@ -176,6 +176,13 @@ export function parseSkill(dirName, text) {
     // `typeof node.value !== "string"` also covers the non-scalar cases: a map
     // or list node has no `.value`, and its own checks above already report it.
     if (!node || typeof node.value !== "string") continue;
+    // Only on a PLAIN scalar is a comment evidence of loss. A quoted or block
+    // scalar can carry a perfectly legitimate trailing one — `description:
+    // "Complete value" # note` keeps its whole value — and flagging that told
+    // the author to quote a value they had already quoted (Copilot). The regex
+    // this replaced tested the same thing by reading the first character; the
+    // node states it outright.
+    if (node.type !== "PLAIN") continue;
     const dropped = node.comment;
     if (typeof dropped !== "string") continue;
     errors.push(

scripts/lib/skill-manifest.test.mjs

@@ -602,6 +602,41 @@ test("the positive floor counts distinct prompts, not entries", () => {
   );
 });
 
+test("a quoted or block scalar may carry a trailing comment", () => {
+  // `yaml` attaches a legitimate trailing comment to these nodes too, so keying
+  // on `.comment` alone rejected valid frontmatter and told the author to quote
+  // a value that was already quoted. Only a PLAIN scalar can lose text to one.
+  for (const value of ['"Complete value" # note', "'Complete value' # note"]) {
+    const parsed = parseSkill(
+      "alpha",
+      [
+        "---",
+        "name: alpha",
+        `description: ${value}`,
+        "disable-model-invocation: false",
+        "---",
+        "body",
+      ].join("\n"),
+    );
+    assert.deepEqual(parsed.errors, [], value);
+    assert.equal(parsed.description, "Complete value");
+  }
+
+  const block = parseSkill(
+    "alpha",
+    [
+      "---",
+      "name: alpha",
+      "description: | # note",
+      "  Complete value",
+      "disable-model-invocation: false",
+      "---",
+      "body",
+    ].join("\n"),
+  );
+  assert.deepEqual(block.errors, []);
+});
+
 test("the truncation guard covers noncanonical key forms", () => {
   // `"description":` and `description :` both populate the mapping, so a guard
   // that located the field by matching a `^description:` LINE skipped them