addyosmani / addyosmani/agent-skills

bug(validator): stripFencedCodeBlocks misses tilde/indented fences and mismatched closer length

オープン
#437 コメント 2 件 リアクション 0 件 担当者 0 名 GitHub で見る
主要言語
JavaScript
スター
93.8k
フォーク
10k
平均マージ
3日 15時間
マージ済み PR(30日)
23

説明

Surfaced by @ujjwalks while reviewing #428: https://github.com/addyosmani/agent-skills/pull/428#issuecomment-5132076791

**Where:** `stripFencedCodeBlocks` in `scripts/lib/skill-lint.js`. It matches fences with a single regex over the whole document:

```
content.replace(/^(`{3,})[^\n]*\n[\s\S]*?^\1\s*$/gm, '')
```

This only recognizes a backtick fence opening at column zero and closing with a run of backticks of exactly the same length. Three CommonMark-legal forms slip through and their contents stay in the prose both downstream rules read:

- tilde fences (`~~~`)
- fences indented up to three spaces
- a closing fence longer than its opener

**Impact on `main` today (not just #428):** the section-heading check (fixed in #405 to strip fences before matching) routes through this helper. A `## Overview` heading inside a `~~~` example block in a skill's `SKILL.md` currently satisfies the Overview requirement for a skill that has no real Overview section — the exact false-negative #405 was written to close, just via a fence form the regex doesn't see. This is live on `main`, independent of #428.

**Impact on #428 (in flight):** #428's new prose cross-reference patterns also route through this helper (matched against fence-stripped content, per its stated design). The same gap means a fenced `` see `made-up-skill` `` inside a tilde or indented fence still produces a dead-reference warning — exactly the case #428's own test `references inside fenced code blocks are not collected` is asserting against, just in a fence form the current helper doesn't recognize.

**Fix, already built and verified by @ujjwalks:** a line-by-line scan tracking the open fence's marker and length (an unterminated fence runs to end of file, which correctly surfaces as missing-section errors rather than prose that silently passes). Rebased on #428's commit `0980192`, at `ujjwalks/agent-skills@d76aa7f` (branch `fix/skill-lint-fence-parsing-on-428`), touching `skill-lint.js` and 13 new cases appended to `scripts/skill-lint-test.js` — 6 of 13 fail against the current helper (both tilde cases, both indented cases, backtick-doesn't-close-tilde, unterminated), including the two cross-reference cases. Full suite green with the fix: 31/31 in `skill-lint-test.js`, 24 skills, 8 commands, 124 eval checks at rank-1 86%, `run-evals-test.js` 12/12. Corpus output unchanged.

**Why a separate issue instead of folding into #428:** the section-check half of this bug predates #428 and affects `main` regardless of #428's outcome. @addyosmani already flagged #428 as the fourth in-flight change touching this file and asked for @nucliweb's eyes before merge — tracking this separately avoids stacking a fifth concern onto that review and lets the `main`-branch fix land on its own schedule. Whoever picks this up should decide whether the standalone fix lands against `main` first (with #428 rebasing after) or the reverse.

Non-goal: this issue is not proposing changes to #428's frontmatter-parsing or cross-reference-pattern scope, only to the shared `stripFencedCodeBlocks` helper both depend on.

コントリビューションガイド

コントリビューションガイドを開く

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。