feat(scripts): gate hand-written skill counts against the tree by kyle-sexton · Pull Request #3034 · melodic-software/claude-code-plugins

kyle-sexton · 2026-08-19T17:13:38Z

No linked issue

Summary

A plugin's prose counts its own skills by hand — "bundling fourteen skills", "the other thirteen skills are zero-config" — and nothing recomputed those numbers when a skill landed. Adding one silently falsified every sentence that had counted the old set, in files nobody opens during that change.

The measured record: PR #3011 went stale on six such counts, each caught by a different human or automated reviewer rather than by anything mechanical, and session-flow's own CHANGELOG records an "other eleven skills" line that was already off by one before that PR touched it. A reviewer catch is not a control.

This adds the gate, and fixes the four live drifts it found on a clean main.

Fix

New gate — scripts/check-skill-count-claims.sh + .test.sh + skill-count-claim-exemptions.txt, wired into ci.yml as skill-count-claim-gate (self-test first, then --check) and added to ci-status.needs. It matches a closed set of composition-claim grammars across each plugin's README, manifest, and skill bodies, and compares the claimed number — spelled or numeric — against the skills the plugin actually has.

The spelled form is why nothing found this before: the claims read "fourteen skills", not "14 skills", so no numeric grep ever hit them.

Four live drifts found and fixed:

Plugin	Claimed	Actual
`testing`	"Four skills"	five — and the README table omitted `/testing:audit` entirely
`verification`	"Two skills"	three
`improvement`	"One skill"	two, in its own first release
`docs-hygiene`	"the other five skills do not use it"	eight-skill plugin, and `extract-ssot` does use it

The testing case is the payoff: a count drift marked a wholly undocumented skill, so a reader on that front page had no way to learn audit exists. Both halves are fixed — the count and the missing table row.

For docs-hygiene the sentence was substantively false too, so it now names its subjects instead of counting them. That is the treatment this plugin's own audit-noise enum-list shape prescribes for a hardcoded consumer count; the two agree on the diagnosis and differ only in enforcement.

Scope is stated in the script header, not left implicit:

The recognized grammars are a closed set, because most sentences pairing a number with the word "skills" assert nothing about plugin size ("gate one skill", "nine skills moved into three new plugins"). A gate that fires on those trains people to route around it.
One grammar was evaluated and rejected: a definite-article reference ("the two skills…") reads as a claim about a named pair as often as about the whole set — discipline, which has 17 skills, says exactly that.
CHANGELOG.md is out of scope. A dated entry describes the release it shipped with; rewriting it to match today's tree would falsify history.
Document-internal counts ("the four buckets" in a document defining five) are a real second class this gate does not hold, and the header says so rather than letting the coverage read as complete.

Portability annotations. check-shell-portability.sh --all was permanently red on 13 hits across three files, which made it unusable as an audit tool: a mode that always fails teaches you to ignore it, so a genuine 14th hit would land in the noise. Every one is a false positive in one of two classes — PCRE tokens as test data in resolve-convention-pattern.test.sh (the shorthand is the thing under test; the assertions prove the resolver rejects it), and Windows path fixtures (C:\temp\skills, \\server\share\skills, D:\work\acme\keepme). In each case the construct cannot be spelled portably because it is the input under test, so a per-site portability-ok: with a stated reason is the correct disposition. The whole-file portability-scope: escape was deliberately not used — the gate reserves it for its own fixture corpus, and these are neighbouring gates' tests.

Review findings addressed

Three Codex P2s, all verified real against the tree before acting (b8df7ec2):

A plugin whose last skill is removed loses its skills/ directory, and the scan skipped any plugin without one — the stale-count-by-removal case the gate most needs, reported as success. Every plugin is now scanned; missing or empty skills/ is a count of zero.
Matching worked per physical line, so a markdown wrap between the verb and the count hid a claim, and reflowing a paragraph could permanently disable the gate for a claim it used to hold. Each line is now tested alone and joined with its successor, stopping at block boundaries. This found a live wrapped claim in plugins/naming/README.md (correct) and one in claude-config/skills/setup/SKILL.md (a prerequisite-scope subset count, now exempted with its reason).
The prefilter tested *[Ss]kill* on raw text, which rejects an all-caps SKILLS, dropping a heading before the case fold could reach it. The fold now runs first.

