diff --git a/plugins/kyberforge/.apm/skills/agent-audit/scripts/validate.sh b/plugins/kyberforge/.apm/skills/agent-audit/scripts/validate.sh index 46319f4..738ab4e 100755 --- a/plugins/kyberforge/.apm/skills/agent-audit/scripts/validate.sh +++ b/plugins/kyberforge/.apm/skills/agent-audit/scripts/validate.sh @@ -69,6 +69,24 @@ import glob import yaml +# Output is UTF-8 for the same reason input is: under LC_ALL=C the streams +# default to ASCII, and this script's own message text carries em dashes (the +# ADR-0020 boundary SUGGESTION is one). Pinning only the reads moved the crash +# from the read to the write — a UnicodeEncodeError raised while PRINTING, after +# every check has already run, which loses the whole report and (here) flips a +# clean exit 0 into a traceback and an exit 1. read_text() in the shared +# resolver block below pins the reads; this pins the writes. +# +# Deliberately OUTSIDE the ADR-0020 shared boundary resolver block: the two +# validate.sh copies print findings, skill-size-check.sh has its own top-level +# equivalent, and tests/test-adr0020-contract.sh hashes that block for +# byte-identity across all three. +for _stream in (sys.stdout, sys.stderr): + try: + _stream.reconfigure(encoding='utf-8') + except AttributeError: # pragma: no cover — Python < 3.7 + pass + agent_file = os.path.abspath(sys.argv[1]) script_dir = sys.argv[2] @@ -552,9 +570,9 @@ def known_targets(start_dir): # ambiguity to resolve, and an author who wants a route checked unconditionally # has two ways to say so. # -# BOTH FORMS ARE SWEPT FOR ON THEIR OWN inside a boundary sentence, and that is -# a repair of the promise above rather than a widening of it. Until the sweeps -# existed, notation was only ever seen as the OBJECT OF A ROUTE VERB (`use +# BOTH FORMS ARE SWEPT FOR ON THEIR OWN, and that is a repair of the promise +# above rather than a widening of it. Until the sweeps existed, notation was +# only ever seen as the OBJECT OF A ROUTE VERB (`use # /name`) or as the tail of a `not ... ->` clause with no `;` or sentence end in # between. Every one of these therefore exited 0 in total silence — no ERROR, no # SUGGESTION, not even the target's name: @@ -565,6 +583,7 @@ def known_targets(start_dir): # Do not use for Y — defer to /no-such-skill. # Do not use for Y — /no-such-skill. # Do not use for Y; -> no-such-skill covers it. +# For W, /no-such-skill is the right entry point. # The target was never EXTRACTED, so the notation-first rule in _add() had # nothing to apply itself to and the "always blocks" promise was false for the # ordinary way an author writes the thing. The SUGGESTION tier made it worse @@ -573,12 +592,43 @@ def known_targets(start_dir): # visible SUGGESTION into silence — the gate teaching the one edit that blinds # it. # -# The sweeps are gated on the sentence carrying a BOUNDARY_MARKER, the same gate -# the backtick sweep uses, and NOTATION_SLASH refuses a token that is part of a -# PATH: a following `/`, or a `.` followed by a non-space, means -# `references/foo.md`, `docs/a/b.md` or `https://x/y`, not a route. A sentence's -# closing `.` is not followed by a non-space, so `— /no-such-skill.` still -# counts. +# THE TWO SWEEPS ARE GATED DIFFERENTLY, and the asymmetry is the whole point. +# `/name` is Claude Code's invocation syntax and nothing else — no English +# sentence contains one by accident — so the ADR-0020 amendment and +# docs/spec/gates.md both promise it blocks UNCONDITIONALLY, for any name. So +# NOTATION_SLASH is swept over every sentence, boundary marker or not. Gating it +# on BOUNDARY_MARKER made that promise false for the last sentence of +# Do not use for Z — use /real-skill instead. +# For W, /no-such-skill is the right entry point. +# which exited 0 in total silence: the boundary clause is one sentence up, so +# the sweep never looked at the sentence carrying the broken route. Extraction is +# per-sentence by design (corroboration is scoped to one sentence), which is +# exactly what made the gap invisible. +# +# NOTATION_ARROW stays gated on BOUNDARY_MARKER, and so does the backtick sweep. +# Neither form is unambiguous: `-> name` is also how a process chain is written +# ("reproduce -> minimise -> regression-test") and a code span is how a tool, a +# file and a skill are all cited. Ungating either would fire on prose that +# carries no routing intent at all — the false-positive class this whole +# extractor is tuned against. +# +# BOTH `/name` PATTERNS REFUSE A TOKEN THAT IS PART OF A PATH: a following `/`, +# or a `.` followed by a non-space, means `references/foo.md`, `docs/a/b.md` or +# `https://x/y`, not a route. A sentence's closing `.` is not followed by a +# non-space, so `— /no-such-skill.` still counts. +# +# THAT GUARD IS WRITTEN `(?![\w-])` AND NOT `\b`, because `\b` is not a guard at +# all here: it holds after a hyphen, so when the trailing lookahead rejected the +# full segment the engine simply backtracked to a shorter hyphen-terminated +# prefix and reported THAT as a route. Every one of these was a hard blocking +# ERROR naming a skill nobody had written: +# the config lives at /opt-tools/bin/thing. -> 'opt' +# see /api-docs/v2.md for the schema. -> 'api' AND 'api-docs' +# the file /no-such-skill.md documents it. -> 'no-such' +# `(?![\w-])` forbids the shortened prefix outright, so the whole segment is +# rejected as the path it is. MARKED_TARGET carries the same guard: it had no +# trailing lookahead whatsoever, so `see /api-docs/v2.md` raised the second of +# the two errors above through the route-verb path rather than the sweep. # # NAMESPACE: `plugin:skill` is live in this repo (native user-scope installs # still resolve `gitea:gitea-prs`), so the patterns admit an optional @@ -590,7 +640,8 @@ ROUTE_VERB = (r"(?:use|uses|using|run|runs|invoke|invokes|invoking|try|see" r"|that'?s|compose|composes|call|calls" r"|routes?\s+to|delegates?\s+to|prefers?|switch(?:es)?\s+to" r"|hands?\s+off\s+to)") -MARKED_TARGET = r"(?:`/?(%s)`|(?|→)\s*%s" % MARKED_TARGET, re.I) -# The two EXPLICIT ROUTE NOTATION sweeps, scoped to a boundary sentence by their -# caller. NOTATION_SLASH is deliberately not a reuse of MARKED_TARGET's `/name` -# alternative: that one only ever runs behind a route verb or an arrow, and the -# trailing lookahead here is the part that makes a FREE-STANDING sweep safe. +# The two EXPLICIT ROUTE NOTATION sweeps. NOTATION_SLASH runs over EVERY +# sentence; NOTATION_ARROW is scoped to a boundary sentence by its caller (see +# the asymmetry note in the header). NOTATION_SLASH is deliberately not a reuse +# of MARKED_TARGET's `/name` alternative: that one only ever runs behind a route +# verb or an arrow, and it may match a namespaced or path-adjacent token in +# positions this free-standing sweep must refuse. # NOTATION_ARROW is ARROW_BOUNDARY minus its leading `\bnot\b%s*?`, which is # what made `Do not use for Y; -> no-such-skill covers it.` invisible: # CLAUSE_BODY cannot cross the `;`, so the clause's own punctuation disarmed the @@ -618,7 +671,8 @@ ARROW_MARKED = re.compile(r"(?:->|→)\s*%s" % MARKED_TARGET, re.I) # hard ERROR under ARROW_BOUNDARY, so this changes which boundary words reach the # arrow, not whether prose can. An author who means the chain and not a route # writes it in its own sentence, where neither pattern looks. -NOTATION_SLASH = re.compile(r"(?|→)\s*(%s)\b" % NAME_HYPH, re.I) # CLAUSE_BODY is what may sit between `Not` and the arrow, and it is NOT # `[^.;]`. That class cannot cross a `.`, so every boundary clause naming a @@ -794,14 +848,16 @@ def _extract_sentence(sentence): for match in ARROW_BOUNDARY.finditer(sentence): _add(out, sentence, match.group(1), match.start(1), match.end(1), strict=True, arrow=True) + # `/name` wherever it sits, in ANY sentence — not only where a route verb or + # an arrow happens to precede it, and NOT only inside a boundary sentence. + # See the EXPLICIT ROUTE NOTATION note in the header for the eight phrasings + # this recovers and for why silence was the failure mode. The sweep takes no + # follower test: _add() reads the notation first and marks it. + for match in NOTATION_SLASH.finditer(sentence): + _add(out, sentence, match.group(1), match.start(1), match.end(1)) if boundary: - # Route notation wherever it sits in the clause, not only where a route - # verb or an arrow happens to precede it. See the EXPLICIT ROUTE - # NOTATION note in the header for the seven phrasings this recovers and - # for why silence was the failure mode. Neither sweep takes the follower - # test: _add() reads the notation first and both forms reach it marked. - for match in NOTATION_SLASH.finditer(sentence): - _add(out, sentence, match.group(1), match.start(1), match.end(1)) + # The arrow and backtick forms are ambiguous in ordinary prose, so they + # stay scoped to a sentence that carries a boundary marker. for match in NOTATION_ARROW.finditer(sentence): _add(out, sentence, match.group(1), match.start(1), match.end(1), strict=True, arrow=True) diff --git a/plugins/kyberforge/.apm/skills/agent-audit/tests/validate.bats b/plugins/kyberforge/.apm/skills/agent-audit/tests/validate.bats index b58033e..9839841 100644 --- a/plugins/kyberforge/.apm/skills/agent-audit/tests/validate.bats +++ b/plugins/kyberforge/.apm/skills/agent-audit/tests/validate.bats @@ -943,3 +943,23 @@ EOF refute_output --partial "Traceback" refute_output --partial "FileNotFoundError" } + +# --------------------------------------------------------------------------- +# Encoding, write side: sys.stdout/stderr.reconfigure(encoding='utf-8') +# +# read_text() in the shared resolver block pins the READS to UTF-8. That moved +# the LC_ALL=C crash to the WRITE: this script's own message text carries em +# dashes (the ADR-0020 boundary SUGGESTION is one), so the streams' ASCII +# default raised UnicodeEncodeError while PRINTING — after every check had +# already run. Here it also flipped a clean exit 0 into a traceback and exit 1. +# --------------------------------------------------------------------------- + +@test "under LC_ALL=C the report is printed, not lost to a UnicodeEncodeError" { + local root="$TMPDIR/locale-pkg" + make_apm_agent "$root" "locale-agent" + run env LC_ALL=C PYTHONUTF8=0 bash "$SCRIPT" "$root/.apm/agents/locale-agent.agent.md" + assert_success + assert_output --partial "description has no boundary clause" + refute_output --partial "UnicodeEncodeError" + refute_output --partial "Traceback" +} diff --git a/plugins/kyberforge/.apm/skills/skill-audit/scripts/validate.sh b/plugins/kyberforge/.apm/skills/skill-audit/scripts/validate.sh index 8934687..e5ce8f3 100755 --- a/plugins/kyberforge/.apm/skills/skill-audit/scripts/validate.sh +++ b/plugins/kyberforge/.apm/skills/skill-audit/scripts/validate.sh @@ -59,6 +59,24 @@ import glob import yaml +# Output is UTF-8 for the same reason input is: under LC_ALL=C the streams +# default to ASCII, and this script's own message text carries em dashes (the +# ADR-0020 boundary SUGGESTION is one). Pinning only the reads moved the crash +# from the read to the write — a UnicodeEncodeError raised while PRINTING, after +# every check has already run, which loses the whole report and (here) flips a +# clean exit 0 into a traceback and an exit 1. read_text() in the shared +# resolver block below pins the reads; this pins the writes. +# +# Deliberately OUTSIDE the ADR-0020 shared boundary resolver block: the two +# validate.sh copies print findings, skill-size-check.sh has its own top-level +# equivalent, and tests/test-adr0020-contract.sh hashes that block for +# byte-identity across all three. +for _stream in (sys.stdout, sys.stderr): + try: + _stream.reconfigure(encoding='utf-8') + except AttributeError: # pragma: no cover — Python < 3.7 + pass + skill_dir = os.path.abspath(sys.argv[1]) skill_md = os.path.join(skill_dir, "SKILL.md") @@ -478,9 +496,9 @@ def known_targets(start_dir): # ambiguity to resolve, and an author who wants a route checked unconditionally # has two ways to say so. # -# BOTH FORMS ARE SWEPT FOR ON THEIR OWN inside a boundary sentence, and that is -# a repair of the promise above rather than a widening of it. Until the sweeps -# existed, notation was only ever seen as the OBJECT OF A ROUTE VERB (`use +# BOTH FORMS ARE SWEPT FOR ON THEIR OWN, and that is a repair of the promise +# above rather than a widening of it. Until the sweeps existed, notation was +# only ever seen as the OBJECT OF A ROUTE VERB (`use # /name`) or as the tail of a `not ... ->` clause with no `;` or sentence end in # between. Every one of these therefore exited 0 in total silence — no ERROR, no # SUGGESTION, not even the target's name: @@ -491,6 +509,7 @@ def known_targets(start_dir): # Do not use for Y — defer to /no-such-skill. # Do not use for Y — /no-such-skill. # Do not use for Y; -> no-such-skill covers it. +# For W, /no-such-skill is the right entry point. # The target was never EXTRACTED, so the notation-first rule in _add() had # nothing to apply itself to and the "always blocks" promise was false for the # ordinary way an author writes the thing. The SUGGESTION tier made it worse @@ -499,12 +518,43 @@ def known_targets(start_dir): # visible SUGGESTION into silence — the gate teaching the one edit that blinds # it. # -# The sweeps are gated on the sentence carrying a BOUNDARY_MARKER, the same gate -# the backtick sweep uses, and NOTATION_SLASH refuses a token that is part of a -# PATH: a following `/`, or a `.` followed by a non-space, means -# `references/foo.md`, `docs/a/b.md` or `https://x/y`, not a route. A sentence's -# closing `.` is not followed by a non-space, so `— /no-such-skill.` still -# counts. +# THE TWO SWEEPS ARE GATED DIFFERENTLY, and the asymmetry is the whole point. +# `/name` is Claude Code's invocation syntax and nothing else — no English +# sentence contains one by accident — so the ADR-0020 amendment and +# docs/spec/gates.md both promise it blocks UNCONDITIONALLY, for any name. So +# NOTATION_SLASH is swept over every sentence, boundary marker or not. Gating it +# on BOUNDARY_MARKER made that promise false for the last sentence of +# Do not use for Z — use /real-skill instead. +# For W, /no-such-skill is the right entry point. +# which exited 0 in total silence: the boundary clause is one sentence up, so +# the sweep never looked at the sentence carrying the broken route. Extraction is +# per-sentence by design (corroboration is scoped to one sentence), which is +# exactly what made the gap invisible. +# +# NOTATION_ARROW stays gated on BOUNDARY_MARKER, and so does the backtick sweep. +# Neither form is unambiguous: `-> name` is also how a process chain is written +# ("reproduce -> minimise -> regression-test") and a code span is how a tool, a +# file and a skill are all cited. Ungating either would fire on prose that +# carries no routing intent at all — the false-positive class this whole +# extractor is tuned against. +# +# BOTH `/name` PATTERNS REFUSE A TOKEN THAT IS PART OF A PATH: a following `/`, +# or a `.` followed by a non-space, means `references/foo.md`, `docs/a/b.md` or +# `https://x/y`, not a route. A sentence's closing `.` is not followed by a +# non-space, so `— /no-such-skill.` still counts. +# +# THAT GUARD IS WRITTEN `(?![\w-])` AND NOT `\b`, because `\b` is not a guard at +# all here: it holds after a hyphen, so when the trailing lookahead rejected the +# full segment the engine simply backtracked to a shorter hyphen-terminated +# prefix and reported THAT as a route. Every one of these was a hard blocking +# ERROR naming a skill nobody had written: +# the config lives at /opt-tools/bin/thing. -> 'opt' +# see /api-docs/v2.md for the schema. -> 'api' AND 'api-docs' +# the file /no-such-skill.md documents it. -> 'no-such' +# `(?![\w-])` forbids the shortened prefix outright, so the whole segment is +# rejected as the path it is. MARKED_TARGET carries the same guard: it had no +# trailing lookahead whatsoever, so `see /api-docs/v2.md` raised the second of +# the two errors above through the route-verb path rather than the sweep. # # NAMESPACE: `plugin:skill` is live in this repo (native user-scope installs # still resolve `gitea:gitea-prs`), so the patterns admit an optional @@ -516,7 +566,8 @@ ROUTE_VERB = (r"(?:use|uses|using|run|runs|invoke|invokes|invoking|try|see" r"|that'?s|compose|composes|call|calls" r"|routes?\s+to|delegates?\s+to|prefers?|switch(?:es)?\s+to" r"|hands?\s+off\s+to)") -MARKED_TARGET = r"(?:`/?(%s)`|(?|→)\s*%s" % MARKED_TARGET, re.I) -# The two EXPLICIT ROUTE NOTATION sweeps, scoped to a boundary sentence by their -# caller. NOTATION_SLASH is deliberately not a reuse of MARKED_TARGET's `/name` -# alternative: that one only ever runs behind a route verb or an arrow, and the -# trailing lookahead here is the part that makes a FREE-STANDING sweep safe. +# The two EXPLICIT ROUTE NOTATION sweeps. NOTATION_SLASH runs over EVERY +# sentence; NOTATION_ARROW is scoped to a boundary sentence by its caller (see +# the asymmetry note in the header). NOTATION_SLASH is deliberately not a reuse +# of MARKED_TARGET's `/name` alternative: that one only ever runs behind a route +# verb or an arrow, and it may match a namespaced or path-adjacent token in +# positions this free-standing sweep must refuse. # NOTATION_ARROW is ARROW_BOUNDARY minus its leading `\bnot\b%s*?`, which is # what made `Do not use for Y; -> no-such-skill covers it.` invisible: # CLAUSE_BODY cannot cross the `;`, so the clause's own punctuation disarmed the @@ -544,7 +597,8 @@ ARROW_MARKED = re.compile(r"(?:->|→)\s*%s" % MARKED_TARGET, re.I) # hard ERROR under ARROW_BOUNDARY, so this changes which boundary words reach the # arrow, not whether prose can. An author who means the chain and not a route # writes it in its own sentence, where neither pattern looks. -NOTATION_SLASH = re.compile(r"(?|→)\s*(%s)\b" % NAME_HYPH, re.I) # CLAUSE_BODY is what may sit between `Not` and the arrow, and it is NOT # `[^.;]`. That class cannot cross a `.`, so every boundary clause naming a @@ -720,14 +774,16 @@ def _extract_sentence(sentence): for match in ARROW_BOUNDARY.finditer(sentence): _add(out, sentence, match.group(1), match.start(1), match.end(1), strict=True, arrow=True) + # `/name` wherever it sits, in ANY sentence — not only where a route verb or + # an arrow happens to precede it, and NOT only inside a boundary sentence. + # See the EXPLICIT ROUTE NOTATION note in the header for the eight phrasings + # this recovers and for why silence was the failure mode. The sweep takes no + # follower test: _add() reads the notation first and marks it. + for match in NOTATION_SLASH.finditer(sentence): + _add(out, sentence, match.group(1), match.start(1), match.end(1)) if boundary: - # Route notation wherever it sits in the clause, not only where a route - # verb or an arrow happens to precede it. See the EXPLICIT ROUTE - # NOTATION note in the header for the seven phrasings this recovers and - # for why silence was the failure mode. Neither sweep takes the follower - # test: _add() reads the notation first and both forms reach it marked. - for match in NOTATION_SLASH.finditer(sentence): - _add(out, sentence, match.group(1), match.start(1), match.end(1)) + # The arrow and backtick forms are ambiguous in ordinary prose, so they + # stay scoped to a sentence that carries a boundary marker. for match in NOTATION_ARROW.finditer(sentence): _add(out, sentence, match.group(1), match.start(1), match.end(1), strict=True, arrow=True) diff --git a/plugins/kyberforge/.apm/skills/skill-audit/tests/validate.bats b/plugins/kyberforge/.apm/skills/skill-audit/tests/validate.bats index d41c364..3740033 100755 --- a/plugins/kyberforge/.apm/skills/skill-audit/tests/validate.bats +++ b/plugins/kyberforge/.apm/skills/skill-audit/tests/validate.bats @@ -743,3 +743,33 @@ make_hand_invoked_skill() { refute_output --partial "has no boundary clause" assert_output --partial "no target could be read" } + +# --------------------------------------------------------------------------- +# Encoding, write side: sys.stdout/stderr.reconfigure(encoding='utf-8') +# +# read_text() in the shared resolver block pins the READS to UTF-8. That moved +# the LC_ALL=C crash to the WRITE: this script's own message text carries em +# dashes (the ADR-0020 boundary SUGGESTION is one), so the streams' ASCII +# default raised UnicodeEncodeError while PRINTING — after every check had +# already run, losing the whole report at the last step. +# --------------------------------------------------------------------------- + +@test "under LC_ALL=C the report is printed, not lost to a UnicodeEncodeError" { + local dir="$TMPDIR/locale-skill" + mkdir -p "$dir" + cat > "$dir/SKILL.md" <` clause with no `;` or sentence end in # between. Every one of these therefore exited 0 in total silence — no ERROR, no # SUGGESTION, not even the target's name: @@ -565,6 +583,7 @@ def known_targets(start_dir): # Do not use for Y — defer to /no-such-skill. # Do not use for Y — /no-such-skill. # Do not use for Y; -> no-such-skill covers it. +# For W, /no-such-skill is the right entry point. # The target was never EXTRACTED, so the notation-first rule in _add() had # nothing to apply itself to and the "always blocks" promise was false for the # ordinary way an author writes the thing. The SUGGESTION tier made it worse @@ -573,12 +592,43 @@ def known_targets(start_dir): # visible SUGGESTION into silence — the gate teaching the one edit that blinds # it. # -# The sweeps are gated on the sentence carrying a BOUNDARY_MARKER, the same gate -# the backtick sweep uses, and NOTATION_SLASH refuses a token that is part of a -# PATH: a following `/`, or a `.` followed by a non-space, means -# `references/foo.md`, `docs/a/b.md` or `https://x/y`, not a route. A sentence's -# closing `.` is not followed by a non-space, so `— /no-such-skill.` still -# counts. +# THE TWO SWEEPS ARE GATED DIFFERENTLY, and the asymmetry is the whole point. +# `/name` is Claude Code's invocation syntax and nothing else — no English +# sentence contains one by accident — so the ADR-0020 amendment and +# docs/spec/gates.md both promise it blocks UNCONDITIONALLY, for any name. So +# NOTATION_SLASH is swept over every sentence, boundary marker or not. Gating it +# on BOUNDARY_MARKER made that promise false for the last sentence of +# Do not use for Z — use /real-skill instead. +# For W, /no-such-skill is the right entry point. +# which exited 0 in total silence: the boundary clause is one sentence up, so +# the sweep never looked at the sentence carrying the broken route. Extraction is +# per-sentence by design (corroboration is scoped to one sentence), which is +# exactly what made the gap invisible. +# +# NOTATION_ARROW stays gated on BOUNDARY_MARKER, and so does the backtick sweep. +# Neither form is unambiguous: `-> name` is also how a process chain is written +# ("reproduce -> minimise -> regression-test") and a code span is how a tool, a +# file and a skill are all cited. Ungating either would fire on prose that +# carries no routing intent at all — the false-positive class this whole +# extractor is tuned against. +# +# BOTH `/name` PATTERNS REFUSE A TOKEN THAT IS PART OF A PATH: a following `/`, +# or a `.` followed by a non-space, means `references/foo.md`, `docs/a/b.md` or +# `https://x/y`, not a route. A sentence's closing `.` is not followed by a +# non-space, so `— /no-such-skill.` still counts. +# +# THAT GUARD IS WRITTEN `(?![\w-])` AND NOT `\b`, because `\b` is not a guard at +# all here: it holds after a hyphen, so when the trailing lookahead rejected the +# full segment the engine simply backtracked to a shorter hyphen-terminated +# prefix and reported THAT as a route. Every one of these was a hard blocking +# ERROR naming a skill nobody had written: +# the config lives at /opt-tools/bin/thing. -> 'opt' +# see /api-docs/v2.md for the schema. -> 'api' AND 'api-docs' +# the file /no-such-skill.md documents it. -> 'no-such' +# `(?![\w-])` forbids the shortened prefix outright, so the whole segment is +# rejected as the path it is. MARKED_TARGET carries the same guard: it had no +# trailing lookahead whatsoever, so `see /api-docs/v2.md` raised the second of +# the two errors above through the route-verb path rather than the sweep. # # NAMESPACE: `plugin:skill` is live in this repo (native user-scope installs # still resolve `gitea:gitea-prs`), so the patterns admit an optional @@ -590,7 +640,8 @@ ROUTE_VERB = (r"(?:use|uses|using|run|runs|invoke|invokes|invoking|try|see" r"|that'?s|compose|composes|call|calls" r"|routes?\s+to|delegates?\s+to|prefers?|switch(?:es)?\s+to" r"|hands?\s+off\s+to)") -MARKED_TARGET = r"(?:`/?(%s)`|(?|→)\s*%s" % MARKED_TARGET, re.I) -# The two EXPLICIT ROUTE NOTATION sweeps, scoped to a boundary sentence by their -# caller. NOTATION_SLASH is deliberately not a reuse of MARKED_TARGET's `/name` -# alternative: that one only ever runs behind a route verb or an arrow, and the -# trailing lookahead here is the part that makes a FREE-STANDING sweep safe. +# The two EXPLICIT ROUTE NOTATION sweeps. NOTATION_SLASH runs over EVERY +# sentence; NOTATION_ARROW is scoped to a boundary sentence by its caller (see +# the asymmetry note in the header). NOTATION_SLASH is deliberately not a reuse +# of MARKED_TARGET's `/name` alternative: that one only ever runs behind a route +# verb or an arrow, and it may match a namespaced or path-adjacent token in +# positions this free-standing sweep must refuse. # NOTATION_ARROW is ARROW_BOUNDARY minus its leading `\bnot\b%s*?`, which is # what made `Do not use for Y; -> no-such-skill covers it.` invisible: # CLAUSE_BODY cannot cross the `;`, so the clause's own punctuation disarmed the @@ -618,7 +671,8 @@ ARROW_MARKED = re.compile(r"(?:->|→)\s*%s" % MARKED_TARGET, re.I) # hard ERROR under ARROW_BOUNDARY, so this changes which boundary words reach the # arrow, not whether prose can. An author who means the chain and not a route # writes it in its own sentence, where neither pattern looks. -NOTATION_SLASH = re.compile(r"(?|→)\s*(%s)\b" % NAME_HYPH, re.I) # CLAUSE_BODY is what may sit between `Not` and the arrow, and it is NOT # `[^.;]`. That class cannot cross a `.`, so every boundary clause naming a @@ -794,14 +848,16 @@ def _extract_sentence(sentence): for match in ARROW_BOUNDARY.finditer(sentence): _add(out, sentence, match.group(1), match.start(1), match.end(1), strict=True, arrow=True) + # `/name` wherever it sits, in ANY sentence — not only where a route verb or + # an arrow happens to precede it, and NOT only inside a boundary sentence. + # See the EXPLICIT ROUTE NOTATION note in the header for the eight phrasings + # this recovers and for why silence was the failure mode. The sweep takes no + # follower test: _add() reads the notation first and marks it. + for match in NOTATION_SLASH.finditer(sentence): + _add(out, sentence, match.group(1), match.start(1), match.end(1)) if boundary: - # Route notation wherever it sits in the clause, not only where a route - # verb or an arrow happens to precede it. See the EXPLICIT ROUTE - # NOTATION note in the header for the seven phrasings this recovers and - # for why silence was the failure mode. Neither sweep takes the follower - # test: _add() reads the notation first and both forms reach it marked. - for match in NOTATION_SLASH.finditer(sentence): - _add(out, sentence, match.group(1), match.start(1), match.end(1)) + # The arrow and backtick forms are ambiguous in ordinary prose, so they + # stay scoped to a sentence that carries a boundary marker. for match in NOTATION_ARROW.finditer(sentence): _add(out, sentence, match.group(1), match.start(1), match.end(1), strict=True, arrow=True) diff --git a/plugins/kyberforge/skills/skill-audit/scripts/validate.sh b/plugins/kyberforge/skills/skill-audit/scripts/validate.sh index 8934687..e5ce8f3 100755 --- a/plugins/kyberforge/skills/skill-audit/scripts/validate.sh +++ b/plugins/kyberforge/skills/skill-audit/scripts/validate.sh @@ -59,6 +59,24 @@ import glob import yaml +# Output is UTF-8 for the same reason input is: under LC_ALL=C the streams +# default to ASCII, and this script's own message text carries em dashes (the +# ADR-0020 boundary SUGGESTION is one). Pinning only the reads moved the crash +# from the read to the write — a UnicodeEncodeError raised while PRINTING, after +# every check has already run, which loses the whole report and (here) flips a +# clean exit 0 into a traceback and an exit 1. read_text() in the shared +# resolver block below pins the reads; this pins the writes. +# +# Deliberately OUTSIDE the ADR-0020 shared boundary resolver block: the two +# validate.sh copies print findings, skill-size-check.sh has its own top-level +# equivalent, and tests/test-adr0020-contract.sh hashes that block for +# byte-identity across all three. +for _stream in (sys.stdout, sys.stderr): + try: + _stream.reconfigure(encoding='utf-8') + except AttributeError: # pragma: no cover — Python < 3.7 + pass + skill_dir = os.path.abspath(sys.argv[1]) skill_md = os.path.join(skill_dir, "SKILL.md") @@ -478,9 +496,9 @@ def known_targets(start_dir): # ambiguity to resolve, and an author who wants a route checked unconditionally # has two ways to say so. # -# BOTH FORMS ARE SWEPT FOR ON THEIR OWN inside a boundary sentence, and that is -# a repair of the promise above rather than a widening of it. Until the sweeps -# existed, notation was only ever seen as the OBJECT OF A ROUTE VERB (`use +# BOTH FORMS ARE SWEPT FOR ON THEIR OWN, and that is a repair of the promise +# above rather than a widening of it. Until the sweeps existed, notation was +# only ever seen as the OBJECT OF A ROUTE VERB (`use # /name`) or as the tail of a `not ... ->` clause with no `;` or sentence end in # between. Every one of these therefore exited 0 in total silence — no ERROR, no # SUGGESTION, not even the target's name: @@ -491,6 +509,7 @@ def known_targets(start_dir): # Do not use for Y — defer to /no-such-skill. # Do not use for Y — /no-such-skill. # Do not use for Y; -> no-such-skill covers it. +# For W, /no-such-skill is the right entry point. # The target was never EXTRACTED, so the notation-first rule in _add() had # nothing to apply itself to and the "always blocks" promise was false for the # ordinary way an author writes the thing. The SUGGESTION tier made it worse @@ -499,12 +518,43 @@ def known_targets(start_dir): # visible SUGGESTION into silence — the gate teaching the one edit that blinds # it. # -# The sweeps are gated on the sentence carrying a BOUNDARY_MARKER, the same gate -# the backtick sweep uses, and NOTATION_SLASH refuses a token that is part of a -# PATH: a following `/`, or a `.` followed by a non-space, means -# `references/foo.md`, `docs/a/b.md` or `https://x/y`, not a route. A sentence's -# closing `.` is not followed by a non-space, so `— /no-such-skill.` still -# counts. +# THE TWO SWEEPS ARE GATED DIFFERENTLY, and the asymmetry is the whole point. +# `/name` is Claude Code's invocation syntax and nothing else — no English +# sentence contains one by accident — so the ADR-0020 amendment and +# docs/spec/gates.md both promise it blocks UNCONDITIONALLY, for any name. So +# NOTATION_SLASH is swept over every sentence, boundary marker or not. Gating it +# on BOUNDARY_MARKER made that promise false for the last sentence of +# Do not use for Z — use /real-skill instead. +# For W, /no-such-skill is the right entry point. +# which exited 0 in total silence: the boundary clause is one sentence up, so +# the sweep never looked at the sentence carrying the broken route. Extraction is +# per-sentence by design (corroboration is scoped to one sentence), which is +# exactly what made the gap invisible. +# +# NOTATION_ARROW stays gated on BOUNDARY_MARKER, and so does the backtick sweep. +# Neither form is unambiguous: `-> name` is also how a process chain is written +# ("reproduce -> minimise -> regression-test") and a code span is how a tool, a +# file and a skill are all cited. Ungating either would fire on prose that +# carries no routing intent at all — the false-positive class this whole +# extractor is tuned against. +# +# BOTH `/name` PATTERNS REFUSE A TOKEN THAT IS PART OF A PATH: a following `/`, +# or a `.` followed by a non-space, means `references/foo.md`, `docs/a/b.md` or +# `https://x/y`, not a route. A sentence's closing `.` is not followed by a +# non-space, so `— /no-such-skill.` still counts. +# +# THAT GUARD IS WRITTEN `(?![\w-])` AND NOT `\b`, because `\b` is not a guard at +# all here: it holds after a hyphen, so when the trailing lookahead rejected the +# full segment the engine simply backtracked to a shorter hyphen-terminated +# prefix and reported THAT as a route. Every one of these was a hard blocking +# ERROR naming a skill nobody had written: +# the config lives at /opt-tools/bin/thing. -> 'opt' +# see /api-docs/v2.md for the schema. -> 'api' AND 'api-docs' +# the file /no-such-skill.md documents it. -> 'no-such' +# `(?![\w-])` forbids the shortened prefix outright, so the whole segment is +# rejected as the path it is. MARKED_TARGET carries the same guard: it had no +# trailing lookahead whatsoever, so `see /api-docs/v2.md` raised the second of +# the two errors above through the route-verb path rather than the sweep. # # NAMESPACE: `plugin:skill` is live in this repo (native user-scope installs # still resolve `gitea:gitea-prs`), so the patterns admit an optional @@ -516,7 +566,8 @@ ROUTE_VERB = (r"(?:use|uses|using|run|runs|invoke|invokes|invoking|try|see" r"|that'?s|compose|composes|call|calls" r"|routes?\s+to|delegates?\s+to|prefers?|switch(?:es)?\s+to" r"|hands?\s+off\s+to)") -MARKED_TARGET = r"(?:`/?(%s)`|(?|→)\s*%s" % MARKED_TARGET, re.I) -# The two EXPLICIT ROUTE NOTATION sweeps, scoped to a boundary sentence by their -# caller. NOTATION_SLASH is deliberately not a reuse of MARKED_TARGET's `/name` -# alternative: that one only ever runs behind a route verb or an arrow, and the -# trailing lookahead here is the part that makes a FREE-STANDING sweep safe. +# The two EXPLICIT ROUTE NOTATION sweeps. NOTATION_SLASH runs over EVERY +# sentence; NOTATION_ARROW is scoped to a boundary sentence by its caller (see +# the asymmetry note in the header). NOTATION_SLASH is deliberately not a reuse +# of MARKED_TARGET's `/name` alternative: that one only ever runs behind a route +# verb or an arrow, and it may match a namespaced or path-adjacent token in +# positions this free-standing sweep must refuse. # NOTATION_ARROW is ARROW_BOUNDARY minus its leading `\bnot\b%s*?`, which is # what made `Do not use for Y; -> no-such-skill covers it.` invisible: # CLAUSE_BODY cannot cross the `;`, so the clause's own punctuation disarmed the @@ -544,7 +597,8 @@ ARROW_MARKED = re.compile(r"(?:->|→)\s*%s" % MARKED_TARGET, re.I) # hard ERROR under ARROW_BOUNDARY, so this changes which boundary words reach the # arrow, not whether prose can. An author who means the chain and not a route # writes it in its own sentence, where neither pattern looks. -NOTATION_SLASH = re.compile(r"(?|→)\s*(%s)\b" % NAME_HYPH, re.I) # CLAUSE_BODY is what may sit between `Not` and the arrow, and it is NOT # `[^.;]`. That class cannot cross a `.`, so every boundary clause naming a @@ -720,14 +774,16 @@ def _extract_sentence(sentence): for match in ARROW_BOUNDARY.finditer(sentence): _add(out, sentence, match.group(1), match.start(1), match.end(1), strict=True, arrow=True) + # `/name` wherever it sits, in ANY sentence — not only where a route verb or + # an arrow happens to precede it, and NOT only inside a boundary sentence. + # See the EXPLICIT ROUTE NOTATION note in the header for the eight phrasings + # this recovers and for why silence was the failure mode. The sweep takes no + # follower test: _add() reads the notation first and marks it. + for match in NOTATION_SLASH.finditer(sentence): + _add(out, sentence, match.group(1), match.start(1), match.end(1)) if boundary: - # Route notation wherever it sits in the clause, not only where a route - # verb or an arrow happens to precede it. See the EXPLICIT ROUTE - # NOTATION note in the header for the seven phrasings this recovers and - # for why silence was the failure mode. Neither sweep takes the follower - # test: _add() reads the notation first and both forms reach it marked. - for match in NOTATION_SLASH.finditer(sentence): - _add(out, sentence, match.group(1), match.start(1), match.end(1)) + # The arrow and backtick forms are ambiguous in ordinary prose, so they + # stay scoped to a sentence that carries a boundary marker. for match in NOTATION_ARROW.finditer(sentence): _add(out, sentence, match.group(1), match.start(1), match.end(1), strict=True, arrow=True) diff --git a/scripts/skill-size-check.sh b/scripts/skill-size-check.sh index 619b9ed..36828c1 100755 --- a/scripts/skill-size-check.sh +++ b/scripts/skill-size-check.sh @@ -106,40 +106,52 @@ BODY_MAX_WORDS=900 FAIL=0 -for f in "$@"; do - # NOT a silent skip — see the matching note on the Python side. A broken - # symlink named SKILL.md is storable in git and a directory named SKILL.md - # reaches this hook the same way; both used to make the whole run exit 0 with - # no output at all, which is the one thing this script must never do. - if [[ ! -f "$f" ]]; then - if [[ -d "$f" ]]; then - why="is a directory, not a file" - elif [[ -L "$f" ]]; then - why="is a symlink that does not resolve to a file" - elif [[ -e "$f" ]]; then - why="is not a regular file" - else - why="does not exist" - fi - echo "ERROR: $f $why, so the line and word ceilings could not be measured. A path this hook was handed and could not read does not get to pass in silence." >&2 - FAIL=1 - continue - fi +# ZERO ARGUMENTS IS A USAGE ERROR, exit 2 — not a clean run. +# +# This hook is `pass_filenames: true` in both .pre-commit-config.yaml and +# .pre-commit-hooks.yaml, and pre-commit skips a filename-passing hook entirely +# when nothing matches its `files:` pattern, so it never invokes this script +# with an empty argument list. Every no-argument invocation therefore comes from +# somewhere else — a hand-run command, a wrapper, or a `files:` pattern edited +# into matching nothing — and printing nothing and exiting 0 made all three +# indistinguishable from a clean corpus. A mis-scoped pattern would have +# silently disabled the whole ADR-0020 gate family while every hook reported +# green. +# +# Exit 2, not 1, for the same reason a8cd5e8 split validate-adapter.sh's usage +# exits out: {0,1} are this script's verdict codes (clean / findings), and a +# caller that reads a non-zero exit as "the SKILL.md needs editing" must be able +# to tell a broken invocation from a real finding. +if [[ $# -eq 0 ]]; then + echo "usage: skill-size-check.sh [SKILL.md ...]" >&2 + echo " Measures the agentskills.io spec ceilings and the ADR-0020 context" >&2 + echo " budget for each SKILL.md named on the command line." >&2 + echo " No paths were given. This is a usage error, not a clean run: a hook" >&2 + echo " whose files: pattern matches nothing would otherwise be" >&2 + echo " indistinguishable from a corpus with no findings." >&2 + exit 2 +fi - # The MAX_LINES / MAX_WORDS ceilings are NOT measured here. They used to be, - # in a single awk pass, and that pass was wrong twice over: - # * `read -r lines words <<< "$(awk ...)"` discarded awk's exit status, so a - # file awk could not read yielded empty variables, bash arithmetic read - # them as 0, and both ceilings passed in total silence — the one outcome - # this script forbids itself. - # * awk's NR/NF do not agree with the Python splitlines()/split() that - # skill-audit/scripts/validate.sh uses for the SAME two constants. - # splitlines() also breaks on \x0b \x0c \x1c \x1d \x1e \x85 U+2028 U+2029 - # and split() on every Unicode space, so a body padded with U+2028 read as - # 6 lines here and 606 lines there — hook green, audit FAIL. - # One implementation now owns both: the Python block below already reads every - # file (with a real diagnostic on failure), so it counts there. -done +# An unreadable path — a broken symlink named SKILL.md is storable in git, and a +# directory named SKILL.md reaches this hook the same way — is diagnosed ONCE, +# in the Python per-file loop below. There used to be a bash pre-loop here doing +# exactly the same stat dance and printing exactly the same sentence, so every +# such path was reported twice with two ERROR lines for one broken file. It is +# still NOT a silent skip; the diagnosis simply lives where the file is read. +# +# The MAX_LINES / MAX_WORDS ceilings are not measured in bash either. They used +# to be, in a single awk pass, and that pass was wrong twice over: +# * `read -r lines words <<< "$(awk ...)"` discarded awk's exit status, so a +# file awk could not read yielded empty variables, bash arithmetic read +# them as 0, and both ceilings passed in total silence — the one outcome +# this script forbids itself. +# * awk's NR/NF do not agree with the Python splitlines()/split() that +# skill-audit/scripts/validate.sh uses for the SAME two constants. +# splitlines() also breaks on \x0b \x0c \x1c \x1d \x1e \x85 U+2028 U+2029 +# and split() on every Unicode space, so a body padded with U+2028 read as +# 6 lines here and 606 lines there — hook green, audit FAIL. +# One implementation now owns both: the Python block below already reads every +# file (with a real diagnostic on failure), so it counts there. if ! command -v python3 > /dev/null 2>&1; then echo "ERROR: python3 is required for the ADR-0020 description/body/boundary-target gates but was not found on PATH." >&2 @@ -165,6 +177,23 @@ import sys import yaml +# Output is UTF-8 for the same reason input is: under LC_ALL=C the streams +# default to ASCII, and this script's own message text carries em dashes (the +# ADR-0020 boundary SUGGESTION is one). Pinning only the reads moved the crash +# from the read to the write — a UnicodeEncodeError raised while PRINTING, after +# every check has already run, which loses the whole report. read_text() in the +# shared resolver block below pins the reads; this pins the writes. +# +# Deliberately OUTSIDE the ADR-0020 shared boundary resolver block: all three +# scripts in the family need this, but tests/test-adr0020-contract.sh hashes +# that block for byte-identity, so shared-looking edits belong beside it, not +# inside it. +for _stream in (sys.stdout, sys.stderr): + try: + _stream.reconfigure(encoding='utf-8') + except AttributeError: # pragma: no cover — Python < 3.7 + pass + DESC_SUGGEST_CHARS = int(sys.argv[1]) DESC_MAX_CHARS = int(sys.argv[2]) BODY_SUGGEST_WORDS = int(sys.argv[3]) @@ -580,9 +609,9 @@ def known_targets(start_dir): # ambiguity to resolve, and an author who wants a route checked unconditionally # has two ways to say so. # -# BOTH FORMS ARE SWEPT FOR ON THEIR OWN inside a boundary sentence, and that is -# a repair of the promise above rather than a widening of it. Until the sweeps -# existed, notation was only ever seen as the OBJECT OF A ROUTE VERB (`use +# BOTH FORMS ARE SWEPT FOR ON THEIR OWN, and that is a repair of the promise +# above rather than a widening of it. Until the sweeps existed, notation was +# only ever seen as the OBJECT OF A ROUTE VERB (`use # /name`) or as the tail of a `not ... ->` clause with no `;` or sentence end in # between. Every one of these therefore exited 0 in total silence — no ERROR, no # SUGGESTION, not even the target's name: @@ -593,6 +622,7 @@ def known_targets(start_dir): # Do not use for Y — defer to /no-such-skill. # Do not use for Y — /no-such-skill. # Do not use for Y; -> no-such-skill covers it. +# For W, /no-such-skill is the right entry point. # The target was never EXTRACTED, so the notation-first rule in _add() had # nothing to apply itself to and the "always blocks" promise was false for the # ordinary way an author writes the thing. The SUGGESTION tier made it worse @@ -601,12 +631,43 @@ def known_targets(start_dir): # visible SUGGESTION into silence — the gate teaching the one edit that blinds # it. # -# The sweeps are gated on the sentence carrying a BOUNDARY_MARKER, the same gate -# the backtick sweep uses, and NOTATION_SLASH refuses a token that is part of a -# PATH: a following `/`, or a `.` followed by a non-space, means -# `references/foo.md`, `docs/a/b.md` or `https://x/y`, not a route. A sentence's -# closing `.` is not followed by a non-space, so `— /no-such-skill.` still -# counts. +# THE TWO SWEEPS ARE GATED DIFFERENTLY, and the asymmetry is the whole point. +# `/name` is Claude Code's invocation syntax and nothing else — no English +# sentence contains one by accident — so the ADR-0020 amendment and +# docs/spec/gates.md both promise it blocks UNCONDITIONALLY, for any name. So +# NOTATION_SLASH is swept over every sentence, boundary marker or not. Gating it +# on BOUNDARY_MARKER made that promise false for the last sentence of +# Do not use for Z — use /real-skill instead. +# For W, /no-such-skill is the right entry point. +# which exited 0 in total silence: the boundary clause is one sentence up, so +# the sweep never looked at the sentence carrying the broken route. Extraction is +# per-sentence by design (corroboration is scoped to one sentence), which is +# exactly what made the gap invisible. +# +# NOTATION_ARROW stays gated on BOUNDARY_MARKER, and so does the backtick sweep. +# Neither form is unambiguous: `-> name` is also how a process chain is written +# ("reproduce -> minimise -> regression-test") and a code span is how a tool, a +# file and a skill are all cited. Ungating either would fire on prose that +# carries no routing intent at all — the false-positive class this whole +# extractor is tuned against. +# +# BOTH `/name` PATTERNS REFUSE A TOKEN THAT IS PART OF A PATH: a following `/`, +# or a `.` followed by a non-space, means `references/foo.md`, `docs/a/b.md` or +# `https://x/y`, not a route. A sentence's closing `.` is not followed by a +# non-space, so `— /no-such-skill.` still counts. +# +# THAT GUARD IS WRITTEN `(?![\w-])` AND NOT `\b`, because `\b` is not a guard at +# all here: it holds after a hyphen, so when the trailing lookahead rejected the +# full segment the engine simply backtracked to a shorter hyphen-terminated +# prefix and reported THAT as a route. Every one of these was a hard blocking +# ERROR naming a skill nobody had written: +# the config lives at /opt-tools/bin/thing. -> 'opt' +# see /api-docs/v2.md for the schema. -> 'api' AND 'api-docs' +# the file /no-such-skill.md documents it. -> 'no-such' +# `(?![\w-])` forbids the shortened prefix outright, so the whole segment is +# rejected as the path it is. MARKED_TARGET carries the same guard: it had no +# trailing lookahead whatsoever, so `see /api-docs/v2.md` raised the second of +# the two errors above through the route-verb path rather than the sweep. # # NAMESPACE: `plugin:skill` is live in this repo (native user-scope installs # still resolve `gitea:gitea-prs`), so the patterns admit an optional @@ -618,7 +679,8 @@ ROUTE_VERB = (r"(?:use|uses|using|run|runs|invoke|invokes|invoking|try|see" r"|that'?s|compose|composes|call|calls" r"|routes?\s+to|delegates?\s+to|prefers?|switch(?:es)?\s+to" r"|hands?\s+off\s+to)") -MARKED_TARGET = r"(?:`/?(%s)`|(?|→)\s*%s" % MARKED_TARGET, re.I) -# The two EXPLICIT ROUTE NOTATION sweeps, scoped to a boundary sentence by their -# caller. NOTATION_SLASH is deliberately not a reuse of MARKED_TARGET's `/name` -# alternative: that one only ever runs behind a route verb or an arrow, and the -# trailing lookahead here is the part that makes a FREE-STANDING sweep safe. +# The two EXPLICIT ROUTE NOTATION sweeps. NOTATION_SLASH runs over EVERY +# sentence; NOTATION_ARROW is scoped to a boundary sentence by its caller (see +# the asymmetry note in the header). NOTATION_SLASH is deliberately not a reuse +# of MARKED_TARGET's `/name` alternative: that one only ever runs behind a route +# verb or an arrow, and it may match a namespaced or path-adjacent token in +# positions this free-standing sweep must refuse. # NOTATION_ARROW is ARROW_BOUNDARY minus its leading `\bnot\b%s*?`, which is # what made `Do not use for Y; -> no-such-skill covers it.` invisible: # CLAUSE_BODY cannot cross the `;`, so the clause's own punctuation disarmed the @@ -646,7 +710,8 @@ ARROW_MARKED = re.compile(r"(?:->|→)\s*%s" % MARKED_TARGET, re.I) # hard ERROR under ARROW_BOUNDARY, so this changes which boundary words reach the # arrow, not whether prose can. An author who means the chain and not a route # writes it in its own sentence, where neither pattern looks. -NOTATION_SLASH = re.compile(r"(?|→)\s*(%s)\b" % NAME_HYPH, re.I) # CLAUSE_BODY is what may sit between `Not` and the arrow, and it is NOT # `[^.;]`. That class cannot cross a `.`, so every boundary clause naming a @@ -822,14 +887,16 @@ def _extract_sentence(sentence): for match in ARROW_BOUNDARY.finditer(sentence): _add(out, sentence, match.group(1), match.start(1), match.end(1), strict=True, arrow=True) + # `/name` wherever it sits, in ANY sentence — not only where a route verb or + # an arrow happens to precede it, and NOT only inside a boundary sentence. + # See the EXPLICIT ROUTE NOTATION note in the header for the eight phrasings + # this recovers and for why silence was the failure mode. The sweep takes no + # follower test: _add() reads the notation first and marks it. + for match in NOTATION_SLASH.finditer(sentence): + _add(out, sentence, match.group(1), match.start(1), match.end(1)) if boundary: - # Route notation wherever it sits in the clause, not only where a route - # verb or an arrow happens to precede it. See the EXPLICIT ROUTE - # NOTATION note in the header for the seven phrasings this recovers and - # for why silence was the failure mode. Neither sweep takes the follower - # test: _add() reads the notation first and both forms reach it marked. - for match in NOTATION_SLASH.finditer(sentence): - _add(out, sentence, match.group(1), match.start(1), match.end(1)) + # The arrow and backtick forms are ambiguous in ordinary prose, so they + # stay scoped to a sentence that carries a boundary marker. for match in NOTATION_ARROW.finditer(sentence): _add(out, sentence, match.group(1), match.start(1), match.end(1), strict=True, arrow=True) @@ -1228,7 +1295,9 @@ for path in files: # so pre-commit really can hand one to this hook, and a directory named # SKILL.md reaches it the same way — both used to exit 0 with zero # output, which is precisely the "stay quiet about a measurement it did - # not take" failure this script forbids itself two screens up. + # not take" failure this script forbids itself two screens up. This is + # the ONLY place that diagnosis is made; the bash pre-loop that used to + # duplicate it printed a second ERROR line for the same broken file. if os.path.isdir(path): why = "is a directory, not a file" elif os.path.islink(path): @@ -1250,8 +1319,8 @@ for path in files: # SPEC CONFORMANCE (family 1). Whole file, frontmatter included, counted # with the SAME primitives skill-audit/scripts/validate.sh uses for these - # two constants — see the note in the bash loop above for what the previous - # awk pass got wrong. + # two constants — see the note in bash above for what the previous awk pass + # got wrong. lines = len(raw.splitlines()) words = len(raw.split()) if lines > MAX_LINES: diff --git a/tests/test-adr0020-targets.sh b/tests/test-adr0020-targets.sh index 3247ee5..cd1f20a 100755 --- a/tests/test-adr0020-targets.sh +++ b/tests/test-adr0020-targets.sh @@ -780,6 +780,150 @@ else fail "a populated skills/ghost-skill/ directory still did not resolve (exit $GHOST_RC): ${GHOST_OUT:-}" fi +# --------------------------------------------------------------------------- +# 2g. The FREE-STANDING /name sweep, and its reach beyond a boundary sentence +# --------------------------------------------------------------------------- +# NOTATION_SLASH's own sweep in _extract_sentence() is what sees `/name` when no +# route verb and no arrow precedes it. Nothing pinned it: every `/name` fixture +# in this suite before these cases ALSO carried a route verb ("use +# /no-such-slash-skill instead"), which ROUTE_ANY/ROUTE_MARKED extract on their +# own, so deleting the sweep outright left the whole suite green. The eight +# phrasings below carry no route verb in front of the target, so each of them is +# invisible without the sweep — which is exactly the silence the sweep exists to +# repair, and the shape the SUGGESTION tier's own remedy ("write it as `/name` +# and it will be checked properly") used to teach an author to produce. +echo "" +echo "--- /name with no route verb in front of it is still extracted ---" +grammar_case sweep-dash errors "routes to 'no-such-skill'" \ + "Use when doing the thing. Do not use for Y — /no-such-skill instead." +grammar_case sweep-semicolon errors "routes to 'no-such-skill'" \ + "Use when doing the thing. Do not use for Y; /no-such-skill handles that." +grammar_case sweep-paren errors "routes to 'no-such-skill'" \ + "Use when doing the thing. Do not use for Y (/no-such-skill covers it)." +grammar_case sweep-possessive errors "routes to 'no-such-skill'" \ + "Use when doing the thing. Do not use for Y — that is /no-such-skill's job." +grammar_case sweep-defer errors "routes to 'no-such-skill'" \ + "Use when doing the thing. Do not use for Y — defer to /no-such-skill." +grammar_case sweep-terminal errors "routes to 'no-such-skill'" \ + "Use when doing the thing. Do not use for Y — /no-such-skill." +# The arrow twin. `;` ends CLAUSE_BODY, so ARROW_BOUNDARY cannot reach across it +# from `not`; only NOTATION_ARROW's own sweep sees this one. +grammar_case sweep-arrow-after-semicolon errors "routes to 'no-such-skill'" \ + "Use when doing the thing. Do not use for Y; -> no-such-skill covers it." + +# THE SWEEP IS NOT SCOPED TO A BOUNDARY SENTENCE, and this is the case that +# proves it. Extraction is per-sentence (corroboration is scoped to one +# sentence), so gating the `/name` sweep on the sentence carrying a +# BOUNDARY_MARKER meant a route written one sentence AFTER the boundary clause +# was never looked at: exit 0, no ERROR, no SUGGESTION, not even the name. That +# contradicts ADR-0020's amendment and docs/spec/gates.md, which both promise +# `/name` blocks unconditionally, for any name. +# +# The first sentence's `/sibling-skill` is deliberate: it resolves, so the +# fixture is not "the gate fires on any slash it sees" — it fires on the one +# that dangles, in the sentence that carries no boundary marker at all. +echo "" +echo "--- /name is checked in a sentence that carries no boundary marker ---" +grammar_case sweep-outside-boundary errors "routes to 'no-such-skill'" \ + "Use for X. Do not use for Z — use /sibling-skill instead. For W, /no-such-skill is the right entry point." +# Control, so the case above is not satisfied by a gate that fires on every +# unresolvable-looking token in a non-boundary sentence: the same shape with a +# name that RESOLVES stays silent. +grammar_case sweep-outside-boundary-control silent "" \ + "Use for X. Do not use for Z — use /sibling-skill instead. For W, /sibling-skill is the right entry point." + +# --------------------------------------------------------------------------- +# 2h. A slash PATH is not a route (the trailing guard, and its backtracking) +# --------------------------------------------------------------------------- +# There was no path or URL fixture anywhere in this suite, and the guard was +# defeated by ordinary regex backtracking. `/(NAME_ANY)\b(?!/|\.\S)` looks like +# it refuses a path, and does not: when the lookahead rejects the FULL segment +# the engine backtracks to a shorter hyphen-terminated prefix, `\b` still holds +# after a hyphen, and the phantom is reported as a hard BLOCKING ERROR naming a +# skill nobody wrote: +# /opt-tools/bin/thing -> ERROR: routes to 'opt' +# /api-docs/v2.md -> ERROR: routes to 'api' AND to 'api-docs' +# /no-such-skill.md -> ERROR: routes to 'no-such' +# `(?![\w-])` is the guard that actually holds, because it forbids the shortened +# prefix instead of merely disliking the full one. MARKED_TARGET carries it too: +# that pattern had NO trailing lookahead at all, which is where the second +# 'api-docs' error above came from. +# +# These are `silent`, not `suggests`. A path is not a routing target at any +# tier — reporting one would be the same false positive one notch quieter, on +# the skills most likely to name a path in a boundary clause. +echo "" +echo "--- a slash PATH in a boundary sentence is not a routing target ---" +grammar_case path-absolute silent "" \ + "Use when doing the thing. Do not use for Y; the config lives at /opt-tools/bin/thing." +grammar_case path-dotted-file silent "" \ + "Use when doing the thing. Do not use for Y — see /api-docs/v2.md for the schema." +grammar_case path-dotted-backticked silent "" \ + "Use when doing the thing. Do not use for Y — see \`/api-docs/v2.md\` for the schema." +grammar_case path-md-suffix silent "" \ + "Use when doing the thing. Do not use for Y — the file /no-such-skill.md documents it." +# The two suppressions that were already working and must keep working: a URL +# (the `/` is preceded by a word character or by another `/`) and a relative +# references/ pointer. Asserted explicitly because the guard above is a change to +# the same lookarounds, and a fix that traded one silence for another would look +# identical from the corpus. +grammar_case path-url silent "" \ + "Use when doing the thing. Do not use for Y — see https://example.com/no-such-skill for details." +grammar_case path-relative silent "" \ + "Use when doing the thing. Do not use for Y — see references/no-such-skill.md for details." +# The other direction, which is what stops the guard from becoming a hole: a +# name whose only follower is the SENTENCE-ENDING dot is still a route. A +# closing `.` is not followed by a non-space, so `(?!\.\S)` does not reject it. +# Without these, "refuse every /name near a dot or a slash" would pass every +# case above and silently delete the notation tier. +echo "" +echo "--- the path guard does not swallow a /name at a real sentence end ---" +grammar_case path-guard-sentence-end errors "routes to 'no-such-skill'" \ + "Use when doing the thing. Do not use for Y — defer to /no-such-skill." +grammar_case path-guard-mid-sentence errors "routes to 'no-such-skill'" \ + "Use when doing the thing. Do not use for Y — use /no-such-skill for that instead." + +# --------------------------------------------------------------------------- +# 2i. A DIRECTORY named .md is not an agent +# --------------------------------------------------------------------------- +# The skills branch of _collect_package() tests for a SKILL.md; the agents +# branch takes every `*.md` glob hit on trust, and glob does not distinguish a +# file from a directory. A leftover directory named `ghost-agent.md` — a botched +# `mkdir`, an editor's stray save, a half-deleted agent — is untracked by git, so +# it exists on the machine that made it and nowhere else, and it resolved a +# routing target there and dangled everywhere else. That is exactly the +# install-dependence fixture 2f pins one directory over, and the isfile() guard +# closing it had no test at all: deleting it left every suite green. +echo "" +echo "--- an agents/.md DIRECTORY does not make a routing target resolve ---" +GHOST_AGENT="$TMPDIR_T/ghost-agent-dir" +write_skill "$GHOST_AGENT/plugins/p/.apm/skills/my-skill" my-skill \ + "Use when doing the thing. Do not use for the other thing — use /ghost-agent instead." +mkdir -p "$GHOST_AGENT/plugins/p/.apm/agents/ghost-agent.md" +set +e +GHOST_AGENT_OUT="$(bash "$HOOK" "$GHOST_AGENT/plugins/p/.apm/skills/my-skill/SKILL.md" 2>&1)" +GHOST_AGENT_RC=$? +set -e +if [[ $GHOST_AGENT_RC -ne 0 && "$GHOST_AGENT_OUT" == *"routes to 'ghost-agent'"* ]]; then + pass "a DIRECTORY named ghost-agent.md is not a resolvable agent name" +else + fail "a directory named agents/ghost-agent.md resolved a routing target (exit $GHOST_AGENT_RC): ${GHOST_AGENT_OUT:-}" +fi +# The confirming half, exactly as in 2f: replace the directory with a real file +# and the identical description resolves. Without it the rule could be +# implemented as "agents/ never contributes anything" and still pass above. +rmdir "$GHOST_AGENT/plugins/p/.apm/agents/ghost-agent.md" +: > "$GHOST_AGENT/plugins/p/.apm/agents/ghost-agent.md" +set +e +GHOST_AGENT_OUT="$(bash "$HOOK" "$GHOST_AGENT/plugins/p/.apm/skills/my-skill/SKILL.md" 2>&1)" +GHOST_AGENT_RC=$? +set -e +if [[ $GHOST_AGENT_RC -eq 0 && -z "$GHOST_AGENT_OUT" ]]; then + pass "the same path as a FILE resolves, so the rule is 'not a file' and not 'never'" +else + fail "a real agents/ghost-agent.md file still did not resolve (exit $GHOST_AGENT_RC): ${GHOST_AGENT_OUT:-}" +fi + # And the confirming half of the grammar rule: a compound-modifier target is # CONFIRM-ONLY, not ignored. When the name does exist it still counts as a route # — the rule suppresses the ERROR, it does not delete the target. diff --git a/tests/test-skill-size-check.sh b/tests/test-skill-size-check.sh index 2762e8b..ac1bf64 100755 --- a/tests/test-skill-size-check.sh +++ b/tests/test-skill-size-check.sh @@ -702,6 +702,175 @@ expect_gate "a fixture with no authoring root reports DID NOT RUN and exits 0" \ # # If a real dangling target ever reappears, add its probe back here. +# --------------------------------------------------------------------------- +# Usage tier: zero arguments is exit 2, not a clean run +# --------------------------------------------------------------------------- +# The script used to print nothing and exit 0 when handed no paths, which made +# a mis-scoped `files:` pattern indistinguishable from a corpus with no +# findings — the whole ADR-0020 gate family silently disabled while every hook +# reported green. Exit 2 (not 1) is the same split a8cd5e8 made in +# provider-adapter-author's validate-adapter.sh and the one vale-wrap.sh already +# used: {0,1} are verdicts, 2 is "you invoked this wrong". +# +# SAFE FOR THE HOOK. Both manifests declare pass_filenames: true and neither +# sets always_run, and pre-commit skips a filename-passing hook outright when +# its `files:` pattern matches nothing, so pre-commit never invokes this script +# with an empty argument list. That claim is asserted below rather than left in +# prose, so a config edit that turns it false fails here. +echo "" +echo "--- zero arguments is a usage error (exit 2), not a silent clean run ---" +set +e +USAGE_OUT="$("$SCRIPT" 2>&1)" +USAGE_RC=$? +set -e +if [[ $USAGE_RC -eq 2 ]]; then + pass "no arguments exits 2" +else + fail "no arguments exited $USAGE_RC, expected 2 (output: ${USAGE_OUT:-})" +fi +if [[ "$USAGE_OUT" == *usage* ]]; then + pass "no arguments prints a usage message" +else + fail "no arguments produced no usage message (output: ${USAGE_OUT:-})" +fi +# The exit code must be DISTINCT from both verdicts, or the split buys nothing. +# $SMALL is the clean fixture built at the top of this file; $MANY_LINES is over +# the line ceiling. +set +e +"$SCRIPT" "$SMALL" > /dev/null 2>&1 +CLEAN_RC=$? +"$SCRIPT" "$MANY_LINES" > /dev/null 2>&1 +FINDING_RC=$? +set -e +if [[ $CLEAN_RC -eq 0 && $FINDING_RC -eq 1 && $USAGE_RC -eq 2 ]]; then + pass "the three exit codes are distinct: clean=0, findings=1, usage=2" +else + fail "exit codes collide — clean=$CLEAN_RC findings=$FINDING_RC usage=$USAGE_RC" +fi +# The hook contract the usage exit depends on. If either manifest ever stops +# passing filenames, or starts always_run, pre-commit could invoke the script +# with no paths and exit 2 would break the hook rather than diagnose a caller. +HOOK_CONTRACT="$(python3 - "$REPO_ROOT" <<'PYHOOK' +import os +import sys + +import yaml + +root = sys.argv[1] +problems = [] + + +def check(label, hook): + if hook is None: + problems.append('%s declares no such hook' % label) + return + if hook.get('pass_filenames') is False: + problems.append('%s sets pass_filenames: false' % label) + if hook.get('always_run'): + problems.append('%s sets always_run: true' % label) + + +with open(os.path.join(root, '.pre-commit-config.yaml'), encoding='utf-8') as fh: + cfg = yaml.safe_load(fh) or {} +found = None +for repo in cfg.get('repos') or []: + for hook in (repo.get('hooks') or []): + if hook.get('id') == 'skill-size-check': + found = hook +check('.pre-commit-config.yaml skill-size-check', found) + +with open(os.path.join(root, '.pre-commit-hooks.yaml'), encoding='utf-8') as fh: + hooks = yaml.safe_load(fh) or [] +found = None +for hook in hooks: + if isinstance(hook, dict) and hook.get('id') == 'kyberforge-skill-size-check': + found = hook +check('.pre-commit-hooks.yaml kyberforge-skill-size-check', found) + +print('; '.join(problems)) +PYHOOK +)" +if [[ -z "$HOOK_CONTRACT" ]]; then + pass "both manifests pass filenames and neither is always_run, so pre-commit never invokes the script with no paths" +else + fail "the usage exit would break the hook: $HOOK_CONTRACT" +fi + +# --------------------------------------------------------------------------- +# An unreadable path is diagnosed ONCE +# --------------------------------------------------------------------------- +# The stat dance lived twice — a bash pre-loop and the Python per-file loop — +# and both printed the same sentence, so one broken file produced two ERROR +# lines with two different "so ... could not be measured" clauses. Duplicated +# output on a blocking gate reads as two problems and sends the author hunting +# for a second one. The check must still FIRE (silence is the failure this +# script forbids itself); it must fire exactly once. +echo "" +echo "--- an unreadable path produces exactly one ERROR line, not two ---" +UNREADABLE_DIR="$TMPDIR/unreadable" +mkdir -p "$UNREADABLE_DIR/a-directory.md" +ln -sf "$TMPDIR/definitely-not-here.md" "$UNREADABLE_DIR/broken-link.md" + +# unreadable_case