feat(ci): add check-skill-tiers lint to surface untagged skills#34
feat(ci): add check-skill-tiers lint to surface untagged skills#34naimkatiman merged 1 commit intomainfrom
Conversation
Closes the silent "## Other skills" gap from the tier-rendering generator (commit 56c3ed4). The bundled README groups skills by tier; an untagged skill would slide into a catch-all "Other skills" bucket without anyone noticing in code review. This lint fails the build when any source skill (SKILL.md or skills/<name>.md) is missing or has an unrecognized `tier:` value. The lint caught a real problem on first run: the root SKILL.md was the only source that hadn't been tagged in commit 56c3ed4. Added `tier: core` to fix it. Files: - src/bin/check-skill-tiers.mts (new) — pattern-matches src/bin/ check-skill-mirror.mts: pure `checkSkillTiers(repoRoot)` helper for unit tests, plus a CLI `main()` that exits 0/1 with a clear message. - src/test/check-skill-tiers.test.mts (new) — 10 tests covering missing tier, unrecognized tier, ignored README.md, missing skills/ dir, variant-spelling acceptance (FEATURED, tier1, tier-2), and CLI exit-code behavior on synthetic fixtures plus the live-repo no-drift assertion. - package.json — adds `verify:skill-tiers` npm script alongside the existing `verify:skill-mirror` and `verify:docs-substrings` lints. - SKILL.md — adds `tier: core` to the core 7 Laws skill (caught by the lint). - Generated: bundled mirror SKILL.md updates to match (byte-identity required by check-skill-mirror), plus the .mjs siblings. Verification: - npm test: 238/238 green (was 228; +10 tier-lint tests). - npm run verify:skill-tiers: exit 0 against the live repo. - npm run verify:skill-mirror: still clean (mirror copy updated).
There was a problem hiding this comment.
Code Review
This pull request introduces a new linting utility, check-skill-tiers, to ensure that all skill markdown files, including the core SKILL.md, declare a recognized tier in their YAML frontmatter. The implementation includes the script in both TypeScript and JavaScript, comprehensive unit and integration tests, and a new npm script for verification. The review feedback suggests improving the robustness of the core skill file check by verifying it is a file rather than a directory and optimizing the directory traversal in discoverSkillSources by using withFileTypes: true to reduce system calls.
| try { | ||
| statSync(corePath); | ||
| out.push({ name: CORE_SKILL_NAME, path: corePath }); | ||
| } catch { |
There was a problem hiding this comment.
The statSync call for the core skill doesn't verify that the path is actually a file. If SKILL.md were a directory, statSync would succeed, but the subsequent readFileSync in checkSkillTiers would throw an EISDIR error and crash the script.
try {
const stat = statSync(corePath);
if (stat.isFile()) {
out.push({ name: CORE_SKILL_NAME, path: corePath });
}
} catch {
| try { | ||
| entries = readdirSync(skillsDir); | ||
| } catch { | ||
| return out; | ||
| } | ||
| for (const entry of entries) { | ||
| if (!entry.endsWith(".md")) continue; | ||
| if (entry === "README.md") continue; | ||
| const fullPath = join(skillsDir, entry); | ||
| try { | ||
| const stat = statSync(fullPath); | ||
| if (!stat.isFile()) continue; | ||
| } catch { | ||
| continue; | ||
| } | ||
| out.push({ name: entry.replace(/\.md$/, ""), path: fullPath }); | ||
| } |
There was a problem hiding this comment.
Using readdirSync with { withFileTypes: true } is more efficient as it avoids calling statSync for every file in the loop. This reduces the number of system calls.
try {
const entries = readdirSync(skillsDir, { withFileTypes: true });
for (const entry of entries) {
if (!entry.isFile() || !entry.name.endsWith(".md") || entry.name === "README.md") continue;
out.push({
name: entry.name.replace(/\.md$/, ""),
path: join(skillsDir, entry.name),
});
}
} catch {
return out;
}
Summary
Closes the silent "## Other skills" gap from the tier-rendering generator added in
feat(skills): tier in frontmatter + generator-rendered tier table(already on main). The bundled README groups skills by tier; an untagged skill would slide into a catch-all bucket without anyone noticing in code review. This lint fails the build when any source skill (SKILL.mdorskills/<name>.md) is missing or has an unrecognizedtier:value.The lint caught a real problem on first run: the root
SKILL.mdwas the only source that hadn't been tagged. Addedtier: corein the same commit.What changed
src/bin/check-skill-tiers.mts(new) — purecheckSkillTiers(repoRoot)helper for unit tests, plus a CLImain()that exits 0/1 with a clear failure message. Pattern-matchessrc/bin/check-skill-mirror.mts.src/test/check-skill-tiers.test.mts(new) — 10 tests: missing tier, unrecognized tier, ignoredREADME.md, missingskills/dir, variant-spelling acceptance (FEATURED,tier1,tier-2), and CLI exit-code behavior on synthetic fixtures plus a live-repo no-drift assertion.package.json— addsverify:skill-tiersnpm script alongside the existingverify:skill-mirrorandverify:docs-substringslints.SKILL.md— addstier: core(caught by the lint)..mjssiblings.Test plan
npm testpasses (228 → 238 tests; +10 from the new suite)npm run verify:skill-tiersexits 0:OK skill-tiers: all 12 skill source(s) declare a recognized tier.npm run verify:skill-mirrorstill clean (mirror updated)tier:from any skill, expect non-zero exit)🤖 Generated with Claude Code