Fixing 2 surfaced two defects the review did not name: joining double-counted every wrapped-adjacent claim (46 sites reported where 24 exist — the guard must re-test the pattern against the next line, not check containment of the matched extent, because most forms open with a boundary group that captures the join's own space); and one sentence can satisfy two grammars at once, so the guard re-tests every form rather than only the one that matched.

One Claude review finding (3369fbc6): the --check FAIL message reconstructed the plugin's real count as expected + 1, which is wrong exactly when the zero-clamp fires — it reported "has one" directly above "should be zero". The count is now carried through the report row and read from it. Enforcement was never affected; the diagnostic was, in precisely the last-skill-removed case the zero-skill scan was built for.

Verification

22 black-box assertions, with the negative ones load-bearing — ordinary prose like "gate one skill" and "nine skills moved into three new plugins" must not flag.

Mutation-tested. Reverting any of these turns the suite red: mismatch-reported-as-ok, --check-never-fails, drop-the-minus-one-basis, drop-the-stale-exemption-guard, scan-CHANGELOG, exemption-never-hits, drop-the-case-fold, skip-plugins-without-a-skills-dir, no-line-joining, narrow-the-dedup-guard-to-one-form, and restore-the-expected + 1-reconstruction.

Gates run locally, all green on the merged tree:

scripts/check-skill-count-claims.sh --check — 24 claims match the tree, 2 exempted
bash scripts/check-skill-count-claims.test.sh — all 22 assertions passed
shellcheck --rcfile=.shellcheckrc and shfmt -d on both new scripts and the three annotated files
actionlint on ci.yml
scripts/check-shell-portability.sh --all — No unexcused GNU-only constructs in 1325 shell file(s) (was exit 1 with 13 hits)
scripts/check-changelog-parity.sh in all four modes — 212 headings preserved across 6 changed changelogs
scripts/check-lane-coverage.sh --check — all 39 lanes reachable from ci-status.needs
scripts/validate-plugins.sh, catalog sync, cheat-sheet generator tests, sync-manifest check
scripts/affected-tests.sh --run — all 11 selected suites passed, no unmapped files

The gate proved itself on the merge. main landed a claude-ops eleventh skill while this branch was open. Resolving that conflict took main's manifest wholesale (so the new skill's description survived rather than being clobbered by a stale ten-skill string), and the gate then verified the new claim: plugins/claude-ops/README.md:4 and its manifest both read "eleven skills" against eleven skill directories.

Version bumps + CHANGELOG entries: testing 0.6.2, verification 0.5.3, improvement 0.1.1, docs-hygiene 0.17.2, claude-ops 0.33.1, repo-hygiene 0.10.7.

(This section previously listed claude-ops 0.32.10 and repo-hygiene 0.10.6 — stale after the merge with main renumbered both. A hand-written count in prose going stale is the exact defect this PR gates against, and the PR body is not a surface the gate covers. Caught by review.)

Refs feat(session-flow): add show-options, a nothing-hidden skill menu #3011 — the PR whose six stale counts are this gate's motivating evidence
Refs feat(claude-ops): add audit-skill-visibility — why most of a skill fleet never gets used #3035 — landed the claude-ops eleventh skill mid-review; its count is what the gate verified on the merge
docs/adr/0016-source-skill-recommendation-from-the-catalog-not-the-listing.md — the decision record from feat(session-flow): add show-options, a nothing-hidden skill menu #3011
plugins/docs-hygiene/skills/audit-noise/SKILL.md — the enum-list noise shape, this gate's advisory counterpart
scripts/check-skill-leaf-names.sh — the existing gate whose script/self-test/CI-lane shape this one follows

A plugin's prose counts its own skills by hand — "bundling fourteen skills", "the other thirteen skills are zero-config" — and nothing recomputed those numbers when a skill landed. Adding one silently falsified every sentence that had counted the old set, in files nobody opens during that change. The measured record: PR #3011 went stale on six such counts, each caught by a different human or automated reviewer rather than by anything mechanical, and session-flow's own CHANGELOG records an "other eleven skills" line that was already off by one before that PR touched it. A reviewer catch is not a control. check-skill-count-claims.sh matches a closed set of composition-claim grammars across each plugin's README, manifest, and skill bodies, and compares the claimed number against the skills the plugin actually has. It found four live drifts on a clean main: - testing said "Four skills" with five, and its table omitted /testing:audit entirely — the count drift marked a wholly undocumented skill, now added. - verification said "Two skills" with three. - improvement said "One skill" with two, in its first release. - docs-hygiene said "the other five skills do not use markdownlint-cli2" in an eight-skill plugin, and the substance was wrong too: extract-ssot does use it. That sentence now names its subjects instead of counting them, which is what this plugin's own audit-noise `enum-list` shape prescribes. Scope is stated in the script header rather than left implicit. The recognized grammars are a closed set, because most sentences pairing a number with the word "skills" assert nothing about plugin size ("gate one skill", "nine skills moved into three new plugins"); a gate that fires on those trains people to route around it. One form was evaluated and rejected: a definite-article reference reads as a claim about a named pair as often as about the whole set. CHANGELOG.md is out of scope — a dated entry describes the release it shipped with, and rewriting it to match today's tree would falsify history. Document-internal counts ("the four buckets" in a document defining five) are a real second class this gate does not hold, and the header says so. Verification: 16 black-box assertions, the negative ones load-bearing; six mutations of the gate's logic each turn the suite red. shellcheck at the repo rcfile, shfmt, actionlint, portability, lane-coverage, changelog-parity, and all 11 affected suites are green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADsDCToTtjvYut3ZQXJHDM

