From 3324a73225233bc6f2bc9318559292fee4beb88f Mon Sep 17 00:00:00 2001 From: Defame1297 Date: Fri, 24 Jul 2026 20:04:10 +0000 Subject: [PATCH] feat(lint): expand Vale audit prefilter into a broader plugin-content harness Deferred item from PR #85 review. Per ADR-0013: cherry-picks two low-noise rules from trialing write-good/alex against the real corpus (VagueQualifier, SentenceOpenerThereIs) into styles/Kyberforge rather than adopting either package wholesale (both are tuned for blog prose and were noisy on this repo's terse, imperative instruction files - see the ADR's rejected-rule list). Adds a new skill-size-check pre-commit hook enforcing agentskills.io's 500-line/5,000-token SKILL.md ceiling, currently unenforced. Fixes the 28 resulting violations across 20 existing SKILL.md/agent files so the enforcing pre-commit hook lands clean. governance.md/CONTROLS.md were evaluated and excluded as rule sources - they're org/CI-infrastructure controls, not prose patterns Vale can express. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01QUDczvw1H3eEeMD29Q9Lbi --- .pre-commit-config.yaml | 9 ++ CONTEXT.md | 2 + ...013-vale-harness-scope-and-rule-sources.md | 84 +++++++++++++++++++ plugins/bin/skills/caveman/SKILL.md | 2 +- plugins/bin/skills/diagnose/SKILL.md | 4 +- .../improve-codebase-architecture/SKILL.md | 2 +- plugins/bin/skills/prototype/SKILL.md | 10 +-- plugins/bin/skills/tdd/SKILL.md | 2 +- plugins/bin/skills/write-docs/SKILL.md | 2 +- plugins/git/skills/git-branches/SKILL.md | 2 +- plugins/git/skills/pc-author/SKILL.md | 2 +- plugins/gitea/skills/gitea-issues/SKILL.md | 6 +- plugins/gitea/skills/gitea-prs/SKILL.md | 2 +- plugins/gitea/skills/gitea-releases/SKILL.md | 4 +- plugins/gitea/skills/gitea-workflow/SKILL.md | 4 +- .../kyberforge/skills/agent-author/SKILL.md | 2 +- plugins/kyberforge/skills/forge/SKILL.md | 2 +- .../skills/marketplace-author/SKILL.md | 2 +- .../kyberforge/skills/skill-author/SKILL.md | 2 +- scripts/skill-size-check.sh | 35 ++++++++ styles/Kyberforge/SentenceOpenerThereIs.yml | 7 ++ styles/Kyberforge/VagueQualifier.yml | 36 ++++++++ tests/test-skill-size-check.sh | 65 ++++++++++++++ 23 files changed, 263 insertions(+), 25 deletions(-) create mode 100644 docs/adr/0013-vale-harness-scope-and-rule-sources.md create mode 100755 scripts/skill-size-check.sh create mode 100644 styles/Kyberforge/SentenceOpenerThereIs.yml create mode 100644 styles/Kyberforge/VagueQualifier.yml create mode 100755 tests/test-skill-size-check.sh diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 586ef59..ad1a9d6 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -98,6 +98,15 @@ 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 stages: ['pre-commit'] name: Vale audit prefilter diff --git a/CONTEXT.md b/CONTEXT.md index 8e9bfe8..5b2e90b 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -74,5 +74,7 @@ Wiring Vale as a deterministic prefilter for `skill-audit`/`agent-audit`'s Descr Both skills' Step 1, and the `vale-audit-prefilter` pre-commit hook, call `scripts/vale-wrap.sh` rather than `vale` directly — a workaround for a confirmed Vale 3.15.2 limitation (see `vale-config`'s Gotchas): `text.frontmatter.description` silently stops matching once the description is a YAML block scalar (`>`/`|`) spanning 2+ physical lines, which is how most skills/agents in this repo write it. The wrapper flattens the description to one physical line in a scratch copy (padding with blank lines so every other line number is unchanged) before handing off to real `vale`; single-line descriptions pass through untouched. `tests/test-vale-wrap.sh` regression-tests this. Both call sites still scope every invocation to the specific file(s) being audited, never a repo-wide sweep — Vale's glob matching crosses directory boundaries (`plugins/*/agents/*.md` matches nested `docs/research/examples/**/agents/*.md` too), so scoping is what keeps research-example files out of the audit's lint pass. The pre-commit hook's own glob is tightened to `^plugins/[^/]+/(skills/[^/]+/SKILL\.md|agents/[^/]+\.md)$` (single-segment, not `.*`) for the same reason, since pre-commit invokes it automatically against whatever staged files match rather than a manually-scoped target. +This scope expands per ADR-0013: cherry-picked low-noise `write-good`/`alex` rules into `styles/Kyberforge` (still pending implementation) plus a new sibling pre-commit hook, `skill-size-check` (`scripts/skill-size-check.sh`), enforcing `skill-authoring.md`'s 500-line/5,000-token `SKILL.md` ceiling — scoped to `^plugins/[^/]+/skills/[^/]+/SKILL\.md$` only, same as `vale-audit-prefilter`, so it never lints `docs/research/examples/` reference skills. 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). + ### 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/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..c867ab5 --- /dev/null +++ b/docs/adr/0013-vale-harness-scope-and-rule-sources.md @@ -0,0 +1,84 @@ +# 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 (`plugins/*/agents/*.md`, +`plugins/*/agents/*.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). 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. + +## Consequences + +- `styles/Kyberforge/` will gain new rule files once the (separate, later) implementation pass + finalizes the exact cherry-picked `write-good`/`alex` rules and any new wording — none are named + by this ADR, since none have been chosen yet. +- A new pre-commit hook (name TBD by the implementer) enforces the 500-line/5,000-token `SKILL.md` + ceiling, sibling to `skill-frontmatter`. +- `styles/KyberforgeTrial/` and `.vale.trial.ini` are deliberately not created — noted here so a + future reader doesn't wonder if a trial tier was forgotten. +- Follow-up work — not part of this ADR — is: syncing and trialing `write-good`/`alex`, cherry-picking + low-noise rules, writing the size-ceiling script and hook, fixing existing corpus violations, and + the atomic commit landing all of it. diff --git a/plugins/bin/skills/caveman/SKILL.md b/plugins/bin/skills/caveman/SKILL.md index 85770a3..632e132 100644 --- a/plugins/bin/skills/caveman/SKILL.md +++ b/plugins/bin/skills/caveman/SKILL.md @@ -15,7 +15,7 @@ ACTIVE EVERY RESPONSE once triggered. No revert after many turns. No filler drif ## Rules -Drop: articles (a/an/the), filler (just/really/basically/actually/simply), pleasantries (sure/certainly/of course/happy to), hedging. Fragments OK. Short synonyms (big not extensive, fix not "implement a solution for"). Abbreviate common terms (DB/auth/config/req/res/fn/impl). Strip conjunctions. Use arrows for causality (X -> Y). One word when one word enough. +Drop: articles (a/an/the), filler (just/really/basically/actually/literally), pleasantries (sure/certainly/no worries/happy to), hedging. Fragments OK. Short synonyms (big not extensive, fix not "implement a solution for"). Abbreviate common terms (DB/auth/config/req/res/fn/impl). Strip conjunctions. Use arrows for causality (X -> Y). One word when one word enough. Technical terms stay exact. Code blocks unchanged. Errors quoted exact. diff --git a/plugins/bin/skills/diagnose/SKILL.md b/plugins/bin/skills/diagnose/SKILL.md index ed55bda..9f444d5 100644 --- a/plugins/bin/skills/diagnose/SKILL.md +++ b/plugins/bin/skills/diagnose/SKILL.md @@ -86,7 +86,7 @@ Tool preference: **Tag every debug log** with a unique prefix, e.g. `[DEBUG-a4f2]`. Cleanup at the end becomes a single grep. Untagged logs survive; tagged logs die. -**Perf branch.** For performance regressions, logs are usually wrong. Instead: establish a baseline measurement (timing harness, `performance.now()`, profiler, query plan), then bisect. Measure first, fix second. +**Perf branch.** For performance regressions, logs rarely reveal the cause. Instead: establish a baseline measurement (timing harness, `performance.now()`, profiler, query plan), then bisect. Measure first, fix second. ## Phase 5 — Fix + regression test @@ -111,7 +111,7 @@ Required before declaring done: - [ ] Original repro no longer reproduces (re-run the Phase 1 loop) - [ ] Regression test passes (or absence of seam is documented) - [ ] All `[DEBUG-...]` instrumentation removed (`grep` the prefix) -- [ ] Throwaway prototypes deleted (or moved to a clearly-marked debug location) +- [ ] Throwaway prototypes deleted (or moved to an explicitly-marked debug location) - [ ] The hypothesis that turned out correct is stated in the commit / PR message — so the next debugger learns **Then ask: what would have prevented this bug?** If the answer involves architectural change (no good test seam, tangled callers, hidden coupling) hand off to the `/improve-codebase-architecture` skill with the specifics. Make the recommendation **after** the fix is in, not before — you have more information now than when you started. diff --git a/plugins/bin/skills/improve-codebase-architecture/SKILL.md b/plugins/bin/skills/improve-codebase-architecture/SKILL.md index 05984a6..71cee67 100644 --- a/plugins/bin/skills/improve-codebase-architecture/SKILL.md +++ b/plugins/bin/skills/improve-codebase-architecture/SKILL.md @@ -55,7 +55,7 @@ Present a numbered list of deepening opportunities. For each candidate: **Use CONTEXT.md vocabulary for the domain, and [LANGUAGE.md](LANGUAGE.md) vocabulary for the architecture.** If `CONTEXT.md` defines "Order," talk about "the Order intake module" — not "the FooBarHandler," and not "the Order service." -**ADR conflicts**: if a candidate contradicts an existing ADR, only surface it when the friction is real enough to warrant revisiting the ADR. Mark it clearly (e.g. _"contradicts ADR-0007 — but worth reopening because…"_). Don't list every theoretical refactor an ADR forbids. +**ADR conflicts**: if a candidate contradicts an existing ADR, only surface it when the friction is real enough to warrant revisiting the ADR. Flag it explicitly (e.g. _"contradicts ADR-0007 — but worth reopening because…"_). Don't list every theoretical refactor an ADR forbids. Do NOT propose interfaces yet. Ask the user: "Which of these would you like to explore?" diff --git a/plugins/bin/skills/prototype/SKILL.md b/plugins/bin/skills/prototype/SKILL.md index 8bd99d8..664ec88 100644 --- a/plugins/bin/skills/prototype/SKILL.md +++ b/plugins/bin/skills/prototype/SKILL.md @@ -1,6 +1,6 @@ --- name: prototype -description: Build a throwaway prototype to flush out a design before committing to it. Routes between two branches — a runnable terminal app for state/business-logic questions, or several radically different UI variations toggleable from one route. Use when the user wants to prototype, sanity-check a data model or state machine, mock up a UI, explore design options, or says "prototype this", "let me play with it", "try a few designs". +description: Build a throwaway prototype to flush out a design before committing to it. Routes between two branches — a runnable terminal app for state/business-logic questions, or multiple radically different UI variations toggleable from one route. Use when the user wants to prototype, sanity-check a data model or state machine, mock up a UI, explore design options, or says "prototype this", "let me play with it", "try a few designs". --- # Prototype @@ -11,14 +11,14 @@ A prototype is **throwaway code that answers a question**. The question decides Identify which question is being answered — from the user's prompt, the surrounding code, or by asking if the user is around: -- **"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. +- **"Does this logic / state model feel right?"** → [LOGIC.md](LOGIC.md). Build a minimal 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 multiple 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 -1. **Throwaway from day one, and clearly marked as such.** Locate the prototype code close to where it will actually be used (next to the module or page it's prototyping for) so context is obvious — but name it so a casual reader can see it's a prototype, not production. For throwaway UI routes, obey whatever routing convention the project already uses; don't invent a new top-level structure. +1. **Throwaway from day one, and labeled as such.** Locate the prototype code close to where it will actually be used (next to the module or page it's prototyping for) so the context is unambiguous — but name it so a casual reader can see it's a prototype, not production. For throwaway UI routes, obey whatever routing convention the project already uses; don't invent a new top-level structure. 2. **One command to run.** Whatever the project's existing task runner supports — `pnpm `, `python `, `bun `, etc. The user must be able to start it without thinking. 3. **No persistence by default.** State lives in memory. Persistence is the thing the prototype is *checking*, not something it should depend on. If the question explicitly involves a database, hit a scratch DB or a local file with a clear "PROTOTYPE — wipe me" name. 4. **Skip the polish.** No tests, no error handling beyond what makes the prototype *runnable*, no abstractions. The point is to learn something fast and then delete it. diff --git a/plugins/bin/skills/tdd/SKILL.md b/plugins/bin/skills/tdd/SKILL.md index 7a98941..cdd0dad 100644 --- a/plugins/bin/skills/tdd/SKILL.md +++ b/plugins/bin/skills/tdd/SKILL.md @@ -91,7 +91,7 @@ Rules: After all tests pass, look for [refactor candidates](refactoring.md): - [ ] Extract duplication -- [ ] Deepen modules (move complexity behind simple interfaces) +- [ ] Deepen modules (move complexity behind narrow interfaces) - [ ] Apply SOLID principles where natural - [ ] Consider what new code reveals about existing code - [ ] Run tests after each refactor step diff --git a/plugins/bin/skills/write-docs/SKILL.md b/plugins/bin/skills/write-docs/SKILL.md index 0d2da2d..1d6ccb2 100644 --- a/plugins/bin/skills/write-docs/SKILL.md +++ b/plugins/bin/skills/write-docs/SKILL.md @@ -61,7 +61,7 @@ You are a technical writer that produces documentation by reading code and spec 1. **Identify scope.** User names specific files or sections. If not provided, propose candidates based on the description — wait for explicit approval before reading. -2. **Read and extract.** Read approved files. Extract: public API surface, described behaviour, visible constraints, non-obvious invariants. Note what the code does NOT explain (caller intent, error handling rationale, non-obvious side effects). +2. **Read and extract.** Read approved files. Extract: public API surface, described behaviour, visible constraints, hidden invariants. Note what the code does NOT explain (caller intent, error handling rationale, hidden side effects). 3. **Gap check.** Present extracted behaviour to the user. Ask them to fill only the gaps — what the code does not explain. Log any explicitly deferred gaps. If the user requests to skip this step, log the reason and proceed. diff --git a/plugins/git/skills/git-branches/SKILL.md b/plugins/git/skills/git-branches/SKILL.md index cef3f0c..ade4c06 100644 --- a/plugins/git/skills/git-branches/SKILL.md +++ b/plugins/git/skills/git-branches/SKILL.md @@ -48,7 +48,7 @@ Default to **GitHub Flow** (simpler, modern, CI/CD-friendly). Fall back to **Git ## Workflow - [ ] **Determine pattern:** Check git plugin config (`.claude/plugins/git/config.json`, if present — see `config.example.json` in the plugin root for the expected shape) for `branching_pattern` (default: `github-flow`). If not set, inspect repo for `develop` branch or `release/*` branches; if present, assume Gitflow. -- [ ] **Create branch:** Use `git switch -c `. Base defaults to config's `base_branch` (usually `main` or `develop`). Include intent metadata in branch name or return as structured result (e.g., `{ "branch": "feature/x", "intent": "implement feature X" }`). +- [ ] **Create branch:** Use `git switch -c `. Base defaults to config's `base_branch` (typically `main` or `develop`). Include intent metadata in branch name or return as structured result (e.g., `{ "branch": "feature/x", "intent": "implement feature X" }`). - [ ] **Track remote:** If pushing, always use `git push -u origin ` to establish tracking. - [ ] **Safety checks before destructive ops:** Before delete/force-push/rebase with history loss, check: (1) Is this branch tracking a remote? Warn if yes. (2) Are there unpushed commits? Warn if yes. (3) Does the orchestrator call include `confirm: true`? Fail if not. For humans, prompt interactively. - [ ] **Return structured results:** Always return branch operations as JSON or structured text: `{ "action": "create", "branch": "feature/x", "base": "main", "tracking": "origin/feature/x", "intent": "implement feature X" }`. Agents need to parse this for subsequent operations. diff --git a/plugins/git/skills/pc-author/SKILL.md b/plugins/git/skills/pc-author/SKILL.md index 3bcad4a..4449da2 100644 --- a/plugins/git/skills/pc-author/SKILL.md +++ b/plugins/git/skills/pc-author/SKILL.md @@ -66,7 +66,7 @@ Read `.pre-commit-config.yaml` first. Note any stale `rev` values (see **Rev sta 1. Identify the hook entry and its repo block. 2. State what will be removed: hook ID, and whether the parent repo block will also be deleted (if it would have zero hooks remaining). Wait for confirmation. 3. Remove the hook entry. If the repo block now has zero hooks remaining, remove the entire repo block. -4. Write. Run `pre-commit validate-config`. If non-zero: revert the edit, show the error, and stop — do not leave a broken config (removal edits are not safely auto-fixable, unlike a bad new hook block, which can usually be corrected in place). +4. Write. Run `pre-commit validate-config`. If non-zero: revert the edit, show the error, and stop — do not leave a broken config (removal edits are not safely auto-fixable, unlike a bad new hook block, which can often be corrected in place). ### Configuring top-level keys diff --git a/plugins/gitea/skills/gitea-issues/SKILL.md b/plugins/gitea/skills/gitea-issues/SKILL.md index 7dfaab5..93acc02 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.) @@ -72,7 +72,7 @@ Call `list_issues owner: repo: state: <"open"|"closed"|"all", def ### create 1. Extract `title` and `body` from conversation context (the most recent task, bug description, or explicit statement). Fall back to an empty body if nothing is available. -2. Run the enrichment sequence in `references/enrichments.md`: infer labels (composing `gitea-labels-milestones`), check for a clearly-fitting open milestone (composing the same skill), and check for a configured default assignee. +2. Run the enrichment sequence in `references/enrichments.md`: infer labels (composing `gitea-labels-milestones`), check for a well-matched open milestone (composing the same skill), and check for a configured default assignee. 3. Call `issue_write method: "create" owner: repo: title: body: <body> labels: [<resolved IDs, or omit>] milestone: <resolved ID, or omit> assignees: [<default login, or omit>]`. 4. Fire immediately — no confirmation step for the create itself. @@ -86,7 +86,7 @@ Call `issue_read method: "get_comments" owner: <owner> repo: <repo> issue_number ### close `<N>` -Call `issue_write method: "update" owner: <owner> repo: <repo> issue_number: <N> state: "closed"`. There is no `method: "close"`. +Call `issue_write method: "update" owner: <owner> repo: <repo> issue_number: <N> state: "closed"`. No `method: "close"` exists. ### comment `<N>` 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/gitea/skills/gitea-workflow/SKILL.md b/plugins/gitea/skills/gitea-workflow/SKILL.md index 92b7a6e..c374fd1 100644 --- a/plugins/gitea/skills/gitea-workflow/SKILL.md +++ b/plugins/gitea/skills/gitea-workflow/SKILL.md @@ -56,7 +56,7 @@ Never dispatch to `gitea-issues` or `gitea-prs` based on guessing from phrasing ## Step 3 — Route explicit but domain-unclear requests -For requests that name a capability but not obviously which skill owns it, use this index: +For requests that name a capability without a clear owning skill, use this index: | Skill | Covers | |---|---| @@ -67,7 +67,7 @@ For requests that name a capability but not obviously which skill owns it, use t | `gitea-files` | Read/write/delete individual files, list a directory, walk the full repo tree. | | `gitea-releases` | Release and tag CRUD — draft/prerelease flags, release notes, semver tags. | -If a request clearly names one of these (e.g. "create a milestone" → `gitea-labels-milestones`, "read this file from the repo" → `gitea-files`), invoke that skill directly rather than routing through here. Use this table only when the user or an upstream agent is unsure which skill applies. +If a request unambiguously names one of these (e.g. "create a milestone" → `gitea-labels-milestones`, "read this file from the repo" → `gitea-files`), invoke that skill directly rather than routing through here. Use this table only when the user or an upstream agent is unsure which skill applies. ## Step 4 — Report 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/<name>.agent.md` (project) or `<plugin>/agents/<name>.agent.md` (plugin) diff --git a/plugins/kyberforge/skills/forge/SKILL.md b/plugins/kyberforge/skills/forge/SKILL.md index ea2ea5b..2b7e50b 100644 --- a/plugins/kyberforge/skills/forge/SKILL.md +++ b/plugins/kyberforge/skills/forge/SKILL.md @@ -32,7 +32,7 @@ This step always runs inline, in the current conversation — grilling is intera ## Step 2 — Classify the artifact type -Match the grilled intent against exactly one row (or more than one, if the intent genuinely spans several): +Match the grilled intent against exactly one row (or more than one, if the intent genuinely spans multiple): | Intent | Artifact type | Route to | | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -----------------------------| ---------------------------------| diff --git a/plugins/kyberforge/skills/marketplace-author/SKILL.md b/plugins/kyberforge/skills/marketplace-author/SKILL.md index 6fae3d8..a9692a0 100644 --- a/plugins/kyberforge/skills/marketplace-author/SKILL.md +++ b/plugins/kyberforge/skills/marketplace-author/SKILL.md @@ -182,7 +182,7 @@ Read `.claude-plugin/marketplace.json`. Identify the entry to remove. State the ### Step 2 — HITL gate -State clearly before proceeding: +State the following before proceeding: > "I will remove the `<name>` entry from both `.claude-plugin/marketplace.json` and `.github/plugin/marketplace.json`. This does not delete the plugin files. Confirm?" diff --git a/plugins/kyberforge/skills/skill-author/SKILL.md b/plugins/kyberforge/skills/skill-author/SKILL.md index 35911d0..3b84d36 100644 --- a/plugins/kyberforge/skills/skill-author/SKILL.md +++ b/plugins/kyberforge/skills/skill-author/SKILL.md @@ -114,7 +114,7 @@ Rename the placeholder section heading to one that fits the skill's structure Ask of every sentence: "Would the agent get this wrong without it?" Cut anything that answers "no." **Include:** -- Non-obvious sequences or ordering constraints — the agent may skip or reorder steps without this +- Sequences or ordering constraints that aren't self-evident — the agent may skip or reorder steps without this - Domain conventions the agent cannot infer from general knowledge — this is the core value a skill adds - One default per decision point, plus one escape hatch — never a menu; menus cause the agent to pause or pick arbitrarily - Gotchas — facts that defy reasonable assumptions; the agent will get these wrong every time without them diff --git a/scripts/skill-size-check.sh b/scripts/skill-size-check.sh new file mode 100755 index 0000000..8d92692 --- /dev/null +++ b/scripts/skill-size-check.sh @@ -0,0 +1,35 @@ +#!/usr/bin/env bash +set -euo pipefail + +# Enforces agentskills.io's skill-authoring.md guidance: keep SKILL.md under 500 +# lines and 5,000 tokens, so the full body doesn't crowd out conversation +# history and other active skills once loaded into context. Vale can't express +# a whole-file length ceiling (its checks operate on text patterns, not raw +# file size), so this is a plain script instead of a Vale rule. +# +# Token counts aren't computed exactly here — word count (`wc -w`) is used as +# a proxy. For English prose this typically runs somewhat below true BPE token +# counts, so a 5,000-word file is already at or past 5,000 tokens in practice; +# treat this as a conservative, cheap approximation, not an exact measure. + +MAX_LINES=500 +MAX_WORDS=5000 +FAIL=0 + +for f in "$@"; do + [[ -f "$f" ]] || continue + + lines=$(wc -l < "$f") + if (( lines > MAX_LINES )); then + echo "ERROR: $f has $lines lines, exceeding the $MAX_LINES-line ceiling (agentskills.io skill-authoring.md)" >&2 + FAIL=1 + fi + + words=$(wc -w < "$f") + if (( words > MAX_WORDS )); then + echo "ERROR: $f has $words words (proxy for tokens), exceeding the $MAX_WORDS-token ceiling (agentskills.io skill-authoring.md)" >&2 + FAIL=1 + fi +done + +exit $FAIL diff --git a/styles/Kyberforge/SentenceOpenerThereIs.yml b/styles/Kyberforge/SentenceOpenerThereIs.yml new file mode 100644 index 0000000..b636ada --- /dev/null +++ b/styles/Kyberforge/SentenceOpenerThereIs.yml @@ -0,0 +1,7 @@ +extends: existence +message: "Don't start a sentence with '%s' — name the subject directly" +level: warning +scope: text +ignorecase: false +raw: + - '(?:[;-]\s)There\s(is|are)|\bThere\s(is|are)\b' diff --git a/styles/Kyberforge/VagueQualifier.yml b/styles/Kyberforge/VagueQualifier.yml new file mode 100644 index 0000000..2de0ab4 --- /dev/null +++ b/styles/Kyberforge/VagueQualifier.yml @@ -0,0 +1,36 @@ +extends: existence +message: "'%s' is vague filler wording — state the point precisely instead" +level: warning +scope: text +ignorecase: true +tokens: + - clearly + - obviously + - obvious + - simply + - simple + - easily + - easy + - of course + - everyone knows + - completely + - exceedingly + - excellent + - extremely + - fairly + - huge + - interestingly + - largely + - mostly + - quite + - relatively + - remarkably + - several + - significantly + - substantially + - surprisingly + - tiny + - usually + - various + - vast + - very diff --git a/tests/test-skill-size-check.sh b/tests/test-skill-size-check.sh new file mode 100755 index 0000000..f43de7e --- /dev/null +++ b/tests/test-skill-size-check.sh @@ -0,0 +1,65 @@ +#!/usr/bin/env bash +# Regression test for scripts/skill-size-check.sh: enforces agentskills.io's +# 500-line/5,000-word(proxy-for-token) SKILL.md size ceiling. +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +SCRIPT="$REPO_ROOT/scripts/skill-size-check.sh" +PASS=0 +FAIL=0 + +pass() { echo " PASS: $1"; PASS=$((PASS + 1)); } +fail() { echo " FAIL: $1"; FAIL=$((FAIL + 1)); } + +TMPDIR="$(mktemp -d)" +trap 'rm -rf "$TMPDIR"' EXIT + +make_fixture() { + local name="$1" lines="$2" words_per_line="$3" file + file="$TMPDIR/$name.md" + { + echo "---" + echo "name: $name" + echo "description: Test fixture." + echo "---" + for ((i = 1; i <= lines; i++)); do + w="" + for ((j = 1; j <= words_per_line; j++)); do + w="$w word" + done + echo "$w" + done + } > "$file" + echo "$file" +} + +echo "" +echo "--- passes a file under both limits ---" +SMALL="$(make_fixture small 10 5)" +if "$SCRIPT" "$SMALL"; then + pass "file under both limits exits 0" +else + fail "file under both limits should have exited 0" +fi + +echo "" +echo "--- fails a file over the line limit ---" +MANY_LINES="$(make_fixture many-lines 600 1)" +if "$SCRIPT" "$MANY_LINES" 2>/dev/null; then + fail "file over the 500-line ceiling should have exited non-zero" +else + pass "file over the 500-line ceiling exits non-zero" +fi + +echo "" +echo "--- fails a file over the word-count limit ---" +MANY_WORDS="$(make_fixture many-words 10 600)" +if "$SCRIPT" "$MANY_WORDS" 2>/dev/null; then + fail "file over the 5,000-word ceiling should have exited non-zero" +else + pass "file over the 5,000-word ceiling exits non-zero" +fi + +echo "" +echo "Results: $PASS passed, $FAIL failed" +[[ $FAIL -eq 0 ]]