Compare commits
22 Commits
cc5f366450
...
v1.0.0
| Author | SHA1 | Date | |
|---|---|---|---|
| 8f523da270 | |||
| cf5de2bd87 | |||
| 76e0df6f5b | |||
| 389a4f0f7a | |||
| e62f68a1cc | |||
| 680aa4f43c | |||
| 6910f1b5a5 | |||
| 050aec4c80 | |||
| 0a41b2c7d3 | |||
| 9a3f72b696 | |||
| 7cf9a98509 | |||
| 997f0df23b | |||
| 302f6d0c19 | |||
| f6eb0d295e | |||
| ad1e5aaa9b | |||
| d25355077f | |||
| 57654c4b02 | |||
| 4ae2429840 | |||
| aa8cc22695 | |||
| afc2b7fdfd | |||
| 16c038b178 | |||
| 14c2c91521 |
@@ -13,7 +13,7 @@ This repo dogfoods its own plugins. Before shelling out to git, gitea, or lint t
|
||||
|
||||
- Commits, branches, history, worktrees, remotes → `git:git-commits`, `git:git-branches`, `git:git-history`, `git:git-worktrees`, `git:git-remotes`
|
||||
- Pre-commit hook install/config/troubleshooting → `git:pc-run` / `git:pc-author`
|
||||
- Issues, PRs, labels, milestones → `bin:gitea`
|
||||
- Issues, PRs, labels, milestones → `gitea:gitea-issues`, `gitea:gitea-prs`, `gitea:gitea-labels-milestones`; also `gitea:gitea-branches`, `gitea:gitea-files`, `gitea:gitea-releases`, or `gitea:gitea-workflow` when the domain is ambiguous
|
||||
- Vale prose linting → `lint:vale-config` / `lint:vale-run`
|
||||
- This repo's own AGENTS.md → `core:agentsmd-author` / `core:agentsmd-audit`
|
||||
|
||||
|
||||
@@ -72,9 +72,9 @@ A standalone, repo-agnostic plugin (`plugins/lint/`) for configuring and running
|
||||
### 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 it doesn't live in `plugins/lint/` — but per ADR-0014 it also doesn't live at the repo root anymore. Two copies live inside `plugins/kyberforge/`, one per skill, since a plugin's cache-install only copies each skill's own files (no cross-skill sharing): `plugins/kyberforge/skills/agent-audit/assets/vale/` is canonical (`.vale.ini` plus a custom `Kyberforge` style covering description-opener banning ("This skill/agent..."), vague-capability wording ("helps with", "utilize", ...), and generic "see references/ for details" padding — and a `KyberforgeCopilot` style scoped only to `.agent.md` files for the Copilot-only "Use proactively has no effect" check), and `plugins/kyberforge/skills/skill-audit/assets/vale/` is a smaller duplicate (`Kyberforge` only, scoped to `SKILL.md`) kept in sync by `scripts/check-vale-style-sync.sh` (pre-push). A root-level `.pre-commit-hooks.yaml` exposes both copies (plus `skill-size-check`) so any external repo can enforce the same rules via `repo: <this-repo-url>, rev: <tag>` in its own `.pre-commit-config.yaml` — pre-commit clones the pinned rev into its own cache, independent of whether Claude Code or the `kyberforge` plugin is installed at all, and the same mechanism covers CI (`pre-commit run --all-files`). This repo's own `vale-audit-prefilter-skill`/`-agent` pre-commit hooks consume the identical plugin-bundled copies via `repo: local` (not a third root copy, and not a pinned self-reference — a pinned self-reference would lint working-tree edits against the last tagged release rather than the change being made). Every rule is `level: error` and every alert is a FAIL — no ignorable tier, same as shellcheck, the test suite, and conventional-pre-commit. Graded severities do not work here: Vale's exit code keys on `error` alerts alone, so `warning`/`suggestion` rules exit 0 and pre-commit swallows the output of a passing hook, leaving them invisible and blocking nothing. `MinAlertLevel` and `--minAlertLevel` are correspondingly absent from `.vale.ini` and the hook, being no-ops under this model. Vale covers the pattern-matchable sub-checks named in issue #84 (imperative opener, vague filler, `Use proactively`, generic reference-pointer padding) plus, per ADR-0013, one body-wide prose-pattern check ("There is/are" sentence openers) — everything else about body discipline (defaults-vs-menus, why-rationale, non-pattern-matchable judgment calls), near-miss exclusion strength, and control calibration stays LLM judgment.
|
||||
|
||||
Both skills' Step 1, and the `vale-audit-prefilter-skill`/`-agent` pre-commit hooks, call each copy's own `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. Handed no `--config` at all, the wrapper falls back to its own sibling `assets/vale/.vale.ini`, located from `${BASH_SOURCE[0]}` rather than from the cwd — which is why both manifests' `entry:` is now the bare script path with no argument after it. pre-commit prefixes only `entry[0]` with the hook-repo clone path (`cmd = (prefix.path(cmd[0]), *cmd[1:])`), so every later argument resolves against the *consuming* repo's root: a `--config` in `.pre-commit-hooks.yaml` pointed at a path no consumer has and hard-failed every external run with `E100 [--config] Runtime error`. `.pre-commit-config.yaml` drops the argument too, deliberately keeping the two entries identical — the local `repo: local` hook resolved its `--config` correctly only because the consuming repo *was* this repo, and that divergence is why three review rounds exercised a path no external consumer takes and missed the defect. An explicit `--config` still wins, in all three argv forms (`--config X`, `--config=/abs`, `--config=rel`), and a relative one still resolves against the caller's cwd, matching bare `vale`, not the repo root; both audit skills' Step 1 still passes `--config assets/vale/.vale.ini` and is unaffected. `tests/test-vale-wrap.sh` regression-tests this against skill-audit's copy specifically (its fixtures are all `SKILL.md`-shaped, and only skill-audit's `.vale.ini` has that glob section). Each `.vale.ini`'s section globs are path-agnostic (`[**/SKILL.md]` for skill-audit's copy; `[**/agents/*.md]`/`[**/*.agent.md]` for agent-audit's) and do no scoping on their own: Vale's `*` crosses `/`. Scoping comes from each pre-commit hook's own `files:` regex and from the audit skills passing one explicit file per invocation. The two manifests scope differently on purpose: this repo's `.pre-commit-config.yaml` pins its own layout — `^plugins/[^/]+/skills/[^/]+/SKILL\.md$` for `-skill`, `^plugins/[^/]+/agents/[^/]+\.md$` for `-agent` — while the shipped `.pre-commit-hooks.yaml` stays layout-agnostic for external consumers whose skills live anywhere, using `(^|/)SKILL\.md$` and `(^|/)agents/[^/]+\.md$|\.agent\.md$`. Both manifests split the prefilter into two hooks precisely because one combined hook pointed at only one copy would silently 0-file-skip the other file type. A `SKILL.md` outside `plugins/` (e.g. project-scope `.claude/skills/foo/SKILL.md`) still matches `[**/SKILL.md]` and gets linted normally — the globs constrain filename shape, not location. Vale reports 0 files only when the path it is handed matches no glob section at all: a differently-named file, or a directory argument holding nothing that matches. That run prints `✔ 0 errors ... in 0 files.` and exits 0, indistinguishable from a clean pass, so both audits treat a 0-file Vale run as NOT RUN and fall back to full LLM judgment.
|
||||
Both skills' Step 1, and the `vale-audit-prefilter-skill`/`-agent` pre-commit hooks, call each copy's own `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 on most — not all — multi-line descriptions. Verified by reproduction, not assumed: `>` folded scalars, plain (unquoted) continuation lines, and single- or double-quoted multi-line scalars all yield 0 alerts and exit 0 on a deliberately-bad fixture, while a `|` literal block spanning the same 2+ lines lints normally (alerts fire, exit 1). The wrapper flattens those three broken forms 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`; `|` literal blocks and single-line descriptions pass through untouched, already linting correctly. The plain and quoted forms previously passed silently — unflattened and unmatched — so a bad description in either sailed through the prefilter. Handed no `--config` at all, the wrapper falls back to its own sibling `assets/vale/.vale.ini`, located from `${BASH_SOURCE[0]}` rather than from the cwd — which is why both manifests' `entry:` is now the bare script path with no argument after it. pre-commit prefixes only `entry[0]` with the hook-repo clone path (`cmd = (prefix.path(cmd[0]), *cmd[1:])`), so every later argument resolves against the *consuming* repo's root: a `--config` in `.pre-commit-hooks.yaml` pointed at a path no consumer has and hard-failed every external run with `E100 [--config] Runtime error`. `.pre-commit-config.yaml` drops the argument too, deliberately keeping the two entries identical — the local `repo: local` hook resolved its `--config` correctly only because the consuming repo *was* this repo, and that divergence is why three review rounds exercised a path no external consumer takes and missed the defect. An explicit `--config` still wins, in all three argv forms (`--config X`, `--config=/abs`, `--config=rel`), and a relative one still resolves against the caller's cwd, matching bare `vale`, not the repo root. Both audit skills' Step 1 now passes no `--config` either: it resolves the script relative to the skill's own directory so the call works from an installed plugin cache, but a relative `--config` alongside it would still resolve against the cwd, yielding `E100 Runtime error ... does not exist` and exit 2 — which both skills' fallback misreads as "vale unavailable" and silently downgrades to full LLM judgment. `tests/test-vale-wrap.sh` regression-tests this against skill-audit's copy specifically (its fixtures are all `SKILL.md`-shaped, and only skill-audit's `.vale.ini` has that glob section). Each `.vale.ini`'s section globs are path-agnostic (`[**/SKILL.md]` for skill-audit's copy; `[**/agents/*.md]`/`[**/*.agent.md]` for agent-audit's) and do no scoping on their own: Vale's `*` crosses `/`. Scoping comes from each pre-commit hook's own `files:` regex and from the audit skills passing one explicit file per invocation. The two manifests scope differently on purpose: this repo's `.pre-commit-config.yaml` pins its own layout — `^plugins/[^/]+/skills/[^/]+/SKILL\.md$` for `-skill`, `^plugins/[^/]+/agents/[^/]+\.md$` for `-agent` — while the shipped `.pre-commit-hooks.yaml` stays layout-agnostic for external consumers whose skills live anywhere, using `(^|/)SKILL\.md$` and `(^|/)agents/[^/]+\.md$|\.agent\.md$`. Both manifests split the prefilter into two hooks precisely because one combined hook pointed at only one copy would silently 0-file-skip the other file type. A `SKILL.md` outside `plugins/` (e.g. project-scope `.claude/skills/foo/SKILL.md`) still matches `[**/SKILL.md]` and gets linted normally — the globs constrain filename shape, not location. Vale reports 0 files only when the path it is handed matches no glob section at all: a differently-named file, or a directory argument holding nothing that matches. That run prints `✔ 0 errors ... in 0 files.` and exits 0, indistinguishable from a clean pass, so both audits treat a 0-file Vale run as NOT RUN and fall back to full LLM judgment.
|
||||
|
||||
This scope expands per ADR-0013: one cherry-picked low-noise `write-good`/`alex` rule landed in `styles/Kyberforge`, `Kyberforge.SentenceOpenerThereIs` (22 held-out hits, both in-corpus hits clean rewrites, zero suppressions). A second, `Kyberforge.VagueQualifier`, was cherry-picked and then deleted: 2 hits across the 41 skill/agent files, one marginal and one an unfixable false positive (`caveman/SKILL.md` quotes `of course` as an example of filler — a mention, not a use) that forced the repo's only Vale suppression comments. Also new is a sibling pre-commit hook, `skill-size-check` (`scripts/skill-size-check.sh`), enforcing agentskills.io's 500-line/5,000-token `SKILL.md` ceiling — failing only above 500 lines, matching `skill-audit/scripts/validate.sh`'s `<= 500` pass — scoped to `^plugins/[^/]+/skills/[^/]+/SKILL\.md$` only, same as `vale-audit-prefilter-skill`, so it never lints `docs/research/examples/` reference skills. It's also exposed in the root-level `.pre-commit-hooks.yaml` as `kyberforge-skill-size-check` — it has no external asset dependency, so it needed no relocation, only exposure to external consumers. File scope (`SKILL.md` + agent files) and enforcement model (rules land directly in `styles/Kyberforge`, blocking immediately, no trial tier) stay unchanged; governance.md/CONTROLS.md were evaluated and excluded as rule sources (nothing prose-pattern-matchable to mine). House convention: banned phrasing that must be mentioned rather than used goes in backticks or a fenced code block — Vale skips code spans and fences, so no suppression is needed; inline `<!-- vale Rule = NO -->` (HTML-comment form; the MDX `{/* */}` form does not work in plain Markdown) is the fallback only where backticking is impossible.
|
||||
This scope expands per ADR-0013: one cherry-picked low-noise `write-good`/`alex` rule landed in `styles/Kyberforge`, `Kyberforge.SentenceOpenerThereIs` (22 held-out hits, both in-corpus hits clean rewrites, zero suppressions). A second, `Kyberforge.VagueQualifier`, was cherry-picked and then deleted: 2 hits across the 41 skill/agent files, one marginal and one an unfixable false positive (`caveman/SKILL.md` quotes `of course` as an example of filler — a mention, not a use) that forced the repo's only Vale suppression comments. Also new is a sibling pre-commit hook, `skill-size-check` (`scripts/skill-size-check.sh`), enforcing agentskills.io's `SKILL.md` ceiling as two blocking gates: `MAX_LINES=500` and `MAX_WORDS=2770` (a word-count proxy for the 5,000-token limit, calibrated to the densest prose measured in this repo — 1.81 tokens per word — so even a worst-case `SKILL.md` at the ceiling stays under 5,000 tokens). Both are inclusive, and `skill-audit/scripts/validate.sh` checks the same pair on the same terms, so a `SKILL.md` can no longer pass its own audit yet be blocked by the commit hook. Scoped to `^plugins/[^/]+/skills/[^/]+/SKILL\.md$` only, same as `vale-audit-prefilter-skill`, so it never lints `docs/research/examples/` reference skills. It's also exposed in the root-level `.pre-commit-hooks.yaml` as `kyberforge-skill-size-check` — it has no external asset dependency, so it needed no relocation, only exposure to external consumers. File scope (`SKILL.md` + agent files) and enforcement model (rules land directly in `styles/Kyberforge`, blocking immediately, no trial tier) stay unchanged; governance.md/CONTROLS.md were evaluated and excluded as rule sources (nothing prose-pattern-matchable to mine). House convention: banned phrasing that must be mentioned rather than used goes in backticks or a fenced code block — Vale skips code spans and fences, so no suppression is needed; inline `<!-- vale Rule = NO -->` (HTML-comment form; the MDX `{/* */}` form does not work in plain Markdown) is the fallback only where backticking is impossible.
|
||||
|
||||
### 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.
|
||||
|
||||
14
LESSONS.md
14
LESSONS.md
@@ -142,6 +142,8 @@ During write-skill refactor, an "open thread" note (about a deferred research st
|
||||
|
||||
Three separate times in one PR (#85), a check reported success because it had silently not run. (1) Vale's `text.frontmatter.description` scope stops matching once the value is a multi-line YAML block scalar — the style most skills here use — so a repo-wide sweep returned 0 alerts across 49 files and was read as a clean repo. (2) Five of six rules were `level: warning`, but Vale's exit code keys on `error` alone and pre-commit hides output from passing hooks, so those rules were invisible and blocked nothing for two review rounds while the ADR described them as "enforcing immediately." (3) `.vale.ini`'s globs matched no file outside `plugins/`, so Vale printed "0 files" and exited 0, which both audit skills read as "no findings" and used to skip their own judgment passes. Each time the green result was worse than no check at all, because it was cited as positive evidence of cleanliness. Fix: for any new check, prove it fails before trusting that it passes — run it against a deliberately-bad fixture, confirm the failure, then run the real corpus. Where a check can scan zero inputs, assert on the input count, not just the exit code. **[graduated → core/instructions/testing.md]** (4th instance below, kept for audit trail).
|
||||
|
||||
**5th instance (2026-08-09, PR #85 round 6):** `tests/test-vale-hooks-consumer.sh` asserted `grep -c "VagueWording" >= 2` across the *combined* output of both shipped Vale hooks, and the SKILL.md fixture alone raised two alerts — so one working hook satisfied the threshold and the agent hook could be disabled entirely (glob retargeted to match nothing) while the suite still reported `3 passed` under the message "both hooks flatten and flag". The `Skipped` guard did not catch it: the hook still *matched* the file, Vale simply linted nothing, reported `0 errors in 1 file`, and exited 0, which pre-commit renders as `Passed`. The general shape: **an assertion that aggregates over N subjects proves nothing about any individual subject** — a total is satisfiable by a proper subset. Fix: attribute each signal to its source before asserting (alerts are now filed by path, with a distinct trigger token per fixture so one hook's alert cannot be credited to another), and assert per subject. Corollary technique, now standing practice for any check whose failure mode is silence: run the mutation sweep in *reverse* as well — neuter each assertion in turn and confirm exactly one test case fails. Applied to `check-vale-style-sync.sh` it exposed two assertions bound to no failing case at all, one of them masked by a stronger check that ran first.
|
||||
|
||||
**4th instance (2026-08-09, ADR-0014):** splitting the single root `.vale.ini` into two skill-scoped copies (skill-audit: `SKILL.md` only; agent-audit: agent files only) meant a single retargeted pre-commit hook pointed at agent-audit's copy alone would have silently scanned 0 `SKILL.md` files and exited 0 — caught only because the full corpus was dry-run against both the old and new config and the outputs diffed before the old config was deleted, not because any test asserted on file counts. Standing practice going forward: when a Vale (or any linter) config that serves multiple file-glob scopes is split or moved, dry-run the full corpus through both the old and new config and diff the outputs before removing the superseded source — a hook silently scanning 0 files looks identical to a clean pass.
|
||||
|
||||
## 2026-08-08 — One signal, two consumers, no named distinction
|
||||
@@ -151,3 +153,15 @@ Vale's output fed two consumers with different contracts: the audit skills read
|
||||
## 2026-08-08 — Measure a rule's false-positive rate at the severity you will ship it at
|
||||
|
||||
`Kyberforge.VagueQualifier` was cherry-picked from `write-good` after being trialled as "low-noise against this repo's corpus" — but the trial ran at `level: warning`, where a false positive costs nothing because nobody ever sees it. Shipped at `error`, the same false positive costs a blocked commit and a permanent suppression comment. Re-measured at the severity it actually shipped at, the rule scored one marginal true positive and one unfixable false positive across 41 files (`caveman/SKILL.md` *quotes* filler words as its subject matter — a mention, not a use), and was deleted. Fix: trial conditions must match shipping conditions. A noise measurement taken where false positives are free does not transfer to a context where they are expensive, and "low-noise" is not a property of a rule alone — it is a property of the rule at a severity.
|
||||
|
||||
## 2026-08-09 — Exercising a config's "local" mode proves nothing about the mode that ships
|
||||
|
||||
The root `.pre-commit-hooks.yaml` shipped Vale hooks whose `entry:` carried a `--config <repo-relative-path>` argument. pre-commit prefixes only `entry[0]` with the hook-repo clone path (`cmd = (prefix.path(cmd[0]), *cmd[1:])`), so every later argument resolves against the *consuming* repo's root: each external consumer hard-failed with `E100 [--config] Runtime error ... does not exist`, and two of the three hooks ADR-0014 promised were unusable. The defect survived three review rounds of PR #85 and a green `pre-commit run --all-files` every time, because this repo consumes the same hooks through `repo: local`, where the clone prefix, the cwd, and the repo root are one directory — the byte-identical `entry:` string worked locally for a reason that exists only locally. Nothing under `tests/` exercised the manifest as a hook repo at all. The sharp part: the local run was not weaker evidence of the same thing, it was evidence of a different thing, and the two were indistinguishable by reading either file. Fix: when a config has a local mode whose resolution semantics differ from the shipped mode, test the shipped mode against a real consumer — `tests/test-vale-hooks-consumer.sh` stands up a `file://` clone of this repo and runs the hooks from it — and then delete the divergence rather than living with it: `vale-wrap.sh` now self-locates its config from `${BASH_SOURCE[0]}`, and the local and shipped `entry:` lines are identical, so the local run no longer exercises a path no consumer takes.
|
||||
|
||||
## 2026-08-09 — Deleting a token from a shared artifact breaks whatever parses it, silently
|
||||
|
||||
Dropping the `--config` argument from `.pre-commit-hooks.yaml` was the right fix, but `scripts/check-release-needed.sh` derived its release-relevant path list by scanning those same `entry:` lines for `--config` and taking the target's `dirname` — that parse was the only thing giving the bundled `.vale.ini` and its sibling `styles/` tree release coverage. With the token gone the loop simply never fired: no error, no failing test, no warning, just a path list that shrank from six entries to four and lost both `assets/vale/` trees. Consequence: a change to a Vale *rule* could land on `main` without demanding a release tag, leaving external consumers pinned to an old `rev:` with stale rules — the exact drift the gate exists to prevent. It surfaced only because the agent making the change reported it as a suspected side effect of its own edit, and was confirmed by diffing the derived path list before and after. Fix: before removing a token from an artifact more than one script reads, grep for everything that *parses* the artifact, not just everything that consumes its documented purpose. The smell to watch for is a loop that builds a list, where an empty or short list is indistinguishable from a correct one — assert on the expected members, so a derivation whose input vanished fails loudly instead of quietly covering less.
|
||||
|
||||
## 2026-08-09 — A documented impossibility is a claim, not a constraint
|
||||
|
||||
`vale-wrap.sh` flattens multi-line YAML `description:` scalars so Vale's `text.frontmatter.description` scope keeps matching. Its last-resort branch rewrote ASCII `'` to U+2019, justified at the emission site and in review as "the single combination no YAML scalar can carry verbatim" — an accepted-by-design residual, documented and test-covered, which is exactly why nobody retested it. The claim was false: a `|-` literal block with one indented content line carries `'`, `"`, `\` and `: ` verbatim, keeps the scope alive, and the wrapper's own header docstring already said literal blocks were unaffected. The cost of the unexamined claim was a silent underlint on 12 of 54 in-scope files — any rule whose token contained an apostrophe simply never fired, and the covering test (case 20) pinned only "the scope stays alive", so it passed either way. Fix: when a residual is accepted because something is "impossible", write down the specific claim in a falsifiable form and test *that*, not the workaround built on top of it. The tell here was that the residual and its justification were documented in the same breath by the same author — documentation records a belief, and a belief adjacent to a workaround is the one most worth attacking. Related: an assertion written to cover an accepted residual tends to assert the residual's *presence* rather than the behaviour it costs; case 20b asserted the scope survived flattening, never that a rule matching the rewritten characters still fired.
|
||||
|
||||
@@ -95,8 +95,14 @@ every rule to `level: error` is what actually implements this decision.
|
||||
catch does not pay for a permanent suppression, so the rule is deleted and this ADR's
|
||||
"cherry-picked rules" is one rule, not two.
|
||||
- A new pre-commit hook, `skill-size-check` (`scripts/skill-size-check.sh`), enforces the
|
||||
500-line/5,000-token `SKILL.md` ceiling, sibling to `skill-frontmatter`. It fails only *above*
|
||||
500 lines, matching `skill-audit/scripts/validate.sh`'s long-standing `<= 500` pass.
|
||||
500-line/5,000-token `SKILL.md` ceiling, sibling to `skill-frontmatter`. Both halves of that
|
||||
ceiling are blocking gates, not just the line count: `MAX_LINES=500`, and `MAX_WORDS=2770` as a
|
||||
word-count proxy for the 5,000-token limit (calibrated to the densest prose this repo measured,
|
||||
1.81 tokens per word, so a worst-case `SKILL.md` at the ceiling still lands under 5,000 tokens —
|
||||
`wc -w` is not BPE tokenization). Either one exceeded fails the hook. Both are
|
||||
inclusive: a file at exactly 500 lines or exactly 2,770 words passes, and only one past a ceiling
|
||||
fails. `skill-audit/scripts/validate.sh` enforces the same pair on the same inclusive terms, so
|
||||
the audit and the commit hook cannot disagree about whether a given `SKILL.md` is over size.
|
||||
- `styles/KyberforgeTrial/` and `.vale.trial.ini` were deliberately not created — noted here so a
|
||||
future reader doesn't wonder if a trial tier was forgotten.
|
||||
- The styles-portability question — whether `styles/` and `.vale.ini` should move into
|
||||
|
||||
@@ -151,6 +151,38 @@ doesn't wonder if it was overlooked.
|
||||
script now derives the bundle's `assets/` tree from `tokens[0]` instead (double-`dirname`,
|
||||
guarded on the candidate existing and on not resolving to `.`), which is the only derivation
|
||||
compatible with the argument-free `entry:` contract above.
|
||||
- **Accepted residual in the release gate:** deleting a hook's *entire* `assets/` tree is not
|
||||
flagged — the derived candidate path stops existing, so the guard drops it before it reaches
|
||||
the pathspec. Deleting individual files inside a surviving tree is flagged, and tested.
|
||||
- **Accepted residual in the release gate (closed — see the update below):** deleting a hook's
|
||||
*entire* `assets/` tree is not flagged — the derived candidate path stops existing, so the guard
|
||||
drops it before it reaches the pathspec. Deleting individual files inside a surviving tree is
|
||||
flagged, and tested.
|
||||
|
||||
**Update (commit `14c2c91`):** the accepted residual above no longer holds and is recorded here
|
||||
only as the state at the time this ADR was written. `check-release-needed.sh` no longer derives
|
||||
release-relevant paths from the worktree alone. It runs `collect_release_paths` twice — once over
|
||||
the worktree's `.pre-commit-hooks.yaml`, once over the manifest read back from `$LAST_TAG` via
|
||||
`git cat-file -p "$LAST_TAG:$HOOKS_MANIFEST"` — and unions the two path sets, so a path the tag
|
||||
exposed stays in the pathspec even after the worktree's `-d` guard drops it. Wholesale deletion of
|
||||
a hook's bundled `assets/` tree is therefore flagged, and `tests/test-check-release-needed.sh`
|
||||
(case 12) asserts exit 1 for exactly that case. The union does not over-fire: any manifest edit
|
||||
that makes the two disagree already touches `$HOOKS_MANIFEST`, itself a release-relevant path. An
|
||||
unreadable tagged tree (shallow clone, truncated fetch) fails closed rather than silently degrading
|
||||
to worktree-only derivation; a manifest simply absent at the tag — legitimate, it was added since —
|
||||
does not.
|
||||
|
||||
**Update — the flattener rewrites no characters.** This ADR never recorded it as a decision, but
|
||||
`vale-wrap.sh`'s flattener carried a lossy last-resort branch: when a description needed quoting
|
||||
*and* held an ASCII apostrophe *and* held a double quote or backslash, it substituted U+2019 (`’`)
|
||||
for every `'` before writing the scratch copy, on the stated rationale that no verbatim YAML scalar
|
||||
could carry that combination. The rationale was wrong. A `|-` literal block with a single indented
|
||||
content line carries `'`, `"`, `\` and `: ` byte for byte — a block scalar's body has no escape
|
||||
syntax at all — and vale's `text.frontmatter.description` scope still matches and fires rules on it
|
||||
(verified against vale 3.15.2; it is the same property that makes the `|` blocks in the wrapper's
|
||||
header safe to leave unflattened). The branch fired on 12 of the 54 in-scope files in this repo,
|
||||
silently disabling every rule whose token contains an apostrophe on each of them. The flattener now
|
||||
emits that literal block instead, so its output is verbatim in all four forms and no Vale rule can
|
||||
be silently disabled by the prefilter. The `|-` form is two physical lines where the three inline
|
||||
forms are one, so the blank-line pad that preserves later line numbers drops by one — reachable
|
||||
only when the original span is already two or more lines, so the pad count stays non-negative.
|
||||
`tests/test-vale-wrap.sh` case 20 asserts an apostrophe-bearing token actually fires on a flattened
|
||||
description in all three apostrophe-carrying branches, and case 20b pins the pad arithmetic against
|
||||
a body line's true line number.
|
||||
|
||||
@@ -15,5 +15,5 @@
|
||||
],
|
||||
"license": "MIT",
|
||||
"name": "gitea",
|
||||
"version": "1.3.2"
|
||||
"version": "1.3.3"
|
||||
}
|
||||
|
||||
@@ -20,5 +20,5 @@
|
||||
"skills": [
|
||||
"skills/"
|
||||
],
|
||||
"version": "1.3.2"
|
||||
"version": "1.3.3"
|
||||
}
|
||||
|
||||
@@ -8,5 +8,5 @@
|
||||
"keywords": [],
|
||||
"license": "MIT",
|
||||
"name": "kyberforge",
|
||||
"version": "1.2.6"
|
||||
"version": "1.2.8"
|
||||
}
|
||||
|
||||
@@ -13,5 +13,5 @@
|
||||
"skills": [
|
||||
"skills/"
|
||||
],
|
||||
"version": "1.2.6"
|
||||
"version": "1.2.8"
|
||||
}
|
||||
|
||||
@@ -4,7 +4,7 @@ Audits a Claude Code and Copilot agent definition file pair for correctness and
|
||||
|
||||
## What it does
|
||||
|
||||
Accepts either file in a CC `.md` / Copilot `.agent.md` pair, derives the counterpart automatically, and validates both. Runs structural checks via `validate.sh` (required fields, kebab-case name, no placeholders, no CC-only fields in the Copilot file, silently-ignored fields at plugin scope), provenance chain validation via `validate-provenance.sh` (checks `source_keys` against `sources.md` at the plugin root), then qualitative checks on description phrasing and system prompt quality. Produces a compact findings report in the same format as `skill-audit`.
|
||||
Accepts either file in a CC `.md` / Copilot `.agent.md` pair, derives the counterpart automatically, and validates both. Runs structural checks via `validate.sh` (required fields, kebab-case name, no placeholders, no CC-only fields in the Copilot file, silently-ignored fields at plugin scope), provenance chain validation via `validate-provenance.sh` (checks `source_keys` against `sources.md` at the plugin root), then qualitative checks on description phrasing and system prompt quality. Step 1 also runs a Vale-based prose sub-check via `vale-wrap.sh` against both files of the pair, using the `Kyberforge` style (both files) and `KyberforgeCopilot` style (Copilot file only) — every alert is a `FAIL`, cited by rule ID — falling back to Step 2 judgment when the `vale` binary is unavailable or reports `0 files` scanned. Produces a compact findings report in the same format as `skill-audit`.
|
||||
|
||||
## Usage
|
||||
|
||||
@@ -19,6 +19,12 @@ Pass the path to either agent file as the argument.
|
||||
| File | Purpose |
|
||||
|------|---------|
|
||||
| `SKILL.md` | Skill instructions for agents |
|
||||
| `assets/vale/.vale.ini` | Vale config: scopes `Kyberforge` to `**/agents/*.md`, `Kyberforge`+`KyberforgeCopilot` to `**/*.agent.md` |
|
||||
| `assets/vale/styles/Kyberforge/DescriptionOpener.yml` | Flags descriptions opening with "This skill/agent" instead of an imperative "Use when..." |
|
||||
| `assets/vale/styles/Kyberforge/PaddingPhrase.yml` | Flags generic "see references/ for info" pointers instead of specific file references |
|
||||
| `assets/vale/styles/Kyberforge/SentenceOpenerThereIs.yml` | Flags sentences opening with "There is/are" instead of naming the subject directly |
|
||||
| `assets/vale/styles/Kyberforge/VagueWording.yml` | Flags vague capability wording ("helps with", "utilize", "assists with", "used for") in descriptions |
|
||||
| `assets/vale/styles/KyberforgeCopilot/ProactivePhrase.yml` | Flags CC-specific "Use proactively" phrasing with no effect in Copilot descriptions |
|
||||
| `references/README.md` | Directory documentation for references/ |
|
||||
| `references/description-quality.md` | Qualitative guide for borderline description findings |
|
||||
| `references/field-inventory.md` | Authoritative list of valid CC and Copilot agent fields |
|
||||
@@ -26,6 +32,7 @@ Pass the path to either agent file as the argument.
|
||||
| `scripts/README.md` | Directory documentation for scripts/ |
|
||||
| `scripts/validate.sh` | Structural validation script for agent file pairs |
|
||||
| `scripts/validate-provenance.sh` | Provenance chain validation script for agent pairs against `sources.md` (plugin root) |
|
||||
| `scripts/vale-wrap.sh` | Drop-in `vale` wrapper that works around a frontmatter-description NLP scope limitation |
|
||||
| `tests/README.md` | Bats test dependency and run instructions |
|
||||
| `tests/validate.bats` | Bats tests for validate.sh |
|
||||
| `tests/validate-provenance.bats` | Bats tests for validate-provenance.sh |
|
||||
|
||||
@@ -36,12 +36,12 @@ metadata:
|
||||
```bash
|
||||
bash scripts/validate.sh <path-to-agent-file>
|
||||
bash scripts/validate-provenance.sh <path-to-agent-file>
|
||||
scripts/vale-wrap.sh --config assets/vale/.vale.ini <path-to-cc-file> <path-to-copilot-file>
|
||||
scripts/vale-wrap.sh <path-to-cc-file> <path-to-copilot-file>
|
||||
```
|
||||
|
||||
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`.
|
||||
|
||||
`vale-wrap.sh` and `.vale.ini` ship inside this skill's own `scripts/`/`assets/` — resolve them relative to this skill's directory the same way `scripts/validate.sh` is resolved above, so the invocation works whether this skill is running from this repo or from an installed plugin cache. Run it against both files of the pair (not just the one passed in). `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. Every Vale alert is a `FAIL` — all rules are graded `error` — so report each one in the `### Description` / `### Body` dimensions citing its rule ID (e.g. `KyberforgeCopilot.ProactivePhrase`). Skip and fall back to Step 2 judgment if vale or `.vale.ini` is unavailable. If Vale reports `0 files` scanned, treat the pass as NOT RUN — not as clean — and fall back to full Step 2 judgment for the dimensions it would have covered.
|
||||
`vale-wrap.sh` ships inside this skill's own `scripts/` — resolve it relative to this skill's directory the same way `scripts/validate.sh` is resolved above, so the invocation works whether this skill is running from this repo or from an installed plugin cache. Pass no `--config`: handed none, the wrapper loads its own sibling `assets/vale/.vale.ini`, located from the script's path rather than from the cwd. Adding an explicit relative `--config` breaks exactly the case the self-location covers — a resolved script path plus an unresolved config path yields `E100 Runtime error ... does not exist`, exit 2, which the fallback below then misreads as "vale unavailable". Run it against both files of the pair (not just the one passed in). `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. Every Vale alert is a `FAIL` — all rules are graded `error` — so report each one in the `### Description` / `### Body` dimensions citing its rule ID (e.g. `KyberforgeCopilot.ProactivePhrase`). Skip and fall back to Step 2 judgment if the `vale` binary is unavailable. If Vale reports `0 files` scanned, treat the pass as NOT RUN — not as clean — and fall back to full Step 2 judgment for the dimensions it would have covered.
|
||||
|
||||
`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.
|
||||
|
||||
|
||||
@@ -2,27 +2,47 @@
|
||||
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.
|
||||
# silently stops matching once the `description:` value spans 2+ physical lines
|
||||
# in any form YAML joins back into one string — a `>`/`>-`/`>+` folded block
|
||||
# scalar (the style used by most skills/agents in this repo), a plain scalar
|
||||
# wrapped onto continuation lines, or a double- or single-quoted scalar wrapped
|
||||
# the same way. A `|`/`|-`/`|+` literal block scalar is NOT affected: its parsed
|
||||
# value keeps exactly the line breaks the source has, and vale matches it fine
|
||||
# (verified against vale 3.15.2), so literal blocks are deliberately left alone.
|
||||
# This script flattens an affected description to a one-line scalar in a scratch
|
||||
# copy — or, for the rare value no inline scalar can spell out verbatim, to a
|
||||
# `|-` literal block with a single content line, which vale matches just as well
|
||||
# (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, bar the two documented divergences
|
||||
# below.
|
||||
#
|
||||
# "Same args" means relative paths — `--config` values and path arguments alike
|
||||
# — resolve against the caller's current directory, exactly as bare `vale`
|
||||
# resolves them. (An earlier version resolved them against the repo root, an
|
||||
# invented convention that hard-errored on `--config ../../.vale.ini` from a
|
||||
# subdirectory and, worse, silently dropped file arguments that didn't happen to
|
||||
# resolve from the repo root — skipping the flattening this script exists for.)
|
||||
# "Same args" means relative paths — path arguments and the values of the
|
||||
# path-valued flags (`--config`, `--output`, `--path`) alike — resolve against
|
||||
# the caller's current directory, exactly as bare `vale` resolves them. The flag
|
||||
# values are rewritten to absolute form because the run ends up `cd`'d into the
|
||||
# scratch mirror, where a relative one would no longer resolve. (An earlier
|
||||
# version resolved path arguments against the repo root, an invented convention
|
||||
# that hard-errored on `--config ../../.vale.ini` from a subdirectory and, worse,
|
||||
# silently dropped file arguments that didn't happen to resolve from the repo
|
||||
# root — skipping the flattening this script exists for.)
|
||||
#
|
||||
# The one addition to bare `vale`'s argument handling: with no `--config` at
|
||||
# all, this script's own sibling `assets/vale/.vale.ini` is used instead of
|
||||
# vale's upward search. pre-commit prefixes only `entry[0]` with the hook-repo
|
||||
# clone path, so a `--config` in `.pre-commit-hooks.yaml` would resolve against
|
||||
# the *consuming* repo and hard-fail (E100) for every external consumer. The
|
||||
# manifest therefore passes the script alone, and an explicit `--config` from
|
||||
# any other caller still wins.
|
||||
# Divergence 1: with no `--config` at all, this script's own sibling
|
||||
# `assets/vale/.vale.ini` is used instead of vale's upward search. pre-commit
|
||||
# prefixes only `entry[0]` with the hook-repo clone path, so a `--config` in
|
||||
# `.pre-commit-hooks.yaml` would resolve against the *consuming* repo and
|
||||
# hard-fail (E100) for every external consumer. The manifest therefore passes the
|
||||
# script alone, and an explicit `--config` from any other caller still wins.
|
||||
#
|
||||
# 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
|
||||
# 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
|
||||
# unsupported here — it already was, since the no-path handoff closes stdin so
|
||||
# vale can't block on a pipe that will never carry content.
|
||||
#
|
||||
# Vale prints each path exactly as it was handed to it, so the scratch tree
|
||||
# mirrors the caller's absolute cwd: a relative path argument is passed through
|
||||
@@ -34,55 +54,137 @@ set -euo pipefail
|
||||
|
||||
cwd="$(pwd -P)"
|
||||
|
||||
# Every array below is expanded as `${arr[@]+"${arr[@]}"}`: bash before 4.4 —
|
||||
# including the 3.2 that macOS still ships as /bin/bash — treats `"${arr[@]}"`
|
||||
# on an empty array as an unbound variable under `set -u`. No expansion site is
|
||||
# reachable while empty on today's control flow, so this is insurance against a
|
||||
# later edit breaking that invariant, not a live fix.
|
||||
vale_args=()
|
||||
path_args=()
|
||||
config_next=false
|
||||
pending_flag=""
|
||||
config_given=false
|
||||
|
||||
# `--output` takes either one of vale's built-in style names or a template file
|
||||
# path. Only the file form needs absolutizing, and the built-in names have to be
|
||||
# excluded by name *before* the existence test below: a file or directory
|
||||
# literally called `line` in the caller's cwd would otherwise rewrite the
|
||||
# built-in into `$cwd/line`, flipping vale into template mode (`E100 [template]
|
||||
# Runtime error`) where bare vale just uses the built-in. `--path` has no such
|
||||
# names — it is always a path — so the check is keyed on the flag too.
|
||||
is_builtin_output() {
|
||||
case "$2" in
|
||||
line|JSON|CLI) [[ "$1" == "--output" ]] ;;
|
||||
*) false ;;
|
||||
esac
|
||||
}
|
||||
# Absolutizes a `--config` value against the caller's cwd. Shared by both
|
||||
# argument forms below — separated (`--config X`) and joined (`--config=X`)
|
||||
# — so the "already absolute vs. needs $cwd prefixed" check lives in exactly
|
||||
# one place instead of being duplicated per form.
|
||||
abs_config_value() {
|
||||
if [[ "$1" == /* ]]; then
|
||||
printf '%s' "$1"
|
||||
else
|
||||
printf '%s' "$cwd/$1"
|
||||
fi
|
||||
}
|
||||
for arg in "$@"; do
|
||||
if [[ "$config_next" == true ]]; then
|
||||
config_next=false
|
||||
if [[ "$arg" == /* ]]; then
|
||||
vale_args+=("$arg")
|
||||
else
|
||||
vale_args+=("$cwd/$arg")
|
||||
fi
|
||||
if [[ -n "$pending_flag" ]]; then
|
||||
# Value of a separated two-argv flag. It is never a lint target, however
|
||||
# file-like it looks. The run ends up `cd`'d into the scratch mirror, so a
|
||||
# value naming a file has to be absolutized here or it stops resolving.
|
||||
case "$pending_flag" in
|
||||
--config)
|
||||
# Always a path, and required to exist.
|
||||
vale_args+=("$(abs_config_value "$arg")")
|
||||
;;
|
||||
--output|--path)
|
||||
# See `is_builtin_output` above for why the built-in `--output` names
|
||||
# are excluded first. Anything that names nothing is passed through and
|
||||
# left for vale to interpret.
|
||||
if is_builtin_output "$pending_flag" "$arg"; then
|
||||
vale_args+=("$arg")
|
||||
elif [[ "$arg" != /* && -e "$arg" ]]; then
|
||||
vale_args+=("$cwd/$arg")
|
||||
else
|
||||
vale_args+=("$arg")
|
||||
fi
|
||||
;;
|
||||
*)
|
||||
vale_args+=("$arg")
|
||||
;;
|
||||
esac
|
||||
pending_flag=""
|
||||
continue
|
||||
fi
|
||||
case "$arg" in
|
||||
--config)
|
||||
vale_args+=("$arg")
|
||||
config_next=true
|
||||
config_given=true
|
||||
continue
|
||||
;;
|
||||
--config=/*)
|
||||
vale_args+=("$arg")
|
||||
pending_flag="$arg"
|
||||
config_given=true
|
||||
continue
|
||||
;;
|
||||
--config=*)
|
||||
vale_args+=("--config=$cwd/${arg#--config=}")
|
||||
vale_args+=("--config=$(abs_config_value "${arg#--config=}")")
|
||||
config_given=true
|
||||
continue
|
||||
;;
|
||||
# Same cwd-relative resolution for the `--flag=value` spelling of the two
|
||||
# other path-valued flags.
|
||||
--output=*|--path=*)
|
||||
flag_val="${arg#*=}"
|
||||
if is_builtin_output "${arg%%=*}" "$flag_val"; then
|
||||
vale_args+=("$arg")
|
||||
elif [[ "$flag_val" != /* && -n "$flag_val" && -e "$flag_val" ]]; then
|
||||
vale_args+=("${arg%%=*}=$cwd/$flag_val")
|
||||
else
|
||||
vale_args+=("$arg")
|
||||
fi
|
||||
continue
|
||||
;;
|
||||
# Vale's remaining value-taking flags, per `vale --help` (3.x). In the
|
||||
# separated two-argv form the value must not be classified as a lint target
|
||||
# — `--output tmpl.tmpl` names a real template file, and treating it as
|
||||
# input both lints the template and reorders argv so vale sees
|
||||
# `--output --no-wrap`. The `--flag=value` form needs no entry here: it
|
||||
# starts with `-` and falls through to vale untouched. A value flag added by
|
||||
# some future vale release is simply absent from this list and lands back on
|
||||
# today's behaviour, so this list going stale is never worse than not having
|
||||
# it.
|
||||
--ext|--filter|--glob|--minAlertLevel|--output|--path)
|
||||
vale_args+=("$arg")
|
||||
pending_flag="$arg"
|
||||
continue
|
||||
;;
|
||||
# Vale's subcommands are bare words that name no file, so they would trip
|
||||
# the not-found error below. A lint target literally named `sync` (no
|
||||
# extension, no slash) is misread as the subcommand — accepted, because the
|
||||
# alternative is failing every `vale-wrap.sh ls-config`.
|
||||
ls-config|ls-dirs|ls-metrics|ls-vars|sync)
|
||||
vale_args+=("$arg")
|
||||
continue
|
||||
;;
|
||||
esac
|
||||
# `-f`/`-d` resolve relative paths against the caller's cwd, same as vale does.
|
||||
# A path that doesn't exist is left for vale to report on, exactly as bare
|
||||
# vale would.
|
||||
if [[ "$arg" != -* && ( -f "$arg" || -d "$arg" ) ]]; then
|
||||
# An absolute path inside the caller's cwd is relativized so the report cites
|
||||
# a path that resolves against the real tree. Left absolute, it would be
|
||||
# rewritten to its scratch copy and printed as `/tmp/tmp.XXXX/...` — a real
|
||||
# path to a file that is deleted on exit, which reads as a bug in any report
|
||||
# quoting it. Absolute paths outside the cwd have no relative form and keep
|
||||
# the scratch-path behaviour documented above.
|
||||
if [[ "$arg" == "$cwd"/* ]]; then
|
||||
path_args+=("${arg#"$cwd"/}")
|
||||
else
|
||||
path_args+=("$arg")
|
||||
fi
|
||||
else
|
||||
if [[ "$arg" == -* ]]; then
|
||||
vale_args+=("$arg")
|
||||
continue
|
||||
fi
|
||||
# Everything left is a lint target: `vale [options] [input...]` has no third
|
||||
# kind of argument. See divergence 2 above for why a missing one is fatal here.
|
||||
if [[ ! -e "$arg" ]]; then
|
||||
echo "vale-wrap.sh: no such file or directory: $arg" >&2
|
||||
exit 2
|
||||
fi
|
||||
# An absolute path inside the caller's cwd is relativized so the report cites
|
||||
# a path that resolves against the real tree. Left absolute, it would be
|
||||
# rewritten to its scratch copy and printed as `/tmp/tmp.XXXX/...` — a real
|
||||
# path to a file that is deleted on exit, which reads as a bug in any report
|
||||
# quoting it. Absolute paths outside the cwd have no relative form and keep
|
||||
# the scratch-path behaviour documented above.
|
||||
if [[ "$arg" == "$cwd"/* ]]; then
|
||||
path_args+=("${arg#"$cwd"/}")
|
||||
else
|
||||
path_args+=("$arg")
|
||||
fi
|
||||
done
|
||||
|
||||
@@ -93,7 +195,7 @@ fi
|
||||
if [[ ${#path_args[@]} -eq 0 ]]; then
|
||||
# Nothing to flatten. Hand off directly, with stdin closed so vale doesn't
|
||||
# block waiting on a pipe that will never carry content.
|
||||
exec vale "${vale_args[@]}" < /dev/null
|
||||
exec vale ${vale_args[@]+"${vale_args[@]}"} < /dev/null
|
||||
fi
|
||||
|
||||
# `realpath -m` would be the obvious normalizer, but `-m` (canonicalize-missing)
|
||||
@@ -104,68 +206,262 @@ abspath() {
|
||||
}
|
||||
|
||||
flatten() {
|
||||
python3 - "$1" "$2" <<'PYTHON'
|
||||
# Two call shapes: `flatten src dest` (dest already resolved and inside the
|
||||
# scratch tree — the per-markdown-file calls in the directory branch below)
|
||||
# writes straight to `dest`. `flatten src raw_dest tmpdir` (the single-file
|
||||
# branch further down) additionally resolves `raw_dest` the way a separate
|
||||
# `abspath` call used to, applies the same sandbox-escape guard, and prints
|
||||
# the resolved path — folding two python3 spawns per file into one.
|
||||
python3 - "$@" <<'PYTHON'
|
||||
import os
|
||||
import re
|
||||
import sys
|
||||
|
||||
src, dest = sys.argv[1], sys.argv[2]
|
||||
src, dest_input = sys.argv[1], sys.argv[2]
|
||||
tmpdir = sys.argv[3] if len(sys.argv) > 3 else None
|
||||
|
||||
if tmpdir is None:
|
||||
dest = dest_input
|
||||
else:
|
||||
dest = os.path.abspath(dest_input)
|
||||
if not dest.startswith(tmpdir + os.sep):
|
||||
print(
|
||||
f"vale-wrap.sh: refusing to lint '{src}': its scratch copy would "
|
||||
f"land outside {tmpdir}",
|
||||
file=sys.stderr,
|
||||
)
|
||||
sys.exit(2)
|
||||
os.makedirs(os.path.dirname(dest), exist_ok=True)
|
||||
|
||||
# surrogateescape keeps a non-UTF-8 file (reachable via a directory argument)
|
||||
# a byte-for-byte round trip instead of aborting the whole run on a decode error.
|
||||
with open(src, encoding='utf-8', errors='surrogateescape') as fh:
|
||||
content = fh.read()
|
||||
|
||||
# YAML 1.2 double-quoted escapes (spec 5.7 / 7.3.1). `\<newline>` is handled
|
||||
# separately in unescape_double because it also swallows the next indentation.
|
||||
DQ_ESCAPES = {
|
||||
'0': '\0', 'a': '\a', 'b': '\b', 't': '\t', '\t': '\t', 'n': '\n',
|
||||
'v': '\v', 'f': '\f', 'r': '\r', 'e': '\x1b', ' ': ' ', '"': '"',
|
||||
'/': '/', '\\': '\\', 'N': '\x85', '_': '\xa0', 'L': '\u2028',
|
||||
'P': '\u2029',
|
||||
}
|
||||
|
||||
# First characters that make a plain (unquoted) scalar mean something other than
|
||||
# text: YAML's c-indicator set.
|
||||
PLAIN_UNSAFE_FIRST = '-?:,[]{}#&*!|>\'"%@`'
|
||||
|
||||
|
||||
def unescape_double(text):
|
||||
"""Decode a double-quoted YAML scalar's body to the string YAML parses."""
|
||||
out = []
|
||||
i = 0
|
||||
while i < len(text):
|
||||
char = text[i]
|
||||
if char != '\\':
|
||||
out.append(char)
|
||||
i += 1
|
||||
continue
|
||||
i += 1
|
||||
if i >= len(text):
|
||||
break
|
||||
esc = text[i]
|
||||
if esc == '\n':
|
||||
i += 1
|
||||
while i < len(text) and text[i] in ' \t':
|
||||
i += 1
|
||||
continue
|
||||
if esc in 'xuU':
|
||||
width = {'x': 2, 'u': 4, 'U': 8}[esc]
|
||||
digits = text[i + 1:i + 1 + width]
|
||||
if len(digits) == width:
|
||||
try:
|
||||
out.append(chr(int(digits, 16)))
|
||||
except ValueError:
|
||||
pass
|
||||
else:
|
||||
i += 1 + width
|
||||
continue
|
||||
out.append(DQ_ESCAPES.get(esc, esc))
|
||||
i += 1
|
||||
return ''.join(out)
|
||||
|
||||
|
||||
def close_quote(text, quote):
|
||||
"""Index of the closing `quote` in `text`, which starts just past the
|
||||
opening one. None while the scalar is still unterminated."""
|
||||
i = 0
|
||||
while i < len(text):
|
||||
char = text[i]
|
||||
if quote == '"' and char == '\\':
|
||||
i += 2
|
||||
continue
|
||||
if char == quote:
|
||||
if quote == "'" and text[i + 1:i + 2] == "'":
|
||||
i += 2
|
||||
continue
|
||||
return i
|
||||
i += 1
|
||||
return None
|
||||
|
||||
|
||||
def continuation_lines(rest):
|
||||
"""Yield the physical lines of `rest` that continue the value started on the
|
||||
`description:` line. Indentation-based and blank-line-tolerant, per YAML:
|
||||
a blank line (any amount of whitespace) always stays inside; the indent is
|
||||
set by the first content line; the value ends at the first line indented
|
||||
less than that, at any line flush with the key (that is the next mapping
|
||||
key, not a continuation), or at EOF."""
|
||||
indent = None
|
||||
for line in rest.splitlines(keepends=True):
|
||||
text = line.rstrip('\n')
|
||||
if text.strip() == '':
|
||||
yield line
|
||||
continue
|
||||
line_indent = len(text) - len(text.lstrip(' \t'))
|
||||
if line_indent == 0:
|
||||
return
|
||||
if indent is None:
|
||||
indent = line_indent
|
||||
elif line_indent < indent:
|
||||
return
|
||||
yield line
|
||||
|
||||
|
||||
def emit(value):
|
||||
"""Render `value` as a YAML scalar whose source text spells the value out
|
||||
verbatim. Vale locates the description by matching the parsed value back
|
||||
against the source, so a scalar carrying any escape — `''` in a
|
||||
single-quoted scalar, `\\"` or `\\\\` in a double-quoted one — makes the
|
||||
whole `text.frontmatter.description` scope vanish, the same failure this
|
||||
script exists to work around. Verbatim forms only, therefore, tried in
|
||||
descending order of fidelity. The first three occupy one physical line; the
|
||||
`|-` fallback occupies two, which the caller accounts for when padding."""
|
||||
if (value
|
||||
and value[0] not in PLAIN_UNSAFE_FIRST
|
||||
and ': ' not in value
|
||||
and not value.endswith(':')
|
||||
and ' #' not in value):
|
||||
return value # plain: nothing needs escaping at all
|
||||
if "'" not in value:
|
||||
return "'" + value + "'" # single-quoted: only `'` would escape
|
||||
if '"' not in value and '\\' not in value:
|
||||
return '"' + value + '"' # double-quoted: only `"`/`\` would
|
||||
# Last resort: the value needs quoting AND holds an apostrophe AND a double
|
||||
# quote or backslash, so no *inline* scalar can carry it verbatim. A `|-`
|
||||
# literal block can — a block scalar's body has no escape syntax at all, so
|
||||
# `'`, `"`, `\` and `: ` all survive byte for byte, and vale still matches
|
||||
# the description scope against it (the header above says the same of the
|
||||
# `|` blocks this script deliberately leaves alone; verified against vale
|
||||
# 3.15.2). One content line, indented two spaces, `-`-chomped so the parsed
|
||||
# value is exactly `value` with no trailing newline.
|
||||
return '|-\n ' + value
|
||||
|
||||
|
||||
fm_match = re.match(r'^(---\n)(.*?\n)(---\n)', content, re.DOTALL)
|
||||
if fm_match:
|
||||
fm = fm_match.group(2)
|
||||
# Only `>`/`>-`/`>+` (folded) scalars break Vale's frontmatter-description
|
||||
# scope. `|`/`|-`/`|+` (literal) scalars already work fine with bare vale,
|
||||
# so they're deliberately left unmatched here.
|
||||
header_m = re.search(r'^description:[ \t]*(>[+-]?)[ \t]*\n', fm, re.MULTILINE)
|
||||
if header_m:
|
||||
# Body capture is indentation-based and blank-line-tolerant, per YAML
|
||||
# block-scalar rules: a blank line (any amount of whitespace) always
|
||||
# stays inside the block; the indent is set by the first content line;
|
||||
# the block ends at the first line indented less than that, or EOF.
|
||||
rest = fm[header_m.end():]
|
||||
indent = None
|
||||
body_lines = []
|
||||
for line in rest.splitlines(keepends=True):
|
||||
text = line.rstrip('\n')
|
||||
if text.strip() == '':
|
||||
body_lines.append(line)
|
||||
continue
|
||||
line_indent = len(text) - len(text.lstrip(' \t'))
|
||||
if indent is None:
|
||||
indent = line_indent
|
||||
elif line_indent < indent:
|
||||
header_m = re.search(r'^description:[ \t]*', fm, re.MULTILINE)
|
||||
else:
|
||||
header_m = None
|
||||
|
||||
if header_m:
|
||||
head_start = header_m.start()
|
||||
value_start = header_m.end()
|
||||
header_end = fm.find('\n', value_start)
|
||||
header_end = len(fm) if header_end == -1 else header_end
|
||||
first = fm[value_start:header_end]
|
||||
body_start = header_end + 1
|
||||
indicator = first.rstrip()
|
||||
|
||||
block_m = re.fullmatch(r'([|>])([+-]?[0-9]*|[0-9]*[+-]?)', indicator)
|
||||
if block_m and block_m.group(1) == '|':
|
||||
kind = None # literal blocks keep their line breaks; vale is fine
|
||||
elif block_m:
|
||||
kind = 'block' # folded (`>`): the value starts on the next line
|
||||
elif indicator == '':
|
||||
kind = 'block' # bare `description:`: a plain scalar on later lines
|
||||
elif first[:1] == '"':
|
||||
kind = 'double'
|
||||
elif first[:1] == "'":
|
||||
kind = 'single'
|
||||
elif first[:1] in '#&*!':
|
||||
kind = None # comment, anchor, alias or tag — not a plain scalar
|
||||
else:
|
||||
kind = 'plain'
|
||||
|
||||
text = ''
|
||||
value_end = value_start
|
||||
value_lines = 0
|
||||
if kind in ('block', 'plain'):
|
||||
body = ''.join(continuation_lines(fm[body_start:]))
|
||||
value_end = body_start + len(body)
|
||||
if kind == 'block':
|
||||
text = body
|
||||
value_lines = body.count('\n')
|
||||
else:
|
||||
text = fm[value_start:value_end]
|
||||
value_lines = 1 + body.count('\n')
|
||||
if ' #' in text or text.lstrip().startswith('#'):
|
||||
# A `#` opens a comment inside a plain scalar. Folding it in
|
||||
# would lint text YAML never treats as part of the value, so
|
||||
# leave the file alone rather than lint the wrong string.
|
||||
kind = None
|
||||
elif kind in ('double', 'single'):
|
||||
quote = '"' if kind == 'double' else "'"
|
||||
inner_start = value_start + 1
|
||||
acc = fm[inner_start:body_start]
|
||||
idx = close_quote(acc, quote)
|
||||
lines = continuation_lines(fm[body_start:])
|
||||
while idx is None:
|
||||
try:
|
||||
acc += next(lines)
|
||||
except StopIteration:
|
||||
break
|
||||
body_lines.append(line)
|
||||
raw = ''.join(body_lines)
|
||||
if raw.count('\n') >= 2:
|
||||
flat = re.sub(r'\s+', ' ', raw).strip()
|
||||
# YAML single-quoted scalars have no backslash-escape mechanism at
|
||||
# all, so wrapping in single quotes sidesteps the backslash-escape
|
||||
# bug entirely for embedded double quotes, backslashes, and
|
||||
# non-ASCII text. The one YAML-spec-correct way to embed a literal
|
||||
# apostrophe is to double it ('') — but Vale's own frontmatter
|
||||
# scanner isn't a full YAML parser and doesn't understand that
|
||||
# doubling: empirically, it silently truncates the value at the
|
||||
# first ' it sees, hiding everything after it from the NLP scope
|
||||
# (a different flavor of the same bug this whole script exists to
|
||||
# work around). Since this copy is scratch-only and never written
|
||||
# back, sidestep it by substituting a Unicode right single
|
||||
# quotation mark (U+2019) for any literal apostrophe instead of
|
||||
# doubling it — visually a smart quote, but never triggers a YAML
|
||||
# escape sequence at all.
|
||||
flat_q = "'" + flat.replace("'", "’") + "'"
|
||||
pad = '\n' * raw.count('\n')
|
||||
start = header_m.start()
|
||||
end = header_m.end() + len(raw)
|
||||
new_fm = fm[:start] + f'description: {flat_q}\n{pad}' + fm[end:]
|
||||
content = fm_match.group(1) + new_fm + fm_match.group(3) + content[fm_match.end():]
|
||||
idx = close_quote(acc, quote)
|
||||
if idx is None:
|
||||
kind = None # unterminated quote: invalid YAML, leave it to vale
|
||||
else:
|
||||
inner = acc[:idx]
|
||||
value_end = inner_start + idx + 1
|
||||
text = unescape_double(inner) if quote == '"' else inner.replace("''", "'")
|
||||
value_lines = 1 + inner.count('\n')
|
||||
|
||||
flat = re.sub(r'\s+', ' ', text).strip()
|
||||
if kind and flat and value_lines >= 2:
|
||||
# `value_end` can land mid-line, just past a closing quote, so extend to
|
||||
# the end of that physical line and carry whatever follows (a trailing
|
||||
# comment) across unchanged.
|
||||
if value_end > 0 and fm[value_end - 1] == '\n':
|
||||
span_end = value_end
|
||||
trailer = ''
|
||||
else:
|
||||
newline = fm.find('\n', value_end)
|
||||
span_end = len(fm) if newline == -1 else newline + 1
|
||||
trailer = fm[value_end:span_end].rstrip('\n')
|
||||
scalar = emit(flat)
|
||||
# A trailing comment carried across from the original line stays on the
|
||||
# `description:` line itself: after a block scalar's `|-` header it is
|
||||
# still a comment, but inside the block body it would become part of the
|
||||
# value.
|
||||
head, newline_sep, block_body = scalar.partition('\n')
|
||||
# The replacement displaces the whole span, so the blank-line pad makes
|
||||
# up the difference between the lines it displaced and the lines it
|
||||
# occupies — every later line number is unchanged. That is one line for
|
||||
# the three inline forms and two for the `|-` block; the span itself is
|
||||
# at least two lines here (`value_lines >= 2` is a precondition), so the
|
||||
# pad count never goes negative.
|
||||
pad = '\n' * (fm[head_start:span_end].count('\n') - 1 - scalar.count('\n'))
|
||||
new_fm = (fm[:head_start] + 'description: ' + head + trailer
|
||||
+ newline_sep + block_body + '\n' + pad + fm[span_end:])
|
||||
content = (fm_match.group(1) + new_fm + fm_match.group(3)
|
||||
+ content[fm_match.end():])
|
||||
|
||||
with open(dest, 'w', encoding='utf-8', errors='surrogateescape') as fh:
|
||||
fh.write(content)
|
||||
|
||||
if tmpdir is not None:
|
||||
print(dest)
|
||||
PYTHON
|
||||
}
|
||||
|
||||
@@ -178,39 +474,46 @@ mirror="$tmpdir$cwd"
|
||||
mkdir -p "$mirror"
|
||||
|
||||
argv_paths=()
|
||||
for arg in "${path_args[@]}"; do
|
||||
for arg in ${path_args[@]+"${path_args[@]}"}; do
|
||||
if [[ "$arg" == /* ]]; then
|
||||
dest="$tmpdir$arg"
|
||||
raw_dest="$tmpdir$arg"
|
||||
else
|
||||
dest="$mirror/$arg"
|
||||
raw_dest="$mirror/$arg"
|
||||
fi
|
||||
dest="$(abspath "$dest")"
|
||||
# A path argument with enough leading `..` to climb past the mirror root would
|
||||
# write outside the scratch dir. The real filesystem clamps such a path at
|
||||
# `/`; the mirror can't, so refuse rather than scribble outside the sandbox.
|
||||
case "$dest" in
|
||||
"$tmpdir"/*) ;;
|
||||
*)
|
||||
echo "vale-wrap.sh: refusing to lint '$arg': its scratch copy would land outside $tmpdir" >&2
|
||||
exit 2
|
||||
;;
|
||||
esac
|
||||
mkdir -p "$(dirname "$dest")"
|
||||
if [[ -d "$arg" ]]; then
|
||||
dest="$(abspath "$raw_dest")"
|
||||
# A path argument with enough leading `..` to climb past the mirror root would
|
||||
# write outside the scratch dir. The real filesystem clamps such a path at
|
||||
# `/`; the mirror can't, so refuse rather than scribble outside the sandbox.
|
||||
case "$dest" in
|
||||
"$tmpdir"/*) ;;
|
||||
*)
|
||||
echo "vale-wrap.sh: refusing to lint '$arg': its scratch copy would land outside $tmpdir" >&2
|
||||
exit 2
|
||||
;;
|
||||
esac
|
||||
mkdir -p "$(dirname "$dest")"
|
||||
# A directory is mirrored whole — vale applies its own format filtering to
|
||||
# the tree, so any file dropped here would be silently unlinted — and then
|
||||
# every markdown file in the copy is flattened in place. `.git` is pruned:
|
||||
# vale never lints it and copying it can dwarf the rest of the tree.
|
||||
# `find -L` follows symlinks because vale does: it lints both a symlinked
|
||||
# file and a file under a symlinked directory, and a bare `-type f` walk
|
||||
# would report "0 files" where bare vale reports one. (A symlink loop makes
|
||||
# `find` warn on stderr and carry on, which is also what vale does.) The
|
||||
# second walk needs no `-L`: the mirror is all real files by construction.
|
||||
mkdir -p "$dest"
|
||||
while IFS= read -r -d '' rel; do
|
||||
mkdir -p "$dest/$(dirname "$rel")"
|
||||
cp "$arg/$rel" "$dest/$rel"
|
||||
done < <(cd "$arg" && find . -name .git -prune -o -type f -print0)
|
||||
done < <(cd "$arg" && find -L . -name .git -prune -o -type f -print0)
|
||||
while IFS= read -r -d '' md; do
|
||||
flatten "$md" "$md"
|
||||
done < <(find "$dest" -type f -name '*.md' -print0)
|
||||
else
|
||||
flatten "$arg" "$dest"
|
||||
# `abspath` + `flatten` folded into one python3 process — see the comment
|
||||
# atop `flatten` above.
|
||||
dest="$(flatten "$arg" "$raw_dest" "$tmpdir")"
|
||||
fi
|
||||
if [[ "$arg" == /* ]]; then
|
||||
argv_paths+=("$dest")
|
||||
@@ -220,4 +523,4 @@ for arg in "${path_args[@]}"; do
|
||||
done
|
||||
|
||||
cd "$mirror"
|
||||
vale "${vale_args[@]}" "${argv_paths[@]}"
|
||||
vale ${vale_args[@]+"${vale_args[@]}"} ${argv_paths[@]+"${argv_paths[@]}"}
|
||||
|
||||
@@ -4,7 +4,7 @@ Audit a skill directory against the agentskills.io specification. Runs structura
|
||||
|
||||
## What it does
|
||||
|
||||
1. Runs `scripts/validate.sh` and `scripts/validate-provenance.sh` for structural and provenance checks
|
||||
1. Runs `scripts/validate.sh` and `scripts/validate-provenance.sh` for structural and provenance checks, plus `scripts/vale-wrap.sh` — a Vale prefilter that deterministically flags known-bad description openers, vague wording, padding phrases, and "There is/are" sentence openers
|
||||
2. Reads all files in the skill directory
|
||||
3. Applies qualitative checks across seven dimensions
|
||||
4. Outputs a compact findings report — findings only, grouped by dimension, each with Why and Fix — and a result block with handoff to /skill-improve
|
||||
@@ -24,6 +24,12 @@ Provide the path to the skill directory to audit when invoking.
|
||||
| `SKILL.md` | Skill instructions for agents |
|
||||
| `scripts/validate.sh` | Structural validator — checks name format, name matches directory, description length, line count, placeholder detection, script executable bit, and interactive-prompt detection |
|
||||
| `scripts/validate-provenance.sh` | Provenance validator — checks sources.md completeness, source_keys/slug consistency, Contributing files existence, bidirectional linkage, Research doc: fields, and upstream research doc alignment |
|
||||
| `scripts/vale-wrap.sh` | Vale prefilter wrapper — runs the bundled `Kyberforge` Vale styles against SKILL.md and reports alerts as deterministic FAILs ahead of Step 3's qualitative review |
|
||||
| `assets/vale/.vale.ini` | Vale configuration — points Vale at the bundled `Kyberforge` style path, self-located relative to `vale-wrap.sh` |
|
||||
| `assets/vale/styles/Kyberforge/DescriptionOpener.yml` | Vale rule — flags literal "This skill..."/"This agent..." description openers |
|
||||
| `assets/vale/styles/Kyberforge/PaddingPhrase.yml` | Vale rule — flags generic "see references/" padding phrasing in conditional references |
|
||||
| `assets/vale/styles/Kyberforge/SentenceOpenerThereIs.yml` | Vale rule — flags body sentences starting with "There is"/"There are" |
|
||||
| `assets/vale/styles/Kyberforge/VagueWording.yml` | Vale rule — flags known filler wording (e.g. "helps with", "utilize") |
|
||||
| `references/description-quality.md` | Spec-grounded rubric for description auditing — loaded when a finding is borderline |
|
||||
| `references/body-discipline.md` | Spec-grounded rubric for body discipline auditing — loaded when padding vs necessity is unclear |
|
||||
| `references/sources.md` | Provenance record — agentskills.io sources that informed this skill and which files each contributed to |
|
||||
|
||||
@@ -34,14 +34,14 @@ metadata:
|
||||
```bash
|
||||
bash scripts/validate.sh <skill-dir>
|
||||
bash scripts/validate-provenance.sh <skill-dir>
|
||||
scripts/vale-wrap.sh --config assets/vale/.vale.ini <skill-dir>/SKILL.md
|
||||
scripts/vale-wrap.sh <skill-dir>/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 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 and ≤2770 words (the word count is a proxy for the ~5,000-token ceiling, and blocks a commit exactly like the line count does), 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-wrap.sh` and `.vale.ini` ship inside this skill's own `scripts/`/`assets/` — resolve them relative to this skill's directory the same way `scripts/validate.sh` is resolved above, so the invocation works whether this skill is running from this repo or from an installed plugin cache. It applies `.vale.ini`'s `Kyberforge` style — a deterministic prefilter for a subset of the Description/Patterns/Body dimensions below, not a replacement for Step 3. Every Vale alert is a `FAIL` — all rules are graded `error` — so report each one citing its rule ID (e.g. `Kyberforge.DescriptionOpener`). Skip and fall back to Step 3 judgment if vale or `.vale.ini` is unavailable. If Vale reports `0 files` scanned, treat the pass as NOT RUN — not as clean — and fall back to full Step 3 judgment for the dimensions it would have covered.
|
||||
`vale-wrap.sh` ships inside this skill's own `scripts/` — resolve it relative to this skill's directory the same way `scripts/validate.sh` is resolved above, so the invocation works whether this skill is running from this repo or from an installed plugin cache. Pass no `--config`: handed none, the wrapper loads its own sibling `assets/vale/.vale.ini`, located from the script's path rather than from the cwd. Adding an explicit relative `--config` breaks exactly the case the self-location covers — a resolved script path plus an unresolved config path yields `E100 Runtime error ... does not exist`, exit 2, which the fallback below then misreads as "vale unavailable". It applies that config's `Kyberforge` style — a deterministic prefilter for a subset of the Description/Patterns/Body dimensions below, not a replacement for Step 3. Every Vale alert is a `FAIL` — all rules are graded `error` — so report each one citing its rule ID (e.g. `Kyberforge.DescriptionOpener`). Skip and fall back to Step 3 judgment if the `vale` binary is unavailable. If Vale reports `0 files` scanned, treat the pass as NOT RUN — not as clean — and fall back to full Step 3 judgment for the dimensions it would have covered.
|
||||
|
||||
## Step 2 — Read all skill files
|
||||
|
||||
@@ -55,6 +55,7 @@ Work through each dimension internally. Collect findings only; report them in St
|
||||
|
||||
Vale's `Kyberforge.DescriptionOpener` ("This skill..." openers) and `Kyberforge.VagueWording` (filler like "helps with", "utilize") alerts from Step 1 — both FAILs — cover imperative phrasing and known vague-wording filler directly; report them as findings without re-deriving by judgment. The rest is still a judgment call:
|
||||
|
||||
- **Action-verb opening**: does the description start with a verb ("Audits...", "Reviews...", "Validates...")? Vale's `Kyberforge.DescriptionOpener` alert only catches the literal "This skill..." pattern — confirming an arbitrary opening word is genuinely a strong verb still requires judgment.
|
||||
- **Specificity beyond the filler blocklist**: are capabilities stated precisely ("parses OpenAPI specs") or genuinely vaguely ("handles files")?
|
||||
- **Indirect triggers**: does it cover cases where the user doesn't name the domain directly?
|
||||
- **Near-miss exclusions**: are "Do not use when..." clauses present if a near-miss skill could steal activations?
|
||||
|
||||
@@ -2,27 +2,47 @@
|
||||
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.
|
||||
# silently stops matching once the `description:` value spans 2+ physical lines
|
||||
# in any form YAML joins back into one string — a `>`/`>-`/`>+` folded block
|
||||
# scalar (the style used by most skills/agents in this repo), a plain scalar
|
||||
# wrapped onto continuation lines, or a double- or single-quoted scalar wrapped
|
||||
# the same way. A `|`/`|-`/`|+` literal block scalar is NOT affected: its parsed
|
||||
# value keeps exactly the line breaks the source has, and vale matches it fine
|
||||
# (verified against vale 3.15.2), so literal blocks are deliberately left alone.
|
||||
# This script flattens an affected description to a one-line scalar in a scratch
|
||||
# copy — or, for the rare value no inline scalar can spell out verbatim, to a
|
||||
# `|-` literal block with a single content line, which vale matches just as well
|
||||
# (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, bar the two documented divergences
|
||||
# below.
|
||||
#
|
||||
# "Same args" means relative paths — `--config` values and path arguments alike
|
||||
# — resolve against the caller's current directory, exactly as bare `vale`
|
||||
# resolves them. (An earlier version resolved them against the repo root, an
|
||||
# invented convention that hard-errored on `--config ../../.vale.ini` from a
|
||||
# subdirectory and, worse, silently dropped file arguments that didn't happen to
|
||||
# resolve from the repo root — skipping the flattening this script exists for.)
|
||||
# "Same args" means relative paths — path arguments and the values of the
|
||||
# path-valued flags (`--config`, `--output`, `--path`) alike — resolve against
|
||||
# the caller's current directory, exactly as bare `vale` resolves them. The flag
|
||||
# values are rewritten to absolute form because the run ends up `cd`'d into the
|
||||
# scratch mirror, where a relative one would no longer resolve. (An earlier
|
||||
# version resolved path arguments against the repo root, an invented convention
|
||||
# that hard-errored on `--config ../../.vale.ini` from a subdirectory and, worse,
|
||||
# silently dropped file arguments that didn't happen to resolve from the repo
|
||||
# root — skipping the flattening this script exists for.)
|
||||
#
|
||||
# The one addition to bare `vale`'s argument handling: with no `--config` at
|
||||
# all, this script's own sibling `assets/vale/.vale.ini` is used instead of
|
||||
# vale's upward search. pre-commit prefixes only `entry[0]` with the hook-repo
|
||||
# clone path, so a `--config` in `.pre-commit-hooks.yaml` would resolve against
|
||||
# the *consuming* repo and hard-fail (E100) for every external consumer. The
|
||||
# manifest therefore passes the script alone, and an explicit `--config` from
|
||||
# any other caller still wins.
|
||||
# Divergence 1: with no `--config` at all, this script's own sibling
|
||||
# `assets/vale/.vale.ini` is used instead of vale's upward search. pre-commit
|
||||
# prefixes only `entry[0]` with the hook-repo clone path, so a `--config` in
|
||||
# `.pre-commit-hooks.yaml` would resolve against the *consuming* repo and
|
||||
# hard-fail (E100) for every external consumer. The manifest therefore passes the
|
||||
# script alone, and an explicit `--config` from any other caller still wins.
|
||||
#
|
||||
# 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
|
||||
# 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
|
||||
# unsupported here — it already was, since the no-path handoff closes stdin so
|
||||
# vale can't block on a pipe that will never carry content.
|
||||
#
|
||||
# Vale prints each path exactly as it was handed to it, so the scratch tree
|
||||
# mirrors the caller's absolute cwd: a relative path argument is passed through
|
||||
@@ -34,55 +54,137 @@ set -euo pipefail
|
||||
|
||||
cwd="$(pwd -P)"
|
||||
|
||||
# Every array below is expanded as `${arr[@]+"${arr[@]}"}`: bash before 4.4 —
|
||||
# including the 3.2 that macOS still ships as /bin/bash — treats `"${arr[@]}"`
|
||||
# on an empty array as an unbound variable under `set -u`. No expansion site is
|
||||
# reachable while empty on today's control flow, so this is insurance against a
|
||||
# later edit breaking that invariant, not a live fix.
|
||||
vale_args=()
|
||||
path_args=()
|
||||
config_next=false
|
||||
pending_flag=""
|
||||
config_given=false
|
||||
|
||||
# `--output` takes either one of vale's built-in style names or a template file
|
||||
# path. Only the file form needs absolutizing, and the built-in names have to be
|
||||
# excluded by name *before* the existence test below: a file or directory
|
||||
# literally called `line` in the caller's cwd would otherwise rewrite the
|
||||
# built-in into `$cwd/line`, flipping vale into template mode (`E100 [template]
|
||||
# Runtime error`) where bare vale just uses the built-in. `--path` has no such
|
||||
# names — it is always a path — so the check is keyed on the flag too.
|
||||
is_builtin_output() {
|
||||
case "$2" in
|
||||
line|JSON|CLI) [[ "$1" == "--output" ]] ;;
|
||||
*) false ;;
|
||||
esac
|
||||
}
|
||||
# Absolutizes a `--config` value against the caller's cwd. Shared by both
|
||||
# argument forms below — separated (`--config X`) and joined (`--config=X`)
|
||||
# — so the "already absolute vs. needs $cwd prefixed" check lives in exactly
|
||||
# one place instead of being duplicated per form.
|
||||
abs_config_value() {
|
||||
if [[ "$1" == /* ]]; then
|
||||
printf '%s' "$1"
|
||||
else
|
||||
printf '%s' "$cwd/$1"
|
||||
fi
|
||||
}
|
||||
for arg in "$@"; do
|
||||
if [[ "$config_next" == true ]]; then
|
||||
config_next=false
|
||||
if [[ "$arg" == /* ]]; then
|
||||
vale_args+=("$arg")
|
||||
else
|
||||
vale_args+=("$cwd/$arg")
|
||||
fi
|
||||
if [[ -n "$pending_flag" ]]; then
|
||||
# Value of a separated two-argv flag. It is never a lint target, however
|
||||
# file-like it looks. The run ends up `cd`'d into the scratch mirror, so a
|
||||
# value naming a file has to be absolutized here or it stops resolving.
|
||||
case "$pending_flag" in
|
||||
--config)
|
||||
# Always a path, and required to exist.
|
||||
vale_args+=("$(abs_config_value "$arg")")
|
||||
;;
|
||||
--output|--path)
|
||||
# See `is_builtin_output` above for why the built-in `--output` names
|
||||
# are excluded first. Anything that names nothing is passed through and
|
||||
# left for vale to interpret.
|
||||
if is_builtin_output "$pending_flag" "$arg"; then
|
||||
vale_args+=("$arg")
|
||||
elif [[ "$arg" != /* && -e "$arg" ]]; then
|
||||
vale_args+=("$cwd/$arg")
|
||||
else
|
||||
vale_args+=("$arg")
|
||||
fi
|
||||
;;
|
||||
*)
|
||||
vale_args+=("$arg")
|
||||
;;
|
||||
esac
|
||||
pending_flag=""
|
||||
continue
|
||||
fi
|
||||
case "$arg" in
|
||||
--config)
|
||||
vale_args+=("$arg")
|
||||
config_next=true
|
||||
config_given=true
|
||||
continue
|
||||
;;
|
||||
--config=/*)
|
||||
vale_args+=("$arg")
|
||||
pending_flag="$arg"
|
||||
config_given=true
|
||||
continue
|
||||
;;
|
||||
--config=*)
|
||||
vale_args+=("--config=$cwd/${arg#--config=}")
|
||||
vale_args+=("--config=$(abs_config_value "${arg#--config=}")")
|
||||
config_given=true
|
||||
continue
|
||||
;;
|
||||
# Same cwd-relative resolution for the `--flag=value` spelling of the two
|
||||
# other path-valued flags.
|
||||
--output=*|--path=*)
|
||||
flag_val="${arg#*=}"
|
||||
if is_builtin_output "${arg%%=*}" "$flag_val"; then
|
||||
vale_args+=("$arg")
|
||||
elif [[ "$flag_val" != /* && -n "$flag_val" && -e "$flag_val" ]]; then
|
||||
vale_args+=("${arg%%=*}=$cwd/$flag_val")
|
||||
else
|
||||
vale_args+=("$arg")
|
||||
fi
|
||||
continue
|
||||
;;
|
||||
# Vale's remaining value-taking flags, per `vale --help` (3.x). In the
|
||||
# separated two-argv form the value must not be classified as a lint target
|
||||
# — `--output tmpl.tmpl` names a real template file, and treating it as
|
||||
# input both lints the template and reorders argv so vale sees
|
||||
# `--output --no-wrap`. The `--flag=value` form needs no entry here: it
|
||||
# starts with `-` and falls through to vale untouched. A value flag added by
|
||||
# some future vale release is simply absent from this list and lands back on
|
||||
# today's behaviour, so this list going stale is never worse than not having
|
||||
# it.
|
||||
--ext|--filter|--glob|--minAlertLevel|--output|--path)
|
||||
vale_args+=("$arg")
|
||||
pending_flag="$arg"
|
||||
continue
|
||||
;;
|
||||
# Vale's subcommands are bare words that name no file, so they would trip
|
||||
# the not-found error below. A lint target literally named `sync` (no
|
||||
# extension, no slash) is misread as the subcommand — accepted, because the
|
||||
# alternative is failing every `vale-wrap.sh ls-config`.
|
||||
ls-config|ls-dirs|ls-metrics|ls-vars|sync)
|
||||
vale_args+=("$arg")
|
||||
continue
|
||||
;;
|
||||
esac
|
||||
# `-f`/`-d` resolve relative paths against the caller's cwd, same as vale does.
|
||||
# A path that doesn't exist is left for vale to report on, exactly as bare
|
||||
# vale would.
|
||||
if [[ "$arg" != -* && ( -f "$arg" || -d "$arg" ) ]]; then
|
||||
# An absolute path inside the caller's cwd is relativized so the report cites
|
||||
# a path that resolves against the real tree. Left absolute, it would be
|
||||
# rewritten to its scratch copy and printed as `/tmp/tmp.XXXX/...` — a real
|
||||
# path to a file that is deleted on exit, which reads as a bug in any report
|
||||
# quoting it. Absolute paths outside the cwd have no relative form and keep
|
||||
# the scratch-path behaviour documented above.
|
||||
if [[ "$arg" == "$cwd"/* ]]; then
|
||||
path_args+=("${arg#"$cwd"/}")
|
||||
else
|
||||
path_args+=("$arg")
|
||||
fi
|
||||
else
|
||||
if [[ "$arg" == -* ]]; then
|
||||
vale_args+=("$arg")
|
||||
continue
|
||||
fi
|
||||
# Everything left is a lint target: `vale [options] [input...]` has no third
|
||||
# kind of argument. See divergence 2 above for why a missing one is fatal here.
|
||||
if [[ ! -e "$arg" ]]; then
|
||||
echo "vale-wrap.sh: no such file or directory: $arg" >&2
|
||||
exit 2
|
||||
fi
|
||||
# An absolute path inside the caller's cwd is relativized so the report cites
|
||||
# a path that resolves against the real tree. Left absolute, it would be
|
||||
# rewritten to its scratch copy and printed as `/tmp/tmp.XXXX/...` — a real
|
||||
# path to a file that is deleted on exit, which reads as a bug in any report
|
||||
# quoting it. Absolute paths outside the cwd have no relative form and keep
|
||||
# the scratch-path behaviour documented above.
|
||||
if [[ "$arg" == "$cwd"/* ]]; then
|
||||
path_args+=("${arg#"$cwd"/}")
|
||||
else
|
||||
path_args+=("$arg")
|
||||
fi
|
||||
done
|
||||
|
||||
@@ -93,7 +195,7 @@ fi
|
||||
if [[ ${#path_args[@]} -eq 0 ]]; then
|
||||
# Nothing to flatten. Hand off directly, with stdin closed so vale doesn't
|
||||
# block waiting on a pipe that will never carry content.
|
||||
exec vale "${vale_args[@]}" < /dev/null
|
||||
exec vale ${vale_args[@]+"${vale_args[@]}"} < /dev/null
|
||||
fi
|
||||
|
||||
# `realpath -m` would be the obvious normalizer, but `-m` (canonicalize-missing)
|
||||
@@ -104,68 +206,262 @@ abspath() {
|
||||
}
|
||||
|
||||
flatten() {
|
||||
python3 - "$1" "$2" <<'PYTHON'
|
||||
# Two call shapes: `flatten src dest` (dest already resolved and inside the
|
||||
# scratch tree — the per-markdown-file calls in the directory branch below)
|
||||
# writes straight to `dest`. `flatten src raw_dest tmpdir` (the single-file
|
||||
# branch further down) additionally resolves `raw_dest` the way a separate
|
||||
# `abspath` call used to, applies the same sandbox-escape guard, and prints
|
||||
# the resolved path — folding two python3 spawns per file into one.
|
||||
python3 - "$@" <<'PYTHON'
|
||||
import os
|
||||
import re
|
||||
import sys
|
||||
|
||||
src, dest = sys.argv[1], sys.argv[2]
|
||||
src, dest_input = sys.argv[1], sys.argv[2]
|
||||
tmpdir = sys.argv[3] if len(sys.argv) > 3 else None
|
||||
|
||||
if tmpdir is None:
|
||||
dest = dest_input
|
||||
else:
|
||||
dest = os.path.abspath(dest_input)
|
||||
if not dest.startswith(tmpdir + os.sep):
|
||||
print(
|
||||
f"vale-wrap.sh: refusing to lint '{src}': its scratch copy would "
|
||||
f"land outside {tmpdir}",
|
||||
file=sys.stderr,
|
||||
)
|
||||
sys.exit(2)
|
||||
os.makedirs(os.path.dirname(dest), exist_ok=True)
|
||||
|
||||
# surrogateescape keeps a non-UTF-8 file (reachable via a directory argument)
|
||||
# a byte-for-byte round trip instead of aborting the whole run on a decode error.
|
||||
with open(src, encoding='utf-8', errors='surrogateescape') as fh:
|
||||
content = fh.read()
|
||||
|
||||
# YAML 1.2 double-quoted escapes (spec 5.7 / 7.3.1). `\<newline>` is handled
|
||||
# separately in unescape_double because it also swallows the next indentation.
|
||||
DQ_ESCAPES = {
|
||||
'0': '\0', 'a': '\a', 'b': '\b', 't': '\t', '\t': '\t', 'n': '\n',
|
||||
'v': '\v', 'f': '\f', 'r': '\r', 'e': '\x1b', ' ': ' ', '"': '"',
|
||||
'/': '/', '\\': '\\', 'N': '\x85', '_': '\xa0', 'L': '\u2028',
|
||||
'P': '\u2029',
|
||||
}
|
||||
|
||||
# First characters that make a plain (unquoted) scalar mean something other than
|
||||
# text: YAML's c-indicator set.
|
||||
PLAIN_UNSAFE_FIRST = '-?:,[]{}#&*!|>\'"%@`'
|
||||
|
||||
|
||||
def unescape_double(text):
|
||||
"""Decode a double-quoted YAML scalar's body to the string YAML parses."""
|
||||
out = []
|
||||
i = 0
|
||||
while i < len(text):
|
||||
char = text[i]
|
||||
if char != '\\':
|
||||
out.append(char)
|
||||
i += 1
|
||||
continue
|
||||
i += 1
|
||||
if i >= len(text):
|
||||
break
|
||||
esc = text[i]
|
||||
if esc == '\n':
|
||||
i += 1
|
||||
while i < len(text) and text[i] in ' \t':
|
||||
i += 1
|
||||
continue
|
||||
if esc in 'xuU':
|
||||
width = {'x': 2, 'u': 4, 'U': 8}[esc]
|
||||
digits = text[i + 1:i + 1 + width]
|
||||
if len(digits) == width:
|
||||
try:
|
||||
out.append(chr(int(digits, 16)))
|
||||
except ValueError:
|
||||
pass
|
||||
else:
|
||||
i += 1 + width
|
||||
continue
|
||||
out.append(DQ_ESCAPES.get(esc, esc))
|
||||
i += 1
|
||||
return ''.join(out)
|
||||
|
||||
|
||||
def close_quote(text, quote):
|
||||
"""Index of the closing `quote` in `text`, which starts just past the
|
||||
opening one. None while the scalar is still unterminated."""
|
||||
i = 0
|
||||
while i < len(text):
|
||||
char = text[i]
|
||||
if quote == '"' and char == '\\':
|
||||
i += 2
|
||||
continue
|
||||
if char == quote:
|
||||
if quote == "'" and text[i + 1:i + 2] == "'":
|
||||
i += 2
|
||||
continue
|
||||
return i
|
||||
i += 1
|
||||
return None
|
||||
|
||||
|
||||
def continuation_lines(rest):
|
||||
"""Yield the physical lines of `rest` that continue the value started on the
|
||||
`description:` line. Indentation-based and blank-line-tolerant, per YAML:
|
||||
a blank line (any amount of whitespace) always stays inside; the indent is
|
||||
set by the first content line; the value ends at the first line indented
|
||||
less than that, at any line flush with the key (that is the next mapping
|
||||
key, not a continuation), or at EOF."""
|
||||
indent = None
|
||||
for line in rest.splitlines(keepends=True):
|
||||
text = line.rstrip('\n')
|
||||
if text.strip() == '':
|
||||
yield line
|
||||
continue
|
||||
line_indent = len(text) - len(text.lstrip(' \t'))
|
||||
if line_indent == 0:
|
||||
return
|
||||
if indent is None:
|
||||
indent = line_indent
|
||||
elif line_indent < indent:
|
||||
return
|
||||
yield line
|
||||
|
||||
|
||||
def emit(value):
|
||||
"""Render `value` as a YAML scalar whose source text spells the value out
|
||||
verbatim. Vale locates the description by matching the parsed value back
|
||||
against the source, so a scalar carrying any escape — `''` in a
|
||||
single-quoted scalar, `\\"` or `\\\\` in a double-quoted one — makes the
|
||||
whole `text.frontmatter.description` scope vanish, the same failure this
|
||||
script exists to work around. Verbatim forms only, therefore, tried in
|
||||
descending order of fidelity. The first three occupy one physical line; the
|
||||
`|-` fallback occupies two, which the caller accounts for when padding."""
|
||||
if (value
|
||||
and value[0] not in PLAIN_UNSAFE_FIRST
|
||||
and ': ' not in value
|
||||
and not value.endswith(':')
|
||||
and ' #' not in value):
|
||||
return value # plain: nothing needs escaping at all
|
||||
if "'" not in value:
|
||||
return "'" + value + "'" # single-quoted: only `'` would escape
|
||||
if '"' not in value and '\\' not in value:
|
||||
return '"' + value + '"' # double-quoted: only `"`/`\` would
|
||||
# Last resort: the value needs quoting AND holds an apostrophe AND a double
|
||||
# quote or backslash, so no *inline* scalar can carry it verbatim. A `|-`
|
||||
# literal block can — a block scalar's body has no escape syntax at all, so
|
||||
# `'`, `"`, `\` and `: ` all survive byte for byte, and vale still matches
|
||||
# the description scope against it (the header above says the same of the
|
||||
# `|` blocks this script deliberately leaves alone; verified against vale
|
||||
# 3.15.2). One content line, indented two spaces, `-`-chomped so the parsed
|
||||
# value is exactly `value` with no trailing newline.
|
||||
return '|-\n ' + value
|
||||
|
||||
|
||||
fm_match = re.match(r'^(---\n)(.*?\n)(---\n)', content, re.DOTALL)
|
||||
if fm_match:
|
||||
fm = fm_match.group(2)
|
||||
# Only `>`/`>-`/`>+` (folded) scalars break Vale's frontmatter-description
|
||||
# scope. `|`/`|-`/`|+` (literal) scalars already work fine with bare vale,
|
||||
# so they're deliberately left unmatched here.
|
||||
header_m = re.search(r'^description:[ \t]*(>[+-]?)[ \t]*\n', fm, re.MULTILINE)
|
||||
if header_m:
|
||||
# Body capture is indentation-based and blank-line-tolerant, per YAML
|
||||
# block-scalar rules: a blank line (any amount of whitespace) always
|
||||
# stays inside the block; the indent is set by the first content line;
|
||||
# the block ends at the first line indented less than that, or EOF.
|
||||
rest = fm[header_m.end():]
|
||||
indent = None
|
||||
body_lines = []
|
||||
for line in rest.splitlines(keepends=True):
|
||||
text = line.rstrip('\n')
|
||||
if text.strip() == '':
|
||||
body_lines.append(line)
|
||||
continue
|
||||
line_indent = len(text) - len(text.lstrip(' \t'))
|
||||
if indent is None:
|
||||
indent = line_indent
|
||||
elif line_indent < indent:
|
||||
header_m = re.search(r'^description:[ \t]*', fm, re.MULTILINE)
|
||||
else:
|
||||
header_m = None
|
||||
|
||||
if header_m:
|
||||
head_start = header_m.start()
|
||||
value_start = header_m.end()
|
||||
header_end = fm.find('\n', value_start)
|
||||
header_end = len(fm) if header_end == -1 else header_end
|
||||
first = fm[value_start:header_end]
|
||||
body_start = header_end + 1
|
||||
indicator = first.rstrip()
|
||||
|
||||
block_m = re.fullmatch(r'([|>])([+-]?[0-9]*|[0-9]*[+-]?)', indicator)
|
||||
if block_m and block_m.group(1) == '|':
|
||||
kind = None # literal blocks keep their line breaks; vale is fine
|
||||
elif block_m:
|
||||
kind = 'block' # folded (`>`): the value starts on the next line
|
||||
elif indicator == '':
|
||||
kind = 'block' # bare `description:`: a plain scalar on later lines
|
||||
elif first[:1] == '"':
|
||||
kind = 'double'
|
||||
elif first[:1] == "'":
|
||||
kind = 'single'
|
||||
elif first[:1] in '#&*!':
|
||||
kind = None # comment, anchor, alias or tag — not a plain scalar
|
||||
else:
|
||||
kind = 'plain'
|
||||
|
||||
text = ''
|
||||
value_end = value_start
|
||||
value_lines = 0
|
||||
if kind in ('block', 'plain'):
|
||||
body = ''.join(continuation_lines(fm[body_start:]))
|
||||
value_end = body_start + len(body)
|
||||
if kind == 'block':
|
||||
text = body
|
||||
value_lines = body.count('\n')
|
||||
else:
|
||||
text = fm[value_start:value_end]
|
||||
value_lines = 1 + body.count('\n')
|
||||
if ' #' in text or text.lstrip().startswith('#'):
|
||||
# A `#` opens a comment inside a plain scalar. Folding it in
|
||||
# would lint text YAML never treats as part of the value, so
|
||||
# leave the file alone rather than lint the wrong string.
|
||||
kind = None
|
||||
elif kind in ('double', 'single'):
|
||||
quote = '"' if kind == 'double' else "'"
|
||||
inner_start = value_start + 1
|
||||
acc = fm[inner_start:body_start]
|
||||
idx = close_quote(acc, quote)
|
||||
lines = continuation_lines(fm[body_start:])
|
||||
while idx is None:
|
||||
try:
|
||||
acc += next(lines)
|
||||
except StopIteration:
|
||||
break
|
||||
body_lines.append(line)
|
||||
raw = ''.join(body_lines)
|
||||
if raw.count('\n') >= 2:
|
||||
flat = re.sub(r'\s+', ' ', raw).strip()
|
||||
# YAML single-quoted scalars have no backslash-escape mechanism at
|
||||
# all, so wrapping in single quotes sidesteps the backslash-escape
|
||||
# bug entirely for embedded double quotes, backslashes, and
|
||||
# non-ASCII text. The one YAML-spec-correct way to embed a literal
|
||||
# apostrophe is to double it ('') — but Vale's own frontmatter
|
||||
# scanner isn't a full YAML parser and doesn't understand that
|
||||
# doubling: empirically, it silently truncates the value at the
|
||||
# first ' it sees, hiding everything after it from the NLP scope
|
||||
# (a different flavor of the same bug this whole script exists to
|
||||
# work around). Since this copy is scratch-only and never written
|
||||
# back, sidestep it by substituting a Unicode right single
|
||||
# quotation mark (U+2019) for any literal apostrophe instead of
|
||||
# doubling it — visually a smart quote, but never triggers a YAML
|
||||
# escape sequence at all.
|
||||
flat_q = "'" + flat.replace("'", "’") + "'"
|
||||
pad = '\n' * raw.count('\n')
|
||||
start = header_m.start()
|
||||
end = header_m.end() + len(raw)
|
||||
new_fm = fm[:start] + f'description: {flat_q}\n{pad}' + fm[end:]
|
||||
content = fm_match.group(1) + new_fm + fm_match.group(3) + content[fm_match.end():]
|
||||
idx = close_quote(acc, quote)
|
||||
if idx is None:
|
||||
kind = None # unterminated quote: invalid YAML, leave it to vale
|
||||
else:
|
||||
inner = acc[:idx]
|
||||
value_end = inner_start + idx + 1
|
||||
text = unescape_double(inner) if quote == '"' else inner.replace("''", "'")
|
||||
value_lines = 1 + inner.count('\n')
|
||||
|
||||
flat = re.sub(r'\s+', ' ', text).strip()
|
||||
if kind and flat and value_lines >= 2:
|
||||
# `value_end` can land mid-line, just past a closing quote, so extend to
|
||||
# the end of that physical line and carry whatever follows (a trailing
|
||||
# comment) across unchanged.
|
||||
if value_end > 0 and fm[value_end - 1] == '\n':
|
||||
span_end = value_end
|
||||
trailer = ''
|
||||
else:
|
||||
newline = fm.find('\n', value_end)
|
||||
span_end = len(fm) if newline == -1 else newline + 1
|
||||
trailer = fm[value_end:span_end].rstrip('\n')
|
||||
scalar = emit(flat)
|
||||
# A trailing comment carried across from the original line stays on the
|
||||
# `description:` line itself: after a block scalar's `|-` header it is
|
||||
# still a comment, but inside the block body it would become part of the
|
||||
# value.
|
||||
head, newline_sep, block_body = scalar.partition('\n')
|
||||
# The replacement displaces the whole span, so the blank-line pad makes
|
||||
# up the difference between the lines it displaced and the lines it
|
||||
# occupies — every later line number is unchanged. That is one line for
|
||||
# the three inline forms and two for the `|-` block; the span itself is
|
||||
# at least two lines here (`value_lines >= 2` is a precondition), so the
|
||||
# pad count never goes negative.
|
||||
pad = '\n' * (fm[head_start:span_end].count('\n') - 1 - scalar.count('\n'))
|
||||
new_fm = (fm[:head_start] + 'description: ' + head + trailer
|
||||
+ newline_sep + block_body + '\n' + pad + fm[span_end:])
|
||||
content = (fm_match.group(1) + new_fm + fm_match.group(3)
|
||||
+ content[fm_match.end():])
|
||||
|
||||
with open(dest, 'w', encoding='utf-8', errors='surrogateescape') as fh:
|
||||
fh.write(content)
|
||||
|
||||
if tmpdir is not None:
|
||||
print(dest)
|
||||
PYTHON
|
||||
}
|
||||
|
||||
@@ -178,39 +474,46 @@ mirror="$tmpdir$cwd"
|
||||
mkdir -p "$mirror"
|
||||
|
||||
argv_paths=()
|
||||
for arg in "${path_args[@]}"; do
|
||||
for arg in ${path_args[@]+"${path_args[@]}"}; do
|
||||
if [[ "$arg" == /* ]]; then
|
||||
dest="$tmpdir$arg"
|
||||
raw_dest="$tmpdir$arg"
|
||||
else
|
||||
dest="$mirror/$arg"
|
||||
raw_dest="$mirror/$arg"
|
||||
fi
|
||||
dest="$(abspath "$dest")"
|
||||
# A path argument with enough leading `..` to climb past the mirror root would
|
||||
# write outside the scratch dir. The real filesystem clamps such a path at
|
||||
# `/`; the mirror can't, so refuse rather than scribble outside the sandbox.
|
||||
case "$dest" in
|
||||
"$tmpdir"/*) ;;
|
||||
*)
|
||||
echo "vale-wrap.sh: refusing to lint '$arg': its scratch copy would land outside $tmpdir" >&2
|
||||
exit 2
|
||||
;;
|
||||
esac
|
||||
mkdir -p "$(dirname "$dest")"
|
||||
if [[ -d "$arg" ]]; then
|
||||
dest="$(abspath "$raw_dest")"
|
||||
# A path argument with enough leading `..` to climb past the mirror root would
|
||||
# write outside the scratch dir. The real filesystem clamps such a path at
|
||||
# `/`; the mirror can't, so refuse rather than scribble outside the sandbox.
|
||||
case "$dest" in
|
||||
"$tmpdir"/*) ;;
|
||||
*)
|
||||
echo "vale-wrap.sh: refusing to lint '$arg': its scratch copy would land outside $tmpdir" >&2
|
||||
exit 2
|
||||
;;
|
||||
esac
|
||||
mkdir -p "$(dirname "$dest")"
|
||||
# A directory is mirrored whole — vale applies its own format filtering to
|
||||
# the tree, so any file dropped here would be silently unlinted — and then
|
||||
# every markdown file in the copy is flattened in place. `.git` is pruned:
|
||||
# vale never lints it and copying it can dwarf the rest of the tree.
|
||||
# `find -L` follows symlinks because vale does: it lints both a symlinked
|
||||
# file and a file under a symlinked directory, and a bare `-type f` walk
|
||||
# would report "0 files" where bare vale reports one. (A symlink loop makes
|
||||
# `find` warn on stderr and carry on, which is also what vale does.) The
|
||||
# second walk needs no `-L`: the mirror is all real files by construction.
|
||||
mkdir -p "$dest"
|
||||
while IFS= read -r -d '' rel; do
|
||||
mkdir -p "$dest/$(dirname "$rel")"
|
||||
cp "$arg/$rel" "$dest/$rel"
|
||||
done < <(cd "$arg" && find . -name .git -prune -o -type f -print0)
|
||||
done < <(cd "$arg" && find -L . -name .git -prune -o -type f -print0)
|
||||
while IFS= read -r -d '' md; do
|
||||
flatten "$md" "$md"
|
||||
done < <(find "$dest" -type f -name '*.md' -print0)
|
||||
else
|
||||
flatten "$arg" "$dest"
|
||||
# `abspath` + `flatten` folded into one python3 process — see the comment
|
||||
# atop `flatten` above.
|
||||
dest="$(flatten "$arg" "$raw_dest" "$tmpdir")"
|
||||
fi
|
||||
if [[ "$arg" == /* ]]; then
|
||||
argv_paths+=("$dest")
|
||||
@@ -220,4 +523,4 @@ for arg in "${path_args[@]}"; do
|
||||
done
|
||||
|
||||
cd "$mirror"
|
||||
vale "${vale_args[@]}" "${argv_paths[@]}"
|
||||
vale ${vale_args[@]+"${vale_args[@]}"} ${argv_paths[@]+"${argv_paths[@]}"}
|
||||
|
||||
@@ -133,12 +133,32 @@ else:
|
||||
if desc:
|
||||
ok("description has no unfilled placeholders")
|
||||
|
||||
# SKILL.md line count
|
||||
# SKILL.md size ceilings (agentskills.io skill-authoring.md: 500 lines,
|
||||
# ~5,000 tokens). Both constants are DUPLICATED from the repo-root pre-commit
|
||||
# hook scripts/skill-size-check.sh — a plugin skill's scripts cannot read files
|
||||
# outside the plugin directory once the plugin is cache-installed, so there is
|
||||
# no single source to share. Keep the two in sync by hand: if they drift, this
|
||||
# audit will report a skill ready to ship that the commit hook then rejects.
|
||||
MAX_LINES = 500
|
||||
# Word-count proxy for the ~5,000-token ceiling, calibrated to the densest
|
||||
# prose in the corpus (7.22 chars/word): 2770 words is ~20,000 characters,
|
||||
# ~5,000 tokens at 4 characters per token. See skill-size-check.sh's header
|
||||
# for the full measurement.
|
||||
MAX_WORDS = 2770
|
||||
|
||||
line_count = len(content.splitlines())
|
||||
if line_count <= 500:
|
||||
ok(f"SKILL.md line count {line_count} (limit: 500)")
|
||||
if line_count <= MAX_LINES:
|
||||
ok(f"SKILL.md line count {line_count} (limit: {MAX_LINES})")
|
||||
else:
|
||||
fail(f"SKILL.md line count {line_count} — exceeds 500-line limit")
|
||||
fail(f"SKILL.md line count {line_count} — exceeds {MAX_LINES}-line limit")
|
||||
|
||||
# str.split() with no argument splits on runs of whitespace, matching the
|
||||
# `wc -w` the hook uses, and counts the whole file including frontmatter.
|
||||
word_count = len(content.split())
|
||||
if word_count <= MAX_WORDS:
|
||||
ok(f"SKILL.md word count {word_count} (limit: {MAX_WORDS}, proxy for ~5,000 tokens)")
|
||||
else:
|
||||
fail(f"SKILL.md word count {word_count} — exceeds {MAX_WORDS}-word limit (proxy for ~5,000 tokens)")
|
||||
|
||||
# Body unfilled placeholders
|
||||
body = content[body_start:]
|
||||
|
||||
@@ -13,5 +13,5 @@
|
||||
],
|
||||
"license": "MIT",
|
||||
"name": "lint",
|
||||
"version": "1.1.4"
|
||||
"version": "1.1.5"
|
||||
}
|
||||
|
||||
@@ -8,7 +8,14 @@ source_keys:
|
||||
|
||||
Vale supports inline markup comments to disable checks for a section of content. Syntax varies by format:
|
||||
|
||||
Markdown/MDX:
|
||||
Markdown — HTML comments; the MDX `{/* */}` form suppresses nothing in a plain `.md` file:
|
||||
```markdown
|
||||
<!-- vale off -->
|
||||
This text will be ignored.
|
||||
<!-- vale on -->
|
||||
```
|
||||
|
||||
MDX:
|
||||
```mdx
|
||||
{/* vale off */}
|
||||
This text will be ignored.
|
||||
@@ -24,7 +31,15 @@ This text will be ignored.
|
||||
|
||||
## Disabling a Specific Rule for Specific Matches
|
||||
|
||||
Rather than disabling all checks, target one rule and specific known-exception strings, then re-enable:
|
||||
Rather than disabling all checks, target one rule and specific known-exception strings, then re-enable. Same per-format comment syntax as above — Markdown:
|
||||
|
||||
```markdown
|
||||
<!-- vale Style.Redundancy["ACT test","OTHER"] = NO -->
|
||||
This is some text ACT test
|
||||
<!-- vale Style.Redundancy["ACT test","OTHER"] = YES -->
|
||||
```
|
||||
|
||||
MDX:
|
||||
|
||||
```mdx
|
||||
{/* vale Style.Redundancy["ACT test","OTHER"] = NO */}
|
||||
|
||||
@@ -18,5 +18,5 @@
|
||||
"skills": [
|
||||
"skills/"
|
||||
],
|
||||
"version": "1.1.4"
|
||||
"version": "1.1.5"
|
||||
}
|
||||
|
||||
@@ -19,10 +19,10 @@ metadata:
|
||||
|
||||
## Gotchas
|
||||
|
||||
- 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.
|
||||
- Installing the `vale` binary installs no styles, but only *package* styles need fetching. A fresh `.vale.ini` naming a style in `BasedOnStyles` that is declared in `Packages` will fail or find nothing until `vale sync` downloads it. A built-in style (`Vale`) or a style whose YAML rule files are already committed under `StylesPath` lints immediately, with no `Packages` entry and no sync.
|
||||
- `.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.<key>` (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.
|
||||
- A rule scoped to `text.frontmatter.<key>` (e.g. `text.frontmatter.description`) matches reliably when that field's value is a single physical line, and breaks on most — not all — multi-line forms. Confirmed against Vale 3.15.2 with a deliberately-bad fixture: a `>` folded block scalar, plain (unquoted) continuation lines, and single- or double-quoted multi-line scalars each yield 0 findings and exit 0, silently and with no error; a `|` literal block scalar spanning the same 2+ lines lints normally and exits 1. Do not assume `|` and `>` behave alike — reproduce both against your own config before trusting a frontmatter-scoped rule in production. If the field is commonly authored in one of the broken forms, flatten it to one physical line ahead of the `vale` call rather than relying on the scope alone.
|
||||
|
||||
## Setup workflow
|
||||
|
||||
|
||||
@@ -21,6 +21,27 @@ if [[ "${PRE_COMMIT_REMOTE_BRANCH:-}" != "$TARGET_BRANCH" ]]; then
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# What is actually being pushed, which is only HEAD for the common
|
||||
# `git push <remote> <current-branch>` case. pre-commit's pre-push hook-impl
|
||||
# exports the local sha of each pushed ref as PRE_COMMIT_TO_REF; a
|
||||
# `git push <remote> topic:main` from a different checkout would otherwise be
|
||||
# gated on the wrong tip — a false negative when HEAD is behind the pushed ref
|
||||
# (unreleased changes sail through), a false positive when it is ahead.
|
||||
# PRE_COMMIT_FROM_REF, the *remote's* current tip, is deliberately not used
|
||||
# anywhere here: the baseline is the last release tag, not what the remote
|
||||
# already has. Diffing from the remote tip would let an untagged
|
||||
# release-relevant commit already on main excuse the next push from cutting a
|
||||
# tag, which is precisely the drift this gate exists to catch.
|
||||
PUSHED_REF="${PRE_COMMIT_TO_REF:-HEAD}"
|
||||
|
||||
# pre-commit passes an all-zeros sha (40 hex zeros under sha1, 64 under sha256)
|
||||
# as the "to" ref when the push deletes a branch. Nothing is being shipped, and
|
||||
# every rev-taking command below would fail on an unresolvable sha, so bail out
|
||||
# rather than turning a branch deletion into a confusing "could not diff".
|
||||
if [[ "$PUSHED_REF" =~ ^0+$ ]]; then
|
||||
exit 0
|
||||
fi
|
||||
|
||||
REPO_ROOT="$(git rev-parse --show-toplevel)"
|
||||
cd "$REPO_ROOT"
|
||||
|
||||
@@ -30,6 +51,23 @@ if [[ ! -f "$HOOKS_MANIFEST" ]]; then
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# Only vX.Y.Z release tags count as a baseline — an incidental checkpoint or
|
||||
# experiment tag reachable from the pushed ref must not shift the diff baseline.
|
||||
# The tag is resolved from $PUSHED_REF, not HEAD, for the same reason the diff
|
||||
# is: a tag reachable only from HEAD is not part of the history being pushed.
|
||||
# --match is a shell glob, not a regex: its trailing `*`s match any suffix, so
|
||||
# without --exclude a pre-release/checkpoint tag like v1.2.3-checkpoint or
|
||||
# v1.2.3-rc1 also satisfies 'v[0-9]*.[0-9]*.[0-9]*' and could be picked over the
|
||||
# true last release tag. --exclude is glob syntax too, so '*-*' is what actually
|
||||
# rules out any tag carrying a hyphenated suffix, leaving only bare vMAJOR.MINOR.PATCH.
|
||||
LAST_TAG="$(git describe --tags --abbrev=0 --match 'v[0-9]*.[0-9]*.[0-9]*' --exclude '*-*' "$PUSHED_REF" 2>/dev/null || true)"
|
||||
|
||||
if [[ -z "$LAST_TAG" ]]; then
|
||||
echo "FAIL: no release tag exists yet, but .pre-commit-hooks.yaml already exposes hooks to external consumers." >&2
|
||||
echo " Fix: cut the first release tag (e.g. v1.0.0) before this lands on main." >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Derive release-relevant paths from .pre-commit-hooks.yaml's own entry: lines
|
||||
# instead of hand-maintaining a parallel list — the manifest is the single
|
||||
# source of truth for what external consumers actually pull at a pinned rev,
|
||||
@@ -46,36 +84,152 @@ fi
|
||||
# from inventing paths: a bundle root of "." is skipped, because a script in a
|
||||
# top-level directory (scripts/skill-size-check.sh) would derive the repo's own
|
||||
# shared assets/, which no hook owns and whose churn must not demand a release;
|
||||
# and the directory is added only when it exists, since a hook that bundles
|
||||
# nothing must not contribute a pathspec matching nothing.
|
||||
# and the assets/ directory is added only where it is known to exist, since a
|
||||
# hook that bundles nothing must not contribute a pathspec matching nothing.
|
||||
RELEASE_PATHS=("$HOOKS_MANIFEST")
|
||||
while IFS= read -r entry; do
|
||||
read -ra tokens <<< "$entry"
|
||||
[[ ${#tokens[@]} -eq 0 ]] && continue
|
||||
RELEASE_PATHS+=("${tokens[0]}")
|
||||
bundle_root="$(dirname "$(dirname "${tokens[0]}")")"
|
||||
if [[ "$bundle_root" != "." && -d "$bundle_root/assets" ]]; then
|
||||
RELEASE_PATHS+=("$bundle_root/assets")
|
||||
|
||||
add_release_path() {
|
||||
local candidate="$1" existing
|
||||
for existing in "${RELEASE_PATHS[@]}"; do
|
||||
[[ "$existing" == "$candidate" ]] && return 0
|
||||
done
|
||||
RELEASE_PATHS+=("$candidate")
|
||||
}
|
||||
|
||||
# Emits one "<hook id><TAB><entry value>" line per hook so a rejected entry can
|
||||
# name the hook a human has to go fix. The id sits on its own line above its
|
||||
# entry: in YAML, so it is carried forward and then cleared; a hook that somehow
|
||||
# has no id still reports something printable rather than an empty name. Kept in
|
||||
# bash rather than awk: matching `[[:space:]]` inside a bracket expression is
|
||||
# reliable in bash's own globs but not in the BWK awk macOS ships. `read -r` with
|
||||
# a single variable is the trimmer — it strips leading and trailing whitespace
|
||||
# while preserving anything in between, so a multi-token entry survives intact
|
||||
# for the error message to quote back.
|
||||
manifest_entries() {
|
||||
local line id="" value
|
||||
while IFS= read -r line; do
|
||||
# Drop the indentation and the optional list dash, so that `- id: x` and
|
||||
# ` entry: y` both reduce to the same bare "key: value" shape.
|
||||
line="${line#"${line%%[![:space:]]*}"}"
|
||||
if [[ "$line" == -* ]]; then
|
||||
line="${line#-}"
|
||||
line="${line#"${line%%[![:space:]]*}"}"
|
||||
fi
|
||||
case "$line" in
|
||||
id:*)
|
||||
read -r id <<< "${line#id:}"
|
||||
;;
|
||||
entry:*)
|
||||
read -r value <<< "${line#entry:}"
|
||||
printf '%s\t%s\n' "${id:-(unnamed hook)}" "$value"
|
||||
id=""
|
||||
;;
|
||||
esac
|
||||
done
|
||||
}
|
||||
|
||||
# A hook's script is legitimate if it exists in the working tree *or* at
|
||||
# $LAST_TAG — the same union the pathspec itself spans. Checking per-scope
|
||||
# instead would reject exactly the case this gate exists to flag: a script
|
||||
# deleted since the tag while its entry survives (see the no -e filtering note
|
||||
# further down) is a real deletion to report, not a malformed manifest.
|
||||
entry_path_exists() {
|
||||
local candidate="$1"
|
||||
[[ -e "$candidate" ]] && return 0
|
||||
git cat-file -e "$LAST_TAG:$candidate" 2>/dev/null && return 0
|
||||
return 1
|
||||
}
|
||||
|
||||
# $1 selects where the "does this hook bundle an assets/ tree?" guard looks:
|
||||
# "worktree" probes the filesystem, anything else is a rev whose tree is probed
|
||||
# with git plumbing. Reading entry lines from stdin keeps one derivation for
|
||||
# both the tagged manifest and the current one.
|
||||
collect_release_paths() {
|
||||
local scope="$1" line hook_id entry bundle_root where
|
||||
local -a tokens
|
||||
if [[ "$scope" == "worktree" ]]; then
|
||||
where="the working tree's $HOOKS_MANIFEST"
|
||||
else
|
||||
where="$HOOKS_MANIFEST at $scope"
|
||||
fi
|
||||
done < <(sed -n 's/^[[:space:]]*entry:[[:space:]]*//p' "$HOOKS_MANIFEST")
|
||||
while IFS= read -r line; do
|
||||
hook_id="${line%%$'\t'*}"
|
||||
entry="${line#*$'\t'}"
|
||||
read -ra tokens <<< "$entry"
|
||||
[[ ${#tokens[@]} -eq 0 ]] && continue
|
||||
# ADR-0014 binds every entry to a bare script path and nothing else, because
|
||||
# pre-commit rewrites only entry[0] into the hook-repo clone. That is a
|
||||
# constraint nothing else enforces, and the sibling .pre-commit-config.yaml
|
||||
# already ships the multi-token `bash <script>` shape one copy-paste away —
|
||||
# so an entry like `bash scripts/foo.sh` would add "bash" as a pathspec that
|
||||
# matches nothing and derive a bundle root of ".", dropping that hook's
|
||||
# entire surface out of the gate silently. Both malformed shapes below fail
|
||||
# loudly instead: silent degradation here is the same class of defect as the
|
||||
# --config token already recorded in LESSONS.md.
|
||||
if [[ ${#tokens[@]} -gt 1 ]]; then
|
||||
echo "FAIL: hook '$hook_id' in $where has a multi-token entry: $entry" >&2
|
||||
echo " Why: pre-commit rewrites only entry[0] into the hook-repo clone, so every later" >&2
|
||||
echo " token resolves against the *consuming* repo and can never name a file this" >&2
|
||||
echo " repo ships — and this gate would derive its release paths from '${tokens[0]}'." >&2
|
||||
echo " Fix: make the entry a bare script path and have the script self-locate anything" >&2
|
||||
echo " else from \${BASH_SOURCE[0]} (see ADR-0014, 'Consequences')." >&2
|
||||
exit 1
|
||||
fi
|
||||
if ! entry_path_exists "${tokens[0]}"; then
|
||||
echo "FAIL: hook '$hook_id' in $where names a path that exists neither in the working tree nor at $LAST_TAG: ${tokens[0]}" >&2
|
||||
echo " Why: this gate derives its release-relevant pathspec from that path, so a name" >&2
|
||||
echo " that resolves to no file silently drops the hook's whole surface from the diff." >&2
|
||||
echo " Fix: point the entry at a script path this repo actually ships (see ADR-0014," >&2
|
||||
echo " 'Consequences'); a bare command name is not a valid entry here." >&2
|
||||
exit 1
|
||||
fi
|
||||
add_release_path "${tokens[0]}"
|
||||
bundle_root="$(dirname "$(dirname "${tokens[0]}")")"
|
||||
[[ "$bundle_root" == "." ]] && continue
|
||||
if [[ "$scope" == "worktree" ]]; then
|
||||
[[ -d "$bundle_root/assets" ]] && add_release_path "$bundle_root/assets"
|
||||
else
|
||||
git cat-file -e "$scope:$bundle_root/assets" 2>/dev/null && add_release_path "$bundle_root/assets"
|
||||
fi
|
||||
done
|
||||
return 0
|
||||
}
|
||||
|
||||
# Only vX.Y.Z release tags count as a baseline — an incidental checkpoint or
|
||||
# experiment tag reachable from HEAD must not shift the diff baseline.
|
||||
LAST_TAG="$(git describe --tags --abbrev=0 --match 'v[0-9]*.[0-9]*.[0-9]*' 2>/dev/null || true)"
|
||||
# The worktree alone is not enough: a path is release-relevant if it was part of
|
||||
# the contract at $LAST_TAG *or* is part of it now, so both trees have to be
|
||||
# derived and unioned. Deriving only from the worktree meant that deleting a
|
||||
# hook's entire assets/ tree made the `-d` guard drop the path from the pathspec
|
||||
# altogether, and the deletion — which breaks every consumer at the next rev —
|
||||
# diffed clean. The two manifests can genuinely disagree (an entry added,
|
||||
# removed, or renamed since the tag), and the union is the conservative side of
|
||||
# that disagreement: a path the tag exposed and HEAD no longer does is a removal
|
||||
# consumers must be told about, and a path only HEAD exposes is new contract
|
||||
# surface they cannot reach without a new tag. The union never over-fires on its
|
||||
# own, either — any manifest edit that makes the two disagree already changes
|
||||
# $HOOKS_MANIFEST, which is itself a release-relevant path.
|
||||
collect_release_paths worktree < <(manifest_entries < "$HOOKS_MANIFEST")
|
||||
|
||||
if [[ -z "$LAST_TAG" ]]; then
|
||||
echo "FAIL: no release tag exists yet, but .pre-commit-hooks.yaml already exposes hooks to external consumers." >&2
|
||||
echo " Fix: cut the first release tag (e.g. v1.0.0) before this lands on main." >&2
|
||||
# A missing manifest at the tag is legitimate (the manifest was added since) but
|
||||
# is indistinguishable from an unreadable tagged tree by its exit status alone,
|
||||
# so the tag's root tree is verified separately. An absent tree object — a
|
||||
# shallow clone, a truncated fetch — fails closed exactly like a `git diff`
|
||||
# failure does, rather than silently degrading to worktree-only derivation.
|
||||
if MANIFEST_AT_TAG="$(git cat-file -p "$LAST_TAG:$HOOKS_MANIFEST" 2>/dev/null)"; then
|
||||
collect_release_paths "$LAST_TAG" < <(printf '%s\n' "$MANIFEST_AT_TAG" | manifest_entries)
|
||||
elif ! git cat-file -e "$LAST_TAG^{tree}" 2>/dev/null; then
|
||||
echo "FAIL: could not read the tree at $LAST_TAG to determine which paths that release exposed." >&2
|
||||
echo " Fix: ensure full tag history is available (e.g. git fetch --unshallow) and retry." >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# No -e/existence filtering: a path deleted since $LAST_TAG is exactly the case
|
||||
# that must be caught (external consumers pinning the old tag would hit a
|
||||
# missing file), and `git diff` reports deletions fine without it existing at
|
||||
# HEAD. A git failure (e.g. a shallow clone missing $LAST_TAG's history) must
|
||||
# fail closed, not be swallowed into an empty, falsely-clean diff.
|
||||
if ! CHANGED="$(git diff --name-only "$LAST_TAG"..HEAD -- "${RELEASE_PATHS[@]}")"; then
|
||||
echo "FAIL: could not diff $LAST_TAG..HEAD to check for release-relevant changes (see git error above)." >&2
|
||||
# No -e/existence filtering on the pathspec: a path deleted since $LAST_TAG is
|
||||
# exactly the case that must be caught (external consumers pinning the old tag
|
||||
# would hit a missing file), and `git diff` reports deletions fine without it
|
||||
# existing at the pushed ref. A git failure (e.g. a shallow clone missing
|
||||
# $LAST_TAG's history) must fail closed, not be swallowed into an empty,
|
||||
# falsely-clean diff.
|
||||
if ! CHANGED="$(git diff --name-only "$LAST_TAG".."$PUSHED_REF" -- "${RELEASE_PATHS[@]}")"; then
|
||||
echo "FAIL: could not diff $LAST_TAG..$PUSHED_REF to check for release-relevant changes (see git error above)." >&2
|
||||
echo " Fix: ensure full tag history is available (e.g. git fetch --unshallow) and retry." >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
@@ -10,6 +10,17 @@ set -euo pipefail
|
||||
# only one of the two. Run from repo root or pass REPO_ROOT as arg.
|
||||
|
||||
REPO_ROOT="${1:-$(git rev-parse --show-toplevel 2>/dev/null || pwd)}"
|
||||
# A nonexistent REPO_ROOT must fail loudly, not fall through to the "neither
|
||||
# copy present" no-op below — that guard exists for a repo that legitimately
|
||||
# has no kyberforge plugin installed, not for a typo'd or stale path, and a
|
||||
# clean exit 0 here would read as "checked, in sync" when nothing ran at all.
|
||||
if [[ ! -d "$REPO_ROOT" ]]; then
|
||||
echo "Vale style sync check failed: REPO_ROOT '$REPO_ROOT' is not a directory." >&2
|
||||
exit 1
|
||||
fi
|
||||
# Absolutized because the glob probe below `cd`s into a scratch tree, where a
|
||||
# relative --config path would stop resolving.
|
||||
REPO_ROOT="$(cd "$REPO_ROOT" && pwd)"
|
||||
FAIL=0
|
||||
|
||||
err() { echo " FAIL: $1" >&2; FAIL=$((FAIL + 1)); }
|
||||
@@ -41,7 +52,150 @@ if ! diff -rq "$SKILL_AUDIT/assets/vale/styles/Kyberforge" "$AGENT_AUDIT/assets/
|
||||
err "assets/vale/styles/Kyberforge differs between skill-audit and agent-audit"
|
||||
fi
|
||||
|
||||
# --- .vale.ini coverage ------------------------------------------------------
|
||||
# The two .vale.ini files are deliberately NOT identical — agent-audit's carries
|
||||
# an extra [**/*.agent.md] section and the KyberforgeCopilot style — so they
|
||||
# cannot be diffed like the styles above. Nothing else in the repo read them at
|
||||
# all, and that is what let a one-character glob typo silently disable the
|
||||
# prefilter for a whole file type: the hook still MATCHES the file via its
|
||||
# `files:` regex, so pre-commit reports neither `Skipped` nor an error; vale
|
||||
# lints zero files, prints `0 errors ... in 1 file` and exits 0, and the hook
|
||||
# shows `Passed`. So check the parts that must hold in both, not equality.
|
||||
|
||||
SKILL_INI="$SKILL_AUDIT/assets/vale/.vale.ini"
|
||||
AGENT_INI="$AGENT_AUDIT/assets/vale/.vale.ini"
|
||||
|
||||
for ini in "$SKILL_INI" "$AGENT_INI"; do
|
||||
rel_ini="${ini#"$REPO_ROOT"/}"
|
||||
if [[ ! -f "$ini" ]]; then
|
||||
err "$rel_ini is missing — without it vale falls back to an upward config search and lints with whatever it finds"
|
||||
continue
|
||||
fi
|
||||
# StylesPath is resolved relative to the .vale.ini, which is the only reason
|
||||
# the bundled styles are found from a consuming repo's clone prefix.
|
||||
if ! grep -Eq '^[[:space:]]*StylesPath[[:space:]]*=[[:space:]]*styles[[:space:]]*$' "$ini"; then
|
||||
err "$rel_ini has no 'StylesPath = styles' — the bundled styles/ directory would not be found"
|
||||
fi
|
||||
# Matches `Kyberforge` as a whole name, so `KyberforgeCopilot` alone does not
|
||||
# satisfy it. Avoids \b, which is a GNU grep extension.
|
||||
if ! grep -Eq '^[[:space:]]*BasedOnStyles[[:space:]]*=.*Kyberforge([[:space:],]|$)' "$ini"; then
|
||||
err "$rel_ini has no section whose BasedOnStyles names Kyberforge — every rule the audit prefilters on lives in that style"
|
||||
fi
|
||||
done
|
||||
|
||||
# Prints the `files:` regex of every hook, in either manifest, whose entry is
|
||||
# $1's vale-wrap.sh. Records are delimited by their `- id:` line, so the check
|
||||
# does not depend on `entry:` preceding `files:` within a record.
|
||||
#
|
||||
# Cached per skill (parallel HOOK_REGEX_CACHE_KEYS/_VALS arrays, populated
|
||||
# lazily) because the final validation loop below probes agent-audit twice —
|
||||
# once for its CC agent-file shape, once for its Copilot .agent.md shape — and
|
||||
# both probes need the same regex set. Without the cache, that pair of calls
|
||||
# would each re-parse both manifest files from scratch for no new information.
|
||||
# Plain indexed arrays, not `declare -A`: associative arrays are bash 4.0+ and
|
||||
# this script must run on macOS's stock bash 3.2. Only ${#arr[@]} (always safe
|
||||
# on an empty/unset array under `set -u`) and index access are used below —
|
||||
# never a bare `${arr[@]}` expansion, which aborts on bash < 4.4 under nounset.
|
||||
HOOK_REGEX_CACHE_KEYS=()
|
||||
HOOK_REGEX_CACHE_VALS=()
|
||||
hook_file_regexes() {
|
||||
local skill="$1" manifest raw result idx=0
|
||||
while [[ $idx -lt ${#HOOK_REGEX_CACHE_KEYS[@]} ]]; do
|
||||
if [[ "${HOOK_REGEX_CACHE_KEYS[$idx]}" == "$skill" ]]; then
|
||||
printf '%s' "${HOOK_REGEX_CACHE_VALS[$idx]}"
|
||||
return
|
||||
fi
|
||||
idx=$((idx + 1))
|
||||
done
|
||||
result="$(
|
||||
for manifest in "$REPO_ROOT/.pre-commit-hooks.yaml" "$REPO_ROOT/.pre-commit-config.yaml"; do
|
||||
[[ -f "$manifest" ]] || continue
|
||||
awk -v skill="$skill" '
|
||||
function flush() {
|
||||
if (entry ~ skill "/scripts/vale-wrap.sh" && files != "") print files
|
||||
entry = ""; files = ""
|
||||
}
|
||||
/^[ \t]*-[ \t]*id:/ { flush() }
|
||||
/^[ \t]*entry:/ { entry = $0 }
|
||||
/^[ \t]*files:/ { files = $0; sub(/^[ \t]*files:[ \t]*/, "", files) }
|
||||
END { flush() }
|
||||
' "$manifest"
|
||||
done | while IFS= read -r raw; do
|
||||
# Strip the surrounding YAML quotes; the regex itself never carries them.
|
||||
raw="${raw%\'}"; raw="${raw#\'}"
|
||||
raw="${raw%\"}"; raw="${raw#\"}"
|
||||
printf '%s\n' "$raw"
|
||||
done
|
||||
)"
|
||||
HOOK_REGEX_CACHE_KEYS[${#HOOK_REGEX_CACHE_KEYS[@]}]="$skill"
|
||||
HOOK_REGEX_CACHE_VALS[${#HOOK_REGEX_CACHE_VALS[@]}]="$result"
|
||||
printf '%s' "$result"
|
||||
}
|
||||
|
||||
# Asks vale — the thing that actually applies these globs — whether a config
|
||||
# covers a path, rather than reimplementing doublestar matching. The probe file
|
||||
# carries a description with a token Kyberforge.VagueWording flags, so a config
|
||||
# whose glob matches but whose BasedOnStyles lost Kyberforge fails too: it would
|
||||
# lint the file and report nothing.
|
||||
vale_flags_path() {
|
||||
local cfg="$1" rel="$2" tmp out
|
||||
tmp="$(mktemp -d)"
|
||||
mkdir -p "$tmp/$(dirname "$rel")"
|
||||
{
|
||||
echo "---"
|
||||
echo "name: probe"
|
||||
echo "description: Use when the caller wants a probe that helps with things."
|
||||
echo "---"
|
||||
echo ""
|
||||
echo "Body."
|
||||
} > "$tmp/$rel"
|
||||
out="$(cd "$tmp" && vale --config "$cfg" "$rel" 2>&1)" || true
|
||||
rm -rf "$tmp"
|
||||
printf '%s\n' "$out" | grep -qF "Kyberforge.VagueWording"
|
||||
}
|
||||
|
||||
VALE_AVAILABLE=true
|
||||
if ! command -v vale >/dev/null 2>&1; then
|
||||
VALE_AVAILABLE=false
|
||||
echo " WARNING: vale is not installed — .vale.ini glob coverage was NOT verified. Install it (https://vale.sh/docs/vale-cli/installation/) before trusting a clean run." >&2
|
||||
fi
|
||||
|
||||
# One representative path per file shape the prefilter is supposed to cover. Each
|
||||
# is cross-checked against the shipped hooks' `files:` regexes first, so a path
|
||||
# that goes stale because a hook was rescoped fails loudly here instead of
|
||||
# quietly probing a shape nothing lints any more.
|
||||
while IFS='|' read -r skill rel; do
|
||||
[[ -n "$skill" ]] || continue
|
||||
dir="$REPO_ROOT/plugins/kyberforge/skills/$skill"
|
||||
ini="$dir/assets/vale/.vale.ini"
|
||||
[[ -f "$ini" ]] || continue
|
||||
|
||||
regexes="$(hook_file_regexes "$skill")"
|
||||
if [[ -n "$regexes" ]]; then
|
||||
in_scope=false
|
||||
while IFS= read -r re; do
|
||||
[[ -n "$re" ]] || continue
|
||||
if printf '%s\n' "$rel" | grep -Eq "$re"; then
|
||||
in_scope=true
|
||||
fi
|
||||
done <<EOF_RE
|
||||
$regexes
|
||||
EOF_RE
|
||||
if [[ "$in_scope" == false ]]; then
|
||||
err "$rel matches no 'files:' regex of any $skill hook — the probe path is stale, or the hook was rescoped away from a shape it still needs to lint"
|
||||
fi
|
||||
fi
|
||||
|
||||
if [[ "$VALE_AVAILABLE" == true ]] && ! vale_flags_path "$ini" "$rel"; then
|
||||
err "$skill/assets/vale/.vale.ini raises no Kyberforge alert on $rel — its glob sections do not cover a path its own pre-commit hook is scoped to, so the hook passes that shape without linting it"
|
||||
fi
|
||||
done <<'EOF_PROBE'
|
||||
skill-audit|plugins/demo/skills/demo/SKILL.md
|
||||
agent-audit|plugins/demo/agents/demo.md
|
||||
agent-audit|copilot/demo.agent.md
|
||||
EOF_PROBE
|
||||
|
||||
if [[ $FAIL -gt 0 ]]; then
|
||||
echo "Vale style sync check failed: $FAIL error(s). agent-audit's copy is canonical — run scripts/sync-vale-styles.sh to regenerate skill-audit's copy, then commit both." >&2
|
||||
echo "Vale style sync check failed: $FAIL error(s). For a drifted wrapper or style, agent-audit's copy is canonical — run scripts/sync-vale-styles.sh to regenerate skill-audit's copy, then commit both. A .vale.ini finding is not drift and sync-vale-styles.sh will not fix it: edit that file's own StylesPath, BasedOnStyles or glob sections." >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
@@ -15,30 +15,53 @@ set -euo pipefail
|
||||
# audit and still be blocked by the commit hook.
|
||||
#
|
||||
# Token counts aren't computed exactly here — word count (`wc -w`) is used as
|
||||
# a proxy. This repo's own SKILL.md corpus measures ~5.7-6.5 characters per
|
||||
# word, which at the standard ~4-characters-per-token English approximation
|
||||
# works out to roughly 1.6-1.7 tokens per word. MAX_WORDS below is calibrated
|
||||
# from that measured ratio against the 5,000-token ceiling, with margin — it's
|
||||
# still a proxy, not exact BPE tokenization, but now grounded in actual repo
|
||||
# content rather than an unverified "conservative" assumption.
|
||||
# a proxy. Measured over this repo's 39 in-scope SKILL.md files, characters per
|
||||
# word runs min 5.97 / median 6.79 / mean 6.77 / max 7.22. At the standard
|
||||
# ~4-characters-per-token English approximation that is 1.49 / 1.70 / 1.69 /
|
||||
# 1.81 tokens per word.
|
||||
#
|
||||
# MAX_WORDS=2770 is therefore calibrated to the corpus WORST case rather than
|
||||
# its median: 2770 words at the densest observed 7.22 chars/word is ~20,000
|
||||
# characters, or ~5,000 tokens at the 4-characters-per-token approximation. So
|
||||
# what this gate guarantees is "under 5,000 tokens even for the densest prose
|
||||
# the corpus has produced" — the earlier median-calibrated MAX_WORDS=2900 let
|
||||
# such a file sit at exactly the ceiling and still spend ~5,240 tokens. A
|
||||
# median-density file at 2770 words spends ~4,700 tokens, so typical prose
|
||||
# gives up ~130 words of headroom to close that gap. The largest SKILL.md in
|
||||
# the repo is 2,489 words, so no current file is affected.
|
||||
#
|
||||
# It is a one-sided proxy in the useful direction — nothing under the word
|
||||
# ceiling is wildly over the token ceiling — but it is not exact BPE
|
||||
# tokenization and does not replace one. Re-measure the corpus before treating
|
||||
# any of these numbers as still current.
|
||||
|
||||
# These constants are intentionally duplicated in
|
||||
# skill-audit/scripts/validate.sh (Python) rather than shared from one file:
|
||||
# this script is a standalone bash pre-commit hook, that one is an in-skill
|
||||
# Python validator invoked in a different context (same rationale as
|
||||
# vale-wrap.sh's per-plugin duplication — see its own header comment).
|
||||
# tests/test-skill-size-check.sh asserts both files agree on these values, so
|
||||
# drift between them fails CI rather than silently diverging.
|
||||
MAX_LINES=500
|
||||
MAX_WORDS=2900
|
||||
MAX_WORDS=2770
|
||||
FAIL=0
|
||||
|
||||
for f in "$@"; do
|
||||
[[ -f "$f" ]] || continue
|
||||
|
||||
# awk's NR counts the final line even without a trailing newline, matching
|
||||
# Python's splitlines() semantics (used by skill-audit/scripts/validate.sh
|
||||
# for its own line count) — `wc -l` undercounts by 1 in that case.
|
||||
lines=$(awk 'END{print NR}' "$f")
|
||||
# Single awk pass computes both line count and word count, avoiding a
|
||||
# second read of the file. NR counts the final line even without a
|
||||
# trailing newline, matching Python's splitlines() semantics (used by
|
||||
# skill-audit/scripts/validate.sh for its own line count) — `wc -l`
|
||||
# undercounts by 1 in that case. Word count uses awk's default
|
||||
# whitespace-splitting NF, matching `wc -w` semantics.
|
||||
read -r lines words <<< "$(awk '{w += NF} END{print NR, w+0}' "$f")"
|
||||
|
||||
if (( lines > MAX_LINES )); then
|
||||
echo "ERROR: $f has $lines lines, exceeding the $MAX_LINES-line ceiling (agentskills.io skill-authoring.md)" >&2
|
||||
FAIL=1
|
||||
fi
|
||||
|
||||
words=$(wc -w < "$f")
|
||||
if (( words > MAX_WORDS )); then
|
||||
echo "ERROR: $f has $words words (proxy for tokens), exceeding the $MAX_WORDS-word ceiling (~5,000 tokens, agentskills.io skill-authoring.md)" >&2
|
||||
FAIL=1
|
||||
|
||||
@@ -35,14 +35,24 @@ fi
|
||||
|
||||
run_bats
|
||||
|
||||
mapfile -t SCRIPTS < <(
|
||||
# Collected with a `while read` loop rather than `mapfile` — macOS ships
|
||||
# /bin/bash 3.2, which has no `mapfile`. Process substitution (not a pipe)
|
||||
# keeps the loop in this shell so the appends survive. `sort` is still fed
|
||||
# newline-delimited output, exactly as before.
|
||||
SCRIPTS=()
|
||||
while IFS= read -r script; do
|
||||
SCRIPTS+=("$script")
|
||||
done < <(
|
||||
find "$SEARCH_ROOT" -name "test-*.sh" \
|
||||
-not -path "*/.git/*" \
|
||||
-not -path "*/.claude/worktrees/*" \
|
||||
| sort
|
||||
)
|
||||
|
||||
for script in "${SCRIPTS[@]}"; do
|
||||
# bash before 4.4 treats "${arr[@]}" on an empty array as unbound under
|
||||
# `set -u`, so every array expansion here uses the ${arr[@]+"${arr[@]}"} guard,
|
||||
# including the SKIPPED/FAILED loops already fenced by a count check.
|
||||
for script in ${SCRIPTS[@]+"${SCRIPTS[@]}"}; do
|
||||
rel="${script#"$SEARCH_ROOT/"}"
|
||||
echo "=== $rel ==="
|
||||
rc=0
|
||||
@@ -60,13 +70,13 @@ done
|
||||
echo "=== Summary: $PASSED passed, ${#SKIPPED[@]} skipped, ${#FAILED[@]} failed ==="
|
||||
if [[ ${#SKIPPED[@]} -gt 0 ]]; then
|
||||
echo "Skipped scripts:"
|
||||
for s in "${SKIPPED[@]}"; do
|
||||
for s in ${SKIPPED[@]+"${SKIPPED[@]}"}; do
|
||||
echo " $s"
|
||||
done
|
||||
fi
|
||||
if [[ ${#FAILED[@]} -gt 0 ]]; then
|
||||
echo "Failed scripts:"
|
||||
for s in "${FAILED[@]}"; do
|
||||
for s in ${FAILED[@]+"${FAILED[@]}"}; do
|
||||
echo " $s"
|
||||
done
|
||||
exit 1
|
||||
|
||||
@@ -52,9 +52,46 @@ make_tagged_fixture() {
|
||||
echo "$dir"
|
||||
}
|
||||
|
||||
# Helper: a fixture whose manifest carries one malformed entry: at the tag *and*
|
||||
# at HEAD, plus a post-tag change to the file that entry was meant to cover.
|
||||
# Committing the bad entry before the tag is what makes the assertion sharp — an
|
||||
# edited manifest is itself release-relevant, so the gate would fail for the
|
||||
# wrong reason and hide a parser that degrades silently.
|
||||
make_malformed_fixture() {
|
||||
local entry="$1" dir
|
||||
dir="$(mktemp -d)"
|
||||
(cd "$dir" && git init -q && git config user.email t@t.t && git config user.name t)
|
||||
write_release_paths "$dir"
|
||||
cat > "$dir/.pre-commit-hooks.yaml" <<EOF
|
||||
- id: fake-size-check
|
||||
entry: $entry
|
||||
language: script
|
||||
EOF
|
||||
(cd "$dir" && git add -A && git commit -q -m "initial" && git tag v1.0.0)
|
||||
echo "v2" > "$dir/scripts/skill-size-check.sh"
|
||||
(cd "$dir" && git add -A && git commit -q -m "change the file the malformed entry should cover")
|
||||
echo "$dir"
|
||||
}
|
||||
|
||||
# $3 is optional: pre-commit's PRE_COMMIT_TO_REF, the local sha being pushed.
|
||||
# Left off entirely, the variable stays unset and the script falls back to HEAD,
|
||||
# exactly as a plain `git push <remote> <current-branch>` behaves.
|
||||
# The fixture repo is the subject under test, so every PRE_COMMIT_* input must
|
||||
# come from this function and nowhere else. Any such variable already in the
|
||||
# environment belongs to the *caller's* repo: run under the pre-push hook this
|
||||
# suite guards, PRE_COMMIT_TO_REF holds a sha of the real repo, which does not
|
||||
# exist in the fixture, and the script resolves against the wrong rev. Clearing
|
||||
# them is what makes a standalone run and a pre-push run the same test — this
|
||||
# suite passed everywhere except under the hook it exists to protect.
|
||||
run_check() {
|
||||
local dir="$1" branch="$2"
|
||||
(cd "$dir" && PRE_COMMIT_REMOTE_BRANCH="$branch" bash "$SCRIPT" 2>&1)
|
||||
if [[ $# -ge 3 ]]; then
|
||||
(cd "$dir" && unset PRE_COMMIT_FROM_REF \
|
||||
&& PRE_COMMIT_REMOTE_BRANCH="$branch" PRE_COMMIT_TO_REF="$3" bash "$SCRIPT" 2>&1)
|
||||
else
|
||||
(cd "$dir" && unset PRE_COMMIT_FROM_REF PRE_COMMIT_TO_REF \
|
||||
&& PRE_COMMIT_REMOTE_BRANCH="$branch" bash "$SCRIPT" 2>&1)
|
||||
fi
|
||||
}
|
||||
|
||||
CLEANUP_DIRS=()
|
||||
@@ -211,6 +248,195 @@ else
|
||||
fail "invented a bogus assets/ path for a hook script with no bundled tree"
|
||||
fi
|
||||
|
||||
# --- 12. Deleting a hook's entire bundled assets/ tree is release-relevant ---
|
||||
# The worktree-only derivation guarded the assets/ path on the directory still
|
||||
# existing, so wiping the whole tree removed the path from the pathspec instead
|
||||
# of diffing it: the single most consumer-breaking change possible diffed clean.
|
||||
# The path list therefore has to be unioned with what $LAST_TAG exposed.
|
||||
echo ""
|
||||
echo "--- exits 1 when a hook's entire bundled assets/ tree was deleted since the tag ---"
|
||||
FIXTURE12="$(make_tagged_fixture)"; track "$FIXTURE12"
|
||||
rm -rf "${FIXTURE12:?}/$HOOK_DIR/assets"
|
||||
(cd "$FIXTURE12" && git add -A && git commit -q -m "delete the whole bundled assets tree")
|
||||
OUT12=$(run_check "$FIXTURE12" "refs/heads/main" || true)
|
||||
if echo "$OUT12" | grep -q "assets/vale/.vale.ini"; then
|
||||
pass "flags a wholesale deletion of a hook's bundled assets/ tree"
|
||||
else
|
||||
fail "a hook's entire bundled assets/ tree vanished since the tag without demanding a release"
|
||||
fi
|
||||
|
||||
# --- 13. A hook script deleted while its manifest entry survives is flagged ---
|
||||
# Characterisation test, not a bug fix: tokens[0] is added to the pathspec
|
||||
# unconditionally (no existence guard), so this case was already covered. It is
|
||||
# pinned here so the tagged-tree union can't accidentally introduce an existence
|
||||
# guard on tokens[0] and reopen the hole its assets/ sibling had.
|
||||
echo ""
|
||||
echo "--- exits 1 when a hook script was deleted but its manifest entry remains ---"
|
||||
FIXTURE13="$(make_tagged_fixture)"; track "$FIXTURE13"
|
||||
rm -f "$FIXTURE13/$HOOK_DIR/scripts/vale-wrap.sh"
|
||||
(cd "$FIXTURE13" && git add -A && git commit -q -m "delete a hook script, keep its manifest entry")
|
||||
OUT13=$(run_check "$FIXTURE13" "refs/heads/main" || true)
|
||||
if echo "$OUT13" | grep -q "vale-wrap.sh"; then
|
||||
pass "flags a hook script deleted out from under a surviving manifest entry"
|
||||
else
|
||||
fail "a manifest entry's script vanished since the tag without demanding a release"
|
||||
fi
|
||||
|
||||
# --- 14. Retiring a whole hook names what the tag exposed, not just the manifest ---
|
||||
# Removing the entry and everything it shipped changes $HOOKS_MANIFEST, so the
|
||||
# gate fires either way — but a derivation that only reads the current manifest
|
||||
# can no longer name the retired script or its assets, and the failure message
|
||||
# understates the breakage to consumers pinned at the old rev. The tagged
|
||||
# manifest is what makes those paths reportable.
|
||||
echo ""
|
||||
echo "--- names the retired hook's own paths when an entry and its files are removed together ---"
|
||||
FIXTURE14="$(make_tagged_fixture)"; track "$FIXTURE14"
|
||||
cat > "$FIXTURE14/.pre-commit-hooks.yaml" <<'EOF'
|
||||
- id: fake-size-check
|
||||
entry: scripts/skill-size-check.sh
|
||||
language: script
|
||||
EOF
|
||||
rm -rf "${FIXTURE14:?}/$HOOK_DIR"
|
||||
(cd "$FIXTURE14" && git add -A && git commit -q -m "retire the vale hook entirely")
|
||||
OUT14=$(run_check "$FIXTURE14" "refs/heads/main" || true)
|
||||
if echo "$OUT14" | grep -q "vale-wrap.sh" && echo "$OUT14" | grep -q "assets/vale/.vale.ini"; then
|
||||
pass "names the retired hook's script and bundled assets, not just the manifest edit"
|
||||
else
|
||||
fail "reported only the manifest change and hid which shipped paths the retirement removed"
|
||||
fi
|
||||
|
||||
# --- 15. A multi-token entry: is rejected loudly, not silently mis-parsed ---
|
||||
# ADR-0014 binds entries to a bare script path, but nothing enforced it, and the
|
||||
# sibling .pre-commit-config.yaml already ships `entry: bash <script>`. Under the
|
||||
# old parser tokens[0] became "bash": a pathspec matching nothing (which git diff
|
||||
# accepts in silence) and a bundle root of "." (skipped), so the hook's whole
|
||||
# surface dropped out of the gate and the post-tag change below diffed clean.
|
||||
echo ""
|
||||
echo "--- exits 1 naming the hook when an entry: carries more than one token ---"
|
||||
# The entry is quoted back verbatim, not just its first token: that is what makes
|
||||
# the diagnostic point at the argument the author has to remove, and what
|
||||
# distinguishes this from the unresolvable-path rejection test 16 covers.
|
||||
FIXTURE15="$(make_malformed_fixture "bash scripts/skill-size-check.sh")"; track "$FIXTURE15"
|
||||
OUT15=$(run_check "$FIXTURE15" "refs/heads/main" || true)
|
||||
if run_check "$FIXTURE15" "refs/heads/main" > /dev/null; then
|
||||
fail "silently exited 0 on a multi-token entry, dropping that hook's paths from the gate"
|
||||
elif echo "$OUT15" | grep -q "fake-size-check" \
|
||||
&& echo "$OUT15" | grep -q "bash scripts/skill-size-check.sh" \
|
||||
&& echo "$OUT15" | grep -q "ADR-0014"; then
|
||||
pass "rejects a multi-token entry, quoting it back and naming the hook and ADR-0014"
|
||||
else
|
||||
fail "rejected the multi-token entry without naming the hook, the entry, and ADR-0014"
|
||||
fi
|
||||
|
||||
# --- 16. An entry naming no file this repo ships is rejected loudly ---
|
||||
# The token-count guard alone still lets a single bare command name (`entry:
|
||||
# vale`, valid for language: system) through as a pathspec matching nothing.
|
||||
# Existence is checked against the union of the worktree and $LAST_TAG, so this
|
||||
# cannot misfire on the deletion cases tests 12-14 pin.
|
||||
echo ""
|
||||
echo "--- exits 1 naming the hook when an entry: names no file in the worktree or at the tag ---"
|
||||
FIXTURE16="$(make_malformed_fixture "vale")"; track "$FIXTURE16"
|
||||
OUT16=$(run_check "$FIXTURE16" "refs/heads/main" || true)
|
||||
if run_check "$FIXTURE16" "refs/heads/main" > /dev/null; then
|
||||
fail "silently exited 0 on an entry that names no shipped file"
|
||||
elif echo "$OUT16" | grep -q "fake-size-check" && echo "$OUT16" | grep -q "ADR-0014"; then
|
||||
pass "rejects an entry that resolves to no file, naming the hook and the ADR-0014 constraint"
|
||||
else
|
||||
fail "rejected the unresolvable entry without naming the hook and the ADR-0014 constraint"
|
||||
fi
|
||||
|
||||
# --- 17. The pushed ref, not HEAD, is what gets gated ---
|
||||
# pre-commit exports the local sha of each pushed ref as PRE_COMMIT_TO_REF.
|
||||
# `git push <remote> pushed-tip:main` from a checkout sitting on an older commit
|
||||
# is the false-negative direction: HEAD is still at the tag and diffs clean while
|
||||
# the branch actually landing on main carries an untagged, release-relevant
|
||||
# change. HEAD is reset back to the tag so the two genuinely differ.
|
||||
echo ""
|
||||
echo "--- exits 1 on a release-relevant change reachable only from PRE_COMMIT_TO_REF ---"
|
||||
FIXTURE17="$(make_tagged_fixture)"; track "$FIXTURE17"
|
||||
echo "v2" > "$FIXTURE17/scripts/skill-size-check.sh"
|
||||
(cd "$FIXTURE17" && git add -A && git commit -q -m "release-relevant change" \
|
||||
&& git branch pushed-tip && git reset -q --hard v1.0.0)
|
||||
OUT17=$(run_check "$FIXTURE17" "refs/heads/main" "pushed-tip" || true)
|
||||
if echo "$OUT17" | grep -q "skill-size-check.sh"; then
|
||||
pass "gates the pushed ref's tip, not HEAD, when HEAD is behind it"
|
||||
else
|
||||
fail "diffed HEAD instead of PRE_COMMIT_TO_REF and missed a release-relevant change"
|
||||
fi
|
||||
|
||||
# --- 18. Neither the diff tip nor the tag baseline may come from a newer HEAD ---
|
||||
# The false-positive direction: HEAD has moved past a v2.0.0 that the pushed ref
|
||||
# never saw. Reading either end of the diff off HEAD fails a push that is clean
|
||||
# since its own baseline — diffing v2.0.0..HEAD flags HEAD's untagged commit, and
|
||||
# resolving the tag from HEAD while diffing pushed-tip flags v2.0.0's change.
|
||||
echo ""
|
||||
echo "--- exits 0 when the pushed ref is clean since its own tag but HEAD has moved on ---"
|
||||
FIXTURE18="$(make_tagged_fixture)"; track "$FIXTURE18"
|
||||
(cd "$FIXTURE18" && git branch pushed-tip)
|
||||
echo "v2" > "$FIXTURE18/scripts/skill-size-check.sh"
|
||||
(cd "$FIXTURE18" && git add -A && git commit -q -m "released change" && git tag v2.0.0)
|
||||
echo "v3" > "$FIXTURE18/scripts/skill-size-check.sh"
|
||||
(cd "$FIXTURE18" && git add -A && git commit -q -m "unreleased change on HEAD's line")
|
||||
if run_check "$FIXTURE18" "refs/heads/main" "pushed-tip" > /dev/null; then
|
||||
pass "exits 0 for a pushed ref clean since the tag reachable from it, ignoring HEAD's line"
|
||||
else
|
||||
fail "gated HEAD's tag or tip and falsely demanded a release for a clean pushed ref"
|
||||
fi
|
||||
|
||||
# --- 19. A branch deletion is a no-op, not a confusing git failure ---
|
||||
# pre-commit sets PRE_COMMIT_TO_REF to an all-zeros sha when the push deletes a
|
||||
# branch. Nothing is being shipped, and the sha resolves to nothing, so without
|
||||
# an explicit guard the gate reports "could not diff" on an unrelated operation.
|
||||
echo ""
|
||||
echo "--- exits 0 when PRE_COMMIT_TO_REF is the all-zeros branch-deletion sha ---"
|
||||
FIXTURE19="$(make_tagged_fixture)"; track "$FIXTURE19"
|
||||
echo "v2" > "$FIXTURE19/scripts/skill-size-check.sh"
|
||||
(cd "$FIXTURE19" && git add -A && git commit -q -m "release-relevant change")
|
||||
if run_check "$FIXTURE19" "refs/heads/main" "0000000000000000000000000000000000000000" > /dev/null; then
|
||||
pass "treats an all-zeros PRE_COMMIT_TO_REF as a branch deletion and exits 0"
|
||||
else
|
||||
fail "turned a branch deletion into a failure instead of a no-op"
|
||||
fi
|
||||
|
||||
# --- 20. The repo's own .pre-commit-hooks.yaml satisfies the entry constraints ---
|
||||
# The parser guards above are only safe to ship if the manifest actually in tree
|
||||
# passes them. It is replayed into a fixture (with the paths its entries name
|
||||
# created) rather than run against the real repo, which has no release tag yet.
|
||||
echo ""
|
||||
echo "--- accepts the real .pre-commit-hooks.yaml this repo ships ---"
|
||||
FIXTURE20="$(mktemp -d)"; track "$FIXTURE20"
|
||||
(cd "$FIXTURE20" && git init -q && git config user.email t@t.t && git config user.name t)
|
||||
cp "$REPO_ROOT/.pre-commit-hooks.yaml" "$FIXTURE20/.pre-commit-hooks.yaml"
|
||||
while IFS= read -r real_entry; do
|
||||
mkdir -p "$FIXTURE20/$(dirname "$real_entry")"
|
||||
echo "v1" > "$FIXTURE20/$real_entry"
|
||||
done < <(sed -n 's/^[[:space:]]*entry:[[:space:]]*//p' "$REPO_ROOT/.pre-commit-hooks.yaml")
|
||||
(cd "$FIXTURE20" && git add -A && git commit -q -m "initial" && git tag v1.0.0)
|
||||
OUT20=$(run_check "$FIXTURE20" "refs/heads/main" || true)
|
||||
if [[ -z "$OUT20" ]]; then
|
||||
pass "parses every entry in the repo's real .pre-commit-hooks.yaml without complaint"
|
||||
else
|
||||
fail "the repo's own .pre-commit-hooks.yaml no longer satisfies the entry constraints: $OUT20"
|
||||
fi
|
||||
|
||||
# --- 21. A vX.Y.Z-suffixed checkpoint tag must not satisfy the release gate ---
|
||||
# git describe --match uses shell-glob semantics, not regex: the trailing `*` in
|
||||
# 'v[0-9]*.[0-9]*.[0-9]*' matches any suffix, so a pre-release/checkpoint tag like
|
||||
# v1.0.1-checkpoint also satisfies the glob and can be picked as LAST_TAG instead
|
||||
# of the true last release tag — hiding a real release-relevant change that landed
|
||||
# before the checkpoint tag from the diff.
|
||||
echo ""
|
||||
echo "--- ignores a vX.Y.Z-checkpoint tag and still flags the change since the real release tag ---"
|
||||
FIXTURE21="$(make_tagged_fixture)"; track "$FIXTURE21"
|
||||
echo "v2" > "$FIXTURE21/scripts/skill-size-check.sh"
|
||||
(cd "$FIXTURE21" && git add -A && git commit -q -m "real release-relevant change" && git tag v1.0.1-checkpoint)
|
||||
OUT21=$(run_check "$FIXTURE21" "refs/heads/main" || true)
|
||||
if echo "$OUT21" | grep -q "skill-size-check.sh"; then
|
||||
pass "still flags the release-relevant change since v1.0.0, ignoring the vX.Y.Z-checkpoint tag"
|
||||
else
|
||||
fail "a vX.Y.Z-checkpoint tag satisfied the glob and hid a real release-relevant change"
|
||||
fi
|
||||
|
||||
echo ""
|
||||
echo "Results: $PASS passed, $FAIL failed"
|
||||
[[ $FAIL -eq 0 ]]
|
||||
|
||||
@@ -9,41 +9,72 @@ FAIL=0
|
||||
pass() { echo " PASS: $1"; PASS=$((PASS + 1)); }
|
||||
fail() { echo " FAIL: $1"; FAIL=$((FAIL + 1)); }
|
||||
|
||||
# One trap over a registry, rather than rebuilding the trap line per fixture:
|
||||
# the guard is there because bash 3.2 treats "${arr[@]}" on an empty array as
|
||||
# unbound under `set -u`.
|
||||
FIXTURES=()
|
||||
cleanup() { [[ ${#FIXTURES[@]} -eq 0 ]] || rm -rf "${FIXTURES[@]}"; }
|
||||
trap cleanup EXIT
|
||||
|
||||
# Helper: make a fixture repo with skill-audit/agent-audit's Vale copies, in sync by default.
|
||||
# The wrapper is a stub — the script only diffs it — but the Vale assets and both
|
||||
# pre-commit manifests are the repo's real ones, because the .vale.ini checks ask
|
||||
# vale to apply those globs for real and cross-check them against the shipped
|
||||
# hooks' `files:` regexes. A synthetic style or manifest would prove nothing, and
|
||||
# copying the real ones keeps agent-audit's intentional KyberforgeCopilot
|
||||
# divergence in the fixture instead of a sanitized stand-in for it.
|
||||
make_fixture() {
|
||||
local dir
|
||||
dir="$(mktemp -d)"
|
||||
local skill_audit="$dir/plugins/kyberforge/skills/skill-audit"
|
||||
local agent_audit="$dir/plugins/kyberforge/skills/agent-audit"
|
||||
mkdir -p "$skill_audit/scripts" "$skill_audit/assets/vale/styles/Kyberforge"
|
||||
mkdir -p "$agent_audit/scripts" "$agent_audit/assets/vale/styles/Kyberforge"
|
||||
mkdir -p "$skill_audit/scripts" "$agent_audit/scripts"
|
||||
|
||||
echo '#!/usr/bin/env bash' > "$skill_audit/scripts/vale-wrap.sh"
|
||||
echo 'echo wrap' >> "$skill_audit/scripts/vale-wrap.sh"
|
||||
cp "$skill_audit/scripts/vale-wrap.sh" "$agent_audit/scripts/vale-wrap.sh"
|
||||
|
||||
echo 'extends: existence' > "$skill_audit/assets/vale/styles/Kyberforge/Rule.yml"
|
||||
cp "$skill_audit/assets/vale/styles/Kyberforge/Rule.yml" "$agent_audit/assets/vale/styles/Kyberforge/Rule.yml"
|
||||
cp -R "$REPO_ROOT/plugins/kyberforge/skills/skill-audit/assets" "$skill_audit/"
|
||||
cp -R "$REPO_ROOT/plugins/kyberforge/skills/agent-audit/assets" "$agent_audit/"
|
||||
cp "$REPO_ROOT/.pre-commit-hooks.yaml" "$REPO_ROOT/.pre-commit-config.yaml" "$dir/"
|
||||
|
||||
echo "$dir"
|
||||
}
|
||||
|
||||
# Helper: rewrite a glob section header in one copy's .vale.ini, leaving every
|
||||
# other line — StylesPath, BasedOnStyles — intact. This is the shape of the
|
||||
# typo the check exists to catch: the hook still matches the file via its
|
||||
# `files:` regex, vale lints nothing, and pre-commit reports `Passed`.
|
||||
break_glob() {
|
||||
local ini="$1" old="$2" new="$3"
|
||||
python3 - "$ini" "$old" "$new" <<'PYTHON'
|
||||
import sys
|
||||
path, old, new = sys.argv[1], sys.argv[2], sys.argv[3]
|
||||
with open(path, encoding='utf-8') as fh:
|
||||
content = fh.read()
|
||||
assert old in content, f"{old} not found in {path}"
|
||||
with open(path, 'w', encoding='utf-8') as fh:
|
||||
fh.write(content.replace(old, new))
|
||||
PYTHON
|
||||
}
|
||||
|
||||
# --- 1. Exits 0 when the two copies are in sync ---
|
||||
echo ""
|
||||
echo "--- exits 0 when skill-audit and agent-audit copies are in sync ---"
|
||||
FIXTURE="$(make_fixture)"
|
||||
trap 'rm -rf "$FIXTURE"' EXIT
|
||||
FIXTURES+=("$FIXTURE")
|
||||
if bash "$SCRIPT" "$FIXTURE" > /dev/null 2>&1; then
|
||||
pass "exits 0 when copies are in sync"
|
||||
else
|
||||
fail "exited non-zero against in-sync copies"
|
||||
bash "$SCRIPT" "$FIXTURE" 2>&1 | sed 's/^/ /' || true
|
||||
fi
|
||||
|
||||
# --- 2. Exits 1 when vale-wrap.sh differs between the two copies ---
|
||||
echo ""
|
||||
echo "--- exits 1 when vale-wrap.sh differs ---"
|
||||
FIXTURE2="$(make_fixture)"
|
||||
trap 'rm -rf "$FIXTURE" "$FIXTURE2"' EXIT
|
||||
FIXTURES+=("$FIXTURE2")
|
||||
echo 'echo different' >> "$FIXTURE2/plugins/kyberforge/skills/skill-audit/scripts/vale-wrap.sh"
|
||||
if bash "$SCRIPT" "$FIXTURE2" > /dev/null 2>&1; then
|
||||
fail "exited 0 when vale-wrap.sh copies differ — expected exit 1"
|
||||
@@ -55,8 +86,8 @@ fi
|
||||
echo ""
|
||||
echo "--- exits 1 when a Kyberforge style rule differs ---"
|
||||
FIXTURE3="$(make_fixture)"
|
||||
trap 'rm -rf "$FIXTURE" "$FIXTURE2" "$FIXTURE3"' EXIT
|
||||
echo 'level: error' >> "$FIXTURE3/plugins/kyberforge/skills/agent-audit/assets/vale/styles/Kyberforge/Rule.yml"
|
||||
FIXTURES+=("$FIXTURE3")
|
||||
echo ' - divergent token' >> "$FIXTURE3/plugins/kyberforge/skills/agent-audit/assets/vale/styles/Kyberforge/VagueWording.yml"
|
||||
if bash "$SCRIPT" "$FIXTURE3" > /dev/null 2>&1; then
|
||||
fail "exited 0 when a style rule differs — expected exit 1"
|
||||
else
|
||||
@@ -67,8 +98,14 @@ fi
|
||||
echo ""
|
||||
echo "--- exits 1 when a rule file is missing from one copy ---"
|
||||
FIXTURE4="$(make_fixture)"
|
||||
trap 'rm -rf "$FIXTURE" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4"' EXIT
|
||||
echo 'extends: existence' > "$FIXTURE4/plugins/kyberforge/skills/agent-audit/assets/vale/styles/Kyberforge/Extra.yml"
|
||||
FIXTURES+=("$FIXTURE4")
|
||||
cat > "$FIXTURE4/plugins/kyberforge/skills/agent-audit/assets/vale/styles/Kyberforge/Extra.yml" <<'EOF'
|
||||
extends: existence
|
||||
message: "Extra: '%s'"
|
||||
level: error
|
||||
tokens:
|
||||
- divergent token
|
||||
EOF
|
||||
if bash "$SCRIPT" "$FIXTURE4" > /dev/null 2>&1; then
|
||||
fail "exited 0 when a rule file exists in only one copy — expected exit 1"
|
||||
else
|
||||
@@ -79,13 +116,26 @@ fi
|
||||
echo ""
|
||||
echo "--- exits 0 when kyberforge skills are absent (no-op) ---"
|
||||
FIXTURE5="$(mktemp -d)"
|
||||
trap 'rm -rf "$FIXTURE" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5"' EXIT
|
||||
FIXTURES+=("$FIXTURE5")
|
||||
if bash "$SCRIPT" "$FIXTURE5" > /dev/null 2>&1; then
|
||||
pass "exits 0 as a no-op when skill-audit/agent-audit don't exist"
|
||||
else
|
||||
fail "exited non-zero when skill-audit/agent-audit are simply absent"
|
||||
fi
|
||||
|
||||
# --- 5b. Exits 1 when REPO_ROOT does not exist ---
|
||||
# A nonexistent path used to fall through to the "neither copy present" no-op
|
||||
# (test 5 above) and exit 0 — indistinguishable from a real, verified in-sync
|
||||
# result. That guard is for a repo legitimately missing kyberforge, not a
|
||||
# typo'd or stale path.
|
||||
echo ""
|
||||
echo "--- exits 1 when REPO_ROOT does not exist ---"
|
||||
if bash "$SCRIPT" "/nonexistent/path/$(date +%s)-$$" > /dev/null 2>&1; then
|
||||
fail "exited 0 for a nonexistent REPO_ROOT — expected exit 1"
|
||||
else
|
||||
pass "exits non-zero for a nonexistent REPO_ROOT"
|
||||
fi
|
||||
|
||||
# --- 6. Exits 1 when only one of the two copies is present ---
|
||||
# The no-op guard used `||`, so a single missing copy also exited 0 — a deleted
|
||||
# or renamed copy passed the sync check silently.
|
||||
@@ -93,7 +143,7 @@ echo ""
|
||||
echo "--- exits 1 when only one of the two copies is present ---"
|
||||
FIXTURE6="$(make_fixture)"
|
||||
FIXTURE7="$(make_fixture)"
|
||||
trap 'rm -rf "$FIXTURE" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5" "$FIXTURE6" "$FIXTURE7"' EXIT
|
||||
FIXTURES+=("$FIXTURE6" "$FIXTURE7")
|
||||
rm -rf "$FIXTURE6/plugins/kyberforge/skills/skill-audit"
|
||||
rm -rf "$FIXTURE7/plugins/kyberforge/skills/agent-audit"
|
||||
if bash "$SCRIPT" "$FIXTURE6" > /dev/null 2>&1; then
|
||||
@@ -107,6 +157,172 @@ else
|
||||
pass "exits non-zero when agent-audit's canonical copy is missing but skill-audit's is present"
|
||||
fi
|
||||
|
||||
# --- 7. Exits 1 when a .vale.ini is missing entirely ---
|
||||
# Without it vale falls back to an upward config search and lints the file with
|
||||
# whatever config it happens to find, which is not a failure anyone sees.
|
||||
echo ""
|
||||
echo "--- exits 1 when a .vale.ini is missing ---"
|
||||
FIXTURE8="$(make_fixture)"
|
||||
FIXTURES+=("$FIXTURE8")
|
||||
rm -f "$FIXTURE8/plugins/kyberforge/skills/skill-audit/assets/vale/.vale.ini"
|
||||
if bash "$SCRIPT" "$FIXTURE8" > /dev/null 2>&1; then
|
||||
fail "exited 0 when skill-audit's .vale.ini is missing — expected exit 1"
|
||||
else
|
||||
pass "exits non-zero when a .vale.ini is missing"
|
||||
fi
|
||||
|
||||
# --- 8. Exits 1 when the shared StylesPath line is dropped from either copy ---
|
||||
# StylesPath resolves relative to the .vale.ini, which is the only reason the
|
||||
# bundled styles are found from a consuming repo's clone prefix.
|
||||
echo ""
|
||||
echo "--- exits 1 when StylesPath is missing from either .vale.ini ---"
|
||||
FIXTURE9="$(make_fixture)"
|
||||
FIXTURE10="$(make_fixture)"
|
||||
FIXTURES+=("$FIXTURE9" "$FIXTURE10")
|
||||
break_glob "$FIXTURE9/plugins/kyberforge/skills/skill-audit/assets/vale/.vale.ini" \
|
||||
'StylesPath = styles' 'StylesPath = elsewhere'
|
||||
break_glob "$FIXTURE10/plugins/kyberforge/skills/agent-audit/assets/vale/.vale.ini" \
|
||||
'StylesPath = styles' 'StylesPath = elsewhere'
|
||||
if bash "$SCRIPT" "$FIXTURE9" > /dev/null 2>&1; then
|
||||
fail "exited 0 when skill-audit's .vale.ini lost StylesPath — expected exit 1"
|
||||
else
|
||||
pass "exits non-zero when skill-audit's .vale.ini lost StylesPath"
|
||||
fi
|
||||
if bash "$SCRIPT" "$FIXTURE10" > /dev/null 2>&1; then
|
||||
fail "exited 0 when agent-audit's .vale.ini lost StylesPath — expected exit 1"
|
||||
else
|
||||
pass "exits non-zero when agent-audit's .vale.ini lost StylesPath"
|
||||
fi
|
||||
|
||||
# --- 9. Exits 1 when no section's BasedOnStyles names Kyberforge ---
|
||||
# Every rule the prefilter gates on lives in that style, so a section that keeps
|
||||
# its glob but loses the style lints the file and reports nothing.
|
||||
echo ""
|
||||
echo "--- exits 1 when BasedOnStyles no longer names Kyberforge ---"
|
||||
FIXTURE11="$(make_fixture)"
|
||||
FIXTURES+=("$FIXTURE11")
|
||||
break_glob "$FIXTURE11/plugins/kyberforge/skills/agent-audit/assets/vale/.vale.ini" \
|
||||
'BasedOnStyles = Kyberforge' 'BasedOnStyles = KyberforgeCopilot'
|
||||
if bash "$SCRIPT" "$FIXTURE11" > /dev/null 2>&1; then
|
||||
fail "exited 0 when agent-audit's .vale.ini stopped naming Kyberforge — expected exit 1"
|
||||
else
|
||||
pass "exits non-zero when a .vale.ini no longer names the Kyberforge style"
|
||||
fi
|
||||
|
||||
# --- 10. Exits 1 when a glob section stops matching the shape its hook lints ---
|
||||
# One case per glob section, because each covers a file shape the others don't:
|
||||
# agent-audit's [**/*.agent.md] is the only section covering a Copilot agent file
|
||||
# outside an agents/ directory, so breaking it alone is invisible to the others.
|
||||
echo ""
|
||||
echo "--- exits 1 when a .vale.ini glob no longer matches its hook's file shape ---"
|
||||
FIXTURE12="$(make_fixture)"
|
||||
FIXTURE13="$(make_fixture)"
|
||||
FIXTURE14="$(make_fixture)"
|
||||
FIXTURES+=("$FIXTURE12" "$FIXTURE13" "$FIXTURE14")
|
||||
break_glob "$FIXTURE12/plugins/kyberforge/skills/skill-audit/assets/vale/.vale.ini" \
|
||||
'[**/SKILL.md]' '[**/NOMATCH.md]'
|
||||
break_glob "$FIXTURE13/plugins/kyberforge/skills/agent-audit/assets/vale/.vale.ini" \
|
||||
'[**/agents/*.md]' '[**/NOMATCH-agents/*.md]'
|
||||
break_glob "$FIXTURE14/plugins/kyberforge/skills/agent-audit/assets/vale/.vale.ini" \
|
||||
'[**/*.agent.md]' '[**/*.NOMATCH.md]'
|
||||
if bash "$SCRIPT" "$FIXTURE12" > /dev/null 2>&1; then
|
||||
fail "exited 0 when skill-audit's SKILL.md glob matched nothing — expected exit 1"
|
||||
else
|
||||
pass "exits non-zero when skill-audit's SKILL.md glob matches nothing"
|
||||
fi
|
||||
if bash "$SCRIPT" "$FIXTURE13" > /dev/null 2>&1; then
|
||||
fail "exited 0 when agent-audit's agents/*.md glob matched nothing — expected exit 1"
|
||||
else
|
||||
pass "exits non-zero when agent-audit's agents/*.md glob matches nothing"
|
||||
fi
|
||||
if bash "$SCRIPT" "$FIXTURE14" > /dev/null 2>&1; then
|
||||
fail "exited 0 when agent-audit's *.agent.md glob matched nothing — expected exit 1"
|
||||
else
|
||||
pass "exits non-zero when agent-audit's *.agent.md glob matches nothing"
|
||||
fi
|
||||
|
||||
# --- 11. Exits 1 when a probe path falls out of every hook's `files:` regex ---
|
||||
# The probe paths are hardcoded, so they can silently stop representing anything
|
||||
# the hooks lint. Rescoping the shipped agent hook away from the `.agent.md`
|
||||
# shape has to fail here rather than leave a probe testing a shape no hook
|
||||
# matches any more.
|
||||
echo ""
|
||||
echo "--- exits 1 when a probe path matches no hook's files: regex ---"
|
||||
FIXTURE16="$(make_fixture)"
|
||||
FIXTURES+=("$FIXTURE16")
|
||||
break_glob "$FIXTURE16/.pre-commit-hooks.yaml" \
|
||||
"files: '(^|/)agents/[^/]+\\.md\$|\\.agent\\.md\$'" "files: '(^|/)agents/[^/]+\\.md\$'"
|
||||
if bash "$SCRIPT" "$FIXTURE16" > /dev/null 2>&1; then
|
||||
fail "exited 0 when the agent hook was rescoped away from .agent.md — expected exit 1"
|
||||
else
|
||||
pass "exits non-zero when a probe path is in no hook's scope any more"
|
||||
fi
|
||||
|
||||
# --- 12. The text-level assertions hold on a machine without vale ---
|
||||
# They are the fallback when the glob probe cannot run. With vale on PATH the
|
||||
# probe fails on these same mutations, so it would mask them: only masking vale
|
||||
# proves a clean run here means the text assertions themselves ran.
|
||||
echo ""
|
||||
echo "--- the StylesPath / BasedOnStyles assertions still gate with vale masked off PATH ---"
|
||||
VALE_DIR="$(dirname "$(command -v vale 2>/dev/null || echo /nonexistent/vale)")"
|
||||
PATH_NO_VALE="$(printf '%s' "$PATH" | tr ':' '\n' | grep -vxF "$VALE_DIR" | paste -sd: -)"
|
||||
if (PATH="$PATH_NO_VALE"; command -v vale >/dev/null 2>&1); then
|
||||
fail "could not mask vale off PATH — the vale-absent fallback was not exercised"
|
||||
else
|
||||
FIXTURE17="$(make_fixture)"
|
||||
FIXTURE18="$(make_fixture)"
|
||||
FIXTURE19="$(make_fixture)"
|
||||
FIXTURES+=("$FIXTURE17" "$FIXTURE18" "$FIXTURE19")
|
||||
break_glob "$FIXTURE18/plugins/kyberforge/skills/skill-audit/assets/vale/.vale.ini" \
|
||||
'StylesPath = styles' 'StylesPath = elsewhere'
|
||||
break_glob "$FIXTURE19/plugins/kyberforge/skills/agent-audit/assets/vale/.vale.ini" \
|
||||
'BasedOnStyles = Kyberforge' 'BasedOnStyles = KyberforgeCopilot'
|
||||
if PATH="$PATH_NO_VALE" bash "$SCRIPT" "$FIXTURE17" > /dev/null 2>&1; then
|
||||
pass "exits 0 on in-sync copies with vale unavailable"
|
||||
else
|
||||
fail "exited non-zero on in-sync copies with vale unavailable — the missing binary must warn, not fail"
|
||||
fi
|
||||
if PATH="$PATH_NO_VALE" bash "$SCRIPT" "$FIXTURE18" > /dev/null 2>&1; then
|
||||
fail "exited 0 on a dropped StylesPath with vale unavailable — expected exit 1"
|
||||
else
|
||||
pass "exits non-zero on a dropped StylesPath with vale unavailable"
|
||||
fi
|
||||
if PATH="$PATH_NO_VALE" bash "$SCRIPT" "$FIXTURE19" > /dev/null 2>&1; then
|
||||
fail "exited 0 on a BasedOnStyles that dropped Kyberforge with vale unavailable — expected exit 1"
|
||||
else
|
||||
pass "exits non-zero on a BasedOnStyles that dropped Kyberforge with vale unavailable"
|
||||
fi
|
||||
# A clean run without vale must say so — silence would read as verified.
|
||||
if PATH="$PATH_NO_VALE" bash "$SCRIPT" "$FIXTURE17" 2>&1 | grep -q "vale is not installed"; then
|
||||
pass "warns that glob coverage was not verified when vale is unavailable"
|
||||
else
|
||||
fail "exited clean without vale and said nothing — an unverified run looks identical to a verified one"
|
||||
fi
|
||||
fi
|
||||
|
||||
# --- 13. The intentional agent-audit-only divergence is NOT flagged ---
|
||||
# The two .vale.ini files are deliberately different: agent-audit ships an extra
|
||||
# [**/*.agent.md] section and the KyberforgeCopilot style. A check that diffed
|
||||
# them would fail the repo as it stands, so assert the divergence is really in
|
||||
# the fixture before asserting the check tolerates it — otherwise this case would
|
||||
# still pass if the fixture had quietly stopped carrying it.
|
||||
echo ""
|
||||
echo "--- exits 0 despite agent-audit's KyberforgeCopilot divergence ---"
|
||||
FIXTURE15="$(make_fixture)"
|
||||
FIXTURES+=("$FIXTURE15")
|
||||
AGENT_INI15="$FIXTURE15/plugins/kyberforge/skills/agent-audit/assets/vale/.vale.ini"
|
||||
SKILL_INI15="$FIXTURE15/plugins/kyberforge/skills/skill-audit/assets/vale/.vale.ini"
|
||||
if ! grep -q "KyberforgeCopilot" "$AGENT_INI15" \
|
||||
|| grep -q "KyberforgeCopilot" "$SKILL_INI15" \
|
||||
|| [[ ! -d "$FIXTURE15/plugins/kyberforge/skills/agent-audit/assets/vale/styles/KyberforgeCopilot" ]]; then
|
||||
fail "the fixture no longer carries the agent-audit-only KyberforgeCopilot divergence, so tolerating it proves nothing"
|
||||
elif bash "$SCRIPT" "$FIXTURE15" > /dev/null 2>&1; then
|
||||
pass "exits 0 with agent-audit's extra KyberforgeCopilot section and style present"
|
||||
else
|
||||
fail "flagged the intentional agent-audit-only KyberforgeCopilot divergence — expected exit 0"
|
||||
bash "$SCRIPT" "$FIXTURE15" 2>&1 | sed 's/^/ /' || true
|
||||
fi
|
||||
|
||||
echo ""
|
||||
echo "Results: $PASS passed, $FAIL failed"
|
||||
[[ $FAIL -eq 0 ]]
|
||||
|
||||
@@ -1,10 +1,14 @@
|
||||
#!/usr/bin/env bash
|
||||
# Regression test for scripts/skill-size-check.sh: enforces agentskills.io's
|
||||
# 500-line/5,000-word(proxy-for-token) SKILL.md size ceiling.
|
||||
# 500-line/5,000-token SKILL.md size ceiling. The token half is enforced via a
|
||||
# word-count proxy (MAX_WORDS, currently 2770) — 5,000 is the token ceiling,
|
||||
# 2,770 is the word budget the script derives from it at the corpus's densest
|
||||
# measured prose.
|
||||
set -euo pipefail
|
||||
|
||||
REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
|
||||
SCRIPT="$REPO_ROOT/scripts/skill-size-check.sh"
|
||||
VALIDATE="$REPO_ROOT/plugins/kyberforge/skills/skill-audit/scripts/validate.sh"
|
||||
PASS=0
|
||||
FAIL=0
|
||||
|
||||
@@ -55,9 +59,9 @@ echo ""
|
||||
echo "--- fails a file over the word-count limit ---"
|
||||
MANY_WORDS="$(make_fixture many-words 10 600)"
|
||||
if "$SCRIPT" "$MANY_WORDS" 2>/dev/null; then
|
||||
fail "file over the 5,000-word ceiling should have exited non-zero"
|
||||
fail "file over the word ceiling should have exited non-zero"
|
||||
else
|
||||
pass "file over the 5,000-word ceiling exits non-zero"
|
||||
pass "file over the word ceiling exits non-zero"
|
||||
fi
|
||||
|
||||
# Boundary-pair tests below read the script's current MAX_WORDS rather than
|
||||
@@ -65,6 +69,29 @@ fi
|
||||
MAX_WORDS="$(grep -oE '^MAX_WORDS=[0-9]+' "$SCRIPT" | cut -d= -f2)"
|
||||
MAX_LINES="$(grep -oE '^MAX_LINES=[0-9]+' "$SCRIPT" | cut -d= -f2)"
|
||||
|
||||
# The audit (skill-audit/scripts/validate.sh) duplicates both ceilings, because
|
||||
# a cache-installed plugin's scripts cannot read files outside the plugin
|
||||
# directory. Nothing but this assertion stops the copies drifting, and drift
|
||||
# means a SKILL.md passes its own audit and is then rejected by the commit hook.
|
||||
echo ""
|
||||
echo "--- the hook and skill-audit's validate.sh agree on both ceilings ---"
|
||||
if [[ ! -f "$VALIDATE" ]]; then
|
||||
fail "skill-audit validate.sh not found at $VALIDATE"
|
||||
else
|
||||
V_MAX_WORDS="$(grep -oE '^MAX_WORDS = [0-9]+' "$VALIDATE" | grep -oE '[0-9]+')"
|
||||
V_MAX_LINES="$(grep -oE '^MAX_LINES = [0-9]+' "$VALIDATE" | grep -oE '[0-9]+')"
|
||||
if [[ "$V_MAX_WORDS" == "$MAX_WORDS" ]]; then
|
||||
pass "both enforce MAX_WORDS=$MAX_WORDS"
|
||||
else
|
||||
fail "MAX_WORDS drift: hook says $MAX_WORDS, validate.sh says ${V_MAX_WORDS:-<unset>}"
|
||||
fi
|
||||
if [[ "$V_MAX_LINES" == "$MAX_LINES" ]]; then
|
||||
pass "both enforce MAX_LINES=$MAX_LINES"
|
||||
else
|
||||
fail "MAX_LINES drift: hook says $MAX_LINES, validate.sh says ${V_MAX_LINES:-<unset>}"
|
||||
fi
|
||||
fi
|
||||
|
||||
# make_line_fixture builds a file with an exact total line count (frontmatter
|
||||
# included), independent of word count, for the line-boundary tests.
|
||||
make_line_fixture() {
|
||||
|
||||
@@ -47,8 +47,11 @@ git -C "$HOOK_REPO" add -A
|
||||
git -C "$HOOK_REPO" -c user.email=test@example.invalid -c user.name=test commit -qm "hook repo"
|
||||
HOOK_REV="$(git -C "$HOOK_REPO" rev-parse HEAD)"
|
||||
|
||||
# The two Vale hooks scope by filename, so the consumer needs one file of each
|
||||
# shape: a hook with nothing to match reports `Skipped` and proves nothing.
|
||||
# Every hook scopes by filename, so the consumer needs one file of each shape:
|
||||
# a hook with nothing to match reports `Skipped` and proves nothing. All three
|
||||
# hooks .pre-commit-hooks.yaml ships are registered — an unregistered one would
|
||||
# let a regression (a lost `100755` bit, a bad entry path) reach every external
|
||||
# consumer while this repo's own `repo: local` runs stayed green.
|
||||
mkdir -p "$CONSUMER/skills/demo" "$CONSUMER/agents"
|
||||
git -C "$CONSUMER" init -q
|
||||
cat > "$CONSUMER/.pre-commit-config.yaml" <<EOF
|
||||
@@ -58,15 +61,21 @@ repos:
|
||||
hooks:
|
||||
- id: kyberforge-vale-audit-skill
|
||||
- id: kyberforge-vale-audit-agent
|
||||
- id: kyberforge-skill-size-check
|
||||
EOF
|
||||
|
||||
# The two fixtures carry DIFFERENT flagged tokens so an alert can never be
|
||||
# credited to the hook that did not raise it. Both bodies land mid-sentence in a
|
||||
# folded block scalar that still spans two physical lines, which is the
|
||||
# flattening the wrapper exists to do.
|
||||
write_fixtures() {
|
||||
local body="$1"
|
||||
local skill_body="$1"
|
||||
local agent_body="${2:-$1}"
|
||||
cat > "$CONSUMER/skills/demo/SKILL.md" <<EOF
|
||||
---
|
||||
name: demo
|
||||
description: >
|
||||
Use when the caller wants a demonstration skill $body across two
|
||||
Use when the caller wants a demonstration skill $skill_body across two
|
||||
physical lines of one folded block scalar.
|
||||
---
|
||||
|
||||
@@ -76,7 +85,7 @@ EOF
|
||||
---
|
||||
name: demo
|
||||
description: >
|
||||
Use when the caller wants a demonstration agent $body across two
|
||||
Use when the caller wants a demonstration agent $agent_body across two
|
||||
physical lines of one folded block scalar.
|
||||
---
|
||||
|
||||
@@ -85,43 +94,102 @@ EOF
|
||||
git -C "$CONSUMER" add -A
|
||||
}
|
||||
|
||||
run_hooks() {
|
||||
(cd "$CONSUMER" && pre-commit run --all-files 2>&1) || true
|
||||
# Vale prints each linted path as its own header line with that file's alerts
|
||||
# indented beneath it, so an alert belongs to the nearest preceding path line.
|
||||
# Reads a hook log on stdin and prints only the alert lines filed under `$1`.
|
||||
# The `sed` strips vale's ANSI colouring, which it emits into pre-commit's pipe
|
||||
# too, so the header lines compare as plain paths.
|
||||
alerts_for() {
|
||||
sed $'s/\033\\[[0-9;]*m//g' | awk -v want="$1" '
|
||||
/^[^[:space:]].*\.md$/ { cur = $0; next }
|
||||
/^[[:space:]]*[0-9]+:[0-9]+[[:space:]]/ { if (cur == want) print }
|
||||
'
|
||||
}
|
||||
|
||||
# --- 1. Both hooks resolve their config and actually gate on a bad file ---
|
||||
# --- 1. Each Vale hook resolves its config and gates its own file shape ---
|
||||
# Asserted per hook, against that hook's own fixture path and its own token. An
|
||||
# aggregate alert count over both hooks' combined output does not prove this:
|
||||
# one fixture description carries every flagged token, so ONE working hook
|
||||
# already clears a `>= 2` threshold. And a hook whose .vale.ini globs match
|
||||
# nothing reaches neither of the guards below — it still MATCHES the file via
|
||||
# its `files:` regex, so pre-commit does not report `Skipped`; vale simply lints
|
||||
# nothing, prints `0 errors ... in 1 file` and exits 0, and the hook shows
|
||||
# `Passed`. Attribution is the only thing that catches it.
|
||||
echo ""
|
||||
echo "--- both Vale hooks run and fail a bad file in an external consumer repo ---"
|
||||
write_fixtures "that helps with and utilize things"
|
||||
OUT_BAD="$(run_hooks)"
|
||||
if echo "$OUT_BAD" | grep -q "does not exist"; then
|
||||
fail "hooks hard-errored on a path resolved against the consumer repo (E100) — the bug this test guards against"
|
||||
echo "$OUT_BAD" | sed 's/^/ /'
|
||||
elif echo "$OUT_BAD" | grep -q "Skipped"; then
|
||||
fail "a hook matched no files, so it proved nothing"
|
||||
echo "$OUT_BAD" | sed 's/^/ /'
|
||||
elif [[ "$(echo "$OUT_BAD" | grep -c "VagueWording")" -ge 2 ]]; then
|
||||
pass "both hooks flatten and flag the folded description in a consumer repo"
|
||||
else
|
||||
fail "hooks did not flag both fixtures"
|
||||
echo "$OUT_BAD" | sed 's/^/ /'
|
||||
fi
|
||||
echo "--- each Vale hook flags its own fixture in an external consumer repo ---"
|
||||
write_fixtures "that helps with things" "that will utilize things"
|
||||
while IFS='|' read -r HOOK_ID FIXTURE TOKEN; do
|
||||
[[ -n "$HOOK_ID" ]] || continue
|
||||
LOG="$WORK/$HOOK_ID.log"
|
||||
set +e
|
||||
(cd "$CONSUMER" && pre-commit run "$HOOK_ID" --all-files > "$LOG" 2>&1)
|
||||
RC_HOOK=$?
|
||||
set -e
|
||||
if grep -q "does not exist" "$LOG"; then
|
||||
fail "$HOOK_ID hard-errored on a path resolved against the consumer repo (E100) — the bug this test guards against"
|
||||
sed 's/^/ /' "$LOG"
|
||||
elif grep -q "Skipped" "$LOG"; then
|
||||
fail "$HOOK_ID matched no files, so it proved nothing"
|
||||
sed 's/^/ /' "$LOG"
|
||||
elif [[ $RC_HOOK -eq 0 ]]; then
|
||||
fail "$HOOK_ID passed $FIXTURE despite its flagged '$TOKEN' — a .vale.ini glob matching nothing lints zero files and exits 0"
|
||||
sed 's/^/ /' "$LOG"
|
||||
elif alerts_for "$FIXTURE" < "$LOG" | grep -qF "'$TOKEN'"; then
|
||||
pass "$HOOK_ID flattens $FIXTURE and flags its '$TOKEN' in a consumer repo"
|
||||
else
|
||||
fail "$HOOK_ID failed, but no alert quoting '$TOKEN' was filed under $FIXTURE"
|
||||
sed 's/^/ /' "$LOG"
|
||||
fi
|
||||
done <<'EOF'
|
||||
kyberforge-vale-audit-skill|skills/demo/SKILL.md|helps with
|
||||
kyberforge-vale-audit-agent|agents/demo.md|utilize
|
||||
EOF
|
||||
|
||||
# --- 2. Clean files pass — the hooks gate, they don't just always fail ---
|
||||
echo ""
|
||||
echo "--- both Vale hooks pass clean files in an external consumer repo ---"
|
||||
echo "--- all three hooks pass clean files in an external consumer repo ---"
|
||||
write_fixtures "of the packaged hook contract"
|
||||
set +e
|
||||
(cd "$CONSUMER" && pre-commit run --all-files > "$WORK/clean.log" 2>&1)
|
||||
RC_CLEAN=$?
|
||||
set -e
|
||||
if [[ $RC_CLEAN -eq 0 ]]; then
|
||||
pass "both hooks exit 0 on clean files"
|
||||
if grep -q "Skipped" "$WORK/clean.log"; then
|
||||
fail "a hook matched no files on the clean run, so it proved nothing"
|
||||
sed 's/^/ /' "$WORK/clean.log"
|
||||
elif [[ $RC_CLEAN -eq 0 ]]; then
|
||||
pass "all three hooks exit 0 on clean files"
|
||||
else
|
||||
fail "hooks failed on clean files (rc=$RC_CLEAN)"
|
||||
sed 's/^/ /' "$WORK/clean.log"
|
||||
fi
|
||||
|
||||
# --- 3. The size hook gates too. It ran clean above, which is what proves it
|
||||
# is executable and its entry path resolves; this half proves it still fails a
|
||||
# file that breaks the ceiling rather than passing everything. ---
|
||||
echo ""
|
||||
echo "--- kyberforge-skill-size-check fails an oversized SKILL.md in an external consumer repo ---"
|
||||
mkdir -p "$CONSUMER/skills/oversized"
|
||||
{
|
||||
echo "---"
|
||||
echo "name: oversized"
|
||||
echo "description: Use when the caller wants an oversized fixture."
|
||||
echo "---"
|
||||
for ((i = 1; i <= 600; i++)); do
|
||||
echo "word"
|
||||
done
|
||||
} > "$CONSUMER/skills/oversized/SKILL.md"
|
||||
git -C "$CONSUMER" add -A
|
||||
set +e
|
||||
(cd "$CONSUMER" && pre-commit run kyberforge-skill-size-check --all-files > "$WORK/size.log" 2>&1)
|
||||
RC_SIZE=$?
|
||||
set -e
|
||||
if [[ $RC_SIZE -ne 0 ]] && grep -q "500-line ceiling" "$WORK/size.log"; then
|
||||
pass "kyberforge-skill-size-check exits non-zero and names the ceiling it broke"
|
||||
else
|
||||
fail "kyberforge-skill-size-check did not gate an oversized SKILL.md (rc=$RC_SIZE)"
|
||||
sed 's/^/ /' "$WORK/size.log"
|
||||
fi
|
||||
|
||||
echo ""
|
||||
echo "Results: $PASS passed, $FAIL failed"
|
||||
[[ $FAIL -eq 0 ]]
|
||||
|
||||
@@ -321,7 +321,12 @@ else
|
||||
fail "an absolute path was silently skipped — the bug this test guards against"
|
||||
fi
|
||||
|
||||
# --- 11. A literal (|) block scalar passes through unflattened (no regression) ---
|
||||
# --- 11. A literal (|) block scalar passes through unflattened. Unlike every
|
||||
# other multi-line form, `|` is not broken in Vale: its parsed value keeps the
|
||||
# same line breaks the source has, so the description scope still matches. The
|
||||
# second assertion pins that down — without it, a wrapper that broke `|` and a
|
||||
# Vale that never matched `|` would agree on zero alerts and the comparison
|
||||
# would pass vacuously.
|
||||
echo ""
|
||||
echo "--- leaves a literal (|) block scalar untouched (narrowed >-only scope) ---"
|
||||
FIXTURE11="$(make_raw_fixture <<'EOF'
|
||||
@@ -339,7 +344,9 @@ trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5" "$FIXTU
|
||||
REL11="plugins/testplugin/skills/zzzskill/SKILL.md"
|
||||
WRAPPED_OUT=$(run_wrap "$FIXTURE11" --config "$VALE_CONFIG" "$REL11")
|
||||
BARE_OUT=$(cd "$FIXTURE11" && vale --config "$VALE_CONFIG" "$REL11" 2>&1 || true)
|
||||
if [[ "$WRAPPED_OUT" == "$BARE_OUT" ]]; then
|
||||
if ! echo "$BARE_OUT" | grep -q "VagueWording"; then
|
||||
fail "bare vale reports nothing for a literal (|) block scalar — the 'literal blocks are not broken' premise is wrong"
|
||||
elif [[ "$WRAPPED_OUT" == "$BARE_OUT" ]]; then
|
||||
pass "literal (|) block scalar output matches bare vale exactly — untouched by flattening"
|
||||
else
|
||||
fail "wrapper altered output for a literal (|) block scalar description — should be left untouched"
|
||||
@@ -425,6 +432,429 @@ else
|
||||
fail "a path with a space was dropped from the directory walk"
|
||||
fi
|
||||
|
||||
# --- 16. No unguarded `"${arr[@]}"` expansion survives in any script that runs
|
||||
# on macOS. bash before 4.4 — including the 3.2 that macOS still ships as
|
||||
# /bin/bash — treats that form on an *empty* array as an unbound variable under
|
||||
# `set -u` and aborts. The portable form is `${arr[@]+"${arr[@]}"}`. This is a
|
||||
# static check because no bash 5 host can reproduce the abort at runtime: the
|
||||
# construct is only fatal on the older shell, so absence of the construct is the
|
||||
# property to assert. `${#arr[@]}` is deliberately not flagged — the count form
|
||||
# is safe on 3.2. Neither is an array seeded with at least one element where it
|
||||
# is declared and never reset to empty: it cannot be empty at any expansion
|
||||
# site, so the construct is not a hazard there and demanding the guarded form
|
||||
# would be a wrong test. The file list covers every script this repo ships or
|
||||
# runs that a macOS user reaches: the wrapper itself, the two pre-commit hook
|
||||
# scripts, and the test runner AGENTS.md tells contributors to run by hand.
|
||||
# `mapfile` is checked alongside, because it is bash 4.0+ and the expansion scan
|
||||
# cannot see it — run-tests.sh carried one until it was replaced with a
|
||||
# `while read` loop, and nothing would have caught its return. `declare -A`
|
||||
# (bash 4.0+ associative arrays) is checked for the same reason — the
|
||||
# expansion scan cannot see it, and check-vale-style-sync.sh carried a pair of
|
||||
# them until they were replaced with index-scanned plain arrays.
|
||||
echo ""
|
||||
echo "--- no unguarded array expansion remains in the macOS-facing scripts ---"
|
||||
unguarded_expansions() {
|
||||
local file="$1" hit name
|
||||
while IFS= read -r hit; do
|
||||
name="$(printf '%s\n' "$hit" \
|
||||
| grep -oE '\$\{[A-Za-z_][A-Za-z0-9_]*\[@\]\}' | head -1 \
|
||||
| sed -E 's/^\$\{//; s/\[@\]\}$//')"
|
||||
if grep -qE "^[[:space:]]*((local|declare|readonly)[[:space:]]+)?(-a[[:space:]]+)?$name=\([^)]" "$file" \
|
||||
&& ! grep -qE "^[[:space:]]*$name=\(\)" "$file"; then
|
||||
continue
|
||||
fi
|
||||
printf '%s:%s\n' "${file##*/}" "$hit"
|
||||
done < <(
|
||||
# Blank out whole-line comments (keeping line numbers), delete every
|
||||
# correctly guarded expansion, then anything still matching is a candidate.
|
||||
awk '{ if ($0 ~ /^[[:space:]]*#/) print ""; else print }' "$file" \
|
||||
| sed -E 's/\$\{([A-Za-z_][A-Za-z0-9_]*)\[@\]\+"\$\{\1\[@\]\}"\}//g' \
|
||||
| grep -nE '\$\{[A-Za-z_][A-Za-z0-9_]*\[@\]\}' || true
|
||||
)
|
||||
}
|
||||
HAZARDS16=""
|
||||
for BASH32_SCRIPT in \
|
||||
"$SCRIPT" \
|
||||
"$REPO_ROOT/scripts/skill-size-check.sh" \
|
||||
"$REPO_ROOT/scripts/check-release-needed.sh" \
|
||||
"$REPO_ROOT/scripts/check-vale-style-sync.sh" \
|
||||
"$REPO_ROOT/tests/run-tests.sh"; do
|
||||
FOUND16="$(unguarded_expansions "$BASH32_SCRIPT")"
|
||||
if [[ -n "$FOUND16" ]]; then
|
||||
HAZARDS16+="$FOUND16 "
|
||||
fi
|
||||
# `mapfile`/`readarray` are bash 4.0+ builtins with no 3.2 fallback. Whole-line
|
||||
# comments are blanked first so prose naming the builtin is not a hit.
|
||||
FOUND16B="$(awk '{ if ($0 ~ /^[[:space:]]*#/) print ""; else print }' "$BASH32_SCRIPT" \
|
||||
| grep -nE '(^|[^[:alnum:]_])(mapfile|readarray)[[:space:]]' || true)"
|
||||
if [[ -n "$FOUND16B" ]]; then
|
||||
HAZARDS16+="${BASH32_SCRIPT##*/}:$FOUND16B "
|
||||
fi
|
||||
# `declare -A` (associative arrays) is bash 4.0+ with no 3.2 fallback. The
|
||||
# flag cluster can carry other letters in any order (-Ag, -rA, ...); what
|
||||
# matters is a literal uppercase A appearing in it, so match on that rather
|
||||
# than the exact string "-A".
|
||||
FOUND16C="$(awk '{ if ($0 ~ /^[[:space:]]*#/) print ""; else print }' "$BASH32_SCRIPT" \
|
||||
| grep -nE '(^|[^[:alnum:]_])declare[[:space:]]+-[a-zA-Z]*A[a-zA-Z]*([[:space:]]|$)' || true)"
|
||||
if [[ -n "$FOUND16C" ]]; then
|
||||
HAZARDS16+="${BASH32_SCRIPT##*/}:$FOUND16C "
|
||||
fi
|
||||
done
|
||||
if [[ -n "$HAZARDS16" ]]; then
|
||||
fail "unguarded array expansion(s) abort on bash < 4.4 under set -u: $(echo "$HAZARDS16" | tr '\n' ' ')"
|
||||
else
|
||||
pass "every array expansion uses the bash-3.2-safe \${arr[@]+\"\${arr[@]}\"} form"
|
||||
fi
|
||||
|
||||
# --- 17. The invocations whose arrays are closest to empty actually run. Under
|
||||
# a bash older than 4.4 this is genuine macOS-shell coverage; on a modern bash it
|
||||
# degrades to a smoke test, so the pass message names the shell that really ran.
|
||||
# Point VALE_WRAP_TEST_BASH at a 3.2 build to get the real thing in CI.
|
||||
echo ""
|
||||
echo "--- degenerate invocations survive on the oldest available bash ---"
|
||||
OLD_BASH="bash"
|
||||
OLD_BASH_VER="$(bash -c 'echo "${BASH_VERSINFO[0]}.${BASH_VERSINFO[1]}"')"
|
||||
for CAND in "${VALE_WRAP_TEST_BASH:-}" bash-3.2 bash3 /bin/bash /usr/local/bin/bash; do
|
||||
[[ -n "$CAND" ]] && command -v "$CAND" >/dev/null 2>&1 || continue
|
||||
CAND_VER="$("$CAND" -c 'echo "${BASH_VERSINFO[0]}.${BASH_VERSINFO[1]}"' 2>/dev/null)" || continue
|
||||
[[ -n "$CAND_VER" ]] || continue
|
||||
if (( ${CAND_VER%.*} * 100 + ${CAND_VER#*.} < ${OLD_BASH_VER%.*} * 100 + ${OLD_BASH_VER#*.} )); then
|
||||
OLD_BASH="$CAND"
|
||||
OLD_BASH_VER="$CAND_VER"
|
||||
fi
|
||||
done
|
||||
FIXTURE17="$(make_fixture 2)"
|
||||
mkdir -p "$FIXTURE17/emptydir"
|
||||
trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5" "$FIXTURE6" "$FIXTURE7" "$FIXTURE8" "$FIXTURE10" "$FIXTURE11" "$FIXTURE12" "$STUB13" "$FIXTURE14" "$FIXTURE17"' EXIT
|
||||
# Zero args, flags with no path, and a directory that walks to nothing are the
|
||||
# three shapes that leave vale_args/path_args/argv_paths at their emptiest.
|
||||
OUT17=""
|
||||
for ARGS17 in "" "--config $VALE_CONFIG" "--config $VALE_CONFIG emptydir"; do
|
||||
# shellcheck disable=SC2086 # deliberate word splitting of the argv fixture
|
||||
OUT17+="$( (cd "$FIXTURE17" && "$OLD_BASH" "$SCRIPT" $ARGS17 </dev/null 2>&1) || true)"
|
||||
done
|
||||
if echo "$OUT17" | grep -q "unbound variable"; then
|
||||
fail "aborted with 'unbound variable' on bash $OLD_BASH_VER — the bug this test guards against"
|
||||
else
|
||||
pass "degenerate invocations run clean under bash $OLD_BASH_VER ($OLD_BASH)"
|
||||
fi
|
||||
|
||||
# --- 18. The guarded expansion must keep argv word boundaries intact. Dropping
|
||||
# the quotes (`${arr[@]}`) also silences the unbound-variable abort, so it is the
|
||||
# tempting wrong fix — and it splits any path containing a space into two bogus
|
||||
# arguments. Case 15 covers spaces found by the directory walk; this covers a
|
||||
# space in the path argument itself, which is what argv_paths expands.
|
||||
echo ""
|
||||
echo "--- a path argument containing a space survives the guarded expansion ---"
|
||||
FIXTURE18="$(make_fixture 2)"
|
||||
trap 'rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5" "$FIXTURE6" "$FIXTURE7" "$FIXTURE8" "$FIXTURE10" "$FIXTURE11" "$FIXTURE12" "$STUB13" "$FIXTURE14" "$FIXTURE17" "$FIXTURE18"' EXIT
|
||||
SPACED18="$FIXTURE18/plugins/testplugin/skills/zzz skill dir"
|
||||
mkdir -p "$SPACED18"
|
||||
mv "$FIXTURE18/plugins/testplugin/skills/zzzskill/SKILL.md" "$SPACED18/SKILL.md"
|
||||
OUT18=$(run_wrap "$FIXTURE18" --config "$VALE_CONFIG" "plugins/testplugin/skills/zzz skill dir/SKILL.md")
|
||||
if echo "$OUT18" | grep -q "zzz skill dir/SKILL.md" && echo "$OUT18" | grep -q "VagueWording"; then
|
||||
pass "a path argument with a space is passed to vale as one word"
|
||||
else
|
||||
fail "a path argument with a space was split by the array expansion: $OUT18"
|
||||
fi
|
||||
|
||||
# The cases below share one cleanup list. The per-case trap rebuilding above
|
||||
# does not scale past the fixture count it already carries, and this trap is
|
||||
# installed last, so it is the one that runs.
|
||||
EXTRA_FIXTURES=()
|
||||
new_fixture() { EXTRA_FIXTURES+=("$1"); }
|
||||
cleanup_all() {
|
||||
rm -rf "$FIXTURE1" "$FIXTURE2" "$FIXTURE3" "$FIXTURE4" "$FIXTURE5" "$FIXTURE6" \
|
||||
"$FIXTURE7" "$FIXTURE8" "$FIXTURE10" "$FIXTURE11" "$FIXTURE12" "$STUB13" \
|
||||
"$FIXTURE14" "$FIXTURE17" "$FIXTURE18" \
|
||||
${EXTRA_FIXTURES[@]+"${EXTRA_FIXTURES[@]}"}
|
||||
}
|
||||
trap cleanup_all EXIT
|
||||
|
||||
# --- 19. Every YAML form whose parsed value is joined back out of 2+ physical
|
||||
# lines breaks the `text.frontmatter.description` scope identically, not just
|
||||
# the `>` folded block the flattener originally handled: a plain scalar wrapped
|
||||
# onto continuation lines, a double-quoted one, a single-quoted one, and a bare
|
||||
# `description:` whose value starts on the next line all report zero alerts
|
||||
# under bare vale. Each must come back with the same alerts as the single-line
|
||||
# spelling of the same sentence. Line and column numbers legitimately move (the
|
||||
# value lands on one physical line), so the comparison drops the `line:col`
|
||||
# prefix and compares the alert text — message, matched token, and rule name.
|
||||
REL_SKILL19="plugins/testplugin/skills/zzzskill/SKILL.md"
|
||||
DESC19_A="Use when the caller helps with a specific job"
|
||||
DESC19_B="and the second physical line will utilize the wrap"
|
||||
|
||||
# make_form_fixture spells the same two-clause description in one YAML scalar
|
||||
# form: single, folded, plain, dquote, squote, or keyonly.
|
||||
make_form_fixture() {
|
||||
local form="$1" dir
|
||||
dir="$(mktemp -d)"
|
||||
new_fixture "$dir"
|
||||
(cd "$dir" && git init -q)
|
||||
mkdir -p "$dir/plugins/testplugin/skills/zzzskill"
|
||||
{
|
||||
echo "---"
|
||||
echo "name: zzzskill"
|
||||
case "$form" in
|
||||
single) echo "description: $DESC19_A $DESC19_B" ;;
|
||||
folded) echo "description: >"; echo " $DESC19_A"; echo " $DESC19_B" ;;
|
||||
plain) echo "description: $DESC19_A"; echo " $DESC19_B" ;;
|
||||
dquote) echo "description: \"$DESC19_A"; echo " $DESC19_B\"" ;;
|
||||
squote) echo "description: '$DESC19_A"; echo " $DESC19_B'" ;;
|
||||
keyonly) echo "description:"; echo " $DESC19_A"; echo " $DESC19_B" ;;
|
||||
*) echo "make_form_fixture: unknown form '$form'" >&2; exit 1 ;;
|
||||
esac
|
||||
echo "---"
|
||||
echo ""
|
||||
echo "Body."
|
||||
} > "$dir/plugins/testplugin/skills/zzzskill/SKILL.md"
|
||||
echo "$dir"
|
||||
}
|
||||
|
||||
# Alert text with the `line:col` prefix and ANSI colouring stripped, sorted.
|
||||
# The `|| true` matters under this file's `set -o pipefail`: a report with no
|
||||
# alerts at all makes grep exit 1, which would abort the whole run inside the
|
||||
# command substitutions below — silently, before the empty-baseline guard could
|
||||
# print anything. Returning empty output instead is what makes that guard
|
||||
# reachable.
|
||||
alert_text() {
|
||||
echo "$1" \
|
||||
| sed -E 's/\x1b\[[0-9;]*m//g' \
|
||||
| { grep -oE '(error|warning|suggestion)[[:space:]]+.*' || true; } \
|
||||
| sed -E 's/[[:space:]]+/ /g' \
|
||||
| sort
|
||||
}
|
||||
|
||||
echo ""
|
||||
echo "--- every multi-line description form reports what its single-line form reports ---"
|
||||
FIXTURE19_SINGLE="$(make_form_fixture single)"
|
||||
BASELINE19="$(alert_text "$(run_wrap "$FIXTURE19_SINGLE" --config "$VALE_CONFIG" "$REL_SKILL19")")"
|
||||
if [[ -z "$BASELINE19" ]]; then
|
||||
# The loop below has to be skipped, not merely reported on: an empty baseline
|
||||
# compares equal to five empty results, so it would print five vacuous PASSes
|
||||
# alongside this one FAIL. The FAIL alone still fails the run at the end.
|
||||
fail "the single-line baseline reported nothing — the comparisons below would be vacuous, so they are skipped"
|
||||
else
|
||||
for FORM19 in folded plain dquote squote keyonly; do
|
||||
DIR19="$(make_form_fixture "$FORM19")"
|
||||
BARE19="$(cd "$DIR19" && vale --config "$VALE_CONFIG" "$REL_SKILL19" 2>&1 || true)"
|
||||
GOT19="$(alert_text "$(run_wrap "$DIR19" --config "$VALE_CONFIG" "$REL_SKILL19")")"
|
||||
if echo "$BARE19" | grep -q "VagueWording"; then
|
||||
fail "bare vale already flags the $FORM19 form, so this case can't detect a silently-skipped flattening"
|
||||
elif [[ "$GOT19" == "$BASELINE19" ]]; then
|
||||
pass "a $FORM19 multi-line description reports the same alerts as its single-line form"
|
||||
else
|
||||
fail "a $FORM19 multi-line description diverged from its single-line form: got [$GOT19]"
|
||||
fi
|
||||
done
|
||||
fi
|
||||
|
||||
# --- 20. A style token containing an ASCII apostrophe matches inside a
|
||||
# flattened description. The flattener used to substitute U+2019 for every `'`
|
||||
# before writing the scratch copy, so no rule whose token carried an apostrophe
|
||||
# could ever fire on a flattened description — a silent, rule-shaped blind spot.
|
||||
# All three branches that can hold an apostrophe are exercised: a value that is
|
||||
# safe unquoted; one that must be quoted (it contains `: `) and so lands in a
|
||||
# double-quoted scalar, since a single-quoted one would need the `''` escape
|
||||
# that kills the scope outright; and one that also holds a double quote, which
|
||||
# no inline scalar can spell verbatim and which therefore lands in a `|-`
|
||||
# literal block.
|
||||
echo ""
|
||||
echo "--- a style token containing an apostrophe matches in a flattened description ---"
|
||||
APOS_STYLE="$(mktemp -d)"
|
||||
new_fixture "$APOS_STYLE"
|
||||
mkdir -p "$APOS_STYLE/styles/Apostrophe"
|
||||
cat > "$APOS_STYLE/styles/Apostrophe/Token.yml" <<'EOF'
|
||||
extends: existence
|
||||
message: "apostrophe token: '%s'"
|
||||
level: error
|
||||
scope: text.frontmatter.description
|
||||
ignorecase: true
|
||||
tokens:
|
||||
- "user's task"
|
||||
EOF
|
||||
# A body-scoped companion rule, used by case 20b to read back the line number of
|
||||
# a line *after* the frontmatter — the only way to catch the blank-line pad
|
||||
# being off in either direction.
|
||||
cat > "$APOS_STYLE/styles/Apostrophe/Body.yml" <<'EOF'
|
||||
extends: existence
|
||||
message: "body token: '%s'"
|
||||
level: error
|
||||
scope: text
|
||||
tokens:
|
||||
- flattening marker phrase
|
||||
EOF
|
||||
cat > "$APOS_STYLE/.vale.ini" <<'EOF'
|
||||
StylesPath = styles
|
||||
|
||||
[**/SKILL.md]
|
||||
BasedOnStyles = Apostrophe
|
||||
EOF
|
||||
FIXTURE20_PLAIN="$(make_raw_fixture <<'EOF'
|
||||
---
|
||||
name: zzzskill
|
||||
description: >
|
||||
Use when the user's task needs handling, and a second physical
|
||||
line continues the folded scalar.
|
||||
---
|
||||
|
||||
Body.
|
||||
EOF
|
||||
)"
|
||||
new_fixture "$FIXTURE20_PLAIN"
|
||||
FIXTURE20_QUOTED="$(make_raw_fixture <<'EOF'
|
||||
---
|
||||
name: zzzskill
|
||||
description: >
|
||||
Triggers on: the user's task needing handling, and a second
|
||||
physical line continues the folded scalar.
|
||||
---
|
||||
|
||||
Body.
|
||||
EOF
|
||||
)"
|
||||
new_fixture "$FIXTURE20_QUOTED"
|
||||
# Needs quoting (`: `), holds an apostrophe AND a double quote — the one
|
||||
# combination no inline scalar can carry, so this is the `|-` literal-block
|
||||
# branch. The VagueWording tokens are there for case 20b, which reuses it.
|
||||
FIXTURE20_BLOCK="$(make_raw_fixture <<'EOF'
|
||||
---
|
||||
name: zzzskill
|
||||
description: >
|
||||
Triggers on: the user's task and "audit this" phrasing, which helps
|
||||
with and utilize things across a second physical line.
|
||||
---
|
||||
|
||||
Body carrying a flattening marker phrase for the line-number check.
|
||||
EOF
|
||||
)"
|
||||
new_fixture "$FIXTURE20_BLOCK"
|
||||
for CASE20 in "unquoted:$FIXTURE20_PLAIN" "double-quoted:$FIXTURE20_QUOTED" \
|
||||
"literal-block:$FIXTURE20_BLOCK"; do
|
||||
if run_wrap "${CASE20#*:}" --config "$APOS_STYLE/.vale.ini" "$REL_SKILL19" \
|
||||
| grep -q "Apostrophe.Token"; then
|
||||
pass "an apostrophe-bearing token matches in a flattened ${CASE20%%:*} description"
|
||||
else
|
||||
fail "an apostrophe-bearing token was rewritten out of a flattened ${CASE20%%:*} description"
|
||||
fi
|
||||
done
|
||||
|
||||
# --- 20b. The `|-` literal-block branch that case 20 just proved lossless must
|
||||
# also keep the rest of the scope working and keep the line accounting right.
|
||||
# The block is 2 physical lines where every inline form is 1, so the blank-line
|
||||
# pad that preserves later line numbers has to drop by one. The second
|
||||
# assertion pins that arithmetic against the body line's true number: case 3's
|
||||
# `<= original line count` bound would not, since a pad that is one line short
|
||||
# shifts every later line *up*, staying inside the bound while still lying.
|
||||
echo ""
|
||||
echo "--- the |- literal-block fallback lints normally and preserves line numbers ---"
|
||||
OUT20B=$(run_wrap "$FIXTURE20_BLOCK" --config "$VALE_CONFIG" "$REL_SKILL19")
|
||||
if echo "$OUT20B" | grep -q "VagueWording"; then
|
||||
pass "a description needing quotes with both an apostrophe and a double quote is still linted"
|
||||
else
|
||||
fail "a description needing quotes with both an apostrophe and a double quote produced no alerts"
|
||||
fi
|
||||
WANT20B_LINE="$(grep -n 'flattening marker phrase' "$FIXTURE20_BLOCK/$REL_SKILL19" | cut -d: -f1)"
|
||||
# `--output line` prints `file:line:col:Rule:message`, so the line number reads
|
||||
# back without any wrapping or colour to strip.
|
||||
GOT20B_LINE="$(run_wrap "$FIXTURE20_BLOCK" --config "$APOS_STYLE/.vale.ini" --output line "$REL_SKILL19" \
|
||||
| grep 'Apostrophe.Body' | head -1 | cut -d: -f2)"
|
||||
if [[ "$GOT20B_LINE" == "$WANT20B_LINE" ]]; then
|
||||
pass "a body line after a |- flattened description keeps its original line number ($WANT20B_LINE)"
|
||||
else
|
||||
fail "the |- block's blank-line pad shifted the body: vale reported line $GOT20B_LINE, the file has it at $WANT20B_LINE"
|
||||
fi
|
||||
|
||||
# --- 21. A symlinked file inside a directory argument is mirrored and linted.
|
||||
# Vale follows symlinks (both a symlinked file and a file under a symlinked
|
||||
# directory), so a `-type f` walk of the tree reported "0 files" where bare vale
|
||||
# reports one — and the audit skills read a "0 files" report as NOT RUN.
|
||||
echo ""
|
||||
echo "--- mirrors a symlinked file reached through a directory argument ---"
|
||||
FIXTURE21="$(make_fixture 2)"
|
||||
new_fixture "$FIXTURE21"
|
||||
mkdir -p "$FIXTURE21/real"
|
||||
mv "$FIXTURE21/$REL_SKILL19" "$FIXTURE21/real/SKILL.md"
|
||||
ln -s ../../../../real/SKILL.md "$FIXTURE21/$REL_SKILL19"
|
||||
BARE21="$(cd "$FIXTURE21" && vale --config "$VALE_CONFIG" plugins 2>&1 || true)"
|
||||
WRAPPED21="$(run_wrap "$FIXTURE21" --config "$VALE_CONFIG" plugins)"
|
||||
BARE21_FILES="$(echo "$BARE21" | sed -E 's/\x1b\[[0-9;]*m//g' | grep -oE 'in [0-9]+ files?' | tail -1)"
|
||||
WRAPPED21_FILES="$(echo "$WRAPPED21" | sed -E 's/\x1b\[[0-9;]*m//g' | grep -oE 'in [0-9]+ files?' | tail -1)"
|
||||
if [[ "$BARE21_FILES" != "in 1 file" ]]; then
|
||||
fail "bare vale did not lint the symlinked file ($BARE21_FILES), so this case can't detect the walk dropping it"
|
||||
elif [[ "$WRAPPED21_FILES" != "$BARE21_FILES" ]]; then
|
||||
fail "the directory walk dropped a symlinked file: wrapper saw '$WRAPPED21_FILES', bare vale '$BARE21_FILES'"
|
||||
elif echo "$WRAPPED21" | grep -q "VagueWording"; then
|
||||
pass "a symlinked file under a directory argument is mirrored, flattened and flagged"
|
||||
else
|
||||
fail "a symlinked file was mirrored but not flattened — no alert came back"
|
||||
fi
|
||||
|
||||
# --- 22. The value of a separated two-argv flag is never treated as a lint
|
||||
# target, however file-like it looks. `--output tmpl.tmpl` names a real
|
||||
# template file: classifying it as input both linted the template and reordered
|
||||
# argv, so vale received `--output --no-wrap` and died on `open :`.
|
||||
echo ""
|
||||
echo "--- a separated flag value that names a real file is not linted as a target ---"
|
||||
FIXTURE22="$(make_fixture 1)"
|
||||
new_fixture "$FIXTURE22"
|
||||
printf 'TMPL{{range .Files}} {{.Path}}{{end}}\n' > "$FIXTURE22/tmpl.tmpl"
|
||||
WRAPPED22="$(run_wrap "$FIXTURE22" --config "$VALE_CONFIG" --output tmpl.tmpl --no-wrap "$REL_SKILL19")"
|
||||
BARE22="$(cd "$FIXTURE22" && vale --config "$VALE_CONFIG" --output tmpl.tmpl --no-wrap "$REL_SKILL19" 2>&1 || true)"
|
||||
# The fixture's description is a single physical line, so flattening is a no-op
|
||||
# and the two invocations must agree byte for byte.
|
||||
if [[ "$WRAPPED22" == "$BARE22" ]]; then
|
||||
pass "a separated --output value is passed through to vale, not linted"
|
||||
else
|
||||
fail "a separated --output value was misrouted: wrapper gave [$WRAPPED22], bare vale [$BARE22]"
|
||||
fi
|
||||
|
||||
# --- 23. A path argument that does not exist is a hard error. Bare vale drops
|
||||
# it, falls back to stdin and prints `0 errors ... in stdin` with exit 0, so a
|
||||
# typo'd target is indistinguishable from a clean run — and the audit skills'
|
||||
# NOT RUN guard string-matches `0 files`, which `in stdin` never produces. This
|
||||
# is a deliberate divergence from bare vale, documented in the wrapper header.
|
||||
echo ""
|
||||
echo "--- a nonexistent path argument fails loudly instead of falling back to stdin ---"
|
||||
set +e
|
||||
OUT23="$(cd "$FIXTURE22" && bash "$SCRIPT" --config "$VALE_CONFIG" plugins/testplugin/skills/zzzskill/SKILLL.md 2>&1)"
|
||||
RC23=$?
|
||||
set -e
|
||||
if [[ $RC23 -eq 0 ]]; then
|
||||
fail "a typo'd path exited 0 — indistinguishable from a clean run, the bug this test guards against"
|
||||
elif echo "$OUT23" | grep -q "in stdin"; then
|
||||
fail "a typo'd path fell back to reading stdin and reported 'in stdin' instead of erroring"
|
||||
elif echo "$OUT23" | grep -q "SKILLL.md"; then
|
||||
pass "a typo'd path exits nonzero with a message naming the path"
|
||||
else
|
||||
fail "a typo'd path exited $RC23 but the message does not name it: $OUT23"
|
||||
fi
|
||||
|
||||
# --- 24. `--output`'s built-in style names must not be path-absolutized. The
|
||||
# wrapper rewrites path-valued flag values to absolute form so they still
|
||||
# resolve after the `cd` into the scratch mirror, deciding with an `-e`
|
||||
# existence test — but `line`, `JSON` and `CLI` are style names, not paths. With
|
||||
# a file or directory of that name sitting in the caller's cwd the test hit, the
|
||||
# built-in became `$cwd/line`, and vale flipped into template mode and died with
|
||||
# `E100 [template] Runtime error` where bare vale prints a normal report.
|
||||
echo ""
|
||||
echo "--- a built-in --output style name survives a same-named entry in the cwd ---"
|
||||
FIXTURE24="$(make_fixture 2)"
|
||||
new_fixture "$FIXTURE24"
|
||||
mkdir -p "$FIXTURE24/line"
|
||||
: > "$FIXTURE24/JSON"
|
||||
for FORM24 in "--output line" "--output=line" "--output JSON" "--output=JSON"; do
|
||||
# shellcheck disable=SC2086 # deliberate word splitting of the argv fixture
|
||||
OUT24="$(run_wrap "$FIXTURE24" --config "$VALE_CONFIG" $FORM24 "$REL_SKILL19")"
|
||||
if echo "$OUT24" | grep -q "E100"; then
|
||||
fail "'$FORM24' was rewritten to a cwd path and vale flipped into template mode — the bug this test guards against"
|
||||
elif echo "$OUT24" | grep -q "VagueWording"; then
|
||||
pass "'$FORM24' is passed through as a built-in style name"
|
||||
else
|
||||
fail "'$FORM24' produced no alert: $OUT24"
|
||||
fi
|
||||
done
|
||||
|
||||
echo ""
|
||||
echo "Results: $PASS passed, $FAIL failed"
|
||||
[[ $FAIL -eq 0 ]]
|
||||
|
||||
Reference in New Issue
Block a user