check-shell-portability.sh --all reported 13 GNU-only-construct hits across three test files. Every one is a false positive, and none could be spelled portably, so each gets a per-site portability-ok: marker with its reason. Two classes: - lib/resolve-convention-pattern.test.sh (7) carries \w, \s and \b as TEST DATA — PCRE tokens written into a fixture config and handed to the resolver so the case can assert the resolver REJECTS them. None is a pattern this suite passes to grep or sed, so none can behave differently on a BSD userland, and the shorthand cannot be removed because the shorthand is the thing under test. - claude-ops-paths.test.sh (3) and batch-common.test.sh (1) carry Windows paths — 'C:\temp\skills', '\\server\share\skills', 'telemetry\skills', 'D:\work\acme\keepme'. The backslashes are the input these cases normalize, not GNU classes. Same disposition the fleet already applies to HKCU\SOFTWARE registry paths in managed-scope.test.sh and permission-state.test.sh. Deliberately NOT a whole-file portability-scope: declaration on the regex corpus, tidier though that diff would be. The gate reserves the whole-file escape for its OWN fixture corpus; this file is the convention resolver's, and stretching a blanket exemption to a neighbouring gate's tests is how such hatches become mutes. The claude-ops markers ride their own case arms rather than a shared comment block above them. A first pass used the block, which cleared the adjacent arm but not one two lines below — block adjacency does not reach past intervening code. Co-locating each marker means reordering the arms cannot silently detach an exemption from the site it excuses. No behavior change: every assertion is untouched and all three suites pass. The lib edit does not fan out — sync-resolve-convention-pattern.sh copies the implementation, not the test, and --check confirms the one plugin copy still matches. shellcheck at the repo rcfile, shfmt, changelog-parity in all four modes, manifests and catalog are green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADsDCToTtjvYut3ZQXJHDM

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 927f5aab16

ℹ️ 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".

The PR was opened through the GitHub API as an App installation token, and GitHub's recursion guard means events raised by that token do not start `pull_request` workflow runs. Observed on PR #3034: the two `pull_request_target` workflows (pr-issue-linkage, do-not-merge) attached and passed, while both `pull_request` workflows (ci, pr-title) never ran. This empty commit raises a `synchronize` event from a git push, which is the event every prior ci.yml run on this branch was triggered by. Squash-merge drops it from main. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADsDCToTtjvYut3ZQXJHDM

All three were found by Codex review on #3034, and all three were verified against the tree before acting -- each is a way the gate reported success while a stale count stood. 1. A plugin whose LAST skill is removed loses its skills/ directory, and the scan skipped any plugin without one. That is precisely stale-count-by- removal, the case the gate most needs to hold, reported as success. Every plugin is now scanned; a missing or empty skills/ is a count of zero. 2. Matching worked per physical line, so an ordinary markdown wrap between the verb and the count hid a claim -- and reflowing a paragraph could permanently disable the gate for a claim it used to hold. Each line is now tested alone and joined with its successor, stopping at block boundaries so a heading or table row cannot manufacture a claim. This found a live wrapped claim in plugins/naming/README.md (correct) and one in claude-config/skills/setup/SKILL.md (a prerequisite-scope subset count, now exempted with its reason). 3. The cheap prefilter tested `*[Ss]kill*` on raw text, which rejects an all-caps SKILLS -- so a heading or emphasized sentence was dropped before the case fold could reach it. The fold now runs first. A prefilter that changes which claims are found is a hole, not an optimization. Fixing 2 surfaced two defects of my own that the review did not name: - Joining double-counted every wrapped-adjacent claim, reporting 46 sites where 24 exist. The guard must re-test the pattern against the next line rather than check containment of the matched extent: most forms open with a boundary group that captures the join's own space, so the extent never appears verbatim in the next line and containment passes everything. - One sentence can satisfy two grammars at once ("It ships" + "two skills, one concern"), so the guard re-tests EVERY form, not just the one that matched, or the single claim lands at two line numbers. Also fixed while in here: moving the prefilter below normalization made the gate spawn three processes per line across ~100k lines, taking it from sub-second to multi-minute. normalize_line is now pure bash writing to a global, and it handles BOTH dash spellings -- the real UTF-8 character a README carries and the JSON escape a manifest carries. The escape branch was lost once to an editor normalizing it, so it is built from a variable now. Verification: 21 black-box assertions, up from 16; the four new fixtures cover removal, a wrap falling inside the claim phrase, an all-caps claim, and a two-grammar overlap. Each fix is mutation-tested -- reverting any one of the three, plus the two dedup guards, turns the suite red. shellcheck at the repo rcfile, shfmt, portability, editorconfig and typos all clean; the gate reports 24 claims matching the tree with 2 exempted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADsDCToTtjvYut3ZQXJHDM

