diff --git a/package-lock.json b/package-lock.json index 3076e2d..d4233e3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "skillcheck", - "version": "0.5.0", + "version": "0.6.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "skillcheck", - "version": "0.5.0", + "version": "0.6.0", "license": "MIT", "dependencies": { "commander": "^14.0.3", diff --git a/src/checks.ts b/src/checks.ts index e42372c..ff5c384 100644 --- a/src/checks.ts +++ b/src/checks.ts @@ -10,6 +10,7 @@ import type { const MAX_DESCRIPTION_CHARS = 500; const COLLISION_THRESHOLD = 0.6; +const MAX_TOOLS_COUNT = 10; export function runChecks( parsed: ParsedSkill[], @@ -28,6 +29,7 @@ export function runChecks( validated.push(v); diagnostics.push(...checkTools(v, config)); + diagnostics.push(...checkToolsOverloaded(v)); diagnostics.push(...checkDescriptionLength(v)); diagnostics.push(...checkNameDrift(v)); } @@ -94,6 +96,20 @@ function checkTools(v: ValidatedSkill, config: SkillcheckConfig): Diagnostic[] { return out; } +function checkToolsOverloaded(v: ValidatedSkill): Diagnostic[] { + if (v.tools.length >= MAX_TOOLS_COUNT) { + return [ + { + severity: "warn", + rule: "tools-overloaded", + message: `tools: lists ${v.tools.length} tools; narrow the list to the tools this skill actually needs`, + file: v.file, + }, + ]; + } + return []; +} + function checkDescriptionLength(v: ValidatedSkill): Diagnostic[] { if (v.description.length > MAX_DESCRIPTION_CHARS) { return [ diff --git a/src/sarif.ts b/src/sarif.ts index a67d02f..328c4d6 100644 --- a/src/sarif.ts +++ b/src/sarif.ts @@ -79,7 +79,15 @@ export const RULES: readonly SarifRule[] = [ id: "description-collision", name: "descriptionCollision", shortDescription: - "Two skills' descriptions overlap on triggers (Jaccard ≥ 0.6).", + "Two skills' descriptions overlap on triggers (Jaccard >= 0.6).", + helpUri: HELP_BASE, + defaultLevel: "warning", + }, + { + id: "tools-overloaded", + name: "toolsOverloaded", + shortDescription: + "tools: lists too many entries; listing everything defeats the purpose of the tools filter.", helpUri: HELP_BASE, defaultLevel: "warning", }, diff --git a/test/checks.test.ts b/test/checks.test.ts index ccb0968..20047aa 100644 --- a/test/checks.test.ts +++ b/test/checks.test.ts @@ -167,4 +167,38 @@ describe("runChecks", () => { const ds = runChecks([s], config); expect(ds.find((d) => d.rule === "name-drift")).toBeUndefined(); }); + + it("warns when tools lists 10 or more entries", () => { + const tools = ["Read", "Write", "Edit", "Bash", "Glob", "Grep", "WebFetch", "WebSearch", "TodoWrite", "Agent"]; + const s = mkSkill("/test/foo/foo.md", { + name: "foo", + description: "do the foo thing", + tools, + }); + const ds = runChecks([s], config); + expect(ds.some((d) => d.rule === "tools-overloaded")).toBe(true); + }); + + it("does not warn when tools lists 9 entries", () => { + const tools = ["Read", "Write", "Edit", "Bash", "Glob", "Grep", "WebFetch", "WebSearch", "TodoWrite"]; + const s = mkSkill("/test/foo/foo.md", { + name: "foo", + description: "do the foo thing", + tools, + }); + const ds = runChecks([s], config); + expect(ds.find((d) => d.rule === "tools-overloaded")).toBeUndefined(); + }); + + it("tools-overloaded message includes the count", () => { + const tools = ["Read", "Write", "Edit", "Bash", "Glob", "Grep", "WebFetch", "WebSearch", "TodoWrite", "Agent", "ToolSearch"]; + const s = mkSkill("/test/foo/foo.md", { + name: "foo", + description: "do the foo thing", + tools, + }); + const ds = runChecks([s], config); + const d = ds.find((d) => d.rule === "tools-overloaded"); + expect(d?.message).toContain("11"); + }); });