From bbb0dcd21a2f5fe53e33c7f18247798616b97764 Mon Sep 17 00:00:00 2001 From: Defame1297 Date: Fri, 24 Jul 2026 10:16:35 +0000 Subject: [PATCH] fix(lint): flatten multi-line frontmatter descriptions before Vale runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Vale's text.frontmatter.description scope silently stops matching once the description is a YAML block scalar spanning 2+ physical lines — the style used by most skills/agents in this repo. scripts/vale-wrap.sh flattens the description to one line in a scratch copy (preserving the repo-relative path and total line count) before invoking real vale, and both audit skills plus the pre-commit hook now call it instead of vale directly. Also tightens the pre-commit hook's file glob to single path segments so it can't cross into docs/research examples or asset templates the way the audit skills' scoped invocations already avoid. Addresses PR #85 review feedback. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01FxG5T8EJDgkABXxuneuFfn --- .pre-commit-config.yaml | 6 +- CONTEXT.md | 4 +- .../kyberforge/skills/agent-audit/SKILL.md | 4 +- .../kyberforge/skills/skill-audit/SKILL.md | 4 +- plugins/lint/skills/vale-config/SKILL.md | 1 + scripts/vale-wrap.sh | 79 ++++++++++++++++++ tests/test-vale-wrap.sh | 83 +++++++++++++++++++ 7 files changed, 173 insertions(+), 8 deletions(-) create mode 100755 scripts/vale-wrap.sh create mode 100755 tests/test-vale-wrap.sh diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index cb404ed..586ef59 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -102,9 +102,9 @@ repos: stages: ['pre-commit'] name: Vale audit prefilter description: Run Vale against skill/agent markdown files as a deterministic prefilter for skill-audit/agent-audit - entry: vale --config .vale.ini - language: unsupported - files: '^plugins/.*/(skills/.*/SKILL\.md|agents/.*\.md)$' + entry: scripts/vale-wrap.sh --config .vale.ini + language: script + files: '^plugins/[^/]+/(skills/[^/]+/SKILL\.md|agents/[^/]+\.md)$' pass_filenames: true - repo: meta diff --git a/CONTEXT.md b/CONTEXT.md index b9c1320..8e9bfe8 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -70,7 +70,9 @@ A companion skill (`core` plugin) that detects a target repo's provider-specific A standalone, repo-agnostic plugin (`plugins/lint/`) for configuring and running linters — not scoped to kyberforge's own meta-tooling. First linter is Vale (prose style linting), split into two skills per the git/gitea per-concern pattern: `vale-config` (setup — `.vale.ini`, `StylesPath`, styles) and `vale-run` (invoke Vale, interpret/report findings). A `lint-runner` agent composes these for isolated-context lint sweeps; it is report-only (no `Edit` tool) — it flags findings, it does not rewrite prose. Vale's research docs (`docs/research/docs/vale/`) moved from `plugins/kyberforge/` to `plugins/lint/` to keep the provenance chain same-plugin. ### Vale audit prefilter (skill-audit / agent-audit) -Wiring Vale as a deterministic prefilter for `skill-audit`/`agent-audit`'s Description dimension (ADR motivation: issue #84) is repo-specific, not part of the generic `lint` plugin, so its config lives at the repo root rather than inside `plugins/lint/`: `.vale.ini` plus a custom `Kyberforge` style (`styles/Kyberforge/`) covering description-opener banning ("This skill/agent..."), vague-capability wording ("helps with", "utilize", ...), and generic "see references/ for details" padding — and a `KyberforgeCopilot` style (`styles/KyberforgeCopilot/`) scoped only to `.agent.md` files for the Copilot-only "Use proactively has no effect" check. Both skills' Step 1 run `vale --config .vale.ini ` against the specific file(s) being audited (never a repo-wide sweep) — Vale's glob matching crosses directory boundaries (`plugins/*/agents/*.md` matches nested `docs/research/examples/**/agents/*.md` too), so scoping every invocation to a known target file/dir is what keeps research-example files out of the audit's lint pass rather than the glob pattern itself. `error` alerts map to FAIL, `warning`/`suggestion` map to SUGGESTION. Vale only replaces the specific pattern-matchable sub-checks named in issue #84 (imperative opener, vague filler, `Use proactively`, generic reference-pointer padding) — body discipline, near-miss exclusion strength, and control calibration stay LLM judgment per the issue's explicit non-goals. +Wiring Vale as a deterministic prefilter for `skill-audit`/`agent-audit`'s Description dimension (ADR motivation: issue #84) is repo-specific, not part of the generic `lint` plugin, so its config lives at the repo root rather than inside `plugins/lint/`: `.vale.ini` plus a custom `Kyberforge` style (`styles/Kyberforge/`) covering description-opener banning ("This skill/agent..."), vague-capability wording ("helps with", "utilize", ...), and generic "see references/ for details" padding — and a `KyberforgeCopilot` style (`styles/KyberforgeCopilot/`) scoped only to `.agent.md` files for the Copilot-only "Use proactively has no effect" check. `error` alerts map to FAIL, `warning`/`suggestion` map to SUGGESTION. Vale only replaces the specific pattern-matchable sub-checks named in issue #84 (imperative opener, vague filler, `Use proactively`, generic reference-pointer padding) — body discipline, near-miss exclusion strength, and control calibration stay LLM judgment per the issue's explicit non-goals. + +Both skills' Step 1, and the `vale-audit-prefilter` pre-commit hook, call `scripts/vale-wrap.sh` rather than `vale` directly — a workaround for a confirmed Vale 3.15.2 limitation (see `vale-config`'s Gotchas): `text.frontmatter.description` silently stops matching once the description is a YAML block scalar (`>`/`|`) spanning 2+ physical lines, which is how most skills/agents in this repo write it. The wrapper flattens the description to one physical line in a scratch copy (padding with blank lines so every other line number is unchanged) before handing off to real `vale`; single-line descriptions pass through untouched. `tests/test-vale-wrap.sh` regression-tests this. Both call sites still scope every invocation to the specific file(s) being audited, never a repo-wide sweep — Vale's glob matching crosses directory boundaries (`plugins/*/agents/*.md` matches nested `docs/research/examples/**/agents/*.md` too), so scoping is what keeps research-example files out of the audit's lint pass. The pre-commit hook's own glob is tightened to `^plugins/[^/]+/(skills/[^/]+/SKILL\.md|agents/[^/]+\.md)$` (single-segment, not `.*`) for the same reason, since pre-commit invokes it automatically against whatever staged files match rather than a manually-scoped target. ### LESSONS.md Long-loop feedback log for patterns observed across sessions. Three or more entries on the same pattern graduate to the relevant standing file (e.g. a coding convention, a governance rule). Updated by the session-handoff skill or directly by the human. Lives at the repo root. diff --git a/plugins/kyberforge/skills/agent-audit/SKILL.md b/plugins/kyberforge/skills/agent-audit/SKILL.md index 2897dd3..6c5786b 100644 --- a/plugins/kyberforge/skills/agent-audit/SKILL.md +++ b/plugins/kyberforge/skills/agent-audit/SKILL.md @@ -36,12 +36,12 @@ metadata: ```bash bash scripts/validate.sh bash scripts/validate-provenance.sh -vale --config .vale.ini +scripts/vale-wrap.sh --config .vale.ini ``` The script accepts either the CC file or the Copilot file. It detects provider from extension, derives the counterpart, and runs all structural checks. Note FAILs and SUGGESTIONs for the `### Structure` and `### Provider safety` report dimensions. Findings about missing fields, bad name format, empty body, or missing frontmatter → `### Structure`. Findings about CC-only fields in a Copilot file, Copilot-only fields in a CC file, plugin-silently-ignored fields, body length, or subagent-unavailable tools → `### Provider safety`. A missing counterpart file → `### Pair consistency`. -Run `vale` from the repo root, against both files of the pair (not just the one passed in), using the repo-root `.vale.ini`. It applies the `Kyberforge` style to both files and the `KyberforgeCopilot` style to the `.agent.md` file only — that split is how the Copilot-only `Use proactively` check stays scoped to the Copilot file without a manual per-file judgment call. Map `error` alerts to `FAIL` and `warning`/`suggestion` alerts to `SUGGESTION` in the `### Description` / `### Body` dimensions below, citing the rule ID (e.g. `KyberforgeCopilot.ProactivePhrase`). If `vale` is not installed or `.vale.ini` is missing, skip this and fall back to the manual judgment calls in Step 2 — do not block the audit on tooling absence. +Run `vale-wrap.sh` from the repo root against both files of the pair (not just the one passed in), using `.vale.ini`. `Kyberforge` applies to both files; `KyberforgeCopilot` applies to the `.agent.md` file only, since its one rule (`Use proactively`) flags CC-specific phrasing that's meaningless in a Copilot description — there's nothing to flag in the CC file, so it isn't scoped there. Map `error` → `FAIL` and `warning`/`suggestion` → `SUGGESTION` in the `### Description` / `### Body` dimensions, citing the rule ID (e.g. `KyberforgeCopilot.ProactivePhrase`). Skip and fall back to Step 2 judgment if vale or `.vale.ini` is unavailable. `validate-provenance.sh` validates the provenance chain between the agent pair's `source_keys` and the plugin-scoped `sources.md` (plugin root — see ADR-0010). It exits 0 silently for non-plugin-scope agents and when no provenance data exists. Note FAILs from this script for the `### Provenance` dimension — surface them verbatim with Why and Fix. diff --git a/plugins/kyberforge/skills/skill-audit/SKILL.md b/plugins/kyberforge/skills/skill-audit/SKILL.md index 8923bb0..18c2084 100644 --- a/plugins/kyberforge/skills/skill-audit/SKILL.md +++ b/plugins/kyberforge/skills/skill-audit/SKILL.md @@ -34,14 +34,14 @@ metadata: ```bash bash scripts/validate.sh bash scripts/validate-provenance.sh -vale --config .vale.ini /SKILL.md +scripts/vale-wrap.sh --config .vale.ini /SKILL.md ``` Note any structural FAILs — they will appear in the report as a `### Structure` dimension. If the script cannot execute (python3 unavailable, Bash denied, or permission error), perform structural checks manually: name format, name matches directory, description length ≤1024 chars, SKILL.md ≤500 lines, no unfilled `FILL IN:` placeholders, scripts executable and free of interactive prompts. Note any Provenance FAILs and INFO findings from `validate-provenance.sh` — they surface in the report as a `### Provenance` dimension (separate from `### Structure`). The script embeds full FAIL/INFO format with Why and Fix per finding; surface them verbatim. -`vale` runs from the repo root against the target `SKILL.md` using the repo-root `.vale.ini` (custom `Kyberforge` style) — it is a deterministic prefilter for a subset of the Description and Patterns dimensions below, not a replacement for Step 3's qualitative pass. Map `error` alerts to `FAIL` and `warning`/`suggestion` alerts to `SUGGESTION` in the `### Description` / `### Patterns` report dimensions, citing the rule ID (e.g. `Kyberforge.DescriptionOpener`) as the finding. If `vale` is not installed or `.vale.ini` is missing, skip this and fall back to the manual judgment calls described in Step 3 — do not block the audit on tooling absence. +`vale-wrap.sh` runs from the repo root using `.vale.ini`'s `Kyberforge` style — a deterministic prefilter for a subset of the Description/Patterns dimensions below, not a replacement for Step 3. Map `error` → `FAIL` and `warning`/`suggestion` → `SUGGESTION` in those dimensions, citing the rule ID (e.g. `Kyberforge.DescriptionOpener`). Skip and fall back to Step 3 judgment if vale or `.vale.ini` is unavailable. ## Step 2 — Read all skill files diff --git a/plugins/lint/skills/vale-config/SKILL.md b/plugins/lint/skills/vale-config/SKILL.md index 43591c4..731b361 100644 --- a/plugins/lint/skills/vale-config/SKILL.md +++ b/plugins/lint/skills/vale-config/SKILL.md @@ -22,6 +22,7 @@ metadata: - Installing the `vale` binary installs no styles. A fresh `.vale.ini` with `BasedOnStyles` set will fail or find nothing until `vale sync` runs and downloads the `Packages` it declares. - `.vale.ini` is order-sensitive: global (core) settings first, then the optional `[formats]` section, then glob sections (`[*]`, `[*.md]`, …). Settings in a glob section only apply to files matching that glob. - `Packages` (top-level, fetched by `vale sync`) and `BasedOnStyles` (per-glob, activates) are separate keys — a style only lints files once it's in both. This is the step people forget. +- A rule scoped to `text.frontmatter.` (e.g. `text.frontmatter.description`) only reliably matches when that field's value is a single physical line. If it's a YAML block scalar (`>`/`|`) spanning 2+ physical lines, the scope silently stops matching — no error, just 0 findings — confirmed against Vale 3.15.2. Verify with a deliberately-bad multi-line fixture before trusting a frontmatter-scoped rule in production; if the field is commonly authored as a multi-line block scalar, flatten it to one line ahead of the `vale` call rather than relying on the scope alone. ## Setup workflow diff --git a/scripts/vale-wrap.sh b/scripts/vale-wrap.sh new file mode 100755 index 0000000..67b69fa --- /dev/null +++ b/scripts/vale-wrap.sh @@ -0,0 +1,79 @@ +#!/usr/bin/env bash +set -euo pipefail + +# Works around a Vale limitation: the `text.frontmatter.description` NLP scope +# silently stops matching once the `description:` value is a YAML block scalar +# (`>`/`|`) spanning 2+ physical lines — the style used by most skills/agents in +# this repo. Flattens the description to one physical line in a scratch copy +# (padding with blank lines so every other line number is unchanged), then runs +# the real `vale` binary against the copies. Drop-in replacement for calling +# `vale` directly: same args, same exit code. + +repo_root="$(git rev-parse --show-toplevel 2>/dev/null || pwd)" + +vale_args=() +files=() +config_next=false +for arg in "$@"; do + if [[ "$config_next" == true ]]; then + if [[ "$arg" == /* ]]; then + vale_args+=("$arg") + else + vale_args+=("$repo_root/$arg") + fi + config_next=false + continue + fi + if [[ "$arg" == "--config" ]]; then + vale_args+=("$arg") + config_next=true + continue + fi + if [[ "$arg" != -* && -f "$repo_root/$arg" ]]; then + files+=("$arg") + else + vale_args+=("$arg") + fi +done + +if [[ ${#files[@]} -eq 0 ]]; then + exec vale "${vale_args[@]}" +fi + +tmpdir="$(mktemp -d)" +trap 'rm -rf "$tmpdir"' EXIT + +for rel in "${files[@]}"; do + dest="$tmpdir/$rel" + mkdir -p "$(dirname "$dest")" + python3 - "$repo_root/$rel" "$dest" <<'PYTHON' +import json +import re +import sys + +src, dest = sys.argv[1], sys.argv[2] +with open(src) as fh: + content = fh.read() + +fm_match = re.match(r'^(---\n)(.*?\n)(---\n)', content, re.DOTALL) +if fm_match: + fm = fm_match.group(2) + desc_m = re.search(r'^description:\s*([>|][+-]?)\n((?:[ \t]+.+\n?)+)', fm, re.MULTILINE) + if desc_m and desc_m.group(2).count('\n') >= 2: + raw = desc_m.group(2) + flat = re.sub(r'\s+', ' ', raw).strip() + # JSON string escaping is a valid subset of YAML double-quoted scalar + # escaping, so this is always a well-formed YAML value regardless of + # colons, quotes, or backslashes in the description text. + flat_q = json.dumps(flat) + pad = '\n' * raw.count('\n') + new_fm = fm[:desc_m.start()] + f'description: {flat_q}\n{pad}' + fm[desc_m.end():] + content = fm_match.group(1) + new_fm + fm_match.group(3) + content[fm_match.end():] + +with open(dest, 'w') as fh: + fh.write(content) +PYTHON +done + +cd "$tmpdir" +vale "${vale_args[@]}" "${files[@]}" diff --git a/tests/test-vale-wrap.sh b/tests/test-vale-wrap.sh new file mode 100755 index 0000000..729c0b5 --- /dev/null +++ b/tests/test-vale-wrap.sh @@ -0,0 +1,83 @@ +#!/usr/bin/env bash +# Regression test for scripts/vale-wrap.sh: Vale's `text.frontmatter.description` +# NLP scope silently stops matching when the description value is a YAML block +# scalar spanning 2+ physical lines. vale-wrap.sh flattens it to one line before +# handing off to the real vale binary — this asserts that actually happens. +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +SCRIPT="$REPO_ROOT/scripts/vale-wrap.sh" +PASS=0 +FAIL=0 + +pass() { echo " PASS: $1"; PASS=$((PASS + 1)); } +fail() { echo " FAIL: $1"; FAIL=$((FAIL + 1)); } + +if ! command -v vale &>/dev/null; then + echo "vale is not installed — skipping (matches skill-audit/agent-audit's own fallback behavior)" + exit 0 +fi + +make_fixture() { + local dir desc_lines file + dir="$(mktemp -d)" + (cd "$dir" && git init -q) + mkdir -p "$dir/plugins/testplugin/skills/zzzskill" + desc_lines="$1" + file="$dir/plugins/testplugin/skills/zzzskill/SKILL.md" + { + echo "---" + echo "name: zzzskill" + echo "description: >" + for ((i = 1; i <= desc_lines; i++)); do + echo " Line $i mentions helps with and utilize, plus a colon: like this." + done + echo "---" + echo "" + echo "Body." + } > "$file" + echo "$dir" +} + +# --- 1. A known-bad single-line description is caught (sanity check on Vale itself) --- +echo "" +echo "--- catches vague wording in a single-line description ---" +FIXTURE1="$(make_fixture 1)" +trap 'rm -rf "$FIXTURE1"' EXIT +if (cd "$FIXTURE1" && bash "$SCRIPT" --config "$REPO_ROOT/.vale.ini" \ + plugins/testplugin/skills/zzzskill/SKILL.md) | grep -q "VagueWording"; then + pass "flags vague wording when description is a single physical line" +else + fail "did not flag known-bad single-line description" +fi + +# --- 2. The same known-bad wording across 2+ physical lines is still caught --- +echo "" +echo "--- catches vague wording in a multi-line folded description ---" +FIXTURE2="$(make_fixture 2)" +trap 'rm -rf "$FIXTURE1" "$FIXTURE2"' EXIT +if (cd "$FIXTURE2" && bash "$SCRIPT" --config "$REPO_ROOT/.vale.ini" \ + plugins/testplugin/skills/zzzskill/SKILL.md) | grep -q "VagueWording"; then + pass "flags vague wording when description spans 2+ physical lines" +else + fail "silently missed known-bad wording in a multi-line description — the bug this test guards against" +fi + +# --- 3. Line count is preserved so unrelated report line numbers don't shift --- +echo "" +echo "--- preserves total line count when flattening ---" +FIXTURE3="$(make_fixture 3)" +trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3"' EXIT +ORIG_LINES=$(wc -l < "$FIXTURE3/plugins/testplugin/skills/zzzskill/SKILL.md") +OUT=$(cd "$FIXTURE3" && bash "$SCRIPT" --config "$REPO_ROOT/.vale.ini" \ + plugins/testplugin/skills/zzzskill/SKILL.md 2>&1 || true) +MAX_LINE=$(echo "$OUT" | grep -oE '^[[:space:]]*[0-9]+:[0-9]+' | tr -d '[:space:]' | cut -d: -f1 | sort -n | tail -1) +if [[ -n "$MAX_LINE" ]] && (( MAX_LINE <= ORIG_LINES )); then + pass "reported line numbers stay within the original file's line count" +else + fail "reported line number ($MAX_LINE) exceeds original file line count ($ORIG_LINES)" +fi + +echo "" +echo "Results: $PASS passed, $FAIL failed" +[[ $FAIL -eq 0 ]]