Resolves three conflicts, all version/changelog collisions from main moving under this branch: - plugins/claude-ops/.claude-plugin/plugin.json — main landed 0.33.0 with an ELEVENTH skill (audit-skill-visibility) and the matching "Eleven skills" description. Took main's side wholesale and re-applied this branch's bump on top as 0.33.1, so the new skill's description survives intact rather than being clobbered by a stale ten-skill string. - plugins/claude-ops/CHANGELOG.md — kept main's 0.33.0 entry and moved this branch's portability-annotation entry above it as 0.33.1. - plugins/repo-hygiene/CHANGELOG.md — main used 0.10.6 for its disable-model-invocation change, so this branch's entry renumbers to 0.10.7 and the manifest follows. Two different changes cannot share one version. The skill-count gate passes on the merged tree and verifies main's new claim: plugins/claude-ops/README.md:4 and the manifest both read "eleven skills" against eleven skill directories. That is the gate doing its job on a count that changed while this branch was open. Verified on the merged tree: the gate's 21 assertions, --check at 24 claims with 2 exempted, and changelog-parity in all four modes (212 headings preserved across 6 changed changelogs). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADsDCToTtjvYut3ZQXJHDM

claude · 2026-08-19T17:53:29Z

Claude finished @kyle-sexton's task in 1m 30s —— View job

Security review

Gather context (PR diff, changed files, review comments)
Run /review:security-review skill
Post findings

Reviewed the full diff (gh pr diff 3034) against the security-review criteria: injection surfaces, authorization/access-control gaps, secret/token handling, and dangerous GitHub Actions patterns.

No security issues found.

Details considered:

.github/workflows/ci.yml (+22 lines): new skill-count-claim-gate job checks out with persist-credentials: false, uses an already-pinned actions/checkout SHA, and only runs two local scripts (self-test then --check) against files already in the checkout. No secrets, no widened permissions:, no pull_request_target/workflow_run exposure, nothing derived from PR-controlled input (titles, branch names, issue bodies) is interpolated into a run: block.
scripts/check-skill-count-claims.sh (new file): pure text-processing over the repo's own tracked files (README/manifest/SKILL.md), using bash parameter expansion and [[ =~ ]] — no eval, no sed/awk command construction from file content, no subprocess spawned with untrusted data. All inputs are repo-controlled, not attacker-controlled (this isn't processing PR titles/bodies/branch names from an external contributor).
scripts/check-skill-count-claims.test.sh: self-contained, operates only inside its own mktemp -d sandbox, trapped for cleanup.
scripts/skill-count-claim-exemptions.txt: static data file, no code path treats it as executable/interpolated shell.
lib/resolve-convention-pattern.test.sh, claude-ops-paths.test.sh, batch-common.test.sh: only added inline portability-ok: comment annotations to existing PCRE-shorthand/Windows-path test fixtures — no logic changes.
Remaining changed files are prose-only (README.md, CHANGELOG.md, plugin.json version bumps).

This lane defers to zizmor's advisory checks for supply-chain/unpinned-action/permission findings, which don't apply here since the workflow addition follows the existing pinned-SHA, minimal-permission pattern already used elsewhere in ci.yml.

(Note: the earlier Codex review comments on this PR are correctness/false-negative concerns in the count-detection logic, not security findings — out of scope for this security-only lane.)

github-actions · 2026-08-19T17:55:25Z

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

claude · 2026-08-19T18:00:19Z

Claude finished @kyle-sexton's task in 4m 51s —— View job

Code review — `/review:code-review`

Gather context (PR diff, changed files)
Review scripts/check-skill-count-claims.sh for correctness/maintainability
Review scripts/check-skill-count-claims.test.sh and exemptions file
Review CI wiring (ci.yml) and per-plugin fixes
Consider existing Codex review comments (context only, not auto-applied)
Post findings as inline comments + summary

No blocking findings. This is a well-scoped, well-tested gate and I found nothing a careful reviewer would need to block on.

What I checked

