fix(skill-quality): check the description field cap separately from the listing cap - #3150
Conversation
…he listing cap `DESC_CHAR_CAP=1536` is Claude Code's in-context listing truncation for the assembled entry (`description` + " - " + `when_to_use`). The Agent Skills spec states a separate, smaller maximum for the `description` field alone: "Must be non-empty / Maximum 1024 characters / Cannot contain XML tags". The gate checked only the looser limit, so a description could sit under 1536 combined, breach 1024 on its own, and pass clean. Nineteen skills in this marketplace do. Add check 2b for the field maximum, reported separately from check 2. WARN rather than FAIL, on measured evidence: no local validator enforces the field maximum. `claude plugin validate --strict` (Claude Code 2.1.241) passes a 1248-char description clean, verified against a throwaway fixture plugin, the only warning raised being an unrelated missing `author`. The breach is latent for filesystem and plugin skills and hard only on Skills API upload, so failing the build would block the fleet over a limit nothing local applies. Numbered 2b rather than 26 to avoid renumbering checks 3-25, whose identities are cited by number across this repo and in tracker items. Tests cover the boundary (1024 legal, 1025 warns) and the discriminating case: a description over 1024 whose assembled entry stays under 1536, which check 2 passes and only check 2b catches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0188gyMe4eBgtcw6izKzPusM
…c-char-cap # Conflicts: # plugins/skill-quality/.claude-plugin/plugin.json # plugins/skill-quality/CHANGELOG.md
|
Claude finished @kyle-sexton's task in 55s —— View job Security Review
No security issues found. This PR adds check 2b to
|
|
Last security-reviewed head: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9a4b37614
ℹ️ 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".
…t bytes Review finding from the Codex reviewer on #3150. `${#CUR_DESC}` counts UTF-8 bytes, not characters, under a byte-oriented locale. The Agent Skills spec limit is "Maximum 1024 characters", so a non-ASCII description false-warns: measured, 600 'é' characters report as 1200 under LC_ALL=C. Count codepoints via the same UTF-8 -> UTF-32BE iconv form check 22 already uses (every codepoint becomes exactly 4 bytes, so byte-count/4 is the codepoint count on any host), with the same UTF-8-locale fallback where iconv is absent. DESC_LEN stays a byte count for check 2, whose 1536 listing cap is a separate measure, so check 2 behavior is unchanged. Regression test runs the checker under LC_ALL=C against a 600-codepoint multibyte fixture, so it fails on the byte-counting form rather than passing incidentally on a UTF-8 host. Verified as a negative control: reverting the fix fails that assertion and only that one. Side effect worth noting: docs-hygiene:compress now measures 1030 codepoints where the byte form reported 1044. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0188gyMe4eBgtcw6izKzPusM
|
Claude finished @kyle-sexton's task in 1m 0s —— View job Security Review
No security issues found. Reviewed the diff since the prior security review (commit
This stays within the skip-gate condition for "no security-relevant surface after reading the diff" ( |
|
Claude finished @kyle-sexton's task in 47s —— View job Tasks
ResultSkipped per the For reference, the follow-up Codex review comment on this thread flags a real, worth-tracking issue for a future PR: |
|
Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
2 participants
Closes #3119
Summary
skill-quality:checkcarried one description limit and treated it as the only one. There are two, at different layers, and only the looser was checked — so nineteen skills in this marketplace breach the Agent Skills spec's field maximum and pass the gate clean.Fix
DESC_CHAR_CAP=1536is Claude Code's in-context listing truncation for the assembled entry (description+" - "+when_to_use). The Agent Skills spec states a separate, smaller maximum for thedescriptionfield alone:(platform.claude.com Agent Skills overview, fetched 2026-08-23.)
These do not unify. A description can sit under 1536 combined and still breach 1024 on its own.
Adds check 2b for the field maximum, reported separately from check 2 with its own message.
WARN, not FAIL — on measured evidence, not preference. No local validator enforces the field maximum:
claude plugin validate --strict(Claude Code 2.1.241) passes a 1248-char description clean, verified against a throwaway fixture plugin, the only warning raised being an unrelated missingauthor. The breach is latent for filesystem and plugin skills and hard only for a skill uploaded through the Skills API. Failing the build on it would block the fleet over a limit nothing in the local toolchain applies.Counted in codepoints, not bytes (fixed in review — see below). The spec says "Maximum 1024 characters", and
${#var}degrades to byte counting under a byte-oriented locale, so 600écharacters report as 1200 underLC_ALL=C. Uses the same UTF-8 → UTF-32BEiconvform check 22 already uses, with the same UTF-8-locale fallback.DESC_LENstays a byte count for check 2, so check 2 behavior is unchanged.Numbered
2b, not26— same concern as check 2 at a second layer, and renumbering would break the identity of checks 3–25, which are cited by number across this repo and in tracker items. Consequence worth a reviewer's eye: the plugin's "twenty-five deterministic checks" claim is left standing on that reading. Say the word if you'd rather it read twenty-six and I'll renumber.Verification
plugins/skill-quality/scripts/check-skill.test.sh— full suite passes, including four new assertions.>1024, not>=), 1025 warns.desc(1100)+ joiner +wtu(40)= 1143. Check 2 passes the entry; only check 2b catches the field breach. Without this case a single cap could satisfy both assertions, so it is the one that proves the layers are independent.LC_ALL=Cmust stay silent. Verified as a negative control — reverting to the byte-counting form fails that assertion and only that one.CHECK_SKILL_SKILLS_ROOT=plugins/docs-hygiene/skills check-skill.sh compress→WARN: description alone is 1030 codepoints (Agent Skills spec field maximum 1024), run stillPASS — 0 errors.scripts/check-changelog-parity.sh --checkand--check-bumpboth pass.claude plugin validate plugins/skill-qualitypasses.markdownlint-cli2clean on the changed markdown.bash -nclean on both scripts. (shellcheckis not installed in this environment — CI covers it.)Nineteen skills currently trip the new warning. Trimming them is deliberately not in this PR — check 3 protects quoted trigger phrases, so it is gated work, not a mechanical sweep.
Correction to an earlier revision of this description: it claimed nineteen was a lower bound because the gate reported
compressat 1044 where an independent scan said 1030. That gap was the byte-vs-codepoint bug, not a scan error — under the corrected codepoint count the gate reports 1030 and the two agree. Nineteen is the count, not a floor.Related
Refs #3118 — the spec container this defect was surfaced by. Not closed here; it closes when the detector work ships.