Repository navigation
refactor(tools): move skill limit checks to Go #808
Description
Problem
The skill-limit gate still discovers skills, extracts frontmatter and reports findings through shell and embedded text processing. The current discovery, parsing and verdict boundaries are tools/skill-limits.sh:48, tools/skill-limits.sh:56, tools/skill-limits.sh:89 and tools/skill-limits.sh:116, on origin/main at a0347a715fab580ece45edfcb3d13509620459a8.
This is the skill-limit split of #332. Part of #588.
Acceptance criteria
- AC1: With no explicit targets, the command checks the same packaged and repository-local skills as the frozen shell, without also checking deployed mirrors of packaged skills.
- AC2: With explicit targets, the command checks the same manifests, including the existing missing-manifest skip behavior.
- AC3: The finding list for each manifest equals the frozen shell's for missing fields, name length, name character set, reserved names, description length and XML-like text.
- AC4: An unreadable manifest produces its own unreadable finding rather than reusing a previous manifest's parsed values.
- AC5: The command preserves the frozen shell's finding format, order, total and exit status, including silence on a clean run.
- AC6: Non-functional: checking skills requires no network access.
- AC7: Existing entry points end as thin shell wrappers that probe the Go subcommand without side effects and fall back to the frozen shell only when that capability is absent.
- AC8: Failure of a supported Go invocation never becomes success through shell fallback or a repeated check.
- AC9: Non-functional: release N-2 inputs and invocations retain their existing verdicts without a raised binary floor or required consumer configuration changes.
- AC10: Verdicts and diagnostics are deterministic across supported Linux, macOS and Windows environments, including non-ASCII input.
- AC11: Non-functional: checking skills leaves repository contents and Git state unchanged.
Scope and non-goals
Preserve the current 64/1024 length boundaries and their actual treatment of multibyte input; lowercase/digit/hyphen checks; reserved-name substring checks; folded and single-line descriptions; the existing XML-like-text predicate; explicit-target order; and per-manifest parsing failure. These are witnessed by tools/skill-limits.sh:28, tools/skill-limits.sh:33, tools/skill-limits.sh:93, tools/skill-limits.sh:100, tools/skill-limits.sh:119, tools/skill-limits.sh:129 and tools/skill-limits.sh:144.
No dedicated skill-limit Bats suite exists in the inspected tree. Add behavioral coverage from the frozen command rather than claiming coverage from the unrelated version or generation suites.
Non-goals: broadening frontmatter syntax, changing length semantics, changing platform limits, enforcing token body budgets, changing the set of published harnesses, or retiring the fallback. The body-budget warning belongs to token measurement (tools/skill-tokens.sh:266); fallback retirement follows separately under D9.
Approach
Start with failing checks for stale parsed values and incorrect boundary verdicts. Move the gate behavior to Go and compare the complete finding transcript with the frozen shell on a recorded corpus. Keep the current task interface (mise.toml:434). Leave implementation design to the pull request.
Dependencies
None blocking. The probe-and-fallback wrapper lets this split land without waiting for #542, the #333 → #544 → #382 → #545 → #546 chain or the #359 delivery chain. Removing the fallback later waits for #333, #544 and the N-2 window, and is filed as a follow-up. Skill limit checks are repository build tooling, not merge, commit or pull request enforcement, so they do not wait for #753. #332 and #588 track the enclosing migration.
Size
S. One pull request for a single offline gate and its compatibility coverage.
Verify plan
- AC4: first show a valid manifest followed by an unreadable one cannot inherit the first manifest's fields; assert an unreadable finding and nonzero result. Also cover a failed parser that prints plausible values.
- AC3: first capture failing boundary cases, then compare shell and Go for empty fields, 63/64/65 name lengths, 1023/1024/1025 description lengths, uppercase and punctuation, each reserved substring, folded/single-line/missing frontmatter and XML-like text. Include UTF-8 cases at both boundaries to freeze actual verdicts rather than infer them from comments.
- AC1: fixtures with packaged skills, deployed mirrors and an additional local-only skill compare the exact checked set with the frozen command.
- AC2: explicit target lists, trailing separators and missing manifests compare order, findings and skips.
- AC5: compare stdout, stderr and status for a clean run, one finding and multiple findings across multiple skills, including final totals.
- AC6: network traps remain untouched on every success and failure.
- AC11: compare repository bytes, index and refs before and after every fixture; mutation traps remain untouched.
- AC7: missing binary and older binary without the capability select the frozen command once; assert unchanged arguments, results and a side-effect-free probe.
- AC8: a supported command that fails, including after printing findings, returns nonzero with no fallback and no repeat; a failed probe that cannot establish capability absence refuses.
- AC9: run the unchanged task and wrapper against the N-2 corpus with absent, older and new binaries and compare all verdicts and transcripts.
- AC10: run the Go corpus under differing available locales on Linux, macOS and Windows; retain wrapper smoke coverage. Run focused tests,
mise run lintandmise run teston the reviewed head. - Definition of done: D1 each AC is verified at the merged head, with the commands and results cited in the pull request's Verification. D2 an independent review approves the exact merged head, and every finding has a recorded disposition. D3 required checks pass with no gate loosened. D4 the leak scans the repository specifies are clean, and every post made for this work (comments, descriptions) passes the per-post leak review before it is posted. D5 every commit is signed. D6 the documentation matches, or the description says
No doc impactwith the reason. D7 every commit carries anAssisted-bytrailer and the pull request carries theprovenance:agent-assistedlabel. D8 the change stays within the pull request size limits. D9 follow-up work, including deferred review findings, is filed as issues, each with a Problem section and numbered acceptance criteria and linking this issue; the pull request's Related section lists them, and any code that defers work links its issue. D10 the merge closes this issue through a closing reference, the project board shows the state Done, and a finalStatus:comment names the merge commit.
Risks
Replacing the narrow frontmatter extraction with broader interpretation could silently change existing verdicts. Length handling can also differ for UTF-8. Freeze observed results before porting and keep rule changes out of this PR. Undo: revert the port.
Operator authority
None.
Activity
tbhb-agent commentedon Oct 26, 2026
Refinement: READY — #808 skill-limit Go port requirements and verification are complete and accurate
Model: gemini-4-argon-high
Checked
tbhb-dev/devtools#808): Read live#808body and comments (0 comments) and all named issues (#332,#333,#359,#382,#542,#544,#545,#546,#588,#753).origin/mainata0347a715fab580ece45edfcb3d13509620459a8):tools/skill-limits.sh:48definespackaged(), which checkspackages/agents/*/.apm/skills/"$1".tools/skill-limits.sh:56handles target discovery (dirs=("$@"), defaulting attools/skill-limits.sh:57-72topackages/agents/*/.apm/skills/*/plus non-packaged.claude/skills/*/).tools/skill-limits.sh:89extractsskill_nameandskill_descfrom YAML frontmatter through an embedded Python block (tools/skill-limits.sh:89-114).tools/skill-limits.sh:116begins the field and limit checks and summary exit handling (tools/skill-limits.sh:116-154).#808accurately identifies itself as the skill-limit split of#332and part of#588(tools/skill-limits.shis inventoried at 154 physical lines in#588and 99 source lines in#332anddocs/script-inventory.md:117).AC1–AC11):AC6, matchingtools/skill-limits.sh:16-17andmise.toml:431), N-2 compatibility without raising the binary floor or requiring consumer configuration changes (AC9, matchingdocs/compatibility.md:3), cross-platform/locale determinism including non-ASCII input (AC10, matchingtools/skill-limits.sh:28-29), and read-only execution leaving repository contents and Git state untouched (AC11).AC7) and refusal to mask a failing supported Go invocation via fallback or retry (AC8) match#588AC5/AC7 and the N-2 transition model.origin/main:tools/skill-limits.sh:28(export LC_ALL=CwithPYTHONUTF8=1at line 29),tools/skill-limits.sh:33(readonly NAME_MAX=64andDESC_MAX=1024at line 34),tools/skill-limits.sh:93(frontmatter fence match),tools/skill-limits.sh:100(folded and single-line description extraction through line 106),tools/skill-limits.sh:119(name length and charset checks through line 128),tools/skill-limits.sh:129(reserved wordsanthropicandclaude), andtools/skill-limits.sh:144(XML-like<...>check on$skill_name$skill_desc).tests/tools/that no dedicatedskill-limitsBats suite exists onorigin/main(onlycheck-versions.bats,generate-skills.bats,masked-status.bats,release-dispatch.bats,release-readiness.bats, andrelease-verify.bats).tools/skill-tokens.sh:266owns the token body-budget warning (OVER the ${BODY_BUDGET}-token body budgetattools/skill-tokens.sh:266-270,281-284), andmise.toml:434defines[tasks.lint-skills]runningbash tools/skill-limits.sh(mise.toml:436, invoked bymise.toml:309).tools/skill-limits.shdecouples landing#808from#542,#333→#544→#382→#545→#546, and#359, while fallback removal is deferred to a follow-up after#333,#544, and the N-2 window.tools/skill-limits.shis repository lint tooling (mise.toml:434) rather than commit, pull request, or merge enforcement,#753does not block#808.AC1–AC11) has a concrete verification check in## Verify planthat proves its outcome, including boundary/UTF-8 equivalence (AC3), parser failure isolation (AC4), discovery and mirror exclusion (AC1,AC2), transcript/exit parity (AC5), network and mutation traps (AC6,AC11), probe/fallback and non-masking (AC7,AC8), N-2 compatibility (AC9), and multi-OS locale determinism (AC10).Sfits one pull request.tools/skill-limits.sh:91-108) and Bash underLC_ALL=C(tools/skill-limits.sh:28,119,139), with a clean revert undo path.Operator authority: Nonematches.agents/agent-skills.conf:4-9: preserving themise.toml:434task interface and wrappingtools/skill-limits.shtouches no path matched bygolden_paths,constitutional_paths,gate_config_paths, orowner_paths.Gaps
None.
Split
None needed. The scope is a single offline tooling gate (
tools/skill-limits.sh, 154 physical lines) plus its thin wrapper and behavioral/compatibility tests, which fits oneSpull request.Definition of done
The definition of done for #808 (Feature). Tick each item and give its evidence. (A ticked box below records planning coverage in the refined issue, not completed implementation work.)
## Verify planmapsAC1–AC11to concrete checks and explicitly requiresD1verification at the merged head with commands and results cited in the pull request'sVerificationsection.## Verify plan(Definition of done: D2) requires independent review approval on the exact merged head and a recorded disposition for every finding.## Verify plan(AC10andDefinition of done: D3) requiresmise run lint,mise run test, and required checks passing on the reviewed/merged head with no gate loosened.check-leaks.shwith the repository's private-strings list, and the leak review of each post. — Covered in planning:## Verify plan(Definition of done: D4) requires clean repository leak scans and per-post leak review before posting comments or descriptions.## Verify plan(Definition of done: D5) requires every commit to be signed.docs/, CHANGELOG or help text), or the Verification section says "No doc impact" and why. — Covered in planning:## Verify plan(Definition of done: D6) requires matching documentation updates orNo doc impactwith the reason in the pull request description.provenance:agent-assistedlabel. — Covered in planning:## Verify plan(Definition of done: D7) requiresAssisted-bytrailers on every commit and theprovenance:agent-assistedlabel on the pull request.## Sizescopes the change toS(one pull request for a single offline gate) and## Verify plan(Definition of done: D8) requires staying within pull request size limits.## Scope and non-goals,## Dependencies, and## Verify plan(Definition of done: D9) require filing follow-up work (including fallback retirement after#333,#544, and the N-2 window, plus any deferred review findings) as issues with aProblemsection and numbered acceptance criteria linked underRelated.Status:comment names the merge commit. — Covered in planning:## Verify plan(Definition of done: D10) requires closing#808via a closing reference at merge, the project board showingDone, and a finalStatus:comment naming the merge commit.Round: 1 of 3