diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 4ee8774..6ed72bf 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -38,7 +38,12 @@ "repo": "mattpocock/skills", "source": "github" } + }, + { + "description": "Skills and agents for configuring and running linters.", + "name": "lint", + "source": "./plugins/lint" } ], - "version": "0.2.0" + "version": "0.3.1" } diff --git a/.github/plugin/marketplace.json b/.github/plugin/marketplace.json index 4ee8774..6ed72bf 100644 --- a/.github/plugin/marketplace.json +++ b/.github/plugin/marketplace.json @@ -38,7 +38,12 @@ "repo": "mattpocock/skills", "source": "github" } + }, + { + "description": "Skills and agents for configuring and running linters.", + "name": "lint", + "source": "./plugins/lint" } ], - "version": "0.2.0" + "version": "0.3.1" } diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 39ae3de..165ea75 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -61,6 +61,24 @@ repos: pass_filenames: false always_run: true + - id: check-vale-style-sync + name: Check Vale style copies are in sync + description: Diff skill-audit's Vale copy against agent-audit's canonical copy + entry: bash scripts/check-vale-style-sync.sh + language: system + stages: [pre-push] + pass_filenames: false + always_run: true + + - id: check-release-needed + name: Check a release tag covers .pre-commit-hooks.yaml's paths + description: On push to main only, fail if files exposed via .pre-commit-hooks.yaml changed since the last tag + entry: bash scripts/check-release-needed.sh + language: system + stages: [pre-push] + pass_filenames: false + always_run: true + - id: validate-plugins name: Validate plugins description: Run claude plugin validate --strict on every plugin directory @@ -98,6 +116,33 @@ repos: fi done + - id: skill-size-check + stages: ['pre-commit'] + name: SKILL.md size ceiling + description: Enforce agentskills.io's 500-line/5,000-token SKILL.md size ceiling + entry: scripts/skill-size-check.sh + language: script + files: '^plugins/[^/]+/skills/[^/]+/SKILL\.md$' + pass_filenames: true + + - id: vale-audit-prefilter-skill + stages: ['pre-commit'] + name: Vale audit prefilter (SKILL.md) + description: Run Vale against SKILL.md files as a deterministic prefilter for skill-audit, via skill-audit's own bundled copy + entry: plugins/kyberforge/skills/skill-audit/scripts/vale-wrap.sh + language: script + files: '^plugins/[^/]+/skills/[^/]+/SKILL\.md$' + pass_filenames: true + + - id: vale-audit-prefilter-agent + stages: ['pre-commit'] + name: Vale audit prefilter (agent files) + description: Run Vale against agent markdown files as a deterministic prefilter for agent-audit, via agent-audit's own bundled copy + entry: plugins/kyberforge/skills/agent-audit/scripts/vale-wrap.sh + language: script + files: '^plugins/[^/]+/agents/[^/]+\.md$' + pass_filenames: true + - repo: meta hooks: - id: check-hooks-apply diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml new file mode 100644 index 0000000..eee3986 --- /dev/null +++ b/.pre-commit-hooks.yaml @@ -0,0 +1,20 @@ +- id: kyberforge-vale-audit-skill + name: Kyberforge Vale prose audit (SKILL.md) + description: Deterministic prose-pattern prefilter for kyberforge's skill-audit, via its own bundled Vale config/styles + entry: plugins/kyberforge/skills/skill-audit/scripts/vale-wrap.sh + language: script + files: '(^|/)SKILL\.md$' + +- id: kyberforge-vale-audit-agent + name: Kyberforge Vale prose audit (agent files) + description: Deterministic prose-pattern prefilter for kyberforge's agent-audit, via its own bundled Vale config/styles + entry: plugins/kyberforge/skills/agent-audit/scripts/vale-wrap.sh + language: script + files: '(^|/)agents/[^/]+\.md$|\.agent\.md$' + +- id: kyberforge-skill-size-check + name: SKILL.md size ceiling + description: Enforce agentskills.io's 500-line/5,000-token SKILL.md size ceiling + entry: scripts/skill-size-check.sh + language: script + files: '(^|/)SKILL\.md$' diff --git a/AGENTS.md b/AGENTS.md index 905626e..d169ff3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -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` @@ -22,6 +22,7 @@ Fall back to raw shell only when no skill covers it. ## Setup and testing - Install git hooks via `git:pc-run`, wiring all three stages — this repo's `.pre-commit-config.yaml` has no `default_install_hook_types`, so a plain install silently skips `commit-msg` (Conventional Commits) and `pre-push` (tests, manifest check). +- Install the `vale` binary — required by the `vale-audit-prefilter-skill`/`-agent` pre-commit hooks, which run on every commit touching a `SKILL.md` or agent `.md` file. Without it the hooks fail with a bare "command not found" and no install pointer. `brew install vale` (macOS), `snap install vale` (Linux), `choco install vale` (Windows), or see https://vale.sh/docs/vale-cli/installation/. No `vale sync` needed — the `Kyberforge` styles are committed under `plugins/kyberforge/skills/{skill-audit,agent-audit}/assets/vale/styles/`, not downloaded packages (see ADR-0014). - Run `bash tests/run-tests.sh` before considering any change done — it runs every `test-*.sh` script in the repo plus the bats suite (`--bats-only` for just bats). First run auto-initializes the bats submodules; no manual `git submodule update` needed. - Pushing re-runs the full suite plus `scripts/check-manifests.sh` via the pre-push hook — same commands, so run them locally first. - Author commits with `git:git-commits` — it validates Conventional Commits (enforced at `commit-msg`) for you. diff --git a/CONTEXT.md b/CONTEXT.md index bb27229..b77c576 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -66,5 +66,15 @@ A skill pair in the `core` plugin for writing, updating, and reviewing a repo's ### provider-adapter-author A companion skill (`core` plugin) that detects a target repo's provider-specific instruction file (`CLAUDE.md`, `.cursor/rules/*.mdc`, `copilot-instructions.md`, etc.) and, where it duplicates content AGENTS.md should own, converts it into a thin adapter that imports AGENTS.md — mirroring this repo's own ADR-0002/ADR-0003 two-tier adapter pattern. Self-validates via its own bundled deterministic script (`scripts/validate-adapter.sh`: checks for an import reference, no duplicated headings, size threshold) rather than a separate paired audit skill — the check is mechanical, so a script suffices per governance.md's "prefer deterministic code for repeatable tasks." `agentsmd-author` calls this skill via skill composition when it detects an existing provider file with overlapping content. +### lint plugin +A standalone, repo-agnostic plugin (`plugins/lint/`) for configuring and running linters — not scoped to kyberforge's own meta-tooling. First linter is Vale (prose style linting), split into two skills per the git/gitea per-concern pattern: `vale-config` (setup — `.vale.ini`, `StylesPath`, styles) and `vale-run` (invoke Vale, interpret/report findings). A `lint-runner` agent composes these for isolated-context lint sweeps; it is report-only (no `Edit` tool) — it flags findings, it does not rewrite prose. Vale's research docs (`docs/research/docs/vale/`) moved from `plugins/kyberforge/` to `plugins/lint/` to keep the provenance chain same-plugin. + +### Vale audit prefilter (skill-audit / agent-audit) +Wiring Vale as a deterministic prefilter for `skill-audit`/`agent-audit`'s Description dimension (ADR motivation: issue #84) is repo-specific, not part of the generic `lint` plugin, so 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: , rev: ` 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 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 `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 `` (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. diff --git a/LESSONS.md b/LESSONS.md index b81c9d8..835a386 100644 --- a/LESSONS.md +++ b/LESSONS.md @@ -137,3 +137,31 @@ After a PR merge (with Gitea's default auto-delete-branch behavior), `git branch ## 2026-05-18 — Planning meta-commentary does not belong in deployed artifacts During write-skill refactor, an "open thread" note (about a deferred research step) was written directly into the SKILL.md Process section. The user caught it. The rule it violated: a deployed artifact (SKILL.md, a runtime file loaded by agents) must not contain planning meta-commentary — deferred items, open threads, and implementation notes belong in the issue file, which is the planning artifact. The skill body should contain only content relevant to runtime execution. If a decision is deferred, record it in the issue and leave no trace in the skill. The distinction: issue = planning record; skill = executable instruction. + +## 2026-08-08 — A clean linter result can mean "nothing was checked" + +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 + +Vale's output fed two consumers with different contracts: the audit skills read severity *strings* to grade a report (`error`→FAIL, `warning`→SUGGESTION), while the pre-commit hook read the process *exit code* to allow or block a commit. Severities were tuned for the first consumer; the second silently inherited whatever exit code that produced, which was always 0. CONTEXT.md described both as a single mechanism under one heading, which is precisely why the divergence went unnoticed — there was no vocabulary in which "the gate" and "the prefilter" were different things that could disagree. Fix: when one output feeds two consumers, name them separately in the domain language and state each contract explicitly. If they cannot be given independent contracts, collapse them into one — which is what happened here: every rule became `level: error`, so the gate and the audit now share a single verdict with nothing to keep in sync. + +## 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 ` 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. diff --git a/core/instructions/testing.md b/core/instructions/testing.md index 69566d5..33fa97b 100644 --- a/core/instructions/testing.md +++ b/core/instructions/testing.md @@ -4,3 +4,4 @@ - Automate everything automatable. Manual testing only for nuanced UI/UX or agent interaction behaviour requiring human judgment. - Test observable end-state, not implementation internals. Tests must survive refactoring. - No test is better than a wrong test. A passing mock that masks a real failure is actively harmful. +- A clean result can mean nothing ran. Before trusting a new check, prove it fails against a deliberately-bad fixture, then run it against the real target. Where a check can scan zero inputs, assert on the input count, not just the exit code — a zero-file run and a real clean pass look identical otherwise. diff --git a/docs/adr/0013-vale-harness-scope-and-rule-sources.md b/docs/adr/0013-vale-harness-scope-and-rule-sources.md new file mode 100644 index 0000000..2dfb67d --- /dev/null +++ b/docs/adr/0013-vale-harness-scope-and-rule-sources.md @@ -0,0 +1,121 @@ +# Vale audit prefilter expands into a plugin-content harness, scoped to prose-pattern rules only + +Issue #84 wired Vale as a deterministic prefilter for `skill-audit`/`agent-audit`, scoped to +exactly four pattern-matchable checks (imperative description opener, vague capability wording, +generic reference-pointer padding, Copilot's dead `Use proactively` phrasing), documented only in +CONTEXT.md's "Vale audit prefilter" section — never its own ADR — and explicitly excluding body +discipline, near-miss exclusion strength, and control calibration as non-goals. This ADR records a +deferred PR #85 review item to broaden that coverage, retroactively captures #84's own rationale +(since it was never recorded as a decision in its own right), and layers the expansion on top +without reversing or weakening the original four rules. + +**File scope stays the same.** `SKILL.md` plus agent files (`**/agents/*.md`, +`**/*.agent.md`) only — matching the existing prefilter's globs. Skill-level +`README.md` files and `plugin.json` manifests are not added: README.md files are navigational, not +spec-governed content, and `plugin.json` is JSON, not prose Vale can meaningfully lint. + +**Rule categories are prose-pattern-matchable only.** Structural, schema, and security concerns +stay out of this Vale-based harness because this repo already has dedicated tools for them: +`skill-frontmatter` (required frontmatter fields), `validate-plugins`/`validate-marketplace` +(`claude plugin validate --strict`, schema), and `gitleaks`/`detect-private-key` (secrets). +Duplicating those concerns as Vale rules would fight tools that already own them better. + +**Governance docs are excluded as a rule source.** `docs/research/governance_principles/CONTROLS.md` +and `governance.md` were investigated and found to contribute nothing minable: CONTROLS.md is +org/CI-infrastructure controls (secret scanning, dependency/license scanning, agent permission +scoping, audit logging, human approval gates, periodic reviews) — none of it is a prose pattern +expressible as a Vale rule against SKILL.md/agent-file text, and what it does cover is either +already handled elsewhere (gitleaks) or genuinely out of scope for a plugin-content prose harness +(dependency/license scanning is a code-dependency concern, not skill authoring). + +**Spec-derived custom rules stay mostly as-is.** Re-reading agentskills.io's +`optimizing-descriptions.md` and `skill-authoring.md`, plus `claude-code-plugins/agent-definition.md` +and `github-copilot-plugins/agent-definition.md`, found that the existing four Kyberforge rules +already cover the pattern-matchable surface those specs describe. The remaining spec guidance — +calibrating control vs. giving freedom, avoiding menus of options, coherent skill scope, moderate +detail level — is semantic judgment, already `skill-audit`'s job via LLM review, not new lintable +rules. One confirmation surfaced: Claude Code's `Use proactively` phrasing is meaningful for `.md` +agent files (it triggers auto-invocation), unlike Copilot's `.agent.md` files where it's dead +phrasing — so `KyberforgeCopilot/ProactivePhrase`'s existing `.agent.md`-only scope is correct and +must not be extended to `.md` files. + +**`write-good`/`alex` are trialed, not adopted wholesale.** These built-in/third-party Vale +packages are tuned for general blog-style prose (passive voice, weasel words, wordy phrases) and +are expected to be noisy against this repo's terse, imperative instruction-file corpus. Only +individual rules proven low-noise against the existing corpus get cherry-picked into +`styles/Kyberforge`; the packages are never referenced wholesale in `BasedOnStyles`. + +**A new non-Vale check closes a real gap.** `skill-authoring.md` states `SKILL.md` should stay +under 500 lines / 5,000 tokens — currently unenforced anywhere in this repo. This is a whole-file +length ceiling, not a text pattern, so it isn't a Vale rule — it becomes a new deterministic script +and pre-commit hook, sibling to the existing `skill-frontmatter` hook. + +**Rules land directly in `styles/Kyberforge`, enforcing immediately.** No trial/report-only tier +is introduced (see Considered Options). "Enforcing immediately" holds only because every rule in +both styles is `level: error`: Vale's exit code keys on `error`-level alerts alone, so a +`warning`- or `suggestion`-level rule prints an alert and still exits 0, and pre-commit suppresses +output from hooks that pass — such a rule is invisible and blocks nothing. Every Vale alert is +therefore a FAIL, in the audit skills and in the blocking pre-commit hook alike, with no ignorable +tier; that matches every other gate in this repo (shellcheck, the test suite, +conventional-pre-commit). The implementation pass finalizes the cherry-picked +`write-good`/`alex` rules and any new spec-derived rule wording, runs the full set against the +existing SKILL.md/agent-file corpus, fixes any resulting violations across that corpus, and lands +the rule changes and the corpus fixes as one atomic commit — the same enforcement model as the +original four rules, never a partial or opt-in state. + +## Considered options + +**Phased rollout via a separate trial style + config (rejected).** A `styles/KyberforgeTrial/` +directory plus a parallel `.vale.trial.ini` (mirroring the root config's globs but with +`BasedOnStyles = Kyberforge, KyberforgeTrial`) would let new rules be swept report-only via +`lint-runner`/`vale-run` before promotion into the enforcing `styles/Kyberforge` + root +`.vale.ini`. This was considered because `BasedOnStyles = Kyberforge` activates every rule file +under that directory automatically — there's no partial/opt-in application within a style, so a +rule dropped straight into `styles/Kyberforge` goes live in the blocking pre-commit hook +immediately. Rejected in favor of finalizing rules directly and fixing violations via subagent +before committing: simpler, no new trial-config machinery to build or maintain — at the cost of no +standing report-only tier for future candidate rules. Note that the first implementation shipped +graded severities (`error`/`warning`/`suggestion`) and thereby recreated the rejected option by +accident: the five non-`error` rules never affected an exit code and never surfaced output through +a passing pre-commit hook, so they were a report-only tier that reported to nobody. Flattening +every rule to `level: error` is what actually implements this decision. + +## Consequences + +- `styles/Kyberforge/` gained one new rule file, cherry-picked from `write-good`/`alex` as + low-noise against this repo's corpus: `SentenceOpenerThereIs.yml` (22 hits across 273 held-out + markdown files; both in-corpus hits were clean rewrites, needing no suppression). +- A second candidate, `VagueQualifier.yml`, was cherry-picked and then dropped. Against the 41 + skill/agent files it hit twice: one marginal real finding (`prototype/SKILL.md`, "very different" + → "fundamentally different") and one false positive (`caveman/SKILL.md`, which *quotes* `of + course` as an example of filler — a mention, not a use) that no rewrite could clear, forcing the + repo's only Vale suppression comments. Of its 15 held-out hits, 9 were in `docs/research/examples/` + (out-of-scope upstream material) and the remaining 6 were the word "very" in two idioms in a + single research doc, each already adjacent to the hard number carrying the fact. One marginal + 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`. 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 + `plugins/lint/` so the prefilter also works for repos that install `kyberforge@holocron` as an + external plugin, rather than living at this repo's root — was deliberately deferred, not fixed, + in this pass. This repo-root placement remains intentional: this ADR's "File scope stays the + same" framing is specific to Kyberforge's own authoring conventions in this repo, not a generic + `lint`-plugin feature. Portability is a known limitation, tracked for a separate future session, + not silently forgotten. + +**What this ADR's implementation pass did:** synced and trialed `write-good`/`alex` against the +existing SKILL.md/agent-file corpus, cherry-picked the one low-noise rule above into +`styles/Kyberforge`, wrote `scripts/skill-size-check.sh` and its pre-commit hook, fixed the +resulting corpus violations, and landed the rule changes and corpus fixes as one atomic commit — +matching the enforcement model described above (no partial or opt-in state), with every rule at +`level: error` so that model is real rather than nominal. diff --git a/docs/adr/0014-vale-prefilter-ships-from-the-plugin.md b/docs/adr/0014-vale-prefilter-ships-from-the-plugin.md new file mode 100644 index 0000000..91a6e91 --- /dev/null +++ b/docs/adr/0014-vale-prefilter-ships-from-the-plugin.md @@ -0,0 +1,188 @@ +# Kyberforge's Vale prefilter ships from the plugin, with `.pre-commit-hooks.yaml` for external git-hook/CI enforcement + +**Resolves:** ADR-0013's deferred "styles-portability" consequence — `.vale.ini`/`styles/` moving +out of the repo root was deliberately deferred there, not fixed. ADR-0013's other content +(rule scope, `level: error` model, `SentenceOpenerThereIs`/`VagueQualifier` trial outcomes) is +unaffected and remains in force. + +`skill-audit`/`agent-audit`'s Step 1 called +`"$(git rev-parse --show-toplevel)/scripts/vale-wrap.sh" --config "$(git rev-parse --show-toplevel)/.vale.ini"` +— which resolves to whichever repo the skill happens to be running in. Inside `ai-development` +that's this repo; in any external repo that installs `kyberforge@holocron` as a plugin, it's that +repo's own root, which has no `.vale.ini` or `vale-wrap.sh`. The prefilter silently fell back to +full LLM judgment every time outside this repo — the exact gap ADR-0013 named and deferred. + +## Decision + +**Runtime (a live Claude Code session):** the Vale config, styles, and wrapper script move into +the plugin itself, following the no-cross-skill-path rule already established in +`skill-author/references/deployment-modes.md` (a plugin's cache-install only copies each skill's +own files; there is no plugin-level shared directory). `agent-audit` needs both `Kyberforge` and +`KyberforgeCopilot` (it lints `.agent.md` files), so `plugins/kyberforge/skills/agent-audit/assets/vale/` +is the canonical, superset copy. `skill-audit` needs a second, smaller copy +(`plugins/kyberforge/skills/skill-audit/assets/vale/`, `Kyberforge` only) since it cannot +reference agent-audit's copy across the skill boundary. Both skills' Step 1 now resolve +`scripts/vale-wrap.sh`/`assets/vale/.vale.ini` relative to their own directory, the same way +`scripts/validate.sh ` already does — no new resolution mechanism, just applying the +existing one consistently. + +**git hooks / CI outside a Claude Code session** have no plugin cache and no +`${CLAUDE_PLUGIN_ROOT}` — a CI runner in particular is guaranteed not to have one. The mechanism +that works there for any consumer, with or without Claude Code installed, is pre-commit's own +hook-repo protocol: this repo now ships a root-level `.pre-commit-hooks.yaml` exposing +`kyberforge-vale-audit-skill`, `kyberforge-vale-audit-agent`, and `kyberforge-skill-size-check`. +Any external repo adds `repo: , rev: ` to its own `.pre-commit-config.yaml` +and gets all three, fully decoupled from Claude Code. CI is the identical `pre-commit run +--all-files` call, so the same manifest covers "possibly CI" from the original ask. + +**This repo's own dev-time gate** consumes the same plugin-bundled copies instead of a third +root-level copy — per explicit instruction, this repo should be set up like any other consumer +would be, not dogfood a special root-only path. The existing `repo: local` hook is retargeted +(not removed): `entry:` now points at `plugins/kyberforge/skills/{skill-audit,agent-audit}/scripts/vale-wrap.sh`. +`repo: local` is kept rather than switching to a pinned self-reference +(`repo: , rev: `) — a pinned self-reference would lint working-tree edits against +the *last tagged release*, not the change actually being made, which is wrong for the repo that +*is* the source of the hook. This mirrors standard practice among hook-author repos (pre-commit's +own `pre-commit-hooks`, `shellcheck-py`): `repo: local` for self-consumption, `.pre-commit-hooks.yaml` +for everyone else, same underlying files and commands either way. + +**One hook per file-scope, not one combined hook.** The old root `.vale.ini` had both the +`[**/SKILL.md]` and `[**/agents/*.md]`/`[**/*.agent.md]` glob sections in a single file, so one +pre-commit hook covered both. Splitting the config into two skill-scoped copies means a single +hook entry pointed at only one copy would silently 0-file-skip the other file type. Both the +local `.pre-commit-config.yaml` hooks and the external-facing `.pre-commit-hooks.yaml` therefore +define separate `-skill`/`-agent` hook IDs, each with a `files:` regex matching exactly what its +target copy's glob covers. (Confirmed empirically before deleting the root files: retargeting a +single hook at agent-audit's copy silently scanned 0 SKILL.md files.) + +**The hook `entry:` is the wrapper alone; the wrapper self-locates its config.** pre-commit +prefixes only `entry[0]` with the hook-repo clone path (`cmd = (prefix.path(cmd[0]), *cmd[1:])`); +every later argument is handed to the process untouched and so resolves against the *consuming* +repo's root. A `--config plugins/kyberforge/skills/…/assets/vale/.vale.ini` in +`.pre-commit-hooks.yaml` therefore named a path no consumer has, and every external run died with +`E100 [--config] Runtime error`. The external-consumer contract this ADR exists to establish +cannot be expressed as a `--config` argument at all — the config path has to be derived inside +the process, from the script's own location. `vale-wrap.sh` accordingly defaults to its sibling +`assets/vale/.vale.ini`, resolved from `${BASH_SOURCE[0]}`, whenever no `--config` is supplied; +an explicit `--config` from any other caller still wins and still resolves against the caller's +cwd, so both audit skills' Step 1 (`--config assets/vale/.vale.ini`) is unaffected. Both +manifests now carry the identical argument-free `entry:`. Keeping them identical is part of the +decision: the local `repo: local` hook resolved its `--config` correctly only because the +consuming repo *was* this repo, and that one difference is why three review rounds exercised a +code path no external consumer ever takes. + +**Vale's `StylesPath` resolves relative to the `.vale.ini` file's own location**, confirmed +against `docs.vale.sh/keys/stylespath` — so a config path into the plugin finds that ini's +sibling `styles/` regardless of the caller's cwd, whether it arrives as an explicit `--config` or +as the wrapper's self-located default. No extra path-juggling is needed beyond `vale-wrap.sh`'s +cwd-relative `--config`/path-argument handling and that fallback. + +**A sync-check catches drift between the two copies.** `scripts/check-vale-style-sync.sh` diffs +`scripts/vale-wrap.sh` and `assets/vale/styles/Kyberforge/` between skill-audit and agent-audit +(not `.vale.ini` — those legitimately differ, scoped to different glob sections), wired at +`pre-push` alongside `check-manifests`. `.vale.ini` itself isn't diffed since divergence there is +by design. + +**External `.pre-commit-hooks.yaml` consumers pin `rev:` to a tag, not a commit SHA.** This repo +had no tags before this change; going forward, a `vX.Y.Z` tag is cut whenever hook-relevant files +change, matching how every other `repo:` entry in this repo's own `.pre-commit-config.yaml` +already pins (`v2.4.0`, `v8.21.2`, ...). + +## Considered options + +**Keep a third root-level copy, dogfooded specially (rejected).** Simpler in that this repo's own +hook wouldn't need retargeting at all. Rejected on explicit instruction: this repo should consume +the same portability path an external repo would, not carve out a special root-only case that +never gets exercised the way external consumers exercise it. + +**Publish styles as a hosted Vale package via `Packages = ` (deferred, not rejected).** +Vale supports fetching a style from a direct `.zip` URL via `vale sync`, fully decoupled from +Claude Code and from pre-commit's hook-repo protocol — usable by any repo, even ones that never +install `kyberforge` at all. This is a larger, separate investment (a release/versioning pipeline +for the package itself) not required to satisfy the current ask; noted here so a future reader +doesn't wonder if it was overlooked. + +## Consequences + +- Root `.vale.ini`, `styles/`, `scripts/vale-wrap.sh` are deleted. Two copies remain: + `plugins/kyberforge/skills/agent-audit/assets/vale/` (canonical, superset) and + `plugins/kyberforge/skills/skill-audit/assets/vale/` (subset, `Kyberforge` only). +- `plugins/kyberforge`'s `plugin.json` and `.claude-plugin/plugin.json` both patch-bump for every + shipped content change (per ADR-0006's version-parity invariant): `1.2.5` for the relocation + itself, `1.2.6` for the self-locating `vale-wrap.sh` that followed. +- **`.pre-commit-hooks.yaml` entries are a bare script path and nothing else — a constraint, not a + house style, and it binds every future hook here, not just the Vale two.** Since pre-commit + rewrites only `entry[0]` into the hook-repo clone, no argument token in any entry can reference + a file this repo ships: a relative path resolves against the *consuming* repo and hard-fails, + and the absolute path is unknowable at author time. A hook that needs one of its own bundled + files must have the script self-locate it from `$0`/`${BASH_SOURCE[0]}`, exactly as + `vale-wrap.sh` now does for `.vale.ini`. Anything else rediscovers this as another `E100`. + `.pre-commit-config.yaml` stays byte-identical to the shipped manifest on those `entry:` lines + so the local gate keeps exercising the same resolution path a consumer does. +- `tests/test-vale-wrap.sh` now exercises skill-audit's copy specifically — its fixtures are all + `SKILL.md`-shaped, and only skill-audit's `.vale.ini` has the matching glob section. +- The first `vX.Y.Z` tag is cut once this change and its tests pass, giving external + `.pre-commit-hooks.yaml` consumers something to pin. +- **Cutting the tag is not left to memory.** `scripts/check-release-needed.sh`, wired at + `pre-push`, hard-fails — but only when `PRE_COMMIT_REMOTE_BRANCH` (set by pre-commit's + `hook-impl` for pre-push hooks) is `refs/heads/main` — if any path `.pre-commit-hooks.yaml` + exposes changed since the last tag reachable from `HEAD`. It is a silent no-op on every other + branch: hard-failing on feature-branch pushes mid-review would force a premature tag on a + commit that might not survive a squash-merge, the exact problem `repo: local` (above) already + avoids for this repo's own dev-time gate. A tag not existing at all is also a hard fail on + `main`, covering the very first release. This is deterministic tooling, not a standing + instruction to remember — consistent with `check-manifests.sh`/`check-vale-style-sync.sh` + already using the same pre-push, main-agnostic-elsewhere pattern. +- **Known limitation, not yet closed:** `check-release-needed.sh` only fires when a human runs + `git push` locally with pre-commit's hooks installed — `PRE_COMMIT_REMOTE_BRANCH` is set by + pre-commit's client-side `hook-impl` script parsing `git push`'s stdin protocol. A PR merged + through Gitea's merge button (server-side, no local push) or a CI runner invoking + `pre-commit run --hook-stage pre-push` directly never sets it, so the gate silently doesn't run + in either path. This repo has no CI workflow yet (`has_actions` is enabled but unused), so + closing this gap needs a server-side job re-running the same script on merge to `main` — deferred + as a separate piece of infrastructure, not fixed here. `RELEASE_PATHS` is derived from + `.pre-commit-hooks.yaml`'s own `entry:` lines rather than hand-maintained, so at least the set of + paths it checks can't drift from the manifest on its own. +- **Dropping `--config` moved the release gate's path derivation too.** `check-release-needed.sh` + used to reach each hook's bundled assets through the `dirname` of its `--config` target. With + no `--config` token left, that loop went dead and silently dropped both `assets/vale/` trees + from release coverage — a Vale *rule* change could then land on `main` without demanding a tag, + leaving consumers pinned to an old `rev:` running stale rules while the gate stayed green. The + 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 (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. diff --git a/plugins/bin/.claude-plugin/plugin.json b/plugins/bin/.claude-plugin/plugin.json index 490b0ca..e2808ac 100644 --- a/plugins/bin/.claude-plugin/plugin.json +++ b/plugins/bin/.claude-plugin/plugin.json @@ -8,5 +8,5 @@ "keywords": [], "license": "MIT", "name": "bin", - "version": "1.1.0" + "version": "1.1.1" } diff --git a/plugins/bin/plugin.json b/plugins/bin/plugin.json index b0be24c..e02f8dd 100644 --- a/plugins/bin/plugin.json +++ b/plugins/bin/plugin.json @@ -11,5 +11,5 @@ "skills": [ "skills/" ], - "version": "1.1.0" + "version": "1.1.1" } diff --git a/plugins/bin/skills/prototype/SKILL.md b/plugins/bin/skills/prototype/SKILL.md index 8bd99d8..84478e0 100644 --- a/plugins/bin/skills/prototype/SKILL.md +++ b/plugins/bin/skills/prototype/SKILL.md @@ -14,7 +14,7 @@ Identify which question is being answered — from the user's prompt, the surrou - **"Does this logic / state model feel right?"** → [LOGIC.md](LOGIC.md). Build a tiny interactive terminal app that pushes the state machine through cases that are hard to reason about on paper. - **"What should this look like?"** → [UI.md](UI.md). Generate several radically different UI variations on a single route, switchable via a URL search param and a floating bottom bar. -The two branches produce very different artifacts — getting this wrong wastes the whole prototype. If the question is genuinely ambiguous and the user isn't reachable, default to whichever branch better matches the surrounding code (a backend module → logic; a page or component → UI) and state the assumption at the top of the prototype. +The two branches produce fundamentally different artifacts — getting this wrong wastes the whole prototype. If the question is genuinely ambiguous and the user isn't reachable, default to whichever branch better matches the surrounding code (a backend module → logic; a page or component → UI) and state the assumption at the top of the prototype. ## Rules that apply to both diff --git a/plugins/gitea/.claude-plugin/plugin.json b/plugins/gitea/.claude-plugin/plugin.json index 15a1214..2fc7c81 100644 --- a/plugins/gitea/.claude-plugin/plugin.json +++ b/plugins/gitea/.claude-plugin/plugin.json @@ -15,5 +15,5 @@ ], "license": "MIT", "name": "gitea", - "version": "1.3.2" + "version": "1.3.3" } diff --git a/plugins/gitea/plugin.json b/plugins/gitea/plugin.json index 9601cec..ccef22b 100644 --- a/plugins/gitea/plugin.json +++ b/plugins/gitea/plugin.json @@ -20,5 +20,5 @@ "skills": [ "skills/" ], - "version": "1.3.2" + "version": "1.3.3" } diff --git a/plugins/gitea/skills/gitea-issues/SKILL.md b/plugins/gitea/skills/gitea-issues/SKILL.md index 7dfaab5..8cfc6fe 100644 --- a/plugins/gitea/skills/gitea-issues/SKILL.md +++ b/plugins/gitea/skills/gitea-issues/SKILL.md @@ -31,7 +31,7 @@ allowed-tools: Bash mcp__gitea__list_issues mcp__gitea__issue_read mcp__gitea__i ## Gotchas -- **`list_issues` has no `type` or `milestones` parameter — despite `api-reference.md` documenting both.** The live MCP schema (re-verified via `ToolSearch` at authoring time — see `references/sources.md`) only accepts `owner`, `repo` (required), `state` (default `"all"`), `labels` (array of label *names*), `since`, `before` (ISO 8601), `page`, `per_page` (default 30). There is no way to filter issues-vs-PRs or by milestone through this tool. Since issues and PRs share one number space, `list_issues` results can include PR entries with no client-side filter to exclude them. If you need to know whether a specific number is a PR, call `issue_read method: "get"` and check `is_pull` — that field only appears on the single-item response, never in a list item. This exact drift (a prior skill trusted the research doc's `type` param and broke) is why this skill's reference files were re-verified live rather than copied from `api-reference.md`. +- **`list_issues` has no `type` or `milestones` parameter — despite `api-reference.md` documenting both.** The live MCP schema (re-verified via `ToolSearch` at authoring time — see `references/sources.md`) only accepts `owner`, `repo` (required), `state` (default `"all"`), `labels` (array of label *names*), `since`, `before` (ISO 8601), `page`, `per_page` (default 30). This tool provides no way to filter issues-vs-PRs or by milestone. Since issues and PRs share one number space, `list_issues` results can include PR entries with no client-side filter to exclude them. If you need to know whether a specific number is a PR, call `issue_read method: "get"` and check `is_pull` — that field only appears on the single-item response, never in a list item. This exact drift (a prior skill trusted the research doc's `type` param and broke) is why this skill's reference files were re-verified live rather than copied from `api-reference.md`. - **`search_issues` does have a working `type` filter** (`"issues"` | `"pulls"`) — unlike `list_issues`. Its `labels` parameter is also shaped differently: a comma-separated string, not an array of names. - **Labels are numeric IDs on write, name strings on read.** `issue_write`'s `labels` parameter (used by `add_labels`/`replace_labels`) takes IDs. `list_issues`/`issue_read` return names. Never resolve this yourself — compose `gitea-labels-milestones` (see `references/enrichments.md`) to get IDs. - **Milestone on `issue_read` is `{id, title}`** — an object, not a bare string. This skill only ever needs the `id`. (The bare-title-string case only happens on the PR side, which is `gitea-prs`' problem, not this skill's.) @@ -86,7 +86,7 @@ Call `issue_read method: "get_comments" owner: repo: issue_number ### close `` -Call `issue_write method: "update" owner: repo: issue_number: state: "closed"`. There is no `method: "close"`. +Call `issue_write method: "update" owner: repo: issue_number: state: "closed"`. No `method: "close"` exists. ### comment `` diff --git a/plugins/gitea/skills/gitea-prs/SKILL.md b/plugins/gitea/skills/gitea-prs/SKILL.md index ee2c449..c0c2545 100644 --- a/plugins/gitea/skills/gitea-prs/SKILL.md +++ b/plugins/gitea/skills/gitea-prs/SKILL.md @@ -29,7 +29,7 @@ allowed-tools: mcp__gitea__list_pull_requests mcp__gitea__pull_request_read mcp_ - **Issues and PRs share one number space.** A number the user mentions (`#42`) might be an issue, not a PR — there is only one counter per repo. If you're not certain, call `pull_request_read method: "get"` and treat a 404 as "this number is an issue, not a PR" (or check `is_pull` on an `issue_read` response first if you already have one). - **`pull_request_read method: "get"` returns `review_scomments`, not `review_comments`.** Source-level typo in gitea-mcp v1.3.0. Never reference `review_comments` — it will always be undefined. -- **`draft: true` on create prepends `"WIP:"` to the title.** There is no first-class draft field — Gitea implements draft PRs via title prefix. To un-draft, call `update` and pass the title without the `WIP:` prefix. +- **`draft: true` on create prepends `"WIP:"` to the title.** Gitea has no first-class draft field — it implements draft PRs via title prefix. To un-draft, call `update` and pass the title without the `WIP:` prefix. - **Cross-repo fork PRs require `head` as `"fork-owner:branch-name"`.** A bare branch name causes Gitea to search the base repo for it and return 422. Same-repo PRs use a bare branch name. - **PR `milestone` is a bare title string, not `{id, title}`.** Unlike issues, you cannot recover a milestone's ID from a PR response. If you need the ID (e.g. to filter or to pass to another write), call into `gitea-labels-milestones` and match by title via `milestone_read method: "list"`. - **CI status and review/approval state are independent merge gates.** `get_status` only reports CI. Branch-protection rules (required approvals, requested-reviewer coverage, stale-approval handling) are enforced server-side by the merge call itself and will error if unmet — passing CI does not mean the merge will succeed. diff --git a/plugins/gitea/skills/gitea-releases/SKILL.md b/plugins/gitea/skills/gitea-releases/SKILL.md index cee7096..7bcc8e9 100644 --- a/plugins/gitea/skills/gitea-releases/SKILL.md +++ b/plugins/gitea/skills/gitea-releases/SKILL.md @@ -22,7 +22,7 @@ metadata: - **`delete_release` takes a numeric `id`, never a tag name.** `delete_tag` is the mirror opposite — it takes the `tag_name` string, never a numeric id. These two tools are asymmetric on purpose; passing a tag name to `delete_release` or a numeric id to `delete_tag` fails. Always resolve the numeric release id via `list_releases` or `get_release` first if you only have a tag name in hand. - **Deleting a release does not delete its tag.** They are separate destructive operations against separate resources — a release is a wrapper (title, notes, draft/prerelease flags, assets) around a tag, not the tag itself. If the intent is to remove both, call `delete_release` and `delete_tag` separately. -- **`list_releases`/`list_tags` default to `per_page: 20`**, unlike most other gitea-mcp tools which default to 30. There is no auto-pagination in the MCP layer — to get a complete result set, loop `page` upward until a page returns fewer than `per_page` results. +- **`list_releases`/`list_tags` default to `per_page: 20`**, unlike most other gitea-mcp tools which default to 30. The MCP layer does no auto-pagination — to get a complete result set, loop `page` upward until a page returns fewer than `per_page` results. - **`is_draft`/`is_pre_release` are explicit booleans the caller sets on `create_release` — never inferred from `tag_name`.** Note the input param is `is_draft`, which maps to the `draft` field on the *response* object (see Dispatch table below and `references/call-signatures.md`) — `draft` is never a valid input key. Practitioner convention (per the `tea` CLI) uses `-beta`/`-rc` suffixes for prereleases (e.g. `v2.0.0-beta.1`), but Gitea does not enforce or infer this from the tag string. If the user names a tag that looks like a prerelease, set `is_pre_release: true` explicitly rather than assuming the flag is redundant with the name. - **Tag names are conventionally semver, `v`-prefixed** (`v1.2.0`, `v2.0.0-beta.1`), but this is a practitioner convention, not a Gitea constraint — don't reject or rewrite a caller-supplied tag name that doesn't follow it. @@ -47,6 +47,6 @@ metadata: - [ ] **Creating a release:** Call `create_release` directly with `tag_name` + `target` + `title` — Gitea is assumed to create the underlying tag automatically if `tag_name` doesn't already exist (this is plausible behavior inferred from the API shape, not directly confirmed in the research docs), so a separate `create_tag` call is only needed when you want to tag a commit without wrapping it in a release yet. Verify the tag exists afterward if this matters to the caller. Set `is_pre_release`/`is_draft` explicitly per the Gotchas above; don't leave them to default inference. - [ ] **Deleting a release safely:** Resolve the numeric id first — call `list_releases` (paginate if needed, see Gotchas) or `get_release` if the id is already known, find the entry matching the target `tag_name`, then call `delete_release` with that `id`. Never pass `tag_name` to `delete_release`. - [ ] **Deleting a tag along with its release:** Delete the release first (frees the id lookup), then call `delete_tag` with the `tag_name` separately — confirm both are intended before proceeding, since each is an independent irreversible operation. -- [ ] **Listing completely:** If the caller needs all releases or tags (not just the first page), loop `page: 1, 2, 3...` until a response has fewer than `per_page` entries. +- [ ] **Listing every page:** If the caller needs all releases or tags (not just the first page), loop `page: 1, 2, 3...` until a response has fewer than `per_page` entries. If exact response field shapes or additional conventions are needed, read `references/call-signatures.md` and `references/conventions.md`. diff --git a/plugins/kyberforge/.claude-plugin/plugin.json b/plugins/kyberforge/.claude-plugin/plugin.json index 8a5badf..98049c5 100644 --- a/plugins/kyberforge/.claude-plugin/plugin.json +++ b/plugins/kyberforge/.claude-plugin/plugin.json @@ -8,5 +8,5 @@ "keywords": [], "license": "MIT", "name": "kyberforge", - "version": "1.2.3" + "version": "1.2.8" } diff --git a/plugins/kyberforge/plugin.json b/plugins/kyberforge/plugin.json index f540bc0..1554ed0 100644 --- a/plugins/kyberforge/plugin.json +++ b/plugins/kyberforge/plugin.json @@ -13,5 +13,5 @@ "skills": [ "skills/" ], - "version": "1.2.3" + "version": "1.2.8" } diff --git a/plugins/kyberforge/skills/agent-audit/README.md b/plugins/kyberforge/skills/agent-audit/README.md index 9476dec..2053c91 100644 --- a/plugins/kyberforge/skills/agent-audit/README.md +++ b/plugins/kyberforge/skills/agent-audit/README.md @@ -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 | diff --git a/plugins/kyberforge/skills/agent-audit/SKILL.md b/plugins/kyberforge/skills/agent-audit/SKILL.md index 894a823..08cc88d 100644 --- a/plugins/kyberforge/skills/agent-audit/SKILL.md +++ b/plugins/kyberforge/skills/agent-audit/SKILL.md @@ -36,10 +36,13 @@ metadata: ```bash bash scripts/validate.sh bash scripts/validate-provenance.sh +scripts/vale-wrap.sh ``` 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` 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. If the scripts cannot run (Bash denied, python3 unavailable), perform checks manually: counterpart file exists, required fields present (`name`, `description`, non-empty body), `name` is kebab-case, Copilot CLI `.agent.md` `name` must match filename stem (CC files are exempt — the CC platform does not require name to match filename), no `FILL IN:` placeholders, no CC-only fields in Copilot file, no Copilot-only fields in CC file (read `references/field-inventory.md` for the authoritative field lists). @@ -49,15 +52,17 @@ If the scripts cannot run (Bash denied, python3 unavailable), perform checks man Read both agent files. Work through each dimension internally. Collect findings only; report in Step 3. **Description (both files):** -- Action-verb opening: description starts with a verb ("Reviews...", "Analyzes...", "Generates...") — FAIL if absent -- Specificity: is the trigger condition stated precisely? — SUGGESTION if vague -- `Use proactively` in a Copilot description: CC-specific phrasing, has no effect in Copilot — SUGGESTION to remove +- Action-verb opening: description starts with a verb ("Reviews...", "Analyzes...", "Generates...") — FAIL if absent. Vale's `Kyberforge.DescriptionOpener` alert flags the specific known-bad "This agent..." opener directly; verifying an arbitrary opening word is genuinely a strong verb still requires judgment. +- Specificity: is the trigger condition stated precisely? — SUGGESTION if vague. Vale's `Kyberforge.VagueWording` alert covers known filler ("helps with", "utilize", ...) directly; report those as FAILs without re-deriving by judgment. +- `Use proactively` in a Copilot description: Vale's `KyberforgeCopilot.ProactivePhrase` alert (Copilot file only) flags this directly — report it without re-deriving by judgment. If a description finding is borderline, read `references/description-quality.md`. **Body:** - Direct role instruction: system prompt opens with `You are a [role]. When invoked, [action].` — SUGGESTION if absent - One job per agent: system prompt describes a single bounded task — SUGGESTION if scope appears unbounded +- Generic, non-specific reference pointers to the `references/` directory: Vale's `Kyberforge.PaddingPhrase` alert flags this directly — report it without re-deriving by judgment +- Sentences that open with "There is"/"There are": Vale's `Kyberforge.SentenceOpenerThereIs` alert flags this directly — report it without re-deriving by judgment **Body/Frontmatter comments:** - Inspect each comment block in the YAML frontmatter. For each comment, apply: *"Would the agent get this wrong without this comment?"* Flag any that answer "no" as padding. diff --git a/plugins/kyberforge/skills/agent-audit/assets/vale/.vale.ini b/plugins/kyberforge/skills/agent-audit/assets/vale/.vale.ini new file mode 100644 index 0000000..a93c7c8 --- /dev/null +++ b/plugins/kyberforge/skills/agent-audit/assets/vale/.vale.ini @@ -0,0 +1,7 @@ +StylesPath = styles + +[**/agents/*.md] +BasedOnStyles = Kyberforge + +[**/*.agent.md] +BasedOnStyles = Kyberforge, KyberforgeCopilot diff --git a/plugins/kyberforge/skills/agent-audit/assets/vale/styles/Kyberforge/DescriptionOpener.yml b/plugins/kyberforge/skills/agent-audit/assets/vale/styles/Kyberforge/DescriptionOpener.yml new file mode 100644 index 0000000..d978d73 --- /dev/null +++ b/plugins/kyberforge/skills/agent-audit/assets/vale/styles/Kyberforge/DescriptionOpener.yml @@ -0,0 +1,7 @@ +extends: existence +message: "Description opens with '%s' — use an imperative 'Use when...' opener instead" +level: error +scope: text.frontmatter.description +ignorecase: true +raw: + - '^This (skill|agent)\b' diff --git a/plugins/kyberforge/skills/agent-audit/assets/vale/styles/Kyberforge/PaddingPhrase.yml b/plugins/kyberforge/skills/agent-audit/assets/vale/styles/Kyberforge/PaddingPhrase.yml new file mode 100644 index 0000000..4c5f5ae --- /dev/null +++ b/plugins/kyberforge/skills/agent-audit/assets/vale/styles/Kyberforge/PaddingPhrase.yml @@ -0,0 +1,7 @@ +extends: existence +message: "Generic reference pointer: '%s' — use the specific 'If X, read `references/file.md`' form instead" +level: error +scope: text +ignorecase: true +raw: + - 'see references?/? for (more )?(info|information|details)\b' diff --git a/plugins/kyberforge/skills/agent-audit/assets/vale/styles/Kyberforge/SentenceOpenerThereIs.yml b/plugins/kyberforge/skills/agent-audit/assets/vale/styles/Kyberforge/SentenceOpenerThereIs.yml new file mode 100644 index 0000000..c443bad --- /dev/null +++ b/plugins/kyberforge/skills/agent-audit/assets/vale/styles/Kyberforge/SentenceOpenerThereIs.yml @@ -0,0 +1,7 @@ +extends: existence +message: "Don't start a sentence with '%s' — name the subject directly" +level: error +scope: sentence +ignorecase: false +raw: + - '^There\s(is|are)\b' diff --git a/plugins/kyberforge/skills/agent-audit/assets/vale/styles/Kyberforge/VagueWording.yml b/plugins/kyberforge/skills/agent-audit/assets/vale/styles/Kyberforge/VagueWording.yml new file mode 100644 index 0000000..45cb87f --- /dev/null +++ b/plugins/kyberforge/skills/agent-audit/assets/vale/styles/Kyberforge/VagueWording.yml @@ -0,0 +1,10 @@ +extends: existence +message: "Vague capability wording: '%s' — state the capability precisely instead" +level: error +scope: text.frontmatter.description +ignorecase: true +tokens: + - helps with + - utilize + - assists with + - used for diff --git a/plugins/kyberforge/skills/agent-audit/assets/vale/styles/KyberforgeCopilot/ProactivePhrase.yml b/plugins/kyberforge/skills/agent-audit/assets/vale/styles/KyberforgeCopilot/ProactivePhrase.yml new file mode 100644 index 0000000..a057bb0 --- /dev/null +++ b/plugins/kyberforge/skills/agent-audit/assets/vale/styles/KyberforgeCopilot/ProactivePhrase.yml @@ -0,0 +1,7 @@ +extends: existence +message: "'%s' is CC-specific phrasing with no effect in Copilot descriptions — remove it" +level: error +scope: text.frontmatter.description +ignorecase: true +tokens: + - Use proactively diff --git a/plugins/kyberforge/skills/agent-audit/scripts/vale-wrap.sh b/plugins/kyberforge/skills/agent-audit/scripts/vale-wrap.sh new file mode 100755 index 0000000..862f44c --- /dev/null +++ b/plugins/kyberforge/skills/agent-audit/scripts/vale-wrap.sh @@ -0,0 +1,526 @@ +#!/usr/bin/env bash +set -euo pipefail + +# Works around a Vale limitation: the `text.frontmatter.description` NLP scope +# silently stops matching once the `description:` value 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 — 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.) +# +# 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 +# verbatim and resolves to its flattened copy, keeping the report byte-identical +# to bare `vale`'s. An absolute path inside the cwd is relativized to keep that +# property. Only an absolute path outside the cwd is rewritten to its scratch +# copy and so reports a scratch path — unavoidable, since a file can only be +# read from where it actually is. + +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=() +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 [[ -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") + pending_flag="$arg" + config_given=true + continue + ;; + --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 + 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 + +if [[ "$config_given" == false ]]; then + vale_args+=(--config "$(cd "$(dirname "${BASH_SOURCE[0]}")/../assets/vale" && pwd)/.vale.ini") +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[@]+"${vale_args[@]}"} < /dev/null +fi + +# `realpath -m` would be the obvious normalizer, but `-m` (canonicalize-missing) +# is a GNU extension the BSD realpath on macOS doesn't have — and every dest +# below is a path that doesn't exist yet. python3 is already a hard dependency. +abspath() { + python3 -c 'import os, sys; print(os.path.abspath(sys.argv[1]))' "$1" +} + +flatten() { + # 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_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). `\` 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) + 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 + 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 +} + +tmpdir="$(cd "$(mktemp -d)" && pwd -P)" +trap 'rm -rf "$tmpdir"' EXIT + +# Mirror of the caller's cwd inside the scratch tree; relative path arguments +# are resolved from here. +mirror="$tmpdir$cwd" +mkdir -p "$mirror" + +argv_paths=() +for arg in ${path_args[@]+"${path_args[@]}"}; do + if [[ "$arg" == /* ]]; then + raw_dest="$tmpdir$arg" + else + raw_dest="$mirror/$arg" + fi + 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 -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 + # `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") + else + argv_paths+=("$arg") + fi +done + +cd "$mirror" +vale ${vale_args[@]+"${vale_args[@]}"} ${argv_paths[@]+"${argv_paths[@]}"} diff --git a/plugins/kyberforge/skills/agent-author/SKILL.md b/plugins/kyberforge/skills/agent-author/SKILL.md index 936e6b7..96045f1 100644 --- a/plugins/kyberforge/skills/agent-author/SKILL.md +++ b/plugins/kyberforge/skills/agent-author/SKILL.md @@ -123,7 +123,7 @@ source_keys: ### Step 3 — Fill in the Copilot agent file -There are **two distinct Copilot agent formats** with different paths and field sets. Choose one based on the deployment target: +**Two distinct Copilot agent formats** exist, with different paths and field sets. Choose one based on the deployment target: **CLI format** (default — what the scaffold creates): - Path: `.github/agents/.agent.md` (project) or `/agents/.agent.md` (plugin) diff --git a/plugins/kyberforge/skills/skill-audit/README.md b/plugins/kyberforge/skills/skill-audit/README.md index 83d0876..95b714e 100644 --- a/plugins/kyberforge/skills/skill-audit/README.md +++ b/plugins/kyberforge/skills/skill-audit/README.md @@ -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 | diff --git a/plugins/kyberforge/skills/skill-audit/SKILL.md b/plugins/kyberforge/skills/skill-audit/SKILL.md index eac4a0f..474c784 100644 --- a/plugins/kyberforge/skills/skill-audit/SKILL.md +++ b/plugins/kyberforge/skills/skill-audit/SKILL.md @@ -34,12 +34,15 @@ metadata: ```bash bash scripts/validate.sh bash scripts/validate-provenance.sh +scripts/vale-wrap.sh /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` 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 Read every file in the skill directory: `SKILL.md`, `README.md` (if present), all files in `scripts/`, `references/`, `assets/`, and `tests/`. Skip binary files only. Do not skip text files — internal consistency checks require the full picture. @@ -50,8 +53,10 @@ Work through each dimension internally. Collect findings only; report them in St ### Description -- **Imperative phrasing**: does it use "Use when..." not "This skill..."? -- **Specificity**: are capabilities stated precisely ("parses OpenAPI specs") or vaguely ("helps with APIs")? +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? - **Length**: under 1024 characters? @@ -66,6 +71,8 @@ For each sentence in the body, apply: *"Would the agent get this wrong without t - **Why rationale**: include/exclude rules explain why, not just what - **Control calibration**: prescriptive for fragile or critical sequences (e.g. a script invocation where flag order or exact arguments must not change); flexible where multiple approaches are valid +Vale's `Kyberforge.SentenceOpenerThereIs` alert from Step 1 (FAIL — sentences starting with "There is"/"There are") covers pattern-matchable body-wide filler directly; report it as a finding without re-deriving by judgment. + If uncertain whether a sentence is padding or whether a control decision is correctly calibrated, read `references/body-discipline.md`. ### Patterns @@ -75,7 +82,7 @@ Check each pattern is appropriate and correctly formed: - **Gotchas**: placed near the top; each entry is a specific fact that defies a reasonable assumption — not a general tip - **Prescriptive sequence**: inner code fences escaped as `\`\`\`` when nested inside a markdown block - **Checklists**: used for multi-step workflows, not single steps -- **Conditional references**: specific trigger stated ("If X, read `references/file.md`") — not a generic "see references/" +- **Conditional references**: specific trigger stated ("If X, read `references/file.md`") — not a generic "see references/". Vale's `Kyberforge.PaddingPhrase` alert from Step 1 flags the generic phrasing directly; other malformed conditional-reference forms still require judgment. - **Output templates**: present when the agent must produce a specific format; absent otherwise ### File structure diff --git a/plugins/kyberforge/skills/skill-audit/assets/vale/.vale.ini b/plugins/kyberforge/skills/skill-audit/assets/vale/.vale.ini new file mode 100644 index 0000000..b7ce2e5 --- /dev/null +++ b/plugins/kyberforge/skills/skill-audit/assets/vale/.vale.ini @@ -0,0 +1,4 @@ +StylesPath = styles + +[**/SKILL.md] +BasedOnStyles = Kyberforge diff --git a/plugins/kyberforge/skills/skill-audit/assets/vale/styles/Kyberforge/DescriptionOpener.yml b/plugins/kyberforge/skills/skill-audit/assets/vale/styles/Kyberforge/DescriptionOpener.yml new file mode 100644 index 0000000..d978d73 --- /dev/null +++ b/plugins/kyberforge/skills/skill-audit/assets/vale/styles/Kyberforge/DescriptionOpener.yml @@ -0,0 +1,7 @@ +extends: existence +message: "Description opens with '%s' — use an imperative 'Use when...' opener instead" +level: error +scope: text.frontmatter.description +ignorecase: true +raw: + - '^This (skill|agent)\b' diff --git a/plugins/kyberforge/skills/skill-audit/assets/vale/styles/Kyberforge/PaddingPhrase.yml b/plugins/kyberforge/skills/skill-audit/assets/vale/styles/Kyberforge/PaddingPhrase.yml new file mode 100644 index 0000000..4c5f5ae --- /dev/null +++ b/plugins/kyberforge/skills/skill-audit/assets/vale/styles/Kyberforge/PaddingPhrase.yml @@ -0,0 +1,7 @@ +extends: existence +message: "Generic reference pointer: '%s' — use the specific 'If X, read `references/file.md`' form instead" +level: error +scope: text +ignorecase: true +raw: + - 'see references?/? for (more )?(info|information|details)\b' diff --git a/plugins/kyberforge/skills/skill-audit/assets/vale/styles/Kyberforge/SentenceOpenerThereIs.yml b/plugins/kyberforge/skills/skill-audit/assets/vale/styles/Kyberforge/SentenceOpenerThereIs.yml new file mode 100644 index 0000000..c443bad --- /dev/null +++ b/plugins/kyberforge/skills/skill-audit/assets/vale/styles/Kyberforge/SentenceOpenerThereIs.yml @@ -0,0 +1,7 @@ +extends: existence +message: "Don't start a sentence with '%s' — name the subject directly" +level: error +scope: sentence +ignorecase: false +raw: + - '^There\s(is|are)\b' diff --git a/plugins/kyberforge/skills/skill-audit/assets/vale/styles/Kyberforge/VagueWording.yml b/plugins/kyberforge/skills/skill-audit/assets/vale/styles/Kyberforge/VagueWording.yml new file mode 100644 index 0000000..45cb87f --- /dev/null +++ b/plugins/kyberforge/skills/skill-audit/assets/vale/styles/Kyberforge/VagueWording.yml @@ -0,0 +1,10 @@ +extends: existence +message: "Vague capability wording: '%s' — state the capability precisely instead" +level: error +scope: text.frontmatter.description +ignorecase: true +tokens: + - helps with + - utilize + - assists with + - used for diff --git a/plugins/kyberforge/skills/skill-audit/scripts/vale-wrap.sh b/plugins/kyberforge/skills/skill-audit/scripts/vale-wrap.sh new file mode 100755 index 0000000..862f44c --- /dev/null +++ b/plugins/kyberforge/skills/skill-audit/scripts/vale-wrap.sh @@ -0,0 +1,526 @@ +#!/usr/bin/env bash +set -euo pipefail + +# Works around a Vale limitation: the `text.frontmatter.description` NLP scope +# silently stops matching once the `description:` value 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 — 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.) +# +# 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 +# verbatim and resolves to its flattened copy, keeping the report byte-identical +# to bare `vale`'s. An absolute path inside the cwd is relativized to keep that +# property. Only an absolute path outside the cwd is rewritten to its scratch +# copy and so reports a scratch path — unavoidable, since a file can only be +# read from where it actually is. + +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=() +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 [[ -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") + pending_flag="$arg" + config_given=true + continue + ;; + --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 + 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 + +if [[ "$config_given" == false ]]; then + vale_args+=(--config "$(cd "$(dirname "${BASH_SOURCE[0]}")/../assets/vale" && pwd)/.vale.ini") +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[@]+"${vale_args[@]}"} < /dev/null +fi + +# `realpath -m` would be the obvious normalizer, but `-m` (canonicalize-missing) +# is a GNU extension the BSD realpath on macOS doesn't have — and every dest +# below is a path that doesn't exist yet. python3 is already a hard dependency. +abspath() { + python3 -c 'import os, sys; print(os.path.abspath(sys.argv[1]))' "$1" +} + +flatten() { + # 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_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). `\` 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) + 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 + 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 +} + +tmpdir="$(cd "$(mktemp -d)" && pwd -P)" +trap 'rm -rf "$tmpdir"' EXIT + +# Mirror of the caller's cwd inside the scratch tree; relative path arguments +# are resolved from here. +mirror="$tmpdir$cwd" +mkdir -p "$mirror" + +argv_paths=() +for arg in ${path_args[@]+"${path_args[@]}"}; do + if [[ "$arg" == /* ]]; then + raw_dest="$tmpdir$arg" + else + raw_dest="$mirror/$arg" + fi + 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 -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 + # `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") + else + argv_paths+=("$arg") + fi +done + +cd "$mirror" +vale ${vale_args[@]+"${vale_args[@]}"} ${argv_paths[@]+"${argv_paths[@]}"} diff --git a/plugins/kyberforge/skills/skill-audit/scripts/validate.sh b/plugins/kyberforge/skills/skill-audit/scripts/validate.sh index 5203bad..0c0473e 100755 --- a/plugins/kyberforge/skills/skill-audit/scripts/validate.sh +++ b/plugins/kyberforge/skills/skill-audit/scripts/validate.sh @@ -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:] diff --git a/plugins/lint/.claude-plugin/plugin.json b/plugins/lint/.claude-plugin/plugin.json new file mode 100644 index 0000000..76e11d9 --- /dev/null +++ b/plugins/lint/.claude-plugin/plugin.json @@ -0,0 +1,17 @@ +{ + "author": { + "name": "Defame1297", + "url": "https://git.dev.rkdr.net/Defame1297/" + }, + "description": "Skills and agents for configuring and running linters.", + "displayName": "Lint", + "keywords": [ + "lint", + "style", + "prose", + "linter" + ], + "license": "MIT", + "name": "lint", + "version": "1.1.5" +} diff --git a/plugins/lint/.mcp.json b/plugins/lint/.mcp.json new file mode 100644 index 0000000..da39e4f --- /dev/null +++ b/plugins/lint/.mcp.json @@ -0,0 +1,3 @@ +{ + "mcpServers": {} +} diff --git a/plugins/lint/agents/lint-runner.agent.md b/plugins/lint/agents/lint-runner.agent.md new file mode 100644 index 0000000..073cc3c --- /dev/null +++ b/plugins/lint/agents/lint-runner.agent.md @@ -0,0 +1,41 @@ +--- +name: lint-runner + +description: Runs a linter sweep over a target file or directory scope and reports findings. Currently backs onto Vale (prose/style linting) via the vale-config and vale-run skills; built to add other linters later without changing its own contract. Use when a caller needs a lint pass run in an isolated context and wants findings back, not fixes applied. + +tools: ["execute", "read", "search"] + +source_keys: + - context7-websites-vale-sh + +--- + +You are a linter runner. When invoked, you run the appropriate linter(s) over the requested scope, collect their findings, and report them back in a structured, reviewable form. You never edit files. + +## Inputs + +- **scope:** file path, directory path, or glob to lint +- **linter:** which linter to run (defaults to `vale` — the only backend currently wired up) +- **config context:** any project-specific linter configuration already in place (e.g. an existing `.vale.ini` for Vale, or whatever config format the requested linter expects); if none exists, say so in your report rather than inventing one + +## Process + +1. Determine whether the target scope already has configuration in place for the requested `linter` (whatever config format that linter expects). If not, use the `-config` skill (e.g. `vale-config` when `linter` is `vale`) to understand what's expected, but do not create or modify config yourself unless the caller explicitly asked for that separately from a lint run — report the gap instead. +2. Use the `-run` skill (e.g. `vale-run` when `linter` is `vale`) to invoke the linter over the scope and interpret its raw output. +3. Normalize findings into one shape regardless of backend linter: file, line, rule/check, severity, message. +4. Do not edit, fix, or rewrite any flagged content. If a finding looks trivially fixable, note that in the report — do not act on it. +5. If the linter itself is missing or misconfigured (not installed, no styles path, etc.), or if no `-config`/`-run` skill pair exists for the requested linter, report that as a blocking finding rather than attempting to install, configure, or substitute a fallback silently. + +## Output + +Report findings as a flat list, most-severe first: + +``` +- file: + line: + rule: + severity: + message: +``` + +Follow with a one-line summary: total findings by severity, and whether the run was blocked (e.g. linter not configured). If there are zero findings, say so explicitly — do not omit the report. diff --git a/plugins/lint/agents/lint-runner.md b/plugins/lint/agents/lint-runner.md new file mode 100644 index 0000000..c2950d9 --- /dev/null +++ b/plugins/lint/agents/lint-runner.md @@ -0,0 +1,41 @@ +--- +name: lint-runner + +description: Runs a linter sweep over a target file or directory scope and reports findings. Currently backs onto Vale (prose/style linting) via the vale-config and vale-run skills; built to add other linters later without changing its own contract. Use when a caller needs a lint pass run in an isolated context and wants findings back, not fixes applied. + +tools: Bash, Read, Grep, Glob + +source_keys: + - context7-websites-vale-sh + +--- + +You are a linter runner. When invoked, you run the appropriate linter(s) over the requested scope, collect their findings, and report them back in a structured, reviewable form. You never edit files. + +## Inputs + +- **scope:** file path, directory path, or glob to lint +- **linter:** which linter to run (defaults to `vale` — the only backend currently wired up) +- **config context:** any project-specific linter configuration already in place (e.g. an existing `.vale.ini` for Vale, or whatever config format the requested linter expects); if none exists, say so in your report rather than inventing one + +## Process + +1. Determine whether the target scope already has configuration in place for the requested `linter` (whatever config format that linter expects). If not, use the `-config` skill (e.g. `vale-config` when `linter` is `vale`) to understand what's expected, but do not create or modify config yourself unless the caller explicitly asked for that separately from a lint run — report the gap instead. +2. Use the `-run` skill (e.g. `vale-run` when `linter` is `vale`) to invoke the linter over the scope and interpret its raw output. +3. Normalize findings into one shape regardless of backend linter: file, line, rule/check, severity, message. +4. Do not edit, fix, or rewrite any flagged content. If a finding looks trivially fixable, note that in the report — do not act on it. +5. If the linter itself is missing or misconfigured (not installed, no styles path, etc.), or if no `-config`/`-run` skill pair exists for the requested linter, report that as a blocking finding rather than attempting to install, configure, or substitute a fallback silently. + +## Output + +Report findings as a flat list, most-severe first: + +``` +- file: + line: + rule: + severity: + message: +``` + +Follow with a one-line summary: total findings by severity, and whether the run was blocked (e.g. linter not configured). If there are zero findings, say so explicitly — do not omit the report. diff --git a/plugins/lint/docs/README.md b/plugins/lint/docs/README.md new file mode 100644 index 0000000..371f066 --- /dev/null +++ b/plugins/lint/docs/README.md @@ -0,0 +1,27 @@ +# docs/ + +Plugin documentation. Not read automatically by Claude Code or GitHub Copilot CLI — reference specific files from skill bodies or agent prompts as needed. + +## research/ + +Upstream reference material gathered during skill authoring. Not shipped with the plugin — used at development time only. + +| Path | Purpose | +|------|---------| +| `research/docs/vale/` | Vale documentation (vale.sh), gathered while authoring `vale-config`/`vale-run` — see below | + +### research/docs/vale/ + +| File | Covers | +|------|--------| +| `overview.md` | What Vale is; the style/rule/check configuration model; the built-in `Vale` style's four rules; styles directory layout | +| `installation.md` | Installing Vale via OS package managers (Homebrew, Snap, Chocolatey) and Docker | +| `configuration.md` | `.vale.ini` structure — global, `[formats]`, and per-glob sections | +| `cli-reference.md` | Core invocation and key subcommands/flags (`vale sync`, `vale ls-config`, etc.) | +| `examples.md` | Walkthroughs — project initialization, typical `.vale.ini` configs | +| `troubleshooting.md` | Suppressing false positives via inline markup (format-specific `vale off`/`vale on` syntax, spelling ignore lists) | +| `sources.md` | Provenance record for this directory — source URL, description, and which files above were extracted from it | + +Each file's frontmatter carries a `source_keys` entry keyed to the same provenance record. The plugin-level provenance file, `plugins/lint/sources.md`, cross-references this directory from the `context7-websites-vale-sh` entry and lists the agents/skills whose content drew on it. + +`vale-config` and `vale-run` are the skills that consume this research — read the relevant file here before changing either skill's Vale-facing behavior. diff --git a/plugins/lint/docs/research/docs/vale/cli-reference.md b/plugins/lint/docs/research/docs/vale/cli-reference.md new file mode 100644 index 0000000..16b6fdb --- /dev/null +++ b/plugins/lint/docs/research/docs/vale/cli-reference.md @@ -0,0 +1,29 @@ +--- +topic: cli-reference +source_keys: + - context7-websites-vale-sh +--- + +## Core Invocation + +```bash +$ vale README.md +``` + +Lints the given file(s)/glob against the styles configured in `.vale.ini`. + +## Key Flags and Subcommands + +| Command/Flag | Purpose | +|---|---| +| `vale sync` | Downloads and installs packages/styles declared in `.vale.ini`. Run after install and whenever `Packages` changes. | +| `vale ls-config` | Prints the currently active, fully-resolved configuration as JSON. Useful for debugging what settings actually apply to a file. | +| `--output=