The three Codex P2 comments (last-skill-removal, wrapped-prose joining, all-caps case-fold) are already addressed in this diff — verified by reading check-skill-count-claims.sh (every plugins/*/ is scanned, missing skills/ = count 0), lines 728–758 (physical-line + joined-with-next-line matching, with a block-boundary guard and a same-form dup check to avoid double-counting), and lines 764–779 (case fold happens before the *skill* prefilter, not after). The corresponding zeta/eta/theta fixtures and assertions 7b/7c/7d in the test file exercise exactly these cases.
Verified the four "live drift" fixes against the actual tree (not just trusting the PR description): testing has 5 skill directories, verification 3, improvement 2, docs-hygiene 8 — all match the corrected counts in the README/manifest edits.
Verified the docs-hygiene substantive fix: extract-ssot/context/execution-checklist.md does reference markdownlint-cli2, confirming the reworded README sentence ("extract-ssot names it as one option for its ship-gate lint step") is accurate, not just a count fix.
Verified both skill-count-claim-exemptions.txt entries against the actual source files, including the one that only resolves after line-joining (plugins/claude-config/skills/setup/SKILL.md's "the other three / skills still run" wraps exactly at that boundary) — both are genuine non-composition claims (a shared-dependency count and a named-subset count), not miscounts being swept under the rug.
Cross-checked portability-ok: annotations added in lib/resolve-convention-pattern.test.sh, claude-ops-paths.test.sh, and batch-common.test.sh against check-shell-portability.sh's own recognition logic (is_annotated() matches the literal substring portability-ok: anywhere on the line) — the markers are correctly placed and will be honored.
CI wiring: skill-count-claim-gate runs the self-test before --check (so a broken detector can't mask a regression) and is correctly added to ci-status's needs list, following the same shape as the neighboring skill-leaf-name-gate.

Not independently verified

This review lane's tool access is read-only (Read/Grep/Glob/gh pr diff); I could not execute scripts/check-skill-count-claims.sh, its test suite, shellcheck, or shfmt myself. The PR body's local-run results (16/16 assertions, mutation testing, shellcheck/shfmt clean, check-shell-portability.sh --all clean) are author-claimed and unverified from my side — everything above is what I could confirm by reading the code and cross-referencing it against the actual repository tree.

comment-hygiene failed the hygiene lane on two tracker refs in the new gate's header, and both are real violations of a convention every other script here already follows: scripts/check-skill-count-claims.sh:18 tracker-ref:repo-issue scripts/check-skill-count-claims.sh:56 tracker-ref:pr-reference The rule allows a bare `(#N)` in a `#` comment -- check-shell-portability.sh alone carries six of them -- and rejects the fully-qualified `owner/repo#N` form and the `PR #N` prefix. Both flagged lines are rewritten to the bare form and the paragraphs rewrapped. This class could not be caught locally: comment-hygiene runs as an external composite action (melodic-software/ci-workflows) that this repo does not vendor, so there is no local invocation to run before pushing. CI is the first place it can fail. Swept every file this branch touches in a scanned extension for the same two forms plus `Fixes #`/`Closes #`/`issue #`: no others. Unchanged and re-verified: shellcheck at the repo rcfile, shfmt, the gate's 21 assertions, and --check at 24 claims with 2 exempted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADsDCToTtjvYut3ZQXJHDM

claude · 2026-08-19T18:02:46Z

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

Security review

Gather context (PR diff, changed files, prior review state)
Run /review:security-review skill for the new commits since the last reviewed head
Post findings

Skipped — no security-relevant surface in the new commits.

The prior security review on this PR covered head 58716a2 and found no issues. The only commit since then is 8267fa2 ("style(scripts): use the repo's bare issue-reference form in comments"), and its entire diff is:

--- a/scripts/check-skill-count-claims.sh
+++ b/scripts/check-skill-count-claims.sh
@@ -14,12 +14,12 @@
- # nobody opens during that change. The measured record: a single PR
- # (melodic-software/claude-code-plugins#3011) went stale on six such counts, each
+ # nobody opens during that change. The measured record: a single PR (#3011) went
+ # stale on six such counts, each
@@ -53,8 +53,8 @@
- #     cardinality, and picking the wrong one flags correct prose. PR #3011 hit
- #     this class too (a "four buckets" line surviving the addition of a fifth),
+ #     cardinality, and picking the wrong one flags correct prose. That same PR
+ #     (#3011) hit this class too (a "four buckets" line surviving a fifth),

This rewords two comment lines inside scripts/check-skill-count-claims.sh (shortening a cross-repo issue reference to the repo's bare form). No code, logic, permissions, or workflow changes — nothing to review here per the skip gate (§3 of /review:security-review: prior review still applies).

No security issues found — same conclusion as the previous review at head 58716a2, which still applies at head 8267fa2.

github-actions · 2026-08-19T18:05:40Z

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

claude · 2026-08-19T18:06:12Z

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

Code review — `/review:code-review`

Gather context (PR diff, changed files, prior review comments)
Invoke /review:code-review per repo instructions
Post findings as inline comments + summary

Scope: correctness/maintainability only (this repo has a dedicated security lane, already run separately on this PR). Reviewed all 23 changed files: the new gate (scripts/check-skill-count-claims.sh + .test.sh + exemptions), the ci.yml job wiring, the four count-drift fixes, the three portability-annotation diffs, and the version bump/CHANGELOG entries.

The three P2s Codex flagged earlier on this PR (zero-skill-plugin scan skip, wrapped-line joining, all-caps prefilter) all read as already closed by b8df7ec — each has a corresponding fix and a dedicated comment explaining the "why," plus a matching negative/positive test case (.test.sh cases 7b, 7c/7d, 7e/7f).

One finding, posted inline:

🟡 scripts/check-skill-count-claims.sh:466-469 — the --check FAIL message reconstructs "plugin has N skills" as expected + 1, but expected is clamped to 0 for a zero-skill plugin on a minus-one-basis claim, so the reconstruction reports "has one" instead of "has zero" — self-contradicting the very next line ("the number should be zero"). Doesn't break enforcement (exit code / MISMATCH detection are still correct) and no plugin in the fleet currently has 0 skills, but it's exactly the scenario the PR's whole-fleet zero-skill scan was built to catch, so it's the path most likely to eventually get hit.

Everything else checked out:

The four live drifts (testing, verification, improvement, docs-hygiene) match the actual skill-directory counts I verified independently, and the docs-hygiene/extract-ssot markdownlint claim in the corrected prose checks out against extract-ssot/context/execution-checklist.md.
The two exemption rows are legitimately dependency-sharing/prerequisite-scope counts, not plugin-size claims, and are self-checking against the skills they name.
The ci.yml addition follows the existing pattern (pinned checkout SHA, persist-credentials: false, self-test before --check, added to ci-status.needs).
The three portability-ok: annotations are on genuine test-data false positives (PCRE shorthand as fixture input, Windows path fixtures), each with its own inline reason rather than a whole-file escape, matching the script's own recognized marker.
Test suite (check-skill-count-claims.test.sh) is genuinely black-box and the negative assertions (non-claims, CHANGELOG exclusion, definite-article rejection) are present, not just the positive ones.

Minor, non-blocking: the PR body's version-bump table says claude-ops 0.32.10 / repo-hygiene 0.10.6, but the actual plugin.json values are 0.33.1 / 0.10.7 — looks like the description predates a rebase that picked up unrelated version bumps from main. Not a code issue, just flagging in case the description should be updated.

I could not execute the gate, its test suite, or the other verification commands the PR description lists (shellcheck, shfmt, check-shell-portability.sh, etc.) in this sandbox — the findings above are from static reading of the diff, not independent re-execution of the author's claimed verification.
· branch claude/skill-recommendation-system-opdhxd

Review finding on #3034, verified against the code and confirmed real. The --check FAIL message recovered the plugin's real count as `expected + 1`. That works on the normal minus-one path (actual - 1 + 1 == actual) but not when the clamp fires: `expected` is clamped up from -1 to 0 for a plugin with NO skills, so the message reported "has one" directly above "the number should be zero" -- the two lines contradicting each other. The count is now carried through the report row and read from it, never recomputed. Reconstruction was the bug; the row already knew the answer. Enforcement was never affected -- exit code and MISMATCH detection were correct throughout, and no plugin in the fleet has zero skills today. What was wrong is the diagnostic in exactly the last-skill-removed case that whole-fleet zero-skill scanning was added for, which makes it the path most likely to be the first one a real user hits. The zeta fixture now carries an "other three skills" line so the clamp path is actually exercised, and a 22nd assertion pins both halves of the message. Mutation-tested: restoring the `expected + 1` reconstruction turns the suite red on that assertion. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADsDCToTtjvYut3ZQXJHDM

claude · 2026-08-19T18:15:24Z

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

Security review

Gather context (PR diff since last reviewed head)
Run /review:security-review skill
Post findings

Skipped — no security-relevant surface in the new commit.

The last reviewed head was 8267fa2 (no issues found, per the prior review comment on this PR). The only commit since then is 3369fbc ("fix(scripts): read the real skill count instead of reconstructing it"), which addresses the diagnostic-text nit raised in the prior code review: scripts/check-skill-count-claims.sh#L414-L473 now carries the real $actual count through each report row instead of reconstructing it as expected + 1 (which produced a self-contradictory "has one" / "should be zero" message for a zero-skill plugin on the minus-one basis). scripts/check-skill-count-claims.test.sh#L235-L246 adds a black-box assertion for it.

Reviewed the change against the skip gate (§3 of /review:security-review, "no security-relevant surface"):

The diff only threads an existing integer ($actual, the array-derived skill count from skill_count()) through a pipe-delimited report row and into a printf/int_to_word call — no new input source, no eval, no command construction from file content, no change to what data is trusted.
int_to_word() (L101-112) is arithmetic-only (((n <= 20)), array indexing, %d/%s printf) — passing it $actual_count instead of the old reconstructed $total introduces no injection surface.
No workflow, permissions, or CI wiring changes in this commit.

This matches the prior conclusion at 58716a2/8267fa2: no security issues found, and this commit doesn't change that.
· branch claude/skill-recommendation-system-opdhxd

claude · 2026-08-19T18:17:05Z

Claude finished @kyle-sexton's task in 1m 22s —— View job

Code review — `/review:code-review`

Gather context (PR diff, changed files)
Invoke /review:code-review skill and follow its instructions
Post findings as inline comments + summary

No blocking findings. This PR was already reviewed twice by this lane (at heads 58716a2 and 8267fa2). The only change since the last review is commit 3369fbc, which fixes exactly the one nit both this lane and Codex-adjacent review had flagged at scripts/check-skill-count-claims.sh:466-469 (the zero-skill, minus-one-basis FAIL message reconstructing "has one" instead of "has zero").

What I verified in the fix commit

The real, unclamped skill count (actual) is now carried through the report row as a 7th pipe-delimited field on every push site — line 417 (ok), 424 (exempt), 427 (MISMATCH) — and read back consistently at both consumers, line 449 (discover mode) and line 463 (--check FAIL path). Field order matches on every write/read pair — no off-by-one in the IFS='|' read -r destructuring.
Traced the value itself: expected="$actual" is captured at line 402 before the minus-one clamp at line 407, so $actual pushed into the row is always the true, unclamped plugin skill count — int_to_word "$actual_count" at line 473 is now correct for the zero-skill case (number_words[0] = "zero", confirmed at line 86), not reconstructed.
The $expected-based total reconstruction the finding was about is gone entirely — no lingering dead code.
New test coverage: the zeta fixture gained a genuine minus-one-basis claim ('the other three skills are zero-config') on a plugin with no skills/ directory, so actual == 0 and the clamp at line 407 actually fires — this exercises the previously-buggy path rather than just asserting around it. Assertion 7g (check-skill-count-claims.test.sh:238-248) checks both halves of the message agree (has zero / should be zero), which is exactly the self-contradiction the original bug produced.
This is a surgical fix: no other logic paths, grammars, or CI wiring touched. Nothing new to flag.

Not independently verified

This lane's tool grant is Read/Glob/Grep/gh pr diff|view|comment|review only — no general command execution. I traced the fix and the new test logic by reading the source (confirming field ordering, the expected="$actual" capture point, and number_words[0]), but did not execute the test suite, shellcheck, or shfmt myself. The PR's own commit message reports the suite green and the mutation test (reverting to expected + 1 turns the new assertion red) — that is author-claimed and unverified from my side.
· branch claude/skill-recommendation-system-opdhxd

No linked issue ## Summary The skill-count gate's header states which *class* of stale count it holds (repository-derivable) and which it does not (document-internal), then asserts "Nothing else was scoped out" — which reads as completeness the gate does not have. It never named the *surface* boundary: the gate reads files in the repo, so a class-A count written anywhere else is equally derivable and equally unguarded, because nothing here can see it. ## Fix Adds a `SURFACES, not just classes` paragraph to the header and qualifies the "nothing else" sentence to the files the gate actually reads. The omission has a measured cost, and it is this gate's own introduction that paid it: PR #3034 carried a hand-written version-and-assertion table in its body, and that table went stale within the hour when a merge with `main` renumbered two plugins. Those counts were class A — plainly derivable from the tree — and invisible to the gate, because a PR description is not a file in the repo. A reviewer caught it, which is precisely the control this gate exists to replace. So the header now says: prose describing the CURRENT repo that lives on GitHub rather than on disk — a PR or issue body, a wiki page — derives its counts at write time or drifts, and a green gate must not be read as covering it. A second commit narrows that list. The first draft included "a release note", which contradicted a decision stated 40 lines above it in the same header: `CHANGELOG.md` is deliberately not scanned, because a dated entry describes the release it shipped with and rewriting it to match today's tree falsifies history rather than fixing a claim. A GitHub release note is that same kind of record. The header now states that carve-out explicitly as travelling off-disk with the count, rather than leaving it implied — off-disk surfaces get the same current-state test as the scanned ones, not a broader one. Raised by an automated reviewer; the contradiction was real. Comment-only change throughout. No behavior, no new surface scanned, no token-list edit. ## Verification - `shellcheck` at the repo's own `.shellcheckrc` (not a laxer severity) — clean - `shfmt -d` — clean - `scripts/check-skill-count-claims.test.sh` — all assertions passed - `scripts/check-skill-count-claims.sh --check` — every claim matches the tree - `typos`, `editorconfig-checker` — clean - `scripts/check-changelog-parity.sh --check` and `--check-bump origin/main` — no version bump required; `scripts/` is not a versioned plugin, and this PR touches nothing under `plugins/` - Tracker references in the added comment use the bare `(#N)` form the `comment-hygiene` lane requires, checked before pushing because that lane is an external composite action this repo does not vendor and so cannot be run locally All of the above were re-run after the second commit, not just the first. This body deliberately carries no hand-written counts of assertions, claims, or versions — the exact failure mode the change documents. The numbers are in the command output above, where they are regenerated rather than transcribed. **This body was itself corrected once.** After the second commit it still described the superseded wording, which is the same off-disk drift the change documents, occurring in this PR's own description. Fixed here rather than left as a demonstration. ## Related Refs #3034 — the PR that introduced this gate, and whose own body supplied the stale-count evidence cited above. --------- Co-authored-by: Claude <noreply@anthropic.com>

…aky (#3057) No linked issue ## Summary `scripts/check-shell-portability.sh --all` was treated as environmentally unreliable across four separate attempts, each written off as a command timeout or a container restart. That diagnosis was wrong, and it was the self-perpetuating kind: "it got unlucky" invites retrying the same run, which fails the same way and yields no new information. Nothing in the repo recorded the real reason. ## Fix Adds a `COST of --all` block to the script's header. The cause is structural, not environmental. Per-file cost grows superlinearly with file length, so a handful of the longest files dominate the whole sweep while the great majority cost almost nothing. It cannot fit a 600s command timeout on a tree this size, so it must be detached rather than retried. Two traps are recorded alongside it, because neither is discoverable from a failing run: - `pgrep -f 'check-shell-portability'` matches the **waiting** shell's own command line, so a waiter built on it can never see its condition clear. Two waiters were lost to this before it was spotted. Waiting on the pid is correct. - A per-file sweep keyed only on the timeout status silently passes over any file exiting 1, so its silence reads as a clean audit when nothing was actually checked. The block also states why CI runs the changed-file mode and never `--all` — true of `ci.yml`'s `shell-portability-lint` already, but written down nowhere. **On the numbers:** they are stated as a dated observation rather than a standing claim, and no file list or file count is hardcoded. The exact figures move with the corpus; the shape does not. That is deliberately the same surfaces-and-derivability discipline the sibling skill-count gate's header gained in #3050 — hardcoding "seven files over N lines" here would have created precisely the drift that gate exists to catch. Comment-only. No behavior, no token-list edit, no new construct class. ## Verification - `shellcheck` at the repo's own `.shellcheckrc` — clean (re-run after rebase) - `shfmt -d` — clean (re-run after rebase) - `scripts/check-shell-portability.test.sh`, which CI runs ahead of the gate — `PASS=333 FAIL=0` - `typos`, `editorconfig-checker` — clean - `scripts/check-changelog-parity.sh --check` — no version bump required; `scripts/` is not a versioned plugin and nothing under `plugins/` is touched - Tracker references use the bare `(#N)` form the `comment-hygiene` lane requires, checked before pushing because that lane is an external composite action this repo does not vendor and cannot be run locally - Rebased onto current `main` rather than left on the base it was written against, so `stale-base-overlap-gate` sees a current merge base The claim this documents was itself measured end to end: `--all` completed at exit 0 with `No unexcused GNU-only constructs in 1329 shell file(s)`, which is the verification that had been outstanding since #3034. ## Related Refs #3050 — the sibling gate's surfaces boundary, whose derivability discipline this block deliberately follows. Refs #3034 — the PR whose outstanding `--all` verification this work completed. --- _Generated by [Claude Code](https://claude.ai/code/session_01ADsDCToTtjvYut3ZQXJHDM)_ --------- Co-authored-by: Claude <noreply@anthropic.com>

claude added 2 commits August 19, 2026 13:20

chatgpt-codex-connector Bot reviewed Aug 19, 2026

View reviewed changes

Comment thread scripts/check-skill-count-claims.sh Outdated

Comment thread scripts/check-skill-count-claims.sh Outdated

Comment thread scripts/check-skill-count-claims.sh Outdated

claude added 2 commits August 19, 2026 17:23

kyle-sexton enabled auto-merge (squash) August 19, 2026 17:46

claude Bot reviewed Aug 19, 2026

View reviewed changes

Comment thread scripts/check-skill-count-claims.sh Outdated

kyle-sexton merged commit edbc08f into main Aug 19, 2026
51 checks passed

kyle-sexton deleted the claude/skill-recommendation-system-opdhxd branch August 19, 2026 18:30

kyle-sexton mentioned this pull request Aug 20, 2026

docs(scripts): name the surfaces the skill-count gate cannot see #3050

Merged

kyle-sexton mentioned this pull request Aug 20, 2026

docs(scripts): record why the --all portability sweep is slow, not flaky #3057

Merged

Navigation Menu

Uh oh!

feat(scripts): gate hand-written skill counts against the tree - #3034

feat(scripts): gate hand-written skill counts against the tree#3034
kyle-sexton merged 7 commits into
mainfrom
claude/skill-recommendation-system-opdhxd

kyle-sexton commented Aug 19, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

claude Bot commented Aug 19, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 19, 2026 •

edited

Loading

Uh oh!

claude Bot commented Aug 19, 2026 •

edited

Loading

Uh oh!

claude Bot commented Aug 19, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 19, 2026 •

edited

Loading

Uh oh!

claude Bot commented Aug 19, 2026 •

edited

Loading

Uh oh!

Uh oh!

claude Bot commented Aug 19, 2026 •

edited

Loading

Uh oh!

claude Bot commented Aug 19, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

kyle-sexton commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Fix

Review findings addressed

Verification

Related

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

claude Bot commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security review

Uh oh!

github-actions Bot commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code review — /review:code-review

What I checked

Not independently verified

Uh oh!

claude Bot commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security review

Uh oh!

github-actions Bot commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code review — /review:code-review

Uh oh!

Uh oh!

claude Bot commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Security review

Uh oh!

claude Bot commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code review — /review:code-review

What I verified in the fix commit

Not independently verified

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kyle-sexton commented Aug 19, 2026 •

edited

Loading

claude Bot commented Aug 19, 2026 •

edited

Loading

github-actions Bot commented Aug 19, 2026 •

edited

Loading

claude Bot commented Aug 19, 2026 •

edited

Loading

Code review — `/review:code-review`

claude Bot commented Aug 19, 2026 •

edited

Loading

github-actions Bot commented Aug 19, 2026 •

edited

Loading

claude Bot commented Aug 19, 2026 •

edited

Loading

Code review — `/review:code-review`

claude Bot commented Aug 19, 2026 •

edited

Loading

claude Bot commented Aug 19, 2026 •

edited

Loading

Code review — `/review:code-review`