diff --git a/plugins/skill-quality/.claude-plugin/plugin.json b/plugins/skill-quality/.claude-plugin/plugin.json index 5f95ea01dd..7eb9223f88 100644 --- a/plugins/skill-quality/.claude-plugin/plugin.json +++ b/plugins/skill-quality/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "skill-quality", - "version": "0.15.5", + "version": "0.15.6", "description": "Skill-authoring QA tooling: a static contract checker that runs twenty-two deterministic checks over a Claude Code skill (frontmatter, per-skill listing-entry cap, trigger-keyword preservation, line caps, broken internal refs, markdownlint, gotchas surface, evals presence, precompute opportunity, injection shell-declaration, fresh-eyes declaration conformance), a shared skill-listing budget reporter across a set of skills, and a bundled evals.json schema plus a deterministic eval-quality lint (duplicate case identities, missing fixtures, empty or vague grading criteria, set-coverage warnings). Runs against any repo's skills directory via the convention-resolution ladder — no baked layout.", "author": { "name": "Melodic Software", diff --git a/plugins/skill-quality/CHANGELOG.md b/plugins/skill-quality/CHANGELOG.md index 9187852e6c..499f5afa19 100644 --- a/plugins/skill-quality/CHANGELOG.md +++ b/plugins/skill-quality/CHANGELOG.md @@ -3,6 +3,24 @@ All notable changes to the `skill-quality` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.15.6] + +### Changed + +- **Check 21 parsing contract narrowed (#1493).** Inline code spans, backslash escapes, and + cross-line span carries are no longer modeled — a line with a backtick run or a backslash-escaped + `<` declines directive hard verdicts and is skipped by the Form 1 and judgment detectors rather + than attempting CommonMark pairing in `awk`. Fenced code blocks, container-nested fences, and + indented-code ambiguity handling are unchanged. Spec: + `skills/check/reference/fresh-eyes-declarations.md`. + +### Fixed + +- **Check 21 carries multi-line HTML comment state.** Delegation wording split across a multi-line + comment (`` on a third) no longer satisfies Form 1 while + the judgment line still WARNs. Directive classification still runs before comment stripping so + `fresh-eyes-exempt` directives remain visible. + ## [0.15.5] ### Fixed diff --git a/plugins/skill-quality/scripts/check-skill.sh b/plugins/skill-quality/scripts/check-skill.sh index 0833ee1b81..e21711226b 100755 --- a/plugins/skill-quality/scripts/check-skill.sh +++ b/plugins/skill-quality/scripts/check-skill.sh @@ -891,9 +891,10 @@ fi # spec for authors: skills/check/reference/fresh-eyes-declarations.md. # Scan surface excludes vendor/ (byte-frozen per check 8 — findings would be # permanently unclearable) and evals/ (fixtures contain arbitrary prose). -# Fenced blocks and inline code spans are ignored by both detectors so docs -# can show literal examples (self-reference guard); a trailing \r is tolerated -# per line (third-party checkouts without eol=lf normalization). +# Fenced code blocks are ignored by both detectors so docs can show literal +# examples (self-reference guard); lines with backtick runs or backslash-escaped +# `<` are structurally ambiguous and skipped per the parsing contract. A +# trailing \r is tolerated per line (third-party checkouts without eol=lf normalization). FRESH_EYES_PROXIMITY_LINES=8 # Lowercase POSIX ERE (matched against the lowercased, span-stripped line). # Seeded from the phrasing of the audited skills and their exempted steps; @@ -949,7 +950,7 @@ for fe_file in "${FRESH_EYES_FILES[@]}"; do done < <(awk -v P="$FRESH_EYES_PROXIMITY_LINES" -v JR="$FRESH_EYES_JUDGE_RE" ' # Start of file counts as a paragraph break, so an indented block opening on # line one is recognized as one. - BEGIN { fe_blank = 1 } + BEGIN { fe_blank = 1; cm_in_comment = 0 } # Blockquote nesting depth of a raw line: the number of leading `>` markers, # each optionally followed by one space, under the three-space indent cap. function fe_depth_of(s, d) { @@ -957,18 +958,6 @@ for fe_file in "${FRESH_EYES_FILES[@]}"; do while (match(s, /^ {0,3}> ?/)) { d++; s = substr(s, RSTART + RLENGTH) } return d } - # Position in s of the first backtick run of EXACTLY n characters, or 0. - # Escapes are deliberately ignored: a code span is literal, so a backslash - # inside one is content and cannot suppress the closer. - function fe_span_close(s, n, base) { - base = 0 - while (match(s, /`+/)) { - if (RLENGTH == n) return base + RSTART - base += RSTART + RLENGTH - 1 - s = substr(s, RSTART + RLENGTH) - } - return 0 - } { sub(/\r$/, "") } # YAML frontmatter is not markdown, so nothing in it is a fence, a span, or a # directive. Parsing it as markdown let a block-scalar description containing @@ -979,7 +968,7 @@ for fe_file in "${FRESH_EYES_FILES[@]}"; do fe_fm { if (/^---[ \t]*$/) { fe_fm = 0 - fe_fence = 0; fe_open_pre = 0; sp_open = 0; fe_icode = 0; fe_blank = 1 + fe_fence = 0; fe_open_pre = 0; fe_icode = 0; fe_blank = 1; cm_in_comment = 0 } next } @@ -1050,11 +1039,7 @@ for fe_file in "${FRESH_EYES_FILES[@]}"; do # such a line is ordinary prose, so it falls through to the scanners # instead of opening a fence. if (!(fe_char == "`" && index(fe_info, "`"))) { - # A fence interrupts the paragraph, so any code span still waiting for - # its closer expires here rather than blinding the first paragraph - # after the fence closes. - fe_fence = 1; fe_open_char = fe_char; fe_open_len = fe_len; sp_open = 0 - fe_open_pre = fe_stripped; fe_open_depth = fe_bq_depth + fe_fence = 1; fe_open_char = fe_char; fe_open_len = fe_len; fe_open_pre = fe_stripped; fe_open_depth = fe_bq_depth fe_open_bq = (fe_bq_depth > 0); fe_open_ind = fe_strip_len next } @@ -1062,62 +1047,6 @@ for fe_file in "${FRESH_EYES_FILES[@]}"; do fe_fence { next } { line = $0 - # A code span lives inside one paragraph, so a blank line expires a span - # still waiting for its closer — otherwise one stray backtick would blind - # the detectors for the rest of the file. - if (line ~ /^[ \t]*$/) sp_open = 0 - # Escapes and code spans resolve in ONE left-to-right pass because - # CommonMark couples them: escapes apply only outside a span, and a span is - # literal inside. Two independent passes cannot express that — either pass - # order breaks one of the two rules. - sp_keep = "" - # A span opened on an earlier line: spans may cross a newline, so scan for - # the closer first. With no run of exactly the opener length, the whole - # line is span content and never reaches the detectors. - if (sp_open) { - sp_at = fe_span_close(line, sp_open) - if (!sp_at) next - line = substr(line, sp_at + sp_open) - sp_keep = " " - sp_open = 0 - } - while (1) { - if (!match(line, /[\\`]/)) { sp_keep = sp_keep line; break } - sp_p = RSTART - sp_keep = sp_keep substr(line, 1, sp_p - 1) - if (substr(line, sp_p, 1) == "\\") { - # Only the three characters this scanner keys on need escape handling, - # which makes the set complete for it. Any other backslash is ordinary - # text and must survive, or prose like a Windows path would lose the - # character after it. - sp_next = substr(line, sp_p + 1, 1) - if (sp_next == "`" || sp_next == "<" || sp_next == "\\") { - sp_keep = sp_keep " " - line = substr(line, sp_p + 2) - } else { - sp_keep = sp_keep "\\" - line = substr(line, sp_p + 1) - } - continue - } - # An opening backtick run pairs with the next run of EXACTLY the same - # length; a shorter or longer run inside is span content, so a naive - # /`[^`]*`/ would split a ``…`` span and expose its content. - sp_len = 0 - while (substr(line, sp_p + sp_len, 1) == "`") sp_len++ - line = substr(line, sp_p + sp_len) - sp_at = fe_span_close(line, sp_len) - # No closer on this line: the span may continue on the next one, so carry - # the opener length forward and drop the remainder as span content. The - # carry is optimistic — a run that never closes before the paragraph ends - # masks content CommonMark would call literal — but the tradeoff is - # deliberate: masking risks missing a declaration, whereas scanning risks - # a blocking failure on legitimate code-span text. - if (!sp_at) { sp_open = sp_len; break } - sp_keep = sp_keep " " - line = substr(line, sp_at + sp_len) - } - line = sp_keep # Structural ambiguity: a four-space-indented line here is either an # indented code block or a list-item continuation, and telling those # apart needs a block parser this scanner does not have. Rather than @@ -1132,6 +1061,12 @@ for fe_file in "${FRESH_EYES_FILES[@]}"; do fe_icode = (fe_ind2 >= 4 && (fe_icode || fe_blank)) fe_blank = 0 } + # Inline code spans and backslash escapes are not reconstructed — a line + # that carries either is ambiguous for directive hard verdicts and is + # skipped by the Form 1 and judgment detectors. See the parsing contract. + fe_bt_ambig = (index(line, "`") > 0) + fe_esc_ambig = (index(line, "\\<") > 0) # portability-ok: index() for literal backslash+less-than, not a GNU grep word boundary + fe_line_ambig = (fe_icode || fe_bt_ambig || fe_esc_ambig) # Classify each directive on the line independently, bounded at its own # `-->`. Testing the whole line let a valid directive elsewhere on it lend # its class and reason to a malformed neighbour, so an unknown-class @@ -1147,7 +1082,7 @@ for fe_file in "${FRESH_EYES_FILES[@]}"; do if (dir_end) dir_one = substr(dir_one, 1, dir_end + 2) dir_n++ d[dir_n] = NR - damb[dir_n] = fe_icode + damb[dir_n] = fe_line_ambig if (dir_one ~ //) { dt[dir_n] = "valid" } else if (dir_one ~ //) { @@ -1160,18 +1095,32 @@ for fe_file in "${FRESH_EYES_FILES[@]}"; do # a hidden `` would be exactly # the parallel marker Form 1 exists to rule out, and it was satisfying the # delegation detector. Comments come off only AFTER the directives above are - # classified, since a directive IS a comment. An unterminated ``, so delegation wording split across a + # multi-line comment does not satisfy Form 1. + if (cm_in_comment) { + cm_e = index(line, "-->") + if (cm_e) { + line = substr(line, cm_e + 3) + cm_in_comment = 0 + } else { + line = "" + } + } cm_keep = "" while ((cm_p = index(line, "") - if (!cm_e) { line = ""; break } + if (!cm_e) { + if (!fe_bt_ambig && !fe_esc_ambig) cm_in_comment = 1 + line = "" + break + } line = substr(line, cm_e + 3) } line = cm_keep line + if (fe_bt_ambig || fe_esc_ambig) next low = tolower(line) # Delegation wording needs a worker actually named on the line — bare # "in a fresh context" prose assigns no one and does not declare. Both diff --git a/plugins/skill-quality/scripts/check-skill.test.sh b/plugins/skill-quality/scripts/check-skill.test.sh index 40ebc42f8e..c5efa8456c 100755 --- a/plugins/skill-quality/scripts/check-skill.test.sh +++ b/plugins/skill-quality/scripts/check-skill.test.sh @@ -1532,8 +1532,8 @@ else fail "info-string fence line should not desync the fence guard (rc=$rc): $out" fi -# 37c. Check 21: a multi-backtick inline span hides its content (CommonMark -# exact-length pairing — a naive single-backtick stripper would expose it). +# 37c. Check 21: a line with backtick runs is structurally ambiguous — literal +# directive examples in inline code yield no hard verdict. make_skill fe-span-double '--- name: fe-span-double description: "Fresh-eyes fixture. Use when: '"'"'fe span double fixture'"'"'." @@ -1550,9 +1550,9 @@ None known. out="$(run fe-span-double 2>&1)" rc=$? if [[ $rc -eq 0 ]] && ! grep -q 'fresh-eyes-exempt directive' <<<"$out"; then - pass "double-backtick span content is not scanned (check 21)" + pass "backtick-bearing line declines directive hard verdicts (check 21)" else - fail "double-backtick span should hide the literal directive (rc=$rc): $out" + fail "backtick-bearing line should not FAIL on a literal directive (rc=$rc): $out" fi # 37d. Check 21: bare fresh-context wording naming no worker does not declare — @@ -1678,8 +1678,8 @@ else fail "agentless stem should not satisfy check 21 (rc=$rc): $out" fi -# 37i. Check 21: a code span that crosses a newline keeps masking its content on -# the following line — a literal directive inside it must not be parsed. +# 37i. Check 21: a line with a backtick run is structurally ambiguous — a +# directive fragment on the same line yields no hard verdict. make_skill fe-span-crossline '--- name: fe-span-crossline description: "Fresh-eyes fixture. Use when: '"'"'fe crossline span fixture'"'"'." @@ -1697,13 +1697,13 @@ None known. out="$(run fe-span-crossline 2>&1)" rc=$? if [[ $rc -eq 0 ]] && ! grep -q 'malformed fresh-eyes-exempt directive' <<<"$out"; then - pass "code span crossing a line boundary still masks its content (check 21)" + pass "backtick-bearing line declines directive hard verdicts (check 21)" else - fail "cross-line span content should not be parsed as a directive (rc=$rc): $out" + fail "backtick-bearing line should not FAIL on a literal directive (rc=$rc): $out" fi -# 37j. Check 21: the cross-line carry expires at the paragraph break, so one -# stray unclosed backtick cannot blind the scanner for the rest of the file. +# 37j. Check 21: a stray backtick on one line does not blind later lines — the +# scanner no longer carries span state across lines. make_skill fe-span-stray '--- name: fe-span-stray description: "Fresh-eyes fixture. Use when: '"'"'fe stray backtick fixture'"'"'." @@ -1722,9 +1722,9 @@ None known. out="$(run fe-span-stray 2>&1)" rc=$? if [[ $rc -ne 0 ]] && grep -q 'malformed fresh-eyes-exempt directive' <<<"$out"; then - pass "stray-backtick span carry expires at the paragraph break (check 21)" + pass "directive on a later line still FAILs after a stray backtick (check 21)" else - fail "directive after a stray-backtick paragraph should still FAIL (rc=$rc): $out" + fail "directive after a stray-backtick line should still FAIL (rc=$rc): $out" fi # 37k. Check 21: a fence nested in a blockquote is still a fence — its literal @@ -1757,8 +1757,9 @@ else fail "directive inside a blockquoted fence should not be parsed (rc=$rc): $out" fi -# 37l. Check 21: backslash-escaped backticks render literally, so they open no -# code span and the directive between them is a real malformed directive. +# 37l. Check 21: a line with backtick runs is structurally ambiguous — the +# scanner does not pair spans or resolve escapes, so a directive shown +# between escaped backticks yields no hard verdict rather than a FAIL. make_skill fe-escaped-span '--- name: fe-escaped-span description: "Fresh-eyes fixture. Use when: '"'"'fe escaped span fixture'"'"'." @@ -1774,10 +1775,10 @@ None known. ' out="$(run fe-escaped-span 2>&1)" rc=$? -if [[ $rc -ne 0 ]] && grep -q 'malformed fresh-eyes-exempt directive' <<<"$out"; then - pass "escaped backticks open no code span (check 21)" +if [[ $rc -eq 0 ]] && ! grep -q 'malformed fresh-eyes-exempt directive' <<<"$out"; then + pass "backtick-bearing line declines directive hard verdicts (check 21)" else - fail "directive between escaped backticks should FAIL (rc=$rc): $out" + fail "backtick-bearing line should not FAIL on a literal directive (rc=$rc): $out" fi # 37m. Check 21: each directive on a shared line is classified on its own, so a @@ -1949,9 +1950,9 @@ else fail "list fence body should be suppressed and the line-14 directive FAIL (rc=$rc): $out" fi -# 37s. Check 21: escapes are not processed inside a code span, so a literal -# backslash before the closing run does not stop the run from closing — -# the directive AFTER the span is real and still blocks. +# 37s. Check 21: a line with a backtick run is structurally ambiguous — the +# scanner does not pair spans, so a directive after an inline-code +# fragment on the same line yields no hard verdict. make_skill fe-span-literal-backslash '--- name: fe-span-literal-backslash description: "Fresh-eyes fixture. Use when: '"'"'fe span literal backslash fixture'"'"'." @@ -1967,10 +1968,10 @@ None known. ' out="$(run fe-span-literal-backslash 2>&1)" rc=$? -if [[ $rc -ne 0 ]] && grep -q 'malformed fresh-eyes-exempt directive' <<<"$out"; then - pass "a literal backslash does not suppress a span closer (check 21)" +if [[ $rc -eq 0 ]] && ! grep -q 'malformed fresh-eyes-exempt directive' <<<"$out"; then + pass "backtick-bearing line declines directive hard verdicts (check 21)" else - fail "directive after a backslash-ending span should FAIL (rc=$rc): $out" + fail "backtick-bearing line should not FAIL on a literal directive (rc=$rc): $out" fi # 37v. Check 21: a directive at four-space indent is structurally ambiguous — @@ -2056,6 +2057,33 @@ else fail "commented wording should not satisfy check 21 (rc=$rc): $out" fi +# 37u2. Check 21: multi-line HTML comment state carries until the closing +# `-->`, so delegation wording on a middle line does not satisfy Form 1. +make_skill fe-comment-multiline '--- +name: fe-comment-multiline +description: "Fresh-eyes fixture. Use when: '"'"'fe comment multiline fixture'"'"'." +--- + +## Steps + + + +Self-review your own work. + +## Gotchas + +None known. +' +out="$(run fe-comment-multiline 2>&1)" +rc=$? +if [[ $rc -eq 0 ]] && grep -q 'same-context judgment language with no' <<<"$out"; then + pass "delegation wording in a multi-line HTML comment does not declare (check 21)" +else + fail "multi-line commented wording should not satisfy check 21 (rc=$rc): $out" +fi + # 37w. Check 21: the directive name needs a terminator. An ordinary comment about # "fresh-eyes-exemption" shares the prefix but is not a directive, and a # prefix-only match FAILed the skill on prose. diff --git a/plugins/skill-quality/skills/check/reference/fresh-eyes-declarations.md b/plugins/skill-quality/skills/check/reference/fresh-eyes-declarations.md index 12f733fe8d..345c573536 100644 --- a/plugins/skill-quality/skills/check/reference/fresh-eyes-declarations.md +++ b/plugins/skill-quality/skills/check/reference/fresh-eyes-declarations.md @@ -99,13 +99,6 @@ cannot tell. A finding against this check is measured against this list, not aga a prefix strips prefixes inside the fence, so a quoted run cannot close an unprefixed fence. - **Container termination.** A nested fence ends with its container: a blockquote when the quote depth drops below the opener's, a list item on a dedent below the opener's content column. -- **Inline code spans**, pairing a backtick run with the next run of exactly equal length, including - multi-backtick spans and spans that cross a newline (the carry expires at the next blank line or - fence, since a span cannot outlive its paragraph). -- **Backslash escapes**, resolved in the same pass as spans because CommonMark couples them: outside - a span an escape makes the next character literal, so `` \` `` opens no span and `\` is - text rather than a directive; inside a span nothing is escaped, so a literal backslash before the - closing run does not stop it closing. - **Multiple directives on one line**, each classified independently and bounded at its own `-->`. The name must be followed by `:`, whitespace, or `-->`, so an ordinary comment about a longer identifier such as `fresh-eyes-exemption` is not read as a directive. @@ -113,7 +106,9 @@ cannot tell. A finding against this check is measured against this list, not aga directive, and every structural carry resets at its closing `---`. - **Whole-word wording boundaries** on both halves of Form 1, so `agentless` names no worker and `Refresh context` is not the fresh-context wording. -- **Single-line HTML comments**, stripped before the Form 1 detector so hidden wording cannot declare. +- **HTML comments**, stripped before the Form 1 detector so hidden wording cannot declare. Comment + state carries across lines until the closing `-->`, so delegation wording split across a multi-line + comment does not satisfy Form 1. ### Not attempted @@ -130,8 +125,11 @@ contract exists to stop: - **Paragraph-interrupting block constructs.** A pending cross-line span carry expires at a blank line or a fence, but not at an ATX heading, thematic break, or table that also interrupts the paragraph in CommonMark. -- **Multi-line HTML comments.** Comment state is not carried across lines: an unterminated `` sequence is not distinguished from a real comment opener. - **Reference definitions, HTML blocks, setext headings, and link/image syntax** are not interpreted at all; they are scanned as ordinary prose. @@ -146,15 +144,13 @@ The two verdict families are asymmetric, and the whole posture follows from that So where the structure pass reaches a configuration it cannot resolve, it **withholds the hard verdicts** for directives on that line. It also withholds the stale WARN, and refuses to let such a -directive satisfy a nearby judgment step — the same lack of confidence cuts both ways, so a literal -exemption inside an indented example cannot silence the warning that step deserves. The judgment -detector itself continues to run. Today this fires on the -indented-code case above; the mechanism could extend to a resolved cross-line span carry and to an -unterminated comment, and deliberately does not, because both occur throughout ordinary prose and -suppressing them would widen the blind spot far past anything observed. - -Where an unmodeled construct instead causes content to be **skipped** — an unclosed fence or span -carry swallowing lines — no verdict forms at all. That is already the safe direction, and it is worth +directive satisfy a nearby judgment step — the same lack of confidence cuts both ways. On an +indented-code line, `fe_icode` feeds that directive-side suppression only; the judgment detector +still runs on the line's own prose. On a line with a backtick run or a backslash-escaped `<`, the +scanner declines the line's own Form 1 and judgment detectors via an explicit skip. + +Where an unmodeled construct instead causes content to be **skipped** — an unclosed fence +swallowing lines — no verdict forms at all. That is already the safe direction, and it is worth being exact: suppression prevents wrong FAILs; it is not what makes a skipped line harmless. **This posture is specific to check 21**, whose verdicts are authoring nudges. Do not carry it into a