fix(skill-check): honor primary host skipSkills in Templates check#1430
Open
EricXu-0805 wants to merge 1 commit into
Open
fix(skill-check): honor primary host skipSkills in Templates check#1430EricXu-0805 wants to merge 1 commit into
EricXu-0805 wants to merge 1 commit into
Conversation
When `hosts/claude.ts` has `generation.skipSkills: ['claude']` (the existing config — the /claude outside-voice skill is intentionally for non-Claude hosts), `bun run gen:skill-docs` correctly skips generating `claude/SKILL.md`. But `scripts/skill-check.ts` Templates section still flags it as `❌ generated file missing!` and the script exits 1. Fix: mirror the same `skipSkills` filter that `gen-skill-docs.ts` already uses (see `scripts/gen-skill-docs.ts:510-516`). Read the primary host's skipSkills once, then in the Templates loop, if the output file is missing AND the skill dir is in skipSkills, log it as `- skipped per <host> host config` and skip the error flag instead of marking it missing. Impact: `bun run skill:check` now exits 0 for any host config that uses `skipSkills`. No behavior change when skipSkills is empty (default for most hosts) or when the skipped skill's output is in fact missing for some other reason. Test plan: configure `skipSkills: ['claude']` (default `hosts/claude.ts`), run `bun run gen:skill-docs`, confirm `claude/SKILL.md` is not generated, then `bun run skill:check` — before the patch, exit=1 with `❌ claude/SKILL.md — generated file missing!`; after the patch, exit=0 with `- claude/SKILL.md — skipped per claude host config`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
scripts/skill-check.tsTemplates section flagsclaude/SKILL.mdas❌ generated file missing!and exits 1, even thoughhosts/claude.tshasgeneration.skipSkills: ['claude']andgen-skill-docs.tscorrectly skips generating it. This is an inconsistency between the generator (honors skipSkills) and the checker (doesn't).Root cause
scripts/gen-skill-docs.ts:510-516already filters skills byskipSkills/includeSkillsper the current host config — soclaude/SKILL.mdnever gets generated for the Claude host.scripts/skill-check.ts:75-79iteratesdiscoverTemplates(ROOT)and naively checksfs.existsSync(outPath)without consulting any host config. Result: the legitimately-skippedclaude/SKILL.mdshows up as a hard failure, killingbun run skill:checkwith exit 1.Fix
Import the primary host (
claude) and check itsskipSkillsset before flagging missing outputs. The change mirrors the existing logic ingen-skill-docs.tsso check and generation stay aligned.11 added lines in
scripts/skill-check.ts. No behavior change whenskipSkillsis empty (the default for most hosts) or when a missing output is unrelated to skipSkills.Test plan
Pre-patch (with default
hosts/claude.tsskipSkills: ['claude']):Post-patch:
All other checks remain unchanged.
Notes
Spotted while running
/gstack-upgradeon a fresh clone after v1.31.1.0 — the upgrade itself succeeded cleanly (binaries rebuilt, 338 tests pass), butbun run skill:checkexited 1 on the cosmetic claude/SKILL.md false positive. No functional impact for users, but the check should be self-consistent.Need help on this PR? Tag
@codesmithwith what you need.