Compare commits
2 Commits
ac72898a55
...
2777a834b2
| Author | SHA1 | Date | |
|---|---|---|---|
| 2777a834b2 | |||
| 95ba57d0d5 |
12
LESSONS.md
12
LESSONS.md
@@ -98,6 +98,18 @@ When running a full test audit, `claude plugin validate --strict` was not includ
|
|||||||
|
|
||||||
The pre-commit hook ran `shellcheck "$f"` without `-x`. Without `-x`, shellcheck fires SC1091 for every `source` statement and exits non-zero, blocking the commit. This was a latent bug since the hook was written, only triggered when `install.sh` (which sources `deploy-manifest.sh`) was staged for the first time. Compounding it: the `# shellcheck source=` directive in `install.sh` pointed to `deploy-manifest.sh` (bare filename, resolved from CWD = repo root) rather than `scripts/deploy-manifest.sh` (correct repo-root-relative path), so even with `-x` the file wasn't found on the first attempt. Fix: always pass `-x` to shellcheck in hooks. When writing a `source=` directive, use a path that resolves correctly from the CWD where shellcheck will be invoked — verify with `shellcheck -x <file>` before committing.
|
The pre-commit hook ran `shellcheck "$f"` without `-x`. Without `-x`, shellcheck fires SC1091 for every `source` statement and exits non-zero, blocking the commit. This was a latent bug since the hook was written, only triggered when `install.sh` (which sources `deploy-manifest.sh`) was staged for the first time. Compounding it: the `# shellcheck source=` directive in `install.sh` pointed to `deploy-manifest.sh` (bare filename, resolved from CWD = repo root) rather than `scripts/deploy-manifest.sh` (correct repo-root-relative path), so even with `-x` the file wasn't found on the first attempt. Fix: always pass `-x` to shellcheck in hooks. When writing a `source=` directive, use a path that resolves correctly from the CWD where shellcheck will be invoked — verify with `shellcheck -x <file>` before committing.
|
||||||
|
|
||||||
|
## 2026-06-22 — Plugin cache isolation rules out shared/ directories between skills
|
||||||
|
|
||||||
|
When two skills in the same plugin share a resource (e.g. validate.sh), the instinct is to put it in a shared/ directory and reference it with a relative path. This breaks silently after install: plugins are copied to a cache, and `../` paths across skill directories stop resolving. The correct pattern is duplication with clear ownership — one skill owns the canonical copy and the other delegates to it via a skill invocation (e.g. /skill-audit) rather than a file path. If delegation is not possible, duplicate the file and note the owning skill in a comment.
|
||||||
|
|
||||||
|
## 2026-06-22 — Qualitative rubrics should be grounded in upstream spec docs, not derived from in-repo usage
|
||||||
|
|
||||||
|
When skill-audit's qualitative checks for description quality and body discipline were first written, they were derived from skill-write's own authoring conventions — a circular dependency. Any drift in skill-write's conventions would silently propagate into the audit criteria. Fix: extract condensed reference files directly from the upstream spec (agentskills.io) and load them conditionally from the audit skill. The rubric is then grounded in the authoritative source and independent of in-repo convention drift.
|
||||||
|
|
||||||
|
## 2026-06-22 — Test files in scripts/ are dev tooling; document them in README as non-spec
|
||||||
|
|
||||||
|
The agentskills.io spec defines scripts/ for bundled executable scripts — it says nothing about test infrastructure. Bats test files placed in scripts/ (or scripts/tests/) are invisible to auditors following the spec and create silent README drift if not documented. Fix: place test files directly in scripts/ (no subdirectory), add a row to the README file table for each with a "dev tooling, not shipped with the plugin" note, and don't nest them in a tests/ subdirectory since that creates a non-spec directory structure.
|
||||||
|
|
||||||
## 2026-05-18 — Planning meta-commentary does not belong in deployed artifacts
|
## 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.
|
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.
|
||||||
|
|||||||
@@ -25,4 +25,5 @@ Provide the path to the skill directory to audit when invoking.
|
|||||||
| `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.sh` | Structural validator — checks name format, name matches directory, description length, line count, placeholder detection, script executable bit, and interactive-prompt detection |
|
||||||
| `references/description-quality.md` | Spec-grounded rubric for description auditing — loaded when a finding is borderline |
|
| `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/body-discipline.md` | Spec-grounded rubric for body discipline auditing — loaded when padding vs necessity is unclear |
|
||||||
|
| `scripts/validate.bats` | Bats test suite for validate.sh — dev tooling, not shipped with the plugin |
|
||||||
| `README.md` | This file |
|
| `README.md` | This file |
|
||||||
|
|||||||
@@ -1,13 +1,13 @@
|
|||||||
---
|
---
|
||||||
name: skill-audit
|
name: skill-audit
|
||||||
description: >
|
description: >
|
||||||
Audit a skill directory against the agentskills.io specification — structural
|
Use when the user wants to review a skill they wrote, says "audit this skill",
|
||||||
|
"check if my skill follows best practices", "review my SKILL.md", or wants to
|
||||||
|
know if a skill is ready to ship — even if they don't use the word "audit".
|
||||||
|
Audits a skill directory against the agentskills.io specification — structural
|
||||||
checks plus qualitative review of description quality, body discipline, formatting,
|
checks plus qualitative review of description quality, body discipline, formatting,
|
||||||
file structure, and internal consistency. Produces a PASS/FAIL/SUGGESTION punch
|
file structure, and internal consistency. Produces a PASS/FAIL/SUGGESTION punch
|
||||||
list with a specific fix proposal for every FAIL. Use when the user wants to
|
list with a specific fix proposal for every FAIL. Do not use to run evals, fix
|
||||||
review a skill they wrote, says "audit this skill", "check if my skill follows
|
|
||||||
best practices", "review my SKILL.md", or wants to know if a skill is ready to
|
|
||||||
ship — even if they don't use the word "audit". Do not use to run evals, fix
|
|
||||||
application code bugs, or perform general code review unrelated to skill quality.
|
application code bugs, or perform general code review unrelated to skill quality.
|
||||||
allowed-tools: Bash Read
|
allowed-tools: Bash Read
|
||||||
metadata:
|
metadata:
|
||||||
@@ -46,7 +46,7 @@ For each sentence in the body, apply: *"Would the agent get this wrong without t
|
|||||||
|
|
||||||
- **Defaults not menus**: every decision point gives one default + one escape hatch, not a list of options
|
- **Defaults not menus**: every decision point gives one default + one escape hatch, not a list of options
|
||||||
- **Why rationale**: include/exclude rules explain why, not just what
|
- **Why rationale**: include/exclude rules explain why, not just what
|
||||||
- **Control calibration**: prescriptive for fragile or critical sequences; flexible where multiple approaches are valid
|
- **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
|
||||||
|
|
||||||
If uncertain whether a sentence is padding or whether a control decision is correctly calibrated, read `references/body-discipline.md`.
|
If uncertain whether a sentence is padding or whether a control decision is correctly calibrated, read `references/body-discipline.md`.
|
||||||
|
|
||||||
|
|||||||
@@ -1,11 +1,11 @@
|
|||||||
#!/usr/bin/env bats
|
#!/usr/bin/env bats
|
||||||
|
|
||||||
setup() {
|
setup() {
|
||||||
REPO_ROOT="$(cd "$BATS_TEST_DIRNAME/../../../../../../" && pwd)"
|
REPO_ROOT="$(cd "$BATS_TEST_DIRNAME/../../../../../" && pwd)"
|
||||||
load "$REPO_ROOT/tests/test_helper/bats-support/load"
|
load "$REPO_ROOT/tests/test_helper/bats-support/load"
|
||||||
load "$REPO_ROOT/tests/test_helper/bats-assert/load"
|
load "$REPO_ROOT/tests/test_helper/bats-assert/load"
|
||||||
|
|
||||||
SCRIPT="$(cd "$(dirname "$BATS_TEST_FILENAME")/.." && pwd)/validate.sh"
|
SCRIPT="$(cd "$(dirname "$BATS_TEST_FILENAME")" && pwd)/validate.sh"
|
||||||
TMPDIR="$(mktemp -d)"
|
TMPDIR="$(mktemp -d)"
|
||||||
|
|
||||||
# Helper: create a minimal valid skill directory
|
# Helper: create a minimal valid skill directory
|
||||||
@@ -35,6 +35,7 @@ This skill produces its best output when you arrive with rich context:
|
|||||||
| `assets/templates/scripts/README.md` | Placeholder for bundled scripts |
|
| `assets/templates/scripts/README.md` | Placeholder for bundled scripts |
|
||||||
| `assets/templates/references/README.md` | Placeholder for reference docs |
|
| `assets/templates/references/README.md` | Placeholder for reference docs |
|
||||||
| `assets/templates/assets/README.md` | Placeholder for static assets |
|
| `assets/templates/assets/README.md` | Placeholder for static assets |
|
||||||
|
| `scripts/new-skill.bats` | Bats test suite for new-skill.sh — dev tooling, not shipped with the plugin |
|
||||||
|
|
||||||
## Placement
|
## Placement
|
||||||
|
|
||||||
|
|||||||
@@ -1,11 +1,11 @@
|
|||||||
#!/usr/bin/env bats
|
#!/usr/bin/env bats
|
||||||
|
|
||||||
setup() {
|
setup() {
|
||||||
REPO_ROOT="$(cd "$BATS_TEST_DIRNAME/../../../../../../" && pwd)"
|
REPO_ROOT="$(cd "$BATS_TEST_DIRNAME/../../../../../" && pwd)"
|
||||||
load "$REPO_ROOT/tests/test_helper/bats-support/load"
|
load "$REPO_ROOT/tests/test_helper/bats-support/load"
|
||||||
load "$REPO_ROOT/tests/test_helper/bats-assert/load"
|
load "$REPO_ROOT/tests/test_helper/bats-assert/load"
|
||||||
|
|
||||||
SCRIPT="$(cd "$(dirname "$BATS_TEST_FILENAME")/.." && pwd)/new-skill.sh"
|
SCRIPT="$(cd "$(dirname "$BATS_TEST_FILENAME")" && pwd)/new-skill.sh"
|
||||||
DEST="$(mktemp -d)"
|
DEST="$(mktemp -d)"
|
||||||
}
|
}
|
||||||
|
|
||||||
Reference in New Issue
Block a user