feat(skill-quality): enforce the invocation-mode rubric fleet-wide

[kyle-sexton]

Closes #2968

Summary

Executes course lane 8's enforcement deliverable (#2910; rubric locked 2026-08-17): every skill in the fleet now states its invocation mode explicitly, skill-quality:check enforces that, and playbooks:skill-authoring routes the authoring-time decision to the rubric. The locked decisions are not relitigated here — this is their enforcement.

Fix

• 17 skills normalized to explicit disable-model-invocation: false — exactly the list the issue names (code-tidying:batch-simplify, education:explain, education:quiz-me, event-storming:methodology, event-storming:simulation, github:advise, github:audit, knowledge:book-distill, machine-health:audit, naming:name-it-better, plugin-quality:audit, repo-fleet-hygiene:audit, repo-hygiene:clean, toolchain:check, toolchain:lint, visualization:visualize, wizard:generate), all in the rubric's default class. The absent-key default is already false, so this is a no-behavior-change normalization. Fleet after it: 220 top-level skills, 161 false / 59 true / 0 missing the key.
• New check-skill.sh check 24 — explicit invocation mode. Severity is scoped by tree: a marketplace plugin skill (plugins/*/skills/*) missing the key FAILs; anywhere else it WARNs, because the absent-key default already behaves as false and a consumer's own skill should be informed by this fleet's convention rather than broken by it. A non-boolean value FAILs everywhere. Class attribution is not machine-checkable — only a setup skill's true is decidable by a static scan (class (ii), the PLUGIN-PHILOSOPHY setup contract) — so every other true emits a note pointing at the rubric rather than a warning no scan could clear.
• Six new contract tests (24a–24f) pinning: explicit false reports the default; a missing key outside plugins/ warns without failing; the same omission inside plugins/*/skills/* fails; a non-boolean fails; a non-setup true is noted, not warned; a setup skill's true is attributed to class (ii).
• playbooks:skill-authoring gains an authoring-time pointer to docs/conventions/invocation-mode/README.md (points, never restates) in its Skill-tool composition section.
• Rubric doc records the landed state: the normalization and check 24 shipped, with the recounted fleet figures, and the cross-references name the criterion instead of the filed issue.
• First eval suites for knowledge:book-distill (6 cases) and wizard:generate (5 cases) — the changed-skills gate requires evals for any skill whose SKILL.md changes, and these were the only two of the 17 without a suite. Cases are grounded in each skill's own contract (read-one-write-one interleave, untrusted source text, path-traversal slugification, EPUB package-document path, copyright caution; repo-first scoping, names-only .env reads, the human-approval gate before chmod +x, the off-limits library above the STAGES marker, gh absence degrading rather than failing).
• Version bumps + CHANGELOG entries for all 15 touched plugins (skill-quality minor for the new check; the rest patch).

Verification

• bash scripts/check-changed-skills.sh origin/main — 19 skills checked, 0 failed.
• bash plugins/skill-quality/scripts/check-skill.test.sh — 93 assertions pass; the failing set is byte-identical to the origin/main baseline (21 pre-existing container failures from mawk's missing ERE intervals, skill-quality: check-skill.sh fresh-eyes scanner breaks under mawk — ERE interval expressions unsupported #3005 — CI runs gawk). Diffed baseline-vs-head failure lists to confirm no new failure.
• Fleet audit: all 220 top-level skills carry the key and every value is a boolean (161 false / 59 true).
• bash scripts/validate-plugins.sh — all manifests + catalog validated; generate-catalog.mjs --check and generate-cheatsheet.mjs --check in sync.
• bash scripts/check-changelog-parity.sh --check / --check-bump origin/main / --check-order — all green (83 changelogs).
• check-jsonschema + check-evals-quality.sh on both new suites — ok / PASS with 0 warnings; scripts/check-orphaned-fixtures.sh --check — no orphans.
• npx markdownlint-cli2 on all 35 changed markdown files — 0 issues; shellcheck and shfmt -d clean on both changed shell scripts; typos clean.

Related

• Refs skill-quality/plugin-philosophy: aihero course lane 8: invocation-mode doctrine and evidence-driven fleet re-grade #2910 (lane 8 specification), docs/upstream/aihero-course.md lane 8 rows
• Refs planning: flip questionnaire to model-invoked — the one invocation-mode re-grade flip #2969 (the sibling lane 8 item: flipping planning:questionnaire to false)
• Refs skill-quality: check-skill.sh fresh-eyes scanner breaks under mawk — ERE interval expressions unsupported #3005 (the mawk artifact behind the pre-existing local test failures)

🤖 Generated with Claude Code

https://claude.ai/code/session_01QbfCrj3X9FfGL7VRZYmrn4

---

Generated by Claude Code

[claude]

Claude finished @kyle-sexton's task in 2m 43s —— View job

---

Claude's review

• Gather context (PR body, changed files list)
• Review the diff against origin/main (54 files, +535/-24)
• Post inline findings
• Post summary review comment

review:code-review skill invocation errored out repeatedly (returned <error>Execute skill: review:code-review</error> with no loaded instructions on 3 attempts), so this review was done manually against the same intent — correctness and maintainability findings — by reading the diff directly.

Summary

This is a clean, well-scoped enforcement PR. I traced the core logic (new check 24 in check-skill.sh), its six contract tests (24a–24f), the rubric doc, the 17 skill normalizations, the two new eval suites, and the version/CHANGELOG bumps. Everything checks out:

• Check 24's severity scoping (FAIL inside plugins/*/skills/*, WARN elsewhere, FAIL everywhere for a non-boolean) is implemented exactly as described, and the tests exercise every branch including the setup-skill class-(ii) attribution.
• All 17 listed skills got exactly disable-model-invocation: false added — no drift from the issue's list.
• Both new eval suites (book-distill 6 cases, wizard:generate 5 cases) are well-formed and grounded in each skill's actual contract (read-one-write-one interleave, untrusted source text, path-traversal slugification, EPUB spine handling, copyright caution / repo-first scoping, names-only .env reads, the approval gate before chmod +x, the off-limits library, gh-absence degradation).
• Spot-checked CHANGELOGs and version bumps for knowledge and wizard (the two with eval-suite additions) — both correctly document the eval suite and the disable-model-invocation change as separate entries.

Finding (posted inline)

Stale check-count in the plugin manifest description — plugins/skill-quality/.claude-plugin/plugin.json#L5 still reads "runs twenty-three deterministic checks," but check-skill.sh now runs 24 (this PR's own check 24). plugins/skill-quality/skills/check/SKILL.md was correctly updated to "twenty-four-check" / "twenty-four checks" in the same commit — the manifest description is the one place this PR's own description-sync effort missed. Low severity, but worth a one-line fix given how carefully the rest of the PR keeps cross-references in sync.

Note: I could not execute the test suite or check-changed-skills.sh myself in this environment (only git/read-only commands were permitted; bash <script> and python3 both required approval that wasn't available), so I relied on static reading rather than re-running the verification the PR description cites. Nothing in the static read contradicts those claims.
· branch claude/pocock-steering-course-00zkvd

[claude]

Claude finished @kyle-sexton's task in 59s —— View job

---

Security review

• Gather context (PR diff, changed files)
• Invoke /review:security-review
• Post findings

No security issues found.

This PR has no security-relevant surface:

• 44 of 54 changed files are SKILL.md frontmatter (adding an explicit disable-model-invocation: false), plugin.json version bumps, or CHANGELOG.md entries — pure metadata, no logic.
• The only code change is a new check (check 24) appended to check-skill.sh. It reads the disable-model-invocation frontmatter value through the file's existing skill_frontmatter::field / skill_frontmatter::strip_quotes helpers (heredoc-piped, no eval, no unsanitized value reaching a shell command) — the same safe pattern every other check in this script already uses. Its companion test file only adds fixture-driven assertions.
• docs/conventions/invocation-mode/README.md and the two SKILL.md prose additions (skill-quality:check, playbooks:skill-authoring) are documentation only.
• The two new evals/evals.json files are static eval-case data, validated against the existing schema.
• No .github/workflows/* files, no permission or trust-boundary changes, no secrets/token handling, no new dependencies, no injection surfaces.

Per the skip gate in /review:security-review, no deep hunting was warranted beyond confirming the absence of a security-relevant surface, so no findings are reported.

[github-actions]

Last security-reviewed head: 9ed4518613f9f862ae460e77a53298f699a3f697. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

[claude]

Claude finished @kyle-sexton's task in 53s —— View job

---

Security review

• Gather context (PR diff, changed files)
• Invoke /review:security-review
• Post findings

No security issues found.

Last security-reviewed head was adf2ce71b1c7b3c0fecd2d7e4e0f95f638f7053f. The only commit since then is 31ed374 ("test(scripts): state the invocation mode in the gate's skill fixtures"), which touches a single file:

• scripts/check-changed-skills.test.sh — adds three disable-model-invocation: false lines inside existing test-fixture heredocs (plugins/p1/skills/newbie/SKILL.md, plugins/p1/skills/legacy/SKILL.md fixture content used by the test). No shell logic changed, nothing executed as code, no new inputs, no injection surface.

This satisfies the skip gate (no security-relevant surface introduced by the delta). The prior review of the full PR diff (metadata normalization across 17 skills, the new check-skill.sh check 24 reading frontmatter through the file's existing safe helper functions, and doc/CHANGELOG additions) already found no issues and that finding still applies — nothing in this delta changes that assessment.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: adf2ce71b1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[github-actions]

Claude has reviewed this PR 3 times. The lane skips further automatic reviews after 5; deleting this comment resets the count.

[claude]

Claude finished @kyle-sexton's task in 2m 6s —— View job

---

Code review (/review:code-review)

• Gather context (PR diff, changed files)
• Invoke /review:code-review criteria
• Post findings

No blocking findings.

Reviewed against origin/main...HEAD (55 files, 538/-24). This is a mechanical, well-tested enforcement change:

• check-skill.sh check 24 (lines 1328–1367) reads disable-model-invocation through the existing skill_frontmatter::field/strip_quotes helpers, matching the pattern every other check in the file already uses. Verified the helper's comment/quote-stripping logic (skill-frontmatter.sh:36-95) handles the plain-scalar case correctly, and confirmed the severity split (FAIL under plugins/*/skills/*, WARN elsewhere, FAIL everywhere for a non-boolean) matches both the PR description and the six new tests (24a–24f).
• The six new tests exercise each branch (explicit false, missing key in/out of plugins/, non-boolean, non-setup true, setup true) and don't collide with any other fixture in the suite (only one make_skill setup call).
• The 17 SKILL.md normalizations are identical single-line disable-model-invocation: false insertions — spot-checked all of them via diff, no stray edits.
• Version bumps line up with the changelog: skill-quality is the one minor bump (0.16.0 → 0.17.0, new check), the other 14 touched plugins are patch bumps — matches "minor for the new check; the rest patch" in the PR body.
• Both new evals/evals.json files (knowledge:book-distill, wizard:generate) validate against plugins/skill-quality/reference/evals.schema.json (required id/prompt, a grading criterion via expectations) and read as grounded, skill-specific cases rather than filler.
• docs/conventions/invocation-mode/README.md, playbooks:skill-authoring, and skill-quality:check's own SKILL.md/gotchas are updated consistently with the shipped behavior (fleet count 220 = 161 false + 59 true matches the check-24 semantics).

One minor observation, not blocking: the setup-skill class-(ii) attribution in check 24 (check-skill.sh:1361) keys off $SKILL_NAME == "setup" alone, so any skill directory literally named setup gets attributed to the PLUGIN-PHILOSOPHY setup contract even if it isn't one. This only affects a note (not a warn/fail), and the PR's own comments are explicit that class attribution is a hand-verified, name-convention heuristic — so this reads as an accepted, documented limitation rather than a bug.
· branch claude/pocock-steering-course-00zkvd

[claude]

Claude finished @kyle-sexton's task in 2m 5s —— View job

---

Code review (/review:code-review)

• Gather context (PR body, changed files, diff origin/main...HEAD)
• Invoke /review:code-review skill (errored out on 3 attempts, as in the prior review pass on this PR — reviewed manually against the same correctness/maintainability scope by reading the diff directly)
• Verify prior findings against the latest commit
• Post summary review comment

Summary

This synchronize pushed one commit (9ed4518) since the last review pass, and it resolves both previously open findings on this PR:

• Stale check-count in the plugin manifest (flagged in my earlier inline comment on plugins/skill-quality/.claude-plugin/plugin.json) — now reads "twenty-four deterministic checks", matching skills/check/SKILL.md and docs/CATALOG.md. Confirmed no other "twenty-three" references remain repo-wide.
• Codex's P2 finding (quoted/whitespace-broken scalars normalizing into a passing boolean) — fixed correctly. Check 24 (check-skill.sh:1348-1379) no longer calls skill_frontmatter::strip_quotes; it now trims only leading/trailing whitespace via the standard bash ${var#"${var%%[![:space:]]*}"} / ${var%"${var##*[![:space:]]}"} idiom (verified this is the correct trim pattern — doesn't collapse internal whitespace), then explicitly detects a quoted scalar ("false" or 'false') and fails it with a message naming the quoting, rather than silently normalizing it to a boolean. Traced through the three new regression tests (24d-quoted, 24d-internal-space, 24d-comment) and confirmed each exercises the intended branch — including that a trailing # comment annotating the exception class still parses cleanly (via skill_frontmatter::field's existing comment-stripping, unaffected by this change).
• Spot-checked edge cases not covered by the new tests (empty value, value opening with #, case-insensitive TRUE) — all resolve to pre-existing behavior from the original commit (adf2ce7), not new regressions from this fix.
• docs/conventions/invocation-mode/README.md and playbooks:skill-authoring's new authoring-time pointer are consistent with the shipped check semantics; no drift found.

No new findings. Nothing blocking.

Note: as in the prior review pass, I could not execute check-skill.test.sh or check-changed-skills.sh in this environment (bash <script> requires approval not available here), so this is a static read rather than a re-run of the PR's own verification — consistent with the prior review's stated limitation.
· branch claude/pocock-steering-course-00zkvd

[claude]

Claude finished @kyle-sexton's task in 56s —— View job

---

Security review

• Gather context (PR diff, changed files, custom instructions)
• Invoke /review:security-review
• Post findings

No security issues found.

Last security-reviewed head was 31ed374fcf735116fb49017236a2db8268950e20. The only commit since then is 9ed4518 ("fix(skill-quality): validate the invocation mode as a bare YAML boolean"), which is itself the fix for the codex bot's earlier P2 finding on check 24 (quoted/whitespace-normalized strings passing as booleans). Reviewed the full delta:

• plugins/skill-quality/scripts/check-skill.sh#L1328-L1381 — the fix trims only leading/trailing whitespace (via parameter expansion, not a wholesale-whitespace strip) instead of stripping quotes, so disable-model-invocation: "false" now correctly fails as a quoted string rather than normalizing to a passing boolean, and a value with an internal space (e.g. f alse) no longer gets spliced back into false. The value still flows only through bash string comparisons and parameter expansion (DMI_RAW → DMI_TRIMMED → DMI_VAL), sourced from the existing skill_frontmatter::field helper — no eval, no unsanitized value reaching a shell command or file path. Same safe pattern as the rest of the script.
• plugins/skill-quality/scripts/check-skill.test.sh — three new regression tests (quoted "false", internal-space scalar, annotated boolean with trailing comment) are fixture-driven heredoc data, no logic changes.
• docs/CATALOG.md, plugins/skill-quality/.claude-plugin/plugin.json, plugins/skill-quality/skills/check/SKILL.md — description/version-bump/doc text only (the "twenty-three" → "twenty-four" sync flagged in an earlier review pass), no code.

No .github/workflows/* files touched, no permission or trust-boundary changes, no secrets/token handling, no new dependencies, no injection surface introduced. This satisfies the skip gate for this delta — the prior full-diff review (metadata normalization, the new check 24 reading frontmatter through the file's existing safe helpers, doc/CHANGELOG additions) still applies and is unaffected by this fix.
· branch claude/pocock-steering-course-00zkvd