diff --git a/plugins/kyberforge/.apm/skills/factory-audit/SKILL.md b/plugins/kyberforge/.apm/skills/factory-audit/SKILL.md index e9050f8..3fed83d 100644 --- a/plugins/kyberforge/.apm/skills/factory-audit/SKILL.md +++ b/plugins/kyberforge/.apm/skills/factory-audit/SKILL.md @@ -7,7 +7,7 @@ description: > fixes -> agent-author. allowed-tools: Bash Read metadata: - version: "1.0.2" + version: "1.0.3" category: factory source_keys: - agentskills-home diff --git a/plugins/kyberforge/.apm/skills/factory-audit/assets/vale/styles/Kyberforge/CompositionNote.yml b/plugins/kyberforge/.apm/skills/factory-audit/assets/vale/styles/Kyberforge/CompositionNote.yml index 90ea015..26144b4 100644 --- a/plugins/kyberforge/.apm/skills/factory-audit/assets/vale/styles/Kyberforge/CompositionNote.yml +++ b/plugins/kyberforge/.apm/skills/factory-audit/assets/vale/styles/Kyberforge/CompositionNote.yml @@ -1,5 +1,5 @@ extends: existence -message: "Composition or architecture note in a description: '%s' — a description carries a trigger, one capability clause and a boundary clause only; move this to README.md" +message: "Composition or architecture note in a description: '%s' — a description carries a trigger, one capability clause and a boundary clause only; move this to the body or a references/ file" level: error scope: text.frontmatter.description ignorecase: true diff --git a/plugins/kyberforge/.apm/skills/factory-audit/references/agent-description-quality.md b/plugins/kyberforge/.apm/skills/factory-audit/references/agent-description-quality.md index 99bfe6f..e633103 100644 --- a/plugins/kyberforge/.apm/skills/factory-audit/references/agent-description-quality.md +++ b/plugins/kyberforge/.apm/skills/factory-audit/references/agent-description-quality.md @@ -55,7 +55,8 @@ A model-invoked description carries exactly three things: 3. **Boundary clause.** Compressed form: `Not -> .` The target must resolve to a real skill directory or agent file in the authoring source. -Everything else belongs in the body or in the plugin's `README.md`. +Everything else belongs in the body or in a `references/` file. Not a `README.md`: `plugins/gitea/` +and `plugins/lint/` both ship agents this file governs and neither has one. ## Indirect triggers — conditional, never blanket diff --git a/plugins/kyberforge/.apm/skills/factory-audit/scripts/lib-boundary-resolver.sh b/plugins/kyberforge/.apm/skills/factory-audit/scripts/lib-boundary-resolver.sh index 834708f..be8168c 100755 --- a/plugins/kyberforge/.apm/skills/factory-audit/scripts/lib-boundary-resolver.sh +++ b/plugins/kyberforge/.apm/skills/factory-audit/scripts/lib-boundary-resolver.sh @@ -732,8 +732,41 @@ def boundary_targets(description): return sorted({name for name, _, _ in _extract(description)}) -def _arrow_targets(description): - """Names extracted from ARROW notation specifically. +def _clause_end(description, pos): + """Where CLAUSE_BODY stops scanning forward from `pos`. + + The same two stops the class itself encodes: a `;`, or a `.` that is not + followed by a non-space character (a sentence end rather than a dot inside + `AGENTS.md`). + """ + for index in range(pos, len(description)): + char = description[index] + if char == ';': + return index + if char == '.' and not description[index + 1:index + 2].strip(): + return index + return len(description) + + +def _arrow_clause_spans(description): + """(start, end) for EACH ADR-0020 arrow clause, one span per clause. + + A clause runs from its `Not` to whichever comes first: the start of the + NEXT arrow clause, or the end of the clause body. Bounding on the next + clause is what keeps two clauses joined by a comma inside one sentence + apart — a sentence-scoped span would merge them and let the second clause's + target vouch for the first. + """ + starts = [match.start() for match in BOUNDARY_ARROW.finditer(description)] + spans = [] + for index, start in enumerate(starts): + limit = starts[index + 1] if index + 1 < len(starts) else len(description) + spans.append((start, min(limit, _clause_end(description, start)))) + return spans + + +def _arrow_clause_parses(clause): + """True when either arrow extractor reads a target out of ONE clause. Kept apart from boundary_targets() because the arrow form is the one shape that ALWAYS names a target: ADR-0020's `Not -> `. A clause @@ -741,15 +774,11 @@ def _arrow_targets(description): that deserves its own message, and telling it apart needs the arrow targets alone rather than every target in the description. """ - out = [] - for sentence in SENTENCE_SPLIT.split(description): - for match in ARROW_MARKED.finditer(sentence): - name, _, _ = _first(match) - if name: - out.append(name) - for match in ARROW_BOUNDARY.finditer(sentence): - out.append(match.group(1)) - return out + for match in ARROW_MARKED.finditer(clause): + name, _, _ = _first(match) + if name: + return True + return bool(ARROW_BOUNDARY.search(clause)) def boundary_clause_status(description): @@ -761,16 +790,28 @@ def boundary_clause_status(description): three of them reworded a correct clause to satisfy a regex instead. 'unparsed' is the narrow, certain case: an ADR-0020 arrow clause was - detected and NO target came out of it. The arrow form always names one, so + detected and NO target came out of IT. The arrow form always names one, so zero targets means the name is written in a shape the extractor cannot see — a single-word bare target (`Not X -> forge`, which has to be written `` `forge` `` or `/forge`) is the live example, since single-word names are deliberately not matchable bare. + The test is PER CLAUSE, and that is the whole point of the span walk. Both + operands used to take the whole description, so ONE arrow clause that + parsed suppressed the diagnostic for every other clause beside it: a + backticked hyphenated target wrapped across a line break inside a `>` + folded scalar — `` `git-`` / ``commits` `` — went unchecked with no ERROR + and no SUGGESTION, while the same wrap written bare was reported correctly. + 26 of this corpus's 38 skill descriptions carry more than one arrow clause, + so the suppression covered most of it. This is the issue #100 regression + class, and a whole-description test cannot see it by construction. + A PROSE clause yielding no target is NOT reported: "Do not use for anything else" is a complete and legitimate boundary clause that names nowhere to go. """ - if BOUNDARY_ARROW.search(description) and not _arrow_targets(description): + spans = _arrow_clause_spans(description) + if spans and not all(_arrow_clause_parses(description[start:end]) + for start, end in spans): return 'unparsed' if has_boundary_clause(description): return 'present' diff --git a/plugins/kyberforge/.apm/skills/factory-audit/scripts/vale-wrap.sh b/plugins/kyberforge/.apm/skills/factory-audit/scripts/vale-wrap.sh index 98c0fc7..f862407 100755 --- a/plugins/kyberforge/.apm/skills/factory-audit/scripts/vale-wrap.sh +++ b/plugins/kyberforge/.apm/skills/factory-audit/scripts/vale-wrap.sh @@ -38,7 +38,7 @@ set -euo pipefail # Divergence 2: a path-shaped argument that does not exist is a hard error # (exit 2). Bare vale drops it, falls back to reading stdin, and prints # `0 errors ... in stdin` with exit 0 — a typo'd target is then indistinguishable -# from a clean run. Both audit skills treat a `0 files` report as NOT RUN rather +# from a clean run. factory-audit treats a `0 files` report as NOT RUN rather # than clean, and `in stdin` does not match that guard, so the silent form would # read as "prefilter clean" and skip the LLM fallback. Erroring is the only way # to keep that guard honest. Linting prose piped on stdin is therefore diff --git a/plugins/kyberforge/.apm/skills/factory-audit/tests/validate-skill.bats b/plugins/kyberforge/.apm/skills/factory-audit/tests/validate-skill.bats index f6983c7..9ab0ac0 100644 --- a/plugins/kyberforge/.apm/skills/factory-audit/tests/validate-skill.bats +++ b/plugins/kyberforge/.apm/skills/factory-audit/tests/validate-skill.bats @@ -945,6 +945,48 @@ make_hand_invoked_skill() { assert_output --partial "no target could be read" } +# --------------------------------------------------------------------------- +# ADR-0020 — the unparsed diagnostic is PER CLAUSE, not per description +# +# boundary_clause_status() used to test `BOUNDARY_ARROW.search(description) and +# not _arrow_targets(description)`. Both operands took the WHOLE description, +# so ONE arrow clause that parsed suppressed the diagnostic for every other +# clause beside it. +# +# The shape that hides there is a backticked hyphenated target wrapped across +# the line break of a `>` folded scalar: the fold turns `` `fixture-sibling- `` +# / `` skill` `` into `fixture-sibling- skill`, which no extractor can read. +# Written BARE the same wrap is reported correctly, so the two spellings +# disagreed. 26 of this repo's 38 skill descriptions carry more than one arrow +# clause, which is how wide the suppression was. This is the #100 regression +# class: no ERROR, no SUGGESTION, exit 0. +# --------------------------------------------------------------------------- + +@test "ADR-0020: an unparsed arrow clause is reported even when a sibling clause parses" { + local skill + skill="$(make_fixture_tree "$TMPDIR/tree" "my-skill")" + # Written by hand rather than through make_sized_skill: the `>` folded + # scalar and the wrap INSIDE the backticks are the fixture. The second + # clause parses and resolves against the fixture sibling, and that is what + # used to silence the first. + cat > "$skill/SKILL.md" <<'EOF' +--- +name: my-skill +description: > + Use when doing the thing. Not the other thing -> `fixture-sibling- + skill`. Not a third thing -> `fixture-sibling-skill`. +metadata: + version: "1.0.0" +--- + +word word word word word word word word word word +EOF + run bash "$SCRIPT" "$skill" + assert_success + assert_output --partial "no target could be read" + refute_output --partial "has no boundary clause" +} + # --------------------------------------------------------------------------- # Encoding, write side: sys.stdout/stderr.reconfigure(encoding='utf-8') # diff --git a/tests/test-adr0020-targets.sh b/tests/test-adr0020-targets.sh index f012baa..f4b5563 100755 --- a/tests/test-adr0020-targets.sh +++ b/tests/test-adr0020-targets.sh @@ -442,6 +442,24 @@ fi # terminal and therefore danglable. The issue #99 retrofit cut that composition # sentence and the dangling target went with it, so the set is down to one. # +# Correction (2026-09-20): the historical text carried NO backticks. `7801589^` +# has gitea-issues' description as "Composes gitea-labels-\n milestones for all +# label inference/resolution and milestone lookup", bare, so the token was read +# by the route-verb path — `Composes` is a ROUTE_VERB and the name matched +# NAME_HYPH — and not by the backtick sweep. Everything the paragraph above says +# about the fold and the trailing hyphen holds; only the spelling is wrong. +# +# The spelling is the load-bearing part, because the two are not equally +# visible. Backticked, that same wrap reaches every extractor as +# `` `gitea-labels- milestones` ``, which none of them can read: the opening +# backtick blocks the bare NAME_HYPH alternative and the space inside blocks the +# backticked one. Bare, it was extracted and reported all along, which is the +# only reason this dangling target was ever measured. In an ARROW clause the +# backticked wrap was silent until the 2026-09-20 fix to +# boundary_clause_status() in the shared resolver made the unparsed diagnostic +# per clause: before it, one sibling clause that parsed suppressed the finding +# for the whole description. +# # `neuledge-context` was the last one. The issue #99 wave-3 retrofit deleted that # boundary clause outright — commit `6146120` had already deleted the skill it # named, and nothing has owned MCP-server installation since — so the corpus